fix(update): block autoupdate setup when running as root and add doctor install subcommand (swamp-club#368)

[stack72]

Summary

• Block swamp update --setup-auto when running as root (POSIX) — the scheduler runs as the normal user and cannot replace a root-owned binary
• Add swamp doctor install subcommand that reports installation health: binary path/owner, writability, version, autoupdate status, last update outcome
• Update inline permission-denied warning to reference swamp doctor install

Fixes swamp-club#368.

What it looks like

Healthy install:

Installation Health Check

  Binary path:    /home/user/.local/bin/swamp
  Version:        20260518.045246.0-sha.e6eda98d
  Binary owner:   user
  Writable:       ✓ Binary is owned by current user

Autoupdate

  Enabled:        yes
  Cadence:        daily
  Scheduler:      installed
  Last check:     2026-05-18T09:00:00.000Z (updated)

HEALTHY

Unhealthy install (root-owned binary):

Installation Health Check

  Binary path:    /usr/local/bin/swamp
  Version:        20260509.235714.0-sha.7ace6b02
  Binary owner:   root (uid 0)
  Writable:       ✗ Binary is root-owned and not writable by current user

Autoupdate

  Enabled:        yes
  Cadence:        daily
  Scheduler:      installed
  Last check:     2026-05-17T09:22:50.722Z (error)
  Last error:     Cannot update /usr/local/bin/swamp: permission denied

  ⚠ Autoupdate is enabled but the binary is not writable.

UNHEALTHY

Follow-ups

• Docs: swamp-club#372 — update doctor reference and autoupdate how-to
• UAT: systeminit/swamp-uat#224 — end-to-end test scenarios

Test plan

• 12 new unit tests (6 domain, 4 renderer, 2 CLI) — all pass
• Full suite: 5980 passed, 0 failed (1 flaky shutdown_handlers_test unrelated)
• deno check, deno lint, deno fmt — all clean
• Manual verification: swamp doctor install and swamp doctor install --json both produce correct output
• Verified works outside a swamp repo (no --repo-dir needed)
• swamp doctor --help lists install subcommand

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. Missing remediation advice for scheduler-not-installed (src/presentation/renderers/doctor_install.ts)
   The hasProblem check marks the install UNHEALTHY when autoupdate.enabled && !schedulerInstalled, and the Scheduler: not installed line turns red — but the advisory block at the bottom only fires for the writable === "fail" case. A user who sees UNHEALTHY with no bottom advisory (because writability is fine but the scheduler is missing) has to infer the fix themselves. A line like ⚠ Autoupdate is enabled but the scheduler is not installed. Run \swamp update --setup-auto` to install it.` would close the loop.

2. No recovery hint when last check errored but binary is now writable (src/presentation/renderers/doctor_install.ts)
   When lastEntry.outcome === "error" is the sole reason for UNHEALTHY (writable is pass, scheduler is installed), the command exits 1 with UNHEALTHY and shows the error text inline, but gives no "what next" hint. A brief Run \swamp update` to retry.` below the Last error line would help.

3. Final status wording differs from sibling doctor commands (src/presentation/renderers/doctor_install.ts)
   doctor_extensions ends with OVERALL: PASS / OVERALL: FAIL; doctor_install ends with HEALTHY / UNHEALTHY. Both live under swamp doctor, so users who run both will see different conventions. Either is fine on its own — worth aligning if a future pass standardises the doctor suite.

Verdict

PASS — the new swamp doctor install command is well-scoped, help text is clear, both log and JSON modes are implemented, the root-check error in update --setup-auto is actionable, and exit codes are correct. The suggestions above are advisory and do not block merge.

[github-actions]

Code Review

Clean, well-structured PR. The domain logic, presentation layer, CLI wiring, and root guard all follow established codebase patterns.

Blocking Issues

None.

Suggestions

1. Write-probe race window (src/cli/commands/doctor_install.ts:50-56): The probeBinaryWritable dep writes to binaryPath + ".swamp-write-test" — the same path used by runSetupAuto in update.ts:145. If a user somehow runs both commands concurrently, they'd race on the same probe file. This is extremely unlikely in practice, but a randomized suffix (e.g. .swamp-write-test.${pid}) would close it.

2. Username in owner when uid is null (src/domain/update/install_health.ts:103-106): When stat.uid is null (Windows), username falls through as the current user's name. The renderer skips the owner line for uid === null, so this is unused — but setting username to null explicitly when uid is unknown would make the data model more honest.

Both are minor — neither affects correctness or mergeability.

Overall: license headers present on all new files, both log and json output modes supported, 12 tests cover the key branches, no libswamp boundary violations, no fire-and-forget promises, root guard properly uses UserError with rethrow pattern. Looks good to ship.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. Duplicated health-status logic across renderers — src/presentation/renderers/doctor_install.ts:91-93 and src/presentation/renderers/doctor_install.ts:127-130: The hasProblem determination is computed independently in both LogDoctorInstallRenderer and JsonDoctorInstallRenderer with identical logic. If one is updated without the other, the JSON and log modes will disagree on whether an install is healthy.

   Breaking example: A future contributor adds a new unhealthy condition (e.g. cadence === "weekly" && lastEntry older than 14 days) to the log renderer but forgets the JSON renderer. swamp doctor install reports UNHEALTHY and exits 1, but swamp doctor install --json reports "overallStatus": "healthy" and exits 0.

   Suggested fix: Extract the health determination into a shared pure function (e.g. function isHealthy(report: InstallHealthReport): boolean in the domain layer or alongside the renderer factory) and call it from both renderers.

Low

1. probeBinaryWritable returns wrong result if remove fails — src/cli/commands/doctor_install.ts:51-57: If Deno.writeTextFile succeeds but Deno.remove throws (e.g. some exotic filesystem race), the catch block returns false even though the directory is writable, and the probe file is leaked. This matches the existing pattern in update.ts:145-148 and is extremely unlikely in practice, so not actionable on its own.

2. No test for "different non-root user" ownership branch — src/domain/update/install_health.ts:93-98: The else branch (binary owned by a non-root, non-current user) is untested. The logic is straightforward but a test would pin the uid:${stat.uid} message format.

3. TOCTOU between probe and scheduled autoupdate — The writability probe is a point-in-time check; permissions can change before the scheduler runs. This is inherent to the diagnostic nature of the tool and not a real bug.

Verdict

PASS — Clean dependency injection, proper error boundary in the root-check (update.ts), correct cross-platform fallbacks, thorough test coverage. The duplicated health-status logic (Medium #1) is worth extracting but doesn't block.