Skip to content

Commit 3e343de

Browse files
fix(metadata-protocol): order the ADR-0067 commit timeline by instant, so rollbackToPackageCommit stops planning off the weekday name (#14036)
* fix(metadata-protocol): order the ADR-0067 commit timeline by instant, 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 * chore(gates): re-point the isSystem census anchor and register the new 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 * chore(docs): re-derive the isSystem census after merging origin/main 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. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent def3b7c commit 3e343de

5 files changed

Lines changed: 382 additions & 5 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): order the ADR-0067 commit timeline by INSTANT, so `rollbackToPackageCommit` stops planning off the weekday name (#13995)
6+
7+
`created_at` is an engine-injected audit column: it is not in `datetimeFields`,
8+
and `SqlDriver#formatOutput` repairs it only inside `if (this.isSqlite)`. So the
9+
live SQL dialects hand it out of the record read door as a JS `Date` while the
10+
SQLite family hands out canonical ISO-Z text. Both ADR-0067 commit-timeline
11+
consumers compared `String(created_at)` — and `String(aDate)` is
12+
`"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)"`, whose LEADING token
13+
is the weekday NAME. Lexicographic order over those strings is
14+
`Fri < Mon < Sat < Sun < Thu < Tue < Wed`: unrelated to chronology, stable
15+
across the whole set, and therefore wrong on every run and wrong the same way —
16+
there was never an "it worked once" to warn anyone.
17+
18+
- `listCommits` returned the package timeline in weekday-name order while
19+
claiming newest-first. Its own comment stated the assumption in as many words
20+
("sort by the ISO timestamp") and it was false on the production default
21+
driver.
22+
- `rollbackToPackageCommit` both CONSUMED that ordering and re-derived the same
23+
comparison itself, so neither site could correct the other. On Postgres and
24+
MySQL it reverted `apply` commits OLDER than the target and skipped the newer
25+
ones it exists to undo — a destructive operation planning off a wrong
26+
predicate.
27+
28+
Both sites now compare canonical absolute instants, through a sibling of the
29+
`canonicalVersionInstant` helper #13382 landed one seam over in this same file
30+
for the OCC `updated_at` comparison. The canonicalisation is reused; the
31+
ordering is new, because `versionTokensAgree` answers equality between two
32+
client-supplied version tokens and an ordering question needs `<`/`>`. When
33+
either side does not denote an instant the two are compared verbatim exactly as
34+
before, so the only verdicts that change are the pairs that denote one.
35+
36+
On SQLite and the memory driver both sides were already canonical ISO-Z text and
37+
lexicographic order equalled chronological order, so nothing changes there —
38+
which is why every test these sites had stayed green through the defect.

content/docs/permissions/system-context.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ that silently does not happen.
112112
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10787` |
113113
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:10949` |
114114
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9680` |
115-
| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1664` |
115+
| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1736` |
116116
| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9717`, `readonly-strict-errors.ts:66` |
117117
| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5705` |
118118
| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3574`, `:3584`, `:3611` |
Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#13995] The ADR-0067 package commit timeline sorted by WEEKDAY NAME on the
4+
// production default drivers, and `rollbackToPackageCommit` planned its reverts
5+
// off the same comparison.
6+
//
7+
// ---------------------------------------------------------------------------
8+
// The defect
9+
// ---------------------------------------------------------------------------
10+
// `created_at` is an engine-injected audit column: it is not in `datetimeFields`
11+
// and `SqlDriver#formatOutput` repairs it only inside `if (this.isSqlite)`, so
12+
// the live SQL dialects hand it out of the record read door as a JS `Date` while
13+
// the SQLite family hands out canonical ISO-Z text. Pinned one layer down by
14+
// `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
15+
//
16+
// Both timeline consumers in `protocol.ts` compared `String(created_at)`:
17+
//
18+
// listCommits mapped.sort(… String(b.createdAt).localeCompare(…))
19+
// rollbackToPackageCommit all.filter(c => String(c.createdAt) > targetCreatedAt)
20+
//
21+
// `String(aDate)` is `"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)"`
22+
// — the LEADING token is the weekday name — so lexicographic order over those
23+
// strings is `Fri < Mon < Sat < Sun < Thu < Tue < Wed`, unrelated to chronology.
24+
// The order is stable across the whole set, so the failure is systematic and
25+
// identical on every run: there is never an "it worked once" to warn anyone.
26+
//
27+
// The two sites reinforce rather than backstop each other. `listCommits` returns
28+
// a mis-ordered timeline while claiming newest-first, and
29+
// `rollbackToPackageCommit` both CONSUMES that ordering and re-derives the same
30+
// comparison itself — so it reverts `apply` commits OLDER than the target and
31+
// skips the newer ones it exists to undo. That is a destructive operation
32+
// planning off a wrong predicate.
33+
//
34+
// ---------------------------------------------------------------------------
35+
// Why the fixture is FOUR consecutive days, and why that makes this pin
36+
// timezone-independent
37+
// ---------------------------------------------------------------------------
38+
// `String(aDate)` renders the weekday in the PROCESS timezone, so which weekday
39+
// name each instant carries depends on `TZ`. Rather than pin `TZ` (which Node
40+
// caches per platform) the fixture is chosen so the old comparison is wrong in
41+
// EVERY alignment: map each weekday to its rank in the lexicographic order
42+
// (`Fri`=0, `Mon`=1, `Sat`=2, `Sun`=3, `Thu`=4, `Tue`=5, `Wed`=6) and read off
43+
// the seven windows of four consecutive weekdays —
44+
//
45+
// Sun Mon Tue Wed -> 3 1 5 6 Thu Fri Sat Sun -> 4 0 2 3
46+
// Mon Tue Wed Thu -> 1 5 6 4 Fri Sat Sun Mon -> 0 2 3 1
47+
// Tue Wed Thu Fri -> 5 6 4 0 Sat Sun Mon Tue -> 2 3 1 5
48+
// Wed Thu Fri Sat -> 6 4 0 2
49+
//
50+
// — not one of the seven is monotonic, so no timezone can make the old sort
51+
// agree with chronology. (Three consecutive days is NOT enough: `Mon Tue Wed`
52+
// and `Fri Sat Sun` are both increasing.) The same table settles the planner:
53+
// with the target at the second day, the old predicate selects `{1,3,4}`,
54+
// `{3}`, `{}`, `{1}`, `{1,3,4}`, `{3}`, `{4}` across the seven alignments and
55+
// never the correct `{3,4}`. Every instant is 24h apart at 12:00Z, far enough
56+
// from local midnight that no offset or DST step can collapse two onto one
57+
// local day. `assertTheFixtureDiscriminates` below asserts the property
58+
// mechanically rather than trusting this comment.
59+
//
60+
// ---------------------------------------------------------------------------
61+
// Why the `Date` is hand-made rather than read off a driver
62+
// ---------------------------------------------------------------------------
63+
// `@objectstack/metadata-protocol` has no driver dependency and must not grow
64+
// one — the layering runs the other way. This is the same split
65+
// `sql-driver-13567-audit-stamp-materialisation.test.ts` documents for the OCC
66+
// seam: the driver package pins WHAT the dialects materialise, and the consumer
67+
// package pins that it is correct FOR that shape.
68+
//
69+
// ---------------------------------------------------------------------------
70+
// Reverse verification, direction predicted BEFORE running
71+
// ---------------------------------------------------------------------------
72+
// Ordinary red, and separated by site so the ablation says which one broke:
73+
//
74+
// * Restore `String(b.createdAt ?? '').localeCompare(String(a.createdAt ?? ''))`
75+
// in `listCommits` -> the two ordering cases go red, the planner case stays
76+
// GREEN (a `filter` preserves order but the selected SET does not depend on
77+
// it), and the ISO-text cases stay green.
78+
// * Restore `String(c.createdAt ?? '') > String(target.created_at ?? '')` in
79+
// `rollbackToPackageCommit` -> the planner case goes red on the SET, and the
80+
// ordering cases stay green.
81+
//
82+
// The ISO-text half stays green in both directions on purpose: it is what shows
83+
// the repair is the `Date` shape and not a blanket rewrite of the comparison.
84+
85+
import { describe, it, expect, vi } from 'vitest';
86+
import { ObjectStackProtocolImplementation } from './protocol.js';
87+
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
88+
89+
/** Four consecutive days plus one more, at 12:00Z. See the header for why. */
90+
const DAY_1 = '2026-08-30T12:00:00.000Z';
91+
const DAY_2 = '2026-08-31T12:00:00.000Z';
92+
const DAY_3 = '2026-09-01T12:00:00.000Z';
93+
const DAY_4 = '2026-09-02T12:00:00.000Z';
94+
const DAY_5 = '2026-09-03T12:00:00.000Z';
95+
96+
/** A registry with nothing in it — the commit store is the only source here. */
97+
function emptyRegistry() {
98+
return {
99+
getObject: () => undefined,
100+
getItem: () => undefined,
101+
listItems: () => [],
102+
applyNavContributions: (x: any) => x,
103+
isPackageDisabled: () => false,
104+
getObjectOwner: () => undefined,
105+
};
106+
}
107+
108+
/** One `sys_metadata_commit` row, in the driver's snake_case wire shape. */
109+
function commitRow(id: string, createdAt: Date | string, operation = 'apply') {
110+
return {
111+
id,
112+
package_id: 'pkg_crm',
113+
organization_id: null,
114+
operation,
115+
message: `commit ${id}`,
116+
actor: 'alice',
117+
item_count: 1,
118+
items: JSON.stringify([{ type: 'object', name: 'acct', existedBefore: true, prevVersion: 3 }]),
119+
created_at: createdAt,
120+
};
121+
}
122+
123+
/**
124+
* The five commits, in the shape ONE dialect family hands them over.
125+
*
126+
* `stamp` is the whole difference between the two families: `Date` is what
127+
* Postgres and MySQL materialise for this column, the ISO-Z string is what the
128+
* SQLite family and memory return. Rows are handed over oldest-first on purpose
129+
* — the sort, not the driver, is what must make the timeline newest-first.
130+
*/
131+
function timeline(stamp: (iso: string) => Date | string) {
132+
return [
133+
commitRow('cmt_d1', stamp(DAY_1)),
134+
commitRow('cmt_d2', stamp(DAY_2)),
135+
commitRow('cmt_d3', stamp(DAY_3)),
136+
commitRow('cmt_d4', stamp(DAY_4)),
137+
commitRow('cmt_d5', stamp(DAY_5), 'revert'),
138+
];
139+
}
140+
141+
const asDate = (iso: string) => new Date(iso);
142+
const asIsoText = (iso: string) => iso;
143+
144+
/** An engine that answers both commit-store reads out of `rows`. */
145+
function engineWithCommits(rows: any[]) {
146+
return {
147+
registry: emptyRegistry(),
148+
find: vi.fn(async () => rows),
149+
findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => {
150+
assertEngineFindOnePredicate(object, query);
151+
const id = (query as any)?.where?.id;
152+
return rows.find((r) => r.id === id) ?? null;
153+
}),
154+
} as any;
155+
}
156+
157+
/**
158+
* A protocol whose `revertCommit` only RECORDS what it was handed.
159+
*
160+
* The defect under test is which commits `rollbackToPackageCommit` SELECTS, not
161+
* what reverting one does; stubbing the per-commit revert keeps the assertion on
162+
* the plan and off `revertCommit`'s own (separately pinned) machinery.
163+
*/
164+
function protocolWithPlanRecorder(rows: any[]) {
165+
const protocol = new ObjectStackProtocolImplementation(engineWithCommits(rows));
166+
(protocol as any).revertCommit = async () => ({
167+
success: true,
168+
revertedCount: 1,
169+
failedCount: 0,
170+
reverted: [{ type: 'object', name: 'acct', action: 'restored' }],
171+
failed: [],
172+
});
173+
return protocol;
174+
}
175+
176+
describe('[#13995] the commit timeline orders by INSTANT, not by the weekday name', () => {
177+
it('the fixture discriminates: the old `String(...)` compare disagrees with chronology', () => {
178+
// The positive control for everything below. If this ever passes, the
179+
// `Date` cases stop being able to catch the defect and the pin is
180+
// vacuous — which is exactly the state the OCC seam was in.
181+
const stamps = [DAY_1, DAY_2, DAY_3, DAY_4].map(asDate);
182+
const byOldStringCompare = [...stamps]
183+
.sort((a, b) => String(b).localeCompare(String(a)))
184+
.map((d) => d.toISOString());
185+
const byChronology = [...stamps]
186+
.sort((a, b) => b.getTime() - a.getTime())
187+
.map((d) => d.toISOString());
188+
189+
expect(byOldStringCompare).not.toEqual(byChronology);
190+
});
191+
192+
describe('the `Date`-materialising dialects (Postgres, MySQL)', () => {
193+
it('listCommits returns the timeline newest-first', async () => {
194+
const p = new ObjectStackProtocolImplementation(engineWithCommits(timeline(asDate)));
195+
196+
const commits = await p.listCommits({ packageId: 'pkg_crm' });
197+
198+
expect(commits.map((c) => c.id)).toEqual([
199+
'cmt_d5', 'cmt_d4', 'cmt_d3', 'cmt_d2', 'cmt_d1',
200+
]);
201+
});
202+
203+
it('rollbackToPackageCommit reverts exactly the `apply` commits NEWER than the target', async () => {
204+
const p = protocolWithPlanRecorder(timeline(asDate));
205+
206+
const result = await p.rollbackToPackageCommit({ commitId: 'cmt_d2' });
207+
208+
// Asserted as a SET: membership is decided by this site's predicate
209+
// alone, so this case stays green if only `listCommits`' sort is
210+
// reverted and red if only this site's is.
211+
expect([...result.revertedCommits].sort()).toEqual(['cmt_d3', 'cmt_d4']);
212+
// The target itself and everything older than it are untouched, and
213+
// the `revert` commit is skipped — its effect is already captured by
214+
// re-reverting the apply it undid.
215+
expect(result.revertedCommits).not.toContain('cmt_d1');
216+
expect(result.revertedCommits).not.toContain('cmt_d2');
217+
expect(result.revertedCommits).not.toContain('cmt_d5');
218+
expect(result.success).toBe(true);
219+
});
220+
});
221+
222+
describe('the ISO-text dialects (the SQLite family, memory) are unchanged', () => {
223+
it('listCommits still returns the timeline newest-first', async () => {
224+
const p = new ObjectStackProtocolImplementation(engineWithCommits(timeline(asIsoText)));
225+
226+
const commits = await p.listCommits({ packageId: 'pkg_crm' });
227+
228+
expect(commits.map((c) => c.id)).toEqual([
229+
'cmt_d5', 'cmt_d4', 'cmt_d3', 'cmt_d2', 'cmt_d1',
230+
]);
231+
expect(commits[0]!.createdAt).toBe(DAY_5);
232+
});
233+
234+
it('rollbackToPackageCommit still reverts exactly the `apply` commits newer than the target', async () => {
235+
const p = protocolWithPlanRecorder(timeline(asIsoText));
236+
237+
const result = await p.rollbackToPackageCommit({ commitId: 'cmt_d2' });
238+
239+
expect([...result.revertedCommits].sort()).toEqual(['cmt_d3', 'cmt_d4']);
240+
expect(result.success).toBe(true);
241+
});
242+
});
243+
});

0 commit comments

Comments
 (0)