Skip to content

Fix Codex notifications not triggering#214

Merged
saddlepaddle merged 1 commit intomainfrom
navy-wind-15
Dec 1, 2025
Merged

Fix Codex notifications not triggering#214
saddlepaddle merged 1 commit intomainfrom
navy-wind-15

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Dec 1, 2025

Summary

  • Codex passes notification JSON as a command-line argument ($1), while Claude pipes it to stdin
  • The notify script was only reading from stdin, causing Codex notifications to silently fail
  • Now checks for argument first (Codex), falls back to stdin (Claude), and handles Codex's "type": "agent-turn-complete" event format

Test plan

  • Open a terminal in Superset desktop app
  • Run Codex and let it complete a task
  • Verify the red notification dot appears on the tab
  • Verify system notification shows "Agent Complete"
  • Confirm Claude notifications still work as before

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved event type detection to support multiple input formats for enhanced compatibility.
    • Enhanced input handling to accept JSON arguments or read from alternative input sources.
    • Added fallback event type handling to ensure consistent behavior in edge cases.

✏️ Tip: You can customize this high-level summary in your review settings.

Codex passes notification JSON as a command-line argument while Claude
pipes it to stdin. The notify script was only reading from stdin,
causing Codex notifications to never fire.

- Check for $1 argument first (Codex), fall back to stdin (Claude)
- Handle Codex's "type": "agent-turn-complete" event format
- Map Codex event type to "Stop" for consistent handling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 1, 2025

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

Project Deployment Preview Comments Updated (UTC)
website Ready Ready Preview Comment Dec 1, 2025 8:14pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 1, 2025

Walkthrough

Input handling for the notification script now accepts either command-line JSON arguments or stdin, while event type extraction supports both Claude's hook_event_name and Codex's type field formats with a "Stop" default fallback.

Changes

Cohort / File(s) Change Summary
Event type extraction and input handling improvements
apps/desktop/src/main/lib/agent-setup.ts
Enhanced createNotifyScript to read input from stdin when no first argument is provided; added support for extracting EVENT_TYPE from both Claude's hook_event_name and Codex's type field; added logic to map Codex's "agent-turn-complete" type to "Stop"; implemented "Stop" as default EVENT_TYPE when no event type is found

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file modification with localized logic changes
  • Input handling and field mapping are straightforward with clear fallback defaults
  • No changes to exported signatures or public APIs

Possibly related PRs

Poem

🐰 A script now listens far and wide,
To stdin's whisper, arguments' tide,
Events find their way with grace,
Codex and Claude in one place,
When all else fails, we Stop and hide!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main fix: resolving an issue where Codex notifications were not triggering.
Description check ✅ Passed The description includes a clear summary, test plan, and addresses the problem and solution, though it omits some template sections like explicit 'Related Issues' and 'Type of Change' categorization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch navy-wind-15

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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/desktop/src/main/lib/agent-setup.ts (1)

51-59: Codex event-type mapping works; consider future-proofing if more Codex events are added

The added CODEX_TYPE branch correctly maps Codex’s "type": "agent-turn-complete" to EVENT_TYPE="Stop" while preserving Claude’s existing hook_event_name parsing and the final default-to-Stop behavior. If Codex later emits other event types you care about (e.g., permission-style events), you may want to extend this mapping instead of always collapsing everything non‑Claude to "Stop", but that’s optional for this fix.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0848e4d and 8200c1b.

📒 Files selected for processing (1)
  • apps/desktop/src/main/lib/agent-setup.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
apps/desktop/src/main/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Node.js modules (fs, path, os, net, etc.) can be used in main process code only

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md

Applied to files:

  • apps/desktop/src/main/lib/agent-setup.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/src/main/lib/agent-setup.ts (1)

44-49: Input source handling for Codex vs Claude looks correct

Using $1 when present and falling back to cat from stdin matches the described Codex/Claude behaviors and avoids the previous blocking-on-stdin issue. This is a targeted change that keeps the script simple and backwards compatible.

@saddlepaddle saddlepaddle merged commit fac0d44 into main Dec 1, 2025
7 checks passed
@Kitenite Kitenite deleted the navy-wind-15 branch December 1, 2025 23:17
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