Skip to content

fix: reject output shapes that redeclare reserved envelope fields - #52

Merged
beogip merged 1 commit into
mainfrom
beogip/fix-issue-50
Jul 29, 2026
Merged

fix: reject output shapes that redeclare reserved envelope fields#52
beogip merged 1 commit into
mainfrom
beogip/fix-issue-50

Conversation

@beogip

@beogip beogip commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

Output shapes passed to the type helpers could redeclare reserved envelope fields (ok, message, instructions). Because the author shape was spread after the composed envelope fields, an author declaration silently overrode the envelope contract — procedureTypeOutput({ instructions: z.string().optional() }) quietly made instructions optional, defeating the whole point of the Procedure type.

This change makes that collision loud at both layers:

  • Type layeroutput / fields now reject reserved keys at compile time via NoReserved<R>, applied to the three standalone helpers and to the three per-type subcommand spec interfaces.
  • Runtime layerassertNoReservedCollision throws a RangeError naming every colliding key in author declaration order, as the first statement of each helper. Since all three per-type subcommand methods funnel through the standalone helpers, one guard covers the class path, the legacy tool.subcommand path, and the @deprecated l1Output / l2Output / l3Output aliases with no extra wiring.

Reserved sets, declared once in src/output-helpers.ts and consumed by both layers:

Helper Reserved
dataTypeOutput ok, message
classificationTypeOutput ok, message
procedureTypeOutput ok, message, instructions

classification stays a legal author-declared key — it is a required key inside the author's own shape by design, structurally different from a fixed spread-in field.

Implementation notes

  • Follows the RESERVED_SUBCOMMANDS precedent (src/args.ts:18): declared in the file that owns the concept, exported at module level, absent from the src/index.ts barrel.
  • Error message mirrors the existing RangeError style in src/tool-class.ts:279-284 (fact + em-dash + causal clause).
  • New import edge types.ts → output-helpers.ts is type-only; no cycle.
  • Rejected alternative: reordering the spread so system fields win. It converts the silent loss into a loud validation error, but changes key order in both the parsed output and the --schema JSON Schema (ok,message,countcount,ok,message), breaking an invariant src/tool-class.ts:199-201 deliberately maintains.

Breaking-change assessment

Throwing where the helpers previously accepted silently is technically breaking, but it only breaks code that was already silently broken. A search across src/, tests/, examples/changeset.ts and README.md for colliding declarations returns zero hits — nothing in this repo collides. Ships as fix:.

Testing

  • tests/output-helpers.test.ts (new, 295 lines) — helper-level unit tests with no Tool and no invoke. The 9 schema-level tests moved out of index.test.ts, plus rejection tests per helper (reserved key present, weakened with .optional(), multiple simultaneous collisions) and pass-through tests (empty shape, no-arg overload, near-miss casing, prototype-inherited key, classification alone and with extras).
  • Rejection tests assert both the class and the message — a new idiom for this repo, where all 5 existing throw assertions check the class only, which would have left the "names every colliding key" criterion untested.
  • tests/index.test.ts — registration-path parity: a colliding output passed to each of dataSubcommand, classificationSubcommand, procedureSubcommand.
  • @ts-expect-error directives double as compile-time assertions: if the type guard regresses, bunx tsc --noEmit fails with Unused '@ts-expect-error' directive.

Verification: bun test180 pass / 0 fail. bunx tsc --noEmit clean. bunx biome check . clean.

Files changed

File +/-
src/output-helpers.ts +83
src/types.ts +36
tests/output-helpers.test.ts +295 (new)
tests/index.test.ts ±130
README.md +45
docs/specs/issue-50-*.md +263 (plan artifact)

Plan level: full / depth medium.

Closes #50

@beogip
beogip merged commit 262cbf6 into main Jul 29, 2026
6 checks passed
@beogip
beogip deleted the beogip/fix-issue-50 branch July 29, 2026 22:43
beogip pushed a commit that referenced this pull request Jul 29, 2026
## [0.3.1](v0.3.0...v0.3.1) (2026-07-29)

### Bug Fixes

* reject output shapes that redeclare reserved envelope fields ([#50](#50)) ([#52](#52)) ([262cbf6](262cbf6))
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: reject output shapes that redeclare reserved envelope fields

1 participant