feat(pianola): add a "Watched by Pianola" section to the Dashboard - #1271
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Pianola supervisor watch-state derivation and polling, exposes watch controls in the dashboard, and tests filtering, health enrichment, fallback naming, and AI-tab selection. ChangesPianola supervisor watch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PianolaDashboard
participant usePianolaSupervisor
participant SupervisorAPI
PianolaDashboard->>usePianolaSupervisor: refresh or change watch state
usePianolaSupervisor->>SupervisorAPI: list, watch, unwatch, or setEnabled
SupervisorAPI-->>usePianolaSupervisor: targets and health snapshots
usePianolaSupervisor-->>PianolaDashboard: watched and watchable state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 SummaryThis PR adds Pianola watch management to the dashboard. The main changes are:
Confidence Score: 4/5Supervisor snapshot reconciliation can display state that no longer matches the daemon.
src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts Important Files Changed
Reviews (1): Last reviewed commit: "feat(pianola): add a "Watched by Pianola..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
chr1syy
force-pushed
the
followup/pianola-watch-control
branch
from
July 22, 2026 06:43
08d3c37 to
3bfc220
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts (1)
115-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared polling lifecycle
usePianolaSupervisorrepeats the same nonce, cancel flag, and interval cleanup pattern used inusePianolaDashboardData. Pull that lifecycle into a shared hook so only the IPC fetch logic stays local.🤖 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 `@src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts` around lines 115 - 147, The polling lifecycle in usePianolaSupervisor, including nonce state, cancellation, interval setup, and cleanup, should be extracted into a shared hook reused by usePianolaDashboardData. Update usePianolaSupervisor so only its supervisor.list IPC fetch and snapshot state updates remain local, while preserving refresh-triggered polling and cancellation behavior.Source: Path instructions
🤖 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.
Inline comments:
In `@src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts`:
- Around line 123-170: Update the polling effect and the watch, unwatch, and
setEnabled callbacks to handle failures explicitly. Treat the known
PianolaDisabled case as recoverable, but rethrow unexpected polling errors for
Sentry capture; report mutator failures through the canonical renderer
notification utility or hook so users receive feedback instead of unhandled
rejections. Preserve successful snapshot updates and apply the same handling
consistently to all three mutators.
---
Nitpick comments:
In `@src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts`:
- Around line 115-147: The polling lifecycle in usePianolaSupervisor, including
nonce state, cancellation, interval setup, and cleanup, should be extracted into
a shared hook reused by usePianolaDashboardData. Update usePianolaSupervisor so
only its supervisor.list IPC fetch and snapshot state updates remain local,
while preserving refresh-triggered polling and cancellation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e51bc01-6a1e-4bff-add5-7599304605ae
📒 Files selected for processing (3)
src/__tests__/renderer/components/PianolaDashboard/deriveWatchState.test.tssrc/renderer/components/PianolaDashboard/PianolaDashboard.tsxsrc/renderer/components/PianolaDashboard/usePianolaSupervisor.ts
Collaborator
|
Thanks for the contribution, @chr1syy - this is a clean, well-scoped addition that reuses the existing supervisor IPC path rather than adding new backend surface. I reviewed the PR alongside the Greptile and CodeRabbit feedback, and you've already handled the substantive findings in your follow-up commits:
The added CI is green on both the ubuntu and windows matrix legs, and there are no merge conflicts. Approving. 🎉 |
Watching an agent was CLI-only (`maestro-cli pianola supervise watch ...`); the supervisor IPC + preload (`window.maestro.pianola.supervisor.*`) already existed but had no UI. Add a pinned "Watched by Pianola" section at the top of the Pianola Dashboard: it lists current watch targets with live daemon health (status dot + lastError), an enable/disable toggle, and an unwatch button, plus a "+ Watch an agent" picker that adds a watch through the same supervisor path the CLI uses. The picker lists top-level, non-Pianola agents that have an AI tab and aren't already watched; it targets the agent's active AI tab (else the first). The Dashboard's Refresh now refreshes the watch snapshot too.
Address two review P1s in usePianolaSupervisor: - A disabled Pianola (supervisor.list rejects with 'PianolaDisabled') left the last watched/running rows on screen. The poll catch now clears targets+health on that specific rejection; a transient IPC error still keeps the last snapshot so the list doesn't flicker. - A slow poll issued before a mutation could resolve afterward and clobber the mutation's fresher snapshot with stale data. Mutations are now authoritative: each bumps a mutation epoch before and after its IPC call and always applies its returned snapshot, while a poll applies only if the epoch is unchanged on resolve. Adds usePianolaSupervisor.test.ts covering both paths.
Watch/unwatch/enable calls were fire-and-forget (void), so an IPC failure became a silent unhandled rejection with no user feedback. The hook's mutate now catches, shows a red toast naming the action, and reports the error to Sentry.
chr1syy
force-pushed
the
followup/pianola-watch-control
branch
from
July 22, 2026 09:08
f17ef0c to
4a41e77
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@src/__tests__/renderer/components/PianolaDashboard/usePianolaSupervisor.test.ts`:
- Around line 110-118: Update the test for usePianolaSupervisor().watch so the
unexpected “disk full” mutation error is asserted to reject after verifying the
red toast and captureException calls. Preserve the existing reporting
assertions, and only expect resolution if the implementation explicitly
classifies this failure as recoverable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f93e2cdd-5eaa-4bba-befb-10f2618711f5
📒 Files selected for processing (4)
src/__tests__/renderer/components/PianolaDashboard/deriveWatchState.test.tssrc/__tests__/renderer/components/PianolaDashboard/usePianolaSupervisor.test.tssrc/renderer/components/PianolaDashboard/PianolaDashboard.tsxsrc/renderer/components/PianolaDashboard/usePianolaSupervisor.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/components/PianolaDashboard/usePianolaSupervisor.ts
- src/renderer/components/PianolaDashboard/PianolaDashboard.tsx
Match PianolaModal's convention for recoverable IPC mutations: a failed watch/unwatch/enable now toasts and reports to Sentry only when the error is unexpected (isPianolaDisabled gate), then resolves rather than rejecting so a fire-and-forget click never becomes an unhandled rejection. Adds a test that an expected PianolaDisabled mutation failure is toasted but not reported, and makes the recoverable (resolves) classification explicit in the existing test.
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.
Problem
Adding an agent to Pianola's watch list was CLI-only (
maestro-cli pianola supervise watch <tabId> --agent <id>). The supervisor IPC and preload bridge (window.maestro.pianola.supervisor.{list,add,setEnabled,remove}) already existed and are typed inglobal.d.ts, but nothing in the renderer called them, so there was no in-app way to see or manage what Pianola watches.Change
A pinned "Watched by Pianola" section at the top of the Pianola Dashboard (answering "where do we put the button?" - it lives right where you manage the workspace's agents):
watchtargets with the agent name, a live health dot (running / backing-off / failed / disabled from the daemon snapshot), the daemonlastErrorwhen present, an enable/disable toggle, and an unwatch button. Rows click-to-jump to the agent.+ Watch an agentpicker (rem-sized,whitespace-nowrap, click-away) lists top-level, non-Pianola agents that have an AI tab and aren't already watched; selecting one callssupervisor.add({ kind: 'watch', agentId, tabId, enabled: true }). It targets the agent's active AI tab (falling back to the first) - agents with no AI tab are excluded since awatchneeds atabId.PianolaDashboard).All through the same supervisor path the CLI uses - deterministic, reuses the existing gated IPC, no new backend.
Files
usePianolaSupervisor.ts(new) - polledsupervisor.list()hook + add/remove/toggle mutators; purederiveWatchState(sessions, targets, health)returning{ watched, watchable }.PianolaDashboard.tsx-Sectiongains an optionalheaderAction; newWatchedSection; hook lifted to the top component so Refresh covers both.deriveWatchState.test.ts(new) - name/health/enabled enrichment, short-id fallback,lastError, orchestrate-target exclusion, watchable filtering (Pianola / children / already-watched / no-tab), and active-vs-first tab selection.Test
deriveWatchState.test.ts6/6 green; fullnpm run linttypecheck clean; ESLint clean. Like the sibling Pianola PRs, its shard 2/2 inherits the pre-existing brokenSessionListtest onrcuntil #1269 merges and I rebase.Summary by CodeRabbit