Skip to content

desktop: self-healing agent wrappers + notification hook reliability + opencode hook support#533

Merged
Kitenite merged 10 commits intosuperset-sh:mainfrom
andreasasprou:fix/agent-hooks-notifications
Dec 31, 2025
Merged

desktop: self-healing agent wrappers + notification hook reliability + opencode hook support#533
Kitenite merged 10 commits intosuperset-sh:mainfrom
andreasasprou:fix/agent-hooks-notifications

Conversation

@andreasasprou
Copy link
Copy Markdown
Contributor

@andreasasprou andreasasprou commented Dec 29, 2025

Why

  • Notifications depend on wrapper scripts + notify.sh, but those were only created at app startup. If users install codex/claude later or delete wrappers, notifications silently stop.
  • OpenCode had no Superset hook integration at all, so OpenCode sessions could never trigger "needs attention" notifications.
  • NEW: When using oh-my-opencode with background subagents (explore, librarian, oracle, etc.), each subagent runs in its own OpenCode session. These child sessions emit session.idle events when they complete, causing excessive notification spam.

What / How

This PR makes agent hooks self-healing and adds OpenCode notifications:

  1. Self-healing wrappers (Claude + Codex)
  • Superset always writes wrapper scripts, even if the real agent binary is missing.
  • At invocation time, the wrapper scans PATH (skipping ~/.superset/bin + ~/.superset-dev/bin) to find the real binary and execs it with notification hooks injected.
  • If the agent isn't installed, the wrapper prints a clear install/path hint and exits 127.
  • Wrappers include a # Superset agent-wrapper v1 marker for safe upgrades.
  1. Async ensure/repair on terminal creation
  • New ensureAgentHooks() runs fire-and-forget when creating a terminal session.
  • It yields immediately and uses async FS to avoid delaying terminal creation.
  • It verifies wrappers + hooks/notify.sh + Claude settings exist (and match expected markers) and rewrites them if missing/outdated.
  1. OpenCode wrapper + plugin
  • Adds an opencode wrapper that sets OPENCODE_CONFIG_DIR and uses the same self-healing PATH resolution as Claude/Codex.
  • Generates hooks/opencode/plugin/superset-notify.js to listen for session.idle and permission.ask and call notify.sh with Claude-style hook_event_name payloads.
  • ensureAgentHooks() now repairs the OpenCode wrapper + plugin when missing/outdated.
  1. Subagent notification filtering (NEW)
  • When using oh-my-opencode or similar tools that spawn background subagents, each subagent runs in its own OpenCode session with a parentID pointing to the main session.
  • The OpenCode plugin now uses client.session.list() to check if a session has a parentID before sending notifications.
  • Only main sessions (no parentID) trigger notifications - subagent completions are silently ignored.
  • This mirrors OpenCode's own notification handling in packages/app/src/context/notification.tsx.
  1. Notification hook refactor + server readiness
  • notify.sh is refactored to expose content/path helpers and a marker so it can be validated/repaired.
  • Adds express.json() so we can accept POST-based hook payloads in follow-ups (current hook remains GET /hook/complete).
  1. Dependency hygiene
  • Imports getShellEnv directly from agent-setup/shell-wrappers.ts to avoid widening an existing terminal/envagent-setup dependency cycle.

Not in this PR

  • Windows: not supported end-to-end yet:
    • Agent wrappers are bash scripts (#!/bin/bash), so they won't run in the default Superset Windows shells (PowerShell/CMD).
    • Hook execution relies on bash + curl + grep/cut (hooks/notify.sh), which aren't guaranteed on Windows.
    • Superset's PATH interception is implemented for zsh/bash only, so ~/.superset*/bin typically isn't on PATH for PowerShell/CMD terminals.
    • It may work in Git Bash/WSL environments, but that's not supported behavior.

Files

  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts (new)
  • apps/desktop/src/main/lib/agent-setup/index.ts
  • apps/desktop/src/main/lib/agent-setup/paths.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/notifications/server.ts

Testing

  • Not run (automated).
  • Manual:
    • Verified ~/.superset-dev/bin/opencode, ~/.superset-dev/hooks/opencode/plugin/superset-notify.js, and ~/.superset-dev/hooks/notify.sh exist after bun dev.
image image

Summary by CodeRabbit

  • New Features

    • Added OpenCode plugin support with local/global plugin handling and multi-asset wrapper generation.
  • Improvements

    • Modularized wrapper/script generation for Claude, Codex and OpenCode.
    • Extracted and parameterized notify script creation.
    • Automatic agent hook initialization during session startup with guarded waiting and timeout.
    • Notifications server now parses JSON bodies; terminal startup timing/initial-command flow refined.

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

Ensure agent wrappers/hooks are repaired on terminal creation and allow JSON bodies in notifications server.
@andreasasprou andreasasprou changed the title desktop: self-healing agent wrappers + notifications hook fixes desktop: self-healing agent wrappers + OpenCode notifications Dec 29, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

Refactors wrapper/script generation into modular builders and path getters, adds OpenCode plugin support, introduces ensureAgentHooks() to create/write notify, wrapper, and plugin assets with marker-based rewrites and exec permissions, and integrates hook readiness into terminal session startup.

Changes

Cohort / File(s) Summary
Agent wrapper & generators
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
New constants/markers and public path getters, real-binary resolver + missing-binary helper, content builders for Claude/OpenCode, script builders for Claude/Codex/OpenCode, and create* flows (Claude settings, wrappers, OpenCode plugin/global write with guarded error handling).
Agent hooks initializer
apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
New async ensureAgentHooks() implementing singleton-in-flight init, directory creation (BIN_DIR, HOOKS_DIR, OPENCODE dirs), marker-aware read/write, exec permission setting, and orchestration to generate notify script, Claude settings, wrappers, and OpenCode plugin/wrapper.
Notify script modularization
apps/desktop/src/main/lib/agent-setup/notify-hook.ts
Extracted notify script into NOTIFY_SCRIPT_NAME, NOTIFY_SCRIPT_MARKER, getNotifyScriptPath(), getNotifyScriptContent(); createNotifyScript() now composes and writes content via helpers.
Path constants
apps/desktop/src/main/lib/agent-setup/paths.ts
Added OPENCODE_CONFIG_DIR and OPENCODE_PLUGIN_DIR (derived from HOOKS_DIR).
Agent setup integration
apps/desktop/src/main/lib/agent-setup/index.ts
Added imports/calls to create OpenCode plugin and wrapper; ensures OPENCODE_PLUGIN_DIR creation during setup.
Terminal manager/session integration
apps/desktop/src/main/lib/terminal/manager.ts, apps/desktop/src/main/lib/terminal/session.ts
manager.ts: kicks off non-blocking ensureAgentHooks() and detects agent commands to conditionally await readiness. session.ts: adds beforeInitialCommands?: Promise<void>, AGENT_HOOKS_TIMEOUT_MS, and races initial command dispatch against hooks timeout.
Small/adjacent changes
apps/desktop/src/main/lib/terminal/env.ts, apps/desktop/src/main/lib/notifications/server.ts
env.ts: updated import to ../agent-setup/shell-wrappers. server.ts: added express.json() middleware.

Sequence Diagram

sequenceDiagram
    participant TM as TerminalManager
    participant EAH as ensureAgentHooks()
    participant WB as AgentWrappers (builders)
    participant FS as FileSystem
    participant TS as TerminalSession

    rect rgb(220,235,255)
    note right of TM: Session creation triggers hooks init (non-blocking)
    TM->>EAH: start ensureAgentHooks()
    end

    rect rgb(235,255,235)
    note right of EAH: Initialization (singleton)
    EAH->>FS: ensure BIN_DIR / HOOKS_DIR / OPENCODE dirs exist
    EAH->>FS: read existing files (notify/settings/plugins)
    end

    rect rgb(255,250,230)
    note right of WB: Build assets and markers
    EAH->>WB: getNotifyScriptContent()
    WB-->>EAH: notify script text
    EAH->>FS: write notify script (marker-aware) + set exec
    EAH->>WB: buildClaudeWrapperScript(settingsPath)
    WB-->>EAH: claude wrapper text
    EAH->>FS: write claude wrapper + set exec
    EAH->>WB: buildCodexWrapperScript(notifyPath)
    WB-->>EAH: codex wrapper text
    EAH->>FS: write codex wrapper + set exec
    EAH->>WB: getOpenCodePluginContent(notifyPath)
    WB-->>EAH: opencode plugin text
    EAH->>FS: write plugin (local) and try global write (guarded)
    EAH->>WB: buildOpenCodeWrapperScript(opencodeConfigDir)
    WB-->>EAH: opencode wrapper text
    EAH->>FS: write opencode wrapper + set exec
    end

    rect rgb(250,235,235)
    note right of EAH: Completion
    EAH-->>TM: promise resolves (optional await by session)
    TM->>TS: proceed with session (initial commands may race with hooks)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped through scripts with markers bright,
Builders stitched wrappers in morning light,
OpenCode a guest, placed neat in its nest,
ensureAgentHooks hummed and did its best,
A rabbit’s small clap for assets set right.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the three main changes: self-healing agent wrappers, notification hook reliability improvements, and OpenCode hook support.
Description check ✅ Passed The PR description is comprehensive and well-structured. It clearly explains the why, what, and how with detailed sections covering self-healing wrappers, async ensure/repair, OpenCode integration, subagent filtering, notification hook refactoring, and dependency hygiene. All required template sections are addressed.
✨ Finishing touches
  • 📝 Generate docstrings

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.

@andreasasprou andreasasprou changed the title desktop: self-healing agent wrappers + OpenCode notifications desktop: self-healing agent wrappers + notification hook reliability Dec 29, 2025
@andreasasprou andreasasprou marked this pull request as ready for review December 29, 2025 08:55
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: 2

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85064bf and 293e586.

📒 Files selected for processing (8)
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/agent-setup/index.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/agent-setup/paths.ts
  • apps/desktop/src/main/lib/notifications/server.ts
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}

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

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

  • apps/desktop/src/main/lib/notifications/server.ts
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/agent-setup/paths.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/agent-setup/index.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use type safety and avoid any types unless absolutely necessary in TypeScript files

Files:

  • apps/desktop/src/main/lib/notifications/server.ts
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/agent-setup/paths.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/agent-setup/index.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
apps/desktop/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Move Node.js functionality needed in Electron renderer to src/main/lib/ and communicate via IPC

Files:

  • apps/desktop/src/main/lib/notifications/server.ts
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/agent-setup/paths.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/agent-setup/index.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
🧠 Learnings (4)
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/**/src/(middleware|proxy).ts : In Next.js 16, never create `middleware.ts`. Always use `proxy.ts` instead for request interception

Applied to files:

  • apps/desktop/src/main/lib/notifications/server.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in `src/main/index.ts` with `override: true` before any other imports in the desktop app

Applied to files:

  • apps/desktop/src/main/lib/terminal/env.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC

Applied to files:

  • apps/desktop/src/main/lib/terminal/env.ts
📚 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/ensure-agent-hooks.ts
🧬 Code graph analysis (4)
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
apps/desktop/src/main/lib/agent-setup/paths.ts (1)
  • HOOKS_DIR (5-5)
apps/desktop/src/main/lib/agent-setup/index.ts (3)
apps/desktop/src/main/lib/agent-setup/paths.ts (1)
  • OPENCODE_PLUGIN_DIR (9-9)
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
  • createNotifyScript (53-57)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (2)
  • createOpenCodePlugin (196-202)
  • createOpenCodeWrapper (207-212)
apps/desktop/src/main/lib/terminal/manager.ts (1)
apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts (1)
  • ensureAgentHooks (85-149)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (2)
apps/desktop/src/main/lib/agent-setup/paths.ts (4)
  • BIN_DIR (4-4)
  • HOOKS_DIR (5-5)
  • OPENCODE_PLUGIN_DIR (9-9)
  • OPENCODE_CONFIG_DIR (8-8)
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
  • getNotifyScriptPath (9-11)
🔇 Additional comments (18)
apps/desktop/src/main/lib/notifications/server.ts (1)

19-20: LGTM!

The JSON body parser middleware is correctly positioned and prepares the server for future POST-based hook payloads without affecting the existing GET endpoint.

apps/desktop/src/main/lib/terminal/env.ts (1)

5-5: LGTM!

The import path change correctly targets the modular shell-wrappers submodule, avoiding potential circular dependency issues between terminal/env and the broader agent-setup module.

apps/desktop/src/main/lib/agent-setup/index.ts (2)

2-15: LGTM!

The imports are well-organized, grouping related wrapper functions and path constants together. The modular structure improves maintainability.


35-42: LGTM!

The OpenCode plugin directory creation and asset generation follow the established pattern. The order correctly ensures the directory exists before writing plugin/wrapper files.

apps/desktop/src/main/lib/terminal/manager.ts (1)

66-68: LGTM!

The fire-and-forget pattern with error logging is appropriate for self-healing infrastructure. The void prefix correctly indicates the intentional discard of the promise, and the catch block ensures failures are logged without blocking terminal session creation.

apps/desktop/src/main/lib/agent-setup/paths.ts (1)

8-9: LGTM!

The new path constants follow the established naming convention and correctly derive from existing paths, maintaining a logical directory hierarchy.

apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts (3)

1-28: LGTM!

The imports correctly use the promises API for async operations, and the inFlight singleton effectively deduplicates concurrent calls to prevent race conditions during hook initialization.


73-83: LGTM!

The string-based check for "hooks" is acceptable since the settings are generated via JSON.stringify with consistent formatting. The approach avoids JSON parsing overhead for the common case where settings are valid.


85-149: LGTM!

The implementation correctly handles:

  • Windows platform early exit
  • Singleton pattern preventing concurrent runs
  • Immediate yield via setImmediate for non-blocking behavior
  • Sequential script file initialization
  • Cleanup of inFlight in the finally block

The async/await flow with proper error propagation is well-structured.

apps/desktop/src/main/lib/agent-setup/notify-hook.ts (3)

6-11: LGTM!

The constants and path helper follow the established pattern used by other agent-setup modules, enabling consistent marker-based script validation.


13-48: LGTM!

The script handles both Claude (stdin) and Codex (argument) input methods gracefully. The grep-based JSON parsing is pragmatic for shell scripting, and the fallback to "Stop" ensures reliability even if parsing fails. The curl call correctly uses URL encoding for parameters.


50-57: LGTM!

The refactored function properly delegates to the new helpers while maintaining the same synchronous behavior appropriate for startup-time initialization.

apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (6)

1-14: LGTM!

The versioned markers (v1) enable future safe upgrades, and the constants are properly exported for consistent usage across the agent-setup module.


16-32: LGTM!

The find_real_binary function correctly:

  • Skips Superset wrapper directories to avoid infinite recursion
  • Handles empty PATH entries and trailing slashes
  • Verifies the target is an executable file (not a directory)

The shell quoting and pattern matching are robust.


38-56: LGTM!

Path getters are consistent and correctly use path.join for cross-platform compatibility. The file locations align with the directory structure established in paths.ts and index.ts.


58-69: LGTM!

The Claude settings structure correctly configures notification hooks for both Stop and PermissionRequest events, aligning with the PR's notification integration goals.


71-121: LGTM!

The wrapper scripts:

  • Use exec to replace the shell process with the real binary
  • Exit with code 127 (standard "command not found") when the binary is missing
  • Provide user-friendly error messages with installation hints
  • Correctly pass through all arguments with "$@"

159-212: LGTM!

The creation functions follow a consistent pattern:

  • Each generates content via the corresponding builder/getter
  • Uses synchronous FS operations appropriate for startup
  • Logs creation events for debugging

Note that createClaudeSettings is intentionally private (not exported), correctly encapsulating the settings creation as an internal detail of createClaudeWrapper.

Comment thread apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
Comment thread apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
@andreasasprou andreasasprou marked this pull request as draft December 29, 2025 09:03
- Add curl timeouts (1s connect, 2s max) to prevent blocking agent completion
- Document OpenCode plugin auto-loading mechanism from ~/.config/opencode/plugin/
- Fix agent command detection to use regex (handles 'cd repo && claude ...')
- Only check executability for files with execute bits (0o755), skip for 0o644
- Extract AGENT_HOOKS_TIMEOUT_MS constant for hook wait timeout
- Add comment explaining String.fromCharCode trick for template literals
@andreasasprou andreasasprou marked this pull request as ready for review December 29, 2025 11:44
@andreasasprou andreasasprou changed the title desktop: self-healing agent wrappers + notification hook reliability desktop: self-healing agent wrappers + notification hook reliability + opencode hook support Dec 29, 2025
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 (3)
apps/desktop/src/main/lib/terminal/session.ts (2)

16-17: Consider exporting or documenting the timeout constant.

The 2000ms timeout is reasonable, but consider adding a brief comment explaining why this value was chosen (e.g., balancing responsiveness with hook setup time).


178-191: Consider extracting the async IIFE into a named function for clarity.

The nested void (async () => { ... })(); pattern works but is somewhat dense. Extracting to a named async helper could improve readability.

🔎 Optional refactor
+const dispatchInitialCommands = async (
+  session: TerminalSession,
+  initialCommandString: string,
+  beforeInitialCommands?: Promise<void>,
+): Promise<void> => {
+  if (beforeInitialCommands) {
+    const timeout = new Promise<void>((resolve) =>
+      setTimeout(resolve, AGENT_HOOKS_TIMEOUT_MS),
+    );
+    await Promise.race([beforeInitialCommands, timeout]).catch(() => {});
+  }
+  if (session.isAlive) {
+    session.pty.write(initialCommandString);
+  }
+};
+
 // Then in the onData handler:
 if (initialCommandString && !commandsSent) {
   commandsSent = true;
   setTimeout(() => {
     if (session.isAlive) {
-      void (async () => {
-        if (beforeInitialCommands) {
-          const timeout = new Promise<void>((resolve) =>
-            setTimeout(resolve, AGENT_HOOKS_TIMEOUT_MS),
-          );
-          await Promise.race([beforeInitialCommands, timeout]).catch(
-            () => {},
-          );
-        }
-
-        if (session.isAlive) {
-          session.pty.write(initialCommandString);
-        }
-      })();
+      void dispatchInitialCommands(session, initialCommandString, beforeInitialCommands);
     }
   }, 100);
 }
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (1)

183-190: Sync fs operations differ from async pattern in ensure-agent-hooks.ts.

createClaudeSettings uses synchronous fs.writeFileSync, while ensureAgentHooks in ensure-agent-hooks.ts uses async fs operations with ensureScriptFile. This is intentional—create* functions are for direct/blocking use, while build* functions provide content for async flows.

Consider adding a brief JSDoc note clarifying this distinction to prevent future confusion.

🔎 Suggested documentation
+/**
+ * Creates the Claude Code settings JSON file with notification hooks.
+ * Uses synchronous fs for blocking initialization paths.
+ * For async/non-blocking use, see `ensureAgentHooks()` which uses `ensureScriptFile()`.
+ */
 function createClaudeSettings(): string {
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 293e586 and 1ba72c4.

📒 Files selected for processing (5)
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/terminal/session.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts
  • apps/desktop/src/main/lib/agent-setup/notify-hook.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}

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

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Use type safety and avoid any types unless absolutely necessary in TypeScript files

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Move Node.js functionality needed in Electron renderer to src/main/lib/ and communicate via IPC

Files:

  • apps/desktop/src/main/lib/terminal/manager.ts
  • apps/desktop/src/main/lib/terminal/session.ts
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
🧠 Learnings (3)
📓 Common learnings
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
📚 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/terminal/manager.ts
  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC

Applied to files:

  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
🧬 Code graph analysis (2)
apps/desktop/src/main/lib/terminal/manager.ts (2)
apps/desktop/src/main/lib/agent-setup/ensure-agent-hooks.ts (1)
  • ensureAgentHooks (89-179)
apps/desktop/src/main/lib/terminal/session.ts (3)
  • createSession (63-142)
  • setupDataHandler (144-196)
  • reinitializeHistory (221-235)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (3)
apps/desktop/src/main/lib/agent-setup/paths.ts (4)
  • BIN_DIR (4-4)
  • HOOKS_DIR (5-5)
  • OPENCODE_PLUGIN_DIR (9-9)
  • OPENCODE_CONFIG_DIR (8-8)
packages/local-db/src/schema/schema.ts (1)
  • settings (117-129)
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
  • getNotifyScriptPath (9-11)
⏰ 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 (8)
apps/desktop/src/main/lib/terminal/manager.ts (2)

66-79: Clean fire-and-forget pattern with conditional agent hook synchronization.

The implementation correctly:

  1. Initiates ensureAgentHooks() early in session creation (fire-and-forget)
  2. Uses regex word boundaries to detect agent commands even in chained commands
  3. Only passes the promise to setupDataHandler when agent commands are present

This avoids unnecessarily delaying non-agent terminal sessions.


82-88: Signature alignment verified.

The setupDataHandler signature correctly accepts an optional Promise<void> as the 5th parameter (beforeInitialCommands), and the code properly passes either agentHooksReady (which resolves to Promise<void>) or undefined based on the shouldAwaitAgentHooks condition. Type alignment is correct.

apps/desktop/src/main/lib/terminal/session.ts (1)

174-191: Robust timeout-guarded command dispatch with session liveness checks.

The implementation correctly:

  1. Uses Promise.race to cap wait time at AGENT_HOOKS_TIMEOUT_MS
  2. Swallows errors via .catch(() => {}) for best-effort behavior
  3. Double-checks session.isAlive before and after the async wait

One minor observation: the 100ms setTimeout plus the potential 2000ms race could result in up to ~2.1s delay for agent commands. This is documented behavior but worth noting in comments.

apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (5)

17-33: Shell binary resolver is well-designed.

The find_real_binary function correctly:

  1. Splits PATH using IFS
  2. Skips Superset's own bin directories to avoid infinite recursion
  3. Checks for executable files (not directories)
  4. Returns proper exit codes

Minor: The pattern matching uses $HOME which may differ from the TypeScript-resolved home directory in edge cases (e.g., sudo -u), but this is acceptable for the use case.


64-70: XDG Base Directory spec compliance.

Correctly handles XDG_CONFIG_HOME with fallback to ~/.config. The .trim() and length check handle edge cases like whitespace-only values.


217-233: Global plugin creation has appropriate error handling.

The try-catch around global plugin path creation/writing is correct—the global path may fail due to permissions, but local plugin creation should still succeed.


137-158: Template literal escaping is correct. The String.fromCharCode(36, 123) generates ${ as intended, and the shellLine produces the correct bash template syntax: await $`bash ${notifyPath} ${payload}`;. The previously flagged missing brace issue has been resolved.


115-116: The -c 'notify=["bash","${notifyPath}"]' syntax is correct and matches the documented Codex CLI format for passing array configuration via the inline -c flag.

When using oh-my-opencode with background subagents (explore, librarian,
oracle, etc.), each subagent runs in its own OpenCode session. These
child sessions emit session.idle events when they complete, causing
excessive notification spam.

This commit adds child session detection by:
- Using client.session.list() to look up sessions by ID
- Checking if session has parentID (indicates subagent session)
- Only sending notifications for main sessions (no parentID)

This mirrors OpenCode's own notification handling in their app.

Refs: https://github.com/sst/opencode/blob/dev/packages/app/src/context/notification.tsx
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: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba72c4 and d93c5e7.

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

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

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Use type safety and avoid any types unless absolutely necessary in TypeScript files

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Move Node.js functionality needed in Electron renderer to src/main/lib/ and communicate via IPC

Files:

  • apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
🧠 Learnings (1)
📓 Common learnings
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
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (2)
apps/desktop/src/main/lib/agent-setup/paths.ts (4)
  • BIN_DIR (4-4)
  • HOOKS_DIR (5-5)
  • OPENCODE_PLUGIN_DIR (9-9)
  • OPENCODE_CONFIG_DIR (8-8)
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
  • getNotifyScriptPath (9-11)
⏰ 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/agent-wrappers.ts (1)

137-141: Previous critical issue has been resolved.

The missing closing brace in the template literal (flagged in previous review) has been correctly fixed. Line 140 now properly generates await $\bash ${notifyPath} ${payload}`;` with both template variables correctly closed.

Comment thread apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
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.

2 participants