Summary
cmd/ox/doctor_session.go:58 invokes every session check via check.Run(ctx, false) — the fix parameter is hardcoded false instead of being threaded from opts.shouldFix(slug). When a user runs ox doctor --fix, the session checks execute in report-only mode regardless of the flag.
Consequence
Any session check with an auto-repair path is effectively neutered under --fix. The check detects the problem, returns a checkResult saying "broken and repairable", but the repair never executes because Run was told fix=false.
User experience:
$ ox doctor --fix
...
[session] detected N orphaned session artifacts — repairable
...
$ ox doctor --fix # same output, nothing was actually repaired
...
The user sees repair-hinting output, reasonably expects --fix to have done the repair, finds nothing changed, files a bug report.
How it was discovered
Found during a systemic scan of DoctorCheckRegistry invocation paths while investigating a separate bug (see #498). The scan was looking for registered-but-not-invoked checks; this one is the opposite pattern — the check IS invoked, but the fix plumbing is wrong.
Why it's distinct from #498
Different root cause, different fix, different blast radius.
Fix
Thread the fix bool through per-slug:
check.Run(ctx, opts.shouldFix(slug))
…where slug is whatever slug the session check registered under. One-line conceptual change per call site, but the refactor has to plumb slugs into the internal/doctor/session iteration interface, which is a modestly larger touch — probably ~20-40 lines.
Pre-fix audit needed
The fix itself is mechanical, but before landing it, enumerate which session checks actually have an auto-repair path. Some internal/doctor/session/* checks may be detect-only and therefore unaffected in practice; those don't need audit. The ones with real repair logic need individual verification that:
- The repair is safe to run under
FixLevelAuto semantics
- The repair doesn't have side effects the hardcoded
false was inadvertently guarding against (i.e., confirm the hardcoding wasn't a Chesterton's fence)
If any repair path turns out to be unsafe, that specific check stays at fix=false with an explicit comment explaining why; all others flip to opts.shouldFix(slug).
Context
Discovered during the team-timezone revert PR (internal beads: ox-h8v). The revert PR explicitly scoped this out per CLAUDE.md's "revert PR is not cleanup time" rule. Filing here so the work can be picked up with a proper test plan.
Related
Summary
cmd/ox/doctor_session.go:58invokes every session check viacheck.Run(ctx, false)— thefixparameter is hardcodedfalseinstead of being threaded fromopts.shouldFix(slug). When a user runsox doctor --fix, the session checks execute in report-only mode regardless of the flag.Consequence
Any session check with an auto-repair path is effectively neutered under
--fix. The check detects the problem, returns acheckResultsaying "broken and repairable", but the repair never executes becauseRunwas toldfix=false.User experience:
The user sees repair-hinting output, reasonably expects
--fixto have done the repair, finds nothing changed, files a bug report.How it was discovered
Found during a systemic scan of
DoctorCheckRegistryinvocation paths while investigating a separate bug (see #498). The scan was looking for registered-but-not-invoked checks; this one is the opposite pattern — the check IS invoked, but the fix plumbing is wrong.Why it's distinct from #498
DoctorCheckRegistrybut their call sites inrunDoctorChecksare missing entirely. Checks never run at all under bareox doctor.runDoctorChecks→doctor_session.go:58, but thefixbool is hardcodedfalserather than plumbed through fromopts. Checks run but can't auto-repair.Different root cause, different fix, different blast radius.
Fix
Thread the fix bool through per-slug:
…where
slugis whatever slug the session check registered under. One-line conceptual change per call site, but the refactor has to plumb slugs into theinternal/doctor/sessioniteration interface, which is a modestly larger touch — probably ~20-40 lines.Pre-fix audit needed
The fix itself is mechanical, but before landing it, enumerate which session checks actually have an auto-repair path. Some
internal/doctor/session/*checks may be detect-only and therefore unaffected in practice; those don't need audit. The ones with real repair logic need individual verification that:FixLevelAutosemanticsfalsewas inadvertently guarding against (i.e., confirm the hardcoding wasn't a Chesterton's fence)If any repair path turns out to be unsafe, that specific check stays at
fix=falsewith an explicit comment explaining why; all others flip toopts.shouldFix(slug).Context
Discovered during the team-timezone revert PR (internal beads:
ox-h8v). The revert PR explicitly scoped this out per CLAUDE.md's "revert PR is not cleanup time" rule. Filing here so the work can be picked up with a proper test plan.Related