Skip to content

fix(snapshot): preserve external policy authority - #10117

Closed
apurvvkumaria wants to merge 19 commits into
codex/9833-rebuild-authorityfrom
codex/9833-snapshot-authority
Closed

fix(snapshot): preserve external policy authority#10117
apurvvkumaria wants to merge 19 commits into
codex/9833-rebuild-authorityfrom
codex/9833-snapshot-authority

Conversation

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Summary

Snapshot, restore, and clone operations now preserve the recorded OpenShell policy authority. NemoClaw refuses policy-sensitive lifecycle work when recorded and live authority differ.

Related Issue

Part of #9833

Changes

  • Bind source and target policy authority during snapshot, restore, and clone planning.
  • Revalidate authority before target creation, policy mutation, registry publication, and success output.
  • Retain recovery receipts when an operation stops after partial work.
  • Cover managed, external, drift, rollback, and interrupted recovery paths.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — focused snapshot, restore, clone, and recovery tests passed
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c6e2c2ea-e446-4545-840d-2f949ce86a85

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 9ca8f72 in the codex/9833-snapshot-... branch remains at 96%, unchanged from commit 8f9842d in the codex/9833-rebuild-a... branch.


Updated August 28, 2026 06:32 UTC

@wscurran wscurran added area: policy Network policy, egress rules, presets, or sandbox policy area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression labels Aug 24, 2026
apurvvkumaria and others added 13 commits August 24, 2026 11:48
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…authority

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…authority

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two unresolved defects in commit 354bf07. These defects violate the accepted policy-authority lifecycle decision in #9833.

  1. src/lib/actions/sandbox/policy-authority/snapshot.ts:192-200 persists the observed authority before inspection.verifyRequirements(). If the effective policy lacks a required entry, qualification fails after the source registry changes. Issue #9833 requires this case to stop before registry mutation. Verify requirements before registry.updateSandbox() and before changing sourceEntry.

  2. src/lib/state/sandbox.ts:2481-2497 validates authority before restoreStateFile(). The openclaw.json path can then spend up to 120 seconds reading remote merge input before src/lib/state/state-file-restore.ts:157-191 performs the mutating SSH command. Authority can change during that read. Pass the validator into the state-file restore path and run it immediately before the mutating command.

The Product Scope Gate passes because issue #9833 contains an accepted maintainer decision for snapshot and restore policy authority. DCO passes, and GitHub marks every PR commit as verified. CodeRabbit skipped review because this PR targets a non-default branch. The PR Advisor produced no published synthesis because several specialists encountered provider rate limits. I did not use those unavailable gates or the failed CI gates as review reasons.

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One additional unresolved lifecycle defect applies to commit 354bf07.

src/lib/actions/sandbox/snapshot.ts:733-739 can finalize clone registration and clear pendingRouteReservation before the final authority revalidation fails. The error at lines 545-547 tells the user to retry. However, lines 1606-1608 recognize recovery only when pendingRouteReservation remains true. A retry therefore reaches the existing-destination refusal at lines 1846-1854. This leaves a Ready, registered clone without restored snapshot state, and the advised nondestructive retry cannot recover it.

The changed test at src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts:473-492 only asserts that the registry row remains after this failure. Add a second invocation that proves the same restore can recover without --force, and preserve enough recovery state for that invocation to reconcile the clone.

@copy-pr-bot

copy-pr-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Closing this PR because the accepted policy decision in #10514 supersedes #9833. This PR preserves external policy authority across snapshot, restore, and clone operations, but #10514 removes that durable authority model and makes current OpenShell policy authoritative. Replacement work is in #10515, which delivers the accepted cutover and states that no intermediate authority-stack PR is intended to merge. This closure records supersession. It does not assess the contributor or the implementation quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: policy Network policy, egress rules, presets, or sandbox policy area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants