feat(service): add restore command for the private state backup roundtrip - #132
Conversation
…trip service-state-backup.mjs gains a restore mode that validates the snapshot exactly like verify, refuses any pre-existing canonical root (clean-clone restore only), stages beside the target repo, and swaps each root atomically via rename with full rollback on failure. The service-engine test roundtrip now restores through the product command instead of cpSync, and new tests cover missing/malformed input, existing-target refusal, byte+mode parity, re-backup equivalence, and an injected mid-swap interruption that fails closed and preserves the target.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reached
Next review available in: 9 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: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (2)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![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: 018ac8f138
ℹ️ 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".
| for (const root of roots) { | ||
| const target = join(repoRoot, root) | ||
| mkdirSync(dirname(target), {recursive: true}) | ||
| renameSync(join(staging, root), target) |
There was a problem hiding this comment.
If the restore process is killed or the machine loses power after one renameSync but before all roots are swapped, the catch block never runs, leaving a partial mix of restored and absent canonical roots. A subsequent restore then refuses to run because the first restored root already exists, so recovery requires unsafe manual cleanup; the injected exception test does not cover this termination scenario. Use a durable restore journal/recovery path or a genuinely atomic commit boundary for the multi-root swap.
AGENTS.md reference: AGENTS.md:L2-L3
Useful? React with 👍 / 👎.
| const repoRoot = realpathSync(resolve(repoPath)) | ||
| const manifest = validateManifest(JSON.parse(readFileSync(join(verified.input, MANIFEST), "utf8"))) | ||
| const roots = [...RECORD_ROOTS, OUTPUT_ROOT].filter(root => verified.roots.includes(root)) | ||
| const release = acquireLock(queuePaths(repoRoot).lockDir) |
There was a problem hiding this comment.
When a verified restore later fails—for example because clients/ already exists—acquireLock has already created runs/service-engine/ for the queue lock, and releasing the lock removes only .lock. The command therefore mutates a target it reports as rejected, violating the zero-write/fail-closed restore behavior; the new tests miss this because snap() records files but ignores empty directories. Acquire the lock without leaving new parents behind, or remove parents created solely for the failed restore.
AGENTS.md reference: AGENTS.md:L2-L3
Useful? React with 👍 / 👎.
…le with main after PRs #135/#104/#99/#112/#100/#105) The PR #130 branch had already been reconciled with origin/main through c617364 (PRs #131 + #133/#134/#132/#116). Since then origin/main moved to d4f3ef4 with six more merges (#135 #104 #99 #112 #100 #105). Pull them in so PR #130 can be reviewed against current main without weakening the capture/rollback/reverification implementation or the PR #131 fail-closed missing-secret gate. The merge resolved cleanly: - PR #130 capture/rollback logic (publish-public-site.mjs canonical_deployment, rollbackTo, restored identity re-verify, accept-again) is intact. - PR #130 test file (scripts/test-pages-release.mjs) is intact and wired into the npm ci/test scripts. - PR #131 fail-loud missing-secret workflow step in .github/workflows/deploy-public-site.yml is preserved byte-for-byte. - The new origin/main changes touch operator surfaces and the deploy bundle prep; none of them modify the publish-public-site.mjs rollback contract or the Pages token gate, so there were no conflict markers.
…on main (#130/#132/#133) (#197) Record the decision for the inbox backlog line (tinystudio-in priority=normal AUG13-16). Investigation shows every item is already in main: - truthful-reversible-pages-release: LANDED via PR #130 (ea9eab2); release lane fails truthfully on missing credentials and rolls back bad uploads (scripts/test-pages-release.mjs passes 38/38). - service-backup-restore-roundtrip: LANDED via PR #132 (4a7f442); restore mode in service-state-backup.mjs. - operator-checks-strict: LANDED via PR #133 (c617364); named operator checks fail on blocked readiness with advisory aliases explicitly named. No code change required; this closes the backlog by recording the decision.
1 participant
Provide and behaviorally test the restore half of the advertised backup roundtrip.
Changes
scripts/service-state-backup.mjs: newrestoremode--inputexactly likeverify(outside repo, 0700 root, manifest + byte/mode match)scripts/test-service-engine.mjs:cpSyncValidation
node scripts/test-service-engine.mjs→{"status":"passed","checks":38}git diff --checkcleanNo package.json changes (owned by the strict-check packet).