feat(server): instructions accepts a function form (lazy/per-session) — closes #1347#1374
Merged
Merged
Conversation
Closes #1347. `createAdcpServer.instructions` and `DecisioningPlatform.instructions` now accept either a static string (historical form) or a `(ctx: SessionContext) => string | undefined` function. Under `serve({ reuseAgent: false })` (the default) the factory runs per HTTP request, so the function fires per session — adopters surface tenant-shaped prose (per-buyer brand manifests, storefront copy, premium-vs-standard partner guidance) without a Map shim outside the SDK. Three guardrails resolve the design questions raised in #1347 triage: 1. reuseAgent: true + function-form is refused. The function would only fire once for the lifetime of the shared agent; silently degrading to "constant after all" is worse than failing loud. `serve()` returns 500 with a message naming the workaround. 2. Async return is not yet supported. A function returning a Promise throws ConfigurationError at construction; pre-resolve in caller code if you need to fetch. 3. SessionContext is reserved. authInfo + agent are typed for forward compatibility but currently undefined — the framework does not yet plumb auth/registry state into the factory before MCP initialize. Closures in HTTP-scoped factory state are the today-pattern for tenant identity. Throw semantics via onInstructionsError: - 'skip' (default): log server-side, treat as undefined. Right for prose-of-flavor where a registry fetch failure must not kill the buyer's session. - 'fail': rethrow → MCP initialize transport-level failure. Right when instructions carry load-bearing policy where stale/missing guidance is worse than a connection retry. New exports from @adcp/sdk/server: - SessionContext type - OnInstructionsError type - ADCP_INSTRUCTIONS_FN symbol — serve() reads it to refuse reuseAgent + function combination Plumbed through DecisioningPlatform.instructions (v6 surface) + createAdcpServer.instructions (v5 escape hatch); platform wins when both supplied, same precedence as agentRegistry. Tests: 10 new in test/server-instructions-fn.test.js - string form regression (passes through unchanged) - function evaluated at construction; re-eval per createAdcpServer call drives per-session semantics under reuseAgent: false - ADCP_INSTRUCTIONS_FN marker presence/absence - function returning undefined → no instructions, marker still set - SessionContext shape (currently empty, future-compatible) - onInstructionsError 'skip' (default) catches throws - onInstructionsError 'fail' rethrows - 'skip' explicit choice matches default - Promise return → ConfigurationError regardless of onInstructionsError Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses code-reviewer + dx-expert reviews on PR #1374. Fixes (code-reviewer): - Promise detection guarded for `null` + non-object before reading `.then`, so `() => null` no longer crashes with TypeError. Surfaces a framework-internal stack only when the user genuinely returned a thenable. - Reject non-string non-undefined returns with a clear error message instead of silent `String({})` → "[object Object]" coercion. Routed through the same try/catch as user-thrown errors so onInstructionsError governs both — a buggy callback is the same shape of problem either way. (Promise return remains a hard ConfigurationError because the framework can't recover.) - Hint included in `serve()` 500 JSON body (was log-only) so adopters whose harness only surfaces the wire response see the workaround. DX (dx-expert): - JSDoc on `createAdcpServer.instructions` tightened: replaces compressed "per session" wording with the truthful "once per createAdcpServer invocation, which under serve({ reuseAgent: false }) is per HTTP request, which is per session for streamable-HTTP MCP." Calls out custom transports. - Added `@example` block showing the factory-closure pattern (`{ host }` captured outside `ctx`) — pre-empts the cargo-cult `ctx.agent?.agent_url` read that today silently returns undefined. - `SessionContext.{authInfo,agent}` JSDocs strengthened with `@reserved` framing and explicit "use closures, not ctx" steering, so coding agents scraping the type don't reach for the reserved fields. Tests added (4 new): - `() => null` round-trip (regression for the null crash fix) - non-string return rejected with onInstructionsError: 'fail' - non-object non-string return rejected - 'skip' default catches bad-type returns and resolves to undefined Total now 14 tests in test/server-instructions-fn.test.js (all pass). 218/218 pass across the wider config-related suite (server-decisioning-*, server-create-adcp-server). Format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 3, 2026
Closed
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.
Closes #1347.
Lifts the captured-once limitation on
instructions.createAdcpServer.instructionsandDecisioningPlatform.instructionsnow accept either a staticstring(historical form) or a(ctx: SessionContext) => string | undefinedfunction. Underserve({ reuseAgent: false })(the default) the factory runs per HTTP request, so the function fires per session — adopters surface tenant-shaped prose (per-buyer brand manifests, storefront-platform copy, premium-vs-standard partner guidance) without aMapshim outside the SDK.Surface
Three design questions resolved (per the #1347 triage thread)
SessionContextshape — slim{ authInfo?: ResolvedAuthInfo; agent?: BuyerAgent }. Both reserved; currently alwaysundefinedbecause the framework doesn't yet plumb auth/registry state into the factory before MCPinitialize. Today-pattern for tenant identity is closures captured in HTTP-scoped factory state. Forward-compatible: when the framework wires authInfo/agent through, existing function bodies pick up populated fields without changes.reuseAgent: true+ function-form is refused. The function would only fire once for the lifetime of the shared agent — silently degrading to "constant after all" is worse than failing loud.serve()returns 500 with a message naming the workaround (dropreuseAgent: truefor fresh agent per request, OR pass a static string).Throw semantics via
onInstructionsError:'skip'(default) — log server-side, treat asundefined. Right for prose-of-flavor (brand manifests, marketing copy) where a registry fetch failure must not kill the buyer's session.'fail'— rethrow → MCPinitializetransport-level failure. Right when instructions carry load-bearing policy where stale/missing guidance is worse than a connection retry.What ships
createAdcpServer.instructionstype extended;onInstructionsErroroption added (default'skip').DecisioningPlatform.instructionstype extended;onInstructionsErrorplumbed throughfrom-platform.ts. Platform wins overopts.instructionswhen both supplied (same precedence asagentRegistry).SessionContext+OnInstructionsErrortype exports from@adcp/sdk/server.ADCP_INSTRUCTIONS_FNsymbol export —serve()reads it to detect function-form servers and refusereuseAgent: true.PromisethrowsConfigurationErrorat construction (regardless ofonInstructionsError); pre-resolve before invokingcreateAdcpServerif you need to fetch. Future-compat: type signature accepts only sync return today; future revisions can extend.Tests (
test/server-instructions-fn.test.js)10 new tests cover:
createAdcpServercall drives per-session semantics underreuseAgent: falseADCP_INSTRUCTIONS_FNmarker presence (function form) / absence (string form)undefined→ no instructions, marker still setSessionContextshape (currently empty, future-compatible)onInstructionsError: 'skip'(default) catches throws — function returning throw resolves to undefinedonInstructionsError: 'fail'rethrows'skip'matches defaultConfigurationErrorregardless ofonInstructionsError214/214 pass across the broader server-config test suite (no regressions).
Test plan
npm run buildnpx tsc --noEmitnpm run format:checknode --test test/server-instructions-fn.test.js(10/10)node --test test/server-decisioning-from-platform.test.js test/server-decisioning-instructions.test.js test/server-create-adcp-server.test.js(204/204)🤖 Generated with Claude Code