From fe6e2722f8d032a978c206ff469f33418a955333 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 14:17:52 +0200 Subject: [PATCH] =?UTF-8?q?docs(lore):=20batching,=20no=20"main=20checkout?= =?UTF-8?q?",=20trust=20the=20queue,=20killed=E2=89=A0failed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator-requested after PR #405 pushed 5 times, orphaning live runs the operator had to cancel by hand. Batches every standing-lore correction this session produced into one commit (per the batching rule it adds). New HARD CONSTRAINTS: - BATCH PUSHES. Cancellation is impossible from the agent side on Gitea 1.25.4 — REST .../runs/{id}/cancel and MCP cancel_run both 404, and the web-UI route needs a session+CSRF that doesn't script. Only the operator can cancel, so an orphaned run holds a runner slot until it finishes. If you must supersede a live run, SAY SO. - TRUST THE GITEA BUILD QUEUE. Do not gate/throttle a push on host health; the runners were retuned for stability. Batch because you can't cancel what you orphan, not to protect the host. - BOM-CHECK touched .cs before pushing. The #311 gate is fix-as-you-touch, and it bit two sessions the same day (PR #405 ×6; #70/PR #402 ×19 via Python utf-8-sig writing BOMs back). Verify your detector — an od-based grep reported all-clean while 19 files were dirty. Corrects a claim I nearly published: `dotnet format --include` DOES work here. The apparent no-op was the SHELL — CI's mapfile is bash-only, zsh has no mapfile → empty array → zero files → exit 0. Run it under bash -c. THERE IS NO "main checkout" — the biggest correction here. /Users/timothy/ersatztv is a shared mutable working tree whose HEAD is whatever the last session left there. Its name lies, and it bit TWO sessions on 2026-07-17, both doing the obvious thing: one assumed main and committed onto the #604/CI session's branch 24s after that session's own commit; another read git log there and concluded main was "4 behind origin" — a phantom. Framed as a design flaw, not a discipline failure: "check git status first" appears to confirm the false assumption and then goes stale (it WAS on main at 12:46 and wasn't by 14:17). Worktree discipline itself is healthy — 10 feature worktrees. Diagnosing CI reds — three ways to misread one, all hit this session: - A KILLED job reports conclusion=failure, not cancelled. The tell is a log that stops mid-step with NO error and NO failure marker. A runner retune killed run 1006's migration + E2E on a BOM-only diff that couldn't break them. Log timestamps are UTC, host is UTC+2 — convert before correlating. - cancelled ≠ failure: a cancel is NO verdict, and a run marked failure may hold a genuine job failure from BEFORE the cancel. Monitors must count FAILED and CANCELLED separately. - "Unable to pull refs/heads/v4" is act refreshing its action cache and is followed by "Cloned …" — noise, not a cause. Grep the failure marker, not the word "error". An infra-shaped red (setup/cache step, before your code compiles) is not a code failure; don't file a CI bug off one sample. Also: the cheap selector's failure modes are wider than deps+priority — it also misses in-progress claim state and umbrella-vs-child. Docs-only. --- docs/handoffs/chicorytv-issue-queue.md | 126 ++++++++++++++++++++++++- 1 file changed, 121 insertions(+), 5 deletions(-) diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index 5affbb953..e8ccc1373 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -215,7 +215,11 @@ Then work the queue: applicable `AGENTS.md` / `CLAUDE.md` instructions (including the `done` workflow when required). HARD CONSTRAINTS: -- Work in worktrees off origin/main. Copy web/node_modules from the main checkout. +- Work in worktrees off `origin/main` — ALWAYS, including for a one-line docs commit. `/Users/timothy/ersatztv` + is a **shared mutable tree, not a `main` mirror**: never commit in it, and never read its HEAD/`git log` + to learn anything about `main` (you'll get another session's branch). Its only sanctioned use is + **read-only**: copying `web/node_modules` out of it. `origin/main` is the source of truth — `git fetch + origin main` and reference it explicitly. See the lore bullet for the two sessions this bit on 2026-07-17. - PARALLELIZE BY DEFAULT when there are genuinely disjoint slices: 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. At most 3–4 concurrent @@ -226,6 +230,45 @@ HARD CONSTRAINTS: - Never two committing agents on ONE worktree — give each parallel slice its own worktree branched off the feature branch and merge back. Sequence only where a slice genuinely depends on another's output (backend-first narrow, SPA-wide after worked well for #216). +- **BATCH YOUR PUSHES — you cannot cancel a CI run** (operator-requested, 2026-07-17). Hold review + fixes, doc corrections and format fixes locally, verify, and push **once**; each push spawns a run + that orphans the previous one on a 4-slot runner. Cancellation is impossible from the agent side on + this Gitea (**1.25.4**): `POST /api/v1/.../actions/runs/{id}/cancel` → **404**, MCP + `actions_run_write` `cancel_run` → **404**, and the web-UI route needs a session+CSRF that does not + script (login 303s, 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.) Corrects the older "superseded + runs drain on their own" note below: they do finish, but they hold a slot while doing it. +- **Do NOT gate or throttle your push on host health — trust the Gitea build queue.** The runners were + retuned for stability (operator, 2026-07-17); queueing is the queue's job, not yours. Don't SSH to + bumblebee to sample load/RAM before pushing, and don't hand-schedule around other sessions' runs. Batch + your pushes (above) because orphaned runs can't be cancelled — not because the host needs protecting. +- **An infra-shaped red under host pressure is not a code failure — check the host before diagnosing the + diff** (2026-07-17). While bumblebee was saturated, two unrelated jobs died in ways that look like code + bugs: `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 once the host recovered; the migration job had no model change to test in the + first place. Tell: the failure is in a *setup/cache* step, before your code compiles. Don't file a CI bug + off one sample under pressure — you'll send the next session chasing a phantom (this nearly got filed as + #390 lane-rebalance fallout, which the evidence did not support). +- **Before any push touching `.cs`, check the touched set for a UTF-8 BOM.** The #311 gate is + fix-as-you-touch: a legacy file's inherited BOM becomes *your* problem the moment you touch it, and + `charset=utf-8` means no BOM. + `for f in $(git diff --name-only origin/main...HEAD -- '*.cs'); do head -c3 "$f" | xxd -p | grep -q "^efbbbf" && echo "BOM: $f"; done` + Strip any hit (1-line diff, zero content change). Bit **two sessions on 2026-07-17** — PR #405 (6 + files) and #70/PR #402 (19 files, where Python `io.open(..., encoding='utf-8-sig')` wrote BOMs back + while "preserving existing style"). **Verify your detector**: an `od -An -c | grep '357 273 277'` + check reported all-clean while 19 files were dirty — a detector that can only say "ok" is worse than + none. Use the `xxd` form or read bytes directly. + **`dotnet format --include` DOES work here** (corrected 2026-07-17, #70/PR #402 — 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. Run it under `bash -c`: + ```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 + ``` - Merge consent is DERIVED, not asked: the H6/H10 gate (`pretooluse-merge-consent.sh`) auto-grants a merge when the linked issue's `## Done-when` boxes are all ticked, a fresh positive `Review-verdict` references the current head, and CI is green — no conversational confirmation on that satisfied path @@ -260,6 +303,17 @@ HARD CONSTRAINTS: filter silently failed; re-derive membership locally. (Dependencies API is fine: `POST /issues/{n}/dependencies` with `{"owner","repo","index"}` sets blocked-by; the bare `{index}` form 201s but may not attach — verify with the GET.) +- **Confirmed again + widened (2026-07-17, #72/#164 selection): treat EVERY mechanical claim in the + selector packet as unverified — not just deps and priority.** That run's dependency data was + correct, yet it still ranked **#383 first while missing that #384 was `in-progress`** (claimed 40 + minutes earlier) **and that #383 is that bundle's umbrella** (its body: "Follow-up arc to #69" with + #384–#386 as deferred sub-work). Claiming its #1 pick would have collided head-on with a live + parallel session. The packet does not reliably see: `blocked-by`, priority-within-tier, + **`in-progress` claim state**, or umbrella-vs-child. Before claiming, check the candidate's **last + comments** for a live claim (a claim stays live until explicitly released) and read its **body** for + umbrella framing. An empty-ish tier is normal in maintenance mode — three of the four open + milestone issues that day were legitimately blocked or claimed, which is the expected steady state, + not a reason to relax the checks. - **The cheap selector's dependency + priority data is NOT trustworthy — re-derive the whole contested tier, not just the winner (2026-07-17, #73 selection)**: the selector packet reported **#385/#386 as `deps:clear` when both were blocked** (#385 by open #70; #386 by open #385+#384) — it never called @@ -286,8 +340,33 @@ HARD CONSTRAINTS: adds a dep) — worktrees copy it, and a stale copy broke typecheck in a #198 worktree. - **Playwright-MCP E2E: never open tabs/window.open for file-download endpoints** — curl them. If browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. -- The user's main checkout (/Users/timothy/ersatztv) stays ON main and pulled each session. - Feature work in worktrees off origin/main; copy `web/node_modules` from the main checkout. +- **THERE IS NO "main checkout". `/Users/timothy/ersatztv` is a shared mutable working tree whose HEAD + is whatever the last session left there — it is NOT a `main` mirror, and its name lies.** Treat it as + **read-only, and never as a source of truth**: + - **Never commit in it** — not even a one-line docs fix. Need a commit on `main`? Make a worktree off + `origin/main` (`git worktree add -b origin/main`) and commit there. + - **Never read `git log` / `git status` / `HEAD` there and infer anything about `main`** — you will be + reading some other session's branch. + - **`origin/main` is the only source of truth.** `git fetch origin main` and reference `origin/main` + explicitly. Do not trust the local `main` ref either (it lags; nobody is obliged to update it). + - Its **one sanctioned use is read-only**: copying `web/node_modules` into your worktree (keep it + fresh — a stale copy broke typecheck in a #198 worktree). + + **This is a design flaw, not a discipline failure — do not "fix" it by telling sessions to check + harder.** A check does not stay true. Worktree discipline is healthy (10 feature worktrees on + 2026-07-17: Claude under `.claude/worktrees/`, Codex under `.worktrees/`, some in `/private/tmp/`); + the shared tree is the single place the model breaks, and it broke for **two different sessions on the + same day, both doing the obvious thing**: + - Session A assumed it was on `main` (true at 12:46 when it pulled — **correct when made**, stale by + 14:17) and committed → the commit landed on the #604/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 `ci/604-lane-rebalance`'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. (Supersedes the old "the user's main checkout stays ON main and pulled each + session" note.) Same family as the sibling bullet below: never commit inside a checkout or worktree + you didn't create. - Subagent connection drops = laptop sleep/transient; re-resume via SendMessage (work survives). - Build/verify locally, then trust it; CI (VM 127) confirms (~9 min warm). Watch by commit status: `/api/v1/repos/timothy/ersatztv/commits/{sha}/status`. @@ -300,6 +379,40 @@ HARD CONSTRAINTS: green → merge (derived consent gate) → structured close comments per the applicable `AGENTS.md` / `CLAUDE.md` protocol. - Never run TWO committing agents on one worktree concurrently. Read-only review agents are fine. +- **A KILLED job reports `conclusion: failure` — the conclusion field is not enough. Read the log tail.** + (2026-07-17.) When the operator restarts the runners (a retune, a reboot), in-flight jobs die and Gitea + marks them **`failure`**, not `cancelled`. Tell: the log **stops mid-step with no error and no + `❌ Failure - Main …` marker** — a real failure always leaves a marker. Run 1006's `EF migration + integrity` + `Functional E2E` "failed" on a **BOM-removal-only** diff (6 files, 1 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 2h off and + the 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, don't re-litigate the diff. Also beware log **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". +- **`cancelled` is NOT `failure` — never read a cancel as a CI verdict** (2026-07-17, operator-surfaced). + The operator cancels runs by hand (they're the only one who can — see the batching constraint), and a + run-level `conclusion: cancelled` means **no verdict**, not a pass and not a fail. Two traps: (1) a + run whose overall state is `failure` may hold a *genuine* job failure that happened **before** the + cancel — check job-level `conclusion` + timestamps, don't attribute it to the cancel; (2) a + `cancelled` run tells you nothing about your diff, so don't claim green on it. Report FAILED and + CANCELLED counts **separately** in any CI monitor: + `fail=[j for j in jobs if j['conclusion']=='failure']; canc=[j for j in jobs if j['conclusion']=='cancelled']` + — a monitor that only asks "is state != pending" will call a cancelled run a failure and send you + debugging a phantom. +- **An infra-shaped red under host load is not a code failure — check `uptime` on bumblebee BEFORE + diagnosing the diff** (2026-07-17). At load average **243**, two unrelated jobs died in ways that + look like code bugs but aren'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; the migration job had no model change to test in + the first place. Tell: the failure is in a *setup/cache* step, before your code compiles. Don't file + a CI bug off one sample under load — you'll send the next session chasing a phantom (this nearly got + filed as #390 lane-rebalance fallout, which the evidence did not support). A game server + (`FactoryServer-L`) shares that host with the runners, so load isn't always CI's doing. - **Arm a CI monitor the moment the PR opens** (commit-status endpoint), not at the end — in the #216 session three PR runs sat red ~an hour on a CI-only flake while review/E2E ran. (The old MySQL host-port 3306 collision is fixed on main, `ef8915f1`; a lone MySQL-apply red now = the @@ -316,8 +429,11 @@ HARD CONSTRAINTS: duplicate pickup, NOT overlapping code changes — check the tracker's dependency notes ("#234 after #231", "coordinate with #215") before touching shared surfaces. Editing THIS lore: prune covered/stale bullets too (not append-only — git keeps history), `git pull --rebase` - before commit. Two runners (ci-runner VM 127 + bumblebee-runner, 4 slots) → superseded runs - drain on their own; no cancel route exists on this Gitea version. + before commit. Two runners (ci-runner VM 127 + bumblebee-runner, 4 slots); **no cancel route exists + on this Gitea version — verified 1.25.4: REST and MCP `cancel_run` both 404** (see the batching + constraint above). Superseded runs do finish on their own, but they hold a runner slot while doing + it, so with 4 parallel sessions they are a real cost — batch pushes rather than relying on them to + drain. - **Two sessions touching one machine**: a branch may be checked out in ANOTHER session's worktree — never commit/merge inside a worktree you didn't create. To land a merge on such a branch without touching their checkout: plumbing merge (`git read-tree -m base ours theirs` -- 2.47.3