Merge pull request #3835 from jeremymcs/fix/gate-enforcement-hardening
fix: fail closed for discussion gate enforcement
This commit is contained in:
Jeremy McSpadden
GitHub
commit
5c31b5f6d7
9 changed files with 342 additions and 101 deletions
|
|
@ -7,6 +7,16 @@ import { loadEffectiveGSDPreferences } from "../preferences.js";
|
|||
import { ensureDbOpen } from "./dynamic-tools.js";
|
||||
import { StringEnum } from "@gsd/pi-ai";
|
||||
import { logError } from "../workflow-logger.js";
|
||||
import { getErrorMessage } from "../error-utils.js";
|
||||
import { shouldBlockContextArtifactSave } from "./write-gate.js";
|
||||
|
||||
const SUPPORTED_SUMMARY_ARTIFACT_TYPES = ["SUMMARY", "RESEARCH", "CONTEXT", "ASSESSMENT", "CONTEXT-DRAFT"] as const;
|
||||
|
||||
export function isSupportedSummaryArtifactType(
|
||||
artifactType: string,
|
||||
): artifactType is (typeof SUPPORTED_SUMMARY_ARTIFACT_TYPES)[number] {
|
||||
return (SUPPORTED_SUMMARY_ARTIFACT_TYPES as readonly string[]).includes(artifactType);
|
||||
}
|
||||
|
||||
/**
|
||||
* Register an alias tool that shares the same execute function as its canonical counterpart.
|
||||
|
|
@ -283,13 +293,23 @@ export function registerDbTools(pi: ExtensionAPI): void {
|
|||
details: { operation: "save_summary", error: "db_unavailable" } as any,
|
||||
};
|
||||
}
|
||||
const validTypes = ["SUMMARY", "RESEARCH", "CONTEXT", "ASSESSMENT"];
|
||||
if (!validTypes.includes(params.artifact_type)) {
|
||||
if (!isSupportedSummaryArtifactType(params.artifact_type)) {
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `Error: Invalid artifact_type "${params.artifact_type}". Must be one of: ${validTypes.join(", ")}` }],
|
||||
content: [{ type: "text" as const, text: `Error: Invalid artifact_type "${params.artifact_type}". Must be one of: ${SUPPORTED_SUMMARY_ARTIFACT_TYPES.join(", ")}` }],
|
||||
details: { operation: "save_summary", error: "invalid_artifact_type" } as any,
|
||||
};
|
||||
}
|
||||
const contextGuard = shouldBlockContextArtifactSave(
|
||||
params.artifact_type,
|
||||
params.milestone_id ?? null,
|
||||
params.slice_id ?? null,
|
||||
);
|
||||
if (contextGuard.block) {
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `Error saving artifact: ${contextGuard.reason ?? "context write blocked"}` }],
|
||||
details: { operation: "save_summary", error: "context_write_blocked" } as any,
|
||||
};
|
||||
}
|
||||
try {
|
||||
let relativePath: string;
|
||||
if (params.task_id && params.slice_id) {
|
||||
|
|
@ -333,16 +353,17 @@ export function registerDbTools(pi: ExtensionAPI): void {
|
|||
"Computes the file path from milestone/slice/task IDs automatically.",
|
||||
promptSnippet: "Save a GSD artifact (summary/research/context/assessment) to DB and disk",
|
||||
promptGuidelines: [
|
||||
"Use gsd_summary_save to persist structured artifacts (SUMMARY, RESEARCH, CONTEXT, ASSESSMENT).",
|
||||
"Use gsd_summary_save to persist structured artifacts (SUMMARY, RESEARCH, CONTEXT, ASSESSMENT, CONTEXT-DRAFT).",
|
||||
"milestone_id is required. slice_id and task_id are optional — they determine the file path.",
|
||||
"The tool computes the relative path automatically: milestones/M001/M001-SUMMARY.md, milestones/M001/slices/S01/S01-SUMMARY.md, etc.",
|
||||
"artifact_type must be one of: SUMMARY, RESEARCH, CONTEXT, ASSESSMENT.",
|
||||
"artifact_type must be one of: SUMMARY, RESEARCH, CONTEXT, ASSESSMENT, CONTEXT-DRAFT.",
|
||||
"Use CONTEXT-DRAFT for incremental draft persistence; use CONTEXT for the final milestone context after depth verification.",
|
||||
],
|
||||
parameters: Type.Object({
|
||||
milestone_id: Type.String({ description: "Milestone ID (e.g. M001)" }),
|
||||
slice_id: Type.Optional(Type.String({ description: "Slice ID (e.g. S01)" })),
|
||||
task_id: Type.Optional(Type.String({ description: "Task ID (e.g. T01)" })),
|
||||
artifact_type: Type.String({ description: "One of: SUMMARY, RESEARCH, CONTEXT, ASSESSMENT" }),
|
||||
artifact_type: Type.String({ description: "One of: SUMMARY, RESEARCH, CONTEXT, ASSESSMENT, CONTEXT-DRAFT" }),
|
||||
content: Type.String({ description: "The full markdown content of the artifact" }),
|
||||
}),
|
||||
execute: summarySaveExecute,
|
||||
|
|
|
|||
|
|
@ -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, isDepthConfirmationAnswer, isQueuePhaseActive, markDepthVerified, resetWriteGateState, shouldBlockContextWrite, shouldBlockQueueExecution, isGateQuestionId, setPendingGate, clearPendingGate, getPendingGate, shouldBlockPendingGate, shouldBlockPendingGateBash } from "./write-gate.js";
|
||||
import { clearDiscussionFlowState, isDepthConfirmationAnswer, isQueuePhaseActive, markDepthVerified, resetWriteGateState, shouldBlockContextWrite, shouldBlockQueueExecution, isGateQuestionId, setPendingGate, clearPendingGate, getPendingGate, shouldBlockPendingGate, shouldBlockPendingGateBash, extractDepthVerificationMilestoneId } 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";
|
||||
|
|
@ -24,6 +24,7 @@ import { logWarning as safetyLogWarning } from "../workflow-logger.js";
|
|||
import { installNotifyInterceptor } from "./notify-interceptor.js";
|
||||
import { initNotificationStore } from "../notification-store.js";
|
||||
import { initNotificationWidget } from "../notification-widget.js";
|
||||
import { initHealthWidget } from "../health-widget.js";
|
||||
|
||||
// Skip the welcome screen on the very first session_start — cli.ts already
|
||||
// printed it before the TUI launched. Only re-print on /clear (subsequent sessions).
|
||||
|
|
@ -39,6 +40,7 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
initNotificationStore(process.cwd());
|
||||
installNotifyInterceptor(ctx);
|
||||
initNotificationWidget(ctx);
|
||||
initHealthWidget(ctx);
|
||||
resetWriteGateState();
|
||||
resetToolCallLoopGuard();
|
||||
resetAskUserQuestionsCache();
|
||||
|
|
@ -162,24 +164,25 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
});
|
||||
|
||||
pi.on("tool_call", async (event) => {
|
||||
const discussionBasePath = process.cwd();
|
||||
// ── Loop guard: block repeated identical tool calls ──
|
||||
const loopCheck = checkToolCallLoop(event.toolName, event.input as Record<string, unknown>);
|
||||
if (loopCheck.block) {
|
||||
return { block: true, reason: loopCheck.reason };
|
||||
}
|
||||
|
||||
// ── Discussion gate enforcement: track pending questions ─────────
|
||||
// During a discussion flow, EVERY ask_user_questions call matters.
|
||||
// When ask_user_questions is called, mark it as pending. It stays
|
||||
// pending until the user responds. This prevents the model from
|
||||
// continuing if ask_user_questions fails, errors, or is cancelled.
|
||||
// ── Discussion gate enforcement: track pending gate questions ─────────
|
||||
// Only gate-shaped ask_user_questions calls should block execution.
|
||||
// The gate stays pending until the user selects the approval option.
|
||||
if (event.toolName === "ask_user_questions") {
|
||||
const milestoneId = getDiscussionMilestoneId();
|
||||
const milestoneId = getDiscussionMilestoneId(discussionBasePath);
|
||||
const inDiscussion = milestoneId !== null || isQueuePhaseActive();
|
||||
if (inDiscussion) {
|
||||
const questions: any[] = (event.input as any)?.questions ?? [];
|
||||
const questionId = questions[0]?.id ?? "ask_user_questions";
|
||||
setPendingGate(typeof questionId === "string" ? questionId : "ask_user_questions");
|
||||
const questionId = questions.find((question) => typeof question?.id === "string" && isGateQuestionId(question.id))?.id;
|
||||
if (typeof questionId === "string") {
|
||||
setPendingGate(questionId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -187,7 +190,7 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
// If ask_user_questions was called with a gate ID but hasn't been confirmed,
|
||||
// block all non-read-only tool calls to prevent the model from skipping gates.
|
||||
if (getPendingGate()) {
|
||||
const milestoneId = getDiscussionMilestoneId();
|
||||
const milestoneId = getDiscussionMilestoneId(discussionBasePath);
|
||||
if (isToolCallEventType("bash", event)) {
|
||||
const bashGuard = shouldBlockPendingGateBash(
|
||||
event.input.command,
|
||||
|
|
@ -247,8 +250,7 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
const result = shouldBlockContextWrite(
|
||||
event.toolName,
|
||||
event.input.path,
|
||||
getDiscussionMilestoneId(),
|
||||
isDepthVerified(),
|
||||
getDiscussionMilestoneId(discussionBasePath),
|
||||
isQueuePhaseActive(),
|
||||
);
|
||||
if (result.block) return result;
|
||||
|
|
@ -276,7 +278,7 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
|
||||
pi.on("tool_result", async (event) => {
|
||||
if (event.toolName !== "ask_user_questions") return;
|
||||
const milestoneId = getDiscussionMilestoneId();
|
||||
const milestoneId = getDiscussionMilestoneId(process.cwd());
|
||||
const queueActive = isQueuePhaseActive();
|
||||
if (!milestoneId && !queueActive) return;
|
||||
|
||||
|
|
@ -294,9 +296,13 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
// Gate stays pending — model will be blocked from non-read-only tools
|
||||
// until it re-asks and gets a valid response
|
||||
} else {
|
||||
// User responded (confirmed or requested adjustment) — clear the pending gate.
|
||||
// The prompt-level instructions handle the "needs adjustment" re-ask flow.
|
||||
clearPendingGate();
|
||||
const pendingQuestion = questions.find((question) => question?.id === currentPendingGate);
|
||||
if (pendingQuestion) {
|
||||
const answer = details.response?.answers?.[currentPendingGate];
|
||||
if (isDepthConfirmationAnswer(answer?.selected, pendingQuestion.options)) {
|
||||
clearPendingGate();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -308,7 +314,7 @@ export function registerHooks(pi: ExtensionAPI): void {
|
|||
// 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();
|
||||
markDepthVerified(extractDepthVerificationMilestoneId(question.id) ?? milestoneId);
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
const MILESTONE_CONTEXT_RE = /M\d+(?:-[a-z0-9]{6})?-CONTEXT\.md$/;
|
||||
const CONTEXT_MILESTONE_RE = /(?:^|[/\\])(M\d+(?:-[a-z0-9]{6})?)-CONTEXT\.md$/i;
|
||||
const DEPTH_VERIFICATION_MILESTONE_RE = /depth_verification[_-](M\d+(?:-[a-z0-9]{6})?)/i;
|
||||
|
||||
/**
|
||||
* Path segment that identifies .gsd/ planning artifacts.
|
||||
|
|
@ -26,7 +28,7 @@ const QUEUE_SAFE_TOOLS = new Set([
|
|||
*/
|
||||
const BASH_READ_ONLY_RE = /^\s*(cat|head|tail|less|more|wc|file|stat|du|df|which|type|echo|printf|ls|find|grep|rg|awk|sed\b(?!.*-i)|sort|uniq|diff|comm|tr|cut|tee\s+-a\s+\/dev\/null|git\s+(log|show|diff|status|branch|tag|remote|rev-parse|ls-files|blame|shortlog|describe|stash\s+list|config\s+--get|cat-file)|gh\s+(issue|pr|api|repo|release)\s+(view|list|diff|status|checks)|mkdir\s+-p\s+\.gsd|rtk\s)/;
|
||||
|
||||
let depthVerificationDone = false;
|
||||
const verifiedDepthMilestones = new Set<string>();
|
||||
let activeQueuePhase = false;
|
||||
|
||||
/**
|
||||
|
|
@ -64,7 +66,15 @@ const GATE_SAFE_TOOLS = new Set([
|
|||
]);
|
||||
|
||||
export function isDepthVerified(): boolean {
|
||||
return depthVerificationDone;
|
||||
return verifiedDepthMilestones.size > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a specific milestone has passed depth verification.
|
||||
*/
|
||||
export function isMilestoneDepthVerified(milestoneId: string | null | undefined): boolean {
|
||||
if (!milestoneId) return false;
|
||||
return verifiedDepthMilestones.has(milestoneId);
|
||||
}
|
||||
|
||||
export function isQueuePhaseActive(): boolean {
|
||||
|
|
@ -76,18 +86,19 @@ export function setQueuePhaseActive(active: boolean): void {
|
|||
}
|
||||
|
||||
export function resetWriteGateState(): void {
|
||||
depthVerificationDone = false;
|
||||
verifiedDepthMilestones.clear();
|
||||
pendingGateId = null;
|
||||
}
|
||||
|
||||
export function clearDiscussionFlowState(): void {
|
||||
depthVerificationDone = false;
|
||||
verifiedDepthMilestones.clear();
|
||||
activeQueuePhase = false;
|
||||
pendingGateId = null;
|
||||
}
|
||||
|
||||
export function markDepthVerified(): void {
|
||||
depthVerificationDone = true;
|
||||
export function markDepthVerified(milestoneId?: string | null): void {
|
||||
if (!milestoneId) return;
|
||||
verifiedDepthMilestones.add(milestoneId);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -97,6 +108,23 @@ export function isGateQuestionId(questionId: string): boolean {
|
|||
return GATE_QUESTION_PATTERNS.some(pattern => questionId.includes(pattern));
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the milestone ID embedded in a depth-verification question id.
|
||||
* Prompts are expected to use ids like `depth_verification_M001_confirm`.
|
||||
*/
|
||||
export function extractDepthVerificationMilestoneId(questionId: string): string | null {
|
||||
const match = questionId.match(DEPTH_VERIFICATION_MILESTONE_RE);
|
||||
return match?.[1] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the milestone ID from a milestone CONTEXT file path.
|
||||
*/
|
||||
function extractContextMilestoneId(inputPath: string): string | null {
|
||||
const match = inputPath.match(CONTEXT_MILESTONE_RE);
|
||||
return match?.[1] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark a gate as pending (called when ask_user_questions is invoked with a gate ID).
|
||||
*/
|
||||
|
|
@ -127,15 +155,11 @@ export function getPendingGate(): string | null {
|
|||
*/
|
||||
export function shouldBlockPendingGate(
|
||||
toolName: string,
|
||||
milestoneId: string | null,
|
||||
queuePhaseActive?: boolean,
|
||||
_milestoneId: string | null,
|
||||
_queuePhaseActive?: boolean,
|
||||
): { block: boolean; reason?: string } {
|
||||
if (!pendingGateId) return { block: false };
|
||||
|
||||
const inDiscussion = milestoneId !== null;
|
||||
const inQueue = queuePhaseActive ?? false;
|
||||
if (!inDiscussion && !inQueue) return { block: false };
|
||||
|
||||
if (GATE_SAFE_TOOLS.has(toolName)) return { block: false };
|
||||
|
||||
// Bash read-only commands are also safe
|
||||
|
|
@ -159,15 +183,11 @@ export function shouldBlockPendingGate(
|
|||
*/
|
||||
export function shouldBlockPendingGateBash(
|
||||
command: string,
|
||||
milestoneId: string | null,
|
||||
queuePhaseActive?: boolean,
|
||||
_milestoneId: string | null,
|
||||
_queuePhaseActive?: boolean,
|
||||
): { block: boolean; reason?: string } {
|
||||
if (!pendingGateId) return { block: false };
|
||||
|
||||
const inDiscussion = milestoneId !== null;
|
||||
const inQueue = queuePhaseActive ?? false;
|
||||
if (!inDiscussion && !inQueue) return { block: false };
|
||||
|
||||
// Allow read-only bash commands
|
||||
if (BASH_READ_ONLY_RE.test(command)) return { block: false };
|
||||
|
||||
|
|
@ -215,16 +235,24 @@ export function shouldBlockContextWrite(
|
|||
toolName: string,
|
||||
inputPath: string,
|
||||
milestoneId: string | null,
|
||||
depthVerified: boolean,
|
||||
queuePhaseActive?: boolean,
|
||||
_queuePhaseActive?: boolean,
|
||||
): { block: boolean; reason?: string } {
|
||||
if (toolName !== "write") return { block: false };
|
||||
|
||||
const inDiscussion = milestoneId !== null;
|
||||
const inQueue = queuePhaseActive ?? false;
|
||||
if (!inDiscussion && !inQueue) return { block: false };
|
||||
if (!MILESTONE_CONTEXT_RE.test(inputPath)) return { block: false };
|
||||
if (depthVerified) return { block: false };
|
||||
|
||||
const targetMilestoneId = extractContextMilestoneId(inputPath) ?? milestoneId;
|
||||
if (!targetMilestoneId) {
|
||||
return {
|
||||
block: true,
|
||||
reason: [
|
||||
`HARD BLOCK: Cannot write milestone CONTEXT.md without knowing which milestone it belongs to.`,
|
||||
`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" and the milestone id.`,
|
||||
].join(" "),
|
||||
};
|
||||
}
|
||||
|
||||
if (isMilestoneDepthVerified(targetMilestoneId)) return { block: false };
|
||||
|
||||
return {
|
||||
block: true,
|
||||
|
|
@ -238,6 +266,40 @@ export function shouldBlockContextWrite(
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a gsd_summary_save CONTEXT artifact should be blocked.
|
||||
* Slice-level CONTEXT artifacts are allowed; milestone-level CONTEXT writes
|
||||
* require the milestone to be depth-verified first.
|
||||
*/
|
||||
export function shouldBlockContextArtifactSave(
|
||||
artifactType: string,
|
||||
milestoneId: string | null,
|
||||
sliceId?: string | null,
|
||||
): { block: boolean; reason?: string } {
|
||||
if (artifactType !== "CONTEXT") return { block: false };
|
||||
if (sliceId) return { block: false };
|
||||
if (!milestoneId) {
|
||||
return {
|
||||
block: true,
|
||||
reason: [
|
||||
`HARD BLOCK: Cannot save milestone CONTEXT without a milestone_id.`,
|
||||
`This is a mechanical gate — you MUST NOT proceed, retry, or rationalize past this block.`,
|
||||
].join(" "),
|
||||
};
|
||||
}
|
||||
if (isMilestoneDepthVerified(milestoneId)) return { block: false };
|
||||
|
||||
return {
|
||||
block: true,
|
||||
reason: [
|
||||
`HARD BLOCK: Cannot save milestone CONTEXT without depth verification for ${milestoneId}.`,
|
||||
`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_${milestoneId}".`,
|
||||
`The user MUST select the "(Recommended)" confirmation option to unlock this gate.`,
|
||||
].join(" "),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Queue-mode execution guard (#2545).
|
||||
*
|
||||
|
|
@ -283,7 +345,10 @@ export function shouldBlockQueueExecution(
|
|||
};
|
||||
}
|
||||
|
||||
// Unknown tools — allow by default (custom extension tools, etc.)
|
||||
return { block: false };
|
||||
// Unknown tools — block by default in queue mode so custom tools cannot
|
||||
// bypass execution restrictions.
|
||||
return {
|
||||
block: true,
|
||||
reason: `Blocked: /gsd queue is a planning tool — it creates milestones, not executes work. Unknown tools are not permitted during queue mode.`,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -186,12 +186,13 @@ export function checkAutoStartAfterDiscuss(): boolean {
|
|||
// Parse PROJECT.md for milestone sequence, warn if any are missing context.
|
||||
// Don't block — milestones can be intentionally queued without context.
|
||||
const projectFile = resolveGsdRootFile(basePath, "PROJECT");
|
||||
let projectIds: string[] = [];
|
||||
if (projectFile) {
|
||||
try {
|
||||
const projectContent = readFileSync(projectFile, "utf-8");
|
||||
const milestoneIds = parseMilestoneSequenceFromProject(projectContent);
|
||||
if (milestoneIds.length > 1) {
|
||||
const missing = milestoneIds.filter(id => {
|
||||
projectIds = parseMilestoneSequenceFromProject(projectContent);
|
||||
if (projectIds.length > 1) {
|
||||
const missing = projectIds.filter(id => {
|
||||
const hasContext = !!resolveMilestoneFile(basePath, id, "CONTEXT");
|
||||
const hasDraft = !!resolveMilestoneFile(basePath, id, "CONTEXT-DRAFT");
|
||||
const hasDir = existsSync(join(gsdRoot(basePath), "milestones", id));
|
||||
|
|
@ -210,9 +211,17 @@ export function checkAutoStartAfterDiscuss(): boolean {
|
|||
|
||||
// Gate 4: Discussion manifest process verification (multi-milestone only)
|
||||
// The LLM writes DISCUSSION-MANIFEST.json after each Phase 3 gate decision.
|
||||
// If the manifest exists but gates_completed < total, the LLM hasn't finished
|
||||
// presenting all readiness gates to the user — block auto-start.
|
||||
// If the project is multi-milestone, the manifest is required. When it is
|
||||
// missing, fail closed instead of assuming the discussion finished.
|
||||
const manifestPath = join(gsdRoot(basePath), "DISCUSSION-MANIFEST.json");
|
||||
const requiresManifest = projectIds.length > 1 || findMilestoneIds(basePath).length > 1;
|
||||
if (requiresManifest && !existsSync(manifestPath)) {
|
||||
ctx.ui.notify(
|
||||
"Multi-milestone discussion manifest is missing. Auto-start will remain paused until the manifest is written.",
|
||||
"warning",
|
||||
);
|
||||
return false;
|
||||
}
|
||||
if (existsSync(manifestPath)) {
|
||||
try {
|
||||
const manifest = JSON.parse(readFileSync(manifestPath, "utf-8"));
|
||||
|
|
@ -225,9 +234,7 @@ export function checkAutoStartAfterDiscuss(): boolean {
|
|||
}
|
||||
|
||||
// Cross-check manifest milestones against PROJECT.md if available
|
||||
if (projectFile) {
|
||||
const projectContent = readFileSync(projectFile, "utf-8");
|
||||
const projectIds = parseMilestoneSequenceFromProject(projectContent);
|
||||
if (projectIds.length > 0) {
|
||||
const manifestIds = Object.keys(manifest.milestones ?? {});
|
||||
const untracked = projectIds.filter(id => !manifestIds.includes(id));
|
||||
if (untracked.length > 0) {
|
||||
|
|
|
|||
|
|
@ -245,6 +245,41 @@ describe('gsd-tools', () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('gsd_summary_save supports CONTEXT-DRAFT persistence', async () => {
|
||||
const tmpDir = makeTmpDir();
|
||||
try {
|
||||
const dbPath = path.join(tmpDir, '.gsd', 'gsd.db');
|
||||
openDatabase(dbPath);
|
||||
|
||||
await saveArtifactToDb(
|
||||
{
|
||||
path: 'milestones/M001/M001-CONTEXT-DRAFT.md',
|
||||
artifact_type: 'CONTEXT-DRAFT',
|
||||
content: '# M001 Draft Context\n\nDraft notes.',
|
||||
milestone_id: 'M001',
|
||||
},
|
||||
tmpDir,
|
||||
);
|
||||
|
||||
const draftPath = path.join(tmpDir, '.gsd', 'milestones', 'M001', 'M001-CONTEXT-DRAFT.md');
|
||||
assert.ok(fs.existsSync(draftPath), 'Draft context file should be created');
|
||||
const draftContent = fs.readFileSync(draftPath, 'utf-8');
|
||||
assert.ok(draftContent.includes('Draft Context'), 'Draft context file should contain draft content');
|
||||
|
||||
const adapter = _getAdapter();
|
||||
assert.ok(adapter !== null, 'Adapter should be available');
|
||||
const rows = adapter!.prepare(
|
||||
"SELECT * FROM artifacts WHERE path = 'milestones/M001/M001-CONTEXT-DRAFT.md'",
|
||||
).all();
|
||||
assert.deepStrictEqual(rows.length, 1, 'Should have 1 draft artifact row');
|
||||
assert.deepStrictEqual(rows[0]['artifact_type'] as string, 'CONTEXT-DRAFT', 'Artifact type should be CONTEXT-DRAFT');
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
cleanupDir(tmpDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('DB unavailable error paths', async () => {
|
||||
// (d) All tools return isError when DB unavailable
|
||||
// Close any open DB and don't open a new one
|
||||
|
|
|
|||
|
|
@ -10,11 +10,15 @@
|
|||
|
||||
import { describe, test, beforeEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import {
|
||||
getDiscussionMilestoneId,
|
||||
setPendingAutoStart,
|
||||
clearPendingAutoStart,
|
||||
checkAutoStartAfterDiscuss,
|
||||
} from "../guided-flow.ts";
|
||||
|
||||
// ─── Tests ─────────────────────────────────────────────────────────────────
|
||||
|
|
@ -95,3 +99,33 @@ describe("#2985 Bug 4 — getDiscussionMilestoneId must be keyed by basePath", (
|
|||
assert.equal(result, "M001", "should return the only active milestone for backward compat");
|
||||
});
|
||||
});
|
||||
|
||||
test("checkAutoStartAfterDiscuss fails closed when a multi-milestone manifest is missing", () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-auto-start-manifest-"));
|
||||
try {
|
||||
const gsdDir = join(base, ".gsd");
|
||||
const milestoneDir = join(gsdDir, "milestones", "M001");
|
||||
mkdirSync(milestoneDir, { recursive: true });
|
||||
mkdirSync(join(gsdDir, "milestones", "M002"), { recursive: true });
|
||||
writeFileSync(
|
||||
join(gsdDir, "PROJECT.md"),
|
||||
`# Project\n\n| M001 | First milestone | active |\n| M002 | Second milestone | queued |\n`,
|
||||
);
|
||||
writeFileSync(join(gsdDir, "STATE.md"), "# State\n");
|
||||
writeFileSync(join(milestoneDir, "M001-CONTEXT.md"), "# M001 Context\n");
|
||||
|
||||
clearPendingAutoStart();
|
||||
setPendingAutoStart(base, {
|
||||
basePath: base,
|
||||
milestoneId: "M001",
|
||||
ctx: { ui: { notify: () => undefined } } as any,
|
||||
pi: { setActiveTools: () => undefined, getActiveTools: () => [] } as any,
|
||||
});
|
||||
|
||||
const started = checkAutoStartAfterDiscuss();
|
||||
assert.equal(started, false, "auto-start should fail closed without the manifest");
|
||||
} finally {
|
||||
clearPendingAutoStart();
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
formatRelativeTime,
|
||||
type HealthWidgetData,
|
||||
} from "../health-widget-core.ts";
|
||||
import { registerHooks } from "../bootstrap/register-hooks.ts";
|
||||
|
||||
function makeTempDir(prefix: string): string {
|
||||
const dir = join(
|
||||
|
|
@ -177,3 +178,47 @@ test("detectHealthWidgetProjectState: metrics file alone does not imply project"
|
|||
);
|
||||
assert.equal(detectHealthWidgetProjectState(dir), "initialized");
|
||||
});
|
||||
|
||||
test("session_start bootstraps the health widget alongside notifications", async (t) => {
|
||||
const dir = makeTempDir("bootstrap");
|
||||
mkdirSync(join(dir, ".gsd"), { recursive: true });
|
||||
|
||||
const originalCwd = process.cwd();
|
||||
process.chdir(dir);
|
||||
t.after(() => {
|
||||
process.chdir(originalCwd);
|
||||
cleanup(dir);
|
||||
});
|
||||
|
||||
const widgets: string[] = [];
|
||||
const handlers = new Map<string, (event: unknown, ctx: any) => Promise<void> | void>();
|
||||
const pi = {
|
||||
on(event: string, handler: (event: unknown, ctx: any) => Promise<void> | void) {
|
||||
handlers.set(event, handler);
|
||||
},
|
||||
} as any;
|
||||
|
||||
registerHooks(pi);
|
||||
const sessionStart = handlers.get("session_start");
|
||||
assert.ok(sessionStart, "session_start handler is registered");
|
||||
|
||||
await sessionStart!({}, {
|
||||
hasUI: true,
|
||||
ui: {
|
||||
notify: () => {},
|
||||
setStatus: () => {},
|
||||
setWorkingMessage: () => {},
|
||||
onTerminalInput: () => () => {},
|
||||
setWidget: (key: string) => {
|
||||
widgets.push(key);
|
||||
},
|
||||
},
|
||||
sessionManager: {
|
||||
getSessionId: () => null,
|
||||
},
|
||||
model: null,
|
||||
} as any);
|
||||
|
||||
assert.ok(widgets.includes("gsd-health"), "health widget is bootstrapped");
|
||||
assert.ok(widgets.includes("gsd-notifications"), "notification widget still boots");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@
|
|||
* (e) write/edit to source path → block
|
||||
* (f) bash command → block (could execute work)
|
||||
* (g) registered GSD tools (gsd_milestone_generate_id, gsd_summary_save) → pass
|
||||
* (h) unknown custom tools → block
|
||||
*/
|
||||
|
||||
import test from 'node:test';
|
||||
|
|
@ -155,3 +156,11 @@ test('queue-guard: allows web search and library tools during queue mode', () =>
|
|||
const r4 = shouldBlockQueueExecution('fetch_page', '', true);
|
||||
assert.strictEqual(r4.block, false, 'fetch_page should pass');
|
||||
});
|
||||
|
||||
// ─── Scenario 10: Unknown custom tools are blocked during queue mode ──
|
||||
|
||||
test('queue-guard: blocks unknown custom tools during queue mode', () => {
|
||||
const result = shouldBlockQueueExecution('custom_codegen_tool', '', true);
|
||||
assert.strictEqual(result.block, true, 'unknown custom tools should be blocked');
|
||||
assert.ok(result.reason, 'should explain the queue restriction');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3,9 +3,9 @@
|
|||
*
|
||||
* Exercises shouldBlockContextWrite() — a pure function that implements:
|
||||
* (a) toolName !== "write" → pass
|
||||
* (b) milestoneId null → pass (not in discussion)
|
||||
* (b) milestone context must resolve to a verified milestone
|
||||
* (c) path doesn't match /M\d+-CONTEXT\.md$/ → pass
|
||||
* (d) depthVerified → pass
|
||||
* (d) non-context files → pass
|
||||
* (e) else → block with actionable reason
|
||||
*/
|
||||
|
||||
|
|
@ -14,12 +14,12 @@ import assert from 'node:assert/strict';
|
|||
import {
|
||||
isDepthConfirmationAnswer,
|
||||
shouldBlockContextWrite,
|
||||
isDepthVerified,
|
||||
isQueuePhaseActive,
|
||||
setQueuePhaseActive,
|
||||
} from '../index.ts';
|
||||
import {
|
||||
markDepthVerified,
|
||||
isMilestoneDepthVerified,
|
||||
shouldBlockContextArtifactSave,
|
||||
clearDiscussionFlowState,
|
||||
resetWriteGateState,
|
||||
} from '../bootstrap/write-gate.ts';
|
||||
|
|
@ -53,26 +53,27 @@ test('write-gate: blocks CONTEXT.md write during discussion without depth verifi
|
|||
// ─── Scenario 3: Allows CONTEXT.md write after depth verification ──
|
||||
|
||||
test('write-gate: allows CONTEXT.md write after depth verification', () => {
|
||||
clearDiscussionFlowState();
|
||||
markDepthVerified('M001');
|
||||
const result = shouldBlockContextWrite(
|
||||
'write',
|
||||
'/Users/dev/project/.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
'M001',
|
||||
true,
|
||||
);
|
||||
assert.strictEqual(result.block, false, 'should not block after depth verification');
|
||||
assert.strictEqual(result.reason, undefined, 'should have no reason');
|
||||
clearDiscussionFlowState();
|
||||
});
|
||||
|
||||
// ─── Scenario 4: Allows CONTEXT.md write outside discussion phase (milestoneId null) ──
|
||||
// ─── Scenario 4: Ambiguous session context no longer bypasses the gate ──
|
||||
|
||||
test('write-gate: allows CONTEXT.md write outside discussion phase', () => {
|
||||
test('write-gate: blocks CONTEXT.md write when milestoneId is ambiguous', () => {
|
||||
const result = shouldBlockContextWrite(
|
||||
'write',
|
||||
'.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
null,
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(result.block, false, 'should not block outside discussion phase');
|
||||
assert.strictEqual(result.block, true, 'should block when milestone context is ambiguous');
|
||||
});
|
||||
|
||||
// ─── Scenario 5: Allows non-CONTEXT.md writes during discussion ──
|
||||
|
|
@ -83,7 +84,6 @@ test('write-gate: allows non-CONTEXT.md writes during discussion', () => {
|
|||
'write',
|
||||
'.gsd/milestones/M001/M001-DISCUSSION.md',
|
||||
'M001',
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(r1.block, false, 'DISCUSSION.md should pass');
|
||||
|
||||
|
|
@ -92,7 +92,6 @@ test('write-gate: allows non-CONTEXT.md writes during discussion', () => {
|
|||
'write',
|
||||
'.gsd/milestones/M001/slices/S01/S01-PLAN.md',
|
||||
'M001',
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(r2.block, false, 'slice plan should pass');
|
||||
|
||||
|
|
@ -101,7 +100,6 @@ test('write-gate: allows non-CONTEXT.md writes during discussion', () => {
|
|||
'write',
|
||||
'src/index.ts',
|
||||
'M001',
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(r3.block, false, 'regular code file should pass');
|
||||
});
|
||||
|
|
@ -113,7 +111,6 @@ test('write-gate: regex does not match slice context files (S01-CONTEXT.md)', ()
|
|||
'write',
|
||||
'.gsd/milestones/M001/slices/S01/S01-CONTEXT.md',
|
||||
'M001',
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(result.block, false, 'S01-CONTEXT.md should not be blocked');
|
||||
});
|
||||
|
|
@ -125,7 +122,6 @@ test('write-gate: blocked reason contains depth_verification keyword and anti-by
|
|||
'write',
|
||||
'.gsd/milestones/M999/M999-CONTEXT.md',
|
||||
'M999',
|
||||
false,
|
||||
);
|
||||
assert.strictEqual(result.block, true);
|
||||
assert.ok(result.reason!.includes('depth_verification'), 'reason should mention depth_verification question id');
|
||||
|
|
@ -141,7 +137,6 @@ test('write-gate: blocks CONTEXT.md write in queue mode without depth verificati
|
|||
'write',
|
||||
'.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
null, // no milestoneId in queue mode
|
||||
false, // not depth-verified
|
||||
true, // queue phase active
|
||||
);
|
||||
assert.strictEqual(result.block, true, 'should block in queue mode without depth verification');
|
||||
|
|
@ -151,46 +146,70 @@ test('write-gate: blocks CONTEXT.md write in queue mode without depth verificati
|
|||
// ─── Scenario 9: Queue mode allows CONTEXT.md write after depth verification ──
|
||||
|
||||
test('write-gate: allows CONTEXT.md write in queue mode after depth verification', () => {
|
||||
clearDiscussionFlowState();
|
||||
markDepthVerified('M001');
|
||||
const result = shouldBlockContextWrite(
|
||||
'write',
|
||||
'.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
null, // no milestoneId in queue mode
|
||||
true, // depth-verified
|
||||
true, // queue phase active
|
||||
);
|
||||
assert.strictEqual(result.block, false, 'should not block in queue mode after depth verification');
|
||||
clearDiscussionFlowState();
|
||||
});
|
||||
|
||||
// ─── Scenario 10: markDepthVerified works in queue-only mode (no milestoneId) ──
|
||||
// This is the core regression for #1812: in queue mode, the tool_result handler
|
||||
// must call markDepthVerified() even when getDiscussionMilestoneId() is null.
|
||||
// ─── Scenario 10: depth verification is scoped per milestone, not global ──
|
||||
|
||||
test('write-gate: markDepthVerified unblocks queue-mode writes when milestoneId is null', () => {
|
||||
test('write-gate: markDepthVerified unlocks only the matching milestone', () => {
|
||||
clearDiscussionFlowState();
|
||||
setQueuePhaseActive(true);
|
||||
markDepthVerified('M001');
|
||||
|
||||
// Before marking: should block
|
||||
const blocked = shouldBlockContextWrite(
|
||||
'write',
|
||||
'.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
null,
|
||||
isDepthVerified(),
|
||||
isQueuePhaseActive(),
|
||||
);
|
||||
assert.strictEqual(blocked.block, true, 'should block before markDepthVerified');
|
||||
|
||||
// Simulate what the fixed tool_result handler does
|
||||
markDepthVerified();
|
||||
|
||||
// After marking: should pass
|
||||
const allowed = shouldBlockContextWrite(
|
||||
'write',
|
||||
'.gsd/milestones/M001/M001-CONTEXT.md',
|
||||
null,
|
||||
isDepthVerified(),
|
||||
isQueuePhaseActive(),
|
||||
);
|
||||
assert.strictEqual(allowed.block, false, 'should allow after markDepthVerified in queue mode');
|
||||
assert.strictEqual(allowed.block, false, 'should allow the verified milestone');
|
||||
|
||||
const blockedOther = shouldBlockContextWrite(
|
||||
'write',
|
||||
'.gsd/milestones/M002/M002-CONTEXT.md',
|
||||
null,
|
||||
);
|
||||
assert.strictEqual(blockedOther.block, true, 'other milestones should remain blocked');
|
||||
assert.strictEqual(isMilestoneDepthVerified('M001'), true);
|
||||
assert.strictEqual(isMilestoneDepthVerified('M002'), false);
|
||||
|
||||
clearDiscussionFlowState();
|
||||
});
|
||||
|
||||
// ─── Scenario 11: gsd_summary_save CONTEXT contract is milestone-scoped ──
|
||||
|
||||
test('write-gate: gsd_summary_save only blocks final milestone CONTEXT writes', () => {
|
||||
clearDiscussionFlowState();
|
||||
|
||||
assert.strictEqual(
|
||||
shouldBlockContextArtifactSave('CONTEXT-DRAFT', 'M001').block,
|
||||
false,
|
||||
'draft CONTEXT should be allowed',
|
||||
);
|
||||
assert.strictEqual(
|
||||
shouldBlockContextArtifactSave('CONTEXT', 'M001', 'S01').block,
|
||||
false,
|
||||
'slice CONTEXT should be allowed',
|
||||
);
|
||||
assert.strictEqual(
|
||||
shouldBlockContextArtifactSave('CONTEXT', 'M001').block,
|
||||
true,
|
||||
'final milestone CONTEXT should block before verification',
|
||||
);
|
||||
|
||||
markDepthVerified('M001');
|
||||
assert.strictEqual(
|
||||
shouldBlockContextArtifactSave('CONTEXT', 'M001').block,
|
||||
false,
|
||||
'final milestone CONTEXT should pass after verification',
|
||||
);
|
||||
|
||||
clearDiscussionFlowState();
|
||||
});
|
||||
|
|
@ -281,15 +300,15 @@ test('write-gate: shouldBlockPendingGate allows read-only and ask_user_questions
|
|||
clearDiscussionFlowState();
|
||||
});
|
||||
|
||||
// ─── Scenario 23: shouldBlockPendingGate does not block outside discussion ──
|
||||
// ─── Scenario 23: shouldBlockPendingGate still blocks when the session is ambiguous ──
|
||||
|
||||
test('write-gate: shouldBlockPendingGate does not block outside discussion', () => {
|
||||
test('write-gate: shouldBlockPendingGate blocks outside discussion when a gate is pending', () => {
|
||||
clearDiscussionFlowState();
|
||||
setPendingGate('layer1_scope_gate');
|
||||
|
||||
// No milestoneId and no queue phase — not in discussion
|
||||
// No milestoneId and no queue phase — still block because the gate is pending
|
||||
const result = shouldBlockPendingGate('write', null, false);
|
||||
assert.strictEqual(result.block, false, 'should not block outside discussion');
|
||||
assert.strictEqual(result.block, true, 'should block even when milestoneId is null');
|
||||
|
||||
clearDiscussionFlowState();
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue