Conversation
The reconciler's spool recovery keeps a run's session-checkpoint columns exactly as its abandon does: both land through the same terminal CAS, which never touches them. Three comments named only the abandon; they now name both, as ArtifactRetention already did. Two retry notes said things that are not always true. The honest-redo line ended "(no pushed branch was found to continue from)", which a dependent supervisor unit reads in the same goal as its producer's "Continue from this branch"; it now says the prior attempt pushed no branch of its own. The lost-host preamble said the machine was lost, but the reconciler abandons the same way when only the process died (ProcessConfirmedDead, or a lapsed lease whose orphan it kills), so it now says "the machine or the process", and the published-branch hint no longer says the unpublished work died with the machine. The grading heartbeat slept on the wall clock, so its tests could only race real milliseconds. It now takes its clock from the caller as a required parameter, as HeartbeatLoop.RunAsync has since #2005, and the tests drive a FakeTimeProvider: exactly one heartbeat per elapsed interval. Converting them showed the cancel test could never fail on the escape it is named for, because Should.NotThrowAsync passes a canceled task; it records the outcome directly now. The listener-death flake was not a slow wait but a lost one. Install handed the accept loop to Task.Run, so a close that landed while a pool thread was still registering the loop's first wait was never delivered by the managed HttpListener, and the loop waited forever with the lease still claimed. Against the real broker, opening and immediately breaking a lease left 23 of 1500 leases claimed; calling the loop directly, which registers its first wait before Install returns, left none. The test now wakes on the drop's warning, its last effect, instead of polling HasLease. The raw NUL byte in that file's ThisHost constant is written as an escape, so grep stops skipping the file as binary.
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
AgentRunReconcilerService.CasTerminalAsync) — in the cancel and resumable-lookup comments ofAgentRunService.csand inArtifactRetentionPolicy.cs, whose 162-character doc line is also re-wrapped to the file's width.AgentRetryContinuity.HonestNoContinuityHintnow ends "(your prior attempt pushed no branch of its own)" instead of "(no pushed branch was found to continue from)", which contradicted a dependent supervisor unit's "Continue from this branch" handoff in the same goal.LostHostPreamblenow says "the machine or the process running your previous attempt was lost mid-run", because the reconciler abandons the same way forProcessConfirmedDeadand for a lapsed lease whose orphan it kills;LostHostPublishedBranchHintsays the unpublished remainder "was lost with that attempt" instead of "died with the machine".SupervisorTurnService.RunGradingHeartbeatLoopAsynctakes itsTimeProvideras a required parameter (the three call sites passTimeProvider.System; no constructor changes), matchingHeartbeatLoop.RunAsyncsince Run every heartbeat on the clock its caller passed #2005.SupervisorGradingHeartbeatTestsdrive aFakeTimeProviderand assert exactly one heartbeat per elapsed interval. The cancel test records the loop's outcome directly:Should.NotThrowAsyncpasses a canceled task, so it could never catch the escapingOperationCanceledExceptionthe test is named for.LoopbackModelCredentialBroker.InstallcallsAcceptAsyncdirectly instead of throughTask.Run, so the loop's first accept wait is registered beforeInstallreturns. A close that raced that registration was never delivered by the managedHttpListener, leaving the loop waiting forever with the lease still claimed — the cause of theA_lease_whose_listener_died_stops_being_claimedflake (open-then-break against the real broker: 23 of 1500 leases stuck before, 0 of 1500 after). The test now wakes on the drop's warning, its last effect, instead of pollingHasLease, and the raw NUL byte in the file'sThisHostconstant is written as\0so grep stops skipping the file as binary.Test plan
dotnet build backend/CodeSpace.sln: 0 errorsAgentNodeFlowTests,SupervisorRetryWorldStateFlowTests,AgentRunSessionCheckpointFlowTests,SupervisorPreClaimCrashRecoveryFlowTests(57/57);AgentRunExecutorTestsincluding the credential-broker partial (109/109);AgentRunReattachFlowTests(13/13);SupervisorAcceptanceGradeFlowTests,SupervisorAcceptanceFoldFlowTests,SupervisorUnitAcceptanceFoldFlowTests(116/116)SupervisorGradingHeartbeatTestsred; a drop that keeps the table entry, a silent drop, or an accept loop that swallows the failure turnsA_lease_whose_listener_died_stops_being_claimedredModelCredentialBrokerNetnsE2ETestsdegrade-skips on macOS; the privileged Linux job is authoritative