Fix the tests failing on main - #208
Merged
Merged
Conversation
c0b60f9 (CL-6462) reordered DEFAULT_WORKFLOWS so the setup agent deploys first, but seed.test.ts still asserted the old echo-first order, failing on every run since.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diagnosis
packages/cli/test/seed.test.ts — fixed. Genuinely broken, not order-dependent in the test-execution sense.
c0b60f90("Land in Myra's room as soon as she is live, not when the seed set finishes", CL-6462) deliberately reorderedDEFAULT_WORKFLOWSinpackages/hub-client/src/seed.tsto deployassistantbeforeecho(documented in the source comment above the array), butseed.test.tswas never updated and still asserted the oldecho-first order. Fixed the twotoEqualexpectations to match the current, intended order. Fullpackages/clisuite is green (27/27).apps/web/test/library-page-selection.test.tsx — could not reproduce as failing. Ran it standalone (3x) and as part of the full
apps/websuite via the realtestscript (bun run build && bun test ./src ./test, the same command CI uses) — 774/774 pass, 0 fail, every time. One real gotcha: this file needsapps/web'sbunfig.tomlDOM preload, so runningbun testfrom repo root instead of fromapps/web/gives a misleadingdocument is not defined— not the failure the reviewer saw, but worth knowing if anyone else tries to repro. I could not find a shared-mutable-state leak or any other cause. Did not touch this file. If it's still red on a fresh main-HEAD CI run, it's likely CI-environment-specific (parallelism/memory pressure) rather than a code defect — worth a second data point from an actual CI log before spending more time here.scripts/e2e/chat.test.ts — could not run, static analysis only. Ports 3100/3199 were already LISTEN-bound (another lane's live stack), so per the machine notes I did not attempt to start a second stack. Read through the file for the "run-id mismatch" pattern (an assertion using an id it never captured) but didn't find one on inspection — the one prior fan-out rewrite (#199) already captures
echoLocalPartfrom the actual participant response before polling/workflows/runs/:runId/eventswith it. I don't have enough confidence from static reading alone to claim this one is fixed or to identify the exact flake; it needs an actual run against a free stack to observe the mismatch and pinpoint the id that's captured too early/late.What's in this PR
packages/cli/test/seed.test.ts: assert the currentassistant-firstDEFAULT_WORKFLOWSorder (matches CL-6462 intent, no production code changed).Verified
packages/cli: 27/27 passapps/web: 774/774 pass (viabun run test, build+test)bun run lint: clean (prettier clean, 0 eslint errors, only pre-existing unrelated warnings)Not verified / needs a peer
apps/web/test/library-page-selection.test.tsxred on CI despite passing locally every time I ran it — needs a real CI log to compare against, or someone to try reproducing under CI-like resource constraints.scripts/e2e/chat.test.ts— needs an actual run against a free stack; I only did static analysis.Do not merge — for peer review per the CI-recovery task.