fix(sandbox): wait for managed gateway lease - #8262
Conversation
Wait for an active expected-exit lease before reporting SUPERVISOR_BUSY. Refs NVIDIA#7429. Signed-off-by: harjoth <harjoth.khara@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe managed gateway controller acquires the expected-exit lock before discovery and uses one recovery deadline for lock acquisition, termination, and replacement health checks. Tests cover contention, cleanup, restart, inode safety, permissions, and deadline behavior. ChangesManaged gateway recovery control
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ManagedController
participant ExpectedExitLock
participant Gateway
participant ReplacementHealth
ManagedController->>ExpectedExitLock: Acquire lock until recovery deadline
ExpectedExitLock-->>ManagedController: Return lock or SUPERVISOR_BUSY
ManagedController->>Gateway: Terminate within remaining deadline
Gateway-->>ManagedController: Termination result
ManagedController->>ReplacementHealth: Verify replacement within remaining deadline
ReplacementHealth-->>ManagedController: Health result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🤖 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 `@scripts/managed-gateway-control.py`:
- Around line 476-484: Update the retry loop around the nonblocking fcntl.flock
call so every retry remains within lock_deadline: check the deadline before
retrying, compute the remaining duration, and cap time.sleep(POLL_SECONDS) to
that duration. Preserve the existing SUPERVISOR_BUSY ControlError behavior while
ensuring no flock attempt or sleep occurs after the recovery timeout.
In `@test/managed-gateway-control.test.ts`:
- Around line 740-744: Update the timeout test around
control._publish_expected_exit_lease to verify bounded waiting, not just the
ControlError code: inject a fake monotonic clock and sleep hook or measure
elapsed time at the controller boundary, then assert polling advances through
RECOVERY_TIMEOUT_SECONDS before returning SUPERVISOR_BUSY. Preserve the existing
error-code assertion while proving the configured deadline was reached.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: abe37f77-8764-42b9-84fd-9cc6a07fed67
📒 Files selected for processing (3)
docs/manage-sandboxes/gateway-lifecycle-control.mdxscripts/managed-gateway-control.pytest/managed-gateway-control.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
5 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Keep expected-exit lock polling within the recovery deadline. Signed-off-by: harjoth <harjoth.khara@gmail.com>
wscurran
added
area: cli
|
✨ Thanks for the fix. This resolves the controller contention boundary for consecutive Hermes policy-remove calls. Related open issues: Related open PRs: Related open issues: |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Security review for the current branch revision
Verification on
The branch now includes current main. This incorporates the merged managed-image privilege-transition fix that addresses the earlier Hermes image-build failure. |
|
I reviewed the current fork diff and the trusted E2E risk plan. The changes are limited to bounded managed-gateway lease waiting, focused tests, and documentation, and the selected protected multi-architecture startup lane is appropriate. I authorized the trusted controller to run that plan; fork code does not receive repository credentials. I will reconcile the resulting evidence before merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
The authorized protected E2E plan has completed successfully. All 41 current checks pass, including the selected managed-image multi-architecture startup lane, aggregate CI, both advisor lanes, CodeQL, DCO, documentation receipt, and commit verification. There are no unresolved review threads. An independent approval is the only remaining merge gate. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/managed-gateway-control-locking.test.ts (2)
104-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting a clock bound instead of an exact fake-clock total.
lock_timeout_resultrecords the exact accumulated fake-clock value and line 302 asserts it equals0.01. This pins the test to the internal retry-sleep schedule of_acquire_expected_exit_lock. If the implementation changes its retry interval or adds one extra sleep before the deadline check, the total overshoots0.01and this test fails even though the observable contract (SUPERVISOR_BUSYraised at or after the deadline) is unchanged.Assert the error code exactly and the clock as a lower bound. This keeps the deadline-enforcement claim and removes the schedule coupling.
♻️ Proposed change to assert a bound
- lock_timeout_result = [lock_timeout, timeout_clock[0]] + lock_timeout_result = [ + lock_timeout, + timeout_clock[0] >= control.RECOVERY_TIMEOUT_SECONDS, + ]Then update the Vitest assertion:
- lock_timeout: ["SUPERVISOR_BUSY", 0.01], + lock_timeout: ["SUPERVISOR_BUSY", true],Note that
control.RECOVERY_TIMEOUT_SECONDSis restored at line 122, so capture the0.01value before thefinallyblock if you use this form.As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 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 `@test/managed-gateway-control-locking.test.ts` around lines 104 - 124, Update the assertions for lock_timeout_result to require the exact SUPERVISOR_BUSY error code while asserting the fake clock is at least the configured recovery timeout, rather than matching an exact total. Capture the 0.01 timeout value before the finally block restores RECOVERY_TIMEOUT_SECONDS, and use that value in the lower-bound assertion.Source: Path instructions
262-266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the first wait expectation from
STOP_GRACE_SECONDS.The first wait is
min(STOP_GRACE_SECONDS, 3.0), so[3, 0]depends on the current value5.0. Use a deadline above both grace values and advance the fake clock to that deadline, or derive the first expected value fromcontrol.STOP_GRACE_SECONDS. Keep the0assertion because it proves the shared deadline clamps the kill wait.🤖 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 `@test/managed-gateway-control-locking.test.ts` around lines 262 - 266, Update the test around record_deadline_wait so the first wait expectation is independent of the current STOP_GRACE_SECONDS value: either set the fake deadline above both grace values and advance deadline_clock to it, or derive the first expected wait from control.STOP_GRACE_SECONDS. Preserve the 0-second second-wait assertion to verify the shared deadline clamps the kill wait.
🤖 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 `@test/managed-gateway-control-locking.test.ts`:
- Around line 295-299: Update the spawnSync invocation in the locking harness
test to include an explicit test-scale timeout and killSignal. After spawning,
assert that result.error is not a timeout error before asserting result.status
is 0, so a hung child produces a clear failure while normal harness assertion
failures remain distinguishable.
---
Nitpick comments:
In `@test/managed-gateway-control-locking.test.ts`:
- Around line 104-124: Update the assertions for lock_timeout_result to require
the exact SUPERVISOR_BUSY error code while asserting the fake clock is at least
the configured recovery timeout, rather than matching an exact total. Capture
the 0.01 timeout value before the finally block restores
RECOVERY_TIMEOUT_SECONDS, and use that value in the lower-bound assertion.
- Around line 262-266: Update the test around record_deadline_wait so the first
wait expectation is independent of the current STOP_GRACE_SECONDS value: either
set the fake deadline above both grace values and advance deadline_clock to it,
or derive the first expected wait from control.STOP_GRACE_SECONDS. Preserve the
0-second second-wait assertion to verify the shared deadline clamps the kill
wait.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f7e76603-27a0-48ae-b30a-19d1ebd92403
📒 Files selected for processing (4)
docs/manage-sandboxes/gateway-lifecycle-control.mdxscripts/managed-gateway-control.pytest/managed-gateway-control-locking.test.tstest/managed-gateway-control.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/manage-sandboxes/gateway-lifecycle-control.mdx
- test/managed-gateway-control.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Security review — exact head 50eac324c3afd345f697851e23004ab48571e246 against base c176af5f08595ed748b4eeac8988c554e180183e: PASS with no findings.
- Secrets and credentials — PASS. No credential material, secret value, credential source, or sensitive logging changes.
- Input validation and injection resistance — PASS. No new untrusted parser or command interpolation. Runtime files retain descriptor-relative validation for links, ownership, modes, process identity, and inode replacement.
- Authentication and authorization — PASS. Mutating requests acquire the root-owned expected-exit lock before process inspection; marker publication remains bound to gateway and live-controller identity.
- Dependencies and supply chain — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
- Error handling and information exposure — PASS. Lock timeout returns
SUPERVISOR_BUSY, publishes no marker, and closes descriptors. The shared recovery deadline is rechecked before SIGKILL; exhausted operations send no later signal. - Cryptography and data protection — PASS / not applicable. No cryptographic behavior changes. The lock remains mode
0600, the marker mode0444, and both require the trusted runtime owner and a single link. - Configuration and infrastructure — PASS. Lifecycle mutation is serialized without broadening network policy, ports, users, privileges, capabilities, or supported configuration.
- Security testing — PASS. Exact focused validation passes 3 files and 8 tests for contention, deadline enforcement, marker isolation, descriptor cleanup, refreshed process discovery, deadline propagation, and SIGTERM-only exhaustion.
npm run check:diff, strict docs validation, and diff hygiene pass. - System security — PASS. Process identity is refreshed after contention, pidfd signaling remains in place, and no signal occurs after the recovery deadline. Base PR #7894's health-streak reset is complementary post-recovery accounting and does not change expected-exit authorization or serialization.
The signed merges were conflict-free and preserve the effective five-file patch byte-for-byte (raw SHA-256 d21036f50ae0af21e9fed2b13167babb0f3c14d4e691b4b0b1accb082b7d3c9d; stable patch ID 84297edbceeeba90c29fb17b920290f599720578). Base PR #8371 changes internal vLLM model acquisition behind a preserved API and does not alter managed-gateway lifecycle state. Product scope remains established by accepted issue #7429. Fresh CI, protected E2E, documentation receipt, and independent post-push approval remain mandatory.
|
CI disposition for the current branch revision:
No PR code change is indicated by the advisor failure. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head a22e40f493e34e442780551f149df170fc1f0359 against base 3b208d79e5d3bda4183704145ee5c28d79876ae1: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. No credential material, secret value, credential source, or sensitive logging changes.
-
Input Validation and Data Sanitization — PASS. No new untrusted parser or command interpolation. Runtime files retain descriptor-relative validation for links, ownership, modes, process identity, and inode replacement.
-
Authentication and Authorization — PASS. Mutating requests acquire the root-owned expected-exit lock before process inspection; marker publication remains bound to gateway and live-controller identity.
-
Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
-
Error Handling and Logging — PASS. Lock timeout returns
SUPERVISOR_BUSY, publishes no marker, and closes descriptors. The shared recovery deadline is rechecked before SIGKILL; exhausted operations send no later signal. -
Cryptography and Data Protection — PASS / not applicable. No cryptographic behavior changes. The lock remains mode
0600, the marker mode0444, and both require the trusted runtime owner and a single link. -
Configuration and Security Headers — PASS. Lifecycle mutation is serialized without broadening network policy, ports, users, privileges, capabilities, or supported configuration.
-
Security Testing — PASS. Exact focused validation passes 3 files and 8 tests for contention, deadline enforcement, marker isolation, descriptor cleanup, refreshed process discovery, deadline propagation, and SIGTERM-only exhaustion.
npm run check:diff, strict docs validation, and diff hygiene pass. -
System Security — PASS. Process identity is refreshed after contention, pidfd signaling remains in place, and no signal occurs after the recovery deadline. Base PR #7894's health-streak reset is complementary post-recovery accounting and does not change expected-exit authorization or serialization.
Files Reviewed
docs/manage-sandboxes/gateway-lifecycle-control.mdxscripts/managed-gateway-control.pytest/managed-gateway-control-locking.test.tstest/managed-gateway-control.test.tstest/openclaw-managed-restart-respawn.test.ts
Provenance and Required Gates
The signed refresh merge is conflict-free and preserves the effective 5-file patch byte-for-byte (raw SHA-256 d21036f50ae0af21e9fed2b13167babb0f3c14d4e691b4b0b1accb082b7d3c9d; stable patch ID 84297edbceeeba90c29fb17b920290f599720578). Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.
Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.
|
CI disposition after the latest branch refresh:\n\n- The newest refresh commit is signed off and GitHub-verified, and the fresh DCO check passed. It merges the current base into the branch. The PR patch retains the same stable patch identity, with no change to the five PR-owned files.\n- Advisor run 31038118964 was on attempt 1. GPT-5.6 completed the review with no blockers, warnings, or suggestions, but its final same-session synthesis was incomplete. Nemotron received an external provider 429 response before completing analysis.\n- I used the single permitted failed-job retry. Attempt 2 completed successfully: Nemotron reports no blockers, warnings, or suggestions, and all nine security categories passed. The retry allowance for this branch revision is consumed; no further retry will be used.\n- Fresh documentation review, Fern preview, DCO, commit lint, macOS and WSL E2E, security checks, and managed-image startup checks have passed. The remaining CI rerun and protected E2E gate are still in progress.\n- Protected E2E evidence from the previous branch refresh passed on both architectures, but it is superseded by this refresh and cannot satisfy the current gate.\n\nNo merge is appropriate until every current required gate finishes. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head f0ff49aada08a1e17e3d636c9d52e6db1dcebfc6 against base c6713c9dd17a9682654713ccb6829b8714f1b307: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. No credential material, secret value, credential source, or sensitive logging changes.
-
Input Validation and Data Sanitization — PASS. No new untrusted parser or command interpolation. Runtime files retain descriptor-relative validation for links, ownership, modes, process identity, and inode replacement.
-
Authentication and Authorization — PASS. Mutating requests acquire the root-owned expected-exit lock before process inspection; marker publication remains bound to gateway and live-controller identity.
-
Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
-
Error Handling and Logging — PASS. Lock timeout returns
SUPERVISOR_BUSY, publishes no marker, and closes descriptors. The shared recovery deadline is rechecked before SIGKILL; exhausted operations send no later signal. -
Cryptography and Data Protection — PASS / not applicable. No cryptographic behavior changes. The lock remains mode
0600, the marker mode0444, and both require the trusted runtime owner and a single link. -
Configuration and Security Headers — PASS. Lifecycle mutation is serialized without broadening network policy, ports, users, privileges, capabilities, or supported configuration.
-
Security Testing — PASS. Exact focused validation passes 3 files and 8 tests for contention, deadline enforcement, marker isolation, descriptor cleanup, refreshed process discovery, deadline propagation, and SIGTERM-only exhaustion.
npm run check:diff, strict docs validation, and diff hygiene pass. -
System Security — PASS. Process identity is refreshed after contention, pidfd signaling remains in place, and no signal occurs after the recovery deadline. Base PR #7894's health-streak reset is complementary post-recovery accounting and does not change expected-exit authorization or serialization.
Files Reviewed
docs/manage-sandboxes/gateway-lifecycle-control.mdxscripts/managed-gateway-control.pytest/managed-gateway-control-locking.test.tstest/managed-gateway-control.test.tstest/openclaw-managed-restart-respawn.test.ts
Provenance and Required Gates
The signed refresh merge is conflict-free and preserves the effective 5-file patch byte-for-byte (raw SHA-256 d21036f50ae0af21e9fed2b13167babb0f3c14d4e691b4b0b1accb082b7d3c9d; stable patch ID 84297edbceeeba90c29fb17b920290f599720578). Base PR #8383 adds only the v0.0.103 release-note page and does not alter any effective PR file or reviewed runtime trust boundary. Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.
Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.
|
Current CI is blocked by an unrelated timeout in |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security revalidation — exact head e3f858d526c97de83321b02279d48409464f5835 against current base 1608281462923ff8282151c7be1a109ee262b4e9: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged from the prior exact review. The base refresh adds no overlapping secret, credential, environment, or sensitive-logging path.
-
Input Validation and Data Sanitization — PASS. The reviewed validation and command/path construction are unchanged. The new base delta does not alter an effective PR file.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary changed in the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PR #8387 adds verified llama.cpp GGUF acquisition in eight disjoint files and does not change this PR's dependency surface.
-
Error Handling and Logging — PASS. The reviewed fail-closed behavior and diagnostics are unchanged; the base refresh introduces no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. The PR's reviewed cryptographic and protected-data behavior is unchanged. No new cross-patch data flow exists.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries.
-
Security Testing — PASS. Raw patch SHA-256 remains
d21036f50ae0af21e9fed2b13167babb0f3c14d4e691b4b0b1accb082b7d3c9dand stable patch ID remains84297edbceeeba90c29fb17b920290f599720578. Documentation writer revalidation passed at this exact head; the existing PR documentation remains accurate. Prior targeted test evidence remains applicable; fresh exact-head GitHub Actions is authoritative. -
System Security — PASS. The signed refresh is conflict-free, preserves the reviewed patch byte-for-byte, and introduces no new runtime trust-boundary interaction with the disjoint base delta.
Provenance and Required Gates
This exact-head revalidation supplements the full prior nine-category review at #8262 (review). It applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa.
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head 651a19cd4ac3a02da4a18e93ac599741590248f2 against current base b148bc13ee53b7a1f612d07e42275ec732e71877: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged and adds no secret, credential source, environment exposure, or sensitive logging. The refreshed base introduces no overlapping credential path.
-
Input Validation and Data Sanitization — PASS. Reviewed validation, command construction, and path handling remain unchanged. The portable-profile base delta has no exact file overlap with this PR.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary is widened by the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PRs #8333 and #8376 add disjoint E2E artifact restoration and portable experimental onboarding behavior.
-
Error Handling and Logging — PASS. Reviewed fail-closed behavior and diagnostics remain unchanged; the signed integrations add no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. Reviewed cryptographic and protected-data behavior remains unchanged, with no new cross-patch data flow.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries. The hidden portable profile does not bypass this PR's authority checks.
-
Security Testing — PASS. Raw patch SHA-256 remains
d21036f50ae0af21e9fed2b13167babb0f3c14d4e691b4b0b1accb082b7d3c9dand stable patch ID remains84297edbceeeba90c29fb17b920290f599720578. The prior exact lifecycle-control locking and deadline evidence remains applicable. Exact-head documentation writer revalidation passed; fresh GitHub Actions is authoritative. -
System Security — PASS. Both signed refreshes are conflict-free, preserve the effective patch byte-for-byte, and introduce no new runtime trust-boundary interaction. For #7853 specifically, the portable profile supplies onboarding defaults but neither stages messaging credentials nor bypasses registry-driven credential-drift validation.
Provenance and Required Gates
The exact head preserves contributor history through signed merge commits and applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa. This supplements the full prior nine-category review at #8262 (review).
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Current revision Focused validation exposed four PR-related failures before CI reached them:
The follow-up is test-only. It restores test isolation, snapshots observations at the assertion boundary, wires the simulated clock to the controller, and keeps the stubs compatible with the public recovery flow. Validation now passes 4 focused files and 10 tests. The CLI build and type check pass, the documentation build reports 0 errors and the 2 existing Fern warnings, and commit/push hooks pass. Compliance and security disposition:
After reviewing the fork scope, I authorized the repository standard CI and protected E2E workflows to run. Fresh CI, advisor, CodeQL, documentation review, and protected E2E results are still required. The merge freeze remains in effect, so no merge or main-branch write will occur. |
|
Final validation receipt for branch revision
Validation is complete. The merge freeze remains active, so this PR is intentionally not being merged or queued. |
8 participants
Summary
For
recoverandgateway restart, the managed gateway controller now acquires the expected-exit lock before it inspects the supervisor or gateway. Lock acquisition, gateway termination, and replacement health share one recovery deadline. If lock acquisition reaches that deadline, the controller returnsSUPERVISOR_BUSYwithout publishing an expected-exit marker.This serializes consecutive Hermes
policy-removecalls that reach the controller while gateway recovery is active.Related Issue
Refs #7429
PR #7498 addressed the separate Hermes version probe for
upgrade-sandboxes --check. This PR addresses the remaining consecutivepolicy-removebehavior and does not close the issue.Changes
Type of Change
Quality Gates
Documentation Writer Review
docs-updatedSUPERVISOR_BUSYbehavior, marker cleanup, and existing retry guidance with no findings. The follow-up commit changes only test-harness isolation and clock wiring. The docs build passed with 0 errors and 2 existing warnings.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHub — all 32 commits appear asVerified.pre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable — normal commit and push hooks passed for the current follow-up.npm run docsbuilds without warnings (doc changes only) — no local docs build ran; GitHub Actions is authoritative.Sensitive-Path Security Review
Independent review found no security finding for PR commit
8b52f32ac37b4dc3e2832d8948c206ce9d0db1c1against base SHAdb31c286129e878c3356eed49f76ab259561e47e.finallyblock.The documentation records the existing shared-UID provenance and mutable-configuration TOCTOU limitations. Required GitHub Actions checks remain separate merge requirements.
GitHub Actions
GitHub Actions is authoritative for PR commit
8b52f32ac37b4dc3e2832d8948c206ce9d0db1c1against base SHAdb31c286129e878c3356eed49f76ab259561e47e.Required checks are running.
No check has a maintainer waiver.
Signed-off-by: harjoth harjoth.khara@gmail.com
Summary by CodeRabbit