Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughClaude and Codex managed-hook detection now recognizes runtime and legacy notify paths. Codex hook generation uses a runtime-resolved shared command. Tests cover worktree independence, replacement, preservation, idempotency, and cleanup. ChangesManaged notify hook alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Visual evidenceBefore — stale absolute hook: Codex is working, but SessionStart and UserPromptSubmit fail with code 127 and the sidebar remains idle. 
After — active Codex turn: native hooks run successfully and the amber spinner is visible beside Delta newest. 
After completion: the response completes and the sidebar returns to its idle dot after the Stop lifecycle. 
|
Greptile SummaryThe PR replaces worktree-specific absolute paths in global Codex lifecycle hooks with a runtime-resolved managed command and updates reconciliation tests accordingly. The runtime command currently becomes a silent no-op when Codex is launched outside an environment exporting
Confidence Score: 4/5The direct-launch regression must be fixed before merging because globally installed Codex hooks silently stop reporting lifecycle events when The new global command solves stale worktree paths but conditions all notification delivery on an environment variable that arbitrary direct Codex launches do not receive, despite a known default Superset home directory. Files Needing Attention: apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts | Introduces worktree-independent Codex hook commands, but direct launches without SUPERSET_HOME_DIR silently lose lifecycle notifications. |
| apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts | Updates expected Codex hook commands and worktree-idempotency coverage, but does not exercise execution with an unset runtime home variable. |
Sequence Diagram
sequenceDiagram
participant Shell as Direct shell
participant Codex
participant Hook as Global hook command
participant Notify as notify.sh
participant Desktop as Superset desktop
Shell->>Codex: Launch without SUPERSET_HOME_DIR
Codex->>Hook: Run lifecycle hook
Hook->>Hook: Check SUPERSET_HOME_DIR
Hook-->>Codex: Exit successfully without notification
Note over Notify,Desktop: Lifecycle event is not delivered
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts:400
**Direct-launch hooks silently no-op**
If Codex is launched directly from an environment without `SUPERSET_HOME_DIR`, the generated global hook command exits successfully through `|| true` without invoking `notify.sh`, causing `SessionStart`, `UserPromptSubmit`, and `Stop` events to be lost and the working indicator to remain idle.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(desktop): stabilize Codex lifecycle ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const codexCommand = `SUPERSET_AGENT_ID=codex "${notifyScriptPath}"`; | ||
| // Resolve the active Superset home when the hook runs. A global absolute path | ||
| // becomes stale whenever another dev worktree rewrites hooks.json or is removed. | ||
| const codexCommand = getManagedNotifyHookCommand("codex"); |
There was a problem hiding this comment.
If Codex is launched directly from an environment without SUPERSET_HOME_DIR, the generated global hook command exits successfully through || true without invoking notify.sh, causing SessionStart, UserPromptSubmit, and Stop events to be lost and the working indicator to remain idle.
Knowledge Base Used: Desktop App (Electron main + renderer)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts
Line: 400
Comment:
**Direct-launch hooks silently no-op**
If Codex is launched directly from an environment without `SUPERSET_HOME_DIR`, the generated global hook command exits successfully through `|| true` without invoking `notify.sh`, causing `SessionStart`, `UserPromptSubmit`, and `Stop` events to be lost and the working indicator to remain idle.
**Knowledge Base Used:** [Desktop App (Electron main + renderer)](https://app.greptile.com/personal-org-6810/-/custom-context/knowledge-base/superset-sh/superset/-/docs/desktop-app.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts">
<violation number="1" location="apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts:400">
P2: Switching to `getManagedNotifyHookCommand("codex")` makes the Codex global hook depend on `$SUPERSET_HOME_DIR` being set at hook-execution time. If Codex is launched outside the Superset wrapper (e.g. directly from a terminal without that env var), the generated shell command may silently no-op instead of invoking notify.sh, causing SessionStart/UserPromptSubmit/Stop events to be lost and the sidebar indicator to stay idle. Consider verifying the fallback behavior when SUPERSET_HOME_DIR is unset and ensure it fails loudly or falls back to a known-good path rather than silently swallowing the error.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Codex as Codex CLI
participant Superset as Superset Desktop
participant Hooks as hooks.json
participant AgentSetup as Agent Setup Module
participant Common as agent-wrappers-common
participant Notify as notify.sh
participant Sidebar as Sidebar Indicator
Note over Codex,Sidebar: PR: Stabilize Codex working indicator via runtime hook path resolution
Codex->>Hooks: Read hooks on SessionStart/UserPromptSubmit/Stop
alt Stale absolute path (old behavior)
Hooks-->>Codex: Absolute path like /tmp/.superset-old/notify.sh
Codex->>Notify: Execute notify.sh (missing)
Notify-->>Codex: Exit code 127 (not found)
Codex-->>Sidebar: No lifecycle event → idle indicator
end
Note over AgentSetup,Common: Hook reconciliation at Superset startup
Superset->>AgentSetup: Initialize agent wrappers
AgentSetup->>Hooks: Read existing hooks.json
Hooks-->>AgentSetup: Current hook definitions (may include stale paths)
AgentSetup->>Common: getManagedNotifyHookCommand("codex")
Common->>Common: Build command with $SUPERSET_HOME_DIR/hooks/notify.sh
Common-->>AgentSetup: SUPERSET_AGENT_ID=codex $SUPERSET_HOME_DIR/...
AgentSetup->>Hooks: reconcileManagedEntries()
alt Stale managed hook found
AgentSetup->>Hooks: Replace stale command with dynamic marker
Hooks-->>AgentSetup: Updated hook entry
else User-defined hook preserved
AgentSetup->>Hooks: Keep user hook unchanged
end
Hooks-->>AgentSetup: Reconciled hooks.json
Note over Codex,Superset: Happy path after fix
Codex->>Hooks: Read hooks on SessionStart
Hooks-->>Codex: Dynamic command with $SUPERSET_HOME_DIR
Codex->>Codex: Resolve $SUPERSET_HOME_DIR at runtime
Codex->>Notify: Execute notify.sh from active worktree
Notify-->>Codex: Success (exit 0)
Codex-->>Superset: Lifecycle event received
Superset->>Sidebar: Update indicator (spinner visible)
Codex->>Hooks: Read hooks on Stop
Hooks-->>Codex: Same dynamic command
Codex->>Notify: Execute notify.sh for stop event
Notify-->>Codex: Success
Codex-->>Superset: Stop lifecycle event
Superset->>Sidebar: Reset indicator to idle dot
Note over AgentSetup,Common: Idempotent across worktree changes
Superset->>AgentSetup: Reconcile hooks (different worktree)
AgentSetup->>Hooks: Read hooks.json
Hooks-->>AgentSetup: Already has dynamic marker
AgentSetup->>Hooks: No changes needed (already correct)
Hooks-->>AgentSetup: Idempotent reconciliation
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const codexCommand = `SUPERSET_AGENT_ID=codex "${notifyScriptPath}"`; | ||
| // Resolve the active Superset home when the hook runs. A global absolute path | ||
| // becomes stale whenever another dev worktree rewrites hooks.json or is removed. | ||
| const codexCommand = getManagedNotifyHookCommand("codex"); |
There was a problem hiding this comment.
P2: Switching to getManagedNotifyHookCommand("codex") makes the Codex global hook depend on $SUPERSET_HOME_DIR being set at hook-execution time. If Codex is launched outside the Superset wrapper (e.g. directly from a terminal without that env var), the generated shell command may silently no-op instead of invoking notify.sh, causing SessionStart/UserPromptSubmit/Stop events to be lost and the sidebar indicator to stay idle. Consider verifying the fallback behavior when SUPERSET_HOME_DIR is unset and ensure it fails loudly or falls back to a known-good path rather than silently swallowing the error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts, line 400:
<comment>Switching to `getManagedNotifyHookCommand("codex")` makes the Codex global hook depend on `$SUPERSET_HOME_DIR` being set at hook-execution time. If Codex is launched outside the Superset wrapper (e.g. directly from a terminal without that env var), the generated shell command may silently no-op instead of invoking notify.sh, causing SessionStart/UserPromptSubmit/Stop events to be lost and the sidebar indicator to stay idle. Consider verifying the fallback behavior when SUPERSET_HOME_DIR is unset and ensure it fails loudly or falls back to a known-good path rather than silently swallowing the error.</comment>
<file context>
@@ -394,11 +395,9 @@ export function getCodexGlobalHooksJsonContent(
- const codexCommand = `SUPERSET_AGENT_ID=codex "${notifyScriptPath}"`;
+ // Resolve the active Superset home when the hook runs. A global absolute path
+ // becomes stale whenever another dev worktree rewrites hooks.json or is removed.
+ const codexCommand = getManagedNotifyHookCommand("codex");
const managedEvents: Array<{
</file context>
Summary
SUPERSET_HOME_DIRat runtime instead of persisting one worktree's absolutenotify.shpathRoot cause
Claude already stores a guarded runtime command in its global hook config, but Codex stored the absolute notify path from whichever Superset instance initialized most recently. Starting a different dev worktree rewrote that global path and Codex's exact hook-definition trust entry; removing that worktree left native
SessionStart,UserPromptSubmit, andStophooks pointing at a missing script. Codex itself continued working, but Superset never received the lifecycle event that drives the sidebar indicator.The wrapper log watcher could mask this for the default preset, which is why the failure is most visible when Codex is launched directly or the watcher cannot observe the current session format.
This is intentionally the narrow hook-path correction. It overlaps the Codex command portion of #6072, while leaving that PR's broader per-agent toggle and hook-scoping work untouched.
Verification
bun test apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts— 52 passedbun run lint— passed, zero warningsbun run --cwd apps/desktop typecheck— passedbun run --cwd apps/desktop compile:app— passedhttp://localhost:3205, CDP9341, route/v2-workspace/f1582787-5b36-4eba-a906-c13bf7000263/Users/roshvan/.local/bin/codexdirectly with native hooks enabledStopbun run typecheck— unrelated current-main failure inapps/docs: cannot resolve@superset/ui/overflow-fade-container; desktop-scoped typecheck passesVisual verification
Before — stale absolute hook: Codex is working, but
SessionStartandUserPromptSubmitfail with code 127 and the sidebar remains idle.After — active Codex turn: native hooks run successfully and the amber spinner is visible beside Delta newest.
After completion: the response completes and the sidebar returns to its idle dot after the
Stoplifecycle.Summary by CodeRabbit