Skip to content

fix(sandbox): probe a sandbox with no portable receipt without lock evidence - #10864

Merged
prekshivyas merged 5 commits into
mainfrom
fix/10783-connect-authority-gateway-port
Sep 3, 2026
Merged

fix(sandbox): probe a sandbox with no portable receipt without lock evidence#10864
prekshivyas merged 5 commits into
mainfrom
fix/10783-connect-authority-gateway-port

Conversation

@Dongni-Yang

@Dongni-Yang Dongni-Yang commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

nemoclaw {sandbox} connect fails at the authority stage for every sandbox on a non-default gateway port, on plain OpenClaw sandboxes, on hosts that have never used the portable profile:

... result=failed failedStage=authority
Error: Hermes portable lifecycle receipt schema-8 requalification requires the sandbox
       lifecycle lock for 'conn-iso'
connect --probe-only exit=1
status exit=0

Two state roots disagree, and only off the default port:

resolver port 8080 port 18224
lock acquired resolveNemoclawStateDir() ~/.nemoclaw/state ~/.nemoclaw/gateways/18224/state
lock checked join(defaultPortableStateDir(env), "state") ~/.nemoclaw/state ~/.nemoclaw/state

isMcpLifecycleLockHeld is an AsyncLocalStorage lookup keyed by the lock path, so on a non-default port the held lock is invisible and the requalifying reader throws. On the default port the two roots coincide, the lookup hits, and connect works — which is exactly the reported asymmetry.

A probe whose readiness is not already accepted always reaches requalifyPortableAgentSandboxAuthority (connect.ts:2509). That call is not behind the Hermes gate at connect.ts:2296, so a plain OpenClaw sandbox reaches it too, which is why the message names a Hermes portable receipt on a host that never used the portable profile.

Fix

Route a sandbox with no portable receipt directory to the classifying reader instead of the requalifying one.

The two readers are provably equal for that input: both bottom out in readHermesPortableLifecycleReceiptInternal, which returns null when the receipt directory raises ENOENTbefore it reads any of the three extra admission flags that distinguish the requalifying reader. So the lock evidence it demands buys no information, and refusing to proceed without it is pure cost.

Deliberately not done: making defaultPortableStateDir gateway-port-aware. That root is host-global on purpose — uninstall lists portable-demo-lifecycle in its shared host state entries (run-plan.ts:384). Repointing it would be a state-layout change for every existing install, not a fix.

Why the default gateway cannot change

hasHermesPortableReceiptCandidate lstats exactly the directory whose ENOENT makes the two readers agree, and returns false only on ENOENT. So candidate=false implies the readers are equal, and candidate=true leaves the old path untouched. Every other errno (EACCES, ENOTDIR, ELOOP) already threw from the reader and still does — the guard only moves which syscall raises it. A symlinked receipt directory still lstats successfully, so it stays on the requalifying path.

The second test below is the standing regression guard for this: it fails the moment the guard changes anything on port 8080.

Scope

Refs, not Closes. A sandbox that does have a genuine Hermes portable receipt still hits the same lock-evidence failure on a non-default gateway port — the guard is a no-op in that case, and the third test pins it. Closing that needs the lock key and the portable receipt root to be reconciled, which is a state-layout decision for a maintainer. This change fixes the reported case: plain OpenClaw sandboxes with no portable receipt, which is what "any sandbox on a non-default gateway port" means for anyone not running the portable profile.

Refs #10783

Test plan

New src/lib/onboard/experimental/portable-agent-lifecycle-gateway-port.test.ts, real modules, no receipt-layer mocks. GATEWAY_PORT is a module-load constant and both resolvers carry a NEMOCLAW_TEST_BASE_HOME escape hatch, so the tests stub HOME/NEMOCLAW_TEST_BASE_HOME/NEMOCLAW_TEST_STATE_DIR/NEMOCLAW_GATEWAY_PORT, vi.resetModules(), then dynamically import the real modules. The first two cases run inside a real withMcpLifecycleLockSync frame; the missing-lock case deliberately invokes requalification without that frame:

  • requalifies a sandbox that has no portable receipt on a non-default gateway portred before this change with the issue's verbatim string, green after.
  • reports the default gateway outcome for the same sandbox and state — green both ways; the default-port regression guard.
  • requires the lifecycle lock when a sandbox has a portable receipt — invokes requalification without the lock and proves the existing lock requirement remains enforced for a genuine receipt.

Also run on current origin/main: npm run validate:pr passed, and npx vitest run --project cli src/lib/onboard/experimental/portable-agent-lifecycle-gateway-port.test.ts passed (3 tests).

src/lib/onboard/experimental/ has 6 test files failing on my host with Hermes portable startup contract manifest source is unsafe. I baselined them against unmodified HEAD: 99 failed / 83 passed both with and without this change — byte-identical, so they are a pre-existing host condition and not a regression here.

Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved portable-agent sandbox requalification by selecting the appropriate classification process when a portable receipt candidate is present.
    • Sandboxes without a portable receipt candidate now follow the standard classification process.
    • Corrected requalification behavior across default and non-default gateway ports, including lifecycle-lock handling.

…vidence

connect failed at the authority stage for every sandbox on a non-default
gateway port, reporting a Hermes portable lifecycle receipt on plain
OpenClaw sandboxes and on hosts that never used the portable profile.

A probe with no accepted readiness always reaches
requalifyPortableAgentSandboxAuthority, whose reader demands the sandbox
lifecycle lock. That evidence is keyed on the host-global portable
receipt root while every acquisition keys on the per-gateway state root,
so off the default gateway the held lock is invisible and the read
fails. The two roots coincide only on the default port, which is why the
default gateway works.

Route a sandbox with no portable receipt directory to the classifying
reader instead. Its answer is already identical: the requalifying reader
returns null on that directory's ENOENT before it reads any of its extra
admission flags, so the lock evidence it asks for buys no information.

The portable receipt root stays host-global, which is deliberate:
uninstall lists portable-demo-lifecycle in its shared host state.

Refs #10783

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@github-code-quality

github-code-quality Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit e3e3e55 in the fix/10783-connect-au... branch remains at 96%, unchanged from commit b08eaa0 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit e3e3e55 in the fix/10783-connect-au... branch remains at 83%, unchanged from commit b08eaa0 in the main branch.

Show a line coverage summary of the most impacted files.
File main b08eaa0 fix/10783-connect-au... e3e3e55 +/-
src/lib/onboard...able-receipt.ts 82% 82% 0%
src/lib/onboard...uild-context.ts 75% 75% 0%
src/lib/sandbox...rce-identity.ts 82% 82% 0%
src/lib/state/m...-acquisition.ts 86% 86% 0%
src/lib/onboard...nt-lifecycle.ts 78% 80% +2%

Updated September 03, 2026 03:52 UTC

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4fc5d50f-ebdf-4fcb-a483-f9f99517743e

📥 Commits

Reviewing files that changed from the base of the PR and between b08eaa0 and e3e3e55.

📒 Files selected for processing (3)
  • src/lib/onboard/experimental/portable-agent-lifecycle-gateway-port.test.ts
  • src/lib/onboard/experimental/portable-agent-lifecycle.test.ts
  • src/lib/onboard/experimental/portable-agent-lifecycle.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/onboard/experimental/portable-agent-lifecycle.test.ts
  • src/lib/onboard/experimental/portable-agent-lifecycle.ts
  • src/lib/onboard/experimental/portable-agent-lifecycle-gateway-port.test.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The lifecycle now checks for a Hermes portable receipt candidate before choosing the requalification reader. Tests cover default and non-default gateway ports, receipt-free sandboxes, and sandboxes with portable receipts.

Changes

Portable receipt routing

Layer / File(s) Summary
Candidate-aware requalification
src/lib/onboard/experimental/portable-agent-lifecycle.ts
The lifecycle uses the requalification reader when a Hermes portable receipt candidate exists. Otherwise, it uses the standard classification reader.
Gateway-port routing validation
src/lib/onboard/experimental/portable-agent-lifecycle-gateway-port.test.ts, src/lib/onboard/experimental/portable-agent-lifecycle.test.ts
Tests mock candidate detection and validate outcomes for default and non-default gateway ports, including lifecycle-lock behavior for portable receipts.

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

Merge Risk: ⚪ Minimal · up to e3e3e

Receipt-free OpenClaw sandboxes can now complete probe-only connection checks on non-default gateway ports without entering portable-receipt requalification, while sandboxes with portable receipts retain their existing lock-protected behavior. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: probing sandboxes without portable receipts no longer requires lock evidence.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/10783-connect-authority-gateway-port

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

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit f856ab4. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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.

@prekshivyas
prekshivyas merged commit 3076188 into main Sep 3, 2026
68 of 77 checks passed
@prekshivyas
prekshivyas deleted the fix/10783-connect-authority-gateway-port branch September 3, 2026 06:33
cjagwani added a commit that referenced this pull request Sep 5, 2026
<!-- markdownlint-disable MD041 -->
## Outcome

Adds the canonical dated documentation entry for v0.0.120 and records
the release's material user-facing changes before tag planning. The
Hermes rebuild guide now also documents the fail-closed immutable-base
requirement for legacy sandboxes without an image hint.

## Reason

Release planning requires a merged `docs/changelog/2026-09-04.mdx`
containing exactly one `## v0.0.120` heading. The existing automation
draft does not contain that required changelog and does not cover the
full release scope, so this PR provides a fresh, independently reviewed
release-docs update.

### Related issues

Relates to #10919

## Changes

- Add three release-note lead paragraphs and detailed, user-facing
v0.0.120 changes with canonical documentation routes.
- Cover configuration export and doctor (#11015, #11012); Hermes
runtime, recovery, and Discord policy (#10595, #11071, #11024, #10927,
#10983, #10988, #10999, #11019, #10682); Shields retirement (#10722,
#10996); OpenShell forwarding and runtime authority (#10695, #10814,
#10815, #10810); onboarding and recovery (#10690, #10900, #11046,
#10882, #10864); inference behavior (#10956, #10910, #11070); Deep
Agents MCP projection safety (#10911, #10909); and provider-profile
validation (#10884, #10895).
- Scope the legacy Hermes immutable-base rebuild guidance to the
Hermes-rendered recovery page.

## Verification

- `npx vitest run --project integration
test/generation/check-docs-links.test.ts
test/generation/check-docs-published-routes.test.ts
test/generation/post-merge-docs.test.ts` — 3 files and 125 tests passed.
- `npm run docs` — passed with 0 errors and 5 existing Fern warnings.
- Independent documentation audit — reconciled all 71 commits in
`v0.0.119..origin/main`, validated all 29 PR links and published routes,
and found no unsupported product claims or remaining corrections.
- Normal `pre-commit`, `commit-msg`, and `pre-push` hooks — passed.
- `git diff --check` — passed.
- GitHub commit verification —
`a22fe0989fd72c7daaa9b2e7a4734a3edc069aba` is Verified with reason
`valid`.
- Secret review — the diff contains no secrets, API keys, or
credentials.

## Review notes

The existing automation draft #10919 is intentionally left untouched.
This PR supersedes its release-docs content with the complete canonical
changelog and a variant-correct Hermes recovery update.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


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

## Summary by CodeRabbit

- **Documentation**
- Added release notes covering verified configuration export, host and
gateway diagnostics, service forwarding, sandbox recovery, onboarding
safeguards, inference retries, MCP projection safety, provider setup,
and Discord runtime policy.
- Clarified sandbox rebuild behavior, including use of the
release-pinned immutable base image when required.
- Documented that rebuilds stop before modifying sandbox data when the
required image cannot be resolved or validated.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants