Repository navigation
Conversation
Reads ~/.claude/projects transcripts read-only and converts them to Maka StoredMessages, so Claude Code conversations import as native Sessions. Claude Code writes one model response as several JSONL records, one per content block, sharing message.id; they fold back into one assistant step. Tool results arrive as `type: 'user'` records and must not import as user messages. Sub-agent transcripts are whole `isSidechain` files and are excluded during cataloging. Each Turn closes with a recorded turn_state: Runtime Ledger repair only trusts a reconstructed terminal RuntimeEvent when one is present, and without it every imported Run is repaired to failed/missing_terminal_event. Fixtures cover both on-disk formats: 2.1 splits a response across records and carries thinking and title records; 2.0 has neither and puts text and tool_use in a single record. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
Astro-Han
left a comment
There was a problem hiding this comment.
The adapter extends the right seam. Registration is three lines, the importer and the persistence path are untouched, and the conversion body is already a single streaming pass. The review below is almost entirely about one decision.
Root cause: the synthesized turn_state is a forged document, and most of the defects here are downstream of it
isTrustworthyRecoveredTerminal exists to stop a live run's header from being contradicted by a half-written transcript. On the import path the header is derived from that same transcript — transcriptRunHeader (runtime-ledger-repair.ts:395) reads turn.status, which deriveTurnRecords supplies for inferred turns as well — so the contradiction the guard protects against is structurally impossible there. The guard fires anyway, and the adapter answers by asserting a recorded terminal state it did not observe.
Four of the findings below are consequences of that one assertion: the fabricated completed at EOF is the forgery's default value; the flag leak is the mutable state accumulated to compute it; a tool-approval rejection recorded as completed is an input that computation misses; the phantom Turn exists so a stray tool_result has something to hang a turn_state on. Removing the synthesis removes closeTurn, turnAborted, turnHadApiError and roughly 60 lines with them.
Suggested minimal shape — two changes that must ship together, net deletion
- Emit
turn_stateonly for facts the transcript actually records: interrupt notice ->aborted,isApiErrorMessage->failed. Drop the EOF close and thecompleteddefault. - Let the Ledger accept an inferred terminal on the transcript-materialization path. The call site is already unique: only
materializeTranscriptRunpassesmodelHistory: 'conversation_text'.
Change 1 alone regresses imported history into a wall of errors; change 2 alone leaves the forgery in place.
Where this should end up
Adapters carry no status vocabulary at all — they emit observations (user, assistant, tool_call, tool_result, system_note{kind:'abort'|'error'}) and inferLegacyTurnStatus derives the status, honestly labelled inferred, which is exactly what "reconstructed from a transcript" means. That also lets the Codex adapter's conditional turn_state go, and closes the matching Codex gap where an unterminated rollout imports as failed. It removes a concept rather than adding an abstraction, but it touches Runtime terminal-state authority — worth its own PR and its own human review rather than this one.
On the shared-module question you raised
Don't extract it here. The duplication is larger than estimated (~145 lines, not ~90), but the 60 lines that are literally identical are generic fs/JSON/sort utilities — extracting them yields a junk-drawer module, not an adapter abstraction. The ~30 structurally-similar lines differ exactly where no decision has been made yet (skip malformed lines vs. throw, what the directory walk collects, whether bounded reads need a chunked probe); merging now would settle those three questions under the cover of deduplication. A third adapter is what makes that boundary knowable.
Two things are worth reusing today, because they are existing seams rather than new abstractions: the '[Request interrupted by user' literal is duplicated from foreign-session.ts:282, and claudeUserText re-implements core's claudeUserMessageText — both decide "what did the user say / what counts as an interrupt", and a drift between the scanner and the importer would be a real bug. Keep the strict session-id charset check as-is: core's isSafeForeignId lets .. through and this id comes from a filename.
Tests
Strong. Killing the sidechain exclusion, the tail-window collectClaudeMeta, the message-id fold, the tool_result branch or the interrupt handling each fails a test, and both fixtures earn their place — v2.0 covers four branches v2.1 cannot reach. Two real gaps:
- The projects-root escape is never asserted. The test named
never reads outside the projects root and rejects unsafe idsonly exercises the id check — deletingstartsWith(root + sep)stays green, and the Codex suite does cover this. - The head-window growth path never executes. Narrowing
[256KB, 4MB]to[256KB]fails nothing, though that path is the one the comment justifies.
If the synthesis goes away, roughly a third of the status-related tests go with it.
Also, outside the inline comments
The "include archived" checkbox (external-session-import-dialog.tsx:255) renders for every source, but this adapter never reads query.includeArchived and never sets archived. Under Claude Code it is a dead control that still triggers a reload through the effect on line 145. Gating it on the adapter — hardcoded at the same level as sourceLabel — is enough for now.
中文
Adapter 扩展的是正确的 seam。注册三行,importer 与持久化零改动,转换本体已是单趟流式。以下评审几乎全部围绕一个决定。
根本原因:合成的 turn_state 是一份伪造文书,本次多数缺陷都是它的下游
isTrustworthyRecoveredTerminal 的存在意义,是防止活跃 run 的 header 被一份写到一半的 transcript 推翻。而在导入路径上,header 恰恰派生自同一份 transcript——transcriptRunHeader(runtime-ledger-repair.ts:395)读的是 turn.status,deriveTurnRecords 对 inferred 的 Turn 同样会给出它——所以这个守卫要防的矛盾在该路径上结构性地不可能发生。守卫照样开火,adapter 便以断言一个自己并未观测到的 recorded 终态作为回应。
以下四条 findings 都是这一次断言的后果:EOF 处伪造的 completed 是伪造品的默认值;标志泄漏是为算出该值而攒的可变状态;工具审批被拒却记为 completed 是该计算漏掉的一个输入;幽灵 Turn 的存在是为了让游离的 tool_result 有个地方挂载 turn_state。移除合成,closeTurn、turnAborted、turnHadApiError 连同约 60 行一并移除。
建议的最小形态——两处改动必须同批,且净删代码
- 只为 transcript 真正记录下来的事实发
turn_state:中断通知 ->aborted,isApiErrorMessage->failed。去掉 EOF 收尾与completed默认分支。 - 让 Ledger 在 transcript 物化路径上接受 inferred 终态。调用点本就唯一:只有
materializeTranscriptRun传modelHistory: 'conversation_text'。
只做 1 会让导入的历史回归成一片错误;只做 2 则伪造品仍在。
终局应该是
Adapter 完全不持有状态词汇,只发出观测(user、assistant、tool_call、tool_result、system_note{kind:'abort'|'error'}),由 inferLegacyTurnStatus 推导状态并诚实标记为 inferred——这正是「从 transcript 重建而来」的准确含义。这同时让 Codex adapter 的条件式 turn_state 得以删除,并补上 Codex 那条对应缺口:未闭合的 rollout 现在会导入成 failed。它移除一个概念而非新增抽象,但触及 Runtime 的终态权威,值得单独一个 PR 与单独的人工评审,不宜并入本 PR。
关于你提出的共享模块问题
本 PR 不要抽。重复量比估计的大(约 145 行,而非 90),但其中字面完全相同的 60 行全是通用 fs/JSON/排序工具,抽出来得到的是杂物袋,不是 adapter 抽象。另外约 30 行结构相似的代码,差异点恰好落在尚未做出决定的地方(坏行跳过还是抛错、目录遍历采集什么、有界读取是否需要分块探针),此时合并等于借去重之名替这三个问题定调。第三个 adapter 才会让这个边界变得可知。
有两处今天就该复用,因为它们是既有 seam 而非新抽象:'[Request interrupted by user' 字面量与 foreign-session.ts:282 重复,claudeUserText 重新实现了 core 的 claudeUserMessageText——两者都在裁定「用户说了什么/什么算中断」,scanner 与 importer 在此漂移会是真实的 bug。严格的 session-id 字符集校验请保持原样:core 的 isSafeForeignId 会放过 ..,而这个 id 来自文件名。
测试
很扎实。删掉 sidechain 排除、tail 窗口的 collectClaudeMeta、message-id 折叠、tool_result 分支或中断处理,每一个都会让测试变红;两个 fixture 都称职——v2.0 覆盖了 v2.1 到不了的四条分支。两处真实缺口:
- projects 根目录逃逸从未被断言。名为
never reads outside the projects root and rejects unsafe ids的测试只跑了 id 校验,删掉startsWith(root + sep)依旧全绿,而 Codex 测试是覆盖了的。 - 头窗口增长路径从未执行。把
[256KB, 4MB]收窄为[256KB]不会失败,而这条路径正是注释所论证的场景。
如果合成被移除,状态相关的测试约有三分之一会随之消失。
另外,不在行内评论里的一条
「包含已归档会话」勾选框(external-session-import-dialog.tsx:255)对所有来源都渲染,但本 adapter 从不读 query.includeArchived,也从不设置 archived。在 Claude Code 来源下它是个死控件,勾选还会通过第 145 行的 effect 触发一次重新加载。目前只需按 adapter 做门控即可——与 sourceLabel 已有的硬编码同层级。
| * that, so the status is recorded rather than inferred. | ||
| */ | ||
| const closeTurn = (line: number): void => { | ||
| if (activeTurnId === undefined) return; |
There was a problem hiding this comment.
P1 — closeTurn returns here before the resets on 311-312, so turnAborted / turnHadApiError survive and are consumed by the next Turn, which then renders as cancelled or failed despite completing normally.
Reachable whenever an interrupt notice or an isApiErrorMessage record arrives with no open Turn: 2 of 376 real root transcripts under ~/.claude/projects have exactly that shape (interrupt with no open Turn, followed by a genuine prompt).
Moving both resets above the early return fixes it, though the cleaner outcome is that this state disappears entirely along with the synthesis.
| */ | ||
| const closeTurn = (line: number): void => { | ||
| if (activeTurnId === undefined) return; | ||
| const status = turnAborted ? 'aborted' : turnHadApiError ? 'failed' : 'completed'; |
There was a problem hiding this comment.
P1 — the completed default asserts a fact the transcript does not carry. Combined with the unconditional close on 462, any transcript whose last user prompt has no assistant record after it — killed process, crash, or a session that is still running — imports as a successfully completed Turn.
50 of 376 real root transcripts hit this, and importing a live session hits it every time. Because it is written as a recorded terminal, isTrustworthyRecoveredTerminal believes it.
Suggest emitting nothing here and letting inferLegacyTurnStatus label the Turn inferred.
|
|
||
| const blocks = contentBlocks(message); | ||
| const toolResults = blocks.filter((block) => stringField(block, 'type') === 'tool_result'); | ||
| if (toolResults.length > 0) { |
There was a problem hiding this comment.
P2 — taking continue as soon as a record carries a tool_result means neither is_error nor the sibling text block is read, so the interrupt check on 444 is unreachable for this shape. A Turn the user cancelled at the tool-approval prompt imports as completed.
Real data: 52 rejection-shaped tool results, 3 of them with no separate interrupt notice afterwards.
Mapping this to system_note{kind:'abort'} lets core infer aborted without the adapter asserting anything.
| messages.push({ | ||
| type: 'tool_result', | ||
| id: uniqueId(recordId(value, sessionId, 'tool-result', record.line)), | ||
| turnId: ensureTurnId(record.line), |
There was a problem hiding this comment.
P3 — ensureTurnId here opens a Turn holding only a tool_result, with a toolUseId pointing into a different Turn's tool_call. Two real root transcripts begin with a tool_result before any prompt.
Impact is limited — materializeTranscriptLedger:76 only materializes Turns containing a user message, so this never becomes a Run — but it renders as a stray row. Attaching to the last known Turn, or dropping it when there is none, is more faithful.
|
|
||
| const messageId = | ||
| stringField(message, 'id') ?? generatedClaudeId(sessionId, 'assistant', record.line); | ||
| if (step && step.messageId !== messageId) flushStep(); |
There was a problem hiding this comment.
P3 (count unverified) — the fold only merges consecutive records, and 397 flushes on any non-assistant record, so a response whose parallel tool_use blocks are separated by a tool_result splits into msg_X and msg_X-2.
An automated scan of a local store reported this shape ~8.5k times; worth confirming before acting on the number. No data is lost and the order is right, but it contradicts the class comment's promise that a single answer does not import as several rows. Keying the fold on message.id and flushing at Turn boundaries would hold the promise.
| * Read at most `maxBytes` from one end of the file. Every read of the external | ||
| * store is bounded: it is not ours to trust with our memory. | ||
| */ | ||
| async function readUtf8Window( |
There was a problem hiding this comment.
P3 — a single handle.read for up to 64 MB, with no loop until bytesRead === 0. A short read truncates the transcript, the resulting half-line is swallowed by the silent malformed-line skip in parseClaudeRecords, and the import still reports success.
readWholeTranscript also stats twice, so a file that grows past the bound between the two stats is silently truncated rather than refused. The Codex equivalent (codex-session-adapter.ts:628) loops and probes past the bound.
| } | ||
|
|
||
| private async listCatalog(query: ExternalSessionQuery): Promise<ClaudeCatalogEntry[]> { | ||
| const candidates = (await walkTranscriptFiles(this.projectsRoot)).sort( |
There was a problem hiding this comment.
Cleanup: this mtime sort is dead — 125 re-sorts the whole list by updatedAt, with no early exit or limit in between.
| for (const candidate of await walkTranscriptFiles(this.projectsRoot)) { | ||
| if (basename(candidate.path) !== `${sessionId}.jsonl`) continue; | ||
| const entry = await this.catalogEntry(candidate); | ||
| if (entry?.id === sessionId) return entry; |
There was a problem hiding this comment.
Cleanup: entry?.id === sessionId is always true. 130 already asserts the basename, and catalogEntry derives id from that same basename on 140.
| const meta: ClaudeTranscriptMeta = {}; | ||
| const titles: ClaudeTitleCandidates = {}; | ||
| const usedIds = new Set<string>(); | ||
| const emittedToolCallIds = new Set<string>(); |
There was a problem hiding this comment.
Cleanup: emittedToolCallIds overlaps usedIds semantically and forces the manual usedIds.add(tool.id) on 339. None of the 17 tests cover it. Either add a fixture showing a duplicate tool_use id really occurs, or drop the set and check usedIds.
| function sourceLabel(adapterId: string, codexLabel: string): string { | ||
| return adapterId === 'codex' ? codexLabel : adapterId; | ||
| function sourceLabel(adapterId: string, copy: ExternalSessionImportCopy): string { | ||
| if (adapterId === 'codex') return copy.codex; |
There was a problem hiding this comment.
Cleanup: this if-chain means a third adapter has to edit this file again. An adapter-supplied display name, or an id -> label lookup on the copy side, keeps it out of the component.
Addresses the review on apache#2954. Two changes that only work together. **The adapter no longer asserts a status it did not observe.** It emitted a `recorded` terminal `turn_state` for every Turn, defaulting to `completed`, because `isTrustworthyRecoveredTerminal` discards a reconstructed terminal without one and every imported Run was then repaired to `failed`/`missing_terminal_event`. The default was a forgery: 50 of 376 real root transcripts end with a prompt that has no answer after it — a killed process, a crash, or a Session still running — and every one of them imported as a Turn that finished normally. Now only two facts produce a `turn_state`, because only two are written down: Claude Code's interrupt notice, and a record it flagged `isApiErrorMessage`. Everything else is derived by `deriveTurnRecords` and labelled `inferred`, which is what "reconstructed from a transcript" means. `closeTurn`, `turnAborted` and `turnHadApiError` go with it — including the leak where an interrupt arriving with no open Turn left its flag set for the *next* Turn, which then rendered as cancelled despite completing. **Runtime accepts an inferred terminal on the transcript path.** The guard exists so a live Run's header is not contradicted by a half-written transcript. On the materialization path there is no such header: `transcriptRunHeader` builds it from `deriveTurnRecords` over the same messages. Requiring `recorded` there only pushed the pressure onto importers, which answered by inventing one. Both the trust guard and `terminalStatus` in the backfill now take the transcript path explicitly. Also from the review: - A tool call the user declined imports as `system_note{kind:'abort'}`, so the Turn infers `aborted`. Matched on Claude Code's rejection sentence, not on `is_error` — an ordinary tool failure is an errored result too. - A `tool_result` arriving before any Turn is dropped rather than opening one to hold it; its `toolUseId` points into a conversation not in the file. - The assistant fold keys on `message.id` across the Turn. A response whose parallel `tool_use` blocks are split by a `tool_result` was importing as two rows: 383 occurrences across 33 of 1236 local transcripts, 378 of them carrying only `tool_use`. - Both bounded reads loop until the buffer is filled, and the whole-file read checks its bound against the handle it reads from rather than a separate `stat`. - `claudeUserText` and the interrupt literal come from core, so the scanner and the importer cannot drift on what the user said. - The archive checkbox is gated per source; under Claude Code it only re-ran the catalog load. - Dead mtime sort, always-true id check, and `emittedToolCallIds` removed. - Tests: the projects-root escape and the head-window growth are asserted, each mutation-checked; the escape needs both the symlink-blind walk and the root-prefix check removed before it fails, which is what those two layers are. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
The synthesized You said the two changes have to ship together and I did both. The second one needed one more layer than described: after Other findings:
The two test gaps are covered and mutation-checked. One result is worth stating: deleting Not done: the shared module stays unextracted, per your call. The end state where adapters carry no status vocabulary is not here either — you said it deserves its own PR and its own human review, and I agree; it also has to change the Codex adapter and close the unterminated-rollout gap. The Net +542/-75. Build, lint, format and the full 6452-test suite pass locally (5 ripgrep failures reproduce on a clean tree). Note on how this was pushed: the change was cherry-picked onto the remote branch tip as a fast-forward rather than force-pushed — my local copy had been rebased, and pushing it would have rewritten the history your inline comments are anchored to. |
|
Both red lanes on this PR are timing flakes in areas the branch does not touch. This commit changes 7 files: the storage adapter and its test,
The first drives a I do not have rerun rights. Could you trigger one — or, if you would rather, I can open a follow-up issue for the |
Addresses the remaining cleanup on apache#2954. `sourceLabel` and `sourceHasArchive` were separate id chains in the component, so a third adapter had to find both. One record keyed by adapter id answers each question, and an unknown source falls back to its own id with no archive rather than a wrong assumption. The label stays on the copy side so it remains localized; `hasArchive` is a capability whose eventual home is the adapter contract, which is an IPC change worth making when a third source lands rather than inside this PR. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
I kept the label on the copy side, which is the second of the two options you offered — the first, an adapter-supplied display name, would have to give up localization. This also re-triggers CI, which was red twice on flakes in files this branch does not touch (details in the comment above). |
Summary
Adds the Claude Code adapter for the external-session import path, so
~/.claude/projectsconversations import as native Maka Sessions. Nothingoutside the adapter learns the format: the importer, the persistence path and
the wire protocol are untouched, and registering it is one line.
Three things about the format drove the design:
record per content block (thinking, text, tool_use) sharing
message.id;they fold back into one assistant step, or a single answer imports as three
separate rows.
type: 'user'records. Taking them at face valuewould turn every tool output into a user message.
isSidechainfiles, not recordsinterleaved into their parent, so they are excluded while cataloging.
Each Turn closes with a recorded
turn_state. Without one, Runtime Ledgerrepair does not trust the reconstructed terminal RuntimeEvent
(
isTrustworthyRecoveredTerminal) and repairs every imported Run tofailed/missing_terminal_event— an imported conversation then renders asa wall of errors. This is the one place the adapter must assert a fact rather
than copy one; the transcript is the record that the Turn ended.
Refs #2499 (the PR 4 stage).
Verification
17 adapter tests. Fixtures cover both on-disk formats: 2.1 splits a response
across records and carries thinking and title records; 2.0 has neither and
puts text and tool_use in a single record, so its title falls back to the
opening request.
Against a real
~/.claude/projects(1154 conversations, Claude Code 2.0.77through 2.1.220):
decodeStoredMessagelint,format:check,build,typecheck,knip(desktop + ui) andastryx:themepass.@maka/storage786/786,@maka/desktop787/787,@maka/runtime-host896/896.Also imported and read back a 1790-message conversation end to end through
ExternalSessionImporter+SessionAuthorityStorein a local packaged build.Review focus
The adapter is self-contained, which means ~90 lines of it (bounded reads,
directory walk, path containment, the small JSON helpers) are logically the
same as
codex-session-adapter.ts. Extracting a shared module would removethat duplication from both, at the cost of touching the Codex adapter in this
PR. Happy to do it here or as a follow-up — say which you prefer.
Checklist
Does this PR entail a change in behavior?