feat(wfctl): interactive + non-interactive secrets setup wizard (PR2/7)#801
Merged
Conversation
…put/Confirm) Adds cmd/wfctl/internal/prompt with five files: - prompt.go: ErrNotInteractive sentinel + Item type + isTTY helper - select.go: single-choice list via bubbletea model - multiselect.go: multi-choice list with space-to-toggle - input.go: text input using bubbles/v2 textinput (EchoPassword when masked) - confirm.go: y/n question with configurable default - prompt_test.go: all constructors return ErrNotInteractive in non-TTY env Adds charm.land/bubbles/v2 v2.0.0 to go.mod (pairs with bubbletea/v2 v2.0.2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…red report) Adds cmd/wfctl/secrets_setup_engine.go: generic runSetupEngine[D] that accepts an injected selector (which declared secrets to set), valuer (where to get the value), and audit callback. - Queries per-secret Check() status before calling selector. - Selector-excluded secrets go to report.Skipped. - Valuer returning provided=false skips without error. - Set errors go to report.Failed; stopOnErr=true returns on first failure. - audit callback is NEVER passed the secret value. Full table of test cases in secrets_setup_engine_test.go with engineTestProvider (in-memory fake SecretsProvider, distinct from existing fakeSecretsProvider). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y/--skip-existing) - secrets_setup_noninteractive.go: nonInteractiveSetupArgs struct + runSecretsSetupNonInteractive/Ctx: loads config, resolves store, builds selector (--only/--skip-existing) + valuer (--from-env > stdin-KV > --secret literals), routes through runSetupEngine, prints summary. Never logs values. - secrets_setup.go: adds --non-interactive, --from-env, --secret (repeatable), --only, --skip-existing, --store flags; auto-routes to non-interactive path when stdin is not a TTY; interactive path gains masked input for sensitive names and --store override. - infra_secrets.go: adds 'file' provider case to resolveSecretsProvider (NewFileProvider, creates dir if missing). - secrets_setup_noninteractive_test.go: --from-env reads $NAME; --only restricts; --secret B=v overrides; --skip-existing skips set secrets; missing value → named error (no hang); provider receives exactly the expected Sets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- secrets_audit.go: writeSecretsAuditRecord appends a JSONL line to
${XDG_STATE_HOME:-~/.local/state}/wfctl/plugins/wfctl/secrets-audit.jsonl.
Fields: {ts, secret_name, store, scope, actor}. Value is NEVER written.
resolveSetupStoreName implements 5-priority store resolution:
1. --store flag 2. defaultStore 3. exactly-one secretStores entry
4. interactive prompt (returns ""/nil) 5. error (non-interactive)
- secrets_audit_test.go: (a) audit table — single JSONL line written,
fields present, forbidden value absent, appends on second write.
(b) store-resolver table — all 5 cases.
- secrets_setup_noninteractive.go: wires writeSecretsAuditRecord after
each successful Set (best-effort, error discarded); uses
resolveSetupStoreName instead of manual priority chain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When --from-env is set and $NAME is not in the environment, treat the secret as "not provided" (skip) rather than a hard error. This matches CI usage where only a subset of secrets are injected per job. Hard error is preserved for the case where no value source at all is configured (no --from-env, no --secret, no stdin KEY=VALUE). Caught by Task 10 runtime-launch-validation (step 3: --skip-existing with --from-env was failing on B because $B wasn't set). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + unused field)
- prompt/{confirm,input,multiselect,select}.go: rewrite single-case type
switch to type assertion if-statement per gocritic singleCaseSwitch.
- secrets_setup_noninteractive.go: remove unused 'all bool' field from
nonInteractiveSetupArgs (the --all selector is the default when no
--only/--skip-existing flags are set).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k + masked input) Replaces the legacy buffered-reader interactive loop in runSecretsSetup with a bubbletea-backed wizard that reuses the SAME runSetupEngine as the non-interactive path (only the selector/valuer differ). secrets_setup_interactive.go (new): - runSecretsSetupInteractive: resolves the store (prompt.Select over secretStores keys + builtin provider types env/github/vault/aws/keychain/file when unresolved), prints a redacted store-access line via the concrete secretsProviderAdapter.checkAccess, queries per-entry status, offers prompt.MultiSelect (rows: "NAME ✓ set · updated 3d ago" / "NAME ✗ unset", unset preselected), prompt.Confirm summary, then runs the engine with a selection-backed selector + masked prompt.Input valuer + shared audit. - Pure helpers (unit-tested): storePickOptions, formatStatusLabel, formatRotatedAge, buildMultiSelectItems, queryDeclStatuses, printStoreAccessLine, redactAccessError. secrets_setup.go: - routes to the wizard on a TTY; non-interactive (--non-interactive, --auto-gen-keys, or non-TTY stdin) keeps the engine-backed flag path. - if the wizard hits prompt.ErrNotInteractive mid-flow it falls back to the non-interactive path (never hangs). - --auto-gen-keys now implies non-interactive and generates values via the non-interactive valuer. secrets_setup_plugin.go: - runSecretsSetupPluginWithIO now drives the SAME engine: GitHub provider wrapped in secretsProviderAdapter, prompt.Input valuer on a TTY (masked for sensitive) / promptOne reader valuer for piped/test input, audit per Set. - buildSecretWriter returns secrets.Provider (was scopedWriter) so it can be adapted; scopedWriter removed. secrets_setup_noninteractive.go: valuer gains --auto-gen-keys branch. Tests: interactive wiring helpers + --auto-gen-keys non-interactive path. Existing plugin tests (piped reader) still pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…all + binary test; minors CRITICAL: interactive setup now checks captured promptErr BEFORE the engine's err. With stopOnErr=false the engine returns (report, nil) even when a prompt.Input valuer reported ErrNotInteractive (e.g. stdin goes non-TTY mid-flow); the check must run regardless of err so the caller's non-interactive fallback triggers instead of reporting 'N failed'. IMPORTANT: --plugin non-TTY path no longer blocks on fmt.Fscanln(os.Stdin) (which hangs on an open, dataless pipe). The valuer now has three explicit modes: reader (in!=nil, tests/pipe), interactive prompt.Input (TTY), and non-interactive value sources (--from-env/--secret) for non-TTY — a secret with no source is skipped, never read from stdin. promptOne is reduced to a reader-only helper (no os.Stdin/Fscanln/term), removing the hang at its source; the unused golang.org/x/term import is dropped. --plugin gains --from-env and --secret for CI parity with the main path. IMPORTANT: register --all (Bool) on 'secrets setup'; mutually exclusive with --only; explicit form of the existing default (set every declared secret). IMPORTANT: add TestSecretsSetupNonTTYDoesNotHang — builds the real wfctl binary and runs 'secrets setup --config <tmp>' with empty piped (non-TTY) stdin under a 20s exec deadline; asserts non-zero exit, output names the missing secret, and it returns well before the deadline. Regression guard for the two findings above (follows TestLinkedVersionOverridesBuildInfo). MINORS: (a) go mod tidy moves charm.land/bubbles/v2 to the direct require block (it is directly imported). (b) non-interactive valuer's actionable 'no value for secret ...' error is now printed alongside each [failed] line instead of being swallowed by the engine accumulator. (c) remove write-only selected/done bool fields from the prompt models. (d) replace bespoke stableSort with sort.Strings in secrets_audit.go. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if err != nil { | ||
| return fmt.Errorf("audit: open %s: %w", path, err) | ||
| } | ||
| defer f.Close() //nolint:errcheck |
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
1 participant
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.
What
PR2 of the wfctl secrets wizard + smart CI cascade (design+plan in
workspace:docs/plans/2026-05-30-wfctl-secrets-wizard-and-smart-ci{-design,}.md, scope-locked). Builds the store-aware secrets wizard on PR1's metadata/adapter foundation.Tasks (6–10)
cmd/wfctl/internal/prompt— reusable bubbletea/v2 widgets:Select/MultiSelect/Input(masked)/Confirm. Each returnsErrNotInteractive(never hangs) when stdin isn't a TTY. (+charm.land/bubbles/v2.){Set,Skipped,Failed}report; one engine drives both front-ends and bothsecrets setupandsecrets setup --plugin.--store>secrets.defaultStore> single store > prompt to pick > error), checks store access (✓/✗, redacted), lists each declared secret with✓ set · updated Nd ago/✗ unsetand multi-select which to set (unset pre-selected), masked input for sensitive, confirm before write.--from-env(recommended) / stdinKEY=VALUE/--secret NAME=VALUE(help warns argv leak); selectors--all/--only/--skip-existing; back-compat--scope/--org/--visibility/--token-env. Missing value → error names the secret, never hangs.${XDG_STATE_HOME:-~/.local/state}/wfctl/plugins/wfctl/secrets-audit.jsonl(name/store/scope/ts; never the value).Review findings fixed (adversarial + code review)
ErrNotInteractiveregardless of engine return (was reporting "N failed" instead of falling back).--pluginnon-TTY path no longer reachesFscanln(os.Stdin)— proven no-hang on an open pipe;--allflag added; binary-level no-hang regression testTestSecretsSetupNonTTYDoesNotHangadded.Verification
GOWORK=off go test ./cmd/wfctl/ ./cmd/wfctl/internal/prompt/ ./secrets/→ allok;golangci-lint→0 issues.--pluginopen-pipe returns in 0s (no hang).Notes
🤖 Generated with Claude Code