Make durable scheduling cross-runtime#844
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughChangesScheduled work now supports cron and one-shot creation, state inspection, pause/resume controls, provider-aware delivery, tracked CLI sessions, desktop/TUI rendering, web and iOS remote APIs, and updated documentation. Scheduled work platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SkillSpector (2.3.11)apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.mdSkillSpector returned invalid JSON output 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 |
arul28
changed the title
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
July 17, 2026 03:35
…ckstop Every CronCreate is now mirrored into ADE's durable scheduler (durable gate removed); a 90s grace window lets the SDK's native scheduler fire first via claimNativeFire, with ADE's timer as the backstop for busy, skipped, or dead-process cases. Busy-session wakes queue for a turn boundary instead of steering the active turn. Restart reconciliation no longer cancels rows owned by a prior provider session when the fresh snapshot is empty. ADE state wins; the SDK's in-process CronList view is advisory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e durable wakes New action next to list/cancel/pause: creates provider-neutral durable chat schedules (recurring cron or one-shot), scoped to the caller's own session by default via daemon arg scoping; cross-session requests are denied for bound agents. Typed CLI: ade chat scheduled-work create. Remote commands chat.createScheduledWork and chat.setScheduledWorkPaused registered for mobile; desktop IPC/preload exposed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… parity Prompts: Claude blocks now state native cron tools are ADE-durable-backed (SDK view advisory); Codex/Cursor/Droid/OpenCode blocks replace 'no autonomous wake' with chat.createScheduledWork guidance; CLI guidance + ade-cli-control-plane skill document the scheduled-work surface. Docs aligned (ARCHITECTURE, chat features, public capabilities.mdx). iOS: scheduledWorkPaused/nextWakeAt summary fields (optional, legacy-safe), pause/resume via chat.setScheduledWorkPaused with optimistic rollback, paused row treatment, next-wake line. TUI: paused header suffix. Desktop: regression coverage for action-origin rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/wow-something-cool-jsut-happeend-7f820d16
branch
from
July 17, 2026 03:46
d70cfd0 to
b12cb82
Compare
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b12cb82dbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32149fad9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+15448
to
+15451
| const nativeCronScheduleId = taskType === "cron" | ||
| ? resolveClaudeCronScheduledWorkId(runtime, taskId, parentToolUseId, msg) | ||
| : null; | ||
| const explicitNativeCron = subtype === "task_started" && taskType === "cron"; |
There was a problem hiding this comment.
When Claude fires a native ScheduleWakeup/loop, the durable row created above is kind: "wakeup"/"loop", not a taskType === "cron" task, so this branch never passes its provider schedule id into startClaudeIdleTurn. The row therefore remains scheduled until ADE's 90s backstop fires, causing the same scheduled prompt to be delivered a second time after Claude already resumed the turn. This affects provider-owned one-shot wakeups created with ScheduleWakeup; recurring CronCreate jobs still get claimed through the cron path.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ade-cli/src/services/personalChats/personalChatScope.ts (1)
182-205: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winImplement missing
listScheduledWorkandgetScheduledWorkStateactions.The
PersonalChatScope.callswitch statement handlescreateScheduledWork,cancelScheduledWork, andsetScheduledWorkPaused, but it is missing cases forlistScheduledWorkandgetScheduledWorkState. Without these, CLI commands likeade chat scheduled-work list --personalandade chat schedules <session> --personalwill fall through and returnundefined.📦 Proposed fix to add the missing cases
case "cancelScheduledWork": { const sessionId = readSessionId(args); await this.requirePersonalSession(service, sessionId); result = await service.cancelScheduledWork({ sessionId, scheduleId: requiredString(args.scheduleId, "scheduleId"), }); break; } + case "listScheduledWork": { + const sessionId = typeof args.sessionId === "string" ? args.sessionId.trim() : null; + if (sessionId) await this.requirePersonalSession(service, sessionId); + const items = await service.listScheduledWork({ + ...(sessionId ? { sessionId } : {}), + ...(args.includeTerminal === true ? { includeTerminal: true } : {}), + } as never); + // If unfiltered by session, restrict the results to personal chats only + if (!sessionId) { + const summaries = await service.listSessions(undefined, { includeAutomation: true }); + const personalSessionIds = new Set(summaries.filter((s) => s.surface === "personal").map((s) => s.id)); + result = items.filter((item) => personalSessionIds.has(item.sessionId)); + } else { + result = items; + } + break; + } + case "getScheduledWorkState": { + const sessionId = readSessionId(args); + await this.requirePersonalSession(service, sessionId); + result = await service.getScheduledWorkState({ sessionId } as never); + break; + } case "setScheduledWorkPaused": {🤖 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 `@apps/ade-cli/src/services/personalChats/personalChatScope.ts` around lines 182 - 205, Update the PersonalChatScope.call switch to add listScheduledWork and getScheduledWorkState cases alongside the existing scheduled-work actions. For each case, read and validate the sessionId, call requirePersonalSession, invoke the corresponding service method with the sessionId and required arguments, and assign its result so both CLI commands return their service responses instead of falling through.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/usage/usageStatsStore.ts (1)
141-148: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider extracting the
aliasesdictionary outside the function.Declaring the static
aliasesdictionary outside of the function scope prevents it from being reallocated on every invocation, slightly improving performance.♻️ Proposed refactor
Add the dictionary outside the function:
const AGENT_CHAT_ALIASES: Record<string, string> = { "scheduledWork.create": "createScheduledWork", "scheduledWork.cancel": "cancelScheduledWork", };Then update the function body to reference it:
if (action.startsWith("agentChat.")) { const chatAction = action.slice("agentChat.".length); - const aliases: Record<string, string> = { - "scheduledWork.create": "createScheduledWork", - "scheduledWork.cancel": "cancelScheduledWork", - }; - return `chat.${aliases[chatAction] ?? chatAction}`; + return `chat.${AGENT_CHAT_ALIASES[chatAction] ?? chatAction}`; }🤖 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 `@apps/desktop/src/main/services/usage/usageStatsStore.ts` around lines 141 - 148, Extract the static aliases dictionary from the agent chat action-mapping function into a module-level AGENT_CHAT_ALIASES constant, then update the function to reference it while preserving the existing alias fallback behavior.
🤖 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 `@apps/desktop/src/main/services/adeActions/registry.ts`:
- Around line 1508-1515: Update
apps/desktop/src/main/services/adeActions/registry.ts lines 1508-1515 by adding
a createScheduledWork wrapper that reads the action arguments and validates
sessionId with requireNonEmptyString before calling
agentChatService.createScheduledWork. Also update the createScheduledWork input
definition at lines 887-901 to require sessionId as a string and include a valid
sessionId in its example payload.
In `@apps/desktop/src/main/services/chat/agentChatService.test.ts`:
- Around line 28454-28460: Update the queued-event assertion in the test to
first require that the matching queued user_message event exists, then assert
its scheduledWake metadata is undefined. Remove optional chaining from the
metadata assertion so a missing event causes the test to fail.
- Around line 28547-28557: After the existing assertion in the test, dispose the
active Codex service created for the second turn using the test’s established
service cleanup mechanism. Ensure cleanup runs after the assertion so the
spawned turn cannot leak state into subsequent tests.
In `@apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts`:
- Around line 340-343: Protect the shouldDefer invocation in the scheduled-work
timer flow so that if options.shouldDefer throws, armBusyDeferRetry(schedule) is
still called before the error is propagated or handled. Preserve the existing
retry-and-return behavior when shouldDefer returns true, ensuring the durable
scheduled row always has another timer after either defer outcome or an
exception.
- Line 431: Update the overdue check in the restored-scheduler re-arming flow
around overdueWhenArmed to apply the same one-second late-tolerance window used
by the existing scheduling path, so a restart at fireAt + 90.5 seconds remains
late: false. Add a restored-scheduler test covering START + 90_500 and asserting
late is false.
In `@apps/desktop/src/preload/preload.ts`:
- Around line 5540-5545: Update the chat.createScheduledWork call in
callProjectRuntimeActionOr to pass args directly instead of wrapping it as {
args }, matching the ipcRenderer.invoke fallback and preserving the expected
root-level payload fields.
In `@apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift`:
- Around line 812-848: Update setScheduledWorkPausedOptimistically so its error
rollback restores only the scheduledWorkPaused field changed by the optimistic
update, rather than replacing chatSummary and lastKnownChatSummary with
previousSummary. Preserve concurrent status, transcript, and other summary
updates received while the request was in flight, while retaining the existing
errorMessage behavior.
---
Outside diff comments:
In `@apps/ade-cli/src/services/personalChats/personalChatScope.ts`:
- Around line 182-205: Update the PersonalChatScope.call switch to add
listScheduledWork and getScheduledWorkState cases alongside the existing
scheduled-work actions. For each case, read and validate the sessionId, call
requirePersonalSession, invoke the corresponding service method with the
sessionId and required arguments, and assign its result so both CLI commands
return their service responses instead of falling through.
---
Nitpick comments:
In `@apps/desktop/src/main/services/usage/usageStatsStore.ts`:
- Around line 141-148: Extract the static aliases dictionary from the agent chat
action-mapping function into a module-level AGENT_CHAT_ALIASES constant, then
update the function to reference it while preserving the existing alias fallback
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 53f644ad-a022-4c62-b9f5-4ae7d1118699
⛔ Files ignored due to path filters (11)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/ade-code/README.mdis excluded by!docs/**docs/features/agents/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/transcript-and-turns.mdis excluded by!docs/**docs/features/personal-chats/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/sync-and-multi-device/remote-commands.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/pty-and-processes.mdis excluded by!docs/**docs/features/web-client/README.mdis excluded by!docs/**
📒 Files selected for processing (53)
apps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/services/personalChats/personalChatScope.test.tsapps/ade-cli/src/services/personalChats/personalChatScope.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.test.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/tuiClient/__tests__/Drawer.test.tsxapps/ade-cli/src/tuiClient/__tests__/RightPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/chatInfo.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/chatInfo.tsapps/ade-cli/src/tuiClient/closedCliSessions.tsapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/ade-cli/src/tuiClient/types.tsapps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.mdapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/ai/tools/systemPrompt.test.tsapps/desktop/src/main/services/ai/tools/systemPrompt.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/chatScheduledWorkScheduler.test.tsapps/desktop/src/main/services/chat/chatScheduledWorkScheduler.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/usage/usageStatsStore.tsapps/desktop/src/main/services/usage/usageTrackingService.test.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.test.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.test.tsxapps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsxapps/desktop/src/renderer/components/settings/AiFeaturesSection.tsxapps/desktop/src/renderer/webclient/adapter/__tests__/adapter.test.tsapps/desktop/src/renderer/webclient/adapter/agentChat.tsapps/desktop/src/shared/adeCliGuidance.test.tsapps/desktop/src/shared/adeCliGuidance.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/chat.tsapps/desktop/src/shared/types/personalChats.tsapps/desktop/src/shared/types/sessions.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/Work/WorkChatRichCardViews.swiftapps/ios/ADE/Views/Work/WorkSessionDestinationView.swiftapps/ios/ADETests/ADETests.swiftchat/capabilities.mdx
Comment on lines
+1508
to
+1515
| if (typeof base.getScheduledWorkState === "function") { | ||
| service.getScheduledWorkState = (args?: unknown) => { | ||
| const record = readObjectActionArg(args, "chat.getScheduledWorkState"); | ||
| return agentChatService.getScheduledWorkState({ | ||
| sessionId: requireNonEmptyString(record.sessionId, "sessionId"), | ||
| }); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Implement missing validation and correct the input contract for createScheduledWork.
The chat.createScheduledWork CLI action is currently missing its validation wrapper, and its input schema incorrectly advertises sessionId as optional. When a user runs the example command from the CLI, the raw arguments bypass validation, passing sessionId: undefined to the service, which will crash or corrupt state because AgentChatCreateScheduledWorkArgs requires a valid session ID.
apps/desktop/src/main/services/adeActions/registry.ts#L1508-L1515: add a wrapper forcreateScheduledWorkto safely parse and enforce the required arguments (e.g.,requireNonEmptyString) before callingagentChatService.createScheduledWork.apps/desktop/src/main/services/adeActions/registry.ts#L887-L901: update thecreateScheduledWorkinput definition to makesessionId: stringrequired, and add a validsessionIdto the payload in theexamplestring.
🐛 Proposed fixes
For the input contract:
createScheduledWork: {
description: "Create a durable recurring or one-shot wakeup for an eligible chat or tracked provider CLI session.",
- input: "object { sessionId?: string, cron: string, prompt: string, recurring?: boolean, reason?: string }",
- example: "ade actions run chat.createScheduledWork --input-json '{\"cron\":\"9,29,49 * * * *\",\"prompt\":\"Check CI and report\"}' --text",
+ input: "object { sessionId: string, cron: string, prompt: string, recurring?: boolean, reason?: string }",
+ example: "ade actions run chat.createScheduledWork --input-json '{\"sessionId\":\"chat-123\",\"cron\":\"9,29,49 * * * *\",\"prompt\":\"Check CI and report\"}' --text",
},For the missing wrapper:
+ if (typeof base.createScheduledWork === "function") {
+ service.createScheduledWork = (args?: unknown) => {
+ const record = readObjectActionArg(args, "chat.createScheduledWork");
+ return agentChatService.createScheduledWork({
+ sessionId: requireNonEmptyString(record.sessionId, "sessionId"),
+ cron: requireNonEmptyString(record.cron, "cron"),
+ prompt: requireNonEmptyString(record.prompt, "prompt"),
+ ...(typeof record.recurring === "boolean" ? { recurring: record.recurring } : {}),
+ ...(typeof record.reason === "string" ? { reason: record.reason } : {}),
+ });
+ };
+ }
if (typeof base.getScheduledWorkState === "function") {
service.getScheduledWorkState = (args?: unknown) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof base.getScheduledWorkState === "function") { | |
| service.getScheduledWorkState = (args?: unknown) => { | |
| const record = readObjectActionArg(args, "chat.getScheduledWorkState"); | |
| return agentChatService.getScheduledWorkState({ | |
| sessionId: requireNonEmptyString(record.sessionId, "sessionId"), | |
| }); | |
| }; | |
| } | |
| if (typeof base.createScheduledWork === "function") { | |
| service.createScheduledWork = (args?: unknown) => { | |
| const record = readObjectActionArg(args, "chat.createScheduledWork"); | |
| return agentChatService.createScheduledWork({ | |
| sessionId: requireNonEmptyString(record.sessionId, "sessionId"), | |
| cron: requireNonEmptyString(record.cron, "cron"), | |
| prompt: requireNonEmptyString(record.prompt, "prompt"), | |
| ...(typeof record.recurring === "boolean" ? { recurring: record.recurring } : {}), | |
| ...(typeof record.reason === "string" ? { reason: record.reason } : {}), | |
| }); | |
| }; | |
| } | |
| if (typeof base.getScheduledWorkState === "function") { | |
| service.getScheduledWorkState = (args?: unknown) => { | |
| const record = readObjectActionArg(args, "chat.getScheduledWorkState"); | |
| return agentChatService.getScheduledWorkState({ | |
| sessionId: requireNonEmptyString(record.sessionId, "sessionId"), | |
| }); | |
| }; | |
| } |
📍 Affects 1 file
apps/desktop/src/main/services/adeActions/registry.ts#L1508-L1515(this comment)apps/desktop/src/main/services/adeActions/registry.ts#L887-L901
🤖 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 `@apps/desktop/src/main/services/adeActions/registry.ts` around lines 1508 -
1515, Update apps/desktop/src/main/services/adeActions/registry.ts lines
1508-1515 by adding a createScheduledWork wrapper that reads the action
arguments and validates sessionId with requireNonEmptyString before calling
agentChatService.createScheduledWork. Also update the createScheduledWork input
definition at lines 887-901 to require sessionId as a string and include a valid
sessionId in its example payload.
Comment on lines
28454
to
28460
| const queued = events.find((event): event is AgentChatEventEnvelope & { | ||
| event: Extract<AgentChatEventEnvelope["event"], { type: "user_message" }>; | ||
| } => | ||
| event.event.type === "user_message" | ||
| && event.event.deliveryState === "queued" | ||
| && event.event.text === "Check PR CI"); | ||
| expect(queued?.event.metadata?.scheduledWake).toBeUndefined(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the queued event exists.
Optional chaining makes this assertion pass even if no matching user_message was emitted.
Proposed fix
);
+ expect(queued).toBeDefined();
- expect(queued?.event.metadata?.scheduledWake).toBeUndefined();
+ expect(queued!.event.metadata?.scheduledWake).toBeUndefined();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const queued = events.find((event): event is AgentChatEventEnvelope & { | |
| event: Extract<AgentChatEventEnvelope["event"], { type: "user_message" }>; | |
| } => | |
| event.event.type === "user_message" | |
| && event.event.deliveryState === "queued" | |
| && event.event.text === "Check PR CI"); | |
| expect(queued?.event.metadata?.scheduledWake).toBeUndefined(); | |
| const queued = events.find((event): event is AgentChatEventEnvelope & { | |
| event: Extract<AgentChatEventEnvelope["event"], { type: "user_message" }>; | |
| } => | |
| event.event.type === "user_message" | |
| && event.event.deliveryState === "queued" | |
| && event.event.text === "Check PR CI"); | |
| expect(queued).toBeDefined(); | |
| expect(queued!.event.metadata?.scheduledWake).toBeUndefined(); |
🤖 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 `@apps/desktop/src/main/services/chat/agentChatService.test.ts` around lines
28454 - 28460, Update the queued-event assertion in the test to first require
that the matching queued user_message event exists, then assert its
scheduledWake metadata is undefined. Remove optional chaining from the metadata
assertion so a missing event causes the test to fail.
Comment on lines
+28547
to
+28557
| expect(events).toEqual(expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| event: expect.objectContaining({ | ||
| type: "user_message", | ||
| metadata: expect.objectContaining({ | ||
| scheduledWake: expect.objectContaining({ scheduleId: "codex-wake-boundary-1" }), | ||
| }), | ||
| }), | ||
| }), | ||
| ])); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Dispose the active Codex service after the assertion.
Completing the first turn starts a second turn, but the test exits without completing or disposing it, risking leaked state across tests.
Proposed fix
expect(events).toEqual(expect.arrayContaining([
// ...
]));
+ service.forceDisposeAll();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(events).toEqual(expect.arrayContaining([ | |
| expect.objectContaining({ | |
| event: expect.objectContaining({ | |
| type: "user_message", | |
| metadata: expect.objectContaining({ | |
| scheduledWake: expect.objectContaining({ scheduleId: "codex-wake-boundary-1" }), | |
| }), | |
| }), | |
| }), | |
| ])); | |
| }); | |
| expect(events).toEqual(expect.arrayContaining([ | |
| expect.objectContaining({ | |
| event: expect.objectContaining({ | |
| type: "user_message", | |
| metadata: expect.objectContaining({ | |
| scheduledWake: expect.objectContaining({ scheduleId: "codex-wake-boundary-1" }), | |
| }), | |
| }), | |
| }), | |
| ])); | |
| service.forceDisposeAll(); | |
| }); |
🤖 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 `@apps/desktop/src/main/services/chat/agentChatService.test.ts` around lines
28547 - 28557, After the existing assertion in the test, dispose the active
Codex service created for the second turn using the test’s established service
cleanup mechanism. Ensure cleanup runs after the assertion so the spawned turn
cannot leak state into subsequent tests.
Comment on lines
+340
to
+343
| if (options.shouldDefer?.(cloneSchedule(schedule)) === true) { | ||
| armBusyDeferRetry(schedule); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Re-arm the durable row if shouldDefer throws.
Line 340 invokes the injected readiness hook after the timer handle has been removed. If it throws, the outer timer callback swallows the rejection and the schedule remains scheduled without another timer, potentially losing delivery until restart.
Proposed fix
- if (options.shouldDefer?.(cloneSchedule(schedule)) === true) {
+ let shouldDefer = false;
+ try {
+ shouldDefer = options.shouldDefer?.(cloneSchedule(schedule)) === true;
+ } catch {
+ armBusyDeferRetry(schedule);
+ return;
+ }
+ if (shouldDefer) {
armBusyDeferRetry(schedule);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (options.shouldDefer?.(cloneSchedule(schedule)) === true) { | |
| armBusyDeferRetry(schedule); | |
| return; | |
| } | |
| let shouldDefer = false; | |
| try { | |
| shouldDefer = options.shouldDefer?.(cloneSchedule(schedule)) === true; | |
| } catch { | |
| armBusyDeferRetry(schedule); | |
| return; | |
| } | |
| if (shouldDefer) { | |
| armBusyDeferRetry(schedule); | |
| return; | |
| } |
🤖 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 `@apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts` around
lines 340 - 343, Protect the shouldDefer invocation in the scheduled-work timer
flow so that if options.shouldDefer throws, armBusyDeferRetry(schedule) is still
called before the error is propagated or handled. Preserve the existing
retry-and-return behavior when shouldDefer returns true, ensuring the durable
scheduled row always has another timer after either defer outcome or an
exception.
|
|
||
| const currentTime = now(); | ||
| const overdueWhenArmed = schedule.fireAt != null && schedule.fireAt < currentTime; | ||
| const overdueWhenArmed = providerAdjustedFireAt != null && providerAdjustedFireAt < currentTime; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor the late-tolerance window when re-arming overdue work.
Line 431 marks any timer past the 90-second grace deadline as late, so overdueWhenArmed overrides the extra one-second tolerance used at Line 347. A restart at fireAt + 90.5s is therefore incorrectly reported as late.
Proposed fix
- const overdueWhenArmed = providerAdjustedFireAt != null && providerAdjustedFireAt < currentTime;
+ const overdueWhenArmed = providerAdjustedFireAt != null
+ && providerAdjustedFireAt + TIMER_LATE_TOLERANCE_MS < currentTime;Add a restored-scheduler test at START + 90_500 expecting late: false.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const overdueWhenArmed = providerAdjustedFireAt != null && providerAdjustedFireAt < currentTime; | |
| const overdueWhenArmed = providerAdjustedFireAt != null | |
| && providerAdjustedFireAt + TIMER_LATE_TOLERANCE_MS < currentTime; |
🤖 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 `@apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts` at line
431, Update the overdue check in the restored-scheduler re-arming flow around
overdueWhenArmed to apply the same one-second late-tolerance window used by the
existing scheduling path, so a restart at fireAt + 90.5 seconds remains late:
false. Add a restored-scheduler test covering START + 90_500 and asserting late
is false.
Comment on lines
+5540
to
+5545
| const result = await callProjectRuntimeActionOr( | ||
| "chat", | ||
| "createScheduledWork", | ||
| { args }, | ||
| () => ipcRenderer.invoke(IPC.agentChatCreateScheduledWork, args), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Pass args directly to avoid nesting the payload.
Wrapping args in an object ({ args }) sends { args: { sessionId, cron, ... } } to the runtime action. Since the chat.createScheduledWork service method expects these fields at the root of the payload, this will cause the remote command to fail validation (cron is required, etc.). Pass args directly, exactly as done in the ipcRenderer.invoke fallback.
🐛 Proposed fix
const result = await callProjectRuntimeActionOr(
"chat",
"createScheduledWork",
- { args },
+ args,
() => ipcRenderer.invoke(IPC.agentChatCreateScheduledWork, args),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = await callProjectRuntimeActionOr( | |
| "chat", | |
| "createScheduledWork", | |
| { args }, | |
| () => ipcRenderer.invoke(IPC.agentChatCreateScheduledWork, args), | |
| ); | |
| const result = await callProjectRuntimeActionOr( | |
| "chat", | |
| "createScheduledWork", | |
| args, | |
| () => ipcRenderer.invoke(IPC.agentChatCreateScheduledWork, args), | |
| ); |
🤖 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 `@apps/desktop/src/preload/preload.ts` around lines 5540 - 5545, Update the
chat.createScheduledWork call in callProjectRuntimeActionOr to pass args
directly instead of wrapping it as { args }, matching the ipcRenderer.invoke
fallback and preserving the expected root-level payload fields.
Comment on lines
+812
to
+848
| private var scheduledWorkPauseAction: (@MainActor (Bool) async -> Void)? { | ||
| guard syncService.canInvokeChatRemoteAction("chat.setScheduledWorkPaused", sessionId: sessionId) else { | ||
| return nil | ||
| } | ||
| return { paused in | ||
| await setScheduledWorkPausedOptimistically(paused) | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| private func setScheduledWorkPausedOptimistically(_ paused: Bool) async { | ||
| let previousSummary = composerChatSummary | ||
| if var optimisticSummary = previousSummary { | ||
| optimisticSummary.scheduledWorkPaused = paused | ||
| chatSummary = optimisticSummary | ||
| lastKnownChatSummary = optimisticSummary | ||
| } | ||
|
|
||
| do { | ||
| let result = try await syncService.setScheduledWorkPaused(sessionId: sessionId, paused: paused) | ||
| if var confirmedSummary = composerChatSummary { | ||
| confirmedSummary.scheduledWorkPaused = result.paused | ||
| confirmedSummary.nextWakeAt = result.nextWakeAt | ||
| chatSummary = confirmedSummary | ||
| lastKnownChatSummary = confirmedSummary | ||
| } | ||
| await refreshChatSummaryFromHost() | ||
| refreshScheduledWorkSnapshots() | ||
| } catch { | ||
| if let previousSummary { | ||
| chatSummary = previousSummary | ||
| lastKnownChatSummary = previousSummary | ||
| } | ||
| errorMessage = error.localizedDescription | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Patch specific fields during optimistic rollback to prevent clobbering concurrent updates.
Reverting the entire chatSummary object to previousSummary on error drops any concurrent changes (e.g., status flips or new transcript metadata) that arrived during the request flight. Patching only the fields mutated by the optimistic update preserves data integrity until the next poll.
♻️ Proposed fix
`@MainActor`
private func setScheduledWorkPausedOptimistically(_ paused: Bool) async {
let previousSummary = composerChatSummary
if var optimisticSummary = previousSummary {
optimisticSummary.scheduledWorkPaused = paused
chatSummary = optimisticSummary
lastKnownChatSummary = optimisticSummary
}
do {
let result = try await syncService.setScheduledWorkPaused(sessionId: sessionId, paused: paused)
if var confirmedSummary = composerChatSummary {
confirmedSummary.scheduledWorkPaused = result.paused
confirmedSummary.nextWakeAt = result.nextWakeAt
chatSummary = confirmedSummary
lastKnownChatSummary = confirmedSummary
}
await refreshChatSummaryFromHost()
refreshScheduledWorkSnapshots()
} catch {
- if let previousSummary {
- chatSummary = previousSummary
- lastKnownChatSummary = previousSummary
+ if var currentSummary = composerChatSummary {
+ currentSummary.scheduledWorkPaused = previousSummary?.scheduledWorkPaused
+ currentSummary.nextWakeAt = previousSummary?.nextWakeAt
+ chatSummary = currentSummary
+ lastKnownChatSummary = currentSummary
}
errorMessage = error.localizedDescription
}
}🤖 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 `@apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift` around lines 812 -
848, Update setScheduledWorkPausedOptimistically so its error rollback restores
only the scheduledWorkPaused field changed by the optimistic update, rather than
replacing chatSummary and lastKnownChatSummary with previousSummary. Preserve
concurrent status, transcript, and other summary updates received while the
request was in flight, while retaining the existing errorMessage behavior.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+3832
to
+3839
| ...(recurring !== undefined ? { recurring } : {}), | ||
| ...(reason ? { reason } : {}), | ||
| }); | ||
| }); | ||
| register("chat.listScheduledWork", { viewerAllowed: true, queueable: false }, async (payload) => { | ||
| const sessionId = asTrimmedString(payload.sessionId); | ||
| return requireService(args.agentChatService, "Agent chat service not available.").listScheduledWork({ | ||
| ...(sessionId ? { sessionId } : {}), |
There was a problem hiding this comment.
chat.getScheduledWorkState is added to the ADE action allowlist and CLI/TUI callers use it to inspect pause state and next wake, but the sync remote command surface only registers create/list/cancel/pause here. A paired web/mobile client invoking the documented inspect action gets an unsupported-command response instead of schedule state, so cross-runtime inspection is broken for that surface.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Line: 3832-3839
Comment:
**Register state inspection**
`chat.getScheduledWorkState` is added to the ADE action allowlist and CLI/TUI callers use it to inspect pause state and next wake, but the sync remote command surface only registers create/list/cancel/pause here. A paired web/mobile client invoking the documented inspect action gets an unsupported-command response instead of schedule state, so cross-runtime inspection is broken for that surface.
How can I resolve this? If you propose a fix, please make it concise.
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Greptile Summary
This PR adds cross-runtime durable scheduled work for ADE chats and tracked provider CLI sessions. The main changes are:
Confidence Score: 4/5
Mostly safe to merge after fixing the remote-command registry gap.
The main scheduler and service paths are consistent, but one documented inspection action is unavailable over sync remote commands.
apps/ade-cli/src/services/sync/syncRemoteCommandService.tsWhat T-Rex did
Important Files Changed
chat.getScheduledWorkStatefrom the sync command registry.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Runtime as ADE-bound runtime / CLI / Web / Mobile participant Actions as ADE chat actions participant AgentChat as AgentChatService participant Scheduler as ChatScheduledWorkScheduler participant Chat as SDK chat session participant PTY as Tracked provider CLI PTY Runtime->>Actions: create/list/getState/cancel/pause scheduled work Actions->>AgentChat: scoped scheduled-work request AgentChat->>Scheduler: upsert/list/nextWakeAt/setSessionPaused/cancel Scheduler-->>AgentChat: durable schedule state Scheduler->>Scheduler: wait until fireAt and safe boundary alt Chat-backed session Scheduler->>AgentChat: fire(schedule) AgentChat->>Chat: "messageSession(kind=wake, prompt)" else Tracked provider CLI Scheduler->>PTY: sendToSession(prompt) PTY-->>Scheduler: delivered or retry before delivery end AgentChat-->>Runtime: schedule item/state/result%%{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 Runtime as ADE-bound runtime / CLI / Web / Mobile participant Actions as ADE chat actions participant AgentChat as AgentChatService participant Scheduler as ChatScheduledWorkScheduler participant Chat as SDK chat session participant PTY as Tracked provider CLI PTY Runtime->>Actions: create/list/getState/cancel/pause scheduled work Actions->>AgentChat: scoped scheduled-work request AgentChat->>Scheduler: upsert/list/nextWakeAt/setSessionPaused/cancel Scheduler-->>AgentChat: durable schedule state Scheduler->>Scheduler: wait until fireAt and safe boundary alt Chat-backed session Scheduler->>AgentChat: fire(schedule) AgentChat->>Chat: "messageSession(kind=wake, prompt)" else Tracked provider CLI Scheduler->>PTY: sendToSession(prompt) PTY-->>Scheduler: delivered or retry before delivery end AgentChat-->>Runtime: schedule item/state/resultComments Outside Diff (2)
General comment
src/adeRpcServer.test.ts:routes spawn_agent to lane-scoped tracked pty sessions,launches default Codex spawn_agent sessions with supported sandbox flags,starts Codex spawn_agent with current default permission flags, andstarts spawn_agent without writing an attached ADE server config. The assertions expectedptyService.createcalls to includecommandand/orargs, but the observed call only includesstartupCommand,env,laneId, dimensions, title, toolType, and tracked metadata. One direct assertion showscreateCall.argswasundefinedinstead of containing Codex sandbox flags.startupCommandand no longer supplies the separatecommand/argsfields that the RPC tests expect.commandandargson theptyService.createpayload while also keepingstartupCommand, or update the implementation/tests together if the intentional API contract is nowstartupCommand-only and all PTY consumers can safely handle that contract.General comment
src/main/services/chat/agentChatService.test.ts > explicit provider-thread continuity recovery > decompresses a gzipped transcript when building the recovery capsule. The assertion expectedresult.capsulePreviewto containCompressed original task, but the generated preview did not include it, meaning the recovery capsule did not surface the task from the compressed history fixture.agentChatServiceand ensure compressed history bytes are decompressed and incorporated into the capsule preview before fallback/empty preview handling is used.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(pty): avoid peer-owned scheduled res..." | Re-trigger Greptile