Conversation
* save * feat(desktop): wire v2 agent-hook sidebar status + terminalId payload v2 terminals don't carry SUPERSET_PANE_ID, only SUPERSET_TERMINAL_ID, and agent payloads send empty strings for missing fields rather than omitting them. The sidebar status writer now threads terminalId through the whole pipeline (shell hook -> host-service -> event bus -> renderer) and falls back to terminalId/sessionId/hookSessionId as the store key when paneId is blank. Empty strings are now treated as missing (was using ??, now uses a firstNonBlank helper). Adds a workspace-level v2 pane-status store so the dashboard sidebar can render the same working/permission/review indicator the v1 sidebar did, and clears attention statuses when the user views the workspace. Also logs each hop of the pipeline so future debugging doesn't require guessing where the chain breaks. * chore(desktop): hoist v2 agent-hook listener + drop debug logs Mount `V2AgentHookListeners` at the authenticated layout level so every open v2 workspace subscribes for agent-lifecycle events. Backgrounded workspaces now light up the sidebar dot and play the finish sound, not just the currently-viewed one. Multiple listeners per host reuse one WebSocket connection, so this is O(1 socket per host). Strips the per-hop console.logs added while debugging the initial pipeline — they served their purpose (empty-string paneId fallback bug in the coalesce chain, missing terminalId forwarding). * fix(desktop): address PR review on v2 agent-hook sidebar - isCurrentWorkspace now matches v2 routes (`/v2-workspace/...`), not just v1. Without this, Stop events always marked `review` even while the user was viewing the workspace. - shouldSuppress falls back to isCurrentWorkspace + focus when the payload lacks paneId/tabId — v2 terminals only expose terminalId, so the previous early-return made v1-parity suppression dead code. - Native notification `tag` reuses firstNonBlank so v2 events don't collide on `workspaceId:_` and stomp each other. - useClearPaneAttentionOnView now re-runs when a new review status arrives for the viewed workspace (not just on mount), so Stop events arriving while on-page also clear the sidebar dot. - Split WorkspaceListener into its own file per AGENTS.md one-component rule. - Plan doc now references the actual endpoint `/trpc/notifications.hook`. Not addressed in this commit (held for discussion): - HOST_SERVICE_SECRET exposure via SUPERSET_HOST_AGENT_HOOK_TOKEN — needs a hook-scoped token design. - v1 fallback when the v2 POST returns non-2xx — design choice between silent fallback and surfacing v2 failures. - Autoplay priming listener stacking + keyboard-retry path. * fix(host-service): drop auth on notifications.hook, stop leaking PSK notifications.hook only broadcasts a chime + sidebar dot — no state change, no data access, no code execution. Previously it reused the global HOST_SERVICE_SECRET as a bearer, which was both (a) exposed to every agent shell's env as SUPERSET_HOST_AGENT_HOOK_TOKEN and (b) redundant, since that same secret is already stored in the user- readable manifest.json alongside HOST_SERVICE_SECRET. - Route is now publicProcedure with a note explaining the rationale. - Removes SUPERSET_HOST_AGENT_HOOK_TOKEN from v2 PTY env entirely; the agent script only needs the URL. - Shell hook drops the Authorization header. If the endpoint ever grows capabilities beyond "fan out a chime", re-introduce auth with a hook-scoped secret — not the global PSK. * fix(desktop): v1 fallback on v2 non-2xx + safer autoplay priming - Shell hook now captures the status code from the v2 POST (regardless of debug mode) and only exits if it was 2xx. Otherwise falls through to the electron v1 endpoint — covers host-service restarts, crashes, transient 5xxs without silently dropping the notification. - primeRingtoneAudioOnFirstGesture is now idempotent: if called twice before any gesture, listeners are only installed once. Audio is marked primed only after silent.play() resolves. On rejection the listeners are re-armed so the next gesture retries — with both pointerdown and keydown preserved (the retry path previously dropped keydown, breaking keyboard-only users). * docs(plans): rewrite v2 notif hooks doc as shipped retrospective * wip(desktop): partial-merge resolutions for v2 notification hooks Intermediate commit capturing in-progress merge work before resolving remaining conflicts with main. * fix(desktop): simplify v2 notification hook identity * refactor(desktop): normalize v2 notification state * feat(desktop): show v2 notification status on tabs and panes * refactor(desktop): type v2 notification sources * feat(desktop): show v2 native notifications from main * fix(desktop): polish v2 notification focus and audio * [codex] Align ringtone playback with VS Code pattern * [codex] Move v2 notification lifecycle logic * [codex] Refresh notification docs and v1 fallback test
* port v2 terminal hotkeys to v1 * Fix terminal hotkey PR feedback
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (58)
✨ 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…vice path Add v2 POST branch (SUPERSET_HOST_AGENT_HOOK_URL + SUPERSET_TERMINAL_ID) mirroring notify-hook.template.sh. 2xx response exits immediately; non-2xx or network error falls through to v1 localhost endpoint unchanged. Also update the early-exit guard and debug log line to include terminalId.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 768c202cfb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| poolOptions={{ workerFactory: createPierreWorker, poolSize: 8 }} | ||
| highlighterOptions={{ preferredHighlighter: "shiki-wasm" }} | ||
| > | ||
| <AgentHooks /> |
There was a problem hiding this comment.
MainWindowEffects already mounts AgentHooks behind a tear-off ownership guard, so adding a second <AgentHooks /> here mounts useDevicePresence/useCommandWatcher twice in the owning window and also runs them in non-owning tear-off windows (where MainWindowEffects intentionally returns null). That creates duplicate command-watcher/device-registration side effects and can cause unnecessary claim traffic and racey behavior when multiple windows are open.
Useful? React with 👍 / 👎.
| <LanguageServicesProvider /> | ||
| <MainWindowEffects /> | ||
| <WorktreeAutoSyncManager /> | ||
| <V2NotificationController /> |
There was a problem hiding this comment.
V2NotificationController subscribes to host lifecycle events and triggers side effects (ringtone playback and notifications.showNative mutations). Mounting it unconditionally at layout scope means every authenticated renderer window processes the same events; in multi-window/tear-off scenarios this duplicates chimes and notification dispatches. It should be mounted only in the singleton-owner path (similar to MainWindowEffects) or otherwise deduplicated.
Useful? React with 👍 / 👎.
MocA-Love
added a commit
that referenced
this pull request
Apr 25, 2026
upstream の 10 commits は #426 と #427 で fork 固有の差分を保ちながら個別に cherry-pick 取り込み済み。本 merge は ours strategy で **記録だけ** マージ済みに することで behind=0 を達成し、git 履歴上の追跡を正しくする。 Cherry-pick 取り込み済 (PR #426): - 5aab22a fix closed picker filters (superset-sh#3702) → cdb52f9 - 99db5be [codex] simplify workspace controls (superset-sh#3714) → f079606 - 186078a fix(chat): prevent ask_user question from shadowing sandbox access prompt (superset-sh#3662) → 09d6b57 - 47893c2 fix desktop workspace creation title clamp (superset-sh#3718) → 6a8c4ae - 09323ff Add diff pane file viewer action (superset-sh#3715) → 817ed8d - a5891c6 remove pending launch log (superset-sh#3721) → 0764d03 - c83de0c Add Tiptap table support (superset-sh#3719) → e67a885 - 486b621 [codex] Fix v2 terminal lifecycle after sleep (superset-sh#3711) → b71fbbb (+ #426 内 review fixups) Cherry-pick 取り込み済 (PR #427): - e07aef6 feat(desktop): play v2 notification hooks client-side (superset-sh#3675) → 27ac18a - eae6008 [codex] Port v2 terminal hotkeys to v1 (superset-sh#3724) → 05a77b8 (+ #427 内 Windows .ps1 v2 化) Fork 固有領域は変更ゼロで保持: 19 tRPC procedures (workspaces.githubExtended)、 AudioScheduler / Aivis TTS / notification-manager、terminal suggestion handler (新 terminalKeyboardHandler.ts に移植)、TERMINAL_OPTIONS、SUPERSET_WORKSPACE_NAME、 MainWindowEffects、INCEPTION_AUTH_PROVIDER_ID、v1MigrationState、TiptapPromptEditor、 electron-builder.ts (dmg.size="4g", fileAssociations)、Service Status Dashboard、 Linux daemon systemd、Worktree auto-sync、Windows support、DnD scratch route 他。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
upstream バッチ 2026-04-25 の PR B。残り 2 件の commit は fork 固有領域 (AudioScheduler/Aivis、terminal suggestion handler) との統合作業が必要なため、PR A (#426) から分離して取り込む。
ベース: fork main
3a1fb4714(PR A マージ後)取り込み先:
upstream/batch-2026-04-25-pr-b→ main取り込み内容
e07aef63727ac18a52SUPERSET_HOST_AGENT_HOOK_URLに POST、rendererV2NotificationControllerで event bus 経由 ringtone/native notification/pane attentioneae60087405a77b84ahelpers.tsから新規terminalKeyboardHandler.tsに分離その他、レビュー対応 commit 1 件 (
768c202cflint 修正)。Fork 側のコンフリクト解決
e07aef637(superset-sh#3675) —setup-terminal.ts,usePaneRegistry.tsx,page.tsx,layout.tsx,routers/index.ts,notify-hook.template.sh,DashboardSidebarWorkspaceItem.tsxsetup-terminal.tssetup.ps1 / .cmd / .bat) を維持しつつ upstream のeventBus: args.ctx.eventBusを各createTerminalSessionInternal呼び出しに追加notify-hook.template.shterminalId含む、wrapperPid削除) に更新DashboardSidebarWorkspaceItem.tsxuseV2WorkspaceNotificationStatusimport のみ追加usePaneRegistry.tsxgetV2NotificationSourcesForPane+V2NotificationStatusIndicatorと fork のisSpreadsheetFilepage.tsxLayoutNode,SplitPath+ upstream のWorkspaceStoreを両方保持。focusRequestIdをuseConsumeAutomationRunLinkに追加しつつ fork のcollections,rightSidebarOpenViewWidth等のフックも維持layout.tsxLanguageServicesProvider,MainWindowEffects,WorktreeAutoSyncManager+ upstream のAgentHooks,V2NotificationControllerを全て保持routers/index.tsgithubMetrics維持 +notifications: createNotificationsRouter(getWindow)に更新eae600874(superset-sh#3724) —terminalKeyboardHandler.ts,useTerminalLifecycle.ts,useTerminalSuggestion.tsupstream は
helpers.tsのActiveSuggestionHandleinterface とsetupKeyboardHandler関数を新規terminalKeyboardHandler.tsに移動。fork 固有の suggestion options 移植:
helpers.tsのsetupKeyboardHandlerには fork PR で追加された以下の options が存在:activeSuggestionRefcanOpenSuggestionsonOpenSuggestionsこれらを新
terminalKeyboardHandler.tsのKeyboardHandlerOptionsに追加移植。useTerminalLifecycle.tsとuseTerminalSuggestion.tsの import パスを../helpers→../terminalKeyboardHandlerに更新。v2 通知 vs main プロセス AudioScheduler の関係
fork は PR #405 で
apps/desktop/src/main/lib/notifications/audio-scheduler.ts,aivis-tts.ts,notification-manager.tsを導入済 (main プロセス側で ringtone + Aivis TTS 直列化)。e07aef637は v2 通知経路を renderer client-side に移すため、apps/desktop/src/renderer/lib/ringtones/play.tsで renderer 側 ringtone 再生を新規追加:Aivis TTS を v2 にも適用する follow-up は別 issue として残す。
Fork 固有機能ヘルスチェック
workspaces.githubExtended)apps/desktop/src/main/lib/notifications/)helpers.tsの suggestion handler optionsterminalKeyboardHandler.tsに完全移植drizzle migration
2 commits とも
packages/db/drizzle//packages/local-db/drizzle/への変更なし → idx 衝突なし。Test plan
bun install --frozen --ignore-scripts— cleanbun run lint— 0 errorsbunx turbo typecheck --filter=@superset/desktop --filter=@superset/host-service --filter=@superset/chat— 3/3 pass参考
/tmp/upstream-batch-2026-04-25/codex-preanalysis.md