diff --git a/docs-web/content/docs/user-dashboard-agents.mdx b/docs-web/content/docs/user-dashboard-agents.mdx index 25d7ef81f1..1cf69e7492 100644 --- a/docs-web/content/docs/user-dashboard-agents.mdx +++ b/docs-web/content/docs/user-dashboard-agents.mdx @@ -86,7 +86,7 @@ When a selected built-in has not diverged from its tracked baseline, a newer bun The Agents page heading is **Planning agent base update available** or **Project manager base update available**. It explains whether the named preset has customized instructions or is assigned to that route, followed by: **Updating invokes an agent to compare both base files and apply only important system-compatibility instructions. Your main prompt, custom instructions, and behavior are preserved.** No merge runs until you choose **Update with AI**. -To apply a notice, the dashboard uses `POST /api/projects/:projectId/agent-presets/base-updates/:baseAgentRole/apply`, where the role must be `planning_agent` or `project_manager`. The endpoint consumes no request body and runs through the configured supported local planning provider. The agent compares the previous base, current bundle, and selected preset, and may add only compatibility-critical system instructions such as changed MCP or strict output-schema requirements. Code UX accepts only raw JSON containing a single non-empty `instructionMarkdown` value, verifies that all original preset lines remain in order, rechecks that routing still selects the noticed preset, and writes the markdown itself. +To apply a notice, the dashboard uses `POST /api/projects/:projectId/agent-presets/base-updates/:baseAgentRole/apply`, where the role must be `planning_agent` or `project_manager`. The endpoint consumes no request body and runs through the configured supported local planning provider. The agent compares the previous base, current bundle, and selected preset, and may add only compatibility-critical system instructions such as changed MCP or strict output-schema requirements. Code UX tolerates harmless presentation text, markdown fences, and supported provider response envelopes, but the extracted payload must still be a JSON object containing exactly one non-empty `instructionMarkdown` string and no other properties. It verifies that all original preset lines remain in order, rechecks that routing still selects the noticed preset, and writes the markdown itself only after parsing and safety checks succeed. The merge cannot change the main prompt, custom behavior, avatar, labels, routing, provider/model, memory, MCP access, persistent skills, or source metadata. A failed provider call, unsupported provider, malformed or destructive response, stale notice, or route change leaves both the preset and its stored bundled revision unchanged, so the notice remains available for retry. A successful merge records the current bundled revision even when the preserved custom markdown differs from the bundle, preventing the same notice from returning. diff --git a/docs-web/user/dashboard/agents.md b/docs-web/user/dashboard/agents.md index 17439a5bdb..5353b8c59d 100644 --- a/docs-web/user/dashboard/agents.md +++ b/docs-web/user/dashboard/agents.md @@ -86,7 +86,7 @@ When a selected built-in has not diverged from its tracked baseline, a newer bun The Agents page heading is **Planning agent base update available** or **Project manager base update available**. It explains whether the named preset has customized instructions or is assigned to that route, followed by: **Updating invokes an agent to compare both base files and apply only important system-compatibility instructions. Your main prompt, custom instructions, and behavior are preserved.** No merge runs until you choose **Update with AI**. -To apply a notice, the dashboard uses `POST /api/projects/:projectId/agent-presets/base-updates/:baseAgentRole/apply`, where the role must be `planning_agent` or `project_manager`. The endpoint consumes no request body and runs through the configured supported local planning provider. The agent compares the previous base, current bundle, and selected preset, and may add only compatibility-critical system instructions such as changed MCP or strict output-schema requirements. Code UX accepts only raw JSON containing a single non-empty `instructionMarkdown` value, verifies that all original preset lines remain in order, rechecks that routing still selects the noticed preset, and writes the markdown itself. +To apply a notice, the dashboard uses `POST /api/projects/:projectId/agent-presets/base-updates/:baseAgentRole/apply`, where the role must be `planning_agent` or `project_manager`. The endpoint consumes no request body and runs through the configured supported local planning provider. The agent compares the previous base, current bundle, and selected preset, and may add only compatibility-critical system instructions such as changed MCP or strict output-schema requirements. Code UX tolerates harmless presentation text, markdown fences, and supported provider response envelopes, but the extracted payload must still be a JSON object containing exactly one non-empty `instructionMarkdown` string and no other properties. It verifies that all original preset lines remain in order, rechecks that routing still selects the noticed preset, and writes the markdown itself only after parsing and safety checks succeed. The merge cannot change the main prompt, custom behavior, avatar, labels, routing, provider/model, memory, MCP access, persistent skills, or source metadata. A failed provider call, unsupported provider, malformed or destructive response, stale notice, or route change leaves both the preset and its stored bundled revision unchanged, so the notice remains available for retry. A successful merge records the current bundled revision even when the preserved custom markdown differs from the bundle, preventing the same notice from returning. diff --git a/docs/architecture/agent-preset-foundation.md b/docs/architecture/agent-preset-foundation.md index 1dfc51332e..d37e45a998 100644 --- a/docs/architecture/agent-preset-foundation.md +++ b/docs/architecture/agent-preset-foundation.md @@ -95,7 +95,7 @@ Dashboard endpoints: Base-agent updates are limited to `planning_agent` and `project_manager`. The notice endpoint performs no provider work; it reports only changed bundled baselines that cannot be applied automatically because the selected preset has custom instructions or the role is routed to an alternate preset. -Applying a notice uses the existing `planning` virtual-provider route and structured invocation pipeline, recorded as execution invocation type `agent_base_update`. The provider receives the previous bundled/base instructions, current bundled instructions, and the selected preset instructions, but is restricted to returning one raw JSON property: `instructionMarkdown`. Its prompt permits only compatibility-critical additions, such as changed MCP or JSON-schema rules, and forbids workspace writes or metadata changes. Code UX parses the response strictly and then applies only instruction markdown through `AgentPresetSyncService`; avatar, labels, routing, provider/model, memory, MCP access, persistent skills, and source metadata remain unchanged. The stored bundled revision advances only after provider execution and parsing succeed, and the selected preset ID is checked again immediately before application to prevent a concurrent route change from redirecting the result. +Applying a notice uses the existing `planning` virtual-provider route and structured invocation pipeline, recorded as execution invocation type `agent_base_update`. The provider receives the previous bundled/base instructions, current bundled instructions, and the selected preset instructions, but is restricted to returning one JSON property: `instructionMarkdown`. The parser tolerates presentation text, markdown fences, and the shared extractor's supported provider envelopes, then still requires the extracted payload to be a non-array object with exactly one non-empty string property named `instructionMarkdown`. Its prompt permits only compatibility-critical additions, such as changed MCP or JSON-schema rules, and forbids workspace writes or metadata changes. Code UX verifies the original preset remains line-for-line and in order before applying only instruction markdown through `AgentPresetSyncService`; avatar, labels, routing, provider/model, memory, MCP access, persistent skills, and source metadata remain unchanged. Extraction, payload validation, and preservation checks all complete before persistence. The stored bundled revision advances only after provider execution and parsing succeed, and the selected preset ID is checked again immediately before application to prevent a concurrent route change from redirecting the result. These endpoints are project-scoped and intentionally separate from: diff --git a/src/services/agent-base-update-service.ts b/src/services/agent-base-update-service.ts index 7e9b7374aa..307cf7ad53 100644 --- a/src/services/agent-base-update-service.ts +++ b/src/services/agent-base-update-service.ts @@ -5,6 +5,7 @@ import type { BaseAgentUpdateNotice, } from "../contracts/agent-preset-types.js"; import type { ProviderId, Subtask } from "../contracts/app-types.js"; +import { extractJsonFromText } from "../domain/llm/json-extraction.js"; import type { ProjectManagementRepository } from "../repositories/project-management-repository.js"; import type { SettingsRepository } from "../repositories/settings-repository.js"; import { EntityNotFoundError, ValidationError } from "../repositories/repository-utils.js"; @@ -184,12 +185,11 @@ export class AgentBaseUpdateService { } private parseUpdatePayload(bodyMarkdown: string): AgentBaseUpdatePayload { - let parsed: unknown; - try { - parsed = JSON.parse(bodyMarkdown.trim()); - } catch { + const extraction = extractJsonFromText(bodyMarkdown); + if (!extraction.success) { throw new ValidationError("Base-agent update response was not raw valid JSON."); } + const parsed = extraction.data; if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { throw new ValidationError("Base-agent update response must be a JSON object."); } diff --git a/tests/backend/services/agent-base-update-service.test.ts b/tests/backend/services/agent-base-update-service.test.ts index 3d040ed724..f986282324 100644 --- a/tests/backend/services/agent-base-update-service.test.ts +++ b/tests/backend/services/agent-base-update-service.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it, vi } from "vitest"; import type { AgentPresetRecord, BaseAgentUpdateContext } from "../../../src/contracts/agent-preset-types.js"; import { DEFAULT_DASHBOARD_SETTINGS } from "../../../src/repositories/settings-defaults.js"; +import { ValidationError } from "../../../src/repositories/repository-utils.js"; import { AgentBaseUpdateService } from "../../../src/services/agent-base-update-service.js"; +const SELECTED_INSTRUCTIONS = "# Main prompt\nPlan carefully.\n\n## Custom behavior\nKeep the user's custom workflow."; +const COMPATIBLE_INSTRUCTIONS = `${SELECTED_INSTRUCTIONS}\n\n## System compatibility\nUse the current strict JSON schema.`; + function createPreset(overrides: Partial = {}): AgentPresetRecord { return { id: "base-planning", @@ -42,7 +46,7 @@ function createContext(overrides: Partial = {}): BaseAge const selectedAgentPreset = createPreset({ id: "selected-planning", name: "Specialist planner", - instructionMarkdown: "# Main prompt\nPlan carefully.\n\n## Custom behavior\nKeep the user's custom workflow.", + instructionMarkdown: SELECTED_INSTRUCTIONS, }); return { role: "planning_agent", @@ -99,7 +103,7 @@ function createHarness(options: { if (options.providerError) throw options.providerError; const bodyMarkdown = options.providerOutput ?? JSON.stringify({ - instructionMarkdown: `${context!.selectedAgentPreset.instructionMarkdown}\n\n## System compatibility\nUse the current strict JSON schema.`, + instructionMarkdown: COMPATIBLE_INSTRUCTIONS, }); return { parsed: args.parseFn(bodyMarkdown), @@ -167,6 +171,37 @@ describe("AgentBaseUpdateService", () => { }); }); + it.each([ + ["raw JSON", JSON.stringify({ instructionMarkdown: ` ${COMPATIBLE_INSTRUCTIONS}\n` })], + [ + "fenced JSON", + `Here is the requested update:\n\`\`\`json\n${JSON.stringify({ instructionMarkdown: COMPATIBLE_INSTRUCTIONS })}\n\`\`\``, + ], + [ + "JSON with presentation noise", + `Update follows.\n${JSON.stringify({ instructionMarkdown: COMPATIBLE_INSTRUCTIONS })}\nEnd of update.`, + ], + [ + "a supported object envelope", + JSON.stringify({ response: { instructionMarkdown: COMPATIBLE_INSTRUCTIONS } }), + ], + [ + "a supported string envelope", + JSON.stringify({ content: JSON.stringify({ instructionMarkdown: COMPATIBLE_INSTRUCTIONS }) }), + ], + ])("accepts %s without changing the update contract", async (_label, providerOutput) => { + const { service, applyBaseAgentInstructionUpdate } = createHarness({ providerOutput }); + + await service.applyUpdate("project-1", "planning_agent"); + + expect(applyBaseAgentInstructionUpdate).toHaveBeenCalledWith( + "project-1", + "planning_agent", + COMPATIBLE_INSTRUCTIONS, + "selected-planning", + ); + }); + it("does not invoke a provider when no base update is available", async () => { const { service, executeRequest, applyBaseAgentInstructionUpdate } = createHarness({ context: null }); @@ -175,12 +210,29 @@ describe("AgentBaseUpdateService", () => { expect(applyBaseAgentInstructionUpdate).not.toHaveBeenCalled(); }); - it("leaves the preset and baseline untouched when provider output is malformed", async () => { + it.each([ + ["malformed output", '{"instructionMarkdown":"unterminated} ', "was not raw valid JSON"], + ["empty output", " \n", "was not raw valid JSON"], + ["an array", JSON.stringify([{ instructionMarkdown: COMPATIBLE_INSTRUCTIONS }]), "must be a JSON object"], + ["an empty instructionMarkdown value", JSON.stringify({ instructionMarkdown: " \n" }), "must contain only"], + [ + "extra payload properties", + JSON.stringify({ instructionMarkdown: COMPATIBLE_INSTRUCTIONS, avatarConfig: { accent: "red" } }), + "must contain only", + ], + [ + "extra properties inside a supported envelope", + JSON.stringify({ response: { instructionMarkdown: COMPATIBLE_INSTRUCTIONS, model: "other-model" } }), + "must contain only", + ], + ])("leaves the preset and baseline untouched for %s", async (_label, providerOutput, expectedMessage) => { const { service, applyBaseAgentInstructionUpdate } = createHarness({ - providerOutput: '{"instructionMarkdown":"valid","avatarConfig":{"accent":"red"}}', + providerOutput, }); - await expect(service.applyUpdate("project-1", "planning_agent")).rejects.toThrow("must contain only"); + const update = service.applyUpdate("project-1", "planning_agent"); + await expect(update).rejects.toBeInstanceOf(ValidationError); + await expect(update).rejects.toThrow(expectedMessage); expect(applyBaseAgentInstructionUpdate).not.toHaveBeenCalled(); });