fix(cue): scan watched file on manual task.pending trigger - #1107
fix(cue): scan watched file on manual task.pending trigger#1107scriptease wants to merge 2 commits into
Conversation
A manual `maestro-cli cue trigger` of a task.pending subscription built a
bare event with no task payload, so the prompt's {{CUE_TASK_COUNT}} and
{{CUE_TASK_LIST}} fell back to "0"/"" and the run saw no work. Only the
polling scanner populated those fields, so the documented manual fallback
was broken.
Extract `buildTaskPendingPayload` and add `scanTaskFilesOnce` in the task
scanner (hash-free live scan), and have `CueEngine.triggerSubscription`
scan the sub's watch glob against the owner's projectRoot before dispatch,
merging the first matching file's task fields into the event payload.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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 (1)
📝 WalkthroughWalkthrough
ChangesManual task.pending trigger with scanned payload
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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)
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 |
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/main/cue/cue-engine.ts`:
- Around line 1161-1181: The scanTaskFilesOnce() call in the task.pending event
handler can throw on filesystem access failures without a guard, causing
triggerSubscription to abort before dispatching a run. Wrap the
scanTaskFilesOnce(sub.watch, projectRoot) call in a try-catch block within the
projectRoot check. In the catch block, log the error context using
this.deps.onLog(), call captureException() from src/utils/sentry.ts to report
the error, and continue with a default { manual: true } payload instead of
letting the exception propagate. This ensures manual triggers are resilient to
filesystem errors.
🪄 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: 867d2b16-31e7-4796-b1c9-a5564ee7e747
📒 Files selected for processing (3)
src/__tests__/main/cue/cue-engine.test.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-task-scanner.ts
Greptile SummaryThis PR fixes the broken manual
Confidence Score: 3/5The manual trigger can throw out of the dispatch loop when the project root is inaccessible, instead of gracefully degrading; review this before merging. The core fix is correct and well-tested, but src/main/cue/cue-engine.ts — the Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI as maestro-cli cue trigger
participant CE as CueEngine.triggerSubscription
participant STO as scanTaskFilesOnce
participant FS as File System
participant DS as dispatchService
CLI->>CE: triggerSubscription("task-queue")
CE->>CE: "resolve anchor sub (event=task.pending, watch=tasks/**/*.md)"
CE->>STO: scanTaskFilesOnce(watch, projectRoot)
STO->>FS: walkDir(projectRoot)
FS-->>STO: [relPaths...]
STO->>FS: readFileSync(absPath) per match
FS-->>STO: file content
STO->>STO: buildTaskPendingPayload(absPath, relPath, content)
STO-->>CE: "[{path, taskCount, taskList, tasks, ...}]"
CE->>CE: "merge payloads[0] into event payload {manual:true, ...taskPayload}"
CE->>DS: dispatchSubscription(ownerSessionId, sub, event, "manual")
DS-->>CE: dispatched count
CE-->>CLI: true
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CLI as maestro-cli cue trigger
participant CE as CueEngine.triggerSubscription
participant STO as scanTaskFilesOnce
participant FS as File System
participant DS as dispatchService
CLI->>CE: triggerSubscription("task-queue")
CE->>CE: "resolve anchor sub (event=task.pending, watch=tasks/**/*.md)"
CE->>STO: scanTaskFilesOnce(watch, projectRoot)
STO->>FS: walkDir(projectRoot)
FS-->>STO: [relPaths...]
STO->>FS: readFileSync(absPath) per match
FS-->>STO: file content
STO->>STO: buildTaskPendingPayload(absPath, relPath, content)
STO-->>CE: "[{path, taskCount, taskList, tasks, ...}]"
CE->>CE: "merge payloads[0] into event payload {manual:true, ...taskPayload}"
CE->>DS: dispatchSubscription(ownerSessionId, sub, event, "manual")
DS-->>CE: dispatched count
CE-->>CLI: true
Reviews (1): Last reviewed commit: "fix(cue): scan watched file on manual ta..." | Re-trigger Greptile |
|
Thanks for the fix, @scriptease! Great catch on the broken manual One change before this is good to merge: Guard the one-shot scan in
|
scanTaskFilesOnce -> walkDir re-throws when the root itself is unreadable
(deleted/unmounted/permission change). The call sat unguarded in the
triggerSubscription dispatch loop, so such a failure escaped the whole
method: remaining subs in the group were skipped and the trigger threw
instead of returning cleanly.
Mirror doScan's graceful degradation: catch, log a warning, report via
captureException, and dispatch with the bare { manual: true } payload.
Adds an error-path test asserting dispatch still happens on scan failure.
Addresses review feedback on RunMaestro#1107.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@scriptease this got a positive review but has since gone into conflict with |
|
Current One follow-up is still needed on current |
|
@scriptease apologies - my rebase request above is now the wrong ask, and I want to correct it before you spend time on it. I checked current So I think the right move is to close this as superseded rather than rebase. That is not a reflection on the work - your diagnosis was correct and your If you would like to keep going on this area, there is a real open bug on current
The fix is to normalize to POSIX separators for matching while keeping the original path for filesystem access, e.g. match on Entirely your call whether you want to pick that up - happy to review it if you do. Thanks again for this one. |
|
@scriptease friendly follow-up on the note above, no action needed beyond a quick decision. I re-checked current To say it plainly one more time: the diagnosis and the extraction here were correct, and the behavior you fixed is in the product. Thank you for that. The Windows follow-up is also still open on Happy either way. If you would rather not spend more time on it, just close this out and we will call it done. |
A manual
maestro-cli cue triggerof a task.pending subscription built a bare event with no task payload, so the prompt's {{CUE_TASK_COUNT}} and {{CUE_TASK_LIST}} fell back to "0"/"" and the run saw no work. Only the polling scanner populated those fields, so the documented manual fallback was broken.Extract
buildTaskPendingPayloadand addscanTaskFilesOncein the task scanner (hash-free live scan), and haveCueEngine.triggerSubscriptionscan the sub's watch glob against the owner's projectRoot before dispatch, merging the first matching file's task fields into the event payload.Summary by CodeRabbit