[repro] Bisect: known-good base + #3198 only - #3238
Conversation
🦋 Changeset detectedLatest commit: d4f34fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests▲ Vercel Production (3 failed)hono (1 failed):
nextjs-turbopack (1 failed):
sveltekit (1 failed):
📋 Other (2 failed)e2e-vercel-prod-nest (1 failed):
e2e-vercel-prod-tanstack-start (1 failed):
E2E Test SummarySummary
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 📋 Other
✅ vercel-multi-region
|
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 388501ms → this run 413359ms (Δ +24858ms, +6%) ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
There was a problem hiding this comment.
Pull request overview
This PR isolates the behavioral change from #3198 on top of a known-good base to help bisect the “hook-storm” regression, by adjusting the core idle-check logic so it does not observe “idle” while a committed (self-resolving) delivery is still parked between its queue slot and its detached resolve() continuation.
Changes:
- Add an idle-check guard that also considers the delivery-barrier registry (not just
pendingDeliveries) to prevent prematureWorkflowSuspensionwhile deliveries are parked mid-deferral. - Document the required invariant for armed delivery barriers (armed barriers must have an unconditional delivery chain).
- Add regression tests covering the suspension timing case (parallel steps + fire-and-forget sleep), and add a changeset for the core patch.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/private.ts | Extends scheduleWhenIdle to wait out parked committed deliveries using the barrier registry; documents the barrier invariant. |
| packages/core/src/delivery-barrier-coverage.test.ts | Adds suspension-timing regression coverage reproducing the parked-delivery/idle race and a serial control. |
| .changeset/idle-check-parked-deliveries.md | Records the @workflow/core patch-level change for release notes/versioning. |
Comments suppressed due to low confidence (1)
packages/core/src/delivery-barrier-coverage.test.ts:660
- Same as above: prefix this fire-and-forget
sleep()withvoidso it’s clearly intentional and doesn’t trip floating-promise linting (consistent with other tests, e.g.packages/core/src/abort-consistency.test.ts:634-642).
sleep('25m').catch(() => {});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const error = await replay(ctx, async () => { | ||
| // Watchdog: arms the idle check on every replay, completes on none. | ||
| sleep('25m').catch(() => {}); |
Event Log Race Repro1138 of 1400 latest repro runs hit event-log regressions. 2 runs hit harness-side Run History
Latest Scenario Breakdown
Latest Non-Completed Runs
Showing 20 of 1140 non-completed runs. |
VaguelySerious
removed
the
event-log-race-repro
Storm bisect for the hook-storm regression that entered main in e8934ad..b12f248 (replicated on two independent branches: 145/600→596/600 and 95/600→600/600 across the same merge). This branch = known-good e8934ad + cherry-pick of #3198 (barrier idle/parked-delivery change) ONLY. Companion: base + #3190 only. Known-good base measured hook-storm 145/600 (run 30494191064). Not for merge.
🤖 Generated with Claude Code