fix(#2752): make tool_input.path authoritative over model-controlled file_path in gsd-phase-boundary.sh (#2860)
* test(#2752): path is authoritative over model-controlled file_path in phase-boundary hook Rewrite the #2304 precedence test (which pinned the buggy file_path-wins behavior with an impossible no-tool_name {file_path,path} payload) to assert the correct precedence with realistic Kimi-shaped payloads: a real .planning/ write with a decoy file_path must still fire the reminder (suppression repro), and a write elsewhere with a decoy .planning/ file_path must NOT fabricate one (fabrication repro). Update the parity vocabulary alarm to the new expression. * fix(#2752): make tool_input.path authoritative over model-controlled file_path in gsd-phase-boundary.sh The hook consulted file_path first, path second. kimi-cli executes on path and sends path only; file_path on a Kimi payload is always model-supplied. So a model-supplied decoy file_path could suppress the reminder for a real .planning/ write or fabricate one for a file never touched. Flip the precedence so path is authoritative and file_path is the fallback (Claude Code emits file_path and no path, so the fallback must remain). Mirrors the #2595 JS-guard fix. * chore(#2752): changeset fragment * chore(#2752): clarify comment/changeset — JS guards use upstream normalization, shell hook applies precedence directly (review minor 1) * chore(#2752): backfill changeset PR number (2860) --------- Co-authored-by: Test <test@example.com>
This commit is contained in:
5
.changeset/gentle-ibex-forage.md
Normal file
5
.changeset/gentle-ibex-forage.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2860
|
||||
---
|
||||
**The .planning/ write reminder can no longer be suppressed or fabricated by a model-supplied file_path** — the phase-boundary hook now treats `tool_input.path` (the field kimi-cli actually executes on) as authoritative and `file_path` as the fallback, reaching the same "path authoritative" outcome the JS guards establish via upstream normalization (#2595). Previously a model-controlled decoy `file_path` could silence the reminder for a genuine .planning/ write or raise one naming a file never touched. (#2752)
|
||||
@@ -22,7 +22,15 @@ INPUT=$(cat)
|
||||
# and its file tools name the field `path`, not `file_path` (kimi-cli
|
||||
# src/kimi_cli/tools/file/write.py + replace.py) — fall back to tool_input.path
|
||||
# when file_path is absent, mirroring normalizeKimiPayload in the JS guards.
|
||||
FILE=$(echo "$INPUT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{const i=JSON.parse(d).tool_input||{};process.stdout.write(i.file_path||(typeof i.path==='string'?i.path:'')||'')}catch{}})" 2>/dev/null)
|
||||
# #2752: `path` is AUTHORITATIVE (kimi-cli executes on it; it sends `path` only,
|
||||
# never `file_path`). `file_path` is model-controlled on Kimi, so consulting it
|
||||
# first let a model-supplied decoy suppress/fabricate the reminder. `path` wins,
|
||||
# `file_path` is the fallback (Claude Code emits `file_path` and no `path`, so the
|
||||
# fallback must remain). The JS guards reach the same "path authoritative" outcome
|
||||
# via an upstream normalizeKimiPayload step (copies path→file_path before any guard
|
||||
# reads); this shell hook parses tool_input once, raw, so it applies the precedence
|
||||
# directly at the read site.
|
||||
FILE=$(echo "$INPUT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{const i=JSON.parse(d).tool_input||{};process.stdout.write((typeof i.path==='string'&&i.path)||(typeof i.file_path==='string'&&i.file_path)||'')}catch{}})" 2>/dev/null)
|
||||
|
||||
# Emit a structured JSON envelope (#2974). additionalContext carries the
|
||||
# user-visible reminder text; the typed `planning_modified` boolean and
|
||||
|
||||
@@ -420,22 +420,50 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require
|
||||
assert.strictEqual(parsed.hookSpecificOutput.file_path, '.planning/STATE.md');
|
||||
});
|
||||
|
||||
test('phase-boundary prefers Claude file_path when both fields are present (#2304)', () => {
|
||||
// #2752 — `path` is the AUTHORITATIVE field (kimi-cli executes on it; its file
|
||||
// tools send `path` only). `file_path` is model-controlled on Kimi (kimi-cli never
|
||||
// sends it). The old precedence (`file_path || path`) let a model-supplied decoy
|
||||
// `file_path` suppress the reminder for a real write or fabricate one for a file
|
||||
// never touched. Mirrors the #2595 JS-guard fix: `path` wins, `file_path` is the
|
||||
// fallback (Claude emits `file_path` and no `path`, so the fallback must remain).
|
||||
test('phase-boundary prefers Kimi tool_input.path when both fields are present (#2752)', () => {
|
||||
const hookPath = path.join(HOOKS_DIR, 'gsd-phase-boundary.sh');
|
||||
const input = JSON.stringify({
|
||||
tool_input: { file_path: '.planning/STATE.md', path: 'unrelated.txt' }
|
||||
// Suppression repro: a real .planning/ write WITH a decoy non-empty file_path.
|
||||
const suppressionInput = JSON.stringify({
|
||||
tool_name: 'kimi_cli.tools.file:StrReplaceFile',
|
||||
tool_input: { path: '.planning/STATE.md', file_path: 'unrelated.txt', edit: { old: 'a', new: 'b' } }
|
||||
});
|
||||
|
||||
const result = spawnHook(hookPath, {
|
||||
input,
|
||||
const suppressionResult = spawnHook(hookPath, {
|
||||
input: suppressionInput,
|
||||
encoding: 'utf-8',
|
||||
cwd: tmpDir,
|
||||
});
|
||||
|
||||
assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`);
|
||||
const parsed = JSON.parse(result.stdout);
|
||||
assert.strictEqual(parsed.hookSpecificOutput.file_path, '.planning/STATE.md',
|
||||
'file_path must win over path — normalization is a fallback, not an override');
|
||||
assert.strictEqual(suppressionResult.status, 0, `Should exit 0: ${suppressionResult.stderr}`);
|
||||
const suppressionParsed = JSON.parse(suppressionResult.stdout);
|
||||
assert.strictEqual(suppressionParsed.hookSpecificOutput.planning_modified, true,
|
||||
'A real .planning/STATE.md write must NOT be suppressed by a model-supplied decoy file_path (#2752)');
|
||||
assert.strictEqual(suppressionParsed.hookSpecificOutput.file_path, '.planning/STATE.md',
|
||||
'path must win over file_path — the runtime executes on path, file_path is the fallback');
|
||||
|
||||
// Fabrication repro: a write ELSEWHERE with a decoy file_path pointing into .planning/.
|
||||
const fabricationInput = JSON.stringify({
|
||||
tool_name: 'kimi_cli.tools.file:StrReplaceFile',
|
||||
tool_input: { path: 'src/index.ts', file_path: '.planning/STATE.md', edit: { old: 'a', new: 'b' } }
|
||||
});
|
||||
|
||||
const fabricationResult = spawnHook(hookPath, {
|
||||
input: fabricationInput,
|
||||
encoding: 'utf-8',
|
||||
cwd: tmpDir,
|
||||
});
|
||||
|
||||
assert.strictEqual(fabricationResult.status, 0, `Should exit 0: ${fabricationResult.stderr}`);
|
||||
// No reminder emitted — the write was to src/index.ts; the decoy .planning/
|
||||
// file_path must NOT fabricate a reminder for a file never touched.
|
||||
assert.strictEqual(fabricationResult.stdout, '',
|
||||
'A decoy .planning/ file_path must NOT fabricate a reminder when the real path is outside .planning/ (#2752)');
|
||||
});
|
||||
|
||||
test('phase-boundary negative control: Kimi path outside .planning/ stays silent (#2304)', () => {
|
||||
|
||||
@@ -159,12 +159,13 @@ describe('Kimi shell-guard vocabulary parity (#2304)', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('gsd-phase-boundary.sh falls back to Kimi\'s tool_input.path field', () => {
|
||||
test('gsd-phase-boundary.sh prefers Kimi\'s authoritative tool_input.path, falls back to file_path (#2752)', () => {
|
||||
const src = readHook('hooks/gsd-phase-boundary.sh');
|
||||
assert.ok(
|
||||
src.includes('i.file_path||(typeof i.path===\'string\'?i.path:\'\')'),
|
||||
'gsd-phase-boundary.sh no longer falls back to tool_input.path — ' +
|
||||
'the hook reads an empty path on Kimi (#2304)'
|
||||
src.includes('(typeof i.path===\'string\'&&i.path)||(typeof i.file_path===\'string\'&&i.file_path)||\'\''),
|
||||
'gsd-phase-boundary.sh no longer prefers tool_input.path over file_path — ' +
|
||||
'path is the authoritative field (kimi-cli executes on it); a model-supplied ' +
|
||||
'decoy file_path must not suppress or fabricate a reminder (#2752, mirrors #2595)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user