Separate the JSON serialization probe from the schema DDL in PostgresKvStore

[heeoneie]

Summary

PostgresKvStore's initialized option skipped the driver JSON serialization
probe along with the schema DDL, so a store constructed with
initialized: true wrote every value into the JSONB column as a string rather
than an object. This pulls the option's two jobs apart: it now gates the
CREATE UNLOGGED TABLE statement only, and the probe runs on first use either
way.

This is the same defect as #1014, in the same package and from the same cause,
but it is the more damaging of the two and worth fixing on its own terms. The
queue's bad row consumes itself — it is dequeued, matches no task type, and is
deleted, so the damage is bounded to whatever was enqueued while the
misconfigured process ran. The key–value store's bad row stays in the table.
It remains a JSONB string, and every later read returns a string, including
reads from a process that never passed initialized: true. One misconfigured
writer therefore poisons entries that correctly configured readers go on to
consume, and nothing reports an error anywhere: get() returns successfully,
just with the wrong type. That breaks the KvStore contract, since get() no
longer returns the value set() was given, and Fedify keeps actor key pairs
and remote documents in a KvStore.

Targets 2.0-maintenance rather than main, per @dahlia's direction on the
issue: "We no longer support 1.x, so please target 2.0-maintenance for this
fix." (comment)

This is a fork PR, so the workflows land as action_required — they need a
maintainer to approve the run before CI can report.

Related issue

• fixes PostgresKvStore initialized option skips JSON serialization detection #1031

The PostgresMessageQueue half of the same defect was fixed separately in
#1032, which this branch is based on.

Reproduction

The script from the issue, run unchanged against a real PostgreSQL 16.14 with
postgres.js 3.4.8:

await sql`
  CREATE UNLOGGED TABLE IF NOT EXISTS ${sql(table)} (
    key text[] PRIMARY KEY,
    value jsonb NOT NULL,
    created timestamp with time zone DEFAULT CURRENT_TIMESTAMP,
    ttl interval
  );
`;

const store = new PostgresKvStore(sql, { tableName: table, initialized: true });
const value = { keyPair: { id: "https://example.com/actor#main-key" }, n: 1 };
await store.set(["cache", "a"], value);

const [row] = await sql`
  SELECT value, jsonb_typeof(value) AS json_type FROM ${sql(table)};
`;
console.log(row.json_type, typeof row.value, typeof await store.get(["cache", "a"]));

// A second store that never passes the option, reading the row written above:
const healthy = new PostgresKvStore(sql, { tableName: table });
console.log(typeof await healthy.get(["cache", "a"]));

Before this change every line prints string; after it, object. (The
issue's JSON.stringify(readBack) === JSON.stringify(value) line still prints
false after the fix because JSONB reorders object keys, but
deepStrictEqual(readBack, value) passes.)

Cause

initialize() does two unrelated things. It runs the CREATE UNLOGGED TABLE
statement, and then, at the very end, it sets #driverSerializesJson from the
driverSerializesJson() probe.

The constructor assigned options.initialized straight into #initialized,
which is the same flag initialize() checks before returning early. So
initialized: true did not skip the DDL — it skipped the whole method, probe
included. #driverSerializesJson then stayed at its false default, #json()
took the JSON.stringify() branch, and postgres.js sql.json() serialized the
result a second time.

Changes

• packages/postgres/src/kv.ts: split the constructor flag in two.
  options.initialized now lands in a new #skipDdl, while #initialized
  always starts false.
• packages/postgres/src/kv.ts: move the CREATE UNLOGGED TABLE statement
  into #initializeTable(), and call it from initialize() only when
  #skipDdl is unset. The probe and #initialized = true now run
  unconditionally. get(), set(), delete() and list() already await
  initialize() before touching the table, so the probe runs on first use
  without introducing a new lazy path, and no caller contract changes.
• packages/postgres/src/kv.ts: document that the option skips only the
  schema DDL.
• packages/postgres/src/kv.test.ts: two regression tests, one per half of
  the contract. There was no coverage of the initialized option in this
  file before.
• changes.d/postgres/kv-initialized-json-probe.md and CHANGES.md.

The shape deliberately mirrors #1032 so the two halves of this fix read the
same way.

Benefits

initialized: true stops corrupting stored values, which is the documented
behaviour it was always supposed to have: skip the schema DDL, keep get() and
set() identical to the default path. Deployments that manage their own
schema — the reason the option exists — no longer poison their own key–value
store for every other process that reads it.

Verification

Run against PostgreSQL 16.14 in Docker, with POSTGRES_URL set so the
Postgres-gated tests actually execute rather than skip.

The regression test fails without the patch. Restoring the old constructor
assignment reports exactly the symptom the issue describes:

AssertionError: initialized: true should still store the value as a JSONB object
'string' !== 'object'

Both halves are enforced. The second test guards the other direction: if
the #skipDdl gate is removed so the DDL always runs, it fails with Missing expected rejection. A fix that simply stopped honouring initialized would
not pass. Each mutation fails exactly one of the two tests.

The DDL really is still skipped. With log_statement = 'all', comparing
the statements the store issues between two marker queries:

statement	initialized: true	initialized: false
CREATE UNLOGGED TABLE	none	1
SELECT $1::jsonb (the probe)	1	1
INSERT / DELETE (set() and #expire())	1 each	1 each
resulting jsonb_typeof	object	object

Tests. @fedify/postgres across all three runtimes: Deno 27 passed / 0
failed, Node.js 26 passed / 0 failed, Bun 27 passed / 0 failed. mise run check passes, sacho check included.

mise run test does not come back clean here, and it does not on the base
commit either. This is the same @fedify/vocab-tools timeout described in
#1032 — generateClasses() imports the browser-safe jsonld entrypoint times
out only when Deno, Node.js, and Bun run the whole monorepo concurrently on
this machine. Nothing in this patch is in that package's dependency graph.

AI use

Claude Opus 5 assisted with the patch, the tests, and running the checks above;
the commit carries an Assisted-by trailer. Every number in this description
was measured on a real PostgreSQL instance rather than inferred from reading
the code.

Checklist

• Did you add a changelog entry to the CHANGES.md?
• Did you write some relevant docs about this change (if it's a new
  feature)? — not a new feature; the initialized option's TSDoc is updated
  to say what it skips.
• Did you write a regression test to reproduce the bug (if it's a bug
  fix)?
• Did you write some tests for this change (if it's a new feature)?
• Did you run mise test on your machine? — yes; it fails identically on
  this branch and on its base commit, see Verification.

Additional notes

• This does not repair rows already written. Values stored by a
  misconfigured process before the upgrade are already JSONB strings and will
  still read back as strings. Only new writes are affected.

  A read-side compatibility shim was considered and rejected: get() cannot
  distinguish a double-serialized object from a value that was legitimately
  stored as a string, so parsing on read would corrupt correct data.
  Affected rows are overwritten on the next set(), and cache-shaped keys
  clear themselves as their TTLs expire.

  Operators who suspect they hit this can find the affected rows with
  SELECT key FROM <table> WHERE jsonb_typeof(value) = 'string', though a
  legitimately stored string value looks the same there for the same reason,
  so the result needs judgement rather than a blind DELETE.

• One extra statement on the initialized: true path. The probe is a
  single SELECT $1::jsonb, run once per store instance on first use. That
  is the entire runtime cost of the fix.

• The DDL moved rather than changed; the diff reads more cleanly with
  whitespace hidden.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6e3b4063-386a-4905-9416-ec0f22d231b4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines	Coverage Δ
packages/postgres/src/kv.ts	97.02% <100.00%> (+0.12%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dahlia]

Thanks for your work!

[dahlia]

Your fix is shipped with Fedify 2.0.27, 2.1.23, 2.2.12, and 2.3.7.