PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
PR Gates / Docs update reminder (pull_request) Successful in 12s
PR Gates / decisions lifecycle (pull_request) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 40s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 23s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 20s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 21s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 13s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Amends ci.monitor-armed-at-pr-open (prose + Signals only; heading and key unchanged, no supersession) with the monitor-classification rules that were missing. Landing #436/#583 today, my CI monitor filtered per-context statuses on `!= "success"` and announced "NOT all green" on two fully green PRs, because `Build & push image (amd64)` reports `skipped`. Nothing was blocked — Gitea's combined /status already treats skipped as non-blocking and reported overall=success — but a false red costs a diagnosis cycle every time. Two corrections recorded: - The image job is skipped on EVERY PR (job-level `if: github.event_name != 'pull_request'`; images build only on push-to-main and tags), NOT because of the docs-only mechanism. Misattributing it to docs-only is a plausible-sounding wrong diagnosis, since docs-only gates STEPS precisely so required jobs still report success. decisions.md already stated the fact from the branch-protection angle; the monitor-authoring consequence was missing. - skipped / failure / cancelled are three distinct meanings and must not be collapsed. Prefer gating on the combined `.state`. The documented filter is verified in BOTH directions: silent on a green PR carrying a skipped build, and still dirty on a genuinely pending run. My first draft of it was itself broken — `select(.status != …)` after the pipeline had renamed `.status` to `.st`, so it compared against null and reported a green PR as nine failures. That failure is recorded in the note, per the "verify your detector" rule. fixes #592
622 lines
51 KiB
Markdown
622 lines
51 KiB
Markdown
# Session-workflow, CI-triage & review-process decisions (#542)
|
||
|
||
How we *work* — orchestration, CI-run triage, review routing, worktree and parallel-session hygiene,
|
||
and the tooling gotchas that have cost real sessions. Distinct from the engineering "why the code
|
||
does X" records: nothing here decides product behaviour.
|
||
|
||
These records were extracted from `docs/handoffs/chicorytv-issue-queue.md` in #542. That doc had
|
||
become the **only** copy of most of this knowledge — an inventory found ~30 of ~38 lore bullets had
|
||
no decision record at all — so it was neither retrievable via the catalog nor safe from a careless
|
||
prune. The rules are now records; the handoff doc keeps the instruction and points here for the why.
|
||
|
||
Related: `release-ci-governance.md` (merge/release gates and hooks), `startup.parallel-orientation`
|
||
(session-start tracks), `session.shared-checkout-refresh` (session-end shared-tree refresh).
|
||
|
||
---
|
||
|
||
## 2026-07-21 — A killed CI job reports `conclusion: failure`; read the log tail before diagnosing the diff (#542)
|
||
|
||
`key: ci.killed-job-triage` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Never trust a job's `conclusion` field alone — read the log tail and require an `❌ Failure - Main …` marker before treating a red as a real failure.
|
||
**Signals:** killed job · runner restart · conclusion failure · log truncated mid-step · `❌ Failure - Main` marker · `Unable to pull refs/heads/v4` · act cache noise · UTC vs UTC+2 · semantically-null diff · paths: n/a · issues: #542
|
||
**Mechanics:** Gitea Actions job logs + runner container uptime on the runner host.
|
||
|
||
When the operator restarts the runners (a retune, a reboot), in-flight jobs die and Gitea marks them
|
||
**`failure`**, not `cancelled`. The tell is that the log **stops mid-step with no error and no
|
||
`❌ Failure - Main …` marker** — a real failure always leaves that marker. On run 1006, `EF migration
|
||
integrity` and `Functional E2E` both "failed" on a **BOM-removal-only** diff (6 files, one line each,
|
||
zero content change) that could not possibly break them; the logs simply truncated mid-`dotnet build`
|
||
at 12:08 UTC and both runners showed `Up About an hour` — the retune had killed them.
|
||
|
||
**Log timestamps are UTC; the host is UTC+2.** Convert before correlating, or the restart looks two
|
||
hours off and a correct theory gets wrongly discarded.
|
||
|
||
Corollary: a diff that *cannot* cause a failure is evidence the failure isn't yours — when a job that
|
||
passed on the previous head fails on a semantically-null delta, suspect the environment and go read
|
||
the log rather than re-litigating the diff. Beware warnings that look fatal: `Unable to pull
|
||
refs/heads/v4: …` is act refreshing its `/root/.cache/act` action cache and is followed by `Cloned …`
|
||
— it is noise, not a cause. Grep for the failure marker, not for the word "error".
|
||
|
||
## 2026-07-21 — `cancelled` is not `failure`; a cancelled run is no verdict (#542)
|
||
|
||
`key: ci.cancelled-is-not-a-verdict` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Treat a `cancelled` conclusion as "no verdict" — never as pass or fail — and report FAILED and CANCELLED counts separately in any CI monitor.
|
||
**Signals:** conclusion cancelled · run-level vs job-level conclusion · pre-cancel genuine failure · CI monitor state != pending · phantom failure · paths: n/a · issues: #542
|
||
**Mechanics:** Gitea Actions run/job API; monitor logic, e.g. `fail=[j for j in jobs if j['conclusion']=='failure']; canc=[j for j in jobs if j['conclusion']=='cancelled']`.
|
||
|
||
The operator cancels runs by hand — they are the only party who can (see
|
||
`ci.batch-pushes-no-cancel-route`) — and a run-level `conclusion: cancelled` means the run produced
|
||
**no verdict** about your diff. Two traps follow. First, a run whose *overall* state is `failure` may
|
||
hold a **genuine job failure that happened before the cancel**: check job-level `conclusion` plus
|
||
timestamps rather than attributing the red to the cancel. Second, a cancelled run tells you nothing,
|
||
so never claim green on one.
|
||
|
||
A monitor that only asks "is state != pending" will report a cancelled run as a failure and send the
|
||
next session debugging a phantom. Split the two counts explicitly.
|
||
|
||
## 2026-07-21 — An infra-shaped red under host load is not a code failure (#542)
|
||
|
||
`key: ci.infra-shaped-red-under-load` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure.
|
||
**Signals:** load average 243 · `Setup .NET` 11ms failure · `remove /root/.cache/act/…/.gitignore: no such file or directory` · buildx `DeadlineExceeded: failed to compute cache key` · `Build & push image` · `EF migration integrity` · `FactoryServer-L` game server · phantom CI bug · paths: n/a · issues: #542
|
||
**Mechanics:** `uptime` on the runner host (bumblebee); Gitea Actions job step logs.
|
||
|
||
On 2026-07-17, with bumblebee at load average **243**, two unrelated jobs died in ways that look like
|
||
code bugs but weren't: `EF migration integrity` failed in ~11ms inside **`Setup .NET`** with
|
||
`remove /root/.cache/act/…/.gitignore: no such file or directory` (act's shared cache), and main's
|
||
`Build & push image` died on buildx `DeadlineExceeded: failed to compute cache key` after ~210s of
|
||
retries. Both re-ran green at normal load, and the migration job had no model change to test in the
|
||
first place.
|
||
|
||
The diagnostic tell is the **location** of the failure: a setup or cache step, before your code
|
||
compiles. One sample under load is not evidence of a systemic problem — this nearly got filed as #390
|
||
lane-rebalance fallout, which the evidence did not support, and would have sent the next session
|
||
chasing a phantom. Note also that a game server (`FactoryServer-L`) shares that host with the
|
||
runners, so high load is not always CI's own doing.
|
||
|
||
## 2026-07-21 — Batch your pushes: there is no agent-side cancel route on Gitea 1.25.4 (#542)
|
||
|
||
`key: ci.batch-pushes-no-cancel-route` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Hold review fixes, doc corrections and format fixes locally and push **once** — a superseded run cannot be cancelled from the agent side and holds a runner slot until it finishes.
|
||
**Signals:** cancel run 404 · Gitea 1.25.4 · `POST /api/v1/.../actions/runs/{id}/cancel` · MCP `actions_run_write` `cancel_run` · session+CSRF · `dispatch_workflow` · orphaned run · 4-slot runner · paths: n/a · issues: #542
|
||
**Mechanics:** Gitea Actions REST + MCP `actions_run_write`; operator-only cancel in the browser UI.
|
||
|
||
Cancellation is impossible from the agent side on this Gitea (**1.25.4**):
|
||
`POST /api/v1/.../actions/runs/{id}/cancel` returns **404**, MCP `actions_run_write`'s `cancel_run`
|
||
returns **404**, and the web-UI route needs a session + CSRF that does not script (login 303s with no
|
||
session cookie). **Only the operator can cancel, in the browser** — so if you must supersede a live
|
||
run, say so explicitly instead of leaving it burning. (`dispatch_workflow` is a different route and
|
||
still works for re-triggering a **main** run.)
|
||
|
||
This corrects the older "superseded runs drain on their own" framing: they do finish, but they hold
|
||
one of the 4 runner slots while doing it, which is a real cost with several parallel sessions.
|
||
|
||
## 2026-07-21 — Do not gate or throttle pushes on host health (#542)
|
||
|
||
`key: ci.no-host-health-gating` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Push when your work is validated — never SSH to bumblebee to sample load/RAM first, and never hand-schedule around other sessions' runs.
|
||
**Signals:** trust the build queue · runner retune · don't sample load before pushing · queueing is the queue's job · paths: n/a · issues: #542
|
||
**Mechanics:** Gitea build queue (two runners, 4 slots).
|
||
|
||
The runners were retuned for stability (operator, 2026-07-17); queueing is the queue's job, not the
|
||
agent's. Sampling host load before a push is both unreliable and unnecessary, and hand-scheduling
|
||
around other sessions produces coordination that no one can verify.
|
||
|
||
Batch your pushes (see `ci.batch-pushes-no-cancel-route`) because orphaned runs cannot be cancelled —
|
||
**not** because the host needs protecting. The two rules have the same action and different reasons;
|
||
conflating them leads to load-watching behaviour that this record forbids.
|
||
|
||
## 2026-07-21 — A lone `decisions lifecycle` red is a known infra flake: do nothing (#542)
|
||
|
||
`key: ci.decisions-lifecycle-flake` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** When `decisions lifecycle` is the **only** red job, do not investigate and do not create a new run to clear it — no rebase, no `--amend`, no no-op push; the operator reruns that single job from the Gitea UI.
|
||
**Signals:** decisions lifecycle red · append-only gate · known flake · operator selective rerun · no-op push · convincing local explanation · paths: `docs/decisions.md` · issues: #542, #473, PR #479
|
||
**Mechanics:** Gitea Actions `decisions lifecycle` job; operator-driven single-job rerun in the web UI.
|
||
|
||
Operator-stated, 2026-07-19. Report it as a known flake and carry on; only if *other* jobs are red too
|
||
does the run deserve diagnosis. Same family as the killed-job rule — a spurious single-job red is
|
||
cleared by the operator's selective rerun, never by pushing, and pushes cannot be cancelled anyway.
|
||
|
||
**The trap is that a convincing local explanation is always available.** On #473/PR #479 the job went
|
||
red just after `main` landed its own `decisions.md` entry, so "mine is no longer at EOF, I must
|
||
rebase" looked airtight. The rebase happened — and the job went red **again** on a head whose diff was
|
||
a verified pure EOF append with zero deleted lines. A rebase that provably satisfied the gate's stated
|
||
rule did not turn it green, which is the proof that the red was never about the diff. Check this rule
|
||
*before* theorising about a cause.
|
||
|
||
## 2026-07-21 — Arm the CI monitor at PR-open, via the commit-status endpoint (#542)
|
||
|
||
`key: ci.monitor-armed-at-pr-open` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Arm a CI monitor on the PR head sha the moment the PR opens, polling the commit-status endpoint — not at the end of the work.
|
||
**Signals:** arm monitor at PR open · commit status endpoint · head sha · red run sat unnoticed · MySQL-apply flake · `skipped` misread as red · `Build & push image (amd64)` skipped · monitor classification · `!= "success"` filter · combined `.state` · paths: n/a · issues: #542, #216, #583
|
||
**Mechanics:** `GET /api/v1/repos/timothy/ersatztv/commits/{sha}/status` for the combined verdict (`.state`); `…/statuses?limit=50` only when you need per-context detail.
|
||
|
||
CI runs concurrently with the review and E2E work that follows a push, so the cost of a late-armed
|
||
monitor is entirely wasted wall-clock. In the #216 session three PR runs sat red for roughly an hour
|
||
on a CI-only flake while review and E2E ran to completion — the reds were only discovered afterwards.
|
||
|
||
**Classify per-context states correctly, or prefer the combined `.state`.** A monitor that enumerates
|
||
contexts and treats anything `!= "success"` as red is WRONG on this repo: `Build & push image (amd64)`
|
||
is `if:`-gated at the JOB level on `github.event_name != 'pull_request'`, so it reports **`skipped` on
|
||
every PR**, by design and regardless of content (images are built only on push-to-main and tags — see
|
||
`ci.docs-only-skip-steps`, which records the same fact from the branch-protection angle: "`build` is
|
||
`skipped` on every PR").
|
||
Such a monitor cries "NOT all green" on a perfectly green PR. Note this is *not* the docs-only skip:
|
||
the docs-only mechanism deliberately gates individual STEPS so required jobs still report `success` in
|
||
seconds — misattributing the image job's skip to docs-only is a plausible-sounding wrong diagnosis
|
||
(#583 session, 2026-07-25).
|
||
|
||
Three distinct non-`success` states, three meanings — do not collapse them:
|
||
- **`skipped`** — deliberately not applicable. Settled, and *not* red. Gitea's combined `.state` already
|
||
treats it as non-blocking (a PR with a skipped `build` reports `overall=success`), which is why the
|
||
combined endpoint is the safer thing to gate on.
|
||
- **`failure`** — a real red; diagnose it (but first check `ci.killed-job-triage` and
|
||
`ci.infra-shaped-red-under-load`).
|
||
- **`cancelled`** — no verdict at all; see `ci.cancelled-is-not-a-verdict`.
|
||
|
||
Working filter when you do enumerate — verified silent on a green PR carrying a skipped `build`, and
|
||
verified to still report a genuinely unfinished run (i.e. proven able to go dirty, per
|
||
`process.bom-format-detection-recipe`'s "verify your detector" rule):
|
||
|
||
```bash
|
||
curl -s -u "$ETV_GITEA_BASICAUTH" ".../commits/$SHA/statuses?limit=50" \
|
||
| jq -r '[.[]|{c:.context,st:.status}]|group_by(.c)|map(.[0])|.[]
|
||
|select(.st!="success" and .st!="skipped")|"NOT-GREEN: \(.c) = \(.st)"'
|
||
```
|
||
|
||
**Mind the renamed key.** The first draft of this snippet said `select(.status != …)` after the
|
||
pipeline had already renamed `.status` to `.st`, so the comparison ran against `null`, passed
|
||
*everything*, and reported a fully green PR as nine failures. Report `failure` and `cancelled` in
|
||
separate counts.
|
||
|
||
Related context for interpreting an early red: the old MySQL host-port 3306 collision is fixed on main
|
||
(`ef8915f1`), so a lone MySQL-apply red now indicates the known infra flake and warrants a rerun
|
||
rather than diagnosis.
|
||
|
||
## 2026-07-21 — Heavy-render web tests need explicit per-test vitest timeouts on the CI VM (#542)
|
||
|
||
`key: ci.web-test-per-test-timeouts` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Give heavy-render web tests an explicit per-test vitest timeout (e.g. 15s); never raise the global default to fix one slow test.
|
||
**Signals:** vitest timeout · 5s default · 100+ item grid · CI VM slower than local · run 686 · paths: `web/` tests · issues: #542
|
||
**Mechanics:** per-test timeout argument in the vitest test declaration.
|
||
|
||
The CI VM is materially slower than a dev Mac for render-heavy work. A web test rendering a 100+ item
|
||
grid runs in about 1s locally but hit the **5s** vitest default on the CI VM (run 686) and went red
|
||
for no code reason.
|
||
|
||
Bump the timeout on the specific test. Raising the global default hides genuine hangs across the whole
|
||
suite in exchange for fixing one known-slow case.
|
||
|
||
## 2026-07-21 — Build and verify locally, then trust it; CI confirms (#542)
|
||
|
||
`key: ci.verify-locally-ci-confirms` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Treat the local build/verify/review pass as the decision point and CI as confirmation — don't idle waiting on a run you have no reason to doubt.
|
||
**Signals:** ~9 min warm CI · docs-only PR seconds · CI VM 127 · commit status watch · local gate first · paths: n/a · issues: #542, #416
|
||
**Mechanics:** `GET /api/v1/repos/timothy/ersatztv/commits/{sha}/status`; CI on VM 127.
|
||
|
||
A warm full CI run takes roughly **9 minutes**; a docs-only PR completes in seconds since ersatztv#416's
|
||
docs-only skip. Neither duration justifies blocking on the run when the same gates already passed
|
||
locally.
|
||
|
||
Watch the run by commit status rather than by polling the UI, and continue working. CI *confirms* the
|
||
local verdict; it is not the first line of defence.
|
||
|
||
## 2026-07-21 — Independent cross-model review is mandatory on risky diffs; a skip is a stated, auditable exemption (#542)
|
||
|
||
`key: process.independent-review-rubric` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Run an independent review pass — preferably a different model family, otherwise a cold-context review-only agent — on any diff touching locks/concurrency, auth/security, API write-path handlers, or DB migrations, or larger than ~150 changed C# lines; skip only for a pure-SPA/docs leaf with no server-state effect, and state the skip and its reason in the PR or close comment.
|
||
**Signals:** cross-model review rotation · correlated blindspots · skip rubric · self-exemption · review-only agent · quota exhaustion · reviewer does not implement · paths: n/a · issues: #542, #235, #250
|
||
**Mechanics:** rotation — Codex reviews Claude-authored changes, Claude reviews Codex-authored changes; use whatever native integration is installed (client helpers like `codex:rescue` are optional accelerators, not protocol dependencies). On quota/tool exhaustion, fall back to a cold-context review-only agent in the current client and retry cross-model next window. Independent reviewers do review and stuck-diagnosis work only — never implementation on the same change they review.
|
||
|
||
The justification is correlated blindspots: independent model families miss *different* classes of
|
||
defect, so a single reviewer on a risky diff is the gap, not a saving. #235 (a reviewer clearing a lock
|
||
as sound because it never grepped for every producer) and #250 (a vacuous negative control that
|
||
silently ran a stale dll) are both cases the second, differently-blind pass caught.
|
||
|
||
The skip clause is deliberately narrow and deliberately loud. A self-exemption is an auditable claim,
|
||
not a vibe — silence is indistinguishable from forgetting, so the exemption must be written down where
|
||
the merge record can be re-read later.
|
||
|
||
## 2026-07-21 — Local build/test gate + cold-context review run BEFORE the push, not after (#542)
|
||
|
||
`key: process.local-gate-before-push` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Run the local build/test gate and a cold-context, scoped "review only" adversarial review over the diff, fold the fixes, and only then push or open the PR.
|
||
**Signals:** local gate before push · cold review before CI · CI is not the first line of defence · uncancellable run · queue pressure · paths: n/a · issues: #542
|
||
**Mechanics:** local build/test gate + a scoped review-only agent over `origin/main...HEAD`; CI (armed on the PR head sha at PR-open) *confirms* the result.
|
||
|
||
Gitea runners should only ever see already-validated work. CI here is a scarce shared slow resource and
|
||
a run cannot be cancelled on this Gitea version, so spending a run to surface a lint error or a
|
||
review-catchable bug is pure waste plus queue pressure on every other in-flight session.
|
||
|
||
CI confirms; it is not the first line of defence. Treating it as the first line inverts the cost: the
|
||
cheap fast private check is skipped in favour of the expensive slow shared one.
|
||
|
||
## 2026-07-21 — Review disagreement on a gate PR escalates to the frontier judge — and the proposed FIX escalates with it (#542)
|
||
|
||
`key: process.review-disagreement-frontier-judge` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** When independent reviews disagree on a gate PR, escalate to the frontier judge, and put the proposed fix approach in front of it — not just the disputed finding.
|
||
**Signals:** review disagreement · frontier judge · escalate the fix not just the verdict · gate PR reconciliation · paths: n/a · issues: #542, #235
|
||
**Mechanics:** n/a — escalation is a routing decision by the orchestrator.
|
||
|
||
The reconciliation is not only "who is right about the finding". On #235 the frontier model ratified
|
||
the disputed High *and* caught that the orchestrator's own proposed fix (a per-source lock-or-skip over
|
||
a fixed loop order) would deterministically starve the 2nd and later media source — prescribing instead
|
||
the pattern that shipped.
|
||
|
||
That is the whole argument: a judge given only the verdict question would have returned "the High is
|
||
real" and let a starving fix ship. Send the fix approach up with the finding.
|
||
|
||
## 2026-07-21 — "Make X consistent with Y" review findings are new code and get new-code scrutiny (#542)
|
||
|
||
`key: process.consistency-fix-new-code-scrutiny` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Review a "make X consistent with Y" change as new code, not as a mechanical copy — and for any timer or effect involved, ask explicitly "when does this fire?", including on mount.
|
||
**Signals:** consistency fix · propagating the pattern propagates the bug · mount-debounce · timer/effect fire timing · paths: n/a · issues: #542, #216
|
||
**Mechanics:** n/a — a review-attention rule.
|
||
|
||
Propagating a pattern propagates its bugs. A #216 review fix copied MediaBrowseScreen's mount-debounce
|
||
selection-wipe race into SearchScreen instead of catching it — the finding said "be consistent", and
|
||
consistency was achieved, including with the defect.
|
||
|
||
The mount question is the cheap general form of the check: most of this class hides in an effect or
|
||
timer that fires once more than the author pictured.
|
||
|
||
## 2026-07-21 — A lock/channel "no cross-release" verdict must enumerate every producer via grep (#542)
|
||
|
||
`key: process.lock-ownership-enumerate-producers` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Before trusting any "single owner / no double release / no cross-release" claim, grep the whole host project for every writer of that channel message (or acquirer of that lock) — the background scheduler/worker is the usual missing producer.
|
||
**Signals:** cross-release · lock ownership · channel message producers · scheduler enqueues too · grep the host project · paths: n/a · issues: #542, #235, #267, #250
|
||
**Mechanics:** grep for every sender of the message type across the host project (e.g. all `Synchronize{X}Collections(` call sites), not just the controllers present in the PR diff. General form of "re-derive from a grep rather than enumerating from a list": see `sched.weighted-shuffle`.
|
||
|
||
On #235/#267 a cold fork cleared a new `scan-collections` per-provider lock as sound — "only the
|
||
acquirer enqueues, so only its message triggers the scanner unlock" — having checked the three new
|
||
controllers in the diff and stopped there. Codex grepped: `SchedulerService` also enqueues those
|
||
messages periodically and unlocked, and `ScannerService`'s `finally` released the lock whenever held,
|
||
so a scheduled scan cross-releases an API scan's lock (the #250 class).
|
||
|
||
Corollary: the independent pass earns its keep even when the first reviewer returns MERGEABLE. The
|
||
first verdict was not careless — it was complete over the evidence it looked at, which is exactly the
|
||
failure mode a second, differently-scoped reviewer catches.
|
||
|
||
## 2026-07-21 — Before deleting a "workaround", enumerate every behavior it provided (#542)
|
||
|
||
`key: process.enumerate-workaround-behaviors-before-deleting` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** When an issue says "delete X", enumerate every behavior X provided before removing it — a workaround often serves a second purpose that outlives the first.
|
||
**Signals:** remove the workaround · double duty · second purpose · bounded version re-scoped · PENDING_GRACE_TICKS · paths: n/a · issues: #542, #232
|
||
**Mechanics:** n/a — a scoping rule applied when reading a removal issue.
|
||
|
||
#232 said "remove the SPA `PENDING_GRACE_TICKS` grace-tick workaround now the API is honest". The grace
|
||
ticks did double duty: compensating the lying-200 (genuinely obsolete) *and* absorbing the inherent
|
||
queue→observed-active lag and fast-completion race (still needed). Removing them wholesale wedged the
|
||
scan button disabled-until-reload for short scans; both the fork and Codex caught it.
|
||
|
||
The landing move is usually not "delete" or "keep" but a bounded version re-scoped to the purpose that
|
||
still holds.
|
||
|
||
## 2026-07-21 — The shared tree at /Users/timothy/ersatztv is read-only and tells you nothing about `main` (#542)
|
||
|
||
`key: process.shared-tree-readonly` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Never commit in `/Users/timothy/ersatztv` and never read its `git log`/`git status`/HEAD to infer anything about `main` — work in a worktree off `origin/main`, which is the only source of truth.
|
||
**Signals:** shared checkout · "main checkout" · worktree discipline · stale HEAD · origin/main is truth · node_modules copy · paths: `/Users/timothy/ersatztv`, `.claude/worktrees/`, `.worktrees/` · issues: #542
|
||
**Mechanics:** `git worktree add -b <branch> <path> origin/main`; `git fetch origin main` and reference `origin/main` explicitly — the local `main` ref lags and nobody is obliged to update it. The one sanctioned use of the shared tree is read-only: copying `web/node_modules` out of it (keep that fresh — a stale copy broke typecheck in a #198 worktree).
|
||
|
||
There is no "main checkout". `/Users/timothy/ersatztv` is a shared mutable working tree whose HEAD is
|
||
whatever the last session left there; its name lies. The rule holds even for a one-line docs fix — make
|
||
a worktree.
|
||
|
||
This is a design flaw, not a discipline failure: do not "fix" it by telling sessions to check harder,
|
||
because a check does not stay true. It broke for two different sessions on the same day, 2026-07-17,
|
||
both doing the obvious thing. Session A assumed it was on `main` — correct when it pulled at 12:46,
|
||
stale by 14:17 — and committed; the commit landed on the CI session's `ci/604-lane-rebalance`, 24
|
||
seconds after that session's own commit at 14:17:28. Two writers, one tree; nothing tangled, by luck.
|
||
Session B ran `git log` there, read that same branch's HEAD, and concluded `main` was "4 behind origin"
|
||
— a phantom. A session-start `git status` *appears to confirm* the false assumption, which makes it
|
||
worse than a plainly wrong claim.
|
||
|
||
Cross-reference `session.shared-checkout-refresh`, which covers the complementary failure mode: any
|
||
FILE (including a pasted kickoff prompt) read out of that tree may be stale, and the session-end
|
||
refresh script that mitigates it. This record covers the git-state failure mode only.
|
||
|
||
## 2026-07-21 — One worktree, one committing agent (#542)
|
||
|
||
`key: process.one-worktree-one-committing-agent` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Never run two committing agents concurrently on one worktree — give each parallel slice its own worktree branched off the feature branch and merge back.
|
||
**Signals:** parallel agents · concurrent commits · worktree per slice · read-only review agent · paths: n/a · issues: #542
|
||
**Mechanics:** `git worktree add` a fresh tree per slice off the feature branch; merge the slices back into the feature branch when they land.
|
||
|
||
Read-only review agents sharing a tree are fine — the constraint is on *writers*. Sequence slices only
|
||
where one genuinely depends on another's output; "backend-first narrow, SPA-wide after" worked well for
|
||
#216.
|
||
|
||
## 2026-07-21 — Never commit or merge inside a worktree you did not create (#542)
|
||
|
||
`key: process.foreign-worktree-plumbing-merge` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Never commit or merge inside a worktree another session created; land the merge with git plumbing against the branch ref instead.
|
||
**Signals:** sibling session worktree · branch already checked out · plumbing merge · read-tree · commit-tree · GIT_INDEX_FILE · paths: n/a · issues: #542
|
||
**Mechanics:** `git read-tree -m <base> <ours> <theirs>` into a temporary `GIT_INDEX_FILE` → `git write-tree` → `git commit-tree -p <ours> -p <theirs>` → push the resulting commit to the branch ref. The owning session then `git pull`s it.
|
||
|
||
With several sessions on one machine, any given branch may already be checked out in a sibling
|
||
session's worktree. Touching that checkout writes into another agent's working state; the plumbing
|
||
route mutates only the ref, so the merge lands without disturbing the owner's tree.
|
||
|
||
## 2026-07-21 — Fix onto an unmerged feature branch by branching off it and FF-pushing (#542)
|
||
|
||
`key: process.branch-off-feature-branch` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** To fix work on an unmerged feature branch, branch off that branch and land by fast-forward push — and after creating a worktree, drive the first Edit/Read from ITS absolute paths and `git status` it before building.
|
||
**Signals:** unmerged feature branch · FF-push · path leak · sibling worktree recon · wrong tree edits · "0 new tests" · paths: n/a · issues: #542
|
||
**Mechanics:** `git worktree add -b <fix-branch> <path> <feature-branch>`; land with a fast-forward push to the feature branch. Never touch a sibling worktree that has the branch checked out (see `process.foreign-worktree-plumbing-merge`).
|
||
|
||
The path-leak trap, hit on 2026-07-12 (#289 → PR#76): reconnaissance by `Read`ing a *sibling* worktree
|
||
leaked that tree's absolute paths into subsequent `Edit`s, so the edits landed there rather than in the
|
||
fresh worktree. `dotnet` then built the unchanged tree, and the tell was a build reporting "0 new
|
||
tests". A `git status` in the new worktree before the first build catches this in one command.
|
||
|
||
## 2026-07-21 — Bound parallel builds by FREE RAM, never regenerate goldens (#542)
|
||
|
||
`key: process.build-concurrency-limits` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Run at most 3–4 concurrent dotnet/npm builds on this Mac, gate launches on FREE RAM rather than CPU load, and never set `ETV_UPDATE_GOLDENS` / `ETV_UPDATE_PLAYOUT_GOLDENS`.
|
||
**Signals:** build concurrency · memory_pressure · RAM starvation · fan-out cap · golden regeneration · paths: n/a · issues: #542
|
||
**Mechanics:** `memory_pressure -Q` — under 20% free, do not launch more build agents; under 10%, pause or stagger. Host is an M4, 10 cores, 16 GB.
|
||
|
||
Parallelize by default when slices are genuinely disjoint: use up to the client's concurrency limit,
|
||
normally 2–3 delegated agents plus the orchestrator. Fan out bounded recon early, but do not invent
|
||
artificial slices or exceed a runtime-imposed agent cap. CPU load spikes during builds are benign and
|
||
are not a stop signal; the resource that actually runs out is memory — the historic crash was RAM
|
||
starvation from an 8–9-way fan-out, which is why 5+ concurrent builds is never acceptable.
|
||
|
||
Separately: never set the golden-regeneration env vars. Regenerating goldens to make a diff disappear
|
||
hides exactly the change the golden net exists to surface.
|
||
|
||
## 2026-07-21 — A dropped subagent connection is transient; resume, don't restart (#542)
|
||
|
||
`key: process.subagent-drop-resume` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Treat a subagent connection drop as laptop sleep or transient network and re-resume via SendMessage — the work survives.
|
||
**Signals:** subagent disconnect · connection drop · laptop sleep · SendMessage resume · paths: n/a · issues: #542
|
||
**Mechanics:** n/a — client-side resume.
|
||
|
||
Restarting the agent from scratch discards context the agent still holds; the drop is at the transport,
|
||
not the agent.
|
||
|
||
## 2026-07-21 — Every open issue carries a `priority:` label — run the H12 audit at session end (#542)
|
||
|
||
`key: process.issue-qualification-audit` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Run `scripts/issue-qualification-audit.sh` at session end and label everything it flags, including issues you filed that session.
|
||
**Signals:** issue qualification · priority label · queue tiering · unqualified issues · H12 · session-end check · paths: `scripts/issue-qualification-audit.sh` · issues: #542, #312
|
||
**Mechanics:** `scripts/issue-qualification-audit.sh` (H12, ersatztv#312) lists OPEN issues missing a `priority:` label. It is fail-open without Gitea creds and advisory — exit 1 when any issue is unqualified. Sibling to H11 (both #311/#312).
|
||
|
||
Queue tiering keys off `priority:` and gate labels, so an issue without one is simply invisible to the
|
||
selector — it is filed but will never be picked. "Fully qualified" means the issue has a
|
||
`priority: {high,medium,low}` label, which signals triage ran; gate-vs-backlog is then derivable from
|
||
the `review` label and milestone. A milestone is **not** required — backlog is unmilestoned.
|
||
|
||
## 2026-07-21 — Claim with `in-progress` before working; claiming is not collision safety (#542)
|
||
|
||
`key: process.parallel-session-claim` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Apply the `in-progress` label before starting an issue, and still read its dependency notes before touching shared surfaces — a claim prevents duplicate pickup, not overlapping code changes.
|
||
**Signals:** parallel sessions · in-progress label · claim race · dependency notes · shared surfaces · lore pruning · paths: `docs/handoffs/chicorytv-issue-queue.md` · issues: #542
|
||
**Mechanics:** `in-progress` label on the Gitea issue. The tiny read→claim race window is accepted; the later claimant backs off. Runner topology: two runners (ci-runner VM 127 + bumblebee-runner), 4 slots total.
|
||
|
||
The critical nuance is what a claim does *not* buy. Two sessions on non-overlapping issues can still
|
||
collide in the same files. Before touching a shared surface, read the issue's dependency notes ("#234
|
||
after #231", "coordinate with #215").
|
||
|
||
When editing the standing lore/handoff doc, prune covered and stale bullets rather than appending — it
|
||
is not append-only, and git keeps the history. `git pull --rebase` before committing it, since it is
|
||
the single most contended file across parallel sessions.
|
||
|
||
## 2026-07-21 — BOM/format pre-push detection: use the `xxd` loop, and run `dotnet format --include` under `bash -c` (#542)
|
||
|
||
`key: process.bom-format-detection-recipe` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Before any push touching `.cs`, detect BOMs with the `xxd` byte check and verify the format gate with `dotnet format --include` run under `bash -c`, never bare zsh.
|
||
**Signals:** UTF-8 BOM · `efbbbf` · `dotnet format --verify-no-changes` · `--include` · `mapfile` · zsh vs bash · #311 format gate · detector verification · paths: `ErsatzTV.sln`, `.editorconfig` · issues: #542, #311, #70, PR #402, PR #405
|
||
**Mechanics:** Local pre-push shell; CI's format job recipe (`ci.format-gate-folder-mode`). The BOM *policy* is `release.format-as-you-touch-rebase`; this record is the mechanics only.
|
||
|
||
Detection loop, verbatim:
|
||
|
||
```bash
|
||
for f in $(git diff --name-only origin/main...HEAD -- '*.cs'); do head -c3 "$f" | xxd -p | grep -q "^efbbbf" && echo "BOM: $f"; done
|
||
```
|
||
|
||
**Verify your detector.** An `od -An -c | grep '357 273 277'` check reported all-clean while 19 files
|
||
were actually dirty. A detector that can only say "ok" is worse than none — use the `xxd` form or read
|
||
the bytes directly, and distrust a clean result you didn't prove can go dirty.
|
||
|
||
**`dotnet format --include` DOES work here** — an earlier note claiming it silently no-ops was WRONG.
|
||
The apparent no-op was **the shell**: CI's recipe uses `mapfile`, which is bash-only, and the default
|
||
shell here is zsh → empty array → zero files → exit 0. Working form:
|
||
|
||
```bash
|
||
bash -c 'mapfile -t files < <(git diff --name-only --diff-filter=ACM origin/main...HEAD -- "*.cs")
|
||
dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include "${files[@]}"' # exit 0 = gate passes
|
||
```
|
||
|
||
## 2026-07-21 — Run `scripts/e2e-local.sh` against a FRESH config dir; a reused one hangs the readiness probe (#542)
|
||
|
||
`key: testing.e2e-local-fresh-config-dir` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Always point `scripts/e2e-local.sh` at a fresh config dir — a reused one never emits the probe's ready line and the script kills a healthy server.
|
||
**Signals:** `READY_LINE` · `Done migrating search index` · `Search index is already version 50` · readiness probe timeout · e2e-local hang · paths: `scripts/e2e-local.sh`, `docs/e2e-local.md` · issues: #542, #533, #502, #524
|
||
**Mechanics:** The script greps one fixed `READY_LINE` — there is no "wait for either line" option today.
|
||
|
||
The probe waits for `Done migrating search index`, which only a **fresh** config emits. A reused config
|
||
logs `Search index is already version 50` instead, so the line never arrives, the probe times out, and
|
||
the script kills a perfectly healthy server — a failure that reads as a broken build.
|
||
|
||
Surfaced in the #502 session (two failed launches before the cause was obvious) and recovered from the
|
||
old tracker's comment log by the #524 triage. `docs/e2e-local.md` documents the ready line and
|
||
recommends a fresh dir, but not this failure mode. Widening the probe to
|
||
`Done migrating search index|Search index is already version` is tracked as **#533**.
|
||
|
||
## 2026-07-21 — Channel branding is NOT testable through the troubleshooting-playback API (#542)
|
||
|
||
`key: testing.troubleshoot-path-cannot-test-branding` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding.
|
||
**Signals:** `PrepareTroubleshootingPlaybackHandler` · synthetic channel · `Artwork = []` · `ChannelId > 0` branch · watermark · channel logo · bug overlay · paths: `ErsatzTV.Application` troubleshooting handlers · issues: #542
|
||
**Mechanics:** n/a — a property of the handler, not a toggle.
|
||
|
||
`PrepareTroubleshootingPlaybackHandler` builds a **synthetic** channel with `Artwork = []`, and its
|
||
`ChannelId > 0` branch ignores the media item. Both together mean no branding state ever reaches the
|
||
pipeline through that path.
|
||
|
||
So the troubleshoot endpoint is a false witness for this class: it exercises the transcode pipeline but
|
||
not the overlay inputs. Route branding verification through a real channel playout instead.
|
||
|
||
## 2026-07-21 — Playwright-MCP: curl download endpoints, never open a tab or `window.open` (#542)
|
||
|
||
`key: testing.playwright-mcp-download-and-recovery` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session.
|
||
**Signals:** Playwright MCP · file download endpoint · window.open · browser tab stall · `ms-playwright-mcp` · paths: n/a · issues: #542
|
||
**Mechanics:** `pkill -f ms-playwright-mcp` then re-drive from a fresh MCP session.
|
||
|
||
Download endpoints hand the browser a file rather than a page; the tab has nothing to snapshot and the
|
||
tool waits on navigation that never completes. curl gives you the bytes and the status code directly.
|
||
|
||
Repeated stalls are a wedged MCP browser process, not a slow app — kill it and restart rather than
|
||
retrying the same call.
|
||
|
||
## 2026-07-21 — Launch Codex cheap workers via `codex exec`, not `spawn_agent` (#542)
|
||
|
||
`key: process.codex-cheap-worker-launch` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** For bounded tool-bearing selector/recon work, launch a Codex worker with `codex exec -m gpt-5.4-mini -c model_reasoning_effort=low -s read-only`; `spawn_agent` buys parallelism but no cost savings.
|
||
**Signals:** codex exec · `--ephemeral` · `spawn_agent` · `gpt-5.4-mini` · `gpt-5.1-codex-mini` HTTP 400 · `model_reasoning_effort` · `~/.codex/models_cache.json` · paths: `~/.codex/models_cache.json` · issues: #542
|
||
**Mechanics:** Local `codex` CLI on this ChatGPT-authenticated host. Tested 2026-07-14.
|
||
|
||
The native `spawn_agent` interface currently exposes no model or effort selector, so it parallelises at
|
||
full cost. Launch a separate worker instead:
|
||
|
||
```
|
||
codex exec --ephemeral --color never -m gpt-5.4-mini -c model_reasoning_effort=low -s read-only -C <repo> "<hard-capped scope contract>"
|
||
```
|
||
|
||
GPT-5.4-Mini is the current supported small/cost-efficient profile on this host; the older
|
||
`gpt-5.1-codex-mini` guess fails with **HTTP 400**. Keep `low` whenever any shell/MCP/web tool is
|
||
exposed; reserve `minimal` for explicitly tool-free synthesis over already-supplied bounded evidence.
|
||
On model rollover, inspect the visible entries in `~/.codex/models_cache.json` rather than guessing
|
||
names, then update this record once verified.
|
||
|
||
## 2026-07-21 — Live-E2E runs BEFORE the push, and downloads are curled, not browsed (#542)
|
||
|
||
`key: testing.live-e2e-prepush-timing` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Run live-E2E via `scripts/e2e-local.sh` before pushing a write-path or UI change, and exercise download endpoints with curl, never a browser tab.
|
||
**Signals:** live E2E timing · pre-push validation · `scripts/e2e-local.sh` · download endpoint · browser tab · paths: `scripts/e2e-local.sh`, `docs/e2e-local.md` · issues: #542
|
||
**Mechanics:** Seeding recipe: `docs/e2e-local.md` → "Seeding a local TV library for E2E" (tiny ffmpeg testsrc MKVs + `LibraryPath` SQL rows + scan). The *requirement* itself is `release.live-e2e-required`; this record adds only the timing and the download rule.
|
||
|
||
Live-E2E after the push wastes a CI run to learn something the local harness would have said first —
|
||
and on this Gitea a run cannot be cancelled. Fold E2E findings into the branch while nothing is pushed.
|
||
|
||
Download endpoints must be curled for the same reason as the Playwright rule: a tab has no page to
|
||
settle on and the browser tool hangs.
|
||
|
||
## 2026-07-21 — The PR routine is a fixed sequence: validate locally, then push, then only follow-up commits (#542)
|
||
|
||
`key: process.pr-routine-sequence` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Worktree off origin/main → implement → regenerate API artifacts → full local tests + cold review + live-E2E ALL before the push → push, open PR, arm the CI monitor at open → fixes after the push are follow-up commits, never amend/force-push.
|
||
**Signals:** PR routine · worktree off origin/main · `./scripts/update-openapi.sh` · `npm run generate:api` · cold-context review · CI monitor at PR open · no force-push on a pushed branch · derived-consent merge gate · paths: `scripts/update-openapi.sh`, `scripts/e2e-local.sh` · issues: #542
|
||
**Mechanics:** CI watch by commit status: `/api/v1/repos/timothy/ersatztv/commits/{sha}/status`. Merge via the derived-consent gate (`pretooluse-merge-consent.sh`).
|
||
|
||
The sequence: create a worktree off `origin/main`; implement; for **API changes build the app project
|
||
FIRST**, then `./scripts/update-openapi.sh`, then `npm run generate:api` (order matters — the generator
|
||
reads the built app's spec). Then run the full local test pass, a cold-context adversarial review
|
||
scoped to "review only", and live-E2E for UI/write-path changes — **all before the push**. Fold
|
||
pre-push fixes into the branch; amend/reword is fine while nothing is pushed.
|
||
|
||
Then push, open the PR, and arm the CI monitor **at open**, not at the end. Once the branch is pushed,
|
||
every further fix — a post-push cross-model pass, or a CI-surfaced issue — lands as a **follow-up
|
||
commit**; never amend or force-push a pushed branch. On green, merge through the derived-consent gate
|
||
and write the structured close comments the applicable `AGENTS.md` / `CLAUDE.md` protocol requires.
|
||
|
||
## 2026-07-21 — Harden a soon-to-grow feature with a runtime posture, not a hardcoded clamp (#542)
|
||
|
||
`key: process.harden-with-runtime-posture-not-clamp` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** When a security fix constrains a capability the roadmap will later want, make the safe state the DEFAULT OF A SWITCH rather than a wall — and read the feature's own issue for its end-state first.
|
||
**Signals:** runtime posture · feature toggle · `ERSATZTV_ALLOW_WRITES` · read-only default · hardcoded clamp · roadmap end-state · paths: n/a · issues: #542, #289, #58
|
||
**Mechanics:** `ERSATZTV_ALLOW_WRITES` env var, default `false`. See `mcp.server-foundation`.
|
||
|
||
The MCP hardening in #289 could have shipped a GET-only clamp. But the MCP's final design (#58) is read
|
||
**and** write, so that clamp would have been ripped out within a release — churn plus a window where
|
||
the removal, not the original fix, is the risky change.
|
||
|
||
A runtime read-only posture (`ERSATZTV_ALLOW_WRITES`, default false) gives the same safety today and
|
||
grows into the intended design without a revert. The generalisable step is the cheap one: check the
|
||
feature's issue for its end-state *before* choosing between a switch and a wall.
|
||
|
||
## 2026-07-21 — Gitea's `?milestones=` issue filter silently no-ops on names containing `:` or `+` (#542)
|
||
|
||
`key: ci.gitea-milestone-filter-noop` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** Never filter issues with the server-side `?milestones=<name>` parameter — fetch all open issues once and filter LOCALLY on each issue's `.milestone.title`.
|
||
**Signals:** `GET /issues?milestones=` · milestone name filter · returns the whole open-issue list · `:` or `+` in a milestone title · mis-tiered issues · count looks like "all open issues" · paths: `scripts/select-queue.sh` · issues: #542, #77, #72
|
||
**Mechanics:** `?state=open&type=issues&limit=50`, then filter in the client, e.g. `[i for i in issues if (i['milestone'] or {}).get('title') == NAME]`. `scripts/select-queue.sh` already does this; the trap is for anyone writing a *new* query.
|
||
|
||
The filter does not error on a name it cannot round-trip — it returns the **entire open-issue list** as
|
||
though every issue matched. Milestone titles here routinely contain the offending characters (e.g.
|
||
`Scheduling: refactor + distribution`), so the failure is the normal case, not an edge case.
|
||
|
||
Anything trusting that response mis-tiers issues: during #77 selection it made unmilestoned #72 look
|
||
like a member of the milestone and hid #77's true sibling set. **The tell is a result count suspiciously
|
||
equal to the total number of open issues** — if a filtered query returns everything, the filter silently
|
||
failed; re-derive membership locally rather than reasoning about the result.
|
||
|
||
The dependencies API is unaffected: `POST /issues/{n}/dependencies` with `{"owner","repo","index"}` sets
|
||
blocked-by correctly, though the bare `{index}` form returns 201 without reliably attaching — verify
|
||
with the GET. (That last sentence is carried forward from an earlier revision of the handoff doc,
|
||
commit `f93458c7`, where it was dropped by a later prune rather than disproved.)
|
||
|
||
## 2026-07-25 — Name the model tier for every dispatched agent; a PreToolUse gate makes the silent default visible (#583)
|
||
|
||
`key: process.per-agent-model-routing` · `status: active` · `since: 2026-07-25` · `supersedes: none` · `superseded-by: none`
|
||
**Rule:** State the model tier (and effort, where the client exposes it) in the dispatch itself for every delegated agent — bounded recon → cheapest fast tier at `low`; mechanical slice against a documented contract → mid tier; judgment-heavy work → orchestrator tier; independent review → a different model family than the implementer.
|
||
**Signals:** subagent model routing · `model` omitted · silent tier inheritance · orchestrator tier for a mechanical slice · capability routing · prose rule vs HARD CONSTRAINT · dispatch-time checkpoint · paths: `.claude/hooks/pretooluse-agent-model.sh`, `docs/handoffs/chicorytv-issue-queue.md` · issues: #583, #436, #440
|
||
**Mechanics:** `Agent` tool `model` parameter; PreToolUse hook on the existing `Agent|Task` matcher in `.claude/settings.json`.
|
||
|
||
On 2026-07-25 a session dispatched two implementers (#436, #440) with `model` omitted on both calls;
|
||
both silently inherited the Opus orchestrator tier. #440 was a mechanical SPA slice against an
|
||
already-shipped backend contract — a plausible mid-tier candidate.
|
||
|
||
The interesting part is *why*, because it wasn't forgetfulness. **Every rule in `HARD CONSTRAINTS` was
|
||
followed in that same session** — worktree off `origin/main`, one committing agent per worktree,
|
||
parallelize on disjoint slices, local gate before push. Routing was the one instruction living only in
|
||
a prose paragraph, and it was the one that got defaulted. Treat that as the general lesson: in a
|
||
kickoff doc that is pasted into every session, **the bulleted imperative list is what actually
|
||
functions as the checklist**, and prose above it is read as background. A rule you want followed
|
||
belongs in the list, keyed, or it is advisory in practice.
|
||
|
||
Three aggravating factors, all worth checking when writing any future rule here:
|
||
|
||
- **Scope gap.** The low-cost-routing paragraph is written entirely about queue selection and recon
|
||
("bounded searches, inventories, log triage, report drafting"). It never named *implementers*, and
|
||
gave no default for the bounded-but-not-trivial case — so the largest-cost dispatch fell in a gap.
|
||
- **The wrong default is the silent one.** Omitting `model` produces no artifact. Nothing in the
|
||
session report revealed the tier; the operator had to ask. Contrast the BOM trap
|
||
(`process.bom-format-detection-recipe`), where a hook fires because a memory describing the trap
|
||
demonstrably failed to prevent it twice in one day.
|
||
- **Distance from the decision point.** The rule sits ~line 84 of the kickoff; dispatch happens after
|
||
orientation, claiming, the bundle scan and doc reading.
|
||
|
||
Hence the two-part fix: a keyed HARD CONSTRAINT that requires the tier to be **stated out loud in the
|
||
dispatch** (a self-correcting mechanism — it turns an invisible omission into visible output), plus
|
||
`.claude/hooks/pretooluse-agent-model.sh`, which `ask`s whenever an agent is dispatched with no
|
||
explicit `model`. It exempts only `fork`, whose `model` override the tool ignores by design, so a
|
||
prompt there could not be acted on. It is `ask`, never `deny`: routing is a judgment call with no
|
||
derivable right answer, unlike the H6/H10 merge gate (`release.merge-consent-autogrant`), which
|
||
derives a verifiable state and can therefore grant or refuse outright.
|
||
|
||
**The first cut was narrower, and review killed it** — worth recording, because the reasoning was
|
||
seductive. It fired only when the prompt *text* matched implementer signals (`git commit`, `worktree`,
|
||
`fixes #`), on the theory that a gate firing on every fan-out trains one-shot dismissal. Independent
|
||
review confirmed the heuristic both over- and under-fired: a read-only recon brief merely *mentioning*
|
||
"worktree" nagged, while "author the change and open a PR", "land this on the branch" and "make the
|
||
changes and commit them" all passed silently — it missed precisely the case it existed to catch.
|
||
**Prompt prose is not a reliable signal for authority**, and a gate with an unreliable catch rate is
|
||
worse than none, because it gets trusted.
|
||
|
||
Two further reasons the broad form is right, both of which the narrow version had backwards:
|
||
|
||
- **It now matches the rule it enforces.** The HARD CONSTRAINT says "every dispatched agent"; a hook
|
||
gating only implementer-looking dispatches contradicted its own rule.
|
||
- **Routing matters MOST for the cheap cases.** The old exemption list justified itself as
|
||
"read-only, so routing barely matters" — but bounded recon is exactly what should be explicitly
|
||
routed *down* to a fast tier. The premise was also false: `Explore`, `Plan` and `claude-code-guide`
|
||
all carry Bash, so none of them provably "cannot commit".
|
||
|
||
The noise objection is answered by the escape hatch rather than by scoping: **naming a tier costs one
|
||
parameter and the hook never fires again**. The prompt is self-eliminating for anyone following the
|
||
rule, which is the habit being built.
|