Skip to content

feat(spec): IHttpServer gains an afterResponse response-observing hook — HTTP metrics become transport-agnostic - #9895

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-9835-http-server-response-hook
Aug 19, 2026
Merged

feat(spec): IHttpServer gains an afterResponse response-observing hook — HTTP metrics become transport-agnostic#9895
os-elon merged 2 commits into
mainfrom
claude/issue-9835-http-server-response-hook

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#9835

Clause-② notice: this PR widens the public IHttpServer contract surface — one new optional member (afterResponse), two new exported types (HttpResponseObservation, HttpResponseObserver) and one new exported constant (UNMATCHED_ROUTE_PATTERN) on @objectstack/spec/contracts. Additive only; nothing is removed or narrowed. Draft by dispatch contract — the PM reviews and lands it.

Ruling implemented

Maintainer ruling of 2026-08-18 on #9650, quoted in the card: Option B — the IHttpServer response-observing contract hook — is the principled successor to the transport-owned Hono seam, filed as its own spec card. The documented expectation the ruling demands is stated verbatim in the contract JSDoc, the docs, and below: a transport that does not implement the seam reports no HTTP metrics — stated plainly rather than letting it read as coverage.

The contract shape chosen, and why

afterResponse?(observer: HttpResponseObserver): void — an observer registration, invoked by the transport exactly once per answered request with { method, routePattern, status, elapsedMs } after the response exists. Chosen over a single-handler set…-style member because observation is naturally many-consumer (metrics + access log), so registration APPENDS in registration order — the contract states explicitly how this differs from setFallbackHandler's replace semantics. Observers are isolated (a throwing observer affects neither the response nor siblings) and have no channel back into the response. The member name follows the card's own "afterResponse-style registration" sketch; the types follow the getMountedRoutes idiom of shaping payloads inline on the contract file.

The three ruling constraints, each met and pinned:

  1. Route label is the PATTERN, never the concrete path. Stated as a hard requirement in the contract JSDoc (on both the member and the HttpResponseObservation.routePattern field) so no adapter re-decides cardinality. The reserved label for unrouted requests is contract-owned too: UNMATCHED_ROUTE_PATTERN = 'unmatched', exported so every transport reports one spelling. Pinned in the spec contract tests, the cross-adapter conformance suite (/api/v1/data/:id, never rec_42) and the runtime integration file.
  2. Optionality is visible and runtime-real. Feature-detected with typeof-equals-function, like getRawApp. Per runtime 导出的 HttpServer 包装器静默丢弃 IHttpServer 的全部可选成员(getPort / getRawApp / setFallbackHandler) #5122's optional-member-erasure record, the contract states that a wrapper must forward the member conditionally, and the spec test file pins the erasure failure mode itself (an erasing wrapper makes an instrumented transport read "not instrumented"). The dispatcher's own Proxy forwards via Reflect.get, so detection survives it — pinned in the integration file against the real registered service.
  3. The runtime emits against the contract. See de-dup ownership below.

De-dup ownership (http_requests_total#9833 read before deciding)

The ownership rule, recorded in the contract JSDoc: the transport owns the counter; exactly one counter-emitting observer per server. Three mechanisms make it structural rather than aspirational:

Net effect measured on the booted shipped-order composition: the §4 pin that documented the #9833 duplicate flips from toBe(2) to toBe(1) (dispatcher route once, auth wildcard once), and a NO-DOUBLE-COUNT pin holds all three potential emitters at exactly 1 per surface with an extra observer registered. A host that wires metrics only on the dispatcher — the wiring the production-readiness docs demonstrate — previously counted only dispatcher routes; it now counts every inbound surface through the seam (pinned). The #9833 double count is thereby retired in the shipped composition; #9833 remains open for the PM to disposition (this PR deliberately carries no closing reference to it).

Reverse verification of the suppression: ablating emitHttpRequestsTotal: !transportCountsRequests to a hard true turns the retirement pin red in the predicted direction (expected 2, wanted 1); restoring the committed fix turns it green. Both legs ran against source-resolved code (the integration test imports ./dispatcher-plugin.js relatively), so no rebuild was involved in that measurement.

Documented no-metrics expectation

Stated in three places, per the ruling: the contract JSDoc (own section: zero means "not instrumented", never "no traffic"; a consumer must ask via feature detection and must never infer traffic absence from a counter it never confirmed was armed), docs/OBSERVABILITY.md (TL;DR callout), and content/docs/deployment/production-readiness.mdx. armHttpRequestCounter surfaces the same fact programmatically as the unsupported result.

Conformance-adapter decision: implement (the cheap, preferred branch)

NodeHttpServer in packages/qa/http-conformance implements the seam (~30 lines: a finish-event delivery armed once the router's verdict is known, so route, fallback, 404 and 405 answers are all observed with the status as sent). This retires the "zero means not instrumented" hole for that transport — the card's preferred outcome — and, exactly like the #6143setFallbackHandler precedent the adapter itself records, it is what lets the semantics be locked CROSS-adapter: a new response-observation.conformance.test.ts runs 7 cases against both adapters over real sockets (pattern-never-path, unmatched label, status-as-sent, append registration, observer isolation, exactly-once, runtime-real detection).

Behavior notes

  • Unrouted-request label corrected on Hono: measured during implementation, routePath(c) after next() reports the seam's own use('*') registration (the string slash-asterisk) for a request no route matched — indistinguishable from a real static catch-all — so the previous seam's unrouted label was that spelling, not the intended unmatched. The notFound sink (the authority on "nothing matched") now marks such requests and the seam labels them with the reserved constant; a method-mismatch 405 and fallback-served requests are unmatched too (pinned adapter-locally). Called out in the changeset.
  • The afterResponse delivery middleware is mounted unconditionally in HonoServerPlugin.init() (so consumers may register observers at any later moment — pinned: an observer registered after bootstrap sees every surface); disarmed cost is one array-length check per request. The counter still only exists when a registry resolves — "not a disabled counter; no counter" is preserved.

Remaining references / consumers (whole-tree grep, each deliberate)

  • afterResponse implementors: HonoHttpServer (delivery via the observation seam + notFound marker) and NodeHttpServer (finish-event delivery). No other IHttpServer implementation exists in this repo; wrappers were retired by refactor(runtime)!: 退役 runtime 导出的 HttpServer 委派包装器 (#5122) #6141 (runtime 导出的 HttpServer 包装器静默丢弃 IHttpServer 的全部可选成员(getPort / getRawApp / setFallbackHandler) #5122).
  • Counter arming sites: exactly two, both through armHttpRequestCounterHonoServerPlugin.installHttpMetricsSeam and the dispatcher's Phase-2 offer. RUNTIME_METRICS.httpRequestsTotal string emitters: the helper and the gated instrumentRouteHandler line; no others.
  • @objectstack/core re-exports the new types/constant beside its existing contracts block (the QA adapter imports through core, as it always has).
  • Spec generated artifacts regenerated via the repo tooling (check:generated --fix after build): api-surface/contracts.json and export-origins/contracts.json, each showing exactly the three new exports. check:generated reports all 13 artifacts up to date at head.

Verification (all at head 7660d5c, after merging origin/main and rebuilding)

  • Suites, all green at head: spec 413 files / 11014 tests (full run; the one initially-stale export-origins pin regenerated and re-run green), runtime 175/2624 (full), plugin-hono-server 19/215 (full, incl. the new adapter-local seam file), http-conformance 5/86 (full, incl. the new cross-adapter suite), observability 6/79 (incl. new armHttpRequestCounter unit pins), core 35/857 (full).
  • Typecheck green for spec, plugin-hono-server, runtime, http-conformance (observability declares no typecheck script — ledgered).
  • node scripts/pm/dispatch-gates.mjs re-derived on the actual committed diff (no hand-fed path list); every named gate run and green at head, including all PM dispatch-time readings (check:changeset-gate-self-tests, check:cross-package-test-inputs + its node twin, check:doc-formula-expressions, spec check:empty-state/check:liveness/check:strictness-ledger/check:variant-docs, check:merge-driver, check:spec-parsed-alias, check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-dev-prereqs, check-affected-docs) plus the derivation's additions (check:doc-anchors, check:docs-audit-scope, check:docs-redirects, check:kernel-hook-pairs, check:objectui-changeset, check:published-readme-links, check:role-word, check:route-envelope, check-changeset-no-major, check-empty-changeset, check:nul-bytes) and the convention-triggered set on new test files (check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:where-matcher, and check:type-check-debt --re-measure on the fully built workspace — every ledger entry exactly at its measurement).
  • Reverse verification: the ablation described under de-dup ownership (red at 2, restored green at 1, source-resolved — no rebuild in the loop).

Changeset / ADR-0087

One changeset: @objectstack/specminor (public contract surface widened, additive — never major, per the standing gate), patch for @objectstack/core, @objectstack/observability, @objectstack/plugin-hono-server, @objectstack/runtime, @objectstack/http-conformance. ADR-0087: not required — a contract MEMBER addition on an existing interface authors nothing and retires nothing; no authorable metadata key changes shape, so there is no conversion or migration to register, and the changeset declares no breaking change (the marker gate applies only to declared-breaking changesets; check:adr-0087-registration is green).

Out of scope

No new issues filed: the one adjacent defect encountered (#9833) was already on file as the sibling card, and the pre-existing mislabel of unrouted requests fell inside this card's own constraint 2, fixed here and pinned.


Generated by Claude Code

… so HTTP metrics are transport-agnostic (#9835)
- contract: optional afterResponse member + HttpResponseObservation/
HttpResponseObserver types + reserved UNMATCHED_ROUTE_PATTERN label;
pattern-never-path stated as a hard requirement; runtime-real feature
detection; transport-side counter ownership documented (de-dup, #9833);
no-seam => no-HTTP-metrics expectation stated plainly per the 2026-08-18
ruling on #9650
- hono adapter: implements the seam (the ruled raw-app middleware becomes
its delivery path — reach unchanged); unrouted requests labelled with the
reserved unmatched pattern via the notFound marker
- observability: armHttpRequestCounter arms http_requests_total through the
seam at most once per server (first caller wins)
- runtime: dispatcher offers its registry to the seam and suppresses its
per-route counter copy when the transport implements the seam (retires
the #9833 double count; histogram/request-id/error signals unchanged)
- qa/http-conformance: NodeHttpServer implements the seam; cross-adapter
conformance suite locks the semantics
- docs: OBSERVABILITY.md + production-readiness.mdx state the transport
seam and the no-metrics expectation
- spec artifacts regenerated (api-surface, export-origins); changeset:
spec minor, others patch
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 6 package(s): @objectstack/core, @objectstack/observability, @objectstack/plugin-hono-server, @objectstack/http-conformance, @objectstack/runtime, @objectstack/spec, touching 29 documentable anchor(s).

30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb.

2 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 3 changed file(s) yielded no anchor (packages/core/src/index.ts, packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 126 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 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from e12c7e096a23237574987471c6b73afbcef758d5 — the merge of head 7660d5cc52381f4defe67d26d4b25eeb25e4fe0e into base 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb, 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 e12c7e096a23237574987471c6b73afbcef758d5 && git checkout e12c7e096a23237574987471c6b73afbcef758d5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb 7660d5cc52381f4defe67d26d4b25eeb25e4fe0e && git checkout -B drift-repro 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb && git merge --no-ff 7660d5cc52381f4defe67d26d4b25eeb25e4fe0e
node scripts/docs-audit/affected-docs.mjs --json 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb

⚠️ 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 4b99386acfa768b6b2c3e9e8d379a6aa3f75cbfb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 19, 2026
@os-elon
os-elon marked this pull request as ready for review August 19, 2026 06:28
@os-elon
os-elon enabled auto-merge August 19, 2026 06:28
@os-elon
os-elon added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 7ff3975Aug 19, 2026
27 checks passed
@os-elon
os-elon deleted the claude/issue-9835-http-server-response-hook branch August 19, 2026 07:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: give IHttpServer a response-observing hook so HTTP metrics are transport-agnostic instead of Hono-only

2 participants

@os-elon@claude