Skip to content

重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入#343

Merged
phantom5099 merged 8 commits into
1024XEngineer:mainfrom
wynxing:refactor/sqlite-persistence
Apr 17, 2026
Merged

重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入#343
phantom5099 merged 8 commits into
1024XEngineer:mainfrom
wynxing:refactor/sqlite-persistence

Conversation

@wynxing

@wynxing wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

背景

当前会话持久化基于单个 session.json 快照文件:

  • 会话列表需要扫描并读取每个会话文件
  • 运行过程中用户消息、assistant 回复、tool result 都会触发整份会话重写
  • 会话越多,列表越慢
  • 会话越长,写入越慢

这次改动将会话持久化主路径重构为 SQLite 增量存储,不再兼容旧 JSON 会话数据。

主要改动

1. 会话存储从 JSON 改为 SQLite

  • internal/session 新增 SQLiteStore
  • 每个工作区使用单个 session.db
  • 启用 WAL、foreign_keys=ONbusy_timeout=5000
  • 使用 SQLite user_version 管理 schema 版本

2. 拆分会话头与消息正文

  • sessions 表保存标题、更新时间、provider/model、workdir、task_state、todos、skills、token 统计等摘要信息
  • messages 表按 session_id + seq 逐条存储消息
  • session_assets 表保存资产元数据,二进制内容仍落盘到 assets/

3. Store 接口改为意图型写入

移除快照式整份 Save,改为:

  • CreateSession
  • LoadSession
  • ListSummaries
  • AppendMessages
  • UpdateSessionState
  • ReplaceTranscript

这样 runtime 不再提交整份 Session 快照,而是表达具体变更意图。

4. runtime 持久化链路改为增量写入

  • 新会话创建改为 CreateSession
  • 用户消息改为 AppendMessages
  • assistant 回复改为 AppendMessages,并同步更新 provider/model/token
  • tool result 改为 AppendMessages
  • todo、skills、workdir、task_state 更新改为 UpdateSessionState
  • compact 后 transcript 重写改为 ReplaceTranscript

5. 移除旧 JSON 主路径

  • 不再使用 session.json 作为生产持久化格式
  • 不做旧数据兼容、不做迁移、不做回读
  • 旧开发期 JSON 会话数据直接忽略

6. 文档与测试同步更新

  • 更新会话持久化设计文档
  • 更新 Todo 持久化文档
  • 重写并补充 internal/sessioninternal/runtimeinternal/app 相关测试
  • 修复 compact 后错误覆盖会话标题的问题
  • 统一缺失会话时的错误语义,避免直接泄漏 SQLite 外键错误

验证

已通过:

  • go build ./...
  • go test ./internal/session ./internal/runtime ./internal/app

当前覆盖率:

  • internal/session: 86.5%
  • internal/runtime: 95.6%
  • internal/app: 92.5%

说明:

  • go test ./... 仍存在仓库内其他既有失败,主要不在本次会话持久化改造范围内

影响

  • 会话列表读取不再依赖扫描和解析完整历史文件
  • 常规消息写入不再与历史消息长度线性耦合
  • 会话资产仍保持文件系统存储,不将二进制内容塞入数据库
  • 当前 runtime.LoadSession / TUI 仍保持完整加载单会话,不做懒加载

@wynxing wynxing changed the title refactor(session): migrate persistence to sqlite 重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入 Apr 17, 2026
@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

@wynxing

wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@wynxing

wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

/simplify

@fennoai fennoai Bot 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.

发现 1 个需要修正的问题(其余在代码质量、性能、安全和文档一致性检查中未发现值得单独评论的问题)。

Comment thread internal/session/sqlite_store.go Outdated

// ensureDB 懒加载数据库并执行 schema 初始化。
func (s *SQLiteStore) ensureDB(ctx context.Context) (*sql.DB, error) {
s.once.Do(func() {

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.

ensureDB调用方的 request-scoped ctx 直接传进 sync.Once 初始化链路。这样如果第一次调用刚好因超时/取消失败(例如 context canceled),initErr 会被永久缓存,后续即使用健康 context 也无法重试初始化,整个 store 会进入不可恢复状态。建议将一次性初始化与请求生命周期解耦(例如使用 context.Background() + 内部超时),或改为可重试的初始化门闩而不是 sync.Once 硬缓存失败。

@fennoai fennoai Bot 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.

Found 2 noteworthy issues after code-quality/performance/security/docs passes. Both are in the new SQLite/session persistence path.

Comment thread internal/session/sqlite_store.go Outdated

// ensureDB 懒加载数据库并执行 schema 初始化。
func (s *SQLiteStore) ensureDB(ctx context.Context) (*sql.DB, error) {
s.once.Do(func() {

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.

[high] ensureDB binds initialization to the first caller's ctx via sync.Once. If that first call is canceled/times out (or hits a transient init error), s.initErr is permanently cached and all future store operations fail until process restart. This makes initialization non-recoverable under transient failures. Consider decoupling one-time schema init from request-scoped context (or allowing retry when init fails).

Comment thread internal/session/storage_helpers.go Outdated

// ensurePathWithinBase 校验目标路径位于指定基目录内,避免路径越界。
func ensurePathWithinBase(baseDir string, target string) error {
baseAbs, err := filepath.Abs(baseDir)

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.

[medium][security] ensurePathWithinBase uses filepath.Abs + Rel, which is purely lexical and does not resolve symlinks. A symlink inside the project/assets tree can still redirect writes/reads outside the workspace while passing this check. For boundary enforcement, resolve trusted base/target with EvalSymlinks (or open with no-follow semantics) before containment checks.

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Result

Implemented the unresolved review fixes and pushed to the PR branch Yumiue:refactor/sqlite-persistence.

What I changed

  1. Fixed non-recoverable SQLite init path:
  • Updated internal/session/sqlite_store.go ensureDB to remove sync.Once + cached permanent init failure.
  • Switched to mutex-guarded lazy init that allows retry after a transient/canceled first initialization.
  1. Hardened path boundary checks against symlink escape:
  • Updated internal/session/storage_helpers.go to resolve symlinks before containment checks.
  • Added resolvePathForContainment to handle both existing paths and non-existing final targets via parent resolution.
  1. Added regression tests:
  • internal/session/store_test.go: TestSQLiteStoreEnsureDBCanRetryAfterInitFailure
  • internal/session/helpers_test.go: symlink-escape case for ensurePathWithinBase

Validation

  • go test ./internal/session
  • go test ./internal/runtime ./internal/app

Notes

  • I attempted to run a dedicated code-simplifier sub-agent twice as requested, but both runs failed with upstream 502 Bad Gateway. I then manually reviewed/simplified the touched changes and kept behavior-focused minimal edits.

Commit

  • e814384 fix(session): make sqlite init retryable and harden symlink boundary checks

View job run

@wynxing

wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

/code 补充测试率

…checks

- replace sync.Once init cache with mutex-guarded retryable init path
- resolve symlinks in containment checks to prevent asset path escapes
- add regression tests for init retry and symlink escape cases

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

处理结果

已按“补充测试率”完成补测并推送到当前 PR 分支。

变更内容

  • 新增测试文件:internal/session/sqlite_store_additional_test.go
  • 主要补充了以下分支覆盖:
    • SQLiteStore 多个接口的 context canceled 早返回分支
    • 会话 ID 非法输入分支(LoadSession / AppendMessages / UpdateSessionState / ReplaceTranscript
    • expectRowsAffected 的错误分支与 rows=0 分支
    • normalizeMessages(nil)fromUnixMillis(0)boolToInt(true/false) 等工具函数分支
    • mustJSONStringnil 分支和 panic 分支
    • cloneMessage / cloneSessionValue 深拷贝分支
    • storage_helpers 的额外异常路径(缺失父目录、替换目录失败)
    • initializeSQLiteSchema 在关闭 DB 上的失败路径

验证结果

  • go test ./internal/session ./internal/runtime ./internal/app 通过
  • internal/session 覆盖率从约 86.7% 提升到 89.5%

提交信息

  • Commit: f259ac8
  • 已推送到:Yumiue:refactor/sqlite-persistence

View job run

@wynxing

wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

/code 处理冲突

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

冲突处理结果

已完成本次“处理冲突”。本次冲突点集中在依赖清单,已在当前 PR 分支完成整合并推送。

已完成内容

  • 合并 go.mod 依赖:纳入 origin/main 新增的 updater 相关依赖,同时保留本 PR 的 SQLite 持久化依赖。
  • 合并 go.sum:补齐双方依赖校验条目,避免后续合并时在模块校验文件再次冲突。
  • 验证通过:
    • go build ./...
    • go test ./internal/session ./internal/runtime ./internal/app
  • 已推送到当前 PR 分支:Yumiue:refactor/sqlite-persistence(commit: 2bda024)。

未做内容

  • 未处理本 PR 之外的功能改动,仅针对本次冲突相关的依赖文件做了最小修复。

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
@wynxing

wynxing commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

/code 我叫你解决合并冲突!!

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

冲突处理完成

已完成这次“解决合并冲突”,并把结果推送到分支 fork-pr-343-1776427778(commit: 66ec304)。

本次处理内容

  • 合并 origin/main 到当前工作分支并清理全部冲突标记。
  • 处理冲突文件:go.modgo.suminternal/runtime/*_test.gointernal/session/store.gointernal/session/asset_store_test.go
  • 修复合并后接口不兼容:
    • internal/session/input_preparer.go 适配到新的 SQLite 意图型 Store 接口(CreateSession/LoadSession/UpdateSessionState)。
    • SQLiteStore 补充 DeleteSessionDeleteAsset(用于输入准备失败时的清理回滚)。
    • 更新相关测试中的旧 Save/Load 调用。

验证

  • go build ./... 通过。
  • go test ./internal/session ./internal/runtime ./internal/app 通过。

推送与 PR

View job run

xgopilot and others added 2 commits April 17, 2026 12:32
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
fix: resolve merge conflicts with main
@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 检查一遍当前修改有没有破坏项目中的图片输入链路

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

检查结论

未发现本次 SQLite 持久化改造破坏图片输入主链路的问题。

核查范围

  1. 输入归一化链路:internal/runtime/input_prepare.go -> internal/session/input_preparer.go
  2. 图片资产持久化:internal/session/sqlite_store.goSaveAsset/Open/Stat/DeleteAsset
  3. 推理阶段读取链路:internal/runtime/session_assets.go(provider 读取 session asset
  4. 启动装配:internal/app/bootstrap.go 仍将同一个 sessionStore 注入为 SessionStore + SessionAssetStore + UserInputPreparer

验证结果

已执行并通过以下相关测试:

  • go test ./internal/session -run 'TestInputPreparer|TestResolveImage|TestAssetStore|TestSQLiteStoreSaveAsset|TestSQLiteStoreOpen|TestSQLiteStoreStat|TestSQLiteStoreDeleteAsset'
  • go test ./internal/runtime -run 'TestServicePrepareUserInput|TestRuntimeSessionAssetReader|TestBuildSessionAssetReader|TestServiceSubmit|TestServiceRunWithImageOnlyInputUsesImageTitle|TestServiceRunWithImageInputSetsTitleFromText'

备注

  • 当前 PR 明确不兼容旧 JSON 会话;若本地仍有旧会话中的图片引用,无法通过新 session_assets 元数据读取,这属于本 PR 设计范围内行为,不是新增回归。
  • 本次未做 TUI 端人工端到端点测,仅完成代码路径与自动化测试核查。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 针对本次PR,请给我一份给AIreview用的提示词,针对的是修改中所有可能存在的风险点

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

AI Review 提示词(风险点全覆盖版)

你是资深 Go 代码审查工程师。请对 PR #343(“重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入”)做“风险导向”审查。

【审查目标】
重点发现:功能回归、数据一致性问题、并发/事务问题、错误语义退化、安全问题、性能退化、测试遗漏。
不要给正向表扬,只输出问题与风险。

【对比基线】
- 必须以 `origin/main...HEAD` 为差异范围
- 重点文件:
  - `internal/session/sqlite_store.go`
  - `internal/session/store.go`
  - `internal/session/storage_helpers.go`
  - `internal/session/input_preparer.go`
  - `internal/runtime/session_mutation.go`
  - `internal/runtime/run.go`
  - `internal/app/bootstrap.go`
  - 相关测试与文档变更

【项目约束(必须检查)】
- 架构边界:不得跨层直连,runtime/tui 不直接执行工具逻辑
- 持久化职责:session 状态由 runtime 管理,store 仅负责持久化语义
- 配置与安全:路径边界、工作目录限制、避免明文敏感信息
- 本 PR 设计:不兼容旧 JSON,会话主路径为 SQLite 增量写入

【必查风险清单】
1. 数据一致性与原子性
- `AppendMessages` / `UpdateSessionState` / `ReplaceTranscript` 是否满足事务原子性
- `messages` 与 `sessions` 摘要字段(`last_seq`、`message_count`、token 统计)是否始终一致
- `ReplaceTranscript` 后序号、计数、更新时间、上下文字段是否正确

2. 状态覆盖与字段回归
- compact 后是否错误覆盖 `title/provider/model/workdir/task_state/todos/skills/tokens`
- 空消息分支(assistant 无文本但有 token/provider/model 变化)是否正确持久化

3. 并发与初始化可靠性
- SQLite 初始化是否可恢复(首次 ctx cancel/超时后是否会永久失败)
- 多协程并发 append/replace 是否可能导致序号冲突或状态丢失
- WAL + busy_timeout 配置是否足以避免常见锁竞争失败语义异常

4. 错误语义与可观测性
- “会话不存在”是否统一为领域错误(如 `os.ErrNotExist`),而不是泄漏底层 SQLite 外键错误
- 错误包装是否保留上下文(操作名、sessionID、关键参数)

5. 安全与路径边界
- 资产路径校验是否防止目录穿越与 symlink escape
- 资产文件落盘/删除流程是否存在 TOCTOU、孤儿文件、误删风险
- `relative_path` 与真实文件路径映射是否可被构造绕过

6. 性能与可扩展性
- 会话列表与加载路径是否使用必要索引,避免全表扫描退化
- 长会话下 append/replace 的复杂度与 I/O 放大是否可接受
- 大附件处理是否有上限与 backpressure(避免内存/磁盘压力失控)

7. 接口契约与兼容性
- Store 从快照式 `Save` 改为意图式写入后,runtime 调用是否全量迁移且语义一致
- InputPreparer/Bootstrap 注入链路是否完整,图片输入链路不应回归

8. 测试充分性
- 新增/修改逻辑是否覆盖:正常路径、边界条件、异常分支、回归场景
- 特别关注:context cancel、缺失会话、事务失败回滚、并发竞争、路径安全、compact 后状态保持

【输出格式(严格)】
- 先列 Findings(按严重级别降序:high -> medium -> low)
- 每条包含:
  - 标题(简短)
  - 严重级别
  - 证据(文件路径 + 行号)
  - 风险说明(会导致什么错误行为)
  - 修复建议(最小改动优先)
- 若无问题,明确写:`No blocking issues found`,并列出残余风险与测试盲区。

【额外要求】
- 不做臆测;每条结论必须能在 diff 中定位证据
- 不输出与本 PR 无关的泛泛建议
- 不改代码,只做审查结论

说明

按本次触发指令,仅提供了“AI review 提示词”。

View job run

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

结论

会有压力,而且在你这个会话场景里通常不划算把图片二进制直接塞进 SQLite 主库。你们这次 PR 采用的“附件落盘 + SQLite 存元数据”是更稳妥的默认方案。

对比(结合当前 PR)

  1. 图片直接存 SQLite(BLOB)
  • 优点:事务一致性最好(消息与图片可同事务提交)、备份恢复单文件简单、路径管理更少。
  • 缺点:session.db 体积增长快,WAL/Checkpoint 开销上升;大对象读写会拉高锁等待与 I/O 峰值;VACUUM/备份/同步变慢;热点查询更容易被大页读拖慢。
  1. 图片存本地目录,SQLite 只存路径/元数据(你们当前方案)
  • 优点:数据库保持轻量,消息增量写入更稳定;大文件 I/O 与结构化查询解耦;列表、摘要、分页查询性能更可控。
  • 缺点:需要处理“DB 记录与文件”双写一致性(孤儿文件/悬挂记录)、搬迁与备份要同时覆盖 DB+assets、路径安全与清理策略要做好。

建议

  • 对 NeoCode 这种“文本消息高频 + 图片附件低频且可能偏大”的负载,优先保持现在方案。
  • 若后续要进一步增强可靠性,可加三件事:
  1. 启动时或定时做 session_assets 与磁盘文件对账修复。
  2. 明确原子写流程(先临时文件再 rename,再写元数据;失败回滚清理)。
  3. 备份/迁移工具强制打包 session.db + assets/,避免只备份其一。

本次完成情况

  • 已完成:问题分析与两种方案优劣对比,并给出针对本 PR 的落地建议。
  • 未执行:未做代码改动、未新增测试(本次是分析答复)。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 回答我一个问题:当前如果直接把图片数据存到sqlite中,会不会对数据库本身造成压力?如果改成图片之类的附件存到本地目录,sqlite只需保存图片路径,和原来的方案相比有什么优劣?

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

结论

会有压力。把图片二进制直接写进 SQLite(BLOB)在图片数量/体积上来后,会明显放大 DB 文件、WAL 文件和 checkpoint 成本;读写高峰时也更容易让数据库承担混合负载压力。

你说的“附件落本地目录,SQLite 只存元数据+路径”就是当前这个 PR 采用的方向(session_assets 存 metadata,二进制在 assets/)。对这个场景总体更合适。

两种方案对比

  1. 图片直接存 SQLite(BLOB)
  • 优点:
    • 单库备份/迁移简单(一个 session.db
    • 元数据与内容天然事务一致,删除/回滚更直观
  • 缺点:
    • DB/WAL 膨胀快,checkpoint 与 VACUUM 成本高
    • 大对象 I/O 会挤占数据库页缓存,影响会话列表/消息查询
    • 备份和复制变重,任何变更都可能导致更大增量
  1. 附件落文件系统 + SQLite 存路径/大小/MIME
  • 优点:
    • DB 保持“结构化小数据”,查询稳定,写入延迟更可控
    • 图片读写走文件系统流式 I/O,更适合大对象
    • 可单独做附件清理、限额、冷热分层,不拖慢核心会话表
  • 缺点:
    • 需要处理“数据库记录与文件”双写一致性(孤儿文件/悬挂记录)
    • 备份需同时覆盖 DB 和 assets/ 目录
    • 需要额外做路径边界与安全校验

对本 PR 的建议

  • 当前设计选型是合理的:会话主数据走 SQLite,附件二进制落盘。
  • 重点继续守住三点:
    1. 原子性与补偿:保存元数据失败时回滚文件、删除记录时幂等清理文件。
    2. 安全边界:严格限制 assets 路径在工作区基目录内(防止路径穿越/软链接逃逸)。
    3. 运维可观测:增加附件目录体积、孤儿文件数、DB/WAL 大小的巡检指标。

本次执行说明

  • 已基于当前 PR 的实现与文档完成分析答复。
  • 未做代码修改、未新增提交。

View job run

@phantom5099

phantom5099 commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

@xgopilot 你是资深 Go 代码审查工程师。请对 PR #343(“重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入”)做“风险导向”审查。

【审查目标】
重点发现:功能回归、数据一致性问题、并发/事务问题、错误语义退化、安全问题、性能退化、测试遗漏。
不要给正向表扬,只输出问题与风险。

【对比基线】

  • 必须以 origin/main...HEAD 为差异范围
  • 重点文件:
    • internal/session/sqlite_store.go
    • internal/session/store.go
    • internal/session/storage_helpers.go
    • internal/session/input_preparer.go
    • internal/runtime/session_mutation.go
    • internal/runtime/run.go
    • internal/app/bootstrap.go
    • 相关测试与文档变更

【项目约束(必须检查)】

  • 架构边界:不得跨层直连,runtime/tui 不直接执行工具逻辑
  • 持久化职责:session 状态由 runtime 管理,store 仅负责持久化语义
  • 配置与安全:路径边界、工作目录限制、避免明文敏感信息
  • 本 PR 设计:不兼容旧 JSON,会话主路径为 SQLite 增量写入

【必查风险清单】

  1. 数据一致性与原子性
  • AppendMessages / UpdateSessionState / ReplaceTranscript 是否满足事务原子性
  • messagessessions 摘要字段(last_seqmessage_count、token 统计)是否始终一致
  • ReplaceTranscript 后序号、计数、更新时间、上下文字段是否正确
  1. 状态覆盖与字段回归
  • compact 后是否错误覆盖 title/provider/model/workdir/task_state/todos/skills/tokens
  • 空消息分支(assistant 无文本但有 token/provider/model 变化)是否正确持久化
  1. 并发与初始化可靠性
  • SQLite 初始化是否可恢复(首次 ctx cancel/超时后是否会永久失败)
  • 多协程并发 append/replace 是否可能导致序号冲突或状态丢失
  • WAL + busy_timeout 配置是否足以避免常见锁竞争失败语义异常
  1. 错误语义与可观测性
  • “会话不存在”是否统一为领域错误(如 os.ErrNotExist),而不是泄漏底层 SQLite 外键错误
  • 错误包装是否保留上下文(操作名、sessionID、关键参数)
  1. 安全与路径边界
  • 资产路径校验是否防止目录穿越与 symlink escape
  • 资产文件落盘/删除流程是否存在 TOCTOU、孤儿文件、误删风险
  • relative_path 与真实文件路径映射是否可被构造绕过
  1. 性能与可扩展性
  • 会话列表与加载路径是否使用必要索引,避免全表扫描退化
  • 长会话下 append/replace 的复杂度与 I/O 放大是否可接受
  • 大附件处理是否有上限与 backpressure(避免内存/磁盘压力失控)
  1. 接口契约与兼容性
  • Store 从快照式 Save 改为意图式写入后,runtime 调用是否全量迁移且语义一致
  • InputPreparer/Bootstrap 注入链路是否完整,图片输入链路不应回归
  1. 测试充分性
  • 新增/修改逻辑是否覆盖:正常路径、边界条件、异常分支、回归场景
  • 特别关注:context cancel、缺失会话、事务失败回滚、并发竞争、路径安全、compact 后状态保持

【输出格式(严格)】

  • 先列 Findings(按严重级别降序:high -> medium -> low)
  • 每条包含:
    • 标题(简短)
    • 严重级别
    • 证据(文件路径 + 行号)
    • 风险说明(会导致什么错误行为)
    • 修复建议(最小改动优先)
  • 若无问题,明确写:No blocking issues found,并列出残余风险与测试盲区。

【额外要求】

  • 不做臆测;每条结论必须能在 diff 中定位证据
  • 不输出与本 PR 无关的泛泛建议
  • 不改代码,只做审查结论

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Findings

  1. Prepare 阶段的 workdir 持久化会覆盖并发 Run 的会话头字段
  • 严重级别: high
  • 证据: internal/session/input_preparer.go:376, internal/session/input_preparer.go:404, internal/session/sqlite_store.go:276, internal/runtime/input_prepare.go:50
  • 风险说明: Prepare 对已有会话仅修改了 Workdir,但 persistSessionWorkdirUpdate 会把旧快照里的 provider/model/task_state/todos/skills/token totals 全量写回;而 PrepareUserInput 本身未持有会话锁。若与同会话 Run 并发,可能把较新的 token 统计与状态回写成旧值,造成状态回退。
  • 修复建议: 增加最小粒度写接口(例如仅更新 workdir + updated_at_ms),或在 Prepare 对已有会话路径与 Run 共享同一会话锁,避免全字段覆盖。
  1. SaveAsset 对“会话不存在”未统一为领域错误,仍可能泄漏 SQLite 外键错误
  • 严重级别: medium
  • 证据: internal/session/sqlite_store.go:454, internal/session/sqlite_store.go:467, internal/session/sqlite_store.go:635, internal/session/asset_store_test.go:72
  • 风险说明: SaveAsset 插入 session_assets 失败时直接返回底层错误;若会话不存在/并发被删,调用方会拿到 SQLite 外键报错字符串,无法稳定用 errors.Is(err, os.ErrNotExist) 判定,错误语义与 LoadSession/AppendMessages/UpdateSessionState/ReplaceTranscript 不一致。
  • 修复建议: 对插入阶段的外键违规做错误映射,统一包装为 os.ErrNotExist(保留操作上下文);测试里把“missing session”从“仅报错”升级为 errors.Is(err, os.ErrNotExist) 断言。
  1. 并发回归场景缺少针对性测试(Prepare vs Run)
  • 严重级别: low
  • 证据: internal/runtime/input_prepare_test.go:15, internal/runtime/runtime_test.go:2979
  • 风险说明: 现有测试覆盖了输入归一化事件与 Run/Compact 互斥,但未覆盖“已有会话 + requestedWorkdir 更新”与并发 Run 的交互,字段覆盖回退类问题难以及时发现。
  • 修复建议: 增加并发测试:同一 session 并发执行 PrepareUserInput(requestedWorkdir)Run,断言仅 workdir/updated_at 变化,不回退 provider/model/task_state/todos/tokens

残余风险与盲区

  • 本次审查只基于 origin/main...HEAD 静态 diff 与当前测试代码,未执行压力并发压测。
  • AppendMessages/ReplaceTranscript 的高并发冲突语义(锁等待/冲突重试策略)仍缺少系统级测试验证。

View job run

@phantom5099 phantom5099 mentioned this pull request Apr 17, 2026
@phantom5099 phantom5099 merged commit e009864 into 1024XEngineer:main Apr 17, 2026
2 checks passed
@wynxing wynxing deleted the refactor/sqlite-persistence branch April 20, 2026 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants