Skip to content

perf(test): compile ./cmd/gc test binary once instead of once per shard - #2

Merged
atbrace merged 1 commit into
mainfrom
fix/gcy-cmf-compile-cmdgc-once
Aug 1, 2026
Merged

perf(test): compile ./cmd/gc test binary once instead of once per shard#2
atbrace merged 1 commit into
mainfrom
fix/gcy-cmf-compile-cmdgc-once

Conversation

@atbrace

@atbrace atbrace commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Refs gcy-cmf. Complementary to #1 — that PR bounds the outer fan-out; this one cuts what each job costs.

The defect

Every cmd/gc shard ran go test ./cmd/gc -list '^Test' to discover which tests it owned, and -list must build the test binary before it can print a single name. With CMD_GC_PROCESS_TOTAL=6 that is six shards concurrently compiling the same 503,559-line package purely to read out test names — measured at 1.2–1.7 GB each, ~7.7 GB of 8 GB physical on a 6-core host, with 392k pageins/sec and 93 MB free.

The shards' own -timeout cannot bound it: -timeout bounds test execution, not compilation, so a livelocked compile runs straight past a timeout that can never fire.

The change

test-local-parallel compiles the binary once, up front, and exports it as GO_TEST_PREBUILT_BIN; test-go-test-shard enumerates with -test.list and runs with -test.run against that binary. This mirrors the pattern add_fsys_compile_job already uses for ./internal/fsys. Six compiles collapse to one and the shards become nearly free.

test-local-parallel now also sources scripts/lib/common.sh and calls configure_cgo_platform_paths before the prebuild. It previously never did — only test-go-test-shard did — so the prebuild would otherwise link with no icu4c include path and fail on macOS with undefined _uregex_* symbols. Verified directly: a bare go test -c ./internal/testenv fails with 'unicode/regex.h' file not found, while the same build with CGO_CPPFLAGS=-I$(brew --prefix icu4c)/include succeeds.

The prebuilt path declines, falling through to the original go test behaviour unchanged, whenever it would be incorrect:

  • GO_TEST_COVERPROFILE set — -coverpkg is a compile-time flag, so a binary built without it cannot produce a coverage profile
  • GO_TEST_TIMING_FILE set — scripts/go-test-observable shells out to go test -json specifically and cannot wrap a prebuilt binary
  • GO_TEST_TAGS set — -tags is compile-time, so a binary built without the caller's tags would silently run the wrong set of tests
  • the binary is missing or not executable

The prebuilt binary runs with the package directory as its working directory, matching go test, so tests reading testdata/ or other relative paths are unaffected.

Verification

Verified on small packages only — deliberately not on ./cmd/gc, which is the build this change exists to avoid and which is under an operational hold on the affected host:

  • ./internal/deps — prebuilt and original paths select and partition an identical test set across shards 1of2 and 2of2
  • ./internal/testenv (reads testdata/*.golden) — passes via the prebuilt binary, confirming working-directory handling
  • decline paths confirmed for GO_TEST_TAGS and a non-executable binary
  • GO_TEST_PREBUILT_BIN confirmed to survive the runner's env -i allowlist, without which the change would silently no-op

Not verified: end-to-end on ./cmd/gc, and the actual memory reduction. No magnitude is claimed beyond the per-shard figures measured above.

Every cmd/gc shard ran `go test ./cmd/gc -list '^Test'` to discover which
tests it owned, and `-list` must BUILD the test binary before it can print a
single name. With CMD_GC_PROCESS_TOTAL=6 that meant six shards compiling the
same 503,559-line package concurrently purely to read out test names --
measured at 1.2-1.7 GB each, ~7.7 GB of 8 GB physical on a 6-core host, with
392k pageins/sec and 93 MB free. The shards' own -timeout could not bound it:
-timeout bounds test EXECUTION, not COMPILATION, so a livelocked compile runs
straight past a timeout that can never fire.

test-local-parallel now compiles the binary once, up front, and exports it as
GO_TEST_PREBUILT_BIN; test-go-test-shard enumerates with -test.list and runs
with -test.run against that binary. This mirrors the pattern
add_fsys_compile_job already uses for ./internal/fsys. Six compiles collapse
to one and the shards become nearly free.

test-local-parallel now also sources scripts/lib/common.sh and calls
configure_cgo_platform_paths before the prebuild. It previously never did --
only test-go-test-shard did -- so the prebuild would otherwise link with no
icu4c include path and fail on macOS with undefined _uregex_* symbols.
Verified directly: a bare `go test -c ./internal/testenv` fails with
"'unicode/regex.h' file not found", while the same build with
CGO_CPPFLAGS=-I$(brew --prefix icu4c)/include succeeds.

The prebuilt path is declined, falling through to the original `go test`
behaviour unchanged, whenever it would be incorrect:
  - GO_TEST_COVERPROFILE set: -coverpkg is a COMPILE-time flag, so a binary
    built without it cannot produce a coverage profile;
  - GO_TEST_TIMING_FILE set: scripts/go-test-observable shells out to
    `go test -json` specifically and cannot wrap a prebuilt binary;
  - GO_TEST_TAGS set: -tags is COMPILE-time, so a binary built without the
    caller's tags would silently run the wrong set of tests;
  - the binary is missing or not executable.

The prebuilt binary is run with the package directory as its working
directory, matching what `go test` does, so tests reading testdata/ or other
relative paths are unaffected.

Verified on small packages only -- deliberately NOT on ./cmd/gc, which is the
build this change exists to avoid and which is currently under an operational
hold on this host:
  - ./internal/deps: prebuilt and original paths select and partition an
    identical test set across shards 1of2 and 2of2;
  - ./internal/testenv (reads testdata/*.golden): passes via the prebuilt
    binary, confirming the working-directory handling;
  - decline paths confirmed for GO_TEST_TAGS and a non-executable binary;
  - GO_TEST_PREBUILT_BIN confirmed to survive the runner's `env -i` allowlist,
    without which the change would silently no-op.

Refs: gcy-cmf
@atbrace

atbrace commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

The magnitude this PR deliberately declined to claim now has a measurement behind it — partially. Recording what it does and does not support.

sys-3hem0 (closed validation:proven) measured peak RSS of a ./cmd/gc compile via /usr/bin/time -v on linux/amd64 in a throwaway container:

  • one 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 per job

Per-job peak RSS is flat as concurrency rises. Concurrent compiles do not share compile memory. That is the result that matters for this PR: collapsing six compiles to one removes ~5× the peak memory, not some smaller shared-cost fraction. Extrapolated to the 8 GiB host this gate runs on, six concurrent compiles project to 21–23 GiB — 2.6–2.9× physical RAM. The gate cannot fit and never could.

It also forecloses the cheaper alternative that would have made this PR unnecessary: a warm shared GOCACHE is worth ~3.4× on wall time but only ~3.6% on memory, and even the null case (identical ref, zero source change) still costs 2.73 GiB. There is no cache-topology fix. Reducing the number of concurrent compiles — what this PR does — is the mechanism.

What it still does not support, so the PR description stays as written: the measurement is the linux variant of cmd/gc and it measured compile cost only (go test -c, nothing executed). This PR remains verified on small packages only, never on ./cmd/gc, and I am still not claiming a measured end-to-end reduction for this change on this host — only that the cost being avoided is now quantified and does not shrink under concurrency.

@atbrace
atbrace merged commit 064bd65 into main Aug 1, 2026
69 checks passed
@atbrace
atbrace deleted the fix/gcy-cmf-compile-cmdgc-once branch August 1, 2026 17:16
quad341 pushed a commit that referenced this pull request Aug 13, 2026
… (ga-fq54n) (gastownhall#5162)

## The bug (ga-fq54n)

win-mc-forge's measurement row #2, the one that started this program,
verified
still open on `f08858b8a4`. On a converged split city:

```
gc bd list --metadata-field gc.root_bead_id=<gcg root>   →   0 rows, exit 0
```

Every piece worked as designed. The dialect guard was scoped to
`sql`/`query`;
`--metadata-field` is not an id-VALUED flag, so `cmd_bd_by_id.go`'s
by-id door
correctly declined it (a QUOTED id decides nothing about ownership —
`cmd_bd_by_id.go:78-81`, `:830-835`); and bd then ran the projection
successfully against the one ledger that holds no `gcg-` row. The value
named
an id, but the **verb is a PROJECTION** over a class this ledger cannot
see.

**Invariant 0: a projection that cannot see a class MUST fail loudly.
Returning `[]` is forbidden.**

The asymmetry is what made it urgent: `gc bd dep tree <gcg id>` already
**refuses with exit 1** (`cmd_bd_by_id.go:580-586`, `:630-637`) while
`list`
answered `[]` with exit 0. Two projections over the same molecule,
through the
same command, with opposite failure semantics — worse than either alone,
because an operator who learned the loud one trusts the quiet one.

## The shape chosen: refuse loudly, and name the federated reader

**(b) refuse**, not (a) federate. `gc bd list` is a bd passthrough with
~40
value flags and ~23 bool flags (`internal/bdflags`), plus output shapes
(`--long`, `--tree`, `--pretty`, the human table) that cannot be
reproduced in
process. Federating the representable subset would still leave every
other
spelling silent — so it would need this refusal arm **anyway**, on top
of a
partial in-process imitation of bd, which is precisely what
`cmd_bd_by_id.go`'s own header refuses to build ("the caller leaves it
on its
existing path rather than serving a partial imitation of bd"). It would
also
make split-city `gc bd list` bytes diverge from single-store bytes for
the same
query, which `gc ready` never had to do because it was a new command
with its
own contract.

So (a)'s *user outcome* is delivered by steering to the surface that
already
has the leg composition, the fail-loud rule and CLI==API conformance:
the
refusal names `gc ready --metadata-field "key=value"`, which federates
city → rigs ascending → graph last, first-leg-wins, and fails loud on a
dead or
unopenable leg (`cmd/gc/ready_federation.go`, gastownhall#5158) — **and the message
states
exactly how far that gets**, because it answers a narrower question than
the
read it replaces (see "The steering is honest about its limit" below).
`gc beads list` is deliberately **not** offered: its API lane federates,
but
its no-controller fallback opens only the city and rig stores
(`openAllConvoyStoresAt`), so it would hand back the same blind answer
this
message is refusing — and an operator hits this at 2am often *because*
the
controller is down.

`list`, `ready` and `search` join the existing dialect guard rather than
growing a new mechanism. The argv scan now also reads a flag's
**inline**
value, so `--metadata-field=k=gcg-1` cannot switch the guard off with a
single `=`. The
pre-existing `GC_BD_ALLOW_RELOCATED_CLASS_READ` override still covers
the real
false positive (the work ledger legitimately carries `gcg-` strings in
metadata — `ensureDrainUnitConvoy` stamps `gc.drain_control_id`), and
honoring
it stays loud.

## The two projections now agree

| invocation | before | after |
| --- | --- | --- |
| `gc bd dep tree gcg-abc123` | exit 1, refused | exit 1, refused
(unchanged) |
| `gc bd list --metadata-field gc.root_bead_id=gcg-abc123 --json` |
**`[]`, exit 0** | exit 1, refused |

Both exit non-zero, both name the id namespace that cannot be seen
**and** the
binding it is served from, and neither reaches the ledger that cannot
answer.
`I14-projection-coherence` asserts that correspondence on **both**
topologies
from the two production predicates that decide it
(`bdSQLRelocatedClassRefusal` for `list`, `bdArgsNameClassOwnedBead` for
everything else), and `TestGcBdProjectionsAgreeOnAClassTheyCannotSee`
drives it
end to end through the real `doBd` against a bd stub that answers `[]`
and
exits 0.

The messages are deliberately not compared verbatim: `dep tree` is
refused by
the by-id door, which knows the exact bead and reports OWNERSHIP of it,
while
`list` is refused by the dialect guard, which knows only that the query
names
the namespace. Pinning identical wording would force one arm to say
something
it does not know.

## Single-store cities are byte-identical, proven by mutation

`relocatedBeadClasses` returns nothing on a city with no `[storage]`
split, the
scan is never entered, and the same argv reaches bd verbatim
(`TestGcBdListIsUnchangedOnASingleStoreCity` asserts bd received the
exact
projection and that stdout is bd's own answer untouched). I14's
single-store
row is not hardcoded: the fixture mints work-prefixed ids there, so the
same
two argvs carry no reserved prefix and both projections pass through.

Mutations run, all of which turn the new rows red:

* removing the `"list"` entry from `bdRelocatedClassGuardedVerbs` → 5
table
  rows + 3 end-to-end tests + I14's split row fail
* reverting the inline-value scan → `list on a graph root id inline`
fails
* removing `"ready"`/`"search"` → 3 table rows, 3 end-to-end rows,
`TestBdRelocatedClassGuardCoversEverySelectorVerb` and I14's split row
fail
* reverting `atSelectorValueStart` to `atQueryValueStart` → 8 internal
rows,
  15 table rows and 8 end-to-end rows fail

## Stale docs in the same files (ga-kqk8j)

1. `cmd/gc/bd_relocated_classes.go` still claimed `show`/`dep tree` were
raw
passthroughs with **no class routing**. Falsified by gastownhall#5132
(`18b1743e07`),
wired at `cmd_bd.go:260`. Rewritten to state what each verb actually
does:
`show`, `update` (incl. `--claim`), `release-if-current` and `dep list`
are
class-routed in process; `dep tree` is refused on a class-owned id;
every
   other subcommand that ADDRESSES a reserved-prefix id is refused by
   ownership rather than servability.
2. `internal/beads/membership.go` attributed spec-sidecar dependency
isolation
to `formula.newSourceSpecStep` "clearing"
`DependsOn`/`Needs`/`WaitsFor`.
   Verified against the tree: that function
(`internal/formula/source_spec.go:22-33`) is a fresh `&Step{...}`
literal
that never SETS them; `namespaceSourceSpecStep:52-54` is the one place
that
actively nils them. The conclusion holds; the citation sent the next
reader
to the wrong function, and it is load-bearing for why the measured 48
looked
   stable. Two verbatim copies of the same wrong citation
(`internal/beads/membership_test.go`,
`internal/dispatch/fanout_membership_test.go`)
move with it, and `namespaceSourceSpecStep` — which had no doc — now
says
   why a spec carries no edges.

## Conformance gap accounting

No KNOWN GAP became closable. I12's gap is the **object-model** half of
Invariant 0 (`beads.DirectMembers` handed the wrong store still answers
with an
empty member set); closing it means giving a store the ability to refuse
a
class it does not hold, which is a behavior change on every
`DirectMembers`
consumer, not a read-path guard. I12's paragraph now says so explicitly,
so a
reader does not mistake I14 for having closed it. No new gap was
introduced, so
no new skip.

## Gates

* `go build ./...` — clean
* `go vet ./...` — clean
* `gofmt -l ./cmd ./internal` — empty
* `golangci-lint` 2.10.1 over `./cmd/gc/... ./internal/beads/...
./internal/formula/... ./internal/dispatch/...` — **0 issues**
* `go test ./cmd/gc -timeout 25m` (serial) — **ok, 756.3s**
* `go test ./internal/...` — **exit 0, 144 packages**
* `scripts/check-core-boundary.sh` — OK
* `make check-split-topology-rows` — exit 0

---

# Council review round 2 — three defects, fixed (`d05afb11dd`)

Rebased onto `31ecad2364` (gastownhall#5163). Seven confirmed majors reduced to
three
distinct defects; all three plus the three minors are fixed.

## Defect 1 — the refusal was far too broad

`RelocatedClassesInListSelector` reused `atQueryValueStart`, which
returns
`true` at offset 0. For `bd query` one token is a whole expression, so a
bare
term IS a value position. For a **selector flag** the flag has already
consumed
its own token, and `bd list` accepts **no positional arguments at all**
(`cmd/bd/list.go:494`: *"bd list does not accept positional arguments;
use
flags instead"*), so every token the scan sees is some flag's VALUE.
Offset-0
anchoring therefore made **every free-text selector id-shaped**, and
`--title-contains gcg-abc123` became indistinguishable from
`--metadata-field gc.root_bead_id=gcg-abc123`.

That is the exact false positive the guard's own header
(`internal/beads/bdsql_relocation.go`) names FIRST as the thing the
anchoring
rules exist to let through — a LIKE-contains over a text column is a
question
about the rows THIS ledger holds, and the work ledger really does carry
`gcg-`
strings.

**Anchor chosen: `atSelectorValueStart` — the id must be preceded by the
`=` of
a `key=value` predicate** (whitespace and quotes skipped on the way
back, so
`k="gcg-1"` and `k= gcg-1` still anchor). Dropping the offset-0 arm is
the
whole fix.

**Why this over gating on `bdflags.ValueFlags("list")`:**

1. `bdflags` records **arity, not meaning**. A flag allowlist needs a
*second*
manifest of what each flag MEANS, maintained by hand against every flag
bd
grows — and its failure mode is *silent under-guarding*, which is the
bug
   class this PR exists to close.
2. It needs one list **per verb**. `list`, `ready` and `search` have
different
selector sets; the `=` anchor is correct for all three with no list at
all.
3. Every other dialect in this family classifies **text**. The `=`
anchor
derives the answer from the token's own shape the way
`atSQLLiteralStart`
   and `atQueryValueStart` do, so the three dialects stay one mechanism.
4. It is strictly **narrower** where it matters. `--id`/`--parent`/`-p`
fall
through to the by-id ownership door — which was already refusing them on
`main` — and that door names the **bead and its binding**, not just the
namespace. The offset-0 arm was shadowing a more specific refusal with a
   vaguer one, for zero added coverage.
   `TestGcBdListOnAnIDValuedFlagRefusesByOwnership` pins that.

## Defect 2 — the same bug was still live one verb over

`gc bd ready --metadata-field <k>=<gcg id>` and `gc bd search` answered
`[]`
exit 0, and the shipped comment justified the exemption with *"`ready`
stays
out: its selectors name templates and labels, not bead ids"* — which
`internal/bdflags/bdflags.go` falsifies in the same tree.

**Both verbs are now guarded.** `--metadata-field` is registered on
exactly
three bd subcommands (`cmd/bd/list.go:831`, `ready.go:820`,
`search.go:390`),
and all three are in `bdRelocatedClassGuardedVerbs`.
`TestBdRelocatedClassGuardCoversEverySelectorVerb` derives that
requirement
from `bdflags` so a fourth cannot appear unguarded. The false rationale
is
gone; the replacement states the checkable completeness claim.

Verified safe for the work loop: the pool-demand probe and the control
dispatcher select on `gc.routed_to` / `gc.run_target`, whose values are
pool
template names, and they invoke raw `bd`, not `gc bd`.
`TestGcBdReadyKeepsAnsweringItsOrdinaryWorkQueries` pins the argv-level
half.

## Defect 3 — the steering is honest about its limit

`gc ready --metadata-field` answers a **narrower** question than the
refused
read. With no `--status` it issues a `ReadyQuery` — claimable, unblocked
work
only — so a molecule whose steps are in flight, blocked or closed comes
back
`[]` **from the command the refusal recommended**. And that is the modal
case:
a stuck molecule is precisely one with no claimable step. `--status`
takes
exactly one of `open`/`in_progress`/`blocked`/`closed`; there is no
`--all`, no
comma list, and `deferred` is not selectable at all.

The message now says all of that and states plainly that **there is no
federated equivalent of `bd list --all` yet**:

> For a SET of beads rather than one id, `gc ready --metadata-field
"key=value"`
> federates the city store, the rig stores and the relocated binding as
ordered
> legs and fails loud on a leg it cannot read — but it answers a
NARROWER
> question than this read did: with no --status it returns only
claimable work,
> and --status takes exactly one of open, in_progress, blocked, closed
(no
> --all, no comma list, and deferred is not selectable), so enumerating
a
> molecule's full membership takes one invocation per status and cannot
reach a
> deferred member at all. There is no federated equivalent of `bd list
--all`
> yet.

I14's last leg previously ran against a molecule whose only step was
open — the
one status for which the escape works. It now mints a **mid-flight**
step and
asserts both halves: the bare spelling does NOT return it, and
`--status in_progress` does. The steering cannot silently become wrong
again.

## The three minors

1. **The refusal now names `GC_BD_ALLOW_RELOCATED_CLASS_READ`.**
Appended at
   the CLI seam (`bdRelocatedClassEscapeHint`), *not* inside
`beads.RelocatedClassRefusal` — that same string is returned by
`BdStore`'s
id-scoped guard, which honors no env var, so naming it there would
advertise
   an escape that does not work on half the paths that print it.
`TestRelocatedClassRefusalLeavesTheOverrideToTheCLI` pins the negative;
   `TestGcBdRefusalNamesTheOverride` pins the positive.
2. **`cmd_bd_by_id.go`'s header** said `gc bd list --metadata-field
workflow_id=gcg-…` "is a work question about work rows, and refusing it
exec-fails the consumer that asks it" — the exact argv this PR refuses.
Both
sites now state what is true: the by-id door **declines it on
OWNERSHIP**
(a quoted id decides nothing about which store serves the read), and the
   dialect guard refuses it one pre-flight earlier on **SERVABILITY**.
3. `RelocatedClassesInListSelector` → `RelocatedClassesInSelector`,
since it
now serves three verbs. A name that says "list" while `ready` uses it is
the
   same class of false citation ga-kqk8j exists to remove.

## Per-flag A/B: `main` (31ecad2) → old head (17d7d83) → now
(d05afb1)

Predicates measured directly (`bdSQLRelocatedClassRefusal ‖
bdArgsNameClassOwnedBead`) on `splitCityConfig()`.

| argv | main | old head | now |
| --- | --- | --- | --- |
| `list --title-contains gcg-abc123` | pass | **REFUSE** | pass |
| `list --title-contains=gcg-abc123` | pass | **REFUSE** | pass |
| `list --desc-contains gcg-abc123` | pass | **REFUSE** | pass |
| `list --notes-contains gcg-abc123` | pass | **REFUSE** | pass |
| `list --title gcg-abc123` | pass | **REFUSE** | pass |
| `list --label gcg-abc123` | pass | **REFUSE** | pass |
| `list --exclude-label gcg-abc123` | pass | **REFUSE** | pass |
| `list --label-pattern 'gcg-*'` | pass | **REFUSE** | pass |
| `list --label-regex 'gcg-.*'` | pass | **REFUSE** | pass |
| `list --assignee gcg-worker` | pass | **REFUSE** | pass |
| `list -a gcg-worker` | pass | **REFUSE** | pass |
| `list --sort gcg` | pass | **REFUSE** | pass |
| `list --spec gcg-abc123` | pass | **REFUSE** | pass |
| `list --title-contains gcg` | pass | **REFUSE** | pass |
| `list --title-contains "fix (gcg-1) regression"` | pass | **REFUSE** |
pass |
| `list --title-contains "regressions: gcg-1, gcg-2"` | pass |
**REFUSE** | pass |
| `list --title-contains "root is 'gcg-1' here"` | pass | **REFUSE** |
pass |
| `list --title-contains "fix gcg-1 regression"` | pass | pass | pass |
| `list --metadata-field gc.root_bead_id=gcg-abc123` | pass | refuse |
**refuse** |
| `list --metadata-field=gc.root_bead_id=gcg-abc123` | pass | refuse |
**refuse** |
| `list --json --metadata-field gc.root_bead_id=gcg-abc123` | pass |
refuse | **refuse** |
| `--json list --metadata-field gc.root_bead_id=gcg-abc123` | pass |
refuse | **refuse** |
| `list --metadata-field gc.nudge_id=gcn-1` | pass | refuse | **refuse**
|
| `list --metadata-field 'gc.root_bead_id="gcg-abc123"'` | pass | refuse
| **refuse** |
| `list --metadata-field gc.root_bead_id=GCG-ABC123` | pass | refuse |
**refuse** |
| `list --metadata-field gc.root_bead_id=demo-abc` | pass | pass | pass
|
| `list --metadata-field gc.root_bead_id=gcgx-1` | pass | pass | pass |
| `list --has-metadata-key gc.root_bead_id` | pass | pass | pass |
| `list --status open` | pass | pass | pass |
| `list --id gcg-abc123` | refuse (by-id) | refuse | refuse (by-id) |
| `list --parent gcg-abc123` | refuse (by-id) | refuse | refuse (by-id)
|
| `list -p gcg-abc123` | refuse (by-id) | refuse | refuse (by-id) |
| `ready --metadata-field gc.root_bead_id=gcg-abc123` | **pass** |
**pass** | **refuse** |
| `ready --metadata-field=gc.root_bead_id=gcg-abc123` | **pass** |
**pass** | **refuse** |
| `ready --label gcg-abc123` | pass | pass | pass |
| `ready --metadata-field gc.routed_to=demo/worker` | pass | pass | pass
|
| `ready --parent gcg-abc123` | refuse (by-id) | refuse (by-id) | refuse
(by-id) |
| `search --metadata-field gc.root_bead_id=gcg-abc123` | **pass** |
**pass** | **refuse** |
| `search gcg-abc123` | refuse (by-id) | refuse (by-id) | refuse (by-id)
|
| `sql "... id = 'gcg-abc'"` | refuse | refuse | refuse |
| `sql "... title like '%gcg-abc123%'"` | pass | pass | pass |
| `sql --format=gcg-abc123 'select 1'` | refuse (by-id) | refuse |
refuse (by-id) |
| `query id=gcg-abc123` | refuse | refuse | refuse |
| `query title~gcg-abc123` | pass | pass | pass |

Every regressed row is back to its `main` behaviour; the original bug
still
refuses in every spelling; `ready`/`search --metadata-field` are the
only
intentionally-new refusals.

## Red-before strings

Reverting `atSelectorValueStart` → `atQueryValueStart`:

```
--- FAIL: TestRelocatedClassesInSelectorMatchesOnlyThePredicateValue/bare_id_as_a_whole_selector_value
    RelocatedClassesInSelector("gcg-abc123") matched = true, want false
--- FAIL: TestRelocatedClassesInSelectorMatchesOnlyThePredicateValue/prose_with_a_paren_before_the_id
    RelocatedClassesInSelector("fix (gcg-1) regression") matched = true, want false
--- FAIL: TestRelocatedClassesInSelectorMatchesOnlyThePredicateValue/prose_with_a_quote_before_the_id
    RelocatedClassesInSelector("root is 'gcg-1' here") matched = true, want false
--- FAIL: TestRelocatedClassesInSelectorDivergesFromTheQueryDialect
    RelocatedClassesInSelector("gcg-abc123") matched [{graph gcg ...}]; a whole-token
    selector value is a search string, and refusing it breaks `--title-contains`
--- FAIL: TestGcBdListForwardsAFreeTextSearchThatNamesAGraphID/title_search
    `gc bd list --title-contains gcg-abc123 --json` exited 1 on a split city; a
    LIKE-contains over this ledger's own columns is a question bd answers, and
    refusing it withholds rows that exist.
```

(plus 15 rows of `TestBdSQLRelocatedClassRefusalOnASplitCity` and 7 more
subtests of `TestGcBdListForwardsAFreeTextSearchThatNamesAGraphID`.)

Removing `"ready"`/`"search"` from the guard map:

```
--- FAIL: TestBdSQLRelocatedClassRefusalOnASplitCity/ready_on_a_graph_root_id
    bdSQLRelocatedClassRefusal([ready --metadata-field gc.root_bead_id=gcg-abc123]) refused = false, want true
--- FAIL: TestGcBdReadyRefusesAGraphClassProjectionOnASplitCity/ready
    `gc bd ready --metadata-field gc.root_bead_id=gcg-abc123 --json` exited 0 with
    stdout="[]\n"; that empty array is the same silent-empty `gc bd list` refuses
    on the same molecule
--- FAIL: TestBdRelocatedClassGuardCoversEverySelectorVerb
    bd "ready" takes --metadata-field but is not in bdRelocatedClassGuardedVerbs
--- FAIL: TestSplitTopologyConformance/I14-projection-coherence/split
    `gc bd ready --metadata-field gc.root_bead_id=gcg-1` refused = false but `gc bd
    list` with the same selector refused = true
```

Reverting the steering + the override hint:

```
--- FAIL: TestRelocatedClassRefusalStatesTheLimitsOfTheSetEscape
    the steering does not state "with no --status it returns only claimable work"
    the steering does not state "exactly one of open, in_progress, blocked, closed"
    the steering does not state "deferred is not selectable"
    the steering does not state "no federated equivalent of `bd list --all`"
--- FAIL: TestGcBdRefusalNamesTheOverride
    the refusal never names GC_BD_ALLOW_RELOCATED_CLASS_READ, so the operator
    holding a false positive has no in-band way out
--- FAIL: TestSplitTopologyConformance/I14-projection-coherence/split
    the list refusal does not name "with no --status it returns only claimable work"
```

## Gates (on `d05afb11dd`, rebased on `31ecad2364`)

* `go build ./...` — clean
* `go vet ./...` — clean
* `gofmt -l ./cmd ./internal` — empty
* `golangci-lint` 2.10.1 over `./...` — **0 issues**
* `go test ./cmd/gc -timeout 25m` (serial, on the committed tree) —
**ok, 736.3s**
* `go test ./internal/...` — **exit 0, 144 packages**
* `scripts/check-core-boundary.sh` — OK
* `scripts/check-split-topology-rows.sh` — exit 0
* `scripts/check-routed-test-rows.sh` — exit 0

`cmd/gc/cmd_bd_by_id.go` is touched for **comments only** (the two doc
sites
that contradicted shipped behaviour); no by-id routing logic changed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
sjarmak pushed a commit that referenced this pull request Aug 26, 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant