feat(automation): decouple schedules from project selection - #1274
Conversation
Astro-Han
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/context/global-sync.tsx, packages/app/src/pages/automations/automation-detail-editors.tsx, packages/app/src/pages/automations/automation-detail.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the cross-project automation move logic to update the owner project and directory in place, preserving the automation's ID and run history. It also updates the scheduler to manage tasks across multiple project scopes globally and ensures active runs are not stopped during instance disposal. A review comment suggests wrapping the Project.get call in a try-catch block to prevent an unhandled NotFoundError from bypassing validation when a target project is not found.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Warning Review limit reached
More reviews will be available in 50 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds per-directory Scope to automations, makes the scheduler process-owned and scope-aware (GlobalBus routing), implements native fresh automation moves via scoped updates, updates server startup/shutdown and instance disposal, and wires frontend folder-picker/open-project callbacks with matching E2E and server tests. ChangesProcess-level scheduling with multi-directory scopes
Sequence Diagram(s)sequenceDiagram
participant UI as AutomationDetail / FolderPicker
participant GlobalSync as globalSync.automation.update
participant Server as Server PUT /:automationID
participant Automation as Automation.update
participant Scheduler as AutomationScheduler
participant Instance as Instance (runtime / DB)
UI->>GlobalSync: update where.projectID + targetDirectory
GlobalSync->>Server: PUT /automation/:id (updated definition)
Server->>Automation: update(definition, scope?)
Automation->>Instance: write scoped definition and source tombstone
Automation->>Scheduler: publishDefinitionUpdatedForScope / publishDefinitionDeleted
Scheduler->>Instance: runInScope -> schedule timers / execute runs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/src/automation/scheduler.ts (1)
645-648: 💤 Low valueClarify or remove the unused
directoryparameter.
stopDirectoryOwnedRunsaccepts adirectoryparameter but ignores it (void directory). This appears intentional per the PR design where the process scheduler owns all runs regardless of directory, but the function signature is misleading. Consider either:
- Removing the parameter if callers don't need directory-scoped stopping
- Adding a brief comment explaining why it's intentionally ignored
🤖 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 `@packages/opencode/src/automation/scheduler.ts` around lines 645 - 648, The stopDirectoryOwnedRuns function currently accepts a directory parameter but ignores it (void directory), which is misleading; either remove the unused parameter from the function signature and update all callers to call stopDirectoryOwnedRuns() (leave logic as current().stopOwnedRuns()), or keep the parameter and add a concise comment above stopDirectoryOwnedRuns explaining that the scheduler owns all runs and the directory is intentionally unused so callers can remain compatible; update any type declarations accordingly and ensure references to stopDirectoryOwnedRuns and current().stopOwnedRuns() remain consistent.
🤖 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 `@packages/opencode/src/automation/index.ts`:
- Around line 699-701: The move-checking logic currently only blocks moves when
hasActiveRun(previous.id) and previous.context === "continue", but later code
drops reconciliation if the definition is out of scope which can miss applying
recordRunOutcome (and its failureStreak/nextFireAt) for runs that just finished;
update the move validation to, before adding "project_not_found" or dropping
reconciliation via addDetail(updateDetails,...), attempt a fallback lookup of
the definition in its current scope (e.g., resolve the definition by
previous.definitionId or caller scope) and only mark "project_not_found" or drop
reconciliation if that fallback fails; ensure this touches the same conditional
paths where previous.context, hasActiveRun(previous.id), targetProject, and
addDetail(updateDetails, ...) are used so recordRunOutcome will still be applied
when a run has just transitioned to succeeded/failed.
In `@packages/opencode/src/server/server.ts`:
- Around line 84-86: The server currently starts before the process scheduler
finishes settling; after calling built.runtime.listen(opts) you must await the
scheduler ownership settlement so startup fails if reconciliation fails—retrieve
the scheduler via AutomationScheduler.current(), call and await
automationScheduler.settleOwner(), and only then proceed (so any errors from
settleOwner are propagated and server readiness reflects the scheduler
contract).
---
Nitpick comments:
In `@packages/opencode/src/automation/scheduler.ts`:
- Around line 645-648: The stopDirectoryOwnedRuns function currently accepts a
directory parameter but ignores it (void directory), which is misleading; either
remove the unused parameter from the function signature and update all callers
to call stopDirectoryOwnedRuns() (leave logic as current().stopOwnedRuns()), or
keep the parameter and add a concise comment above stopDirectoryOwnedRuns
explaining that the scheduler owns all runs and the directory is intentionally
unused so callers can remain compatible; update any type declarations
accordingly and ensure references to stopDirectoryOwnedRuns and
current().stopOwnedRuns() remain consistent.
🪄 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 Plus
Run ID: 9f41b8ca-8d67-40f0-98e4-7f8f0538875f
📒 Files selected for processing (11)
packages/app/e2e/automations/automations-panel.spec.tspackages/app/src/context/global-sync.tsxpackages/app/src/pages/automations/automation-detail-editors.tsxpackages/app/src/pages/automations/automation-detail.tsxpackages/opencode/src/automation/index.tspackages/opencode/src/automation/scheduler.tspackages/opencode/src/project/instance.tspackages/opencode/src/server/instance/automation.tspackages/opencode/src/server/server.tspackages/opencode/test/server/automation-routes.test.tspackages/opencode/test/server/automation-scheduler.test.ts
💤 Files with no reviewable changes (1)
- packages/opencode/src/project/instance.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/app/e2e/automations/automations-panel.spec.ts (1)
630-630: ⚡ Quick winUse
.filter({ hasText })for consistency with existing tests.This file's existing move tests (lines 593-594, 693, 737) use the
.filter({ hasText })pattern. For consistency, refactor to match:- await surface.locator('[data-action="automation-row"]', { hasText: "Single project digest" }).first().click() + await surface.locator('[data-action="automation-row"]').filter({ hasText: "Single project digest" }).first().click()Both APIs are valid in Playwright 1.57.0, but maintaining a consistent pattern within the file improves readability.
🤖 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 `@packages/app/e2e/automations/automations-panel.spec.ts` at line 630, Replace the direct locator call that uses the hasText option with the file's established pattern: use surface.locator('[data-action="automation-row"]').filter({ hasText: "Single project digest" }).first().click() so it matches other tests; update the call referencing surface.locator and the '[data-action="automation-row"]' selector to use .filter({ hasText }) for consistency.
🤖 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.
Nitpick comments:
In `@packages/app/e2e/automations/automations-panel.spec.ts`:
- Line 630: Replace the direct locator call that uses the hasText option with
the file's established pattern: use
surface.locator('[data-action="automation-row"]').filter({ hasText: "Single
project digest" }).first().click() so it matches other tests; update the call
referencing surface.locator and the '[data-action="automation-row"]' selector to
use .filter({ hasText }) for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 38c78c92-5070-4eb9-ba14-4be90816fbfd
📒 Files selected for processing (9)
packages/app/e2e/automations/automations-panel.spec.tspackages/app/src/pages/automations/automation-create-dialog.tsxpackages/app/src/pages/automations/automation-detail-editors.tsxpackages/app/src/pages/automations/automation-detail.tsxpackages/app/src/pages/automations/automation-folder-picker.tsxpackages/app/src/pages/automations/automations-route.tsxpackages/app/src/pages/automations/automations-surface.tsxpackages/app/src/pages/layout.tsxpackages/app/src/pages/layout/surface-page-context.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app/src/pages/automations/automation-detail.tsx
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 `@packages/opencode/src/automation/scheduler.ts`:
- Around line 468-471: The preserved timer logic needs to update the
ScheduledTask.scope when a run is moved to a different scope: after calling
refreshScopedRunOutcome(...) and before/inside scheduleNextInterval(...), detect
if nextScope differs from the current scheduled task's scope and update the
preserved timer entries so they reflect nextScope (i.e. modify the preserved
ScheduledTask.scope stored by preservePendingSchedule()/preserveDueSchedule() or
add a helper to patch those entries). Ensure both code paths that short-circuit
to preservePendingSchedule and preserveDueSchedule update the
ScheduledTask.scope to nextScope so the retained timer fires in the correct
{projectID, ownerDirectory} instead of the old source scope.
🪄 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 Plus
Run ID: 6a821cbc-c11c-43f2-9bb4-ab2862714eab
📒 Files selected for processing (3)
packages/opencode/src/automation/scheduler.tspackages/opencode/src/server/server.tspackages/opencode/test/server/server-listen.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/server/server.ts
Fix the Windows advisory flake in the prompt cancellation race tests.\n\nRoot cause:\n- The tests used a 1 second guard while waiting for cancellation-race checkpoints.\n- Windows CI can legitimately take longer than that to reach the processor-handle checkpoint.\n- The product contract is not that the checkpoint happens within one second; it is that cancellation after the checkpoint still finalizes correctly.\n\nChanges:\n- Introduce a shared 5 second checkpoint timeout for the two adjacent prompt cancellation race tests.\n- Keep the change test-only; no production behavior changes.\n\nVerification:\n- bun test --timeout 30000 test/session/prompt-effect.test.ts -t "cancel after assistant scaffold save finalizes before processor handle|cancel after processor handle creation finalizes before process starts"\n- bun test --timeout 30000 test/session/prompt-effect.test.ts\n- bun run typecheck\n- git diff --check\n- PR CI: all checks passed, including windows-advisory / unit-windows-opencode-session.\n\nFollow-up:\n- This unblocks using a clean dev base when returning to #1274.
1 participant
Summary
Decouple Automation scheduling from the currently selected project by introducing one process-owned scheduler that scans scoped automation definitions and enters project instances only when a run fires.
This also retires the temporary cross-project move workaround for fresh automations: moving now updates the existing automation in place, preserving its id, schedule, pause state, and run history.
The Automations detail Project row now reuses the existing workspace-picker experience even when only one project is open: fresh automations stay clickable, the menu includes the current project plus the existing Open project footer action, and opening another project from this surface does not navigate away from Automations. Continue automations remain read-only because they are bound to their source conversation.
Automation detail hover states are also unified: internal Project / Repeats / Model controls and the continue source-session jump use the same 30px row-hover surface with the default cursor, with underline reserved for external-link semantics.
Why
Closes #1271.
Scheduled automations were tied to project instance lifetime, so switching or closing a project could cancel scheduler-owned runs and closed projects had no scheduler watching their due times. The user promise should instead be: automations run while PawWork is open, regardless of which project is visible.
The Automations Project row also had a one-project trap: with only one project open it became plain text, so users could not reach the same Open project affordance that already exists in the composer workspace picker.
Related Issue
Closes #1271
Human Review Status
Pending
Review Focus
Please focus on the ownership boundary: the process scheduler should own due-fire decisions globally, while actual execution still enters the target project instance and keeps the existing writer / active-run guards intact.
Also check the native fresh move path: source stores should receive a tombstone, target stores should receive the moved definition, and continue automations should remain bound to their source conversation.
For the UI follow-up, check that the Automations Project row is just an extension of the existing workspace-picker path: fresh automations can open the picker with one project, Open project adds to the available projects without leaving Automations, and continue automations remain read-only. Internal detail-page controls should use the same 30px row-hover feedback and default cursor, not underline or web-link pointer treatment.
Risk Notes
stopDirectoryOwnedRuns,stopCurrentOwnedRuns,stopAllOwnedRuns) after grep found no callers; the remaining explicit entry point isstopProcess({ stopRuns }).How To Verify
Screenshots or Recordings
Visual check performed with
bun --cwd packages/app run snap automations-surface; reviewed the generated grid atdocs/design/preview/screenshots/automations-surface.pnglocally. The snap shows the Automations detail Project row as a picker, the detail hover targets using the same row surface, and the create card still using the same folder picker surface.Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
New Features
Bug Fixes
Tests