fix(dolt): surface + auto-clear stale compact quarantines; label backup-sync timeouts (gc-h7mc0tz) - #1
Open
vbtcl wants to merge 1 commit into
Open
fix(dolt): surface + auto-clear stale compact quarantines; label backup-sync timeouts (gc-h7mc0tz)#1vbtcl wants to merge 1 commit into
vbtcl wants to merge 1 commit into
Conversation
…up sync timeouts
A transient post-flatten value-hash quarantine silently disabled ALL
compaction/GC for a DB with no alert and no auto-staleness, letting the
noms journal grow toward the corrupted-journal city-down threshold
(beads_hq sat quarantined 16 days; journal 5.1G, data dir 13G). Three
improvements close the gap:
1. mol-dog-doctor: emit a [HIGH] health advisory whenever an active
compact-quarantine marker exists (it silently disables GC on critical
infra). Counts only valid-db-name markers, matching the compactor's own
has_compact_marker lookup, so operator archives like
beads_hq.stale-cleared-20260607 do not false-alarm.
2. compact: auto-clear a quarantine marker older than
GC_DOLT_COMPACT_QUARANTINE_STALE_SECS (default 6h) once the DB reads
clean (row counts) and is quiescent (whole-DB value hash stable across
two probes a settle apart), then retry compaction. The post-flatten
re-verification re-quarantines if real drift remains, so auto-clear is
a supervised retry that never bypasses integrity enforcement or GCs
unverified data. Kill switch: GC_DOLT_COMPACT_QUARANTINE_AUTOCLEAR=0.
3. mol-dog-backup: distinguish a sync timeout (run_bounded rc 124 ->
"sync timed out >120s; likely journal bloat/size", surfaced in the mail
subject) from a generic sync error ("sync failed rc=N"), so journal
bloat is diagnosable from the advisory.
Tests: 7 new hermetic tests in dog_exec_scripts_test.go (auto-clear when
quiescent, keep-fresh, keep-when-writer-active, kill switch, backup
timeout vs error, doctor advisory) plus a quarantine_writer_active fake
mode. Full examples/dolt suite green except two pre-existing,
environment-specific failures unrelated to this change
(TestCompactScriptRealDoltRemotePush: dolt 2.0.7 ANSI color in remote-HEAD
parse; TestRuntimeScriptManagedStateBeatsStaleEnvPort: port-resolve env).
Refs gc-h7mc0tz.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vbtcl
pushed a commit
that referenced
this pull request
Jun 16, 2026
…(ga-c4w) (gastownhall#3103) ## Summary Makes the mouse wheel drive **tmux copy-mode scrollback** in interactive `gc` sessions instead of leaking the wheel to the focused TUI (Claude Code's own history, a pager, or the shell) — durably and out-of-the-box — while **headless agent sessions stay mouse-off** (controller-poll safety). This is the proper in-source fix that supersedes the portharbour city-local `po-vtg2` `set-hook` stopgap. Two facts made the wheel inert before this change, so the fix has two parts: - **Part B — runtime default (`internal/api/session_runtime.go`).** `sessionCreateHints` now sets `MouseOn: true`. The runtime skips `disableMouseAndActivity` only when `MouseOn` is true (`internal/runtime/tmux/adapter.go:930`), so the `mouse on` set at session create (`tmux-theme.sh`) survives and the wheel binding can fire. This seam flips exactly the two human-interactive callers — provider-adhoc (`session_resolved_config.go`) and named sessions (`session_resolution.go`). The headless agent path resolves `MouseOn` from `cmd/gc/template_resolve.go` (`cfgAgent.MouseModeOn()`) and is **not** involved → stays mouse-off. - **Part A — pack binding (`examples/gastown/.../tmux-keybindings.sh`).** Adds root-table `WheelUpPane → copy-mode -e` / `WheelDownPane → send-keys -M` bindings (forces copy-mode even over mouse-reporting apps so scrollback wins; Shift+wheel keeps native terminal selection). **No** `client-attached` `set-hook` stopgap — the `MouseOn` default replaces the prototype's. > Why `sessionCreateHints` and not the bead's suggested `mouse_mode='on'` > template default: provider/named sessions build their runtime hints solely via > `sessionCreateHints`; their synthetic `&config.Agent{}` is discarded after > provider resolution, so a template `mouse_mode` would never reach them. The > hints builder is the minimal correct seam, and it keeps the change off the > agent-template path entirely (guaranteeing headless behavior is unchanged). ## Micro-tasks (TDD red→green, per-task commits) | task | commit | test | | --- | --- | --- | | T-001/T-002 interactive mouse-on default | `19d6a9cdf` | `TestSessionCreateHintsEnablesMouse` | | T-003 headless stays mouse-off (guard) | `fe1c2149f` | `TestResolveTemplateHeadlessAgentStaysMouseOff` | | T-004/T-005 pack wheel binding + no stopgap | `6bc2d400a` | `TestTmuxKeybindingsScrollWheel` | | T-006 build + targeted tests + CHANGELOG | `0745b53d8` | — | ## Testing Run under the hermetic `env -i` wrapper (Makefile `TEST_ENV`) + `icu4c@78` CGO flags. - `go build ./...` → **Success** - `go test ./internal/api/... ./examples/gastown/...` → **1635 passed** - `go test ./cmd/gc/...` → all ga-c4w tests pass. **Pre-existing, unrelated failures (not introduced here):** `TestBdRuntimeEnvManagedCityProjectsHostOverride` and `TestBdRuntimeEnvForRigInheritedManagedCityProjectsHostOverride` fail **identically on base `dd3ee8524`** with none of this branch's changes present (managed-Dolt host-override port resolution; the local sandbox's proxied-server setup does not produce the override). `TestProbeDetachedWork_TmuxExitStatus` timeouts were host-env flakes that pass under the hermetic `env -i` wrapper. ## Manual verification (acceptance #1, gastownhall#3 — not unit-testable) After merge + pack roll, in a fresh interactive `gc session new <provider>`: 1. Wheel-up in a Claude pane enters copy-mode scrollback; wheel-down scrolls down and exits at the bottom. 2. Mouse pane-select, drag-resize, the `MouseDown1StatusRight` mail popup, and Shift+wheel native selection all still work. 3. A headless agent session shows `mouse off` (`tmux show-options -t <sess> mouse`). ## For the reviewer (open questions, downstream-resolvable) 1. **`monitor-activity` side-effect.** `MouseOn=true` skips the whole `disableMouseAndActivity`, so interactive sessions also keep `monitor-activity on` — same as `mouse_mode=on` agents already get, benign for a human-attended session. Split the helper (mouse conditional, activity always) only if you want activity off regardless. Out of scope unless flagged. 2. **`WheelDownPane send-keys -M` at bottom of scrollback** — exit-clean is covered by manual verification #1. ## Compliance - **GDPR:** no-op. Governs tmux mouse-mode / key bindings for dev tooling; no personal or special-category data read, written, transmitted, or logged. - **MDR Class I:** no-op. Outside the voxmemo → voxist-api clinical pipeline. ## Follow-up (separate, not this PR) Removing the portharbour city-local `po-vtg2` stopgap is a separate city-store task to file once this ships and the gastown pack is rolled. Refs: ga-c4w (supersedes po-vtg2). Plan: `docs/plans/durable-mouse-wheel-scrollback.md`. --------- Co-authored-by: Eric Cestari <eric@escapevelocity.fr>
vbtcl
pushed a commit
that referenced
this pull request
Jun 16, 2026
…ession) (gastownhall#3139) ## Summary Post-merge regression fix for **ga-c4w / PR gastownhall#3103**. `internal/api` `sessionResumeHints` emitted `MouseOn: true` **unconditionally** for every resumed session — including pool/headless agents resumed through the API worker factory — re-enabling tmux mouse on controller-polled sessions and breaking ga-c4w's controller-poll-safety invariant. This is human reviewer **sjarmak's MAJOR #1** (review 4437810731), which was dismissed and merged without a code fix. ## Root cause `resolveWorkerSessionRuntimeWithMetadata` (wired as the worker factory's `ResolveSessionRuntime` in `worker_factory.go`) calls `sessionResumeHints` and builds `runtime.Config` **directly** — it never routes through `cmd/gc/template_resolve.go`. So the in-code assumption that headless agents "re-resolve MouseOn mouse-off downstream" did not hold for this path, and a resumed pool agent got mouse **on**. `MouseOn` has exactly one consumer (`internal/runtime/tmux/adapter.go`: `if !cfg.MouseOn { disableMouseAndActivity }`), so `MouseOn=true` means mouse is not disabled. ## Fix Gate `MouseOn` on an explicit interactive signal instead of hardcoding `true`: - `sessionResumeHints(..., interactive bool)` sets `MouseOn: interactive`. - `sessionResumeInteractive(metadata)` derives it from `session_origin == "manual"`, mirroring the create-path gate `templateParamsSessionOrigin(tp) == "manual"` in `templateParamsToConfig`. - Both resume call sites (`buildSessionResume`, `resolveWorkerSessionRuntimeWithMetadata`) pass the metadata-derived signal. Only interactive (human-attached) resumes keep mouse-on. Pool/headless resumes — and any unknown/empty origin — resolve mouse-**off** (the safe direction: never enable mouse on a polled agent). ## Test plan - **RED→GREEN:** new `TestResolveWorkerSessionRuntimeResolvesMouseOnlyForInteractiveResume` exercises the real worker-factory resolver (`resolveWorkerSessionRuntimeWithMetadata`, not a stub) for both cases: pool agent (`session_origin=worker`) → `MouseOn=false`, interactive (`session_origin=manual`) → `MouseOn=true`. Failed first on the pool case (`MouseOn = true, want false`), passes after the fix. - `TestSessionResumeHintsEnablesMouse` extended with the `interactive=false` → `MouseOn=false` case (previously proved only the true case). - `go test ./internal/api/` green; `go vet ./internal/api/` clean. Refs ga-g7go, ga-c4w #1 (sjarmak review 4437810731), PR gastownhall#3103. Co-authored-by: Eric Cestari <eric@escapevelocity.fr>
vbtcl
pushed a commit
that referenced
this pull request
Jun 16, 2026
…rted before creation_complete) (gastownhall#3466) (gastownhall#3503) Fixes the crash-loop reported in gastownhall#3466 (sibling of gastownhall#3109; relates to gastownhall#534): a tmux-transport agent whose work_dir loads a project-scoped MCP server blocks on Claude Code's "New MCP server found in this project" trust modal, which a headless managed agent cannot answer, so the session-create handshake aborts ("aborted before creation_complete") and `mode=always` agents crash-loop. Defense in depth, two independent commits: 1. **Preventive** — `enableAllProjectMcpServers: true` in the projected Claude settings template (`internal/hooks/config/claude.json`), next to the existing `skipDangerousModePermissionPrompt`. The modal never renders for projected agents. (Issue ask #2.) 2. **Reactive** — a new MCP-trust dialog class in `internal/runtime/dialog.go` that selects option 2 ("Use this and all future MCP servers in this project"), covering agents gc does not project settings for. (The narrow, still-open piece of gastownhall#534 / issue ask #1.) ### Verification - Reproduced and fix-checked the modal directly against Claude Code 2.1.177 in a throwaway tmux session: an untrusted project `.mcp.json` renders the modal on launch; the same launch with `enableAllProjectMcpServers: true` in the `--settings` file goes straight to the prompt with no modal. (Note: `-p`/print mode does not render the project-MCP gate, so it cannot reproduce this — the modal only appears on the interactive tmux launch path.) - Tests: extended `TestInstallClaude` to assert the key reaches the projected `.gc/settings.json`; added matcher + peek + stream tests in `internal/runtime/dialog_test.go`. - `make check` (fmt, lint, vet, full test suite) green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sjarmak
pushed a commit
that referenced
this pull request
Jun 28, 2026
…l (ga-oa8173) (gastownhall#3611) ## Summary - `integration-sqlite-coordstore` job was orphaned in ci.yml (no `needs:` references, no fan-in) and was the #1 CI pole at 13.5min per push - Move to nightly.yml keeps SQLite coordstore coverage without blocking every PR - nightly.yml uses ubuntu-latest matching the existing nightly job pattern Bead: ga-oa8173 ## Test plan - [ ] ci.yml no longer contains `integration-sqlite-coordstore` - [ ] nightly.yml contains the moved job - [ ] No timeout-bump commits included (branch is clean off origin/main) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: quad341 <james@wordelman.name> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
sjarmak
pushed a commit
that referenced
this pull request
Jun 28, 2026
…t read collapse (review-formulas wedge) (gastownhall#3626) ## What & why The **Review Formulas** workflow was ~85% red on `main` (last 30 runs: 22 fail / 4 pass / 4 cancelled). It is **not a required check** (required = `Check` + CodeQL `Analyze`, both green), but it runs on every push to `main` and on PRs, so it is the visible "main is broken" signal — and it represents a real production defect, not just a test flake. ### Root cause (RCA + workflow-based adversarial review) In `TestPersonalWorkFormulaCompileAndRun`, after the design-review `compose.expand` fan-out the single managed Dolt sql-server suffers a **read-side saturation collapse**: every `gc hook` ready-query times out (`rc=124`, `timeout 10`) for ~22 min until the 24-min deadline, so no agent can read the ready queue and the molecule never advances. Replacement polecats also time out — the **store itself is wedged**, not a stuck process. It is a cumulative-load threshold crossing, not a single-commit regression (`04eef3468` is a non-causal WARN→DEBUG log change). Two stacking amplifiers on the per-op-subprocess store model: 1. **(dominant)** A polecat `scale_check` makes pool demand non-event-backed, so `demandSnapshotsEnabled()` is false and `shouldRefreshDemandSnapshot` rebuilt the full desired state — including the `scale_check` subprocess `bd ready --metadata-field gc.routed_to=polecat --limit=0` — on **every** patrol tick. At `patrol_interval=100ms` that ran ~10×/s, and the metadata-filtered probe cannot use `COUNT(*)` (`doltliteCountSupported` bails on metadata filters) so it fell back to a full hydrated `List` each time. 2. **(permanence)** A query killed by `timeout 10` (SIGKILL, no clean `COM_QUIT`) orphans a server-side `Sleep` connection until `read_timeout` (was 30s), so under load orphans accumulated faster than they reaped. ## The fix (3 parts) - **`fix #1` (reconciler, the real fix):** floor patrol re-eval of a non-event-backed (`scale_check`) demand snapshot to `scaleCheckDemandMinInterval = 1s` via the new `demandSnapshotPatrolMaxAge()`. Non-patrol triggers (config reloads, sling pokes), config changes, session-fingerprint changes, and the no-event-provider case still rebuild immediately / every tick. **No-op at the 30s default `patrol_interval`** — it only bites pathologically fast (sub-second) cadences. - **`fix gastownhall#3` (config hardening):** lower `DefaultDoltReadTimeoutMillis` 30000 → 15000 so orphaned per-call `Sleep` connections reap sooner. `read_timeout` is the listener idle / inter-row produce reaper (go-mysql-server `ErrRowTimeout` re-arms per row), **not** a live-query wall-clock timeout, so it cannot cut a long but steadily-producing query. Regenerated schema/docs. - **`fix #2` (test):** `review_formula_test` `patrol_interval` 100ms → 1s and bound the `scale_check` probe with `--limit=8` (pool ceiling is 3). ## Verification - New unit tests: `TestCityRuntimeDemandSnapshotThrottlesScaleCheckPatrolReeval` (throttle cadence + interval-elapse + poke-bypass + fingerprint-change rebuild) and the updated `…CachesCustomDemandCommands` table. - Full `go test ./cmd/gc/` (659s), `internal/config`, `internal/doctor`, `test/docsync` all green; `go build` + `go vet` clean; `genschema` idempotent. - **Workflow-based adversarial review** (16 agents, 10 candidate findings): 9 refuted as false alarms against the code/trace; the 1 confirmed finding was a comment-accuracy issue (fixed here). `read_timeout=15000` confirmed safe (idle/inter-row reaper, not a wall-clock query cut); `cfg==nil` throttle path confirmed unreachable in production. > "Wait for green" note: a single Review Formulas pass is not decisive (it was green ~15% even while broken). The deterministic unit test is the primary signal; CI on this PR is the integration confirmation. ## Follow-ups (out of scope; can file as beads) - Make `scaleCheckDemandMinInterval` configurable for cities needing sub-1s scale_check reaction. - Have the control-dispatcher poke the controller after creating routed pool work (mirroring sling), if sub-second fan-out scale-up is ever required. - Reduce the per-tick session-snapshot `store.List` fan-out. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sjarmak
pushed a commit
that referenced
this pull request
Jun 28, 2026
…-m3ev9r) (gastownhall#3625) ## What `gc` city-targeting commands now accept a **registered city name** (as shown by `gc cities`) in addition to a path. `gc unregister chris-city`, `gc stop chris-city`, `gc reload prod`, `gc start --city prod`, `GC_CITY=prod gc status`, etc. all work — no more "I typed the name and nothing happened." Applies to the **positional arg** of `unregister`, `stop`, `start`, `restart`, `reload`, `suspend`, `resume`, `status`, plus the persistent `--city` flag and `GC_CITY`. `register` stays path-only (you can't register *by* a name that doesn't exist yet) but gains name *hints* in completion. Shell completion (`completeCityNames`) is wired on all of them. This is the follow-up to **gastownhall#3623** (which made `gc unregister` *fail loudly* on an unknown name/path instead of silently reporting success). Design doc: `engdocs/contributors/design-gc-city-name-resolution.md`. > **Note on base:** this branch is stacked on gastownhall#3623, so until that merges this PR's diff includes its one commit (`de3735111`). Merge gastownhall#3623 first and this reduces to the feature commits. ## How — shape-classify-then-route The load-bearing constraint (verified against `findCity`): the existing path resolvers **walk *up*** the directory tree. So a naive "try path first, fall back to name" is unsafe — a bare name run from inside any city silently resolves to the **ambient ancestor city** (and `--json` reports `ok:true` for the wrong city). The adversarial design review caught this. So the arg is classified by **shape** before any resolution (a registered name can't contain `/`, per `validCityName`): - **path-shaped** (`/abs`, `./x`, `../x`, `~/x`, separator) → existing path resolver, byte-for-byte unchanged. - **name-shaped + a local `cwd/<name>` city** → that local city (path wins); if it also matches a *different* registration → **loud ambiguity error**. - **name-shaped + no local city** → `Registry.LookupCityByName` only — **never** the walk-up resolver. - **neither** → loud not-found error (preserves gastownhall#3623). `restart` resolves the reference **once** and threads the resolved path into both the stop and start legs. `GC_CITY` uses path-first/local-wins precedence (documented — an ambient env var differs from explicit arg/flag input). ## Key files - `internal/supervisor/registry.go`: `LookupCityByName` + `IsValidCityName` (mirror `LookupRigByName`). - `cmd/gc/city_arg_resolve.go`: `classifyCityRef`, `resolveCityNameRef`, `resolveCityRef`, `resolveCityFlagValue` — the single shared seam; each command injects its own path resolver as a closure. - `cmd/gc/main.go` (`resolveCommandContext`, `resolveContext`), `cmd/gc/city_context.go` (`GC_CITY`): central wiring covering reload/suspend/resume/status + `--city`. - `cmd/gc/cmd_{register,stop,start,restart}.go`, `cmd/gc/completion.go`. ## Process Built phase-by-phase (primitives → resolver → wiring), then ran a **30-agent adversarial review** (5 dimensions × verify): 8 confirmed of 25 findings (3 medium, 2 low, 3 nit; 17 refuted as intentional/pre-existing), all addressed in the final commit. The #1 review target — any path where a bare name still reaches a walk-up resolver — found none. ## Test plan - Resolver matrix incl. the **from-inside-a-city walk-up guard** (`resolveCityRef`, `resolveCommandCity`, `resolveStopCityPath`). - Central seams: `resolveCommandCity` by name (+ walk-up guard + unknown-name loud failure), `resolveCityFlagValue`, `GC_CITY` by name + local-wins + `GC_CITY_PATH` path-only. - `restartTarget` single-resolution; `resolveStartDir` by name; `unregister` by name; completion prefix filtering. - Backward-compat: path-shaped args unchanged (existing resolution/`RigAnywhere` tests stay green). - Full local gate green per commit: `lint-changed` (0), `go vet ./...`, all fast shards (`test-local-parallel fast`), `check-docs`. ## Deferred ga-xgfs92: per-command `--city` flags that bypass the central chain (`gc bd/import/analyze`) accepting a name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
quad341
pushed a commit
that referenced
this pull request
Sep 6, 2026
…ed inherited rig (ga-5k989) (gastownhall#5484) Fixes ga-5k989. Also fixes root cause #1 of ga-yc5rc. ## The bug `ensureCanonicalScopeMetadataIfPresent` was named "IfPresent" but had require semantics: its first act was to call `requireCanonicalScopeMetadata`, which returns `missing canonical metadata <path>` when `.beads/metadata.json` does not exist. One of its three callers is the loop over a city's *inherited rig plans*, so a single rig registered with the city but never initialized made every `gc beads city use-external` / `use-managed` fail: ``` gc beads city use-external: canonicalizing inherited rig metadata: missing canonical metadata /tmp/adopt/.beads/metadata.json ``` exitCode 1 at controller startup -> CrashLoopBackOff with no recovery path (ga-hnle2). It killed city `fresh-03479` (ga-5k989) and, with `/workspace/aprig/.beads/metadata.json`, `gc-controller/controller-2053b66b9f7f03d87b37a865` over 186 restarts (ga-yc5rc root cause #1). ## The three call sites and their policies | Call site | Scope | Policy | | --- | --- | --- | | `cmd/gc/cmd_rig_endpoint.go` `doRigSetEndpoint` | the rig named by `gc rig set-endpoint <rig>` | **fails closed** — unchanged | | `cmd/gc/cmd_beads_city.go:208` | the city's own store | **fails closed** — unchanged; the city is the scope being reconfigured | | `cmd/gc/cmd_beads_city.go:220` | each *inherited* rig plan | **skips when metadata.json is absent** — this was the defect | The first two now call `requireCanonicalizedScopeMetadata` (renamed, behaviour byte-identical to today). The third calls the new `canonicalizeScopeMetadataIfPresent`, which `Stat`s the path and returns `nil` only on `errors.Is(err, os.ErrNotExist)` — no string matching — and otherwise delegates to the require variant. Absent means absent and nothing else. A metadata.json that *exists* but pins no `dolt_database` is a misconfigured store, not an uninitialized one, and still fails. `requireCanonicalScopeMetadata` itself is untouched. The rename is deliberate: reusing the old name would have let a caller keep the wrong policy silently. Renaming forced the compiler to surface all five call sites (3 production, 2 test). ## Test evidence These tests are the point of the PR, so here is proof they discriminate rather than an assertion that they do. ### 1. Before the fix — new test is RED with the production message ``` === RUN TestDoBeadsCityUseExternalToleratesUninitializedInheritedRig/rig_root_vanished cmd_beads_city_test.go:624: doBeadsCityEndpoint() = 1, want 0; stderr = rig "adopt" still declares path in city.toml; ... gc beads city use-external: canonicalizing inherited rig metadata: missing canonical metadata /data/tmp/.../003/adopt/.beads/metadata.json --- FAIL: TestDoBeadsCityUseExternalToleratesUninitializedInheritedRig (0.13s) --- FAIL: .../rig_root_vanished (0.10s) --- FAIL: .../rig_root_present_but_never_initialized (0.03s) === RUN TestDoBeadsCityUseExternalRejectsUnusableInheritedRigMetadata --- PASS: TestDoBeadsCityUseExternalRejectsUnusableInheritedRigMetadata (0.03s) ``` The failure message is byte-identical to the one from the ga-5k989 incident. The malformed-metadata control already passed pre-fix, as expected — that path fails closed today and must keep doing so. ### 2. After the fix, revert ONLY the production change, keep the tests — RED again ``` --- FAIL: TestDoBeadsCityUseExternalToleratesUninitializedInheritedRig (0.08s) --- FAIL: .../rig_root_vanished: doBeadsCityEndpoint() = 1, want 0; ... canonicalizing inherited rig metadata: missing canonical metadata .../adopt/.beads/metadata.json --- FAIL: .../rig_root_present_but_never_initialized --- FAIL: TestCanonicalizeScopeMetadataIfPresentSkipsOnlyAbsentMetadata/absent_metadata_is_not_an_error_and_fabricates_nothing cmd_rig_endpoint_test.go:249: canonicalizeScopeMetadataIfPresent: missing canonical metadata .../never-initialized/.beads/metadata.json ``` ### 3. Make the skip too broad (swallow every Stat error) — the controls catch it ``` --- FAIL: TestDoBeadsCityUseExternalRejectsUnusableInheritedRigMetadata (0.01s) cmd_beads_city_test.go:669: doBeadsCityEndpoint() = 0, want 1; ... --- FAIL: TestCanonicalizeScopeMetadataIfPresentSkipsOnlyAbsentMetadata/metadata_without_a_pinned_dolt_database_still_errors cmd_rig_endpoint_test.go:266: canonicalizeScopeMetadataIfPresent error = <nil>, want missing pinned dolt_database ``` ### 4. With the fix — GREEN ``` go test ./cmd/gc/ -run 'RigEndpoint|CanonicalMetadata|StorageModeRewrite|BeadsCity|CanonicalizeScopeMetadata|EveryDoorThatFlips' -count=1 ok github.com/gastownhall/gascity/cmd/gc 2.889s ``` `TestDoRigSetEndpointRequiresCanonicalMetadata` is untouched and green — the named-scope door still fails closed. The new tests deliberately do **not** call `skipSlowCmdGCTest`, so they run in the default fast suite instead of becoming a check that cannot fail. ## Also `TestEveryDoorThatFlipsTheStorageModeAnnouncesIt` grew from 2 table entries to 3 (`init path`, `endpoint path, named scope`, `endpoint path, inherited rig`), so the announce guarantee now covers both endpoint doors rather than one. ## Gates - `go build ./...` — clean - `go vet ./...` — clean - `gofmt -l cmd/gc/` — clean - `make test-fast-parallel` — All fast jobs passed (10 jobs, all 6 `cmd/gc` shards) - `make test-cmd-gc-process-parallel` — all 6 `cmd-gc-process` shards ok. The `productmetrics-testhook` job fails, but it fails identically on the unmodified base commit `be633d9c37` in a disposable worktree, so it is pre-existing and unrelated (this change touches no startup or pack-discovery path): `TestProductMetricsTaggedBinaryProcessContracts/control_flow_bypasses_city_and_pack_state: gc help exceeded process deadline` - pre-commit hook ran (`core.hooksPath` = `.githooks`): `lint-changed: ./cmd/gc 0 issues.` ## Note on branch base This branch was cut from the worktree's HEAD, which carries two commits not yet on `origin/main` (`be633d9c37`, `89426fa3ab`). They ride along in this PR.
quad341
pushed a commit
that referenced
this pull request
Sep 6, 2026
…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>
quad341
pushed a commit
that referenced
this pull request
Sep 6, 2026
…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>
quad341
pushed a commit
that referenced
this pull request
Sep 14, 2026
…e journal-confirmed marker (kill the ~757/min re-materialization storm without losing self-healing) (gastownhall#6277) ## Summary The control-dispatcher re-emits `execution.step_defined` for a run's **entire** step graph on **every** control tick (`EmitCurrent` at `cmd/gc/cmd_convoy_dispatch.go`, driven by the serve loop). Observed at **~757/min sustained** (199,738 of 200,000 recent events were this one call), filling `events.jsonl` and drowning real progress. (ga-rd8le) ## Why not edge-triggered An earlier edge-triggered rewrite (emit only newly-created steps) was rejected: it **violates the codebase's convergence-via-restatement reliability model** — `EmitCurrent` re-states the full projection every tick precisely so a definition missed on a failed/crashed creator pass self-heals next tick. Edge-triggered dropped definitions on 4+ recovery paths (fanout-resume, attach-duplicate, ralph clone-retry, retry-eval) with no self-heal. ## Approach: level-triggered + durable per-step marker Keep the full-projection restatement (self-healing preserved for every creator/recovery path, no `CreatedIDs` threading), but make emission idempotent via a durable per-step marker (`beadmeta.StepDefinedEmittedMetadataKey`). `EmitCurrent` emits `step_defined` for an **unmarked** step and marks it; **marked** steps are skipped. An unmarked step (created on any pass, including a failed one) re-emits on the next tick — self-healing intact — while marked steps stop the storm. ### Two correctness criticals fixed (both caught by adversarial review) 1. **Never mark after a possibly-dropped emit.** `Recorder.Record` is best-effort/void (drops on flock timeout / ENOSPC; `events.Discard` swallows everything). Marking after a dropped emit = permanent loss. Fixed with a new `events.AckRecorder{ RecordAck(Event) error }`: `FileRecorder.RecordAck` surfaces the durable-write outcome, and `EmitCurrent` marks a step **only** on a nil ack. A recorder that can't acknowledge durability (`events.Discard`, any bare `Recorder`) emits best-effort but is **never** marked, so it re-emits next tick. This mirrors the file's own `applyConvergenceStamp` discipline while avoiding a per-tick journal read on the hot path. 2. **Clones must not inherit the marker.** ralph clone-retry + retry-eval clone the step's metadata; `clearRetryEphemera` now strips `StepDefinedEmittedMetadataKey` so attempt-N+1 steps aren't born-marked. Audit confirmed fanout/attach/drain build step beads from recipe **templates** (never marked), so this is the complete fix. ## Tests RED-then-GREEN: `TestEmitCurrentDoesNotMarkOnDroppedOrDiscardEmit` (dropped/Discard emit leaves the step unmarked → re-emits next healthy tick) and `TestRetryClonedStepGetsItsOwnStepDefined`. Reviewed sound by a Fable council. ## Known minor follow-ups (non-blocking, noted for review) - `FileRecorder.RecordAck` returns nil after `Write` (page cache, no `fsync`) — a whole-machine crash before writeback can leave a marked-but-unlogged step; matches the journal's existing durability posture (`AppendBatch` never fsyncs), and `gc events reemit-execution` (ignores the marker, full restate) is the recovery. - Worth adding a direct `internal/events` test pinning `RecordAck` fidelity (closed→err, healthy→nil+readable) so a future relaxation can't silently reintroduce critical #1. - The `fake` events provider acks non-durable in-memory appends (explicit test double). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
quad341
pushed a commit
that referenced
this pull request
Sep 14, 2026
…ot just the order's own (gastownhall#4378) (gastownhall#4547) ## Summary The 1.4 webhook receiver works end-to-end (HMAC verify, rule match, order fires — live-verified against a real GitHub `pull_request` delivery), but the fired order then fails at formula dispatch: `formula "github-pr-review" not found in search paths` — even though `gc formula list`/`gc formula show` resolve that exact formula, same build, same city. "A PR opens → my factory reviews it" is the natural first demo of the headline 1.4 webhook feature; this breaks it for any user configuration where the order and its formula live in different layers. The issue bundles four claims; this PR scopes to just the confidently root-caused one: 1. **Resolver divergence** (this fix) — confirmed still live. 2. Rig-scoped orders have no user-writable home — real feature gap, not a same-day fix, left alone. 3. `gc order run` exits 0 on failure — did **not** reproduce against current main; `cmdOrderRun`'s formula path already returns 1 on `prepareOrderWispRecipe` error and is correctly wired through `RunE` to the `errExit` sentinel. Likely already fixed by other work since the issue was filed. 4. Webhook dispatch hits the identical divergence as #1 — covered by the same fix (both paths share the new helper). ## Fix Root cause traced through `internal/orderdiscovery/discovery.go` → `cmd/gc/cmd_order.go` / `order_dispatch.go`: an order's `FormulaLayer` field records where the **order file itself** was discovered (e.g. a city-authored order in `<city>/orders/` gets the city's own local formulas dir) — it exists for name-collision precedence when the same order name appears in multiple layers, not to scope which layers the order's **formula** may resolve from. Both order-dispatch call sites (`gc order run` in `cmd_order.go`, and the controller-driven path in `order_dispatch.go`'s `dispatchWisp`) searched only `a.FormulaLayer` for the formula — so an order authored in one layer referencing a formula shipped by a different layer (the exact "webhook → pack formula" shape) could never resolve, while `gc formula list`/`show` (which aggregate the full `cfg.FormulaLayers.City` + every rig's layers via the existing `formulaSearchPathsForList`) resolved it fine. Added `orderFormulaSearchPaths(cfg, a) []string` next to `formulaSearchPathsForList` in `cmd_formula.go`: the same full city+rig aggregation, with `a.FormulaLayer` appended last (highest priority) so a same-named formula co-located with the order itself still wins on a collision. Both call sites now share this one helper instead of each independently restricting to `a.FormulaLayer` alone. Fixes gastownhall#4378. ## Test plan New test `TestOrderRunResolvesFormulaFromAnyConfiguredLayerNotJustItsOwn` (`cmd/gc/pack_import_formula_order_test.go`), modeled on the existing `TestPackV2ImportedFormulasAndOrdersVisibleToCityAndRig`/`TestTransitiveGastownPackDigestOrderResolvesAndRuns` fixtures but specifically constructed so the order and its formula live in **different** layers (a city-local order file referencing a formula shipped by an imported pack) — the existing gastown-digest test doesn't reproduce the bug because in that fixture the order and its formula happen to be co-located in the same pack. RED-confirmed: `doOrderRun = 1`, stderr `formula "pack-formula" not found in search paths` — exact match to the reported error. GREEN after the fix. Full `TestOrder*`/`TestPack*`/`TestTransitiveGastown*` suites re-run clean, confirming the existing tests that construct a fake `cityPath` with `cfg == nil` (relying on `a.FormulaLayer` alone) still work — `orderFormulaSearchPaths` falls back to exactly that when `cfg` has no layers to aggregate. - [x] New test + full `TestOrder*` suite (60+ tests) + `TestPack*`/`TestTransitiveGastown*`: pass, no regressions - [x] `go build ./...` (full repo, untagged): clean - [x] Full untagged pre-commit hook (`lint-changed`, spec/client/schema codegen, `go vet ./...`) passed clean, no `--no-verify` - [x] Full sharded local test suite — pre-existing sandbox flakiness this run (recurring subprocess/timing/Docker/dolt-startup failures seen across today's other pushes) — no `TestOrder*`/`TestPack*`/`TestTransitiveGastown*` test (the code this change touches) appears in any failing shard 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01TGjSaP5EtcXZYqgBafw61m --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: rjgeng <rjgeng@users.noreply.github.com>
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.
Why
A transient post-flatten value-hash compact-quarantine marker silently disables ALL compaction/GC for a DB with no alert and no auto-staleness, letting the noms journal grow toward the corrupted-journal city-down threshold. beads_hq sat quarantined 16 days (journal 5.1G, data dir 13G) — the same path that ends in a city-wide Dolt outage. This is a recurring incident class (see gc-h7mc0tz).
What
Three improvements close the gap (commit b5116b0, examples/dolt source — the deployed
.gc/system/packs/doltis reconciler-managed so the fix must originate here):[HIGH]health advisory whenever an active compact-quarantine marker exists. Counts only valid-db-name markers (matching the compactor's ownhas_compact_markerlookup) so operator archives likebeads_hq.stale-cleared-*do not false-alarm.GC_DOLT_COMPACT_QUARANTINE_STALE_SECS(default 6h) only once the DB reads clean (row counts) and is quiescent (whole-DB value hash stable across two probes a settle apart), then retry. The post-flatten re-verification re-quarantines on real drift — so this is a supervised retry that never bypasses integrity enforcement.Tests
7 new hermetic tests + full
examples/doltsuite green (2 pre-existing env-specific failures unrelated).Open for reviewer
vbtcl/gascity:main(the branch base). Retarget to upstreamgastownhall/gascityif that's where the release is cut.Filed by gastown.mayor on behalf of claude-1 (gc-wisp-ncb1). Refs gc-h7mc0tz, gc-sffnhkx.