fix(commands): the removal gate was classified against the wrong root
Found by review after the SUB-WRITE commit, and both defects were in the template rather than the engine every prediction in the fasit was about. `--repo` is what a write target is classified AGAINST. The template passed the SCAN target, and under `--global` that target IS ~/.claude -- so ~/.claude/CLAUDE.md matched `in-repo` and the gate went `silent`. Measured against the real config: gate silent, scopeClass in-repo, 29 removals applied with no approval asked. That is the same silent downgrade #62 measured for a naive .git-upward walk, arriving through a different door, on the one target this chunk was sequenced behind M-BUG-41 to protect. Every other gated template already passed `--repo "$PWD"`; this one was the only outlier. The dry run also could not validate the machine-wide case -- the case that is mandatory in v1. The gate returned before any file was read, so a dry run there reported 29 scope-gate refusals and zero checked spans, and the first run able to find a stale approval would have been the one that writes. A gate guards a WRITE, and a dry run is not one: `requiresApproval` and the disclosures are still reported, so the operator is still asked. The new caller-arm guard was itself red against the corrected template, matching prose that merely NAMES the CLI. Narrowed to lines that invoke it. Guards seen red against the original defects: `--repo "<target-path>"` red, `--repo` omitted red, gate-blocks-dry-run red. Re-dogfooded as the template now calls it: require-ok / user-scope / 29 spans validated / 0 files written. Suite 1659 -> 1662/0. Frozen baselines untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017A6vrtPKsVuM4DJ27p7jzw
This commit is contained in:
parent
000e47f9d2
commit
33bfd5ff5b
5 changed files with 93 additions and 4 deletions
|
|
@ -164,10 +164,16 @@ text no longer matches the file:
|
|||
```
|
||||
|
||||
**7c — dry run first.** Always. It costs one call and proves the spans still
|
||||
match before anything is written:
|
||||
match before anything is written.
|
||||
|
||||
`--repo` is the **session's own root (`$PWD`), never the scanned path**. It is
|
||||
what the target is classified *against*: pass the scan target and
|
||||
`~/.claude/CLAUDE.md` classifies as `in-repo`, which drops the gate to `silent`
|
||||
on the one target that most needs it (measured — the same silent downgrade as a
|
||||
naive `.git`-upward walk, arriving through a different door).
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/scanners/subtraction-write-cli.mjs --approved ~/.claude/config-audit/sessions/{session-id}/subtraction-approved.json --repo "<target-path>" --dry-run --output-file ~/.claude/config-audit/sessions/{session-id}/subtraction-dryrun.json 2>/dev/null; echo $?
|
||||
node ${CLAUDE_PLUGIN_ROOT}/scanners/subtraction-write-cli.mjs --approved ~/.claude/config-audit/sessions/{session-id}/subtraction-approved.json --repo "$PWD" --dry-run --output-file ~/.claude/config-audit/sessions/{session-id}/subtraction-dryrun.json 2>/dev/null; echo $?
|
||||
```
|
||||
|
||||
Read the payload. Exit 3 is a real error (bad or unreadable approval file).
|
||||
|
|
@ -185,7 +191,7 @@ CLAUDE.md in front of you. Without a clear yes, stop here.
|
|||
if the operator gave that go-ahead in 7d:
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/scanners/subtraction-write-cli.mjs --approved ~/.claude/config-audit/sessions/{session-id}/subtraction-approved.json --repo "<target-path>" --output-file ~/.claude/config-audit/sessions/{session-id}/subtraction-result.json 2>/dev/null; echo $?
|
||||
node ${CLAUDE_PLUGIN_ROOT}/scanners/subtraction-write-cli.mjs --approved ~/.claude/config-audit/sessions/{session-id}/subtraction-approved.json --repo "$PWD" --output-file ~/.claude/config-audit/sessions/{session-id}/subtraction-result.json 2>/dev/null; echo $?
|
||||
```
|
||||
|
||||
**7f — report.** From the result payload, tell the user: what was removed (file
|
||||
|
|
|
|||
|
|
@ -166,7 +166,14 @@ export async function applySubtraction(removals, opts = {}) {
|
|||
|
||||
// The gate is a verdict about a write, not a tool failure: the caller renders
|
||||
// the disclosure and asks. Nothing is written, and nothing is exit 3 (#62).
|
||||
if (gate === 'require-ok' && !approveScope) {
|
||||
//
|
||||
// `!dryRun` is load-bearing. The gate guards a WRITE, and a dry run is not
|
||||
// one — refusing it early bought nothing and cost the dry run its whole
|
||||
// purpose on the machine-wide target, which is the mandatory v1 case: the
|
||||
// operator would approve a removal whose spans had never been checked, and
|
||||
// the first run able to discover a stale approval would be the one that
|
||||
// writes. `requiresApproval` is reported either way, so the caller still asks.
|
||||
if (gate === 'require-ok' && !approveScope && !dryRun) {
|
||||
return {
|
||||
...base,
|
||||
refused: normalized.map((r) => ({ ...r, reason: REFUSAL_REASONS.SCOPE_GATE })),
|
||||
|
|
|
|||
|
|
@ -76,6 +76,30 @@ test('every caller dry-runs before it writes', async () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('no caller classifies the write against the path it scanned', async () => {
|
||||
// Measured (#63): `--repo "<target-path>"` under `--global` hands the CLI
|
||||
// `~/.claude` as the session root, so `~/.claude/CLAUDE.md` classifies
|
||||
// `in-repo` and the gate drops to `silent` — 29 removals applied with no
|
||||
// approval asked. `--repo` is what a target is classified AGAINST; it is the
|
||||
// session's own root. Every other gated template already passes `$PWD`.
|
||||
for (const { name, content } of await commandsDrivingTheRemovalCli()) {
|
||||
for (const line of content.split('\n')) {
|
||||
// Invocations only. Prose that merely names the CLI (the Notes section
|
||||
// explaining why removal is not a `fix` action) carries no argv, and
|
||||
// matching it made this guard red against its own fixed template.
|
||||
if (!/^node\s.*subtraction-write-cli\.mjs/.test(line.trim())) continue;
|
||||
const repoArg = line.match(/--repo\s+("[^"]*"|\S+)/);
|
||||
assert.ok(repoArg, `${name} must pass --repo explicitly on every removal-CLI invocation.`);
|
||||
assert.equal(
|
||||
repoArg[1],
|
||||
'"$PWD"',
|
||||
`${name} passes ${repoArg[1]} as --repo. Anything but the session root can classify a\n` +
|
||||
'machine-wide target as in-repo and silently downgrade the strongest gate on this axis.',
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('every caller surfaces the scope gate in the user\'s words', async () => {
|
||||
for (const { name, content } of await commandsDrivingTheRemovalCli()) {
|
||||
assert.match(
|
||||
|
|
|
|||
|
|
@ -102,6 +102,28 @@ describe('subtraction-write-cli', () => {
|
|||
assert.equal(await readFile(userFile, 'utf-8'), FIXTURE);
|
||||
});
|
||||
|
||||
it('dry-runs a machine-wide target: spans validated, gate still reported, nothing written', async () => {
|
||||
// The template's Step 7c promise ("the dry run proves the spans before
|
||||
// anything is written") has to hold on the machine-wide target too — that
|
||||
// is the mandatory v1 case. A gate that also blocked the dry run made 7c
|
||||
// decorative exactly where it mattered.
|
||||
const userFile = join(dir, 'home', '.claude', 'CLAUDE.md');
|
||||
await writeFile(userFile, FIXTURE, 'utf-8');
|
||||
await writeApproval([{ file: userFile, line: 5, endLine: 5, text: BLOCK_A }]);
|
||||
|
||||
const { code } = await run([
|
||||
'--approved', approvedPath, '--repo', repo, '--dry-run', '--output-file', outPath,
|
||||
]);
|
||||
|
||||
assert.equal(code, 0);
|
||||
const payload = JSON.parse(await readFile(outPath, 'utf-8'));
|
||||
assert.equal(payload.requiresApproval, true, 'the operator must still be asked');
|
||||
assert.ok(payload.disclosures.length >= 1);
|
||||
assert.equal(payload.counts.applied, 1, 'the span was actually checked, not refused unseen');
|
||||
assert.equal(payload.counts.filesWritten, 0);
|
||||
assert.equal(await readFile(userFile, 'utf-8'), FIXTURE);
|
||||
});
|
||||
|
||||
it('proceeds on that same target with --approve-scope', async () => {
|
||||
const userFile = join(dir, 'home', '.claude', 'CLAUDE.md');
|
||||
await writeFile(userFile, FIXTURE, 'utf-8');
|
||||
|
|
|
|||
|
|
@ -203,6 +203,36 @@ describe('applySubtraction (filesystem + gate)', () => {
|
|||
assert.equal(await readFile(userFile, 'utf-8'), FIXTURE);
|
||||
});
|
||||
|
||||
it('P8b — a dry run on a require-ok target validates the spans AND still reports the gate', async () => {
|
||||
// The gate guards a WRITE. A dry run is not one, so refusing it early
|
||||
// bought nothing and cost the thing the dry run exists for: on the
|
||||
// machine-wide target — the mandatory v1 case — the operator would have
|
||||
// been asked to approve a removal whose spans had never been checked, and
|
||||
// the first run that could discover a stale approval would be the one that
|
||||
// writes.
|
||||
const home = join(dir, 'home');
|
||||
const userConfig = join(home, '.claude');
|
||||
await mkdir(userConfig, { recursive: true });
|
||||
const userFile = join(userConfig, 'CLAUDE.md');
|
||||
await writeFile(userFile, FIXTURE, 'utf-8');
|
||||
|
||||
const result = await applySubtraction(
|
||||
[
|
||||
{ file: userFile, line: 5, endLine: 5, text: BLOCK_A },
|
||||
{ file: userFile, line: 9, endLine: 9, text: '- Not in this file.' },
|
||||
],
|
||||
{ repoRoot: repo, home, dryRun: true },
|
||||
);
|
||||
|
||||
assert.equal(result.requiresApproval, true, 'the gate must still be reported');
|
||||
assert.ok(result.disclosures.length >= 1);
|
||||
assert.equal(result.applied.length, 1, 'the valid span is validated, not refused unseen');
|
||||
assert.equal(result.refused[0].reason, 'block-mismatch', 'the stale one is found HERE, not at write time');
|
||||
assert.equal(result.filesWritten.length, 0);
|
||||
assert.equal(result.backupId, null);
|
||||
assert.equal(await readFile(userFile, 'utf-8'), FIXTURE, 'a dry run writes nothing, gate or no gate');
|
||||
});
|
||||
|
||||
it('P9 — the same target proceeds once the scope is explicitly approved', async () => {
|
||||
const home = join(dir, 'home');
|
||||
const userConfig = join(home, '.claude');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue