Skip to content

feat(sdk): StreamContext mints the stream identity and closes every stream - #4822

Open
VascoSch92 wants to merge 19 commits into
mainfrom
vasco/streaming-4682-stream-context
Open

feat(sdk): StreamContext mints the stream identity and closes every stream#4822
VascoSch92 wants to merge 19 commits into
mainfrom
vasco/streaming-4682-stream-context

Conversation

@VascoSch92

@VascoSch92 VascoSch92 commented Sep 2, 2026

Copy link
Copy Markdown
Member

HUMAN:

Step 5 of the streaming epic, stacked on #4807. This is the piece that gives a
stream an identity the client can close it by.


AGENT:

Why

A streamed turn leaves the agent on two channels that know nothing about each
other. StreamingDeltaEvent carries text, a random id and a timestamp, and
nothing else — no stream identity, no attempt number, no pointer to the message
that supersedes it. So the browser re-marries them by comparing strings, and a
stream that dies (cancellation, provider failure, policy rejection, an
unhandled exception) leaves an open slot on the client forever.

Agent.astep alone has three except … return branches that emit an error
message and return without ever touching the stream.

Two smaller things came out of the same path. The LLM stream path masked
nothing at all, so a registered secret appearing in a token delta reached the
wire in cleartext. And chunk.id / choice.index are in scope at
event_service.py's delta callback and never read — litellm hands us the
provider's identity and we drop it.

Summary

StreamContext (openhands-sdk/openhands/sdk/agent/stream_context.py) owns one
streaming slot: it mints the id, stamps and masks each delta, and closes the
slot exactly once.

  • Minting is not a write. Event.id is already client-minted in-process
    (event/base.py, default_factory=lambda: str(uuid.uuid4())); the log never
    assigns ids, it receives events that already have one. This changes only
    when uuid4() runs. Same bytes, one append, same ordering. If the stream
    dies the id is simply never used and nothing on disk referenced it.
  • The durable event retires the slot by its own id. claim() hands the
    minted id to the MessageEvent, or to the first ActionEvent when the turn
    ends in tool calls — the streamed text is that action's thought, so a
    tool-calling turn closes on a Durable frame rather than aborting. Claiming
    happens at the construction site, so an error path that never builds the
    event leaves the slot open for the abort.
  • Every opened slot closes. __exit__ emits exactly one ItemAborted
    unless the id was claimed. pi enforces the same invariant one level lower —
    its stream function may not throw. This is that rule retrofitted onto code
    that can.
  • A slot opens lazily, on the first delta rather than at open(), so a step
    that never streams (early condensation return, non-streaming model) has
    nothing to abort.
  • Four entry points, not two. Agent.step, Agent.astep and both ACP
    equivalents. Both agents stream on their synchronous path too, so an
    async-only fix covers half the problem. The ACP bridge is also why this cannot
    live in llm.py: it calls on_token(text) with a bare str and never enters
    the LLM layer.
  • Masking on the stream path uses SecretRegistry.compile_output_mask() — a
    snapshot of the already-resolved values, single-pass, no lock and no I/O while
    streaming. mask_secrets_in_output resolves uncached sources first, and its
    own comment warns get_value() "may do blocking network I/O"; on the sync
    path that would run under the state lock. Trade-off: a secret registered
    after the stream opened is not masked in that stream's deltas, but it is
    masked in the durable message, which is the authoritative one.
  • Attempts. litellm mints a new completion id per retry attempt, so a change
    of chunk.id mid-item is a re-stream: the context bumps attempt and
    restarts order. The ACP prompt-retry loops call new_attempt() directly.
    chunk_id and choice_index ride along on the delta frame as corroboration.

Delivery, without touching the event bus

Progress frames are not events. They ride a second fan-out on EventService
(subscribe_to_stream_progress), and _ProgressSubscriber in
session_socket.py maps them one-for-one onto the ItemStarted / Delta /
ItemAborted envelopes #4807 defined but nothing produced. Nothing is added to
PubSub[Event], so webhooks, telemetry and the legacy /sockets/events/{id}
endpoint see no new traffic.

StreamContext.on_chunk forwards the raw chunk downstream unchanged before
emitting its own frames, so on_token consumers — the CLI, the legacy socket,
examples/01_standalone_sdk/29_llm_streaming.py, any user callback — are
byte-for-byte unaffected. AgentBase.step keeps its signature; the sink is read
off the conversation (LocalConversation.on_stream, new stream_callbacks=
argument, appended last), so no third-party AgentBase subclass breaks.

No progress frame is buffered against the replay boundary and none is replayed:
a dropped one costs a repaint, and the real text arrives with the durable event.

Issue Number

Fixes #4682

How to Test

  • uv run pytest tests/sdk -q6026 passed, 9 skipped, 10 xfailed.
  • uv run pytest tests/agent_server -q2085 passed, 13 deselected.
  • uv run pre-commit run --files <changed> — ruff format, ruff lint,
    pycodestyle, pyright, import rules, Tool registration all pass.
  • uv run pytest tests/cross tests/tools -q — 5 failures, all reproduced on
    the base branch with this branch's code absent (two terminal-session tests,
    two live-server tests; test_ctrl_c passes in isolation on both and flakes
    under load).
  • uv run python .github/scripts/check_docstrings.py — core API files pass, no
    warnings on any file this PR touches.

New coverage:

  • tests/sdk/agent/test_stream_context.py (13) — lazy open; exactly one abort
    per opened slot across a provider failure, a cancellation and a clean return
    with no durable event; claim() is once; a retry re-streams the same
    item_id under a higher attempt with order restarted; reasoning and text
    are separately ordered; masking; the raw chunk still reaches on_token; a
    sink that raises does not fail the turn.
  • tests/sdk/agent/test_stream_identity.py (5) — through a real Agent and
    LocalConversation: the MessageEvent carries the minted id on both step
    and astep; a tool-calling turn retires on its first ActionEvent; a
    provider failure retires with one abort; a step that never reaches the
    provider opens nothing.
  • tests/sdk/agent/test_acp_agent.py — the ACP turn's FinishAction event
    carries the minted id, and the context is released with the turn.
  • tests/agent_server/test_session_socket.py (2) — the three frames reach the
    wire with their identity and the provider corroboration; an oversized delta
    drops the connection rather than a durable frame.
  • tests/agent_server/test_event_streaming.py — progress reaches its own
    subscribers and nothing on the event bus.
  • tests/sdk/conversation/test_secrets_manager.py (2) — the snapshot masks only
    already-resolved values and never calls an unresolved source; longest
    overlapping value wins.

What is not covered

Stated plainly so a reviewer knows where to look hardest:

  • No end-to-end assertion of a progress frame over a real WebSocket. The
    five test_agent_server_wsproto.py tests exercise the handler (so the new
    subscribe/unsubscribe runs on every session-socket connection), but producing
    a delta needs a streaming LLM inside a server that those tests start as a
    separate process. The path is covered in two halves instead: publish reaches
    its subscribers (test_event_streaming.py) and a subscriber reaches the wire
    (test_session_socket.py).
  • The attempt boundary is exercised with synthetic chunks, not against real
    litellm retry behavior. See the note below on why it is inferred at all.
  • The ACP prompt-retry new_attempt() calls are untested — reaching them
    needs injected connection errors against a live ACP subprocess.
  • The MaxSubscribersError branch for progress is untested.

stream_context.py and session_protocol.py are at 100% statement coverage.

Video/Screenshots

N/A — no UI surface.

Design Doc

N/A — the module docstring in stream_context.py carries the rationale inline.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • Second commit is a self-review pass that found three defects in the
    first, each with a regression test:

    • A retry that died before its first token stranded the slot.
      new_attempt() clears the per-attempt "started" flag and close() keyed
      its abort on that flag, so the attempt that had streamed was never
      retired — the exact invariant this PR exists to establish. close() now
      keys on whether the item was ever opened.
    • A raising progress sink escaped close(), which runs from __exit__, so
      a broken subscriber could replace the exception a failing step was
      unwinding, or fail a step that had succeeded.
    • The agent handed the LLM a wrapper that was never None, which defeats the
      fallback in llm.completion that degrades a stream=True model to a
      non-streaming call when no on_token is wired (Profile-launched conversations never enable LLM streaming — on_token crash on switch_llm #4014). StreamContext
      now reports no callback when nothing consumes one, so a caller with no
      token and no stream callbacks gets exactly today's behavior.
  • Merge order: fix(sdk): persist events before publishing them, return the assigned seq #4806 (Streaming step 2: append_event returns its sequence number; publish moves after persist #4680), then feat(agent-server): add /sockets/session/{id} with a non-Event envelope #4807 (Streaming step 3+4: /sockets/session/{id} — envelope wire, cursor, paged replay, byte-budget admission #4681), then this one. This branch
    is based on vasco/streaming-4681-session-socket, so the diff here is only
    the step-5 work.

  • One fix outside the issue's scope, and it was load-bearing. The new
    agent-level tests made tests/sdk fail 17 tests in modules they never touch,
    with Duplicate class definition … ClientAction_persist_navigate_to. The
    cause is in utils/models.py: clear_subclass_cache() bumped the generation
    counter but left the stale _concrete_cache / _checked_cache entries in
    place, and each of those is a strong reference to every subclass it
    captured. So a dynamically-created class survived the test that unregistered
    it, and the next rebuild saw two classes with one name. The same failure
    reproduces on main's own files with no new code at all:
    uv run pytest tests/sdk/agent/test_agent_content_policy_violation.py tests/sdk/conversation/local/test_client_tools_persistence.py tests/sdk/tool/test_tool_serialization.py → 6 failed. clear_subclass_cache() now drops the
    entries, and _wipe_client_tool_globals calls it. Happy to split this out if
    you would rather it landed on its own.

  • The attempt boundary is inferred, not reported. The retry wrapper in
    llm.py owns attempt, but it has no per-call channel to the token callback,
    so the context detects a new attempt from a change in chunk.id. This is a
    heuristic: a provider that varies its completion id mid-response would read as
    a retry. Documented at the detection site.

  • The ACP bridge still masks each chunk itself before on_token, so a
    streamed ACP delta is masked twice now (idempotent). Removing the bridge's own
    masking is [ACP on Cloud] Streaming-redaction for split-chunk secrets on the live ACP token relay OpenHands#15720's business, not this PR's.

  • Deltas still publish as StreamingDeltaEvent on the event bus for the legacy
    endpoint. Deleting that channel is step 7 (Streaming step 7: stop emitting StreamingDeltaEvent and freeze the legacy events endpoint #4683).


🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Architectures Base Image Docs / Tags
java amd64, arm64 eclipse-temurin:17-jdk Link
python-slim amd64, arm64 nikolaik/python-nodejs:python3.13-nodejs22-slim Link
python amd64, arm64 nikolaik/python-nodejs:python3.13-nodejs22-slim Link
golang amd64, arm64 golang:1.21-bookworm Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:71e3e1a-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-71e3e1a-python \
  ghcr.io/openhands/agent-server:71e3e1a-python

All tags pushed for this build

ghcr.io/openhands/agent-server:71e3e1a-golang-amd64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-golang-amd64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-golang-amd64
ghcr.io/openhands/agent-server:71e3e1a-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:71e3e1a-golang-arm64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-golang-arm64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-golang-arm64
ghcr.io/openhands/agent-server:71e3e1a-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:71e3e1a-java-amd64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-java-amd64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-java-amd64
ghcr.io/openhands/agent-server:71e3e1a-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:71e3e1a-java-arm64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-java-arm64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-java-arm64
ghcr.io/openhands/agent-server:71e3e1a-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:71e3e1a-python-amd64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python-amd64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python-amd64
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-amd64
ghcr.io/openhands/agent-server:71e3e1a-python-arm64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python-arm64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python-arm64
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-arm64
ghcr.io/openhands/agent-server:71e3e1a-python-slim-amd64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python-slim-amd64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python-slim-amd64
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-slim-amd64
ghcr.io/openhands/agent-server:71e3e1a-python-slim-arm64
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python-slim-arm64
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python-slim-arm64
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-slim-arm64
ghcr.io/openhands/agent-server:71e3e1a-golang
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-golang
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-golang
ghcr.io/openhands/agent-server:71e3e1a-golang_tag_1.21-bookworm
ghcr.io/openhands/agent-server:71e3e1a-java
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-java
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-java
ghcr.io/openhands/agent-server:71e3e1a-eclipse-temurin_tag_17-jdk
ghcr.io/openhands/agent-server:71e3e1a-python-slim
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python-slim
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python-slim
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-slim
ghcr.io/openhands/agent-server:71e3e1a-python
ghcr.io/openhands/agent-server:71e3e1a8045f6e47fefe6694b30d0cf75bfcfe05-python
ghcr.io/openhands/agent-server:vasco-streaming-4682-stream-context-python
ghcr.io/openhands/agent-server:71e3e1a-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim

About Multi-Architecture Support

  • Each variant tag (e.g., 71e3e1a-python) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., 71e3e1a-python-amd64) are also available if needed

VascoSch92 and others added 13 commits September 1, 2026 10:32
compose_callbacks put caller callbacks (e.g. a PubSub publish) ahead of
the default callback that appends the event to state, so a socket could
be told about an event before EventLog had durably written it. Run the
default callback first so persistence always happens before publish,
and stop it there if append raises.

EventLog.append and ConversationState.append_event now return the
sequence number the log assigned, so a subsequent get_index lookup for
the same event is no longer needed.
The legacy /sockets/events/{id} endpoint ships the durable record itself over
the wire: sockets.py sends event.model_dump(...), event_store.py persists
event.model_dump_json(...), and remote_conversation.py decodes with
Event.model_validate. One class, extra="forbid", three roles - so every
additive wire field is a storage migration, which is why a token delta cannot
carry a stream identity today.

This adds a second endpoint whose frames are envelopes rather than events. The
Event rides inside one as an untouched payload (verified byte-identical to
what the legacy endpoint sends), and the protocol's own fields - seq, stream
identity, progress - live on the envelope where adding a field costs nothing.
The URL is the protocol version: a client speaks one endpoint or the other, so
there is no handshake and no dual-decode path. The legacy endpoint is
unchanged.

Three fixes come with it:

- History and live traffic no longer interleave. The subscriber registers in
  buffering mode, the high-water seq is read after that, and the buffer is
  flushed against it once paged replay finishes. No state lock is needed:
  subscribe-then-mark plus a seq filter gives the same guarantee without
  adding a seventh consumer to ConversationState's FIFOLock.

- A slow consumer can no longer wedge the publisher. Admission is synchronous
  and byte-bounded, and the socket is awaited only by this connection's single
  writer task. Over budget drops the connection, not a frame, because the
  client resumes losslessly with after_seq.

- StreamingDeltaEvent is filtered rather than forwarded. Deltas do not belong
  on the durable channel.

seq is the event's index in the log, already on disk as the {idx:05d}
component of event-{idx:05d}-{event_id}.json, so exposing it needs no
migration and no SDK change.

ItemStarted/Delta/ItemAborted are defined and carried but nothing produces
them yet; that needs StreamContext at the four streaming entry points.
…ataclasses

- Condense the session_protocol module docstring and the endpoint's
  replay-boundary comment to their load-bearing content.
- Type MAX_FRAME_BYTES/MAX_PENDING_BYTES/REPLAY_PAGE_SIZE as Final[int].
- Make _ConnectionWriter and _SessionSubscriber slots=True, frozen=True;
  the handful of fields each still mutates after construction now go
  through object.__setattr__, the standard escape hatch for a frozen
  dataclass with a small amount of controlled internal state.
- Drop the plain-comment section dividers.
receive_json decodes with json.loads, so a non-JSON text frame raises
JSONDecodeError rather than WebSocketDisconnect. Only the latter was
caught, so a single garbage frame unwound _inbound_loop and escaped the
handler. The legacy endpoint absorbs this in its one broad except and
answers the client; do the same here via the ErrorFrame path that already
exists for a bad message body, and keep the connection open.
…fakes

The reorder made two comments wrong: the chain is now
visualizer -> default(persist) -> user callbacks, and the visualizer still
renders ahead of persist, so say that rather than claiming nothing is
announced first.

The two EventLog test doubles still declared append() -> None, which no
longer matches the int contract they inherit; return the assigned index.
Review follow-ups on the endpoint itself, none of which the unit tests
could reach because nothing executed the handler:

- Replay yields per frame, not per page. The writer task is the only
  thing that drains pending bytes, so replaying a whole page without
  letting it run could exhaust the budget on a page of large events and
  drop a client that was never slow.
- _read_page tolerates OSError, not just FileNotFoundError, so a
  transient read error on the networked filesystems EventLog warns about
  costs one event rather than the connection.
- aclose() no longer swallows a cancellation aimed at the calling task,
  which was hiding server shutdown.
- A conversation closed between the lookup and get_conversation() now
  closes 4004 like every other failure path instead of raising out of
  the handler.

Adds five end-to-end tests against the real server (reusing the wsproto
harness): the opening sync frame, durable frames carrying seq over the
wire, lossless resume via after_seq, 4004 on an unknown conversation,
and surviving a malformed inbound frame.

Also drops the size-limits comment divider, keeping the note itself.
…at never ran

Two real defects found reviewing the endpoint:

_seq_of treated every ConversationStateUpdateEvent as unpersisted. Only
the snapshot subscribe_to_events synthesises on connect is; the rest are
appended like anything else (append_event stores them and merely skips
advancing HEAD, verified against a live conversation). Deciding by type
stripped their real seq and defeated the seq <= through_seq dedupe in
go_live, so a state update caught in the replay window arrived twice --
once from disk, once from the buffer. Decide by lookup instead, and keep
the published-before-persist warning for the events it was meant for.

go_live was handed the high-water mark even when no replay ran. A
live-only client (no after_seq) got nothing from disk, so a buffered
event below the mark was discarded as "already replayed" and lost for
good, since such a client has no cursor to recover it with.

Also: clean up the writer and socket if subscribe_to_events fails after
registering, and consume the receiver's result when it completes in the
same wakeup as a writer failure, which was leaking "Task exception was
never retrieved" noise on every racing close.
No behaviour change. Cuts the narrative, the contrasts with how the legacy
endpoint does it, and the restatement of what the code already says, keeping
the non-obvious why: why no lock at the replay boundary, why send is
synchronous, why seq is decided by lookup, why the budget is provisional.
Also drops a leftover divider comment in the unit tests.
Both _ConnectionWriter and _SessionSubscriber declare fields with
init=False and write them after construction, so frozen only forbade
plain assignment and bought six object.__setattr__ calls. Nothing hashes,
copies or compares them: PubSub keys subscribers by a generated uuid. The
bypass also hid the writes from pyright, since object.__setattr__ is
untyped.
…tream

A streamed turn left the agent on two channels that knew nothing about
each other: token deltas with a random id and no stream identity, and the
durable event appended when the turn resolved. A client had to re-marry
them by comparing strings, and a stream that died left an open slot
forever.

StreamContext mints the id when the stream opens and hands it to whichever
durable event the stream became — the MessageEvent, or the first
ActionEvent when the turn ends in tool calls. Minting is not a write:
Event.id is already client-minted in-process, so this changes only when
uuid4() runs.

Wrapped at all four streaming entry points (Agent.step/astep and both ACP
equivalents), each in a try/finally that emits exactly one ItemAborted if
no durable event claimed the id, so cancellation, a provider failure, a
policy rejection and an unhandled exception all retire the slot.

Deltas are masked from a snapshot compiled once at open: resolved values
only, single-pass, no lock or blocking I/O while streaming. The LLM stream
path was not masked at all before this.

Progress rides its own fan-out in the event service and reaches the
session socket as the ItemStarted/Delta/ItemAborted frames #4681 defined
but nothing produced. The event bus, the legacy socket and every on_token
consumer are untouched: the raw chunk still passes through unchanged.

Fixes #4682
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Endpoint audit

⚠️ 18 actionable Agent Server contract divergence(s) · report-only

Contract: pinned release artifact

Category Count
Actionable client-only calls 0
Actionable server-only operations 18
Documented non-divergences 13
Agent Server contract operations 129
Audited handwritten client endpoints 119

Actionable client-only calls (0)

none

Actionable server-only operations (18)

  • DELETE /api/canvas-extensions/installed/{}
  • DELETE /api/llm/provider-connections/{}
  • GET /api/canvas-extensions/installed
  • GET /api/canvas-extensions/installed/{}
  • GET /api/canvas-extensions/installed/{}/bundle
  • GET /api/conversations/{}/events
  • GET /api/file/archive
  • GET /api/git/commits
  • GET /api/git/commits/{}/changes
  • GET /api/init
  • GET /api/llm/provider-connections
  • PATCH /api/canvas-extensions/installed/{}
  • PATCH /api/llm/provider-connections/{}
  • POST /api/canvas-extensions/install
  • POST /api/conversations/{}/load_plugin
  • POST /api/file/create_directory
  • POST /api/init
  • POST /api/llm/provider-connections
Documented non-divergences (13)

Client calls intentionally absent from the filtered contract (11)

  • GET /
  • GET /alive
  • GET /health
  • GET /ready
  • GET /server_info

Reason: Operational Agent Server endpoints intentionally excluded from the filtered public release artifact.
Owner: OpenHands runtime maintainers

  • DELETE /api/meta-profiles/{}
  • GET /api/meta-profiles
  • GET /api/meta-profiles/{}
  • POST /api/meta-profiles/{}
  • POST /api/meta-profiles/{}/activate

Reason: Client-ahead API stacked on the pending Agent Server meta-profiles implementation.
Owner: OpenHands SDK maintainers
Tracking: #3744

  • POST /api/profiles/{}/validate

Reason: Client-ahead API stacked on the pending Agent Server pre-flight LLM validation endpoint.
Owner: OpenHands TypeScript client maintainers
Tracking: #4422

Server operations covered by an exposed browser URL (2)

  • GET /api/conversations/{}/workspace
  • GET /api/conversations/{}/workspace/{}

Reason: RemoteWorkspace.startWorkspaceSession exposes these authenticated URLs for browser iframe and file requests; they are not HttpClient method calls.
Owner: OpenHands TypeScript client maintainers

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-agent-server/openhands/agent_server
   event_service.py90313086%179–180, 302, 306, 311, 338, 345, 379, 382–383, 387–388, 399, 405, 415–419, 422–425, 496, 517–518, 592, 646, 666, 693, 717–718, 722, 730, 733, 749, 781, 792, 799, 805, 865, 868, 883–884, 972, 1002, 1005, 1043, 1083, 1101, 1115, 1258, 1378–1381, 1385, 1414, 1418, 1425, 1439, 1454, 1501–1503, 1583, 1608, 1614, 1616, 1626, 1628, 1635, 1645, 1647–1648, 1652, 1666–1671, 1673, 1700, 1705–1711, 1715–1718, 1726–1729, 1771–1773, 1828–1829, 1831–1838, 1840–1841, 1850–1851, 1853–1854, 1861–1862, 1864–1865, 1876, 1901, 1907, 1913, 1922–1923, 1945, 1955
   session_socket.py2508964%141, 145–146, 150, 152–155, 168–170, 218, 306, 332–333, 335–340, 343–346, 349, 351–353, 355–356, 359, 361–363, 368–373, 376–377, 383–386, 388–391, 394, 397, 399–400, 402–403, 405–410, 415, 417, 419–425, 449–450, 454–456, 474, 477–478, 480–485, 488–489, 492
openhands-sdk/openhands/sdk/agent
   acp_agent.py1671139317%213–217, 222–223, 226, 270, 305–314, 318–327, 352, 355–361, 364, 367, 369–372, 380–385, 387–388, 391–394, 398, 401–402, 406, 414, 423–433, 441, 450–451, 459–460, 468–469, 484–486, 491–496, 521–529, 532–535, 540, 542–545, 560–562, 566, 573–576, 594–596, 600–601, 621–623, 627, 633, 667–668, 672–677, 680–688, 697, 702–704, 710–711, 714, 749–756, 760, 768–772, 779, 782–783, 787, 793, 798, 827–829, 832–833, 839–840, 847–849, 856, 888–894, 901–903, 910, 924–926, 933–937, 947–948, 950–956, 966, 968–974, 977, 990–996, 1001–1005, 1010, 1015, 1021–1022, 1032–1037, 1041–1043, 1045, 1052–1054, 1058–1060, 1067–1068, 1099–1112, 1117–1120, 1130–1133, 1151–1157, 1159–1163, 1189–1199, 1209–1218, 1268–1270, 1272–1273, 1278, 1282, 1289–1292, 1297, 1299–1302, 1304–1305, 1308–1310, 1313–1321, 1333, 1343, 1347–1350, 1354, 1358–1359, 1370–1382, 1395–1397, 1400–1401, 1411, 1417, 1423–1426, 1429, 1431–1432, 1438–1449, 1451–1458, 1467–1470, 1473–1474, 1480–1482, 1486–1508, 1516, 1521–1524, 1526, 1537–1542, 1545, 1555–1557, 1570–1578, 1589–1590, 1595, 1603, 1613, 1625, 1630, 1635, 1640, 1645, 1652, 1659, 1662, 1776–1778, 1803–1804, 1849–1851, 1854, 1860–1864, 1957–1959, 1962, 1965–1967, 1970–1971, 1974, 1982–1986, 1988–1991, 1993–1994, 1997–2000, 2020–2028, 2031, 2034–2035, 2051–2052, 2055–2056, 2058, 2062, 2067–2068, 2070–2074, 2081, 2090, 2095, 2100, 2107, 2112, 2137, 2158, 2174–2177, 2196, 2203, 2219–2220, 2224–2225, 2229–2230, 2232, 2234–2236, 2240, 2249, 2263, 2266, 2268–2277, 2289–2291, 2298–2300, 2302, 2314–2315, 2318, 2320, 2322, 2334, 2351–2352, 2355, 2358–2360, 2365–2367, 2370–2372, 2374–2375, 2385, 2425–2426, 2431–2432, 2439–2442, 2446–2447, 2460–2463, 2477–2478, 2480–2481, 2484–2485, 2487–2490, 2499, 2505–2506, 2521–2526, 2530–2531, 2576–2582, 2591, 2608–2610, 2612, 2615–2618, 2623–2626, 2629–2631, 2636–2642, 2645–2648, 2651–2659, 2662–2666, 2668–2673, 2681–2682, 2694–2699, 2702–2704, 2711–2714, 2719–2723, 2734–2735, 2746–2752, 2755–2763, 2767–2768, 2771–2779, 2784–2787, 2789–2792, 2794, 2797–2800, 2803, 2806–2813, 2815–2818, 2822–2823, 2826, 2834–2835, 2843–2844, 2857–2859, 2864, 2869, 2872–2873, 2878, 2880, 2882, 2884–2885, 2889, 2891–2897, 2901–2902, 2933–2936, 2942–2943, 2945–2947, 2951, 2958, 2960, 2967, 2976–2978, 2982–2983, 2986, 2990, 3003–3007, 3010–3016, 3021, 3029, 3034–3036, 3046–3051, 3055–3057, 3060–3065, 3069–3071, 3075, 3079–3080, 3083–3084, 3087, 3094, 3098, 3101–3102, 3110, 3121–3126, 3131, 3137, 3140, 3148, 3153–3154, 3165–3166, 3171–3172, 3177, 3181, 3192, 3204, 3212, 3221, 3230–3231, 3234–3236, 3238, 3250–3251, 3259, 3262–3264, 3284–3286, 3291–3294, 3297, 3316–3325, 3337–3338, 3360–3364, 3368–3371, 3373–3376, 3378–3380, 3384–3385, 3388–3390, 3397–3398, 3401–3402, 3405–3407, 3413–3414, 3417–3418, 3422, 3425–3427, 3430–3432, 3436, 3439–3440, 3450–3453, 3458–3459, 3471, 3477–3480, 3482–3485, 3487–3490, 3496–3506, 3510, 3521–3523, 3539–3541, 3558–3569, 3574, 3577, 3605–3611, 3614–3616, 3619–3620, 3622–3623, 3638, 3644, 3654, 3658, 3660–3662, 3674, 3680–3685, 3687, 3694–3695, 3698–3701, 3717–3720, 3728, 3737, 3746, 3760–3762, 3771–3776, 3779–3780, 3785, 3788, 3800, 3807, 3816–3821, 3831–3836, 3838–3840, 3842, 3848, 3850–3852, 3860–3865, 3881–3884, 3886, 3894, 3896, 3899, 3904–3913, 3915, 3922–3924, 3929–3930, 3932, 3937, 3941–3949, 3952, 3960–3964, 3971–3972, 3976, 3980, 3983, 3992–3996, 4003, 4005–4012, 4016, 4018–4019, 4056–4059, 4063, 4072, 4074, 4077, 4079–4081, 4083–4091, 4093, 4100–4103, 4108, 4110–4113, 4120, 4126–4127, 4130–4135, 4138, 4146–4150, 4157–4159, 4163, 4166, 4175–4179, 4186, 4188–4189, 4194–4196, 4208–4214, 4217–4223, 4225, 4228–4244, 4247–4253, 4255, 4258–4260, 4262–4267, 4269–4270, 4274–4279, 4281–4283, 4285–4286, 4290, 4292–4297, 4301–4303, 4306–4307, 4312, 4317–4319, 4325, 4327–4328, 4330–4331, 4333–4334, 4388–4391, 4395–4397, 4403–4406, 4412–4413, 4423, 4427–4428, 4436, 4441, 4449–4452, 4459–4460, 4469–4477, 4480–4481, 4484–4491, 4493–4496, 4499–4501, 4506–4511, 4516–4518, 4520–4526, 4529–4531, 4533–4538, 4540–4546, 4550, 4554–4555, 4574–4579, 4582–4589, 4594–4595, 4598–4601
   agent.py44518159%111, 113–115, 128, 132, 149–153, 155–160, 169, 171, 173–175, 182, 223–224, 249, 293, 322–323, 330, 337, 363, 432, 436, 514–515, 519, 658, 662–663, 670–674, 696–697, 702–703, 708, 710, 715, 721–722, 737–739, 746–748, 751–752, 769–770, 776, 780, 788–791, 797–798, 800, 804, 807–808, 810–811, 826–827, 866, 870–871, 876–880, 899–900, 905–906, 911, 913, 918, 924–925, 945–946, 953–954, 958–959, 976, 984, 988, 996–999, 1006, 1010, 1014, 1017–1018, 1020–1021, 1035–1036, 1066, 1082, 1085, 1110, 1147–1151, 1178–1181, 1192, 1204–1205, 1211, 1218, 1259–1262, 1275, 1279, 1301, 1310–1315, 1317–1318, 1323–1324, 1337, 1363–1364, 1366, 1396, 1404–1405, 1418, 1421–1423, 1429, 1445, 1452, 1456–1457, 1461–1462, 1468–1470, 1473–1474, 1478–1479, 1486, 1498
   response_dispatch.py882077%71, 76, 78, 156, 183, 187, 210, 237, 241, 280–284, 309–311, 349, 353, 368
   stream_context.py1435562%137–144, 146, 150–155, 157, 186–188, 204–205, 208–218, 223–225, 235, 250–254, 262, 273–274, 276–280, 283–288
openhands-sdk/openhands/sdk/conversation
   secret_registry.py1695766%30, 38, 41, 43, 45, 72–74, 78–82, 86–89, 92–95, 98–106, 221–229, 256, 258, 277–280, 283, 286, 342, 344, 348–349, 351, 354–355, 357, 360, 369
openhands-sdk/openhands/sdk/conversation/impl
   local_conversation.py109856249%147–148, 155–159, 163–169, 177, 346, 361, 363–365, 368–369, 412, 444, 446–447, 503, 595, 599–600, 602–604, 621, 630–635, 639–643, 679–680, 701, 705, 710, 720–723, 730–732, 742, 746, 752, 755–757, 764, 787, 825, 832–834, 840, 843–844, 850–854, 857, 872–873, 876, 878–879, 881–883, 886, 891, 897, 900, 903, 906–907, 913–914, 916, 918, 924, 941–944, 948–949, 967, 985, 996–1000, 1003–1004, 1009–1014, 1023, 1028–1029, 1036, 1039, 1043–1044, 1046–1049, 1051–1052, 1059, 1070–1071, 1074–1075, 1079, 1082–1084, 1087–1088, 1091–1092, 1094, 1116–1117, 1122, 1125–1126, 1130, 1132–1134, 1136–1139, 1141–1142, 1153–1154, 1160–1161, 1165–1166, 1168, 1174–1177, 1180, 1188–1191, 1195–1197, 1200, 1211, 1217–1218, 1230, 1238–1239, 1243, 1252–1255, 1259, 1290, 1299–1301, 1305–1307, 1311, 1326–1327, 1332–1333, 1335, 1340–1342, 1344–1346, 1356–1358, 1370–1377, 1382, 1385, 1392, 1397, 1404–1405, 1412–1414, 1417–1423, 1431–1432, 1434–1436, 1441–1442, 1447–1450, 1456–1458, 1461–1462, 1468–1469, 1479–1480, 1487–1488, 1492–1493, 1495–1498, 1500–1502, 1506–1510, 1559, 1578–1580, 1645, 1650, 1655–1656, 1658, 1661–1662, 1716–1722, 1732–1740, 1771–1772, 1775, 1780–1782, 1800–1805, 1810–1811, 1820, 1892–1893, 1907–1908, 1959, 1967, 1970–1973, 1976, 1983–1984, 1987, 1993, 2000, 2028, 2035–2036, 2042, 2046–2047, 2051–2053, 2060–2063, 2068, 2071, 2080, 2126–2127, 2130–2131, 2134–2135, 2160, 2171–2173, 2176, 2183–2184, 2187, 2191, 2197, 2206, 2211–2212, 2216, 2224–2225, 2230, 2236, 2241, 2297, 2304, 2308, 2311–2312, 2316, 2319–2321, 2324, 2332, 2336, 2339, 2342, 2346–2347, 2351–2352, 2355, 2362, 2370–2372, 2375, 2380–2382, 2386, 2389–2390, 2393, 2396–2397, 2400, 2402–2404, 2408–2411, 2418, 2424–2426, 2430–2431, 2435, 2438–2439, 2448–2449, 2452, 2454, 2458–2460, 2463, 2465–2466, 2471, 2475–2476, 2480, 2483–2484, 2487, 2491, 2495, 2497–2498, 2502–2503, 2505–2506, 2510–2511, 2515–2517, 2524, 2547, 2561–2567, 2570, 2573, 2580, 2634, 2638, 2640, 2644, 2646–2648, 2650, 2652, 2658, 2665–2666, 2681, 2686, 2710, 2756, 2806–2807, 2825–2826, 2830, 2854, 2857–2859, 2862, 2864, 2868, 2873, 2878, 2883–2885, 2888, 2895, 2898, 2902, 2905, 2907–2909, 2911, 2936–2937, 2953, 2957, 2965, 2981–2982, 2986, 2988, 2990, 3029, 3032–3037, 3039, 3041–3044, 3046, 3048–3049, 3052–3055, 3062–3063, 3067, 3070–3072, 3075, 3077, 3079, 3086–3088, 3092, 3094–3095, 3125, 3128–3131, 3136–3138, 3144–3145
openhands-sdk/openhands/sdk/utils
   models.py1695667%42–43, 78, 80, 88–92, 154, 160, 181–183, 222–223, 231–234, 244–246, 249, 252, 256, 266, 304, 310–313, 315–316, 319, 322–326, 329–333, 335, 357, 370–371, 373–375, 377, 381, 383, 387
TOTAL422101760858% 

A retry that died before its first token stranded the slot. new_attempt()
cleared the per-attempt "started" flag and close() keyed its abort on that
flag, so the attempt that had already streamed was never retired. close()
now keys on whether the item was ever opened.

A raising progress sink escaped close(), which runs from __exit__ — so a
broken subscriber could replace the exception a failing step was already
unwinding, or fail a step that had succeeded. Emission is now contained.

The agent handed the LLM a wrapper that was never None, which defeated the
fallback in llm.completion that degrades a stream=True model to a
non-streaming call when no on_token is wired (#4014). StreamContext now
reports no callback when nothing consumes one.
Takes stream_context.py to 100% statement coverage and adds the two
assertions that were structural rather than behavioural: that a registered
secret really is masked in the deltas a real Agent step produces, and that
the async ACP entry point retires its slot the same way the sync one does.
The module docstring restated the issue; several method docstrings restated
the wire protocol that session_protocol.py already documents. Keeps the
non-obvious why — why minting up front is safe, why the LLM callback may be
None, why close() keys on ever-opened — and drops the rest.
Comment thread openhands-sdk/openhands/sdk/agent/stream_context.py
Comment thread openhands-agent-server/openhands/agent_server/session_socket.py Outdated
Comment thread openhands-sdk/openhands/sdk/agent/stream_context.py Outdated
_split_chunk used getattr for every field; only reasoning_content needs it,
because litellm deletes that attribute when the provider omits it. The rest
is direct access on a typed chunk now, and the one remaining getattr says
why it is there.

Drops the session-socket docstring paragraph about progress frames: the
subscriber already explains at the drop site why a delta never rides the
durable channel.
@VascoSch92
VascoSch92 marked this pull request as ready for review September 2, 2026 14:25
@all-hands-bot

Copy link
Copy Markdown
Collaborator

🤖 OpenHands is reviewing this PR.

Head commit: bc008f04c9edba380ab76f55c3824620025a9d1a
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/7be2fe4c-414f-4b82-bd4b-6113efadc51c

This comment was posted by an AI agent (OpenHands).

@all-hands-bot all-hands-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

🔴 Needs improvement

Two material issues remain in the core stream guarantees: split-token secrets can reach the wire unmasked, and stream ownership is committed before the durable event is known to exist.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🔴 HIGH — this changes the agent/server streaming path and currently has both a secret-disclosure path and a stranded-slot failure mode. Do not auto-merge; a human maintainer should validate the corrected streaming semantics and run lightweight evals because this touches agent execution behavior.

VERDICT:
Needs rework: Fix the masking and retirement ordering before merging.

KEY INSIGHT:
A streamed slot can only be declared safe once all bytes are masked across chunk boundaries and the matching durable event has actually crossed the persistence boundary.


Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

  1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
  2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
  3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

attempt=self._attempt,
order=next(self._order),
kind=kind,
content=self._mask(text),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Security: masking each chunk independently leaks secrets split across token boundaries. For a registered hunter2, two normal deltas hunt and er2 both pass this masker unchanged, and the client reconstructs the full secret on the wire. I reproduced that with this StreamContext: joining its emitted StreamDelta.content values produces hunter2. The later durable-event masking cannot retract already delivered bytes. Please use a streaming-aware masker that retains enough trailing input to match across chunk boundaries (and flushes safely when the stream closes), and add a regression test where the secret is split across chunks.

Comment thread openhands-sdk/openhands/sdk/agent/response_dispatch.py
@all-hands-bot

Copy link
Copy Markdown
Collaborator

⚠️ OpenHands PR Reviewer encountered a problem at commit bc008f04c9ed (status: error).

This comment was posted by an AI agent (OpenHands).

…ly once durable

Two defects found in review of the streaming identity work.

Masking each chunk independently cannot see a secret the provider splits
across token boundaries: for a registered `hunter2`, the deltas `hunt` and
`er2` both pass a per-chunk masker unchanged and the client reassembles the
value on the wire. Subword tokenization makes that the common case, not the
edge one. `StreamOutputMask` accumulates instead, releasing only the prefix
that no further input can change: it holds back the longest registered value
minus one character, so any match starting before the cut is already maximal.
`compile_output_mask` becomes `compile_stream_mask` and returns it; the tail
is flushed when the slot is claimed or aborted.

`claim()` retired the slot before the durable event existed. It set the
claimed flag, then the event was constructed and emitted, and `on_event` can
raise while persisting — callbacks are composed without a guard, so any
callback that raises reaches it. `__exit__` then saw a claimed slot, emitted
no `ItemAborted`, and left the client holding a slot no durable event would
ever close: the exact failure this work exists to prevent. Reserving the id
and retiring the slot are now separate, and the three durable-event sites
commit only after `on_event` returns.

Tests: the split value is masked across chunks and with one character per
chunk; a claim that never commits still aborts, both on the context and
end-to-end through a real Agent whose callback raises while persisting. Both
new assertions fail without the corresponding fix.
@all-hands-bot

Copy link
Copy Markdown
Collaborator

🤖 OpenHands is reviewing this PR.

Head commit: d3017a3444a63a21a46f9f74390c1a3abdb69182
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/3ec439e9-3864-409d-9765-540702dc6951

This comment was posted by an AI agent (OpenHands).

@all-hands-bot all-hands-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Reviewed at head d3017a3. The two defects flagged in the prior all-hands-bot review (against bc008f0) are now addressed by the second commit:

  • Split-token secrets are now masked. StreamOutputMask holds back max_len - 1 characters per masker and re-emits on flush(), so a secret split across chunk boundaries reassembles before release. compile_stream_mask() sorts values longest-first so an overlapping shorter value can't leave a suffix unmasked. The "only already-resolved values" limitation is a documented, load-bearing tradeoff (masks without resolving on the hot sync path, which runs under the state lock), and the durable MessageEvent remains the authoritative, fully-masked record.
  • The slot is retired only after the durable event crosses on_event. claim() reserves the id and commit() is now called after on_event(...) in the message, action, and ACP-finish paths, so a persistence failure leaves the slot open for the __exit__ abort (covered by test_a_persistence_failure_after_streaming_retires_the_slot).

No material code defects found in this pass: the claim/commit invariant is coherent, token_callback preserves the #4014 degradation when nothing consumes stream output, on_chunk forwards the raw chunk byte-for-byte so existing on_token consumers see no change, and the progress fan-out is distinct from the event bus (webhooks/telemetry/legacy endpoint unaffected).

⚠️ Not approving: eval risk. This touches the agent/LLM streaming path (agent.py, response_dispatch.py, acp_agent.py) — the durable MessageEvent/ActionEvent now carry a StreamContext-minted id, and the token callback is routed through StreamContext.on_chunk. Although the text delivered to on_token consumers and the persisted event content are unchanged, the repo review policy classifies anything that could plausibly affect agent execution as eval-risk. No eval-monitor link or human "eval looks good" confirmation is present, so this needs a human maintainer to run lightweight evals before merge.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ 🟡 MEDIUM — no correctness/security defect identified, but the change is in the agent streaming hot path and lacks eval validation against the review policy. A human maintainer should confirm no benchmark regression.

VERDICT: ✅ Worth merging pending human eval confirmation — core logic is sound; the remaining item is process (eval evidence), not code.

KEY INSIGHT: The design correctly treats the durable event as the single source of truth for slot retirement, with progress frames as recoverable, best-effort UX.

Base automatically changed from vasco/streaming-4681-session-socket to main September 3, 2026 06:15
@VascoSch92

Copy link
Copy Markdown
Member Author

@all-hands-bot fix the conflicts

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Python API breakage checks — ✅ PASSED

Result:PASSED

Action log

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

REST API breakage checks (OpenAPI) — ✅ PASSED

Result:PASSED

Action log

@all-hands-bot

Copy link
Copy Markdown
Collaborator

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streaming step 5: StreamContext mints the stream identity and guarantees every stream closes

4 participants