Reclaim workflow concurrency slots from abandoned executions

[xBalbinus]

What

A workflow_executions row occupies one of the user's ten concurrency slots for as long as its status is in ACTIVE_EXECUTION_STATUSES. Before this change, only two things ever wrote a terminal status: the runtime, from inside the live Cloudflare Workflow instance, and the cancel pipeline. When neither ran, the row stayed active permanently. Four changes close that:

• interpreter.ts wrapped runWorkflowDag in a try/catch that finalizes the row as failed before rethrowing. The original error is always rethrown unchanged, and a failed finalize is logged rather than raised, so a broken D1 cannot mask the real error. Ordinary node failures are unaffected — those return a result and persist their own terminal status.

• runtime.ts terminal write now accepts any active status as its prior, not just running. A run that reached the end while its row was parked in waiting_approval or waiting_time was silently no-op'ing its own terminal write and leaking the slot — a leak on the success path, not only on crash. cancelling/cancelled stay excluded so a competing cancel still wins.

• stale-execution-sweep.ts (new, registered on the minutely cron) is the backstop for instances that never run code again. It selects active rows past a 10-minute floor oldest-first and asks the Workflows binding for each instance's state. Elapsed time only bounds the query; the instance decides, so a legitimately long-running node is never touched.

  It fails closed. Finalization requires either a terminal instance state (complete → completed, terminated → cancelled, errored → failed) or an error that positively identifies the instance as missing. Any other probe failure leaves the row for the next tick and logs the message verbatim. A wrong verdict here is unrecoverable — the row would read terminal while the instance kept running, its own terminal write would no-op against the CAS, and the spawned-session sweep would tear down its sandboxes.

• nodes/approval.ts status step keys now carry the :i:N foreach iteration suffix the invocation id already used. Without it every iteration reused iteration 0's memoized step.do result, so an approval inside a foreach stopped tracking waiting_approval and could strand the row.

No schema change. The sweep is registered in EXPECTED_INTERVAL_MS and reports its count through cron_heartbeats, and it logs every tick rather than only when it reclaims — an inert sweep and a healthy one must not look identical.

This also reclaims executions that are unreachable through the cancel API: workflows.id is ON DELETE SET NULL, and the cancel endpoint resolves through workflow_id, so rows whose workflow was deleted 404 for users and operators alike.

Why

Reported from prod: a user was rejected with Too many concurrent workflow executions while only four runs of one workflow were in flight. The cap is 10 and counts every active execution across all of that user's workflows, so the remaining slots were held by rows that had leaked — nothing reclaims an execution whose instance died, and each occurrence lowers the user's effective limit by one until they cannot start work at all.

Existing leaked rows are drained without a migration or backfill: every pre-existing leak is past the age floor and the whole candidate set fits in one tick, so the first sweep after deploy is a full pass. Rows whose instance is still genuinely parked recover when the node hits its own bound, which now lands because of the widened CAS.

Two classes are deliberately not reclaimed: an instance paused from the Cloudflare dashboard, and one reporting unknown. Both are indistinguishable from live work, and the cost of guessing wrong on a live run is the run.

Test plan

Full worker suite and tsc --build pass locally; the new tests cover each of the four defects, including that a degraded Workflows binding reclaims nothing.

• CI green
• New/changed behavior covered by tests

Checklist

• Subsystem behavior changes: matching spec in docs/specs/ updated in this PR

[valet-valet-turnkey-dev]

The PR is well-reasoned and the implementation is careful, but there are two real issues and one question worth raising.

1. packages/worker/src/workflows/stale-execution-sweep.ts:200 — isInstanceNotFound contains a message-text fallback that can fire on unrelated errors, undermining the "fail closed" guarantee the whole sweep is built around.

   return typeof message === 'string' && /not found|does not exist|no instance/i.test(message);

   • The regex matches generic HTTP/fetch errors ("resource not found", "socket does not exist"), a D1 error about a missing row, or any third-party middleware message that happens to include those words. Any of those would be misread as "instance is gone" and trigger the destructive finalization the function was designed to avoid.
   • The comment says the message match is "a hedge in case the code is absent." But if the code is absent you don't know it's a Cloudflare not-found error — the hedge undermines the very invariant it's hedging for. The right answer is: if code !== 1001, throw (leave for next tick). Drop the regex branch entirely, or narrow it to a more specific string Cloudflare actually emits and document the exact source.

2. packages/worker/src/workflows/stale-execution-sweep.ts:176–179 — instance.status() result is destructured as { status, error }, but the error field (used to enrich the error message at line 189) is assumed to have shape { message?: string }. There is no type guard or null check before accessing error?.message.

   • If the platform ever returns error as a string or another shape, error?.message silently resolves to undefined and the context is lost with no warning. Given that this is untrusted external API data, at minimum add typeof error === 'object' && error !== null && before error.message, or just stringify it.

3. packages/worker/src/workflows/execution-status.ts:129–140 — The CAS uses a completedAt timestamp as the write receipt, then reads it back to check if the write landed. This is elegant but has a subtle race: two concurrent callers (e.g., the interpreter catch and the sweep arriving in the same minute) generate different timestamps; the second write's CAS will no-op on the inArray check because the row is already terminal after the first, so after?.completedAt won't match. That means the second caller correctly returns false. However, if two callers race and both find the row in an active status simultaneously (before either write commits), both writes pass the inArray WHERE clause, the second overwrites the first, and only one caller sees true. This is acceptable for a best-effort finalization (both write the same status: 'failed'), but worth confirming: does the caller that sees false in this race do anything harmful, like logging a spurious warning? The interpreter catch discards the return value, so it's fine — but it's worth a glance at any future caller.

---

Created on behalf of Xiangan He xiangan@turnkey.io

[github-actions]

Preview deployment: https://pr-218.dev-valet-turnkey-client.pages.dev

[xBalbinus]

Addressed in f4ad30b0.

1. Message-text fallback in isInstanceNotFound — fixed, and the point is well taken: a substring match is not positive identification, which is the entire premise of the fail-closed design. The regex is gone; code === 1001 is now the only thing that authorises treating an instance as missing. An error we cannot attribute is exactly an error we should not act on, so it falls through to the caller, which leaves the row and logs the message verbatim — that log is how the real error shape becomes visible in production rather than being guessed at here. Added cases pinning that resource not found, socket does not exist, and 404: not found all leave the row untouched.

2. Unguarded error?.message on InstanceStatus — fixed. Extracted describeInstanceError, which handles the declared {name, message} shape, a bare string, an unrecognised object, and null. The declared type says {name, message}, but it crosses a platform boundary and silently losing the detail is the failure mode this sweep exists to stop repeating. Covered for all four shapes.

3. Concurrent-finalization race — checked; nothing harmful downstream. The interpreter discards the return value, and the sweep uses it only to increment its reclaimed counter, so the losing caller in that race undercounts by one and does nothing else. The two callers also cannot write divergent outcomes in practice: the interpreter's catch only runs when a throw escapes the wave loop, which errors the instance, so the sweep's probe would independently return errored → failed. A complete instance means no throw escaped, so the catch never ran. Left as is rather than adding coordination for a cosmetic counter.

Full worker suite and tsc --build pass.