fix(orchestrator): fail closed on cross-conversation run identity - #748
Merged
Conversation
added 6 commits
September 12, 2026 00:37
A Slack waiter used to require requestId but treat every absent scope, session, origin, remoteKey, and target as agreement. The print terminal that triggered the September 11 incident omitted those fields, so a new waiter could adopt an older run's control or completion event. Run-pin matching now requires requestId/origin/scope/session and, when the waiter owns them, the exact remoteKey and target. Producers are filled first: request settlement retains the admission pin, queue start/update and failure events carry it, and Slack terminals expose scope/session/remoteKey even when multi-session mode is off. The collector applies the same rule to Slack orchestrate_done events. A steer can mutate only the conversation that owns the active run. Another remoteKey is queued as a separate follow-up before in-band or kill-steer input is dispatched, and collect batches no longer join different remote keys that happen to share a scope. Preserves the existing boot-drain/orphan target-reply contract and synthetic thread keying. Refs #743
Update queued Slack and collector fixtures to carry the production terminal pin, and lock the new behavior: a foreign remoteKey waits as its own turn, collect does not join another remote conversation, and request settlement preserves the admission target even if its caller mutates the original object.
Independent review found that the new gateway follow-up guard ran before native replaceTurn validation. A foreign remoteKey was queued with the native owner's scope, session, and target, which could store another conversation's prompt in the owner transcript. Native replacements now retain their typed owner-mismatch failure. Non-native in-band and kill-steer paths still queue another remote conversation, deriving the owner key from its captured target when remoteKey is absent. Terminal targets are snapshotted, incomplete targets are omitted from pins, and steer supersession requires the exact origin/session/remoteKey. Refs #743
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 11, 2026
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
1 participant
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.
Problem: a Slack reply waiter accepted completion/control events that named only a requestId. Missing origin/scope/session/remoteKey/target counted as agreement, so a terminal from another run could be adopted. A second conversation could also be steered into an active non-native run or collected with another remote key.
This layer makes run identity carried and fail-closed:
matchesRunPinrequires requestId/origin/scope/session and exact remoteKey/target when owned by the waiter.It also adds the exact #742 race regression (last-active moves while a pinned run finishes) and the #744 stream-loss metric assertion.
Validation: targeted strict-pin/producers 197/197 before audit and 284/284 after audit; final
npm run gate:all23/23. Full root+unit at the combined upper tip: 13,490 tests, 13,469 pass, 20 skip, one load-sensitive Pi discovery timeout; that Pi file passes 19/19 alone and in the 114-test runtime batch. Independent grok-4.6 review: PASS, no merge blocker.Closes #742.
Closes #743.
Refs #744.