Merge upstream/main into fork — 2026-08-06 resync (68 commits) - #123
Merged
Conversation
…torage-binding fix(gc): restore storage binding dispatch
…f first-hit (Fixes gastownhall#4746) (gastownhall#4747) Fixes gastownhall#4746 ## Finding fixed `gc hook`'s cross-store discovery (`firstStoreWithWork`) returned on the FIRST store reporting any ready work, with no priority comparison across stores. A city-scoped crew agent's own city store is ordered ahead of any federated rig store, and such an agent essentially always has some city work — so a rig-routed bead, however urgent, stayed invisible behind city work indefinitely. Priority could not rescue it, because priority was never compared across the store boundary. This is a different defect from the one gastownhall#3785/gastownhall#3818 fixed: those handle a bead that discovery FINDS but the claim cannot reach after it is taken between discovery and claim. This bug is upstream of that in the pipeline — the bead is never discovered in the first place, so the claim path is never reached. **Overlap with gastownhall#4322, and how I would like to handle it.** gastownhall#4322 defines exactly the canonical priority order this PR wants, including `nil priority as P2`. It does not close this hole: I checked out its head (`2e2526f63`) and the repro from gastownhall#4746 is still RED there, with `firstStoreWithWork` unchanged. So the two are complementary rather than duplicate — but they will conflict textually in `hook_cross_store.go` and `cmd_hook.go`. **If gastownhall#4322 lands first I will rebase onto it and re-express this ranking in its vocabulary rather than introducing a second one; I would rather that PR own the naming.** Merge them in whichever order suits you. ## Change - `firstStoreWithWork` -> `bestStoreWithWork`: queries every store (instead of short-circuiting on the first hit) and selects the best-ranked ready candidate across all of them. - Rank = (tier, priority), lower wins. Tier mirrors the work_query's three-tier shape (in_progress-assigned / assigned / routed-unassigned), read directly off each row — every store's query already matches this agent's own identity, so an assignee on a row means it is assigned to this agent and no assignee means it is routed-unassigned. This keeps priority compared WITHIN a tier and never across one, so a rig P0 cannot preempt this session's own in-progress crash-recovery bead. - Two deliberate carve-outs: (1) a tier-0 (in_progress) row in the PRIMARY store short-circuits unconditionally — resuming this session's own interrupted work must never be preempted, and this keeps the hot resume path at one query; (2) unrankable output (non-JSON, or JSON that is not an array of objects) degrades the whole call to first-hit, since reordering on a comparison that could not be made would be worse than the prior behavior. - Ties keep the original slice order, so an equally-ranked candidate in the agent's own store still wins — the pre-existing behavior whenever ranking does not discriminate. - Mechanical rename of `firstStoreWithWork` -> `bestStoreWithWork` and its doc comments in `cmd_hook.go`, `cmd_hook_test.go`, `cross_store_pipeline_test.go`. ## Cost, stated plainly First-hit could stop early; ranking cannot. Outside the tier-0-in-primary short-circuit, discovery now runs the work query against **every** store on each call instead of stopping at the first hit — for the common crew agent that is 2 queries where it was 1. That is the price of comparing across the boundary at all, and the short-circuit keeps the hot crash-recovery resume path at a single query. If you would prefer a cheaper shape (e.g. only scanning further when the first hit is worse than some threshold), I am happy to rework it. ## Tests - `gofmt -l cmd/gc/` clean, `go vet ./cmd/gc/` clean, `go build ./cmd/gc/...` succeeds with the repo's ICU CGO flags. - `go test ./cmd/gc/ -run 'BestStoreWithWork|BestHookCandidateRank|FirstStoreWithWork|ClaimHookWork|CrossStorePipeline' -count=1` — **ok**. - FALSIFIABLE FLOOR: the repro test in gastownhall#4746 is RED on stock `af42a9424` (output pasted in the issue) and GREEN with this change. - Anti-inversion is tested explicitly in both directions: a higher- or equal-priority candidate in the agent's OWN store still wins (`TestBestStoreWithWorkDoesNotInvertTheBug`), so a fix that merely preferred the federated store would fail this suite. - Disclosure on scope of local verification: I ran the targeted suites above, not a full green `go test ./cmd/gc/` — that package exceeds Go's default 10m timeout on my machine before finishing, on stock main as well as with this change, so I am relying on CI for the whole-package result rather than claiming a green I did not see. ## Validation Not only unit-tested: this fix has been running continuously in a real multi-rig deployment since 2026-07-21, where the reported symptom was a crew agent's rig-routed P0 that never dispatched while the agent held ordinary city work. Since the change, rig-routed P0s dispatch ahead of lower-priority city work, and no in-progress resume has been observed preempted (the carve-out that protects it is the one the deployment exercises most).
…lean-promotion fix(lint): keep module checksums read-only
…townhall#4757) Fixes gastownhall#4756 ## Finding fixed `normalizeVersion` in `cmd/gc/cmd_version.go` truncated at the first `+`, discarding all SemVer build metadata from `gc version` output. This PR strips only the Go-specific `+incompatible` suffix and preserves everything else, extending the pseudo-version-collapse regexes to tolerate a trailing build-metadata suffix so `+dirty` handling is unaffected. ## Why this doesn't conflict with SemVer §10 Precedence must ignore build metadata — but the value this function produces is display-only, so no precedence decision is involved. It flows from `resolveBuildMetadata` into the package-level `version` var, which is read only by `gc version`'s stdout/JSON output and by a status accessor (`cmd/gc/api_state.go`). I checked every non-test call site of `compareSemver` and `deps.CompareVersions` on current main (a72480e): `cmd/gc/cmd_pack_registry.go:770`, `internal/doctor/checks.go:435`, `internal/packman/resolve.go` (several), and `internal/beads/bdstore_ready_projection.go:93`. None of them receives this value — the two that use a variable spelled `version` take an independently-sourced one (`c.getVersion()` for an external binary, and the parsed output of `bd version` respectively). `internal/deps/version.go` has its own separate, unexported `normalizeVersion` used solely for comparison, which already strips build metadata for precedence purposes per spec; this PR does not touch it. ## Tests Extended `TestNormalizeVersion` with the falsifiable pre-fix case, a newer pseudo-version timestamp, and an explicit `+incompatible` pin. RED on unmodified main with the test cases applied alone: ``` --- FAIL: TestNormalizeVersion (0.00s) cmd_version_test.go:29: normalizeVersion("1.3.5+ra.1") = "1.3.5", want "1.3.5+ra.1" ``` GREEN with the change. `gofmt -l cmd/gc/` clean, `go vet ./cmd/gc/...` clean. ## Honest test scope I ran `go test ./cmd/gc/ -run 'TestNormalizeVersion|TestVersion'`, not the full `./cmd/gc/...` suite — that package independently exceeds Go's default test timeout on this machine on stock main as well as with this change, so a failure there would not be attributable to this diff. Leaning on CI for the full package rather than claiming a green I did not see. ## Dogfooding This change has run continuously on a patched build in a live deployment since 2026-07-19; it is what makes that build distinguishable from stock in `gc version`, which is the reason it was written.
…lity-goflags Fix quality gate module mode
…ked-state signal the hook uses (gastownhall#4759) Fixes gastownhall#4758 ## What `workBeadHasAwakeDemand` fired assigned-work wake demand for an `in_progress` bead from its mere presence, never checking whether it carried an open blocking dependency or gate. The hook's crash-recovery work-query tier already checks this (gastownhall#4726, `IsBlocked`-equivalent enrichment) and refuses to dispatch such a bead. The two paths disagreed: the reconciler kept waking the session, the hook kept returning `no_work`, and nothing reconciled them. ## How - `AwakeWorkBead` gains a `Blocked bool`, populated only for `in_progress` beads from bd's existing `IsBlocked` denormalized ready-work projection (`beads.Bead.IsBlocked`). No new store read — the field was already fetched and simply not threaded through. - `workBeadHasAwakeDemand`'s `in_progress` case changes from unconditional `true` to `!bead.Blocked`. - `open` work is untouched — its blocker state was already folded into `Ready`. Zero-value `Blocked` is `false`, so every existing caller and test that does not populate it keeps today's behavior. This is strictly narrowing: it can only stop a wake, never cause one. ## Relationship to gastownhall#4752 (my other open PR on this file) Different predicates on different arms of the same function, verified empirically rather than by inspection: I fetched gastownhall#4752's head and ran this PR's repro against it — still RED, and still with wake reason `assigned-work` rather than `reset-pending`. They are complementary and can land in either order. If you would prefer them as one PR, I am happy to combine them. ## Tests - `TestRegression_PolecatWithBlockedInProgressWork_DoesNotWake` (new), proven RED on unmodified main a72480e by applying the test plus the inert field and reverting only the behavior line: ``` compute_awake_set_test.go:1341: session "polecat-mc-p1" should be asleep but is awake (reason: assigned-work) ``` - `TestRegression_PolecatWithInProgressWork_StaysAwake` (pre-existing) still passes — unblocked `in_progress` work still wakes. This is the anti-inversion guard: a "fix" that simply stopped waking `in_progress` work would fail it. - `gofmt -l cmd/gc/` clean, `go vet ./cmd/gc/...` clean. - Blast radius: `go test ./cmd/gc/ -run 'Awake|Wake|Reconcil|Drain|Hook|Session|Regression|NamedOnDemand|Suspend|Scale'` — 284s, exactly one failure, `TestReapClosedBeadWorktrees_ProtectsViaActiveSessionDir` (`Protected = [], want 1 session-protected entry`). ## Honest test scope That one failure is **not** attributable to this change, and I controlled for it rather than asserting it: with the patch stashed, stock main fails the same test with the identical assertion. It is a macOS `/private` symlink artifact in a session-dir path comparison, in a file this diff does not touch. I did not run the full `./cmd/gc/...` package suite. It does not complete on this machine on stock main either — a `t.Parallel()` hang plus ~45 TMPDIR path-assertion failures, a count three separate runs here have independently reproduced on unmodified upstream. I ran the broad filtered suite above instead and am leaning on CI for the rest, rather than claiming a green I never saw. ## Dogfooding The symptom this fixes was measured in a live deployment, not constructed: one named session drain-acked with assigned work 38 times in 6 hours, all clean completions, including once on a message bead — which is why the fix keys on blocked-ness rather than on anything work-bead-specific.
…nciler-owned codex hooks (gastownhall#3919) ## Problem The `build_desired_state` home-dir reconcile tick has **two writers** of a Codex agent's `.codex/hooks.json`, and they disagree — leaving a permanent hybrid document that `gc doctor` flags as `codex-hooks-drift` ("needs upgrade") forever, never converging even across `gc stop/start` bounces or repeated `--fix`. Both writers run in `prepareTemplateResolution` (`cmd/gc/build_desired_state.go`) against the **same** home `workDir`: 1. **Overlay staging** — `materializeProviderOverlaysBeforeFingerprint` → `runtime.StageProviderOverlayDir` → `internal/overlay` merge writes the overlay's SessionStart entry with **`matcher:""`** (unbound `gc prime`). 2. **Reconciler** — `hooks.InstallWithResolver` writes the SessionStart entry with **`matcher:"startup"`** (bound `gc --city '<root>' prime`, per gastownhall#3866). The overlay merge keys hook-entry identity on the `matcher` value (the dedupe introduced in gastownhall#3808), so the bound and unbound entries are treated as distinct and **both survive**. `hooks.Install` converges the document to the single bound entry, but the very next staging tick re-merges the unbound `matcher:""` entry back in — so the on-disk document a fresh session-start / `gc doctor` reads is perpetually `[startup, ""]`. This is a regression surfaced by gastownhall#3866 (which introduced the bound matcher) in combination with gastownhall#3808's matcher-keyed dedupe. **It reproduces on a clean `main`** — see the TDD proof below. ## Fix Make the `build_desired_state` home-dir staging path **skip reconciler-owned mergeable files** (`overlay.IsMergeablePath` — `.codex/hooks.json`, `.claude/settings.json`, …) so `hooks.Install` is the **sole writer** of those files in the home dir. The two writers can no longer disagree because there is only one. - `internal/overlay`: factor the existing per-provider skip into `isPerProviderPath`; thread an optional `SkipFunc` through `copyDir`; add `CopyDirForProvidersWithSkip`. - `internal/runtime`: add `StageProviderOverlayDirSkippingMergeable` wrapping the shared staging with an `IsMergeablePath` skip. - `cmd/gc`: `materializeProviderOverlaysBeforeFingerprint` uses the skip variant (6-line call-site swap). ### No-regression boundary (important) The **runtime task-worktree** staging path (`StageSessionWorkDir` → `StageProviderOverlayDir`, nil skip) is deliberately **left untouched**. For live task sessions `hooks.Install` never runs against those dirs, so overlay staging is their *sole* hook source and must keep staging the mergeable files. Only the home-dir path — where `hooks.Install` runs immediately after staging — gets the skip. Tests guard both entry points. ## TDD proof (on a clean `main`) - **RED:** with the production call-site reverted to the non-skip variant, `TestMaterializeProviderOverlays_SkipsMergeableCodexHook` fails — `build_desired_state staging wrote reconciler-owned .codex/hooks.json`. `TestCodexHooksConvergeWithSkipStaging` also demonstrates the legacy path re-drifting the hybrid (>1 SessionStart entry) after a re-stage. - **GREEN:** with the fix, both converge to a single bound `[startup]` entry that stays stable across stage → install → stage, and the converged document keeps the managed `PreCompact` (context-cycle handoff) and `UserPromptSubmit` (mail check + nudge drain) hooks. `go vet ./cmd/gc/ ./internal/overlay/ ./internal/runtime/` is clean. ## Notes for reviewers - cc @ the author of gastownhall#3866 / gastownhall#3808 (Saren) — this builds directly on the matcher-binding + dedupe those PRs introduced. - `gcw-mnck` / `gcw-zd0v` in code comments are our downstream fork's tracker IDs for provenance; the fix itself is upstream-general (shared overlay / runtime / build_desired_state code, no fork- or deployment-specific behavior). --------- Co-authored-by: wbern <wbern@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…rage-binding-ownership fix: bypass managed Dolt lifecycle for complete storage bindings
…astownhall#3954) ## The juicy parts A `claude` session never persists a resume key, so `wake_mode=resume` has nothing to resume: every recycle — config-drift restart, runtime-missing wake, any reconciler bounce — silently starts a **fresh conversation**. What the user loses is that session's accumulated context, with no error and nothing logged. All three capture paths are closed for claude in `main` today: up-front minting was removed in `b01cfb8b0` (`claude --session-id` was unsupported at the time), hook-stdin capture has been codex-only since gastownhall#3220, and claude was never in the history-derive allow-list. `b01cfb8b0` pointed at "resume metadata must come from the provider after startup" as the replacement — claude was simply never wired to it. The fix is one predicate gating one branch, existing keys are never overwritten, the env path is untouched, and a successful persist now emits a one-line diagnostic so an operator debugging a fresh wake can see resume was armed. @julianknutsen revalidated this against `main` on 2026-07-23, confirmed the cherry-pick is clean and the focused hook session-key tests pass, and described it as "the session-key persistence half of the Slack delivery regression." ## What this fixes A `claude` session never persists a resume key, so `wake_mode=resume` silently starts a **fresh conversation on every recycle** (config-drift restart, runtime-missing wake, any reconciler bounce). ## How the gap arose Claude resume used to work — a correct cleanup left it without a replacement: 1. **Originally** — a `session_key` was minted up front and the reconciler injected `--resume`; claude resumed (too eagerly, in fact: #81, #112). 2. **`b01cfb8b0`** — up-front minting was removed for claude: `claude --session-id <uuid>` is unsupported, so the builtin profile dropped `session_id_flag`. The intended replacement is stated on `session_id_flag` itself — "resume metadata must come from the provider after startup." 3. **The post-start capture was wired for codex** (gastownhall#3220, hook-stdin) **and the env-based providers — claude was never included.** So minting was retired without a claude replacement, and `session_key` has gone unwritten for claude since. Current state — every post-start capture path is closed for claude: | path | state for claude | | --- | --- | | up-front mint | removed in `b01cfb8b0` (`--session-id` unsupported) | | hook stdin (`persistPrimeHookProviderSessionKey`) | codex-only since gastownhall#3220 | | history-derive | never in the allow-list (kimi / opencode / pi / antigravity) | With all three closed, `resolveSessionCommand` has no key and relaunches the base command — a new conversation every wake. ## Fix Extend the gastownhall#3220 hook-stdin persistence to the claude family — the mechanism `b01cfb8b0` pointed to as the post-mint replacement — via `providerAcceptsHookStdinSessionID(codex|claude)`. One predicate, gating only the stdin branch. | Given a session… | Then (SessionStart reports a stdin id) | | --- | --- | | claude, no key yet | persisted → next wake resumes | | codex | still persisted (unchanged) | | already has a key | unchanged (never overwritten) | | off the allowlist (e.g. gemini) | not persisted — env path handles those | ## Why it's safe Only the stdin-capture branch changes. Retained guards: an existing key is never overwritten; a provider id equal to `GC_SESSION_ID` is rejected; a stale key whose transcript is absent is cleared on the next wake (gastownhall#2688). Env-delivered ids (`GC_PROVIDER_SESSION_ID`, `GEMINI_SESSION_ID`) are handled before this gate — unchanged. Newly-active behavior is not silent: a successful persist emits a one-line diagnostic (once per session, guarded by the empty-key check), so an operator debugging a fresh wake can see resume was armed. ## Alternative considered — restore up-front minting (and a version question) Instead of capturing the id post-start, gc could mint a deterministic one up front: re-add `session_id_flag = "--session-id"` (removed in `b01cfb8b0` as "unsupported") and launch `claude --session-id <uuid>`. Current Claude Code documents `--session-id` as taking a caller-supplied UUID ([CLI reference](https://code.claude.com/docs/en/cli-reference)), and `GenerateSessionKey` already emits a valid RFC-4122 v4 UUID, so the shape fits. Minting would also repair the fork-launch path (which already assumes `session_id_flag` + `--fork-session`) and remove the rotated / never-written-key `--resume` case (gastownhall#3849) by construction. This PR deliberately does **not** take that route. `--session-id` is not called out in the changelog, so the version floor where it can be relied on is unclear, and older Claude Code without it would break if the flag were restored unconditionally. (Related: Claude Code 2.1.187 fixed `--resume` failing with "No conversation found" — so resume behavior itself moves version to version.) Capturing the provider-reported id, by contrast, works on any version whose hook emits a `session_id`, so it is the conservative, version-agnostic fix — and it composes with minting (if a key is minted up front, the capture no-ops on the already-set key). If the project wants to establish a minimum Claude Code version, or probe `--session-id` availability at launch, minting becomes the cleaner long-term shape and this capture path becomes the fallback. Raising it as a discussion point — happy to follow up with that change if maintainers prefer it. ## Related - **gastownhall#3220** — Persist Codex hook session keys; this extends the same mechanism to the claude family. - **`b01cfb8b0`** — removed unsupported claude `--session-id` minting; established that resume metadata must be captured after startup. - **gastownhall#2688** — clear stale claude `session_key` before `--resume`; the recovery guard relied on here. - **gastownhall#3849** — `--resume` crash-loop on a never-written transcript; adjacent resume/session-key hardening. ## Files - `cmd/gc/cmd_prime.go` — the `providerAcceptsHookStdinSessionID` gate. - `cmd/gc/prime_session_key_capture_test.go` — capture, codex-unchanged, no-overwrite, predicate, unsupported-family rejection, env path, and id-equals-`GC_SESSION_ID` cases. --------- Co-authored-by: wbern <wbern@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… drop (gastownhall#4910) ## The juicy parts This is silent data loss with a success exit code. `bd update <id> --set-metadata a=1 b=2 c=3` stores **one** pair, prints its success line, and **exits 0** — `b=2` and `c=3` land in bd's variadic issue-id slot, fail to resolve on stderr, and are discarded. A caller cannot distinguish a full write from a 1-of-N write, so ordinary `|| exit 1` error handling is blind to it. This refuses the shape before any store work: nothing is written, the exit code is honest, and the message names each dropped pair and the repeated-flag form that works. It cannot condemn an invocation that previously worked, because **no bead id contains `=`** — a `=`-bearing positional was already failing under bd, just quietly. The only change is a silent partial write becoming a loud refusal, issued before the write. Scope is `gc bd`, which execs bd, so nothing between the caller and this behaviour sees it. The guard is keyed off the bd subcommand and is **not** disarmed by a global flag before the verb — `gc bd --actor bob update <id> --set-metadata a=1 b=2` is refused with a non-zero exit and writes nothing, verified against a real store. A raw `bd` invocation is still exposed; the exit-code contract itself is filed upstream as gastownhall/beads#5247. ## TL;DR `bd update <id> --set-metadata a=1 b=2 c=3` writes **one** pair, reports success, and **exits 0**. `gc bd` execs bd, so nothing between the caller and that behaviour sees it. This refuses the shape before any store work — nothing written, honest exit code. ## The behaviour `--set-metadata` is repeatable and takes ONE `key=value` per occurrence; `bd update` is variadic over issue ids. So only `a=1` is the flag's value — `b=2` and `c=3` become positional issue ids. ```console $ bd update bd-abc --set-metadata probe_a=1 probe_b=2 probe_c=3 ✓ Updated issue: bd-abc — scratch probe Error resolving probe_b=2: no issue found matching "probe_b=2" Error resolving probe_c=3: no issue found matching "probe_c=3" $ echo $? 0 $ bd show bd-abc --json | jq -c '.[0].metadata' {"probe_a": 1} ``` Three pairs in, one stored, exit 0. A caller cannot distinguish a full write from a 1-of-N write, so no `|| exit` guard can catch it. Reproduced on bd 1.1.0, unchanged in 1.1.2 (`cmd/bd/update.go` is identical between them). Reported upstream as gastownhall/beads#5247. The asymmetry that hides it: repeated `--unset-metadata` flags all apply — only the set path loses pairs. ## Why it can't break a working invocation **No issue id contains `=`.** A `=`-bearing positional therefore never resolved under bd either — it was already failing, silently. The guard converts a silent partial write into a loud refusal, issued *before* the write rather than after. ## Behaviour ```gherkin Given `gc bd update <id> --set-metadata a=1 b=2` When doBd runs Then it exits non-zero, names the dropped pair and the repeated-flag form, and performs no store work ``` ```gherkin Given `gc bd update <id> --set-metadata a=1 --set-metadata b=2` When doBd runs Then it proceeds unchanged ``` ```gherkin Given `gc bd update <id> --add-label role=worker` When doBd runs Then it proceeds unchanged — the value belongs to --add-label, not the id slot ``` ## The part that needs care Positional detection needs the **complete** value-flag table. With a subset, the value of any omitted flag is read as a positional id — and `gc bd update <id> --add-label <key>=<value>` is shipped verbatim in `internal/bootstrap/packs/core/skills/gc-work/SKILL.md:50`, so a partial table breaks a documented command. `internal/bdflags` already declares itself the single source of truth for bd's per-subcommand flag names, so the argv parsing lives there rather than beside it. `SplitGlobalFlags` also skips global value-flag values: locating the subcommand by first non-flag token reads `bob` out of `bd --actor bob update …`, which would bypass any guard keyed off the subcommand — including this one. Tests cover both directions — a drift guard proves **every** value-taking `update` flag is a non-false-positive, the real dropped-pair shapes are caught, and the refusal is scoped to `update`. ## What this does not cover Same failure mode — bd reports success for a partial write and exits 0 — outside this guard's reach: - **A raw `bd` invocation.** `gc bd` is the only entry point guarded here. Filed upstream as gastownhall/beads#5247. - **A partial multi-id update.** `bd update <good-id> <bad-id> --set-metadata k=v` writes one bead, fails to resolve the other on stderr, and exits 0. The token carries no `=`, so this guard cannot distinguish it from a legitimate id. Measured on bd 1.1.0. - **`bdMutationWriteIDs` (pre-existing, `cmd/gc/cmd_bd.go`).** The exact-ID guard added for gcy-g4o takes `sub := args[0]`, so *any* leading global flag — value or boolean — skips it, and `gc bd --json update <id> …` reaches bd unverified. That is a different guard against a different failure (substring resolution mutating the wrong bead), it predates this PR, and this PR does not change it. `SplitGlobalFlags` is the obvious fix, but it widens that guard's activation surface, so it belongs in its own change rather than being smuggled in here. ## Scope | File | Change | |---|---| | `internal/bdflags/bdargs.go` | new — argv parsing (+120) | | `internal/bdflags/bdflags.go` | `GlobalValueFlags()` accessor (+7) | | `cmd/gc/bd_mistyped_metadata.go` | new — the guard (+16) | | `cmd/gc/cmd_bd.go` | hook in `doBd` (+6) | | tests | `bdargs_test.go`, `bd_mistyped_metadata_test.go` | ## Related - gastownhall/beads#5247 — the exit-code contract, upstream in bd. This guard is the downstream mitigation; it protects `gc bd` only, not a bd invocation an agent improvises. - gastownhall#4901 — publishing the bead DELETE endpoint's soft-delete contract in the spec (same theme: a CLI/API surface that reports success for something other than what the caller asked). --------- Co-authored-by: wbern <kenneth.bernting@me.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…nce at exit (gastownhall#4770) (gastownhall#4771) ## Summary Fixes gastownhall#4770. `renudge-stale-human-gates.sh` wrote its per-gate dedup ledger to disk exactly once, at the end of the sweep, after every gate had been processed — so the mail was durable before the record that it was sent. A process death anywhere between the first successful `gc mail send` and that final write loses every already-sent gate's ledger entry, and the next 5-minute cooldown run re-nudges all of them. This is the abnormal-death analogue of the loud-fail argument gastownhall#4543 made and gastownhall#4553 (which introduced this script) adopted for the documented-non-zero-exit case; this PR extends the same "state durable before the process can end" reasoning to death the script never gets a chance to handle at all. ## Fix Extracted the existing atomic mktemp+mv write into a `write_state()` helper and call it immediately after each successful `gc mail send`, in addition to the existing end-of-sweep call (now routed through the same helper) before the retention prune. Additive, 3 hunks, 11 lines added / 4 removed — the 4 removed lines are the old single end-of-sweep write, replaced by a call to the new helper. Dedup semantics, closed-gate re-verification, and the one-reminder-per-gate-per-hour cadence are untouched. **Cost, stated deliberately rather than left to be discovered:** this turns one mktemp+rename per *sweep* into one per *successful send*. On a city with N stale human gates that is N atomic writes per sweep instead of 1 — in our own deployment, roughly 25 per 5-minute sweep rather than 1. We think that is the right trade: the ledger is a small JSON object bounded by `GC_STALE_GATE_STATE_RETENTION` (default 24h), the write is a few hundred bytes to the pack state dir, and it only occurs on sweeps that actually send mail — while the failure it prevents is a re-notification storm at 12x the intended cadence aimed at a human. Worth flagging because this script runs under a controller exec-timeout, so its own runtime budget is not free. If you would rather bound it (e.g. write at most once per K sends, or only when the ledger has grown), say so and I will adjust — but a partial ledger is what makes the fix work, so batching re-opens a smaller version of the same window. ## Testing RED, on unmodified current main (`679e6e46`), isolated harness — fake `gc` first on `PATH`; no live order run, gate, bead or mailbox touched. Each iteration is a pair: run 1 killed ~0.35s after its first successful send, run 2 the next cooldown sweep seconds later. With `RENUDGE_INTERVAL=1h`, no gate sent in run 1 may be sent again in run 2. ``` iter 1: rc1=137 state-after-kill=ABSENT run1-sent=[g1] run2-sent=[g1 g2 g3 g4 g5] RED (re-sent: g1) iter 2: rc1=137 state-after-kill=ABSENT run1-sent=[g1 g2] run2-sent=[g1 g2 g3 g4 g5] RED (re-sent: g1 g2) iter 3: rc1=137 state-after-kill=ABSENT run1-sent=[g1] run2-sent=[g1 g2 g3 g4 g5] RED (re-sent: g1) iter 4: rc1=137 state-after-kill=ABSENT run1-sent=[g1 g2] run2-sent=[g1 g2 g3 g4 g5] RED (re-sent: g1 g2) iter 5: rc1=137 state-after-kill=ABSENT run1-sent=[g1 g2] run2-sent=[g1 g2 g3 g4 g5] RED (re-sent: g1 g2) TOTAL: 0 clean / 5 re-send, over 5 iterations ``` GREEN, this branch, identical setup and kill timing: ``` iter 1: rc1=137 state-after-kill={"g1":...} run1-sent=[g1] run2-sent=[g2 g3 g4 g5] GREEN iter 2: rc1=137 state-after-kill={"g1":...,"g2":...} run1-sent=[g1 g2] run2-sent=[g3 g4 g5] GREEN iter 3: rc1=137 state-after-kill={"g1":...,"g2":...} run1-sent=[g1 g2] run2-sent=[g3 g4 g5] GREEN iter 4: rc1=137 state-after-kill={"g1":...,"g2":...} run1-sent=[g1 g2] run2-sent=[g3 g4 g5] GREEN iter 5: rc1=137 state-after-kill={"g1":...,"g2":...} run1-sent=[g1 g2] run2-sent=[g3 g4 g5] GREEN TOTAL: 5 clean / 0 re-send, over 5 iterations ``` 5/5 both directions, deterministic. The `run2-sent` column is the load-bearing half: the gates that were genuinely never sent are still sent on the next sweep, so the fix is scoped rather than merely permissive — it suppresses re-sends, not sends. - [x] `bash -n` on the fixed script — clean - [x] RED/GREEN floor re-derived at this PR's own base (`679e6e46`), 5 runs each direction - [ ] `go test ./internal/bootstrap/packs/...` — could not run locally: `go-icu-regex` fails to cgo-compile on this machine (`unicode/regex.h` not found). A/B-verified identical on unmodified stock, so it is environmental and not this change; no Go code is touched. Deferred to CI. ## Checklist - [x] Linked an issue (gastownhall#4770, opened alongside this PR) - [x] Added test evidence for the behavior change (RED/GREEN transcripts above). A Go behavioral test mirroring `TestRenudgeStaleHumanGatesScriptContract` in `pack_orders_test.go` would be a reasonable follow-up but is not required to land this fix — the existing contract test does not exercise abnormal death, and the shell harness above covers that case without teaching the Go harness to SIGKILL a subprocess. - [x] No breaking changes — purely additive within the same script; no CLI surface, no other file touched. Co-authored-by: rand <home@callindor.halibut-banjo.ts.net>
…lose-gate (Fixes gastownhall#4764) (gastownhall#4765) Fixes gastownhall#4764. ## Summary - The drain-ack finalize path now uses a dedicated assigned-work probe (`...ForCloseGate` variants) that excludes the session's own `mol-do-work` "drain" step, so a session that has already signaled completion is not perpetually judged to still have open work. - The drain-step match is on the **last dot-segment** of `gc.step_ref` (formula-qualified, e.g. `mol-do-work.drain`), not a bare-literal `"drain"` comparison — the store never writes the bare form, so a bare-literal match is a no-op against real data. - The exclusion is reached **only** from the drain-ack finalize path. The pre-existing probe used by the awake-work chain, the failed-create close, and the generic idle/config-drift close is untouched. ## Root cause A pool session's own `mol-do-work` drain step ("Close drain step and signal completion") is an open, session-assigned bead. The close gate counted it as assigned work, so the session bead never closed and the pool controller respawned a new session onto the same still-open step — a livelock, observed in production as 154 drain-acks from one session in 43 minutes. The two halves must land together: the exclusion is inert without the segment-wise `gc.step_ref` match, because the bare literal never matches a stored value. ## Test plan Verified at `431711fe009e354c22f146aed887563797dde98b` (main at the time of writing), macOS arm64. - [x] **Failure floor, 5/5 RED and deterministic.** With only the two new tests applied to unmodified main, `TestReconcileSessionBeads_DrainAckOwnDrainStepClosesWithoutEvent` fails on all 5 consecutive runs. - [x] **Negative control, 5/5 PASS in the same runs.** `TestReconcileSessionBeads_DrainAckStepNamedDrainInOtherFormulaStillBlocksClose` — a step named `drain` in a different formula — passes both before and after, so the fix is shown to be scoped rather than merely permissive. - [x] `go build ./...` — clean - [x] `go vet ./cmd/gc/...` — clean - [x] `gofmt -l` on the three touched files — clean - [x] `go test ./cmd/gc/ -run 'DrainAck' -count=1` — ok, including both new tests - [x] An untargeted full-package `go test ./cmd/gc/...` does not complete in this dev environment for two pre-existing reasons reproduced identically on unmodified main (macOS `/private` TMPDIR symlink path assertions, and one unrelated hang). Neither touches the changed files or call chain.
… --dry-run A formula attach routes the cooked wisp/workflow root but left the work bead's own gc.routed_to untouched. gc.routed_to on the work bead is what the claim path reads, so after an attach the bead looked unrouted to anything reading that field directly, even though the sling reported success and a workflow was running against it. The convoy-first graph.v2 branch is the path that drops it silently: it passes an empty sourceBeadID into the shared launch helper by design (the source is tracked through the input convoy rather than gc.source_bead_id), so the helper's own restamp never fires for it. --dry-run did not disclose the split either: its formula-attach preview printed only the plain-routing line, with no mention that a second bead is cooked and routed. - internal/sling/sling_core.go: add restampWorkBeadRouting; call it from the convoy-first graph.v2 branch and from doStartGraphWorkflow whenever sourceBeadID is non-empty. Widen onFormulaNeedsAttachment's guard to usesFormulaBackedRoute so the routed-raw override covers a target's default_sling_formula, not just an explicit --on. - cmd/gc/cmd_sling.go: disclose the wisp/workflow root in the dry-run route section when a formula attach is in play.
…(refs ga-f43t9b) Adds tests for the fix plan's items 2, 5, and 6, plus item 1's already-applied fix to restampWorkBeadRouting: - TestSlingAttachGraphFormulaCreatesConvoyFirstRoot (item 6): asserts gc.execution_routed_to is stamped, gc.routed_to is not. - TestRestampWorkBeadRoutingCollapsesPoolInstanceResolvedViaResolveAgent (item 2): resolves a pool instance via agentutil.ResolveAgent instead of a hand-built config.Agent literal, so the missing PoolName on a real resolved copy is actually exercised. - TestOnFormulaNeedsAttachmentAppliesToDefaultSlingFormula (item 5): proves the usesFormulaBackedRoute guard applies via a target's default_sling_formula, not only an explicit --on. - TestDoSlingSkippedForClaimWarningNamesDefaultFormula (item 3): RED — the SkippedForClaim warning renders "--on was skipped" (double space, no formula named) when reached via default_sling_formula instead of an explicit --on flag.
…oot disclosure to graph.v2 (refs ga-f43t9b) resolveIdempotentShortCircuit hardcoded --on %s when rendering the skipped-attach warning, producing "--on was skipped..." when the attach was reached via the target's default_sling_formula rather than an explicit --on. Fall back to naming the default formula in that case. cmd_sling.go's --dry-run preview unconditionally claimed a formula attach would also route a second (wisp/workflow root) bead. That's only true for graph.v2 attaches -- legacy attach deliberately leaves the wisp root unrouted (see the finalize() design-intent comment in sling_core.go, citing gastownhall#2848 and TestOnFormulaAttachesAndRoutes). Scope the disclosure to graph.v2 formulas via a new helper built on the existing graphv2.IsGraphV2Formula + sling.SlingFormulaSearchPaths. TestDryRunOnFormula was asserting the over-claim: its "code-review" fixture formula is version=1 (legacy), so the fix correctly removes the line there. Inverted the assertion and added TestDryRunOnFormulaGraphV2 for the positive (graph.v2) case. Completes exit_contract items 3 and 4 of ga-f43t9b; items 1, 2, 5, 6 were already satisfied on this branch.
…n window (gastownhall#4993) ## Problem The private uploader budgets 100 ms to acquire its lock, but it spends that budget **waiting** before ever testing whether the lock is free (`internal/productmetrics/spawn.go`). Under ordinary scheduler delay the wait alone consumes the budget, so the batch is skipped as "contended" when in fact nothing held the lock. Symptom: product metrics silently dropped on a busy machine — no error, just missing batches. ## Fix Try a **non-blocking** acquire first (`tryAcquireLock` on the storage backend), and only open the contention wait when that genuinely fails. This adds `tryAcquireLock` to the `storageDirectoryBackend` interface; both implementations (`lock_unix.go`, `platform_unsupported.go`) are updated. ## Riding along: two test-stability fixes Both are the same flake shape — a real scheduler delay inside a fixed window: - **`cmd/gc/productmetrics_testhook.go`**: freeze the tagged-process decision clock so a delay inside the 50 ms window can't break the contract under test. (Build-tagged test hook; the `Now` field already existed.) - **`internal/session/productmetrics_child_env_test.go`**: publish the child env snapshot atomically (temp + rename) so the spy can't read a torn half-written file. Happy to split these into a separate PR if you'd rather keep this one to the lock change. ## Tests Existing coverage already pins the behavior: `TestPrivateUploaderAttemptsFreeLockBeforeStartingContentionWait`, `TestStorageTryUploaderLockDistinguishesFreeAndContended`. Full `internal/productmetrics` suite green (25.9 s), `internal/session` green, `go build ./...` and `go vet` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#4992) ## Problem Gate scripts run with `HOME` deliberately sandboxed to the city directory, so a gate cannot write into the operator's real home. But `gc` resolves its **machine-level** state directory as `HOME/.gc` when `GC_HOME` is unset (`internal/gchome/gchome.go`). So any `gc` invoked from a gate script resolves its cache and registry to `<city>/.gc` instead of the machine's `~/.gc` — a second, divergent copy of machine state that nothing else in the fleet reads. Silent: the gate succeeds, it just populated the wrong directory. ## Fix Pass `GC_HOME` through explicitly to gate subprocesses. The sandboxed `HOME` stays sandboxed; machine-level state stays machine-level. Resolution order mirrors `gchome`: explicit `GC_HOME` → `HOME/.gc` → temp fallback. ## Tests `condition_test.go` asserts `GC_HOME` is present in the gate subprocess environment (fails on this base without the fix: `missing env var GC_HOME`). `go test ./internal/convergence/` and `go vet` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- mpr:issue-refs v1 --> --- 🔗 **Maintainer cross-reference** — added by the gascity maintainers, no action needed from you: - Related to gastownhall#4859 — adds GC_HOME to the gate/check subprocess environment in ConditionEnv.Environ(), so a nested gc resolves its machine-level cache and registry against the operator's real ~/.gc instead of the empty city-local one, while leaving the sandboxed HOME intact — this is the issue's recommended fix candidate A, implemented by resolving GC_HOME inside the convergence package rather than threading a field from runRalphCheck <sub>Linked for triage visibility — not auto-closing. If this looks off, just delete this block.</sub> <!-- /mpr:issue-refs --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…est (refs ga-mwrstg) TestDryRunOnFormulaGraphV2 called formulatest.EnableV2ForTest, tripping the cmd/gc test-file ceiling in TestLegacyFormulaV2MechanismFrozen (6th coupled file vs. a frozen ceiling of 5). rollout.ForTest is not a valid substitute yet: internal/formula.IsFormulaV2Enabled (the actual graph.v2 compile gate) reads only the legacy atomic.Bool global, and no production code consumes rollout.Flags.FormulaV2 (Stage 1 of the rollout migration resolves the gate but nothing wires it in yet). The legacy flag defaults to true at init() and no cmd/gc test disables it, so the explicit enable call was redundant. Drop it (and the now- unused formulatest import) instead of adding a decorative rollout.ForTest call with no functional effect.
## Summary This is the `orders`-scoped replacement for gastownhall#3845. It preserves Karel Bourgois's two original orders commits and authorship while separating the CLI and watchdog behavior from the mixed-scope branch. - requires `--confirm` when eligible retention deletions exceed the configured threshold - fails closed when the eligibility count cannot be read - guards the controller retention watchdog with backup freshness - regenerates the CLI reference from the Cobra source Original commits assigned here: `c57d7dbb6fff56b0a1a253c106e17dbe951d5070` and `e3f0dd420ad67a0d6d2b723c799f300bfbd780f6`. ## Dependency Depends on gastownhall#4957 for `doctor.BulkDeleteSafe`. This branch intentionally does not duplicate that doctor-scoped commit; focused orders tests pass when gastownhall#4957 is layered underneath it. ## Test plan - with gastownhall#4957 layered: `go test ./cmd/gc -run 'OrderSweepTracking|OrderTrackingRetentionWatchdog'` - with gastownhall#4957 layered: `go run ./cmd/genschema` (clean worktree afterward) - `make check-docs` Split from and supersedes the `orders` portion of gastownhall#3845. Please credit @bourgois for the original implementation. --------- Co-authored-by: bourgois <karel@voxist.com>
…ent (gastownhall#4994) ## Problem Model-usage facts are minted **only when a session retires**. The single model-fact emitter on the controller tick lives in `emitDueComputeFacts` (`cmd/gc/usage_compute.go`) and is gated on `isComputeTerminalState` — `asleep` / `drained` / `archived` / `suspended` / `quarantined`. A session that is still awake is never even `Get`, because `computeFactGetCandidate` is the only pre-Get filter and it requires a terminal state. The user-visible effect: **"model calls today" undercounts every session that is still running.** A long-lived agent burns tokens for hours and contributes nothing to the day's totals until it finally closes, and an agent awake across a day boundary bills its entire interval to the wrong day when it does. On a fleet where pool-routed agents self-drive for long stretches, the live portion of spend is invisible. ## Fix The reconcile tick now sweeps awake sessions incrementally, beside the existing terminal lane: - `isLiveModelSweepState` / `liveModelSweepCandidate` select awake rows from the reconcile snapshot. They are deliberately **disjoint** from `isComputeTerminalState` (asserted in the tests), so every session is handled by exactly one lane. - `emitDueComputeFacts` routes each loaded bead through one `processSessionBead` step: awake beads take the live model-usage sweep, terminal beads take the **unchanged** compute-fact + terminal-sweep path. The snapshot loop's only behavioral change is that a live candidate now also earns a `Get`. - `sweepLiveSessionModelUsage` records the interval's model facts **without closing the interval**: neither `usage_compute_emitted_at` nor `usage_model_swept_at` is stamped, so the real end-of-interval compute fact and terminal sweep still happen later exactly as before. Repeat ticks are made idempotent by the **already-persisted invocation cursor**, not by a marker — which is the correct mechanism here, since a live session is legitimately a candidate on every single tick. Because a live session is re-examined every tick, transcript discovery would otherwise repeat its bounded rollout scan indefinitely. Two additive `internal/worker` entry points split discovery from extraction: - `Factory.DiscoverSweepTranscript` — resolves the path under the same bounded keyed/keyless rules as `SweepSessionModelUsage`, without reading. A keyless Codex scan clouded by an I/O fault returns no path so the tick retries rather than trusting an ambiguous result. - `Factory.SweepSessionModelUsageAtPath` — the same cursor-guarded extraction, fact emission, OTel metrics, and cursor persistence, against an already-resolved path. Both delegate to a new shared `sweepResolvedTranscript`, which is `SweepSessionModelUsage`'s own post-discovery body lifted out verbatim — so the cursor, metrics, and settle semantics cannot drift between entry points. `CityRuntime.liveSweepTranscriptPaths` then memoizes the resolved path per `(session id, awake epoch, provider session key)`. A new awake epoch or a replacement conversation resolves its own rollout instead of reusing a stale path. ## What was deliberately left out This change was split out of a larger internal commit that also carried a storage refactor. **The routed-enumeration block is intentionally not here**, along with its `internal/classdb/sessions` import and the `processed map[string]bool` that existed only to de-duplicate against it. That block re-listed the whole sessions-class store each tick (`session.ListAllSessionBeads` with `IncludeClosed: true`, `TierBoth`, `AllowScan: true`) to reach rows the open snapshot cannot supply. It only does anything on a city that has routed `[beads.classes.sessions]` to its own store — where retired sessions are *closed* and therefore vanish from the open reconcile snapshot. On `main`'s single-store topology it is dead weight: an unconditional full-store scan added to a synchronous reconcile tick, guarded by a routing check that is always false. It also solves a different problem (recently-*closed* rows going unaccounted) than the one this PR fixes (*live* rows going unaccounted). Everything carried here is single-store safe and reaches live sessions through the snapshot the tick already has, adding no new store enumeration. ## Tests - **`TestEmitDueComputeFactsSweepsLiveSessionModelUsage`** (new) — the regression, on plain single-store `main` using the established `writeCodexRolloutForSweep` / `usage.NewLocalSink` harness. An awake codex session present in the open snapshot: - **tick 1** bills both transcript invocations (2 model facts, **0** compute facts), advances the cursor to `total:450`, and leaves **both** interval markers unset so the terminal lane is not pre-empted; - **tick 2**, with no transcript activity, appends nothing; - **tick 3**, after one invocation is appended, bills only that delta. The idempotency assertions deliberately count **raw sink lines** via a new `rawSinkKindCount` helper rather than `usage.ReadFacts`, because `ReadFacts` collapses replays by `IdempotencyKey` at read time and would pass even if a tick re-recorded work the cursor should have skipped. - **`TestLiveModelSweepCandidate`** (new) — pins the live/terminal split, including the assertion that no state is ever both. - `writeCodexRolloutForSweep` now takes its session key from a shared `codexSweepSessionKey` const instead of a parameter. Every keyed sweep scenario passes the same value, and adding a third call site makes `unparam` (correctly) flag the parameter as constant. **Red-then-green evidence.** With the production change reverted to `main`'s terminal-only gate (`computeFactGetCandidate` alone, live beads not routed to the sweep) and the new test applied: ``` --- FAIL: TestEmitDueComputeFactsSweepsLiveSessionModelUsage (0.00s) usage_compute_test.go:436: tick 1 model facts = 0, want 2 (a live session's invocations must bill before it closes); facts: [] FAIL FAIL github.com/gastownhall/gascity/cmd/gc 2.049s ``` With the fix restored, it passes. **Verification** (`CGO_ENABLED=0`, linux/amd64): - `go build ./...` — clean - `go vet ./cmd/gc/ ./internal/worker/` — clean - `gofmt -l` on the four touched files — clean - `go test ./internal/worker/` — ok (84.6s) - `golangci-lint run ./cmd/gc/ ./internal/worker/` — 0 issues - `go test ./internal/worker/` — ok - `go test ./cmd/gc/ -run 'Usage|ComputeFacts|ModelSweep' -count=1` — ok, 12 tests including both new ones An untargeted full-package `go test ./cmd/gc/` does not pass in this dev environment: it times out at 600s inside the order-dispatch / managed-Dolt path (`cmd/gc/order_store.go`, `cmd/gc/order_dispatch.go`), which is unrelated to the usage lane and untouched here. The CI `cmd/gc process` shards are the authoritative check for that package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Gate PASS, evidence at release-gates/ga-mwrstg-sling-formula-attach-routing-fix-gate.md committed at f3d1d70. CI 59 pass / 0 fail, mergeable CLEAN. Internally authored; deploy gate cleared for this head per ga-6z76y6.2.
A formula step closed through the gc-outcome-close typed contract records its disposition under gc.coordinator_outcome.producer_disposition (plus gc.outcome.producer) and never sets gc.outcome. classifyRetryAttempt read only gc.outcome, so a helper-closed attempt hit the empty-outcome branch, was recorded transient/missing_outcome, and the controller minted a spurious retry even though a valid typed outcome existed (gc-e2xqk; observed on gpk-u06l4 -> gpk-2d2p0, and a second root city dr-17bl). Consume the typed close: when gc.outcome is empty, a contract_version=1 producer_disposition that names the subject as its own work_id and carries a known disposition (deliverable or non-deliverable; gc-outcome-close only records clean closes, failures take the gc.outcome=fail path) folds as pass exactly once. Malformed, wrong-version, foreign-work_id, or unknown-disposition records stay missing_outcome, so a genuinely missing outcome still retries. Ships beadmeta constants for the typed-close key and vocabulary, a table unit test over classifyRetryAttempt, and an end-to-end processRetryControl test proving no spurious retry is minted for a typed-closed attempt.
The exact-head Codex review of the prior commit requested changes: - P1: folding a non-deliverable typed close to pass can mask a failure. The retry contract requires an explicit gc.outcome for attempt beads and treats its absence as invalid (formula-v2-transient-retries.md), and non-deliverable means "intentionally not a deliverable", not success. Fold ONLY a deliverable close (an explicit producer-named success); a non-deliverable close now stays missing_outcome and retries per the contract. - P2: the envelope was under-validated. Decode the full envelope with unknown-field rejection and require contract_version 1, work_id == subject.ID, non-empty recorded_by and reason, and a present non-empty producer, so a truncated or schema-skewed record cannot forge a pass. The producer is validated structurally (present and non-empty) rather than against a hardcoded set of actor kinds: the producer/actor kind is caller- supplied configuration, so enumerating it in Go would violate ZERO hardcoded roles / ZFC. No role name appears in Go source. Regression tests cover each invariant, including that an arbitrary novel producer string is accepted structurally.
The exact-head Codex review flagged that json.Decoder.Decode consumes only the first JSON value and DisallowUnknownFields guards only that first object, so a valid deliverable envelope followed by trailing JSON or garbage would forge a pass. Require a second decode to return io.EOF so typedDeliverableCloseFor fails closed on any trailing content. Adds a regression case.
…e-mail-session-id fix(mail): preserve typed session IDs
Remove the legacy default-restoration mutation so bf97's explicit-ID schema remains intact. Align native, matrix, and container source pins with a drift guard.
Closes gc-szyof.\n\n- atomically skips generated workflow members when a root reaches terminal disposition\n- repairs terminal-root residue during wisp GC without touching live roots\n- prevents pool session cwd stamping from manufacturing incomplete worktree evidence\n\nVerification: make test-fast-parallel; go vet ./...; pre-commit and push gates. Co-authored-by: sjarmak <t@t.co>
The Beads upgrade now pins x/net above the fixed threshold, so retain the external-tool waivers but stop masking gc.
…rAndConfirm (gastownhall#5012) ## Summary `internal/runtime/tmux/tmux.go`'s `NudgeSession` discarded the `confirmed` bool from `submitEnterAndConfirm` (`if _, err := submitEnterAndConfirm(...); err != nil {...}`) and always reported clean delivery (`nil`) whenever the Enter send itself didn't error — even when the busy-confirm loop burned its full budget and never observed the agent go busy, i.e. the message may still be sitting drafted-but-unsubmitted in the pane. This is ra-3x46cy finding 1 (PROVEN by code read): the queue-ack path (`tryDeliverQueuedNudgesByPoller`) and the idle-claim backstop's attempt counter both treat a nil error as "delivered," so an unconfirmed submit was silently swallowed instead of retried — the root cause of the 15-minute nudge stall the bead observed live. ## Fix `NudgeSession` now captures `confirmed` and, when false, returns a typed sentinel error (`ErrNudgeSubmitUnconfirmed`, wrapped with the session name) instead of nil. Callers already propagate `NudgeSession`'s error verbatim up through `Provider.Nudge`/`NudgeNow` (no wrapping in between), so a retry-capable caller now correctly sees a non-nil error and does not ack the queue item or advance its attempt counter. `delivered` (which gates the poke timestamp used for session-activity discounting) is still set on any error-free Enter delivery, confirmed or not — that accounting is unrelated to this fix's scope (see ra-3x46cy finding 3). Two pre-existing tests turned out to rely on the exact bug this patch fixes — both nudge a `claude`-provider pane whose fake command (`cat -v`) can never emit a busy indicator, so `confirmed` was always false and they only passed because `NudgeSession` used to swallow that into `nil`: - `TestNudgeSessionSkipsEscapeForClaude` (tmux_test.go) - `TestNudgePokeRealTmux`'s "never-busy claude nudge" subtest (nudge_poke_integration_test.go, gated behind `GC_TMUX_INTEGRATION=1`) Both now explicitly tolerate `ErrNudgeSubmitUnconfirmed` as the correct, expected outcome for their never-busy fake panes, with a comment explaining why. ## Test New: `TestNudgeSessionReturnsUnconfirmedErrorWhenNeverBusyForClaude` (nudge_submit_confirm_integration_test.go) — a fake `claude`-family binary that never prints a busy indicator (`GC_TEST_BUSY_AFTER=100`, far beyond the confirm budget). Proven fail-before (`NudgeSession` returned nil) / pass-after (`errors.Is(err, ErrNudgeSubmitUnconfirmed)`). ``` go test -tags integration ./internal/runtime/tmux/... -run 'TestNudgeSession|TestSubmitEnterAndConfirm' -v ... (all PASS, 20 tests) go test ./internal/runtime/tmux/... ok github.com/gastownhall/gascity/internal/runtime/tmux GC_TMUX_INTEGRATION=1 go test -tags integration ./internal/runtime/tmux/... -run TestNudgePokeRealTmux -v --- PASS: TestNudgePokeRealTmux (all 5 subtests) ``` Full integration suite (`GC_TMUX_INTEGRATION=1 go test -tags integration ./internal/runtime/tmux/...`) also run; two failures are pre-existing, unrelated environment flakes independent of this change: `TestNudgeSessionConfirmsSubmitForClaude` (passes reliably in isolation — reruns clean 3/3 — flakes only under the full batch's concurrent tmux sessions) and `TestGetKeyBinding_CapturesDefaultBinding{,WithArgs}` (depends on this machine's default tmux key-binding config, unrelated to nudging). Source bead: ra-3x46cy (finding 1). --------- Co-authored-by: Jacob Hausler <jacob@hausler.cc>
The integration test binary can prune Beads from its build metadata, so\nresolve the pinned module version through the module graph before building bd.\n\nRefs: gastownhall#3744
…er family (gastownhall#5018) ## Summary Makes the tmux nudge carrier's post-paste submit action a declarative, per-provider-family key sequence (the design proposed in upstream gastownhall#4706), instead of a single hardcoded "Enter" call site. Zero behavior change for every provider today — this is infrastructure, not a claude-specific fix (see "What this patch does NOT do" below). ## Problem ra-oudpha's dispatch note: "the idle-nudge composer STILL types-without- submitting into claude TUI sessions... This is upstream gastownhall#4706's exact shape (declarative per-provider nudge submit-key sequence)." This recurred *after* gascity#5012 (propagate the unconfirmed-submit error instead of a false `nil`) and gastownhall#5013 (clear pending input before pasting) had already landed — so the failure is now honestly reported (no false "delivered" acks) but still not resolved. gastownhall#4706 itself documents the concrete, evidenced version of this problem for codex: a k8s codex agent's first turn never started because a `send-keys -l <text> Enter` burst gets buffered by codex's TUI as a paste, and the trailing `Enter` is swallowed as a composer newline instead of triggering submit — codex's actual submit sequence is `Escape` then `Enter`. The proposed fix is to stop hardcoding per-provider key heuristics in Go and make the submit sequence declarative per provider family instead. ## Investigation for claude specifically I could not identify a wrong key as the cause of the claude-specific residual, and did not implement an unverified fix for it — reporting per the bead's "report if not obvious" instruction rather than guessing: - gastownhall#4706 itself specifies claude's default submit sequence as plain `Enter`, which is exactly what this fork already sends (`providersSkippingEscapeBeforeEnter` already includes `"claude"`, so no spurious Escape is synthesized before it either). - The mechanic's own investigation (ra-3x46cy) explicitly ruled out a busy-indicator false negative for the specimen that motivated this bead: the composer text was observed **visibly still sitting unsubmitted**, not silently-submitted-but-unconfirmed — so this isn't `paneContainsBusyIndicator` missing a fast turn. - `submitEnterAndConfirm` already retries Enter up to 3 times with busy polling between sends (~1.8-2.4s budget) before giving up honestly via `ErrNudgeSubmitUnconfirmed`. Pinning the actual cause needs a live trace against a failing session, which this fork-patch pass does not have (the city this bead is scoped against is live and read-only for this pass; ra-3x46cy's own investigator reached the identical conclusion trying to bisect the *dispatch*-side gate: "I could not safely bisect... without adding a trace line and restarting the supervisor — out of scope for a read-only pass"). ## What this patch does - `internal/runtime/tmux/tmux.go`: new `nudgeSubmitKeySequences map[string][]string` (provider family → ordered tmux key names) and `defaultNudgeSubmitKeySequence = []string{"Enter"}`, with a lookup (`nudgeSubmitKeySequenceForFamily`) and a target-resolving wrapper (`nudgeSubmitKeySequence`, mirroring how `submitVerifyEligible` and `shouldSendEscapeBeforeEnter` already resolve provider family from the `GC_PROVIDER` pane env var with a process-name-sniff fallback). - New `sendNudgeSubmitSequence(target string, keys []string) error` sends each key via `tmux send-keys`, pausing `nudgeSubmitKeySettle` (100ms) between keys in a multi-key sequence. - `NudgeSession` and `NudgePane` both now resolve and send the target's declared sequence instead of a hardcoded literal `"Enter"` string, for both the confirm/retry path (`submitEnterAndConfirm`, renamed its `sendEnter` param to `sendSubmit` — a rename only, same injected-callback shape) and the historical best-effort fallback path. - **`nudgeSubmitKeySequences` starts empty.** No family (including `claude` and `codex`) has an explicit entry, so every provider keeps exactly its current single-Enter behavior. This is deliberately scoped as pure infrastructure: I did not add codex's `["Escape", "Enter"]` entry from gastownhall#4706 in this patch, since validating it against codex's actual TUI is outside a claude-focused bead's scope and I have no live codex session to verify against — flagging it as a natural, low-risk follow-up once someone can test it. - Once a live trace pins claude's actual requirement (whatever it turns out to be — a different key, a double-Enter, more settle time), landing it is a one-line table entry plus a test, not another pass through `NudgeSession`'s delivery mechanics. ## Testing - `TestNudgeSubmitKeySequenceForFamilyDefaultsToEnter` / `TestNudgeSubmitKeySequenceForFamilyHonorsTableEntry` (`internal/runtime/tmux/nudge_submit_key_sequence_test.go`, no build tag): pure unit tests on the lookup table and its default fallback. - `TestSendNudgeSubmitSequenceSendsEachKeyInOrder` / `TestNudgeSessionUsesDeclaredSequenceForProviderFamily` (`internal/runtime/tmux/nudge_submit_key_sequence_integration_test.go`, `//go:build integration`, gated on `hasTmux()`): live-tmux tests using a `cat -v` pane (which echoes control bytes as visible caret notation, e.g. Escape → `^[`) and a throwaway `testfam` provider family registered only for the test, proving both the low-level primitive and `NudgeSession` itself actually emit every key in a declared multi-key sequence, not just the last one — this is the real wiring a future claude/codex-specific fix would depend on, not just that the lookup function returns the right slice. - Fail-before proven: temporarily made `sendNudgeSubmitSequence` send only the sequence's last key (simulating broken multi-key wiring) — both integration tests failed (`CapturePaneAll missing Escape...`). Restored → both pass. - `go test ./internal/runtime/tmux/...` (no tag) — all PASS. - `go test -tags integration ./internal/runtime/tmux/... -run 'TestNudgeSubmitKeySequence|TestSendNudgeSubmitSequence|TestNudgeSessionUsesDeclaredSequence'` — all PASS. - `go build ./...` and `go vet ./...` — clean. - Full `GC_TMUX_INTEGRATION=1 go test -tags integration ./internal/runtime/tmux/...`: only the two pre-existing, machine-config-dependent failures already documented on ra-3x46cy's earlier landing note — `TestGetKeyBinding_CapturesDefaultBinding{,WithArgs}` ("depends on this machine's default tmux key-binding config") — everything else, including `TestNudgeSessionSkipsEscapeForClaude`/`TestNudgeSessionSkipsEscapeForOpenCode` and the full nudge-submit-confirm suite, PASS with no regressions. ## Scope `internal/runtime/tmux/tmux.go` (declarative table + two new methods + the `sendEnter`→`sendSubmit` rename inside `submitEnterAndConfirm`, `NudgeSession`, `NudgePane`), plus the two new test files above. No config/TOML surface added — the table is a Go-level declarative source of truth today, matching how the existing `providersSkippingEscapeBeforeEnter` per-provider list is already Go-level rather than threaded through `config.City`; that's a bigger, separate change (full gastownhall#4706/gastownhall#4110-style config plumbing) out of scope for this fork patch. --------- Co-authored-by: Jacob Hausler <jacob@hausler.cc>
…ansient ones (gastownhall#4484) (gastownhall#4485) ## Problem `gc supervisor`'s init-failure backoff (`recordInitFailure` in `cmd/gc/cmd_supervisor.go`) applies the same capped exponential backoff (10s doubling to a 5-minute ceiling) and the same single reset trigger (`city.toml` mtime advancing) to every init failure, regardless of cause. That's correct for transient failures. It's wrong for a **structural** failure like a `bd` schema-version gate (`schema version mismatch: database is at vN, binary knows up to vM`) — no `city.toml` edit can ever resolve that; the only real fix is an out-of-band `bd` binary upgrade. Observed locally: 27+ identical failures over ~6 days, stuck at the 5-minute retry ceiling the whole time, with no change in log format or severity to signal this failure class needs a human to act outside `gc` entirely. ## Fix Two new pure, testable helpers: - `isStructuralInitFailureMessage(msg string) bool` — classifies a failure by substring match on bd's stable `"schema version mismatch"` error text, mirroring `runtime.IsSessionGone`'s existing style for external-subprocess errors with no typed sentinel available. - `initFailureBackoffDelay(count int, msg string) time.Duration` — returns the existing capped-exponential delay for transient failures unchanged, or a flat 1-hour backoff for structural ones, immediately rather than escalating gradually. `recordInitFailure` now uses `initFailureBackoffDelay` for the actual delay and emits a distinctly labeled log line — "STRUCTURAL init failure (retrying cannot resolve this — needs an out-of-band fix)" — instead of the generic `(skipping)` repeat, so an operator scanning supervisor logs sees immediately that this failure class needs external action, not more waiting. ## Testing - `TestIsStructuralInitFailureMessageDetectsSchemaVersionGate` - `TestInitFailureBackoffDelayEscalatesStructuralFailuresBeyondTransientCeiling` Both confirmed RED by temporarily neutering the structural check — the test reproduced the exact prior behavior (10s first failure, capping at 5m0s, the same symptom observed live for ~6 days) — restored, GREEN. - Broader supervisor sweep (`TestSupervisor*`, `TestReconcileCities*`, `TestRegisterCityWithSupervisor*`, `TestUnregisterCityFromSupervisor*`, `TestInitFail*`): pass, 30.5s - `go build -tags gms_pure_go ./cmd/gc/...`: clean - `go vet -tags gms_pure_go ./cmd/gc/...`: clean Closes gastownhall#4484
kubectl is an external prebuilt binary that embeds x/text 0.33.0.\nKeep the waiver path-specific and expiry-bound until upstream reaches 0.39.0.\n\nRefs: gastownhall#3744
…kGate opt-out) (gastownhall#3961) ## Summary Fixes the dispatch-starvation root cause (GAP D from vp-cixi.5) for gate-less cooldown probes like `provider-health-probe`. `provider-health-probe` is a pure cooldown probe that tracks **no beads**, yet the dispatcher runs two open-work gates per tick (`hasOpenTracking` then `hasOpenWork`), each issuing `bd list` / `bd query` reads against Dolt bounded by `orderGateTimeout` (8s). On store slowness the gate times out and `gateFailClosed` **skips the order every cycle** → the provider-health cache goes stale → fail-closed provider health → failover can't pick `claude2` or anything else. Confirmed live: 60–90+min gaps between probe runs despite a 10m interval. PR gastownhall#357 shipped a mitigation (hysteresis + wider TTL) that absorbs the skips. **This PR is the deeper gc-core fix:** let an order OPT OUT of the open-work gates entirely, since they are meaningless for orders that consume no bead work. ## Approach Add an order-level opt-out flag `no_work_gate` (Go `Order.NoWorkGate`). When `true`, the dispatcher skips **both** open-work gates for that order — no `gateOpenWorkBounded` call, no Dolt reads, no fail-closed skip, no gate-timeout backoff. The order still respects its own cooldown interval and per-order exec timeout; single-flight stays naturally bounded by the cooldown interval + the synchronous tracking-bead the dispatcher creates before launch. **Why a new flag, not reusing `Idempotent`:** `Idempotent` flips semantics to fail-**OPEN** on timeout (may double-dispatch). A gate-less probe must not even *enter* the gate — it must not depend on a Dolt read completing inside 8s, and should never emit `order.gate_timeout_fail_open`. "Safe to re-run" and "consumes no bead work" are distinct properties; conflating them would keep a probe's dispatch contingent on store health, which is exactly the bug. ## Layer split (plan ends at the boundary) The gc-core mechanism ships here. **Activating** the flag on the live probe is a deployed-pack-layer edit (`packs/voxist-city/orders/provider-health-probe.toml`, not tracked in this repo) — filed as a separate cross-layer follow-up bead (plan sling S-1), not part of this PR. This PR is mergeable on its own: it ships the opt-out mechanism + tests; the pack flip turns it on for the one order that needs it today. ## Changes (TDD, red/green/commit-on-green) - **T-001** `feat(orders)`: add `Order.NoWorkGate` field + TOML decode + validation guard (`415429c91`) — green at `TestOrderNoWorkGateParsed`. - **T-002** `fix(dispatch)`: skip both gates (`hasOpenTracking` @515 + `hasOpenWork` @612) + the `gateBackoffActive` short-circuit for `NoWorkGate` orders (`6612e768c`) — green at `TestOrderDispatchNoWorkGateSkipsGatesUnderStoreDelay`. - **T-003** `test(orders)`: provider-health-probe-shaped order opts out of the work gate (`bd0ff6b40`) — green at `TestProviderHealthProbeOrderOptsOutOfWorkGate`. - **T-004** `docs(orders)`: order-author guide note for `no_work_gate` (`45a3548fa`). ## Validation - `go vet ./...` clean - `internal/orders` + `internal/dispatch` test suites green; no regressions - 4 targeted `NoWorkGate` tests pass - Fork pre-receive CI: all 8 fast-parallel jobs passed ## GDPR / MDR impact None. The change alters *dispatch scheduling* (which gate checks run for an order), not what data the order reads, processes, or persists. No new PII is accessed, stored, transmitted, or retained. Entirely outside the voxmemo→voxist-api clinical documentation pipeline. --- Plan: \`engdocs/plans/vp-cixi.6-drop-open-work-gate-for-gateless-cooldown-probes.md\` Bead: vp-cixi.6 (child of EPIC vp-cixi). Related: gastownhall#2893, gastownhall#357. --------- Co-authored-by: quad341 <quad341@users.noreply.github.com>
Direct BdStore integration calls now use the pinned test shim, and the pinned binary assertion validates module semver correctly.\n\nRefs: gastownhall#3744
Refresh the hashed Python lock for the fixed GitPython, aiohttp, and cryptography releases required by the image scan.\n\nRefs: gastownhall#3744
…tive-store fix: align native Beads v59 schema handling
) ## Summary - include ready routed work in the runtime demand snapshot cache key - refresh patrol demand snapshots when new ready work appears without a session/config change - add a regression covering routed work that should produce poolDesired after a cached zero-demand patrol ## Test - go test ./cmd/gc -run 'TestCityRuntimeDemandSnapshot(ReusesStablePatrolDemand|RefreshesForNewRoutedReadyWork)'\n\nThis targets the observed failure where ready beads routed to imported role pools existed in the active bead store, but supervisor patrol reused a stale zero-demand snapshot and never started the role worker. Co-authored-by: t3code/worker-1 <noreply@t3code.local>
… red) (gastownhall#5038) ## Problem `main` does not compile. gastownhall#3667 added `sessionBeadSnapshotFingerprint`, which calls `fnv.New64a()`, without adding `hash/fnv` to `cmd/gc/city_runtime.go`'s import block: ``` $ git checkout 3f4173e # current main, no other commits $ go build ./cmd/gc/ # github.com/gastownhall/gascity/cmd/gc cmd/gc/city_runtime.go:3424:7: undefined: fnv ``` Because CI builds each PR **merged with** `main`, this fails every job that compiles `cmd/gc` on every open pull request: all twelve `cmd/gc process` shards, the integration suites, `Preflight / static checks`, and `Preflight / generated artifacts` — the last surfacing it as `genschema: generating CLI docs: exit status 1`, which reads like a codegen problem rather than a missing import. I found it while working out why 28 checks went red on gastownhall#4073 with a diff that touches none of those files. ## Fix The import. One line, in sorted position; nothing else in gastownhall#3667 needed changing. ## Verification ``` $ go build ./... # clean $ go vet ./cmd/gc/ # clean $ gofumpt -l cmd/gc/city_runtime.go # clean ``` gastownhall#3667's own tests pass with it in place: ``` --- PASS: TestSessionBeadSnapshotFingerprintReflectsRawMetadata --- PASS: TestCityRuntimeDemandSnapshotRefreshesForNewRoutedReadyWork --- PASS: TestCityRuntimeDemandSnapshotReusesStablePatrolDemand --- PASS: TestCityRuntimeDemandSnapshotRetainsOnlyPoolScaleCheckPartials --- PASS: TestCityRuntimeTickDispatchesOrdersBeforeDemandSnapshot ``` Those could not have run as merged, since the package they live in does not build — which is presumably how this got through. Sending it as its own PR rather than folding it into gastownhall#4073, so it can land immediately and unblock everyone else's CI too.
…p-completed fix(events): reconcile graph step completions on patrol
…leted-rig-store fix(events): reconcile rig graph completions
…n-reconcile-index fix(events): batch completion reconciliation facts
…econcile-archive-dedup fix(events): preserve completion reconciliation through rotation
Brings the fork current with origin/main (4893092). 81 conflicts, 65 of them dashboard dist bundles; generated artifacts were regenerated from the merged tree (dist + typed client + OpenAPI spec on node 22.22.3, census baselines re-derived, catalog and CI execution-shape pins verified unchanged), never picked from a side. THE LOAD-BEARING DECISION — go.mod keeps the beads bridge pin (e97839a2, schema 0054); upstream's new pin (bf97b737, schema 0062, another pseudo-version despite their own gm-mkijn rule) is NOT taken. Moving gc's vendored beads to 0062-era code while the fleet's bd and live stores sit at 0054 risks crossing the irreversible migration door (0059/0061/0062 ship no down.sql) as a resync side effect. That jump is the planned, rehearsed cutover (ga-zzcjs, ga-y1xxg), not a merge artifact. Everything that names the bridge moves together and stayed together: go.mod, deps.env BD_SOURCE_REF/SHA256, Dockerfile.agent ARGs, and the container test's divergence comment (updated to describe upstream's new 0062 reality). The merged deps.env is deliberately split-brained in the fork's favor: BD_SOURCE_REF stays on the bridge, while upstream's new BD_CURRENT_REF=bf97b737 points the contract matrix's bleeding-edge cell at 0062-era bd — a free forward-compatibility signal for exactly the migration we are rehearsing. Upstream's new unified-pin test (go.mod == BD_CURRENT_REF == image ref) encodes their one-commit world and is now gated on bridge mode being absent; in bridge mode the fork's stricter go.mod == BD_SOURCE_REF lockstep governs (scripts/bd_version_pin_test.go). checks_order_firing.go: both sides rewrote the same doctor check. Kept the fork's vc-89s structure (Since-bounded reads, archive-corruption tolerance, newest-first controller-start lookup, staleness disambiguation probe) and grafted upstream's parallel last-run prefetch into it (prefetchLastRuns, prefetchedLastRunFunc, pendingLastRunOrders, eventEvidenceSuffices, the factored latestOrderFiredAtUsing, orderFiringLastRunConcurrency). Upstream's count-bounded readEventTail seam is superseded by the Since bound, and its dependent test file checks_order_firing_bounded_test.go is REMOVED — that deletion is deliberate and visible, not silent: the seam it exercises does not exist in the merged structure. internal/doctor suite green. cmd_wait_test.go: parallel evolution of the bd pin test. Upstream renamed it (...UsesGoModSource) and added a go-version -m provenance check but weakened the version assertion to semver-validity; the fork's pseudo-version-aware machinery (declared-version via deps.env + go.mod<->deps.env commit lockstep) is strictly stronger. Union: upstream's name and provenance check, fork's assertions, wantVersion wired against the self-reported version. Verified live with GC_FAST_UNIT=0 (24s, real bd build). Parallel-evolution dedupes: mcp-agent-mail floors were bumped byte-identically on both sides (gitpython 3.1.58, aiohttp 3.14.3, cryptography 50.0.0) — the fork's fuller provenance comments kept, upstream's duplicate cryptography check removed. .trivyignore.yaml differed only in statement prose; fork's kept. .golangci.yml is the union (fork's allow-parallel-runners + upstream's modules-download-mode). city_runtime.go takes both sides' struct fields. native_dolt_store_integration_test.go takes upstream's new v59-contract test (integration-tagged; compiles against the bridge lib). Gates: build, go vet ./..., full-repo make lint, make dashboard-ci, ./internal/... and ./scripts/... green. ./cmd/... reports zero test assertion failures; the package-level FAIL is the known pre-existing darwin dolt leak guard (ga-35n07), which reproduces on clean checkouts of both parents. Refs: ga-zzcjs, ga-y1xxg, ga-35n07
Ten findings from the xhigh post-merge review of #123, two of which CI independently caught. All are defects of the MERGE RESOLUTION, not of either parent. Resilience (the severe one): bdContextCommandRunnerForCity — upstream's new early-return path for scopes with a complete external storage binding — executed bd with no scope breaker, no admission semaphore, and no outcome recording, silently exempting exactly the external-endpoint scopes the #118/gastownhall#3318 resilience work targets. The context runner now applies the same breaker + admission + recording as the managed path (it skips managed recovery, not resilience); with no managed retry, the single invocation's outcome is the breaker's final word. TestCompleteBindingScopesStayBehindTheBreaker pins the routing: a complete-binding city trips to ErrStoreUnavailable after 3 wedged timeouts and an open breaker spawns zero subprocesses. Doctor (ga-klv regressions reintroduced by keeping the fork's shape): restored upstream's bounded event read INSIDE the fork's vc-89s structure — the order.fired read now goes through a readEvents seam (events.ReadFilteredTail) doubly bounded by the 2000-event tail and the fork's Since window; the archive-spanning reads (controller-start lookup, never-fired probe) keep the corruption-degrades-to-warning behavior, and the corrupt-archive test now targets those paths. Restored upstream's corrected timeout FixHint (query cost, not connectivity). Ported upstream's deleted guard file (9 tests: bounded reads, large-log budget, tail fallback, hint text, parallel lookups, error preservation, prefetch skip, positive limit, log path), adapted to the seam split. pendingLastRunOrders now filters the same monitored slice the classification loop iterates — the duplicated filter chain the graft introduced is gone, as are its duplicated/misattached doc comments. Integration build: the fork's TestNativeDoltStoreEventsIDDefaultRepair survived while the merge adopted upstream 9f2e1a1's removal of the repairIDDefault self-heal, leaving dangling references that broke every integration-tagged build of internal/beads (CI packages-core-4-of-4). The removal was deliberate — upstream's new TestNativeDoltStoreOpenPreservesMissingIDDefaults pins the opposite contract (open must not mutate schema) — so the obsolete test is deleted, not the contract reverted. Container scan: the merge kept BOTH CVE-2026-56852 waivers — the fork's kubectl-only narrowing (#121) AND upstream's broad gh+dolt+kubectl entry, so Trivy silently re-waived binaries the fork rebuilds patched. Dropped the broad entry and restored the fork's stricter test (gh must carry no waiver at all; no allowed-waiver escape hatch). Static checks: the module-graph cap failed at 728 > 727 because each parent sat exactly at the cap with a different marginal module (bridge pin keeps wk8/go-ordered-map/v2; upstream's otel v1.44 adds otel/metric/x). Cap bumped to 728 with the revert condition (ga-zzcjs repin) recorded in place. Hygiene: untracked the two .omc session-state files the merge commit accidentally added and ignored .omc/* (with the .omc/skills/ committable exception); added REQUIREMENTS.md row SESSION-ID-012 reconciling upstream 2e1a9cf's bead-actor/alias alignment per the session package's ledger rule. Gates: vet clean; full lint clean except one untracked node_modules vendored Go file (local artifact, absent from CI checkouts); go vet -tags integration ./internal/beads/ compiles; ./internal/... green (one unrelated flake, TestProvider_StartCancellationInterrupts- ForegroundChild, passes 3/3 in isolation); ./scripts/... green; ./cmd/... zero assertion failures (package FAIL is the pre-existing darwin dolt leak guard, ga-35n07); dependency-surface guard passes. Refs: ga-2bo4m, ga-klv, ga-zzcjs, ga-35n07
Every waiver in the file carried expired_at: 2026-08-07 — the ga-xft2v review horizon — and the whole set expired at once at midnight UTC, turning the image-vulnerability gate red on the first scan of 08-07 (CVE-2026-41602, apache/thrift in dolt, was merely the first image's first hit; the loop stops there). Each entry's removal condition is still unmet (no Dolt release with thrift >= 0.23.0, no kubectl built against patched x/text, etc.), and the upstream question about how this horizon is meant to be managed (gastownhall#5054, filed before the deadline) remains unanswered. Extend the horizon one month rather than per-entry: the statements already carry their individual removal conditions, and the shared date keeps the next review a single deliberate event instead of 45 staggered alarms. Refs: ga-xft2v
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
13 participants
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.
Brings the fork current with
origin/main(489309232). 0 behind after this. 81 conflicts, 65 of them dist bundles; every generated artifact regenerated from the merged tree, never picked from a side.The load-bearing decision: the beads pin stays on the bridge
Upstream moved their beads pin to
bf97b737— schema 0062, another pseudo-version despite their own gastownhall#4920 rule. This resync deliberately does NOT take it: gc's vendored beads jumping to 0062-era while the fleet's bd and live stores sit at 0054 risks crossing the irreversible migration door (0059/0061/0062 ship nodown.sql) as a merge side effect. That jump is the planned rehearsed cutover (ga-zzcjs,ga-y1xxg), not a resync artifact. Everything naming the bridge moved together:go.mod,deps.env,Dockerfile.agent, and the divergence comment (updated to describe upstream's new 0062 reality).A useful accident of the merge: upstream's new
BD_CURRENT_REF=bf97b737points the contract matrix's bleeding-edge cell at 0062-era bd — a free forward-compat signal for exactly our planned migration. Their new unified-pin test (go.mod == BD_CURRENT_REF == image ref) encodes their one-commit world, so it is now gated on bridge mode being absent; in bridge mode the fork's stricter go.mod↔BD_SOURCE_REF lockstep governs.One deliberate, visible deletion
Both sides rewrote
checks_order_firing.go. Kept the fork's vc-89s structure (Since-bounded reads, corruption tolerance, disambiguation probe), grafted upstream's parallel last-run prefetch into it (verified:internal/doctorsuite green). Upstream's count-boundedreadEventTailseam is superseded by the Since bound, and its dependent test filechecks_order_firing_bounded_test.gois removed — stated here rather than buried: the seam it exercises does not exist in the merged structure.Parallel evolution, deduplicated
go version -mprovenance check adopted, fork's strictly-stronger deps.env-lockstep assertions kept,wantVersionwired against the self-reported version — verified live withGC_FAST_UNIT=0(real bd build, 24s).golangci.ymlunion;city_runtime.gotakes both sides' struct fields; upstream's v59-contract integration test taken (compiles against the bridge lib)Gates
Build,
go vet ./..., full-repomake lint,make dashboard-ci,./internal/...,./scripts/...all green../cmd/...: zero test assertion failures; the package-level FAIL is the known pre-existing darwin dolt leak guard (ga-35n07), reproducible on clean checkouts of both parents.Census re-derived (4 baselines, Small tracked separately from Debt); catalog and CI execution-shape pins verified unchanged; dist/client/spec rebuilt on node 22.22.3.