test(cli): preserve gateway teardown mock exports - #11327
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughThe gateway teardown authority mock now preserves the original module exports while overriding ChangesGateway teardown test
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The gateway teardown runtime-evidence test now retains adjacent module exports while mocking its resolver, avoiding the prior missing-export test failure. No current merge-readiness risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
<!-- markdownlint-disable MD041 --> ## Outcome Managed-provider recovery and snapshot cloning now use the typed OpenShell provider adapter for provider reads, profile imports, and provider creation. Existing recovery, reconciliation, and fail-closed behavior stays intact; destructive provider cleanup remains in Slice 8. ## Reason Issue #9806 is moving managed-provider consumers behind one typed protocol boundary. Recovery and clone paths still owned raw OpenShell provider commands and duplicated diagnostic parsing, which left those flows outside the adapter contract completed in the earlier slices. ### Related issues Part of #9806 ## Changes - Add a managed-provider adapter factory so rebuild and snapshot consumers share the selected-gateway target and typed CLI adapter. - Route rebuild provider registration and recovered credential metadata checks through typed provider reads, preserving selected runtime authority at the delete edge. - Route snapshot clone inspection, profile import, and provider creation through the adapter while retaining exact reconciliation and ownership checks. - Keep rollback inspection and deletion on the existing raw path for Slice 8. - Preserve bounded, suppressed provider-create diagnostics in the CLI adapter and protect the behavior in its unit test. ## Verification - Focused CLI recovery, snapshot, gateway teardown, and provider-export contracts - 208 tests passed on the final source revision. - Focused integration recovery, reconciliation, and provider-export contracts - 3 tests passed and 2 platform-inapplicable tests skipped on the final source revision. - Current-main dashboard, WeChat install, and managed-image bundle contracts - 69 tests passed on the final source revision. - `npx vitest run --project package-contract test/package-contract/cli/credentials-cli-command.test.ts` - 26 tests passed on the final source revision. - `npx vitest run --project integration test/agents/hermes/hermes-image-build-probes.test.ts` - 57 tests passed on `73eacfee5c`. - `npm run build:cli` - passed. - `npm --prefix nemoclaw run build` - passed. - `npm run typecheck:cli -- --pretty false` - passed. - `npm run validate:pr` - passed on `73eacfee5c` against canonical main `f893b8359e`. - `npm run review:local` - unavailable because the temporary OpenShell review gateway refused connections; no local Advisor result is claimed. - The diff contains no secrets, API keys, or credentials. ## Review notes Slice 6 merged in #11234. This revision integrates canonical `main` through `f893b8359e` and leaves only the 20-file Slice 7 recovery, snapshot, and adapter-ownership documentation diff. Mainline PRs #11314, #11327, #11298, #11332, and #11338 resolved the inherited cache-seed, gateway teardown mock, Hermes image-pointer and probe-digest, dashboard, WeChat, and image-bundle contract failures observed on earlier revisions. The complete CodeRabbit review for `be4b1de3e1` reported three related cutover findings. This revision addresses all three by injecting the adapter in the preparation proof, documenting the bounded raw cleanup bridge and Slice 8 exit criteria, and sharing the adapter credential-name predicate with clone preflight. The resulting CI fixture fallout was repaired across the complete affected root-cause group. The incremental CodeRabbit review for `49ce5d9032` reported two comments. The adapter already provides the sole credential-name validator, so that comment required no change. The missing-provider process fixture now asserts its status and exact stderr contract. The incremental CodeRabbit review for `cb788fe0f0` reported an orphaned Hermes handoff on a pre-delete bailout. Canonical main commit `7c54bc084a` introduced every reported line in #10780. Slice 7 changes only the later provider inspection await, so this finding is inherited and requires no Slice 7 change. The complete PR Review Advisor run 34367204068 reviewed commit `cc647e8`. Seven specialists found no required change. The architecture specialist requested typed destructive cleanup, which is the accepted Slice 8 boundary and is not part of Slice 7. The reduction specialist suggested removing the managed-provider facade. The facade remains because three current recovery consumers use it to bind the selected-gateway policy and Slice 8 is its next current consumer. The complete exact-head PR Review Advisor run 34405962090 reviewed `2cfcae6b38`. Its documentation specialist found the OpenShell adapter README still described the former ownership model; `73eacfee5c` applies the requested paragraph-only correction. The architecture, migration, and reduction specialists each requested the same typed destructive-cleanup cutover. That work is the accepted #9806 Slice 8 boundary and is implemented in stacked PR #11328, so it does not require a Slice 7 production-code change. The other five specialists found no required change. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Improved reliability of sandbox rebuilds by completing provider and credential checks before proceeding. - Enhanced managed clone setup with more consistent provider creation, profile import, rollback, and cleanup handling. - Added clearer handling for missing, incompatible, or invalid gateway provider configurations. - Standardized credential-name validation, including supported length limits. - Improved recovery behavior and provider metadata reporting during rebuild and snapshot workflows. - **Documentation** - Updated adapter documentation to clarify provider inspection and managed recovery coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Outcome
The gateway runtime-evidence test now preserves the production teardown module's exports while overriding its authority resolver. The test reaches the cleanup behavior it owns instead of failing when the production module adds another export used by
destroy-gateway.Reason
Current
maindeterministically fails this test because its full module mock omitsremoveGatewayRegistrationWithPolicy.Changes
resolveGatewayTeardownAuthorityoverride.Verification
npx vitest run --project cli src/lib/actions/sandbox/destroy-gateway-runtime-evidence.test.ts— 1/1 passed after rebasing onto currentmain.npm run check:diff— passed before commit; normal commit hooks and pre-push CLI typecheck passed.9823259eccddc43c3d520cceae03035da87a30e0is Verified (valid).Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit