Skip to content

fix(cli): child exposure, explain routing and image rollback (D-0837, D-0512, D-0445, D-0804) - #1014

Open
acamarata wants to merge 4 commits into
mainfrom
dw10-cli-1
Open

acamarata wants to merge 4 commits into
mainfrom
dw10-cli-1

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Machine-invoked children now refuse exposure changes in their freshly mounted command tree, config explain applies its global options before monorepo redirection, and database image rollback removes env assignments that were originally absent.

Verified all four rows against fresh origin/main at 86627b8c. No repo-root AGENTS.md or .claude/CLAUDE.md exists. Searched open PRs by row IDs and affected filenames and checked existing remote fix branches. PR #990 only reports three rows as blocked and does not implement them. The old fix/D-* branches have no PRs; their relevant commits were reviewed and reused here, with an additional byte-for-byte env rollback assertion.

Row Result and implementation
D-0837 FIXED. After plugin mounting and a successful path pin check, rebuild the child registry and run invoke.Exposure for the command and named flags. Refuse with E421 before dispatch. Carry the exact transport in an invoker-owned environment marker so HTTP documents and NDJSON streams retain distinct exposure rules. Parent/child plugin fixture covers exposure-only upgrades, including CLI-only, missing JSON, interactive output, stream output/flags and a missing destructive confirmation gate.
D-0512 BLOCKED. cmd/commands/runner_provision.go:51 still accepts --token. Resolution requires coordinated deprecation in the plugins repository (free/ci/internal/nodes/provision/cli.go) plus recaptured help/parity cases. Editing any other repository is explicitly forbidden by this task. No partial core-only deprecation applied.
D-0445 FIXED. Prepare config explain's raw global options at the start of root PersistentPreRunE. This honors --no-monorepo before chdir and routes the default redirect notice to stderr for JSON. Full Cobra execution covers opt-out, default redirect, explicit false and repeated flags.
D-0804 FIXED. Capture actual .env key presence in switch state; missing keys mean removal and explicit empty values stay explicit. Teach reconcile to preview removals and use the same statement-aware removal when applying. Clear stale exported switch values during regeneration. Tests cover absent and empty keys, duplicate/export/multiline statements, persisted rollback state, and byte-identical env/Compose after rollback. Legacy states retain their recorded values because their original presence cannot be recovered.

Choices follow the requested resolutions: exposure is checked with the existing policy rather than introducing a new policy; explain uses its existing parser before root hooks; rollback uses map membership to retain presence without a state schema/version bump. No P7 ownership exclusions were applied.

Validation (Go 1.26.10, -mod=vendor; scratch paths abbreviated):

  • go test -mod=vendor -overlay <scratch>/baseline-overlay.json ./cmd/commands ./internal/postgres/imageswitch -run 'TestInvokePinExposureUpgrade|TestConfigExplainMonorepoExecution|TestRollbackPersistsAbsent' -count=1: FAIL as expected with origin/main's invoke_pin.go, root.go and imageswitch/run.go. Reproduces accepted changed exposure, non-JSON redirect/wrong directory, and loss of absent-key state.
  • Focused new regression tests across all four touched packages: PASS. go test -mod=vendor ./cmd/commands -run TestImageSwitchEnvApplierRestoresPresence -count=1: PASS after adding byte equality assertions.
  • go test -mod=vendor -short -timeout=300s ./cmd/commands ./internal/invoke ./internal/postgres/imageswitch ./internal/reconcile: PASS (commands 81.809s).
  • go test -mod=vendor -timeout=300s ./internal/invoke ./internal/postgres/imageswitch ./internal/reconcile: PASS, including real CLI binary invocation.
  • go vet -mod=vendor ./cmd/commands ./internal/invoke ./internal/postgres/imageswitch ./internal/reconcile: PASS.
  • go build -mod=vendor -o <scratch>/nself ./cmd/nself: PASS.
  • Built binary, synthetic root/backend .env, config explain BASE_DOMAIN --json --reveal with and without --no-monorepo: PASS, one JSON document and correct effective value in both cases.
  • GOFLAGS=-mod=vendor golangci-lint run --timeout=5m ./cmd/commands/... ./internal/invoke/... ./internal/postgres/imageswitch/... ./internal/reconcile/... (pinned v2.13.2): PASS, zero issues.
  • bash scripts/ci/gofmt-check.sh and git diff --check: PASS.

Only cli changed. No Docker, production access, version changes, releases or publishing. No hook exclusions were needed. Scratch artifacts are removed after PR creation.

… check

The child registry build in checkInvokeExposure did not mount disabled
builtin families, so pinned MCP/HTTP children whose path lived under a
disabled family (e.g. admin) failed E421 even though the parent classified
them as exposed. The same gap existed in checkInvokePin where root.Find
could not resolve the path at all. Both now reuse mountDisabledBuiltinFamilies
for the duration of the check, matching the parent registry-build steps
(D-0863). A regression test proves the disabled-builtin-family case.
@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed review findings (CHANGES 1–2): checkInvokeExposure and checkInvokePin now mount disabled builtin families for the duration of the check, matching the parent registry-build steps so pinned MCP/HTTP children under a disabled family (e.g. admin) no longer fail E421. Added TestInvokeExposureDisabledBuiltinFamily as a regression test; build passes, commit afddfb7 pushed to dw10-cli-1.

This branch has not been deployed

No deployments
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.

1 participant