Skip to content

fix(run-engine): don't mislabel DB errors as UnclassifiableWaitpointId in completeWaitpoint#4259

Draft
claude[bot] wants to merge 3 commits into
mainfrom
fix/waitpoint-completion-db-error-mislabel
Draft

fix(run-engine): don't mislabel DB errors as UnclassifiableWaitpointId in completeWaitpoint#4259
claude[bot] wants to merge 3 commits into
mainfrom
fix/waitpoint-completion-db-error-mislabel

Conversation

@claude

@claude claude Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Changelog

WaitpointSystem.completeWaitpoint wrapped the runStore.forWaitpointCompletion(id) call in a broad try/catch that rethrew any error as UnclassifiableWaitpointId. But forWaitpointCompletion resolves the owning store by probing the database (not by classifying the id string), so a transient database/infra error surfaced from that call was being caught and mislabelled.

Before: a transient database problem (e.g. can't reach the database server under load) surfaced as a misleading UnclassifiableWaitpointId — length N matches neither cuid nor run-ops id error. This hid the real cause and lost the underlying error's type, retryability, and error-grouping.

After: the underlying database error surfaces unchanged (keeping its type, retryability, and grouping). Only a genuine id-classification failure produces UnclassifiableWaitpointId.

How: In internal-packages/run-engine/src/engine/systems/waitpointSystem.ts, the catch around forWaitpointCompletion is narrowed to convert only UnclassifiableRunId (the documented classification-failure signal — see RunStore.forWaitpointCompletion in internal-packages/run-store/src/types.ts) into UnclassifiableWaitpointId. Every other error, including DB connectivity failures, is rethrown as-is. A comment explains why DB/infra errors must not be wrapped.


Testing

No behavior change for the existing test paths:

  • The single-DB completion path returns the one store without throwing, so it never enters the narrowed catch.
  • The cross-seam-guard throw tests short-circuit in the engine layer before reaching this catch and assert only that the call rejects; they remain green.

The change is confined to error classification in the store-resolution catch; the id-classification path that produces UnclassifiableWaitpointId is unchanged.


Checklist

  • The PR title follows the convention.
  • I ran and tested the code works

Generated by Claude Code

…d in completeWaitpoint

forWaitpointCompletion resolves the owning store by probing the database, so a
transient DB/infra error surfaced from that call was being caught and rethrown
as UnclassifiableWaitpointId with a misleading "length matches neither cuid nor
run-ops id" message, losing the original error's type, retryability, and error
grouping.

Narrow the catch so only a genuine id-classification failure (UnclassifiableRunId)
becomes UnclassifiableWaitpointId; every other error (including DB connectivity
failures) is rethrown unchanged.
@changeset-bot

changeset-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 2f7fff6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@d-cs d-cs self-assigned this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants