diff --git a/frontend/src/components/BootstrapSplash.jsx b/frontend/src/components/BootstrapSplash.jsx index 9d983e9d..6e67271d 100644 --- a/frontend/src/components/BootstrapSplash.jsx +++ b/frontend/src/components/BootstrapSplash.jsx @@ -127,6 +127,10 @@ const STEPS = [ // journey chrome should already be armed by the time the step list appears. const INSTALL_STAGES = ['downloading_uv', 'creating_venv', 'installing_deps', 'awaiting_setup']; +// Stages the bootstrap restarts *from*. Arriving at one of these from +// anywhere else means a new attempt began (Retry, or a Rust-side restart). +const RESTART_STAGES = new Set(['checking', 'awaiting_setup']); + const MAX_LOG_LINES = 200; /** Scan logs + error message for known failure patterns and return i18n keys @@ -452,14 +456,37 @@ export function BootstrapSplash({ stage, message }) { const [progress, setProgress] = useState(null); const [region, setRegionState] = useState('auto'); const [retrying, setRetrying] = useState(false); - // Stages actually seen this session (sticky/monotonic — see the tracking - // effect below). Drives "done" ticks and journey visibility off observed - // reality instead of list position (#1894). - const [observedStages, setObservedStages] = useState(() => new Set([stage])); + // Stages actually seen during the CURRENT bootstrap attempt (see the + // tracking effect below). Drives "done" ticks and journey visibility off + // observed reality instead of list position (#1894). + const [polledStages, setPolledStages] = useState(() => new Set([stage])); + // Wall-clock start of the current attempt. A retry restarts the bootstrap; + // log lines from the previous attempt must not count toward this one. + const [attemptStart, setAttemptStart] = useState(0); const logRef = useRef(null); + const prevStageRef = useRef(stage); // previous stage, for restart detection const prevProgRef = useRef(null); // {bytes, t} — last progress event const rateRef = useRef(0); // EMA bytes/sec across events + // The union of what we polled and what actually logged. + // + // Neither source alone is complete. `bootstrap_status` is sampled ~1/s, so + // a stage that starts and finishes between samples is never polled — on a + // fast disk `creating_venv` routinely does. Stage-tagged `bootstrap-log` + // lines close that gap: the Rust side emits them as the work happens, so a + // line tagged with a stage is proof that stage ran, whether or not the + // poll ever saw it. Lines are filtered to the current attempt so a retry + // cannot inherit the previous one's evidence (the visible log is left + // alone — clearing it on a Rust-side restart would destroy the user's + // context, cf. #1847). + const observedStages = useMemo(() => { + const seen = new Set(polledStages); + for (const entry of logs) { + if (entry?.stage && (entry.t ?? 0) >= attemptStart) seen.add(entry.stage); + } + return seen; + }, [polledStages, logs, attemptStart]); + const label = t(`bootstrap.${stage}`, STAGE_LABEL[stage]); const stepIndex = Math.max(0, STEPS.indexOf(stage)); const isFailed = stage === 'failed'; @@ -501,13 +528,28 @@ export function BootstrapSplash({ stage, message }) { } }; - // Record `stage` as observed the moment it's seen. Sticky/monotonic: the - // functional updater bails out (same Set reference) once a stage is - // already recorded, so this never un-observes anything and never loops. - // `bootstrap_status` is polled ~1/s by useBootstrapStage, so a stage that - // actually ran is guaranteed to land here at least once (#1894). + // Record `stage` as observed the moment it's seen, and reset when a new + // attempt begins. + // + // Sticky WITHIN an attempt: the functional updater bails out (same Set + // reference) once a stage is recorded, so it never un-observes and never + // loops. But NOT across attempts — a Retry restarts the bootstrap from + // `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. useEffect(() => { - setObservedStages((prev) => (prev.has(stage) ? prev : new Set(prev).add(stage))); + const prev = prevStageRef.current; + prevStageRef.current = stage; + const restarted = RESTART_STAGES.has(stage) && !RESTART_STAGES.has(prev); + if (restarted) { + setAttemptStart(Date.now()); + setPolledStages(new Set([stage])); + return; + } + setPolledStages((p) => (p.has(stage) ? p : new Set(p).add(stage))); }, [stage]); // Load persisted region on mount. diff --git a/frontend/src/test/BootstrapSplashObservedStages.test.jsx b/frontend/src/test/BootstrapSplashObservedStages.test.jsx index 36b00103..d8a82aef 100644 --- a/frontend/src/test/BootstrapSplashObservedStages.test.jsx +++ b/frontend/src/test/BootstrapSplashObservedStages.test.jsx @@ -12,13 +12,19 @@ * but `downloading_uv`/`creating_venv` still rendered done), and `JourneyRail` * hardcoded Setup=done/Installing=active regardless of `stage`. * - * The fix tracks which stages were actually observed (sticky, via the - * `bootstrap_status` poll) and derives doneness + journey-chrome visibility - * from that instead of list position. + * The fix tracks which stages were actually observed and derives doneness + + * journey-chrome visibility from that instead of list position. + * + * Two follow-up findings from bot review on PR #1896 are covered at the end: + * - the ~1s `bootstrap_status` poll can miss a stage that starts and + * finishes between samples, so stage-tagged `bootstrap-log` lines are + * unioned in as independent proof a stage ran; + * - a Retry restarts the bootstrap, so stages observed during the previous + * attempt must not carry over and render as done in the new one. */ import React from 'react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import { BootstrapSplash } from '../components/BootstrapSplash'; vi.mock('@tauri-apps/api/core', () => ({ @@ -110,4 +116,49 @@ describe('BootstrapSplash — observed-stage tracking (#1894)', () => { } } }); + + it('a stage the 1s poll never sampled still counts as done when its logs prove it ran', async () => { + // Greptile finding on #1896: `bootstrap_status` is sampled ~1/s, so on a + // fast disk `creating_venv` can start and finish between two samples and + // never be polled. Stage-tagged bootstrap logs are emitted as the work + // happens, so they are independent proof the stage ran. + window.__TAURI_INTERNALS__ = {}; + const { invoke } = await import('@tauri-apps/api/core'); + invoke.mockImplementation(async (cmd) => + cmd === 'get_bootstrap_logs' + ? [{ stage: 'creating_venv', line: 'Creating virtualenv at .venv' }] + : null, + ); + + // Poll sequence skips creating_venv entirely. + const { rerender } = render(); + rerender(); + + await waitFor(() => { + const venvStep = screen.getByText('Creating Python virtual environment…'); + expect(venvStep.className).toMatch(/text-fg-muted/); + }); + }); + + it('a retry drops stages observed during the previous attempt', () => { + // Greptile finding on #1896: the observed set was add-only and the splash + // stays mounted across a Retry, so a stage the FAILED attempt reached + // would still render done in the new attempt even if that attempt skips + // it. Arriving back at `checking` from elsewhere means a new attempt. + const { rerender } = render(); + rerender(); + rerender(); + rerender(); + + // Retry: Rust goes back to `checking`, then this attempt finds the venv + // healthy and jumps straight to starting_backend. + rerender(); + rerender(); + + // Nothing from the previous attempt may be presented as this attempt's + // completed work — so the install chrome is gone entirely again. + expect(screen.queryByText('Downloading uv (Python package manager)…')).toBeNull(); + expect(screen.queryByText(/first run, 5.10 min/)).toBeNull(); + expect(screen.getByText('Starting backend…')).toBeInTheDocument(); + }); });