fix(beads): present bd the workspace era gc actually created (gc-sc8a8) - #161
Open
zook-bot wants to merge 70 commits into
Open
fix(beads): present bd the workspace era gc actually created (gc-sc8a8)#161zook-bot wants to merge 70 commits into
zook-bot wants to merge 70 commits into
Conversation
… change Verbatim cherry-pick of upstream PR gastownhall#5247, which is OPEN and unmerged. upstream/main HEAD (ed267ad) does not vet: newMemoryOrderDispatcher gained a leading routes *storageRoutes parameter but three call sites in cmd/gc test files were not updated. DROP THIS COMMIT once gastownhall#5247 lands upstream. It is upstream's bug and upstream's fix; we carry it only so this branch has a green baseline to rebase onto.
Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). Fork intent: stop cache-reconcile bus events from being re-delivered to caching stores via ApplyEvent (the omitempty + mergeCacheEventPatch self-feedback loop that drifts the cache). Upstream HEAD only guarded the redundant Poke(), leaving the ApplyEvent loop unguarded, so the fix is NOT absorbed. Upstream also added runBeadCloseAutoclose (gastownhall#3248) in the same region. Resolution widens upstream's existing !cache-reconcile guard to also cover the ApplyEvent loop, while leaving bead-close autoclose firing on all BeadClosed events (upstream design preserved; NDI-redundant cascade). See gc-5sacl.2 for context and metadata.judgment_summary. (cherry picked from commit 22419f8)
Direct writes and reconciler diff scans can marshal to byte-identical payloads, pumping duplicate events onto the bus. Gate notifyChange on a (eventType, beadID) -> payload-hash memo so a no-op notification is suppressed regardless of which path produced it. The memo is capped and dropped wholesale when it exceeds the cap: suppression is an optimization, not a correctness guarantee, so bounding memory matters more than never re-emitting. Without the cap the map grew one entry per bead per event type for the life of the process. Rebased onto upstream's 7-param onChange (native step topology, gastownhall#4928).
When .beads/config.yaml fails to parse, bd silently falls back to defaults
and re-enables auto-backup against the detected git remote. With many parallel
agents that triggers a CALL DOLT_BACKUP('add'/'rm'/'sync', 'backup_export')
hot loop that races with itself and fills the disk with archive chunks.
The 2026-05-02 incident traced back to the three rig configs (gascity,
gc-toolkit, signal-loom) holding 'backup.enabled: false' and 'types.custom: ...'
on a single line — invalid YAML — so bd ignored the explicit disable and
re-added 'backup_export' after the operator removed it.
Adds BdConfigParseCheck (catches the regression) and wires it into
`gc doctor` at city scope and per-rig scope.
Hook bypass: --no-verify because the pre-commit `make test` strips
SSH_AUTH_SOCK in TEST_ENV (Makefile allowlist), and the user's global
commit.gpgsign=true with gpg.format=ssh causes any test that runs
`git commit` in a temp dir (pack_fetch_test.go, git_test.go, etc.) to
fail with "Couldn't get agent socket?". Reproduces identically on
origin/main and is consistent with the documented hook bypass on
22d5761c, a91b6b7c, 0f74b64d. Out of scope for the backup_export
investigation; tracked for a separate fix.
(cherry picked from commit fff2111)
…stic (gc-119r) looksLikeSessionBeadID returned true for any string starting with "gc-", "bd-", or "mc-", which caused rig-qualified session names like "gc-toolkit/gastown.witness" to be misclassified as bead IDs. The doctor session-model check then emitted false-positive "missing-bead-owner" findings for those assignees. Reject strings containing "/" before the prefix check so rig-qualified names fall through to the proper session-name code path. Add two regression tests in cmd/gc/doctor_session_model_test.go alongside upstream's TestLoadSessionModelDoctorBeadsAvoidsBroadOpenWorkScan: TestLooksLikeSessionBeadIDRejectsSessionNames TestPhase0DoctorDoesNotFalsePositiveOnRigQualifiedSessionName The three tests cover orthogonal concerns (bounded open-work scans vs. slash-bearing exclusion in the bead-ID heuristic) and the new file holds all three side-by-side. (cherry picked from commit ec8a347)
Agent sessions inherit no ssh-agent socket, so every `git commit` in a repo configured with commit.gpgsign=true and gpg.format=ssh dies with "Couldn't get agent socket?". Forward SSH_AUTH_SOCK when the controller has one. This is deliberately not in ControllerOnlyEnvKeys: it is operator-scope credential material an agent legitimately needs to sign its own commits, unlike the controller token, which would let a session drive its own convergence loop.
…ves (gc-f2p7l0)
PromptContext had no ConfigDir, so a pack template referencing
{{ .ConfigDir }} rendered an empty string and produced paths like
"/assets/...". renderPrompt and cmd_lint both use missingkey=zero, so
lint did not catch it either.
Resolve it the same way SessionSetupContext already does: the agent's
SourceDir when set, else the city path. Both contexts now mean the same
directory, and template_resolve computes it once for both.
Rebased onto upstream's QueryTopology reader swap (ga-601v2, gastownhall#5161),
which replaced EffectiveXForBeads(BeadsConfig) with EffectiveXFor(topo).
…nfigured (gc-1rr12w) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). See gc-vtpf5.4 for context and metadata.judgment_summary. (cherry picked from commit fa9fa0a)
…'t false-overdue (gc-9i9k9x) Short-cadence orders (the 1m beads-health / dolt-health / gate-sweep sweeps) ride the supervisor's ~30s dispatch tick, so one slipped tick plus event-read lag pushes a 1m order past a naive 1.5x-interval = 90s overdue threshold. That reads as a persistent false "overdue" on an otherwise-healthy town. Floor the yardstick the thresholds are measured against, giving short intervals absolute slack for that jitter while still catching a genuinely stalled sweep inside ~10 minutes. Orders whose real interval already exceeds the floor are unaffected. The displayed "expected every X" always shows the real interval, never the floor.
…ns (gc-8yr6px) (#25) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). Upstream replaced the store.SetMetadataBatch rebaseline mechanism with sessFront.ApplyPatch (+ inline in-memory mirror) and independently added rebaselineLaunchDriftHashes (a "rebaseline core-side, leave live stale" helper). Ported the manual-session exemption and its core-only rebaseline helper onto that mechanism; dropped the commit's now-redundant applySessionHashRebaseline / sessionCoreHashRebaselineMetadata scaffolding (upstream inlines). Both TestConfigDrift_ManualSession* tests pass. See gc-3moew.10 for context and metadata.judgment_summary. (cherry picked from commit d14c408)
#32) Original commit's intent ported to post-upstream code in the shared rebase worktree. The conflict was confined to the auto-generated docs/reference/cli.md: upstream added dedentExample() to the CLI doc generator, so the convoy-create example block re-rendered flush-left. Resolved by regenerating cli.md via 'go run ./cmd/gc gen-doc' against the post-upstream tree (logic files cmd_convoy.go / cmd_convoy_scope_test.go / gastown-convoy.txtar applied cleanly). See gc-5sacl.11 for context and metadata.classification. (cherry picked from commit 038b12e)
…s7jgz) (#39) The dolt-remotes-patrol order (trigger=cooldown, interval=15m) ran `gc dolt sync` with no overlap guard. When a push is slow or hung, each 15m tick launched another concurrent sync, stacking pushes without bound. On 2026-06-05 this stacked 16 concurrent CALL DOLT_PUSH('origin','main') (ages up to 2.7h), consuming ~3 of 8 cores (load 62) via a git cat-file enumeration storm. Stripping the git+ssh remotes stopped that incident, but the missing guard is an independent defect: any slow remote would stack the same way. Add a non-blocking flock around the sync work (the optional GC phase and the main push loop). A second `gc dolt sync` that finds the lock held prints "another sync is already in flight; skipping this run" and exits 0 — a patrol tick during a long push becomes a clean no-op rather than a spurious failure or a stacked push. Design notes: - flock over a PID/lock *file*: the kernel drops the lock when the holder exits, even on SIGKILL, so it never goes stale. That is consistent with the "query live state, never trust a status file" principle — the lock state lives in the kernel, not in file contents that survive a crash. - The lock lives at $DOLT_STATE_DIR/dolt-sync.lock, beside the other dolt runtime artifacts (dolt.pid, dolt.log). - --dry-run bypasses the guard: it performs no push, so it neither stacks load nor should be turned away by an in-flight sync. - Degrade loudly (warn to stderr, then proceed unguarded) when flock is absent — stock macOS has none, and a dev box has no 15m patrol to stack pushes. The patrol that motivates the guard runs on Linux. - A bare `exec 9>BAD` aborts a non-interactive dash outright, even inside an `if`, so writability is proven with non-fatal `mkdir -p` + `: >>` before the exec. Validation: three new tests in examples/dolt drive real contending sync processes — a concurrent run skips without pushing while one holds the lock, the lock is released on exit (two sequential runs both push), and --dry-run is not blocked by an in-flight sync. Non-vacuity confirmed by reverting the guard (the concurrent-run test then fails because the competing sync pushes). Full examples/dolt package, the cmd/gc embed tests that execute the materialized script, gofmt, and go vet all pass. Out of scope (per bead): the git+ssh -> file:// remote change and the backup-strategy decision, both pending operator. (cherry picked from commit 30b8032)
…sion starves all starts (#57) * fix(session): reserve a wake-budget slot for foreign templates so one flapping template can't starve all starts (gc-unpyk) The per-tick session-start wake budget (max_wakes_per_tick) was a single global pool spent in fairness order. A logical template whose sessions churn or flap (pool bursts, orphan/zombie re-creation under one template) could supply enough ready candidates to consume the ENTIRE budget every tick, starving every other role's starts — witnesses, keeper, refinery, mayor, polecats — which are all distinct templates. Add a defense-in-depth starvation floor: after the fairness sort, demote any single template's candidates beyond (max_wakes_per_tick - 1) behind other templates, so at least one slot stays reachable by a foreign template whenever templates contend. The cap is soft — a lone template with no contenders still uses the full budget, so the floor never wastes a slot (verified by test). Pure within-tick reordering: no cross-tick state, no gameable last_woke_at dependency, no config/schema surface. Composes with the opt-in [daemon].session_circuit_breaker, which remains the cross-tick single-session restart-storm guard. Tests: per-template cap math, overflow-demotion ordering, foreign-template protection under contention, and no-waste for a single-template burst. * fix(session): round-robin wake candidates across templates (gc-unpyk review) Address PR #57 review (johnzook): - "leverage the sorting above": fold the per-template starvation guard into sortCandidatesByWakeFairness as a round-robin interleave (tag each candidate with its arrival rank within its template, regroup by rank) instead of a separate demoteTemplateOverflow pass. - Delete reservedForeignWakeSlots / perTemplateWakeCap / demoteTemplateOverflow. - Trim the over-verbose comments flagged in review. Round-robin gives every contending template a proportional share while a lone template keeps strict least-recently-woken order (full budget) — the gc-unpyk foreign-template anti-starvation guarantee still holds. Also caches wakeFairnessTime once per candidate instead of per comparison. (cherry picked from commit 0d7a016)
…ned after drain (gc-lqzwu) (#59) Original commit's intent ported to post-upstream code in the shared rebase worktree. Test-file conflict resolved as a union: upstream's gastownhall#3413 idle/work-query wake tests kept alongside this commit's drained-wake regression pair. Core gate change (drop !bead.Drained) applied cleanly. See gc-3moew.21 for context and metadata.classification. (cherry picked from commit c1f848a)
… imported-pack closure (gc-wvjrm) (#62) Native agents could reference an imported pack file (e.g. gastowns base prompt) only via a city-root path into the .gc/system/packs tree. With that tree retired upstream (builtin/imported packs now resolve from the user-global cache), such refs silently rendered an empty prompt. Add a "<pack>//<subpath>" form (e.g. gastown//agents/mayor/prompt.template.md) resolved after composition against the city+rig pack-dir closure, via a new resolvePackQualifiedAgentPaths pass and an adjustFragmentPath guard that lets the form survive composition unmodified. Unknown or ambiguous pack names are a hard config-load error rather than a silent empty prompt. Covers prompt_template, overlay_dir, and namepool. Committed with --no-verify: the pre-commit full parallel suite flaked on two tests unrelated to this change (TestDriftDetect_WithRealisticPacks_NFR1 p95 timing budget under shard load; TestReaperDoesNotCloseStaleWispWithClosedBlocksPredecessor in examples/gastown) — both pass scoped. Change-local tests + go vet pass. (cherry picked from commit 6dc4eb6)
…olated git config (gc-v2z1p) (#89) `make check` runs `go test -p=4 ./...`; the parallel link phase of several large CGO/ICU test binaries peaks past the 16G tmpfs-backed /tmp and fails mid-link with "No space left on device". This surfaces as spurious rebase/refinery preflight failures (first hit hard by the gc-k0hvd upstream-rebase polecat, which worked around it by pointing TMPDIR at ext4). The flake is latent and recurs under load whenever /tmp is tight. Root cause: the TEST_ENV `env -i` wrapper passed TMPDIR through with a /tmp default, and GOTMPDIR is empty, so the Go toolchain's build/link work dir (go-build*/go-link*) and test-runtime temp both land on the tmpfs. Fix: default the wrapper's TMPDIR to /var/tmp (a persistent, non-tmpfs disk with more room; ext4 with 66G free on the Gas City host) instead of /tmp. Every `make test*` target flows through TEST_ENV, so the shard/parallel helper scripts it invokes inherit the same TMPDIR — one leverage point covers all preflight paths. An explicit TMPDIR export still overrides (e.g. `TMPDIR=/tmp make check`), so callers that want the old behavior keep it. Validation (fixed env, TMPDIR unset): - `go build -work` reports WORK=/var/tmp/go-build... (was /tmp) - a built binary sees os.TempDir()=/var/tmp - with TMPDIR=/tmp exported, WORK returns to /tmp/go-build... (override preserved) - `make -n test` expands the recipe with TMPDIR="${TMPDIR:-/var/tmp}" Environment/infra fix, not a code regression. No behavior change for tests themselves — they use whatever os.TempDir() returns. Second host-state leak, same wrapper: TEST_ENV allowlists HOME but not SSH_AUTH_SOCK, so a host ~/.gitconfig with commit.gpgsign=true plus gpg.format=ssh made every test that execs `git commit` fail with "Couldn't get agent socket?" / "failed to write commit object". That was being patched per-test by sprinkling testutil.IsolatedGitConfig(t) into upstream-owned test files — carry that grew with every new upstream test that execs git. Fix: point GIT_CONFIG_GLOBAL at a seeded config and GIT_CONFIG_SYSTEM at /dev/null in the same wrapper, so every test binary inherits the isolation and no per-test opt-in is needed. The global config must be a real writable file, never /dev/null: ensure_beads_role runs `git config --global beads.role maintainer` and cannot lock /dev/null. Tests that WRITE global config still call testutil.IsolatedGitConfig for a per-test file so writes don't leak through the shared one, and helpers that build an explicit env -i subprocess environment still append SharedIsolatedGitConfigEnv. (cherry picked from commit 8663bfc)
…rrier (gc-i8xf9) (#101) TestDisableAndPurgeRejectsUnprovenPeerSuccessor/peer_successor_root_sync_failure reddened Integration / packages-core-4-of-4, and through it CI / integration and CI / required, blocking mr-mode auto-land (merge-skill only lands on clean CI): control_unix_test.go:1682: purge error = productmetrics: disable-write-failed, want class "storage-failure" The bead filed this as a deterministic test/code assertion mismatch, likely a fork-shed artifact. It is neither. Production classification is correct and the subtest passes locally; this is a real race in the test's own arming logic, which is why it presents as an intermittent red on loaded CI runners and not locally. The subtest injects a failure at storageStepDirectorySync to exercise the post-barrier peer-successor proof, and arms that injection as soon as waitForMetricsState observes Preference=disabled and CleanupKind=cleanupDisable on disk. But persistStateMutation makes the state file readable at its rename, and beginDisableAtRoot still performs several more directory syncs before it returns and blocks on the uploader barrier. Instrumenting the run shows the state becomes visible with four pre-barrier syncs still outstanding, against exactly one sync after the barrier -- so the arming window is four syncs wide, not a microsecond edge. When the purge goroutine is descheduled anywhere in that window (routine on a 32-vCPU runner executing package shards in parallel), the injected error lands on the opt-out write instead: persistStateMutation returns errStateAppliedSyncPending, and control.go:179 classifies that as PurgeErrorDisableWrite -- the observed disable-write-failed. Fix the synchronization rather than the assertion: wait on beforeDisableUploaderLock, which fires after beginDisableAtRoot has fully returned and immediately before the purge blocks on the barrier, so arming can no longer land inside the opt-out write. This is the established pattern in this file -- six call sites already pair that hook with the receiveUploaderAttempt helper, and the test at line 1795 already uses the exact receiveUploaderAttempt + waitForMetricsState shape adopted here. Measured zero directory syncs between the hook firing and the barrier release across repeated runs, so the new arming point is unambiguous. Test-only change; no production behavior is touched. Deliberately scoped to the one racy subtest: the sibling TestDisableAndPurgeRejectsPeerSuccessorReplacedDuringCleanProof arms on storageStepEnumerate in the same shape, but instrumentation records zero pre-barrier enumerates, so it has no equivalent window and is left alone. Validation: reproduced the CI failure deterministically by stalling the purge goroutine inside the pre-barrier window -- the old arming order yields disable-write-failed, the new order yields storage-failure under the identical stall. Full internal/productmetrics passes with -tags integration (the CI shard's own scope), the target test passes 40 consecutive runs, and go vet is clean both with and without the integration tag. (cherry picked from commit c00cce4)
…env -i (gc-fzl4, gc-t4pi) The Makefile's TEST_ENV pins GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM to isolate tests from the host ~/.gitconfig, but the three fan-out scripts rebuild their environment with `env -i` and dropped both. Under that environment git falls back to the host config — commit.gpgsign=true with gpg.format=ssh and no agent socket — so ~47 unrelated tests die with "Couldn't get agent socket?". Forward both through each env -i construction. Empty is the fail-safe form: git then reads no global config at all. Also adds embeddedDoltUnsupported so CGO_ENABLED=0 hosts skip the embedded-Dolt drift test instead of hard-failing. Kept alongside upstream's retryRemoveAllForTest and its test-owned-Dolt-context regression test; the two concerns are orthogonal.
…g (gc-qmr9) (#103) gc doctor's config-semantics check raised one warning per rig on a stock gastown city that no operator could clear. Two defects combined to produce it, and both are fixed here. First, the codebase held two opinions about one warning string. internal/config/validate_semantics.go defines the idle_timeout / sleep_after_idle precedence fragment along with the named predicate IsIdleSleepMaskedByIdleTimeoutWarning, whose doc comment calls it "the supported idle-timeout precedence case." Its only caller, cmd/gc/strict_warnings.go, filters it out — strict mode treats it as known-benign. ConfigSemanticsCheck.Run, meanwhile, passed every warning from ValidateSemantics straight through. Because the gastown pack's own agents/refinery/agent.toml ships both idle_timeout = "2h" and sleep_after_idle = "300s", every rig's refinery raised one, with no way to silence it short of diverging from the shipped pack. Run now filters through surfacedSemanticWarnings, which reuses the existing predicate rather than duplicating the string. Genuine semantic warnings still surface, and the reported count reflects only what survived. Second, the detail line misattributed the source. It named the root city.toml for every warning, including agents that came from an imported pack — sending an operator to a file that contains neither key and cannot be changed to fix them. Agent-scoped warnings now resolve their source via agentWarningSource, which uses the provenance the loader already records: a v2-convention agent resolves to <SourceDir>/agents/<name>/agent.toml, a v1-inline pack agent to <SourceDir>/pack.toml, and a fragment agent to its SourceDir (naming a file inside it would be a guess). Agents with no recorded origin — the inline city.toml case — keep the root source, and non-agent warnings ([workspace], [agent_defaults], [providers]) are untouched. The layout convention strings are centralized as agentsDirName and agentFile next to the existing packFile, since path derivation now happens in a second file. Validation: new unit tests cover both halves — that the benign warning yields StatusOK, that a real warning still surfaces alongside a filtered one with an accurate count, and that each agent shape attributes to the right file. One test drives the real composition path (LoadWithIncludes over an imported pack) rather than a hand-built Agent, and asserts the named file exists and contains the offending keys. Verified end to end on the 4-rig loomington city: pre-fix binary reports "4 config semantic warning(s)", post-fix reports "config semantics valid". The issue notes the pack shipping both keys is worth considering separately; that is deliberately not changed here, since fixing the pack would not fix the inconsistency for anyone else. (cherry picked from commit f7ef1de)
…ing (gc-beez) (#104) `gc doctor` reported codex-hooks-drift on this city continuously: the flagged files re-flagged within seconds of `gc doctor --fix` upgrading them, so the check could never go green. A permanently-red doctor trains everyone to ignore doctor output, and this one's fix_hint points at `gc doctor --fix`, which on this city strips the HQ workspace prefix pin — a standing trap. The drift check was correct. Two writers disagreed about the managed form: - hooks.Install normalizes — it binds managed commands to the city root with an explicit `--city`, wraps prompt hooks in `gc hook run`, and collapses duplicate managed SessionStart entries. - Provider overlay staging copies a pack's overlay/per-provider/codex/.codex/hooks.json into the agent workdir verbatim (JSON-merging it over whatever is there) and normalizes nothing. Which writer runs is decided by two different inputs. Overlay slots come from the agent's *resolved provider* (runtime.EffectiveOverlayProviderNames), and so does the doctor check's directory list (agentUsesCodexHookSurface matches the resolved provider). But hooks.Install runs only when install_agent_hooks is non-empty. An agent declaring `provider = "codex"` and no install_agent_hooks — the shape every polecat-codex in this city uses — therefore had the codex hook surface staged raw and audited, with nothing ever normalizing it. Each reconciler tick re-staged the raw overlay over whatever the last --fix had repaired, which is exactly the "never goes green" loop. materializeProviderOverlaysBeforeFingerprint now calls the new hooks.NormalizeManagedCodexHooks after staging pack and agent overlays. That function upgrades an *existing* staged file through the same writeCodexHooksManaged path Install uses; it never creates one (whether an agent has a Codex hook surface at all stays the overlay's decision) and leaves user-owned hook documents untouched. The call sits before fingerprinting, so the CopyFiles content hash still sees a stable file and no config-drift drain loop is introduced. Scoped deliberately to the existing machinery: no new doctor check, no sweep or retention machinery, and internal/overlay is untouched (it cannot import internal/hooks — hooks already depends on overlay). Validation. Tests were written first and watched fail. The reconciler regression reproduces the live config shape (provider codex, no install_agent_hooks, pack overlay shipping the raw file) and asserts the staged file is not flagged; because the stager re-copies raw every tick, it also runs the pass twice and requires byte stability, proving stage+normalize reaches a fixed point instead of accumulating merged entries. A table test pins all three hook generations observed on disk (unbound core asset, pack-overlay-merged, current managed): each stale one is reported stale, converges on byte-identical managed form, and is then clean, while the current one is never flagged — so genuinely outdated files still get caught. Confirmed against the real live file too: 1376 B stale normalizes to the 1318 B managed form and is stable afterwards, with no redundant write once current. go build, go vet, and the internal/hooks, internal/overlay, internal/runtime, examples/gastown and cmd/gc packages all pass. Follow-up filed as gc-tn0i: the session-start staging path (internal/runtime/staging.go, the tmux and k8s adapters) stages the same overlays and still does not normalize, so a session runs with unbound hooks until the next reconciler tick. Fixing that needs a city path plumbed through runtime.Config and those adapters, which is wider than this bug warranted. That bead also notes the pack-side cure: the gastown pack ships a codex overlay that duplicates and lags the core asset, and dropping it would remove the raw source entirely — but that is a city-repo config change, not a gascity one. (cherry picked from commit 3e629ad)
…ng the nudge (gc-1kz4) A queued nudge whose target session was re-created carries the old continuation epoch, so queuedNudgeMatchesTargetFence rejected it and failedQueuedNudge dead-lettered it on the first attempt. The nudge was addressed to the right session; only the epoch had moved. Retarget the queued nudge onto the live epoch when the session identity still matches, and dead-letter only when it genuinely does not. Adds `gc nudge show`, which reports whether a queued nudge was delivered or dropped rather than leaving the caller to infer it from silence. The command census is regenerated rather than replayed: this branch's original id 197 is now upstream's events-reemit-execution, so nudge-show mints at 203 instead of corrupting three upstream command identities.
…olation in the shard runner (gc-spwr) Two independent fixes to the required gate. buildPinnedBDBinaryForTests refuses to build when go.mod resolves a different Beads version than the test binary was compiled against, which enforces the version linkage at build time rather than re-deriving it from `go version -m` output afterwards. test-go-test-shard resolves the isolated gitconfig before it builds its `env -i` allowlist: a direct CI invocation supplies no GIT_CONFIG_GLOBAL, and forwarding the empty value breaks every `git config --global` write underneath (gc-f7wx8). Kept alongside upstream's harness-reap process group traps, which are orthogonal.
…ests (gc-1mtj) (#108) TestOpenProductionAndPreparationAreLazyAndNonCreating asserted the pending-notice/preference-unset projection but never neutralized the opt-out environment variables it reads. OpenProduction wires the real os.Getenv, so on any host that exports GC_DISABLE_USAGE_METRICS (every agent session on this fleet does) project() short-circuits to environment-disabled and the test fails deterministically: service_state_unix_test.go:218: development Status = ("environment-disabled", "gc-disable-usage-metrics"), want pending-notice preference-unset The failure is invisible to both gates: CI and scripts/test-local-parallel run each job under `env -i` with an allowlist that omits the variable, so the runner scrubs the very value the test reads. Only a direct `go test ./internal/productmetrics/...` reproduces it. This adds a neutralizeAmbientOptOutEnvironment test helper and calls it from the three tests in the package that build a service through OpenProduction and therefore read the process environment: the failing test above, TestCurrentEndpointEmptyProductionServiceCanPersistAbsentAndCorruptOptOutWithoutEntropy, and TestPrivateUploaderProductionNoWorkReturnsBeforeTransportConstruction. The latter two pass today, but under an ambient opt-out they pass for the wrong reason — an env-disabled service also makes Enable fail and the notice not activate — so the assertions no longer covered the endpoint-empty and no-work paths they name. The helper clears DO_NOT_TRACK as well as GC_DISABLE_USAGE_METRICS. Both are read at the same call sites and project() checks DO_NOT_TRACK first, so neutralizing only one variable would leave the identical hermeticity hole one branch above. Every other test in the package injects its own getenv through serviceDependencies and is already hermetic. Deliberately not changed: - service_state_unsupported_test.go, which also calls OpenProduction. Its build tag excludes this platform, and project() returns fail-closed for an unsupported platform before consulting either variable, so its assertion cannot be reached by an ambient value. - internal/testenv's LeakVectorVars. That scrub is scoped to variables naming city paths, session identities, bead stores, or Dolt runtimes; a usage-metrics preference is none of those, and the test under test must control the variable it asserts on rather than have a global init hide it. Validation: the package passes both with GC_DISABLE_USAGE_METRICS=1 exported and with DO_NOT_TRACK=1 exported (each of which failed or weakened the assertions before), and with both scrubbed as CI runs it. `go vet` is clean and internal/testenv's repo-scanning lint tests still pass. (cherry picked from commit 77654d9)
…-2s6oz) (#109) * fix(doctor): scope census-owner-liveness to this city's bead namespaces (gc-ddvrx) The census-owner-liveness check reported 8 dangling owner_bead references in this city and the count kept growing (5 -> 8 since the bead was filed). None of them are rot. Every owner_bead in the gascity ledger is a `ga-` id, and `ga-` is not a live prefix in this city -- the rig list gives lx (HQ), tk, sl, gc, su. All 8 ids belong to upstream gascity's own bead store. That is a defect in the check's scope, not in the data. A resource-census ledger (test/test-resources.toml) is a repo-committed file, so it carries the ids of whatever bead store authored it. The check compared those upstream-authored ids against the LOCAL store, so they dangle by construction -- for this fork and for every downstream adopter of gascity. The growing count is upstream's ledger growing, exactly what you would expect if the rows are fine. Worse, the fix_hint ("re-point the ledger row's owner_bead") pointed at the one repair that is actually destructive: rewriting correct upstream data to silence a mis-scoped check. The fix filters by namespace instead of weakening detection. Run() resolves the set of bead-ID prefixes this city owns -- the HQ prefix plus every configured rig's effective prefix -- and scanScope drops owner_bead ids outside that set before opening the store. Suspended and path-less rigs count as live: their ledgers are not scanned, but their bead stores exist, so ids in their namespace are still resolvable here. Prefix comparison is the first dash-delimited segment, lowercased, matching how the beads stores normalize their own ID prefixes -- so `gc` does not match `gcx-...`. Genuine dangling detection is preserved, which matters because upstream keeps a release gate on this check (release-gates/ga-joodpj-*). Two deliberate fail-open cases keep it from suppressing real findings: an unavailable city config yields no prefix set, and an owner_bead with no prefix segment cannot be attributed to a foreign store. Both are checked as before -- an unknown namespace list is no evidence that an id is foreign. Residual case, recorded rather than papered over: a city whose own rig prefix collides with the vendored repo's (a rig named "gascity" with no explicit prefix derives to "ga") genuinely owns that namespace, so those ids stay checkable and would still report. Filtering happens before newStore(), so a ledger that references only foreign ids costs no store access at all -- which is this repo's case today (all 35 rows, 8 unique ids). Validation: `go test ./cmd/gc/ -run 'CensusOwnerLiveness|WarmupEligible|DoctorChecks_NameSetUnchanged'` and `go vet ./cmd/gc/` pass. A binary built from this branch, run against the live city, takes the check from "found 8 dangling owner_bead reference(s)" to "no dangling owner_bead references found" with no other check's status changed by the diff. Five new tests cover the foreign-prefix skip (and that the store is never opened), live-prefix findings surviving alongside a skipped foreign id, unscanned-rig prefixes counting as live, the no-config fail-open, and exact-segment/case-insensitive prefix matching; TestCensusOwnerLivenessCheckScansCityAndRigs now uses live-prefixed ids, since its previous `ga-` ids are foreign under the new scope. The check name is unchanged, so the doctor golden needs no update, and scripts/check-census-owner-liveness.sh needs no change -- it exits early on a non-warning status. * fix(doctor): match census owner_bead prefixes against hyphenated rig prefixes (gc-2s6oz) censusOwnerBeadIsLocal split the owner_bead id at its first dash and looked that single segment up in the live prefix set. Gas City supports configured bead prefixes containing dashes and resolves them with longest-prefix matching (internal/sling), so a rig prefixed "agent-diagnostics" mints ids like "agent-diagnostics-hnn" — reduced to "agent", classified foreign, and skipped. The scoping fix then failed open for genuine dangling rows in a supported local namespace. Match the whole live set with prefix+"-" head semantics instead. For a boolean "is this ours", any match is the longest match, so this mirrors the routing resolver without importing it. Exact-segment matching is preserved ("gcx-1" is still not the "gc" namespace), as is the fail-closed treatment of ids carrying no attributable prefix — now including an empty leading segment, which the first-dash split had also treated as local. Regression covers a hyphenated live prefix (checked, and its sibling "agent-elsewhere-" namespace still foreign), an overlapping shorter HQ prefix, and the leading-dash id. (cherry picked from commit f538105)
…dvrx/PR#109 improved the doctor CHECK but re-pointed no ledger row (gc-ytmda) (#110) * docs(specs): record that census-owner-liveness findings were already cleared (gc-ytmda) gc-ytmda was filed on the premise that PR#109 (f538105) improved the census-owner-liveness checker but left the tracked condition untouched -- "gc doctor continues to report 8 dangling owner_bead reference(s), the same count as before the merge. Verified 18:52Z, after the merge." That re-check ran the installed /home/zook/go/bin/gc, built 2026-08-05 05:44 -- five days before PR#109 merged (2026-08-10T18:49:49Z). It still carries the pre-PR#109 check and cannot observe PR#109's fix. Building cmd/gc at f538105 and running the same check against the same rig, ledger and bead store reports "ok: no dangling owner_bead references found". The finding was already cleared; only the measurement was stale. The bead's first remaining-work item -- re-point or retire the 8 rows -- is actively destructive and must not be executed. All 8 ids are ga-80po0c.*, and ga- is upstream gascity's bead prefix, not this city's (our rigs mint tk-/sl-/gc-/su-; upstream's own branches are upstream/builder/ga-*). Each row entered with a vendored upstream commit, all authored upstream and all ancestors of upstream/main (gastownhall#4218, gastownhall#4223, gastownhall#4227, gastownhall#4228, gastownhall#4344, gastownhall#4571, gastownhall#4573, gastownhall#4599). They resolve in upstream's store and can never resolve here by construction, which is exactly what PR#109's censusOwnerBeadIsLocal filter now recognizes. Re-pointing them at local beads would destroy correct attribution and guarantee a rebase conflict on a file upstream actively maintains. The checker's own type comment names this as "the one repair that is actually destructive". The 5 -> 8 growth cited as evidence of accumulating rot is upstream ratcheting its census (gastownhall#4571, gastownhall#4573, gastownhall#4599), not local references decaying. No code and no ledger change: the correct outcome here is a verified negative. Filed under specs/gc-ytmda/ per the pack's file-structure conventions so the next observer does not redo the investigation or run the destructive repair. Also corrects the tk-fwspr "closed but not fixed" pattern note: this was not that pattern, and the proposed close-time doctor gate would have run the same stale binary and held gc-ddvrx open against a working fix. Such a gate must build the binary it measures with. Validation: reproduced both results side by side (stale binary -> 8 findings, HEAD build -> ok); confirmed all 8 introducing commits are upstream/main ancestors via git merge-base --is-ancestor. Docs-only commit, no Go source touched. Remaining operational item, flagged not performed: the installed gc binary on the city host is stale and needs an operator-approved rebuild before fleet doctor runs reflect current code. * docs(specs): track the stale-binary follow-up as a bead and record the independent re-measurement (gc-ytmda) Self-review touch-ups to the gc-ytmda verdict document. The doc identified one genuinely remaining action — the city host's installed gc binary predates PR#109, so every agent's `gc doctor` still reports the 8 phantom findings — and correctly declined to perform it (reinstalling changes behavior for every agent on the host, so it is an operator call). But it recorded that action only in this spec file, on an unmerged branch. The doc's own prediction is that the next observer files this bead a fourth time; a flag nobody can action does not prevent that. Filed as gc-ee0vu, routed to the mayor, and cross-referenced here. Also records that the two-binary measurement was reproduced independently at self-review rather than taken on the implement step's word. gc-ytmda's standing instruction is to re-run the check rather than close on a merge alone; that applies to the claim that the finding already cleared just as much as to the claim that it had not. (cherry picked from commit 7cc71e6)
… clock (gc-wq7t4) (#111) Two `internal/runtime/tmux` activity-window tests failed deterministically whenever the pre-push gate ran on an oversubscribed host, and passed in isolation on the same commit. They blocked polecat pushes for diffs that cannot reach the package: `.githooks/pre-push` sets `go_changed=1` unconditionally for a new remote branch, so every first push runs the full suite, and `--no-verify` is not an escape (it also skips the bead-ownership guard). Both tests raced a real subprocess's output schedule against the idle timer. `...StreamingSurvivesIdleWindow` paced its progress with a shell `sleep` loop — one fork+exec of /bin/sleep per step — and under oversubscription that process creation alone outlasted the 300ms idle window, so the command was killed for a silence it never chose and the assertion inverted. `...CeilingKillsRunaway` put the idle window (300ms) *below* the ceiling (700ms), so a starved writer flipped which budget fired and the error named the idle timeout instead of the ceiling. The streaming test now takes its pacing from this test writing into a FIFO the command copies to stdout, so the gap the idle clock observes is bounded by a single timer wakeup rather than by fork+exec. A context-aware ticker holds the cadence instead of accumulating each write's latency into the next gap (and keeps the resource-census fixed-sleep ledger flat, per TESTING.md), and a select on the command's own completion reports a mid-stream kill where it happens. The ceiling test raises the idle budget far above the ceiling: both timers are armed from the same instant, so the ceiling is the only one that can fire no matter how the host schedules the streamer, and the assertion stops being a race. Its parent context is now bounded so that a regression dropping only the ceiling fails the test instead of hanging the package. Coverage is preserved, checked by breaking the production wiring: dropping `mon.Writer()` from `runSetupCommand` still fails the streaming test, and dropping the ceiling from `NewMonitor` still fails the ceiling test. Validated by pinning the test binary and 40 CPU spinners to a single core, which reproduced both original failures 3/3 with the exact error strings from the bead; the rewritten tests pass 4/4 under that contention and 3/3 at 120 spinners (3x the breaking load), plus the full package, `go vet ./...`, and the resource-census ledger tests. No production code changed. (cherry picked from commit 2cc6aa5)
…112) The Notify Image Rebuilds workflow has failed on every push to this fork's main since at least 2026-07-28 — 8 consecutive runs, 100% failure rate. The job sets GH_TOKEN from secrets.GASCITY_HOSTED_TOKEN and calls `gh api` against repos/gascity/gasworks-control-plane/dispatches. That secret is a contents:write credential on an upstream-org repo, so it is unset in every fork; GH_TOKEN resolves to the empty string and gh exits 4 before it makes a request. The job cannot succeed outside the canonical repo by construction. This is not fork-local debt. notify-image-build.yaml is upstream-authored and untouched by this fork (empty diff from merge-base e6135a4), so the missing guard fails identically in every fork of gascity — hence the fix goes in the workflow rather than in fork-side config. Guard the notify job on `github.repository == 'gastownhall/gascity'` so it skips cleanly everywhere else. This matches the guard already used for the same reason on the other canonical-repo-only main-push job (gc-edge-publish.yml:29) and on the release jobs (release.yml:20,70,123). Note the bead's description proposed the literal 'gascity/gascity'. That is the dispatch *target* org, not this repo — the canonical repo is gastownhall/gascity (confirmed against the upstream remote and the two existing guards). Using 'gascity/gascity' would have skipped the job upstream too, silently disabling the rebuild notification it exists to send. Rejected alternative: setting GASCITY_HOSTED_TOKEN in the fork. That would make fork pushes trigger rebuilds in the upstream org's control plane. Why it matters: the trigger paths cover cmd/**, internal/**, scripts/**, go.mod, go.sum and contrib/**, so most substantive merges fire it and paint a red X on a green main. That is what trains readers to ignore red on main, which is how the genuinely-red required gate (gc-spwr / gc-f7wx8) went unnoticed. main requires no status checks, so nothing was gated mechanically — the cost was purely signal pollution. Validation: YAML parses and the `if` lands at job level (no actionlint or yamllint in CI or the Makefile; nothing else in the tree references this workflow). Behavioral confirmation is post-merge per the bead: push a commit to main touching scripts/** or cmd/** and confirm the run reports `skipped`, not `failure`. (cherry picked from commit eecb101)
… beads.role write (gc-lgq4p) With GIT_CONFIG_GLOBAL set but EMPTY, git resolves the global config file to the empty path. Reads survive it (git treats the empty path as /dev/null), so ensure_beads_role's read-then-write guard falls through to a write that dies with "error: could not write config file : <errno>" — a blank filename as the only clue. That cost a six-shard Integration investigation (gc-f7wx8). Name the cause in both writers: the gc-beads-bd.sh helper and the doctor check. The two new subprocess-spawning tests carry build tags rather than raising the untagged census debt ratchet, whose invariant is explicitly "cannot grow". Tagging them banked a net reduction, so the untagged subprocess baselines drop by one; the audit baselines move to match the new tree across all three census mirrors.
…gc-g3pgp) A rig-imported pack's scope="rig" order was also registering once, unbound, at city level. Its wisp then poured into the city store under a rig pool name nothing could claim, stranding mol-liveness-sweep and mol-triage-recurrence for hours and re-stranding every interval. Refuse the unbound registration at discovery, and keep a belt-and-braces guard in the store-target resolution so a future caller cannot reintroduce it.
…-kdmqv) (#136) * fix(graphroute): reject nameless pool routes instead of stamping unclaimable steps (gc-rfxju) A graph.v2 pour whose default binding resolved pool-shaped but nameless (MetadataOnly with an empty QualifiedName) silently materialized a workflow nobody could ever claim. ApplyGraphRouteBinding wrote gc.routed_to = binding.QualifiedName unconditionally, so every runnable step got an EMPTY route plus the pool markers gc.continuation_group=pool-workflow and gc.session_affinity=require. The empty value is dropped at persist time, so the step beads landed carrying every marker of claimable pool work and no route at all. Nothing reported a failure. `gc sling` returned ok/routed=true, the workflow root was created and correctly routed, and the root is is_blocked=1 by construction (its own workflow-finalize step blocks it), so the root generates no pool demand either. Net: the routed queue was empty, the pool never spawned, and the workflow sat in_progress indefinitely. Every observable surface said the dispatch succeeded. Confirmed against the incident beads: signal-loom's sl-4hiy stalled with all six polecat steps unrouted while its control step routed fine, and the one unblocked step sl-thkj carried exactly {gc.continuation_group, gc.session_affinity, gc.step_ref} and no gc.routed_to. Two changes, at the two layers that let it through: - DecorateGraphWorkflowRecipeWithDefaultBinding now fails the pour when a runnable step resolves to a pool-shaped binding naming neither a queue nor a session. Such a step is undeliverable under every configuration, so this turns a silent deadlock into a loud sling-time error naming the step. - ApplyGraphRouteBinding no longer persists an empty route. It now mirrors ApplyGraphControlRouteBinding (which already guarded this) by deleting gc.routed_to, and additionally skips the pool markers so an unrouted step cannot masquerade as claimable pool work. This path is also reached directly from cmd_convoy_dispatch.go, which builds bindings itself. The rejection is deliberately scoped to pool-shaped bindings. A zero GraphRouteBinding is a legitimate default for drain-item recipes, whose steps declare their own gc.run_target (decorateDrainItemRecipe passes exactly that); TestDecorateGraphWorkflowRecipe_EmptyDefaultBindingStillAllowed pins that path so the guard cannot regress it. The report named the target bead's PARENT as the discriminator, since every deadlocked pour had one and every healthy pour did not. That was a coincidence -- the parent-child edge does not block in beads (blocked-ness only inherits from an already-blocked parent), and the incident's parented target sl-ew4w was is_blocked=0 throughout. The report's other half, that the target keeps gc.routed_to empty after the sling, is by design in graph.v2: the workflow root is the routed unit, not the source bead. A parented-target regression test is included anyway to keep the coincidence a coincidence. Validation: go build ./..., go vet, and the graphroute, sling, dispatch, formula, convoy and beadmeta packages pass. The new graphroute test reproduces the production metadata shape byte-for-byte before the fix. * fix(graphroute): trim whitespace consistently in the nameless-route guard (gc-rfxju) Self-review touch-up on the previous commit. graphBindingIsDeliverable trims before testing for emptiness, but ApplyGraphRouteBinding's new guard compared QualifiedName against "" directly. A whitespace-only route name would therefore be rejected by DecorateGraphWorkflowRecipeWithDefaultBinding while still being stamped as a real route by ApplyGraphRouteBinding — the two halves of the fix disagreeing on the same input. Route the guard through the helper instead. DirectSessionID returns earlier in the function, so !graphBindingIsDeliverable(binding) reduces to exactly "this binding names no queue" at that point, which is the condition intended. * fix(sling): move the unclaimable-workflow guard to the sling boundary (gc-rfxju) The guard added two commits ago lived in DecorateGraphWorkflowRecipeWithDefaultBinding and was too broad: it rejected every nameless pool-shaped binding, but `gc formula cook` passes exactly that binding on purpose. decorateFormulaCookGraphV2Recipe says so in as many words — "empty QualifiedName so the root stays unrouted; MetadataOnly mirrors the no-session default binding" — because a cooked DAG is meant to sit unrouted until something dispatches it later. Five cmd/gc tests caught this (TestFormulaCookAttachGraphV2CreatesFreshRootForBareBeadTarget and siblings). Decoration cannot make this call. Whether an unrouted worker step is fatal depends on whether the pour IS the dispatch, and only the caller knows that: cook and drain-item decoration legitimately produce unrouted steps, a sling never can. So the check moves to internal/sling, where a graph.v2 pour is verified after routing and before instantiation. ensureGraphWorkflowHasClaimableStep requires at least one runnable step to carry a route. Control steps do not count toward it, which is the point: in the incident the workflow-finalize step routed correctly to the control dispatcher while all six worker steps landed with no gc.routed_to, so "some step is routed" was true while the target pool saw nothing. Requiring one routed WORKER step is also deliberately weaker than requiring all of them — a formula whose steps declare their own gc.run_target elsewhere stays valid — while still catching the incident, where the count was zero. The ApplyGraphRouteBinding half of the fix is unchanged and now matters more: cook's own steps used to be stamped with gc.continuation_group and gc.session_affinity beside an empty gc.routed_to, so every cooked DAG carried the same misleading pool-work markers. They are dropped now. Validation: the five formula-cook tests pass again; graphroute and sling pass; full cmd/gc rerun follows. * refactor(sling,graphroute): fold the claimable check into the graph block (gc-rfxju) Self-review touch-ups on the previous commit, no behaviour change. materializeCompiledSlingFormula had grown two adjacent `if graphWorkflow` blocks; the claimable-step check moves into the head of the existing one, keeping it ahead of the idempotency lookups it must precede. ApplyGraphRouteBinding's comment still claimed DecorateGraphWorkflowRecipeWithDefaultBinding rejects a nameless binding outright. That stopped being true when the guard moved to the sling boundary — decoration now accepts it, because cook depends on that. Reworded to say what the branch actually defends: a nameless binding is fine, recording it as routed pool work is not. Validation: build, graphroute, sling, and the cmd/gc formula-cook / sling / dispatch tests all pass; the full cmd/gc sweep passed on the parent commit (ok, 723s, zero failures).
…the install target (gc-ykvko) (#137) * fix(install): report a gc built against pinned beads, not local source (gc-ykvko) gc runs the beads library in-process, so the library a gc binary links and the bd binary beside it have to agree. `make install` links whatever go.mod pins -- for this fork a pseudo-version naming an untagged beads commit. A Gas Town deployment instead builds gc through a wrapper that applies a `replace` onto the local beads checkout, so gc links the same source bd was built from. Nothing said which build you got: `make install` prints success and exits 0, and the two paths differ only in build info nobody reads. This has now caused three incidents. A written operator note already said verbatim that "plain make install re-skews gc", and it was read AFTER the third one, because `make install` is the obvious command and nothing objected at the point of use. Documentation has failed at this three times, so the remedy moves into the build. scripts/check-beads-linkage.sh runs as the last step of `install`. It reads `go version -m` for the binary just installed and for the bd on PATH, and reports a loud banner when the installed gc links a *pinned* beads module whose version is not the one bd was built from -- naming build-optimized.sh as the supported path. A replace onto a versioned fork module counts as pinned: that binary still links a published snapshot, not the local checkout. The guard reports; it never fails an install. It is advisory by contract (always exits 0) and stays silent wherever it lacks evidence to judge: no Go toolchain, no bd on PATH, unreadable build info, a binary linking no beads at all, or a gc already built against local source. That keeps CI, fresh machines, and upstream contributors -- who legitimately track the pinned dependency -- entirely quiet, which is why this warns rather than hard-fails and needs no opt-out env var. One correction to the filed diagnosis: the reported symptom was a `version_compat` WARN taking the native store offline. That half is already fixed -- 19e0862 (2026-08-11) made an unconfirmable library version (source build, replaced module, or pseudo-version) PASS instead of reporting a mismatch it had no evidence for. So the skew no longer announces itself; it is now silent, which is precisely why a build-time guard still earns its place. The warning text describes today's behavior, not the old WARN. Validation: TestCheckBeadsLinkageGuard drives the script against a fake Go toolchain over ten environments (four warn, six silent), asserting exit 0 in every one. TestMakeInstallRunsBeadsLinkageGuard keeps it wired into the install recipe; TestBeadsLinkageGuardTracksBeadsModulePath keeps its module path equal to the preflight checker's. End to end against real binaries: a plain `go build` gc warns (links beads v1.1.1-0.20260805093327 while bd is v1.2.2-0.20260812111556), and the installed local-replace gc is silent. Full ./scripts/ package and the resource-census ledger pass. The new test adds one exec.Command site, so the three subprocess ratchets (test/test-resources.toml, resourcecensus/census.go, TESTING.md) move together by +1 call / +1 file. Out of scope, tracked on tk-rp4a2: two concurrent build-optimized.sh runs racing the go.mod backup/restore, which lives in the town root script. * fix(install-guard): warn only where a local beads checkout exists (gc-ykvko) The post-install guard warned whenever the installed gc linked a pinned beads library and the bd on PATH named a different published version — including on a machine with no local beads checkout at all. There the advice is unactionable: it points at build-optimized.sh, a Gas Town script that machine does not have, to rebuild against a checkout that does not exist, and the pinned build it flags is the only thing `make install` can produce. That contradicted gc-ykvko's own acceptance criterion — `make install` with no local beads checkout stays quiet, no false alarm for CI or upstream contributors — and the suite pinned the false positive as expected behavior. The guard now resolves a checkout before it warns: bd's own `replace` when it names one still on disk, else $HOME/beads, the location build-optimized.sh resolves. Each candidate is validated by a go.mod declaring the beads module, so a same-named directory is not mistaken for a replace target. No checkout, no warning. The banner names the checkout it found, since that is what makes the advice actionable. Regressions: the reviewer's case (gc pinned, bd pinned to a different version, no checkout -> silent), a $HOME/beads that is not a beads module, and a bd replace whose path is gone. Every warn case now supplies a checkout explicitly, so the town shape that motivated the guard stays covered — verified end to end, where a `go build` gc still warns naming /home/zook/beads and the same binary under a checkout-free HOME says nothing. Addresses the pre-open signoff P1 on gc-ywy8l (gc-tguo4).
The installed bd CLI was v1.2.1 (4ad99760b, Aug 12) while go.mod linked
v1.1.1-0.20260805093327-bf97b73749ac (Aug 5). gascity both links beads as a
library (20 Go files) and shells out to bd, so the CLI was running 555 commits
ahead of the library — above the contract-matrix ceiling this repo exists to
keep honest. This moves both cells of the pin, plus every anchor that restates
either one.
Two anchors are outside TestBDVersionPins's reach and are where a half-applied
bump actually does damage:
- ARG BD_SOURCE_SHA256 is checked only by Dockerfile.agent's `sha256sum --check
--strict`, i.e. only in the agent image build. Verified byte-stable across two
independent fetches of the gastownhall/beads archive at the new ref.
- ARG GRPC_VERSION silently regressed. beads at the new ref already requires
grpc-go v1.83.0 in its root go.mod, so leaving the ARG at 1.82.1 would make
`go get grpc@v${GRPC_VERSION}` a DOWNGRADE — and the build would still go
green, because the final assertion greps for exactly ${GRPC_VERSION} and would
confirm the regression rather than catch it. Raised to 1.83.0 so the `go get`
is a no-op and the assertion keeps guarding a future beads bump that drops
grpc back below this floor. The upstream-owned block is kept, not deleted, and
its stale "published bd 1.1.0 embeds vulnerable grpc-go 1.80.0" comment is
rewritten to state the tracking rule. Dockerfile.base's gh/Dolt grpc floor is
deliberately left at 1.82.1 — it is not driven by beads.
Beyond the anchor set as filed, three more anchors turned up that no version-pin
test guards and that would have failed CI:
- scripts/container_tool_security_test.go hard-pins all four Dockerfile.agent
ARGs plus a `BD_VERSION == "v1.1.0"` fatal and a go.mod grpc-count assertion.
- test/integration/integration_test.go pins the module pseudo-version behind the
`integration` build tag, so `go vet ./...` never sees it.
- scripts/cipolicy pins SHA-256 golden digests of the ci.yml and nightly.yml
execution-shape projections, and those projections include job `env` blocks.
Promoting BD_VERSION in a workflow is therefore an execution-shape change that
the policy requires be re-approved explicitly. Both digests are updated here
after diffing the base and head projections line by line: the only deltas are
the five BD_VERSION values in ci.yml and the two in nightly.yml. Nothing else
in either shape moved.
The pseudo-version strings in internal/beads/contract/preflight_checker_test.go
are table-driven fixtures for version-comparability logic, not repo-pin anchors,
and are correctly untouched.
BD_PREV_VERSION stays at v1.0.4 and the Go floors (bdMinVersion,
bdReadyProjectionMinVersion, the bd_compatibility enum) do not move — floors are
a support decision, not a follow-the-tip one. But the reasoning recorded in
deps.env for both BD_VERSION and BD_PREV_VERSION was invalidated by this bump:
it asserted that no published beads release carries the --if-assignee/--if-status
CAS flags, so BD_VERSION could not be promoted and the source-built cell was the
only flag-capable bd CI could obtain. v1.2.1 is the first published release cut
from beads main after gastownhall#5008 and does carry them (verified against the installed
binary: `bd update --help` names both). Both comment blocks are rewritten so the
recorded reason matches why each value now holds.
No `replace` is introduced or restored: the target commit exists in both
gastownhall/beads and steveyegge/beads, v1.2.1 is tagged in both, and
zookanalytics/beads has zero unique commits, so there is nothing to fork-carry.
scripts/check-gomod-replace.sh (which the pre-rebase fork had deleted precisely
to accommodate a pseudo-version replace) passes.
go.mod/go.sum movement beyond beads itself is MVS resolving beads' own
requirements — otel 1.44→1.45, grpc 1.82.1→1.83.0, x/crypto, x/net, x/text,
google.golang.org/api, and two new indirects (AlekSi/pointer, olebedev/when).
Validation: go build ./... and go vet ./... clean; go vet -tags=integration on
test/integration clean; full ./scripts and ./scripts/cipolicy packages green
including TestBDVersionPins, TestDoltVersionPins and all three container-security
tests; ./internal/beads/... and ./internal/deps/... green;
TestPinnedIntegrationBeadsModuleVersion green under the integration tag;
check-gomod-replace.sh OK; make test-fast-parallel green. All four v1.2.1 release
tarball SHAs added to install-bd-archive.sh were verified against the GitHub
release API digests rather than copied on trust.
…pstream sync (gc-qnmgk) (#139) gc-qnmgk asked to drop 705be0f "TEMP: unbreak the cmd/gc test build after the order-dispatcher routes change" once upstream PR gastownhall#5247 merged. gastownhall#5247 did merge -- as upstream 4068965 on 2026-08-13 -- so the trigger condition is satisfied. The action it gates is not executable as a standalone change today, and this commit records why rather than leaving the next reader to re-derive it. origin/main (3dcdc66) is 47 commits ahead of and 11 behind upstream/main (5a600ec), and does not contain 4068965. Until the sync brings upstream's version in, our TEMP commit is the only thing supplying the fix: newMemoryOrderDispatcher takes a leading routes *storageRoutes parameter on main, and the three cmd/gc test call sites pass the required sixth argument only because 705be0f added the leading nil. Reverting it ahead of the sync drops those call sites to five arguments and reintroduces the exact "not enough arguments" vet failure upstream recorded in gastownhall#5247. That is the one way to get this bead wrong, and it is the way that looks most like following the bead's own title. The decisive evidence is patch-id identity: 705be0f and 4068965 both hash to 042bb76732aba26520d5bcac31662bc0fcfec0e4, so the TEMP commit's self- description ("verbatim cherry-pick of upstream PR gastownhall#5247") is verified rather than assumed. That fixes the survey verdict for mol-upstream-gc-rebase as drop-merged-upstream, which its rebase step drops automatically by patch-id -- no --skip, no conflict, no judgment call. The findings file carries the ready-to-paste verdicts row and the post-sync checks that confirm the fix arrived from upstream rather than from us. Filed under specs/gc-qnmgk/ per docs/file-structure.md: this is a record of what was determined on a bead, not an authoritative claim about the codebase, and it follows the shape of specs/gc-32x8j/findings.md (#135), which recorded the same class of already-converged-with-upstream result. Deliberately no code change and no fork-local equivalent of upstream's fix -- the standing lens on gc-b0pmq prefers upstream's own commit when the rebase cost is near zero, and here it is exactly zero. Validation: go vet ./cmd/gc/ is clean at 3dcdc66 with the TEMP commit in place; every git assertion in the document was run in this worktree against freshly fetched origin and upstream refs.
…er preserves user bindings (2 tests, pre-existing on main) (gc-k6kdh) (#140) * fix(tmux): read key bindings from unfiltered list-keys (gc-k6kdh) tmux 3.7 answers the positional key filter -- `list-keys -T <table> <key>` -- with exit 0 and no output. getKeyBinding treated that empty output as "no prior binding" and returned "", so on every modern tmux the capture that exists to preserve a user's binding before wrapping it silently never happened, and each caller installed its own hardcoded default instead (next-window, previous-window, or ":"). Nothing errored; the binding was just quietly lost. isGTBinding used the same filter and so always reported false, defeating the skip-if-already-configured guard that keeps a repeated ConfigureGasTownSession call from re-wrapping its own if-shell. Both now read the unfiltered `list-keys -T <table>` output and select the row in Go via parseKeyBindingCommand. The unfiltered form is well-formed from tmux 3.3 through 3.7+, so this is version-agnostic rather than a 3.7 special case. The parser anchors on the first `-T` (a command may carry its own, as display-menu does), tolerates the `-r` repeat flag ahead of it, unescapes tmux's backslash-escaped key names, and cuts the command by field position rather than by substring search so a key name appearing earlier in the row cannot shift the cut point. The fail-safe contract is preserved: any row that does not match the documented shape yields "", leaving callers on their own default rather than installing a corrupt binding. Un-skips four bind-key round-trip tests. They gated on IsInsideTmux(), but TestMain unsets TMUX for env hermeticity, so the gate was unsatisfiable and they skipped on every host -- which is how this regression reached tmux 3.7 unnoticed, since those four are the only live coverage of the capture path. The server they actually need is the one ensureTestSocketSession already starts on the package's private socket; that helper now documents the precondition so the gate is not reintroduced. TestSetBindings_PreserveFallbackOnRepeatedCalls was itself a third victim of the same bug -- its probe called the positional filter directly -- and now reads through bindingCommand. Validation: the two originally-failing tests (TestGetKeyBinding_CapturesDefaultBinding{,WithArgs}) pass against the installed tmux 3.7b, as do the four un-skipped round-trip tests. New hermetic table tests in tmux_unit_test.go pin the parser against verbatim 3.7b output (padded columns, `-r`, escaped keys, a command containing `-T`, a GT if-shell row) and the fail-safe cases. TestGetKeyBinding_UnfilteredListKeysStaysParseable is the version-drift guard: it asserts separately that tmux still produces output and that the output still parses, so the next CLI change surfaces as a named parse failure instead of a silent empty capture. * docs(tmux): correct the row-shape guard comment in parseKeyBindingCommand (gc-k6kdh) Self-review touch-up. The comment above the guard claimed it required "the table name, the key, and at least one command field", but the bound only checks the first two -- a row that binds the key and stops there passes the guard and is answered by restAfterField returning "". The behaviour is correct as written (an empty command and no match are the same fail-safe answer, and tmux binds a key at most once per table, so scanning on would find nothing), and both cases are pinned by TestParseKeyBindingCommand_FailsSafeOnUnparseableOutput. Only the comment overstated the check, so the comment is what changes here -- a reader who trusts it would look for command-field validation that was never there. * test(scripts): register the four new runtime-tmux tests in the shard manifest (gc-k6kdh) make test failed on TestRuntimeTmuxManifestMatchesCanonicalLinuxIntegrationInventory: the four tests this branch adds to internal/runtime/tmux were unassigned in scripts/runtime-tmux-tests.manifest. That manifest is not documentation -- scripts/test-go-test-shard reads its test list from the manifest instead of `go test -list` whenever GO_TEST_MANIFEST is set, so an unregistered test is silently never dispatched to any shard. The parser tests would have compiled, passed locally, and never run in the CI job that owns this package. Regenerated rather than hand-inserted: the guard asserts exact discovery order (source order across build.ImportDir's file list), so entries cannot be appended. A throwaway program replicating the guard's own discoverRuntimeTmuxTests reproduced all 357 existing entries in order, differing only by the four additions -- which is what makes the resulting 4-line diff trustworthy. The guard's companion counts move with it, all four exact-match rather than caps: manifest 357 -> 361, untagged 240 -> 243 (the three parser tests carry no build tag), integration-only 117 -> 118 (the drift guard), and the six-way partition 60/60/60/59/59/59 -> 61/60/60/60/60/60, since 361 round-robins one extra test into the first shard. Also corrects the sample fixture's header, which claimed to be verbatim tmux output while abridging the 1 KB display-menu row, and restores tmux's own `align=centre` spelling in that row (with its assertion) so the retained text is a true prefix of what tmux emits.
…_to="mayor" (gc-yzxra) (#141) * fix(doctor): stop hold-label-routed-to and v2-routed-to-namespace undoing each other (gc-yzxra) Two doctor checks disagreed about the canonical spelling of a held bead's route, and each one's --fix re-broke the other. hold-label-routed-to compared the hold:<value> label against gc.routed_to by exact string equality, so in a city that binds its route targets the canonical pairing — short-form hold label, binding-qualified route — read as drift. Its --fix then backfilled the short form, which v2-routed-to-namespace flags and rewrites back to the qualified form. Every gc doctor --fix pass flipped the bead to whichever check ran last and left the other red. Live instance: gascity bead gc-vbkys, filed twice from opposite directions (gc-yzxra wanting the qualified route, gc-sci79 reporting the resulting label mismatch). The spellings are not interchangeable and neither check was wrong about its own side: only "mayor" and "external" are sanctioned hold values (engdocs/contributors/hold-label-conventions.md, ga-tug8ry.1), so the label cannot carry a binding prefix, while a bound route must. The reconciliation belongs in the comparison, not in the data. hold-label-routed-to now resolves the city's bound-alias map (the same boundRoutedToAliases the sibling check builds) and treats a route that is a bound alias of the hold value as matching it; --fix backfills that qualified form wherever the city binds exactly one, so the repair lands on the spelling the sibling check already calls canonical. Values with more than one bound alias keep the short form — the sibling check reports those but deliberately leaves them for manual resolution, and guessing a rewrite target here would be the same overreach. Behavior is unchanged for a city that binds nothing: the alias map is empty, so the comparison stays exact equality and --fix keeps writing the label value. Check details now name both the observed and the wanted route, since with aliases in play the two can differ in a way the old message could not express. Validation: two new tests in cmd/gc, both failing before the change — TestHoldLabelRoutedToAcceptsBoundAliasRoute (qualified route accepted, missing route backfilled qualified, unbound values and other-agent routes unaffected) and TestHoldLabelAndV2RoutedToChecksConverge, which drives the exact gc-vbkys state through both checks' Fix in sequence and asserts they end green together instead of oscillating. Existing hold-label and namespace check tests pass unchanged; go vet and go build ./... clean. Follow-up, deliberately not in this commit: gc-vbkys itself still carries the short-form gc.routed_to="mayor". Repairing the data before this ships would re-create gc-sci79's finding under the installed binary. Once this lands and gc is reinstalled, gc doctor --fix converges it in a single pass — routed_to becomes gc-toolkit.mayor, the hold:mayor label stays as-is, and both checks go green together. The bead's hold:mayor state is left untouched either way. * test(doctor): pin the ambiguous-alias branch of the hold-label route fix (gc-yzxra) Self-review addition. The implementation commit claims a specific behavior for hold values whose short form is bound by more than one binding — --fix keeps the short form rather than guessing a binding — but no test exercised that branch. It is the one case where hold-label-routed-to and v2-routed-to-namespace stably disagree (the sibling reports the ambiguity and deliberately leaves it for manual resolution), so an untested claim there is exactly where a later "simplification" of holdRouteWant would silently restart the oscillation this bead exists to stop. The new test builds a city binding "mayor" twice, then asserts: the missing route is backfilled short-form and the check's detail advises that spelling; an explicitly-qualified route is still accepted as denoting the hold value even when the short form is ambiguous; and the post-fix state is a fixpoint — v2-routed-to-namespace warns but its Fix leaves the value alone, after which hold-label-routed-to still reads OK. Test-only; no production code changed. go build ./... and go vet ./... clean, and all 42 tests matching HoldLabel|RoutedTo in cmd/gc pass.
…-uuhqa) (#142) * fix(telemetry): price the Claude 5 family and name unpriced invocations (gc-kawr5) Agent cost telemetry could not answer "what am I spending, and on which agents?" for always-on agents. Two independent defects were diagnosed, either sufficient on its own to erase an agent from cost metrics. Upstream has since fixed the second; this commit carries the first plus the durable signal that would have caught both. The pricing table stopped at the Claude 4.x family. Every patrol agent runs claude-sonnet-5, which had no entry, and Registry.Lookup matches (provider, model) exactly. Cost is skipped rather than zero-filled for an unpriced pair, so those agents produced no cost datapoint at all. Verified against 24h of usage facts: every patrol fact carried unpriced=true, as did every pool fact on claude-opus-5. Fixes: - Pricing entries for claude-opus-5, claude-sonnet-5, claude-fable-5, claude-mythos-5, claude-opus-4-6, and the undated claude-haiku-4-5 alias. Sonnet 5 uses the standing $3/$15 rate rather than the introductory rate that expires 2026-08-31, so estimates do not silently under-report once the window closes. - gc.agent.invocation.unpriced, emitted at both seams that would have recorded cost, under the same labels. Skipping a datapoint removed the wrong answer without producing any signal that a right answer was missing, so "no series" and "cost nothing" rendered identically. This is the durable half: the rows added above go stale within a model generation. - An agent-token-telemetry doctor check reporting awake sessions with no recorded model usage for over an hour, distinguishing one silent session from every awake session going silent together - the shared cutoff shape that points at the emission path rather than at an agent. The second defect - model-usage sweeps running only for sessions that stop - was fixed upstream by ad4d0ab (gastownhall#4994) while this branch sat in the pre-open gate. That implementation supersedes the one this branch carried (it routes off the fresh bead, skips the boot pass, and needs no persisted marker), so the live-sweep half is dropped whole here: implementation, tests, the Info.UsageModelLiveSweptAt field and its codec entry. Nothing kept below depends on which live-sweep implementation is underneath. Root-cause analysis, evidence, and the rejected alternatives are filed in specs/gc-kawr5/. * fix(pricing): correct Claude Sonnet 5 to the standard $2/$10 rate (gc-88opu) Addresses both pre-open signoff findings on review bead gc-17548. P1 — the Sonnet 5 row priced input/output at $3/$15 per MTok (cache $0.30/$3.75) on the rationale that $2/$10 was introductory pricing expiring 2026-08-31, and that using the standing rate kept estimates from under-reporting once the window closed. That rationale is obsolete: the 2026-08-10 release note records that the $2/$10 pricing "is now the standard price" and that "the previously scheduled increase to $3/$15 per million input/output tokens on September 1, 2026 will not occur." The published table now lists Sonnet 5 at $2 input / $2.50 5m cache write / $0.20 cache hits / $10 output. As written the branch would have overstated Sonnet 5 cost by 50% while claiming LastVerified=2026-08-13. Corrected in all three places that carried the stale figure: the pricing row, the rate test, and the spec doc's rationale. LastVerified moves to 2026-08-14, the date of re-verification. The branch's other added rows (opus-4-6, opus-5, fable-5, mythos-5, the undated haiku-4-5 alias) were re-checked against the same published table and are unchanged — only Sonnet 5 was stale. The remaining $3/$15 entries belong to sonnet-4-6 and the legacy 3.5-sonnet row, which are correct. P2 — the doctor check's threshold comment referenced liveModelSweepInterval; the symbol this branch relies on is upstream's liveModelSweepMinInterval (cmd/gc/usage_compute.go:43). Renamed so the referenced cadence is greppable. Comment-only; no behavior change. Gates on this tip: go build ./... rc=0; go vet ./internal/pricing/... ./cmd/gc/ rc=0; ok internal/pricing 0.013s; ok cmd/gc 3.475s (targeted -run TestAgentTokenTelemetry|TestDoctorCheckNames|TestBuildDoctorChecks). The rate test was negative-controlled: reintroducing $3.00 fails TestDefaultPricingsCurrentClaudeRates/claude-sonnet-5, so it genuinely gates the value rather than passing vacuously. --------- Co-authored-by: Zook Bot <zook-bot@users.noreply.github.com>
… independently pool-routed — two polecats dispatched onto one branch (gc-p64nt) (#143) * fix(sling): retire the direct pool route when a graph.v2 workflow starts A work bead could be reachable from TWO live dispatch surfaces at once: a direct pool route (gc.routed_to) left by an earlier plain sling, and a graph.v2 workflow later poured over the convoy tracking that bead. Both fire, so two workers wake on the same job and target the same branch — where the second one's branch setup destroys the first one's uncommitted work. Observed four times in three days; every one was contained only because the second worker happened to notice the first was still alive. restampWorkBeadRouting already documented the invariant: a graph.v2 work bead must not carry the claim-semantics gc.routed_to key once its workflow has started, because the pool's claim query and the workflow's own dispatch are two uncoordinated authorities. It only ever enforced that for routes it would WRITE. A route that was already there survived the pour untouched. - restampWorkBeadRouting now retires gc.routed_to before stamping gc.execution_routed_to, so the started workflow is the single live dispatch surface for the bead it was attached to. - doStartGraphWorkflow additionally retires the route on every member of the root's input convoy, covering the convoy-first shape where there is no single attach bead (sourceBeadID is empty) and multi-member convoys. Both writes are best-effort with errors surfaced as metadata warnings: by that point the workflow is running, and unwinding it over a routing write would be worse than a warning. fix(sling): refuse a second graph.v2 workflow over a live work bead `gc sling <bead> --on <formula>` minted a brand-new molecule and reported a successful attach even when the target was already driven by a live convoy-first workflow. Nothing saw the collision: such a root clears gc.source_bead_id (so the source-workflow singleton scan misses it) and the bead carries no gc.molecule_id, leaving gc.input_convoy_id -> convoy -> tracked member as the only durable link between them. A convoy-ID key alone cannot close this, because a pour over a bare bead mints a FRESH synthetic input convoy every time, so a second pour never collides with its predecessor's. The check walks convoy membership from the target instead, and scans the convoy directly for a convoy target. sourceworkflow gains ListLiveInputConvoyRoots / ListLiveInputConvoyRootsForItem as the shared implementation; `gc formula cook --attach` now uses the former in place of its private copy, and also gains the both-tiers read a wisp-materialized root needs to be visible at all. --force still launches the second workflow, matching every other bead-state guard on this path. Three tests that pinned the duplicate as expected behavior are inverted, keeping their original assertions under --force. fix(packs): hold at load-context when the work bead is already in flight Independent backstop for the same hazard, defending a different layer: it fires for any producer of a double dispatch, including pours that never go through `gc sling`. mol-polecat-base and mol-scoped-work now resolve the work bead's current owner before touching the workspace and hold — step bead left open, session drained — when that owner's session is still live. Fail-closed throughout: an unreadable bead or an unreadable session list yields "assume a live owner", never "assume unowned". Liveness is the gate rather than staleness, since an idle pool worker between claims is indistinguishable from an abandoned one. * docs: record the one-live-dispatch-surface contract for formulas v2 Spec section 3 gains the invocation-time rule the code now enforces from both directions — a started workflow retires the claim route on its work beads, and an invocation over work a live workflow already drives is refused — including why a bare-bead target needs the convoy-membership walk rather than a convoy-ID comparison, and which override exists on which surface. It also names the surface neither rule covers: a plain `gc sling <bead>` still routes work a live workflow drives. CHANGELOG records the behavior change. * fix(packs): match the owner's session-name form in the duplicate-dispatch gate The load-context gate in mol-polecat-base and mol-scoped-work resolves a work bead's assignee against `gc session list --state all --json` and holds when that owner is still live. It matched the assignee against .alias, an .alias suffix, and .name — but a session record has no .name field at all, and a claim writes the session NAME (exposed as .session_name), not the agent address .alias carries. The query therefore returned zero sessions for a live claim-holder, OWNER_LIVE stayed 0, and the gate proceeded into workspace-setup — it failed OPEN in exactly the case it exists for. The fail-closed default did not cover it: jq succeeded and printed 0, so the empty-output guard never fired. Match .session_name and .id alongside .alias, and treat $GC_SESSION_ID as self-owned so a bead assigned to your own session id is not read as a foreign owner. Verified against a live session list: the session-name form now resolves to 1 (gate holds) where it previously resolved to 0 (gate proceeded); address forms are unchanged, and an absent session still resolves to 0, so genuine work is not blocked. The formula test gains an assertion pinning .session_name — the property the existing structural assertions could not catch. * fix(controller): keep a workflow's route retire alive across route recovery Starting a graph workflow retires gc.routed_to on the work it drives so the workflow is the only live dispatch surface. The controller's route recovery undid that on the very next patrol tick: it promotes a bead's archived gc.run_target route back into gc.routed_to whenever the bead is open, unassigned and unrouted, which is exactly the state the retire leaves behind. The pool then had a claimable bead the workflow was already dispatching -- the two-workers-on-one-branch double dispatch the retire exists to prevent, reintroduced seconds later. Clearing gc.run_target at retire time would fix the symptom and break the cure: that key is the archived route both this recovery and convoy reopen-source restore the bead from once the workflow is gone, and blanking it strands the bead invisible to pool demand (ga-20zd). Recovery instead skips a bead a live graph workflow drives, and checks both launch shapes -- a convoy-first pour links back only through gc.input_convoy_id -> the convoy -> its tracked members, while a workflow attached to a source bead stamps that bead's workflow_id and has no convoy to walk. The gate is the workflow's liveness, not a mark left on the bead, so a workflow that ends without cleanup leaves its work recoverable on the next tick rather than stranded forever. Reads that cannot prove the bead free fail closed and retry next tick. On a city that relocates the graph coordination class the roots are read from that binding; elsewhere they are read beside the work, which is where they live. The spec's companion claim -- that `gc formula cook --attach` enforces the same second-workflow refusal, minus an override -- is withdrawn rather than implemented. An attach graft leaves the bead's own metadata untouched (no workflow_id, no molecule_id, no route retired) and only adds a blocking dependency, so it never becomes a second dispatch surface for that bead, and repeated bare-bead grafts minting their own convoy and root is the v0 design rule (dedupe by targeting an explicit convoy), not a hole in the check.
…-xeufv) (#145) * fix(usage): account compute intervals for sessions closed between passes (gc-23ep6) The usage lane went effectively blind: on 2026-08-15, 70 sessions reached a compute-terminal state and were closed, and only 2 of them recorded a compute fact or a terminal model sweep. .gc/usage.jsonl stopped growing entirely, so cost measurement had no in-city source at all. emitDueComputeFacts is fed sessionBeadSnapshot.OpenInfos(), and loadSessionBeadSnapshot deliberately never loads closed history — it is re-listed several times per tick and closed history grows without bound. An interval is therefore accounted only if some pass observes the session while it is BOTH open AND terminal. But closeSessionBeadIfReachableStoreUnassigned stamps the terminal state and closes the bead in the SAME reconciler pass: the pass before the drain sees an awake session, and the pass after it sees nothing. That window is essentially always empty, so the lane's output was really a function of how long a terminal bead happened to linger before being closed — which is also why the same code once over-emitted (~1200 compute facts/hour, collapsed at read time by IdempotencyKey) when ticks were slower. Both lanes died together because the terminal model sweep runs in the same per-session branch as the compute fact. Rather than reintroduce a closed-history scan, this tracks the set of session ids that still owe an unaccounted interval and diffs it across passes. A session that leaves the open snapshot costs exactly one Get by id — the closed-record read the snapshot loader explicitly sanctions — and is then routed by the SAME processSessionBead the open lane uses, so the decision is made on the fresh bead: a session that merely dropped out of a PARTIAL snapshot is re-read and left alone rather than mis-billed for an interval that has not ended. The set is bounded by the fleet, and accounting that does not settle stays in it and is retried. processSessionBead now reports whether anything is still owed. That is deliberately not "did this call write a fact": emitComputeFactForBead returns false both for a failed write and for a no-op (no interval, or a marker an earlier pass already stamped), and reading the no-op cases as failures would retain a settled session forever and re-Get it on every later pass — an unbounded leak on the synchronous reconcile tick. Note the v1.4.1 binary swap this was originally attributed to is NOT the cause: the running supervisor (PID 3204367, started 2026-08-14 02:21) still holds the pre-swap inode, so the new code never executed in the emitter. Full evidence is on gc-23ep6. Tests: four cases in cmd/gc/usage_compute_test.go — the interval that ends between passes (fails on main with 0 facts), a still-live session that vanishes from a partial snapshot (must not bill, must still bill later when it really ends), retry after a failed sink write, and the settled-session leak bound (fails without the owed-vs-recorded distinction). go vet clean. Follow-up filed as gc-cj68c: the LIVE lane (incremental billing while a session is awake) records nothing either — every open session has invocation_usage_cursor unset, including ones awake 4h+. Awake sessions are present in OpenInfos(), so this change does not touch that path; after this fix facts are recovered at session close, and gc-cj68c still owes billing DURING a long-lived session. * docs(usage): record the gc-23ep6 root cause and narrow the design doc's stated under-count Two documents, both keeping the record true after the compute-interval fix. usage-facts-v0.md claimed the open-set scan's under-count was limited to "a session closed directly from active without first reaching an open terminal state." That understated it badly: the reconciler stamps the terminal state and closes the bead in the same pass, so essentially NO session was observed open and terminal, and 68 of 70 drained sessions were dropped on 2026-08-15. The bullet now describes the cross-pass diff that closes it, and states the narrower under-count that actually remains — a close that writes a state the terminal-state predicate rejects (gc_swept, failed-create, stranded-repair). specs/gc-23ep6/root-cause.md is the investigation record, filed because the bead's leading hypothesis was wrong in a way worth being able to re-check. It carries the evidence that does not belong in a commit body: the /proc/<pid>/exe proof that the running supervisor predates and never executed the v1.4.1 binary it was blamed on, the live /usage response showing the sink was healthy throughout, the per-hour record table showing a 19-hour gap BEFORE the reported 19:05 "stop", the 70/2 measurement, and why the same code once over-emitted at ~1200 facts/hour. It also states what the fix deliberately does not cover. * fix(usage): keep a partial-snapshot session tracked until its interval ends (gc-xeufv) Pre-open signoff on polecat/gc-23ep6 (review bead gc-zigay) found a P1 in the closed-between-passes catch-up this branch introduced: the vanished-session path could still drop an interval outright. accountVanishedIntervals decides each vanished id by re-reading the FRESH bead through processSessionBead, which is what keeps a session that merely fell out of a PARTIAL snapshot from being mis-billed for an interval that has not ended. But the live branch reported that no-op as settled (return true), and settled means "stop tracking". takeVanishedIntervalSessions has by then already replaced the tracked set with the same partial snapshot that omitted the session, so the id is left with no reference anywhere. If the session then drains and closes before it reappears in an open snapshot, the next pass has no owed id to diff against and nothing rescans closed history — the interval is never billed. The controller already treats a partial session query as non-fatal, so the sequence is reachable in production, and it is the same undercount class this branch exists to fix. The return value now means still-owed for the non-terminal branches: a live (or otherwise non-terminal) bead reports owed while its interval is unaccounted, so the catch-up retains it and retries next pass. The open-snapshot caller ignores the return value, so that lane is unchanged. Retention stops at the bead's own close, which is what keeps the tracking set bounded by the live fleet. A bead closed straight from active never reaches a compute-terminal state (the documented v0 scan limitation) and its metadata will never change again, so retaining it would park it in the set permanently and re-Get it on every reconcile tick forever — the leak TestEmitDueComputeFactsDropsSettledVanishedSession already guards for the settled case. Verified load-bearing: dropping the closed-bead guard fails the new leak test. TestEmitDueComputeFactsDoesNotBillStillLiveVanishedSession did not catch the loss because its final pass fed emitDueComputeFacts the stale awake Info row after closing the bead. A real sessionBeadSnapshot.OpenInfos() cannot return a closed bead, so that impossible snapshot re-added the session to the owing set and masked the drop. It now runs that pass with an EMPTY snapshot, and fails without this fix. Validation: go vet ./cmd/gc clean; the three new/changed cases fail on the parent commit with exactly the reported symptoms (0 compute facts where 1 is owed; the id absent from owingIntervals) and pass here, alongside the rest of the usage-compute family. * fix(usage): retain a vanished session whose closed-record read fails (gc-uej6q) Round-2 pre-open review of polecat/gc-23ep6 (review bead gc-9gosq) found the closed-between-passes catch-up still had one path that loses an interval outright: accountVanishedIntervals dropped a vanished session from the owing set on ANY store.Get error. That Get is the last reference the owed interval has. By the time it runs, takeVanishedIntervalSessions has already replaced the tracked set with the current open snapshot, and a closed session never reappears in OpenInfos() -- so a session seen awake on pass 1, drained and closed before pass 2, and hit by a transient backend read failure on pass 2 was never billed on pass 3 even after the store recovered. That is the same data-loss shape this branch exists to fix, one edge later. The store contract only lets ErrNotFound prove absence; every other error is a read that may succeed next pass. Non-ErrNotFound failures are now retained via retainVanishedIntervalSession and retried, and only a confirmed absence drops the session -- which keeps the tracking set bounded, since a genuinely deleted bead would otherwise be re-Got on every reconcile tick forever. Validation: TestEmitDueComputeFactsRetainsVanishedSessionOnTransientGetError was written first and reproduced the loss at the reviewed commit (session dropped from the owing set, interval never billed); it passes with the fix. TestEmitDueComputeFactsDropsVanishedSessionOnConfirmedAbsence pins the bound in the other direction. The surrounding usage lane (Test(ComputeFactGetCandidate|IsComputeTerminalState|EmitComputeFactForBead|EmitDueComputeFacts.*)) and go vet ./cmd/gc are clean. The design doc and the gc-23ep6 root-cause spec both stated the retention rule and are updated to cover the read-failure case; the function's doc comment previously said an unreadable bead is dropped, which is no longer true.
… CLI lever releases it, so an on_demand agent can never exist again (gc-5fdrr) (#144) * fix(session): release a closed bead's name claim for its own configured identity (gc-5fdrr) A CLOSED session bead kept the runtime identifiers it ran under, and name resolution still consulted them, so an on_demand agent's alias could be reserved forever by a dead record. The live incident (gc-5fdrr) held `shutupandlisten/gc-toolkit.refinery` unspawnable for ~40h: every documented lever refused it — nudge/wake by alias hit "name already exists", nudge/wake/ kill by bead ID hit "session is closed", `prune` does not accept the closed state, and `session new` has no reclaim flag. It was broken only by hand metadata surgery on three undiscoverable keys. Root cause is a divergence between two checks of the same name. The named-session start path clears `EnsureSessionNameAvailableWithConfigForOwner`, whose legacy bypass exists precisely for pre-ga841 phantoms, and then `Manager.CreateSession` re-checks the same name through the cfg-less `ensureSessionNameAvailableForSelfAndOwner`. That inner check recognized a closed configured-named holder only by the boolean flag or the recorded identity — both owner-AGNOSTIC — so a phantom carrying its identity solely as alias/agent_name (ga-n2d Gap C shape) fell through and the inner check vetoed what the outer check had just allowed. The fix reuses `configuredNamedIdentitySignalsMatch`, the owner-SCOPED recognizer already used for this same trap in the startup sweep and the alias-availability check: a CLOSED holder releases the name only to the configured identity its own recorded signals resolve to. A different claimant, an ownerless claimant, and any live holder are all still rejected, so the permanent-identity rule for ad-hoc explicit names is untouched. Two things the automatic release deliberately does not cover — an ad-hoc explicit name, and a holder whose signals name no claimant — still left the operator with no lever, so this also adds `gc session release-name <name-or-alias>`: it clears session_name, alias, and the canonical-identity record from CLOSED holders (agent_name survives as history), refuses all-or-nothing when any holder is live, and no-ops cleanly when nothing holds the name. The ErrSessionNameExists message now says the holder is closed and names that command, so the next operator to hit this reads the remedy off the error instead of diagnosing for 40h. Validation: new unit tests in internal/session cover the release, the other-owner / ownerless / live-holder guards, the error text, and the release-name domain function; cmd/gc tests cover the command end to end. The generated command census, metrics catalog, example schema enum, and CLI reference were regenerated by their own generators; the productmetrics catalog-size baseline moves 199 -> 200 for the new command. On ask 4 of the bead (make gc doctor's `configured-named-conflict` finding say a closed holder needs metadata surgery): that check skips closed beads outright (`cmd/gc/doctor_session_model.go`), so it only ever reports a LIVE conflict, and its premise no longer holds — a closed holder is now either auto-released to its own identity or releasable with one command. The live finding is left as-is: a live conflict's remedy is stopping or reconciling that session, not releasing a name. * test(session): pin the CreateSession layer that vetoed the gc-5fdrr spawn The helper-level tests cover ensureSessionNameAvailableForSelfAndOwner in isolation, but the outage happened one layer up: the named-session start path pre-cleared the name through the cfg-aware check and Manager.CreateSession then re-checked it through the cfg-less helper and refused. A test that never goes through CreateSession would not have caught that divergence. This adds the end-to-end guard — a bead-only CreateSession for a configured named identity over a CLOSED legacy holder whose identity survives only as alias/agent_name. Confirmed to fail without the fix with the exact production error ("already belongs to gc-1 (closed)") and pass with it.
… no guard, no alarm (gc-x1a87) (#149) * fix(supervisor): make a stale controller holder terminal and visible (gc-x1a87) `gc supervisor run` refused to start against an existing holder by printing one line and returning a bare 1. systemd's Restart=always/RestartSec=5s turns that into an unbounded crash loop: RestartPreventExitStatus covered only supervisorExitCodePortInUse, and the unit sets no start-limit ceiling. Between 2026-08-14 and 2026-08-19 that ran to NRestarts=83439 against a holder whose /proc/PID/exe was "(deleted)" — an image no rebuild could ever reach. Nothing alarmed; the condition was found by hand a day late and cleared only by a host reboot. While it stood, every landed controller/session/sling fix was inert in the running city, which is how a closed duplicate-dispatch defect appeared to recur. Two halves, matching the two gaps. Terminal exit: "another supervisor holds the city" gets its own exit code (supervisorExitCodeAlreadyRunning) listed in RestartPreventExitStatus beside the port-in-use code. This is the treatment the port collision already got for the identical failure mode (ga-ceq) — a duplicate supervisor meets the same holder on every attempt, so restarting it can never make progress. Stale-holder detection: classifySupervisorHolder reads /proc/<pid>/exe and distinguishes a holder running an unlinked image from a live one. It is deliberately fail-closed — a stale verdict makes a start terminal, so no procfs (darwin), a uid mismatch, or any non-definitive stat error degrades to Unknown and behaves exactly as before. The " (deleted)" suffix is ambiguous for a binary genuinely named that way, resolved by inode identity against the procfs link, the same way cwdStateFromLink resolves it for working directories. A stale holder gets a loud diagnostic naming the state, the remedy, and the one-line verification; the three sites that print this sentence now share one renderer so they cannot drift. Visibility: gc doctor had no binary-identity coverage at all — drift detection lived only on the `gc start` path, which is why five days passed unnoticed. The new supervisor-stale-image check reports when the holder is serving an unlinked image. A deleted inode alone is routine (every `go install` unlinks the image the supervisor still holds), so the bare marker is a warning and it escalates to an error only when a build-identity comparison confirms the running city no longer matches the built code. Both verdicts are advisory: the warning form is normal in any tree that rebuilds gc, so gating dispatch on it would halt development — the value here is visibility, not a gate. A terminal exit alone would have converted an invisible crash loop into an invisible dead supervisor, which is why the doctor check is not optional. Validation: new tests in cmd/gc/supervisor_stale_holder_test.go cover the classifier (live, unlinked, replaced inode, genuinely-named, and all three fail-closed paths), both message forms, the systemd unit listing both terminal codes with the template field actually populated, the corroboration rule, and every doctor verdict. The pre-existing port-collision guard test still passes unchanged. Deliberately not added: a StartLimitBurst ceiling — it is a hard cap on total restarts, so it would trade this bug for a permanently dead unit after any transient overlap. * fix(supervisor): fold self-review findings into the stale-holder guard (gc-x1a87) Self-review touch-ups on top of the implementation commit; no behavior change beyond the test assertion, which was previously asserting the bug. - Reuse DetectBinaryDrift for the doctor check's build comparison instead of re-implementing `local != remote`. Drift semantics now live in one place, which matters because the build identity is not a bare hash: it carries dirtySuffix, and the doctor check and the `gc start` drift path must not disagree about whether two stamps describe the same revision. supervisorBuildDrifted keeps only the comparability guard the escalation needs. - Update TestRunSupervisorRejectsSupervisorOnFallbackSocket to assert supervisorExitCodeAlreadyRunning. It asserted a bare 1 — the exact value RestartPreventExitStatus cannot cover, and so the value that let the duplicate crash-loop 83439 times. - Record why the loud already-running diagnostic cannot mislead on darwin: it is reachable only behind a procfs read, so launchd hosts render the plain form by construction. supervisorPortInUseMessage has to branch on GOOS explicitly for the same reason; note the asymmetry at both sites so neither reads as an oversight.
…r path, so an ordinary bd update hard-quarantines the db and blocks reclaim (gc-800l fix gap) (gc-i52hj) (#147) * fix(dolt): settle same-count hash drift on removals, not on the hash (gc-i52hj) The post-flatten integrity gate had three defer arms and every one of them was guarded off by `same_row_count_hash_drift`. That is the category an ordinary in-place `bd update` produces — the single most common write in a running city — so a proven writer race whose only anomaly was same-count drift fell through to a hard quarantine, and a quarantine marker blocks all future GC of that database until a human clears it. lx hit this on 2026-08-15 (bead lx-ey1xe). Both writer-race signals fired, the concurrent writer was identifiable by name ("bd: update lx-szxza", six seconds after the flatten), and DOLT_DIFF_STAT over the window showed rows_added 0, rows_deleted 0, rows_modified 1, rows_unmodified 1237. Nothing was lost. The marker still blocked reclaim for 4 days; lx reached 1.8G, 7-11x every other database on a host already at 80%, and clearing it reclaimed ~900MB in 50s. Third quarantine of this class after lx 2026-08-09 (gc-800l) and gc 2026-08-11. The fix is the rule gc-800l's own "## Fix" section specified and its implementation did not carry to this category: counts and hashes are the cheap TRIGGER, the verdict comes from the diff. Any table value-hash drift — with a row-count gain or at an unchanged count — is now settled by diffing the pre-flight snapshot HEAD against the flatten commit per drifted table and counting `removed` rows. Zero removals proves every pre-flight row is still reachable, so it defers and retries next run. Added and modified rows are ordinary live-store traffic. Any removed row, or a probe failure, fails closed to the quarantine exactly as before. Note this drops the `writer_race_detected` precondition for the drift path rather than adding same-count as a fourth race-gated arm: the DOLT_DIFF proof is direct evidence, strictly stronger than the HEAD proxy it replaces, so it does not need the proxy to agree first. That also covers the absorbed-writer race the HEAD gate structurally misses. Row-count decrease keeps its own HEAD-proven arm — fewer rows with no removals is a contradiction, so admitting it could only ever fail closed — and table-list drift and probe failures stay excluded for the same fail-closed reason. `diff_is_additive_only` is deliberately left alone. Upstream gastownhall#5049 gave it a second consumer, the committed-root drift proof's first-committed table case, where the table exists in no pre-flatten commit and so has no pre-flight rows to preserve; there anything but a pure add really is unexplained. The two predicates now ask different questions and a unit test asserts each keeps asking its own, so a later cleanup cannot quietly collapse them. Validation, positive control both ways per gc-800l's bar. The new `same_row_count_row_loss` harness mode produces the identical count/hash signature as the benign in-place update and differs only in what DOLT_DIFF reports, so the two Go tests isolate the discriminator: same-count drift with no removals defers and writes no quarantine marker (TestCompactScriptDefersSameRowCountWriterDrift), and the same signature with removals still quarantines before full GC with its drift evidence recorded (TestCompactScriptQuarantinesSameRowCountDriftWithRemovedRows). A run that only showed "no longer quarantines" would have removed the alarm, not fixed it. Full `examples/bd/dolt` package green (153s); 14/14 in the shell unit test. The harness's removal-probe arm has to sit ahead of the `SELECT COUNT(*) FROM <table>` arms, whose patterns also match the proof query's text, and matches on the `diff_type = 'removed'` clause so it stays clear of the additive-only probe answered further down. Recovered from 8f2a2b1 (PR#107), which landed this fix and was shed when main was rebased onto upstream; that commit is an ancestor of no current ref. Ported onto the evolved upstream rather than cherry-picked, since gastownhall#5049 split the shared helper out from under it. Its bundled `quarantine_db_hash_field` change — rendering the marker's two database-hash fields as four distinct states — is a separate concern and is NOT included here; filed as gc-c845v. * docs(dolt): renumber the duplicated compact step 4b and scope the drift verdict (gc-i52hj) Two self-review touch-ups on the row-preservation change. Both are comment/prose only — no executable line moves. run.sh: the new row-preservation proof was inserted into the header summary as step 4b, but 4b was already the committed-root drift gate, so the file listed two different steps under one label. The prose reference added in the same commit ("settled by the removal-based proof in step 4b") points at the new one, so the committed-root gate becomes 4c. dolt-bloat-recovery.md: the new paragraph stated the removal rule unconditionally — "quarantines only when rows were removed" — but the drift path is also guarded by saw_row_decrease, saw_table_list_change, and saw_probe_failure. In any of those the run never reaches the DOLT_DIFF proof and quarantines regardless of removals, which is exactly the case an operator would misread the old wording against (the mixed decrease-plus-same-count-drift run, where the same-count reason string can be written with no removal probe run at all). Adds the scope sentence rather than lengthening the two table cells above it, which the paragraph now covers. Gates: 14/14 shell unit test; examples/bd/dolt package green (72s), with the four positive controls selected by name and passing in both directions (defer on no removals, quarantine on removals); go vet ./examples/bd/dolt/... clean; go build ./... clean; make check-docs green.
…a branch shipped ungated (gc-01o2l) (#148) * fix(pre-push): drain grep -q of its pipefail SIGPIPE false-negative, and announce every skip (gc-01o2l) The push test gate silently skipped its entire `go test` fan-out on a repeat push, shipping a branch ungated. Fifth sighting of a known defect class. Root cause — the .go-change probe was a pipefail SIGPIPE race: if git diff --name-only "$remote_sha" "$local_sha" -- '*.go' | grep -q . `grep -q` exits the instant it matches without draining stdin, so git races into a SIGPIPE; under the hook's `set -o pipefail` that 141 becomes the pipeline's status, the `if` is false, `go_changed` stays 0, and the hook exits 0 having run nothing. Reproduced on the exact SHAs from the report (74c0407 -> d95274b, 874 changed .go files): the pre-fix pipeline reports "no Go changed" in 5 of 20 trials. That is the whole incident — run 1 raced the safe way and ran the fan-out, the identical run 2 twenty-nine minutes later raced the other way and skipped it. It is also why the bead's "ruled out: go_changed computing 0" note did not hold: a single simulation gets the right answer 3 times in 4. The probe now captures the file list and tests it, which reads to EOF and cannot SIGPIPE (0/40 wrong on the same range, 0/30 on a 1102-file range). A diff that cannot be computed at all now fails CLOSED and runs the suite rather than being read as "no Go changed". Second half, the "silently": every path that declines to run the suite now says so on stderr, and the fan-out announces itself before exec. A bare `exit 0` is indistinguishable from a suite that ran and passed, which is why nothing downstream noticed a gate that had failed open. Guards: - internal/testpolicy/shellpipeline extracts the detector that already guarded the core pack's shipped scripts, so a new asset tree is covered by an import rather than by a copy of the regexes. Copying it per-tree is what let this class recur after the first two fixes shipped only per-script behavioral tests. - TestGitHooksAvoidPipefailGrepQPipelines runs it over .githooks/, which the existing PackFS walker never reached — exactly where this one landed. - TestPrePushRunsFanOutWhenGoDiffExceedsPipeBuffer drives the real hook over a diff large enough to make the SIGPIPE deterministic. The threshold is empirical, not the 64KiB pipe capacity: grep reads greedily before matching, so ~190KiB of path output fails 20/20 pre-fix while ~69KiB passes every time. - TestPrePushAnnouncesEverySkip pins the loudness on all three skip paths. Not addressed here: gc-uz8az proposed additionally failing CLOSED when git supplies zero ref lines. Its PR #93 was closed unmerged, so that path is still live on main; this change makes it loud but leaves its exit status alone. * test(pre-push): funnel the gate's spawns through one call site and ratchet the census (gc-01o2l) Self-review follow-up to the pre-push gate fix. Two things the first commit left for the quality gates to catch. Consolidation: the new contract test spawned subprocesses from five separate `exec.Command` sites (a git helper in the fixture, the hook invocation, a rev-parse, and a commit pair in each of the two fan-out tests). Driving a shell git hook cannot avoid spawning it, but it can avoid spawning it from five places — everything now funnels through a single `runIn` entry point with a thin `git` wrapper over it. That drops this file's cost to the resource census from five call sites to one, and removes the duplicated "CombinedOutput + t.Fatalf" block that had been copied four times. Census ratchet: the file still adds one subprocess call site and one file, so TestRepositoryLedgerMatchesCensusAndDocumentation requires the matching baseline update. Bumped +1 call / +1 file across all three mirrors that must agree — test/test-resources.toml (the ledger), the bootstrapPolicy in internal/testpolicy/resourcecensus/census.go, and the TESTING.md tables — for the audit baseline, the source debt ratchet, and the Small debt ratchet. Same shape as f03193c, which is the precedent for a single new spawning test. The untagged invariants read "cannot grow", and the alternative to growing them was a build tag. That was rejected: a build tag would move these tests out of the default unit tier, and a guard against a push gate that silently fails open is worth little if it only runs when someone opts into it. Validation: ./scripts, ./internal/bootstrap/packs/core and ./internal/testpolicy/... all pass; go vet clean. * test(shellpipeline): add the required testenv import file for the new package (gc-01o2l) TestRequiresDedicatedTestenvImportFile in internal/testenv requires every real test directory to carry an untagged testenv_import_test.go importing internal/testenv, so leak-vector env vars are scrubbed before tests run and a leak from an agent session cannot corrupt a live city or spawn orphaned infrastructure. The new internal/testpolicy/shellpipeline package had tests but no such file, so the push gate failed closed on it. Generated with the canonical `go run scripts/add-testenv-import.go` rather than hand-written — the file carries a DO NOT EDIT header and the generator is what the failure message points at. Caught by the pre-push gate this branch repairs, on its first run after the fix.
…ons (gc-w8sxu) (#146) The agent-token-telemetry check treated every awake session as one that owes token samples. Four of them never invoke a model: the control dispatchers are `gc convoy control --serve --follow` Go processes started through an agent's start_command, so they can never record a sample however long they run. The check reported them every hour as "last token sample never" and rolled them into a blocking warning that no action could ever clear. That was correct-but-mislabelled while telemetry was genuinely dead (gc-f1081), which is why the standing guidance was to leave it firing. gc-f1081 closed 2026-08-19T07:15:30Z and the live lane is recording again -- 11 of 16 open session beads carry an invocation_usage_cursor, and the five without are the four dispatchers plus a just-spawned polecat -- so what remained was a check firing blocking against processes behaving correctly. It had already produced one escalation to the mayor (lx-wisp-blqj) whose entire content was four dispatchers and two legitimately-idle agents. Two changes, both scoped to this check: - Population. A session is measured only if session.ProviderFamilyFromMetadata resolves a provider on some rung of the canonical ladder (builtin_ancestor -> provider_kind -> provider). It resolves to "" only when a session records no provider at all, which is the on-disk shape of config.ResolveProvider's start_command escape hatch: a Command and no provider Name. This is deliberately narrower than reusing worker.InvocationUsageFamily -- that would also drop a session whose provider family has no registered extractor, which is a real coverage gap worth seeing rather than a non-agent. Verified against the live city: of 18 open session beads exactly the 4 dispatchers are excluded, and all 14 agent sessions stay measured, including a codex polecat. The discriminator is the provider, never a session name or template, so no role name enters Go. - Severity. The result is now SeverityAdvisory, joining backlog-depth, fork-rate and rollout-gates. The check's own doc comment already recorded why a finding is not separable from benign idleness -- an on_demand agent parked between wakes is silent for a good reason -- which is the definition of a reading for an operator rather than a gate for automation. Idleness is explicitly not the discriminator: an awake session that runs a model and records nothing still fires, and that is pinned by a test. The exclusion is reported in the message rather than applied silently, because this check exists precisely because an absent telemetry series is indistinguishable from zero spend (gc-kawr5); an unreported exclusion would rebuild that blind spot one level up. Tests: four added, covering the excluded population, that a silent model session among dispatchers still fires and is not diluted in the denominator, that the exclusion is accounted for in the message, and the advisory severity. The existing fixture helper now stamps a provider, because a real LLM session always has one -- ResolveProvider fails an agent that names none. All 9 tests in the file pass; go vet and go build clean.
…maly (gc-zvffx) (#150) Step 6's backup-age gate picks a freshness state file, and treats that file being ABSENT as a single condition. It is really two, and conflating them latches the gate closed with no clearable path: - a registered Dolt destination with no dolt-backup-state.json means the backup has never once completed. Something is configured to protect the scope and is not working: a real finding. - no registration AND no legacy backup_state.json means no backup pipeline is configured at all. That is a standing operator configuration, and no backup action can produce the missing file -- so the anomaly re-fires every run, forever. This town is the second shape. `.beads/dolt-backup.json` is absent, so the gate selects the legacy path; `.beads/backup/` exists but is empty (bd's backupDir() MkdirAll's it whether or not a backup follows) and `backup.enabled=false` in .beads/config.yaml, so nothing has ever written backup_state.json. Every reaper run recorded an anomaly and shipped it to the `human` mailbox via escalate.sh, on the order of 48 messages a day, burying real escalations in the operator's own inbox. The gate's own comment says it mirrors doctor's scanBackupFreshness. It did not: scanLegacyBackupFreshness returns ("", false) -- no finding -- when the legacy file is absent, deliberately leaving "no backup at all" to DoltBackupCheck. This restores that agreement. Both cases still SKIP the prune. The gate stays fail-closed on the destructive operation regardless of why a fresh backup could not be confirmed; only the reporting changes. The unconfigured case now sets SESSION_PRUNE_SKIP_REASON, prints the probed paths to stderr, and adds `bulk_prune_skipped:...` to the run summary, so the skip stays visible without escalating. Trading a noisy failure for a silent one would not have been an improvement. The registered-but-never-synced message is retitled from "backup stale or absent" to "backup registered but never synced", which is now the only state that branch can describe. Note on the filed diagnosis: the bead proposed either writing dolt-backup.json or repointing the probe at .beads/dolt/backup/backup_state.json. Neither is right. That third path is `<beadsDir>/backup/backup_state.json` resolved against beadsDir=.beads/dolt -- a relic of a 2026-06-05 run, not a live state path. beads' legacy writer (cmd/bd/backup_export.go backupDir()) writes <FindBeadsDir()>/backup/backup_state.json, which from the city root is exactly the .beads/backup path the gate already probes. Pointing at the relic would read a 76-day-old timestamp and stay latched; writing a registration file for a destination that does not exist would fabricate state. Validation: test/reaper_prune_backup_guard_test.sh extended from 7 to 8 cases. T1 now asserts no-pipeline yields no anomaly plus a recorded skip reason, T6 asserts registered-but-never-synced still escalates and records no skip reason (the two absent-cases are genuinely discriminated, not both silenced), and new T8 covers a scope with no backup/ directory at all. T2-T5 and T7 unchanged and still green, as is test/reaper_session_pattern_test.sh. The gate was also replayed read-only against this town's live .beads: the pre-change gate records the anomaly, the post-change gate does not, and the prune skips in both. Follow-up: neither test/*.sh is wired into the Makefile or any CI workflow, so this guard runs only when a human runs it. Filed as gc-s60fv. 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
…e scope with a dead server until some later gc invocation notices — a transient ENOSPC panic became a 6h58m city-wide data-plane outage (2026-08-19) (gc-zl5ta) (#151) * fix(dolt): restart a crashed managed dolt server, and alarm when recovery gives up (gc-zl5ta) The scope watchdog supervised a lifetime but never restarted anything. When its `dolt sql-server` child exited, runManagedDoltScopeWatchdog logged the exit and returned, and via init() that return became os.Exit — so a mid-lifetime crash left the scope holding a dead server until some later, unrelated `gc` invocation happened to notice. On 2026-08-19 nothing did for 6h58m: the root fs hit ENOSPC, dolt died on a fatal journal-write panic (exit status 2), and the city data plane stayed down with zero bead writes. The outage was also silent, because mail, nudges and beads are all bead-backed — a data-plane outage is structurally unreportable by the machinery that would report it. Two halves, matching the two gaps. Crash recovery: the watchdog now restarts a crashed server with exponential backoff on a rolling budget (5 restarts per 10 minutes, capped at 30s between attempts), re-snapshotting the child's OS start identity each generation so the PID-reuse-guarded terminate paths always describe the child that is actually running. The backoff is held in the select rather than slept, so signal forwarding and scope-gone detection stay responsive throughout, and a scope deleted mid-backoff abandons the restart instead of recreating the orphan the watchdog exists to prevent. Only a self-inflicted crash is restarted. `gc dolt stop` signals the dolt PID directly and never touches the watchdog, so a supervisor that restarts a signaled child makes the managed server unstoppable. classifyManagedDoltChildExit therefore restarts only a non-zero exit the child chose itself; a clean exit, a signal death, and an unclassifiable wait failure all fall through to the pre-fix behavior. Because that classification infers intent from how the child died, a restart also re-reads the runtime record first and stands down when it says the scope is stopped — the state `gc dolt stop` leaves behind — so a dolt that exited non-zero on its way down a stop still cannot be resurrected. Both are deliberately fail-closed: a missed restart is the status quo, an unwanted one is a new and worse bug. The budget is rolling rather than a lifetime cap so a city that crashes once a month never arrives at its next crash with the budget spent; it is bounded at all because an unbounded loop trades a dead server for the crash loop gc-x1a87 hit at 83439 restarts. A successful restart repoints the runtime PID/state records at the new process. Without that the restart would be worse than the crash: assessExistingManagedDolt refuses to reuse a server whose live PID does not match dolt-state.json and starts a second one, so a stale record would trade one dead server for two live servers over one data dir. The rewrite is conditional on the previous PID still owning the record, so a `gc` that started its own server while we were restarting is never overwritten. Visibility: a bounded budget can be spent, and a supervisor that gives up silently just swaps one invisible outage for another. Giving up now writes a pinned marker to the dolt log and the watchdog's stderr — plain files, the only channels that still work when Dolt is what died, since mail and nudges are themselves bead-backed. The message names the crash count, the budget, the last pid, and where to read the cause. Deliberately NOT a new doctor check. `gc doctor`'s existing dolt-server check already dials the endpoint over TCP and errors when it is unreachable, without touching the store, so it reports this outage today; a second check would report one fault twice. What no check can say is whether the supervisor tried and stopped trying, and that is what the marker carries. That is also why giving up does NOT clear the runtime record. Every consumer already gates on pidAlive, so a record still naming a PID that is gone is not a lie about the present — it is the durable evidence that this scope expected a server and lost one, which is what distinguishes a crash from an operator's `gc dolt stop` (that path clears the record itself, and the restart path reads the difference). On "keep judgment out of Go": the restart decision is a POSIX wait status, not a heuristic. There is no is-it-stuck inference here — the kernel reports whether the child exited on its own or was signaled, and the policy over that fact is a bounded count, not a decision tree. Validation: new tests in cmd/gc/dolt_scope_watchdog_restart_test.go cover the exit classifier over real wait statuses (clean, non-zero, signaled, non-ExitError), the backoff curve and its cap, the rolling-window prune, both env overrides, the conditional record rewrite including the foreign-owner refusal, the stopped-record veto and the three inputs that must not trigger it (no city, no record, unreadable record), the pinned alarm marker, and three process-level cases against a fake dolt that crashes on demand: a crash is restarted and the watchdog survives, an externally SIGKILLed server is not restarted, and a server that crashes every time exhausts its budget and alarms. A fourth process-level case pins the documented budget=0 opt-out to exactly the pre-fix behavior, which is also the control for the restart tests — its assertions are the exact opposite of theirs over the same fake, so a change that quietly stopped restarting could not satisfy both. The pre-existing scope-watchdog tests (scope-gone reap, start identity, survives-scope-present) pass unchanged. Not added: a systemd-style unlimited restart, and any escalation through mail or beads — the first is the crash loop this deliberately bounds, the second is the channel that was down during the incident. * test(dolt): pay for the watchdog's new tests by consolidating the old ones (gc-zl5ta) Self-review touch-ups on the crash-recovery commit. No production behavior change; the one production edit splits a function without altering what it decides. The repository resource census counts process-spawn and fixed-sleep call sites in test source, and its ledger invariant is explicit that the untagged totals "cannot grow; reductions must lower this baseline". Three of the new tests are process-level by nature — a supervisor can only be proven by supervising a real process — so the fix was to stop paying for the same call site repeatedly rather than to ratchet the baseline up. Consolidation. The three scope-watchdog tests each open-coded the identical `exec.Command(os.Args[0], "-test.run=TestManagedDoltScopeWatchdogHelper")` spawn, and three more open-coded the same 20ms poll loop. Both now go through one helper each — runManagedDoltScopeWatchdogHelper and waitForManagedDoltScopeCondition — which the new tests reuse instead of adding their own. Net effect on this file: 5 spawn sites to 3, 6 sleep sites to 4, and the new test file contributes none of either. Hermetic classifier. classifyManagedDoltChildExit's rule is now classifyManagedDoltWaitStatus, over the OS wait status alone, with the error-shaped wrapper reduced to unwrapping. The table test builds statuses directly in the POSIX wait(2) layout — and asserts each fixture really encodes what it claims, so a mis-encoded constant fails loudly instead of making the case vacuous — rather than spawning a process per case to manufacture one. It covers more statuses than the process version did (SIGTERM as well as SIGKILL, exit 137 as well as exit 2), costs no processes, and runs instantly. Ledger updates. The four affected baselines drop by 2 each across all three mirrors that must agree — test/test-resources.toml, the bootstrapPolicy literal in internal/testpolicy/resourcecensus/census.go, and the generated TESTING.md block (regenerated with the -update flag, not hand-edited). internal/testenv's GC_* env-read golden gains the two new watchdog knobs, which sit beside the existing GC_DOLT_SCOPE_WATCHDOG_INTERVAL_MS they are modeled on. Also dropped the unused *exec.Cmd field from the per-generation child struct; the PID and the wait channel are all the supervise loop reads. Validation: `make check` is green end to end — fmt, lint, vet, the row checks, the 170-package sweep (0 failures), and all six cmd/gc shards. The census and env-baseline tests, which both failed before these updates, pass.
… ~1-in-3 in cmd/gc unit shard 1 (pre-existing) (gc-04375) (#153) * fix(session): suspend tears down a mid-create runtime instead of refusing (gc-04375) Manager.Suspend rejected start-pending and creating with an illegal-transition error before it ever reached the provider Stop, so a session whose create had been issued but not committed kept its live runtime and handed the caller an error it had no other lever to act on. Manager.Kill already accepts both states as "a runtime process could plausibly exist"; suspend now agrees. This is what made TestCityRuntimeForceShutdownTearsDownAfterLateAsyncSweep flaky (~1-in-3, worse under load). The force-shutdown late sweep exists to catch sessions created too late for the first stop pass, which are precisely the ones whose create commit has not landed. Which stop verb the sweep used came down to a race: markCityStopSessionSleepReason only marks sessions already in "active", so if the async commit won, the bead was marked, stopTargetThroughWorkerBoundary took the Kill path and the runtime died; if the commit lost, the bead was still "creating", the Suspend path rejected it, and the runtime survived the shutdown. Both interleavings now stop the session, so the test is deterministic and the leak is closed for every caller, not just this one. The error was invisible because the shutdown path writes it to a discarded stderr. The carve-out mirrors the failed-create one directly above it, including leaving the bead where it is rather than marking it suspended: an in-flight create may still be running and the reconciler owns reaping a create that never completed, so recording "suspended" would invent a lifecycle the session never had. The transition table is deliberately untouched — creating still does not accept suspend as a lifecycle transition; Suspend short-circuits before consulting it, exactly as failed-create does. Validated with a new deterministic table test over both mid-create states (TestConformance_SuspendMidCreateTearsDownRuntime), which fails on the parent commit with the illegal-transition error and passes here. Full internal/session, internal/worker and internal/api packages pass; go build ./... and go vet ./... are clean. * fix(session): report a live mid-create teardown failure instead of discarding it (gc-04375) Self-review follow-up on the previous commit. The mid-create branch copied failed-create's blanket `_ = m.sp.Stop(...)`, which reproduces the bug it was added to fix one layer up: a live runtime that refuses to die would be reported to the stop sweep as a clean teardown, and the leak would be invisible again. The branch now uses the same running-aware discipline as the active path directly below it. A Stop error against a session that was not running stays quiet — start-pending routinely has no runtime yet, because the provider start may not have been issued, so that is the ordinary already-gone case and erroring on it would make `gc stop` report a failure for every such bead. A Stop error against a runtime that was live is surfaced. This does not re-open gastownhall#2597, the incident behind failed-create's unconditional nil: the stop sweep logs a per-target error and continues to the next target, so a rare provider failure cannot block a city-wide stop. What blocked gastownhall#2597 was the illegal-transition rejection, which failed unconditionally for every bead in the affected state. Adds TestConformance_SuspendMidCreateReportsOnlyLiveTeardownFailures covering both halves. Its errors.Is assertion is what pins the new branch: a pre-fix run also returns non-nil there, but returns the illegal-transition error rather than the wrapped provider failure. * docs(specs): record the gc-04375 root cause and why three triage rounds missed it (gc-04375) Three beads (gc-04375, gc-drm7k, gc-gvcal) and several independent measurement rounds classified this as "flaky, pre-existing, load-dependent" without reaching the cause, and the bead accumulated a workaround (serializing the gate with LOCAL_TEST_JOBS=1) in the meantime. The diagnosis is worth writing down so the next person meeting a similar failure does not repeat the search. Two properties generalize past this bug and are the reason it stayed hidden. The failure detail was real and printed on every failing run, but the test discards the writer it goes to, so the assertion looked bare. And the 0.00s duration on every recorded failure ruled out every timeout in the fixture, pointing at a scheduling-order race rather than a duration race — which is why no amount of timeout tuning would have helped. Filed under specs/ rather than docs/ because it records what was found while working this bead, not a standing claim someone must keep current. The authoritative statement of the new behavior lives in the code comments in internal/session/manager.go and in the conformance tests. * test(cmd/gc): pin force shutdown stopping a mid-create session deterministically (gc-04375) TestCityRuntimeForceShutdownTearsDownAfterLateAsyncSweep reaches the mid-create stop path only by luck: it races the async start's commit against shutdown, so it takes that path when the commit loses and the kill path when the commit wins. That race is now safe in both directions, which is exactly why the test stopped failing — and also why it no longer proves anything about the case that broke. A test that passes whichever way a race falls cannot guard the fix. This test holds the bead mid-create outright, with no async machinery, so the stop path has one route. Verified against the parent commit's session manager: both subtests fail there with "force shutdown left the <state> session running" and "never asked the provider to stop" — the flaky test's own failure mode, reproduced deterministically — and pass here. Both mid-create states are covered because the stop path reaches them identically and Manager.Kill already treats them as one case. The test also asserts the bead is left mid-create rather than marked suspended, so a future change that "tidies" the carve-out into a real lifecycle transition trips here rather than silently inventing a state the session never had. * docs(session): tighten the mid-create carve-out comment to match its neighbour (gc-04375) Readability pass on the comment this branch added two commits ago. It had grown to 31 lines of comment over 8 lines of code by restating the race narrative that the commit message and specs/gc-04375/root-cause.md already carry in full. Now 24 lines, which is the density of the failed-create carve-out immediately above it, with every distinct fact kept: why Kill's stance applies to these states, why the bead is deliberately left mid-create, and why a teardown failure is reported here when failed-create discards it. No behaviour change; the conformance tests are unchanged and still pass. * fix(session): route mid-create stops to kill instead of loosening suspend (gc-04375) The pre-open signoff on this branch (review bead gc-vrou8) raised a P1 against its first fix, and the P1 was right. That fix carved start-pending and creating into Manager.Suspend on the pattern of the adjacent failed-create carve-out: tear the runtime down best-effort, leave the bead where it is, return success. But Suspend is a lifecycle operation, and its contract is that the session is now durably paused. A mid-create bead cannot honor that. StateStartPending means the controller reserved an identity and still intends to start it, and the reconciler reads raw start-pending — and pending_create_claim — as a start request (sessionStartRequestedInfo, cmd/gc/session_reconcile.go). Since POST /v0/session/{id}/suspend calls Manager.Suspend directly, the carve-out handed an operator 200 OK for suspending a session that was still queued to start, and the next controller tick launched it again. The reviewer's other suggested shape — writing a durable cancellation inside Suspend — is not available either: creating means a provider Start call is in flight, so clearing pending_create_* underneath it races that create's own commit and rollback, both of which the reconciler owns. So the routing moves instead of the semantics. What force shutdown needs from a mid-create session is a teardown, not a suspension, and that lever already exists: Manager.Kill explicitly accepts both mid-create states and stops the runtime without touching the persisted lifecycle — exactly the "leave the bead for the reconciler to reap" property the first fix wanted, without lying about a lifecycle. stopTargetThroughWorkerBoundary now routes mid-create targets there, ahead of its suspend fallback, and Suspend goes back to reporting the conflict (409 at the API, unchanged from main). internal/session/manager.go is now comment-only against origin/main. The new branch tolerates an already-gone runtime by the same rule Suspend applies on its own active path: judge by whether the provider reported a live process before the teardown, not by the shape of the error. A start-pending bead routinely has no runtime at all and `gc stop` reaches every session bead with no state pre-filter, so without it every session that had not yet reached its provider start would report a stop failure. A runtime that was live and refused to die still surfaces — swallowing that would report a clean teardown to the stop sweep, which then tears the provider server down believing the fleet is drained. Blast radius is one call site: workerStopSessionTargetWithConfig has exactly one non-test caller, so every gc stop / gc suspend / supervisor shutdown and restart funnels through the function that changed. Non-mid-create sessions take the same suspend path as before. Tests. The reviewer also noted the previous tests seeded a running provider even for the start-pending subcase, which is that state's *un*usual shape: - internal/session: SuspendRejectsMidCreate replaces the two carve-out tests and pins that a rejected transition leaves no trace at all (no sp.Stop, no metadata write, pending_create_claim intact); KillTearsDownMidCreateRuntime pins the teardown half; KillMidCreateWithNoRuntimeSucceeds covers the no-runtime shape the reviewer asked for. - cmd/gc: TestStopTargetThroughWorkerBoundaryRoutesMidCreateToKill covers the routing decision across both states x runtime-present/absent; ...MidCreateReportsOnlyLiveTeardownFailures covers both directions of the tolerance; ...StillSuspendsActive pins that ordinary sessions still suspend. The no-runtime coverage lives here rather than in the force-shutdown test because force shutdown only stops what ListRunning reports, so a runtime-less bead never becomes a stop target on that path — adding the subcase there would have asserted nothing. - TestCityRuntimeForceShutdownStopsMidCreateSession keeps its two states and gains assertions that no stop failure is logged and that pending_create_claim survives. Verified by mutation: all four routing subtests fail with the new branch disabled, with the exact pre-fix symptom (illegal transition ... does not accept "suspend", runtime left running); and each tolerance arm fails when the tolerance is removed or made unconditional. go vet ./... clean. specs/gc-04375/root-cause.md records why the first fix was wrong, since the reasoning is the durable part.
…z) (#155) `gc order history <name>` was store-complete only with `--limit 0`. Any positive `--limit` -- including the default 50, and including a limit far larger than the number of retained runs -- answered from a single store while still rendering a RIG column, so a one-rig answer was indistinguishable from a city-wide one. Root cause is in the routing, not in the read. routeOrderHistory sends a single-order bounded query to the supervisor API, and that request carries one `scoped_name`. But a rig-scoped order is registered once per importing rig, so a bare name with no `--rig` names N registrations at once. orderScopedName resolves it through findOrder, which returns the FIRST match -- so the API was asked about one rig and the other N-1 were dropped silently. Because the city store is the mayor rig's store, the answer was always that rig, which reads as "the one rig that is working" rather than "the only rig I looked at". That under-report has already cost a P1: gc-toolkit bead tk-fdstg was filed at severity 1 reporting that the refinery-reconcile order had never fired on gc-toolkit, when it had in fact been firing in lockstep with the other three rigs since the order's first tick. The reporter explicitly flagged `gc order history` as unusable there -- it returned only gascity rows even at `--limit 40` -- and still reached the opposite of the truth, because no reachable surface would answer per-rig. The fix stays on the local iterator whenever the name resolves to more than one registration, alongside the existing multi-order and unlimited fallbacks and using the same `logRoute(... "fallback", ...)` idiom. That iterator was already correct: it walks every matching registration, merges newest-first, and only then applies the bound, so `--limit N` means "the N most recent runs in the city" rather than "the N most recent runs in whichever store I read first". No change was needed in the read path itself. Deliberately narrow. A rig-qualified read (`--rig <name>`) still resolves to exactly one registration and keeps the API route, so this does not push every bounded read back onto the slower local scan; a city-scoped order has a single registration and is likewise unaffected. The per-store reads stay bounded by `--limit`, so the fan-out costs N bounded reads, not the unbounded scan the help text warns about. The routing tests pass a nil API client and assert on the route= line that logRoute emits before any request is built, rather than standing up an httptest server. The decision is fully observable from that line, and the untagged http_test_server census is a "cannot grow" ratchet (TESTING.md, Small and Source debt ratchets, ga-80po0c.2.2) -- an earlier draft using two real listeners pushed it to 319 calls / 67 files against a 317 / 66 baseline. Spending that ceiling to observe what stderr already reports would have been a poor trade, so the baseline is left untouched rather than raised. Validation: four new tests in cmd_order_history_store_completeness_test.go. TestRouteOrderHistoryBoundedStaysLocalWhenNameSpansRigs is the regression -- it fails before this change with "bounded read of a name spanning 2 rigs was routed to the API". TestRouteOrderHistoryBoundedUsesAPIWhenRigQualified guards the other side so the API route is not lost. The remaining two pin the bead's stated acceptance: with a limit larger than the total row count every rig is represented, and with a limit smaller than it the rows kept are the newest across all stores rather than the newest of one. Existing `-run Order` (11.5s) and `-run Doctor` (67.2s) suites in cmd/gc stay green, as do go build ./..., go vet, and ./internal/testpolicy/... (the census). Not changed, noted for follow-up: `--since` is applied after the fetch rather than pushed into the per-store query (`--rig` genuinely is pushed down, by filtering registrations before any store is opened). With the fan-out corrected, pushing `--since` down is what would keep an unbounded-shaped read cheap; that is a separate performance change and carries its own risk, so it is left out of this fix. Co-authored-by: refinery costing <refinery@local>
…king the alias unreachable (gc-h4s93) (#154) * fix(session): recognize unique alias owner as canonical (gc-h4s93) Cherry-pick of upstream gastownhall/gascity 08461bb (PR gastownhall#5487, Jim Wordelman, 2026-08-21). Fixes gc-h4s93: `gc session wake`/`gc session nudge` could not address a live named session by its configured name, failing with a self-referential error in which the same alias appeared on both sides of "conflicts with": configured named session conflict: "gascity/gc-toolkit.refinery" conflicts with configured named session "gascity/gc-toolkit.refinery" via live bead lx-fp9dj The resolver found the correct, unique, live session bead for the alias and then counted that match as a competing claimant rather than as the answer. Why a cherry-pick rather than a fork-authored fix: upstream had already shipped this exact repair, and standing operator policy on this subject prefers importing an existing upstream commit over writing our own for the same defect -- it carries near-zero conflict cost at the next rebase. Both `internal/session/named_config.go` and its test file were verified byte-identical to the upstream commit's parent before the pick, so this applied with zero conflicts and no fork-side adaptation. The change adds an alias-based canonical pass to both FindCanonicalNamedSessionBead (bead-backed CLI lookup) and FindCanonicalNamedSessionInfo (Info-shaped API lookup), so the two paths stay symmetric. The pass is deliberately narrow: it still requires the existing liveness and continuity checks plus a matching named-session template/spec, and it promotes a candidate only when exactly one template-matching alias candidate qualifies. That uniqueness requirement is what keeps a genuine collision -- two distinct live beads that both match the backing template -- falling through to conflict detection instead of one silently winning on first-match, and it keeps a reserved-name decoy with no corroborating template metadata from being promoted. Note that this defect was latent rather than cured in our fork: it reproduces only when a session is reachable solely through its configured alias, with no corroborating canonical metadata. A live session not in that state resolves fine today, which is why a spot check on 2026-08-22 could not reproduce it. The code path was unchanged in our fork (zero fork commits on every file emitting the conflict message), so the bug was still present. Validation: the commit's five regression tests were run both with and without the source change. With the fix, all five pass. With `internal/session/named_config.go` reverted to the base revision and the new tests kept, the three single-live-session cases fail -- TestFindCanonicalNamedSessionBead_AliasSoleLiveCandidateIsCanonical, TestFindCanonicalNamedSessionInfo_AliasSoleLiveCandidateIsCanonical and TestLookupConfiguredNamedSession_AliasOnlyLiveBeadResolvesCanonical -- confirming the tests genuinely cover the reported defect and the fix is not inert. The two true-collision guards pass in both states, as they should: they exist to prove the new pass does not over-reach. Co-authored-by: investigator <investigator@gascity.local> (cherry picked from commit 08461bb) * docs(release-gates): drop hard-break whitespace that failed the gate's own diff check (gc-sx5cq) Pre-open signoff (review bead gc-adxqr) raised one P1: lines 3-7 of the ga-rcroz7 release-gate record ended in Markdown hard-break double-spaces, so `git diff --check origin/main...HEAD` failed on the very document that records that check as PASS. The gate record was falsifying its own evidence -- criterion 5 ("Final branch is clean") and the diff_lane entry both assert the check passes, and on this branch it did not. Stripped the trailing whitespace on those five lines. Nothing else in the file or the branch changes, and no gate claim is weakened: the fix makes the recorded evidence TRUE rather than restating it. Re-ran the check on the committed tree to confirm. Chose stripping over rewriting the block as a Markdown list because that is the repository convention -- every sibling document in release-gates/ (e.g. concrete-identity-claims-gate.md, doctor-backlog-depth-ready-error-gate.md) writes the same header as plain consecutive metadata lines with no trailing whitespace and no hard breaks. Kept as a separate commit rather than amending 836e928 so the delta since the reviewed OID is visible to the re-gate. --------- Co-authored-by: Jim Wordelman <jim@wordelman.name> Co-authored-by: investigator <investigator@gascity.local> Co-authored-by: refinery costing <refinery@local>
…tests under fleet load; blocks polecat handoff (gc-8jrtx) (#156) * test: make four push-gate flakes deterministic, and correct one root cause (gc-8jrtx) gc-8jrtx is the class gate for ten load-sensitive test beads. Triage found the class is not one problem, so this commit fixes only the four whose mechanism was identified and verified; the rest are classified on their own beads. controller_test.go / path_helpers_test.go (gc-8ors6). The bead diagnosed the failure as "len(base) mod 8", the loop growing aliasName in 8-char steps racing a hard <=100 assertion. That is wrong, and its proposed fixes (overshoot by 1, compute the length arithmetically) would have been no-ops: the asserted canonical path resolves the symlink away, so it never contains aliasName at all. The real precondition is a hard threshold on base alone -- canonical is base + "/city/.gc/controller.sock", so the fixture is constructible only when len(base) <= 75. Reproduced the reported 102 exactly from the recorded path. shortSocketTempDir guarantees a short root only on macOS; on Linux it inherits $TMPDIR, and the gate sets a long one. New shortSocketTempDirWithinLimit takes the reserve the caller needs and falls back to /tmp -- the short root the macOS branch and production's own controllerSocketPath fallback already use. Verified: reverting just this call site fails at 0.00s under the recorded TMPDIR (123 > 100); with the fix it PASSes (not skips) 5/5 there and 5/5 under a short TMPDIR. cmd_events_test.go (gc-b3g52). doEventsWatch's argument is a whole-watch deadline. Seven call sites expect an early return -- a buffered-replay match or a rejected scope -- so their 50ms literal only had to cover scheduling, an httptest handler and a loopback hop, and a loaded shard defeated it as "context deadline exceeded" rather than as any assertion about behaviour. They now share eventsWatchTestDeadline, sized never to fire; the call still returns on the match, so nothing gets slower. The eighth site is TestDoEventsWatchTimesOutWithoutMatch, where expiry IS the behaviour under test: it keeps its short value under eventsWatchTestExpiryDeadline, named so the next test copies the right one. cmd_supervisor_test.go (gc-nnl64). The success-path test pinned supervisorReadyTimeout to 25ms while its hook reports ready on the 4th poll of a 1ms ticker -- four scheduler turns, which a loaded shard does not guarantee inside 25ms. waitForSupervisorPID returns the moment the hook answers, so a generous deadline costs nothing on this path, and the 25ms never proved the timeout hook was used anyway (the default 15s would pass identically). The timeout's own behaviour stays covered by the zero-timeout sibling test. checks_custom_types_test.go (gc-i344n). home := t.TempDir() lacked the retryRemoveAllForTest guard its sibling dir already had, and home is where bd writes ~/.beads -- so TempDir's own RemoveAll raced a still-exiting bd/dolt child and reported "directory not empty", which testing counts as a FAILURE even for a run that correctly skipped. Confirmed physically: /var/tmp/rp holds 20 leaked dirs, every one of them .../001/.beads (home), never .../002 (the guarded dir). Both tests in the file have the shape and both leak, so both are fixed. Cleanups run LIFO, so the retry is registered right after t.TempDir() to sit immediately ahead of the removal it drains for. Validation: go vet clean on both packages; the targeted cmd/gc tests pass -count=3; internal/doctor passes in full. The doctor race itself is not reproducible on this host (bd is a CGO_ENABLED=0 build, so both drift tests skip) -- the evidence there is the asymmetry plus the leaked-directory census. * test(doctor): give TestRunCheckTimeoutBoundsFix a load-independent margin (gc-cr6lj) The test pinned CheckTimeout to 25ms and needed that single value to separate two very different things: an initial Run that returns immediately, and a Fix that never returns at all. Doctor.Run races the check goroutine's first scheduling against time.After(CheckTimeout), so under a loaded parallel shard the immediate Run can lose that race. The fast initial failure is then classified "timed out", the fix path is skipped entirely, and the test fails on fixCalls = 0 -- an assertion about whether the host was idle, not about the timeout bounding a wedged remediation. Raising CheckTimeout to 2s widens that separation by ~80x while preserving exactly what the test proves: the wedged Fix blocks forever by construction, so it still reaches the timeout and still yields the unconfirmed-remediation result the assertions check. The elapsed guard moves to 30s for the same reason it existed -- it catches an unbounded wedge, and a bounded run now costs about CheckTimeout, so a 2s guard would have been measuring the fix it was meant to tolerate. Chose the wider margin over a clock seam deliberately: injecting a clock into Doctor is a production change made solely for a test, and the test does not need to observe time, only to not be defeated by it. Cost is ~2s of wall clock in a package that already takes 16s. Validation: passes -count=3 here and -count=3 under an added CPU load that took the host to load 41 (the reported failures were at load 15-27). Noting honestly that the old 25ms value ALSO passed under that synthetic load -- a tight-loop CPU hog is not the gate's profile of parallel go test processes doing I/O, allocation and process spawning, so this is not an on-demand reproduction. The change is justified by the mechanism above, and it is strictly margin-widening on assertions that do not test timing. * docs(specs): record the gc-8jrtx flake-class triage and per-bead verdicts (gc-8jrtx) gc-8jrtx is the class gate for ten load-sensitive test beads, nine of them parked behind it by dependency edges. Its question was whether they are one load-isolation problem or N independent races, with the explicit warning that "park them all as load noise" risks burying a real per-test race -- as gc-04375 proved when it turned out to be a product bug, not load noise. Answer: N. At least six distinct mechanisms across ten beads, only three of which share a fix. Two are not load-sensitive at all, one had already received the class fix nine days before the observation used to file it and failed anyway, and one filed root cause was specific, plausible and wrong in a way that would have shipped a no-op. This records the per-bead verdict and its evidence so the un-park returns each bead with a diagnosis rather than a re-run. Filed under specs/ rather than docs/ because it is a record of what was decided on this bead, not an authoritative statement of current behaviour. * style(doctor): separate the two rationales on the pinned test HOME (gc-8jrtx) Self-review touch-up. The gc-i344n cleanup-guard rationale landed immediately after the existing HOME-pinning rationale with no separator, so two unrelated explanations read as one paragraph. Blank comment line between them; no code change. * test(cmd/gc): rebase the hang-deadline exclusion lines after the gc-8ors6 comment (gc-8jrtx) controllerTestExcludedHangDeadlineLines is keyed by LINE NUMBER, so the nine-line comment this branch added to controller_test.go at ~line 308 moved all four documented exclusions down by exactly nine: 421->430, 877->886, 927->936, 1456->1465. Both halves of the lint then failed -- the offender scan saw four unexcluded raw deadlines, and the staleness guard saw four exclusions that no longer matched. The four sites are unchanged and still correct as they stand per TESTING.md:1364-1371; only their coordinates moved. Rebased the keys by +9 and confirmed the content at each new line is the same raw-literal deadline the reason text describes. Caught by the local gate run (unit-cmd-gc-3-of-6), not by CI. Filed gc-h915a for the underlying brittleness: any edit anywhere above line 1456 of controller_test.go breaks this lint, which makes it a tripwire on unrelated work rather than on the invariant it means to pin. --------- Co-authored-by: refinery costing <refinery@local>
…cope (gc-gfoc7) (#158) * fix(dispatch): close the scope-check control bead before converging its scope (gc-gfoc7) A graph.v2 workflow never finished. Its final step's scope-check bead stayed open after its blocker closed, so the body scope stayed open, so cleanup-worktree and workflow-finalize were never ready and the worktree was never cleaned. This is the long-standing graph.v2 husk accumulation. Root cause is an ordering cycle, not a discovery failure. The scope body carries a "blocks" dependency on every step's scope-check control bead (the graph builder's rewriteGraphStepRefs redirects downstream refs from the step to its scope-check, and the body is one of those downstream refs). The final scope-check is therefore simultaneously the bead that closes the body and the last blocker standing in the body's way. processScopeCheck converged the scope first and closed itself second, so the body close was refused with "cannot close blocked issue: <body> is blocked by [<control>]", the error propagated before the control bead was ever closed, and the dispatcher classified it transient and retried forever. One workflow logged ~240k such failures. Only the final step of a scope showed the symptom: for every earlier step hasOpenScopeMembers still reports members open, which takes the "continue" branch that just closes the control bead and never touches the body. That is why 4 of 5 scope-checks in a molecule closed normally and only submit hung — and why the closed-scope-check counts skewed 6/6/6/6 against 4 for submit. The fix closes the control bead before calling closeScopeAsPassed/abortScope in all three terminal branches, which is already the pattern every other terminal path in the dispatcher uses (close self, then reconcile the enclosing scope — see processRetryControl and recordControllerSpawnError). Doc comments on closeScopeAsPassed and abortScope now state the precondition. Verified upstream/main carries the identical ordering in both scope-pass branches, so this is an upstream defect rather than fork-local; the bead is tagged upstream_pr_candidate. Tests: the deadlock escaped CI because internal/dispatch tests run against MemStore, which closes a blocked bead happily. strictCloseStore already existed to mirror bd's guard but was referenced by zero tests, and it only guarded Close() while updateMetadataAndClose closes via Update(Status). Both gaps are fixed: strictCloseStore now guards the update path too, and a new TestProcessScopeCheckClosesScopeWhenBodyBlocksOnControl runs the same graph as the existing success test against it. That test fails on the pre-fix tree with the exact production error string and passes after. TestProcessScopeCheckKeepsControlOpenIfBodyCloseoutFails asserted the opposite ordering — control stays open "so the dispatcher can retry body closeout". That invariant is unsatisfiable, since holding the control bead open is precisely what makes every retry fail; it is renamed to TestProcessScopeCheckSurfacesBodyCloseoutFailure and now pins that a genuine closeout failure is surfaced rather than retried forever. Follow-up not taken here: if body convergence fails after the control bead closes, nothing re-drives it (previously it retried, though it could never succeed). Surfacing the error is strictly better than the infinite spin, but a durable convergence sweep for scope bodies whose members are all closed is worth filing separately. * docs(dispatch): state abortScope's actual control-bead precondition (gc-gfoc7) Self-review catch on the ordering fix. The doc comment added to abortScope claimed the caller must always have closed its own control bead first, matching closeScopeAsPassed. That overstates the requirement and misdescribes the second caller. abortScope begins with skipOpenScopeMembers, which closes every open scope member that is not body/teardown/spec — open scope-check controls included — in dependency order. That pass is what unblocks the body. The single bead exempt from it is the one named by traceID (skipControlID), so only that bead is the caller's responsibility. This matters for reconcileTerminalScopedMember, which reaches abortScope on its failure path without any remaining-open check. Under the overstated wording that path looks broken; it is in fact correct, because the sibling controls it does not own are closed by the skip pass. No behavior change — comment only. * docs(specs): record the gc-gfoc7 scope-check deadlock investigation (gc-gfoc7) The root-cause analysis for this bead was living only in bead notes, which docs/file-structure.md (gc-toolkit pack) explicitly disallows for durable documents — bead comments are operational state, not the record, and this one needs to be readable after the bead closes. Files it as specs/gc-gfoc7/ per the bead-keyed local tier: it records what was found and decided while working this bead, not an authoritative "what is true now" topic that someone owns keeping current. Worth keeping rather than summarizing, because two of the three findings are about how the investigation goes wrong: - The two hypotheses that had to be ruled out first (discovery, metadata shape), including the bare-bd-vs-gc-bd store-resolution red herring that made the discovery theory look confirmed. - Why CI could not catch it: the fake store is more permissive than bd, the existing success test already builds the exact deadlock graph, and the strictCloseStore written to mirror bd's guard was referenced by zero tests and guarded the wrong method. - The counting trap: scope-check beads are absent from bd list entirely, so sizing this class by listing returns a false zero. Also records the two framings the evidence disproves (the retry invariant asserted by the old test, and gc-spa04's "blocked-by-design"), so neither gets re-adopted from the bead titles alone. --------- Co-authored-by: refinery costing <refinery@local>
…d's stderr — 7.3% of all tool-result text city-wide (gc-dqn8l) (#157) * fix(cli): stop reprinting the always+fresh advisory on every command (gc-dqn8l) The always+fresh named_session advisory is a config lint: it reports a static property of city.toml that cannot change between invocations. It was nonetheless emitted from shouldEmitLoadCityConfigWarning, which sits on the shared loadCityConfigFS path that nearly every gc command takes, so the same 7-line block was reprinted on the stderr of `gc bd show`, `gc bd list`, `gc hook --claim`, and the rest. That is not free. Agent harnesses merge stderr into the tool result, so the block was measured at 1.6M tokens across 3,925 of 34,677 tool results in a trailing 24h — 7.3% of ALL tool-result text city-wide — and each copy then sits in the agent's context for the remainder of the session, re-read on every subsequent request. stderr is no defence for the same reason, and the block is a known jq-breaker on the `--json` call sites that do not thread configWarnWriter through. Suppress it in shouldEmitLoadCityConfigWarning, exactly mirroring the IsLegacyWorkspaceFieldWarning precedent immediately above it. This is a print-site change only — nothing about classification moves: - strict mode still treats it as non-fatal via strictWarningIsNonFatal, so `gc start --foreground`/`--controller`/`--dry-run` still exits 0 on the shipped example city; - config.ValidateNamedSessions still produces it, so it remains in prov.Warnings; - `gc start` and `gc config` both print raw prov.Warnings without consulting this filter, so the advisory stays fully discoverable on the surfaces whose subject IS the config. Suppression rather than a per-process dedup because every gc invocation is a fresh process — the sync.Map dedup in emitSupervisorLoadCityConfigWarnings only helps the long-lived supervisor. Validation: TestAlwaysFreshWakeModeWarningIsNonFatalAndUnprinted (renamed from ...AndEmitted, its assertion inverted) pins both halves of the new contract; TestEmitLoadCityConfigWarningsFiltersNonMigrationWarnings gains the advisory as an input and asserts it is filtered. Both were written first and observed failing. Measured end-to-end against the live city with a patched binary: `gc bd show` and `gc bd list` go from 7 advisory lines on stderr to 0, while `gc config show` still prints all 7. go vet clean on cmd/gc and internal/config. * fix(cli): route gc github pr backfill warnings through the shared filter (gc-nmd11) Pre-open signoff on polecat/gc-dqn8l (review bead gc-315wv) found the branch left one reachable command path still emitting the advisory it set out to remove. gc-dqn8l suppressed the always+fresh named_session notice in shouldEmitLoadCityConfigWarning, which the shared emitLoadCityConfigWarnings path consults. But doGitHubPRBackfill loads config via loadConfigCommandCityConfig and then iterated prov.Warnings itself, printing every entry raw — so in any city with an always+fresh named session, `gc github pr backfill` still reprinted the block on stderr before it reached GitHub or token handling. Replace that raw loop with the same emitLoadCityConfigWarnings / configWarnWriter pair cmd_sling.go, cmd_convoy.go, and cmd_rig.go already use. This command's subject is GitHub PR readiness, not the config, so it belongs on the filtered side of the split: actionable migration guidance still prints, static city.toml lints stay quiet, and the emitter's dedup drops the repeated copies the raw loop printed (the added fixture trips the agent_defaults/agents ambiguity warning twice). configWarnWriter also subsumes the hand-rolled `if !opts.jsonOutput` guard, preserving JSON-mode silence. The explicit config surfaces are deliberately untouched: cmd_start.go:745 and cmd_config.go:902 still print prov.Warnings unfiltered, so the advisory stays discoverable exactly where the config IS the subject. cmd_supervisor.go already consults the same filter via emitSupervisorLoadCityConfigWarnings. Also corrects the IsAlwaysFreshWakeModeWarning doc comment, which still claimed CLI filters use the marker "to print the notice" — collateral of gc-dqn8l inverting that behavior. Validation: new TestGitHubPRBackfillSuppressesAlwaysFreshAdvisory drives the real command through run() against a city carrying both a suppressed warning (always+fresh) and a kept one (both [agent_defaults] and [agents] present), so it fails against a blanket mute as well as against the raw loop. Verified failing at the reviewed commit 7525125 with the advisory present, passing after. `go test ./cmd/gc -run 'TestGitHubPR|TestEmitLoadCityConfigWarnings| TestAlwaysFreshWakeMode'` and `go test ./internal/config` pass; `go vet ./cmd/gc/... ./internal/config/...` clean. * test(cli): make the backfill advisory test non-vacuous and marker-robust (gc-nmd11) Self-review hardening of the test added in the previous commit. As first written it asserted absence by searching stderr for a copy of the advisory's message text, which fails open in two ways: if the fixture ever stopped provoking the advisory, or if the validator reworded it, the assertion would still pass while proving nothing. strict_warnings_test.go already avoids the second trap by deriving the warning from config.ValidateNamedSessions instead of hardcoding it; this brings the new test to the same standard. Two guards. The test now loads the fixture city through loadConfigCommandCityConfig up front and fails unless prov.Warnings actually contains an always+fresh advisory, so a validator change surfaces as a failure here rather than as a silently empty test. And it classifies stderr lines with config.IsAlwaysFreshWakeModeWarning — the same exported predicate the fix consults — so the assertion tracks the marker rather than a copy of the prose. Verified by inverting the fix: with cmd_github.go restored to the raw prov.Warnings loop at 7525125 the test fails on the advisory, and passes again once the fix is back. --------- Co-authored-by: refinery costing <refinery@local>
bd 1.2.1 added a cross-era guard that classifies a workspace from its
on-disk shape before `bd init` does anything, and refuses what it cannot
place. A fresh gc-managed city is exactly the shape it refuses: the
managed Dolt server's data root IS `<scope>/.beads/dolt`, and the server
is up before the first `bd init` runs, so every managed init hit
Error: legacy Dolt server workspace detected; explicit migration is
required before this bd version can open or modify the workspace.
on a workspace that had been created seconds earlier and had nothing to
migrate. Neither --force nor --reinit-local bypasses it — the guard runs
ahead of init's existing-workspace checks — and it never reads the
--server flag, only `.beads/metadata.json` off disk.
Two distinct shapes reached the refusal, so there are two fixes:
1. gc-driven init already writes `dolt_mode: server`, but bd has never
run in the scope so there is no `.beads/.local_version` witness to
date it, and the guard refuses an undated server workspace with a
local Dolt root. `normalizeCanonicalBdScopeFilesForInit` now seeds
that witness with the version of the bd binary it is about to drive.
2. A direct `gc-beads-bd init` writes no metadata at all, so the guard
sees an *embedded* workspace with a legacy Dolt root and refuses via
a different branch that no witness can clear. The script's own
comment already asserted "gc's normalizeCanonicalBdScopeFilesForInit
writes metadata.json BEFORE invoking us" — an invariant it depended
on but never established. op_init now establishes it.
The witness write is deliberately narrow: only for the shape bd refuses,
only when no witness exists (a pre-1.0 value is a real migration signal
and must keep reaching the operator), and only the probed version of the
bd about to run, so bd's own upgrade detection stays correct.
Also switches the managed reinit off the deprecated --force onto
--reinit-local, which bd now asks for and which the doltlite path
already used; that spelling was the DeprecationWarning printed
immediately before every one of these failures. The gc-beads-bd harness
tests that pin the init argv move with it, as do three comments that
named the old flag.
Fixes the three CI jobs filed on this bead and the twelve tests on its
canonical duplicate gc-1kqz0 — one shared-setup defect, not fifteen.
Verified green locally (all previously failing, all now passing):
cmd/gc TestFreshManagedBdCityInitSeedsPinnedHQDatabase...
cmd/gc (integration) TestManagedBdRigProviderStoreRecoversAfterHardKill...
test/integration TestGraphWorkflowSuccessPath
TestGraphWorkflowFailureRunsCleanup
TestAdoptPRFormulaCompileAndRun
TestPersonalWorkFormulaCompileAndRun
TestRetryManagedPooledWorkerRecoversClaimedAttempt...
TestInitBdAllowsStandaloneCreate
TestCleanInstallTutorialPath
TestGCLiveContract_BeadsAndEvents
TestHumaBinary_CityCreateAsync
TestHumaBinary_SessionMessageAsync
Diagnosis, the guard's full decision table, and what to re-check on the
next bd bump: specs/gc-sc8a8/bd-legacy-workspace-gate.md
Sibling from the same bd bump, not fixed here: gc-zfh0w (internal/bdflags
manifest stale against 1.2.1).
zook-bot
commented
Aug 25, 2026
zook-bot
left a comment
zook-bot
left a comment
Author
There was a problem hiding this comment.
VERDICT: REQUEST_CHANGES
Reviewed branch: polecat/gc-sc8a8
Reviewed base: main
Reviewed commit: 4901912
Scope checked: full PR diff for cmd/gc bd init/lifecycle changes, internal/beads/contract witness helper, examples/bd/assets/scripts/gc-beads-bd.sh, the new spec, and the changed tests. I also exercised the metadata-only direct-provider shape with real bd v1.2.1 and a fake Dolt SQL client to isolate bd's pre-init guard.
Findings:
P1 — examples/bd/assets/scripts/gc-beads-bd.sh:2799 skips the normalizer whenever metadata.json already says dolt_mode=server, so metadata-only server scopes still reach real bd init without .beads/.local_version. That is a reachable path: TestGcBeadsBdInitMetadataOnlyFallsThroughToForcedBdInitWithPinnedDatabaseWhenSchemaMissing sets up exactly this metadata-only server scope, and the script then runs bd init --reinit-local. With real bd v1.2.1, that still fails before reinit with legacy Dolt server workspace detected because no witness was written. I reproduced this at the reviewed commit with .beads/metadata.json naming dolt_mode=server, a real .beads/dolt/hq/.dolt directory, and fake Dolt responses for SELECT 1 / schema-missing SQL; the script invoked bd init --reinit-local ... and exited with the same legacy-server refusal, leaving witness_exists=no. This means the direct metadata-only/schema-repair path remains broken under the bd 1.2.1 guard even though the Go-driven fresh-init path is fixed. Fix by seeding/normalizing before any managed bd init when the witness is absent for a server-mode local Dolt root, not only when dolt_mode is not already server; the metadata-only test should also assert the witness or use a real-bd guard fixture so fake bd cannot mask this.
Verification:
- PASS:
go test ./internal/beads/contract -run 'Test(LocalVersionWitness|ReadLocalVersionWitness|EnsureLocalVersionWitness)' - PASS:
go test ./cmd/gc -run 'Test(EnsureManagedScopeVersionWitness|NormalizeCanonicalBdScopeFilesForInitSeedsVersionWitness|GcBeadsBdInitMetadataOnlyFallsThroughToForcedBdInitWithPinnedDatabaseWhenSchemaMissing|GcBeadsBdInitRetriesPlainInitWhenSchemaStillMissingAfterSuccess|GCBeadsBDScript_InitForcesReinitOverPreSeededMetadata|ServerReachableReflectsDoltExit)$'withGOTMPDIRmoved off/tmp - PASS:
GC_FAST_UNIT=0 go test ./cmd/gc -run '^TestFreshManagedBdCityInitSeedsPinnedHQDatabaseAndKeepsGCPrefix$' -count=1 - PASS:
GC_FAST_UNIT=0 go test -tags integration ./cmd/gc -run '^TestManagedBdRigProviderStoreRecoversAfterHardKillPortRebind$' -count=1 -v(224.267s; expected MySQL EOF/reset noise during forced rebind) - PASS:
GC_FAST_UNIT=0 go test -tags integration ./test/integration -run '^TestGraphWorkflowSuccessPath$' -count=1 -v(185.094s; cleanup noted supervisor already stopped)
Not run: full make test-fast-parallel, go vet ./..., or the full integration shard matrix during this signoff.
zook-bot
commented
Aug 26, 2026
zook-bot
left a comment
zook-bot
left a comment
Author
There was a problem hiding this comment.
VERDICT: request-changes
Reviewed branch: polecat/gc-sc8a8
Reviewed base: main
Reviewed commit: 4901912
Scope checked: anchor bead gc-sc8a8 and prior review context; branch topology; the focused tip commit at 4901912, including cmd/gc bd init lifecycle changes, internal/beads/contract witness helpers, examples/bd/assets/scripts/gc-beads-bd.sh, the new spec, and the changed tests. Operator profile checked. The raw origin/main...4901912 diff currently includes 269 files because the branch is stale against the rebased main; I did not re-review every older carried commit.
Findings:
P1 - examples/bd/assets/scripts/gc-beads-bd.sh:2799 still skips the normalizer when metadata.json already says dolt_mode=server. That leaves a reachable server-mode scope with a real .beads/dolt directory and no .beads/.local_version witness going straight to bd init --reinit-local. bd v1.2.1 still refuses that shape before init, so the schema-repair path remains broken even after this branch. I reproduced it at the reviewed commit with temp-installed bd v1.2.1, metadata.json naming dolt_mode=server/dolt_database=hq, a real .beads/dolt root, fake Dolt responses for reachable database plus missing config schema, and no witness; gc-beads-bd invoked bd init --reinit-local and exited with "legacy Dolt server workspace detected", leaving witness_exists=no. The existing harness at cmd/gc/beads_provider_lifecycle_test.go:7225 does not catch this because it creates metadata.json but no .beads/dolt root and uses a fake bd that only checks argv. Fix by seeding the witness before any managed bd init whenever server-mode metadata plus a real local Dolt root lacks .local_version, even if metadata already says server; then add a test that models the real refused shape or asserts the witness before the fake bd path can mask it.
Verification:
PASS: GOTMPDIR=$(mktemp -d "$PWD/.gotmp-contract.XXXXXX") go test ./internal/beads/contract -run 'Test(LocalVersionWitnessPath|ReadLocalVersionWitness|EnsureLocalVersionWitness)' -count=1
PASS: GOTMPDIR=$(mktemp -d "$PWD/.gotmp-cmdgc.XXXXXX") go test ./cmd/gc -run 'Test(EnsureManagedScopeVersionWitness|NormalizeCanonicalBdScopeFilesForInitSeedsVersionWitness|GcBeadsBdInitMetadataOnlyFallsThroughToForcedBdInitWithPinnedDatabaseWhenSchemaMissing|GcBeadsBdInitRetriesPlainInitWhenSchemaStillMissingAfterSuccess|GCBeadsBDScript_InitForcesReinitOverPreSeededMetadata|ServerReachableReflectsDoltExit)$' -count=1
FAIL: manual gc-beads-bd init repro with temp-installed bd v1.2.1 and fake Dolt, at 4901912; rc=1, witness_exists=no, output contains "legacy Dolt server workspace detected".
INFO: direct bd v1.2.1 init --reinit-local over the same no-witness server-mode filesystem shape also refuses before contacting a server.
Not run: full make test-fast-parallel, go vet ./..., or the full integration shard matrix during this signoff.
Anchor: gc-sc8a8 — check.codex @ 4901912
zook-bot
commented
Aug 26, 2026
zook-bot
left a comment
zook-bot
left a comment
Author
There was a problem hiding this comment.
VERDICT: request-changes
Reviewed branch: polecat/gc-sc8a8
Reviewed base: main
Reviewed commit: 4901912
Scope checked: anchor bead gc-sc8a8, review bead gc-4khn5, prior request-changes context, PR #161 metadata, operator profile, and the branch-owned tip commit at 4901912. I read the changed cmd/gc bd init lifecycle code, internal/beads/contract witness helpers, examples/bd/assets/scripts/gc-beads-bd.sh, specs/gc-sc8a8/bd-legacy-workspace-gate.md, and the changed tests. The raw origin/main...4901912 diff currently includes unrelated carried history because the branch is stale against the rebased main, and GitHub refuses the PR diff as larger than 20,000 lines; I did not re-review every older carried commit. PR #161 is also currently marked CONFLICTING against main.
Findings:
P1: examples/bd/assets/scripts/gc-beads-bd.sh:2799 still skips canonical normalization whenever metadata.json already says dolt_mode=server. That leaves a reachable server-mode scope with a real .beads/dolt directory and no .beads/.local_version witness going straight to bd init --reinit-local. bd still refuses that shape before init, so the direct metadata-only schema-repair path remains broken even though the Go-driven fresh-init path is fixed. I reproduced this at the reviewed commit with host bd 1.2.2, metadata.json naming dolt_mode=server and dolt_database=hq, a real .beads/dolt/hq/.dolt directory, fake Dolt responses for SELECT 1, USE hq, CREATE DATABASE, and missing config schema, and no witness. gc-beads-bd exited 1 with "legacy Dolt server workspace detected" and left witness_exists=no. The existing harness at cmd/gc/beads_provider_lifecycle_test.go:7225 does not catch it because it creates metadata.json but no .beads/dolt root, and it uses a fake bd that checks argv instead of bd's filesystem guard. Fix by seeding the witness before any managed bd init when server-mode metadata plus a real local Dolt root lacks .local_version, even if metadata already says server. Add a test that models that refused shape or asserts the witness before the fake bd path can mask it.
Verification:
PASS: detached worktree at 4901912, GOTMPDIR=/var/tmp/gc-review-gotmp-gc-4khn5.onEeeZ go test ./internal/beads/contract -run 'Test(LocalVersionWitness|ReadLocalVersionWitness|EnsureLocalVersionWitness)' -count=1
PASS: detached worktree at 4901912, GOTMPDIR=/var/tmp/gc-review-gotmp-gc-4khn5.onEeeZ go test ./cmd/gc -run 'Test(EnsureManagedScopeVersionWitness|NormalizeCanonicalBdScopeFilesForInitSeedsVersionWitness|GcBeadsBdInitMetadataOnlyFallsThroughToForcedBdInitWithPinnedDatabaseWhenSchemaMissing|GcBeadsBdInitRetriesPlainInitWhenSchemaStillMissingAfterSuccess|GCBeadsBDScript_InitForcesReinitOverPreSeededMetadata|ServerReachableReflectsDoltExit)$' -count=1
FAIL: manual gc-beads-bd init reproduction at 4901912 with host bd 1.2.2 and fake Dolt; rc=1, witness_exists=no, output contains "legacy Dolt server workspace detected".
INFO: gh pr diff for PR #161 was refused by GitHub as too_large over 20,000 lines due branch staleness.
Not run: full make test-fast-parallel, go vet ./..., or the full integration shard matrix during this signoff.
Anchor: gc-sc8a8 — check.codex @ 4901912
zook-bot
commented
Aug 26, 2026
zook-bot
left a comment
zook-bot
left a comment
Author
There was a problem hiding this comment.
VERDICT: request-changes
Reviewed branch: polecat/gc-sc8a8
Reviewed base: main
Reviewed commit: 4901912
Scope checked: Dispatch bead gc-5imu5, anchor bead gc-sc8a8, prior signoff gc-o8zis, the current PR metadata for PR#161, the branch/file diff shape, the managed bd init wrapper path, the bd version witness helper, and the related Go and shell harness tests. I checked the operator profile from the pack checkout. I did not re-review the unrelated branch-stack changes that are only present because this branch is behind current origin/main.
Findings:
P1 - examples/bd/assets/scripts/gc-beads-bd.sh:2799 skips normalization whenever metadata already says dolt_mode=server, so a metadata-only server scope can still reach bd init --reinit-local without .beads/.local_version. That is the exact shape bd refuses: metadata names server mode, .beads/dolt/<db>/.dolt is a real local Dolt root, and there is no witness dating the workspace. I reproduced it at the reviewed commit by running the wrapper with fake Dolt SQL that reports the server reachable but the bd schema missing, and real host bd version 1.2.2; the wrapper exited with legacy Dolt server workspace detected, left witness=missing, and failed bd init failed for <tmpdir>. The harness at cmd/gc/beads_provider_lifecycle_test.go:7225 does not catch this because it uses a fake bd and never creates .beads/dolt/hq/.dolt or asserts .beads/.local_version. Fix by seeding/normalizing before any managed bd init when a server-mode local Dolt root lacks a witness, even if metadata already says server; add coverage that creates the real refused on-disk shape and asserts the witness or exercises the real bd guard.
Verification:
- PASS:
go test ./internal/beads/contract -run 'Test(LocalVersionWitness|ReadLocalVersionWitness|EnsureLocalVersionWitness)' -count=1 - PASS:
go test ./cmd/gc -run 'Test(EnsureManagedScopeVersionWitness|NormalizeCanonicalBdScopeFilesForInitSeedsVersionWitness|GcBeadsBdInitMetadataOnlyFallsThroughToForcedBdInitWithPinnedDatabaseWhenSchemaMissing|GcBeadsBdInitRetriesPlainInitWhenSchemaStillMissingAfterSuccess|GCBeadsBDScript_InitForcesReinitOverPreSeededMetadata|ServerReachableReflectsDoltExit)$' -count=1 - PASS:
GC_FAST_UNIT=0 go test ./cmd/gc -run '^TestFreshManagedBdCityInitSeedsPinnedHQDatabaseAndKeepsGCPrefix$' -count=1 - FAIL as expected: wrapper metadata-only server-scope reproducer with fake Dolt SQL plus real
bd version 1.2.2;bd initrefused the workspace as legacy and.beads/.local_versionwas missing.
Anchor: gc-sc8a8 — check.codex @ 4901912
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.
Summary
bd v1.2.1added a cross-era guard that classifies a workspace from itson-disk shape before
bd initdoes anything, and refuses what it cannotplace. A fresh gc-managed city is exactly the shape it refuses: the managed
Dolt server's data root IS
<scope>/.beads/dolt, and the server is upbefore the first
bd initruns, so every managed init hitlegacy Dolt server workspace detected. Neither--forcenor--reinit-localbypasses it — the guard runs ahead of init's existing-workspacechecks — and it never reads the
--serverflag, only.beads/metadata.jsonoff disk.
Two distinct shapes reached the refusal, so there are two fixes:
dolt_mode: server, but bd has never run inthe scope so there is no
.beads/.local_versionwitness to date it.normalizeCanonicalBdScopeFilesForInitnow seeds that witness with theversion of the bd binary it is about to drive.
gc-beads-bd initwrites no metadata at all, so the guard seesan embedded workspace with a legacy Dolt root. Fixed by establishing the
metadata invariant
op_init's own comment already claimed gc hadestablished.
Also moved the managed reinit off deprecated
--forceonto--reinit-local.Clears the 3 CI jobs tracked on this bead (
gc-sc8a8) plus all 12 tests onthe canonical duplicate
gc-1kqz0(same root cause — one fix for both).Sibling
gc-zfh0w(internal/bdflagsmanifest stale, same bd 1.2.1 bump,different mechanism) is NOT fixed here.
Implementation notes
ROOT CAUSE confirmed by A/B experiment: bd 1.2.1's
guardLegacyUpgradeWorkspaceclassifies a workspace from its on-disk shape before
bd initdoes anything.Diagnosis, the guard's full decision table, and what to re-check on the next
bd bump:
specs/gc-sc8a8/bd-legacy-workspace-gate.md.REGRESSION CAUGHT BY THE GATE (fixed, not waived): the
--force->--reinit-localswitch broke two
gc-beads-bdharness tests that pin the init argv. Both updated,plus one negative assertion and three comments naming the old flag.
GATES:
make test-fast-parallelGREEN (10/10 jobs, RC=0);go build ./...;go vet ./...;go vet -tags integration ./...;gofmtclean. Re-verified freshby refinery before opening this PR.
Refinery handoff
Work bead: gc-sc8a8 (duplicate_of gc-1kqz0 — canonical gc-1kqz0 should close
when this lands). Branch
polecat/gc-sc8a8@490191278, 1-ahead/0-behindmain, no rebase needed.