fix(onboard): allow packaged service teardown fallback - #8239
Conversation
📝 WalkthroughWalkthroughThe change allows full uninstall teardown to accept a packaged-service authority that resolves as standalone after service removal. Credential mutation and other authority changes remain rejected. Tests and lifecycle documentation cover both outcomes. ChangesGateway authority teardown
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
4004db3 to
6e2814f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/deployment/gateway-lifecycle-authority.mdx (1)
154-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the section heading to include full uninstall.
This section now documents both managed authority rebuilds and full uninstall. The current heading names only rebuilds. Rename it to
### Migrate Managed Authority During Rebuild and Full Uninstall, or split the two exceptions into separate sections.As per coding guidelines, documentation must be clear, accurate, and task-oriented.
🤖 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 `@docs/deployment/gateway-lifecycle-authority.mdx` around lines 154 - 160, Update the section heading associated with “Migrate Managed Authority During Rebuild” to explicitly include “Full Uninstall,” reflecting that the section documents both exception cases. Preserve the existing content and heading level.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@docs/deployment/gateway-lifecycle-authority.mdx`:
- Around line 154-160: Update the section heading associated with “Migrate
Managed Authority During Rebuild” to explicitly include “Full Uninstall,”
reflecting that the section documents both exception cases. Preserve the
existing content and heading level.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d9957e67-9cb4-4183-be33-e596984b1903
📒 Files selected for processing (1)
docs/deployment/gateway-lifecycle-authority.mdx
6e2814f to
b77cfa2
Compare
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 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. |
|
✨ Thanks for the fix. This resolves the uninstall failure when the packaged gateway service has already been removed. Related open issues: Related open issues: |
|
Maintainer follow-up for the current PR revision:
Remaining blockers:
No merge action will occur until every repository gate passes. |
|
The selected E2E run passed. Both advisor lanes failed again on the already-permitted second attempt because their inference service returned HTTP 429 during analysis; this is external rate limiting, not a PR test or implementation failure. I will not issue a third rerun without new evidence. The documented advisor-service failure and independent approval remain the merge blockers; there are no unresolved review threads. |
Summary
nemohermes uninstall --yesno longer fails when teardown revalidates a default NemoClaw-managed gateway after the packaged gateway service has already been removed. Uninstall opts into that expectedpackaged-servicetostandaloneresolution, while other teardown callers and credential mutation still use the default fail-closed authority check.Related Issue
Fixes #8215
Changes
packaged-serviceand the current default resolution isstandalonebecause the packaged service has already been removed.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/deployment/gateway-lifecycle-authority.mdx; writing rules and documentation style reviewed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project cli src/lib/onboard/gateway-teardown-authority.test.ts src/lib/onboard/gateway-authority-migration.test.ts src/lib/actions/uninstall/run-plan-gateway-service.test.ts src/lib/actions/uninstall/run-plan.test.tspassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run check:diffpassed;npm run docspassed with Fern reporting 0 errors and 2 unrelated warnings.fern check --warningsreported an unauthenticated redirects check and a global light-mode accent color contrast warning.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Vinay Bhagavath bhagavathvinay@gmail.com
Summary by CodeRabbit
Bug Fixes
Documentation