Merge PR #3523: fix(advisor): filter content-free advisor notes at enqueue boundary (@roboomp)

# Conflicts:
#	packages/coding-agent/src/session/agent-session.ts

docs/advisor-watchdog.md

@@ -8,6 +8,7 @@ The advisor is not a second executor. It cannot edit files, run commands, approv
 
 - [`src/advisor/runtime.ts`](../packages/coding-agent/src/advisor/runtime.ts)
 - [`src/advisor/advise-tool.ts`](../packages/coding-agent/src/advisor/advise-tool.ts)
+- [`src/advisor/emission-guard.ts`](../packages/coding-agent/src/advisor/emission-guard.ts)
 - [`src/advisor/watchdog.ts`](../packages/coding-agent/src/advisor/watchdog.ts)
 - [`src/advisor/transcript-recorder.ts`](../packages/coding-agent/src/advisor/transcript-recorder.ts)
 - [`src/prompts/advisor/system.md`](../packages/coding-agent/src/prompts/advisor/system.md)
@@ -100,6 +101,19 @@ When you deliberately interrupt the agent (Esc, or a cancel from collab, ACP, RP
 
 `advisor.immuneTurns` limits interruption frequency. After the advisor successfully delivers a `concern` or `blocker` through the steering channel, later concerns/blockers are routed as non-interrupting asides until the configured number of primary turns has completed. The default is `3`. `nit` notes are unchanged, and advice raised while user-interrupt auto-resume suppression is active is still preserved instead of restarting a stopped run.
 
+### Emission guard
+
+`AdvisorEmissionGuard` (in `src/advisor/emission-guard.ts`) sits on the `enqueueAdvice` boundary in `AgentSession` and enforces — in code — the advisor system prompt's "at most one `advise` per update" and "NEVER send the same advice twice" rules. Each call to the advisor's `advise` tool runs through the guard before it routes to the YieldQueue / steer channel:
+
+1. **Normalization.** Lowercase, NFKC, collapse every run of non-alphanumeric characters to a single space, trim. `"Stop."`, `"*Stop*"`, and `"  stop  "` all key to `stop`.
+2. **Content-free phrase filter.** A small allowlist of normalized phrases the advisor occasionally emits but that carry no concrete reason — `stop`, `done`, `complete`, `no issue continue`, `lgtm`, `nothing to add`, `no further input`, and similar — is suppressed silently. Silence is the correct expression of "no concerns".
+3. **Exact-text dedupe.** Any normalized note already accepted in this session is dropped. The dedupe history is bounded by a FIFO ring (default 4096 entries).
+4. **Per-update rate limit.** At most one note per advisor model `prompt()` cycle is accepted; the runtime calls `host.beginAdvisorUpdate?.()` before each cycle to reset the gate. Suppressed calls never consume the budget — a noise call doesn't displace a real concern that follows in the same update.
+
+Suppression is invisible to the advisor model: `AdviseTool` still returns `Recorded.` for a dropped call. Surfacing "suppressed" back into advisor context risks the model rephrasing the same useless note to bypass the dedupe.
+
+The guard's full state — dedupe history and per-update gate — clears on every advisor reset (compaction, session switch, `/new`), so a re-primed reviewer can re-raise issues it already raised against the rewritten transcript.
+
 ## Bounded catch-up with `advisor.syncBacklog`
 
 `advisor.syncBacklog` is not lockstep turn execution. It is a bounded catch-up delay for the primary agent when the advisor falls behind.

packages/coding-agent/CHANGELOG.md

@@ -66,6 +66,10 @@
 - Fixed repeated todo updates in one TUI turn stacking full todo panels; superseded todo snapshots now stay live until the next todo update replaces them or the turn ends. ([#3516](https://github.com/can1357/oh-my-pi/issues/3516))
 - Fixed MCP OAuth authorization failing with `Authorization failed: An unexpected error occurred` against authorization servers (Plane is the live example) that reject redundant fallback `resource` indicators. OMP now drops same-origin resources only when it synthesized them from the server URL fallback (e.g. `https://mcp.plane.so/http/mcp`). Provider-advertised resources from OAuth/protected-resource discovery or an embedded authorization-URL `resource` query parameter are preserved even when they are same-origin or origin-only, so gateway-hosted MCP services can still request the audience they advertised. The refresh-token path uses the same policy, filtered against the authorization-server origin persisted on the credential as `authorizationUrl`, with `tokenUrl`'s origin as the legacy fallback when that field is absent. ([#3502](https://github.com/can1357/oh-my-pi/issues/3502))
 
+### Fixed
+
+- Fixed the advisor entering a spam loop in which it emitted hundreds of repeated `Stop.`, `Done.`, and `No issue; continue.` `<advisory severity="blocker">` injections, polluting the primary transcript and destabilizing the watched agent after the task was already complete. The advisor system prompt's rules ("at most one `advise` per update", "NEVER send the same advice twice") are now enforced in code by a new `AdvisorEmissionGuard` on the `enqueueAdvice` boundary in `AgentSession`: it normalizes each note (case-insensitive, punctuation-folded), drops content-free self-talk filler (`stop`/`done`/`no issue continue`/`lgtm`/etc.), dedupes by exact normalized text across the session (bounded FIFO history), and rate-limits to one accepted note per advisor model prompt cycle. Reset on advisor reset (compaction, session switch, `/new`) so a re-primed reviewer can re-raise old issues. ([#3520](https://github.com/can1357/oh-my-pi/issues/3520))
+
 ## [16.1.20] - 2026-06-25
 
 ### Fixed

packages/coding-agent/src/advisor/__tests__/emission-guard.test.ts

@@ -0,0 +1,147 @@
+import { describe, expect, it } from "bun:test";
+import { AdvisorEmissionGuard, normalizeAdvisorNote } from "../emission-guard";
+
+describe("normalizeAdvisorNote", () => {
+	it("collapses punctuation, casing, and surrounding whitespace into one canonical key", () => {
+		// The reporter's three top duplicates all key to the same canonical form
+		// regardless of trailing punctuation or casing — that's what makes the
+		// dedupe + suppression checks single-membership.
+		expect(normalizeAdvisorNote("Stop.")).toBe("stop");
+		expect(normalizeAdvisorNote("  STOP!  ")).toBe("stop");
+		expect(normalizeAdvisorNote("*Stop*")).toBe("stop");
+		expect(normalizeAdvisorNote("Done.")).toBe("done");
+		expect(normalizeAdvisorNote("No issue; continue.")).toBe("no issue continue");
+	});
+
+	it("returns empty string for whitespace-only input so callers can short-circuit", () => {
+		expect(normalizeAdvisorNote("")).toBe("");
+		expect(normalizeAdvisorNote("   ")).toBe("");
+		expect(normalizeAdvisorNote("...")).toBe("");
+	});
+
+	it("preserves internal letters/digits but folds non-alphanumeric runs to one space", () => {
+		expect(normalizeAdvisorNote("Refactor `auth-flow.ts`: drop legacy branch.")).toBe(
+			"refactor auth flow ts drop legacy branch",
+		);
+	});
+});
+
+describe("AdvisorEmissionGuard", () => {
+	it("drops the exact content-free filler the reporter observed flooding the chat", () => {
+		// Issue #3520: 114× "Stop.", 52× "No issue; continue.", 41× "Done." —
+		// none of these carry a concrete reason and they cannot be acted on, so
+		// the guard suppresses them regardless of severity.
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("Stop.")).toBe(false);
+		expect(guard.accept("Done.")).toBe(false);
+		expect(guard.accept("No issue; continue.")).toBe(false);
+		expect(guard.accept("LGTM")).toBe(false);
+		expect(guard.accept("No further watcher input needed.")).toBe(false);
+	});
+
+	it("dedupes by normalized text across the session, ignoring casing and trailing punctuation", () => {
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("Move retries into the queue, not the request path.")).toBe(true);
+		// Same advice with different casing and trailing punctuation must NOT
+		// land twice in the primary transcript.
+		expect(guard.accept("move retries into the queue, not the request path")).toBe(false);
+		expect(guard.accept("Move retries into the queue, not the request path!")).toBe(false);
+	});
+
+	it("rate-limits to one accepted advise per advisor update cycle", () => {
+		// The advisor system prompt says "at most one `advise` per update". Real
+		// models violate this; the guard enforces it at the boundary so the
+		// primary transcript never receives two advisories from one model cycle.
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("First concern: missing await in #handleRetry.")).toBe(true);
+		expect(guard.accept("Second concern: wrong env var name.")).toBe(false);
+		guard.beginUpdate();
+		// New cycle: budget reset.
+		expect(guard.accept("Second concern: wrong env var name.")).toBe(true);
+	});
+
+	it("does not let a suppressed call consume the per-update budget", () => {
+		// A noise call like "Stop." must never displace a real concern that
+		// follows in the same advisor model cycle.
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("Stop.")).toBe(false);
+		expect(guard.accept("Concrete: read race in #handleRetry.")).toBe(true);
+	});
+
+	it("does not let a deduped call consume the per-update budget", () => {
+		// A repeat of a prior session note is dropped, but the model can still
+		// follow it with a fresh concrete concern in the same cycle.
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("Concrete: read race in #handleRetry.")).toBe(true);
+		guard.beginUpdate();
+		expect(guard.accept("Concrete: read race in #handleRetry.")).toBe(false);
+		expect(guard.accept("New concern: cache eviction never fires.")).toBe(true);
+	});
+
+	it("reset clears dedupe and the per-update gate so a re-primed advisor can re-raise old issues", () => {
+		// Compaction / session-switch rewrites the primary transcript. The
+		// advisor is re-primed from scratch and may legitimately re-raise the
+		// same concerns — they're new context for a freshly-primed reviewer.
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("Race in #handleRetry.")).toBe(true);
+		expect(guard.accept("Race in #handleRetry.")).toBe(false);
+		guard.reset();
+		expect(guard.accept("Race in #handleRetry.")).toBe(true);
+	});
+
+	it("evicts oldest entries when dedupe history exceeds capacity", () => {
+		// Bounded so very long sessions cannot grow the dedupe state without
+		// bound. Pre-eviction unique notes are remembered; post-eviction the
+		// oldest one is forgotten and can resurface.
+		const guard = new AdvisorEmissionGuard({ capacity: 3 });
+		expect(guard.accept("first")).toBe(true);
+		guard.beginUpdate();
+		expect(guard.accept("second")).toBe(true);
+		guard.beginUpdate();
+		expect(guard.accept("third")).toBe(true);
+		guard.beginUpdate();
+		// "first" still in history.
+		expect(guard.accept("first")).toBe(false);
+		guard.beginUpdate();
+		// Fourth unique entry evicts "first".
+		expect(guard.accept("fourth")).toBe(true);
+		guard.beginUpdate();
+		expect(guard.accept("first")).toBe(true);
+	});
+
+	it("rejects empty / whitespace-only notes without consuming the budget", () => {
+		const guard = new AdvisorEmissionGuard();
+		expect(guard.accept("")).toBe(false);
+		expect(guard.accept("   ")).toBe(false);
+		expect(guard.accept("Concrete advice.")).toBe(true);
+	});
+
+	it("end-to-end: the reporter's 309-call spam log produces ≤1 accepted note across many updates", () => {
+		// Mimic the issue's distribution: 114× "Stop.", 52× "No issue; continue.",
+		// 41× "Done.", plus 102 copies of one concrete-but-repeated nit. Spread
+		// the calls across 50 advisor update cycles. Each cycle is allowed at
+		// most one accepted note, and identical-text repeats never escape the
+		// guard. After all calls, exactly the concrete nit has been accepted
+		// — and only once.
+		const guard = new AdvisorEmissionGuard();
+		const accepted: string[] = [];
+		const stream: string[] = [
+			...Array(114).fill("Stop."),
+			...Array(52).fill("No issue; continue."),
+			...Array(41).fill("Done."),
+			...Array(102).fill("Concrete-but-repeated nit: x"),
+		];
+		// Interleave across 50 update cycles.
+		const cycles = 50;
+		const perCycle = Math.ceil(stream.length / cycles);
+		for (let c = 0; c < cycles; c++) {
+			guard.beginUpdate();
+			for (let i = 0; i < perCycle; i++) {
+				const note = stream[c * perCycle + i];
+				if (note === undefined) break;
+				if (guard.accept(note)) accepted.push(note);
+			}
+		}
+		expect(accepted).toEqual(["Concrete-but-repeated nit: x"]);
+	});
+});

packages/coding-agent/src/advisor/emission-guard.ts

@@ -0,0 +1,172 @@
+/**
+ * Per-session policy gate for advisor `advise()` calls.
+ *
+ * The advisor system prompt tells the watcher model:
+ *
+ * > at most one `advise` per update
+ * > NEVER repeat advice you already gave, and NEVER send the same advice twice
+ *
+ * Real advisor models violate this. Issue #3520 captured a session where
+ * `__advisor.jsonl` recorded 309 `advise` calls covering 92 unique notes —
+ * 114× `Stop.`, 52× `No issue; continue.`, 41× `Done.` — flooding the primary
+ * transcript with `<advisory severity="blocker">Stop.</advisory>` after the
+ * task was already complete. The fix is to make the rules load-bearing in code
+ * instead of prose: silently drop duplicates, content-free self-talk, and
+ * over-budget calls at the `enqueueAdvice` boundary so the primary stays
+ * clean even when the advisor misbehaves.
+ *
+ * The gate is intentionally invisible to the advisor model — `AdviseTool`
+ * still returns `Recorded.` for a suppressed call. Surfacing "suppressed"
+ * back into advisor context risks the model rephrasing the same useless note
+ * to bypass the dedupe ("Stop.", then "Halt." then "Stop now.").
+ */
+
+/**
+ * Case-insensitive, punctuation-folded normalization. Collapses every run of
+ * non-letter / non-digit characters into a single space and trims, so
+ * `"Stop."`, `"*Stop*"`, and `"  stop  "` all key to `stop`, while
+ * `"No issue; continue."` keys to `no issue continue`.
+ *
+ * Exported for tests.
+ */
+export function normalizeAdvisorNote(note: string): string {
+	return note
+		.toLowerCase()
+		.normalize("NFKC")
+		.replace(/[^\p{L}\p{N}]+/gu, " ")
+		.trim();
+}
+
+/**
+ * Normalized phrases the advisor occasionally emits that carry no concrete
+ * actionable content. Each must be the output of {@link normalizeAdvisorNote}
+ * so a single membership check covers every punctuation/casing variant
+ * (`"Stop."`, `"stop"`, `"STOP!"`).
+ *
+ * The list is conservative — only short, content-free filler the reporter
+ * observed driving primary-transcript pollution. A genuine `blocker` like
+ * `"Stop: 'await' missing on writeStream.end() will lose buffered writes."`
+ * does not match.
+ */
+const SUPPRESSED_NORMALIZED_PHRASES: Record<string, true> = {
+	// Self-stop noise — telling the agent to "stop" without a reason is useless.
+	stop: true,
+	"stop here": true,
+	"stop now": true,
+	halt: true,
+	abort: true,
+	// Completion self-talk — the agent already finished the task.
+	done: true,
+	"task done": true,
+	"task complete": true,
+	complete: true,
+	finished: true,
+	ok: true,
+	okay: true,
+	"ok done": true,
+	// "Nothing to flag" — silence is the correct expression of "no concerns".
+	"no issue": true,
+	"no issues": true,
+	"no issue continue": true,
+	"no concerns": true,
+	"no concern": true,
+	"nothing to add": true,
+	"nothing to flag": true,
+	"nothing to report": true,
+	"no notes": true,
+	"no further input": true,
+	"no further input needed": true,
+	"no further input required": true,
+	"no further watcher input": true,
+	"no further watcher input needed": true,
+	"no further advice": true,
+	"no further advice needed": true,
+	// Endorsements — equivalent to silence.
+	lgtm: true,
+	"looks good": true,
+	"all good": true,
+	"agent is on track": true,
+	"agent on track": true,
+	"on track": true,
+	continue: true,
+	"carry on": true,
+};
+
+/**
+ * Bounds the dedupe history. Sessions with very long advisor activity could
+ * otherwise grow the set without bound. The reporter's pathological session
+ * had 92 unique notes; 4096 leaves headroom while staying tiny (≤ ~256 KB of
+ * normalized strings even at long max).
+ */
+const DEFAULT_HISTORY_CAPACITY = 4096;
+
+/**
+ * Decides whether an advisor `advise()` call should reach the primary agent.
+ *
+ * Enforces — in this order — the noise filter, session-scoped exact-text
+ * dedupe (FIFO-evicted at {@link DEFAULT_HISTORY_CAPACITY}), and a per-update
+ * rate limit of one accepted note per advisor model prompt. Suppressed calls
+ * never consume the per-update budget — a noise call doesn't burn the slot
+ * for a real concern that follows in the same update.
+ *
+ * Reset on advisor reset (compaction, session switch, `/new`) via
+ * {@link reset}. Per-update gate is cleared at the start of every advisor
+ * `agent.prompt()` cycle via {@link beginUpdate}.
+ */
+export class AdvisorEmissionGuard {
+	#seen = new Set<string>();
+	/** Insertion-order log to drive FIFO eviction without an extra Map. */
+	#seenOrder: string[] = [];
+	#consumedThisUpdate = false;
+	readonly #capacity: number;
+
+	constructor(opts: { capacity?: number } = {}) {
+		this.#capacity = opts.capacity ?? DEFAULT_HISTORY_CAPACITY;
+	}
+
+	/**
+	 * Drop all dedupe and per-update state. Called from
+	 * `AgentSession#resetAdvisorSessionState()` whenever the advisor runtime is
+	 * reset — same boundary as `yieldQueue.clear("advisor")`, so a re-primed
+	 * advisor can re-raise old issues (the primary transcript was rewritten).
+	 */
+	reset(): void {
+		this.#seen.clear();
+		this.#seenOrder.length = 0;
+		this.#consumedThisUpdate = false;
+	}
+
+	/**
+	 * Clear the per-update rate-limit gate. Called by `AdvisorRuntime` right
+	 * before each `agent.prompt(batch)` invocation so the next advisor model
+	 * cycle starts with a fresh budget of one advise.
+	 */
+	beginUpdate(): void {
+		this.#consumedThisUpdate = false;
+	}
+
+	/**
+	 * Whether the proposed note should reach the primary. On `true` the gate
+	 * has already recorded the note (consumed the per-update budget and added
+	 * it to the dedupe history) — caller delivers the note. On `false` the
+	 * caller drops it.
+	 *
+	 * Empty / whitespace-only notes are suppressed; the model's
+	 * tool-args contract still requires a non-empty string but defense-in-depth.
+	 */
+	accept(note: string): boolean {
+		const key = normalizeAdvisorNote(note);
+		if (!key) return false;
+		if (SUPPRESSED_NORMALIZED_PHRASES[key]) return false;
+		if (this.#seen.has(key)) return false;
+		if (this.#consumedThisUpdate) return false;
+		this.#consumedThisUpdate = true;
+		this.#seen.add(key);
+		this.#seenOrder.push(key);
+		if (this.#seenOrder.length > this.#capacity) {
+			const stale = this.#seenOrder.shift();
+			if (stale !== undefined) this.#seen.delete(stale);
+		}
+		return true;
+	}
+}

packages/coding-agent/src/advisor/index.ts

@@ -1,4 +1,5 @@
 export * from "./advise-tool";
+export * from "./emission-guard";
 export * from "./runtime";
 export * from "./transcript-recorder";
 export * from "./watchdog";

packages/coding-agent/src/advisor/runtime.ts

@@ -30,6 +30,13 @@ export interface AdvisorRuntimeHost {
 	 * the primary's next compaction triggers {@link AdvisorRuntime.reset}).
 	 */
 	maintainContext?(incomingTokens: number): Promise<boolean>;
+	/**
+	 * Called immediately before each `agent.prompt(batch)` cycle. Lets the host
+	 * clear per-update advisor state — currently the one-advise-per-update gate
+	 * in {@link AdvisorEmissionGuard}, which the host owns because it is the
+	 * one that routes `advise()` results back to the primary.
+	 */
+	beginAdvisorUpdate?(): void;
 }
 
 interface PendingDelta {
@@ -275,6 +282,10 @@ export class AdvisorRuntime {
 
 				let success = false;
 				try {
+					// Reset the host's per-update advisor state (one-advise-per-update
+					// gate) before each model cycle, so the new batch starts with a
+					// fresh budget. Dedupe history persists across cycles.
+					this.host.beginAdvisorUpdate?.();
 					await this.agent.prompt(batch);
 					success = true;
 					this.#consecutiveFailures = 0;

packages/coding-agent/src/session/agent-session.ts

@@ -130,6 +130,7 @@ import * as snapcompact from "@oh-my-pi/snapcompact";
 import {
 	AdviseTool,
 	type AdvisorAgent,
+	AdvisorEmissionGuard,
 	type AdvisorMessageDetails,
 	type AdvisorNote,
 	AdvisorRuntime,
@@ -1172,6 +1173,11 @@ export class AgentSession {
 	#advisorAutoResumeSuppressed = false;
 	#advisorPrimaryTurnsCompleted = 0;
 	#advisorInterruptImmuneTurnStart: number | undefined;
+	/** Dedupe + per-update rate-limit + content-free-phrase filter applied to
+	 *  every accepted advisor `advise()` call. Owned by the session because the
+	 *  session is what routes accepted notes back to the primary transcript.
+	 *  Reset on advisor reset (compaction, session switch, `/new`). */
+	readonly #advisorEmissionGuard = new AdvisorEmissionGuard();
 	#planModeState: PlanModeState | undefined;
 	#goalModeState: GoalModeState | undefined;
 	#goalRuntime: GoalRuntime;
@@ -1817,6 +1823,7 @@ export class AgentSession {
 		this.#advisorAgentUnsubscribe = undefined;
 		this.#advisorRuntime?.reset();
 		this.#advisorAdviseTool?.resetDeliveredNotes();
+		this.#advisorEmissionGuard.reset();
 		this.#attachAdvisorRecorderFeed();
 		this.#advisorPrimaryTurnsCompleted = 0;
 		this.#advisorInterruptImmuneTurnStart = undefined;
@@ -1856,7 +1863,17 @@ export class AgentSession {
 		// since steering an active run auto-resumes nothing; parking it there would
 		// strand the advice and dump the backlog as one burst at the next prompt. A
 		// plain nit always rides the non-interrupting YieldQueue aside.
+		// Apply the per-session emission policy (one-advise-per-update gate,
+		// exact-text dedupe, content-free phrase filter) before any routing.
+		// Suppression here means the advisor model called `advise()` but the call
+		// is dropped silently — the model still sees `Recorded.` from the tool, so
+		// telling it "suppressed" doesn't tempt it into rephrasing the same useless
+		// note to bypass the dedupe.
 		const enqueueAdvice = (note: string, severity?: AdvisorSeverity) => {
+			if (!this.#advisorEmissionGuard.accept(note)) {
+				logger.debug("advisor advice suppressed by emission guard", { severity });
+				return;
+			}
 			const interrupting = isInterruptingSeverity(severity);
 			const channel = resolveAdvisorDeliveryChannel({
 				severity,
@@ -1980,6 +1997,7 @@ export class AgentSession {
 			enqueueAdvice,
 			maintainContext: incomingTokens => this.#maintainAdvisorContext(incomingTokens),
 			obfuscator: this.#obfuscator,
+			beginAdvisorUpdate: () => this.#advisorEmissionGuard.beginUpdate(),
 		});
 		if (seedToCurrent) {
 			this.#advisorRuntime.seedTo(this.agent.state.messages.length);