Merge upstream/main into fork — v1.4.0+ resync (117 commits) - #120
Merged
Conversation
…ws.sh (gastownhall#4681) ## What changed `scripts/check-routed-test-rows.sh`'s six-row-matrix violation message pointed readers at `docs/plans/ga-h6w-read-path-api-routing.md` — a path that has never existed on `main` (this repo has no `docs/plans` directory at all). The hint now points at the script's own header comment instead, which already documents the same six-row matrix and cites the same tracking bead. ## Why built this way Rather than inventing a new doc to satisfy the old citation, the fix points at documentation that already exists and already covers the exact same content, so there's nothing new to keep in sync. ## What to look at Single-line, static-string change in one file. No logic in the check itself changed — same six required rows, same violation conditions. ## Test plan - Forced a synthetic manifest-missing violation locally to confirm the new hint line prints correctly and the script still exits 1; restored the manifest afterward. - `go test ./cmd/gc -run TestRoutedRowsManifestFullyCovered` and `make check-routed-test-rows` both green. - Full fast test suite green on this branch. --------- Co-authored-by: investigator <investigator@gascity.local>
…gastownhall#4628) ## Root cause `archiveOverlapsFilter` (`internal/events/rotation_archive.go:130`) consulted only `AfterSeq`/`BeforeSeq`. It ignored `Since`/`Until` entirely — even though `archiveInfo.Timestamp` (line 16) already carries the rotation instant parsed from the filename. That matters because the city event list exposes **no `after_seq` param** (`EventListInput`, `internal/api/huma_types_events.go:15`) — `since` is the only lower bound a client can send. So for a selective filter: 1. `fetchEventPageAscending` (`internal/api/huma_handlers_events.go:163`) tries the cheap `ListTail` first; 2. a selective filter returns fewer than `limit+1` rows, so it falls through to `listWithInFlight` — the full scan; 3. `archiveOverlapsFilter` excludes nothing, so 4. `streamArchive` (`internal/events/reader.go:326`, which discards its `Filter`) gunzips and JSON-parses **the entire archive history** to match nothing. On this fleet that is **53 archives / 2.1 GB per request**. ## Evidence from the running supervisor `perf record` against the live process (no restart, no pprof needed): | symbol | share of process CPU | | --- | --- | | `streamArchive` (cumulative) | **68.55%** | | `encoding/json.Unmarshal` beneath it | 48.48% | | `compress/flate` (gunzip) | ~15% | Access log — these are abandoned long-polls, **31 of 39** event requests in the sampled window: ``` GET /v0/city/gc-management/events 499 3m22.306013s GET /v0/city/gc-management/events 499 3m21.733299s GET /v0/city/gc-management/events 499 3m28.211963s ``` Direct reproduction against the live server: ``` ?type=zzz.nonexistent.event.type&limit=50 -> 176.36s ``` ## The fix Every event in an archive was appended to the live log before that log was rotated at true instant T, so `event.Time <= T`. But `info.Timestamp` (parsed from the filename) is `T` truncated to whole seconds, so only `info.Timestamp <= T < info.Timestamp+1s` is known — the true rotation instant, and therefore every event in the archive, can land anywhere up to (but not including) the next whole second. An archive is safe to skip only once `Since` reaches that `+1s` slack; a `Since` inside the truncation second must still be read. Skipping under that bound is sound, not a heuristic. Measured against the real 2.1 GB archive set: **53 of 53 archives skipped** for `since=5m`, and the same selective query goes **176s -> 1.0s** — the `+1s` slack costs nothing at that distance. ## Deliberate bounds - **Zero `Timestamp` is still read.** Legacy basenames predate the stamped convention and carry no such guarantee. - **`Until` is deliberately not handled.** The filename records the rotation instant, not the archive's *first* event, so there is no sound upper-bound skip without extra metadata. - **Assumes events do not carry future timestamps.** An event stamped after its own rotation instant could be skipped by a `Since` between the two. This is the same assumption the existing seq-based skip already makes. No current writer does this (traced every non-test `Ts:` producer in the tree); tracked as a platform-contract follow-up rather than enforced here. ## Not fixed here A selective filter with **no** `since` still replays all history — there is no lower bound to skip on. That is a contract question (bounded scan? default window? expose `after_seq`?), filed separately as a `needs-architecture` bead rather than decided from this seat. ## Test `TestArchiveOverlapsFilterSkipsArchivesOlderThanSince` pins both sides of the `+1s` truncation boundary (`Since == Timestamp+1s` must still read; `Since == Timestamp+1s+1ns` may skip), plus the pre-existing guard cases (`Since` before rotation, `Since` exactly at rotation, zero `Since`). `TestReadFilteredIncludesEventWithinArchiveSubSecondWindow` pins the same scenario end-to-end against a real gzip'd archive: an event at `rotation+500ms`, queried with `Since=rotation+250ms`, must still come back. Gates: `go vet` clean; `internal/events` and `internal/api` both green. <!-- mpr:issue-refs v1 --> --- 🔗 **Maintainer cross-reference** — added by the gascity maintainers, no action needed from you: - Related to gastownhall#4167 — implements the Since-vs-archive-basename-timestamp prune in archiveOverlapsFilter that the filed issue proposes as its first fix bullet, so time-bounded reads no longer gunzip every retained archive; it does not make ReadFilteredTail archive-aware (the truncation-at-rotation-boundary half), does not add the per-request archive/byte budget, does not handle Until, and a selective filter sent with no lower time bound still replays all history <sub>Linked for triage visibility — not auto-closing. If this looks off, just delete this block.</sub> <!-- /mpr:issue-refs --> --------- Co-authored-by: investigator <investigator@gascity.local>
…ound timeout in linked worktrees (gastownhall#4683) ## Problem `push_gate_slots_dir()` in `scripts/push-gate-lock-lib.sh` falls back to `<repo_root>/.git/gate-slots` when it cannot resolve a city root. In a **linked worktree** `.git` is a *file*, not a directory, so `push_gate_acquire_slot()`'s `mkdir -p ... || return 1` can never succeed. The contract reads `return 1` as "timed out", so the caller prints the wait-bound message — "giving up after the wait bound" — for a condition that is not contention at all and will never clear. A push from a worktree that does not resolve a city root therefore fails **100% of the time while reporting transient contention**, which invites exactly the wrong remedy (retry, or bypass the gate). ## Evidence this is a misreport, not a timeout - `mkdir` fails with `Not a directory` - the wait loop's own announce line ("all N slot(s) busy, waiting up to 600s") never appears in the failing log - elapsed was ~5s against a 600s wait bound - slot 1 was verifiably free at the time ## Fix Degrade instead of misreporting: when the slot directory cannot be created, say so and fall through, rather than returning the code that means "timed out". A gate that cannot run must not claim it ran and lost a race. ## Tests `scripts/test-push-gate-lock.sh` gains coverage for the linked-worktree case (`.git` as a file), committed RED first (`03366c24e`) and then GREEN (`5b36d1eb0`). --------- Co-authored-by: investigator <investigator@gascity.local>
…ktree (gastownhall#4684) ## Problem Five architectural guard tests walk the repository looking for violations. Each one skips `.git` with an unguarded `SkipDir` check that also matches the **walk root itself**. In a linked worktree `.git` is a *file* at the root, so the guard matches at the very first entry and returns `filepath.SkipDir` for the root — ending the walk immediately. The test then finds zero violations and passes. So in any git worktree these five guards are **silent no-ops**: they report green without having examined a single file. Every agent working in a linked worktree — which is how the fleet works — has been running them for nothing. ## Affected guards | file | guards | |---|---| | `internal/beads/boundary_test.go` | bd-exec boundary violations | | `internal/beads/contract/identity_test.go` | identity contract | | `internal/builtinpacks/registry_test.go` | builtin pack registry | | `internal/logutil/walkthrough_urls_test.go` | walkthrough URL policy | | `internal/pgauth/no_external_env_test.go` | external env access | ## Fix Guard the skip with a `path != root` condition so the walk root is never itself skipped. `internal/testenv/lint_test.go`'s `isNestedWorktreeRoot` is the reference shape. ## Tests `TestFindBdExecViolationsScansWorktreeRoot` added, committed RED first (`0e6b9f4ca`) then GREEN (`57795dd08`) — the new test fails on the pre-fix tree, confirming the guards really were returning early. Each of the five sites was fixed with the same `path != root` guard rather than one-off patches, so the class is closed rather than the instance. --------- Co-authored-by: investigator <investigator@gascity.local>
…ownhall#4487) ## What An autonomous/promptless restart runs the SessionStart prime hook but **not** the UserPromptSubmit mail hook, so it starts blind to unread mail (including `priority:1` restart handoffs). This folds unread mail into the SessionStart payload via `primeInjectMailContent`, reusing the check path's provider open, self-recipient resolution (`defaultMailIdentityCandidates`), and `formatInjectOutput` (the same priority sort as the companion priority-sort PR). ## Safety Defense-in-depth and read-only: it never archives/mutates mail (that stays the check path's job) and degrades silently to `""` on any error or an empty inbox, so a prime is never blocked. ## LOC breakdown | Category | +/− | |---|---| | Production (`cmd_prime.go`) | +30 / 0 | | Tests (`cmd_prime_test.go`) | +47 / 0 | | Docs | 0 | | Generated | 0 | Bead: `dip-bj7pgj` --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gastownhall#4701) ## Summary - `gc sling --on <formula>` on a bead that is already routed+claimed (assignee set) but has no molecule attached correctly stays idempotent (by design, per gastownhall#4204 — never re-attach onto a worker's in-progress bead), but silently did so: the CLI printed only the generic `"already routed to X — skipping (idempotent)"` message, with no indication the requested formula was never attached or that `--force` would override it. - `onFormulaNeedsAttachment` now returns a small `attachmentDecision` struct instead of a bare `bool`, distinguishing "nothing to do" (molecule already present) from "skipped because another worker claimed this bead" (`SkippedForClaim`). - `resolveIdempotentShortCircuit` gains a switch case for `SkippedForClaim` that appends an explicit bead warning naming the bead, the claiming assignee, the skipped formula, and `--force`. `printSlingWarnings` already prints `BeadWarnings` unconditionally, so this surfaces on stderr with no `cmd_sling.go` changes needed. - The claimed-bead idempotency behavior itself is unchanged by design — only the missing observability around that choice is fixed. Fixes ga-juszt2. Real-world incident: ga-f9udbh's `--force` re-run (wisp `ga-46ofeu`, branch `builder/ga-46ofeu`) is the exact case this warning now explains up front. ## Test plan - [x] `TestOnFormulaNeedsAttachment` (claimed-bead case extended to assert `SkippedForClaim`/`Assignee`) - [x] `TestRoutedRawBeadReadsIdempotentWhichOnFormulaMustOverride` — unclaimed routed-raw override still fires - [x] `TestOnFormulaNeedsAttachmentMoleculePresentStaysIdempotent` — molecule-present path unaffected, not a claim skip - [x] `TestOnFormulaNeedsAttachmentProbeErrorStaysIdempotent` — probe-error fail-closed path unaffected - [x] New: `TestResolveIdempotentShortCircuitWarnsWhenOnFormulaSkippedForClaim` — pins the new warning's content (bead ID, assignee, formula, `--force`) - [x] `go test ./internal/sling/...` — full package, pass - [x] `go test ./cmd/gc/...` — full package (incl. `TestDoSlingIdempotentWithOnFormula`, the pre-existing test covering this exact claimed+idempotent+`--on` scenario), pass - [x] `go vet ./...`, `gofmt -l` — clean - [x] pre-commit and pre-push hooks (lint, fast local suite) — clean Co-authored-by: investigator <investigator@gascity.local>
… collision (gastownhall#4724) ## Summary - `assert_bead_still_claimed()` in `scripts/push-ownership-guard.sh` bound a local named `status`, which is a read-only zsh special parameter (linked to `$?`, alongside `$pipestatus`). Since this function is sourced into the deployer's ambient zsh shell (via `rebase-resolve-lib.sh`), the assignment aborted the function with `read-only variable: status` instead of returning a clean nonzero status — silently defeating the guard's fail-closed contract under zsh. - Renamed the local to `bead_status`; no behavior change under bash. - Added zsh-mode regression coverage to `test-push-ownership-guard.sh` (`run_guard_zsh` helper + `test_allow_clean_claim_under_zsh`), skipping gracefully when zsh isn't on PATH. - Checked the other 3 sites the bead flagged as "same pattern, not yet confirmed broken" (`test-push-ownership-guard.sh:136`, `:468`, `smoke-macos.sh:95`) — all have their own bash shebangs and are never sourced anywhere in the repo, so they're not exposed to ambient-zsh invocation. Left untouched per the bead's explicit scope boundary. Fixes ga-xi7wi6. Discovered as a second, independent zsh incompatibility while unblocking ga-ql4bmm's deploy gate (ga-g1mlel hit this after ga-ab7pgm's BASH_SOURCE fix cleared the first one). ## Test plan - [x] TDD: new zsh test written first, confirmed RED against the unfixed script (`read-only variable: status`, pass=20 fail=1), then GREEN after the fix (pass=21 fail=0) - [x] `shellcheck` clean on both changed files - [x] `go build ./... && go vet ./...` - [x] `go test ./scripts/... -run TestPushOwnershipGuard -v` - [x] `go test ./internal/testpolicy/resourcecensus/...` (no baseline bump needed — the new `zsh -c` call is nested inside a script already wrapped by one pre-existing Go-level `exec.Command`) - [x] `make test-fast-parallel` — 9/9 jobs green Co-authored-by: investigator <investigator@gascity.local>
…reads (gastownhall#4644) ## What this changes When routed but unassigned work targets an asleep on-demand named session, the controller now wakes the existing canonical alias holder instead of spawning a pool standby that cannot acquire the same alias. The wake decision uses raw pre-suppression routed demand, so alias suppression can prevent the redundant standby without erasing the signal needed to wake the holder. The push ownership guard now retries transient `bd list` and `bd show` read failures before blocking a push. Reads remain bounded and fail closed after three attempts by default; a parseable ownership change still blocks immediately, and the diagnostic recommends retrying before naming `--no-verify` as a last resort. ## Review notes - `NamedSessionRoutedDemand` is wake-only. It does not feed pool sizing, merge into assignee demand, or override sleep suppression. - `POG_READ_ATTEMPTS` is environment-overridable and defaults to 3; the existing per-attempt timeout is unchanged. - Retry wraps only the two ownership reads and does not weaken `POG_DISABLE`, ownership parsing, or the force-with-lease path. - There are no configuration migrations or public API changes. ## Test plan - [x] Run the six focused alias-holder, pool-suppression, awake-set, and end-to-end reconciler regressions. - [x] Run `scripts/test-push-ownership-guard.sh` and verify transient recovery, retry exhaustion, and genuine ownership-change blocking (`26/26`). - [x] Run `go build ./...`, `go vet ./...`, and the serialized eight-shard `make test-fast-parallel` baseline. - [x] Release gate: [`release-gates/ga-tg4m6s-named-session-routed-demand-push-guard-retry-gate.md`](release-gates/ga-tg4m6s-named-session-routed-demand-push-guard-retry-gate.md) <!-- mpr:issue-refs v1 --> --- 🔗 **Maintainer cross-reference** — added by the gascity maintainers, no action needed from you: - Related to gastownhall#3914 — touches the same singleton-pool / same-template named-session alias self-collision: canonicalSingletonAliasHeldTemplates now treats an asleep named holder — and one whose identity differs from its backing template — as still owning the canonical alias, so no redundant pool standby is minted to park on pool_alias_conflict. It does not touch the identity-normalization deferral path, so the per-pass deferral log line and conflict-metadata write described in that issue remain. <sub>Linked for triage visibility — not auto-closing. If this looks off, just delete this block.</sub> <!-- /mpr:issue-refs --> --------- Co-authored-by: investigator <investigator@gascity.local>
gastownhall#4722) ## Summary Part of the `ga-klo4gz` ambient-city-discovery test migration (mayor sequencing ruling in mail `gm-wisp-0zj3zar`). Batch 5/N: adds `t.Setenv("GC_CITY_PATH", dir)` immediately after the existing cwd setup in 7 sites across 4 files, so `resolveCity()` resolves via the explicit-env step instead of the ambient upward-walk step (step 10) that `ga-klo4gz`'s guard will refuse in test binaries. - `cmd_order_test.go`: `TestOpenCityOrderStoreUsesProviderAwareStore` - `cmd_pack_commands_test.go`: `TestNewRootCmdExposesRootPackCommands` - `provider_store_resolution_test.go`: `TestOpenRigAwareStoreUsesScopeLocalFileStore`, `TestCmdOrderHistoryUsesProviderAwareCityStore`, `TestCmdOrderRunExecSkipsStoreOpenForScopedFileProvider`, `TestCmdOrderRunFormulaUsesProviderAwareCityStore` - `cmd_session_test.go`: `TestSessionListProviderConstructionFailureReturnsThroughRun` needed a different fix — it drives a subprocess that re-invokes the `gc.test` binary, and that subprocess's own `TestMain` unconditionally wipes `GC_CITY`/`GC_CITY_PATH`/`GC_CITY_ROOT` via `clearProcessLiveEnvForTests()` before the test body runs, regardless of how the parent set `cmd.Env`. CLI flags survive that clearing where env vars can't, so the fix prepends `--city .` to the subprocess's argv instead. This is a newly-observed failure category (env-clearing defeats the standard env fix for subprocess-driving tests) beyond the guard-blind/guard-false-fail/true-ambient-conflict taxonomy from earlier batches. `cmd_bd_test.go`'s `TestGcBdUsesEnclosingRigWhenNoFlag` is **deliberately left unmigrated**: its purpose is to verify ambient rig-from-cwd discovery itself (it explicitly blanks `GC_CITY_PATH`), so it needs a semantic rewrite rather than a mechanical env fix. Left for the guard's own landing PR, per the existing `TestResolveCityFlag`/`TestRigAnywhere_ResolveContext` precedent from earlier batches. Guard itself is **not included** — reserved for its own PR per the mayor's sequencing ruling. Remaining after this batch: `cmd_commands_test.go` (~8 sites) and `cmd_sling_test.go` (~10 sites). ## Test plan - [x] `go build ./...` clean - [x] `go vet ./cmd/gc/...` clean - [x] Both-ways-green: full `cmd/gc` suite passes with the canary guard (`isTestBinary()` gate on `resolveContextFromDir` step 10) applied locally and uncommitted - [x] Full `cmd/gc` suite passes clean on plain `origin/main` + this diff alone (294.208s, 0 failures) - [x] Pre-push fast suite (`scripts/test-local-parallel fast`) passed Co-authored-by: investigator <investigator@gascity.local>
…adiness-aware (gastownhall#4726) Fixes gastownhall#4725. ### What The `in_progress + assigned (crash recovery)` work-query tier returned a candidate without consulting gate or dependency state, and short-circuited ahead of the ready-gated tier. A step held by an open human gate or an unclosed blocking dependency was therefore re-served to a worker on every hook tick, forever. Both tiers carrying that query are fixed together: `standardAssignedInProgressWorkQueryScript` and `legacyControlAssignedInProgressWorkQueryScript`. ### How A new `inProgressBlockedByEnrichmentScript` re-fetches the candidate with `bd show --json` and: 1. **Skips** it when any ready-blocking blocker is not closed -- `blocks`, `waits-for`, `conditional-blocks`, matching `beads.IsReadyBlockingDependencyType`. `parent-child` and `tracks` edges never suppress, which matters because every molecule step carries one to its root. 2. **Falls through** to the ready-gated tier rather than swallowing the tick, so a session holding one blocked step can still be served its other ready assigned work. 3. **Attaches `blocked_by`** to the rows it does serve, so the existing hook-side filter (`filterUnreadyHookCandidates` -> `isDepBlockedHookCandidate`) stops being a structural no-op on this tier. The re-fetch is required rather than fastidious: `bd list --json` embeds bare edge rows with no blocker status (`{issue_id, depends_on_id, type}`), while `bd show --json` emits `{id, dependency_type, await_type, status}`. Only the latter can answer whether a blocker is still open. `bd ready` was deliberately NOT substituted for `bd list`: it excludes `in_progress` by design, so that swap would silence the churn by disabling crash recovery altogether. This is strictly narrowing -- the dispatcher serves a subset of what it served before. Nothing becomes servable that was not servable already. ### Tests New `internal/config/workquery_inprogress_blocked_test.go` executes the GENERATED SHELL against a fake `bd` on PATH, so it pins observable behaviour rather than the script's spelling (the byte-for-byte shape stays pinned by `TestWorkQueryGolden`): - `TestInProgressTierSkipsGateBlockedCandidate` -- the reported incident shape. - `TestInProgressTierSkipsDependencyBlockedCandidate` -- not gate-specific; a plain `blocks` edge suppresses identically. - `TestInProgressTierServesUnblockedCandidate` -- crash recovery still works, and the row carries `blocked_by`. - `TestInProgressTierServesCandidateWithClosedBlocker` -- an answered gate releases the step. - `TestInProgressTierIgnoresNonBlockingDependencyTypes` -- `parent-child`/`tracks`/`related`/ `discovered-from` never suppress. - `TestInProgressTierFallsThroughWhenBlocked` -- a blocked candidate does not swallow the tick. - Two matching cases for the legacy-control tier. The three "still serves" cases are load-bearing: without them, a fix that stopped the churn by serving nothing at all would look green. All four primary cases fail on stock at 0488abb and pass with this change. `TestWorkQueryGolden` goldens regenerated (12 files) for the two affected kinds across `normal`/`pool`/`legacy` × `bd104`/`bd105`. ### Verification - `go test -race ./internal/config/ -count=1` -> ok (34.8s) - `go vet ./internal/config/` -> clean - `make build` -> ok - Falsifiable both directions, on this branch: with `internal/config/workquery.go` stashed back to stock, `TestWorkQueryGolden`, `TestEffectiveAssignedInProgressQueryDefault` and the four primary new cases all FAIL; restored, all PASS. - Live end-to-end against an isolated throwaway `bd init` store, driving the **generated** shell (extracted from the regenerated golden) rather than a hand-copy: - gate open -> stock tier serves the step (`["wq-repro2-in1"]`); fixed tier returns `[]` - gate resolved -> fixed tier serves the step again, with `blocked_by` attached - `bd show --json` confirms the edge is a real open `blocks` dependency throughout. ### Not included `OpenAssignedTo` in `cmd/gc/session_reconciler.go` -- the drain-ack close gate -- is also dep-blind. It cannot reproduce this churn once the tier stops serving the row, but it can leave a session idling-but-alive on structurally blocked work. Left for a separate change; called out in the issue. --------- Co-authored-by: Jacob Hausler <jacob@hausler.cc> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…astownhall#4730) ## Summary - `TestCityRuntimeReloadDrainBoundedByTimeout` asserts `reloadConfig`'s elapsed time is bounded near the production `reloadOrderDrainTimeout` (1s) constant. - The upper-bound slop was a bare 500ms, too tight for CI scheduler contention — observed failures overshot by 159ms and 439ms on separate prior occasions. - Since elapsed time is the subject under test here (the assertion literally depends on duration), this is TESTING.md's documented exception and is not eligible for migration to the `hangBudget`/`awaitCond` pattern (unlike PR gastownhall#4638's sibling fixes). - Widened only the upper-bound jitter tolerance (500ms → 3s slop, so 1s → 4s total window). `reloadOrderDrainTimeout` itself and the lower-bound check are untouched. ## Test plan - [x] `go build ./cmd/gc/...` clean - [x] `go vet ./cmd/gc/...` clean - [x] Targeted test: 20/20 passes under idle load - [x] Targeted test: 15/15 passes under artificial 28-way CPU contention (approximating CI shard-parallel load) - [x] Full `go test ./cmd/gc/...` passes (417s, no regressions) - [x] Real pre-push gate (`make test-fast-parallel`, not `--no-verify`) passed on retry after first attempt hit an unrelated, already-tracked host-contention flake (ga-alwb9s, `TestCmdStopWallClockTimeoutBoundsDirectStop`) Closes ga-ajmj0y. Co-authored-by: investigator <investigator@gascity.local>
…astownhall#4694) Fixes nondeterministic `examples/bd/dolt` doctor-script failures. Test-only; no production code. ## Root cause Five doctor-script tests pass `GC_DOCTOR_BACKUP_STALE_S=1`. Each sets a backup's mtime to `time.Now()` and *then* execs the doctor script — so **any delay above one second** between those two steps ages a deliberately-FRESH backup past the staleness horizon and fails the assertion. Under a parallel runner that delay is routine. This explains both observed symptoms precisely: - they pass **in isolation** but fail in full runs - a **different one of the five** fails on each run It is not contention or shared state — the tests use `t.TempDir()` and there is no shared server helper. ## Fix Raises the horizon to **300s** behind a named constant carrying the reasoning. That sits far above any plausible process-startup delay and far below the **only** stale fixture in the file (`-2h`, in `TestDoctorScriptChecksBackupArtifactFreshnessPerDatabase`). Freshness *discrimination* is therefore still exercised exactly as before: the fresh backup must still not be flagged, the 2-hour-old one must still be flagged. I checked all five before choosing a uniform value — four create only fresh backups, and only that one has a stale fixture. Nothing depended on the horizon being tight. ## Platform note Surfaced on darwin, where process startup is slower; Linux CI hides it by being fast enough to usually win the race. The bug is in the test's timing assumption, not in the platform, so it can fire anywhere under load. Verified on this branch (based on `main`): the five pass together, pass 5x repeated, and the full `examples/bd/dolt` package is green.
…astownhall#4702) ## What Completes the routed-demand wake fix in this PR's branch: adds `"routed-demand"` to the idle-sleep exemption list in `ComputeAwakeSet` (`cmd/gc/compute_awake_set.go`), alongside the existing `"named-demand"` and `"work-query"` exemptions. ## Why Without this, a live named-session holder woken by `NamedSessionRoutedDemand` is immediately re-slept by the idle-sleep check on the same tick whenever its bead's `IdleSince` predates the idle timeout (the common case for a long-lived holder). This silently undoes the routed-demand wake this branch otherwise adds, and — because the branch also marks the template alias-held while asleep — removes the working alias-deferred pool-standby fallback without restoring the holder it was supposed to replace. ## Testing - New test `TestPR4644_RoutedDemandWakesAsleepNamedHolder` (subtests A/B): A passes unmodified; B reproduces the re-sleep on the unpatched branch and passes after the fix. - `go test ./cmd/gc/ -run 'Sleep|Wake|Reconcile|Drain|Awake|NamedOnDemand|NamedSession|PoolDesired|Idle' -count=1` — green. - `go vet ./...` — clean. ## Note on branching This targets `deploy/ga-tg4m6s-gate` directly (not `main`) as a stacked PR, rather than pushing straight onto that branch: the branch predates a still-undeployed push-ownership-guard fix, so a direct push false-positives on the guard's stale single-function bead resolution. Opening this as its own PR avoids bypassing that check and keeps the diff scoped to exactly this fix. <!-- mpr:issue-refs v1 --> --- 🔗 **Maintainer cross-reference** — added by the gascity maintainers, no action needed from you: - Related to gastownhall#1427 — touches the same idle-sleep exemption list in ComputeAwakeSet that the filed issue is about, adding "routed-demand" so routed named-session demand is not immediately re-slept; it does not cover that issue's assigned-work case <sub>Linked for triage visibility — not auto-closing. If this looks off, just delete this block.</sub> <!-- /mpr:issue-refs --> --------- Co-authored-by: investigator <investigator@gascity.local>
…all#4612) ## Summary - publish ACP `session/update` timestamps to a durable sidecar so a different `gc` process can read `last_active` - move sidecar I/O off the JSON-RPC dispatch loop - serialize and coalesce writes, replace the sidecar atomically, and retry transient failures - declare ACP's activity-reporting capability for existing activity-aware policies ## Problem ACP records the time of each `session/update` in the `sessionConn` owned by the process that called `Start`. A separate `gc` process constructs a new Provider with an empty connection map, so `GetLastActivity` previously returned the zero time even when the owning process had observed updates. The timestamp is useful cross-process, but publishing it inline from `session/update` handling would put filesystem latency on the only JSON-RPC read and dispatch loop. Rewriting a live sidecar in place would also allow concurrent readers to observe truncated content. ## Fix `Start` now atomically seeds a last-activity sidecar before returning successfully. Subsequent updates are offered to one per-session publisher worker: - `offer` updates in-memory state and never performs filesystem I/O - one worker serializes writes and coalesces a burst to its newest timestamp - successful writes are throttled to five-second resolution - failed writes are reported once per failure streak and retried on a bounded deadline that continuing updates cannot postpone - shutdown makes one final best-effort flush and waits for the worker before metadata cleanup, preventing a late write from recreating a removed sidecar - process exit waits for buffered stdout dispatch before that flush, preserving the last `session/update` - each durable replacement uses a same-directory temporary file and atomic rename, so readers see either the previous complete timestamp or the next one Initial publication failure makes `Start` fail rather than returning an activity-capable session without its initial durable value. `GetLastActivity` still prefers the owning process's exact in-memory timestamp, then falls back to the sidecar. A missing sidecar remains `(zero, nil)` and a malformed one is reported as an error. ## Semantics and scope The signal means only “the last time this process observed a valid `session/update` notification.” Its age does not diagnose why updates stopped and does not independently prove that a provider session or process is dead. ACP now declares `CanReportActivity`, which makes the signal available to timed idle behavior and the existing progress-stall policy. `[session] progress_stall_timeout` remains opt-in and disabled by default. This change does not add a new death detector, watchdog, or deployment-specific recovery policy. ## Validation - `go test ./internal/runtime/acp -count=1` - `go test -race ./internal/runtime/acp -count=1` - focused controller activity-policy tests, with and without `-race` - `go test ./cmd/gc -count=1` - `go vet -buildvcs=false ./internal/runtime/acp ./cmd/gc` - `go build -buildvcs=false ./...` - `git diff upstream/main..HEAD --check` Regressions cover cross-Provider reads, blocked sidecar I/O not blocking JSON-RPC response dispatch, serialized/coalesced/ordered writes, transient failure retry without update-driven starvation, close-time flush, concurrent atomic reads, initial publication failure, and Stop cleanup.
… once (gastownhall#4729) ## Summary - The re-alert loop wasn't in `write_compact_marker`'s quarantine-install path — that path is already one-shot by construction (gated once per `flatten_database()` call). The actual repeat-offender was two byte-identical "already quarantined, refuse" guard blocks in `flatten_database()` and `bare_gc_database()`, which sent an unconditional mail on **every** compact/bare-gc invocation with zero dedup — matching the "re-alerting for 23 days" symptom in ga-5trg1x. - Collapsed both duplicated guards into one shared `report_existing_quarantine` helper (fixing the bug in one place instead of two, and removing pre-existing duplication). - Split `send_compact_quarantine_alert` into `emit_compact_quarantine_event` (unconditional — still fires every cycle so downstream automation can observe every check) and `mail_compact_quarantine_alert` (gated by a new fail-open `quarantine_should_notify` check keyed on `last_notified_reason`). - Added `record_quarantine_notify_state`, which additively patches `seen_count` / `notify_count` / `last_notified_ts` / `last_notified_reason` onto the existing quarantine marker without touching any other field (including `created_at`) — mirroring the notify-once-per-distinct-state marker shape in `hold-notice-lib.sh`. - The fresh-install path in `write_compact_marker` now also calls `record_quarantine_notify_state` (alert stays unconditional there, since it's already one-shot) so the very next run doesn't fail-open and double-mail. - Out of scope, deliberately: the bead's optional "alert on compact-pending-gc too" stretch goal, and the separate `ensure_remote_push_retry_fresh` mechanism (different marker family — `pending_push`/`pending_gc`, not `quarantine`). ## Test plan - [x] New test `TestCompactScriptExistingQuarantineMarkerAlertsOnceAcrossRepeatedCycles`: three consecutive compact runs over an unchanged quarantine condition send exactly one mail, while the event still fires all three times. - [x] Verified retroactively against the original bug via `git stash` on just the source file: the new test fails (3 mails across 3 runs) against pre-fix code, passes against the fix. - [x] Full quarantine-filtered suite (20 tests) passes, including all previously-existing alert/recipient/refusal tests. - [x] Full `go test ./examples/bd/dolt/...` passes. - [x] `go vet ./...` clean. - [x] `shellcheck -s sh` on `run.sh` — no new warnings. Fixes ga-5trg1x. --------- Co-authored-by: investigator <investigator@gascity.local>
…astownhall#4738) ## What this changes `gc init` and `gc start` no longer silently use the process working directory when they are invoked without a path from non-interactive stdin. They now fail with a direct instruction to pass an explicit path, including `.` when the current directory is intentional. This prevents automation, redirected commands, and agent sessions from accidentally initializing or starting a city in an arbitrary checkout directory and leaving managed runtime state behind. Interactive no-argument use and every explicit target form remain unchanged. ## Review notes - The shared guard uses `golang.org/x/term.IsTerminal`, because a file-mode character-device check misclassifies `/dev/null` as interactive. - The new behavior applies only when both conditions hold: no explicit target was supplied and stdin is not a real terminal. Explicit path arguments and `gc start --city` bypass the guard. - A small internal `cmdInitWithOptions` signature cleanup removes two parameters that were always constant at that wrapper boundary. There is no public flag, config, API, or migration change. ## Test plan - [x] Run the focused unit matrix for interactive, non-interactive, explicit-path, file-init, directory-init, and `--city` resolution. - [x] Build the CLI and smoke `gc init` with `/dev/null` and piped stdin, `gc start` with `/dev/null`, and explicit-path `gc init --no-start` in a disposable directory. - [x] Run `go build ./...`, `go vet ./...`, `make lint-new`, and `make test-fast-parallel` with zero regressions. - [x] Release gate: [`release-gates/ga-7vhfyj-cwd-fallback-guard-gate.md`](release-gates/ga-7vhfyj-cwd-fallback-guard-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
## What this changes Makes the file-recorder rotation conformance test reliable on slow or contended disks. The test previously used one 10-second context for watcher setup, rotation fsync/rename work, archive compression and reaping, and the later reads. Enough setup I/O could therefore expire the context before the watcher tried to read events that were already durable. The watcher now has a cancel-only lifetime, while each blocking read gets its own fresh repository-standard timeout. Production event recording and watcher behavior are unchanged. ## Review notes - Scope is limited to `internal/events/eventstest/conformance.go`. - Both pre-rotation and post-rotation reads use the same local buffered helper. - There are no configuration, API, schema, dependency, or migration changes. ## Test plan - [x] `go test ./internal/events/... -run TestFileRecorderConformance -count=20` - [x] `go test ./internal/events/exec/... -count=1` - [x] Fresh test binary: 600 rotation-invariant stress runs, zero failures - [x] `make test-fast-parallel` - [x] `go vet ./...` - [x] Release gate: [`release-gates/ga-u7f149-rotation-conformance-timeout-gate.md`](release-gates/ga-u7f149-rotation-conformance-timeout-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
…astownhall#4734) ## Summary - Swaps the raw `time.After(3*time.Second)` literal in `gatedStartProvider.waitForStarts` (`cmd/gc/session_lifecycle_parallel_test.go`) for the shared `hangBudget` constant, matching the sibling helpers `gatedStopProvider.waitForStops`/`waitForInterrupts` already migrated by gastownhall#4638. - Test-only change, no production code touched. ## Why this PR exists now ga-hp32mm was reviewed PASS (verdict recorded 2026-07-25) but the bead was closed on "branch pushed" without a PR ever being opened, leaving the work stranded and untracked. The mayor flagged this explicitly on 2026-07-26 (06:35 PT, on ga-hp32mm's own notes): *"CLOSED BUT NOT LANDED... NEEDED: open a PR from builder/ga-hp32mm (do not cherry-pick to main)."* This PR is that explicitly-requested action, surfaced during the ga-mlyy1g stale-molecule triage sweep. Not auto-merging — landing follows the normal review/merge process. ## Evidence - RED: `a6b6017a8` — `TestGatedStartProviderWaitForStartsSurvivesDelayPastOldFixedDeadline`, confirmed failing against the raw 3s literal. - GREEN: `212cd3010` — swap to `hangBudget`; new test + `TestCmdStopForceEscalatesInProgressControllerStop` + `TestCmdStopWaitsForStandaloneControllerExit` pass `-race -count=5` (5/5); full `make test-fast-parallel` all 8 shards clean. - Reviewer verdict (gascity/reviewer, independently re-verified in a throwaway worktree): PASS. `go vet`/`gofmt` clean, targeted race tests 10/10 pass, full `test-fast-parallel` rerun clean. - Confirmed today: `212cd3010` is still not an ancestor of `origin/main` — the strand is real and current, not stale. ## Test plan - [x] `go test ./cmd/gc/... -run 'TestCmdStopForceEscalatesInProgressControllerStop|TestCmdStopWaitsForStandaloneControllerExit'` -race -count=5 (reviewer-verified, 10/10 pass) - [x] `make test-fast-parallel` full run, all 8 shards (builder + reviewer both verified clean) - [x] `go vet ./...`, `gofmt` clean Refs: ga-hp32mm, ga-np3ni9 (molecule root), ga-mlyy1g (triage sweep that surfaced this) --------- Co-authored-by: investigator <investigator@gascity.local>
…astownhall#4745) ## What this changes This migrates 31 asynchronous hang guards in `cmd/gc/controller_test.go` from raw sub-10-second timers and hand-rolled polling loops to the existing `awaitClose` and `awaitCond` helpers. Controller tests now use the repository's shared scheduling-aware hang budget, reducing false failures under CPU-starved CI while preserving the same completion assertions. Four deliberately short timers remain unchanged because they define test inputs, negative-assertion windows, or a bounded best-effort probe rather than detecting a hung operation. Each now has a specific exclusion comment. ## Review notes - Production code and runtime behavior are unchanged; this is test infrastructure only. - The checked fixed-sleep resource census drops by six calls, exactly matching the six removed polling sleeps. - The `hangBudget` definition and `cmd_stop_test.go` are intentionally untouched. - New lint tests prevent raw controller hang deadlines from returning or being mixed with `hangBudget` in one test function. ## Test plan - [x] `go build ./...` and `go vet ./...` - [x] Focused controller deadline lint tests - [x] Live resource-census/documentation synchronization test - [x] `make test-fast-parallel` — all 9 shards pass - [x] Release gate: [`release-gates/ga-jhs26o-controller-hang-deadline-migration-gate.md`](release-gates/ga-jhs26o-controller-hang-deadline-migration-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
…wmtr) (gastownhall#4761) ## Summary - `_pog_resolve_bead_id` in `scripts/push-ownership-guard.sh` now prefers the live in-progress assignee over the closed gated bead when resolving a `deploy/*-gate` branch name, instead of trusting the branch-embedded bead ID (which is routinely already closed by push time). Fixes a real cited incident (PR gastownhall#4731 incorrectly blocked). Downgrades the resulting disagreement log line from WARNING to NOTE. - Source bead: ga-wwswme. Review bead: ga-uq9095 (PASS). Deploy bead: ga-anwmtr. - Reviewed commit: `b7e762eaf1eeaaca876d1c14dd63c45777d442ec`. ## Deploy gate PASS on all 7 criteria — full record in `release-gates/ga-anwmtr-gate.md` (this branch's final commit). Highlights: - `shellcheck`, `go build ./...`, `go vet ./...` all clean. - `bash scripts/test-push-ownership-guard.sh`: 28/28, including the new regression `resolve/deploy-gate-branch-prefers-live-assignee`. - `make test-fast-parallel`: 9/9 shards green on a fresh full run (an earlier gate attempt on this same reviewed commit saw a `unit-core` flake unrelated to this diff; this retry's clean full rerun did not reproduce it — see gate record Summary for the fleet-flake cross-reference). - Branch merges cleanly against current `main` (verified via `git merge-tree` + `git diff --check`, no conflicts). ## Merge Per the deploy-bead instructions, this PR is not self-merged — routing a merge request to mayor/mpr. ## Test plan - [x] `shellcheck scripts/push-ownership-guard.sh` - [x] `go build ./...` - [x] `go vet ./...` - [x] `bash scripts/test-push-ownership-guard.sh` (28/28) - [x] `make test-fast-parallel` (9/9 shards, fresh full run) --------- Co-authored-by: investigator <investigator@gascity.local>
…4750) ## What this changes The pre-commit hook now refuses to commit a staged `internal/api/openapi.json` when npm is unavailable and the generated TypeScript client cannot be refreshed. This covers both a spec staged directly by a contributor and the more subtle case where a Go-only change causes the hook's own `genspec` step to regenerate and stage the spec. Unrelated commits still receive the existing npm warning without being blocked. The contributor guidance now points to the current dashboard location and identifies `make dashboard-ci` as the API/dashboard gate that regenerates the client and detects drift. ## Review notes - Both npm paths consume one fresh post-generation `spec_changed` read, avoiding divergent snapshots of the index. - This intentionally tightens pre-commit behavior only when an OpenAPI change would otherwise ship with a stale generated client. - There are no runtime API, configuration, endpoint, or migration changes. - The resource-census increase is exactly two subprocess call sites added by the end-to-end regression fixture. ## Test plan - [x] End-to-end regression for Go-only staging, `genspec` side-effect staging, and npm absent - [x] Direct-spec fail-closed and unrelated-change warn-only boundary tests - [x] Full scripts, resource-census, and docsync packages - [x] `go build ./...` and `go vet ./...` - [x] `make test-fast-parallel` — all nine jobs passed - [x] Release gate: [`release-gates/ga-x86bjw-precommit-openapi-npm-fail-closed-gate.md`](release-gates/ga-x86bjw-precommit-openapi-npm-fail-closed-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local> Co-authored-by: quad341 <quad341@users.noreply.github.com>
## What this changes The sharded local test runner now accounts for current host load when choosing its outer job budget, then divides that budget across concurrent `go test` jobs through `GOFLAGS=-p=<n>`. This prevents each shard from independently using the full Go package-concurrency default and oversubscribing an already busy machine. The fast and full modes also run a direct shell self-test covering the load floor, CPU override, malformed inputs, arithmetic boundaries, and runner wiring. ## Review notes - Load reduction is floored so automatic sizing cannot collapse below the safe minimum, and small machines skip the load adjustment. - Explicit local CPU overrides retain precedence and deterministic contract tests pin load to zero where load is not the behavior under test. - `-p` bounds cross-package/build concurrency only; this change deliberately does not alter within-package `t.Parallel()` fan-out. - There are no runtime, configuration-schema, CI-workflow, timeout, coverage, or resource-ledger changes. ## Test plan - [x] Ten focused runner-budget and environment-allowlist subtests - [x] `scripts/test-local-concurrency.sh` — 25/25 assertions - [x] Full `go test ./scripts/...` - [x] `go build ./...` and `go vet ./...` - [x] `make test-fast-parallel` — all ten jobs passed under the new inner cap - [x] Release gate: [`release-gates/ga-i6a6ds-local-test-concurrency-cap-gate.md`](release-gates/ga-i6a6ds-local-test-concurrency-cap-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
gastownhall#4766) ## Summary Part of the `ga-klo4gz` ambient-city-discovery test migration (mayor sequencing ruling in mail `gm-wisp-0zj3zar`). Batch 4/N: adds `t.Setenv("GC_CITY_PATH", dir)` immediately after the existing cwd setup in 6 sites across 3 files, so `resolveCity()` resolves via the explicit-env step instead of the ambient upward-walk step (step 10) that `ga-klo4gz`'s guard will refuse in test binaries. - `root_argv_test.go`: `TestRootConstructionUsesInjectedArgsInsteadOfAmbientOSArgs`, `TestNewRootCmdCompatibilityWrapperNeverConsultsAmbientArgs` - `cmd_graph_test.go`: `TestOpenRigAwareStoreUsesProviderAwareRigStore`, `TestOpenRigAwareStoreLegacyFileCityUsesSharedCityStore` - `metrics_lifecycle_test.go`: `TestProductMetricsLifecycleRealPackDispatchMatrix`, `TestProductMetricsLifecycleConfigChangeFallbackReportsBeforeInvoke` All 6 sites use incidental city-setup infra (`setupPackExitCity`/`setCwd`/plain `os.Chdir` + `t.TempDir`), not the discovery mechanism under test — confirmed guard-blind by reading each site. Guard itself is **not included** — reserved for its own PR per the mayor's sequencing ruling. **Provenance note:** this diff (commit `e002dd201d961a914a1521f6aefad877566d21ab`) was built and pushed to this branch on 2026-07-26 as part of `ga-klo4gz.5`, which closed with "TDD build complete" but never had a PR opened — it fell out of the review queue. Discovered and opened now while re-measuring blast radius for batch 6 (`ga-klo4gz.7`); the diff itself is unchanged from what `ga-klo4gz.5`'s notes recorded as both-ways-green. `git merge-tree` against current `origin/main` shows a clean merge (no conflicts) despite main having advanced since the branch was cut. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./cmd/gc/...` clean - [x] Both-ways-green (verified 2026-07-26 per `ga-klo4gz.5` notes): full `cmd/gc` suite passes with the canary guard (`isTestBinary()` gate on `resolveContextFromDir` step 10) applied locally and uncommitted - [x] Full `cmd/gc` suite passes clean on plain `origin/main` + this diff alone - [x] `git merge-tree --write-tree origin/main e002dd2` clean (re-verified 2026-07-27, no conflicts against current main) Co-authored-by: investigator <investigator@gascity.local>
gastownhall#4767) ## Summary Part of the `ga-klo4gz` ambient-city-discovery test migration. Batch 6/N: adds `t.Setenv("GC_CITY_PATH", dir)` immediately after each test's existing cwd/temp-city setup across 2 files, so `resolveCity()` resolves via the explicit-env step instead of the ambient upward-walk step (step 10) that `ga-klo4gz`'s guard will refuse in test binaries. - `cmd_sling_test.go` (6 sites): `TestCmdSlingUsesRigScopedFileStoreForBuiltInRouting`, `TestCmdSlingDefaultFormulaDoesNotMaterializePoolSession`, `setupCmdSlingBeadExistsFixture` (shared helper), `TestCmdSlingInlineBeadRigScopedBdProvider`, `TestCmdSlingInlineBeadBareTargetFromRigCwdBdProvider` (also had to stop discarding `setupRigScopedBdCity`'s city-dir return value via `_`, since the test only chdirs into the rig subdir, not the city root), `TestCmdSlingForceMissingBeadPrintsAutoConvoyWarning`. - `cmd_commands_test.go` (5 sites): `TestE1LazyMissingTreeMatchesEagerFlagOwnership`, `TestE1EagerLazyControlDifferentialMatrix`, `TestE1ScopeLookingArgsAfterLeafPassThrough`, `TestTryPackCommandFallbackReturnsTypedNonzeroOutcome` — straightforward env fix. `TestPackCommandExitHelper` needed a different fix: it's the re-exec entry point for 3 subprocess-driving tests (`exec.Command(os.Args[0], ...)`), and that subprocess's own `TestMain` unconditionally scrubs `GC_CITY_PATH` via `clearProcessLiveEnvForTests()` before the test body runs, regardless of how the parent set `cmd.Env` — so the override has to be restored *inside* `TestPackCommandExitHelper` itself (derived from `os.Getwd()`, since `cmd.Dir` already pins the child's cwd to the intended city) rather than threaded through the parent's env/argv. Same failure category `cmd_bd_test.go` hit in batch 5 (env-clearing defeats the standard env fix for subprocess-driving tests), just requiring the fix at the re-exec entry point instead of via `--city`. `TestE1PreLeafBooleanHelpNoScopeEager` is **excluded**: true ambient conflict (its whole point is exercising the no-explicit-scope path), so it fails identically with or without this migration and is left for the guard's own landing PR. Guard itself is **not included** — reserved for its own PR per the mayor's sequencing ruling (mail `gm-wisp-0zj3zar`). Full-suite canary validation surfaced 7 more out-of-scope failures beyond this batch's target files, filed as `ga-klo4gz.8` (batch 7/N): `cmd_graph_test.go`, `metrics_lifecycle_test.go`, `root_argv_test.go`, and a residual `main_test.go` site. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./cmd/gc/...` clean - [x] Both-ways-green: full `cmd/gc` suite passes with the canary guard (`isTestBinary()` gate on `resolveContextFromDir` step 10) applied locally and uncommitted (12 failures, all reconciled against known exclusions + `ga-klo4gz.8`) - [x] Full `cmd/gc` suite passes clean on plain `origin/main` + this diff alone (293.163s, 0 failures) - [x] Pre-push fast suite (`scripts/test-local-parallel fast`) passed Co-authored-by: investigator <investigator@gascity.local>
…stownhall#4773) ## Summary Fixes 2 of the 5 tests in ga-f5clwo (wall-clock-bound tests producing false reds under load) by correcting their assertion SHAPE, per the bead's explicit "not a request to bump timeouts" constraint. - **`TestCityRuntimeReloadDrainShortCircuitsOnTickContextCancel`**: the elapsed check duplicated as a latency SLO what the very next line already proves structurally (`errs[0]==context.Canceled`, captured synchronously at entry by the test fake regardless of scheduling). Loosened to `hangBudget` — it now only guards against the short-circuit regressing into an indefinite block. - **`TestControllerStateCreatedAgentVisibleAfterStaleRuntimeInterleaving`**: the outer `context.WithTimeout` is pure hang-detector shape (test never measures elapsed time). Migrated to `hangBudget`. Left the sibling 100ms negative-assertion window in the same function untouched — it's the substance of that assertion, not a wait to be budgeted. Both changes follow the established shape/rationale from gastownhall#4745 and gastownhall#4734 (input vs. observation vs. negative-assertion framework). The remaining test in ga-f5clwo's original 5 (`TestResolveDoltConnectionTargetManagedCity_EnvOverride`) doesn't fit this bead's shape — zero wall-clock assertions in the test itself, the flake is a hardcoded 250ms `net.DialTimeout` in production code with no injection seam. Split out to ga-qro1xt. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./...` clean (full repo) - [x] `make test-fast-parallel` — all 10 jobs pass (twice: once locally, once via push-gate hook) - [x] Target tests pass at `-count=1` under normal conditions - [x] Target tests + sibling `TestCityRuntimeReloadDrainBoundedByTimeout` run 18/18 clean under synthetic single-core CPU starvation (test process + 12 burn loops taskset-pinned to one core), 2.5-3.9s each, well inside `hangBudget` (60s) Refs: ga-f5clwo, ga-h51wa1, ga-o2bak3 Co-authored-by: investigator <investigator@gascity.local>
…ll#4777) ## What this changes The lifecycle example pack now reapplies its worktree provisioning whenever `worktree-setup.sh` encounters an existing worktree. Worktrees created manually, by an older script, or with later-clobbered metadata now converge on the expected `.beads/redirect`, submodule initialization, and local Git excludes during the next agent startup. Previously, the existing-worktree fast path returned before any of that provisioning ran, leaving agents attached to the wrong beads store or missing the local runtime excludes. ## Review notes - The existing creation-time provisioning is factored into one idempotent helper and called from both the existing-worktree and fresh-create paths. - The helper remains below the worktree existence check, so it does not create or touch the target before `git worktree add` on the fresh path. - This does not change the separately tracked non-zero exit behavior of the lifecycle example's fresh-create path. ## Test plan - [x] `go build ./...` - [x] `go vet ./...` - [x] `make test` — fast-unit baseline - [x] `make test-acceptance` — Tier A command-level PR gate, including fresh and pre-existing lifecycle worktrees - [x] Repository pre-push fast suite — all nine lanes passed - [x] Release gate: [`release-gates/ga-bgh2wi-lifecycle-worktree-provisioning-gate.md`](release-gates/ga-bgh2wi-lifecycle-worktree-provisioning-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
…ee store test (gastownhall#4778) ## Summary - `TestBdRigWorktreeStoreConsistentAcrossRawBdGcBdAndProviderStore` resolved `bd` via ambient PATH/home-dir search, which can drift to a version whose dolt schema is incompatible with what the test fixtures expect. - Builds `bd` fresh via `go install` pinned to the exact go.mod version instead, and reads the version via `runtime/debug.ReadBuildInfo()` (no subprocess) rather than `go list -m`. - Bumps 5 resource-census ledger rows (+1 subprocess / +1 slow_process_gate each) for the new `go install` call site and the new regression test's `skipSlowCmdGCTest` call, following the same-day precedent in 3be16bf. ## Provenance - Work bead: ga-r9cvmi (builder), reviewed via ga-y47bs1 — PASS, independently re-verified build/vet/gofmt clean, both regression tests green (0 skip/0 fail), census ledger self-consistent, no security/CI-integrity concerns. - Deploy-gate bead: ga-hrt3tb. This is a gate branch cut directly from the reviewed commit (13b8cc5) — no additional commits on top. - `git merge-tree --write-tree origin/main 13b8cc5` is clean (rc=0). ## Test plan - [x] Reviewer re-verification (ga-y47bs1, PASS) - [x] Local fast suite green on push (unit-core, unit-cmd-gc 1-6, fsys-darwin-compile, push-gate/local-concurrency selftests) - [ ] Mayor merge approval --------- Co-authored-by: investigator <investigator@gascity.local> Co-authored-by: quad341 <quad341@users.noreply.github.com>
## What this changes `TestRepositoryLedgerMatchesCensusAndDocumentation` now supports an opt-in `-update` mode that regenerates the checked resource-ledger block in `TESTING.md` from `test/test-resources.toml`. When the ledger is stale, the failure tells maintainers the exact command to run instead of requiring a manual table transcription. The resource-census package now shares marker validation through a single span helper and provides `ReplaceMarkdownBlock`, which replaces only the checked marker range and preserves all surrounding documentation byte-for-byte. ## Review notes - Regeneration is opt-in; ordinary test runs never write to `TESTING.md`. - The update path targets the repository's fixed `TESTING.md` location and writes only when generated content differs. - Marker validation still requires exactly one ordered begin/end pair and preserves the existing error behavior. - There are no config changes, migrations, new dependencies, or runtime production paths involved. ## Test plan - [x] Five focused resource-census acceptance tests: 5 PASS, 0 FAIL, 0 SKIP. - [x] Corrupt the checked ledger block, confirm the stale test names the regeneration command, run `-update`, and verify the file returns to its identical Git blob. - [x] `make test-fast-parallel`: 10 PASS, 0 FAIL, 0 SKIP jobs. - [x] `go vet ./...`. - [x] Release gate: [`release-gates/ga-yg3x8u-resourcecensus-ledger-generator-gate.md`](release-gates/ga-yg3x8u-resourcecensus-ledger-generator-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
…oots (gastownhall#4657) ## Summary The gastownhall#3344 retirement of `.gc/system/packs` shipped a **config-only migration**: no sink-side pass ever re-pointed or removed the pre-manifest symlinks that targeted it. The cleanup walk's ownership gate (`OwnedRoots` + the gastownhall#4130 manifest) classifies those orphans as user-owned forever — so every sink written before gastownhall#4130's manifest keeps serving (or skipping on) dead links indefinitely, and `gc doctor` has no check that surfaces them. **Forward-port note:** gastownhall#3647 merged into `release/v1.3.0` at 22:24 UTC on June 21, after the final reconciliation-branch commit for gastownhall#3589 at 21:06 UTC. The release back-merge reached `main` at 00:26 UTC but did not contain gastownhall#3647, and no later release→main reconciliation was opened. The fix remains present in every v1.3 tag from v1.3.2 through v1.3.5 and was never reverted. This PR therefore forward-ports gastownhall#3647 as its first commit (authorship preserved) before the `LegacyOwnedRoots` fix; there is no external merge dependency. The second commit is the complete new behavior introduced by this PR. ## Fix **`Request.LegacyOwnedRoots`** (internal/materialize/skills.go): targets under *retired* gc-managed roots are recognized as gc's own stranded property, never user content. Safety matrix: | link state | name desired | name not desired | |---|---|---| | target under legacy root, dangling | atomic re-point at current source | **delete** | | target under legacy root, live | atomic re-point at current source | **leave alone** (may be serving content) | Both `materialize.Run` callers (stage-1 supervisor, `gc internal materialize-skills`) pass `LegacyOwnedRootsFor(cityPath)` = `<city>/.gc/system/packs` + `<GC_HOME>/cache/repos` (a pruned cache checkout strands pre-manifest links the same way the retired projection does). **`gc doctor` `skill-dangling-sink` check** (sibling of `skill-collision`): walks agent scope-root × vendor sinks plus live session-workdir sinks (lazily enumerated from the session store), Lstat/Readlinks every entry, and reports dangling links classified gc-owned vs user-owned via the newly exported `materialize.TargetUnderManagedRoot`, so doctor and the materializer share exactly one ownership convention. `--fix` removes only gc-owned dangling links — the next materialize pass recreates any still-desired link. Advisory severity; never gates. ## Testing - 6 new materializer tests (delete dangling / re-point desired dangling / re-point desired live / keep live undesired / no-opt-in preserves historical behavior / cache-root variant) — green. - 6 new doctor check tests (clean, missing sink, classify, fix-only-gc-owned, lazy live sinks, dedupe) — green. - Full `internal/materialize` + `internal/doctor` packages green; `cmd/gc` Skill|Materialize|Doctor tests green (incl. updated `doctor_check_names.golden`); `go vet ./...` clean. - E2E on a synthetic city: `gc doctor` flags 2 dangling (1 gc-owned), `--fix` removes only the gc-owned link, user-owned link untouched. - One unrelated pre-existing failure on macOS (`TestDoctorStoreFactoryUsesExplicitCityForRigOutsideCityTree`, /var↔/private/var alias) reproduces on clean `main`. --------- Co-authored-by: Julian Knutsen <julianknutsen@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… defer backstop (gastownhall#4630) ## Summary Adds an **assigned-work defer rung** to the idle-kill ladder, with a same-bead consecutive-defer backstop so a stuck agent is still recyclable. `DecideIdleTimeout` (`internal/session/lifecycle_timers.go`) previously had no `AssignedWork` rung — its own doc comment stated *"Idle stops never consult assigned work."* `DecideMaxSessionAge` in the same file already had one. The result was an infinite kill/wake treadmill: the idle-kill call site stopped a session that `ComputeAwakeSet` immediately marked awake again for the **same unchanged assigned-work reason, on the same tick** (the code's own comment reads *"Mark for immediate re-wake on this same tick"*). Reproduced fleet-wide on 4 templates; roughly 3 min and ~136K context burned per cycle. Implements **Option A** from the architecture decision in `ga-mxwj4g` (verdict: A + C). Option C is a separate sibling bead; the two are independent and can land in either order. ### What changed - `DecideIdleTimeout` gains an `AssignedWork` rung mirroring `DecideMaxSessionAge`'s existing switch: `AssignedWorkUnknown` → `TimerActionGatherAssignedWork`; `AssignedWorkHas` → defer. - The idle-kill call site (`cmd/gc/session_reconciler.go`) sources the fact via `sessionHasAwakeAssignedWorkForReachableStore` — deliberately **not** the cruder `sessionHasOpenAssignedWorkForReachableStore` that max-age uses. The "awake" helper excludes not-ready (deferred/blocked) work, which is the correct "is there something actionable right now" semantics for idle-kill. - **Same-bead consecutive-defer backstop:** the caller tracks consecutive assigned-work defers per (session identity, anchor bead ID) and forces the Stop path once a configurable threshold is exceeded, recorded under its own traceable reason so operators can tell "no assigned work" stops apart from "exhausted defer budget" stops. `DecideIdleTimeout` stays a pure decider with no hidden state. - Constraints honored: no tick reorder, no new persisted cross-tick fact shared with `ComputeAwakeSet`, `compute_awake_set.go` untouched, and `internal/session` stays side-effect-free (all store queries and tracker state live in `cmd/gc`). ## Scope Single-purpose. Three commits: | Commit | Scope | | --- | --- | | `02f5e8163` | RED proof for the idle-kill vs awake-set treadmill (`ga-3ox7rk`) | | `be7ef9854` | The fix — idle-kill ladder consults assigned work, with defer backstop | | `ad875a9d3` | Regenerated spec, schema docs and client for `AssignedWorkDeferLimit` | > **Body corrected 2026-07-28 (mayor).** An earlier version of this description > described a *five*-commit bundle and asked the reviewer to judge the bundling. > That was accurate before the rebase and is not accurate now: the rebase dropped > `fc6ac52d5` (provider-factory test isolation) and `27470a338` (the census > ratchet that existed only to cover it). Verified on the current head — > `git diff --name-only origin/main...` returns **zero** files under > `internal/testpolicy/` and no provider-factory test. The bundling question the > old body raised no longer exists, so it has been removed rather than left for a > reviewer to re-derive. A stale description becomes the durable merge record. ## Testing - Primary acceptance: `cmd/gc/session_idle_kill_wake_treadmill_test.go` assigned-work assertions go GREEN (was RED). The RED proof asserts the cross-engine invariant — a session `ComputeAwakeSet` holds awake must not be idle-killed — rather than merely asserting the new rung exists, and primes `facts.AssignedWork` so the assertion cannot go vacuous. - Named-session variant case added (`assigned-work/named-session-identity`), closing a gap self-flagged in the bead notes. - Tracker unit coverage: default fallback, anchor-change reset, explicit reset, per-name-over-template precedence, template fallback, exemption→default, `setLimit(0)` clear, cross-session isolation. - Reconciler integration: force-stop after limit, reset on anchor change, reset on non-defer outcome — driving the real `reconcileSessionBeadsTraced` path. - `go vet ./...`, lint, `test/docsync`, and full `make dashboard-check` green via the pre-commit gate. CI on this head: green. ## Rebased onto current `main` — conflicts resolved by regeneration This branch previously showed 112 conflicts against `main`, **all** under `internal/api/dashboardspa/dist/` (generated Vite output, `rename/rename` on content-hash churn) with **zero** conflicts in authored source. Generated artifacts are not merged — they are regenerated. Recovered 7 generated artifacts a naive rebase silently dropped: `internal/api/openapi.json`, `internal/api/genclient/client_gen.go`, `docs/reference/config.md`, `docs/reference/schema/city-schema.{json,txt}`, `docs/reference/schema/openapi.{json,txt}`. The `AssignedWorkDeferLimit` Go source survived intact, so these were **regenerated, not copied**: `make generate` → `go run ./cmd/genspec` → `make spec-ci`. Output matches the pre-rebase branch byte-for-byte (`openapi +8`, `client_gen 1803908 → 1803983`), confirming the regeneration reproduces the same spec. Without this the PR would have shipped a spec desync. `zod.gen.ts` gains a runtime schema line for `AssignedWorkDeferLimit`, so the SPA bundle genuinely changes — it is not a type-only, erased-at-compile change. ## Known scope boundary This implements **Option A** from `ga-mxwj4g` (verdict A + C). Option A bounds unbounded deferral; it does not eliminate the treadmill on its own. Where a bead permanently reads as awake-assigned-work — the `ga-3ox7rk` reproducer, a `deferred` bead with NULL `defer_until` erased to `open`+`ready` upstream — the forced stop re-wakes on the same tick, so the cycle becomes `idle_timeout + N ticks` rather than being removed. Option C removes the upstream status erasure that creates the permanent demand and is a separate sibling bead. ## Why this is only being opened now This branch sat complete on `origin` with no PR ever opened. Its bead (`ga-nllza6`) is **closed**, and a closed bead reads as "done" to every agent and every sweep — so nothing picked it up. Opened explicitly at the mayor's direction. Refs `ga-nllza6`, `ga-mxwj4g`, `ga-3ox7rk`. --------- Co-authored-by: investigator <investigator@gascity.local>
## What this changes Gas City now canonicalizes explicit city and rig paths as they enter `gc`. Paths supplied through `--city`, `GC_CITY`/`GC_CITY_PATH`/`GC_CITY_ROOT`, positional city or rig arguments, and the rig-scoped `GC_RIG_ROOT`/`BEADS_DIR` environment now resolve symlinked ancestors to the same absolute path used by discovery. This prevents one physical city or rig from acquiring different string identities depending on whether an operator uses its real path or a symlink alias. Missing leaf paths retain the existing longest-ancestor normalization behavior. CLI flags, environment-variable contracts, configuration schemas, and ordinary non-symlinked paths are unchanged. ## Review notes - The implementation reuses the existing `normalizePathForCompare` boundary helper; it does not introduce another canonicalization mechanism. - The production diff is limited to three ingest points in `cmd/gc/main.go` and `cmd/gc/bd_env.go`, with focused regressions alongside them. - `--rig` and `city.toml` paths already converge through their normalized registry/config paths, so those flows are intentionally unchanged. - There is no migration, new default, endpoint, dependency, or wire-format change. ## Test plan - [x] Focused symlink, relative-input, missing-leaf, precedence, and contextual-error contracts: 9 pass, 0 fail, 0 skip - [x] `make test-fast-parallel`: 10/10 jobs pass - [x] Non-short `cmd/gc` process lane with the CI-pinned `bd`: 15,362 pass, 0 fail; 11 documented helper/platform/opt-in skips - [x] Worker phase-2 conformance for Claude, Codex, and Gemini: 78/78 requirements pass - [x] `go build ./...` and `go vet ./...` - [x] Release gate: [`release-gates/ga-jx0gqf-normalize-configured-paths-gate.md`](release-gates/ga-jx0gqf-normalize-configured-paths-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
## Summary - add an optional exact city allowlist for event export - preserve each registered city identity across dynamic provider rebuilds - fail closed when a transient identity cannot be resolved safely ## Behavior An omitted allowlist keeps the existing all-city behavior. An explicit empty or nonmatching allowlist exports nothing. ## Verification - TDD coverage for omitted, empty, matching, nonmatching, rebuild, and failure paths - focused package tests and vet passed - mandatory pre-push fast test shards passed
…nhall#4716) ## Summary An exec-backed bead store does not report its own ID prefix, so any cache wrapping one is keyed as `(no-prefix)` and rig-scoped bead ownership breaks. `beads.NewCachingStore` derives its prefix from a `*BdStore`, or from any backing that implements the optional `IDPrefix() string` capability (`internal/beads/caching_store.go:245`). `internal/beads/exec.Store` implemented neither. An `exec:`-backed rig store, such as `exec:gc-beads-k8s` for a rig scope, therefore caches with an empty prefix. The reconciler logs `beads cache: reconciled rig=(no-prefix)`, the rig-scoped scale check cannot associate a routed rig bead with the rig pool, and a direct `gc sling <rig>/<agent>` never scales a worker. The prefix is already available. `docs/reference/exec-beads-provider.md` documents `GC_BEADS_PREFIX` as "the bead-ID prefix for this scope", and the provider already projects it into the script's environment. The store just never exposed it. This returns the trimmed value through the capability `NewCachingStore` is already looking for, so there is no new mechanism and no new config. Found while running the Kubernetes provider end-to-end (gastownhall#4704), but it is not k8s-specific. It affects every `exec:` store in a prefixed scope. ### Possible follow-up, not in this PR `NewCachingStore` records a "missing issue prefix" problem only when the backing is a `*BdStore` (`caching_store.go:254`). Any other backing with an empty prefix degrades silently, which is why this needed a live cluster to notice. Widening that diagnostic to all backings seems worthwhile, but it is a separate behavioral change. ## Testing - [ ] `make check` was not run locally. `gofmt` and `go vet` are clean and targeted `go test` is green for every touched package, but the full `golangci-lint` pass exhausts the dev box this was prepared on. We are relying on CI for it. New unit tests cover the env-to-prefix mapping (set, whitespace-trimmed, empty, absent, nil env, nil receiver) plus the regression itself: `NewCachingStore(execStore).IDPrefix()` must return the scope prefix rather than `""`. ## Checklist - [x] Linked an issue: gastownhall#4704 - [x] Added or updated tests for behavior changes - [ ] No docs update needed. This makes the code match the documented `GC_BEADS_PREFIX` contract - [x] No breaking changes. Stores that never had a prefix keep behaving as before. Only exec stores in a prefixed scope change, and only from "no prefix" to the correct one
…ip-provider-readiness) (gastownhall#4717) ## Summary The in-pod `gc init` that scaffolds a worker pod's filesystem runs the full workstation init path, including the provider-readiness preflight. For a deployment whose model access goes through a gateway or proxy rather than first-party OAuth, that preflight cannot pass, so every worker pod dies at startup with "startup is blocked by provider readiness". `initCityInPod` (`internal/runtime/k8s/provider.go`) execs `gc init --from /tmp/city-src /workspace` with no flags. `finalizeInit` then runs the provider-readiness probe, where `probeClaude` requires `APIProvider == "firstParty"` and rejects API-key or gateway auth. It also registers and starts a city, which a pod-local scaffold has no business doing. Neither step belongs here. `initCityInPod` exists only to lay down a session filesystem. The controller owns readiness, and pod sessions consume the projected `GC_DOLT_*` connection target through env rather than standing up their own city. The controller's own init already passes both flags, so this makes the in-pod init consistent with it rather than inventing a new policy. Found running the Kubernetes provider end-to-end (gastownhall#4704). ## Testing - [ ] `make check` was not run locally. `gofmt` and `go vet` are clean and targeted `go test` is green for every touched package, but the full `golangci-lint` pass exhausts the dev box this was prepared on. We are relying on CI for it. `TestInitCityInPodSkipsDolt` is extended to assert both flags reach the in-pod argv, alongside the existing `GC_DOLT=skip` assertion. ## Checklist - [x] Linked an issue: gastownhall#4704 - [x] Added or updated tests for behavior changes - [ ] No docs update needed. This is internal provider behavior - [x] No breaking changes. `--no-start` and `--skip-provider-readiness` only suppress work the pod-local scaffold should never have been doing, and no user-facing flag or config changes
…astownhall#4718) ## Summary `docs/reference/exec-beads-provider.md` says an update request carries `title`, `status`, `type`, `priority`, `description`, `parent_id`, `assignee`, `labels`/`remove_labels` and a metadata overlay. The Store conformance suite only ever asserted that `description` round-trips, so a backend could drop any of the others and still pass green. `Update{Type}` had no coverage anywhere in the repo, not in `RunStoreTests`, `RunFenceConformance`, or `RunConditionalWriterConformance`. Every exec-protocol implementation had drifted into that blind spot: - `internal/beads/exec/testdata/conformance.sh`, the reference fixture a new script is written against, dropped `title`, `status`, `type`, `priority` and `remove_labels`. - `contrib/beads-scripts/gc-beads-k8s` dropped those plus `assignee`, and encoded `parent_id` as a `parent:<id>` label even though the `bd` it wraps models the parent natively via `--parent`. The k8s adapter dropping `type` is what surfaced this. graph.v2 activation restores a step's deferred type via `Store.Update{Type}`, the write reported success, and the step stayed `type=gate` forever: ready-excluded, never dispatched, with no error anywhere. A silent lost write is the worst shape this class of bug can take, and the suite was structurally unable to catch it. This adds `UpdateRoundTripsEveryDocumentedField` to `RunStoreTests`, writing each field on its own so a backend that drops exactly one fails on that field rather than hiding behind the others, and fixes the two implementations above. All four Go stores (MemStore, FileStore, BdStore, NativeDoltStore) already conform and needed no changes. The new subtest is scoped to what the spec already promises, so it should not require a skip-ledger entry for anyone. ### One known gap left open `contrib/beads-scripts/gc-beads-br` has the same defect. It wraps `br` rather than `bd`, and we could not verify which of these fields `br update` can express, so rather than guess at a CLI we cannot test it carries a comment naming the gap. `TestBrProviderConformance` (build tag `integration`, requires `br` on PATH) will now report exactly which fields fail. Happy to fix it here if you can tell us `br`'s surface, or to leave it as a follow-up. Found running the Kubernetes provider end-to-end (gastownhall#4704). ## Testing - [ ] `make check` was not run locally. `gofmt` and `go vet` are clean and targeted `go test` is green for every touched package, but the full `golangci-lint` pass exhausts the dev box this was prepared on. We are relying on CI for it. The argv tests below use a fake `bd`, so the flag surface was confirmed separately against a real `bd 1.1.0` on a live cluster. Every flag this PR emits exists, including the `--parent` change: ``` --title --status --type --priority --description --assignee --parent --add-label --remove-label ``` Verified per-backend: MemStore, FileStore and NativeDoltStore pass the new subtest unchanged. The exec fixture failed on five fields and now passes. The `gc-beads-k8s` argv tests are broadened from two fields to the full documented set, plus a negative test that absent fields are not passed as empty flags. ## Checklist - [x] Linked an issue: gastownhall#4704 - [x] Added or updated tests for behavior changes - [ ] No docs update needed. The spec already documented this contract - [x] Migration note: the new conformance subtest will fail any out-of-tree `beads.Store` or exec script that drops a documented update field. That is intended, but it is a new gate for third-party backends, so it may deserve a release note
gastownhall#4719) ## Summary A pool or workflow worker's pod-mapped agent directory is a per-bead path (`<rig>/<beadID>-<slug>`, from `poolTriggerWorkDir`) that nothing creates before the pod starts. Both Kubernetes session providers put that path in the pod's `workingDir`. containerd does not fail on a missing `workingDir`. It creates the whole chain itself, as `root:root 0755`, before the entrypoint runs. The agent then runs as a non-root user and cannot write into its own working directory: ``` uid=1000 cwd=/workspace/rigs/testrig/tr-xyz-clone drwxr-xr-x. 2 root root 6 . touch: cannot touch './probe': Permission denied ``` So the worker starts, chdirs successfully, and fails the moment it tries to clone. A later `mkdir -p` cannot repair it, because the directory already exists and mkdir leaves ownership alone. This points the manifest at the workspace root instead, which always exists and is already owned by the agent user (the `ws` EmptyDir mount point when staged, `WORKDIR /workspace` in the image when prebaked). containerd then creates nothing, and the entrypoint creates the per-bead directory itself as the agent user, so it comes out owned correctly. ### Why the entrypoint and not the init container Creating the directory from the staging init container works only for staged pods, and it works by the same accident of ordering: the init container gets there first, on the shared volume, as the agent user. A prebaked pod mounts no shared volume at all, since the `ws` EmptyDir is deliberately skipped so it cannot shadow baked image content. An init container therefore cannot create a directory the main container would see, and prebaked is the mode `contrib/k8s/example-city.toml` recommends for production. The entrypoint is the only place that covers both topologies. Ordering matters twice, and the tests pin both. Entering the directory must happen after the staging wait, or the shell sits in a subdirectory of a workspace that is still being written. It must also happen before `pre_start`, which previously ran in the per-bead directory and may use relative paths. The dynamic-user branch already had the mkdir and the cd, in `userSetup` and its `su -c`, but only reached them after the damage was done. That code is now load-bearing. The no-dynamic-user branch gains the same two steps. `contrib/session-scripts/gc-session-k8s` builds the same manifest and had the same defect, so it gets the same fix. `TestPodManifestCompatibility` keeps the native and exec providers interchangeable, and would otherwise have gone red. Found running the Kubernetes provider end-to-end (gastownhall#4704). ## Testing - [ ] `make check` was not run locally. `gofmt` and `go vet` are clean and targeted `go test` is green for every touched package, but the full `golangci-lint` pass exhausts the dev box this was prepared on. We are relying on CI for it. Verified on a live cluster (containerd 2.1.6), in both topologies: | | `workingDir` | creates the per-bead dir | owner | agent can write | |---|---|---|---|---| | before | per-bead path | containerd, pre-entrypoint | `root:root` | no | | after | `/workspace` | entrypoint, as uid 1000 | `gcagent:gcagent` | yes | Unit tests, each covering staged and prebaked: - `workingDir` is always a path that exists and is agent-owned - the entrypoint creates and enters the per-bead dir, with and without `LINUX_USERNAME` - entering the dir happens after the staging wait and before `pre_start` - the staging init container is back to only waiting for staging - `TestPodManifestCompatibility` and `TestSessionScriptStartRigManifestUsesPodPaths` updated to the new contract Note for reviewers: `TestSessionScriptStart*` needs jq 1.7 or newer locally, because the script uses `$var` as an object key. On jq 1.6 those tests fail before reaching any of this. ## Checklist - [x] Linked an issue: gastownhall#4704 - [x] Added or updated tests for behavior changes - [x] No docs update needed. This is internal pod-spec behavior - [x] Migration note: the pod manifest's `workingDir` value changes from the per-bead path to `/workspace`. External tooling that reads `workingDir` off a gc agent pod to discover the agent's directory should read the entrypoint or `GC_DIR` instead. Agents still start in the same directory as before, and now own it --------- Co-authored-by: chris-sanders <chris-sanders@users.noreply.github.com>
…all#4720) ## Summary `gc init --from` silently ignores an external Dolt endpoint. `--dolt-*` was marked mutually exclusive with `--from`, and the `--from` branch returned before hosted-Dolt resolution ran. A city templated from a directory therefore always fell back to the copied template's managed-local Dolt assumption, even with `--dolt-host` or `GC_DOLT_HOST` explicitly supplied. That combination is what a fleet deployment needs: take a known-good city template, then point it at the shared Dolt endpoint the rest of the fleet uses. Without it, each city ends up with its own local Dolt and no shared ledger. The endpoint is now resolved before mode dispatch and applied on the `--from` path the same way the default and wizard paths already apply it, writing `[dolt]` in `city.toml` plus the canonical `.beads/config.yaml`, and reusing the existing `hostedDoltInitOptions` helpers rather than adding a parallel mechanism. Precedence is unchanged (explicit flag, then env, then template). When no endpoint is supplied the copied template is preserved byte-for-byte, and an incomplete endpoint fails before anything is written. `--file` stays mutually exclusive with `--dolt-*`, since it supplies a complete `city.toml` verbatim. Found running the Kubernetes provider end-to-end (gastownhall#4704), but it is not k8s-specific. It affects any templated city pointed at a shared endpoint. ### Reviewer notes - `doInitFromDirWithOptionsFSInternal` now takes a ninth positional parameter, next to two existing bare bools. That follows the established `WithOptions`/`Internal`/`FS` ladder in this file, so it is the smallest diff, but we are happy to convert the tail of that signature to an options struct if you would rather absorb the churn here. - The hosted block is applied inline on the `--from` path, whereas the wizard path spreads the same three steps across `cmd_init.go:1371/1443/1447`. A shared helper would DRY the two. We left it out to keep this diff reviewable. - `hosted.validate()` is called explicitly before `applyToCityConfig` even though `applyInitHostedDoltCanonicalConfig` validates too. That is deliberate, so it fails before `city.toml` is rewritten rather than after. - The new tests deliberately use `clearGCEnv(t)` and assert on written state rather than following the usual `t.Setenv("GC_DOLT", "skip")` / `t.Setenv("GC_BEADS", "bd")` convention seen elsewhere in `cmd/gc`. The resource-census ledger holds `cmd/gc+untagged` environment calls at a baseline that cannot grow, so three new tests written the conventional way fail `TestRepositoryLedgerMatchesCensusAndDocumentation`. Letting the copied city's own config select the bd provider avoids adding any process-environment mutation, which seems to be the direction that ledger is pushing. Say the word if you would rather these matched the local convention and the baseline moved instead. ## Testing - [ ] `make check` was not run locally. `gofmt` and `go vet` are clean and targeted `go test` is green for every touched package, but the full `golangci-lint` pass exhausts the dev box this was prepared on. We are relying on CI for it. - [x] `docs/reference/cli.md` reviewed for the changed flag-compatibility statement Three new tests: the endpoint lands in both `city.toml` `[dolt]` and the canonical `.beads/config.yaml`; no endpoint leaves the template untouched; an incomplete endpoint fails without leaving a half-configured city. Also smoke-tested against a real `gc init --from`. ## Checklist - [x] Linked an issue: gastownhall#4704 - [x] Added or updated tests for behavior changes - [x] Updated docs for user-facing changes (CLI flag compatibility) - [x] Migration note: this widens the CLI contract. `--dolt-*` with `--from` was previously rejected by cobra and is now accepted. No existing invocation changes behavior, since anything that used to error could not have been in use
## Summary - make carried-route recovery query live raw Bead status before restoring `gc.routed_to` - build the workflow projection from live `open` and `in_progress` reads, deduplicated by Bead ID - add regression coverage proving blocked work is neither rerouted nor admitted to the active workflow projection Gas City's cached Bead model collapses `blocked` and `deferred` into its internal `open` state. Non-live status filters therefore allowed blocked workflow roots through: the reaper cleared `gc.routed_to`, then route recovery restored it on a later patrol and the projection could consume it again. These reads now reach the backing store's raw-status filter. ## Test plan - `go test -count=1 ./cmd/gc -run TestRestoreCarriedWorkRoutes` - `go test -count=1 ./internal/api -run TestListActiveWorkflowProjectionBeadsExcludesBlocked` --------- Co-authored-by: sjarmak <sjarmak@users.noreply.github.com> Co-authored-by: sjarmak <t@t.co>
## What this changes Automatic routed dispatch now treats the two canonical hold labels as stop signs across pool-demand queries and control-dispatcher ready scans. Held work remains persisted and observable, but an unassigned agent will not automatically claim it while the hold is present. Deliberately assigned work remains available to its assignee. Crash and boot recovery also remain hold-transparent: recovery can reopen persisted work without stripping its hold, while a later ambient route scan still filters that work out. ## Why this design The implementation keeps recovery and explicit assignment separate from automatic routing. A shared typed list in `internal/beadmeta` drives the modern beads CLI queries, legacy compatibility filters, and in-process control-ready evaluation so those paths cannot drift independently. ## Review notes - Review the boundary between assignee-scoped Tier 1/2 paths and unassigned, route-scoped Tier 3 paths; only the latter filter holds. - The change covers both current `bd ready --exclude-label` behavior and the legacy ephemeral jq path. - There is no new configuration, schema migration, dependency, or user action required. ## Test plan - [x] `make test-fast-parallel` — all 10 jobs pass. - [x] Touched-package run — 17,169 PASS, 0 FAIL; focused hold/recovery tests — 25 PASS, 0 FAIL, 0 SKIP. - [x] `go vet ./...` and `go build ./...`. - [x] Release gate: [`release-gates/ga-9sp6gf-held-work-dispatch-gate.md`](release-gates/ga-9sp6gf-held-work-dispatch-gate.md) --------- Co-authored-by: investigator <investigator@gascity.local>
…n gutter (gastownhall#4579) (gastownhall#4737) ## Problem `gc status` has two rendering defects that surface only when the runtime probe times out (partial status). **Defect 1 — the summary line contradicts the rows above it.** A single run prints `1/18 agents running` while fifteen of the eighteen agent rows render `unknown (partial status)`. gastownhall#4345 fixed the per-row rendering and gastownhall#4343 was closed on that basis, but the summary line was never touched, so the same misreport still reaches the reader one screen further down: a skim of `1/18 agents running` says the fleet is down when it is not known to be. **Defect 2 — the name column eats its own separator.** core.control-dispatcher unknown (partial status) tar-valon/core.control-dispatcherunknown (partial status) A rig-qualified name at or past the pad width runs straight into the status token. ## Root cause At `1a9921943ec4bea15677f9c63ebe517ba47e547b`: - `cmd/gc/city_status_snapshot.go:525` — the summary is `fmt.Fprintf(stdout, "%d/%d agents running\n", …RunningAgents, …TotalAgents)`. `RunningAgents` is incremented only for `obs.Running` (`cmd/gc/city_status_snapshot.go:290-293`), so every row the probe could not answer for is silently counted as not-running. The row renderer (`cmd/gc/cmd_status.go:405-411`) correctly calls those rows `unknown`; the summary has no notion of the partial state at all, even though `snapshot.Partial` is right there in the same function. - `cmd/gc/city_status_snapshot.go:515,517,519` — the name column is a bare `%-24s` (and `%-22s` for expanded rows). A fixed pad guarantees a *minimum total width*, not a *minimum separator*: at exactly 24 characters it emits zero spaces before the next token, and at 23 it emits one. ## Fix Two helpers in `cmd/gc/city_status_snapshot.go`, both narrow: - `agentSummaryLine(running, total, partial)` reports the unknown count separately during partial status — `1 running, 17 unknown of 18 agents`. When the status is not partial, or nothing is unknown, it returns the historical `%d/%d agents running` string byte-for-byte, so normal output does not move. - `padStatusName(name, width)` pads as `%-*s` did whenever the name is short enough to keep the gutter, and otherwise emits the name plus exactly `statusNameColumnGutter` (2) spaces. Names of 22 characters or fewer render identically to today; 23 and longer gain the missing gutter, which is the bug. Scoped to the agent block named in the issue. The `Named sessions:` and `Rigs:` blocks use the same `%-24s` shape and would overflow the same way with a long enough name — untouched here, happy to extend if you would rather fix the column once. ## Testing `cmd/gc/city_status_partial_render_test.go`, table-driven over the renderer: - `TestAgentSummaryLineDoesNotFoldUnknownIntoNotRunning` — five cases, including two non-partial cases pinning the byte-identical old string. - `TestAgentSummaryLineRenderedDuringPartialStatus` — full `renderCityStatusText` output: two `unknown (partial status)` rows must not sit under `1/3 agents running`. - `TestAgentNameColumnKeepsGutter` — flat and expanded rows with `tar-valon/core.control-dispatcher`; asserts the issue's own falsifiable check (no `dispatcherunknown`) and the two-space gutter. - `TestPadStatusNameMatchesFixedPadBelowGutter` — pins the no-change half. Run against the unfixed behaviour first, all four fail, reproducing the issue verbatim: --- FAIL: TestAgentSummaryLineDoesNotFoldUnknownIntoNotRunning/partial_reports_unknown_separately agentSummaryLine(1, 18, true) = "1/18 agents running", want "1 running, 17 unknown of 18 agents" --- FAIL: TestAgentNameColumnKeepsGutter/flat_row stdout = "…\n tar-valon/core.control-dispatcherunknown (partial status)\n…", long agent name overflows into the status column --- FAIL: TestPadStatusNameMatchesFixedPadBelowGutter padStatusName("aaaaaaaaaaaaaaaaaaaaaaa", 24) = "aaaaaaaaaaaaaaaaaaaaaaa ", want "aaaaaaaaaaaaaaaaaaaaaaa " After the fix: `go build ./...` and `go vet ./cmd/gc/` clean, the new tests pass, and `go test ./cmd/gc/ -run 'Status|status'` (the whole existing status suite) passes unchanged. Fixes gastownhall#4579 ## Verification re-run on this branch head (e1531e2, rebased onto main af42a94) Both defects re-confirmed present on current `main` (af42a94) before the fix: `cmd/gc/city_status_snapshot.go:525` still prints the bare `"%d/%d agents running"` without consulting `snapshot.Partial`, and the name columns at `:515,519,521` are still bare `%-24s` / `%-22s`. ``` $ go build ./cmd/gc/ # clean $ go vet ./cmd/gc/ # clean $ go test ./cmd/gc/ -run 'TestAgentSummaryLine|PartialRender|Status|status' -count=1 ok github.com/gastownhall/gascity/cmd/gc 17.372s ``` Falsifiable check — the new tests were watched RED first. With only the `partial` branch of `agentSummaryLine` neutralized (helper kept, so the failure is behavioral rather than a compile error): ``` --- FAIL: TestAgentSummaryLineDoesNotFoldUnknownIntoNotRunning/partial_reports_unknown_separately agentSummaryLine(1, 18, true) = "1/18 agents running", want "1 running, 17 unknown of 18 agents" --- FAIL: TestAgentSummaryLineDoesNotFoldUnknownIntoNotRunning/partial_with_nothing_running_still_names_the_unknowns agentSummaryLine(0, 5, true) = "0/5 agents running", want "0 running, 5 unknown of 5 agents" --- FAIL: TestAgentSummaryLineRenderedDuringPartialStatus summary still folds unknown agents into not-running ``` The non-partial cases in the same table stay green under that neutralization, which is the machine-check that non-partial output is unchanged. ## Adjacent, deliberately not changed The `Named sessions:` and `Rigs:` blocks (`:532`, `:544`) share the same bare `%-24s` and therefore the same latent overflow. gastownhall#4579 names only the agent column, so this PR leaves them alone rather than widening the diff — happy to include them if you'd prefer one sweep. --------- Co-authored-by: rand <home@callindor.halibut-banjo.ts.net> Co-authored-by: jacobhausler <jacobhausler@users.noreply.github.com>
…l#4938) ## Summary - make `t3bridge.ListRunning` return `runtime.ErrRuntimeUnavailable` when its snapshot is transiently unavailable or still initializing - keep total observation failure distinct from a partial-but-usable backend result - add a regression test and reconcile the runtime-provider requirements ledger ## Problem `ListRunning` currently converts a soft T3 bridge failure into `(nil, nil)`. That result is indistinguishable from a healthy snapshot with zero sessions, so callers that reason about absence can treat a transient outage as proof that every T3 session disappeared. This already affects existing fail-closed callers such as the adoption barrier and dead-runtime cleanup. They defer on an error, but no error reaches them on this path. ## Change A soft-unavailable snapshot now returns no names and an error wrapping `runtime.ErrRuntimeUnavailable`. This is a total observation failure, not a `PartialListError`: there is no usable snapshot to preserve. The underlying bridge error remains wrapped for diagnosis. The test disables fallback endpoints, points the provider at a closed loopback port, and asserts: - the result is not empty success - `errors.Is(err, runtime.ErrRuntimeUnavailable)` - the error is not a `PartialListError` - no names are returned with the failed observation ## Tests ```text go test -count=1 ./internal/runtime/t3bridge ok github.com/gastownhall/gascity/internal/runtime/t3bridge 34.905s go test -count=1 ./internal/testpolicy/resourcecensus \ -run '^TestRepositoryLedgerMatchesCensusAndDocumentation$' ok github.com/gastownhall/gascity/internal/testpolicy/resourcecensus 1.649s go vet ./internal/runtime/t3bridge PASS ``` I also searched open and closed issues for `t3bridge ListRunning`, `runtime unavailable`, and T3 snapshot outages; no duplicate surfaced. ## Design filters - **Zero Framework Cognition:** this preserves the existing provider/error boundary; callers already defer on total listing errors. - **Bitter Lesson alignment:** no heuristic is added. The provider reports the direct fact that it could not observe the runtime.
…astownhall#4739) (gastownhall#4740) ## Summary Two defects, one root cause: the claude profile's `model` option is an enumerated select, and a value outside the enum produces NO `FlagArgs`. The launch path then emits no `--model` at all and the session falls through to the CLI default, while `gc config show` still reports the pin as set. The named-session resolution path validates the same `Choices` list strictly and hard-errors instead (`invalid value for model: claude-opus-5`). `claude-sonnet-5` (gastownhall#3867) and `claude-fable-5` (gastownhall#3284) were added as short aliases previously; `claude-opus-5` was never added. Separately, none of the three added the canonical `claude-<family>-5` spelling as a directly accepted value — only the short alias — even though operators commonly pin the full provider model id in `agent.toml`. That second gap predates and is broader than Opus 5 alone. - `profiles.go`: add `opus-5` as a new explicit alias. Bare `opus` is deliberately left pinned to `claude-opus-4-8` — `internal/config/provider_test.go` (`TestBuiltinProvidersClaudeModelChoices`) asserts that as a stability guarantee, unlike the sonnet/fable-5 precedent which repointed the bare alias. Also accept `claude-opus-5`, `claude-sonnet-5`, `claude-fable-5` verbatim as enum values, each mapping to the same flag as its short alias. - `internal/sessionlog/context.go`: Opus 5 ships 1M context natively (there is no 200K Opus 5 variant), but `ModelContextWindow` keyed only on the bare family word `opus` and returned the 200K default for it. Add an early-matched native-1M-model list so Opus 5 resolves correctly, without touching the pre-existing opus-4-8/sonnet-5 bare-window drift — that is tracked separately by gastownhall#4527 (open), which this PR does not duplicate. Note for reviewers of gastownhall#4527: its `millionMarkers` table is also missing `opus-5` — worth a fix there too, or this PR's `nativeMillionTokenModels` approach can be ported into that table when it merges. ## Testing Falsifiable floor: both new test files are demonstrated RED on upstream main (`af42a94245a547a0c47ec26054afa5fd1347b567`) before this fix, GREEN after — run log below, not "should work." RED (baseline — fix reverted, tests kept): ``` --- FAIL: TestBuiltinClaudeModelChoicesIncludeOpus5 (0.00s) claude model choices missing "opus-5" (claude-opus-5 has no enum entry, so resolving it yields no --model FlagArgs and gc silently launches the provider default) --- FAIL: TestBuiltinClaudeModelChoicesAcceptCanonicalIDsVerbatim (0.00s) claude model choices missing canonical id "claude-opus-5" as a directly-accepted value claude model choices missing canonical id "claude-sonnet-5" as a directly-accepted value claude model choices missing canonical id "claude-fable-5" as a directly-accepted value FAIL github.com/gastownhall/gascity/internal/worker/builtin 0.183s --- FAIL: TestOpus5IsNativelyOneMillion (0.00s) ModelContextWindow("claude-opus-5") = 200000, want 1000000 (Opus 5 is natively 1M) ModelContextWindow("opus-5") = 200000, want 1000000 (Opus 5 is natively 1M) FAIL github.com/gastownhall/gascity/internal/sessionlog 0.197s ``` GREEN (with fix): ``` ok github.com/gastownhall/gascity/internal/worker/builtin 0.227s ok github.com/gastownhall/gascity/internal/sessionlog 0.337s ``` Blast-radius — readers of these two tables, enumerated by grep, all green: ``` ok github.com/gastownhall/gascity/internal/config 21.200s ok github.com/gastownhall/gascity/internal/worker/builtin 0.147s ok github.com/gastownhall/gascity/internal/sessionlog 1.763s ``` A first pass without the "bare opus stays pinned" correction broke `internal/config.TestBuiltinProvidersClaudeModelChoices`; caught by this sweep, fixed by not repointing bare `opus`, re-verified green above. `gofmt -l` clean, `go vet` clean, `go build ./cmd/gc/` clean. ## Checklist - [X] Linked an issue (gastownhall#4739) - [X] Added or updated tests for behavior changes - [NA] Updated docs for user-facing changes (no docs reference the model enum) - [NA] Breaking changes — additive only; bare `opus` behavior unchanged Fixes gastownhall#4739 --------- Co-authored-by: rand <home@callindor.halibut-banjo.ts.net> Co-authored-by: jacobhausler <jacobhausler@users.noreply.github.com>
## Summary - replace the drift-restart path's Linux-specific `/proc` PID check with the shared portable `pidutil.Alive` probe - add a Darwin regression proving a live process is not reported gone before restart proceeds Fixes gastownhall#4942. ## Testing - [x] `go test -count=1 ./cmd/gc -run '^TestPIDGoneReturnsFalseForCurrentProcess$'` - [x] `go test -count=1 ./cmd/gc -run '^(TestPIDGoneReturnsFalseForCurrentProcess|TestRunStartDriftCheck_DarwinLaunchdRestartDoesNotRequireProcExe)$'` - [x] `go test -count=1 ./internal/pidutil` - [x] `go test -count=1 ./internal/testpolicy/resourcecensus -run '^TestRepositoryLedgerMatchesCensusAndDocumentation$'` - [x] `make test-cmd-gc-process-shard CMD_GC_PROCESS_SHARD=1 CMD_GC_PROCESS_TOTAL=6` (1,379 tests) - [x] `go vet ./...` - [ ] `make check` — not repeated after the affected process shard; exact `origin/main` currently reproduces unrelated macOS path-canonicalization failures on this host - [ ] `make check-docs` — not applicable; no docs, navigation, or links changed - [ ] `make test-integration` — not run locally; this change is covered by the process-backed `cmd/gc` shard and full PR CI ## Checklist - [x] Linked an issue - [x] Added a regression test for the behavior change - [x] No user-facing docs change is required - [x] No breaking change or migration is introduced
## Summary This is the `doctor`-scoped replacement for gastownhall#3845. It preserves Karel Bourgois's original commit and authorship while separating the backup-freshness API from the mixed-scope branch. - adds `BulkDeleteSafe` over the existing backup-freshness machinery - covers fresh, stale, and unconfigured managed scopes - reuses the check's canonical managed-scope target selection Original commit assigned here: `0398fca9136120d9e5d4613a42ecc90720968338`. ## Test plan - `go test ./internal/doctor` - pre-commit `lint-changed`, generated-doc freshness, and `go vet ./...` Split from and supersedes the `doctor` portion of gastownhall#3845. Please credit @bourgois for the original implementation. --------- Co-authored-by: bourgois <karel@voxist.com> Co-authored-by: sjarmak <t@t.co>
## Summary - qualify the communication tutorial so ordinary mail is not described as categorically unable to wake a recipient - document that `mail send --notify` and `mail reply --notify` can request a managed wake for a non-running recipient, while unread mail alone is not wake demand - regenerate the CLI reference and add a regression for both commands' managed-wake help Fixes gastownhall#4941. ## Testing - [x] `go test ./cmd/gc -run '^(TestMailNotifyHelpDocumentsManagedWake|TestCLIDocsFreshness)$' -count=1` - [x] `make check-docs` - [x] `go vet ./...` - [x] `make lint-changed LINT_CHANGED_SCOPE=tracked LINT_CHANGED_REF=origin/main` - [ ] `make check` — formatting, lint, vet, and routed-test checks passed; the unit sweep then hit unrelated macOS path-canonicalization failures reproduced on exact `origin/main`, plus ambient tmux/Dolt cleanup failures - [ ] `make test-integration` — not applicable; runtime behavior is unchanged ## Checklist - [x] Linked an issue - [x] Added focused help-text regression coverage - [x] Updated the tutorial and generated CLI reference - [x] No breaking change or migration is introduced
Publish exact graph-backed execution facts and preserve topology through the public event surfaces. Keep producer activation deferred until the compatible consumer is live.
## Summary - make each controller socket declare whether it is hosted by the standalone controller or the machine supervisor - add a private typed `identify` response carrying PID and hosting mode while keeping the legacy numeric `ping` response byte-compatible - retain legacy liveness fallback without guessing that an unidentified controller is standalone; same-PID supervisor compatibility remains supported - use authoritative hosting information in status, start/register, and stop/unregister diagnostics, and document the wire contract Fixes gastownhall#4915. Fixes gastownhall#4940. ## Testing - [x] focused controller identity, compatibility, status, start/register, and stop diagnostic suite - [x] `go test -count=1 ./cmd/gc -run '^TestControllerTestHasNoUnmigratedRawHangDeadlines$'` - [x] `go test -count=1 ./internal/testpolicy/resourcecensus -run '^TestRepositoryLedgerMatchesCensusAndDocumentation$'` - [x] `make lint-changed LINT_CHANGED_SCOPE=staged` - [x] `go vet ./...` - [x] `make check-docs` - [x] required generators (`genspec`, `genclient`, and `genschema`) produced no diff - [ ] `make test-fast-parallel` — affected tests passed; the broad gate hit unrelated current-main macOS path-canonicalization failures, an ambient tmux timeout, and an ambient Dolt fixture leak - [ ] `make test-integration` — not run separately; the focused suite includes a process-backed supervisor controller-socket test and full PR CI covers the repository matrix ## Checklist - [x] Linked both duplicate issues - [x] Added regression coverage for authored identity, legacy compatibility, and every affected user-facing classification surface - [x] Updated the controller architecture documentation - [x] Preserved the legacy numeric `ping` protocol - [x] No breaking change or migration is introduced
## Summary This is the `reaper`-scoped replacement for gastownhall#3845. It preserves Karel Bourgois's original reaper work and authorship while keeping the temporary fork plan out of the final diff. - skips bulk session-bead pruning when backup state is absent, invalid, or stale - records an anomaly explaining the rejected prune - adds focused shell coverage for absent, fresh, and stale backup state - keeps the existing configurable session-pattern coverage aligned with the current `gc bd` route Original commits assigned here: `8cb2ec6db1da6c8baa9197029776e258ff07db84` and `466ce4ce276593d9745c8d019140220bf961b4b9`. ## Test plan - `bash test/reaper_prune_backup_guard_test.sh` - `bash test/reaper_session_pattern_test.sh` Split from and supersedes the `reaper` portion of gastownhall#3845. Please credit @bourgois for the original implementation. --------- Co-authored-by: bourgois <karel@voxist.com> Co-authored-by: sjarmak <t@t.co>
## Summary This is the `ci`-scoped replacement for gastownhall#3845. It preserves Karel Bourgois's original fixture commit and authorship while separating test-harness compatibility from the production changes. - gives reaper integration fixtures a fresh backup state where pruning is expected - exposes `date` in the restricted-PATH fixture used by the backup-age guard Original commit assigned here: `ee9bcdbe877821d3f2346299e3af783e2cc34b56`. ## Test plan - `go test ./examples/gastown` Split from and supersedes the `ci` portion of gastownhall#3845. Please credit @bourgois for the original implementation. Co-authored-by: bourgois <karel@voxist.com>
## Summary - apply an explicit `gc stop --timeout` wall-clock cap to the entire stop sequence, including city resolution, supervisor unregister, invalid-config recovery, and loaded-city cleanup - preserve the existing config-derived default budget when the caller does not pass an explicit timeout - emit the final success record only after the bounded worker returns, preventing a timed-out worker from reporting late success - add a regression around a blocked supervisor-managed invalid-config stop Fixes gastownhall#4939. ## Testing - [x] `go test -count=1 ./cmd/gc -run 'TestCmdStop|TestDoStop'` - [x] `go test -count=10 ./cmd/gc -run '^TestCmdStopWallClockTimeoutBoundsSupervisorManagedInvalidConfigStop$'` - [x] `go test -count=1 ./cmd/gc -run '^TestCanonicalSessionProviderFactoryCallerCensus$'` - [x] `make lint-changed LINT_CHANGED_SCOPE=staged` - [x] `go vet ./...` - [x] `make check-docs` - [x] required generators (`genspec`, `genclient`, and `genschema`) produced no diff - [ ] `make check` — not repeated after the focused gates; exact `origin/main` currently reproduces unrelated macOS path-canonicalization failures on this host - [ ] `make test-integration` — not run locally; the supervisor wait is covered by the focused command regression and full PR CI ## Checklist - [x] Linked an issue - [x] Added a regression test for the behavior change - [x] Updated the session reconciliation requirements ledger - [x] No breaking change or migration is introduced
## Summary - skip ephemeral ga- agent work directories in TestDocDirCoverage This is the docsync-scoped replacement for gastownhall#3835. All credit belongs to @quad341; the cherry-picked commit retains original authorship. ## Validation - go test ./test/docsync - go vet ./... - git diff --check origin/main...HEAD No merge without a dedicated review record. Co-authored-by: quad341 <james@wordelman.name> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
… 1/10) (gastownhall#4965) ## Summary Extract the host-side setup-command runner from the tmux adapter into the shared runtime layer. Split from gastownhall#4217 and adapted onto current main. Thank you @McKean for the original implementation. ## Stack This is the stack base. This PR intentionally targets main; merge in numeric order so the displayed diff collapses to this scope. Do not merge without the normal maintainer review record. ## Source commit ledger This part owns exactly these original gastownhall#4217 commits: - `04416e468` ## Verification - Compile-safe cumulative stack: `make test` - `go vet ./...` - Repository pre-push fast suite completed once; a repeat exposed the known test-owned Dolt cleanup race after all assertions passed --------- Co-authored-by: Christopher Scott <christopher@plantime.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…astownhall#4979) ## Problem When a demand read over multiple stores completes only partially, the scale-check pass marks every control-dispatcher template as partial. That marking does two jobs at once: it retains existing dispatcher sessions (correct, fail-safe) and it suppresses new dispatcher creates (incorrect when a healthy store is exposing real control demand). The result is a cold system that cannot spawn its first control dispatcher while any store read is degraded, even though another store has live demand. Follow-up to the demand-read work in gastownhall#4395. ## Fix Split the two behaviors. Partial-read markings are now retention-only: they keep existing dispatchers alive but never veto a create. Create suppression applies only from an ordinary scale-check verdict computed over the stores that did read successfully. ## Tests New two-store regression: one store exposes control demand, the other simulates an outage mid-read. The test fails before the production change (cold dispatcher create vetoed) and passes after (create planned, existing dispatchers still retained). Focused suite run with the race detector, plus vet and build, all green at this head. Co-authored-by: sjarmak <t@t.co>
…astownhall#4744) ## Summary - `storehealth.Health` / `Compute` gain a `RowsMeasured` signal, so "the count did not complete" is representable distinctly from "the count completed and found zero rows" - `cmd/gc`'s `liveRowCount` stops fabricating a `0` on a nil store, a scan error, or a timeout — it now returns `(rows int, measured bool)` - `gc status` reports `Live rows: unknown (count unavailable)` and omits the ratio line entirely when unmeasured; the CLI JSON gains an additive `live_rows_unknown` field ## The defect `liveRowCount` returned a bare `0` on three distinct failure modes (nil store, scan error, `statusStoreHealthTimeout` — 1s over a closed-inclusive full-history scan). `Compute` evaluated `RatioMB`/`Warning` only inside `if retainedRows > 0`, with no way to tell a real zero from a failed measurement. A timed-out count rendered byte-identically to a healthy, empty store, so a store can grow without bound and stay green forever whenever the count cannot finish in time. Full write-up, including the demonstration on unmodified `main`, in gastownhall#4743. `gastownhall#4464` made the happy-path count fast; `gastownhall#4307` fixed the **API** half of this class (`countBeadStoreRows` errors instead of fabricating). Neither touched `cmd/gc/store_health.go` — the current doc comment on `statusStoreHealthTimeout` says as much. This is the CLI-side completion of gastownhall#4307. Deliberately **not** a timeout-widening fix: a bigger bound moves the cliff, it doesn't remove the ambiguity. ## The fix - `Health` gains `RowsMeasured bool`; `Compute` gains a `rowsMeasured bool` parameter and computes `RatioMB`/`Warning` only when `rowsMeasured && retainedRows > 0`. This mirrors the existing `*Health` pointer idiom, where nil means "no data" specifically so it cannot be misread as a zero-valued block. - `liveRowCount` returns `(rows int, measured bool)`; every failure path returns `(0, false)`. - `storeHealthFromInputs` / `collectStoreHealth` thread `rowsMeasured` through and surface it as `StoreHealth.LiveRowsUnknown` (`json:"live_rows_unknown,omitempty"`). - `renderStoreHealthBlock` prints `Live rows: unknown (count unavailable)` and omits the ratio line rather than a misleading `0.0 MB/row`. The cause is **not** named in the message because the caller does not know it — a nil store, a scan error and a timeout are all simply unmeasured, and asserting "timed out" would repeat the same mistake this PR fixes at a smaller scale. - `internal/api`'s `Compute` call site passes `rowsMeasured=true` explicitly, with a comment recording why it is always true there. ## What each state now means - **Measured + `Warning=false`** — the store was counted; the ratio is at or below threshold (or suppressed by the absolute floor). Safe to treat as healthy. - **Measured + `Warning=true`** — the store was counted; the ratio exceeds both the floor and the threshold. Maintenance is actionable now. - **Unmeasured** (`RowsMeasured=false` / `live_rows_unknown=true`) — **nothing** may be concluded about store health. `LiveRows`, `RatioMB` and `Warning` are not meaningful. A caller alarming off `StoreHealth` must treat this as "retry / investigate", never as "pass". `RowsMeasured=false` with `Warning=true` is impossible by construction. ## Test evidence **Behavioural RED on unmodified `af42a94245a547a0c47ec26054afa5fd1347b567`**, using the pre-change signature so it demonstrates the bug rather than the signature change: ``` DEFECT CONFIRMED on stock main: an 11.2 GB store with an UNMEASURED row count yields Warning=false, RatioMB=0.0 — identical to a genuinely empty healthy store. The ratio check can never fire. --- FAIL: TestStockMain_UnmeasuredLargeStoreIsIndistinguishableFromHealthyEmpty ``` The shipped tests also fail to compile against stock `main`, since the capability they assert does not exist there. **GREEN** with the fix: `./internal/storehealth/...` and `./internal/api/` fully pass; `./cmd/gc/ -run 'StoreHealth|LiveRowCount'` passes 20/20, including five new tests: - `TestComputeUnmeasuredRowsNeverWarns` - `TestComputeUnmeasuredIsDistinguishableFromRealZero` - `TestLiveRowCountTimeoutIsUnmeasuredNotZero` - `TestRenderStoreHealthBlockUnmeasuredRowsSaysUnknownAndOmitsRatio` - `TestRenderStoreHealthBlockUnmeasuredRowsStillRendersLastGC` `go build ./...`, `go vet` and `gofmt` clean. The full `./cmd/gc/...` package was also run: 45 failures, all pre-existing on this base and unrelated — every one is the macOS `TMPDIR` artifact where `t.TempDir()`/`os.Getwd()` resolve `/var/folders/...` through its `/private/var/folders/...` symlink (rig-root, worktree-reap and quarantine assertions comparing the two literally). None is in the store-health surface. Verified against stock `main` on the same machine, not assumed. ## Blast radius `storehealth.Compute` has exactly two callers in the whole tree (grep-confirmed), both updated: `cmd/gc/store_health.go` and `internal/api/store_health.go`. Pinned unchanged: warn-path ratio semantics (`DefaultThresholdMB = 1.0`, `MinWarnSizeBytes = 1_000_000_000`, `bytesPerMB = 1_000_000` SI — cited from source, not restated), with `TestComputeWarningHighRatio` and `TestRenderStoreHealthBlockWarning` still passing verbatim. API error propagation is untouched, exactly as gastownhall#4307 left it. `internal/storehealth.Health` is an internal Go struct with no JSON tags and is never itself serialized. The API wire type `StatusStoreHealth` is unchanged, so `/v0/status` and every consumer of it — including the generated dashboard client and zod types — are unaffected and no codegen is needed. Only the CLI-local `cmd/gc.StoreHealth` (`gc status --json`) gains one additive `omitempty` field, false in the common case. Fixes gastownhall#4743 --------- Co-authored-by: rand <home@callindor.halibut-banjo.ts.net> Co-authored-by: jacobhausler <jacobhausler@users.noreply.github.com>
Brings the fork up to origin/main (gastownhall). 101 conflicts, of which 63 were dashboard dist bundles, 12 workquery goldens and 4 other generated artifacts — regenerated from the merged tree rather than picked from a side. The genuine source divergence was small (13-22 lines per file). Conflict resolutions worth recording: - internal/resilience/breaker.go — cosmetic naming (capDur/capacity) plus a fork-side simplification that dropped a post-loop clamp with a correctness proof. Took upstream wholesale: the clamp is harmless, and matching upstream keeps the next merge cheap. - internal/resilience/registry.go — kept ours. SetJitterForTest is fork-only test infrastructure with live consumers. - internal/beads/caching_store.go — mixed: upstream's new dependsOnStepIDs parameter plus our cacheReconcileFailureBackoff constant (still referenced). - cmd/gc/bd_env.go — kept ours; the transport marker tables are fork-only. - cmd/gc/api_state.go — kept ours; realPathForContainment is a documented deliberate evolution, strictly stronger than the two-pass lexical check. - internal/events/rotation_archive.go — took upstream: their truncation-window reasoning (gastownhall#4628) is more correct than ours. - cmd/gc/session_reconciler*.go — genuine both-sides merge. Upstream added namedRoutedDemand, we added registry+failoverChain; the merged signature takes all three, so every call site needed both sides' arguments. Derived values re-derived from the merged tree, never adopted from a side: - resource census + TESTING.md ledger: both sides added test resources, so the union legitimately grew. Baselines raised to the merged reality, with the Small scope tracked separately from Debt (407/114, not 413/117). - product-metrics catalog: 197 = 193 upstream + 4 fork-only. Upstream had taken ids 196/197, which the fork-only commands held; those four moved to 198-201 (next_id 202). Fork-local remap only — none exists upstream. - CI execution-shape pin: ci.yml auto-merged both sides, so the merged shape hashes to neither previous pin. - dashboard dist + generated API client + OpenAPI spec: rebuilt with node 22.22.3 to match ci.yml, after confirming output is identical under node 26. Test fixes, all upstream defects surfaced by the merge on darwin rather than regressions introduced by it (each verified to fail on a clean origin/main worktree on this host): - Upstream migrated several path resolvers to pathutil.NormalizePathForCompare, which on darwin collapses /private/var and /private/tmp back to /var and /tmp — the reverse direction from bare EvalSymlinks. Assertions built with EvalSymlinks then fail on a correct result. Nine assertions across convergence, formula, materialize, sourceworkflow and cmd/gc now use testutil.AssertSamePath, the tolerant helper upstream already uses in its own newer tests in the same files. - internal/beads census: the merge left circuitTripped in both the compared and excluded field sets. Upstream gastownhall#3379 made it part of the compared reconcile end-state; that classification wins. - internal/config workquery: the merged code combines upstream's hold-label exclusions with the fork's --sort hybrid, so expectations needed both. One fork negative-probe searched for `hybrid` where it meant `oldest` and could never fire; corrected. Known, not fixed here: TestInitFromWithoutHostedPreservesTemplate leaks a dolt sql-server on darwin and trips the cmd/gc leak guard. Reproduced on a clean origin/main checkout on this host — an upstream issue, out of scope for a resync. Gates: build, go vet ./..., make lint (full repo), make dashboard-ci, ./internal/..., ./scripts/... and ./cmd/... all green. Refs: ga-y708o
Upstream's TestBuildPinnedBDBinaryForTestsMatchesGoModVersion arrives with the v1.4.0 resync and fails on the fork, because it assumes go.mod pins a RELEASE whose version string the built binary self-reports. The fork pins a pseudo-version instead, and must: no published bd release carries schema migration 0054 (v1.1.2 tops out at 0053), so go.mod names the commit directly — v1.1.1-0.20260704062855-e97839a2e1c0. A binary built from that commit reports the version the COMMIT declares, 1.1.0, never the pseudo-version string. deps.env records exactly this, in as many words: "The pinned commit declares Version = 1.1.0, hence BD_VERSION=v1.1.0." So the test compared a commit-naming pin against a release-naming stamp and could never pass here. Make it pseudo-version aware. When go.mod's pin is a pseudo-version, assert the two things that are actually meaningful: a) the binary reports the version deps.env says the pinned commit declares b) go.mod and deps.env name the SAME commit (b) is new coverage. deps.env's comments promise that lockstep — BD_SOURCE_REF is the commit go.mod pins — and until now nothing enforced it, so the two could drift silently and the bd binary under test would be built from a different commit than the one gascity compiles against. That is the exact class of ambient-drift bug ga-r9cvmi wrote this test to prevent. Release pins keep the original behaviour, so this stays correct if the fork later drops the bridge and repins to a real tag. Verified with GC_FAST_UNIT=0 so the slow gate does not skip it — a pass under the default gate would have proved nothing. Negative control: pointing BD_SOURCE_REF at a different commit fails (b) with the drift message, and a wrong BD_VERSION fails (a). Refs: ga-y708o
Collaborator
Author
CI triageFixed and pushed — This is an upstream test arriving with the merge, and it assumes go.mod pins a release whose version string the built binary self-reports. The fork pins a pseudo-version and has to: no published bd release carries schema migration 0054 (v1.1.2 tops out at 0053), so go.mod names the commit directly. A binary built from that commit reports what the commit declares — So the test compared a commit-naming pin against a release-naming stamp and could never pass here. It is now pseudo-version aware, asserting the two things that are meaningful:
(2) is new coverage. Verified with Not fixed here —
|
…vacuous
A high-effort review of the resync found that my darwin fix destroyed the
tests it was meant to keep green. Verified independently before fixing.
testutil.AssertSamePath canonicalizes BOTH sides through
pathutil.NormalizePathForCompare — which resolves symlinks. Every function I
pointed it at is ITSELF the canonicalizer under test, so the helper re-did the
work being asserted and the assertion became a tautology: an implementation
that resolved nothing at all still passed.
Proved in-tree before changing anything: CanonicalPath("<root>/alias/missing")
== CanonicalPath("<root>/real/missing"), so AssertSamePath(buggyGot, want)
passes on the unresolved value.
Four regression tests were affected — formula canonicalExistingPath (walk-up
past two missing levels), formula descriptionFileBaseDir, materialize
canonicalizePath, and sourceworkflow canonicalScopeRef/canonicalCityPath.
Each guards a canonical-path fix whose regression would silently mis-key
scope refs against beads stored under the real path.
Add testutil.AssertCanonicalPathEquals, which normalizes ONLY the expectation.
That still tolerates the darwin /private-alias spelling difference that made
the raw compares fail on a correct result, while a got that was never resolved
now fails. All 11 conversions move to it.
Negative control, the one I owed the first time: mutating canonicalizePath,
canonicalExistingPath and descriptionFileBaseDir to return their input
unresolved now fails their test. Under AssertSamePath all three passed.
Also from the same review:
- internal/beads/caching_store.go — remove cacheReconcileFailureBackoff. The
merge commit justified keeping it as "still referenced"; it is not, and was
not. `git grep` finds only the declaration, and an unused constant fails no
gate. The merged code runs upstream's exponential schedule exclusively, so
the record was wrong and the constant was dead weight implying otherwise.
- cmd/gc/productmetrics_command_census.json — restore the semantic key order
both parents use. I had re-serialized the 292-entry manifest with
sort_keys=True, which buried the five real changes in a whole-file reformat,
destroyed blame, and would have conflicted wholesale on every future merge.
Content is unchanged; the diff against fork/main is now 202/172 lines rather
than the entire file.
- cmd/gc/cmd_wait_test.go — drop the `git rev-parse` subprocess I had added to
find the repo root, in favour of a filesystem walk-up. CI caught it: it was
a new call site against the resource-census debt ratchet. Better to not grow
the ratchet than to raise its baseline for a test helper.
Refs: ga-y708o
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
17 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(gastownhall). 0 behind after this.101 conflicts, but small in substance: 63 were dashboard dist bundles, 12 workquery goldens and 4 other generated artifacts. Those were regenerated from the merged tree, never picked from a side. Genuine source divergence was 13–22 lines per file.
Conflict resolutions worth reviewing
internal/resilience/breaker.gointernal/resilience/registry.goSetJitterForTestis fork-only test infrastructure with live consumers.internal/beads/caching_store.godependsOnStepIDsparameter + ourcacheReconcileFailureBackoff(still referenced).cmd/gc/bd_env.gocmd/gc/api_state.gorealPathForContainmentis a documented deliberate evolution, strictly stronger than the two-pass lexical check.internal/events/rotation_archive.gocmd/gc/session_reconciler*.gonamedRoutedDemand, we addedregistry+failoverChain; the merged signature takes all three, so every call site needed both sides' arguments.Derived values — re-derived, never adopted
next_id202). Fork-local remap only; none of the four exists upstream. Without this the generator errored on a duplicate id andgc provider rotate-keysilently vanished from the catalog.ci.ymlauto-merged both sides, so the merged shape hashes to neither previous pin.ci.yml, after confirming output is identical under node 26.Test fixes — all upstream defects, not merge regressions
Each verified to fail on a clean
origin/mainworktree on this host:pathutil.NormalizePathForCompare, which on darwin collapses/private/varand/private/tmpback to/varand/tmp— the reverse direction from bareEvalSymlinks. Assertions built withEvalSymlinksthen fail on a correct result. Nine assertions across convergence, formula, materialize, sourceworkflow and cmd/gc now usetestutil.AssertSamePath— the tolerant helper upstream already uses in its own newer tests in the same files.internal/beadscensus: the merge leftcircuitTrippedin both the compared and excluded field sets. Upstream fix(beads): exponential backoff + one-shot breaker signal for persistently-failing reconcile stores gastownhall/gascity#3379 made it part of the compared reconcile end-state; that classification wins.internal/configworkquery: merged code combines upstream's hold-label exclusions with the fork's--sort hybrid, so expectations needed both. One fork negative-probe searched forhybridwhere it meantoldestand could never fire — corrected.Known, not fixed here
TestInitFromWithoutHostedPreservesTemplateleaks adolt sql-serveron darwin and trips the cmd/gc leak guard. Reproduced on a cleanorigin/maincheckout on this host — upstream's issue, out of scope for a resync.Gates
go build,go vet ./...,make lint(full repo — the gate that fails fork PRs),make dashboard-ci,./internal/...,./scripts/...and./cmd/...all green.Refs
ga-y708o. Depends on #119 (merged).