repo-mailbox/tests/selftest.test.mjs
Kjell Tore Guttormsen f39c0df929 feat(hooks): enforce STATE.md's ~60-line convention with a PreToolUse guard
org-ops dispatched a work order (20260814T144553Z) from an /insights sweep
of 160 sessions: a real STATE.md drifted to 155-156 lines before anyone
noticed, and one trim pass on it increased the line count instead of
shrinking it. Prose alone doesn't enforce.

org-ops proposed a PostToolUse hook. Checked against the official hooks
docs first: PostToolUse fires after the tool has already written the file
and cannot block it (confirmed "Can block? No"), only nag afterward. Built
it as PreToolUse instead, the only event that can deny the call before the
file lands.

pre-state-line-guard.mjs denies (stderr + exit 2, matching llm-security's
pre-write-pathguard.mjs) a Write or Edit on any STATE.md whose projected
result exceeds 60 lines. Write projects from the call's own content; Edit
projects from the current on-disk file with old_string replaced by
new_string, honoring replace_all (every occurrence) vs the default (first
occurrence only) the same way the real Edit tool does. Anything the hook
can't project confidently (missing file, old_string not found) is left to
the real tool.

state-line-guard-selftest.sh: 16 checks, including a replace_all fixture
that a first-occurrence-only projection would wrongly allow. Wired into
hooks/hooks.json as PreToolUse on Write|Edit. Version 0.22.0 -> 0.23.0.
Suite total: 191 + 152 + 69 + 16 = 428.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186kZGKddxfA9N84HqMLbb2
2026-08-14 17:01:20 +02:00

131 lines
7.1 KiB
JavaScript

// Node wrapper (marketplace convention: node --test) around the bash
// selftest, which owns every mailbox assertion. The selftest runs against a
// throwaway mailbox (mktemp) and exits non-zero on any failing check.
import { test } from 'node:test';
import assert from 'node:assert';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, writeFileSync, existsSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { basename, dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
const root = join(dirname(fileURLToPath(import.meta.url)), '..');
const hook = join(root, 'hooks', 'scripts', 'session-start.mjs');
test('coord bash selftest passes', () => {
execFileSync('bash', [join(root, 'scripts', 'coord-selftest.sh')], { encoding: 'utf8' });
});
// board.sh reads this plugin's mailbox for its INN column, so the board ships
// here rather than only as a personal script. Pinning the selftest from the
// plugin root is what makes that ownership real: the skill resolves the engine
// through CLAUDE_PLUGIN_ROOT, so a board.sh that exists only in
// ~/.claude/scripts/ would be missing on exactly the path production uses.
test('board bash selftest passes', () => {
execFileSync('bash', [join(root, 'scripts', 'board-selftest.sh')], { encoding: 'utf8' });
});
// route.sh is the WRITER for the next-cost field board.sh already reads, so its
// suite runs the round trip across both scripts. Pinned from the plugin root
// for the same reason as the board: the skill resolves the engine through
// CLAUDE_PLUGIN_ROOT, and a calculator proven only elsewhere is unproven on the
// one path production uses.
test('route bash selftest passes', () => {
execFileSync('bash', [join(root, 'scripts', 'route-selftest.sh')], { encoding: 'utf8' });
});
// pre-state-line-guard.mjs is a PreToolUse hook, so like session-start.mjs it
// must be proven from the plugin root: the hook config resolves it through
// CLAUDE_PLUGIN_ROOT, and a guard proven only elsewhere is unproven on the
// path production actually runs.
test('state-line-guard bash selftest passes', () => {
execFileSync('bash', [join(root, 'scripts', 'state-line-guard-selftest.sh')], { encoding: 'utf8' });
});
// The engine refuses to invent an identity from the cwd, but the hook is the
// FOURTH place repo identity is derived, and a rule enforced in three of four
// places is not a rule: as long as the hook resolved the name itself and passed
// --repo, the engine's guard was bypassed on the only path that runs in
// production. These two tests pin the hook as a pure wrapper - it must not
// resolve identity at all, so the engine's rules apply where they matter.
function runHook(cwd, mailbox, coordRepo) {
// CLAUDE_COORD_REPO is deleted unless a test asks for it: the operator may set
// it globally one day, and a leaked value would silently satisfy the tests
// that exist to prove the hook resolves nothing on its own.
const env = { ...process.env, CLAUDE_COORD_DIR: mailbox };
delete env.CLAUDE_COORD_REPO;
if (coordRepo !== undefined) env.CLAUDE_COORD_REPO = coordRepo;
const out = execFileSync('node', [hook], { cwd, env, encoding: 'utf8' });
return JSON.parse(out);
}
function seedMailbox(mailbox, repo, body) {
mkdirSync(join(mailbox, repo, 'inbox'), { recursive: true });
writeFileSync(join(mailbox, repo, 'inbox', '20260101T000000Z-1-from-someone.md'),
`---\nfrom: someone\nto: ${repo}\nsubject: seeded\ndate: 2026-01-01T00:00:00Z\n---\n${body}\n`);
}
test('hook does not invent a repo identity from the working directory', () => {
const mailbox = mkdtempSync(join(tmpdir(), 'coord-mb-'));
const nonGit = mkdtempSync(join(tmpdir(), 'coord-nogit-'));
// A mailbox that happens to carry the cwd's basename. A hook that falls back
// to basename(cwd) reads it; a hook that leaves identity to the engine does
// not. This is the ~/repos case that delivered mail as the repo "repos".
seedMailbox(mailbox, basename(nonGit), 'CWD-IDENTITY-LEAK');
const parsed = runHook(nonGit, mailbox);
assert.equal(parsed.continue, true);
const ctx = parsed.hookSpecificOutput?.additionalContext ?? '';
assert.ok(!ctx.includes('CWD-IDENTITY-LEAK'),
'hook read a mailbox named after the cwd outside any git repo');
});
test('hook lets the engine derive identity, so the mailbox claim is recorded', () => {
const mailbox = mkdtempSync(join(tmpdir(), 'coord-mb-'));
const repoDir = mkdtempSync(join(tmpdir(), 'coord-repo-'));
execFileSync('git', ['-C', repoDir, 'init', '-q'], { stdio: 'ignore' });
seedMailbox(mailbox, basename(repoDir), 'GIT-IDENTITY-OK');
const parsed = runHook(repoDir, mailbox);
const ctx = parsed.hookSpecificOutput?.additionalContext ?? '';
assert.ok(ctx.includes('GIT-IDENTITY-OK'), 'hook did not deliver the pending message');
// .origin is written only when coord-inbox.sh resolved the repo itself. Its
// presence is the observable proof that the hook stopped overriding identity,
// and its absence is why the collision warning would never fire in production.
assert.ok(existsSync(join(mailbox, basename(repoDir), '.origin')),
'engine never derived the identity: the hook passed --repo and suppressed the claim');
});
// A non-git working surface (~/repos, $HOME) has no derivable identity, and the
// read path declines silently by design - correct, but it means such a surface
// loses its injection with no error and no exit code, which is the same
// loss-looks-like-normal shape 0.6.0 set out to remove. CLAUDE_COORD_REPO lets
// the OPERATOR declare the identity for that surface. This is not the pwd
// fallback returning: the fallback GUESSED from the cwd, while this is a value
// someone wrote down, can read back, and can delete. Identity by declaration.
test('hook honors CLAUDE_COORD_REPO so a non-git surface can declare its identity', () => {
const mailbox = mkdtempSync(join(tmpdir(), 'coord-mb-'));
const nonGit = mkdtempSync(join(tmpdir(), 'coord-nogit-'));
seedMailbox(mailbox, 'declared-surface', 'DECLARED-IDENTITY-OK');
const parsed = runHook(nonGit, mailbox, 'declared-surface');
const ctx = parsed.hookSpecificOutput?.additionalContext ?? '';
assert.ok(ctx.includes('DECLARED-IDENTITY-OK'),
'hook ignored CLAUDE_COORD_REPO: the declared surface got no injection');
});
test('CLAUDE_COORD_REPO is a declaration, so it does not claim the mailbox', () => {
const mailbox = mkdtempSync(join(tmpdir(), 'coord-mb-'));
const repoDir = mkdtempSync(join(tmpdir(), 'coord-repo-'));
execFileSync('git', ['-C', repoDir, 'init', '-q'], { stdio: 'ignore' });
seedMailbox(mailbox, 'declared-surface', 'DECLARED-OVERRIDE');
// Same precedence as an explicit --repo, because that is exactly what it
// becomes: an override never records .origin, or a surface that borrows a
// name would steal the claim from the checkout that owns it.
const parsed = runHook(repoDir, mailbox, 'declared-surface');
const ctx = parsed.hookSpecificOutput?.additionalContext ?? '';
assert.ok(ctx.includes('DECLARED-OVERRIDE'), 'declaration did not override git-derived identity');
assert.ok(!existsSync(join(mailbox, 'declared-surface', '.origin')),
'a declared identity claimed the mailbox; only git-derived reads may claim');
});