Skip to content

[Add] Apply Gutenberg pull requests to a Gutenberg site (#251) - #264

Open
juanmaguitar wants to merge 1 commit into
juanmaguitar/gutenberg-playground-servefrom
juanmaguitar/gutenberg-work-item-and-pr-flow
Open

[Add] Apply Gutenberg pull requests to a Gutenberg site (#251)#264
juanmaguitar wants to merge 1 commit into
juanmaguitar/gutenberg-playground-servefrom
juanmaguitar/gutenberg-work-item-and-pr-flow

Conversation

@juanmaguitar

Copy link
Copy Markdown
Collaborator

Why

Part of #251. With a Gutenberg site now cloning, building and serving (#255, #261), the remaining
half of "see what a Gutenberg pull request actually does" is reading that PR and applying it to
the checkout. Both were hard-wired to WordPress Core, in ways that fail silently rather than
loudly on a Gutenberg site.

Stacked on #261. Part of the Gutenberg feature that merges atomically — see #255.
Do not merge alone.

What changes

  • Patch path layout follows the project. Patches were always rewritten into wordpress-develop's
    src/ layout — a rewrite that exists because a patch attached to a Trac ticket years ago still
    names wp-admin/…. A Gutenberg diff is already repo-relative (packages/…), and a top-level
    wp--prefixed path in one would be moved under a src/ directory Gutenberg does not have.
    parsePatchFiles takes a layout, applyPatchToDir passes it through, and preview, apply and
    revert all resolve it from the site
    so they cannot disagree about where a file lives.
  • Pull requests come from the site's own upstream. parsePrRef, fetchLinkedPrs and
    fetchPrDiff take the repository, so a Gutenberg site lists and fetches WordPress/gutenberg
    PRs — and still refuses one from the other project, whose diff would not fit its checkout.
  • "Which PRs belong to this work item" is per provider. A Core PR cites a Trac URL; a Gutenberg
    PR cites its issue as #1234. Added bodyCitesIssue / citesWorkItemFor, keeping the
    verification narrow — #658 must not match inside #6580, and a bare number is not a citation.
  • The linked-PR cache key now includes the repository: Trac ticket Let a site know which Trac ticket it is being used for #123 and Gutenberg issue Let a site know which Trac ticket it is being used for #123
    are different work items and shared one entry before.

Every new parameter defaults to Core's behaviour, so a site with no project type is unchanged.

Deliberately not here: the work-item side is still Trac-only (entering a GitHub issue, the
ticket panel copy, the Trac attachments panel) and outbound PR authoring still targets
wordpress-develop. Those are the next PR.

How to test this

Platforms: any.

The mechanism, without a full Gutenberg build — this is the failure the change prevents:

const { applyPatchToDir } = require('./src/patch-apply');
// tree: wp-login.php at the root; patch: --- a/wp-login.php
await applyPatchToDir({ dir, patchText });                            // ok:false — looks for src/wp-login.php
await applyPatchToDir({ dir, patchText, layout: 'repo-relative' });   // ok:true

(I ran exactly this, plus a packages/block-library/src/index.js diff applying cleanly to a
Gutenberg-shaped tree.)

Full flow (a real Gutenberg site, from #255/#261):

  1. On a Gutenberg site, paste a https://github.com/WordPress/gutenberg/pull/<n> URL into
    "apply a PR". → It is accepted, the diff is fetched from WordPress/gutenberg, and the preview
    lists packages/… paths unchanged (no src/ prefix).
  2. Apply it, then start the dev server. → The change is visible in the block editor.
  3. Revert it. → The tree returns to trunk; the revert resolves the same paths the apply did.
  4. Paste a wordpress-develop PR URL into the same box on that Gutenberg site. → Refused by name.

On a Core site (must be unchanged): link a Trac ticket, list its linked PRs, apply one, revert.
Identical to before — same src/ rewrite, same wordpress-develop source.

What must not have happened:

  • A Gutenberg diff must never be src/-rewritten — that writes to paths the contributor was
    never shown, and the preview would disagree with the apply.
  • A Core patch must still be rewritten (an old wp-admin/… patch still has to land in src/).
  • The revert must use the same layout as its apply, or it reverses hunks against the wrong files.

Automated: npm run lint clean; 822 tests pass. New coverage in test/patch-plan.test.cjs
(both layouts, incl. packages/ untouched either way and unknown-layout → Core),
test/patch-sources.test.cjs (bodyCitesIssue precision, citesWorkItemFor, per-repo
parsePrRef), test/github-prs.test.cjs (repo/provider actually reach the query), and
test/ipc-wiring.test.cjs (preview and apply and revert all pin their layout, per type).

Risks and limitations

  • bodyCitesIssue is looser than its Trac counterpart. bodyCitesTicket requires a host and
    /ticket/, so a stray number cannot pass. GitHub's convention has no such anchor, so
    Follow-up to #1234 (a PR number) or otherorg/repo#1234 also count as a citation. The blast
    radius is a false entry in the linked-PR list, which a contributor would then have to click.
    Tightening it (requiring a closing keyword, rejecting an owner/repo#id prefix) is a design call
    worth its own pass — flagged rather than guessed at here.
  • Old cache entries are orphaned, not migrated. ticketPatches:<id> keys are inert after the
    key change; nothing reads them. The visible effect is one-time: the first offline/rate-limited
    lookup after upgrading has no stale list to fall back on. A one-line sweep could prune them.
  • The ticket panel is still Trac-shaped for a Gutenberg site (it says "Trac ticket", offers "Show
    Trac attachments"). That is the next PR's scope, and it is the one remaining path that could hand
    a Gutenberg site a Core patch — worth knowing while the stack is in flight.

Related

Part of #251. Stacked on #261.


Review outcome (required — see AGENTS.md)

1 [fix here] · 3 [follow-up] — the fix-here is fixed. Ran the review in
.github/instructions/code-review.instructions.md; judgement pass in a fresh subagent. Lint clean,
822 tests pass.

  • 🟡 Tests · [fix here] · fixed — the preview side pinned its layout but the apply side did
    not, so deleting layout from either applyPatchToDir call would have shipped green while a
    Gutenberg diff went through Core's rewrite. Added layout assertions to the forward-apply and
    revert wiring tests plus a Gutenberg apply case. Also closed the related gap the reviewer noted in
    passing: fetchPrDiff is now injectable and test/github-prs.test.cjs asserts repo/provider
    reach the request and the citation test (previously a typo there shipped green, since the
    IPC-wiring tests stub the module out).
  • 🟡 Security · [follow-up] — bodyCitesIssue precision (documented in Risks). The escape and
    digit-stripping were verified correct; no injection or ReDoS.
  • 🔵 Architecture · [follow-up] — orphaned cache keys (documented in Risks).
  • 🔵 Architecture · [follow-up] — the Trac-shaped ticket panel on a Gutenberg site; confirmed
    deferred
    to the next PR in the stack (documented in Risks).
  • Verified clean: Core byte-identity of parsePatchFiles() across every branch (rename, binary,
    normal, /dev/null) with an unknown/undefined layout still taking the Core rewrite; preview/apply
    cannot disagree (projectType is immutable after wordpress:setup); the changed fetchPrDiff
    IPC signature has no missed call sites; repo is always a registry constant, never renderer
    input.

@juanmaguitar juanmaguitar added the gutenberg-contributions Support Gutenberg as a contribution target (#251) label Aug 11, 2026
@juanmaguitar
juanmaguitar force-pushed the juanmaguitar/gutenberg-work-item-and-pr-flow branch from 941aada to c00c47f Compare August 11, 2026 10:57
With a Gutenberg site now cloning, building and serving, the remaining half of
"see what a Gutenberg PR does" is reading the PR and applying it. Both were
hard-wired to WordPress Core:

- Patch paths were always rewritten into wordpress-develop's src/ layout. That
  rewrite exists because a patch attached to a Trac ticket years ago still names
  `wp-admin/…`, but a Gutenberg diff is already repo-relative, and a top-level
  `wp-`-prefixed path in one would be moved under a `src/` directory Gutenberg
  does not have. parsePatchFiles now takes a `layout`, applyPatchToDir passes it
  through, and preview, apply and revert all resolve it from the site so they
  cannot disagree about where a file lives.
- Pull requests were always read from WordPress/wordpress-develop. parsePrRef,
  fetchLinkedPrs and fetchPrDiff now take the site's upstream, so a Gutenberg
  site lists and fetches WordPress/gutenberg pull requests — and still refuses a
  PR from the other project, whose diff would not fit its checkout.
- "Which PRs belong to this work item" is a different question per provider: a
  Core PR cites a Trac URL, a Gutenberg PR cites its issue as `#1234`. Added
  bodyCitesIssue and citesWorkItemFor; the verification stays narrow (`#658`
  must not match inside `#6580`, and a bare number is not a citation).

The linked-PR cache key now includes the repository: Trac ticket #123 and
Gutenberg issue #123 are different work items and shared one entry before.

Every new parameter defaults to Core's behaviour, so a site with no project type
is unchanged and needs no migration.

Verified by hand that a `packages/…` diff applies to a Gutenberg-shaped tree
under repo-relative, and that the same patch fails under Core's layout — which
is the bug this prevents.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gutenberg-contributions Support Gutenberg as a contribution target (#251)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant