fix(coding-agent/advisor): fixed advisor concern/blocker notes being stranded after interrupts

- Added resolveAdvisorDeliveryChannel in advisor tooling to map each note to aside, steer, or preserve using severity, auto-resume suppression, core-streaming, and abort state.
- Updated AgentSession advice enqueuing to route concern/blocker notes through that resolver, preserving them only when the interrupted turn is idle or tearing down and steering them during active resumed turns.
- Added regression tests for resolveAdvisorDeliveryChannel covering nit versus interrupting severities across streaming, aborting, and suppression combinations.
This commit is contained in:
can1357
2026-06-16 22:53:39 +02:00
parent d30bec5126
commit 3459371724
4 changed files with 159 additions and 25 deletions
+1
View File
@@ -29,6 +29,7 @@
- Fixed accepted IRC asides to be flushed into the transcript during disposal instead of being discarded
- Fixed interactive submissions made while the TUI had no active input waiter: they now start a real prompt directly, with steer fallback if a background turn races in, instead of queueing behind a non-resumable idle transcript and appearing to do nothing.
- Fixed pressing Esc (or Alt+Up dequeue) while agent-authored messages were queued — advisor concern/blocker notes, hidden goal/plan/budget steers, IRC/extension asides — dumping their text into the user's editor. Editor restoration (`clearQueue()`), pending chips (`getQueuedMessages()`), and `popLastQueuedMessage()` now surface only genuinely user-authored queued messages (plain user turns and `attribution: "user"` custom messages like `/skill`). Plain Alt+Up dequeue leaves all other queued messages in place for the continuing stream; only the Esc interrupt path keeps just advisor cards (so abort's preservation still re-records them as visible advice) and drops other internal steers, so a user interrupt can't be silently undone by an auto-resume on leftover internal context. `queuedMessageCount` still reflects all actual queued work (advisor cards included) so `hasPendingMessages()`/RPC and the empty-submit abort gate stay accurate.
- Fixed advisor `concern`/`blocker` advice being withheld from the running agent and then dumped as one burst at the next user prompt after a deliberate interrupt. A user interrupt latches advisor auto-resume suppression, but a non-user resume (synthetic/auto-continue, or a queued steer draining after the abort) leaves the run streaming with that latch still set, so every interrupting note was parked hidden in the next-turn queue instead of steered into the live turn — the agent never heard the advisor mid-run and the backlog flushed all at once on the next prompt. Suppression now only withholds interrupting advice while the agent is idle (or still tearing the interrupted turn down); once a turn is streaming again the note is steered in live, since steering an active run never auto-resumes a stopped one.
- Fixed `omp --continue`/`-c` sometimes resuming into a subagent transcript instead of the interactive session. Subagent (and HTML-export) `SessionManager.open()` calls run in the parent's terminal and were clobbering the per-TTY `--continue` breadcrumb with their own artifact-dir session file; these headless opens now suppress the breadcrumb. `continueRecent()` also recovers already-poisoned breadcrumbs by resolving any session file inside a parent's artifacts dir (`<parent>/<agentId>.jsonl`) back up to the top-level session.
## [16.0.2] - 2026-06-16
@@ -13,6 +13,7 @@ import {
type AdvisorRuntimeHost,
formatAdvisorBatchContent,
isInterruptingSeverity,
resolveAdvisorDeliveryChannel,
} from "..";
describe("advisor", () => {
@@ -639,4 +640,91 @@ describe("advisor", () => {
expect(text).toContain("truncated.");
});
});
// Regression: the advisor must not withhold interrupting advice from a turn
// that is actively streaming again after a user interrupt. The latch only
// guards auto-resume of a stopped/idle run; parking a note mid-stream stranded
// it (the agent never heard it) and dumped the backlog as one burst at the next
// user prompt. See the 7-concern same-instant burst in session 019ed1dd.
//
// `streaming` here means the live agent-CORE loop (agent.state.isStreaming) —
// NOT session `isStreaming`, which also counts `#promptInFlightCount` during
// post-turn unwind. Only a running core loop consumes a steer; in the unwind
// window (`streaming: false`) a suppressed note must `preserve`, never `steer`,
// or it strands and #drainStrandedQueuedMessages auto-resumes it. Do not swap
// the call site back to session `isStreaming`.
describe("resolveAdvisorDeliveryChannel", () => {
it("routes a non-interrupting nit to the aside queue regardless of state", () => {
expect(
resolveAdvisorDeliveryChannel({
severity: "nit",
autoResumeSuppressed: true,
streaming: true,
aborting: true,
}),
).toBe("aside");
expect(
resolveAdvisorDeliveryChannel({
severity: undefined,
autoResumeSuppressed: false,
streaming: false,
aborting: false,
}),
).toBe("aside");
});
it("steers concern/blocker when no user interrupt is in effect", () => {
for (const severity of ["concern", "blocker"] as const) {
for (const streaming of [true, false]) {
expect(
resolveAdvisorDeliveryChannel({
severity,
autoResumeSuppressed: false,
streaming,
aborting: false,
}),
).toBe("steer");
}
}
});
it("preserves an interrupting note while suppressed AND idle (no auto-resume of a stopped run)", () => {
for (const severity of ["concern", "blocker"] as const) {
expect(
resolveAdvisorDeliveryChannel({
severity,
autoResumeSuppressed: true,
streaming: false,
aborting: false,
}),
).toBe("preserve");
}
});
it("preserves an interrupting note while suppressed AND aborting, even though the turn still reports streaming", () => {
// Mid-abort teardown: steering would land after #extractQueuedAdvisorCards
// and could auto-resume on the stranded steer. Keep parking it.
expect(
resolveAdvisorDeliveryChannel({
severity: "blocker",
autoResumeSuppressed: true,
streaming: true,
aborting: true,
}),
).toBe("preserve");
});
it("steers an interrupting note while suppressed once a turn is streaming again and not aborting (the fix)", () => {
for (const severity of ["concern", "blocker"] as const) {
expect(
resolveAdvisorDeliveryChannel({
severity,
autoResumeSuppressed: true,
streaming: true,
aborting: false,
}),
).toBe("steer");
}
});
});
});
@@ -66,6 +66,36 @@ export function isInterruptingSeverity(severity: AdvisorSeverity | undefined): b
return severity === "concern" || severity === "blocker";
}
/** How an advisor note is routed to the primary. */
export type AdvisorDeliveryChannel = "aside" | "steer" | "preserve";
/**
* Decide how one advisor note reaches the primary agent.
*
* - A non-interrupting `nit` always rides the non-interrupting aside queue.
* - An interrupting `concern`/`blocker` is normally steered into the agent: into
* the live turn while one is streaming, or (when idle) a triggered turn so the
* advice is acted on immediately.
* - After a deliberate user interrupt (`autoResumeSuppressed`) the advisor must
* not auto-resume the stopped run. While the agent is idle — or still tearing
* the interrupted turn down (`aborting`) — the note is preserved as a visible
* card instead of restarting the run. But once a turn is actively streaming
* again (a resume the user already drove), steering the note in does NOT
* auto-resume anything, so it is delivered live. Parking it during an active
* run instead strands it (it never reaches the running agent) and the withheld
* notes dump as one burst at the next user prompt — the bug this guards.
*/
export function resolveAdvisorDeliveryChannel(opts: {
severity: AdvisorSeverity | undefined;
autoResumeSuppressed: boolean;
streaming: boolean;
aborting: boolean;
}): AdvisorDeliveryChannel {
if (!isInterruptingSeverity(opts.severity)) return "aside";
if (opts.autoResumeSuppressed && (opts.aborting || !opts.streaming)) return "preserve";
return "steer";
}
/**
* Side-effect-free investigation tools handed to the advisor agent so it can
* inspect the workspace before weighing in. Names match the primary session's
@@ -126,7 +126,7 @@ import {
AdvisorRuntime,
type AdvisorSeverity,
formatAdvisorBatchContent,
isInterruptingSeverity,
resolveAdvisorDeliveryChannel,
} from "../advisor";
import { type AsyncJob, type AsyncJobDeliveryState, AsyncJobManager } from "../async";
import { classifyDifficulty } from "../auto-thinking/classifier";
@@ -1617,33 +1617,48 @@ export class AgentSession {
// channel (aborting in-flight tools at the next steering boundary); when the
// loop has already yielded, triggerTurn resumes it so the advice is acted on
// immediately rather than waiting for the next user prompt. After a deliberate
// user interrupt that auto-resume is suppressed: the concern is recorded as
// visible advice and re-enters context only when the user resumes. A plain nit
// rides the non-interrupting YieldQueue aside.
// user interrupt the auto-resume is suppressed — but only while the agent is
// idle or still tearing the interrupted turn down: a concern is then recorded
// as a visible card and re-enters context when the user resumes. Once a turn
// is streaming again (a resume the user already drove) it is steered in live,
// 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.
const enqueueAdvice = (note: string, severity?: AdvisorSeverity) => {
if (isInterruptingSeverity(severity)) {
const notes: AdvisorNote[] = [{ note, severity }];
const content = formatAdvisorBatchContent(notes);
const details = { notes } satisfies AdvisorMessageDetails;
if (this.#advisorAutoResumeSuppressed) {
this.#preserveAdvisorCard({
role: "custom",
customType: "advisor",
content,
display: true,
attribution: "agent",
details,
timestamp: Date.now(),
});
return;
}
void this.sendCustomMessage(
{ customType: "advisor", content, display: true, attribution: "agent", details },
{ deliverAs: "steer", triggerTurn: true },
).catch(err => logger.debug("advisor delivery failed", { err: String(err) }));
const channel = resolveAdvisorDeliveryChannel({
severity,
autoResumeSuppressed: this.#advisorAutoResumeSuppressed,
// Key on the live agent-core loop, not session `isStreaming` (which also
// counts `#promptInFlightCount` during post-turn unwind). Only a running
// loop will consume a steer at its next boundary; steering into the unwind
// window would strand the card and let #drainStrandedQueuedMessages
// auto-resume it despite the user's interrupt.
streaming: this.agent.state.isStreaming,
aborting: this.#abortInProgress,
});
if (channel === "aside") {
this.yieldQueue.enqueue("advisor", { note, severity });
return;
}
this.yieldQueue.enqueue("advisor", { note, severity });
const notes: AdvisorNote[] = [{ note, severity }];
const content = formatAdvisorBatchContent(notes);
const details = { notes } satisfies AdvisorMessageDetails;
if (channel === "preserve") {
this.#preserveAdvisorCard({
role: "custom",
customType: "advisor",
content,
display: true,
attribution: "agent",
details,
timestamp: Date.now(),
});
return;
}
void this.sendCustomMessage(
{ customType: "advisor", content, display: true, attribution: "agent", details },
{ deliverAs: "steer", triggerTurn: true },
).catch(err => logger.debug("advisor delivery failed", { err: String(err) }));
};
const adviseTool = new AdviseTool(enqueueAdvice);