fix(bootstrap): open the attempt boundary when a retry starts, not when polled

Review finding on ece08bd7. The attempt boundary was stamped when the ~1s
status poll first reported `checking` — which lands after the Rust side has
already emitted the new attempt's first log lines. Those lines were then
filtered out as belonging to the previous attempt, so a fast stage that the
poll also missed stayed pending: the exact evidence loss the log union was
added to prevent.

Retries we initiate now open the attempt in beginAttempt(), at initiation, so
their boundary is exact. A guard stops the stage-transition effect from
re-stamping a boundary we already set a poll interval earlier — without it
the fix would have been undone one tick later.

The effect remains the fallback for restarts begun on the Rust side, where
the poll is the only signal available. That window is documented rather than
hidden: it fails toward showing a step pending (conservative and honest)
rather than done (the fabrication this PR removes). Closing it entirely needs
a Rust-provided attempt id — a new IPC surface, deliberately out of scope.

Regression test fails against ece08bd7, passes here. Suite green (24 tests).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
psiberfunk
2026-09-07 14:02:15 -04:00
co-authored by Claude Opus 5
parent ece08bd790
commit 5e9538a049
2 changed files with 84 additions and 7 deletions
+39 -6
View File
@@ -465,6 +465,9 @@ export function BootstrapSplash({ stage, message }) {
const [attemptStart, setAttemptStart] = useState(0);
const logRef = useRef(null);
const prevStageRef = useRef(stage); // previous stage, for restart detection
// True between a retry WE initiated and the poll catching up to it, so the
// fallback effect below doesn't re-stamp an already-exact attempt boundary.
const selfInitiatedRef = useRef(false);
const prevProgRef = useRef(null); // {bytes, t} — last progress event
const rateRef = useRef(0); // EMA bytes/sec across events
@@ -499,12 +502,25 @@ export function BootstrapSplash({ stage, message }) {
// instead of fabricating completed work (#1894).
const installWorkSeen = INSTALL_STAGES.some((s) => observedStages.has(s));
// Open a new bootstrap attempt. Called at the moment a retry is INITIATED,
// not when the ~1s status poll later reports `checking`: the Rust side
// starts emitting logs for the new attempt immediately, and a boundary
// stamped at detection time would sit *after* those lines and discard them
// as belonging to the old attempt — losing exactly the fast-stage evidence
// the log union exists to capture.
const beginAttempt = () => {
selfInitiatedRef.current = true;
setLogs([]);
setPolledStages(new Set());
setAttemptStart(Date.now());
};
const handleRetry = async () => {
if (retrying) return;
setRetrying(true);
try {
const { invoke } = await import('@tauri-apps/api/core');
setLogs([]);
beginAttempt();
await invoke('retry_bootstrap');
} catch (e) {
console.error('retry failed', e);
@@ -519,7 +535,7 @@ export function BootstrapSplash({ stage, message }) {
setRetrying(true);
try {
const { invoke } = await import('@tauri-apps/api/core');
setLogs([]);
beginAttempt();
await invoke('clean_and_retry_bootstrap');
} catch (e) {
console.error('clean retry failed', e);
@@ -537,15 +553,32 @@ export function BootstrapSplash({ stage, message }) {
// `checking`, and what the previous attempt did says nothing about what
// this one will do. Without the reset, stages the new attempt skips would
// still render as completed, which is the very fabrication this change
// exists to remove. Keyed off the stage moving back to a restart stage
// rather than off our own Retry buttons, so a restart initiated on the
// Rust side resets it too.
// exists to remove.
//
// Retries we initiate call beginAttempt() directly, so their boundary is
// exact. This effect is the FALLBACK for a restart begun on the Rust side,
// where the stage poll is the only signal we get. There the boundary can
// land up to one poll interval late, and log lines from the new attempt in
// that window are discarded rather than counted. That is the deliberate
// direction to fail in: a discarded line can leave a step showing pending
// (conservative, and honest), whereas counting a stale line would render a
// step DONE for work this attempt never did — the bug this change exists to
// fix. Closing the window entirely needs a Rust-provided attempt id, which
// would be a new IPC surface and is deliberately out of scope here.
useEffect(() => {
const prev = prevStageRef.current;
prevStageRef.current = stage;
const restarted = RESTART_STAGES.has(stage) && !RESTART_STAGES.has(prev);
if (restarted) {
setAttemptStart(Date.now());
if (selfInitiatedRef.current) {
// beginAttempt() already opened this attempt with an exact boundary.
// Re-stamping it here — a poll interval later — would discard the
// new attempt's own early log lines, which is the bug this guard
// exists to prevent.
selfInitiatedRef.current = false;
} else {
setAttemptStart(Date.now());
}
setPolledStages(new Set([stage]));
return;
}
@@ -24,7 +24,7 @@
*/
import React from 'react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen, waitFor } from '@testing-library/react';
import { render, screen, waitFor, act, fireEvent } from '@testing-library/react';
import { BootstrapSplash } from '../components/BootstrapSplash';
vi.mock('@tauri-apps/api/core', () => ({
@@ -161,4 +161,48 @@ describe('BootstrapSplash — observed-stage tracking (#1894)', () => {
expect(screen.queryByText(/first run, 5.10 min/)).toBeNull();
expect(screen.getByText('Starting backend…')).toBeInTheDocument();
});
it('logs arriving after Retry but before the next poll are not discarded', async () => {
// Greptile finding on ece08bd7: the attempt boundary was stamped when the
// ~1s poll first reported `checking`, which lands AFTER the Rust side has
// already emitted the new attempt's first log lines — so that evidence was
// filtered out as "previous attempt" and a fast stage missed by polling
// stayed pending. The boundary must open when the retry is initiated.
window.__TAURI_INTERNALS__ = {};
const { invoke } = await import('@tauri-apps/api/core');
const { listen } = await import('@tauri-apps/api/event');
const handlers = {};
listen.mockImplementation(async (name, cb) => {
handlers[name] = cb;
return () => {};
});
invoke.mockImplementation(async () => null);
const { rerender } = render(<BootstrapSplash stage="failed" message="uv sync failed" />);
await waitFor(() => expect(handlers['bootstrap-log']).toBeTypeOf('function'));
// User clicks Retry — this opens the new attempt.
await act(async () => {
fireEvent.click(screen.getByRole('button', { name: /^Retry$/ }));
});
// Rust immediately emits the new attempt's logs, still before the poll
// has reported `checking`.
await act(async () => {
handlers['bootstrap-log']({
payload: { stage: 'creating_venv', line: 'Creating virtualenv at .venv' },
});
});
// Only now does the poll catch up, and it never samples creating_venv.
rerender(<BootstrapSplash stage="checking" message={null} />);
rerender(<BootstrapSplash stage="installing_deps" message={null} />);
// The log line proved creating_venv ran in THIS attempt; it must not have
// been discarded by a boundary stamped after it arrived.
await waitFor(() => {
const venvStep = screen.getByText('Creating Python virtual environment…');
expect(venvStep.className).toMatch(/text-fg-muted/);
});
});
});