fix(create-emdash): remove manual bootstrap step from CLI output - #12
Merged
Conversation
Merged
travisbreaks
pushed a commit
to travisbreaks/emdash
that referenced
this pull request
Apr 5, 2026
…-step fix(create-emdash): remove manual bootstrap step from CLI output
ascorbic
added a commit
that referenced
this pull request
Apr 26, 2026
* test(mcp): add integration coverage for documented bugs Adds 73 MCP integration tests across 8 files exercising the real MCP client/server pair against a real database, with no handler mocks. 43 tests fail today against bugs documented in the local MCP_BUGS.md log (#1, #2, #3, #4, #5, #6, #10, #11, #12) and will pass when the omnibus fix lands. Each failing test maps to a specific bug variant; the passing tests serve as regression guards. Refactors EmDashRuntime's 19-arg private positional constructor to a public single-argument constructor that takes an EmDashRuntimeParts object. Production code path is preserved (create() builds the parts and ends with new EmDashRuntime({ ... })); tests can now construct a real runtime around a pre-migrated test database without duplicating any handler logic. Adds an HTTP-level concurrency check to the smoke matrix to cover the parallel-401 race (the InMemoryTransport used by the integration suite cannot reach the production auth middleware path). * test(mcp): add comprehensive coverage for all MCP tools and edges Extends MCP integration coverage to every registered tool, plus the known gaps and edge cases. Adds 145 tests across 7 new files (218 total across the suite, 58 currently failing). New files: - schema.test.ts (46): list/get/create/delete collection, create/delete field, with validation, ownership, error envelope, and side-effect coverage. All passing — schema tools are largely correct. - taxonomy.test.ts (20): list/list_terms/create_term plus bug #7 (orphan collection drift) and bug #13 (no delete/update term gap). - media.test.ts (22): list/get/update/delete with ownership, mimeType filter, pagination, and bug #14 gap (no upload tool). Confirms media ownership extraction already handles null authorId correctly, pinning down the inconsistency with content extraction (bug #1). - menu.test.ts (10): read-only list/get plus bug #15 gap (no mutation tools). - search.test.ts (9): empty index, no-match, collection scoping, special-character handling, draft filtering, permissions. - content-misc.test.ts (25): content_duplicate, content_permanent_delete, content_translations + locale handling on get/create, _rev optimistic concurrency (happy + race), soft-delete visibility, edit-while-trashed, idempotency for publish/unpublish/schedule, and the content_unschedule MCP-tool gap. - input-schemas.test.ts (13): Zod-level argument validation across every tool — missing required, wrong type, out-of-range, enum violation. All passing — pins down the SDK boundary. Failure breakdown by bug: - bug #1 (null authorId): 10 in ownership.test.ts - bug #2 (stale draft data): 4 in drafts.test.ts - bug #3 (bare error strings): 6 in errors.test.ts + scattered - bug #4/#5/#6 (validation): 11 in validation.test.ts - bug #10 (publishedAt): 3 in lifecycle.test.ts - bug #11 (supports default): 2 in lifecycle.test.ts - bug #12 (cursor): 7 in pagination.test.ts - bug #7 + #13 (taxonomy): 8 in taxonomy.test.ts - bug #14 (media upload gap): 1 in media.test.ts - bug #15 (menu mutation gap): 3 in menu.test.ts - search FTS issues: 2 in search.test.ts - content_unschedule gap: 1 in content-misc.test.ts The remaining 58 failures collectively define the omnibus fix's acceptance criteria — turning these green will resolve every documented issue plus the gaps and edge cases this expansion surfaced. * fix(mcp): preserve error codes through tool response envelope The MCP server's `unwrap()` and `errorResult()` helpers stripped the structured error code from handler results, leaving callers with only a human-readable message. Tools that throw raw errors (schema, search, taxonomy, menu) similarly lost any signal beyond `error.message`. Replaces both helpers with three structured emitters: - respondData(data): success envelope - respondError(code, message, details?): emits the code as a stable `[CODE]` prefix on the message text AND attaches `_meta.code` so MCP-aware clients can read it programmatically - respondHandlerError(error, fallbackCode): for the catch-all sites that wrap raw thrown errors. Recognises the `apiError: { code }` annotation that handlers attach to thrown NOT_FOUND / CONFLICT errors (see api/handlers/content.ts:538), preserves the original message, falls back to `INTERNAL_ERROR` otherwise. `unwrap()` now propagates `error.code`, `error.message`, and `error.details` verbatim from the handler's ApiResult shape. Every call site of the old `errorResult()` is migrated: - ApiResult-returning paths use unwrap (already used everywhere) - SchemaRegistry / search / taxonomy / menu try/catch sites use respondHandlerError with tool-specific fallback codes (SCHEMA_LIST_ERROR, FIELD_CREATE_ERROR, SEARCH_ERROR, etc.) - String-error sites ("Collection 'X' not found", "Menu 'X' not found", "Taxonomy 'X' not found") use respondError("NOT_FOUND", ...) - Revision restore "missing collection or entry reference" uses respondError("VALIDATION_ERROR", ...) This is the A1 commit from .opencode/notes/MCP_FIX_PLAN.md. Flips the `_meta.code` propagation test in tests/integration/mcp/errors.test.ts from red to green. The remaining red tests in that file need handler-level work (Wave 2) that surfaces the underlying repository error instead of the current generic "Failed to create content" / "Failed to list content" placeholders. Backwards compatibility: existing MCP clients that match on the bare message will see a `[CODE] ` prefix added. The prefix is on a stable SCREAMING_SNAKE_CASE token, so regex matchers and substring tests on the original message continue to work. * fix(mcp): allow ownership checks on rows with null authorId The MCP server's extractContentAuthorId() previously threw an InternalError when content had no authorId — typically seed-imported rows. This blocked every mutating operation (update, delete, publish, unpublish, schedule, restore) for those rows, including for admins. The companion change in this commit's parent landed in mcp/server.ts: extractContentAuthorId() now returns "" instead of throwing, mirroring how media_update/media_delete already handle null authorId. The ownership check then defers to canActOnOwn(): an actor with the '*:edit_any' permission succeeds, and an actor with only '*:edit_own' gets a clean permission error rather than an internal one. Updates the unit test that previously asserted the throw was correct. The new tests confirm an ADMIN can edit null-authorId rows and an AUTHOR is denied with a permission message (not an internal one). * fix(content): clear publishedAt when content is unpublished ContentRepository.unpublish() previously left published_at populated when flipping status to draft, making 'currently published' indistinguishable from 'previously published' via the data alone. Clear published_at on unpublish so a missing/null timestamp unambiguously means the item is not live. Re-publishing assigns a fresh timestamp via the existing publish path. * fix(mcp): apply documented supports default in schema_create_collection Adds the changeset for this fix. The source change — defaulting `args.supports ?? ['drafts', 'revisions']` at the MCP boundary so the documented default is actually applied — landed alongside the error envelope refactor in 9986405 due to working-tree interleaving between parallel agents. Tracking it as a separate changeset so the user-visible behavior change is described independently. * fix(content,pagination,taxonomies): wave 2 omnibus fixes Combines the three Wave 2 fixes from .opencode/notes/MCP_FIX_PLAN.md because they entangled in the working tree during parallel agent execution. B (validation, bugs #4/#5/#6): wires generateZodSchema() into handleContentCreate / handleContentUpdate. Required fields enforced; select / multiSelect option lists honored; reference fields verified to point at a real, non-trashed target. New helper at api/handlers/validation.ts. Errors surface as { code: VALIDATION_ERROR, message } with all offending fields named in one message so callers can fix everything in a single round trip. C1 (cursor decoding, bug #12): decodeCursor() now throws InvalidCursorError on bad input instead of silently returning null. Updates every repository caller (content, byline, audit, redirect, comment, plugin-storage, media, user, sections, loader). API handler catch blocks recognise the error and return INVALID_CURSOR. The MCP boundary rejects empty-string cursors via z.string().min(1) on the input schemas. taxonomy_list_terms gets parallel treatment for its in-memory term-id cursor. G1 (taxonomy orphan filter, bug #7): handleTaxonomyList filters each taxonomy's collections array against the real collection set on read. Storage stays untouched so re-creating a deleted collection re-links automatically. Test fix-ups in the same commit (these are tests I committed earlier on this branch that didn't account for production behavior): - taxonomy.test.ts: account for the seeded 'category' and 'tag' taxonomies that migration 006 inserts; reshape bug-#7 test around the seed's 'posts' orphan rather than a manufactured one; correct taxonomy_create_term response envelope ({ term: { ... } }). * fix(mcp): wave 3+4 — error fidelity, gap-feature tools, search and draft fixes start - Handler-level error fidelity (bug #3): handleContentList and handleContentCreate / handleContentUpdate now translate database-level errors into structured codes (COLLECTION_NOT_FOUND, SLUG_CONFLICT, UNKNOWN_FIELD, VALIDATION_ERROR) instead of collapsing to a generic 'Failed to ...' message. - validation.ts: detects unknown keys in content data so callers get a useful error rather than silently dropped writes. - handleTermCreate: validates parentId exists and belongs to the same taxonomy. - New MCP tools (gap fills): - content_unschedule (bug — runtime had handleContentUnschedule but no MCP tool) - taxonomy_update_term, taxonomy_delete_term (bug #13) - menu_create, menu_update, menu_delete, menu_set_items (bug #15) - media_create (bug #14 — registers metadata for files uploaded out-of-band; binary uploads remain off the MCP transport) - Test fixups: content-handlers slug test no longer expects a non-string title to silently produce null slug (validation now rejects that). loader-cursor-pagination expects the loader's error envelope rather than a thrown error. - Taxonomy tests adjusted to account for the seed data inserted by migration 006 ('category' and 'tag'). * fix(content,search): draft data hydration and FTS robustness - EmDashRuntime.handleContentGet / handleContentUpdate / handleContentGetIncludingTrashed now overlay the current draft revision's data onto the response when draftRevisionId is set, with the previously-published values exposed alongside as 'liveData'. Resolves bug #2 — agents calling content_get after content_update on a revision-supporting collection now see their pending draft, not the stale live row. - searchSingleCollection swallows FTS5 syntax errors and returns no matches rather than leaking the SQLite error text to callers. Malformed user-typed queries (unbalanced quotes, stray operators, unclosed parens) produce empty results instead of a 500-style error. - Search test setup activates the FTS index via FTSManager.enableSearch(). Without it, the FTS table and triggers don't exist and the search-after-publish round trip silently produces no matches — same trap a real site would hit if the admin never enabled search. - Minor cleanup: deduplicate inline regex with COLLECTION_SLUG_PATTERN in mcp/server.ts; safer typeof-narrowed cast in hydrateDraftData. * fix(mcp): production correctness wave (F1-F6) F1: media_create now forwards authorId from MCP user to repository so ownership checks succeed on subsequent media_update/delete by the same author. F2: taxonomy_update_term gains parent validation (existence, same taxonomy, self-parent rejection) and cycle detection via parent-chain walk (bounded at 100). Shared helper used by create + update. F3: empty-string parentId/slug normalized to undefined/null at handler and repository layers; previously '' was persisted as a literal slug. F4: content validation runs in EmDashRuntime.handleContentUpdate BEFORE the draft revision write, so revision-supporting collections can no longer slip invalid data into history. The downstream API handler still validates as defense-in-depth. F5: validation.ts reference-target catch is narrowed to isMissingTableError; any other DB error now propagates instead of being silently reported as 'reference not found'. IN clause is also chunked at SQL_BATCH_SIZE for D1 bind-parameter safety. F6: menu_delete (REST + MCP) now deletes _emdash_menu_items first inside a transaction. D1 has FOREIGN KEYS off by default so cascade was a no-op there. Description updated to drop the misleading FK-cascade claim. taxonomy_delete_term description corrected to match the handler's 'cannot delete with children' behavior. Tests: media gap test replaced with full happy-path (AUTHOR creates, gets, updates own; OTHER_AUTHOR is denied). Taxonomy gap presence-only tests replaced with rename/relabel/reparent/detach/cross-taxonomy rejection/self-parent rejection/cycle rejection. Menu gap presence-only tests replaced with create+get round-trip, duplicate CONFLICT, update label, set_items empty, set_items 3-level nesting, set_items parentIndex>=i rejection, F6 delete-cascade verification. 2806 -> 2821 tests passing. * fix(mcp): error envelope codes for SchemaError + auth (F7) respondHandlerError now reads error.code from any Error subclass with a string code field — SchemaError ('RESERVED_SLUG', 'COLLECTION_EXISTS', etc.) and the new EmDashAuthError class flow through with their stable codes intact. Numeric codes (e.g. McpError) are skipped — _meta.code is reserved for stable string codes. Auth helpers (requireScope, requireRole, requireOwnership, requireDraftAccess, the inline publish-perm check) now throw EmDashAuthError with stable codes (INSUFFICIENT_SCOPE, INSUFFICIENT_PERMISSIONS) rather than McpError. The SDK's text-only fallback envelope was stripping the code. To make those throws survive the SDK's catch path, registerTool is locally monkey-patched at server-creation time so every tool callback is wrapped in try/catch that funnels into respondHandlerError — without requiring 41 callbacks to grow individual try/catch blocks. Tests: 3 new F7 cases verify _meta.code === 'INSUFFICIENT_SCOPE' / 'INSUFFICIENT_PERMISSIONS' / SchemaError code (non-fallback) instead of the SDK's previous code-less text envelope. 2821 -> 2824 tests passing. * test(mcp): tighten regex assertions, add happy paths, liveData type (F8-F13) F8: replace broken /not.found/ regex (the . was a metacharacter matching any byte) with a tight /\bNOT_FOUND\b|\bnot found\b/i. Applied across errors / schema / content-misc / menu / taxonomy / media / validation test files. F9: drop the id-alternation in echo regexes — /(NOT_FOUND|01NEVEREXISTED)/ passed if the id alone was echoed, defeating the test. Each NOT_FOUND test now asserts the code/word AND, separately via expect.toContain, the id (when the message body actually echoes it). F10: tighten overly broad collection/field 'word match' assertions to the specific code (COLLECTION_NOT_FOUND / FIELD_NOT_FOUND) plus an expect.toContain on the offending slug. F11: replaced empty-conditional dead branches with positive post-condition assertions: - publish twice / unpublish on draft now check status in the success branch as well as the error branch. - pagination.test 'limit beyond max' now also checks the error message in the rejection branch. - search 'special chars' now checks items is an array in the sanitized-success branch. - search 'scope filter' now also asserts items.length > 0 so the for- loop is reachable. - pagination 'revision_list malformed cursor' was a no-op (expect(result).toBeDefined()) — deleted, with a note explaining revision_list has no cursor parameter. F12: replaced presence-only listTools tests with happy-path coverage: - content_unschedule: schedule + unschedule + verify scheduledAt is null + re-publish succeeds. - (taxonomy/menu/media gap-feature happy paths landed in the previous commit alongside F2/F3/F6.) F13: ContentItem now declares an optional liveData?: Record<string, unknown> field, with JSDoc explaining the contract: populated by hydrateDraftData when a draft revision exists, otherwise undefined. New drafts.test asserts both branches. 2824 -> 2826 tests passing. * fix(mcp): F14-F18 + F23/F24 — supports default, ownership edge, menu refactor, hydration cleanup F14: SchemaRegistry.createCollection now defaults supports to ['drafts', 'revisions'] when undefined. MCP and admin UI defaults removed in favor of the canonical lower-layer default. F17: canActOnOwn treats empty-string ownerId as 'no recorded owner' so authorId-stripping doesn't accidentally grant edit-own to an empty-id user. Test added for the edge. F18: Menu MCP tools delegate to typed handlers in api/handlers/menus.ts. New handleMenuSetItems handler exposes the atomic-replace logic for both REST (future) and MCP. Drops the as-never casts in MCP layer. F23: hydrateDraftData clones the response instead of mutating in place. Future request-cache layers won't observe stale-after-mutation bugs. F24: Strip leading-underscore keys from revision data before merging into the response. _slug etc. are runtime-internal and don't belong on the surfaced data field. F15: hydrateDraftData runs BEFORE afterSave hook so plugins see the just-saved draft data, not the live columns, for revision-supporting collections. F16: hydration error path logs '[emdash] draft hydration failed:' instead of swallowing silently. * fix(content): F19 remove dead unknown-field catch; F25 tighten conflict matchers Validation now rejects unknown keys upfront with VALIDATION_ERROR so the post-INSERT 'no such column' / 'column does not exist' catch branches in handleContentCreate and handleContentUpdate were unreachable. Drop them. Tighten the unique-constraint detection to match 'unique constraint failed' or 'duplicate key' specifically, instead of bare 'unique' (which false-positives on any error message containing the word) or 'constraint failed' (which catches NOT NULL and CHECK violations too). Make the create and update paths symmetric: both produce sanitized SLUG_CONFLICT messages and a generic 'Unique constraint violation' for non-slug uniques, so we don't leak raw DB error text in either path. * fix(auth,mcp,search): F20 menu/taxonomy scopes; F21 tighten fts5 swallow F20: add menus:manage and taxonomies:manage to the API token scope vocabulary (mapped to EDITOR via SCOPE_MIN_ROLE). Switch the seven MCP mutation tools (taxonomy_create_term, taxonomy_update_term, taxonomy_delete_term, menu_create, menu_update, menu_delete, menu_set_items) from content:write to the dedicated scope. REST already uses these names as permissions; bringing scopes in line lets a token hold menu/taxonomy management without granting general content writes. F21: extract isFts5SyntaxError() helper that matches specifically on 'fts5: syntax error' and 'unknown special query' rather than the broad 'fts5' / 'syntax error' string. The old filter would swallow internal table-corruption errors. Apply the helper to both searchSingleCollection and getSuggestions, and log the swallow at warn level so silent failures show up in the logs. * fix(types,mcp,tests): F22 cursor DoS guard; F26 drop harness cast; lint cleanups F22: reject cursors longer than 4096 chars in decodeCursor() before calling decodeBase64() (which is O(N) in input size). Also clamp the MCP and REST input schemas at 2048 chars so oversized cursors fail early at the schema boundary instead of allocating against giant strings inside the repository helper. New pagination test exercises the schema-boundary rejection. F26: drop the 'as unknown as EmDashHandlers' cast in the test harness so real interface drift surfaces instead of being hidden. Add the optional ensureSearchHealthy member to EmDashHandlers — it's a real public surface (search routes call it as emdash.ensureSearchHealthy?.()) that middleware bolts on at runtime; declaring it optional is the honest type and matches the production wiring. Lint cleanups: rename the shadowed 'user' in rbac.test.ts F17 case to 'orphanedUser'; in hydrateDraftData spread the already-narrowed r.data rather than rebroadening to unknown and re-narrowing to Record<string, unknown>. * fix(mcp): apply 3rd-pass adversarial review fixes Production: - F20 backwards compat: add IMPLICIT_SCOPE_GRANTS table so existing content:write tokens continue to work for menu/taxonomy mutations. Reverse direction is not granted. Unit + MCP integration tests. - menu_set_items: existence check moved inside the transaction (closes TOCTOU window where a concurrent menu_delete leaves orphan items on D1) and explicit guard against negative parentIndex. - menu_list / menu_get MCP tools delegate to the typed handlers; drops the last 'as never' casts and aligns response shape with REST (handleMenuList already includes itemCount). - handleMenuGet NOT_FOUND message includes the menu name. - Cycle-detection MAX_DEPTH off-by-one: a chain of exactly MAX_DEPTH ancestors is now accepted (the depth-exceeded error fires only when there's still chain to walk). - Field validation moved entirely to EmDashRuntime (handleContentCreate + handleContentUpdate). Raw API handlers trust pre-validated input; every production caller goes through the runtime wrapper. Removes the duplicate validation pass on the non-revision path. Tests: - validation.test.ts:297 — drop the echo-id alternation; assert field, id, and 'not found' separately. - Three /not\.X/ metachar bugs introduced during F11/F12 (no.changes, not.published, not.empty) escaped to literal phrases. - taxonomy_delete_term leaf test now verifies removal via follow-up taxonomy_list_terms. - ownership null-author tests gain positive _meta.code assertions. - search empty-query and special-char tests pin the swallow contract (success + empty items, never the syntax-error leak). - errors.test.ts orderBy assertion: require the offending column name AND a stable VALIDATION_ERROR code rather than broad alternation. - New cursor-decoder unit tests covering the 4096-byte DoS guard. - New handleMenuSetItems unit tests covering negative / forward parentIndex and missing-menu rejection. - F20 changeset (witty-rocks-knock.md) and F25 changeset (tame-hotels-sort.md) document the user-visible behavior changes. * fix(mcp): pass-4 critical fixes — prototype pollution, comment hygiene, error code mapping H-1: IMPLICIT_SCOPE_GRANTS now backed by a Map<string, readonly string[]> instead of a plain object. Bracket access on the prototype chain (__proto__, constructor, toString, etc.) no longer reaches Function or Object.prototype values that would crash hasScope() with TypeError or accidentally satisfy the check. Test added covering all four chain keys. H-2: 'downstream API handler also validates' comment in EmDashRuntime.handleContentUpdate was false after pass-3 (validation moved entirely to runtime). Removed the misleading sentence. H-3: decodeCursor signature change (T | null -> T, throws) noted in the loud-cursors changeset for plugin authors who use the low-level repositories barrel directly. Strip 'see MCP_BUGS.md #N' references from committed code (3 src files, 6 test files, 1 smoke test). Comments rewritten to be self-contained. Re-export InvalidCursorError from the package root and handleMenuSetItems + MenuSetItemsInput from the api/handlers barrel — both were declared public surface but missed from their respective barrel files. Register INSUFFICIENT_SCOPE (403), INSUFFICIENT_PERMISSIONS (403), and SLUG_CONFLICT (409) in ErrorCode + mapErrorStatus. They were already emitted in the codebase but missing from the central registry, which meant a future apiError() call with one of these codes would silently fall through to the default 400. handleMenuSetItems / handleMenuUpdate / handleMenuDelete NOT_FOUND messages now echo the menu name, matching handleMenuGet. * fix(mcp): pass-4 medium fixes — concurrent-deletion cursor, N+1 menu list, test coverage - taxonomy_list_terms: switch from term-id cursor to base64 keyset over (label, id). Tolerates concurrent deletion of the cursor-term: cursor is a position rather than a row reference, so a missing row just means we skip past it instead of erroring. - handleMenuList: replace N+1 (count per menu) with a single LEFT JOIN + GROUP BY. Postgres-safe number coercion for the count aggregate. - validateParentTerm cycle/depth walk now runs on create as well as update, so a malicious or buggy caller can't grow the chain past MAX_DEPTH (100). Cycle check still scoped to update where termId exists. Two integration tests added: chain of exactly 100 ancestors is accepted, chain of 102 is rejected. - search empty-query and special-char tests seed published content so a regression that interprets bad input as 'match all' would surface a non-empty list and fail. - handleMenuSetItems missing-menu test verifies the transaction rolls back: the menu_items table is empty after the call. - unpublish-on-draft test asserts version is unchanged across the idempotent call (catches phantom-revision / always-bump regressions). - publish twice test pins publishedAt to a known past timestamp before the second publish, replacing the racy 5ms timing trick. A regression that drops COALESCE preservation now fails deterministically. - schema_delete_collection 'has content' test tightened: requires the literal 'has content' phrase and 'force: true' guidance instead of a loose word alternation. - New test: validation rejects non-string value for a string field. Locks in the runtime-layer Zod check after F19 removed the raw handler's UNKNOWN_FIELD catch. - Query-counts snapshot regenerated and matches — no perf regressions from the LEFT JOIN, the cursor switch, or the validation move. * fix(mcp): pass-5 — stable taxonomy ordering, missing rollback/itemCount tests, depth limit docs Bugs: - TaxonomyRepository.findByName/findChildren now order by (label asc, id asc). Without an id tiebreaker the SQL ordering of tied-label rows is dialect- dependent, which breaks the (label, id) keyset cursor — page 2 could duplicate or skip page 1 rows. Regression test added with three terms sharing one label, walked one-at-a-time through pagination. - handleMenuList LEFT JOIN had no correctness coverage. Added a test seeding three menus with known item counts (0, 1, 3); asserts each itemCount and verifies the empty menu appears (guards INNER JOIN regression). - handleMenuSetItems missing-menu rollback test was vacuously true (the table was empty before the call). Now seeds an unrelated menu with an item, runs the failing call, asserts the unrelated item survives. - Added taxonomy_list_terms 'survives concurrent deletion of cursor-term' test — the headline pass-4 fix had no test for the actual concurrent- deletion scenario it was designed to handle. Test tightening: - MAX_DEPTH boundary regex tightened from /maximum depth|MAX_DEPTH|exceeds/ to /maximum depth/i, plus a positive _meta.code === VALIDATION_ERROR assertion. - Non-string title test gains _meta.code === VALIDATION_ERROR. - Stale comment in malformed-cursor test about 'silently resets to start' removed (no longer true after pass-4 keyset switch). Docs: - taxonomy_create_term and taxonomy_update_term descriptions now mention the 100-ancestor depth limit and (for update) cycle detection. - New changeset (poor-buckets-clap.md) covers the taxonomy_list_terms cursor format change and the parent validation expansion. * fix(mcp): pass-6 — bump cursor changeset to minor + clarify depth-limit description Pass-6 reviewer flagged two non-trivial items: - The poor-buckets-clap changeset declared 'patch' but the taxonomy_list_terms cursor format change can produce INVALID_CURSOR for clients with persisted cursors. Per AGENTS.md (post pre-release, real installs depend on current behavior) — bump to minor and emphasize the break clearly. - taxonomy_create_term and taxonomy_update_term descriptions were ambiguous: 'Parent chains are limited to 100 ancestors deep' could read as 'you can have up to 100'. Actual behavior: any pre-existing chain ≥ 100 rejects all new descendants. Clarify. * fix(admin): wire menus:manage and taxonomies:manage scopes into the admin UI The contract test 'admin wire constants match server VALID_SCOPES' caught two scopes added in the omnibus PR (taxonomies:manage and menus:manage) that I'd added to packages/auth but not to the admin client + create-token form. Adds them to API_TOKEN_SCOPES, the form's SCOPE_VALUES list, and updates the contract-test expected default for new collections from ['drafts'] to ['drafts', 'revisions'] (matching the registry default lifted in F14). * fix(search): drop warn log on swallowed FTS5 syntax errors Copilot review on PR #777 flagged that logging swallowed FTS5 syntax errors at warn level was both a log-spam vector (anyone can fire malformed queries in a loop) and an information-disclosure surface (SQLite/D1 embed the raw query in the error message). Removes both warn logs. Behaviour is unchanged: malformed queries still return empty results / continue suggestion loop. Comments updated to explain why the swallow is intentionally silent. * fix(mcp): revision_restore writes to draft, not live, on revision-capable collections For collections that support revisions, the live row's data columns hold the published values and the draft lives in a row in the revisions table pointed to by draft_revision_id. The restore handler was bypassing this model: it called ContentRepository.update directly, overwriting the live columns with the source revision's data and leaving any pending draft unchanged. Per the tool's documented contract ('Replaces the current draft with the specified revision's data. Not automatically published...'), this was the wrong direction. Live should be left alone; the draft should become a copy of the chosen revision. EmDashRuntime.handleRevisionRestore now branches on whether the collection supports revisions: * Revision-capable: create a new draft revision carrying the source revision's data, update draft_revision_id + updated_at on the row. Live columns untouched. Response is hydrated so the returned data reflects the new draft state immediately (closes the bug-#2-shaped staleness in the restore response). * Non-revision: fall through to the legacy raw handler, which still writes to the live row. This is unchanged behavior for opt-out collections. The previous integration test for this case (drafts.test.ts:397) republished v2 before restoring v1, so the assertions passed regardless of whether restore touched draft, live, or both. Replaced with two repros that exercise the actual bug surface (live=v1, draft=v2 -> restore v1 leaves live=v1 and makes draft=v1; and no-draft -> restore creates a new draft). Both tests have been verified to fail against the pre-fix runtime. * style: format --------- Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
0aveRyan
pushed a commit
to 0aveRyan/emdash
that referenced
this pull request
Apr 27, 2026
…s#777) * test(mcp): add integration coverage for documented bugs Adds 73 MCP integration tests across 8 files exercising the real MCP client/server pair against a real database, with no handler mocks. 43 tests fail today against bugs documented in the local MCP_BUGS.md log (emdash-cms#1, emdash-cms#2, emdash-cms#3, emdash-cms#4, emdash-cms#5, emdash-cms#6, emdash-cms#10, emdash-cms#11, emdash-cms#12) and will pass when the omnibus fix lands. Each failing test maps to a specific bug variant; the passing tests serve as regression guards. Refactors EmDashRuntime's 19-arg private positional constructor to a public single-argument constructor that takes an EmDashRuntimeParts object. Production code path is preserved (create() builds the parts and ends with new EmDashRuntime({ ... })); tests can now construct a real runtime around a pre-migrated test database without duplicating any handler logic. Adds an HTTP-level concurrency check to the smoke matrix to cover the parallel-401 race (the InMemoryTransport used by the integration suite cannot reach the production auth middleware path). * test(mcp): add comprehensive coverage for all MCP tools and edges Extends MCP integration coverage to every registered tool, plus the known gaps and edge cases. Adds 145 tests across 7 new files (218 total across the suite, 58 currently failing). New files: - schema.test.ts (46): list/get/create/delete collection, create/delete field, with validation, ownership, error envelope, and side-effect coverage. All passing — schema tools are largely correct. - taxonomy.test.ts (20): list/list_terms/create_term plus bug emdash-cms#7 (orphan collection drift) and bug emdash-cms#13 (no delete/update term gap). - media.test.ts (22): list/get/update/delete with ownership, mimeType filter, pagination, and bug emdash-cms#14 gap (no upload tool). Confirms media ownership extraction already handles null authorId correctly, pinning down the inconsistency with content extraction (bug emdash-cms#1). - menu.test.ts (10): read-only list/get plus bug emdash-cms#15 gap (no mutation tools). - search.test.ts (9): empty index, no-match, collection scoping, special-character handling, draft filtering, permissions. - content-misc.test.ts (25): content_duplicate, content_permanent_delete, content_translations + locale handling on get/create, _rev optimistic concurrency (happy + race), soft-delete visibility, edit-while-trashed, idempotency for publish/unpublish/schedule, and the content_unschedule MCP-tool gap. - input-schemas.test.ts (13): Zod-level argument validation across every tool — missing required, wrong type, out-of-range, enum violation. All passing — pins down the SDK boundary. Failure breakdown by bug: - bug emdash-cms#1 (null authorId): 10 in ownership.test.ts - bug emdash-cms#2 (stale draft data): 4 in drafts.test.ts - bug emdash-cms#3 (bare error strings): 6 in errors.test.ts + scattered - bug emdash-cms#4/emdash-cms#5/emdash-cms#6 (validation): 11 in validation.test.ts - bug emdash-cms#10 (publishedAt): 3 in lifecycle.test.ts - bug emdash-cms#11 (supports default): 2 in lifecycle.test.ts - bug emdash-cms#12 (cursor): 7 in pagination.test.ts - bug emdash-cms#7 + emdash-cms#13 (taxonomy): 8 in taxonomy.test.ts - bug emdash-cms#14 (media upload gap): 1 in media.test.ts - bug emdash-cms#15 (menu mutation gap): 3 in menu.test.ts - search FTS issues: 2 in search.test.ts - content_unschedule gap: 1 in content-misc.test.ts The remaining 58 failures collectively define the omnibus fix's acceptance criteria — turning these green will resolve every documented issue plus the gaps and edge cases this expansion surfaced. * fix(mcp): preserve error codes through tool response envelope The MCP server's `unwrap()` and `errorResult()` helpers stripped the structured error code from handler results, leaving callers with only a human-readable message. Tools that throw raw errors (schema, search, taxonomy, menu) similarly lost any signal beyond `error.message`. Replaces both helpers with three structured emitters: - respondData(data): success envelope - respondError(code, message, details?): emits the code as a stable `[CODE]` prefix on the message text AND attaches `_meta.code` so MCP-aware clients can read it programmatically - respondHandlerError(error, fallbackCode): for the catch-all sites that wrap raw thrown errors. Recognises the `apiError: { code }` annotation that handlers attach to thrown NOT_FOUND / CONFLICT errors (see api/handlers/content.ts:538), preserves the original message, falls back to `INTERNAL_ERROR` otherwise. `unwrap()` now propagates `error.code`, `error.message`, and `error.details` verbatim from the handler's ApiResult shape. Every call site of the old `errorResult()` is migrated: - ApiResult-returning paths use unwrap (already used everywhere) - SchemaRegistry / search / taxonomy / menu try/catch sites use respondHandlerError with tool-specific fallback codes (SCHEMA_LIST_ERROR, FIELD_CREATE_ERROR, SEARCH_ERROR, etc.) - String-error sites ("Collection 'X' not found", "Menu 'X' not found", "Taxonomy 'X' not found") use respondError("NOT_FOUND", ...) - Revision restore "missing collection or entry reference" uses respondError("VALIDATION_ERROR", ...) This is the A1 commit from .opencode/notes/MCP_FIX_PLAN.md. Flips the `_meta.code` propagation test in tests/integration/mcp/errors.test.ts from red to green. The remaining red tests in that file need handler-level work (Wave 2) that surfaces the underlying repository error instead of the current generic "Failed to create content" / "Failed to list content" placeholders. Backwards compatibility: existing MCP clients that match on the bare message will see a `[CODE] ` prefix added. The prefix is on a stable SCREAMING_SNAKE_CASE token, so regex matchers and substring tests on the original message continue to work. * fix(mcp): allow ownership checks on rows with null authorId The MCP server's extractContentAuthorId() previously threw an InternalError when content had no authorId — typically seed-imported rows. This blocked every mutating operation (update, delete, publish, unpublish, schedule, restore) for those rows, including for admins. The companion change in this commit's parent landed in mcp/server.ts: extractContentAuthorId() now returns "" instead of throwing, mirroring how media_update/media_delete already handle null authorId. The ownership check then defers to canActOnOwn(): an actor with the '*:edit_any' permission succeeds, and an actor with only '*:edit_own' gets a clean permission error rather than an internal one. Updates the unit test that previously asserted the throw was correct. The new tests confirm an ADMIN can edit null-authorId rows and an AUTHOR is denied with a permission message (not an internal one). * fix(content): clear publishedAt when content is unpublished ContentRepository.unpublish() previously left published_at populated when flipping status to draft, making 'currently published' indistinguishable from 'previously published' via the data alone. Clear published_at on unpublish so a missing/null timestamp unambiguously means the item is not live. Re-publishing assigns a fresh timestamp via the existing publish path. * fix(mcp): apply documented supports default in schema_create_collection Adds the changeset for this fix. The source change — defaulting `args.supports ?? ['drafts', 'revisions']` at the MCP boundary so the documented default is actually applied — landed alongside the error envelope refactor in 9986405 due to working-tree interleaving between parallel agents. Tracking it as a separate changeset so the user-visible behavior change is described independently. * fix(content,pagination,taxonomies): wave 2 omnibus fixes Combines the three Wave 2 fixes from .opencode/notes/MCP_FIX_PLAN.md because they entangled in the working tree during parallel agent execution. B (validation, bugs emdash-cms#4/emdash-cms#5/emdash-cms#6): wires generateZodSchema() into handleContentCreate / handleContentUpdate. Required fields enforced; select / multiSelect option lists honored; reference fields verified to point at a real, non-trashed target. New helper at api/handlers/validation.ts. Errors surface as { code: VALIDATION_ERROR, message } with all offending fields named in one message so callers can fix everything in a single round trip. C1 (cursor decoding, bug emdash-cms#12): decodeCursor() now throws InvalidCursorError on bad input instead of silently returning null. Updates every repository caller (content, byline, audit, redirect, comment, plugin-storage, media, user, sections, loader). API handler catch blocks recognise the error and return INVALID_CURSOR. The MCP boundary rejects empty-string cursors via z.string().min(1) on the input schemas. taxonomy_list_terms gets parallel treatment for its in-memory term-id cursor. G1 (taxonomy orphan filter, bug emdash-cms#7): handleTaxonomyList filters each taxonomy's collections array against the real collection set on read. Storage stays untouched so re-creating a deleted collection re-links automatically. Test fix-ups in the same commit (these are tests I committed earlier on this branch that didn't account for production behavior): - taxonomy.test.ts: account for the seeded 'category' and 'tag' taxonomies that migration 006 inserts; reshape bug-emdash-cms#7 test around the seed's 'posts' orphan rather than a manufactured one; correct taxonomy_create_term response envelope ({ term: { ... } }). * fix(mcp): wave 3+4 — error fidelity, gap-feature tools, search and draft fixes start - Handler-level error fidelity (bug emdash-cms#3): handleContentList and handleContentCreate / handleContentUpdate now translate database-level errors into structured codes (COLLECTION_NOT_FOUND, SLUG_CONFLICT, UNKNOWN_FIELD, VALIDATION_ERROR) instead of collapsing to a generic 'Failed to ...' message. - validation.ts: detects unknown keys in content data so callers get a useful error rather than silently dropped writes. - handleTermCreate: validates parentId exists and belongs to the same taxonomy. - New MCP tools (gap fills): - content_unschedule (bug — runtime had handleContentUnschedule but no MCP tool) - taxonomy_update_term, taxonomy_delete_term (bug emdash-cms#13) - menu_create, menu_update, menu_delete, menu_set_items (bug emdash-cms#15) - media_create (bug emdash-cms#14 — registers metadata for files uploaded out-of-band; binary uploads remain off the MCP transport) - Test fixups: content-handlers slug test no longer expects a non-string title to silently produce null slug (validation now rejects that). loader-cursor-pagination expects the loader's error envelope rather than a thrown error. - Taxonomy tests adjusted to account for the seed data inserted by migration 006 ('category' and 'tag'). * fix(content,search): draft data hydration and FTS robustness - EmDashRuntime.handleContentGet / handleContentUpdate / handleContentGetIncludingTrashed now overlay the current draft revision's data onto the response when draftRevisionId is set, with the previously-published values exposed alongside as 'liveData'. Resolves bug emdash-cms#2 — agents calling content_get after content_update on a revision-supporting collection now see their pending draft, not the stale live row. - searchSingleCollection swallows FTS5 syntax errors and returns no matches rather than leaking the SQLite error text to callers. Malformed user-typed queries (unbalanced quotes, stray operators, unclosed parens) produce empty results instead of a 500-style error. - Search test setup activates the FTS index via FTSManager.enableSearch(). Without it, the FTS table and triggers don't exist and the search-after-publish round trip silently produces no matches — same trap a real site would hit if the admin never enabled search. - Minor cleanup: deduplicate inline regex with COLLECTION_SLUG_PATTERN in mcp/server.ts; safer typeof-narrowed cast in hydrateDraftData. * fix(mcp): production correctness wave (F1-F6) F1: media_create now forwards authorId from MCP user to repository so ownership checks succeed on subsequent media_update/delete by the same author. F2: taxonomy_update_term gains parent validation (existence, same taxonomy, self-parent rejection) and cycle detection via parent-chain walk (bounded at 100). Shared helper used by create + update. F3: empty-string parentId/slug normalized to undefined/null at handler and repository layers; previously '' was persisted as a literal slug. F4: content validation runs in EmDashRuntime.handleContentUpdate BEFORE the draft revision write, so revision-supporting collections can no longer slip invalid data into history. The downstream API handler still validates as defense-in-depth. F5: validation.ts reference-target catch is narrowed to isMissingTableError; any other DB error now propagates instead of being silently reported as 'reference not found'. IN clause is also chunked at SQL_BATCH_SIZE for D1 bind-parameter safety. F6: menu_delete (REST + MCP) now deletes _emdash_menu_items first inside a transaction. D1 has FOREIGN KEYS off by default so cascade was a no-op there. Description updated to drop the misleading FK-cascade claim. taxonomy_delete_term description corrected to match the handler's 'cannot delete with children' behavior. Tests: media gap test replaced with full happy-path (AUTHOR creates, gets, updates own; OTHER_AUTHOR is denied). Taxonomy gap presence-only tests replaced with rename/relabel/reparent/detach/cross-taxonomy rejection/self-parent rejection/cycle rejection. Menu gap presence-only tests replaced with create+get round-trip, duplicate CONFLICT, update label, set_items empty, set_items 3-level nesting, set_items parentIndex>=i rejection, F6 delete-cascade verification. 2806 -> 2821 tests passing. * fix(mcp): error envelope codes for SchemaError + auth (F7) respondHandlerError now reads error.code from any Error subclass with a string code field — SchemaError ('RESERVED_SLUG', 'COLLECTION_EXISTS', etc.) and the new EmDashAuthError class flow through with their stable codes intact. Numeric codes (e.g. McpError) are skipped — _meta.code is reserved for stable string codes. Auth helpers (requireScope, requireRole, requireOwnership, requireDraftAccess, the inline publish-perm check) now throw EmDashAuthError with stable codes (INSUFFICIENT_SCOPE, INSUFFICIENT_PERMISSIONS) rather than McpError. The SDK's text-only fallback envelope was stripping the code. To make those throws survive the SDK's catch path, registerTool is locally monkey-patched at server-creation time so every tool callback is wrapped in try/catch that funnels into respondHandlerError — without requiring 41 callbacks to grow individual try/catch blocks. Tests: 3 new F7 cases verify _meta.code === 'INSUFFICIENT_SCOPE' / 'INSUFFICIENT_PERMISSIONS' / SchemaError code (non-fallback) instead of the SDK's previous code-less text envelope. 2821 -> 2824 tests passing. * test(mcp): tighten regex assertions, add happy paths, liveData type (F8-F13) F8: replace broken /not.found/ regex (the . was a metacharacter matching any byte) with a tight /\bNOT_FOUND\b|\bnot found\b/i. Applied across errors / schema / content-misc / menu / taxonomy / media / validation test files. F9: drop the id-alternation in echo regexes — /(NOT_FOUND|01NEVEREXISTED)/ passed if the id alone was echoed, defeating the test. Each NOT_FOUND test now asserts the code/word AND, separately via expect.toContain, the id (when the message body actually echoes it). F10: tighten overly broad collection/field 'word match' assertions to the specific code (COLLECTION_NOT_FOUND / FIELD_NOT_FOUND) plus an expect.toContain on the offending slug. F11: replaced empty-conditional dead branches with positive post-condition assertions: - publish twice / unpublish on draft now check status in the success branch as well as the error branch. - pagination.test 'limit beyond max' now also checks the error message in the rejection branch. - search 'special chars' now checks items is an array in the sanitized-success branch. - search 'scope filter' now also asserts items.length > 0 so the for- loop is reachable. - pagination 'revision_list malformed cursor' was a no-op (expect(result).toBeDefined()) — deleted, with a note explaining revision_list has no cursor parameter. F12: replaced presence-only listTools tests with happy-path coverage: - content_unschedule: schedule + unschedule + verify scheduledAt is null + re-publish succeeds. - (taxonomy/menu/media gap-feature happy paths landed in the previous commit alongside F2/F3/F6.) F13: ContentItem now declares an optional liveData?: Record<string, unknown> field, with JSDoc explaining the contract: populated by hydrateDraftData when a draft revision exists, otherwise undefined. New drafts.test asserts both branches. 2824 -> 2826 tests passing. * fix(mcp): F14-F18 + F23/F24 — supports default, ownership edge, menu refactor, hydration cleanup F14: SchemaRegistry.createCollection now defaults supports to ['drafts', 'revisions'] when undefined. MCP and admin UI defaults removed in favor of the canonical lower-layer default. F17: canActOnOwn treats empty-string ownerId as 'no recorded owner' so authorId-stripping doesn't accidentally grant edit-own to an empty-id user. Test added for the edge. F18: Menu MCP tools delegate to typed handlers in api/handlers/menus.ts. New handleMenuSetItems handler exposes the atomic-replace logic for both REST (future) and MCP. Drops the as-never casts in MCP layer. F23: hydrateDraftData clones the response instead of mutating in place. Future request-cache layers won't observe stale-after-mutation bugs. F24: Strip leading-underscore keys from revision data before merging into the response. _slug etc. are runtime-internal and don't belong on the surfaced data field. F15: hydrateDraftData runs BEFORE afterSave hook so plugins see the just-saved draft data, not the live columns, for revision-supporting collections. F16: hydration error path logs '[emdash] draft hydration failed:' instead of swallowing silently. * fix(content): F19 remove dead unknown-field catch; F25 tighten conflict matchers Validation now rejects unknown keys upfront with VALIDATION_ERROR so the post-INSERT 'no such column' / 'column does not exist' catch branches in handleContentCreate and handleContentUpdate were unreachable. Drop them. Tighten the unique-constraint detection to match 'unique constraint failed' or 'duplicate key' specifically, instead of bare 'unique' (which false-positives on any error message containing the word) or 'constraint failed' (which catches NOT NULL and CHECK violations too). Make the create and update paths symmetric: both produce sanitized SLUG_CONFLICT messages and a generic 'Unique constraint violation' for non-slug uniques, so we don't leak raw DB error text in either path. * fix(auth,mcp,search): F20 menu/taxonomy scopes; F21 tighten fts5 swallow F20: add menus:manage and taxonomies:manage to the API token scope vocabulary (mapped to EDITOR via SCOPE_MIN_ROLE). Switch the seven MCP mutation tools (taxonomy_create_term, taxonomy_update_term, taxonomy_delete_term, menu_create, menu_update, menu_delete, menu_set_items) from content:write to the dedicated scope. REST already uses these names as permissions; bringing scopes in line lets a token hold menu/taxonomy management without granting general content writes. F21: extract isFts5SyntaxError() helper that matches specifically on 'fts5: syntax error' and 'unknown special query' rather than the broad 'fts5' / 'syntax error' string. The old filter would swallow internal table-corruption errors. Apply the helper to both searchSingleCollection and getSuggestions, and log the swallow at warn level so silent failures show up in the logs. * fix(types,mcp,tests): F22 cursor DoS guard; F26 drop harness cast; lint cleanups F22: reject cursors longer than 4096 chars in decodeCursor() before calling decodeBase64() (which is O(N) in input size). Also clamp the MCP and REST input schemas at 2048 chars so oversized cursors fail early at the schema boundary instead of allocating against giant strings inside the repository helper. New pagination test exercises the schema-boundary rejection. F26: drop the 'as unknown as EmDashHandlers' cast in the test harness so real interface drift surfaces instead of being hidden. Add the optional ensureSearchHealthy member to EmDashHandlers — it's a real public surface (search routes call it as emdash.ensureSearchHealthy?.()) that middleware bolts on at runtime; declaring it optional is the honest type and matches the production wiring. Lint cleanups: rename the shadowed 'user' in rbac.test.ts F17 case to 'orphanedUser'; in hydrateDraftData spread the already-narrowed r.data rather than rebroadening to unknown and re-narrowing to Record<string, unknown>. * fix(mcp): apply 3rd-pass adversarial review fixes Production: - F20 backwards compat: add IMPLICIT_SCOPE_GRANTS table so existing content:write tokens continue to work for menu/taxonomy mutations. Reverse direction is not granted. Unit + MCP integration tests. - menu_set_items: existence check moved inside the transaction (closes TOCTOU window where a concurrent menu_delete leaves orphan items on D1) and explicit guard against negative parentIndex. - menu_list / menu_get MCP tools delegate to the typed handlers; drops the last 'as never' casts and aligns response shape with REST (handleMenuList already includes itemCount). - handleMenuGet NOT_FOUND message includes the menu name. - Cycle-detection MAX_DEPTH off-by-one: a chain of exactly MAX_DEPTH ancestors is now accepted (the depth-exceeded error fires only when there's still chain to walk). - Field validation moved entirely to EmDashRuntime (handleContentCreate + handleContentUpdate). Raw API handlers trust pre-validated input; every production caller goes through the runtime wrapper. Removes the duplicate validation pass on the non-revision path. Tests: - validation.test.ts:297 — drop the echo-id alternation; assert field, id, and 'not found' separately. - Three /not\.X/ metachar bugs introduced during F11/F12 (no.changes, not.published, not.empty) escaped to literal phrases. - taxonomy_delete_term leaf test now verifies removal via follow-up taxonomy_list_terms. - ownership null-author tests gain positive _meta.code assertions. - search empty-query and special-char tests pin the swallow contract (success + empty items, never the syntax-error leak). - errors.test.ts orderBy assertion: require the offending column name AND a stable VALIDATION_ERROR code rather than broad alternation. - New cursor-decoder unit tests covering the 4096-byte DoS guard. - New handleMenuSetItems unit tests covering negative / forward parentIndex and missing-menu rejection. - F20 changeset (witty-rocks-knock.md) and F25 changeset (tame-hotels-sort.md) document the user-visible behavior changes. * fix(mcp): pass-4 critical fixes — prototype pollution, comment hygiene, error code mapping H-1: IMPLICIT_SCOPE_GRANTS now backed by a Map<string, readonly string[]> instead of a plain object. Bracket access on the prototype chain (__proto__, constructor, toString, etc.) no longer reaches Function or Object.prototype values that would crash hasScope() with TypeError or accidentally satisfy the check. Test added covering all four chain keys. H-2: 'downstream API handler also validates' comment in EmDashRuntime.handleContentUpdate was false after pass-3 (validation moved entirely to runtime). Removed the misleading sentence. H-3: decodeCursor signature change (T | null -> T, throws) noted in the loud-cursors changeset for plugin authors who use the low-level repositories barrel directly. Strip 'see MCP_BUGS.md #N' references from committed code (3 src files, 6 test files, 1 smoke test). Comments rewritten to be self-contained. Re-export InvalidCursorError from the package root and handleMenuSetItems + MenuSetItemsInput from the api/handlers barrel — both were declared public surface but missed from their respective barrel files. Register INSUFFICIENT_SCOPE (403), INSUFFICIENT_PERMISSIONS (403), and SLUG_CONFLICT (409) in ErrorCode + mapErrorStatus. They were already emitted in the codebase but missing from the central registry, which meant a future apiError() call with one of these codes would silently fall through to the default 400. handleMenuSetItems / handleMenuUpdate / handleMenuDelete NOT_FOUND messages now echo the menu name, matching handleMenuGet. * fix(mcp): pass-4 medium fixes — concurrent-deletion cursor, N+1 menu list, test coverage - taxonomy_list_terms: switch from term-id cursor to base64 keyset over (label, id). Tolerates concurrent deletion of the cursor-term: cursor is a position rather than a row reference, so a missing row just means we skip past it instead of erroring. - handleMenuList: replace N+1 (count per menu) with a single LEFT JOIN + GROUP BY. Postgres-safe number coercion for the count aggregate. - validateParentTerm cycle/depth walk now runs on create as well as update, so a malicious or buggy caller can't grow the chain past MAX_DEPTH (100). Cycle check still scoped to update where termId exists. Two integration tests added: chain of exactly 100 ancestors is accepted, chain of 102 is rejected. - search empty-query and special-char tests seed published content so a regression that interprets bad input as 'match all' would surface a non-empty list and fail. - handleMenuSetItems missing-menu test verifies the transaction rolls back: the menu_items table is empty after the call. - unpublish-on-draft test asserts version is unchanged across the idempotent call (catches phantom-revision / always-bump regressions). - publish twice test pins publishedAt to a known past timestamp before the second publish, replacing the racy 5ms timing trick. A regression that drops COALESCE preservation now fails deterministically. - schema_delete_collection 'has content' test tightened: requires the literal 'has content' phrase and 'force: true' guidance instead of a loose word alternation. - New test: validation rejects non-string value for a string field. Locks in the runtime-layer Zod check after F19 removed the raw handler's UNKNOWN_FIELD catch. - Query-counts snapshot regenerated and matches — no perf regressions from the LEFT JOIN, the cursor switch, or the validation move. * fix(mcp): pass-5 — stable taxonomy ordering, missing rollback/itemCount tests, depth limit docs Bugs: - TaxonomyRepository.findByName/findChildren now order by (label asc, id asc). Without an id tiebreaker the SQL ordering of tied-label rows is dialect- dependent, which breaks the (label, id) keyset cursor — page 2 could duplicate or skip page 1 rows. Regression test added with three terms sharing one label, walked one-at-a-time through pagination. - handleMenuList LEFT JOIN had no correctness coverage. Added a test seeding three menus with known item counts (0, 1, 3); asserts each itemCount and verifies the empty menu appears (guards INNER JOIN regression). - handleMenuSetItems missing-menu rollback test was vacuously true (the table was empty before the call). Now seeds an unrelated menu with an item, runs the failing call, asserts the unrelated item survives. - Added taxonomy_list_terms 'survives concurrent deletion of cursor-term' test — the headline pass-4 fix had no test for the actual concurrent- deletion scenario it was designed to handle. Test tightening: - MAX_DEPTH boundary regex tightened from /maximum depth|MAX_DEPTH|exceeds/ to /maximum depth/i, plus a positive _meta.code === VALIDATION_ERROR assertion. - Non-string title test gains _meta.code === VALIDATION_ERROR. - Stale comment in malformed-cursor test about 'silently resets to start' removed (no longer true after pass-4 keyset switch). Docs: - taxonomy_create_term and taxonomy_update_term descriptions now mention the 100-ancestor depth limit and (for update) cycle detection. - New changeset (poor-buckets-clap.md) covers the taxonomy_list_terms cursor format change and the parent validation expansion. * fix(mcp): pass-6 — bump cursor changeset to minor + clarify depth-limit description Pass-6 reviewer flagged two non-trivial items: - The poor-buckets-clap changeset declared 'patch' but the taxonomy_list_terms cursor format change can produce INVALID_CURSOR for clients with persisted cursors. Per AGENTS.md (post pre-release, real installs depend on current behavior) — bump to minor and emphasize the break clearly. - taxonomy_create_term and taxonomy_update_term descriptions were ambiguous: 'Parent chains are limited to 100 ancestors deep' could read as 'you can have up to 100'. Actual behavior: any pre-existing chain ≥ 100 rejects all new descendants. Clarify. * fix(admin): wire menus:manage and taxonomies:manage scopes into the admin UI The contract test 'admin wire constants match server VALID_SCOPES' caught two scopes added in the omnibus PR (taxonomies:manage and menus:manage) that I'd added to packages/auth but not to the admin client + create-token form. Adds them to API_TOKEN_SCOPES, the form's SCOPE_VALUES list, and updates the contract-test expected default for new collections from ['drafts'] to ['drafts', 'revisions'] (matching the registry default lifted in F14). * fix(search): drop warn log on swallowed FTS5 syntax errors Copilot review on PR emdash-cms#777 flagged that logging swallowed FTS5 syntax errors at warn level was both a log-spam vector (anyone can fire malformed queries in a loop) and an information-disclosure surface (SQLite/D1 embed the raw query in the error message). Removes both warn logs. Behaviour is unchanged: malformed queries still return empty results / continue suggestion loop. Comments updated to explain why the swallow is intentionally silent. * fix(mcp): revision_restore writes to draft, not live, on revision-capable collections For collections that support revisions, the live row's data columns hold the published values and the draft lives in a row in the revisions table pointed to by draft_revision_id. The restore handler was bypassing this model: it called ContentRepository.update directly, overwriting the live columns with the source revision's data and leaving any pending draft unchanged. Per the tool's documented contract ('Replaces the current draft with the specified revision's data. Not automatically published...'), this was the wrong direction. Live should be left alone; the draft should become a copy of the chosen revision. EmDashRuntime.handleRevisionRestore now branches on whether the collection supports revisions: * Revision-capable: create a new draft revision carrying the source revision's data, update draft_revision_id + updated_at on the row. Live columns untouched. Response is hydrated so the returned data reflects the new draft state immediately (closes the bug-emdash-cms#2-shaped staleness in the restore response). * Non-revision: fall through to the legacy raw handler, which still writes to the live row. This is unchanged behavior for opt-out collections. The previous integration test for this case (drafts.test.ts:397) republished v2 before restoring v1, so the assertions passed regardless of whether restore touched draft, live, or both. Replaced with two repros that exercise the actual bug surface (live=v1, draft=v2 -> restore v1 leaves live=v1 and makes draft=v1; and no-draft -> restore creates a new draft). Both tests have been verified to fail against the pre-fix runtime. * style: format --------- Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
ascorbic
added a commit
that referenced
this pull request
Jul 1, 2026
…ns to globalThis (#1399) * perf(core): cache taxonomy defs per-isolate; move runtime/db singletons to globalThis Cut per-render D1 round trips on public pages by caching taxonomy definitions across the worker isolate, and harden the runtime/DB singletons against bundler module duplication. - getTaxonomyDefs (query #12, hit on every render that hydrates entry terms) now has a two-tier cache: per-request + per-isolate via a globalThis Symbol holder keyed by locale, invalidated in-memory by every def write (handleTaxonomyCreate, seed). Isolated DBs bypass it. - runtimeInstance/dbCache/dbInitPromise moved onto globalThis behind Symbol keys so Vite SSR chunk duplication can't spawn duplicate caches that re-run cold-start migrations/bootstrap reads. No schema or public-API changes. Cold-start db.batch() batching deferred to a follow-up. * chore: format changeset * ci: update query-count snapshots * chore: regenerate query-count snapshots with taxonomy-defs cache on fixed harness The pre-merge snapshots were measured with the old query-counts harness that missed queries issued during streaming (fixed in #1580). Regenerated against the fixed harness so the numbers reflect reality: the per-isolate taxonomy-defs cache removes the repeated _emdash_taxonomy_defs read on every public render (-1 query per route). * ci: update query-count snapshots --------- Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com> Co-authored-by: Matt Kane <mkane@cloudflare.com>
18 tasks
marcusbellamyshaw-cell
pushed a commit
to Emdash-Bug-Testing/emdash
that referenced
this pull request
Jul 22, 2026
…ns to globalThis (emdash-cms#1399) * perf(core): cache taxonomy defs per-isolate; move runtime/db singletons to globalThis Cut per-render D1 round trips on public pages by caching taxonomy definitions across the worker isolate, and harden the runtime/DB singletons against bundler module duplication. - getTaxonomyDefs (query emdash-cms#12, hit on every render that hydrates entry terms) now has a two-tier cache: per-request + per-isolate via a globalThis Symbol holder keyed by locale, invalidated in-memory by every def write (handleTaxonomyCreate, seed). Isolated DBs bypass it. - runtimeInstance/dbCache/dbInitPromise moved onto globalThis behind Symbol keys so Vite SSR chunk duplication can't spawn duplicate caches that re-run cold-start migrations/bootstrap reads. No schema or public-API changes. Cold-start db.batch() batching deferred to a follow-up. * chore: format changeset * ci: update query-count snapshots * chore: regenerate query-count snapshots with taxonomy-defs cache on fixed harness The pre-merge snapshots were measured with the old query-counts harness that missed queries issued during streaming (fixed in emdash-cms#1580). Regenerated against the fixed harness so the numbers reflect reality: the per-isolate taxonomy-defs cache removes the repeated _emdash_taxonomy_defs read on every public render (-1 query per route). * ci: update query-count snapshots --------- Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com> Co-authored-by: Matt Kane <mkane@cloudflare.com>
marcusbellamyshaw-cell
pushed a commit
to Emdash-Bug-Testing/emdash
that referenced
this pull request
Jul 22, 2026
…ns to globalThis (emdash-cms#1399) * perf(core): cache taxonomy defs per-isolate; move runtime/db singletons to globalThis Cut per-render D1 round trips on public pages by caching taxonomy definitions across the worker isolate, and harden the runtime/DB singletons against bundler module duplication. - getTaxonomyDefs (query emdash-cms#12, hit on every render that hydrates entry terms) now has a two-tier cache: per-request + per-isolate via a globalThis Symbol holder keyed by locale, invalidated in-memory by every def write (handleTaxonomyCreate, seed). Isolated DBs bypass it. - runtimeInstance/dbCache/dbInitPromise moved onto globalThis behind Symbol keys so Vite SSR chunk duplication can't spawn duplicate caches that re-run cold-start migrations/bootstrap reads. No schema or public-API changes. Cold-start db.batch() batching deferred to a follow-up. * chore: format changeset * ci: update query-count snapshots * chore: regenerate query-count snapshots with taxonomy-defs cache on fixed harness The pre-merge snapshots were measured with the old query-counts harness that missed queries issued during streaming (fixed in emdash-cms#1580). Regenerated against the fixed harness so the numbers reflect reality: the per-isolate taxonomy-defs cache removes the repeated _emdash_taxonomy_defs read on every public render (-1 query per route). * ci: update query-count snapshots --------- Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com> Co-authored-by: Matt Kane <mkane@cloudflare.com>
signal-alchemist
pushed a commit
to signal-alchemist/emdash
that referenced
this pull request
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bootstrapstep from the "Next steps" output in create-emdashTesting
Verified the change shows only
cd <project>anddevcommands in the next steps output.