diff options
Diffstat (limited to 'docs/specs')
| -rw-r--r-- | docs/specs/2026-07-09-audio-doctor-spec.org | 76 | ||||
| -rw-r--r-- | docs/specs/2026-07-10-audio-doctor-input-side-spec.org | 494 | ||||
| -rw-r--r-- | docs/specs/2026-07-11-bt-doctor-expansion-spec.org | 250 | ||||
| -rw-r--r-- | docs/specs/2026-07-11-net-doctor-expansion-spec.org | 266 |
4 files changed, 1062 insertions, 24 deletions
diff --git a/docs/specs/2026-07-09-audio-doctor-spec.org b/docs/specs/2026-07-09-audio-doctor-spec.org index 5c4f844..5851332 100644 --- a/docs/specs/2026-07-09-audio-doctor-spec.org +++ b/docs/specs/2026-07-09-audio-doctor-spec.org @@ -4,16 +4,24 @@ #+TODO: TODO | DONE #+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED -* DRAFT Audio Doctor — diagnose and repair a broken sound stack +* IMPLEMENTED Audio Doctor — diagnose and repair a broken sound stack :PROPERTIES: :ID: 6ce3f2ef-052e-40cb-9067-42f8e665f813 :END: -- [2026-07-09 Thu] DRAFT — written from the roam-inbox ask "look into a doctor button for the audio panel: what happens if pulseaudio or its equivalent is borked? would we know? how could we test for it? what remedies would we have?" Probe set, classifier, and remedy tiers below are grounded in a live survey of ratio's stack. Six Decisions are open; Phase 0 shipped ahead of the rest. +- [2026-07-10 Fri] Superseded in part, later the same day: the DOCTOR section-header key and its DIAGNOSE key are replaced by a doctor key per direction, and the classifier grows an input side. See [[file:2026-07-10-audio-doctor-input-side-spec.org][2026-07-10-audio-doctor-input-side-spec.org]] (DRAFT). This spec's other decisions stand. The gap: nothing here ever examined the microphone, and the Non-Goals never said it would not. +- [2026-07-10 Fri] IMPLEMENTED — Phases 0 through 4 shipped (dotfiles =4d42eb3=, =01a1d80=, =76857e3=, =7223c51=, =1550ac8=, =4320255=, =bd33440=). Xruns stayed out of v1 as decided. Per-application stream routing remains a Non-Goal. The wall carries the net panel's copy + close controls, added past the spec at Craig's request; standardizing them across the other panels is tracked as its own task. +- [2026-07-09 Thu] READY — all eight Decisions closed. Written from the roam-inbox ask "look into a doctor button for the audio panel: what happens if pulseaudio or its equivalent is borked? would we know? how could we test for it? what remedies would we have?" Probe set, classifier, and remedy tiers are grounded in a live survey of ratio's stack. Phase 0 shipped ahead of the rest. -* DRAFT Status +* IMPLEMENTED Status :PROPERTIES: :ID: 1cc2b355-9450-4ace-bdb3-b935f7051918 :END: +- [2026-07-10 Fri] IMPLEMENTED — the build closed three things the Decisions did not anticipate. A tenth verdict =tooling-missing= (=remedy: None=), because an absent =pactl= is an unobserved stack rather than a broken one. A =re-probed= marker between a fix run's two row blocks, because unlabelled they read as the doctor contradicting itself. And the read-only key is named DIAGNOSE beside the engraved DOCTOR header rather than being named DOCTOR itself (Craig's call, 2026-07-10, after asking why the panel carries two keys where net's carries one; the two presses stand on the audibility decision, and net's single key auto-fixes because its remedies are cheap). + + Live verification on ratio replaced the planned VM run for remedies 1, 4 and 5, and immediately found two bugs no fake could surface: =pactl get-default-sink= prints the literal =@DEFAULT_SINK@= placeholder when no default is set, and remedy 1 re-probed before WirePlumber had re-created the devices, reporting a repaired stack as broken. Both fixed. +- [2026-07-09 Thu] READY — Craig closed the six open Decisions, all to the recommended option: DOCTOR is a section-header key; CLI-first with the GUI as a face; hung and dead are two verdicts sharing one remedy; tiers stay Auto/Confirm/Arm as drafted; a stream-active guard refuses the audible remedies and is overridable by pressing again; xruns are out of v1. + + Two consequences worth naming, because they were not obvious when the questions were asked. Taking the guard is *why* remedy 4 can stay at Confirm — the danger lives in the state of the machine, not the identity of the remedy. And the guard must read its stream state from =pw-dump= rather than =pactl=, because the remedies it protects are exactly the ones you reach for when the Pulse layer is dead; a =pactl=-fed guard would go blind precisely when it is needed. - [2026-07-09 Thu] DRAFT — Phase 0 shipped (dotfiles =4d42eb3=) and its Decision closed. The first draft claimed =build_status()= had no timeout and froze the panel; driving it disproved that — =pactl.run()= already times out. The real bug was narrower: two of the four reads sat outside the degrade guard, killing waybar's audio module. The claim is corrected in place rather than quietly dropped. - [2026-07-09 Thu] DRAFT — spine complete, Decisions open. Not ready to build: the remedy tiers touch a running meeting's audio, and the classifier's precedence needs Craig's call before code. @@ -21,7 +29,7 @@ | Field | Value | |--------+-------------------------------------------------------------------------------------------------------------| -| Status | draft | +| Status | implemented | |--------+-------------------------------------------------------------------------------------------------------------| | Owner | Craig Jennings | |--------+-------------------------------------------------------------------------------------------------------------| @@ -80,7 +88,9 @@ Four probe tiers, cheapest and most reliable first. Each is independently useful =pactl info= under a hard timeout. Three outcomes, and they are *different faults*: answered / refused / hung. The hung case is the one that currently freezes tooling, and it is invisible to a naive "does pactl work" check because that check hangs too. *** Tier 4 — semantic health (only meaningful once 1–3 pass) -Default sink and source resolve to devices that exist; at least one sink is not =SUSPENDED= when a stream is playing; xrun and suspend lines in the unit journal since boot. Ratio's sink states today are =RUNNING= and =SUSPENDED=; =SUSPENDED= alone is normal idle, not a fault. +Default sink and source resolve to devices that exist; at least one sink is not =SUSPENDED= when a stream is playing. Ratio's sink states today are =RUNNING= and =SUSPENDED=; =SUSPENDED= alone is normal idle, not a fault. + +Xruns are deliberately *not* probed in v1 — see the Decision "Do xruns belong in v1?" below. They are a quality signal with no remedy in this feature's vocabulary, and a finding nobody can act on trains the user to ignore the wall. ** "What remedies would we have?" @@ -213,16 +223,21 @@ Rejected on evidence. =pactl= hangs against a hung server, and it is dead when t Rejected. Remedies 3 through 5 are audible and, in a meeting, disruptive. Diagnosis is free and should be; repair is a press. -* Decisions [2/8] +* Decisions [8/8] -** TODO Where does the DOCTOR key live, and does it replace anything? -The net panel puts diagnose and repair behind one console key. The audio panel's console row already carries INPUT, OUTPUT, and PUSH TALK. Does DOCTOR join that row, or sit in a section header like the maint console's CLEAN UP / REVIEW & FIX pair? +** DONE Where does the DOCTOR key live, and does it replace anything? +CLOSED: [2026-07-09 Thu] +Resolved: a section-header key, the maint console's CLEAN UP / REVIEW & FIX shape. Diagnose and fix are two presses with different consequences, and the header is where the console already teaches that distinction. The console row (INPUT / OUTPUT / PUSH TALK) stays device controls; a doctor is not one. Nothing is replaced. -** TODO Is there an =audio doctor= CLI, and is it the same code path? -The maint console is CLI-first with the GUI as a face; the net panel ships =net doctor= and =net-doctor= as a make target. A CLI here makes the whole feature testable without a display and usable from a TTY when the desktop's sound is dead — arguably the most valuable case. Confirm CLI-first, and whether it prints the same wall text. +** DONE Is there an =audio doctor= CLI, and is it the same code path? +CLOSED: [2026-07-09 Thu] +Resolved: CLI-first, GUI as a face, one code path — the maint console's architecture. =audio doctor= (read-only) and =audio doctor --fix= print the same findings the wall streams. + +The deciding reason: when the desktop's sound is dead, a TTY is where you will be standing. A GUI-only doctor cannot serve the case that motivates the whole feature. It also makes the classifier testable without a display. -** TODO Confirm the classifier precedence above, particularly rows 3 and 4. -Should "pulse compat hung" and "pulse compat down" be one verdict with a detail line, or two verdicts? They share a remedy (restart =pipewire-pulse=) but they are different failures, and the hung case is the one that freezes other tooling. +** DONE Confirm the classifier precedence above, particularly rows 3 and 4. +CLOSED: [2026-07-09 Thu] +Resolved: precedence as drafted, and rows 3 and 4 stay *two verdicts sharing one remedy*. They share a fix but not a diagnosis, and the hung case is the one that freezes other tooling — collapsing them would hide the more interesting fault behind the more common one. ** DONE Does the panel's own status build get the same timeout treatment? CLOSED: [2026-07-09 Thu] @@ -230,14 +245,25 @@ Investigated and answered: it already had one. =pactl.run()= defaults to =timeou What the investigation *did* find is a narrower, real bug, now fixed (dotfiles =4d42eb3=): =build_status()= guarded its two device-list reads but called =default_sink()= and =default_source()= outside the guard, so a server that answered the lists and then stopped answering raised =PactlTimeout= out of a function whose contract is to degrade. =waybar-audio= calls it with nothing catching it, so the bar's audio module died rather than dimming. A server that hangs on the *first* call never exposed this — =list_sinks= raised inside the guard and masked everything behind it, which is why the obvious test would have passed against broken code. -** TODO Which remedies are Auto, which Confirm, which Arm? -The table above proposes 1–2 Auto, 3–4 Confirm, 5 Arm. Remedy 4 drops Pulse clients briefly — is a Confirm enough, or does it want the arm treatment too when a stream is currently playing? +** DONE Which remedies are Auto, which Confirm, which Arm? +CLOSED: [2026-07-09 Thu] +Resolved: the table as drafted. Remedies 1–2 (restart wireplumber, set a present default) are Auto; 3–4 (unmute + restore volume, restart pipewire-pulse) are Confirm; 5 (restart pipewire) is Arm. + +Remedy 4 stays at Confirm rather than Arm *because* the stream guard below was taken. The guard, not the tier, is what catches the mid-meeting case — which is the honest place for it, since the danger is the state of the machine, not the identity of the remedy. Had the guard been declined, remedy 4 would have moved to Arm. + +** DONE Should the doctor refuse to run a disruptive remedy while audio is playing? +CLOSED: [2026-07-09 Thu] +Resolved: yes. A stream-active guard refuses a disruptive remedy and is overridable by pressing again, mirroring the maintenance console's live-update guard. Same shape of problem: an operation that is fine when idle and destructive mid-use, where wording alone does not stop a press. -** TODO Should the doctor refuse to run a disruptive remedy while audio is playing? -A "a stream is active — this will interrupt it" refusal, overridable by pressing again, would mirror the maintenance console's live-update guard. It is the same shape of problem: an operation that is fine when idle and destructive mid-use. Worth the machinery, or is the arm line's wording enough? +Implementation note for the builder: the guard's input is "is any node running" — and it must come from *Tier 2* (=pw-dump=), not from =pactl=. The remedies the guard protects are precisely the ones you reach for when the Pulse layer is dead or hung, so a guard that asked =pactl= would go blind exactly when it is needed, and a PipeWire-native client can be mid-playback while =pactl= answers nothing at all. -** TODO Do xruns belong in v1? -Tier 4 can count xrun and suspend lines in the unit journal. That is a *quality* signal, not a broken/working one, and it has no remedy in this feature's vocabulary. Report it as a note, or leave it out entirely until there is something to do about it? +A cold or unreadable graph means the guard cannot see a stream. Treat that as *no stream* and let the remedy proceed: refusing on an unreadable graph would wedge the doctor shut in the worst case, which is the fault it exists to repair. Say so in the wall, so the user knows the guard abstained rather than passed. + +The guard applies to remedies 3, 4, and 5 — the audible ones. Its refusal names what is playing, not just the remedy. + +** DONE Do xruns belong in v1? +CLOSED: [2026-07-09 Thu] +Resolved: left out. They are a quality signal, not a broken/working one, and they have no remedy in this feature's vocabulary. A finding you cannot act on trains you to ignore the wall — the opposite of what a doctor is for. Revisit when there is something to do about them. * Implementation phases @@ -247,17 +273,19 @@ Sequenced so each phase is independently green and independently useful. Nothing CLOSED: [2026-07-09 Thu] Shipped ahead of the rest (dotfiles =4d42eb3=). =build_status()= now guards all four pactl reads, not just the two list reads, so a half-wedged server dims the bar's audio module instead of killing it. =fake-pactl= grew =FAKE_PACTL_SLEEP_DEFAULTS= to express "answers the lists, hangs on the defaults" — the blanket sleep knob cannot, because the first call would hang and mask the bug. -** Phase 1 — =diag.py=, the four probe tiers +** DONE Phase 1 — =diag.py=, the four probe tiers Bounded probes, a context dict, fakes for every outcome including hung. No classifier, no GUI. =audio diag --json= prints the context. -** Phase 2 — =classify.py=, the verdict +** DONE Phase 2 — =classify.py=, the verdict Pure function, fixture-driven, pairwise over the parameter space. =audio doctor= (read-only) prints findings and a verdict in the CLI. Already useful from a TTY at this point. -** Phase 3 — =repair.py= + =doctor.py=, the fix path +** DONE Phase 3 — =repair.py= + =doctor.py=, the fix path One function per remedy, each re-probing what it claims to fix. =audio doctor --fix= applies the lightest matching remedy. VM-tested for remedies 1, 4, and 5. -** Phase 4 — the DOCTOR key and the results wall -GUI face over the CLI. Streams events to a wall, off the GTK thread. AT-SPI smoke over the key, the wall, and the fix path on fixtures. +** DONE Phase 4 — the DOCTOR key and the results wall +CLOSED: [2026-07-10 Fri] +Shipped (dotfiles =4320255=). GUI face over the CLI. Streams events to a wall, off the GTK thread. AT-SPI smoke over the key, the wall, and the fix path on fixtures. -** Phase 5 — flip this spec to IMPLEMENTED +** DONE Phase 5 — flip this spec to IMPLEMENTED +CLOSED: [2026-07-10 Fri] With a history line naming what shipped and what was left out. diff --git a/docs/specs/2026-07-10-audio-doctor-input-side-spec.org b/docs/specs/2026-07-10-audio-doctor-input-side-spec.org new file mode 100644 index 0000000..0e7e529 --- /dev/null +++ b/docs/specs/2026-07-10-audio-doctor-input-side-spec.org @@ -0,0 +1,494 @@ +#+TITLE: Audio Doctor, Input Side — a doctor per direction +#+AUTHOR: Craig Jennings +#+DATE: 2026-07-10 +#+TODO: TODO | DONE +#+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED + +* DRAFT Audio Doctor, Input Side +:PROPERTIES: +:ID: 45efdd66-6d22-45c0-a633-ec41079e38ec +:END: +- [2026-07-10 Fri] DRAFT — drafted. Extends the audio doctor ([[file:2026-07-09-audio-doctor-spec.org][2026-07-09-audio-doctor-spec.org]], IMPLEMENTED) to the microphone, and replaces its single DOCTOR header key with one doctor per direction. Grounded in a live survey of ratio, not memory. + +* Metadata + +| Field | Value | +|----------+----------------------------------------------------------------------------| +| Status | draft | +|----------+----------------------------------------------------------------------------| +| Owner | Craig Jennings | +|----------+----------------------------------------------------------------------------| +| Reviewer | Craig Jennings | +|----------+----------------------------------------------------------------------------| +| Related | [[file:../../todo.org][todo.org]] : "The audio doctor never checks the microphone" | +|----------+----------------------------------------------------------------------------| +| Parent | [[file:2026-07-09-audio-doctor-spec.org][2026-07-09-audio-doctor-spec.org]] — this supersedes its DOCTOR-key decision | +|----------+----------------------------------------------------------------------------| + +* Summary + +The audio doctor is output-only. It never examines the microphone, so a muted mic, a default source naming an unplugged device, and a mic the sound server cannot see all classify as =healthy=. + +This adds the input side, and in doing so answers a question no probe can: whether a machine with no microphone is broken or merely mic-less. The answer comes from the user's finger. A doctor key on the INPUTS section header means "there should be a mic here", so an empty source list under that press is a failure rather than a shrug. + +* Problem / Context + +Three separate failures, all currently invisible. + +** The classifier does not look at the source + +=diag.probe_semantic= already collects =default_source= and =default_source_present=. =classify.py= reads neither. The word "source" appears once in that module, in the graph row that counts how many exist. Every semantic rule the classifier applies to the sink (is a default set, is it present, is it muted, is it at zero volume) has no source counterpart. + +So the =healthy= verdict prints "Units are up, the graph answers, the Pulse layer answers, and the default output is present and audible." Output. Only. A user reading "the sound stack is healthy" concludes the mic is fine, and the doctor never checked. + +** Nobody can tell an absent microphone from a broken one + +A desktop with no microphone is a normal machine. So an empty source list cannot be a fault the way =no-output-devices= is, and the classifier has no way to know which machine it is on. This is not a probe problem. No amount of reading the graph settles it, because the missing fact lives in the user's head. + +** The doctor cannot see below PipeWire + +When Chrome stopped recognizing the microphone on 2026-07-10, the stack was genuinely healthy: the Shure MV7+ was the default source, unmuted, at 82%, with push-to-talk off. The doctor would have said =healthy= and been right about the stack and useless to the user. + +The layer that would have helped is the one the doctor never reads. The kernel's own capture list distinguishes "the sound server lost a microphone the kernel can see" from "nothing is plugged in", and those two faults have nothing in common. + +** Live survey (ratio, 2026-07-10) + +Facts this spec is built on, verified rather than remembered: + +- =/proc/asound/card*/pcm*c= lists capture-capable ALSA devices. On ratio: =Generic_1= (ALC623 analog), =MV7= (Shure), =BRIO= (Logitech). A =pcm*c= node means the card has a capture PCM; =pcm*p= would be playback. +- That path is a kernel filesystem read. It needs no package, spawns no process, and *cannot hang* — unlike every existing probe tier. =arecord -l= reports the same set but is owned by =alsa-utils=, which is a dependency the engine does not otherwise carry. +- =diag= already tags each stream =input= or =output= (=_STREAM_CLASSES=, mapping =Stream/Input/Audio= and =Stream/Output/Audio=). =active_streams()= returns both without distinction. +- Push-to-talk mutes the default source and leaves it muted between holds (=ptt.toggle_plan=). =ptt.read_state()= persists =armed=, so the deliberate mute is already knowable. +- A monitor source is a legitimate default source. =probe_semantic= passes =include_monitors=True= on purpose. + +* Goals and Non-Goals + +** Goals + +- Classify the input side with the same rigor as the output side: default set, default present, muted, at zero volume. +- Never report an armed push-to-talk mic as a fault. +- Distinguish "the sound server does not see a microphone the kernel does" from "no capture hardware is attached", and say which. +- Let the user assert that a microphone should exist, without a dialog. +- One doctor at a time. + +** Non-Goals + +- Rebuilding push-to-talk. See the decision below; the mute is the safety property, not the bug. +- Per-application capture routing. If Chrome cannot see a source the graph plainly has, that is Chrome's fault, and Helvum and qpwgraph exist. This spec makes the doctor able to *say* the stack is fine, which is the sentence that was missing. It does not chase the client. +- Diagnosing hardware below the kernel. If =/proc/asound= shows no capture device, this spec reports that fact. It does not test cables, USB ports, or drivers. +- Sample-rate, latency, or xrun concerns. Still out, as in the parent spec. +- Silent privileged action. The doctor may use =sudo= for the input and output expansion (see the decision "The doctor may use sudo, resolved by context at run time"), but never silently: every privileged or reboot-tail remedy defaults to Confirm or Arm tier. Auto stays reserved for user-scope, reversible remedies. + +** Scope tiers + +- *v1:* the kernel capture tier; PTT-aware source classification; input verdicts and their remedies; the two section-header doctor keys with mutual exclusion; explicit =audio doctor --output= / =--input= flags, output aliased as the default. +- *Out of scope:* PTT rebuild; per-application routing; hardware diagnosis. +- *vNext:* per-device correlation between the kernel capture set and graph sources (v1 uses the coarse set-emptiness rule instead); and a third sighting of "the graph is fine and one client cannot use it", which turns into its own design question about whether the doctor should name the application layer. Both logged to =todo.org=, not built here. + +* Design + +** For the user + +The DOCTOR section header we shipped on 2026-07-10 goes away. In its place, the OUTPUTS and INPUTS engraved section headers each grow a doctor key, beside the device counts they already carry. The keys diagnose the direction they sit above, and both write to the same results wall below. + +The keys do not repeat the direction words. The CONTROLS section already carries INPUT and OUTPUT keys — the mute toggles that read LIVE / MUTE — so a doctor key labelled OUTPUT a few rows up would read as a fourth mute control. The doctor keys are named for what they examine instead: SPEAKERS on the OUTPUTS header, MICROPHONE on the INPUTS. Each carries a distinct diagnostic treatment so a user reads it as a different kind of key, not a device control. See the decision below; that treatment is the same on every panel's doctor. + +Pressing MICROPHONE says something a probe cannot: that this machine is supposed to have a microphone. That single bit resolves the ambiguity the classifier cannot. An empty source list is normal on a desktop; an empty source list *under a press of the MICROPHONE doctor* is a failure, and the doctor now has license to say so. + +The wall names which doctor ran, and only one runs at a time. Pressing SPEAKERS while a MICROPHONE run is in flight does nothing, because the wall holds one verdict and a second run would overwrite the first mid-read. + +Diagnosis remains free and read-only. The escalation the user asked for — "we tried reloading all the devices and there is still no microphone" — is a *repair*, not a diagnosis, because reloading devices means restarting WirePlumber. So the diagnose verdict names what is wrong, and FIX is what tries the reload and re-probes. The wall then carries a claim the doctor earned rather than one it assumed: + +#+begin_example +DOCTOR · MICROPHONE diagnose · read-only + ok pipewire.service active + ok wireplumber.service active + ok pipewire-pulse.service active + ok graph 4 sinks, 3 sources (via pw-dump) + ok pulse compat answering + FAIL capture hardware kernel sees MV7, BRIO, Generic_1 — the graph has none + -- default input not checked (no input devices) + + verdict: The sound server does not see your microphone. + The kernel has 3 capture devices and PipeWire has none, so the + hardware is attached and the software lost it. FIX reloads the + device profiles. +#+end_example + +And when the kernel sees nothing either, the doctor stops rather than offering a remedy for a fault that is not there: + +#+begin_example + FAIL capture hardware no capture device is attached + + verdict: No microphone is attached. + The kernel sees no capture hardware at all, so this is a cable, a + port, or a device that is off. Nothing in the sound stack can fix it. +#+end_example + +** For the implementer + +*** A tier below the graph + +The probe ladder gains a tier at the bottom, and its position is the point. Today's order is =systemctl= (cannot hang), =pw-dump= (alive when Pulse is dead), =pactl= (may hang). The kernel tier reads =/proc/asound=, which is a filesystem read that cannot hang, cannot fail for want of a package, and answers when every other layer is dead. It is the only probe in the doctor that carries no timeout, because it cannot block. + +It answers one question: which cards expose a capture PCM. A card directory containing a =pcm*c= entry has one. That set is what separates a lost device from an absent one. + +The comparison is deliberately coarse in v1, because the two layers speak different namespaces. The kernel tier names cards by ALSA id (=MV7=, =BRIO=); the graph names sources by PipeWire =node.name=. Mapping one to the other device-by-device is a rabbit hole, and the doctor does not need it. The v1 rule is a set-emptiness test, not a per-device match: =mic-unrecognized= fires when the kernel capture set is non-empty *and* the graph has zero non-monitor hardware sources. A monitor source does not count as a microphone (it is desktop-audio capture), so a graph carrying only monitors still reads as "no real input the kernel's hardware could be feeding." That coarse rule catches the whole-mic-lost case this feature exists for without ever having to prove that card X is the same device as source Y. Per-device correlation is a vNext refinement, logged, not built. + +*** The classifier takes a direction + +=classify(ctx)= becomes =classify(ctx, side)= where side is =output= or =input=. The terminal-fault ladder is shared and unchanged: PipeWire down, WirePlumber down, Pulse hung, Pulse dead, tooling missing. All of them break both directions, and the first one that fires wins regardless of side. + +Below that the ladder forks. The output side keeps its rules exactly as they are. The input side gets, in order: no capture hardware; hardware present but no source in the graph; a default source that names an absent device; a mic muted or at zero. Every input rule is gated on the user having pressed the input doctor, which is what makes an empty source list a fault instead of a fact. + +Push-to-talk sits inside that last rule. =diag.gather()= reads =ptt.read_state()= into the context, and a muted default source with PTT armed is not a finding. It is the mic doing exactly what the user asked. The wall reports it as an informational row rather than saying nothing, because a user who has forgotten PTT is armed deserves to be told why the mic is silent. + +*** The guard learns direction + +=active_streams()= returns running streams of both directions, and =repair.guard()= refuses the audible remedies whenever any of them is running. That is already slightly wrong: a playback remedy is refused today when the only thing running is a recording. Filtering by direction fixes that and is what the mic remedy needs. + +Unmuting a microphone is not audible. It is a privacy event. If something is capturing when the remedy lands, the user's voice goes out. So the mic-unmute remedy is guarded against *input* streams with the same press-again override the audible remedies use, and the playback remedies are guarded against *output* streams. + +* Failure modes and remedies + +The whole point of a doctor is that every fault it can name has a stated fix, and the reader can see the two together. This is that map: what goes wrong at each probe tier, the verdict the classifier reaches, and the remedy FIX runs. The tiers are the probe ladder, top to bottom, so a fault lower in the table is one the doctor reaches only after the tiers above it answered clean. + +The first four tiers are shared: a broken unit, a dead graph, or a dead Pulse layer takes out both directions at once, and the same verdict serves whichever doctor the user pressed. The kernel tier and the source-side semantic rules are what this spec adds. + +| Tier | What goes wrong | Verdict | Remedy | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 1 units (=systemctl=) | =pipewire= not active, or | =graph-down= | 5: restart =pipewire= (Arm) | +| | the graph will not answer | | | +| | though the unit claims up | | | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 1 units | =wireplumber= not active | =no-session-manager= | 1: restart =wireplumber= | +| | | | (Auto) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 1 units | =pipewire-pulse= not active | =pulse-down= | 4: restart =pipewire-pulse= | +| | | | (Confirm) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 2 graph (=pw-dump=) | the graph is up but has no | =no-output-devices= | 1: restart =wireplumber=, | +| | sink | | which re-creates devices | +| | | | (Auto) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 3 pulse (=pactl=) | the socket accepts and never | =pulse-hung= | 4: restart =pipewire-pulse= | +| | answers | | (Confirm) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 3 pulse | the socket refuses | =pulse-down= | 4: restart =pipewire-pulse= | +| | | | (Confirm) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 3 pulse | a probe's own tool is | =tooling-missing= | none: the stack is | +| | missing | | unobserved, not broken | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 4 semantic, output | the default sink names a | =stale-default= | 2: set a present default | +| | device that is absent | | sink (Auto) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 4 semantic, output | the default sink is muted or | =silenced= | 3: unmute and restore volume | +| | at zero volume | | (Confirm) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 4 semantic, input | the default source names a | =mic-stale-default= | 7: set a present default | +| | device that is absent | | source (Auto) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 4 semantic, input | the default source is muted | =mic-silenced= | 8: unmute and restore volume | +| | or at zero, PTT off | | (Confirm) | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 4 semantic, input | the default source is muted, | healthy, informational row | none | +| | PTT armed | | | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 5 kernel (=/proc/asound=) | kernel capture set non- | =mic-unrecognized= | 6: reload profiles, restart | +| | empty, graph has no non- | | =wireplumber= (Confirm) | +| | monitor hardware source | | | +|---------------------------+------------------------------+----------------------------+------------------------------| +| 5 kernel | no capture device is | =no-mic-hardware= | none: a cable, a port, or a | +| | attached at all | | device that is off | +|---------------------------+------------------------------+----------------------------+------------------------------| +| all clean | nothing is wrong | =healthy= | none | +|---------------------------+------------------------------+----------------------------+------------------------------| + +Three entries carry no remedy on purpose, and they are not the same kind of no. =tooling-missing= means the doctor could not observe the stack, =no-mic-hardware= means there is nothing in the stack to fix, and =healthy= means there is nothing wrong. A doctor that offered a button for any of the three would teach the user to distrust the wall. + +Remedies 1 and 6 both restart WirePlumber, and that is deliberate: =no-output-devices= and =mic-unrecognized= are different diagnoses that happen to share a fix. Collapsing them would couple two faults that should read apart, the same call the parent spec made for =pulse-hung= and =pulse-down=. + +* Verdict clusters and remedy classes + +The verdicts and remedies above are the v1 core. Their full shape comes from a failure taxonomy built and saturation-tested after the first draft: [[file:../design/2026-07-10-audio-failure-taxonomy.org][docs/design/2026-07-10-audio-failure-taxonomy.org]], a catalogue of 74 input and 77 output real, user-reported failure modes. A blind resample (108 fresh reports through different sources, none shown the structure) forced zero new clusters, so the taxonomy is representative, not an artifact of sampling. It sorts every failure into nine symptom clusters per direction, and those clusters are what the doctor's verdicts name. + +** The clusters are the verdict structure, and they map to the probe ladder + +The nine input clusters are: (1) no capture device at all, (2) device present but records silence, (3) works but too quiet, (4) works then dies or drops, (5) the server lost a device the hardware has, (6) Bluetooth mic, (7) stack healthy but an app can't hear, (8) hardware/physical, (9) environment/config. Output mirrors this, swapping the mic-specific clusters for speaker-vs-headphone jack routing, HDMI/DisplayPort, and a degraded-audio (crackle/xrun) cluster. + +The clusters are not arbitrary. Seven of the nine are the tiers the doctor's probe ladder already separates (units, graph, pulse, semantic, kernel), and the two it cannot fully see (cluster 7 app-layer, and part of cluster 8 hardware) are exactly the ones that resolve to Guide. So the boundary between what the doctor fixes and what it can only guide falls along the tiers it already probes. The v1 verdicts in the table above are the subset of these clusters the current ladder reaches; the rest are staged below. + +** Remedy classes come from the privilege decision + +Every remedy carries one of four classes, resolved at run time (see the decision "The doctor may use sudo, resolved by context at run time"): Auto, Privileged, Reboot-tail, Guide. The taxonomy tags every entry, so the buildable share is known — a little over half of each direction. The Auto core reduces to five primitives (=set-default=, =set-card-profile=, unmute, restart-services, config-drop-in-plus-restart) applied across roughly fifty diagnoses, so the breadth is in the diagnosis, not the remedies. + +** Three new probes the taxonomy implies + +Read-only and bounded, in the spirit of the kernel tier: + +- *dmesg-pattern hints* (cluster 1). So "no capture hardware" can name the specific cause — firmware-load-fail, NHLT-missing, codec-probe-timeout — instead of a generic verdict, and the Guide can print the right command. +- *the unmute-doesn't-stick signature* (cluster 8). Issue an unmute, re-probe; a mute that will not clear is a hardware switch, not software. This is what tells an Auto (software mute) from a Guide (flip the physical switch). +- *re-probe-after-idle* (cluster 4). The current single-shot probe misses the suspend and autosuspend "works then dies" cases; a second read after an idle window catches them. + +* Alternatives Considered + +** Rebuild push-to-talk so it does not mute the mic + +The idea: keep the source unmuted, and instead unlink its nodes from capturing applications (=pw-link=), or stand up a virtual source that applications connect to and link the real mic into it only while the key is held. + +- Good, because the doctor would see an unmuted mic and need no special case, and because an application's own level meter would keep working. +- Bad, because the microphone stays live at the server and silence becomes a property of graph topology. A crash mid-hold, or a link that fails to tear down, is a hot mic while every indicator reads muted. Source-mute is the one state where the server itself guarantees nothing is captured. +- Bad, because the virtual-source variant requires every application to select the virtual device, which breaks per-application device choice — the thing the panel exists to give. +- Neutral, because the doctor does not need it: =ptt.read_state()= already persists the fact that the mute is deliberate. + +Rejected. The mute is the safety property. + +** One doctor that classifies both directions at once + +- Good, because one press and one verdict. +- Bad, because =classify()= returns exactly one verdict, so a machine with a stale sink default and a muted mic forces the classifier to rank two unrelated faults. Any ranking is a guess about what the user came to fix. +- Bad, because it cannot resolve the absent-microphone ambiguity at all. No press means no assertion, and the doctor is back to shrugging. + +Rejected in favor of one doctor per direction, which supplies the missing intent as a side effect of where the user pressed. + +** Use =arecord -l= for the kernel tier + +- Good, because it is the obvious tool and its output is human-readable. +- Bad, because it is owned by =alsa-utils=, a dependency the audio engine does not otherwise carry, so its absence would have to be modelled as a fourth =tooling-missing= case. +- Bad, because it spawns a process, which means a timeout, which means another way for the bottom tier to fail. +- Neutral, because it reads the same kernel data =/proc/asound= exposes directly. + +Rejected. The bottom tier should be the one thing in the doctor that cannot fail. + +* Review findings [1/10] + +Second review pass, 2026-07-10 (four critical lenses: buildability, technical correctness, adversarial failure-modes, internal consistency). The technical foundations checked out — every load-bearing code and =/proc/asound= claim verified against the real tree. These findings are what the passes surfaced beyond that. The mechanical inconsistencies they also found (a "two new remedies" miscount, two stray OUTPUT/INPUT doctor-key labels) are already fixed in place. + +** TODO Mic-unmute must fail closed on an unreadable graph :blocking: +The stream-active guard abstains and proceeds when =pw-dump= cannot read the graph. That is right for the *output* remedies, whose whole job is repairing a dark graph. It is wrong for the mic-unmute remedy (8), because unmuting is a privacy event, not an availability one: with the graph unreadable the guard cannot see a running capture stream, so FIX would unmute a live mic mid-recording with no second press and no warning. The mic-unmute remedy must fail *closed* on an unreadable graph — refuse, or fall back to =pactl list source-outputs= — never abstain-and-proceed. Every other finding here produces a wrong verdict; this one produces a wrong action. Folds into the open capture-guard decision. (blocking) + +** TODO A non-ALSA microphone classifies as no-mic-hardware :blocking: +A Bluetooth or USB-non-ALSA mic never appears under =/proc/asound/card*=, because bluez runs through PipeWire, not ALSA. The =no-mic-hardware= verdict is gated only on "kernel capture set empty", with no graph clause, so a working BT-headset mic (a real non-monitor source in the graph) reads as "No microphone is attached." The input ladder must consult the graph first: a working non-monitor source in the graph is =healthy= (or routes to the semantic rules) and skips the kernel tier entirely. =no-mic-hardware= fires only when the kernel set is empty *and* the graph has no non-monitor source; =mic-unrecognized= only when the kernel set is non-empty *and* the graph has none. This also resolves the ladder-order contradiction below. Folds into the open input-verdicts decision. (blocking) + +** TODO The input ladder's evaluation order is stated two ways :important: +The "For the implementer" prose lists the kernel-tier rules first (no-mic-hardware, mic-unrecognized), then the semantic rules. The failure table orders the semantic-input rows (tier 4) above the kernel rows (tier 5) and says lower rows are reached only after upper tiers answer clean. Those are opposite orders for the same four rules. The finding above settles it: graph-source presence is the top gate, so define one order and make both the prose and the table match it. + +** TODO The probe does not yet surface source mute, volume, or a monitor flag :important: +=probe_semantic= produces =default_sink_muted= / =default_sink_volume= but no source equivalents, and neither the graph nor the semantic source dicts carry a monitor flag (=pactl._device= strips =device.class=). The =mic-silenced= rule reads source mute/volume the context never provides, and the "non-monitor hardware source" test cannot be computed. Phase 0 must extend =probe_semantic= to surface the source fields and tag sources monitor-vs-hardware, and the =mic-unrecognized= rule must read the =pw-dump= source population (=Audio/Source=, monitor-excluding), not the =pactl include_monitors=True= one — reading the monitor-inclusive list would mean a monitor is always present and the rule never fires. Neither change is scoped by an open decision; both would stall Phase 0/1. + +** TODO The direction must thread through findings() and doctor(), not only classify() :important: +The spec says =classify(ctx)= becomes =classify(ctx, side)= but never that =findings(ctx)= gains =side= too, though the wall shows side-specific rows. =doctor.doctor()= is the sole caller and invokes both =classify= and =findings= twice each (initial and re-probe), plus =final_findings= / =final_verdict=; it must take =side= and thread it to all of them, and =cli.cmd_doctor= must pass the resolved side. Name this in the phase text so it is not left for the implementer to rediscover. + +** TODO Remedy 6 restarts WirePlumber unguarded, mid-call :important: +=mic-unrecognized='s remedy (6, restart WirePlumber) is in neither guard set, so a user on a call can press FIX and reload the device profiles with no guard and no second-press consent. A profile reload can re-select the default sink and drop the call's routing. Decide whether remedy 6 is guarded — against streams of *both* directions, since a WirePlumber restart touches the whole graph — as part of the open capture-guard decision. + +** TODO A stale armed=true PTT state can mask a genuinely muted mic :important: +The safe-degrade argument covers only an unreadable or missing PTT state file (=read_state= returns disarmed). It does not cover =armed=true= left stale — Hyprland restarted and dropped the binds, or the mic was muted by the CONTROLS toggle for an unrelated reason while PTT happened to be armed. =read_state= cannot detect staleness, so the doctor reports "healthy, PTT armed" on a genuinely dead mic, the exact failure this feature exists to fix. At minimum, name the limitation; better, have the informational row say the mic is muted *and* that PTT is armed, so the user is not told the mic is simply fine. + +** TODO Minor build-notes to fold in before Phase 3 :minor: +Three smaller items the passes raised. The =healthy= verdict text is output-worded ("the default output is present and audible") and needs side-aware wording for a clean =MICROPHONE= run. The two keys must clear =doctor_running= on the worker's error path so a raised worker cannot wedge both keys insensitive for the session — the shipped =bg= / =doctor_abort= pattern already does this, so it is a spec-explicitness note, not a code gap. And a USB mic pressed during its ~1s enumeration window (kernel sees the card before WirePlumber creates the source) fires a transient =mic-unrecognized=; self-correcting, worth a line acknowledging the race. + +** TODO Open Decisions still gate implementation :blocking: +The workflow treats unresolved decisions as implementation blockers, because a builder would be accepting product and safety tradeoffs mid-build. Partly addressed as of 2026-07-10: Craig closed the push-to-talk and kernel-tier decisions, and the CLI-flag decision closed with his direction, so the cookie is now =[6/9]=. Three decisions remain open — the direction-aware capture guard, the input verdict and remedy table, and the doctor-key naming and cross-panel style. Close or explicitly risk-accept each before implementation starts. (blocking) + +** DONE Kernel capture devices are not mapped to graph sources :blocking: +CLOSED: [2026-07-10 Fri] +The finding: the kernel names cards by ALSA id and the graph names sources by PipeWire =node.name=, so "compare the kernel capture set against the graph's source list" was undefined, and monitor sources muddied it further. Resolved 2026-07-10 by adopting the reviewer's coarse rule as the v1 definition: =mic-unrecognized= fires when the kernel capture set is non-empty and the graph has zero non-monitor hardware sources. No per-device correlation, no namespace mapping — that whole-mic-lost case is the one this feature exists for, and per-device matching is logged as vNext. The rule now appears in the "For the implementer" kernel-tier note, the failure table, and the input-verdicts decision. + +* Decisions [7/10] + +** DONE A doctor key per direction, on the section headers +CLOSED: [2026-07-10 Fri] +Context: an empty source list is normal on a mic-less desktop and a failure on a machine that should have one. No probe can tell them apart. + +Decision: we will put a doctor key on each of the OUTPUTS and INPUTS engraved section headers, and retire the standalone DOCTOR header and its DIAGNOSE key. Pressing the input doctor asserts that a microphone should exist. + +Consequences: the ambiguity is resolved by the user without a dialog, and the check sits beside the devices it judges. Harder: this supersedes a decision in the parent spec that shipped hours earlier, so the parent's history must say so. The panel also grows a third and fourth console key, and the 400px width has to absorb them. + +** DONE One doctor at a time +CLOSED: [2026-07-10 Fri] +Context: the wall holds one verdict, and a run clears the wall when it begins. + +Decision: we will allow only one doctor run in flight. SPEAKERS must finish before MICROPHONE can be pressed, and both keys go insensitive for the duration. + +Consequences: the wall is never a mix of two runs, and "which verdict is this" never needs asking. Harder: nothing, since the model already clears on begin and locks on =doctor_running=. The keys simply join the lock. + +** DONE An absent microphone is not a fault unless the user says so +CLOSED: [2026-07-10 Fri] +Context: =no-output-devices= is a fault because a machine with no way to play sound is broken. The mirror is false: a machine with no way to record is ordinary. + +Decision: we will not add a source-side =no-input-devices= fault. The input rules fire only under a press of the input doctor, which supplies the assertion that a mic should exist. + +Consequences: the doctor never cries wolf on a mic-less desktop. Harder: the classifier now takes a side parameter, so it is no longer a pure function of the context alone. It stays pure; the side is an input. + +** DONE Does push-to-talk get rebuilt? +CLOSED: [2026-07-10 Fri] +Context: PTT mutes the default source and leaves it muted between holds, so a naive mirror of the sink rules would classify an armed mic as "silenced, FIX unmutes it". Pressing FIX would break PTT and leave a hot mic the user believes is muted. Craig asked whether PTT could avoid muting. + +Decision: we will not rebuild PTT. =diag.gather()= will read =ptt.read_state()= into the context, and an armed-PTT muted mic will be reported as an informational row rather than a finding. + +Consequences: a few lines rather than a rewrite, and the safety property is preserved — source-mute is the only state where the server guarantees no capture. Harder: the doctor now depends on a runtime state file, so a stale or unreadable file must degrade to "not armed" (which =read_state()= already does) and the mic then reads as a genuine finding. That is the safe direction: it over-reports a muted mic rather than under-reporting a hot one. + +Craig ratified 2026-07-10: not rebuilding is fine. + +*** Discussion +The alternatives are recorded above under "Rebuild push-to-talk so it does not mute the mic". The load-bearing argument is that link-based silence fails open. + +** DONE The kernel tier reads =/proc/asound=, not =arecord= +CLOSED: [2026-07-10 Fri] +Context: the doctor needs a layer below PipeWire to tell a lost microphone from an absent one. + +Decision: we will read =/proc/asound/card*/pcm*c= directly. A card exposing a capture PCM has capture hardware. + +Consequences: the bottom tier cannot hang, cannot be absent, and needs no timeout — the only probe in the doctor with that property. Harder: it is Linux-specific and reads a kernel interface rather than a supported API, so a kernel that reorganizes =/proc/asound= would break it silently. The fallback is that an unreadable =/proc/asound= yields "unknown", never "no hardware", so the doctor abstains rather than lying. + +Craig ratified 2026-07-10: all okay. + +** TODO The capture guard, and what it refuses +Context: unmuting a microphone is not audible, but it is a privacy event. =active_streams()= currently ignores stream direction, so a playback remedy is refused today when only a recording is running. + +Decision (proposed): we will make the guard direction-aware. Playback remedies (3, 4, 5) refuse on running *output* streams; the mic-unmute remedy refuses on running *input* streams. Both take the same press-again override, and both abstain on an unreadable graph, as today. + +Consequences: the guard stops over-refusing, and unmuting a mic mid-recording takes deliberate consent. Harder: it changes the behavior of shipped remedies, so the existing guard tests move with it rather than being deleted. + +Owner: Craig. By: before Phase 2 lands. + +** TODO The input verdicts and their remedies +Context: the input side needs its own verdict vocabulary and its own remedy table entries. + +Decision (proposed): four new verdicts, and three new remedies. + +| Verdict | Means | Remedy | Tier | +|---------------------+------------------------------------------+-------------------------------------------+---------| +| =no-mic-hardware= | the kernel sees no capture device | none | — | +|---------------------+------------------------------------------+-------------------------------------------+---------| +| =mic-unrecognized= | kernel capture set non-empty, graph has | 6: reload device profiles (restart | Confirm | +| | no non-monitor hardware source | WirePlumber) | | +|---------------------+------------------------------------------+-------------------------------------------+---------| +| =mic-stale-default= | the default source names a device that | 7: set a present default source | Auto | +| | is absent | | | +|---------------------+------------------------------------------+-------------------------------------------+---------| +| =mic-silenced= | the default source is muted or at zero, | 8: unmute and restore the mic volume | Confirm | +| | and PTT is not armed | | | +|---------------------+------------------------------------------+-------------------------------------------+---------| + +=no-mic-hardware= carries no remedy for the same reason =tooling-missing= carries none: offering a fix for a fault that is not in the stack teaches the user to distrust the wall. + +=mic-unrecognized= uses the coarse set-emptiness rule from the "For the implementer" kernel-tier note, not a per-device match. The kernel names cards by ALSA id and the graph names sources by PipeWire =node.name=; those are different namespaces, and correlating them device-by-device is a vNext refinement. v1 fires =mic-unrecognized= when the kernel capture set is non-empty and the graph has zero non-monitor hardware sources. This resolves the second spec-review finding, which flagged the matching rule as undefined. + +Consequences: the verdict table grows from ten to fourteen, and =classify.findings()= grows a capture-hardware row and a default-input row. Harder: remedy 6 restarts WirePlumber, which remedy 1 already does for a different verdict. Two remedies invoking one action is fine; collapsing them would couple two diagnoses that should stay apart. + +Owner: Craig. By: before Phase 1 lands. + +** DONE The CLI grows explicit =--output= and =--input= flags, output aliased as the default +CLOSED: [2026-07-10 Fri] +Context: =audio doctor= today accepts only =--fix= and =--force=. There is no =--output=, so adding a lone =--input= would read as a special case bolted onto a bare command rather than one of a symmetric pair. + +Decision: we will add both =--output= and =--input= as explicit direction flags, and alias bare =audio doctor= to =audio doctor --output=. Both accept =--fix= and =--force=. The two flags are mutually exclusive; passing both is an error. + +Consequences: the two directions are named symmetrically, and =--input= has a sibling to pair against rather than standing alone. Bare =audio doctor= keeps its current meaning, so nothing that scripts it breaks, and "output is the default" is now a stated alias a reader can see rather than a convention to infer. Harder: =audio doctor --input --fix --force= is a long line, which is the cost of explicitness over =audio doctor input=; the subcommand form was rejected because it would break every existing call. + +Craig's call, 2026-07-10: "alias the command and have =audio doctor --output= be the one that is run by default. That way it's symmetrical, and =audio doctor --input= has a place." + +** TODO Name and style the doctor keys, distinct from the device controls +Context: the CONTROLS section already carries INPUT and OUTPUT keys — the mute toggles that read LIVE / MUTE. A doctor key labelled OUTPUT or INPUT on the section header a few rows up would read as a fourth mute control. Craig flagged the collision and asked for a name and a look that mark the doctor keys as a different function, held consistent across every panel. + +Decision (proposed): the doctor keys are named for what they diagnose — SPEAKERS on the OUTPUTS header, MICROPHONE on the INPUTS. Craig delegated the naming ("I'll let you choose"); these are the choice. Each key carries a distinct diagnostic treatment: a small health glyph (a stethoscope, =nf-md-stethoscope=) ahead of the word, plus a subtly distinct outline, so the key stays inside the dupré aesthetic while reading as diagnostic rather than a device control. The same treatment applies to the doctor keys already shipped in the other panels — net's DOCTOR, maint's CLEAN UP / REVIEW & FIX — so a doctor key looks like a doctor key everywhere. + +Consequences: no word collides with the CONTROLS keys; the two doctor keys carry distinct AT-SPI names the smoke can target; the diagnostic affordance is consistent across the panel family. Harder: it touches every panel's =gui.py= and the shared panel CSS, so the cross-module restyle is its own task rather than part of this build, and it sits beside the already-filed "copy + close on every wall" convergence. The exact glyph and outline are an aesthetic call that wants Craig's eye on a screenshot before it lands — which is why this stays open while the naming is settled. + +Owner: Craig (the visual). By: before Phase 3, where the keys are built. + +** DONE The doctor may use sudo, resolved by context at run time +CLOSED: [2026-07-10 Fri] +Context: the parent spec decided "no sudo anywhere", correct when the feature was purely user-scope PipeWire. The input and output expansion reaches firmware, ALSA saved state, modprobe overrides, and package management, which genuinely need root. Craig also wants the CLI usable as a generic doctor on machines that are not archsetup installs. + +Decision: this supersedes the parent's no-sudo decision. The doctor resolves its privilege at run time from three signals — passwordless sudo available (=sudo -n true=, which succeeds or fails instantly and never hangs), a tty to prompt at, and whether it is the GUI panel. Four remedy classes follow: *Auto* (user-scope, reversible, runs anywhere), *Privileged* (needs sudo — runs silently where passwordless sudo exists, prompts in a CLI with a tty, and degrades to Guide only in a GUI with neither), *Reboot-tail* (the applicable part runs, then the doctor instructs the reboot it cannot complete or verify past), and *Guide* (nothing to run: a physical switch, a BIOS setting, a wait-for-upstream fix). Every archsetup install has passwordless sudo, so on Craig's machines Privileged remedies run; the generic-CLI case prompts. + +Consequences: many failure modes that were guide-only (=alsactl store=, a modprobe override, removing a conflicting package) become applicable remedies on archsetup and on any CLI. Harder, and load-bearing: passwordless sudo is not consequence-free, so every Privileged and Reboot-tail remedy defaults to *Confirm or Arm* tier, never silent Auto — a firmware install or a module reload always takes a deliberate second press even when sudo will not prompt. Auto stays reserved for the user-scope, reversible remedies. This model is not audio-specific: the net, bluetooth, and maint doctors adopt the same run-time privilege resolution and the same Confirm/Arm-default for privileged actions, tracked as its own cross-panel task. + +Craig agreed 2026-07-10 to both the model and the Confirm/Arm-default safety stance, and to making it a cross-panel standard. + +* Implementation phases + +Each phase leaves the tree green and independently useful, as the parent spec's phases did. + +** TODO Phase 0 — the kernel tier, PTT in the context, direction-aware streams +Pure engine, no classifier changes, no UI. =diag.probe_kernel()= reads =/proc/asound=; =diag.gather()= adds =kernel= and =ptt= to the context; =active_streams(graph, direction=None)= filters. =audio diag --json= shows the new context. Fakes: a temp =/proc/asound=-shaped tree via an injectable root, since the real one cannot be faked on PATH. + +** TODO Phase 1 — =classify(ctx, side)= and the input verdicts +Pure, fixture-driven. The shared terminal ladder, then the fork. Pairwise over (kernel capture set × graph sources × default source state × PTT armed). This phase adds the direction flags: =--output= and =--input=, mutually exclusive, with bare =audio doctor= aliased to =--output=. =audio doctor --input= read-only prints findings and a verdict, and =audio doctor= keeps printing exactly what it does today. + +** TODO Phase 2 — the input remedies and the direction-aware guard +Remedies 6, 7, 8, each re-probing its own claim. The guard splits by direction. =audio doctor --input --fix=. The existing guard tests move rather than being deleted. + +** TODO Phase 3 — two section-header doctor keys +Retire the DOCTOR header row and its DIAGNOSE key. OUTPUTS and INPUTS each gain one. Mutual exclusion on =doctor_running=. The wall header names the run. AT-SPI smoke over both keys and the fix path, on fixtures. + +** TODO Phase 4 — flip this spec to IMPLEMENTED +And amend the parent spec's history to record that its DOCTOR-key decision was superseded here. + +** Later phases — the taxonomy expansion (vNext) +v1 (phases 0-4) ships the buildable Auto core and the Guide tail for the clusters the current probe ladder already separates, and flips this spec to IMPLEMENTED. The rest of the taxonomy ([[file:../design/2026-07-10-audio-failure-taxonomy.org][the taxonomy]]) is staged as later phases so v1 is not blocked on them. Each is a small, independent addition; the taxonomy is the backlog and this list is the order: + +- The dmesg-hint probe and the firmware/driver Guide tail (cluster 1). +- The unmute-doesn't-stick signature probe and the hardware-layer Guide (cluster 8). +- Re-probe-after-idle for the suspend and autosuspend cases (cluster 4). +- The Privileged and Reboot-tail remedy machinery: the run-time privilege resolution (=sudo -n= / tty / GUI detection) and the Confirm/Arm-default floor. +- The app-layer Guide tail (clusters 7 and 9): the tail on a healthy verdict that names the likely app or portal cause and prints the fix. + +These grow into their own spec when picked up. + +* Acceptance criteria + +- A machine with no capture hardware, under a press of the input doctor, reports =no-mic-hardware= and offers nothing to press. +- A machine whose kernel lists a capture device the graph has no source for reports =mic-unrecognized=, and FIX reloads the device profiles and re-probes. +- An armed-PTT muted mic classifies as =healthy=, with an informational row saying push-to-talk is armed. +- A muted mic with PTT disarmed classifies as =mic-silenced= and FIX unmutes it. +- FIX on the mic while something is recording is refused, and a second press proceeds. +- Restarting a playback remedy while only a microphone is recording is no longer refused. +- Pressing SPEAKERS while MICROPHONE is running does nothing. +- =audio doctor= with no flag behaves exactly as it does today. + +* Readiness dimensions + +- *Data model & ownership* — the context dict gains =kernel= (generated, per-probe) and =ptt= (read from the runtime state file that =ptt.py= owns; the doctor never writes it). +- *Errors, empty states & failure* — an unreadable =/proc/asound= yields =unknown=, never "no hardware". An unreadable PTT state reads as disarmed, which over-reports a muted mic rather than under-reporting a hot one. Both are the safe direction. +- *Security & privacy* — the mic-unmute remedy is the only privacy-relevant action in the feature, and it is guarded against running capture streams with an explicit second-press override. No credentials, no logs. +- *Observability* — the wall is the observability surface, and it names which doctor ran. The =--json= output carries the full context including the kernel tier. +- *Performance & scale* — the kernel tier is a directory listing. Everything else is unchanged. +- *Reuse & lost opportunities* — =classify.findings()= remains the single row-builder for the CLI and the GUI. =ptt.read_state()= is reused rather than reimplemented. =diag.unit_active= stays the single source of truth for unit state. +- *Architecture fit* — the fork lives in =classify()=, which is pure and already fixture-tested. The GUI change is confined to the section-header rows and the model's key-sensitivity logic. +- *Config surface* — none. N/A. +- *Documentation plan* — the module docstrings carry the design, as in the parent. No user docs; the wall is the documentation. +- *Dev tooling* — =make test= and =make test-panel-audio= cover it. The kernel tier needs an injectable root rather than a PATH fake, which is a new fixture shape for this package. +- *Rollout, compatibility & rollback* — =audio doctor= with no flag is unchanged, so nothing that scripts it breaks. The GUI change is not reversible by config; it is a redesign of a key that shipped the same day. +- *External APIs & deps* — =/proc/asound= layout verified live on ratio, 2026-07-10. No new packages. + +* Risks, rabbit holes, and drawbacks + +The kernel tier reads an interface rather than an API. If a future kernel reorganizes =/proc/asound=, the probe reports =unknown= and the doctor abstains, which is the failure mode we want, but nothing will announce the breakage. A test against a recorded tree pins the shape we expect. + +Remedy 6 restarts WirePlumber to reload device profiles, which is what remedy 1 already does. The temptation is to collapse them. Do not: the two verdicts describe different faults, and a shared remedy is not a shared diagnosis. The parent spec made exactly this call for =pulse-hung= and =pulse-down=. + +The panel is 400px wide and gains two console keys while losing one row. The layout needs eyes on it before Phase 3 closes, and no automated check will catch an ugly one. + +This spec supersedes a decision from a spec that reached IMPLEMENTED the same day. That is a smell worth naming: the DOCTOR-key placement was decided without the input side in view, and one conversation with the user overturned it. The lesson is not that the first decision was careless. It is that the input side should have been in scope from the start, and the parent's Non-Goals never said it was not. + +* Review and iteration history + +** 2026-07-10 Fri @ 18:52:07 -0500 — Craig Jennings — Reviewer +What changed or was recommended: a second review pass across four critical lenses — buildability, technical correctness, adversarial failure-modes, internal consistency. Verified every load-bearing code and =/proc/asound= claim against the real tree; all held. Recorded eight new findings (two blocking: mic-unmute failing open on an unreadable graph, and a non-ALSA mic mis-classifying as no-mic-hardware) and fixed three mechanical inconsistencies in place (a remedy miscount and two stray OUTPUT/INPUT doctor-key labels). +Why: the design was sound at the tier level, but it had two safety and correctness holes that only surface on real hardware (a webcam or Bluetooth mic, an unreadable graph), plus three build gaps that sit outside the open decisions and would stall Phase 0/1. +Artifacts: [[*Review findings][Review findings]]; code read across =~/.dotfiles/audio/src/audio/= (=diag.py=, =classify.py=, =repair.py=, =doctor.py=, =ptt.py=, =pactl.py=); live =/proc/asound= survey on ratio. + +** 2026-07-10 Fri @ 10:55:19 -0500 — Codex — Reviewer +What changed or was recommended: applied the spec-review workflow and recorded two blocking findings: open Decisions still gate implementation, and kernel-capture-to-graph-source matching is undefined. +Why: the current implementation confirms the microphone gap and output-only classifier, but these unresolved choices would force the builder to decide safety and classification behavior during implementation. +Artifacts: [[*Review findings][Review findings]]; current code read in =~/.dotfiles/audio/src/audio/diag.py=, =classify.py=, =repair.py=, =cli.py=; related tracking in [[file:../../todo.org][todo.org]] under "The audio doctor never checks the microphone". + +** 2026-07-10 Fri @ 10:38:12 -0500 — Craig Jennings — Author +What: drafted the spec. Four probe tiers become five, the classifier takes a direction, the panel gets a doctor per direction, and push-to-talk stays as it is. +Why: the doctor calls a stack healthy while the microphone is dead, and the ambiguity of an absent mic cannot be resolved by any probe. +Artifacts: live survey of ratio (=/proc/asound= capture nodes, =_STREAM_CLASSES=, =ptt.read_state()=), and the Chrome incident of the same morning. diff --git a/docs/specs/2026-07-11-bt-doctor-expansion-spec.org b/docs/specs/2026-07-11-bt-doctor-expansion-spec.org new file mode 100644 index 0000000..b255b71 --- /dev/null +++ b/docs/specs/2026-07-11-bt-doctor-expansion-spec.org @@ -0,0 +1,250 @@ +#+TITLE: Bt Doctor Expansion — name the blob, catch the boot-disable +#+AUTHOR: Craig Jennings +#+DATE: 2026-07-11 +#+TODO: TODO | DONE +#+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED + +* DOING Bt Doctor Expansion +:PROPERTIES: +:ID: 3d4d61c4-e5df-44e9-b8e0-40b31452c3f7 +: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:=. Phases 0-1 (the two read-only probes + the firmware-hint Guide verdict) are buildable now and =:solo:=; Phase 2 (the persistent fix) is gated on the shared cross-panel privilege model. +- [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. + +* 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 bt doctor walks a clean chain — adapter → rfkill → bluetooth.service → powered → per-device → audio profile — and applies four safe repairs (unblock, power-on, service-restart, a2dp). The failure taxonomy shows the chain is structurally right but blind in two specific spots the user hits most: when there is no adapter it says "no adapter found" without naming *why* (a missing firmware blob the dmesg log already names), and it never checks whether the adapter is configured to power on at boot, so an =AutoEnable=false= laptop that is dead every login reads as a healthy powered-off adapter. This spec adds those two probes and adopts the cross-panel privilege model. It is an expansion of a working doctor. + +* Problem / Context + +The taxonomy sorted ~55 real bluetooth failure modes into five clusters keyed to the doctor's chain. Read against the live engine (=~/.dotfiles/bluetooth/src/bt/doctor.py=), the doctor already handles cluster 2 well (unblock, service-restart, power-on are its exact auto-fix tiers) and cluster 5 partially (the =a2dp= repair forces the A2DP profile for a card stuck in HSP/HFP or with no sink). Clusters 3 (pairing) and 4 (connection stability) are deliberately light — the doctor treats connecting and pairing as user intents, not health repairs, and that is a correct design choice, not a gap. The two real gaps are at the ends of the chain. + +** "No adapter found" doesn't say why (cluster 1) + +=_adapter_step= reports a missing adapter as "no Bluetooth adapter found (hardware/driver)" and stops. But the taxonomy's cluster 1 is almost entirely *firmware* faults, and the kernel already logged the specific cause: "Direct firmware load for intel/ibt-…sfi failed", "mediatek/BT_RAM_CODE_MT7961… failed", "BCM: Patch …hcd not found", "Reading QCA version information failed". The doctor has that log a =journalctl -k= read away and doesn't consult it. A user who sees "no adapter — hardware/driver" is left to search; a user who sees "the Intel firmware blob ibt-20-1-4.sfi failed to load — update linux-firmware and reboot" has the fix. This is the direct parallel to the audio doctor's cluster-1 dmesg-hint proposal, and the same read-only, bounded shape. + +** 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 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) + +The doctor doesn't auto-pair or auto-connect, and the destructive re-pair is always user-confirmed. That is right. The one addition worth considering is a *stale-bond signature* — a device that fails to connect with a bond present, repeatedly — so the doctor can *offer* the re-pair with confidence instead of the user guessing. That is a candidate, held as a decision, not a v1 commitment. The rest of clusters 3/4 (physical range, USB3 noise, coexistence, connection parameters) are Guide-only and mostly out of a health doctor's reach. + +* Goals and Non-Goals + +** Goals + +- When there is no adapter, name the specific firmware/driver cause by reading the kernel log, and print the matching fix — instead of a generic "hardware/driver." +- Distinguish a *persistently* disabled adapter (AutoEnable off, service not enabled at boot, TLP-disabled) from a merely powered-off one, and offer the persistent fix rather than a one-shot power-on that dies next boot. +- Adopt the cross-panel run-time privilege model for the new root-needing repairs (editing main.conf's AutoEnable, =systemctl enable=, editing tlp.conf), under the Confirm/Arm-default floor. + +** Non-Goals + +- Auto-pairing or auto-connecting devices. Connecting and pairing are user intents; the doctor's existing stance stands. +- Fixing the firmware itself. The dmesg-hint probe *names* the missing blob and prints the update/reboot instruction; it does not install firmware or rebuild initramfs inside a diagnose run (that is a Reboot-tail Guide the user runs). +- The physical/coexistence connection-stability faults (USB3 noise, range, 2.4GHz coexistence, connection parameters). These are Guide-only and stay out of the health chain. +- Silent privileged action. Editing main.conf, enabling the service, or touching tlp.conf all default to Confirm/Arm, never silent Auto. + +** Scope tiers + +- *v1:* the dmesg firmware-hint probe for the no-adapter case (cluster 1); the boot-enablement probe distinguishing AutoEnable-off / service-not-enabled / TLP-disabled from a plain power-off (cluster 2), with the persistent fix; adoption of the run-time privilege model. +- *Out of scope:* auto-pair/auto-connect; firmware installation inside a run; the physical/coexistence connection faults. +- *vNext:* the stale-bond re-pair-offer signature (cluster 3, pending the decision below); a connection-parameter/coexistence hint tail for a "keeps dropping" verdict (cluster 4); the bluetooth-specific audio-profile expansion beyond the current a2dp repair (codec fallback, default-sink, absolute-volume — several overlap the audio taxonomy and want coordination with the audio doctor). All logged to =todo.org=. + +* Design + +** For the user + +Two verdicts that today under-inform start telling the user the actual cause. + +When there is no adapter, the wall stops at "no adapter" no longer. It names the blob and the fix: "the MediaTek BT firmware (BT_RAM_CODE_MT7961) failed to load — update linux-firmware and reboot," pulled straight from the kernel log the doctor now reads. When the log shows a clean absence (no firmware error, genuinely no controller), it says that instead — the same fail-honest distinction the audio doctor draws between "the server lost a device" and "nothing is attached." + +When the adapter is powered off, the doctor asks a second question: is it *supposed* to be on at boot? If AutoEnable is off, or bluetooth.service isn't enabled, or TLP is disabling it on startup, the verdict names that — "your adapter powers off every boot because AutoEnable is false" — and FIX offers the persistent fix (set AutoEnable, enable the service) rather than a power-on that will be undone by the next reboot. A plain "it's off right now" still gets the quick power-on. + +** 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 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]), =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. =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 + +** Print a generic "check firmware" hint without reading the log + +- Good, because it needs no journal read. +- Bad, because "check your firmware" is exactly the uselessly-generic verdict this fixes; the whole value is naming the specific blob the kernel already identified. +- Rejected. The log has the answer; read it. + +** Make power-on always persistent (set AutoEnable on every power-on) + +- Good, because the adapter would then stay on across boots without a separate verdict. +- Bad, because it conflates a deliberate boot-disable (a user or TLP choosing bluetooth-off-by-default for battery) with a fault, and silently overrides a policy the user may have set on purpose. Persistence is a distinct decision that deserves a distinct, confirmed action. +- Rejected in favor of a separate verdict the user confirms. + +** Build the stale-bond re-pair signature into v1 + +- Good, because it would let the doctor offer re-pair with confidence. +- 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 [3/3] + +** 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: 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. +Resolution: accepted as proposed (the generic "check firmware" alternative was rejected — the whole value is naming the specific blob). Owner: Craig. + +** 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: 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. + +** 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: 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. +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. + +** 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. + +** 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. + +** 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. + +** 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. + +** 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 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 (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 (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=. + +* Acceptance criteria + +- [ ] 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 *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). + +* Readiness dimensions + +- *Data model & ownership* — the diagnose report gains a firmware-hint field (generated from the kernel log) and boot-persistence signals (generated from main.conf / systemctl / tlp.conf). The doctor writes main.conf / tlp.conf / the service enablement only under the Confirm floor. +- *Errors, empty states & failure* — an unreadable journal or main.conf yields "unknown," never a false firmware or persistence verdict. A no-match firmware scan falls back to the existing generic verdict (the safe direction). +- *Security & privacy* — device MACs are already redacted by =redact.py=; the new probes read the kernel log and config files, no secrets. The firmware-signature scan must not surface unrelated journal content. +- *Observability* — the wall names the blob and the persistence cause. The =--json= output carries the new signals. +- *Performance & scale* — the journal read runs only in the no-adapter branch; the persistence read only when powered-off/blocked. Neither touches a healthy run. +- *Reuse & lost opportunities* — reuses the existing chain, =btctl=, =sysio=, =priv.py=, and the shared cross-panel privilege model rather than a bt-local one. =diagnose= stays the single report authority. +- *Architecture fit* — both probes are additive inside existing chain branches (=_adapter_step=, =_powered_step=), not new chain links. Weak point: the firmware-signature table is vendor-specific and will drift as new hardware appears; it degrades to the generic verdict rather than mis-naming, which bounds the risk. +- *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. 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 + +The firmware-signature table is the maintenance rabbit hole: each vendor logs its failure differently and new hardware adds new strings. The mitigation is that a miss degrades to the existing generic verdict — the doctor is never *worse* than today, only sometimes not-better — so the table can grow incrementally without a correctness cliff. + +The persistent-power fix must not fight a deliberate choice. A user (or TLP on a battery-conscious laptop) may want bluetooth off by default; blanket-enabling AutoEnable would override that silently. The verdict names the specific persistence cause and the fix is always confirmed, so the user sees exactly what would change before it does. + +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 +- What: drafted the bt doctor expansion from the bluetooth half of the failure taxonomy. v1 adds the dmesg firmware-hint probe (cluster 1) and the boot-enablement probe (cluster 2); the pairing/connection/audio expansions are staged to vNext. +- Why: the taxonomy showed the doctor's chain is structurally right but blind at its ends — a no-adapter verdict that won't name the firmware blob the kernel already logged, and a power-on that doesn't persist across boots. +- Artifacts: [[file:../design/2026-07-10-net-bt-failure-taxonomy.org][the net/bt failure taxonomy]]; code read across =~/.dotfiles/bluetooth/src/bt/= (=doctor.py=, =repair.py=, =btctl.py=, =sysio.py=, =audio.py=, =priv.py=). + +** 2026-07-11 Sat @ 00:59:30 -0500 — Claude Code (archsetup) — Reviewer +- 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 new file mode 100644 index 0000000..0666dfe --- /dev/null +++ b/docs/specs/2026-07-11-net-doctor-expansion-spec.org @@ -0,0 +1,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. |
