From 61485c5befaf5c07cea5766b0aa3ef45c3c7d331 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Sat, 2 May 2026 17:51:38 +0200 Subject: [PATCH] fix(sf): remove legacy completion tool aliases --- ...8-sf-tools-over-mcp-for-provider-parity.md | 4 +- docs/exec-plans/tech-debt-tracker.md | 4 +- docs/user-docs/configuration.md | 2 +- packages/mcp-server/README.md | 2 - .../src/coerce-string-arrays.test.ts | 6 +- .../mcp-server/src/workflow-tools.test.ts | 109 ++---------------- packages/mcp-server/src/workflow-tools.ts | 23 ---- .../extensions/sf/auto-completion-nudge.ts | 5 +- src/resources/extensions/sf/auto-prompts.ts | 2 +- src/resources/extensions/sf/auto-recovery.ts | 2 +- src/resources/extensions/sf/auto.ts | 1 - src/resources/extensions/sf/auto/session.ts | 2 +- .../extensions/sf/bootstrap/db-tools.ts | 15 +-- .../sf/docs/preferences-reference.md | 2 +- .../extensions/sf/preferences-types.ts | 2 +- .../sf/skills/finish-and-verify/SKILL.md | 2 +- .../sf/tests/auto-completion-nudge.test.ts | 10 +- .../tests/auto-mode-interactive-guard.test.ts | 2 +- .../sf/tests/block-db-writes.test.ts | 2 +- .../sf/tests/discuss-tool-scoping.test.ts | 2 - .../tests/integration/auto-recovery.test.ts | 2 +- .../sf/tests/prompt-contracts.test.ts | 30 ++--- .../tool-invocation-error-loop-break.test.ts | 6 +- .../extensions/sf/tests/tool-naming.test.ts | 25 ++-- .../tests/verify-artifact-tightened.test.ts | 2 +- .../sf/tests/write-intercept.test.ts | 4 +- .../extensions/sf/tools/complete-slice.ts | 2 +- .../extensions/sf/tools/complete-task.ts | 2 +- src/resources/extensions/sf/types.ts | 4 +- src/resources/extensions/sf/workflow-mcp.ts | 2 - .../extensions/sf/write-intercept.ts | 4 +- 31 files changed, 80 insertions(+), 202 deletions(-) diff --git a/docs/dev/ADR-008-sf-tools-over-mcp-for-provider-parity.md b/docs/dev/ADR-008-sf-tools-over-mcp-for-provider-parity.md index 7760b40b1..9d6af578b 100644 --- a/docs/dev/ADR-008-sf-tools-over-mcp-for-provider-parity.md +++ b/docs/dev/ADR-008-sf-tools-over-mcp-for-provider-parity.md @@ -22,7 +22,7 @@ The core SF workflow tools are internal extension tools. Examples include: - `sf_plan_milestone` - `sf_plan_slice` - `sf_plan_task` -- `sf_task_complete` / `sf_complete_task` +- `sf_task_complete` - `sf_slice_complete` - `sf_complete_milestone` - `sf_validate_milestone` @@ -44,7 +44,7 @@ The Claude Code CLI provider uses the Anthropic Agent SDK through `src/resources As a result: -- prompts tell the model to call tools like `sf_complete_task` +- prompts tell the model to call tools like `sf_task_complete` - the tools exist in SF - but Claude Code sessions do not actually receive those tools diff --git a/docs/exec-plans/tech-debt-tracker.md b/docs/exec-plans/tech-debt-tracker.md index 17066d077..251dc0561 100644 --- a/docs/exec-plans/tech-debt-tracker.md +++ b/docs/exec-plans/tech-debt-tracker.md @@ -14,11 +14,11 @@ ## MCP workflow mutations — read-only only -**Impact:** External providers (Claude Code CLI, remote orchestrators) that route through MCP can query SF state but cannot advance it. `sf_complete_task`, `sf_plan_milestone`, etc. are in-process only. +**Impact:** External providers (Claude Code CLI, remote orchestrators) that route through MCP can query SF state but cannot advance it. `sf_task_complete`, `sf_plan_milestone`, etc. are in-process only. **Proposed fix:** Extract shared handlers from native tools; expose over MCP server (ADR-008 Phase 1). -**Verification:** Claude Code provider session completes a task via MCP `sf_complete_task` and produces identical STATE.md outcome. +**Verification:** Claude Code provider session completes a task via MCP `sf_task_complete` and produces identical STATE.md outcome. **Tracked in:** [active/index.md — ADR-008 Phase 1](./active/index.md) · [ADR-008](../dev/ADR-008-IMPLEMENTATION-PLAN.md) diff --git a/docs/user-docs/configuration.md b/docs/user-docs/configuration.md index 68b0d95f7..b8c6572ee 100644 --- a/docs/user-docs/configuration.md +++ b/docs/user-docs/configuration.md @@ -294,7 +294,7 @@ auto_supervisor: soft_timeout_minutes: 20 # warn LLM to wrap up idle_timeout_minutes: 10 # detect stalls hard_timeout_minutes: 30 # pause auto mode - completion_nudge_after: 10 # complete-slice tool calls before nudging sf_complete_slice + completion_nudge_after: 10 # complete-slice tool calls before nudging sf_slice_complete ``` ### `budget_ceiling` diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index b304fb0c3..8273712f2 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -94,9 +94,7 @@ The workflow MCP surface includes: - `sf_replan_slice` - `sf_slice_replan` - `sf_task_complete` -- `sf_complete_task` - `sf_slice_complete` -- `sf_complete_slice` - `sf_skip_slice` - `sf_validate_milestone` - `sf_milestone_validate` diff --git a/packages/mcp-server/src/coerce-string-arrays.test.ts b/packages/mcp-server/src/coerce-string-arrays.test.ts index ab4b8b74d..ef356790e 100644 --- a/packages/mcp-server/src/coerce-string-arrays.test.ts +++ b/packages/mcp-server/src/coerce-string-arrays.test.ts @@ -41,7 +41,7 @@ function makeToolCall(overrides: Record) { return { type: "toolCall" as const, id: "call-1", - name: "sf_complete_task", + name: "sf_task_complete", arguments: { projectDir: "/tmp/sf-project", taskId: "T01", @@ -57,9 +57,9 @@ function makeToolCall(overrides: Record) { describe("string-array schema coercion", () => { const sfCompleteTaskTool = { - name: "sf_complete_task", + name: "sf_task_complete", description: "Record a completed task to the SF database and render its SUMMARY.md.", - parameters: workflowToolSchema("sf_complete_task") as any, + parameters: workflowToolSchema("sf_task_complete") as any, }; it("coerces a bare string keyDecisions value before validation", () => { diff --git a/packages/mcp-server/src/workflow-tools.test.ts b/packages/mcp-server/src/workflow-tools.test.ts index 66f235593..be6b39d94 100644 --- a/packages/mcp-server/src/workflow-tools.test.ts +++ b/packages/mcp-server/src/workflow-tools.test.ts @@ -92,6 +92,8 @@ describe("workflow MCP tools", () => { assert.equal(server.tools.length, WORKFLOW_TOOL_NAMES.length); assert.deepEqual(server.tools.map((t) => t.name), [...WORKFLOW_TOOL_NAMES]); + assert.ok(!server.tools.some((t) => t.name === "sf_complete_task")); + assert.ok(!server.tools.some((t) => t.name === "sf_complete_slice")); }); it("sf_summary_save writes artifact through the shared executor", async () => { @@ -305,40 +307,6 @@ describe("workflow MCP tools", () => { } }); - it("sf_complete_task alias delegates to sf_task_complete behavior", async () => { - const base = makeTmpBase(); - try { - mkdirSync(join(base, ".sf", "milestones", "M002", "slices", "S02"), { recursive: true }); - writeFileSync( - join(base, ".sf", "milestones", "M002", "slices", "S02", "S02-PLAN.md"), - "# S02\n\n- [ ] **T02: Demo** `est:5m`\n", - ); - - const server = makeMockServer(); - registerWorkflowTools(server as any); - const aliasTool = server.tools.find((t) => t.name === "sf_complete_task"); - assert.ok(aliasTool, "task completion alias should be registered"); - - const result = await aliasTool!.handler({ - projectDir: base, - taskId: "T02", - sliceId: "S02", - milestoneId: "M002", - oneLiner: "Completed task via alias", - narrative: "Did the work through alias", - verification: "npm test", - }); - - assert.match((result as any).content[0].text as string, /Completed task T02/); - assert.ok( - existsSync(join(base, ".sf", "milestones", "M002", "slices", "S02", "tasks", "T02-SUMMARY.md")), - "alias should write task summary to disk", - ); - } finally { - cleanup(base); - } - }); - it("sf_plan_milestone and sf_plan_slice work end-to-end", async () => { const base = makeTmpBase(); try { @@ -662,7 +630,7 @@ describe("workflow MCP tools", () => { } }); - it("sf_slice_complete and sf_complete_slice work end-to-end", async () => { + it("sf_slice_complete works end-to-end", async () => { const base = makeTmpBase(); try { const server = makeMockServer(); @@ -671,12 +639,10 @@ describe("workflow MCP tools", () => { const sliceTool = server.tools.find((t) => t.name === "sf_plan_slice"); const taskTool = server.tools.find((t) => t.name === "sf_task_complete"); const canonicalTool = server.tools.find((t) => t.name === "sf_slice_complete"); - const aliasTool = server.tools.find((t) => t.name === "sf_complete_slice"); assert.ok(milestoneTool, "milestone planning tool should be registered"); assert.ok(sliceTool, "slice planning tool should be registered"); assert.ok(taskTool, "task completion tool should be registered"); assert.ok(canonicalTool, "slice completion tool should be registered"); - assert.ok(aliasTool, "slice completion alias should be registered"); await milestoneTool!.handler({ projectDir: base, @@ -738,74 +704,13 @@ describe("workflow MCP tools", () => { uatContent: "## UAT\n\nPASS", }); assert.match((canonicalResult as any).content[0].text as string, /Completed slice S03/); - - await milestoneTool!.handler({ - projectDir: base, - milestoneId: "M004", - title: "Alias milestone", - vision: "Prepare alias slice completion state.", - slices: [ - { - sliceId: "S04", - title: "Alias Slice", - risk: "medium", - depends: [], - demo: "Alias slice completes through MCP.", - goal: "Seed alias workflow state.", - successCriteria: "Alias summary and UAT files are written.", - proofLevel: "integration", - integrationClosure: "Alias reaches the shared slice executor.", - observabilityImpact: "Workflow tests cover alias completion.", - }, - ], - }); - await sliceTool!.handler({ - projectDir: base, - milestoneId: "M004", - sliceId: "S04", - goal: "Complete alias slice over MCP.", - planningMeeting: validPlanningMeeting(), - tasks: [ - { - taskId: "T04", - title: "Alias task", - description: "Seed a completed task for alias slice completion.", - estimate: "5m", - files: ["packages/mcp-server/src/workflow-tools.ts"], - verify: "node --test", - inputs: ["M004-ROADMAP.md"], - expectedOutput: ["S04-SUMMARY.md", "S04-UAT.md"], - }, - ], - }); - await taskTool!.handler({ - projectDir: base, - milestoneId: "M004", - sliceId: "S04", - taskId: "T04", - oneLiner: "Completed alias task", - narrative: "Prepared the alias slice for completion.", - verification: "node --test", - }); - - const aliasResult = await aliasTool!.handler({ - projectDir: base, - milestoneId: "M004", - sliceId: "S04", - sliceTitle: "Alias Slice", - oneLiner: "Completed alias slice", - narrative: "Did the slice work via alias", - verification: "npm test", - uatContent: "## UAT\n\nPASS", - }); - assert.match((aliasResult as any).content[0].text as string, /Completed slice S04/); assert.ok( - existsSync(join(base, ".sf", "milestones", "M004", "slices", "S04", "S04-SUMMARY.md")), - "alias should write slice summary to disk", + existsSync(join(base, ".sf", "milestones", "M003", "slices", "S03", "S03-SUMMARY.md")), + "canonical tool should write slice summary to disk", ); assert.ok( - existsSync(join(base, ".sf", "milestones", "M004", "slices", "S04", "S04-UAT.md")), - "alias should write slice UAT to disk", + existsSync(join(base, ".sf", "milestones", "M003", "slices", "S03", "S03-UAT.md")), + "canonical tool should write slice UAT to disk", ); } finally { cleanup(base); diff --git a/packages/mcp-server/src/workflow-tools.ts b/packages/mcp-server/src/workflow-tools.ts index 14843f135..a44d9535d 100644 --- a/packages/mcp-server/src/workflow-tools.ts +++ b/packages/mcp-server/src/workflow-tools.ts @@ -501,7 +501,6 @@ export const WORKFLOW_TOOL_NAMES = [ "sf_replan_slice", "sf_slice_replan", "sf_slice_complete", - "sf_complete_slice", "sf_skip_slice", "sf_complete_milestone", "sf_milestone_complete", @@ -512,7 +511,6 @@ export const WORKFLOW_TOOL_NAMES = [ "sf_save_gate_result", "sf_summary_save", "sf_task_complete", - "sf_complete_task", "sf_milestone_status", "sf_journal_query", ] as const; @@ -1313,16 +1311,6 @@ export function registerWorkflowTools(server: McpToolServer): void { }, ); - server.tool( - "sf_complete_slice", - "Alias for sf_slice_complete. Record a completed slice to the SF database and render summary/UAT artifacts.", - sliceCompleteParams, - async (args: Record) => { - const parsed = parseWorkflowArgs(sliceCompleteSchema, args); - return handleSliceComplete(parsed.projectDir, parsed); - }, - ); - server.tool( "sf_skip_slice", "Mark a slice as skipped so auto-mode advances past it without executing.", @@ -1455,17 +1443,6 @@ export function registerWorkflowTools(server: McpToolServer): void { }, ); - server.tool( - "sf_complete_task", - "Alias for sf_task_complete. Record a completed task to the SF database and render its SUMMARY.md.", - taskCompleteParams, - async (args: Record) => { - const parsed = parseWorkflowArgs(taskCompleteSchema, args); - const { projectDir, ...taskArgs } = parsed; - return handleTaskComplete(projectDir, taskArgs); - }, - ); - server.tool( "sf_milestone_status", "Read the current status of a milestone and all its slices from the SF database.", diff --git a/src/resources/extensions/sf/auto-completion-nudge.ts b/src/resources/extensions/sf/auto-completion-nudge.ts index 911391eb5..865eb297b 100644 --- a/src/resources/extensions/sf/auto-completion-nudge.ts +++ b/src/resources/extensions/sf/auto-completion-nudge.ts @@ -2,7 +2,6 @@ import type { AgentMessage } from "@singularity-forge/pi-agent-core"; export const DEFAULT_COMPLETION_NUDGE_AFTER = 10; export const COMPLETION_NUDGE_TOOL_NAMES = new Set([ - "sf_complete_slice", "sf_slice_complete", ]); @@ -117,12 +116,12 @@ function nextCompletionNudgeMessage(): string | null { state.reminderSent = true; state.strongSent = true; state.lowerTemperatureForNextRequest = true; - return `You've performed ${state.toolCalls} tool calls without calling sf_complete_slice. Stop further investigation unless there is a specific blocker. Call sf_complete_slice now with your summary.`; + return `You've performed ${state.toolCalls} tool calls without calling sf_slice_complete. Stop further investigation unless there is a specific blocker. Call sf_slice_complete now with your summary.`; } if (!state.reminderSent && state.toolCalls >= firstThreshold) { state.reminderSent = true; - return `You've performed ${state.toolCalls} tool calls of investigation. Per the slice plan you should now call sf_complete_slice with your summary. If you genuinely need more context, say so explicitly; otherwise call the tool now.`; + return `You've performed ${state.toolCalls} tool calls of investigation. Per the slice plan you should now call sf_slice_complete with your summary. If you genuinely need more context, say so explicitly; otherwise call the tool now.`; } return null; diff --git a/src/resources/extensions/sf/auto-prompts.ts b/src/resources/extensions/sf/auto-prompts.ts index d2d457e14..16dc2f1c8 100644 --- a/src/resources/extensions/sf/auto-prompts.ts +++ b/src/resources/extensions/sf/auto-prompts.ts @@ -2592,7 +2592,7 @@ export async function buildCompleteSlicePrompt( // Gates owned by complete-slice (e.g. Q8). Pull from the DB so the // prompt only prompts for gates the plan actually seeded. The tool // handler closes each gate based on the SUMMARY.md section content - // after the assistant calls sf_complete_slice. + // after the assistant calls sf_slice_complete. const csPending = getPendingGatesForTurn(mid, sid, "complete-slice"); // coverage check: every pending row must be owned by complete-slice. // requireAll:false because a slice may have already closed some gates. diff --git a/src/resources/extensions/sf/auto-recovery.ts b/src/resources/extensions/sf/auto-recovery.ts index eb570dda7..ff48fcffd 100644 --- a/src/resources/extensions/sf/auto-recovery.ts +++ b/src/resources/extensions/sf/auto-recovery.ts @@ -400,7 +400,7 @@ export function verifyExpectedArtifact( } else if (!isDbAvailable()) { // LEGACY: Pre-migration fallback for projects without DB. // Require a CHECKED checkbox — a bare heading or unchecked checkbox - // does not prove sf_complete_task ran. Summary file on disk alone + // does not prove sf_task_complete ran. Summary file on disk alone // is not sufficient evidence (could be a rogue write) (#3607). const planAbs = resolveSliceFile(base, mid, sid, "PLAN"); if (planAbs && existsSync(planAbs)) { diff --git a/src/resources/extensions/sf/auto.ts b/src/resources/extensions/sf/auto.ts index 9935570f9..ac91ee97f 100644 --- a/src/resources/extensions/sf/auto.ts +++ b/src/resources/extensions/sf/auto.ts @@ -526,7 +526,6 @@ export function markToolEnd(toolCallId: string): void { const TASK_COMPLETE_TOOL_NAMES = new Set([ "sf_task_complete", - "sf_complete_task", ]); function normalizeTaskCompleteFailure(errorMsg: string): string { diff --git a/src/resources/extensions/sf/auto/session.ts b/src/resources/extensions/sf/auto/session.ts index 36e110713..b2a26e516 100644 --- a/src/resources/extensions/sf/auto/session.ts +++ b/src/resources/extensions/sf/auto/session.ts @@ -167,7 +167,7 @@ export class AutoSession { /** Last turn-level git action status captured during finalize. */ lastGitActionStatus: "ok" | "failed" | null = null; /** - * Last sf_task_complete/sf_complete_task execution error for the current turn. + * Last sf_task_complete execution error for the current turn. * Unlike malformed tool invocation errors, these are normal tool execution * failures (for example a transient SUMMARY.md write failure) and should be * retried in-flow instead of pausing auto-mode. diff --git a/src/resources/extensions/sf/bootstrap/db-tools.ts b/src/resources/extensions/sf/bootstrap/db-tools.ts index d3e9132f1..1a442fe70 100644 --- a/src/resources/extensions/sf/bootstrap/db-tools.ts +++ b/src/resources/extensions/sf/bootstrap/db-tools.ts @@ -1344,7 +1344,7 @@ export function registerDbTools(pi: ExtensionAPI): void { pi.registerTool(planTaskTool); registerAlias(pi, planTaskTool, "sf_task_plan", "sf_plan_task"); - // ─── sf_task_complete (sf_complete_task alias) ──────────────────────── + // ─── sf_task_complete ───────────────────────────────────────────────── const taskCompleteExecute = async ( _toolCallId: string, @@ -1365,7 +1365,7 @@ export function registerDbTools(pi: ExtensionAPI): void { promptSnippet: "Complete a SF task (DB write + summary render + checkbox toggle)", promptGuidelines: [ - "Use sf_task_complete (or sf_complete_task) when a task is finished and needs to be recorded.", + "Use sf_task_complete when a task is finished and needs to be recorded.", "All string fields are required. verificationEvidence is an array of objects with command, exitCode, verdict, durationMs.", "The tool validates required fields and returns an error message if any are missing.", "On success, returns the summaryPath where the SUMMARY.md was written.", @@ -1441,9 +1441,8 @@ export function registerDbTools(pi: ExtensionAPI): void { }; pi.registerTool(taskCompleteTool); - registerAlias(pi, taskCompleteTool, "sf_complete_task", "sf_task_complete"); - // ─── sf_slice_complete (sf_complete_slice alias) ───────────────────── + // ─── sf_slice_complete ──────────────────────────────────────────────── const sliceCompleteExecute = async ( _toolCallId: string, @@ -1464,7 +1463,7 @@ export function registerDbTools(pi: ExtensionAPI): void { promptSnippet: "Complete a SF slice (DB write + summary/UAT render + roadmap checkbox toggle)", promptGuidelines: [ - "Use sf_slice_complete (or sf_complete_slice) when all tasks in a slice are finished and the slice needs to be recorded.", + "Use sf_slice_complete when all tasks in a slice are finished and the slice needs to be recorded.", "All tasks in the slice must have status 'complete' — the handler validates this before proceeding.", "On success, returns summaryPath and uatPath where the files were written.", "Idempotent — calling with the same params twice will not crash.", @@ -1607,12 +1606,6 @@ export function registerDbTools(pi: ExtensionAPI): void { }; pi.registerTool(sliceCompleteTool); - registerAlias( - pi, - sliceCompleteTool, - "sf_complete_slice", - "sf_slice_complete", - ); // ─── sf_skip_slice (#3477 / #3487) ─────────────────────────────────── diff --git a/src/resources/extensions/sf/docs/preferences-reference.md b/src/resources/extensions/sf/docs/preferences-reference.md index 4df1b2118..3f6264a04 100644 --- a/src/resources/extensions/sf/docs/preferences-reference.md +++ b/src/resources/extensions/sf/docs/preferences-reference.md @@ -125,7 +125,7 @@ Setting `prefer_skills: []` does **not** disable skill discovery — it just mea - `soft_timeout_minutes`: minutes before the supervisor issues a soft warning (default: 20). - `idle_timeout_minutes`: minutes of inactivity before the supervisor intervenes (default: 10). - `hard_timeout_minutes`: minutes before the supervisor forces termination (default: 30). - - `completion_nudge_after`: tool calls in a complete-slice unit before nudging the agent to call `sf_complete_slice` (default: 10; set `0` to disable). + - `completion_nudge_after`: tool calls in a complete-slice unit before nudging the agent to call `sf_slice_complete` (default: 10; set `0` to disable). - `runaway_guard_enabled`: enable active-loop diagnosis for long-running units (default: `true`). - `runaway_tool_call_warning`: unit tool calls before a runaway warning (default: `60`; set `0` to disable this signal). - `runaway_token_warning`: unit tokens before a runaway warning (default: `1000000`; set `0` to disable this signal). diff --git a/src/resources/extensions/sf/preferences-types.ts b/src/resources/extensions/sf/preferences-types.ts index 34416eaea..2d41b56be 100644 --- a/src/resources/extensions/sf/preferences-types.ts +++ b/src/resources/extensions/sf/preferences-types.ts @@ -248,7 +248,7 @@ export interface AutoSupervisorConfig { idle_timeout_minutes?: number; hard_timeout_minutes?: number; phase_timeout_minutes?: number; - /** Tool-call count before nudging complete-slice agents to call sf_complete_slice. Default: 10. Set 0 to disable. */ + /** Tool-call count before nudging complete-slice agents to call sf_slice_complete. Default: 10. Set 0 to disable. */ completion_nudge_after?: number; /** Enable the runaway guard that asks long-running active units for diagnosis before pausing. Default: true. */ runaway_guard_enabled?: boolean; diff --git a/src/resources/extensions/sf/skills/finish-and-verify/SKILL.md b/src/resources/extensions/sf/skills/finish-and-verify/SKILL.md index 270819d78..c9088b0c1 100644 --- a/src/resources/extensions/sf/skills/finish-and-verify/SKILL.md +++ b/src/resources/extensions/sf/skills/finish-and-verify/SKILL.md @@ -22,7 +22,7 @@ This skill does NOT decide whether a slice is shippable in isolation — that's - After `spec-first-tdd` reaches GREEN on the contract test. - After a debugging fix from `systematic-debugging`. -- Before calling `sf_complete_task` or `sf_complete_slice`. +- Before calling `sf_task_complete` or `sf_slice_complete`. - Inside an autonomous iteration loop, between slices. If used inside an autonomous iteration loop and the user goal is still in progress: diff --git a/src/resources/extensions/sf/tests/auto-completion-nudge.test.ts b/src/resources/extensions/sf/tests/auto-completion-nudge.test.ts index 7dbd6c03a..4b6f4d8aa 100644 --- a/src/resources/extensions/sf/tests/auto-completion-nudge.test.ts +++ b/src/resources/extensions/sf/tests/auto-completion-nudge.test.ts @@ -24,7 +24,7 @@ function baseMessages(): AgentMessage[] { ]; } -test("completion nudge injects reminder after N tool calls without sf_complete_slice", () => { +test("completion nudge injects reminder after N tool calls without sf_slice_complete", () => { resetCompletionNudgeState("complete-slice", "M001/S01", 3); recordCompletionNudgeToolCall("read"); recordCompletionNudgeToolCall("grep"); @@ -40,14 +40,14 @@ test("completion nudge injects reminder after N tool calls without sf_complete_s String(nudge.content), /You've performed 3 tool calls of investigation/, ); - assert.match(String(nudge.content), /call sf_complete_slice/); + assert.match(String(nudge.content), /call sf_slice_complete/); }); -test("completion nudge does not inject after sf_complete_slice is called", () => { +test("completion nudge does not inject after sf_slice_complete is called", () => { resetCompletionNudgeState("complete-slice", "M001/S01", 3); recordCompletionNudgeToolCall("read"); recordCompletionNudgeToolCall("grep"); - recordCompletionNudgeToolCall("sf_complete_slice"); + recordCompletionNudgeToolCall("sf_slice_complete"); recordCompletionNudgeToolCall("bash"); const messages = maybeInjectCompletionNudgeMessage(baseMessages()); @@ -66,7 +66,7 @@ test("completion nudge injects stronger message at 2N and lowers temperature", ( const messages = maybeInjectCompletionNudgeMessage(baseMessages()); assert.equal(messages.length, 2); const nudge = messages[1] as CustomTestMessage; - assert.match(String(nudge.content), /without calling sf_complete_slice/); + assert.match(String(nudge.content), /without calling sf_slice_complete/); const payload = { temperature: 0.9, generationConfig: { temperature: 0.8 } }; applyCompletionNudgeTemperature(payload); diff --git a/src/resources/extensions/sf/tests/auto-mode-interactive-guard.test.ts b/src/resources/extensions/sf/tests/auto-mode-interactive-guard.test.ts index c193c3879..da393b2ef 100644 --- a/src/resources/extensions/sf/tests/auto-mode-interactive-guard.test.ts +++ b/src/resources/extensions/sf/tests/auto-mode-interactive-guard.test.ts @@ -3,7 +3,7 @@ * * Bug #2936: When the LLM calls ask_user_questions during auto-mode units * (plan-slice, execute-task, complete-slice), the interactive tool queues a - * user response which causes the subsequent sf_plan_slice / sf_complete_task + * user response which causes the subsequent sf_plan_slice / sf_task_complete * call to fail with "Skipped due to queued user message." The canonical SF * tool call is never recorded, verifyExpectedArtifact finds no artifact, and * the dispatch loop re-dispatches the same unit 2-4x. diff --git a/src/resources/extensions/sf/tests/block-db-writes.test.ts b/src/resources/extensions/sf/tests/block-db-writes.test.ts index 40df09141..a42a97904 100644 --- a/src/resources/extensions/sf/tests/block-db-writes.test.ts +++ b/src/resources/extensions/sf/tests/block-db-writes.test.ts @@ -1,7 +1,7 @@ /** * Regression test for #3674 — block direct writes to sf.db * - * When sf_complete_task was unavailable, agents fell back to shell-based + * When sf_task_complete was unavailable, agents fell back to shell-based * sqlite3 writes, corrupting the WAL-backed database. The fix extends * write-intercept to block file writes and bash commands targeting sf.db. */ diff --git a/src/resources/extensions/sf/tests/discuss-tool-scoping.test.ts b/src/resources/extensions/sf/tests/discuss-tool-scoping.test.ts index 2ce31b246..f598320b9 100644 --- a/src/resources/extensions/sf/tests/discuss-tool-scoping.test.ts +++ b/src/resources/extensions/sf/tests/discuss-tool-scoping.test.ts @@ -39,9 +39,7 @@ const HEAVY_TOOLS = [ "sf_plan_task", "sf_task_plan", "sf_task_complete", - "sf_complete_task", "sf_slice_complete", - "sf_complete_slice", "sf_complete_milestone", "sf_milestone_complete", "sf_validate_milestone", diff --git a/src/resources/extensions/sf/tests/integration/auto-recovery.test.ts b/src/resources/extensions/sf/tests/integration/auto-recovery.test.ts index 82cf5927d..e361d72c6 100644 --- a/src/resources/extensions/sf/tests/integration/auto-recovery.test.ts +++ b/src/resources/extensions/sf/tests/integration/auto-recovery.test.ts @@ -702,7 +702,7 @@ test("verifyExpectedArtifact execute-task rejects heading-style plan without che ); writeFileSync(join(tasksDir, "T01-SUMMARY.md"), "# T01 Summary\n\nDone."); // Heading-style entries no longer count as verified — only checked - // checkboxes prove sf_complete_task ran (#3607). + // checkboxes prove sf_task_complete ran (#3607). assert.strictEqual( verifyExpectedArtifact("execute-task", "M001/S01/T01", base), false, diff --git a/src/resources/extensions/sf/tests/prompt-contracts.test.ts b/src/resources/extensions/sf/tests/prompt-contracts.test.ts index 831ce818a..8e3a6dba7 100644 --- a/src/resources/extensions/sf/tests/prompt-contracts.test.ts +++ b/src/resources/extensions/sf/tests/prompt-contracts.test.ts @@ -301,17 +301,17 @@ test("research prompts stop after saving and forbid planning tools", () => { } }); -// ─── Prompt migration: execute-task → sf_complete_task ─────────────── +// ─── Prompt migration: execute-task → sf_task_complete ─────────────── -test("execute-task prompt references sf_complete_task tool", () => { +test("execute-task prompt references sf_task_complete tool", () => { const prompt = readPrompt("execute-task"); - assert.match(prompt, /sf_complete_task/); + assert.match(prompt, /sf_task_complete/); }); -test("execute-task prompt uses sf_complete_task as canonical summary write path", () => { +test("execute-task prompt uses sf_task_complete as canonical summary write path", () => { const prompt = readPrompt("execute-task"); assert.match(prompt, /\{\{taskSummaryPath\}\}/); - assert.match(prompt, /sf_complete_task/); + assert.match(prompt, /sf_task_complete/); assert.match(prompt, /DB-backed tool is the canonical write path/i); assert.match( prompt, @@ -348,11 +348,11 @@ test("guided-execute-task prompt does not instruct manual file write", () => { ); }); -// ─── Prompt migration: complete-slice → sf_complete_slice ──────────── +// ─── Prompt migration: complete-slice → sf_slice_complete ──────────── -test("complete-slice prompt references sf_complete_slice tool", () => { +test("complete-slice prompt references sf_slice_complete tool", () => { const prompt = readPrompt("complete-slice"); - assert.match(prompt, /sf_complete_slice/); + assert.match(prompt, /sf_slice_complete/); }); test("complete-slice prompt does not instruct LLM to toggle checkboxes manually", () => { @@ -369,7 +369,7 @@ test("complete-slice prompt instructs writing summary and UAT files before tool const prompt = readPrompt("complete-slice"); assert.match(prompt, /\{\{sliceSummaryPath\}\}/); assert.match(prompt, /\{\{sliceUatPath\}\}/); - assert.match(prompt, /sf_complete_slice/); + assert.match(prompt, /sf_slice_complete/); assert.match(prompt, /DB-backed tool is the canonical write path/i); assert.match( prompt, @@ -529,14 +529,14 @@ test("reassess-roadmap prompt names sf_reassess_roadmap as the tool to use", () test("execute-task prompt uses camelCase parameter names matching TypeBox schema", () => { const prompt = readPrompt("execute-task"); - // The sf_complete_task tool schema uses camelCase: milestoneId, sliceId, taskId + // The sf_task_complete tool schema uses camelCase: milestoneId, sliceId, taskId // Prompts must NOT tell the LLM to use snake_case (milestone_id, slice_id, task_id) const toolCallLine = prompt .split("\n") - .find((l) => /sf_complete_task/.test(l) || /sf_task_complete/.test(l)); + .find((l) => /sf_task_complete/.test(l)); assert.ok( toolCallLine, - "prompt must contain a sf_complete_task or sf_task_complete tool call line", + "prompt must contain a sf_task_complete tool call line", ); assert.doesNotMatch( toolCallLine!, @@ -557,13 +557,13 @@ test("execute-task prompt uses camelCase parameter names matching TypeBox schema test("complete-slice prompt uses camelCase parameter names matching TypeBox schema", () => { const prompt = readPrompt("complete-slice"); - // The sf_complete_slice tool schema uses camelCase: milestoneId, sliceId + // The sf_slice_complete tool schema uses camelCase: milestoneId, sliceId const toolCallLine = prompt .split("\n") - .find((l) => /sf_complete_slice/.test(l) || /sf_slice_complete/.test(l)); + .find((l) => /sf_slice_complete/.test(l)); assert.ok( toolCallLine, - "prompt must contain a sf_complete_slice or sf_slice_complete tool call line", + "prompt must contain a sf_slice_complete tool call line", ); assert.doesNotMatch( toolCallLine!, diff --git a/src/resources/extensions/sf/tests/tool-invocation-error-loop-break.test.ts b/src/resources/extensions/sf/tests/tool-invocation-error-loop-break.test.ts index f57d6f0fe..319888717 100644 --- a/src/resources/extensions/sf/tests/tool-invocation-error-loop-break.test.ts +++ b/src/resources/extensions/sf/tests/tool-invocation-error-loop-break.test.ts @@ -1,5 +1,5 @@ /** - * Regression tests for #2883: sf_complete_slice tool invocation fails with + * Regression tests for #2883: sf_slice_complete tool invocation fails with * JSON truncation, causing stuck retry loop. * * When a SF tool is invoked with malformed/truncated JSON arguments, the tool @@ -29,7 +29,7 @@ describe("#2883: tool invocation error tracking on AutoSession", () => { test("lastToolInvocationError is cleared on reset()", () => { const s = new AutoSession(); - s.lastToolInvocationError = "Validation failed for tool sf_complete_slice"; + s.lastToolInvocationError = "Validation failed for tool sf_slice_complete"; assert.ok(s.lastToolInvocationError); s.reset(); assert.equal(s.lastToolInvocationError, null); @@ -54,7 +54,7 @@ describe("#2883: isToolInvocationError classification", () => { test("detects JSON validation failure pattern", () => { assert.equal( isToolInvocationError( - "Validation failed for tool sf_complete_slice: Expected ',' or '}' in JSON", + "Validation failed for tool sf_slice_complete: Expected ',' or '}' in JSON", ), true, ); diff --git a/src/resources/extensions/sf/tests/tool-naming.test.ts b/src/resources/extensions/sf/tests/tool-naming.test.ts index e85d38393..35b804394 100644 --- a/src/resources/extensions/sf/tests/tool-naming.test.ts +++ b/src/resources/extensions/sf/tests/tool-naming.test.ts @@ -1,8 +1,7 @@ -// tool-naming — Verifies canonical + alias tool registration for SF DB tools. +// tool-naming — Verifies canonical + supported alias tool registration for SF DB tools. // -// Each DB tool must register under its canonical sf_concept_action name -// AND under a backward-compatible alias name. -// The alias must share the exact same execute function reference as the canonical tool. +// DB tools with supported aliases must register under both names. +// Completion tools are canonical-only; do not reintroduce legacy aliases. import assert from "node:assert/strict"; import { registerDbTools } from "../bootstrap/db-tools.ts"; @@ -28,8 +27,6 @@ const RENAME_MAP: Array<{ canonical: string; alias: string }> = [ { canonical: "sf_requirement_save", alias: "sf_save_requirement" }, { canonical: "sf_summary_save", alias: "sf_save_summary" }, { canonical: "sf_milestone_generate_id", alias: "sf_generate_milestone_id" }, - { canonical: "sf_task_complete", alias: "sf_complete_task" }, - { canonical: "sf_slice_complete", alias: "sf_complete_slice" }, { canonical: "sf_plan_milestone", alias: "sf_milestone_plan" }, { canonical: "sf_plan_slice", alias: "sf_slice_plan" }, { canonical: "sf_plan_task", alias: "sf_task_plan" }, @@ -43,6 +40,13 @@ const EXTRA_DB_TOOLS = [ "sf_self_report", "sf_skip_slice", "sf_save_gate_result", + "sf_task_complete", + "sf_slice_complete", +] as const; + +const REMOVED_COMPLETION_ALIASES = [ + "sf_complete_task", + "sf_complete_slice", ] as const; // ─── Registration count ────────────────────────────────────────────────────── @@ -83,6 +87,13 @@ for (const name of EXTRA_DB_TOOLS) { ); } +for (const name of REMOVED_COMPLETION_ALIASES) { + assert.ok( + !pi.tools.some((t: any) => t.name === name), + `Removed completion alias "${name}" should not be registered`, + ); +} + // ─── Execute function identity ─────────────────────────────────────────────── console.log("\n── Tool naming: execute function identity (===) ──"); @@ -196,4 +207,4 @@ console.log("\n── Tool naming: milestone planning renderer summarizes work // ═══════════════════════════════════════════════════════════════════════════ -}); \ No newline at end of file +}); diff --git a/src/resources/extensions/sf/tests/verify-artifact-tightened.test.ts b/src/resources/extensions/sf/tests/verify-artifact-tightened.test.ts index 72322c237..c83598e0f 100644 --- a/src/resources/extensions/sf/tests/verify-artifact-tightened.test.ts +++ b/src/resources/extensions/sf/tests/verify-artifact-tightened.test.ts @@ -3,7 +3,7 @@ * * The legacy (pre-migration) fallback in verifyExpectedArtifact previously * accepted either a heading match (### T01 --) or a checked checkbox as proof - * that sf_complete_task ran. A heading alone does not prove completion — + * that sf_task_complete ran. A heading alone does not prove completion — * it could result from a rogue write. * * The fix removes the hdRe heading regex and requires only a checked checkbox diff --git a/src/resources/extensions/sf/tests/write-intercept.test.ts b/src/resources/extensions/sf/tests/write-intercept.test.ts index bce770445..4c5f4f191 100644 --- a/src/resources/extensions/sf/tests/write-intercept.test.ts +++ b/src/resources/extensions/sf/tests/write-intercept.test.ts @@ -81,8 +81,8 @@ test("write-intercept: BLOCKED_WRITE_ERROR is a non-empty string", () => { test("write-intercept: BLOCKED_WRITE_ERROR mentions engine tool calls", () => { assert.ok( - BLOCKED_WRITE_ERROR.includes("sf_complete_task"), - "should mention sf_complete_task", + BLOCKED_WRITE_ERROR.includes("sf_task_complete"), + "should mention sf_task_complete", ); assert.ok( BLOCKED_WRITE_ERROR.includes("engine tool calls"), diff --git a/src/resources/extensions/sf/tools/complete-slice.ts b/src/resources/extensions/sf/tools/complete-slice.ts index 4760a2fb2..26e38066e 100644 --- a/src/resources/extensions/sf/tools/complete-slice.ts +++ b/src/resources/extensions/sf/tools/complete-slice.ts @@ -587,7 +587,7 @@ export async function handleCompleteSlice( ); invalidateStateCache(); return { - error: `database update failed after SUMMARY.md/UAT.md write succeeded at ${summaryPath}: ${msg}. Files were kept; retry sf_complete_slice after fixing the DB.`, + error: `database update failed after SUMMARY.md/UAT.md write succeeded at ${summaryPath}: ${msg}. Files were kept; retry sf_slice_complete after fixing the DB.`, }; } diff --git a/src/resources/extensions/sf/tools/complete-task.ts b/src/resources/extensions/sf/tools/complete-task.ts index d096cfbc5..be2042eb9 100644 --- a/src/resources/extensions/sf/tools/complete-task.ts +++ b/src/resources/extensions/sf/tools/complete-task.ts @@ -1,5 +1,5 @@ /** - * complete-task handler — the core operation behind sf_complete_task. + * complete-task handler — the core operation behind sf_task_complete. * * Validates inputs, atomically renders SUMMARY.md to disk, then writes the * task row to DB in a transaction, toggles the plan checkbox, and invalidates diff --git a/src/resources/extensions/sf/types.ts b/src/resources/extensions/sf/types.ts index 83fc23733..41c72f812 100644 --- a/src/resources/extensions/sf/types.ts +++ b/src/resources/extensions/sf/types.ts @@ -537,7 +537,7 @@ export interface BrowserFlowResult { duration: number; } -// ─── Complete Task Params (sf_complete_task tool input) ───────────────── +// ─── Complete Task Params (sf_task_complete tool input) ───────────────── export interface CompleteTaskParams { taskId: string; @@ -587,7 +587,7 @@ export interface CompleteTaskParams { triggerReason?: string; } -// ─── Complete Slice Params (sf_complete_slice tool input) ─────────────── +// ─── Complete Slice Params (sf_slice_complete tool input) ─────────────── export interface CompleteSliceParams { sliceId: string; diff --git a/src/resources/extensions/sf/workflow-mcp.ts b/src/resources/extensions/sf/workflow-mcp.ts index 7cd97dc83..e95fcfa2c 100644 --- a/src/resources/extensions/sf/workflow-mcp.ts +++ b/src/resources/extensions/sf/workflow-mcp.ts @@ -24,8 +24,6 @@ const MCP_WORKFLOW_TOOL_SURFACE = new Set([ "ask_user_questions", "sf_decision_save", "sf_complete_milestone", - "sf_complete_task", - "sf_complete_slice", "sf_generate_milestone_id", "sf_journal_query", "sf_milestone_complete", diff --git a/src/resources/extensions/sf/write-intercept.ts b/src/resources/extensions/sf/write-intercept.ts index d500b5c75..7d7f995d0 100644 --- a/src/resources/extensions/sf/write-intercept.ts +++ b/src/resources/extensions/sf/write-intercept.ts @@ -96,8 +96,8 @@ function matchesBlockedPattern(path: string): boolean { * Directs the agent to use engine tool calls instead. */ export const BLOCKED_WRITE_ERROR = `Direct writes to .sf/STATE.md and .sf/sf.db are blocked. Use engine tool calls instead: -- To complete a task: call sf_complete_task(milestone_id, slice_id, task_id, summary) -- To complete a slice: call sf_complete_slice(milestone_id, slice_id, summary, uat_result) +- To complete a task: call sf_task_complete(milestone_id, slice_id, task_id, summary) +- To complete a slice: call sf_slice_complete(milestone_id, slice_id, summary, uat_result) - To save a decision: call sf_save_decision(scope, decision, choice, rationale) - To start a task: call sf_start_task(milestone_id, slice_id, task_id) - To record verification: call sf_record_verification(milestone_id, slice_id, task_id, evidence)