feat(setup): add safe environment account profiles - #248
Conversation
📝 WalkthroughWalkthroughAdds guarded environment-backed account profile onboarding for compatible local stdio configurations through CLI and Console flows, with validation, audit-backed atomic writes, fail-closed recovery, API/UI wiring, error mappings, tests, and documentation. ChangesEnvironment-backed profile onboarding
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Operator
participant MiftahCLI
participant EnvironmentProfileOnboarding
participant ConfigFile
Operator->>MiftahCLI: setup --add-profile --credential-env
MiftahCLI->>EnvironmentProfileOnboarding: runEnvironmentProfileAddition
EnvironmentProfileOnboarding->>ConfigFile: validate and write profile
ConfigFile-->>EnvironmentProfileOnboarding: guarded write report
EnvironmentProfileOnboarding-->>MiftahCLI: addition result
MiftahCLI-->>Operator: success and verification not-applicable
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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-environment-profile.ts`:
- Around line 82-109: Update runEnvironmentProfileSetup’s prompt flow to handle
SIGINT and EOF reliably, ensuring each prompt settles and reaches the existing
cancelled CliUsageError path instead of hanging. Mirror the cancellation-race
handling used by confirmReadiness(), or pass an AbortSignal through the
line.question calls, while preserving normal answers and the finally cleanup.
In `@src/setup/environment-profile-onboarding.ts`:
- Around line 197-201: Update the security-setting mutation in the onboarding
flow to compare each existing value before forcing
requireProfileSwitchConfirmation and requireExplicitSelectionForDestructive to
true. When either value changes, add a corresponding entry to plan.actions so
the CLI/Console reports the mutation; avoid recording actions when the values
are already true.
🪄 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: 252962fe-8cf4-41b5-9244-46963b9d12da
📒 Files selected for processing (24)
CHANGELOG.mdREADME.mddocs/cli.mddocs/console-api.mdsrc/cli/exit-codes.tssrc/cli/parse.tssrc/cli/setup-environment-profile.tssrc/cli/setup.tssrc/console/console-application-service.tssrc/console/console-assets.tssrc/console/console-config-metadata.tssrc/console/console-dashboard-application-service.tssrc/console/console-server.tssrc/setup/environment-profile-onboarding.tssrc/utils/errors.tstests/cli-exit-codes.test.tstests/cli-parse.test.tstests/console-application-service.test.tstests/console-config-metadata.test.tstests/console-dashboard-application-service.test.tstests/console-server.test.tstests/environment-profile-onboarding.test.tstests/readme-product-contract.test.tstests/setup-command.test.ts
Dismissed as stale: both actionable threads were resolved in 633d23c, and CodeRabbit is rate-limited for a fresh review. Full local validation and current-head CI are required before merge.
Part of #204
Summary
Safety boundary
This flow refuses remote HTTP/SSE transports, native OAuth configurations, reviewed provider adapters, named/multiple upstreams, arbitrary profile overrides, repeated credential references, and any input that is not a simple local
TARGET: "${SOURCE}"binding. It does not launch the upstream or perform a generic readiness call.Validation
npm test: 1,601 passed, 27 platform skipsnpm run test:core: 415 passed, 22 platform skipsnpm run lintnpm run typechecknpm run buildnpm run smoke:clinpm run check:packnpm run test:package: 24 passednpm run test:coveragehas an existing timing-sensitive full-suite failure tracked in #122. The exact cases pass under coverage in isolation, and the exact currentdevelopmentbase independently fails full coverage in a different timing-sensitive integration test. No timeout, test, coverage, or runtime behavior was relaxed.Summary by CodeRabbit
New Features
Bug Fixes
Documentation