Repository navigation
Conversation
Multiple clients (TUI / kap-server / VS Code / ACP) running core in
separate processes could open the same session and corrupt each
other's wire.jsonl and metadata. Each session now holds a cross-process
occupancy lock ({homeDir}/locks/sessions/{id}.lock) taken only while
writing: runs acquire it on submit / compact / undo / cron / delete /
fork / cold-archive, the append log holds it before flushing, and a
stale check compares the main agent wire's on-disk length with the
length this process knows.
Collisions surface as session.occupied (holder in details); writes by
another client detected after acquire surface as session.stale and the
caller reloads before continuing. kap-server maps them to
SESSION_OCCUPIED (40903) / SESSION_STALE (40930), reloads the session
on stale, and re-attaches WS subscriptions; the TUI shows an occupancy
dialog (retry/cancel) or reloads and replays history keeping the input;
print exits with code 4 on occupied and reloads automatically on stale;
VS Code / ACP get minimal occupancy notices. Locks are released a few
seconds after the session goes idle, on close, and on process exit;
locking I/O failures degrade to unlocked operation with a warning.
🦋 Changeset detectedLatest commit: 9c0d0b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 7d5c9e7401
ℹ️ 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".
| if (!state.lockWarned) { | ||
| state.lockWarned = true; | ||
| console.warn( | ||
| `[append-log] ${scope}/${key}: session lock unavailable; keeping writes in memory`, | ||
| ); |
There was a problem hiding this comment.
When another client owns the session, durable mutations that do not call beginSessionRun first—such as setPermission, setThinking, or setModel—still update the local fold and report success, while this path merely leaves their wire records pending. If the session closes while the owner remains active, retirement deletes that pending state; if a later flush succeeds, it appends the stale mutation without checking externallyModified. Thus settings can be silently lost or written over a newer history; ownership must be acquired before committing the mutation, and failure must propagate to the caller rather than buffering it.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| onSelect: (action) => { | ||
| if (action !== 'retry') return; |
There was a problem hiding this comment.
When a TUI user moves to the displayed Cancel option and presses Enter, onSelect('cancel') hits this return and performs none of the onCancel cleanup: the dialog remains mounted, the submitted text stays removed, and recalled attachments are not released. Esc works because it invokes onCancel directly, so the selected Cancel action should invoke that same path.
Useful? React with 👍 / 👎.
| private async ensureWriteOwnership(): Promise<void> { | ||
| if (this.sessionLocks.held(this.ctx.sessionId)) return; | ||
| this.sessionLocks.beginRun(this.ctx.sessionId); | ||
| const existing = await this.store.get<SessionMeta>(this.scope, META_KEY); |
There was a problem hiding this comment.
When a prompt has already acquired the session run lock, this early return skips re-reading state.json, even though beginSessionRun only checked the main wire journal for external changes. If another client changed metadata while this client was idle—for example, setting a custom title or archiving the session—then this client's next prompt updates lastPrompt from its stale this.data snapshot and overwrites that newer metadata. Refresh or version-check metadata on the first metadata write of each newly acquired run, rather than treating possession of the wire lock as proof that the metadata snapshot is current.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| if (code === SESSION_OCCUPIED_CODE || code === SESSION_STALE_CODE) { | ||
| log.warn('acp: prompt rejected due to a cross-client session conflict', { | ||
| sessionId, | ||
| conflict: code, | ||
| }); | ||
| return RequestError.invalidRequest({ code }, message); |
There was a problem hiding this comment.
For an ACP client whose session was updated by another process, mapping session.stale to invalidRequest leaves the stale engine scope live, so every subsequent prompt fails with the same error. Even issuing ACP session/load or session/resume does not recover because resumeAcpSession explicitly passes an already-live scope through; the client must know to close the session first, despite the error only saying to reload. Handle the stale case by closing and re-materializing the engine session, as the CLI and kap-server paths do, before allowing the ACP client to retry.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
| private alive(holder: SessionLockHolder): boolean { | ||
| if (holder.hostname !== '' && holder.hostname !== this.hostname) return true; | ||
| if (!pidAlive(holder.pid)) return false; | ||
| const bootTime = Date.now() - os.uptime() * 1000; | ||
| return holder.startedAt >= bootTime; |
There was a problem hiding this comment.
After a crash or forced termination leaves a lock file behind, the OS can reuse its PID for an unrelated process during the same boot. In that case pidAlive succeeds and startedAt >= bootTime is necessarily true for any valid lock created this boot, so the abandoned lock is treated as live until the unrelated process exits and the session remains unusable. Persist and compare an actual process-start identity, or use another reclaimable liveness mechanism, rather than using the lock acquisition timestamp only as a reboot check.
Useful? React with 👍 / 👎.
| if (this.activity.state().busy) return; | ||
| await this.appendLogStore.flush().catch(() => undefined); | ||
| if (this.activity.state().busy) return; | ||
| this.sessionLocks.endRun(this.ctx.sessionId); |
There was a problem hiding this comment.
The release timer only consults ISessionActivityView.busy, which tracks turns, background tasks, and compaction but not session lifecycle work. A live archive acquires the run lock through setArchived(true) and then drains agents, waits for close listeners, and closes logs without an explicit lease; once activity becomes idle, this timer can release the lock after three seconds while that archive is still in progress. A second process can then start the same session while the first process is still tearing it down, so lifecycle operations such as archive need an explicit lease for their full duration or must participate in the busy state.
Useful? React with 👍 / 👎.
- metadata: re-read state.json on the first write of each lock-hold epoch (holdToken), so pure-metadata changes made by another client (title, archive) are not clobbered by turn-end lastPrompt writes - lifecycle: hold an explicit lease across live archive(), matching delete(), so the idle release timer cannot drop the lock mid-teardown - tui: route the occupied dialog's Cancel selection through the same cleanup as Esc (restore input, release recalled media) - acp: tell stale-session users to close and reopen the session - append-log: warn when retirement drops records kept in memory because the session lock was unavailable
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ebcf0a22
ℹ️ 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".
| if (known === undefined) return false; | ||
| const disk = this.diskLength(scope, key); | ||
| if (disk === undefined) return false; | ||
| return disk !== known; |
There was a problem hiding this comment.
When an SDK caller deletes and recreates an empty session under the same explicit ID while another process still has the old scope open, both journals contain a single metadata record whose timestamp changes but whose encoded byte length remains identical. This comparison therefore reports no external modification, allowing the old process to acquire the lock and append using its obsolete in-memory fold instead of raising session.stale, silently mixing the old and replacement sessions. Track a journal identity, generation, or content fingerprint rather than only its size.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| releaseQuiescence = await quiesceSessionAgents(reloaded.session, reloaded.agent); | ||
| }; | ||
| flushWires = () => flushSessionWires(reloaded.session, reloaded.agent); | ||
| await runTurn(reloaded.session, reloaded.agent); |
There was a problem hiding this comment.
If the first print-mode attempt reports session.stale and another client acquires the session after the reload but before this retry, the second runTurn throws session.occupied outside the surrounding catch. It therefore bypasses reportSessionOccupied, so headless callers lose the documented exit code 4 and stream-json callers do not receive the structured session.occupied error. Route the retry through the same conflict handler, or use a bounded retry loop that handles both errors consistently.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
| state.detached = undefined; | ||
| this.attachWorkView(session, state); | ||
| this.attachAgents(sessionId, session, state); | ||
| this.attachInteractions(sessionId, state); |
There was a problem hiding this comment.
When the external writer is another kap-server process sharing the same home directory, it advances server/events/<session>.jsonl while this process is detached. The retained SessionEventJournal still caches its old sequence number, but this reattachment reuses it without reopening or rescanning the file, so the next local event appends a duplicate or out-of-order sequence and makes snapshot cursors and WebSocket resume results inconsistent for desktop and web clients. Reopen the journal from disk during reattachment while preserving the subscription targets separately.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
The prompt channels (AgentPromptChannel.submit/submitSteer, kap /prompts, promptWithSkills) write prompt metadata before calling loop.submit, and the metadata write acquires the session lock first. beginSessionRun then short-circuited on 'already held' and skipped the wire-length staleness check, so the main prompt path never detected updates written by another client. Drop the already-held shortcut: the check is a single stat and always passes while the lock is continuously held, so every acquisition path now runs it regardless of order.
If another client acquires the session between the reload and the retry, the retried turn now goes through the same conflict handler: session.occupied reports to stderr / stream-json and exits with code 4 instead of bubbling out as exit code 1.
storage.append grows the file (writeFile) before fsync/close complete, and knownLength only catches up afterwards; a beginSessionRun landing in that window (steer, queued prompts, cron fires during a turn) was misjudged as session.stale and deleted the lock file mid-turn. Track in-flight bytes per log and compare disk against known + in-flight. Cover the analogous rewrite window (post-rename syncDir) with an expected-final-length marker.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60abd1ecdb
ℹ️ 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".
| await this.appendLogStore.flush().catch(() => undefined); | ||
| if (this.activity.state().busy) return; | ||
| this.sessionLocks.endRun(this.ctx.sessionId); |
There was a problem hiding this comment.
When an idle metadata mutation such as setTitle takes longer than three seconds, or starts while this timer is awaiting a slow global flush, it only calls beginRun and does not increment an explicit lease; metadata writes also do not mark session activity as busy. This endRun can therefore unlink the lock while state.json is still being written, allowing another process to acquire the session and overwrite or race a mutation that already reported success. Although lifecycle operations now take explicit leases, the fresh evidence here is that SessionMetadata.applyUpdate still relies solely on the ambient run lock.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| }; | ||
| flushWires = () => flushSessionWires(reloaded.session, reloaded.agent); | ||
| try { | ||
| await runTurn(reloaded.session, reloaded.agent); |
There was a problem hiding this comment.
For print-mode /goal requests against a stale resumed session, the first runTurn calls createGoal before runNativeTurn detects session.stale, and runNativeGoal also emits a goal summary from its finally block. Retrying the entire closure here invokes createGoal again after reload: a normal goal then fails with goal.already_exists once the first goal record is flushed during close, while /goal replace replaces the newly created goal and emits duplicate summaries to stream-json. Reload before the goal mutation or retry only the turn portion.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
| @@ -2037,10 +2042,84 @@ export class KimiTUI { | |||
| return; | |||
| } | |||
| this.staging.trackDispatch(stagingLease, session.prompt(sdkInput, { promptId: submissionId }), (error) => { | |||
There was a problem hiding this comment.
The new occupied/stale recovery is attached only to the normal session.prompt dispatch. Existing inline-skill and plugin-command submissions also reach AgentLoopService.submit and now receive these conflict errors, but their unchanged handlers only show a generic failure after the editor input has been consumed; inline-skill failures additionally leave the optimistic transcript entry in place. Thus these TUI users get neither the retry/cancel dialog nor stale reload and input restoration promised for sending from a second client. Route all prompt-launch variants through the same recovery path.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
| ...(options?.imageAttachmentIds ?? []), | ||
| ...(options?.videoAttachmentIds ?? []), | ||
| ]); | ||
| await reloadSessionAfterStale( |
There was a problem hiding this comment.
When the rejected prompt contains a pasted image or video, releaseRecalled keeps its staged media usable only while the attachment store still owns the corresponding IDs. This reload calls switchToSession, whose transcript reset clears the attachment store and deletes those staged files, and only afterward does handleSessionStale restore the original placeholder text. The next submission treats the now-unknown placeholders as plain text, so clipboard media is silently and irrecoverably dropped instead of being returned with the user's message. Preserve or rehydrate the attachment snapshots across this reload.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
The equality check disk !== known + inFlight moved the false-positive
window to before the append lands (mkdir/open) and into chunked
writes, so beginSessionRun during a turn (steer, cron, queued prompts)
could misjudge, delete the lock file mid-turn, and later append behind
another client. Compare known <= disk <= known + inFlight instead, and
evaluate the check only on a fresh hold (holdToken epoch): re-entrant
runs pass through without a stat, matching the RFC's 'already held
passes through' while every acquisition path still triggers the check
exactly once per hold.
Also unify the occupancy prompt copy across clients: the engine error
message is now the single user-facing text ('该会话正在被其他 Kimi
Code 进程使用；请停止其他进程中的会话或退出该进程后重试'), and the
TUI dialog, /compact /undo errors, print output, and the VS Code
fallback all display it instead of maintaining their own wordings.
- metadata: hold an explicit lease across each state.json write so the idle release timer cannot unlink the lock mid-write - print: preflight the session lock before goal creation; make goal creation idempotent and emit the goal summary only for attempts that did not fail with session.stale, so a stale reload + retry neither duplicates the goal nor the summary - tui: route every prompt-launch path (plain, goal steer, plugin command, inline skill, batch steer) through one conflict handler with the occupied dialog and stale reload + input restore - tui: snapshot and restore the attachment store across stale reloads so pasted media placeholders still resolve after history replay
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 786853ce66
ℹ️ 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".
|
|
||
| SESSION_BUSY: 40901, | ||
| APPROVAL_ALREADY_RESOLVED: 40902, | ||
| SESSION_OCCUPIED: 40903, |
There was a problem hiding this comment.
When a kap-server client encounters session occupancy, this emits 40903, but the published API already assigns 40903 to an already-completed prompt (docs/en/reference/server-api.md:1093,1121,1153). Desktop, web, or external clients that special-case that documented idempotency outcome can therefore interpret an occupied rejection as completion and silently discard the unsent prompt. Assign occupancy an unused protocol code and update both locale documents and API snapshots.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
| const disk = this.diskLength(scope, key); | ||
| if (disk !== undefined) state.knownLength = disk; |
There was a problem hiding this comment.
When another process appends after this reader reaches EOF but before the finally block stats the file, knownLength includes records that were never decoded into the in-memory fold. After the writer releases its lock, externallyModified sees equal lengths and allows this stale client to append instead of raising session.stale, silently mixing histories. Record the byte count actually consumed, or capture a stable file snapshot before reading.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| restore: () => { | ||
| this.staging.releaseRecalled(rewrite.imageAttachmentIds); | ||
| this.restoreInputText(args); | ||
| }, |
There was a problem hiding this comment.
When an occupied plugin command is cancelled, or a stale session is reloaded, this restores only args; the original input was /${pluginId}:${commandName} ${args}. For example, cancelling /foo:bar x leaves only x in the editor, so pressing Enter sends an ordinary prompt rather than retrying the command, while an argument-free command restores an empty editor. Reconstruct and restore the complete command line.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
|
|
||
| submit(input: UserEntry, options?: LoopSubmitOptions): LoopSubmitResult { | ||
| if (this.disposing) throw abortError('Agent loop disposed'); | ||
| beginSessionRun( |
There was a problem hiding this comment.
When a loaded session was changed by another process before a cron job becomes due, this new check throws session.stale, but deliverFire catches every submit exception and merely returns false (cronService.ts:195-205). The cursor remains due while the same stale scope is retried on every tick, so unattended recurring and one-shot cron jobs never fire again until some separate user action reloads the session. Reload or retire the session when cron encounters this error instead of repeatedly retrying the stale scope.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| if (this.tryHoldSessionLock() && !this.log.externallyModified(this.wireScope, AGENT_WIRE_RECORD_KEY)) { | ||
| await this.log.rewrite(this.wireScope, AGENT_WIRE_RECORD_KEY, rewrittenRecords); |
There was a problem hiding this comment.
When a client opens an older-format session while another process holds the lock, this condition skips the required journal rewrite but discards rewrittenRecords instead of remembering that migration is pending. Once the other process releases the lock without changing the file, beginSessionRun considers the journal current and appends modern records under the old protocol header; later readers then apply the old migration to those new records and can misread or corrupt the session. Preserve the deferred rewrite and complete it after acquiring ownership, before permitting any append.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
…release locks on exit SESSION_OCCUPIED reused 40903, which belonged to PROMPT_ALREADY_COMPLETED until #3747 retired it, and web clients still allowlist 40903 for that old meaning; reusing it conflated the two semantics. Move session occupancy to the never-used 40931 and update the server API reference in both locales. The occupied error message was hardcoded in Chinese while every other engine error is English, so the TUI, print mode, VS Code, and ACP clients all rendered mixed-language output; replace it with an English equivalent. The lock release hook ran on 'beforeExit', which never fires when the process leaves through process.exit (SIGHUP in the TUI goes through emergencyTerminalExit), leaving stale lock files that could keep a session looking occupied forever after a hostname change or pid reuse. releaseAll is fully synchronous, so hook 'exit' instead.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 610f8e36ab
ℹ️ 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".
| this.sessionLocks, | ||
| this.appendLogStore, | ||
| this.sessionContext.sessionId, | ||
| this.sessionContext.scope(`agents/${MAIN_AGENT_ID}`), |
There was a problem hiding this comment.
When another client directly advances a subagent journal without changing agents/main/wire.jsonl, submitting to that subagent checks only the unchanged main journal here. Because beginSessionRun also caches the successful check by session token rather than by journal, the subsequent append proceeds from the stale subagent fold and can silently mix histories. Validate the current agent's journal, or all loaded journals, before accepting writes.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 45e05a5. On the first check of each lock hold, beginSessionRun now runs externallyModified over the wire of every agent loaded in this process (IAgentLifecycleService.list()), not only main. If any of them changed on disk, it releases the lock and throws session.stale. An agent that is not loaded is read from disk when it loads, so it does not need the check. All four call sites (loop submit, manual compaction, undo, print preflight) go through the same function.
| this.setAppState({ streamingPhase: 'idle' }); | ||
| this.resetLivePane(); | ||
| const mediaSnapshot = this.imageStore.snapshot(); | ||
| void (async () => { |
There was a problem hiding this comment.
After a stale prompt, the UI is set back to idle and this reload is launched without being awaited or represented as an in-flight session replacement. A user can therefore type another message while reloadSessionAfterStale is pending; when it finishes, the unconditional restore() replaces that newly typed text with the rejected prompt, silently losing the user's intervening input. Keep the editor unavailable until reload and restoration complete, or merge with any text entered during the reload.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 45e05a5. The stale branch no longer drops to idle before the reload. The request stays in flight until the reload, the attachment restore and the input restore have finished, so Enter during the reload queues the message instead of sending it to the closing session. The queue is kept across switchToSession and drained into the reloaded session. When the rejected prompt is restored, text typed during the reload is appended after it instead of being overwritten.
|
|
||
| submit(input: UserEntry, options?: LoopSubmitOptions): LoopSubmitResult { | ||
| if (this.disposing) throw abortError('Agent loop disposed'); | ||
| beginSessionRun( |
There was a problem hiding this comment.
For normal prompts and bundled-skill prompts, AgentPromptChannel calls applyPromptMetadataUpdate before reaching this new stale check. If another process changed the wire journal and released the lock, the rejected client first persists the unsent prompt as lastPrompt (and can generate a title/index update), then beginSessionRun throws session.stale; reloading does not undo that metadata mutation. Perform the stale preflight before prompt metadata is written so a rejected prompt cannot silently alter session metadata.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly addressed in c4c3e88. Whether to derive a title from the prompt is now decided inside the metadata service, after state.json has been re-read for the new lock hold. A stale cache can no longer overwrite a title another client wrote, and SessionMetaUpdated only carries a title that was actually written.
Writing lastPrompt before the stale check is accepted as is. The rejected prompt goes back to the input and is normally resent right after the reload, which records the same lastPrompt. If the user drops it instead, the session list shows that text as the last prompt until the next send. A title is derived from the rejected prompt only when the session is still untitled on disk, and that is the title the resend would produce anyway. Running the stale preflight first would change the call order at six call sites (prompt channel, plugin commands, two skill paths, two kap prompt routes) for a preview field.
| state.knownInteractions.clear(); | ||
| state.deferredWork = undefined; | ||
| state.transcriptSeeded.clear(); | ||
| state.deferredTranscriptSeeds.clear(); |
There was a problem hiding this comment.
When a stale kap-server session is detached while another process finishes its turn or subagents, this block clears subscriptions and transcript state but preserves state.tracker and state.roster. Reattachment only listens for future events, so REST snapshots and WebSocket resyncs can continue reporting an obsolete in-flight turn or running subagents until another local turn happens; clear or rebuild both trackers at the detach/reattach boundary.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 45e05a5. detachSession now clears the in-flight turn tracker and the subagent roster for the session, so after reattach the REST snapshot and the WS resync no longer report the turn or the subagents of the closed handle.
| "session.init_failed": "Failed to initialize the session.", | ||
| "session.closed": "Session was closed.", | ||
| "session.fork_active_turn": "Wait for the current response before forking.", | ||
| "session.stale": "Session was updated by another client. Reopen the session to load the latest updates.", |
There was a problem hiding this comment.
When a VS Code prompt is rejected with session.stale or session.occupied before TurnBegin, SessionRuntime emits a preflight error, whose store handler restores the input without rendering an inline error; the only visible notification is the toast gated by isPreflightError(code). Neither new code is in PREFLIGHT_CODES, and occupancy has no message entry at all, so sending appears to be silently ignored even though this stale message was added. Add both conflict codes to the preflight set and provide the occupancy message or recovery action.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 45e05a5. session.occupied and session.stale are now in PREFLIGHT_CODES, and ERROR_MESSAGES has an entry for session.occupied with the same copy the engine emits. The webview withdraws the message, puts the text back in the input, and shows the toast; the toast text comes from getUserMessage in the extension host.
…sion metadata applyPromptMetadataUpdate decided whether to derive a title from the prompt from the in-memory metadata cache, which is only refreshed when the session lock is re-acquired. If another client named the session in the meantime, the stale cache still looked untitled, so the next prompt from this client overwrote the other client's title — both on the stale path, where the prompt is rejected afterwards, and on the fresh path, where the other client only changed the title on disk. Move the untitled check into SessionMetadata.applyPromptMetadata, which runs after ensureWriteOwnership() has re-read state.json, so the decision is based on the latest persisted metadata. The published SessionMetaUpdated event now carries the derived title only when it was actually written.
- vscode: classify session.occupied and session.stale as preflight errors so the webview toasts them instead of silently swallowing the send, and give session.occupied the same English copy the engine emits (the toast text comes from getUserMessage in the extension host). - engine: beginSessionRun now takes the session context and agent lifecycle service, and on the first check of each lock hold it runs externallyModified over every loaded agent's wire (scope agents/<agentId>, key wire.jsonl) instead of only the main agent's, so a fork that only touched a subagent journal is detected as stale. All four call sites (loop submit, manual compaction begin, undo, print preflight) pass the extra dependency. - tui: keep the request in flight across a stale reload instead of dropping to idle, so input submitted during the reload queues rather than racing the closing session; park the queue around switchToSession (resetSessionRuntime would clear it) and drain it into the reloaded session afterwards; when restoring the rejected text, prepend it before content the user typed during the reload instead of overwriting it. - kap-server: clear the in-flight turn tracker and subagent roster in detachSession so a reattached session no longer reports stale in-flight turns or running subagents in REST snapshots and WS resyncs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45e05a5d30
ℹ️ 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".
| if (this.tryHoldForWrite()) { | ||
| await this.store.set(this.scope, META_KEY, encodeSessionMeta(this.data)); | ||
| } |
There was a problem hiding this comment.
When two processes create the same explicit session ID before either has appended its index entry, the second process reaches this branch while the first holds the new session lock. tryHoldForWrite() returns false, but creation continues, mirrors the in-memory metadata, and reports success without writing state.json; subsequent operations run against a ghost scope for the other process's session and can later persist stale state. Propagate session.occupied and abort materialization instead of silently skipping the initial write.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| beginSessionRun( | ||
| this.sessionLocks, | ||
| this.appendLogStore, | ||
| this.session, | ||
| this.agentLifecycle, | ||
| ); |
There was a problem hiding this comment.
When undo restoration or participant reconciliation takes more than three seconds, beginSessionRun is the only hold acquired here, while ISessionActivityView.busy remains false because undo is not a turn, task, or compaction. The release timer can therefore unlink the lock after the branch switch but before the later restore/reconciliation records finish, allowing another process to mutate the same session concurrently. Acquire an explicit lease around the complete undo transaction.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| private alive(holder: SessionLockHolder): boolean { | ||
| if (holder.hostname !== '' && holder.hostname !== this.hostname) return true; | ||
| if (!pidAlive(holder.pid)) return false; |
There was a problem hiding this comment.
When KIMI_CODE_HOME is on shared storage and the lock-owning host crashes or becomes unavailable, any client on a differently named host treats the holder as alive unconditionally. The PID and timestamp are never examined, so the session remains permanently occupied until someone manually deletes the lock file. Use a remotely reclaimable lease or an expiry/liveness mechanism instead of making every foreign-host lock immortal.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
Detect external appends that race journal reads so a later run cannot miss session.stale. Keep deferred wire migrations pending and finish them before appending, while rejecting the append if migration still cannot complete. Restore the complete plugin command after an occupied cancellation, including commands without arguments.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84e33ca296
ℹ️ 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".
| await closeSessionById(app.accessor, resolved.session.id); | ||
| const reloaded = await resolveNativeSession(app, opts, workDir, defaultModel, stderr); | ||
| resolved = reloaded; | ||
| restorePermission = reloaded.restorePermission; |
There was a problem hiding this comment.
When a resumed print-mode session hits session.stale, resolveNativeSession has already changed its permission mode to auto and saved a closure that restores the previous mode. This closes the old scope without invoking that closure, then replaces restorePermission with the reloaded scope's closure; because closing drains the old wire, the reload observes auto as its previous mode. A one-off headless run can therefore silently leave future interactive clients in auto mode. Restore the original override before closing, or perform stale preflight before applying the temporary permission change.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const lastCheckedTokens = new Map<string, string>(); |
There was a problem hiding this comment.
In a long-lived process such as kap-server, the first run of every distinct session inserts its ID and random lock token into this process-global map, but ending the run, closing the session, deleting it, and disposing the lock service never remove the entry. Servers that encounter an unbounded stream of session IDs therefore retain all of those strings for the lifetime of the process. Keep the checked token in the held-lock state or remove it whenever the corresponding lock hold ends.
Useful? React with 👍 / 👎.
Replace the process-global session-id map with a WeakMap keyed by ISessionContext so closed and reloaded sessions do not retain cache entries for the lifetime of the process.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 325a97d8ea
ℹ️ 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".
| const current = this.readHolder(reclaimPath); | ||
| if (current === undefined || !this.alive(current)) this.unlinkSilent(reclaimPath); | ||
| return; |
There was a problem hiding this comment.
When a process crashes after publishing the .reclaim file but before removing it, the next acquisition's first attempt only deletes that stale reclaim marker and returns; its second and final attempt then acquires the reclaim marker and deletes the original stale lock, but publish() exits the loop and throws session.occupied even though no lock remains. This makes the first prompt after such a crash fail spuriously and forces the user to retry; continue the acquisition loop after either stale marker is removed.
Useful? React with 👍 / 👎.
| case 'session.occupied': | ||
| reply.send( | ||
| errEnvelope(ErrorCode.SESSION_OCCUPIED, err.message, requestId, err.stack, err.details), | ||
| ); |
There was a problem hiding this comment.
When POST /sessions/{session_id}/profile updates a title while another process holds the session, metadata.setTitle() now throws session.occupied and the route reaches this new mapping, but updateProfileRoute declares only validation and not-found responses at lines 502-505. Its Fastify oneOf response serializer therefore cannot serialize the 40931 envelope and replaces the actionable occupancy response with a serialization error; add the conflict code to every route schema that can persist metadata, including title generation.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
| private lockPath(sessionId: string): string { | ||
| return join(this.lockDir, `${sessionId}.lock`); |
There was a problem hiding this comment.
For public SDK callers, CreateSessionOptions.id is an unconstrained string, so an existing ID such as tenant/session is persisted successfully under nested session directories. This path construction turns that ID into locks/sessions/tenant/session.lock, but ensureLockDir() creates only locks/sessions; the temporary lock write therefore fails with ENOENT, publish() fail-opens, and two processes both believe they hold the session while concurrently appending to it. Encode or hash the opaque session ID into one safe filename rather than treating it as a path.
Useful? React with 👍 / 👎.
Declare SESSION_OCCUPIED and SESSION_STALE in the response schemas for profile updates, title generation, and child creation. Cover the OpenAPI declarations and the existing HTTP 200 conflict envelopes with regression tests.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9d5e945f6
ℹ️ 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".
| await this.ensureWriteOwnership(); | ||
| const lease = this.sessionLocks.acquire(this.ctx.sessionId); |
There was a problem hiding this comment.
When a first metadata update begins while SessionLockReleaseService.releaseWhenIdle() is already awaiting the global append-log flush, ensureWriteOwnership() can read state.json under the old run hold, then that callback can unlink the lock before this explicit lease is acquired. Another process can complete an update in that gap; this process then reacquires the lock and writes its pre-gap snapshot, silently overwriting newer title or custom metadata. The fresh evidence after the earlier lease finding is that the lease was added only after the refresh; acquire it before refreshing and retain it through the write.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| requireConfiguredModel(model); | ||
| const goalService = agent.accessor.get(IAgentGoalService); | ||
| const existing = goalService.getGoal().goal; | ||
| if (existing?.status === 'active' && existing.objective === goal.objective) return; |
There was a problem hiding this comment.
For a resumed print session that already has an active goal X, this unconditional same-objective return changes both existing commands: /goal X used to fail with goal.already_exists, while /goal replace X used to clear the old goal and create a new goal ID with reset accounting; both now run another turn against the existing goal. Restrict this idempotence check to the post-stale retry that created that exact goal rather than applying it on the initial request.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
| private tryHoldSessionLock(): boolean { | ||
| try { | ||
| this.sessionLocks.beginRun(this.sessionContext.sessionId); | ||
| return true; | ||
| } catch { | ||
| return false; |
There was a problem hiding this comment.
When a user-facing durable mutation runs while another process holds the session—for example, POST /sessions/{id}/profile with agent_config.model calls profile.setModel()—the event is folded into memory and the setter returns before persistence. This callback catches session.occupied and converts it to false, so AppendLogStore merely retains the record in memory while the API reports success; because no local lock was acquired, no release timer is scheduled to retry, and closing the scope can drop the update or a later flush can append it after external changes. User-initiated agent-state mutations must acquire and validate ownership and surface the conflict rather than using the best-effort buffering path.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| beginSessionRun( | ||
| this.sessionLocks, | ||
| this.appendLogStore, | ||
| this.sessionContext, | ||
| this.agentLifecycle, | ||
| ); |
There was a problem hiding this comment.
When desktop or web invokes POST /sessions/{session_id}/skills/{name}:activate for an occupied or stale loaded session, IAgentSkillService.activate() now reaches this preflight and throws session.occupied or session.stale. The skill route declares neither response and its local mapper rethrows both, so the global handler reports INTERNAL_ERROR; it also never reloads a stale scope, causing every repeated skill activation to fail the same way until another endpoint reloads the session. Add both conflict responses and apply the same stale-session recovery used by the prompt route.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
Move session error mapping and stale reloads into one shared route helper, reuse shared conflict response declarations, and cover skill activation conflicts. Remove the duplicate prompt, session, and skill error mappers while preserving their existing envelopes.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b806c3f223
ℹ️ 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".
| if (isError2(error) && error.code === SessionErrors.codes.SESSION_OCCUPIED) { | ||
| this.log.warn( | ||
| 'session metadata agent registration skipped; session is in use by another client', | ||
| { sessionId: this.ctx.sessionId, agentId }, | ||
| ); | ||
| return; |
There was a problem hiding this comment.
When standalone agent creation such as TUI /btw, kap :btw, or SDK startBtw() runs while another process owns the session, AgentLifecycleService.create() awaits this method, but this branch swallows session.occupied and lets creation report success. The child remains in the in-memory roster without an entry in state.json; if its buffered journal is later persisted after the lock becomes free, it becomes orphaned and disappears on reload. Rethrow the conflict so agent creation rolls back instead of treating registration as best effort.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| let targetId: string | undefined; | ||
| let targetSessionDir: string | undefined; | ||
| const quiescenceHolds: IDisposable[] = []; | ||
| const sourceLease = this.sessionLocks.acquire(sourceId); |
There was a problem hiding this comment.
When this process has a loaded source session and another client updates and releases it before the fork, this lease acquisition succeeds without running the journal staleness check or refreshing metadata. The later ISessionMetadata.read() therefore returns the old cached title, custom metadata, and agent roster; although copySessionFiles() copies the current disk state, the fork then overwrites the target state.json from that stale cache and silently omits the other client's updates. Validate the loaded journals and reread source metadata under this lease, or reject with session.stale, before building the fork.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0d0b9915
ℹ️ 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".
| const tmp = `${lockPath}.tmp-${process.pid}-${++tmpSeq}`; | ||
| try { | ||
| fs.writeFileSync(tmp, body, { mode: 0o600 }); | ||
| fs.linkSync(tmp, lockPath); |
There was a problem hiding this comment.
When KIMI_CODE_HOME is placed on a filesystem without hard-link support (for example, some FAT, FUSE, or network-mounted volumes), fs.linkSync throws an error such as EPERM or ENOTSUP; publish() catches it and returns a token while creating no lock file. Each process then records the session as locally held and proceeds with writes, so concurrent clients can corrupt the journal despite this feature's occupancy guarantee. Use an atomic primitive supported by the target filesystem, or fail the write instead of continuing unlocked.
Useful? React with 👍 / 👎.
|
|
||
| submit(input: UserEntry, options?: LoopSubmitOptions): LoopSubmitResult { | ||
| if (this.disposing) throw abortError('Agent loop disposed'); | ||
| beginSessionRun( |
There was a problem hiding this comment.
When a direct SDK caller invokes Session.activateSkill() on an occupied or stale session, AgentSkillService.activate() reaches this new preflight through loop.submit() and propagates the engine's Error2. Unlike prompt, promptWithSkills, and plugin commands, SDKRpcClientV2.activateSkill() does not pass that exception through restateEngineError, so the public call rejects with a non-KimiError and consumers using the exported isKimiError() cannot recognize or recover from session.occupied or session.stale. Wrap this entry point with the same normalization used by the other prompt-launch methods.
Useful? React with 👍 / 👎.
Requirement or Bug
多个客户端（TUI、kimi web / desktop、VS Code、ACP）同开一个会话时 wire 会分叉、元数据互相覆盖；改为同一时刻只允许一个进程写，写之前发现他端写过就先重新载入。
Bug Reproduction Steps
N/A
Root Cause
N/A
Code Changes
每个会话一把跨进程占用锁
{homeDir}/locks/sessions/{id}.lock，只在写会话时持有，打开、查看不拿锁。锁文件记version / token / pid / hostname / startedAt / clientIdentity，先写临时文件再link发布，已存在即失败。判活规则：pid 存活且加锁晚于本次开机才算活锁；hostname 与本机不同一律按占用处理；拿锁时 IO 出错，告警后不加锁继续。「已知长度」由 AppendLogStore 维护：读取时记下，本进程追加时累加，改写时重置。在途字节用来容忍自己还没落盘或分块写到一半的追加。
错误分流：
session.occupied（40931）──┬─ TUI：占用对话框 Retry / Cancel，Cancel 时原文回到输入框 ├─ kap：SESSION_OCCUPIED，holder 在 details ├─ print：开跑前（含创建 goal 前）预检锁；撞锁时 stderr 提示，退出码 4；stream-json 输出结构化错误 ├─ VS Code：按 preflight 错误处理，撤回消息、原文回到输入框，toast 显示同一文案 └─ ACP：显示 engine 文案 session.stale（40930）────┬─ TUI：重新载入并回放历史，原文和已粘贴的附件回到输入框并提示，由用户再发； │ 重载期间保持请求中：新敲的字接在原文后面，回车的消息排队，重载完成后发给新会话 ├─ kap：重新加载会话并重挂 WS 订阅（推一次 transcript reset），再返回 SESSION_STALE ├─ print：自动重新载入后继续；goal 创建幂等，重试不会重复创建 goal 或重复输出摘要 ├─ VS Code：按 preflight 错误处理，撤回消息、原文回到输入框，toast 提示重新打开会话 └─ ACP：提示重新打开会话TUI 里所有发起 prompt 的路径（普通发送、goal steer、插件命令、inline skill、批量 steer）都走同一个冲突处理。
占用文案以 engine 为准，各端直接显示：
This session is in use by another Kimi Code process. Stop the session in that process or exit it, then try again.VS Code 的 toast 优先取ERROR_MESSAGES，那里放了一份相同的副本，由测试钉住两者逐字一致。文件职责：
TUI 侧埋点：
occupied_prompt_shown（holder 端 × 开跑端）、occupied_choice（retry / cancel）、session_stale_reload。desktop / web 的专门提示在 code-app 另开 PR：MoonshotAI/kimi-code-app#899。web 产物（dist-web）等两边都合入后再同步。在那之前，web 走现有的通用发送失败路径：撤回乐观显示的消息，原文回到输入框；过期时 kap 已重载会话，历史会经 transcript reset 自动刷新。
Behavior Changes and Affected Users
session.occupied；打开、查看不受影响session.stale，本进程已加载的任一 agent wire 被他端写过即触发（含只写了子 agent 的 side chat）：TUI 重载回放并保留输入；kap 重载会话后返回 40930；print 自动重载继续；VS Code / ACP 提示重新打开SESSION_OCCUPIED，可重试PATCH /sessions/{id}的外部脚本session.occupiedsession.occupied；本端会话已关闭，kap 已清掉该会话的 WS 订阅和事件 journal，重新打开即可{homeDir}/locks/sessions/<id>.lock（共享 home、改过 hostname 的机器）exit时同步删除本进程持有的锁文件4（避开 0 / 1 / 2 / 3 / 6 / 129 / 130 / 143）；stream-json 输出{"type":"error","code":"session.occupied","message":…,"details":…}SESSION_OCCUPIED（40931）与SESSION_STALE（40930）；40903 是 9 月退役的PROMPT_ALREADY_COMPLETED，不复用受影响模块与测试覆盖（除注明外，均在 45e05a5 上本地运行）：
agent-core-v2：全量 7215 通过，1 个失败（tool.test.ts前台 subagent 超时），main 上同样失败sessionLockService.test.ts：占用、死锁回收、hostname、lease 配对、IO 降级、退出时释放beginSessionRun：fresh / stale / occupied / 自身追加 / 每次持锁只查一次 / 其他路径先拿锁也会检查 / 只有子 agent 的 wire 被外部追加也判 staleappendLogStore.test.ts：已知长度、改写重置、落盘前持锁、自己的追加落盘前和分块写到一半都不算外部修改sessionMetadata.test.ts：新持锁后重读 state.json；缓存未命名而磁盘上已有他端标题时保留该标题，事件里的标题与实际写入一致kap-server：detach 后快照里没有旧的在途 turn 和子 agent；全量 1447 通过apps/kimi-code：占用对话框、附件保存恢复、print goal 重试、过期重载期间输入合并与排队消息发给新会话等用例；全量 4202 通过apps/vscode：两个码按 preflight 处理，occupied 文案与 engine 一致；全量 384 通过Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.