Skip to content

fix(metadata): honour sortKeys in the typescript/javascript serializer - #19901

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-19872-ts-serializer-sortkeys
Sep 23, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-19872-ts-serializer-sortkeys

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #19872

What

sortKeys — a MetadataSaveOptions field already honoured by the json and yaml formats — is now honoured by the typescript / javascript formats too.

A1 — reproduced; javascript is affected the same way and fixed the same way

Serializing an object with sortKeys: true: the json keys came out sorted, the typescript keys did not (typescript-serializer.ts never read options.sortKeys). javascript is the same TypeScriptSerializer class, routed through the same renderModule() — confirmed it silently ignored sortKeys too, and one fix (in renderModule()) covers both formats. Pinned separately for each.

A2 — JSONSerializer's sort is deep (recursive), matched exactly

JSONSerializer.sortObjectKeys() recurses into nested objects; arrays keep element order, only object keys sort. Extracted verbatim into packages/metadata/src/serializers/sort-object-keys.ts (package-internal, published from no @objectstack/metadata exports entry) so TypeScriptSerializer's renderModule() reuses the exact same sort — no second implementation. JSONSerializer keeps its private sortObjectKeys() method, now a thin delegate to the shared function (see .d.ts note below for why it's kept rather than inlined away).

A3 — byte-identity when sortKeys is off/absent, pinned for object through the loader path

sortKeys absent or false is byte-identical to origin/main, for every format, including through FilesystemLoader.save() → serializeTypeScriptForMetadataType().

.d.ts diff (file-surface constraint honoured)

Built @objectstack/metadata on origin/main (dabf8d795e) and on this branch's head; diffed every exports entry's emitted .d.ts (., ./node, ./migrations, ./errors, ./view-container). Four of five identical byte-for-byte. index.d.ts (and node.d.ts, which re-exports it — also checked, identical) differs by exactly one added JSDoc comment block on JSONSerializer's already-existing private sortObjectKeys member — no declaration line changed, nothing added or removed from the class's shape. That private method's body now delegates to the shared helper instead of holding its own copy of the sort; it's kept (not deleted) specifically so the emitted .d.ts stays byte-identical apart from the comment.

Tests

New describe('sortKeys (#19872)') in packages/metadata/src/serializers/serializers.test.ts:

  • sortKeys: true through the public serialize(), serializeTypeScriptForMetadataType(), and the javascript format — each asserted byte-identical to JSONSerializer's own sorted output wrapped in the module text, using a fixture unsorted at both the top level and a nested level (proves the sort is deep, not top-level-only).
  • sortKeys absent/false: byte-identical to origin/main, both direct calls and through FilesystemLoader.save().
  • Round trip (serialize/serializeTypeScriptForMetadataType → deserialize) with sortKeys: true, for view and object.
  • FilesystemLoader.save() honouring sortKeys: true for object, through the loader path (A3).

Ablation (fix committed first): reverted only the two implementation files to origin/main's version and removed the new shared-sort file, ran the new pins — 4 of 7 went red (every sortKeys: true pin), 3 stayed green (the byte-identity-when-off pins and the order-insensitive round trip) — exactly the expected split. Restored with git checkout HEAD --, verified git diff HEAD empty and git hash-object matches HEAD:<path> for all three touched/added files.

README

packages/metadata/README.md's TypeScriptSerializer paragraph is silent on which SerializeOptions it honours — no false claim about sortKeys to correct — so left unedited, per the file surface's own condition ("only if it states which options the format honours").

Gates

Re-derived on the final commit with node scripts/pm/dispatch-gates.mjs --commands; every listed command run with its exit code captured before any pipe (full list in the report). The known check:issue-citations gap (the derived family runs only --self-test) is closed by also running the live verdict, node scripts/check-issue-citations.mjs, by hand.

Clause-②: no

Acceptance notes

No out-of-scope findings.


Generated by Claude Code

TypeScriptSerializer wraps a JSON body (export const metadata = {...})
and never read options.sortKeys, unlike JSONSerializer and YAMLSerializer,
which already honour it. typescript is FilesystemLoader.save()'s default
format, so a caller asking for deterministic key order silently got
unsorted keys.

Both the public serialize() and the package-internal
serializeTypeScriptForMetadataType() share one renderModule() code path,
so one fix covers both, and covers the javascript format too (same class,
same path).

Extracted JSONSerializer's recursive key sort into a shared,
package-internal sort-object-keys.ts so the typescript format reuses the
exact same sort instead of a second implementation. JSONSerializer keeps
its private sortObjectKeys() as a thin delegate so the class's declared
shape (and the package's emitted .d.ts) is unchanged.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 16 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 029d8a47109c63986a27f53581ec6c980b8e2f14 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from a0931ee2dc5b77139b0ede7dec2318297b87e4f2 — the merge of head 67015feb5f1107858566df986bb8eaeb7b3420fe into base 029d8a47109c63986a27f53581ec6c980b8e2f14, 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 a0931ee2dc5b77139b0ede7dec2318297b87e4f2 && git checkout a0931ee2dc5b77139b0ede7dec2318297b87e4f2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 029d8a47109c63986a27f53581ec6c980b8e2f14 67015feb5f1107858566df986bb8eaeb7b3420fe && git checkout -B drift-repro 029d8a47109c63986a27f53581ec6c980b8e2f14 && git merge --no-ff 67015feb5f1107858566df986bb8eaeb7b3420fe

node scripts/docs-audit/affected-docs.mjs --json 029d8a47109c63986a27f53581ec6c980b8e2f14

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 67015feb5f1107858566df986bb8eaeb7b3420fe

Head = PR head; merge-base dabf8d795e; one commit, 5 files, +159/−18. @objectstack/metadata was built with tsup at the merge-base and at the head in throwaway worktrees (removed afterwards), and both dists were probed read-only. No derived gate was re-run.

① Derived judgments

  • (a) "already honoured by the json and yaml formats": TRUE. YAMLSerializer passes sortKeys to js-yaml, which sorts every mapping at every depth. The two sorts differ only for integer-like keys (js-yaml compares strings; a rebuilt JS object enumerates integer keys first). The changeset claims identity with JSONSerializer's sort, not yaml's, and the typescript format wraps a JSON body, so JSON is the right reference.
  • (b) Deep, array order kept: TRUE. sortObjectKeys is JSONSerializer's previous private body moved verbatim; measured at head across three nesting levels and a mixed array.
  • (c) sortKeys absent or false is byte-identical: TRUE. Base dist vs head dist, json / yaml / typescript / javascript × five option sets: 20/20 identical; through NodeMetadataManager.save() → FilesystemLoader.save(): 8/8 identical.
  • (d) Both paths honour it: TRUE. renderModule() is the single reader, called by serialize() and serializeTypeScriptForMetadataType(); measured for typescript and javascript through both. On the base dist the javascript loader call ignored it, which reproduces the card.
  • (e) Unreachable, declarations unchanged: CONFIRMED. No entry re-exports sort-object-keys.ts. Over 10 emitted declaration files, only index.d.ts / index.d.cts differ, by one comment hunk above private sortObjectKeys; in JSONSerializer; no declaration line changed.
  • (f) Round trip unchanged: TRUE; the base dist's deserialize also reads the head's sorted files.
  • Every changeset sentence, docblock, comment, PR-body section and test name is true at the head. Checks: 34 runs, 31 success, 3 roster skips, 0 failure.

② Semver level

@objectstack/metadata: patch, Clause-②: no, confirmed. It honours an option MetadataSaveOptionsSchema and SerializeOptions already declare and accept; no accept set widens, no interface changes, no export is added, and the emitted declarations change by one comment.

③ Boundary flags

  1. yaml and json/typescript key order differ only for integer-like keys: pre-existing, not claimed otherwise. ACCEPTABLE.
  2. The one-comment delta also appears in index.d.cts: same entry, no declaration change. ACCEPTABLE.
  3. The #19872 tracker number lands in a published declaration comment: comments may carry tracker numbers; check:doc-authoring is green. ACCEPTABLE.
  4. packages/metadata/README.md credits "optional key sorting" to JSONSerializer alone: an understatement, not false. ACCEPTABLE.
  5. javascript needs a formats opt-in on the loader path; the PR pins it through serialize(), as claimed, and the loader path was measured. ACCEPTABLE.
  6. No pin covers array element order: the sort is moved verbatim and the behaviour was measured. ACCEPTABLE.

Implemented-by: claude/issue-19872-ts-serializer-sortkeys
Reviewed-by: session_01TEhopqrWQYBycZzyJHpAZr

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 23, 2026 16:43
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit d1ca874 Sep 23, 2026
35 of 36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-19872-ts-serializer-sortkeys branch September 23, 2026 17:20
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

Development

Successfully merging this pull request may close these issues.

metadata: the declared save option sortKeys is honoured by the json and yaml formats but silently ignored by the default typescript format

1 participant