test: second-pass coverage for the memorable bridge

Policy lookup outcomes (plain dir, repo without policy, corrupt .git/config
fails closed); repoPolicyTier timeoutMs; line/col at line starts, after blank
lines, CRLF and first char; tolerant first-JSON-object parsing; keyed rate
limit; uninstall never creates a config just to say off and flips consent in
a GSTACK_STATE_ROOT outside the removed state dir.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-09-09 03:33:11 +00:00
co-authored by Claude Fable 5.1
parent abf5d96400
commit 3b4f955181
4 changed files with 126 additions and 8 deletions
+11 -1
View File
@@ -19,7 +19,7 @@ import * as path from "path";
import * as os from "os";
import { spawnSync } from "child_process";
import { repoPolicyTierBatch } from "../lib/gbrain-repo-policy-client";
import { repoPolicyTier, repoPolicyTierBatch } from "../lib/gbrain-repo-policy-client";
import { canonicalizeRemote } from "../lib/gstack-memory-helpers";
const ROOT = path.resolve(import.meta.dir, "..");
@@ -209,3 +209,13 @@ describe("normalize parity: bash normalize() ↔ canonicalizeRemote (edge URL sh
expect(verdicts.get(canon)).toEqual({ tier: "deny" });
});
});
describe("repoPolicyTier timeoutMs (hook deadline seam)", () => {
test("a spawn that cannot finish inside timeoutMs classifies as unreadable; the default still reads the tier", () => {
const url = "https://github.com/example/timed.git";
expect(run(["set", url, "deny"]).status).toBe(0);
expect(repoPolicyTier(url, env())).toEqual({ tier: "deny" });
// 1 ms cannot cover a bash+jq spawn; the caller's polarity decides what unreadable means
expect(repoPolicyTier(url, env(), 1)).toEqual({ tier: "none", error: "unreadable" });
});
});
+67 -7
View File
@@ -31,7 +31,7 @@ import * as os from 'os';
import * as path from 'path';
import { listReceipts, sha256Hex, verifyLedger } from '../lib/egress-receipt';
import {
budgetFor, budgetMs, capUtf8, logHookError, pickAdditionalContext, renderContext, resolveVendor, safeStderrTail,
budgetFor, budgetMs, capUtf8, firstJsonObject, logHookError, pickAdditionalContext, renderContext, resolveVendor, safeStderrTail,
stringLeaves, stripControl, vendorEnv,
BUDGET_MS, LOG_RATE_LIMIT_MS, OUTPUT_CAP_BYTES, ENVELOPE_SOURCE,
} from '../hosts/claude/hooks/memorable-user-prompt-hook.ts';
@@ -210,11 +210,9 @@ describe('gate on: the mediated hand-off', () => {
}
});
test('a disable that lands mid-flight wins: the gate is re-checked right before the spawn', () => {
// Simulate with a config that reads `on` for the first check and `off` for the second:
// impossible to interleave deterministically from outside, so drive the pure ordering
// through the config store itself — flip the key off right before the hook runs but after
// a warm run proved the on-path works. The observable contract is "off wins": no spawn.
test('gate flipped off between two runs: the second run spawns nothing (the mid-flight re-check itself cannot be interleaved from outside)', () => {
// The pre-spawn re-check reads the same store; a deterministic mid-flight flip would need a
// seam inside main(). This pins the observable contract only: once off, no spawn.
gateOn();
expect(runHook(PROMPT).stdout).toContain('remembered');
spawnSync('bash', [CONFIG, 'set', 'memorable_recall', 'off'], { env, encoding: 'utf8', timeout: 20_000 });
@@ -467,6 +465,16 @@ describe('pure helpers', () => {
expect(pickAdditionalContext(JSON.stringify({ decision: 'block' }))).toBeNull();
expect(pickAdditionalContext('nope')).toBeNull();
});
test('pickAdditionalContext keeps the answer when a background helper appends a line to stdout, or a banner precedes it', () => {
const answer = JSON.stringify({ hookSpecificOutput: { additionalContext: 'kept {"}"} braces in strings' } });
expect(pickAdditionalContext(`${answer}\nhelper: flushed 3 events\n`)).toBe('kept {"}"} braces in strings');
expect(pickAdditionalContext(`memorable v0.5.18\n${answer}`)).toBe('kept {"}"} braces in strings');
expect(pickAdditionalContext(`{\n "hookSpecificOutput": {\n "additionalContext": "pretty"\n }\n}\n`)).toBe('pretty');
expect(firstJsonObject('{"a": {"b": 1}} trailing')).toEqual({ a: { b: 1 } });
expect(firstJsonObject('{"unterminated": ')).toBeNull();
expect(firstJsonObject('no braces here')).toBeNull();
expect(firstJsonObject('{"s": "\\"}"}')).toEqual({ s: '"}' });
});
test('stripControl drops C0 controls, CR and DEL but keeps tab and newline', () => {
const input = 'a' + String.fromCharCode(0) + 'b' + String.fromCharCode(27) + '\tc\nd' + String.fromCharCode(127) + 'e\rf\r\ng';
expect(stripControl(input)).toBe('ab\tc\ndef\ng');
@@ -496,6 +504,10 @@ describe('pure helpers', () => {
expect(lines()).toHaveLength(2);
logHookError('A', t0 + LOG_RATE_LIMIT_MS + 1);
expect(lines()).toHaveLength(3);
// a caller-supplied key rate-limits messages whose text varies (a vendor's timestamped stderr)
logHookError('vendor timeout: at 12:00:01', t0 + LOG_RATE_LIMIT_MS + 2, 'vendor timeout');
logHookError('vendor timeout: at 12:00:02', t0 + LOG_RATE_LIMIT_MS + 3, 'vendor timeout');
expect(lines()).toHaveLength(4);
if (process.platform !== 'win32') expect(fs.statSync(path.join(home, '.gstack', 'hook-errors.log')).mode & 0o077).toBe(0);
} finally {
if (prev === undefined) delete process.env.GSTACK_STATE_ROOT; else process.env.GSTACK_STATE_ROOT = prev;
@@ -611,7 +623,7 @@ describe('deadline and policy failure paths (review coverage)', () => {
expect(r).toEqual({ status: 0, stdout: '', stderr: '' });
expect(calls()).toBe('');
expect(fs.existsSync(ledger())).toBe(false);
expect(errLog()).toContain('trust policy store unreadable');
expect(errLog()).toContain('trust policy lookup failed');
} finally {
fs.rmSync(repo, { recursive: true, force: true });
}
@@ -625,3 +637,51 @@ describe('deadline and policy failure paths (review coverage)', () => {
expect(r.stdout).toContain('remembered');
});
});
describe('trust-policy lookup outcomes (review coverage, second pass)', () => {
function withStore(): void {
// any policy for any url creates the store; the cwd under test has a different or no remote
const set = spawnSync('bash', [POLICY, 'set', 'https://github.com/example/unrelated.git', 'deny'], { env, encoding: 'utf8', timeout: 20_000 });
expect(set.status).toBe(0);
}
test('store present, cwd is a plain directory (not a repo): recall proceeds', () => {
gateOn();
withStore();
const plain = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-memo-plain-'));
try {
const r = runHook(JSON.stringify({ prompt: 'hello', cwd: plain }), {}, plain);
expect(r.stdout).toContain('remembered');
expect(receipts()).toHaveLength(1);
} finally {
fs.rmSync(plain, { recursive: true, force: true });
}
});
test('store present, repo with an origin but no policy for it: recall proceeds', () => {
gateOn();
withStore();
const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-memo-repo-'));
try {
spawnSync('git', ['init', '-q'], { cwd: repo, timeout: 10_000 });
spawnSync('git', ['remote', 'add', 'origin', 'https://github.com/example/other.git'], { cwd: repo, timeout: 10_000 });
expect(runHook(JSON.stringify({ prompt: 'hello', cwd: repo }), {}, repo).stdout).toContain('remembered');
} finally {
fs.rmSync(repo, { recursive: true, force: true });
}
});
test('store present, repository git cannot read (corrupt .git/config): fails closed, nothing spawned', () => {
gateOn();
withStore();
const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-memo-repo-'));
try {
spawnSync('git', ['init', '-q'], { cwd: repo, timeout: 10_000 });
fs.writeFileSync(path.join(repo, '.git', 'config'), '[core\nbroken = ');
const r = runHook(JSON.stringify({ prompt: 'hello', cwd: repo }), {}, repo);
expect(r).toEqual({ status: 0, stdout: '', stderr: '' });
expect(calls()).toBe('');
expect(fs.existsSync(ledger())).toBe(false);
expect(errLog()).toContain('trust policy lookup failed');
} finally {
fs.rmSync(repo, { recursive: true, force: true });
}
});
});
+14
View File
@@ -589,6 +589,20 @@ describe("redactFindingSpans — machine-egress masking (#1947)", () => {
}
});
test("line/col at boundaries: line start, after blank lines, first char, last unterminated line", () => {
const token = "ghp_" + "1234567890abcdefghijklmnopqrstuvwxyz";
const at = (text: string) => {
const f = scan(text, { repoVisibility: "private" }).findings.find((x) => x.id === "github.pat");
expect(f).toBeDefined();
return [f!.line, f!.col];
};
expect(at(`a\nb\n${token} x`)).toEqual([3, 1]);
expect(at(`a\n\n\n ${token}`)).toEqual([4, 3]);
expect(at(token)).toEqual([1, 1]);
expect(at(`one\r\ntwo ${token}`)).toEqual([2, 5]);
expect(redactFindingSpans(`a\nb\n${token} x`, { repoVisibility: "private" })).toBe("a\nb\n<REDACTED-github.pat> x");
});
test("multiline input redacts a finding past the first line (locateSpan line/col path)", () => {
const token = "ghp_" + "1234567890abcdefghijklmnopqrstuvwxyz";
const out = redactFindingSpans(`line one\nline two has ${token}\nline three`, {
+34
View File
@@ -376,6 +376,8 @@ describe('the Memorable arm stays quiet when nothing of its is registered', () =
expect(result.stdout).not.toContain('Memorable UserPromptSubmit hook');
const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8'));
expect(s.hooks.UserPromptSubmit[0].hooks[0].command).toBe('/Users/me/my-own-hook');
// the consent flip only runs when the key reads on: no config file is created just to say off
expect(fs.existsSync(path.join(mockHome, '.gstack', 'config.yaml'))).toBe(false);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
@@ -436,3 +438,35 @@ describe('hook cleanup under lock contention is loud, never silent (review-army)
// per-test budget is too tight on a busy box.
}, 30000);
});
describe('the consent key never outlives the hook, even when the config lives outside the removed state dir', () => {
test('full uninstall (no --keep-state) with GSTACK_STATE_ROOT elsewhere: memorable_recall flips off there', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-uninstall-memo-root-'));
try {
const mockHome = path.join(tmp, 'home');
const otherRoot = path.join(tmp, 'elsewhere');
const installRoot = path.join(mockHome, '.claude', 'skills', 'gstack');
const installBin = path.join(installRoot, 'bin');
fs.mkdirSync(installBin, { recursive: true });
fs.mkdirSync(otherRoot, { recursive: true });
for (const b of ['gstack-uninstall', 'gstack-settings-hook', 'gstack-session-update', 'gstack-config']) {
const dst = path.join(installBin, b);
fs.copyFileSync(path.join(ROOT, 'bin', b), dst);
fs.chmodSync(dst, 0o755);
}
const settingsFile = path.join(mockHome, '.claude', 'settings.json');
fs.writeFileSync(settingsFile, JSON.stringify({ hooks: {} }));
fs.mkdirSync(path.join(mockHome, '.gstack'), { recursive: true });
const env = { ...process.env, HOME: mockHome, GSTACK_SETTINGS_FILE: settingsFile, GSTACK_STATE_ROOT: otherRoot };
expect(spawnSync('bash', [path.join(installBin, 'gstack-config'), 'set', 'memorable_recall', 'on'], { env, timeout: 20_000 }).status).toBe(0);
const result = spawnSync('bash', [path.join(installBin, 'gstack-uninstall'), '--force'], {
stdio: 'pipe', timeout: 30_000, encoding: 'utf-8', cwd: tmp, env,
});
expect(result.status).toBe(0);
expect(fs.existsSync(path.join(mockHome, '.gstack'))).toBe(false); // the default state dir went
expect(fs.readFileSync(path.join(otherRoot, 'config.yaml'), 'utf-8')).toMatch(/memorable_recall: off/); // the real config did not keep consent
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
});