Skip to content

bugfix: artifact-canvas test 'surfaces a synchronous FileAdapter.watch() failure via onError without throwing (D2)' flakes under CI load (missing waitFor on DOM assertion) #1110

Description

@amrmelsayed

Problem

The test surfaces a synchronous FileAdapter.watch() failure via onError without throwing (D2) in packages/artifact-canvas/src/components/__tests__/artifact-canvas.test.tsx flakes under CI load. It passes 5/5 isolated and 8/8 full-file locally; fails intermittently in CI. Surfaced by the AIR builder for #1108 (PR #1109), which observed the failure on a VS Code-only diff with no artifact-canvas changes (git diff main...HEAD -- packages/artifact-canvas empty on the builder branch).

Root cause (verified against source)

The test waits for onError but synchronously asserts the DOM in the next line:

// packages/artifact-canvas/src/components/__tests__/artifact-canvas.test.tsx
await waitFor(() => expect(onError).toHaveBeenCalled());
expect(document.querySelector('p[data-line]')).not.toBeNull(); // read succeeded → content still renders

The waitFor ensures onError fired, but the very next assertion is a synchronous DOM read with no second waitFor. The async read() render in ArtifactCanvas can resolve AFTER onError fires — under CI load the render is delayed enough that the DOM query lands before the paragraph is in the tree. Race; test fails.

The sibling test immediately below (Disposable.dispose is safe to call more than once (idempotent contract, D2)) uses the correct pattern, which is the same fix to apply here:

await waitFor(() => expect(document.querySelector('p[data-line]')).not.toBeNull());

Fix

Wrap the second assertion in waitFor:

await waitFor(() => expect(onError).toHaveBeenCalled());
await waitFor(() => expect(document.querySelector('p[data-line]')).not.toBeNull());

One-line change. The semantic stays identical (the assertion eventually succeeds), only the timing tolerance widens to match the sibling test's pattern.

Verification

  • Test must pass consistently when the existing test file is run under load (e.g., vitest --runInBand alongside other dashboard suites that compete for the event loop).
  • Existing passing cases in the file must continue passing (no functional change beyond timing).
  • Full pnpm --filter @cluesmith/codev-dashboard test suite stays green.

Out of scope

  • Auditing the broader artifact-canvas test file for other similar await waitFor → bare-assertion patterns. Could ride along if the fixer notices any (the file is small), but not required.
  • Any production code changes to ArtifactCanvas itself. The test is wrong, not the code.

Protocol

BUGFIX. Trivial, isolated, mechanical fix with a clear root cause; no design decisions, no architectural impact.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/dashboardArea: Tower web dashboard package (@cluesmith/codev-dashboard, React app served by Tower)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions