Add construct doctor - #1071
Merged
Merged
Conversation
A read-only diagnostic for the install: paths and writability, config parse, legacy `agentd` layout, daemon liveness and build skew, harness availability, third-party OAuth logins, smith credentials, ambient feature status, router/web-UI ports, and PATH shadowing. The design constraint that shapes everything: doctor is run precisely when something is already broken, so it must work with the daemon down and must never mutate the machine it inspects. It does not autostart a daemon, create directories, rewrite config, refresh a login, or leave probe files behind — writability is an access() check, and update availability is read from the existing on-disk cache without the network refresh `cached_latest_version` would trigger. `Doctor` is excluded from the interactive upgrade prompt for the same reason. Because the checks need the daemon's own probes but must run without a daemon, the logic lives in a new `pub mod doctor` in the daemon crate exposing only plain-data types, with the CLI injecting what only it can observe (whether a daemon answered, and what it said). The harness probe ladder moves out of `SessionManager` into `availability::probe_harness` so both callers ask the identical question — which is what lets doctor name the most confusing real failure, "claude works in my terminal but construct says it's missing." Severity is a claim about the machine: `error` means construct cannot work here and is the only thing that sets exit 1 (unusable state dirs, an unparseable config, a socket that accepts connections but does not answer). A stopped daemon and an expired third-party login are warnings — most healthy machines have both. Every non-ok finding carries the exact command that addresses it; there is no --fix. `--json` emits every finding id unconditionally, with unrunnable checks marked skipped rather than omitted. Spec 0168.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
construct doctor— a read-only diagnostic for the install itself.Construct sets up with one command and then depends on a fair amount of ambient
machine state: four directories, a config file, a daemon, a socket, several
third-party CLIs found through
PATH, and several third-party OAuth logins itreads but does not own. Today, when any of that is wrong, you meet the failure at
the point of use, one symptom at a time. This gives you the whole picture in one
command, and gives bug reports something to paste.
The constraint that shapes it
Doctor is run precisely when something is already broken. So:
mkdir, no configrewrite, no login refresh, no probe files. Writability is an
access()check.Update availability is read from the existing on-disk cache — deliberately not
cached_latest_version, which spawns a refresh againstapi.github.com.Doctoris also added tocommand_allows_upgrade_prompt's exclusion list; theupgrade prompt does a network round trip and can block on stdin.
Checks that genuinely need a daemon are still emitted, as
infomarkedskipped, never omitted — so
--jsonconsumers always get every finding id.Those two together ruled out an IPC method: it would be unreachable exactly when
you need it. Instead the check logic lives in a new
pub mod doctorin the daemoncrate (it needs
config,availability,router::oauth,legacy_migration_notice),exposing only plain-data types, and the CLI injects what only it can observe —
whether a daemon answered, and what it said.
The harness probe ladder moves out of
SessionManagerintoavailability::probe_harnessso the daemon and doctor ask the identicalquestion. That's what lets doctor name the single most confusing real failure:
"
claudeworks in my terminal but construct says it's missing" — aPATHdifference between your shell and the daemon's environment.
Severity means something
error= construct cannot work here, and it is the only thing that sets exit 1.The error-capable set is small on purpose: unusable state directories, an
unparseable config, and a socket that accepts connections but doesn't answer.
A stopped daemon is a warning, not an error — a CLI-only user with no TUI open
has no daemon and is perfectly healthy. So is an expired third-party login. If
those exited non-zero,
doctorwould fail on most machines and the exit codewould mean nothing.
Every non-
okfinding carries the exact command that addresses it. There is no--fix: the remedies span other vendors' tools (codex login,kimi), yourfilesystem permissions, and your editor — Construct shouldn't silently act inside
systems it doesn't own.
Output
Daemon up, on my machine — note it found three real things (an expired kimi
login, build skew from a dirty worktree, and a 349 MiB daemon log):
Daemon down, isolated home — still exit 0, and no socket was created. The port
warnings are correct here: my real daemon holds 8917/5746, which is exactly the
condition that check is for.
Malformed
config.toml— the only non-daemon path that exits 1. The parser's owncaret diagram is carried through, and the rest of the report still renders against
built-in defaults rather than aborting at the first bad line:
Tests
classify_dir,classify_path_shadowing(including canonical-dedupe of a symlinked duplicate),classify_port's full truth table,login_findingasserting the blocker'sreason is passed through verbatim,
Report::summarize, the config-parse fallback(asserts every
BUILTIN_ADAPTERSname survives), and rendering (column clamping,note/fix indentation, both summary variants).
crates/e2e/tests/doctor.rs, 6 tests) — the first e2e tests that run aCLI subcommand as a subprocess and read stdout, because the properties worth
protecting are properties of the command. The load-bearing one asserts that
after a daemon-down run the socket still does not exist; that catches the
worst possible regression here.
cargo test -p construct-daemon -p construct-cli→ 1247 + 467 passing.cargo test -p construct-e2e --test doctor→ 6 passing. Clippy clean on the newfiles.
Deliberately not asserted in e2e: which harnesses are available (CI has no
claude/codex), the port findings (a developer's own daemon may hold them), andthe exact wording of any detail string.
Notes for the reviewer
cargo fmt --allreformats ~50 unrelated files on this branch —mainisn'trustfmt-clean under the current toolchain. I formatted only the files this PR
touches, so the diff stays reviewable.
probe_smithisn'tstrictly zero-network — it TCP-probes
$OLLAMA_HOST(200 ms) and shells out tosecurityon macOS. Fidelity to what the daemon actually does is worth more herethan a strict no-syscall guarantee.
crates/cliandcrates/daemon, both of whichlink into the single
constructbinary →.claude/worktrees/doctor-subcommand/target/debug/construct doctor🤖 Generated with Claude Code