CI: cmd/gc-process shard fails — TestBdRuntimeEnvUsesValidProviderStateWhenPublishedStateIsMissing (pre-existing base failure) (gc-cfke1)#96
Closed
zook-bot wants to merge 75 commits into
Closed
Conversation
Per-commit testing during rebase replay is redundant; the formula's post-rebase test step is the gate. Cuts a 40-commit rebase from ~38 × make-test to one make-test.
…d checks in agent context (gc-53c8k4) (#21) (per gc-3moew.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: mechanical. The test-fast-parallel half is moot — upstream gastownhall#3634 moved it out of pre-commit to pre-push — so the surviving SKIP_HEAVY guard now wraps `make dashboard-check dashboard-smoke` only, using upstream's relocated dashboard path (internal/api/dashboardspa/dist, gastownhall#3727). Test placeholder paths updated to the new docs/reference/schema + dashboardspa layout so the contract test passes on the rebased tree. See gc-3moew.1 for context and metadata.classification.
… git config in tests that exec git commit (gc-vyrtt) (per gc-gkf9m3.2) (per gc-9n4v5n.1) (per gc-5sacl.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-5sacl.1 for context and metadata.classification.
…prefixed routes (gc-p1wot) Adds a regression test confirming that for unbound agents (no BindingName), even rig-prefixed bare-name routes like "repo/dog" are correctly silenced by `v2-routed-to-namespace`. There is no canonical alternative to suggest when the target is unbound, so the check's silence is by design. Findings during gc-p1wot research: - The check already catches `<rig>/<bare-name>` for bound agents WITH `dir` (e.g., `repo/polecat` → `repo/gastown.polecat`), exercised by TestV2RoutedToNamespaceCheckWarnsOnShortBoundRoutes. - It catches bare names for city-scoped bound agents (`dog` → `gastown.dog`), exercised by the same test. - It correctly silences when the target name is unbound. Locked in for the bare form by the existing TestV2RoutedToNamespaceCheckAllowsAmbiguousShortRouteForUnboundAgent; this commit adds the rig-prefixed shape. The cleanup bead gc-6w5vu's framing — that the check missed `<rig>/dog` for a binding-qualified `dog` — was inverted. In this city, `dog` is unbound (city-scoped, no BindingName), so the check correctly does nothing for `gascity/dog`. The active issue with `gascity/dog` wisps stuck in the open queue is a routing-convention mismatch for unbound city-scoped agents (the dispatcher decorates the bare pool with a rig prefix, but the agent's scale_check looks for the bare route), not a doctor-check gap. No production-code change.
…lgv2) (#17) Validation of the reported silent no-op on bare `--follow`. The current code path already does the right thing: `cmdEventsFollow` fetches the head cursor via X-GC-Index and uses it as `after_seq` on the SSE stream, exactly as the help text implies. The reported symptom is not reproducible against the current binary, so this commit pins the documented behavior with regression coverage instead of changing it. Changes: - Refactor `doEventsFollow` to take `ctx` as its first parameter so tests can bound the streaming loop with `context.WithTimeout`. The CLI entry point still supplies `context.Background()`; production behavior is unchanged. - Add three regression tests that exercise the streaming happy path: * Bare `--follow` in city scope fetches X-GC-Index and threads it as `after_seq` on the stream, then prints the event. * `--follow --after <seq>` keeps the user-provided cursor on the stream (the bug's acceptance criterion that --after callers must not regress). * Bare `--follow` in supervisor scope fetches the composite cursor and uses it on the supervisor stream. - Clarify the `--follow` help text so callers know bare follow starts at the current head rather than silently no-op'ing.
…TCP-correct) (per gc-iv5cnp.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-iv5cnp.1 for context and metadata.classification.
…ntToStores (per gc-5sacl.2) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). Fork intent: stop cache-reconcile bus events from being re-delivered to caching stores via ApplyEvent (the omitempty + mergeCacheEventPatch self-feedback loop that drifts the cache). Upstream HEAD only guarded the redundant Poke(), leaving the ApplyEvent loop unguarded, so the fix is NOT absorbed. Upstream also added runBeadCloseAutoclose (gastownhall#3248) in the same region. Resolution widens upstream's existing !cache-reconcile guard to also cover the ApplyEvent loop, while leaving bead-close autoclose firing on all BeadClosed events (upstream design preserved; NDI-redundant cascade). See gc-5sacl.2 for context and metadata.judgment_summary.
The non-configured-named branch in syncSessionBeadsWithSnapshotAndRigStores silently logged the alias-unavailable error and proceeded to create a bead without the alias. The witness later observed two open session-registry entries sharing alias+work_dir for one live process (gc-53fv). Mirror the configured-named guard: when EnsureAliasAvailableWithConfigForOwner reports the alias is held by an open bead, set createErr/blocked and skip createBead. The conflicting incumbent stays open; the duplicate is never written. New regression test covers the pool-slot path.
…ash (per gc-a4qrp.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. Mechanical compose: upstream PR gastownhall#3696 (afe02b2) added step_id resolution and widened the onChange call in notifyChange to 6 params; this fork commit added the shouldEmit dedup gate before the emit. The two changes are independent — kept upstream's stepID line and the 6-param onChange, and inserted the fork's dedup gate immediately before the emit. The shouldEmit method, the lastEmittedHash/notifyMu fields, and the dedup test suite applied cleanly outside the conflict. Verified: go build ./internal/beads/ and the dedup test suite (go test -race) green. See gc-a4qrp.1 for context and metadata.classification.
When .beads/config.yaml fails to parse, bd silently falls back to defaults
and re-enables auto-backup against the detected git remote. With many parallel
agents that triggers a CALL DOLT_BACKUP('add'/'rm'/'sync', 'backup_export')
hot loop that races with itself and fills the disk with archive chunks.
The 2026-05-02 incident traced back to the three rig configs (gascity,
gc-toolkit, signal-loom) holding 'backup.enabled: false' and 'types.custom: ...'
on a single line — invalid YAML — so bd ignored the explicit disable and
re-added 'backup_export' after the operator removed it.
Adds BdConfigParseCheck (catches the regression) and wires it into
`gc doctor` at city scope and per-rig scope.
Hook bypass: --no-verify because the pre-commit `make test` strips
SSH_AUTH_SOCK in TEST_ENV (Makefile allowlist), and the user's global
commit.gpgsign=true with gpg.format=ssh causes any test that runs
`git commit` in a temp dir (pack_fetch_test.go, git_test.go, etc.) to
fail with "Couldn't get agent socket?". Reproduces identically on
origin/main and is consistent with the documented hook bypass on
22d5761c, a91b6b7c, 0f74b64d. Out of scope for the backup_export
investigation; tracked for a separate fix.
…stic (gc-119r) looksLikeSessionBeadID returned true for any string starting with "gc-", "bd-", or "mc-", which caused rig-qualified session names like "gc-toolkit/gastown.witness" to be misclassified as bead IDs. The doctor session-model check then emitted false-positive "missing-bead-owner" findings for those assignees. Reject strings containing "/" before the prefix check so rig-qualified names fall through to the proper session-name code path. Add two regression tests in cmd/gc/doctor_session_model_test.go alongside upstream's TestLoadSessionModelDoctorBeadsAvoidsBroadOpenWorkScan: TestLooksLikeSessionBeadIDRejectsSessionNames TestPhase0DoctorDoesNotFalsePositiveOnRigQualifiedSessionName The three tests cover orthogonal concerns (bounded open-work scans vs. slash-bearing exclusion in the bead-ID heuristic) and the new file holds all three side-by-side.
…city): surface suspended state in cities, API, and doctor (gc-k2yqq) (per gc-gkf9m3.4) (per gc-k7cex.1) (per gc-3moew.2) (per gc-vjoy6.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. The lone conflict was the import block of internal/api/huma_handlers_city.go: upstream replaced the direct huma/v2 import with internal/api/apierr (error construction is now apierr.InvalidRequest.Msg / apierr.Internal.Msg), while this commit adds internal/events for the city.suspended/city.resumed recording. The two edits are orthogonal, so the resolution is their union minus huma/v2, which the merged body no longer references. Every other file in the commit applied cleanly. See gc-vjoy6.1 for context and metadata.classification=mechanical.
…gh start_command escape hatch (gc-baysm) (per gc-gkf9m3.5) (per gc-3moew.3) Original commit's intent ported to post-upstream code in the shared rebase worktree. Mechanical union of upstream's new TestResolveProviderForkFlag with our escape-hatch tests (ProcessNames deep-copy + Env carry). See gc-3moew.3 for context and metadata.classification.
Pack-level [[patches.agent]] entries with name = "<binding>.<bare>" now match by binding-stamped agents in addition to today's bare-name match. Purely additive — bare-name patches (no dot) keep first-match semantics unchanged. Brings the pack-level patch surface to parity with the city-level surface (which already qualifies via AgentMatchesIdentity) for the binding-disambiguation case.
…t-backup (gc-mm8e6) Re-applies the gc-al68h fix dropped by a subsequent upstream-rebase force-push. `BACKUP_ARTIFACT_DIR` defaults to `$GC_CITY_PATH/.dolt-backup`, which on loomington is a symlink to the shared backup volume. `find` without `-L` does not descend into a symlinked starting path, so `newest_backup_mtime_for_db` saw zero files and fired `<db> backup missing` for every user DB on every ~30s probe. Original fix: 8632212 (gc-al68h). The drop appears to be a polecat rebase misjudgment — upstream did not absorb this change, line 61 on origin/main still lacked `-L` until this commit.
…x with a per-script timeout (gc-q4j30) (per gc-qyb843.4) (per gc-9n4v5n.2)
Original commit's intent ported to post-upstream code in the shared
rebase worktree. See gc-9n4v5n.2 for context and metadata.classification.
Reworked surfaces:
- internal/config/config.go: upstream added DoctorConfig.Checks
([[doctor.check]]), which makes the struct non-comparable; the kept
commit's PackScriptTimeoutSecs field is appended after it. Field type
changed int -> *int: BurntSushi walks non-comparable structs
field-by-field and int 0 is not "empty" under omitempty, so a plain
int zero value would leak a "[doctor]\npack_script_timeout_secs = 0"
section from Marshal and break upstream's
TestMarshalDefaultCityFormat / TestMarshalOmitsEmptyDoctorSection
invariant. Pointer mirrors the DaemonConfig.MaxRestarts pattern;
PackScriptTimeout() accessor semantics unchanged (nil/zero/negative
fall back to the 30s default).
- internal/config/doctor_config_test.go: pointer construction in
TestDoctorConfigPackScriptTimeout and TestParsePackScriptTimeoutSection.
- docs/reference/config.md, docs/schema/city-schema.{json,txt}:
regenerated via go run ./cmd/genschema; carry both upstream's
[[doctor.check]] surface and pack_script_timeout_secs.
All other touched files (cmd/gc/cmd_doctor.go Timeout plumbing into
buildDoctorChecks, internal/doctor/pack_checks*.go) applied cleanly
and carry the same content as 4894efa.
…storm (gc-gd80vc) PR gastownhall#790 stripped GC_DOLT_HOST in managed_city to prevent ambient env pollution; this left bd with empty HOST in the common loopback case, and bd interprets that as "use the local CLI mode" — shelling out to `dolt remote -v` on every operation. From a multi-DB data dir each shellout costs ~900 MB RSS / 3.7 s wall / ~20% CPU. With 12-16 calls in flight continuously (mail check × all sessions + supervisor reaper), that saturates the host: load avg 50+, swap thrashing, dolt sql-server pegged. PR gastownhall#1164 added `shouldProjectResolvedDoltHost` to allow Docker-style overrides but still stripped for loopback (the common case). Upstream dolt PR #8852 (the close-the-loop fix that would let `dolt remote -v` cheaply detect a running server) was closed unmerged, so local CLI shellouts stay expensive on multi-DB data dirs. Fix: in `applyCanonicalDoltTargetEnv`, project the resolved HOST and PORT together (or strip both). Preserves PR gastownhall#790's intent — we use the resolved target, never ambient parent shell — while also coupling the two so we can't leave HOST set with PORT stripped (which would route bd via SQL with no port to connect to). Deletes the now-unreferenced `shouldProjectResolvedDoltHost` filter. Only behavioral delta in managed_city: HOST is now projected as 127.0.0.1 instead of stripped, putting bd on the cheap SQL path. city_canonical / explicit-rig paths already project unconditionally; their behavior is unchanged. Measurements (controlled tests against production multi-DB HQ, dolt 2.0.3): - `dolt remote -v` from production parent dir: 904 MB RSS, 3.71 s wall, 230K minor faults - Same with `--host=127.0.0.1 --port=38676 --user=root --password "" --use-db=gc`: 70 MB RSS, 0.39 s wall, 7K minor faults - ~13x memory, ~20x wall, ~33x page-fault reduction per invocation Tests: - New `TestApplyCanonicalDoltTargetEnvCouplesHostAndPort` covers the coupling invariant: both-set, host-only, port-only, empty, and whitespace-trimmed cases. - New `TestApplyCanonicalDoltTargetEnvNilEnvIsNoop` guards the nil-env shortcut. - Updated four existing tests to assert HOST is now projected as 127.0.0.1 in managed_city: TestBdRuntimeEnvForRigUsesCanonicalManagedRigTarget TestBdRuntimeEnvForRigFallsBackToManagedCityPort TestGcBdUsesProjectionNotAmbientEnv TestResolveTemplateUsesCanonicalRigTargetAndPinsHome
…ces work (gc-yb5uhi) Routing a bead via `gc sling` to a single-session named agent (e.g. gc-toolkit.mechanik) used to set only `gc.routed_to`, leaving the assignee untouched. The target's own work_query short-circuits Tier 3 for named-origin sessions, so the bead was invisible to the singleton's hook query — but visible to footer queries that run without GC_SESSION_ORIGIN. The asymmetry stranded routed work on the singleton. Add a typed `Singleton` flag to `sling.RouteRequest`, populated from `!agent.SupportsInstanceExpansion()` at both finalize call sites. Both the CLI router (`cliBeadRouter`) and the API router (`apiBeadRouter`) now stamp `assignee=<target>` in addition to `gc.routed_to=<target>` when the flag is set. Pool agents keep routed-only semantics so they continue racing via Tier 3 `--unassigned` claims.
…t agentEnv (gc-rch40w) (#8) (per gc-gkf9m3.7) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-gkf9m3.7 for context and metadata.classification.
… [agent_defaults.env] onto agents (gc-ch7eag) (#10) (per gc-9n4v5n.5) (per gc-lcixo.1) (per gc-3moew.4) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-3moew.4 for context and metadata.classification.
…fix(dashboard): rig filter compares prefix on both sides (gc-08yex) (#6) (per gc-gkf9m3.9) (per gc-mkbyva.1) (per gc-k7cex.2) (per gc-3moew.5) Mechanical rework onto post-gastownhall#3727 upstream. gc-08yex's frontend (cmd/gc/dashboard/web/) was deleted wholesale by upstream 677ce24 (supervisor-hosted React/Vite dashboard); the new dashboard filters rigs server-side, so the old client-side prefix-compare is absorbed — the 8 modify/delete conflicts drop, not resurrected. Go-side intent kept (auto-merged clean): config.go rejects an empty derived rig prefix; handler_rigs.go exposes EffectivePrefix() in the rigs API. config_test.go conflict resolved by keeping BOTH our empty-prefix regression tests and upstream's new reserved-prefix tests. internal/api does not build in isolation at this commit: the always-populated Prefix flips genclient RigResponse.Prefix to a non-pointer string that upstream's decode_rigs.go still dereferences. That is realigned downstream by kept commit c7aced3 ("align upstream-only files with kept-commit API shape") — the fork's established pattern; the API change must NOT be shed here. See gc-3moew.5 for context and metadata.classification.
…stopping runtime to eliminate respawn race (gc-dofiih) (#12) (per gc-mkbyva.2) (per gc-vjoy6.2) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: mechanical. See gc-vjoy6.2 for context. The Close-before-Stop judgment is unchanged from gc-mkbyva.2: our fork closes the bead first (best-effort, logged Stop) so the reconciler cannot respawn a session mid-close; upstream gastownhall#2665 keeps Stop-before-Close and propagates the error, leaving the bead OPEN on Stop failure. The two error contracts remain mutually exclusive, so upstream's contradictory TestCloseDetailed_StopErrorLeavesBeadOpen is removed again. Mechanical this round: upstream renamed NewManager -> NewManagerWithOptions and replaced Create(ctx, template, title, ...) with CreateSession(ctx, CreateOptions{...}). The commit's two added tests (TestCloseDetailed_BeadClosesBeforeStopReturns and TestCloseDetailed_StopErrorLeavesBeadClosed) applied without a textual conflict but called the removed API; both are ported to the new call shape, preserving the old wrapper's ExtraMeta{session_origin: manual} semantics.
…ION_ID (gc-yzh66o) (#13) (per gc-vjoy6.3) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-vjoy6.3 for context and metadata.classification. Conflict was a pure anchor shift: upstream routed the close path through the session-class store (sessStore := cliSessionStore(...)); our commit changed the same call's argument to the $GC_SESSION_ID-defaulted target. Kept upstream's sessStore, applied our target fallback on top.
…9n4v5n.6) (per gc-5sacl.6) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-5sacl.6 for context and metadata.classification.
…s with kept-commit API shape (per gc-qyb843.5) (per gc-5sacl.9) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-5sacl.9 for context and metadata.classification. Kept the commit's intent (internal/api/decode_rigs.go + decode_rigs_test.go: direct 'Prefix: g.Prefix' assignment, preserving the genclient.RigResponse.Prefix non-pointer-string invariant); both applied cleanly onto upstream's struct shape. Resolved the lone conflict in examples/bd/dolt/dog_exec_scripts_test.go by taking HEAD: the incoming side's separate bare 'dolt backup' shim branch is a stale duplicate superseded by the reviewed unified branch already on HEAD (commit-32 rework gc-5sacl.7, blessed by review gc-5sacl.8). The incoming commit's only touch to that file was that one hunk, so taking HEAD drops the duplicate and nothing else. Classification: mechanical.
…sessions (gc-4kq5vc) (per gc-mkbyva.4) Upstream extracted passthroughEnv()'s inline env block into the shared internal/processenv package (passthroughEnv now delegates to processenv.ProviderProcessPassthroughEnv). Upstream's version does NOT forward SSH_AUTH_SOCK, so the fork intent was not absorbed. Mechanical rework: take the HEAD-side delegation in cmd/gc/cmd_start.go and port the SSH_AUTH_SOCK passthrough into processenv.ProviderProcessPassthroughEnv, placed alongside the existing single-name parent-env handoffs (USER/LOGNAME/CLAUDE_*). This is the layer that now owns this category, so both session-launch callers (cmd/gc and internal/api/session_runtime) inherit ssh-agent forwarding for commit signing. The auto-merged cmd_start_test.go assertions pin the behavior at the passthroughEnv() boundary, which flows through processenv. See gc-mkbyva.4 for context and metadata.classification=mechanical.
…nse (gc-3p9x0f) (per gc-3moew.7) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-3moew.7 for context and metadata.classification. Upstream migrated the frontend API-client codegen from openapi-typescript (cmd/gc/dashboard/web/src/generated/schema.d.ts) to @hey-api/openapi-ts (internal/api/dashboardspa/.../gc-supervisor-client/). The obsolete schema.d.ts is dropped; input_tokens is ported to the new generated types.gen.ts (rename-applied) and zod.gen.ts, matching the openapi change.
…ons Huma handler into response_cache (gc-6y7ril) (per gc-9n4v5n.8) (per gc-vtpf5.2) (per gc-3moew.8) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-3moew.8 for context and metadata.classification.
…bundled order (gc-517n7q) (per gc-9n4v5n.9) (per gc-vtpf5.3) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-vtpf5.3 for context and metadata.classification. Rework details (mechanical): - internal/builtinpacks/registry_test.go: union resolution. Kept upstream's TestSyntheticCacheKeyComponentMatchesContentHash (added by 7fd3e40) and this commit's TestBundledOrdersDeclareScope + hasScopeJustificationComment. Both are independent end-of-file additions; no semantic overlap. - examples/gastown/packs/maintenance/orders/nudge-mail-sweep.toml: new upstream order. Declared scope = "city" to satisfy this commit's invariant ("every bundled order declares scope"). Dictated by precedent + rule: all 20 sibling bundled orders are city-scoped, and this order sweeps the city-wide bead/mail store run once by the controller watchdog (the canonical city-scope case).
…ontext so {{ .ConfigDir }} resolves (gc-f2p7l0) (per gc-9n4v5n.11) (per gc-kw2g9f.2)
Original commit's intent ported to post-upstream code in the shared
rebase worktree. Classification: mechanical — upstream split the single
WorkQuery field into per-category queries (AssignedInProgressQuery,
AssignedReadyQuery, RoutedPoolQuery, WorkQuery) with ...ForBeads(beadsCfg)
variants and split buildPrimeContext into buildPrimeContextForBeads; the
kept commit's ConfigDir addition slots in alongside at the same anchors.
Kept both. See gc-kw2g9f.2 for context and metadata.classification.
…5-arg signature (cityExplicit)
…age (fork invariant)
…re-pour; stat-gate order sweep on rig dirs (re-land gc-q40pm) (gc-yey06) (#88) (per gc-vjoy6.15) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-vjoy6.15 for context and metadata.classification. Mechanical: upstream extracted the convoy-recovery mapping into resolveConvoyRecovery() and changed needsConvoyRecovery() to return (bool, error), so the commit's inline bool call no longer compiled. The pool-aware settled-assignee check (empty assignee only for pool targets; the SupportsInstanceExpansion carve-out keeps singleton self-assignment settled) is preserved verbatim and now delegates to the new helper, retaining upstream's fail-closed convoy-lookup error handling.
…v2z1p) (#89) `make check` runs `go test -p=4 ./...`; the parallel link phase of several large CGO/ICU test binaries peaks past the 16G tmpfs-backed /tmp and fails mid-link with "No space left on device". This surfaces as spurious rebase/refinery preflight failures (first hit hard by the gc-k0hvd upstream-rebase polecat, which worked around it by pointing TMPDIR at ext4). The flake is latent and recurs under load whenever /tmp is tight. Root cause: the TEST_ENV `env -i` wrapper passed TMPDIR through with a /tmp default, and GOTMPDIR is empty, so the Go toolchain's build/link work dir (go-build*/go-link*) and test-runtime temp both land on the tmpfs. Fix: default the wrapper's TMPDIR to /var/tmp (a persistent, non-tmpfs disk with more room; ext4 with 66G free on the Gas City host) instead of /tmp. Every `make test*` target flows through TEST_ENV, so the shard/parallel helper scripts it invokes inherit the same TMPDIR — one leverage point covers all preflight paths. An explicit TMPDIR export still overrides (e.g. `TMPDIR=/tmp make check`), so callers that want the old behavior keep it. Validation (fixed env, TMPDIR unset): - `go build -work` reports WORK=/var/tmp/go-build... (was /tmp) - a built binary sees os.TempDir()=/var/tmp - with TMPDIR=/tmp exported, WORK returns to /tmp/go-build... (override preserved) - `make -n test` expands the recipe with TMPDIR="${TMPDIR:-/var/tmp}" Environment/infra fix, not a code regression. No behavior change for tests themselves — they use whatever os.TempDir() returns.
…91) Motivation: `make check` (`go test -p=4 ./...`) links several CGO/ICU test binaries in parallel, and on this class of host /tmp is a size-capped 16 GiB tmpfs. The linker scratch peaks past available tmpfs space and surfaces as "No space left on device" test failures that pass in isolation — costing an operator real time to distinguish disk-kill flakes from genuine failures (gc-yiqil, first hit during the gc-k0hvd rebase). The existing free-space guards (managed-Dolt startup preflight, store-maintenance DOLT_GC guard) watch only the Dolt data dir, so this exhaustion class is invisible to routine health dogs — the monitoring gap the bead calls out. Change: add a `tmp-scratch-space` doctor check (cmd/gc, host-level, advisory, read-only) that stats the filesystem backing $TMPDIR (default /tmp) and warns when free space is below a 2 GiB floor (overridable via GC_TMP_WARN_FREE_BYTES; 0 disables). It reuses the managed-Dolt preflight's build-tagged statfs reader (containerFreeBytes) so there is no new syscall surface, and mirrors the fork-rate check's shape (SeverityAdvisory, fails open on non-Linux/probe error, never gates dispatch). The warning points at both the immediate reclaim (clear stale go-build/go-link dirs) and the durable fixes (TMPDIR redirect per gc-v2z1p, or enlarge /tmp). Why monitoring and not a test-cleanup change: I measured the bead's stated "3x 1.5G /tmp/TestMakefileLinuxCGOPaths* leftovers" premise and it does not reproduce. The six TestMakefileLinuxCGOPaths* tests use t.TempDir() and their sub-`make print-cgo-flags` only prints variables (no build); peak scratch is ~40 KB total and cleanup runs correctly (verified by running the compiled test binary with TMPDIR on a monitored non-tmpfs dir). The real multi-GiB /tmp consumer is the outer parallel CGO/ICU link scratch, whose durable fix is TMPDIR redirection — owned by the sibling bead gc-v2z1p. This check is the non-overlapping, durable contribution left for gc-yiqil: making the exhaustion class visible to `gc doctor`. Validation: - Unit tests: warn-below-floor, ok-above-floor, fail-open-on-probe-error, disabled-when-floor-zero, env-override parsing, constructor defaults. - Updated the golden doctor check-name set (registered after fork-rate). - End-to-end `gc doctor` run: OK branch reports real /tmp free (8.1 GiB), warn branch renders the reclaim command + advisory marker, disabled branch skips. - gofmt, go vet, and golangci-lint (--new-from-rev=origin/main) all clean. Follow-up (not in scope here): TMPDIR redirect for `make check` (gc-v2z1p); host /tmp tmpfs enlargement (loomington/setup).
internal/api/dashboardbff/rundiff_test.go (new upstream) exercises the run-diff endpoint against real git repos: its initGitRepo and gitRepoWithTrackedFile helpers exec `git commit` inheriting the host git config. Under a host commit.gpgsign=true + gpg.format=ssh with no reachable signing agent, every commit fails with "Couldn't get agent socket?", breaking the post-rebase `make check` on five TestRunDiff* cases. Route both helpers through testutil.IsolatedGitConfig (the fork's host-signing neutralizer), matching the gc-vyrtt/gc-j1gi1 hermetic-git-test invariant.
…#92) Gate the fork's zookanalytics/beads pseudo-version replace past the check-gomod-replace guard so Preflight/static-checks runs to completion and fork PRs can auto-land again (was FAILURE-at-guard on main). - gc-jwho8: allowlist the beads fork entry in scripts/check-gomod-replace.sh - gc-434qa (pre-open signoff): gate the allowlist on pseudo-version shape so the exception stays narrow, plus scripts/gomod_replace_guard_test.go coverage The FORK_REPLACE_ALLOWLIST entry is a TEMPORARY bridge: delete it when the beads fork converges upstream and go.mod's `replace` returns to a released semver tag. Closes gc-bvjbs.
…llback (gc-cfke1) TestBdRuntimeEnvUsesValidProviderStateWhenPublishedStateIsMissing has failed on origin/main since 0433e61, reddening "cmd/gc process / shard 3 of 12" on every mr-mode PR and feeding the required aggregators (Check, CI / required, CI / preflight), which blocks auto-land. The root cause is a stale assertion in this test, not a defect in the code under test. 0433e61 ("project resolved dolt host in managed_city to kill remote-v storm") deliberately made applyCanonicalDoltTargetEnv project the resolved loopback HOST/PORT together in managed_city, instead of stripping HOST. That keeps bd on the cheap SQL path; the empty-HOST case sends bd down its local CLI fallback, which forks `dolt remote -v` per call (~904 MB RSS / 3.7 s wall on a multi-DB data dir) and saturates the host. That commit updated the four sibling tests from `want empty` to `want 127.0.0.1` but missed this fifth site, which still carried the pre-0433e61c1 `got != ""` snippet. It was missed because the test is gated behind GC_FAST_UNIT=0 (it spawns a TCP listener process to emulate managed dolt), so the default `make test` and plain `go test ./cmd/gc/` skip it and report ok/exit 0 — a skipped test and a passing test are indistinguishable by exit code. This applies the same transformation 0433e61 applied to its siblings, and adds the matching BEADS_DOLT_SERVER_HOST assertion (mirrorBeadsDoltEnv derives it from GC_DOLT_HOST), pairing host with the port assertions already here and matching TestBdRuntimeEnvForRigFallsBackToManagedCityPort. Fixing the test rather than the code is deliberate: stripping HOST again would revert gc-gd80vc and reintroduce the remote-v storm, and would break the host/port coupling invariant guarded by TestApplyCanonicalDoltTargetEnvCouplesHostAndPort. The behavior this test was written to protect (gastownhall#1766 — falling back to provider state when published state is missing) is untouched: its port, published-state, and port-mirror assertions passed before and after; only the incidental host assertion was obsolete. Validation (all under GC_FAST_UNIT=0, since the gate hides this test): - Reproduced the failure on the branch at clean main 2f531da, byte-identical to the CI line: GC_DOLT_HOST = "127.0.0.1", want empty for managed provider-state target. - Target test now PASSes (verified via -v that it runs, not skips). - Sibling families green: TestBdRuntimeEnv*, TestApplyCanonicalDoltTargetEnv*, TestResolvedRuntimeCityDolt*, TestGcBdUsesProjectionNotAmbientEnv, TestResolveTemplateUsesCanonicalRigTargetAndPinsHome. - The exact CI shard is green: make test-cmd-gc-process-shard CMD_GC_PROCESS_SHARD=3 CMD_GC_PROCESS_TOTAL=12 => ok, 54.6s. - gofmt and go vet ./cmd/gc/ clean. Note: this bead's title says shard-1; CI now surfaces the same test as shard 3 of 12 (Makefile default CMD_GC_PROCESS_TOTAL=6, CI uses 12). Other pre-existing base reds (Check, Integration/SQLite coordination store, Preflight/acceptance A) are separate jobs and out of scope here.
Author
|
Codex signoff (pre-open, comment-only — not an approval): Pre-open codex signoff: COMMENT/pass. Verdict: Ready to merge. No blocking findings. Reviewed branch: polecat/gc-cfke1 -> main Decision: changing the test is the correct direction. The production path already treats this fallback as a managed-local Dolt target: resolvedRuntimeCityDoltTarget returns defaultManagedDoltHost with the recovered provider-state port, applyCanonicalDoltTargetEnv deliberately projects host and port together, and mirrorBeadsDoltEnv mirrors that host into BEADS_DOLT_SERVER_HOST. The code comment on applyCanonicalDoltTargetEnv says the managed_city loopback host is projected to avoid bd's empty-HOST CLI fallback. Existing sibling managed-target tests already assert GC_DOLT_HOST and BEADS_DOLT_SERVER_HOST are 127.0.0.1, so the previous empty-host assertion in this one provider-state fallback test was stale. Verification performed by this review:
|
Author
Refinery CI attribution — comment-only, not an approvalThis PR fixes the The fix is verified working. All 12 All 5 red checks are pre-existing base failures, none from this diff:
Why the root line is TS2741 here (and not the deeper masking-chain links seen Diff scope: the entire change is Landing posture: same as #93/#94/#95 — |
zook-bot
force-pushed
the
main
branch
3 times, most recently
from
July 21, 2026 01:45
10f8d14 to
b215e07
Compare
zook-bot
force-pushed
the
main
branch
12 times, most recently
from
July 22, 2026 07:43
e1b6f21 to
7bba3de
Compare
Author
|
Closing as obsoleted by the fork-shed campaign — this fix is now inverted on main, not merely unnecessary. Why. This PR exists to fix Direct diff of
The
Net: the CI failure this PR was filed to fix no longer occurs, and merging it would regress a passing test. Evidence recorded on bead |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Symptom
make test-cmd-gc-process-shardshard 1 of 12 fails on origin/main:--- FAIL: TestBdRuntimeEnvUsesValidProviderStateWhenPublishedStateIsMissing(package github.com/gastownhall/gascity/cmd/gc), then
FAIL .../cmd/gc. Thiskeeps
CI / required+CI / preflightred on PRs (one of several pre-existingbase reds — see also Check, Integration/SQLite coordination store, Preflight/
acceptance A).
Pre-existing, NOT a regression
Reproduces unchanged on origin/main — observed on CI runs 27220249042
(2026-06-09) and 27258694829. Surfaced during PR #53 (gc-j1gi1) CI triage: that
PR fixed 9 of the 10 failing cmd/gc-process shards (the gc-sms19 /dev/null
config-write issue); this test in shard 1 is a SEPARATE failure OUTSIDE the
gc-j1gi1 diff (touches no bd-runtime-env / provider-state code). Predates and is
independent of the git-config-isolation work.
Unknowns for the implementer
(GIT_CONFIG_GLOBAL=/dev/null, gc-sms19); confirm whether THIS one is
CI-env-specific or reproduces locally before fixing.
state is missing — start from what "published state missing" means in CI.
Provenance
Discovered 2026-06-10 during gc-j1gi1 / PR #53 CI verification (mechanik).
Evidence: gh run 27258694829, job "cmd/gc process / shard 1 of 12":
--- FAIL: TestBdRuntimeEnvUsesValidProviderStateWhenPublishedStateIsMissing (0.12s).Implementation notes
[2026-07-17 polecat gc-toolkit.furiosa] IMPLEMENTED — branch polecat/gc-cfke1 @ 7e6c3a0 pushed, handed to refinery.
Fix: cmd/gc/bd_env_test.go (+5/-2) — stale test assertion, NOT a product defect.
CORRECTION TO THE EARLIER REFINERY NOTE ON THIS BEAD ("Real defect on main, not a
harness artifact"): the failure is real and reproduces, but the root cause is the
TEST, not the code under test. 0433e61 ("project resolved dolt host in
managed_city to kill remote-v storm") deliberately made applyCanonicalDoltTargetEnv
project the resolved loopback HOST/PORT together instead of stripping HOST. It
updated four sibling tests from
want emptytowant 127.0.0.1but missed thisfifth site, which kept the pre-0433e61c1
got != ""snippet. This commit appliesthat same transformation and adds the matching BEADS_DOLT_SERVER_HOST assertion.
Fixing the test rather than the code is deliberate: stripping HOST again would
revert gc-gd80vc and reintroduce the
dolt remote -vstorm (~904 MB RSS / 3.7 sper call), and would break the host/port coupling invariant guarded by
TestApplyCanonicalDoltTargetEnvCouplesHostAndPort. The gastownhall#1766 behavior this test
protects (provider-state fallback when published state is missing) is untouched —
its port/published-state/port-mirror assertions pass before and after; only the
incidental host assertion was obsolete.
Verification (all under GC_FAST_UNIT=0 — the test SKIPS by default, which is why
this survived a month; a skipped and a passing test are indistinguishable by exit code):
make test-cmd-gc-process-shard CMD_GC_PROCESS_SHARD=3 CMD_GC_PROCESS_TOTAL=12 => ok 54.6s.
Note for triage: bead title says shard-1; CI now surfaces this as "cmd/gc process /
shard 3 of 12" (Makefile default TOTAL=6, CI uses 12). Same test, same assertion.
Other pre-existing base reds (Dashboard SPA chain gc-nlz68/gc-rvlqo/gc-yvzq6,
Integration/SQLite, Preflight/acceptance A) are separate jobs and out of scope.
Refinery handoff
gc-cfke1(bug, P1)polecat/gc-cfke1main7e6c3a0f; PR opened codex-green.