CL-6640: quarantine a missing closure staging dir instead of crash-looping - #311
Merged
Conversation
Fabricates the exact wrapped-ENOENT shape readPackageJSON throws for a missing/incomplete closure staging directory and proves the boot restore loop quarantines it as a permanent failure without stopping, and that a sibling deployment's own restore is still attempted every boot.
…e failure Boot restore was crash-looping: an ENOENT reading a freshly-staged closure's package.json fell through restoreDeploymentFromRecord's default "transient" classification, so it was retried every boot without ever quarantining. applyFrozenWorkflowClosure re-materializes into a brand-new deploy-id directory on every call and reads back from it in the same await chain, so a missing file there is never this boot's timing -- it is corrupt/incomplete persisted input (the tarball cache, or a durable source-asset checkout) that reproduces identically on every future retry. Classify it permanent so it quarantines after RESTORE_QUARANTINE_THRESHOLD attempts instead of warning forever.
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.
Summary
Fixes the P0 sidecar boot crash-loop from CL-6640: an ENOENT reading a
freshly-staged workflow-definition closure's
package.jsonwas fallingthrough the boot restore loop's default "transient" classification in
restoreDeploymentFromRecord(apps/sidecar/src/workflow-host-wiring/index.ts),so it was retried on every boot without ever quarantining.
The escape path
applyFrozenWorkflowClosure(apps/sidecar/src/workflow-closure-apply.ts)re-materializes a deployment's closure into a brand-new
packages/<uuid>/directory on every single restore call (a fresh
crypto.randomUUID()eachtime -- it never persists or trusts a stale directory reference) and reads
the package back in the same
awaitchain. So an ENOENT there is neverthis boot's timing -- it is corrupt/incomplete persisted input (the
shared tarball cache under
workflow-definition-closure-cache/, or adurable source-asset checkout
resolveDeploymentAssetMountsalreadyvalidated) that reproduces byte-for-byte on every future retry. The call
chain is fully
awaited insiderestoreDeploymentFromRecord's try, whichthe boot loop's outer
try/catchdoes classify -- but as a plainError,which falls to the default "transient" bucket, so it never crosses
RESTORE_QUARANTINE_THRESHOLDand warns forever while making no progress.Fix shape
Wrapped the
applyClosure(...)call and addedisMissingClosureStagingFailure,which recognizes
readPackageJSON's (@intx/workflow-host, published --not touched) wrapped-ENOENT
Errorshape and rethrows it asWorkflowRestoreFailure("permanent", ...). This is the smaller correctfix: the closure-apply path already re-derives from source fresh on every
call (no persisted staging pointer to make "durably complete"), so nothing
needed to change there -- only the classification of a failure that recurs
identically every time.
Test plan
apps/sidecar/test/workflow-restore-quarantine.test.ts: new testfabricates the exact wrapped-ENOENT shape via the injectable
materializeDeploymentClosureseam and proves (a) the recordquarantines as
permanentafterRESTORE_QUARANTINE_THRESHOLDboots, (b) a sibling deployment's own restore is still attempted
every boot, unaffected, and (c)
restoreWorkflowDeployments()neverthrows across any of those boots.
WORKBENCH_CHECK_SINCE=origin/main bun run typecheck-- cleanWORKBENCH_CHECK_SINCE=origin/main bun run lint-- clean (0 errors)bun testinapps/sidecar-- 209 pass, 0 fail (no regressions)