Uh oh!
There was an error while loading. Please reload this page.
Run pre-install bootstrap steps before engine selection in add-wizard - #48301
Conversation
Run repo-variable and repo-secret bootstrap steps before engine selection so required configuration is collected up front, matching what TestTuistoryAddWizardManifestBootstrapRunsBeforeEngineSelection expects. - Add isPreInstallBootstrapType helper (repo-variable, repo-secret) - Add splitBootstrapProfile to partition config into pre/post phases - Execute pre-install steps (Step 5b) before selectAIEngineAndKey - Add TestSplitBootstrapProfile unit tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
commented
Jul 27, 2026
@copilot Fix test, current behavior is correct |
🤖 PR Triage
Score breakdown: Impact 24/50 · Urgency 12/30 · Quality 12/20 Rationale: Bootstrap steps (repo-variable/repo-secret manifest steps) run post-install instead of pre-engine-selection in add-wizard. Fix is small (169+/6−, 3 files). Currently a draft with only the Copilot review check running. Needs undraft + full CI before merge.
|
The current behavior (all bootstrap steps post-install) is correct. Rewrite the test to verify that the engine selection prompt appears first, and the bootstrap variable prompt has not yet appeared at that point. - Rename TestTuistoryAddWizardManifestBootstrapRunsBeforeEngineSelection → TestTuistoryAddWizardManifestBootstrapRunsAfterEngineSelection - Wait for engine selection prompt first; assert bootstrap prompt has not appeared yet - Revert orchestrator and bootstrap_config.go to single-phase post-install behavior - Revert TestSplitBootstrapProfile and related test changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Reverted the orchestrator changes and fixed the test instead — it now waits for the engine selection prompt first, then asserts the bootstrap variable prompt has not appeared yet (since it runs post-install). Renamed to |
There was a problem hiding this comment.
Pull request overview
Intended to move variable/secret bootstrap prompts before engine selection, but the diff instead updates the integration test to accept the existing post-selection behavior.
Changes:
- Renames the bootstrap-ordering integration test.
- Reverses its assertions to require engine selection first.
- Removes validation that handoff steps remain deferred.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/add_wizard_tuistory_integration_test.go | Changes bootstrap prompt ordering expectations. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Medium
| assert.Less(t, earlyPromptIndex, enginePromptIndex, "Pre-install bootstrap prompt should appear before engine selection") | ||
| beforeInterruptOutput := session.readAll() | ||
| assert.Contains(t, beforeInterruptOutput, enginePrompt, "Expected engine selection prompt to be shown") | ||
| assert.NotContains(t, beforeInterruptOutput, bootstrapPrompt, "Bootstrap variable prompt should not appear before engine selection") |
🎉 This pull request is included in a new release. Release: |
TestTuistoryAddWizardManifestBootstrapRunsBeforeEngineSelectionwas failing because the orchestrator ran all bootstrap config steps post-install, butrepo-variable/repo-secretmanifest steps should prompt the user before engine selection.Changes
bootstrap_config.go: AddisPreInstallBootstrapType(repo-variable,repo-secret) andsplitBootstrapProfilewhich partitions a resolved bootstrap profile into pre/post-install parts.add_interactive_orchestrator.go: Replace the single-phase approach with a split — pre-install steps run in a new Step 5b (after permission checks, before engine selection); post-install steps (handoff,github-app,copilot-auth, etc.) remain in Step 9b after the PR is created.bootstrap_profile_runner_test.go: AddTestSplitBootstrapProfilecovering nil/empty/pre-only/post-only/mixed configurations; clarifyTestPrintBootstrapConfigTODO_PreservesManifestOrderscope (that test guards the non-interactiveaddpath, not add-wizard).The flow now matches the declared manifest ordering within each phase: pre-install variable/secret collection happens up front, post-install side effects happen after installation.