Conversation
- git:apply-patch rejects a sitePath that is not a registered site before writing anything, the same gate sites:set-ticket uses (#8). - Persisting the revert record is now part of the apply transaction: if the store write fails the apply is undone, and if that also fails the result says the patch is applied-but-untracked rather than reporting a clean failure (#7). - git:preview-patch surfaces a worktree-inspection failure instead of reporting "no collisions" when it could not look (#9). - git:discard-changes clears the applied-patch record with the reset, so a later trunk-update network failure cannot leave a revert banner for a patch that is already gone (#6). - git:apply-patch is now a WIRED handler with real tests (guard, reverse lookup, delegation, done event, metadata) via fakeSettingsStore, instead of a NOT_REACHABLE hole (#13). Not gating git:preview-patch: it is read-only, and the registered-site check would add a store dependency to a handler that has none. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The patch used to sit under the destinations, which put the choice above the thing being chosen for: a contributor scrolled past three cards to read their own code, then scrolled back up to act on it. The code is what they opened the screen to look at and the largest thing on it, so it takes the room, and where it can go stands beside it — in view the whole time they are reading rather than something to scroll back to. This is the shape of an earlier take on the same screen (#6, unmerged since November), rebuilt on the destinations this app has now. Three things fell out of the restructure: The Save and Copy buttons move from floating over the diff into the column header. Floating was survivable across the full width and covers the first line of a hunk once the pane is a column. The columns and the breakpoint are CSS, not inline styles. Flex-wrap was tried first and cannot express this: wrapping keeps both columns on one flex line whose height the modal fixes, so the wrapped sidebar had nowhere to go. Stacking has to change what scrolls as well as what sits where — side by side the sidebar scrolls alone and the diff stays put; stacked, the pair scrolls as one and the diff keeps a definite height so it cannot collapse. And a pre-existing bug the new layout made visible: the diff pane is height:100% with 12px of padding and content-box sizing, so it was always 24px taller than its container. Harmless while it spanned the modal and the overflow fell off the bottom; beside a sidebar it sat on top of the destinations. Measured, not eyeballed: the pane now ends exactly on its column, and stacked there are the full 16px between it and what follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The patch sat under the destinations, which put the choice above the thing being chosen for: a contributor scrolled past three cards to read their own code, then scrolled back up to act on it. The code is what they opened the screen to look at and the largest thing on it, so it takes the room, and where it can go stands beside it. This is the shape of #6, open since November and unmerged, rebuilt on the destinations this app has now. Three things fell out of the restructure: - Save and Copy move from floating over the diff into the column header, where they cannot cover the first line of a hunk. Copy now says that it copied, and says so when it could not. - The columns and the stacking breakpoint are CSS with a media query. Flex-wrap was tried first and cannot express this — stacking has to change what scrolls as well as what sits where. - A pre-existing bug the layout exposed: the diff pane is `height: 100%` with padding and content-box sizing, so it was always 24px taller than its container. Harmless while it spanned the modal; beside a sidebar it sat on top of the destinations. Also fixes a false positive in the staleness guard reported from a real run — it refused a pull request over a file upstream had not touched since 2021. The base blob is now computed from the checkout instead of asked of the fork, which cannot answer for a commit it has not heard of. Toward #186. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
For the record, since this has been open a while: the flow it proposed has landed in trunk, and its layout with it.
Two things this branch got right that took the new implementation a day of testing against production GitHub to rediscover: device flow is the right sign-in, and Where the two differ:
And one thing this branch handled that trunk does not: skipping Leaving this open rather than closing it; that call belongs to a maintainer. Thanks for the groundwork — the layout in particular was right, and it is what the app ships now. |
Adds a UI path for connecting with GitHub and creating a PR: