Skip to content

fix(metadata-protocol): order the ADR-0067 commit timeline by instant, so rollbackToPackageCommit stops planning off the weekday name - #14036

Merged
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-13995-commit-timeline-instant-order
Sep 1, 2026
Merged

fix(metadata-protocol): order the ADR-0067 commit timeline by instant, so rollbackToPackageCommit stops planning off the weekday name#14036
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-13995-commit-timeline-instant-order

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes #13995

created_at is an engine-injected audit column: it is not in datetimeFields, and SqlDriver#formatOutput repairs it only inside if (this.isSqlite). So the live SQL dialects hand it out of the record read door as a JS Date while the SQLite family hands out canonical ISO-Z text — pinned one layer down by packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts.

Both ADR-0067 commit-timeline consumers in packages/metadata-protocol/src/protocol.ts compared String(created_at). String(aDate) is "Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)", whose leading token is the weekday NAME, so lexicographic order over those strings runs Fri, Mon, Sat, Sun, Thu, Tue, Wed — unrelated to chronology, and stable across the whole set, so the failure is systematic rather than intermittent. There is never an "it worked once" to warn anyone.

  • listCommits returned the package timeline in weekday-name order while claiming newest-first. Its own comment stated the assumption in as many words — "sort by the ISO timestamp" — and that assumption was false on the production default driver.
  • rollbackToPackageCommit both consumed that ordering and re-derived the same comparison itself, so neither site could correct the other. It reverted apply commits older than the target and skipped the newer ones it exists to undo: a destructive operation planning off a wrong predicate.

The repair

Route A from the card — compare instants at the consumer. Both sites now go through compareAuditInstants, a sibling of the canonicalVersionInstant helper #13382 landed one seam over in this same file for the OCC updated_at comparison.

The canonicalisation is reused (canonicalVersionInstant's measured input domain — Date, epoch number, canonical ISO text, opaque — is exactly the domain a created_at arrives in). The ordering is new, because versionTokensAgree answers equality between two client-supplied version tokens and an ordering question needs a comparison, not ===; it also takes the pre-tidied pair an HTTP If-Match header produces, which no driver-stamped column has. When either side does not denote an instant the two are compared verbatim exactly as before, so only instant-bearing pairs change verdict. No tolerant ?? alias fallback was added anywhere (#13973's standing prohibition); no driver dependency was added to @objectstack/metadata-protocol.

Option B (normalising at the driver read door) is deliberately not taken here: it reverses withPostgresCalendarDayAsText and belongs to the whole census, not this site.

The pin, and the ablation that proves it bites

packages/metadata-protocol/src/protocol.commit-timeline-instant-order.test.ts drives a hand-made Date on the read side — @objectstack/metadata-protocol has no driver dependency and must not grow one, the same split the driver-side test documents for the OCC seam.

The fixture is four consecutive days, and that number is load-bearing: String(aDate) renders the weekday in the process timezone, so rather than pin TZ, the fixture is chosen so the old comparison is wrong in every alignment. Ranking the weekday names lexicographically (Fri=0, Mon=1, Sat=2, Sun=3, Thu=4, Tue=5, Wed=6), not one of the seven windows of four consecutive weekdays is monotonic — three days is not enough, since Mon Tue Wed and Fri Sat Sun both are. The first case in the file asserts that discriminating property mechanically rather than trusting the comment.

Ablation on the committed tree at ce47ff58f6, each leg mutated with anchored greps proving the edit landed in both directions and a changed git hash-object blob, restored under trap ... EXIT INT TERM with absolute paths and proved restored by an empty git diff HEAD plus a blob match. No rebuild leg is needed and none was fabricated: the pin imports ./protocol.js, a package-relative specifier vitest resolves to src/protocol.ts, and the first green run happened before packages/metadata-protocol/dist existed at all.

leg reverted site result
A listCommits sort 1 failed / 4 passed — only listCommits returns the timeline newest-first, which received cmt_d4, cmt_d3, cmt_d5, cmt_d1, cmt_d2 (weekday order Wed, Tue, Thu, Sun, Mon)
B rollbackToPackageCommit predicate 1 failed / 4 passed — only the planner case, which selected cmt_d1, cmt_d3, cmt_d4: it planned a revert of cmt_d1, a commit older than the target

The two legs are disjoint by construction: the planner case asserts a set, whose membership depends on site 2's predicate alone, so it stays green when only site 1 is reverted. The ISO-text half stays green in both legs — which is what shows the repair is about the Date shape and not a blanket rewrite.

The priority question the triage explicitly left unmeasured

Triage set p1 rather than p0 on one mitigating factor — "rollback is an operator-initiated action, no automatic path triggers it" — and said it had not measured that. Measured now, and the mitigation holds: across objectstack and objectui the only non-test caller of rollbackToPackageCommit is the REST route POST /packages/:id/rollback (packages/runtime/src/domains/packages.ts:750-756), behind requireManageMetadata. rollbackOnFailure in packages/core/src/kernel.ts:379 is plugin-startup rollback (rollbackStartedPlugins) and never reaches it; os rollback was removed from the CLI; client.packages.rollback has no non-test caller. Full search expressions, controls and the one residual are in the seat report.

One amplifier is worth recording even though it does not flip the verdict: objectui's packages/app-shell/src/preview/CommitTimeline.tsx renders GET /packages/:id/commits — the mis-ordered list — as the operator's selection surface, so the human in the loop was choosing a target from the wrong ordering.

Verification

Gate union re-run at b0db873a78, the final commit. All green, with two recorded non-greens that are not findings: check-test-completeness exits 3 = PREREQUISITE NOT MET when run with no saved turbo log (its own text says the local reading is NOT MEASURED), and check-half-states needs --use-env-proxy to reach GitHub, after which it exits 0.

  • 36 families from node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at the first commit, then 31 more after the census repair moved content/** and scripts/** — re-derived rather than reused, and all 67 run.
  • pnpm lint (repo-wide eslint . --no-inline-config) — exit 0, no narrowing claimed.
  • pnpm --filter @objectstack/metadata-protocol exec vitest run — 149 files, 2065 passed.
  • tsc --noEmit --listFiles on the package — 63 errors, the exact count its check-type-check-coverage ledger entry records, and --listFiles confirms both edited files were in the program while none of the 63 is in either. pnpm check:type-check-debt (--re-measure, on a fully built workspace) exits 0.
  • Consumers, against the built dist: objectql protocol-commit-history.test.ts 32 passed; runtime package-list-commits-org-scope + package-revert-commit-org-scope + package-revert-commit-attribution-org-scope + http-dispatcher + packages-capability-gate341 passed.
  • No public surface moves: all three new helpers are module-local and appear 0 times in the emitted dist/index.d.ts; listCommits and rollbackToPackageCommit keep their signatures.

The two gate repairs in the second commit, both applied with the gates' own tooling

  • check-system-context-census --fix re-pointed row 21's anchor metadata-protocol/src/protocol.ts:1664 to :1736 — the shift the new helper block introduced above it. It re-pointed, it did not delete a row; the gate then reports 109 elevation read sites and 145 anchors resolving.
  • check-engine-double-contract --write added one row recording that the new pin file pins 1 findOne double: "1 added or grown, 0 lost". The shrink-only baseline is untouched.

Not addressed here

#13973 (the census) and #13382 (the OCC seam) are backlinks only and stay open. The sibling census cards #13996, #13997, #13998 and #13999 are each a different package and a different repair — none of them is touched by this branch.

One new class-(c) site found while working and not repaired here, because the correct shape is a judgement call rather than mechanical: listCommits declares createdAt?: string in its return type but assigns the raw driver value, so on Postgres and MySQL it hands a Date to every consumer of a field typed string. Filed separately; see the seat report.

Generated by Claude Code


Generated by Claude Code

…, not by the weekday name

`created_at` is an engine-injected audit column: not in `datetimeFields`, and
`SqlDriver#formatOutput` repairs it only inside `if (this.isSqlite)`. The live
SQL dialects therefore hand it out of the record read door as a JS `Date` while
the SQLite family hands out canonical ISO-Z text.

Both ADR-0067 commit-timeline consumers compared `String(created_at)`, and
`String(aDate)` is `"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)"` —
the LEADING token is the weekday NAME, so lexicographic order over those strings
is `Fri < Mon < Sat < Sun < Thu < Tue < Wed`. Unrelated to chronology, and
stable across the whole set, so it is wrong on every run and wrong the same way.

- `listCommits` returned the timeline in weekday-name order while claiming
  newest-first; its own comment stated the assumption ("sort by the ISO
  timestamp") and it was false on the production default driver.
- `rollbackToPackageCommit` both consumed that ordering and re-derived the same
  comparison itself, so neither site could correct the other: it reverted
  `apply` commits OLDER than the target and skipped the newer ones it exists to
  undo.

Both sites now compare canonical absolute instants through `compareAuditInstants`,
a sibling of the `canonicalVersionInstant` helper #13382 landed one seam over in
this same file. The canonicalisation is reused; the ordering is new, because
`versionTokensAgree` answers equality between client-supplied version tokens and
an ordering question needs `<`/`>`. When either side does not denote an instant
the two are compared verbatim exactly as before, so only instant-bearing pairs
change verdict.

The pin drives a hand-made `Date` — `@objectstack/metadata-protocol` has no
driver dependency and must not grow one — over four consecutive days, the
smallest fixture for which no timezone alignment can make the old weekday
comparison agree with chronology.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…w engine double

Both are the gates' own sanctioned repairs for the line/ledger movement the fix
caused, applied with their own tooling and inspected:

- `check-system-context-census --fix` RE-POINTED row 21's anchor
  `metadata-protocol/src/protocol.ts:1664` -> `:1736`, the 72-line shift the new
  `compareAuditInstants` helper block introduced above it. No row was deleted and
  no needle changed; the gate then reports 109 elevation read sites, 145 anchors
  resolving.
- `check-engine-double-contract --write` ADDED one row recording that the new pin
  file pins 1 `findOne` double ("1 added or grown, 0 lost"). The shrink-only
  baseline is untouched.

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

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

6 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 — 8 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 a7002ce5ac9acf6a4d556713c587a770f1848c26packageMentionDocs.

Which tree this was computed on

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

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

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

The merge of origin/main routed content/docs/permissions/system-context.mdx
through the os-regen driver, which exits 0 without text-merging and leaves
git's pre-filled OURS side in place. That silently dropped the 16 anchor
re-points main had landed (#13829, #13934, #13910, #13857) while keeping this
branch's single re-point.

This commit takes main's side of the page and re-derives every anchor from the
merged tree with `pnpm gen:system-context-census`, which re-pointed row 21's
metadata-protocol/src/protocol.ts anchor to 1736. Prose is byte-identical on
both sides once line numbers are normalised, so nothing but line numbers moved.
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit 3e343de Sep 1, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13995-commit-timeline-instant-order branch September 1, 2026 05:02
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

2 participants