Skip to content

fix: EventBus loopback elimination + per-agent resource read truncation - #405

Merged
Million-mo merged 3 commits into
mainfrom
fix/eventbus-loopback-and-resource-config
Aug 29, 2026
Merged

fix: EventBus loopback elimination + per-agent resource read truncation#405
Million-mo merged 3 commits into
mainfrom
fix/eventbus-loopback-and-resource-config

Conversation

@Million-mo

Copy link
Copy Markdown
Collaborator

Summary

Two framework-level fixes previously merged as #391 and #394, replayed cleanly on top of main with all business-specific config removed.

1. EventBus loopback elimination (originally #391, issue #380)

wolfharness serve-opencode + opencode attach rendered the first user message twice in the TUI. Root cause: the OpenCode server republished its own protocol projections back into the EventBus, creating a feedback loop.

  • SSE direct-wire projections: ServerState.broadcast_event() delivers OpenCode projections straight to per-connection SSE subscriber queues. OpenCodeEventBridge (which republished into the EventBus) is deleted.
  • Loopback isolation: EventBus.publish() accepts source_hint; subscribe() accepts exclude_source, preventing a producer from re-consuming its own output.
  • Replay alignment: OpenCode session consumer subscribes with replay=False, matching SSE first-connect policy.
  • Reconnect replay preserved: per-session projection buffers in ServerState keep Last-Event-ID conditional replay working.

2. Per-agent resource read truncation + subscribe-on-read (originally #394)

  • ResourceConfig.max_text_chars: configurable per-agent truncation limit (default 10,000, min 100) via YAML. NativeAgent now constructs a per-agent ResourceCapability instance instead of sharing the pool-level one.
  • MCP subscribe-on-read: McpServerCap best-effort subscribes to resource URIs after read_resource(), enabling notifications/resources/updated. Tracked subscriptions auto-re-establish after reconnect and clean up on disconnect.

Test plan

  • uv run pytest -m unit passes
  • uv run pytest -m "e2e and not slow" passes
  • uv run ruff check src/ passes
  • uv run ruff format --check src/ passes

Million-mo and others added 3 commits August 29, 2026 14:44
…ections (#391)

* fix(opencode): eliminate EventBus loopback causing duplicate SSE projections

The OpenCodeEventBridge republished protocol projections (MessageUpdatedEvent,
PartUpdatedEvent, etc.) back into the same EventBus that carries native agent
events. This created a feedback loop: native events → event bridge → broadcast
→ EventBus republish → SSE delivery, causing duplicate renders in attached
OpenCode TUI clients (issue #380).

Architecture change — direct-wire SSE:
- state.broadcast_event() now fans projections directly to per-connection SSE
  subscriber queues instead of republishing to EventBus
- global_routes._event_generator reads from state.event_subscribers queues
  (no EventBus subscription, no CustomEvent unwrapping)
- Reconnect replay via state.replay_projections() using Last-Event-ID
- Deleted event_bridge.py (the loopback republisher)

EventBus source isolation (defense-in-depth):
- EventEnvelope gains source_hint field; publish() accepts source_hint
- subscribe() accepts exclude_source param (filters both live fanout and replay)
- ProtocolEventConsumerMixin hooks: _get_subscription_replay() and
  _get_subscription_exclude_source() (defaults: replay=True, exclude=None)
- OpenCode overrides: replay=False, exclude_source={"opencode_event_bridge"}

Session consumer replay alignment:
- OpenCode session-level consumers now use replay=False (matching the global
  SSE endpoint's first-connect policy), preventing stale events from being
  redelivered on consumer startup

Testing:
- 7082 tests passed (full suite), ruff/mypy clean
- New e2e test: test_attach_existing_session_first_prompt_renders_once
- New unit tests: EventBus source_hint/exclude_source (4 tests)
- Rewritten integration tests for direct-wire SSE model

Note: A residual TUI-side duplication may still be visible in opencode attach
mode due to the TUI's local echo (createUserMessage) not matching the
server-generated message ID in the SSE event. This is tracked as an opencode
TUI bug (anomalyco/opencode#14372, #24773, #29478) with upstream fix PR #31945
still unmerged. The server-side fix in this commit eliminates the EventBus
loopback path; the remaining duplication is purely client-side.

* fix(opencode): address review — QueueFull policy, typed session-id extraction, mock alignment

Review-driven fixes on PR #391 (direct-wire SSE loopback elimination):

BLOCKER: broadcast_event now handles asyncio.QueueFull per-subscriber with
the same drop-oldest policy as EventBus._enqueue, so a stalled SSE client
can no longer abort fanout to every other subscriber. Adds structured
warning logging on overflow plus a debug fanout log (telemetry on the
delivery critical path). New regression test:
test_broadcast_event_drop_oldest_on_queue_full.

MAJOR: ServerState.extract_session_id now delegates to the typed
global_routes._extract_session_id (match-based, no getattr) instead of the
getattr probe that read info.id for MessageUpdatedEvent — buffering
message.updated under per-message keys and leaking memory. Typed variant
reads props.info.session_id as documented. Test mocks in
test_global_event.py / test_sse_compliance.py aligned with production:
deque(maxlen=100) buffers, drop-oldest overflow, typed extractor — the
queue-full behavior is no longer suppressed out of the green suite.

MINOR: replay_projections replays merged buffers in global event_id order
(monotonic SSE ids for reconnecting clients) and stops with a warning on
QueueFull instead of silently dropping a suffix.

MINOR: _get_subscription_exclude_source documented as currently inert
(its only producer was deleted with the loopback bridge; kept as
defense-in-depth, exercised by unit tests).

Nits: stale event_bridge docstrings/comment/names updated across
test_event_pipeline_e2e.py and conftest.py; corrected the pre-existing
dedup-set claim in src/wolfharness/AGENTS.md (the set is a private
ACPEventConverter field, not on SessionController); changelog trailing
newline; ADR eventbus-replay.md annotated as superseded by the
direct-wire design (PR #391).

Verified: ruff clean, mypy strict clean (686 files), 326 affected tests
pass.
…align (#394)

* feat(resource): per-agent max_text_chars config + kb_diag_agent YAML align

ResourceConfig now accepts max_text_chars (default 10000, min 100) in
agent YAML config. NativeAgent creates a per-agent ResourceCapability
with the agent's max_text_chars instead of sharing the pool-level instance.

ResourceCapability.__init__ gains max_text_chars parameter (backward
compatible). Truncation suffix improved with guidance directing the model
to use narrower URIs or paginated read tools.

kb_diag_agent.yaml aligned with live knowledge_diag server v3.4.4:
- Enabled search_kb (removed from disabled_tools)
- Added get_doc_toc and read_chapter_page tool-schema-overlap rewrites
- Added search_kb rewrite with methods (FULL/FAST/WIKI) param docs
- Updated existing tool descriptions to reference page-based workflow

Supersedes PR #393.

* feat(mcp): resource subscribe-on-read wiring in McpServerCap

Best-effort subscribe to resource URIs after successful read_resource()
calls, enabling notifications/resources/updated for resources the agent
has read. Tracked subscriptions are re-established on reconnect and
cleaned up on disconnect.

No-op for servers with subscribe:false (like knowledge_diag v3.4.4) —
subscribe fails silently, read proceeds normally. Activates automatically
when server enables subscription support.

* fix(mcp): address PR #394 review — wire max_text_chars, fix broken test, consolidate truncation

- Wire self._max_text_chars into read_mcp_resource (was hardcoded
  _DEFAULT_READ_TEXT_LIMIT) and use _truncate_text helper with guidance
  suffix
- Remove dead _truncate_text static method from ResourceCapability
  (zero callers, old suffix format)
- Add constructor validation: max_text_chars < 100 raises ValueError
- Consolidate default constant: _DEFAULT_MAX_TEXT_CHARS in
  resource_resolver.py, aliased in resource_capability.py
- Fix broken test assertion in test_resource_resolution.py to match
  new guidance suffix format
- Add tests: max_text_chars validation, read_mcp_resource truncation
  with per-agent limit, suffix guidance text
- Correct changelog: limit was previously hardcoded, not a pre-existing
  constructor param
- Update capabilities/AGENTS.md: ResourceCapability is per-agent
  constructed, not registered at SESSION scope
@github-actions

Copy link
Copy Markdown

Review: PR #405 — EventBus loopback elimination + per-agent resource read truncation

Solid, well-tested replay of #391/#394. The EventBus source_hint/exclude_source work is cleanly typed, the direct-wire SSE rewrite genuinely tests the new fanout/buffer/replay semantics (not just changed mocks), and broadcast_event's overflow/dead-queue handling faithfully mirrors EventBus._enqueue. No type suppressions added, no circular imports, and zero stale references to the deleted event_bridge.py module in src//tests/. Issues below are mostly around untested subscribe-on-read wiring, one flaky e2e sync, and docs that now contradict the code.

Important

  1. Subscribe-on-read is wired into the legacy path only — not the model-facing read tool. read_resource() (src/wolfharness/capabilities/mcp_server_cap.py:590) gets the subscribe; read_mcp_resource() (:517) does not. The agent's actual reads go through ResourceCapability.read_mcp_resource (resource_capability.py:711provider.read_mcp_resource), so the changelog headline ("enables notifications/resources/updated for resources the agent has read") doesn't hold for the primary agent read path — only for resolve_resource_content/the legacy read_resource tool. Either add the subscribe to read_mcp_resource, or scope the changelog claim.

  2. McpServerCap subscribe-on-read has zero direct test coverage. tests/capabilities/test_mcp_server_cap.py is unmodified. Nothing exercises _subscribed_uris population, _resubscribe_all() (the existing reconnect test reconnects with an empty set, so it's a no-op there), unsubscribe-on-cleanup, or the "server rejects subscribe → read still proceeds" fallback. Per tests/AGENTS.md, this new behavior needs tests.

  3. New e2e test's turn-1 idle wait can never succeed — 20s burn, latent flake. tests/e2e/test_user_message_no_duplicate_e2e.py:1318-1324 polls GET /session/{id} and checks data.get("status") == "idle", but the Session model (models/session.py:45) has no status field — status lives on /session/{id}/status or SSE session.status. The loop always times out. Tests pass because turn 1 finishes before the attach stream connects, but it wastes ~20s in an L4a smoke test (~30s budget) and will flake under a slow model. Poll /session/{id}/status or wait on SSE like the sibling tests.

Minor

  1. src/wolfharness_server/AGENTS.md:116 still documents the old dedup design and now contradicts the updated src/wolfharness/AGENTS.md:110. It claims a per-session dict[str, set[str]] on SessionController passed as displayed_message_ids — that field no longer exists anywhere; the set is _displayed_message_ids on ACPEventConverter (verified event_converter.py:269). Two sibling AGENTS.md files disagree with each other and with the code.

  2. src/wolfharness/AGENTS.md:110 — two factual slips in the rewritten paragraph. (a) "populated by the ACP protocol handler's _emit_user_message_chunks()" is wrong: that method (acp_server/handler.py:854) never touches the set — the converter populates it itself at event_converter.py:607. (b) "(keyed by session_id)" misdescribes it: it's a plain set[str], effectively per-session only because converters are created per session.

  3. No docs/explanation/ doc describes the current SSE direct-wire architecture. The ADR's superseded note points readers to docs/explanation/, but no file there covers broadcast_event fanout, replay_projections/Last-Event-ID, or the "EventBus carries only native agent events" invariant — the exact architecture this PR establishes. Given docs/meta/documentation-guide.md maps architecture explanation → docs/explanation/, a short page (e.g. opencode-sse-event-flow.md) plus a Context Loading row in the root AGENTS.md is warranted. The root table is otherwise not broken by this PR.

  4. Dead mock classes left behind. _MockEventBus/_MockSessionPool/_MockPool remain defined in test_global_event.py:200-259 and test_sse_compliance.py:77-136 but their only instantiation site was removed. Delete them.

  5. Last-Event-ID reconnect replay is never exercised end-to-end. replay_projections() is unit-tested, but no test calls _event_generator(..., last_event_id=...) — the parsed_last_event_id → state.replay_projections() wiring in global_routes.py:243-245 is untested, and it's the headline "reconnect replay preserved" behavior.

  6. Changelog feat(tools): implement extended tool definitions with native Pydantic… #2 overstates the subscribe no-op and leaks the stripped business config. MCPClient.subscribe_resource performs a real RPC on every read_resource for non-subscribe servers (failure swallowed at debug level) — not a "no-op", and a URI that fails on reconnect is retried forever since _resubscribe_all never prunes. Also the slug/title ...-kb-diag-config-align.md and the YAML comment "# allow longer chapter reads for KB agents" are the only remnants of feat(resource): per-agent max_text_chars config + kb_diag_agent YAML align #394's kb_diag_agent alignment, which the PR body says was deliberately removed — rename the file and drop the comment.

  7. Stale architecture narratives in unchanged tests/docs. tests/test_duplication_reproduction.py and tests/test_sse_conditional_replay_integration.py:229-244 still describe the removed CustomEvent bridge; docs/ops/opencode-handler.md:45 documents the queue-overflow log message this PR rewrote; docs/records/audit/opencode-client-audit.md asserts "no replay" and references the now-removed state._broadcast_event_impl() and a non-existent docs/design/eventbus-replay.md — candidate for a "superseded by fix(opencode): eliminate EventBus loopback causing duplicate SSE projections #391/fix: EventBus loopback elimination + per-agent resource read truncation #405" note. docs/rfcs/draft/RFC-0057 line refs to opencode_event_bridge.py:368,638 drifted (638 now lands in the RunFailedEvent path).

Nit

  • agent.py:1168 if resource_cap not in self._external_capabilities is always-true for a freshly built instance (identity equality) — harmless, but dead. Note pool.resource_capability is not dead: still constructed and consumed by agent_routes.py for the OpenCode /agent tool listing.
  • _get_subscription_replay/_get_subscription_exclude_source hooks: replay=False is real, but exclude_source is genuinely inert in production (no producer calls publish(source_hint=...) post-deletion). Documented as defense-in-depth and unit-tested — reasonable to keep, just calling out the "Recovery paths opt in to replay=True" docstring is aspirational (nothing in production overrides it back).
  • Telemetry: broadcast_event/replay_projections and the subscribe-on-read paths add only logger.debug/warning, no logfire spans — consistent with the rest of wolfharness_server/opencode_server/ (zero logfire usage), so not a new regression, but these are the protocol entry points AGENTS.md calls out for instrumentation.

Verified non-issues

  • Function-body import of ResourceCapability in agent.py matches the established pattern in get_agentlet (checkpoint, MCP, skill caps all do it).
  • _SubscriberEntry unpacking is consistent at all four sites; _rebind propagates source_hint; exclude_source composes correctly with replay + last_event_id.
  • broadcast_event fanout is loop-atomic (no await), dead queues pruned post-loop, drop-oldest retry correct; the _event_generator finally block can't interleave.
  • Wire ordering connected → replayed → live is correct (replay enqueued before the connected yield).
  • Changelog files follow changelog/README.md format; technical claims in them check out against code.
  • Both updated subsystem AGENTS.md rows (pipeline row, per-agent ResourceCapability scope) are accurate; capabilities/AGENTS.md per-agent claim verified against agent.py.

@github-actions

Copy link
Copy Markdown

Posted the synthesized review at #405 (comment)

Verdict: solid replay, approve-with-fixes. The EventBus source_hint/exclude_source work is cleanly typed and well-tested; no stale references to the deleted bridge; no type suppressions. Key findings (each verified against source):

Important

  • Subscribe-on-read only wired into the legacy read_resource() path, not the model-facing read_mcp_resource() tool — changelog overclaims.
  • McpServerCap subscribe-on-read has zero test coverage (test_mcp_server_cap.py untouched).
  • New e2e test polls data.get("status") on GET /session/{id}, but the Session model has no status field — the 20s idle-wait always times out (latent flake + 20s burn in a ~30s-budget L4a test).

Docs (from the docs specialist)

  • src/wolfharness_server/AGENTS.md:116 still documents the deleted SessionController dedup set, now contradicting the PR's own update to src/wolfharness/AGENTS.md:110.
  • src/wolfharness/AGENTS.md:110 misattributes the dedup population to _emit_user_message_chunks() (it's the converter itself) and calls the set "keyed by session_id" (it's a plain set).
  • No docs/explanation/ page covers the new SSE direct-wire architecture, leaving the ADR's "see docs/explanation/" pointer dangling; the root Context Loading table needs a row once added.

Minor/Nit: dead mock classes left behind, Last-Event-ID reconnect replay never exercised end-to-end, misleading kb-diag-config-align changelog slug (business config was stripped), stale test/doc narratives about the removed bridge, always-true dedup guard in agent.py, and exclude_source being genuinely inert defense-in-depth.

Verified non-issues: fanout atomicity/ordering, _SubscriberEntry consistency, _rebind provenance, changelog format compliance.

New%20session%20-%202026-08-29T06%3A44%3A43.017Z
opencode session  |  github run

@Million-mo
Million-mo merged commit fb232a1 into main Aug 29, 2026
12 checks passed
@Million-mo
Million-mo deleted the fix/eventbus-loopback-and-resource-config branch August 29, 2026 07:19
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.

1 participant