Skip to content

test(service-settings): store decoded values in the getMany ROWS fixture, and pin the literal - #12379

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-12172-settings-getmany-fixture-encoding
Aug 25, 2026
Merged

test(service-settings): store decoded values in the getMany ROWS fixture, and pin the literal#12379
os-trump merged 2 commits into
mainfrom
claude/issue-12172-settings-getmany-fixture-encoding

Conversation

@os-trump

@os-trumpos-trump commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes#12172

packages/services/service-settings/src/settings-getmany.test.ts's ROWS fixture stored JSON text in value ('"America/New_York"'), so every fixture value resolved one JSON encoding deep.

Premise, verified on the merged ref (e5ce2ed03) before building on it

ClaimVerified atResult
sys_setting.value is a Field.json columnpackages/platform-objects/src/system/sys-setting.object.ts:156holds — value: Field.json({ … 'JSON-encoded value.' })
setMany writes verbatimsettings-service.ts:1667holds — storedValue = rawValue (no JSON.stringify)
materialiseRow reads verbatimsettings-service.ts:2553holds — non-encrypted branch is return row.value ?? null
the driver owns the codecdriver-sql/src/sql-driver.ts:14872-14891 (write), :14929-14938 (read)holds

The codec is dialect-asymmetric but round-trips to the raw value on both: SQLite stores primitives as-is and re-parses on read (JSON.parse('America/New_York') throws, so the string is kept); Postgres JSON.stringifys into the native jsonb column and the client parses it back. Either way a row a real driver hands back carries America/New_York — never "America/New_York". The fake engine here skips the codec entirely, so the fixture had to hold the decoded value already, and did not.

Route taken: 1 — re-encode the fixture (the fake is file-local)

The card left the route open and asked for the measurement. Measured:

  • makeEngine in this file is file-local. The file has zero export statements and no file imports from it — the only repo-wide reference is a comment in settings-loadrows-scope.test.ts noting it hand-copied the same matcher.
  • The sibling fixture in the same package already stores decoded valuessettings-loadrows-scope.test.ts:70,76 use value: 'America/New_York' / 'Asia/Tokyo' under the comment "Values are stored plain, as the persist path writes them."
  • Repo-wide, settings-getmany.test.ts was the only file spelling value: '"…"' in a settings fixture (3 hits, all in ROWS).

So Route 2 would teach a codec to a double that nobody else constructs, and would make it diverge from its own sibling copy in the same package. It is also the wrong shape on the merits: there is no single codec to teach, because the driver's behaviour is dialect-split while the observable contract at the engine boundary is "what you wrote is what you read" — which is exactly what the fake already does correctly. The defect was only ever in the fixture's input values, so that is what changed.

The acceptance criterion: a pin that asserts a literal resolved value

Landed as [#12172] resolves the LITERAL stored value — no doubled JSON encoding, asserting many.timezone.value === 'America/New_York' (plus locale/currency, plus the per-key get path so the pin holds the encoding and not a getMany quirk).

⚠️"The suite is still green" is not evidence here, and this PR does not lean on it. Every pre-existing assertion in the file compares getMany against get, so both sides moved together and were insensitive to the defect by construction. Measured across all three states — the 7 original tests were green in every one:

Fixture statePin7 original tests
JSON text (before), pin addedRED7 passed
decoded (after)GREEN7 passed
JSON text restored (dissolution)RED7 passed

Dissolution verification of the new pin

Mutation re-applied the JSON-text encoding, confirmed on disk before running (3 injected value: '" spellings, 0 remaining decoded spellings) and run under a trap … EXIT INT TERM restore:

AssertionError: expected '"America/New_York"' to be 'America/New_York' // Object.is equality
Expected: "America/New_York"
Received: ""America/New_York""
❯ src/settings-getmany.test.ts:203:33
Test Files 1 failed (1)
Tests 1 failed | 7 passed (8)

No rebuild is involved and none is owed: the mutated symbol is the test file's own ROWS constant, which vitest compiles from source on every run — no dist/ sits between the mutation and the assertion.

Restored with git checkout HEAD -- packages/services/service-settings/src/settings-getmany.test.ts, verified byte-identical, not merely "clean":

disk: 70cbbaa576da0d23341a47ed3cae3626870a17b9
HEAD: 70cbbaa576da0d23341a47ed3cae3626870a17b9

Second-order result: none

No existing assertion changed and none broke. Nothing in this PR was adjusted to match the re-encoded fixture — git diff -U0 | grep '^[+-].*expect' shows no removed assertion line across either commit.

The second commit rewrites one comment, which the fix had made factually false: it stated the assertion sits on the cascade layer because "this fixture stores JSON text in value" — the recorded #11680 workaround. Leaving a note asserting the defect as a standing property of the file is the precise mislead this card exists to remove. The reason changed; the assertions on those lines are byte-identical.

Verification — all at the shipped head a8c030c22

Exit codes captured before any pipe (redirect-then-read), and each gate quoted from its own verdict line:

  • @objectstack/service-settings suite — 29 files / 519 tests passed
  • @objectstack/service-settings typecheck (tsc --noEmit) — clean
  • Derived union, node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (self-derived change set, 1 path, merge base e5ce2ed03) — all green: check:cross-package-test-inputs, check:page-declaration-shape, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check-ci-filter-parity, check-cross-package-test-inputs, check-plugin-teardown-shape, check-affected-docs, check-drift-comment, plus the convention-triggered check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:where-matcher, and check:nul-bytes.

Ratchet families re-run on the shipped head, quoted:

check-engine-double-contract: OK — 415 pinned, 133 in the DEBT ledger, 2 exempt.
✓ where-matcher conformance holds: 301 matcher(s) discovered, 301 answer the
combinator battery correctly or refuse it loudly (188 refuse).
baseline key set verified against e5ce2ed: no files added.
✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) …, none new.
baseline key set verified against e5ce2ed: no files added.
OK: 18 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.

Two declared narrowings

  1. check:type-check-debt (--re-measure) not run locally. Its population is the DEBT ledger inside scripts/check-type-check-coverage.mjs; @objectstack/service-settings is absent from it (the sole textual match is a prose comment at line 508). Its structural half check:type-check-coverage ran green here, the package's own tsc --noEmit is clean, and this diff adds no file and no package — so no ledger entry's count can move. The --re-measure half needs a full built workspace closure, which CI performs on every PR.
  2. Repo-wide pnpm lint narrowed to the affected package, with the narrowing measured rather than assumed: ① the population came from eslint's own config resolution over packages/services/service-settings/src, not from a guess about which files count; ② the count came from --format json60 files, 0 errors, 0 warnings, with the changed file confirmed present in the linted set; ③ type-aware linting is not enabled in eslint.config.* (no projectService/project:), so no rule reads across file boundaries and a one-file diff cannot move the verdict of any untouched file.

Changeset

None — this is a test fixture and a test comment; nothing ships. Carries skip-changeset rather than an empty changeset, which stalls the release here.


Generated by Claude Code

…ure, and pin the literal
`settings-getmany.test.ts`'s `ROWS` fixture stored JSON TEXT in `value`
(e.g. '"America/New_York"'), but `sys_setting.value` is a `Field.json`
column and the service is verbatim in both directions: `setMany` writes
`storedValue = rawValue`, and `materialiseRow`'s non-encrypted branch is
`return row.value ?? null`. The DRIVER owns the JSON codec, and the fake
engine here skips it -- so every fixture value resolved one JSON encoding
deep.
It was invisible because it lied CONSISTENTLY: every assertion in the file
compares `getMany` against per-key `get`, and both sides are equally
affected. Measured -- with the fixture untouched, the new literal-value pin
fails `expected '"America/New_York"' to be 'America/New_York'` while the
other 7 tests stay green.
Re-encodes the fixture to the decoded values a real driver hands back
(matching the sibling fixture in `settings-loadrows-scope.test.ts`, which
already stores them plain), and lands the literal-value pin that holds the
encoding depth mechanically. No existing assertion changed.
…x obsoleted
The comment on the non-equivalence assertions recorded the WORKAROUND: it
said the assertion sits on the cascade layer because "this fixture stores
JSON text in `value`". The fixture no longer does, so the note asserted a
property of the file that had just stopped being true -- the exact
mislead this card exists to remove.
Rewrites the reason only. The assertions on those lines are byte-identical
(`git diff -U0 | grep '^[+-].*expect'` is empty across this commit).
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 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 — 0 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 e5ce2ed031d57d723cacb5b30177d101bb06a57fpackageMentionDocs.

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

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-trump@claude