Repository navigation
feat(doctor): scope the sweep to machine checks with --scope machine - #2337
Conversation
Every doctor check is now classified as machine (what converge produces) or identity (the age key, Bitwarden, the gh login, the knowledge vault). Under --scope machine the identity checks do not run; each prints a SKIP naming the step of the new-machine runbook that restores it, and a skip never fails the run. A plain dotf doctor is unchanged. The age key is split from the secrets tooling check, and the configs rendered from secrets from the opencode/pi binaries check, so each check has one kind.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if pathExists(models) || pathExists(cfgPath) { | ||
| reportAgentLaunchability(sys, rep) | ||
| } |
There was a problem hiding this comment.
Behavior Regression
REAL. The launchability WARN used to run when pi was configured (~/.pi present, the former piConfigured) or when opencode.jsonc existed. The new gate tests only models.json and opencode.jsonc. Trace: ~/.pi exists, agent/models.json and opencode.jsonc are absent, the keys are bw-backed and the daemon is locked. Before, doctor WARNed with the dotf secrets unlock remedy. Now it stays silent, which is the failure WIN-012 added this check for. This also contradicts the PR claim that a plain dotf doctor is unchanged. Fix: keep the ~/.pi directory test in the gate and add a case for it to the launchability test.
| t.Errorf("the default scope skipped a check\n%s", all) | ||
| } | ||
|
|
||
| machine, _ := run(ScopeMachine) |
There was a problem hiding this comment.
Vacuous Assertion
REAL (test gap). TestRun_MachineScopeSkipsEveryIdentityCheck discards the machine-scope exit code (machine, _ := run(ScopeMachine)). The PR and tasks.md state that a skip never fails the run, but no assertion checks it. Mutation: change the identity branch in Run from rep.Skip to rep.Fail with the same text. The SKIP count assertion still passes and the test stays green. Fix: assert on failures, for example that the machine output contains no FAIL line under an identity section heading.
| dotf doctor --scope machine | ||
| ``` | ||
|
|
||
| Every identity check prints a SKIP naming the step below that restores it. A FAIL here is a defect of the bootstrap, not of your restore, and belongs in a ticket. The from-zero CI job runs this same command on fresh macOS and Ubuntu runners. |
There was a problem hiding this comment.
Inaccurate Documentation
REAL (documentation). The runbook says the from-zero CI job runs dotf doctor --scope machine. In this PR the job still runs plain dotf doctor, and the switch to --scope machine is an unchecked follow-up (#2331). Readers would assume CI already judges the machine scope. Fix: state that CI runs dotf doctor today and switches to --scope machine in #2331, or make that switch in this PR.
| func sweep(sys *System, cfg *Config, contract *Contract, rep *Report, opts Options, start string) []check { | ||
| return []check{ | ||
| {kind: kindMachine, run: func() { checkCoreTools(sys, contract, rep) }}, | ||
| {kind: kindMachine, run: func() { checkVersionedPaths(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkVersionMatch(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkSymlinks(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkProfileFiles(sys, contract, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkToolHomeEnvVars(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkOptionalTools(sys, cfg, contract, rep) }}, | ||
| {kind: kindMachine, run: func() { checkMiseTools(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkSystemPackages(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkPython(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Knowledge vault (presence)", restore: restoreVault, run: func() { checkVault(sys, rep) }}, | ||
| {kind: kindIdentity, section: "Knowledge vault hooks (secret gate)", restore: restoreVault, run: func() { checkVaultHooks(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkGitConfig(sys, rep, opts.Fix) }}, | ||
| {kind: kindIdentity, section: "Auto-memory vault link", restore: restoreVault, run: func() { checkAutoMemoryLink(sys, start, rep, opts.Fix) }}, | ||
| {kind: kindIdentity, section: "Auto-memory file shape", restore: restoreVault, run: func() { checkMemoryShape(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkPathFiles(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Secrets integrity", restore: restoreAge, run: func() { checkSecrets(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkSecretsTooling(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Age identity key", restore: restoreAge, run: func() { checkAgeIdentity(sys, rep) }}, | ||
| {kind: kindIdentity, section: "Bitwarden reach (live secrets SSOT)", restore: restoreBW, run: func() { checkBitwardenReach(sys, rep) }}, | ||
| {kind: kindIdentity, section: "bw serve daemon (optional local unlock cache)", restore: restoreBW, run: func() { checkBWServeDaemon(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Bitwarden mapping (registry -> vault)", restore: restoreBW, run: func() { checkBWMapping(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkAgentConfigSecrets(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkHiveBackendCanServe(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkHiveDaemonAnswers(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Disaster recovery", restore: restoreAge, run: func() { checkDisasterRecovery(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "PAT expiry", restore: restoreBW, run: func() { checkPATExpiry(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkGuardHooks(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkTmux(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkOpenCode(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "Agent configs from secrets", restore: restoreBW, run: func() { checkAgentSecretConfigs(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkCopilot(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkGolangciLint(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkModelMap(cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkModelPins(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkModelLimits(sys, cfg, rep) }}, | ||
| {kind: kindIdentity, section: "NaN quota", restore: restoreBW, run: func() { checkNaNQuota(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkPiExtensions(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkPiPackageRequirements(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkHarnessDrift(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkDeployDrift(sys, cfg, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkHomeDeployDrift(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkDockerEngine(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkColimaSize(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkDockerCompose(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkDeployManifest(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkAgentPresence(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkAgentSkillsMigrated(cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkDotfProvenance(sys, cfg, rep) }}, | ||
| {kind: kindMachine, run: func() { checkRepoDirResolves(sys, rep) }}, | ||
| {kind: kindIdentity, section: "spec-issue-state", restore: restoreGH, run: func() { checkSpecIssueState(sys, rep) }}, | ||
| {kind: kindIdentity, section: "branch-protection", restore: restoreGH, run: func() { checkBranchProtection(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkAntigravity(sys, rep) }}, | ||
| {kind: kindMachine, run: func() { checkOrcaHook(sys, rep, opts.Fix) }}, | ||
| {kind: kindMachine, run: func() { checkHookBinding(sys, rep, opts.Fix) }}, | ||
| } | ||
| } |
There was a problem hiding this comment.
Harness compliance
THEORETICAL. AGENTS.md Code Quality Rules set function length below 40 lines. sweep is about 59 lines. The rule has no exemption for literal tables; the Discipline Gate's executable-line exemption covers only the PR size count. Fix: split the table into per-group functions, or record an explicit exception.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
⚙️ Agent run details
|
…each identity section to its SKIP The sweep table ran past the 40-line function limit; four groups, in report order, keep each short. The machine-scope test now asserts each identity section holds only its SKIP, so a skip reported as a failure is caught. The runbook no longer says the from-zero job already runs --scope machine.
Review triageReviewer: PR-Agent (github-actions, review 5481888670, four inline findings on 1d1321a). Fixes in 57fb918.
Verification on 57fb918: |
1 participant
Summary
dotf doctor --scope machinechecks what converge produces and leaves out the owner's identity, so a fresh machine and a CI runner can be judged before the identity is restored (#2013 D10).machineoridentity; there is no default kind, so an entry added without a decision failsTestSweep_EveryCheckIsClassified.--scope machinethe 15 identity entries (vault, age key, Bitwarden, DR drill, PAT expiry, NaN quota, configs rendered from secrets, spec issue state, branch protection) do not run. Each prints a SKIP under its own section naming the restore step indocs/runbooks/guide-new-machine.md; a skip never fails the run, and--fixruns no identity fix.dotf doctoris unchanged.checkAgeIdentityfromcheckSecretsTooling, andcheckAgentSecretConfigsfromcheckOpenCode(its section renamed to "Agent configs from secrets", troubleshooting updated).guide-new-machine.md: the one-liner,--plan, how to read the converge report, then the identity restore in order (age key,bw login+dotf secrets unlock,gh auth login, the vault).Found by X1 run 38110204139: install.sh and both converges passed on fresh macOS and Ubuntu runners and only doctor failed, mostly on identity.
Spec
PLAT-001b-one-entrypoint:
tasks.mdPR 6a and the runbook task of PR 6;verification.mdD10 entry.Test plan
go build ./... && go vet ./... && GOOS=windows go vet ./... && go test ./...: exit 0golangci-lint run ./...(2.12.2, cache cleaned): 0 issuesdotf doctorvs this branch'sdotf doctor:165 passed, 11 failed, 4 warned, 22 skippedvs165 passed, 11 failed, 4 warned, 23 skipped. The section diff is the two split headers ("Age identity key", "Agent configs from secrets") plus "System packages", whose one SKIP is on main since 0.67.0, not from this PR.dotf doctor --scope machine:95 passed, 11 failed, 3 warned, 35 skipped, 15 identity SKIPs, each under its own header once, the same 11 FAILs (kubectl, hive daemon, deploy-dir drift: all machine)ParseScopetypo): each caughtFollow-ups
--scope machineafter this merges.--scope machineand drops its fabricated identity (row in PLAT-001b tasks).Knowledge
Refs #2013