Repository navigation
fix(cloud-agent-next): stop failing a positively running attach - #6268
Merged
Merged
Conversation
reconcileSessionOperation required every running operation carrying a wrapper executionDeadlineAt to persist that deadline, but the only persistence helper for it is prompt-only and always returns undefined for an attach. A positively running attach therefore always reconciled as unverified, became runtime_unhealthy "Original operation outcome is unconfirmed" with retryable: false, and failed the turn as environment_failed while retiring a healthy runtime. - apply the execution-deadline persistence requirement only to session.prompt, so a running attach stays running and is observed to completion instead of being terminalized while the wrapper is still preparing - emit a session_operation_reconcile diagnostic carrying reason and lookupState on the missing and unverified returns, so the remaining unconfirmed branches are decidable instead of inferred
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
marius-kilocode
approved these changes
Sep 17, 2026
This was referenced Sep 18, 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
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 positively running
session.attachwas always reconciled as an unconfirmed outcome, so the turn failed asenvironment_failedand a healthy runtime was retired while the wrapper was still preparing the session.reconcileSessionOperationrequired every running operation carrying a wrapperexecutionDeadlineAtto persist that deadline, but the only persistence helper for it (recordSessionOperationExecutionDeadline) is prompt-only and returnsundefinedwhen the authorization is notsession.prompt. For an attach the supplied closure therefore reportedfalse, reconcile returned{state:'uncertain', reason:'unverified'}, andoperationDispatchErrormapped that to a non-retryableruntime_unhealthy— "Original operation outcome is unconfirmed".Observed on a control-plane canary session (
workspace_512d5b8f) with no agent activity yet: the wrapper had been streaming preparationstep_outputfor 77 s and heartbeating normally, yet 77 ms after the attach dispatch's transport error the message wentqueued -> failedwith causeenvironment_failed, and the sandbox was stopped 88 ms later. Both turns of the session failed this way. The existingreconciles an ambiguous attach through the existing operation receipt pathtest already built this exact combination — attach authorization, dispatched proof, running lookup carryingexecutionDeadlineAt— but asserted only request counts, so the message failure was invisible.Change
session.prompt. A running attach now staysrunning(session_busy, retryable) and is observed to completion inside the original head deadline instead of being terminalized.session_operation_reconcilediagnostic carryingreasonandlookupStateon themissingandunverifiedreturns, so the remaining unconfirmed branches are decidable instead of inferred. The transport branch and the two post-completion returns are deliberately left uninstrumented.Not changed
session.attachRPC retry was prototyped and dropped: fix(cloud-agent-next): recover an attach the replacement runtime never saw #6225 already recovers a lost attach at the reconcile layer (releaseUnconfirmedAttachplus a fresh dispatch in the same delivery), so the retry added no functional gain.Verification
vitest runon the touched suites: 373 passed.pnpm test:integration(Workers runtime, 35 files): 701 passed. Repeated runs under full parallel load show 5 s timeouts insandbox-terminal.test.ts; that suite passes 31/31 in isolation and does not touch this path.typecheck,lint,format:check,git diff --check: clean.Coverage limit
The fixed behaviour and the new diagnostic were not exercised against a live Worker/DO/wrapper path. Forcing a reconcile while an attach is genuinely still running needs a transport fault, and the levers available locally (
docker pause, a DO eviction via a Worker restart) retire the sandbox beforedispatchSessionOperationre-runs with a dispatched proof — reproduced 3/3 including with no fault injection at all. The fix is therefore covered at unit and DO-orchestration level, and the no-regression path was verified end to end (a real cold-to-hot session with a real sandbox and wrapper attach completing all four messages).