Skip to content

feat(storage): scan OpenCode sessions in the foreign-session store - #5125

Closed
ggbdpq wants to merge 9 commits into
apache:mainfrom
ggbdpq:feat/opencode-session-scan
Closed

ggbdpq wants to merge 9 commits into
apache:mainfrom
ggbdpq:feat/opencode-session-scan

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5053. The foreign-session catalog (CLI resume picker) listed Claude Code and Codex sessions but never OpenCode, even though the import adapter (#3403) already reads the same database.

  • FOREIGN_SESSION_SOURCES gains opencode; the scan reuses OpenCodeSessionAdapter for every database read — schema introspection, parent/child session rules, archived filtering, and transcript conversion all live there, so the store adds only the feat(cli): resume sessions from Claude Code, Codex, and Cursor #1057 scan bounds: MAKA_IMPORT_OPENCODE=0 disables the source (same cloak-flag convention as the other two), 50-session / 30-day caps, isSafeForeignId gating at the store boundary.
  • The digest reads through adapter.readSession (read-only SQLite connection) and keeps only user text, assistant text, and file paths from tool-call inputs (file_path / path / notebook_path, the same key set the Claude digest uses). Thinking-only rows carry text: '' and never enter the digest; tool output and system prompts never cross the boundary — the existing sanitize/redact/accumulator pipeline is shared unchanged.
  • foreignSourceLabel renders the new source as "OpenCode" on the resume surface.
  • Non-goals honored: no writes to the OpenCode database (the adapter opens it readOnly), no Cursor/other sources, import behavior (feat(storage): import opencode sessions through ExternalSessionAdapter #3403) untouched.

Verification

Claim Evidence
Flag + presence availableSources() reports opencode only when enabled AND opencode.db exists; MAKA_IMPORT_OPENCODE=0 removes it
Scan bounds new tests: archived and child sessions are dropped, cwd filter reaches the database, 50-cap inherited from the shared choke point
Digest contract new test: user text, assistant text, and tool-touched file paths enter the digest; reasoning/thinking text provably absent
Regression foreign-session-store + opencode adapter suites 45/46 — the one failure is a pre-existing Windows symlink-permission (EPERM) fixture, unrelated
Builds / format / headers core + storage builds exit 0; biome clean; ASF headers intact

AI use

Implemented with ZCode (GLM-5.3-Flash): reused the existing adapter as the single database authority per the issue's non-goals, followed the #1057 safety contract for the digest, and carried the Generated-by trailer.

Checklist

  • Detects ~/.local/share/opencode/opencode.db, disable flag honored
  • Digest carries only user text, assistant text, file paths; bounded reads, read-only
  • Child sessions and archived sessions stay out
  • No writes to the OpenCode database
  • Live check against a real OpenCode install was not run locally (fixtures pin the schema captured from 1.18.21, matching the adapter's verification note)

The foreign-session catalog listed Claude Code and Codex sessions but
never OpenCode: `FOREIGN_SESSION_SOURCES` had no `opencode` entry, so
conversations recorded in `~/.local/share/opencode/opencode.db` never
appeared for a handoff resume, even though the import adapter (apache#3403)
already reads that database.

The scan reuses `OpenCodeSessionAdapter` for every database read
(schema introspection, parent/child rules, transcript conversion) and
layers the apache#1057 bounds on top: `MAKA_IMPORT_OPENCODE=0` disables the
source (cloak-flag convention), results cap at 50 sessions / 30 days,
archived and child sessions stay out, and the digest keeps only user
text, assistant text, and tool-touched file paths — thinking blocks,
tool output, and system prompts never cross the boundary. Reads stay
read-only through the adapter's readOnly connection.

`foreignSourceLabel` renders the new source as "OpenCode" in the
resume surface.

Fixes apache#5053
Generated-by: GLM-5.3-Flash (ZCode)
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 9, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing OpenCodeSessionAdapter as the single database authority instead of opening a second read path is the right shape, and the privacy contract holds where it matters: reasoning parts convert to text: '' with a separate thinking field, so message.text on an assistant row cannot carry reasoning; tool_result rows (including state.output / state.error) never enter the digest; non-user/assistant roles are dropped during conversion; the adapter really does open readOnly and this path only ever prepare().all()s. The 50-cap and recency order are also fine — the adapter sorts by updatedAt descending internally, so the kept 50 are the newest 50 (measured: 3000 sessions → 50 rows, 13ms, 4.5 MB).

The problem is one level up. The adapter was adopted as an interface but not as a contract: neither its data shape nor its failure behaviour was checked against what the store assumes. That shows up twice, and it is why they belong together here rather than as two line comments.

P1 — The adapter was reused without matching its data shape or its failure contract

Reachability ① for the first half, ② for the second.

The tool-argument key set is Claude Code's, not OpenCode's. ['file_path', 'path', 'notebook_path'] is copied from the Claude digest. OpenCode has never used those names: at 1.18 — the version this PR's fixtures pin — read / write / edit took filePath (236cfcbbc3:packages/opencode/src/tool/edit.ts:48); current OpenCode renamed it to path; notebook_path does not exist anywhere in OpenCode. Meanwhile path is a key on glob and grep, where it is an optional relative search directory (RelativePath, glob.ts:19, grep.ts:23).

So on a real database, every file actually read or edited is dropped, and the only thing that can reach filesTouched is a grep's search directory — a relative path, where Claude and Codex digests carry absolute file paths. Measured on a database built with real OpenCode keys, through createForeignSessionStore: filesTouched came back as ["src"] while /repo/src/a.ts and /repo/src/b.ts, both touched by edit/read, were absent.

The test that should have caught this uses input: { file_path: '/repo/src/parser.ts' } — a fixture written to match the implementation rather than captured from a real database. That is what makes "no live OpenCode install was tested" more than a checklist gap: the suite currently proves the code can read its own invented format.

Fix: take the union across versions (filePath / path / file_path) and gate on toolName, which is already present on the tool_call row — only read / write / edit / patch name a single file; glob / grep / list take a directory and must not contribute to filesTouched.

An unreadable OpenCode database takes the whole catalog down with it. The adapter deliberately throws rather than swallowing read errors — opencode-session-adapter.ts:131-139 says so explicitly, to distinguish "database unreadable" from "no sessions". That is correct for a user-initiated import and wrong for a directory scan. listOpencodeSessions doesn't catch, FileForeignSessionStore.listSessions pushes all three sources into one array so any throw rejects the whole call, and the CLI falls to its foreignScanFailed branch with foreignByValue empty — the Claude Code and Codex rows disappear too. Reproduced: one valid Claude Code transcript plus an OpenCode database whose session table is missing the directory column, and the picker showed nothing but an error.

This needs no exotic setup — an OpenCode schema change, a locked database, or a corrupt file all hit one of the adapter's three throws. #5053 asked for one more source in the list; as written, one more source can empty the list.

Fix: catch inside listOpencodeSessions, return [], and surface the reason as a per-source notice. If the diagnostic is worth keeping, make the store return per-source errors — one source's failure must not be the catalog's failure.

P2 — The digest inherits the scan bounds but not the read bound

Reachability ①.

The file-backed digest caps at FOREIGN_SESSION_DIGEST_MAX_READ_BYTES (2 MB) and pushes a truncation warning (foreign-session-store.ts:401-404). readOpencodeDigestadapter.readSession#readTranscript reads every message row and every part row for the session and JSON.parses each, and part data contains state.output — full tool output. No byte bound at all.

Measured: a session carrying ~120 MB of tool output took 255 ms and 236 MB of heap to produce a 166-byte digest, with warnings: [] — the user is told nothing. DatabaseSync is synchronous, so those 255 ms block the TUI event loop, where the file path uses fs/promises.

The bound cannot be added in the store, because the adapter hands back fully materialized arrays. It has to go inside #readTranscript: an optional byte budget, accumulated over data, stopping at FOREIGN_SESSION_DIGEST_MAX_READ_BYTES and returning a truncation marker the store turns into the same warning wording the file path uses. That is the only shape that keeps "the adapter owns every read" and the #1057 bound at the same time.

The comment at foreign-session-store.ts:462-465 also needs to say this: it currently explains only that file-confinement doesn't apply, which reads as if everything else was inherited.

P2 — No test exercises any bound

Reachability: evidence.

The five new cases use 1–3 sessions and 0–4 parts. Nothing touches the 50-session cap, the 30-day window, or any byte boundary; both measurements above needed databases I had to build. Combined with the invented file_path fixture, what the suite currently pins is the implementation's own assumptions.

Worth adding, in rough order of value: a filesTouched case using real OpenCode keys (filePath and path, plus a grep whose directory must not appear); a 51-session case pinning recency order and the cap; a large-part case pinning the truncation warning; and a schema-drift case proving the other two sources survive.

Credit where due: the tests do go through createForeignSessionStoreavailableSources / listSessions / readDigest rather than poking private methods. The seam is right; only the inputs are too small.

中文

OpenCodeSessionAdapter 当作唯一的数据库读取权威来复用、而不是另开一条读取路径,形状是对的;隐私契约在要紧处也成立:reasoning part 转换后是 text: '' 外加独立的 thinking 字段,assistant 行的 message.text 不可能携带 reasoning;tool_result(含 state.output/state.error)从不进 digest;非 user/assistant 角色在转换阶段就被丢弃;adapter 确实以 readOnly 打开,这条路径上只有 prepare().all()。50 条上限和最新优先也没问题——adapter 内部按 updatedAt 降序排过,留下的就是最新 50 条(实测 3000 session → 50 行、13ms、4.5MB)。

问题在上一层:adapter 被当作接口采纳了,但没有被当作契约——它的数据形状和失败行为都没有和 store 的假设对齐过。这一点出现了两次,所以放在一起说,而不是拆成两条行内评论。

P1 — 复用 adapter 时既没对齐数据形状,也没对齐失败契约(前半可达①,后半②)

工具参数的 key 集合是 Claude Code 的,不是 OpenCode 的。 ['file_path', 'path', 'notebook_path'] 抄自 Claude digest。OpenCode 从来没用过这些名字:1.18(本 PR fixture 固定的版本)里 read/write/edit 用的是 filePath236cfcbbc3:packages/opencode/src/tool/edit.ts:48);当前版本改名为 pathnotebook_path 在 OpenCode 全仓不存在。而 path 恰恰是 glob/grep 的 key,在那里是可选的相对搜索目录RelativePathglob.ts:19grep.ts:23)。

于是在真实数据库上,所有被真正读过或改过的文件全部丢失,唯一能进 filesTouched 的是 grep 的搜索目录——一个相对路径,而 Claude 和 Codex 的 digest 里是绝对文件路径。用真实 key 构造数据库、走 createForeignSessionStore 生产入口实测:filesTouched 返回 ["src"],被 edit/read 碰过的 /repo/src/a.ts/repo/src/b.ts 都不在。

本该拦住它的测试用的是 input: { file_path: '/repo/src/parser.ts' }——这个夹具是照着实现写的,不是从真实库里抓的。这让"没在真实 OpenCode 安装上验证"不只是清单上的一个空格:这套测试目前证明的是代码能读自己发明的格式。

修法:key 取跨版本并集(filePath/path/file_path),并按 toolName 门控——tool_call 行上本来就有——只有 read/write/edit/patch 指向单个文件;glob/grep/list 的是目录,不该进 filesTouched

一个读不了的 OpenCode 库会把整个 catalog 一起带走。 adapter 刻意抛错而不吞错,opencode-session-adapter.ts:131-139 的注释明说了原因:要区分"库读不了"和"没有 session"。这对用户主动发起的导入是对的,对目录扫描是错的。listOpencodeSessions 不捕获,FileForeignSessionStore.listSessions 把三个源 push 进同一个数组,任一抛出整体 reject,CLI 落到 foreignScanFailed 分支、foreignByValue 为空——Claude Code 和 Codex 的行也一起消失。已复现:一条有效的 Claude Code transcript,加一个 session 表缺 directory 列的 OpenCode 库,picker 只剩一条报错。

触发不需要特殊构造:OpenCode 改表结构、库被锁、库损坏,都会命中 adapter 里那三处 throw#5053 要的是列表里多一个源;照现在的写法,多一个源可能让列表一个都不剩。

修法:在 listOpencodeSessions 内部捕获,返回 [],把原因降级成该源的一条提示。要保留诊断信息就让 store 返回 per-source 的错误——一个源的失败不能等于整个 catalog 的失败。

P2 — digest 继承了扫描上限,没继承读取上限(可达①)

文件路径的 digest 有 2MB 上限并推一条截断 warning(foreign-session-store.ts:401-404)。readOpencodeDigestadapter.readSession#readTranscript 把该 session 的全部 message 行和 part 行读出并逐个 JSON.parse,而 part 的 data 里含 state.output,即完整工具输出。一个字节上限都没有。

实测:一个含约 120MB 工具输出的 session,产出 166 字节的 digest 用了 255ms、236MB 堆warnings: []——用户什么都不知道。DatabaseSync 是同步 API,这 255ms 阻塞 TUI 事件循环,而文件路径用的是 fs/promises

这个界不能加在 store 层,因为 adapter 返回的是已完全物化的数组。它必须落在 #readTranscript 里:一个可选的 byte budget,在累加 data 时超过 FOREIGN_SESSION_DIGEST_MAX_READ_BYTES 就停止并回传截断标记,由 store 转成与文件路径同样措辞的 warning。这是唯一能同时保住"adapter 是唯一读权威"和 #1057 边界的形状。

foreign-session-store.ts:462-465 的注释也要一并说清:它现在只解释了 file-confinement 不适用,读起来像是别的都继承了。

P2 — 没有任何测试触及边界(证据)

五个新用例只喂 1-3 个 session、0-4 个 part,没有一个触及 50 条上限、30 天窗口或任何字节边界;上面两组测量都得自己造库。加上编造的 file_path 夹具,这套测试目前钉住的是实现自己的假设。

建议补(按价值排序):一个用真实 OpenCode key 的 filesTouched 用例(filePathpath,外加一个 grep,其目录必须出现);一个 51 session 的排序/截断用例;一个大 part 的截断 warning 用例;一个 schema 漂移时另两个源仍然存活的用例。

该说的也说:测试确实走 createForeignSessionStoreavailableSources/listSessions/readDigest,没碰私有方法。接缝是对的,只是输入太小。

Comment thread packages/storage/src/foreign-session-store.ts Outdated
Comment thread packages/storage/src/foreign-session-store.ts
Comment thread packages/storage/src/foreign-session-store.ts Outdated
Comment thread packages/storage/src/foreign-session-store.ts
Three review findings on the OpenCode scan (apache#5125):

- filesTouched used Claude Code's argument keys. OpenCode names its
  file argument `filePath` (1.18) / `path` (newer), and `path` also
  appears on `glob`/`grep` as an optional search *directory* — so on a
  real database every touched file was dropped and only a grep
  directory could land in the digest. Keys now take the cross-version
  union, gated on toolName so only the single-file tools contribute.
- An unreadable OpenCode database took the whole catalog down: the
  adapter's deliberate throw (right for a user-initiated import)
  propagated through listSessions and emptied every source.
  listOpencodeSessions now degrades to an empty opencode row set while
  Claude Code and Codex keep listing.
- The digest inherited the scan bounds but not the read bound: a
  ~120 MB session cost 255 ms and 236 MB of heap on the synchronous
  SQLite caller to produce a 166-byte digest, silently. The adapter
  gains readSessionBounded, which stops reading at a byte budget and
  reports the truncation; the store turns that into the same warning
  wording the file path uses.

Tests now use real OpenCode argument keys with a grep exclusion case,
a 50-cap recency case, a truncation-warning case, and a schema-drift
case proving the other sources survive.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

All findings addressed at 14aa56bb3:

P1 — real OpenCode keys, gated by toolName. filesTouched now takes the cross-version key union (filePath / path / file_path) and only from the single-file tools (read / write / edit / patch); glob / grep / list never contribute, so a grep's relative search directory can no longer masquerade as a touched file. The invented file_path fixture is replaced with real shapes: an edit with filePath, a read with path, and a grep whose path: 'src' is asserted absent from the digest.

P1 — one broken database no longer empties the catalog. listOpencodeSessions catches the adapter's throw and degrades to an empty opencode row set; Claude Code and Codex keep listing. A schema-drift regression seeds a session table missing directory next to a valid Claude transcript and asserts the other source survives.

P2 — the read bound moved into the adapter. #readTranscript now accepts a byte budget, stops accumulating raw row payload at the cap, and reports the cut; the store exposes it as readSessionBounded and turns the marker into the same warning wording the file path uses. A bounded-digest test seeds a ~2.6 MB tool output and asserts the truncation warning fires while the prompt read before the cut survives.

P2 — the missing bound tests. Added: the 50-cap recency case (55 seeded, 50 kept, newest first), the truncation case above, and the schema-drift case above.

Verification: OpenCode suite 9/9; foreign-session + adapter suites 53/54 with the one failure being the pre-existing Windows symlink-permission fixture; storage build clean.


中文:四条全部处理(14aa56bb3)——filesTouched 改用 OpenCode 真实键集(filePath/path 跨版本并集)并按 toolName 门控(glob/grep/list 的目录不算文件),测试换真实形状 + grep 排除断言;坏库容错(listOpencodeSessions 捕获降级,其他源照常),schema-drift 回归钉住;读取上限下沉到 adapter(#readTranscript 字节预算 + 截断标记),store 转成与文件路径同措辞的 warning;补齐 50-cap、截断、schema-drift 三个边界用例。验证:OpenCode 9/9。

@github-actions github-actions Bot added effort/L Under 1000 readable lines and removed effort/M Under 500 readable lines labels Sep 12, 2026
@ggbdpq

ggbdpq commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Pushed d71c175: closes the four open findings from the 09-11 review. The adapter is now the single authority — opencodeDatabasePath() and isUsableOpencodeSessionId() are exported from it and used for the source probe (detect()), the scan filter, and the digest pre-check, so the database filename lives in exactly one place and the store's id gate mirrors the adapter's own pattern instead of a wider second one. The misplaced Codex banner moved over the Codex functions and OpenCode got its own; the shared-database transcriptPath now records in place that OpenCode keeps transcripts in the store rather than per-session files.

…s and ids

Review follow-up on apache#5125. The store no longer re-derives the database
location or a second, wider id gate: `opencodeDatabasePath()` and
`isUsableOpencodeSessionId()` are exported from the adapter and used for
the source probe, the scan filter, and the digest pre-check, so the
filename and the id contract each live in exactly one place. The
misplaced Codex section banner now sits over the Codex functions and
OpenCode has its own. The shared-database `transcriptPath` carries a
comment recording that OpenCode keeps transcripts in the store, not in
per-session files.

Generated-by: GLM-5.3-Flash (ZCode)

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the file-argument and per-source failure cases. I reviewed d71c17508af2226682efadf1c060f5d89835221b in order: problem, solution, architecture, implementation, then simplification. The #5053 gap is real, and extending the existing OpenCode adapter is the right direction. However, its full-import projection is not yet a safe digest contract.

P1 — Preserve provenance until the digest has excluded synthetic instructions and resource output. Reachability: normal resume/import flow crossing the external-content boundary. OpenCode v1.18.21 writes an automatic task-summary instruction as a user TextPart with synthetic: true (writer); it also stores text returned by MCP readResource in synthetic user parts (writer).

convertTranscript at opencode-session-adapter.ts:415-419 joins every user text part and discards that marker. The new bounded method returns this full-import-shaped result; readOpencodeDigest then treats it as human text. TUI's importForeignSession puts the digest into the model-facing handoff. An exact-head SQLite probe using these writer shapes returned both Summarize the task tool output above and continue with your task. and the resource-result text in digest.userMessages, with no warning. This violates #5053's exclusion of tool output and system instructions; resource contents can cross that boundary even though explicit tool_result messages are skipped.

Please apply the digest exclusion while the adapter still has raw part provenance, before convertTranscript erases synthetic. Keep full-session import behavior separate from this restricted projection, while retaining the same database reader/authority. A real synthetic-user-part regression through createForeignSessionStore().readDigest() should exclude these parts and keep ordinary human text. No second database implementation is needed.

P2 — The read cap still does not bound payload materialization, detailed inline. The final small digest and a truncation warning do not prove that the read was bounded.

For simplification, remove the unused path helper/imports noted inline. The existing transcriptPath contract comment also still needs the earlier correction: OpenCode summaries reference one shared database, not a per-session JSONL file.

Validation: scoped core/storage builds and the two affected compiled suites passed (50/50). Independent exact-head SQLite probes exposed the synthetic-part and byte-budget gaps; I cross-checked the upstream writer, adapter conversion, and TUI handoff consumer. Current CI passes. No real installed OpenCode/TUI manual acceptance was performed. AI-assisted review with Codex and an independent deep reviewer.

Comment thread packages/storage/src/opencode-session-adapter.ts Outdated
Comment thread packages/storage/src/opencode-session-adapter.ts Outdated
…zed strings

Astro-Han's two findings on apache#4815's sibling (apache#5125 review):

- The bounded read counted each payload with JS `data.length`, which is
  UTF-16 code units: a 3,600,025-byte CJK payload measured 1,200,025
  units and never tripped the 2 MiB budget. Sizes now come from SQLite
  (`length(CAST(data AS BLOB))`), which is the UTF-8 byte count the
  budget is written in.
- Both transcript queries `.all()`ed every row before the budget loop,
  so each oversized payload was fully materialized just to be dropped.
  The bounded path now sizes first and fetches only the fitting prefix
  (LIMIT), one cumulative budget across messages then parts, and an
  oversized single row is never fetched at all.

Also drops the caller-less `opencodeDatabasePath` helper with the
store's unused imports of it and of `existsSync` — the adapter's
`detect()`/`databasePath()` remain the only path authority.

Regressions: an adapter-level bound test with a CJK payload that fits
the budget in UTF-16 units but exceeds it in UTF-8 bytes (red on the
old counting, green now, with the oversized row and everything behind
it absent from the result), and a store-level CJK sibling of the
truncation-warning test.

Generated-by: GLM-5.3-Flash (ZCode)

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the UTF-8 byte budget and oversized-row reads. I rechecked 751dc866f; that P2 is addressed.

The earlier provenance P1 remains, though. An exact-head SQLite probe still includes synthetic summary instructions and MCP resource output in digest.userMessages, alongside the human request.

Could you exclude synthetic user parts in the adapter’s restricted digest projection, before conversion loses their provenance? Full-session import can retain its existing behavior. A regression through createForeignSessionStore().readDigest() would help verify this remaining case.

AI-assisted rereview with Codex and Reviewer Sol.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working through these reviews. I need to correct my earlier implementation guidance: I focused too narrowly on repairing the digest path, which has contributed to the repeated changes. Sorry for the churn.

Could you pause the incremental fixes while I align the scope in #5053? I’m withdrawing the guidance to extend the digest-specific path.

The direction I propose is one external-session import flow, consistent with #2499: source adapters convert records into the existing StoredMessage model, the existing Host importer creates a native Maka Session atomically, and both Desktop and TUI open that Session.

The TUI should use the existing Host catalog/import operations. Once that replacement is complete, the old foreign-session scanner/digest handoff path should be removed. No second projection framework, importer, or message model is needed.

Before implementation, the issue should explicitly settle how imported history enters subsequent model context, preserving the existing trust requirements. The reproduced provenance and read-budget failures remain constraints to satisfy; the earlier patch-by-patch prescriptions should not dictate the replacement architecture.

Please hold off on further rework until that contract is clear. AI-assisted architecture review with Codex and Reviewer Sol.

…ojection

Astro-Han's remaining P1 on apache#5125: opencode 1.18 writes the automatic
task-summary instruction and MCP `readResource` answers as user text
parts marked `synthetic: true`, and `convertTranscript` drops that
marker on its way to the digest — so `readDigest` returned system text
("Summarize the task tool output above...") alongside the human prompt,
and the TUI handoff carried it into the model-facing import.

The bounded projection now drops `synthetic` parts while the adapter
still holds the raw provenance, before conversion; the full-session
import keeps them. A store-level regression seeds both shapes (summary
instruction on a human message, resource answer as a lone synthetic
part) and asserts the digest carries only the human text — red with the
filter stashed, green with it.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 9754b14: the restricted digest projection now drops parts the writer marked synthetic: true in the adapter, before convertTranscript erases the marker — both shapes from the review (the automatic task-summary instruction riding a human message, and an MCP readResource answer as a lone synthetic part of a follow-up user message). readDigest therefore answers only the human prompt; the full-session import keeps synthetic parts. readSessionBounded's doc states the restricted-projection contract.

Regression: a store-level test seeds those two synthetic shapes around a human prompt and asserts digest.userMessages equals only the human text — red with the filter stashed, green with it. Storage suites: adapter 19/19, store 33/34 (the one failure is the pre-existing Windows symlink-EPERM environment limitation, reproduced without this change). format:check exit 0 on the touched files.


中文:推了 9754b14——受限 digest 投影在 convertTranscript 抹掉标记之前,按写入方的 synthetic: true 过滤部件,评审点名的两种形状(随人工消息的自动任务总结指令、作为后续 user 消息独立部件的 MCP readResource 回答)都被排除,readDigest 只返回人类输入;全量导入保持原行为,readSessionBounded 文档注明受限投影契约。回归测试播种两种合成形状并断言 digest.userMessages 只含人工文本(撤过滤器即红),storage 套件绿(唯一失败为已知的 Windows symlink EPERM 预存),触及文件 format:check 退出码 0。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up and for working through the earlier comments. I rechecked this head with Reviewer Sol. I need to keep the architecture review open, and I also owe you a clearer issue: #5053 still asks for the old scanner/digest extension, while my later review withdrew that direction. That mismatch is not something you should have to resolve by guessing.

P1 — The single import flow is still not implemented. On the normal TUI external-session selection path, this head still calls readDigest, creates a new Session, and submits a digest handoff. In parallel, the existing Host importer already uses the OpenCode adapter to validate and atomically persist canonical StoredMessage history as a native Maka Session. Adding readSessionBounded and a synthetic-filtered projection continues the second path; the latest synthetic-part fix addresses my earlier local prescription, but does not implement the subsequently agreed consolidation.

The intended replacement remains: TUI uses the existing Host source/catalog/import operations and opens the resulting native Session; the replaced scanner/digest handoff and its now-unneeded restricted projection are removed in the same change. No additional importer, projection framework or message model is needed.

Please do not make another round of digest-specific patches. #5053 needs to state that replacement and how imported history enters later model context before the implementation work resumes. I am retaining the earlier withdrawal of my patch-by-patch guidance and closing the remaining old inline threads so they do not look like another checklist to implement. This closes those prescriptions, not the architecture question.

Reviewed head: 9754b1421476c57a23eac0a221229811d06712b9. Design review only; I did not rerun implementation tests after the design gate failed. Current CI passes. Codex-assisted review with Reviewer Sol.

@Astro-Han

Copy link
Copy Markdown
Contributor

谢谢这几轮持续跟进。我已在 #5053 确认统一方案,之前要求先澄清的方向现在已经明确:建议这个 PR 不再继续修补旧 scanner/digest 路径,也不用逐条追着旧的 digest-specific 建议修改了。前面的局部指导造成了反复,我在这里再说明一下,避免继续增加你的工作量。

后续用一个整合 PR 端到端完成:TUI 与 Desktop 都复用现有 Host catalog/import,由同一个 Adapter 转换为原生 Maka Session;同一 PR 删除被替代的 scanner、digest handoff 和受限摘要投影,不留下两条并行路径。导入采用完整或失败的原子性契约,上限用真实数据验证。

请与 #5055 的作者 @wutongyuonce 协调,把这里仍有价值的 Adapter 加固、真实问题证据和回归测试复用到整合 PR,避免两边各自实现一遍。这个 PR 可以在整合 PR 建立后标记为被替代并关闭;我不会再把旧摘要路径的局部补丁作为推进要求。

由 Codex 根据我的确认整理并发布。

@ggbdpq

ggbdpq commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

明白了,感谢把统一方案定下来 — 这确实终止了来回补丁的循环。

我会按 #5053 的方向@wutongyuonce 协调:本 PR 里仍有价值的部分(adapter 单一权威的数据库路径/会话 id 校验、50 会话/30 天上限、synthetic parts 剔除与 CJK UTF-8 字节预算的回归测试、真实 OpenCode schema 的证据)整理后供整合 PR 复用。整合 PR 建立后,本 PR 会标记为被替代并关闭,不新增摘要路径补丁。

@ggbdpq

ggbdpq commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

The integrated path has landed: #5308 unified external session imports through the Host and removed the foreign-session store this PR built on, exactly per @Astro-Han's ruling above. Marking superseded and closing.

What's worth carrying forward if the Host-side OpenCode import ever needs hardening: the adapter is the single authority for the database path and session id shape; the 50-session/30-day scan caps; synthetic-part filtering and the CJK UTF-8 byte-budget regressions; and the real OpenCode schema fixtures (collected from 1.18.21). Thanks for the long review threads — they made the scanner honest even though it didn't survive the architecture shift.

@ggbdpq ggbdpq closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): 统一 TUI 与 Desktop 的外部会话导入流程

2 participants