Skip to content

Fix #867: loosen PT _key validation and fix template seeds#869

Merged
ascorbic merged 5 commits intomainfrom
opencode/issue867-20260430190847
May 1, 2026
Merged

Fix #867: loosen PT _key validation and fix template seeds#869
ascorbic merged 5 commits intomainfrom
opencode/issue867-20260430190847

Conversation

@ask-bonk
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot commented Apr 30, 2026

What does this PR do?

Fixes autosave validation failures on content scaffolded from the blog, portfolio, and starter templates. PR #777 wired the existing generateZodSchema() validator into the runtime content-update path, so autosave now revalidates the body the admin re-sends on every keystroke. Several first-party templates ship seed content that the schema rejected, so users couldn't save edits to seeded entries.

Two related issues, both fixed at the right layer:

  1. _key was strictly required on Portable Text blocks even though the rest of the block schema is .passthrough() and the editor regenerates _key on every change. Requiring it on input rejected legitimate seed/import data without protecting any real invariant. _key is now .optional() in the validator (packages/core/src/schema/zod-generator.ts).
  2. Non-required field columns are nullable in SQLite (SchemaRegistry.addFieldColumn adds them without NOT NULL), but the validator used .optional() which only accepts undefined. Any field that's actually null in the DB failed round-trip on autosave. Switched to .nullish().
  3. Template seeds shipped malformed data: PT blocks without _key, and portfolio's featured_image as bare URL strings (the schema validates image as z.object({id, ...})). Fixed the seed files directly and added a regression guard that runs every shipped template seed through validateContentData(..., { partial: true }) -- the same path the autosave endpoint uses.

The runtime normalizer from the original commits and its unit suite are dropped; the failure mode no longer exists at the data level. The integration test from those commits is repurposed as the regression guard above.

This is a re-roll of the original approach following the maintainer's audit (thread), which correctly identified that:

Closes #867.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes (diagnostic count unchanged from baseline; remaining diagnostics are pre-existing cross-package "error type" warnings)
  • pnpm test passes (or targeted tests for my change) -- seed, schema, and api tests all green; the 8 vite-config test failures and 25 file-load failures in the local run are environmental (Node 20 vs --run, unbuilt admin/auth deps), not from this change, and pre-exist on the branch tip
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). (N/A — no admin UI changes.)
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion. (N/A — bug fix.)

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 4.7 via /bonk

Screenshots / test output

$ pnpm vitest run tests/integration/seed-portable-text-keys.test.ts tests/unit/seed tests/unit/schema
✓ tests/integration/seed-portable-text-keys.test.ts (8 tests)
✓ tests/unit/schema/zod-generator.test.ts (20 tests)
✓ tests/unit/seed/validate.test.ts (51 tests)
✓ tests/unit/seed/skip-media-download.test.ts (4 tests)
✓ tests/unit/seed/apply.test.ts
✓ tests/unit/seed/media.test.ts (9 tests)

Test Files  9 passed (9)
     Tests  181 passed (181)

The new tests/integration/seed-portable-text-keys.test.ts runs the full seed → DB → validator round-trip for all 8 shipped template seeds (blog, blog-cloudflare, portfolio, portfolio-cloudflare, starter, starter-cloudflare, marketing, marketing-cloudflare) and asserts every stored entry survives the autosave validator. This is the regression guard the maintainer asked for.

opencode added 2 commits April 30, 2026 19:16
… autosave

Fixes #867. Templates whose seed.json omits `_key` on Portable Text
blocks would round-trip valid through initial render but fail
autosave validation: the Zod schema generated for portableText fields
requires `_key: z.string()` on every top-level block, and the admin's
autosave path re-sends the data it loaded unchanged.

applySeed now passes content data, section content, and widget content
through a normalizer that injects stable, deterministic keys on every
PT-shaped node missing one. Existing keys in the seed file are
preserved; generated keys avoid colliding with explicit ones.
@ask-bonk ask-bonk Bot mentioned this pull request Apr 30, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-i18n d9623ae May 01 2026, 06:15 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs d9623ae May 01 2026, 06:16 AM

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 30, 2026

🦋 Changeset detected

Latest commit: d9623ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
emdash Patch
create-emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/admin Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-playground d9623ae May 01 2026, 06:16 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-perf-coordinator d9623ae May 01 2026, 06:15 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-cache d9623ae May 01 2026, 06:16 AM

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
✅ (ascorbic)[https://github.com/ascorbic]
@OpenCode
opencode seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 30, 2026

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@869

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@869

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@869

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@869

emdash

npm i https://pkg.pr.new/emdash@869

create-emdash

npm i https://pkg.pr.new/create-emdash@869

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@869

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@869

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@869

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@869

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@869

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@869

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@869

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@869

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@869

commit: d9623ae

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes autosave validation failures for content seeded from templates by normalizing Portable Text nodes to include stable _key values at seed-ingestion time (keeping runtime/admin validators strict).

Changes:

  • Added normalizePortableTextKeys() helper that walks seed payloads and injects missing _key values on PT-shaped nodes while preserving existing keys and avoiding collisions.
  • Wired PT key normalization into seed application for content entries, sections, and widget content.
  • Added unit + integration regression tests and a patch changeset.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/core/src/seed/normalize-portable-text.ts Implements deterministic, collision-avoiding _key injection for PT-shaped nodes during seeding.
packages/core/src/seed/apply.ts Applies the normalizer after $ref/$media resolution for content, and before persisting sections/widgets.
packages/core/tests/unit/seed/normalize-portable-text.test.ts Unit coverage for immutability, recursion, preservation, and collision-avoidance behavior.
packages/core/tests/integration/seed-portable-text-keys.test.ts End-to-end regression ensuring seeded PT survives the same validator used by autosave.
.changeset/seed-portable-text-keys.md Patch release note describing the seeded Portable Text normalization fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ascorbic
Copy link
Copy Markdown
Collaborator

@ask-bonk pause this — it's a regression of #777, not a templates bug, and the fix is at the wrong layer. Audit before re-rolling.

What changed: validateContentData and the Zod check on the content update path landed 4 days ago in #777 (emdash-runtime.ts:1843). The _key: z.string() requirement in zod-generator.ts:108 predates that — it just wasn't enforced on autosave until last week. Pre-#777 all four templates saved fine. #867 was filed 4 days after #777.

Full audit (seed each template with no normalizer, then re-validate every stored entry against validateContentData(..., {partial: true})):

Template Failing entries Failure shapes
blog 9 _key × 8, featured_image: null × 1
portfolio 5 _key × 5, featured_image: <bare URL string> × 4
starter 2 _key × 2, featured_image: null × 1
marketing 0 clean

The _key story is wider than the issue described, and there's a second regression you missed: portfolio ships featured_image as bare URL strings (e.g. "https://images.unsplash.com/..."), which z.object({id, ...}) rejects. The reporter masked it by uploading a real image before the first autosave; anyone who edits a different field first hits featured_image: expected object, received string.

Drop the seed-time normalizer. It only helps fresh installs (setup applies seed once with onConflict: "skip"), doesn't touch existing scaffolds, doesn't address featured_image, and frames a regression as a data hygiene problem.

Do this instead:

  1. Make _key optional in zod-generator.ts:108. One line: _key: z.string().optional(). Restores pre-fix(mcp): bug-fix wave + new tools to fill capability gaps #777 behavior, matches the .passthrough() already in place for spans/markDefs, and is consistent with _key being a UI concern that the editor regenerates on every edit (PortableTextEditor.tsx:203).
  2. Fix the seed files directly. Add _keys to PT blocks in templates/blog, templates/portfolio, templates/starter. Replace bare-URL featured_image values in portfolio seeds with \$media references (or drop them). This is the actual broken data — fixing it is cheaper than the normalizer plus its 314 lines of tests.
  3. If you want strict-_key long-term, do it as a separate hardening PR with: lazy normalization on the content GET handler (self-heals existing installs on first edit, no migration needed), full audit of every other invariant fix(mcp): bug-fix wave + new tools to fill capability gaps #777 newly enforces, and a real plan. Not in this PR.

Tests: keep normalize-portable-text.test.ts only if you keep the helper; otherwise delete both test files. The integration test you wrote is good shape — convert it to "every shipped template seed survives the autosave validator" once the seeds are fixed. That's the regression guard worth keeping.

Also: rebase. Branch is missing #868.

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented Apr 30, 2026

Pausing as requested. I re-audited every claim before responding; you're right on all the verifiable points. Posting findings rather than re-rolling code in this run so we agree on direction first.

What I verified

1. This is a regression of #777, not a templates bug.

PR #777's body: "Validation moved to the runtime. EmDashRuntime.handleContentCreate / handleContentUpdate validate against the collection schema before any write." Confirmed in emdash-runtime.ts:1842-1851 (the update path you cited) and :1755-1762 (create path). Pre-#777 the Zod check existed in zod-generator.ts:108 but wasn't invoked by autosave. Filed #867 timeline: #777 merged 2026-04-26, #867 filed 4 days later. Reporter's stack trace points at the validator inside handleContentUpdate. The diagnosis is correct.

2. The _key-required schema is internally inconsistent with the rest of the validator.

zod-generator.ts:104-111 is z.object({ _type, _key }).passthrough(). Spans, markDefs, and every other PT-internal shape go through .passthrough() — they aren't validated at all. Singling out _key for strict checking on top-level blocks while passing through everything below isn't a coherent invariant; it's an artifact. And PortableTextEditor.tsx:203,221,246,... regenerates _key on every block during the prosemirror→PT conversion, so even with strict validation a malformed _key from a client never reaches the DB through the editor path. The "loosen the validator" path doesn't weaken any real invariant.

3. The featured_image regression is real and this PR doesn't address it.

Portfolio seed audit (templates/portfolio/seed/seed.json):

projects/meridian-brand:  featured_image[str] = "https://images.unsplash.com/photo-1558618666..."
projects/volta-web:       featured_image[str] = "https://images.unsplash.com/photo-1497366216..."
projects/archive-print:   featured_image[str] = "https://images.unsplash.com/photo-1507003211..."
projects/coastal-photo:   featured_image[str] = "https://images.unsplash.com/photo-1505142468..."

zod-generator.ts:113-120 defines image as z.object({id, src?, alt?, width?, height?}) — no .passthrough(), no string variant. Bare URL strings will fail with expected object, received string the moment a portfolio user edits any field. Blog/starter use $media references correctly; only portfolio has this. Marketing has no image fields at all, which matches your "marketing 0 failing entries" line.

I couldn't reproduce your featured_image: null × 1 counts on blog/starter — grep -c '"featured_image": null' returns 0 across all four templates on this branch. Either I'm grepping a different snapshot than your audit hit (post-#868, maybe?), or those came from a different field path. The directional finding stands either way: the PR ignores the entire image-field shape problem.

4. Seed-time normalizer is the wrong layer.

Confirmed in apply.ts: the four call sites all run inside applySeed(), which is invoked from setup (onConflict: "skip") or explicit re-seed. Anyone who ran setup before this PR ships has keyless data already in their DB; this normalizer never sees that data. The fix-at-the-seed-boundary framing doesn't fit the failure mode. The integration test in this PR validates a fresh seed install and passes — but a fresh-install repro isn't actually the bug from #867; the reporter scaffolded the template then edited it.

5. Branch is missing #868.

fbed118 chore(scripts): commit perf query-dump tooling (#868) is on origin/main but not in this branch. Confirmed via git log origin/main.

On scope discipline (where I disagree slightly)

The PR body's "out of scope: actual seed.json files still ship without _key" framing was wrong on my end — once you're already touching the seed write path because seeds are broken, fixing the broken seed files is the in-scope change, not a follow-up. The normalizer is the drive-by here. I should have caught that before opening.

Proposed re-roll

Smaller PR, in this order:

  1. Loosen the validator. _key: z.string().optional() in zod-generator.ts:108. Keep top-level _type required; everything else stays as-is (passthrough already covers it). Restores pre-fix(mcp): bug-fix wave + new tools to fill capability gaps #777 behavior. One-line change.
  2. Fix the seeds.
    • Add stable _key values to every PT block in templates/blog/seed/seed.json, templates/portfolio/seed/seed.json, templates/starter/seed/seed.json, plus the matching *-cloudflare variants. The data-at-rest is broken; fix the data.
    • Replace the four bare-URL featured_image values in templates/portfolio/seed/seed.json (and portfolio-cloudflare) with $media references in the same shape blog uses.
  3. Test as a regression guard. Convert seed-portable-text-keys.test.ts into "every shipped template seed survives validateContentData(..., {partial: true}) for every entry it produces". That's the test that would have caught both Auto save validation errors #867 and the featured_image issue, and the one we want failing if a template ships malformed seed data again.
  4. Drop the normalizer + its unit suite. No normalize-portable-text.ts, no normalize-portable-text.test.ts. The integration test from step 3 replaces both.
  5. Rebase onto current main to pick up chore(scripts): commit perf query-dump tooling #868.

Strict-_key long-term, if we still want it, is a separate hardening PR with: lazy normalization in the content GET handler so existing installs self-heal on first read; a deliberate audit of every other invariant #777 newly enforces against existing seed data; and a real plan. Not in this PR.

Awaiting confirmation

Want me to proceed with that plan? If yes, I'll close this branch's commits with a fresh re-roll on top of main (post-#868). If you'd rather see a different shape — e.g. keep the normalizer as a defensive-depth net behind the validator change — say so and I'll adjust.

For full transparency: my original investigation didn't catch the featured_image bug because I followed the issue reporter's exact repro path, which masks it (the reporter uploaded an image before the first autosave). Your "edit a different field first" path is the one that surfaces it. I'll fold that into the integration test in the re-roll so we don't drop it again.

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented Apr 30, 2026

Posted. Pausing as requested — no code changes in this run. The maintainer's audit was correct on every verifiable point (the regression came from #777, the _key-required schema is internally inconsistent with the surrounding .passthrough(), the seed-time fix doesn't help existing scaffolds, the portfolio template ships 4 bare-URL featured_image values that this PR ignores, and the branch is missing #868). One small honest correction: I couldn't reproduce the featured_image: null × 1 counts on blog/starter from their audit — flagged that in the comment without dismissing the directional finding. Awaiting confirmation before re-rolling.

github run

@ascorbic
Copy link
Copy Markdown
Collaborator

@ask-bonk implement it as suggested

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented Apr 30, 2026

To https://github.com/emdash-cms/emdash.git
! [rejected] opencode/issue867-20260430190847 -> opencode/issue867-20260430190847 (non-fast-forward)
error: failed to push some refs to 'https://github.com/emdash-cms/emdash.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. If you want to integrate the remote changes,
hint: use 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

github run

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented Apr 30, 2026

@ascorbic Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

PR template validation failed

Please fix the following issues by editing your PR description:

  • This PR does not use the required PR template. Please edit the description to use the PR template. Copy it into your PR description and fill out all sections.

See CONTRIBUTING.md for the full contribution policy.

@ascorbic
Copy link
Copy Markdown
Collaborator

ascorbic commented May 1, 2026

@ask-bonk try again

@ascorbic
Copy link
Copy Markdown
Collaborator

ascorbic commented May 1, 2026

and make sure you update the PR title and description

Replaces the seed-time normalizer in the previous commits with the
two-layer fix the maintainer's audit landed on:

  1. zod-generator.ts: `_key` on Portable Text blocks is now
     `.optional()`, and non-required fields use `.nullish()` instead
     of `.optional()` so the validator accepts the `null`s the DB
     stores for unset columns. Both regressions trace back to PR #777
     wiring the existing schema into the runtime content-update path
     (autosave) without auditing whether stored data round-trips.

  2. Template seeds: every shipped `seed.json` (blog, portfolio,
     starter, marketing, plus the four `*-cloudflare` variants) now
     ships stable `_key` values on every Portable Text block / span
     / mark def. The portfolio template's bare-URL `featured_image`
     values are rewritten as `$media` references in the same shape
     the blog template already uses (the validator's `image`
     schema is `z.object({id, ...})`, so bare strings were a second
     latent autosave failure the original PR didn't address).

Drops the runtime normalizer and its unit suite -- the failure mode is
gone at the data level. The integration test that previously exercised
the normalizer is repurposed as a regression guard: every shipped
template seed is applied to a fresh DB and every stored entry is
re-validated against `validateContentData(..., { partial: true })`,
which is exactly what the autosave endpoint runs. Template changes
that break the contract fail before release.

Closes #867.
@ask-bonk ask-bonk Bot changed the title Fix #867: inject _key on seeded PT blocks Fix #867: loosen PT _key validation and fix template seeds May 1, 2026
@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented May 1, 2026

Re-rolled per the audit. Summary of what changed in this push (commit d9623ae):

Validator (packages/core/src/schema/zod-generator.ts)

  • _key: z.string()_key: z.string().optional() on Portable Text blocks. Restores pre-fix(mcp): bug-fix wave + new tools to fill capability gaps #777 behavior; matches the surrounding .passthrough() and the editor's regenerate-on-edit semantics.
  • Non-required fields: .optional().nullish(). addFieldColumn makes those columns nullable, so the DB stores real nulls for unset values; .optional() only accepts undefined and broke autosave round-trip on any entry where any non-required field was unset. This is what was producing the featured_image: expected object, received null errors on blog/starter posts that didn't set a cover image.

Templates (all 8 shipped seed.jsons)

  • Stable _keys on every PT block / span / mark def. Per-entry deterministic counter (k0, k1, ...), so the same input produces the same output and diffs stay local.
  • Portfolio's bare-URL featured_image values rewritten as $media references in the same shape templates/blog already uses (4 entries × 2 variants = 8 fixes).

Tests / dead code

  • Dropped packages/core/src/seed/normalize-portable-text.ts and packages/core/tests/unit/seed/normalize-portable-text.test.ts. The failure mode is gone at the data level; the runtime normalizer doesn't help existing scaffolds.
  • packages/core/tests/integration/seed-portable-text-keys.test.ts is now the regression guard you asked for: it walks every shipped template seed, applies it to a fresh DB, and re-validates every stored entry through validateContentData(..., { partial: true }) — exactly what the autosave endpoint runs. 8 tests, one per template.

Verification (Node 20 environment, so admin/auth deps don't build cleanly here):

  • pnpm vitest run tests/integration/seed-portable-text-keys.test.ts tests/unit/seed tests/unit/schema tests/unit/api tests/integration/api → 389 passed, 0 failed.
  • pnpm typecheck (after building @emdash-cms/gutenberg-to-portable-text) clean.
  • pnpm --silent lint:json diagnostic count unchanged from baseline.
  • pnpm format applied.

Branch state: rebased through 0bc301d and 92d4045 merge commits in earlier iterations; HEAD is now d9623ae on top of the merged main, so #868 is included.

PR title and description updated to use the required template; AI disclosure box ticked.

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented May 1, 2026

Re-rolled the PR per the audit. Summary:

At the validator (packages/core/src/schema/zod-generator.ts):

  • _key: z.string()_key: z.string().optional() on PT blocks (the one-line fix the audit identified)
  • Non-required fields use .nullish() instead of .optional() — the DB stores real nulls for unset columns, and .optional() only accepts undefined, which is what produced the featured_image: expected object, received null failure mode on blog/starter entries without a cover image (exactly the second regression you flagged)

At the data (all 8 template seed.json files):

  • Stable per-entry _keys on every PT block / span / mark def (deterministic k0, k1, ... counter so diffs stay local)
  • Portfolio's 4 bare-URL featured_image values rewritten as $media references in the same shape blog uses

Tests / dead code:

  • Dropped the runtime normalizer and its unit suite
  • Repurposed the integration test as a regression guard — runs every shipped template seed through validateContentData(..., { partial: true }) (the autosave path). 8 tests, one per template, all green

PR meta:

  • Title and description rewritten to use the required PR template
  • AI disclosure box ticked, model named (Claude Opus 4.7 via /bonk)
  • Changeset updated to reflect the new shape

Verification on this Node 20 runner (admin/auth deps don't build because they need node --run from Node 22):

  • All 8 template seeds pass the new regression guard; targeted seed/schema/api suites: 389 passed, 0 failed
  • pnpm typecheck clean
  • pnpm --silent lint:json diagnostic count unchanged from baseline
  • pnpm format applied

github run

@ascorbic
Copy link
Copy Markdown
Collaborator

ascorbic commented May 1, 2026

/review

@ask-bonk
Copy link
Copy Markdown
Contributor Author

ask-bonk Bot commented May 1, 2026

OK, 20 matches the description. None of them specifically test _key being optional or .nullish() accepting null. The behavior is implicitly verified by the integration test, but there's no unit test for the regression — so if someone changes _key: z.string().optional() back to .string(), only the integration test catches it. That's a test coverage observation, not a bug.

Summary of my findings:

  • The validator changes are correct and well-reasoned.
  • The seed fixes (adding _keys, replacing bare URLs with $media) are correct.
  • The integration regression test is well-designed and covers the actual autosave path.
  • The .nullish() change has one subtle interaction with .default(): null input now passes through instead of triggering the default. This doesn't cause data loss but is a minor behavioral change. It's not documented in the comment.
  • No unit-level test for _key being optional or .nullish() accepting null — but the integration test does cover this.

These are not bugs. The PR is solid.

LGTM!

github run

@ascorbic ascorbic merged commit a8bac5d into main May 1, 2026
35 of 36 checks passed
@ascorbic ascorbic deleted the opencode/issue867-20260430190847 branch May 1, 2026 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto save validation errors

2 participants