Merge remote-tracking branch 'origin/farm/3d061a17/stop-stale-advisor-loop'

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);

packages/coding-agent/src/advisor/advise-tool.ts

@@ -139,15 +139,37 @@ export function deriveAdvisorTelemetry(
  */
 export const ADVISOR_READONLY_TOOL_NAMES: ReadonlySet<string> = 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<AdvisorSeverity, number> = { nit: 1, concern: 2, blocker: 3 };
+function advisorSeverityRank(severity: AdvisorSeverity | undefined): number {
+	return ADVISOR_SEVERITY_RANK[severity ?? "nit"];
+}
+
 export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails> {
 	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<string, number>();
 
 	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<typeof adviseSchema, AdviseDetails>
 		_onUpdate?: AgentToolUpdateCallback<AdviseDetails>,
 		_context?: AgentToolContext,
 	): Promise<AgentToolResult<AdviseDetails>> {
+		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." }],

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;
 	}