Skip to content

fix(desktop): use .run() for CAS clear of stale sdk_session_id (#3496) - #3525

Open
Battleplus wants to merge 2 commits into
makecindy:mainfrom
Battleplus:fix/drizzle-returning-cas
Open

fix(desktop): use .run() for CAS clear of stale sdk_session_id (#3496)#3525
Battleplus wants to merge 2 commits into
makecindy:mainfrom
Battleplus:fix/drizzle-returning-cas

Conversation

@Battleplus

@Battleplus Battleplus commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

问题

重开旧 Claude 会话时偶发 "No conversation found" 终态错误。根因是 compareAndClearSdkSessionId() 通过 drizzle IPC 代理使用 .returning(),但代理对 UPDATE 语句的 RETURNING 子句未实现,始终返回 [],导致 CAS(compare-and-swap)即使 UPDATE 成功也报告失败,阻断了 invalid-resume 自愈路径。

改动

  • session-storage.ts: 将 .returning({ id }) 替换为 .run(),按 result.changes > 0 判断 CAS 命中
  • sessionStorageRemoteHostId.test.ts: 更新 mock 从 .returning() 改为 .run(),拆分为3个回归测试(命中/未命中/并发)

此模式与 sessions.ts:1323 中已有的 .run() 用法一致。

这次改了什么

2 个文件:

  • apps/desktop/src/main/maker-host/session-storage.ts — CAS 判定逻辑从 .returning() 改为 .run()(3 行变化)
  • apps/desktop/src/main/maker-host/__tests__/sessionStorageRemoteHostId.test.ts — mock 更新 + 新增 3 个回归测试

怎么验证的

  • Desktop 测试 12/12 通过
  • maker-core 测试 77/77 通过
  • 新增回归测试覆盖:CAS 命中(UPDATE 成功)、CAS 未命中(sdk_session_id 不匹配)、并发 CAS 竞争
  • 仅改动 2 个文件,无 scope creep

风险

  • 改动极小(3 行核心逻辑 + 测试更新),不影响其他 session 操作
  • .run() 模式与现有 sessions.ts:1323 一致,非新模式
  • 无持久化、无数据库 schema、无 UI 变化
  • 如果 result.changes 在 IPC 层异常,行为与修复前相同(自愈失败),不会引入新问题

Fixes #3496

@Battleplus
Battleplus requested a review from a team as a code owner August 27, 2026 11:22
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

此 PR 将桌面端清理陈旧 sdk_session_id 的条件更新从代理不支持的 .returning() 改为 .run(),并通过受影响行数判断 CAS 是否命中。

  • 保留按 session id 和预期 SDK session id 进行原子清理的条件
  • 为命中、未命中及连续调用路径更新单元测试
  • 新增的“并发”测试仍仅模拟返回值,没有验证真实状态转换

Confidence Score: 4/5

此 PR 看起来可以安全合并,但建议增强并发回归测试,使其真实验证 CAS 状态转换及条件谓词。

生产代码正确使用数据库代理支持的受影响行数结果,未发现当前运行时缺陷;唯一问题是新增测试无法捕获 CAS 条件被意外移除的回归。

Files Needing Attention: apps/desktop/src/main/maker-host/tests/sessionStorageRemoteHostId.test.ts

Important Files Changed

Filename Overview
apps/desktop/src/main/maker-host/session-storage.ts 使用代理明确支持的 .run().changes 判断条件 UPDATE 是否命中,生产实现与现有数据库代理契约一致。
apps/desktop/src/main/maker-host/tests/sessionStorageRemoteHostId.test.ts mock 已适配 .run(),但新增并发用例通过手动切换返回值模拟结果,未验证 CAS 条件或数据库状态转换。
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/main/maker-host/__tests__/sessionStorageRemoteHostId.test.ts:179-185
**并发测试未验证状态**

当前用例通过手动切换 `runResult` 来决定两次调用的结果,且 mock 的 `where()` 不检查条件表达式,因此即使实现遗漏 `sdkSessionId = expectedSdkSessionId` 谓词,该测试仍会通过。建议让第一次调用实际改变模拟数据库状态,并由第二次调用的条件匹配自然返回未命中,以覆盖旧恢复请求不得清除并发写入的新 ID 这一 CAS 契约。

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(desktop): use .run() for CAS clear o..." | Re-trigger Greptile

Comment on lines +179 to +185
it('concurrent path: hit then miss, each call correct', async () => {
const storage = new DesktopSessionStorage();
h.runResult = { changes: 1 };
await expect(storage.compareAndClearSdkSessionId('s1', 'sdk-a')).resolves.toBe(true);
h.runResult = { changes: 0 };
await expect(storage.compareAndClearSdkSessionId('s1', 'sdk-a')).resolves.toBe(false);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 并发测试未验证状态

当前用例通过手动切换 runResult 来决定两次调用的结果,且 mock 的 where() 不检查条件表达式,因此即使实现遗漏 sdkSessionId = expectedSdkSessionId 谓词,该测试仍会通过。建议让第一次调用实际改变模拟数据库状态,并由第二次调用的条件匹配自然返回未命中,以覆盖旧恢复请求不得清除并发写入的新 ID 这一 CAS 契约。

Context Used: 使用和PR描述相同的语言进行评论 (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/maker-host/__tests__/sessionStorageRemoteHostId.test.ts
Line: 179-185

Comment:
**并发测试未验证状态**

当前用例通过手动切换 `runResult` 来决定两次调用的结果,且 mock 的 `where()` 不检查条件表达式,因此即使实现遗漏 `sdkSessionId = expectedSdkSessionId` 谓词,该测试仍会通过。建议让第一次调用实际改变模拟数据库状态,并由第二次调用的条件匹配自然返回未命中,以覆盖旧恢复请求不得清除并发写入的新 ID 这一 CAS 契约。

**Context Used:** 使用和PR描述相同的语言进行评论 ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

…indy#3496)

The drizzle proxy over IPC does not serialize .returning() for UPDATE
queries — it always returns []. This caused compareAndClearSdkSessionId()
to report CAS failure even when the UPDATE succeeded, blocking the
self-healing path for old Claude sessions that hit "No conversation found".

Replace .returning() with .run() and check result.changes, matching the
existing pattern in sessions.ts:1323.

Also update the test mock to use .run() and split into three focused
regression tests: CAS hit, CAS miss, and concurrent hit-then-miss.

Fixes makecindy#3496

Signed-off-by: Battleplus <3559424769@qq.com>

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@Battleplus
Battleplus force-pushed the fix/drizzle-returning-cas branch from 701138d to 1073aef Compare August 27, 2026 11:25
@MagicLizi

Copy link
Copy Markdown
Contributor

@Battleplus 👋 这个 PR 还有 1 条 review conversation 没 resolve(apps/desktop/src/main/maker-host/tests/sessionStorageRemoteHostId.test.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。

如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。

@MagicLizi MagicLizi added the status:awaiting-author 等作者修改(review-pr 自动维护,仅展示) label Aug 27, 2026

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

格式门未通过,请按仓库 PR 模板补全 Description 后再请求审查。

缺段落:这次改了什么 / 怎么验证的 / 风险

当前描述无法确认改动范围、验证证据和回滚风险。补全三个段落后推送或编辑 PR 即可重新进入审查。

Signed-off-by: Battleplus <3559424769@qq.com>
@Battleplus
Battleplus force-pushed the fix/drizzle-returning-cas branch from 073370f to bffb7ed Compare August 27, 2026 13:02

@Battleplus Battleplus left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated PR description with required sections (这次改了什么/怎么验证的/风险). Re-requesting format gate review.

@MagicLizi MagicLizi added status:ci-failed CI 失败(review-pr 自动维护,仅展示) and removed status:awaiting-author 等作者修改(review-pr 自动维护,仅展示) labels Aug 27, 2026
@MagicLizi
MagicLizi dismissed their stale review August 27, 2026 14:59

这些问题已在当前 head 的后续 commit 中修复(格式门/安全门已通过),自动 dismiss 旧的 CHANGES_REQUESTED 以解除合并阻塞。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:ci-failed CI 失败(review-pr 自动维护,仅展示)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

重开旧 Claude 会话偶发 "No conversation found":invalid-resume 自愈被 CAS 误判拦截(drizzle 代理 UPDATE .returning() 恒回空数组)

2 participants