harness-runtime: replay history before opening the live attach - #1979
Merged
SSharma-10 merged 1 commit intoSep 22, 2026
Conversation
attachToSession previously opened a single connection whose catching_up
(history) and live phases shared the wire, spliced apart server-side, with
stdin read from the moment the goroutine was launched -- before any of that
history had rendered.
ui-aquarium's /events consumer (useSessionHistoryPager + useStreamSession)
instead does this as two sequential calls: a finite replay_only history read
first, then a live attach anchored strictly after that read's newest event,
with input held until both settle. Give doctl the same shape:
replayHistoryBeforeAttach reads history to completion synchronously, on the
same goroutine that calls streamWithReconnect/runAttach right after, so:
- stdin is never read until history has fully rendered, closing off
MARSOHS-1026's user-visible symptom (a resent/duplicate turn rendering
live, indistinguishable from new output, interleaved with whatever the
user just typed) as a client-side defense independent of OHP's own
ingest-side dedupe.
- the live stream opens with ReplayFrom already seeded to the last history
event's id (drainStream's existing cursor.set(ev.EventID) does this for
free), so there is no overlap window left for OHP to splice.
Best-effort: a terminal error is reported once and returns, matching
streamWithReconnect's own classifyStreamError handling; a transient error
falls through silently to the live stream's own catching_up window, which is
the pre-existing behavior for that failure mode.
logwolvy
approved these changes
Sep 21, 2026
SSharma-10
approved these changes
Sep 22, 2026
SSharma-10
merged commit Sep 22, 2026
d07925a
into
digitalocean:feat/agents-subcommands
2 checks passed
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.
What
attachToSessionopened one connection whose catching_up (history) and live phases shared the wire — spliced apart server-side — with stdin read from the moment the streaming goroutine was launched, before any of that history had rendered.Investigated how
ui-aquarium's/eventsconsumer does this (useSessionHistoryPager+useStreamSessioninapps/ui-openharness): two sequential calls, not one — a finitereplay_onlyhistory read first, then a live attach anchored strictly after that read's newest event (replayFrom: pager.newestEventId), with input held (ready = ... && !streamPending) until both settle. This PR gives doctl's interactive attach the same shape.replayHistoryBeforeAttachreads history to completion synchronously, on the same goroutine that callsstreamWithReconnect/runAttachright after:ReplayFromalready seeded to the last history event's id —drainStream's existingcursor.set(ev.EventID)gives this for free — so there's no overlap window left for OHP to splice.Best-effort on failure: a terminal error (auth, missing session) is reported once and returns, matching
streamWithReconnect's ownclassifyStreamErrorhandling — the live stream will report the same failure again on its own connect. A transient error falls through silently to the live stream's own catching_up window, which is the pre-existing behavior for that failure mode; this read isn't a new failure surface.Doesn't touch
agents logs/ the codex proxy--replaypath (#1903's backward paging) — different code path (one-shot batch read vs. this interactive attach seam).Proof of testing
New:
TestReplayHistoryBeforeAttach_RendersHistoryAndSeedsCursor,TestReplayHistoryBeforeAttach_TerminalErrorReportsAndReturns,TestReplayHistoryBeforeAttach_TransientErrorFallsThroughSilently,TestReplayHistoryBeforeAttach_CancelledContextIsSilent.go test ./commands/...andgo vet ./commands/...clean, including the full existingTestAttach*/TestStreamWithReconnect*/TestDrainStream*suite (unchanged).