fix(collect-models): wait for the credential write, not for a clock (#1355) - #1357
Conversation
…owing a save that never landed (#1355) `Collect models` failed twice in a row on PR #1348 with locator.click: Timeout 20000ms exceeded - waiting for getByRole('button', { name: 'Save', exact: true }) - locator resolved to <button aria-busy="true" aria-disabled="true" …> The button was not broken: the panel is walked one provider at a time and the PREVIOUS provider's validation was still in flight. `click()` does wait for "enabled and stable", but its ceiling is 20s — shorter than the ~35s Google validation this file already documents — and when it expires the error names the click, sending the reader to the wrong step entirely. It surfaced when the CI `OPENAI_API_KEY` was replaced with a funded key: saving openai used to fail fast on `no credits remaining`, so no validation was ever in flight when the loop moved on. Three changes, one per defect: - `waitForButtonIdle` polls the button until it is genuinely actionable — `aria-busy`, `aria-disabled` and `isEnabled()` are three separate claims and any one of them blocks a click — with a 60s ceiling sized against the validation, and `formatSaveBusyFailure` names the provider, the observed attributes and the likely cause. It returns a verdict rather than throwing, so the caller supplies the provider name and the whole decision is unit-testable without a browser. - The `waitFor(Disconnect, 60s)` after the save no longer ends in `.catch(() => {})`. "Never configured" and "configured fine" were the same observation, which is why an empty model list downstream read as a provider without models. - A provider whose key IS set collecting ZERO models now warns. That state is what produces the silent daily: `Collect models` is `continue-on-error` there (#980), so a `models.json` missing a provider makes every parametrized spec skip and the run reports green having tested no agent — #570/#1012's green-by-absence. Six unit tests drive the wait with an injected clock and a scripted locator: the real failure needs a funded key and a slow backend, but the decision — when it gives up and what it reports — is testable without either. They pin both orderings of the not-idle state, and that a zero timeout still observes once rather than reporting a state it never read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tes stop wedging the next (#1355) The instrumentation from the previous commit named the real cause, and it is not the one that commit assumed. The Save button was not merely slow to settle:⚠️ provider "anthropic" never showed the configured state ("Disconnect") within 60s of Save⚠️ provider "anthropic" has a key configured but collected ZERO models Models found (anthropic): [] Error: the "Save" button for provider "google" never became actionable after 60.0s anthropic's Save was clicked on an IDLE button and never completed. Waiting longer was never going to help — 120s across the two providers and nothing settled. The failure screenshot shows why: OpenAI carries a `52 models` badge while google's Save spins without anyone having clicked it, so the busy state is the form's, not the button's, and the write in flight is anthropic's. What puts it there: enabling a model is a WRITE, and this loop enables every model of every provider as fast as the clicks land. With a funded OpenAI key the panel exposes 41 visible models where a drained key exposed none worth toggling — so one provider went from ~0 writes to 41 against a backend the lanes run with `LANGFLOW_WORKERS=1`. The next provider's Save queues behind them. `waitForToggleChecked` confirms `aria-checked="true"` after each click, which is what serialises the writes. Two bounds, mirroring the token-attribution sidecar (#1197 §4.4): 5s per toggle bounds ONE write, and a 60s per-provider budget bounds the sum — a per-item timeout alone would let 41 slow-but-succeeding confirmations spend 41 x 5s and blow the spec's own 5-minute budget. Past the budget the clicks continue unconfirmed and the count is warned about, never silently dropped (#1012). Four more unit tests, same injected-clock approach; deleting the confirmation kills three of them. Locally the healthy path is unchanged — 23.9s against 24.9s before, 3 providers, 90 models, no new warning fired — because a confirmation on a responsive panel costs tens of milliseconds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lane's own
|
…1355) Reproduced locally with a funded OpenAI key and every `/api/v1/variables/` response logged. The measurement settles it: collect-models: provider "anthropic" credential write took 103.1s (HTTP 201) All three saves answer 201 — none is rejected, none is refused. anthropic's POST simply does not come back for ~103s, and the panel keeps its Save `aria-busy` for as long as a write is in flight. So the previous two commits were both measuring the same thing from the outside and calling it different names: the 60s button wait, and the toggle serialisation, were guesses at a duration. Waiting for the RESPONSE removes the guess. `page.waitForResponse` is registered before the click (or the response can land first and be missed) and resolves the moment the write answers, with a 180s backstop set well clear of the worst measurement rather than just above it — the cost grows with how many providers are already configured, which is why any fixed clock here eventually expires again. The outcome is reported three ways, none of them silent: no write observed, a write that answered non-2xx, and a write that succeeded but took over 10s — the last one is a trend line, so the next time this cost approaches the ceiling it is visible before it crosses it. Verified on a fresh container with `--retries=0`: `1 passed (2.6m)`, all three providers collected, where the same command previously left anthropic at zero models and died on google's Save. The toggle serialisation from the previous commit stays, with its comment corrected to say what it is: it was added on a hypothesis the next CI run refuted, it fixed nothing, and it is kept only because a burst of 41 unconfirmed writes against a LANGFLOW_WORKERS=1 backend is worth avoiding at a measured cost of ~0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #1355.
The measurement
Reproduced locally with a funded OpenAI key, a fresh container and every
/api/v1/variables/response logged:All three credential writes answer
201. None is rejected, none is refused. anthropic's simply does not come back for ~103 s — and the providers panel keeps itsSavemarkedaria-busyfor as long as a write is in flight. The next provider's click then lands on a busy button.So the failure was never a broken button, a bad key, or a rejected save. It was a fixed clock waiting on a variable duration, and that duration grows with how many providers are already configured.
What it looked like before
Why it started now: the CI
OPENAI_API_KEYwas replaced with a funded key. A drained key failed fast; a funded one makes the save real, and the panel exposes 41 visible models (52counting the collapsed deprecated ones) where it exposed none worth toggling.The fix
page.waitForResponsefor the credential write, registered before the click (or the response can land first and be missed), resolving the moment the write answers. The 180 s backstop is set well clear of the worst measurement rather than just above it, precisely because a fixed clock here is what failed.The outcome is reported three ways, none silent: no write observed, a write that answered non-2xx, and a write that succeeded but took over 10 s. The last one is a trend line — the next time this cost approaches the ceiling, it is visible before it crosses it.
Two supporting changes from the earlier commits stay:
Collect modelsiscontinue-on-errorindaily-stable.yml(ci: make the provider-key resolver warn-only in the daily (#976) #980), so a missing provider makes every parametrized LLM spec skip and the daily reports green having tested no agent — collect-models: single inaccessible lead model disables whole provider (16 OpenAI agent tests silently skipped) #570/shardguard counts blob files, not tests — a 0-test run reaches triage looking benign #1012's green-by-absence.LANGFLOW_WORKERS=1backend is worth avoiding at a measured cost of ~0 (23.9 s against 24.9 s).Verification
--retries=0:1 passed (2.6m), all three providers collected — where the same command previously left anthropic at zero models and died on google's Save. This is the reproduction and the proof, on the same machine.tsc --noEmitclean; ESLint 0 errors (7 pre-existinganywarnings in this file).The lane's own
Collect modelsis the CI-side confirmation.🤖 Generated with Claude Code