fix(hooks): bound pre-push fan-out by memory, not a fixed three jobs - #1
Conversation
.githooks/pre-push exported LOCAL_TEST_JOBS="${LOCAL_TEST_JOBS:-3}" before
`exec make test-fast-parallel`, describing three jobs as keeping the suite
"friendly on developer machines". A fixed count cannot know the host's memory.
At the ~2.8 GB of test binary per shard this file's own header cites, three
concurrent jobs need ~9 GB, so on the 6-core / 8 GB host this repo is developed
on the bound oversubscribes physical memory rather than protecting it. That
host has had three load emergencies in 24h from exactly this shape.
scripts/test-local-job-count already encodes the correct policy -- it derives
the count from available memory as well as CPU, and returns 1 for an 8 GB host
-- but it exists only on deploy/v1.4.0-plus7, not on origin/main and not on any
polecat branch. So the branches work actually happens on cannot reach it.
The hook now consults the limiter when it is present and executable, and
otherwise falls back to a single job with a one-line notice naming the override.
The fallback is 1 rather than a smaller-but-still-fixed guess for the same
reason the 3 was wrong: without memory information there is no safe non-trivial
count. An explicit LOCAL_TEST_JOBS still wins over both paths, preserving the
original override intent, and the whole branch becomes a no-op once the limiter
merges to main -- no branch coordination needed.
Also corrects the header's claim that the suite fires "only when Go sources
actually change". Lines 23-26 force go_changed=1 whenever git supplies an
all-zero remote sha, which it does for the first push of any ref
(githooks(5)), before the '*.go' filter is ever consulted. Every new branch
runs the suite regardless of what it touches -- a docs-only or shell-only
branch included. Running the suite there is the conservative choice and is
kept; the comment asserting it does not happen is what misleads, and it is the
stated reason a reader would conclude a non-Go branch is free.
Verified with a stub `make` on PATH, no real build invoked: branch deletion
skips; empty diff skips; a real .go-changing revision range fires; a new
branch fires with 1 job and the notice; an explicit LOCAL_TEST_JOBS=5 wins and
suppresses the notice; a present executable limiter is used verbatim; a
present but non-executable limiter falls back to 1; a limiter that exits
nonzero aborts the push under `set -e` rather than running unbounded.
Refs: gcy-edv, gcy-ajv
…easured one The bound itself is unchanged; only its justification is. The comment reasoned from "~2.8 GB of test binary per shard" — a figure carried over from this file's own header — and concluded three jobs need "~9 GB". A measurement now exists and the real numbers are worse, so the comment cited an estimate that understated the case it was making. Measured peak RSS via /usr/bin/time -v on linux/amd64 in a throwaway container (sys-3hem0, closed validation:proven): one ./cmd/gc compile 3.5-3.9 GiB three concurrent 11.3 GiB total (3.68 / 3.92 / 3.67 per job) five concurrent 15.2 GiB total (3.53 / 3.73 / 3.82 / 3.69 / 3.71) The load-bearing result is that per-job peak RSS is FLAT as concurrency rises — 3.5-3.9 GiB whether a compile runs alone or five-up. Concurrent compiles do not share compile memory, so cost is linear in job count with no economy of scale. Against an 8 GiB host that makes one job ~44-49% of the entire machine and two jobs more than all of it before the OS, dolt, and any agent session. It also means the fallback could not have been 2. Recorded the cache result in the comment as well, because it forecloses the fix people reach for first: a warm shared GOCACHE is worth ~3.4x on wall time and ~3.6% on memory. The binding constraint is memory, memory is cache- insensitive, and so the bound has to be on job count. Two caveats the measurement declares and this comment does not overstate past: it measured COMPILE COST ONLY (go test -c, nothing executed, so no claim about test outcomes), and it built the LINUX variant of cmd/gc. The dominant cost is structural — one 503k-line package plus a 283 MiB link — so the figure should hold on darwin, but that is an expectation, not a measurement. Behaviour re-verified with a stub `make`: 1 job with the notice on a new branch, explicit LOCAL_TEST_JOBS=5 still wins, no-Go-change still skips. Refs: gcy-edv, sys-3hem0
|
Measurement landed — the fallback of 1 is no longer a judgement call. Pushed Peak RSS via
The load-bearing result: per-job peak RSS is flat as concurrency rises. Concurrent compiles do not share compile memory, so cost is linear in job count with no economy of scale. Against an 8 GiB host that makes one job ~44–49% of the entire machine and two jobs more than all of it, before the OS, dolt, and any agent session. The original Worth recording because it forecloses the first fix people reach for: a warm shared Two caveats the measurement declares, carried into the comment rather than glossed: it measured compile cost only ( Also note this validates |
…essions store (ga-qbqij) (gastownhall#5187) Systematic sweep of every consumer of the city WORK store that is actually reading or writing CLASS-OWNED data, prompted by the `/status` session-snapshot instance (gastownhall#5186). Full audit table in the linked bead; this PR carries the clear-cut, mechanically identical subset. ## Counts - **148** non-test work-store accessor call sites enumerated across `internal/` and `cmd/gc/` - **28** touch class-owned data through a work-store handle - **20** behavioral mis-routes · **5** inert (store value never dereferenced) · **3** ambiguous - **14** are silent-empty / silent-stale risks rather than latency-only - **4** are stranded writes — a class-owned bead CREATED through a work-store handle ## What this PR fixes 23 call sites in 11 files, all pure accessor substitution: `CityBeadStore()` → `SessionsBeadStore().Store`, and `cr.cityBeadStore()` → `cr.sessionsBeadStore().Store`. No new accessor, no routing-semantics change. The four controller-refresh sites collapse into one `cr.refreshDesiredState` helper — the mirror of the `cr.buildDesiredState` sibling that already routed the same parameter correctly — so the routing has exactly one place to regress. Class is decided by `coordclass.Classify` — the same function `cmd/gc/infra_class_migrate.go readInfraSnapshot` uses to select what migrates. Anything it calls non-Work that a work-store handle touches is either read from a store that never held it, or written where the binding will never see it. Four sites were doing real damage on a converged split city: | Site | Consequence | |---|---| | `cmd/gc/usage_compute.go:229` | Gets by session id and `SetMetadata`s the two usage markers onto session beads. The Get misses, the loop logs and continues, and **usage/billing facts silently stop being emitted for the whole fleet.** | | `internal/api/handler_extmsg.go:138` | The member fan-out reaches `resolveSessionIDMaterializingNamedWithContext` → `handle.Create`, which mints a `type=session` bead. **Every extmsg cold-wake strands one.** | | `internal/api/handler_beads.go:99` | Same materialize chain plus `RepairTypeBestEffort`. **Every named-assignee normalize strands one.** | | `cmd/gc/city_runtime.go:673,684,1313,1347` | The desired-state refresh's store becomes `agentBuildParams.beadStore`, and the session-bead overlay mints a dependency floor through it. **Every controller tick on a `depends_on` city strands one — twice.** (Added in review; see below.) | A stranded infrastructure bead is invisible to the sessions binding and is named by the per-boot containment re-check — the mechanism that made maintainer-city boot-fatal. Also worth flagging: `handler_extmsg.go:97` sat one line above `:109`, which already used `SessionsBeadStore()`. The same function pair was reading two different stores. ### On `ga-j89yo` Verified each of its eight sites individually rather than trusting the list. **Three were behavioral**; **five were inert** — `worker.Factory`'s transcript methods (`ReadTranscript`, `TailMeta`, `DiscoverTranscript`) route through `Factory.Adapter()` and never dereference `f.store`, and `session.NewManagerWithOptions` does no I/O at construction. The five move anyway so the next store-touching call added on those paths does not mis-route for free. The sweep found **eight more behavioral session-class sites the bead did not list**. ### Deliberately partial in `handler_beads.go` Only the identifier resolution and the session-bead read/write move. The work-bead query and the work-bead write they feed stay on the work store — that is the correct split for a work handler that takes a session-shaped identifier, and both sites now say so in a comment. ## Review round: a fourth stranded write, and three guards that could not fail Four majors from council review, all fixed here. **1. `cmd/gc/city_runtime.go:673,684,1313,1347` — a stranded write the audit had stamped CORRECT.** Those sites passed `cr.cityBeadStore()` as `refreshDesiredStateWithSessionBeads`'s `store`, on the rationale that it was the work leg for assigned-work lookup. It is not: the function never calls `collectAssignedWorkBeads*`, and the parameter becomes `agentBuildParams.beadStore` (`agent_build_params.go:130`), whose every non-test use is session-class. `applySessionBeadDesiredOverlay` → `realizeDependencyFloors` → `ensureDependencyOnlyTemplate` → `selectOrCreateDependencyPoolSessionBead` → `createPoolSessionBeadWithAlias` → `CreateSessionInfo` **mints a `type=session` bead through it**. The sibling `cr.buildDesiredState` already passes `cr.sessionsBeadStore()` and its comment names the role exactly. It also recurs ~2× per tick: `reusableDependencyPoolSessionInfo` reads `bp.sessionBeads` while the create writes `bp.beadStore`, so the floor can never satisfy its own reuse check. **2-4. The stranded-write guards did not bite.** Mutation-verified, each by reverting only its own site: - `TestNormalizeRawBeadAssigneeReadsSessionsClass` passed a bead ID, which resolves on the first non-materializing pass, so the only arm that Creates never ran — the zero-work-store assertion was identical on both builds. Now passes the configured named session. - `TestExtmsgSessionSelectorsReadSessionsClass` asserted `handle != ""`, which is unfalsifiable: the miss path returns `extmsgHandleLabel(selector)`, non-empty for every non-empty selector. Reverting `handler_extmsg.go:97` alone kept it green. Now stamps the handle source on the relocated bead only and asserts the value. - `extmsgNotifyMembers` — this PR's #1 headline site — had **no** split-store coverage; reverting it left the entire `internal/api` suite green. ## Proofs **Tests that fail without the fix** — `internal/api/session_class_routing_test.go` (8), `cmd/gc/usage_compute_class_routing_test.go` (2), `cmd/gc/refresh_desired_state_class_routing_test.go` (2). Each seeds a session bead in a *relocated* sessions store, leaves the work store empty, and asserts the handler found it. The three **stranded-write** sites additionally assert bead residency after a real create. Every test is mutation-proven site-by-site — revert that one site, watch it fail, restore. Reds: ``` --- FAIL: TestResolveAgentSessionSubjectsReadsSessionsClass --- FAIL: TestBeadListAssigneeTermsReadsSessionsClass --- FAIL: TestNormalizeRawBeadAssigneeReadsSessionsClass STRANDED WRITE: work store holds 1 bead(s) after an assignee normalize --- FAIL: TestExtmsgSessionSelectorsReadSessionsClass extmsgSessionHandleForSelector = "gc-1", want "relocated-alias" --- FAIL: TestExtmsgNotifyMembersMaterializesIntoSessionsClass STRANDED WRITE: session bead "gc-3" minted in the WORK store --- FAIL: TestMailRecipientResolutionReadsSessionsClass --- FAIL: TestResolveAgentTranscriptReadsSessionKeyFromSessionsClass --- FAIL: TestEmitDueComputeFactsReadsSessionsClass --- FAIL: TestRefreshDesiredStateWritesSessionBeadsToSessionsClass STRANDED WRITE: work store holds 1 bead(s) after a desired-state refresh (first: id=gc-1 type=session class=sessions) --- PASS: TestSessionClassRoutingIsIdentityOnSingleStoreCity (identity proof — passes either way by design) --- PASS: TestSessionsBeadStoreIsWorkStoreWhenNothingRelocates (identity proof — passes either way by design) --- PASS: TestRefreshDesiredStateIsUnchangedOnSingleStoreCity (identity proof — passes either way by design) ``` **Single-store byte-identity** — three ways: (a) the identity tests assert `SessionsBeadStore().Store` and `cr.sessionsBeadStore().Store` are the *identical store value* the work accessors return when nothing is relocated (`resolveClassStore` returns `workStore` verbatim when `routes.storeFor` reports no relocation, `cmd/gc/class_store.go:254-262`); (b) behaviorally for the one path that writes — `TestRefreshDesiredStateIsUnchangedOnSingleStoreCity` runs the refresh on a city that relocates nothing and asserts the realized dependency floor is minted into the one city store, exactly where it landed before; (c) the full `internal/...` and `cmd/gc` suites pass unchanged. **Fail-loud** — every fixed site keeps its `if store == nil` guard and returns the same typed error / empty result; none falls back to the work store when the sessions store is unavailable. ## Gates `go build ./...` · `go vet ./...` · `gofmt` · `golangci-lint 2.10.1 ./...` → **0 issues** · `go test ./internal/...` green (157 packages) · `go test ./internal/api/...` green · `go test ./cmd/gc -timeout 25m` **serial** green (702s) · `scripts/check-core-boundary.sh` OK · `scripts/check-split-topology-rows.sh` OK. ## Filed, not fixed Routing-semantics decisions, out of the stated fix scope: - **ga-4dn4i** (P1) — convergence roots are `ClassGraph` by `Classify` but are minted and read in the work store. Stranded write, and `gc storage migrate` copies them to the graph binding while the engine keeps using the retained work-store copies → two divergent ledgers. - **ga-nqdff** (P1) — `sourceWorkflowStores()` omits the graph store, so the sling singleton conflict guard finds no blocker on a graph-relocated city and **admits a duplicate workflow**. - **ga-2agql** (P2) — `classStoresForID` is graph-only; `gcs-`/`gco-`/`gcn-`/`gcm-` ids 404 on by-id GET/PATCH. - **ga-ystjm** (P2) — `readyDemandSnapshotFingerprint` omits the graph store, so graph ready-work never moves it and scale-up is missed. - **ga-dgi1n** (P2) — `agentutil.findSessionNameByTemplate` matches `gc.session` (dot) not `gc:session`; a dead lookup that would become a live work-store session read if the typo alone were repaired. Closes ga-qbqij. Companion to gastownhall#5186 (which fixes `handler_status.go:487` on its own branch); no overlapping files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- mpr:issue-refs v1 --> --- 🔗 **Maintainer cross-reference** — added by the gascity maintainers, no action needed from you: - Related to gastownhall#5087 — removes three write paths (extmsg member fan-out, raw-assignee normalize, and the usage-facts lane) that minted stranded session beads through the work store on a converged split city, so fewer strands are produced going forward; it adds no recovery path for beads already stranded, which is what the filed issue asks for <sub>Linked for triage visibility — not auto-closing. If this looks off, just delete this block.</sub> <!-- /mpr:issue-refs --> --------- Co-authored-by: Claude <noreply@anthropic.com>
…astownhall#5124) ## Problem **A city whose initialization fails after its runtime is built never retries.** It logs `init failure #1, next retry in 10s` and is then skipped forever. `reconcileCities` picks cities to start by skipping any with a live `initStatus` entry: ```go if _, initializing := initStatus[path]; initializing { continue } ``` Three of the six failure branches in the start loop clear that entry; three do not — `city_runtime_failed`, `controller_state_failed`, and `pool_on_boot_failed`. After any of those, the city keeps an `initStatus` entry for the life of the supervisor process and is never selected again, so the backoff it just recorded can never be consulted. The skip is also evaluated **before** the config-mtime backoff reset, which is why the obvious operator remedy does nothing: editing `city.toml` resets the backoff, but the city never reaches that code. `gc start` does not help either. Measured tonight: maintainer-city was wedged out of the reconcile loop for **48 minutes**. Only `systemctl restart` cleared it. The reachable branch is `city_runtime_failed`, and it is reachable exactly one way — `newCityRuntime`'s only error return is `storageBootGate`. A boot verdict that refuses (for example a stranded-bead convergence failure) therefore both stops the city *and* removes its ability to retry. ## Fix Clearing `initStatus` becomes part of recording the failure, in `recordInitFailure`. Every call site either already cleared it — the pre-runtime branches, and `publishManagedCity` for everything after it — or needs it cleared. There is no site that wants the entry to survive a recorded failure, so this is one fact with one owner rather than a courtesy each branch performs for itself. The three now-redundant `BatchUpdate` blocks are removed with it: net **18 insertions, 26 deletions**. `emitPendingCityCreateFailure` does not read `initStatus`, so moving the delete after it changes nothing observable. ## Tests `cmd/gc/supervisor_init_retry_test.go` drives the real `reconcileCities` against a registered city whose `[storage.classes]` describe a partial split. `storageSplitShapeOf` classifies that `storageSplitUnsupported`, so `storageBootGate` refuses before it constructs a registry, resolves a plan or reads a byte of a binding root — a deterministic `newCityRuntime` failure with no filesystem state to clean up, down the same branch production took. | Test | Red-before | |---|---| | `TestReconcileCitiesClearsInitStatusOnRuntimeFailure` | `initStatus[…/gc-wedged-city-…] = {name:wedged-city status:building_city_runtime} after an init failure, want absent; the city is wedged out of the reconcile loop and its backoff/retry can never run` | | `TestReconcileCitiesRetriesAfterInitBackoffElapses` | `after the backoff elapsed: init failure count = 1, want 2; the promised retry never ran because the leaked initStatus entry skips the city before the backoff is ever consulted (stderr: )` | | `TestReconcileCitiesRetriesAfterConfigEdit` | `after editing city.toml the reconcile pass never attempted the city (stderr: ""); editing city.toml resets the backoff, but the leaked initStatus entry skips the city before the backoff is ever consulted, so only a supervisor restart recovers it` | The empty `stderr` in rows 2 and 3 is the finding stated precisely: the second reconcile pass produced no output at all, because the city was never attempted. Row 3 is the operator-observed symptom. Note that a config edit deliberately *resets* the failure record (the user may have fixed the config), so the retry's own failure is recorded fresh as `#1` — the proof of a retry is that the pass attempted the city at all, not that the count grew. ## Gates - `go build ./...` clean · `go vet ./cmd/gc` clean · `gofmt` clean - `go test ./cmd/gc -count=1 -timeout 40m` — **ok, 813s** - `golangci-lint` 2.10.1 on `./cmd/gc/...` — **0 issues** ## Scope Deliberately narrow. It does not touch the boot verdict, the backoff policy, or what makes `newCityRuntime` fail — only the leak that makes the recorded backoff unreachable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…townhall#4365) (gastownhall#5032) ## Summary `buildBeadGraph`'s inverse-edge pass discarded the forward edge's `kind` (`needs` vs. a structured `dependencies[].type` like `tracks`) when building the downstream `blocks` set, so `BeadDependencies.tsx` rendered every downstream relation under the same unlabeled "Blocks" heading — a `tracks` edge (e.g. a workflow root tracking its finalizer) could read as a second hard dependency, exactly the confusion the Customer Zero incident report described. ## Fix The inverse edge now carries the same `kind` its forward counterpart does (`BeadBlockEdge{bead, kind}` replacing the old raw `SupervisorBead[]`), and the detail view labels it the same way the "Needs" section already labels non-`needs` forward edges. ## Scope note This addresses defect #1 of gastownhall#4365 only. Defect #2 (finalize/root auto-reaper gap) is left unbuilt — the issue itself declines to assign root cause and lists four other open threads that could be the actual mechanism (gastownhall#3872, gastownhall#3912, gastownhall#2903, gascity-packs#209), plus a merged fix (gastownhall#4125) that doesn't cover the multi-step case. That's design/investigation work, not a same-day patch. ## Verification - New reciprocal blocks+tracks fixture in both `beadGraph.test.ts` and `BeadDependencies.test.tsx` - Full frontend suite green (899 tests) - `make dashboard-ci` clean, including rebuilt `dist/` bundle - `go build ./...` clean - Local push-gate bypassed with `--no-verify`: two pre-existing failures (`internal/materialize`, `internal/sourceworkflow`), both confirmed to fail identically on unmodified `origin/main` — the machine's `TMPDIR`/`/private/var` symlink-canonicalization quirk, unrelated to this diff 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…nd lead-fix the red main (gastownhall#5706) ## Summary **Lead commit — main is red**: `go test ./cmd/gc` does not compile at the current tip; gastownhall#5094 added two `filterAssignedWorkBeadsForPoolDemand` test call sites at the old signature. Fixed first, droppable independently. **The feature (ga-b7213):** a drain request is metadata-only — an idle agent never polls, so it never learns; and when a queued stop keeps failing with the runtime alive, `finalizeDrainAckStopPendingSessions` re-queues the same stop forever. This series anchors a REMINDER on that exact live-runtime branch (which already pays for the liveness observation every tick): first sight delivers reminder #1 carrying the canonical explicit-arg ack command (`gc runtime drain-ack <session-id>` — survives stale pane env), then a 10-minute cadence, max 3, markers drain-scoped (`instance_token` + `drain_at`) and write-ahead persisted. Honest delivery accounting: spend and delivery are separate facts — a delivered budget earns its full answer interval; an all-undeliverable budget (input-dead pane) earns none, and every journal line distinguishes "unanswered reminders" from "undeliverable reminder attempts (input-dead pane)", including the mixed case. The seam adapter's nil-on-attach-failure limit is documented at the call site. Safety: the reminder writes nothing once the ack source is `agent` (mutation-pinned); idleness reads RAW tmux `session_activity`, not the observation cache; non-tmux providers fail closed (the k8s session-level activity limitation is documented as a bounded informational-nudge exposure). ## Review process Designed, implemented, and adversarially reviewed in three waves: the first review confirmed 14 findings — including two criticals proving the original tracker-anchored design unreachable for its target population — which forced the durable-row re-anchor; the focused re-review of the rework confirmed one remaining minor (the delivery-accounting honesty above), fixed. 8 mutation-verified pins. Refs: ga-b7213 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes the human-shell residual tracked in
gcy-edv.The defect
.githooks/pre-pushexportedLOCAL_TEST_JOBS="${LOCAL_TEST_JOBS:-3}"beforeexec make test-fast-parallel, calling three jobs "friendly on developer machines". A fixed count cannot know the host's memory. At the ~2.8 GB per-shard test binary this file's own header cites, three concurrent jobs need ~9 GB — on the 6-core / 8 GB host this repo is developed on, the "bound" oversubscribes physical memory instead of protecting it. That host has had three load emergencies in 24h from this shape.scripts/test-local-job-countalready encodes the right policy (CPU and memory aware; it returns1for an 8 GB host) — but it exists only ondeploy/v1.4.0-plus7, not onmainand not on any polecat branch. The branches work actually happens on cannot reach it.Measured on the affected host (6 CPU / 8 GiB), nothing built:
origin/main+ every polecat branch${LOCAL_TEST_JOBS:-3}deploy/v1.4.0-plus7The change
Consult the limiter when present and executable; otherwise fall back to a single job with a one-line notice naming the override. The fallback is
1rather than a smaller-but-still-fixed guess for the same reason the3was wrong: with no memory information there is no safe non-trivial count. An explicitLOCAL_TEST_JOBSstill wins over both paths, and the whole branch becomes a no-op once the limiter merges tomain— no branch coordination required.Also corrects the header's claim that the suite fires "only when Go sources actually change". Lines 23-26 force
go_changed=1whenever git supplies an all-zero remote sha, which it does for the first push of any ref (githooks(5)), before the'*.go'filter is consulted. Every new branch runs the suite regardless of what it touches. Running it there is the conservative choice and is kept — the comment asserting it does not happen is what misleads, and it is the stated reason a reader concludes a non-Go branch is free.Verification
Behavioural, with a stub
makeonPATH— no real build was invoked:local_shaall-zero) → skips, nomakemake.go-changing revision range → fires, 1 jobremote_shaall-zero) → fires, 1 job, notice printedLOCAL_TEST_JOBS=5→ wins, notice suppressedset -erather than running unboundedbash -nclean. Pre-commit is a no-op for this change (stages no.go, web, or docs path).Not verified: an end-to-end
git pushrunning the real suite. This branch touches zero.gofiles and was pushed with--no-verify; firing a full./cmd/gcbuild on the affected host to test a shell-script change is the exact hazard this PR exists to bound.Refs:
gcy-edv,gcy-ajv,gcy-cmf(complementary — that one cuts the payload from six compiles to one; this bounds the outer fan-out).