no check reports that the running gc binary is behind origin/main — merged fixes sit inert for days with no signal (4 stranded right now) (gc-0qbf5) - #152
Conversation
…(gc-0qbf5)
Nothing told an operator that merged fixes were not executing. A fix lands, the
bead closes, the PR merges -- and the running city keeps executing the older
image indefinitely, with every existing signal reading clean.
This is the missing link in a three-part chain:
origin/main --(new)--> on-disk binary --(gc start)--> supervisor
`gc start`'s DetectBinaryDrift already compares the supervisor's reported
buildID against the local binary's, catching a supervisor left on a stale
image. It is structurally blind to the failure mode here, where supervisor and
on-disk binary agree perfectly and BOTH are days behind main. Nothing in that
state looks wrong: the binary's mtime is recent enough to be plausible, the
beads are closed, the PRs are merged. The only evidence is code that silently
is not running, which is why this has now cost real time twice (gc-f1081 was
the same shape at five days, differing only in that a deleted inode held the
supervisor).
BinaryFreshnessCheck reads the running binary's stamped vcs.revision, finds the
configured rig whose object database CONTAINS that commit, and reports
`git rev-list --count <build>..origin/<default>`. Verified against this town:
build f475b68 (2026-08-17) is 4 commits behind origin/main (2026-08-20), and
the four it names are exactly the four the bead identified -- #146, #148, #147,
#149.
Three design points worth stating, since each rules out an easier alternative:
- Repo identity is COMMIT CONTAINMENT, not a name, path, or remote-URL match.
This repo is regularly built from a fork whose origin differs from the module
path, so a URL match would fail exactly here; and any name match would smuggle
a repository identity into Go. The repo that can resolve the commit is the
repo the binary was built from.
- It never fetches. Comparing against the last-fetched remote-tracking ref keeps
the check free of network I/O and side effects inside `gc doctor`. The cost is
understating drift when the checkout itself is stale, so the finding names the
ref it read and says how to get a current reading -- the reading is never
presented as live.
- Severity is advisory and CanFix is false. The remedy is a rebuild plus
`systemctl --user restart`, and that restart bounces the tmux server hosting
every agent session. That is an operator decision, not something a gate should
force. The fix_hint deliberately gives both halves as one command, because a
rebuild WITHOUT an immediate restart recreates the deleted-inode state
gc-f1081 tracked -- the two must happen together.
Every not-applicable state resolves to StatusOK rather than a warning: no
stamped revision (-buildvcs=false), git absent, no configured rig holding the
commit, or no fetched tracking ref. None of those is a stale binary, and
warning on them would produce exactly the unclearable noise this check exists to
replace.
Validation: eight tests over real git repos in t.TempDir covering at-tip,
behind, ahead-of-origin, unstamped revision, no-owning-rig, missing tracking
ref, a non-main default branch, and multi-rig selection.
Three notes from writing them, each a trap that cost a cycle:
- The helper seeds each repo's file content with its own temp path. Git commits
are content-addressed, so two repos built from identical trees, messages and
timestamps produce IDENTICAL SHAs -- which silently defeated the multi-rig
test until the trees were made distinct.
- The non-main-default-branch case exists because the linter flagged the branch
parameter as always-"main". That was a real coverage gap (the check resolves
EffectiveDefaultBranch), not a dead parameter, so the fix was a test rather
than a narrower signature.
- The test constructs NO os/exec commands of its own, reusing the package's
existing runGitForRigRootBranchTest and the production runGitCommand instead.
The repository budgets os/exec construction sites per file
(internal/testpolicy/resourcecensus), and a first draft that defined its own
two helpers pushed the ledger over baseline. Consolidating onto the existing
helpers keeps the new file at zero sites, so no census baseline is raised --
the budget is meant to be spent down, not ratcheted up.
`binary-freshness` added to cmd/gc/testdata/doctor_check_names.golden in
registration order. internal/doctor, internal/testpolicy, internal/testenv,
internal/productmetrics and `go test ./cmd/gc/ -run Doctor` all green;
go vet ./... clean.
Claude-Session: https://claude.ai/code/session_012NnWPBtpRiQFJBaYFQbYyz
|
Codex signoff (pre-open, comment-only — not an approval): VERDICT: COMMENT Scope checked: Full branch diff for cmd/gc/cmd_doctor.go, cmd/gc/testdata/doctor_check_names.golden, internal/doctor/checks_binary_freshness.go, and internal/doctor/checks_binary_freshness_test.go. Also checked surrounding doctor result semantics, git helper behavior, rig default-branch handling, and the doctor check registration/golden test path. Findings: None. No P0/P1 issues found. Verification: go test ./internal/doctor -run 'TestBinaryFreshnessCheck' -count=1 -> PASS. go test ./cmd/gc -run 'TestDoctorCheckNames|TestBuildDoctorChecks|DoctorCheckNames|Doctor.*Check' -count=1 -> PASS. go test ./cmd/gc -run 'TestDoctorCheckNamesGolden|TestBuildDoctorChecksWithBrokenConfig|TestBuildDoctorChecks' -count=1 -> PASS. go test ./internal/doctor -count=1 -> PASS. go vet ./internal/doctor ./cmd/gc -> PASS. All verification ran in a detached worktree at the reviewed commit. Not checked: full repository make targets and a live gc doctor invocation against the running city. |
…(gc-0qbf5) (#152) Nothing told an operator that merged fixes were not executing. A fix lands, the bead closes, the PR merges -- and the running city keeps executing the older image indefinitely, with every existing signal reading clean. This is the missing link in a three-part chain: origin/main --(new)--> on-disk binary --(gc start)--> supervisor `gc start`'s DetectBinaryDrift already compares the supervisor's reported buildID against the local binary's, catching a supervisor left on a stale image. It is structurally blind to the failure mode here, where supervisor and on-disk binary agree perfectly and BOTH are days behind main. Nothing in that state looks wrong: the binary's mtime is recent enough to be plausible, the beads are closed, the PRs are merged. The only evidence is code that silently is not running, which is why this has now cost real time twice (gc-f1081 was the same shape at five days, differing only in that a deleted inode held the supervisor). BinaryFreshnessCheck reads the running binary's stamped vcs.revision, finds the configured rig whose object database CONTAINS that commit, and reports `git rev-list --count <build>..origin/<default>`. Verified against this town: build f475b68 (2026-08-17) is 4 commits behind origin/main (2026-08-20), and the four it names are exactly the four the bead identified -- #146, #148, #147, #149. Three design points worth stating, since each rules out an easier alternative: - Repo identity is COMMIT CONTAINMENT, not a name, path, or remote-URL match. This repo is regularly built from a fork whose origin differs from the module path, so a URL match would fail exactly here; and any name match would smuggle a repository identity into Go. The repo that can resolve the commit is the repo the binary was built from. - It never fetches. Comparing against the last-fetched remote-tracking ref keeps the check free of network I/O and side effects inside `gc doctor`. The cost is understating drift when the checkout itself is stale, so the finding names the ref it read and says how to get a current reading -- the reading is never presented as live. - Severity is advisory and CanFix is false. The remedy is a rebuild plus `systemctl --user restart`, and that restart bounces the tmux server hosting every agent session. That is an operator decision, not something a gate should force. The fix_hint deliberately gives both halves as one command, because a rebuild WITHOUT an immediate restart recreates the deleted-inode state gc-f1081 tracked -- the two must happen together. Every not-applicable state resolves to StatusOK rather than a warning: no stamped revision (-buildvcs=false), git absent, no configured rig holding the commit, or no fetched tracking ref. None of those is a stale binary, and warning on them would produce exactly the unclearable noise this check exists to replace. Validation: eight tests over real git repos in t.TempDir covering at-tip, behind, ahead-of-origin, unstamped revision, no-owning-rig, missing tracking ref, a non-main default branch, and multi-rig selection. Three notes from writing them, each a trap that cost a cycle: - The helper seeds each repo's file content with its own temp path. Git commits are content-addressed, so two repos built from identical trees, messages and timestamps produce IDENTICAL SHAs -- which silently defeated the multi-rig test until the trees were made distinct. - The non-main-default-branch case exists because the linter flagged the branch parameter as always-"main". That was a real coverage gap (the check resolves EffectiveDefaultBranch), not a dead parameter, so the fix was a test rather than a narrower signature. - The test constructs NO os/exec commands of its own, reusing the package's existing runGitForRigRootBranchTest and the production runGitCommand instead. The repository budgets os/exec construction sites per file (internal/testpolicy/resourcecensus), and a first draft that defined its own two helpers pushed the ledger over baseline. Consolidating onto the existing helpers keeps the new file at zero sites, so no census baseline is raised -- the budget is meant to be spent down, not ratcheted up. `binary-freshness` added to cmd/gc/testdata/doctor_check_names.golden in registration order. internal/doctor, internal/testpolicy, internal/testenv, internal/productmetrics and `go test ./cmd/gc/ -run Doctor` all green; go vet ./... clean. Claude-Session: https://claude.ai/code/session_012NnWPBtpRiQFJBaYFQbYyz
…(gc-0qbf5) (#152) Nothing told an operator that merged fixes were not executing. A fix lands, the bead closes, the PR merges -- and the running city keeps executing the older image indefinitely, with every existing signal reading clean. This is the missing link in a three-part chain: origin/main --(new)--> on-disk binary --(gc start)--> supervisor `gc start`'s DetectBinaryDrift already compares the supervisor's reported buildID against the local binary's, catching a supervisor left on a stale image. It is structurally blind to the failure mode here, where supervisor and on-disk binary agree perfectly and BOTH are days behind main. Nothing in that state looks wrong: the binary's mtime is recent enough to be plausible, the beads are closed, the PRs are merged. The only evidence is code that silently is not running, which is why this has now cost real time twice (gc-f1081 was the same shape at five days, differing only in that a deleted inode held the supervisor). BinaryFreshnessCheck reads the running binary's stamped vcs.revision, finds the configured rig whose object database CONTAINS that commit, and reports `git rev-list --count <build>..origin/<default>`. Verified against this town: build f475b68 (2026-08-17) is 4 commits behind origin/main (2026-08-20), and the four it names are exactly the four the bead identified -- #146, #148, #147, #149. Three design points worth stating, since each rules out an easier alternative: - Repo identity is COMMIT CONTAINMENT, not a name, path, or remote-URL match. This repo is regularly built from a fork whose origin differs from the module path, so a URL match would fail exactly here; and any name match would smuggle a repository identity into Go. The repo that can resolve the commit is the repo the binary was built from. - It never fetches. Comparing against the last-fetched remote-tracking ref keeps the check free of network I/O and side effects inside `gc doctor`. The cost is understating drift when the checkout itself is stale, so the finding names the ref it read and says how to get a current reading -- the reading is never presented as live. - Severity is advisory and CanFix is false. The remedy is a rebuild plus `systemctl --user restart`, and that restart bounces the tmux server hosting every agent session. That is an operator decision, not something a gate should force. The fix_hint deliberately gives both halves as one command, because a rebuild WITHOUT an immediate restart recreates the deleted-inode state gc-f1081 tracked -- the two must happen together. Every not-applicable state resolves to StatusOK rather than a warning: no stamped revision (-buildvcs=false), git absent, no configured rig holding the commit, or no fetched tracking ref. None of those is a stale binary, and warning on them would produce exactly the unclearable noise this check exists to replace. Validation: eight tests over real git repos in t.TempDir covering at-tip, behind, ahead-of-origin, unstamped revision, no-owning-rig, missing tracking ref, a non-main default branch, and multi-rig selection. Three notes from writing them, each a trap that cost a cycle: - The helper seeds each repo's file content with its own temp path. Git commits are content-addressed, so two repos built from identical trees, messages and timestamps produce IDENTICAL SHAs -- which silently defeated the multi-rig test until the trees were made distinct. - The non-main-default-branch case exists because the linter flagged the branch parameter as always-"main". That was a real coverage gap (the check resolves EffectiveDefaultBranch), not a dead parameter, so the fix was a test rather than a narrower signature. - The test constructs NO os/exec commands of its own, reusing the package's existing runGitForRigRootBranchTest and the production runGitCommand instead. The repository budgets os/exec construction sites per file (internal/testpolicy/resourcecensus), and a first draft that defined its own two helpers pushed the ledger over baseline. Consolidating onto the existing helpers keeps the new file at zero sites, so no census baseline is raised -- the budget is meant to be spent down, not ratcheted up. `binary-freshness` added to cmd/gc/testdata/doctor_check_names.golden in registration order. internal/doctor, internal/testpolicy, internal/testenv, internal/productmetrics and `go test ./cmd/gc/ -run Doctor` all green; go vet ./... clean. Claude-Session: https://claude.ai/code/session_012NnWPBtpRiQFJBaYFQbYyz
Summary
Defect
Nothing rebuilds or deploys the
gcbinary when gascity merges a fix, andnothing reports that the running city is behind
origin/main. Fixes land, thebead closes, the PR merges — and the running city keeps executing older code
indefinitely, with no signal anywhere.
Live right now (2026-08-20T08:3xZ)
Running binary
/home/zook/go/bin/gcwas built 2026-08-19 04:11. Thesupervisor (started 2026-08-19T07:05:11Z) runs that image — correctly, not a
deleted inode. Four fixes merged to
origin/mainAFTER that build and are allinert in the running city:
The first one is actively costing:
agent-token-telemetryfiresseverity=blocking every patrol ("9 of 12 awake sessions have recorded no token
samples"), the deacon escalates it to the mayor each time, and the fix that
stops it is #146 — merged, not running. Two mayor↔deacon round trips have
already been spent concluding "that's fixed" about code that is not executing.
Why a doctor check, not a habit
This is the second time this pattern has cost real time. gc-f1081 was the same
shape at five days — the difference there was a deleted-inode holder, but the
observable was identical: merged fixes silently not running, and the on-disk
binary date reading as proof they were.
There is no order, no check, and no documented step that closes the loop. The
mechanical remedy is a doctor check that compares the RUNNING image's build time
against
origin/main's HEAD commit date for this repo and warns when main isahead. That turns an invisible multi-day drift into a visible finding, without
requiring anyone to remember.
Deliberately NOT proposing an auto-deploy:
systemctl --user restart gascity-supervisor.servicebounces the whole town (the tmux server hostingevery agent session lives in its cgroup) and is operator-approval-only. And a
rebuild WITHOUT an immediate restart is actively harmful — it recreates the
(deleted)-inode state that gc-f1081 tracked. Detection is the safe half; thedeploy stays a human action.
Done when
gc doctorreports a finding when the running supervisor image predates thenewest commit on this repo's
origin/main, naming both timestamps and thecount of stranded commits. Bonus: name the actual remedy in
fix_hint(rebuild +
systemctl --user restart), since the two must happen together.Implementation notes
Implemented on polecat/gc-0qbf5 (c723796).
Adds doctor check 'binary-freshness'. Independently reproduced the filed defect
before building: the running gc is built from f475b68 (2026-08-17) and is exactly
4 commits behind origin/main (2026-08-20) — the same four the bead named
(#146, #148, #147, #149).
Design notes for review:
supervisor's buildID against the on-disk binary's; it is structurally blind to
the case here, where the two agree and BOTH are behind main. The new check
closes the remaining link: origin/main -> on-disk binary -> supervisor.
path, or remote URL. This town builds from a fork (zookanalytics/gascity) whose
origin differs from the module path (gastownhall/gascity), so a URL match would
fail exactly here, and a name match would put repository identity into Go.
the ref it read and how to refresh, so it is never mistaken for a live reading.
bounces every agent session. fix_hint gives rebuild AND restart as one command
because a rebuild alone recreates the deleted-inode state of gc-f1081.
owning rig, no fetched ref) rather than warning — warning on those would
recreate the unclearable noise this check exists to replace.
Verification: 8 tests over real git repos; go vet ./... clean; internal/doctor,
internal/testpolicy, internal/testenv, internal/productmetrics green; pre-push
gate 'Running 10 fast job(s)' -> 10/10 ok, sentinel RC=0.
Gate history (disclosed): attempts 1 and 2 failed on tests unrelated to this diff,
attempt 3 passed clean. NOT bypassed — no --no-verify at any point.
the pre-existing ~1-in-3 flake filed as gc-04375. My first two samples were
branch=FAIL / merge-base=pass, which reads as my regression; six runs gave
1/3 failures on EACH side. That is the exact false attribution gc-04375 warns
about, and I appended the third-tree measurement to that bead.
that one WAS mine. Fixed by consolidating the new test onto the package's
existing runGitForRigRootBranchTest and runGitCommand helpers, taking the new
file to zero exec.Command sites, so no census baseline was raised.
loopback 'context deadline exceeded' under load average ~15-20 (partly
self-inflicted by the six shard runs above). Passes 3/3 standalone; no bead
filed for a single load-induced timeout.
CORRECTION to the note above: the final commit is 0fe3c55, not c723796.
c723796 was pushed with the resource-census fix still UNCOMMITTED. The
pre-push gate validates the WORKING TREE, not the commit, so it went green on
code that was not on the branch — the pushed commit still had 2 exec.Command
sites and would have failed the census in CI, on the very check I had just
fixed. Caught because 'git checkout --detach' printed
'M internal/doctor/checks_binary_freshness_test.go'.
Recovered by amending the fix into the single commit and force-pushing with the
lease pinned to the old SHA (--force-with-lease=refs/heads/polecat/gc-0qbf5:c72379694...),
not a bare --force, so a concurrent ref move would have refused rather than
clobbered. Sanctioned case: own polecat branch, no PR open (gh pr list empty),
bead not yet handed to refinery.
Final state verified: remote == local == 0fe3c55; the pushed commit contains
the consolidation (0 exec.Command sites); worktree clean; gate 'Running 10 fast
job(s)' -> 10/10 ok, sentinel RC=0, zero job failures.
Additional gate attempt (disclosed): one further attempt failed on
TestDoEventsWatchCityBufferedReplayAfterSeqSkipsHeadProbe — the same load
flake as before, now filed with its root cause as gc-b3g52 (the test gives
doEventsWatch a 50ms wall-clock deadline against a loopback httptest server;
the same 50ms value appears at 8 call sites in cmd_events_test.go). Host load
average was ~20-25 throughout. No --no-verify at any point.
Refinery handoff
gc-0qbf5(bug, P1)polecat/gc-0qbf5main0fe3c552; PR opened codex-green.