Repository navigation
fix(wework): stop Codex tasks during startup - #2290
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCodex shared app-server cancellation now sends a thread-level interrupt with an empty turn ID during the startup window. Runtime tests cover this behavior, while desktop E2E tests defer completion events and verify cancelled tasks remain stopped. ChangesCodex cancellation flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RuntimeTask
participant CodexExecutor
participant CodexAppServer
participant DesktopE2EServer
RuntimeTask->>CodexExecutor: request cancellation
CodexExecutor->>CodexAppServer: turn/interrupt with empty turnId during startup
CodexAppServer-->>CodexExecutor: interruption confirmation
CodexExecutor-->>RuntimeTask: report cancellation
DesktopE2EServer->>DesktopE2EServer: wait for cancellation completion release
DesktopE2EServer-->>RuntimeTask: emit deferred completion events
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 9781-9809: The delayed-completion assertions should await an
explicit completion-observed promise instead of the fixed one-second sleep. In
the cancellation flow around cancellationSnapshotAfterUpstreamCompletion, expose
and await the test server’s completion-observed promise using
DEFAULT_STEP_TIMEOUT_MS, then retain the existing stopped-state assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45fd173d-8edd-41da-8b36-0ef024d863fc
📒 Files selected for processing (5)
docs/en/wegent/developer-guide/architecture.mddocs/zh/wegent/developer-guide/architecture.mdexecutor/src/agents/codex.rsexecutor/tests/app_runtime_work_send_contract.rswework/e2e/desktop/task-flow.e2e.mjs
What changed
no active turn to interruptraceWhy
turn/startcan return a turn ID before Codex app-server publishes the first progress event and registers that turn in its active-turn index. If Wework stops the task in that window, an interrupt using the concrete turn ID can fail withno active turn to interrupt. The executor would mark the local task cancelled while the newly starting Codex turn continued running and emitting content.During that startup window, Codex defines an empty
turnIdas the thread-level startup interrupt. Using it closes the race without changing cancellation behavior for established active turns.User impact
Stopping a Wework task now reliably terminates the Codex turn even when the stop action races with turn startup. Delayed upstream output cannot make the task resume or append content after the stopped notice appears.
Validation
cargo test --manifest-path executor/Cargo.toml --test app_runtime_work_send_contract runtime_tasks_cancel_ -- --nocapturecargo test --manifest-path executor/Cargo.toml --test codex_cancellation_contract -- --nocapturecargo fmt --manifest-path executor/Cargo.toml -- --checkpnpm --filter wework e2e:desktop -- --segment resiliencepnpm --filter wework exec prettier --check e2e/desktop/task-flow.e2e.mjsnode --check wework/e2e/desktop/task-flow.e2e.mjsSummary by CodeRabbit
turnId), then using the concreteturnIdonce available.