Skip to content

test(runtime): the dispatcher envelope check uses the shared predicate (#4090 follow-up) - #4105

Merged
os-zhuang merged 2 commits into
mainfrom
claude/envelope-drift-route-modules-v2zoky
Jul 30, 2026
Merged

test(runtime): the dispatcher envelope check uses the shared predicate (#4090 follow-up)#4105
os-zhuang merged 2 commits into
mainfrom
claude/envelope-drift-route-modules-v2zoky

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

The caller that motivated #4090's extraction, now actually using it. One file, one test body.

Why this isn't cosmetic

domain-handler-registry.test.ts hand-rolled "no key beside the envelope's own may hold the payload" when I wrote it for #4038. #4090 promoted that rule into envelopeViolations precisely so it would stop having two definitions. Leaving the local copy in place would have recreated the failure this whole line has been closing — one rule, two places, and only one of them updated next time.

It also wasn't the same rule. The local version was:

const ENVELOPE_KEYS = new Set(['success', 'data', 'meta']);
for (const key of Object.keys(body ?? {})) {
  expect(ENVELOPE_KEYS.has(key), ).toBe(true);
}

That passes any body whose top-level keys fall in the set — including a success body with no data at all, and one carrying an error alongside success: true. The shared predicate rejects both.

Verified as a strict improvement, not a refactor

I broke the producer two ways and checked the suite catches each:

injected body old local check shared predicate
{ success: true, data: link, link } (the #4049 drift) caught caught
{ success: true } (no payload) passed caught

Both now fail the suite; the second is the one that used to slip through.

Verification

@objectstack/runtime 914 passed. All ten gates in the TypeScript Type Check job, plus the six check:* guards — 16 total, all green.

Stale-dist note, worth recording because it cost me a round and reads as a real failure. After restarting this branch onto the newer main, two gates failed against a stale packages/spec/dist:

  • check:api-surface reported - strictUnknownKeyError, + AggregationFunction … "3 breaking, 8 added" — other people's exports, because the built .d.ts predated their commits.
  • check:i18n-coverage rejected examples/app-showcase/objectstack.config.ts for a 'combo' chart type — which the fresh spec allows.

Neither is a real failure and neither is mine; rebuilding spec cleared both, and I confirmed the i18n one passes identically with and without my change. Related: gen:api-surface reads dist/index.d.ts, so it cannot run under OS_SKIP_DTS=1 at all.


Generated by Claude Code

#4090 follow-up)

`domain-handler-registry.test.ts` hand-rolled "no key beside the envelope's own
may hold the payload" for #4038. #4090 extracted that rule into
`envelopeViolations` so it stops having two definitions; this is the caller that
motivated the extraction, now using it.

Leaving the local copy would have recreated the exact failure this line has been
closing — one rule, two places, and only one of them updated next time. It also
was not the same rule: the local set allowed any body whose top-level keys were
success/data/meta, so it passed a success body with NO `data` at all, and one
carrying an `error` alongside `success: true`. The shared predicate rejects both.

Verified the swap is a strict improvement rather than a refactor, by breaking the
producer two ways and checking the suite catches each:

  { success: true, data: link, link }   → caught (the old check caught this too)
  { success: true }                     → caught (the old check PASSED this)

runtime 914 passed. All ten gates in the TypeScript Type Check job pass, plus the
six `check:*` guards.

Note for the next person touching spec: after restarting a branch onto a newer
main, `check:api-surface` and `check:i18n-coverage` both fail against a stale
`packages/spec/dist` — the first reports other people's exports as added/removed,
the second rejects an example config for a chart type the fresh spec allows.
Neither is a real failure; rebuilding spec clears both. They read as "my change
broke something" and are not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 30, 2026 9:58am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

Check Changeset requires one on every PR; an empty-frontmatter changeset is the
sanctioned form for a change that releases nothing, and the gate's own error
message says so.

Second time this session — #4009 was a comment-only PR that failed the same way.
I fixed that one and did not draw the rule, which is that "this releases nothing"
is a reason to write an EMPTY changeset, not a reason to skip the file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling labels Jul 30, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review July 30, 2026 10:08
@os-zhuang
os-zhuang merged commit fcb6cfa into main Jul 30, 2026
17 checks passed
@os-zhuang
os-zhuang deleted the claude/envelope-drift-route-modules-v2zoky branch July 30, 2026 10:08
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/s tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants