From 92bf8b083d85cd90b4928d1b790d1162ff1fa2a0 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 21 Jul 2026 19:20:44 +0200 Subject: [PATCH] chore(541): fast-forward the shared checkout at session end (H13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session was handed docs/handoffs/chicorytv-issue-queue.md pasted out of /Users/timothy/ersatztv while that tree was 81 commits behind, so it still described the queue protocol #520 retired the day before (read tracker command was ever run against that tree, so every existing "never read its HEAD" guard was irrelevant: a stale checkout serves stale FILES, and docs are what a kickoff depends on. Nothing broke only because selection went through scripts/select-queue.sh. The lore bullet on that tree already prescribed the shape of the fix for its earlier failure modes — "a design flaw, not a discipline failure; a check does not stay true" — so this removes the stale condition instead of adding another check. scripts/refresh-shared-checkout.sh fast-forwards the tree to origin/main and reinstalls web/node_modules when the lockfile moved. It is deliberately timid: it refuses and changes nothing when the tree is not on main, is dirty, is ahead, or is mid-rebase/merge, and it never switches branches, stashes or discards. A NO-OP is a normal outcome. Uses npm ci rather than npm install — the first version used install, which rewrote package-lock.json and left the tree dirty, i.e. the exact state the next run refuses on, so it would have disabled itself after one use. Asserts the tree is clean at exit. refs #541 --- docs/decisions.md | 31 +++++++ docs/decisions/README.md | 1 + docs/handoffs/chicorytv-issue-queue.md | 9 ++ scripts/refresh-shared-checkout.sh | 110 +++++++++++++++++++++++++ 4 files changed, 151 insertions(+) create mode 100755 scripts/refresh-shared-checkout.sh 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 -- 2.47.3