Skip to content

fix: resume interrupted desktop turns after cli restart#468

Open
LeoLB-Wang wants to merge 1 commit into
NanmiCoder:mainfrom
LeoLB-Wang:fix/desktop-resume-interrupted-turn
Open

fix: resume interrupted desktop turns after cli restart#468
LeoLB-Wang wants to merge 1 commit into
NanmiCoder:mainfrom
LeoLB-Wang:fix/desktop-resume-interrupted-turn

Conversation

@LeoLB-Wang
Copy link
Copy Markdown

This pull request updates the logic for setting the CLAUDE_CODE_RESUME_INTERRUPTED_TURN environment variable in the ConversationService. Now, this environment variable is only set when running in SDK sessions, ensuring that interrupted turns are only resumed in that context. The tests have also been updated to verify this behavior.

Environment variable handling:

  • Updated ConversationService so that CLAUDE_CODE_RESUME_INTERRUPTED_TURN is only set to '1' when an SDK URL is present, ensuring that interrupted-turn resume is only forced for SDK sessions.

Testing improvements:

  • Added a test to confirm that CLAUDE_CODE_RESUME_INTERRUPTED_TURN is not set for non-SDK sessions, and updated existing tests to check for its presence in SDK sessions.## Summary

Feature Quality Contract

  • Changed surface: server / provider-runtime / desktop chat runtime
  • Tests added or updated:
    • 补充了同区域单元测试
    • 更新 src/server/__tests__/conversation-service.test.ts
    • 新增 SDK session 会注入 CLAUDE_CODE_RESUME_INTERRUPTED_TURN=1 的断言
    • 新增 non-SDK session 不会误注入该环境变量的断言
  • Coverage evidence:
    • 同区域测试:bun test src/server/__tests__/conversation-service.test.ts
    • 结果:16 passed / 0 failed
    • 全量 quality gate 报告:artifacts/quality-runs/2026-05-15T14-29-07-357Z/report.md
    • coverage 报告:artifacts/coverage/2026-05-15T14-31-31-634Z/coverage-report.md
    • 当前 coverage blocker:
      • server-api / agent-tools / agent-utils coverage command exited with 1
      • changed-lines 显示 src/server/services/conversationService.ts0% (0/10)
  • E2E / live-model evidence:
    • 已完成浏览器/桌面聊天路径手工复测
    • 复测场景:中断当前生成或切换模型导致 CLI restart 后,后续消息不应“忘记”之前被中断的用户输入
    • 复测 session:4027e2dc-751f-4d88-90ca-35252e2ab4e4
    • 未运行 live provider smoke:本次修复聚焦本地 desktop SDK CLI restart/resume 逻辑,未涉及真实 provider 联通性行为变化

修改前测试截图(历史消息会丢失):
image

修改后测试截图(历史消息不丢失):

image
  • Known risk / rollback:
    • 风险较低,改动仅作用于 desktop SDK child process,在 sdkUrl 存在时注入 CLAUDE_CODE_RESUME_INTERRUPTED_TURN=1
    • 不影响 non-SDK CLI 启动路径
    • 若出现意外回归,可直接回滚 src/server/services/conversationService.ts 中该环境变量注入逻辑

Verification

  • I ran the relevant local checks, or explained why they do not apply.
  • I added or updated same-area tests for every production behavior change.
  • I ran bun run verify for code changes, including the coverage gate.
  • New or changed executable production lines meet the changed-line coverage threshold, or the blocker/maintainer override is documented.
  • I attached or summarized the quality report path, JUnit/log artifact path, and pass/fail/skip counts.
  • I ran E2E/live smoke for cross-boundary, provider/runtime, desktop chat, agent-loop, native, or release changes, or documented the blocker.

Verification notes:

  • same-area test passed:
    • bun test src/server/__tests__/conversation-service.test.ts
    • result: 16 passed / 0 failed
  • full quality gate:
    • report: artifacts/quality-runs/2026-05-15T14-29-07-357Z/report.md
    • junit: artifacts/quality-runs/2026-05-15T14-29-07-357Z/junit.xml
    • logs dir: artifacts/quality-runs/2026-05-15T14-29-07-357Z/logs/
    • summary: passed=6 failed=2 skipped=2
  • passed lanes:
    • impact-report
    • policy-checks
    • desktop-checks
    • native-checks
    • persistence-upgrade
    • quarantine
  • failed lanes:
    • server-checks
    • coverage
  • current blockers:
    • coverage 仍未通过,详见 artifacts/coverage/2026-05-15T14-31-31-634Z/coverage-report.md
    • changed-linessrc/server/services/conversationService.ts 仍显示 0% (0/10)
    • quality gate 同时标记了 server-checks failed,详细原因见对应 lane log

Risk

  • This PR does not touch CLI core paths, or it has maintainer approval for allow-cli-core-change.
  • Production code changes include matching tests, or have maintainer approval for allow-missing-tests.
  • Coverage baseline/threshold changes have maintainer approval for allow-coverage-baseline-change.
  • Quarantined tests still have owners, exit criteria, and unexpired review windows.
  • Provider/runtime changes were covered by mock contract tests, and live smoke was run or explicitly deferred.

Additional notes:

  • 该改动没有修改 transcript format、resume 主流程或非-SDK CLI 行为
  • 修复目标是 desktop 在 stop generation / runtime switch / reconnect 等触发 CLI in-place restart 的场景下,正确恢复 interrupted turn

@dosubot review this PR for changed-area risk, missing tests, docs impact, desktop startup risk, and CLI core impact.

@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label May 15, 2026
@github-actions
Copy link
Copy Markdown

PR quality triage

Changed areas: area:server

CLI core policy: No CLI-core policy block detected.

Missing-test policy: Blocked by policy until a maintainer applies allow-missing-tests or matching tests are added.

Coverage baseline policy: No coverage-baseline policy block detected.

CLI core files:

  • none

Coverage policy files:

  • none

Expected checks:

  • change-policy
  • desktop-checks
  • server-checks
  • desktop-native-checks
  • coverage-checks

Test coverage signals:

  • BLOCKING unless allow-missing-tests is applied: Agent/runtime product files changed without a tools/utils test file in the PR.
  • Agent/model runtime path changed: use mock/request-shape tests in PR and maintainer live-model smoke before release.

Risk notes:

  • Session runtime changed: review reconnect, startup diagnostics, provider selection, and thinking settings.

Hard merge gates still come from GitHub Actions, not AI review.

Dosu handoff: Dosu can be used as the AI reviewer for risk explanation, missing-test prompts, and maintainer Q&A. If it does not comment automatically from the PR template, ask:

@dosubot review this PR for changed-area risk, missing tests, docs impact, desktop startup risk, and CLI core impact.

@dosubot
Copy link
Copy Markdown

dosubot Bot commented May 15, 2026

Here's my assessment of this PR across the requested dimensions:

Changed-Area Risk: Low
The change is minimal (~10 lines of production code) and well-scoped. It conditionally injects CLAUDE_CODE_RESUME_INTERRUPTED_TURN=1 into the CLI child process environment only when sdkUrl is present [1]. The consumer of this flag is a single location in src/cli/print.ts [2], which already guards on both the env var and a valid turnInterruptionState. The conditional spread pattern (...(sdkUrl ? {...} : {})) is safe and idiomatic.

Missing Tests

  • ✅ Positive case (SDK session sets the flag) — covered
  • ✅ Negative case (non-SDK session does NOT set the flag) — covered
  • ⚠️ Coverage gate is failing: the PR itself documents 0% (0/10) changed-line coverage for conversationService.ts. This likely means the test runner isn't instrumenting the actual source file (possibly a build/path mismatch in the coverage config). The same-area unit tests pass, but the coverage tooling doesn't see them exercising the changed lines. This should be investigated or documented as a known blocker before merge.
  • 💡 Consider adding: an integration-level test that spawns a real CLI child process (or a lightweight mock) and asserts that process.env.CLAUDE_CODE_RESUME_INTERRUPTED_TURN is visible inside it. This would close the gap between the unit test (which tests buildChildEnv() return value) and the actual subprocess behavior.

Docs Impact: None
This is an internal runtime behavior change with no user-facing API surface, configuration knob, or documentation reference that needs updating.

Desktop Startup Risk: Negligible
The injection happens inside buildChildEnv(), which is called during session creation—not during the desktop app's own startup/render path [3]. The conditional only adds one extra key to the env object; it cannot throw or block. Desktop cold-start latency is unaffected.

CLI Core Impact: None

  • The flag is never injected for non-SDK sessions, so terminal CLI behavior is completely unchanged.
  • The CLI entry chain (bin/claude-hahacli.tsxmain.tsx) [4] [5] is untouched.
  • The resume logic in print.ts already existed and is gated behind the env var check—this PR doesn't modify that code path.

Summary: This is a low-risk, well-scoped fix. The main concern is the coverage gate failure (0% changed-line coverage). I'd recommend either resolving the coverage instrumentation issue or getting a maintainer override (allow-coverage-baseline-change) before merging, since the tests clearly exercise the logic but the tooling doesn't detect it.

To reply, just mention @dosu.


Docs are dead. Just use Dosu.

Leave Feedback Ask Dosu about cc-haha Share Dosu with your team

@LeoLB-Wang
Copy link
Copy Markdown
Author

补充本地审查备注:

  • server-checks 仍然在 src/server/__tests__/desktop-cli-launcher.test.ts 中报告失败
  • 失败的断言期望的是一个临时 HOME 目录下的 zsh 配置路径,但本地测试环境中解析到的却是 /Users/wlb/.zshrc
  • 这处问题出现在本次 PR 修改的代码路径(ConversationService.buildChildEnv)之外,似乎是 launcher 测试本身已存在的、依赖环境的老问题

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

Labels

area:server needs-maintainer-approval size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant