Conversation
bun-version: latest let the gate change with no commit and no review, and it did. bun 1.4.x takes `a persona whose credentials are wrong` (session-builder.test.ts, ceiling STORY_TIMEOUT_MS = 30_000) from ~2s to ~30s. Every 1.4.x run lands between 29.6s and 30.0s, so a pass wins by 30-420ms and is noise rather than health: 2 of 8 recent attempts passed, main is red, and release PR #69 burned nine runs without a merge. 1.3.14 is the last version measured fast here, at 1,962ms on run 31238151616. The break is the toolchain and not a commit: every 1.3.x run is ~2s and every 1.4.x run is ~30s, across unrelated branches. release.yml is pinned to match. That job runs `bun run build` and publishes the result with no test step, so a toolchain regression there reaches npm with nothing in between. This pins the version; it does not diagnose the 1.4.x slowdown, which stays open as #70. Deliberately NOT done: raising STORY_TIMEOUT_MS or quarantining the story, either of which would go green over a path that still takes 30 seconds and turn a visible 15x regression into an invisible one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used🪛 zizmor (1.29.0).github/workflows/release.yml[error] 45-45: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default (cache-poisoning) .github/workflows/ci.yml[warning] 19-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) [warning] 13-79: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) 🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe CI and release workflows replace Bun ChangesBun toolchain pinning
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to CI and release now use the known-good Bun 1.3.14 toolchain, restoring deterministic behavior without changing test timeouts or release publishing configuration. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The workflows pin Bun to exact version 1.3.14, preserve STORY_TIMEOUT_MS, and do not skip or quarantine the affected test. The provided context does not verify ten consecutive green CI runs or confirm the follow-up tracking for the Bun 1.4.x slowdown. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Holding this rather than merging, and correcting what I claimed in the description. The pin does what it says: But it does not make CI reliably green. On the second pinned run, the other persona story — Worth landing on its own merits once someone decides that, but it is not the unblock I opened it as. |
|
Closing. The pin is the wrong fix and the evidence is now in #70. Pinning to 1.3.14 does remove the bun 1.4.x hang in So this trades an occasional red for a reliable one. Keeping The isolation experiment behind that conclusion was #72, now closed: pre-dual-read tree on bun 1.4.2 still hangs, which exonerates the dual-read for the 1.4.x half and implicates it for the 1.3.14 half. |
1 participant
What
Pins
bun-versionto1.3.14inci.ymlandrelease.yml, replacinglatest.Why
latestmeans the gate can change with no commit and no review. It did.a persona whose credentials are wrong fails the run instead of opening it signed out(src/services/session-builder.test.ts, ceilingSTORY_TIMEOUT_MS = 30_000), duration from each run's own log:d8affa0b9c5a754106ec3a07e032d66645abb1a877081f8dPlus five more failures on
77081f8dfrom re-runs, all 30,0xx ms.Every 1.3.x run is about 2 seconds. Every 1.4.x run is between 29.6 and 30.0. The passes win by 30 to 420 ms under the ceiling, which is noise, not health — so re-running until green merges a suite sitting milliseconds from the edge. Two of eight recent attempts passed,
mainis red, and release PR #69 has burned nine CI runs without merging.The break is the toolchain, not a commit.
9c5a7541, the first slow run, is a config-loader branch that cannot make a login test 15x slower, and the two commits blamed since are both exonerated by the same table (details in #70).release.ymlis pinned to match because it runsbun run buildand publishes the result with no test step — a toolchain regression there reaches npm with nothing in between.Changes
.github/workflows/ci.yml:bun-version: latestto1.3.14, with a comment naming the measurement, the run id, and the condition for bumping it..github/workflows/release.yml: same pin, pointing at that note.Verification
The proof is this PR's own CI run. If the diagnosis is right,
a persona whose credentials are wrongreturns to roughly 2 seconds instead of landing within 400 ms of its ceiling. A green that still takes 29-plus seconds would mean the pin is not the fix and this should not merge.actionlintreports only the pre-existing unknown-label warning for the Blacksmith runner, unchanged by this diff. Both files parse as YAML.What this deliberately does not do
STORY_TIMEOUT_MSand does not skip or quarantine the story. Either would go green over a path that still takes 30 seconds, converting a visible regression into an invisible one.npm install -g npm@latestinrelease.yml. Trusted publishing needs npm >= 11.5.1 and that line exists to satisfy it; narrowing it is a separate decision.Closes #70 only in part — the pin, not the diagnosis. Leaving #70 open.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit