--- url: https://spellingcreator.org/docs/developers/web-app/pull-requests.md --- # Pull requests For how to use this, see [Proposing changes](../../guide/pull-requests.md). Anyone can **fork** a published lesson and do what they like with their copy. But nobody can write somebody else's lesson. To offer work back, you open a **pull request** against the original, and its author (or one of the trusted collaborators they named) reviews it and merges it in. The app calls these "proposals" and "proposed changes"; the code and API call them pulls. That review step is the whole feature. There used to be a direct route: a trusted collaborator could merge their fork straight into the original, writing its document and its history in one action. That's gone. A lesson now only ever changes because someone with authority over it chose to change it, and every change from outside arrives as something they can read first. Pull requests are built on the per-lesson git repository described in [Version history](../version-history.md). ## The shape of it ``` fork propose review & merge lesson ───────▶ your copy ──────────────────▶ pull request ────────────────▶ lesson (edit freely) (a snapshot) (+ merge commit) ``` 1. **Fork** the lesson from its page. You get a genuine clone of its git repository: full history, shared ancestry (see [Version history](../version-history.md)). 2. **Edit** your copy. It's yours; publish it, keep it as a draft, whatever. 3. **Propose changes to *lesson*** in the editor. This packs your repository and uploads it with a title and an optional note. Nothing in the original changes. The button is shown only on a fork, to a signed-in user who isn't the original's author. 4. The lesson's author sees it on the lesson's **Proposals** tab (`/hub/:id/proposals`), and gets a notification. Each proposal also has a page of its own at `/hub/:id/proposals/:prId`, which shows **what it changes** and whether it would merge cleanly (see below). They (or a trusted collaborator) hit **Review & merge**. 5. That opens the lesson in *their* editor (`/editor?pull=&lesson=`; the link names both, so the review waits for the lesson it belongs to rather than running against whatever the reviewer already had open) and runs the usual block-by-block merge. The merge dialog opens with a summary of everything that settled on its own, and a choice to make for each genuine clash, often none. Nothing is written until they confirm. A proposal the lesson already contains skips all this and is simply recorded as merged. 6. On confirm: the merge is pushed to the lesson's history, the lesson's document is saved, and the proposal is marked merged. The proposer gets a notification. Either side can also **close** it: you withdraw yours, the author declines it, a moderator removes it. A closed proposal keeps its row (the conversation stays visible); only its stored changes are dropped. Notifications (type `pull_request`) go to the lesson's author when a proposal becomes ready or is updated, and to the proposer when someone else merges or closes it. Trusted collaborators aren't notified; they see proposals on the tab. ## A proposal is a snapshot What a pull request actually contains is a **git packfile**, your fork's lesson as it stood the moment you opened the request, stored in R2 under `git/pulls//pack`, beside the lessons' own packs. A pack is capped at the same `MAX_PACK_BYTES` (10 MB) as a lesson's own history. The lesson, and not your [variations](./variations.md) of it: a variation is an idea you are still turning over, and offering one to somebody else to merge, unasked and unmentioned, is not what "propose changes" means. Which version of your fork? **The one you were working on.** If you developed the idea on a [variation](./variations.md), that is what gets proposed, and only that one, so the rest of your variations stay yours. The proposal records which branch it came from (`head_ref`, null for `main`), and the review queue says so. Snapshotting is deliberate. You carry on editing your fork after proposing, and a request that silently tracked your branch would mean the reviewer reading one thing and merging another. ## Updating a proposal The pack used to be written once and never rewritten, which made "silently" moot by making *any* change impossible, so being asked for a tweak meant closing the proposal and opening another, throwing away the discussion attached to it. When a fork already has a proposal open, the editor's propose button becomes **Update your proposal**, which uploads a new pack to the same proposal and records that it happened: the version number goes up, the commit it used to point at is kept (`previous_head`), and the page shows *"Version 3 · updated 4 March"* along with what that last update changed. Nothing moves silently; what was actually being protected was never immutability, it was that nothing changes without saying so. A proposal may only move **forward**: the new tip has to contain the one the proposal already points at. That is what keeps one pack per proposal honest: the previous version's commit is still reachable in the new pack, which is how "what the last update changed" is answerable without storing a pack per version. It is checked by the client, because the Worker holds a proposal's history as an opaque packfile it cannot walk, the same limit that stops the merge endpoint verifying ancestry, bounded the same way, since the only proposal you can rewrite is your own and you could always have closed and reopened it. Re-sending the commit the proposal already holds is refused rather than recorded as a new version. There is a ceiling of **20 updates** (`MAX_PULL_REVISIONS`). Past a couple of dozen rewrites it is a different change, and the thread attached to it has stopped being about what the proposal now contains. An assistant working over MCP follows the same rule: `propose_changes` from a fork that already has one open updates it rather than stacking a second one beside it. Because a fork is a real clone, that pack shares object ids with the lesson's own history. The reviewer indexes it into the lesson's repository, where its objects meet the commits the two already share, and git finds their true **merge base**. So merging a proposal is the same three-way, block-by-block merge as pulling an original's changes into a fork, just pointed the other way. Opening a request is therefore two steps: insert the row, then upload the pack against its id. A row is `ready` only once the pack has landed, and an unready row is listed **only to the person who opened it**: there's nothing to review, and a reviewer shouldn't have to tell a half-finished submission from a real one. If the upload fails, the client withdraws the empty request rather than leaving it in the author's queue. ## Reading one without merging it A proposal's own page shows its changes (block by block, in the same summary the history view renders) and whether merging it would ask the reviewer for anything: * *"This merges cleanly. There's nothing to decide."* * *"2 blocks have been changed here and in the lesson, so merging will ask you which to keep."* * *"These changes are already part of the lesson."* That is computed on the page, from the git objects themselves (`prepareProposalReview`). Both packs are public exactly as far as the lesson is, so the browser indexes the proposal's pack beside the lesson's, finds the commit the two diverged at, and diffs against **that**, not against the lesson's current tip, which would show the author's own later edits as though the proposer had made them, reversed. Merging still happens in the editor, and that split is deliberate rather than a limitation: merging commits to the lesson's history and pushes it under the reviewer's credentials, which needs the editor's repository. Reading needs none of it. What changed is that nobody has to start a merge to find out whether they want one. There is a second place a proposal can be read and settled, on the same terms: `review_proposal` over MCP (see [MCP tools](../mcp-server/tools.md)) renders the same diff inside an assistant's conversation, with merge and decline on it for the reviewer. It computes the merge base the same way, and it hands conflicts back here: a block both sides rewrote wants the editor's merge dialog and two versions side by side, not a button. The git engine is about 200 KB and is fetched on demand (`loadGitEngine` in `apps/web/src/lib/git/load.js`) when this page opens, so the proposal's title, author and note render first and the diff arrives after. If it can't be read at all, the page says so and the rest of it still works. There is no single-proposal endpoint: the page reads the lesson's list and picks its one out, which also brings `canReview` with it. ## Trying it before deciding Reading a diff tells you what changed. It doesn't tell you whether the lesson still works with the change in it: whether the new question fits where it was put, whether the rewritten section still reads in order. So there is a third answer between merging and declining: **try it in a variation** (`&try=1` on the editor link). That lands the proposal on a [variation](./variations.md) of the reviewer's own, where they can click through the whole lesson with the change in place. The lesson everyone reads is untouched, and the proposal stays open; nothing about it is recorded, because nothing has been decided. Mechanically it is the two features meeting and needing almost nothing new: a merge commits to whatever branch is checked out, so the editor starts a variation named after the proposal and checks it out *before* preparing the merge. What it must not do is record the proposal as being reviewed, since that is what makes the confirm push and mark it merged; a try-out deliberately does neither. Trying the same proposal twice returns to the variation the first attempt made, rather than refusing on the name, so whatever the reviewer did to it last time is still there. ## Landing it without a merge commit When the lesson hasn't moved since the proposal was opened, the merge is a **fast-forward**: the lesson's branch simply moves to the proposal's commit. No merge commit is written, because there is nothing for one to record: no decision was made and no content changed that the proposal's own commits don't already describe. Two conditions, and both are needed. The lesson's tip must be the merge base (the commit-graph half), and the reviewer must have nothing uncommitted in their editor (the half the graph can't see; skipping it would drop whatever they had typed but not yet paused long enough to commit). The proposal is still recorded as merged against the commit the lesson now points at, which is the proposal's own head. If anything that is easier to verify than before. ## Who can do what | You are | Open | Merge | Close | | -------------------------- | ----- | ----- | --------------------------- | | Anyone signed in | Yes | No | No | | The person who opened it | Yes | No | Yes (withdraw) | | The lesson's author | Yes\* | Yes | Yes (decline) | | A **trusted collaborator** | Yes | Yes | Yes | | A moderator/admin | Yes | No | Yes (as with any user text) | \* Only from a fork they own; see below. Opening also needs a display name (so an author never sees a raw email in their review queue) and an account and IP that aren't banned. Merging and uploading a pack check bans too. ### Proposing to your own lesson Out of nowhere, this is a mistake: you can just save, so the Worker refuses a proposal against your own lesson ("This is your own lesson. Save your changes to it directly instead.") rather than creating a request nobody needs. It's allowed when it **carries a fork of that lesson which you own**, that is, `sourceLessonId` names one of your lessons whose `forked_from` is this one. Ownership alone isn't enough, since any other lesson of yours would satisfy it and turn the rule off entirely. Then it means something specific: *here is a copy with changes in it, let me read the diff before it lands.* In practice that is how an **AI assistant over MCP** offers changes. It acts as the account it's signed in with, so changes it proposes to your lesson arrive from your own id. Holding them in the review queue is the entire point: the lesson is untouched until you read the diff and merge it, here or in the conversation itself. See [MCP tools](../mcp-server/tools.md). The web editor doesn't offer this route: it hides **Propose changes** when you are the original's author (`canPropose` in `EditorPage.jsx`), even on a fork made with "Fork into a new lesson". You can then merge it yourself, since you're the author. The notification you get reads "Changes are waiting for your review" rather than naming a proposer, because the account is yours; the proposal's body says what opened it. "Trusted collaborator" is not a new concept: it's the email list the author already manages in the collaboration dialog (`doc.trustedCollaborators`, the same list that auto-admits someone to a live session). See [Live collaboration](./live-collaboration.md). Moderators can **close** a proposal (the same power they have over any other user-submitted text) but not merge one. Merging is authorship, and a moderator's job is removal, not writing under someone else's name. That mirrors comments, where a moderator can delete but not edit. Every one of these rules is re-derived server-side on every request. The frontend checks are only about which buttons to draw, and even those come from the server: the listing endpoint answers `canReview` for the caller, rather than having the browser work it out from a trusted list it shouldn't need to read. ## "Merged" has to be true A reviewer's merge happens in their editor and is pushed to the lesson's history through `PUT /git/:lessonId/pack`, under their own credentials. Only then is the request marked merged. The merge endpoint doesn't take the client's word for that. `mergeCommit` must be the commit the lesson's stored history now actually points at, or the request is refused with a 409 telling the reviewer to save first, and a lesson with no stored history at all can't have been merged into, so that's refused too. A proposal therefore can't be recorded as merged while its changes are quietly dropped, which is the failure that matters: a lesson whose record of what happened to it is fiction. What the endpoint **can't** check is that the commit it's given descends from *this* proposal. That means walking the commit graph, and the Worker holds a lesson's history as an opaque packfile: it has no filesystem to index one into, so the ancestry isn't readable there. A reviewer could merge proposal A and record proposal B against it. That gap is bounded by who can reach it. Only the lesson's author and the collaborators they trust can call it, and they can already rewrite the lesson however they like, or simply close a proposal they don't want, so the exposure is a mislabeled record, not a way in. And nothing is destroyed by it: a merged proposal keeps its packfile, so a merge recorded in error can be read back and settled properly. The order is therefore fixed, and it's the only one that works: 1. push the merged history, 2. save the lesson's document, 3. mark the proposal merged. If the lesson moved on beneath the reviewer while they were reading (its author, or another collaborator, saved), step 1 is refused by the same compare-and-swap that guards every push. Nothing is overwritten, the proposal stays open, and the reviewer merges that in first. Merge and close are both conditional on the row still being `open`, so whichever lands first wins and the other gets a 409 ("already been resolved") rather than, say, closing a proposal out from under a merge. ## Moderation and limits A proposal's title and body are **plain text**, and both go through the same `glin-profanity` check a comment does (`apps/api/src/lib/profanity.js`): the whole thing is rejected with a 422, not censored and kept. Banned users (by name or IP) can't open one, and everyone needs a display name first, so an author never sees a raw email in their review queue. One person may have at most **5 open proposals** against one lesson at a time (`MAX_OPEN_PER_AUTHOR` in `routes/pulls.js`). Titles are capped at 200 characters and descriptions at 4,000 (`PULL_TITLE_MAX`, `PULL_BODY_MAX`); the limits live in `@spelling-creator/core/pulls` and are imported by both the form and the Worker, so the counter and the validation can't drift apart. ## Visibility A proposal is as public as the lesson it targets. On a published lesson, the proposals list and the proposed changes themselves are public: a proposal is part of that lesson's public conversation, like a comment on it. On a private draft, they're as private as the draft. That means what you propose is readable by anyone who can read the target lesson, which is why the submission dialog says so plainly before you send it. ## Worker endpoints | Method & path | Auth | What it does | | ------------------------------------- | ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | | `GET /lessons/:id/pulls` | none, unless a draft\* | `{ "pulls": [...], "canReview": bool }`, newest first (up to 200); unready rows only for their own author | | `POST /lessons/:id/pulls` | `Bearer ` | Opens a proposal (`{ title, body, head, headRef, base, sourceLessonId }`); anyone signed in; the lesson's own author only from a fork they own | | `PUT /lessons/:id/pulls/:prId/pack` | `Bearer ` | Uploads its packfile, the proposer's only. The first must match the head the row was opened with; a later one records a revision | | `GET /lessons/:id/pulls/:prId/pack` | none, unless a draft\* | The packfile; `X-Git-Head` names its tip | | `POST /lessons/:id/pulls/:prId/merge` | `Bearer ` | Records the merge (`{ mergeCommit }`); author or trusted collaborator only | | `POST /lessons/:id/pulls/:prId/close` | `Bearer ` | Closes it; proposer, author, trusted collaborator, or moderator | \* Reads follow the target lesson's own visibility: the single `canReadLesson` rule in `apps/api/src/lib/lesson.js` that also gates `GET /lessons/:id`, its comments and its history. Errors are short plain-text reasons, as everywhere else in this API, so the frontend can surface `res.text()` directly. ## Where it lives | Piece | What it does | | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | | `apps/api/src/routes/pulls.js` | The endpoints above, and every permission rule | | `apps/api/schema.sql` | `lesson_pull_requests` | | `apps/api/src/lib/lessonGit.js` | R2 key layout (`git/pulls//pack`) and the sweeps that delete it | | `@spelling-creator/core/pulls` | The browser client, and the shared length and revision limits | | `@spelling-creator/core/browser/git/sync` | `submitPullRequest` (propose), `prepareProposalReview` (read), `preparePullMerge` (merge) | | `apps/mcp/src/git.js` | The same steps for an AI assistant: fork, propose, and review or merge over MCP | | `apps/web/src/components/ProposeChangesDialog.jsx` | The submission form | | `apps/web/src/pages/lesson/LessonProposals.jsx` | The Proposals tab | | `apps/web/src/pages/lesson/LessonProposal.jsx` | One proposal: its changes, its mergeability, and the hand-off into the editor | | `apps/web/src/lib/git/load.js` | Loads the git engine on demand | | `apps/web/src/components/ChangeSummary.jsx` | The change chips and operation list, shared with the history view | | `apps/web/src/components/PullRequestsSection.jsx` | The list the Proposals tab renders | | `apps/web/src/pages/EditorPage.jsx` | `?pull=&lesson=[&try=1]`, the review, merge and try-out flow; the propose and update buttons | | `apps/web/src/components/MergeDialog.jsx` | Settling conflicts, shared with the fork-sync direction | A pack is swept when its proposal is **closed** (nothing there will ever be merged) and when the lesson is deleted, before the row goes, since the cascade would take the ids with it. A **merged** proposal keeps its pack. Its objects are in the lesson's history by then and so redundant, but only if the merge really did contain them, which is the one thing the endpoint can't check; deleting it would make a mistake unrecoverable to save a few KB.