Skip to content

refactor(runtime): replace PersistentRuntime decorator with EventObserver#2552

Merged
dgageot merged 2 commits intodocker:mainfrom
dgageot:board/runtime-code-extension-points-analysis-776bc127
Apr 28, 2026
Merged

refactor(runtime): replace PersistentRuntime decorator with EventObserver#2552
dgageot merged 2 commits intodocker:mainfrom
dgageot:board/runtime-code-extension-points-analysis-776bc127

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Apr 27, 2026

Summary

Collapses the Runtime interface / LocalRuntime / PersistentRuntime triplet (~200 lines of wrapping decorator) into a single runtime type with an observer chain. Persistence becomes the stock EventObserver implementation, auto-registered against the configured session store. Custom observers (telemetry, audit, metrics, A2A forward, transcript writers, …) now compose alongside via WithEventObserver instead of forking the runtime.

Net change after both commits: +664 / −370 = +294 lines including new tests and rich doc comments. The actual pkg/runtime source is smaller: the deleted persistent_runtime.go (194 lines) is replaced by observer.go (81) + persistence_observer.go (170) = 251 lines, but those carry the new public API and ~80 lines of Go-doc.

Commits

1. refactor(runtime): introduce EventObserver, collapse PersistentRuntime decorator ( 70d4c78e7)

  • New EventObserver interface (originally three methods: OnRunStart / OnEvent / OnRunEnd; trimmed in commit 2).
  • WithEventObserver(o) opt for layering custom observers. Nil observers silently ignored so a conditional caller can pass nil without a guard.
  • LocalRuntime.RunStream fans events through the observer chain between the inner producer goroutine and the consumer's channel.
  • PersistenceObserver replaces persistent_runtime.go's handleEvent state machine. Same per-event-type switch, same sub-session and SessionScoped-mismatch filters, same streaming-message lifecycle — just decoupled from runtime construction. Auto-registered as the first observer in NewLocalRuntime so user-supplied observers see the post-persistence view.
  • pkg/runtime/persistent_runtime.go deleted (194 lines).
  • runtime.New() becomes a thin alias for NewLocalRuntime() returning the Runtime interface, kept for source compatibility with the ~10 callers in cmd/, examples/, e2e/, pkg/chatserver.
  • cmd/root/run.go drops its (*runtime.PersistentRuntime) type assertion in favour of localRt.TitleGenerator(), which has been on the Runtime interface all along.
  • New observer_test.go pins the lifecycle contract.

2. refactor(runtime): trim observer + approval-source ceremony ( 4e82c39fd)

Five small cleanups, no feature changes; net −60 lines on top of the introduction commit:

# Change Why
1 Drop EventObserver.OnRunEnd Only impl was a no-op kept "for symmetry"; YAGNI.
2 Move OnRunStart fan-out into observe() All observer interactions now live next to each other; RunStream loses its bespoke iteration loop.
3 Drop the no-observers fast path in observe() PersistenceObserver is auto-registered against the default in-memory store, so the branch was dead in practice.
4 Make TokenUsageEvent implement SessionScoped (one-method addition) Removes a duplicate "sub-session token guard" inside PersistenceObserver.OnEvent; the existing SessionScoped filter at the top of OnEvent now catches it uniformly with every other cross-session event.
5 Co-locate approval-source labels on permissionChecker allowSourceFor / denySourceFor were fragile string-equality lookups against magic strings duplicated in permissionCheckers(). Replaced with displayName / allowSource / denySource fields populated at construction time — typos caught at compile time.

Plus minor: inline streamingState reset, fold MessageAddedEvent's two error-logging branches into one, drop fnObserver.onEnd from the test helper, replace the now-obsolete approval-source mapper test with one that asserts on the struct fields.

Why this matters

Before:

runtime.New(team, runtime.WithSessionStore(store))
// LocalRuntime + PersistentRuntime decorator + Runtime interface triplet
// to host one (1) event observer.

After:

runtime.New(team,
    runtime.WithSessionStore(store),
    runtime.WithEventObserver(otelObserver),
    runtime.WithEventObserver(auditObserver),
    runtime.WithEventObserver(metricsObserver),
)
// One LocalRuntime, one observer chain. PersistenceObserver is just
// the first entry in r.observers.

The dual-runtime-class confusion is gone, and OTel/audit/metrics observers are now one struct + one WithEventObserver call away rather than a fork of the wrapping decorator.

Verification

  • go build ./...
  • go vet ./...
  • go test -short ./pkg/... ✅ (every package green)
  • go test ./e2e/... -run TestRuntime_MultiAgent ✅ — the test that specifically exercises the persistence + sub-session-filter contract that PR fix(#2053): prevent corrupted session history from breaking Anthropic API calls #2058 introduced; same behaviour preserved.
  • New observer_test.go and on_tool_approval_decision_test.go pin the new contracts.

Assisted-By: docker-agent

@dgageot dgageot requested a review from a team as a code owner April 27, 2026 16:32
@dgageot dgageot force-pushed the board/runtime-code-extension-points-analysis-776bc127 branch from 4e82c39 to 186782f Compare April 27, 2026 17:02
rumpl
rumpl previously approved these changes Apr 28, 2026
@dgageot dgageot force-pushed the board/runtime-code-extension-points-analysis-776bc127 branch from 186782f to 9806a83 Compare April 28, 2026 07:20
rumpl
rumpl previously approved these changes Apr 28, 2026
dgageot added 2 commits April 28, 2026 09:44
…e decorator

Replaces the wrapping-decorator pattern (LocalRuntime + 200-line

PersistentRuntime + Runtime interface triplet) with a small

observer chain owned by LocalRuntime itself.

  - New EventObserver interface with three lifecycle hooks:

    OnRunStart(ctx, sess), OnEvent(ctx, sess, e), OnRunEnd(ctx, sess).

    Documented contract: synchronous, in registration order, no

    error return; observers log internally and never block the

    run loop on long work.

  - New WithEventObserver(o) Opt for layering custom observers

    (telemetry, audit, metrics, A2A forward, ...). Nil observers

    are silently ignored so a conditional caller can pass nil

    without a guard.

  - LocalRuntime.RunStream fans events through the observer chain

    between the inner producer goroutine and the consumer's

    channel. Fast-path when no observers are registered: the inner

    channel is returned directly, so a no-observer runtime pays

    exactly the overhead it did before this commit.

  - PersistenceObserver replaces persistent_runtime.go's handleEvent

    state machine. Same per-event-type switch, same sub-session and

    SessionScoped-mismatch filters, same streamingState lifecycle —

    just decoupled from runtime construction. Auto-registered as

    the first observer in NewLocalRuntime (so user-supplied

    observers see the post-persistence view).

  - pkg/runtime/persistent_runtime.go is gone (194 lines).

  - runtime.New() becomes a thin alias for NewLocalRuntime()

    returning the Runtime interface, kept for source compatibility

    with the ~10 callers in cmd/, examples/, e2e/, pkg/chatserver.

  - cmd/root/run.go drops its (*runtime.PersistentRuntime) type

    assertion in favour of localRt.TitleGenerator(), which has

    been on the Runtime interface all along.

Net effect: 200 lines deleted, the dual-runtime-class confusion

gone, and OTel/audit/metrics observers are now one struct + one

WithEventObserver call away. Existing PersistenceObserver tests

are exercised by every runtime test that runs a stream (the

default in-memory store keeps the auto-registration path covered).

New observer_test.go pins the lifecycle contract: OnRunStart and

OnRunEnd fire exactly once, OnEvent sees every event in order, and

multiple observers fire in registration order. The fnObserver

test helper makes future observer-related tests one-liners.

Assisted-By: docker-agent
Subset of PR docker#2552 commit 9806a83 that survives rebase: drops EventObserver.OnRunEnd, moves OnRunStart fan-out into observe(), drops the no-observers fast path, makes TokenUsageEvent implement SessionScoped.

The approval-source ceremony cleanup no longer applies; that surface moved to pkg/runtime/toolexec on origin/main.

Assisted-By: docker-agent
@dgageot dgageot force-pushed the board/runtime-code-extension-points-analysis-776bc127 branch from 9806a83 to 73f8a28 Compare April 28, 2026 07:50
@dgageot dgageot merged commit 4bdc489 into docker:main Apr 28, 2026
5 checks passed
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.

2 participants