From 8f0d1235014550932c697f5019c7f172b60e5554 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 17 Aug 2026 18:06:28 +0200 Subject: [PATCH] docs(agent): align context design notes with implementation --- packages/agent/docs/rpc.md | 32 +++++++-------- packages/agent/docs/telemetry.md | 67 +++++++++++++++++++------------- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/packages/agent/docs/rpc.md b/packages/agent/docs/rpc.md index 83613bafa..f9ad2391f 100644 --- a/packages/agent/docs/rpc.md +++ b/packages/agent/docs/rpc.md @@ -1,6 +1,6 @@ # Harness RPC Design Notes -> **Status:** Design input, not a normative contract or implementation handoff. After the invocation-context interface migration lands, reconcile these notes with the actual interfaces, resolve the open decisions, and fold the accepted behavior into `harness.md` before creating a work package. +> **Status:** Design input, not a normative contract or implementation handoff. The context references here match `src/harness/context.ts`; RPC integration and context propagation through the harness remain design work. Resolve the open decisions and fold the accepted behavior into `harness.md` before creating a work package. ## Goal @@ -21,16 +21,16 @@ AgentHarness / AgentLane / Session / SessionTree Invocation context kills two integration problems at the boundary: - the adapter reconstructs the telemetry parent for the server invocation; -- the adapter maps request cancellation/disconnect to `context.signal`. +- the adapter maps request cancellation/disconnect to `context.abortSignal`. -The core sees only an ordinary explicit invocation context. +The core sees only an ordinary explicit `Context`. ## Non-goals Do not: - add RPC methods, reference types, request IDs, or transport state to real harness/session implementations; -- serialize `InvocationContext`, `AbortSignal`, `TelemetryContext`, hooks, tools, or callbacks; +- serialize `Context`, `AbortSignal`, `TelemetryContext`, hooks, tools, or callbacks; - make the core depend on one transport or telemetry backend; - use `AsyncLocalStorage` for parent discovery; - hide durable cancellation behind an RPC disconnect; @@ -53,7 +53,7 @@ Current examples requiring review include `SessionReader.getEntries()`, which re ## Context position -A generic proxy can support context first, context last, or no context by using a runtime brand or method metadata. Position does not solve method discovery by itself. +The implemented `Context` has no runtime brand, so a generic proxy must use trusted method metadata to distinguish context first, context last, and no-context methods. Position does not solve method discovery by itself. Context first is preferable under a strict JSON argument contract: @@ -96,13 +96,13 @@ Conceptually, the client proxy performs: ```text method invocation -→ find and remove branded InvocationContext +→ use method metadata to remove Context from its declared position, if present → validate remaining arguments as JSON → start rpc.client span → inject trace carrier → allocate request ID → send target + method + arguments + trace carrier -→ on context.signal abort, send cancel(request ID) +→ if context.abortSignal exists, send cancel(request ID) when it aborts → decode JSON result or remote protocol envelope ``` @@ -131,10 +131,10 @@ The server performs: receive request → validate target, method, and JSON arguments → allocate request-local AbortController +→ derive cancellation context with withAbortSignal(controller.signal, BACKGROUND_CONTEXT) → extract trace carrier into local TelemetryContext -→ construct fresh local InvocationContext → start rpc.server span -→ derive context containing rpc.server span +→ derive Context with withTelemetryContext(serverSpan, cancellationContext) → insert context at declared position → invoke exposed real method or adapter override → validate result @@ -272,7 +272,7 @@ type EventFrame = The server adapts host-local `events.on()` into this protocol. The real event bus remains unaware of RPC. -Each event frame carries trace metadata from the context that emitted the event. The client reconstructs a fresh event-delivery context and invokes local listeners under that parent. It does not receive the server's `InvocationContext` object or arbitrary context values. +Each event frame carries trace metadata from the context that emitted the event. The client reconstructs a fresh event-delivery context and invokes local listeners under that parent. It does not receive the server's `Context` object or arbitrary context values. Subscriptions are ongoing invocation-scoped resources. They may retain their creation context and own cancellation controller; ordinary harness/session proxies may not. Aborting the subscription context or disconnecting sends/unconditionally performs unsubscribe. @@ -294,16 +294,16 @@ A future client facade may reconstruct the familiar local `Events`/`WatchHandle` ## Request cancellation -The adapter maps each invocation signal to one request ID: +The adapter maps each invocation's `abortSignal`, when present, to one request ID: ```text -client signal abort +client context.abortSignal aborts → cancel(request ID) → server request AbortController aborts -→ reconstructed context.signal aborts +→ reconstructed context.abortSignal aborts ``` -Pre-aborted calls must not start server work. Disconnect aborts all active request controllers for that connection. +An undefined client `abortSignal` means that the call has no client-side cancellation signal. Pre-aborted calls must not start server work. Disconnect aborts all active request controllers for that connection. This is process-local invocation cancellation. It must not call `requestAbort()` or write durable `cancel_requested` automatically. @@ -311,7 +311,7 @@ This is process-local invocation cancellation. It must not call `requestAbort()` RPC does not decide whether a `drive()` caller installs execution or joins an existing process-local owner. The core lane arbitration decides that after receiving the reconstructed context. -Every concurrent RPC has an independent signal and telemetry parent. Never combine every joiner's signal into the active execution signal. A canceled joiner must cancel only its own wait. +Every concurrent RPC has an independent `abortSignal` and telemetry parent. Never combine every joiner's signal into the active execution signal. A canceled joiner must cancel only its own wait. The runtime must choose one execution ownership policy: @@ -385,7 +385,7 @@ Because the superseded harness implementation has been removed, all eventual sem ## Open decisions before `harness.md` -- final context type, field names, brand, and argument position; +- context argument position on receiver methods; - whether all ordinary results must be JSON-compatible or some interfaces need RPC projections; - static contract-checking API and test location; - object-reference ownership, lifetime, and nested result encoding; diff --git a/packages/agent/docs/telemetry.md b/packages/agent/docs/telemetry.md index 75ba195d6..346dce58b 100644 --- a/packages/agent/docs/telemetry.md +++ b/packages/agent/docs/telemetry.md @@ -1,6 +1,6 @@ # Invocation Context and Telemetry Design Notes -> **Status:** Design input, not a normative contract or implementation handoff. After the invocation-context interface migration lands, reconcile these notes with the actual types, resolve the open decisions, and fold the accepted design into `harness.md`. `telemetry-schema.md` remains the generated reference for span names and attributes. +> **Status:** Design input, not a normative contract or implementation handoff. The context primitives described here match `src/harness/context.ts`; propagation through the harness and the remaining runtime decisions are still design work. Fold the accepted design into `harness.md` when that work lands. `telemetry-schema.md` remains the generated reference for span names and attributes. ## Goal @@ -9,39 +9,49 @@ The invocation context must solve two related problems without `AsyncLocalStorage`: 1. preserve correct telemetry parentage through concurrent asynchronous work; -2. carry the `AbortSignal` that an RPC adapter can map to request cancellation. +2. carry an `AbortSignal`, when one exists, that an RPC adapter can map to request cancellation. This work must reuse `@earendil-works/pi-telemetry`. It must not introduce another span abstraction. -## Target context model +## Context model -The final names still need reconciliation with the interface migration. The intended semantics are: +The implemented public types are: ```ts -interface InvocationContext { - readonly signal: AbortSignal; - readonly telemetry: TelemetryContext; +interface ContextKey { + readonly token: symbol; + readonly valueType?: (value: T) => T; +} + +interface Context { + readonly abortSignal: AbortSignal | undefined; + readonly telemetryContext: TelemetryContext; value(key: ContextKey): T | undefined; + toString(): string; } ``` -A context is immutable. Derivation creates a parent-linked layer: +`valueType` is a type-only marker. Runtime lookup uses the key's symbol token. `createContextKey(description)` creates and freezes a key with a unique token. + +A context is immutable. Derivation creates a parent-linked, copy-on-write layer. Helper arguments put the value first and parent context last: ```ts -const requestContext = withSignal(parentContext, requestSignal); -const spanContext = withTelemetry(requestContext, span); -const tenantContext = withContextValue(spanContext, tenantKey, tenantId); +const requestContext = withAbortSignal(requestSignal, parentContext); +const spanContext = withTelemetryContext(span, requestContext); +const tenantContext = withContextValue(tenantKey, tenantId, spanContext); ``` -Required properties: +The implemented behavior is: -- `signal` is always present. The background context uses a never-aborted signal rather than `undefined`. -- `telemetry` is always present. The background context uses `NOOP_TELEMETRY_CONTEXT`. -- typed values use symbol identity and immutable copy-on-write layers; -- context values are cross-cutting request metadata, not business dependencies; -- contexts, signals, telemetry objects, and backend-native span objects are never durable data. +- `BACKGROUND_CONTEXT` and `TODO_CONTEXT` are distinct empty roots whose `abortSignal` is `undefined`; +- `telemetryContext` is always available and falls back to `NOOP_TELEMETRY_CONTEXT` when no telemetry value has been installed; +- `withAbortSignal(signal, context)` preserves the supplied signal when the parent has none and otherwise combines it with the parent signal using `AbortSignal.any()`; +- `withCancel(context)` returns an independently cancellable child context and a `cancel(reason?)` function; parent cancellation still reaches the child; +- typed values use symbol identity and immutable copy-on-write layers, and a newer value for the same key shadows its parent value; +- the built-in abort-signal and telemetry keys are private; callers use the named properties instead of retrieving those values by key; +- `toString()` is diagnostic and records the root plus each layered key description. -Suitable typed values include request IDs, authenticated principals, tenant IDs, and diagnostic metadata. Storage, models, tools, durable state, and business payloads do not belong in the context. +Context values are cross-cutting request metadata, not business dependencies. Suitable typed values include request IDs, authenticated principals, tenant IDs, and diagnostic metadata. Storage, models, tools, durable state, and business payloads do not belong in the context. Contexts, signals, telemetry objects, and backend-native span objects are never durable data. ## Receiver ownership @@ -75,11 +85,11 @@ Starting a harness span must derive the invocation context passed to lower work: ```ts return startHarnessSpan( - context.telemetry, + context.telemetryContext, "pi.harness.run", attributes, async (span) => { - const runContext = withTelemetry(context, span); + const runContext = withTelemetryContext(span, context); return runDrive(runContext); }, ); @@ -94,8 +104,8 @@ Do not mutate a context to install an active span. Do not use a process-global o Explicit propagation supports concurrent sibling calls: ```ts -await parent.telemetry.startSpan({ name: "caller" }, async (callerSpan) => { - const callerContext = withTelemetry(parent, callerSpan); +await parent.telemetryContext.startSpan({ name: "caller" }, async (callerSpan) => { + const callerContext = withTelemetryContext(callerSpan, parent); await Promise.all([ laneA.drive(callerContext, optionsA), laneB.drive(callerContext, optionsB), @@ -151,12 +161,14 @@ Distributed traces permit an execution span to outlive the installer RPC span. P An aborted invocation signal is process-local control. It does not mean that durable cancellation was requested. ```text -context.signal aborts +context.abortSignal is present and aborts → stop or detach process-local waiting/work according to drive ownership policy → do not write cancel_requested → preserve a valid durable restart point ``` +An undefined `abortSignal`, as exposed by both empty roots, means that the invocation has no cancellation signal. + Only `requestAbort()`/`abort()` writes durable `cancel_requested` and permits durable aborted settlement. The runtime must track the stop cause instead of interpreting every aborted provider response as durable cancellation: @@ -224,8 +236,9 @@ This migration can make the repository compile before semantics are implemented. ## Required tests for the later handoff -- immutable typed-value layering and key isolation; -- never-aborted background signal; +- immutable typed-value layering, shadowing, and key isolation; +- empty roots expose an undefined abort signal and no-op telemetry; +- parent/child abort composition and sibling cancellation isolation; - crossed concurrent telemetry branches on one shared receiver; - nested hook, tool, event-handler, and session-write parentage; - buffered events retain their emitting context; @@ -242,9 +255,7 @@ This migration can make the repository compile before semantics are implemented. ## Open decisions before `harness.md` -- final type and field names; -- context argument position; -- runtime brand and public visibility; +- context argument position on receiver methods; - installer-owned, lease-owned, or harness-owned drive execution; - whether telemetry links are required for joiners; - trace-carrier adapter ownership and shape;