From 07d51e4add6ee23a56fb8745aa7754f05f3d6d36 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 29 May 2026 17:20:57 +0800 Subject: [PATCH 1/8] chore(agent-core): move tool services type (#206) --- .changeset/tool-support-services.md | 6 ++++++ packages/agent-core/src/agent/index.ts | 2 +- packages/agent-core/src/index.ts | 2 +- packages/agent-core/src/rpc/core-impl.ts | 2 +- packages/agent-core/src/runtime-types.ts | 6 ------ packages/agent-core/src/session/index.ts | 2 +- packages/agent-core/src/tools/support/services.ts | 6 ++++++ packages/agent-core/test/agent/harness/agent.ts | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 .changeset/tool-support-services.md delete mode 100644 packages/agent-core/src/runtime-types.ts create mode 100644 packages/agent-core/src/tools/support/services.ts diff --git a/.changeset/tool-support-services.md b/.changeset/tool-support-services.md new file mode 100644 index 000000000..1d04752ac --- /dev/null +++ b/.changeset/tool-support-services.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +--- + +Relocate shared tool service typing to the tool support layer. diff --git a/packages/agent-core/src/agent/index.ts b/packages/agent-core/src/agent/index.ts index d7c412470..5473f65a5 100644 --- a/packages/agent-core/src/agent/index.ts +++ b/packages/agent-core/src/agent/index.ts @@ -17,7 +17,6 @@ import type { EnabledPluginSessionStart } from '#/plugin'; import type { McpConnectionManager } from '../mcp'; import type { PreparedSystemPromptContext, ResolvedAgentProfile } from '../profile'; import type { ModelProvider } from '../session/provider-manager'; -import type { ToolServices } from '../runtime-types'; import type { SessionSubagentHost } from '../session/subagent-host'; import type { SkillRegistry } from '../skill'; import { noopTelemetryClient, type TelemetryClient } from '../telemetry'; @@ -55,6 +54,7 @@ import { import { UsageRecorder } from './usage'; import { resolveCompletionBudget } from '../utils/completion-budget'; import type { Kaos } from '@moonshot-ai/kaos'; +import type { ToolServices } from '../tools/support/services'; export type { AgentRecord, AgentRecordPersistence } from './records'; export type { BuiltinTool, ToolInfo, ToolSource, UserToolRegistration } from './tool'; diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index e13c5f64a..670def093 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -39,7 +39,7 @@ export type { BackgroundTaskKind, BackgroundTaskStatus, } from './tools/background/manager'; -export type { ToolServices } from './runtime-types'; +export type { ToolServices } from './tools/support/services'; export { SingleModelProvider } from './session/provider-manager'; export type { BearerTokenProvider, diff --git a/packages/agent-core/src/rpc/core-impl.ts b/packages/agent-core/src/rpc/core-impl.ts index 8b1372aec..a20899e34 100644 --- a/packages/agent-core/src/rpc/core-impl.ts +++ b/packages/agent-core/src/rpc/core-impl.ts @@ -22,7 +22,6 @@ import { } from '../config'; import type { Logger } from '../logging/types'; import { resolveSessionMcpConfig, type SessionMcpConfig } from '../mcp'; -import type { ToolServices } from '../runtime-types'; import { Session, type SessionMeta, type SessionSkillConfig } from '../session'; import { exportSessionDirectory } from '../session/export'; import { @@ -84,6 +83,7 @@ import type { ResumedAgentState, ResumeSessionResult } from './resumed'; import type { SDKRPC } from './sdk-api'; import { proxyWithExtraPayload } from './types'; import { KaosShellNotFoundError, LocalKaos, type Kaos } from '@moonshot-ai/kaos'; +import type { ToolServices } from '../tools/support/services'; const KIMI_CODE_PROVIDER_NAME = 'managed:kimi-code'; diff --git a/packages/agent-core/src/runtime-types.ts b/packages/agent-core/src/runtime-types.ts deleted file mode 100644 index 7732feae1..000000000 --- a/packages/agent-core/src/runtime-types.ts +++ /dev/null @@ -1,6 +0,0 @@ -import type { UrlFetcher, WebSearchProvider } from './tools/builtin'; - -export interface ToolServices { - readonly urlFetcher?: UrlFetcher | undefined; - readonly webSearcher?: WebSearchProvider | undefined; -} diff --git a/packages/agent-core/src/session/index.ts b/packages/agent-core/src/session/index.ts index 7192cc614..55af41a52 100644 --- a/packages/agent-core/src/session/index.ts +++ b/packages/agent-core/src/session/index.ts @@ -28,7 +28,6 @@ import { type ResolvedAgentProfile, } from '../profile'; import type { ProviderManager } from './provider-manager'; -import type { ToolServices } from '../runtime-types'; import { registerBuiltinSkills, resolveSkillRoots, @@ -39,6 +38,7 @@ import { } from '../skill'; import { noopTelemetryClient, type TelemetryClient } from '../telemetry'; import { SessionSubagentHost } from './subagent-host'; +import type { ToolServices } from '../tools/support/services'; export interface SessionOptions { readonly kaos: Kaos; diff --git a/packages/agent-core/src/tools/support/services.ts b/packages/agent-core/src/tools/support/services.ts new file mode 100644 index 000000000..ba3d67a62 --- /dev/null +++ b/packages/agent-core/src/tools/support/services.ts @@ -0,0 +1,6 @@ +import type { UrlFetcher, WebSearchProvider } from '../builtin'; + +export interface ToolServices { + readonly urlFetcher?: UrlFetcher; + readonly webSearcher?: WebSearchProvider; +} diff --git a/packages/agent-core/test/agent/harness/agent.ts b/packages/agent-core/test/agent/harness/agent.ts index afe5e247c..1944de83c 100644 --- a/packages/agent-core/test/agent/harness/agent.ts +++ b/packages/agent-core/test/agent/harness/agent.ts @@ -24,7 +24,7 @@ import type { Logger } from '../../../src/logging'; import { ProviderManager } from '../../../src/session/provider-manager'; import type { QuestionResult, RPCCallOptions, SDKAgentRPC } from '../../../src/rpc'; import type { AgentAPI } from '../../../src/rpc/core-api'; -import type { ToolServices } from '../../../src/runtime-types'; +import type { ToolServices } from '../../../src/tools/support/services'; import type { TelemetryClient } from '../../../src/telemetry'; import type { PromisifyMethods } from '../../../src/utils/types'; import { createFakeKaos } from '../../tools/fixtures/fake-kaos'; From f3269eacb9da9a6b66f578a864d0b9bdfb1d6d81 Mon Sep 17 00:00:00 2001 From: Kai Date: Fri, 29 May 2026 17:26:27 +0800 Subject: [PATCH 2/8] fix(tui): show real terminal status for background agents (#197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(tui): show real terminal status for background agents The Agent tool's run_in_background=true call returns a non-error ToolResult whose body just says "status: running". The transcript card derived its done/failed badge from that result, so every terminated background agent — including ones reconcile reclassifies as lost on resume — kept the green "✓ Completed" label even when the actual task failed, was killed, or never came back. Push the real BackgroundTaskInfo.status into the matching Agent card so the badge reflects what happened. The card's resolver prefers subagent agentId (live) and falls back to the description on resume; on resume the apply step also runs after replay finishes so the agent group can reach the borrowed components. Also adds an agent-core regression test that pins live, busy, group, race, and resume scenarios for the bg notification chain. * fix(tui): also propagate bg agent terminal status to standalone cards Standalone Agent cards (only one Agent tool call in a step, never upgraded into an AgentGroupComponent) bypassed the previous `setBackgroundTaskTerminalStatus` path: the standalone header reads `getDerivedSubagentPhase`, which still derived `done` from the non-error spawn-success ToolResult, and the method did not request a header/content rebuild. Lost/failed/killed bg agents in this shape still rendered as `✓ Completed`. Thread the override through `getDerivedSubagentPhase`, populate `subagentError` with the friendly failure message so both render paths share one source of truth, and trigger the same header + content rebuild that `onSubagentFailed` does. Also include the override in `hasSubagentState` / the subagent-block early-return so a replayed solo bg agent (no replayed subagent block, no sub-tool activity) switches to the subagent-aware layout instead of the generic `Used Agent` rendering. Adds two standalone-render regression tests so the path no longer relies on the grouped snapshot to stay correct. * feat(agent-core): make resume actionable from the lost-task notification A backgrounded subagent that ends as `lost`/`failed`/`killed` is already a soft-recoverable thing — `subagentHost.resume` will reanimate the persisted Agent instance — but the LLM had to dig through the original spawn-success ToolResult to find the right id and figure out the recovery shape on its own. The two look-alike identifiers (the BackgroundManager `task_id` aka `source_id`, and the `subagentHost` `agent_id`) regularly got confused in practice. Surface what the model needs at the moment of decision: - Add `agent_id` as a top-level `` attribute for agent-* tasks, so the right id is structural, not buried in prose. Render path keeps backward-compat by omitting the attribute when no agent_id is known (bash tasks, old sessions). - On non-success agent terminal states, append a recovery paragraph to the body: the precise `Agent(resume=...)` call, the disambiguation between `agent_id` and `source_id`, the `run_in_background` option, and what state survives the restart vs. what may need to be redone. - Tighten the spawn-time `resume_hint` with the same disambiguation and an explicit pointer at the `task.lost`/`task.failed`/`task.killed` recovery trigger. - Persist `agent_id` and `subagent_type` in PersistedTask so the recovery body still works after a session restart, where in-memory `BackgroundTaskInfo.agentId` would otherwise be undefined. Optional fields keep the disk schema forward/backward compatible — pre-PR records load without them and silently fall back to the original short body. * fix(tui): route bg-agent terminal events by stable agent_id, not description `tc.subagentAgentId` is left undefined for every backgrounded agent. `handleSubagentSpawned` early-returns for `runInBackground` before calling `tc.onSubagentSpawned`, and the wire replay path drops the `subagent` block entirely (`toolCallFromReplayMessage` returns only id/name/args). So the `agentId` branch in `applyBackgroundTaskTerminalStatus` never matched in practice, every call fell through to the description-based fallback, and the persisted `agent_id` we added in the previous commit was effectively dead. That fallback also has a real failure mode: if a foreground Agent and a backgrounded Agent share the same `args.description`, the only candidate found is the live (unrelated) card, which gets incorrectly relabeled as the lost task's terminal state. Parse `agent_id: agent-N` out of the AgentTool spawn-success ToolResult body inside `getSubagentAgentId` so the id is always recoverable, regardless of whether the in-memory subagent metadata was ever populated. Foreground and backgrounded Agent cards now carry distinct ids and route correctly. Also pipe the real `subagent.failed` error through to the parent card. The background branch of `handleSubagentFailed` previously only appended the dedicated transcript entry; the parent Agent card was left with the generic "Background agent failed" written by the later `background.task.terminated` event. Add an optional `errorText` to `setBackgroundTaskTerminalStatus` / `applyBackgroundTaskTerminalStatus` and pass `event.error` through on the failed branch — the real reason now reaches both the card and the entry. * fix(tui): treat agent_id as authoritative when matching bg terminal events Previously `applyBackgroundTaskTerminalStatus` always tried agent_id first and then fell back to description match on miss. That fallback caused two cross-card bugs: 1. On resume, `applyTerminalBackgroundAgentStatuses` iterates every persisted terminal task, including ones whose tool calls fell outside the `REPLAY_TURN_LIMIT` window and were never mounted. Description fallback could route an old `lost` status onto an unrelated recent Agent card sharing the same `args.description`. 2. During the live spawn → terminate window, the same card briefly lives in both `_pendingToolComponents` and `transcriptContainer`. A description-only walk visits the same component twice and flags itself ambiguous, dropping the otherwise unambiguous update. When `args.agentId` is provided we now match only by id and skip on miss. With `getSubagentAgentId` already parsing `agent_id: agent-N` out of the spawn-success ToolResult, the id path is reliable for both live and resume even though `tc.subagentAgentId` is never populated for backgrounded agents. Description fallback is preserved solely for old pre-PR sessions whose persisted records lack `agent_id` — same best-effort behavior as before. --- .changeset/bg-agent-terminal-status.md | 6 + .../tui/components/messages/agent-group.ts | 11 + .../src/tui/components/messages/tool-call.ts | 132 +++++++- .../tui/controllers/session-event-handler.ts | 22 ++ .../src/tui/controllers/session-replay.ts | 31 ++ .../src/tui/controllers/streaming-ui.ts | 91 ++++++ .../tui/components/messages/tool-call.test.ts | 221 +++++++++++++ .../agent-core/src/agent/background/index.ts | 53 ++- .../src/agent/context/notification-xml.ts | 20 +- .../src/tools/background/manager.ts | 11 + .../src/tools/background/persist.ts | 12 + .../src/tools/builtin/collaboration/agent.ts | 2 +- .../test/agent/background-manager.test.ts | 64 ++++ .../agent/bg-idle-notification-repro.test.ts | 308 ++++++++++++++++++ .../agent-core/test/agent/context.test.ts | 36 ++ packages/agent-core/test/tools/agent.test.ts | 8 + 16 files changed, 1015 insertions(+), 13 deletions(-) create mode 100644 .changeset/bg-agent-terminal-status.md create mode 100644 packages/agent-core/test/agent/bg-idle-notification-repro.test.ts diff --git a/.changeset/bg-agent-terminal-status.md b/.changeset/bg-agent-terminal-status.md new file mode 100644 index 000000000..e895d826a --- /dev/null +++ b/.changeset/bg-agent-terminal-status.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +--- + +Show the real terminal status of background agents in the transcript so lost, failed, and killed ones no longer appear as completed, and include the resume agent id and recovery instructions in the failure notification so the model can resume reliably. diff --git a/apps/kimi-code/src/tui/components/messages/agent-group.ts b/apps/kimi-code/src/tui/components/messages/agent-group.ts index 593ae9746..d936d01b8 100644 --- a/apps/kimi-code/src/tui/components/messages/agent-group.ts +++ b/apps/kimi-code/src/tui/components/messages/agent-group.ts @@ -54,6 +54,17 @@ export class AgentGroupComponent extends Container { return this.entries.length; } + /** + * Exposes the borrowed tool call components so external code (e.g. + * routing background task terminal events back to the corresponding + * Agent card) can reach them — the group renders the tcs' snapshots + * but never mounts the tcs as Container children, so a plain tree + * walk of `transcriptContainer` cannot discover them. + */ + getToolComponents(): readonly ToolCallComponent[] { + return this.entries.map((entry) => entry.tc); + } + /** * Borrows a standalone `ToolCallComponent` into the group as a hidden state * container. Snapshot changes trigger throttled refreshes. Re-attaching the diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index 6887070b3..d0fcc1f6c 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -96,6 +96,22 @@ export interface ToolCallReadSnapshot { readonly lines: number; } +function backgroundFailureMessage( + status: 'completed' | 'failed' | 'killed' | 'lost' | undefined, +): string | undefined { + switch (status) { + case 'lost': + return 'Background agent lost (session restarted before completion)'; + case 'killed': + return 'Background agent killed'; + case 'failed': + return 'Background agent failed'; + case 'completed': + case undefined: + return undefined; + } +} + function str(v: unknown): string { return typeof v === 'string' ? v : ''; } @@ -474,6 +490,17 @@ export class ToolCallComponent extends Container { private subagentThinkingText = ''; // ── Subagent lifecycle state from subagent.spawned/completed/failed ── private subagentPhase: 'spawning' | 'running' | 'done' | 'failed' | 'backgrounded' | undefined; + /** + * Authoritative terminal phase for a backgrounded subagent. Set from + * `BackgroundTaskInfo.status` via `setBackgroundTaskTerminalStatus` once + * the backing task reaches a terminal state — either live (a bg agent + * fails / is killed) or on resume (reconcile reclassifies a still-running + * task as `lost`). Beats the spawn-success ToolResult in both render + * paths (`getDerivedSubagentPhase` for standalone, `getSubagentSnapshot` + * for grouped), which would otherwise mislabel every terminated + * background agent — including lost ones — as `✓ Completed`. + */ + private backgroundTaskTerminalPhase: 'done' | 'failed' | undefined; private subagentContextTokens: number | undefined; private subagentUsage: TokenUsage | undefined; private subagentResultSummary: string | undefined; @@ -707,7 +734,14 @@ export class ToolCallComponent extends Container { // `backgrounded` has no result because background agents do not enter the // transcript. const derivedPhase: ToolCallSubagentSnapshot['phase'] = - this.result !== undefined ? (this.result.is_error ? 'failed' : 'done') : this.subagentPhase; + this.backgroundTaskTerminalPhase ?? + (this.result !== undefined + ? this.result.is_error + ? 'failed' + : 'done' + : this.subagentPhase); + const errorText = + this.subagentError ?? (derivedPhase === 'failed' ? this.result?.output : undefined); return { toolCallId: this.toolCall.id, toolName: this.toolCall.name, @@ -717,8 +751,7 @@ export class ToolCallComponent extends Container { toolCount: finished, tokens, isError: derivedPhase === 'failed', - errorText: - this.subagentError ?? (derivedPhase === 'failed' ? this.result?.output : undefined), + errorText, latestActivity, }; } @@ -934,6 +967,90 @@ export class ToolCallComponent extends Container { this.ui?.requestRender(); } + /** + * Records the actual terminal status of the backing background task so + * the snapshot phase no longer relies on the spawn-success ToolResult. + * Called for `agent-*` background tasks both live (when the bg agent + * terminates non-successfully) and on resume (when reconcile + * reclassifies a previously-running task as `lost`). + */ + setBackgroundTaskTerminalStatus( + status: 'completed' | 'failed' | 'killed' | 'lost', + options: { errorText?: string | undefined } = {}, + ): void { + const phase: 'done' | 'failed' = status === 'completed' ? 'done' : 'failed'; + const { errorText } = options; + const phaseUnchanged = this.backgroundTaskTerminalPhase === phase; + let errorChanged = false; + if (phase === 'failed') { + // Surface the failure line through the same `subagentError` slot that + // `onSubagentFailed` writes. The standalone card reads this in + // `buildSingleSubagentBlock`; the group card reads it via `errorText` + // in `getSubagentSnapshot`. Priority: + // 1. Explicit `errorText` from the caller (the real message from a + // live `subagent.failed` event) always wins — it is the most + // informative. + // 2. Existing `subagentError` (could be from a prior + // `onSubagentFailed` or an earlier explicit override) is kept. + // 3. Fall back to a friendly generic so the failure has SOME + // visible explanation when no source has supplied one. + if (errorText !== undefined && this.subagentError !== errorText) { + this.subagentError = errorText; + errorChanged = true; + } else if (this.subagentError === undefined) { + const generic = backgroundFailureMessage(status); + if (generic !== undefined) { + this.subagentError = generic; + errorChanged = true; + } + } + } + if (phaseUnchanged && !errorChanged) return; + this.backgroundTaskTerminalPhase = phase; + this.subagentEndedAtMs ??= Date.now(); + this.syncSubagentElapsedTimer(); + this.headerText.setText(this.buildHeader()); + this.rebuildContent(); + this.notifySnapshotChange(); + } + + /** + * Subagent id for the backing AgentTool call, used by routing to find a + * tool call's backing subagent when reconciling background task lifecycle + * events. + * + * Two writers, in priority order: + * 1. In-memory `subagentAgentId` — wired by `setSubagentMeta` / + * `onSubagentSpawned` for foreground agents. For backgrounded agents + * this stays undefined: `handleSubagentSpawned` early-returns before + * calling `tc.onSubagentSpawned`, and `applySubagentReplay` early- + * returns when the wire payload omits the `subagent` block — which + * it does for every replayed Agent call. + * 2. The spawn-success ToolResult body — AgentTool unconditionally + * emits `agent_id: agent-N` for every Agent call (foreground and + * background). Parsing it gives the stable identifier even when the + * in-memory field is empty, which is the only way the resume path + * can reliably route a `background.task.terminated` to the right + * card and the only way the live path avoids matching by description + * and accidentally updating an unrelated Agent card that happens to + * share the same `args.description`. + */ + getSubagentAgentId(): string | undefined { + if (this.subagentAgentId !== undefined) return this.subagentAgentId; + if (this.toolCall.name !== 'Agent' || this.result === undefined) return undefined; + const match = this.result.output.match(/^agent_id:\s*(agent-[A-Za-z0-9_-]+)/m); + return match?.[1]; + } + + /** `args.description` for `Agent` tool calls, used as a resume-path + * fallback when the wire format pre-dates persisted subagent ids and + * the only stable cross-restart identifier is the description string. */ + getAgentToolDescription(): string | undefined { + if (this.toolCall.name !== 'Agent') return undefined; + const desc = this.toolCall.args['description']; + return typeof desc === 'string' ? desc : undefined; + } + appendSubagentText(text: string, kind: SubagentTextKind = 'text'): void { if (kind === 'thinking') { this.subagentThinkingText += text; @@ -1150,7 +1267,8 @@ export class ToolCallComponent extends Container { this.ongoingSubCalls.size === 0 && this.finishedSubCalls.length === 0 && this.subagentText.length === 0 && - this.subagentPhase === undefined + this.subagentPhase === undefined && + this.backgroundTaskTerminalPhase === undefined ) { return; } @@ -1273,7 +1391,8 @@ export class ToolCallComponent extends Container { this.subToolActivities.size > 0 || this.subagentText.length > 0 || this.subagentThinkingText.length > 0 || - this.subagentPhase !== undefined + this.subagentPhase !== undefined || + this.backgroundTaskTerminalPhase !== undefined ); } @@ -1288,6 +1407,9 @@ export class ToolCallComponent extends Container { | 'failed' | 'backgrounded' | undefined { + if (this.backgroundTaskTerminalPhase !== undefined) { + return this.backgroundTaskTerminalPhase; + } if (this.result !== undefined) return this.result.is_error ? 'failed' : 'done'; return this.subagentPhase; } diff --git a/apps/kimi-code/src/tui/controllers/session-event-handler.ts b/apps/kimi-code/src/tui/controllers/session-event-handler.ts index 0e310ba91..3e2636662 100644 --- a/apps/kimi-code/src/tui/controllers/session-event-handler.ts +++ b/apps/kimi-code/src/tui/controllers/session-event-handler.ts @@ -754,6 +754,17 @@ export class SessionEventHandler { if (backgroundMeta !== undefined) { this.backgroundAgentMetadata.delete(event.subagentId); this.syncBackgroundAgentBadge(); + // Push the real subagent error onto the parent Agent card too — + // `background.task.terminated` arrives separately (possibly later) + // with no error string and would only stamp the generic + // `Background agent failed`. The card and the separate transcript + // entry now share the same actual reason. + streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: event.subagentId, + description: backgroundMeta.description ?? '', + status: 'failed', + errorText: event.error, + }); const taskId = this.findAgentTaskId(event.subagentId); if (taskId !== undefined && this.backgroundTaskTranscriptedTerminal.has(taskId)) { return; @@ -872,6 +883,17 @@ export class SessionEventHandler { } if (event.type === 'background.task.terminated' && isTerminal) { + if (info.taskId.startsWith('agent-')) { + // The Agent tool's spawn-success ToolResult is not an error, so the + // parent toolCall card would otherwise render `✓ Completed` for any + // terminated bg agent — including `lost` / `failed` / `killed`. + // Push the actual terminal status so the card matches reality. + this.host.streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: info.agentId, + description: info.description, + status: info.status, + }); + } if (!this.backgroundTaskTranscriptedTerminal.has(info.taskId)) { if (info.taskId.startsWith('bash-')) { this.appendBackgroundTaskEntry(info); diff --git a/apps/kimi-code/src/tui/controllers/session-replay.ts b/apps/kimi-code/src/tui/controllers/session-replay.ts index f806cd814..50d9f91ba 100644 --- a/apps/kimi-code/src/tui/controllers/session-replay.ts +++ b/apps/kimi-code/src/tui/controllers/session-replay.ts @@ -66,6 +66,7 @@ export class SessionReplayRenderer { this.hydrateSnapshot(main); this.renderRecords(main); + this.applyTerminalBackgroundAgentStatuses(main); return true; } catch (error) { const message = formatErrorMessage(error); @@ -104,6 +105,36 @@ export class SessionReplayRenderer { this.host.streamingUI.setTodoList(todos); } + /** + * Push real terminal status into each replayed `Agent` card whose + * backing background task is already in a terminal state. Runs AFTER + * `renderRecords` because the tool call components only exist once the + * replay has mounted them — `hydrateBackgroundState` runs too early to + * reach them. Without this, terminated bg agents (including ones that + * reconcile reclassified as `lost`) keep the spawn-success ToolResult's + * default of `✓ Completed`. + */ + private applyTerminalBackgroundAgentStatuses(agent: ResumedAgentState): void { + for (const info of agent.background) { + if (!info.taskId.startsWith('agent-')) continue; + if (!isTerminalBackgroundTask(info)) continue; + const status = info.status; + if ( + status !== 'completed' && + status !== 'failed' && + status !== 'killed' && + status !== 'lost' + ) { + continue; + } + this.host.streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: info.agentId, + description: info.description, + status, + }); + } + } + private hydrateBackgroundState(agent: ResumedAgentState): void { const { state, sessionEventHandler } = this.host; const projection = replayBackgroundProjection(agent.background); diff --git a/apps/kimi-code/src/tui/controllers/streaming-ui.ts b/apps/kimi-code/src/tui/controllers/streaming-ui.ts index 769df3767..cb0a33217 100644 --- a/apps/kimi-code/src/tui/controllers/streaming-ui.ts +++ b/apps/kimi-code/src/tui/controllers/streaming-ui.ts @@ -174,6 +174,97 @@ export class StreamingUIController { } } + /** + * Push the actual terminal status of a background agent task into the + * matching `Agent` tool call component so its snapshot phase no longer + * trusts the spawn-success ToolResult (which would otherwise label every + * terminated bg agent — including `lost` ones — as `✓ Completed`). + * + * Resolution policy: an `args.agentId` is treated as authoritative — we + * either find a card whose `getSubagentAgentId()` returns the same id + * (in-memory metadata for live foreground, parsed from the spawn-success + * `agent_id: ...` line for live backgrounded and replayed cards) or we + * skip. We deliberately do NOT fall back to description match when + * `agentId` is provided, because: + * - On resume, `applyTerminalBackgroundAgentStatuses` iterates every + * persisted terminal task, including ones whose tool calls fell + * outside the `REPLAY_TURN_LIMIT` window. A description fallback + * would let an old `lost` task stamp its status onto an unrelated + * recent Agent card that happens to share `args.description`. + * - During a live spawn / terminate race, the same card can briefly + * appear in both `_pendingToolComponents` and `transcriptContainer`, + * so a description match could double-visit the same component and + * mark itself ambiguous. agentId match short-circuits on the first + * hit and is immune. + * + * Description fallback is kept as a best-effort path only when + * `agentId` is unknown — that is, on resume of pre-PR sessions whose + * disk records pre-date `agent_id` persistence. + * + * Search scope includes both in-flight components and already-mounted + * cards (some live in `transcriptContainer` standalone, others are + * borrowed by an `AgentGroupComponent` and reachable only via + * `getToolComponents()`). + * + * Returns true iff a component was found and updated. + */ + applyBackgroundTaskTerminalStatus(args: { + agentId?: string | undefined; + description: string; + status: 'completed' | 'failed' | 'killed' | 'lost'; + /** + * Real failure message to surface on the card. Pass the `subagent.failed` + * event's `error` for live crashes — it is far more useful than the + * friendly generic the card falls back to. Omit on the resume / terminate + * path where no real error is available. + */ + errorText?: string | undefined; + }): boolean { + const useAgentIdOnly = args.agentId !== undefined; + let agentIdMatch: ToolCallComponent | undefined; + let descMatch: ToolCallComponent | undefined; + let descAmbiguous = false; + const visit = (tc: ToolCallComponent): void => { + if (agentIdMatch !== undefined) return; + if (useAgentIdOnly) { + if (tc.getSubagentAgentId() === args.agentId) agentIdMatch = tc; + return; + } + if (tc.getAgentToolDescription() !== args.description) return; + if (descMatch !== undefined) { + descAmbiguous = true; + return; + } + descMatch = tc; + }; + + for (const tc of this._pendingToolComponents.values()) { + visit(tc); + if (agentIdMatch !== undefined) break; + } + if (agentIdMatch === undefined) { + for (const child of this.host.state.transcriptContainer.children) { + if (child instanceof ToolCallComponent) { + visit(child); + } else if (child instanceof AgentGroupComponent) { + for (const tc of child.getToolComponents()) { + visit(tc); + if (agentIdMatch !== undefined) break; + } + } + if (agentIdMatch !== undefined) break; + } + } + const target = useAgentIdOnly + ? agentIdMatch + : descAmbiguous + ? undefined + : descMatch; + if (target === undefined) return false; + target.setBackgroundTaskTerminalStatus(args.status, { errorText: args.errorText }); + return true; + } + /** Registers a tool call that arrived via tool.call.started. * Clears any pending streaming state for this id, updates or creates the * component, and returns whether the call was new (no previous entry). */ diff --git a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts index 7e803a182..d57e9dfe5 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts @@ -739,6 +739,227 @@ describe('ToolCallComponent', () => { expect(out).not.toContain('Used Agent'); }); + describe('background agent terminal state vs spawn-success ToolResult', () => { + // The Agent tool returns a "task spawned" result the moment a + // run_in_background=true call lands. That result is not an error and its + // body says `status: running`, so for backgrounded agents `this.result` + // alone cannot distinguish a successful completion from a failure / lost + // task. The fix is `setBackgroundTaskTerminalStatus`, which overrides the + // result-based derivation with the actual BackgroundTaskInfo status. + const spawnSuccessResult = { + tool_call_id: 'call_bg_agent', + output: [ + 'task_id: agent-deadbeef', + 'status: running', + 'agent_id: agent-0', + 'actual_subagent_type: coder', + 'automatic_notification: true', + ].join('\n'), + is_error: false, + }; + + function makeBackgroundAgentComponent(): ToolCallComponent { + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { + description: 'background agent 1', + run_in_background: true, + }, + }, + spawnSuccessResult, + darkColors, + ); + component.onSubagentSpawned({ + agentId: 'agent-0', + agentName: 'coder', + runInBackground: true, + }); + return component; + } + + it('reads as "done" by default after spawn — the existing behavior the fix replaces', () => { + // This pins the legacy behavior. Without overrides the snapshot + // trusts the spawn-success result and reports phase='done'. The + // 'lost' / 'killed' / 'failed' overrides below must beat this. + const component = makeBackgroundAgentComponent(); + expect(component.getSubagentSnapshot().phase).toBe('done'); + }); + + it('setBackgroundTaskTerminalStatus("lost") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('lost'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + // The agent-group renderer uses snap.errorText for the "Error:" line. + // The spawn-success ToolResult must NOT leak as the failure message. + expect(snap.errorText).toContain('lost'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("killed") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('killed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + expect(snap.errorText).toContain('killed'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("failed") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + expect(snap.errorText).toContain('failed'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("completed") keeps the snapshot phase at "done"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('completed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('done'); + expect(snap.errorText).toBeUndefined(); + }); + + it('overrides win even when set before the spawn-success result is recorded', () => { + // Order-independence guard: reconcile may run before tool result + // has been replayed back into the component on some boot paths. + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'background agent A', run_in_background: true }, + }, + undefined, + darkColors, + ); + component.setBackgroundTaskTerminalStatus('lost'); + // Now the spawn-success result lands. + component.setResult({ ...spawnSuccessResult, tool_call_id: 'call_bg_agent' }); + expect(component.getSubagentSnapshot().phase).toBe('failed'); + }); + + // Standalone render path — when only ONE Agent tool call lands in a + // step, the card is never upgraded into an `AgentGroupComponent` and is + // mounted on its own. The standalone header derives its label from + // `getDerivedSubagentPhase()` (separate from `getSubagentSnapshot`). + // Without the override threading into that path AND a header rebuild, + // a lost bg agent keeps the green "✓ Completed" label. + it('standalone render: lost bg agent must show Failed/Lost, not Completed', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('lost'); + const out = strip(component.render(120).join('\n')); + expect(out).not.toContain('Completed'); + expect(out).toMatch(/Failed|Lost/); + // Friendly failure message must reach the rendered card. + expect(out).toContain('lost'); + expect(out).not.toContain('task_id:'); + }); + + it('standalone render: completed bg agent still shows Completed', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('completed'); + const out = strip(component.render(120).join('\n')); + expect(out).toContain('Completed'); + expect(out).not.toMatch(/Failed/); + expect(out).not.toContain('task_id:'); + }); + + // Stable id routing — `tc.subagentAgentId` is left undefined for + // backgrounded agents both live (`handleSubagentSpawned` early-returns + // for `runInBackground`, never calling tc.onSubagentSpawned) and on + // resume (the wire format does not carry a `subagent` block back into + // `applySubagentReplay`). The AgentTool's spawn-success ToolResult, + // however, always carries `agent_id: agent-N` — fall back to parsing + // that so callers asking `getSubagentAgentId` always get the right id, + // and `applyBackgroundTaskTerminalStatus` can route by id instead of + // by description (which collides between unrelated cards). + it('getSubagentAgentId parses agent_id from the spawn-success ToolResult', () => { + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'background agent 1', run_in_background: true }, + }, + spawnSuccessResult, + darkColors, + ); + // No spawn metadata was wired in — exactly the resume / backgrounded + // case we are guarding against. + expect(component.getSubagentAgentId()).toBe('agent-0'); + }); + + it('getSubagentAgentId still prefers in-memory subagent metadata when set', () => { + // If `setSubagentMeta` / `onSubagentSpawned` did wire an id, that one + // is authoritative — it survived the in-flight phase before any + // ToolResult landed and can disambiguate concurrent calls. + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'X', run_in_background: true }, + }, + spawnSuccessResult, + darkColors, + ); + component.setSubagentMeta('agent-explicit', 'coder'); + expect(component.getSubagentAgentId()).toBe('agent-explicit'); + }); + + it('getSubagentAgentId returns undefined for non-Agent tool calls even when output looks similar', () => { + const component = new ToolCallComponent( + { + id: 'call_bash', + name: 'Bash', + args: { command: 'echo agent_id: agent-fake' }, + }, + { + tool_call_id: 'call_bash', + output: 'agent_id: agent-fake\nstatus: running', + is_error: false, + }, + darkColors, + ); + expect(component.getSubagentAgentId()).toBeUndefined(); + }); + + it('setBackgroundTaskTerminalStatus errorText overwrites the friendly generic', () => { + // Live failures arrive via `subagent.failed` with the real error from + // the subagent loop. That string is far more informative than the + // generic "Background agent failed" fallback the friendly path emits. + // When the caller supplies errorText it must win, regardless of + // whether the friendly message was written first. + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed'); + expect(component.getSubagentSnapshot().errorText).toBe('Background agent failed'); + + component.setBackgroundTaskTerminalStatus('failed', { + errorText: 'subagent exceeded max_steps', + }); + expect(component.getSubagentSnapshot().errorText).toBe('subagent exceeded max_steps'); + }); + + it('setBackgroundTaskTerminalStatus errorText is written even on first call', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed', { + errorText: 'OAuth refresh failed', + }); + expect(component.getSubagentSnapshot().errorText).toBe('OAuth refresh failed'); + }); + + it('setBackgroundTaskTerminalStatus does not overwrite a real onSubagentFailed error with the generic', () => { + const component = makeBackgroundAgentComponent(); + component.onSubagentFailed({ error: 'real crash from subagent' }); + // background.task.terminated event arrives later without an errorText + // override; the friendly generic must NOT clobber the real message. + component.setBackgroundTaskTerminalStatus('failed'); + expect(component.getSubagentSnapshot().errorText).toBe('real crash from subagent'); + }); + }); + it('scrolls the Write streaming preview to the last COMMAND_PREVIEW_LINES', () => { const lines: string[] = []; for (let i = 1; i <= 30; i++) lines.push(`line${String(i)}`); diff --git a/packages/agent-core/src/agent/background/index.ts b/packages/agent-core/src/agent/background/index.ts index f8753df1b..190e3b720 100644 --- a/packages/agent-core/src/agent/background/index.ts +++ b/packages/agent-core/src/agent/background/index.ts @@ -17,6 +17,12 @@ type BackgroundTaskNotification = Record & { readonly type: string; readonly source_kind: 'background_task'; readonly source_id: string; + /** Subagent id for agent-* tasks. Surfaced as a structured attribute so + * the LLM can pass it verbatim to `Agent(resume=...)` without confusing + * it with `source_id` (the BackgroundManager ledger id). Omitted for + * bash background tasks and for restored tasks whose previous session + * pre-dates agent_id persistence. */ + readonly agent_id?: string | undefined; readonly title: string; readonly severity: 'info' | 'warning'; readonly body: string; @@ -126,19 +132,18 @@ export class BackgroundManager extends BackgroundProcessManager { const tailOutput = (await this.getOutputSnapshot(info.taskId, NOTIFICATION_TAIL_BYTES)) .preview; if (this.hasDeliveredNotification(origin)) return; - const label = info.taskId.startsWith('agent-') ? 'agent' : 'task'; + const isAgentTask = info.taskId.startsWith('agent-'); + const label = isAgentTask ? 'agent' : 'task'; const notification: BackgroundTaskNotification = { id: notificationId, category: 'task', type: `task.${info.status}`, source_kind: 'background_task', source_id: info.taskId, + agent_id: isAgentTask ? info.agentId : undefined, title: `Background ${label} ${info.status}`, severity: info.status === 'completed' ? 'info' : 'warning', - body: - info.status === 'killed' && info.stopReason - ? `${info.description} was killed: ${info.stopReason}.` - : `${info.description} ${info.status}.`, + body: buildBackgroundTaskNotificationBody(info, isAgentTask), tail_output: tailOutput, }; const content = [ @@ -191,3 +196,41 @@ export class BackgroundManager extends BackgroundProcessManager { function notificationKey(origin: BackgroundTaskOrigin): string { return `${origin.taskId}\0${origin.status}\0${origin.notificationId}`; } + +/** + * Build the human/LLM-readable body that lands in the `` + * XML. For agent-* tasks that ended non-successfully and whose subagent id + * we still know, append a paragraph telling the LLM exactly how to resume + * — which id to pass, how to distinguish it from the look-alike `source_id`, + * and what state the resumed subagent will and will not have. The intent is + * to make recovery a one-shot decision instead of a memory lookup against + * the original spawn-success ToolResult. + * + * Bash tasks, successful agent tasks, and restored agent tasks from + * sessions that pre-date `agent_id` persistence keep the original + * single-sentence body. + */ +function buildBackgroundTaskNotificationBody( + info: BackgroundTaskInfo, + isAgentTask: boolean, +): string { + const baseLine = + info.status === 'killed' && info.stopReason + ? `${info.description} was killed: ${info.stopReason}.` + : `${info.description} ${info.status}.`; + + if (!isAgentTask) return baseLine; + if (info.status === 'completed') return baseLine; + const agentId = info.agentId; + if (agentId === undefined || agentId === info.taskId) return baseLine; + + const recovery = [ + '', + `To recover or continue this subagent, call Agent(resume="${agentId}", prompt="Pick up where you left off; redo the last tool call if its result was never observed.").`, + `Use agent_id ("${agentId}"), NOT source_id / task_id ("${info.taskId}") — the two look alike but only agent_id is accepted by the resume parameter.`, + 'Add run_in_background=true to keep it backgrounded, or omit it to take the result inline in the current turn.', + 'The subagent retains its full prior context across the restart, but any in-flight tool call lost its result and may need to be redone.', + ].join('\n'); + + return `${baseLine}${recovery}`; +} diff --git a/packages/agent-core/src/agent/context/notification-xml.ts b/packages/agent-core/src/agent/context/notification-xml.ts index 72e584b48..45eda702c 100644 --- a/packages/agent-core/src/agent/context/notification-xml.ts +++ b/packages/agent-core/src/agent/context/notification-xml.ts @@ -3,7 +3,7 @@ * shared between the live ContextMemory and the projector. * * Output shape: - * + * * Title: ... * Severity: ... * @@ -15,6 +15,13 @@ * The opening-tag names (``) are * load-bearing for the projector's `mergeAdjacentUserMessages` detector * — rename requires updating the detector too. + * + * `agent_id` is emitted only for background_task notifications whose + * source task is an agent subagent — surfacing it structurally lets the + * LLM identify the correct id to pass to `Agent(resume=...)` without + * having to grep the body or the original spawn-success ToolResult. + * It is intentionally a separate attribute from `source_id`: the two + * look alike (`agent-...`) but live in different namespaces. */ export function renderNotificationXml(data: Record): string { @@ -23,12 +30,14 @@ export function renderNotificationXml(data: Record): string { const type = stringAttr(data['type'], 'unknown'); const sourceKind = stringAttr(data['source_kind'], 'unknown'); const sourceId = stringAttr(data['source_id'], 'unknown'); + const agentId = optionalStringAttr(data['agent_id']); const title = typeof data['title'] === 'string' ? data['title'] : ''; const severity = typeof data['severity'] === 'string' ? data['severity'] : ''; const body = typeof data['body'] === 'string' ? data['body'] : ''; + const agentIdAttr = agentId === undefined ? '' : ` agent_id="${agentId}"`; const lines: string[] = [ - ``, + ``, ]; if (title.length > 0) lines.push(`Title: ${title}`); if (severity.length > 0) lines.push(`Severity: ${severity}`); @@ -70,3 +79,10 @@ function stringAttr(value: unknown, fallback: string): string { // where double-escaping would be noisier than literal punctuation. return value.replaceAll('&', '&').replaceAll('"', '"'); } + +/** Like `stringAttr` but returns `undefined` instead of a fallback so the + * caller can omit the attribute entirely when the source value is absent. */ +function optionalStringAttr(value: unknown): string | undefined { + if (typeof value !== 'string' || value.length === 0) return undefined; + return value.replaceAll('&', '&').replaceAll('"', '"'); +} diff --git a/packages/agent-core/src/tools/background/manager.ts b/packages/agent-core/src/tools/background/manager.ts index 97f77fc9e..c87cbe667 100644 --- a/packages/agent-core/src/tools/background/manager.ts +++ b/packages/agent-core/src/tools/background/manager.ts @@ -1055,6 +1055,7 @@ export class BackgroundProcessManager { private persistLive(entry: ManagedProcess): Promise { if (this.sessionDir === undefined) return Promise.resolve(); const sessionDir = this.sessionDir; + const isAgentTask = entry.taskId.startsWith('agent-'); const task: PersistedTask = { task_id: entry.taskId, command: entry.command, @@ -1067,6 +1068,12 @@ export class BackgroundProcessManager { approval_reason: entry.approvalReason, timed_out: entry.timedOut, stop_reason: entry.stopReason, + // Only persist subagent identifiers for agent tasks. The base-class + // fallback `agentId ?? taskId` (registerAgentTask) makes them equal + // for tasks registered without an explicit id — skip those too so the + // disk record stays honest about whether we know a real agent_id. + agent_id: isAgentTask && entry.agentId !== entry.taskId ? entry.agentId : undefined, + subagent_type: isAgentTask ? entry.subagentType : undefined, }; entry.persistWriteQueue = entry.persistWriteQueue .then(() => writeTask(sessionDir, task)) @@ -1185,6 +1192,8 @@ function persistedToInfo(t: PersistedTask): BackgroundTaskInfo { approvalReason: t.approval_reason, timedOut: t.timed_out, stopReason: t.stop_reason, + agentId: t.agent_id, + subagentType: t.subagent_type, }; } @@ -1201,5 +1210,7 @@ function infoToPersisted(info: BackgroundTaskInfo): PersistedTask { approval_reason: info.approvalReason, timed_out: info.timedOut, stop_reason: info.stopReason, + agent_id: info.agentId === info.taskId ? undefined : info.agentId, + subagent_type: info.subagentType, }; } diff --git a/packages/agent-core/src/tools/background/persist.ts b/packages/agent-core/src/tools/background/persist.ts index 91725868a..aaa73b6f5 100644 --- a/packages/agent-core/src/tools/background/persist.ts +++ b/packages/agent-core/src/tools/background/persist.ts @@ -65,6 +65,18 @@ export interface PersistedTask { readonly cwd?: string | undefined; } | undefined; + /** + * Subagent identifier for agent-* tasks (the id `subagentHost.resume` + * accepts). Persisted so a session restart can re-emit recovery + * instructions in the next `` without forcing the LLM to + * cross-reference the original spawn-success ToolResult. Omitted for + * bash tasks. Optional in the schema for forward/backward compatibility: + * pre-PR sessions reload without it and simply skip the recovery hint. + */ + readonly agent_id?: string | undefined; + /** Subagent profile name (agent-* tasks only). Persisted for symmetry + * with `agent_id` so resume surfaces match between disk and memory. */ + readonly subagent_type?: string | undefined; } function tasksDirOf(sessionDir: string): string { diff --git a/packages/agent-core/src/tools/builtin/collaboration/agent.ts b/packages/agent-core/src/tools/builtin/collaboration/agent.ts index 9838f86f9..943d801fb 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/agent.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/agent.ts @@ -283,7 +283,7 @@ export class AgentTool implements BuiltinTool { `description: ${args.description}`, '', `next_step: The completion arrives automatically in a later turn — no polling needed. To peek at progress without blocking, call TaskOutput(task_id="${taskId}", block=false).`, - `resume_hint: To continue this same subagent instance later, call Agent(resume="${handle.agentId}", prompt="...").`, + `resume_hint: To continue or recover this same subagent later, call Agent(resume="${handle.agentId}", prompt="..."). The parameter is agent_id ("${handle.agentId}"), NOT task_id ("${taskId}") or source_id from a later . Recovery cases: a later for this subagent — its conversation history is preserved across session restarts and resume will pick it up.`, ]; return { output: lines.join('\n') }; } diff --git a/packages/agent-core/test/agent/background-manager.test.ts b/packages/agent-core/test/agent/background-manager.test.ts index 680c26a1b..e255d0c4d 100644 --- a/packages/agent-core/test/agent/background-manager.test.ts +++ b/packages/agent-core/test/agent/background-manager.test.ts @@ -622,6 +622,70 @@ describe('BackgroundManager — RPC event emission', () => { ); }); + describe('agent task failure body — actionable recovery instructions', () => { + // For agent-* tasks that end non-successfully (lost / failed / killed), + // the notification body must carry enough information for the LLM to + // recover via `Agent(resume=...)` without digging through old context. + // Three things must land in the body: + // 1. The agent_id (NOT the task_id / source_id) — that is what + // `subagentHost.resume` actually takes. + // 2. An explicit disambiguation between agent_id and source_id — + // they look alike and the LLM regularly confuses them. + // 3. The notification must also surface agent_id as a structural + // XML attribute, not just buried in prose. + it('failed agent task body includes resume instructions with the correct agent_id', async () => { + // Promise.reject (non-AbortError) routes through the registerAgentTask + // `.catch` branch and lands at status `failed`, which is the same + // agent-* failure branch reconcile uses for `lost` tasks. + const taskId = agent.background.registerAgentTask( + Promise.reject(new Error('subagent crashed')), + 'inspect repository', + { agentId: 'agent-7' }, + ); + await agent.background.waitForTerminal(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).toContain('agent_id="agent-7"'); + expect(text).toMatch(/Agent\(resume="agent-7"/); + expect(text).toMatch(/agent_id.*not.*source_id|source_id.*not.*agent_id/i); + }); + + it('completed agent task body does NOT add resume instructions', async () => { + const taskId = agent.background.registerAgentTask( + Promise.resolve({ result: 'all good' }), + 'inspect repository', + { agentId: 'agent-8' }, + ); + await agent.background.wait(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).toContain('agent_id="agent-8"'); + // Recovery prose belongs to failure bodies only. + expect(text).not.toMatch(/Agent\(resume="agent-8"/); + }); + + it('bash task body never mentions resume — bash background tasks are not resumable', async () => { + const taskId = agent.background.register(immediateProcess(1), 'false', 'shell'); + await agent.background.waitForTerminal(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).not.toContain('agent_id='); + expect(text).not.toMatch(/Agent\(resume=/); + }); + }); + // Note: the `records.restoring` guard is enforced inside `Agent.emitEvent` // (see agent/index.ts). BackgroundManager unconditionally forwards // lifecycle events to the agent; suppression is the agent's job. diff --git a/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts b/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts new file mode 100644 index 000000000..c9ba13d0b --- /dev/null +++ b/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts @@ -0,0 +1,308 @@ +/** + * Repro for bug: "after a group of background agents complete, the + * main agent doesn't receive notifications". + * + * Unlike `background-manager.test.ts` (which mocks `agent.turn.steer`), + * this file drives a real `Agent` instance so we can verify the + * full chain: + * + * onLiveTaskTerminal → notifyBackgroundTask → turn.steer() + * → (idle) launch() → turnWorker() → LLM generate called with + * the notification XML in history + * → (busy) buffered into steerBuffer → flushed on next loop step + * + * If either scenario fails to inject the notification into the next + * LLM call, the scripted LLM will throw "Unexpected generate call", + * making the failure mode explicit. + */ + +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'pathe'; + +import { describe, expect, it, vi } from 'vitest'; + +import { appendTaskOutput, writeTask } from '../../src/tools/background/persist'; +import { testAgent } from './harness/agent'; + +describe('background notification → main agent (real Agent instance)', () => { + it('IDLE: completed bg agent auto-starts a new turn with XML', async () => { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + expect(ctx.agent.turn.hasActiveTurn).toBe(false); + expect(ctx.llmCalls.length).toBe(0); + + // The expected auto-launched turn will call generate once, then end. + ctx.mockNextResponse({ type: 'text', text: 'ack from main agent' }); + + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'background agent finished its job' }), + 'idle-state repro', + ); + + await ctx.agent.background.waitForTerminal(taskId); + + // Give the steer→launch→turnWorker→generate chain time to run. + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(1); + }, + { timeout: 2000 }, + ); + + // The latest LLM call must include the notification XML the + // BackgroundManager injected via `turn.steer`. + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + expect(flatHistoryText).toContain(' { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // Step 1 of the user-prompted turn: produce no tool call, end turn. + // But to give the steerBuffer a chance to be flushed we want a + // multi-step turn. So instead: queue a text response for step 1 + // that DOESN'T end the turn yet (set finishReason to tool_calls + // is wrong because we have no tool call). Easiest is to chain two + // responses: first one is text-only (so step ends), the steer + // notification arrives during that step, then a second LLM call + // happens that should contain the notification. + // + // Actually with the scripted-generate harness, a text-only + // response yields finishReason='completed' and the turn ends. + // To force a 2-step turn we need the first step to emit a tool + // call. Since we configured no tools, we can't. So this BUSY + // case is hard to model without LLM-side multi-step. Instead we + // test the buffer mechanism directly: + + const steerSpy = vi.spyOn(ctx.agent.turn, 'steer'); + + // Pretend a turn is active by calling prompt and not awaiting end. + // Queue a response that will be consumed. + ctx.mockNextResponse({ type: 'text', text: 'first turn ack' }); + const promptPromise = ctx.rpc.prompt({ + input: [{ type: 'text', text: 'kick off a turn' }], + }); + + // Right after kicking off, register a background task that + // completes immediately. The notification should be steer()d + // while activeTurn is still set, landing in the steerBuffer. + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'busy-state bg result' }), + 'busy-state repro', + ); + + // Wait for the first turn to end. + await promptPromise; + await ctx.untilTurnEnd(); + + // steer() must have been called at least once for our task. + await vi.waitFor(() => { + expect(steerSpy).toHaveBeenCalled(); + }); + const matchingCall = steerSpy.mock.calls.find((c) => { + const origin = c[1] as { kind?: string; taskId?: string } | undefined; + return origin?.kind === 'background_task' && origin.taskId === taskId; + }); + expect(matchingCall).toBeDefined(); + + // After the turn ends, the steerBuffer should be flushed — + // i.e. the notification text appears as a user message in + // the agent's context history. + const data = ctx.agent.context.data(); + const flatContext = JSON.stringify(data); + expect(flatContext).toContain(' { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // Only one auto-launched turn is expected; its beforeStep should + // drain ALL buffered notifications. So one queued response is enough. + ctx.mockNextResponse({ type: 'text', text: 'ack group' }); + + const taskIds = [ + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #1 result' }), + 'group-1', + ), + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #2 result' }), + 'group-2', + ), + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #3 result' }), + 'group-3', + ), + ]; + + for (const id of taskIds) { + await ctx.agent.background.waitForTerminal(id); + } + + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(1); + }, + { timeout: 2000 }, + ); + + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + + // ⚠️ Each of the 3 tasks' notifications must show up in the LLM + // history of the (single) auto-launched turn. + for (const id of taskIds) { + expect(flatHistoryText).toContain(id); + } + expect(flatHistoryText).toContain('bg #1 result'); + expect(flatHistoryText).toContain('bg #2 result'); + expect(flatHistoryText).toContain('bg #3 result'); + }); + + it('RACE: bg completion fires AFTER LLM returns but BEFORE activeTurn is cleared', async () => { + // We're hunting a window: shouldContinueAfterStop reads an empty + // steerBuffer → returns { continue: false } → runTurn unwinds → + // finally block hasn't yet set activeTurn = null. If a steer() + // lands in this window, it gets buffered, then activeTurn=null + // and the buffer is never flushed until the next user prompt. + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // 1st turn: prompted by user — produces text and ends. + ctx.mockNextResponse({ type: 'text', text: 'first user-prompted ack' }); + + // Schedule the bg completion to fire when the first turn ends. + // The cleanest trigger: hook into the `turn.ended` event. + let onTurnEnded: () => void = () => {}; + const turnEndedPromise = new Promise((resolve) => { + onTurnEnded = resolve; + }); + ctx.emitter.on('turn.ended', () => { + onTurnEnded(); + }); + + // Kick off the user-prompted turn — don't await yet. + await ctx.rpc.prompt({ + input: [{ type: 'text', text: 'hello main agent' }], + }); + + // Wait until turn.ended fires. + await ctx.untilTurnEnd(); + await turnEndedPromise; + + // At this point activeTurn should be null. Now fire the bg + // completion — this is the IDLE path, NOT the racy one. We + // queue an LLM response so the auto-launched turn can run. + ctx.mockNextResponse({ type: 'text', text: 'auto ack from bg notification' }); + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'post-turn bg result' }), + 'race-after-turn', + ); + + await ctx.agent.background.waitForTerminal(taskId); + + // The notification arriving while idle should auto-launch a turn. + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(2); + }, + { timeout: 2000 }, + ); + + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + expect(flatHistoryText).toContain(' { + // Scenario the user described: kimi exits while bg tasks are + // running; on next start, resume() loads them from disk and + // reconcile() classifies them as terminal (lost for in-process + // agent tasks; possibly completed for bash tasks if the process + // wrote a terminal state). The restore path uses + // `appendUserMessage`, NOT `steer`, so: + // - Notification XML lands in context history ✓ + // - No new turn is launched ✗ + // - User sees nothing happen until they type + // + // This test pins that current behavior so any change shows up. + + const sessionDir = await mkdtemp(join(tmpdir(), 'kimi-bg-resume-repro-')); + try { + // Simulate a previous session's bash bg task that completed + // before exit and an agent bg task that didn't (will be lost). + await writeTask(sessionDir, { + task_id: 'bash-prev0000', + command: 'echo previous', + description: 'previous bash task', + pid: 12345, + started_at: 1_700_000_000, + ended_at: 1_700_000_005, + exit_code: 0, + status: 'completed', + }); + await appendTaskOutput(sessionDir, 'bash-prev0000', 'previous bash output'); + + await writeTask(sessionDir, { + task_id: 'agent-prev0000', + command: '[agent] previous agent task', + description: 'previous agent task', + pid: 0, + started_at: 1_700_000_000, + ended_at: null, + exit_code: null, + status: 'running', + }); + + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // We do NOT mock any LLM response. If the resume path + // mistakenly launches a turn, scripted-generate throws + // "Unexpected generate call" and the test fails loudly. + ctx.agent.background.attachSessionDir(sessionDir); + const steerSpy = vi.spyOn(ctx.agent.turn, 'steer'); + + // Reproduce Agent.resume()'s post-replay sequence. + await ctx.agent.background.loadFromDisk(); + const reconcileResult = await ctx.agent.background.reconcile(); + + // The agent-* running task should now be lost. + expect(reconcileResult.lost).toContain('agent-prev0000'); + + // Give the silent append a beat. + await vi.waitFor(() => { + const flatContext = JSON.stringify(ctx.agent.context.data()); + expect(flatContext).toContain('bash-prev0000'); + expect(flatContext).toContain('agent-prev0000'); + }); + + // Hard assertion: steer was NOT called for either restored task. + // The notifications were silently appended, so no new turn ran. + expect(steerSpy).not.toHaveBeenCalled(); + expect(ctx.llmCalls.length).toBe(0); + expect(ctx.agent.turn.hasActiveTurn).toBe(false); + + // Both notifications are in context, waiting for the user. + const flatContext = JSON.stringify(ctx.agent.context.data()); + expect(flatContext).toContain('previous bash output'); + expect(flatContext).toMatch(/task\.completed/); + expect(flatContext).toMatch(/task\.lost/); + } finally { + await rm(sessionDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 2d6f5ff41..5bf02e9f3 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -536,6 +536,42 @@ describe('Agent context notification projection', () => { expect(text.trimEnd()).toMatch(/<\/notification>$/); }); + it('renders an agent_id attribute when the notification carries one', () => { + // Background agent tasks (taskId starts with `agent-`) own a separate + // `agent_id` for the spawned subagent. Surfacing it as a top-level XML + // attribute lets the LLM resume the right thing without having to dig + // it out of the body or cross-reference the spawn-success ToolResult. + const text = renderNotificationXml({ + id: 'n_lost1', + category: 'task', + type: 'task.lost', + source_kind: 'background_task', + source_id: 'agent-w7gq3wwj', + agent_id: 'agent-0', + title: 'Background agent lost', + severity: 'warning', + body: 'Background agent 1 lost.', + }); + + expect(text).toContain('source_id="agent-w7gq3wwj"'); + expect(text).toContain('agent_id="agent-0"'); + }); + + it('omits the agent_id attribute when the notification does not carry one', () => { + const text = renderNotificationXml({ + id: 'n_bash', + category: 'task', + type: 'task.completed', + source_kind: 'background_task', + source_id: 'bash-abcdef00', + title: 'Background task completed', + severity: 'info', + body: 'echo done completed.', + }); + + expect(text).not.toContain('agent_id='); + }); + it('does not render task output blocks for non-task notifications', () => { const text = renderNotificationXml({ id: '', diff --git a/packages/agent-core/test/tools/agent.test.ts b/packages/agent-core/test/tools/agent.test.ts index 9a4e4837c..85fed2a10 100644 --- a/packages/agent-core/test/tools/agent.test.ts +++ b/packages/agent-core/test/tools/agent.test.ts @@ -475,6 +475,14 @@ describe('AgentTool', () => { // M9: resume_hint — continue the same subagent instance expect(result.output).toContain('resume_hint:'); expect(result.output).toContain('Agent(resume="agent-child"'); + // The hint disambiguates the two look-alike identifiers in this output: + // `agent_id` (what `subagentHost.resume` accepts) and `task_id` (the + // BackgroundManager ledger id, which also shows up as `source_id` in + // later entries). LLMs regularly copy the wrong one. + expect(result.output).toMatch(/agent_id.*not.*task_id|task_id.*not.*agent_id/i); + // Recovery scenario — `task.lost` etc. — must be called out so the + // model knows the hint is not only for happy-path follow-up work. + expect(result.output).toMatch(/task\.lost|task\.failed|task\.killed/); }); it('rejects background subagents when background management is unavailable', async () => { From e280f33daf7fbf1271c872dcb224737ec9518f73 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 29 May 2026 17:27:59 +0800 Subject: [PATCH 3/8] fix: recover from model token limit errors (#207) --- .changeset/retry-model-token-limit.md | 7 ++++ .../agent-core/src/agent/compaction/full.ts | 27 +++++++++---- .../agent-core/test/agent/compaction.test.ts | 39 +++++++++++++++++++ packages/kosong/src/errors.ts | 1 + packages/kosong/test/errors.test.ts | 1 + 5 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 .changeset/retry-model-token-limit.md diff --git a/.changeset/retry-model-token-limit.md b/.changeset/retry-model-token-limit.md new file mode 100644 index 000000000..221a6d416 --- /dev/null +++ b/.changeset/retry-model-token-limit.md @@ -0,0 +1,7 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kosong": patch +"@moonshot-ai/kimi-code": patch +--- + +Recover from provider model token limit errors during long conversations. diff --git a/packages/agent-core/src/agent/compaction/full.ts b/packages/agent-core/src/agent/compaction/full.ts index 34c699aa0..47925385c 100644 --- a/packages/agent-core/src/agent/compaction/full.ts +++ b/packages/agent-core/src/agent/compaction/full.ts @@ -46,6 +46,7 @@ export class FullCompaction { startedAt: number; telemetryTrigger: CompactionTelemetryTrigger; promise: Promise; + blockedByTurn: boolean; } | null = null; protected _compactedHistory: CompactedHistory[] = []; protected readonly strategy: CompactionStrategy; @@ -112,6 +113,7 @@ export class FullCompaction { startedAt: Date.now(), telemetryTrigger: compactionTelemetryTrigger(data.source, data.instruction), promise: Promise.resolve(), + blockedByTurn: false, }; this.compacting = active; active.promise = this.compactionWorker(abortController.signal, data, compactedCount); @@ -195,6 +197,7 @@ export class FullCompaction { private async block(signal: AbortSignal): Promise { const active = this.compacting; if (active) { + active.blockedByTurn = true; signal.addEventListener('abort', () => { if (this.compacting === active) { this.cancel(); @@ -312,19 +315,23 @@ export class FullCompaction { this.triggerPostCompactHook(data, result); } catch (error) { if (!isAbortError(error)) { + const active = this.compacting; + const blockedByTurn = active?.blockedByTurn === true; this.agent.log.error('compaction failed', { code: isKimiError(error) ? error.code : undefined, error, }); this.markCanceled(); - const payload = - isKimiError(error) && error.code === ErrorCodes.AUTH_LOGIN_REQUIRED - ? toKimiErrorPayload(error) - : makeErrorPayload(ErrorCodes.COMPACTION_FAILED, String(error)); - this.agent.emitEvent({ - type: 'error', - ...payload, - }); + if (!blockedByTurn) { + const payload = + isKimiError(error) && error.code === ErrorCodes.AUTH_LOGIN_REQUIRED + ? toKimiErrorPayload(error) + : makeErrorPayload(ErrorCodes.COMPACTION_FAILED, String(error)); + this.agent.emitEvent({ + type: 'error', + ...payload, + }); + } this.agent.telemetry.track('compaction_failed', { trigger_type: compactionTelemetryTrigger(data.source, data.instruction), before_tokens: tokensBefore, @@ -332,6 +339,10 @@ export class FullCompaction { retry_count: retryCount, error_type: error instanceof Error ? error.name : 'Unknown', }); + if (blockedByTurn) { + if (isKimiError(error) && error.code === ErrorCodes.AUTH_LOGIN_REQUIRED) throw error; + throw new KimiError(ErrorCodes.COMPACTION_FAILED, String(error), { cause: error }); + } } } } diff --git a/packages/agent-core/test/agent/compaction.test.ts b/packages/agent-core/test/agent/compaction.test.ts index 87e955ae3..cf02938c8 100644 --- a/packages/agent-core/test/agent/compaction.test.ts +++ b/packages/agent-core/test/agent/compaction.test.ts @@ -636,6 +636,45 @@ describe('Agent compaction', () => { await ctx.expectResumeMatches(); }); + it('fails a blocked turn when auto compaction generation fails', async () => { + let attempts = 0; + const generate: GenerateFn = async () => { + attempts += 1; + throw new APIStatusError(400, 'Bad request'); + }; + const ctx = testAgent({ generate, compactionStrategy: alwaysCompactOnce }); + ctx.configure(); + + await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Trigger failed auto compaction' }] }); + const events = await ctx.untilTurnEnd(); + + expect(attempts).toBe(1); + expect(events).not.toContainEqual(expect.objectContaining({ event: 'error' })); + expect(events).toContainEqual( + expect.objectContaining({ + event: 'turn.ended', + args: { + turnId: 0, + reason: 'failed', + error: expect.objectContaining({ + code: 'compaction.failed', + message: 'APIStatusError: Bad request', + }), + }, + }), + ); + const errorEvents = ctx.newEvents(); + expect(errorEvents).toHaveLength(1); + expect(errorEvents[0]).toMatchObject({ + event: 'error', + args: expect.objectContaining({ + code: 'compaction.failed', + message: 'APIStatusError: Bad request', + }), + }); + await ctx.expectResumeMatches(); + }); + it('reports compaction retry_count when retryable generation failures are exhausted', async () => { vi.useFakeTimers(); const records: TelemetryRecord[] = []; diff --git a/packages/kosong/src/errors.ts b/packages/kosong/src/errors.ts index ba104a2e4..df2ae6adb 100644 --- a/packages/kosong/src/errors.ts +++ b/packages/kosong/src/errors.ts @@ -82,6 +82,7 @@ const CONTEXT_OVERFLOW_MESSAGE_PATTERNS = [ /(?:too many tokens.*(?:prompt|input|context)|(?:prompt|input|context).*too many tokens)/, /prompt is too long.*maximum/, /input token count.*exceeds?.*maximum number of tokens/, + /request.*exceed(?:ed|s|ing)?.*model token limit/, ] as const; export function normalizeAPIStatusError( diff --git a/packages/kosong/test/errors.test.ts b/packages/kosong/test/errors.test.ts index 414b82312..70d5ec852 100644 --- a/packages/kosong/test/errors.test.ts +++ b/packages/kosong/test/errors.test.ts @@ -154,6 +154,7 @@ describe('normalizeAPIStatusError', () => { [422, 'Too many tokens in prompt'], [400, 'prompt is too long: 210000 tokens exceeds the maximum'], [400, 'input token count 131072 exceeds the maximum number of tokens allowed'], + [400, 'Invalid request: Your request exceeded model token limit: 262144 (requested: 274613)'], ])('normalizes %i "%s" to APIContextOverflowError', (statusCode, message) => { const error = normalizeAPIStatusError(statusCode, message, 'req-context'); expect(error).toBeInstanceOf(APIContextOverflowError); From 54590d3d464b05eed0837a725b37f3aa491c09af Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 29 May 2026 19:29:00 +0800 Subject: [PATCH 4/8] fix: back off compaction overflow retries by token budget (#211) --- .changeset/compact-overflow-retry-budget.md | 6 +++++ .../src/agent/compaction/strategy.ts | 17 ++++++++++++-- .../agent-core/test/agent/compaction.test.ts | 22 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 .changeset/compact-overflow-retry-budget.md diff --git a/.changeset/compact-overflow-retry-budget.md b/.changeset/compact-overflow-retry-budget.md new file mode 100644 index 000000000..72b47ce16 --- /dev/null +++ b/.changeset/compact-overflow-retry-budget.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +--- + +Back off failed compaction retries by a fixed slice of the model context window. diff --git a/packages/agent-core/src/agent/compaction/strategy.ts b/packages/agent-core/src/agent/compaction/strategy.ts index 98b3fd801..dacf94de4 100644 --- a/packages/agent-core/src/agent/compaction/strategy.ts +++ b/packages/agent-core/src/agent/compaction/strategy.ts @@ -10,6 +10,7 @@ export interface CompactionConfig { maxRecentMessages: number; maxRecentUserMessages: number; maxRecentSizeRatio: number; + minOverflowReductionRatio: number; } export const DEFAULT_COMPACTION_CONFIG: CompactionConfig = { @@ -20,6 +21,7 @@ export const DEFAULT_COMPACTION_CONFIG: CompactionConfig = { maxRecentMessages: 4, maxRecentUserMessages: Infinity, maxRecentSizeRatio: 0.2, + minOverflowReductionRatio: 0.05, }; export interface CompactionStrategy { @@ -117,12 +119,23 @@ export class DefaultCompactionStrategy implements CompactionStrategy { } reduceCompactOnOverflow(messages: readonly Message[]): number { + const minReducedSize = Math.max( + 1, + Math.ceil(this.maxSize * this.config.minOverflowReductionRatio), + ); + let reducedSize = 0; + let bestN: number | undefined; + for (let i = messages.length - 2; i > 0; i--) { + reducedSize += estimateTokensForMessage(messages[i + 1]!); if (canSplitAfter(messages, i)) { - return i + 1; + bestN = i + 1; + if (reducedSize >= minReducedSize) { + return i + 1; + } } } - return messages.length; + return bestN ?? messages.length; } get checkAfterStep(): boolean { diff --git a/packages/agent-core/test/agent/compaction.test.ts b/packages/agent-core/test/agent/compaction.test.ts index cf02938c8..cbd9c4371 100644 --- a/packages/agent-core/test/agent/compaction.test.ts +++ b/packages/agent-core/test/agent/compaction.test.ts @@ -15,6 +15,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import type { AgentOptions } from '../../src/agent'; import { DefaultCompactionStrategy, type CompactionStrategy } from '../../src/agent/compaction'; import { HookEngine, type HookEngineTriggerArgs } from '../../src/session/hooks'; +import { estimateTokensForMessages } from '../../src/utils/tokens'; import { recordingTelemetry, type TelemetryRecord } from '../fixtures/telemetry'; import type { TestAgentContext, TestAgentOptions } from './harness/agent'; import { testAgent } from './harness/agent'; @@ -132,6 +133,24 @@ describe('Agent compaction', () => { expect(strategy.shouldBlock(210_000)).toBe(true); }); + it('backs off overflow compaction by at least five percent of the context window', () => { + const strategy = testCompactionStrategy(1_000); + const messages = [ + textMessage('user', 'old user'), + textMessage('assistant', 'old assistant'), + ...Array.from({ length: 20 }, () => [ + textMessage('user', 'continue'), + textMessage('assistant', ''), + ]).flat(), + ]; + + const reduced = strategy.reduceCompactOnOverflow(messages); + const removed = messages.slice(reduced); + + expect(reduced).toBeGreaterThan(0); + expect(estimateTokensForMessages(removed)).toBeGreaterThanOrEqual(50); + }); + it('ignores reserved context when the reserve is not smaller than the model window', () => { const strategy = new DefaultCompactionStrategy(() => 32_000, { triggerRatio: 0.85, @@ -141,6 +160,7 @@ describe('Agent compaction', () => { maxRecentMessages: 3, maxRecentUserMessages: Infinity, maxRecentSizeRatio: 0.2, + minOverflowReductionRatio: 0.05, }); expect(strategy.shouldCompact(1)).toBe(false); @@ -1601,6 +1621,7 @@ function testCompactionStrategy(maxSize: number = 1_000): DefaultCompactionStrat maxRecentMessages: 10, maxRecentUserMessages: Infinity, maxRecentSizeRatio: 0.2, + minOverflowReductionRatio: 0.05, }); } @@ -1613,6 +1634,7 @@ function overflowOnlyCompactionStrategy(maxSize: number = 14): DefaultCompaction maxRecentMessages: 3, maxRecentUserMessages: Infinity, maxRecentSizeRatio: 0.2, + minOverflowReductionRatio: 0.05, }); } From 2388f20bb3d039e89caefca159801059b90dc64a Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 29 May 2026 19:38:24 +0800 Subject: [PATCH 5/8] fix: handle structured context overflow errors (#213) --- .changeset/context-overflow-responses.md | 7 +++ packages/agent-core/src/agent/turn/index.ts | 14 +---- packages/agent-core/test/agent/turn.test.ts | 11 ++++ packages/kosong/src/errors.ts | 6 ++- packages/kosong/src/index.ts | 1 + .../kosong/src/providers/openai-responses.ts | 53 +++++++++++++++---- packages/kosong/test/openai-responses.test.ts | 31 ++++++++++- 7 files changed, 100 insertions(+), 23 deletions(-) create mode 100644 .changeset/context-overflow-responses.md diff --git a/.changeset/context-overflow-responses.md b/.changeset/context-overflow-responses.md new file mode 100644 index 000000000..ccefcfaa5 --- /dev/null +++ b/.changeset/context-overflow-responses.md @@ -0,0 +1,7 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +"@moonshot-ai/kosong": patch +--- + +Handle context overflow errors consistently across provider responses. diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 102b7f5de..068d5626c 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -7,6 +7,7 @@ import { APIStatusError, APITimeoutError, inputTotal, + isContextOverflowStatusError, type ContentPart, type TokenUsage, } from '@moonshot-ai/kosong'; @@ -738,7 +739,7 @@ function classifyApiError(error: unknown, summary: KimiErrorPayload): ApiErrorCl if (statusCode === 429) return { errorType: 'rate_limit', statusCode }; if (statusCode === 401 || statusCode === 403) return { errorType: 'auth', statusCode }; if (statusCode >= 500) return { errorType: '5xx_server', statusCode }; - if (isContextOverflowMessage(summary.message)) { + if (isContextOverflowStatusError(statusCode, summary.message)) { return { errorType: 'context_overflow', statusCode }; } if (statusCode >= 400) return { errorType: '4xx_client', statusCode }; @@ -787,17 +788,6 @@ function isApiEmptyResponseError(error: unknown, summary: KimiErrorPayload): boo return error instanceof APIEmptyResponseError || summary.name === 'APIEmptyResponseError'; } -function isContextOverflowMessage(message: string): boolean { - const lower = message.toLowerCase(); - return ( - lower.includes('context length') || - lower.includes('context_length') || - lower.includes('max tokens') || - lower.includes('maximum context') || - lower.includes('too many tokens') - ); -} - function currentTurnInputTokens(usage: TokenUsage | undefined): number | undefined { if (usage === undefined) return undefined; return inputTotal(usage); diff --git a/packages/agent-core/test/agent/turn.test.ts b/packages/agent-core/test/agent/turn.test.ts index b39731e0b..094e06fcb 100644 --- a/packages/agent-core/test/agent/turn.test.ts +++ b/packages/agent-core/test/agent/turn.test.ts @@ -1056,6 +1056,17 @@ describe('Agent turn flow', () => { errorType: 'context_overflow', statusCode: 422, }, + { + name: 'context overflow token count status', + createError: () => + new APIStatusError( + 400, + 'input token count 131072 exceeds the maximum number of tokens allowed', + 'req-token-count', + ), + errorType: 'context_overflow', + statusCode: 400, + }, { name: 'connection error', createError: () => new APIConnectionError('socket hang up'), diff --git a/packages/kosong/src/errors.ts b/packages/kosong/src/errors.ts index df2ae6adb..6804fe7ea 100644 --- a/packages/kosong/src/errors.ts +++ b/packages/kosong/src/errors.ts @@ -85,6 +85,10 @@ const CONTEXT_OVERFLOW_MESSAGE_PATTERNS = [ /request.*exceed(?:ed|s|ing)?.*model token limit/, ] as const; +export function isContextOverflowErrorCode(code: string | null | undefined): boolean { + return code === 'context_length_exceeded'; +} + export function normalizeAPIStatusError( statusCode: number, message: string, @@ -96,7 +100,7 @@ export function normalizeAPIStatusError( return new APIStatusError(statusCode, message, requestId); } -function isContextOverflowStatusError(statusCode: number, message: string): boolean { +export function isContextOverflowStatusError(statusCode: number, message: string): boolean { if (statusCode !== 400 && statusCode !== 413 && statusCode !== 422) return false; const lowerMessage = message.toLowerCase(); return CONTEXT_OVERFLOW_MESSAGE_PATTERNS.some((pattern) => pattern.test(lowerMessage)); diff --git a/packages/kosong/src/index.ts b/packages/kosong/src/index.ts index 35401dc79..3404bf36d 100644 --- a/packages/kosong/src/index.ts +++ b/packages/kosong/src/index.ts @@ -60,6 +60,7 @@ export { APIStatusError, APITimeoutError, ChatProviderError, + isContextOverflowStatusError, isRetryableGenerateError, } from './errors'; diff --git a/packages/kosong/src/providers/openai-responses.ts b/packages/kosong/src/providers/openai-responses.ts index 7a9147e8d..4c1677b88 100644 --- a/packages/kosong/src/providers/openai-responses.ts +++ b/packages/kosong/src/providers/openai-responses.ts @@ -1,5 +1,5 @@ import type { ModelCapability } from '#/capability'; -import { ChatProviderError } from '#/errors'; +import { APIContextOverflowError, ChatProviderError, isContextOverflowErrorCode } from '#/errors'; import type { ContentPart, Message, StreamedMessagePart, ToolCall } from '#/message'; import { extractText } from '#/message'; import type { @@ -217,12 +217,39 @@ function formatResponsesErrorEvent( return `${codeText}: ${message}${paramText}`; } -function formatResponsesFailedResponse(response: RawObject): string { +function errorFromOpenAIResponsesEvent( + prefix: string, + code: string | null, + message: string, + param: string | null, +): ChatProviderError { + const formatted = formatResponsesErrorEvent(code, message, param); + const fullMessage = `${prefix}: ${formatted}`; + if (isContextOverflowErrorCode(code)) { + return new APIContextOverflowError(400, fullMessage); + } + return new ChatProviderError(fullMessage); +} + +function readResponsesFailedResponseError(response: RawObject): + | { + code: string | null; + message: string; + } + | undefined { const error = readObjectField(response, 'error'); if (error !== undefined) { const code = readNullableStringField(error, 'code') ?? 'unknown'; const message = readStringField(error, 'message') ?? 'no message'; - return `${code}: ${message}`; + return { code, message }; + } + return undefined; +} + +function formatResponsesFailedResponse(response: RawObject): string { + const error = readResponsesFailedResponseError(response); + if (error !== undefined) { + return formatResponsesErrorEvent(error.code, error.message, null); } const incompleteDetails = readObjectField(response, 'incomplete_details'); @@ -777,16 +804,24 @@ export class OpenAIResponsesStreamedMessage implements StreamedMessage { } case 'error': { const message = requireStringField(chunk, 'message', type); - throw new ChatProviderError( - `OpenAI Responses stream error: ${formatResponsesErrorEvent( - readNullableStringField(chunk, 'code') ?? null, - message, - readNullableStringField(chunk, 'param') ?? null, - )}`, + throw errorFromOpenAIResponsesEvent( + 'OpenAI Responses stream error', + readNullableStringField(chunk, 'code') ?? null, + message, + readNullableStringField(chunk, 'param') ?? null, ); } case 'response.failed': { const responseObject = requireObjectField(chunk, 'response', type); + const error = readResponsesFailedResponseError(responseObject); + if (error !== undefined) { + throw errorFromOpenAIResponsesEvent( + 'OpenAI Responses response.failed', + error.code, + error.message, + null, + ); + } throw new ChatProviderError( `OpenAI Responses response.failed: ${formatResponsesFailedResponse(responseObject)}`, ); diff --git a/packages/kosong/test/openai-responses.test.ts b/packages/kosong/test/openai-responses.test.ts index 266f7c776..93e27b7d8 100644 --- a/packages/kosong/test/openai-responses.test.ts +++ b/packages/kosong/test/openai-responses.test.ts @@ -1,4 +1,4 @@ -import { ChatProviderError } from '#/errors'; +import { APIContextOverflowError, ChatProviderError } from '#/errors'; import { generate } from '#/generate'; import type { ContentPart, Message, StreamedMessagePart, ToolCall } from '#/message'; import { @@ -1635,6 +1635,35 @@ describe('OpenAIResponsesChatProvider', () => { ); }); + it('normalizes response.failed context overflow events', async () => { + const events = [ + { + type: 'response.failed', + response: { + id: 'resp_context_overflow', + status: 'failed', + error: { + code: 'context_length_exceeded', + message: + 'Your input exceeds the context window of this model. Please adjust your input and try again.', + }, + }, + }, + ]; + const stream = new OpenAIResponsesStreamedMessage(makeAsyncIterable(events), true); + + let caughtError: unknown; + try { + await collectStreamParts(stream); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeInstanceOf(APIContextOverflowError); + expect((caughtError as APIContextOverflowError).statusCode).toBe(400); + expect((caughtError as Error).message).toMatch(/context_length_exceeded/); + }); + it('throws when a known stream event is missing a required field', async () => { const stream = new OpenAIResponsesStreamedMessage( makeAsyncIterable([{ type: 'response.output_text.delta' }]), From caaa6d83ee262ba4c954386458ee13aacdb26e1a Mon Sep 17 00:00:00 2001 From: liruifengv Date: Fri, 29 May 2026 19:45:01 +0800 Subject: [PATCH 6/8] fix(update): don't report success when native update fails (#214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(update): don't report success when native update fails The native auto-updater spawned `bash -c "curl -fsSL … | bash"`. A pipeline's exit status is that of its last command, so when curl could not connect (e.g. a dead proxy) it produced no output, the trailing bash read empty stdin and exited 0, and the whole command looked successful — printing "Updated … Restart the CLI" while nothing had been installed. Run the spawned shell with `set -o pipefail` so curl's non-zero status propagates. installUpdate() then rejects and runUpdatePreflight() warns and continues on the current version instead of claiming success. * chore: add changeset for native update fix --- .changeset/fix-native-update-false-success.md | 5 ++ apps/kimi-code/src/cli/update/preflight.ts | 9 +++- .../test/cli/update/preflight.test.ts | 46 +++++++++++++++---- 3 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 .changeset/fix-native-update-false-success.md diff --git a/.changeset/fix-native-update-false-success.md b/.changeset/fix-native-update-false-success.md new file mode 100644 index 000000000..b373dd789 --- /dev/null +++ b/.changeset/fix-native-update-false-success.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix the native self-updater reporting a successful update when the install command actually failed. diff --git a/apps/kimi-code/src/cli/update/preflight.ts b/apps/kimi-code/src/cli/update/preflight.ts index 385f1807e..692784d4e 100644 --- a/apps/kimi-code/src/cli/update/preflight.ts +++ b/apps/kimi-code/src/cli/update/preflight.ts @@ -77,7 +77,7 @@ interface SpawnCommand { readonly args: readonly string[]; } -function spawnForSource( +export function spawnForSource( source: InstallSource, version: string, platform: NodeJS.Platform, @@ -92,7 +92,12 @@ function spawnForSource( case 'bun-global': return { cmd: bunCommand(platform), args: ['add', '-g', `${NPM_PACKAGE_NAME}@${version}`] }; case 'native': - return { cmd: 'bash', args: ['-c', NATIVE_INSTALL_COMMAND_UNIX] }; + // `curl … | bash` reports only the trailing bash's exit status, so a + // failed download (curl can't connect → empty stdin → bash exits 0) + // would look like a successful update. `pipefail` makes the pipeline + // surface curl's non-zero status so installUpdate() rejects and we warn + // instead of printing "Updated …". + return { cmd: 'bash', args: ['-c', `set -o pipefail; ${NATIVE_INSTALL_COMMAND_UNIX}`] }; case 'unsupported': throw new Error('unsupported install source cannot be auto-installed'); } diff --git a/apps/kimi-code/test/cli/update/preflight.test.ts b/apps/kimi-code/test/cli/update/preflight.test.ts index dc115c9e4..e497cba53 100644 --- a/apps/kimi-code/test/cli/update/preflight.test.ts +++ b/apps/kimi-code/test/cli/update/preflight.test.ts @@ -1,10 +1,11 @@ import type * as ChildProcess from 'node:child_process'; +import { spawnSync } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { readUpdateCache } from '#/cli/update/cache'; -import { runUpdatePreflight } from '#/cli/update/preflight'; +import { runUpdatePreflight, spawnForSource } from '#/cli/update/preflight'; import { promptForInstallConfirmation } from '#/cli/update/prompt'; import type * as PromptModule from '#/cli/update/prompt'; import { refreshUpdateCache } from '#/cli/update/refresh'; @@ -182,7 +183,7 @@ describe('runUpdatePreflight', () => { ); }); - it('native on darwin: spawns bash -c curl|bash', async () => { + it('native on darwin: spawns bash -c with pipefail-guarded curl|bash', async () => { mocks.readUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.refreshUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.detectInstallSource.mockResolvedValue('native'); @@ -193,11 +194,16 @@ describe('runUpdatePreflight', () => { try { const { options } = captureOutput(); await runUpdatePreflight('0.4.0', options); - expect(mocks.spawn).toHaveBeenCalledWith( - 'bash', - ['-c', expect.stringContaining('curl -fsSL https://code.kimi.com/kimi-code/install.sh')], - { stdio: 'inherit' }, - ); + const call = mocks.spawn.mock.calls[0]; + expect(call?.[0]).toBe('bash'); + expect(call?.[2]).toEqual({ stdio: 'inherit' }); + const [flag, script] = call?.[1] as string[]; + expect(flag).toBe('-c'); + // pipefail must come before the pipeline so a failed `curl` is not masked + // by the trailing `bash` exiting 0 (see "surfaces a failed curl" below). + expect(script).toContain('set -o pipefail'); + expect(script).toContain('curl -fsSL https://code.kimi.com/kimi-code/install.sh'); + expect(script).toContain('| bash'); } finally { Object.defineProperty(process, 'platform', { value: originalPlatform }); } @@ -240,15 +246,17 @@ describe('runUpdatePreflight', () => { expect(mocks.spawn).not.toHaveBeenCalled(); }); - it('warns and continues when spawn exits non-zero', async () => { + it('warns and continues when spawn exits non-zero, without claiming success', async () => { mocks.readUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.refreshUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.detectInstallSource.mockResolvedValue('npm-global'); mocks.promptForInstallConfirmation.mockResolvedValue(true); mockSpawnExit(1); - const { stderr, options } = captureOutput(); + const { stdout, stderr, options } = captureOutput(); await expect(runUpdatePreflight('0.4.0', options)).resolves.toBe('continue'); expect(stderr.join('')).toContain('warning: failed to install'); + // A failed install must never print the "Updated …" success line. + expect(stdout.join('')).not.toContain('Updated @moonshot-ai/kimi-code'); }); it('tracks update_prompted telemetry', async () => { @@ -267,3 +275,23 @@ describe('runUpdatePreflight', () => { })); }); }); + +describe('spawnForSource native', () => { + // No spawn mock here — we run real bash to prove the failure contract + // end-to-end. `curl … | bash` reports only the trailing bash's exit status, + // so a curl that never connects (exit 7, empty stdin → bash exits 0) is + // masked and the update is wrongly reported as successful. `set -o pipefail` + // makes the pipeline surface curl's failure. Shadowing `curl` with a shell + // function keeps this offline and deterministic; skipped on Windows (no bash, + // and native auto-install is unsupported there anyway). + it.skipIf(process.platform === 'win32')( + 'surfaces a failed curl download as a non-zero exit', + () => { + const { cmd, args } = spawnForSource('native', '0.5.0', 'darwin'); + const script = `curl() { return 7; }\n${args[1] ?? ''}`; + const result = spawnSync(cmd, [args[0] ?? '-c', script], { encoding: 'utf8' }); + expect(result.error).toBeUndefined(); + expect(result.status).toBeGreaterThan(0); + }, + ); +}); From b9860e9f6ec65eb5dfdabbad54f1a87d69f4f00a Mon Sep 17 00:00:00 2001 From: qer Date: Fri, 29 May 2026 19:51:23 +0800 Subject: [PATCH 7/8] feat: align datasource plugin with generic workflow (#215) --- .changeset/use-generic-datasource-stock.md | 5 + .../test/utils/kimi-datasource-plugin.test.ts | 35 ++++++ plugins/marketplace.json | 2 +- plugins/official/kimi-datasource/CHANGELOG.md | 6 + plugins/official/kimi-datasource/SKILL.md | 30 ++--- .../kimi-datasource/bin/kimi-datasource.mjs | 105 +++--------------- .../official/kimi-datasource/kimi.plugin.json | 2 +- 7 files changed, 72 insertions(+), 113 deletions(-) create mode 100644 .changeset/use-generic-datasource-stock.md create mode 100644 plugins/official/kimi-datasource/CHANGELOG.md diff --git a/.changeset/use-generic-datasource-stock.md b/.changeset/use-generic-datasource-stock.md new file mode 100644 index 000000000..b41f46ab9 --- /dev/null +++ b/.changeset/use-generic-datasource-stock.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Align the datasource plugin with the generic two-tool workflow. diff --git a/apps/kimi-code/test/utils/kimi-datasource-plugin.test.ts b/apps/kimi-code/test/utils/kimi-datasource-plugin.test.ts index 815e4f8d2..3219cc80d 100644 --- a/apps/kimi-code/test/utils/kimi-datasource-plugin.test.ts +++ b/apps/kimi-code/test/utils/kimi-datasource-plugin.test.ts @@ -11,6 +11,41 @@ const REPO_ROOT = join(import.meta.dirname, '../../../..'); const SERVER_ENTRY = join(REPO_ROOT, 'plugins/official/kimi-datasource/bin/kimi-datasource.mjs'); describe('kimi-datasource MCP server', () => { + it('exposes the same two generic tools as the Python plugin', async () => { + const tempDir = await mkdtemp(join(tmpdir(), 'kimi-datasource-plugin-')); + const kimiHome = join(tempDir, 'kimi-home'); + let child: ChildProcessWithoutNullStreams | undefined; + + try { + await mkdir(join(kimiHome, 'credentials'), { recursive: true }); + await writeFile( + join(kimiHome, 'credentials', 'kimi-code.json'), + JSON.stringify({ access_token: 'test-token', expires_at: 4_102_444_800 }), + 'utf8', + ); + child = spawn(process.execPath, [SERVER_ENTRY], { + cwd: REPO_ROOT, + env: { + ...process.env, + KIMI_CODE_HOME: kimiHome, + }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + const client = createRpcClient(child); + + await client.request('initialize', {}); + const result = await client.request('tools/list', {}); + + expect(result.error).toBeUndefined(); + const tools = (result.result as { tools: Array<{ name: string }> }).tools; + expect(tools.map((tool) => tool.name)).toEqual(['call_data_source_tool', 'get_data_source_desc']); + } finally { + child?.stdin.end(); + child?.kill(); + await rm(tempDir, { recursive: true, force: true }); + } + }); + it('prefers assistant text and writes response files', async () => { const tempDir = await mkdtemp(join(tmpdir(), 'kimi-datasource-plugin-')); const kimiHome = join(tempDir, 'kimi-home'); diff --git a/plugins/marketplace.json b/plugins/marketplace.json index dd18a9cd2..ffa4fef84 100644 --- a/plugins/marketplace.json +++ b/plugins/marketplace.json @@ -5,7 +5,7 @@ "id": "kimi-datasource", "tier": "official", "displayName": "Kimi Datasource", - "version": "3.0.0", + "version": "3.1.0", "description": "Official datasource workflows.", "keywords": ["data", "mcp"], "source": "./official/kimi-datasource" diff --git a/plugins/official/kimi-datasource/CHANGELOG.md b/plugins/official/kimi-datasource/CHANGELOG.md new file mode 100644 index 000000000..087ef264c --- /dev/null +++ b/plugins/official/kimi-datasource/CHANGELOG.md @@ -0,0 +1,6 @@ +# Changelog + +## 3.1.0 - 2026-05-29 + +- Align the MCP server with the Python plugin's generic two-tool workflow. +- Remove the `query_stock` shortcut; use `get_data_source_desc` before `call_data_source_tool`. diff --git a/plugins/official/kimi-datasource/SKILL.md b/plugins/official/kimi-datasource/SKILL.md index 4375de522..364b00eb7 100644 --- a/plugins/official/kimi-datasource/SKILL.md +++ b/plugins/official/kimi-datasource/SKILL.md @@ -2,20 +2,19 @@ name: kimi-datasource description: | 通用数据源助手。当用户要查股票/财报/技术指标/全球宏观经济/中国企业工商/学术论文这类外部数据时，使用这个 skill。 - 本 plugin 通过 MCP server `plugin-kimi-datasource-data` 提供工具；优先调用 `mcp__plugin-kimi-datasource-data__query_stock`、`mcp__plugin-kimi-datasource-data__get_data_source_desc`、`mcp__plugin-kimi-datasource-data__call_data_source_tool`。 + 本 plugin 通过 MCP server `plugin-kimi-datasource-data` 提供工具；按 `mcp__plugin-kimi-datasource-data__get_data_source_desc` → `mcp__plugin-kimi-datasource-data__call_data_source_tool` 的流程调用。 --- # kimi-datasource — 通用数据源助手 ## 0. 调用方式 -本 skill 使用 datasource MCP server 注册的三个工具，不要通过 Bash 手动执行脚本： +本 skill 使用 datasource MCP server 注册的两个工具，不要通过 Bash 手动执行脚本： -- `mcp__plugin-kimi-datasource-data__query_stock` - `mcp__plugin-kimi-datasource-data__get_data_source_desc` - `mcp__plugin-kimi-datasource-data__call_data_source_tool` -这三个工具由 Kimi Code 托管执行，参数直接按 tool schema 传 JSON。 +这两个工具由 Kimi Code 托管执行，参数直接按 tool schema 传 JSON。 工具会读取 `$KIMI_CODE_HOME/credentials/kimi-code.json`。如果没有登录凭据，让用户先在 Kimi Code 里执行 `/login`。 @@ -98,7 +97,7 @@ A 股 `.SH/.SZ/.BJ`，港股 `.HK`，美股 `.US` 等。用户通常只说中文 ## 4. 怎么读返回结果 -`call_data_source_tool` 和 `query_stock` 的 stdout 一般含两段： +`call_data_source_tool` 的 stdout 一般含两段： 1. **`data_preview`**：CSV 头 + 前几行（通常 1~3 行），方便你直接答简单问题 2. **`CSV 数据已写入：/tmp/xxx.csv`**：完整数据落盘路径 @@ -110,23 +109,9 @@ A 股 `.SH/.SZ/.BJ`，港股 `.HK`，美股 `.US` 等。用户通常只说中文 如果接口返回失败，提示文字一般会写明原因（参数不对 / 不支持 / 数据空等）。把人话原因反馈给用户，不要硬走第二次。 -## 5. `query_stock` — 实时行情快捷命令 +## 5. `watchlist.json` — 用户自选股 -对**实时**类的常见股票查询，可以**不走** `get_data_source_desc → call_data_source_tool` 这套流程，直接用 `query_stock`，省一次调用。它等价于 `stock_finance_data` 的实时接口子集。 - -调用 `mcp__plugin-kimi-datasource-data__query_stock`，参数形如 `{"ticker":"600519.SH","type":"realtime_price","file_path":"/tmp/stock_600519.csv"}`。 - -适用场景（且仅限）： -- 看当前价 / 当日分钟 K 线（`type=realtime_price`） -- 看实时技术指标 MA/MACD/KDJ/RSI/BOLL 等（`type=realtime_tech`，**仅 A 股**，港股/美股会报错） -- 开盘摘要（`type=open_summary`） -- 收盘摘要（`type=close_summary`，**港股盘后看当天数据必须用这个**，`realtime_price` 拿不到） - -涉及**历史日 K / 财报 / 公告 / 股东 / 财务指标 / 选股 / 业务分部 / 预测**等任何"非实时"的股票查询，走标准 `get_data_source_desc → call_data_source_tool` 流程，不要在 `query_stock` 上硬凑。 - -## 6. `watchlist.json` — 用户自选股 - -`${KIMI_SKILL_DIR}/watchlist.json` 是用户的自选股列表。用户问"看一下我的自选股"时，读这个文件然后按每 3 只一组分批执行 `query_stock`。 +`${KIMI_SKILL_DIR}/watchlist.json` 是用户的自选股列表。用户问"看一下我的自选股"时，读这个文件，再走标准 `get_data_source_desc("stock_finance_data") → call_data_source_tool` 流程查实时行情；文档里的实时接口最多 3 个 ticker 一批，多了分批调。 格式： @@ -141,10 +126,9 @@ A 股 `.SH/.SZ/.BJ`，港股 `.HK`，美股 `.US` 等。用户通常只说中文 - 两者都有时顺便算盈亏：`(当前价 - hold_cost) * hold_quantity` - 用户说"帮我加 XX 到自选股"时：先 web_search 核对代码，再追加到 JSON 数组 -## 7. 注意事项 +## 6. 注意事项 - **不要凭记忆猜股票代码 / 企业全称**。错代码会让接口静默返回错数据，用户察觉不到 - **不要在没读 desc 的情况下硬传 `api_name`**。后端会报 `API_NOT_FOUND`。除非这次会话里你已经读过该数据源的 desc 并记得参数 - **不要给投资建议**。给完数据加一句"AI 生成，不构成投资建议"即可 -- **不要在 `query_stock` 上塞历史/财报查询**。它只覆盖实时类，别的会失败 - 如果某个数据源接口返回的报错明显是后端 bug（参数 schema 自相矛盾、内部 Python 报错等），**汇报错误给用户，不要硬试**——这种 bug 我们这边修不了，要后端服务侧改 diff --git a/plugins/official/kimi-datasource/bin/kimi-datasource.mjs b/plugins/official/kimi-datasource/bin/kimi-datasource.mjs index df3c293c8..e67f2e498 100755 --- a/plugins/official/kimi-datasource/bin/kimi-datasource.mjs +++ b/plugins/official/kimi-datasource/bin/kimi-datasource.mjs @@ -18,44 +18,33 @@ import { arch, homedir, hostname, release, type } from 'node:os'; import path from 'node:path'; import readline from 'node:readline'; -const VERSION = '3.0.0'; +const VERSION = '3.1.0'; const API_URL = process.env.KIMI_DATASOURCE_API_URL ?? 'https://api.kimi.com/coding/v1/tools'; const REQUEST_TIMEOUT_MS = 30_000; const PROTOCOL_VERSION = '2025-06-18'; -const VALID_STOCK_QUERY_TYPES = new Set([ - 'realtime_price', - 'realtime_tech', - 'open_summary', - 'close_summary', -]); const TOOLS = [ { - name: 'query_stock', + name: 'call_data_source_tool', description: - 'Query realtime stock price, realtime technical indicators, open summaries, or close summaries for up to 3 tickers.', + "Dispatch a call to any registered data source's API via the Kimi Code gateway. Always call get_data_source_desc(name) first to learn that source's available APIs and required params, then construct this call with api_name and params taken from that description.", inputSchema: { type: 'object', properties: { - ticker: { + data_source_name: { type: 'string', - description: 'Ticker code list separated by commas, for example 600519.SH or 0700.HK.', + description: 'Data source name returned or documented by get_data_source_desc.', }, - type: { + api_name: { type: 'string', - enum: ['realtime_price', 'realtime_tech', 'open_summary', 'close_summary'], - description: 'Realtime stock query type.', + description: 'API name from the data source description.', }, - time: { - type: 'string', - description: 'Optional time parameter for supported realtime endpoints.', - }, - file_path: { - type: 'string', - description: 'Optional CSV output path. When omitted, the tool chooses a temporary path.', + params: { + type: 'object', + description: 'API parameters that match the data source description.', }, }, - required: ['ticker'], + required: ['data_source_name', 'api_name', 'params'], }, }, { @@ -81,70 +70,9 @@ const TOOLS = [ required: ['name'], }, }, - { - name: 'call_data_source_tool', - description: 'Call one API from a Kimi data source after reading get_data_source_desc.', - inputSchema: { - type: 'object', - properties: { - data_source_name: { - type: 'string', - description: 'Data source name returned or documented by get_data_source_desc.', - }, - api_name: { - type: 'string', - description: 'API name from the data source description.', - }, - params: { - type: 'object', - description: 'API parameters that match the data source description.', - }, - }, - required: ['data_source_name', 'api_name', 'params'], - }, - }, ]; const HANDLERS = { - query_stock: { - method: 'get_stock_realtime_price', - buildParams(args) { - const ticker = requiredString(args, 'ticker'); - const tickerList = ticker - .split(',') - .map((item) => item.trim()) - .filter(Boolean); - if (tickerList.length === 0) throw new Error('Missing required argument: ticker.'); - if (tickerList.length > 3) { - throw new Error('ticker accepts at most 3 values separated by commas.'); - } - - const queryType = optionalString(args, 'type') ?? 'realtime_price'; - if (!VALID_STOCK_QUERY_TYPES.has(queryType)) { - throw new Error( - `type must be one of ${JSON.stringify([...VALID_STOCK_QUERY_TYPES])}; received: ${queryType}`, - ); - } - - const params = { - ticker, - type: queryType, - file_path: optionalString(args, 'file_path') ?? defaultStockFilePath(ticker, queryType), - }; - const time = optionalString(args, 'time'); - if (time !== undefined) params.time = time; - return params; - }, - format(text, params) { - return `${text}\n\nCSV data written to: ${params.file_path}`; - }, - }, - get_data_source_desc: { - method: 'get_data_source_desc', - buildParams(args) { - return { name: requiredString(args, 'name') }; - }, - }, call_data_source_tool: { method: 'call_data_source_tool', buildParams(args) { @@ -155,6 +83,12 @@ const HANDLERS = { }; }, }, + get_data_source_desc: { + method: 'get_data_source_desc', + buildParams(args) { + return { name: requiredString(args, 'name') }; + }, + }, }; async function handleRequest(message) { @@ -412,11 +346,6 @@ function extractChannelText(value) { return undefined; } -function defaultStockFilePath(ticker, queryType) { - const safeTicker = ticker.replaceAll(',', '_').replaceAll('.', '_'); - return `/tmp/stock_${safeTicker}_${queryType}.csv`; -} - function requiredString(args, field) { const value = optionalString(args, field); if (value === undefined) throw new Error(`Missing required argument: ${field}.`); diff --git a/plugins/official/kimi-datasource/kimi.plugin.json b/plugins/official/kimi-datasource/kimi.plugin.json index cfbfce121..9fd1274a2 100644 --- a/plugins/official/kimi-datasource/kimi.plugin.json +++ b/plugins/official/kimi-datasource/kimi.plugin.json @@ -1,6 +1,6 @@ { "name": "kimi-datasource", - "version": "3.0.0", + "version": "3.1.0", "description": "Finance, macro, enterprise, and academic data tools for Kimi Code.", "keywords": ["finance", "data-source", "mcp"], "mcpServers": { From 96bbc471c4aca9526e4dcfe00e6bad2b653bbe66 Mon Sep 17 00:00:00 2001 From: liruifengv Date: Fri, 29 May 2026 19:55:10 +0800 Subject: [PATCH 8/8] feat: add experimental feature-flag system (#205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a central, env-driven flag registry in agent-core. Each flag is declared once with an id, full env var name, default, and surface. Within agent-core, flags are consulted through a process-global 'flags' constant that reads live process.env. Resolution precedence: master switch KIMI_CODE_EXPERIMENTAL_FLAG > per-feature KIMI_CODE_EXPERIMENTAL_ > registry default, with lenient boolean parsing via parseBooleanEnv. FlagId is a literal union derived from the registry for compile-time autocomplete and typo-checking. SDK boundary: KimiHarness.getExperimentalFlags() returns the resolved values over RPC, and the SDK re-exports only the flag *types* — no runtime value crosses the boundary. The TUI caches that snapshot once at startup and reads it synchronously for command gating. Gate the plugin system behind the 'plugins' flag, off by default: PluginManager.load() consults flags.enabled('plugins'), so when off no installed plugins are loaded or activated, and the TUI /plugins command is hidden from the palette and resolves as an unknown command. Tests cover the resolver precedence matrix, registry invariants, the FlagId type guard, the live-env singleton, the plugin-load gate, the getExperimentalFlags RPC, and the TUI command gating. --- .changeset/experimental-flags.md | 5 + AGENTS.md | 4 + .../src/tui/commands/experimental-flags.ts | 15 +++ apps/kimi-code/src/tui/commands/index.ts | 1 + apps/kimi-code/src/tui/commands/resolve.ts | 13 ++- apps/kimi-code/src/tui/commands/types.ts | 3 + apps/kimi-code/src/tui/kimi-tui.ts | 8 +- .../test/tui/commands/resolve.test.ts | 1 + .../test/tui/kimi-tui-message-flow.test.ts | 1 + .../test/tui/kimi-tui-startup.test.ts | 1 + packages/agent-core/src/flags/index.ts | 3 + packages/agent-core/src/flags/registry.ts | 16 +++ packages/agent-core/src/flags/resolver.ts | 65 +++++++++++ packages/agent-core/src/flags/types.ts | 19 +++ packages/agent-core/src/index.ts | 1 + packages/agent-core/src/rpc/core-api.ts | 2 + packages/agent-core/src/rpc/core-impl.ts | 12 ++ .../agent-core/test/flags/resolver.test.ts | 108 ++++++++++++++++++ packages/node-sdk/src/index.ts | 10 ++ packages/node-sdk/src/kimi-harness.ts | 6 + packages/node-sdk/src/rpc.ts | 6 + 21 files changed, 297 insertions(+), 3 deletions(-) create mode 100644 .changeset/experimental-flags.md create mode 100644 apps/kimi-code/src/tui/commands/experimental-flags.ts create mode 100644 packages/agent-core/src/flags/index.ts create mode 100644 packages/agent-core/src/flags/registry.ts create mode 100644 packages/agent-core/src/flags/resolver.ts create mode 100644 packages/agent-core/src/flags/types.ts create mode 100644 packages/agent-core/test/flags/resolver.test.ts diff --git a/.changeset/experimental-flags.md b/.changeset/experimental-flags.md new file mode 100644 index 000000000..96de96dc6 --- /dev/null +++ b/.changeset/experimental-flags.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/agent-core": minor +--- + +Add an experimental feature-flag system: a central registry (`flags/registry.ts`) plus an env-driven resolver. Gate a feature with `flags.enabled('id')`, toggled via `KIMI_CODE_EXPERIMENTAL_` or the `KIMI_CODE_EXPERIMENTAL_FLAG` master switch. No flags are defined yet. diff --git a/AGENTS.md b/AGENTS.md index 3c344585c..994f95125 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,6 +44,10 @@ This is a TypeScript monorepo built for agent-assisted development. Keep the roo - When a test fails because of a user modification, default to fixing the test first; do not change the implementation to satisfy an old test unless the implementation truly has a bug. - Do not sacrifice code quality for external compatibility unless the user explicitly asks for it. Breaking changes go through changesets and a `major` bump, gated by the rule below. +## Experimental Features + +- Gate a not-yet-public feature behind an experimental flag. Add the flag to the registry at `packages/agent-core/src/flags/registry.ts`, then check it with `flags.enabled('my-feature')`. Flags are env-driven and default off: `KIMI_CODE_EXPERIMENTAL_` toggles one, `KIMI_CODE_EXPERIMENTAL_FLAG` enables all. Release by flipping the entry's `default` to `true`. + ## Where to Update Instructions - Hard rules that affect almost every task: update the root `AGENTS.md`. diff --git a/apps/kimi-code/src/tui/commands/experimental-flags.ts b/apps/kimi-code/src/tui/commands/experimental-flags.ts new file mode 100644 index 000000000..e1231742c --- /dev/null +++ b/apps/kimi-code/src/tui/commands/experimental-flags.ts @@ -0,0 +1,15 @@ +import type { ExperimentalFlagMap } from '@moonshot-ai/kimi-code-sdk'; + +// Resolved experimental flags, fetched once from the core over RPC at startup and then read +// synchronously by the command palette and dispatch. App-local cache, not a source of truth. +let snapshot: ExperimentalFlagMap = {}; + +/** Replace the cached flag snapshot. Call once after fetching via `harness.getExperimentalFlags()`. */ +export function setExperimentalFlags(flags: ExperimentalFlagMap): void { + snapshot = flags; +} + +/** An `undefined` flag means "not gated" → always enabled, so callers can pass an optional flag id. */ +export function isExperimentalFlagEnabled(flag: string | undefined): boolean { + return flag === undefined || snapshot[flag] === true; +} diff --git a/apps/kimi-code/src/tui/commands/index.ts b/apps/kimi-code/src/tui/commands/index.ts index 2ee577850..60178b265 100644 --- a/apps/kimi-code/src/tui/commands/index.ts +++ b/apps/kimi-code/src/tui/commands/index.ts @@ -1,3 +1,4 @@ +export * from './experimental-flags'; export * from './parse'; export * from './registry'; export * from './resolve'; diff --git a/apps/kimi-code/src/tui/commands/resolve.ts b/apps/kimi-code/src/tui/commands/resolve.ts index 2d0807bf4..a47f11409 100644 --- a/apps/kimi-code/src/tui/commands/resolve.ts +++ b/apps/kimi-code/src/tui/commands/resolve.ts @@ -4,8 +4,13 @@ import { type BuiltinSlashCommand, type BuiltinSlashCommandName, } from './registry'; +import { isExperimentalFlagEnabled } from './experimental-flags'; import { parseSlashInput } from './parse'; -import type { SlashCommandBusyReason, SlashCommandInvalidReason } from './types'; +import type { + KimiSlashCommand, + SlashCommandBusyReason, + SlashCommandInvalidReason, +} from './types'; export type SlashCommandIntent = | { readonly kind: 'not-command' } @@ -45,7 +50,11 @@ export function resolveSlashCommandInput(options: ResolveSlashCommandInput): Sla if (parsed === null) return { kind: 'not-command' }; const command = findBuiltInSlashCommand(parsed.name); - if (command !== undefined) { + // `command` is a literal union where only some members carry `experimentalFlag`; widen to read it. + if ( + command !== undefined && + isExperimentalFlagEnabled((command as KimiSlashCommand).experimentalFlag) + ) { const busyReason = slashCommandBusyReason(options); if ( busyReason !== undefined && diff --git a/apps/kimi-code/src/tui/commands/types.ts b/apps/kimi-code/src/tui/commands/types.ts index cb784f84d..532a301ea 100644 --- a/apps/kimi-code/src/tui/commands/types.ts +++ b/apps/kimi-code/src/tui/commands/types.ts @@ -1,4 +1,5 @@ import type { SlashCommand } from '@earendil-works/pi-tui'; +import type { FlagId } from '@moonshot-ai/kimi-code-sdk'; export type SlashCommandAvailability = 'always' | 'idle-only'; @@ -8,6 +9,8 @@ export interface KimiSlashCommand extends SlashCom readonly description: string; readonly priority?: number; readonly availability?: SlashCommandAvailability | ((args: string) => SlashCommandAvailability); + /** When set, the command is hidden from the palette and blocked unless this flag is enabled. */ + readonly experimentalFlag?: FlagId; } export interface ParsedSlashInput { diff --git a/apps/kimi-code/src/tui/kimi-tui.ts b/apps/kimi-code/src/tui/kimi-tui.ts index 79622dfd5..db5bf7b8a 100644 --- a/apps/kimi-code/src/tui/kimi-tui.ts +++ b/apps/kimi-code/src/tui/kimi-tui.ts @@ -34,6 +34,8 @@ import { detectFdPath } from '#/utils/process/fd-detect'; import { BUILTIN_SLASH_COMMANDS, buildSkillSlashCommands, + isExperimentalFlagEnabled, + setExperimentalFlags, sortSlashCommands, type KimiSlashCommand, type SkillListSession, @@ -287,7 +289,10 @@ export class KimiTUI { // ========================================================================= private getSlashCommands(): readonly KimiSlashCommand[] { - return [...sortSlashCommands(BUILTIN_SLASH_COMMANDS), ...this.skillCommands]; + const builtins = sortSlashCommands(BUILTIN_SLASH_COMMANDS).filter((command) => + isExperimentalFlagEnabled(command.experimentalFlag), + ); + return [...builtins, ...this.skillCommands]; } private setupAutocomplete(): void { @@ -380,6 +385,7 @@ export class KimiTUI { // Mount only after init() succeeds; see mountFooter(). this.mountFooter(); this.renderWelcome(); + setExperimentalFlags(await this.harness.getExperimentalFlags()); this.setupAutocomplete(); void this.loadPersistedInputHistory(); this.state.editorContainer.clear(); diff --git a/apps/kimi-code/test/tui/commands/resolve.test.ts b/apps/kimi-code/test/tui/commands/resolve.test.ts index d151bad80..07381c0bd 100644 --- a/apps/kimi-code/test/tui/commands/resolve.test.ts +++ b/apps/kimi-code/test/tui/commands/resolve.test.ts @@ -131,6 +131,7 @@ describe('resolveSlashCommandInput', () => { input: '/does-not-exist arg', }); }); + }); describe('slash command busy helpers', () => { diff --git a/apps/kimi-code/test/tui/kimi-tui-message-flow.test.ts b/apps/kimi-code/test/tui/kimi-tui-message-flow.test.ts index 40ab4e651..ce198f283 100644 --- a/apps/kimi-code/test/tui/kimi-tui-message-flow.test.ts +++ b/apps/kimi-code/test/tui/kimi-tui-message-flow.test.ts @@ -200,6 +200,7 @@ function makeHarness(session = makeSession(), overrides: Record track: vi.fn(), setTelemetryContext: vi.fn(), interactiveAgentId: 'main', + getExperimentalFlags: vi.fn(async () => ({})), auth: { status: vi.fn(), login: vi.fn(), diff --git a/apps/kimi-code/test/tui/kimi-tui-startup.test.ts b/apps/kimi-code/test/tui/kimi-tui-startup.test.ts index 570f7efe4..de33c6abd 100644 --- a/apps/kimi-code/test/tui/kimi-tui-startup.test.ts +++ b/apps/kimi-code/test/tui/kimi-tui-startup.test.ts @@ -132,6 +132,7 @@ function makeHarness(session = makeSession(), overrides: Record close: vi.fn(async () => {}), track: vi.fn(), setTelemetryContext: vi.fn(), + getExperimentalFlags: vi.fn(async () => ({})), auth: { status: vi.fn(async () => ({ providers: [] })), login: vi.fn(async () => {}), diff --git a/packages/agent-core/src/flags/index.ts b/packages/agent-core/src/flags/index.ts new file mode 100644 index 000000000..e37a8178e --- /dev/null +++ b/packages/agent-core/src/flags/index.ts @@ -0,0 +1,3 @@ +export * from './types'; +export * from './registry'; +export * from './resolver'; diff --git a/packages/agent-core/src/flags/registry.ts b/packages/agent-core/src/flags/registry.ts new file mode 100644 index 000000000..1e9f57b87 --- /dev/null +++ b/packages/agent-core/src/flags/registry.ts @@ -0,0 +1,16 @@ +import type { FlagDefinitionInput } from './types'; + +/** + * Experimental feature flags. Empty by default — there are no experimental features yet. + * + * To add one, append an entry and gate the feature with `flags.enabled('my-feature')`: + * { id: 'my-feature', env: 'KIMI_CODE_EXPERIMENTAL_MY_FEATURE', default: false, surface: 'both' } + * + * Keep the `as const satisfies` — it derives the literal `FlagId` union that gives `enabled()` + * autocomplete and typo-checking. `env` must start with 'KIMI_CODE_EXPERIMENTAL_', be unique, and + * not equal the master switch 'KIMI_CODE_EXPERIMENTAL_FLAG'; `id` must not be 'flag'. + */ +export const FLAG_DEFINITIONS = [] as const satisfies readonly FlagDefinitionInput[]; + +/** Literal union of registered flag ids (currently none → `never`). */ +export type FlagId = (typeof FLAG_DEFINITIONS)[number]['id']; diff --git a/packages/agent-core/src/flags/resolver.ts b/packages/agent-core/src/flags/resolver.ts new file mode 100644 index 000000000..7685cd52f --- /dev/null +++ b/packages/agent-core/src/flags/resolver.ts @@ -0,0 +1,65 @@ +import { parseBooleanEnv } from '#/config/resolve'; + +import { FLAG_DEFINITIONS, type FlagId } from './registry'; +import type { FlagDefinitionInput } from './types'; + +/** Master switch: when truthy, forces every flag on (highest priority). */ +export const MASTER_ENV = 'KIMI_CODE_EXPERIMENTAL_FLAG'; + +/** Shared prefix for per-feature variables. */ +export const EXPERIMENTAL_PREFIX = 'KIMI_CODE_EXPERIMENTAL_'; + +/** + * Conventional env-name generator: flag id → recommended env variable name. Only used when + * authoring a new flag (to fill `env`) and for an optional consistency test; NOT used during + * resolution. + */ +export function flagEnvKey(id: string): string { + return `${EXPERIMENTAL_PREFIX}${id.toUpperCase().replaceAll('-', '_')}`; +} + +/** + * Pure, synchronous flag resolver. State comes entirely from (env, registry) and nothing is + * cached: env is read live on every call, so a single shared instance always reflects the current + * process env. Defaults to process.env + FLAG_DEFINITIONS; tests can inject a custom env / defs. + * + * Precedence (highest wins): + * L1 master switch KIMI_CODE_EXPERIMENTAL_FLAG → every flag is on + * L2 per-feature def.env (parseBooleanEnv, may force on or off) + * L3 registry default + */ +export class FlagResolver { + private readonly env: Readonly>; + private readonly byId: ReadonlyMap; + + constructor( + env: Readonly> = process.env, + definitions: readonly FlagDefinitionInput[] = FLAG_DEFINITIONS, + ) { + this.env = env; + this.byId = new Map(definitions.map((def) => [def.id, def])); + } + + enabled(id: FlagId): boolean { + const def = this.byId.get(id); + if (def === undefined) return false; + if (parseBooleanEnv(this.env[MASTER_ENV]) === true) return true; // L1 master switch + const override = parseBooleanEnv(this.env[def.env]); // L2 per-feature + if (override !== undefined) return override; + return def.default; // L3 default + } +} + +export function createFlagResolver( + env?: Readonly>, + definitions?: readonly FlagDefinitionInput[], +): FlagResolver { + return new FlagResolver(env, definitions); +} + +/** + * Process-global flag accessor. Flags are env-driven and process-global, so a single shared + * instance (reading live process.env) is the canonical way to consult them — import this directly + * rather than constructing or injecting a resolver. + */ +export const flags = new FlagResolver(); diff --git a/packages/agent-core/src/flags/types.ts b/packages/agent-core/src/flags/types.ts new file mode 100644 index 000000000..3c6668036 --- /dev/null +++ b/packages/agent-core/src/flags/types.ts @@ -0,0 +1,19 @@ +import type { FlagId } from './registry'; + +/** Which layer consumes a flag — documentation/grouping only; not used in resolution. */ +export type FlagSurface = 'core' | 'tui' | 'both'; + +/** Shape of a registry entry (id is a loose string so `as const satisfies` can validate it). */ +export interface FlagDefinitionInput { + readonly id: string; + /** Full environment variable name, e.g. `KIMI_CODE_EXPERIMENTAL_MY_FEATURE`. Read directly by the resolver. */ + readonly env: string; + readonly default: boolean; + readonly surface: FlagSurface; +} + +/** FlagId-typed view so consumers can fetch a definition by its literal id. */ +export type FlagDefinition = FlagDefinitionInput & { readonly id: FlagId }; + +/** Resolved enabled-state of every experimental flag (flag id → enabled); used for the SDK snapshot. */ +export type ExperimentalFlagMap = Record; diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index 670def093..c874d2ca2 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -2,6 +2,7 @@ export * from './agent'; export * from './session'; export * from './rpc'; export * from './config'; +export * from './flags'; export * from './session/export'; export * from './telemetry'; export * from './errors'; diff --git a/packages/agent-core/src/rpc/core-api.ts b/packages/agent-core/src/rpc/core-api.ts index 99e244d45..504e9a309 100644 --- a/packages/agent-core/src/rpc/core-api.ts +++ b/packages/agent-core/src/rpc/core-api.ts @@ -4,6 +4,7 @@ import type { PermissionData, PermissionMode } from '#/agent/permission'; import type { PlanData } from '#/agent/plan'; import type { ToolInfo } from '#/agent/tool'; import type { KimiConfig, KimiConfigPatch } from '#/config'; +import type { ExperimentalFlagMap } from '#/flags'; import type { ResumeSessionResult } from '#/rpc/resumed'; import type { SessionMeta } from '#/session'; import type { BackgroundTaskInfo } from '#/tools/builtin'; @@ -307,6 +308,7 @@ type SessionAPIWithId = WithSessionId; export interface CoreAPI extends SessionAPIWithId { getCoreInfo: (payload: EmptyPayload) => CoreInfo; + getExperimentalFlags: (payload: EmptyPayload) => ExperimentalFlagMap; getKimiConfig: (payload: GetKimiConfigPayload) => KimiConfig; setKimiConfig: (payload: SetKimiConfigPayload) => KimiConfig; removeKimiProvider: (payload: RemoveKimiProviderPayload) => KimiConfig; diff --git a/packages/agent-core/src/rpc/core-impl.ts b/packages/agent-core/src/rpc/core-impl.ts index a20899e34..7fa4a6bd3 100644 --- a/packages/agent-core/src/rpc/core-impl.ts +++ b/packages/agent-core/src/rpc/core-impl.ts @@ -20,6 +20,13 @@ import { type KimiConfig, type MoonshotServiceConfig, } from '../config'; +import { + FLAG_DEFINITIONS, + flags, + type ExperimentalFlagMap, + type FlagDefinitionInput, + type FlagId, +} from '../flags'; import type { Logger } from '../logging/types'; import { resolveSessionMcpConfig, type SessionMcpConfig } from '../mcp'; import { Session, type SessionMeta, type SessionSkillConfig } from '../session'; @@ -242,6 +249,11 @@ export class KimiCore implements PromisableMethods { return { version: getCoreVersion() }; } + getExperimentalFlags(): ExperimentalFlagMap { + const defs: readonly FlagDefinitionInput[] = FLAG_DEFINITIONS; + return Object.fromEntries(defs.map((def) => [def.id, flags.enabled(def.id as FlagId)])); + } + async closeSession({ sessionId }: CloseSessionPayload): Promise { const session = this.sessions.get(sessionId); if (session) { diff --git a/packages/agent-core/test/flags/resolver.test.ts b/packages/agent-core/test/flags/resolver.test.ts new file mode 100644 index 000000000..15c300483 --- /dev/null +++ b/packages/agent-core/test/flags/resolver.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from 'vitest'; + +import { + EXPERIMENTAL_PREFIX, + FLAG_DEFINITIONS, + MASTER_ENV, + createFlagResolver, + flagEnvKey, + type FlagDefinitionInput, + type FlagId, +} from '../../src/flags'; + +// Controlled fake definitions to assert the precedence matrix precisely (independent of the +// real registry contents). +const DEFS = [ + { + id: 'a-on-default', + env: 'KIMI_CODE_EXPERIMENTAL_A', + default: true, + surface: 'core', + }, + { + id: 'b-off-default', + env: 'KIMI_CODE_EXPERIMENTAL_B', + default: false, + surface: 'tui', + }, +] as const satisfies readonly FlagDefinitionInput[]; + +type Env = Record; + +function make(env: Env) { + const resolver = createFlagResolver(env, DEFS); + // The fake ids are not part of the real FlagId union, so cast to FlagId when calling. + return (id: string) => resolver.enabled(id as FlagId); +} + +describe('FlagResolver', () => { + it('L3 default: returns the registry default when env is empty', () => { + const enabled = make({}); + expect(enabled('a-on-default')).toBe(true); + expect(enabled('b-off-default')).toBe(false); + }); + + it('L2 per-feature on (lenient truthy values)', () => { + for (const v of ['1', 'true', 'yes', 'on', 'TRUE', ' On ']) { + expect(make({ KIMI_CODE_EXPERIMENTAL_B: v })('b-off-default')).toBe(true); + } + }); + + it('L2 per-feature off (lenient falsy values) overrides default=true', () => { + for (const v of ['0', 'false', 'no', 'off']) { + expect(make({ KIMI_CODE_EXPERIMENTAL_A: v })('a-on-default')).toBe(false); + } + }); + + it('L2 unparseable value falls back to default', () => { + expect(make({ KIMI_CODE_EXPERIMENTAL_B: 'maybe' })('b-off-default')).toBe(false); + expect(make({ KIMI_CODE_EXPERIMENTAL_A: 'maybe' })('a-on-default')).toBe(true); + }); + + it('L1 master switch: every flag is on when enabled (including default=false)', () => { + const enabled = make({ [MASTER_ENV]: '1' }); + expect(enabled('a-on-default')).toBe(true); + expect(enabled('b-off-default')).toBe(true); + }); + + it('L1 master switch beats an L2 per-feature off (D2)', () => { + const enabled = make({ [MASTER_ENV]: '1', KIMI_CODE_EXPERIMENTAL_A: '0' }); + expect(enabled('a-on-default')).toBe(true); + }); + + it('master switch is inactive for lenient falsy values', () => { + const enabled = make({ [MASTER_ENV]: '0' }); + expect(enabled('b-off-default')).toBe(false); + }); + + it('reads the env name declared in the registry (the declared name works, others do not)', () => { + expect(make({ KIMI_CODE_EXPERIMENTAL_B: '1' })('b-off-default')).toBe(true); + // The name mechanically derived from the id must not take effect (env is explicitly ..._B). + expect(make({ KIMI_CODE_EXPERIMENTAL_B_OFF_DEFAULT: '1' })('b-off-default')).toBe(false); + }); + + it('unknown id resolves to false (defensive)', () => { + expect(make({})('not-a-real-flag')).toBe(false); + }); + + it('flagEnvKey convention: kebab -> prefix + upper snake', () => { + expect(flagEnvKey('my-feature')).toBe('KIMI_CODE_EXPERIMENTAL_MY_FEATURE'); + }); +}); + +describe('FLAG_DEFINITIONS invariants', () => { + it('every env satisfies: prefix / unique / not the master switch', () => { + const seenEnv = new Set(); + const seenId = new Set(); + const defs: readonly FlagDefinitionInput[] = FLAG_DEFINITIONS; + for (const def of defs) { + expect(def.env.startsWith(EXPERIMENTAL_PREFIX)).toBe(true); + expect(def.env).not.toBe(MASTER_ENV); + expect(def.id).not.toBe('flag'); // reserved: would collide with the master switch + expect(seenEnv.has(def.env)).toBe(false); + expect(seenId.has(def.id)).toBe(false); + seenEnv.add(def.env); + seenId.add(def.id); + } + }); +}); diff --git a/packages/node-sdk/src/index.ts b/packages/node-sdk/src/index.ts index a3136fb48..ae3d677d2 100644 --- a/packages/node-sdk/src/index.ts +++ b/packages/node-sdk/src/index.ts @@ -44,6 +44,16 @@ export { } from '@moonshot-ai/agent-core'; export type { LogContext, LogLevel, LogPayload, Logger } from '@moonshot-ai/agent-core'; +// Experimental feature flags — types only. Resolved values come from +// `KimiHarness.getExperimentalFlags()` over RPC, not from a re-exported runtime value. +export type { + ExperimentalFlagMap, + FlagDefinition, + FlagDefinitionInput, + FlagId, + FlagSurface, +} from '@moonshot-ai/agent-core'; + export type { KimiAuthLoginResult, KimiAuthLogoutResult, diff --git a/packages/node-sdk/src/kimi-harness.ts b/packages/node-sdk/src/kimi-harness.ts index e9b51831d..769cd3a3d 100644 --- a/packages/node-sdk/src/kimi-harness.ts +++ b/packages/node-sdk/src/kimi-harness.ts @@ -8,6 +8,7 @@ import { resolveKimiHome, resolveLoggingConfig, withTelemetryContext, + type ExperimentalFlagMap, type TelemetryClient, type TelemetryContextPatch, type TelemetryProperties, @@ -191,6 +192,11 @@ export class KimiHarness { return this.rpc.getConfig(options); } + /** Resolved enabled-state of every experimental flag (flag id → enabled). */ + async getExperimentalFlags(): Promise { + return this.rpc.getExperimentalFlags(); + } + async ensureConfigFile(): Promise { await ensureConfigFile(this.configPath); } diff --git a/packages/node-sdk/src/rpc.ts b/packages/node-sdk/src/rpc.ts index a65b3e716..7346e5a5f 100644 --- a/packages/node-sdk/src/rpc.ts +++ b/packages/node-sdk/src/rpc.ts @@ -9,6 +9,7 @@ import { type ApprovalResponse, type CoreAPI, type Event, + type ExperimentalFlagMap, type OAuthTokenProviderResolver, type QuestionRequest, type QuestionResult, @@ -197,6 +198,11 @@ export class SDKRpcClient { return rpc.getKimiConfig(input ?? {}); } + async getExperimentalFlags(): Promise { + const rpc = await this.getRpc(); + return rpc.getExperimentalFlags({}); + } + async setConfig(input: KimiConfigPatch): Promise { const rpc = await this.getRpc(); return rpc.setKimiConfig(input);