Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 58 additions & 14 deletions docs/advisor-watchdog.md

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,16 +420,18 @@ See [Models](./models.md) for the `models.yml` schema and custom-provider defini

### Advisor

The advisor is a second model that reviews each completed turn and can inject advice into the primary session. Assign a model with `modelRoles.advisor`, then enable it with `advisor.enabled`, `/advisor on`, or by launching with the `--advisor` flag.
Advisors are optional reviewer models with configurable cadence. Assign a model with `modelRoles.advisor` or a `WATCHDOG.yml` entry, then enable the subsystem with `advisor.enabled`, `/advisor on`, or the `--advisor` flag.

See [Advisor and WATCHDOG.md](./advisor-watchdog.md) for runtime behavior, `WATCHDOG.md` discovery, and bounded catch-up semantics.
See [Advisor and WATCHDOG.md](./advisor-watchdog.md) for runtime behavior, `WATCHDOG.md` discovery, cadence controls, and catch-up semantics.

| Key | Type | Default | Notes |
| --------------------- | ------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- |
| `advisor.enabled` | boolean | `false` | Enable the advisor runtime when `modelRoles.advisor` resolves to an available model. |
| `task.agentAdvisor` | record | `{}` | Per-agent subagent advisor: agent name → `"on"` / `"off"` / advisor model pattern. Overrides agent frontmatter `advisor`; configured from the `/agents` hub. |
| `advisor.syncBacklog` | enum | `off` | Bounded advisor catch-up delay: `off`, `1`, `3`, or `5`. The primary waits up to 30 seconds only while advisor backlog is at or above the threshold. |
| `advisor.immuneTurns` | number | `3` | After a `concern`/`blocker` interrupts, route further concerns/blockers as non-interrupting asides for this many completed primary turns. |
| `advisor.syncBacklog` | enum | `off` | Default catch-up policy. `off` never waits; `1`, `3`, or `5` wait up to 30 seconds at that backlog threshold; `strict` waits for scheduled reviews without a wall-clock cap. Abort, failure, quota pause, transition, and disposal release waits. Optional `WATCHDOG.yml` per-advisor `syncBacklog` overrides this policy; omission inherits it. |
| `advisor.immuneTurns` | number | `3` | After a concern or blocker interrupts, route further concerns as non-interrupting asides for this many primary turns, including tool-loop continuations. Blockers remain exempt. |
| `advisor.reviewMode` | enum | `turn` | Review every primary turn, or only final yields with `agent-end`. Per-advisor `reviewMode` overrides this default. |
| `advisor.reviewInterval` | number | `1` | Review every Nth eligible update. Skipped deltas accumulate; pending delivery never depends on cadence. |
| `advisor.maxNotesPerUpdate` | number | `4` | Non-blocker notes accepted per advisor review, from 1–32. Higher-severity notes can replace only pending notes from the same review. `WATCHDOG.yml` top-level or per-advisor values override this default. |

### Thinking
Expand Down
9 changes: 9 additions & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

## [Unreleased]

### Added

- Added global and per-advisor review cadence, including final-yield reviews and intervals that accumulate skipped transcript updates ([#12385](https://github.com/can1357/oh-my-pi/pull/12385) by [@olegpulatov](https://github.com/olegpulatov)).
- Added per-advisor catch-up policy and cancellable `strict` waiting, so asynchronous turn reviewers can run beside synchronous final reviewers ([#12385](https://github.com/can1357/oh-my-pi/pull/12385) by [@olegpulatov](https://github.com/olegpulatov)).

### Changed

- Advisor notes merge at final boundaries with age markers and at most one permitted continuation per batch; advisor continuations no longer trigger recursive reviews ([#12387](https://github.com/can1357/oh-my-pi/pull/12387) by [@olegpulatov](https://github.com/olegpulatov)).

## [18.2.8] - 2026-09-21

### Added
Expand Down
52 changes: 43 additions & 9 deletions packages/coding-agent/src/advisor/advise-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,16 @@ const ADVISOR_GUIDANCE = "weigh, don't blindly obey";
* non-interrupting YieldQueue dispatcher and the interrupting steer path so both
* build byte-identical content.
*/
export function formatAdvisorBatchContent(notes: readonly AdvisorNote[]): string {
export function formatAdvisorBatchContent(notes: readonly AdvisorNote[], opts?: { currentTurn?: number }): string {
return notes
.map(n => {
const severity = n.severity ? ` severity="${n.severity}"` : "";
const who = n.advisor ? ` advisor="${escapeXmlAttribute(n.advisor)}"` : "";
return `<advisory${who}${severity} guidance="${ADVISOR_GUIDANCE}">\n${escapeXmlText(n.note)}\n</advisory>`;
const age =
opts?.currentTurn !== undefined && n.turn !== undefined && opts.currentTurn > n.turn
? ` turns_ago="${opts.currentTurn - n.turn}"`
: "";
return `<advisory${who}${severity}${age} guidance="${ADVISOR_GUIDANCE}">\n${escapeXmlText(n.note)}\n</advisory>`;
})
.join("\n");
}
Expand Down Expand Up @@ -92,6 +96,9 @@ export function isAdvisorInterruptImmuneTurnActive(opts: {
* still steers a triggered turn to force the primary to acknowledge and continue
* before the turn is considered done (#5628) — deferring it to the next user
* turn is the bug.
* `allowTerminalConcernSteering` opts out of ONLY this preservation branch
* (a final-review continuation policy); stop/abort suppression, `preserveOnly`,
* and the immune-turn cooldown below still run first and are never bypassed.
* - 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
Expand All @@ -111,11 +118,21 @@ export function resolveAdvisorDeliveryChannel(opts: {
streaming: boolean;
aborting: boolean;
terminalAnswerNoQueuedWork?: boolean;
/** Opt out of terminal-answer preservation for a late `concern` (default
* false). Bypasses ONLY that branch — never stop/abort suppression,
* `preserveOnly`, or the immune-turn cooldown. */
allowTerminalConcernSteering?: boolean;
interruptImmuneTurnActive?: boolean;
preserveOnly?: boolean;
}): AdvisorDeliveryChannel {
if (opts.preserveOnly && !opts.streaming) return "preserve";
if (opts.terminalAnswerNoQueuedWork && opts.severity !== "blocker" && !opts.streaming && !opts.aborting)
if (
opts.terminalAnswerNoQueuedWork &&
opts.severity !== "blocker" &&
!opts.allowTerminalConcernSteering &&
!opts.streaming &&
!opts.aborting
)
return "preserve";
if (!isInterruptingSeverity(opts.severity)) return "aside";
if (opts.autoResumeSuppressed && (opts.aborting || !opts.streaming)) return "preserve";
Expand Down Expand Up @@ -163,6 +180,18 @@ function advisorSeverityRank(severity: AdvisorSeverity | undefined): number {
return ADVISOR_SEVERITY_RANK[severity ?? "nit"];
}

/**
* Merged-batch ordering: most recent turn first, then severity (blocker →
* concern → nit) within a turn. The newest notes describe the latest state of
* the work, so they read first; severity breaks ties so a blocker never hides
* below a same-turn nit.
*/
export function compareAdvisorNotes(a: AdvisorNote, b: AdvisorNote): number {
const turnDelta = (b.turn ?? 0) - (a.turn ?? 0);
if (turnDelta !== 0) return turnDelta;
return advisorSeverityRank(b.severity) - advisorSeverityRank(a.severity);
}

/** Admission acks: one line each — the advisor needs the verdict, not a policy essay. */
const ADVISOR_ACK_SENT = "Delivered.";
/** Held behind the in-progress primary turn; flushed when it completes. */
Expand All @@ -189,6 +218,9 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
*/
readonly #guard: AdvisorEmissionGuard;
#inProgressUpdate = false;
/** Primary-turn count the in-flight update reviews (stamped on emitted notes
* so merged batches can report how stale each review was at delivery). */
#coveredTurn: number | undefined;
/** Notes admitted but withheld while the primary was mid-turn, in arrival
* order. Flushed deterministically at the completed-update transition or an
* explicit {@link flushDeferredNotes}, so delivery does not depend on the
Expand All @@ -199,6 +231,7 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
key: string;
note: string;
severity?: AdviseDetails["severity"];
turn?: number;
}[] = [];

/**
Expand All @@ -211,7 +244,7 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
* {@link ADVISOR_DEFAULT_BUDGET_PER_UPDATE}).
*/
constructor(
private readonly onAdvice: (note: string, severity?: AdviseDetails["severity"]) => void,
private readonly onAdvice: (note: string, severity: AdviseDetails["severity"] | undefined, turn?: number) => void,
guard?: AdvisorEmissionGuard,
) {
this.#guard = guard ?? new AdvisorEmissionGuard();
Expand All @@ -226,9 +259,10 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
* was admitted when emitted, so the flush routes without re-admission and a
* backlog of one note per originating update reaches the primary intact.
*/
beginUpdate(inProgress: boolean): void {
beginUpdate(inProgress: boolean, coveredTurn?: number): void {
const wasInProgress = this.#inProgressUpdate;
this.#inProgressUpdate = inProgress;
this.#coveredTurn = coveredTurn;
this.#guard.beginUpdate();
if (wasInProgress && !inProgress) this.#flushDeferred();
}
Expand Down Expand Up @@ -286,7 +320,7 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
const displacedIndex = this.#deferredNotes.findIndex(item => item.key === decision.displacedKey);
if (displacedIndex !== -1) this.#deferredNotes.splice(displacedIndex, 1);
}
this.#deferredNotes.push({ key, note: args.note, severity: args.severity });
this.#deferredNotes.push({ key, note: args.note, severity: args.severity, turn: this.#coveredTurn });
return this.#result(ADVISOR_ACK_DEFERRED, args);
}
// Live path (completed update, or a blocker that must interrupt now). A
Expand All @@ -298,7 +332,7 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
if (reservedIndex !== -1) this.#deferredNotes.splice(reservedIndex, 1);
const decision = this.#guard.admit(args.note, { rank, pending: false });
if (!decision.accepted) return this.#suppressed(args, decision.reason);
this.onAdvice(args.note, args.severity);
this.onAdvice(args.note, args.severity, this.#coveredTurn);
return this.#result(ADVISOR_ACK_SENT, args);
}

Expand All @@ -309,9 +343,9 @@ export class AdviseTool implements AgentTool<typeof adviseSchema, AdviseDetails>
if (this.#deferredNotes.length === 0) return;
const pending = this.#deferredNotes;
this.#deferredNotes = [];
for (const { note, severity } of pending) {
for (const { note, severity, turn } of pending) {
this.#guard.markRouted(note);
this.onAdvice(note, severity);
this.onAdvice(note, severity, turn ?? this.#coveredTurn);
}
}

Expand Down
41 changes: 40 additions & 1 deletion packages/coding-agent/src/advisor/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,16 @@ import { expandAtImports } from "../discovery/at-imports";
import { BUILTIN_TOOL_NAMES, normalizeToolNames } from "../tools/builtin-names";
import { collectConfigCandidates } from "./watchdog";

import type { AdvisorConfig, AdvisorConfigScope, WatchdogConfigDoc } from "@oh-my-pi/pi-tui/overlays/advisor-config";
import {
ADVISOR_SYNC_BACKLOG_MODES,
type AdvisorConfig,
type AdvisorConfigScope,
type AdvisorSyncBacklog,
type WatchdogConfigDoc,
} from "@oh-my-pi/pi-tui/overlays/advisor-config";

export { ADVISOR_REVIEW_MODES, ADVISOR_SYNC_BACKLOG_MODES } from "@oh-my-pi/pi-tui/overlays/advisor-config";
export type { AdvisorReviewMode, AdvisorSyncBacklog } from "@oh-my-pi/pi-tui/overlays/advisor-config";

/**
* Runtime health of a single advisor, surfaced in stats and the status line.
Expand Down Expand Up @@ -37,10 +46,25 @@ export interface DiscoveredAdvisors {
warnings: string[];
}

const reviewIntervalSchema = type("1 <= number.integer <= 9007199254740991");
const syncBacklogSchema = type.enumerated(...ADVISOR_SYNC_BACKLOG_MODES);
/** Unquoted `syncBacklog: 3` parses as a number; accept the numeric thresholds alongside the string enum. */
const syncBacklogEntrySchema = type.enumerated(...ADVISOR_SYNC_BACKLOG_MODES, 1, 3, 5);
const SYNC_BACKLOG_NUMERIC_THRESHOLDS = { 1: "1", 3: "3", 5: "5" } as const;

function normalizeSyncBacklog(
value: AdvisorSyncBacklog | keyof typeof SYNC_BACKLOG_NUMERIC_THRESHOLDS,
): AdvisorSyncBacklog {
return typeof value === "number" ? SYNC_BACKLOG_NUMERIC_THRESHOLDS[value] : value;
}

const advisorEntrySchema = type({
name: "string",
"model?": "string",
"tools?": "string[]",
"reviewMode?": "'turn' | 'agent-end'",
"reviewInterval?": reviewIntervalSchema,
"syncBacklog?": syncBacklogEntrySchema,
"instructions?": "string",
"enabled?": "boolean",
"maxNotesPerUpdate?": "number",
Expand Down Expand Up @@ -212,6 +236,9 @@ export async function discoverAdvisorConfigs(cwd: string, agentDir?: string): Pr
name: entry.name,
model: entry.model?.trim() || undefined,
tools: filterAdvisorTools(entry.tools, item.path),
reviewMode: entry.reviewMode,
reviewInterval: entry.reviewInterval,
syncBacklog: entry.syncBacklog === undefined ? undefined : normalizeSyncBacklog(entry.syncBacklog),
maxNotesPerUpdate:
typeof entry.maxNotesPerUpdate === "number" &&
Number.isFinite(entry.maxNotesPerUpdate) &&
Expand Down Expand Up @@ -299,6 +326,9 @@ export async function loadWatchdogConfigFile(filePath: string): Promise<Watchdog
const advisor: AdvisorConfig = { name: a.name };
if (a.model?.trim()) advisor.model = a.model;
if (a.tools !== undefined) advisor.tools = [...a.tools];
if (a.reviewMode !== undefined) advisor.reviewMode = a.reviewMode;
if (a.reviewInterval !== undefined) advisor.reviewInterval = a.reviewInterval;
if (a.syncBacklog !== undefined) advisor.syncBacklog = normalizeSyncBacklog(a.syncBacklog);
if (a.instructions?.trim()) advisor.instructions = a.instructions;
if (a.enabled !== undefined) advisor.enabled = a.enabled;
if (typeof a.maxNotesPerUpdate === "number" && Number.isFinite(a.maxNotesPerUpdate) && a.maxNotesPerUpdate >= 1) {
Expand Down Expand Up @@ -367,6 +397,11 @@ export function serializeWatchdogConfig(doc: WatchdogConfigDoc): string {
}
}
}
if (advisor.reviewMode !== undefined) lines.push(` reviewMode: ${YAML.stringify(advisor.reviewMode)}`);
if (advisor.syncBacklog !== undefined) {
syncBacklogSchema.assert(advisor.syncBacklog);
lines.push(` syncBacklog: ${YAML.stringify(advisor.syncBacklog)}`);
}
if (advisor.instructions?.trim()) {
appendYamlString(lines, " ", "instructions", advisor.instructions);
}
Expand All @@ -378,6 +413,10 @@ export function serializeWatchdogConfig(doc: WatchdogConfigDoc): string {
) {
lines.push(` maxNotesPerUpdate: ${Math.trunc(advisor.maxNotesPerUpdate)}`);
}
if (advisor.reviewInterval !== undefined) {
reviewIntervalSchema.assert(advisor.reviewInterval);
lines.push(` reviewInterval: ${advisor.reviewInterval}`);
}
}
}
return lines.length === 0 ? "" : `${lines.join("\n")}\n`;
Expand Down
22 changes: 12 additions & 10 deletions packages/coding-agent/src/advisor/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,35 +410,37 @@ export class AdvisorRuntime {
* Wait until the advisor backlog falls below `threshold`.
*
* Returns `false` when the deadline, abort signal, or a runtime failure releases
* the waiter before the requested backlog was drained.
* the waiter before the requested backlog was drained. An omitted `maxMs` waits
* without a wall-clock deadline; abort, failure, and disposal still release it.
*/
waitForCatchup(maxMs: number, threshold: number, signal?: AbortSignal): Promise<boolean> {
waitForCatchup(maxMs: number | undefined, threshold: number, signal?: AbortSignal): Promise<boolean> {
if (
this.disposed ||
signal?.aborted ||
this.#backlog < threshold ||
this.#quotaExhausted ||
this.#halted ||
this.#sessionTransitionPaused ||
// An advisor mid-failure/retry must NEVER gate the primary agent:
// its backlog cannot drain until the retry cycle resolves, and the
// primary would otherwise park for the full catch-up budget.
// its backlog cannot drain until the retry cycle resolves, and
// the primary would otherwise park for the full catch-up budget.
this.#failing
)
return Promise.resolve(this.#backlog < threshold);
const { promise, resolve } = Promise.withResolvers<boolean>();
const finish = (caughtUp: boolean): void => {
const idx = this.#waiters.indexOf(waiter);
if (idx >= 0) this.#waiters.splice(idx, 1);
clearTimeout(waiter.timer);
if (waiter.timer !== undefined) {
clearTimeout(waiter.timer);
waiter.timer = undefined;
}
signal?.removeEventListener("abort", abort);
resolve(caughtUp);
};
const abort = (): void => finish(false);
const waiter = {
threshold,
finish,
timer: setTimeout(abort, maxMs),
};
const waiter: CatchupWaiter = { threshold, finish };
if (maxMs !== undefined) waiter.timer = setTimeout(abort, maxMs);
this.#waiters.push(waiter);
signal?.addEventListener("abort", abort, { once: true });
if (signal?.aborted) {
Expand Down
Loading
Loading