Verify rollback ownership across continue-as-new - #57
chaptersix wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39c52cb910
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pending := s.WorkflowMigration.StartPending | ||
| s.WorkflowMigration.StartPending = false | ||
| return pending, nil |
There was a problem hiding this comment.
For a global namespace, this transition and StartWorkflowExecution are persisted and replicated independently under different execution keys. If the initial start_pending=true state reaches the standby but this update does not before failover, the new active cluster can consume the stale claim again; when the first destination has continued as new and its descendant has since closed or been deleted, the ALLOW_DUPLICATE start succeeds and creates a second scheduler chain. The claim therefore does not provide the advertised single-start guarantee across failover without a destination-side fence or another cross-cluster reconciliation step.
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return "", fmt.Errorf("failed to resolve migration destination chain: %w", err) |
There was a problem hiding this comment.
When the claim was consumed but the start failed before committing—or the committed destination is not yet visible—GetMutableState returns serviceerror.NotFound, and this wrapping preserves that type. queues.executableImpl.isInvalidTaskError treats any wrapped NotFound as an invalid task and acknowledges it, so with StartPending already false and an idempotent MigrateToWorkflow call adding no replacement task, the source remains paused and migration-pending with no executor left to observe a later destination or operator repair. Return a retryable reconciliation error instead of propagating NotFound.
Useful? React with 👍 / 👎.
1 participant
An owned V1 descendant changes its start request ID after continue-as-new, causing #47 to reject the handoff. Verify its authoritative first-run identity and original create request. Consume a replicated start claim before the only create attempt, then persist an acceptance receipt before closing CHASM. Retries reconcile ownership or use the receipt without recreating a completed destination.
Missing ownership proof remains fail-closed and requires operator reconciliation. Older persisted migrations are reconcile-only; old executors must be drained before relying on this protocol.
Evidence, sequence diagram and recovery boundary. Focused ownership and fault-boundary tests pass; make proto completed. Affected scheduler packages and V1 replay tests pass. The native History continue-as-new test passes in both replication modes.
make lint-code-fastreports zero issues; repeatedmake protoproduces no generated diff;git diff --checkpasses. A bounded three-second migration fuzz smoke run passes (75 cached baseline inputs exercised). Physical crash and namespace-failover integration tests were not run.