Repository navigation
[Fix] Carry the applied-patch record onto the ticket with the work, Fixes #236 - #473
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ixes #236 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K
juanmaguitar
force-pushed
the
juanmaguitar/236-carry-applied-patch
branch
from
September 15, 2026 09:00
f9a1767 to
dba00a6
Compare
This was referenced Sep 15, 2026
juanmaguitar
added a commit
that referenced
this pull request
Sep 16, 2026
## Why This is the **stable 1.1.0**. `v1.1.0-beta.1` shipped on 12 September and `v1.1.0-beta.2` on 14 September; between them they were downloaded 33 times across macOS, Windows and Linux, and no issue was filed against either. The v1.1.0 milestone is closed, 21 of 21. Shipping sooner rather than later is itself part of the reasoning: a site created by 1.0 cannot be written to by 1.1, so every day 1.0.1 remains the stable release is another day of sites being created that the next version can only read. Four commits reach this tag that no beta carried: the Linux target list narrowed to the one artifact that is actually published and probed (#467), the apply panel moved above the site's other tickets (#472), the applied-patch record now travelling with carried work (#473), and the sidebar heading renamed (#482). Three of the four are visible on screen, so the artifact gets a manual pass before the release is published rather than the packaging check alone. The version reaches two places a contributor sees, and both must carry the real build: electron-builder embeds it in the artifact names (`wordpress-contributor-toolkit-1.1.0-*`), and `src/logging.js` writes `app <version>` as the log's first line, so a problem report names the build it came from. That is why the bump merges before the tag. ## What changes Only the version. All three package-version fields in `package.json` and `package-lock.json` move together from `1.1.0-beta.2` to `1.1.0`. No dependency versions change, and there is no application behaviour change in this PR. ## How to test this Platforms: any for the suite; macOS or Windows for the artifact check that follows the merge. In the repository root: 1. `npm run lint` is clean and `npm test` passes. 2. `node -e "console.log(require('./package.json').version)"` prints `1.1.0`. 3. `python3 -c "import json;d=json.load(open('package-lock.json'));print(d['version'], d['packages']['']['version'])"` prints `1.1.0 1.1.0`. **What must not have happened:** no dependency version may change. `git diff trunk` shows exactly three changed lines, all of them the root package version. After merge, the release build must produce artifacts named `wordpress-contributor-toolkit-1.1.0-*`, and the first line of a fresh app log must read `app 1.1.0`. ## Risks and limitations No UI change. The risk is a mis-scoped edit in `package-lock.json`, where dependencies also carry version strings; the replacement was confined to the first 2000 bytes of the file, which holds the two root entries, and both files were re-parsed as JSON afterwards. Unlike the betas, this tag becomes `releases/latest`, so it is what the README badge and every "download the toolkit" link resolve to. ## Related Same shape as #397 (v1.0.1), #457 (v1.1.0-beta.1) and #466 (v1.1.0-beta.2). The Gutenberg stack for v1.2.0 is held as a draft at #468 until this tag is pushed, so it cannot reach trunk and ship inside a release it has never been tested in; #479 waits for the same reason. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> Version-only change, three lines. Verified locally on this branch: `npm run lint` clean, `npm test` 1454 pass, 0 fail, 2 expected skips; `git diff --stat` shows two files, three insertions, three deletions; both JSON files parse. Nothing to fix, nothing deferred. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changes. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Why
Apply a patch on trunk, link a ticket, and choose to take the loose edits along. The files move onto the ticket branch, because a carry is
branchpluscheckoutand the worktree is left alone. The record describing them does not: it stays at site level, where a patch applied on trunk is written.So the two halves end in different places. Return to trunk and the panel names the patch, counts its files and offers to revert it, over a tree that holds none of it. Accept, and the revert fails blaming a trunk update or a discard that never happened. The claim is stored, so it greets the contributor every time they come back. Fixes #236.
Reproduced on v1.1.0-beta.2 before writing anything; the steps and the store evidence are on the issue. The detail that shaped this change is that the per-branch model is fine: a patch applied while already on a ticket is recorded against that branch. Only the carry from trunk fails to move the record, so this is one missing read-change-write, not a fault in the layer model.
What changes
carryAppliedPatchmoves the record onto the branch the carry just took the files to, in a singlechangeSiteMeta, called fromsites:set-ticketwhen the carry happened. The clear and the write are indivisible on purpose: split in two, a concurrent write leaves the record in both places or in neither, and neither is worse — it takes the patch text with it, which is what Revert reverses with.It runs after the branch's own meta is written, so the entry exists with its branch point rather than being created here holding one field (#172). A branch with no entry is left alone:
workMetaScopemakes such a ref read from the site anyway, so the record stays visible over the files that are there.Two things fall out that nobody had filed. A trunk update run from a carried ticket used to reach
mergeSiteMeta({ appliedPatch: null })and destroy the only copy of the patch text while the patch was still in the branch's WIP commit; it now clears an already-null field and leaves the branch record alive. AndappliedPatchSubmissionRefusaldid not fire on the branch, so a carried patch's hunks could be submitted as the contributor's own work — the refusal #328 exists for now happens.How to test this
Platforms: either macOS or Windows for the manual pass; any for the suites.
The journey walks the reported path, so run it rather than clicking through it:
npx playwright test --project=journeys tests/e2e/journeys/patch-apply.spec.jswork carried into a ticket takes the applied-patch record with it (#236)applies a patch on trunk, links a ticket taking the edits along, checks both halves arrived, then returns to trunk with Unlink and asserts nothing is offered over the clean tree. It was run red againsttrunk'ssrc/main.jsand green with this change.The unit regressions, in
tests/unit/ipc-wiring.test.cjs:Three cases. The carry moves the whole record, patch text included. Declining the carry leaves it on trunk. And a patch recorded against the branch while the link was running is not overwritten by trunk's older one.
What must not have happened: no record may exist in both places after a carry; the patch
textmust survive the move, or Revert loses what it reverses with; declining the carry must move nothing; and a patch applied on a ticket directly must still be recorded against that branch as before.Risks and limitations
Sites already in the split state are not repaired. Trunk keeps its stale record on those until someone presses Revert. Deliberately out of scope: dropping a site-level record when trunk is clean is a behaviour change on existing installs and deserves its own decision, not a quiet ride-along with a bug fix.
The move makes a carried patch revertible from the ticket, which is what the issue asks for and is a visible behaviour change: submitting that ticket is now refused while the layer is applied.
Related
Fixes #236. #356, which would have dissolved this bug by retiring the app-held layer, is closed as not planned, so the direct fix is the only one coming.
Review outcome (required — see AGENTS.md)
0 [fix here] · 3 [follow-up]. Fresh-context judgement pass against
origin/trunk, dispatched to a subagent with the diff and.github/instructions/code-review.instructions.mdand nothing else from the session that wrote the code.Deterministic layer:
npm run lintclean;npm test1454 pass, 0 fail, 2 expected skips; electron suite 1455 pass; all 30 journeys pass.The reviewer traced the carry path end to end and confirmed
branchRefis the ref HEAD is on, so the record lands wherereadWorkMeta/workMetaScopelook for it, and checked every consumer of the record for a regression: submission refusal, revert, both discards, trunk update, unlink, ticket switch, branch delete and the branch list. None regressed; two improved, as described above.Acted on here:
Deferred, as issues rather than as comments:
mergeBranchMeta+carryAppliedPatchis two writes, so a quit landing between them persists the An applied patch's record stays on trunk after its changes are carried into a ticket #236 state. The window is two microtasks with no I/O. Closing it means folding both into onechangeSiteMeta, which reshapes the handler.updateIncompleteis the same class of bug and does not travel with a carry either, leaving a false "Update incomplete" banner on trunk. Pre-existing and untouched by this change.Screenshots or recording
The change removes something from the screen rather than adding one: after a carry, trunk no longer shows the applied-patch card. The journey asserts it by count.
🤖 Generated with Claude Code
https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K