fix(doctor): name the check groups dropped when config load fails

[benw5483]

What changed

gc doctor registers seven groups of checks only when the city config loads cleanly, and when that load fails all seven vanish from the run with nothing said about them. The summary gets shorter. It also reads healthier, because the checks that were failing left along with the rest, so you're looking at a shrunken run that looks like a better one.

Two halves, matching the two ways this can be fixed:

Surface it. Each config-dependent group now registers a blocking SkippedCheck naming what didn't run, the summary line gains an N check groups skipped part, and the run ends with a sentence saying the factory was not fully inspected. The config error itself moves into per-result details. Repeating it on all seven lines just buried the group names it was there to explain.

Remove the cause. Pack check scripts run with their working directory set to the pack dir, which for an imported pack lives inside gc's global import cache. gc exported GC_CITY_PATH but never FACTORY_ROOT, so a script resolving its state directory against FACTORY_ROOT fell back to the working directory and wrote into the cache clone. Now the clone is dirty. The next gc invocation refuses the dirty cache, fails config load, and drops every pack check, which closes the loop back to the first half of this PR. PackRuntimeEnv and PackRuntimeEnvMap now export FACTORY_ROOT.

Measured

I built both binaries and ran each against the same city, one whose config doesn't load. The environment was isolated (env -i, scratch HOME) so that neither run could reach a real city and quietly measure the wrong thing:

	checks registered	skipped in --json	dropped groups named
before	41	field absent	0 of 7
after	48	7	7 of 7

The seven named on the after run: pack-doctor-checks, pack-source-checks, config-validation-checks, rig-checks, data-checks, session-checks, dolt-ops-checks.

One thing this does not change, and I'd rather say it than have you find it: the exit code was already non-zero here. The pre-existing expanded-config-load check fails blocking whenever the config can't be resolved, so both binaries exit 1 on this city. What was missing is the inventory of what went unrun, not the exit status.

Controls

Both directions are pinned. I verified each by injecting the defect rather than by reading the test and trusting it:

• Positive — TestBuildDoctorChecksRegistersSkipsWhenConfigFails: on an unloadable config every group in the list must register, and the result must be error + blocking, carry the config error in its details, and name gc import install in its fix hint.
• Negative — TestBuildDoctorChecksNoSkipsWhenConfigLoads: on a healthy config no skip result may appear. Without it, every clean run would claim the factory went uninspected, and gc doctor would never exit 0 again.
• Injection — dropping one group at registration time while leaving it in the list turns the positive control red, naming exactly that group. Reverted. Green again.
• List guard — TestConfigDependentCheckGroupsAreDeclared pins the group list against an independent copy of the names.
• End to end — cmd/gc/testdata/doctor.txtar already built a broken city and ran the real binary, so the three new stdout assertions ride along on it: pack-doctor-checks, check groups skipped, not fully inspected. The unit tests prove the skips register; these prove they reach the operator. A later change to runDoctor that filtered or short-circuited output on this path would keep every unit test green while the operator saw nothing, which is the same silent-invisibility defect the PR exists to close. Run it with GC_FAST_UNIT=0 go test ./cmd/gc/ -run 'TestTutorial01/doctor$'.

Each of those three was injected separately against an untouched upstream tree rather than checked as a set. With only pack-doctor-checks added the fixture goes red naming that string, and the same holds one at a time for check groups skipped and not fully inspected. All three pass together here. Injecting them as a group would have proved only that the first one bites, since the fixture stops at its first failed assertion.

That last one exists because of something the injection pass caught. Both of the first two tests iterate configDependentCheckGroups, so the list was simultaneously the subject and the oracle: I injected a deletion of the pack-source-checks entry and everything stayed green, because the tests just looped over a shorter list and checked less. That is the standard weakness of a list that grades itself. With the guard in place the same injection goes red.

Be clear about what the guard does not cover, though, since it's the more interesting half. It catches a group being removed or renamed. It cannot catch a new cfgErr == nil gated block being added to buildDoctorChecks with no matching entry here — that block would still be dropped silently, which is exactly how the pack-source-checks gap arose in the first place. Catching that needs a tripwire over the gated blocks themselves, and I left it out on purpose: every future PR that adds a gated block would have to update it, and imposing that on other contributors is a call for the maintainers rather than for this PR. The maintenance contract is written on the list, and I'd rather flag the gap than quietly widen scope.

The skip re-reads the config when it runs instead of trusting the value captured at build time, so a cause that --fix repairs mid-run downgrades to a warning rather than reporting a failure that isn't there any more. TestSkippedCheckWarnsWhenCauseRepairedMidRun pins that direction and TestSkippedCheckStillBlocksWhenCausePersists pins the other.

One deliberate call on the resource census

The first push tripped TestRepositoryLedgerMatchesCensusAndDocumentation: my new test file used t.Setenv("GC_DOLT", "skip") twice, which pushed the cmd/gc+untagged environment debt from 4318 to 4320 calls and 202 to 203 files.

Raising the two baselines in test/test-resources.toml would have made it green. I didn't, because that ledger's stated invariant is that existing debt cannot grow, and quietly bumping it is the failure mode the ratchet exists to catch. The t.Setenv turned out to be unnecessary anyway: buildDoctorChecks never reads GC_DOLT, only runDoctor does, and it passes the result down through buildDoctorChecksOpts, which both tests already set. So I dropped both calls, the ledger is untouched, and there's a comment at the call site recording why the tests don't mutate the environment. Verified by running the two tests with GC_DOLT=on and with it unset; both pass either way.

Test plan

• go test ./internal/doctor/... ./internal/citylayout/... — passing locally
• go test ./cmd/gc/... — full package, no narrow -run
• gofmt -l clean and go vet clean on all nine touched files
• go test ./internal/testpolicy/resourcecensus/ — the ledger shard, which the first push tripped
• The configDependentCheckGroups coverage check I asked for below: confirmed, no gap today. Review walked all eight cfgErr == nil gated register blocks in buildDoctorChecks and mapped them onto the seven entries, with dolt-ops-checks covering both the main-block Dolt registrations and the separately-gated postgres-auth one. One block reads like a ninth and is not: the if cfg != nil further down only selects doctorCfg, and the two registers under it are unconditional, so the worktree checks correctly need no entry. The unenforced invariant stays a future-regression risk rather than a live gap, and the tripwire for it is deferred with a stated reason.

Note for reviewers

The pack-source-checks group covers the pack import cache check and the pack-source credential check. The latter arrived with the credentialed-pack-imports work and is config-gated, so it belongs in this inventory.

---

Generated by the operator's software factory.
• City: factory-main · Agent: local-core.builder-3
• On behalf of: @benw5483