Skip to content

fix(cue): scan watched file on manual task.pending trigger - #1107

Open
scriptease wants to merge 2 commits into
RunMaestro:rcfrom
scriptease:cue-task-queue-manual-trigger
Open

fix(cue): scan watched file on manual task.pending trigger#1107
scriptease wants to merge 2 commits into
RunMaestro:rcfrom
scriptease:cue-task-queue-manual-trigger

Conversation

@scriptease

@scriptease scriptease commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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.

Summary by CodeRabbit

  • New Features
    • Manual task triggering now performs a one-time scan of the watched project files for pending tasks and enriches the event payload with task metadata (count, list, path) when available.
    • When no pending tasks are found, the event payload is still dispatched with the manual indicator, omitting task metadata.
  • Bug Fixes
    • If the scan fails, manual triggers still dispatch the bare manual payload and log a warning instead of breaking the workflow.

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>
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b0d35477-6b7b-4883-af71-53d18e5b15a6

📥 Commits

Reviewing files that changed from the base of the PR and between de8d00e and bd4c2cc.

📒 Files selected for processing (2)
  • src/__tests__/main/cue/cue-engine.test.ts
  • src/main/cue/cue-engine.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/cue/cue-engine.ts

📝 Walkthrough

Walkthrough

cue-task-scanner.ts gains two new exports: buildTaskPendingPayload (extracts pending markdown tasks from file content into a payload object or null) and scanTaskFilesOnce (single-pass glob walk returning payloads for files with pending tasks). The existing doScan is refactored to use buildTaskPendingPayload. CueEngine.triggerSubscription now calls scanTaskFilesOnce when manually dispatching a task.pending subscription that has a watch, merging scanned data into the event payload. Test coverage includes successful scan results, empty scans, and error handling with logged warnings.

Changes

Manual task.pending trigger with scanned payload

Layer / File(s) Summary
Task scanner helpers and doScan refactor
src/main/cue/cue-task-scanner.ts
Adds buildTaskPendingPayload to produce a standardized payload or null from file content, adds scanTaskFilesOnce to walk matching files and return payloads for files with pending tasks, and refactors doScan to use buildTaskPendingPayload instead of inline extraction.
Engine manual trigger integration
src/main/cue/cue-engine.ts
Imports scanTaskFilesOnce and updates triggerSubscription to resolve projectRoot, run a one-shot scan for task.pending subscriptions with a watch, log scan outcomes and match counts, handle scan failures via warning logs and exception capture, and merge the scanned task payload with { manual: true } into the Cue event.
Test setup and manual trigger test cases
src/__tests__/main/cue/cue-engine.test.ts
Adds mockScanTaskFilesOnce to the module mock and provides test cases asserting correct payload shape when scan returns pending tasks, when scan returns empty results, and when scan throws an exception with logged warnings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

ready to merge

Poem

🐇 A rabbit hops through markdown files,
Scanning tasks across the miles,
pending tasks found in one pass,
Manual triggers now amass!
No more empty payloads — hooray,
Scanned data joins the cue today! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: scanning watched files during manual task.pending subscription triggers, which is the core fix addressing the documented bug.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between cb36a25 and de8d00e.

📒 Files selected for processing (3)
  • src/__tests__/main/cue/cue-engine.test.ts
  • src/main/cue/cue-engine.ts
  • src/main/cue/cue-task-scanner.ts

Comment thread src/main/cue/cue-engine.ts
@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes the broken manual cue trigger flow for task.pending subscriptions by scanning the watched file(s) synchronously at dispatch time and merging the first matching file's task fields into the event payload, so {{CUE_TASK_COUNT}} and {{CUE_TASK_LIST}} resolve correctly without a polling cycle.

  • Refactoring (cue-task-scanner.ts): Extracts buildTaskPendingPayload from the inline code in createCueTaskScanner and adds scanTaskFilesOnce for a hash-free, one-shot walk — both are now shared by the polling and manual paths, keeping payloads identical.
  • Engine change (cue-engine.ts): triggerSubscription calls scanTaskFilesOnce before building the event for any task.pending sub that has a watch glob, using the first matching file's payload and logging a warning when more than one file matches.
  • Tests (cue-engine.test.ts): Two new cases verify payload population on a successful scan and clean dispatch when no pending tasks are found.

Confidence Score: 3/5

The 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 scanTaskFilesOnce is not guarded with a try/catch at the call site in triggerSubscription. An inaccessible projectRoot causes walkDir to re-throw, escaping the dispatch loop instead of returning false cleanly. The polling scanner handles the same failure by catching and logging. Additionally, a broad glob silently drops all but the first matching file's tasks, diverging from the polling scanner's one-event-per-file behavior.

src/main/cue/cue-engine.ts — the scanTaskFilesOnce call inside triggerSubscription needs error handling; src/main/cue/cue-task-scanner.ts is otherwise clean.

Important Files Changed

Filename Overview
src/main/cue/cue-task-scanner.ts Extracts buildTaskPendingPayload for reuse and adds scanTaskFilesOnce for live, hash-free scanning. Refactoring is clean and the polling scanner behavior is unchanged. scanTaskFilesOnce lacks a top-level try/catch, which can cause exceptions to escape callers that don't guard against it.
src/main/cue/cue-engine.ts Integrates scanTaskFilesOnce into triggerSubscription for task.pending subs. The call is not guarded with try/catch — an inaccessible projectRoot throws out of the dispatch loop, and only the first matching file's tasks are dispatched even when multiple files match the glob.
src/tests/main/cue/cue-engine.test.ts Adds scanTaskFilesOnce mock and two new tests covering the happy path (payload populated) and empty-scan (no task fields). Tests are clear and align with the new behavior. No error-path test for a failing scan.

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
Loading
%%{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
Loading

Reviews (1): Last reviewed commit: "fix(cue): scan watched file on manual ta..." | Re-trigger Greptile

Comment thread src/main/cue/cue-engine.ts Outdated
Comment thread src/main/cue/cue-engine.ts Outdated
@pedramamini

Copy link
Copy Markdown
Collaborator

Thanks for the fix, @scriptease! Great catch on the broken manual cue trigger fallback for task.pending, and extracting buildTaskPendingPayload so the polling and manual paths produce identical payloads is a clean way to keep them in sync. The two new tests (populated payload + empty-scan) cover the behavior well.

One change before this is good to merge:

Guard the one-shot scan in triggerSubscription (src/main/cue/cue-engine.ts)

scanTaskFilesOnce calls walkDir(projectRoot, projectRoot), and walkDir deliberately re-throws when the root itself is unreadable (if (dir === root) throw err). The new call inside the dispatch loop isn't wrapped, and triggerSubscription has no surrounding try/catch, so if a session's projectRoot is deleted, unmounted, or hits a permission change, the exception escapes the whole method: every remaining sub in the dispatch group is skipped and the trigger throws instead of returning cleanly.

The polling scanner already handles this exact case gracefully - doScan wraps its walkDir call in try/catch and logs - so the manual path should degrade the same way. captureException is already imported in this file (and used a few lines down), so the fix matches existing usage:

try {
	const payloads = scanTaskFilesOnce(sub.watch, projectRoot);
	// ...existing payload handling...
} catch (err) {
	this.deps.onLog(
		'warn',
		`[CUE] "${sub.name}" manual trigger scan failed: ${err instanceof Error ? err.message : String(err)}`
	);
	void captureException(err, {
		operation: 'cue.triggerSubscription.scanTaskFilesOnce',
		subscriptionName: sub.name,
		ownerSessionId,
	});
}
// taskPayload stays {} so dispatch continues with { manual: true }

This also lines up with our error-handling guideline of handling expected/recoverable errors explicitly rather than letting them abort the flow. Both Greptile and CodeRabbit flagged the same thing, and I confirmed it against the code.

Minor, your call

For a broad glob like tasks/**/*.md that matches several files, the manual trigger dispatches a single event from payloads[0], whereas the poller fires one event per file. You already log a warning when more than one file matches, which is a reasonable choice for a one-shot trigger, but I'm flagging the asymmetry in case you'd rather fan out to match the poller. Not a blocker.

Once the scan is guarded I'm happy to approve. Thanks again for the contribution!

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>
@pedramamini

Copy link
Copy Markdown
Collaborator

@scriptease this got a positive review but has since gone into conflict with rc. Could you rebase onto the latest rc and re-push? Would like to merge the manual task.pending scan fix right after.

@jSydorowicz21

Copy link
Copy Markdown
Contributor

Current rc already contains a later implementation of this behavior in e559458e2 through #1151/#1152, and this branch now conflicts in the engine, scanner, and test files. I think this PR should close as superseded rather than be rebased.

One follow-up is still needed on current rc: Windows directory walking returns paths such as tasks\\alpha.md, but Cue watches are slash-delimited, for example tasks/**/*.md. Passing the backslash path directly to picomatch returns no payloads, so manual triggering falls back to CUE_TASK_COUNT=0. I reproduced that with real temporary files. Normalize the relative path to POSIX separators before matching in both manual and polling scans, and retain the original path for filesystem access.

@pedramamini

Copy link
Copy Markdown
Collaborator

@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 rc and it already contains e559458e2 ("fix(cue): scan watched files on manual task.pending trigger", via #1151/#1152), which landed the same fix this PR makes: buildTaskPendingPayload extracted in cue-task-scanner.ts and consumed by triggerSubscription in cue-engine.ts, with the one-shot scan guarded the way I asked for in my earlier review. This branch now conflicts in all three files it touches, and a rebase would essentially be re-deriving what is already on rc.

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 buildTaskPendingPayload extraction is the same shape as what shipped. Thank you for finding and fixing the broken manual fallback; it is a genuinely useful catch and it made it into the product.

If you would like to keep going on this area, there is a real open bug on current rc that @jSydorowicz21 spotted and I verified against the code:

walkDir builds relative paths with path.relative(root, fullPath) (src/main/cue/cue-task-scanner.ts:108), which returns backslash-separated paths on Windows (tasks\alpha.md). Cue watch globs are slash-delimited (tasks/**/*.md), so picomatch never matches and the manual trigger falls back to CUE_TASK_COUNT=0 - the exact symptom this PR set out to fix, just on Windows. Both call sites are affected: scanTaskFilesNow (line 138) and doScan (line 187).

The fix is to normalize to POSIX separators for matching while keeping the original path for filesystem access, e.g. match on relPath.split(path.sep).join('/') and continue to use path.resolve(projectRoot, relPath) for reads. Worth a test that exercises a nested path, since our CI runs a windows-latest leg that would catch a regression here.

Entirely your call whether you want to pick that up - happy to review it if you do. Thanks again for this one.

@pedramamini

Copy link
Copy Markdown
Collaborator

@scriptease friendly follow-up on the note above, no action needed beyond a quick decision.

I re-checked current rc today and the picture is unchanged: e559458e2 is on rc and ships the same fix this branch makes (buildTaskPendingPayload plus a guarded one-shot scan, now named scanTaskFilesNow, wired into triggerSubscription), and this branch is still CONFLICTING against rc in all three files it touches. So closing this as superseded is still the right call rather than rebasing into a re-derivation.

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 rc if you want to pick it up as a fresh branch: walkDir returns backslash-separated relative paths on Windows (src/main/cue/cue-task-scanner.ts:108), and scanTaskFilesNow passes that straight to picomatch against a slash-delimited watch glob, so nothing matches and the manual trigger still falls back to CUE_TASK_COUNT=0 there. Normalizing to POSIX separators for the match while keeping the original path for reads would close it out, ideally with a nested-path test since CI runs a windows-latest leg.

Happy either way. If you would rather not spend more time on it, just close this out and we will call it done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants