fix(gsd): validate depth verification answer before unlocking write-gate

The tool_result handler called markDepthVerified() whenever
ask_user_questions returned any response with a depth_verification
question ID — without checking what the user actually selected.
Selecting "Not quite", "None of the above", or garbage input all
unlocked the gate.

- Extract isDepthConfirmationAnswer() into write-gate.ts with structural
  validation: cross-references selected answer against the question's
  defined options, only accepting an exact match of the first option
  (confirmation by convention). Rejects free-form "Other" text and
  decouples from any specific label substring.
- Harden block message with explicit anti-bypass language
- Add anti-bypass instructions to all three discuss prompts
- Add 8 new tests covering: structural validation, free-form bypass
  rejection, label-drift resilience, fallback behavior, edge cases

Closes #3749
This commit is contained in:
Jeremy 2026-04-07 19:26:54 -05:00
parent a3ea23bda1
commit dd08f501e5
7 changed files with 177 additions and 5 deletions

View file

@ -6,7 +6,7 @@ import { isToolCallEventType } from "@gsd/pi-coding-agent";
import { buildMilestoneFileName, resolveMilestonePath, resolveSliceFile, resolveSlicePath } from "../paths.js";
import { buildBeforeAgentStartResult } from "./system-context.js";
import { handleAgentEnd } from "./agent-end-recovery.js";
import { clearDiscussionFlowState, isDepthVerified, isQueuePhaseActive, markDepthVerified, resetWriteGateState, shouldBlockContextWrite, shouldBlockQueueExecution } from "./write-gate.js";
import { clearDiscussionFlowState, isDepthVerified, isDepthConfirmationAnswer, isQueuePhaseActive, markDepthVerified, resetWriteGateState, shouldBlockContextWrite, shouldBlockQueueExecution } from "./write-gate.js";
import { isBlockedStateFile, isBashWriteToStateFile, BLOCKED_WRITE_ERROR } from "../write-intercept.js";
import { cleanupQuickBranch } from "../quick.js";
import { getDiscussionMilestoneId } from "../guided-flow.js";
@ -249,7 +249,12 @@ export function registerHooks(pi: ExtensionAPI): void {
const questions: any[] = (event.input as any)?.questions ?? [];
for (const question of questions) {
if (typeof question.id === "string" && question.id.includes("depth_verification")) {
markDepthVerified();
// Only unlock the gate if the user selected the first option (confirmation).
// Cross-references against the question's defined options to reject free-form "Other" text.
const answer = details.response?.answers?.[question.id];
if (isDepthConfirmationAnswer(answer?.selected, question.options)) {
markDepthVerified();
}
break;
}
}

View file

@ -54,6 +54,35 @@ export function markDepthVerified(): void {
depthVerificationDone = true;
}
/**
* Check whether a depth_verification answer confirms the discussion is complete.
* Uses structural validation: the selected answer must exactly match the first
* option label from the question definition (the confirmation option by convention).
* This rejects free-form "Other" text, decline options, and garbage input without
* coupling to any specific label substring.
*
* @param selected The answer's selected value from details.response.answers[id].selected
* @param options The question's options array from event.input.questions[n].options
*/
export function isDepthConfirmationAnswer(
selected: unknown,
options?: Array<{ label?: string }>,
): boolean {
const value = Array.isArray(selected) ? selected[0] : selected;
if (typeof value !== "string" || !value) return false;
// If options are available, structurally validate: selected must exactly match
// the first option (confirmation) label. Rejects free-form "Other" and decline options.
if (Array.isArray(options) && options.length > 0) {
const confirmLabel = options[0]?.label;
return typeof confirmLabel === "string" && value === confirmLabel;
}
// Fallback when options aren't available (e.g., older call sites):
// accept only if it contains "(Recommended)" — the prompt convention suffix.
return value.includes("(Recommended)");
}
export function shouldBlockContextWrite(
toolName: string,
inputPath: string,
@ -71,7 +100,13 @@ export function shouldBlockContextWrite(
return {
block: true,
reason: `Blocked: Cannot write to milestone CONTEXT.md during discussion phase without depth verification. Call ask_user_questions with question id "depth_verification" first to confirm discussion depth before writing context.`,
reason: [
`HARD BLOCK: Cannot write to milestone CONTEXT.md without depth verification.`,
`This is a mechanical gate — you MUST NOT proceed, retry, or rationalize past this block.`,
`Required action: call ask_user_questions with question id containing "depth_verification".`,
`The user MUST select the "(Recommended)" confirmation option to unlock this gate.`,
`If the user declines, cancels, or the tool fails, you must re-ask — not bypass.`,
].join(" "),
};
}

View file

@ -1,6 +1,7 @@
import type { ExtensionAPI } from "@gsd/pi-coding-agent";
export {
isDepthConfirmationAnswer,
isDepthVerified,
isQueuePhaseActive,
setQueuePhaseActive,

View file

@ -114,6 +114,8 @@ If they clarify, absorb the correction and re-verify.
The depth verification is the required write-gate. Do **not** add another meta "ready to proceed?" checkpoint immediately after it unless there is still material ambiguity.
**CRITICAL — Non-bypassable gate:** The system mechanically blocks CONTEXT.md writes until the user selects the "(Recommended)" option. If the user declines, cancels, or the tool fails, you MUST re-ask — never rationalize past the block ("tool not responding, I'll proceed" is forbidden). The gate exists to protect the user's work; treat a block as an instruction, not an obstacle to work around.
## Wrap-up Gate
Once the depth checklist is fully satisfied, move directly into requirements and roadmap preview. Do not insert a separate "are you ready to continue?" gate unless the user explicitly wants to keep brainstorming or you still see material ambiguity.

View file

@ -100,6 +100,8 @@ If they clarify, absorb the correction and re-verify.
The depth verification is the only required confirmation gate. Do not add a second "ready to proceed?" gate after it.
**CRITICAL — Non-bypassable gate:** The system mechanically blocks CONTEXT.md writes until the user selects the "(Recommended)" option. If the user declines, cancels, or the tool fails, you MUST re-ask — never rationalize past the block ("tool not responding, I'll proceed" is forbidden). The gate exists to protect the user's work; treat a block as an instruction, not an obstacle to work around.
---
## Output

View file

@ -103,6 +103,8 @@ The user confirms or corrects before you write. One depth verification per miles
**If you skip this step, the system will block the CONTEXT.md write and return an error telling you to complete verification first.**
**CRITICAL — Non-bypassable gate:** The system mechanically blocks CONTEXT.md writes until the user selects the "(Recommended)" option. If the user declines, cancels, or the tool fails, you MUST re-ask — never rationalize past the block ("tool not responding, I'll proceed" is forbidden). The gate exists to protect the user's work; treat a block as an instruction, not an obstacle to work around.
## Output Phase
Once the user is satisfied, in a single pass for **each** new milestone:

View file

@ -12,6 +12,7 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import {
isDepthConfirmationAnswer,
shouldBlockContextWrite,
isDepthVerified,
isQueuePhaseActive,
@ -117,9 +118,9 @@ test('write-gate: regex does not match slice context files (S01-CONTEXT.md)', ()
assert.strictEqual(result.block, false, 'S01-CONTEXT.md should not be blocked');
});
// ─── Scenario 7: Error message contains actionable instruction ──
// ─── Scenario 7: Error message contains actionable instruction and anti-bypass language ──
test('write-gate: blocked reason contains depth_verification keyword', () => {
test('write-gate: blocked reason contains depth_verification keyword and anti-bypass language', () => {
const result = shouldBlockContextWrite(
'write',
'.gsd/milestones/M999/M999-CONTEXT.md',
@ -129,6 +130,8 @@ test('write-gate: blocked reason contains depth_verification keyword', () => {
assert.strictEqual(result.block, true);
assert.ok(result.reason!.includes('depth_verification'), 'reason should mention depth_verification question id');
assert.ok(result.reason!.includes('ask_user_questions'), 'reason should mention ask_user_questions tool');
assert.ok(result.reason!.includes('MUST NOT'), 'reason should include anti-bypass language');
assert.ok(result.reason!.includes('(Recommended)'), 'reason should specify the required confirmation option');
});
// ─── Scenario 8: Queue mode blocks CONTEXT.md write without depth verification ──
@ -191,3 +194,125 @@ test('write-gate: markDepthVerified unblocks queue-mode writes when milestoneId
clearDiscussionFlowState();
});
// ─── Standard options fixture used across depth confirmation tests ──
const STANDARD_OPTIONS = [
{ label: 'Yes, you got it (Recommended)' },
{ label: 'Not quite — let me clarify' },
];
// ─── Scenario 11: accepts first option (confirmation) with structural validation ──
test('write-gate: isDepthConfirmationAnswer accepts first option with options present', () => {
assert.strictEqual(
isDepthConfirmationAnswer('Yes, you got it (Recommended)', STANDARD_OPTIONS),
true,
'should accept exact match of first option label',
);
});
// ─── Scenario 12: rejects second option (decline) ──
test('write-gate: isDepthConfirmationAnswer rejects decline option', () => {
assert.strictEqual(
isDepthConfirmationAnswer('Not quite — let me clarify', STANDARD_OPTIONS),
false,
'should reject the clarification option',
);
});
// ─── Scenario 13: rejects "None of the above" ──
test('write-gate: isDepthConfirmationAnswer rejects None of the above', () => {
assert.strictEqual(
isDepthConfirmationAnswer('None of the above', STANDARD_OPTIONS),
false,
'should reject None of the above',
);
});
// ─── Scenario 14: rejects garbage/empty input ──
test('write-gate: isDepthConfirmationAnswer rejects garbage and edge cases', () => {
assert.strictEqual(isDepthConfirmationAnswer('discord', STANDARD_OPTIONS), false, 'garbage string');
assert.strictEqual(isDepthConfirmationAnswer('', STANDARD_OPTIONS), false, 'empty string');
assert.strictEqual(isDepthConfirmationAnswer(undefined, STANDARD_OPTIONS), false, 'undefined');
assert.strictEqual(isDepthConfirmationAnswer(null, STANDARD_OPTIONS), false, 'null');
assert.strictEqual(isDepthConfirmationAnswer(42, STANDARD_OPTIONS), false, 'number');
});
// ─── Scenario 15: handles array-wrapped selection ──
test('write-gate: isDepthConfirmationAnswer handles array-wrapped selected value', () => {
assert.strictEqual(
isDepthConfirmationAnswer(['Yes, you got it (Recommended)'], STANDARD_OPTIONS),
true,
'should accept array-wrapped confirmation',
);
assert.strictEqual(
isDepthConfirmationAnswer(['Not quite — let me clarify'], STANDARD_OPTIONS),
false,
'should reject array-wrapped decline',
);
assert.strictEqual(
isDepthConfirmationAnswer([], STANDARD_OPTIONS),
false,
'should reject empty array',
);
});
// ─── Scenario 16: rejects free-form "Other" text that contains "(Recommended)" ──
test('write-gate: isDepthConfirmationAnswer rejects free-form text containing Recommended', () => {
assert.strictEqual(
isDepthConfirmationAnswer('I think this is fine (Recommended)', STANDARD_OPTIONS),
false,
'free-form text with (Recommended) substring must not unlock gate',
);
assert.strictEqual(
isDepthConfirmationAnswer('(Recommended)', STANDARD_OPTIONS),
false,
'bare (Recommended) string must not unlock gate',
);
});
// ─── Scenario 17: works with changed label text (decoupled from specific copy) ──
test('write-gate: isDepthConfirmationAnswer works with different label text', () => {
const customOptions = [
{ label: 'Looks good, proceed' },
{ label: 'Needs more discussion' },
];
assert.strictEqual(
isDepthConfirmationAnswer('Looks good, proceed', customOptions),
true,
'should accept first option regardless of label text',
);
assert.strictEqual(
isDepthConfirmationAnswer('Needs more discussion', customOptions),
false,
'should reject second option',
);
// Old label should NOT work with new options
assert.strictEqual(
isDepthConfirmationAnswer('Yes, you got it (Recommended)', customOptions),
false,
'old label text should not match new options',
);
});
// ─── Scenario 18: fallback when options not available ──
test('write-gate: isDepthConfirmationAnswer falls back to (Recommended) match without options', () => {
assert.strictEqual(
isDepthConfirmationAnswer('Yes, you got it (Recommended)'),
true,
'should accept via fallback when no options provided',
);
assert.strictEqual(
isDepthConfirmationAnswer('Not quite — let me clarify'),
false,
'should reject non-Recommended via fallback',
);
});