L-0273: cherry-pick 8 upstream reliability fixes - #9
Merged
Conversation
…PendingReactions fields
…te (chenhg5#1436) cleanupInteractiveState sets state.agentSession = nil under state.mu, but three Send goroutines read state.agentSession without holding the lock. When an agent process exits before the Send goroutine is scheduled, cleanup can nil agentSession, causing a nil pointer dereference panic. Fix: capture agentSession into a local variable under state.mu, then use the local in the goroutine. If the captured value is nil, the goroutine returns an error instead of panicking. Co-authored-by: tanghongliang <tanghongliang@citos.cn> Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit a4b4659)
…g5#1436 Follow-up to chenhg5#1436. The third call site in drainPendingMessages was modeled correctly in spirit (capture into local var before goroutine) but missed two details that chenhg5#1436 applied to the first two sites: 1. The capture `as := state.agentSession` happened without holding state.mu, so the same race the PR set out to fix could still nil the field between the unlock above and the capture. 2. The Send goroutine did not have a defensive `if as == nil` check, unlike the other two sites; a nil capture would still panic when the goroutine ran. Also folds the existing nil/Alive check into the post-capture path so the gating uses the local copy (consistent with the new contract). No behavior change for the happy path; in the racy path the goroutine returns an error instead of dereferencing nil, which the existing error handling already covers. Verified locally: - go vet ./... clean - go build clean - go test -race ./core -run TestCUJ_H2_TwoPlatformsConcurrentNoBleed -count=10 PASS - go test ./core -run "Drain|Queue" PASS (cherry picked from commit 5e2d501)
…Refs (chenhg5#1459) (chenhg5#1462) When a user configured a relative work_dir (e.g. "~/project" or ".cc-connect"), SaveFilesToDisk joined relative paths into the attachments directory and the resulting paths were passed verbatim into the agent's prompt. The spawned agent process — typically run from a different cwd by the platform adapter — could not resolve them and silently dropped every attachment. SaveFilesToDisk now calls filepath.Abs(workDir) up front and falls back to the raw value on error, and AppendFileRefs defensively absolutizes each entry. Both behaviors are covered by new tests for relative, absolute, and empty workDir; the empty-workDir case falls back to the process cwd so misconfigured deploys still get a writable attachments directory. Co-authored-by: dev-claudecode <dev-claudecode@cc-connect.local> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> (cherry picked from commit 7e1b53c)
(cherry picked from commit 91c3d44)
…enhg5#1451) * fix(streaming): resume stream preview after permission prompt Add streamPreview.unfreeze() and call it from the EventPermissionRequest handler after <-pending.Resolved, so subsequent EventText in the same turn opens a new streaming card instead of being buffered until EventResult. * fix(test): drop unused nextSessionEventsHook type (cherry picked from commit 230ee3c)
(cherry picked from commit f943666)
* FEAT: 添加会话空闲关闭live agent进程配置 * CHORE: 处理会话空闲关闭配置PR评审意见 * CHORE: 稳定会话空闲关闭测试 (cherry picked from commit 760079b)
filepath.IsAbs is OS-native only, so upstream's chenhg5#1459 cherry-pick (7e1b53c) mis-absolutized already-absolute /tmp/... style paths on Windows into drive-rooted paths. Treat a leading / as absolute on any OS, matching the function's documented passthrough contract.
There was a problem hiding this comment.
Pull request overview
This PR syncs (cherry-picks) a set of upstream reliability fixes into cc-connect, plus a couple fork-specific adaptations, primarily improving streaming UX around permission prompts, attachment file-path correctness, Codex app-server robustness, and per-session live-agent lifecycle management.
Changes:
- Add per-session live agent idle timeout scheduling/cleanup and persist additional session state.
- Fix attachment saving/prompt file references to consistently use absolute paths; add regression tests.
- Improve streaming behavior after permission prompts (resume streaming with a new preview card) and optionally strip agent-emitted footer lines; add unit + CUJ coverage.
- Add Codex app-server request timeout coverage that includes blocked stdin writes.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/release_local/config_matrix/config_matrix_test.go | Updates EffectiveDisplay call sites for new return value. |
| core/streaming.go | Adds unfreeze() to resume streaming previews after interruptions. |
| core/streaming_test.go | Adds regression tests for unfreeze() behavior (resume + throttle bypass + idempotency). |
| core/session.go | Persists additional session fields (provider/activity/output/reactions) in save path. |
| core/message.go | Absolutizes attachment workDir and defensively absolutizes file refs appended to prompts. |
| core/message_test.go | Adds tests covering absolute/relative/empty workDir and AppendFileRefs absolutization. |
| core/engine.go | Adds hide-agent-footer option, stream-preview resume after permission, idle-timeout lifecycle, and send/cleanup race fix alignment. |
| core/engine_test.go | Adds tests for footer stripping and agent session idle-timeout behavior. |
| core/cuj_test.go | Adds streaming-capable CUJ harness + CUJ regression for streaming resuming after permission prompt. |
| config/config.go | Adds hide_agent_footer and agent_session_idle_timeout_mins, updates EffectiveDisplay signature and validation. |
| config/config_test.go | Adds tests for new config fields and updates EffectiveDisplay call sites. |
| config.example.toml | Documents new display option and new idle-timeout option. |
| cmd/cc-connect/main.go | Wires new display and idle-timeout config into engine (including reload behavior). |
| CHANGELOG.md | Documents new agent_session_idle_timeout_mins option and absolute attachment path behavior. |
| agent/codex/appserver_session.go | Ensures request timeout includes blocked stdin writes; aborts transport on write timeout. |
| agent/codex/appserver_session_test.go | Adds regression test for request timeout including blocked stdin write. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+90
to
+96
| // workDir may be absolute or relative; the returned paths are always absolute. | ||
| // When workDir is relative, filepath.Abs resolves it against the cc-connect | ||
| // process's current working directory, so callers running from different cwd | ||
| // contexts (especially those where the agent's "workDir" is itself relative | ||
| // to the user's home, like "~/project") still get paths the agent can | ||
| // actually open. An empty workDir falls back to the process cwd, which is | ||
| // a reasonable last-resort default for misconfigured deploys. |
Comment on lines
+121
to
+127
| base := t.TempDir() | ||
| relWorkDir := filepath.Join(base, "rel") // t.TempDir() is absolute; this makes it relative after the abs() inside SaveFilesToDisk | ||
| // Actually we want a RELATIVE path here, so strip the leading slash: | ||
| // base is absolute (/tmp/xxx), relWorkDir is also absolute. To exercise | ||
| // the relative path, point workDir at "." (cwd-relative) and assert the | ||
| // returned paths are absolute. | ||
| files := []FileAttachment{{FileName: "photo.png", Data: []byte("png")}} |
Comment on lines
+184
to
+189
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| t.Fatalf("getwd: %v", err) | ||
| } | ||
| files := []FileAttachment{{FileName: "hello.txt", Data: []byte("hi")}} | ||
| got := SaveFilesToDisk("", files) |
Comment on lines
+333
to
+337
| // Required pairing: callers must invoke unfreeze() only after a matching | ||
| // freeze()+detachPreview() (or an equivalent sequence that left previewMsgID | ||
| // nil and degraded true). It is intended to be called once the user-visible | ||
| // interruption (permission prompt, AskUserQuestion) has been resolved and | ||
| // the agent is producing new output in the same turn. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
e739e2eb(throttle message recall fallback probes, fix(core): throttle message recall fallback probes chenhg5/cc-connect#1321) was skipped: its content is already onmainviab107669b, a prior sync of the same upstream PR — cherry-pick produced an empty diff and was skipped rather than committed as a no-op.test: fix processInteractiveEvents call sites after cherry-pick— two new tests from the footer-hiding commit didn't pass this fork'sdropReply boolparameter (added independently of upstream);go vetcaught the arity mismatch.fix: recognize POSIX-style absolute paths in AppendFileRefs on Windows—filepath.IsAbsis OS-native, so the [Bug] 入站图片附件提示路径与实际落盘路径不一致，agent 永远读不到文件 chenhg5/cc-connect#1459 cherry-pick's own new tests (TestAppendFileRefs_Absolutize*) failed on Windows because/tmp/...-style already-absolute paths got rewritten with a drive prefix instead of passed through. FixedAppendFileRefs(and the test's own assertion) to also treat a leading/as absolute, matching the function's documented passthrough contract. This fork runs in production on Windows, so this is a real fix, not a workaround.760079bc) merged cleanly alongside the existing ping-only heartbeat mode — confirmed the two mechanisms are orthogonal (agentSessionIdleTimeoutNanosvsExecuteHeartbeatPing), no field collision.Conflict resolution notes
core/engine.go(a4b46593): real conflict — our fork'scurrentPromptLen/currentPromptPreviewstate fields vs upstream'sas := state.agentSessionrace-fix capture landing at the same location twice (once via clean auto-merge, once via the conflict hunk). Resolved by keeping our fields and de-duplicating the capture to one copy (duplicateas :=would have been a compile error).core/engine.go(e739e2eb): trivial struct-field-ordering conflict; both sides' fields kept.core/streaming_test.go(230ee3c6): both sides added a differently-named freeze/unfreeze regression test at the same insertion point; kept both (TestStreamPreview_FreezeAndRecreate+TestStreamPreview_UnfreezeResumesStreaming+ 2 more upstream tests). All streaming tests pass.Test plan
go build ./...cleango vet ./...cleango test ./core/... ./daemon/...— 8 failures remain, all confirmed pre-existing on unmodifiedmain(Windows/environment-specific: home-path shortening, PowerShell shell-output format, nexus skill-dir fixture, path-separator normalization, schtasks file-mode simulation) — none introduced by this syncgo test ./...full suite — all platform adapter packages pass