diff --git a/docs/decisions.md b/docs/decisions.md index d601e60b1..8ccaa8847 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -133,6 +133,7 @@ in-file entries. - [2026-07-21 — External channel-logo URLs are downloaded and cached at save time; the render path never fetches a logo (#525)](#2026-07-21--external-channel-logo-urls-are-downloaded-and-cached-at-save-time-the-render-path-never-fetches-a-logo-525) - [2026-07-21 — Check the worked issue before the decision corpus; a closed tracker's comments need no retrofit (#524)](#2026-07-21--check-the-worked-issue-before-the-decision-corpus-a-closed-trackers-comments-need-no-retrofit-524) - [2026-07-21 — Work-ahead slots are claimed atomically by the caller, released by the transcode it hands them to (#536)](#2026-07-21--work-ahead-slots-are-claimed-atomically-by-the-caller-released-by-the-transcode-it-hands-them-to-536) +- [2026-07-21 — Session end fast-forwards the shared checkout; a stale tree serves stale FILES (#541)](#2026-07-21--session-end-fast-forwards-the-shared-checkout-a-stale-tree-serves-stale-files-541) --- @@ -3377,3 +3378,33 @@ pre-fix shape, 15 912 of 20 000 rounds over-claimed. **Why the return-value plumbing rather than enqueuing inside the shifter.** `PlayoutTimeShifter` lives in `ErsatzTV.Infrastructure`, which cannot reference the `RefreshChannelData` request type (an `ErsatzTV.Application` type), so the enqueue must happen in the Application-layer handler. `TimeShift` therefore returns `Option` — `Some(channelNumber)` only when a non-zero offset was actually persisted, `None` on every early-out (wrong mode, active-and-unforced, empty playout) and on a zero-offset re-tune — so a guide rebuild fires exactly once per real thaw, never on a no-op. The zero-offset `None` gate is covered by a dedicated non-vacuous test. **Per-viewer resume was deliberately not built.** `OnDemandCheckpoint` is a single value on the playout, so resume is per-channel, not per-viewer. #68 states per-channel suffices for a single household; multi-viewer identity would diverge from this model and is out of scope. + +## 2026-07-21 — Session end fast-forwards the shared checkout; a stale tree serves stale FILES (#541) + +`key: session.shared-checkout-refresh` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none` +**Rule:** Session end runs `scripts/refresh-shared-checkout.sh`, which fast-forwards `/Users/timothy/ersatztv` to `origin/main` (and reinstalls `web/node_modules` when the lockfile moved), refusing to touch anything unless that tree is on a clean, non-ahead `main`. +**Signals:** shared checkout, stale kickoff paste, `/Users/timothy/ersatztv`, session-end protocol, H13 · paths: `scripts/refresh-shared-checkout.sh`, `docs/handoffs/chicorytv-issue-queue.md` · issues: #541, #520, #311, #312 +**Mechanics:** `docs/handoffs/chicorytv-issue-queue.md` → session-end step 6 + the shared-tree lore bullet + +**The existing rule had a hole, and it is a hole no rule can close.** The standing guidance — never +commit in the shared tree, never read its `HEAD`/`git log`/`git status` as truth about `main` — is +written entirely around a session **reading git state**. On 2026-07-21 the trap arrived as a **file**: +the kickoff prompt was pasted out of that tree while it was 81 commits behind, and the handoff doc it +carried still described the queue protocol #520 had retired the previous day (read tracker #237, which +`main` now says must *not* be read for queue state). No `git` command touched that tree all session, so +no discipline check could have fired. Selection happened to go through `scripts/select-queue.sh`, which +is why nothing broke — routing luck, not a control. + +**So the fix removes the stale condition rather than adding a check**, which is what the shared-tree +lore bullet already prescribed for its first two failure modes: *"a check does not stay true."* + +**The script is deliberately timid, because the tree is shared.** It refuses — loudly, exit 0, +changing nothing — when the tree is not on `main`, is dirty, has local commits, or is mid-rebase or +mid-merge. It never switches branches, never stashes, never discards. A refusal is a normal outcome, +not a failure, because the common reason for one is that another session is legitimately mid-flight. + +**Two details that testing forced.** The first version used `npm install`, which **rewrote +`package-lock.json`** and left the shared tree dirty — the exact state the next run refuses on, so the +tool would have disabled itself after one use. It uses `npm ci`, which installs strictly from the +lockfile and never writes it. And it asserts the tree is clean at exit, reporting loudly if not: +leaving the shared tree dirty is the one outcome that would make this script a net negative. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 88c4ede06..4cec9a9c6 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -98,6 +98,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `security.fail-closed-api-auth` | Every mutating `/api` request requires `X-Api-Key` (no open mode); reads are gated by `Api:RequireKeyForReads` (default true) OR `[RequiresApiKey]` on sensitive controllers; CORS is an exact-origin allowlist (`ApiCors`); `ForwardedHeaders` trust stays configurable but defaults to trust-all-with-warning. | 2026-07-12 | [link](api-auth-security.md#2026-07-12--fail-closed-api-auth--sensitive-read-tier--corsforwardedheaders-lockdown-197-bundle-a-pr-292) | | `security.session-auth-dual-credential` | `ApiAuthorizationFilter` accepts a request when a valid `X-Api-Key` matches OR the principal is an authenticated session (cookie `ctv-session`, `HttpOnly`/`SameSite=Lax`); session-authenticated mutations require the presence-only `X-CSRF` header or are rejected 403. This narrows the OIDC-inert sub-claim of `security.blazor-removal-auth-posture` (#206) — the rest of that record's auth-surface enumeration still holds. | 2026-07-12 | [link](api-auth-security.md#2026-07-12--browser-spa-session-auth-api-accepts-session-or-machine-key-295-pr1-server-only) | | `security.session-cutover-postify` | The browser SPA authenticates cookie-only (no more `X-Api-Key` from `web/`); the machine key is repurposed to external/MCP-only via `GET /api/auth/machine-key`; every side-effecting GET/HEAD under `/api` is converted to POST so the existing CSRF gate covers it (standing rule: never add a side-effecting GET/HEAD under `/api`). | 2026-07-12 | [link](api-auth-security.md#2026-07-12--295-pr2-spa-session-cutover--301-side-effecting-get-post-ification) | +| `session.shared-checkout-refresh` | Session end runs `scripts/refresh-shared-checkout.sh`, which fast-forwards `/Users/timothy/ersatztv` to `origin/main` (and reinstalls `web/node_modules` when the lockfile moved), refusing to touch anything unless that tree is on a clean, non-ahead `main`. | 2026-07-21 | [link](../decisions.md#2026-07-21--session-end-fast-forwards-the-shared-checkout-a-stale-tree-serves-stale-files-541) | | `spa.add-to-layer` | All add-to-collection/playlist/schedule affordances share one component layer at `web/src/media/addTo/`; multi-select is an explicit screen-level toggle, and the per-card menu offers schedule only for the server-validated kinds. | 2026-07-10 | [link](../decisions.md#2026-07-10--shared-add-to-layer-lives-in-websrcmediaaddto-select-mode-is-an-explicit-toggle) | | `spa.app-shell-extraction` | `App.tsx` is only the composition root over `web/src/app/routes.tsx` (stable route-object identity), `app/AppShell.tsx` (shell chrome), and `app/ScreenContent.tsx` (exhaustive screen dispatch); primary actions are one explicit `PrimaryActionProvider` registration per screen, replacing the old global `ctv:primary-action` window event. | 2026-07-15 | [link](spa-modularization.md#2026-07-15--app-shellrouting-extraction--explicit-primary-action-ownership-247) | | `spa.autotune-detailpanel-slideover` | The Auto-Tune DetailPanel SPA is a reusable `SlideOver` primitive sharing `useOverlayBehavior` with `Dialog`, plus a shared advanced-options model extracted from ChannelBuilder; decorative panes without backend support are dropped. | 2026-07-18 | [link](../decisions.md#2026-07-18--auto-tune-detailpanel-spa-reusable-slideover--shared-advanced-options-model-decorative-panes-dropped-to-match-the-backend-386) | diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index a1469e69b..4e4469b67 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -233,6 +233,10 @@ Then work the queue: record` (template below) on each issue you closed or substantially progressed; remove your `in-progress` labels; and complete the per-issue Task Completion Protocol from the applicable `AGENTS.md` / `CLAUDE.md` instructions (including the `done` workflow when required). +6. **Leave the shared checkout fresh** (H13): run `scripts/refresh-shared-checkout.sh`. It + fast-forwards `/Users/timothy/ersatztv` to `origin/main` and refreshes `web/node_modules` if the + lockfile moved; it refuses and changes nothing unless that tree is on a clean, non-ahead `main`. + A NO-OP is a normal result — report it and carry on. ## Closing record (session-end / issue-close artifact) @@ -394,6 +398,11 @@ HARD CONSTRAINTS: 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. + + **Also treat any FILE read out of that tree as stale** — including a kickoff prompt pasted from it. + The guards above only fire when you run `git` there; a stale checkout serves stale docs silently. + Session end runs `scripts/refresh-shared-checkout.sh` to keep it current (see + `session.shared-checkout-refresh` in decisions.md). - Subagent connection drops = laptop sleep/transient; re-resume via SendMessage (work survives). - Build/verify (and review) locally, then trust it; CI (VM 127) confirms (~9 min warm; seconds for a docs-only PR since ersatztv#416). Watch by commit diff --git a/scripts/refresh-shared-checkout.sh b/scripts/refresh-shared-checkout.sh new file mode 100755 index 000000000..0cded88f2 --- /dev/null +++ b/scripts/refresh-shared-checkout.sh @@ -0,0 +1,110 @@ +#!/usr/bin/env bash +# H13 (ersatztv#541) — session-end: leave the SHARED checkout at /Users/timothy/ersatztv fresh, so +# the next session's kickoff paste is current. +# +# Why this is a script and not a rule. The standing lore already says the shared tree is not a `main` +# mirror and its HEAD must never be read as truth — but on 2026-07-21 the trap arrived through a +# route that rule doesn't cover: a session pasted `docs/handoffs/chicorytv-issue-queue.md` FROM that +# tree, which was 81 commits behind, and the file still described a queue protocol retired the day +# before (read tracker #237 — since closed, and now explicitly "do not read for queue state"). No +# `git log` was consulted, so no discipline check would have fired. A stale checkout silently +# serves stale FILES, and the only durable fix is to stop it being stale. +# +# What it does: fast-forwards the shared tree to origin/main, and reinstalls web/node_modules when +# the lockfile moved (worktrees copy that directory; a stale copy has broken typecheck before). +# +# What it will NOT do — it is deliberately timid, because the tree is shared and another session may +# be mid-flight in it. It refuses (loudly, exit 0) and leaves the WORKING TREE untouched when it is +# not on `main`, is dirty, has local commits, or is mid-rebase/merge. It never switches branches, +# never stashes, never discards. (A refusal reached after the fetch will have advanced +# refs/remotes/origin/main — remote-tracking refs only, never your files.) A refusal is a normal +# outcome, not a failure. +# +# Run at session end (kickoff session-end protocol). Safe to run at session start too — it is +# idempotent. Fail-OPEN: a missing/unreachable tree prints a notice and exits 0. +# +# ETV_SHARED_CHECKOUT overrides the target directory. +set -uo pipefail + +shared="${ETV_SHARED_CHECKOUT:-/Users/timothy/ersatztv}" + +skip() { + echo "refresh-shared-checkout: NO-OP — $1" + echo " (nothing was changed)" + exit 0 +} + +[ -d "$shared/.git" ] || skip "no git checkout at $shared" + +cd "$shared" || skip "cannot cd to $shared" + +# $shared/.git existing is NOT proof $shared is the repo root: if it is an invalid/emptied .git dir +# and some ancestor IS a repo, git walks UP and every command below would silently operate on that +# ancestor instead — the worst possible outcome for a script that fast-forwards and reinstalls. +toplevel="$(git rev-parse --show-toplevel 2>/dev/null)" || skip "not a git repo: $shared" +[ "$toplevel" = "$(pwd -P)" ] || skip "$shared is not a repo root (git resolves it to $toplevel)" + +# mid-operation? never interfere +git_dir="$(git rev-parse --git-dir 2>/dev/null)" || skip "not a git repo: $shared" +# NOTE: keep the `|| true` — without it this loop's `&&` tail would abort the script if anyone +# ever adds `set -e` (a non-match returns 1 as the body's last status). +for state in rebase-merge rebase-apply MERGE_HEAD CHERRY_PICK_HEAD BISECT_LOG REVERT_HEAD; do + { [ -e "$git_dir/$state" ] && skip "a $state is in progress"; } || true +done + +branch="$(git rev-parse --abbrev-ref HEAD 2>/dev/null)" +[ "$branch" = "main" ] || skip "on branch '$branch', not main — that is another session's work" + +if [ -n "$(git status --porcelain 2>/dev/null)" ]; then + skip "working tree is dirty (uncommitted changes)" +fi + +git fetch origin main --quiet || skip "could not fetch origin/main" + +ahead="$(git rev-list --count origin/main..HEAD 2>/dev/null)" || skip "cannot compare against origin/main" +behind="$(git rev-list --count HEAD..origin/main 2>/dev/null)" || skip "cannot compare against origin/main" + +if [ "$ahead" != "0" ]; then + skip "$ahead local commit(s) not on origin/main — someone committed here; resolve by hand" +fi + +if [ "$behind" = "0" ]; then + echo "refresh-shared-checkout: already current with origin/main ($(git rev-parse --short HEAD))." + exit 0 +fi + +lock_before="" +[ -f web/package-lock.json ] && lock_before="$(git hash-object web/package-lock.json 2>/dev/null)" + +if ! git merge --ff-only origin/main --quiet; then + skip "fast-forward to origin/main failed" +fi + +echo "refresh-shared-checkout: fast-forwarded $behind commit(s) to $(git rev-parse --short HEAD)." + +lock_after="" +[ -f web/package-lock.json ] && lock_after="$(git hash-object web/package-lock.json 2>/dev/null)" + +if [ -n "$lock_after" ] && [ "$lock_before" != "$lock_after" ]; then + echo "refresh-shared-checkout: web/package-lock.json changed — refreshing node_modules..." + # `npm ci`, not `npm install`: install can REWRITE the lockfile, which would leave this shared + # tree dirty — and a dirty tree is exactly what the next run (and every worktree-hygiene check) + # refuses on. ci installs strictly from the lockfile and never writes it. + # Accepted trade-off: `ci` DELETES node_modules first, so a session copying it at that instant gets + # a partial copy. Bounded and rare — everything removed is regenerable from the lockfile, no human + # work can be lost, and this only runs on a merge that actually moved the lockfile. + if (cd web && npm ci --no-audit --no-fund >/dev/null 2>&1); then + echo "refresh-shared-checkout: node_modules refreshed." + else + echo "refresh-shared-checkout: WARNING — npm ci failed; worktrees copying web/node_modules" + echo " from here may fail typecheck. Run 'npm ci' in $shared/web by hand." + fi +fi + +# belt and braces: leaving the shared tree dirty is the one outcome that would make this script a +# net negative, so assert it and say so loudly rather than exiting quietly on a bad state +if [ -n "$(git status --porcelain 2>/dev/null)" ]; then + echo "refresh-shared-checkout: WARNING — the shared tree is DIRTY after refresh:" + git status --short | sed 's/^/ /' + echo " Nothing was discarded. Clean it by hand before the next session pastes files from here." +fi