Repository navigation
Conversation
… 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.
Collaborator
Author
|
Fixed review findings (CHANGES 1–2): |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/mainat86627b8c. No repo-rootAGENTS.mdor.claude/CLAUDE.mdexists. 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 oldfix/D-*branches have no PRs; their relevant commits were reviewed and reused here, with an additional byte-for-byte env rollback assertion.invoke.Exposurefor 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.cmd/commands/runner_provision.go:51still 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.--no-monorepobefore chdir and routes the default redirect notice to stderr for JSON. Full Cobra execution covers opt-out, default redirect, explicit false and repeated flags.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.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.config explain BASE_DOMAIN --json --revealwith 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.shandgit 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.