Add persistent worker execution plane - #314
Conversation
mira-2026
added
priority: high
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR introduces a SQLite-backed job execution queue with leased worker execution, cancellation and recovery, resource-scoped child processes, queued operational routes, split web/worker startup, queue-aware dashboard views, and corresponding tests, packaging, deployment units, and documentation. ChangesPersistent job execution plane
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
backend/src/routes/opsRoutes.ts (1)
107-113: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMapped queue error status/message is swallowed by the generic 500 catch.
successfulJobExecutionOutputthrows errors carrying meaningfulstatusCode(e.g. 409 cancelled, 504 timeout) and messages, but the outercatch(Line 119-122) always responds with a generic500 "Ops route failed", discarding both. This defeats the purpose of callingsuccessfulJobExecutionOutputhere and is inconsistent with the sibling queued routes (backupRoutes.ts,openclawConfigRoutes.ts) that preservehttpStatusCode(error).♻️ Suggested handling in the catch
} catch (error) { - console.error("[opsRoutes] Ops route failed", error); - return json({ error: "Ops route failed" }, { status: 500 }); + const status = httpStatusCode(error); + if (status === 500) console.error("[opsRoutes] Ops route failed", error); + return json( + { error: status === 500 ? "Ops route failed" : errorMessage(error, "Ops route failed") }, + { status } + ); }🤖 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 `@backend/src/routes/opsRoutes.ts` around lines 107 - 113, Update the outer catch around the log-rotation handling in the ops route to preserve errors thrown by successfulJobExecutionOutput: derive the response status through the existing httpStatusCode(error) helper and return the error’s message instead of always sending generic 500 "Ops route failed". Keep the existing generic fallback for errors without a mapped status or message, matching the sibling queued routes.backend/src/routes/jobExecutionRoutes.ts (1)
68-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInconsistent error handling vs. sibling routes.
GET /api/job-executions/:idhas no try/catch aroundgetJobExecution(id), unlike the list route (Lines 51-66) and the cancel route (Lines 86-113) which both wrap their queue calls. A DB-layer failure here would propagate unhandled instead of returning a controlled 500 like the others.♻️ Proposed fix
"/api/job-executions/:id": { GET: (request: ParametersRequest<"id">) => { const id = String(request.params.id); if (!isValidExecutionId(id)) { return json({ error: "Invalid job execution id" }, { status: 400 }); } - const execution = getJobExecution(id); - return execution - ? json({ execution: publicExecution(execution, { includeOutput: true }) }) - : json({ error: "Job execution not found" }, { status: 404 }); + try { + const execution = getJobExecution(id); + return execution + ? json({ execution: publicExecution(execution, { includeOutput: true }) }) + : json({ error: "Job execution not found" }, { status: 404 }); + } catch (error) { + console.error("[jobExecutionRoutes] Queue lookup failed", error); + return json({ error: "Job execution lookup failed" }, { status: 500 }); + } }, },🤖 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 `@backend/src/routes/jobExecutionRoutes.ts` around lines 68 - 79, Update the GET handler for /api/job-executions/:id to wrap the getJobExecution(id) call and response construction in try/catch, matching the error-handling pattern of the sibling list and cancel routes. Preserve the existing invalid-ID 400 and missing-execution 404 responses, and return the established controlled 500 response when the lookup fails.
🤖 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 `@backend/src/routes/jobExecutionRoutes.ts`:
- Around line 45-47: Update isValidExecutionId to accept the UUID-style values
produced by Bun.randomUUIDv7(), while preserving lowercase hexadecimal, hyphen
placement, and the expected UUID length. Ensure startJobExecution’s generated
default IDs pass validation before route handling.
In `@backend/src/serverStart.ts`:
- Around line 18-26: Wire server/process shutdown handling to invoke
serverStartState.stopWorkerOnServerClose when it is registered, including the
startup-error path in handleServerListening and the cleanup flow in
stopBackendServer; preserve the wrapper’s idempotent reset behavior and avoid
bypassing it with direct stopDashboardJobWorker calls where the handler applies.
In `@backend/src/services/backups.ts`:
- Around line 820-829: Update the fallback return mapping to derive status from
execution.status rather than using only execution.finishedAt. Preserve "running"
for non-terminal executions, and map failed or cancelled terminal statuses to
their corresponding persisted view statuses instead of reporting "done".
In `@backend/src/services/jobWorker.ts`:
- Around line 35-58: Track the in-flight rollback from startDashboardJobWorker
in workerState, and have stopDashboardJobWorker await that pending promise
before returning when startup has failed. Clear the tracked promise after
cleanup completes, while preserving the existing isStarted/isStopping state
transitions and rollback error reporting.
In `@backend/src/services/scheduledJobs.ts`:
- Around line 32-34: The single global executor slot allows long-lived
interactive jobs to starve all scheduled work. In
backend/src/services/scheduledJobs.ts lines 32-34, add per-resource-class
concurrency or reserve a dedicated interactive lane while preserving capacity
for scheduled jobs; route exec.tracked in backend/src/services/execJobs.ts lines
80-81 and docker.exec in backend/src/routes/dockerRoutes.ts lines 883-898
through that interactive lane or separate concurrency limit.
In `@backend/test/bunNativeServerBehavior.test.ts`:
- Around line 91-112: Update the SIGTERM handler in the generated server script
to handle rejected promises from server.stop(true) or
stopScheduledJobExecutor(). Add a rejection path that reports the error and
exits with failure, while preserving the successful shutdown path’s
process.exit(0) behavior.
In `@docs/architecture/database.md`:
- Around line 48-54: Add an `openclaw_cron_job_metadata` row to the architecture
database schema table near the other scheduled-job tables, using a concise
description consistent with the persisted cron metadata documented in the
scheduler cache backups reference.
In `@docs/setup/new-vps.md`:
- Around line 96-102: Update the service setup instructions before the existing
systemctl commands to enable lingering for the ubuntu user, then verify it with
loginctl show-user. Keep daemon-reload and both mira-dashboard service
enable/start commands after this prerequisite.
- Around line 82-89: Update the unit-install block in the setup guide to work
after the preceding directory change by using the correct repository-relative
paths, and create /home/ubuntu/.config/systemd/user before installing the units.
Keep both mira-dashboard.service and mira-dashboard-worker.service installations
intact.
In `@docs/setup/production-deploy.md`:
- Around line 124-132: Update the rollback procedure around the
mira-dashboard-worker.service commands to evaluate whether workerStart.js exists
before restarting the worker. Move the stop/disable and legacy combined web-unit
restoration branch ahead of any worker restart, and only run the worker restart
when the entrypoint is present; preserve the existing health-check flow
afterward.
- Around line 98-102: Update the production deployment smoke-test curl block by
removing the unauthenticated /api/job-executions request, or move it to the
authenticated verification section with a valid session cookie. Keep the health
and bootstrap checks unchanged, and ensure queue verification is performed only
through an authenticated request.
---
Nitpick comments:
In `@backend/src/routes/jobExecutionRoutes.ts`:
- Around line 68-79: Update the GET handler for /api/job-executions/:id to wrap
the getJobExecution(id) call and response construction in try/catch, matching
the error-handling pattern of the sibling list and cancel routes. Preserve the
existing invalid-ID 400 and missing-execution 404 responses, and return the
established controlled 500 response when the lookup fails.
In `@backend/src/routes/opsRoutes.ts`:
- Around line 107-113: Update the outer catch around the log-rotation handling
in the ops route to preserve errors thrown by successfulJobExecutionOutput:
derive the response status through the existing httpStatusCode(error) helper and
return the error’s message instead of always sending generic 500 "Ops route
failed". Keep the existing generic fallback for errors without a mapped status
or message, matching the sibling queued routes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d26ae47-fcaa-4115-8924-2282ebaef52a
⛔ Files ignored due to path filters (2)
backend/bun.lockis excluded by!**/*.lockand included by**/*bun.lockis excluded by!**/*.lockand included by**/*
📒 Files selected for processing (59)
backend/package.jsonbackend/scripts/build.tsbackend/src/database.tsbackend/src/lib/jobResources.tsbackend/src/lib/processes.tsbackend/src/requestPolicy.tsbackend/src/routes.tsbackend/src/routes/backupRoutes.tsbackend/src/routes/cacheRoutes.tsbackend/src/routes/dockerRoutes.tsbackend/src/routes/jobExecutionRoutes.tsbackend/src/routes/jobRoutes.tsbackend/src/routes/openclawConfigRoutes.tsbackend/src/routes/opsRoutes.tsbackend/src/routes/pullRequestRoutes.tsbackend/src/serverStart.tsbackend/src/serverStartPolicy.tsbackend/src/services/backups.tsbackend/src/services/cacheRefresh.tsbackend/src/services/dockerActions.tsbackend/src/services/dockerUpdater.tsbackend/src/services/execJobs.tsbackend/src/services/gitHygiene.tsbackend/src/services/jobExecutionQueue.tsbackend/src/services/jobWorker.tsbackend/src/services/logRotation.tsbackend/src/services/openclawActions.tsbackend/src/services/pullRequests.tsbackend/src/services/queuedJobExecution.tsbackend/src/services/scheduledJobs.tsbackend/src/workerStart.tsbackend/test/bunNativeServerBehavior.test.tsbackend/test/httpApiBehavior.test.tsbackend/test/jobExecutionQueue.test.tsbackend/test/routeAndServiceBehavior.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsbackend/test/utilityBehavior.test.tsdocs/api/endpoints.mddocs/architecture/database.mddocs/architecture/overview.mddocs/operations/scheduler-cache-backups.mddocs/setup/new-vps.mddocs/setup/production-deploy.mddocs/setup/secrets-and-env.mdpackage.jsonsrc/components/features/dashboard/JobsOverviewCard.tsxsrc/components/features/dashboard/LogRotationCard.tsxsrc/components/features/jobs/JobExecutionQueueCard.tsxsrc/hooks/index.tssrc/hooks/useCache.tssrc/hooks/useJobExecutions.tssrc/hooks/useScheduledJobs.tssrc/pages/Jobs.tsxsrc/test/componentBehavior.test.tsxsrc/test/pageBehavior.test.tsxsrc/test/setup.tssystemd/mira-dashboard-worker.servicesystemd/mira-dashboard.service
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: frontend-checks
🔇 Additional comments (70)
systemd/mira-dashboard-worker.service (1)
1-27: LGTM!systemd/mira-dashboard.service (1)
1-26: LGTM!docs/setup/new-vps.md (1)
91-94: LGTM!Also applies to: 139-139
docs/setup/production-deploy.md (1)
21-30: LGTM!Also applies to: 53-73, 75-78, 89-94, 104-110, 170-173
docs/setup/secrets-and-env.md (1)
46-59: LGTM!docs/architecture/overview.md (1)
14-25: LGTM!Also applies to: 78-79, 121-141
docs/operations/scheduler-cache-backups.md (1)
3-7: LGTM!Also applies to: 33-48, 71-96, 139-141, 200-202
docs/architecture/database.md (1)
32-47: LGTM!package.json (1)
29-45: LGTM!Also applies to: 58-64, 87-91
src/components/features/dashboard/JobsOverviewCard.tsx (1)
24-31: LGTM!Also applies to: 57-57, 156-163
src/components/features/dashboard/LogRotationCard.tsx (1)
182-194: LGTM!src/components/features/jobs/JobExecutionQueueCard.tsx (2)
1-20: LGTM!Also applies to: 26-172
21-23: 📐 Maintainability & Code QualityNo change needed.
The current
resourceClassvalues areinteractive,light,network,host-heavy, andexclusive; none contain more than one hyphen.src/hooks/useCache.ts (1)
111-122: 🎯 Functional Correctness | ⚡ Quick winSequential loop aborts remaining key refreshes on first failure.
Switching from
Promise.allto a sequentialfor...ofawait means a failure on an earlier key now prevents every subsequent key from even being requested, and discards the already-fetched results for earlier keys sinceonSuccessnever runs. This is a behavior change for bulk refreshes (e.g., the moltbook key set) that previously attempted every key regardless of individual failures.🐛 Proposed fix using best-effort settlement
- const results: Array<{ - isOk: boolean; - entry: CacheEnvelope<unknown>; - }> = []; - for (const key of keys) { - results.push( - await apiPostRequired<{ - isOk: boolean; - entry: CacheEnvelope<unknown>; - }>(`/cache/${encodeURIComponent(key)}/refresh`) - ); - } + const settled = await Promise.allSettled( + keys.map((key) => + apiPostRequired<{ + isOk: boolean; + entry: CacheEnvelope<unknown>; + }>(`/cache/${encodeURIComponent(key)}/refresh`) + ) + ); + const results = settled + .filter( + ( + outcome + ): outcome is PromiseFulfilledResult<{ + isOk: boolean; + entry: CacheEnvelope<unknown>; + }> => outcome.status === "fulfilled" + ) + .map((outcome) => outcome.value);backend/test/serviceBehavior.test.ts (2)
354-376: 🩺 Stability & Availability | ⚡ Quick winCollected cleanup errors are never surfaced.
afterEachaccumulates thrown errors from cleanup callbacks intoerrorsbut never logs or rethrows them, so a failingstopScheduledJobExecutor()(leaked interval, unregistered worker, etc.) fails silently instead of flagging the test run.🛠️ Proposed fix: surface cleanup failures
database .prepare( `DELETE FROM job_executions WHERE scheduled_job_id LIKE 'backup.%' OR action_key = 'backup.clear-attention'` ) .run(); + if (errors.length > 0) { + throw errors[0]; + } });
23-23: LGTM!Also applies to: 860-863, 1057-1119, 1274-1281, 2231-2261, 2570-2570, 2603-2603, 2624-2625, 2724-2724, 2768-2768, 2832-2832, 2844-2864, 2911-2911, 2962-2962, 2991-2995, 3031-3044, 3101-3101, 3129-3129, 3176-3176, 3773-3773, 3831-3831, 3897-3897, 4510-4510, 4526-4527, 4536-4541, 4550-4551, 4567-4593, 4608-4609, 4891-4894, 5265-5268, 5279-5284, 5450-5452, 5495-5512, 5658-5662, 5762-5771, 5790-5790, 5809-5811
backend/test/utilityBehavior.test.ts (1)
640-644: LGTM!Also applies to: 665-670
docs/api/endpoints.md (1)
105-119: LGTM!Also applies to: 129-146, 176-195, 196-208, 209-227, 254-261
src/test/setup.ts (1)
2-10: LGTM!src/test/componentBehavior.test.tsx (1)
81-81: LGTM!Also applies to: 3181-3268
src/hooks/index.ts (1)
44-54: LGTM!src/hooks/useJobExecutions.ts (1)
1-81: LGTM!src/hooks/useScheduledJobs.ts (1)
5-5: LGTM!Also applies to: 24-44, 63-92, 116-120, 132-139, 171-175
src/pages/Jobs.tsx (1)
6-6: LGTM!Also applies to: 147-157, 344-363, 463-469, 515-518, 536-538, 965-1010
backend/test/httpApiBehavior.test.ts (1)
206-220: LGTM!Also applies to: 1379-1390, 1520-1571, 1760-1767, 2318-2326
backend/test/jobExecutionQueue.test.ts (1)
1-212: LGTM!backend/test/routeAndServiceBehavior.test.ts (1)
28-28: LGTM!Also applies to: 187-245, 2472-2475, 3143-3149, 3342-3347, 3359-3376, 3400-3403, 3703-3703, 3774-3781, 3844-3845, 4780-4780
backend/test/serverStartupPolicy.test.ts (1)
23-32: LGTM!Also applies to: 72-82
src/test/pageBehavior.test.tsx (1)
1268-1290: LGTM!Also applies to: 1353-1355, 1383-1385
backend/package.json (1)
11-13: LGTM!Also applies to: 31-33
backend/scripts/build.ts (1)
11-14: LGTM!backend/src/lib/processes.ts (1)
58-63: LGTM!Also applies to: 96-133
backend/src/requestPolicy.ts (1)
212-218: LGTM!backend/src/routes.ts (1)
16-16: LGTM!Also applies to: 87-87
backend/src/services/backups.ts (1)
750-782: LGTM!Also applies to: 847-860
backend/src/services/dockerActions.ts (1)
129-277: LGTM!Also applies to: 279-337
backend/src/routes/backupRoutes.ts (1)
4-6: LGTM!Also applies to: 19-24, 35-36
backend/src/routes/openclawConfigRoutes.ts (1)
9-13: LGTM!Also applies to: 309-320
backend/src/routes/pullRequestRoutes.ts (2)
53-53: LGTM!Also applies to: 71-71, 82-82, 93-93
102-105: 🎯 Functional CorrectnessNo change needed for deploy checkout checks.
The production checkout/preflight gating is no longer in this route; it is handled by the direct deploy path before the queued deploy action starts (
ensureProductionCheckout/ensureProductionReadyForDeployat/api/pull-requests/deploymentsthen starts anotherensureProductionCheckoutbeforeensureProductionReadyForDeploy).backend/src/database.ts (1)
308-361: LGTM!backend/src/lib/jobResources.ts (1)
1-154: LGTM!backend/src/services/jobExecutionQueue.ts (2)
169-208: LGTM!
378-634: LGTM!backend/src/services/queuedJobExecution.ts (1)
18-112: LGTM!backend/src/services/scheduledJobs.ts (4)
889-963: LGTM!
965-1115: LGTM!
1226-1272: LGTM!
1145-1153: 🩺 Stability & AvailabilityNo change needed.
enqueueScheduledJobreturns a 409 for the active-execution unique constraint, andisStaleScheduledRunErrortreats any 409 as stale here, so this path is not logged as a recurring per-tick failure.backend/src/services/cacheRefresh.ts (3)
1889-1957: LGTM!
2063-2097: LGTM!
2346-2427: LGTM!backend/src/services/execJobs.ts (1)
341-525: LGTM!backend/src/services/pullRequests.ts (2)
1360-1494: LGTM!
1570-1779: LGTM!backend/src/routes/dockerRoutes.ts (2)
649-683: LGTM!
799-848: LGTM!backend/src/routes/jobRoutes.ts (1)
180-183: LGTM!backend/src/services/jobWorker.ts (1)
1-32: LGTM!backend/src/workerStart.ts (1)
1-32: LGTM!backend/src/serverStart.ts (2)
28-40: LGTM!
5-17: LGTM!Also applies to: 50-60, 63-106, 109-125, 127-137
backend/src/serverStartPolicy.ts (1)
1-17: LGTM!backend/src/services/dockerUpdater.ts (1)
2678-2721: LGTM!backend/src/services/gitHygiene.ts (1)
408-419: LGTM!backend/src/services/logRotation.ts (1)
73-103: LGTM!Also applies to: 1778-1846, 1970-2003
backend/src/services/openclawActions.ts (1)
1-40: LGTM!backend/src/routes/cacheRoutes.ts (2)
309-337: 🚀 Performance & ScalabilityVerify duplicate refresh requests don't queue redundant executions.
Each
POST /api/cache/:key/refreshnow enqueues a brand-newcache.refreshjob execution rather than joining an in-flight one for the same key. If multiple dashboard clients (or a client double-click) hit refresh for the same key concurrently, this could queue redundant executions competing for the same resource class slot instead of sharing one result. This may be intentional (explicit user action should always run), but worth confirmingenqueueJobExecution/waitForJobExecution(inqueuedJobExecution.ts, not in this batch) doesn't already dedupe, since that file isn't included here for verification.
11-16: LGTM!Also applies to: 188-209, 234-255
backend/src/routes/jobExecutionRoutes.ts (1)
1-44: LGTM!Also applies to: 49-67, 80-115
![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: 35bbaae9ba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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 `@backend/test/serviceBehavior.test.ts`:
- Around line 3280-3284: Update the assertion around
clearPersistedBackupAttention("walg") so it does not pin the volatile
backup.stderr value mutated by the status refresh; continue verifying the stable
backup fields and that getPersistedBackupJob("walg") returns undefined.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d4f0b74-c2ce-4f21-9c2d-4d1bce5dd234
📒 Files selected for processing (23)
backend/src/routes/dockerRoutes.tsbackend/src/routes/jobExecutionRoutes.tsbackend/src/routes/opsRoutes.tsbackend/src/serverStart.tsbackend/src/services/backups.tsbackend/src/services/dockerUpdater.tsbackend/src/services/gitHygiene.tsbackend/src/services/jobWorker.tsbackend/src/services/pullRequests.tsbackend/src/services/scheduledJobs.tsbackend/test/bunNativeServerBehavior.test.tsbackend/test/dockerUpdater.test.tsbackend/test/httpApiBehavior.test.tsbackend/test/jobExecutionQueue.test.tsbackend/test/routeAndServiceBehavior.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsdocs/architecture/database.mddocs/setup/new-vps.mddocs/setup/production-deploy.mdsrc/hooks/useBackups.tssrc/hooks/useCache.tssrc/test/frontendBehavior.test.tsx
💤 Files with no reviewable changes (1)
- backend/src/serverStart.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/architecture/database.md
- backend/src/routes/opsRoutes.ts
- docs/setup/new-vps.md
- backend/test/bunNativeServerBehavior.test.ts
- backend/test/jobExecutionQueue.test.ts
- docs/setup/production-deploy.md
- backend/test/httpApiBehavior.test.ts
- backend/src/routes/jobExecutionRoutes.ts
- backend/test/routeAndServiceBehavior.test.ts
- backend/src/services/backups.ts
- backend/src/routes/dockerRoutes.ts
- backend/src/services/scheduledJobs.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
backend/src/services/dockerUpdater.ts
[warning] 1647-1647: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(composePath, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🪛 GitHub Check: backend-checks
backend/test/serviceBehavior.test.ts
[failure] 3281-3281: error: expect(received).toMatchObject(expected)
{
"code": 130,
"endedAt": 1784685600000,
"id": "019f8d31-d786-7000-927e-d9f4b6eb6965",
"startedAt": 1784685600000,
"status": "needs_attention",
- "stderr": "Worker restarted before attention was cleared",
- "stderr":
- "Worker restarted before attention was cleared
- Status refresh failed: Error: docker exec walg wal-g backup-list --detail --json failed with exit code 1: Error response from daemon: No such container: walg"
- ,
"stdout": "",
"type": "walg",
}
- Expected - 1
-
Received + 4
at <anonymous> (/home/runner/work/Mira-Dashboard/Mira-Dashboard/backend/test/serviceBehavior.test.ts:3281:74)
🔇 Additional comments (11)
backend/test/dockerUpdater.test.ts (2)
343-402: LGTM!
493-501: 🎯 Functional CorrectnessNo duplicated
.all()statement here.The
const services = database...assignment uses a single.all()call, followed byconst firstService = services.find(...); this does not introduce a dangling.all()to strip.> Likely an incorrect or invalid review comment.backend/src/services/jobWorker.ts (1)
17-95: LGTM!backend/src/services/dockerUpdater.ts (1)
655-2797: LGTM!backend/src/services/gitHygiene.ts (1)
24-514: LGTM!backend/src/services/pullRequests.ts (1)
299-1803: LGTM!backend/test/serverStartupPolicy.test.ts (1)
83-185: LGTM!backend/test/serviceBehavior.test.ts (1)
993-1099: LGTM!Also applies to: 2670-2715, 3186-3229
src/hooks/useBackups.ts (1)
10-10: LGTM!src/hooks/useCache.ts (1)
111-147: LGTM!src/test/frontendBehavior.test.tsx (1)
106-106: LGTM!Also applies to: 2457-2515
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d617ef497f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
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 `@backend/src/workerStart.ts`:
- Around line 17-23: Update runDashboardWorker so the shutdown promise,
keep-alive handle, and SIGINT/SIGTERM handlers are established before invoking
startDashboardJobWorker. Move worker startup into the guarded execution/cleanup
flow so startup failures and early termination both reach the existing
finally-based shutdown sequence.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56967e9f-b42b-493e-87fe-f68c0f8a1e45
📒 Files selected for processing (8)
backend/src/serverStart.tsbackend/src/services/jobExecutionQueue.tsbackend/src/services/pullRequests.tsbackend/src/services/queuedJobExecution.tsbackend/src/workerStart.tsbackend/test/jobExecutionQueue.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/test/serverStartupPolicy.test.ts
- backend/src/services/queuedJobExecution.ts
- backend/test/jobExecutionQueue.test.ts
- backend/src/serverStart.ts
- backend/src/services/jobExecutionQueue.ts
- backend/src/services/pullRequests.ts
- backend/test/serviceBehavior.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: frontend-checks
🔇 Additional comments (3)
backend/src/workerStart.ts (3)
3-15: LGTM!
24-34: LGTM!
37-44: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cccd9315f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c69835c461
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
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 `@backend/src/routes.ts`:
- Line 60: Update health() so failures from the synchronous
getJobExecutionSummary() queue queries cannot cause /health or /api/health to
return 500. Keep the liveness response available by isolating the telemetry
lookup or catching its errors, and return the established documented degraded
telemetry value when queue access fails.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69b87a6e-c650-4976-acaa-4b3aed7e9489
📒 Files selected for processing (15)
backend/src/routes.tsbackend/src/serverStart.tsbackend/src/services/backups.tsbackend/src/services/jobExecutionQueue.tsbackend/src/services/pullRequests.tsbackend/src/services/scheduledJobs.tsbackend/src/workerStart.tsbackend/test/httpApiBehavior.test.tsbackend/test/jobExecutionQueue.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsdocs/api/endpoints.mddocs/architecture/gateway-and-chat.mddocs/setup/new-vps.mddocs/setup/production-deploy.md
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/setup/new-vps.md
- backend/src/workerStart.ts
- docs/setup/production-deploy.md
- backend/test/httpApiBehavior.test.ts
- backend/src/serverStart.ts
- backend/test/serverStartupPolicy.test.ts
- docs/api/endpoints.md
- backend/src/services/backups.ts
- backend/test/serviceBehavior.test.ts
- backend/src/services/pullRequests.ts
- backend/src/services/scheduledJobs.ts
📜 Review details
🔇 Additional comments (9)
backend/src/routes.ts (1)
16-16: LGTM!Also applies to: 33-33, 89-89
backend/src/services/jobExecutionQueue.ts (4)
85-105: LGTM!
254-277: LGTM!
584-605: LGTM!
424-435: 🩺 Stability & AvailabilityVerify lifecycle handlers are registered by the worker.
The handler map is process-local. In dedicated-worker deployments, ensure
backend/src/workerStart.tsinvokesregisterPullRequestJobLifecycleHandlers()before expired-lease recovery; the web startup path registers it instead.backend/test/jobExecutionQueue.test.ts (3)
26-30: LGTM!
240-282: LGTM!
284-320: 🎯 Functional CorrectnessSchema supports this test.
PRAGMA foreign_keys = ONis enabled;scheduled_job_runs.job_idcascades on remove, whilejob_executions.scheduled_job_idis not a foreign key, so deleting the scheduled job keeps the queued execution visible for the executor.docs/architecture/gateway-and-chat.md (1)
58-58: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6af9ded08a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd98c6097e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79136626ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c82adcd97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d33675a72e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
## Summary - make SQLite WAL mode part of database initialization and use immediate write transactions where web/worker contention can otherwise cause lock-upgrade failures - refresh the Execution queue every 5 seconds, invalidate it immediately after UI-triggered jobs, and retain recent completions so short jobs remain visible - add the Execution queue to the main Dashboard above Actions, with Queue and Actions sharing the column height beside Backups - move the active chat model into a badge and simplify the context line ## Why PR #314 split the web and worker execution planes. The production rollout exposed one transient `database is locked` failure while both processes wrote to SQLite. The Jobs queue also polled every 15 seconds while idle and only rendered active executions, so short jobs could complete without ever appearing. ## Verification All local checks ran inside CPU/memory-limited systemd scopes: - `backend/test/openClawChatSnapshotStore.test.ts`: 13 passed - `src/test/chatHeader.test.tsx`: 2 passed - targeted Execution queue component test: 1 passed - targeted scheduled-job hook/invalidation test: 1 passed - targeted main-page rendering test: 1 passed - targeted chat-page header regression test: 1 passed - ESLint on all 21 touched files: passed - `git diff --check`: passed Full build, typecheck, lint, and test coverage are delegated to GitHub CI to avoid resource contention on the production VPS. ## Production rollout note The live database was backed up and migrated to WAL before this PR. Both split services are active with zero restarts, queue capacity is `1/1`, and a post-migration manual worker job completed successfully without a lock error. The backup is: `/home/ubuntu/.local/state/mira-dashboard-rollbacks/task-367-20260723T1407CEST/mira-dashboard.pre-wal.db`
4 participants
Summary
mira-dashboard-worker.service, so web restarts do not interrupt deploy, exec, Docker exec, cache, backup, log-rotation, OpenClaw, or GitHub actionstypescript@6.0.3tooling workaround and@typescript/nativeTypeScript 7 build pathLocal Mira Dashboard task: 367 (not a GitHub issue).
Operational rollout
This PR does not install units, restart services, or deploy production.
Both tracked units retain the existing Doppler
rajohan/prdlaunch contract, so auth/origin settings such asMIRA_DASHBOARD_ENABLE_LOOPBACK_AUTHremain sourced from the same environment. For the first split-process rollout, the documented order is:systemctl --user daemon-reloadwebroleThe repository review was updated separately; P1 remains open until controlled production rollout and runtime measurement are complete.
Verification
git diff --checkbun audit --audit-level=highin both package rootssystemd-analyze --user verifyfor both tracked units