Upgrade all deps (AI SDK v7, zod v4, TS 7, OTel v2), migrate & harden; add tool approval + active readiness probe#17
Open
sebastianwessel wants to merge 30 commits into
Open
Conversation
Upgrade the AI SDK family (ai 6->7, providers 3->4, mcp 1->2), zod 3->4, and the OpenTelemetry packages to their v2 line, and migrate the code: - AI SDK v7: stepCountIs -> isStepCount, fullStream -> stream, experimental_telemetry -> telemetry (drop redundant isEnabled), and allowSystemInMessages: true where the loop passes trusted server-built system messages into the array. - AI SDK v7 telemetry moved to @ai-sdk/otel: register the OpenTelemetry integration in startTracing so annotated model calls still emit spans. - @ai-sdk/mcp v2 graduated createMCPClient/MCPClient out of experimental. - zod v4: z.record now requires an explicit key type. - OpenTelemetry v2: resourceFromAttributes + ATTR_SERVICE_* replace the removed Resource class and SemanticResourceAttributes.
…6, msw) Upgrade the test/lint toolchain and migrate the Biome config to the v2 format (files.includes, rules.preset). Biome 2 autofixes merged duplicate type imports and replaced Object.prototype.hasOwnProperty.call with Object.hasOwn. Update the @ai-sdk/mcp test mock to the stable createMCPClient export.
Remove the minimumReleaseAge policy from pnpm-workspace.yaml (per request) and upgrade to the newest published versions across the board: TypeScript 7.0.2 (native compiler), ai 7.0.19 and the @ai-sdk/* 4.x/3.x/2.x line, @opentelemetry/semantic-conventions 1.43. Typecheck, build (tsc 7), the full test suite, and lint all pass unchanged.
Error handling & resilience: - serve: guard config load (log + shutdown tracing + exit 1) instead of crashing past the top-level handler and leaking the OTEL SDK. - MCP discovery failures now name the offending server (with cause) rather than surfacing an opaque registry error. - Tool-output summarization and context compaction degrade gracefully on a summarizer outage (truncate / drop-with-placeholder) instead of poisoning a successful tool call or aborting a healthy run. - /invoke: emit wrapper logs agent-level errors server-side (previously only visible to the SSE client) and swallows write failures after client disconnect instead of cascading. Logging: - Route pino to stderr so stdout stays clean for the CLI run record; add request-scoped invoke start/finish logs and per-MCP-server lifecycle logs. Cleanup: - Extract shared safeJson/errorMessage into src/util.ts (was duplicated three times); single-source the version via src/version.ts (CLI banner reported 0.1.0 while package.json was 0.2.1); drop the dead TextPart/Content request schemas; name the summarizer input-char-limit constant. Adds tests for the summarization and compaction fallback paths.
Persist the task instructions + plan under docs/specs, update the README observability note (pino now logs to stderr), and sync the biome.json schema URL to the installed 2.5.3.
Contributor
QualOps Code Quality AnalysisStatus: Summary
🟠 High Issues (3)
🟡 Medium Issues (5)
...and 2 more medium issues 📊 Full ReportPowered by QualOps |
Tool approval (AI SDK v7 native toolApproval): - New safety.approval config: mode none | all | selected, with glob tool-name patterns for 'selected'. todowrite is always exempt. - The loop passes a toolApproval policy to streamText; gated calls surface a tool-approval-request instead of executing. New tool_approval_requested event carries toolCallId, approvalId, name, args (+ HMAC signature when TOOL_APPROVAL_SECRET is set), and the run pauses so the client can resolve it by appending a tool-approval-response to the stateless /invoke history. - Maps SDK parts onto the spec-003 envelope: approval_required while pending, denied (user_denied) when a reviewer declines. Readiness: - /ready gains an opt-in active provider probe (?deep=1 or READINESS_DEEP_PROBE=1, timeout via READINESS_PROBE_TIMEOUT_MS) that issues one minimal generation to catch a wrong key, unreachable/invalid baseUrl, or unknown model — things the env-var presence check cannot. Default stays the cheap presence check. Also: bump version to 0.3.0; silence pino in tests via vitest.config.ts. Adds unit tests for the approval matcher and loop-level pause/none-mode tests.
Add a tool-approval section (config, stateless resolve flow, TOOL_APPROVAL_SECRET) and the tool_approval_requested event + denied_reason field to the README; document the /ready deep probe; add a consolidated environment-variables table. Add the safety.approval block to example.config.yaml and the 013 spec pair.
sebastianwessel
changed the title
Reorganize into Features -> Quick start -> Configuration (model, prompt templating, tools, structured output, safety) -> HTTP API -> Deployment -> Observability -> Environment variables, with nested sections where it helps. Document previously-undocumented user features (env-var expansion in config, prompt templating). Drop internal/implementation detail (library internals, toolChoice, internal function names, design asides) and contributor-only sections so the doc reads for end users.
…ide.md Move the full end-user documentation into docs/user-guide.md and reduce the root README to a short description of what the solution is, a quick start, and a table of contents linking into the guide's sections. All cross-links verified.
- CHANGELOG: record this cycle's work under [Unreleased] (tool approval, readiness probe, dependency upgrades to AI SDK v7 et al., logging/error-handling hardening, docs) — the CI changelog gate requires it, and the release workflow stamps it into a version on release. - Revert the manual package.json bump to 0.2.1: version bumping is owned by the create-release-pr workflow (pnpm version), which a pre-bump would break. The runtime still single-sources the number from package.json. - Dockerfile: bump corepack pnpm 10.30.2 -> 11.1.2 to match packageManager and the pnpm-11 lockfile.
The 55 MB tokenizer fed only approximate 'too big?' threshold gates and was already ~25% off for Anthropic models. A chars/4 estimate (the AI SDK docs' own estimator) serves the same purpose without the dependency and without re-tokenizing the whole history synchronously before every step. Numeric values in compaction_start/compaction_finished events shift accordingly; they are informational only.
chooseSplitIndex required the recent window to start on a user message, but an agentic history after the first user turn contains only assistant/tool messages — so compaction silently no-opped in its primary use case and the context grew until the provider rejected it. The split now only avoids starting the window on a tool message (which would orphan tool results from their assistant tool-call). The summary rides as a user message instead of a system message: providers such as Anthropic reject a conversation whose first non-system message is assistant-role, which can now happen. Compaction also rethrows on client abort instead of misreporting it as a summarizer outage.
raw.slice(-0) returns the entire string, so a zero tail turned the truncated excerpt into a verbatim copy of the oversized output.
Log lines now carry trace_id/span_id/trace_flags of the active span (pino mixin), ISO timestamps, level labels, and the same service.name/service.version as the trace resource (shared via resource.ts), so any log can be joined with its trace. A whitelist err serializer replaces pino's default, which would leak APICallError.requestBodyValues (full prompts) into logs. telemetryOptions() centralizes AI SDK telemetry: span content stays recorded by default (eval tooling reads tool details from traces, spec 004) with an OTEL_RECORD_CONTENT=0 opt-out. OTEL_ENABLED=0/false no longer counts as enabling tracing, and parseTraceparent moves here so serve mode can reuse it.
One streamText call now drives the whole run: stopWhen: stepCountIs(maxSteps)
replaces the manual for-loop, prepareStep handles per-step compaction and the
final-step toolChoice: 'none', totalUsage replaces manual usage summing, and
output: Output.object() replaces the separate generateObject pass (whose
tokens were also missing from the reported usage).
Approval pause now emits a run_paused event carrying the run's response
messages — without them a stateless client could not reconstruct the
assistant message holding the approval request, so every documented resume
failed. Covered by new approve/deny resume tests through the real SDK.
Provider errors are sanitized before they reach SSE clients: no more raw
APICallError (request body incl. system prompt, response body/headers);
the 429 path emits only rate-limit headers. stdio MCP children now receive
only the configured env plus the transport's safe defaults instead of the
full process environment with all provider API keys — use ${VAR} in the
config to pass a variable through explicitly. toModelOutput is now a
first-class tool() option shared via toolResultToModelOutput, dropping the
duplicated logic and 'as unknown as Tool' casts. Abort detection checks the
signal instead of matching 'abort' in error messages, which swallowed real
failures.
writeRunRecord resolves only after the record is flushed, so process.exit can no longer truncate the single machine-readable line on a backpressured pipe. A run paused for tool approval now reports 'run halted waiting for tool approval (CLI is non-interactive)' per spec 004 instead of an empty, error-less failure record. parseTraceparent import follows its move to observability/tracing.
/invoke opens a span parented on the incoming traceparent, so serve-mode logs (via the pino mixin) and AI SDK model spans share one trace; the request id rides on the span and an x-request-id header. Internal failures now send a generic message to the client with details kept in server logs, without the previous double logging. An already-aborted client signal aborts immediately (the listener alone never fires in that case). Shutdown stops accepting connections, drains in-flight requests, and force-closes leftovers after SHUTDOWN_TIMEOUT_MS instead of exiting mid-stream; re-entrant signals are ignored and shutdown failures exit non-zero. READINESS_PROBE_TIMEOUT_MS is validated (a garbage value made every deep probe throw).
Assigning undefined coerces to the string 'undefined', polluting the environment for later tests.
z.url() replaces deprecated z.string().url(), .optional() before .default()
is dropped (redundant in zod 4), and .prefault({}) replaces the triple-
duplicated literal default objects — parsed values are identical.
Documents the deep-review pass: the run_paused resume contract, OTel log/trace correlation fields, OTEL_RECORD_CONTENT and SHUTDOWN_TIMEOUT_MS, the stdio MCP env isolation (supersedes spec 008's process.env merge), and the compaction contract that previously had no owning spec.
commander 15 requires node >=22.12; declaring only >=22 allowed installs on 22.0-22.11 that would fail at runtime. The Docker image (node:22 slim) already tracks a newer patch line.
Audited every src/tests import against package.json. shell-quote (+types) was orphaned by the spec-008 bash-tool removal, msw has no importer, and the direct vite entry duplicated the vite that vitest itself ships — all removed, including the msw references in the Dockerfile rebuild line and pnpm-workspace allowBuilds. @vitest/coverage-v8 was installed but unusable without a script; test:coverage now wires it. Runtime dependencies now map 1:1 to the packages src/ actually imports; @ai-sdk/provider stays pinned at the exact 4.0.3 to match ai@7's own pin (single deduped instance for isInstance checks). No optionalDependencies on purpose: all imports are static and the provider SDKs are selected by config at runtime.
…rors The /invoke and CLI request boundary now validates with the AI SDK's own modelMessageSchema (typed z.ZodType<ModelMessage>), so the boundary accepts exactly what streamText accepts — text, multimodal parts, tool calls/results, approval responses — the 'as ModelMessage[]' casts disappear, and malformed messages fail with a clear 400/exit-2 instead of mid-stream. Todo types are now inferred from the tool's input schema (single source of truth instead of a parallel interface), and createTodoWriteTool returns Tool<TodoWriteInput, ToolResult> so callers keep the typed execute. Zod error reporting moves off the deprecated .format(): API responses carry z.prettifyError + z.treeifyError, CLI stderr and ConfigError messages are human-readable field-by-field reports, and every config constraint now has a descriptive message naming the offending path and the accepted values.
The fake tools structurally satisfy ToolSet once given real jsonSchema inputs, so the loop tests need no casts at all. MCP client mocks go through one explicit asMcpClient() seam instead of scattered 'as never', and the todowrite tests use the tool's own typed execute/toModelOutput instead of re-declaring their shapes.
… deep review
Second adversarial review pass over the PR. serve now refuses an approval
config without TOOL_APPROVAL_SECRET (stateless /invoke history makes unsigned
approvals forgeable). /ready deep probe gets single-flight + short TTL cache,
no retries, and stops returning provider error text to unauthenticated
callers. /invoke gains a body-size limit via hono's bodyLimit. MCP discovery
gets a timeout so a hung server cannot block startup forever. model.baseUrl
is now honored for anthropic/openai/google (was silently ignored). Nested
config objects are strict so typo'd safety limits fail instead of silently
using defaults; the system prompt template is validated at load; $${VAR}
escapes env expansion. Shutdown puts a deadline on MCP/OTEL cleanup; the CLI
also absorbs console.info/debug stdout writes. New endpoint test suite covers
/health, /ready (incl. deep probe non-leak), and /invoke validation, limits,
and SSE streaming.
…ning pass
Documents the enforced TOOL_APPROVAL_SECRET requirement, baseUrl support for
hosted providers, the new body/discovery/probe limits and their env vars, the
$${VAR} escape, and records the deliberate non-fixes from the second review
pass in spec 014.
Verified against the installed ai@7.0.19: the runtime modelMessageSchema models tool-approval-request parts without the signature field, so parsing a re-POSTed history through zod silently dropped the HMAC and every signed approval resume failed with 'missing signature'. parseInvokeRequest now validates and passes the ORIGINAL messages through — the same validate-then-pass-originals approach the SDK's standardizePrompt uses. Also from the SDK verification pass: swap deprecated stream.totalUsage for stream.usage, wire MCPClient onUncaughtError into the logger so transport failures outside a tool call are not silent, raise the readiness probe to 16 output tokens (reasoning models enforce a minimum), and drop an unnecessary cast on the tool-error part.
Author
|
Re: the failing QualOps Code Quality check — all 8 findings target the
Everything else is green: test (108), changelog, docker-build, CodeQL, review. |
The chars/4 estimator (tokens.ts) is gone. Compaction now triggers on the
provider-reported input-token usage of the previous step, delivered by the
SDK's prepareStep steps — triggerTokens finally compares real tokens instead
of an estimate that under-counted code by ~20%. Tool-output summarization is
gated on exact size via safety.toolOutput.triggerChars (default 16000 chars,
the same effective threshold as the old 4000 pseudo-tokens). Compaction
events report exact { messages, chars } sizes.
Consequences by design: compaction never fires before the first step and
stays off for providers that report no usage. New end-to-end test drives the
usage-gated compaction through the real SDK loop. Spec 015 records the
decision and the deferred follow-ups (tool-output modes, summarizer model,
hysteresis).
valdis
approved these changes
Jul 13, 2026
Review feedback on PR #17: buildMcpRegistry's per-server body was four levels deep with an inline async closure — extracted into discoverServerTools and mergeServerTools so the loop reads as one line per concern. Removed comments that only repeated the adjacent code (SUMMARY_INPUT_CHAR_LIMIT, SizeSnapshot, snapshot(), the prepareStep usage note); constraint/why comments stay.
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
2 participants
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.
Summary
Three things, verified end-to-end (
typecheck,buildon tsc 7, 85 tests,lint):[Unreleased]in the changelog.)Specs:
docs/specs/012_deps_refactor_*anddocs/specs/013_tool_approval_and_readiness_*.Dependency upgrades & migration
ai7.0.19,@ai-sdk/*4.x/3.x/2.x):isStepCount,stream,telemetry; telemetry moved to@ai-sdk/otel(registered instartTracing);allowSystemInMessagesfor the loop's trusted system messages; stablecreateMCPClient.resourceFromAttributes+ATTR_SERVICE_*), TypeScript 7.0.2 (native compiler), Biome 2, Vitest 4.1, Vite 8, commander 15, pino 10, hono 4.12, @types/node 26, msw 2.15.minimumReleaseAgegate frompnpm-workspace.yaml(per request) and a straypackage-lock.json.Reliability / cleanup
--versionmismatch (was0.1.0) — single-sourced viasrc/version.ts./invokelogs request start/finish + agent errors server-side and no longer cascades on client disconnect; pino routed to stderr (keeps the CLIrunrecord clean).safeJson/errorMessageutil; removed dead request schemas.New: human-in-the-loop tool approval
safety.approvalconfig —mode: none | all | selectedwith glob tool-name patterns;todowriteexempt.tool_approval_requestedevent (+approval_requiredenvelope) and pause the run instead of executing; the client resolves by appending atool-approval-responseto the stateless/invokehistory and re-POSTing. Denials map todenied/user_denied.TOOL_APPROVAL_SECRETHMAC-signs approvals (the history is client-controlled).New: active readiness probe
/ready?deep=1(orREADINESS_DEEP_PROBE=1) makes one minimal provider call to catch a wrong key, unreachable/invalidbaseUrl, or unknown model — things the env-var presence check can't. Default/readystays cheap for k8s polling.Docs
README (approval flow, event taxonomy,
/readydeep probe, consolidated env-var table, stderr logging),example.config.yaml(safety.approval), and two spec pairs updated/added.Deliberately unchanged
Spec-mandated
denied/approval_requiredstatuses now have a producer;/readypresence check stays permissive for keylessopenai-compatible/ollama. Rationale in the 012/013 plan specs.🤖 Generated with Claude Code
Follow-up: deep review pass (spec 014)
A multi-agent adversarially-verified review (43 raw findings → 21 confirmed) drove a hardening + simplification pass. Verified end-to-end:
typecheck,build, 91 tests,lint. Spec:docs/specs/014_deep_review_hardening_*.Complexity reduction (AI SDK built-ins replace custom code)
streamTextcall (stopWhen: stepCountIs,prepareStepfor per-step compaction + final-steptoolChoice: 'none',totalUsage).output: Output.object()), deleting the separategenerateObjectpass; its tokens now count toward reported usage.toModelOutputbecame a first-classtool()option shared by MCP + todowrite (no moreas unknown as Tool).gpt-tokenizer(chars/4 estimate for threshold gates); modernized zod v4 idioms (z.url(),.prefault({})).Bug fixes
run_pausedSSE event hands the client the messages to append; approve/deny resume covered by integration tests.tailChars: 0leaked the full raw tool output; loose'abort'substring matching swallowed real errors; CLI stdout record could truncate on exit; shutdown killed in-flight SSE streams (now drains withSHUTDOWN_TIMEOUT_MS); garbageREADINESS_PROBE_TIMEOUT_MS500'd every deep probe;OTEL_ENABLED=0enabled tracing.PII / secret hardening
APICallErrorserializes the full request body incl. system prompt + response body/headers); 429 details are whitelisted rate-limit headers only.errserialization is whitelisted for the same reason.TOOL_APPROVAL_SECRET) — only the configuredenv+ safe defaults;${VAR}passes specific vars through (supersedes spec 008's process.env merge).OTEL_RECORD_CONTENT=0opt-out.OTel log/trace correlation
trace_id/span_id/trace_flagsof the active span +service.name/service.versionshared with the trace resource./invokeopens a per-request span parented on an incomingtraceparentand returnsx-request-id.Follow-up 2: second adversarial pass (production-readiness)
Two more agents (installed-SDK API verification against
ai@7.0.19dist types/docs, and an adversarial bug/stability/PII review) over the already-hardened branch. Verified end-to-end:typecheck,build, 108 tests,lint(0 warnings). Recorded indocs/specs/014_deep_review_hardening_plan.md("Second review pass").Security / correctness
/invokehistory is client-controlled —servenow refuses to start when approval is enabled withoutTOOL_APPROVAL_SECRET.modelMessageSchemastrips thesignaturefield on parse, so the re-POSTed approval always failed "missing signature". The boundary now validates and passes the original messages through (same approach as the SDK'sstandardizePrompt); regression test added./ready?deep=1hardened: single-flight + 10 s cache (READINESS_PROBE_CACHE_MS), no retries, provider error text logged but never returned to unauthenticated callers./invokebody cap via hono's built-inbodyLimit(MAX_REQUEST_BODY_BYTES, 10 MiB default, 413).z.strictObject): typo'd keys (e.g.compation:) fail at startup instead of silently using default safety limits.Stability / completeness
MCP_DISCOVERY_TIMEOUT_MS(30 s) — a hung server no longer blocks startup//healthforever; transport errors outside a tool call now hit the logger (onUncaughtError).model.baseUrlwas silently ignored for anthropic/openai/google — now honored (gateway/proxy routing).$${VAR}escapes env expansion; shutdown deadline also covers MCP/OTEL cleanup; CLI absorbsconsole.info/debug; probe asks for 16 tokens (reasoning-model minimums); deprecatedstream.totalUsage→stream.usage.Tests
New endpoint suite (
tests/server.test.ts:/health,/readypresence + deep-probe non-leak,/invokeinvalid JSON / schema error / 413 / SSE happy path via amodeloverride onBuildServerOptions),tests/request.test.ts(signature preservation), andexpandEnvVars+ strict-schema coverage. 91 → 108 tests.Verified keep-custom (no SDK equivalent, checked against dist types)
Compaction summarizer (
prepareStepis the documented seam;pruneMessagesonly strips), tool-output summarization, the approval glob matcher, and the MCP JSON-schema normalization.ToolLoopAgentmigration rejected: it doesn't acceptexperimental_toolApprovalSecretat 7.0.19, so it would lose signed approvals.