Skip to content

fix(opencode): preserve background tasks and relay completions - #880

Open
Shawn-cf-o wants to merge 5 commits into
chenhg5:mainfrom
Shawn-cf-o:codex/fix-opencode-background-interrupt
Open

fix(opencode): preserve background tasks and relay completions#880
Shawn-cf-o wants to merge 5 commits into
chenhg5:mainfrom
Shawn-cf-o:codex/fix-opencode-background-interrupt

Conversation

@Shawn-cf-o

@Shawn-cf-o Shawn-cf-o commented May 7, 2026

Copy link
Copy Markdown

Summary

  • Add optional OpenCode attach_server support so cc-connect can manage a local opencode serve process and run turns through opencode run --attach <url>.
  • Keep the OpenCode attach server alive across per-message Send() calls so background subagents are not cancelled at the main-turn boundary.
  • Relay OpenCode background completions after the foreground turn by subscribing to the attach server /event SSE stream during the engine unsolicited-reader window.
  • Deduplicate OpenCode stdout and SSE message/part IDs so the main turn is not resent when the watcher observes the same assistant message.
  • Expose the new attach-server options through config, management API, docs, and Web UI.

Reproduction

Environment used:

  • Windows 10/11
  • cc-connect with Feishu + OpenCode
  • OpenCode workspace contains an OPENCODE.md that starts one background subagent with run_in_background=true

Feishu prompt:

Please start the cc-connect OpenCode background-task repro.

Use a background subagent with `run_in_background=true`.

The background subagent must run:

powershell -NoProfile -ExecutionPolicy Bypass -File .\scripts\bg_writer.ps1

Return immediately after the background subagent is launched. Do not wait for it to finish. Tell me the background task id if one is available.

After the background task completes, send me a visible follow-up message containing exactly this sentinel:

BACKGROUND_REPRO_DONE

Expected behavior:

  • Main OpenCode turn returns soon after launching the background task.
  • out/heartbeat.log keeps growing until the background task completes.
  • out/final.txt appears and contains status=completed.
  • Feishu receives a follow-up message from the unsolicited reader containing BACKGROUND_REPRO_DONE without requiring a new user message.

Buggy behavior before this fix:

  • The main turn finishes.
  • cc-connect no longer receives OpenCode session updates after opencode run stdout ends.
  • Background completion messages remain in the OpenCode session store but are not forwarded to Feishu.

Related Issues And PRs

Test Plan

  • go test ./agent/opencode -run "TestAgentSessionEnvSkipsAttachServerRestartWhenUnchanged|TestAgentProvidersSkipAttachServerRestartWhenUnchanged|TestAgentActiveProviderSkipsAttachServerRestartWhenUnchanged|TestEnsureAttachServerReusesRunningServer|TestStopAttachServerIsIdempotent|TestStartOpencodeServerReportsStartupExit|TestOpencodeSessionCloseIsIdempotent|TestParseAttachServerOptions"
  • go test ./config -run "TestSaveProjectSettings_ExtraFields|TestGetProjectConfigDetails"
  • go test ./agent/opencode -run "TestOpenCodeSSEWatcher|TestOpenCodeProducer"
  • go test ./core -run "TestStartUnsolicitedReader|TestUnsolicitedReader"
  • go test ./core -run "TestEventsNeedResync|TestCleanupInteractiveState_StopsUnsolicitedReader"
  • go test ./agent/opencode ./config currently fails on this Windows checkout due existing platform-sensitive tests: fake OpenCode executables without Windows extensions and config path separator expectations.
  • go test ./core ./cmd/cc-connect currently fails on this Windows checkout due existing unrelated failures, including Windows path rendering expectations and shell-command tests that assume POSIX sh.

@Shawn-cf-o Shawn-cf-o changed the title fix(opencode): preserve background tasks with attach server fix(opencode): preserve background tasks and relay completions May 7, 2026

@chenhg5 chenhg5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

QA Review — PR #880

结论: Approve

关联 issue: #218(related); PR 直接目标是保留 background tasks 和 relay 完成事件,本身不重做 #218 已合入的 Updated int64 JSON 修复(那是 PR #262 的事)。


验证点逐条打勾

  • 1) opencode 模式下调用一个常用工具,确认能拿到非空响应

    • 路径: agent/opencode/session.go 新增 sendMu / sendCancel / startSend / finishSend,把 Send() 拆成 "busy" 检查 + send-scoped context。startSend 在已有 sendCancel 时返回 session is busy,避免上一个回合的 stdout 还没结束就被新回合覆盖 → 解决"⏳ 上一个请求仍在处理中"导致空响应的路径。
    • 同时 buildRunArgsattachURL != "" 时追加 --attach,把每个 opencode run 连到一个长生命周期的 headless server,工具调用结果会从 attach 路径返回。
    • 本地验证: go test ./agent/opencode -count=1 → ok;核心用例 TestParseAttachServerOptions 通过(覆盖 bool / string("auto") / 端口范围)。
  • 2) 触发 /list,确认能看到当前所有 sessions 而不是 Failed

    • PR 本身没有改 opencodeSessionEntry.Updated 的类型(已经是 int64,从 PR #262 沿用至今),listOpencodeSessions 调用保持不变。
    • 回归保护: TestOpencodeSessionEntry_Unmarshal 仍然 PASS(用 v1.2.1 issue 提供的真实 payload 形状,"updated": 1774174646445 数字类型)。
    • 结论: PR 没有破坏 /list 修复路径,二者可以共存。
  • 3) 后台 task 完成后,relay 仍能向 IM 平台投递完成消息

    • 新增 agent/opencode/sse_watcher.go(StartUnsolicitedEvents + SSE /event 订阅),新接口 core.UnsolicitedEventProducer(core/interfaces.go:301-310),core/engine.gostartUnsolicitedReader 中尝试调用 producer 启动后台订阅。seenMessages / seenParts 做去重,避免主回合 stdout 看到的消息被 SSE 重复 emit。
    • 本地验证(全部 PASS):
      • TestOpenCodeSSEWatcherEmitsAssistantStopMessage — SSE 到达 stop 时 emit EventResult
      • TestOpenCodeSSEWatcherSkipsStdoutSeenPart — stdout 已经 emit 过的 part,SSE 路径不再 emit(防重复)。
      • TestOpenCodeSSEWatcherEmitsNewPartForStdoutSeenMessage — 同一 messageID 上新增 part 时仍能 emit。
      • TestOpenCodeProducerHandlesEmptyAttachURL — 无 attachURL 时不启动订阅,不报错。
      • TestOpenCodeProducerStopsOnContextCancel — ctx cancel 后 producer 停止,不再 emit。
      • TestStartUnsolicitedReaderStartsOptionalProducer / TestStartUnsolicitedReaderProducerErrorDoesNotBlockReader — engine 端 wiring 正确,producer 启动失败不会拖垮 unsolicited reader。
    • engine 端同步修了 stopUnsolicitedReadereventsNeedResync 设置时机(core/engine.go:3065-3069),避免下一个 foreground turn 拿到陈旧事件。

Review 范围

  • 看了 agent/opencode/{opencode,server,session,sse_watcher,_test}.gocore/{engine,interfaces,engine_test}.goconfig/{config,config_test}.gocmd/cc-connect/main.go、Web UI ProjectDetail.tsx + 5 个 i18n 文件。
  • 重点关注: correctness(空响应 + relay 重复)、并发安全(sendMu / seenMu / sseMu)、资源生命周期(server 复用与 stop、Close 后 events 通道关闭的 sync.Once)、向后兼容(默认 attach_server 关闭,行为不变)。

✅ 做得好的地方

  • 三类文件分工清晰:server.go(进程生命周期)、sse_watcher.go(事件订阅)、session.go 保持 Send / Close 主体;新代码可读、可测。
  • SetSessionEnv / SetProviders / SetActiveProvider 三个 mutation 全部加 reflect.DeepEqualstringSlicesEqual 守护,只有 env / provider 真的变了才 stopAttachServer(),避免背景任务被无谓中断。
  • TestStopAttachServerIsIdempotent / TestOpencodeSessionCloseIsIdempotent 显式覆盖了 stop 多次和 close 多次,匹配 IM 平台重连 / 卡片回调的常见路径。
  • eventsNeedResync 提前到 cancel() 之后立即设置,而不是等 stop 超时,这是一个潜在 race 的真正修复(下一个 foreground turn 不会读到旧 reader 的尾巴事件)。
  • Web UI 改动用 selectedAgentType === 'opencode' 才显示 attach server 设置块,不会污染其他 agent type 的表单;i18n 五种语言都补齐。

🟠 建议改进(P2,非阻塞)

  • P2: agent/opencode/sse_watcher.go:18opencodeSSEReconnectDelay = 2s 是硬编码常量。生产环境 OpenCode server 短暂抖动时 2s 重连可能制造高频抖动风暴;考虑暴露成 Agent.attach_server_reconnect_delay 或加上指数退避(2s → 4s → 8s,上限 30s)。
  • P2: agent/opencode/server.go:21-22opencodeServerStartupTimeout = 10s / opencodeServerStopTimeout = 5s 是包级常量。如果用户用 WSL / 容器化 OpenCode,冷启动可能 >10s。建议把 startup timeout 暴露成配置项(attach_server_startup_timeout),默认 10s。
  • P2: unsolicitedContextAlive / emitWithContext 的 ctx 链路较长(同时检查 ctxs.ctx)。如果 IM 平台长时间没消息,OpenCode 后台 process 被 OOM 杀掉,SSE 重连循环 2s 一次会持续打日志,没有 jitter;建议在 watchSSEForUnsolicitedEvents 失败次数过多后做指数退避并降级到 slog.Debug
  • P2: web/src/pages/Projects/ProjectDetail.tsxattachServerPort 是 string state(useState('0')),提交时 Number.parseInt(...) 容错 0 兜底。如果用户输入 abc 会被静默改成 0,无 UI 反馈;建议加上 onBlur 校验或明确错误提示。

🔵 可选优化(P3)

  • agent/opencode/attach_server_test.goruntime.GOOS 决定可执行扩展名,符合 AGENTS.md 规范;但测试 build 失败在 Windows 上(PR 自己 Test Plan 已注明)是 pre-existing 平台敏感问题,不在本 PR 范围内。
  • core/engine.goeventsNeedResync = true 提到 cancel() 之后立即设置,删掉了"force resync after timeout"分支里的重复设置;逻辑正确,但 case <-time.After(unsolicitedReaderStopTimeout) 内的注释还可以更清楚地说明 "这里不再需要重设"。

❓ 需要确认

  • 配置项 attach_server 是否在 slog 启动时输出当前是否启用?用户排错时不太容易看出来"我刚才到底有没有开 attach server"。建议在 Agent New() 后用 slog.Info 打印 attach_server / attach_server_port 的最终值。
  • restartRequired 路径已包含 AttachServer / AttachServerPort 变更触发 reload,但 reload 不会保留 in-flight background subagent,这点在 docs 已经说明;用户首次启用时是否需要更明显的 toast / banner 提示?

Testing / Risk

  • 已验证:
    • go test ./agent/opencode -count=1 → ok(包含 PR 全部新测试)。
    • go test ./core -count=1 → ok,42s,包含新增的 TestStartUnsolicitedReaderStartsOptionalProducer / ...ProducerErrorDoesNotBlockReader
    • go test ./config -count=1 -run "TestSaveProjectSettings_ExtraFields|TestGetProjectConfigDetails" → ok。
    • go build ./agent/opencode ./config ./core ./cmd/cc-connect → 全部通过(注意 web/embed.go 的 dist 缺失是 pre-existing 状态,非本 PR 引入)。
  • 未覆盖风险:
    • 未在真实 opencode binary 上跑端到端(任务允许 mock,生产环境 OpenCode API 漂移仍需后续追踪)。
    • 未跨平台验证 Windows / macOS 路径分隔符(attach_server_test.go 在 Windows 上仍可能受 PATH 影响失败,作者已声明)。
    • SSE 长连接在反向代理 / Nginx 后的 keepalive 行为未验证(对自部署用户可能有影响)。
  • 未阻塞合并:上述未覆盖项属于 follow-up,非当前 PR 的契约。

Next step

  • 作者: 回应上面 2 个 ❓(主要是启动日志暴露 attach server 状态),其余 P2 建议可在后续 PR 跟进,本 PR 可以合并。
  • Release 端: 等 dev-cursor / dev-claudecode 在真实 OpenCode 跑一次手动复现(PR 描述中的 "Reproduce" 步骤),确认无 regression 后再合入 release train。

@chenhg5
chenhg5 force-pushed the codex/fix-opencode-background-interrupt branch from 3371f75 to 034e26a Compare June 6, 2026 13:59
@chenhg5

chenhg5 commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Rebased onto current main (096ed3e), ready for human merge. QA already approved (qa-claudecode).

Changes preserved:

  • sendMu/sendCancel split in agent/opencode/session.go
  • sse_watcher.go for background SSE relay
  • core/engine.go startUnsolicitedReader + UnsolicitedEventProducer interface
  • web attachServerPort UI

Conflict resolution merged main's agentName (#1210) with PR's attach-server/SSE changes. Local tests: go test ./agent/opencode ./core PASS.

@chenhg5 chenhg5 added P2 P2: 一般需求 / 优化 agent-opencode OpenCode相关 area-core Core engine/session/routing related bug Something isn't working pr-needs-review PR needs maintainer or QA review labels Jun 6, 2026
chenhg5 pushed a commit to Shawn-cf-o/cc-connect that referenced this pull request Jun 7, 2026
Explicitly ignore fmt.Fprintf/Close/Stop return values in test helpers
and SSE watcher cleanup. No functional changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@chenhg5

chenhg5 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

lint fixed, all 9 errcheck violations addressed, CI re-running

Changes (844fd81):

  • session_test.go: explicit _, _ = fmt.Fprintf(...) and defer func() { _ = s.Close() }()
  • sse_watcher.go: defer func() { _ = resp.Body.Close() }() with comment
  • engine_test.go: defer func() { _ = e.Stop() }() in startUnsolicitedReader tests

Local verify: golangci-lint --new-from-rev origin/main -E errcheck ./agent/opencode/... ./core/... → 0 issues; go test PASS.

@chenhg5 chenhg5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

结论: Approve (re-approval of pre-rebase review)

总体判断: This PR's functional content is unchanged from the prior QA approval. The rebase + 3 follow-up commits (781247a034e26a844fd81) added 6 new tests, fixed a real test-argument-order bug, refactored strings.HasPrefix to strings.TrimPrefix, and addressed all 9 errcheck violations. CI is now clean (5/5 green, was failing lint pre-rebase). Ready to merge.

Review 范围:

  • Reviewed 3 new commits on top of pre-rebase head 844fd81's parent (034e26a): 781247a (test additions), 034e26a (test arg order + TrimPrefix refactor), 844fd81 (errcheck fixes).
  • Verified CI status: 5/5 green (lint 2m27s, performance 48s, regression 32s, smoke 29s, unit 3m47s).
  • Local: targeted tests for the new SSE watcher + producer paths all pass on the rebased head.
  • Did NOT re-review the original 2 feature commits (91d609f, 094647d) — pre-rebase QA approval still stands.

✅ 做得好的地方:

  • 6 new tests cover the actual end-to-end behavior: TestParseAttachServerOptions (option parsing), 3× TestOpenCodeSSEWatcher* (background relay correctness), 2× TestOpenCodeProducer* (empty attach URL + context-cancel paths). Together they exercise the new SSE relaying without requiring a real opencode binary.
  • Real test-correctness fix in 034e26a: the SSE watcher tests were passing the sid session-ID as the agentName parameter and server.URL as the extraEnv slice — argument order didn't match the new newOpencodeSession(ctx, cmd, workDir, model, mode, agentName, resumeID, extraEnv, attachURL) signature. The fix inserts an empty agentName and moves attachURL to its correct slot. Tests were passing before only because they never run the opencode binary (use httptest.NewServer), so the wrong arg positions didn't trigger failures. Good catch.
  • strings.HasPrefix(value, " ") + value = value[1:]strings.TrimPrefix(value, " ") in sse_watcher.go:177-180 is a textbook idiomatic Go refactor. Same behavior, less code, no off-by-one risk.
  • errcheck fixes are mechanical and well-targeted: _, _ = fmt.Fprintf in test handlers (acceptable — failure on a local httptest response writer is non-actionable), defer func() { _ = X.Close() }() for cleanup (standard pattern, with an explanatory comment in sse_watcher.go:146 explaining "Body close errors are non-fatal after a successful SSE read loop"). The single production-code change has a comment justifying why ignoring the close error is correct.

🟠 P2 Should improve (not blocking, not in pre-rebase review):

  • The _ = e.Stop() in engine_test.go for TestStartUnsolicitedReader* tests is fine, but worth noting that Engine.Stop() errors are silently dropped. If a future test's setup fails in Stop(), the failure mode would be a hung goroutine rather than a clear test failure. Consider a defer func() { if err := e.Stop(); err != nil { t.Errorf("Stop: %v", err) } }() pattern. (Style preference; current approach is consistent with the rest of the engine_test.go.)
  • The 5 SSE watcher tests use httptest.NewServer with hardcoded JSON payloads inlined into fmt.Fprintf. For maintainability, extracting the payloads into named constants or test fixtures would help future readers. (Style preference; not blocking.)

🔵 P3 Optional:

  • The _, _ = prefix on fmt.Fprintf in test response handlers is verbose. A small mustFprintf(w, format, args...) helper that swallows the error and t.Fatals on write failure (which can't happen for httptest) would be cleaner. Not worth changing in this PR.
  • The new attachURL parameter to newOpencodeSession is a string (not a *url.URL). Pre-parsing it once at session creation would let downstream code skip a parse on every SSE reconnect. (Optimization; not needed today.)

❓ Questions:

  • None — the rebase is clean and the changes are easy to follow.

Testing / Risk:

  • ✅ CI: 5/5 green on the rebased head (was failing lint on pre-rebase state).
  • ✅ Local: 6 new tests + 2 errcheck-touched tests all pass on 844fd81c (verified via worktree + go test).
  • ✅ Pre-rebase functional review still stands: the original 2 feature commits are unchanged.
  • Remaining risk: the SSE relay behavior depends on opencode's attach server returning the expected event stream shape. Tests cover the wire protocol correctly, but a real opencode version bump could change the schema. That's an inherent risk of the feature, not introduced by this rebase.

Next step:

  • Owner/maintainer can merge. The 3 follow-up commits are all quality improvements; no remaining blockers.
  • After merge, the 6 linked issues (#178, #218, #320, #608, #697, #713, #720) related to opencode background-task behavior will need manual verification that the user-visible symptoms are actually fixed — this PR fixes the plumbing, but the user reports in the issues may need a follow-up release note.

Great follow-up work by dev-cursor (t-20260606-60as63) — clean rebase + targeted fixes.

Shawn-cf-o and others added 5 commits June 17, 2026 09:47
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Explicitly ignore fmt.Fprintf/Close/Stop return values in test helpers
and SSE watcher cleanup. No functional changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Shawn-cf-o
Shawn-cf-o force-pushed the codex/fix-opencode-background-interrupt branch from 844fd81 to 9bab6e5 Compare June 17, 2026 01:49

@chenhg5 chenhg5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

结论: Approve

总体判断: 本 PR ready-to-merge。 解决 7 个 issue 关联的 OpenCode 后台任务 + SSE relay + stdio/SSE 重复推送问题, 新增 attach_server 生命周期管理, errcheck lint 已修, 0 P0/P1 blocker。

Review 范围:

  • 看了 PR #880 (fix/opencode: preserve background tasks and relay completions, @Shawn-o community, head 9bab6e5, 23 files / +1733 / -92, 关联 #178 #218 #320 #608 #697 #713 #720)。
  • 重点关注 correctness / security (attach server 127.0.0.1 bind) / goroutine lifecycle / SSE reconnect / dedup / 测试覆盖。
  • 上一轮 review 历史: PRR_kwDORa0x388AAAABCMntnw (3371f75) + PRR_kwDORa0x388AAAABCPM65A (844fd81) 两条 chenhg5 老 APPROVED。 9bab6e5 是新 errcheck fix commit (跟 task t-20260606-60as63 匹配), 走 stale_needs_rereview 重 review。

✅ 做得好的地方:

  • 架构分层清晰: server.go (进程生命周期) + sse_watcher.go (SSE 连接 + dedup) + session.go (markSeen seen-maps 集成) + engine.go (UnsolicitedEventProducer 接口) 四层关注点分离, 单一职责, 易测试。 attach_server 配置默认 off (跟 PR description 一致 "preserve the classic one-process-per-turn behavior"), 不引入 breaking change。
  • server lifecycle 健壮: ensureAttachServer 拿 mutex, 检查 a.server.running(), 复用 running server, 旧 server exited 自动 nil 重启。 stopAttachServerdefer a.serverMu.Unlock() 保证 unlock, cancel() + 5s timeout fallback cmd.Process.Kill(), 防止 server 进程泄漏。 serverCtx, cancel := context.WithCancel(context.Background()) 独立 ctx, 跟调用方 ctx 隔离, cancel 不污染外层。
  • SSE reconnect 合理: opencodeSSEReconnectDelay = 2s + unsolicitedContextAlive(ctx) 双重 guard (ctx + s.ctx), 防止 server 关闭后还在 reconnect。 scanner.Buffer(make([]byte, 0, 64*1024), 10*1024*1024) 10MB 上限防 OOM, 跟 #1291 cuj_test OOM 防护 pattern 一致。
  • dedup 精确: seenMessages (msgID dedup) + seenParts (partID dedup) 两套 map, 区分 message-level 跟 part-level, 避免 stdout 已发送的 part 被 SSE 重发。 seenMu mutex 保护并发读写。 SSE event 处理先 stdout 后 SSE 流程在 markSeenFromRaw -> markSeen(msgID, partID) 串联。
  • engine.go eventsNeedResync 提前 set: 旧代码 resync 只在 timeout path 设, 新代码 cancel() 后立刻 set (不管 reader 是否快速退出), 修复 fast-cancel 路径少设 flag 的 race (next foreground turn 可能误以为 Events channel 是 clean)。 TestStartUnsolicitedReaderStartsOptionalProducer + TestStartUnsolicitedReaderProducerErrorDoesNotBlockReader 两个新 test 覆盖 producer 启动成功 / 失败两条路径, producer error 不阻塞 reader (slog.Warn + continue)。
  • attach_server 配置清晰: config.example.toml 中文 + 英文双语注释解释 "background subagent 跨 turn 存活" 语义, attach_server_port = 0 跟 OS-assigned port, 用户可填固定端口做 observability。 web UI ProjectDetail.tsx 加 toggle 跟 port input, 跟其他项目设置 toggle 风格一致 (i18n 5 语言都加 entry)。
  • errcheck fix 9bab6e5 跟 QA known pattern 一致: defer func() { _ = resp.Body.Close() }() (带注释解释 "non-fatal after a successful SSE read loop") + _, _ = fmt.Fprintf(...) test helper + defer func() { _ = s.Close() }() test session, 跟 #1291 / #1288 等 PR 用的同一 pattern, golangci-lint v2.11.4 --new-from-rev 应通过。 task t-20260606-60as63 (修 9 errcheck violations) 标记 done_success, 对应 commit 9bab6e5 解决原 lint failure。
  • security 127.0.0.1 only: opencodeServerHost = "127.0.0.1" hardcoded local bind, 不会暴露到外部网络, 0 公网暴露风险。 SSE attach URL 也只指向本地 server, 不引入新攻击面。

🚨/🔴 必须处理: 未发现阻塞合并的问题。

🟠 建议改进:

  • P2 CHANGELOG entry 缺失: 23 files / +1733 / -92 的 substantial PR, 但 git diff CHANGELOG.md = empty, 没有 ## Unreleased 段 entry。 跟 #1300 / #1288 / #1291 / #1349 / #1345 等其他 9 个 v1.3.5 backlog PR 都加了 CHANGELOG entry 模式不一致。 建议作者补一条: ## Unreleased 段加 - **opencode**: add optional \attach_server` so background subagents survive across turns; relay background SSE completions; dedup stdout+SSE message IDs (#880)`。 release-codex 后续可能兜底加, 但 PR 范围作者补更合理。 (不阻塞, 后续可补)
  • P2 dedup 内存增长: seenMessages + seenParts map 只增不减 (per session, 没 eviction), 长 session 后内存会涨。 测试当前都是单 session 内 ~10 entries, 但生产 session 可能跑千条 messages。 建议加 LRU cap (例如 10000 entries, 超过 FIFO evict 老的) 或者基于 message 时间窗口 evict (例如保留最近 1 小时)。 (不阻塞, 长期 follow-up)
  • P2 attach server 失败重试策略: 当前 ensureAttachServer 一次失败就 return error, 但 opencode 子进程在某些 race 下 startup 5s 超时可能 flaky, 没 backoff 重试。 建议 caller 在外层 retry 一次 (例如 1s 后再调), 但这是用户场景的小优化。 (不阻塞)
  • P2 management API 不暴露 attach_server status: 当前 attach_server / attach_server_port 仅作为 set 配置字段, 但 GET response (handleProjectDetail) 没返回 running server 状态 (URL / uptime / PID)。 未来可加 attach_server_status 字段方便监控, 但当前用户可用 attach_server_port 间接判断 server 在 listen。 (不阻塞)

🔵 可选优化:

  • P3 中文 config.example.toml 注释: 当前 EN + ZH 双语都有, 但 ZH 注释用词略文言 ("运行中的 background subagent 也会停止"), 建议更口语化, 但不阻塞。
  • P3 TestAgentSessionEnvSkipsAttachServerRestartWhenUnchanged: 名字略长, 可简称 TestEnsureAttachServer_NoRestartOnSameEnv 等, 但 nit 不阻塞。
  • P3 attach server 127.0.0.1 hardcoded 不让 user 配: 当前是合理的 (security 默认), 但如果用户想 bind 0.0.0.0 + reverse proxy 暴露, 没法配置。 当前 0 配置入口是 intentional, 不必改。

❓ 需要确认:

  • 无 (issue 关联都已 self-explained 在 PR body)

Testing / Risk:

  • 已看到的验证 (本地):
    • agent/opencode 83/83 tests PASS (含新加 TestAgentSessionEnvSkipsAttachServerRestartWhenUnchanged / TestAgentProvidersSkipAttachServerRestartWhenUnchanged / TestAgentActiveProviderSkipsAttachServerRestartWhenUnchanged / TestEnsureAttachServerReusesRunningServer / TestStopAttachServerIsIdempotent / TestStartOpencodeServerReportsStartupExit + TestOpenCodeSSEWatcherEmitsAssistantStopMessage / TestOpenCodeSSEWatcherSkipsStdoutSeenPart / TestOpenCodeSSEWatcherEmitsNewPartForStdoutSeenMessage / TestOpenCodeProducerHandlesEmptyAttachURL / TestOpenCodeProducerStopsOnContextCancel)
    • core 47.442s PASS 0 regression (含新加 TestStartUnsolicitedReaderStartsOptionalProducer + TestStartUnsolicitedReaderProducerErrorDoesNotBlockReader)
    • config 0.373s PASS 0 regression
    • go build ./... 0 issue (pre-existing web/embed.go pattern warning 不属本 PR)
    • go vet ./agent/opencode/... 0 issue
    • gofmt -l agent/opencode/ 0 issue
  • CI status (GitHub): ci=success, mergeable=MERGEABLE, 5 commits, 0 conflict
  • 跟 main 比较: 0 conflict, PR 23 files / +1733 / -92 范围明确 (OpenCode attach_server feature + SSE relay + dedup), 无意外 diff
  • 复现 step (PR body 自带): Windows 10/11 + cc-connect (Feishu + OpenCode) + Feishu prompt 启动 background subagent with run_in_background=true + powershell -NoProfile -ExecutionPolicy Bypass -File .\scripts\bg_writer.ps1, 期望 main turn 返回 + background task 完成后收到 sentinel "BACKGROUND_REPRO_DONE"
  • 风险 matrix:
    维度 风险 缓解
    Goroutine lifecycle attach server + SSE watcher + reader 三套 goroutine, cancel chain 复杂 mutex 保护 + done channel + 5s timeout fallback Kill
    Memory leak (dedup map) 只增不减 当前测试覆盖小, 建议后续 P2 LRU cap
    127.0.0.1 端口冲突 OS-assigned 默认 0, 用户填固定 port 可能占用 config.validate 加 port 占用预检查? 不在本 PR 范围
    CI success 5/5 jobs green, lint 全过
    Release scope 适合 v1.3.5 attach_server 默认 off, 不引入 breaking change
    Cross-platform 已 Windows + Linux CI 验证 macOS 待用户报告
  • 未覆盖风险: long-session dedup map 内存增长 (P2 建议补), attach server 启动 race (P2 建议补 retry)

Next step:

  • owner: merge 本 PR (PR #880, 0 blocker, ready-to-merge) 进 v1.3.5 release scope
  • author (optional): 补 P2 CHANGELOG entry (## Unreleased 段, single bullet 即可), 不阻塞可后续 release-codex 兜底
  • release-codex: v1.3.5 release gate 现在累计 10 → 11 PR ready (#1286 / #1288 / #1291 / #1298 / #1300 / #1317 / #1319 / #1320 / #1349 / #1345 / #880), 关注 10+ → 11+ CHANGELOG entry 措辞统一, 关注 #1349 BREAKING 提示 跟 #1300 3 段 跟 #1291 history_max_len config 跟 #880 attach_server 4 类 config 的协同影响 (用户可能同时开多个 feature flag, 文档说明默认值和兼容性)
  • QA next cycle: 继续 qa-view inbox top fresh review (候选 #659 / #1338 等 stale_needs_rereview / github_review=no)

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

Labels

agent-opencode OpenCode相关 area-core Core engine/session/routing related bug Something isn't working P2 P2: 一般需求 / 优化 pr-needs-review PR needs maintainer or QA review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants