Skip to content

fix(runtime): the dispatcher's observability Proxy wraps PUT and PATCH, so those routes carry X-Request-Id and report their 5xx - #24180

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24025-put-patch-observability
Oct 17, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24025-put-patch-observability

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24025

Clause-②: no

What was wrong

createDispatcherPlugin wraps its IHttpServer in a Proxy that hands each route registration to instrumentRouteHandler. The trap named get, post and delete only, so every route the dispatcher mounts through put or patch was registered bare. Those routes answered with no X-Request-Id and left req.requestId unset. A 5xx on them never reached observability.errorReporter, and their [5xx] log line carried no method, route or request id.

The bare mounts on origin/main were:

  • PUT and PATCH /keys.
  • The PUT and PATCH mounts of the three /notifications paths.
  • PATCH /packages/:id, /enable and /disable.
  • PUT /automation/:name.
  • The PUT and PATCH /ai/* wildcards.
  • Service-emitted PATCH routes (ai:routes → mountRouteOnServer).

This restores a contract that is already published, in two places:

  • content/docs/protocol/kernel/error-handling.mdx: the id "is attached by the observability instrumentation wrapper", and a deployment that composes it "echoes a valid inbound X-Request-Id and otherwise mints" a req_-prefixed hex id.
  • content/docs/deployment/production-readiness.mdx: "every route the dispatcher mounts now: … Echoes X-Request-Id (honored from caller, or freshly minted). Reports 5xx exceptions to Sentry (or your reporter) with the request id attached."

The change

  • Producer: packages/runtime/src/dispatcher-plugin.ts. The Proxy now wraps all five route verbs that IHttpServer declares (get, post, put, delete, patch). The list is typed with satisfies ReadonlyArray of keyof IHttpServer. use, getRawApp and setFallbackHandler still pass through unchanged.
  • instrumentRouteHandler is unchanged. I read it before deciding: it treats the method only as a metric and reporter label, and nothing in it branches on the verb.
  • No second metrics emitter, measured. On a transport that implements afterResponse (the shipped Hono adapter), the wrapper's counter and histogram are already gated off for every verb (emitHttpRequestsTotal / emitHttpRequestDurationMs = !transportCountsRequests). So PUT and PATCH stay at one sample per request; the real-socket row below pins that. On a transport without the seam, the wrapper now counts and times PUT and PATCH under their own method label, as it always did for GET, POST and DELETE.
  • Comment-only corrections in the same file. The fix made three comments false:
    • The /mcp/skill method-set note gave the three-verb Proxy as a reason not to mount PUT/PATCH. No route is added there.
    • One sentence in the /keys block and one in the /notifications block said their PUT/PATCH mounts are not wrapped.
    • No mount helper or route body changed.
  • One past-tense retitle. The http_requests_total never sees auth routes or the REST data API — the two highest-traffic inbound surfaces are outside the only HTTP counter the docs tell operators to monitor #9650 §2 seam-A measurement in http-metrics-inbound-coverage.hono.integration.test.ts builds its own inline copy of the old three-verb Proxy. Its assertion is unchanged. Its title now reads in the past tense, with a note pointing at the new pin, so it no longer describes today's code.

Pins

  • New packages/runtime/src/dispatcher-observability-every-verb.test.ts. It runs the real plugin over a fake transport, with tables over all five verbs. GET, POST and DELETE are the in-file positive control.
    • /api/v1/keys: a valid inbound id is echoed and lands on req.requestId, and one is minted when none is sent.
    • /api/v1/ai/*: a route-table handler that throws produces a 500. errorReporter receives that same error with requestId, method and route. The [5xx] line carries method, path and request id.
    • One service-emitted PATCH route through ai:routes, which reports its 5xx too.
    • On a transport without afterResponse: one counter sample and one duration sample per request, under the verb's own label.
  • http-metrics-inbound-coverage.hono.integration.test.ts §5. A new case beside the existing GET X-Request-Id case. Every /api/v1/keys verb over a real Hono socket echoes and mints the header, and the transport still counts and times each request exactly once.

Ablation (run once, then restored)

I committed the fix first. Then scripts/ablation-replace.mjs put the old arm back (prop === 'get' || prop === 'post' || prop === 'delete'), and I ran both pin files. Both import ./dispatcher-plugin.js relatively, so they read source and need no rebuild.

  • Fixed tree: Test Files 2 passed (2), Tests 48 passed (48).
  • Old arm: Test Files 2 failed (2), Tests 10 failed | 38 passed (48).
    • The 10 red rows are exactly the PUT and PATCH rows: 9 unit rows and the real-socket row (PUT echoes the inbound id: expected null to be 'probe-put-24025').
    • All 15 GET, POST and DELETE rows stayed green.
  • Restore proven by the tool: blob after restore 0136bea52b19 equals the HEAD blob, and git diff HEAD is empty.

Verification (head 2cf6682f8, after merging origin/main cb8756b00)

  • pnpm --filter @objectstack/runtime exec vitest run --project local --maxWorkers=2 (the package's test script, with workers capped): Test Files 408 passed (408), Tests 6276 passed | 19 skipped (6295), lock VERDICT command-exit 0.
  • pnpm --filter @objectstack/runtime run typecheck: exit 0. check:test-typecheck held 27 files / 190 errors / 68 pinned signatures, and the new file added no debt.
  • pnpm exec eslint --no-inline-config over the three changed .ts files: exit 0, no output.
  • Derived gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack named 62 commands. I ran each with its exit code captured before any pipe. --ran reconciles 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN.
    • Three of them first stopped on a local prerequisite, not a finding, and are green on rerun:
      • check-plugin-teardown-shape --self-test needed its pinned fixture commit fetched on this shallow clone.
      • check:dual-build-cjs-loads needed the workspace built (111 require entry points across 69 packages load).
      • check:type-check-debt never got a turn on the shared lock and was rerun wholly under it (nothing above its ledger).
    • The 5 roster gates whose rosters sit under a touched directory are also green: check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity and check:route-ledger-census.

Acceptance notes

  • Observation, not filed (owner: none). RouteDefinition.method in dispatcher-plugin.ts is typed GET, POST, PATCH or DELETE, and mountRouteOnServer has no put arm. A service that emitted a PUT route on ai:routes would therefore be skipped; only the boot line's mount count would show it. No producer of a PUT route on that hook is named, so nothing is reachable. This is unchanged here and stays outside this card.
  • The returned-envelope path is unchanged. A route that answers a returned 5xx envelope instead of throwing still reaches the [5xx] log line but not errorReporter, on every verb alike. That is consistent with the published "5xx exceptions" / "Unhandled exceptions specifically still reach the errorReporter" wording, so it is not a finding.
  • File surface versus the claim. Besides the Proxy arm, this diff touches comment lines in the /mcp/skill, /keys and /notifications blocks of the same file (H3, extended to the two later comments that said the same thing). It also retitles the one §2 measurement case. Mount helpers, route bodies, plugin-hono-server and packages/rest are untouched.

Generated by Claude Code

…H registrations too

The Proxy over the dispatcher's IHttpServer instrumented only get, post and
delete, so every PUT or PATCH mount answered with no X-Request-Id, left
req.requestId unset and never reached errorReporter on a 5xx. It now wraps
all five route verbs IHttpServer declares. Three comments that described the
three-verb arm are corrected.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
…els on every dispatcher route verb

A table over all five verbs on the real plugin (GET/POST/DELETE are the
in-file positive control), plus a PUT/PATCH row on the real Hono socket that
also pins the transport still counting each request once. The #9650 seam-A
measurement keeps its inline copy of the old Proxy, retitled to the past
tense. Changeset for @objectstack/runtime (patch).

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 8 documentable anchor(s).

11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via packages.uninstall (sdk, the route ledger binds it to DELETE /packages/:id))
  • content/docs/api/environment-routing.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/api/metadata-api.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/data-modeling/formulas.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/deployment/production-readiness.mdx (via createDispatcherPlugin (symbol, a top-level function))
  • content/docs/deployment/publish-and-preview.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/kernel/contracts/metadata-service.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/permissions/permission-sets.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/permissions/system-context.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/plugins/packages.mdx (via createDispatcherPlugin (symbol, a top-level function))
  • content/docs/protocol/kernel/error-handling.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/releases/v17/17-0.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/releases/v17/17-4.mdx (via /packages/:id (route, a path literal in a comment in createDispatcherPlugin))
  • content/docs/releases/v17/17-5.mdx (via packages.get (sdk, the route ledger binds it to GET /packages/:id))
  • content/docs/releases/v17/17-6.mdx (via automation.update (sdk, the route ledger binds it to PUT /automation/:name))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /automation/:name (route, 40 pages)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from db63d53c116d6c80466a0d6563a39516ee192e5b — the merge of head 2cf6682f87db84646f31e845694f48a3e7b4a714 into base cb8756b005a2d77c7c07574dffe4f99b8e91f8d1, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin db63d53c116d6c80466a0d6563a39516ee192e5b && git checkout db63d53c116d6c80466a0d6563a39516ee192e5b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 2cf6682f87db84646f31e845694f48a3e7b4a714 && git checkout -B drift-repro cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 && git merge --no-ff 2cf6682f87db84646f31e845694f48a3e7b4a714

node scripts/docs-audit/affected-docs.mjs --json cb8756b005a2d77c7c07574dffe4f99b8e91f8d1

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 — ACCEPT · PR #24180 at 2cf6682f8 (#24025)

domain:cli seat 2 · marchtian · session session_01QyoWKK9RwUNxBxmevy9Neb · 2026-10-17T15:27Z · holder of claim 6234981195. The dev's report is comment 6235627499 on #24025, and the seat reviewed it against this PR, not the report's own account.

  1. Shape. Draft → main. The first line is Fixes #24025, the only closing keyword in the body (body read in full). Clause-②: no is line-initial, and the assignee is marchtian. The 4 files are inside the claim's surface: the Proxy arm plus comments in dispatcher-plugin.ts, two test files, one changeset. check-governed-merges.mjs --pr 24180: NOT governed, 350 changed lines.

  2. Premise, measured. H1 held: the arm at dispatcher-plugin.ts:1047 named only get/post/delete (the seat read it on b0fe9fad4). The dev's ablation reverted the arm: exactly the 10 PUT/PATCH rows went red, the 15 GET/POST/DELETE controls stayed green, and the restore was proven by blob. Seat spot-check of the new pin dispatcher-observability-every-verb.test.ts, per verb including PUT and PATCH:

    • it echoes the inbound X-Request-Id into the header and req.requestId;
    • it mints ^req_[0-9a-f]+$ when no id is sent;
    • errorReporter captures the thrown error with {requestId, method, route} on the /ai/* wildcard;
    • a service-emitted PATCH route on ai:routes (mountRouteOnServer) reports its 5xx too.
  3. Check-runs on head 2cf6682f8 at this act: 28 success, 6 skipped, 5 in_progress (Test Core 2/6–6/6). The Lint and Type Check families are green. Local: 62 derived / 62 run (--ran exit 0); ESLint over the changed files exit 0.

  4. Accepted deviations:

    Changeset, read sentence by sentence against the diff:

    • "wrapped routes registered through get, post and delete only … now wraps all five verbs" — true.
    • The listed PUT/PATCH mounts — true: the card's list plus the /notifications mounts whose comments this diff corrects.
    • "on the standalone Hono server nothing changes" for metrics — true, and consistent with the emit flags the new code comment cites.
  5. Out of scope. RouteDefinition.method has no PUT and mountRouteOnServer has no put arm. It has no reach: (no named producer emits PUT on ai:routes), so it stays in the PR's Acceptance notes with carrier: none, not filed.

  6. Landing. Ordinary queue landing. Once every check on this head is green or a roster skip, the seat flips ready and arms auto-merge through the relay. Fixes #24025 closes the card on merge, and the seat strips pm:dispatched in the same act.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 17, 2026 15:33
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 17, 2026 15:33
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 17, 2026
Merged via the queue into main with commit 727091c Oct 17, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24025-put-patch-observability branch October 17, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants