Uh oh!
There was an error while loading. Please reload this page.
Review 2: harden Yieldstar deployment scoping and coordination visibility - #39
Closed
djgrant wants to merge 1 commit into
Closed
Review 2: harden Yieldstar deployment scoping and coordination visibility#39djgrant wants to merge 1 commit into
djgrant wants to merge 1 commit into
Conversation
- Encode the deployment segment of resource store IDs so prefix listing in values() and clear() cannot match another deployment whose ID extends this one past the delimiter, which previously allowed cross-deployment orphan deletion. - Emit a durable reconciler.coordination.waiting warning naming the holding execution before an execution suspends on the deployment coordination store, so waiting behind a crashed holder is visible. - Give notation destroy the same terminal error handling as deploy: credentials guidance and a non-zero exit instead of an unhandled rejection. - Add regression tests for deployment store scoping and the coordination waiting event; update reconciler docs.
djgrant
commented
Jul 22, 2026
CollaboratorAuthor
Closing this review-only PR. Its fixes will be carried into the simplify implementation PR stacked on #38. |
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 freeto 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.
Second independent release review of the Yieldstar 0.5.0 integration (follows PR #38). Semantics were verified against the installed Yieldstar 0.5.0 source rather than assumed.
Fixes
YieldstarStateBackendbuilt store IDs asdeploymentId:resourceIdand filteredvalues()/clear()by the rawdeploymentId:prefix, so deploymentappmatched stores of deploymentapp:blueand would list and delete them as orphans. The deployment segment is now URI-encoded so the delimiter cannot occur inside it. Regression test covers listing and clear isolation.store.takeparked the new execution forever with no output. Both deploy and destroy now emit a durablereconciler.coordination.waitingwarning naming the holding execution ID before suspending. Regression test covers the event.notation destroynow matches deploy: credentials guidance and exit code 1 instead of an unhandled rejection.Verified sound against Yieldstar 0.5.0 source
try/finallyrelease: suspended executions are abandoned withoutgenerator.return(), sofinallydoes not run and the holder is correctly retained across durable waits; terminal errors thrown back into the workflow do run the durable release.step.storekeys derive from store name and ID, so loop usage is safe under the driver's duplicate-key check.updateFrom/deleteFrom/takeare recorded in the applied-steps ledger in the same transaction as the state change, closing the store-commit/heap-write crash gap; whole-store version plus UUIDv7instanceIdCAS matches the release intent.retryLaterOnErrortoRetryableErrorwith correct drivermaxAttemptssemantics; crash-resume, destroy ordering, and orphan handling are exercised by the existing suite.Checks
Build, typecheck, and the full test suite pass (77 passed, 4 skipped).