Skip to content

fix(registry): preserve external policy authority during recovery - #10141

Closed
apurvvkumaria wants to merge 18 commits into
codex/9833-mcp-authorityfrom
codex/9833-registry-recovery-authority
Closed

fix(registry): preserve external policy authority during recovery#10141
apurvvkumaria wants to merge 18 commits into
codex/9833-mcp-authorityfrom
codex/9833-registry-recovery-authority

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Registry recovery now preserves externally managed policy authority when it rebuilds a missing sandbox row from a resumable onboarding session. It keeps NemoClaw-managed authority unknown when session state has no durable creation receipt. Recovery also leaves an existing row's policy authority and NemoClaw attribution unchanged when session state is stale.

Related Issue

Related to #9833.

Changes

  • Restore recorded external authority with no NemoClaw-managed policy attribution when the registry row is missing.
  • Keep NemoClaw-managed authority unknown when session state has no durable creation receipt.
  • Preserve an existing row's authority and attribution instead of replacing them from stale session recovery data.
  • Cover missing-row recovery and stale-session conflicts with focused registry recovery tests.

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: Senthil Ravichandran completed the exact 0f05e6680 nine-category review and approved the change: fix(registry): preserve external policy authority during recovery #10141 (review)
  • Non-success, skipped, or missing CI check accepted by maintainer, check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable
  • Supporting evidence: Not applicable

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, command/result or justification: registry-recovery-action.test.ts passed 21/21; CLI type-check, repository hooks, and git diff --check passed.
  • Applicable broad gate passed, npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes, command/result: Not applicable; the change is isolated to registry recovery.
  • 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)

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Reviewed the complete two-file effective diff at exact commit 0f05e6680d029917cd7cb5db34c9594ace5cd48f against base 884b42e23695f8f1a0a12e9ed131575337766755. Registry recovery preserves externally managed policy authority without NemoClaw policy attribution, keeps NemoClaw-managed authority unknown without a durable creation receipt, and preserves an existing registry row's authority and attribution when session state is stale. This accepted Respect externally managed sandbox policy across lifecycle operations #9833 slice changes no public command, flag, configuration input, default, diagnostic, operator workflow, or public schema. Existing documentation treats sandboxes.json as opaque recovery state and makes no contradictory field-level recovery claim. Focused tests passed 21/21; CLI type-check, repository hooks, and git diff --check passed.
  • Agent: Codex documentation writer subagent (/root/docs_review_9176_exact)

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

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: 5cdc412b-4f12-4159-be6c-28ae260c49e5

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 56dd26b in the codex/9833-registry-... branch remains at 96%, unchanged from commit 83fd95b in the codex/9833-mcp-autho... branch.


Updated August 28, 2026 07:24 UTC

@wscurran wscurran added area: policy Network policy, egress rules, presets, or sandbox policy area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Aug 24, 2026
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for the fix. This preserves external policy authority during registry recovery, which addresses the sandbox policy lifecycle issue.


Related open issues:

…very-authority

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…very-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>

@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.

Reviewed commit 0f6614c. The complete two-file diff preserves external policy authority during registry recovery and preserves recorded authority for existing entries. The tests cover both behaviors. Issue #9833 contains the accepted product decision for this lifecycle mode.

I found no concrete unresolved defect in the changed code. DCO passes, GitHub verifies every PR commit, CodeRabbit reports no finding, and GitHub reports no review thread. I am submitting a neutral review because the accepted product decision requires security review and broader validation before approval, and the PR does not contain that complete evidence for this commit. The failed or skipped checks did not determine this review outcome.

Focused local validation could not start because this isolated worktree has no installed Vitest dependency. The PR description records 20 passing focused tests for registry-recovery-action.test.ts.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>

@senthilr-nv senthilr-nv 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.

Reviewed latest PR commit 0f05e66. The accepted #9833 scope is satisfied: recovery records externally managed authority without NemoClaw attribution, leaves managed authority unknown without a durable creation receipt, and preserves an existing registry row's authority and attribution while updating unrelated metadata. All nine security categories pass. Focused tests pass 21/21; CLI type-check and normal repository hooks pass. The terminal review cycle has no unresolved thread or actionable automated finding. CI did not determine this approval.

@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 in registry recovery, but #10514 removes durable policy authority and receipt fields from completed NemoClaw state. 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 bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants