githubnext/chopinPublic

Let's compose rich plans together

Stop a client batch from crashing the server #437

OpenMaggieAppletonwants to merge 0 commits intomainfromrefs/pull/437/head
  1. MA

    ## Problem

    A single writer could crash the server process.

    The crash happened in two steps:

    1. **The serializer wrote MDX the server can't read.** `packages/dialect` serialized with `resourceLink: false`, so `mdast-util-to-markdown` wrote a link whose text equals its URL as an autolink, `<https://example.com>`. MDX has no autolinks. `mdxMd` turns them off, and `<https:` parses as a broken JSX tag, so `parse()` throws `PlanParseError`. - Any browser can create such a link: select a URL and link it to itself with the link toolbar, or paste a URL that is turned into a link. - Any custom write-authorized client can create one directly. 2. **The server treated that error as fatal.** `room.apply()` rethrew every error that was not a `PlanValidationError`. - `apply` runs from `commit()` on `plan.flushing` inside a `setTimeout`, and nothing awaits that promise. - The throw became an unhandled rejection, and Bun exits on those. A minimal repro exits with code 1. - The room also kept the unvalidated update, with no rebuild.

    The review found a second path to the same crash. `submit()` never checks that the Yjs bytes are valid, and `Y.applyUpdate` ran outside the protected block. A malformed `plan:update` (for example the single byte `[255]`) crashed the server the same way.

    ## Repro

    - Serializing a link whose text is its URL gives `"<https://example.com>\n"`. Parsing that gives `PlanParseError: Unexpected character '/' … (note: to create a link in MDX, use [text](url))`. - `room.apply()` with that link's Yjs update throws from `room.ts` at the `parse(project(target))` call. Before the fix, it did not return `{ ok: false }`.

    ## Fix

    - **Serializer:** set `resourceLink: true`, so such a link is written as `[url](url)`. This setting only controls whether to write an autolink. No stored plan can contain the autolink form, because it would never have parsed, so persisted bytes do not change. - **Room:** `apply()` now refuses every failure that happens after it applies the client's bytes. `service.ts` then runs `rejectBatch`, which rebuilds from the last known-good state as documented. The new results are:

    | Failure | Result | | --- | --- | | Yjs can't decode the bytes | `malformed-update` | | Projection won't re-parse (`PlanParseError`) | `unparseable` | | Any other projection error (also logged) | `unprojectable` |

    ## Tests

    - **Dialect:** a link whose text is its URL serializes as a resource link and round-trips. This covers both an MDAST link and a Lexical `LinkNode` exported through `exportPlan`. - **Room:** - A client link whose text is its URL is accepted. - A projection that won't re-parse is refused, not thrown, and the room then rebuilds to the known-good state. - Bytes Yjs can't decode are refused, not thrown.

    `bun test`, `bun run types` and `bun run ci` pass. I did not run E2E locally.

    ## Follow-up (not in this PR)

    `commit()` still has no catch at the `schedule()` call site. Server-side failures can still exit the process: a `commitHosted` storage or fence error, or a throw inside `rejectBatch`. A single client can't trigger these. Losing the fence may be meant to be fatal, so I left that decision open.

    šŸ¤– Generated with [Claude Code](https://claude.com/claude-code)

  2. Unknown GitHub usercommitted d711757f8a19last month on GitHub

  3. The preview deployment for **chopin** failed. šŸ”“

    [Open Build Logs](https://next-appserver.tail49836a.ts.net/project/l9buznc9ua1if0vflxrjwajs/environment/lxqsvczv19eopxk29vfb46mj/application/hb85gdd9mwqqlfwcteflvpbj/deployment/h65hgvfjsprqwhkg43wirmm1) | [Open Application Logs](https://next-appserver.tail49836a.ts.net/project/l9buznc9ua1if0vflxrjwajs/environment/lxqsvczv19eopxk29vfb46mj/application/hb85gdd9mwqqlfwcteflvpbj/logs)

    Last updated at: 2026-10-09 14:38:40 CET