Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m17s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 10m3s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 7m3s
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
232 lines
19 KiB
Markdown
232 lines
19 KiB
Markdown
# ChicoryTV issue-queue handoff (static kickoff + workflow lore)
|
||
|
||
> **PROTOCOL CHANGED 2026-07-11** (decisions.md entry of same date). Queue state lives in the
|
||
> **pinned Gitea tracker [ersatztv#237](http://192.168.1.95:3000/timothy/ersatztv/issues/237)**,
|
||
> not in this file. Do **NOT** write session state, queue order, or next-session prompts here —
|
||
> sessions ending under the old protocol should append their session comment to #237 instead.
|
||
> This file holds only the two stable things: the standing kickoff prompt and the workflow lore.
|
||
> Historical per-session state: `git log` of this file (last state-bearing revision: 8b77d5e7).
|
||
|
||
---
|
||
|
||
# STANDING KICKOFF PROMPT (paste into a fresh session, unchanged every time)
|
||
|
||
You are the ORCHESTRATOR in the main Claude Code session — **Opus 4.8 by default** (Fable only
|
||
when the user picked it; check which model you are). Orchestration = delegation: recon →
|
||
Explore/haiku; mechanical → sonnet; judgment-heavy implementation → opus; independent review →
|
||
Codex when quota allows (`codex:rescue`) + Claude review subagents/forks.
|
||
FIRST read CLAUDE.md, docs/README.md + the convention docs it indexes, and the Lessons below.
|
||
|
||
FABLE ESCALATION — these moments go to Fable, no exceptions. If you ARE Fable, handle them
|
||
inline (forks inherit your context); otherwise spawn an `Agent` with `model: "fable"` and a
|
||
SELF-CONTAINED brief (scope, exact question, file paths, doc pointers — it starts cold), or
|
||
pause and ask the user to `/model`-flip for the stretch:
|
||
- Architecture/API design decisions (e.g. #202's media-source API shape) and anything
|
||
`[PLAN-MODE]`-tagged.
|
||
- Parity verdicts for blazor-route-parity.md (per-capability diff sign-off) and #91b
|
||
removal go/no-go.
|
||
- Reconciling contradictory review findings (fork vs Codex disagree) and the final
|
||
pre-merge judgment on gate PRs.
|
||
- Stuck after two genuinely different approaches, or evidence contradicts the issue/docs.
|
||
Everything else (claiming, worktrees, dispatching implementers, CI monitoring, protocol
|
||
bookkeeping, routine merges of green reviewed PRs with user consent) stays at your level.
|
||
|
||
Then work the queue:
|
||
1. Read the pinned tracker **ersatztv#237** — body = goal + ordered arc + session protocol —
|
||
and its most recent session comments; list open issues in the `Blazor removal (#91 phase b)`
|
||
milestone and with the `review` label; ALSO list open `ersatztv`-labeled issues in
|
||
**timothy/adversarial-reviewer** — unclaimed audits there are pickup candidates too
|
||
(read-only, parallel-safe; see the tracker's "Pending adversarial reviews" section).
|
||
2. Pick the top arc item that is open and NOT labeled `in-progress` (or the item the user names).
|
||
3. **Claim it**: add the `in-progress` label + a "claiming" comment on the issue(s);
|
||
reviewer-repo audits are claimed by comment only.
|
||
4. Read the issue bodies (they carry the task context/evidence) and work the item under the
|
||
HARD CONSTRAINTS below.
|
||
5. Finish by following the session-end protocol in #237: ONE session comment on the tracker
|
||
(template in the tracker body, incl. triage verdicts for any new issues), remove your
|
||
`in-progress` labels, and complete the per-issue Task Completion Protocol from CLAUDE.md.
|
||
|
||
HARD CONSTRAINTS:
|
||
- Work in worktrees off origin/main. Copy web/node_modules from the main checkout.
|
||
- PARALLELIZE BY DEFAULT: split the task into disjoint slices up front and run 3–4 implementer
|
||
agents concurrently (recon agents are free — always fan those out). 3–4 concurrent
|
||
dotnet/npm builds are fine on this Mac (M4, 10 cores, 16 GB); the go/no-go signal is FREE
|
||
RAM, not CPU load (`memory_pressure -Q`: <20% free → don't launch more build agents; <10% →
|
||
pause/stagger). CPU load spikes during builds are benign. Never 5+ builds (the historic
|
||
crash was RAM starvation from an 8–9-way fan-out). NEVER set ETV_UPDATE_GOLDENS.
|
||
- 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).
|
||
- Merge consent in-conversation per session (prior pre-approvals do NOT carry over).
|
||
- Arm a CI monitor on the PR head sha AT PR-OPEN (commit-status endpoint), not at the end.
|
||
- Live-E2E via scripts/e2e-local.sh; NEVER exercise download endpoints via browser tabs (curl
|
||
them). Adversarial review fork per PR diff, SCOPED "review only".
|
||
|
||
---
|
||
|
||
# Lessons / workflow lore (append-only; conventions live in docs/, this is workflow lore)
|
||
|
||
- READ docs/README.md → the convention docs FIRST; point recon/implementer agents at specific
|
||
doc sections. Only recon the task-specific delta.
|
||
- **blazor-route-parity.md carries mutation-depth verdicts** (2026-07-09 sweep table at the
|
||
top of Section 2). A row is only trustworthy if its cluster verdict is PARITY-OK; the sweep
|
||
evidence lives in the issues #207–#213. Keep the verdict table updated as gates close.
|
||
- "Screen exists" ≠ parity: the root cause of the false SPA-READY rows was same-session
|
||
verification that never diffed capabilities against the Blazor page. Per-capability diff vs
|
||
the Blazor page is the standard for any future parity claim.
|
||
- Several gaps are UNWIRED EXISTING endpoints (playout delete/reset, block copy, scan-show,
|
||
collections add-item kinds) — check the endpoint index before building new API surface.
|
||
- Keep the MAIN checkout's `web/node_modules` fresh (`npm install` after pulling a PR that
|
||
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.
|
||
- 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`.
|
||
- PR routine (works): worktree off origin/main → implement (opus judgment-heavy / sonnet
|
||
mechanical; doc pointers + exact facts; csharp-lsp for C#) → API changes: build app project
|
||
FIRST then `./scripts/update-openapi.sh` then `npm run generate:api` → full local test pass
|
||
→ push, PR → fork adversarial review (SCOPED "review only") + live-E2E for UI changes
|
||
(`scripts/e2e-local.sh`) → review fixes as follow-up COMMITS (never amend/force-push) → CI
|
||
green → merge (consent per session) → structured close comments per CLAUDE.md protocol.
|
||
- Never run TWO committing agents on one worktree concurrently. Read-only review forks are fine.
|
||
- Reusable API facts: DTO records in ErsatzTV.Core/Api need file-scoped `#nullable enable`;
|
||
ErsatzTV.Application has NO nullable context (CS8632); `Option<T>`→nullable =
|
||
`MatchUnsafe(v => (T?)v, () => null)`; `LanguageExtensions.Apply` collapses NotFoundError→422,
|
||
so real 404s need a controller pre-check; id-taking child GETs get an
|
||
OpenApiErrorResponseContractTests [TestCase].
|
||
- **Arm a CI monitor the moment the PR opens** — in the #216 session three PR runs sat red for
|
||
~an hour (a CI-only debounce-timing flake) while review/E2E ran, because CI was only checked
|
||
"at the end". Also: concurrent runs can collide on the MySQL service host port 3306
|
||
("port is already allocated") — that's infra flake, rerun; consider unpinning the host port.
|
||
- **"Make X consistent with Y" review findings deserve new-code scrutiny** — a #216 review fix
|
||
propagated MediaBrowseScreen's mount-debounce selection-wipe race to SearchScreen instead of
|
||
catching it. For any timer/effect, ask "when does this fire?" (incl. mount).
|
||
- SPA mutation affordances on media items: USE the shared `web/src/media/addTo/` layer
|
||
(spa-conventions §5c) — don't build screen-local pickers. Schedule adds are valid only for
|
||
shows/seasons/artists (server validator).
|
||
- Live E2E seeding: the local library isn't API-seedable; the #216 E2E agent generated tiny
|
||
ffmpeg testsrc MKVs + inserted LibraryPath rows via SQL then scanned. Recipe not yet in
|
||
docs/e2e-local.md — worth adding next time it's needed.
|
||
- **Parallel sessions (2026-07-11 protocol)**: claim before working (`in-progress` label — the
|
||
tiny read→claim race window is accepted; later claimant backs off). Claiming prevents
|
||
duplicate pickup, NOT overlapping code changes — check the tracker's dependency notes
|
||
("#234 after #231", "coordinate with #215") before touching shared surfaces. Lessons edits
|
||
to THIS file: append bullets only, `git pull --rebase` before commit.
|
||
- **Fan-out health = RAM, not CPU load** (2026-07-11, user calibration): the 8–9-way crash was
|
||
RAM starvation. `uptime` load of 10–24 during parallel Roslyn/vitest bursts is benign with
|
||
memory healthy. Watchdog pattern: background monitor emitting only when `memory_pressure -Q`
|
||
free % < 20 (silence = healthy); pause/stagger agents below 10%.
|
||
- **CI MySQL host-port collision FIXED on main** (`ef8915f1`, issue #236): the migrations
|
||
service no longer publishes host 3306. Branches created before that commit still collide —
|
||
merge main in. There are now TWO runners (ci-runner VM 127 + bumblebee-runner), 4 slots:
|
||
faster drains, and superseded-run results are ignorable. Neither the cancel-run API route
|
||
nor the web cancel route exists on this Gitea version — stale runs just 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`
|
||
into a temp GIT_INDEX_FILE → `write-tree` → `commit-tree -p ours -p theirs` → push the
|
||
commit to the branch ref); the owning session then `git pull`s.
|
||
- **Codex is back in the review rotation** (2026-07-11): the 2026-07-06 "retired — usage
|
||
exhausted" was a quota window, not retirement. Codex has 5-hourly + weekly limits (same
|
||
structure as Claude); rule: **if we have usage, use it** — for INDEPENDENT review passes
|
||
(per-PR adversarial review alongside the fork review, adversarial-reviewer audits/re-reviews,
|
||
stuck-diagnosis second opinions) via `codex:rescue`, never for implementation. Check
|
||
readiness/quota with `/codex:setup`; on exhaustion fall back to fork/subagent review and
|
||
retry next window.
|
||
- **Fixed-point round-trip tests can't see lazy-evaluation crashes** (2026-07-11, PR #229): a
|
||
write-path 500 (unset navs projected after SaveChanges) survived a green fixed-point test
|
||
because the handler returned a lazy LanguageExt `Map` the test never enumerated. Rules:
|
||
force enumeration of handler return values in tests; write-path responses reload through the
|
||
read path's include chain (api-conventions §7); live E2E remains the only net for this class.
|
||
- **Merging main into an open PR branch: REGENERATE generated artifacts** (v1.json, v1.d.ts,
|
||
endpoint-index) after resolving source conflicts — git text-merges them plausibly-but-wrong;
|
||
`npm run check:api` is the guard. Also expect other sessions to push to YOUR PR branch
|
||
(merge-main updates): `git pull` before push, treat non-FF as signal not error.
|
||
- **CI VM test timeouts**: heavy-render web tests (100+ item grids) need explicit vitest
|
||
timeouts (e.g. 15s) — the CI VM hit the 5s default on a test that runs in ~1s locally
|
||
(run 686). Bump per-test, don't raise the global default.
|
||
- **Concurrency tests need a negative control + a stress loop, not a single round** (2026-07-11,
|
||
#231): a one-shot `Barrier(N)` + `Task.WhenAll` "exactly one winner" test did NOT catch a
|
||
deliberately non-atomic check-then-set flag on this Mac — the load→store window is too narrow to
|
||
collide when threads release once. It green-lit broken code. Fix: hammer the race over many rounds
|
||
(N worker threads × ~20k rounds, two Barriers per round: acquire → validate one-winner → reset),
|
||
and ALWAYS prove non-vacuous by temporarily breaking the primitive and watching the test fail.
|
||
Cheap (sub-second) and deterministic-enough. Same discipline as #12's negative control.
|
||
- **Before deleting a "workaround", check it isn't serving a SECOND purpose** (2026-07-11, #232):
|
||
an issue 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 (now genuinely obsolete) AND
|
||
absorbing the inherent queue→observed-active lag / fast-completion race (still needed). Removing
|
||
wholesale wedged the scan button disabled-until-reload for short scans; both fork + Codex caught
|
||
it. Keep a bounded version, re-scoped honestly. When an issue says "delete X", enumerate every
|
||
behavior X provided before deleting.
|
||
- **Re-review the FIX COMMIT, not just the initial PR diff, before closing** (2026-07-11, #242,
|
||
user-surfaced): the pre-fix review is only half the loop — the fix commit is new code no
|
||
independent reviewer has seen. Here the first fix (gate Edit on `saving`) only PARTIALLY closed
|
||
the in-flight-save clobber class; a Codex re-review of the fix commit caught that **Delete** was
|
||
the one schedule-switch path (`applySwitch`) still missing the guard. Fold a scoped re-review
|
||
(Codex if quota, else a fork) over the fix commit into the close protocol; loop until a clean
|
||
MERGEABLE verdict or an explicit acceptable-defer (file a tracked issue for the deferral, as #248
|
||
here). Cheap relative to shipping a half-fix that reopens the issue.
|
||
- **`disabled={saving}` is the SchedulesScreen convention for "no schedule-switch/mutation mid-save"**
|
||
(2026-07-11, #242): the Select + `guardedSwitch` already guard `saving`; Edit and Delete must too
|
||
— Delete switches schedules via `applySwitch`, and a stale items PUT resolving after the switch
|
||
clobbers the next schedule's draft. When adding any control that can switch the active schedule or
|
||
reopen the editor, gate it on `saving` (and, for the editor, on the dirty draft).
|
||
- **Negative-controlling a lock/flag fix: invert the condition, don't stub `if (true)`** (2026-07-11,
|
||
#250): to prove a lock-release test is non-vacuous I temporarily broke the fix. First attempt made
|
||
the guard `if (true)` — but that left the ownership flag "assigned but never read" → **CS0219 under
|
||
warnings-as-errors → the Application build silently FAILED → `dotnet test --no-build` ran the STALE
|
||
(fixed) dll → false PASS** that looked like the control had failed to catch the bug. The tell: build
|
||
output must show `Build succeeded` / `0 Error(s)`, not just a truncated tail. Break the primitive by
|
||
**inverting** the real condition (`if (!lockAcquired)`) so the flag stays read and the code compiles;
|
||
always grep the build for `error CS` before trusting a `--no-build` test result. (Codex-found High
|
||
this session: an outer `catch` that unlocked unconditionally cross-released another session's lock on
|
||
a pre-acquisition exception — the fix was a Handle-scoped ownership flag gating the catch.)
|
||
- **Post-commit side-effect invalidation must run its QUERIES on `CancellationToken.None`, not just the
|
||
enqueue** (2026-07-11, #251, Codex-found Medium the fork missed): the deco-invalidation handlers committed
|
||
the edit on the request token (correct — a pre-commit cancel safely aborts), then queried the affected
|
||
playouts *also* on the request token before a `WriteAsync(..., None)` enqueue. A cancel landing in the
|
||
window between commit and query throws before the enqueue → edit persisted, no rebuild queued, stale-content
|
||
bug re-opens. Fix: once `SaveChangesAsync` has committed, run the ENTIRE side effect (affected-entity
|
||
queries + enqueue) on `CancellationToken.None` — a committed change's compensating side effect must not be
|
||
half-abortable. Generalizes the audit #22 policy from "the enqueue" to "everything after the commit". The
|
||
fork review OK'd it; only the independent Codex pass caught the query-token gap — evidence for keeping both
|
||
reviewers when quota allows.
|
||
- **Replace-all handler with NO client stable id → positional in-place reconcile via `CurrentValues.SetValues`**
|
||
(2026-07-11, #252): to stop a delete-and-reinsert handler from cascade-wiping a child that FKs the item
|
||
(`PlayoutScheduleItemFillGroupIndex` owns fill-group enumerator state, `OnDelete(Cascade)`), reconcile by
|
||
ordered POSITION (the DTO carries no id): same-runtime-type slot → `built = BuildItem(...); built.Id =
|
||
existing.Id; dbContext.Entry(existing).CurrentValues.SetValues(built)` (reuse the builder as the SetValues
|
||
source so NO field is hand-copied/dropped; set the key first so SetValues doesn't zero it), then rebuild
|
||
child join rows; subtype change/surplus → delete+insert that slot only. Keeps the row id → the cascade never
|
||
fires. The schedule-item hierarchy is **TPT** (table-per-type), not TPH — SetValues is still safe (same-type
|
||
guard; no discriminator). This is the SCOPED placeholder; true content-aware stable child identity is #253
|
||
(a moved fill-group item still inherits the state of whatever held its new slot). Anyone doing #253 builds on
|
||
/ replaces this positional reconcile in `ReplaceProgramScheduleItemsHandler`.
|
||
- **A docs PR that lists gates/cross-refs can go STALE mid-flight** (2026-07-11, #205/#206 PR #261): while the
|
||
PR sat in review a *listed* gate (#204) merged. The PR FF-rejected → rebased; the `decisions.md` append
|
||
conflict was trivial, but the real trap was the **prose** — the new removal runbook still named #204 an open
|
||
removal gate. After rebasing over a parallel merge, re-read your prose cross-references (gate lists, "blocked
|
||
on #N", status tables), not just resolve the conflict markers — git fast-forwards stale *facts* silently. The
|
||
hand-written-docs sibling of the "regenerate generated artifacts after merging main" lesson. Also: docs-only
|
||
PRs still trip main's `Build & test` branch protection, so a rebase = a fresh full-CI cycle before merge.
|
||
- **A lock/channel-ownership "no cross-release" verdict must enumerate EVERY producer of that message
|
||
type, not just the enqueuers visible in the PR diff** (2026-07-11, #235/#267). The cold fork cleared the
|
||
new `scan-collections` per-provider lock as sound — "only the acquirer enqueues, so only its message
|
||
triggers the scanner unlock" — because it checked the three new controllers but never grepped for ALL
|
||
`Synchronize{X}Collections(` senders. Codex did: `SchedulerService` also enqueues them *periodically and
|
||
unlocked*, and `ScannerService`'s `finally` released the lock whenever held → a scheduled scan
|
||
cross-releases an API scan's lock (#250 class). 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. This is why the
|
||
independent Codex pass earns its keep alongside the fork even when the fork returns MERGEABLE.
|
||
- **Fork-vs-Codex disagreement on a gate PR → Fable, and Fable often improves the fix, not just the verdict**
|
||
(2026-07-11, #235): the reconciliation isn't only "who's right" — Fable ratified Codex's High AND caught
|
||
that the orchestrator's proposed *fix* (per-source lock-or-skip) would deterministically starve the 2nd+
|
||
media source (fixed loop order), prescribing the shipped library-scan §C7 pattern instead (lock once per
|
||
provider, batch-enqueue, release on the `Unlock: isLast` message). Escalate the FIX approach too, not just
|
||
the finding.
|