From 6d58901cc3dbec3e1b7105117254e1d1c10d9fec Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 00:26:01 +0000 Subject: [PATCH 1/3] fix(advisor): suppressed repeated advisories Deduplicated advisor notes inside the advise tool so a model cannot enqueue the same advisory repeatedly in one session. Added focused regression coverage for duplicate advisory suppression. Fixes #3511 --- .../src/advisor/__tests__/advisor.test.ts | 12 ++++++++++++ packages/coding-agent/src/advisor/advise-tool.ts | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index 7c62c86cf..a7ed94786 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -199,6 +199,18 @@ 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("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..f21e22fdd 100644 --- a/packages/coding-agent/src/advisor/advise-tool.ts +++ b/packages/coding-agent/src/advisor/advise-tool.ts @@ -139,12 +139,17 @@ 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, " "); +} + export class AdviseTool implements AgentTool { readonly name = "advise"; readonly label = "Advise"; readonly description = adviseDescription; readonly parameters = adviseSchema; readonly intent = "omit" as const; + #deliveredNoteKeys = new Set(); constructor(private readonly onAdvice: (note: string, severity?: AdviseDetails["severity"]) => void) {} @@ -155,6 +160,15 @@ export class AdviseTool implements AgentTool _onUpdate?: AgentToolUpdateCallback, _context?: AgentToolContext, ): Promise> { + const key = advisorNoteDedupeKey(args.note); + if (this.#deliveredNoteKeys.has(key)) { + return { + content: [{ type: "text", text: "Duplicate advice ignored." }], + details: { note: args.note, severity: args.severity }, + useless: true, + }; + } + this.#deliveredNoteKeys.add(key); this.onAdvice(args.note, args.severity); return { content: [{ type: "text", text: "Recorded." }], From cce5133bfa179610b56c1b3c6316ffc0a857920e Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 00:31:51 +0000 Subject: [PATCH 2/3] fix(advisor): reset advisory dedupe state Cleared delivered-note memory when the advisor session state resets across conversation boundaries. Added coverage that repeated advice is allowed again after the dedupe state resets. Fixes #3511 --- .../src/advisor/__tests__/advisor.test.ts | 14 ++++++++++++++ packages/coding-agent/src/advisor/advise-tool.ts | 5 +++++ packages/coding-agent/src/session/agent-session.ts | 4 ++++ 3 files changed, 23 insertions(+) diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index a7ed94786..18a6d7eb2 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -211,6 +211,20 @@ describe("advisor", () => { 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("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 f21e22fdd..2d6b3e3ed 100644 --- a/packages/coding-agent/src/advisor/advise-tool.ts +++ b/packages/coding-agent/src/advisor/advise-tool.ts @@ -153,6 +153,11 @@ export class AdviseTool implements AgentTool constructor(private readonly onAdvice: (note: string, severity?: AdviseDetails["severity"]) => void) {} + /** Clear delivered-note memory when the advisor starts a fresh conversation. */ + resetDeliveredNotes(): void { + this.#deliveredNoteKeys.clear(); + } + async execute( _toolCallId: string, args: AdviseParams, 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; } From f62ee17080581787cfa24e0075d9b07bc1737ccc Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 00:34:04 +0000 Subject: [PATCH 3/3] fix(advisor): forwarded severity escalations past dedupe Tracked the highest delivered severity per note so a nit can later land as a concern or blocker without being silently dropped. De-escalation back to nit/concern stays treated as a duplicate so the model cannot flap severities to bypass dedupe. Fixes #3511 --- .../src/advisor/__tests__/advisor.test.ts | 18 +++++++++++++++ .../coding-agent/src/advisor/advise-tool.ts | 22 +++++++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index 18a6d7eb2..be7dc2cfd 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -225,6 +225,24 @@ describe("advisor", () => { 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 2d6b3e3ed..88ee1f79a 100644 --- a/packages/coding-agent/src/advisor/advise-tool.ts +++ b/packages/coding-agent/src/advisor/advise-tool.ts @@ -143,19 +143,31 @@ 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; - #deliveredNoteKeys = new Set(); + /** 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.#deliveredNoteKeys.clear(); + this.#deliveredNoteSeverities.clear(); } async execute( @@ -166,14 +178,16 @@ export class AdviseTool implements AgentTool _context?: AgentToolContext, ): Promise> { const key = advisorNoteDedupeKey(args.note); - if (this.#deliveredNoteKeys.has(key)) { + 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.#deliveredNoteKeys.add(key); + this.#deliveredNoteSeverities.set(key, rank); this.onAdvice(args.note, args.severity); return { content: [{ type: "text", text: "Recorded." }],