Skip to content

Iphone 17 Handoff Failure#673

Merged
arul28 merged 3 commits into
mainfrom
ade/ade-contesxt-find-ade-iphone-857bb2df
Jun 30, 2026
Merged

Iphone 17 Handoff Failure#673
arul28 merged 3 commits into
mainfrom
ade/ade-contesxt-find-ade-iphone-857bb2df

Conversation

@arul28

@arul28 arul28 commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

ADE   Open in ADE  ·  ade/ade-contesxt-find-ade-iphone-857bb2df branch  ·  PR #673

Summary by CodeRabbit

  • Bug Fixes

    • Improved brief chat handoff behavior so inherited context is applied at the right time for certain providers, helping avoid missing or out-of-order handoff details.
    • Fixed tab highlighting in the file editor so the visible tab is shown as active even when the original active tab is hidden by lane scoping.
  • Tests

    • Added coverage for chat handoff ordering and editor tab selection behavior.

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: Introduces deferInheritedGoalUntilHandoffDispatch for Codex brief handoffs, moving applyInheritedGoal(), persistChatState(), and seedCodexThreadGoalFromSessionGoal() into a finally block so they execute only after sendMessage dispatches; a nested try/catch swallows goal-seed failures with a warning rather than surfacing them to the caller.
  • EditorGroup.tsx: Single-line fix replacing group.activeTabId with activeTab?.id in the tab-button loop, so the already-computed fallback activeTab drives the aria-selected highlight 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

Filename Overview
apps/desktop/src/main/services/chat/agentChatService.ts Defers inherited goal application for Codex brief handoffs until after sendMessage dispatches, using a try/finally block; includes nested try/catch for goal seeding errors with a defensive extra persistChatState call in the catch branch.
apps/desktop/src/main/services/chat/agentChatService.test.ts Adds two targeted tests: one verifying handoff message precedes goal seeding for Codex, and one ensuring a goal-seed failure doesn't abort the handoff. Coverage is well-scoped.
apps/desktop/src/renderer/components/files/v2/EditorGroup.tsx Single-line fix: compares tab id against the computed activeTab (which already has the lane-scoped fallback logic) instead of the raw group.activeTabId, correctly highlighting the fallback tab when the stored active tab is hidden by lane scoping.
apps/desktop/src/renderer/components/files/v2/EditorGroup.test.tsx Adds a test that renders with a cross-lane activeTabId and asserts the visible tab gets aria-selected=true while the hidden tab is absent from the DOM.

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 }"
Loading
%%{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 }"
Loading

Reviews (3): Last reviewed commit: "Keep deferred Codex goal seeding best ef..." | Re-trigger Greptile

@mintlify

mintlify Bot commented Jun 30, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
ade-ac1c6011 🟢 Ready View Preview Jun 30, 2026, 5:57 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview Jun 30, 2026 6:27am

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two fixes: (1) for Codex brief handoff sessions, the inherited goal is now deferred and applied after sendMessage completes rather than before, ensuring turn/start precedes thread/goal/set; (2) EditorGroup tab highlighting now uses the resolved scoped active tab (activeTab?.id) instead of the raw store value.

Changes

Codex brief handoff: deferred inherited goal

Layer / File(s) Summary
Deferred applyInheritedGoal for Codex brief handoff
apps/desktop/src/main/services/chat/agentChatService.ts, apps/desktop/src/main/services/chat/agentChatService.test.ts
Extracts inherited goal update into applyInheritedGoal() helper; for brief + codex sessions, defers it to a finally block after sendMessage. Test asserts turn/start precedes thread/goal/set and that the injected turn text includes the ADE handoff marker and inherited goal.

EditorGroup scoped active tab fix

Layer / File(s) Summary
TabButton active state uses resolved activeTab?.id
apps/desktop/src/renderer/components/files/v2/EditorGroup.tsx, apps/desktop/src/renderer/components/files/v2/EditorGroup.test.tsx
Changes the active prop from group.activeTabId to activeTab?.id. Test adds a tabScope="lane" case where the stored activeTabId points to a hidden tab, verifying the visible fallback tab is marked active.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • arul28/ADE#500: Modifies Codex goal state updates and codex_goal_updated/cleared emission around thread/goal/set, directly related to how goal ordering is managed in agentChatService.ts.
  • arul28/ADE#659: Modifies Codex goal propagation logic in agentChatService.ts, specifically around when thread/goal/set is issued relative to turn startup.
  • arul28/ADE#671: Refactors active tab and lane-aware rendering in EditorGroup.tsx, directly preceding this fix to the TabButton active state.

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is misleading and refers to an unrelated "Iphone 17" issue, while the PR fixes Codex brief handoff goal ordering. Rename the PR to describe the actual change, such as fixing Codex brief handoff goal ordering for inherited goals.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/ade-contesxt-find-ade-iphone-857bb2df

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@arul28

arul28 commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

@copilot review but do not make fixes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between bd5409f and e886e22.

📒 Files selected for processing (4)
  • apps/desktop/src/main/services/chat/agentChatService.test.ts
  • apps/desktop/src/main/services/chat/agentChatService.ts
  • apps/desktop/src/renderer/components/files/v2/EditorGroup.test.tsx
  • apps/desktop/src/renderer/components/files/v2/EditorGroup.tsx

Comment thread apps/desktop/src/main/services/chat/agentChatService.test.ts Outdated
Comment thread apps/desktop/src/main/services/chat/agentChatService.ts
@arul28

arul28 commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: cce5334f6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment thread apps/desktop/src/main/services/chat/agentChatService.ts
@arul28

arul28 commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: c735d71f22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@arul28 arul28 merged commit c059e1e into main Jun 30, 2026
27 checks passed
@arul28 arul28 deleted the ade/ade-contesxt-find-ade-iphone-857bb2df branch June 30, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant