diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index 7c62c86cf..be7dc2cfd 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -199,6 +199,50 @@ describe("advisor", () => { expect(result.useless).toBe(true); }); + it("suppresses duplicate advice notes from the same advisor session", async () => { + const onAdvice = vi.fn(); + const tool = new AdviseTool(onAdvice); + const note = "I'll pause here and wait for the YAML revision."; + + await tool.execute("tc-1", { note, severity: "nit" }); + await tool.execute("tc-2", { note, severity: "nit" }); + + expect(onAdvice).toHaveBeenCalledTimes(1); + expect(onAdvice).toHaveBeenCalledWith(note, "nit"); + }); + + it("allows the same advice after delivered-note memory resets", async () => { + const onAdvice = vi.fn(); + const tool = new AdviseTool(onAdvice); + const note = "Acknowledged."; + + await tool.execute("tc-1", { note, severity: "nit" }); + tool.resetDeliveredNotes(); + await tool.execute("tc-2", { note, severity: "nit" }); + + expect(onAdvice).toHaveBeenCalledTimes(2); + expect(onAdvice).toHaveBeenNthCalledWith(1, note, "nit"); + expect(onAdvice).toHaveBeenNthCalledWith(2, note, "nit"); + }); + + it("forwards escalations of an already-delivered note and suppresses downgrades", async () => { + const onAdvice = vi.fn(); + const tool = new AdviseTool(onAdvice); + const note = "Rename collides with the existing helper."; + + await tool.execute("tc-1", { note, severity: "nit" }); + await tool.execute("tc-2", { note, severity: "concern" }); + await tool.execute("tc-3", { note, severity: "blocker" }); + // De-escalation back to nit or concern is treated as a duplicate. + await tool.execute("tc-4", { note, severity: "concern" }); + await tool.execute("tc-5", { note, severity: "nit" }); + + expect(onAdvice).toHaveBeenCalledTimes(3); + expect(onAdvice).toHaveBeenNthCalledWith(1, note, "nit"); + expect(onAdvice).toHaveBeenNthCalledWith(2, note, "concern"); + expect(onAdvice).toHaveBeenNthCalledWith(3, note, "blocker"); + }); + it("validates parameters using ArkType", () => { const onAdvice = vi.fn(); const tool = new AdviseTool(onAdvice); diff --git a/packages/coding-agent/src/advisor/advise-tool.ts b/packages/coding-agent/src/advisor/advise-tool.ts index 1c9e7d908..88ee1f79a 100644 --- a/packages/coding-agent/src/advisor/advise-tool.ts +++ b/packages/coding-agent/src/advisor/advise-tool.ts @@ -139,15 +139,37 @@ export function deriveAdvisorTelemetry( */ export const ADVISOR_READONLY_TOOL_NAMES: ReadonlySet = new Set(["read", "search", "find"]); +function advisorNoteDedupeKey(note: string): string { + return note.trim().replace(/\s+/g, " "); +} + +/** Rank advisor severities so the dedupe state can detect a real escalation + * (nit → concern → blocker) versus a verbatim repeat. `undefined` defers to + * `nit` because the schema treats an omitted severity as a plain nit. */ +const ADVISOR_SEVERITY_RANK: Record = { nit: 1, concern: 2, blocker: 3 }; +function advisorSeverityRank(severity: AdvisorSeverity | undefined): number { + return ADVISOR_SEVERITY_RANK[severity ?? "nit"]; +} + export class AdviseTool implements AgentTool { readonly name = "advise"; readonly label = "Advise"; readonly description = adviseDescription; readonly parameters = adviseSchema; readonly intent = "omit" as const; + /** Highest delivered severity rank per normalized note. A new call passes + * through only when its rank strictly exceeds the recorded one (a real + * escalation: nit → concern → blocker), so an advisor cannot bypass dedupe + * by retagging the same text at a lower or equal severity. */ + #deliveredNoteSeverities = new Map(); constructor(private readonly onAdvice: (note: string, severity?: AdviseDetails["severity"]) => void) {} + /** Clear delivered-note memory when the advisor starts a fresh conversation. */ + resetDeliveredNotes(): void { + this.#deliveredNoteSeverities.clear(); + } + async execute( _toolCallId: string, args: AdviseParams, @@ -155,6 +177,17 @@ export class AdviseTool implements AgentTool _onUpdate?: AgentToolUpdateCallback, _context?: AgentToolContext, ): Promise> { + const key = advisorNoteDedupeKey(args.note); + const rank = advisorSeverityRank(args.severity); + const previousRank = this.#deliveredNoteSeverities.get(key) ?? 0; + if (rank <= previousRank) { + return { + content: [{ type: "text", text: "Duplicate advice ignored." }], + details: { note: args.note, severity: args.severity }, + useless: true, + }; + } + this.#deliveredNoteSeverities.set(key, rank); this.onAdvice(args.note, args.severity); return { content: [{ type: "text", text: "Recorded." }], diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 038729401..d7c03c5bc 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1164,6 +1164,7 @@ export class AgentSession { #advisorEnabled = false; /** The advisor's own agent, retained so `/dump advisor` can serialize its transcript. Undefined when no advisor is active. */ #advisorAgent?: Agent; + #advisorAdviseTool?: AdviseTool; #advisorReadOnlyTools?: AgentTool[]; #advisorWatchdogPrompt?: string; #advisorYieldQueueUnsubscribe?: () => void; @@ -1798,6 +1799,7 @@ export class AgentSession { this.#advisorAgentUnsubscribe?.(); this.#advisorAgentUnsubscribe = undefined; this.#advisorRuntime?.reset(); + this.#advisorAdviseTool?.resetDeliveredNotes(); this.#attachAdvisorRecorderFeed(); this.#advisorPrimaryTurnsCompleted = 0; this.#advisorInterruptImmuneTurnStart = undefined; @@ -1878,6 +1880,7 @@ export class AgentSession { }; const adviseTool = new AdviseTool(enqueueAdvice); + this.#advisorAdviseTool = adviseTool; const advisorReadOnlyTools = this.#advisorReadOnlyTools ?? []; const appendOnlyContext = new AppendOnlyContextManager(); @@ -2004,6 +2007,7 @@ export class AgentSession { if (this.#advisorAgent) { this.#advisorAgent = undefined; } + this.#advisorAdviseTool = undefined; this.#advisorYieldQueueUnsubscribe?.(); this.#advisorYieldQueueUnsubscribe = undefined; }