1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
|
#+TITLE: Net Doctor Expansion — the clusters the ladder doesn't yet name
#+AUTHOR: Craig Jennings
#+DATE: 2026-07-11
#+TODO: TODO | DONE
#+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED
* DOING Net Doctor Expansion
:PROPERTIES:
:ID: ce29b103-ed9d-4f56-bf8c-9ed8fe680ff3
:END:
- [2026-07-11 Sat @ 02:30 -0500] DOING — decomposed into build tasks (spec-response Phase 6); parent task in =todo.org= bound by =:SPEC_ID:=. Phase 0 (read-only control-plane probe) is buildable now and =:solo:=; Phases 1-2 are gated on the shared cross-panel privilege model. The systemic connection-name redaction gap is a separate filed task.
- [2026-07-11 Sat @ 02:00 -0500] READY — third skeptical re-review returned Ready with caveats, no blocking findings; all round-2 resolutions verified against the engine. Caveat accepted: Phases 1-2 depend on the shared cross-panel run-time privilege model, which doesn't exist yet, so only Phase 0 (read-only detection) is buildable today. The redaction de-scope (parity + separate systemic task) is the one product call for Craig's eye.
- [2026-07-11 Sat @ 01:45 -0500] DRAFT — round-2 review + response. A skeptical re-review caught two blockers the first round missed (one I introduced): the redaction "copy/--json surface" does not exist, and =rival-manager= as =needs-user-action= could never run its own fix. Both corrected — parity redaction + systemic gap filed separately; all three control-plane verdicts are =fixable=. Auth signal repointed at profile key-mgmt + scan security; masked-NM early-return path named. Findings =[9/9]=, decisions =[3/3]=. Awaiting a third re-review.
- [2026-07-11 Sat @ 01:20 -0500] DRAFT — review incorporated (spec-response). All five findings dispositioned (=[5/5]=), all three decisions accepted and closed (=[3/3]=). Both cookies complete; awaiting a re-review to flip DRAFT → READY. Redaction blocker resolved (later found wrong in round 2).
- [2026-07-11 Sat @ 00:59:30 -0500] DRAFT — reviewed (spec-review). Stays DRAFT: three decisions open plus one =:blocking:= finding (connection names are not redacted today, contra the spec's Security dimension). Design confirmed against the live engine — the two target gaps (=is-enabled= reads, =system-connections= reads) genuinely do not exist yet, so the expansion targets real ground. Findings in =* Review findings=.
- [2026-07-11 Sat @ 00:08:41 -0500] DRAFT — drafted. Extends the existing net doctor (=~/.dotfiles/net/=, shipped) using the network half of the failure taxonomy ([[file:../design/2026-07-10-net-bt-failure-taxonomy.org][2026-07-10-net-bt-failure-taxonomy.org]]). Grounded in a read of the live engine, not memory.
* Metadata
| Field | Value |
|----------+-----------------------------------------------------------------------------------|
| Status | doing |
|----------+-----------------------------------------------------------------------------------|
| Owner | Craig Jennings |
|----------+-----------------------------------------------------------------------------------|
| Reviewer | Craig Jennings |
|----------+-----------------------------------------------------------------------------------|
| Related | [[file:../design/2026-07-10-net-bt-failure-taxonomy.org][net/bt failure taxonomy]] ; the cross-panel run-time-privilege and copy+close tasks |
|----------+-----------------------------------------------------------------------------------|
* Summary
The net doctor is the most mature of the three panel doctors: its probe ladder already walks link → IP → gateway → route → DNS → egress, and its classifier already names most of the taxonomy's failure clusters with the lightest fix. This spec closes the two clusters it does *not* reach — control-plane conflicts (two network managers fighting over one interface, a masked NetworkManager, a keyfile the daemon silently refuses) and the sharper naming of the terminal auth cluster — and adopts the cross-panel run-time privilege model. It is an expansion of a working doctor, not a rewrite.
* Problem / Context
The failure taxonomy sorted ~74 real network failure modes into eight symptom clusters. Read against the live classifier (=~/.dotfiles/net/src/net/classify.py=), the net doctor already reaches six of them: cluster 1 (=rfkill=, =manage-device=), cluster 2 (the DHCP-failed path), cluster 3 (=tunnel-down=, =vpn-policy=), cluster 5 (=resolved-restart=, =dns-test=), and cluster 6 (=portal=, =clock-sync=, =proxy=, and the =upstream-not-local= terminal STOP). Those are =classify='s real action identifiers; a few taxonomy labels (no-hardware, DHCP-failed, DNS-not-resolving) are message text the actions carry, not separate verdicts, and =dns-override= is a doctor =FIX_CHAIN=, not a classifier action. The classifier's terminal-first ordering already refuses to loop repairs against a wrong password, a held portal, or a VPN-owned route. That is a lot of the taxonomy, already built.
Two clusters fall through.
** The control plane can be broken while the radio looks fine (cluster 7)
The taxonomy's largest untouched cluster is NetworkManager itself. When =dhcpcd.service= runs beside NM's internal DHCP client, or =systemd-networkd= and NM both claim one link, or =iwd= and =wpa_supplicant= are both active, the interface flaps or never leases — and every existing probe reads a plausible-looking radio with no verdict that names the fight. NM masked (=systemctl start= returns "Unit is masked") reads as "NetworkManager isn't running" today, which points =nm-restart= at a service that cannot start. A =.nmconnection= keyfile that isn't =600= root-owned is silently skipped by the daemon, so a saved network "just won't connect" with no signal the doctor surfaces. These are distinct root causes with distinct fixes, and no current probe detects any of them. The classifier does carry granular control-plane actions (=reset=, =bounce=, =rfkill=, =nm-restart=), but =nm-restart= is narrowly the dead-service restart, and nothing reads =systemctl is-enabled= or the =system-connections= keyfiles — the reads that would surface a masked NM, a rival manager, or a bad keyfile. The gap is missing detection, not one verdict overloaded across three faults.
** The auth cluster is named too coarsely (cluster 4)
=classify= detects an auth failure from NM state 120 / GENERAL.REASON and returns =needs-user-action= — correct, and correctly terminal. But the taxonomy shows the auth cluster is not one failure: a pure-WPA3/SAE association failure, a hidden SSID never probed, an enterprise cert mismatch, and a wrong regulatory domain are different faults with different next actions, and some (SAE key-mgmt, =wifi.hidden yes=) are one-line profile fixes rather than "re-enter the password." The doctor collapses them all to one message.
** The flaky/drops cluster needs signal the one-shot doctor doesn't collect (cluster 8)
Powersave disconnects, roaming stalls, USB autosuspend, no-reconnect-after-resume, firmware crashloops — these are intermittent, and a single-shot "why am I offline right now" probe cannot see them. Naming them needs event-log correlation the doctor doesn't do. This cluster is real but out of v1; it is named here so the boundary is explicit.
* Goals and Non-Goals
** Goals
- Add a control-plane cluster: a probe that detects a second network/DHCP manager active alongside NetworkManager, a masked/failed NM distinct from a merely-stopped one, and a keyfile-permission fault, each with its own verdict and the lightest fix.
- Sharpen the auth verdict: extract the specific auth cluster cause (SAE, hidden SSID, enterprise cert, regdom) so =needs-user-action= names the real next step, and apply the one-line profile fix where one exists.
- Adopt the cross-panel run-time privilege model (Auto / Privileged / Reboot-tail / Guide, resolved from =sudo -n= + tty + GUI), so control-plane repairs that need root (=systemctl disable dhcpcd=, =chmod= a keyfile, =systemctl unmask=) run under the same Confirm/Arm-default floor the audio doctor defined.
** Non-Goals
- Rebuilding the existing ladder. Clusters 1/2/3/5/6 stay exactly as they classify today; this spec only adds where the taxonomy shows a gap.
- The flaky/drops cluster (cluster 8). Naming intermittent faults needs event-log correlation that a one-shot doctor doesn't do. Logged to =todo.org=, not built here.
- Turning the doctor into a NetworkManager profile editor. The auth-cluster fixes are limited to the one-line profile settings that get a stuck association online (SAE key-mgmt, hidden flag); it will not manage certificates, enterprise identities, or credential entry — those stay Guide.
- Silent privileged action. Every Privileged/Reboot-tail control-plane repair defaults to Confirm/Arm, never silent Auto — same stance as the audio spec.
** Scope tiers
- *v1:* the control-plane conflict probe and its verdicts (rival-manager, NM-masked, keyfile-perms); sharper auth-cluster reason extraction plus the SAE/hidden one-line fixes; adoption of the run-time privilege model for the new root-needing repairs.
- *Out of scope:* the flaky/drops cluster; certificate/enterprise credential management; any change to the six clusters already classified.
- *vNext:* event-log correlation for cluster 8 (powersave/roam/suspend/autosuspend drop signatures); a DoT/DNSSEC-specific verdict distinguishing "the venue resolver mangles DNSSEC" from the generic DNS-not-resolving; per-profile autoconnect/duplicate-profile hygiene. All logged to =todo.org=.
* Design
** For the user
Nothing changes for the six clusters that already work. What changes is that two failure shapes that today produce a wrong or vague verdict start naming themselves.
When a second manager is fighting NetworkManager for the link, the wall says so by name — "dhcpcd is running alongside NetworkManager and they're fighting over the interface" — and offers the fix (stop the rival) rather than bouncing a connection that will flap again the moment the other daemon re-grabs it. When NM is masked, the verdict distinguishes "masked — it can't start until unmasked" from "stopped," so FIX unmasks rather than uselessly restarting. When a saved network silently won't load because its keyfile is world-readable, the doctor names the permission fault instead of leaving the user to wonder why a known-good network never activates.
When association fails on auth, the wall stops saying only "authentication failed" and names which auth: a WPA3-only network the profile isn't set for, a hidden SSID that needs the hidden flag, an enterprise network missing its CA cert. The first two carry a one-press fix; the rest tell the user the specific thing to supply.
** For the implementer
*** The control-plane probe
A new read-only probe tier, beside the existing ones, answering three questions the current ladder never asks:
1. *Is a rival manager active?* Check whether =dhcpcd.service=, =systemd-networkd.service=, or a standalone =iwd.service= is active while NetworkManager is also active and owns (or wants to own) the link. This is a =systemctl is-active= read plus NM's backend setting — bounded, no hang. A rival that is active is the verdict; the fix is =systemctl disable --now <rival>= (Privileged), never a connection bounce.
2. *Is NM masked or failed, as distinct from stopped?* =systemctl is-enabled NetworkManager= returns =masked=; the unit's =ActiveState=/=Result= distinguishes a crash-loop from a clean stop. A masked NM gets an =unmask= verdict (Privileged), not the existing =nm-restart=.
3. *Does the active profile's keyfile have the wrong permissions?* For the selected connection, stat its =/etc/NetworkManager/system-connections/*.nmconnection=; a non-=600= or non-root-owned file is the silent-skip fault. Fix: =chmod 600= + =chown root= (Privileged).
The probe runs before the existing "NetworkManager isn't running" rule, because a masked NM and a rival-manager fight are both more specific than "not running" and would otherwise be mis-verdicted by it. One ordering subtlety verified against the engine: a fully-down NM makes =nmcli= raise and =diagnose= early-returns with only the service and link steps (=diag.py:503-511=), so the masked/failed check has to run inside that early-return path — otherwise a masked NM short-circuits before the new probe and never reaches its verdict.
*** The classifier gains control-plane verdicts
=classify= adds, in specificity order ahead of the generic =nm-restart= rule: =rival-manager= (fixable → Privileged disable-rival), =nm-masked= (fixable → unmask), =keyfile-perms= (fixable → chmod/chown). All three are =fixable= — the doctor only applies a repair when the outcome is =fixable= (=doctor.py:181=), so a terminal =needs-user-action= would never run its fix — and each is Privileged, so =net doctor --fix= still gates it on the Confirm floor rather than acting silently. Each carries evidence naming the specific rival/unit/file. These are additive; the existing rules below them are untouched.
*** The auth verdict takes a reason
The specific auth cause is not in =auth_failed_reason= — that REASON/journal string (=doctor.py:32-56=) only marks *that* auth failed, not whether it was SAE, a hidden SSID, or an enterprise cert. The distinction is read from signals the engine already has: the active profile's key-mgmt (=manage.py:36-47=, which already detects WPA3/SAE incompatibility) and the scanned network's SECURITY flags (=nmcli.py:164=). From those, =gather_context= derives a small classifier: SAE/PMF, hidden-SSID, enterprise-cert, or generic-PSK. (regdom has no engine signal — grep-clean — so it stays Guide.) The =needs-user-action= message is keyed off it. For SAE and hidden-SSID — the two with a deterministic one-line profile fix — the verdict becomes =fixable= with a Privileged/Auto profile-modify action (=key-mgmt sae= + PMF, or =wifi.hidden yes=) rather than terminal. The rest stay =needs-user-action= with a sharpened message.
*** The privilege model
The new repairs (=systemctl disable <rival>=, =unmask=, =chmod=/=chown= a keyfile, profile-modify) are the net doctor's first root-needing doctor repairs beyond the ones already in =priv.py=. They adopt the cross-panel run-time resolution the audio spec defined: Privileged remedies run silently where passwordless sudo exists (every archsetup install), prompt on a CLI with a tty, and default to Confirm/Arm — never silent Auto. This is the same standard, not a net-specific one; the shared implementation is the tracked cross-panel task. Concretely, the three fixes register through =priv.py='s =VERBS= table and =repair.py='s =ACTIONS= registry (=disable-rival=, =unmask-nm=, =chmod-keyfile=) — the existing dispatch path, not a new one — and each verb is a narrowly-scoped command (disable one named unit, unmask NetworkManager, chmod/chown one keyfile), never a general run-as-root, so the passwordless-sudo grant stays tight.
*** Redaction of the new evidence
The new verdicts surface a connection name (=rival-manager=, =keyfile-perms=) and a =/etc/NetworkManager/system-connections/*.nmconnection= basename. The redaction reality, verified against the engine: SSID redaction exists only in the event log (=redact_event=, gated on =redact_ssid=, default off); the copyable report scrubs MAC/IP only (=scrub_text=, =redact.py=), and =--json= is a raw =json.dumps(out)= with no redaction at all (=cli.py=). Connection names already appear in the clear in the link-step evidence (=diag.py:103=, e.g. "wlan0 connected (HomeNetwork)") and in =--json= today. So there is no copy-vs-wall redaction surface to piggyback on, and the new verdicts add no new leak class — a connection name already shows for the link step. v1 keeps parity: the new evidence is redacted exactly as the existing link-step evidence is (MAC/IP via =scrub_text=; connection name in the clear). The systemic gap — connection names and SSIDs leaking into the copyable report and =--json= across every step, gated behind a default-off toggle — predates this spec and spans the whole engine, so it is filed as its own task rather than half-solved for two new verdicts. The keyfile probe reads permissions, not secrets; the auth-reason extraction reads NM's reason string, not the PSK.
* Alternatives Considered
** Fold the control-plane faults into the existing =nm-restart= verdict
- Good, because it is zero new classifier surface.
- Bad, because =nm-restart= is the wrong fix for all three: restarting a masked NM fails, restarting NM does not stop a rival =dhcpcd=, and it does nothing for a bad keyfile. A shared verdict would send the doctor's one fix at three faults it can't fix.
- Rejected. Different root causes with different fixes are different verdicts — the same principle the audio spec used for =pulse-hung= vs =pulse-down=.
** Make the doctor a full profile editor for the auth cluster
- Good, because it could fix more auth failures automatically.
- Bad, because credential and certificate entry is a genuine user decision, not a repair — the doctor cannot invent an enterprise CA or a password. Auto-editing profiles beyond the two deterministic one-liners risks writing a wrong setting the user then has to unwind.
- Rejected in favor of fixing only SAE key-mgmt and the hidden flag, and guiding the rest.
** Build the flaky/drops cluster now with a synthetic re-probe
- Good, because it would catch powersave/autosuspend "works then dies" cases.
- Bad, because a one-shot doctor invoked when the user is already offline has no drop history to read; catching intermittent faults needs the panel's event log correlated over time, which is a separate probe surface.
- Rejected for v1; logged as vNext.
* Decisions [3/3]
** DONE The control-plane probe and its three verdicts
Context: cluster 7 is the taxonomy's largest untouched cluster, and no current probe detects a masked NM, a rival manager, or a bad keyfile.
Decision: we will add a read-only control-plane probe (rival-manager active-check, NM masked-vs-failed-vs-stopped, active-profile keyfile permissions) and three verdicts ahead of the generic =nm-restart= rule, each with the lightest specific fix.
Consequences: the doctor names the fight instead of bouncing a link that will re-flap; harder — three new verdicts and a probe that reads =systemctl= state plus a stat, and the ordering has to sit ahead of the existing not-running rule without disturbing it.
Resolution: accepted as proposed (the fold-into-nm-restart alternative was rejected — that fix cannot address any of the three faults). Owner: Craig.
** DONE How far the auth-cluster fix goes
Context: the auth cluster is terminal today; some members (SAE, hidden SSID) have deterministic one-line profile fixes, others (enterprise cert, credential) do not.
Decision: we will extract the specific auth reason and make only SAE-key-mgmt and hidden-flag =fixable=; everything else stays =needs-user-action= with a sharpened, cause-named message. Both profile-modify fixes are Confirm-tier (persisted state).
Consequences: two more auth failures self-heal; harder — the doctor now writes to a connection profile, which is a heavier action than a bounce, and the boundary between "fix" and "guide" inside one cluster has to be defended so it doesn't creep into credential management (the Non-Goal is the guardrail).
Resolution: accepted as proposed. Owner: Craig.
** DONE Adopt the run-time privilege model as the cross-panel standard
Context: the new control-plane repairs need root; the audio spec already defined the four-class run-time model and made it a cross-panel standard.
Decision: we will adopt it verbatim — Privileged repairs run where passwordless sudo exists, prompt on a tty, default to Confirm/Arm, never silent Auto — sharing the implementation with the other panels rather than reimplementing it.
Consequences: the net doctor's root repairs are consistent with audio/bt/maint; harder — it couples this spec to the shared privilege-model task, so the ordering across panels has to be settled (which panel lands the shared code). That sequencing caveat is recorded in Risks.
Resolution: accepted — this adopts an already-decided cross-panel standard, not a new choice. Owner: Craig.
* Review findings [9/9]
** DONE Connection names are not redacted today, but the spec assumes they are :blocking:
The Security & privacy dimension stated "SSIDs and connection names are already redacted by =redact.py=." Verified against the live engine: =redact.py= redacts SSID, MAC, IP, secret-keys, and =portal_url= only (=redact.py:9-53=) — there is no connection-name redaction. The new =rival-manager= and =keyfile-perms= verdicts surface a connection name and a =/etc/NetworkManager/system-connections/*.nmconnection= path (the file basename is the connection name) into the wall and the =--json= output.
Disposition: accepted, modified in one detail. Rather than redact everywhere (which would blank the network name on the user's own screen), v1 redacts the connection name and keyfile basename at the same copy/=--json= surface the existing SSID redaction covers — the shareable text is scrubbed, the on-screen wall still names the network. Folded into Design "Redaction of the new evidence," the Security & privacy dimension, Phase 1, and a new acceptance criterion.
** DONE "One =nm-restart= verdict covers the whole control plane" overstates current scope
Problem/Context and the first Alternative rested the motivation on the claim that the doctor "currently has one verdict (=nm-restart=) covering the whole control plane." Verified: =nm-restart= is one narrow action for a dead NetworkManager *service* only (=classify.py:96-97=, =repair.py:705=); the classifier already carries granular actions (=reset=, =bounce=, =rfkill=, =manage-device=, =resolved-restart=…). The real, verified gap is that *no* probe detects a rival manager, a masked NM, or a bad keyfile (=is-enabled= and =system-connections= reads are grep-clean).
Disposition: accepted. Reworded the cluster-7 paragraph to rest on the missing detections; the first Alternative already argues against reusing =nm-restart= as a fix, which stays valid.
** DONE "=systemctl is-enabled= already used by the engine" is inaccurate
External APIs & deps said =systemctl is-active/is-enabled= are "already used by the engine." Verified: only =is-active= is used (=cmd.py:27=); =is-enabled= has zero hits, and =system-connections= is not read today either.
Disposition: accepted. Corrected the dimension to say =is-active= and =nmcli= are used today, =is-enabled= and the keyfile reads/=stat= are new bounded calls.
** DONE Problem/Context lists message text as if it were classifier verdicts
The six-cluster evidence listed =no-hardware=, =DHCP-failed=, =DNS-not-resolving=, =dns-override= as classifier verdicts. Verified: the first three are message text, and =dns-override= is a doctor =FIX_CHAIN= (=doctor.py:24=), not a =classify= action.
Disposition: accepted. Rewrote the listing to cite =classify='s real action identifiers and note which taxonomy labels are message text.
** DONE Name the =priv.py= / =repair.py= integration point for the new privileged repairs
The spec said it reuses =priv.py= but the phase plan did not name the concrete integration: privileged repairs dispatch through =priv.py='s =VERBS= table (=priv.py:150=) and =repair.py='s =ACTIONS= registry (=repair.py:695=), not a new path.
Disposition: accepted, extended. Phase 1 and the privilege-model design now name =disable-rival=/=unmask-nm=/=chmod-keyfile= as =VERBS= + =ACTIONS= entries, and add that each verb is a narrowly-scoped command (not a general run-as-root) so the passwordless-sudo grant stays tight.
** DONE Round 2 (skeptical review): the redaction "copy/--json surface" does not exist :blocking:
The round-1 disposition claimed v1 would redact at "the same copy/=--json= surface the existing SSID redaction covers." A second skeptical review traced every redaction call site and found no such surface: SSID redaction lives only in =redact_event= (event log, gated on =redact_ssid=, default off), =--json= is a raw =json.dumps(out)= with no redaction, and the copyable report scrubs MAC/IP only (=scrub_text=). Connection names already appear in the clear in the link-step evidence (=diag.py:103=) today. So the round-1 resolution invented a surface that isn't there.
Disposition: accepted — the round-1 fix was wrong and is corrected. v1 keeps parity with existing link-step behavior (the new verdicts leak no more than the link step already does); the systemic connection-name/SSID redaction gap across the report and =--json= is filed as a separate task. Rewrote Design "Redaction of the new evidence," the Security dimension, Phase 1, and the acceptance criterion.
** DONE Round 2 (skeptical review): rival-manager can't satisfy its own FIX criterion :blocking:
Round 1 marked =rival-manager= =needs-user-action= (terminal) while the acceptance criterion required FIX to stop the rival. But the doctor only applies a repair when the outcome is =fixable= (=doctor.py:181=), so a terminal =rival-manager= would never run =disable-rival=.
Disposition: accepted. =rival-manager= is now =fixable= (Privileged/Confirm), consistent with =nm-masked= and =keyfile-perms=; the Confirm floor still gates it. Corrected the classifier design and added an acceptance criterion that all three control-plane verdicts are =fixable=.
** DONE Round 2 (skeptical review): auth-cluster classifier pointed at the wrong signal
Round 1 said =auth_failed_reason= would carry the SAE/hidden/enterprise distinction. The skeptical review confirmed that REASON/journal string only marks *that* auth failed; the actual discriminating data lives in the profile key-mgmt (=manage.py:36-47=, already detects SAE incompatibility) and the scanned SECURITY flags (=nmcli.py:164=).
Disposition: accepted. Phase 2 / the auth-verdict design now reads those signals instead of =auth_failed_reason=; regdom stays Guide (no engine signal). No new data collection is forced.
** DONE Round 2 (skeptical review): masked-NM must be reached in the NM-down early-return
When NM is fully down, =nmcli= raises and =diagnose= early-returns with only the service and link steps (=diag.py:503-511=), so a masked NM would short-circuit before the control-plane probe and never reach the new verdict.
Disposition: accepted. The control-plane probe design now states the masked/failed check must run inside that early-return path.
* Implementation phases
Each phase leaves the tree green and independently useful, as the existing net phases did.
** TODO Phase 0 — the control-plane probe (read-only)
Pure engine, no classifier changes. A probe module that reports rival-manager state, NM masked/failed/stopped, and active-profile keyfile permissions into the diag context. =net diag --json= shows the new signals. Fakes: injected =systemctl is-active/is-enabled= results and a temp system-connections tree.
** TODO Phase 1 — control-plane verdicts + the privilege model
=classify= gains =rival-manager=, =nm-masked=, =keyfile-perms= (all =fixable=), ordered ahead of the generic not-running rule, with the masked/failed check reachable in the NM-down early-return path. The run-time privilege resolution lands (shared with the cross-panel task) and the three new fixes register as Privileged/Confirm through =priv.py='s =VERBS= table and =repair.py='s =ACTIONS= registry (=disable-rival=, =unmask-nm=, =chmod-keyfile=), each a narrowly-scoped verb. The new evidence keeps parity with existing redaction (MAC/IP via =scrub_text=); the systemic connection-name gap is a separate task, not this phase. =net doctor= names them; =net doctor --fix= applies them under the Confirm floor. Hard ordering gate: this phase must not land before the shared run-time privilege model exists — =priv.py= today is a bare =VERBS=+sudo dispatcher with no Confirm/Arm resolution (=priv.py:150=) and =_attempt= runs repairs ungated, so shipping the Privileged =fixable= verdicts first would let =net doctor --fix= silently disable =dhcpcd= via passwordless sudo, the exact outcome the model forbids. Phase 0 (read-only detection) has no such dependency and lands first; if the fault-naming is wanted before the privilege model, the verdicts can ship detection-only (no =--fix= action) as an interim slice.
** TODO Phase 2 — the sharpened auth verdict
The auth-reason classifier; SAE and hidden-SSID become =fixable= profile-modify repairs; the rest get cause-named =needs-user-action= messages. Pairwise over (reason × profile-state).
** TODO Phase 3 — flip this spec to IMPLEMENTED
And log the vNext items (flaky/drops cluster, DoT/DNSSEC verdict, profile hygiene) to =todo.org=.
* Acceptance criteria
- [ ] With =dhcpcd.service= active alongside NetworkManager, the doctor reports =rival-manager= naming dhcpcd, and FIX stops it rather than bouncing the connection.
- [ ] A masked NetworkManager reports =nm-masked= (distinct from stopped), and FIX unmasks it.
- [ ] A non-600 keyfile for the active profile reports =keyfile-perms=, and FIX corrects the permissions.
- [ ] A WPA3-only association failure reports the SAE cause and (with --fix) sets the profile's key-mgmt, rather than saying only "authentication failed."
- [ ] An enterprise-cert auth failure stays =needs-user-action= but names the missing CA cert.
- [ ] Every new root-needing repair defaults to Confirm/Arm and never runs silently as Auto.
- [ ] =rival-manager=, =nm-masked=, and =keyfile-perms= are all =fixable= outcomes, so =net doctor --fix= actually runs their repairs (a terminal outcome would be skipped by =doctor.py:181=).
- [ ] The =rival-manager= and =keyfile-perms= verdicts expose the connection name no more than the existing link-step evidence does (MAC/IP scrubbed, connection name in the clear — parity, not a new leak). The systemic connection-name/=--json= redaction gap is tracked as a separate task.
- [ ] The six existing clusters classify exactly as they do today (regression).
* Readiness dimensions
- *Data model & ownership* — the diag context gains control-plane signals (generated per-probe) and a richer =auth_failed_reason= (generated from GENERAL.REASON + journal, as today). The doctor never writes NM config except the two auth profile-modifies and the keyfile-perms fix, all under the Confirm floor.
- *Errors, empty states & failure* — an unreadable =systemctl=/=stat= yields "unknown," never a false rival/masked/perms verdict (the safe direction). Partial reads degrade to the existing behavior.
- *Security & privacy* — verified: SSID redaction exists only in the event log (=redact_event=, gated on =redact_ssid=, default off); the copyable report scrubs MAC/IP only (=scrub_text=) and =--json= is raw. Connection names already appear in the clear in the link-step evidence and =--json= today, so the new =rival-manager=/=keyfile-perms= verdicts add no new leak class — they reach parity with existing behavior. The systemic connection-name/SSID redaction gap across the report and =--json= is pre-existing and filed as a separate task. The keyfile probe reads permissions, not secrets; the auth-reason extraction reads NM's reason string, not the PSK.
- *Observability* — the wall names the specific rival/unit/file. =--json= carries the new context.
- *Performance & scale* — three =systemctl= reads and a stat; negligible beside the existing probe cost.
- *Reuse & lost opportunities* — reuses =gather_context='s existing auth-reason extraction, =priv.py=, and the shared cross-panel privilege model rather than a net-local one. =classify= stays the single verdict authority.
- *Architecture fit* — the new probe is additive beside the existing tiers; the classifier additions sit ahead of the generic not-running rule and leave the rest untouched. Weak point: ordering — the new verdicts must precede =nm-restart= or a masked/rival case mis-classifies.
- *Config surface* — none new. N/A.
- *Documentation plan* — module docstrings, as the package does today. The wall is the user documentation.
- *Dev tooling* — =make test= and the net panel smoke cover it; the control-plane probe needs injected =systemctl= state, a fixture shape the package already uses elsewhere.
- *Rollout, compatibility & rollback* — additive; =net doctor= with no new fault behaves as today. The auth profile-modify and keyfile-perms fixes change persisted NM state, so both are Confirm-tier and reversible by the user.
- *External APIs & deps* — =systemctl is-active= and =nmcli= are already used by the engine; =systemctl is-enabled= and reads/=stat= under =/etc/NetworkManager/system-connections= are new calls (bounded with an explicit timeout, like every existing probe). No new packages. The exact rival-manager set (dhcpcd/networkd/iwd) is verified against the live system before Phase 0.
* Risks, rabbit holes, and drawbacks
The rival-manager check can false-positive if a rival service is active but not actually contending for the same interface (e.g. =systemd-networkd= managing a container bridge while NM owns wifi). The probe must scope the conflict to the link the doctor is diagnosing, not merely "is networkd running" — otherwise it cries wolf on a legitimate split. This is the main correctness rabbit hole and wants a test with a bridge-only networkd.
The auth-cluster boundary between "fix" and "guide" is a slope. SAE and the hidden flag are safe because they are deterministic and reversible; the temptation is to add "just one more" auto-fix until the doctor is editing enterprise profiles it shouldn't. The Non-Goal is the guardrail; hold it.
Coupling to the shared privilege-model task means this spec can't fully land until that model exists somewhere. Sequence it: whichever panel lands the shared code first, the others depend on it.
* Review and iteration history
** 2026-07-11 Sat @ 00:08:41 -0500 — Craig Jennings — Author
- What: drafted the net doctor expansion from the network half of the failure taxonomy. v1 adds the control-plane cluster (rival-manager, NM-masked, keyfile-perms) and sharpens the auth verdict; the flaky/drops cluster is staged to vNext.
- Why: the taxonomy showed the net doctor already reaches six of eight clusters, and the two it misses (control plane, auth naming) are where its verdicts are wrong or vague today.
- Artifacts: [[file:../design/2026-07-10-net-bt-failure-taxonomy.org][the net/bt failure taxonomy]]; code read across =~/.dotfiles/net/src/net/= (=classify.py=, =diag.py=, =doctor.py=, =repair.py=, =priv.py=).
** 2026-07-11 Sat @ 00:59:30 -0500 — Claude Code (archsetup) — Reviewer
- What: ran spec-review. Rubric =Not ready=. Recorded five findings, one =:blocking:= (connection-name redaction assumed but absent) and four non-blocking accuracy/integration corrections. The three proposed decisions remain open.
- Why: the design is sound and the code read confirmed both target gaps are real (=is-enabled= and =system-connections= reads do not exist in the engine today), so the expansion is well-grounded. What holds the rubric is the privacy claim the spec states as already-true (=redact.py= redacts SSID/MAC/IP/secret-keys/portal_url only — not connection names, =redact.py:9-53=) and the still-open decisions. Once the redaction finding is dispositioned and Craig accepts the three decisions, this reaches =Ready with caveats= (the cross-panel shared privilege-model code is the remaining sequencing caveat, already recorded in Risks).
- Artifacts: engine facts verified across =classify.py= (:96-97, :55-62), =diag.py= (:518-537, bounded probes), =doctor.py= (:112-116 auth-reason), =cmd.py:27= (=is-active= only), =priv.py:150=/=repair.py:695= (dispatch registry), =redact.py:9-53=. Findings in =* Review findings=.
** 2026-07-11 Sat @ 01:20:00 -0500 — Claude Code (archsetup) — Responder
- What: ran spec-response. Dispositioned all five findings (four accepts, one accept-with-modify: the redaction scrubs at the copy/=--json= surface, not the on-screen wall) and closed all three decisions as accepted. Folded the redaction requirement into Design, the Security dimension, Phase 1, and a new acceptance criterion; reworded the =nm-restart=/control-plane motivation and the six-cluster evidence onto the verified facts; named the =VERBS=/=ACTIONS= dispatch integration and tightened the new verbs to narrowly-scoped commands. Both =[/]= cookies now read complete.
- Why: convergence toward implementation-ready. The redaction modify avoids blanking the network name on the user's own screen while still scrubbing the shareable text — a connection is usually named after its SSID, so leaving it unredacted leaks the already-redacted SSID by a side channel.
- Artifacts: findings =[5/5]=, decisions =[3/3]=. Scope expanded only by the redaction obligation, which is filed as an acceptance criterion and a Phase 1 deliverable, so the readiness rubric holds. Awaiting re-review to flip DRAFT → READY.
** 2026-07-11 Sat @ 01:45:00 -0500 — Claude Code (archsetup) — Reviewer + Responder (round 2)
- What: a skeptical adversarial re-review returned Not ready with two blockers, both in the parts round 1 claimed to resolve. (1) The redaction "copy/--json surface" I asserted in round 1 does not exist — SSID redaction is event-log-only, =--json= is raw, the copyable report scrubs MAC/IP only. (2) =rival-manager= marked =needs-user-action= could never run its fix, since the doctor only repairs =fixable= outcomes (=doctor.py:181=). Fixed both, plus two non-blocking corrections (auth signal repointed at =manage.py:36-47= + =nmcli.py:164=; masked-NM early-return path). Findings now =[9/9]=.
- Why: the first round introduced a factual error by asserting a redaction surface without verifying =redact.py='s structure. Round 2 verified every claim against the engine before writing. The redaction resolution is now a de-scope: v1 keeps parity with the pre-existing link-step behavior and the systemic redaction gap is a separate task — a scope call worth Craig's eye.
- Artifacts: engine re-verified at =doctor.py:181= (fix gate), =cli.py= (raw =--json=), =report.py= / =redact.py= (MAC-IP-only scrub), =diag.py:103= (link evidence), =diag.py:503-511= (NM-down early return), =manage.py:36-47= / =nmcli.py:164= (auth signals). Awaiting a third re-review.
** 2026-07-11 Sat @ 02:00:00 -0500 — Claude Code (archsetup) — Reviewer (round 3)
- What: third skeptical adversarial re-review. Verdict =Ready with caveats=, no blocking findings. Verified all five round-2 resolutions against the engine: redaction parity is coherent and buildable (no criterion assumes unbuilt redaction), rival-manager as =fixable= preserves terminal-first ordering with no loop (=doctor.py:184-195=), the auth signals (=manage.py:46= SAE, =nmcli.py:159-176= hidden/SECURITY) distinguish all three cases, and the masked-NM check is addable in the early-return path (=diag.py:503-511=). Flipped DRAFT → READY.
- Why: the loop terminates at the rubric, not at exhaustion. Two independent skeptical passes plus a code re-verification found no remaining blocker. The one named caveat — Phase 1 needs the shared privilege model or =--fix= runs ungated (=priv.py:150=, =_attempt= ungated) — is honestly recorded in Risks, Decision 3, and now a hard ordering gate on Phase 1; Phase 0 is fully buildable now.
- Artifacts: findings =[9/9]=, decisions =[3/3]=. Non-blocking: keyfile-perms on an inactive profile names a connection the disconnected link step wouldn't — same data class, covered by the separate redaction task.
|