Skip to content

doc-update: gascity-routing-model.md — bare gc sling <pool> <bead> now clears a stale assignee on an open bead, so Lane 1's field contract omits a write and Lane 3 (--reassign) is no longer the only way to clear (tk-qxocm)#209

Closed
zook-bot wants to merge 2 commits into
mainfrom
polecat/tk-qxocm

Conversation

@zook-bot

Copy link
Copy Markdown
Contributor

Summary

Target

  • Brief(s): docs/gascity-routing-model.md

Change

  • Lane 1 (gc sling <pool> <bead>) now CLEARS a stale assignee on an existing
    open bead, so the brief's field-level contract is wrong in two places:
    Lane 1's explicit "Sets:" / "Does NOT:" list omits that write, and
    Lane 3 (--reassign) is still presented as the way to clear a prior
    assignee while routing to a pool. A reader following the brief concludes a
    bare pool sling leaves a stale assignee in place and that --reassign is
    required to bounce an assigned bead back to a pool. Both are now false.
  • What is still true and must be preserved: Lane 1 does not stamp an
    assignee on a pool target; the singleton stamp is unchanged; --reassign
    remains meaningfully distinct because the new bare-sling normalization is
    scoped — it applies only to status=open beads (in_progress marks a live
    claim that sling must not strip), only to pool targets
    (SupportsInstanceExpansion()), and is skipped for dry-run, formula pours,
    and custom sling_query agents (IsCustomSlingQuery). So --reassign is
    still the lane for clearing an in_progress assignee.
  • The rationale worth capturing: pool demand is defined as ready + unassigned +
    routed, the shared predicate behind both reconciler spawn (scale_check) and
    worker claim (work_query Tier 3). A re-pour that only rewrote
    gc.routed_to to the same value was invisible to both and stalled silently
    (gc-q40pm). Note also the companion change in sling_attachment.go: a
    routed-but-assigned pool bead is no longer treated as an already-settled
    idempotent no-op — it now falls through so preflight can normalize it.
  • The audit does not propose prose: read the cited commits ('git log -p' in the
    named repo) and write the real edit. If review shows nothing needs changing,
    close as a no-op.

Provenance

  • Drift: an upstream change moved a surface one or more briefs above track.
  • gascity fa487632b^..fa487632b (2026-07-11) — "sling: clear stale assignee on
    bare pool re-pour (re-land gc-q40pm) (gc-yey06) (feat(lesson-capture): correction→bake-in loop v1 — lesson-capture fragment + mechanik bake-in doctrine (tk-7druqk) #88)".
    • internal/sling/sling_core.go — new clearStaleOpenAssigneeForPoolRoute /
      clearStaleOpenAssigneeInStore, invoked from preflight under the guard
      !opts.DryRun && !opts.IsFormula && a.SupportsInstanceExpansion() && !IsCustomSlingQuery(a).
    • internal/sling/sling_attachment.goCheckBeadStateWithOptions now
      treats only an empty assignee as settled for pool targets.
    • engdocs/architecture/dispatch.md — upstream's own prose for the new rule
      (a good model for the brief's wording).
    • Verified still present at gascity origin/main (7bba3de3b).
  • Note on SHA freshness: the brief's provenance table cites gascity/main
    4ff645484, which is no longer an ancestor of origin/main (the fork was
    rebased). Per standing audit practice, rebase-churned provenance pins are NOT
    themselves drift — do not chase them beyond what this change requires.

Implementation notes

Implemented (tk-qxocm): docs/gascity-routing-model.md updated for gascity fa487632b (verified live at gascity/main 7bba3de3b).

Lane 1: added an 'Also clears (pool targets, open beads)' bullet — the stale-assignee clear, its rationale (pool demand = ready + unassigned + routed, the shared scale_check / work_query Tier 3 predicate; a re-pour that only rewrote gc.routed_to stalled silently, gc-q40pm), the emitted warning text, and all three scope conditions (status=open only; SupportsInstanceExpansion() pool targets only; skipped for --dry-run, --formula pours, custom sling_query). Reworded 'Does NOT: set assignee' to 'Does NOT: stamp an assignee', distinguishing not-stamping from not-writing. Singleton stamp rule preserved unchanged.

Lane 3: reframed — '--reassign is no longer the only way to clear'; it stays the lane for what the normalization excludes: in_progress beads (and there it also resets status in_progress->open in the same store write, which is what actually makes the bead claimable — IsReadyCandidate requires status=open, gastownhall/gascity#3231), singleton targets, --formula pours, custom sling_query agents. Added the status reset to Lane 3's 'Sets:' — the brief never documented it and it is load-bearing for the distinctness claim.

Also: noted the companion sling_attachment.go change (routed-and-assigned pool bead no longer short-circuits as a settled idempotent no-op); tightened the targetless-sling section's restatement of Lane 1's contract so it does not read as 'no assignee write'; added the provenance row.

Disambiguation worth flagging to review: the guard is !opts.IsFormula, and IsFormula is the 'gc sling --formula' shape — NOT '--on '. The doc now says so explicitly, because the two are easy to conflate.

ADJACENT DRIFT, NOT FILED (out of scope for this bead, target was the brief only): doctor/check-keeper-repour-reassign/run.sh lines 8-11 state 'gc sling stamps gc.routed_to but does NOT clear assignee by default, so without --reassign the pool reconciler sees a stale claim'. Those keeper re-pour sites use ' --on mol-...', which is NOT the --formula shape, so the new normalization DOES apply to them on open beads — the check's stated rationale is now stale. The check itself may still be justified (it holds for a parked in_progress bead); the comment needs re-derivation. Suggest the doc-keeper file this separately.
Rework tk-c07sq landed on polecat/tk-qxocm at b3122d1; re-review tk-e0r6z dispatched (one-anchor-per-PR, tk-ynz4b).

Refinery handoff

  • Issue: tk-qxocm (task, P2)
  • Source branch: polecat/tk-qxocm
  • Target: main
  • Codex signed off pre-open at b3122d14; PR opened codex-green.

zook-bot added 2 commits July 22, 2026 08:23
gascity fa487632b ("sling: clear stale assignee on bare pool re-pour
(re-land gc-q40pm)", merged 2026-07-11, live at gascity/main 7bba3de3b)
made a bare `gc sling <pool> <bead>` clear a stale assignee on an
existing open bead. That falsified the brief's field-level contract in
two places:

- Lane 1's **Sets:** / **Does NOT:** list omitted the write entirely, so
  a reader concluded a bare pool sling leaves a stale assignee in place.
- Lane 3 (`--reassign`) was presented as *the* way to clear a prior
  assignee while routing to a pool.

Lane 1 now documents the clear, its rationale (pool demand is ready +
unassigned + routed — the shared predicate behind scale_check and
work_query Tier 3; a re-pour that only rewrote gc.routed_to stalled
silently, gc-q40pm), and each load-bearing scope condition: status=open
only, pool targets only, skipped for --dry-run / --formula pours /
custom sling_query agents. Notes the companion sling_attachment.go
change: a routed-and-assigned pool bead no longer short-circuits as a
settled idempotent no-op. Preserved intact: sling does not *stamp* an
assignee on a pool target, and the singleton stamp is unchanged — the
"Does NOT" bullet now distinguishes not-stamping from not-writing.

Lane 3 keeps its distinct role, stated in terms of what the new
normalization excludes: in_progress beads (where --reassign also resets
status back to open, the part that actually makes the bead claimable —
IsReadyCandidate requires status=open, gastownhall/gascity#3231),
singleton targets, --formula pours, and custom sling_query agents.

Also disambiguates "formula pour" as the --formula shape specifically:
`--on <mol>` on an ordinary bead sling is NOT excluded, so the clear
still applies there. Adds the provenance row.
…astownhall (tk-c07sq)

Addresses the pre-open signoff finding on tk-dmqdq (P2, line 31).

The provenance row credited `fa487632b` to `gastownhall/gascity` as a
2026-07-11 merge. Verified against the local gascity checkout:

  - `fa487632b` is NOT an ancestor of `upstream/main` (gastownhall);
    it IS an ancestor of `origin/main` (zookanalytics fork).
  - `clearStaleOpenAssigneeForPoolRoute` appears nowhere in
    `upstream/main` — the change is fork-local, not upstream at all.
  - `(#88)` in the subject is the fork PR, not a gastownhall PR.
  - 2026-07-11 is the AUTHOR date; committer date is 2026-07-22. The
    fork rebases `main`, so the SHA churns (the same subject appears on
    a dozen rebased commits) — no durable "merged on <date>" claim.
  - `engdocs/architecture/dispatch.md:180` was cited as "upstream's own
    prose", but upstream's copy of that file carries no such passage.

Rewrites the row to name the fork as producer, state plainly that the
change is absent from canonical gascity, and pin verification to
`zookanalytics/gascity` `origin/main` `7bba3de3b`. Line-number citations
re-verified at that commit and left as-is.

Also scopes the two body claims that inherited the same misattribution
(Lane 1 "Also clears", Lane 3 "no longer the only way to clear") so the
doc's field contract does not read as canonical-gascity behavior.
@zook-bot

Copy link
Copy Markdown
Contributor Author

Codex signoff (pre-open, comment-only — not an approval):

Code Review Results

Scope: origin/main...b3122d144f9f4ed278b1c213853e607fa4062954 (1 file, documentation-only)
Intent: Update docs/gascity-routing-model.md so Lane 1 documents bare pool sling clearing stale assignees on open beads, corrects the fork-local provenance for fa487632b, and scopes Lane 3 / --reassign to the remaining clear-and-reopen cases.
Mode: pre-open branch review

Reviewers: correctness, testing, maintainability, project-standards

  • correctness -- verified changed prose against zookanalytics/gascity main=7bba3de3b6f690ca8bf0c5e54262deae3d620541 and gastownhall/gascity main=921b32993efe90a89afce0348947df4143f19d36
  • testing -- documentation-only review; ran whitespace/diff validation and source inspection rather than product tests
  • maintainability -- checked the new Lane 1 / Lane 3 wording for internal consistency and ambiguity around --formula vs --on
  • project-standards -- no applicable repo-local AGENTS.md or CLAUDE.md standards found for the changed file

Coverage

  • Findings: none.
  • Verified reviewed branch head: b3122d144f9f4ed278b1c213853e607fa4062954.
  • Verified the previous review's provenance issue is addressed: the row now identifies fa487632b as fork-local to zookanalytics/gascity, says it is not in canonical gastownhall/gascity, and distinguishes author/committer dates from a canonical merge claim.
  • Verified Lane 1's clear against fork source: internal/sling/sling_core.go preflight guard at lines 158-159, clearStaleOpenAssigneeForPoolRoute at line 1617, and clearStaleOpenAssigneeInStore open-only gate at line 1660.
  • Verified the companion idempotency change against internal/sling/sling_attachment.go line 463: routed-and-assigned pool beads are no longer treated as settled.
  • Verified Lane 3's status-reset claim against reopenForReassignInStore: in_progress is reset to open in the same update path.
  • Verified upstream-negative claim: canonical gastownhall/gascity main=921b32993efe90a89afce0348947df4143f19d36 has no clearStaleOpenAssigneeForPoolRoute / stale-assignee clear passage.
  • git diff --check passed for docs/gascity-routing-model.md.
  • Residual risk: doctor/check-keeper-repour-reassign/run.sh still has stale rationale text saying bare gc sling does not clear assignee by default. That script is outside this branch's target file, and its enforced --reassign invariant may still be correct for parked in_progress keeper re-pours, so I did not block this doc-only signoff on it.

Actionable findings: none.


Verdict: Ready to merge

Reasoning: The changed routing-model prose matches the forked Gas City implementation this city runs, the fork-vs-upstream provenance correction is now accurate, and no blocking documentation inconsistencies were found in the reviewed file.

| `CrossStoreRouteError` cross-store route guard | gascity source | `rigs/gascity/internal/sling/sling_core.go:607` (`validateBuiltInRouteStoreReachable`), gated by `shouldValidateBuiltInRouteStoreReachable` (`sling_core.go:210`) — note its predicate omits the `!opts.Force` bypass that `shouldGuardCrossRig` (`sling_core.go:202`) carries, so `--force` does not relax it; error text at `internal/sling/sling.go:686`. Verified current at gascity/main `434d57656` (the singleton assignee-stamping change, last commit to touch the guard). | 2026-06-19 |
| PR #2779 — `gc.routed_to` made the sole persisted routing key; `gc.run_target` demoted to compile-time-only (merged 2026-06-01) | gastownhall/gascity | https://github.com/gastownhall/gascity/pull/2779 (commit `fb32be6941be7627aaf169809e31629f0baf6118`); definition in `engdocs/design/session-model-unification.md` | 2026-06-19 |
| PR #3670 — `feat: add default_sling_targets for multi-target random dispatch` (merged 2026-07-03) | gastownhall/gascity | https://github.com/gastownhall/gascity/pull/3670; field at `rigs/gascity/internal/config/config.go:645`, resolver at `rigs/gascity/cmd/gc/cmd_sling.go:291`. Verified current at gascity/main `4ff645484`. | 2026-07-16 |
| `fa487632b` — `sling: clear stale assignee on bare pool re-pour (re-land gc-q40pm) (gc-yey06) (#88)` — **fork-local; not in canonical gascity** | zookanalytics/gascity (fork) | **Not in `gastownhall/gascity`:** `fa487632b` is not an ancestor of `upstream/main`, and `clearStaleOpenAssigneeForPoolRoute` appears nowhere in it — so Lane 1's clear describes the fork this city runs, not canonical gascity. Landed on the fork as `zookanalytics/gascity#88`; author date 2026-07-11, committer date 2026-07-22 (the fork rebases `main`, so this SHA churns — identify the change by subject, not by SHA). Guard and clear in `internal/sling/sling_core.go` — `preflight` block at `:158`, `clearStaleOpenAssigneeForPoolRoute` at `:1617`, `clearStaleOpenAssigneeInStore` (the `status=open` gate) at `:1660`; companion idempotency change `settledAssignee` in `internal/sling/sling_attachment.go:463`; the fork's own prose at `engdocs/architecture/dispatch.md:180` (upstream's copy of that file carries no such passage). All verified current at `zookanalytics/gascity` `origin/main` `7bba3de3b`. | 2026-07-22 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely do not commit details about fork local routing models. Plus we are trying hard to remove any of our local commits.

still runs *after* the clear, so the net effect is "prior assignee
cleared, `assignee=<target>` set."

**This is no longer the only way to clear** — on the fork this city

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, definitely no

@zook-bot

Copy link
Copy Markdown
Contributor Author

Closing unmerged per operator review (johnzook, 2026-07-22). This update documents fork-local routing behavior — commit fa487632b (zookanalytics/gascity fork; not in canonical gastownhall/gascity). The city is shedding its fork-local commits rather than documenting them, so this divergence should not be entrenched in docs/gascity-routing-model.md. Abandoning the change; the doc stays describing canonical routing. Anchor bead tk-qxocm resolved wont-do.

@zook-bot zook-bot closed this Jul 24, 2026
@zook-bot
zook-bot deleted the polecat/tk-qxocm branch July 24, 2026 04:42
zook-bot added a commit that referenced this pull request Jul 24, 2026
…sling as Lane 4, correcting two inverted claims (tk-wd9do)

The brief is the authority on which routing field each delivery path
sets, but its lanes covered only the bare sling, the direct `--assignee`
write, and `--reassign`. The formula sling was an undocumented fourth
path. Adds Lane 4 plus three subsections: the graph.v2 duplicate-wisp
trap, the standalone-launch contrast, and why assignee residue strands a
routed bead.

Re-verifying at the pinned revision (the bead asked for this explicitly)
showed the filing bead's two central claims were INVERTED. Both are
corrected here; the doc states the verified behavior, not the premise:

1. Bead: "`gc.routed_to` lands on the WISP ROOT, not the target bead."
   Actual: `--on` routes the SOURCE bead and deliberately leaves the wisp
   root unrouted. `TestOnFormulaAttachesAndRoutes`
   (cmd/gc/cmd_sling_test.go:4105) asserts both halves; sling_core.go:488
   says "Do not 'fix' this to wispRootID — it would orphan the work"
   (gastownhall/gascity#2848). Routing the root is the STANDALONE LAUNCH
   shape (slingFormula, sling_core.go:373) — the path the premise
   actually describes.

2. Bead: "`--reassign` is IsFormula-gated on this path, leaving assignee
   residue." Actual: `--formula` and `--on` are mutually exclusive
   (cmd_sling.go:158) and AttachFormula leaves IsFormula false
   (sling.go:326), so `shouldReopenForReassign` runs normally on `--on`.
   The guaranteed no-op is a standalone LaunchFormula — exactly what
   upstream's own comment says (sling.go:273-279).

The bead's operational conclusion still holds, via a different mechanism
worth documenting: a graph.v2-attached work bead really does show no
`gc.routed_to` and no assignee while fully dispatched — not because
routing moved to the wisp root, but because the graph path returns
before the routing call (sling_core.go:477-481 -> doStartGraphWorkflow).
Re-slinging on that misread still pours a duplicate wisp.

Claim 3 (pool demand counts routed AND unassigned) verified as stated:
workquery.go:41-43.

Scope note: cites canonical gastownhall/gascity only, pinned at
upstream/main 1dbf0731e. Per the operator ruling on the sibling PR #209
("do not commit details about fork local routing models"), no
fork-local content is added — all four behaviors were differentially
confirmed identical in fork and upstream, so the canonical citation is
also correct for the running city. Stays out of Lane 1/Lane 3 assignee
semantics, which are tk-qxocm's territory.
zook-bot added a commit that referenced this pull request Jul 24, 2026
…sling as Lane 4, correcting two inverted claims (tk-wd9do)

The brief is the authority on which routing field each delivery path
sets, but its lanes covered only the bare sling, the direct `--assignee`
write, and `--reassign`. The formula sling was an undocumented fourth
path. Adds Lane 4 plus three subsections: the graph.v2 duplicate-wisp
trap, the standalone-launch contrast, and why assignee residue strands a
routed bead.

Re-verifying at the pinned revision (the bead asked for this explicitly)
showed the filing bead's two central claims were INVERTED. Both are
corrected here; the doc states the verified behavior, not the premise:

1. Bead: "`gc.routed_to` lands on the WISP ROOT, not the target bead."
   Actual: `--on` routes the SOURCE bead and deliberately leaves the wisp
   root unrouted. `TestOnFormulaAttachesAndRoutes`
   (cmd/gc/cmd_sling_test.go:4105) asserts both halves; sling_core.go:488
   says "Do not 'fix' this to wispRootID — it would orphan the work"
   (gastownhall/gascity#2848). Routing the root is the STANDALONE LAUNCH
   shape (slingFormula, sling_core.go:373) — the path the premise
   actually describes.

2. Bead: "`--reassign` is IsFormula-gated on this path, leaving assignee
   residue." Actual: `--formula` and `--on` are mutually exclusive
   (cmd_sling.go:158) and AttachFormula leaves IsFormula false
   (sling.go:326), so `shouldReopenForReassign` runs normally on `--on`.
   The guaranteed no-op is a standalone LaunchFormula — exactly what
   upstream's own comment says (sling.go:273-279).

The bead's operational conclusion still holds, via a different mechanism
worth documenting: a graph.v2-attached work bead really does show no
`gc.routed_to` and no assignee while fully dispatched — not because
routing moved to the wisp root, but because the graph path returns
before the routing call (sling_core.go:477-481 -> doStartGraphWorkflow).
Re-slinging on that misread still pours a duplicate wisp.

Claim 3 (pool demand counts routed AND unassigned) verified as stated:
workquery.go:41-43.

Scope note: cites canonical gastownhall/gascity only, pinned at
upstream/main 1dbf0731e. Per the operator ruling on the sibling PR #209
("do not commit details about fork local routing models"), no
fork-local content is added — all four behaviors were differentially
confirmed identical in fork and upstream, so the canonical citation is
also correct for the running city. Stays out of Lane 1/Lane 3 assignee
semantics, which are tk-qxocm's territory.
zook-bot added a commit that referenced this pull request Jul 24, 2026
…d side) and that hold metadata is inert (tk-16753)

The brief was the authority on which routing field each delivery path
SETS, but never stated what makes a bead claimable — so the enforcement
boundary those writes feed was left implicit. work-bead-state-machine.md
already forward-referenced this doc for the predicate; that reference
had nothing to land on.

Adds "The read side: the claim predicate":

- The tiers, mapped onto the lanes: assignee match (Lane 2's read side)
  over $GC_SESSION_ID/$GC_SESSION_NAME/$GC_ALIAS, and the routed-pool
  query (Lanes 1 and 3's read side). Full term table for the pool case:
  status=open, no active blockers, gc.routed_to=<pool>, unassigned,
  not epic.
- Offer and demand are ONE predicate read two ways — work_query returns
  a sorted limited list, scale_check counts the same filter at
  --limit 0. They differ in the shape of the answer, not in the terms.
- Metadata is not enforcement: gc.routed_to is the only metadata key in
  the predicate. A bespoke park flag (rebase_hold=true, hold_reason=...)
  is documentation — the bead is still open, routed and unassigned, so
  it is still offered and still claimable. The failure is silent and
  asymmetric: the stamper gets no error and believes the bead is parked;
  only the worker that claims it anyway finds out.
- How to actually hold: falsify a term. Clear gc.routed_to (covers the
  pool tier, offer and demand) AND clear assignee, or move status off
  open. Notes the recognized hold:<value> label convention that gc
  doctor checks against the routing fields.

Re-verified against the running implementation rather than copied from
the source memory entry, per the bead. That re-verification CORRECTED
the bead's own framing: it expected demand to add "unassigned" to an
offer predicate that lacked it, but both tiers carry --unassigned, so
the two are the same predicate. Provenance row cites gc hook --help,
gc bd ready --help, the built-in queries in the gc binary, the
UnassignedRoutedWork/UnassignedInProgressPoolWork symbols, this rig's
proactive agent config, and the live gc doctor hold-label checks.

Coordination: PR #209 (tk-qxocm) is in flight on this same file. It
states the shared predicate in passing to justify Lane 1's stale-assignee
clear; this change names it as its own section and adds the inert-metadata
consequence, which #209 does not cover. The two agree. Both append a
Provenance row, so expect a trivial both-added conflict there on rebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants