fix: consolidate Dashboard runtime paths and delivery state - #342
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughDeployment cutover jobs now use ChangesDashboard runtime, deployment lifecycle, and preview controls
Estimated code review effort: 4 (Complex) | ~60 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 |
mira-2026
added
type: bugfix
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: 2
🧹 Nitpick comments (1)
backend/test/serviceBehavior.test.ts (1)
1998-2001: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the persisted terminal timestamp, not only generated script text.
These assertions only prove the guardian command contains
updatedAt: new Date().toISOString(). They would still pass if the terminal SQL update persisted the old timestamp. After deploy and rollback completion, querydeployment_jobs.updated_atand assert it is a valid, newer timestamp than the scheduled job.Also applies to: 2581-2584
🤖 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/test/serviceBehavior.test.ts` around lines 1998 - 2001, Update the deployment rollback test around the guardian assertions to query the persisted deployment_jobs.updated_at value after deploy and rollback completion, then validate it is a valid timestamp newer than the scheduled job’s timestamp. Keep the existing script-text assertion, but add the database-level assertion so generated SQL and persisted state are both verified.
🤖 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/services/pullRequests.ts`:
- Around line 351-363: Update the DeploymentJob status union to include the
internal "restart-scheduled" value, while retaining "verifying" in
PublicDeploymentJob as the externally mapped status. Ensure publicDeploymentJob
and the cutoverJob literals type-check while preserving the mapping from
"restart-scheduled" to "verifying".
In `@src/test/pageBehavior.test.tsx`:
- Around line 3303-3326: Update the test setup around the globalThis.fetch
override to preserve the original fetch property and restore it in a finally
block after the test completes, including when assertions or cleanup fail.
Alternatively, replace the manual defineProperty mock with a Jest spy and call
mockRestore(), while preserving the existing deployment response behavior.
---
Nitpick comments:
In `@backend/test/serviceBehavior.test.ts`:
- Around line 1998-2001: Update the deployment rollback test around the guardian
assertions to query the persisted deployment_jobs.updated_at value after deploy
and rollback completion, then validate it is a valid timestamp newer than the
scheduled job’s timestamp. Keep the existing script-text assertion, but add the
database-level assertion so generated SQL and persisted state are both verified.
🪄 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: e6295ab2-b430-4201-aa71-89386bc5a45f
📒 Files selected for processing (5)
backend/src/services/pullRequests.tsbackend/test/serviceBehavior.test.tssrc/hooks/useDelivery.tssrc/pages/Delivery.tsxsrc/test/pageBehavior.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Analyze JavaScript and TypeScript
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
🔇 Additional comments (5)
backend/src/services/pullRequests.ts (1)
752-773: LGTM!Also applies to: 1879-1882, 2126-2130, 2233-2233, 2343-2343
backend/test/serviceBehavior.test.ts (1)
1574-1574: LGTM!Also applies to: 1594-1594, 1974-1977, 2541-2541, 2680-2680
src/hooks/useDelivery.ts (1)
38-40: LGTM!src/pages/Delivery.tsx (1)
293-299: LGTM!src/test/pageBehavior.test.tsx (1)
1805-1805: LGTM!
mira-2026
changed the title
![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: c3cfc43e8e
ℹ️ 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: 7
🧹 Nitpick comments (2)
backend/src/database.ts (1)
32-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated production-gated dashboard project-paths ternary into a shared helper. All three files independently reimplement
configuredDashboardProjectPaths() ?? (process.env.NODE_ENV === "production" ? resolveDashboardProjectPaths() : undefined). Moving this intobackend/src/lib/dashboardPaths.ts(e.g.resolveDashboardProjectPathsForRuntime()) keeps the production-gating rule defined once.
backend/src/database.ts#L32-L39: replace the inline ternary inresolveDatabasePath()with a call to the shared helper.backend/src/gateway.ts#L38-L44: replace the inline ternary computingDEFAULT_DASHBOARD_OPENCLAW_HOMEwith a call to the shared helper.backend/src/services/logRotation.ts#L85-L97: replace the inline ternary inresolveLogRotationLockFile()with a call to the shared helper.🤖 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/database.ts` around lines 32 - 39, Extract the repeated production-gated project-path resolution into a shared helper such as resolveDashboardProjectPathsForRuntime in backend/src/lib/dashboardPaths.ts, preserving configuredDashboardProjectPaths() precedence and production-only fallback behavior. Update resolveDatabasePath in backend/src/database.ts, the DEFAULT_DASHBOARD_OPENCLAW_HOME initialization in backend/src/gateway.ts, and resolveLogRotationLockFile in backend/src/services/logRotation.ts to use the helper at the specified ranges.backend/src/development/developmentStack.ts (1)
599-673: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winBackfill's initial
DELETEisn't part of the same transaction as the guard check + insert.
target.run("DELETE FROM deployment_jobs WHERE status NOT IN ('isOk', 'failed')")(line 602) auto-commits immediately, before theBEGIN IMMEDIATEtransaction (line 636) that guards the existing-check and inserts backfilled rows. If the process is interrupted between these two points, the target database is left with its non-terminal rows deleted but no completed history restored yet.♻️ Proposed fix: fold the cleanup delete into the same transaction
- target.run("DELETE FROM deployment_jobs WHERE status NOT IN ('isOk', 'failed')"); - const existing = target - .query( - "SELECT 1 FROM deployment_jobs WHERE status IN ('isOk', 'failed') LIMIT 1" - ) - .get(); - if (existing) return; + const existing = target + .query( + "SELECT 1 FROM deployment_jobs WHERE status IN ('isOk', 'failed') LIMIT 1" + ) + .get(); + if (existing) { + target.run( + "DELETE FROM deployment_jobs WHERE status NOT IN ('isOk', 'failed')" + ); + return; + } @@ target.run("BEGIN IMMEDIATE"); try { + target.run( + "DELETE FROM deployment_jobs WHERE status NOT IN ('isOk', 'failed')" + ); const insert = target.prepare(🤖 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/development/developmentStack.ts` around lines 599 - 673, Move the cleanup DELETE in the backfill flow into the same BEGIN IMMEDIATE transaction as the existing completed-history check and insert, so deletion, guard evaluation, and restoration commit or roll back together. Update the transaction structure around the target database logic and preserve rollback handling through the existing target.inTransaction path.
🤖 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/services/pullRequestPreviews.ts`:
- Around line 154-160: Apply the MIRA_DASHBOARD_DEV_SAFE_MODE guard inside the
mutation helpers prepareAndStartPullRequestPreview() and
prepareAndStopPullRequestPreview(), before they enqueue any preview lifecycle
jobs or manage resources. Return the same controls-unavailable response used by
the existing safe-mode status path so both preview start routes consistently
prevent mutations while preserving normal behavior outside safe mode.
In `@backend/src/services/pullRequests.ts`:
- Line 99: Update ACTIVE_DEPLOYMENT_STATUSES and the related orphan-recovery and
pending-cutover reconciliation checks to recognize both "verifying" and legacy
"restart-scheduled" statuses. Preserve existing handling for "building", and
ensure rows using either transitional status remain eligible for active-status
and cutover reconciliation until they become terminal.
In `@backend/test/pullRequestPreview.test.ts`:
- Line 35: Update the pull request preview status test to import and invoke
getPullRequestPreviewStatus from pullRequestPreviews.ts rather than the aliased
getManagedPullRequestPreviewStatus from pullRequestPreviewHost.ts, including the
call site around the asserted status result. Preserve the existing equality
assertions for the Delivery wrapper’s safe-mode fields.
In `@backend/test/releaseDeployment.test.ts`:
- Around line 142-154: Update the obsolete-variable assertions in the release
deployment test around the obsoleteEnvironment loop to detect each variable
anywhere within an Environment= directive, not only immediately after the
directive. Match both the first assignment and variables appended in
multi-assignment forms, while preserving the existing rejection behavior.
In `@docs/architecture/database.md`:
- Around line 125-127: Update the documented deploy:prepare command to set
MIRA_DASHBOARD_PROJECT_ROOT to /home/ubuntu/projects/mira-dashboard/production
before execution, ensuring preflight resolves the production database path
rather than local backend/data; keep the existing directory and Doppler
invocation intact.
In `@docs/setup/new-vps.md`:
- Around line 51-55: Update the install -d command in the VPS setup instructions
so development/state is created with owner-only permissions (0700), matching
production/state and the documented state-root policy; leave the
development/worktrees and production/releases directory permissions unchanged.
In `@README.md`:
- Around line 145-146: Update the README wording for production/checkout to
describe it only as the clean control source kept on main, not as the location
where production builds run. Align the statement with the detached-worktree
build process documented in production deployment guidance, while preserving the
approval requirement.
---
Nitpick comments:
In `@backend/src/database.ts`:
- Around line 32-39: Extract the repeated production-gated project-path
resolution into a shared helper such as resolveDashboardProjectPathsForRuntime
in backend/src/lib/dashboardPaths.ts, preserving
configuredDashboardProjectPaths() precedence and production-only fallback
behavior. Update resolveDatabasePath in backend/src/database.ts, the
DEFAULT_DASHBOARD_OPENCLAW_HOME initialization in backend/src/gateway.ts, and
resolveLogRotationLockFile in backend/src/services/logRotation.ts to use the
helper at the specified ranges.
In `@backend/src/development/developmentStack.ts`:
- Around line 599-673: Move the cleanup DELETE in the backfill flow into the
same BEGIN IMMEDIATE transaction as the existing completed-history check and
insert, so deletion, guard evaluation, and restoration commit or roll back
together. Update the transaction structure around the target database logic and
preserve rollback handling through the existing target.inTransaction path.
🪄 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: f2576129-ed1d-4cae-8226-8c9db13d0865
📒 Files selected for processing (41)
README.mdbackend/package.jsonbackend/src/database.tsbackend/src/development/developmentStack.tsbackend/src/gateway.tsbackend/src/lib/dashboardPaths.tsbackend/src/releaseDeployment.tsbackend/src/releaseManager.tsbackend/src/services/cacheRefresh.tsbackend/src/services/logRotation.tsbackend/src/services/pullRequestPreviewHost.tsbackend/src/services/pullRequestPreviews.tsbackend/src/services/pullRequests.tsbackend/src/services/scheduledJobs.tsbackend/src/services/sqliteMaintenance.tsbackend/test/developmentStack.test.tsbackend/test/jobExecutionQueue.test.tsbackend/test/pullRequestPreview.test.tsbackend/test/releaseDeployment.test.tsbackend/test/serviceBehavior.test.tsbackend/test/testDatabaseGuard.test.tsdocs/architecture/database.mddocs/architecture/gateway-and-chat.mddocs/development/local-dev.mddocs/development/testing-and-prs.mddocs/index.mddocs/operations/runbooks.mddocs/operations/scheduler-cache-backups.mddocs/operations/troubleshooting.mddocs/security/auth-and-trust-boundaries.mddocs/setup/new-vps.mddocs/setup/production-deploy.mddocs/setup/secrets-and-env.mdsrc/components/features/delivery/PullRequestDevelopmentCard.tsxsrc/hooks/useDelivery.tssrc/pages/Delivery.tsxsrc/test/openClawAdapterVariants.test.tssrc/test/pageBehavior.test.tsxsrc/test/pullRequestDevelopmentCard.test.tsxsystemd/mira-dashboard-worker.servicesystemd/mira-dashboard.service
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
🧰 Additional context used
🪛 ast-grep (0.44.1)
backend/src/releaseManager.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 LanguageTool
docs/setup/secrets-and-env.md
[grammar] ~48-~48: Ensure spelling is correct
Context: ...sometimes need deliberately nonstandard roots. Production mutable state lives in `/home...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (49)
backend/test/releaseDeployment.test.ts (7)
14-14: LGTM!Also applies to: 29-29
38-40: LGTM!
87-120: LGTM!
137-141: LGTM!
164-169: LGTM!
531-531: LGTM!
592-607: LGTM!README.md (1)
147-160: LGTM!docs/architecture/database.md (1)
14-14: LGTM!Also applies to: 153-157, 171-177, 200-201
docs/architecture/gateway-and-chat.md (1)
620-621: LGTM!docs/development/local-dev.md (1)
58-58: LGTM!Also applies to: 74-84, 141-149, 167-170, 192-192
docs/setup/new-vps.md (2)
32-37: LGTM!Also applies to: 64-66, 136-136, 166-166
88-96: 🎯 Functional CorrectnessVerify the activation commands inherit the exported project root.
These changed command blocks are not included in the supplied snippet. Confirm they run in the same shell as the earlier
export MIRA_DASHBOARD_PROJECT_ROOT, or repeat the export inside each standalone block so activation cannot fall back to non-production paths.docs/setup/production-deploy.md (1)
5-71: LGTM!Also applies to: 83-83, 100-102, 135-137, 174-175, 204-209
docs/setup/secrets-and-env.md (1)
26-54: LGTM!Also applies to: 166-187
docs/development/testing-and-prs.md (1)
122-122: LGTM!docs/index.md (1)
54-54: LGTM!docs/operations/runbooks.md (1)
63-68: LGTM!Also applies to: 77-77, 88-96, 105-105, 116-116, 137-137, 203-203, 282-285, 343-356
docs/operations/scheduler-cache-backups.md (1)
237-252: LGTM!Also applies to: 262-264
docs/operations/troubleshooting.md (1)
12-12: LGTM!Also applies to: 105-105
docs/security/auth-and-trust-boundaries.md (1)
113-113: LGTM!Also applies to: 190-196, 207-207
backend/src/services/pullRequestPreviews.ts (1)
42-43: LGTM!Also applies to: 97-129, 196-196
src/hooks/useDelivery.ts (1)
40-40: LGTM!Also applies to: 94-106
src/pages/Delivery.tsx (1)
294-299: LGTM!Also applies to: 834-912
src/components/features/delivery/PullRequestDevelopmentCard.tsx (1)
61-61: LGTM!Also applies to: 77-95, 142-154
backend/test/pullRequestPreview.test.ts (1)
219-234: LGTM!src/test/pageBehavior.test.tsx (1)
1729-1731: LGTM!Also applies to: 1805-1805, 2942-3005, 3316-3372, 3375-3408
src/test/pullRequestDevelopmentCard.test.tsx (1)
42-58: LGTM!src/test/openClawAdapterVariants.test.ts (1)
733-733: LGTM!Also applies to: 759-759
backend/src/lib/dashboardPaths.ts (1)
1-61: LGTM!backend/src/services/cacheRefresh.ts (1)
19-19: LGTM!Also applies to: 63-63, 84-87, 110-113
backend/src/services/logRotation.ts (1)
76-83: LGTM!backend/src/services/pullRequestPreviewHost.ts (1)
25-25: LGTM!Also applies to: 67-79, 287-300, 371-371, 407-407
backend/package.json (1)
12-12: LGTM!backend/src/development/developmentStack.ts (2)
397-404: LGTM!Also applies to: 436-436, 465-465, 726-730, 1016-1018
638-651: 🗄️ Data Integrity & IntegrationNo schema columns omitted from the
deployment_jobsbackfill INSERT.The table schema contains exactly
id,status,started_at,updated_at,commit_sha,commit_title,note,stdout, andstderr, and there are noALTER TABLE deployment_jobsmigrations adding columns.backend/test/testDatabaseGuard.test.ts (1)
9-10: LGTM!Also applies to: 151-151
systemd/mira-dashboard-worker.service (1)
9-15: 📐 Maintainability & Code QualityNo change needed.
%his not tied to theUser=value in the[Service]section, so replacingMIRA_DASHBOARD_PROJECT_ROOT=/home/ubuntuwith%h/projects/mira-dashboardwould not make it “track” the service user; both values are still decoupled fromUser=.> Likely an incorrect or invalid review comment.backend/src/services/scheduledJobs.ts (1)
1371-1409: 🩺 Stability & AvailabilityConsistent with the status rename — see root-cause comment in
backend/src/services/pullRequests.ts.These three query sites correctly mirror the
'verifying'literal used elsewhere, but they share the same backward-compatibility concern raised onbackend/src/services/pullRequests.ts(Lines 212-223): a cutover row written by the pre-PR binary asrestart-scheduledwould not be matched by any of these queries after this release activates.Also applies to: 1411-1496, 1506-1528
backend/src/services/sqliteMaintenance.ts (1)
240-247: 🩺 Stability & AvailabilitySame root-cause status rename as flagged in
backend/src/services/pullRequests.ts.The exclusion list now only protects
'building'/'verifying'rows from pruning; a transitionalrestart-scheduledrow (see root-cause comment) would not be protected here either, though the practical impact is lower (pruning only applies after 90 days).backend/src/releaseManager.ts (2)
15-18: Fallback chain forresolveDashboardReleasesRootlooks correct.Short-circuiting via
??correctly avoids callingresolveDashboardProjectPaths({})unless both the env var andconfiguredDashboardProjectPaths()are unset, and the final value is still validated throughresolveAbsoluteNonRootPath.Also applies to: 371-377
30-30: 🎯 Functional CorrectnessNo remaining references to
DEFAULT_DASHBOARD_RELEASES_ROOT.backend/src/releaseDeployment.ts (1)
218-266: LGTM!Also applies to: 331-367
backend/src/services/pullRequests.ts (1)
4-4: Dashboard root/worktree defaults now derive from the project-root contract.
getDashboardRoot()/getDashboardWorktreeRoot()correctly delegate toresolveDashboardProjectPaths()for defaults while still allowing explicit env-var overrides viaresolveConfiguredRoot.Also applies to: 136-145
systemd/mira-dashboard.service (1)
9-15: 🩺 Stability & AvailabilityConfirm the production host filesystem has been migrated to the new project-root layout before this unit is rolled out.
MIRA_DASHBOARD_PROJECT_ROOTandWorkingDirectorynow point at.../projects/mira-dashboard/production/releases/..., a different layout than the legacy releases path implied elsewhere in the codebase (e.g. oldmira-dashboard-releasesnaming in test fixtures). If the production host's actual releases/state directories haven't been physically relocated (or symlinked) to this new layout as part of this rollout, the service will fail to locatecurrent/previousrelease slots on first boot after this deploy.backend/test/developmentStack.test.ts (1)
86-168: LGTM!Also applies to: 377-404, 504-520
backend/test/jobExecutionQueue.test.ts (1)
84-101: LGTM!Also applies to: 104-320
backend/test/serviceBehavior.test.ts (2)
50-78: LGTM!Also applies to: 2072-2093, 2916-2937
1901-1988: Environment/systemd fixture wiring correctly mirrors the reduced preserved-env contract.Matches
MANAGED_DASHBOARD_PRESERVED_ENVIRONMENTinbackend/src/releaseDeployment.tsand the systemd unit's--preserve-envlist.Also applies to: 2402-2548
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
backend/src/services/logRotation.ts (1)
152-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
resolveBunExecutablenow that both bodies collapsed toprocess.execPath.Graph evidence shows
backend/src/services/pullRequests.tsdefines the identical function. Now that both are a one-liner, worth extracting to a shared helper (e.g.lib/jobResources.tsorlib/values.ts) to avoid drift if the executable-resolution logic changes again.🤖 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/services/logRotation.ts` around lines 152 - 154, Extract the shared process-executable resolution from resolveBunExecutable in logRotation.ts and the identical function in pullRequests.ts into a common helper module, then import and reuse that helper from both call sites. Remove the duplicate local function definitions while preserving the existing process.execPath 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 `@backend/src/development/developmentStack.ts`:
- Around line 572-590: Update backfillCompletedDeploymentHistory so it clears
deployment_lock rows associated with non-terminal deployment_jobs before
deleting those jobs, matching the ordering used by scrubDevelopmentDatabase.
Keep completed and failed job history intact, and ensure the cleanup runs within
the existing transaction on the target database.
In `@backend/src/routes/sttRoutes.ts`:
- Around line 7-8: Update the ElevenLabs STT configuration around
ELEVENLABS_STT_LANGUAGE so language selection is not unconditionally forced to
"nor"; restore omission/null-based auto-detection or an equivalent configurable
default while preserving explicit Norwegian selection when requested. Ensure the
request-building logic uses the selected language value consistently.
In `@backend/src/services/pullRequests.ts`:
- Line 99: Update the migration that creates idx_deployment_jobs_retention so
its partial-index predicate excludes both building and verifying statuses,
matching the retention DELETE behavior. Add a new migration or advance the
existing one to recreate the index with these active deployment exclusions.
In `@docs/setup/secrets-and-env.md`:
- Around line 44-47: Update the documented production state, development state,
source database, and releases paths to be derived from the resolved
MIRA_DASHBOARD_PROJECT_ROOT rather than hardcoded home-directory paths. Document
the fallback root only when MIRA_DASHBOARD_PROJECT_ROOT is unset, and ensure all
backup, restore, and snapshot examples use these derived paths.
---
Nitpick comments:
In `@backend/src/services/logRotation.ts`:
- Around line 152-154: Extract the shared process-executable resolution from
resolveBunExecutable in logRotation.ts and the identical function in
pullRequests.ts into a common helper module, then import and reuse that helper
from both call sites. Remove the duplicate local function definitions while
preserving the existing process.execPath 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d8c98fa-1d47-495f-9aa5-7f2501810e59
📒 Files selected for processing (53)
README.mdbackend/src/database.tsbackend/src/development/developmentStack.tsbackend/src/frontendAssets.tsbackend/src/gateway.tsbackend/src/http.tsbackend/src/lib/dashboardPaths.tsbackend/src/lib/jobResources.tsbackend/src/lib/logRoots.tsbackend/src/lib/values.tsbackend/src/releaseDeployment.tsbackend/src/releaseManager.tsbackend/src/releaseManifest.tsbackend/src/requestPolicy.tsbackend/src/routes/authRoutes.tsbackend/src/routes/mediaRoutes.tsbackend/src/routes/metricsRoutes.tsbackend/src/routes/sttRoutes.tsbackend/src/routes/ttsRoutes.tsbackend/src/serverStart.tsbackend/src/serverStartPolicy.tsbackend/src/services/cacheRefresh.tsbackend/src/services/dockerUpdater.tsbackend/src/services/jobWorker.tsbackend/src/services/logRotation.tsbackend/src/services/pullRequestPreviewHost.tsbackend/src/services/pullRequestPreviewPolicy.tsbackend/src/services/pullRequestPreviews.tsbackend/src/services/pullRequests.tsbackend/test/developmentStack.test.tsbackend/test/dockerUpdater.test.tsbackend/test/jobExecutionQueue.test.tsbackend/test/pullRequestPreview.test.tsbackend/test/releaseDeployment.test.tsbackend/test/routeAndServiceBehavior.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsbackend/test/setup.tsbackend/test/utilityBehavior.test.tsdocs/architecture/database.mddocs/architecture/gateway-and-chat.mddocs/architecture/overview.mddocs/development/local-dev.mddocs/operations/docker-updater.mddocs/operations/runbooks.mddocs/operations/scheduler-cache-backups.mddocs/security/auth-and-trust-boundaries.mddocs/setup/new-vps.mddocs/setup/production-deploy.mddocs/setup/secrets-and-env.mdscripts/developmentTailscale.tssystemd/mira-dashboard-worker.servicesystemd/mira-dashboard.service
💤 Files with no reviewable changes (5)
- backend/src/routes/mediaRoutes.ts
- docs/operations/docker-updater.md
- scripts/developmentTailscale.ts
- backend/test/routeAndServiceBehavior.test.ts
- docs/operations/runbooks.md
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/operations/scheduler-cache-backups.md
- docs/architecture/gateway-and-chat.md
- backend/src/lib/dashboardPaths.ts
- docs/development/local-dev.md
- backend/src/database.ts
- docs/security/auth-and-trust-boundaries.md
- backend/src/services/pullRequestPreviews.ts
- docs/architecture/database.md
- docs/setup/production-deploy.md
- docs/setup/new-vps.md
- backend/test/developmentStack.test.ts
- backend/test/serviceBehavior.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
🧰 Additional context used
🪛 ast-grep (0.44.1)
backend/src/releaseDeployment.ts
[error] 87-91: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. if (key === "__proto__" || key === "constructor" || key === "prototype") continue;), use a null-prototype object (Object.create(null)), or use a safe merge utility instead.
Context: for (const key of MANAGED_RELEASE_BUILD_ENVIRONMENT) {
if (process.env[key] !== undefined) {
environment[key] = process.env[key];
}
}
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').
(prototype-pollution-recursive-merge-typescript)
backend/src/releaseManager.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
backend/test/releaseDeployment.test.ts
[warning] 178-181: Do not use variable for regular expressions
Context: new RegExp(
String.raw(?:^Environment=|\s)${obsoleteEnvironment}=,
"m"
)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (47)
backend/src/releaseManager.ts (1)
18-18: LGTM!Also applies to: 373-377
backend/test/releaseDeployment.test.ts (1)
14-18: LGTM!Also applies to: 64-75, 105-130, 178-183, 208-224, 277-281, 317-345, 547-584
docs/setup/secrets-and-env.md (3)
8-43: LGTM!Also applies to: 48-66
85-128: LGTM!
153-158: LGTM!Also applies to: 162-178
docs/architecture/overview.md (1)
136-136: LGTM!Also applies to: 166-170
backend/test/pullRequestPreview.test.ts (4)
35-35: 🎯 Functional CorrectnessImport now correctly aliases the Delivery wrapper.
Addresses the prior review comment about invoking the managed-host reader instead of the Delivery preview-status wrapper —
getPullRequestPreviewStatus(Delivery/safe-mode aware) is now imported and aliased here.
68-96: LGTM!
205-307: LGTM!
155-204: 📐 Maintainability & Code QualityNo change needed.
stateNumbersSpyis asserted withexpect(stateNumbersSpy).not.toHaveBeenCalled()and restored withstateNumbersSpy.mockRestore()in this test.backend/src/services/pullRequestPreviewPolicy.ts (1)
1-6: LGTM!systemd/mira-dashboard-worker.service (1)
9-12: LGTM!
WorkingDirectory(%h-based) and the literalMIRA_DASHBOARD_PROJECT_ROOTboth resolve to the same location for the intendedubuntudeployment user, and the--preserve-envlist matches exactly the twoEnvironment=variables declared, sodoppler runwon't silently override either.systemd/mira-dashboard.service (1)
9-12: LGTM!backend/src/releaseManifest.ts (1)
712-722: LGTM!Consistent with the consolidated
.../releases/current/backendWorkingDirectoryin the systemd units and with the updatedserverStartupPolicy.test.tstests that synthesize abackend-named cwd to exercise this exact heuristic.backend/test/serverStartupPolicy.test.ts (6)
13-19: LGTM!
39-47: LGTM!
71-91: LGTM!
194-253: LGTM!
397-568: LGTM!
614-742: LGTM!backend/test/dockerUpdater.test.ts (2)
631-658: LGTM!Also applies to: 750-786, 832-869
462-497: 📐 Maintainability & Code QualityNo change needed.
MIRA_DOCKER_UPDATER_PLATFORMis no longer referenced anywhere in the repository, and the update path consumes the Composeplatform:field directly.backend/src/gateway.ts (1)
8-11: LGTM!Also applies to: 38-40, 210-213
backend/src/serverStartPolicy.ts (1)
1-5: LGTM!backend/src/serverStart.ts (2)
36-45: LGTM!
130-139: 📐 Maintainability & Code QualityNo action needed. No stale
startServer/startOnImportoptions orMIRA_DASHBOARD_START_ON_IMPORTreferences remain in the relevant code/docs/scripts.README.md (1)
145-160: LGTM! The prior feedback about clarifying that production builds run from a detached worktree (notproduction/checkout) has been addressed.backend/src/development/developmentStack.ts (1)
368-391: LGTM!Also applies to: 402-417, 440-451, 1058-1089
backend/test/setup.ts (1)
12-17: LGTM!Also applies to: 32-35
backend/test/jobExecutionQueue.test.ts (1)
84-101: LGTM!Also applies to: 104-140, 252-306, 480-485
backend/src/lib/jobResources.ts (1)
29-29: 🩺 Stability & AvailabilityNo change needed.
spawnProcessis only reachable from worker job execution paths in this codebase, so binding the transientsystemd-runscope tomira-dashboard-worker.serviceis appropriate.backend/src/releaseDeployment.ts (1)
228-235: 🗄️ Data Integrity & IntegrationNo action needed. The
previewRoot/previewWorktreePathmapping matches the existing preview config path assignments:previewRootusesdevelopmentPreviewStateRootandpreviewWorktreePathusesdevelopmentPreviewRoot.> Likely an incorrect or invalid review comment.backend/src/services/pullRequestPreviewHost.ts (1)
18-26: LGTM!Also applies to: 45-46, 69-72, 114-115, 186-206, 235-285, 552-568, 1004-1046, 1063-1194
backend/src/frontendAssets.ts (1)
6-16: LGTM!backend/src/http.ts (1)
9-26: LGTM!backend/src/services/logRotation.ts (1)
7-8: LGTM!Also applies to: 75-103, 2045-2045, 2056-2070
backend/src/routes/metricsRoutes.ts (1)
70-71: LGTM!Also applies to: 113-165, 239-239
backend/src/routes/ttsRoutes.ts (1)
5-6: LGTM!backend/src/routes/authRoutes.ts (1)
208-210: 🎯 Functional CorrectnessNo change needed —
OPENCLAW_TOKENis no longer referenced.All tracked references now use
OPENCLAW_GATEWAY_TOKEN.backend/src/services/cacheRefresh.ts (1)
19-19: LGTM!Also applies to: 63-63, 87-87, 110-113, 1867-1869
backend/src/lib/logRoots.ts (1)
16-27: LGTM!backend/src/lib/values.ts (1)
24-28: LGTM!backend/src/requestPolicy.ts (1)
161-161: LGTM!Also applies to: 177-180, 195-195
backend/src/services/dockerUpdater.ts (1)
1124-1124: LGTM!Also applies to: 1225-1228
backend/src/services/jobWorker.ts (1)
35-41: LGTM!backend/test/utilityBehavior.test.ts (1)
471-473: LGTM!Also applies to: 583-609, 909-929
backend/src/services/pullRequests.ts (1)
4-4: LGTM!Also applies to: 106-107, 118-132, 910-912, 940-940, 951-951, 1326-1326, 1657-1657, 1847-1850, 1913-1917, 2068-2070, 2090-2094, 2187-2194, 2253-2253, 2304-2309, 2343-2348, 2489-2492
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8c61231de
ℹ️ 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
🧹 Nitpick comments (2)
backend/src/services/deploymentCutoverState.ts (1)
10-19: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThis runs a full
deployment_jobsscan on every production mutation.
isProductionDeploymentCutoverActiveis invoked per API request (requestPolicy.secureHandler) and per/wsupgrade, and the retention index added in migration 0007 is partial onstatus NOT IN ('building','verifying'), so it cannot serve this predicate. With up to 500 retained rows, each write request scans the table. A tiny partial index (or a cached flag invalidated on deployment-job writes) removes it from the hot path.⚡ Suggested index (new migration)
CREATE INDEX IF NOT EXISTS idx_deployment_jobs_active ON deployment_jobs(status) WHERE status = 'verifying';🤖 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/services/deploymentCutoverState.ts` around lines 10 - 19, Add a new migration creating the suggested partial index on deployment_jobs(status) for rows where status = 'verifying', ensuring isProductionDeploymentCutoverActive can use it without changing the existing predicate or query behavior.backend/src/requestPolicy.ts (1)
160-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why the activity header also blocks.
The
|| header === "1"clause reads like an exemption but is actually an additional block (a safeGETcarryingx-mira-user-activity: 1writes session activity). One inline comment prevents a future "fix" from inverting this into a bypass.📝 Proposed clarification
return ( + // Safe methods still write session activity when the touch header is set. !SAFE_REQUEST_METHODS.has(request.method.toUpperCase()) || request.headers.get("x-mira-user-activity")?.trim() === "1" );🤖 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/requestPolicy.ts` around lines 160 - 163, Update the request policy condition around SAFE_REQUEST_METHODS to add an inline comment explaining that x-mira-user-activity: 1 is an additional blocking condition because even safe methods write session activity; make clear this clause is not a bypass or exemption.
🤖 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/services/pullRequests.ts`:
- Around line 1922-1988: The deploymentCutoverHandoffCommand polling window is
shorter than the interrupt-handler cleanup grace period, allowing completed
deployments to be marked as handoff failures. Increase
DEPLOYMENT_CUTOVER_HANDOFF_TIMEOUT_MS to exceed the 60-second cleanup grace,
while preserving the existing readiness checks and polling behavior.
---
Nitpick comments:
In `@backend/src/requestPolicy.ts`:
- Around line 160-163: Update the request policy condition around
SAFE_REQUEST_METHODS to add an inline comment explaining that
x-mira-user-activity: 1 is an additional blocking condition because even safe
methods write session activity; make clear this clause is not a bypass or
exemption.
In `@backend/src/services/deploymentCutoverState.ts`:
- Around line 10-19: Add a new migration creating the suggested partial index on
deployment_jobs(status) for rows where status = 'verifying', ensuring
isProductionDeploymentCutoverActive can use it without changing the existing
predicate or query 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ac54d29-ab44-4412-a210-e5eeae379f35
📒 Files selected for processing (19)
backend/src/databaseSchemaCompatibility.tsbackend/src/development/developmentStack.tsbackend/src/releaseLifecycle.tsbackend/src/releaseManager.tsbackend/src/requestPolicy.tsbackend/src/server.tsbackend/src/services/deploymentCutoverState.tsbackend/src/services/pullRequests.tsbackend/src/sqliteBackup.tsbackend/test/databaseLifecycle.test.tsbackend/test/developmentStack.test.tsbackend/test/healthReadiness.test.tsbackend/test/releaseManager.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsbackend/test/utilityBehavior.test.tsdocs/architecture/database.mddocs/operations/scheduler-cache-backups.mddocs/setup/production-deploy.md
💤 Files with no reviewable changes (1)
- backend/src/releaseManager.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/src/databaseSchemaCompatibility.ts
- docs/architecture/database.md
- backend/test/developmentStack.test.ts
- backend/test/serverStartupPolicy.test.ts
- backend/src/development/developmentStack.ts
- backend/test/serviceBehavior.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: frontend-checks
🧰 Additional context used
🪛 ast-grep (0.44.1)
backend/test/releaseManager.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (14)
backend/test/healthReadiness.test.ts (1)
83-83: LGTM!backend/test/utilityBehavior.test.ts (1)
8-8: LGTM!Also applies to: 39-39, 55-55, 74-75, 476-478, 588-615, 617-697, 996-1016
backend/test/releaseManager.test.ts (1)
59-68: LGTM!Also applies to: 658-666, 700-720, 731-756, 758-769, 779-798, 810-819, 828-828
backend/src/services/pullRequests.ts (2)
94-104: LGTM!Also applies to: 383-502
2191-2254: LGTM!Also applies to: 2367-2532, 2659-2677
backend/src/sqliteBackup.ts (2)
40-97: LGTM!Also applies to: 226-250, 252-288, 294-406, 416-437
185-192: 🩺 Stability & AvailabilityNo change needed for
VACUUM INTOtarget creation.The backup code creates an empty target file before running
VACUUM INTO ?, and SQLite accepts an existing zero-length target file.> Likely an incorrect or invalid review comment.backend/src/releaseLifecycle.ts (1)
1-25: LGTM!Also applies to: 47-140, 238-238
backend/test/databaseLifecycle.test.ts (1)
31-41: LGTM!Also applies to: 650-772
docs/operations/scheduler-cache-backups.md (1)
226-231: LGTM!docs/setup/production-deploy.md (1)
50-52: LGTM!Also applies to: 90-106, 132-142, 253-282
backend/src/requestPolicy.ts (1)
37-37: LGTM!Also applies to: 143-159, 500-508
backend/src/server.ts (1)
25-25: LGTM!Also applies to: 210-222
backend/src/services/deploymentCutoverState.ts (1)
1-9: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd30d5b073
ℹ️ 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: 03f6c39848
ℹ️ 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".
2 participants
Summary
verifying, with clearer activation, readiness, rollback, and worker-stability messagesMIRA_DASHBOARD_PROJECT_ROOT, leaving/home/ubuntu/projects/mira-dashboardonly as the validated fallbackNODE_ENVplus the single root variablebuildingandverifyingstatesBehavior and regression coverage
verifyingduring the detached guardian phase and terminal jobs receive the guardian completion timestamp.language_code, allowing automatic language detection.isOk/failedrelease history. Reused snapshots no longer require the production source database to remain visible.0007leaves immutable migration0002unchanged and recreatesidx_deployment_jobs_retentionwith the current active-status predicate.verifyingwithout rollback data.Verification
bun run lint:frontend(viabun run lint)bun run build:frontend(viabun run build)bun run test:frontend— 483 passed, 0 failedbun run lint:backendbun run build:backend(viabun run build)bun run test:backend— 594 passed, 0 failedbun run test:backend:coverage— 594 passed, thresholds satisfiedbun run format:checkgit diff --checkRisk checklist
.envfiles, database dumps, or runtime state committed0007is forward-only and index-only; deploy preflight tests it against a verified database copy before activationView onlyin isolated dev; deployment jobs showVerifyingDeployment / operations
RAJOHAN_GITHUB_USERNAMEfrom Doppler; no token or credential value changedNODE_ENV=productionandMIRA_DASHBOARD_PROJECT_ROOT=/home/ubuntu/projects/mira-dashboardNotes for reviewers
restart-scheduledremains only as historical schema/test input. Runtime retention protectsbuildingandverifying, while an oldrestart-scheduledrow is eligible for cleanup.DASHBOARD_DATABASE_SCHEMA_COMPATIBILITYdescribes the schemas the running release can open, not reversible SQL. Failed initial activation can cross that window because the exact old database is restored before old code starts.MIRA_GITHUB_TOKENhandles PR list/merge/reject/deploy operations;RAJOHAN_GITHUB_TOKENis retained only because a Mira-authored PR must be approved by another GitHub account.