Skip to content

test(shields): verify child exit before policy restore - #7506

Merged
cv merged 2 commits into
mainfrom
codex/7451-shields-child-lifecycle
Jul 26, 2026
Merged

test(shields): verify child exit before policy restore#7506
cv merged 2 commits into
mainfrom
codex/7451-shields-child-lifecycle

Conversation

@cjagwani

@cjagwani cjagwani commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

The Shields hung-owner takeover test now proves that the exact weakening child process is gone or a zombie before policy restoration. This replaces a wall-clock sentinel oracle without changing production behavior.

Related Issue

Closes #7259

Part of #7451

Changes

  • Keep the weakening child alive until takeover instead of scheduling a delayed sentinel write.
  • Identify that child by PID and process-start identity at the policy restoration boundary.
  • Preserve assertions for owner-stop ordering, descendant enumeration, transition cleanup, policy restoration, and cleanup of the exact child.

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:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: this is test-only oracle hardening; documented Shields behavior and public contracts are unchanged.
  • 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: exact refreshed diff changes only src/lib/shields/flow.test.ts; the nine-category review passes because no production, credential, authorization, dependency, configuration, logging, cryptography, or runtime behavior changes.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: src/lib/shields/flow.test.ts only strengthens coverage that the weakening child exits before policy restoration. It changes no command, output, configuration, workflow, or supported behavior. Existing runtime-controls and command-reference docs already describe this ordering. The focused CLI suite passed 22/22, npm run test:changed exited 0 with no additional tests, and Biome passed.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

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

Verification

  • PR description includes a Signed-off-by: line and every published commit appears as Verified in GitHub
  • npm run check:diff reproduced the pre-commit, commit-msg, and pre-push gates on exact refreshed head e3e9eaa45; all applicable checks passed.
  • Targeted behavior tests pass for the current change set — npx vitest run --project cli src/lib/shields/flow.test.ts: 22/22 passed; npm run test:changed: exited 0 with no additional test files; npm run typecheck:cli and Biome passed.
  • Applicable broad gate passed — not required for this one-file test-only change; focused tests and exact-diff hooks passed, with broad coverage delegated to CI.
  • 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: Charan Jagwani cjagwani@nvidia.com

Summary by CodeRabbit

  • Tests
    • Improved coverage for recovering from a stalled policy owner.
    • Added stronger verification that policy restoration completes, transition state is cleaned up, and takeover events occur in the expected order.
    • Simplified test coordination for more reliable validation of subprocess replacement and recovery behavior.

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cjagwani cjagwani self-assigned this Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Shields takeover test replaces sentinel-based timing with readiness-file coordination, process liveness checks at policy restoration, transition-file cleanup assertions, and takeover event-order validation.

Changes

Shields takeover test

Layer / File(s) Summary
Replace sentinel coordination
src/lib/shields/flow.test.ts
Embedded owner and child scripts use readiness-file coordination and long-running intervals; the owner spawn arguments and test setup are updated accordingly.
Validate deterministic takeover
src/lib/shields/flow.test.ts
A custom harness verifies policy restoration and weakening-child termination state, while assertions check transition cleanup and takeover event ordering.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The test-only changes match #7259 by replacing timer-based safety with a deterministic restore-boundary check and preserving cleanup/order assertions.
Out of Scope Changes check ✅ Passed The PR stays within the Shields test file and doesn't introduce unrelated production or extra feature changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the test-only Shields change to verify the child exits before policy restoration.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/7451-shields-child-lifecycle

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

@github-code-quality

github-code-quality Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit e3e9eaa in the codex/7451-shields-c... branch remains at 96%, unchanged from commit 5f5971c in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit e3e9eaa in the codex/7451-shields-c... branch remains at 81%, unchanged from commit 5f5971c in the main branch.

Show a code coverage summary of the most impacted files.
File main 5f5971c codex/7451-shields-c... e3e9eaa +/-
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/domain/.../connect-env.ts 89% 97% +8%

Updated July 26, 2026 00:26 UTC

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

1 optional E2E recommendation
  • shields-config

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: Carlos Villela <cvillela@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.

Approved at exact head e3e9eaa after product-scope, DCO/Verified, documentation-writer, nine-category security, unresolved-thread, exact-head advisor, focused local validation, canonical CI, and selected E2E gates passed. Canonical gate checker reports allPass=true with all 51 current checks green.

@cv
cv merged commit f3127e0 into main Jul 26, 2026
52 checks passed
@cv
cv deleted the codex/7451-shields-child-lifecycle branch July 26, 2026 00:32
@cv cv mentioned this pull request Jul 26, 2026
23 tasks
apurvvkumaria pushed a commit that referenced this pull request Jul 27, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the
exact `## v0.0.96` heading.
The entry reconciles all 90 first-parent commits since v0.0.95 with all
92 merged PRs in the live `v0.0.96` label ledger and groups the
user-visible changes by operator journey.

## Changes

- Add the parser-safe dated MDX changelog entry for v0.0.96 with
root-absolute links to the focused user guides.
- Source summary:
- [#7194](#7194) ->
`docs/changelog/2026-07-25.mdx`: Document persistent baseline network
policy exclusions and their inspection, rebuild, and snapshot behavior.
- [#7188](#7188),
[#7427](#7427), and
[#7546](#7546) ->
`docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference
routing, keyless loopback endpoints, and provider-marker isolation.
- [#7238](#7238) ->
`docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider
identifier validation before state writes or OpenShell calls, with
bounded terminal-safe rejection previews.
- [#7319](#7319),
[#7274](#7274),
[#7528](#7528),
[#7353](#7353), and
[#7560](#7560) ->
`docs/changelog/2026-07-25.mdx`: Document the managed default gateway
service, onboarding readiness, and container-runtime identity
safeguards.
- [#7349](#7349),
[#7498](#7498),
[#7406](#7406),
[#7196](#7196),
[#7559](#7559),
[#7421](#7421),
[#7510](#7510),
[#7295](#7295), and
[#7565](#7565) ->
`docs/changelog/2026-07-25.mdx`: Document gateway-scoped status,
lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and
fail-closed CLI prompt and command output.
- [#7591](#7591) ->
`docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP
tool-name discovery, its bounded and names-only contract, probe
interaction, and rebuild requirement.
- [#7305](#7305),
[#7480](#7480),
[#7471](#7471),
[#7365](#7365), and
[#7541](#7541) ->
`docs/changelog/2026-07-25.mdx`: Document installer version checks,
version-tag reporting, license guidance, WSL Ollama selection, and DGX
Station vLLM detection.
- [#7482](#7482),
[#7466](#7466),
[#7208](#7208),
[#7434](#7434), and
[#7586](#7586) ->
`docs/changelog/2026-07-25.mdx`: Document Ollama resource details,
reasoning precedence, Hermes onboarding behavior, and preserved managed
Hermes BuildKit failures.

- [#6830](#6830),
[#7492](#7492),
[#7563](#7563), and
[#7582](#7582) ->
`docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw
production lock, fixed managed-image dependencies, immutable Hermes base
adoption, and Hermes image-size reduction.
- [#7505](#7505),
[#7530](#7530),
[#7547](#7547),
[#7508](#7508),
[#7548](#7548),
[#7549](#7549),
[#7537](#7537),
[#7534](#7534),
[#7515](#7515),
[#7511](#7511),
[#7551](#7551),
[#7562](#7562),
[#7575](#7575),
[#7496](#7496),
[#7594](#7594),
[#7595](#7595), and
[#7599](#7599) ->
`docs/changelog/2026-07-25.mdx`: Summarize release validation, transient
and bounded dispatch reconciliation, exact pre-tag qualification,
identity revalidation, npm-audit retry, sharding, image reuse, timeout,
telemetry, and workflow-hardening changes.
- Reconciled without separate changelog prose:
- [#7539](#7539),
[#7526](#7526),
[#7507](#7507),
[#7506](#7506),
[#7519](#7519),
[#7516](#7516),
[#7396](#7396),
[#7254](#7254),
[#7583](#7583),
[#7596](#7596), and
[#7598](#7598): Test-harness or
fixture-only changes.
- [#7403](#7403),
[#7161](#7161),
[#6877](#6877),
[#7531](#7531),
[#7525](#7525),
[#7522](#7522),
[#7536](#7536),
[#7552](#7552),
[#7566](#7566),
[#7553](#7553),
[#7561](#7561),
[#7577](#7577),
[#7569](#7569),
[#7585](#7585),
[#7584](#7584),
[#7592](#7592),
[#7580](#7580),
[#7571](#7571),
[#7517](#7517),
[#7589](#7589),
[#7402](#7402),
[#7558](#7558),
[#7544](#7544), and
[#7601](#7601): Dependency,
internal recovery, validation, contributor-workflow, E2E optimization,
telemetry, or CI trust changes with no separate user-facing release
claim.
- [#7556](#7556),
[#7573](#7573),
[#7576](#7576), and
[#7578](#7578): Experimental
repository-maintainer conflict automation with no canonical user
documentation surface.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates dated changelog structure,
version headings, and published links.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs 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:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head
`0f5dedb47` against 90 first-parent release commits and 92 merged PRs
labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version
heading, literal CLI names, writing style, skip terms, all 20
root-absolute published links, and the accepted #7591 opt-in
authenticated discovery bounds. #7544, #7599, and #7601 remain internal
or CI-only release-ledger entries. Changelog tests passed 6/6, the docs
build passed with 0 errors and two pre-existing Fern warnings, and `npm
run check:diff` plus the final diff check passed.
- Agent: Codex Desktop documentation-writer subagent
<!-- docs-review-head-sha: 0f5dedb -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts`: 6/6 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 to this
prose-only changelog entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — the
build passed with 0 errors and 2 existing Fern warnings; the
published-route check passed.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— native changelog files use the required parser-safe MDX SPDX comment
and no frontmatter.

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Persistent network policy exclusions with consistent restore/exclusion
reporting across rebuilds/snapshots.
* Opt-in MCP tool discovery via `mcp status --tools` with bounded,
redacted authenticated traffic.
* Improved HTTPS inference switching for custom endpoints and refreshed
onboarding/model menu details.
* Refined OpenShell gateway defaults for port `8080`, including more
reliable readiness checks.
* **Bug Fixes**
* Prevent incorrect provider/model restoration after compatible-provider
update failures.
* Preserve managed MCP state after exec loss and tighten gateway/doctor
status scoping.
* **Tests**
* Stronger, fail-closed release validation with hardened
evidence/artifact handoff and bounded timeouts/retries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
@wscurran wscurran added the area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(platform): remove macOS watcher path mismatch and Shields timer race

3 participants