Skip to content

perf(session,memo,context): 三模块全量性能优化与安全加固#371

Merged
phantom5099 merged 6 commits into
1024XEngineer:mainfrom
wynxing:optimize/session-memo-context
Apr 20, 2026
Merged

perf(session,memo,context): 三模块全量性能优化与安全加固#371
phantom5099 merged 6 commits into
1024XEngineer:mainfrom
wynxing:optimize/session-memo-context

Conversation

@wynxing

@wynxing wynxing commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

摘要

针对 session、memo、context 三大核心模块进行系统性性能优化与安全加固,共涉及 17 个文件,修复/优化 10 个问题项。

变更详情

Session 模块

  • S-1 会话过期自动清理:应用启动时自动清除 30 天未更新的过期会话及附件目录,防止数据库无限膨胀
  • S-2 消息数自动裁剪:单会话消息数超过 8192 时在事务内自动裁剪最旧消息,对 runtime 透明
  • S-5 文件权限收紧:会话目录权限收紧为 0700,数据库文件权限收紧为 0600(Linux/macOS 生效)

Memo 模块

  • M-1 按优先级淘汰记忆:索引裁剪改为按类型权重淘汰(user > feedback > project > reference),手动创建优先于自动提取
  • M-2 LoadIndex 内存缓存:基于文件 mtime 的缓存机制,减少高频读取时的磁盘 I/O
  • M-4 增量指纹检测:AutoExtractor 使用 FNV-1a 64bit 哈希计算消息指纹,相同消息跳过重复 LLM 提取调用

Context 模块

  • C-1 git 命令超时保护:git 命令添加 5 秒超时,避免网络挂载或损坏仓库阻塞上下文构建
  • C-3 选择性深拷贝:MicroCompact 仅对需要压缩的消息做深拷贝,未修改消息共享原始引用

Runtime 模块

  • S-3 读写锁分离:sessionLock 改为 RWMutex,system_tool 读路径使用 RLock 降低并发锁竞争

测试计划

  • go build ./... 编译通过
  • go test ./... 全量测试通过
  • gofmt -w ./cmd ./internal 格式化检查
  • 新增函数均有对应测试覆盖(缓存命中/失效、淘汰优先级、指纹增量检测、消息裁剪边界值)

session 模块:
- 新增会话过期自动清理,启动时按 30 天阈值清除过期会话及附件
- 消息数超 8192 时自动裁剪最旧消息,防止数据库无限膨胀
- 目录权限收紧为 0700,数据库文件权限收紧为 0600
- Store 接口新增 CleanupExpiredSessions 方法

memo 模块:
- 索引淘汰改为按类型优先级(user > feedback > project > reference)
- FileStore LoadIndex 增加 mtime 缓存,减少高频读取磁盘 I/O
- AutoExtractor 增加 FNV-1a 指纹增量检测,相同消息跳过重复提取

context 模块:
- git 命令添加 5 秒超时保护,避免损坏仓库阻塞上下文构建
- MicroCompact 改为选择性深拷贝,未压缩消息共享引用减少分配

runtime 模块:
- sessionLock 改为 RWMutex,system_tool 读路径使用 RLock 降低锁竞争
- 补齐所有测试桩的 CleanupExpiredSessions 实现

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov

codecov Bot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.14286% with 64 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/session/sqlite_store.go 64.70% 20 Missing and 22 partials ⚠️
internal/memo/auto_extractor.go 63.33% 11 Missing ⚠️
internal/memo/store.go 75.67% 7 Missing and 2 partials ⚠️
internal/memo/service.go 92.85% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

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

Reviewed in four passes (code quality, performance, security, docs). I found a few correctness issues that can cause inconsistent state or data loss.

Comment thread internal/session/sqlite_store.go Outdated
Comment thread internal/session/sqlite_store.go
Comment thread internal/memo/store.go Outdated
Comment thread internal/memo/auto_extractor.go Outdated
@wynxing

wynxing commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@wynxing

wynxing commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator Author

/simplify

@fennoai

fennoai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

codeagent is processing your request, please wait a moment...

@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 给我一个能给AI用来review的提示词,先确认当前总体方案设计,再确认有没有存在风险点(诸如方案设计、模块边界等等)

@fennoai

fennoai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

可直接使用的 Review 提示词

你可以把下面整段直接贴给 AI(建议配合当前 PR 的 diff 一起输入):

你是一个严格的软件架构与代码审查专家。请对当前 PR 做“先方案、后风险”的审查。

【审查目标】
1) 先确认总体方案设计是否成立(设计目标、主链路闭环、模块职责、跨层依赖、演进方向)。
2) 再识别风险点(方案设计风险、模块边界风险、并发/一致性风险、性能退化风险、安全风险、可测试性风险、回归风险)。

【仓库关键约束(必须用于判定)】
- 主链路必须可用:用户输入 -> Agent 推理 -> 调用工具 -> 获取结果 -> 继续推理 -> UI 展示。
- 分层边界:TUI -> Runtime -> Provider/Tool Manager,不允许跨层直连。
- Provider 差异只能收敛在 `internal/provider`,不能泄漏到 runtime/tui。
- 工具能力必须在 `internal/tools`,runtime/tui 不直接执行工具逻辑。
- 会话状态与消息历史应由 runtime 管理,不散落到 UI。
- 配置/安全:不能明文密钥;关键配置需校验;工具要有越界/超时/输出限制。
- 测试目标:新增或变更逻辑要覆盖正常、边界、异常、回归分支。

【输入信息】
- PR 标题:perf(session,memo,context): 三模块全量性能优化与安全加固
- 重点模块:session / memo / context / runtime
- 声称改动:过期清理、消息裁剪、权限收紧、索引缓存、指纹去重、git 超时、RWMutex 等

【输出格式(严格遵守)】
A. 总体方案评估(先给结论)
- 结论:通过 / 有条件通过 / 不通过
- 设计一致性:是否符合主链路与分层边界
- 关键证据:引用具体文件与行号(`path:line`)

B. 风险清单(按严重级别排序)
- 每条风险必须包含:
  - 标题
  - 严重级别(Critical/High/Medium/Low)
  - 证据(文件+行号+触发条件)
  - 影响面(功能/性能/安全/可维护性)
  - 修复建议(最小可行改动)
  - 需要补充的测试点

C. 边界与职责专项检查
- 是否存在跨层调用、职责下沉/上浮、厂商字段泄漏、状态归属错误
- 若存在,给出重构落点(应该放在哪个模块)

D. 回归与发布建议
- 必测回归用例列表
- 发布阻断项(若有)

【审查要求】
- 不要泛泛而谈,只输出有证据的问题。
- 没有证据的推测必须显式标注“待确认”。
- 若未发现问题,明确写“未发现阻断问题”,并给出剩余观察风险。

说明

已按你的要求给出“先确认总体方案,再确认风险点(含模块边界)”的可执行提示词。本次仅提供提示词模板,未直接执行代码审查。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 你是一个严格的软件架构与代码审查专家。请对当前 PR 做“先方案、后风险”的审查。

【审查目标】

  1. 先确认总体方案设计是否成立(设计目标、主链路闭环、模块职责、跨层依赖、演进方向)。
  2. 再识别风险点(方案设计风险、模块边界风险、并发/一致性风险、性能退化风险、安全风险、可测试性风险、回归风险)。

【仓库关键约束(必须用于判定)】

  • 主链路必须可用:用户输入 -> Agent 推理 -> 调用工具 -> 获取结果 -> 继续推理 -> UI 展示。
  • 分层边界:TUI -> Runtime -> Provider/Tool Manager,不允许跨层直连。
  • Provider 差异只能收敛在 internal/provider,不能泄漏到 runtime/tui。
  • 工具能力必须在 internal/tools,runtime/tui 不直接执行工具逻辑。
  • 会话状态与消息历史应由 runtime 管理,不散落到 UI。
  • 配置/安全:不能明文密钥;关键配置需校验;工具要有越界/超时/输出限制。
  • 测试目标:新增或变更逻辑要覆盖正常、边界、异常、回归分支。

【输入信息】

  • PR 标题:perf(session,memo,context): 三模块全量性能优化与安全加固
  • 重点模块:session / memo / context / runtime
  • 声称改动:过期清理、消息裁剪、权限收紧、索引缓存、指纹去重、git 超时、RWMutex 等

【输出格式(严格遵守)】
A. 总体方案评估(先给结论)

  • 结论:通过 / 有条件通过 / 不通过
  • 设计一致性:是否符合主链路与分层边界
  • 关键证据:引用具体文件与行号(path:line

B. 风险清单(按严重级别排序)

  • 每条风险必须包含:
    • 标题
    • 严重级别(Critical/High/Medium/Low)
    • 证据(文件+行号+触发条件)
    • 影响面(功能/性能/安全/可维护性)
    • 修复建议(最小可行改动)
    • 需要补充的测试点

C. 边界与职责专项检查

  • 是否存在跨层调用、职责下沉/上浮、厂商字段泄漏、状态归属错误
  • 若存在,给出重构落点(应该放在哪个模块)

D. 回归与发布建议

  • 必测回归用例列表
  • 发布阻断项(若有)

【审查要求】

  • 不要泛泛而谈,只输出有证据的问题。
  • 没有证据的推测必须显式标注“待确认”。
  • 若未发现问题,明确写“未发现阻断问题”,并给出剩余观察风险。

@fennoai

fennoai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

A. 总体方案评估(先给结论)

  • 结论:有条件通过
  • 设计一致性:本 PR 的改动主要落在 session/memo/context/runtime/app 内部实现,未发现 tui -> runtime -> provider/tools 主链路被跨层打穿;provider 厂商差异也未扩散到 runtime/tui
  • 关键证据:
    • 启动期清理接入在 app 装配层:internal/app/bootstrap.go:160
    • 运行时系统工具仍通过 runtime + tool manager:internal/runtime/system_tool.go:76
    • provider 相关改动未进入本 PR 核心逻辑路径(仅 runtime 内部锁优化):internal/runtime/session_scheduler.go:110
    • context git 超时收敛在 context 模块:internal/context/source_system.go:111

B. 风险清单(按严重级别排序)

  • 标题:过期清理删除附件目录时缺少路径越界防护

  • 严重级别:High

  • 证据(文件+行号+触发条件):internal/session/sqlite_store.go:126-129 直接 os.RemoveAll(filepath.Join(s.assetsDir, id));触发条件是 DB 内存在异常/篡改的 id(绕过 API 校验)。对比已有安全做法:internal/session/sqlite_store.go:669-671 在删除单会话资产前会做 ensurePathWithinBase

  • 影响面:安全 / 数据完整性(潜在越界删除)。

  • 修复建议(最小可行改动):在清理循环中复用 ensurePathWithinBase(或先 validateStorageIDensurePathWithinBase),校验失败时跳过并记录告警。

  • 需要补充的测试点:构造“恶意 session id(通过直接写库)+ cleanup”场景,断言工作目录外文件不被删除。

  • 标题:权限“收紧”对已存在目录不生效

  • 严重级别:Medium

  • 证据(文件+行号+触发条件):internal/session/sqlite_store.go:693-698MkdirAll(..., 0o700);已存在目录时不会修改其 mode。触发条件:历史安装目录此前为 0755

  • 影响面:安全 / 合规(S-5 目标与实际不一致)。

  • 修复建议(最小可行改动):MkdirAll 后补 os.Chmod(s.projectDir, 0o700)os.Chmod(s.assetsDir, 0o700)(Unix 生效,Windows 可按平台分支处理)。

  • 需要补充的测试点:预建 0755 目录后初始化 store,断言 mode 被收紧(Unix)。

  • 标题:增量指纹未覆盖非文本消息,可能误判“未变化”

  • 严重级别:Medium

  • 证据(文件+行号+触发条件):internal/memo/auto_extractor.go:324-334 仅哈希 Role + Text parts。触发条件:文本不变但图片/其他非文本内容变化。

  • 影响面:功能正确性(会跳过应执行的自动提取)。

  • 修复建议(最小可行改动):指纹至少纳入 part.Kind 及非文本关键字段(建议对 message 做稳定序列化后哈希,避免字段遗漏)。

  • 需要补充的测试点:两次调度文本相同但图片/非文本 part 不同,应触发第二次提取。

  • 标题:启动清理入口退化为具体实现耦合,削弱 Store 抽象

  • 严重级别:Low

  • 证据(文件+行号+触发条件):internal/app/bootstrap.go:38-44cleanupExpiredSessions 显式依赖 *agentsession.SQLiteStore,而接口已在 internal/session/store.go:127-128 定义 CleanupExpiredSessions

  • 影响面:可维护性 / 演进性(未来切换 store 实现时,app 层需改代码)。

  • 修复建议(最小可行改动):将 cleanupExpiredSessions 及调用面收敛到 agentsession.Store 接口。

  • 需要补充的测试点:使用 Store mock/stub 验证 BuildRuntime 启动清理分支,而不依赖 sqlite 具体类型。

C. 边界与职责专项检查

  • 跨层调用:未发现 tui 直连 provider/tool 执行。
  • 职责下沉/上浮:未发现 runtime/tui 直接承载 tools 执行逻辑。
  • 厂商字段泄漏:未发现 provider 厂商差异泄漏到 runtime/tui。
  • 状态归属:会话状态仍由 runtime/session 管理,未散落到 UI。
  • 待优化点:app 启动清理对具体类型 *SQLiteStore 的耦合(见 internal/app/bootstrap.go:38-44),建议回归 Store 抽象。

D. 回归与发布建议

  • 必测回归用例列表:

    1. 启动清理:过期/未过期会话混合,验证仅删除过期会话及其附件。
    2. 安全防护:异常 session id(直写 DB)触发 cleanup,不得越界删除。
    3. 权限收紧:已存在 projectDir/assetsDir 的 mode 从 0755 收敛到 0700(Unix)。
    4. Append 裁剪:MaxSessionMessages 边界(=8192、>8192、大批次追加)。
    5. AutoExtractor 指纹:文本相同但非文本变化必须触发提取。
    6. Context git 超时:模拟阻塞命令,确认 5s 超时后不中断主链路。
  • 发布阻断项:

    • High:cleanup 附件删除缺少路径越界校验(建议修复后再合入)。

补充:本地已执行 go test ./internal/session ./internal/memo ./internal/context ./internal/runtime ./internal/app,均通过。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator

@xgopilot 现在修复过一轮review了,请你再进行一轮审查式review

@fennoai

fennoai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

审查结果(问题项)

  1. 按字节裁剪路径未遵循“按优先级淘汰”策略(Medium)
  • 证据:internal/memo/service.go:503-505maxIndexBytes 超限时调用 trimIndexEntriesByBytesinternal/memo/service.go:535-549 通过 entries[lo:] 仅按“最旧优先”删除前缀条目。
  • 触发条件:maxEntries 未触发或触发后仍超 maxIndexBytes
  • 影响:会把高优先级但更旧的记忆先删掉,和 PR 声称的 user > feedback > project > reference(且 manual 优先)策略不一致。
  • 最小修复建议:字节裁剪阶段也按 evictionPriority() 选 victim(循环移除直到 RenderIndex(index) <= maxIndexBytes),不要仅按时间前缀裁剪。
  • 建议补测:构造“旧的高优先级 + 新的低优先级”并触发 maxIndexBytes,断言删除低优先级条目。
  1. 过期会话附件清理不响应 context 取消(Low)
  • 证据:internal/session/sqlite_store.go:127-129 在事务提交后直接调用 cleanupExpiredSessionAssets(expiredIDs)internal/session/sqlite_store.go:1226-1232 内部循环 os.RemoveAll,无 ctx.Err() 检查。
  • 触发条件:过期会话/附件规模较大、慢磁盘或网络挂载场景下,外部已取消上下文。
  • 影响:启动阶段取消不及时,可能额外阻塞启动/退出路径。
  • 最小修复建议:将 ctx 传入 cleanupExpiredSessionAssets,每轮删除前检查 ctx.Err() 并中止。
  • 建议补测:构造多个待清理目录,在清理中途取消 context,断言函数尽快返回并停止后续删除。

说明

  • 本轮仅执行审查与验证,未改动代码。
  • 已本地执行:go test ./internal/session ./internal/memo ./internal/context ./internal/runtime ./internal/app

View job run

@phantom5099 phantom5099 merged commit 6afe196 into 1024XEngineer:main Apr 20, 2026
1 of 2 checks passed
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.

2 participants