Skip to content

Lint errors should steer agents away from common spinner-waiter workarounds #30

Description

@mmkal

Field report from iterate/iterate#2492: an agent (me) enabled the full lint plugin on ~91 legacy violations and reached for every wrong move before landing on the right ones. Each was eventually corrected by human review. The lint error messages could have prevented most of them.

What I got wrong, in order

  1. Escape-hatch comments claiming UI doesn't exist without checking. For prefer-positive-waits I wrote "the decided batch vanishes with no replacement UI" — false: an outcome badge existed in the expansion and a ✓/✗ glyph on the collapsed card header. The message could say: before writing the exception comment, hunt for existing positive UI (badges, glyphs, status text) — and if there truly is none, consider adding it to the product first.

  2. Inline timeout: over real spinners instead of raising the ceiling. Cold-build waits (120s) had honest data-spinner loading UI the whole way, but I annotated inline timeouts because the wait exceeded spinnerTimeout. The right move was spinnerWaiter.settings.run({ spinnerTimeout: 130_000 }, ...). The require-timeout-comment message never mentions the override exists.

  3. Trusting stale settings.run({ disabled: true }) blocks. Two specs sat the middleware out citing a strict-mode isVisible crash on multiple simultaneous spinners — fixed upstream in videoMode: autoStart — trim the blank startup lead-in #3 a week after the disables were written. I annotated around the disables instead of questioning them. Consider a rule (or a note in the README) that disabled: true needs a dated reason so staleness is detectable.

  4. Assuming the no-spinner fast-fail bites when it doesn't. I kept an expect(locator).toBeVisible() workaround (with disable comment) for a spinnerless React transition that completes well inside the 1s readiness quick-check — plain waitFor() worked fine. The prefer-locator-waits message could state the quick-check window explicitly so agents can reason about when the fast-fail actually applies (roughly: spinnerless waits are fine under ~1s; beyond that add loading UI).

Suggested message edits

  • require-timeout-comment: mention spinnerWaiter.settings.run({ spinnerTimeout }) as the preferred fix when loading UI exists but outlasts the ceiling; keep inline timeouts for waits the middleware can't see (raw pages, expect.poll, event waits).
  • prefer-positive-waits: "look for existing positive UI before writing the exception; prefer adding product UI over documenting its absence."
  • README's spinner-waiter section: document the 1s readiness quick-check and the multi-element-safe spinner check (post-videoMode: autoStart — trim the blank startup lead-in #3), so "it fast-fails me" and "it crashes on two spinners" stop being reasons to disable.

🤖 Filed by Claude Code from session 99f7d462-f777-4a97-8825-90660a32001a.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions