fix(dev-fleet): report slow actions as indeterminate, not as a fake percentage

[chenmingwei23]

Problem

Two Dev Fleet progress affordances read as a hung page.

1. Prune merged — clicking it goes quiet for seconds on a large fleet. The button was only disabled for that window, which is visually indistinguishable from a wedged page, so the user clicks again or reloads mid-scan.
2. Pull+Build — the sync row showed a percentage that is wrong. It sat at ~8% through the entire pip install, then jumped.

Why it matters

Both are the slowest actions on the page, which is exactly where a user needs to know the difference between "working" and "dead". A wrong percentage is worse than no percentage: it invites the user to wait for a number that isn't tracking anything, and then to give up when it stalls. And an unresponsive-looking Prune button gets re-clicked or reloaded mid-scan.

Fix (symptoms → root cause → change)

Prune merged: silence during a real scan

prune-candidates walks worktrees serially, running several git calls plus a gh PR lookup per worktree (_prune_candidates → _prunable → _pr_status_cached / _fetch_pr_head_oid). The delay is real work, not a hang — but the UI had no way to say so, because disabled is the only state the button had.

The trash glyph now becomes a spinner in place and the button sets aria-busy for the duration of the scan.

The serial loop itself is the cause of the wait, and parallelizing it (as _prune_run already does with a semaphore) would shorten it — deliberately left to a separate PR, since that is a different problem.

Pull+Build: a percentage the backend cannot support

syncPercent() mapped the step index onto a hardcoded SYNC_STEP_CUM = [0, 5, 8, 25, 55, 100] curve, plus a time-based "creep" that crawled toward the next band at one point per 4s. The five steps (fetch / merge / pip / npm ci / build) differ in duration by more than an order of magnitude and shift with network and cache state, so the bar parks in one band for most of the run and then jumps — precision the backend does not have, and a stall to anyone watching.

Progress is now indeterminate: a spinner whose role="progressbar" carries no aria-valuenow (the ARIA form for "in progress, amount unknown"), beside the server's own step label and the elapsed timer. step_label comes from the run worker and is the honest signal — it survives the 60-line output tail window and names the step actually in flight.

The whole step-index model behind the percentage is deleted with it: SYNC_STEP_CUM, SYNC_TOTAL_STEPS, syncPhaseFromLines(), syncPercent(), and SyncRun.phase / phaseAt across all five setSyncRun call sites. STEP_MARKER_RE stays — it still strips markers from the log panel and picks the last real output line.

Static spinners

Three LoaderCircle icons on this page (pod busy, provisioning, sync) carried no spin class at all — static "loading" glyphs. They spin now, and the restart overlay's inline animation: 'spin 1s linear infinite' moves to the same animate-spin utility.

No new catalog keys

The spinner reuses the existing pages.devFleetPage.sync_progress string as its accessible name, so there is no locale churn and no dead key. The change also deletes a hardcoded, unlocalized '~' + pct + '%' render, so the i18n surface gets strictly smaller.

Screenshots

Captured from an isolated pod running this branch's built bundle (kirocrew pod up fleet-progress), driving a real Pull+Build against the main worktree. Nothing is injected or stubbed — the step labels and elapsed times below are a genuine sync in flight, which is the only way this row is reachable.

Mid-sync, early — spinner + the server's pip install step label + elapsed. No percentage, no bar:

Later in the same run — the step label has moved to npm build + stage at 0:22, with the log panel open. This stretch is exactly where the old bar sat parked in its [8, 25] band and looked stalled; the label plus a ticking timer says the same thing honestly:

At rest, and the terminal state

The fleet at rest — Prune merged in its normal trash-glyph state, for comparison with the spinner it swaps to while scanning:

The same run reaching its terminal state — the indeterminate row is replaced by the existing green Synced summary, unchanged by this PR:

The prune-scan spinner has no capture. On a two-worktree fleet the scan finishes in well under a second, so the in-flight window is not reliably photographable, and the only way to hold it open would be to stub the backend response — which produces a frame that shows a state the server was forced into. The two vitest cases below assert that state instead; that is the honest artifact for it.

Tests

website/src/test/DevFleetPage.test.tsx:

• reports a running sync as indeterminate — a spinner, never a percentage (replaces the old test, which asserted the percentage band). Locks in: aria-valuenow and aria-valuemax are absent (not zeroed), the progressbar carries animate-spin, no % text renders anywhere, and the server's step_label is still displayed. The mock feeds both a ::step::0:: marker and step: 3 so a reintroduced percentage of any flavour fails it.
• spins the Prune merged button while the merged-scan is in flight (new). Gates the prune-candidates response on a manually-resolved promise so the in-flight window is observable, then asserts: no spinner before the click → aria-busy="true" + spinner after → spinner gone once the dialog opens.

Manual verification

The screenshots above are the manual verification for the sync half, taken against a real in-flight Pull+Build in an isolated pod. The prune half is covered by unit assertions for the reason given above.

Local gates, all green in the worktree:

Gate	Result
npx tsc -b	clean
npx vitest run	869 files / 11628 passed / 0 failed
npx eslint src/ --max-warnings 1116	0 errors (574 warnings)
npx jscpd .	0 clones
npm run i18n:check	13 / 13 PASS
./scripts/docs-lint.sh	passed (190 files)
./scripts/scrub-lint.sh --no-history	passed
check_brand_name.py	passed

Backend untouched, so no pytest run.

Docs

docs/system-specs/modules/dev-fleet.md gains the scan-feedback rationale under the prune section, and the sync section now states that progress is indeterminate by design and why the step index is a poor basis for a percentage. The stale "beside the 'Syncing' progress bar" sentence is corrected.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e — updated in place on each push; does not block merge.

The change is small, well-scoped, and deletes more model than it adds. The temp-screenshots/ convention is established in the repo (~180 prior entries), animate-spin is a standard utility in the codebase, the spec doc is updated in the same commit, and the description matches the diff bidirectionally — including honestly deferring the real prune-scan latency (serial git/gh walk) to a separate PR rather than smuggling a backend change in here. Replacing a fabricated step-weight percentage with an indeterminate spinner plus the server's own step label is the correct shape: the alternative (calibrating a determinate bar from historical step durations) would add state the problem doesn't warrant.

Design-Verdict: PASS

Deleting the fabricated percentage model instead of tuning it is the right call; indeterminate + server step label matches what the backend can actually assert.

[DESIGN-REVIEWED] 32dccba

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e — this comment is updated in place on each push.

Review details

The diff removes the determinate progress bar and replaces it with an indeterminate spinner. Verified: no dangling references to removed functions (syncPercent, SYNC_STEP_CUM, syncPhaseFromLines) or the removed phase/phaseAt fields; the sync_progress i18n key exists across locales; icons use lucide-react with aria-label present. No AUTOSDE rule violations, no reachable security/crash/data-loss issues, no removed guards.

No findings.

[OPUS-REVIEWED] 32dccba

Verdict parsed from the review's SHA-scoped output markers for commit 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ✅ PASS

Advisory UX-level review of 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e — updated in place on each push; does not block merge.

UX-Verdict: PASS

Replacing a stalling fake percentage with spinner + server step label + elapsed time is honest, verifiable in the committed screenshots, and ARIA-correct.

Suggestions

• While the merged-scan runs, Prune merged shows a spinner on a danger button with unchanged text — during those seconds a user can read it as "deletion already running" and reload mid-scan (the exact panic this PR targets); swap the label or add adjacent status to "Scanning…" for the busy window.
• The description claims "No screenshots" and "N/A" manual verification, yet the diff commits four under temp-screenshots/dev-fleet-progress-affordance/ showing a live sync — update the description so human reviewers trust the evidence that's actually there.
• sync-running-npm-build-with-log.png shows the step label "npm build + stage" over a log tail that is entirely pip output; if the 60-line tail routinely lags the step, the label and log contradict each other — worth a one-line "(recent output)" hint on the log toggle if this proves common.

[UX-REVIEWED] 32dccba

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 32dccba

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e: <one-sentence reason>

[chenmingwei23]

Disposition — UX Review suggestion on 3b437f3ef927f45fc135cf96a43b6e1ddece0ade

motion-reduce:animate-none on the five animate-spin usages — rebutted (no behavioural difference), not applied.

The suggestion's premise is that the spinner keeps spinning for reduced-motion users, so a 30-minute sync would animate the whole time. It does not. website/src/index.css:1227 carries a global rule:

@media(prefers-reduced-motion:reduce){*:not([data-virtuoso-scroller] *),*::before,*::after{animation-duration:0.01ms !important;animation-iteration-count:1 !important}}

Tailwind's animate-spin is a non-important animation: spin 1s linear infinite shorthand, so those two !important longhands win: under prefers-reduced-motion: reduce the spin already runs once for 0.01ms and stops, landing on rotate(360deg) — visually identical to animation: none. Adding motion-reduce:animate-none would change the declaration but not a single rendered frame, on these five call sites or on the ones cited as precedent.

Two supporting points:

• The precedent is the minority pattern, not the house style: 23 of the 111 files that use animate-spin pair it with the modifier, and none of the pre-existing spinners in DevFleetPage.tsx do (including the prune-checklist spinner at line 1459 this PR does not touch). Adding it to only the lines this PR touches would make the file internally inconsistent while changing nothing.
• What actually keeps the row from reading as hung for reduced-motion users is the 1s elapsed-time interval (DevFleetPage.tsx:678-685) plus the server's step label — both of which this PR keeps, and the second of which it now gives more room. So the bug this PR fixes does not recur for them.

If we do want a moving indicator under reduced motion — a defensible position for a long-running action, and arguably in tension with that blanket global rule — it needs a per-element animation-duration override that outranks index.css:1227. That is a repo-wide accessibility policy change affecting every spinner in the dashboard, not something to smuggle into this PR; happy to file it as a follow-up if you want it.

[chenmingwei23]

Disposition — the two red Windows shards on 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e

Both are accepted-and-deferred: neither is caused by this branch, and neither is fixed here (each has its own issue). This PR changes no Python at all — git diff --name-only origin/main...HEAD is one doc, two TypeScript files, and four PNGs — and both failing tests are byte-identical to main. Everything else is green: 48 of 51 checks pass, and the third red is PR Readiness aggregating these two.

1. Backend Tests (Windows) (3) — test_mcp_gateway_transport.py::test_create_server_pipe_closes_the_handle_when_the_read_mode_flip_fails

AssertionError: the orphaned pipe handle was not closed
assert 3 == 1

Already tracked as #2301, and demonstrably pre-existing: the same test failed on main itself in run 31296839010 (assert 5 == 1). The test monkeypatches the process-wide _winapi.CloseHandle and asserts an exact count of 1, so any unrelated handle close inside the block inflates it — which is why the number differs run to run (5 there, 3 here). Deferred to #2301.

2. Backend Tests (Windows) (4) — test_turn_duration_subagent.py::test_subagent_turn_records_local_wall_clock

assert 15 >= 20

Filed as #2449. A 30 ms asyncio.sleep with a hardcoded 20 ms floor, measured against real wall clock, on a platform whose timer resolution is ~15.625 ms — the observed 15 ms is exactly one Windows tick, so the assertion's margin is thinner than the platform's own quantum. Class 2 (wall-clock race) in docs/system-specs/common/testing-conventions.md § Determinism.

Honest limit on this one: it is not yet observed failing on main. ~40 recent main runs were inspected and the shard-4 job passed in all of them, including the five that failed for unrelated reasons. So the claim here is not "main is red too" — it is that the test is untouched by this diff, is a wall-clock race by construction, and the failure mode is fully explained by the Windows timer quantum. Latent on main rather than branch-specific.

Why no fix in this PR. Both are backend test-determinism bugs in files this change does not touch; folding either in would widen a frontend-only PR into unrelated territory, and the repo's own convention is one problem per PR. The conventions doc is also explicit that a flake is a root-cause bug rather than something to paper over, so neither gets a rerun-with-longer-sleep or a relaxed assertion here.

The two jobs have been re-run to clear the red on this SHA; the diff is unchanged, since re-pushing an identical diff against a flake would just reroll the same dice.

[chenmingwei23]

Dispositions — UX Review's three suggestions on 32dccbadc1c32c6a68bf9070fbda99bcfa90ca3e

1. "Prune merged" keeps its label while spinning, so a spinner on a danger button can read as "deletion already running" — accepted-and-deferred to #2451.

Legitimate, and a sharp catch: this is the second-order version of the bug the spinner fixes. The spinner answers "is the page frozen?"; it does not answer "is something destructive already happening?", and on a danger-styled button the latter is the natural read. Both misreads end the same way — a reload mid-scan, which throws the git + gh walk away and starts it over.

Not fixed here because the fix is copy, and copy is not free in this repo: a scanning label needs a new pages.devFleetPage key plus translations for all 11 other shipped locales, since catalogParity.test.ts requires every language to carry every key. This PR deliberately adds zero new catalog keys — the spinner reuses the existing sync_progress string as its accessible name — so folding this in would pull a 12-locale translation pass into a frontend affordance fix. #2451 carries the full shape, including the open question of whether the danger styling itself should be suppressed while a read-only scan runs.

2. The description claims "No screenshots" and "N/A" manual verification while committing four — rebutted, already fixed.

Correct when observed, stale now: that wording was in the body at gh pr create time, before the screenshots existed. The body was rewritten in the same step that committed them. Against the live description:

$ gh api repos/kirodotdev/KiroCrew/pulls/2439 --jq .body | grep -nE "No screenshots|N/A"
(no matches)

## Manual verification now opens with "The screenshots above are the manual verification for the sync half, taken against a real in-flight Pull+Build in an isolated pod," and a ## Screenshots section carries all four SHA-pinned. The only "no capture" sentence left is the deliberate one explaining why the prune spinner has no frame. No change needed.

3. sync-running-npm-build-with-log.png shows step label npm build + stage over a log tail that is entirely pip output — rebutted: that frame is the design working, not a contradiction.

The skew is real and expected. /run returns only the last 60 output lines (a sliding tail), while step / step_label are recorded authoritatively on the run entry by the worker. When a step advances, the label flips immediately but the tail still holds the previous step's output until the new step emits enough lines to push it out. docs/system-specs/modules/dev-fleet.md states this as the reason the label exists: the worker records the label "so /run can name the CURRENT step even after the marker scrolls out of the 60-line output tail window."

So in that frame the label is the trustworthy element and the log is the lagging one — which is exactly the property the step-label mechanism was added to provide. Nothing in this PR touches the log panel or the tail window; both predate it. A "(recent output)" hint on the log toggle is a reasonable idea on its own merits, but it would be a change to an unrelated pre-existing surface — and it needs new copy, so it carries the same 12-locale cost as suggestion 1 without being caused by this diff.