feat(desktop): wire pi terminal agent to notification hooks#4083
Conversation
Follow-up to superset-sh#2562, which added pi as a built-in agent (preset, command, icon, settings UI, docs). This PR adds the corresponding notification-hook integration in apps/desktop/src/main/lib/agent-setup/, so pi participates in the same Start/Stop/PostToolUse lifecycle that drives Superset's working indicator and completion chime for claude, codex, gemini, cursor, copilot, opencode, mastra, amp, and droid. Mechanism: ship a small TypeScript pi extension to the global pi extensions directory (~/.pi/agent/extensions/superset-hooks.ts). The extension subscribes to pi's lifecycle events and spawns notify.sh with Claude-Code-format hook_event_name JSON payloads, which the existing notify.sh dispatcher already speaks natively — no per-agent translator shim needed. Mapping: pi before_agent_start → UserPromptSubmit → Superset Start pi tool_execution_end → PostToolUse → progress pi agent_end → Stop → completion / chime pi session_shutdown → Stop → cleanup on Ctrl+C, /quit, /reload Subagent flicker is prevented by gating every event on ctx.hasUI === false (strict equality, so older pi versions where hasUI is undefined still fire). Structurally identical to the opencode integration: template file + wrapper module + barrel re-export + capabilities target + runners map entry. Mirrors writeFileIfChanged + signature/version marker convention. No changes required to notify.sh, the v2 host-service tRPC endpoint, the renderer event bus, or the server-side mapEventType.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughInstalls a Superset Pi extension under the user's home ( ChangesPi Agent Extension Integration
Sequence DiagramsequenceDiagram
participant Pi as Pi Agent
participant Ext as Pi Extension
participant Notify as notify.sh
participant SS as Superset Renderer
Pi->>Ext: before_agent_start
alt ctx.hasUI !== false
Ext->>Notify: spawn with {"hook_event_name":"UserPromptSubmit"}
Notify->>SS: deliver event to renderer
SS->>SS: show working indicator / chime
end
Pi->>Ext: tool_execution_end
alt ctx.hasUI !== false
Ext->>Notify: spawn with {"hook_event_name":"PostToolUse"}
Notify->>SS: deliver event to renderer
SS->>SS: update progress
end
Pi->>Ext: agent_end / session_shutdown
alt ctx.hasUI !== false
Ext->>Notify: spawn with {"hook_event_name":"Stop"}
Notify->>SS: deliver event to renderer
SS->>SS: clear indicator
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile Summary
Confidence Score: 4/5Safe to merge; one cosmetic marker formatting issue but no functional defects. The integration is structurally sound, follows established patterns, and the notify.sh stdin interface is correct. Only a P2 style issue (double apps/desktop/src/main/lib/agent-setup/templates/pi-extension.template.ts — line 1 marker placeholder
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/lib/agent-setup/agent-wrappers-pi.ts | New wrapper module — follows the established __dirname+writeFileIfChanged pattern correctly; no functional issues found. |
| apps/desktop/src/main/lib/agent-setup/templates/pi-extension.template.ts | Extension template is structurally correct; stdin-piped notify.sh invocation and hasUI gate match the design, but // {{MARKER}} + a //-prefixed MARKER produces a double-// comment in the installed file. |
| apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts | Three new pi test cases follow existing test patterns; mock setup and path assertions are correct. |
| apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts | Barrel re-export of pi symbols; consistent with other agent wrapper modules. |
| apps/desktop/src/main/lib/agent-setup/desktop-agent-capabilities.ts | Adds "pi-extension" action and pi target without managedBinary; follows cursor-agent precedent correctly. |
| apps/desktop/src/main/lib/agent-setup/desktop-agent-setup.ts | Wires createPiExtension into the runners map; straightforward one-liner addition. |
| biome.jsonc | Adds !**/*.template.ts exclusion consistent with existing .template.js/.template.sh exclusions. |
Sequence Diagram
sequenceDiagram
participant Desktop as Desktop App (launch)
participant FS as ~/.pi/agent/extensions/
participant Pi as pi CLI
participant Notify as notify.sh
participant Superset as Superset Host Service
Desktop->>FS: writeFileIfChanged(superset-hooks.ts)
Note over FS: Extension auto-discovered on next pi session start
Pi->>Pi: before_agent_start (ctx.hasUI != false)
Pi->>Notify: spawn + stdin {hook_event_name:UserPromptSubmit}
Notify->>Superset: POST /hook → Start (working indicator on)
Pi->>Pi: tool_execution_end (ctx.hasUI != false)
Pi->>Notify: spawn + stdin {hook_event_name:PostToolUse}
Notify->>Superset: POST /hook → PostToolUse (progress)
Pi->>Pi: agent_end (ctx.hasUI != false)
Pi->>Notify: spawn + stdin {hook_event_name:Stop}
Notify->>Superset: POST /hook → Stop (chime + indicator off)
Note over Pi,Notify: session_shutdown (Ctrl+C / SIGTERM / /quit) also fires Stop as safety net
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/main/lib/agent-setup/templates/pi-extension.template.ts:1
**Double `//` prefix in installed marker line**
`PI_EXTENSION_MARKER` is `"// Superset pi extension v1"` (already starts with `//`), but the template wraps the placeholder in another comment: `// {{MARKER}}`. After substitution the installed extension begins with `// // Superset pi extension v1` — an unintended double-comment prefix.
It is functionally harmless (substring detection still matches), but inconsistent with the opencode plugin template which places `{{MARKER}}` on a bare line so the marker renders cleanly. Fix is either:
- Drop the leading `// ` from the template line (rely on the biome exclusion already added): `{{MARKER}}`
- Or strip `// ` from `PI_EXTENSION_SIGNATURE` so the marker is `"Superset pi extension v1"` and the installed line becomes `// Superset pi extension v1`
Reviews (1): Last reviewed commit: "feat(desktop): wire pi terminal agent to..." | Re-trigger Greptile
| @@ -0,0 +1,98 @@ | |||
| // {{MARKER}} | |||
There was a problem hiding this comment.
Double
// prefix in installed marker line
PI_EXTENSION_MARKER is "// Superset pi extension v1" (already starts with //), but the template wraps the placeholder in another comment: // {{MARKER}}. After substitution the installed extension begins with // // Superset pi extension v1 — an unintended double-comment prefix.
It is functionally harmless (substring detection still matches), but inconsistent with the opencode plugin template which places {{MARKER}} on a bare line so the marker renders cleanly. Fix is either:
- Drop the leading
//from the template line (rely on the biome exclusion already added):{{MARKER}} - Or strip
//fromPI_EXTENSION_SIGNATUREso the marker is"Superset pi extension v1"and the installed line becomes// Superset pi extension v1
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/agent-setup/templates/pi-extension.template.ts
Line: 1
Comment:
**Double `//` prefix in installed marker line**
`PI_EXTENSION_MARKER` is `"// Superset pi extension v1"` (already starts with `//`), but the template wraps the placeholder in another comment: `// {{MARKER}}`. After substitution the installed extension begins with `// // Superset pi extension v1` — an unintended double-comment prefix.
It is functionally harmless (substring detection still matches), but inconsistent with the opencode plugin template which places `{{MARKER}}` on a bare line so the marker renders cleanly. Fix is either:
- Drop the leading `// ` from the template line (rely on the biome exclusion already added): `{{MARKER}}`
- Or strip `// ` from `PI_EXTENSION_SIGNATURE` so the marker is `"Superset pi extension v1"` and the installed line becomes `// Superset pi extension v1`
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 9ce69be — dropped the // from the template line so it now reads bare {{MARKER}}, matching opencode's pattern. After substitution the installed file's first line is the clean // Superset pi extension v1. Tests still pass (29/29), biome clean.
PI_EXTENSION_MARKER already starts with '//' ("// Superset pi extension v1"),
so wrapping the placeholder in another comment produced '// // Superset pi
extension v1' on the installed file's first line. Aligns with opencode's
template, which places {{MARKER}} on a bare line for the same reason.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts (1)
1291-1331: 💤 Low valueTests cover the core scenarios well.
The three new
it()blocks validate marker substitution, default-export shape, and end-to-end installation. Theos.homedirmock is correctly applied via the module-levelmock.module("node:os", ...)which covers thedefault.homedirimport path used inagent-wrappers-pi.ts.One optional addition: an idempotency test that calls
createPiExtension()twice and asserts the file content is unchanged on the second call (mirrors the opencode idempotency tests). Not blocking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts` around lines 1291 - 1331, Add an idempotency test that calls createPiExtension() twice and verifies the file content doesn't change: use getPiExtensionPath() to locate the installed file, call createPiExtension(), read and store the installed content, call createPiExtension() again, read the content again and assert equality, and also assert it still contains PI_EXTENSION_MARKER and the "export default function" shape; reference the existing helpers getPiExtensionPath, getPiExtensionContent, and the createPiExtension and PI_EXTENSION_MARKER symbols in the new `it()` block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts`:
- Around line 1291-1331: Add an idempotency test that calls createPiExtension()
twice and verifies the file content doesn't change: use getPiExtensionPath() to
locate the installed file, call createPiExtension(), read and store the
installed content, call createPiExtension() again, read the content again and
assert equality, and also assert it still contains PI_EXTENSION_MARKER and the
"export default function" shape; reference the existing helpers
getPiExtensionPath, getPiExtensionContent, and the createPiExtension and
PI_EXTENSION_MARKER symbols in the new `it()` block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09a48075-79d1-4b4c-8627-976c6c46d8c8
📒 Files selected for processing (7)
apps/desktop/src/main/lib/agent-setup/agent-wrappers-pi.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.test.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/main/lib/agent-setup/desktop-agent-capabilities.tsapps/desktop/src/main/lib/agent-setup/desktop-agent-setup.tsapps/desktop/src/main/lib/agent-setup/templates/pi-extension.template.tsbiome.jsonc
|
Works great thank you @garritfra! |
* feat(desktop): wire pi terminal agent to notification hooks Follow-up to #2562, which added pi as a built-in agent (preset, command, icon, settings UI, docs). This PR adds the corresponding notification-hook integration in apps/desktop/src/main/lib/agent-setup/, so pi participates in the same Start/Stop/PostToolUse lifecycle that drives Superset's working indicator and completion chime for claude, codex, gemini, cursor, copilot, opencode, mastra, amp, and droid. Mechanism: ship a small TypeScript pi extension to the global pi extensions directory (~/.pi/agent/extensions/superset-hooks.ts). The extension subscribes to pi's lifecycle events and spawns notify.sh with Claude-Code-format hook_event_name JSON payloads, which the existing notify.sh dispatcher already speaks natively — no per-agent translator shim needed. Mapping: pi before_agent_start → UserPromptSubmit → Superset Start pi tool_execution_end → PostToolUse → progress pi agent_end → Stop → completion / chime pi session_shutdown → Stop → cleanup on Ctrl+C, /quit, /reload Subagent flicker is prevented by gating every event on ctx.hasUI === false (strict equality, so older pi versions where hasUI is undefined still fire). Structurally identical to the opencode integration: template file + wrapper module + barrel re-export + capabilities target + runners map entry. Mirrors writeFileIfChanged + signature/version marker convention. No changes required to notify.sh, the v2 host-service tRPC endpoint, the renderer event bus, or the server-side mapEventType. * fix(desktop): drop redundant comment prefix from pi extension template PI_EXTENSION_MARKER already starts with '//' ("// Superset pi extension v1"), so wrapping the placeholder in another comment produced '// // Superset pi extension v1' on the installed file's first line. Aligns with opencode's template, which places {{MARKER}} on a bare line for the same reason.
…-sh#4083) * feat(desktop): wire pi terminal agent to notification hooks Follow-up to superset-sh#2562, which added pi as a built-in agent (preset, command, icon, settings UI, docs). This PR adds the corresponding notification-hook integration in apps/desktop/src/main/lib/agent-setup/, so pi participates in the same Start/Stop/PostToolUse lifecycle that drives Superset's working indicator and completion chime for claude, codex, gemini, cursor, copilot, opencode, mastra, amp, and droid. Mechanism: ship a small TypeScript pi extension to the global pi extensions directory (~/.pi/agent/extensions/superset-hooks.ts). The extension subscribes to pi's lifecycle events and spawns notify.sh with Claude-Code-format hook_event_name JSON payloads, which the existing notify.sh dispatcher already speaks natively — no per-agent translator shim needed. Mapping: pi before_agent_start → UserPromptSubmit → Superset Start pi tool_execution_end → PostToolUse → progress pi agent_end → Stop → completion / chime pi session_shutdown → Stop → cleanup on Ctrl+C, /quit, /reload Subagent flicker is prevented by gating every event on ctx.hasUI === false (strict equality, so older pi versions where hasUI is undefined still fire). Structurally identical to the opencode integration: template file + wrapper module + barrel re-export + capabilities target + runners map entry. Mirrors writeFileIfChanged + signature/version marker convention. No changes required to notify.sh, the v2 host-service tRPC endpoint, the renderer event bus, or the server-side mapEventType. * fix(desktop): drop redundant comment prefix from pi extension template PI_EXTENSION_MARKER already starts with '//' ("// Superset pi extension v1"), so wrapping the placeholder in another comment produced '// // Superset pi extension v1' on the installed file's first line. Aligns with opencode's template, which places {{MARKER}} on a bare line for the same reason.
superset-sh#4101 import agents as v2 terminal presets with live link: - useV2PresetExecution.ts: drop upstream useV2AgentConfigs + useWorkspace import. useWorkspace arrives with superset-sh#4067 (cycle 44a WorkspaceProvider); fork keeps its workspaceId/projectId props until then. - V2AgentsSettings.tsx: adopt upstream initialAgentPresetId prop and useV2AgentConfigs re-import (fork retains the hook elsewhere). - PresetEditorDialog.tsx / V2PresetsSection.tsx: adopt upstream Switch / Link / ExternalLink additions for live-link UX. - useMigrateV1PresetsToV2/: re-delete fork-removed hook; drop import from V2PresetsSection.tsx (call sites were stub-only). - PresetEditorDialog.tsx: dedupe Switch import (one survived from each side of the merge). superset-sh#4107 v2 preset commands in new tabs (skipped): introduces useV2TerminalLauncher hook that takes a launcher prop through useV2PresetExecution / useDefaultContextMenuActions / useDefaultPaneActions / useWorkspacePaneOpeners. This pattern is tightly coupled to superset-sh#4067's WorkspaceProvider (workspaceId/projectId stop being passed down by page.tsx and instead come from useWorkspace()). Deferred to cycle 44a so the launcher and provider land together. superset-sh#4112 include agent args (skipped): depends on superset-sh#4107. superset-sh#4057 / superset-sh#4083 fix-compat: - RunInWorkspacePopoverV2.tsx / OpenInWorkspaceV2.tsx: drop iconUrl from project map and ProjectThumbnail props; pass githubOwner={null} until cycle 41 (superset-sh#3823 v2 project icon settings) lands. Phase 4 cycle 39 net: 3 cherry-picks landed (superset-sh#4057, superset-sh#4083, superset-sh#4101); 2 deferred to cycle 44a (superset-sh#4107, superset-sh#4112). useV2PresetExecution preserves fork's workspaceId/projectId calling convention.
Closes #4082.
Follow-up to #2562, which added pi as a built-in agent (preset, command, icon, settings UI, docs). This PR adds the corresponding notification-hook integration in
apps/desktop/src/main/lib/agent-setup/, so pi participates in the same Start/Stop/PostToolUse lifecycle that drives Superset's working indicator and completion chime for claude, codex, gemini, cursor, copilot, opencode, mastra, amp, and droid.Mechanism
Ship a small TypeScript pi extension to the global pi extensions directory (
~/.pi/agent/extensions/superset-hooks.ts). The extension subscribes to pi's lifecycle events and spawnsnotify.shwith Claude-Code-formathook_event_nameJSON payloads, which the existingnotify.shdispatcher already speaks natively — no per-agent translator shim needed.Install lifecycle: at desktop app launch, the new wrapper writes the extension using the standard
writeFileIfChanged+ signature-marker + version pattern. Pi's auto-discovery picks it up at the next session start. Install is unconditional on whether pi is installed — if pi is later installed via npm, hooks just start working.Event mapping
before_agent_startUserPromptSubmitStarttool_execution_endPostToolUsePostToolUse(progress)agent_endStopStopsession_shutdownStopStop(cleanup on Ctrl+C, SIGTERM, SIGHUP, /quit, /reload, /new, /resume, /fork)Subagent filtering
Every event is gated on
ctx.hasUI === false. Subagents spawned via the bundledsubagentextension run viapi --mode json -pand havehasUI: false; user-facing interactive and RPC sessions havehasUI: true. The strict-equality check (rather than!ctx.hasUI) is intentional: pi versions older than 0.38.0 (wherehasUIdid not yet exist) still fire hooks correctly becauseundefined !== false. Subagent flicker is possible on those older versions only — documented limitation.Structural symmetry
Identical to the opencode integration:
agent-wrappers-pi.ts— wrapper module exportingcreatePiExtension,getPiExtensionPath,getPiExtensionContent,PI_EXTENSION_MARKERtemplates/pi-extension.template.ts— the extension itself, with// {{MARKER}}placeholder substituted at install timeagent-wrappers.ts— barrel re-exportdesktop-agent-capabilities.ts— adds"pi-extension"action and{ id: "pi", setupActions: ["pi-extension"] }target (nomanagedBinary— pi is user-installed via npm; cursor-agent is precedent)desktop-agent-setup.ts— runners map entryNo changes required to
notify.sh, the v2 host-service tRPC endpoint, the renderer event bus, or the server-sidemapEventType.Failure modes
All silent: notify.sh missing, spawn errors (EACCES/ENOENT), broken pipe, missing curl. Outside Superset (no
SUPERSET_TERMINAL_ID/SUPERSET_TAB_ID/SUPERSET_PANE_IDset) the extension is a complete no-op.Pi version compatibility
Soft minimum (not enforced): pi 0.31.0 introduced
before_agent_startandsession_shutdown; 0.38.0 introducedctx.hasUI; 0.52.10 introducedtool_execution_end; 0.67.3 madesession_shutdownfire on SIGHUP/SIGTERM. Pi'spi.on(eventName, handler)is permissive — unknown event names silently never fire. Extension degrades gracefully on older versions.Tests
Three new
it()blocks underdescribe("agent-wrappers pi", ...)inagent-wrappers.test.ts:getPiExtensionContent()returns a string containingPI_EXTENSION_MARKER(cleanup recognition).getPiExtensionContent()returns a string containingexport default function(valid pi extension shape).createPiExtension()writes to~/.pi/agent/extensions/superset-hooks.tswith marker present.Local validation:
bun test apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts→ 29 pass / 0 failbunx biome check apps/desktop/src/main/lib/agent-setup/ biome.jsonc→ cleanbun --filter='./apps/desktop' typecheck→ exit 0bun run lint(whole repo) → cleanEnd-to-end was smoke-tested against
notify.shduring the design conversation:SUPERSET_DEBUG_HOOKS=1 echo '{"hook_event_name":"UserPromptSubmit"}' | ~/.superset/hooks/notify.shreturned HTTP 200 for all three managed event types (UserPromptSubmit,PostToolUse,Stop).Out of scope (flagged for visibility)
SUPERSET_HOST_AGENT_HOOK_URLnot set in v2 hook env) — affects all agents; will mask pi's improvements until fixed independently.Stophook transitions pane toidleinstead ofreview) — affects all agents; same disposition.before_permission_requestlifecycle event; permission UI is in-process viactx.ui.confirm. Reaching parity with Claude requires a new pi feature filed upstream atbadlogic/pi-mono.cleanupGlobalPiExtension) — not needed in v1; no prior install location to migrate from.apps/docs/content/docs/agent-integration.mdxalready promises this works for "agents" generically and lists pi under Supported Agents. This PR makes that promise true; pi-specific copy under Notifications would be off-pattern.Allowing edits from maintainers per
CONTRIBUTING.md. Requesting review from @Kitenite (shipped #2562, has full pi context).Summary by cubic
Wired the
pidesktop agent to Superset’s notification hooks so the working indicator and completion chime fire during runs. Installs a globalpiextension that maps lifecycle events tonotify.sh, addressing #4082.New Features
~/.pi/agent/extensions/superset-hooks.tsat app launch (idempotent, version-marked).pievents to Claude-Code hooks sent tonotify.sh: Start,PostToolUse, Stop.ctx.hasUI === false; no-op outside Superset or whennotify.shis missing; failures are silent.Bug Fixes
Written for commit 9ce69be. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Chores