Skip to content

fix(refinery): include gc.routed_to=polecat in prompt Rejection Flow#11

Open
eric-jones wants to merge 1 commit into
mainfrom
topic/refinery-rejection-routing
Open

fix(refinery): include gc.routed_to=polecat in prompt Rejection Flow#11
eric-jones wants to merge 1 commit into
mainfrom
topic/refinery-rejection-routing

Conversation

@eric-jones
Copy link
Copy Markdown
Owner

Fork-PR for review. Upstream: gastownhall#1934 (OPEN). julianknutsen asked for manual verification; gc-rm0ha.49 schedules L5c rerun.

@eric-jones eric-jones force-pushed the topic/refinery-rejection-routing branch from 1752368 to e5edc5b Compare May 20, 2026 16:07
@eric-jones eric-jones force-pushed the topic/refinery-rejection-routing branch from e5edc5b to 7282cec Compare May 20, 2026 16:30
julianknutsen added a commit that referenced this pull request May 21, 2026
…r 3 (gastownhall#1594)

## Summary

Make the equivalence between the worker's claim path
(`EffectiveWorkQuery` Tier 3) and the reconciler's spawn path
(`EffectiveScaleCheck` / new `EffectivePoolDemandQuery`) **structural**
rather than coincidental.

PR gastownhall#1516 symmetrically simplified both predicates to "ready unassigned
routed_to" by retiring the molecule tier from each side. Post-gastownhall#1516 the
two are textually equivalent — but they remained two separate code
paths. A future PR adding a tier to one without the other would silently
re-introduce the protocol-mismatch class addressed by fo-spawn-storm
(the reconciler spawning sessions for work the worker can't claim, or
vice versa).

This refactor closes the residual gap by extracting the shared predicate
into a single helper:

```go
func bdReadyPoolDemandShell(target string) string {
    return `bd ready --metadata-field gc.routed_to=` + target + ` --unassigned --json`
}
```

Both `EffectiveWorkQuery` Tier 3 (with `--limit=1` appended, first-row
form) and the new `EffectivePoolDemandQuery` (piped through `jq
'length'`, count form) derive from this helper. The reconciler's
`scaleParamsFor` now calls `EffectivePoolDemandQuery` directly so the
dependency on the work-query predicate is explicit at the call site.
`EffectiveScaleCheck` remains as a thin pass-through for back-compat
with code and configs that name the predicate "scale_check".

## Tests

- **Structural** — `TestPoolDemandPredicateSharedWithWorkQuery` asserts
both methods reference the `bdReadyPoolDemandShell` output for the same
target. Future divergence becomes a compile-touched test failure rather
than a quiet behavior regression.
- **Behavioral** — `TestPoolDemandAndWorkQueryAgreeOnRoutedSemantics`
runs both queries against the same fake `bd` and asserts they agree on
\"routed work present\" / \"no routed work\". This is the behavioral
counterpart called out in fo-glz1h: a state the worker can't claim must
produce demand=0 from the reconciler.
- **Override pass-through** —
`TestEffectivePoolDemandQueryRespectsOverride` confirms a user-set
`scale_check` short-circuits both methods.

## Docs

`engdocs/architecture/dispatch.md` gets:
- A new \"scale_check ↔ work_query correspondence\" section explaining
the two read sides and why they must stay symmetric, with the
spawn-storm pre-gastownhall#1516 symptom called out as the failure mode.
- A new invariant (#11) that any tier-shape change to one MUST flow
through `bdReadyPoolDemandShell` to the other, with the regression tests
named.
- The Code Map row for `internal/config/config.go` gains the new
symbols.
- \"Last verified against code\" date bumped to 2026-05-01.

## Stack

Based on `fix/main-auto-port-bind-retry-20260501` (PR gastownhall#1582) so the
unrelated tempfile flake on `main` doesn't keep failing CI for this
branch.

## Test plan

- [x] `go test ./internal/config/...` passes (existing + 3 new tests).
- [x] `go vet ./...` clean.
- [x] Targeted pool tests (`TestEvaluatePool*`, `TestScaleParams*`,
`TestPool*`) pass.
- [ ] Full CI run on the PR (broader cmd/gc suite has pre-existing
failures on `fix/main-auto-port-bind-retry-20260501` —
TestCmdHookPoolInstanceUsesTemplatePoolLabel,
TestDoPrimePoolAgentFallback,
TestControllerStateCreateRigPokesReconciler,
TestReconcileCitiesNameDriftStopsBeadsProvider — verified to fail on
this base before any of these changes; not introduced by this PR).

## Related

- Closes fo-glz1h (surviving \"Keep\" item from the now-closed fo-78wj3,
plus the dispatch.md updates flagged by gc-wisp-acvu).
- Follows up on PR gastownhall#1516 (commit 6b5d912) — the symmetric
simplification this builds on.
- Closes the residual structural gap behind fo-spawn-storm (DEFERRED).

<!-- codesmith:footer -->
---
<a
href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1594"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img
alt="View in Codesmith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a>
<sup>Need help on this PR? Tag <code>@codesmith</code> with what you
need.</sup>

- [ ] Let Codesmith autofix CI failures and bot reviews
<!-- /codesmith:footer -->

---------

Co-authored-by: Julian Knutsen <julianknutsen@users.noreply.github.com>
@eric-jones eric-jones force-pushed the topic/refinery-rejection-routing branch 2 times, most recently from 0d1d80c to b39e555 Compare May 22, 2026 08:18
The refinery prompt's "Rejection Flow" quick reference showed a partial
`gc bd update $WORK --status=open --assignee="" --set-metadata
rejection_reason="..."` that omitted `--set-metadata gc.routed_to=...polecat`.
LLM agents ran exactly that partial form (verbatim from the prompt) instead
of the formula's full update. Result: rejected work beads sat in the queue
with stale `gc.routed_to=...refinery` (set when the polecat originally
submitted to refinery), and the pool reconciler — which matches on
`gc.routed_to`, not `rejection_reason` — never spawned a polecat to resume.
The bead stayed open forever; the convoy stalled.

Reproduced on validate-l5-20260510-v4 (gascity local/integration 4d656c14
with gastownhall#1925, gastownhall#1916, gastownhall#1913 in flight): two L5c work beads with conflicting
edits. Refinery merged the first as a fast-forward and rejected the
second correctly, but the rejected bead still had
`gc.routed_to=...refinery` after the rejection update — verified directly
from the rig's `.beads/issues.jsonl` and the refinery agent's transcript.
The agent's exact rejection command:

  gc bd update vl2vr-j4y --status=open --assignee="" \
    --set-metadata rejection_reason="rebase conflict on shared_func.go: ..."

— matches the prompt's incomplete flow, not the formula's full one.

## What changed

- `examples/gastown/packs/gastown/agents/refinery/prompt.template.md` —
  rewrote "Rejection Flow" to show the full bd update with
  `--set-metadata gc.routed_to="${GC_RIG:+$GC_RIG/}{{ .BindingPrefix }}polecat"`,
  plus a one-paragraph "why" naming the pool reconciler match. Pointed
  readers at the formula's `rebase` / `handle-failures` step as canonical.

- `examples/gastown/gastown_test.go` —
  `TestRefineryPromptRejectionFlowIncludesRoutedTo` asserts the prompt's
  Rejection Flow contains both `--set-metadata rejection_reason=` and the
  full `gc.routed_to=...polecat` line, in order. Guards against silently
  reverting to a partial bd update.

Surface: refinery rejection metadata via `gc bd update --set-metadata`
(canonical bd CLI), shown in canonical agent prompt template (rendered
by `gc init`); not direct Dolt edits, not inline ad-hoc metadata queries.

Fixes: stg-66wz (Issue 2: spawn-on-rejection)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Upstream-PR: gastownhall#1934
@eric-jones eric-jones force-pushed the topic/refinery-rejection-routing branch from b39e555 to 9a239af Compare May 23, 2026 12:59
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