From 064b84eec3489d05d312654835a34f355e9eac0c Mon Sep 17 00:00:00 2001 From: norvalbv Date: Wed, 12 Aug 2026 11:24:54 +0100 Subject: [PATCH 1/2] fix(ship): a committed cache can no longer shadow the live one (sc-1489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `link_untracked_gate_configs` links gate inputs the base checkout cannot carry into the ephemeral ship worktree, and skips any path already present there. `.qavis/receipt.json` is in that candidate set — it is the gitignored, content-addressed cache `qavis qa` writes on a pass, which the ship-time `qavis-advisory` gate reads to clear its block. If a consumer commits that receipt by accident, the stale tracked copy rides the base checkout into `$WT`, the link is skipped, and the gate compares the staged sha against staleness forever. In frink (receipt committed 2026-07-28) that made every strict ship of a UI-affecting change **unsatisfiable-by-compliance**: two genuine `qavis qa --staged --route vision` passes still ended in "UI-affecting change with no qavis QA on this staged tree", with `GUARD_QAVIS_OK=1` the only exit — the exact shape report 690d2b15 already named. Shipping a fresh receipt as a PR path cannot help either: its sha covers every staged path, so a receipt cannot attest a set including itself. ## Fix A new `GATE_PROJECTION_CACHE_CANDIDATES` set (currently just `.qavis/receipt.json`) marks candidates whose bytes are a rebuildable cache rather than source. For those, a copy the BASE CHECKOUT materialised no longer wins: it is removed from the worktree and the live one linked over it, with a notice naming `git rm --cached` as the permanent fix. The shadow can never be silent again. `gate_projection_cache_is_shadowed` deliberately fires on nothing else: - a symlink — we placed it on an earlier pass - a path absent from `$WT`'s HEAD — change-application put it there, so those bytes are already live - any candidate not on the cache list — tracked source must keep winning ## Why this is safe for the shipped diff The removal is **working-tree only**. Nothing here runs `git add`/`git rm`, and the ship commit is index-only (no `-a`), so `write-tree` is invariant across the window `ship_assert_staged_unchanged` guards. A caller who also ships that path keeps their staged blob; the link just points the gate at the same live bytes. The stale comment in `assert-staged-set.sh` asserting the old "only UNTRACKED symlinks" invariant is updated to say what actually holds. ## Coverage `cli/__tests__/ship-gate-cache-shadow.test.mts` commits a `STALE` receipt, writes a `LIVE` one post-commit, and ships. It asserts the gate reads `LIVE`, that the notice names `git rm --cached`, and that the shipped commit still contains only `note.txt` — the tracked receipt is untouched by the override. Verified to fail on the pre-fix code (gate read `RECEIPT_STALE`). The hook greps CONTENT, not presence: the bug delivered a file to the gate, just the wrong one, so a `-e` probe passed straight through it. New file rather than an addition to `ship-branch.test.mts`, which is at its size ceiling. Scope: the ship/reship link path only. `qavis-advisory` is a pre-commit gate, so the review-projection branch never reads the receipt and is left unchanged. Closes sc-1489. --- cli/__tests__/ship-gate-cache-shadow.test.mts | 95 +++++++++++++++++++ cli/lib/ship/assert-staged-set.sh | 8 +- cli/lib/ship/link-gate-configs.sh | 58 ++++++++++- dist/cli/lib/ship/assert-staged-set.sh | 8 +- dist/cli/lib/ship/link-gate-configs.sh | 58 ++++++++++- 5 files changed, 213 insertions(+), 14 deletions(-) create mode 100644 cli/__tests__/ship-gate-cache-shadow.test.mts diff --git a/cli/__tests__/ship-gate-cache-shadow.test.mts b/cli/__tests__/ship-gate-cache-shadow.test.mts new file mode 100644 index 00000000..d4e077be --- /dev/null +++ b/cli/__tests__/ship-gate-cache-shadow.test.mts @@ -0,0 +1,95 @@ +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, describe, expect, it } from 'vitest'; +import { testExecFileSync as execFileSync, testSpawnSync as spawnSync } from './_helpers.mts'; + +// sc-1489. link-gate-configs.sh links gate inputs a fresh checkout cannot carry into the ephemeral +// ship worktree, and skips any path the checkout already put there. `.qavis/receipt.json` is in that +// set: the gitignored, content-addressed cache `qavis qa` writes on a pass, which the ship-time +// qavis-advisory gate reads to clear its block. +// +// Commit that receipt by accident and the stale copy rides the base checkout into $WT, the link is +// skipped, and the gate compares the staged sha against staleness forever — no number of real QA +// passes can clear it (GUARD_QAVIS_OK=1 the only exit). A cache candidate the BASE materialised must +// therefore lose to the live one, loudly. + +const scriptPath = fileURLToPath(new URL('../lib/ship/ship-branch.sh', import.meta.url)); +const GENV = { GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_SYSTEM: '/dev/null' }; +const WT_RE = /worktree kept at (.+?)\. Remove/; +const dirs: string[] = []; +afterAll(() => { + for (const d of dirs) rmSync(d, { recursive: true, force: true }); +}); + +/** + * A repo on branch `work` whose husky pre-commit hook reports the receipt's CONTENT. Presence is not + * enough to see this bug: the shadow delivered a file to the gate, just the wrong one, so a `-e` probe + * passes straight through it. + */ +function seedRepo() { + const dir = mkdtempSync(join(tmpdir(), 'shipcache-')); + dirs.push(dir); + const env = { ...process.env, ...GENV }; + const git = (args: string[], opts = {}) => + execFileSync('git', args, { cwd: dir, env, encoding: 'utf8', ...opts }); + mkdirSync(join(dir, '.husky/_'), { recursive: true }); + writeFileSync(join(dir, '.husky/.keep'), ''); + for (const a of [ + ['init', '-q', '-b', 'work'], + ['config', 'user.email', 'a@b.c'], + ['config', 'user.name', 'a'], + ['config', 'commit.gpgsign', 'false'], + ['add', '.husky/.keep'], + ['commit', '-q', '-m', 'base'], + ['config', 'core.hooksPath', '.husky/_'], + ['remote', 'add', 'origin', 'git@github.com:acme/app.git'], + ]) + git(a, { stdio: 'ignore' }); + writeFileSync( + join(dir, '.husky/_/pre-commit'), + '#!/bin/sh\ngrep -q LIVE .qavis/receipt.json 2>/dev/null && echo RECEIPT_LIVE || echo RECEIPT_STALE\nexit 0\n', + ); + chmodSync(join(dir, '.husky/_/pre-commit'), 0o755); + return { dir, env, git }; +} + +describe('ship — a COMMITTED gate cache cannot shadow the live one', () => { + it('links the live qavis receipt over a tracked stale one, and says to untrack it', () => { + const { dir, env, git } = seedRepo(); + mkdirSync(join(dir, '.qavis'), { recursive: true }); + writeFileSync(join(dir, '.qavis/recipe.json'), '{"from":"acme/qavis"}\n'); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"STALE"}\n'); // the accident + git(['add', '.qavis/recipe.json', '.qavis/receipt.json'], { stdio: 'ignore' }); + git(['commit', '-q', '--no-verify', '-m', 'accidentally track the receipt'], { + stdio: 'ignore', + }); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"LIVE"}\n'); // a real pass, post-commit + writeFileSync(join(dir, 'note.txt'), 'hi\n'); + + const r = spawnSync('/bin/bash', [scriptPath, 'feat/qavis-shadow', 't', 'note.txt'], { + cwd: dir, + input: 'b\n', + encoding: 'utf8', + env: { ...env, SHIP_DRY_RUN: '1' }, + }); + const wt = WT_RE.exec(r.stderr)?.[1]; // dry-run keeps it; drop so afterAll's rm isn't blocked + if (wt) { + try { + git(['worktree', 'remove', '--force', wt], { stdio: 'ignore' }); + } catch { + /* best-effort */ + } + } + + expect(r.status, r.stderr).toBe(0); + expect(r.stderr).toMatch(/\.qavis\/receipt\.json .*COMMITTED copy shadowed the live cache/); + expect(r.stderr).toMatch(/git rm --cached \.qavis\/receipt\.json/); // names the permanent fix + const log = readFileSync(join(dir, '.devkit/last-ship-gates-feat-qavis-shadow.log'), 'utf8'); + expect(log).toMatch(/RECEIPT_LIVE/); // the gate read the pass, not the committed staleness + expect(log).not.toMatch(/RECEIPT_STALE/); + // WORKTREE-only override: the shipped commit must not delete or rewrite the tracked receipt. + expect(git(['diff', '--name-only', 'work', 'feat/qavis-shadow']).trim()).toBe('note.txt'); + }); +}); diff --git a/cli/lib/ship/assert-staged-set.sh b/cli/lib/ship/assert-staged-set.sh index cfa63b08..e5365260 100644 --- a/cli/lib/ship/assert-staged-set.sh +++ b/cli/lib/ship/assert-staged-set.sh @@ -121,9 +121,11 @@ _ship_report_object_environment() { } # ship_assert_staged_unchanged -# Preflight, run immediately before the commit: nothing between staging and here may touch the index -# (prepare_gate_worktree and link_untracked_gate_configs only create UNTRACKED symlinks), so this is -# an exact equality check. Catches a clobber that lands before the gate chain even starts. +# Preflight, run immediately before the commit: nothing between staging and here may touch the index, +# so this is an exact equality check. Catches a clobber that lands before the gate chain even starts. +# prepare_gate_worktree and link_untracked_gate_configs only write the WORKING TREE — symlinks at +# untracked paths, plus (sc-1489) one replacing a committed cache that would otherwise shadow the live +# one. Neither runs `git add`/`rm`, and the commit is index-only (no -a), so write-tree is invariant. ship_assert_staged_unchanged() { local wt=$1 state=$2 expected actual expected=$(_ship_state_tree "$state") diff --git a/cli/lib/ship/link-gate-configs.sh b/cli/lib/ship/link-gate-configs.sh index 77d4e546..80be3d13 100644 --- a/cli/lib/ship/link-gate-configs.sh +++ b/cli/lib/ship/link-gate-configs.sh @@ -37,6 +37,40 @@ GATE_PROJECTION_FIXED_CANDIDATES=( .qavis/receipt.json ) +# Candidates whose bytes are a rebuildable, content-addressed CACHE rather than source. Committing one +# is always a mistake — the sha it attests covers a change set that stops being the one being shipped +# the instant anything else changes — but the link loop below skips any path the base checkout already +# put in $WT. So a cache that got committed by accident SILENTLY shadows the live one, and the gate +# that reads it can never be cleared by doing the work it asks for: in frink a tracked .qavis/receipt.json +# made every strict ship of a UI-affecting change unsatisfiable-by-compliance, GUARD_QAVIS_OK=1 the only +# exit, however many real `qavis qa` passes preceded it (sc-1489). These entries overwrite instead, loudly. +GATE_PROJECTION_CACHE_CANDIDATES=( + .qavis/receipt.json +) + +gate_projection_is_cache_candidate() { + local rel=$1 cache + for cache in "${GATE_PROJECTION_CACHE_CANDIDATES[@]}"; do + [ "$rel" = "$cache" ] && return 0 + done + return 1 +} + +# gate_projection_cache_is_shadowed +# +# True only for the accident above: a cache candidate that the BASE CHECKOUT materialised, i.e. a real +# file git tracks at $WT's HEAD (the ship base — the commit is still ahead of us). Deliberately NOT: +# - a symlink — we placed it ourselves on an earlier pass; overwriting would be a no-op at best. +# - a path absent from HEAD — change-application put it there from the invoking checkout, so those +# bytes ARE the live ones. Nothing to prefer. +gate_projection_cache_is_shadowed() { + local wt=$1 rel=$2 + gate_projection_is_cache_candidate "$rel" || return 1 + [ -L "$wt/$rel" ] && return 1 + [ -f "$wt/$rel" ] || return 1 + git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null +} + gate_config_path_emitter() { local self_dir emitter self_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -93,9 +127,9 @@ gate_projection_source_is_ignored() { # link_untracked_gate_configs [purpose] link_untracked_gate_configs() { local wt=$1 root=$2 purpose=${3:-ship} emitter resolved rel line index_rel='' candidate_manifest='' - local main_root='' candidate_root=$root source='' + local main_root='' candidate_root=$root source='' shadowed='' local projection_manifest=${DEVKIT_REVIEW_PROJECTION_MANIFEST:-} projection_tool='' - local linked=() linked_sources=() candidates=() + local linked=() linked_sources=() linked_shadowed=() candidates=() case "$purpose" in ship | review | review-baseline) ;; *) @@ -169,6 +203,7 @@ link_untracked_gate_configs() { [ -e "$root/$rel" ] && [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue linked+=("$rel") linked_sources+=("$root/$rel") + linked_shadowed+=('') done { if [ "${#linked[@]}" -gt 0 ]; then @@ -181,11 +216,21 @@ link_untracked_gate_configs() { # -L guard also skips a pre-existing symlink so `ln` never aborts on it. Empty local projection # dirs are unusable, so a populated main-worktree copy wins; files keep root-first precedence. source=$(gate_link_source "$root" "$main_root" "$rel" prefer-populated) || continue - [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue + shadowed='' + if [ -e "$wt/$rel" ] || [ -L "$wt/$rel" ]; then + gate_projection_cache_is_shadowed "$wt" "$rel" || continue + # WORKTREE-ONLY removal. The index is not touched: assert-staged-set.sh holds the ship to an + # exact `write-tree` equality across this window and the commit is index-only (no -a), so this + # changes what the GATES read and nothing about what is shipped. A caller who also shipped this + # path keeps their staged blob — the link just points the gate at the same live bytes. + rm -f "$wt/$rel" + shadowed=1 + fi mkdir -p "$wt/$(dirname "$rel")" ln -s "$source" "$wt/$rel" linked+=("$rel") linked_sources+=("$source") + linked_shadowed+=("$shadowed") done fi @@ -193,6 +238,8 @@ link_untracked_gate_configs() { # under `set -u`; cf. commit-with-gate-capture.sh). [ "${#linked[@]}" -eq 0 ] && return 0 { + # "absent from" covers every entry but a shadowed cache, which is present-but-stale. The per-entry + # line below says which it was, so the count stays one number instead of two headline clauses. echo "⚠️ ship: ${#linked[@]} gate config(s) present in the repo but absent from the committed tree —" if is_review_projection_purpose "$purpose"; then echo " copied into the isolated review worktree so gates match the target (not defaults):" @@ -202,7 +249,10 @@ link_untracked_gate_configs() { local linked_index=0 for rel in "${linked[@]}"; do # `check-ignore -q` inside the `if` → its exit-1 "not ignored" is errexit-safe. - if gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then + if [ -n "${linked_shadowed[$linked_index]:-}" ]; then + echo " - $rel (a COMMITTED copy shadowed the live cache — overridden for the gates only;" + echo " run \`git rm --cached $rel\` and gitignore it, or the gate reading it stays stale)" + elif gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then echo " - $rel (gitignored cache — normal)" else echo " - $rel (untracked — commit it so gates are consistent for everyone)" diff --git a/dist/cli/lib/ship/assert-staged-set.sh b/dist/cli/lib/ship/assert-staged-set.sh index cfa63b08..e5365260 100644 --- a/dist/cli/lib/ship/assert-staged-set.sh +++ b/dist/cli/lib/ship/assert-staged-set.sh @@ -121,9 +121,11 @@ _ship_report_object_environment() { } # ship_assert_staged_unchanged -# Preflight, run immediately before the commit: nothing between staging and here may touch the index -# (prepare_gate_worktree and link_untracked_gate_configs only create UNTRACKED symlinks), so this is -# an exact equality check. Catches a clobber that lands before the gate chain even starts. +# Preflight, run immediately before the commit: nothing between staging and here may touch the index, +# so this is an exact equality check. Catches a clobber that lands before the gate chain even starts. +# prepare_gate_worktree and link_untracked_gate_configs only write the WORKING TREE — symlinks at +# untracked paths, plus (sc-1489) one replacing a committed cache that would otherwise shadow the live +# one. Neither runs `git add`/`rm`, and the commit is index-only (no -a), so write-tree is invariant. ship_assert_staged_unchanged() { local wt=$1 state=$2 expected actual expected=$(_ship_state_tree "$state") diff --git a/dist/cli/lib/ship/link-gate-configs.sh b/dist/cli/lib/ship/link-gate-configs.sh index 77d4e546..80be3d13 100644 --- a/dist/cli/lib/ship/link-gate-configs.sh +++ b/dist/cli/lib/ship/link-gate-configs.sh @@ -37,6 +37,40 @@ GATE_PROJECTION_FIXED_CANDIDATES=( .qavis/receipt.json ) +# Candidates whose bytes are a rebuildable, content-addressed CACHE rather than source. Committing one +# is always a mistake — the sha it attests covers a change set that stops being the one being shipped +# the instant anything else changes — but the link loop below skips any path the base checkout already +# put in $WT. So a cache that got committed by accident SILENTLY shadows the live one, and the gate +# that reads it can never be cleared by doing the work it asks for: in frink a tracked .qavis/receipt.json +# made every strict ship of a UI-affecting change unsatisfiable-by-compliance, GUARD_QAVIS_OK=1 the only +# exit, however many real `qavis qa` passes preceded it (sc-1489). These entries overwrite instead, loudly. +GATE_PROJECTION_CACHE_CANDIDATES=( + .qavis/receipt.json +) + +gate_projection_is_cache_candidate() { + local rel=$1 cache + for cache in "${GATE_PROJECTION_CACHE_CANDIDATES[@]}"; do + [ "$rel" = "$cache" ] && return 0 + done + return 1 +} + +# gate_projection_cache_is_shadowed +# +# True only for the accident above: a cache candidate that the BASE CHECKOUT materialised, i.e. a real +# file git tracks at $WT's HEAD (the ship base — the commit is still ahead of us). Deliberately NOT: +# - a symlink — we placed it ourselves on an earlier pass; overwriting would be a no-op at best. +# - a path absent from HEAD — change-application put it there from the invoking checkout, so those +# bytes ARE the live ones. Nothing to prefer. +gate_projection_cache_is_shadowed() { + local wt=$1 rel=$2 + gate_projection_is_cache_candidate "$rel" || return 1 + [ -L "$wt/$rel" ] && return 1 + [ -f "$wt/$rel" ] || return 1 + git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null +} + gate_config_path_emitter() { local self_dir emitter self_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -93,9 +127,9 @@ gate_projection_source_is_ignored() { # link_untracked_gate_configs [purpose] link_untracked_gate_configs() { local wt=$1 root=$2 purpose=${3:-ship} emitter resolved rel line index_rel='' candidate_manifest='' - local main_root='' candidate_root=$root source='' + local main_root='' candidate_root=$root source='' shadowed='' local projection_manifest=${DEVKIT_REVIEW_PROJECTION_MANIFEST:-} projection_tool='' - local linked=() linked_sources=() candidates=() + local linked=() linked_sources=() linked_shadowed=() candidates=() case "$purpose" in ship | review | review-baseline) ;; *) @@ -169,6 +203,7 @@ link_untracked_gate_configs() { [ -e "$root/$rel" ] && [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue linked+=("$rel") linked_sources+=("$root/$rel") + linked_shadowed+=('') done { if [ "${#linked[@]}" -gt 0 ]; then @@ -181,11 +216,21 @@ link_untracked_gate_configs() { # -L guard also skips a pre-existing symlink so `ln` never aborts on it. Empty local projection # dirs are unusable, so a populated main-worktree copy wins; files keep root-first precedence. source=$(gate_link_source "$root" "$main_root" "$rel" prefer-populated) || continue - [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue + shadowed='' + if [ -e "$wt/$rel" ] || [ -L "$wt/$rel" ]; then + gate_projection_cache_is_shadowed "$wt" "$rel" || continue + # WORKTREE-ONLY removal. The index is not touched: assert-staged-set.sh holds the ship to an + # exact `write-tree` equality across this window and the commit is index-only (no -a), so this + # changes what the GATES read and nothing about what is shipped. A caller who also shipped this + # path keeps their staged blob — the link just points the gate at the same live bytes. + rm -f "$wt/$rel" + shadowed=1 + fi mkdir -p "$wt/$(dirname "$rel")" ln -s "$source" "$wt/$rel" linked+=("$rel") linked_sources+=("$source") + linked_shadowed+=("$shadowed") done fi @@ -193,6 +238,8 @@ link_untracked_gate_configs() { # under `set -u`; cf. commit-with-gate-capture.sh). [ "${#linked[@]}" -eq 0 ] && return 0 { + # "absent from" covers every entry but a shadowed cache, which is present-but-stale. The per-entry + # line below says which it was, so the count stays one number instead of two headline clauses. echo "⚠️ ship: ${#linked[@]} gate config(s) present in the repo but absent from the committed tree —" if is_review_projection_purpose "$purpose"; then echo " copied into the isolated review worktree so gates match the target (not defaults):" @@ -202,7 +249,10 @@ link_untracked_gate_configs() { local linked_index=0 for rel in "${linked[@]}"; do # `check-ignore -q` inside the `if` → its exit-1 "not ignored" is errexit-safe. - if gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then + if [ -n "${linked_shadowed[$linked_index]:-}" ]; then + echo " - $rel (a COMMITTED copy shadowed the live cache — overridden for the gates only;" + echo " run \`git rm --cached $rel\` and gitignore it, or the gate reading it stays stale)" + elif gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then echo " - $rel (gitignored cache — normal)" else echo " - $rel (untracked — commit it so gates are consistent for everyone)" From 328147ef213ba3f66829352f1f921da342233448 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Wed, 12 Aug 2026 14:36:06 +0100 Subject: [PATCH 2/2] fix(ship): a committed gate cache loses to the live one, and its untracking can land (sc-1489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The bug `.qavis/receipt.json` is the gitignored, content-addressed cache `qavis qa` writes on a pass, read by the ship-time `qavis-advisory` gate to clear its block. `link_untracked_gate_configs` links it into the ephemeral ship worktree — but skips any path the base checkout already put there. frink committed that receipt by accident on 2026-07-28. The stale copy therefore rode the base checkout into the worktree, the link was skipped as "already present", and `qavis route` compared the staged sha against staleness forever. Two genuine `qavis qa --staged --route vision` passes still ended in "UI-affecting change with no qavis QA on this staged tree", with `GUARD_QAVIS_OK=1` the only exit. ## Two fixes, because the remedy has to be landable **1. A committed cache loses to the live one.** `GATE_PROJECTION_CACHE_CANDIDATES` names the gate inputs that are content-addressed caches rather than source. For those, a base-committed copy is removed from the gate worktree, the live one is linked over it, and it gets its own notice — never the `linked` list, whose "absent from the committed tree" wording is false for it. ```bash rm -f "$wt/$rel" # worktree only; the shipped commit is asserted unchanged by this file's test ``` Nothing there runs `git add`/`git rm` and the ship commit is index-only, so `write-tree` is invariant across the window `ship_assert_staged_unchanged` guards. `assert-staged-set.sh`'s parenthetical is corrected from "only create UNTRACKED symlinks" to "write only the WORKING TREE". **2. Ship no longer resurrects a path staged for deletion.** ```bash git -C "$WT" diff --cached --quiet --diff-filter=D -- "$f" || continue ``` The force-add pass exists to catch ignored files the diff MISSED, never to overrule one it EXPRESSED. Without this the notice's own remedy cannot land: `git rm` also deletes the operator's live receipt, so they re-run the tool, the cache comes back untracked AND gitignored, and the force-add pass sweeps it into the commit. This is general to ship, not cache-specific — it is what the original report meant by *"devkit ship cannot express deleting the tracked receipt while the gitignored cache exists on disk at the same path."* `reship.sh` has a similar shape but documented "current content of each path" semantics; unchanged, and recorded as a scope decision. ## Tests `cli/__tests__/ship-gate-cache-shadow.test.mts` — the hook greps the receipt's CONTENT, since the bug handed the gate a file, just the wrong one: 1. Committed stale receipt → gate reads `RECEIPT_LIVE`; the pasteable `git rm .qavis/receipt.json` is present and `--cached` is not; no `(untracked — commit it …)` contradiction; the shipped commit still contains only `note.txt`. 2. The untracking ship, **with the cache regenerated in between** → lands, receipt gone from the tree, notice quiet, gate still sees the live receipt. 3. Already-gitignored receipt → normal link path, no false positive. Each verified to fail against the defect it pins. Standalone file because `ship-branch.test.mts` is at its 2000-line ceiling. ## Rejected en route An earlier revision **aborted** the ship on a committed cache. It reads as the stricter fix, but the guard reads the ship BASE, so no ship can clear itself — the untracking would need a separate PR round-trip while every ship in the repo stays blocked. That is `qavis-advisory-gate.md` Rejected (c)'s shape (a block whose remedy the blocked path cannot carry), rebuilt while fixing its original instance. Recorded in the decision log along with the reship scope call. Closes sc-1489. --- cli/__tests__/ship-gate-cache-shadow.test.mts | 148 ++++++++++++------ cli/lib/ship/assert-staged-set.sh | 8 +- cli/lib/ship/link-gate-configs.sh | 89 +++++------ cli/lib/ship/ship-branch.sh | 5 + dist/cli/lib/ship/assert-staged-set.sh | 8 +- dist/cli/lib/ship/link-gate-configs.sh | 89 +++++------ dist/cli/lib/ship/ship-branch.sh | 5 + docs/decisions/qavis-advisory-gate.md | 1 + 8 files changed, 201 insertions(+), 152 deletions(-) diff --git a/cli/__tests__/ship-gate-cache-shadow.test.mts b/cli/__tests__/ship-gate-cache-shadow.test.mts index d4e077be..73be082b 100644 --- a/cli/__tests__/ship-gate-cache-shadow.test.mts +++ b/cli/__tests__/ship-gate-cache-shadow.test.mts @@ -5,15 +5,9 @@ import { fileURLToPath } from 'node:url'; import { afterAll, describe, expect, it } from 'vitest'; import { testExecFileSync as execFileSync, testSpawnSync as spawnSync } from './_helpers.mts'; -// sc-1489. link-gate-configs.sh links gate inputs a fresh checkout cannot carry into the ephemeral -// ship worktree, and skips any path the checkout already put there. `.qavis/receipt.json` is in that -// set: the gitignored, content-addressed cache `qavis qa` writes on a pass, which the ship-time -// qavis-advisory gate reads to clear its block. -// -// Commit that receipt by accident and the stale copy rides the base checkout into $WT, the link is -// skipped, and the gate compares the staged sha against staleness forever — no number of real QA -// passes can clear it (GUARD_QAVIS_OK=1 the only exit). A cache candidate the BASE materialised must -// therefore lose to the live one, loudly. +// sc-1489: a consumer committed `.qavis/receipt.json`, so the base checkout kept putting a stale copy +// in the ship worktree, the link was skipped as "already present", and the qavis-advisory gate could +// never be cleared by running qavis — GUARD_QAVIS_OK=1 was the only exit. const scriptPath = fileURLToPath(new URL('../lib/ship/ship-branch.sh', import.meta.url)); const GENV = { GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_SYSTEM: '/dev/null' }; @@ -23,11 +17,11 @@ afterAll(() => { for (const d of dirs) rmSync(d, { recursive: true, force: true }); }); -/** - * A repo on branch `work` whose husky pre-commit hook reports the receipt's CONTENT. Presence is not - * enough to see this bug: the shadow delivered a file to the gate, just the wrong one, so a `-e` probe - * passes straight through it. - */ +// The hook reports the receipt's CONTENT. Presence is not enough to see this bug: the shadow handed +// the gate a file, just the wrong one, so a `-e` probe passes straight through it. +const HOOK = + '#!/bin/sh\ngrep -q live .qavis/receipt.json 2>/dev/null && echo RECEIPT_LIVE || echo RECEIPT_STALE\nexit 0\n'; + function seedRepo() { const dir = mkdtempSync(join(tmpdir(), 'shipcache-')); dirs.push(dir); @@ -36,6 +30,8 @@ function seedRepo() { execFileSync('git', args, { cwd: dir, env, encoding: 'utf8', ...opts }); mkdirSync(join(dir, '.husky/_'), { recursive: true }); writeFileSync(join(dir, '.husky/.keep'), ''); + writeFileSync(join(dir, '.husky/_/pre-commit'), HOOK); // gitignored runner; ship fails closed without it + chmodSync(join(dir, '.husky/_/pre-commit'), 0o755); for (const a of [ ['init', '-q', '-b', 'work'], ['config', 'user.email', 'a@b.c'], @@ -47,49 +43,101 @@ function seedRepo() { ['remote', 'add', 'origin', 'git@github.com:acme/app.git'], ]) git(a, { stdio: 'ignore' }); - writeFileSync( - join(dir, '.husky/_/pre-commit'), - '#!/bin/sh\ngrep -q LIVE .qavis/receipt.json 2>/dev/null && echo RECEIPT_LIVE || echo RECEIPT_STALE\nexit 0\n', - ); - chmodSync(join(dir, '.husky/_/pre-commit'), 0o755); + mkdirSync(join(dir, '.qavis'), { recursive: true }); + writeFileSync(join(dir, '.qavis/recipe.json'), '{"from":"acme/qavis"}\n'); return { dir, env, git }; } -describe('ship — a COMMITTED gate cache cannot shadow the live one', () => { - it('links the live qavis receipt over a tracked stale one, and says to untrack it', () => { +function ship(dir: string, env: NodeJS.ProcessEnv, git, branch: string, paths = ['note.txt']) { + const r = spawnSync('/bin/bash', [scriptPath, branch, 't', ...paths], { + cwd: dir, + input: 'b\n', + encoding: 'utf8', + env: { ...env, SHIP_DRY_RUN: '1' }, + }); + const wt = WT_RE.exec(r.stderr)?.[1]; // dry-run keeps it; drop so afterAll's rm isn't blocked + if (wt) { + try { + git(['worktree', 'remove', '--force', wt], { stdio: 'ignore' }); + } catch { + /* best-effort */ + } + } + return { + ...r, + gateLog: () => + readFileSync(join(dir, `.devkit/last-ship-gates-${branch.replace(/\//g, '-')}.log`), 'utf8'), + }; +} + +describe('ship — a committed gate cache must lose to the live one', () => { + it('links the live receipt over the stale committed copy, without touching the shipped commit', () => { const { dir, env, git } = seedRepo(); - mkdirSync(join(dir, '.qavis'), { recursive: true }); - writeFileSync(join(dir, '.qavis/recipe.json'), '{"from":"acme/qavis"}\n'); - writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"STALE"}\n'); // the accident - git(['add', '.qavis/recipe.json', '.qavis/receipt.json'], { stdio: 'ignore' }); - git(['commit', '-q', '--no-verify', '-m', 'accidentally track the receipt'], { - stdio: 'ignore', - }); - writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"LIVE"}\n'); // a real pass, post-commit + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"stale"}\n'); + git(['add', '.qavis'], { stdio: 'ignore' }); + git(['commit', '-q', '-m', 'accidentally track the receipt'], { stdio: 'ignore' }); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"live"}\n'); // a real pass, post-commit writeFileSync(join(dir, 'note.txt'), 'hi\n'); - const r = spawnSync('/bin/bash', [scriptPath, 'feat/qavis-shadow', 't', 'note.txt'], { - cwd: dir, - input: 'b\n', - encoding: 'utf8', - env: { ...env, SHIP_DRY_RUN: '1' }, - }); - const wt = WT_RE.exec(r.stderr)?.[1]; // dry-run keeps it; drop so afterAll's rm isn't blocked - if (wt) { - try { - git(['worktree', 'remove', '--force', wt], { stdio: 'ignore' }); - } catch { - /* best-effort */ - } - } + const r = ship(dir, env, git, 'feat/committed-receipt'); + + expect(r.status, r.stderr).toBe(0); + expect(r.gateLog()).toMatch(/RECEIPT_LIVE/); // the gate read the pass, not the committed staleness + expect(r.gateLog()).not.toMatch(/RECEIPT_STALE/); + expect(r.stderr).toMatch(/gate cache\(s\) are COMMITTED/); + expect(r.stderr).toContain('git rm .qavis/receipt.json'); // pasteable, and landable by ship + expect(r.stderr).not.toContain('git rm --cached'); // --cached leaves it on disk → ship re-adds it + // It must not ALSO reach the linked-config classifier, which would tell the operator to commit the + // very file the notice above says to untrack (and "untracked" is false — the base tracks it). + expect(r.stderr).not.toMatch(/\.qavis\/receipt\.json \(untracked/); + expect(r.stderr).not.toMatch(/commit it so gates/); + // The override is worktree-only: the shipped commit must not delete or rewrite the tracked receipt. + expect(git(['diff', '--name-only', 'work', 'feat/committed-receipt']).trim()).toBe('note.txt'); + }); + + // The notice is only worth printing if devkit can LAND what it prints, in the sequence an operator + // actually follows: `git rm` deletes their live receipt too, so they re-run the tool and it comes + // back — untracked AND gitignored, exactly what ship's force-add pass sweeps up. Regenerating it here + // is the whole point of the test; without that write the deletion lands trivially and pins nothing. + it('lands the untracking even after the tool regenerates the cache', () => { + const { dir, env, git } = seedRepo(); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"stale"}\n'); + git(['add', '.qavis'], { stdio: 'ignore' }); + git(['commit', '-q', '-m', 'accidentally track the receipt'], { stdio: 'ignore' }); + git(['rm', '-q', '.qavis/receipt.json'], { stdio: 'ignore' }); // exactly what the notice prints + writeFileSync(join(dir, '.gitignore'), '.qavis/receipt.json\n'); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"live"}\n'); // `qavis qa` re-run + writeFileSync(join(dir, 'note.txt'), 'hi\n'); + + const r = ship(dir, env, git, 'feat/untrack-receipt', [ + '.gitignore', + '.qavis/receipt.json', + 'note.txt', + ]); + + expect(r.status, r.stderr).toBe(0); + const tree = git(['ls-tree', '-r', '--name-only', 'feat/untrack-receipt']).trim().split('\n'); + expect(tree).not.toContain('.qavis/receipt.json'); // the defect is gone from the base, for good + expect(tree).toContain('.gitignore'); + // Nothing stale reached the worktree, so the notice must stay quiet on the ship that fixes it. + expect(r.stderr).not.toMatch(/gate cache\(s\) are COMMITTED/); + // ...and the regenerated cache still reached the gate, so this ship is QA-clearable like any other. + expect(r.gateLog()).toMatch(/RECEIPT_LIVE/); + }); + + it('leaves an already-gitignored receipt on the normal link path', () => { + const { dir, env, git } = seedRepo(); + writeFileSync(join(dir, '.gitignore'), '.qavis/receipt.json\n'); + git(['add', '.qavis/recipe.json', '.gitignore'], { stdio: 'ignore' }); + git(['commit', '-q', '-m', 'track recipe, ignore receipt'], { stdio: 'ignore' }); + writeFileSync(join(dir, '.qavis/receipt.json'), '{"sha":"live"}\n'); + writeFileSync(join(dir, 'note.txt'), 'hi\n'); + + const r = ship(dir, env, git, 'feat/ignored-receipt'); expect(r.status, r.stderr).toBe(0); - expect(r.stderr).toMatch(/\.qavis\/receipt\.json .*COMMITTED copy shadowed the live cache/); - expect(r.stderr).toMatch(/git rm --cached \.qavis\/receipt\.json/); // names the permanent fix - const log = readFileSync(join(dir, '.devkit/last-ship-gates-feat-qavis-shadow.log'), 'utf8'); - expect(log).toMatch(/RECEIPT_LIVE/); // the gate read the pass, not the committed staleness - expect(log).not.toMatch(/RECEIPT_STALE/); - // WORKTREE-only override: the shipped commit must not delete or rewrite the tracked receipt. - expect(git(['diff', '--name-only', 'work', 'feat/qavis-shadow']).trim()).toBe('note.txt'); + expect(r.gateLog()).toMatch(/RECEIPT_LIVE/); + expect(r.stderr).toMatch(/\.qavis\/receipt\.json \(gitignored cache/); + expect(r.stderr).not.toMatch(/gate cache\(s\) are COMMITTED/); // no false positive }); }); diff --git a/cli/lib/ship/assert-staged-set.sh b/cli/lib/ship/assert-staged-set.sh index e5365260..c8ef17f1 100644 --- a/cli/lib/ship/assert-staged-set.sh +++ b/cli/lib/ship/assert-staged-set.sh @@ -121,11 +121,9 @@ _ship_report_object_environment() { } # ship_assert_staged_unchanged -# Preflight, run immediately before the commit: nothing between staging and here may touch the index, -# so this is an exact equality check. Catches a clobber that lands before the gate chain even starts. -# prepare_gate_worktree and link_untracked_gate_configs only write the WORKING TREE — symlinks at -# untracked paths, plus (sc-1489) one replacing a committed cache that would otherwise shadow the live -# one. Neither runs `git add`/`rm`, and the commit is index-only (no -a), so write-tree is invariant. +# Preflight, run immediately before the commit: nothing between staging and here may touch the index +# (prepare_gate_worktree and link_untracked_gate_configs write only the WORKING TREE), so this is +# an exact equality check. Catches a clobber that lands before the gate chain even starts. ship_assert_staged_unchanged() { local wt=$1 state=$2 expected actual expected=$(_ship_state_tree "$state") diff --git a/cli/lib/ship/link-gate-configs.sh b/cli/lib/ship/link-gate-configs.sh index 80be3d13..e4196a34 100644 --- a/cli/lib/ship/link-gate-configs.sh +++ b/cli/lib/ship/link-gate-configs.sh @@ -37,40 +37,29 @@ GATE_PROJECTION_FIXED_CANDIDATES=( .qavis/receipt.json ) -# Candidates whose bytes are a rebuildable, content-addressed CACHE rather than source. Committing one -# is always a mistake — the sha it attests covers a change set that stops being the one being shipped -# the instant anything else changes — but the link loop below skips any path the base checkout already -# put in $WT. So a cache that got committed by accident SILENTLY shadows the live one, and the gate -# that reads it can never be cleared by doing the work it asks for: in frink a tracked .qavis/receipt.json -# made every strict ship of a UI-affecting change unsatisfiable-by-compliance, GUARD_QAVIS_OK=1 the only -# exit, however many real `qavis qa` passes preceded it (sc-1489). These entries overwrite instead, loudly. +# Candidates that are a content-addressed CACHE, not source. A copy in the base commit is stale by +# construction — the sha it attests cannot cover the set being shipped — so it must lose to the live +# one, or the gate reading it can never be cleared by running the tool (sc-1489). GATE_PROJECTION_CACHE_CANDIDATES=( .qavis/receipt.json ) -gate_projection_is_cache_candidate() { - local rel=$1 cache +# gate_projection_is_stale_cache +# A cache candidate the BASE COMMIT put in $WT and that is still there: not a symlink (we placed that), +# not a path change-application already removed (a ship that untracks it), and not one absent from HEAD +# (change-application put those there from the invoking checkout — already the live bytes). +gate_projection_is_stale_cache() { + local wt=$1 rel=$2 cache for cache in "${GATE_PROJECTION_CACHE_CANDIDATES[@]}"; do - [ "$rel" = "$cache" ] && return 0 + [ "$rel" = "$cache" ] || continue + [ -L "$wt/$rel" ] && return 1 + [ -f "$wt/$rel" ] || return 1 + git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null && return 0 + return 1 done return 1 } -# gate_projection_cache_is_shadowed -# -# True only for the accident above: a cache candidate that the BASE CHECKOUT materialised, i.e. a real -# file git tracks at $WT's HEAD (the ship base — the commit is still ahead of us). Deliberately NOT: -# - a symlink — we placed it ourselves on an earlier pass; overwriting would be a no-op at best. -# - a path absent from HEAD — change-application put it there from the invoking checkout, so those -# bytes ARE the live ones. Nothing to prefer. -gate_projection_cache_is_shadowed() { - local wt=$1 rel=$2 - gate_projection_is_cache_candidate "$rel" || return 1 - [ -L "$wt/$rel" ] && return 1 - [ -f "$wt/$rel" ] || return 1 - git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null -} - gate_config_path_emitter() { local self_dir emitter self_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -127,9 +116,9 @@ gate_projection_source_is_ignored() { # link_untracked_gate_configs [purpose] link_untracked_gate_configs() { local wt=$1 root=$2 purpose=${3:-ship} emitter resolved rel line index_rel='' candidate_manifest='' - local main_root='' candidate_root=$root source='' shadowed='' + local main_root='' candidate_root=$root source='' stale_hit='' local projection_manifest=${DEVKIT_REVIEW_PROJECTION_MANIFEST:-} projection_tool='' - local linked=() linked_sources=() linked_shadowed=() candidates=() + local linked=() linked_sources=() candidates=() stale=() case "$purpose" in ship | review | review-baseline) ;; *) @@ -203,7 +192,6 @@ link_untracked_gate_configs() { [ -e "$root/$rel" ] && [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue linked+=("$rel") linked_sources+=("$root/$rel") - linked_shadowed+=('') done { if [ "${#linked[@]}" -gt 0 ]; then @@ -215,31 +203,43 @@ link_untracked_gate_configs() { # Present in the repo but absent from the committed worktree = the gate would fail open. The # -L guard also skips a pre-existing symlink so `ln` never aborts on it. Empty local projection # dirs are unusable, so a populated main-worktree copy wins; files keep root-first precedence. + # Recorded BEFORE the source lookup so the diagnostic still prints when the operator's checkout + # has no live copy to link. A stale cache never enters `linked`: it is present in the committed + # tree, so that notice's wording and count would both be wrong for it. + stale_hit= + if gate_projection_is_stale_cache "$wt" "$rel"; then + rm -f "$wt/$rel" # worktree only; the shipped commit is asserted unchanged by this file's test + stale+=("$rel") + stale_hit=1 + fi source=$(gate_link_source "$root" "$main_root" "$rel" prefer-populated) || continue - shadowed='' - if [ -e "$wt/$rel" ] || [ -L "$wt/$rel" ]; then - gate_projection_cache_is_shadowed "$wt" "$rel" || continue - # WORKTREE-ONLY removal. The index is not touched: assert-staged-set.sh holds the ship to an - # exact `write-tree` equality across this window and the commit is index-only (no -a), so this - # changes what the GATES read and nothing about what is shipped. A caller who also shipped this - # path keeps their staged blob — the link just points the gate at the same live bytes. - rm -f "$wt/$rel" - shadowed=1 + if [ -z "$stale_hit" ]; then + [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue + linked+=("$rel") + linked_sources+=("$source") fi mkdir -p "$wt/$(dirname "$rel")" ln -s "$source" "$wt/$rel" - linked+=("$rel") - linked_sources+=("$source") - linked_shadowed+=("$shadowed") done fi + if [ "${#stale[@]}" -gt 0 ]; then + { + echo "⚠️ ship: ${#stale[@]} gate cache(s) are COMMITTED, so the base checkout carried a stale copy" + echo " into the gate worktree — the live one was used instead. These are content-addressed: a" + echo " committed copy can never match the set being shipped. Untrack them and LAND it on the" + echo " base, or every ship repeats this. Use \`git rm\`, NOT \`--cached\` — a file left on disk" + echo " stages no deletion. Ship BOTH paths, ideally on their own so no QA gate is in the way:" + for rel in "${stale[@]}"; do + echo " - git rm $rel && printf '%s\\n' '$rel' >> .gitignore" + done + } >&2 + fi + # Guard the empty array BEFORE expanding it (stock-macOS bash 3.2 aborts on "${arr[@]}" when empty # under `set -u`; cf. commit-with-gate-capture.sh). [ "${#linked[@]}" -eq 0 ] && return 0 { - # "absent from" covers every entry but a shadowed cache, which is present-but-stale. The per-entry - # line below says which it was, so the count stays one number instead of two headline clauses. echo "⚠️ ship: ${#linked[@]} gate config(s) present in the repo but absent from the committed tree —" if is_review_projection_purpose "$purpose"; then echo " copied into the isolated review worktree so gates match the target (not defaults):" @@ -249,10 +249,7 @@ link_untracked_gate_configs() { local linked_index=0 for rel in "${linked[@]}"; do # `check-ignore -q` inside the `if` → its exit-1 "not ignored" is errexit-safe. - if [ -n "${linked_shadowed[$linked_index]:-}" ]; then - echo " - $rel (a COMMITTED copy shadowed the live cache — overridden for the gates only;" - echo " run \`git rm --cached $rel\` and gitignore it, or the gate reading it stays stale)" - elif gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then + if gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then echo " - $rel (gitignored cache — normal)" else echo " - $rel (untracked — commit it so gates are consistent for everyone)" diff --git a/cli/lib/ship/ship-branch.sh b/cli/lib/ship/ship-branch.sh index 93729116..6d7fa6d0 100644 --- a/cli/lib/ship/ship-branch.sh +++ b/cli/lib/ship/ship-branch.sh @@ -250,7 +250,12 @@ done # vanished, publishing a devkit whose gate supervisor could not resolve its own import. # Every PATHS entry is caller-explicit (positional after --; directories rejected above), so # forcing them is precisely what was asked — same reasoning as reship.sh's `git add -f` (#199). +# A path the patch staged as DELETED is skipped: this pass exists to catch ignored files the diff +# MISSED, never to overrule one it expressed. Without the guard, deleting a tracked file whose +# gitignored copy is back on disk (a regenerable cache — sc-1489's receipt) silently re-adds it and +# the deletion can never land, however many times it is shipped. git -C "$ROOT" ls-files -o -i --exclude-standard -- "${PATHS[@]}" | while IFS= read -r f; do + git -C "$WT" diff --cached --quiet --diff-filter=D -- "$f" || continue mkdir -p "$WT/$(dirname "$f")" cp -Pp "$ROOT/$f" "$WT/$f" git -C "$WT" add -f -- "$f" diff --git a/dist/cli/lib/ship/assert-staged-set.sh b/dist/cli/lib/ship/assert-staged-set.sh index e5365260..c8ef17f1 100644 --- a/dist/cli/lib/ship/assert-staged-set.sh +++ b/dist/cli/lib/ship/assert-staged-set.sh @@ -121,11 +121,9 @@ _ship_report_object_environment() { } # ship_assert_staged_unchanged -# Preflight, run immediately before the commit: nothing between staging and here may touch the index, -# so this is an exact equality check. Catches a clobber that lands before the gate chain even starts. -# prepare_gate_worktree and link_untracked_gate_configs only write the WORKING TREE — symlinks at -# untracked paths, plus (sc-1489) one replacing a committed cache that would otherwise shadow the live -# one. Neither runs `git add`/`rm`, and the commit is index-only (no -a), so write-tree is invariant. +# Preflight, run immediately before the commit: nothing between staging and here may touch the index +# (prepare_gate_worktree and link_untracked_gate_configs write only the WORKING TREE), so this is +# an exact equality check. Catches a clobber that lands before the gate chain even starts. ship_assert_staged_unchanged() { local wt=$1 state=$2 expected actual expected=$(_ship_state_tree "$state") diff --git a/dist/cli/lib/ship/link-gate-configs.sh b/dist/cli/lib/ship/link-gate-configs.sh index 80be3d13..e4196a34 100644 --- a/dist/cli/lib/ship/link-gate-configs.sh +++ b/dist/cli/lib/ship/link-gate-configs.sh @@ -37,40 +37,29 @@ GATE_PROJECTION_FIXED_CANDIDATES=( .qavis/receipt.json ) -# Candidates whose bytes are a rebuildable, content-addressed CACHE rather than source. Committing one -# is always a mistake — the sha it attests covers a change set that stops being the one being shipped -# the instant anything else changes — but the link loop below skips any path the base checkout already -# put in $WT. So a cache that got committed by accident SILENTLY shadows the live one, and the gate -# that reads it can never be cleared by doing the work it asks for: in frink a tracked .qavis/receipt.json -# made every strict ship of a UI-affecting change unsatisfiable-by-compliance, GUARD_QAVIS_OK=1 the only -# exit, however many real `qavis qa` passes preceded it (sc-1489). These entries overwrite instead, loudly. +# Candidates that are a content-addressed CACHE, not source. A copy in the base commit is stale by +# construction — the sha it attests cannot cover the set being shipped — so it must lose to the live +# one, or the gate reading it can never be cleared by running the tool (sc-1489). GATE_PROJECTION_CACHE_CANDIDATES=( .qavis/receipt.json ) -gate_projection_is_cache_candidate() { - local rel=$1 cache +# gate_projection_is_stale_cache +# A cache candidate the BASE COMMIT put in $WT and that is still there: not a symlink (we placed that), +# not a path change-application already removed (a ship that untracks it), and not one absent from HEAD +# (change-application put those there from the invoking checkout — already the live bytes). +gate_projection_is_stale_cache() { + local wt=$1 rel=$2 cache for cache in "${GATE_PROJECTION_CACHE_CANDIDATES[@]}"; do - [ "$rel" = "$cache" ] && return 0 + [ "$rel" = "$cache" ] || continue + [ -L "$wt/$rel" ] && return 1 + [ -f "$wt/$rel" ] || return 1 + git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null && return 0 + return 1 done return 1 } -# gate_projection_cache_is_shadowed -# -# True only for the accident above: a cache candidate that the BASE CHECKOUT materialised, i.e. a real -# file git tracks at $WT's HEAD (the ship base — the commit is still ahead of us). Deliberately NOT: -# - a symlink — we placed it ourselves on an earlier pass; overwriting would be a no-op at best. -# - a path absent from HEAD — change-application put it there from the invoking checkout, so those -# bytes ARE the live ones. Nothing to prefer. -gate_projection_cache_is_shadowed() { - local wt=$1 rel=$2 - gate_projection_is_cache_candidate "$rel" || return 1 - [ -L "$wt/$rel" ] && return 1 - [ -f "$wt/$rel" ] || return 1 - git -C "$wt" cat-file -e "HEAD:$rel" 2>/dev/null -} - gate_config_path_emitter() { local self_dir emitter self_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -127,9 +116,9 @@ gate_projection_source_is_ignored() { # link_untracked_gate_configs [purpose] link_untracked_gate_configs() { local wt=$1 root=$2 purpose=${3:-ship} emitter resolved rel line index_rel='' candidate_manifest='' - local main_root='' candidate_root=$root source='' shadowed='' + local main_root='' candidate_root=$root source='' stale_hit='' local projection_manifest=${DEVKIT_REVIEW_PROJECTION_MANIFEST:-} projection_tool='' - local linked=() linked_sources=() linked_shadowed=() candidates=() + local linked=() linked_sources=() candidates=() stale=() case "$purpose" in ship | review | review-baseline) ;; *) @@ -203,7 +192,6 @@ link_untracked_gate_configs() { [ -e "$root/$rel" ] && [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue linked+=("$rel") linked_sources+=("$root/$rel") - linked_shadowed+=('') done { if [ "${#linked[@]}" -gt 0 ]; then @@ -215,31 +203,43 @@ link_untracked_gate_configs() { # Present in the repo but absent from the committed worktree = the gate would fail open. The # -L guard also skips a pre-existing symlink so `ln` never aborts on it. Empty local projection # dirs are unusable, so a populated main-worktree copy wins; files keep root-first precedence. + # Recorded BEFORE the source lookup so the diagnostic still prints when the operator's checkout + # has no live copy to link. A stale cache never enters `linked`: it is present in the committed + # tree, so that notice's wording and count would both be wrong for it. + stale_hit= + if gate_projection_is_stale_cache "$wt" "$rel"; then + rm -f "$wt/$rel" # worktree only; the shipped commit is asserted unchanged by this file's test + stale+=("$rel") + stale_hit=1 + fi source=$(gate_link_source "$root" "$main_root" "$rel" prefer-populated) || continue - shadowed='' - if [ -e "$wt/$rel" ] || [ -L "$wt/$rel" ]; then - gate_projection_cache_is_shadowed "$wt" "$rel" || continue - # WORKTREE-ONLY removal. The index is not touched: assert-staged-set.sh holds the ship to an - # exact `write-tree` equality across this window and the commit is index-only (no -a), so this - # changes what the GATES read and nothing about what is shipped. A caller who also shipped this - # path keeps their staged blob — the link just points the gate at the same live bytes. - rm -f "$wt/$rel" - shadowed=1 + if [ -z "$stale_hit" ]; then + [ ! -e "$wt/$rel" ] && [ ! -L "$wt/$rel" ] || continue + linked+=("$rel") + linked_sources+=("$source") fi mkdir -p "$wt/$(dirname "$rel")" ln -s "$source" "$wt/$rel" - linked+=("$rel") - linked_sources+=("$source") - linked_shadowed+=("$shadowed") done fi + if [ "${#stale[@]}" -gt 0 ]; then + { + echo "⚠️ ship: ${#stale[@]} gate cache(s) are COMMITTED, so the base checkout carried a stale copy" + echo " into the gate worktree — the live one was used instead. These are content-addressed: a" + echo " committed copy can never match the set being shipped. Untrack them and LAND it on the" + echo " base, or every ship repeats this. Use \`git rm\`, NOT \`--cached\` — a file left on disk" + echo " stages no deletion. Ship BOTH paths, ideally on their own so no QA gate is in the way:" + for rel in "${stale[@]}"; do + echo " - git rm $rel && printf '%s\\n' '$rel' >> .gitignore" + done + } >&2 + fi + # Guard the empty array BEFORE expanding it (stock-macOS bash 3.2 aborts on "${arr[@]}" when empty # under `set -u`; cf. commit-with-gate-capture.sh). [ "${#linked[@]}" -eq 0 ] && return 0 { - # "absent from" covers every entry but a shadowed cache, which is present-but-stale. The per-entry - # line below says which it was, so the count stays one number instead of two headline clauses. echo "⚠️ ship: ${#linked[@]} gate config(s) present in the repo but absent from the committed tree —" if is_review_projection_purpose "$purpose"; then echo " copied into the isolated review worktree so gates match the target (not defaults):" @@ -249,10 +249,7 @@ link_untracked_gate_configs() { local linked_index=0 for rel in "${linked[@]}"; do # `check-ignore -q` inside the `if` → its exit-1 "not ignored" is errexit-safe. - if [ -n "${linked_shadowed[$linked_index]:-}" ]; then - echo " - $rel (a COMMITTED copy shadowed the live cache — overridden for the gates only;" - echo " run \`git rm --cached $rel\` and gitignore it, or the gate reading it stays stale)" - elif gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then + if gate_projection_source_is_ignored "$root" "${linked_sources[$linked_index]}" "$rel"; then echo " - $rel (gitignored cache — normal)" else echo " - $rel (untracked — commit it so gates are consistent for everyone)" diff --git a/dist/cli/lib/ship/ship-branch.sh b/dist/cli/lib/ship/ship-branch.sh index 93729116..6d7fa6d0 100644 --- a/dist/cli/lib/ship/ship-branch.sh +++ b/dist/cli/lib/ship/ship-branch.sh @@ -250,7 +250,12 @@ done # vanished, publishing a devkit whose gate supervisor could not resolve its own import. # Every PATHS entry is caller-explicit (positional after --; directories rejected above), so # forcing them is precisely what was asked — same reasoning as reship.sh's `git add -f` (#199). +# A path the patch staged as DELETED is skipped: this pass exists to catch ignored files the diff +# MISSED, never to overrule one it expressed. Without the guard, deleting a tracked file whose +# gitignored copy is back on disk (a regenerable cache — sc-1489's receipt) silently re-adds it and +# the deletion can never land, however many times it is shipped. git -C "$ROOT" ls-files -o -i --exclude-standard -- "${PATHS[@]}" | while IFS= read -r f; do + git -C "$WT" diff --cached --quiet --diff-filter=D -- "$f" || continue mkdir -p "$WT/$(dirname "$f")" cp -Pp "$ROOT/$f" "$WT/$f" git -C "$WT" add -f -- "$f" diff --git a/docs/decisions/qavis-advisory-gate.md b/docs/decisions/qavis-advisory-gate.md index f5023b24..8cdb705d 100644 --- a/docs/decisions/qavis-advisory-gate.md +++ b/docs/decisions/qavis-advisory-gate.md @@ -29,3 +29,4 @@ created: 2026-07-07 - 2026-07-22 — Fail-open stays exit 0 but is now LOUD: when the advisory cannot run, the gate prints one stderr line naming WHY (qavis not on PATH / route failed / unparseable verdict) instead of returning a bare null that read exactly like SILENT. defaultRoute now returns a RouteResult whose null arm carries the reason, so there is one printer and no detail is discarded; ENOENT is discriminated from a qavis that ran and failed. Silent fail-open made a dead gate indistinguishable from 'nothing to QA'. - 2026-07-22 — devkit doctor now reports the advisory gate's liveness outside commit time: recipe present + qavis on PATH, printed in all three doctor modes. Advisory only — never a CheckResult and never a --fix target, since a repo that keeps the guard but skips installing qavis made a choice, not drift. Resolved against the git root, the cwd the husky fragment shells the gate from. Paid for the doctor.mts size ratchet (grandfathered at 894, shrink-only) by extracting the four synced-asset drift checks and the CheckResult primitive into cli/lib/doctor/ — which also let commit-msg-block.mts import the type instead of restating it structurally to dodge an import cycle. - 2026-07-28 — sc-1307: An ADVISE remedy now forces qavis qa --route vision because the gate's diff-aware classifier intentionally catches semantic UI impact in backend-only paths that qa's auto filename router can skip. A skip receipt was rejected: it would waive exactly those backend-to-UI cases. The classifier's deterministic launchability pre-gate guarantees the forced vision route is available. +- 2026-08-12 — sc-1489: A COMMITTED receipt now loses to the live one in the gate worktree. frink committed .qavis/receipt.json by accident; the ship link loop skips any path the base checkout already placed in the gate worktree, so the stale copy shadowed the live cache and qavis route compared the staged sha against staleness — two genuine "qavis qa --staged --route vision" passes still blocked, with GUARD_QAVIS_OK=1 the only exit. That is Rejected (c) reached from the other direction: the receipt mechanism was satisfiable, but a tracked copy silently disabled the clearing path. Ruling: GATE_PROJECTION_CACHE_CANDIDATES names the gate inputs that are content-addressed CACHES rather than source; for those, a base-committed copy is removed from the gate worktree and the live one linked over it, with a loud notice naming the untrack. The removal is worktree-only — the index is untouched and the ship commit is index-only, asserted by the test. REJECTED en route: aborting the ship on a committed cache. It reads as the stricter fix, but the guard reads the ship BASE, so no ship can clear itself — the untracking has to land on the base in a SEPARATE PR round-trip first, while every ship in the repo stays blocked. That is Rejected (c)'s shape (a block whose remedy the blocked path cannot carry), and it would also fire for consumers the advisory fail-opens for anyway. The notice prints "git rm", never "--cached": with "--cached" the file stays on disk, so the base-vs-worktree diff stages no deletion and ship's force-add pass for ignored paths re-commits it. Landing "git rm" needed a second fix, in ship-branch.sh: that force-add pass now SKIPS any path the applied patch staged as deleted. It exists to catch ignored files the diff missed, never to overrule one it expressed — and without the guard the realistic sequence silently defeats the remedy, because "git rm" also deletes the operator's live receipt, so they re-run the tool and the regenerated cache (now untracked AND gitignored) is swept back into the commit. That is general to ship, not cache-specific: it is what "devkit ship cannot express deleting the tracked receipt while the gitignored cache exists on disk at the same path" (the original report) actually was. reship.sh keeps its documented "current content of each path" semantics and is unchanged. Tests pin the resulting tree, with the regeneration step included.