Repository navigation
🐛 fix(cmd/hive): tear down sidecar event stream and hosted runtime in supervision boot test - #14508
Conversation
… supervision boot test The test never unsubscribed the sidecar client's event observer, so streamEvents kept reconnecting to the closed httptest server. Clear the observer in t.Cleanup, which runs before the server closes. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Changelog: this PR changes code but carries no changelog entry If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
There was a problem hiding this comment.
Read the tree, not just the diff. This looks correct to me.
src/cmd/hive/agent_runtime.go:36-44— in sidecar modenewHostedRuntimereturns*sidecarHostwrapping*client.Runtime, so both type assertions in the new cleanup succeed on this path.src/crux/api/client/client.go:343-364—syncStreamcancels and blocks on<-donewhen the last observer clears; nothing incmd/hive(non-test) callsSetKickObserver, soSetEventObserver(nil)is the last observer and thestreamEventsgoroutine (which otherwise retries everyeventsRetry= 1s against the closed server) is joined before returning.- Ordering: the cleanup is registered inside
newRuntime, i.e. afterliveServe'st.Cleanupatagent_runtime_supervise_test.go:58, so LIFO runs it first — matches the comment in the diff. - If boot never called
install()(no observer registered),syncStreamis a no-op, so the cleanup is safe either way.
No test-ratchet concern: no time.Sleep added.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
TestBootAgentsWith_SidecarStartsSupervision never cleared the sidecar client's event observer, so client.Runtime.streamEvents kept reconnecting every second to the closed httptest server. The test now captures the hosted runtime and calls SetEventObserver(nil) in t.Cleanup (LIFO, so it runs before the server closes), which cancels and joins the stream goroutine.
Closes #14477
Closes #14479
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88