feat: engine-native image references via kimi-file:// media resolver - #2593
feat: engine-native image references via kimi-file:// media resolver#25937Sageer wants to merge 61 commits into
Conversation
🦋 Changeset detectedLatest commit: 533df90 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
commit: |
|
❌ Nix build failed |
|
@codex review |
![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: eb31813cc9
ℹ️ 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 file = await store.open(part.source.file_id); | ||
| if (file === undefined) throw fileNotFoundError(part.source.file_id); |
There was a problem hiding this comment.
When a client resubmits a stored attachment using the new session_media source, this path only checks that the file exists. Unlike transient file sources above, it never verifies file.mediaType against the requested image/video kind, so {type:'image', source:{kind:'session_media', file_id:<video>}} is accepted and persisted as an image daemon ref; the resolver can then degrade/mislabel the video as an <image path=...> instead of rejecting the bad prompt before controls/metadata mutate. Apply the same assertMediaFile check to the opened session media.
Useful? React with 👍 / 👎.
| const tail = text.slice(cursor); | ||
| pushText(parts, tail); | ||
|
|
||
| store.retainFileIds(imageAttachmentIds); |
There was a problem hiding this comment.
This retain happens during extraction, before a submission is actually leased. If the idle cache-hint dialog intercepts a prompt with a pasted image and the user dismisses/restores the input, sendNormalUserInput returns before creating a StagingLease, so this retain is never balanced by takeFileIds; submitting the restored prompt later retains the same daemon file a second time, and turn-end cleanup only decrements one use, leaving the upload staged until transcript/session clear. Retain only after dispatch is leased, or release the retained ids when the cache-hint path restores the stashed input.
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: eb31813cc9
ℹ️ 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".
| readonly origin: PromptOrigin; | ||
| readonly prompt?: string; | ||
| /** The prompt record id when the turn was opened by a prompt submission. */ | ||
| readonly promptId?: string; |
There was a problem hiding this comment.
This adds promptId to the public TurnStartedEvent type, but the matching turnStartedEventSchema in this file still only accepts type, turnId, origin, and prompt. Because Zod object parsing strips unknown keys by default, any client that validates WS events through sessionEventMessageSchema/turnStartedEventSchema will drop the server's promptId, so prompt-to-turn correlation for client-chosen prompt IDs (for example staged media cleanup keyed off the echoed prompt id) stops working after validation.
Useful? React with 👍 / 👎.
…media ref contract
…ath, keep unpaired tags
…nal projection A projector that attached after turn.started built the terminal turn.upsert with an empty header, and the whole-header replace downstream wiped the backfilled origin / prompt / attachmentIds — only the debounced best-effort heal could restore them. Fall back to the producer store's seeded header (via a new optional ProjectorLookups.turn) when currentTurn misses, and cover the mid-turn attach path with a service-level regression test.
…gent/media The kimi-file:// daemon reference grammar, media path tags, and the tag/ref fold are engine-internal conventions, not provider-wire contract; keep src/kosong untouched. Root exports and SDK re-exports are unchanged.
Pasted and uploaded media now materialize under the session's own media/ dir instead of the shared cache, so the copies follow the session's lifecycle: fork carries them along, session deletion cleans them up. A new Session-scope ISessionMediaStore owns the dir: atomic tmp+rename materialization with a unified extension policy, and canonical-vs-hint display-path resolution. The persisted ?path= is a write-time snapshot — readers prefer the session-canonical location, so fork and home relocation never hand the model a dead path. Prompt intake normalizes every daemon reference through the single enqueue funnel (REST edge, SDK prompt/steer, gateway), serialized in arrival order to keep the FIFO across the async file I/O. The kap-server edge materializes through the same store with a shared-cache fallback, and the request-time resolver refreshes stale persisted and memoized path tags; a claimed video reference degrades to its tag alone instead of duplicating it.
… path The record now joins the FIFO synchronously and its daemon-ref intake runs as a per-record promise, awaited by the launch and steer paths before the message is consumed — queue order, list/abort visibility, and prompt submission latency no longer wait on file I/O, and a slow intake no longer head-of-line blocks later prompts. The launching record is tracked so abort and clear stay reachable inside the launch window; startNext re-checks cancellation after every await (intake race, hook, turn admission), a cancelled record is never re-queued, and a compaction requeue waits for onDidFinishCompaction instead of busy-looping the scheduler.
…ring pairMediaPathTagRefs now exposes claimingRefByTagIndex, and claimingRefIndex reads it instead of recovering the claimer by path equality — which mis-attributed a tag when two different fileIds carried the same path in an interleaved sequence, breaking the pair and leaking the tag as user text. Also covers the memoized-video-tag claimed-drop branch.
…g parity The cold rebuild's user-slash branch now folds the turn-opening input like any user turn (claimed tag out of the prompt text, one attachment entity), matching the live projection. The ref extraction is consolidated into the contract module (daemonFileRefFromPairingPart, the mirror of the engine's daemonFileRefFromPart) and the mirror carries the new claimingRefByTagIndex map. A new kap-server parity test imports both implementations and asserts identical pairings over shared fixtures, so the engine/mirror pair can no longer drift silently.
The global search indexer concatenated every text part of a persisted user message, so the upload pair's <image path> tag made pure-image prompts searchable and wrote the materialization path into the index — breaking the module's documented pure-image invariant and diverging from the live route. textOfContent now folds the pair like every other read model (with a fold-safe coercion for malformed wire parts). Also pins the prompt-media cache-dir fallback with a read-only session media dir test (skipped as root).
…oadFile rejection foldMediaPathTagRefs and matchSingleMediaPathTag join the daemon file-reference helper re-exports so hosts can fold the upload tag+ref pair without importing agent-core-v2; the v1 harness's uploadFile not_implemented rejection is pinned by a test.
…s atomic in steer input Resumed-session replay rendered the upload pair raw — the <image path> tag as user text and the kimi-file:// url as an XML-ish reference — and the markdown export leaked the tag into both the turn body and the overview topic. contentPartsToText and the exporter now fold the pair, and daemon references render as a bare [image]/[video] placeholder. combineSteerInput moves to tui/utils/steer-input and no longer merges a standalone media tag into adjacent text, which would have broken the engine-side pairing for steered image messages.
A queued pure-image message opens with a standalone `<media path>` tag, which combineSteerInput keeps atomic. With the previous item ending in a media part, the '\n\n' separator landed as a stranded whitespace-only text part between the media part and the tag, normalizePromptInput rejected the steer, and the already-cleared queue lost the messages. Treat a leading standalone tag as media so the separator is dropped there.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aae53f0ff5
ℹ️ 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 cacheKey = `${ref.fileId}\0${providerKey}`; | ||
|
|
||
| const memoed = this.resolved.get(cacheKey); | ||
| if (memoed !== undefined) return this.memoedOutcome(ref, memoed); |
There was a problem hiding this comment.
When a video is first resolved with a video-capable model and the same session later switches to a same-provider model whose capabilities have video_in: false, this memo hit returns the prior ms:// video_url before the uncached path's capability check runs. That makes subsequent history requests send a video part to a model that cannot accept video instead of degrading to the path tag, so conversations can start failing after a model switch; include the capability/model in the cache key or check video_in before honoring cached upload parts.
Useful? React with 👍 / 👎.
…oads The video memo hit path returned a cached ms:// part before the current model's capability check, so switching to a same-provider model with video_in:false sent a video part the model cannot accept instead of degrading to the path tag. Gate on capability first, mirroring the image strategy.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1e2db64b9
ℹ️ 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.state.queuedMessages.length === 0) return undefined; | ||
| const last = this.state.queuedMessages.at(-1)!; | ||
| this.state.queuedMessages = this.state.queuedMessages.slice(0, -1); | ||
| this.staging.releaseQueued([last]); |
There was a problem hiding this comment.
When the user recalls a queued media prompt with Up-arrow while a turn is still running, this releases the queued item's staged files even though the item is not being discarded—it is moved back into the editor. For queued videos, releaseQueued deletes the cached stagingPaths, and the restored editor text only contains the placeholder; if the original video source path has already disappeared, the next submit cannot recreate the cache copy and the media that was safely queued is lost. Transfer ownership to the editor instead of releasing here, and only release if the restored draft is actually abandoned.
Useful? React with 👍 / 👎.
- error-codes 40923 编号冲突：保留 main 的 SESSION_TITLE_UNAVAILABLE， PROMPT_ID_CONFLICT 顺延为 40924（同步 kap-server / protocol / klient dispatcher 及相关测试断言） - prompts.test.ts / sdk-rpc-client-v2.test.ts 合并两侧新增测试 - promptMetadataText.test.ts 合并两侧头部注释
…g it Recalling a queued media prompt into the editor is not a discard, but the recall path released the staged files: image attachments lost their daemon upload (resubmit silently downgraded to inline), and a recalled video's cache copy was deleted even though re-materialization needs a source that may already be gone. Recall now consumes only the retain (the next submit re-retains), retires the cache copy to session lifetime, and rebases the video attachment onto that copy.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Count launching media prompts in queued events
When an idle submission contains a kimi-file:// media part, enqueueAccepted() calls startNext() before publishing prompt.queued; startNext() synchronously shifts the record out of pending into launchingItem, so this event reports queueLength: 0 even though the prompt is accepted, abortable, and list() now exposes it as queued. Consumers of the prompt.queued event/external hook will see an impossible empty queue for that media prompt; compute the count from the same snapshot used by list() or publish before shifting.
ℹ️ 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".
…eueLength startNext shifts the record into launchingItem before publishQueued computes the count, so a media prompt's prompt.queued reported queueLength 0 even though the prompt is accepted, abortable, and listed as queued. Compute the count from the same snapshot list() exposes.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c78cbf88c1
ℹ️ 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 stagingPaths = input.flatMap((item) => item.stagingPaths ?? []); | ||
| const stagingLease = this.staging.create(imageAttachmentIds, stagingPaths, 'user'); | ||
| const currentTurnId = this.streamingUI.getTurnContext().turnId; | ||
| if (currentTurnId !== undefined) this.staging.bindToTurn(stagingLease, currentTurnId); |
There was a problem hiding this comment.
When a Ctrl-S/user steer with staged media races the current turn ending, submitSteer can fall back to launching a new turn when no active prompt remains. This pre-binds the lease to the old UI turn before session.steer(...) resolves, so the new turn's turn.started cannot claim it; the old turn end can delete the daemon upload before intake, or if that turn already ended the lease is never released. Wait for the steer result/next turn.started before binding, or rebind when the steer launches a new turn.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Intake keeps the upload-backed reference when the canonical write fails instead of double-writing into an unowned global cache scope; the session media store's reads collapse to the canonical scope, and non-filesystem deployments no longer write every media blob twice.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2b575d814
ℹ️ 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".
| }; | ||
| return { meta: wire.meta, data: Buffer.from(wire.data, 'base64') }; | ||
| }, | ||
| delete: (fileId) => call('fileService', 'delete', [fileId]) as Promise<void>, |
There was a problem hiding this comment.
When callers delete a stale or expired upload through the new global.files.delete facade, this goes through the generic dispatcher path even though the file-service special cases only normalize save and get. In the memory transport that rejects with the raw engine Error2, while the IPC host serializes the same non-RPCError as 50001, so callers cannot reliably detect file-not-found and the two transports diverge; add the same rethrowFileErrorAsRpc handling for fileService.delete.
AGENTS.md reference: packages/klient/AGENTS.md:L21-L25
Useful? React with 👍 / 👎.
…mon file references The kimi-file:// reference persisted in context memory bundled a durable identity (fileId) with a perishable machine-local absolute path (?path=), which forked sessions and home relocations would stale. The reference now carries only the file id; the display path is derived from the session media store by file id at read time. Parsers tolerate and strip the legacy ?path= query so old records keep resolving.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0ecda4695
ℹ️ 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".
|
|
||
| private async findKey(fileId: string): Promise<string | undefined> { | ||
| const keys = await this.storage.list(this.scope, fileId); | ||
| return keys.find((key) => key === fileId || key.startsWith(`${fileId}.`)); |
There was a problem hiding this comment.
When a media upload is still being materialized on the filesystem backend, atomicWriteStream creates a sibling temp file named like <fileId>.<ext>.tmp.<pid>.<hex>. This fallback matches that temp name because it accepts any key starting with ${fileId}., so an immediate /sessions/{id}/media/{file_id} fetch (or any display-path lookup before metadata is written) can expose or stream the in-progress temp file instead of waiting for the canonical <fileId>.<ext> copy. Filter out the atomic temp suffix or resolve through validated metadata/canonical extensions only.
Useful? React with 👍 / 👎.
…by-id resolution The fs backend stages atomic writes at <key>.tmp.<pid>.<hex> next to the target key, and the media store's prefix-listing predicate matched them, so a lookup racing an unfinished materialize could return the partial copy as the canonical file.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fb96fb3ef
ℹ️ 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 stagingLease = this.staging.create(imageAttachmentIds, stagingPaths, 'user'); | ||
| const currentTurnId = this.streamingUI.getTurnContext().turnId; | ||
| if (currentTurnId !== undefined) this.staging.bindToTurn(stagingLease, currentTurnId); | ||
| this.staging.trackDispatch(stagingLease, session.steer(combineSteerInput(input)), (error) => { |
There was a problem hiding this comment.
When Ctrl-S steers queued media after the paste-time daemon upload is near/over its TTL, this path sends the queued parts directly through combineSteerInput; unlike sendQueuedMessage, it never calls refreshExpiringImageFileRefs, so a queued image can still contain an expired kimi-file:// URL even though the attachment bytes are available for inline fallback. Refresh each item's parts before combining them for session.steer.
Useful? React with 👍 / 👎.
| void Promise.race([handle.launched, handle.completion]).then( | ||
| () => staging?.discard(), | ||
| () => staging?.discard(), | ||
| ); |
There was a problem hiding this comment.
When an uploaded image is compressed, the prompt content references the new owned daemon upload, but prompt intake treats session-media materialization as best effort. If that canonical write is skipped or fails, handle.launched can still resolve before the first LLM request reads media; this cleanup then deletes the only daemon copy, so the resolver has neither the upload nor a session copy and emits an omitted-media placeholder. Keep the owned upload until turn end, or only discard it after materialization is known to have succeeded.
Useful? React with 👍 / 👎.
…spatch Create the staging lease right after extraction so every pre-dispatch exit releases through the tracker: validation/session failures release it, queueing defers it to the queue item's raw ids/paths, and the cache-hint stash takes over ownership. A forgotten exit now degrades to an unclaimed lease swept at session close instead of a permanently retained upload. The cache-hint restore exits (dismiss, chained restore, session switch during fetch, failed compact/new-session) previously returned only the text to the editor, leaking the extraction's retains and staged cache copies. They now go through queue-recall semantics: retains are consumed, staged copies retire, and recalled videos rebase onto them.
…udgeted LRU A memoized inline image part pins a multi-MB base64 string, and the agent state registry's snapshot/inspect path serializes every registered state in full — so the memo no longer lives in agentState. It is now a private per-file-id LRU with the existing 8MB per-entry cap plus a 64MB total budget; eviction simply re-reads the bytes on the next request. The video memo stays in agentState.
…route Prompt intake materializes bytes into the session media store asynchronously and best-effort, but a session_media ref is projected to clients as soon as the prompt is queued — so the download route could 404 during the intake window, and forever after an intake failure. The route now reads the canonical session store first and falls back to the App-scope staged upload, adapting it to the same served shape; only a double miss is a 404. The header note also records that resolving the store resumes cold sessions, an accepted short-term semantic with a TODO for a cold-read channel.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 533df902e4
ℹ️ 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 fileId = attachment?.kind === 'image' ? attachment.fileId : undefined; | ||
| this.byId.delete(id); | ||
| this.stagingUses.delete(id); | ||
| return fileId; |
There was a problem hiding this comment.
When a user reuses a pasted image from an older transcript turn and the new submit pushes that older turn out of the TUI window, trimTranscriptWindow() calls removeMany() while the new extraction has already retained the same attachment id. This path ignores the retain count, removes the attachment, and returns its daemon fileId for deletion, so the prompt still contains a kimi-file://... part but the upload can be deleted before intake; refreshExpiringImageFileRefs() also cannot inline it because the attachment was removed. Keep retained attachments/uploads alive, or drop only thumbnail bytes until stagingUses reaches zero.
Useful? React with 👍 / 👎.
2 participants
Related Issue
No linked issue — the problem is explained below. Opening as a draft for early discussion per CONTRIBUTING.
Problem
Pasted/uploaded images are inlined into prompts as base64 data URLs with no on-disk file. As a result the model cannot re-read an image after the context projector drops old image parts (the "re-read the file" hint is a dead end for pasted images), cannot process it with tools (Bash/Python), and cannot feed it to file-based workflows. Video already has engine-native file references (
kimi-file://+ a resolver); images don't.What changed
Images now follow the same engine-native reference model as video, unified under one media-ref abstraction:
agent-core-v2'smediadomain;packages/kosongitself is untouched): newmediaRef.ts—kimi-file://URL helpers,MediaKindclassification, MIME/extension tables, and<image|video|audio path>tag build/match helpers, with a transcript-side contract copy atpackages/transcript/src/contract/mediaRef.ts.kimiFileUrl.tsand the file-type suffix table become aliases/re-exports.AgentVideoResolverService→AgentMediaResolverService(old names kept as deprecated aliases). Image strategy is inline-only: references resolve to base64 data URLs at LLM request time; when the referenced file is gone, the part is dropped if an adjacent<image path>tag already conveys the path (the model can re-read it with ReadMediaFile), otherwise an[image omitted]notice is emitted. Video semantics are byte-identical.[caption?, <image path> text, image_url kimi-file ref]; compression-altered bytes are saved as a new upload. Transcript projection emits{kind:'file'}without leaking internal URLs. The generic-file flow, theATTACHED_FILE_NOTICEwording, and the REST inline-base64 path are untouched.global.filessave/get/deleteover both memory and ipc transports (base64 wire format).harness.uploadFile()plus re-exported daemon file-reference helpers (changeset:@moonshot-ai/kimi-code-sdkminor).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. (behind the experimental v2 engine; no user-facing doc surface yet)