Keep the status updates a dying stream already showed - #5074
Open
elias-ba wants to merge 4 commits into
Open
Conversation
This was referenced Aug 15, 2026
Security Review ✅
|
elias-ba
force-pushed
the
timeout-separate-dials
branch
from
August 15, 2026 22:50
aacaf77 to
4428533
Compare
elias-ba
force-pushed
the
timeout-keep-status-timeline
branch
from
August 15, 2026 22:50
909036d to
bda6aac
Compare
The partial save kept the text and the workflow YAML but dropped the status timeline, so a failed workflow chat reloaded missing the steps the user had watched go by. Text is closed off at each status update, so the saved timeline is in the order it appeared. A reply with no status in it still saves flat, since that renders the same from the content alone.
Covers the whole stack, including the APOLLO_TIMEOUT replacement, which deployments have to act on.
elias-ba
force-pushed
the
timeout-separate-dials
branch
from
August 15, 2026 23:40
4428533 to
c7821e5
Compare
elias-ba
force-pushed
the
timeout-keep-status-timeline
branch
from
August 15, 2026 23:40
bda6aac to
261a6ff
Compare
Nine things, all from reviewing the corrected stack rather than the
original diff. They span every PR in it, so they land here rather than in
another six-deep rebase.
The timeout message was unreachable. Finch raises its own
%Mint.TransportError{reason: :timeout} from receive_timeout inside its
streaming task, and usually wins the race against the adapter's own after
clause, so a hung Apollo read as "the connection was lost" - the exact
conflation the stack exists to end. Both now say the same thing.
Apollo's error text no longer reaches the panel or the row. Apollo wraps
any unhandled exception as str(e), which carries internal hostnames,
upstream URLs and container paths. That it failed is worth recording,
because it separates a service giving up from a socket going quiet; its
words are not.
A partial save in a global chat lost from_global, so it was treated as job
code and had the session's job attached - the workflow YAML came back
rendered as a code suggestion, in the chat type where keeping it matters
most.
An empty status segment invalidated the whole changeset, discarding the
text too. The complete path drops invalid segments rather than carrying
them; this one now does the same, and the comment claiming they already
matched is gone.
The reaper told the panel a session had failed but not which message, and
the panel assumed the newest - so clearing something stranded several
exchanges back marked an unrelated success as failed. The id travels with
the broadcast now.
The partial was saved and never sent, so a user watched their answer be
replaced by an error and only found it again by reloading.
The drain window is derived from the same three settings the job ceiling
is, instead of being fixed at six minutes. Dev's own defaults exceeded it,
so the warning added in this stack fired on every boot, advising a change
to something no variable could reach. OBAN_SHUTDOWN_GRACE_PERIOD_MS now
can.
Also: failure_message is bounded, since it is written through change/2
which validates nothing and is re-sent on every join; one internal failure
no longer logs three times, which was three Sentry events; the reaper's
sweep is batched, since each message reaped loads its whole session and
the backlog it exists for is when that is largest; and the pool comment no
longer implies http1 is being changed when it is already Finch's default.
A turn that failed before producing anything re-sent the partial kept from an earlier turn, so the client showed it twice. The last message in the session is the right one to look at, not the newest assistant one. Dropping empty segments missed whitespace-only ones, and validate_required trims before it checks - so a status update of spaces still invalidated the embed and took the text down with it.
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.
Description
The partial save kept the text and the workflow YAML but dropped the status timeline, so a failed workflow chat reloaded missing the steps the user had watched go by.
Text is now closed off at each status update, so the saved timeline reads in the order it appeared. A reply with no status in it still saves flat, since that renders the same from the content alone and changing it would alter how every job chat failure looks.
The tests here drive a real socket rather than the Tesla mock, because none of this behaviour exists until something walks the lazy body stream. They cover job, workflow and global chat.
Closes #__
Validation steps
mix test test/lightning/ai_assistant/stream_failure_test.exsMap.put(attrs, :response_segments, segments)line and check the timeline test fails.AI Usage
Pre-submission checklist