fix(hooks): state-line-guard ratchets against current size, not a flat gate
Advisor review caught this before the v0.23.0 tag landed: the guard compared the projected line count only against the fixed 60-line max, never against the file's current size, so trimming an already-oversized STATE.md (e.g. 156 -> 100 lines, still over 60 but smaller) was denied exactly like growing it would be. Verified against the real tree: 23 of the machine's STATE.md files are already over 60 lines today, one at 1405. Shipped as a flat gate, this hook would have made most of them un-editable except by a single write landing at <=60 in one shot -- backwards for a guard meant to make trimming possible. Fixed with a ratchet: deny only when the projection is over the max AND larger than the file's current line count (0 for a file that doesn't exist yet), for both Write and Edit. A compliant file still cannot grow past the limit and a new file still cannot be created oversized, but an oversized file can now be edited toward compliance one write at a time. state-line-guard-selftest.sh: 16 -> 21 checks (new section 8: shrink allows, same-size allows, grow-while-oversized still denies, new-oversized still denies). Suite total: 191 + 152 + 69 + 21 = 433. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186kZGKddxfA9N84HqMLbb2
This commit is contained in:
parent
f39c0df929
commit
c1dabf109d
5 changed files with 154 additions and 10 deletions
23
CHANGELOG.md
23
CHANGELOG.md
|
|
@ -24,9 +24,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
`old_string` replaced by `new_string` (every occurrence when `replace_all`
|
||||
is set, mirroring the real Edit tool), so the replace_all case is counted
|
||||
correctly rather than only the first occurrence. Wired into
|
||||
`hooks/hooks.json` as `PreToolUse` on `Write|Edit`. Pinned by
|
||||
`state-line-guard-selftest.sh` (16 checks). Suite total: coord 191 + board
|
||||
152 + route 69 + state-line-guard 16 = 428.
|
||||
`hooks/hooks.json` as `PreToolUse` on `Write|Edit`.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`pre-state-line-guard.mjs` denied trimming an already-oversized STATE.md,
|
||||
not just growing one.** Found by advisor review before the tag landed: the
|
||||
first cut compared the projected line count only against the fixed 60-line
|
||||
max, never against the file's current size, so shrinking a 156-line
|
||||
STATE.md to 100 (still over 60, but smaller) was denied identically to
|
||||
growing it. Verified against the real tree (2026-08-14): 23 of the
|
||||
machine's STATE.md files were already over 60 lines, one at 1405 — shipped
|
||||
as a flat gate, the hook would have made most of them un-editable except by
|
||||
a single write landing at `<=60` in one shot. Fixed with a ratchet: denies
|
||||
only when the projection is over the max *and* larger than the file's
|
||||
current line count (0 for a file that doesn't exist yet), for both `Write`
|
||||
and `Edit`. A compliant file still cannot grow past the limit and a new
|
||||
file still cannot be created oversized, but an oversized file can now be
|
||||
edited toward compliance one write at a time. Pinned by
|
||||
`state-line-guard-selftest.sh` section 8 (4 new checks). Suite total: coord
|
||||
191 + board 152 + route 69 + state-line-guard 21 = 433.
|
||||
|
||||
## [0.22.0] - 2026-08-13
|
||||
|
||||
|
|
|
|||
27
CLAUDE.md
27
CLAUDE.md
|
|
@ -75,7 +75,7 @@ marketplace plugin. Three components, one boundary:
|
|||
otherwise only the first, mirroring what the real Edit tool does. Getting
|
||||
`replace_all` wrong in either direction is not a hypothetical: a hook that
|
||||
only ever replaced the first occurrence would silently pass a bulk edit that
|
||||
balloons the file, so `state-line-guard-selftest.sh` (16 checks) pins a
|
||||
balloons the file, so `state-line-guard-selftest.sh` (21 checks) pins a
|
||||
fixture where only counting every `replace_all` occurrence produces the
|
||||
correct denial. Anything the hook cannot project with confidence — a
|
||||
missing file, an `old_string` that is not present, fields of the wrong
|
||||
|
|
@ -83,6 +83,29 @@ marketplace plugin. Three components, one boundary:
|
|||
here would; the guard only ever touches files named exactly `STATE.md`, at
|
||||
any depth, matching the same basename rule the global session-start hook's
|
||||
nearest-STATE-wins search already uses.
|
||||
|
||||
**It is a RATCHET against the file's current size, not a flat gate at 60 —
|
||||
found by advisor review before the tag landed, not by the selftest, which
|
||||
had no fixture for it.** The first cut compared the projected line count
|
||||
only against `MAX_LINES`, never against what the file already was, so
|
||||
trimming an oversized STATE.md from, say, 156 to 100 lines — still over 60,
|
||||
but strictly smaller — was denied exactly like growing it would have been.
|
||||
Verified empirically against the real tree (2026-08-14):
|
||||
`wc -l ~/repos/*/STATE.md ~/repos/*/*/STATE.md | awk '$1 > 60'` found 23
|
||||
files already over 60 lines, one at 1405. Shipped as a flat gate, this hook
|
||||
would have made most of the machine's STATE.md files un-editable except by
|
||||
a single write landing at `<=60` in one shot — backwards for a guard whose
|
||||
whole point is making the trim the /insights finding asked for actually
|
||||
possible. The fix reads the file's current line count for BOTH tool types
|
||||
(previously only `Edit` read the file at all) and denies only when the
|
||||
projection is over `MAX_LINES` **and** larger than that current count: a
|
||||
compliant file still cannot grow past the limit, a brand-new file still
|
||||
cannot be created oversized (current defaults to 0), but an already-oversized
|
||||
file can always be edited toward compliance, one write at a time, without
|
||||
ever making it worse. Section 8 of the selftest pins all four cases:
|
||||
shrink-while-still-over-limit allows, same-size-rewrite allows, grow-an-
|
||||
already-oversized-file still denies, and create-new-oversized-file still
|
||||
denies.
|
||||
- **Board (`scripts/board.sh`):** cross-repo attention board. Reads STATE.md
|
||||
next-step blocks + board lines, `git status`, and mailbox pending counts, and
|
||||
prints one line per repo. Read-only by construction: it writes to no repo, no
|
||||
|
|
@ -395,7 +418,7 @@ obligations in another repo.
|
|||
`bash scripts/coord-selftest.sh` must exit 0 (191/191),
|
||||
`bash scripts/board-selftest.sh` must exit 0 (152/152),
|
||||
`bash scripts/route-selftest.sh` must exit 0 (69/69) and
|
||||
`bash scripts/state-line-guard-selftest.sh` must exit 0 (16/16).
|
||||
`bash scripts/state-line-guard-selftest.sh` must exit 0 (21/21).
|
||||
- English for all code, docs, and commit messages (public repo). Norwegian
|
||||
trigger aliases in the skill description are deliberate.
|
||||
- Conventional Commits: `type(scope): description`.
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ Session A in repo X leaves a message for repo Y; the next session in repo Y gets
|
|||

|
||||

|
||||

|
||||

|
||||

|
||||
|
||||
---
|
||||
|
||||
|
|
@ -166,7 +166,7 @@ Note that raising the inbox's priority (Rule 7) deliberately does **not** widen
|
|||
bash scripts/coord-selftest.sh # 191 checks against a throwaway mailbox
|
||||
bash scripts/board-selftest.sh # 152 checks against a throwaway repo tree
|
||||
bash scripts/route-selftest.sh # 69 checks, incl. the route->board round trip
|
||||
bash scripts/state-line-guard-selftest.sh # 16 checks, incl. the Edit replace_all projection
|
||||
bash scripts/state-line-guard-selftest.sh # 21 checks, incl. the Edit replace_all projection and the ratchet
|
||||
npm test # all four selftests via node --test
|
||||
|
||||
TDD is the house rule: every behavior change lands with a failing selftest check first.
|
||||
|
|
|
|||
|
|
@ -28,6 +28,17 @@
|
|||
// project confidently (file missing, old_string not found, fields of
|
||||
// the wrong type) is left to the real tool, which will give a clearer
|
||||
// error than a guess here would.
|
||||
// - RATCHET: denies only when the projected line count is BOTH over
|
||||
// MAX_LINES and larger than the file's CURRENT line count (0 for a file
|
||||
// that doesn't exist yet). A file already over the limit is the normal
|
||||
// starting point for a trim, not an edge case - measured on the real
|
||||
// tree 2026-08-14, 23 of the machine's STATE.md files were already over
|
||||
// 60 lines, one at 1405. Comparing only against MAX_LINES (no ratchet)
|
||||
// would deny every incremental trim of those files that doesn't land at
|
||||
// <=60 in one shot - the opposite of what a guard meant to make trimming
|
||||
// possible should do. The ratchet still blocks what the guard exists to
|
||||
// block: a compliant file growing past the limit, or a brand-new file
|
||||
// being created oversized.
|
||||
// - Block: stderr + exit 2
|
||||
// - Allow: exit 0, no output
|
||||
|
||||
|
|
@ -45,6 +56,14 @@ function countLines(text) {
|
|||
return matches ? matches.length : 0;
|
||||
}
|
||||
|
||||
function currentLineCountOf(path) {
|
||||
try {
|
||||
return countLines(readFileSync(path, 'utf-8'));
|
||||
} catch {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
let input;
|
||||
try {
|
||||
input = JSON.parse(readFileSync(0, 'utf-8'));
|
||||
|
|
@ -65,9 +84,11 @@ if (
|
|||
}
|
||||
|
||||
let projected;
|
||||
let currentLines;
|
||||
if (toolName === 'Write') {
|
||||
if (typeof toolInput.content !== 'string') allow();
|
||||
projected = toolInput.content;
|
||||
currentLines = currentLineCountOf(filePath);
|
||||
} else {
|
||||
let current;
|
||||
try {
|
||||
|
|
@ -83,15 +104,17 @@ if (toolName === 'Write') {
|
|||
projected = toolInput.replace_all
|
||||
? current.split(oldStr).join(newStr)
|
||||
: current.replace(oldStr, newStr);
|
||||
currentLines = countLines(current);
|
||||
}
|
||||
|
||||
const lines = countLines(projected);
|
||||
if (lines > MAX_LINES) {
|
||||
if (lines > MAX_LINES && lines > currentLines) {
|
||||
process.stderr.write(
|
||||
`\n[repo-mailbox] STATE LINE GUARD: ${toolName} blocked\n` +
|
||||
` File: ${filePath}\n` +
|
||||
` Projected: ${lines} lines (max ${MAX_LINES} per the STATE.md convention)\n\n` +
|
||||
`Trim STATE.md before writing -- history belongs in git, not STATE.md.\n`
|
||||
` Projected: ${lines} lines (current: ${currentLines}, max ${MAX_LINES} per the STATE.md convention)\n\n` +
|
||||
`This would grow STATE.md further past the limit. Trim it instead -- ` +
|
||||
`any write that reduces the line count is allowed, even if still over ${MAX_LINES}.\n`
|
||||
);
|
||||
process.exit(2);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -208,6 +208,87 @@ process.stdout.write(JSON.stringify({
|
|||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 0 ]; check "Edit: nonexistent file fails open" $?
|
||||
|
||||
# --- 8. Ratchet: an already-oversized file must stay editable --------------
|
||||
# The guard's job is "never let it grow past the limit", not "never let it be
|
||||
# touched again once over the limit". A file already over 60 lines is the
|
||||
# NORMAL case a trim session starts from (measured on the real tree,
|
||||
# 2026-08-14: 23 of the machine's STATE.md files were over 60 lines, one at
|
||||
# 1405). Denying every write that doesn't land at <=60 in a single shot would
|
||||
# make every one of those files un-editable except by a perfect one-shot
|
||||
# rewrite - exactly backwards for a hook meant to make trimming possible.
|
||||
|
||||
FIXTURE3="$TMPDIR/c"
|
||||
mkdir -p "$FIXTURE3"
|
||||
node -e '
|
||||
const fs = require("fs");
|
||||
fs.writeFileSync(process.argv[1], "x\n".repeat(156));
|
||||
' "$FIXTURE3/STATE.md"
|
||||
|
||||
# Write: 156 -> 100 lines. Still over 60, but strictly smaller: allow.
|
||||
P="$(payload "
|
||||
process.stdout.write(JSON.stringify({
|
||||
tool_name: 'Write',
|
||||
tool_input: { file_path: '$FIXTURE3/STATE.md', content: 'x\\n'.repeat(100) }
|
||||
}));
|
||||
")"
|
||||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 0 ]; check "Write: shrinking an oversized file allows, even if still over the limit" $?
|
||||
|
||||
# Write: 156 -> 156 lines (untouched size, e.g. only prose changed): allow.
|
||||
P="$(payload "
|
||||
process.stdout.write(JSON.stringify({
|
||||
tool_name: 'Write',
|
||||
tool_input: { file_path: '$FIXTURE3/STATE.md', content: 'x\\n'.repeat(156) }
|
||||
}));
|
||||
")"
|
||||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 0 ]; check "Write: same-size rewrite of an oversized file allows" $?
|
||||
|
||||
# Write: 156 -> 200 lines. Still growing an already-oversized file: deny.
|
||||
P="$(payload "
|
||||
process.stdout.write(JSON.stringify({
|
||||
tool_name: 'Write',
|
||||
tool_input: { file_path: '$FIXTURE3/STATE.md', content: 'x\\n'.repeat(200) }
|
||||
}));
|
||||
")"
|
||||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 2 ]; check "Write: growing an already-oversized file still denies" $?
|
||||
|
||||
# Write: brand-new STATE.md (no current file) at 61 lines: deny (the ratchet
|
||||
# must not read "no current file" as "anything goes" -- current defaults to 0).
|
||||
P="$(payload '
|
||||
const content = "x\n".repeat(61);
|
||||
process.stdout.write(JSON.stringify({
|
||||
tool_name: "Write",
|
||||
tool_input: { file_path: "/tmp/brand-new-dir-xyz/STATE.md", content }
|
||||
}));
|
||||
')"
|
||||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 2 ]; check "Write: creating a new oversized STATE.md still denies" $?
|
||||
|
||||
# Edit: same ratchet, via the Edit path. Fixture at 156 lines; old_string is
|
||||
# the first 60 "x\n" occurrences (a contiguous substring), new_string is 4 of
|
||||
# them -> projects to 100 lines: still over 60, but smaller than 156. A
|
||||
# pre-ratchet hook denies this (100 > 60); the ratchet must allow it.
|
||||
FIXTURE4="$TMPDIR/d"
|
||||
mkdir -p "$FIXTURE4"
|
||||
node -e '
|
||||
const fs = require("fs");
|
||||
fs.writeFileSync(process.argv[1], "x\n".repeat(156));
|
||||
' "$FIXTURE4/STATE.md"
|
||||
P="$(payload "
|
||||
process.stdout.write(JSON.stringify({
|
||||
tool_name: 'Edit',
|
||||
tool_input: {
|
||||
file_path: '$FIXTURE4/STATE.md',
|
||||
old_string: 'x\\n'.repeat(60),
|
||||
new_string: 'x\\n'.repeat(4)
|
||||
}
|
||||
}));
|
||||
")"
|
||||
run_hook "$P"
|
||||
[ "$HOOK_EXIT" -eq 0 ]; check "Edit: shrinking an oversized file allows, even if still over the limit" $?
|
||||
|
||||
echo ""
|
||||
echo "state-line-guard-selftest: $PASS passed, $FAIL failed"
|
||||
[ "$FAIL" -eq 0 ] || exit 1
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue