From fd0dbdeea2aff124200d607533b977eee7e4140d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 14:13:50 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20adversarial=20round=20=E2=80=94=20the=20?= =?UTF-8?q?P0=20finalize=20fail-safe=20and=2012=20hardened=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three adversarial passes (Claude fresh-context, Codex chaos, Codex structured with P1 gate) on the full wave diff. Multi-source findings, all fixed: - P0: finalize_queue is now explicit-delete-only — a record is unlinked ONLY when classification proves it staged or dropped; a classifier crash, a missing class file, or a malformed pulled .brain-privacy-map.json (which previously nuked the whole snapshotted queue, remotely triggerable) now retains everything, warns, and re-drains next run. load_privacy_map treats corrupt maps as retain-all, never as empty. - next-version cannot silently drop a live claim: unreadable advertised refs get a targeted --depth=1 fetch + retry; still-unreadable claims surface as UNKNOWN warnings instead of duplicate-version silence. - session-update lock: ownership-checked EXIT trap (a TTL-reclaimed holder can no longer delete the new holder's lock) + a 5-min background heartbeat so a legitimately-slow pull/setup is never reclaimed while alive. - ensure-event collapses ALL same-(event,source) duplicates to one canonical entry; unique per-process tmp path; setup call sites surface (not swallow) the hardened refusals. - memory-ingest: --limit counts only policy-permitted pages (denied records no longer starve permitted ones); --probe applies the same policy filter as --bulk (skipped_policy_* fields on the report). - version-bump repair accepts a genuine literal 0.0.0.0 VERSION file. - slug heal restricted to the stray-.git shape — package.json-anchored wrapper roots keep their legit sticky identity (#2212 preserved). - brain-sync: idle fast path sees leftover .migrating records; unparseable spool records quarantine instead of warning forever; migration comment stops overclaiming the transition-window race. - CDP throttling justifications document override persistence (callers own restoration), pinned in the allowlist test. Deferred with record: deny retroactivity for already-ingested pages (P2 TODO, same semantics as the code-import gate); legacy-migration tail race (transition-window, requires pre-spool writers). 288 pass / 0 fail across the 10 touched suites. Co-Authored-By: Claude Fable 5 --- bin/gstack-brain-sync | 136 +++++++++++++----- bin/gstack-memory-ingest.ts | 104 +++++++++++--- bin/gstack-next-version | 39 +++++ bin/gstack-session-update | 23 ++- bin/gstack-settings-hook | 28 +++- bin/gstack-slug | 21 +-- bin/gstack-version-bump | 24 +++- browse/src/cdp-allowlist.ts | 4 +- browse/test/cdp-allowlist.test.ts | 4 + lib/bin-context.ts | 25 +++- setup | 14 +- test/bin-context-windows-slug.test.ts | 29 ++++ test/brain-sync.test.ts | 91 ++++++++++-- test/gstack-memory-ingest.test.ts | 85 ++++++++++- test/gstack-next-version.test.ts | 96 +++++++++++++ .../gstack-settings-hook-schema-aware.test.ts | 71 +++++++++ test/gstack-slug-parity.test.ts | 22 +++ test/gstack-version-bump.test.ts | 28 ++++ test/session-update-autostash.test.ts | 30 +++- test/timeline-stop-hook.test.ts | 14 ++ 20 files changed, 789 insertions(+), 99 deletions(-) diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 2f81554b6..0462c1ce6 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -140,8 +140,10 @@ spool_has_records() { # os.replace, one file per line). Reads the file TWICE before unlinking: a # pre-rename writer can still append through its already-open fd after our # rename, and those appends land in the renamed file — the second pass -# catches them (the tail race the shared-file design could never close). -# Unparseable lines migrate as-is; finalize_queue keeps + warns on them. +# NARROWS the tail-race window (transition-only: it applies to pre-spool +# writers, and a writer that appends after the second read but before the +# unlink can still lose that line; spool-native writers are immune). +# Unparseable lines migrate as-is; finalize_queue quarantines + warns on them. convert_legacy_file() { local legacy="$1" python3 - "$legacy" "$QUEUE_DIR" <<'PYEOF' 2>/dev/null || true @@ -244,16 +246,29 @@ def load_lines(path): return [] def load_privacy_map(path): + # Returns (entries, corrupt). Non-dict entries are filtered out + # defensively — the map may be PULLED from the artifacts remote, so a + # malformed entry like ["bad"] is remotely triggerable and used to raise + # mid-classification (after the snapshot manifest was written), which the + # old finalize turned into a full queue wipe. Any malformed shape also + # marks the map CORRUPT: privacy classification cannot be trusted, so the + # caller holds every queued record instead of guessing (a corrupt privacy + # map silently treated as empty would over-share behavioral data). try: with open(path) as f: data = json.load(f) - # Expected: [{"pattern": "glob", "class": "artifact" | "behavioral"}] - return data if isinstance(data, list) else [] - except (FileNotFoundError, json.JSONDecodeError): - return [] + except FileNotFoundError: + return [], False + except json.JSONDecodeError: + return [], True + if not isinstance(data, list): + return [], True + # Expected: [{"pattern": "glob", "class": "artifact" | "behavioral"}] + entries = [e for e in data if isinstance(e, dict)] + return entries, len(entries) != len(data) allowlist_globs = load_lines(allowlist_path) -privacy_map = load_privacy_map(privacy_path) +privacy_map, privacy_corrupt = load_privacy_map(privacy_path) # Normalize skip entries to the POSIX form queued paths use, so a backslash # entry in .brain-skip.txt still matches on Windows. The drain is the safety # boundary that actually stages files, so it must normalize identically to @@ -318,6 +333,14 @@ def mode_allows(cls, mode): final = [] classified = {"retained": [], "dropped": {"skipped": [], "invalid": [], "unmatched": [], "missing": []}} +if privacy_corrupt: + # Fail-safe: with an untrustworthy privacy map, stage NOTHING and drop + # NOTHING — retain every queued record until the map is fixed. The next + # drain re-classifies from scratch. + print("BRAIN_SYNC: warning: privacy map at " + privacy_path + + " is malformed — holding all queued records until it is fixed", file=sys.stderr) + classified["retained"] = sorted(queue_paths) + queue_paths = set() for p in sorted(queue_paths): if p in skip_lines: classified["dropped"]["skipped"].append(p) @@ -353,25 +376,34 @@ PYEOF } # Finalize the drain: delete exactly the spool record files this drain -# consumed (per the snapshot manifest), keeping retained (privacy/mode-held) -# and unparseable records queued. The predecessor (a shared-file queue -# rewrite) had a lockless-append race between its live re-read and the -# os.replace; with one file per record that race class is structurally gone — -# a concurrent enqueue is a separate file the snapshot never listed, so -# finalize cannot touch it. Crash semantics are at-least-once: a drain that -# dies before finalize leaves its spool files in place and the next run -# re-drains them; downstream content-hash dedup absorbs the duplicates. -# Dropped-path detail goes to a 0600 sidecar so the status line can stay -# content-free (counts only). +# consumed (per the snapshot manifest) AND positively classified. Deletion is +# EXPLICIT-DELETE-ONLY: a record is unlinked only when its path appears in +# (staged paths ∪ classified dropped). The old polarity ("delete unless +# retained") turned a missing/unparseable classification into retained=∅ and +# wiped every snapshotted record — remotely triggerable via a malformed +# pulled privacy map that raised AFTER the manifest write. Now a +# missing/unparseable class_file or paths_file deletes NOTHING (warn + +# return), and a path the classification never mentions stays queued. +# The predecessor (a shared-file queue rewrite) had a lockless-append race +# between its live re-read and the os.replace; with one file per record that +# race class is structurally gone — a concurrent enqueue is a separate file +# the snapshot never listed, so finalize cannot touch it. Crash semantics are +# at-least-once: a drain that dies before finalize leaves its spool files in +# place and the next run re-drains them; downstream content-hash dedup +# absorbs the duplicates. Unparseable records move to $QUEUE_DIR/quarantine/ +# (never deleted) so they stop re-warning at every boundary. Dropped-path +# detail goes to a 0600 sidecar so the status line can stay content-free +# (counts only). finalize_queue() { local snapshot_file="$1" # spool filenames this drain consumed, one per line local class_file="$2" # classification JSON from compute_paths_to_stage + local paths_file="$3" # staged paths (compute_paths_to_stage stdout), one per line # Fail-open by design (a failed finalize self-corrects next run: re-stage → # nothing-to-commit), but say so — a silent failure here would let the # subsequent "ok/idle" status claim a drain that did not happen. - python3 - "$QUEUE_DIR" "$snapshot_file" "$class_file" "$GSTACK_HOME/.brain-sync-drops.json" <<'PYEOF' || echo "BRAIN_SYNC: warning: queue finalize failed — entries retained; next run re-drains" >&2 + python3 - "$QUEUE_DIR" "$snapshot_file" "$class_file" "$paths_file" "$GSTACK_HOME/.brain-sync-drops.json" <<'PYEOF' || echo "BRAIN_SYNC: warning: queue finalize failed — entries retained; next run re-drains" >&2 import json, os, sys, time -spool_dir, snapshot_file, class_file, drops_file = sys.argv[1:5] +spool_dir, snapshot_file, class_file, paths_file, drops_file = sys.argv[1:6] def lines(path): try: @@ -380,15 +412,26 @@ def lines(path): except FileNotFoundError: return [] +# Explicit-delete-only inputs. Either input unreadable → delete NOTHING. try: with open(class_file) as f: classified = json.load(f) + if not isinstance(classified, dict): + raise ValueError("classification is not an object") except Exception: - classified = {"retained": [], "dropped": {}} -retained = set(classified.get("retained", [])) + print("BRAIN_SYNC: warning: classification unreadable — no queue records deleted; next run re-drains", file=sys.stderr) + sys.exit(0) +try: + with open(paths_file) as f: + staged = {l.strip() for l in f if l.strip()} +except Exception: + print("BRAIN_SYNC: warning: staged-paths file unreadable — no queue records deleted; next run re-drains", file=sys.stderr) + sys.exit(0) + dropped = set() for group in (classified.get("dropped", {}) or {}).values(): dropped.update(group) +deletable = staged | dropped unparseable = 0 for name in lines(snapshot_file): @@ -404,16 +447,24 @@ for name in lines(snapshot_file): except Exception: pass if not isinstance(p, str): - unparseable += 1 # keep — never destroy what we can't read + # Never destroy what we can't read — but don't leave it re-warning at + # every boundary either: move it aside for inspection. + unparseable += 1 + try: + qdir = os.path.join(spool_dir, "quarantine") + os.makedirs(qdir, exist_ok=True) + os.replace(full, os.path.join(qdir, name)) + except OSError: + pass # quarantine move failed — leave in place; next run retries continue - if p in retained: - continue # stays queued: syncs under a higher mode + if p not in deletable: + continue # retained / unclassified: stays queued (explicit-delete-only) try: os.unlink(full) # staged or dropped: fully processed except FileNotFoundError: pass if unparseable: - print(f"BRAIN_SYNC: {unparseable} unparseable spool record(s) held (inspect {spool_dir})", file=sys.stderr) + print(f"BRAIN_SYNC: {unparseable} unparseable spool record(s) moved to quarantine (inspect {os.path.join(spool_dir, 'quarantine')})", file=sys.stderr) if dropped: fd = os.open(drops_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) @@ -573,11 +624,13 @@ subcmd_once() { # nothing to classify, retain, or drop, and a record created after this # check simply waits for the next boundary. The legacy file is checked too: # an OLD writer may have recreated it after the migration above (it gets - # migrated next run, but the depth is honest now). (The detector above - # already ran: its whole point is re-pushing stranded commits when the - # queue is empty.) The lock-release trap installed at acquisition covers - # this exit. - if ! spool_has_records && [ ! -s "$QUEUE" ]; then + # migrated next run, but the depth is honest now) — and so is a leftover + # .migrating file: if its conversion failed above (e.g. python3 missing), + # records are still pending, so "idle" would be dishonest. (The detector + # above already ran: its whole point is re-pushing stranded commits when + # the queue is empty.) The lock-release trap installed at acquisition + # covers this exit. + if ! spool_has_records && [ ! -s "$QUEUE" ] && [ ! -s "$QUEUE.migrating" ]; then write_status "idle" "queue empty" exit 0 fi @@ -589,11 +642,20 @@ subcmd_once() { # Single trap covers all: lock cleanup AND tempfile cleanup. trap 'rm -f "$paths_file" "$class_file" "$snapshot_file" 2>/dev/null; rm -rf "$lock_dir" 2>/dev/null || true' EXIT INT TERM - compute_paths_to_stage "$mode" "$class_file" "$snapshot_file" > "$paths_file" + # Fail-safe (G1): a classifier that dies mid-run (ENOSPC/OOM/SIGKILL, or a + # shape the defensive filters don't cover) may have already written the + # snapshot manifest but no classification. Finalizing on that state is what + # used to wipe the queue — so on a nonzero exit, warn loudly, do NOT call + # finalize_queue, and leave everything queued for the next drain. + if ! compute_paths_to_stage "$mode" "$class_file" "$snapshot_file" > "$paths_file"; then + echo "BRAIN_SYNC: warning: queue classification failed — no records consumed; next run re-drains" >&2 + write_status "error" "classification failed; queue preserved (next run retries)" + exit 0 + fi if [ ! -s "$paths_file" ]; then # Nothing stageable. Finalize the snapshot (retained entries survive; # classified drops removed; records created after the snapshot untouched). - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" local summary summary=$(queue_summary "$class_file") write_status "idle" "no stageable changes${summary:+ ($summary)}" @@ -645,7 +707,7 @@ subcmd_once() { commit -q -m "$msg" 2>/dev/null || { # Nothing to commit (e.g. all files already committed). The drained # records leave the spool; retained + post-snapshot records survive. - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" write_status "idle" "queue drained but no new changes to commit" exit 0 } @@ -662,7 +724,7 @@ subcmd_once() { # Drained records leave the spool — they live in the local commit, which # the run-start detector re-pushes next time (#2549). Retained + # post-snapshot records survive the finalize. - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" exit 0 fi @@ -676,7 +738,7 @@ subcmd_once() { if git -C "$GSTACK_HOME" merge --no-edit "origin/$branch" >/dev/null 2>&1; then if GSTACK_HOME="$GSTACK_HOME" _receipted_git closed brain-sync "$push_host" curated-memory-git-push "artifacts_sync_mode!=off" \ bash -c 'git -C "$1" push origin HEAD 2>/dev/null' _ "$GSTACK_HOME"; then - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" date -u +%Y-%m-%dT%H:%M:%SZ > "$LAST_PUSH_FILE" write_status "ok" "pushed $n file(s) after rebase" exit 0 @@ -685,12 +747,12 @@ subcmd_once() { fi # Commit exists locally; the run-start detector re-pushes it next time. write_status "push_failed" "push failed: $(printf '%s' "$push_err" | head -1); commit retained locally, will retry next run" - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" exit 0 } # Success: drained records leave the spool (retained + post-snapshot survive). - finalize_queue "$snapshot_file" "$class_file" + finalize_queue "$snapshot_file" "$class_file" "$paths_file" date -u +%Y-%m-%dT%H:%M:%SZ > "$LAST_PUSH_FILE" write_status "ok" "pushed $n file(s)" exit 0 diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index 4fdb2cdfa..f38c0ad71 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -143,6 +143,13 @@ interface ProbeReport { updated_count: number; unchanged_count: number; skipped_unattributed: number; + /** + * #2392 parity: transcripts whose remote's trust tier is `deny` / + * `read-only`. Probe applies the SAME per-remote policy filter --bulk + * applies, so its ingestible counts match what --bulk would write. + */ + skipped_policy_deny: number; + skipped_policy_readonly: number; estimate_minutes: number; } @@ -1079,9 +1086,10 @@ export function readNewFailures( /** * The ONE attribution gate (#2394): a transcript is attributable iff its cwd - * resolves to a git remote. Both probeMode (via transcriptIsAttributable) and - * preparePages route through THIS function, so the two stages' post-attribution - * counts are structurally identical — the parity the probe report promises. + * resolves to a git remote. Both probeMode (via transcriptCwdFromPrefix + + * resolveGitRemote — the same memoized resolver) and preparePages route + * through THIS logic, so the two stages' post-attribution counts are + * structurally identical — the parity the probe report promises. */ function sessionIsAttributable(cwd: string | undefined | null): boolean { if (!cwd) return false; @@ -1096,12 +1104,12 @@ function sessionIsAttributable(cwd: string | undefined | null): boolean { const TRANSCRIPT_PROBE_MAX_BYTES = 256 * 1024; /** - * Lightweight attribution check: does a transcript have a resolvable git - * remote for its cwd? Reads a BOUNDED prefix (first 256KB, never the whole - * file — plan C7: the probe must stay a cheap parse on multi-MB transcripts), - * extracts the cwd with EXACTLY parseTranscriptJsonl's rules, and calls - * resolveGitRemote. Avoids the full parse (body rendering, message counting) - * because probe only needs the yes/no answer. + * Lightweight cwd extraction for the probe: reads a BOUNDED prefix (first + * 256KB, never the whole file — plan C7: the probe must stay a cheap parse on + * multi-MB transcripts) and extracts the cwd with EXACTLY + * parseTranscriptJsonl's rules. The caller resolves attribution/policy via + * resolveGitRemote (memoized). Avoids the full parse (body rendering, message + * counting) because probe only needs the cwd. * * Extraction MIRRORS parseTranscriptJsonl (the single source of truth for * cwd semantics — keep the two in lockstep): @@ -1117,7 +1125,7 @@ const TRANSCRIPT_PROBE_MAX_BYTES = 256 * 1024; * Non-transcript types (artifacts) always pass — the attribution filter in * preparePages only applies to transcripts (#2394). */ -function transcriptIsAttributable(path: string): boolean { +function transcriptCwdFromPrefix(path: string): string { let raw: string; try { const fd = openSync(path, "r"); @@ -1129,10 +1137,10 @@ function transcriptIsAttributable(path: string): boolean { closeSync(fd); } } catch { - return false; + return ""; } const lines = raw.split("\n").filter((l) => l.trim().length > 0); - if (lines.length === 0) return false; + if (lines.length === 0) return ""; let cwd = ""; let sawFirstParseable = false; @@ -1159,8 +1167,7 @@ function transcriptIsAttributable(path: string): boolean { break; } } - if (!cwd) return false; - return sessionIsAttributable(cwd); + return cwd; } async function probeMode(args: CliArgs): Promise { @@ -1184,17 +1191,55 @@ async function probeMode(args: CliArgs): Promise { let updatedCount = 0; let unchangedCount = 0; let skippedUnattributed = 0; + let skippedPolicyDeny = 0; + let skippedPolicyReadonly = 0; + // Two-phase walk (#2392 parity): collect candidates first (remembering each + // transcript's resolved remote), THEN apply the same per-remote policy + // filter --bulk applies via one repoPolicyTierBatch spawn. Counting during + // the walk would report policy-denied transcripts as ingestible — probe's + // numbers must match what --bulk would actually write. + const candidates: Array<{ path: string; type: MemoryType; remote: string }> = []; for (const { path, type } of walkAllSources(ctx)) { // Apply the same attribution filter preparePages uses (#2394): // skip transcripts with no resolvable git remote unless --include-unattributed. - if (type === "transcript" && !args.includeUnattributed) { - if (!transcriptIsAttributable(path)) { + let remote = ""; + if (type === "transcript") { + const cwd = transcriptCwdFromPrefix(path); + remote = cwd ? resolveGitRemote(cwd) : ""; + if (!args.includeUnattributed && remote === "") { skippedUnattributed++; continue; } } + candidates.push({ path, type, remote }); + } + // Batch policy check — same hasRepoPolicyStore fast path as preparePages: + // no store on disk → zero policy work. Only transcripts with a resolved + // remote are policy-filtered; artifacts never are (#2392). A missing or + // errored verdict counts as "none" here — probe is read-only and must not + // hard-fail the way the write path does. + if (hasRepoPolicyStore()) { + const remotes = [...new Set(candidates.filter((c) => c.type === "transcript" && c.remote).map((c) => c.remote))]; + if (remotes.length > 0) { + const verdicts = repoPolicyTierBatch(remotes); + for (let i = candidates.length - 1; i >= 0; i--) { + const c = candidates[i]; + if (c.type !== "transcript" || !c.remote) continue; + const tier = verdicts.get(c.remote)?.tier ?? "none"; + if (tier === "deny") { + skippedPolicyDeny++; + candidates.splice(i, 1); + } else if (tier === "read-only") { + skippedPolicyReadonly++; + candidates.splice(i, 1); + } + } + } + } + + for (const { path, type } of candidates) { totalFiles++; let size = 0; try { @@ -1224,6 +1269,8 @@ async function probeMode(args: CliArgs): Promise { updated_count: updatedCount, unchanged_count: unchangedCount, skipped_unattributed: skippedUnattributed, + skipped_policy_deny: skippedPolicyDeny, + skipped_policy_readonly: skippedPolicyReadonly, estimate_minutes: estimateMinutes, }; } @@ -1277,8 +1324,16 @@ function preparePages( let parseFailed = 0; let partialPages = 0; + // --limit semantics: "stop after N pages WRITTEN" = N policy-eligible pages. + // When a per-remote policy store exists, eligibility is only known after the + // batch policy check below, so the walk must not stop early — a denied-first + // corpus would otherwise consume the limit and starve permitted pages. With + // no store on disk, every prepared page is eligible and the in-loop break + // keeps --limit cheap. + const policyStoreExists = hasRepoPolicyStore(); + for (const { path, type } of walkAllSources(ctx)) { - if (args.limit !== null && prepared.length >= args.limit) break; + if (args.limit !== null && !policyStoreExists && prepared.length >= args.limit) break; if (args.mode === "incremental" && !fileChangedSinceState(path, state)) { skippedDedup++; @@ -1354,7 +1409,7 @@ function preparePages( let skippedPolicyReadonly = 0; let skippedPolicyDeny = 0; let policyError: string | undefined; - if (hasRepoPolicyStore()) { + if (policyStoreExists) { const remotes = [ ...new Set( prepared @@ -1399,6 +1454,13 @@ function preparePages( } } + // --limit applies AFTER policy filtering, over permitted pages only. In the + // no-store fast path the walk already stopped at the limit, so this slice + // is a no-op there. + if (args.limit !== null && finalPrepared.length > args.limit) { + finalPrepared = finalPrepared.slice(0, args.limit); + } + return { prepared: finalPrepared, skippedSecret, @@ -2282,6 +2344,12 @@ function printProbeReport(r: ProbeReport, json: boolean): void { if (r.skipped_unattributed > 0) { console.log(`Skipped (unattributed): ${r.skipped_unattributed} (no git remote; use --include-unattributed to include)`); } + if (r.skipped_policy_deny > 0) { + console.log(`Skipped (policy deny): ${r.skipped_policy_deny} (remote tier is deny; change with: gstack-gbrain-repo-policy set read-write)`); + } + if (r.skipped_policy_readonly > 0) { + console.log(`Skipped (policy read-only): ${r.skipped_policy_readonly} (remote tier is read-only; transcript ingest writes pages)`); + } console.log("By type:"); for (const [t, v] of Object.entries(r.by_type)) { if (v.count > 0) { diff --git a/bin/gstack-next-version b/bin/gstack-next-version index 3820edb9c..990567c6c 100755 --- a/bin/gstack-next-version +++ b/bin/gstack-next-version @@ -552,6 +552,45 @@ function fetchGitClaimed( // read from the local remote-tracking ref. show = runCommand("git", ["show", `refs/remotes/origin/${branch}:${versionPath}`]); } + if (!show.ok && sha) { + // ls-remote advertises SHAs without objects: a branch pushed after our + // last fetch has NO local object, so both reads above fail. The old + // `continue` here silently dropped a LIVE claim — the exact duplicate- + // allocation this fallback exists to prevent. Distinguish "object + // missing" from "branch has no VERSION file" before deciding. + const haveObject = runCommand("git", ["cat-file", "-e", sha]); + if (haveObject.ok) { + // Object is local and the path read still failed → the branch simply + // carries no version file. Genuinely not a claim; skip quietly. + continue; + } + // Fetch just this ref shallowly (no prompts, no tags, bounded) and + // retry reading VERSION from the now-local object (or FETCH_HEAD). + const fetch = spawnSync( + "git", + ["fetch", "origin", `refs/heads/${branch}`, "--depth=1", "--no-tags"], + { encoding: "utf8", timeout: 10000, env: { ...process.env, GIT_TERMINAL_PROMPT: "0" } }, + ); + if (fetch.status === 0 && !fetch.error) { + show = runCommand("git", ["show", `${sha}:${versionPath}`]); + if (!show.ok) show = runCommand("git", ["show", `FETCH_HEAD:${versionPath}`]); + if (!show.ok && runCommand("git", ["cat-file", "-e", sha]).ok) { + // Fetched and the object exists but the path doesn't → no VERSION + // file on this branch. Not a claim. + continue; + } + } + if (!show.ok) { + // STILL unreadable — never skip silently. Surface it as an UNKNOWN + // claim so the caller knows the allocation may be unsafe. + warnings.push( + `origin/${branch}: VERSION unreadable even after a targeted fetch — ` + + `counted as an UNKNOWN claim; allocation may collide with this branch. ` + + `Run \`git fetch origin ${branch}\` and re-run to verify.`, + ); + continue; + } + } if (!show.ok) continue; const raw = extractVersion(show.stdout, versionPath); if (!raw || !parseVersion(raw)) continue; diff --git a/bin/gstack-session-update b/bin/gstack-session-update index 692104b7c..15f8caa87 100755 --- a/bin/gstack-session-update +++ b/bin/gstack-session-update @@ -106,11 +106,26 @@ fi # Write the HOLDER's PID for stale lock detection (see #2613 note above; # macOS ships bash 3.2 with no BASHPID — the sh child's $PPID IS this - # subshell, so the fallback is exact there). - echo "${BASHPID:-$(sh -c 'echo $PPID')}" > "$LOCK_DIR/pid" 2>/dev/null + # subshell, so the fallback is exact there). MYPID is captured once at + # write time so the trap below can prove ownership before removing. + MYPID="${BASHPID:-$(sh -c 'echo $PPID')}" + echo "$MYPID" > "$LOCK_DIR/pid" 2>/dev/null - # Clean up lock on exit - trap 'rm -rf "$LOCK_DIR" 2>/dev/null' EXIT + # In-flight heartbeat: the step-boundary touches below only fire AFTER the + # pull / setup return, so a legitimately-slow step (cold clone, huge setup) + # older than the TTL got reclaimed while ALIVE. This background loop + # freshens the pidfile mtime every 5 minutes for as long as we still own + # the lock (ownership re-checked each beat: if another updater reclaimed + # and wrote its own pid, the loop exits instead of touching THEIR file). + ( while :; do sleep 300; [ "$(cat "$LOCK_DIR/pid" 2>/dev/null)" = "$MYPID" ] || exit 0; touch "$LOCK_DIR/pid" 2>/dev/null; done ) & + HB_PID=$! + + # Clean up lock on exit — ownership-checked: after a TTL reclaim by another + # updater, $LOCK_DIR belongs to the NEW holder, and an unconditional rm -rf + # here would delete the live holder's lock (cascading reclaims). Remove the + # lock ONLY while $LOCK_DIR/pid still contains MYPID; always stop the + # heartbeat. + trap 'kill "$HB_PID" 2>/dev/null; [ "$(cat "$LOCK_DIR/pid" 2>/dev/null)" = "$MYPID" ] && rm -rf "$LOCK_DIR" 2>/dev/null' EXIT # ── Pull latest ── OLD_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index 7b6d58200..057c7207b 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -104,7 +104,7 @@ case "$ACTION" in hooks: [{ type: "command", command: hookCmd }] }); } - const tmp = settingsPath + ".tmp"; + const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); fs.renameSync(tmp, settingsPath); ' @@ -130,7 +130,7 @@ case "$ACTION" in if (settings.hooks.SessionStart.length === 0) delete settings.hooks.SessionStart; if (Object.keys(settings.hooks).length === 0) delete settings.hooks; } - const tmp = settingsPath + ".tmp"; + const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); fs.renameSync(tmp, settingsPath); ' 2>/dev/null @@ -218,7 +218,19 @@ case "$ACTION" in return sameMatcher && sameCommand; }; - let existing = settings.hooks[event].find(matchesEntry); + // Collect ALL matches, not just the first: pre-existing installs can + // carry two entries with the same (event, _gstack_source) from the old + // matcher-keyed dedup. `.find()` updated only the first and left the + // stale twin running forever. Keep ONE canonical entry (the first), + // remove the rest in the same atomic write. + const matched = settings.hooks[event].filter(matchesEntry); + let existing = matched.length > 0 ? matched[0] : undefined; + let collapsed = 0; + if (matched.length > 1) { + const extras = new Set(matched.slice(1)); + settings.hooks[event] = settings.hooks[event].filter((e) => !extras.has(e)); + collapsed = matched.length - 1; + } const hookEntry = { type: "command", command: cmd }; if (timeoutRaw) { const n = Number(timeoutRaw); @@ -270,7 +282,10 @@ case "$ACTION" in // Atomic tmp+rename: the settings file is either the old JSON (with // the old single registration) or the new JSON (with the replaced // one) — a failed update can never leave zero or two registrations. - const tmp = settingsPath + ".tmp"; + // Per-process tmp suffix: a fixed settings.json.tmp let two parallel + // writers consume one another. (No apostrophes here: this JS lives + // inside a bash single-quoted string.) + const tmp = settingsPath + ".tmp." + process.pid; fs.writeFileSync(tmp, after + "\n"); fs.renameSync(tmp, settingsPath); } catch (e) { @@ -280,6 +295,9 @@ case "$ACTION" in console.error("error: could not update " + settingsPath + ": " + (e && e.message ? e.message : e)); process.exit(1); } + if (collapsed > 0) { + console.error("collapsed " + collapsed + " duplicate (event, source) hook entr" + (collapsed === 1 ? "y" : "ies") + " for " + event + " (source: " + source + ")"); + } if (ensure && existing) { console.log("OK: " + event + " hook re-pointed (source: " + source + ")"); } else { @@ -318,7 +336,7 @@ case "$ACTION" in if (settings.hooks[event].length === 0) delete settings.hooks[event]; } if (Object.keys(settings.hooks).length === 0) delete settings.hooks; - const tmp = settingsPath + ".tmp"; + const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); fs.renameSync(tmp, settingsPath); console.log("OK: removed " + removed + " hook entry/entries tagged source=" + source); diff --git a/bin/gstack-slug b/bin/gstack-slug index ede341e4c..12f0ed397 100755 --- a/bin/gstack-slug +++ b/bin/gstack-slug @@ -188,12 +188,16 @@ _resolve_remote() { # - Old-bug shape (#1125): the pre-walk-up resolver cached basename(pwd) # for a SUBDIRECTORY of the real project — cached == pwd basename while # the walk-up says pwd is NOT the project root. -# - Degraded-ancestor shape (2026-08-17): the pre-remote-first resolver -# cached basename(PROJECT_ROOT) for a marker-only ancestor (stray -# ~/.git) that is NOT the remote-bearing repo — cached == the marker -# root's basename while a remote-bearing repo BELOW it exists. Legit -# #2212 stickiness is safe: there the repo that adopted the remote IS -# the marker root (REMOTE_ROOT == PROJECT_ROOT), so the heal never fires. +# - Degraded-ancestor shape (2026-08-17), STRAY-REPO shape ONLY: the +# pre-remote-first resolver cached basename(PROJECT_ROOT) for an +# ancestor anchored by a .git entry whose origin does NOT resolve (the +# stray empty ~/.git live bug) while a remote-bearing repo BELOW it +# exists. A marker root anchored by package.json / pyproject etc. with +# NO .git is legit #2212 sticky identity (a monorepo wrapper that used +# gstack before its inner dir grew a remote) and must NOT be healed. +# Legit remote-adopting stickiness is safe too: there the repo that +# adopted the remote IS the marker root (REMOTE_ROOT == PROJECT_ROOT), +# so the heal never fires. if [[ -z "$SLUG" && -f "$CACHE_FILE" ]]; then _CACHED=$(cat "$CACHE_FILE" 2>/dev/null | tr -cd 'a-zA-Z0-9._-') if [[ -n "$_CACHED" ]]; then @@ -204,9 +208,10 @@ if [[ -z "$SLUG" && -f "$CACHE_FILE" ]]; then fi if [[ "$_CACHED" == "$_PWD_BASE" && -n "$PROJECT_ROOT" && "$PROJECT_ROOT" != "$PROJECT_DIR" ]]; then : # old-bug shape — recompute below and self-heal the cache - elif [[ -n "$PROJECT_ROOT" && "$_CACHED" == "$_ROOT_BASE" ]] \ + elif [[ -n "$PROJECT_ROOT" && "$_CACHED" == "$_ROOT_BASE" && -e "$PROJECT_ROOT/.git" ]] \ + && ! git -C "$PROJECT_ROOT" remote get-url origin >/dev/null 2>&1 \ && { _resolve_remote; [[ -n "$REMOTE_URL" && "$REMOTE_ROOT" != "$PROJECT_ROOT" ]]; }; then - : # degraded-ancestor shape — recompute below and self-heal the cache + : # degraded-ancestor (stray-repo) shape — recompute below and self-heal the cache else SLUG="$_CACHED" fi diff --git a/bin/gstack-version-bump b/bin/gstack-version-bump index 6717272bc..4cc22501f 100755 --- a/bin/gstack-version-bump +++ b/bin/gstack-version-bump @@ -463,13 +463,25 @@ function cmdRepair(args: string[], cwd: string): void { const current = readVersionFile(versionPath, versionRel); // Guard against readVersionFile folding "file exists but is empty / unparsable" // into DEFAULT ("0.0.0.0") — same data-corruption pathway as file-missing (#2600). - // A fabricated version must never propagate into package.json. + // A fabricated version must never propagate into package.json. But DEFAULT is + // ambiguous: a VERSION file that GENUINELY reads "0.0.0.0" (a brand-new repo) + // is a legitimate version, not the sentinel. Disambiguate on the raw bytes: + // if the trimmed file content itself matches the version shape, proceed with + // the repair; reject only when the raw content is empty or unparseable. if (current === DEFAULT) { - fail( - `VERSION file at ${versionRel} is empty or contains no parsable version. ` + - "Cannot repair package.json with a fabricated version.", - 2, - ); + let rawTrimmed = ""; + try { + rawTrimmed = readFileSync(versionPath, "utf-8").trim(); + } catch { + rawTrimmed = ""; + } + if (!VERSION_RE.test(rawTrimmed)) { + fail( + `VERSION file at ${versionRel} is empty or contains no parsable version. ` + + "Cannot repair package.json with a fabricated version.", + 2, + ); + } } if (!VERSION_RE.test(current)) { fail( diff --git a/browse/src/cdp-allowlist.ts b/browse/src/cdp-allowlist.ts index b4faa46ff..752ce464c 100644 --- a/browse/src/cdp-allowlist.ts +++ b/browse/src/cdp-allowlist.ts @@ -167,14 +167,14 @@ export const CDP_ALLOWLIST: ReadonlyArray = Object.freeze([ method: 'setCPUThrottlingRate', scope: 'tab', output: 'trusted', - justification: 'CPU slowdown multiplier on the active tab, for measuring performance on a realistic low-end client instead of the developer workstation. Same domain and mutating character as setDeviceMetricsOverride; affects only timing, reads nothing, exfiltrates nothing.', + justification: 'CPU slowdown multiplier on the active tab, for measuring performance on a realistic low-end client instead of the developer workstation. Same domain and mutating character as setDeviceMetricsOverride; affects only timing, reads nothing, exfiltrates nothing. NOTE: like setEmulatedMedia the override persists on the tab until cleared (rate: 1) — callers own restoration.', }, { domain: 'Network', method: 'emulateNetworkConditions', scope: 'tab', output: 'trusted', - justification: 'Bandwidth/latency emulation on the active tab, for measuring page behaviour on a slow connection. Constrains traffic rather than reading it — no request bodies, headers or cookies are exposed.', + justification: 'Bandwidth/latency emulation on the active tab, for measuring page behaviour on a slow connection. Constrains traffic rather than reading it — no request bodies, headers or cookies are exposed. NOTE: like setEmulatedMedia the override persists on the tab until cleared (offline: false plus default throughput/latency) — callers own restoration.', }, // ─── Page capture (output, not navigation) ───────────────── { diff --git a/browse/test/cdp-allowlist.test.ts b/browse/test/cdp-allowlist.test.ts index 0693781a1..8256e7a19 100644 --- a/browse/test/cdp-allowlist.test.ts +++ b/browse/test/cdp-allowlist.test.ts @@ -92,6 +92,10 @@ describe('CDP allowlist (T2: deny-default)', () => { expect(e).not.toBeNull(); expect(e!.scope).toBe('tab'); expect(e!.output).toBe('trusted'); + // Like setEmulatedMedia, both overrides persist on the tab until + // cleared (rate: 1 / offline: false + defaults) — the justification + // must say so, since callers own restoration. + expect(e!.justification).toContain('persists on the tab until cleared'); } }); diff --git a/lib/bin-context.ts b/lib/bin-context.ts index 204f34f7f..d3be44009 100644 --- a/lib/bin-context.ts +++ b/lib/bin-context.ts @@ -129,12 +129,16 @@ export function outermostRemoteRepo(startDir: string): { root: string; url: stri * - old-bug shape (#1125): cached value equals basename(cwd) while the * walk-up says cwd is NOT the project root; that cache came from the * pre-walk-up resolver, so recompute and heal. - * - degraded-ancestor shape (2026-08-17): cached equals the marker root's - * basename while a remote-bearing repo BELOW the marker root exists — - * the pre-remote-first resolver degraded to a stray ancestor's basename - * (stray empty ~/.git → SLUG=). Legit #2212 stickiness is - * safe: there the repo that adopted the remote IS the marker root - * (remote root == project root), so the heal never fires. + * - degraded-ancestor shape (2026-08-17), STRAY-REPO shape ONLY: cached + * equals the marker root's basename, the marker root is anchored by a + * .git entry whose origin does NOT resolve (the stray empty ~/.git + * live bug), and a remote-bearing repo BELOW it exists — the + * pre-remote-first resolver degraded to that stray ancestor's basename + * (SLUG=). A marker root anchored by package.json / + * pyproject etc. with NO .git is legit #2212 sticky identity and must + * NOT be healed. Legit remote-adopting stickiness is safe too: there + * the repo that adopted the remote IS the marker root (remote root == + * project root), so the heal never fires. * 3. Canonical remote-derived slug from the OUTERMOST remote-bearing repo * (see outermostRemoteRepo — never PROJECT_ROOT, which may be a * marker-only ancestor with no remote): [:/]/[.git] → @@ -176,7 +180,16 @@ export function slugFromEnvironment(gstackHome?: string, cwd: string = process.c !oldBugShape && projectRoot !== "" && cached === rootBase && + // STRAY-REPO shape only: the marker root must be anchored by a .git + // entry whose origin does NOT resolve. A root anchored by + // package.json etc. (no .git) is legit #2212 sticky identity. + existsSync(join(projectRoot, ".git")) && (() => { + const rootOrigin = spawnSync("git", ["-C", projectRoot, "remote", "get-url", "origin"], { + encoding: "utf-8", + }); + const rootUrl = rootOrigin.status === 0 ? (rootOrigin.stdout || "").trim() : ""; + if (rootUrl) return false; // marker root's own origin resolves — not the stray shape const r = resolveRemote(); return r.url !== "" && r.root !== projectRoot; })(); diff --git a/setup b/setup index db492f975..7b1121e16 100755 --- a/setup +++ b/setup @@ -2095,7 +2095,12 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ # Consent already recorded — no prompt. But a registration from an earlier # setup may carry a stale absolute path (a since-deleted dev worktree); # ensure-event re-points it in place and no-ops when everything matches. - _install_plan_tune_hooks >/dev/null 2>&1 || true + # Non-fatal to setup, but never silent: the hardened settings-hook refuses + # to rewrite a corrupt settings.json (exit 1), and swallowing that refusal + # left users with stale hooks and no signal. + if ! _PT_ENSURE_ERR=$(_install_plan_tune_hooks 2>&1 >/dev/null); then + log " warning: settings hook update failed: $(printf '%s\n' "$_PT_ENSURE_ERR" | head -1) — run $SETTINGS_HOOK manually" + fi log "" log "Plan-tune hooks already installed. Run \`$SETTINGS_HOOK list-sources\` to inspect." elif [ "$PT_DECISION" = "yes" ]; then @@ -2203,7 +2208,7 @@ if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -x "$TIMELINE_STOP_ --event Stop \ --command "$TIMELINE_STOP_HOOK" \ --source gstack-timeline-stop \ - --timeout 5 2>/dev/null); then + --timeout 5 2>&1); then case "$_TL_ENSURE_OUT" in *unchanged*) : # already registered with the canonical command — quiet no-op @@ -2215,6 +2220,11 @@ if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -x "$TIMELINE_STOP_ log " registered Stop hook: session timeline entries now close even when a skill is interrupted (backup: settings.json.bak.; remove: $SETTINGS_HOOK remove-source --source gstack-timeline-stop)" ;; esac + else + # Non-fatal to setup, but never silent: the hardened settings-hook refuses + # to rewrite a corrupt settings.json (exit 1), and swallowing that refusal + # left the Stop hook unregistered with no signal. + log " warning: settings hook update failed: $(printf '%s\n' "$_TL_ENSURE_OUT" | head -1) — run $SETTINGS_HOOK manually" fi fi diff --git a/test/bin-context-windows-slug.test.ts b/test/bin-context-windows-slug.test.ts index 0597bfb3d..4764cbcb1 100644 --- a/test/bin-context-windows-slug.test.ts +++ b/test/bin-context-windows-slug.test.ts @@ -326,6 +326,35 @@ describe("walk-up parity with bin/gstack-slug (outermost project root)", () => { expect(fs.readFileSync(cacheFile, "utf-8")).toBe("garrytan-gstack"); }); + test("package.json wrapper root (no .git): sticky basename slug is PRESERVED — heal is stray-repo-shape only", () => { + // Legit #2212 shape: a monorepo wrapper anchored by package.json used + // gstack before an inner dir grew a remote-bearing repo. The degraded- + // ancestor heal must NOT fire — it is restricted to marker roots anchored + // by a .git entry whose origin does NOT resolve (the live-bug shape). + const wrapper = path.join(tmp, "wrapperproj"); + const inner = path.join(wrapper, "apps", "web"); + fs.mkdirSync(inner, { recursive: true }); + fs.writeFileSync(path.join(wrapper, "package.json"), '{"name":"wrapper"}\n'); + spawnSync("git", ["init", "-q", inner]); + spawnSync("git", ["-C", inner, "remote", "add", "origin", "https://github.com/acme/web.git"]); + + const cacheDir = path.join(nativeHome(), "slug-cache"); + fs.mkdirSync(cacheDir, { recursive: true }); + const cacheFile = path.join(cacheDir, toMsysPath(inner).replace(/\//g, "_")); + fs.writeFileSync(cacheFile, "wrapperproj"); // legit sticky identity + + expect(slugFromEnvironment(nativeHome(), inner)).toBe("wrapperproj"); // NOT healed to acme-web + expect(fs.readFileSync(cacheFile, "utf-8")).toBe("wrapperproj"); + + // The bash implementation agrees on the same fixture (own home, seeded cache). + if (HAS_BASH) { + const bashCacheDir = path.join(tmp, "bash-home", ".gstack", "slug-cache"); + fs.mkdirSync(bashCacheDir, { recursive: true }); + fs.writeFileSync(path.join(bashCacheDir, toMsysPath(inner).replace(/\//g, "_")), "wrapperproj"); + expect(bashSlug(inner)).toBe("wrapperproj"); + } + }); + test("sticky identity preserved (#2212): a remote adopted AT the marker root is NOT healed", () => { // Legit sticky shape: the repo that adopted the remote IS the marker root // (remote root == project root), so the degraded-ancestor heal must not diff --git a/test/brain-sync.test.ts b/test/brain-sync.test.ts index 7a0f1a70a..47d8b5931 100644 --- a/test/brain-sync.test.ts +++ b/test/brain-sync.test.ts @@ -553,15 +553,20 @@ describe('#2549 queue integrity', () => { expect(detail.dropped.missing).toContain('projects/p/learnings.jsonl'); }); - test('an unparseable legacy queue line migrates as-is and is preserved, never destroyed', () => { + test('an unparseable legacy queue line migrates as-is and is quarantined, never destroyed', () => { // The line lands in the legacy single-file queue (pre-spool writer); - // migration converts it verbatim to a spool record, and the drain keeps - // what it cannot parse. + // migration converts it verbatim to a spool record, and the drain moves + // what it cannot parse into quarantine (never deletes it, and never + // leaves it re-warning at every boundary). initWithMode('full'); fs.appendFileSync(path.join(tmpHome, '.brain-queue.jsonl'), 'not json at all\n'); const r = run(['gstack-brain-sync', '--once']); expect(r.status).toBe(0); - expect(spoolText()).toContain('not json at all'); + const qDir = path.join(spoolDir(), 'quarantine'); + expect(fs.existsSync(qDir)).toBe(true); + const qFiles = fs.readdirSync(qDir); + expect(qFiles.length).toBe(1); + expect(fs.readFileSync(path.join(qDir, qFiles[0]), 'utf-8')).toContain('not json at all'); }); test('finalize: a synced record leaves the spool while a held sibling survives the same drain', () => { @@ -841,7 +846,7 @@ describe('C12 spool queue', () => { expect(spoolText()).not.toContain('learnings.jsonl'); }); - test('an unparseable spool record is kept and warned about; the drain continues', () => { + test('an unparseable spool record is quarantined with a warning; the drain continues', () => { initWithMode('full'); fs.mkdirSync(path.join(tmpHome, 'projects', 'p'), { recursive: true }); fs.writeFileSync(path.join(tmpHome, 'projects/p/learnings.jsonl'), '{"skill":"x","ts":"2026-01-01T00:00:00Z"}\n'); @@ -850,10 +855,13 @@ describe('C12 spool queue', () => { const r = run(['gstack-brain-sync', '--once']); expect(r.status).toBe(0); expect(r.stderr).toContain('unparseable'); - // The good sibling synced; the unreadable record was never destroyed. + // The good sibling synced; the unreadable record was never destroyed — + // it moved to quarantine so it stops re-warning at every boundary. expect(remoteLog()).toMatch(/sync: 1 file/); - expect(spoolFiles()).toEqual([badFile]); - expect(spoolText()).toContain('this is not json'); + expect(spoolFiles()).toEqual([]); + const qPath = path.join(spoolDir(), 'quarantine', badFile); + expect(fs.existsSync(qPath)).toBe(true); + expect(fs.readFileSync(qPath, 'utf-8')).toContain('this is not json'); }); test('--status queue_depth counts spool records plus unmigrated legacy lines', () => { @@ -867,6 +875,73 @@ describe('C12 spool queue', () => { expect(supplemental.queue_depth).toBe(3); }); + test('G1: a malformed pulled privacy map (["bad"]) holds the queue — warns, deletes NOTHING, next run re-drains', () => { + // Remotely triggerable kill vector: the privacy map arrives via the + // artifacts-repo pull. A non-dict entry used to raise mid-classification + // AFTER the snapshot manifest was written, and the old finalize polarity + // ("delete unless retained") then unlinked EVERY snapshotted record. + initWithMode('full'); + fs.mkdirSync(path.join(tmpHome, 'projects', 'p'), { recursive: true }); + fs.writeFileSync(path.join(tmpHome, 'projects/p/learnings.jsonl'), '{"skill":"x","ts":"2026-01-01T00:00:00Z"}\n'); + run(['gstack-brain-enqueue', 'projects/p/learnings.jsonl']); + const seeded = spoolFiles(); + expect(seeded.length).toBe(1); + fs.writeFileSync(path.join(tmpHome, '.brain-privacy-map.json'), '["bad"]'); + + const r = run(['gstack-brain-sync', '--once']); + expect(r.status).toBe(0); + expect(r.stderr).toContain('privacy map'); + // Zero records deleted; nothing pushed. + expect(spoolFiles()).toEqual(seeded); + expect(remoteLog()).not.toMatch(/sync:/); + + // Fix the map: the surviving queue re-drains and syncs. + fs.writeFileSync(path.join(tmpHome, '.brain-privacy-map.json'), '[]'); + expect(run(['gstack-brain-sync', '--once']).status).toBe(0); + expect(spoolFiles().length).toBe(0); + expect(remoteLog()).toMatch(/sync: 1 file/); + }); + + test('G1: a classifier that dies AFTER the snapshot write consumes nothing (call-site exit check + explicit-delete finalize)', () => { + // A dict entry with a non-string pattern passes the shape filter but + // raises inside fnmatch DURING classification — the post-manifest crash + // window (same shape as ENOSPC/OOM mid-run). The call site must see the + // nonzero exit, warn, skip finalize, and leave everything queued. + initWithMode('full'); + fs.mkdirSync(path.join(tmpHome, 'projects', 'p'), { recursive: true }); + fs.writeFileSync(path.join(tmpHome, 'projects/p/learnings.jsonl'), '{"skill":"x","ts":"2026-01-01T00:00:00Z"}\n'); + run(['gstack-brain-enqueue', 'projects/p/learnings.jsonl']); + const seeded = spoolFiles(); + expect(seeded.length).toBe(1); + fs.writeFileSync(path.join(tmpHome, '.brain-privacy-map.json'), '[{"pattern": 123}]'); + + const r = run(['gstack-brain-sync', '--once']); + expect(r.status).toBe(0); + expect(r.stderr).toContain('classification failed'); + expect(spoolFiles()).toEqual(seeded); // zero records deleted + expect(remoteLog()).not.toMatch(/sync:/); + const status = JSON.parse(fs.readFileSync(path.join(tmpHome, '.brain-sync-status.json'), 'utf-8')); + expect(status.status).toBe('error'); + expect(status.message).toContain('queue preserved'); + + // Fix the map: the surviving queue re-drains and syncs. + fs.writeFileSync(path.join(tmpHome, '.brain-privacy-map.json'), '[]'); + expect(run(['gstack-brain-sync', '--once']).status).toBe(0); + expect(spoolFiles().length).toBe(0); + expect(remoteLog()).toMatch(/sync: 1 file/); + }); + + test('G1: finalize is explicit-delete-only and the fast path is .migrating-aware (static pins)', () => { + const src = fs.readFileSync(path.join(BIN, 'gstack-brain-sync'), 'utf-8'); + // The compute call site checks the python exit status before finalizing. + expect(src).toMatch(/if ! compute_paths_to_stage /); + // finalize_queue takes the staged-paths file and deletes only staged ∪ dropped. + expect(src).toContain('deletable = staged | dropped'); + expect(src).toContain('if p not in deletable:'); + // The empty fast path also treats a leftover .migrating file as non-idle. + expect(src).toMatch(/spool_has_records && \[ ! -s "\$QUEUE" \] && \[ ! -s "\$QUEUE\.migrating" \]/); + }); + test('--drop-queue keeps the --yes gate and counts spool + legacy entries', () => { initWithMode('full'); seedSpool('{"file":"projects/p/a.jsonl","ts":"t"}'); diff --git a/test/gstack-memory-ingest.test.ts b/test/gstack-memory-ingest.test.ts index 2d6c525a6..26289afe6 100644 --- a/test/gstack-memory-ingest.test.ts +++ b/test/gstack-memory-ingest.test.ts @@ -300,9 +300,16 @@ describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => { describe("gstack-memory-ingest --limit", () => { it("respects --limit by stopping after N writes (mocked via --probe shortcut)", () => { - const r = runScript(["--probe", "--limit", "1"]); + // Hermetic home: against the operator's real HOME this walked the whole + // transcript corpus (and, post policy-parity, batch-checked its real + // policy store), making a pure arg-parsing assertion slow and flaky. + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + const r = runScript(["--probe", "--limit", "1"], { HOME: home, GSTACK_HOME: gstackHome }); // --limit doesn't apply to probe but argument should parse without error expect(r.exitCode).toBe(0); + rmSync(home, { recursive: true, force: true }); }); it("rejects --limit 0 with exit 1", () => { @@ -1201,6 +1208,82 @@ describe("#2392: transcript ingest honors per-remote trust policy", () => { rmSync(home, { recursive: true, force: true }); }); + it("(f) probe policy parity: a denied remote's transcript lands in skipped_policy_deny, not new_count", () => { + // --probe used to count policy-denied transcripts as ingestible (it only + // applied attribution), so its numbers overstated what --bulk would write. + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + + const denyCwd = makeRepoWithRemote(home, "denied", "https://github.com/denyme/denied.git"); + writeSessionForRepo(home, "work-denied", "denysess1", denyCwd); + setPolicy(gstackHome, "https://github.com/denyme/denied.git", "deny"); + + const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain("Total files in window: 0"); + expect(r.stdout).toMatch(/New \(never ingested\):\s+0/); + expect(r.stdout).toMatch(/Skipped \(policy deny\):\s+1/); + expect(r.stdout).not.toMatch(/Skipped \(policy read-only\)/); + rmSync(home, { recursive: true, force: true }); + }); + + it("(g) probe policy parity: a read-only remote's transcript lands in skipped_policy_readonly", () => { + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + + const roCwd = makeRepoWithRemote(home, "readonly", "https://github.com/roorg/rorepo.git"); + writeSessionForRepo(home, "work-readonly", "rosess1", roCwd); + setPolicy(gstackHome, "https://github.com/roorg/rorepo.git", "read-only"); + + const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain("Total files in window: 0"); + expect(r.stdout).toMatch(/Skipped \(policy read-only\):\s+1/); + rmSync(home, { recursive: true, force: true }); + }); + + it("(h) --limit counts policy-PERMITTED pages only: a denied-first corpus still writes the allowed page", () => { + // Walk order is deterministic here: Claude Code projects are walked + // BEFORE Codex sessions (walkAllSources), so the DENIED transcript is + // prepared first. Pre-fix, --limit 1 was applied to the unfiltered + // prepared array — the denied record consumed the limit and the permitted + // one starved (written: 0). + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + const { binDir } = installFakeGbrain(home); + + const denyCwd = makeRepoWithRemote(home, "denied", "https://github.com/denyme/denied.git"); + writeSessionForRepo(home, "work-denied", "denysess1", denyCwd); // Claude Code: walked first + const okCwd = makeRepoWithRemote(home, "allowed", "https://github.com/okorg/okrepo.git"); + const today = new Date(); + const ymd = `${today.getFullYear()}-${String(today.getMonth() + 1).padStart(2, "0")}-${String(today.getDate()).padStart(2, "0")}`; + writeCodexSession( + home, ymd, + `{"type":"session_meta","payload":{"id":"oksess-codex","cwd":"${okCwd.replace(/\\/g, "\\\\")}"},"timestamp":"${today.toISOString()}"}\n`, + ); + setPolicy(gstackHome, "https://github.com/denyme/denied.git", "deny"); + + const r = runScript(["--bulk", "--quiet", "--limit", "1"], { + HOME: home, + GSTACK_HOME: gstackHome, + PATH: `${binDir}:${process.env.PATH || ""}`, + }); + + expect(r.exitCode).toBe(0); + expect(r.stdout).toMatch(/written:\s+1/); + expect(r.stdout).toMatch(/skipped \(policy deny\):\s+1/); + // The page that landed is the PERMITTED one (the Codex session), not + // whichever record happened to be walked first. + const sessions = stateSessions(gstackHome); + expect(sessions.length).toBe(1); + expect(sessions[0]).toContain("rollout-"); + + rmSync(home, { recursive: true, force: true }); + }); + it("artifacts are never policy-filtered, even when their project's remote is denied", () => { const home = makeTestHome(); const gstackHome = join(home, ".gstack"); diff --git a/test/gstack-next-version.test.ts b/test/gstack-next-version.test.ts index 8cae867f9..6a3f24b0b 100644 --- a/test/gstack-next-version.test.ts +++ b/test/gstack-next-version.test.ts @@ -662,6 +662,102 @@ describe("fetchGitClaimed — non-mutating live remote query (ls-remote first)", }); }); +describe("fetchGitClaimed — unfetched live claims (G2: ls-remote advertises SHAs without objects)", () => { + // `git ls-remote` lists a branch's tip sha without transferring objects, so + // a branch pushed AFTER the last local fetch has no local object and both + // VERSION reads fail. The old `continue` silently dropped that LIVE claim — + // the exact duplicate-allocation this fallback exists to prevent. + function git(cwd: string, ...args: string[]) { + return Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", ...args], { cwd }); + } + + function cloneFixture(): { root: string; origin: string; clone: string } { + const root = mkdtempSync(join(tmpdir(), "nextver-unfetched-")); + const origin = join(root, "origin"); + mkdirSync(origin); + git(origin, "init", "-q", "-b", "main"); + writeFileSync(join(origin, "VERSION"), "0.1.66.0\n"); + git(origin, "add", "-A"); + git(origin, "commit", "-qm", "v0.1.66.0 chore: base"); + const clone = join(root, "clone"); + git(root, "clone", "-q", origin, clone); + return { root, origin, clone }; + } + + test("a claim branch pushed after the last local fetch is read via a targeted fetch", () => { + const { root, origin, clone } = cloneFixture(); + const cwd = process.cwd(); + try { + // The claim lands on origin AFTER the clone — its objects are absent + // locally, so `git show :VERSION` and the remote-tracking read + // both fail until the targeted fetch runs. + git(origin, "checkout", "-q", "-b", "late-claim"); + writeFileSync(join(origin, "VERSION"), "0.1.70.0\n"); + git(origin, "add", "-A"); + git(origin, "commit", "-qm", "v0.1.70.0 feat: late claim"); + git(origin, "checkout", "-q", "main"); + + process.chdir(clone); + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + expect(claims.map((c) => c.version)).toContain("0.1.70.0"); + expect(warnings.join(" ")).not.toContain("UNKNOWN claim"); + } finally { + process.chdir(cwd); + rmSync(root, { recursive: true, force: true }); + } + }); + + test("a claim STILL unreadable after the fetch surfaces as an UNKNOWN-claim warning, never silence", () => { + const { root, origin, clone } = cloneFixture(); + const cwd = process.cwd(); + try { + // A ref origin advertises but cannot serve: dangling sha written + // straight into refs/. ls-remote lists it; every local read fails, the + // targeted fetch fails ("not our ref"), and the object never appears. + writeFileSync( + join(origin, ".git", "refs", "heads", "ghost"), + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n", + ); + + process.chdir(clone); + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + expect(claims.map((c) => c.branch)).not.toContain("origin/ghost"); + const joined = warnings.join(" "); + expect(joined).toContain("origin/ghost"); + expect(joined).toContain("UNKNOWN claim"); + } finally { + process.chdir(cwd); + rmSync(root, { recursive: true, force: true }); + } + }); + + test("a live branch that simply carries no VERSION file is not a claim and not an UNKNOWN warning", () => { + const { root, origin, clone } = cloneFixture(); + const cwd = process.cwd(); + try { + // Branch exists BEFORE the clone (objects local), VERSION deleted on it: + // the read fails because the PATH is absent, not the object. Old + // semantics (skip quietly) must hold — no phantom UNKNOWN noise. + git(origin, "checkout", "-q", "-b", "docs-only"); + git(origin, "rm", "-q", "VERSION"); + git(origin, "commit", "-qm", "docs: no version file"); + git(origin, "checkout", "-q", "main"); + git(clone, "fetch", "-q", "origin"); + + process.chdir(clone); + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + expect(claims.map((c) => c.branch)).not.toContain("origin/docs-only"); + expect(warnings.join(" ")).not.toContain("docs-only"); + } finally { + process.chdir(cwd); + rmSync(root, { recursive: true, force: true }); + } + }); +}); + describe("width pinned on failed base read (3-digit repos)", () => { // readBaseVersion used to return a literal "0.0.0.0" when origin/ was // unreadable — a 4-digit string, which flipped versionWidth() to 4 and diff --git a/test/gstack-settings-hook-schema-aware.test.ts b/test/gstack-settings-hook-schema-aware.test.ts index 5a5f302ec..cc6f4f2bb 100644 --- a/test/gstack-settings-hook-schema-aware.test.ts +++ b/test/gstack-settings-hook-schema-aware.test.ts @@ -219,6 +219,77 @@ describe('add-event', () => { }); }); +// ---------------------------------------------------------------------- +// ensure-event: duplicate (event, source) collapse +// ---------------------------------------------------------------------- + +describe('ensure-event collapses duplicate (event, source) entries', () => { + test('two same-source entries from the old matcher-keyed dedup collapse to ONE updated entry', () => { + // Pre-existing installs can carry two entries with the same + // (event, _gstack_source) — the old dedup keyed on the matcher too, so a + // matcher change pushed a second registration. `.find()` updated only the + // first and left the stale twin running forever. + const { spawnSync } = require('child_process'); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [ + { _gstack_source: 'plan-tune-cathedral', matcher: 'OldMatcherA', hooks: [{ type: 'command', command: '/old-a', timeout: 5 }] }, + { matcher: 'Bash', hooks: [{ type: 'command', command: '/user-own-hook' }] }, + { _gstack_source: 'plan-tune-cathedral', matcher: 'OldMatcherB', hooks: [{ type: 'command', command: '/old-b', timeout: 5 }] }, + ], + }, + }, null, 2)); + + const r = spawnSync('bash', [ + SETTINGS_HOOK, 'ensure-event', + '--event', 'PostToolUse', + '--matcher', 'NewMatcher', + '--command', '/canonical', + '--source', 'plan-tune-cathedral', + '--timeout', '5', + ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 }); + + expect(r.status).toBe(0); + // The collapse is reported on stderr, never silent. + expect(r.stderr).toContain('collapsed 1 duplicate'); + const s = settings(); + const mine = s.hooks.PostToolUse.filter((e: any) => e._gstack_source === 'plan-tune-cathedral'); + expect(mine).toHaveLength(1); // ONE canonical entry — the stale twin is gone + expect(mine[0].matcher).toBe('NewMatcher'); + expect(mine[0].hooks[0].command).toBe('/canonical'); + // Unrelated user hook untouched. + const bash = s.hooks.PostToolUse.find((e: any) => e.matcher === 'Bash'); + expect(bash.hooks[0].command).toBe('/user-own-hook'); + expect(s.hooks.PostToolUse).toHaveLength(2); + }); + + test('no duplicates → no collapse message, single entry updated as before', () => { + const { spawnSync } = require('child_process'); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [ + { _gstack_source: 'plan-tune-cathedral', matcher: 'OldMatcher', hooks: [{ type: 'command', command: '/old', timeout: 5 }] }, + ], + }, + }, null, 2)); + + const r = spawnSync('bash', [ + SETTINGS_HOOK, 'ensure-event', + '--event', 'PostToolUse', + '--matcher', 'NewMatcher', + '--command', '/new', + '--source', 'plan-tune-cathedral', + '--timeout', '5', + ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 }); + + expect(r.status).toBe(0); + expect(r.stderr).not.toContain('collapsed'); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe('/new'); + }); +}); + // ---------------------------------------------------------------------- // remove-source // ---------------------------------------------------------------------- diff --git a/test/gstack-slug-parity.test.ts b/test/gstack-slug-parity.test.ts index 7636dcb65..58d09f64a 100644 --- a/test/gstack-slug-parity.test.ts +++ b/test/gstack-slug-parity.test.ts @@ -233,6 +233,28 @@ describe('gstack-slug ↔ remote-slug parity', () => { expect(fs.readFileSync(cacheFile, 'utf8').trim()).toBe('dotty'); }); + test('package.json wrapper root (no .git): sticky basename slug is PRESERVED — heal is stray-repo-shape only', () => { + // Legit #2212 shape: a monorepo wrapper anchored by package.json used + // gstack before an inner dir grew a remote-bearing repo. The degraded- + // ancestor heal must NOT fire here — it is restricted to marker roots + // anchored by a .git entry whose origin does NOT resolve (the live-bug + // stray-repo shape). + const wrapper = path.join(fixtures, 'wrapperproj'); + fs.mkdirSync(wrapper, { recursive: true }); + fs.writeFileSync(path.join(wrapper, 'package.json'), '{"name":"wrapper"}\n'); + const inner = makeRepo(path.join(wrapper, 'apps', 'web'), 'https://github.com/acme/web.git'); + + const cacheDir = path.join(tmpHome, '.gstack', 'slug-cache'); + fs.mkdirSync(cacheDir, { recursive: true }); + const cacheFile = path.join(cacheDir, encodedCacheKey(inner)); + fs.writeFileSync(cacheFile, 'wrapperproj'); + + const r = runSlug(inner, tmpHome); + expect(r.status).toBe(0); + expect(slugOf(r)).toBe('wrapperproj'); // NOT healed to acme-web + expect(fs.readFileSync(cacheFile, 'utf8').trim()).toBe('wrapperproj'); + }); + test('sticky identity preserved (#2212): repo that adopted a remote after first use is NOT healed', () => { // Legit sticky shape: the repo itself is the marker root (REMOTE_ROOT == // PROJECT_ROOT) and its cached identity is its pre-origin basename slug. diff --git a/test/gstack-version-bump.test.ts b/test/gstack-version-bump.test.ts index 84e24e97c..8e1706e82 100644 --- a/test/gstack-version-bump.test.ts +++ b/test/gstack-version-bump.test.ts @@ -651,6 +651,34 @@ describe('#2600: repair must not write fabricated 0.0.0.0 when VERSION is missin expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('0.5.0'); }); + test('repair proceeds when VERSION genuinely reads 0.0.0.0 (a real file, not the sentinel)', () => { + // current === DEFAULT is ambiguous: it is BOTH the missing/unparseable + // sentinel AND a legitimate literal "0.0.0.0" in a brand-new repo. The + // guard now disambiguates on the raw bytes — a real 0.0.0.0 repairs + // package.json to the npm-valid 0.0.0. + const dir = makeDir(); + fs.writeFileSync(path.join(dir, 'VERSION'), '0.0.0.0\n'); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '0.5.0' }, null, 2) + '\n'); + + const out = execFileSync('bun', [BIN, 'repair'], { cwd: dir }).toString(); + const result = JSON.parse(out); + expect(result.repaired).toBe('0.0.0.0'); + expect(result.packageJsonVersion).toBe('0.0.0'); + expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('0.0.0'); + }); + + test('repair still rejects whitespace-only VERSION content (sentinel path, not a real version)', () => { + const dir = makeDir(); + fs.writeFileSync(path.join(dir, 'VERSION'), ' \n\n'); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '0.5.0' }, null, 2) + '\n'); + + let code = 0; + try { execFileSync('bun', [BIN, 'repair'], { cwd: dir, stdio: 'pipe' }); } + catch (e: any) { code = e.status; } + expect(code).toBe(2); + expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('0.5.0'); + }); + test('repair reproduces the exact issue scenario: VERSION in root, package.json in app/ (#2600)', () => { // The exact layout from the issue: VERSION at repo root, package.json in app/ // Running repair from app/ cwd with no VERSION there used to write 0.0.0.0 into app/package.json. diff --git a/test/session-update-autostash.test.ts b/test/session-update-autostash.test.ts index 90624b1e1..dc7f89b54 100644 --- a/test/session-update-autostash.test.ts +++ b/test/session-update-autostash.test.ts @@ -245,10 +245,36 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const src = fs.readFileSync(SCRIPT, 'utf8'); const mvAside = src.match(/mv "\$LOCK_DIR" "\$LOCK_DIR\.reap\.\$\$" 2>\/dev\/null \|\| \{ log_entry "SKIP lock_contested"; exit 0; \}/g) || []; expect(mvAside.length).toBe(2); // TTL branch + dead-PID branch - // The only rm -rf of the live lock dir is the holder's EXIT trap. + // The only rm -rf of the live lock dir is the holder's EXIT trap — and + // even that one is ownership-checked (see the static pin below). const bareRms = src.match(/rm -rf "\$LOCK_DIR"(?!\.)/g) || []; expect(bareRms.length).toBe(1); - expect(src).toContain(`trap 'rm -rf "$LOCK_DIR" 2>/dev/null' EXIT`); + expect(src).toContain( + `trap 'kill "$HB_PID" 2>/dev/null; [ "$(cat "$LOCK_DIR/pid" 2>/dev/null)" = "$MYPID" ] && rm -rf "$LOCK_DIR" 2>/dev/null' EXIT`, + ); + }); + + test('EXIT trap is ownership-checked and a heartbeat runs during pull/setup (static pins)', () => { + const src = fs.readFileSync(SCRIPT, 'utf8'); + // (a) After a TTL reclaim by another updater, $LOCK_DIR belongs to the + // NEW holder — the old holder's trap must remove the lock ONLY while the + // pidfile still contains ITS pid (MYPID captured at write time). + const trapLine = src.split('\n').find((l) => l.includes("trap '") && l.includes('rm -rf "$LOCK_DIR"')); + expect(trapLine).toBeDefined(); + expect(trapLine!).toContain('[ "$(cat "$LOCK_DIR/pid" 2>/dev/null)" = "$MYPID" ] && rm -rf "$LOCK_DIR"'); + // MYPID is written to the pidfile (the identity the trap compares against). + expect(src).toContain('MYPID="${BASHPID:-$(sh -c \'echo $PPID\')}"'); + expect(src).toContain('echo "$MYPID" > "$LOCK_DIR/pid"'); + // (b) In-flight heartbeat: the step-boundary touches only fire AFTER the + // pull / setup return, so a legitimately-slow step past the 30-min TTL + // got reclaimed while ALIVE. The loop re-checks ownership each beat and + // exits instead of touching a reclaimed holder's pidfile. + expect(src).toMatch( + /while :; do sleep 300; \[ "\$\(cat "\$LOCK_DIR\/pid" 2>\/dev\/null\)" = "\$MYPID" \] \|\| exit 0; touch "\$LOCK_DIR\/pid" 2>\/dev\/null; done/, + ); + expect(src).toContain('HB_PID=$!'); + // The trap stops the heartbeat so it can never outlive the holder. + expect(trapLine!).toContain('kill "$HB_PID"'); }); test('an expired-TTL lock is reclaimed even when its pid is alive (PID reuse)', async () => { diff --git a/test/timeline-stop-hook.test.ts b/test/timeline-stop-hook.test.ts index 24f39d4c0..7a20eda1b 100644 --- a/test/timeline-stop-hook.test.ts +++ b/test/timeline-stop-hook.test.ts @@ -237,6 +237,20 @@ describe('timeline-stop-hook wiring', () => { expect(teardown).toContain('remove-source --source gstack-timeline-stop'); }); + test('setup surfaces a settings-hook refusal instead of swallowing it', () => { + // The hardened settings-hook refuses to rewrite a corrupt settings.json + // (exit 1). Both setup call sites (ALREADY_INSTALLED plan-tune re-point, + // timeline ensure-event) must stay non-fatal but PRINT the failure. + const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + const warnings = setup.match(/settings hook update failed/g) || []; + expect(warnings.length).toBeGreaterThanOrEqual(2); + // The old swallow patterns are gone (the --no-team remove-source teardown + // legitimately keeps its 2>/dev/null; only the ensure-event registration + // must surface stderr). + expect(setup).not.toContain('_install_plan_tune_hooks >/dev/null 2>&1 || true'); + expect(setup).not.toMatch(/ensure-event[\s\S]{0,220}--source gstack-timeline-stop[\s\S]{0,40}2>\/dev\/null/); + }); + test('setup routes the Stop hook through ensure-event, not presence-only dedup', () => { const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); // ensure-event registers when missing AND re-points a stale path in place.