Skip to content

rig 3.4.0

Choose a tag to compare

@github-actions github-actions released this 20 Sep 01:45
c4f4c38

doctor's checks are printed lines, not findings (#97)

doctor's checks are printed lines, not findings

Tickets: #84

Direction

bin/doctor.mjs is pure and returns findings; cmds.doctor gathers a snapshot and prints
them.
next.mjs is the model, not checkouts.mjs — the split is deliberate, and it is the
one design choice here worth arguing about, so it is argued below.

The finding

{ verdict: 'ok' | 'warn' | 'bad' | 'note', says, counts }

verdict picks the glyph the four existing channels already use — ✓, !, ✗, and a dim
· — and nothing else. counts is whether this finding moves the exit code, and it is a
separate axis on purpose, because today it already is one and nowhere says so: the dirty
data root warns and does not count, draft catalogue entries warn and do not count, and the
other nineteen problems++ sit inline beside identical-looking warn calls. The ✗-vs-!
rule and the counts-vs-does-not rule then get decided once, in one table, instead of
twenty-one times down the length of a print.

Why this module narrates, when checkouts.mjs refuses to

checkouts.mjs answers named outcomes and never prose, because its wording is policy served
to two audiences: "run rig save" for the data root and "git -C … status shows them" for the
tool are one outcome said twice. That reasoning does not reach here. doctor.mjs has exactly
one audience
— the doctor output — so the wording is not a policy a caller could differ on,
it is the check. next.mjs already settled this: offer(phase, says, command) carries prose,
is pure, and rig next renders the glyphs around it. A doctor module returning outcome codes
would put the twenty-one message strings back into rig.mjs and buy nothing, because the
assertions worth writing are about what it says.

The snapshot is the interface

One plain object, gathered by cmds.doctor, holding what the machine and the records answered:
node and git versions, the tool's state and release mark, the freshness reading, gh auth, twg
presence, the two git config values, the data root's checkouts.describe(), the record-format
stamp and pending migrations, per-org identity and tracker, per-work contradictions and strays,
draft catalogue entries, free space. No fs, no git, no gh below that line — which is what makes
~20 checks assertable from an object literal, the contract phase.mjs, workstate.mjs,
next.mjs, stages.mjs, dash.mjs and freshness.mjs all already state in their headers.

Decision 54 becomes a field, not a branch. A check this machine cannot make arrives as
null in the snapshot, and the pure half emits a note or nothing — never a bad. The
asymmetry that exposes is real and is kept: freshness not checked — <skipReason> does not
count, while freshness not checked — could not fetch does, because the first is a machine
that was never going to answer and the second is one that tried and failed.

The gathering stays impure, and keeps its one mutation. doctor fetches before it answers
and writes the freshness cache — it is the one command that refuses to report what the cache
last saw. That stays above the line. Nothing else there mutates, which is what keeps doctor
the command you can always run to ask a question without answering it.

Scope

  • One branch, one PR, no stages. The module without its caller is dead code, and the caller
    without the module is the function being deleted; there is no slice that lands on its own.
  • rig update ends in these checks (bin/rig.mjs:2681) and keeps doing so. cmds.doctor
    returns its findings, so update counts them rather than reading process.exitCode back.
  • feat/ branch → minor, so package.json goes to 3.4.0 (ADR-0003, decision 50).
  • Lands as decision 81 in DESIGN.md. Decision 54 is amended in prose, never reversed.

The three "did it die partway" tests stay — all of them

The issue's Wins list says this "retires a whole class of did it die before the verdict
tests". ❌ REFUTED (2026-09-19). run (bin/rig.mjs:64) calls die() when the binary is not
on PATH, so "doctor crashed partway" is a property of the probing, and the probing is the
half that stays impure. A fixture with git.present: false asserts what doctor says about
missing git; it cannot assert that doctor survives missing git. Those are different bugs, and
the second is the one that shipped (#7, the PowerShell free-space probe).

So all three keep their full value, and only a real subprocess can replace them:

  • test/installation.test.mjs:262 — PATH stripped to node plus the Windows system folders, so
    git is gone. Proves doctor complains about it and still reaches the verdict line.
  • test/installation.test.mjs:277 — PATH stripped to the node folder, so neither powershell
    nor df exists. Proves the disk line is dropped silently and the verdict still lands. This is
    decision 54 in test form.
  • test/smoke.test.mjs:720 — the only test that proves gather → decide → render joins up at
    all. After the split, a snapshot built wrong leaves every fixture passing and doctor wrong on
    every real machine; this is the only thing watching that wire, and the only thing watching
    that findings still become an exit code.

Delete nothing; add the fixtures on top. What the split actually retires is the need for a
new crippled-PATH subprocess every time a decision is added — not the existing crash guards.

One wart becomes fixable for the first time: that smoke test asserts
r.code === /only \d+ GB free/.test(r.out) ? 1 : 0 — it greps its own output to decide what to
expect, because the host's disk might be full. Once the count is
findings.filter(f => f.counts).length, that contortion can go.

Considered and rejected

  • Move the gathering into doctor.mjs too. It would make the module impure for no test
    surface — the gathering is exactly the part a fixture cannot replace — and cmds.next already
    sets the precedent of a thin impure gatherer beside a pure decider.
  • A finding per probe rather than per check. Tempting, and wrong: several checks read
    one probe (the data root's describe() feeds four findings), so probe-shaped findings would
    reintroduce the nesting this exists to flatten.
  • Keep problems as a counter the module increments. That is the mutable-accumulator shape
    that made the current function untestable. The count is
    findings.filter(f => f.counts).length, and it is the caller's arithmetic.

Context doc: https://github.com/hugoforte/rig-data/blob/main/work/doctor-findings/context.md

What landed

  • bin/doctor.mjs — pure, ~20 checks as findings, plus problemCount.
  • bin/rig.mjs — doctorSnapshot() gathers (the one impure half, and doctor's one mutation: the fetch and the freshness cache), render() prints, cmds.doctor returns its findings. 311 lines of rig.mjs replaced.
  • rig update counts those findings instead of reading an exit code back out of the process.
  • test/doctor.test.mjs — 39 fixtures. Full suite: 627 pass, 0 fail.
  • Output parity verified against the installed 3.3.0 on this machine: byte-identical bar the version, the checkout path, and the freshness line a linked worktree always skips.
  • Decision 81 in DESIGN.md; finding added to CONTEXT.md; package.json → 3.4.0.

One correction to the issue

The issue's Wins list said this "retires a whole class of did it die before the verdict tests". ❌ REFUTED — surviving a missing probe is a property of the probing, which is the half that stays impure, so all three crippled-PATH tests keep their full value and nothing was deleted. What the split retires is the need for a new subprocess every time a decision is added.

What it did fix: the smoke test no longer greps its own output for only \d+ GB free to decide which exit code to expect — it reads the count out of the verdict line and asserts the exit code agrees.

Known follow-on

#77 (more than one data root per installation) has to make doctor report every configured data root, and will want the per-data-root findings to be a repeatable group rather than one flat dataRoot object in the snapshot. Not built for here — that is one reason, not three — but it is one object away.

Full changelog: v3.3.0...v3.4.0