Do not merge — test the simplified v1.0-rc2 ticket-work stack - #312
Closed
juanmaguitar wants to merge 25 commits into
Closed
Do not merge — test the simplified v1.0-rc2 ticket-work stack#312juanmaguitar wants to merge 25 commits into
juanmaguitar wants to merge 25 commits into
Conversation
juanmaguitar
added
the
ticket-work
9 tasks
juanmaguitar
force-pushed
the
v1.0-rc2
branch
10 times, most recently
from
August 12, 2026 14:07
6f715c5 to
5040f6b
Compare
juanmaguitar
changed the title
juanmaguitar
force-pushed
the
v1.0-rc2
branch
2 times, most recently
from
August 12, 2026 14:24
4eb1c1b to
08af74e
Compare
Collaborator
Author
|
The integration branch was rebuilt once more after the final review. Test the artifact for commit The last two corrections are in #318:
Local validation on the rebuilt stack: lint clean; 1005 tests pass in Node and Electron’s bundled Node. GitHub checks and the signed artifact for this exact head must finish before manual testing. |
Collaborator
Author
|
The integration branch has been rebuilt after updating #324 so both visible discard links share the same blocked-state tooltip. Previous artifacts are obsolete. Test only a signed artifact whose source commit is |
Adminer became an anchor, so it needs the preventDefault a Button never did — nothing binds will-navigate on the main window. Pins that, its href coming from adminerUrl, and that the old button is gone.
Three pages described an "Open Adminer" button in the action row. Two of those sentences are now false: the widget and its name are both gone. Also drops "Append /wp-admin/ to the site URL" from running-the-site, which the wp-admin link made unnecessary a PR ago. Part of #249.
The existing assertions covered the widget, not the placement — an anchor rendered back inside the action-button row passed all of them, which is the state this change exists to move away from. Part of #249.
Two things from review: The rebase onto trunk resolved the action-row conflict by keeping both sides, so the old Open Adminer button came back alongside the new link and Adminer had two entry points. The test for #249 caught it. Removed the button; the row that #247 reworked keeps only actions. "Adminer" alone says nothing to a contributor who has not met it, so the link now reads DB inspect (Adminer) — the name stays in parentheses because the docs and the project use it. Guide prose follows.
Two useEffects were both keyed off tracTicket: one auto-triggered a scrape right after a ticket was freshly linked, the other bumped scrapeGenRef to mark later scrapes as stale. React runs same-component passive effects in declaration order, so the auto-scrape fired first and captured the previous ticket's generation — its finally guard then never matched, leaving the "Reading ticket..." spinner stuck with no way to recover short of switching tasks again. Merges the two effects and bumps the generation synchronously at the top, before anything below can trigger a scrape. Fixes #299 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Self-review caught a regression in the first fix: merging the generation bump into the auto-read effect widened its deps to [tracTicket, isActive, loadTicketPatches], so an isActive toggle (switching site tabs and back) now bumped the generation of an in-flight scrape that had nothing to do with a ticket change — reproducing the #299 stuck-spinner symptom through a different trigger. Reverts to two effects, as on trunk, but with the bump declared first so it still lands before the auto-triggered scrape on an actual ticket change. Adds a test pinning the bump's effect to [tracTicket] alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The pre-apply warning ("You have your own edits to ...") asked
`collectDirtyFiles`, which is HEAD-relative. Under the ticket-as-branch
model a ticket's work lives in its parked WIP commit, so HEAD is the
ticket's last saved state — and a ticket that has been left and resumed
has a worktree matching it exactly. The warning went silent on precisely
the tree it exists to protect, and spoke up about files edited back to
what the base holds.
`git:preview-patch` now asks `collectUnsubmittedFiles` — the same
base-relative walk the patch itself, the PR, the card's note and
`git:discard-to-base` are measured with. On trunk `patchBaseOid` answers
null and the walk falls back to HEAD, so trunk sites keep their answer.
Fixes #301
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A failed apply narrated an open pull request from its state alone: written against an older trunk, and here is a button to ask its author for a rebase. That was never checked against the checkout it had just tried to apply to, so a change that would not fit because the contributor's own work sits in the same lines — the ordinary shape of a resumed ticket — sent a newcomer to a stranger to ask for work that would not help. The app applies two-way, without the pull request's base, so it cannot prove which side any single failed region belongs to. It does not need to: it already knows which files this ticket has work in, and #302 makes that measurement base-relative. The preview's collision list is now threaded into describeApplyFailure, and the framing branches on it — own work in the way, a pull request behind trunk, or both said together, since picking one would be guessing again. Closed, merged and already-in-trunk pull requests are untouched, and so are loose patches, which have no author to misblame. Fixes #303. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deciding what a changed file is, the generator compared the two sides' normalized text before consulting the status codes that say which side the file exists on. An added empty file and a deleted already-empty file both render '' on both sides, so both came out "unchanged": dropped from the patch, and uncounted by the card's unsubmitted-work note, which reads the same classification. The contributor was told the ticket had no changes at all. Whether a file was added or deleted is not a question about its contents. The walk already carries the answer — `inHead` and `inWorkdir` come from statusMatrix — so consult them first, the rule #85 established for whole-file reads. An empty file has no line to diff, so jsdiff emits a section with no hunk, which `git apply` rejects as garbage — taking the whole patch and every unrelated file with it. Git's own extended header is what carries an empty file, so that is what the generator emits, and the reader classifies it from `/dev/null` the same way it does a hunked section. That gives the applier a deletion with no pre-image to match, where its existing hunk check cannot run. A deletion with no hunk claims the file was empty, so removing one that has content since would discard work no hunk mentioned. It now refuses, as `git apply` does. Fixes #311 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A ticket's base — the trunk snapshot its branch was born at — is what the patch flow measures against: the contributor's own work, the generated patch, and the files the pre-apply warning names (#301). It was read through a helper that answered with an oid or null, folding four situations into two, both folds silent. A ticket branch with no recorded base got today's `refs/heads/trunk` substituted for it, which an update has very likely moved past the point the branch was really born at. And any throw came back as null, which every caller reads as "this site is on trunk" — sending the measurement back to HEAD, the exact reading #301 exists to remove, reached through the failure path. So the status is the answer: `patchBase` returns {status: trunk|recorded|unrecorded|unreadable, baseOid}, and the callers read the last two as themselves. An unrecorded base still measures against today's trunk, because there is nothing better available, but every surface that shows that answer hedges it — the preview's warning, and the handoff patch's Base line. An unreadable base refuses: no patch, no pull request, no discard, and a preview that fails through the fail-closed path it already had rather than reporting a clean tree. Recorded and trunk sites are untouched, which the tests pin. Fixes #308. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A ticket is a branch (#108): trunk, at most one applied patch or pull request, and the contributor's own edits. The branch holds that faithfully; the app's bookkeeping did not. `appliedPatch` was remembered only as something to undo, so everything the UI said about the ticket was read off the bare diff — and every file the applied pull request brought was announced as the contributor's own writing. Attribution. The record already stores which paths the patch touched, so the pre-apply warning now names two owners instead of one: a file the layer brought and nobody has edited over is described as coming from it, and a file the contributor has also worked on keeps naming their work, which is the answer that decides what they do next. Neither is dropped from the warning. The same split reaches the failure narration: a revert that will not come back out is the contributor's own edits sitting on lines they applied, never a pull request its author should rebase. Absorption. `revertable` used to mean "we stored the text". It now means what the banner actually asks — can this layer still be lifted out of this checkout, right now — measured by `diagnoseRemoval`, which reverses the stored patch through the same `resolveFile`/`diagnoseHunks` machinery the apply path uses and writes nothing. Once the contributor's edits are on the patch's own lines it is absorbed: Revert stands down, and the honest exits are saving a copy of the work and discarding the ticket to its base, which on this project is a normal way forward rather than a defeat. Nothing persists an "absorbed" flag, so undoing the overlapping edit brings Revert back on its own — and absorption never frees the one-patch slot, because the record survives as provenance. The banner's decision lives in `src/renderer/applied-layer.cjs`, unit tested directly; the patch text still never crosses IPC. Fixes #306 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Integration rebuilt with #268 added to the previously validated v1.0-rc2 stack. New exact head: |
Collaborator
Author
|
Integration complete. The approved artifact at commit was manually validated on macOS and Windows. The final commit is , and its Git tree is exactly identical to the approved artifact. This draft was test scaffolding and must not be merged. |
Collaborator
Author
|
Correction to the commit references in the preceding close comment (shell formatting removed them):
The integration branch has now been deleted as planned. |
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
2 participants
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
This branch exists to be built and driven, not merged. Individual fixes are reviewed in their own PRs; this draft produces one signed artifact for the complete 1.0 ticket-work flow.
What this is
Current
trunkplus exactly these simplified PR stacks:Explicitly excluded: #319 (remote staleness probe), #320 (heavy carry detection) and #321 (automatic carry). #321 also has an unresolved concurrency risk and is not merge-ready.
This PR itself must never be merged. Its branch is rebuilt and force-pushed as the test artifact.
How to test this
Install the artifact from the newest build for commit
4eb1c1b.Platforms: test the signed build on the platform you intend to ship; the logical flows are platform-independent.
What must not have happened: no edit lost at a switch, Revert, discard or update; no stale notice naming the next ticket; no partial patch apply; no silent dependency rebuild.
Report findings on the individual PR that owns the behavior.
Risks and limitations
Local validation is green: lint plus 1003 tests in both Node and Electron’s bundled Node. GitHub checks and the signed artifact for the final force-push must finish before manual testing begins. Manual desktop testing remains the release gate.
Related
Tracking issue #309. Individual PRs carry
v1.0-rc2.