feat(tui): run interactive shell on agent-core-v2 with deferred session creation#1543
Open
liruifengv wants to merge 604 commits into
Open
feat(tui): run interactive shell on agent-core-v2 with deferred session creation#1543liruifengv wants to merge 604 commits into
liruifengv wants to merge 604 commits into
Conversation
…into kimi-code-v2
- add `multi_server` experimental flag in agent-core-v2 and wire it into the package barrel - add kap-server instance registry that writes per-server files under server/instances/ with heartbeat and stale-pid sweep - thread PromptOrigin through AgentTurnService.launch and emit it on turn.started - drop the error field from cancelled loop/turn results and use userCancellationReason in promptLegacy
- gate startServer on KIMI_CODE_EXPERIMENTAL_MULTI_SERVER: register under server/instances/ and rely on port+1 retry for coexistence, or take the legacy single-instance lock when off - release/update the chosen handle on close, boot refusal, and after bind so the advertised port stays accurate - export the instance registry API from the package barrel - fix session.meta.updated to dispatch under the real session id instead of '__global__' so auto-titled and renamed sessions update in clients
`resolveLegacy` always resolved the main agent, ignoring `agent_id` in the prompt submission body. A `/btw <question>` submit carries the forked side-channel child's `agent_id`, so it was answered in the main view instead of the right-side BTW panel. Resolve the agent named by `agent_id` (falling back to `main`), and return 40401 for an unknown agent id. Add regression tests for side-channel routing and the unknown-agent case.
…mpty server-v2 binds the main agent via profile:setModel without a cwd, so the profile cwd is empty and planFilePathFor() produced a relative path like plan/<id>.md. The plan-mode guard compares that against the Write tool's workDir-resolved absolute path with strict equality, so it denied every write to the plan file; the relative path also landed in process.cwd() instead of the session workDir, breaking ExitPlanMode. Make the plan path always absolute, falling back to the session workDir (the same root the file tools resolve against) when the profile cwd is empty.
Forward event.session.created to WS clients (it was published on the core bus but never forwarded) and re-emit event.session.status_changed(running) on turn.started. v2 derives session status via ISessionActivity, a pure pull that publishes nothing, so these v1 broadcasts were dropped: clients that did not issue a create never learned the session existed, and the running transition never reached kimi-web, whose Stop button is gated on session.status === 'running'. Mirrors v1's SessionService status emission and isGlobalSessionEvent fan-out.
- add PromptHarness/PromptSession interfaces so the print driver is decoupled from the concrete SDK harness/session - add an in-process agent-core-v2 harness (bootstrap + session + prompt/goal/permission adapters) under cli/v2/ - adapt v2 IEventBus events to the v1 Event union, with unit tests - select the engine in createPromptHarness via isKimiV2Enabled() (KIMI_CODE_EXPERIMENTAL_FLAG), lazy-importing v2 off the default path
- remove unused imports and variables - add void to floating promises - drop redundant return-await - bind unbound method references - replace == null with strict null/undefined checks - add explicit sort comparators and Array.from helpers - rename background_tasks capability to tasks - migrate event sink to event bus in tests
- add a per-connection outbound send buffer in WsConnectionV1; sendFrame enqueues and a flush drains on a 16ms interval or once 64 frames queue - coalesce adjacent volatile assistant/thinking deltas for the same session/agent/turn into one frame, keeping the first offset/seq so client alignment is unchanged - defer flushing while socket.bufferedAmount exceeds a 1MiB high-water mark, merging new deltas into the queued frame instead of growing it - flush remaining frames on close to avoid truncating the tail of a stream - expose flushIntervalMs / maxBatchSize / highWaterMarkBytes via registerWsV1
- bridge split v2 status slices into one combined `agent.status.updated` event via a LegacyStatus derived model at the kap-server edge, so a usage-only event no longer overwrites the live context window with a stale zero - fall back to the configured default model's context window when the agent has no model bound yet, so fresh sessions don't show "0/0" - use `size` (measured + estimated) for the live context token count to mirror v1's `context.tokenCount` - export `defineDerivedModel` / `DerivedModelDef` from agent-core-v2
- add custom onSend/onResponse access logging that records the envelope code - disable Fastify built-in request logging (every response is HTTP 200) - drop pino-pretty; logger always emits newline-delimited JSON - rename dev:server-v2 script to dev:kap-server
The v2 server package is @moonshot-ai/kap-server, but three changesets still referenced the old @moonshot-ai/server-v2 name, which no longer exists in the workspace.
Subagents created via the Agent tool or AgentSwarm fell back to the default `manual` permission mode because CreateAgentOptions offered no way to seed one, so write/execute tool calls inside a subagent prompted for approval even when the parent ran in auto/yolo. Add an optional permissionMode to CreateAgentOptions and have the Agent tool and AgentSwarm pass the caller's current mode when creating a child.
- add reduceContextTranscript in agent-core-v2: keeps the full history across context.apply_compaction (summary marker instead of dropping the prefix) and stops context.undo at compaction summaries, matching v1's transcript view - SnapshotReader: reduce the wire with the transcript reducer instead of the folded live context, so a later undo no longer hides the pre-compaction assistant reply - MessageLegacyService: read the main agent wire log and merge the unflushed live tail instead of the folded IAgentContextMemoryService.get()
- v1 session route handlers and the sessionLegacy adapter resolved the target via ISessionLifecycleService.get, which only sees live (in-memory) sessions. A freshly-opened session is persisted (index + disk) but not live until the first prompt, so commands such as undo / fs:* / skills / :btw / warnings / terminals / profile / questions / approvals / archive reported `session does not exist` (40401). - Resolve via ISessionLifecycleService.resume instead, which cold-loads a persisted session from disk (matches v1's resumeSession); a genuinely missing session still 404s. - Add a cold-load regression for `:undo` and update the skills test for the new cold-load behaviour.
…into kimi-code-v2
…into kimi-code-v2
- add an Agent-scope runtime domain that holds the whole live phase as one discriminated-union field (idle, running, streaming, tool_call, retrying, awaiting_approval, interrupted, ended) - drive it from existing turn, step, delta, tool, retry, interrupt and approval events, edge-triggered with reference-equality dedup so delta bursts do not flood the wire log - carry the phase on the existing agent.status.updated channel and add the AgentPhase type/schema to the protocol (backward compatible)
…into kimi-code-v2
- register `defaultPermissionMode` and `defaultPlanMode` config sections - apply `defaultPermissionMode` when creating the main agent - enter plan mode on fresh sessions when `defaultPlanMode` is true - fold `yolo: true` into `default_permission_mode` on kap-server config write, derive `yolo` on read (yolo stays wire sugar, never a persisted domain)
Align with v1: the model can pass `background: true` to get a task_id immediately while the question waits in the background for the user's answer; completion is delivered to the agent automatically through the task service's terminal notification. - New QuestionBackgroundTask (AgentTask kind 'question') that runs the question request on a detached task and settles completed/failed/killed. - AskUserQuestionTool gains the optional `background` schema field, the description suffix, an IAgentTaskService dependency, and a background execution branch whose output block matches v1 verbatim. - Tests: harness injects a task-service stub, two legacy 'no background' assertions are flipped to the v1-aligned behavior, and three background cases (immediate task_id, settle completed, abort killed) are added.
Align with v1: when KIMI_MODEL_NAME is set without KIMI_MODEL_BASE_URL, the reserved __kimi_env__ provider now gets a per-type default baseUrl (kimi -> api.moonshot.ai/v1, openai -> api.openai.com/v1; anthropic is left unset so the SDK picks its default). Previously v2 left baseUrl empty and later threw "missing a base URL" for the openai env-model path, regressing v1's out-of-the-box behavior. An explicit KIMI_MODEL_BASE_URL still wins.
… no-goal fallbacks Align with v1: completing or blocking a goal now returns the dynamic summary/blocked-reason prompt (buildGoalCompletionSummaryPrompt / buildGoalBlockedReasonPrompt, already present in outcome-prompts.ts but unused) instead of the static "Goal marked complete/blocked." text, and all three statuses report the "no active/current goal" fallback when there is nothing to transition.
- add media-owned `image` config section (`max_edge_px`, `read_byte_budget`) with `KIMI_IMAGE_MAX_EDGE_PX` / `KIMI_IMAGE_READ_BYTE_BUDGET` env bindings (env > config.toml > default) - add Agent-scope `ImageConfigBridge` that pushes the env-resolved section into the image-compress resolver seam on load and on change, so all call sites honor config without per-call wiring - resolve `maxEdge` / read-byte-budget defaults in image-compress via the new seam; keep the support module config-agnostic - apply the read-image byte budget in ReadMediaFile's default downscale path (previously fell back to the 3.75 MB provider ceiling)
Reconcile the parallel goal-mode work on goal-parity with origin's newer kimi-code-v2 architecture. Conflicts resolved by keeping origin's mechanisms and layering parity's behavior on top: - goalService: keep origin's activity-lease continuation launch; graft parity's settleGoalAfterContinuationFailure so a failed relaunch pauses the goal instead of stranding it. Drop the orphaned tokenUsageTotal helper (accounting already charges ctx.usage.output) and its unused TokenUsage import; remove a duplicate MAX_GOAL_COMPLETION_CRITERION_LENGTH. - update-goal: layer parity's outcome-prompt tool results and no-goal guards onto origin's goalIsActive stopBatch refinement. - promptService / fullCompactionService: dedupe imports; keep both DI params (toolSelect and instantiation are each used). - tests: align continuation tests to the lease model (hold the turn lane and mock launchWithLease), swap abortController for signal, and keep parity's token-accounting assertions to match the merged code.
Port v1 #1508 into v2: lower the longest-edge downscale cap back to 2000px (v2 was stuck on the 3000px it had ported from an earlier v1 change) and make it overridable, add the 256 KB read-image byte budget used by ReadMediaFile, and widen the over-budget fallback ladder to [2000, 1000, 768, 512, 384, 256]. - MAX_IMAGE_EDGE_PX 3000 -> 2000, with KIMI_IMAGE_MAX_EDGE_PX env and a config-pushed value resolved via resolveMaxImageEdgePx. - READ_IMAGE_BYTE_BUDGET=256KB with KIMI_IMAGE_READ_BYTE_BUDGET env and resolveReadImageByteBudget; ReadMediaFile's default compress path now uses it (region / full_resolution still honor IMAGE_BYTE_BUDGET). - Test expectations that hard-coded 3000px / 1500px updated to 2000 / 1000 to match the v1 behavior. The [image] config.toml section is intentionally not added: the env vars already cover the override path, and wiring a config section plus a runtime push would add v2-specific scaffolding beyond v1.
…aFile Align with v1: a HEIC/HEIF read is now refused up front with an os-specific conversion command (sips / heif-convert / ImageMagick) so the unsupported format never reaches the provider (which would reject the whole session once it lands in history). The two guidance builders are ported verbatim from v1, and the check sits after the image-capability guard (using IHostEnvironment.osKind). Also give the existing EXIF-rotation test a longer timeout: it does heavy jimp encode/decode and was flaking around the default 5s boundary.
Align with v1: expose `startBtw` on AgentAPI and delegate it to the existing ISessionBtwService (already implemented and DI-registered as SessionBtwService), so the /btw slash command can fork a side-question child agent once the server runs on v2.
- Remove resume-debug.test.ts: a one-off diagnosis script with a hardcoded local path, not a real test. - Move streamTiming.test.ts to app/model/modelImpl.test.ts: it only exercises buildStreamTiming in app/model/modelImpl.ts. - Rename wire/store.test.ts to wire/wireServiceImpl.test.ts to match the module it covers (there is no store.ts in src).
Align with v1: replace the hand-rolled subset validator with v1's Ajv- based implementation (draft-07/2019/2020 + ajv-formats), so tool-call argument validation once again honors the JSON Schema `format` keyword (and the full keyword set), not just the previously hard-coded subset. - args-validator.ts is now byte-identical to v1 (93 lines, replacing the 289-line hand-rolled subset). - Adds ajv@^8.18.0 and ajv-formats@^3.0.1 (same versions as v1) plus the pnpm-lock.yaml update. - The two call sites (compileToolArgsValidator -> validateToolArgs) keep working unchanged; a small test locks in format / required / additionalProperties / subset behavior.
Align with v1: replace the hand-rolled subset validator with v1's Ajv- based implementation (draft-07/2019/2020 + ajv-formats), so tool-call argument validation once again honors the JSON Schema `format` keyword (and the full keyword set), not just the previously hard-coded subset. - args-validator.ts is now byte-identical to v1 (93 lines, replacing the 289-line hand-rolled subset). - Adds ajv@^8.18.0 and ajv-formats@^3.0.1 (same versions as v1) plus the pnpm-lock.yaml update. - The two call sites (compileToolArgsValidator -> validateToolArgs) keep working unchanged; a small test locks in format / required / additionalProperties / subset behavior.
…ist tool source Match the prompt change in 5cc8e52: the CronDelete parameter description and invalid-id error now say "ULID" only (not "ULID or legacy 8-hex"), and the CronList id doc comment likewise. The validation regex is unchanged so loading any legacy 8-hex tasks from disk still works.
…into kimi-code-v2
Pull the 8 new upstream commits on top of the resolved goal-parity merge. Reconciled where upstream now overlaps the parity work: - update-goal: take origin's reimplementation (d9e411f) — no-goal fallbacks return a plain result, not a tool error. Dropped parity's contradicting "fail as a tool error" test; origin's goal-tools.test.ts covers the non-error behavior. - promptService: keep parity's steer-buffer semantics (gap G24) — steers survive a cancelled/failed turn and flush into the next turn. Did not re-adopt origin's turn-result observation that discards them; kept parity's compaction-defer feature. Updated the retained test to the new LoopRunResult shape. - goalService: keep the imports still used by the surviving goal-outcome code; drop TurnResult (no longer referenced after the upstream merge). - goal.test.ts: adopt the reorganized test path (test/agent/goal) and its relative imports; keep the activity-lease import used by the continuation tests.
Fast catch-up merge; no conflicts. Brings in tool-args JSON Schema format validation (adds ajv/ajv-formats deps), cron tool-source cleanup, and test file tidy. No overlap with the goal-parity work.
It round-tripped restore over a hardcoded local dataset (kimi-code-mini-bench/.vitest-results) that is not in the repo, so it vacuously passed everywhere else. Removed at the original author's request.
…test The fuzz-style invariant test now drives the full v1 fallback ladder ([2000, 1000, 768, 512, 384, 256]) for over-budget inputs, which takes longer than the default 5s boundary in this environment. Give it 30s.
- agent/task/manager.test.ts -> taskManager.test.ts (no manager.ts in agent/task; disambiguate from taskService.test.ts). - app/cron/persist.test.ts -> cronTaskPersistenceService.test.ts (mirrors the module it covers; agent/task/persist.test.ts mirrors agent/task/persist.ts, so it is left as-is). - agent/contextMemory/message.test.ts -> message-history.test.ts (its describe is 'message history (IAgentContextMemoryService)'; the dir already names the domain).
…into tui-v2 # Conflicts: # packages/agent-core-v2/src/index.ts
Fresh TUI startup no longer creates an empty session. The initial render reads config-level defaults (model, context window, permission/plan mode, thinking effort) from the new App-scope-only CoreHarness.getStartupState(), which resolves through IConfigService + IModelResolver without touching the session lifecycle. The session is created lazily on the first user message (sendAfterLazySessionStart), with OAuth failures entering the login-required state instead of failing startup. Model switches without a live session (activateModelSelection, formerly activateModelAfterLogin) now only record the choice in appState instead of creating a session.
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99fe170204
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
1099
to
1103
| if (session === undefined) { | ||
| this.showError(LLM_NOT_SET_MESSAGE); | ||
| // Session-less startup: create the session lazily, then deliver this | ||
| // input as its first turn. | ||
| void this.sendAfterLazySessionStart(text, extraction); | ||
| return; |
There was a problem hiding this comment.
When a fresh TUI has not created a session yet, every submitted prompt reaches this branch while this.session is still undefined; because the async sendAfterLazySessionStart() is fire-and-forget and no pending-start flag/busy state is set until after createSessionFromCurrentState() resolves, two quick Enter presses can start two independent session creations. The later one will setSession() and close/unload the earlier session while its first prompt may already be sent, dropping the first turn or leaving it in a different session. Please gate this path with a shared pending promise or mark input as busy before awaiting session creation.
Useful? React with 👍 / 👎.
The v2 engine now records `!`-command input/output with the `shell_command` prompt origin and compaction summaries with the `compaction_summary` origin, but the replay renderer had no branches for them and dumped both as plain user messages (raw <bash-*> XML and the full summary text). Restore the v1 render paths in session-replay: - shell_command input replays as the `$ cmd` echo (shellMode color, suppressed bullet); output is unwrapped from the XML tags and formatted through formatBashOutputForDisplay. - compaction_summary replays as the collapsible "Compaction complete" card (compactionData) instead of a plain message; the COMPACTION_SUMMARY_PREFIX marker is stripped via a core re-export. Token counts stay absent until replay reads wire records (G-1).
Resume replay no longer reads the compacted context view. New module src/core/transcript.ts declares a TranscriptModel via v2's public defineDerivedModel and folds it manually over IAgentWireRecordService records (the facade cannot wire.attach — replay runs inside v2 before the facade sees the session, and replay is read once per resume). The fold recovers the full pre-compaction history (user/assistant/tool messages through loop-event folding), emits compaction card entries with summary + token counts from context.apply_compaction records, and maps goal/plan/permission/approval/config ops to their replay records. The display-side TranscriptMessage type mirrors but does not alias ContextMessage, so transcript content can never be fed to the model by accident. Blob references are rehydrated through IAgentBlobService. Entry times stay 0 (wire recordToPayload drops the envelope timestamp); TUI renders by position, so this is safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
No related issue — this lands the
tui-v2work branch ontokimi-code-v2.Problem
The interactive TUI still reached the agent engine through the legacy v1 SDK (
@moonshot-ai/kimi-code-sdk). To move the TUI ontoagent-core-v2, a dedicated facade (apps/kimi-code/src/core/) was introduced.A follow-up problem with the initial migration: fresh TUI startup eagerly created an empty session just to read initial state (default model, context window, modes). That polluted the session list and forced every startup through the full session bootstrap.
What changed
Two commits:
250bd460feat(tui): migrate interactive shell to agent-core-v2apps/kimi-code/src/core/(CoreHarnessat App scope,CoreSessionat session scope); the interactive TUI reaches the engine only through this facade. Print (kimi -p), ACP and other subcommands still consume the v1 SDK.TODO(v2-gap), documented insrc/core/README.md.99fe1702feat(tui): defer session creation to the first message on fresh startupCoreHarness.getStartupState()(IConfigService+IModelResolver, plus the profile service's own thinking resolution).sendAfterLazySessionStart); OAuth login-required errors now surface at first message instead of failing startup.activateModelSelection, formerlyactivateModelAfterLogin) only record the choice in appState — used by both post-login activation and/model.Tests: new
getStartupStateunit tests intest/core/harness.test.ts; TUI startup / message-flow / replay suites adapted to the deferred-creation flow. Full suite: 2365 passed (6 failures intest/cli/telemetryandtest/cli/update/preflightare pre-existing on the base branch, verified by stash comparison).Checklist
gen-changesetsskill, or this PR needs no changeset. — no changeset per the author's call (internal migration branch, not user-facing release content).gen-docsskill, or this PR needs no doc update. — no user-doc impact:/new, resume and startup flags behave identically; only the eagerly-created empty session is gone.