Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,10 @@ alone.
| Decisions | `docs/adr/` in the repository they concern | The project |
| The task | `task.md` at the workspace root | The task |

The two top-level documents are held to about 200 lines each. The limit is the
feature: they are read in full by every task, and a document too long to read in one
sitting stops being read.
Both top-level documents are bounded — product design at 200 lines, architecture at
350, the difference being that how a system is built has reference material that
prose cannot replace. The limit is the feature: they are read in full by every task,
and a document too long to read in one sitting stops being read.

Decision records are searched, not browsed — `find_adr` returns the few that bear on
the question. They are written during the design stage as decisions are made, and
Expand Down
448 changes: 179 additions & 269 deletions docs/architecture.md

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions docs/product.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ in.
**The design document nobody reads.** An architecture document grows until reading
it is a project of its own, so nobody does — including Claude, which then infers
the design from whatever files it happened to open. Two top-level documents,
architecture and product design, each held to about 200 lines. The limit is the
architecture and product design, both bounded — 350 lines and 200. The limit is the
feature: a document short enough to read in one sitting can be expected of every
task, and that expectation is what makes it worth maintaining.
task, and that expectation is what makes it worth maintaining. A limit nothing checks
is a wish, so both are held by a test.

**The design document that is no longer true.** Updating it is always a separate
task, and separate tasks do not get scheduled. A task that changes what those
Expand Down
6 changes: 4 additions & 2 deletions skills/init/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ And for the product design, exactly one copy:
Never overwrite a file that exists. Report which ones you created and which were
already there.

Both top-level documents are held to about 200 lines. Say so to the user: the limit
is the point, because they are read by every task.
The top-level documents are bounded: product design at about 200 lines, architecture
at about 350, because how a system is built carries reference material that prose
cannot replace. Say so to the user: the limit is the point, because they are read by
every task.

## Step 5 — Ask for what cannot be discovered

Expand Down
2 changes: 1 addition & 1 deletion skills/init/templates/architecture.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Architecture

<!-- Keep this under about 200 lines. Every task reads it in full, and a document
<!-- Keep this under about 350 lines. Every task reads it in full, and a document
too long to read in one sitting stops being read. Detail that belongs to one
decision goes in docs/adr/ instead. -->

Expand Down
21 changes: 21 additions & 0 deletions test/conventions.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ test('10 — no stale run naming survives', () => {
);
});

// The bound each top-level document is held to. They differ because the jobs do, and
// they are here because only one of them used to be checked: architecture.md reached
// 456 lines while three places in the project said the limit was 200. A limit nothing
// checks is a wish.
const LIMITS = { 'docs/product.md': 200, 'docs/architecture.md': 350 };

test('11 — one product document, under 200 lines', () => {
assert.ok(!existsSync(join(REPO, 'docs/prd.md')), 'docs/prd.md should be gone');
assert.ok(existsSync(join(REPO, 'docs/product.md')), 'docs/product.md should exist');
Expand All @@ -66,6 +72,21 @@ test('11 — one product document, under 200 lines', () => {
);
});

test('53 — both top-level documents are within the bound stated for them', () => {
for (const [doc, limit] of Object.entries(LIMITS)) {
const lines = read(doc).split(/\r?\n/).length;
assert.ok(lines <= limit, `${doc} is ${lines} lines; the limit is ${limit}`);

// A number stated in one place and enforced in another drifts apart silently,
// which is the failure this whole file exists to catch.
const stated = new RegExp(`\\b${limit}\\b`);
assert.match(read(doc), stated, `${doc} should say the bound it is held to`);
}

assert.match(read('README.md'), /200 lines, architecture at\s+350/);
assert.match(read('skills/init/SKILL.md'), /about 200 lines, architecture\s+at about 350/);
});

test('38 — one version, declared in three files, and an install line that reaches it', () => {
const plugin = JSON.parse(read('.claude-plugin/plugin.json'));
const marketplace = JSON.parse(read('.claude-plugin/marketplace.json'));
Expand Down
119 changes: 119 additions & 0 deletions test/refusal.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Test cases 49-52: the one refusal, driven the way Claude Code drives it — as a
// child process fed a PreToolUse event on stdin.
//
// The refusal is a path comparison, so its failure mode is not an error: a path it
// fails to recognise is classified `outside`, and `outside` is allowed. These cases
// spell one file every way a caller might and insist the answer never changes.
//
// What makes that safe today is that both sides of the comparison come from the same
// string: the guard hands `context()` the target's own directory, and compares the
// target against the workspace that call derived. Case, separators and dot segments
// therefore cannot disagree. Case 50 holds that property, which is a property of the
// call site rather than of `classify`, and so could be lost by a caller that passed a
// workspace from anywhere else.

import assert from 'node:assert/strict';
import { spawnSync } from 'node:child_process';
import { mkdirSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import { after, test } from 'node:test';

import { REPO, cleanup, makeProject } from './helpers.mjs';

const GUARD = join(REPO, 'scripts', 'guard-write.mjs');

after(cleanup);

/** A project with one active task at stage 3 — before the design is approved. */
function projectAtStage(stage) {
const root = makeProject();
const branch = 'add-note-search';
mkdirSync(join(root, branch, 'api', 'src'), { recursive: true });
writeFileSync(
join(root, 'tickets', `${branch}.json`),
JSON.stringify({
branch,
description: 'a task',
project: root,
workspace: `${root}/${branch}`,
repos: ['api'],
session_id: null,
stage,
status: 'active',
created_at: new Date().toISOString(),
history: [],
}),
);
return { root, workspace: `${root}/${branch}` };
}

/** Ask the guard about one write. Returns 'deny' or 'allow'. */
function ask(filePath) {
const event = JSON.stringify({ tool_name: 'Write', tool_input: { file_path: filePath } });
const { stdout } = spawnSync(process.execPath, [GUARD], { input: event, encoding: 'utf8' });
if (!stdout.trim()) return 'allow';
return JSON.parse(stdout).hookSpecificOutput?.permissionDecision ?? 'allow';
}

test('49 — a write inside a repository is refused before the design is approved', () => {
const { workspace } = projectAtStage(3);
assert.equal(ask(`${workspace}/api/src/search.ts`), 'deny');
});

test('50 — the refusal does not depend on how the path is spelled', () => {
const { workspace } = projectAtStage(3);
const target = `${workspace}/api/src/search.ts`;

const spellings = [
['dot segments', `${workspace}/api/../api/src/search.ts`],
['a trailing dot segment', `${workspace}/api/src/./search.ts`],
];

// These spell the same file only on Windows. Elsewhere a backslash is an ordinary
// character in a name and case is significant, so each names a different file, and
// folding them would refuse writes the harness never meant to refuse.
if (process.platform === 'win32') {
spellings.push(
['separators', target.split('/').join('\\')],
['a lower-case drive letter', target[0].toLowerCase() + target.slice(1)],
['an upper-cased segment', target.replace('/api/', '/API/')],
['an upper-cased workspace', target.replace(workspace, workspace.toUpperCase())],
);
}

for (const [how, spelling] of spellings) {
assert.equal(ask(spelling), 'deny', `a path written with ${how} reached the file unrefused`);
}
});

test('51 — the documentation paths stay writable, however they are spelled', () => {
const { workspace } = projectAtStage(3);
const writable = [
`${workspace}/task.md`,
`${workspace}/api/docs/architecture.md`,
`${workspace}/api/docs/product.md`,
`${workspace}/api/docs/adr/0001-a-decision.md`,
];

for (const path of writable) {
assert.equal(ask(path), 'allow', `${path} must stay writable in every stage`);
}

// After the design is approved nothing is refused at all.
const approved = projectAtStage(6);
assert.equal(ask(`${approved.workspace}/api/src/search.ts`), 'allow');
});

test('52 — a ticket the guard cannot read leaves the write unrefused', () => {
// Asserted because it is the design, not because it is desirable: a guard that
// cannot tell what stage a task is at allows the write rather than freezing the
// project. The same is true of a crashed or timed-out hook, which Claude Code
// treats as no decision. See docs/architecture.md §10 — this test exists so that
// degradation is a stated property with a case behind it, rather than something a
// reader has to infer from a `catch {}`.
const { root, workspace } = projectAtStage(3);
assert.equal(ask(`${workspace}/api/src/search.ts`), 'deny');

writeFileSync(join(root, 'tickets', 'add-note-search.json'), '{ not json');
assert.equal(ask(`${workspace}/api/src/search.ts`), 'allow');
});
12 changes: 10 additions & 2 deletions test/tickets.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,17 @@ test('32 — the architecture document describes the lifecycle it now has', () =
assert.doesNotMatch(architecture, /No workspace or ticket lifecycle management in version one/);
assert.match(architecture, /eleven tools/);
for (const tool of ['add_ticket', 'set_ticket_status', 'archive_ticket']) {
assert.match(architecture, new RegExp(`\`${tool}\``), `the tool table lists ${tool}`);
assert.match(architecture, new RegExp(`\`${tool}\``), `§8 names ${tool}`);
}
assert.match(architecture, /No deletion, of anything, ever/, 'it still promises nothing is deleted');

// The promise that nothing is deleted is the product's, and is asserted where it
// is made. The architecture states the consequence it has to live with.
assert.match(
readFileSync(join(REPO, 'docs/product.md'), 'utf8'),
/\*\*Delete anything\.\*\* Cleanup is archiving/,
'it still promises nothing is deleted',
);
assert.match(architecture, /Workspaces are never deleted/);

const product = readFileSync(join(REPO, 'docs/product.md'), 'utf8').split(/\r?\n/).length;
assert.ok(product <= 200, `docs/product.md is ${product} lines; the limit is about 200`);
Expand Down