fix(codex): preserve hook-driven agent status

[bbingz]

Summary

• stop treating Codex SessionStart as an active turn while retaining its provider session identity for the next real status event
• deduplicate repeated relayed SessionStart clears while retaining the tombstone required for reconnect replay
• support SubagentStart and SubagentStop without letting a child stop mark the root turn done
• keep prompt/tool/interrupt-derived status previews aligned with current Codex hook payloads
• prepend managed remote hooks like local hooks, while migrating existing index-addressed user trust entries and preserving their hash/enabled state
• keep local, SSH relay, and WSL managed-event behavior deterministic

This is the focused hooks/status replacement extracted from #7950. It intentionally excludes startup delivery, session bridging/native chat, auth/config, and rate-limit changes.

Refs #7950

Screenshots

No visual redesign.

Testing

• focused cumulative Vitest: 8 files / 597 passed / 8 skipped
• final latest-main relay/local/direct-remote regression suite: 3 files / 345 passed
• Node 24 Node/CLI/Web typecheck
• targeted oxlint
• targeted oxfmt --check
• pnpm check:max-lines-ratchet
• git diff --check origin/main...HEAD
• regression coverage for SessionStart identity without synthetic status, duplicate relay delivery, reconnect tombstone replay, working-state overwrite, local/relay propagation, remote trust-index migration, repeat-install idempotence, and WSL managed events
• Electron/E2E/GUI
• physical SSH/WSL hook installation

AI Review Report

The first independent gate found three blockers: remote prepend shifted user trust indices, SessionStart discarded the only session ID, and old server/WSL expectations contradicted the new event set. All three were reproduced with failing tests and fixed before submission.

A later CodeRabbit review found that a second relayed SessionStart could delete the cached tombstone and broadcast a duplicate clear. The final fix restores the prior tombstone after the shared normalizer's synchronous delete and returns without forwarding. Tests prove one clear, retained reconnect replay, and later working-state replacement. An independent second gate ended spec PASS / quality APPROVED.

Final behavior separates session identity from visible status: SessionStart resets prior turn state and caches the new provider session, but only a later real prompt/tool/stop event reaches the UI.

Security Audit

• preserves existing user hook approval hash/enabled state instead of broad-trusting remote hooks
• migrates only parsed trust entries that match unchanged user hook content
• does not add commands, IPC methods, credentials, dependencies, or persisted secrets
• keeps SSH relay payload normalization and connection stamping at the existing trust boundary

Compatibility

The same normalizer serves local and relay hook traffic. Windows/WSL managed event lists include the new subagent events; no path separator, keyboard, UI label, or Git-provider behavior changes.

The branch was rebased onto current origin/main at 667e04e72; range-diff preserved the two existing commits and added only the duplicate-SessionStart fix.

Notes

Validation used the repository-declared Node 24 toolchain. Electron/E2E and physical SSH/WSL verification were intentionally not run in this review cycle.

[coderabbitai]

📝 Walkthrough

Walkthrough

Codex hook support now includes SubagentStart and SubagentStop, prepended remote managed hooks, and trust-entry key migration. SessionStart events are metadata-only, with provider sessions cached across subsequent normalized events and pane statuses cleared idempotently across local and relayed paths. Codex lifecycle, interruption, subagent, and tool-input normalization was expanded. Codex-native Action Required titles are recognized as permission states and used in auto-approval suppression.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The template is mostly followed, but the AI Review Report lacks the required explicit macOS/Linux/Windows compatibility check details.	Expand the AI Review Report with explicit macOS, Linux, and Windows compatibility coverage, including shortcuts, labels, paths, shell behavior, and any Electron-specific differences.

✅ 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.
Title check	✅ Passed	The title matches the main change: Codex hook/status handling is being preserved and adjusted.

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/relay/agent-hook-server.ts (1)

317-334: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Prevent duplicate Codex SessionStart clears on the relay path. In src/relay/agent-hook-server.ts:317-334, normalizeHookPayload(..., 'codex', ...) deletes the cached status for SessionStart, so a second identical POST can re-enter forwardSessionStartClear after the tombstone is written and broadcast a second clear. Skip this branch when previousStatus.hookEventName === 'SessionStart'.

🧹 Nitpick comments (1)

src/main/agent-hooks/server.ts (1)

1310-1330: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the delete-side-effect inference used to detect a Codex SessionStart clear.

previousStatus && !this.state.lastStatusByPaneKey.has(paneKey) only works because normalizeHookPayload's Codex branch deletes the cache entry as a side effect of returning null for SessionStart. Nothing here checks hookEventName/hook_event_name directly, so this reads as an accidental correlation rather than an intentional contract, and any future Codex event that also nulls-and-deletes for an unrelated reason would silently route through this same "clear" path (with its listener/telemetry side effects).

A one-line comment stating this dependency (and ideally an explicit hookEventName === 'SessionStart' check in clearStatusForSessionStart itself, mirroring the relay's ingestRemote explicit check) would make the contract self-evident.

Based on coding guidelines: "When code is driven by a design document or non-obvious constraint, add a brief one- or two-line comment explaining why it behaves that way."

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: be012b83-a822-4745-a4d4-09e9d1be5457

📥 Commits

Reviewing files that changed from the base of the PR and between 490086a and 600e536.

📒 Files selected for processing (16)

• src/main/agent-hooks/server.test.ts
• src/main/agent-hooks/server.ts
• src/main/codex/codex-hook-identity.ts
• src/main/codex/config-toml-trust.ts
• src/main/codex/hook-service-wsl-runtime.test.ts
• src/main/codex/hook-service.test.ts
• src/main/codex/hook-service.ts
• src/relay/agent-hook-server.test.ts
• src/relay/agent-hook-server.ts
• src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts
• src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts
• src/renderer/src/lib/agent-status.test.ts
• src/shared/agent-detection.ts
• src/shared/agent-hook-listener.test.ts
• src/shared/agent-hook-listener.ts
• src/shared/agent-title-status.ts

✅ Files skipped from review due to trivial changes (1)

• src/main/codex/hook-service-wsl-runtime.test.ts

🚧 Files skipped from review as they are similar to previous changes (10)

• src/shared/agent-detection.ts
• src/main/codex/codex-hook-identity.ts
• src/renderer/src/lib/agent-status.test.ts
• src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts
• src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts
• src/shared/agent-title-status.ts
• src/main/codex/hook-service.ts
• src/main/codex/hook-service.test.ts
• src/shared/agent-hook-listener.ts
• src/main/codex/config-toml-trust.ts

[bbingz]

Addressed the latest CodeRabbit outside-diff finding in 7582f64f3.

The relay now recognizes a prior SessionStart tombstone, restores it after the shared normalizer's synchronous delete, and returns without forwarding a duplicate clear. This is intentionally not just an early return: keeping the tombstone is what preserves reconnect replay. The expanded lifecycle test proves one clear across duplicate events, replay of that clear, and replacement by the next real working status.

Post-rebase verification: 3 focused files / 345 tests, full Node/CLI/Web typecheck, targeted lint/format, max-lines, diff check, and independent PASS / APPROVED review.