Preserve planner guard on null turn-start notifications

[arul28]

Summary

Fixes the remaining Greptile review issue from #301: a malformed Codex turn/started notification without a turn id no longer consumes the pending planner approval guard before the real turn id arrives.

Validation

• cd apps/desktop && npx vitest run src/main/services/chat/agentChatService.test.ts

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of agent chat events when turn identifiers are missing, with appropriate warning logging to facilitate debugging of edge cases.

Greptile Summary

Fixes a bug where a malformed Codex turn/started notification (missing turn.id) would consume the pending planner approval guard before the real notification arrived, causing the guard to be silently bound to a null turn id. A focused early-return is added when the null-id case arrives while a guard is pending, and the fix is covered by a new regression test.

• agentChatService.ts: Adds a guard that discards a turn/started with no turn id when pendingTurnPlanningApprovalGuarded is non-null, logging a warning and returning without modifying runtime state.
• agentChatService.test.ts: Injects the malformed notification before the real one and asserts the planner contract violation still fires correctly on the subsequent command approval.
• packagedRuntimeSmoke.ts: Extends the PTY probe timeout to 15 s on Windows (4 s elsewhere) and includes the timeout value in the error message.

Confidence Score: 5/5

Safe to merge — the change is a narrow, well-tested guard that only fires when both conditions are true simultaneously.

The fix is minimal and targeted: one early-return behind a two-condition check, backed by a regression test that exercises the exact sequence. The packagedRuntimeSmoke.ts change is an unrelated timeout widening for Windows and carries no correctness risk.

No files require special attention.

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/chat/agentChatService.ts	Adds an early-return guard: when a turn/started arrives with no turn id and a pending planning approval guard exists, the notification is discarded and the guard is preserved for the real turn id.
apps/desktop/src/main/services/chat/agentChatService.test.ts	Adds a regression test that injects a malformed turn/started (no id) before the real one and asserts the planner contract violation guard still fires correctly.
apps/desktop/src/main/packagedRuntimeSmoke.ts	Widens PTY probe timeout to 15 s on Windows (was 4 s for all platforms) and includes the timeout duration in the error message for easier debugging.

Sequence Diagram

sequenceDiagram
    participant Codex
    participant Handler as turn/started handler
    participant Runtime

    Note over Codex,Runtime: Malformed notification (no turn id)
    Codex->>Handler: "turn/started { turn: {} }"
    Handler->>Runtime: "pendingTurnPlanningApprovalGuarded !== null?"
    Runtime-->>Handler: yes
    Handler->>Handler: warn + return (guard preserved)

    Note over Codex,Runtime: Real notification (with turn id)
    Codex->>Handler: "turn/started { turn: { id: "turn-1" } }"
    Handler->>Runtime: rememberCodexPlanningApprovalGuard(runtime, "turn-1", guard)
    Runtime-->>Runtime: "pendingTurnPlanningApprovalGuarded = null"
    Handler->>Runtime: "activeTurnId = "turn-1""

    Note over Codex,Runtime: Planner guard is active for "turn-1"

Loading

_{Reviews (4): Last reviewed commit: "Harden Windows packaged PTY smoke timeou..." | Re-trigger Greptile}

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	May 14, 2026 8:17pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The agentChatService now defers turn/started events that lack a valid turnId when operating under the planner approval guard. The implementation check and corresponding test case for this empty-turn scenario are added together.

Changes

Deferred turn/started handling in approval guard mode

Layer / File(s)	Summary
Turn/started early guard with test coverage apps/desktop/src/main/services/chat/agentChatService.ts, apps/desktop/src/main/services/chat/agentChatService.test.ts	Implementation adds an early guard that checks for missing turnId and active pendingTurnPlanningApprovalGuarded, logs a warning, and returns immediately to defer processing. Test case updated to mock turn/started with an empty turn object to validate this guard behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• arul28/ADE#301: Introduces the per-turn planner approval guard state (pendingTurnPlanningApprovalGuarded) that this PR's guard check directly uses for deferring turn/started events without a turnId.

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Preserve planner guard on null turn-start notifications' directly aligns with the main change: handling malformed Codex notifications without turn IDs to prevent consuming the pending planner approval guard.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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/fix-planner-guard-null-turn

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[arul28]

@copilot review but do not make fixes

[capy-ai]

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[Copilot]

@copilot review but do not make fixes

Reviewed commit 9e4fac3 only (no fixes made). I don’t see additional issues in this patch: the new guard correctly preserves pending planner approval when turn/started arrives without a valid turn.id, and the regression test covers malformed-then-valid sequencing. No UI changes in this review, so no screenshot applies.

[arul28]

@copilot review but do not make fixes

[arul28]

@copilot review but do not make fixes