test: pin contracts a framework swap can silently move - #847
Merged
juliopolycarpo merged 7 commits intoAug 13, 2026
Merged
Conversation
The shared contracts derive most of their shape rather than writing it down: eight public schemas are folded by `Type.Composite`, several runtime constant arrays are built by reading `anyOf` back at module load, and `ReadonlyArraySchema` asserts a static type over an array through `Type.Unsafe`. None of that is protected by the compiler — a schema library that keeps type-checking while emitting a different keyword changes behavior silently. Pin what those operators must keep producing: - every `Type.Composite` site, as a table stating the folded required set, property set, unknown-key policy, and per-field rejections, with each fixture annotated with the schema's exported `Static<>` type; - `anyOf`-derived constant lists, which become arrays of `undefined` if unions stop emitting `const` members; - dynamically built literal unions, `Type.Never` as an optional field, `Type.Exclude`, `Type.Omit` over a strict object, `Type.Record`, and anchored string patterns; - `ReadonlyArraySchema` across its `Type.Unsafe` boundary, including nested inside a strict object; - `Value.Equal`, which decides whether a runtime manifest changed. One case is characterized rather than endorsed: `Type.Composite` does not inherit a member's `additionalProperties: false`, so `RuntimePairingIssueSchema` accepts unknown keys even though both of its members refuse them. Pinned so a change in either direction is visible, and flagged for a separate decision. Test-only. No production code, dependency, or contract changes. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
The hooks in `app.ts` rest on orderings and scopes that no call site states. `requireAuth` publishes `user` from a `derive` its own `onBeforeHandle` reads; the rate limiter reads a `clientIp` the same way and silently stops enforcing when it is absent; `apiKeyGuard` is mounted once on the `/api` instance and must stay once; and each scoped guard has to cover its own subtree without reaching a sibling module. A framework change can move any of those while everything still compiles. A widened guard scope makes a public module answer 401, a narrowed one opens a protected module, and a re-ordered derive turns the rate limiter off with no error anywhere. Drive real requests and assert the outcome: - a protected route receives the derived user, session, and authentication method, and reports `api-key` when that header is used; - the session lookup runs once per request across three modules that each mount the guard, which is what plugin deduplication buys; - an adjacent module and the parent stay public while their sibling is protected, and every route inside the protected module is covered; - a rejecting guard's status, content type, and body all reach the client, and headers written before the handler survive; - `clientIp` is visible to hooks and handlers after the limiter's derive, and the classified bucket is the one enforced; - `apiKeyGuard` refuses an unverifiable key without draining the request body, and returns early when no key header is present. Test-only. No production code, dependency, or contract changes. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
Every failure this API can answer with is funnelled through one handler
and two wire shapes: `ApiErrorResponse` over JSON, `SSEErrorEvent` over
an event stream. The status mapping in between is what tells a caller
whether to fix its request or report an outage, and the sanitization is
the only thing standing between a framework's own error object and the
client — a rejected payload carries whatever was sent, credentials
included, and a raw exception carries file paths.
Pin the whole boundary rather than the framework's error codes:
- the five status arms, each with its exact body, JSON content type, and
`ApiErrorResponse` conformance, including the unknown-route 404 and the
generic 500 that were previously untested;
- `{ as: 'global' }` still reaching a route on a sibling instance;
- no rejected credential, no rejected response value, and no internal
exception message in the client response, and no credential in the
captured logs;
- a streaming failure keeps `text/event-stream`, its `data: ` framing,
and an `SSEErrorEvent` carrying the terminal `done: true`.
The upload suite now mounts `errorHandler` the way `app.ts` does. It was
asserting against Elysia's raw validation object, a shape production
never emits, which is why two of its cases could only accept "400 or
422". Both are exact now: an unusable file is refused 422 before the auth
guard runs, a valid file from an anonymous caller is refused 401, and the
two domain rejections are 400 with their real messages.
The five `Value.Errors(...).First()` consumers assert their rendered
message and JSON-pointer location instead of the iterator object, since
that pointer is the entire diagnostic — for a maintainer reading CI, and
for a model expected to self-correct a tool call from it.
Test-only. No production code, dependency, or contract changes.
Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
CORS, OpenAPI, the two file-serving prefixes, and the request logger are configured once at the top of `app.ts` and never referenced again. Nothing downstream fails to compile if a plugin answers on a different path, drops an origin check, or stops publishing an operation — the HTTP response is the only evidence, so that is what these assert, against the real composed application. The generated document doubles as a route inventory. A framework change that silently drops a route leaves every other suite green, because a suite only covers routes it already knows about, so the sorted path-to-methods map is pinned as a fixture. `/uploads` is excluded from it: `@elysiajs/static` derives its published paths from whatever is on disk at registration, which makes that prefix a property of the machine rather than of the API. Also covered: the allowed and disallowed CORS preflights, `/scalar` and `/scalar/json` reachability, representative param, multipart-file, response, and `ApiErrorResponse` schemas, an operation id on every published operation, and the request logger staying silent for frontend assets while logging `/api`. The filesystem frontend branch now has its own precedence suite beside the embedded one — root, `/index.html`, a hashed asset, a deep link, an unknown API path, a real upload, and API-only mode — so a plugin swap has to keep each outcome before the documented `ignorePatterns` workaround can be dropped. It has to await `app.modules` first: the static plugin registers asynchronously, and a request issued before it settles sees a half-registered app that answers differently. Two findings recorded rather than fixed. The filesystem branch serves SPA deep links with the shell under a 404 while the embedded branch answers 200, so source and `npm install` deployments report 404 for every deep link. And `loadConfigForTest` sandboxed `library.backupDir` and `toolImages.dir` but not `uploads.dir` or `images.dir`, so the upload suites wrote real files into the developer's own ~/.mango/uploads; that one is fixed here, since it is a test-only helper with no production callers. Test-only apart from that helper. No production code, dependency, or contract changes. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
Both socket families already have deep protocol suites. Three things they did not cover are exactly the three a framework change moves. The transport caps were asserted as a constant and never as behavior: the realtime test server built a bare root instance, so `REALTIME_WEBSOCKET_OPTIONS` was compared field by field while no request ever met the limit. Both families now refuse an oversized frame, which also pins that the options are applied once on the root and inherited — the two routes are registered by different modules, and only the shared root ties them together. The realtime handshake now proves it ignores query parameters, including duplicated and unknown keys. The browser client connects to a bare `/api/ws`, so nothing may depend on a query string, and a parser change must not be able to reach the handshake. The runtime socket now proves the credential decision taken in the pre-upgrade `derive` reaches the opened socket, asserted through close codes and a working method call rather than through `ws.data`. The environment it binds comes from the verified token, so a query string naming a different one changes nothing. Test-only. No production code, dependency, or contract changes. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
`treaty<App>` is the only place the frontend's types come from the
backend's routes, and it is the quietest thing here to break. When Eden
loses hold of `App`, every route collapses to `any` and the application
keeps compiling — wrong field names, wrong request bodies, and deleted
endpoints all become runtime bugs, and no assertion anyone would write
catches them, because `any` satisfies all of them.
Pin the seam with compile-time assertions that `tsc` runs under
`bun run check`:
- the `/api` namespace is still a namespace, not `any`;
- a representative GET keeps its response union, including the shared
error body the route's missing response schema unions in;
- the transport error channel stays typed;
- a schema-backed POST keeps `title` required and `model` optional,
exactly as the TypeBox schema declares;
- its response keeps the whole entity, including the discriminated
`runner` union and the `user:${string}` agent id — that id survives a
`Type.Unsafe` in shared, so it is the strongest available proof that
precise types cross schema, route, Eden, and component intact;
- a list route stays an array of that entity;
- the WebSocket route is still reachable through the same client.
The fetcher test now also asserts `credentials: 'include'` and that the
caller's own init survives the spread. Losing the credential logs every
user out at the next request while every type and status code stays
exactly as it was.
No `any` and no double assertion bridges the client; `IsAny` guards make
that a failure rather than a silent pass.
Test-only. No production code, dependency, or contract changes.
Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
juliopolycarpo
added
type: test
github-actions
Bot
added
the
area: build
Summary by CodeRabbit
WalkthroughThis pull request expands contract coverage across the API, frontend, runtime, shared schemas, and QA gates. It tests route publication, CORS, OpenAPI metadata, static serving, authentication, rate limits, WebSocket behavior, SSE errors, upload responses, error sanitization, typed API-client behavior, tool validation paths, TypeBox operators, composed schemas, and nested schema error reporting. Test configuration now isolates upload, image, and tool-image directories. Possibly related PRs
Mergeability Score: 🔵 Low · up to The PR adds characterization tests and test-environment isolation, but one configuration path can still write under a developer’s ~/.mango directory and several tests need failure-safe cleanup or awaited assertions. This is mergeable with explicit owner follow-up because the risk is limited to test isolation and suite reliability, not production behavior. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/api/tests/integration/routes/lifecycle-contract.integration.test.ts (1)
225-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRelease the limiter in
afterEachinstead of after the assertions.
limiter.teardown()runs only when every precedingexpectpasses. A failing assertion throws first and leaves the limiter's internal state and timers alive for the rest of the file. The same pattern repeats at Lines 239, 262, and 284.Register the limiter with the existing
cleanuphook so teardown always runs.♻️ Proposed refactor
it('keeps headers a hook set before the handler ran', async () => { const limiter = rateLimit({ classify: classifyRateLimit, trustProxy: true }); + cleanup = () => limiter.teardown(); const app = createApiTestApp(limiter).get('/probe', () => ({ ok: true })); @@ expect(response.headers.get('x-ratelimit-reset')).toBeTruthy(); - - limiter.teardown(); });apps/api/tests/integration/routes/realtime-routes.integration.test.ts (1)
38-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute the root-options branch through the test harness.
Lines 61-63 construct
new Elysia({ websocket: REALTIME_WEBSOCKET_OPTIONS })directly.createApiTestAppcallsassertTestEnvironmentReady('createApiTestApp')before it builds the app, so this branch skips that guard and can run against an unprepared test environment.createApiTestAppbuildsnew Elysia()with no constructor options, so it cannot currently carry the websocket settings.Add an options parameter to
createApiTestAppand use it for both branches. Based on learnings, integration tests must usetests/support/harness/create-api-test-app.ts.Source: Learnings
apps/api/tests/integration/routes/runtime-socket.integration.test.ts (1)
428-447: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the open promise on
closeas well.Lines 440-442 await an
openevent only. If the upgrade is refused, the promise never settles and the test fails as a timeout with no diagnostic.dialRuntimealready avoids this by resolving itsopenedpromise on bothopenandclose. The raw socket is also not registered indialed, soafterEachcannot close it if the test aborts early.♻️ Proposed refactor
- await new Promise<void>((resolve) => { - socket.addEventListener('open', () => resolve(), { once: true }); - }); + const opened = await new Promise<boolean>((resolve) => { + socket.addEventListener('open', () => resolve(true), { once: true }); + socket.addEventListener('close', () => resolve(false), { once: true }); + }); + expect(opened).toBe(true);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 57b554fa-2a37-4455-8e12-e3693014e591
📒 Files selected for processing (19)
apps/api/src/lib/config.tsapps/api/tests/integration/routes/app-plugin-contract.integration.test.tsapps/api/tests/integration/routes/lifecycle-contract.integration.test.tsapps/api/tests/integration/routes/realtime-routes.integration.test.tsapps/api/tests/integration/routes/respond-stream-provider-turn.integration.test.tsapps/api/tests/integration/routes/runtime-socket.integration.test.tsapps/api/tests/integration/routes/upload.integration.test.tsapps/api/tests/support/fixtures/openapi-route-inventory.jsonapps/api/tests/unit/plugins/error-handler.test.tsapps/api/tests/unit/server/frontend-static.test.tsapps/api/tests/unit/services/tools/ask-user-question.test.tsapps/api/tests/unit/services/tools/todo-tool.test.tsapps/frontend/tests/unit/lib/api-client-types.test.tsapps/frontend/tests/unit/lib/api-client.test.tsapps/runtime/tests/unit/services/external-agent-supervisor.test.tsapps/shared/tests/unit/composed-schemas.test.tsapps/shared/tests/unit/typebox-operators.test.tsscripts/qa-gate/ci-durations.unit.test.tsscripts/qa-gate/metrics-envelope.unit.test.ts
|
PR head: Commits — 7 commitsBase
Full commit messages
|
| Metric | Base | Head | Δ |
|---|---|---|---|
| Wall clock (shared jobs) | 8m 39s | 8m 42s | 🔴 ▲ +3s |
| Critical path | Test / Unit & Integration Tests · 7m 4s |
Test / Unit & Integration Tests · 6m 55s |
🟢 ▼ -9s |
Since previous PR run: 8m 50s → 8m 42s (🟢 ▼ -8s)
Five slowest head jobs
| Job | Base | Head | Δ |
|---|---|---|---|
Test / Unit & Integration Tests |
7m 4s | 6m 55s | 🟢 ▼ -9s |
Distribution / Build immutable distribution |
3m 32s | 2m 48s | 🟢 ▼ -44s |
QA Metrics / Collect |
1m 26s | 1m 30s | 🔴 ▲ +4s |
Check / Check (tooling + typecheck) |
54s | 51s | 🟢 ▼ -3s |
Smoke — Browser / Chromium smoke suite |
52s | 48s | 🟢 ▼ -4s |
QA Gate — Coverage & Quality
Base: 7f83da6 • Head: 6c8794b • generated 2026-08-13T13:11:50.822Z
✅ No attention signals — collected metrics look healthy against base.
LoC (code): 🔴 ▲ +1480 • Line coverage (all workspaces): ⚪ ▲ = 0 • Quick check: pass • Duplication: 🟢 ▼ -0.03pp • Bundle gzip: ⚪ ▲ = 0 • Locked deps: ⚪ ▲ = 0 • Tests passed: 🟢 ▲ +144
Metric details (coverage, LoC, bundle, dependencies, tests, duplication, tooling)
Coverage
API/shared/runtime branches and statements are source-derived from LCOV line hits because Bun LCOV does not emit branch or statement records.
| Workspace | Metric | Base | Head | Δ |
|---|---|---|---|---|
| frontend | lines | 87.32% (13,269/15,195) | 87.32% (13,269/15,195) | ⚪ ▲ = 0 |
| frontend | statements | 75.98% (7,660/10,081) | 75.97% (7,659/10,081) | 🔴 ▼ -0.01pp |
| frontend | functions | 72.88% (2,357/3,234) | 72.88% (2,357/3,234) | ⚪ ▲ = 0 |
| frontend | branches | 67.82% (5,505/8,117) | 67.81% (5,504/8,117) | 🔴 ▼ -0.01pp |
| api | lines | 81.99% (67,118/81,860) | 82.00% (67,348/82,133) | 🟢 ▲ +0.01pp |
| api | statements | 81.25% (26,814/33,000) | 81.23% (26,861/33,067) | 🔴 ▼ -0.02pp |
| api | functions | 84.78% (6,762/7,976) | 84.74% (6,767/7,986) | 🔴 ▼ -0.04pp |
| api | branches | 49.56% (9,976/20,130) | 49.50% (9,987/20,174) | 🔴 ▼ -0.06pp |
| shared | lines | 98.21% (13,906/14,160) | 98.21% (13,906/14,160) | ⚪ ▲ = 0 |
| shared | statements | 94.88% (2,409/2,539) | 94.88% (2,409/2,539) | ⚪ ▲ = 0 |
| shared | functions | 92.73% (408/440) | 92.73% (408/440) | ⚪ ▲ = 0 |
| shared | branches | 60.95% (793/1,301) | 60.95% (793/1,301) | ⚪ ▲ = 0 |
| runtime | lines | 82.12% (24,756/30,146) | 82.12% (24,756/30,146) | ⚪ ▲ = 0 |
| runtime | statements | 78.58% (8,901/11,328) | 78.58% (8,901/11,328) | ⚪ ▲ = 0 |
| runtime | functions | 76.95% (1,856/2,412) | 76.95% (1,856/2,412) | ⚪ ▲ = 0 |
| runtime | branches | 49.43% (3,489/7,059) | 49.43% (3,489/7,059) | ⚪ ▲ = 0 |
Lines of Code
| Workspace | Base | Head | Δ |
|---|---|---|---|
| frontend | 580 files / 66,559 lines | 581 files / 66,618 lines | files 🔴 ▲ +1 • code 🔴 ▲ +59 |
| api | 961 files / 136,685 lines | 963 files / 137,484 lines | files 🔴 ▲ +2 • code 🔴 ▲ +799 |
| shared | 186 files / 25,090 lines | 188 files / 25,693 lines | files 🔴 ▲ +2 • code 🔴 ▲ +603 |
| runtime | 824 files / 39,622 lines | 824 files / 39,641 lines | files ⚪ ▲ = 0 • code 🔴 ▲ +19 |
| total | 2,551 files / 267,956 lines | 2,556 files / 269,436 lines | files 🔴 ▲ +5 • code 🔴 ▲ +1480 |
Frontend Bundle
| Metric | Base | Head | Δ |
|---|---|---|---|
| gzip total | 817.1 KiB | 817.1 KiB | ⚪ ▲ = 0 |
| gzip JavaScript | 801.2 KiB | 801.2 KiB | ⚪ ▲ = 0 |
| gzip CSS | 15.0 KiB | 15.0 KiB | ⚪ ▲ = 0 |
| gzip HTML | 949 B | 949 B | ⚪ ▲ = 0 |
| tracked files | 35 | 35 | ⚪ ▲ = 0 |
Dependencies
| Metric | Base | Head | Δ |
|---|---|---|---|
| locked packages | 861 | 861 | ⚪ ▲ = 0 |
| direct dependencies | 40 | 40 | ⚪ ▲ = 0 |
| direct devDependencies | 42 | 42 | ⚪ ▲ = 0 |
| workspace manifests | 5 | 5 | ⚪ ▲ = 0 |
Tests
Single full-suite pass (unit + integration, from the coverage run).
| Base | Head | Δ passed |
|---|---|---|
| 5,785 passed (root 5,785 / frontend 0 / api 0 / shared 0 / runtime 0) · exit 0 · 400s | 5,929 passed (root 5,929 / frontend 0 / api 0 / shared 0 / runtime 0) · exit 0 · 392s | 🟢 ▲ +144 |
Code Duplication (jscpd)
| Metric | Base | Head | Δ |
|---|---|---|---|
| clones | 1,154 | 1,155 | 🔴 ▲ +1 |
| duplicated lines | 13,490 | 13,498 | 🔴 ▲ +8 |
| percentage | 4.07% | 4.05% | 🟢 ▼ -0.03pp |
Repo Tooling
| Metric | Base | Head | Δ |
|---|---|---|---|
| Full repo check | pass | pass | ⚪ ▲ = 0 |
| TS errors (total) | 0 | 0 | ⚪ ▲ = 0 |
| Circular dependencies | 0 | 0 | ⚪ ▲ = 0 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ff414004b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Several new assertions stayed green without exercising the contract they named. Request URL canonicalization skipped the image handler, the runtime payload cap was proven with a text frame the protocol rejects first, and the WebSocket cap was applied on a reconstructed root rather than the exported app. Isolate toolImages in the test sandbox the same way uploads and images already are. Await rejection matchers, close the supervisor in finally, and inspect structured console arguments so a nested credential cannot hide behind String(error). Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
juliopolycarpo
changed the title
juliopolycarpo
deleted the
test/characterize-framework-migration-contracts
branch
August 13, 2026 13:25
4 of 5 tasks
This was referenced Aug 14, 2026
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
1 participant
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
An upcoming HTTP-framework and schema-library swap cannot be staged, because shared TypeBox schemas go straight into routes. These tests pin current behavior that a swap can move while the repo still type-checks, so the cutover is judged against assertions instead of inference.
No production route, contract, or error behavior changes. The only
src/change isloadConfigForTest, which now sandboxesuploads.dir,images.dir, andtoolImages.dirso suites stop writing into the developer's real~/.mango.Changes
Type.Compositesite plusanyOfconstants,Type.Never,Type.Exclude,Type.Omit,Type.Record, anchored patterns,ReadonlyArraySchema, andValue.Equal.requireAuth, rate-limit derive order, sibling module isolation, andapiKeyGuardbody handling.ApiErrorResponse/SSEErrorEventshape, sanitization, upload 422/401/400, and JSON-pointer locations fromValue.Errors(...).First().openapi-route-inventory.jsonfixture. Cover both WebSocket families for frame caps, query-string handshake, and credential derive. Pin the Edentreaty<App>type seam andcredentials: 'include'.close()infinally, structured console credential checks.loadConfigForTest/loadTestSandboxConfig.Left as recorded findings rather than product changes:
Type.Compositedrops member-leveladditionalProperties: false(RuntimePairingIssueSchemaaccepts unknown keys).Test Plan
bun run checkpassesbun run testpassesbun run buildpasses@mangostudio/shared/i18n(no hardcoded strings)CI on this head: Check, Test, Build, Chromium smoke, and binary/container smokes all passed. No new UI strings. Local Playwright was not available on the author's machine; Chromium smoke ran in CI.
Screenshots / GIFs
N/A. Test-only.
Notes
errorHandlerthe wayapp.tsdoes, so 400 vs 422 is exact.