test(cycle): a retry the browser walk can actually pass

[JArmandoAnaya]

Every attempt of the real-server browser walk now starts on an empty workspace, so a retry reaches the failure it was started for instead of dying three steps in on a screen the product was never wrong about.

Closes #688.
Closes #691.
Closes #692.

What changed

Two files under frontend/app/cycle/, no product code. A new helper, _workspace.ts, empties the workspace through the routes the product already publishes — DELETE /projects/{id}?confirm=true and DELETE /inference/connections/{id} — and an unconditional test.beforeEach in cycle.spec.ts calls it before every attempt and every repetition.

The walk retries once whenever CI is set, and that retry had never been able to pass. The workspace is built once per server start: scripts/cycle_server.sh does rm -rf and then visionset init, and playwright.cycle.config.ts runs that script once as its webServer. A retry is therefore a second attempt against the workspace the first one left behind, and three steps in the walk asserts Home's first-run invitation, which is gated on the workspace holding zero projects and on nothing else. So the retry asked a fresh-workspace question of a used workspace and failed there, every time, whatever the real failure had been. Because trace is on-first-retry and screenshot is only-on-failure, every artifact a person opened belonged to that retry — a report naming the wrong screen, with the real failure further up the log.

scripts/check.sh sets CI=1 for the browser stages itself, so this was never only a CI phenomenon.

Deleting through the API rather than the filesystem is what makes the repair possible at all: the server owns the workspace for as long as it runs, and webServer is Playwright's to start and stop. A project delete takes its dataset, batches, jobs, sources and releases with it, and connections are the only other workspace-level row this walk creates. Content blobs survive both by design, which is why a repeated walk re-ingests the same three fixture images into a new dataset rather than into a new store — the one thing about this approach that had to be run rather than reasoned about, and the first thing the test plan below does.

The reset is loud on purpose. Each list asserts ok() and that the page it returned is the whole collection, each delete asserts 204, and a body arriving without an items array is named rather than left to throw a bare TypeError one line later. A reset that half-succeeded and said nothing would leave the next attempt failing on a stale screen, which is this defect again one layer down and harder to see the second time.

Alongside it, the seven reads of connection-status and download-weights now happen inside their connection row instead of off the page, and the one-connection assumption is asserted once where the count is decided rather than depending on strict mode at seven separate call sites.

Why this shape

The issue that raised this offered two repairs and one retreat, and none of them is what landed. The walk could have tolerated a workspace that is not fresh, reaching the create dialog from the project list when the invitation is absent — but that costs a branch whose second arm runs only on a retry, and gives up the new-installation route on exactly the run somebody is debugging. The workspace could have been rebuilt per attempt, which runs into webServer being Playwright's to manage. Or retries could have gone to zero, which is honest and buys nothing back; it would also remove every trace this suite ever captures, since traces are taken on the first retry and nowhere else.

What decided it is that nothing in the walk changed to accommodate the fix. No assertion was weakened, moved or removed, and no branch was added. home-first-run, the classes-first invitation and the "exactly one filled button on the page" check all still mean what they say, and the create-project step still walks the route a person walks on a new installation. That is the property a reviewer should check first, and it is why the reset is unconditional: a repair whose own body ran only on the rare attempt would inherit the property of the defect it repairs — a path nothing exercises until the day it matters. Running every time, its two reads are proved by every run of the suite.

The two issues alongside it, which close in different ways

The connection name is not scoped per repetition (#691) asks for the connection name to be scoped the way projectFor scopes the project name. With an empty workspace at the start of every attempt no name collides and none needs scoping, so it closes because the collision cannot occur — not because any name moved. What survives from it is the comment correction it also asks for: projectFor's closing claim that the project is "the only name that has to move" was already untrue of the connection name when it was written, and the paragraph justifying its suffix by a POST /projects → 409 described a wall that no longer stands. Both are rewritten; the suffix itself stays, because it costs nothing and makes a failure message name the attempt it came from.

Three cycle-walk assertions assume exactly one connection (#692) is independent of that by its own argument — any future second connection reaches it, whatever put it there — so an empty workspace is not an answer to it, and it gets the fix it asks for. connection-status and download-weights are rendered inside a connection row, so each read is now scoped to its row; the connection is renamed mid-step, so the two halves of that step address it by the two names it has. The suggest panel's assertion is left alone, since its subject genuinely changes shape with the count, and a toHaveCount(1) on the row's type badge now states the precondition where the count is decided. A second connection therefore fails as a count, on the screen that produced it, instead of as a missing string on the suggest panel two hundred lines later.

Test plan

Three runs, each answering one claim, plus the full gate.

A repetition can complete against a used workspace. --repeat-each=2 with CI=1: 2 passed. This is also what answered the open question about content-addressed re-ingest — the store already held the three fixture images' content, and the second repetition ingested them into its new dataset without complaint.

A retry reaches the failure instead of the first-run screen. A deliberate failure was injected as the first line of the walk's final step and the suite run with CI=1. Both the first attempt and the retry failed at that line, inside "the whole walk produced a clean console". Before this change the retry failed instead at the home-first-run assertion in the walk's third step, having reached nothing. The raw server log for that run shows the retry's DELETE /projects answering 204 and a fresh POST /projects answering 201 before the walk begins again, which is the reset doing the thing the report claims for it. The injection was reverted by git apply -R on its recorded diff and the tree confirmed clean.

Nothing in the walk regressed. The suite run plainly after the row-scoping commit: 1 passed, no retry, no strict-mode violation, no count failure.

The full local gate is green on the rebased tip, run as five separate scripts/check.sh invocations — python, generated, frontend, browser and docs — each reporting check.sh: PASSED. The real-server cycle suite inside the browser stage reports 1 passed (25.7s). The stages are split because the combined run exceeds this environment's command timeout.

The rebase matters more than usual here. Two commits landed on main during this work that touch paths this walk drives: the active-schema compatibility check on release publish (#694), and the schema preview narrowing before publish (#695). The browser stage was re-run against both rather than carried forward from the earlier base, since the walk publishes a release and drives the schema editor. Its release publish answers 201 with the verify and export that follow it, so neither refusal collides with it.

An earlier run of the gate, on the previous base, reported two failures that did not recur here and are not this branch's doing. Both interruption cases in tests/scripts/test_cooldown_lockfile.py failed on assert 0 == 130, a SIGINT losing a race under full-suite parallel load; and frontend/ui-core/src/annotator/addClass.test.ts raised an unhandled dispatchEvent from a focus-scope timer firing after teardown. Both pass in isolation in this worktree and on main, and neither file is in this diff, which is two Playwright specs under frontend/app/cycle/.

Found, not fixed

A refused API call the app itself made is invisible to this suite. Every response at or above 400 on a fetch or xhr is swallowed into the apiRefusals set, which exists only to suppress the console errors Chrome logs for them and is never asserted against. The walk contains such calls by design — a schema read answering 404 is how the editor knows to open on an empty draft — but nothing distinguishes those from a genuine 500 raised by the product. One is happening today: GET /inference/download-size answers 500 on an installation without the optional inference extra, which is what the cycle job runs. A walk whose final step is titled "the whole walk produced a clean console" ought to have an opinion about that, and it currently has none.

Nine code comments in cycle.spec.ts cite an issue number, against this repository's rule that a comment states the reason itself rather than pointing at a ticket a reader in an editor or a fork cannot follow. All nine predate this branch and none is in this diff, so rewriting them here would bury a change whose value is that it can be read in one pass. Worth a sweep of its own — and in the same pass, cycle.spec.ts:1510 carries a stranded docstring describing a function forty lines below the one it sits above.

The issue's own body cites cycle.spec.ts:195 for the assertion the retry died on. That was accurate when the issue was filed and is now 205: the connection-edit coverage that gave the walk its edit step (#510) added a constant near the top of the file and pushed everything below it down by ten lines. The issue is left as written, since it is a record of what was true then; this is the correction beside it.

The scoping traded seven implicit checks for one explicit one. Before this change each unscoped read was, in effect, a one-connection assertion, because Playwright's strict mode refuses a locator matching two elements — seven checkpoints spread across the walk. That is now asserted once, deliberately, where the count is decided, and nothing re-checks it in the edit step. The walk creates no connection in between, so this is the shape the design asked for rather than an oversight, but it is a trade and it should be visible.

The reset can delete a project out from under a running worker. If an attempt dies while a weights download, an ingest or an export is still live in the server process, nothing refuses the delete: ProjectService.delete raises only ProjectNotFound and ConfirmationRequired, the cascade is the database's own, and no guard on that path asks whether a job is running. Asserting 204 is right either way, and what this leaves is a worker erroring in the server log rather than a reset that lies about having completed. If it turns out to happen in practice, the answer is a documented wait before the first delete.