mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-11 08:40:22 +02:00
v1.61.0.0 fix wave: guards failing open / silent failures (9 fixes, 4 community PRs absorbed) (#2472)
* fix(careful): warn on chained rm even when the last target is safe The safe-exception block whitelisted rm -rf of build artifacts by extracting targets with a single greedy match (.*rm ...), which only ever inspects the LAST rm in the command. A chain like 'rm -rf /; rm -rf node_modules' was therefore judged solely by its trailing safe target and allowed without warning, waving through the destructive 'rm -rf /'. Gate the shortcut to single rm invocations: when any shell separator (; | & newline, incl. JSON-escaped \n/\r from the grep extraction path) is present, fall through to the destructive-pattern check, which warns on any recursive rm. Single-command artifact cleanups still allow. Adds 3 regression tests covering semicolon and && chains in both orders. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * harden(careful): substitution separators + capital -R recursive flag (#2039) Two residual fail-opens in the same guard PR #2040 hardened, both verified by executing the script pre-fix: - rm -rf $(./wipe-all)/node_modules silently allowed: the substitution token ends in a whitelisted suffix and the safe-exception early exit skipped ALL downstream checks. $( and backtick now count as chain separators; plain $VAR expansion stays allowed. - rm -R / silently allowed: both greps required a lowercase r in the flag cluster; capital -R is the documented BSD/macOS recursive flag. Both greps now match -[a-zA-Z]*[rR]. Six new tests: substitution x2 -> ask, capital-R x2 -> ask, rm -Rf node_modules single-command -> still allowed, escaped-newline branch (existing code, previously untested), and a pinned deliberate FP (cd app && rm -rf node_modules -> ask) documenting the fail-closed direction on chains. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(context-restore): prefer the current branch's own checkpoint (#2052) All worktrees of a repo share one origin-derived slug, so they share one `~/.gstack/projects/<slug>/checkpoints/` dir. `/context-restore` loaded the newest checkpoint across the whole dir, so in one worktree it could silently restore a *sibling worktree's* newer checkpoint. Step 1 now orders candidates current-branch-first (read from each file's `branch:` frontmatter), keeping other branches as a fallback. A branch is checked out in at most one worktree, so this stops cross-worktree contamination while preserving Conductor cross-branch handoff: when the current branch has no checkpoint of its own, the full newest-first set is still used. - scan the 200 newest before partitioning so a current-branch checkpoint sitting below a burst of sibling saves is still found; output still capped at 20 - non-git / detached HEAD / branchless legacy saves fall back to the old newest-first behavior (back-compat) - +5 regression tests in context-save-hardening.test.ts (the #2052 bug case fails on the old pipeline); regenerated SKILL.md + proactive-suggestions.json Fixes #2052 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(gbrain): pass --confirm-destructive on drift re-register (#1985) ensureSourceRegistered() handles match-but-different-path by removing the old source then re-adding it at the new path. The remove was issued as `gbrain sources remove <id> --yes`, but gbrain >= 0.42 gates `sources remove` behind `--confirm-destructive` (`--yes` alone no longer suppresses the data-loss prompt). The remove therefore fails with "To proceed, pass --confirm-destructive", which ensureSourceRegistered surfaces as "source registration failed" — aborting the entire /sync-gbrain code stage for any already-registered source whose path has drifted. The memory and brain-sync stages still pass, so the code index silently stops refreshing. The orchestrator's own safeSourcesRemove() already passes --confirm-destructive; this brings the lib helper in line with that convention. Keeps --yes for older gbrain. Tests: extend the fake gbrain shim in gbrain-sources.test.ts to simulate the gbrain >= 0.42 guard (remove without --confirm-destructive exits 1), update the drift re-register assertion, and add a regression test that proves the drift path no longer throws. Both fail on main with the exact "To proceed, pass --confirm-destructive" error and pass with the fix. Fixes #1985 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * harden(gbrain-sources): route drift remove through #1734 guards + realpath drift check Absorbing #2031 un-blocked a destructive remove that bypassed the #1734 data-loss guards: ensureSourceRegistered's drift path issued `gbrain sources remove` directly, without the detectAutopilot + decideSourceRemove checks every other remove routes through via safeSourcesRemove. gbrain >= 0.42's own prompt was accidentally blocking that path; with --confirm-destructive passed it is live again. - Drift remove now refuses LOUDLY (throws, actionable message) while an autopilot is active or when decideSourceRemove disallows; a silent changed=false would hide the drifted registration. - decideSourceRemove's extraArgs (--keep-storage when supported) propagate to the remove call, matching safeSourcesRemove. - Drift is realpath-normalized before being declared: a symlink alias of the same directory (macOS /tmp -> /private/tmp) is a match, not drift — the probable cause of #1985's reporter hitting the remove on an unmoved repo. - Drift fires a loud stderr line (old -> new path); perpetual drift in logs is the trigger for promoting #1985's reindex-in-place design. Tests: autopilot-active refusal (no remove in call log), fail-closed refusal on unreadable sources list, --keep-storage propagation, symlink-alias no-drift; existing drift tests pin the guard probes so a live autopilot on the dev machine can't flip them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(developer-profile): exclude mode:resources rows from SESSION_COUNT, TIER, NUDGE_ELIGIBLE (#2067) Every /office-hours run appends a mode:"resources" bookkeeping row alongside the real session row, so --read double-counted sessions (~2x): tiers promoted early and the builder-to-founder nudge armed prematurely. The file already filtered resources rows for LAST_*/CROSS_PROJECT; the same realSessions filter now feeds SESSION_COUNT/TIER, and the nudge predicate is the faithful allowlist (mode === 'builder') so a future mode #4 fails closed instead of re-opening this bug. 8 regression tests: count vs resources noise, tier boundaries both sides, nudge false-with-noise / true-at-3-builders, cross-project trailing row. Absorbed from PR #1991 by @mvann (fix + tests commits; the PR's version-bump commit is superseded by this wave's consolidated release commit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(hooks): passThrough() two-branch contract — never emit permissionDecision:'defer' (#2035, #2006) Every AskUserQuestion died with "Tool result missing due to internal error" on current Claude Code builds (Desktop 1.14271.0, CC 2.1.177). Root cause: the question-preference-hook emitted permissionDecision:'defer' on every pass-through path. 'defer' is a real PreToolUse value, but since CC v2.1.89 its semantics are "pause this tool call for external resumption" (headless resume) — never "abstain". Interactive sessions have nothing to resume the paused call, so the tool orphaned. Pre-2.1.89 builds ignored the unknown value, which is why the hook worked when it shipped and broke later. The fix is the two-branch pass-through contract: - no context -> exit 0 with EXACTLY empty stdout - memory nuggets present -> hookSpecificOutput with hookEventName + additionalContext ONLY (the documented shape; plan-tune Layer 8 memory injection ships through this branch and keeps working) defer() is renamed passThrough() so the function says what it does, and docs/spikes/claude-code-hook-mutation.md's protocol contract (cited by the hook header) is corrected in the same commit — it taught '"defer" — let permission flow continue' and was the reintroduction vector. Test contract rewritten in the same commit (13 assertions across 3 files, verified fail-first against the unfixed hook): pass-through paths assert exact-empty stdout (a garbage/partial write cannot slip past an optional-chained parse), the nugget path asserts permissionDecision is ABSENT while additionalContext survives, and a new tripwire asserts no non-deny path ever puts the string "permissionDecision" on stdout. The deny (auto-decide) and Conductor prose-redirect paths are unchanged. Deployment: no migration needed — settings.json points at the absolute bash shim which execs the .ts live; /gstack-upgrade delivers the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(one-way-doors): unify credential noun net + wire it into the runtime (#2024) Library fix: revoke/reset/rotate now share ONE noun alternation (api key, token, secret, credential, access key, password) with optional plural s?. Pre-fix leaks: "reset my secret", "reset my access key", "revoke my secret" (mismatched per-verb lists) and every plural form ("rotate the credentials", "revoke all tokens" — \b(...)\b cannot match a trailing s). Runtime wiring — the regexes could never fire in production before: - gstack-question-preference --check gains --summary-stdin: the question text pipes via stdin (never argv — summaries carry quotes/newlines/shell metacharacters) and feeds isOneWayDoor alongside the id, so an ad-hoc destructive question with a stored never-ask preference now forces ASK_NORMALLY. Empty/absent stdin keeps exact id-only semantics. - question-preference-hook falls back to classifyQuestion(question text) when the registry lookup misses, so unregistered destructive questions pass through to a human instead of auto-deciding. - question-tuning resolver prose shows the piped form (SKILL.md regen lands in the wave's release commit). Tripwires (verified fail-first): full verbs x nouns x singular/plural matrix with the #2024 repro rows, benign-summary no-over-match rows, stdin transport survival (quotes/newlines), empty-stdin fail-safe, and hook fallback both directions (destructive -> pass-through, benign -> deny). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(design): loud integer-flag contract for --count/--retry/--timeout (#2032) design variants --count abc silently generated ZERO variants and exited 0: parseInt(NaN) flowed through Math.min into the generation loop bound. The same NaN class was live on the two sibling flags in the same file: --retry abc made generate() a silent no-op (attempt <= NaN never true, null output, exit 0) and --timeout abc killed the serve board ~immediately (setTimeout(NaN)). New design/src/flag-utils.ts: parseIntFlag (pure, unit-testable) + normalizeIntFlag (CLI wrapper). Contract matches the --viewports precedent (error loudly on nonsense — these commands spend real image-API money, a silent fixup hides typos from calling agents): undefined -> default; bare flag/empty/non-integer ("3.7" rejected, not truncated)/below-min -> exit 1 with usage hint; above-max -> clamp with stderr warning. --count normalizes at the variants() consumption site so programmatic callers are covered, with the ceiling derived from STYLE_VARIATIONS.length instead of a magic 7; the CLI passes the raw flag through (a pre-parseInt would truncate "3.7"). Tripwires live in test/design-flag-utils.test.ts — deliberately under test/, not design/test/, which is invisible to the bun test glob, TEST_ROOTS, and every workflow (wiring design/test/ into CI is a captured TODO). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(gbrain): thin-client state — remote-MCP brains no longer classify as broken-config (#2051) A thin client (remote-HTTP MCP brain, no local engine by design) probed `gbrain sources list`, which gbrain's dispatch guard REFUSES on thin clients (exit 1, no recognized error string), so the classifier fell to its defensive broken-config default and every suppression gate silently hid brain-aware blocks from exactly the users on a shared team brain. New 'thin-client' state, detected PRE-probe from gbrain's own remote_mcp config marker via the existing gbrainConfigPath() helper (mirrors gbrain's isThinClient(); honors GBRAIN_HOME; zero network, immune to error-string drift), with a /thin[- ]client/ stderr backstop in the probe catch. Remote reachability is deliberately NOT probed by the classifier — that is the #1964 pathology; gbrain calls degrade gracefully at use time, and the detect JSON says so honestly (gbrain_thin_client: {probed: false}). The state is admitted at every suppression gate — gstack-gbrain-detect --is-ok (drives setup + gbrain-refresh), gen-skill-docs' detection override, gstack-config gbrain-refresh — while the sync stages (code/memory/dream) SKIP with an accurate reason: code indexing runs on the brain server, memory syncs via the remote brain's artifacts pull. The two consumer classes need opposite answers, which is why this is a distinct state and not a skip-the-probe special case. sync-gbrain Step 1.5 and setup-gbrain prose route thin-client to proceed, never into broken-config remediation. detectMcpMode secondary generalization: url-match against the config's remote_mcp.mcp_url (deterministic — gbrain mounts at the generic /mcp path) -> name pattern gbrain[-_]* -> stdio command token; gbrain_mcp_mode stays a 3-value enum. Tripwires: end-to-end --is-ok exits 0 on a thin-client fixture AND still exits 1 on broken-config (the gate didn't widen); pre-probe + stderr-fallback classifier paths; 4 detectMcpMode identification cases incl. a non-matching url that must NOT false-positive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * release: v1.60.0.0 — regen SKILL.md, VERSION, CHANGELOG, TODOS follow-ups - Regenerate all SKILL.md from templates (question-tuning --summary-stdin prose from #2024, context-restore branch preference from PR #2054, sync-gbrain/setup-gbrain thin-client prose from #2051) + llms.txt. - VERSION + package.json -> 1.60.0.0 (bin/gstack-next-version, queue-aware: #1815 claims 1.59.0.0, #2213 claims 1.59.1.0). - CHANGELOG release summary + itemized entry crediting @jbetala7 (x3) and @mvann. - TODOS.md: three eng-review follow-ups (design/test CI wiring + documented pre-existing retry-after flake, /context-save worktree identity, gbrain reindex-in-place conditional on the new drift log). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(resolvers): compress --summary-stdin preamble prose to fit parity budget; re-bless ship goldens The v1.57.7.0 parity suite caps investigate's generated size at 1.09x baseline; the #2024 question-tuning prose (duplicated into every tier->=2 skill) tipped it to 1.092. Compressed to a single inline command + short pointer (the full rationale lives in bin/gstack-question-preference's header and the one-way-doors module docs). Ship goldens re-blessed against the final resolver text (conscious template-change acknowledgment, per the golden-file regression contract). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): office-hours-spec-review turn budget fits the carved skill layout (#2473) The test failed deterministically with error_max_turns at 9 turns on main and this branch alike (CI attempt logs + local main repro). Root cause from the failing transcript: the Spec Review Loop content is carved out of office-hours/SKILL.md into office-hours/sections/, so the agent needs discovery hops (grep SKILL.md -> ls sections/ -> read the section) before it can write — 8 tool turns + the closing text turn = 9 > the 8-turn budget, which predates the carve. Observed failures wrote a CORRECT summary on tool turn 8 and died on the closing turn. maxTurns 8 -> 12. Verified: PASS locally post-fix (7 turns this run — the extra headroom absorbs discovery-path nondeterminism). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): review-dashboard-via session budget survives runner contention (#2473) The test failed on CI (and its baseline run) with the timeout signature: 0 turns, $0.00, exactly 183s, 3/3 attempts — the spawned claude -p session never emitted a single stream event before the 180s inner timeout. The file's tests run concurrently on one runner; session startup queues behind sibling sessions, and this test had the tightest budget in the file (the 240s-budget tests in the same job passed). A clean local run takes 270s wall for 4 turns, confirming 180s was too tight even without contention. Inner timeout 180s -> 300s; outer bun timeout 240s -> 360s to keep headroom over the inner budget. Verified: PASS locally post-fix (4 turns, 270s). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): retro-base-branch session budget survives runner contention (#2473) Same class as review-dashboard-via, one test over in the same file: /retro is a long multi-step flow whose clean pass measures 225-239s — a coin flip against the 240s inner budget. First CI run passed at 225s; the rerun timed out at the 240s line on all 3 attempts (exitReason "timeout"); the local verification run passed at 239s, ONE second under the old cap. Inner timeout 240s -> 360s; outer bun timeout 300s -> 480s for headroom. Verified: PASS locally post-fix (17 turns, 239s). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jayesh Betala <jayesh.betala7@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Michael Vann <9221873+mvann@users.noreply.github.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Jayesh Betala
Michael Vann
Claude Fable 5
Jayesh Betala
Michael Vann
parent
960c3a8d6c
commit
94993f7401
@@ -36,15 +36,33 @@ echo "TITLE_SLUG=$TITLE_SLUG"
|
||||
echo "FILE=$FILE"
|
||||
`;
|
||||
|
||||
// The exact find + sort + head used by context-restore/SKILL.md Step 1.
|
||||
// The exact selection used by context-restore/SKILL.md Step 1: scan newest 200,
|
||||
// order current-branch checkpoints first (fallback: all branches), cap at 20.
|
||||
// CURRENT_BRANCH is injected via env in tests; the skill resolves it from git.
|
||||
const RESTORE_FIND_BASH = `
|
||||
if [ ! -d "$CHECKPOINT_DIR" ]; then
|
||||
echo "NO_CHECKPOINTS"
|
||||
else
|
||||
FILES=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -20)
|
||||
if [ -z "$FILES" ]; then
|
||||
ALL=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -200)
|
||||
if [ -z "$ALL" ]; then
|
||||
echo "NO_CHECKPOINTS"
|
||||
else
|
||||
: "\${CURRENT_BRANCH:=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}"
|
||||
SAME=""; OTHER=""
|
||||
while IFS= read -r f; do
|
||||
[ -n "$f" ] || continue
|
||||
b=$(grep -m1 '^branch:' "$f" 2>/dev/null | sed 's/^branch:[[:space:]]*//')
|
||||
if [ -n "$CURRENT_BRANCH" ] && [ "$b" = "$CURRENT_BRANCH" ]; then
|
||||
SAME="\${SAME}\${f}
|
||||
"
|
||||
else
|
||||
OTHER="\${OTHER}\${f}
|
||||
"
|
||||
fi
|
||||
done <<EOF
|
||||
$ALL
|
||||
EOF
|
||||
FILES=$(printf '%s%s' "$SAME" "$OTHER" | grep -v '^[[:space:]]*$' | head -20)
|
||||
echo "$FILES"
|
||||
fi
|
||||
fi
|
||||
@@ -312,6 +330,73 @@ describe('context-restore: find + sort + head cap', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Current-branch preference (#2052) ──────────────────────────────────────
|
||||
//
|
||||
// All worktrees of a repo share one origin-derived slug → one checkpoints dir.
|
||||
// Restore must prefer the CURRENT branch's own checkpoint so a sibling
|
||||
// worktree's newer save can't shadow it, while still falling back across
|
||||
// branches (Conductor handoff) when the current branch has none.
|
||||
|
||||
describe('context-restore: current-branch preference (#2052)', () => {
|
||||
let tmp: string;
|
||||
beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ctx-branch-')); });
|
||||
afterEach(() => { try { fs.rmSync(tmp, { recursive: true, force: true }); } catch {} });
|
||||
|
||||
function writeCheckpoint(ts: string, branch: string | null): string {
|
||||
const file = `${tmp}/${ts}-work.md`;
|
||||
const fm = branch === null
|
||||
? `---\nstatus: in-progress\n---\n`
|
||||
: `---\nstatus: in-progress\nbranch: ${branch}\n---\n`;
|
||||
fs.writeFileSync(file, fm);
|
||||
return file;
|
||||
}
|
||||
|
||||
function firstCandidate(currentBranch?: string): string {
|
||||
const env: Record<string, string> = { CHECKPOINT_DIR: tmp };
|
||||
if (currentBranch !== undefined) env.CURRENT_BRANCH = currentBranch;
|
||||
const out = runBash(RESTORE_FIND_BASH, env).stdout;
|
||||
return out.trim().split('\n').filter(Boolean)[0] ?? '';
|
||||
}
|
||||
|
||||
test('the bug: current-branch save is NOT shadowed by a newer sibling-worktree save', () => {
|
||||
const mine = writeCheckpoint('20260101-120000', 'feature-a'); // older, my branch
|
||||
writeCheckpoint('20260619-120000', 'feature-b'); // newer, sibling worktree
|
||||
// On feature-a, restore must load feature-a's own (older) checkpoint.
|
||||
expect(firstCandidate('feature-a')).toBe(mine);
|
||||
});
|
||||
|
||||
test('fallback: current branch has no checkpoint → newest across all branches (Conductor handoff)', () => {
|
||||
writeCheckpoint('20260101-120000', 'feature-a');
|
||||
const newest = writeCheckpoint('20260619-120000', 'feature-b');
|
||||
// On feature-c (no own checkpoint), cross-branch resume still works.
|
||||
expect(firstCandidate('feature-c')).toBe(newest);
|
||||
});
|
||||
|
||||
test('back-compat: empty current branch (non-git) → newest across all', () => {
|
||||
writeCheckpoint('20260101-120000', 'feature-a');
|
||||
const newest = writeCheckpoint('20260619-120000', 'feature-b');
|
||||
expect(firstCandidate('')).toBe(newest);
|
||||
});
|
||||
|
||||
test('checkpoints without a branch frontmatter still rank as fallback, never lost', () => {
|
||||
const mine = writeCheckpoint('20260101-120000', 'feature-a');
|
||||
writeCheckpoint('20260301-120000', null); // legacy save, no branch field
|
||||
const out = runBash(RESTORE_FIND_BASH, { CHECKPOINT_DIR: tmp, CURRENT_BRANCH: 'feature-a' }).stdout;
|
||||
const lines = out.trim().split('\n').filter(Boolean);
|
||||
expect(lines[0]).toBe(mine); // current branch first
|
||||
expect(lines.length).toBe(2); // legacy file is still present
|
||||
});
|
||||
|
||||
test('within the current branch, ordering stays newest-first', () => {
|
||||
const older = writeCheckpoint('20260101-120000', 'feature-a');
|
||||
const newer = writeCheckpoint('20260619-120000', 'feature-a');
|
||||
const out = runBash(RESTORE_FIND_BASH, { CHECKPOINT_DIR: tmp, CURRENT_BRANCH: 'feature-a' }).stdout;
|
||||
const lines = out.trim().split('\n').filter(Boolean);
|
||||
expect(lines[0]).toBe(newer);
|
||||
expect(lines[1]).toBe(older);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Migration HOME guard ──────────────────────────────────────────────────
|
||||
|
||||
describe('migration v1.1.3.0: HOME guard', () => {
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* design/src/flag-utils.ts — integer-flag contract (#2032, eng-review 7A/8A).
|
||||
*
|
||||
* Lives under test/ (NOT design/test/) deliberately: design/test/ is invisible
|
||||
* to the bun test glob, scripts/test-free-shards TEST_ROOTS, and every CI
|
||||
* workflow (eng-review 11A), so a tripwire there guards nothing. flag-utils is
|
||||
* a pure module, so importing it from here is clean.
|
||||
*
|
||||
* The bug class: the design CLI parser yields string | true | undefined;
|
||||
* parseInt produced NaN that flowed silently into loop bounds and setTimeout —
|
||||
* `variants --count abc` generated ZERO variants and exited 0, `generate
|
||||
* --retry abc` was a silent no-op, `serve --timeout abc` died at boot.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from "bun:test";
|
||||
import { spawnSync } from "child_process";
|
||||
import * as path from "path";
|
||||
import { parseIntFlag } from "../design/src/flag-utils";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, "..");
|
||||
|
||||
const COUNT_SPEC = { name: "count", def: 3, min: 1, max: 7 } as const;
|
||||
const RETRY_SPEC = { name: "retry", def: 0, min: 0 } as const;
|
||||
const TIMEOUT_SPEC = { name: "timeout", def: 600, min: 1 } as const;
|
||||
|
||||
describe("parseIntFlag contract (#2032, codex 17a-c)", () => {
|
||||
test("undefined → default (flag absent)", () => {
|
||||
expect(parseIntFlag(undefined, COUNT_SPEC)).toEqual({ ok: true, value: 3 });
|
||||
expect(parseIntFlag(undefined, RETRY_SPEC)).toEqual({ ok: true, value: 0 });
|
||||
});
|
||||
|
||||
test("non-numeric string → error, never a silent default ('--count abc')", () => {
|
||||
const r = parseIntFlag("abc", COUNT_SPEC);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.error).toContain('"abc" is not an integer');
|
||||
});
|
||||
|
||||
test("bare flag (parser yields true) → error 'requires a value'", () => {
|
||||
const r = parseIntFlag(true, COUNT_SPEC);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.error).toContain("requires a value");
|
||||
});
|
||||
|
||||
test("empty string → error 'requires a value'", () => {
|
||||
const r = parseIntFlag("", COUNT_SPEC);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.error).toContain("requires a value");
|
||||
});
|
||||
|
||||
test("non-integer '3.7' → error (rejected, not silently truncated to 3)", () => {
|
||||
const r = parseIntFlag("3.7", COUNT_SPEC);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.error).toContain("not an integer");
|
||||
});
|
||||
|
||||
test("below min → error ('--count 0' billed the user for 1 they asked 0 of; now loud)", () => {
|
||||
expect(parseIntFlag("0", COUNT_SPEC).ok).toBe(false);
|
||||
expect(parseIntFlag("-2", COUNT_SPEC).ok).toBe(false);
|
||||
// retry allows 0 (min: 0)
|
||||
expect(parseIntFlag("0", RETRY_SPEC)).toEqual({ ok: true, value: 0 });
|
||||
});
|
||||
|
||||
test("above max → clamp WITH warning (capability limit, not a user mistake)", () => {
|
||||
const r = parseIntFlag("99", COUNT_SPEC);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) {
|
||||
expect(r.value).toBe(7);
|
||||
expect(r.warning).toContain("exceeds the maximum");
|
||||
}
|
||||
});
|
||||
|
||||
test("in-range integers pass through untouched", () => {
|
||||
expect(parseIntFlag("3", COUNT_SPEC)).toEqual({ ok: true, value: 3 });
|
||||
expect(parseIntFlag(5, COUNT_SPEC)).toEqual({ ok: true, value: 5 });
|
||||
expect(parseIntFlag("120", TIMEOUT_SPEC)).toEqual({ ok: true, value: 120 });
|
||||
});
|
||||
|
||||
test("retry-NaN and timeout-NaN are errors, not silent no-ops (#2032 siblings)", () => {
|
||||
// Pre-fix: --retry abc → generate() loop never ran (attempt <= NaN),
|
||||
// printed null, exited 0. --timeout abc → setTimeout(NaN) ≈ immediate
|
||||
// SERVE_TIMEOUT. Both members of the same NaN class, same file.
|
||||
expect(parseIntFlag("abc", RETRY_SPEC).ok).toBe(false);
|
||||
expect(parseIntFlag("abc", TIMEOUT_SPEC).ok).toBe(false);
|
||||
});
|
||||
|
||||
test("NaN number input (legacy pre-parsed callers) → error", () => {
|
||||
expect(parseIntFlag(Number.NaN, COUNT_SPEC).ok).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeIntFlag CLI wrapper (exit-1 semantics)", () => {
|
||||
function runWrapper(rawExpr: string, specExpr: string): { status: number; stderr: string } {
|
||||
const script = `
|
||||
import { normalizeIntFlag } from "${ROOT}/design/src/flag-utils";
|
||||
const v = normalizeIntFlag(${rawExpr}, ${specExpr});
|
||||
console.log("VALUE:" + v);
|
||||
`;
|
||||
const res = spawnSync("bun", ["-e", script], { encoding: "utf-8", cwd: ROOT });
|
||||
return { status: res.status ?? -1, stderr: res.stderr ?? "" };
|
||||
}
|
||||
|
||||
test("invalid input exits 1 with the error on stderr", () => {
|
||||
const r = runWrapper('"abc"', '{ name: "count", def: 3, min: 1, max: 7 }');
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stderr).toContain("not an integer");
|
||||
});
|
||||
|
||||
test("clamp warns on stderr but exits 0", () => {
|
||||
const r = runWrapper('"99"', '{ name: "count", def: 3, min: 1, max: 7 }');
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stderr).toContain("exceeds the maximum");
|
||||
});
|
||||
});
|
||||
+1
-1
@@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST
|
||||
|
||||
## Question Tuning (skip entirely if `QUESTION_TUNING: false`)
|
||||
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check "<id>"`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "<question summary>" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "<id>" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
|
||||
**Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `<gstack-qid:{question_id}>` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`.
|
||||
|
||||
|
||||
+1
-1
@@ -693,7 +693,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST
|
||||
|
||||
## Question Tuning (skip entirely if `QUESTION_TUNING: false`)
|
||||
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `$GSTACK_BIN/gstack-question-preference --check "<id>"`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "<question summary>" | $GSTACK_BIN/gstack-question-preference --check "<id>" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
|
||||
**Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `<gstack-qid:{question_id}>` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`.
|
||||
|
||||
|
||||
+1
-1
@@ -695,7 +695,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST
|
||||
|
||||
## Question Tuning (skip entirely if `QUESTION_TUNING: false`)
|
||||
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `$GSTACK_BIN/gstack-question-preference --check "<id>"`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "<question summary>" | $GSTACK_BIN/gstack-question-preference --check "<id>" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask.
|
||||
|
||||
**Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `<gstack-qid:{question_id}>` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`.
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@ import {
|
||||
utimesSync,
|
||||
} from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { join, dirname } from "path";
|
||||
|
||||
import { spawnSync } from "child_process";
|
||||
|
||||
@@ -62,8 +62,10 @@ interface FakeEnv {
|
||||
*/
|
||||
function makeEnv(opts: {
|
||||
withGbrain?: boolean;
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow";
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow" | "thin-refusal";
|
||||
withConfig?: boolean;
|
||||
/** #2051: config carries gbrain's remote_mcp thin-client marker. */
|
||||
thinClientConfig?: boolean;
|
||||
}): FakeEnv {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-"));
|
||||
const bindir = join(tmp, "bin");
|
||||
@@ -77,7 +79,12 @@ function makeEnv(opts: {
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
mkdirSync(configDir, { recursive: true });
|
||||
|
||||
if (opts.withConfig) {
|
||||
if (opts.thinClientConfig) {
|
||||
writeFileSync(
|
||||
configPath,
|
||||
JSON.stringify({ remote_mcp: { mcp_url: "https://brain.example.com/mcp" } }),
|
||||
);
|
||||
} else if (opts.withConfig) {
|
||||
writeFileSync(
|
||||
configPath,
|
||||
JSON.stringify({ engine: "pglite", database_url: "pglite:///fake" }),
|
||||
@@ -103,7 +110,7 @@ function makeEnv(opts: {
|
||||
}
|
||||
|
||||
function makeFakeGbrainScript(
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow",
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow" | "thin-refusal",
|
||||
): string {
|
||||
// "slow": healthy engine on a cold pooler connection (#1964) — sleeps past
|
||||
// the (test-lowered) probe timeout, then would answer fine.
|
||||
@@ -130,7 +137,9 @@ exit 0
|
||||
? 'echo "gbrain sources: connect timed out (default 10000ms; pass --timeout=Ns to override)." >&2'
|
||||
: behavior === "throws"
|
||||
? 'echo "unexpected gbrain failure" >&2'
|
||||
: "";
|
||||
: behavior === "thin-refusal"
|
||||
? 'echo "Error: gbrain sources is not routable to the remote brain (thin-client of https://brain.example.com/mcp)" >&2'
|
||||
: "";
|
||||
const exitCode = behavior === "ok" ? 0 : behavior === "engine-locked" ? 124 : 1;
|
||||
return `#!/bin/sh
|
||||
if [ "$1" = "--version" ]; then
|
||||
@@ -448,3 +457,72 @@ describe("lib/gbrain-local-status — cache behavior", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #2051: thin-client classification + the end-to-end --is-ok gate
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("lib/gbrain-local-status — thin-client (#2051)", () => {
|
||||
let env: FakeEnv | null = null;
|
||||
let restoreEnv: (() => void) | null = null;
|
||||
|
||||
afterEach(() => {
|
||||
if (restoreEnv) restoreEnv();
|
||||
if (env) env.cleanup();
|
||||
env = null;
|
||||
restoreEnv = null;
|
||||
});
|
||||
|
||||
it("returns 'thin-client' when config carries gbrain's remote_mcp marker (pre-probe, no engine call)", () => {
|
||||
// The fake gbrain would answer "ok" if probed — proving the marker is
|
||||
// read from config BEFORE any probe (zero network, no error-string
|
||||
// dependence).
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", thinClientConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
it("returns 'thin-client' via the stderr refusal fallback when the config marker is unreadable", () => {
|
||||
// Regular (non-thin) config on disk, but gbrain itself refuses with the
|
||||
// dispatch-guard message — the catch-path backstop.
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "thin-refusal", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
// The eng-review 3A tripwire: the END-TO-END gate, not just the classifier
|
||||
// return. --is-ok drives setup:1299 and gstack-config gbrain-refresh — this
|
||||
// exit code is what decides whether brain-aware blocks render for a
|
||||
// thin-client user (the #2051 report).
|
||||
it("--is-ok exits 0 on a thin-client fixture (end-to-end gate)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", thinClientConfig: true });
|
||||
const detectBin = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect");
|
||||
const bunDir = dirname(process.execPath);
|
||||
const r = spawnSync(detectBin, ["--is-ok"], {
|
||||
encoding: "utf-8",
|
||||
env: {
|
||||
HOME: env.home,
|
||||
PATH: `${env.bindir}:${bunDir}:/usr/bin:/bin`,
|
||||
GSTACK_HOME: env.gstackHome,
|
||||
GSTACK_DETECT_NO_CACHE: "1",
|
||||
},
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
});
|
||||
|
||||
it("--is-ok still exits 1 on broken-config (thin-client did not widen the gate)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "broken-config", withConfig: true });
|
||||
const detectBin = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect");
|
||||
const bunDir = dirname(process.execPath);
|
||||
const r = spawnSync(detectBin, ["--is-ok"], {
|
||||
encoding: "utf-8",
|
||||
env: {
|
||||
HOME: env.home,
|
||||
PATH: `${env.bindir}:${bunDir}:/usr/bin:/bin`,
|
||||
GSTACK_HOME: env.gstackHome,
|
||||
GSTACK_DETECT_NO_CACHE: "1",
|
||||
},
|
||||
});
|
||||
expect(r.status).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
+146
-5
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync } from "fs";
|
||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync, symlinkSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
|
||||
@@ -32,8 +32,11 @@ interface FakeGbrainSetup {
|
||||
* Build a temp dir with a fake `gbrain` shell script on PATH. The fake honors:
|
||||
* gbrain sources list --json → cat $STATE_PATH
|
||||
* gbrain sources add <id> --path <p> [--federated] → append to state, log
|
||||
* gbrain sources remove <id> --yes → drop from state, log
|
||||
* gbrain --version → echo "gbrain 0.25.1"
|
||||
* gbrain sources remove <id> --confirm-destructive → drop from state, log
|
||||
* (#1985: remove WITHOUT
|
||||
* --confirm-destructive
|
||||
* fails like gbrain >= 0.42)
|
||||
* gbrain --version → echo "gbrain 0.42.40.0"
|
||||
* Anything else exits 1.
|
||||
*/
|
||||
function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path: string; federated?: boolean; page_count?: number }> }): FakeGbrainSetup {
|
||||
@@ -49,7 +52,7 @@ function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path:
|
||||
echo "$@" >> "${logPath}"
|
||||
case "$1 $2" in
|
||||
"--version ")
|
||||
echo "gbrain 0.25.1"
|
||||
echo "gbrain 0.42.40.0"
|
||||
exit 0
|
||||
;;
|
||||
"sources list")
|
||||
@@ -75,6 +78,16 @@ case "$1 $2" in
|
||||
;;
|
||||
"sources remove")
|
||||
ID="$3"
|
||||
# #1985: gbrain >= 0.42 gates remove behind --confirm-destructive; --yes
|
||||
# alone no longer suppresses the data-loss prompt. Refuse without it so the
|
||||
# drift re-register path is exercised against real gbrain 0.42 behavior.
|
||||
case " $* " in
|
||||
*" --confirm-destructive "*) : ;;
|
||||
*)
|
||||
echo "This will permanently delete pages. To proceed, pass --confirm-destructive" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
NEW=$(jq --arg id "$ID" '.sources = (.sources | map(select(.id != $id)))' "${statePath}")
|
||||
echo "$NEW" > "${statePath}"
|
||||
exit 0
|
||||
@@ -155,6 +168,13 @@ describe("ensureSourceRegistered", () => {
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
// Drift-path tests pin the #1734 guard inputs (inactive autopilot, allowed
|
||||
// remove) so a REAL autopilot running on the dev machine can't flip them.
|
||||
const guardsPinnedInactive = {
|
||||
autopilotProbe: { lockPaths: [], processRunning: () => false },
|
||||
removeDecision: { keepStorage: false },
|
||||
} as const;
|
||||
|
||||
it("recreates source when path differs (gbrain has no `sources update`), returns changed=true", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
@@ -162,17 +182,138 @@ describe("ensureSourceRegistered", () => {
|
||||
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
|
||||
federated: true,
|
||||
env: fake.env,
|
||||
...guardsPinnedInactive,
|
||||
});
|
||||
expect(result.changed).toBe(true);
|
||||
expect(result.state.status).toBe("match");
|
||||
expect(result.state.registered_path).toBe("/new/path");
|
||||
|
||||
const log = readFileSync(fake.logPath, "utf-8");
|
||||
expect(log).toContain("sources remove gstack-code-foo --yes");
|
||||
// #1985: the remove must carry --confirm-destructive (gbrain >= 0.42 gate).
|
||||
expect(log).toContain("sources remove gstack-code-foo --yes --confirm-destructive");
|
||||
expect(log).toContain("sources add gstack-code-foo --path /new/path --federated");
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
// #1985: regression. gbrain >= 0.42 refuses `sources remove` without
|
||||
// --confirm-destructive. On the drift path that surfaces as "source
|
||||
// registration failed" and aborts the /sync-gbrain code stage for every
|
||||
// already-registered source whose path drifted. Before the fix the remove
|
||||
// was issued with `--yes` only, so the guard-simulating fake rejects it and
|
||||
// ensureSourceRegistered throws. The fix passes --confirm-destructive, so
|
||||
// the re-register succeeds.
|
||||
it("re-registers across the gbrain >= 0.42 destructive-remove guard (does not throw)", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
});
|
||||
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
|
||||
federated: true,
|
||||
env: fake.env,
|
||||
...guardsPinnedInactive,
|
||||
});
|
||||
expect(result.changed).toBe(true);
|
||||
expect(result.state.status).toBe("match");
|
||||
expect(result.state.registered_path).toBe("/new/path");
|
||||
|
||||
// The old source was actually removed (the guarded remove succeeded), then
|
||||
// the new path was added — not left behind as a stale duplicate.
|
||||
const finalState = JSON.parse(readFileSync(fake.statePath, "utf-8"));
|
||||
expect(finalState.sources).toHaveLength(1);
|
||||
expect(finalState.sources[0].local_path).toBe("/new/path");
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
// #1734 tripwire: the drift remove deletes pages/chunks/embeddings, so it
|
||||
// must refuse while a gbrain autopilot is active — and refuse LOUDLY (throw),
|
||||
// not silently return changed=false. Before the guard routing, this path
|
||||
// issued the remove unconditionally.
|
||||
it("REFUSES the drift remove while autopilot is active (throws, no remove issued)", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
});
|
||||
await expect(
|
||||
ensureSourceRegistered("gstack-code-foo", "/new/path", {
|
||||
env: fake.env,
|
||||
autopilotProbe: { lockPaths: [], processRunning: () => true },
|
||||
removeDecision: { keepStorage: false },
|
||||
}),
|
||||
).rejects.toThrow(/autopilot active/);
|
||||
|
||||
const log = readFileSync(fake.logPath, "utf-8");
|
||||
expect(log).not.toContain("sources remove");
|
||||
expect(log).not.toContain("sources add");
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
it("REFUSES the drift remove when decideSourceRemove disallows (fail closed, throws)", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
});
|
||||
await expect(
|
||||
ensureSourceRegistered("gstack-code-foo", "/new/path", {
|
||||
env: fake.env,
|
||||
autopilotProbe: { lockPaths: [], processRunning: () => false },
|
||||
// A sources-list read failure makes decideSourceRemove fail closed.
|
||||
removeDecision: {
|
||||
keepStorage: false,
|
||||
fetchRows: () => {
|
||||
throw new Error("sources list unavailable");
|
||||
},
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow(/fail closed/);
|
||||
|
||||
const log = readFileSync(fake.logPath, "utf-8");
|
||||
expect(log).not.toContain("sources remove");
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
it("propagates decideSourceRemove extraArgs (--keep-storage) to the drift remove", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
});
|
||||
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
|
||||
env: fake.env,
|
||||
autopilotProbe: { lockPaths: [], processRunning: () => false },
|
||||
removeDecision: { keepStorage: true },
|
||||
});
|
||||
expect(result.changed).toBe(true);
|
||||
|
||||
const log = readFileSync(fake.logPath, "utf-8");
|
||||
expect(log).toContain(
|
||||
"sources remove gstack-code-foo --yes --confirm-destructive --keep-storage",
|
||||
);
|
||||
fake.cleanup();
|
||||
});
|
||||
|
||||
// Realpath normalization: a registered path that is a symlink alias of the
|
||||
// requested path is a MATCH, not drift. Declaring it drift triggers a
|
||||
// destructive remove + full re-index for a no-op (#1985 reporter hit the
|
||||
// remove on an unmoved repo; macOS /tmp -> /private/tmp is the usual cause).
|
||||
it("does NOT declare drift when registered path is a symlink alias of the requested path", async () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gbrain-sources-realpath-"));
|
||||
const realDir = join(base, "real-repo");
|
||||
const linkDir = join(base, "link-repo");
|
||||
mkdirSync(realDir, { recursive: true });
|
||||
symlinkSync(realDir, linkDir);
|
||||
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: realDir }],
|
||||
});
|
||||
const result = await ensureSourceRegistered("gstack-code-foo", linkDir, {
|
||||
env: fake.env,
|
||||
...guardsPinnedInactive,
|
||||
});
|
||||
expect(result.changed).toBe(false);
|
||||
expect(result.state.status).toBe("match");
|
||||
|
||||
const log = readFileSync(fake.logPath, "utf-8");
|
||||
expect(log).not.toContain("sources remove");
|
||||
expect(log).not.toContain("sources add");
|
||||
fake.cleanup();
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("when reregister_on_drift=false and source is at different path, returns changed=false", async () => {
|
||||
const fake = makeFakeGbrain({
|
||||
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
|
||||
|
||||
@@ -556,3 +556,143 @@ describe('gstack-developer-profile --log-session (#1671 fix)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// SESSION_COUNT / TIER / NUDGE_ELIGIBLE must ignore mode:resources entries.
|
||||
//
|
||||
// Phase 6 of /office-hours auto-appends one (or more) mode:resources bookkeeping
|
||||
// entries every run, to dedupe which founder-resource links the user has seen.
|
||||
// Those are not sessions. Counting them inflated SESSION_COUNT (and therefore
|
||||
// TIER) and pushed NUDGE_ELIGIBLE over its threshold from bookkeeping alone —
|
||||
// e.g. a single real session plus three closings reported as tier `regular`
|
||||
// with the builder->founder nudge armed.
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
describe('gstack-developer-profile resources entries do not inflate count/tier/nudge', () => {
|
||||
function logStartup(extra: Record<string, unknown> = {}) {
|
||||
return runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-20T00:00:00Z', mode: 'startup', project_slug: 'p',
|
||||
signal_count: 5, signals: ['a', 'b', 'c', 'd', 'e'], ...extra,
|
||||
}));
|
||||
}
|
||||
function logResources(i: number) {
|
||||
return runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-20T01:00:00Z', mode: 'resources', project_slug: 'p',
|
||||
resources_shown: [`url${i}`],
|
||||
}));
|
||||
}
|
||||
|
||||
test('SESSION_COUNT counts only real sessions, not resources entries', () => {
|
||||
logStartup();
|
||||
logResources(1);
|
||||
logResources(2);
|
||||
logResources(3);
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('SESSION_COUNT: 1');
|
||||
expect(r.stdout).toContain('TIER: welcome_back');
|
||||
});
|
||||
|
||||
test('TIER is not bumped to regular by resources bookkeeping', () => {
|
||||
// 3 real sessions = welcome_back; adding resources entries must not reach the
|
||||
// 4-session `regular` threshold.
|
||||
logStartup();
|
||||
logStartup();
|
||||
logStartup();
|
||||
for (let i = 0; i < 4; i++) logResources(i);
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('SESSION_COUNT: 3');
|
||||
expect(r.stdout).toContain('TIER: welcome_back');
|
||||
});
|
||||
|
||||
test('NUDGE_ELIGIBLE stays false when builder-session bar is unmet despite resources noise', () => {
|
||||
// One startup session carrying 5 signals, plus resources entries. builderSessions
|
||||
// (mode === "builder") is 0, so the nudge must not arm regardless of signal count.
|
||||
logStartup();
|
||||
logResources(1);
|
||||
logResources(2);
|
||||
logResources(3);
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('NUDGE_ELIGIBLE: false');
|
||||
});
|
||||
|
||||
test('NUDGE_ELIGIBLE arms on 3 real builder sessions with enough signals', () => {
|
||||
runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-20T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['a', 'b'],
|
||||
}));
|
||||
runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-21T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['c', 'd'],
|
||||
}));
|
||||
runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-22T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['e'],
|
||||
}));
|
||||
logResources(1); // bookkeeping must not change the verdict either way
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('NUDGE_ELIGIBLE: true');
|
||||
});
|
||||
|
||||
// Boundary cases around the two `>=` gates, so a future >= → > regression
|
||||
// (or a re-loosening of the builder filter) is caught, not just the happy path.
|
||||
function logBuilder(signals: string[], day = 20) {
|
||||
return runDev('--log-session', JSON.stringify({
|
||||
date: `2026-05-${day}T00:00:00Z`, mode: 'builder', project_slug: 'p', signals,
|
||||
}));
|
||||
}
|
||||
|
||||
test('NUDGE_ELIGIBLE stays false at 2 builder sessions (below the 3-session gate)', () => {
|
||||
logBuilder(['a', 'b', 'c'], 20);
|
||||
logBuilder(['d', 'e', 'f'], 21); // 6 signals total — signal gate met, session gate is not
|
||||
logResources(1);
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('NUDGE_ELIGIBLE: false');
|
||||
});
|
||||
|
||||
test('NUDGE_ELIGIBLE stays false at 3 builder sessions with too few signals', () => {
|
||||
logBuilder(['a'], 20);
|
||||
logBuilder(['b'], 21);
|
||||
logBuilder(['c', 'd'], 22); // 4 signals total — session gate met, signal gate (>=5) is not
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('NUDGE_ELIGIBLE: false');
|
||||
});
|
||||
|
||||
test('TIER reaches regular at 4 real sessions even when resources entries are present', () => {
|
||||
logStartup();
|
||||
logStartup();
|
||||
logStartup();
|
||||
logStartup();
|
||||
for (let i = 0; i < 5; i++) logResources(i);
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('SESSION_COUNT: 4');
|
||||
expect(r.stdout).toContain('TIER: regular');
|
||||
});
|
||||
|
||||
test('TIER stays regular at 7 real sessions and crosses to inner_circle at 8 (resources ignored)', () => {
|
||||
// Upper-tier boundary: the >=8 inner_circle gate must key off real sessions
|
||||
// only, so a pile of resources bookkeeping can never tip a regular into the
|
||||
// inner circle, and 8 genuine sessions still reach it.
|
||||
for (let i = 0; i < 7; i++) logStartup();
|
||||
for (let i = 0; i < 6; i++) logResources(i); // 13 raw rows; pre-fix would read inner_circle
|
||||
let r = runDev('--read');
|
||||
expect(r.stdout).toContain('SESSION_COUNT: 7');
|
||||
expect(r.stdout).toContain('TIER: regular');
|
||||
|
||||
logStartup(); // 8th real session
|
||||
r = runDev('--read');
|
||||
expect(r.stdout).toContain('SESSION_COUNT: 8');
|
||||
expect(r.stdout).toContain('TIER: inner_circle');
|
||||
});
|
||||
|
||||
test('CROSS_PROJECT ignores a trailing resources entry on a different project', () => {
|
||||
// The last two REAL sessions are the same project, so CROSS_PROJECT is false.
|
||||
// A trailing resources row carrying a different project_slug must not become
|
||||
// the `last` entry and flip CROSS_PROJECT true off bookkeeping.
|
||||
logStartup({ project_slug: 'samep' });
|
||||
logStartup({ project_slug: 'samep' });
|
||||
runDev('--log-session', JSON.stringify({
|
||||
date: '2026-05-20T02:00:00Z', mode: 'resources', project_slug: 'otherp',
|
||||
resources_shown: ['url1'],
|
||||
}));
|
||||
const r = runDev('--read');
|
||||
expect(r.stdout).toContain('CROSS_PROJECT: false');
|
||||
expect(r.stdout).toContain('LAST_PROJECT: samep');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -208,6 +208,61 @@ describe('gbrain_mcp_mode — Tier 3: ~/.claude.json jq read', () => {
|
||||
);
|
||||
expect(runDetect().json.gbrain_mcp_mode).toBe('none');
|
||||
});
|
||||
|
||||
// #2051 name generalization: a gbrain server registered under a variant
|
||||
// name still counts. Identification order: url-match against the config's
|
||||
// remote_mcp.mcp_url (deterministic — gbrain mounts at generic /mcp so
|
||||
// URL-path heuristics are impossible) → name pattern gbrain[-_]* → stdio
|
||||
// command token.
|
||||
test('server named gbrain-remote (name pattern) → remote-http', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.claude.json'),
|
||||
JSON.stringify({
|
||||
mcpServers: { 'gbrain-remote': { type: 'url', url: 'https://brain.corp.example/mcp' } },
|
||||
})
|
||||
);
|
||||
expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http');
|
||||
});
|
||||
|
||||
test('arbitrarily-named server whose url matches config remote_mcp.mcp_url → remote-http', () => {
|
||||
fs.mkdirSync(path.join(tmpHome, '.gbrain'), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.gbrain', 'config.json'),
|
||||
JSON.stringify({ remote_mcp: { mcp_url: 'https://team-brain.example.com/mcp' } })
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.claude.json'),
|
||||
JSON.stringify({
|
||||
mcpServers: { 'our-team-brain': { type: 'url', url: 'https://team-brain.example.com/mcp' } },
|
||||
})
|
||||
);
|
||||
expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http');
|
||||
});
|
||||
|
||||
test('unrelated server with a non-matching url does NOT false-positive → none', () => {
|
||||
fs.mkdirSync(path.join(tmpHome, '.gbrain'), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.gbrain', 'config.json'),
|
||||
JSON.stringify({ remote_mcp: { mcp_url: 'https://team-brain.example.com/mcp' } })
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.claude.json'),
|
||||
JSON.stringify({
|
||||
mcpServers: { linear: { type: 'url', url: 'https://mcp.linear.app/mcp' } },
|
||||
})
|
||||
);
|
||||
expect(runDetect().json.gbrain_mcp_mode).toBe('none');
|
||||
});
|
||||
|
||||
test('stdio server with gbrain in the command token → local-stdio', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpHome, '.claude.json'),
|
||||
JSON.stringify({
|
||||
mcpServers: { 'my-brain': { type: 'stdio', command: '/usr/local/bin/gbrain' } },
|
||||
})
|
||||
);
|
||||
expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio');
|
||||
});
|
||||
});
|
||||
|
||||
describe('gbrain_mcp_mode — no info anywhere', () => {
|
||||
|
||||
@@ -39,6 +39,20 @@ function run(...args: string[]): { stdout: string; stderr: string; status: numbe
|
||||
};
|
||||
}
|
||||
|
||||
function runWithStdin(input: string, ...args: string[]): { stdout: string; stderr: string; status: number } {
|
||||
const res = spawnSync(BIN, args, {
|
||||
env: { ...process.env, GSTACK_HOME: tmpHome },
|
||||
encoding: 'utf-8',
|
||||
cwd: ROOT,
|
||||
input,
|
||||
});
|
||||
return {
|
||||
stdout: res.stdout ?? '',
|
||||
stderr: res.stderr ?? '',
|
||||
status: res.status ?? -1,
|
||||
};
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// --check
|
||||
// -----------------------------------------------------------------------
|
||||
@@ -103,6 +117,49 @@ describe('--check with preferences set', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #2024: the keyword net only fires when the question TEXT reaches the
|
||||
// classifier. --summary-stdin pipes it (stdin, not argv — summaries carry
|
||||
// quotes/newlines/shell metacharacters). Without the summary, an unregistered
|
||||
// id with never-ask auto-decides even for destructive phrasings.
|
||||
describe('--check --summary-stdin (#2024 keyword net plumb-through)', () => {
|
||||
function setPref(id: string, pref: string) {
|
||||
return run('--write', JSON.stringify({ question_id: id, preference: pref, source: 'plan-tune' }));
|
||||
}
|
||||
|
||||
test('destructive summary on unregistered never-ask id → ASK_NORMALLY (keyword net fires)', () => {
|
||||
setPref('adhoc-cleanup-question', 'never-ask');
|
||||
const r = runWithStdin('Should I reset my secrets now?', '--check', 'adhoc-cleanup-question', '--summary-stdin');
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain('ASK_NORMALLY');
|
||||
expect(r.stdout).toContain('one-way door overrides');
|
||||
});
|
||||
|
||||
test('same id WITHOUT summary still AUTO_DECIDEs (id-only fallback, current semantics)', () => {
|
||||
setPref('adhoc-cleanup-question', 'never-ask');
|
||||
const r = run('--check', 'adhoc-cleanup-question');
|
||||
expect(r.stdout.trim()).toContain('AUTO_DECIDE');
|
||||
});
|
||||
|
||||
test('benign summary on unregistered never-ask id → AUTO_DECIDE (no over-match)', () => {
|
||||
setPref('adhoc-cleanup-question', 'never-ask');
|
||||
const r = runWithStdin('Reorganize the TODOs file?', '--check', 'adhoc-cleanup-question', '--summary-stdin');
|
||||
expect(r.stdout.trim()).toContain('AUTO_DECIDE');
|
||||
});
|
||||
|
||||
test('summary with quotes/newlines/dashes survives the stdin transport', () => {
|
||||
setPref('adhoc-cleanup-question', 'never-ask');
|
||||
const summary = 'Run "cleanup" --now\nthen rotate the access keys?';
|
||||
const r = runWithStdin(summary, '--check', 'adhoc-cleanup-question', '--summary-stdin');
|
||||
expect(r.stdout).toContain('ASK_NORMALLY');
|
||||
});
|
||||
|
||||
test('empty stdin with --summary-stdin → id-only behavior (fail-safe)', () => {
|
||||
setPref('adhoc-cleanup-question', 'never-ask');
|
||||
const r = runWithStdin('', '--check', 'adhoc-cleanup-question', '--summary-stdin');
|
||||
expect(r.stdout.trim()).toContain('AUTO_DECIDE');
|
||||
});
|
||||
});
|
||||
|
||||
// Split-chain carve-out: question_ids matching <skill>-split-<option-slug>
|
||||
// must always ASK_NORMALLY regardless of stored preferences.
|
||||
// See scripts/resolvers/preamble/generate-ask-user-format.ts
|
||||
|
||||
@@ -97,6 +97,90 @@ describe('check-careful.sh', () => {
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// The safe exception matches the COMPLETE command against an anchored
|
||||
// whitelist shape — anything else (chains, comments, substitution) falls
|
||||
// through to the destructive-pattern warning.
|
||||
test('rm -rf /; rm -rf node_modules warns (semicolon chain, dangerous first)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /; rm -rf node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test('rm -rf /etc/data && rm -rf dist warns (&& chain, dangerous first)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/data && rm -rf dist'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test('rm -rf node_modules; rm -rf /home/user/data warns (safe first, dangerous last)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules; rm -rf /home/user/data'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// Command substitution can end in a whitelisted suffix while running
|
||||
// anything inside $(...) or backticks — the whitelist's target tokens
|
||||
// exclude `(` and backtick so these cannot ride the safe exception.
|
||||
test('rm -rf $(./wipe-all)/node_modules warns (command substitution)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf $(./wipe-all)/node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test('rm -rf `./wipe-all`/node_modules warns (backtick substitution)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf `./wipe-all`/node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// Capital -R is the documented recursive flag on BSD rm (macOS) and accepted
|
||||
// by GNU rm. Both greps previously required a lowercase r, so `rm -R /`
|
||||
// silently allowed.
|
||||
test('rm -R / warns (capital -R recursive)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -R /'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test('rm -fR /home/user warns (capital R in flag cluster)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -fR /home/user'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test('rm -Rf node_modules allows (capital R, single safe target)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -Rf node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBeUndefined();
|
||||
});
|
||||
|
||||
// JSON-escaped newline (literal two-char \n surviving the grep extraction
|
||||
// path) breaks the anchored whitelist shape → falls through to the warn.
|
||||
test('newline-chained rm warns (escaped-newline separator branch)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/x\nrm -rf node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// Deliberate false positive, pinned: a safe-prefix chain ending in a safe rm
|
||||
// is indistinguishable from the dangerous-first exploit shape without real
|
||||
// shell parsing, so warn-on-all-chains is the designed fail-closed direction.
|
||||
// A future per-segment parser must consciously change this test.
|
||||
test('cd app && rm -rf node_modules asks (fail-closed on chains, by design)', () => {
|
||||
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('cd app && rm -rf node_modules'));
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.permissionDecision).toBe('ask');
|
||||
expect(output.message).toContain('recursive delete');
|
||||
});
|
||||
|
||||
test.each([
|
||||
'rm -rf /; rm -rf node_modules',
|
||||
'rm -rf / && rm -rf node_modules',
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
* Layer 8 memory cache + injection (plan-tune cathedral T12).
|
||||
*
|
||||
* Verifies the PreToolUse hook reads ~/.gstack/free-text-memory.json and
|
||||
* surfaces matching nuggets via additionalContext on the hook response.
|
||||
* surfaces matching nuggets via additionalContext-only output (#2035: never a permissionDecision).
|
||||
* Cache: per-session memory-cache.json populated on first read, sub-1ms
|
||||
* thereafter (D13 perf).
|
||||
*/
|
||||
@@ -43,9 +43,9 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe
|
||||
env.GSTACK_STATE_ROOT = stateRoot;
|
||||
env.GSTACK_QUESTION_LOG_NO_DERIVE = '1';
|
||||
delete env.GSTACK_HOME;
|
||||
// These cases assert the defer-path memoryContext injection. Strip ambient
|
||||
// These cases assert the pass-through memoryContext injection. Strip ambient
|
||||
// Conductor markers so running inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT
|
||||
// set) doesn't flip the hook into the [conductor] prose deny instead of defer.
|
||||
// set) doesn't flip the hook into the [conductor] prose deny instead of pass-through.
|
||||
delete env.CONDUCTOR_WORKSPACE_PATH;
|
||||
delete env.CONDUCTOR_PORT;
|
||||
const res = spawnSync(HOOK, [], {
|
||||
@@ -69,7 +69,7 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
describe('memory injection', () => {
|
||||
test('injects matching nugget into additionalContext on defer', () => {
|
||||
test('injects matching nugget into additionalContext on pass-through', () => {
|
||||
writeMemory([
|
||||
{
|
||||
nugget: 'User prefers verbose explanations with tradeoffs',
|
||||
@@ -91,7 +91,10 @@ describe('memory injection', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
// #2035: nugget delivery is additionalContext-ONLY — a permissionDecision
|
||||
// here (any value) would orphan the tool call on CC >= 2.1.89.
|
||||
expect('permissionDecision' in (r.parsed?.hookSpecificOutput ?? {})).toBe(false);
|
||||
expect(r.parsed?.hookSpecificOutput?.hookEventName).toBe('PreToolUse');
|
||||
expect(r.parsed?.hookSpecificOutput?.additionalContext).toContain('verbose explanations');
|
||||
});
|
||||
|
||||
@@ -115,8 +118,9 @@ describe('memory injection', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined();
|
||||
// No nugget → pure pass-through: exit 0 with EXACTLY empty stdout.
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toBe('');
|
||||
});
|
||||
|
||||
test('caps to 3 most-recent nuggets when many match', () => {
|
||||
@@ -219,7 +223,8 @@ describe('per-session memory cache', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined();
|
||||
// No nugget → pure pass-through: exit 0 with EXACTLY empty stdout.
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -43,3 +43,50 @@ describe("one-way-door credential keyword net (#1839)", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("one-way-door credential keyword net (#2024)", () => {
|
||||
const VERBS = ["revoke", "reset", "rotate"];
|
||||
const NOUNS = ["api key", "token", "secret", "credential", "access key", "password"];
|
||||
|
||||
// #2024 repro rows: these leaked as two-way pre-fix because the noun
|
||||
// alternations were mismatched across verbs (revoke lacked secret; reset
|
||||
// lacked secret AND access key). The password-parallel test above passes on
|
||||
// buggy code, so THESE rows are the fails-first proof.
|
||||
test('"reset my secret" / "reset my access key" / "revoke my secret" classify one-way', () => {
|
||||
for (const summary of ["reset my secret", "reset my access key", "revoke my secret"]) {
|
||||
const r = classifyQuestion({ summary });
|
||||
expect(r.oneWay).toBe(true);
|
||||
expect(r.reason).toBe("keyword");
|
||||
}
|
||||
});
|
||||
|
||||
test("full verbs x nouns matrix classifies one-way (singular and plural)", () => {
|
||||
for (const verb of VERBS) {
|
||||
for (const noun of NOUNS) {
|
||||
for (const form of [noun, `${noun}s`]) {
|
||||
const r = classifyQuestion({ summary: `${verb} the production ${form}` });
|
||||
expect(r.oneWay).toBe(true);
|
||||
expect(r.reason).toBe("keyword");
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// Plural forms leaked before AND after the original #2024 report: \b(...)\b
|
||||
// cannot match "credentials" (no word boundary between the noun and its s).
|
||||
test('plurals: "rotate the credentials" / "revoke all tokens" / "reset the passwords" classify one-way', () => {
|
||||
for (const summary of ["rotate the credentials", "revoke all tokens", "reset the passwords"]) {
|
||||
expect(classifyQuestion({ summary }).oneWay).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("benign summaries stay two-way (no over-match)", () => {
|
||||
for (const summary of [
|
||||
"reset the flaky test runner",
|
||||
"rotate the log files nightly",
|
||||
"revoke the meeting invite",
|
||||
]) {
|
||||
expect(classifyQuestion({ summary }).oneWay).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,15 +3,18 @@
|
||||
*
|
||||
* Covers:
|
||||
* - never-ask + marker + two-way + clean recommendation → deny+reason
|
||||
* - never-ask + no marker → defer (D18 marker gate)
|
||||
* - never-ask + one-way → defer (safety override)
|
||||
* - never-ask + ambiguous recommendation → defer (D2 refuse-on-ambiguous)
|
||||
* - always-ask → defer
|
||||
* - no preference → defer
|
||||
* - never-ask + no marker → pass-through (D18 marker gate)
|
||||
* - never-ask + one-way → pass-through (safety override)
|
||||
* - never-ask + ambiguous recommendation → pass-through (D2 refuse-on-ambiguous)
|
||||
* - always-ask → pass-through
|
||||
* - no preference → pass-through
|
||||
* - project preference wins over global (D8 precedence)
|
||||
* - global preference applies when no project preference set
|
||||
* - mcp__*__AskUserQuestion matcher accepted
|
||||
* - empty stdin → defer (crash safety)
|
||||
* - empty stdin → pass-through (crash safety)
|
||||
*
|
||||
* Pass-through contract (#2035/#2006): exit 0 + EXACTLY empty stdout, or
|
||||
* additionalContext-only hookSpecificOutput — never a permissionDecision.
|
||||
* - auto-decided event logged via gstack-question-log (PostToolUse won't fire)
|
||||
* - auto-decided marker written to ~/.gstack/sessions/<id>/.auto-decided-<tool_use_id>
|
||||
*/
|
||||
@@ -97,6 +100,20 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record<string, string>)
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* #2035/#2006 contract: pass-through (abstain) is exit 0 with EXACTLY empty
|
||||
* stdout — never a permissionDecision. 'defer' is a real PreToolUse value,
|
||||
* but its semantics are pause-for-external-resumption (CC v2.1.89+), so
|
||||
* emitting it orphans the tool call in interactive sessions. Exact-empty
|
||||
* (not trim) is deliberate: whitespace on stdout is still hook output, and a
|
||||
* garbage/partial write must fail this assertion rather than slip past an
|
||||
* optional-chained parse.
|
||||
*/
|
||||
function expectPassThrough(r: { status: number; stdout: string }): void {
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toBe('');
|
||||
}
|
||||
|
||||
function autoDecidedEvents(): Array<Record<string, unknown>> {
|
||||
const f = path.join(stateRoot, 'projects', cwdSlug, 'question-log.jsonl');
|
||||
if (!fs.existsSync(f)) return [];
|
||||
@@ -113,8 +130,8 @@ function autoDecidedEvents(): Array<Record<string, unknown>> {
|
||||
// Defer paths
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
describe('defers (no enforcement)', () => {
|
||||
test('no preference set → defer', () => {
|
||||
describe('passes through (no enforcement)', () => {
|
||||
test('no preference set → pass-through (empty stdout, no permissionDecision)', () => {
|
||||
const r = runHook({
|
||||
session_id: 's1',
|
||||
tool_name: 'AskUserQuestion',
|
||||
@@ -125,11 +142,10 @@ describe('defers (no enforcement)', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('marker missing → defer (D18)', () => {
|
||||
test('marker missing → pass-through (D18)', () => {
|
||||
writeProjectPref('test-q', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's2',
|
||||
@@ -141,10 +157,10 @@ describe('defers (no enforcement)', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('always-ask preference → defer', () => {
|
||||
test('always-ask preference → pass-through', () => {
|
||||
writeProjectPref('test-q', 'always-ask');
|
||||
const r = runHook({
|
||||
session_id: 's3',
|
||||
@@ -156,10 +172,10 @@ describe('defers (no enforcement)', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('empty stdin → defer (crash safety)', () => {
|
||||
test('empty stdin → pass-through (crash safety)', () => {
|
||||
const env: Record<string, string> = {};
|
||||
for (const [k, v] of Object.entries(process.env)) {
|
||||
if (v !== undefined) env[k] = v;
|
||||
@@ -167,14 +183,39 @@ describe('defers (no enforcement)', () => {
|
||||
env.GSTACK_STATE_ROOT = stateRoot;
|
||||
const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' });
|
||||
expect(res.status).toBe(0);
|
||||
const parsed = JSON.parse(res.stdout || '{}');
|
||||
expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expect(res.stdout).toBe('');
|
||||
});
|
||||
|
||||
test('non-AUQ tool_name → defer (defensive)', () => {
|
||||
test('non-AUQ tool_name → pass-through (defensive)', () => {
|
||||
writeProjectPref('test-q', 'never-ask');
|
||||
const r = runHook({ session_id: 's4', tool_name: 'Bash', tool_use_id: 'tu-4', tool_input: {} });
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
// #2035 tripwire: no non-deny/non-allow path may EVER put the string
|
||||
// "permissionDecision" on stdout. Emitting one on a pass-through path (any
|
||||
// value — 'defer' included) hands the platform a decision where the hook
|
||||
// has none, and 'defer' specifically pauses the call for a resumption that
|
||||
// never comes in interactive sessions.
|
||||
test('pass-through stdout never contains "permissionDecision" (#2035)', () => {
|
||||
const paths = [
|
||||
runHook({
|
||||
session_id: 's-trip-1',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-trip-1',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{ question: '<gstack-qid:test-q> Approve?', options: ['A) Yes (recommended)', 'B) No'] },
|
||||
],
|
||||
},
|
||||
}),
|
||||
runHook({ session_id: 's-trip-2', tool_name: 'Bash', tool_use_id: 'tu-trip-2', tool_input: {} }),
|
||||
runHook({ session_id: 's-trip-3', tool_name: 'AskUserQuestion', tool_use_id: 'tu-trip-3', tool_input: { questions: [] } }),
|
||||
];
|
||||
for (const r of paths) {
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).not.toContain('"permissionDecision"');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -204,7 +245,7 @@ describe('enforces never-ask preferences', () => {
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('Fix now');
|
||||
});
|
||||
|
||||
test('one-way door → defer even with never-ask (safety override)', () => {
|
||||
test('one-way door → pass-through even with never-ask (safety override)', () => {
|
||||
writeProjectPref('ship-test-failure-triage', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's6',
|
||||
@@ -219,10 +260,10 @@ describe('enforces never-ask preferences', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('ambiguous recommendation (two labels) → defer (D2 refuse-on-ambiguous)', () => {
|
||||
test('ambiguous recommendation (two labels) → pass-through (D2 refuse-on-ambiguous)', () => {
|
||||
writeProjectPref('ship-pre-landing-review-fix', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's7',
|
||||
@@ -237,10 +278,10 @@ describe('enforces never-ask preferences', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('no recommendation marker AND no prose match → defer', () => {
|
||||
test('no recommendation marker AND no prose match → pass-through', () => {
|
||||
writeProjectPref('ship-pre-landing-review-fix', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's8',
|
||||
@@ -255,7 +296,48 @@ describe('enforces never-ask preferences', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
// #2024: unregistered ids used to default straight to two-way without ever
|
||||
// consulting the keyword classifier — an ad-hoc DESTRUCTIVE question with a
|
||||
// stored never-ask preference auto-decided. The hook now falls back to
|
||||
// classifyQuestion on the question text when the registry lookup misses.
|
||||
test('unregistered id + never-ask + destructive text → pass-through (keyword net fires, #2024)', () => {
|
||||
writeProjectPref('adhoc-credential-cleanup', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's-kw-1',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-kw-1',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{
|
||||
question: '<gstack-qid:adhoc-credential-cleanup> Reset my secret and proceed?',
|
||||
options: ['A) Yes (recommended)', 'B) No'],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
test('unregistered id + never-ask + benign text → still deny (auto-decide unchanged)', () => {
|
||||
writeProjectPref('adhoc-credential-cleanup', 'never-ask');
|
||||
const r = runHook({
|
||||
session_id: 's-kw-2',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-kw-2',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{
|
||||
question: '<gstack-qid:adhoc-credential-cleanup> Reorganize the TODOs file?',
|
||||
options: ['A) Yes (recommended)', 'B) No'],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('plan-tune auto-decide');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -301,7 +383,7 @@ describe('precedence: project wins over global (D8)', () => {
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
|
||||
});
|
||||
|
||||
test('project always-ask + global never-ask → defer (project wins)', () => {
|
||||
test('project always-ask + global never-ask → pass-through (project wins)', () => {
|
||||
writeProjectPref('ship-pre-landing-review-fix', 'always-ask');
|
||||
writeGlobalPref('ship-pre-landing-review-fix', 'never-ask');
|
||||
const r = runHook({
|
||||
@@ -317,7 +399,7 @@ describe('precedence: project wins over global (D8)', () => {
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -437,13 +519,13 @@ describe('Conductor prose redirect', () => {
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).not.toContain('[conductor]');
|
||||
});
|
||||
|
||||
test('non-AUQ tool in Conductor → still defer (no redirect on unrelated tools)', () => {
|
||||
test('non-AUQ tool in Conductor → still pass-through (no redirect on unrelated tools)', () => {
|
||||
const r = runHook(
|
||||
{ session_id: 'c6', tool_name: 'Bash', tool_use_id: 'tu-c6', tool_input: {} },
|
||||
undefined,
|
||||
CONDUCTOR,
|
||||
);
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
expectPassThrough(r);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -296,7 +296,10 @@ describeIfSelected('PlanTune cathedral E2E: annotation', ['plan-tune-annotation'
|
||||
});
|
||||
expect(res.status).toBe(0);
|
||||
const parsed = JSON.parse(res.stdout || '{}');
|
||||
expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer');
|
||||
// #2035: memory-nugget delivery is additionalContext-ONLY. Emitting a
|
||||
// permissionDecision here (the old 'defer') pauses the tool call for a
|
||||
// resumption that never comes in interactive sessions.
|
||||
expect('permissionDecision' in (parsed.hookSpecificOutput ?? {})).toBe(false);
|
||||
expect(parsed.hookSpecificOutput?.additionalContext).toContain('verbose explanations');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -549,7 +549,13 @@ Summarize what the "Spec Review Loop" section does — specifically:
|
||||
|
||||
Write your summary to ${ohDir}/spec-review-summary.md`,
|
||||
workingDirectory: ohDir,
|
||||
maxTurns: 8,
|
||||
// 12, not 8 (#2473): the Spec Review Loop content is CARVED out of
|
||||
// SKILL.md into office-hours/sections/, so the agent legitimately needs
|
||||
// discovery hops (grep SKILL.md -> ls sections/ -> read the section)
|
||||
// before it can write. The 8-turn budget predates the carve — observed
|
||||
// failures wrote a correct summary on tool-turn 8 and hit the cap on
|
||||
// the closing text turn (error_max_turns at 9 turns, deterministic).
|
||||
maxTurns: 12,
|
||||
timeout: 120_000,
|
||||
testName: 'office-hours-spec-review',
|
||||
runId,
|
||||
|
||||
@@ -427,7 +427,11 @@ This is a local-only repo so use the local branch (main) instead of origin/main
|
||||
Write your retrospective to ${dir}/retro-output.md`,
|
||||
workingDirectory: dir,
|
||||
maxTurns: 25,
|
||||
timeout: 240_000,
|
||||
// 360s, not 240s: same runner-contention class as review-dashboard-via.
|
||||
// /retro is a long multi-step flow — a clean pass measured 225s and the
|
||||
// next CI run timed out at the 240s line (exitReason "timeout", 3/3
|
||||
// attempts). Outer bun timeout below rises to 480s for headroom.
|
||||
timeout: 360_000,
|
||||
testName: 'retro-base-branch',
|
||||
runId,
|
||||
});
|
||||
@@ -444,7 +448,7 @@ Write your retrospective to ${dir}/retro-output.md`,
|
||||
const content = fs.readFileSync(retroPath, 'utf-8');
|
||||
expect(content.length).toBeGreaterThan(100);
|
||||
}
|
||||
}, 300_000);
|
||||
}, 480_000);
|
||||
});
|
||||
|
||||
// --- Retro E2E ---
|
||||
@@ -614,7 +618,13 @@ Skip the preamble, lake intro, telemetry, and all other ship steps.
|
||||
Write the dashboard output to ${dashDir}/dashboard-output.md`,
|
||||
workingDirectory: dashDir,
|
||||
maxTurns: 12,
|
||||
timeout: 180_000,
|
||||
// 300s, not 180s: on a saturated CI runner this file's concurrent
|
||||
// sessions queue behind each other and session STARTUP can eat the
|
||||
// whole budget — observed as deterministic timeout at 0 turns/$0.00
|
||||
// for exactly 180s across 3 attempts (PR #2472 CI + its baseline),
|
||||
// while the 240s-budget tests in the same job passed. Outer bun
|
||||
// timeout below rises to 360s to keep headroom over the inner budget.
|
||||
timeout: 300_000,
|
||||
testName: 'review-dashboard-via',
|
||||
runId,
|
||||
});
|
||||
@@ -648,7 +658,7 @@ Write the dashboard output to ${dashDir}/dashboard-output.md`,
|
||||
);
|
||||
// Ship dashboard should not gate when eng review is clear
|
||||
expect(gateQuestions).toHaveLength(0);
|
||||
}, 240_000);
|
||||
}, 360_000);
|
||||
});
|
||||
|
||||
// Module-level afterAll — finalize eval collector after all tests complete
|
||||
|
||||
Reference in New Issue
Block a user