diff --git a/.changeset/sdk-advisor-api.md b/.changeset/sdk-advisor-api.md new file mode 100644 index 00000000..8f41c041 --- /dev/null +++ b/.changeset/sdk-advisor-api.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code-sdk": minor +--- + +Add advisor status and control methods to the session client. diff --git a/.changeset/session-advisor.md b/.changeset/session-advisor.md new file mode 100644 index 00000000..97fee51a --- /dev/null +++ b/.changeset/session-advisor.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": minor +--- + +Add a session advisor that reviews work in the background, with the /advisor command to show and control it. diff --git a/.changeset/thinking-activity-pane.md b/.changeset/thinking-activity-pane.md new file mode 100644 index 00000000..aac17693 --- /dev/null +++ b/.changeset/thinking-activity-pane.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Stream in-progress thinking in the activity pane and move it into the transcript when complete. diff --git a/.changeset/workflow-spinner.md b/.changeset/workflow-spinner.md new file mode 100644 index 00000000..044ec554 --- /dev/null +++ b/.changeset/workflow-spinner.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Show running dynamic workflow rows with the same braille spinner glyphs as other loaders. diff --git a/apps/pythinker-code/src/tui/commands/advisor.ts b/apps/pythinker-code/src/tui/commands/advisor.ts new file mode 100644 index 00000000..4c5ecbd4 --- /dev/null +++ b/apps/pythinker-code/src/tui/commands/advisor.ts @@ -0,0 +1,90 @@ +import { formatErrorMessage, type AdvisorStatusSnapshot } from '@pymodel/pythinker-code-sdk'; +import type { SlashCommandHost } from './dispatch'; + +const ADVISOR_STATUS_GLYPHS: Record = { + running: '●', + paused: '○', + no_model: '○', + quota_exhausted: '✕', + error: '✕', +}; + +const ADVISOR_STATUS_LABELS: Record = { + running: 'running', + paused: 'off', + no_model: 'no model', + quota_exhausted: 'quota exhausted', + error: 'error', +}; + +export async function handleAdvisorCommand(host: SlashCommandHost, args: string): Promise { + const parts = args.trim().split(/\s+/u).filter(Boolean); + const verb = parts[0] ?? 'status'; + const advisorId = parts[1]; + if (parts.length > 2 || !['on', 'off', 'reload', 'status', 'toggle'].includes(verb)) { + host.showError('Usage: /advisor [on|off|status|reload|toggle] [advisor-id]'); + return; + } + if (host.session === undefined) { + host.showError('No active session.'); + return; + } + + if (verb === 'status') { + host.showNotice('Advisor status', formatAdvisorStatuses(await host.session.advisor.status())); + return; + } + if (verb === 'reload') { + await host.session.advisor.reload(); + host.showStatus('Advisor configuration reloaded.'); + return; + } + + const statuses = await host.session.advisor.status(); + if (advisorId !== undefined && !statuses.some((status) => status.id === advisorId)) { + host.showError( + `Unknown advisor: ${advisorId}. Run /advisor status to list configured advisors.`, + ); + return; + } + const enabled = + verb === 'toggle' + ? !( + statuses.find((status) => + advisorId === undefined ? true : status.id === advisorId, + )?.enabled ?? false + ) + : verb === 'on'; + let updatedStatuses: readonly AdvisorStatusSnapshot[]; + try { + updatedStatuses = await host.session.advisor.setEnabled(enabled, advisorId); + } catch (error) { + host.showError(formatErrorMessage(error)); + return; + } + const target = advisorId === undefined ? 'Advisor' : `Advisor ${advisorId}`; + const applied = + advisorId === undefined + ? updatedStatuses.length > 0 && + updatedStatuses.every((status) => status.enabled === enabled) + : updatedStatuses.find((status) => status.id === advisorId)?.enabled === enabled; + if (!applied) { + host.showError(`${target} remains ${enabled ? 'disabled' : 'enabled'}.`); + return; + } + host.showStatus(`${target} ${enabled ? 'enabled' : 'disabled'}.`); +} + +function formatAdvisorStatuses(statuses: readonly AdvisorStatusSnapshot[]): string { + if (statuses.length === 0) return 'Advisor is disabled.'; + return statuses + .map((advisor) => { + const glyph = ADVISOR_STATUS_GLYPHS[advisor.status] ?? '?'; + const label = ADVISOR_STATUS_LABELS[advisor.status] ?? advisor.status; + const model = advisor.model === undefined ? '' : `\n Model: ${advisor.model}`; + const details = `\n ${advisor.notes} notes · $${advisor.costUsd.toFixed(4)} · ${advisor.failures} failures`; + const message = advisor.message === undefined ? '' : `\n ${advisor.message}`; + return `${glyph} ${advisor.name} [${label}]${advisor.enabled ? '' : ' (disabled)'}${model}${details}${message}`; + }) + .join('\n\n'); +} diff --git a/apps/pythinker-code/src/tui/commands/diff.ts b/apps/pythinker-code/src/tui/commands/diff.ts index a8cdd628..1986f630 100644 --- a/apps/pythinker-code/src/tui/commands/diff.ts +++ b/apps/pythinker-code/src/tui/commands/diff.ts @@ -117,7 +117,10 @@ async function showWorkingTreeFileDiff( 'primary', ' Diff ', ); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } catch (error) { host.showError(`Failed to load diff for ${path}: ${formatErrorMessage(error)}`); diff --git a/apps/pythinker-code/src/tui/commands/dispatch.ts b/apps/pythinker-code/src/tui/commands/dispatch.ts index beb418eb..ec03a865 100644 --- a/apps/pythinker-code/src/tui/commands/dispatch.ts +++ b/apps/pythinker-code/src/tui/commands/dispatch.ts @@ -19,6 +19,7 @@ import type { TranscriptEntry, } from '../types'; import { formatErrorMessage } from '../utils/event-payload'; +import { handleAdvisorCommand } from './advisor'; import { handleAddDirCommand } from './add-dir'; import { handleAgentsCommand } from './agents'; import { handleLoginCommand, handleLogoutCommand } from './auth'; @@ -388,6 +389,9 @@ async function handleBuiltInSlashCommand( case 'fast': await handleFastCommand(host, args); return; + case 'advisor': + await handleAdvisorCommand(host, args); + return; case 'provider': await handleProviderCommand(host); return; diff --git a/apps/pythinker-code/src/tui/commands/dynamic-workflow.ts b/apps/pythinker-code/src/tui/commands/dynamic-workflow.ts index 260d3190..35dce5d7 100644 --- a/apps/pythinker-code/src/tui/commands/dynamic-workflow.ts +++ b/apps/pythinker-code/src/tui/commands/dynamic-workflow.ts @@ -286,8 +286,9 @@ function dynamicWorkflowModeSubcommand(input: string): boolean | undefined { } function renderDynamicWorkflowModeMarker(host: SlashCommandHost, state: DynamicWorkflowModeMarkerState): void { - host.state.transcriptContainer.addChild( + host.state.transcriptContainer.addTranscriptChild( new DynamicWorkflowModeMarkerComponent(state), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, ); host.state.ui.requestRender(); } diff --git a/apps/pythinker-code/src/tui/commands/goal.ts b/apps/pythinker-code/src/tui/commands/goal.ts index 0eaa208e..8dce038c 100644 --- a/apps/pythinker-code/src/tui/commands/goal.ts +++ b/apps/pythinker-code/src/tui/commands/goal.ts @@ -205,8 +205,9 @@ async function queueNextGoal( } host.track('goal_queue_append'); if (!hasCurrentGoal) host.requestQueuedGoalPromotion?.(); - host.state.transcriptContainer.addChild( + host.state.transcriptContainer.addTranscriptChild( new UpcomingGoalAddedMessageComponent(), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, ); host.state.ui.requestRender(); } @@ -413,7 +414,10 @@ async function startGoal( return false; } host.track('goal_create', { replace: parsed.replace }); - host.state.transcriptContainer.addChild(new GoalSetMessageComponent()); + host.state.transcriptContainer.addTranscriptChild(new GoalSetMessageComponent(), { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); if (options.sendInput !== undefined) { options.sendInput(parsed.objective); @@ -484,9 +488,10 @@ async function showGoalStatus(host: SlashCommandHost): Promise { host.showStatus('No goal set. Start one with `/goal `.'); return; } - host.state.transcriptContainer.addChild( - new GoalStatusMessageComponent(goal), - ); + host.state.transcriptContainer.addTranscriptChild(new GoalStatusMessageComponent(goal), { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } diff --git a/apps/pythinker-code/src/tui/commands/index.ts b/apps/pythinker-code/src/tui/commands/index.ts index 5294672d..d9870a56 100644 --- a/apps/pythinker-code/src/tui/commands/index.ts +++ b/apps/pythinker-code/src/tui/commands/index.ts @@ -30,6 +30,7 @@ export { export { handleCopyCommand, showMessageActions } from './copy'; export { handleDebugCommand } from './debug'; export { buildWorkingTreeDiffLines, handleDiffCommand } from './diff'; +export { handleAdvisorCommand } from './advisor'; export { handleDynamicWorkflowCommand } from './dynamic-workflow'; export { handleFastCommand } from './fast'; export { diff --git a/apps/pythinker-code/src/tui/commands/info.ts b/apps/pythinker-code/src/tui/commands/info.ts index 695238f4..2bbfe3d4 100644 --- a/apps/pythinker-code/src/tui/commands/info.ts +++ b/apps/pythinker-code/src/tui/commands/info.ts @@ -56,7 +56,10 @@ export function showCost(host: SlashCommandHost): void { 'primary', ' Cost ', ); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } @@ -70,7 +73,10 @@ export async function showUsage(host: SlashCommandHost): Promise { maxContextTokens: host.state.appState.maxContextTokens, }; const panel = new UsagePanelComponent(() => buildUsageReportLines(reportArgs), 'primary'); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } @@ -110,7 +116,10 @@ export async function showContextReport( 'primary', ' Context ', ); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } catch (error) { host.showError(`Failed to load context usage: ${formatErrorMessage(error)}`); @@ -139,7 +148,10 @@ export async function showStatusReport(host: SlashCommandHost): Promise { statusError: runtimeStatus.error, }; const panel = new UsagePanelComponent(() => buildStatusReportLines(reportArgs), 'primary', ' Status '); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } @@ -158,7 +170,10 @@ export async function showMcpServers(host: SlashCommandHost): Promise { 'primary', title, ); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } diff --git a/apps/pythinker-code/src/tui/commands/plugins.ts b/apps/pythinker-code/src/tui/commands/plugins.ts index 6ea7ed67..f7a9e7fb 100644 --- a/apps/pythinker-code/src/tui/commands/plugins.ts +++ b/apps/pythinker-code/src/tui/commands/plugins.ts @@ -468,7 +468,10 @@ async function renderPluginsList( 'primary', title, ); - host.state.transcriptContainer.addChild(panel); + host.state.transcriptContainer.addTranscriptChild(panel, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); host.state.ui.requestRender(); } @@ -479,7 +482,10 @@ async function renderPluginInfo(host: SlashCommandHost, id: string): Promise args.trim().toLowerCase() === 'status' ? 'always' : 'idle-only', }, + { + name: 'advisor', + aliases: [], + description: 'Show or control the second-opinion advisor', + priority: 95, + completeArgs: advisorArgumentCompletions, + availability: (args) => { + const verb = args.trim().toLowerCase(); + return verb === '' || verb === 'status' ? 'always' : 'idle-only'; + }, + }, { name: 'provider', aliases: ['providers'], diff --git a/apps/pythinker-code/src/tui/commands/undo.ts b/apps/pythinker-code/src/tui/commands/undo.ts index 2c66b969..f036519d 100644 --- a/apps/pythinker-code/src/tui/commands/undo.ts +++ b/apps/pythinker-code/src/tui/commands/undo.ts @@ -702,9 +702,10 @@ function renderWelcome(host: SlashCommandHost): void { ) { return; } - host.state.transcriptContainer.addChild( + host.state.transcriptContainer.addTranscriptChild( new WelcomeComponent(host.state.appState, () => { host.state.ui.requestRender(); }), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, ); } diff --git a/apps/pythinker-code/src/tui/components/chrome/transcript-container.ts b/apps/pythinker-code/src/tui/components/chrome/transcript-container.ts new file mode 100644 index 00000000..d72e78ed --- /dev/null +++ b/apps/pythinker-code/src/tui/components/chrome/transcript-container.ts @@ -0,0 +1,146 @@ +import type { Component } from '@earendil-works/pi-tui'; + +import { GutterContainer } from './gutter-container'; +import { + getTranscriptChildMetadata, + type TranscriptChildMetadata, + type TranscriptChildRole, + markTranscriptChild, +} from '../../utils/transcript-component-metadata'; + +export type { TranscriptChildMetadata, TranscriptChildRole } from '../../utils/transcript-component-metadata'; + +export class TranscriptContainer extends GutterContainer { + private readonly leftGutter: number; + private readonly rightGutter: number; + private renderedRowsAfterChildDepth = 0; + + constructor(leftPad: number, rightPad: number) { + super(leftPad, rightPad); + this.leftGutter = leftPad; + this.rightGutter = rightPad; + } + + addTranscriptChild(child: Component, metadata: TranscriptChildMetadata): void { + markTranscriptChild(child, metadata); + super.addChild(child); + } + + addTranscriptChildAt( + index: number, + child: Component, + metadata: TranscriptChildMetadata, + ): void { + markTranscriptChild(child, metadata); + this.children.splice(Math.max(0, Math.min(index, this.children.length)), 0, child); + this.invalidate(); + } + replaceTranscriptChild( + current: Component, + next: Component, + metadata: TranscriptChildMetadata, + ): void { + const index = this.children.indexOf(current); + if (index < 0) { + this.addTranscriptChild(next, metadata); + return; + } + markTranscriptChild(next, metadata); + this.children[index] = next; + this.invalidate(); + } + override addChild(_child: Component): void { + throw new Error('TranscriptContainer requires addTranscriptChild() metadata'); + } + + renderedRowsAfterChild(width: number, child: Component): number { + const index = this.children.indexOf(child); + if (index < 0) return 0; + const metadata = getTranscriptChildMetadata(child); + if (metadata === undefined) { + throw new Error('Transcript child was added without metadata'); + } + const inner = Math.max(1, width - this.leftGutter - this.rightGutter); + const nestedRender = this.renderedRowsAfterChildDepth > 0; + this.renderedRowsAfterChildDepth += 1; + try { + let rows = 0; + let previousDurable = nestedRender ? isDurable(metadata.role) : false; + if (!nestedRender) { + for (let previousIndex = index; previousIndex >= 0; previousIndex -= 1) { + const previousChild = this.children[previousIndex]!; + const previousMetadata = getTranscriptChildMetadata(previousChild); + if (previousMetadata === undefined) { + throw new Error('Transcript child was added without metadata'); + } + const previousRows = this.normalizeRows( + previousChild.render(inner), + previousMetadata, + ); + if (previousRows.length === 0) continue; + previousDurable = isDurable(previousMetadata.role); + break; + } + } + for (let childIndex = index + 1; childIndex < this.children.length; childIndex += 1) { + const followingChild = this.children[childIndex]!; + const followingMetadata = getTranscriptChildMetadata(followingChild); + if (followingMetadata === undefined) { + throw new Error('Transcript child was added without metadata'); + } + const followingRows = this.normalizeRows( + followingChild.render(inner), + followingMetadata, + ); + if (followingRows.length === 0) continue; + if (previousDurable && isDurable(followingMetadata.role)) rows += 1; + rows += followingRows.length; + previousDurable = isDurable(followingMetadata.role); + } + return rows; + } finally { + this.renderedRowsAfterChildDepth -= 1; + } + } + + override render(width: number): string[] { + const inner = Math.max(1, width - this.leftGutter - this.rightGutter); + const lead = ' '.repeat(this.leftGutter); + const rows: string[] = []; + let hasVisible = false; + let previousDurable = false; + for (const child of this.children) { + const metadata = getTranscriptChildMetadata(child); + if (metadata === undefined) { + throw new Error('Transcript child was added without metadata'); + } + const childRows = this.normalizeRows(child.render(inner), metadata); + if (childRows.length === 0) continue; + if (hasVisible && previousDurable && isDurable(metadata.role)) rows.push(lead); + for (const row of childRows) rows.push(lead + row); + hasVisible = true; + previousDurable = isDurable(metadata.role); + } + return rows; + } + + private normalizeRows( + rows: readonly string[], + metadata: TranscriptChildMetadata, + ): readonly string[] { + if (metadata.edgeBlankPolicy === 'preserve') return rows; + let start = 0; + let end = rows.length; + while (start < end && isPlainBlank(rows[start]!)) start += 1; + while (end > start && isPlainBlank(rows[end - 1]!)) end -= 1; + return rows.slice(start, end); + } +} + +function isPlainBlank(value: string): boolean { + return /^[ ]*$/u.test(value); +} + +function isDurable(role: TranscriptChildRole): boolean { + return role === 'durable' || role === 'live-durable'; +} diff --git a/apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts b/apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts index 5199508c..f5392a5e 100644 --- a/apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts +++ b/apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts @@ -109,9 +109,10 @@ const PHASE_LABELS: Record = { cancelled: 'STOP', }; -const PHASE_GLYPHS: Record, string> = { +const PHASE_GLYPHS: Record = { pending: '○', queued: '○', + running: BRAILLE_SPINNER_FRAMES[0] ?? '⠋', suspended: '◑', completed: '✓', failed: '×', @@ -1136,7 +1137,7 @@ function commonPrefixLength(left: string, right: string, limit: number): number function renderProgressGlyph(phase: DynamicWorkflowPhase, frame: number): string { const glyph = phase === 'running' ? DYNAMIC_WORKFLOW_RENDERING.progressFrames[frame % DYNAMIC_WORKFLOW_RENDERING.progressFrames.length] ?? - DYNAMIC_WORKFLOW_RENDERING.progressFrames[0] + PHASE_GLYPHS.running : PHASE_GLYPHS[phase]; return currentTheme.fg(PHASE_COLORS[phase], glyph); } diff --git a/apps/pythinker-code/src/tui/constant/rendering.ts b/apps/pythinker-code/src/tui/constant/rendering.ts index b2abe4f9..fc447bf2 100644 --- a/apps/pythinker-code/src/tui/constant/rendering.ts +++ b/apps/pythinker-code/src/tui/constant/rendering.ts @@ -30,8 +30,8 @@ export const DYNAMIC_WORKFLOW_RENDERING = { memberProgressWidth: 8, /** Least width of the lifecycle STATE column in member rows. */ stateColumnWidth: 6, - /** Half-circle frames for a running row; all rows share one clock. */ - progressFrames: ['◐', '◓', '◑', '◒'], + /** Braille frames for a running row; all rows share one clock. */ + progressFrames: BRAILLE_SPINNER_FRAMES, /** Rotation cadence in milliseconds — deliberately slow; this is ambience, not progress. */ progressFrameMs: 300, /** Least room the task keeps before the detail may claim any of the row. */ diff --git a/apps/pythinker-code/src/tui/controllers/session-event-handler.ts b/apps/pythinker-code/src/tui/controllers/session-event-handler.ts index b6fd0744..e77f7875 100644 --- a/apps/pythinker-code/src/tui/controllers/session-event-handler.ts +++ b/apps/pythinker-code/src/tui/controllers/session-event-handler.ts @@ -1,5 +1,6 @@ import type { Component, Focusable } from '@earendil-works/pi-tui'; import type { + AdvisorStatusEvent, AgentStatusUpdatedEvent, AssistantDeltaEvent, BackgroundTaskInfo, @@ -173,10 +174,10 @@ export class SessionEventHandler { this.subAgentEventHandler.resetRuntimeState(); this.renderedSkillActivationIds.clear(); this.renderedMcpServerStatusKeys.clear(); - this.mcpServers.clear(); this.mcpServerSnapshotReady = false; this.mcpServerSnapshotEpoch += 1; this.mcpLiveServerNames.clear(); + this.mcpServers.clear(); this.goalCompletionAwaitingClear = false; this.goalCompletionTurnEnded = false; this.currentTurnHasAssistantText = false; @@ -297,6 +298,7 @@ export class SessionEventHandler { case 'tool.call.delta': this.handleToolCallDelta(event); break; case 'tool.result': this.handleToolResult(event); break; case 'agent.status.updated': this.handleStatusUpdate(event); break; + case 'advisor.status': this.handleAdvisorStatus(event); break; case 'session.meta.updated': this.handleSessionMetaChanged(event); break; case 'goal.updated': this.handleGoalUpdated(event); break; case 'skill.activated': this.handleSkillActivated(event); break; @@ -345,7 +347,10 @@ export class SessionEventHandler { } const tint = (text: string): string => currentTheme.fg('textMuted', text); const spinner = new ActivityLoader(state.ui, tint, event.content); - state.transcriptContainer.addChild(spinner); + state.transcriptContainer.addTranscriptChild(spinner, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); this.hookStatusSpinners.set(event.statusId, spinner); state.ui.requestRender(); } @@ -789,8 +794,9 @@ export class SessionEventHandler { } private renderDynamicWorkflowModeMarker(state: DynamicWorkflowModeMarkerState): void { - this.host.state.transcriptContainer.addChild( + this.host.state.transcriptContainer.addTranscriptChild( new DynamicWorkflowModeMarkerComponent(state), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, ); this.host.state.ui.requestRender(); } @@ -843,7 +849,10 @@ export class SessionEventHandler { } const marker = buildGoalMarker(change, state.toolOutputExpanded, change.actor); if (marker !== null) { - state.transcriptContainer.addChild(marker); + state.transcriptContainer.addTranscriptChild(marker, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); state.ui.requestRender(); } } @@ -855,7 +864,10 @@ export class SessionEventHandler { const { state } = this.host; const marker = buildGoalMarker(change, state.toolOutputExpanded, 'model'); if (marker !== null) { - state.transcriptContainer.addChild(marker); + state.transcriptContainer.addTranscriptChild(marker, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); state.ui.requestRender(); } } @@ -1033,6 +1045,16 @@ export class SessionEventHandler { private handleSessionWarning(event: WarningEvent): void { this.host.showStatus(`Warning: ${event.message}`, 'warning'); } + private handleAdvisorStatus(event: AdvisorStatusEvent): void { + const color: ColorToken = + event.status === 'error' || event.status === 'quota_exhausted' + ? 'error' + : event.status === 'running' + ? 'success' + : 'warning'; + const message = event.message === undefined ? '' : ` · ${event.message}`; + this.host.showStatus(`Advisor ${event.name}: ${event.status}${message}`, color); + } private renderMcpServerStatus(server: McpServerStatusSnapshot): void { const key = mcpServerStatusKey(server); diff --git a/apps/pythinker-code/src/tui/controllers/streaming-ui.ts b/apps/pythinker-code/src/tui/controllers/streaming-ui.ts index 5e39e401..27c5ffb1 100644 --- a/apps/pythinker-code/src/tui/controllers/streaming-ui.ts +++ b/apps/pythinker-code/src/tui/controllers/streaming-ui.ts @@ -1,3 +1,4 @@ +import type { Component } from '@earendil-works/pi-tui'; import type { Session } from '@pymodel/pythinker-code-sdk'; import { AgentGroupComponent } from '../components/messages/agent-group'; @@ -12,6 +13,7 @@ import { appendStreamingArgsPreview, parseStreamingArgs } from '../utils/event-p import { notifyTerminalOnce } from '../utils/terminal-notification'; import { nextTranscriptId } from '../utils/transcript-id'; import { ScrollbackBridge } from '../runtime/scrollback/scrollback-bridge'; +import { markTranscriptComponent } from '../utils/transcript-component-metadata'; import type { TodoItem } from '../components/chrome/todo-panel'; import type { AppState, @@ -82,9 +84,14 @@ export class StreamingUIController { solo?: ToolCallComponent; group?: ReadGroupComponent; } | null = null; - constructor(private readonly host: StreamingUIHost) {} + private addLiveTranscriptChild(child: Component): void { + this.host.state.transcriptContainer.addTranscriptChild(child, { + role: 'live-durable', + edgeBlankPolicy: 'trim-plain', + }); + } // --------------------------------------------------------------------------- // Turn context — read/write accessors // --------------------------------------------------------------------------- @@ -374,10 +381,11 @@ export class StreamingUIController { // --------------------------------------------------------------------------- disposeActiveThinkingComponent(): void { - if (this._activeThinkingComponent !== undefined) { - this._activeThinkingComponent.dispose(); - this._activeThinkingComponent = undefined; - } + const component = this._activeThinkingComponent; + if (component === undefined) return; + this.host.state.activityContainer.removeChild(component); + component.dispose(); + this._activeThinkingComponent = undefined; } disposeAndClearPendingToolComponents(): void { @@ -571,10 +579,11 @@ export class StreamingUIController { content: '', }; const component = new AssistantMessageComponent(); + markTranscriptComponent(component, entry); this._streamingBlock = { component, entry }; this.scrollback?.begin(entry.id, this._currentTurnId); this.host.pushTranscriptEntry(entry); - state.transcriptContainer.addChild(component); + this.addLiveTranscriptChild(component); state.ui.requestRender(); } @@ -609,7 +618,7 @@ export class StreamingUIController { state.ui, ); if (state.toolOutputExpanded) this._activeThinkingComponent.setExpanded(true); - state.transcriptContainer.addChild(this._activeThinkingComponent); + state.activityContainer.addChild(this._activeThinkingComponent); this._thinkingEntryId = nextTranscriptId(); this.scrollback?.begin(this._thinkingEntryId, this._currentTurnId); } else { @@ -622,8 +631,11 @@ export class StreamingUIController { } onThinkingEnd(): void { - if (this._activeThinkingComponent === undefined) return; - this._activeThinkingComponent.finalize(); + const component = this._activeThinkingComponent; + if (component === undefined) return; + component.finalize(); + this.host.state.activityContainer.removeChild(component); + this.addLiveTranscriptChild(component); this._activeThinkingComponent = undefined; if (this._thinkingEntryId !== undefined) { this.scrollback?.complete(this._thinkingEntryId); @@ -651,7 +663,7 @@ export class StreamingUIController { let handled = this.tryAttachAgentToolCall(toolCall, tc); if (!handled) handled = this.tryAttachReadToolCall(toolCall, tc); if (!handled) { - state.transcriptContainer.addChild(tc); + this.addLiveTranscriptChild(tc); state.ui.requestRender(); } @@ -687,7 +699,7 @@ export class StreamingUIController { state.appState.workDir, ); if (state.toolOutputExpanded) completed.setExpanded(true); - state.transcriptContainer.addChild(completed); + this.addLiveTranscriptChild(completed); state.ui.requestRender(); } } @@ -710,7 +722,7 @@ export class StreamingUIController { } const block = new CompactionComponent(state.ui, instruction); this._activeCompactionBlock = block; - state.transcriptContainer.addChild(block); + this.addLiveTranscriptChild(block); state.ui.requestRender(); } @@ -780,7 +792,7 @@ export class StreamingUIController { const cur = this._pendingAgentGroup; if (cur === null) { this._pendingAgentGroup = { step, turnId, solo: tc }; - state.transcriptContainer.addChild(tc); + this.addLiveTranscriptChild(tc); state.ui.requestRender(); return true; } @@ -793,7 +805,7 @@ export class StreamingUIController { const solo = cur.solo; if (solo === undefined) { this._pendingAgentGroup = { step, turnId, solo: tc }; - state.transcriptContainer.addChild(tc); + this.addLiveTranscriptChild(tc); state.ui.requestRender(); return true; } @@ -810,9 +822,12 @@ export class StreamingUIController { const children = state.transcriptContainer.children; const idx = children.indexOf(solo); if (idx >= 0) { - children[idx] = group; + state.transcriptContainer.replaceTranscriptChild(solo, group, { + role: 'live-durable', + edgeBlankPolicy: 'trim-plain', + }); } else { - state.transcriptContainer.addChild(group); + this.addLiveTranscriptChild(group); } group.attach(solo.toolCallView.id, solo); return group; @@ -836,7 +851,7 @@ export class StreamingUIController { const cur = this._pendingReadGroup; if (cur === null) { this._pendingReadGroup = { step, turnId, solo: tc }; - state.transcriptContainer.addChild(tc); + this.addLiveTranscriptChild(tc); state.ui.requestRender(); return true; } @@ -849,7 +864,7 @@ export class StreamingUIController { const solo = cur.solo; if (solo === undefined) { this._pendingReadGroup = { step, turnId, solo: tc }; - state.transcriptContainer.addChild(tc); + this.addLiveTranscriptChild(tc); state.ui.requestRender(); return true; } @@ -866,9 +881,12 @@ export class StreamingUIController { const children = state.transcriptContainer.children; const idx = children.indexOf(solo); if (idx >= 0) { - children[idx] = group; + state.transcriptContainer.replaceTranscriptChild(solo, group, { + role: 'live-durable', + edgeBlankPolicy: 'trim-plain', + }); } else { - state.transcriptContainer.addChild(group); + this.addLiveTranscriptChild(group); } group.attach(solo.toolCallView.id, solo); return group; diff --git a/apps/pythinker-code/src/tui/controllers/subagent-event-handler.ts b/apps/pythinker-code/src/tui/controllers/subagent-event-handler.ts index a4909282..605a6d5b 100644 --- a/apps/pythinker-code/src/tui/controllers/subagent-event-handler.ts +++ b/apps/pythinker-code/src/tui/controllers/subagent-event-handler.ts @@ -651,7 +651,10 @@ export class SubAgentEventHandler { missionControl.updateArgs(args, options); this.dynamicWorkflowMissionControls.set(toolCallId, missionControl); this.host.streamingUI.finalizeLiveTextBuffers('tool'); - this.host.state.transcriptContainer.addChild(missionControl); + this.host.state.transcriptContainer.addTranscriptChild(missionControl, { + role: 'live-durable', + edgeBlankPolicy: 'trim-plain', + }); this.host.updateActivityPane(); this.requestRender(); return missionControl; @@ -692,7 +695,7 @@ export class SubAgentEventHandler { const width = Math.floor(terminalColumns); const followingTranscriptRows = missionControl === undefined ? 0 - : renderedRowsAfterChild(state.transcriptContainer.children, missionControl, width); + : state.transcriptContainer.renderedRowsAfterChild(width, missionControl); // Under the fixed layout the transcript lives inside the layout root, so // the rows below it include the root's chrome + footer measurement and // later transcript siblings. diff --git a/apps/pythinker-code/src/tui/pythinker-tui.ts b/apps/pythinker-code/src/tui/pythinker-tui.ts index 88788b90..2f46fdec 100644 --- a/apps/pythinker-code/src/tui/pythinker-tui.ts +++ b/apps/pythinker-code/src/tui/pythinker-tui.ts @@ -583,9 +583,15 @@ export class PythinkerTUI { ); const banner = new BannerComponent(this.state.appState.banner); if (welcomeIndex >= 0) { - this.state.transcriptContainer.children.splice(welcomeIndex + 1, 0, banner); + this.state.transcriptContainer.addTranscriptChildAt(welcomeIndex + 1, banner, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); } else { - this.state.transcriptContainer.children.unshift(banner); + this.state.transcriptContainer.addTranscriptChildAt(0, banner, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); } this.state.transcriptContainer.invalidate(); } @@ -1855,7 +1861,10 @@ export class PythinkerTUI { const component = this.createTranscriptComponent(entry); if (component) { markTranscriptComponent(component, entry); - this.state.transcriptContainer.addChild(component); + this.state.transcriptContainer.addTranscriptChild(component, { + role: 'durable', + edgeBlankPolicy: 'trim-plain', + }); this.state.ui.requestRender(); } } @@ -1899,7 +1908,10 @@ export class PythinkerTUI { const welcome = new WelcomeComponent(this.state.appState, () => { this.state.ui.requestRender(); }); - this.state.transcriptContainer.addChild(welcome); + this.state.transcriptContainer.addTranscriptChild(welcome, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); } private clearTerminalInlineImages(): void { @@ -1924,14 +1936,19 @@ export class PythinkerTUI { this.imageStore.clear(); this.renderWelcome(); } - showStatus(message: string, color?: ColorToken): void { - this.state.transcriptContainer.addChild(new StatusMessageComponent(message, color)); + this.state.transcriptContainer.addTranscriptChild( + new StatusMessageComponent(message, color), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, + ); this.state.ui.requestRender(); } showNotice(title: string, detail?: string): void { - this.state.transcriptContainer.addChild(new NoticeMessageComponent(title, detail)); + this.state.transcriptContainer.addTranscriptChild( + new NoticeMessageComponent(title, detail), + { role: 'ephemeral', edgeBlankPolicy: 'preserve' }, + ); this.state.ui.requestRender(); } @@ -1946,8 +1963,14 @@ export class PythinkerTUI { showProgressSpinner(label: string): LoginProgressSpinnerHandle { const tint = (s: string): string => currentTheme.fg('primary', s); const spinner = new ActivityLoader(this.state.ui, tint, label); - this.state.transcriptContainer.addChild(new Spacer(1)); - this.state.transcriptContainer.addChild(spinner); + this.state.transcriptContainer.addTranscriptChild(new Spacer(1), { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); + this.state.transcriptContainer.addTranscriptChild(spinner, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); this.state.ui.requestRender(); return { stop: ({ ok, label: finalLabel }) => { @@ -2110,9 +2133,14 @@ export class PythinkerTUI { toggleToolOutputExpansion(): void { this.state.toolOutputExpanded = !this.state.toolOutputExpanded; - for (const child of this.state.transcriptContainer.children) { - if (isExpandable(child)) { - child.setExpanded(this.state.toolOutputExpanded); + for (const container of [ + this.state.transcriptContainer, + this.state.activityContainer, + ]) { + for (const child of container.children) { + if (isExpandable(child)) { + child.setExpanded(this.state.toolOutputExpanded); + } } } this.state.ui.requestRender(); diff --git a/apps/pythinker-code/src/tui/tui-state.ts b/apps/pythinker-code/src/tui/tui-state.ts index 6d997fcb..fded2482 100644 --- a/apps/pythinker-code/src/tui/tui-state.ts +++ b/apps/pythinker-code/src/tui/tui-state.ts @@ -5,9 +5,10 @@ import { } from '@earendil-works/pi-tui'; import { FooterComponent } from './components/chrome/footer'; +import { StatusBarComponent } from './components/chrome/status-bar'; import { GutterContainer } from './components/chrome/gutter-container'; +import { TranscriptContainer } from './components/chrome/transcript-container'; import type { ActivityLoader } from './components/chrome/activity-loader'; -import { StatusBarComponent } from './components/chrome/status-bar'; import { TodoPanelComponent } from './components/chrome/todo-panel'; import { TranscriptViewport } from './components/chrome/transcript-viewport'; import { ViewportLayoutRoot } from './components/chrome/viewport-layout'; @@ -34,7 +35,7 @@ export interface TUIState { terminal: ProcessTerminal; layout: TuiLayout; copyFullResponse: boolean; - transcriptContainer: Container; + transcriptContainer: TranscriptContainer; transcriptViewport: TranscriptViewport; layoutRoot: ViewportLayoutRoot; footerWrap: GutterContainer; @@ -86,7 +87,7 @@ export function createTUIState(options: PythinkerTUIOptions): TUIState { // pi-tui's types; ui.start() flips it back to false. (ui as unknown as { stopped: boolean }).stopped = true; - const transcriptContainer = new GutterContainer(CHROME_GUTTER, CHROME_GUTTER); + const transcriptContainer = new TranscriptContainer(CHROME_GUTTER, CHROME_GUTTER); const activityContainer = new GutterContainer(CHROME_GUTTER, CHROME_GUTTER); const todoPanelContainer = new GutterContainer(CHROME_GUTTER, CHROME_GUTTER); const todoPanel = new TodoPanelComponent(); diff --git a/apps/pythinker-code/src/tui/utils/transcript-component-metadata.ts b/apps/pythinker-code/src/tui/utils/transcript-component-metadata.ts index 12151958..ddfb6861 100644 --- a/apps/pythinker-code/src/tui/utils/transcript-component-metadata.ts +++ b/apps/pythinker-code/src/tui/utils/transcript-component-metadata.ts @@ -2,10 +2,22 @@ import type { Component } from '@earendil-works/pi-tui'; import type { TranscriptEntry } from '../types'; +export type TranscriptChildRole = 'durable' | 'live-durable' | 'ephemeral'; + +export interface TranscriptChildMetadata { + readonly role: TranscriptChildRole; + readonly edgeBlankPolicy: 'trim-plain' | 'preserve'; +} + const componentEntries = new WeakMap(); +const componentMetadata = new WeakMap(); export function markTranscriptComponent(component: Component, entry: TranscriptEntry): void { componentEntries.set(component, entry); + markTranscriptChild(component, { + role: 'durable', + edgeBlankPolicy: 'trim-plain', + }); } export function getTranscriptComponentEntry( @@ -13,3 +25,16 @@ export function getTranscriptComponentEntry( ): TranscriptEntry | undefined { return componentEntries.get(component); } + +export function markTranscriptChild( + component: Component, + metadata: TranscriptChildMetadata, +): void { + componentMetadata.set(component, metadata); +} + +export function getTranscriptChildMetadata( + component: Component, +): TranscriptChildMetadata | undefined { + return componentMetadata.get(component); +} diff --git a/apps/pythinker-code/test/tui/activity-pane.test.ts b/apps/pythinker-code/test/tui/activity-pane.test.ts index 0e3d1d66..b9274c0a 100644 --- a/apps/pythinker-code/test/tui/activity-pane.test.ts +++ b/apps/pythinker-code/test/tui/activity-pane.test.ts @@ -380,7 +380,7 @@ describe('updateActivityPane terminal progress', () => { const output = strip(missionControl.render(100).join('\n')); expect(output).toContain('– Cancelled'); expect(output).not.toMatch(/[◐◓◑◒] Orchestrating/); - for (const frame of BRAILLE_SPINNER_FRAMES) expect(output).not.toContain(frame); + expect(output).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN\s+Review changed files/u); state.activitySpinner?.instance.stop(); } finally { vi.useRealTimers(); diff --git a/apps/pythinker-code/test/tui/commands/advisor.test.ts b/apps/pythinker-code/test/tui/commands/advisor.test.ts new file mode 100644 index 00000000..ad5188a3 --- /dev/null +++ b/apps/pythinker-code/test/tui/commands/advisor.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { handleAdvisorCommand } from '#/tui/commands/index'; +import type { SlashCommandHost } from '#/tui/commands/dispatch'; + +function makeHost() { + const securityStatus = { + id: 'security', + name: 'Security', + enabled: true, + status: 'running' as const, + model: 'reviewer', + failures: 0, + notes: 2, + costUsd: 0.0123, + }; + const advisor = { + status: vi.fn(async () => [securityStatus]), + setEnabled: vi.fn(async () => [securityStatus]), + reload: vi.fn(async () => []), + }; + const host = { + session: { advisor }, + showError: vi.fn(), + showNotice: vi.fn(), + showStatus: vi.fn(), + } as unknown as SlashCommandHost; + return { host, advisor, securityStatus }; +} + +describe('handleAdvisorCommand', () => { + it('renders advisor status with runtime details', async () => { + const { host } = makeHost(); + + await handleAdvisorCommand(host, 'status'); + + expect(host.showNotice).toHaveBeenCalledWith( + 'Advisor status', + expect.stringContaining('● Security [running]'), + ); + expect(host.showNotice).toHaveBeenCalledWith( + 'Advisor status', + expect.stringContaining('2 notes · $0.0123'), + ); + }); + it('rejects an unknown advisor instead of reporting success', async () => { + const { host, advisor } = makeHost(); + advisor.setEnabled.mockResolvedValueOnce([]); + + await handleAdvisorCommand(host, 'on securty'); + + expect(host.showError).toHaveBeenCalledWith( + 'Unknown advisor: securty. Run /advisor status to list configured advisors.', + ); + expect(host.showStatus).not.toHaveBeenCalled(); + }); + it('does not report success when the runtime status remains disabled', async () => { + const { host, advisor, securityStatus } = makeHost(); + advisor.setEnabled.mockResolvedValueOnce([{ ...securityStatus, enabled: false }]); + + await handleAdvisorCommand(host, 'on security'); + + expect(host.showError).toHaveBeenCalledWith('Advisor security remains disabled.'); + expect(host.showStatus).not.toHaveBeenCalled(); + }); + it('does not report global enable success for an empty advisor set', async () => { + const { host, advisor } = makeHost(); + advisor.status.mockResolvedValueOnce([]); + advisor.setEnabled.mockResolvedValueOnce([]); + + await handleAdvisorCommand(host, 'on'); + + expect(host.showError).toHaveBeenCalledWith('Advisor remains disabled.'); + expect(host.showStatus).not.toHaveBeenCalled(); + }); + + it('toggles one advisor without changing its configuration file', async () => { + const { host, advisor, securityStatus } = makeHost(); + advisor.setEnabled.mockResolvedValueOnce([{ ...securityStatus, enabled: false }]); + + await handleAdvisorCommand(host, 'toggle security'); + + expect(advisor.setEnabled).toHaveBeenCalledWith(false, 'security'); + expect(host.showStatus).toHaveBeenCalledWith('Advisor security disabled.'); + }); + + it('reloads watchdog configuration', async () => { + const { host, advisor } = makeHost(); + + await handleAdvisorCommand(host, 'reload'); + + expect(advisor.reload).toHaveBeenCalledOnce(); + expect(host.showStatus).toHaveBeenCalledWith('Advisor configuration reloaded.'); + }); +}); diff --git a/apps/pythinker-code/test/tui/commands/dynamic-workflow.test.ts b/apps/pythinker-code/test/tui/commands/dynamic-workflow.test.ts index 5a852f81..e24b5312 100644 --- a/apps/pythinker-code/test/tui/commands/dynamic-workflow.test.ts +++ b/apps/pythinker-code/test/tui/commands/dynamic-workflow.test.ts @@ -50,7 +50,7 @@ function makeHost( workDir: overrides.workDir ?? '/workspace', }, theme: currentTheme, - transcriptContainer: { addChild: vi.fn() }, + transcriptContainer: { addTranscriptChild: vi.fn() }, ui: { requestRender: vi.fn() }, lastDynamicWorkflowArgs: overrides.lastDynamicWorkflowArgs, }, @@ -79,7 +79,7 @@ function mountedPicker(host: SlashCommandHost): TestPicker { } function markerAddChild(host: SlashCommandHost): ReturnType { - return host.state.transcriptContainer.addChild as ReturnType; + return host.state.transcriptContainer.addTranscriptChild as ReturnType; } function expectDynamicWorkflowMarker(host: SlashCommandHost, text: string): void { diff --git a/apps/pythinker-code/test/tui/commands/goal.test.ts b/apps/pythinker-code/test/tui/commands/goal.test.ts index 3fa068f1..bdb88da0 100644 --- a/apps/pythinker-code/test/tui/commands/goal.test.ts +++ b/apps/pythinker-code/test/tui/commands/goal.test.ts @@ -97,7 +97,7 @@ function makeHost( cancel: vi.fn(async () => {}), }; const hasSession = overrides.hasSession ?? true; - const transcriptContainer = { addChild: vi.fn() }; + const transcriptContainer = { addTranscriptChild: vi.fn() }; const host = { state: { appState: { @@ -451,8 +451,8 @@ describe('handleGoalCommand', () => { expect(host.showStatus).not.toHaveBeenCalledWith( 'Upcoming goal added. It will start after the current goal is complete.', ); - const addChild = host.state.transcriptContainer.addChild as ReturnType; - const message = addChild.mock.calls[0]?.[0] as { render(width: number): string[] }; + const addTranscriptChild = host.state.transcriptContainer.addTranscriptChild as ReturnType; + const message = addTranscriptChild.mock.calls[0]?.[0] as { render(width: number): string[] }; expect(stripAnsi(message.render(80).join('\n'))).toBe( '\n● Upcoming goal added. It will start after the current goal is complete.', ); diff --git a/apps/pythinker-code/test/tui/commands/hooks.test.ts b/apps/pythinker-code/test/tui/commands/hooks.test.ts index 16fe2ed7..f18bb280 100644 --- a/apps/pythinker-code/test/tui/commands/hooks.test.ts +++ b/apps/pythinker-code/test/tui/commands/hooks.test.ts @@ -134,11 +134,11 @@ describe('files slash command', () => { describe('context slash command', () => { it('renders the model-visible context report in the existing usage panel', async () => { - const addChild = vi.fn(); + const addTranscriptChild = vi.fn(); const requestRender = vi.fn(); const host = { state: { - transcriptContainer: { addChild }, + transcriptContainer: { addTranscriptChild }, ui: { requestRender }, }, requireSession: () => ({ @@ -160,8 +160,8 @@ describe('context slash command', () => { await showContextReport(host, ''); - expect(addChild).toHaveBeenCalledOnce(); - const panel = addChild.mock.calls[0]?.[0] as { render(width: number): string[] }; + expect(addTranscriptChild).toHaveBeenCalledOnce(); + const panel = addTranscriptChild.mock.calls[0]?.[0] as { render(width: number): string[] }; expect(panel.render(100).join('\n')).toContain('Context'); expect(panel.render(100).join('\n')).toContain('mock-model'); expect(requestRender).toHaveBeenCalledOnce(); diff --git a/apps/pythinker-code/test/tui/commands/registry.test.ts b/apps/pythinker-code/test/tui/commands/registry.test.ts index 3083a5d4..0dea3516 100644 --- a/apps/pythinker-code/test/tui/commands/registry.test.ts +++ b/apps/pythinker-code/test/tui/commands/registry.test.ts @@ -100,6 +100,15 @@ describe('built-in slash command registry', () => { { value: 'status', label: 'status', description: 'Show Fast mode status' }, ]); }); + it('keeps advisor status and the omitted verb available while busy', () => { + const advisor = findBuiltInSlashCommand('advisor'); + expect(advisor).toBeDefined(); + expect(resolveSlashCommandAvailability(advisor!, '')).toBe('always'); + expect(resolveSlashCommandAvailability(advisor!, 'status')).toBe('always'); + expect(resolveSlashCommandAvailability(advisor!, 'on')).toBe('idle-only'); + expect(resolveSlashCommandAvailability(advisor!, 'off')).toBe('idle-only'); + }); + it('marks plan clear as idle-only while normal plan toggles are always available', () => { const plan = findBuiltInSlashCommand('plan'); diff --git a/apps/pythinker-code/test/tui/components/chrome/transcript-container.test.ts b/apps/pythinker-code/test/tui/components/chrome/transcript-container.test.ts new file mode 100644 index 00000000..3631ae65 --- /dev/null +++ b/apps/pythinker-code/test/tui/components/chrome/transcript-container.test.ts @@ -0,0 +1,97 @@ +import type { Component } from '@earendil-works/pi-tui'; +import { describe, expect, it } from 'vitest'; + +import { + TranscriptContainer, + type TranscriptChildMetadata, +} from '#/tui/components/chrome/transcript-container'; + +class StubLines implements Component { + constructor(private readonly lines: readonly string[]) {} + + render(): string[] { + return [...this.lines]; + } + + invalidate(): void {} +} + +const durable: TranscriptChildMetadata = { + role: 'durable', + edgeBlankPolicy: 'trim-plain', +}; + +const ephemeral: TranscriptChildMetadata = { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', +}; + +describe('TranscriptContainer', () => { + it('trims opted-in edge blanks and inserts one durable separator', () => { + const container = new TranscriptContainer(2, 2); + const first = new StubLines(['', 'first', '']); + const second = new StubLines(['', 'second', '']); + + container.addTranscriptChild(first, durable); + container.addTranscriptChild(second, durable); + + expect(container.render(20)).toEqual([' first', ' ', ' second']); + expect(container.children).toEqual([first, second]); + expect(container.renderedRowsAfterChild(20, first)).toBe(2); + }); + + it('preserves ANSI blank rows and does not invent gaps around ephemeral children', () => { + const container = new TranscriptContainer(1, 1); + const first = new StubLines(['', '\u001B[48;5;1m \u001B[0m', 'first', '']); + const status = new StubLines(['status']); + const second = new StubLines(['', 'second']); + + container.addTranscriptChild(first, durable); + container.addTranscriptChild(status, ephemeral); + container.addTranscriptChild(second, durable); + expect(container.render(20)).toEqual([ + ' \u001B[48;5;1m \u001B[0m', + ' first', + ' status', + ' second', + ]); + expect(container.renderedRowsAfterChild(20, first)).toBe(2); + }); + + it('skips empty durable segments when placing separators', () => { + const container = new TranscriptContainer(1, 1); + const first = new StubLines(['first']); + const empty = new StubLines([]); + const second = new StubLines(['second']); + + container.addTranscriptChild(first, durable); + container.addTranscriptChild(empty, { + role: 'live-durable', + edgeBlankPolicy: 'trim-plain', + }); + container.addTranscriptChild(second, durable); + + expect(container.render(20)).toEqual([' first', ' ', ' second']); + }); + + it('matches rows after an invisible child to visible separator state', () => { + const container = new TranscriptContainer(1, 1); + const status = new StubLines(['status']); + const empty = new StubLines([]); + const second = new StubLines(['second']); + + container.addTranscriptChild(status, ephemeral); + container.addTranscriptChild(empty, durable); + container.addTranscriptChild(second, durable); + + expect(container.render(20)).toEqual([' status', ' second']); + expect(container.renderedRowsAfterChild(20, empty)).toBe(1); + }); + + it('keeps unregistered policy out of normalization by requiring explicit metadata', () => { + const container = new TranscriptContainer(0, 0); + const child = new StubLines(['', 'status', '']); + + expect(() => container.addChild(child)).toThrow(/addTranscriptChild/u); + }); +}); diff --git a/apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts b/apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts index 23d2005f..0a655840 100644 --- a/apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts +++ b/apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts @@ -7,7 +7,11 @@ import { type DynamicWorkflowMissionControlOptions, dynamicWorkflowResultSummaryFromOutput, } from '#/tui/components/messages/dynamic-workflow-mission-control'; -import { BRAILLE_SPINNER_INTERVAL_MS } from '#/tui/constant/rendering'; +import { + BRAILLE_SPINNER_FRAMES, + DYNAMIC_WORKFLOW_RENDERING, + BRAILLE_SPINNER_INTERVAL_MS, +} from '#/tui/constant/rendering'; import { currentTheme, darkColors } from '#/tui/theme'; const DESCRIPTION = 'Review the interface'; @@ -21,7 +25,8 @@ function renderText(component: DynamicWorkflowMissionControlComponent, width = 1 } /** Lifecycle progress glyph and label for a running row. */ -const RUNNING_CELL = /[◐◓◑◒]\s+RUN/u; +const RUNNING_GLYPH = /[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/u; +const RUNNING_CELL = new RegExp(`${RUNNING_GLYPH.source}\\s+RUN`, 'u'); /** Head of a task cell that lost the preamble every row shared. */ const TASK_ELISION_MARK = '…'; @@ -321,7 +326,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { expect(aggregateLine(output)).toContain('2/3 complete'); expect(aggregateLine(output)).not.toMatch(/\b\d+%/u); expect(aggregateLine(output)).not.toContain('━'); - expect(memberLine(output, 1)).toMatch(/[◐◓◑◒]\s+RUN\s+Layout hierarchy/u); + expect(memberLine(output, 1)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN\s+Layout hierarchy/u); expect(memberLine(output, 2)).toMatch(/✓\s+DONE\s+Interaction audit/u); expect(output).not.toMatch(/[⣿⣷⣯⣟⡿⢿⣻⣽]{4,}/u); }); @@ -414,8 +419,8 @@ describe('DynamicWorkflowMissionControlComponent', () => { vi.setSystemTime(300); const second = colouredMemberLine(); - expect(first).toContain(chalk.hex(darkColors.primary)('◐')); - expect(second).toContain(chalk.hex(darkColors.primary)('◓')); + expect(first).toContain(chalk.hex(darkColors.primary)('⠋')); + expect(second).toContain(chalk.hex(darkColors.primary)('⠙')); expect(first).toContain(chalk.hex(darkColors.primary)('RUN')); vi.setSystemTime(BRAILLE_SPINNER_INTERVAL_MS * 1.5); @@ -444,7 +449,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { const output = renderText(component, 120); expect(output).toContain('– Cancelled'); - expect(memberLine(output, 1)).toMatch(/[◐◓◑◒]\s+RUN\s+Running work/u); + expect(memberLine(output, 1)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN\s+Running work/u); expect(memberLine(output, 2)).toMatch(/○\s+WAIT\s+Queued work/u); expect(output).not.toContain('– STOP'); expect(output).not.toContain('⠋ Orchestrating'); @@ -520,7 +525,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { expect(renderText(component, 100)).toContain('Rate limited'); component.markStarted('agent-1'); - expect(memberLine(renderText(component, 100), 1)).toMatch(/[◐◓◑◒]\s+RUN/u); + expect(memberLine(renderText(component, 100), 1)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN/u); }); it('prefers a suspension detail over stale model progress in the member row', () => { @@ -649,7 +654,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { const running = renderText(component, 100); expect(running).toContain('PROGRESS'); expect(running).not.toContain('WORK IDLE'); - expect(memberLine(running, 1)).toMatch(/◐\s+RUN\s+Live work/u); + expect(memberLine(running, 1)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN\s+Live work/u); expect(memberLine(running, 2)).toMatch(/○\s+WAIT\s+Queued work/u); expect(running).not.toMatch(/\b\d+%|⚒|━/u); }); @@ -670,7 +675,8 @@ describe('DynamicWorkflowMissionControlComponent', () => { component.registerSubagent({ agentId: 'agent-1' }); component.markStarted('agent-1'); - for (const [time, glyph] of [[0, '◐'], [300, '◓'], [600, '◑'], [900, '◒']] as const) { + for (const [index, glyph] of BRAILLE_SPINNER_FRAMES.slice(0, 4).entries()) { + const time = index * DYNAMIC_WORKFLOW_RENDERING.progressFrameMs; vi.setSystemTime(time); const line = component.render(100).find((candidate) => strip(candidate).includes('001')); expect(line).toContain(chalk.hex(darkColors.primary)(glyph)); @@ -707,7 +713,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { const output = renderText(component, 100); expect(memberLine(output, 1)).toMatch(/○\s+WAIT/u); - expect(memberLine(output, 2)).toMatch(/[◐◓◑◒]\s+RUN/u); + expect(memberLine(output, 2)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN/u); expect(memberLine(output, 3)).toMatch(/✓\s+DONE/u); } finally { chalk.level = previousLevel; @@ -726,7 +732,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { const output = renderText(component, 100); const glyphColumns = [ memberLine(output, 1).indexOf('○'), - memberLine(output, 2).search(/[◐◓◑◒]/u), + memberLine(output, 2).search(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/u), memberLine(output, 3).indexOf('✓'), ]; expect(glyphColumns[0]).toBeGreaterThan(0); @@ -892,7 +898,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { const output = strip(rendered.join('\n')); expect(rendered.every((line) => visibleWidth(line) <= width)).toBe(true); - expect(memberLine(output, 1)).toMatch(/[◐◓◑◒]\s+RUN/u); + expect(memberLine(output, 1)).toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN/u); expect(output.includes('PROGRESS')).toBe(expectedProgress); expect(output.includes('STATUS')).toBe(expectedStatus); expect(output).not.toContain('WORK IDLE'); @@ -909,7 +915,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { component.markStarted('agent-1'); vi.setSystemTime(30_000); - const before = memberLine(renderText(component, 100), 1).match(/[◐◓◑◒]/u)?.[0]; + const before = memberLine(renderText(component, 100), 1).match(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/u)?.[0]; component.recordToolCall({ agentId: 'agent-1', name: 'Read' }); for (let index = 0; index < 200; index += 1) { component.appendModelDelta({ agentId: 'agent-1', delta: `chunk ${String(index)} ` }); @@ -917,7 +923,7 @@ describe('DynamicWorkflowMissionControlComponent', () => { component.recordToolCall({ agentId: 'agent-1', name: 'Bash' }); const output = renderText(component, 100); - const after = memberLine(output, 1).match(/[◐◓◑◒]/u)?.[0]; + const after = memberLine(output, 1).match(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/u)?.[0]; expect(before).toBeDefined(); expect(after).toBe(before); expect(output).not.toMatch(/\b\d+%|⚒/u); diff --git a/apps/pythinker-code/test/tui/controllers/session-event-handler-goal-queue.test.ts b/apps/pythinker-code/test/tui/controllers/session-event-handler-goal-queue.test.ts index 9e389f91..d8c307d2 100644 --- a/apps/pythinker-code/test/tui/controllers/session-event-handler-goal-queue.test.ts +++ b/apps/pythinker-code/test/tui/controllers/session-event-handler-goal-queue.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, beforeEach, vi } from 'vitest'; import { MCP_STATUS_TRANSIENT_DURATION_MS } from '#/tui/constant/pythinker-tui'; import { FooterComponent, footerStatusFromAppState } from '#/tui/components/chrome/footer'; +import { TranscriptContainer } from '#/tui/components/chrome/transcript-container'; import { DEFAULT_STATUS_LINE_CONFIG } from '#/tui/config'; import { createFooterState, @@ -82,7 +83,7 @@ function makeHost(options: { createGoalRejects?: boolean } = {}) { toolOutputExpanded: false, todoPanel: { getTodos: vi.fn(() => []) }, footer: { setTokenSpeed: vi.fn() }, - transcriptContainer: { addChild: vi.fn() }, + transcriptContainer: { addTranscriptChild: vi.fn() }, mcpStatusContainer: new Container(), ui: { requestRender: vi.fn() }, }, @@ -236,7 +237,7 @@ function modelBlockedEvent() { } function addedTranscriptText(host: ReturnType['host']): string { - const component = host.state.transcriptContainer.addChild.mock.calls.at(-1)?.[0]; + const component = host.state.transcriptContainer.addTranscriptChild.mock.calls.at(-1)?.[0]; return component.render(80).join('\n').replaceAll(/\u001B\[[0-9;]*m/g, ''); } @@ -939,7 +940,7 @@ describe('SessionEventHandler goal queue promotion', () => { handler.handleEvent(modelBlockedEvent(), vi.fn()); - expect(host.state.transcriptContainer.addChild).not.toHaveBeenCalled(); + expect(host.state.transcriptContainer.addTranscriptChild).not.toHaveBeenCalled(); }); it('renders a blocked fallback when the model does not explain the blocked goal', () => { @@ -969,7 +970,7 @@ describe('SessionEventHandler goal queue promotion', () => { ); handler.handleEvent(turnEndedEvent(), vi.fn()); - expect(host.state.transcriptContainer.addChild).not.toHaveBeenCalled(); + expect(host.state.transcriptContainer.addTranscriptChild).not.toHaveBeenCalled(); }); it('does not render a blocked fallback after earlier assistant text in the same turn', () => { @@ -989,7 +990,7 @@ describe('SessionEventHandler goal queue promotion', () => { handler.handleEvent(modelBlockedEvent(), vi.fn()); handler.handleEvent(turnEndedEvent(), vi.fn()); - expect(host.state.transcriptContainer.addChild).not.toHaveBeenCalled(); + expect(host.state.transcriptContainer.addTranscriptChild).not.toHaveBeenCalled(); }); it('does not promote on paused or cancelled updates', async () => { @@ -1096,7 +1097,7 @@ describe('SessionEventHandler MCP startup status', () => { expect(output).toContain('MCP servers · 2/4 connected · 2 loading…'); expect(occurrences(output, 'MCP servers')).toBe(1); expect(output).not.toContain('"second"'); - expect(host.state.transcriptContainer.addChild).not.toHaveBeenCalled(); + expect(host.state.transcriptContainer.addTranscriptChild).not.toHaveBeenCalled(); handler.disposeMcpServerStatusRows(); expect(vi.getTimerCount()).toBe(0); }); @@ -1216,7 +1217,7 @@ describe('SessionEventHandler MCP startup status', () => { describe('SessionEventHandler hook status', () => { it('shows configured hook status only while the hook is running', () => { const { host } = makeHost(); - const transcriptContainer = new Container(); + const transcriptContainer = new TranscriptContainer(0, 0); host.state.transcriptContainer = transcriptContainer as never; const handler = new SessionEventHandler(host); const event = { diff --git a/apps/pythinker-code/test/tui/parity/feature-matrix.ts b/apps/pythinker-code/test/tui/parity/feature-matrix.ts index 07557788..a3bfb14a 100644 --- a/apps/pythinker-code/test/tui/parity/feature-matrix.ts +++ b/apps/pythinker-code/test/tui/parity/feature-matrix.ts @@ -96,6 +96,7 @@ const SESSION_EVENTS = [ 'cron.fired', 'mcp.server.status', 'tool.list.updated', + 'advisor.status', ] as const; const TRANSCRIPT_ENTRY_KINDS = [ @@ -148,6 +149,7 @@ const DIALOG_VIEW_ROUTES = [ const COMMANDS = [ 'add-dir', + 'advisor', 'agents', 'yolo', 'auto', diff --git a/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts b/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts index a19c3944..f6a17751 100644 --- a/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts +++ b/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts @@ -26,6 +26,7 @@ import { performHeapDump } from '#/utils/heap-dump'; import { getInputHistoryFile } from '#/utils/paths'; import { DynamicWorkflowMissionControlComponent } from '#/tui/components/messages/dynamic-workflow-mission-control'; import { ThinkingComponent } from '#/tui/components/messages/thinking'; +import { BRAILLE_SPINNER_FRAMES } from '#/tui/constant/rendering'; import { BtwPanelComponent } from '#/tui/components/panes/btw-panel'; import { WelcomeComponent } from '#/tui/components/chrome/welcome'; import { ChoicePickerComponent } from '#/tui/components/dialogs/choice-picker'; @@ -3948,7 +3949,7 @@ command = "vim" const transcript = stripSgr(renderTranscript(driver)); expect(transcript).toContain('Dynamic Workflow'); // The running row advances through the approved progress-glyph frames. - expect(transcript).toMatch(/001\s+[◐◓◑◒]\s+RUN\s+src\/a.ts/u); + expect(transcript).toMatch(/001\s+[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]\s+RUN\s+src\/a.ts/u); expect(transcript).toMatch(/002\s+✓\s+DONE\s+src\/b.ts/u); expect(transcript).toMatch(/Orchestrating\s+1\/2 complete/u); expect(transcript).not.toContain('━'); @@ -4248,7 +4249,10 @@ command = "vim" render: () => ['Later transcript row one', 'Later transcript row two', 'Later transcript row three'], invalidate: () => {}, }; - driver.state.transcriptContainer.addChild(followingTranscript); + driver.state.transcriptContainer.addTranscriptChild(followingTranscript, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); const lines = missionControl.render(100); expect(lines).toHaveLength(5); @@ -4280,7 +4284,10 @@ command = "vim" render: () => ['Later transcript row one', 'Later transcript row two', 'Later transcript row three'], invalidate: () => {}, }; - driver.state.transcriptContainer.addChild(followingTranscript); + driver.state.transcriptContainer.addTranscriptChild(followingTranscript, { + role: 'ephemeral', + edgeBlankPolicy: 'preserve', + }); const lines = missionControl.render(100); expect(driver.state.transcriptContainer.children).toContain(missionControl); @@ -5894,6 +5901,39 @@ command = "vim" // Collapsed live thinking renders only the spinner header, never the text. expect(stripSgr(renderTranscript(driver))).not.toContain('visible reasoning'); }); + it('keeps the live thinking spinner in prompt chrome, not the transcript', async () => { + const { driver } = await makeDriver(); + + driver.sessionEventHandler.handleEvent( + { + type: 'thinking.delta', + agentId: 'main', + sessionId: 'ses-1', + delta: 'visible reasoning', + } as Event, + vi.fn(), + ); + driver.streamingUI.flushNow(); + + const activity = stripSgr(renderActivity(driver)); + const transcript = stripSgr(renderTranscript(driver)); + expect(activity).toContain(BRAILLE_SPINNER_FRAMES[0]); + expect(transcript).not.toMatch(/[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/u); + expect( + driver.state.activityContainer.children.some((child) => child instanceof ThinkingComponent), + ).toBe(true); + expect( + driver.state.transcriptContainer.children.some((child) => child instanceof ThinkingComponent), + ).toBe(false); + }); + it('expands the prompt-mounted thinking component with the shared toggle', async () => { + const { driver } = await makeDriver(); + + driver.streamingUI.onThinkingUpdate('line one\nline two'); + (driver as unknown as PythinkerTUI).toggleToolOutputExpansion(); + + expect(stripSgr(renderActivity(driver))).toContain('line two'); + }); it('does not create a thinking component for whitespace-only replay content', async () => { const { driver } = await makeDriver(); diff --git a/docs/reference/slash-commands.md b/docs/reference/slash-commands.md index 39d9d796..36a825b9 100644 --- a/docs/reference/slash-commands.md +++ b/docs/reference/slash-commands.md @@ -101,6 +101,19 @@ pythinker -p "/goal Fix the failing checkout test" Prompt mode exits with code `0` when the goal completes, `3` when it blocks, and `6` when it pauses. Other `/goal` subcommands, including `next`, are TUI controls and are not handled by `pythinker -p`. +## Session Advisor + +The session advisor runs reviewer agents in the background to check your work while the session progresses. It is configured through `WATCHDOG.md` or `WATCHDOG.yml` files in the user and project scopes. The `/advisor` command shows and controls it. + +| Command | Action | Availability | +| --- | --- | --- | +| `/advisor` | Show advisor status (same as `/advisor status`) | Idle only | +| `/advisor status` | Show each configured advisor and its runtime status | Always available | +| `/advisor on [advisor-id]` | Enable all advisors, or the named advisor | Idle only | +| `/advisor off [advisor-id]` | Disable all advisors, or the named advisor | Idle only | +| `/advisor toggle [advisor-id]` | Toggle all advisors, or the named advisor | Idle only | +| `/advisor reload` | Reload the advisor configuration from disk | Idle only | + ## Information & Status | Command | Alias | Description | Always available | diff --git a/packages/agent-core/src/agent/compaction/full.ts b/packages/agent-core/src/agent/compaction/full.ts index 0665802a..af4b364e 100644 --- a/packages/agent-core/src/agent/compaction/full.ts +++ b/packages/agent-core/src/agent/compaction/full.ts @@ -342,7 +342,7 @@ export class FullCompaction { startIndex + compactedCount, ); const messages = [ - ...this.agent.context.project(messagesToCompact), + ...this.agent.context.project(messagesToCompact, startIndex), createUserMessage(renderPrompt(compactionInstructionTemplate, { customInstruction: data.instruction ?? '' })), ]; try { diff --git a/packages/agent-core/src/agent/compaction/micro.ts b/packages/agent-core/src/agent/compaction/micro.ts index f69eb9ac..f8f7f085 100644 --- a/packages/agent-core/src/agent/compaction/micro.ts +++ b/packages/agent-core/src/agent/compaction/micro.ts @@ -94,7 +94,10 @@ export class MicroCompaction { } } - compact(messages: readonly ContextMessage[]): readonly ContextMessage[] { + compact( + messages: readonly ContextMessage[], + offset = 0, + ): readonly ContextMessage[] { if (!this.agent.experimentalFlags.enabled('micro_compaction')) return messages; const config = this.config; @@ -102,7 +105,7 @@ export class MicroCompaction { let i = 0; for (const msg of messages) { if ( - i < this.cutoff && + i + offset < this.cutoff && msg.role === 'tool' && msg.toolCallId !== undefined && estimateTokensForContentParts(msg.content) >= config.minContentTokens diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 5947f461..f2cea9b0 100644 --- a/packages/agent-core/src/agent/context/index.ts +++ b/packages/agent-core/src/agent/context/index.ts @@ -41,6 +41,7 @@ const TOOL_INTERRUPTED_ON_RESUME_OUTPUT = // later messages in deferredMessages until those ids are resolved. export class ContextMemory { private _history: ContextMessage[] = []; + private _historyRevision = 0; private _tokenCount = 0; private tokenCountCoveredMessageCount = 0; private openSteps: Map = new Map(); @@ -87,11 +88,13 @@ export class ContextMemory { } else { this._history.pop(); } + this._historyRevision += 1; return true; } clear(): void { this.agent.records.logRecord({ type: 'context.clear' }); + this._historyRevision += 1; this._history = []; this._tokenCount = 0; this.tokenCountCoveredMessageCount = 0; @@ -138,6 +141,7 @@ export class ContextMemory { } this.agent.replayBuilder.removeLastMessages(removedMessages); + this._historyRevision += 1; this.openSteps.clear(); this.pendingToolResultIds.clear(); @@ -202,6 +206,7 @@ export class ContextMemory { }, ...this._history.slice(endIndex), ]; + this._historyRevision += 1; this.openSteps.clear(); this.flushDeferredMessagesIfToolExchangeClosed(); this._tokenCount = result.tokensAfter; @@ -281,8 +286,12 @@ export class ContextMemory { return this._history; } - project(messages: readonly ContextMessage[]): Message[] { - return project(this.agent.microCompaction.compact(messages)); + get historyRevision(): number { + return this._historyRevision; + } + + project(messages: readonly ContextMessage[], offset = 0): Message[] { + return project(this.agent.microCompaction.compact(messages, offset)); } get messages(): Message[] { diff --git a/packages/agent-core/src/config/schema.ts b/packages/agent-core/src/config/schema.ts index 115b40f4..8edbfc36 100644 --- a/packages/agent-core/src/config/schema.ts +++ b/packages/agent-core/src/config/schema.ts @@ -61,6 +61,7 @@ export const AdvisorConfigSchema = z.object({ enabled: z.boolean().optional(), model: z.string().optional(), instructions: z.string().optional(), + tools: z.array(z.string()).optional(), }); export type AdvisorConfig = z.infer; diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index c16f6e69..9c35ef9b 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -6,6 +6,12 @@ export type { WorkingTreeChangeStatus, WorkingTreeFileDiff, } from './session/working-tree'; +export type { + AdvisorConfigEntry, + AdvisorRuntimeStatus, + AdvisorStatusSnapshot, + DiscoveredAdvisors, +} from './session/advisor-config'; export * from './rpc'; export * from './config'; export * from './flags'; diff --git a/packages/agent-core/src/rpc/core-api.ts b/packages/agent-core/src/rpc/core-api.ts index 1a5cfb02..e4dc374f 100644 --- a/packages/agent-core/src/rpc/core-api.ts +++ b/packages/agent-core/src/rpc/core-api.ts @@ -27,11 +27,14 @@ import type { FileCheckpointSummary, RestoreFileCheckpointResult, } from '#/session/file-checkpoints'; +import type { AdvisorStatusSnapshot } from '#/session/advisor-config'; import type { WorkingTreeChanges, WorkingTreeFileDiff } from '#/session/working-tree'; import type { UsageStatus } from './events'; import type { WithAgentId, WithSessionId } from './types'; +export type AdvisorStatus = AdvisorStatusSnapshot; + export type JsonPrimitive = string | number | boolean | null; export type JsonValue = | JsonPrimitive @@ -425,6 +428,17 @@ export interface RemovePythinkerProviderPayload { readonly providerId: string; } +export interface SetAdvisorEnabledPayload { + readonly enabled: boolean; + readonly advisorId?: string; +} + +export interface SessionAdvisorAPI { + getAdvisorStatus: (payload: EmptyPayload) => readonly AdvisorStatus[]; + setAdvisorEnabled: (payload: SetAdvisorEnabledPayload) => readonly AdvisorStatus[]; + reloadAdvisor: (payload: EmptyPayload) => readonly AdvisorStatus[]; +} + export interface AgentAPI { prompt: (payload: PromptPayload) => void; steer: (payload: SteerPayload) => void; @@ -469,7 +483,7 @@ export interface AgentAPI { type AgentAPIWithId = WithAgentId; -export interface SessionAPI extends AgentAPIWithId { +export interface SessionAPI extends AgentAPIWithId, SessionAdvisorAPI { renameSession: (payload: RenameSessionPayload) => void; updateSessionMetadata: (payload: UpdateSessionMetadataPayload) => void; getSessionMetadata: (payload: EmptyPayload) => SessionMeta; diff --git a/packages/agent-core/src/rpc/core-impl.ts b/packages/agent-core/src/rpc/core-impl.ts index 9dfa8cde..b05016a6 100644 --- a/packages/agent-core/src/rpc/core-impl.ts +++ b/packages/agent-core/src/rpc/core-impl.ts @@ -109,6 +109,7 @@ import type { ResumeSessionPayload, SessionSummary, SetActiveToolsPayload, + SetAdvisorEnabledPayload, SetFastModePayload, SetPythinkerConfigPayload, SetModelPayload, @@ -850,6 +851,26 @@ export class PythinkerCore implements PromisableMethods { }: SessionScopedPayload): Promise { return this.sessionApi(sessionId).listSkills(payload); } + getAdvisorStatus({ + sessionId, + ...payload + }: SessionScopedPayload) { + return this.sessionApi(sessionId).getAdvisorStatus(payload); + } + + setAdvisorEnabled({ + sessionId, + ...payload + }: SessionScopedPayload) { + return this.sessionApi(sessionId).setAdvisorEnabled(payload); + } + + reloadAdvisor({ + sessionId, + ...payload + }: SessionScopedPayload) { + return this.sessionApi(sessionId).reloadAdvisor(payload); + } reloadSkills({ sessionId, diff --git a/packages/agent-core/src/rpc/events.ts b/packages/agent-core/src/rpc/events.ts index 2eae5c26..8ff4333a 100644 --- a/packages/agent-core/src/rpc/events.ts +++ b/packages/agent-core/src/rpc/events.ts @@ -2,6 +2,7 @@ export { MCP_OAUTH_AUTHORIZATION_URL_TOOL_UPDATE } from '@pymodel/protocol'; export type { AgentEvent, + AdvisorStatusEvent, AgentStatusUpdatedEvent, AssistantDeltaEvent, BackgroundTaskStartedEvent, diff --git a/packages/agent-core/src/session/advisor-config.ts b/packages/agent-core/src/session/advisor-config.ts new file mode 100644 index 00000000..82c84ee5 --- /dev/null +++ b/packages/agent-core/src/session/advisor-config.ts @@ -0,0 +1,287 @@ +import { readFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import path from 'node:path'; +import { load as loadYaml } from 'js-yaml'; +import { isPlainRecord } from '../agent/turn/canonical-args'; +import { findProjectRoot } from '../skill/scanner'; + +export interface AdvisorConfigEntry { + readonly name: string; + readonly model?: string; + readonly tools?: readonly string[]; + readonly instructions?: string; + readonly enabled?: boolean; +} + +export type AdvisorRuntimeStatus = + | 'running' + | 'paused' + | 'quota_exhausted' + | 'error' + | 'no_model'; + +export interface DiscoveredAdvisors { + readonly advisors: readonly AdvisorConfigEntry[]; + readonly sharedInstructions?: string; + readonly files: readonly string[]; +} + +export interface AdvisorStatusSnapshot { + readonly id: string; + readonly name: string; + readonly enabled: boolean; + readonly status: AdvisorRuntimeStatus; + readonly model?: string; + readonly failures: number; + readonly notes: number; + readonly costUsd: number; + readonly message?: string; +} + +interface WatchdogConfigDocument { + readonly instructions?: unknown; + readonly advisors?: unknown; +} + +interface AdvisorConfigDocumentEntry { + readonly name?: unknown; + readonly model?: unknown; + readonly tools?: unknown; + readonly instructions?: unknown; + readonly enabled?: unknown; +} + +/** + * Normalize a configured advisor name into a safe, stable runtime id. + * Collisions are resolved by the discovery caller's last-writer-wins map. + */ +export function slugifyAdvisorName(name: string): string { + const slug = name + .toLowerCase() + .replaceAll(/[^a-z0-9]+/gu, '-') + .replaceAll(/^-+|-+$/gu, ''); + return slug.length === 0 ? 'advisor' : slug; +} + +/** + * Discover WATCHDOG.md and WATCHDOG.yml files from user and project scopes. + * User files are read first. Project files are then applied from ancestor to + * leaf, so a more specific project entry replaces an earlier same-id entry. + */ +export async function discoverAdvisorConfigs( + cwd: string, + userHomeDir = homedir(), + onWarning: (message: string, details?: Record) => void = () => {}, +): Promise { + const candidates = await collectConfigCandidates( + cwd, + userHomeDir, + ['WATCHDOG.md', 'WATCHDOG.yml', 'WATCHDOG.yaml'], + onWarning, + ); + const watchdogPaths = candidates + .filter((candidate) => candidate.fileName === 'WATCHDOG.md') + .map((candidate) => candidate.path); + const advisors = new Map(); + const sharedParts: string[] = []; + const configFiles: string[] = []; + + for (const candidate of candidates) { + if (candidate.fileName === 'WATCHDOG.md') { + const content = candidate.content.trim(); + if (content.length > 0) { + sharedParts.push(`Especially pay attention to:\n\n${content}\n`); + } + continue; + } + configFiles.push(candidate.path); + let parsed: unknown; + try { + parsed = loadYaml(candidate.content); + } catch (error) { + onWarning('Advisor config YAML could not be parsed', { + path: candidate.path, + error: error instanceof Error ? error.message : String(error), + }); + continue; + } + if (!isPlainRecord(parsed)) { + onWarning('Advisor config must be a YAML mapping', { path: candidate.path }); + continue; + } + + const document = parsed as WatchdogConfigDocument; + if (typeof document.instructions === 'string' && document.instructions.trim().length > 0) { + sharedParts.push(document.instructions.trim()); + } + if (document.advisors === undefined) continue; + if (!Array.isArray(document.advisors)) { + onWarning('Advisor config advisors must be a YAML list', { path: candidate.path }); + continue; + } + + for (const rawEntry of document.advisors) { + if (!isPlainRecord(rawEntry)) { + onWarning('Advisor config entry must be a YAML mapping', { path: candidate.path }); + continue; + } + const entry = rawEntry as AdvisorConfigDocumentEntry; + if (typeof entry.name !== 'string' || entry.name.trim().length === 0) { + onWarning('Advisor config entry requires a name', { path: candidate.path }); + continue; + } + if (entry.model !== undefined && typeof entry.model !== 'string') { + onWarning('Advisor config model must be a string', { path: candidate.path }); + continue; + } + if (entry.instructions !== undefined && typeof entry.instructions !== 'string') { + onWarning('Advisor config instructions must be a string', { path: candidate.path }); + continue; + } + if (entry.enabled !== undefined && typeof entry.enabled !== 'boolean') { + onWarning('Advisor config enabled must be a boolean', { path: candidate.path }); + continue; + } + const tools = normalizeTools(entry.tools, candidate.path, onWarning); + if (entry.tools !== undefined && tools === undefined) continue; + const config: AdvisorConfigEntry = { + name: entry.name.trim(), + model: normalizeOptionalString(entry.model), + tools, + instructions: normalizeOptionalString(entry.instructions), + enabled: entry.enabled, + }; + advisors.set(slugifyAdvisorName(config.name), config); + } + } + + return { + advisors: [...advisors.values()], + sharedInstructions: sharedParts.length > 0 ? sharedParts.join('\n\n') : undefined, + files: [...new Set([...watchdogPaths, ...configFiles])], + }; +} + +interface ConfigCandidate { + readonly path: string; + readonly fileName: string; + readonly content: string; + readonly user: boolean; + readonly depth: number; +} + +async function collectConfigCandidates( + cwd: string, + userHomeDir: string, + fileNames: readonly string[], + onWarning: (message: string, details?: Record) => void, +): Promise { + const resolvedCwd = path.resolve(cwd); + const userDirs = uniquePaths([ + userHomeDir, + path.join(userHomeDir, '.pythinker-code'), + path.join(userHomeDir, '.agents'), + ]); + const candidates: Array<{ readonly path: string; readonly user: boolean; readonly depth: number }> = []; + + for (const directory of userDirs) { + for (const fileName of fileNames) { + candidates.push({ path: path.join(directory, fileName), user: true, depth: -1 }); + } + } + + const projectRoot = await findProjectRoot(resolvedCwd); + const projectDirs: string[] = []; + let current = resolvedCwd; + while (true) { + projectDirs.push(current); + if (current === projectRoot) break; + const parent = path.dirname(current); + if (parent === current) break; + current = parent; + } + projectDirs.reverse(); + for (const [depth, directory] of projectDirs.entries()) { + for (const fileName of fileNames) { + candidates.push( + { path: path.join(directory, fileName), user: false, depth }, + { path: path.join(directory, '.omp', fileName), user: false, depth }, + ); + } + } + + const unique = new Map(); + for (const candidate of candidates) unique.set(path.resolve(candidate.path), candidate); + const results = await Promise.all( + [...unique.values()].map(async (candidate) => { + try { + const content = await readFile(candidate.path, 'utf8'); + return { candidate, content }; + } catch (error) { + if (isMissingFile(error)) return undefined; + return { candidate, error }; + } + }), + ); + const readable: ConfigCandidate[] = []; + for (const result of results) { + if (result === undefined) continue; + if ('error' in result) { + onWarning('Advisor config could not be read', { + path: result.candidate.path, + error: result.error instanceof Error ? result.error.message : String(result.error), + }); + continue; + } + readable.push({ + ...result.candidate, + path: path.resolve(result.candidate.path), + fileName: path.basename(result.candidate.path), + content: result.content, + }); + } + readable.sort((left, right) => { + if (left.user !== right.user) return left.user ? -1 : 1; + return left.depth - right.depth; + }); + return readable; +} + +function normalizeTools( + value: unknown, + sourcePath: string, + onWarning: (message: string, details?: Record) => void, +): readonly string[] | undefined { + if (value === undefined) return undefined; + if (!Array.isArray(value)) { + onWarning('Advisor config tools must be a YAML list', { path: sourcePath }); + return undefined; + } + const tools: string[] = []; + for (const item of value) { + if (typeof item !== 'string' || item.trim().length === 0) { + onWarning('Advisor config tool names must be non-empty strings', { path: sourcePath }); + return undefined; + } + const name = item.trim(); + if (!tools.includes(name)) tools.push(name); + } + return tools; +} + +function normalizeOptionalString(value: unknown): string | undefined { + return typeof value === 'string' && value.trim().length > 0 ? value.trim() : undefined; +} + +function uniquePaths(paths: readonly string[]): string[] { + return [...new Set(paths.map((entry) => path.resolve(entry)))]; +} + +function isMissingFile(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + error.code === 'ENOENT' + ); +} \ No newline at end of file diff --git a/packages/agent-core/src/session/index.ts b/packages/agent-core/src/session/index.ts index 491cb6bb..967c3e9b 100644 --- a/packages/agent-core/src/session/index.ts +++ b/packages/agent-core/src/session/index.ts @@ -183,6 +183,7 @@ export interface SessionSkillConfig { export interface AgentMeta { readonly type: AgentType; readonly parentAgentId: string | null; + readonly emitEvents?: boolean; readonly dynamicWorkflowItem?: string; } @@ -193,6 +194,8 @@ export interface CreateAgentOptions { readonly parentAgentId?: string; readonly dynamicWorkflowItem?: string; readonly persistMetadata?: boolean; + /** Whether this agent forwards events to the session RPC. Defaults to true. */ + readonly emitEvents?: boolean; } export interface SessionMeta { @@ -215,6 +218,7 @@ const AgentMetaSchema = z type: z.enum(['main', 'sub', 'independent']), parentAgentId: z.string().nullable(), dynamicWorkflowItem: z.string().optional(), + emitEvents: z.boolean().optional(), }) .strict(); @@ -550,6 +554,7 @@ export class Session { Array.from(this.readyAgents(), async (agent) => agent.cron?.stop()), ); await this.cancelActiveTurnsOnClose(); + await this.advisor.close(); await this.stopBackgroundTasksOnExit(); await this.flushMetadata(); await this.triggerSessionEnd('exit'); @@ -569,6 +574,7 @@ export class Session { await Promise.allSettled( Array.from(this.readyAgents(), async (agent) => agent.cron?.stop()), ); + await this.advisor.close(); await this.flushMetadata(); } finally { try { @@ -708,7 +714,14 @@ export class Session { const id = type === 'main' ? 'main' : this.nextGeneratedAgentId(); const homedir = this.agentDir(id); const parentAgentId = options.parentAgentId ?? null; - const agent = this.instantiateAgent(id, homedir, type, config, parentAgentId); + const agent = this.instantiateAgent( + id, + homedir, + type, + config, + parentAgentId, + options.emitEvents !== false, + ); if (options.profile) { await this.bootstrapAgentProfile( agent, @@ -723,6 +736,7 @@ export class Session { type, parentAgentId, dynamicWorkflowItem: options.dynamicWorkflowItem, + emitEvents: options.emitEvents !== false, }; void this.writeMetadata(); } @@ -1301,9 +1315,12 @@ export class Session { type: AgentType, config: Partial = {}, parentAgentId: string | null = null, + emitEvents = true, ): Agent { const parentAgent = parentAgentId !== null ? this.getReadyAgent(parentAgentId) : undefined; const cwd = parentAgent?.config.cwd ?? this.toolKaos.getcwd(); + const rpc = proxyWithExtraPayload(this.rpc, { agentId: id }); + const agentRpc = emitEvents ? rpc : { ...rpc, emitEvent: async () => {} }; const agent = new Agent({ ...config, type, @@ -1312,7 +1329,7 @@ export class Session { config: this.options.config, homedir, skills: this.skills, - rpc: proxyWithExtraPayload(this.rpc, { agentId: id }), + rpc: agentRpc, modelProvider: this.options.providerManager, hookEngine: config.hookEngine ?? this.hookEngine, subagentHost: config.subagentHost ?? new SessionSubagentHost(this, id), @@ -1414,7 +1431,14 @@ export class Session { } try { - const agent = this.instantiateAgent(id, this.agentDir(id), meta.type, {}, parentAgentId); + const agent = this.instantiateAgent( + id, + this.agentDir(id), + meta.type, + {}, + parentAgentId, + meta.emitEvents !== false, + ); await agent.resume(); this.agents.set(id, agent); return agent; diff --git a/packages/agent-core/src/session/rpc.ts b/packages/agent-core/src/session/rpc.ts index f56c6f19..d2572599 100644 --- a/packages/agent-core/src/session/rpc.ts +++ b/packages/agent-core/src/session/rpc.ts @@ -2,6 +2,7 @@ import { ErrorCodes, PythinkerError } from '#/errors'; import { convertMCPContentBlock } from '#/mcp/output'; import type { ActivateSkillPayload, + AdvisorStatus, AgentAPI, BeginCompactionPayload, CancelPayload, @@ -20,6 +21,7 @@ import type { RegisterToolPayload, SessionAPI, SetActiveToolsPayload, + SetAdvisorEnabledPayload, SetFastModePayload, SetModelPayload, SetPermissionPayload, @@ -97,6 +99,17 @@ export class SessionAPIImpl implements PromisableMethods { listSkills(_payload: EmptyPayload): Promise { return this.session.listSkills(); } + getAdvisorStatus(_payload: EmptyPayload): Promise { + return this.session.advisor.status(); + } + + setAdvisorEnabled(payload: SetAdvisorEnabledPayload): Promise { + return this.session.advisor.setEnabled(payload.enabled, payload.advisorId); + } + + reloadAdvisor(_payload: EmptyPayload): Promise { + return this.session.advisor.reload(); + } reloadSkills(_payload: EmptyPayload): Promise { return this.session.reloadSkills(); diff --git a/packages/agent-core/src/session/session-advisor.ts b/packages/agent-core/src/session/session-advisor.ts index f75724c8..0427957f 100644 --- a/packages/agent-core/src/session/session-advisor.ts +++ b/packages/agent-core/src/session/session-advisor.ts @@ -1,7 +1,26 @@ -import type { PromptOrigin } from '../agent/context'; +import { appendFile, mkdir, readFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +import { isProviderRateLimitError } from '@pymodel/kosong'; + +import type { Agent } from '../agent'; +import type { ContextMessage, PromptOrigin } from '../agent/context'; import { InMemoryAgentRecordPersistence } from '../agent/records'; import { expandModelRef, resolveModelRoleAlias } from '../config/model-roles'; +import type { AgentEvent } from '../rpc'; import { HookEngine } from './hooks'; +import { escapeXml, escapeXmlAttr } from '../utils/xml-escape'; +import { abortError } from '../utils/abort'; +import { trimTrailingOpenToolExchange } from '../agent/context/projector'; +import { + discoverAdvisorConfigs, + slugifyAdvisorName, + type AdvisorConfigEntry, + type AdvisorRuntimeStatus, + type AdvisorStatusSnapshot, + type DiscoveredAdvisors, +} from './advisor-config'; import type { Session } from '.'; const ADVISOR_SYSTEM_PROMPT = @@ -25,21 +44,58 @@ const ADVISOR_OUTPUT_SCHEMA = { }, }, } as const; +const DEFAULT_ADVISOR_TOOLS = ['Read', 'Grep', 'Glob'] as const; +const ADVISOR_FAILURE_LIMIT = 3; +const ADVISOR_TIMEOUT_MS = 120_000; + +type AdvisorStatusEvent = Extract; interface AdvisoryNote { readonly note: string; readonly severity?: 'nit' | 'concern' | 'blocker'; } +interface AdvisorRuntimeState { + readonly id: string; + persistent: boolean; + config: AdvisorConfigEntry; + enabledOverride: boolean | undefined; + running: boolean; + failures: number; + notes: number; + costUsd: number; + lastUsageCostUsd: number; + historyCursor: number; + historyRevision: number; + status: AdvisorRuntimeStatus; + message: string | undefined; + pendingAdvisory: string | undefined; + agent: Agent | undefined; + transcriptLoaded: boolean; + warnedCrossProvider: boolean; +} + +interface AdvisorTranscriptRecord { + readonly type: 'review'; + readonly at: string; + readonly notes: readonly AdvisoryNote[]; + readonly costUsd: number; +} + export class SessionAdvisor { + #discoveryPromise: Promise; + #runtimeStates = new Map(); + #activeAgents = new Set(); + #closing = false; #running = false; - #disabled = false; - #warnedCrossProvider = false; - #consecutiveFailures = 0; #reviewCurrentTurn = false; - #pendingAdvisory: string | undefined; - - constructor(private readonly session: Session) {} + #globalEnabled: boolean | undefined; + #warnedDiscoveryFailure = false; + #writeQueue = Promise.resolve(); + constructor(private readonly session: Session) { + this.#globalEnabled = session.options.config?.advisor?.enabled === false ? false : undefined; + this.#discoveryPromise = this.#discover(); + } /** Called when a main-agent turn starts. Delivers notes without starting a new turn. */ onMainTurnStarted(origin: PromptOrigin): void { @@ -58,131 +114,582 @@ export class SessionAdvisor { onMainTurnEnded(): void { const shouldReview = this.#reviewCurrentTurn; this.#reviewCurrentTurn = false; - if (!shouldReview) return; - if (this.#running || this.#disabled) return; + if (this.#closing || !shouldReview || this.#running) return; this.#running = true; - void this.#run() - .catch((error: unknown) => this.#recordFailure(error)) - .finally(() => { - this.#running = false; - }); + const run = this.#runConfiguredAdvisors(); + void run.finally(() => { + this.#running = false; + }); } - async #run(): Promise { - const config = this.session.options.config; - if (config?.advisor?.enabled !== true) return; + /** Return current advisor status after loading WATCHDOG configuration and transcripts. */ + async status(): Promise { + await this.#ensureRuntimeStates(); + return this.#snapshotStatuses(); + } + + /** Reload WATCHDOG files and preserve runtime state for unchanged advisor ids. */ + async reload(): Promise { + this.#discoveryPromise = this.#discover(); + await this.#ensureRuntimeStates(); + return this.#snapshotStatuses(); + } + /** Enable or disable all advisors, or one named advisor, without changing config files. */ + async setEnabled(enabled: boolean, advisorId?: string): Promise { + await this.#ensureRuntimeStates(); + if (advisorId !== undefined && !this.#runtimeStates.has(advisorId)) { + throw new Error(`Advisor "${advisorId}" was not found`); + } + if (advisorId === undefined) { + this.#globalEnabled = enabled; + for (const state of this.#runtimeStates.values()) { + state.enabledOverride = enabled ? undefined : false; + if (enabled) this.#resetAfterManualEnable(state); + else this.#setStatus(state, 'paused', 'Disabled by user'); + } + } else { + const state = this.#runtimeStates.get(advisorId); + if (state !== undefined) { + state.enabledOverride = enabled; + if (enabled) this.#resetAfterManualEnable(state); + else this.#setStatus(state, 'paused', 'Disabled by user'); + } + } + return this.#snapshotStatuses(); + } + + /** Stop persistent advisor agents and flush their JSONL transcripts. */ + async close(): Promise { + this.#closing = true; + for (const agent of this.#activeAgents) { + agent.turn.cancel(undefined, abortError('Session closed')); + } + // Cancellation is best effort. Do not block session shutdown on a provider + // that ignores the abort signal. + for (const state of this.#runtimeStates.values()) { + if (state.agent !== undefined) this.session.agents.delete(state.agent.agentId); + state.agent = undefined; + } + await this.#writeQueue; + } + + async #discover(): Promise { + try { + return await discoverAdvisorConfigs( + this.session.options.kaos.getcwd(), + this.session.options.skills?.userHomeDir ?? homedir(), + (message, details) => this.session.log.warn(message, details), + ); + } catch (error) { + if (!this.#warnedDiscoveryFailure) { + this.#warnedDiscoveryFailure = true; + this.session.log.warn('advisor configuration discovery failed', { error }); + } + return { advisors: [], files: [] }; + } + } + async #runConfiguredAdvisors(): Promise { + if (this.#closing) return; const main = this.session.getReadyAgent('main'); if (main === undefined) return; - const advisorAlias = - config.advisor.model === undefined - ? resolveModelRoleAlias(config, 'advisor') - : expandModelRef(config, config.advisor.model); - if (!main.config.canResolveModel(advisorAlias) || advisorAlias === undefined) return; + const startedIds = new Set(); + try { + const discovered = await this.#ensureRuntimeStates(); + const states = [...this.#runtimeStates.values()]; + if (this.#closing || !states.some((state) => this.#isEnabled(state))) return; + await this.#runStates(states, discovered.sharedInstructions, main, startedIds); + } catch (error) { + this.session.log.debug('advisor configuration run failed', { error }); + } + } + + async #runStates( + states: readonly AdvisorRuntimeState[], + sharedInstructions: string | undefined, + main: Agent, + startedIds: Set, + ): Promise { + await Promise.all( + states.map(async (state) => { + if (this.#closing || startedIds.has(state.id) || state.running || !this.#isEnabled(state)) return; + startedIds.add(state.id); + state.running = true; + try { + await this.#runOne(state, sharedInstructions, main); + } catch (error) { + if (!this.#closing) this.#recordFailure(state, error); + } finally { + state.running = false; + if (state.status === 'running' && !this.#isEnabled(state)) { + this.#setStatus(state, 'paused', 'Disabled by user'); + } + } + }), + ); + } + + async #runOne( + state: AdvisorRuntimeState, + sharedInstructions: string | undefined, + main: Agent, + ): Promise { + const config = this.session.options.config; + const advisorAlias = this.#resolveAdvisorAlias(state.config, config); + if (advisorAlias === undefined || !main.config.canResolveModel(advisorAlias)) { + this.#setStatus(state, 'no_model', 'No resolvable advisor model'); + return; + } const mainAlias = main.config.modelAlias; - if (mainAlias === undefined) return; - const advisorProvider = config.models?.[advisorAlias]?.provider ?? config.defaultProvider; - const mainProvider = config.models?.[mainAlias]?.provider ?? config.defaultProvider; + if (mainAlias === undefined) { + this.#setStatus(state, 'no_model', 'Main model is not configured'); + return; + } + const advisorProvider = config?.models?.[advisorAlias]?.provider ?? config?.defaultProvider; + const mainProvider = config?.models?.[mainAlias]?.provider ?? config?.defaultProvider; if (advisorProvider !== mainProvider) { - if (!this.#warnedCrossProvider) { - this.#warnedCrossProvider = true; + if (!state.warnedCrossProvider) { + state.warnedCrossProvider = true; this.session.log.warn('advisor skipped because its provider differs from the main model', { advisorProvider, mainProvider, + advisor: state.id, }); } + this.#setStatus(state, 'paused', 'Advisor provider differs from the main provider'); return; } - let id: string | undefined; + let child: Agent; + let childId: string | undefined; + let activeChild: Agent | undefined; + let usageRecorded = false; + let runCost = 0; try { - const created = await this.session.createAgent( - { - type: 'sub', - generate: main.rawGenerate, - persistence: new InMemoryAgentRecordPersistence(), - hookEngine: new HookEngine(), - }, - { parentAgentId: main.agentId, persistMetadata: false }, - ); - id = created.id; - const child = created.agent; + if (state.agent !== undefined) { + child = state.agent; + } else { + const created = await this.session.createAgent( + { + type: 'sub', + generate: main.rawGenerate, + persistence: new InMemoryAgentRecordPersistence(), + hookEngine: new HookEngine(), + }, + { + parentAgentId: main.agentId, + persistMetadata: false, + emitEvents: false, + }, + ); + if (this.#closing) { + this.session.agents.delete(created.id); + return; + } + childId = created.id; + child = created.agent; + if (state.persistent) state.agent = child; + } + activeChild = child; + + this.#activeAgents.add(child); + if (this.#closing) return; + child.config.update({ modelAlias: advisorAlias, thinkingLevel: 'off', - systemPrompt: - ADVISOR_SYSTEM_PROMPT + - (config.advisor.instructions === undefined - ? '' - : `\n\n${config.advisor.instructions}`), + systemPrompt: this.#systemPrompt(state.config, sharedInstructions), }); - child.tools.setActiveTools([]); - child.context.useProjectedHistoryFrom(main.context); + this.#setAdvisorTools( + child, + state.config.tools ?? (state.persistent ? DEFAULT_ADVISOR_TOOLS : []), + ); + this.#appendReviewContext(state, child, main); + if (this.#closing) return; + const turnId = child.turn.prompt( [{ type: 'text', text: ADVISOR_USER_PROMPT }], { kind: 'system_trigger', name: 'advisor' }, ADVISOR_OUTPUT_SCHEMA, ); if (turnId === null) throw new Error('Advisor turn could not start.'); - const result = await child.turn.waitForCurrentTurn(AbortSignal.timeout(120_000)); + const result = await child.turn.waitForCurrentTurn(AbortSignal.timeout(ADVISOR_TIMEOUT_MS)); + runCost = this.#recordUsageCost(state, child); + usageRecorded = true; + if (this.#closing) return; if (result.event.reason !== 'completed') { throw new Error('Advisor turn did not complete.'); } const notes = parseNotes(result.event.structuredOutput); - this.#consecutiveFailures = 0; - if (notes.length === 0) return; - - const lines = notes.map(({ note, severity }) => - severity === undefined ? `- ${note}` : `- [${severity}] ${note}`, + state.failures = 0; + this.#setStatus(state, 'running'); + state.notes += notes.length; + const block = formatAdvisory( + notes, + state.persistent ? state.config.name : undefined, ); - const block = [ - '', - 'The following notes are from a second reviewing model. Weigh them; do not blindly obey.', - ...lines, - '', - ].join('\n'); - this.#pendingAdvisory = block; + if (block !== undefined) state.pendingAdvisory = block; + this.#appendTranscript(state, { type: 'review', at: new Date().toISOString(), notes, costUsd: runCost }); } finally { - if (id !== undefined) this.session.agents.delete(id); + if (activeChild !== undefined) { + if (!usageRecorded) this.#recordUsageCost(state, activeChild); + this.#activeAgents.delete(activeChild); + } + if (childId !== undefined && !state.persistent) this.session.agents.delete(childId); + } + } + + #appendReviewContext(state: AdvisorRuntimeState, child: Agent, main: Agent): void { + if (!state.persistent) { + child.context.useProjectedHistoryFrom(main.context); + state.historyCursor = main.context.history.length; + state.historyRevision = main.context.historyRevision; + return; + } + const history = main.context.history; + const historyRevision = main.context.historyRevision; + if (state.historyRevision !== historyRevision || state.historyCursor > history.length) { + child.context.clear(); + state.historyCursor = 0; + state.historyRevision = historyRevision; + } + if (state.historyCursor === 0) { + child.context.useProjectedHistoryFrom(main.context); + state.historyCursor = trailingOpenToolExchangeStart(history) ?? history.length; + state.historyRevision = historyRevision; + return; } + const pending = history.slice(state.historyCursor); + const openExchangeStart = trailingOpenToolExchangeStart(pending); + const consumable = openExchangeStart === undefined + ? pending + : pending.slice(0, openExchangeStart); + const projected = trimTrailingOpenToolExchange( + main.context.project(consumable, state.historyCursor), + ); + for (const message of projected) { + child.context.appendMessage(message); + } + state.historyCursor += consumable.length; + state.historyRevision = historyRevision; + } + + #resolveAdvisorAlias(entry: AdvisorConfigEntry, config: NonNullable | undefined): string | undefined { + if (entry.model !== undefined) return expandModelRef(config, entry.model); + return resolveModelRoleAlias(config, 'advisor'); + } + + #systemPrompt(entry: AdvisorConfigEntry, sharedInstructions: string | undefined): string { + const parts = [ADVISOR_SYSTEM_PROMPT, sharedInstructions, entry.instructions] + .filter((part): part is string => part !== undefined && part.trim().length > 0) + .map((part) => part.trim()); + return parts.join('\n\n'); + } + + #setAdvisorTools(child: Agent, configuredTools: readonly string[] | undefined): void { + const requested = configuredTools ?? DEFAULT_ADVISOR_TOOLS; + const available = new Set(child.tools.data().map((tool) => tool.name)); + child.tools.setActiveTools(requested.filter((tool) => available.has(tool))); + } + #recordUsageCost(state: AdvisorRuntimeState, child: Agent): number { + const childCostAfter = child.usage.data().totalCostUsd; + const previousCost = state.persistent ? state.lastUsageCostUsd : 0; + const runCost = + childCostAfter === undefined ? 0 : Math.max(0, childCostAfter - previousCost); + state.lastUsageCostUsd = state.persistent + ? childCostAfter ?? state.lastUsageCostUsd + : 0; + state.costUsd += runCost; + return runCost; } - #recordFailure(error: unknown): void { - this.#consecutiveFailures += 1; + #recordFailure(state: AdvisorRuntimeState, error: unknown): void { + state.failures += 1; + state.message = error instanceof Error ? error.message : String(error); + state.status = isProviderRateLimitError(error) ? 'quota_exhausted' : 'error'; + this.#emitStatus(state); this.session.log.debug('advisor run failed', { error }); - if (this.#consecutiveFailures < 3) return; - this.#disabled = true; + if (state.failures < ADVISOR_FAILURE_LIMIT) return; + state.enabledOverride = false; + this.#setStatus(state, 'paused', 'Disabled after three consecutive failures'); this.session.log.warn('advisor disabled after three consecutive failures'); } #deliverPending(): void { const main = this.session.getReadyAgent('main'); - if (this.#pendingAdvisory === undefined || main?.turn.hasActiveTurn !== true) return; - const block = this.#pendingAdvisory; - this.#pendingAdvisory = undefined; + if (main?.turn.hasActiveTurn !== true) return; + const states = [...this.#runtimeStates.values()].filter( + (state) => state.pendingAdvisory !== undefined, + ); + if (states.length === 0) return; + const block = states.map((state) => state.pendingAdvisory).join('\n\n'); main.turn.steer([{ type: 'text', text: block }], { kind: 'hook_result', event: 'advisor', }); + for (const state of states) state.pendingAdvisory = undefined; + } + + async #ensureRuntimeStates(): Promise< + DiscoveredAdvisors & { + readonly advisors: readonly (AdvisorConfigEntry & { readonly id: string })[]; + } + > { + const discovered = await this.#discoveryPromise; + const config = this.session.options.config; + const entries: AdvisorConfigEntry[] = []; + entries.push(...discovered.advisors); + if (entries.length === 0 && config?.advisor !== undefined) { + entries.push({ + name: 'Advisor', + model: config.advisor.model, + tools: config.advisor.tools, + instructions: config.advisor.instructions, + enabled: config.advisor.enabled ?? false, + }); + } + + const currentIds = new Set(); + const persistent = discovered.advisors.length > 0; + for (const entry of entries) { + const id = slugifyAdvisorName(entry.name); + currentIds.add(id); + const existing = this.#runtimeStates.get(id); + if (existing === undefined) { + const state: AdvisorRuntimeState = { + id, + persistent, + config: entry, + enabledOverride: undefined, + running: false, + failures: 0, + notes: 0, + costUsd: 0, + lastUsageCostUsd: 0, + historyCursor: 0, + historyRevision: 0, + status: this.#isGloballyEnabled() && entry.enabled !== false ? 'running' : 'paused', + message: undefined, + pendingAdvisory: undefined, + agent: undefined, + transcriptLoaded: false, + warnedCrossProvider: false, + }; + this.#runtimeStates.set(id, state); + await this.#loadTranscript(state); + this.#emitStatus(state); + continue; + } + const wasEnabled = this.#isEnabled(existing); + const configChanged = advisorConfigChanged(existing.config, entry); + existing.config = entry; + const isEnabled = this.#isEnabled(existing); + if (wasEnabled !== isEnabled) { + if (!isEnabled) { + this.#setStatus(existing, 'paused', 'Disabled by configuration'); + } else if (existing.status === 'paused') { + this.#setStatus(existing, 'running'); + } else { + this.#emitStatus(existing); + } + } else if (configChanged) { + this.#emitStatus(existing); + } + if (existing.persistent !== persistent) { + if (existing.agent !== undefined) this.session.agents.delete(existing.agent.agentId); + existing.agent = undefined; + existing.historyCursor = 0; + existing.historyRevision = 0; + existing.persistent = persistent; + } + await this.#loadTranscript(existing); + } + for (const [id, state] of this.#runtimeStates) { + if (currentIds.has(id)) continue; + if (state.agent !== undefined) this.session.agents.delete(state.agent.agentId); + this.#runtimeStates.delete(id); + } + + return { + ...discovered, + advisors: entries.map((entry) => ({ ...entry, id: slugifyAdvisorName(entry.name) })), + }; + } + + async #loadTranscript(state: AdvisorRuntimeState): Promise { + if (state.transcriptLoaded) return; + state.transcriptLoaded = true; + const transcriptPath = this.#transcriptPath(state); + if (transcriptPath === undefined) return; + try { + const content = await readFile(transcriptPath, 'utf8'); + for (const line of content.split('\n')) { + if (line.trim().length === 0) continue; + try { + const record = JSON.parse(line) as unknown; + if (!isTranscriptRecord(record)) continue; + state.notes += record.notes.length; + state.costUsd += record.costUsd; + } catch { + this.session.log.debug('advisor transcript record ignored', { advisor: state.id }); + } + } + } catch (error) { + if (!isMissingFile(error)) { + this.session.log.debug('advisor transcript load failed', { advisor: state.id, error }); + } + } + } + #appendTranscript(state: AdvisorRuntimeState, record: AdvisorTranscriptRecord): void { + const transcriptDirectory = this.#transcriptDirectory(); + const transcriptPath = this.#transcriptPath(state); + if (transcriptDirectory === undefined || transcriptPath === undefined) return; + const line = `${JSON.stringify(record)}\n`; + this.#writeQueue = this.#writeQueue + .then(async () => { + await mkdir(transcriptDirectory, { recursive: true }); + await appendFile(transcriptPath, line, 'utf8'); + }) + .catch((error) => { + this.session.log.debug('advisor transcript write failed', { advisor: state.id, error }); + }); + } + #transcriptDirectory(): string | undefined { + const homedir = this.session.options.homedir; + return homedir === undefined ? undefined : join(homedir, 'advisors'); + } + #transcriptPath(state: AdvisorRuntimeState): string | undefined { + const directory = this.#transcriptDirectory(); + return directory === undefined ? undefined : join(directory, `${state.id}.jsonl`); + } + + #isGloballyEnabled(): boolean { + return this.#globalEnabled ?? this.session.options.config?.advisor?.enabled !== false; + } + + #isEnabled(state: AdvisorRuntimeState): boolean { + if (state.enabledOverride !== undefined) return state.enabledOverride; + if (this.#globalEnabled !== undefined) return this.#globalEnabled; + return state.config.enabled !== false; + } + + #resetAfterManualEnable(state: AdvisorRuntimeState): void { + state.failures = 0; + state.message = undefined; + if (state.status !== 'no_model') this.#setStatus(state, 'running'); + } + + #setStatus(state: AdvisorRuntimeState, status: AdvisorRuntimeStatus, message?: string): void { + state.status = status; + state.message = message; + this.#emitStatus(state); + } + #emitStatus(state: AdvisorRuntimeState): void { + const event: AdvisorStatusEvent = { + type: 'advisor.status', + advisorId: state.id, + name: state.config.name, + status: state.status, + enabled: this.#isEnabled(state), + model: state.config.model, + message: state.message, + }; + void this.session.rpc.emitEvent({ ...event, agentId: 'main' }).catch((error) => { + this.session.log.debug('advisor status event failed', { error }); + }); + } + + #snapshotStatuses(): readonly AdvisorStatusSnapshot[] { + return [...this.#runtimeStates.values()].map((state) => ({ + id: state.id, + name: state.config.name, + enabled: this.#isEnabled(state), + status: state.status, + model: state.config.model, + failures: state.failures, + notes: state.notes, + costUsd: state.costUsd, + message: state.message, + })); + } +} + + +function trailingOpenToolExchangeStart( + history: readonly ContextMessage[], +): number | undefined { + let assistantIndex = history.length - 1; + while (assistantIndex >= 0 && history[assistantIndex]?.role === 'tool') { + assistantIndex -= 1; } + const assistant = history[assistantIndex]; + if (assistant?.role !== 'assistant' || assistant.toolCalls.length === 0) return undefined; + const toolResultIds = new Set(); + for (const message of history.slice(assistantIndex + 1)) { + if (message.role !== 'tool' || message.toolCallId === undefined) continue; + toolResultIds.add(message.toolCallId); + } + return assistant.toolCalls.every((toolCall) => toolResultIds.has(toolCall.id)) + ? undefined + : assistantIndex; +} + +function advisorConfigChanged( + previous: AdvisorConfigEntry, + next: AdvisorConfigEntry, +): boolean { + return ( + previous.name !== next.name || + previous.model !== next.model || + previous.instructions !== next.instructions || + previous.enabled !== next.enabled || + !sameStringArray(previous.tools, next.tools) + ); +} + +function sameStringArray( + left: readonly string[] | undefined, + right: readonly string[] | undefined, +): boolean { + if (left === right) return true; + if (left === undefined || right === undefined || left.length !== right.length) return false; + return left.every((value, index) => value === right[index]); +} + +function formatAdvisory( + notes: readonly AdvisoryNote[], + advisorName?: string, +): string | undefined { + if (notes.length === 0) return undefined; + const source = advisorName === undefined ? '' : ` advisor="${escapeXmlAttr(advisorName)}"`; + const lines = notes.map(({ note, severity }) => { + const renderedNote = escapeXml(note); + return severity === undefined + ? `- ${renderedNote}` + : `- [${severity}] ${renderedNote}`; + }); + return [ + ``, + 'The following notes are from a second reviewing model. Weigh them; do not blindly obey.', + ...lines, + '', + ].join('\n'); } function parseNotes(output: unknown): AdvisoryNote[] { - if (typeof output !== 'object' || output === null || !Array.isArray((output as { notes?: unknown }).notes)) { - throw new Error('Advisor did not return structured notes.'); + if (!isRecord(output)) throw new Error('Advisor did not return structured notes.'); + const outputNotes = output['notes']; + if (!Array.isArray(outputNotes)) { + throw new TypeError('Advisor did not return structured notes.'); } const notes: AdvisoryNote[] = []; - for (const value of (output as { notes: unknown[] }).notes) { - if (typeof value !== 'object' || value === null) continue; - const { note, severity } = value as { note?: unknown; severity?: unknown }; + for (const value of outputNotes) { + if (!isRecord(value)) continue; + const note = value['note']; + const severity = value['severity']; if (typeof note !== 'string') continue; - if ( - severity !== undefined && - severity !== 'nit' && - severity !== 'concern' && - severity !== 'blocker' - ) { + if (severity !== undefined && severity !== 'nit' && severity !== 'concern' && severity !== 'blocker') { continue; } notes.push({ note: Array.from(note.trim()).slice(0, 500).join(''), severity }); @@ -190,3 +697,21 @@ function parseNotes(output: unknown): AdvisoryNote[] { } return notes; } + +function isTranscriptRecord(value: unknown): value is AdvisorTranscriptRecord { + return ( + isRecord(value) && + value['type'] === 'review' && + typeof value['at'] === 'string' && + Array.isArray(value['notes']) && + typeof value['costUsd'] === 'number' + ); +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isMissingFile(error: unknown): boolean { + return isRecord(error) && error['code'] === 'ENOENT'; +} diff --git a/packages/agent-core/test/agent/compaction/full.test.ts b/packages/agent-core/test/agent/compaction/full.test.ts index 95513dbd..232da85e 100644 --- a/packages/agent-core/test/agent/compaction/full.test.ts +++ b/packages/agent-core/test/agent/compaction/full.test.ts @@ -393,6 +393,35 @@ describe('FullCompaction', () => { expect(messageText(compactionCall?.history[2])).toBe('[Old tool result content cleared]'); expect(messageText(compactionCall?.history[5])).toBe('lookup result'); }); + it('uses global history offsets when projecting a from compaction range', async () => { + enableMicroCompactionFlag(); + const ctx = testAgent({ + microCompaction: { + keepRecentMessages: 0, + minContentTokens: 1, + }, + }); + ctx.configure({ + provider: CATALOGUED_PROVIDER, + modelCapabilities: CATALOGUED_MODEL_CAPABILITIES, + }); + ctx.appendExchange(1, 'old user one', 'old assistant one', 20); + ctx.appendToolExchange(); + const longToolResult = 'lookup result '.repeat(100); + const toolResult = ctx.agent.context.history.at(-1); + if (toolResult?.role !== 'tool') throw new Error('Expected a tool result.'); + toolResult.content[0] = { type: 'text', text: longToolResult }; + ctx.appendExchange(3, 'recent user two', 'recent assistant two', 40); + ctx.agent.microCompaction.apply(3); + const compacted = ctx.once('context.apply_compaction'); + + ctx.mockNextResponse({ type: 'text', text: 'Compacted summary.' }); + await ctx.rpc.beginCompaction({ promptFromEnd: 2, direction: 'from' } as never); + await compacted; + + const [compactionCall] = ctx.llmCalls; + expect(messageText(compactionCall?.history[2])).toBe(longToolResult); + }); it('fires PreCompact and PostCompact hooks from the compaction module', async () => { const dir = mkdtempSync(join(tmpdir(), 'pythinker-compact-hooks-')); diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index e0107e2b..b6aa60da 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -578,6 +578,32 @@ describe('Agent context', () => { ); }); + it('tracks revisions for context rewrites without changing on append', () => { + const ctx = testAgent(); + ctx.configure(); + + const initialRevision = ctx.agent.context.historyRevision; + ctx.agent.context.appendUserMessage([{ type: 'text', text: 'append' }]); + expect(ctx.agent.context.historyRevision).toBe(initialRevision); + + ctx.agent.context.clear(); + const afterClear = ctx.agent.context.historyRevision; + expect(afterClear).toBe(initialRevision + 1); + + ctx.agent.context.appendUserMessage([{ type: 'text', text: 'undo' }]); + ctx.agent.context.undo(1); + const afterUndo = ctx.agent.context.historyRevision; + expect(afterUndo).toBe(afterClear + 1); + + ctx.agent.context.appendUserMessage([{ type: 'text', text: 'compact' }]); + ctx.agent.context.applyCompaction({ + summary: 'summary', + compactedCount: 1, + tokensBefore: 10, + tokensAfter: 1, + }); + expect(ctx.agent.context.historyRevision).toBe(afterUndo + 1); + }); it('undo only counts real user prompts, skipping background notifications', () => { const ctx = testAgent(); ctx.configure(); diff --git a/packages/agent-core/test/session/advisor-config.test.ts b/packages/agent-core/test/session/advisor-config.test.ts new file mode 100644 index 00000000..7930cb37 --- /dev/null +++ b/packages/agent-core/test/session/advisor-config.test.ts @@ -0,0 +1,127 @@ +import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'pathe'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { discoverAdvisorConfigs, slugifyAdvisorName } from '../../src/session/advisor-config'; + +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); +}); + +describe('advisor configuration discovery', () => { + it('merges user and project watchdog files with leaf project precedence', async () => { + const userHome = await makeTempDir('pythinker-advisor-user-'); + const project = await makeTempDir('pythinker-advisor-project-'); + const cwd = join(project, 'packages', 'app'); + await mkdir(cwd, { recursive: true }); + await mkdir(join(project, '.git')); + await writeFile( + join(userHome, 'WATCHDOG.yml'), + [ + 'instructions: Check security boundaries.', + 'advisors:', + ' - name: Security', + ' model: small', + ' tools: [Read]', + ].join('\n'), + ); + await writeFile(join(project, 'WATCHDOG.md'), 'Treat generated files as untrusted evidence.'); + await writeFile( + join(cwd, 'WATCHDOG.yaml'), + [ + 'advisors:', + ' - name: Security', + ' model: reviewer', + ' enabled: false', + ' - name: Performance', + ' model: fast', + ].join('\n'), + ); + + const result = await discoverAdvisorConfigs(cwd, userHome); + + expect(result.advisors).toEqual([ + { name: 'Security', model: 'reviewer', tools: undefined, instructions: undefined, enabled: false }, + { name: 'Performance', model: 'fast', tools: undefined, instructions: undefined, enabled: undefined }, + ]); + expect(result.sharedInstructions).toContain('Check security boundaries.'); + expect(result.sharedInstructions).toContain('Treat generated files as untrusted evidence.'); + expect(result.files).toEqual( + expect.arrayContaining([join(userHome, 'WATCHDOG.yml'), join(project, 'WATCHDOG.md'), join(cwd, 'WATCHDOG.yaml')]), + ); + }); + it('does not load project watchdog files above the project root', async () => { + const userHome = await makeTempDir('pythinker-advisor-user-'); + const parent = await makeTempDir('pythinker-advisor-parent-'); + const project = join(parent, 'repo'); + const cwd = join(project, 'packages', 'app'); + await mkdir(join(project, '.git'), { recursive: true }); + await mkdir(cwd, { recursive: true }); + await writeFile( + join(parent, 'WATCHDOG.yml'), + ['advisors:', ' - name: Outside', ' model: outside'].join('\n'), + ); + await writeFile( + join(project, 'WATCHDOG.yml'), + ['advisors:', ' - name: Inside', ' model: inside'].join('\n'), + ); + + const result = await discoverAdvisorConfigs(cwd, userHome); + + expect(result.advisors.map((advisor) => advisor.name)).toEqual(['Inside']); + expect(result.files).toEqual([join(project, 'WATCHDOG.yml')]); + }); + + it('reports malformed entries and keeps valid advisors', async () => { + const userHome = await makeTempDir('pythinker-advisor-user-'); + const project = await makeTempDir('pythinker-advisor-project-'); + const warnings: string[] = []; + await writeFile( + join(project, 'WATCHDOG.yml'), + [ + 'advisors:', + ' - name: Valid', + ' tools: [Read, Read]', + ' - model: missing-name', + ' - name: BadTools', + ' tools: [Read, 3]', + ].join('\n'), + ); + + const result = await discoverAdvisorConfigs(project, userHome, (message) => warnings.push(message)); + + expect(result.advisors).toEqual([ + { name: 'Valid', model: undefined, tools: ['Read'], instructions: undefined, enabled: undefined }, + ]); + expect(warnings).toEqual([ + 'Advisor config entry requires a name', + 'Advisor config tool names must be non-empty strings', + ]); + }); + it('warns when a config candidate cannot be read', async () => { + const userHome = await makeTempDir('pythinker-advisor-user-'); + const project = await makeTempDir('pythinker-advisor-project-'); + const warnings: string[] = []; + await mkdir(join(project, 'WATCHDOG.yml')); + + await discoverAdvisorConfigs(project, userHome, (message) => warnings.push(message)); + + expect(warnings).toContain('Advisor config could not be read'); + }); + +}); + +it('slugifies advisor names into stable ids', () => { + expect(slugifyAdvisorName(' Security Review / API ')).toBe('security-review-api'); + expect(slugifyAdvisorName('---')).toBe('advisor'); +}); + +async function makeTempDir(prefix: string): Promise { + const directory = await mkdtemp(join(tmpdir(), prefix)); + tempDirs.push(directory); + return directory; +} diff --git a/packages/agent-core/test/session/init.test.ts b/packages/agent-core/test/session/init.test.ts index 7a5f52a1..d6f8afe4 100644 --- a/packages/agent-core/test/session/init.test.ts +++ b/packages/agent-core/test/session/init.test.ts @@ -1,4 +1,4 @@ -import { access, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { access, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'pathe'; @@ -621,6 +621,56 @@ describe('Session.init', () => { await session.close(); }); + it('preserves disabled child event forwarding when resuming persisted agents', async () => { + const workDir = await makeTempDir(); + const sessionDir = await makeTempDir(); + const events: Array> = []; + const session = new Session({ + id: 'test-init-event-forwarding', + kaos: testKaos.withCwd(workDir), + homedir: sessionDir, + rpc: createSessionRpc(events), + skills: { explicitDirs: [join(workDir, 'missing-skills')] }, + providerManager: testProviderManager(), + }); + const enabledChild = await session.createAgent( + { type: 'sub' }, + { profile: testProfile() }, + ); + session.agents.delete(enabledChild.id); + const resumedEnabled = await session.ensureAgentResumed(enabledChild.id); + resumedEnabled.config.update({ modelAlias: 'mock-model', thinkingLevel: 'off' }); + events.length = 0; + resumedEnabled.emitStatusUpdated(); + expect(events).toContainEqual( + expect.objectContaining({ + type: 'agent.status.updated', + agentId: enabledChild.id, + }), + ); + events.length = 0; + + const { id } = await session.createAgent( + { type: 'sub' }, + { profile: testProfile(), emitEvents: false }, + ); + await session.writeMetadata(); + const persisted = JSON.parse(await readFile(join(sessionDir, 'state.json'), 'utf8')) as { + agents: Record; + }; + expect(persisted.agents[id]?.emitEvents).toBe(false); + + session.agents.delete(id); + const resumed = await session.ensureAgentResumed(id); + resumed.config.update({ modelAlias: 'mock-model', thinkingLevel: 'off' }); + events.length = 0; + + resumed.emitStatusUpdated(); + + expect(events).toEqual([]); + await session.close(); + }); + it('runs an isolated system-trigger turn and records the latest AGENTS as a system reminder', async () => { const workDir = await makeTempDir(); const sessionDir = await makeTempDir(); diff --git a/packages/agent-core/test/session/session-advisor.test.ts b/packages/agent-core/test/session/session-advisor.test.ts index 34178f34..6a7a10d0 100644 --- a/packages/agent-core/test/session/session-advisor.test.ts +++ b/packages/agent-core/test/session/session-advisor.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'pathe'; @@ -40,6 +40,69 @@ describe('SessionAdvisor', () => { expect(spawn).not.toHaveBeenCalled(); }); + it('materializes WATCHDOG advisors when the root advisor setting is disabled', async () => { + const fixture = await createFixture({ + advisorAlias: 'advisor', + advisorEnabled: false, + watchdog: ['advisors:', ' - name: Security', ' model: advisor'].join('\n'), + }); + + const statuses = await fixture.session.advisor.setEnabled(true); + + expect(statuses).toMatchObject([{ id: 'security', enabled: true, status: 'running' }]); + }); + it('keeps a root advisor disabled when enabled is omitted', async () => { + const fixture = await createFixture({ + advisorAlias: 'reviewer', + omitAdvisorEnabled: true, + }); + const spawn = vi.spyOn(fixture.session, 'createAgent'); + + const [status] = await fixture.session.advisor.status(); + expect(status).toMatchObject({ id: 'advisor', enabled: false, status: 'paused' }); + + fixture.scripted.mockNextResponse({ type: 'text', text: 'Done.' }); + await runMainTurn(fixture.main); + await flushAsync(); + + expect(spawn).not.toHaveBeenCalled(); + }); + + it('rejects an unknown advisor ID', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + + await expect( + fixture.session.advisor.setEnabled(true, 'securty'), + ).rejects.toThrow('Advisor "securty" was not found'); + }); + it('allows manual enable to override a disabled advisor config', async () => { + const fixture = await createFixture({ + advisorAlias: 'advisor', + watchdog: ['advisors:', ' - name: Advisor', ' model: advisor', ' enabled: false'].join( + '\n', + ), + }); + + const [before] = await fixture.session.advisor.status(); + expect(before).toMatchObject({ id: 'advisor', enabled: false }); + + const [after] = await fixture.session.advisor.setEnabled(true, 'advisor'); + expect(after).toMatchObject({ id: 'advisor', enabled: true, status: 'running' }); + }); + + it('allows per-advisor enable after a global runtime disable', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const spawn = vi.spyOn(fixture.session, 'createAgent'); + + await fixture.session.advisor.setEnabled(false); + const [after] = await fixture.session.advisor.setEnabled(true, 'advisor'); + + expect(after).toMatchObject({ id: 'advisor', enabled: true, status: 'running' }); + queueReview(fixture.scripted, 'Review after a targeted re-enable.', 'concern'); + await runMainTurn(fixture.main); + await waitForAdvisor(fixture); + expect(spawn).toHaveBeenCalledOnce(); + }); it('buffers notes while idle and steers them into the next user turn', async () => { const fixture = await createFixture({ advisorAlias: 'advisor' }); @@ -70,6 +133,25 @@ describe('SessionAdvisor', () => { ], { kind: 'hook_result', event: 'advisor' }, ); + fixture.scripted.mockNextResponse({ type: 'text', text: 'Later turn.' }); + await runMainTurn(fixture.main); + expect(steer).toHaveBeenCalledOnce(); + }); + + + it('does not publish advisor child events to session clients', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const spawn = vi.spyOn(fixture.session, 'createAgent'); + const emitEvent = vi.mocked(fixture.session.rpc.emitEvent); + queueReview(fixture.scripted, 'Keep the review private.', 'concern'); + + await runMainTurn(fixture.main); + await waitForAdvisor(fixture); + + expect(emitEvent.mock.calls.filter(([event]) => event.agentId !== 'main')).toEqual([]); + expect(emitEvent.mock.calls.some(([event]) => event.agentId === 'main')).toBe(true); + const child = (await spawn.mock.results[0]!.value).agent; + expect(child.rpc?.requestApproval).toEqual(expect.any(Function)); }); it('waits until the next turn when a review finishes mid-turn', async () => { @@ -94,6 +176,7 @@ describe('SessionAdvisor', () => { queueReview(fixture.scripted, 'Check the active turn.', 'concern'); await runMainTurn(fixture.main, { kind: 'user' }); + await vi.waitFor(() => expect(fixture.scripted.calls).toHaveLength(2)); queueReview(fixture.scripted); const turnId = fixture.main.turn.prompt( @@ -185,12 +268,476 @@ describe('SessionAdvisor', () => { await runMainTurn(fixture.main); fixture.scripted.mockNextResponse({ type: 'text', text: 'Second.' }); await runMainTurn(fixture.main); - await flushAsync(); + await vi.waitFor(() => expect(warn).toHaveBeenCalledOnce()); expect(spawn).not.toHaveBeenCalled(); expect(warn).toHaveBeenCalledOnce(); expect(steer).not.toHaveBeenCalled(); }); + it('runs multiple configured advisors as persistent reviewers', async () => { + const fixture = await createFixture({ + watchdog: [ + 'advisors:', + ' - name: Security', + ' model: advisor', + ' - name: Performance', + ' model: reviewer', + ].join('\n'), + }); + fixture.scripted.mockNextResponse({ type: 'text', text: 'Main turn complete.' }); + mockAdvisorResponse(fixture.scripted, 'Check auth.', 'concern'); + mockAdvisorResponse(fixture.scripted, 'Check latency.', 'nit'); + + await runMainTurn(fixture.main); + + await vi.waitFor(() => { + expect(fixture.scripted.calls).toHaveLength(3); + expect(fixture.session.agents.size).toBe(3); + }); + const statuses = await fixture.session.advisor.status(); + expect(statuses.map((status) => status.name)).toEqual(['Security', 'Performance']); + expect(statuses.map((status) => status.status)).toEqual(['running', 'running']); + expect(statuses.map((status) => status.notes)).toEqual([1, 1]); + }); + it('pauses a running advisor when reload disables it', async () => { + const fixture = await createFixture({ + watchdog: ['advisors:', ' - name: Security', ' model: advisor', ' enabled: true'].join( + '\n', + ), + }); + await fixture.session.advisor.status(); + const emitEvent = vi.mocked(fixture.session.rpc.emitEvent); + emitEvent.mockClear(); + + await writeFile( + join(fixture.workDir, 'WATCHDOG.yml'), + ['advisors:', ' - name: Security', ' model: advisor', ' enabled: false'].join('\n'), + ); + const [status] = await fixture.session.advisor.reload(); + + expect(status).toMatchObject({ id: 'security', enabled: false, status: 'paused' }); + expect(emitEvent).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'advisor.status', + advisorId: 'security', + enabled: false, + status: 'paused', + }), + ); + }); + + it('resumes a paused advisor when reload enables it', async () => { + const fixture = await createFixture({ + watchdog: ['advisors:', ' - name: Security', ' model: advisor', ' enabled: false'].join( + '\n', + ), + }); + await fixture.session.advisor.status(); + const emitEvent = vi.mocked(fixture.session.rpc.emitEvent); + emitEvent.mockClear(); + + await writeFile( + join(fixture.workDir, 'WATCHDOG.yml'), + ['advisors:', ' - name: Security', ' model: advisor'].join('\n'), + ); + const [status] = await fixture.session.advisor.reload(); + + expect(status).toMatchObject({ id: 'security', enabled: true, status: 'running' }); + expect(emitEvent).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'advisor.status', + advisorId: 'security', + enabled: true, + status: 'running', + }), + ); + }); + + it('emits updated advisor config when reload changes its model', async () => { + const fixture = await createFixture({ + watchdog: ['advisors:', ' - name: Security', ' model: advisor', ' enabled: true'].join( + '\n', + ), + }); + await fixture.session.advisor.status(); + const emitEvent = vi.mocked(fixture.session.rpc.emitEvent); + emitEvent.mockClear(); + + await writeFile( + join(fixture.workDir, 'WATCHDOG.yml'), + ['advisors:', ' - name: Security', ' model: reviewer', ' enabled: true'].join('\n'), + ); + const [status] = await fixture.session.advisor.reload(); + + expect(status).toMatchObject({ id: 'security', enabled: true, status: 'running', model: 'reviewer' }); + expect(emitEvent).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'advisor.status', + advisorId: 'security', + enabled: true, + model: 'reviewer', + status: 'running', + }), + ); + }); + + it('rebuilds persistent advisor context after main history rewrites', async () => { + const fixture = await createFixture({ + watchdog: ['advisors:', ' - name: Security', ' model: advisor'].join('\n'), + }); + queueReview(fixture.scripted, 'First review.', 'concern'); + await runMainTurn(fixture.main); + await vi.waitFor(async () => { + const [status] = await fixture.session.advisor.status(); + expect(status?.notes).toBe(1); + }); + + const historyLength = fixture.main.context.history.length; + fixture.main.context.applyCompaction({ + summary: 'Rewritten context.', + startIndex: 0, + compactedCount: 1, + tokensBefore: 100, + tokensAfter: 1, + }); + expect(fixture.main.context.history).toHaveLength(historyLength); + + queueReview(fixture.scripted, 'Second review.', 'concern'); + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(fixture.scripted.calls).toHaveLength(4)); + expect(JSON.stringify(fixture.scripted.calls[3]?.history)).toContain('Rewritten context.'); + }); + it('keeps tool exchanges paired across persistent incremental reviews', async () => { + const fixture = await createFixture({ + watchdog: ['advisors:', ' - name: Security', ' model: advisor'].join('\n'), + }); + const longToolResult = 'gap result '.repeat(100); + const originalOnMainTurnEnded = fixture.session.advisor.onMainTurnEnded.bind( + fixture.session.advisor, + ); + vi.spyOn(fixture.session.advisor, 'onMainTurnEnded').mockImplementationOnce(() => { + fixture.main.microCompaction.apply(fixture.main.context.history.length); + fixture.main.context.appendLoopEvent({ + type: 'step.begin', + uuid: 'advisor-gap-step', + turnId: 'advisor-gap-turn', + step: 1, + }); + fixture.main.context.appendLoopEvent({ + type: 'tool.call', + uuid: 'advisor-gap-call', + turnId: 'advisor-gap-turn', + step: 1, + stepUuid: 'advisor-gap-step', + toolCallId: 'advisor-gap-call', + name: 'Read', + args: { path: 'src/gap.ts' }, + }); + originalOnMainTurnEnded(); + }); + + queueReview(fixture.scripted, 'First review.', 'concern'); + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(fixture.scripted.calls).toHaveLength(2)); + + fixture.main.context.appendLoopEvent({ + type: 'tool.result', + parentUuid: 'advisor-gap-call', + toolCallId: 'advisor-gap-call', + result: { output: longToolResult }, + }); + fixture.main.context.appendLoopEvent({ + type: 'step.end', + uuid: 'advisor-gap-step', + turnId: 'advisor-gap-turn', + step: 1, + finishReason: 'tool_use', + }); + + queueReview(fixture.scripted, 'Second review.', 'concern'); + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(fixture.scripted.calls).toHaveLength(4)); + + const history = fixture.scripted.calls[3]?.history ?? []; + const toolCallIndex = history.findIndex((message) => + message.toolCalls.some((toolCall) => toolCall.id === 'advisor-gap-call'), + ); + expect(toolCallIndex).toBeGreaterThanOrEqual(0); + expect(history[toolCallIndex + 1]).toMatchObject({ + role: 'tool', + toolCallId: 'advisor-gap-call', + }); + expect(history[toolCallIndex + 1]).toMatchObject({ + content: [{ type: 'text', text: longToolResult }], + }); + }); + it('attributes notes from named watchdog advisors', async () => { + const fixture = await createFixture({ + watchdog: [ + 'advisors:', + ' - name: Security', + ' model: advisor', + ].join('\n'), + }); + const steer = vi.spyOn(fixture.main.turn, 'steer').mockReturnValue(null); + queueReview(fixture.scripted, 'Check & "logging".', 'concern'); + + await runMainTurn(fixture.main); + await vi.waitFor(async () => { + const [status] = await fixture.session.advisor.status(); + expect(status?.notes).toBe(1); + }); + + fixture.scripted.mockNextResponse({ type: 'text', text: 'Next turn.' }); + await runMainTurn(fixture.main); + + await vi.waitFor(() => expect(steer).toHaveBeenCalledOnce()); + expect(steer).toHaveBeenCalledWith( + [ + { + type: 'text', + text: expect.stringContaining( + '\nThe following notes are from a second reviewing model. Weigh them; do not blindly obey.\n- [concern] Check <auth> & "logging".', + ), + }, + ], + { kind: 'hook_result', event: 'advisor' }, + ); + }); + + it('loads persisted transcript records when reporting advisor status', async () => { + const fixture = await createFixture({ + watchdog: [ + 'advisors:', + ' - name: Security', + ' model: advisor', + ].join('\n'), + }); + const transcriptDir = join(fixture.sessionDir, 'advisors'); + await mkdir(transcriptDir, { recursive: true }); + await writeFile( + join(transcriptDir, 'security.jsonl'), + [ + JSON.stringify({ + type: 'review', + at: '2026-08-14T00:00:00.000Z', + notes: [{ note: 'Check auth.' }], + costUsd: 0.1, + }), + JSON.stringify({ + type: 'review', + at: '2026-08-14T00:01:00.000Z', + notes: [{ note: 'Check retries.' }, { note: 'Check timeouts.' }], + costUsd: 0.2, + }), + 'not valid JSON', + ].join('\n'), + ); + + const [status] = await fixture.session.advisor.status(); + + expect(status).toMatchObject({ + id: 'security', + notes: 3, + }); + expect(status?.costUsd).toBeCloseTo(0.3); + }); + + it('keeps shared instructions after status initializes advisors', async () => { + const fixture = await createFixture({ + watchdog: [ + 'instructions: Use the repository test conventions.', + 'advisors:', + ' - name: Security', + ' model: advisor', + ].join('\n'), + }); + await fixture.session.advisor.status(); + const spawn = vi.spyOn(fixture.session, 'createAgent'); + queueReview(fixture.scripted, 'Check auth.', 'concern'); + + await runMainTurn(fixture.main); + await vi.waitFor(() => { + expect(fixture.scripted.calls.length).toBeGreaterThanOrEqual(2); + expect(spawn).toHaveBeenCalledOnce(); + }); + + const child = (await spawn.mock.results[0]!.value).agent; + expect(child.config.systemPrompt).toContain('Use the repository test conventions.'); + }); + it('discovers watchdog advisors before the first configured review', async () => { + const fixture = await createFixture({ + advisorAlias: 'advisor', + watchdog: [ + 'instructions: Use the repository test conventions.', + 'advisors:', + ' - name: Security', + ' model: advisor', + ].join('\n'), + }); + const spawn = vi.spyOn(fixture.session, 'createAgent'); + queueReview(fixture.scripted, 'Check auth.', 'concern'); + + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + + const child = (await spawn.mock.results[0]!.value).agent; + expect(child.config.systemPrompt).toContain('Use the repository test conventions.'); + expect((await fixture.session.advisor.status()).map((status) => status.name)).toEqual(['Security']); + }); + + it('keeps a manual disable applied while a review is in flight', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const gate = createDeferred(); + const originalCreate = fixture.session.createAgent.bind(fixture.session); + vi.spyOn(fixture.session, 'createAgent').mockImplementation(async (...args) => { + const created = await originalCreate(...args); + if (created.agent.type === 'sub') { + const wait = created.agent.turn.waitForCurrentTurn.bind(created.agent.turn); + vi.spyOn(created.agent.turn, 'waitForCurrentTurn').mockImplementation(async (signal) => { + const result = await wait(signal); + await gate.promise; + return result; + }); + } + return created; + }); + queueReview(fixture.scripted, 'Review pending.', 'concern'); + + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(fixture.session.agents.size).toBe(2)); + await fixture.session.advisor.setEnabled(false, 'advisor'); + + gate.resolve(); + await vi.waitFor(async () => { + const [status] = await fixture.session.advisor.status(); + expect(status).toMatchObject({ id: 'advisor', enabled: false, status: 'paused' }); + }); + }); + it('cancels an in-flight advisor before waiting for shutdown', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const advisorStarted = createDeferred(); + const originalGenerate = fixture.main.rawGenerate; + vi.spyOn(fixture.main, 'rawGenerate') + .mockImplementationOnce(originalGenerate) + .mockImplementationOnce(async (...args) => { + const result = await originalGenerate(...args); + const signal = args[5]?.signal; + if (signal === undefined) throw new Error('Advisor test generation signal is missing.'); + advisorStarted.resolve(); + await new Promise((_, reject) => { + const abort = (): void => reject(signal.reason); + if (signal.aborted) { + abort(); + return; + } + signal.addEventListener('abort', abort, { once: true }); + }); + return result; + }); + let advisor: Agent | undefined; + const originalCreate = fixture.session.createAgent.bind(fixture.session); + vi.spyOn(fixture.session, 'createAgent').mockImplementation(async (...args) => { + const created = await originalCreate(...args); + if (created.agent.type === 'sub') advisor = created.agent; + return created; + }); + queueReview(fixture.scripted, 'Review pending.', 'concern'); + + await runMainTurn(fixture.main); + await advisorStarted.promise; + const cancel = vi.spyOn(advisor!.turn, 'cancel'); + let settled = false; + const close = fixture.session.advisor.close().finally(() => { + settled = true; + }); + try { + await Promise.resolve(); + expect(cancel).toHaveBeenCalled(); + await close; + expect(settled).toBe(true); + } finally { + advisor?.turn.cancel(); + await close; + } + }); + it('does not wait for an advisor that ignores cancellation during shutdown', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const advisorStarted = createDeferred(); + const generationGate = createDeferred(); + const originalGenerate = fixture.main.rawGenerate; + vi.spyOn(fixture.main, 'rawGenerate') + .mockImplementationOnce(originalGenerate) + .mockImplementationOnce(async (...args) => { + const result = await originalGenerate(...args); + advisorStarted.resolve(); + await generationGate.promise; + return result; + }); + queueReview(fixture.scripted, 'Review pending.', 'concern'); + + await runMainTurn(fixture.main); + await advisorStarted.promise; + + let closeSettled = false; + const close = fixture.session.advisor.close().then(() => { + closeSettled = true; + }); + try { + await vi.waitFor(() => expect(closeSettled).toBe(true), { timeout: 1000 }); + } finally { + generationGate.resolve(); + await close; + } + }); + it('does not start an advisor created during shutdown', async () => { + const fixture = await createFixture({ advisorAlias: 'advisor' }); + const creationGate = createDeferred(); + let creationStarted = false; + let advisorPromptStarted = false; + const originalCreate = fixture.session.createAgent.bind(fixture.session); + vi.spyOn(fixture.session, 'createAgent').mockImplementation(async (...args) => { + const created = await originalCreate(...args); + if (created.agent.type === 'sub') { + const prompt = created.agent.turn.prompt.bind(created.agent.turn); + vi.spyOn(created.agent.turn, 'prompt').mockImplementation((...promptArgs) => { + advisorPromptStarted = true; + return prompt(...promptArgs); + }); + creationStarted = true; + await creationGate.promise; + } + return created; + }); + + queueReview(fixture.scripted, 'Review pending.', 'concern'); + await runMainTurn(fixture.main); + await vi.waitFor(() => expect(creationStarted).toBe(true)); + + const close = fixture.session.advisor.close(); + creationGate.resolve(); + await close; + + await vi.waitFor(() => expect([...fixture.session.agents.keys()]).toEqual(['main'])); + expect(advisorPromptStarted).toBe(false); + }); + it('ignores a malformed persisted transcript record', async () => { + const fixture = await createFixture({ + watchdog: [ + 'advisors:', + ' - name: Security', + ' model: advisor', + ].join('\n'), + }); + const transcriptDir = join(fixture.sessionDir, 'advisors'); + await mkdir(transcriptDir, { recursive: true }); + await writeFile(join(transcriptDir, 'security.jsonl'), 'not valid JSON\n'); + + const [status] = await fixture.session.advisor.status(); + + expect(status).toMatchObject({ id: 'security', notes: 0, costUsd: 0 }); + }); + it('stays idle without an advisor model', async () => { const fixture = await createFixture({ enabled: true }); @@ -358,7 +905,7 @@ describe('SessionAdvisor', () => { for (let turn = 0; turn < 3; turn += 1) { fixture.scripted.mockNextResponse({ type: 'text', text: 'Done.' }); await runMainTurn(fixture.main); - await flushAsync(); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(turn + 1)); } await vi.waitFor(() => expect(warn).toHaveBeenCalledWith('advisor disabled after three consecutive failures'), @@ -402,50 +949,72 @@ describe('SessionAdvisor', () => { it('counts an aborted advisor wait as a failure', async () => { const fixture = await createFixture({ advisorAlias: 'advisor' }); const timeoutError = new Error('advisor timed out'); + const timeoutSignal = AbortSignal.abort(timeoutError); + vi.spyOn(AbortSignal, 'timeout').mockReturnValue(timeoutSignal); const originalCreate = fixture.session.createAgent.bind(fixture.session); const spawn = vi .spyOn(fixture.session, 'createAgent') .mockRejectedValueOnce(new Error('first failure')) .mockRejectedValueOnce(new Error('second failure')) - .mockImplementationOnce((...args) => originalCreate(...args)); - const timeout = vi - .spyOn(AbortSignal, 'timeout') - .mockReturnValue(AbortSignal.abort(timeoutError)); + .mockImplementationOnce(async (...args) => { + const created = await originalCreate(...args); + vi.spyOn(created.agent.usage, 'data').mockReturnValue({ totalCostUsd: 0.42 }); + vi.spyOn(created.agent.turn, 'waitForCurrentTurn').mockRejectedValueOnce(timeoutError); + return created; + }); const warn = vi.spyOn(fixture.session.log, 'warn'); for (let turn = 0; turn < 2; turn += 1) { fixture.scripted.mockNextResponse({ type: 'text', text: 'Done.' }); await runMainTurn(fixture.main); - await flushAsync(); + await vi.waitFor(async () => { + const [status] = await fixture.session.advisor.status(); + expect(status?.failures).toBe(turn + 1); + }); } queueReview(fixture.scripted); await runMainTurn(fixture.main); - await vi.waitFor(() => - expect(warn).toHaveBeenCalledWith('advisor disabled after three consecutive failures'), - ); + await vi.waitFor(async () => { + const [status] = await fixture.session.advisor.status(); + expect(status).toMatchObject({ + failures: 3, + status: 'paused', + enabled: false, + costUsd: 0.42, + }); + expect(warn).toHaveBeenCalledWith('advisor disabled after three consecutive failures'); + }); + expect(spawn).toHaveBeenCalledTimes(3); fixture.scripted.mockNextResponse({ type: 'text', text: 'Done.' }); await runMainTurn(fixture.main); await flushAsync(); - - expect(timeout).toHaveBeenCalledWith(120_000); expect(spawn).toHaveBeenCalledTimes(3); }); }); interface FixtureOptions { readonly enabled?: boolean; + readonly advisorEnabled?: boolean; + readonly omitAdvisorEnabled?: boolean; readonly advisorAlias?: 'advisor' | 'cross-advisor' | 'reviewer'; readonly advisorModel?: string; + readonly watchdog?: string; } async function createFixture(options: FixtureOptions = {}): Promise<{ readonly session: Session; readonly main: Agent; readonly scripted: ReturnType; + readonly sessionDir: string; + readonly workDir: string; }> { const workDir = await makeTempDir(); const sessionDir = await makeTempDir(); + const userHomeDir = await makeTempDir(); + if (options.watchdog !== undefined) { + await writeFile(join(workDir, 'WATCHDOG.yml'), options.watchdog); + } const config = testConfig(options); const scripted = createScriptedGenerate(); const session = new Session({ @@ -453,7 +1022,7 @@ async function createFixture(options: FixtureOptions = {}): Promise<{ kaos: testKaos.withCwd(workDir), homedir: sessionDir, rpc: createSessionRpc(), - skills: { explicitDirs: [join(workDir, 'missing-skills')] }, + skills: { userHomeDir, explicitDirs: [join(workDir, 'missing-skills')] }, config, providerManager: new ProviderManager({ config }), }); @@ -463,7 +1032,7 @@ async function createFixture(options: FixtureOptions = {}): Promise<{ { profile: testProfile() }, ); main.config.update({ modelAlias: 'main', thinkingLevel: 'off' }); - return { session, main, scripted }; + return { session, main, scripted, sessionDir, workDir }; } function testConfig(options: FixtureOptions): PythinkerConfig { @@ -489,8 +1058,12 @@ function testConfig(options: FixtureOptions): PythinkerConfig { advisor: options.enabled === true || options.advisorAlias !== undefined || - options.advisorModel !== undefined - ? { enabled: true, model: options.advisorModel } + options.advisorModel !== undefined || + options.advisorEnabled !== undefined + ? { + enabled: options.omitAdvisorEnabled ? undefined : options.advisorEnabled ?? true, + model: options.advisorModel, + } : undefined, }; } @@ -508,6 +1081,18 @@ function queueReview( arguments: JSON.stringify({ notes: note === undefined ? [] : [{ note, severity }] }), }); } +function mockAdvisorResponse( + scripted: ReturnType, + note: string, + severity: 'nit' | 'concern' | 'blocker', +): void { + scripted.mockNextResponse({ + type: 'function', + id: `advisor-output-${note}`, + name: 'StructuredOutput', + arguments: JSON.stringify({ notes: [{ note, severity }] }), + }); +} function mockAdvisorOutput(session: Session, structuredOutput: unknown) { const originalCreate = session.createAgent.bind(session); diff --git a/packages/node-sdk/src/events.ts b/packages/node-sdk/src/events.ts index fa565c36..66808e52 100644 --- a/packages/node-sdk/src/events.ts +++ b/packages/node-sdk/src/events.ts @@ -19,6 +19,7 @@ export type { ErrorEvent, WarningEvent, UsageStatus, + AdvisorStatusEvent, } from '@pymodel/agent-core'; // Turn and step lifecycle events plus the turn-ending reason enum. diff --git a/packages/node-sdk/src/index.ts b/packages/node-sdk/src/index.ts index 8b6bdefe..973486aa 100644 --- a/packages/node-sdk/src/index.ts +++ b/packages/node-sdk/src/index.ts @@ -1,6 +1,6 @@ export { PythinkerHarness } from '#/pythinker-harness'; export type { PythinkerHarnessRuntimeOptions } from '#/pythinker-harness'; -export { Session } from '#/session'; +export { Session, type SessionAdvisor } from '#/session'; export { createPythinkerHarness, SDKRpcClient, type SDKRpcClientOptions } from '#/sdk-rpc-client'; export { runPythinkerMcpServer, type PythinkerMcpServerOptions } from '#/mcp-server'; export { @@ -14,6 +14,7 @@ export { } from '#/config-rpc'; export { SDKRpcClientBase, + type SetSessionAdvisorEnabledRpcInput, type SetSessionDynamicWorkflowModeRpcInput, type SetSessionFastModeRpcInput, } from '#/rpc'; diff --git a/packages/node-sdk/src/rpc.ts b/packages/node-sdk/src/rpc.ts index 1c240057..2e033d29 100644 --- a/packages/node-sdk/src/rpc.ts +++ b/packages/node-sdk/src/rpc.ts @@ -3,11 +3,12 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { ErrorCodes, makeErrorPayload, + type AdvisorStatus, type AgentContextData, - type ContextUsageReport, type ApprovalRequest, type ApprovalResponse, type CoreAPI, + type DynamicWorkflowModeTrigger, type Event, type ExperimentalFeatureState, type QuestionRequest, @@ -17,7 +18,6 @@ import { type SkillActivationResult, type ToolCallRequest, type ToolCallResponse, - type DynamicWorkflowModeTrigger, } from '@pymodel/agent-core'; import type { Kaos } from '@pymodel/kaos'; @@ -26,6 +26,7 @@ import type { AgentProfileCatalog, BackgroundTaskInfo, ConfigDiagnostics, + ContextUsageReport, CreateSessionOptions, ExportSessionInput, ExportSessionResult, @@ -107,6 +108,10 @@ export interface SetSessionPlanModeRpcInput extends SessionIdRpcInput { export type SetSessionDynamicWorkflowModeRpcInput = | (SessionIdRpcInput & { readonly enabled: true; readonly trigger: DynamicWorkflowModeTrigger }) | (SessionIdRpcInput & { readonly enabled: false }); +export interface SetSessionAdvisorEnabledRpcInput extends SessionIdRpcInput { + readonly enabled: boolean; + readonly advisorId?: string; +} export interface ActivateSkillRpcInput extends SessionIdRpcInput { readonly name: string; @@ -559,6 +564,26 @@ export abstract class SDKRpcClientBase { return rpc.listSkills({ sessionId: input.sessionId }); } + async getAdvisorStatus(input: SessionIdRpcInput): Promise { + const rpc = await this.getRpc(); + return rpc.getAdvisorStatus({ sessionId: input.sessionId }); + } + + async setAdvisorEnabled( + input: SetSessionAdvisorEnabledRpcInput, + ): Promise { + const rpc = await this.getRpc(); + return rpc.setAdvisorEnabled({ + sessionId: input.sessionId, + enabled: input.enabled, + advisorId: input.advisorId, + }); + } + + async reloadAdvisor(input: SessionIdRpcInput): Promise { + const rpc = await this.getRpc(); + return rpc.reloadAdvisor({ sessionId: input.sessionId }); + } async reloadSkills(input: SessionIdRpcInput): Promise { const rpc = await this.getRpc(); await rpc.reloadSkills({ sessionId: input.sessionId }); diff --git a/packages/node-sdk/src/session.ts b/packages/node-sdk/src/session.ts index 88d5eb6f..78cdff1d 100644 --- a/packages/node-sdk/src/session.ts +++ b/packages/node-sdk/src/session.ts @@ -11,6 +11,7 @@ import { import { type ApprovalHandler, type Event, type QuestionHandler } from '#/events'; import type { SDKRpcClientBase } from '#/rpc'; import type { + AdvisorStatusSnapshot, BackgroundTaskInfo, CompactOptions, CreateGoalInput, @@ -42,9 +43,14 @@ import type { WorkingTreeChanges, WorkingTreeFileDiff, } from '#/types'; - const MAIN_AGENT_ID = 'main'; +export interface SessionAdvisor { + status(): Promise; + setEnabled(enabled: boolean, advisorId?: string): Promise; + reload(): Promise; +} + export interface SessionOptions { readonly id: string; readonly workDir: string; @@ -63,6 +69,7 @@ export class Session { private readonly rpc: SDKRpcClientBase; private readonly onClose?: (() => void | Promise) | undefined; private closed = false; + readonly advisor: SessionAdvisor; constructor(options: SessionOptions) { this.id = options.id; @@ -71,6 +78,11 @@ export class Session { this.resumeState = options.resumeState ?? resumeStateFromSummary(options.summary); this.rpc = options.rpc; this.onClose = options.onClose; + this.advisor = { + status: () => this.getAdvisorStatus(), + setEnabled: (enabled, advisorId) => this.setAdvisorEnabled(enabled, advisorId), + reload: () => this.reloadAdvisor(), + }; } getResumeState(): ResumedSessionState | undefined { @@ -90,6 +102,27 @@ export class Session { this.ensureOpen(); return this.rpc.getSessionMetadata({ sessionId: this.id }); } + async getAdvisorStatus(): Promise { + this.ensureOpen(); + return this.rpc.getAdvisorStatus({ sessionId: this.id }); + } + + async setAdvisorEnabled( + enabled: boolean, + advisorId?: string, + ): Promise { + this.ensureOpen(); + return this.rpc.setAdvisorEnabled({ + sessionId: this.id, + enabled, + advisorId, + }); + } + + async reloadAdvisor(): Promise { + this.ensureOpen(); + return this.rpc.reloadAdvisor({ sessionId: this.id }); + } async updateSessionMetadata(metadata: SessionMetadataPatch): Promise { this.ensureOpen(); diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index f3d6fa15..c08a2970 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -23,6 +23,8 @@ export type JsonObject = { readonly [key: string]: JsonValue }; export type Unsubscribe = () => void; export type { + AdvisorRuntimeStatus, + AdvisorStatusSnapshot, AgentReplayRecord, AgentProfileCatalog, AgentProfileSummary, diff --git a/packages/node-sdk/test/session-event-types.test.ts b/packages/node-sdk/test/session-event-types.test.ts index eaa6be69..50e34681 100644 --- a/packages/node-sdk/test/session-event-types.test.ts +++ b/packages/node-sdk/test/session-event-types.test.ts @@ -86,6 +86,7 @@ describe('Event public types', () => { case 'turn.step.completed': case 'turn.step.retrying': case 'turn.step.interrupted': + case 'advisor.status': case 'assistant.delta': case 'hook.result': case 'hook.status': diff --git a/packages/protocol/src/__tests__/events.test.ts b/packages/protocol/src/__tests__/events.test.ts index 3ab53932..25b4a559 100644 --- a/packages/protocol/src/__tests__/events.test.ts +++ b/packages/protocol/src/__tests__/events.test.ts @@ -5,6 +5,7 @@ import { fileURLToPath } from 'node:url'; import { describe, it, expect } from 'vitest'; import { + advisorStatusEventSchema, agentStatusUpdatedEventSchema, agentEventSchema, assistantDeltaEventSchema, @@ -99,6 +100,25 @@ describe('events / display re-exports', () => { }).success, ).toBe(true); }); + it('validates advisor status events through the event union', () => { + const status = { + type: 'advisor.status', + advisorId: 'security', + name: 'Security', + status: 'running', + enabled: true, + model: 'reviewer', + message: 'Reviewing the latest turn.', + } as const; + + expect(advisorStatusEventSchema.parse(status)).toEqual(status); + expect( + agentEventSchema.parse({ ...status, agentId: 'main', sessionId: 'session' }), + ).toMatchObject(status); + expect( + advisorStatusEventSchema.safeParse({ ...status, status: 'unknown' }).success, + ).toBe(false); + }); it('uses dynamic workflow fields and rejects the removed swarm fields', () => { expect( diff --git a/packages/protocol/src/events.ts b/packages/protocol/src/events.ts index 4f2cd967..b4beb719 100644 --- a/packages/protocol/src/events.ts +++ b/packages/protocol/src/events.ts @@ -467,6 +467,16 @@ export interface HookStatusEvent { readonly content: string; readonly active: boolean; } +export interface AdvisorStatusEvent { + readonly type: 'advisor.status'; + readonly advisorId: string; + readonly name: string; + readonly status: 'running' | 'paused' | 'quota_exhausted' | 'error' | 'no_model'; + readonly enabled: boolean; + readonly model?: string; + readonly message?: string; +} + export interface ThinkingDeltaEvent { readonly type: 'thinking.delta'; @@ -674,6 +684,7 @@ export type AgentEvent = | AssistantDeltaEvent | HookResultEvent | HookStatusEvent + | AdvisorStatusEvent | ThinkingDeltaEvent | ToolCallDeltaEvent | ToolCallStartedEvent @@ -1159,6 +1170,15 @@ export const hookStatusEventSchema = z.object({ content: z.string(), active: z.boolean(), }) satisfies z.ZodType; +export const advisorStatusEventSchema = z.object({ + type: z.literal('advisor.status'), + advisorId: z.string(), + name: z.string(), + status: z.enum(['running', 'paused', 'quota_exhausted', 'error', 'no_model']), + enabled: z.boolean(), + model: z.string().optional(), + message: z.string().optional(), +}) satisfies z.ZodType; export const thinkingDeltaEventSchema = z.object({ type: z.literal('thinking.delta'), @@ -1343,6 +1363,7 @@ export const agentEventSchema = z.discriminatedUnion('type', [ sessionStatusChangedEventSchema, goalUpdatedEventSchema, skillActivatedEventSchema, + advisorStatusEventSchema, turnStartedEventSchema, turnEndedEventSchema, turnStepStartedEventSchema,