fix: provider wizard data loss and startup provider fallback#680
fix: provider wizard data loss and startup provider fallback#680llupRisinglll wants to merge 4 commits into
Conversation
|
Hi @llupRisinglll, thanks for putting this together! Great work tracking down both the provider wizard data loss issue and the startup provider fallback logic. Overall the implementation is heading in the right direction, but I found a couple of blockers regarding test coverage and test hygiene before we can merge this. 1. Missing Integration Test Coverage for Initialization FlowWhere it is wrong: Why it is wrong: Specifically, the following new behaviors are currently untested:
These are the primary user-visible behaviors introduced by this PR, so they should be covered to prevent future regressions. What should be done: Please add tests to
2. Temporary Directory CleanupWhere it is wrong: Why it is wrong: What should be done: Please clean up the temporary directory after the tests complete (for example using 3. Please Open an Issue Before Large Behavioral ChangesThis PR introduces multiple behavioral changes (provider wizard persistence improvements, startup fallback logic, and provider recovery flow), but there isn't an associated GitHub issue describing the problem, expected behavior, or proposed solution. For changes of this scope, I'd recommend opening an issue first so we can discuss the architecture and expected user flow before implementation. It also makes future maintenance much easier by providing clear context for why the change was made. Overall, this is a solid improvement and addresses some important edge cases. Once the missing integration tests and test cleanup are addressed, I'll be happy to take another look. Great work! |
Description
agents.config.json.agents.config.json— it warns and falls back to the first configured provider (an explicit--providerCLI arg stays strict and still errors).Type of Change
Testing
Automated Tests
.spec.ts/tsxfilesuseAppInitializationspec,tsc --noEmit, build — all clean; no behavior removed, only new guard paths added)Manual Testing
Verified across four scenarios on a real multi-provider config:
Checklist