Skip to content

fix(cli): restore JSON and execution contracts (D-0980, D-0834, D-0914, D-0652) - #1016

Open
acamarata wants to merge 6 commits into
mainfrom
dw10-cli-3
Open

acamarata wants to merge 6 commits into
mainfrom
dw10-cli-3

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Restore the compatibility and execution behavior covered by D-0980, D-0834, D-0914, and D-0652. All four rows still applied on fresh origin/main (86627b8c). One commit per row.

  • D-0980 — FIXED: Restore the bare no-key license show error object and cached account status's cached: true object when NSELF_JSON_LEGACY is requested. Reuse the cached v1.4 data builder and keep the normal envelopes unchanged. Add exact-output goldens for both paths; both tests fail on the original implementations and pass after the fix.
  • D-0834 — FIXED: Pass plugin.ExecEnv(os.Environ(), compat.V15) to the dev-to-debug child, dev watcher, and Delve child, preserving NSELF_PLUGIN_DEV=1. Extend the existing exec-site source check to both command files and recognize qualified ExecEnv calls. The extended source check fails on both original files and passes after the fix.
  • D-0914 — FIXED: Export output.LegacyRequested and remove both duplicate legacyBare implementations. Assert the missing-compose E400 envelope and message, the exact legacy plugin-info exit class, one bare document, and credential redaction on stdout and stderr. Preserve admin follow-log producer failures on stderr as well as the closing envelope. Shorten argv-fixture reasons, correct the root-test comment and plugin wiki text, and document unresolved-tier error envelopes in generated OpenAPI. The admin diagnostic and OpenAPI regression checks fail against the original implementation files and pass after the fix.
  • D-0652 — FIXED: Complete command logs from Execute after Cobra returns, including failing commands and explicit state exits; record the real exit, duration, and scrubbed error. A machine-restored literal - reaches ordinary secret-value validation instead of reading stdin. Code stdin failures as E401. Document writable token-directory remediation and the sudo ownership refusal in the machine-token and error-code help, and document secret_args dash semantics.

D-0652 choices follow existing repository contracts: machineauth.Lookup explicitly promises read-only lookup, so retain that behavior and document chmod go-w ~/.nself, as permitted by the row. The secret store rejects implausibly short values for every key; retain that validation. A machine-supplied - is now interpreted literally and rejected by that existing length validation, rather than silently attempting empty stdin. Regression checks demonstrate both differences, and log tests verify success, auth failure, explicit exit 12, absence of a premature entry, and secret scrubbing.

Checked open PRs by row IDs and affected filenames, including PR #1004's file list (unrelated changes). Older fix/D-0980-legacy-bare-shapes, fix/D-0834-plugin-dev-exec-env, fix/D-0914-legacy-bare-export, and fix/D-0652-command-lifecycle branches exist but gh pr list --head <branch> --state all returns no PR for any of them. Neither repository AGENTS.md nor .claude/CLAUDE.md exists; no hook exclusion was needed.

Validation:

  • PASS: go test -mod=vendor ./cmd/commands -count=1 -timeout 5m (whole command package).
  • PASS: go test -mod=vendor ./internal/plugin -count=1 -timeout 3m.
  • PASS: go test -mod=vendor ./internal/output ./internal/builtin/admin ./internal/httpapi -count=1 -timeout 3m.
  • PASS: go test -mod=vendor ./internal/errs ./internal/machineauth ./cmd/nself -count=1 -timeout 3m.
  • PASS: go test -mod=vendor ./internal/repoqa -run 'TestOpenAPI|TestSchemasAreCurrent|TestGofmtClean|TestFileSizeBudget|TestPackageSizeBudget|TestLayeringDebtHiddenImports' -count=1 -timeout 3m.
  • PASS: focused legacy JSON, services, plugin-info, command-log lifecycle, stdin, and literal-dash regressions.
  • PASS: go vet -mod=vendor ./cmd/commands ./cmd/nself ./internal/output ./internal/plugin ./internal/builtin/admin ./internal/httpapi ./internal/errs; go build -mod=vendor -o <scratch>/nself ./cmd/nself.
  • PASS: CI-pinned golangci-lint v2.13.2 with GOTOOLCHAIN=go1.26.10, run --timeout=5m over all touched implementation packages (0 issues). The worker default Go is 1.27.3.
  • PASS: bash scripts/ci/gofmt-check.sh; git diff --check. OpenAPI regenerated with a scratch-built tools/openapigen binary and checked for drift. The package-size ratchet initially caught a four-line increase; the final implementation stays within its existing ceiling without changing the allowance.

No Docker daemon required, production access, HQ edits, release/version changes, tags, or publishing.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main and fixed the package-size ratchet: moved cachedAccountJSON from cmd/commands/account_status.go to internal/auth/storage.go (where AuthFile lives) so cmd/commands is back under its 65362-line ceiling. Regenerated .github/facts.json via make cmd-inventory to clear the stale-facts failure. Verified TestPackageSizeBudget and tools/facts pass locally; pushed with force-with-lease after rebase.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed gofmt indentation on cmd/commands/account_status.go:68; TestGofmtClean now passes (cc58b32).

@acamarata

Copy link
Copy Markdown
Collaborator Author

Regenerated .github/facts.json via make cmd-inventory to update the stale openapi_sha256 after the /v1/commands/config/plugins/info description change. Local go test ./tools/facts/... now passes; branch cc58b32 is already on dw10-cli-3.

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