aboutsummaryrefslogtreecommitdiff
path: root/docs
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-11 01:59:56 -0500
committerCraig Jennings <c@cjennings.net>2026-07-11 01:59:56 -0500
commit31d4475b0562227f7d3fd385f087c6a1edf6e920 (patch)
tree34693400c1bc07fa985e7382c30adf7e29535782 /docs
parentdae873709af60ed63ab0e3b766b8e5551c260fc1 (diff)
downloadarchsetup-31d4475b0562227f7d3fd385f087c6a1edf6e920.tar.gz
archsetup-31d4475b0562227f7d3fd385f087c6a1edf6e920.zip
docs: take net + bt doctor specs to READY after review rounds
I ran spec-response on both, then two more skeptical review rounds until each reached Ready with caveats, no blocking findings. I verified every current-behavior claim against the live engine before each response. The skeptical passes earned their place by catching real blockers the first round missed. Net: my round-1 "redaction copy/--json surface" was invented. That surface doesn't exist: SSID redaction is event-log-only and --json is raw. I re-resolved it as parity with the existing link-step behavior, plus a separate task for the systemic gap. I also made all three control-plane verdicts fixable, since a terminal outcome would never run its own fix (doctor.py:181). And I repointed the auth classifier at the profile key-mgmt and scan-security signals that carry the SAE/hidden distinction. Bt: the AutoEnable default is true, not false (bluez 5.87), so round 1 had the "dead every boot" premise backwards and would false-positive on healthy machines. The fault now fires only on an explicit AutoEnable=false, a disabled service, or a TLP entry. The verdict "code" is an additive schema.step key, not one that exists today. Both specs carry one caveat: Phases 1-2 need the shared cross-panel run-time privilege model, which doesn't exist yet. A hard ordering gate now sits on each: shipping the Privileged verdicts before the Confirm floor would let --fix run root ops via passwordless sudo ungated. Only Phase 0 is buildable today.
Diffstat (limited to 'docs')
-rw-r--r--docs/specs/2026-07-11-bt-doctor-expansion-spec.org111
-rw-r--r--docs/specs/2026-07-11-net-doctor-expansion-spec.org115
2 files changed, 158 insertions, 68 deletions
diff --git a/docs/specs/2026-07-11-bt-doctor-expansion-spec.org b/docs/specs/2026-07-11-bt-doctor-expansion-spec.org
index bced674..77ee46d 100644
--- a/docs/specs/2026-07-11-bt-doctor-expansion-spec.org
+++ b/docs/specs/2026-07-11-bt-doctor-expansion-spec.org
@@ -4,10 +4,13 @@
#+TODO: TODO | DONE
#+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED
-* DRAFT Bt Doctor Expansion
+* READY Bt Doctor Expansion
:PROPERTIES:
:ID: 3d4d61c4-e5df-44e9-b8e0-40b31452c3f7
:END:
+- [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: Phase 2 (the persistent fix) depends on the shared cross-panel privilege model, which doesn't exist yet; Phase 0 (both read-only probes) and Phase 1 (the firmware-hint Guide) are buildable today.
+- [2026-07-11 Sat @ 01:45 -0500] DRAFT — round-2 review + response. A skeptical re-review caught a real blocker: the AutoEnable default is =true=, not =false= (verified, bluez 5.87), so round 1's "dead every boot on absent config" premise was backwards and would fire a false positive on healthy machines. Corrected — the fault fires only on explicit AutoEnable=false / disabled service / TLP. Also folded three non-blocking corrections (the =code= key is additive-not-existing; the INI-setter is real Phase 2 work; the sequencing caveat now actually in Risks). Findings =[8/8]=, decisions =[3/3]=. Awaiting a third re-review.
+- [2026-07-11 Sat @ 01:20 -0500] DRAFT — review incorporated (spec-response). All four findings dispositioned (=[4/4]=), all three decisions accepted and closed (=[3/3]=). The verdict-representation blocker resolved by defining a "verdict" as a step outcome in the existing status model.
- [2026-07-11 Sat @ 00:59:30 -0500] DRAFT — reviewed (spec-review). Stays DRAFT: three decisions open plus one =:blocking:= finding (the bt doctor has no named-verdict layer, so the proposed =no-adapter-firmware=/=powered-off-persistent= verdicts need their representation defined first). Design confirmed against the live engine — the two target gaps (kernel-log firmware hint, boot-persistence read) genuinely do not exist yet. Findings in =* Review findings=.
- [2026-07-11 Sat @ 00:08:41 -0500] DRAFT — drafted. Extends the existing bt doctor (=~/.dotfiles/bluetooth/=, shipped) using the bluetooth 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.
@@ -15,7 +18,7 @@
| Field | Value |
|----------+-----------------------------------------------------------------------------------|
-| Status | draft |
+| Status | ready |
|----------+-----------------------------------------------------------------------------------|
| Owner | Craig Jennings |
|----------+-----------------------------------------------------------------------------------|
@@ -38,7 +41,7 @@ The taxonomy sorted ~55 real bluetooth failure modes into five clusters keyed to
** A boot-disabled adapter reads as merely powered-off (cluster 2)
-=_powered_step= checks whether the adapter is powered *now* and offers =power-on=. But =power-on= via bluetoothctl doesn't persist: on an =AutoEnable=false= machine (bluez's static default when main.conf is absent), or a TLP laptop that disables bluetooth on startup, the adapter is dead again next boot. The doctor fixes the symptom every session and never names the cause. The taxonomy's cluster 2 has a whole sub-family here — AutoEnable off, service-not-enabled-at-boot, TLP-disables-on-startup, systemd-rfkill-restores-a-stale-block — that all present as "powered off / blocked" and all need a *persistent* fix the doctor doesn't distinguish from a one-shot power-on.
+=_powered_step= checks whether the adapter is powered *now* and offers =power-on=. But =power-on= via bluetoothctl doesn't persist: on a machine with =AutoEnable=false= explicitly set (bluez's compiled default is =true= — verified in bluez 5.87's =/etc/bluetooth/main.conf= — so an absent or unset config auto-enables, and the fault is an explicit opt-out, not the missing-config case), or a TLP laptop that disables bluetooth on startup, the adapter is dead again next boot. The doctor fixes the symptom every session and never names the cause. The taxonomy's cluster 2 has a whole sub-family here — AutoEnable explicitly off, service-not-enabled-at-boot, TLP-disables-on-startup, systemd-rfkill-restores-a-stale-block — that all present as "powered off / blocked" and all need a *persistent* fix the doctor doesn't distinguish from a one-shot power-on.
** Pairing and connection clusters stay light on purpose (clusters 3, 4)
@@ -77,17 +80,21 @@ When the adapter is powered off, the doctor asks a second question: is it *suppo
** For the implementer
+*** How the new "verdicts" are represented
+
+The bt doctor has no verdict-enum layer like the net doctor's =classify= actions. A step result today carries a fixed key set — =id=, =status= (=pass=/=fail=/=warn=/=info=), =title=, =evidence=, =elapsed_ms=, =safety=, =next_action= (=schema.step=) — and the run rolls the statuses up into an =overall= (=ok=/=warn=/=fail=, =doctor.py:182-183=). So a "verdict" in this spec is not a new =overall= value — it is a distinct step *outcome*: the same =status= with a specific message and evidence, and a =next_action= carrying the fix for the persistent case. The human distinction rides in the existing =evidence= and =next_action= fields — =format_doctor_human= renders =evidence= (=cli.py:94=), so the named blob or persistence cause needs no formatter change. If a stable machine identifier is wanted so =--json= consumers can branch on the outcome without string-matching, that is an *additive* =code= key on =schema.step= — and because the step schema is a locked test contract, adding it ripples into any test asserting exact step shape, so it is called out here rather than assumed. =_adapter_step= and =_powered_step= already own their results; the new probes add branches inside them. Nothing touches the =overall= vocabulary or =AUTO_FIX= tiers — the additions stay inside the existing step-status model rather than inventing a parallel verdict system.
+
*** The dmesg firmware-hint probe
-When =_adapter_step= finds no adapter, run a bounded =journalctl -k -b --no-pager= (or =dmesg=) read, scanned for the known firmware-load-failure signatures per vendor (Intel ibt-*.sfi, MediaTek BT_RAM_CODE, Realtek rtl_bt, Broadcom BCM .hcd, Qualcomm QCA version-read). A match yields a specific =no-adapter-firmware= verdict naming the blob and the Reboot-tail Guide (update linux-firmware / symlink the blob / reboot). No match yields the existing generic =no-adapter=. The read is bounded and read-only; it runs only in the no-adapter branch, so it costs nothing on a healthy adapter.
+When =_adapter_step= finds no adapter, run a bounded =journalctl -k -b --no-pager= (or =dmesg=) read, scanned for the known firmware-load-failure signatures per vendor (Intel ibt-*.sfi, MediaTek BT_RAM_CODE, Realtek rtl_bt, Broadcom BCM .hcd, Qualcomm QCA version-read). A match sets the =no-adapter-firmware= code on the adapter step, naming the blob and the Reboot-tail Guide (update linux-firmware / symlink the blob / reboot). No match keeps the existing generic no-adapter outcome. The read reuses the engine's existing bounded pattern — =cmd.run(journalctl -u bluetooth -n 1)= already runs for service-log evidence (=doctor.py:84=, 5s timeout) — so the kernel-log read is the same bounded shape, read-only, and runs only in the no-adapter branch, costing nothing on a healthy adapter.
*** The boot-enablement probe
-When =_powered_step= finds the adapter powered off (or =_rfkill_step= finds a soft-block), consult three persistence signals: bluez =AutoEnable= (parse =/etc/bluetooth/main.conf= [Policy], defaulting to the bluez static default when absent), =systemctl is-enabled bluetooth=, and whether TLP's =DEVICES_TO_DISABLE_ON_STARTUP= lists bluetooth. A powered-off adapter with a persistence fault gets a =powered-off-persistent= verdict distinct from the plain =powered-off=; its fix is the persistent one (set AutoEnable / enable the service / edit tlp.conf), Privileged/Confirm. The existing =power-on= stays for the plain case.
+When =_powered_step= finds the adapter powered off (or =_rfkill_step= finds a soft-block), consult three persistence signals: bluez =AutoEnable= (parse =/etc/bluetooth/main.conf= [Policy]), =systemctl is-enabled bluetooth=, and whether TLP's =DEVICES_TO_DISABLE_ON_STARTUP= lists bluetooth. The fault fires only on an *explicit* boot-disable: =AutoEnable=false= set in main.conf (bluez's compiled default is =true=, so an absent or unset key means auto-enable-on and is never the fault), a =disabled= bluetooth.service, or a TLP entry. A powered-off adapter with one of those gets a =powered-off-persistent= outcome distinct from the plain =powered-off=; its fix is the persistent one (set AutoEnable / enable the service / edit tlp.conf), Privileged/Confirm. The existing =power-on= stays for the plain case (including the common absent-config machine, which auto-enables by default).
*** The privilege model
-The new repairs — editing main.conf, =systemctl enable bluetooth=, editing tlp.conf — are root-needing and adopt the cross-panel run-time model: run where passwordless sudo exists, prompt on a tty, default to Confirm/Arm, never silent Auto. Same shared implementation as the net and audio doctors. The existing four auto-fix tiers (unblock, power-on, service-restart, a2dp) are unchanged — they are already user-scope or already the doctor's safe tier.
+The new repairs — editing main.conf, =systemctl enable bluetooth=, editing tlp.conf — are root-needing and adopt the cross-panel run-time model: run where passwordless sudo exists, prompt on a tty, default to Confirm/Arm, never silent Auto. Same shared implementation as the net and audio doctors. =priv.py= today exposes exactly one privileged verb, =restart-bluetooth=, via a plain verb→argv dispatch (=priv.py:25-34=); the three new fixes add verbs to it, each a narrowly-scoped operation — set the single =AutoEnable= key, enable one named unit, set the single TLP key — not a general edit-file-as-root. The passwordless-sudo grant widens by three tight verbs, not by a root file-editor, so the Confirm floor is the second guard rather than the only one. One caveat the implementer must not miss: =systemctl enable bluetooth= is a clean argv verb, but the two config edits are not. main.conf is INI — setting =AutoEnable= needs an idempotent setter that creates the [Policy] section when absent and preserves comments (a shipped helper or crudini). tlp.conf is a flat =KEY=VALUE= file — the fix removes =bluetooth= from the =DEVICES_TO_DISABLE_ON_STARTUP= list, a list-edit, not a section-create. Both are real Phase 2 work, not one-line argv verbs. The existing four auto-fix tiers (unblock, power-on, service-restart, a2dp) are unchanged — they are already user-scope or already the doctor's safe tier.
* Alternatives Considered
@@ -109,52 +116,72 @@ The new repairs — editing main.conf, =systemctl enable bluetooth=, editing tlp
- Bad, because re-pair is destructive (removes the bond) and the signature ("fails to connect with a bond present, repeatedly") needs care to not fire on a merely-out-of-range device; getting it wrong offers a destructive fix for a transient condition.
- Held as an open decision, not a v1 commitment.
-* Decisions [0/3]
+* Decisions [3/3]
-** TODO The dmesg firmware-hint probe
+** DONE The dmesg firmware-hint probe
Context: cluster 1 is almost all firmware faults the kernel already logs, and the doctor reports a generic "hardware/driver" instead of the named blob.
-Decision (proposed): we will read the kernel log in the no-adapter branch, match the known per-vendor firmware-load-failure signatures, and emit a =no-adapter-firmware= verdict naming the blob and its Reboot-tail Guide; no match falls back to the generic verdict.
+Decision: we will read the kernel log in the no-adapter branch, match the known per-vendor firmware-load-failure signatures, and set a =no-adapter-firmware= outcome code naming the blob and its Reboot-tail Guide; no match falls back to the generic no-adapter outcome.
Consequences: the most common no-adapter cause becomes self-explaining; harder — a per-vendor signature table to maintain, and the read must stay bounded and only run in the no-adapter branch so it never slows a healthy run.
-Owner: Craig. By: before Phase 1 lands.
+Resolution: accepted as proposed (the generic "check firmware" alternative was rejected — the whole value is naming the specific blob). Owner: Craig.
-** TODO The boot-enablement probe and the persistent-vs-transient split
+** DONE The boot-enablement probe and the persistent-vs-transient split
Context: =power-on= doesn't persist, so an AutoEnable-off / not-enabled / TLP-disabled adapter is dead every boot and the doctor fixes only the symptom.
-Decision (proposed): we will add a persistence probe (AutoEnable, service-enabled, TLP) and a =powered-off-persistent= verdict distinct from =powered-off=, whose fix is the persistent one under the Confirm floor; the plain power-on stays for the transient case.
-Consequences: the boot-disable cause is named and fixed once instead of every session; harder — the doctor must not override a *deliberate* boot-disable, so the persistent fix is always confirmed and the verdict names the specific persistence cause rather than blanket-enabling.
-Owner: Craig. By: before Phase 2 lands.
+Decision: we will add a persistence probe (AutoEnable, service-enabled, TLP) and a =powered-off-persistent= outcome code distinct from =powered-off=, whose fix is the persistent one under the Confirm floor; the plain power-on stays for the transient case.
+Consequences: the boot-disable cause is named and fixed once instead of every session; harder — the doctor must not override a *deliberate* boot-disable, so the persistent fix is always confirmed and the outcome names the specific persistence cause rather than blanket-enabling.
+Resolution: accepted as proposed (the always-persistent-power-on alternative was rejected — it would silently override a deliberate battery-saving boot-disable). Round-2 correction: the verdict fires only on an *explicit* AutoEnable=false / disabled service / TLP entry — bluez's compiled default is =true= (verified, bluez 5.87), so absent or unset config is auto-enable-on and never the fault. Owner: Craig.
-** TODO Whether the stale-bond re-pair-offer lands in v1 or vNext
+** DONE Whether the stale-bond re-pair-offer lands in v1 or vNext
Context: a device failing to connect with a bond present, repeatedly, is a re-pair candidate — but re-pair is destructive and the signature can misfire on an out-of-range device.
-Decision (proposed): defer to vNext. v1 keeps re-pair strictly user-initiated; the signature is designed and validated before the doctor ever *offers* it.
+Decision: defer to vNext. v1 keeps re-pair strictly user-initiated; the signature is designed and validated before the doctor ever *offers* it.
Consequences: no risk of the doctor offering a destructive fix for a transient condition in v1; harder — the "keeps failing to connect" case stays unnamed for now, which is the one cluster-3 gap a user might reasonably expect the doctor to catch.
-Owner: Craig. By: before Phase 1 (scope confirmation).
+Resolution: accepted — deferral confirmed. The "keeps failing to connect" gap is logged to vNext. Owner: Craig.
+
+* Review findings [8/8]
+
+** DONE The bt doctor has no named-verdict layer, so "emit a verdict named X" is undefined :blocking:
+The spec proposes named verdicts throughout — =no-adapter-firmware=, =powered-off-persistent= "distinct from =powered-off=" — mirroring the net doctor's action-identifier vocabulary. But the bt doctor has no such layer. Verified: a step carries a =status= ∈ =pass/fail/warn/info= and the run has an =overall= ∈ =ok/warn/fail= (=doctor.py:182-183=); there are no verdict identifiers as *names*. So "add a =powered-off-persistent= verdict distinct from =powered-off=" had no existing mechanism to attach to.
+Disposition: accepted. Added Design "How the new 'verdicts' are represented": a verdict here is a distinct step *outcome* — the same =status= plus a stable machine =code= and a specific message/evidence, set inside =_adapter_step= / =_powered_step=, rendered by =format_doctor_human= and carried in =--json=. Nothing touches the =overall= vocabulary or =AUTO_FIX= tiers. The Decisions and probe descriptions now speak in "outcome code" terms, so the representation is defined before Phase 1.
+
+** DONE New root repairs expand the NOPASSWD sudoers surface — name it in rollout
+=priv.py= today exposes a single privileged verb, =restart-bluetooth= (=priv.py:27-34=), backed by a NOPASSWD sudoers entry. The three new persistent fixes (edit main.conf, =systemctl enable bluetooth=, edit tlp.conf) are each a new root-capable verb.
+Disposition: accepted, modified. Rather than just "name the expansion," the resolution constrains each new verb to a narrowly-scoped operation (set the single =AutoEnable= key, enable one named unit, set the single TLP key) — not a general edit-file-as-root — so the passwordless-sudo grant widens by three tight verbs, not a root file-editor. Folded into the privilege-model design and the Rollout dimension.
-* Review findings [/]
+** DONE The diagnostic surface is =bt doctor='s JSON, not a =bt diag= subcommand
+Phase 0 said "=bt diag --json= (or the equivalent) shows the new signals." Verified: there is no =bt diag= subcommand; =diagnose()= is the report authority (=doctor.py:174=), =doctor()= wraps it (=:207=), JSON is the default output, and =format_doctor_human= (=cli.py:94=) renders the human view.
+Disposition: accepted. Phase 0 now points at the real surface (the doctor's JSON report + =format_doctor_human=) and drops the =bt diag= reference.
-** TODO The bt doctor has no named-verdict layer, so "emit a verdict named X" is undefined :blocking:
-The spec proposes named verdicts throughout — =no-adapter-firmware=, and =powered-off-persistent= "distinct from =powered-off=" — mirroring the net doctor's action-identifier vocabulary. But the bt doctor has no such layer. Verified: a step carries a =status= ∈ =pass/fail/warn/info= and the run has an =overall= ∈ =ok/warn/fail= (=doctor.py:182-183=); there are no verdict identifiers like =powered-off= or =power-on= as *names* — those are message text and the =power-on= auto-fix tier, not a verdict enum. So "add a =powered-off-persistent= verdict distinct from =powered-off=" has no existing mechanism to attach to, and the implementer would have to invent the representation: is the distinction a new =status= value, a field on the powered step, a message variant, an added =fix= identifier? Whatever it is, it also touches =format_doctor_human= (=cli.py:94=) and any consumer of =overall=. Define how each new "verdict" is represented in the step-status model before Phase 1 — this is the one place the spec would force the implementer to invent product/data behavior. (blocking)
+** DONE Firmware-hint probe is new but has a bounded precedent — cite it
+Phase 0's =journalctl -k= reader is genuinely new (=_adapter_step= reads nothing beyond =btctl.show=, =doctor.py:46-50=), but the engine already runs a bounded =journalctl -u bluetooth -n 1= for service-log evidence (=doctor.py:84=, timeout 5s via =cmd.run=).
+Disposition: accepted. The firmware-hint probe design now cites =doctor.py:84= as the bounded precedent the kernel-log read reuses, so the implementer reaches for the same =cmd.run= shape rather than an unbounded call.
-** TODO New root repairs expand the NOPASSWD sudoers surface — name it in rollout
-=priv.py= today exposes a single privileged verb, =restart-bluetooth= (=priv.py:27-34=), backed by a NOPASSWD sudoers entry. The three new persistent fixes (edit =/etc/bluetooth/main.conf=, =systemctl enable bluetooth=, edit =/etc/tlp.conf=) are each a new root-capable verb and each widens the passwordless-sudo grant from "restart one service" to "edit system config files as root." That is a heavier privilege surface than the existing tier; the Rollout/compatibility dimension should name the sudoers expansion explicitly and confirm each new verb is a tightly-scoped command (not a general "run as root"), so the privilege model's Confirm floor is the *second* guard, not the only one. (non-blocking)
+** DONE Round 2 (skeptical review): the AutoEnable default is true, not false :blocking:
+Round 1 asserted (twice) that =AutoEnable=false= is "bluez's static default when main.conf is absent," and the whole "dead every boot" premise rested on it. A skeptical re-review read the installed bluez 5.87 =/etc/bluetooth/main.conf= and found the opposite: "Defaults to 'true'." Verified independently (=#AutoEnable=true= is the commented compiled default, bluez 5.87). An implementer trusting the round-1 text would code default=False and report =powered-off-persistent= falsely on every common healthy machine with no/default config — the exact "targets a non-problem" failure.
+Disposition: accepted — a real blocker. Corrected the premise everywhere: the fault now fires only on an *explicit* AutoEnable=false / disabled service / TLP entry, never on config absence. Fixed Problem/Context, the boot-enablement probe design, decision 2, and added an acceptance criterion that absent/default config gets the transient power-on.
-** TODO The diagnostic surface is =bt doctor='s JSON, not a =bt diag= subcommand
-Phase 0 says "=bt diag --json= (or the equivalent) shows the new signals." Verified: there is no =bt diag= subcommand; =diagnose()= is the report authority (=doctor.py:174=), =doctor()= wraps it (=:207=), and JSON is already the default output of the doctor subparser (=cli.py:173= — the =--json= flag is effectively redundant). Point Phase 0 at the real surface: the new signals land in =diagnose()='s report dict and render through =format_doctor_human= (=cli.py:94=). Cosmetic — the author already hedged with "(or the equivalent)." (non-blocking)
+** DONE Round 2 (skeptical review): the machine =code= field doesn't exist on the step schema
+Round 1's representation subsection spoke of "a stable machine =code=" as if the step result already had one. The skeptical review confirmed =schema.step= returns a fixed key set (=id=, =status=, =title=, =evidence=, =elapsed_ms=, =safety=, =next_action=) with no =code=, and the step schema is a locked test contract.
+Disposition: accepted. The subsection now carries the human distinction in the existing =evidence=/=next_action= fields (no formatter change), and names the optional =code= key as an *additive* schema change with the test-contract ripple called out — not assumed to exist.
-** TODO Firmware-hint probe is new but has a bounded precedent — cite it
-Phase 0's =journalctl -k= reader is genuinely new (=_adapter_step= reads nothing beyond =btctl.show=, =doctor.py:46-50=), but the engine already runs a bounded =journalctl -u bluetooth -n 1= for service-log evidence (=doctor.py:84=, timeout 5s via =cmd.run=). Cite it as the precedent so the implementer reuses the same bounded-=cmd.run= shape for the kernel-log read rather than introducing an unbounded call. Confirms feasibility; not a gap. (non-blocking)
+** DONE Round 2 (skeptical review): "single-key set" for main.conf/tlp.conf hides real work
+Round 1 called the config-edit verbs "narrowly-scoped" without naming the mechanism. The skeptical review noted =systemctl enable= is a clean argv verb but editing an INI key needs an idempotent setter (create the section if absent, preserve comments), which is more than a one-liner.
+Disposition: accepted. The privilege-model design now names the INI-setter requirement and marks it Phase 2 work, distinct from the argv-clean service-enable verb.
+
+** DONE Round 2 (skeptical review): the shared-privilege-code sequencing caveat was claimed but missing
+The round-1 review-history entry said the shared-privilege-model sequencing caveat was "already in Risks," but the Risks section did not mention it, and Phase 1 lands "shared cross-panel" code while =priv.py= is bt-local with one verb today — so Phase 1 isn't independently landable until that shared model ships.
+Disposition: accepted. Added the sequencing caveat to Risks, naming Phase 0 (the two read-only probes) as the independently-landable slice and noting the AutoEnable correction makes those probes worth landing on their own.
* Implementation phases
Each phase leaves the tree green and independently useful, as the existing bt phases did.
** TODO Phase 0 — the two read-only probes
-Pure engine, no repair changes. The dmesg firmware-hint reader (no-adapter branch) and the boot-enablement reader (AutoEnable / service-enabled / TLP), both reporting into the diagnose report. =bt diag --json= (or the equivalent) shows the new signals. Fakes: a canned journal buffer with each vendor's signature, and injected main.conf / =systemctl is-enabled= / tlp.conf states.
+Pure engine, no repair changes. The dmesg firmware-hint reader (no-adapter branch) and the boot-enablement reader (AutoEnable / service-enabled / TLP), both reporting into the =diagnose()= report dict. That dict feeds both views: the human summary (the default) and =--json= (=cli.py:113-116=); =format_doctor_human= (=cli.py:94=) renders the new evidence. There is no separate =bt diag= subcommand. Fakes: a canned journal buffer with each vendor's signature, and injected main.conf / =systemctl is-enabled= / tlp.conf states.
-** TODO Phase 1 — the firmware-hint verdict + privilege model
-=diagnose= emits =no-adapter-firmware= (naming the blob, Reboot-tail Guide) when the signature matches. The run-time privilege resolution lands (shared cross-panel code). No auto-fix — this verdict is a Guide.
+** TODO Phase 1 — the firmware-hint verdict (Guide, no privilege dependency)
+=diagnose= emits =no-adapter-firmware= (naming the blob, Reboot-tail Guide) when the signature matches. No auto-fix — this verdict is a Guide, so it needs no privilege model and lands independently of the shared cross-panel code. The run-time privilege resolution is a Phase 2 prerequisite, not this phase.
-** TODO Phase 2 — the persistent-power verdict and its fix
-=powered-off-persistent= distinct from =powered-off=; the persistent fix (set AutoEnable / enable service / tlp) registers Privileged/Confirm. =bt doctor --fix= applies it under the Confirm floor; the plain power-on path is unchanged.
+** TODO Phase 2 — the persistent-power verdict and its fix (gated on the shared privilege model)
+=powered-off-persistent= distinct from =powered-off=; the persistent fix (set AutoEnable / enable service / tlp) registers Privileged/Confirm through =priv.py= (which is one verb today), plus the INI/list setters above. =bt doctor --fix= applies it under the Confirm floor; the plain power-on path is unchanged. Hard ordering gate: this phase must not land before the shared cross-panel run-time privilege model exists — shipping a Privileged fix before the Confirm floor would let =--fix= edit root config via passwordless sudo ungated, the exact outcome the model forbids.
** TODO Phase 3 — flip this spec to IMPLEMENTED
And log the vNext items (stale-bond signature, connection-parameter hints, bt-audio-profile expansion) to =todo.org=.
@@ -163,8 +190,9 @@ And log the vNext items (stale-bond signature, connection-parameter hints, bt-au
- [ ] With no adapter and a MediaTek/Intel/Realtek/Broadcom/Qualcomm firmware-load error in the kernel log, the doctor names the specific blob and prints the update-and-reboot Guide.
- [ ] With no adapter and no firmware error in the log, the doctor reports the generic "no controller attached" without inventing a firmware cause.
-- [ ] An adapter that is powered off with AutoEnable=false reports =powered-off-persistent= and FIX offers the persistent fix, not just a one-shot power-on.
+- [ ] An adapter that is powered off with AutoEnable=false *explicitly set* reports =powered-off-persistent= and FIX offers the persistent fix, not just a one-shot power-on.
- [ ] An adapter powered off with AutoEnable on (a transient off) still gets the quick power-on.
+- [ ] An adapter powered off with main.conf absent or AutoEnable unset is treated as auto-enable-on (bluez's compiled default is true) and gets the transient power-on, never =powered-off-persistent=.
- [ ] Every new root-needing repair defaults to Confirm/Arm and never runs silently as Auto.
- [ ] The existing chain (rfkill/service/powered/device/audio) and its four auto-fix tiers classify and repair exactly as today (regression).
@@ -180,7 +208,7 @@ And log the vNext items (stale-bond signature, connection-parameter hints, bt-au
- *Config surface* — none new for the doctor; it *reads* bluez/tlp config and *writes* it only as a confirmed repair. N/A for its own knobs.
- *Documentation plan* — module docstrings, as the package does today. The wall is the user documentation.
- *Dev tooling* — =make test= and the bt panel smoke cover it; the new probes need a canned journal fixture and injected config states, fixture shapes the package can adopt.
-- *Rollout, compatibility & rollback* — additive; the existing chain is untouched. The persistent fixes change bluez/tlp config and service enablement, so all are Confirm-tier and reversible by the user.
+- *Rollout, compatibility & rollback* — additive; the existing chain is untouched. The persistent fixes change bluez/tlp config and service enablement, so all are Confirm-tier and reversible by the user. Each new privileged repair adds a narrowly-scoped verb to =priv.py= (single-key sets, one named unit-enable) rather than a general root file-edit, so the passwordless-sudo surface grows by three tight verbs beyond today's single =restart-bluetooth=; the sudoers/priv change ships with the fixes.
- *External APIs & deps* — =journalctl -k=/=dmesg=, =/etc/bluetooth/main.conf=, =systemctl is-enabled=, and =/etc/tlp.conf= layouts are verified against the live system before Phase 0. The per-vendor firmware signatures come from the taxonomy's cluster-1 sources; no new packages.
* Risks, rabbit holes, and drawbacks
@@ -191,6 +219,8 @@ The persistent-power fix must not fight a deliberate choice. A user (or TLP on a
The bt-audio-profile expansion (vNext) overlaps the audio taxonomy's Bluetooth-mic cluster. When it is picked up, it needs coordination with the audio doctor so the two panels don't both claim the same A2DP/HFP diagnosis with divergent verdicts. Named here so the seam is known.
+The privilege model this spec adopts is shared cross-panel code that does not exist yet — =priv.py= is bt-local today with one verb (=restart-bluetooth=). Phase 1 (which lands the run-time privilege resolution) can only land independently once that shared model ships somewhere; sequence it so whichever panel lands the shared code first, the others depend on it. Until then, Phase 0 (the two read-only probes) is the independently-landable slice, and the AutoEnable-default correction means those probes are worth landing on their own — they add the firmware and boot-persistence *signals* even before any privileged fix exists.
+
* Review and iteration history
** 2026-07-11 Sat @ 00:08:41 -0500 — Craig Jennings — Author
@@ -202,3 +232,18 @@ The bt-audio-profile expansion (vNext) overlaps the audio taxonomy's Bluetooth-m
- What: ran spec-review. Rubric =Not ready=. Recorded four findings, one =:blocking:= (no named-verdict layer to attach the proposed verdicts to) and three non-blocking (sudoers-surface expansion, the real diagnostic surface, the bounded firmware-read precedent). The three proposed decisions remain open.
- Why: the design is sound and the code read confirmed both target gaps are real — the engine reads no kernel log for firmware hints and nothing for boot persistence (=main.conf=/=is-enabled=/=tlp.conf= all grep-clean). What holds the rubric is that the spec borrows the net doctor's verdict-naming language, but the bt doctor is step-status-based (=status= pass/fail/warn/info, =overall= ok/warn/fail, =doctor.py:182-183=) with no verdict enum — so how each new "verdict" is represented is undefined. Once that representation is decided and Craig accepts the three decisions, this reaches =Ready with caveats= (shared privilege-model code being the sequencing caveat, already in Risks).
- Artifacts: engine facts verified across =doctor.py= (:25 AUTO_FIX, :46-50 no-adapter branch, :84 bounded journalctl precedent, :100-107 powered step, :174-183 diagnose/overall), =priv.py:27-34= (single =restart-bluetooth= verb), =cli.py:94/173= (formatter + json default), =redact.py:11-20=. Findings in =* Review findings=.
+
+** 2026-07-11 Sat @ 01:20:00 -0500 — Claude Code (archsetup) — Responder
+- What: ran spec-response. Dispositioned all four findings (three accepts, one accept-with-modify: the new privileged verbs are narrowly scoped, not a general root file-editor) and closed all three decisions as accepted. Added Design "How the new 'verdicts' are represented" defining a verdict as a step outcome code in the existing status model; cited the bounded =journalctl= precedent; corrected the Phase 0 diagnostic surface; constrained the sudoers expansion in the privilege-model design and the Rollout dimension. Both =[/]= cookies now read complete.
+- Why: convergence toward implementation-ready. The verdict-representation finding was the real blocker — the spec had borrowed net's verdict language onto a step-status engine, so defining "verdict = outcome code" is what makes the two probes implementable without inventing a parallel system.
+- Artifacts: findings =[4/4]=, decisions =[3/3]=. Scope expanded only by the tightened privilege-verb obligation, filed in the Rollout dimension, 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 one hard blocker — the AutoEnable default is =true=, not =false= (round 1 had it backwards, verified against bluez 5.87), which would fire =powered-off-persistent= falsely on every healthy default-config machine. Corrected the premise everywhere so the fault fires only on explicit signals. Also fixed three non-blocking issues: the machine =code= is an additive schema change (=schema.step= has no =code= key today), the INI-setter for main.conf/tlp.conf is real Phase 2 work not a one-liner, and the shared-privilege-code sequencing caveat is now actually in Risks (round 1's history claimed it was). Findings now =[8/8]=.
+- Why: round 1 rubber-stamped the AutoEnable default without checking the installed bluez. Round 2 verified it directly (=#AutoEnable=true= in =/etc/bluetooth/main.conf=). The correction also strengthens phasing: Phase 0's read-only probes are worth landing on their own, before any privileged fix.
+- Artifacts: bluez 5.87 =/etc/bluetooth/main.conf= (AutoEnable default true); =schema.step= (fixed key set, no =code=); =priv.py:25-34= (verb→argv dispatch). 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 four round-2 resolutions: the AutoEnable premise is now self-consistent (every fault sentence keys on an explicit signal, acceptance criteria cover the absent-config case), the verdict representation is correct against =schema.step= (no =code= key; the distinction rides =evidence=/=next_action=, rendered by =cli.py:98-102= with no formatter change), the INI-setter is concretely named, and Phase 0 is genuinely independently landable. Folded two non-blocking corrections (the human summary is the doctor default not JSON; tlp.conf is a flat list-edit not an INI section) and decoupled Phase 1's Guide-only firmware verdict from the blocked privilege scaffolding. Flipped DRAFT → READY.
+- Why: the loop terminates at the rubric. Two skeptical passes plus code re-verification found no remaining blocker. The caveat — Phase 2's persistent fix needs the shared privilege model — is recorded in Risks, Decision 2/the privilege design, and a hard ordering gate on Phase 2; Phases 0 and 1 build today.
+- Artifacts: findings =[8/8]=, decisions =[3/3]=. Non-blocking notes folded into wording; no open blocker.
diff --git a/docs/specs/2026-07-11-net-doctor-expansion-spec.org b/docs/specs/2026-07-11-net-doctor-expansion-spec.org
index 9f51d74..b7867c1 100644
--- a/docs/specs/2026-07-11-net-doctor-expansion-spec.org
+++ b/docs/specs/2026-07-11-net-doctor-expansion-spec.org
@@ -4,10 +4,13 @@
#+TODO: TODO | DONE
#+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED
-* DRAFT Net Doctor Expansion
+* READY Net Doctor Expansion
:PROPERTIES:
:ID: ce29b103-ed9d-4f56-bf8c-9ed8fe680ff3
:END:
+- [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.
@@ -15,7 +18,7 @@
| Field | Value |
|----------+-----------------------------------------------------------------------------------|
-| Status | draft |
+| Status | ready |
|----------+-----------------------------------------------------------------------------------|
| Owner | Craig Jennings |
|----------+-----------------------------------------------------------------------------------|
@@ -30,13 +33,13 @@ The net doctor is the most mature of the three panel doctors: its probe ladder a
* 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=, no-hardware), cluster 2 (DHCP-failed), cluster 3 (=tunnel-down=, =vpn-policy=), cluster 5 (=resolved-restart=, DNS-not-resolving, =dns-test=/=dns-override=), and cluster 6 (=portal=, =clock-sync=, =proxy=, and the =upstream-not-local= terminal STOP). 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.
+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 the doctor currently has one verdict (=nm-restart=) covering the whole control plane.
+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)
@@ -87,19 +90,23 @@ A new read-only probe tier, beside the existing ones, answering three questions
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.
+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 terminal-and-specificity order ahead of the generic =nm-restart= rule: =rival-manager= (needs-user-action → Privileged fix), =nm-masked= (fixable → unmask), =keyfile-perms= (fixable → chmod/chown). Each carries evidence naming the specific rival/unit/file. These are additive; the existing rules below them are untouched.
+=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
-=gather_context='s existing =auth_failed_reason= (already extracted from GENERAL.REASON and the journal) grows a small classifier: SAE/PMF, hidden-SSID, enterprise-cert, regdom, or generic-PSK. 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 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.
+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
@@ -121,42 +128,63 @@ The new repairs (=systemctl disable <rival>=, =unmask=, =chmod=/=chown= a keyfil
- 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 [0/3]
+* Decisions [3/3]
-** TODO The control-plane probe and its three verdicts
-Context: cluster 7 is the taxonomy's largest untouched cluster, and the current single =nm-restart= verdict mis-serves a masked NM, a rival manager, and a bad keyfile.
-Decision (proposed): 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.
+** 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.
-Owner: Craig. By: before Phase 1 lands.
+Resolution: accepted as proposed (the fold-into-nm-restart alternative was rejected — that fix cannot address any of the three faults). Owner: Craig.
-** TODO How far the auth-cluster fix goes
+** 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 (proposed): 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.
-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.
-Owner: Craig. By: before Phase 2 lands.
+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.
-** TODO Adopt the run-time privilege model as the cross-panel standard
+** 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 (proposed): 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).
-Owner: Craig. By: before Phase 1 lands.
+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.
-* Review findings [/]
+** 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.
-** TODO Connection names are not redacted today, but the spec assumes they are :blocking:
-The Security & privacy dimension states "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. So the spec ships a privacy leak it believes is already closed. V1 must either add connection-name / keyfile-basename redaction to =redact.py= (and cover it in the redaction test), or make an explicit, recorded decision that connection names are non-sensitive display content. Blocking because it is a privacy contract the spec states as already-true and isn't. (blocking)
+** 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.
-** TODO "One =nm-restart= verdict covers the whole control plane" overstates current scope
-Problem/Context and the first Alternative rest 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 not "one verdict for everything" — it is that *no* probe detects a rival manager, a masked NM, or a bad keyfile (=is-enabled= and =system-connections= reads genuinely do not exist: grep-clean). Reword the motivation onto those three missing detections so the argument rests on a true premise. Does not change the design; the gap it targets is real. (non-blocking)
+** 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.
-** TODO "=systemctl is-enabled= already used by the engine" is inaccurate
-External APIs & deps says =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. Not a design problem — Phase 0 adds it — but the readiness claim should say =is-active= is used today and =is-enabled= is new surface, so the implementer knows to add the bounded call and its fake rather than assuming a helper exists. (non-blocking)
+** 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.
-** TODO Problem/Context lists message text as if it were classifier verdicts
-The "already reaches six clusters" evidence lists =no-hardware=, =DHCP-failed=, =DNS-not-resolving=, =dns-override= as classifier verdicts. Verified: the first three are message text, not action identifiers, and =dns-override= is emitted only by the doctor's =FIX_CHAINS= (=doctor.py:24=), not by =classify=. The real classifier action identifiers are =portal=, =vpn-policy=, =tunnel-down=, =nm-restart=, =resolved-restart=, =rfkill=, =reset=, =manage-device=, =dns-test=, =proxy=, =clock-sync=, =bounce= (=classify.py=). Cite those so the six-cluster claim is checkable against the code. Cosmetic — the conclusion holds. (non-blocking)
+** 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=.
-** TODO Name the =priv.py= / =repair.py= integration point for the new privileged repairs
-The spec says it reuses =priv.py=, which is correct, but the phase plan does 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. Phase 1 should state that =disable-rival=, =unmask-nm=, and =chmod-keyfile= are added as =VERBS= + =ACTIONS= entries, so the implementer extends the existing dispatch rather than building a parallel one. (non-blocking)
+** 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
@@ -166,7 +194,7 @@ Each phase leaves the tree green and independently useful, as the existing net p
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=, ordered ahead of the generic not-running rule. The run-time privilege resolution lands (shared with the cross-panel task) and the three new fixes register as Privileged/Confirm. =net doctor= names them; =net doctor --fix= applies them under the Confirm floor.
+=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).
@@ -182,13 +210,15 @@ And log the vNext items (flaky/drops cluster, DoT/DNSSEC verdict, profile hygien
- [ ] 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* — SSIDs and connection names are already redacted by =redact.py=; the keyfile probe reads permissions, not secrets. The auth-reason extraction must not leak the PSK (it reads NM's reason string, not the key).
+- *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.
@@ -197,7 +227,7 @@ And log the vNext items (flaky/drops cluster, DoT/DNSSEC verdict, profile hygien
- *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/is-enabled=, =nmcli=, and =/etc/NetworkManager/system-connections= layout are all already used by the engine; no new packages. The exact rival-manager set (dhcpcd/networkd/iwd) is verified against the live system before Phase 0.
+- *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
@@ -218,3 +248,18 @@ Coupling to the shared privilege-model task means this spec can't fully land unt
- 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.