fix(bootstrap): treat leaving failed as a new attempt
Review finding on5e9538a0. The reset keyed only off arriving at a restart stage, so a retry whose restart stage the poll never sampled did not reset at all: failed -> checking -> starting_backend inside one ~1s window surfaces as failed -> starting_backend, leaving the FAILED attempt's stages in polledStages and rendering its install chrome as this attempt's completed work. Not merely a late boundary — no boundary. retry_bootstrap / clean_and_retry_bootstrap are the only exits from `failed`, so leaving that stage is itself proof a new attempt began. Keying off it as well as off restart stages closes the case without new producer state. Regression test fails against5e9538a0, passes here. Suite green (25 tests). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5e9538a049
commit
b5879343a0
@@ -568,7 +568,17 @@ export function BootstrapSplash({ stage, message }) {
|
||||
useEffect(() => {
|
||||
const prev = prevStageRef.current;
|
||||
prevStageRef.current = stage;
|
||||
const restarted = RESTART_STAGES.has(stage) && !RESTART_STAGES.has(prev);
|
||||
// Two ways a new attempt shows up in the poll. Arriving at a restart
|
||||
// stage is the common one. But the poll can also miss the restart stage
|
||||
// entirely — `failed` -> (retry) -> `checking` -> `starting_backend`
|
||||
// inside one ~1s sample window surfaces as `failed` -> `starting_backend`,
|
||||
// and keying only off restart stages would leave the FAILED attempt's
|
||||
// evidence in place and render its install chrome as this attempt's
|
||||
// completed work. Leaving `failed` at all means a retry began, since
|
||||
// retry_bootstrap/clean_and_retry_bootstrap are the only exits from it.
|
||||
const restarted =
|
||||
(RESTART_STAGES.has(stage) && !RESTART_STAGES.has(prev)) ||
|
||||
(prev === 'failed' && stage !== 'failed');
|
||||
if (restarted) {
|
||||
if (selfInitiatedRef.current) {
|
||||
// beginAttempt() already opened this attempt with an exact boundary.
|
||||
|
||||
@@ -205,4 +205,25 @@ describe('BootstrapSplash — observed-stage tracking (#1894)', () => {
|
||||
expect(venvStep.className).toMatch(/text-fg-muted/);
|
||||
});
|
||||
});
|
||||
|
||||
it('a retry whose restart stage the poll never sampled still drops the old evidence', () => {
|
||||
// CodeRabbit finding on 5e9538a0: a retry can go failed -> checking ->
|
||||
// starting_backend inside one ~1s sample window, so the poll observes
|
||||
// only failed -> starting_backend. Keying the reset solely off arriving
|
||||
// at a restart stage would leave the FAILED attempt's stages in place and
|
||||
// present them as this attempt's completed work.
|
||||
const { rerender } = render(<BootstrapSplash stage="checking" message={null} />);
|
||||
rerender(<BootstrapSplash stage="downloading_uv" message={null} />);
|
||||
rerender(<BootstrapSplash stage="installing_deps" message={null} />);
|
||||
rerender(<BootstrapSplash stage="failed" message="uv sync failed" />);
|
||||
|
||||
// Retry — and the poll misses `checking` entirely.
|
||||
rerender(<BootstrapSplash stage="starting_backend" message={null} />);
|
||||
|
||||
expect(screen.getByText('Starting backend…')).toBeInTheDocument();
|
||||
// Nothing from the failed attempt may be shown as this attempt's work.
|
||||
expect(screen.queryByText('Downloading uv (Python package manager)…')).toBeNull();
|
||||
expect(screen.queryByText(/first run, 5.10 min/)).toBeNull();
|
||||
expect(screen.queryByText('Installing')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user