fix(cue): dormant visibility, shared-workspace dedup, Run Now dedup, pipeline deletion, UX polish - #902
Conversation
…Now dedup, pipeline deletion #866: remove if(!deps.enabled()) guard from getStatus/getGraphData so sessions with a config file on disk are always surfaced as dormant (enabled:false). #867: add sharedTriggerOwners registry (first-registered-wins) so unowned subs at a shared projectRoot only create one trigger source instead of N. #868: deduplicate by pipeline_name in SessionsTable Run Now onClick — fire one representative per group so the engine's anchor-group logic fires it once, not N×N. #847: filter error-node pipelines out of handleSave instead of aborting; valid changes (including deletions) now persist with a warning toast for skipped pipelines. Tests: 3 new test files (cue-query-service, cue-shared-trigger-dedup, SessionsTable), updated usePipelinePersistence and cue-session-registry tests. cue-engine test assertions updated to reflect correct dormant-session semantics. 26 325 tests passing, 0 failures.
…stry dedup RC brought two Cue-related commits since the cue-polish merge point: #849 — resolvePipelinesWriteRoots: seed lastWrittenRootsRef with per-pipeline write roots (common ancestor) rather than each agent's individual projectRoot, so delete+save clears YAML at the correct location. #873 — owner_agent_id: computeOwnershipWarning + runnableSubscriptions filter in cue-session-runtime-service resolves shared-workspace trigger duplication at init time, surfaces a dashboard warning, and respects explicit YAML config. Our cue-polish commit contained a registry-level first-registered-wins approach for the same #867 problem. That approach is now superseded: runnableSubscriptions already filters unowned subs for non-owner sessions before the trigger-source loop runs, so the claimSharedTriggerOwner check was unreachable for non-owners and redundant for the owner. Resolution: - Keep RC's ownershipWarning/runnableSubscriptions (#873) — more complete, user-visible, handles explicit owner_agent_id YAML config - Remove sharedTriggerOwners map + 3 methods from cue-session-registry.ts - Remove claimSharedTriggerOwner + releaseSharedTriggersForSession calls from cue-session-runtime-service.ts - Delete cue-shared-trigger-dedup.test.ts (tests the removed registry approach; cue-session-state.test.ts from #873 covers computeOwnershipWarning instead) - Keep all other cue-polish fixes: #866 dormant visibility, #868 Run Now dedup, #847 error-node pipeline filter, SessionsTable ownershipWarning indicator 26 371 tests passing, 0 failures.
…stency in activeRunCount decrements - Add InfoTooltip component to CueSettingsPanel with placement prop (above/below) to explain each setting inline; first setting uses placement=below to avoid overlap with the canvas header - Fix ?? 1 → ?? 0 in activeRunCount decrement paths (finally block and stopRun) to be consistent with all read-side ?? 0 defaults in the same file
…base Replaced 7 mixed patterns (some were single-trigger examples, not coordination) with 6 accurate patterns grounded in real YAML fields: Sequential Pipeline, Fan-Out, Fan-In (Gather), Swarm, Command Action, and Task Queue.
📝 WalkthroughWalkthroughCue now always reports dormant/on-disk sessions regardless of runtime enablement; pipeline persistence skips pipelines with unresolved-agent error nodes instead of aborting; session-triggering deduplicates by pipeline name; assorted tests and UI/help content updated to match these behaviors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR fixes five distinct bugs in the Cue subsystem: dormant agent visibility, shared-workspace subscription deduplication, Run Now N×N firing, pipeline deletion being blocked by error-node pipelines, and a run-count decrement default mismatch. The fixes are individually well-reasoned and the new tests cover the primary scenarios.
Confidence Score: 3/5Mostly safe to merge, but there is a narrow data-loss path in usePipelinePersistence that can silently delete a user's cue.yaml when all agents in a pipeline have been replaced by error nodes. The P1 in usePipelinePersistence is a genuine silent data-loss path: errorPipelineRoots misses fully-broken pipelines (no surviving agent nodes), so deleteYaml fires on the user's root without explicit intent. All other fixes are correct and well-tested. The P1 pulls the score below the 4/5 P1 ceiling. src/renderer/hooks/cue/usePipelinePersistence.ts — the errorPipelineRoots computation and the validPipelines.length === 0 safety-net bypass Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[handleSave called] --> B{settingsLoaded?}
B -- No --> C[Warning toast: settings loading]
B -- Yes --> D[flushAllPendingEdits]
D --> E[Partition: validPipelines vs pipelinesWithErrors]
E --> F{pipelinesWithErrors > 0?}
F -- Yes --> G[Warning toast: skipped pipelines]
G --> H[validatePipelines on validPipelines]
F -- No --> H
H --> I[Partition by project root]
I --> J[Compute errorPipelineRoots from agent nodes in error pipelines]
J --> K{validPipelines > 0 AND pipelinesByRoot empty AND no errors?}
K -- Yes --> L[Push safety-net error]
K -- No --> M{errors.length > 0?}
L --> M
M -- Yes --> N[setValidationErrors, return]
M -- No --> O[Write YAML for each root]
O --> P[Delete orphaned roots not in currentRoots AND not in errorPipelineRoots]
P --> Q[Refresh sessions]
Q --> R[savedStateRef = validPipelines]
R --> S[Success toast]
style J fill:#f9a,stroke:#c33
style P fill:#f9a,stroke:#c33
|
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
| const errorPipelineRoots = new Set<string>(); | ||
| for (const p of pipelinesWithErrors) { | ||
| for (const node of p.nodes) { | ||
| if (node.type === 'agent') { | ||
| const root = resolveRoot(node.data as AgentNodeData); | ||
| if (root) errorPipelineRoots.add(root); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
errorPipelineRoots misses fully-broken pipelines, enabling silent YAML deletion
errorPipelineRoots is populated only from node.type === 'agent' nodes inside error-flagged pipelines. If all agents in a broken pipeline have been replaced by error nodes (the only agent was deleted), no agent node survives, so errorPipelineRoots remains empty for that pipeline. Its root is therefore not guarded by the if (errorPipelineRoots.has(root)) continue check and will be deleted from disk when it appears in previousRoots.
Concretely: a user has Pipeline-X with one agent at /proj-b. That agent is deleted; the pipeline shows as broken. The user hits Save. pipelinesWithErrors = [Pipeline-X], validPipelines = [], errorPipelineRoots = {}. The orphaned-root loop deletes /proj-b, wiping the user's cue.yaml without any explicit delete intent.
The existing tests do not cover this path — 'persists deletion of valid pipeline...' only places /proj-a in previousRoots; the fully-broken pipeline's root is never in previousRoots in that test, so the guard is never exercised for that case.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/hooks/cue/usePipelinePersistence.ts (2)
254-360:⚠️ Potential issue | 🟡 Minor
lastWrittenRootsRefloses error-pipeline roots, risking orphaned YAML on a follow-up save.The deletion loop correctly skips
errorPipelineRoots(line 329), but on line 360lastWrittenRootsRef.current = new Set(currentRoots)drops those protected roots entirely. Trace:
- Initial: A valid at
/proj-a, B valid at/proj-b. Save →lastWrittenRootsRef = {/proj-a, /proj-b}.- B's session reference breaks (one error node, one resolvable agent at
/proj-b). User saves.validPipelines=[A],errorPipelineRoots={/proj-b}./proj-bis protected from deletion ✓ butlastWrittenRootsRefis reset to{/proj-a}—/proj-bis forgotten.- User deletes B from the editor (without fixing). Save →
previousRoots={/proj-a}./proj-b's stale YAML on disk is never cleaned up.The new test on lines 295–332 verifies step 2 but doesn't follow through to step 3. Suggested fix:
🛡️ Preserve error-pipeline roots in the written-set
- savedStateRef.current = JSON.stringify(validPipelines); - lastWrittenRootsRef.current = new Set(currentRoots); + savedStateRef.current = JSON.stringify(validPipelines); + // Keep error-pipeline roots that were previously written so a + // subsequent save (after the user deletes the error pipeline + // without fixing it) still cleans up the stale YAML. + const preservedErrorRoots = [...errorPipelineRoots].filter((r) => + previousRoots.has(r) + ); + lastWrittenRootsRef.current = new Set([...currentRoots, ...preservedErrorRoots]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/cue/usePipelinePersistence.ts` around lines 254 - 360, The bug is that lastWrittenRootsRef.current is reset to only currentRoots, dropping roots in errorPipelineRoots and allowing their on-disk YAML to become orphaned later; fix by preserving protected error roots when updating lastWrittenRootsRef (e.g. assign it to the union of currentRoots and errorPipelineRoots or otherwise merge errorPipelineRoots into lastWrittenRootsRef.current) so lastWrittenRootsRef (used as previousRoots) continues to include roots referenced by pipelines with unresolved agent nodes.
268-381:⚠️ Potential issue | 🟡 MinorMisleading success toast when only error-node pipelines exist and there are no previous roots.
If
currentPipelinesis entirely error pipelines andpreviousRootsis empty:
validPipelines.length === 0→ the "Nothing to save" guard on line 271 is skipped.pipelinesByRootis empty,previousRootsis empty → no writes, no deletes,totalPipelinesWritten=0,rootsCleared=0.- The function still falls through to the success path:
setSaveStatus('success'),setIsDirty(false), and the toast"Saved 0 pipelines to 0 projects."(line 380) — appearing on top of the "Some pipelines skipped" warning.The first test case (lines 244–262) only asserts no
writeYamlcall; it doesn't catch the spurious success toast. Suggest either returning after the warning when nothing will happen, or suppressing the success toast when nothing was actually written or cleared:♻️ Suppress success toast on no-op partial save
const rootLabel = currentRoots.size === 1 ? 'project' : 'projects'; const pipelineLabel = totalPipelinesWritten === 1 ? 'pipeline' : 'pipelines'; const clearedSuffix = rootsCleared > 0 ? ` (cleared ${rootsCleared} empty ${rootsCleared === 1 ? 'project' : 'projects'})` : ''; - notifyToast({ - type: 'success', - title: 'Cue pipelines saved', - message: `Saved ${totalPipelinesWritten} ${pipelineLabel} to ${currentRoots.size} ${rootLabel}${clearedSuffix}.`, - }); + // Only surface the success toast when something actually changed on disk. + // Pure warning-only saves (only error pipelines, no roots to clear) would + // otherwise produce a confusing "Saved 0 pipelines to 0 projects" message + // alongside the skip warning. + if (totalPipelinesWritten > 0 || rootsCleared > 0) { + notifyToast({ + type: 'success', + title: 'Cue pipelines saved', + message: `Saved ${totalPipelinesWritten} ${pipelineLabel} to ${currentRoots.size} ${rootLabel}${clearedSuffix}.`, + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/cue/usePipelinePersistence.ts` around lines 268 - 381, The save flow can reach the success branch with zero writes/deletes, producing a misleading success toast; after the write/delete loops check if totalPipelinesWritten === 0 && rootsCleared === 0 and treat the save as a no-op: setSaveStatus('idle') (or an appropriate non-success status), avoid calling setIsDirty(true/false) changes that would be incorrect, do NOT call onSaveSuccess, do NOT call notifyToast, and return early; locate and modify the block around totalPipelinesWritten, rootsCleared, setSaveStatus, onSaveSuccess, and notifyToast in usePipelinePersistence (the code that runs after the session refresh loop) to implement this early return/ suppression of the success notification.
🧹 Nitpick comments (5)
src/renderer/components/CueHelpModal.tsx (1)
637-697: Optional: consolidate Fan-Out / Fan-In coverage with the new Coordination Patterns section.The pre-existing "Multi-Agent Orchestration" section (lines 637–697) and the new "Coordination Patterns" section (lines 700+) both describe Fan-Out and Fan-In with overlapping prose, examples, and ASCII diagrams. Now that Coordination Patterns presents the full set of six patterns with consistent diagrams (and Fan-In there is more accurate — explicitly noting the array form for
source_session), the older section is largely redundant.Consider either folding the unique bits into Coordination Patterns and removing the older section, or trimming Multi-Agent Orchestration to a one-liner that links to the patterns below. Not a blocker — purely a docs-clarity nit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueHelpModal.tsx` around lines 637 - 697, The "Multi-Agent Orchestration" block in CueHelpModal is redundant with the new "Coordination Patterns" section; either remove the entire Multi-Agent Orchestration <section> (keeping the Coordination Patterns content only) or replace it with a one-line pointer that links to the Coordination Patterns section; if any unique detail must be retained, merge the accurate Fan-In note about using an array for source_session and the preferred ASCII diagram into the Coordination Patterns section (search for the "Multi-Agent Orchestration" heading inside the CueHelpModal component and update or remove that <section> accordingly).src/__tests__/main/cue/cue-query-service.test.ts (1)
84-102: Optional: add coverage for disabled subscriptions in the dormant path.The dormant
subscriptionCountis computed viacountActiveSubscriptions, which excludesenabled === falseentries. None of the dormant tests mix enabled and disabled subs, so a regression that swappedcountActiveSubscriptionsfor plainlength(or forisSubscriptionParticipantfilter without the enabled check) would not be caught here. Consider adding a single test where a dormant session's config has one enabled and one disabled sub and assertssubscriptionCount === 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-query-service.test.ts` around lines 84 - 102, Add a test variant for the dormant path that verifies disabled subscriptions are excluded from subscriptionCount: update the existing test that calls makeSession('s1'), makeConfig(...) and makeDeps(...) and createCueQueryService(...)/svc.getStatus() to provide a config with two subscriptions (one with enabled: true and one with enabled: false) and assert result[0].subscriptionCount === 1; this ensures countActiveSubscriptions (used by getStatus) is exercised and that disabled subs are not counted.src/main/cue/cue-query-service.ts (1)
63-83: loadCueConfig performs synchronous disk I/O without memoization on every status/graph-data query.
readCueConfigFile()callsfs.readFileSync()directly (line 47 in cue-config-repository.ts) with no caching. SincegetStatus()andgetGraphData()invokedeps.loadConfigForProjectRoot()once per dormant session on every call, and these methods are exposed via IPC handlers invoked by React components (e.g., whenactiveTabchanges in useCueGraphData), this creates redundant disk reads — especially problematic if there are multiple dormant sessions or if the query methods are called frequently. Consider memoizingloadCueConfigresults (e.g., keyed byprojectRoot) or deferring to an async loader that can batch/cache across multiple query invocations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-query-service.ts` around lines 63 - 83, The loadCueConfig path (readCueConfigFile -> deps.loadConfigForProjectRoot) is doing synchronous fs.readFileSync on every getStatus()/getGraphData() call for dormant sessions; add memoization or an async cached loader keyed by projectRoot to avoid repeated disk I/O. Update the implementation behind loadConfigForProjectRoot (or export a new cachedLoadConfigForProjectRoot) so callers like getStatus() and getGraphData() use the cached async loader; ensure cache invalidation or TTL when files change (or expose a manual invalidate method) and keep the API signature so callers (deps.loadConfigForProjectRoot, getStatus, getGraphData) can remain unchanged.src/__tests__/renderer/hooks/cue/usePipelinePersistence.test.ts (2)
264-293: UnusedpipelineAconstant.
pipelineA(lines 268–273) is constructed but never referenced — onlypipelineBis passed tosetup({ pipelines: [pipelineB], ... }). The intent reads from the comment ("User deletes A → A is no longer in currentPipelines"), but constructing an unused object is misleading and will trip lint rules in stricter configurations.♻️ Remove the unused construct
- // Valid pipeline A (has proper agent) + Broken pipeline B (error node) - // User deletes A → A is no longer in currentPipelines - // Save should: skip B (error), write nothing (A deleted), and NOT block - const pipelineA = pipeline( - 'p-a', - 'Pipeline A', - [triggerNode('t1'), agentNode('a1', 'Alpha')], - [{ id: 'e1', source: 't1', target: 'a1' }] - ); - const pipelineB = pipeline('p-b', 'Broken B', [triggerNode('t2'), makeErrorNode('e-b')]); + // Pipeline A was previously saved at /proj-a but the user has now deleted + // it (so it's NOT in currentPipelines below). Pipeline B has an error + // node. Save should: skip B, clean up A's root, and NOT block. + const pipelineB = pipeline('p-b', 'Broken B', [triggerNode('t2'), makeErrorNode('e-b')]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/hooks/cue/usePipelinePersistence.test.ts` around lines 264 - 293, Remove the unused pipelineA local from the test to avoid lint noise: delete the pipelineA construction (the call to pipeline that assigns to pipelineA) and keep the rest of the test as-is so setup({ pipelines: [pipelineB], ... }) reflects the intended "A deleted" scenario; no changes needed to setup, handleSave, mockDeleteYaml, or mockNotifyToast assertions.
334-356: Coverage gap: assert subsequent save still cleans up an error-pipeline root after the error pipeline is removed.This test verifies
savedStateRefexcludes the broken pipeline, but doesn't exercise thelastWrittenRootsReffollow-through. Combined with the partial-save scenario whereerrorPipelineRootsprotects a root from deletion, it would be valuable to add a test that:
- Saves with [valid A at
/proj-a, error B with a resolvable agent at/proj-b] (previousRoots = {/proj-a, /proj-b}).- Verifies
/proj-bis protected (already covered).- Removes B from the editor and saves again.
- Asserts
deleteYaml('/proj-b')is now called.This catches the orphaned-YAML scenario flagged on
usePipelinePersistence.ts:360.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/hooks/cue/usePipelinePersistence.test.ts` around lines 334 - 356, Add a new test that reproduces the orphaned-YAML scenario by: 1) initializing the hook with two pipelines (valid pipeline A at project root "/proj-a" and a pipeline B that initially contains a resolvable agent at "/proj-b"), 2) calling handleSave() and asserting that lastWrittenRootsRef or errorPipelineRoots protected "/proj-b" (existing behavior), 3) removing pipeline B from the editor state (simulate user delete) and calling handleSave() again, and 4) asserting that deleteYaml was invoked for "/proj-b" and that lastWrittenRootsRef no longer contains "/proj-b"; target symbols to locate code are handleSave, lastWrittenRootsRef, errorPipelineRoots, and deleteYaml in usePipelinePersistence and the existing test file so the new test plugs into the same setup/cleanup helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/CueModal/SessionsTable.tsx`:
- Around line 154-167: The current dedup in SessionsTable (seenPipelineNames +
loop over subs calling onTriggerSubscription) only keys by pipeline_name and
will drop distinct trigger groups that share a pipeline; change the dedup key to
include the trigger group identity (use the same fields as the engine's
triggerGroupKey) so each unique event config is allowed through. Either (A)
extract and export the existing triggerGroupKey helper into a shared module and
call it from SessionsTable, or (B) replicate the triggerGroupKey computation
locally in SessionsTable to produce a composite key (e.g.,
`${pipeline_name}|${triggerGroupKey(...)}`) and use that in the Set instead of
just pipeline_name; update the code that iterates subs (the for loop and
seenPipelineNames Set) to use this composite key and add tests covering multiple
triggers under the same pipeline_name.
In `@src/renderer/components/CuePipelineEditor/panels/CueSettingsPanel.tsx`:
- Around line 20-60: InfoTooltip currently only toggles visibility on mouse
events and the HelpCircle SVG isn't keyboard focusable or exposed to assistive
tech; update InfoTooltip to make the trigger focusable (either wrap HelpCircle
in a button/span with tabIndex=0 or give HelpCircle tabIndex=0), add
onFocus/onBlur handlers that mirror onMouseEnter/onMouseLeave to setVisible,
generate a stable id for the tooltip content and add role="tooltip" plus
aria-describedby on the trigger (or aria-label/title as fallback) so screen
readers can access the text; ensure pointerEvents/presentation attributes are
adjusted so the element remains interactive for keyboard users while preserving
visual styling.
---
Outside diff comments:
In `@src/renderer/hooks/cue/usePipelinePersistence.ts`:
- Around line 254-360: The bug is that lastWrittenRootsRef.current is reset to
only currentRoots, dropping roots in errorPipelineRoots and allowing their
on-disk YAML to become orphaned later; fix by preserving protected error roots
when updating lastWrittenRootsRef (e.g. assign it to the union of currentRoots
and errorPipelineRoots or otherwise merge errorPipelineRoots into
lastWrittenRootsRef.current) so lastWrittenRootsRef (used as previousRoots)
continues to include roots referenced by pipelines with unresolved agent nodes.
- Around line 268-381: The save flow can reach the success branch with zero
writes/deletes, producing a misleading success toast; after the write/delete
loops check if totalPipelinesWritten === 0 && rootsCleared === 0 and treat the
save as a no-op: setSaveStatus('idle') (or an appropriate non-success status),
avoid calling setIsDirty(true/false) changes that would be incorrect, do NOT
call onSaveSuccess, do NOT call notifyToast, and return early; locate and modify
the block around totalPipelinesWritten, rootsCleared, setSaveStatus,
onSaveSuccess, and notifyToast in usePipelinePersistence (the code that runs
after the session refresh loop) to implement this early return/ suppression of
the success notification.
---
Nitpick comments:
In `@src/__tests__/main/cue/cue-query-service.test.ts`:
- Around line 84-102: Add a test variant for the dormant path that verifies
disabled subscriptions are excluded from subscriptionCount: update the existing
test that calls makeSession('s1'), makeConfig(...) and makeDeps(...) and
createCueQueryService(...)/svc.getStatus() to provide a config with two
subscriptions (one with enabled: true and one with enabled: false) and assert
result[0].subscriptionCount === 1; this ensures countActiveSubscriptions (used
by getStatus) is exercised and that disabled subs are not counted.
In `@src/__tests__/renderer/hooks/cue/usePipelinePersistence.test.ts`:
- Around line 264-293: Remove the unused pipelineA local from the test to avoid
lint noise: delete the pipelineA construction (the call to pipeline that assigns
to pipelineA) and keep the rest of the test as-is so setup({ pipelines:
[pipelineB], ... }) reflects the intended "A deleted" scenario; no changes
needed to setup, handleSave, mockDeleteYaml, or mockNotifyToast assertions.
- Around line 334-356: Add a new test that reproduces the orphaned-YAML scenario
by: 1) initializing the hook with two pipelines (valid pipeline A at project
root "/proj-a" and a pipeline B that initially contains a resolvable agent at
"/proj-b"), 2) calling handleSave() and asserting that lastWrittenRootsRef or
errorPipelineRoots protected "/proj-b" (existing behavior), 3) removing pipeline
B from the editor state (simulate user delete) and calling handleSave() again,
and 4) asserting that deleteYaml was invoked for "/proj-b" and that
lastWrittenRootsRef no longer contains "/proj-b"; target symbols to locate code
are handleSave, lastWrittenRootsRef, errorPipelineRoots, and deleteYaml in
usePipelinePersistence and the existing test file so the new test plugs into the
same setup/cleanup helpers.
In `@src/main/cue/cue-query-service.ts`:
- Around line 63-83: The loadCueConfig path (readCueConfigFile ->
deps.loadConfigForProjectRoot) is doing synchronous fs.readFileSync on every
getStatus()/getGraphData() call for dormant sessions; add memoization or an
async cached loader keyed by projectRoot to avoid repeated disk I/O. Update the
implementation behind loadConfigForProjectRoot (or export a new
cachedLoadConfigForProjectRoot) so callers like getStatus() and getGraphData()
use the cached async loader; ensure cache invalidation or TTL when files change
(or expose a manual invalidate method) and keep the API signature so callers
(deps.loadConfigForProjectRoot, getStatus, getGraphData) can remain unchanged.
In `@src/renderer/components/CueHelpModal.tsx`:
- Around line 637-697: The "Multi-Agent Orchestration" block in CueHelpModal is
redundant with the new "Coordination Patterns" section; either remove the entire
Multi-Agent Orchestration <section> (keeping the Coordination Patterns content
only) or replace it with a one-line pointer that links to the Coordination
Patterns section; if any unique detail must be retained, merge the accurate
Fan-In note about using an array for source_session and the preferred ASCII
diagram into the Coordination Patterns section (search for the "Multi-Agent
Orchestration" heading inside the CueHelpModal component and update or remove
that <section> accordingly).
🪄 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: d1ca4d91-0fe8-4c86-a2b1-65defb4230ba
📒 Files selected for processing (13)
src/__tests__/main/cue/cue-engine.test.tssrc/__tests__/main/cue/cue-query-service.test.tssrc/__tests__/main/cue/cue-session-registry.test.tssrc/__tests__/renderer/components/CueHelpModal.test.tsxsrc/__tests__/renderer/components/CueModal/SessionsTable.test.tsxsrc/__tests__/renderer/hooks/cue/usePipelinePersistence.test.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-query-service.tssrc/main/cue/cue-run-manager.tssrc/renderer/components/CueHelpModal.tsxsrc/renderer/components/CueModal/SessionsTable.tsxsrc/renderer/components/CuePipelineEditor/panels/CueSettingsPanel.tsxsrc/renderer/hooks/cue/usePipelinePersistence.ts
💤 Files with no reviewable changes (1)
- src/main/cue/cue-engine.ts
| // Deduplicate by pipeline_name: the engine fires all | ||
| // sibling subs in a group when any one is triggered | ||
| // (anchor-group logic). Firing every sub individually | ||
| // would re-fire the whole group N times. Fire one | ||
| // representative per group; ungrouped subs (no | ||
| // pipeline_name) fire individually. | ||
| const seenPipelineNames = new Set<string>(); | ||
| for (const sub of subs) { | ||
| if (sub.pipeline_name) { | ||
| if (seenPipelineNames.has(sub.pipeline_name)) continue; | ||
| seenPipelineNames.add(sub.pipeline_name); | ||
| } | ||
| onTriggerSubscription(sub.name); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether single-session multi-trigger pipelines are reachable in this UI
rg -nP --type=ts -C5 '\btriggerGroupKey\s*\('
ast-grep --pattern 'function triggerGroupKey($_) { $$$ }'Repository: RunMaestro/Maestro
Length of output: 5866
🏁 Script executed:
# Get the full triggerGroupKey implementation from backend
sed -n '77,110p' src/main/cue/cue-engine.tsRepository: RunMaestro/Maestro
Length of output: 1097
🏁 Script executed:
# Get the full triggerGroupKey implementation from frontend
sed -n '278,310p' src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tsRepository: RunMaestro/Maestro
Length of output: 1132
🏁 Script executed:
# Check the CueSubscription type definition to see what fields are available
rg -A 30 'type CueSubscription\s*=' --type=ts | head -50Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Check the subscription type used in SessionsTable at line 141
sed -n '130,175p' src/renderer/components/CueModal/SessionsTable.tsxRepository: RunMaestro/Maestro
Length of output: 2377
🏁 Script executed:
# Check if triggerGroupKey is exported from either module
rg 'export.*triggerGroupKey' --type=ts
# Check tests for SessionsTable to see if multi-trigger-per-pipeline is tested
sed -n '1,50p' src/renderer/components/CueModal/SessionsTable.test.tsx
# Check if there are tests that verify the dedup behavior
rg -A 20 'Run Now|seenPipelineNames|dedup' src/renderer/components/CueModal/SessionsTable.test.tsx | head -80Repository: RunMaestro/Maestro
Length of output: 257
🏁 Script executed:
# Find test files for SessionsTable
find . -name "*SessionsTable*" -type f
# Check if triggerGroupKey is exported from backend
rg 'export.*triggerGroupKey' src/main/cue/cue-engine.ts
# Check what's exported from cue-engine
rg 'export' src/main/cue/cue-engine.ts | head -20
# Check CueSubscription type definition across the codebase
rg 'interface CueSubscription|type CueSubscription' --type=ts -A 15 | head -80Repository: RunMaestro/Maestro
Length of output: 5283
🏁 Script executed:
# Read the test file to see if multi-trigger scenarios are tested
cat src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx | head -200
# Check if there's any test coverage for the dedup logic
rg -A 30 'Run Now|seenPipelineNames' src/__tests__/renderer/components/CueModal/SessionsTable.test.tsxRepository: RunMaestro/Maestro
Length of output: 11238
🏁 Script executed:
# Check if triggerGroupKey is a private function (not exported)
rg 'function triggerGroupKey' src/main/cue/cue-engine.ts -B 2
# Check the CueSubscription type to see all available fields
sed -n '1,100p' src/shared/cue/contracts.ts | grep -A 50 'export interface CueSubscription'
# Search for any existing shared utility that computes triggerGroupKey
rg 'triggerGroupKey' --type=ts | grep -v 'test' | grep -v 'src/main/cue'Repository: RunMaestro/Maestro
Length of output: 933
🏁 Script executed:
# Check if there's a shared utilities module where triggerGroupKey could be extracted
find src/shared -name "*.ts" -type f | head -20
# Look for what fields CueSubscription has beyond what we've seen
sed -n '1,150p' src/shared/cue/contracts.ts | tail -100Repository: RunMaestro/Maestro
Length of output: 5195
🏁 Script executed:
# Check if there's a shared/cue module where triggerGroupKey could be re-exported from backend
ls -la src/shared/cue/
# Check if cue-types.ts is available in shared
rg 'CueEventType|CueSubscription' src/shared/cue/ --type=ts -l
# Check the actual full CueSubscription definition to see all fields
sed -n '70,180p' src/shared/cue/contracts.tsRepository: RunMaestro/Maestro
Length of output: 6036
🏁 Script executed:
# Check the backend test that covers morning/evening scenario mentioned in the original comment
rg -B 5 -A 30 'does NOT fire unrelated triggers that share pipeline_name but differ in event config' src/main/cue/Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Search for tests in cue-engine.test.ts around the line mentioned
rg -A 20 'morning|evening' src/main/cue/ --type=ts | head -100
# Check for any test files related to cue-engine
find . -name "*cue-engine*test*" -o -name "*test*cue-engine*"
# Look for tests that verify dedup/anchor-group logic
rg 'anchor.*group|grouping.*trigger' src/main/cue/ --type=ts -A 5 | head -80Repository: RunMaestro/Maestro
Length of output: 576
🏁 Script executed:
# Check the cue-engine tests for scenarios with multiple triggers per pipeline
rg -B 5 -A 25 'pipeline.*Deploy|pipeline.*same|multiple.*pipeline' src/__tests__/main/cue/cue-engine.test.ts | head -150
# Check if there are tests verifying that different event configs create different groups
rg -B 3 -A 15 'schedule_times.*different|event.*config|triggerGroupKey' src/__tests__/main/cue/cue-engine.test.ts | head -100Repository: RunMaestro/Maestro
Length of output: 1366
🏁 Script executed:
# Get the full test for the multi-trigger pipeline scenario
rg -A 70 'does NOT fire unrelated triggers that share pipeline_name but differ in event config' src/__tests__/main/cue/cue-engine.test.tsRepository: RunMaestro/Maestro
Length of output: 1861
🏁 Script executed:
# Quick check to see if there's any way to import triggerGroupKey from shared
rg 'export.*triggerGroupKey|from.*triggerGroupKey' --type=ts
# Check the SessionsTable test file one more time to confirm multi-trigger scenario is not tested
rg 'morning|evening|different.*schedule|multiple.*config' src/__tests__/renderer/components/CueModal/SessionsTable.test.tsxRepository: RunMaestro/Maestro
Length of output: 44
Dedup strategy is incomplete: multiple event configs under same pipeline_name will be silently skipped.
The backend test "does NOT fire unrelated triggers that share pipeline_name but differ in event config" proves this scenario is real and intentional: a single pipeline can own multiple independent trigger groups (e.g., morning schedule at 07:00 and evening at 19:00 both under "Pipeline 1"). The current dedup keyed only on pipeline_name will skip the evening trigger after the morning group fires, because the frontend doesn't distinguish groups by event-specific config like schedule_times.
Backend's anchor-group logic groups by both pipeline_name AND triggerGroupKey (which includes schedule_times, watch, repo, poll_minutes, gh_state, label, and filter). The renderer must do the same or risk silently dropping valid triggers.
triggerGroupKey is a private function in both src/main/cue/cue-engine.ts and src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts and currently cannot be imported. This needs an architectural decision: either extract it to src/shared/cue and export it, or replicate the logic in SessionsTable. SessionsTable tests also lack coverage for the multi-trigger case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/CueModal/SessionsTable.tsx` around lines 154 - 167,
The current dedup in SessionsTable (seenPipelineNames + loop over subs calling
onTriggerSubscription) only keys by pipeline_name and will drop distinct trigger
groups that share a pipeline; change the dedup key to include the trigger group
identity (use the same fields as the engine's triggerGroupKey) so each unique
event config is allowed through. Either (A) extract and export the existing
triggerGroupKey helper into a shared module and call it from SessionsTable, or
(B) replicate the triggerGroupKey computation locally in SessionsTable to
produce a composite key (e.g., `${pipeline_name}|${triggerGroupKey(...)}`) and
use that in the Set instead of just pipeline_name; update the code that iterates
subs (the for loop and seenPipelineNames Set) to use this composite key and add
tests covering multiple triggers under the same pipeline_name.
…bility, test coverage - SessionsTable: use composite (pipeline_name + triggerGroupKey) dedup for Run Now so distinct trigger groups within the same pipeline both fire - usePipelinePersistence: preserve errorPipelineRoots in lastWrittenRootsRef to prevent orphaned YAML on subsequent saves; add zero-write guard to skip success toast/callback when nothing was written - CueSettingsPanel: make InfoTooltip keyboard-accessible (tabIndex, aria-describedby, role=tooltip, onFocus/onBlur, useId) - CueHelpModal: remove stale "Multi-Agent Orchestration" section that duplicated "Coordination Patterns" - Tests: add composite-key dedup test to SessionsTable.test.tsx; add disabled-sub exclusion test to cue-query-service.test.ts; remove two stale CueHelpModal tests; clean unused variable in usePipelinePersistence.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/components/CueHelpModal.tsx (1)
752-775:⚠️ Potential issue | 🟡 MinorDiagram contradicts prose—shows invalid
action:syntax.Line 760 correctly instructs users to set
action: command, but lines 771–773 incorrectly showaction: shellandaction: cli, which are not valid Cue action values. The schema defines only two valid actions ('prompt'and'command'), and shell/cli are modes of a command action, not action types themselves. The diagram should either be removed or corrected to show the proper nested structure (e.g.,action: commandwithcommand: {mode: shell, ...}).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueHelpModal.tsx` around lines 752 - 775, The diagram in the CueHelpModal component is showing invalid `action:` values (`shell`/`cli`) that contradict the prose; update the JSX diagram block (the div with className "font-mono text-xs p-2 rounded border mt-1" inside CueHelpModal) to show the correct nested structure by using `action: command` and then a nested `command: { mode: "shell", ... }` and `command: { mode: "cli", ... }` example (or remove the diagram entirely if you prefer), so the diagram matches the schema which only allows `'prompt'` and `'command'` as action values.src/renderer/hooks/cue/usePipelinePersistence.ts (1)
254-318:⚠️ Potential issue | 🟠 MajorMixed-root error pipelines are silently dropped from disk on save.
pipelinesByRootis built fromvalidPipelinesonly (line 198). When a root contains both valid and error pipelines, the write loop at lines 295–318 callspipelinesToYaml(rootPipelines, …)with only the valid pipelines. The generated YAML is then passed towriteYaml, which usesfs.writeFileSyncto overwrite the entire file (not merge). The error pipeline disappears from disk, contradicting the comment on lines 254–257 ("the pipeline still exists in the editor; it just can't be written until the user fixes the unresolved agent references") — this is true in-editor but false after the next reload.The
errorPipelineRootsprotection only covers the delete path (line 329). The write path needs a parallel safeguard — either skip rewriting roots that contain error pipelines, or carry original YAML entries for skipped pipelines throughpipelinesToYamlso they round-trip untouched.No existing test covers mixed-root saves (valid + error pipelines at the same root), which is why this data-loss path went undetected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/cue/usePipelinePersistence.ts` around lines 254 - 318, The save loop currently writes only pipelinesByRoot (built from validPipelines) and thus overwrites on-disk YAML for roots that also contain error pipelines, causing data loss; modify the save logic in usePipelinePersistence so that before calling pipelinesToYaml/writeYaml for each root it checks errorPipelineRoots (computed earlier) and if a root contains any error pipelines either (A) skip writing that root entirely (preserving on-disk YAML) or (B) merge the original on-disk YAML entries for the errored pipelines into the output of pipelinesToYaml so they round-trip unchanged; implement option A for simplicity by adding a conditional around the loop body that continues when errorPipelineRoots.has(root), and update related bookkeeping (totalPipelinesWritten/rootsCleared/lastWrittenRootsRef) and add a unit test covering a mixed-root case to prevent regressions.
🧹 Nitpick comments (2)
src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx (2)
87-92: Test description doesn't match assertions.The
itname mentions "status columns", but only the session name andtoolTypeare asserted. Either narrow the description or add a status-column assertion to match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx` around lines 87 - 92, The test named in the SessionsTable.spec uses makeSession and renders SessionsTable with makeProps but only asserts the session name and toolType ('MyAgent' and 'claude-code'); update the test to make the description and assertions consistent by either renaming the it(...) string to mention only "session name and agent type columns" or add an assertion that verifies the status column is rendered (e.g., assert the status text derived from the session returned by makeSession is present). Locate the test using the it(...) block, the helper makeSession('s1','MyAgent'), and the render(<SessionsTable ... />) call to implement the change.
178-213: Good coverage of composite-key dedup — consider one more case to lock in the contract.This nicely validates that differing
event(plusinterval_minutesvswatch) produces distincttriggerGroupKeys. To guard against future regressions where someone simplifiestriggerGroupKeyback toevent-only, consider adding a case where two subs share the samepipeline_nameandeventbut differ in only one secondary field (e.g. sametime.heartbeatevent with differentinterval_minutes, or twofile.changedsubs with differentwatchglobs). That would assert the full composite key — not just event-level differentiation — is honored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx` around lines 178 - 213, Add a new test in SessionsTable.test.tsx that verifies the triggerGroupKey uses the full composite key (not just event) by creating two subscriptions with the same pipeline_name and event but differing in a secondary field (e.g., two time.heartbeat subs with different interval_minutes or two file.changed subs with different watch globs) via makeGraphSession, render the SessionsTable with onTriggerSubscription mocked (onTrigger) and assert that firing "Run Now" calls onTrigger twice with the two subscription names (similar to the existing test that uses makeSession/makeGraphSession and asserts onTrigger called with 'heartbeat-sub' and 'file-sub').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx`:
- Around line 62-77: The test uses React in a type position
(React.ComponentProps<typeof SessionsTable>) but does not import React; add an
explicit import statement "import React from 'react';" at the top of this test
file to match the repo convention and ensure type-safety and future-proofing.
Locate the makeProps helper and the SessionsTable type usage to confirm the
addition covers React.ComponentProps<typeof SessionsTable> and other React types
in this file.
---
Outside diff comments:
In `@src/renderer/components/CueHelpModal.tsx`:
- Around line 752-775: The diagram in the CueHelpModal component is showing
invalid `action:` values (`shell`/`cli`) that contradict the prose; update the
JSX diagram block (the div with className "font-mono text-xs p-2 rounded border
mt-1" inside CueHelpModal) to show the correct nested structure by using
`action: command` and then a nested `command: { mode: "shell", ... }` and
`command: { mode: "cli", ... }` example (or remove the diagram entirely if you
prefer), so the diagram matches the schema which only allows `'prompt'` and
`'command'` as action values.
In `@src/renderer/hooks/cue/usePipelinePersistence.ts`:
- Around line 254-318: The save loop currently writes only pipelinesByRoot
(built from validPipelines) and thus overwrites on-disk YAML for roots that also
contain error pipelines, causing data loss; modify the save logic in
usePipelinePersistence so that before calling pipelinesToYaml/writeYaml for each
root it checks errorPipelineRoots (computed earlier) and if a root contains any
error pipelines either (A) skip writing that root entirely (preserving on-disk
YAML) or (B) merge the original on-disk YAML entries for the errored pipelines
into the output of pipelinesToYaml so they round-trip unchanged; implement
option A for simplicity by adding a conditional around the loop body that
continues when errorPipelineRoots.has(root), and update related bookkeeping
(totalPipelinesWritten/rootsCleared/lastWrittenRootsRef) and add a unit test
covering a mixed-root case to prevent regressions.
---
Nitpick comments:
In `@src/__tests__/renderer/components/CueModal/SessionsTable.test.tsx`:
- Around line 87-92: The test named in the SessionsTable.spec uses makeSession
and renders SessionsTable with makeProps but only asserts the session name and
toolType ('MyAgent' and 'claude-code'); update the test to make the description
and assertions consistent by either renaming the it(...) string to mention only
"session name and agent type columns" or add an assertion that verifies the
status column is rendered (e.g., assert the status text derived from the session
returned by makeSession is present). Locate the test using the it(...) block,
the helper makeSession('s1','MyAgent'), and the render(<SessionsTable ... />)
call to implement the change.
- Around line 178-213: Add a new test in SessionsTable.test.tsx that verifies
the triggerGroupKey uses the full composite key (not just event) by creating two
subscriptions with the same pipeline_name and event but differing in a secondary
field (e.g., two time.heartbeat subs with different interval_minutes or two
file.changed subs with different watch globs) via makeGraphSession, render the
SessionsTable with onTriggerSubscription mocked (onTrigger) and assert that
firing "Run Now" calls onTrigger twice with the two subscription names (similar
to the existing test that uses makeSession/makeGraphSession and asserts
onTrigger called with 'heartbeat-sub' and 'file-sub').
🪄 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: 0fa4b342-d7b5-49c9-aad2-360f2c6a9198
📒 Files selected for processing (8)
src/__tests__/main/cue/cue-query-service.test.tssrc/__tests__/renderer/components/CueHelpModal.test.tsxsrc/__tests__/renderer/components/CueModal/SessionsTable.test.tsxsrc/__tests__/renderer/hooks/cue/usePipelinePersistence.test.tssrc/renderer/components/CueHelpModal.tsxsrc/renderer/components/CueModal/SessionsTable.tsxsrc/renderer/components/CuePipelineEditor/panels/CueSettingsPanel.tsxsrc/renderer/hooks/cue/usePipelinePersistence.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/components/CuePipelineEditor/panels/CueSettingsPanel.tsx
- src/tests/renderer/components/CueHelpModal.test.tsx
- src/tests/renderer/hooks/cue/usePipelinePersistence.test.ts
…, tests - usePipelinePersistence: skip writing a root if it is also referenced by an error-node pipeline; writing the valid-only subset would silently strip the error pipeline's YAML from disk (data loss in mixed-root scenario) - Add usePipelinePersistence test: mixed root with valid + error pipeline must not call writeYaml for that root - CueHelpModal: fix Command Action diagram (action: shell/cli are invalid values; correct schema is action: command with command.mode: shell|cli) - SessionsTable.test.tsx: add React import for React.ComponentProps namespace usage; rename test to drop "status columns" from assertion-only description - SessionsTable.test.tsx: add triggerGroupKey secondary-field test — two time.heartbeat subs with identical pipeline_name but different interval_minutes must both fire (different composite keys)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/hooks/cue/usePipelinePersistence.ts (1)
360-370:⚠️ Potential issue | 🟠 Major
savedStateRefis updated for valid pipelines that weren't actually written (mixed-root regression).When a project root contains both a valid pipeline
Aand an error-node pipelineB, the write to that root is correctly skipped at lines 295–300, but line 364 still setssavedStateRef.current = JSON.stringify(validPipelines)— claimingAis persisted on disk when it isn't.Concrete regression: open a pre-existing workspace where
cue.yamlalready contains a validAplus an error-nowB(target deleted). State on load:savedStateRef = JSON.stringify([A, B]),isDirty=false. The user clicks Save without any edits:
validPipelines = [A],errorPipelineRoots = {/proj-ab}, write loop skips/proj-ab.- Line 364 overwrites
savedStateReftoJSON.stringify([A])— strictly worse than before the save.- The dirty-tracking effect compares
[A, B]vs[A]and flipsisDirtyto true.So just clicking Save with no edits silently turns a clean editor dirty, and the warning toast message "Other changes were saved." (lines 162–167) is also misleading —
Awasn't actually saved either.Two related concerns:
savedStateRefshould reflect the bytes on disk. After a partial save, that's: the pipelines we actually wrote + the error pipelines we left untouched (whose disk YAML is unchanged) — notvalidPipelines.- The static "Other changes were saved." tail is wrong in the all-error and shared-root-only cases. Consider deriving it from
totalPipelinesWritten/rootsCleared.🛠️ Sketch of a fix
+ const writtenPipelines: CuePipeline[] = []; for (const root of currentRoots) { if (errorPipelineRoots.has(root)) continue; const rootPipelines = pipelinesByRoot.get(root)!; // ...write + verify... totalPipelinesWritten += rootPipelines.length; + writtenPipelines.push(...rootPipelines); } // ...delete loop... - savedStateRef.current = JSON.stringify(validPipelines); + // Reflect actual disk state: pipelines we wrote + error pipelines whose + // YAML we deliberately left untouched on shared roots. Skipped valid + // pipelines on shared roots are NOT included — their edits are still + // pending until the user fixes the unresolved agents. + const stillOnDisk = [ + ...writtenPipelines, + ...pipelinesWithErrors.filter((p) => /* came from disk, not newly created */ true), + ]; + savedStateRef.current = JSON.stringify(stillOnDisk);And adjust the warning toast tail to be conditional on whether anything else actually got written/cleared.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/cue/usePipelinePersistence.ts` around lines 360 - 370, savedStateRef is being set to JSON.stringify(validPipelines) even when some valid pipelines were skipped due to shared-root errors, causing savedStateRef to no longer match on-disk bytes and re-mark the editor dirty; change the update so savedStateRef.current reflects the actual disk state after save by combining the pipelines that were successfully written plus any untouched error-node pipelines still present on disk (use the variables tracking written results such as totalPipelinesWritten/rootsCleared or the write-result set and errorPipelineRoots), update lastWrittenRootsRef.current as now but only include roots that were truly written or intentionally preserved, and make the toast message conditional on whether any pipelines/roots were actually written/cleared rather than always using the static "Other changes were saved." tail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/hooks/cue/usePipelinePersistence.ts`:
- Around line 360-370: savedStateRef is being set to
JSON.stringify(validPipelines) even when some valid pipelines were skipped due
to shared-root errors, causing savedStateRef to no longer match on-disk bytes
and re-mark the editor dirty; change the update so savedStateRef.current
reflects the actual disk state after save by combining the pipelines that were
successfully written plus any untouched error-node pipelines still present on
disk (use the variables tracking written results such as
totalPipelinesWritten/rootsCleared or the write-result set and
errorPipelineRoots), update lastWrittenRootsRef.current as now but only include
roots that were truly written or intentionally preserved, and make the toast
message conditional on whether any pipelines/roots were actually written/cleared
rather than always using the static "Other changes were saved." tail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6051b428-c2f1-46d7-b367-0163f7cc8816
📒 Files selected for processing (4)
src/__tests__/renderer/components/CueModal/SessionsTable.test.tsxsrc/__tests__/renderer/hooks/cue/usePipelinePersistence.test.tssrc/renderer/components/CueHelpModal.tsxsrc/renderer/hooks/cue/usePipelinePersistence.ts
1 participant
Summary
Cue dashboard ignores agents with valid cue.yaml until they're opened in the UI #866 — Dormant agents (cue.yaml on disk, not yet engine-initialized) were invisible in the Cue dashboard when the engine was running. Removed the
if (!deps.enabled())guard from bothgetStatus()andgetGraphData()incue-query-service.tsso dormant sessions are always surfaced withenabled: false, which naturally hides the Run Now button.Multiple agents sharing a project root run every Cue subscription N times — no way to designate an owner #867 — Multiple agents at the same
projectRooteach registered their own trigger source for unowned subscriptions, causing every heartbeat to fire N times. Resolved viacomputeOwnershipWarning(from RC feat(cue): resolve shared-workspace conflicts via owner_agent_id #873): at session init, the runtime service computes arunnableSubscriptionsview that strips unowned subs from non-owner sessions. Supports explicitsettings.owner_agent_idincue.yaml; falls back to first-in-list when unset. The dashboard surfaces a red ⚠ indicator with the reason when a session is suppressed.Cue dashboard Run Now only fires the first subscription when an agent has multiple #868 — Clicking Run Now on a session with N subscriptions sharing a
pipeline_namecaused N×N fires because the engine's anchor-group logic re-fires all siblings for each individual trigger call. Fixed inSessionsTable.tsxby deduplicating bypipeline_namebefore dispatching — one representative per group, ungrouped subs fire individually.Bug: Cue Scenario deletion not persisting after save #847 / fix(cue): seed lastWrittenRootsRef with per-pipeline write roots (#847) #849 — Pipeline deletion was silently lost when any other pipeline had unresolved agent references (error nodes). The save was aborted for the entire batch. Now error-node pipelines are filtered out and skipped with a warning toast; valid changes (including deletions) always persist. Additionally,
lastWrittenRootsRefis now seeded with per-pipeline write roots (common ancestor for cross-directory pipelines) so delete+save clears YAML at the correct path.Settings panel tooltips — Added inline
InfoTooltipcomponents toCueSettingsPanelexplaining each setting (timeout, concurrency, queue size, failure mode). First tooltip usesplacement=belowto avoid overlap with the canvas header. Also fixed a?? 1→?? 0inconsistency incue-run-manager.ts: thefinallyblock andstopRunpath were defaulting the decrement base to1while all read-side paths defaulted to0, causing the active run count to go negative under certain conditions.Help modal accuracy — Rewrote the Coordination Patterns section in
CueHelpModal.tsxto replace 7 loosely-defined mixed examples with 6 accurate patterns grounded in real YAML fields: Sequential Pipeline, Fan-Out, Fan-In (Gather), Swarm, Command Action, and Task Queue.Test plan
enabled: falsewhen engine is runningsettings.owner_agent_idin cue.yaml explicitly pins execution to the named agentowner_agent_idvalue shows ⚠ on all agents in the root with an explanatory tooltipSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests