Skip to content

fix(setup): explain environment secret readiness - #304

Merged
mohanagy merged 5 commits into
developmentfrom
fix/301-secret-readiness
Jul 29, 2026
Merged

fix(setup): explain environment secret readiness#304
mohanagy merged 5 commits into
developmentfrom
fix/301-secret-readiness

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • add one names-only environment readiness model shared by CLI and Console
  • distinguish not required, not checked, missing, and available without retaining or returning credential values
  • recognize interpolation plus canonical env and dotenv references while keeping configured env-file availability honest
  • show readiness before the actual CLI client handoff and persist client-specific guidance beside Console JSON
  • keep provider verification separate, including the honest Sentry no-safe-check state

Security impact

  • credential values are never retained in setup completion data, browser payloads, generated client JSON, or guidance
  • Console guidance uses text-only rendering; no secret-derived HTML is introduced
  • subprocess behavior, redaction, audit guarantees, containment, and public APIs are unchanged
  • configured environment files are not opened during setup; their availability is reported as not fully checked

Validation

  • npm test (1,797 passed; 33 platform/conditional skips)
  • npm run test:core (425 passed; 26 platform/conditional skips)
  • npm run test:coverage (1,797 passed; 33 skips; 95.67% statements)
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run smoke:cli
  • npm run check:pack
  • npm run test:package

Closes #301

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Setup now detects environment-backed secrets, reports readiness in CLI and Console completion, and adds client-specific handoff guidance while keeping secret values out of generated JSON, configuration output, and browser payloads.

Changes

Environment readiness

Layer / File(s) Summary
Readiness model and reference extraction
src/setup/setup-completion.ts
Adds typed readiness states, extracts validated environment references, inspects availability, and produces completion messages.
Client snippet guidance and wiring
src/cli/client-snippets.ts, src/cli/init.ts, src/cli/main.ts, src/console/console-application-service.ts
Validates required variable names, adds client-specific guidance, and passes environment requirements through snippet rendering.
CLI and Console completion surfaces
src/cli/setup.ts, src/cli/init.ts, src/console/console-application-service.ts, src/console/console-assets.ts
Reports environment readiness, supports deferred handoff output, and renders environment completion messages in the Console UI.
Readiness and redaction coverage
tests/client-snippets.test.ts, tests/setup-completion.test.ts, tests/setup-command.test.ts, tests/console-application-service.test.ts, tests/console-server.test.ts
Covers readiness states, reference extraction, handoff guidance, persistence placeholders, UI output, and secret non-disclosure.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Config
  participant SetupCompletion
  participant ClientSnippets
  participant CLIConsole
  Config->>SetupCompletion: extract environment references
  SetupCompletion->>SetupCompletion: inspect variable availability
  SetupCompletion->>CLIConsole: return readiness and next action
  Config->>ClientSnippets: pass required variable names
  ClientSnippets->>CLIConsole: render non-secret handoff guidance
Loading

Possibly related PRs

Poem

A rabbit saw secrets hop by,
Their names were shown, values stayed shy.
“Pass them along,” the handoff said,
“Then safely start,” the bunny read.
No secret spilled—just guidance bright!


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Docstring Coverage ❌ Error Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement a shared readiness model, safe next-step guidance, redaction, and Sentry no-safe-check handling required by #301.
Out of Scope Changes check ✅ Passed The diff appears focused on setup environment readiness, client handoff guidance, and related tests with no unrelated feature work.
Title check ✅ Passed The title is concise and accurately reflects the main change: explaining setup environment secret readiness.
Description check ✅ Passed The description matches the template with Summary, Security impact, and Validation sections filled in with relevant details.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/301-secret-readiness

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@src/cli/setup.ts`:
- Around line 378-381: Update src/cli/setup.ts lines 378-381 to call
inspectSetupEnvironment(requiredEnvironmentVariables) unconditionally,
preserving the returned not-required state when no references exist, and pass
that result through completion. In src/console/console-application-service.ts
lines 465-472, pass the unconditional inspection result to createSetupCompletion
so both setup flows emit not-required readiness states.

In `@src/console/console-application-service.ts`:
- Around line 941-942: Update the Console rendering logic in console-assets.ts
to display first.guidance persistently alongside the generated first.json client
snippet. Preserve the existing JSON rendering and ensure the guidance explains
required variables and process inheritance for the client handoff.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 958c4f8d-c2c3-4290-b8b6-665d1809c543

📥 Commits

Reviewing files that changed from the base of the PR and between d98eb9a and 9a3dc51.

📒 Files selected for processing (12)
  • src/cli/client-snippets.ts
  • src/cli/init.ts
  • src/cli/main.ts
  • src/cli/setup.ts
  • src/console/console-application-service.ts
  • src/console/console-assets.ts
  • src/setup/setup-completion.ts
  • tests/client-snippets.test.ts
  • tests/console-application-service.test.ts
  • tests/console-server.test.ts
  • tests/setup-command.test.ts
  • tests/setup-completion.test.ts

Comment thread src/cli/setup.ts Outdated
Comment thread src/console/console-application-service.ts Outdated
coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@src/setup/setup-completion.ts`:
- Around line 294-296: Update the missingVariables initialization in the
setup-completion validation flow to preserve all normalized
readiness.missingVariables without filtering against requiredVariables. Leave
the normalized requiredVariables handling unchanged so state-specific validation
can reject contradictory names such as missing variables that are not required.
- Around line 179-184: The env-file deferral return in the setup completion flow
must preserve all variables required by the separately launched process. Update
the `requiredVariables` field to use `readiness.requiredVariables`, while
retaining `readiness.missingVariables` only for deciding whether to defer and
keeping `missingVariables` as the missing subset.

In `@tests/setup-command.test.ts`:
- Around line 462-464: Update inspectSetupEnvironment’s readiness detection to
treat empty string environment values the same as undefined values when
determining missing setup requirements. Preserve existing handling for populated
values and ensure the “Missing from this setup process” message is emitted for
an empty SENTRY_ACCESS_TOKEN.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c3762664-a5a2-4e21-8578-324e84f2419b

📥 Commits

Reviewing files that changed from the base of the PR and between 9a3dc51 and 472372c.

📒 Files selected for processing (10)
  • src/cli/client-snippets.ts
  • src/cli/init.ts
  • src/cli/main.ts
  • src/cli/setup.ts
  • src/console/console-application-service.ts
  • src/setup/setup-completion.ts
  • tests/client-snippets.test.ts
  • tests/console-server.test.ts
  • tests/setup-command.test.ts
  • tests/setup-completion.test.ts

Comment thread src/setup/setup-completion.ts
Comment thread src/setup/setup-completion.ts Outdated
Comment thread tests/setup-command.test.ts
@mohanagy
mohanagy dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot] July 29, 2026 20:33

Dismissed as stale after all actionable findings were resolved. Exact head c592986 passed full Linux, macOS, and Windows CI and independent review approved it. The latest CodeRabbit incremental review was rate-limited, so this dismissal does not claim a new CodeRabbit approval.

@mohanagy

Copy link
Copy Markdown
Owner Author

Final review record for c592986:

  • all actionable CodeRabbit findings from earlier commits were fixed with regression tests and their threads were resolved
  • the latest incremental CodeRabbit review was rate-limited; the current-head CodeRabbit status check passed, but this is not being represented as a fresh CodeRabbit approval
  • an independent exact-head review approved the change and specifically rechecked the two final blockers, redaction, text-only Console rendering, Windows behavior, credential-free client JSON, and public API compatibility
  • exact-head CI passed Linux quality plus Ubuntu, macOS, and Windows on Node 20, 22, and 24

The two stale CodeRabbit change-request reviews were dismissed with this same rationale.

@mohanagy
mohanagy merged commit 43f9f52 into development Jul 29, 2026
12 checks passed
@mohanagy
mohanagy deleted the fix/301-secret-readiness branch July 29, 2026 20:34
mohanagy added a commit that referenced this pull request Jul 29, 2026
* fix(console): explain filtered configurations (#302)

Show aggregate discovered/ready/attention counts without exposing rejected paths, explain safe repair categories, and preserve trusted discovery containment. Includes regression contracts and all review fixes.

Closes #203.

* fix(console): preserve valid sessions across reloads (#303)

* fix(console): resume valid sessions after reload

* fix(console): explain unlock rate limiting

* fix(setup): explain environment secret readiness (#304)

* fix(setup): explain environment secret readiness

* fix: make secret readiness guidance truthful

* fix: keep readiness guidance visible

* fix: preserve environment readiness invariants

* fix: prevent stale readiness guidance

* release: prepare v0.5.2 (#306)

* release: prepare v0.5.2

* test: bind release notes to current patch

* test: scope release links to fixed section
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.

[Setup] Make environment-secret readiness explicit before client handoff

1 participant