Skip to content

fix(metadata-protocol): listCommits emits the ISO-8601 string createdAt declares - #15008

Merged
os-musk merged 9 commits into
mainfrom
claude/issue-14038-listcommits-createdat-iso
Sep 4, 2026
Merged

fix(metadata-protocol): listCommits emits the ISO-8601 string createdAt declares#15008
os-musk merged 9 commits into
mainfrom
claude/issue-14038-listcommits-createdat-iso

Conversation

@os-musk

@os-musk os-musk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14038

The defect

listCommits's declared return type says createdAt?: string
(packages/metadata-protocol/src/protocol.ts:19049), but the mapping
assigned the raw driver value straight through
(protocol.ts:19107 before this change:
...(r.created_at ? { createdAt: r.created_at } : {})). created_at on
sys_metadata_commit is an engine-injected audit column — not in
datetimeFields — and SqlDriver#formatOutput repairs it only inside its
if (this.isSqlite) arm (packages/drivers/driver-sql/src/sql-driver.ts:16056,
audit-column repair at :16123, the datetimeFields fold at :16137
both inside the same arm). So on Postgres and MySQL, listCommits handed
every in-process consumer a Date in a field the type promised was a
string. The REST door (GET /packages/:id/commits) was never affected —
JSON.stringify already renders a Date as canonical ISO-Z text.

The fix

A narrow per-site conversion at the producer
(protocol.ts:1726, isoFromValidDate), following #14037's
precedent (landed as #14939) rather than the shared canonicalIsoInstant
spelling: a valid Date is canonicalised to ISO-8601, and every other
shape — an already-canonical SQLite string, an absent column, and
an Invalid Date — passes through unchanged. #14037 deliberately did
not adopt canonicalIsoInstant here because #14078 measured an Invalid
Date reachable on both live dialects (a MySQL zero datetime; a Postgres
year in 275760..294276) where that spelling raises RangeError, and #13973
is blocked on that ruling. This PR imports neither answer to #14078 — it
just doesn't regress the field it touches. The Invalid-Date neutrality is
pinned in §D of the new test file so it reds if the contested spelling is
ever swapped in here.

Evidence (Zone 2, file:line)

  • A — declared packages/metadata-protocol/src/protocol.ts:19049;
    emitted (pre-fix) protocol.ts:19107 (r.created_at, unconverted).
  • B — route: narrow per-site conversion, not canonicalIsoInstant; see
    the doc comment on isoFromValidDate (protocol.ts:1726) and §D of the
    new test.
  • C — Postgres/MySQL-only divergence confirmed: sql-driver.ts's
    formatOutput gates both the builtin-audit-column repair and the
    datetimeFields fold inside one if (this.isSqlite) arm
    (:16056..:16137). No live cell needed — the new test drives a
    hand-made Date, matching this package's existing convention (no driver
    dependency).
  • D — one real in-process consumer: rollbackToPackageCommit
    (protocol.ts:19742) reads listCommits' createdAt through
    compareAuditInstants, which already normalises both a Date and an
    ISO string to epoch ms via canonicalVersionInstant — so canonicalising
    the emitted value to a string does not change its behaviour (no stop
    condition; not a regression).

Scope

Out of scope, per the dispatch: #14938 (listDrafts, a sixth site with
an inline return type) and the family's standing question of a shared
@objectstack/metadata-core export (ruled: after #14078, not before).

Tests

New: packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts
(§A Postgres/MySQL Date → ISO; §B SQLite text passthrough; §C absent
column; §D #14078 neutrality pin).

Union run at final head 8c9c060f2 (after merging origin/main twice while
this branch was in flight, most recently to pick up #14939 itself):
pnpm --filter @objectstack/metadata-protocol exec vitest run over the new
file plus the sibling #13995 and #14037 pins in this package — 3 files,
13 tests, all passed; pnpm --filter @objectstack/metadata-protocol typecheck
clean; pnpm check:engine-double-contract, check:system-context-census,
check:doc-authoring, check:nul-bytes, check:type-check-coverage, and
the rest of the ~70 commands node scripts/pm/dispatch-gates.mjs --commands
derives for this diff — all green. check:dual-build-cjs-loads,
check:test-completeness and check:type-check-debt read NOT MEASURED
locally (each needs a full-tree build or a CI test log this environment
doesn't produce) — not evidence of a red, left for CI.

Mechanical fallout from merging origin/main twice: two
check:system-context-census re-anchors (doc-table line numbers shifted by
unrelated commits) and one check:engine-double-contract pinned-ledger
entry for the new test's read-only fake engine. No behaviour change in
either.

Merge conflict resolution (2026-09-04)

The PR sat at head 8c9c060f2 / base 23c72be3c and went mergeable_state: dirty against main (never entered the merge queue). Resolved with a real
merge commit (no rebase/amend/force-push) of origin/main (f594e70d7) —
three conflicted files, each resolved per its own kind:

File Kind Resolution
packages/metadata-protocol/src/protocol.ts hand-written source kept all three disjoint edits — this PR's isoFromValidDate + its listCommits call site, #14982's ADR-0094 mutation-projector calls (revertCommit, rollbackMetaItem, deleteMetaItem's legacy exit), and #15043's bumpWriteEpoch call in applyRemoteMetadataMutation's receipt path
content/docs/permissions/system-context.mdx generated line-anchor table re-derived with node scripts/check-system-context-census.mjs --fix — 22 anchors rewritten (pure line rot), 0 elevation read sites added or removed; check-system-context-census.mjs now reports OK
scripts/engine-double-contract.pinned.json append-only ledger kept both sides (git merged this one without a real conflict — the two sides' rows landed in disjoint regions); check-engine-double-contract.mjs reports 772 pinned, 0 dropped

Post-merge verification at new head c313a7c20 (base now f594e70d7):

  • isoFromValidDate confirmed still declared (protocol.ts:1727) and
    still called at the listCommits createdAt mapping (protocol.ts:19125).
  • The three fix(metadata-protocol): recovery doors run the ADR-0094 mutation projector #14982/fix(metadata-protocol): bump write epoch on remote metadata mutation receipt #15043 symbol sets confirmed present and in the right
    functions (revertCommit :19178-19831, rollbackMetaItem :19832-20321,
    deleteMetaItem :20322+, applyRemoteMetadataMutation :5235+).
  • PR's own pins re-run: protocol-14038-list-commits-created-at-iso.test.ts
    • sys-metadata-repository-14037-event-ts-canonicalisation.test.ts +
      protocol.commit-timeline-instant-order.test.ts — 3 files, 13 tests, all
      passed.
  • Full package suite re-run (pnpm --filter @objectstack/metadata-protocol test, now carrying fix(metadata-protocol): recovery doors run the ADR-0094 mutation projector #14982's and fix(metadata-protocol): bump write epoch on remote metadata mutation receipt #15043's behaviour changes in the same
    file): 161 passed / 2 skipped test files, 2370 passed / 10 skipped tests,
    0 failed.
  • pnpm --filter @objectstack/metadata-protocol typecheck: clean.
  • node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands re-derived post-merge (merge base moved to f594e70d7): 78
    commands, 75 exit 0; the remaining 3 (check:dual-build-cjs-loads,
    check:type-check-debt, check:skill-examples) all fail closed on the
    same precondition — no full-tree dist/ build in this worktree — and are
    unrelated to this diff (none touch client-react, and the first two
    explicitly exit their own PREREQUISITE-NOT-MET code); left for CI same as
    before.
  • node scripts/pm/check-governed-merges.mjs --test on the 5 changed paths:
    NOT governed — ordinary queue landing applies.

Generated by Claude Code

…At declares

`listCommits`'s declared return type says `createdAt?: string`, but the
mapping assigned the raw driver value straight through. `created_at` is an
engine-injected audit column, and `SqlDriver#formatOutput` repairs it only
inside its `if (this.isSqlite)` arm, so Postgres and MySQL hand it out of the
record read door as a JS `Date` — a value every in-process consumer received
in a field the type promised was a `string`.

Follows #14037's precedent: a narrow per-site `isoFromValidDate` helper
converts the one measured shape (a valid `Date`) and returns every other
shape, including an Invalid `Date`, unchanged — deliberately not the shared
`canonicalIsoInstant` spelling, which raises RangeError on an Invalid `Date`
reachable on both live dialects (#14078, on which #13973 is blocked).

Fixes #14038

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
check:system-context-census pins doc-table anchors by line number; the
listCommits fix above inserted ~40 lines earlier in protocol.ts and shifted
one anchored elevation-read site. Mechanical re-anchor via
`node scripts/check-system-context-census.mjs --fix` — no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
check:engine-double-contract requires every findOne/update/delete fake
engine double in a test file to be registered in the pinned ledger.
Registers the read-only findOne double the new #14038 pin test uses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Merging origin/main shifted lines in identity-write-guard.ts, auth-plugin.ts
and share-link-service.ts (unrelated incoming commits); mechanical re-anchor
via `node scripts/check-system-context-census.mjs --fix` — no behaviour
change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions github-actions Bot added the size/m label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

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 listnot 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 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 9 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 a54855005d337927632be4d864f0c85fa3a698e6packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json a54855005d337927632be4d864f0c85fa3a698e6

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

os-musk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review — PASS; marked ready and armed for the merge queue

An independent reviewer (not the author) re-derived this PR from the card, the ruling and the code rather than from the PR body. Verdict PASS, no blocking defects. What was re-proved rather than accepted:

The helper decision is not an implementer's invention — it executes a ruling that predates #14037. Triage's ruling on #14038 (comment 5487875301) rules option 1 (canonicalise at the mapping) and names this route explicitly: 「与 #13997 走的、以及 #14037 将走的是同一条路」. So the local-helper pattern was pre-authorised before #14037 existed, and #14939 (which closed #14037) then landed the byte-identical isoFromValidDate — same name, same body, same !Number.isNaN(value.getTime()) guard — into two files with the same docblock reason. This PR is the third faithful instance, not a competing idiom.

The three premises behind refusing canonicalIsoInstant were each verified rather than taken on trust: it does throw on an Invalid Date (sys-metadata-repository.ts:118-123value.toISOString() with no NaN guard), #14078 is genuinely open and genuinely about that (bug, priority:p1, pm:retriage), and #13973 is genuinely pm:blocked on it.

The reviewer found stronger evidence for the choice than the PR argues for itself. Tracing what would happen if the RangeError were allowed to propagate: listCommits' own try/catch (protocol.ts:19122-19127) routes it through rethrowUnlessMetadataStoreUnprovisioned, which converts it into a generic "metadata store could not be read". Adopting the contested spelling here would not merely risk a loud 500 — it would mislabel a code defect as a store outage. Confirmed by the ablation stack trace below.

Ruling compliance on the one instruction that constrains the prose. The ruling also says ⛔ 「PR说明里不要把这次修复写成「行为变更」」 — the fix honours a declaration rather than changing behaviour. Verified: no "behaviour/behavior change" framing appears in this PR's own text (the two hits in protocol.ts are pre-existing lines elsewhere).

Changeset patch, against the direct precedent. #14939's changeset, quoted: '@objectstack/metadata': patch / '@objectstack/metadata-protocol': patch — the identical defect class, matched exactly. The declared type does not move (createdAt?: string before and after); only the runtime value an in-process Postgres/MySQL caller receives does. That is not published type narrowing (no minor + **BREAKING** + adr-0087: owed) and adds no export or payload key.

Clause-② = no. isoFromValidDate is declared without export at protocol.ts:1726 and is re-exported nowhere; createdAt already existed on listCommits' return type (:19049) and on ListPackageCommitsResponseSchema (packages/spec/src/api/package-lifecycle.zod.ts:65, untouched).

Symbol fence, resolved on this head rather than from quoted numbers. listCommits opens 19036 and closes 19128; revertCommit opens 19160 — 32 lines clear of #14982's territory, and far from #13609's receipt path (~:5062-5199). The :19107 figure that appeared in an earlier reading as revertCommit's start came from a different base; on this head it is inside listCommits.

Two ablations, both directions, restore proven by state. Restoring the raw assignment reddens exactly §A (1 failed | 3 passed). Dropping the !Number.isNaN guard — i.e. importing the contested spelling's consequence — reddens exactly §D, which proves the neutrality pin is not vacuous. Both restores verified by blob-hash equality (c0730b3…), empty git diff HEAD and empty git status --porcelain, never by an exit code.

Consumers searched independently. rollbackToPackageCommit normalises both shapes to epoch ms via compareAuditInstantsauditInstantMscanonicalVersionInstant (.getTime() for a Date, Date.parse for a string — no throw), confirmed by reading the chain. The only other in-process caller is the REST handler (packages/runtime/src/domains/packages.ts:803-809), which never reads createdAt itself. Eleven test files mentioning listCommits carry no createdAt assertions outside the new and sibling pins. No missed consumer.

Merge fallout resolved the right way. The two check:system-context-census re-anchor commits are tool-generated (--fix) and touch only line-number columns; the generator was re-run on the final head (OK — 106 elevation read sites, all anchored). The engine-double-contract ledger diff is purely additive and carries both #14037's four rows and this card's one simultaneously.

Gates: 72 commands derived off the merge base with no path arguments — 68 exit 0, 4 genuine PREREQUISITE NOT MET, and 4 that first failed on an unbuilt prerequisite and passed clean once built. check-governed-merges --test: NOT governed. All 37 check runs on head 8c9c060f2 are success or skipped, zero red — ready, and auto-merge armed.

One non-blocking fast-follow, recorded not filed

protocol.ts already imports from ./sys-metadata-repository.js (:31), and that same-package file already declares a byte-identical, unexported isoFromValidDate (:165). Exporting it within the package and importing it — a same-package import, not a new package export, so clause-② and the public surface are untouched — would remove the duplication. Deliberately not filed as its own card: whatever resolves #14078 will replace all three local copies at once, so this cleanup belongs to that card's sweep rather than to a card of its own that would be closed by it.


Generated by Claude Code

Line numbers in protocol.ts and engine.ts shifted after merging main's
in-flight work (through f594e70); re-run
scripts/check-system-context-census.mjs --fix to repair pure line rot.
22 anchors rewritten, 0 elevation read sites added or removed.

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

os-musk commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

os-dev-report

{
  "issue": 14038,
  "status": "done",
  "branch": "claude/issue-14038-listcommits-createdat-iso",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/15008",
  "premise_still_valid": true,
  "summary": "Resolved PR #15008's mergeable_state:dirty by merging origin/main (f594e70d7) into the branch with a real merge commit (no rebase/amend/force-push). Three conflicts, each resolved per its artifact kind: protocol.ts kept all three disjoint symbol sets (this PR's isoFromValidDate + listCommits call site, #14982's ADR-0094 projector calls, #15043's bumpWriteEpoch call); system-context.mdx was re-derived with check-system-context-census.mjs --fix (22 pure line-rot anchors, 0 population change); engine-double-contract.pinned.json merged cleanly keeping both sides' rows (append-only, 772 pinned, 0 dropped). Added one re-anchor commit, pushed both, and updated the PR body with a merge-resolution section.",
  "tests": "Build: pnpm --filter '@objectstack/metadata-protocol^...' build, exit 0. PR's own pins (protocol-14038-list-commits-created-at-iso.test.ts + sys-metadata-repository-14037-event-ts-canonicalisation.test.ts + protocol.commit-timeline-instant-order.test.ts): 3 files, 13 tests, all passed. Full package suite (pnpm --filter @objectstack/metadata-protocol test): 161 passed/2 skipped test files, 2370 passed/10 skipped tests, 0 failed. Typecheck (pnpm --filter @objectstack/metadata-protocol typecheck): clean, exit 0. isoFromValidDate confirmed declared at protocol.ts:1727 AND called at the listCommits createdAt mapping protocol.ts:19125 (not merged away). node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands re-derived post-merge: 78 commands, 75 exit 0; the other 3 (check:dual-build-cjs-loads, check:type-check-debt, check:skill-examples) all fail closed on the same precondition -- no full-tree dist/ build in this worktree -- none touch this diff's files, and the first two print their own explicit PREREQUISITE NOT MET / exit 3; read as NOT MEASURED, not red, per dispatch instructions. node scripts/pm/check-governed-merges.mjs --test the 5 changed paths: 0 of 5 hit the register -- NOT governed, ordinary queue landing applies. node scripts/check-system-context-census.mjs (no --fix, post-commit): OK. node scripts/check-engine-double-contract.mjs: OK -- 772 pinned, 133 DEBT, 3 exempt.",
  "mcp_calls": 4,
  "open_questions": [],
  "out_of_scope_findings": []
}

Conflicted files

File Kind Resolution Evidence
packages/metadata-protocol/src/protocol.ts hand-written source kept all three disjoint edits isoFromValidDate declared :1727, called at listCommits mapping :19125; ADR-0094 projector calls present inside revertCommit (:19178-19831), rollbackMetaItem (:19832-20321), deleteMetaItem (:20322+); bumpWriteEpoch call present inside applyRemoteMetadataMutation (:5235+) at :5263
content/docs/permissions/system-context.mdx generated line-anchor table re-derived with check-system-context-census.mjs --fix 22 anchors rewritten (pure line rot), 0 population change; re-run without --fix reports OK
scripts/engine-double-contract.pinned.json append-only ledger kept both sides (merged cleanly, no rows dropped) check-engine-double-contract.mjs reports 772 pinned, 0 dropped

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit e37456e Sep 4, 2026
41 checks passed
@os-musk
os-musk deleted the claude/issue-14038-listcommits-createdat-iso branch September 4, 2026 02:44
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-protocol: listCommits declares createdAt as a string but emits the raw driver value, so it hands consumers a Date on Postgres/MySQL

2 participants