Iphone 17 Handoff Failure#673
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughTwo fixes: (1) for Codex brief handoff sessions, the inherited goal is now deferred and applied after ChangesCodex brief handoff: deferred inherited goal
EditorGroup scoped active tab fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/desktop/src/main/services/chat/agentChatService.test.ts`:
- Around line 3201-3213: The RPC ordering check in agentChatService.test.ts is
too broad because mockState.codexRequestPayloads includes setup traffic from
createSession(...) before handoffSession(...). Update the assertion near
handoffSession so it only inspects the Codex payloads emitted by that handoff
exchange (for example by slicing/filtering around the relevant request or
capturing the payloads within the handoff call), then verify turn/start precedes
any thread/goal/set using that scoped subset instead of the full global array.
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 19973-20007: Codex brief handoff is losing the inherited goal
because the deferred path in the handoff flow never triggers the thread goal
seeding. Update the handoff logic around applyInheritedGoal(), sendMessage(),
and the brief/codex deferment so the goal is either applied before sendMessage()
or re-seeded afterward by calling the same Codex thread goal setup path that
reaches seedCodexThreadGoalFromSessionGoal() and thread/goal/set. Make sure the
restored goal is persisted consistently via sessionService.updateMeta() and
persistChatState().
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 64798392-d734-4286-bbc5-3bdd9760de88
📒 Files selected for processing (4)
apps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/files/v2/EditorGroup.test.tsxapps/desktop/src/renderer/components/files/v2/EditorGroup.tsx
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Bug Fixes
Tests
Greptile Summary
This PR fixes two bugs: a Codex handoff ordering issue where the inherited session goal was being applied before the handoff message was dispatched (causing out-of-order context for the provider), and an editor tab highlighting regression where the visible fallback tab wasn't marked active when lane scoping hid the stored active tab.
agentChatService.ts: IntroducesdeferInheritedGoalUntilHandoffDispatchfor Codex brief handoffs, movingapplyInheritedGoal(),persistChatState(), andseedCodexThreadGoalFromSessionGoal()into afinallyblock so they execute only aftersendMessagedispatches; a nestedtry/catchswallows goal-seed failures with a warning rather than surfacing them to the caller.EditorGroup.tsx: Single-line fix replacinggroup.activeTabIdwithactiveTab?.idin the tab-button loop, so the already-computed fallbackactiveTabdrives thearia-selectedhighlight instead of the raw stored id.Confidence Score: 5/5
Safe to merge; both fixes are narrowly scoped with corresponding tests, and the deferred goal path is guarded entirely within the Codex brief handoff branch.
The agentChatService change correctly defers goal application and wraps goal-seed failures so they can't abort an otherwise successful handoff. The EditorGroup one-liner is straightforward and well-tested. No unhandled error paths or state divergence risks were found beyond what is already caught and logged.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant handoffSession participant sendMessage participant applyInheritedGoal participant persistChatState participant seedCodexGoal Caller->>handoffSession: "handoffSession({ sourceSessionId, targetModelId })" handoffSession->>handoffSession: compute deferInheritedGoalUntilHandoffDispatch alt NOT deferred (non-Codex or non-brief) handoffSession->>applyInheritedGoal: applyInheritedGoal() end handoffSession->>persistChatState: persistChatState() alt "handoffMode === brief" handoffSession->>sendMessage: "sendMessage(handoffPrompt, { awaitDispatch: true })" Note over sendMessage: dispatch to Codex turn/start sendMessage-->>handoffSession: resolved (or throws) Note over handoffSession: finally block always runs alt deferInheritedGoalUntilHandoffDispatch (Codex brief) handoffSession->>applyInheritedGoal: applyInheritedGoal() handoffSession->>persistChatState: persistChatState() handoffSession->>seedCodexGoal: seedCodexThreadGoalFromSessionGoal() alt seeding fails seedCodexGoal-->>handoffSession: throws handoffSession->>handoffSession: logger.warn(...) handoffSession->>persistChatState: persistChatState() [defensive] else seeding succeeds seedCodexGoal-->>handoffSession: ok end end end handoffSession-->>Caller: "{ session, usedFallbackSummary }"%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Caller participant handoffSession participant sendMessage participant applyInheritedGoal participant persistChatState participant seedCodexGoal Caller->>handoffSession: "handoffSession({ sourceSessionId, targetModelId })" handoffSession->>handoffSession: compute deferInheritedGoalUntilHandoffDispatch alt NOT deferred (non-Codex or non-brief) handoffSession->>applyInheritedGoal: applyInheritedGoal() end handoffSession->>persistChatState: persistChatState() alt "handoffMode === brief" handoffSession->>sendMessage: "sendMessage(handoffPrompt, { awaitDispatch: true })" Note over sendMessage: dispatch to Codex turn/start sendMessage-->>handoffSession: resolved (or throws) Note over handoffSession: finally block always runs alt deferInheritedGoalUntilHandoffDispatch (Codex brief) handoffSession->>applyInheritedGoal: applyInheritedGoal() handoffSession->>persistChatState: persistChatState() handoffSession->>seedCodexGoal: seedCodexThreadGoalFromSessionGoal() alt seeding fails seedCodexGoal-->>handoffSession: throws handoffSession->>handoffSession: logger.warn(...) handoffSession->>persistChatState: persistChatState() [defensive] else seeding succeeds seedCodexGoal-->>handoffSession: ok end end end handoffSession-->>Caller: "{ session, usedFallbackSummary }"Reviews (3): Last reviewed commit: "Keep deferred Codex goal seeding best ef..." | Re-trigger Greptile