Repository navigation
chore: cut v0.23.0 - #133
Merged
Merged
Conversation
Nine pull requests since v0.22.0: reviewing a pull request inside the diff, review comments handed to the session as one prompt, a dev-server port per worktree, the session cost readout, the new app icon, and the Windows and session-lifecycle fixes behind them. Minor rather than patch: four of the nine add something a user did not have.
omartelo
added a commit
that referenced
this pull request
Aug 3, 2026
* feat: add custom themes * fix(themes): improve custom theme management * feat(themes): add import template download * feat: adapted template to be more intuitive * fix: disable the Merge button when GitHub will not merge (#132) The merge gate read `mergeable` alone, which reports only whether the branch conflicts. A pull request missing a required review, waiting on a required check, sitting behind its base branch, or one GitHub had not finished recomputing after a push all passed the gate — the button was offered, gh refused, and the toast said "it has conflicts or an unmet requirement" without naming which. Carry gh's `mergeStateStatus` — GitHub's own answer to "can this merge right now" — through to the panel and gate the button on it, with the reason on the hover. UNSTABLE stays mergeable: a failing or pending check that no rule requires does not block a merge on GitHub, and the checks readout already shows it red. The gate lives in `lib/pulls/merge-gate.ts` so the node-only suite can test it; an answer with no `mergeStateStatus` falls back to the old `mergeable` check rather than blocking a merge GitHub would have taken. * feat: read and write pull request reviews inside the diff (#130) * feat: read and write pull request reviews inside the diff Pulls only ever showed the parts of a pull request that travel one way: the checks rollup, the commits, the changed files, and the two writes that end it. The conversation was missing entirely, so a review that asked for a change was invisible here and reading it meant the browser — which is the thing this screen set out to stop. The whole review now happens in the diff. A thread opens inline, in a gap between two lines of the file it is about. The gap is a CodeMirror block widget that owns nothing but an empty element; React fills it through a portal. That split is the point: a comment box holds text somebody is typing and the editor rebuilds its DOM whenever it likes, so the widget is compared by a key that outlives a rebuild and the text lives outside it. Slots arrive through a StateEffect rather than through the view's extensions — resolving a thread must not rebuild the editor under the cursor. Comments of your own are written the same way and held back. The diff's context menu grows "Comment on lines N-M" beside "Inject lines N-M" — the same gesture, the two destinations it has always had to choose between: the session next door, or GitHub. Each comment waits with the others until Submit review sends them as one call, with a verdict and a summary above them. That is what makes it a review rather than a pile of separate remarks, and it is why the pending state is a store backed by localStorage: the panel refetches on focus and while checks run, a collapsed file destroys its editor, and an app update reloads the page. A review written over an afternoon has to survive all three. Everything a line cannot hold goes to a new Conversation tab, in the order it was said: the verdicts, the comments on the pull request itself, and the threads whose lines the branch has since rewritten — GitHub nulls their anchor, so no diff can show them. Settled threads fold themselves away behind a count wherever they render. Behind it: - One `gh api graphql` read brings back reviews, comments and threads together. gh's own `pr view` cannot answer this: --json reviews has a review's body but never the threads under it, and no REST endpoint reports whether a thread was resolved. - A thread carries two identifiers because GitHub needs both — the node id resolves it, the comment's database id is what a reply is addressed to. - A submitted review is one REST call carrying every line comment. Its body goes to gh through a temporary file: --field cannot express an array of objects, and widening every gh call site with a stdin argument to serve this one call is the worse trade. - ApprovePullRequest is gone. Approving is that same submit with no body and no comments, so the button keeps its single click and grows into "Submit review N" the moment a comment is waiting. - A reply posts immediately; a new comment always waits. It is GitHub's own split between answering something already said and starting a review. Deliberately new-file lines only for a comment of your own: a comment on a deleted line is a thread GitHub anchors on the other side of the diff, and that needs a gesture of its own. Verified by driving a headless instance over CDP, since the frontend suite is node-only and cannot render a widget: a draft planted in the store reappears inside the right file's diff after a reload, a real drag-selection produces the menu item, and the composer it opens files the comment as pending with its line span intact. Co-authored-by: Claude <noreply@anthropic.com> * fix: give TooltipTrigger its ref back, and key skeleton bars by position Two React warnings the Pulls screen printed on every visit. Neither was new — the review work just made them loud enough to read. TooltipTrigger was a plain function component, so the ref base-ui hands a trigger nested inside another part went nowhere. The session card does exactly that (a TooltipTrigger inside a ContextMenuTrigger), which is the composition ui/button.tsx already forwards for, and the same trap the frontend's CLAUDE.md names: the only sign is a console warning, while the part quietly never registers its node. SkeletonLines keyed each bar by its width class. The widths are ragged on purpose and repeat, so two bars claimed the same key and React folded them into one — a placeholder stack shorter than the text it stands in for. Keyed by position now; the list is static, so the index is the identity. The Pulls screen's console is empty after both. * feat: let any review thread fold away, not only a settled one A resolved thread could be folded to a line and opened again; an open one could not, so the thread you had just read and dealt with kept as much of the diff as the argument it started — and the only way to get the code back was to resolve something that was not resolved. Folding is reading, not an action on the thread, so it takes no control of its own: the label is the toggle, the way a file's diff header already works, with the chevron saying which way it goes. The two kinds now differ in one thing only, where they start — open by default, closed when settled — and either can be moved from there. Closed, a card is its header alone: it drops the body's padding and carries the comment count, which is the only thing left saying how much is under that line. * fix: keep the line numbers level with their code past a thread Every line below an inline thread showed the number of the line above it: a comment on 839 left the next line reading 841 where GitHub reads 840, and the drift held for the rest of the file. Two causes, both about the height of the gap the widget opens. The gutter draws nothing for a block widget unless it is handed a marker for one, so it had one cell fewer than the content had blocks and every number below sat one line high. An empty marker through lineNumberWidgetMarker buys the widget a cell of its own height and nothing else. That alone left 12px of drift per thread, because the cards inside the gap carried a vertical margin. CodeMirror measures a block widget by the element's own box, and a child's margin falls outside it — height the gutter is never told about. The spacing is the slot's own padding now, and the rule is written where someone would otherwise put the margin back. Verified against the reported shape — a thread on an added line that replaced a deleted one — by pairing every gutter cell with the content block at the same offset: the orphan cell is gone, 840 sits beside 840, and folding the card (which changes its height) keeps them level, since that part CodeMirror re-measures on its own. * fix: stop a discarded comment from moving the one being edited A review pass over the inline-review work, carrying two behaviour fixes and the cleanup around them. Editing a pending comment addressed it by its index in the review. Open the editor on one comment, discard an earlier one, and the indices shift under the open box: saving wrote the text over whichever comment had moved into that slot. The editor holds the comment itself now — the store replaces comments rather than mutating them, so the reference is stable for exactly as long as the comment is. A file's header counted every unresolved thread on it, including the ones the diff cannot anchor: an outdated thread, or one on a line outside every hunk. The header read three where expanding the file showed one. isAnchored already answered this and had no caller outside its own test; the count uses it. PullsFiles built a fresh review object per file on every render, and that object is the identity of each file's CodeMirror decorations — so every render of the screen re-dispatched a transaction into every mounted editor. On a wide pull request that is the shape of the freeze in #92. The per-file objects are memoised, and draftsOnFile takes the comments rather than the whole review, so writing the review summary no longer reads as the line comments having moved. The rest is duplication and magic values: the "t:" slot prefix is one constant behind isThreadSlot, the byline copied across three components is one Byline, the comment field's class is shared with the submit dialog, DiffSelection.range drops a null case the selection can never produce, and CommentOnPullRequest returns its error directly. buildSlots is exported and covered — the sort and the bounds are what stand between an unordered slot list and a RangeSetBuilder that throws on the diff. With it: a thread with no comments left in it, a clear that finds no draft, a non-object inside a stored one, and the array identity the memoisation rests on. * fix: stop a base branch's rules from killing the Merge button GitHub reports mergeStateStatus BLOCKED for any base branch carrying a protection rule or a ruleset. It means "a rule applies here", not "I will not take this" — and a ruleset whose commit-message pattern can only be evaluated against the merge commit leaves an approved pull request with green checks and no conflicts sitting at BLOCKED for good, while GitHub's own button merges it. The gate read BLOCKED as a refusal, so the Merge button was dead on every pull request in a governed repository. Measured on a real one: approved, three checks passed, MERGEABLE, no conflicts, no classic branch protection, one ruleset with a Conventional Commits pattern — and BLOCKED, for an account holding admin on the repository. Invert the gate: name the refusals and let everything else through, rather than allow-listing the states GitHub merges from. A conflict, a base branch that has moved, a draft, a pull request already over, and BLOCKED where reviewDecision says the review the rule wants is still missing. Everything else is offered, and gh's own refusal reaches the screen if GitHub does refuse after all. Two more states go the same way. UNKNOWN is GitHub still recomputing after a push, which is waiting rather than refusing; a state this build has never seen is not evidence of anything. Blocking on either is how a button dies quietly. A merge that is never tried cannot report why it failed, which is worse than the toast this gate was written to replace. The four tests that asserted the old reading assert its inverse, each carrying the reason: the contract changed. The changelog entry described the behaviour being corrected and had not shipped, so it is rewritten rather than answered with a second entry. * style: wrap the merge gate's blocked-review assertions as biome does Formatting only. The frontend gate was reported clean off a truncated biome run, and CI caught what the truncation hid. * feat: offer the merge methods the base branch actually accepts A repository ruleset can pin a branch to one merge method, and squash alone is the common setting. Nothing in a pull request's own fields says so — mergeStateStatus answers whether this merge can happen, never which methods the branch would take — so the menu offered all three and GitHub refused two of them after the click, with a sentence that named none. Read the base branch's rules through `repos/{owner}/{repo}/rules/branches/{branch}` and build the menu from what comes back. The read is keyed by the branch rather than by the pull request, which is what the answer is about: every pull request onto the same base shares one. It is not polled and does not refetch on focus — a ruleset is a repository setting somebody changes once a quarter, and all it narrows is a menu. Everything unknown widens rather than narrows: no ruleset, an unreadable answer, a failed call, or a ruleset naming only methods lich cannot ask gh for. A menu missing an option lich could have offered says nothing about why it is missing; one that offers an option GitHub turns down at least answers with gh's own refusal. The endpoint returns [] for a branch it has no rules for and for a branch that does not exist, so a wrong path escape and an ungoverned branch are indistinguishable from outside — which is also why the degradation is safe: either reads as "no rule known". Merging over red CI now says so before the click. A failing check that no rule requires does not block a merge on GitHub, and lich offers it for the same reason — the call is the reader's. It offered it in silence, though, with the red a line away in the status row. The button carries the count of failing checks, and its hover says GitHub will take the merge anyway. * fix: size a dropdown menu by its own text, not by its trigger Every menu carried w-(--anchor-width), so it was exactly as wide as the button that opened it. That is a select's behaviour — a select's popup stands in for its field and should match it, which is why ui/select.tsx keeps it — but a menu is sized by what it says. Hanging off a small control, the items wrapped: "Squash and merge, edit message…" came out over three lines under a Merge button, and New Session and the pull request filter did the same. The popup sizes to its content now, with min-w-32 as the floor, and an item is one line: a command that wraps reads as two. Items also carry a little more room around the text, which is the other half of what made the narrow menus hard to read. Both call sites that already fought the default keep working: the notifications menu pins its own width and truncates inside each item, and the New Session menu's cap is wider than the provider names ever reach. * fix: keep a price refresh from outliving the test that started it CI failed in internal/pricing with "TempDir RemoveAll cleanup: unlinkat /tmp/TestUnknownModelIsUnpricedNotFree…/001: directory not empty", on a commit that touched only the frontend. Rate answers a miss and spawns the refresh that may teach it, and the refresh ends by writing the cache. TestUnknownModelIsUnpricedNotFree is made of misses, so it returned with that goroutine still running: the write landed inside the temp directory's own removal. The test that fails is whichever one loses the race, never the one that leaked — which is why it read as unrelated to the change under it. Proven rather than guessed: a probe that waits after the assertions found the goroutine writing prices.json into the directory the cleanup was about to remove. It would not reproduce here at -count=200, with -race, or across 40 cold processes — the window is the microseconds between RemoveAll's readdir and its rmdir, which a loaded runner widens and this machine does not. Rate's goroutine joins a WaitGroup, and awaitRefresh drains it. Nothing in the app waits — the caller already had its answer, and the refresh only teaches the next turn — so this exists for the suite, where a component still writing to disk after its owner is gone is a flake looking for a test to fail. The sibling tests already avoided this by hand: the one below reaches for lookup instead of Rate, with a comment saying why. That worked per-test and left the next one to rediscover it. TestARefreshDoes NotOutliveItsTest locks the drain itself, so deleting the WaitGroup fails a test rather than a future CI run. Verified with the package at -race -count=200, and the whole backend suite at -race. --------- Co-authored-by: Claude <noreply@anthropic.com> * chore: cut v0.23.0 (#133) Nine pull requests since v0.22.0: reviewing a pull request inside the diff, review comments handed to the session as one prompt, a dev-server port per worktree, the session cost readout, the new app icon, and the Windows and session-lifecycle fixes behind them. Minor rather than patch: four of the nine add something a user did not have. * feat: merge past a rule that is holding a pull request back (#134) * feat: merge past a rule that is holding a pull request back A ruleset can hold a pull request that has nothing wrong with it. Measured on a real one: approved, three checks passed, MERGEABLE, no conflicts, and mergeStateStatus BLOCKED — because the repository requires commit messages to match a Conventional Commits pattern, and the message the squash would write does not. Nothing on the pull request says that, and nothing lich showed came close: the toast said "it has conflicts or an unmet requirement", so the reading was conflicts a pull request did not have. There was also nothing to do about it from here. gh checks mergeStateStatus itself and refuses before it ever calls GitHub, so the "edit message" flow — the one thing that would fix a message-pattern rule — died at the same client-side check as the plain merge. Three changes, smallest first. gh writes one sentence for two very different failures and puts the difference in its tail. gherror.go matched only "not mergeable" and claimed conflicts either way, which is the wrong half of the time. Match the tails: "the merge commit cannot be cleanly created" is a conflict, "the base branch policy prohibits the merge" is a rule. The bare match stays last for a wording neither one catches. BranchRules already fetched the whole ruleset payload and threw away everything but allowed_merge_methods. The commit-scoped pattern rules are in that same answer, so keep them: message, author email and committer email, with the operator and the pattern each one tests. Where GitHub answers BLOCKED and one exists, the Merge button's hover and the failure name what the base branch requires. Branch and tag name patterns are left out on purpose — they gate a push, never a merge, and naming them would send the reader after the wrong rule. It is a note, never a verdict. GitHub publishes no "why is this BLOCKED", so the honest answer is where to look. Re-running the pattern here would be a guess dressed as an answer, in a regex dialect that is not the one GitHub evaluates. And the merge itself takes --admin, GitHub's own administrator override — which is also the only way gh calls GitHub at all once a rule holds the merge, so a real rule violation finally reaches the screen. The menu offers it only on BLOCKED and BEHIND, which is gh's own allowsAdminOverride: an override skips a rule, it does not resolve a conflict or finish a draft. Where it is offered, the ordinary items are gone rather than shown beside it — gh has already refused those from the client, so a menu carrying both would put the dead click next to the working one. Without the privilege the dead click stays, because it is what carries gh's refusal and the rule note back. Whether the account may bypass is read, not assumed: `gh repo view --json viewerPermission`, and only ADMIN counts. This is the one answer in the merge gate that fails closed. Everything else there would rather offer an option GitHub turns down than drop one it would have taken, because a refusal at least says what happened — but a bypass reads as a privilege you hold, so one that is not there turns GitHub's refusal into what looks like a broken button. A ruleset can name any role as a bypass actor, so an account that may bypass without administering the repository does not get the option; reading each ruleset's bypass list is the upgrade path. A failed permission read costs the bypass and nothing else — the rules are worth drawing on their own. The tests that changed did so because the contract did: mergeArgs took a parameter, and the gherror case asserted the single sentence that no longer exists. fakeGH grew per-call answers and a call history, which the two-call read needs. Not smoke-tested in task dev: seeing the bypass menu needs a pull request sitting at BLOCKED, and this repository has none. The parser was run against the real ruleset payload that prompted this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: never offer a bypass over a conflict a stale merge state hides canAdminOverride read mergeStateStatus alone, so BLOCKED reported over a conflict — the case mergeBlockedReason already reads both fields for — took the bypass past every guard: the Merge button went live, the menu read "Squash and merge, bypass rules", and the click failed on a conflict no override resolves. That is the dead click the fail-closed design exists to remove, and the shipped promise is "never over a conflict or a draft, which no override resolves". Read mergeable too, as the rest of the gate does. Also names what the menu's all-or-nothing rule costs: BLOCKED is what GitHub answers for any governed base branch, rules met or not, so an administrator's ordinary approved merge goes out with --admin and GitHub records it as an override. gh leaves no other way through, and nothing on screen or in the code said so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix: cut a review thread's snippet to the lines it comments on (#135) GitHub sends a review comment with the whole hunk it sits in, and the Conversation tab printed it verbatim: on a file the branch adds, a remark about ten lines arrived under a hundred and twenty of them. The snippet is now parsed and cut to the span the thread covers — the range for a multi-line thread, the line plus a little context for a single one — with a fallback to the hunk's tail for a thread GitHub re-anchored, since the hunk always ends on the line the comment was filed on. The lines carry the diff viewer's own green and red, numbered from the hunk header, and the header's file reference reads the whole range. The diff colours lose their cm- prefix along the way: they are no longer the CodeMirror viewer's alone. Threads GitHub marks outdated null their startLine, so the query asks for originalStartLine too — without it an outdated range collapsed to one line and took the snippet with it. * fix: open a session on a fork PR that allows maintainer edits (#136) Every pull request from a fork was refused a session, on the grounds that an agent's commits would have nowhere to push. GitHub asks the author that exact question when the pull request is opened, and "allow edits by maintainers" is on by default — so the blanket refusal turned a permission that was already granted into a dead button. The gate now reads maintainerCanModify alongside isCrossRepository, in both places that held the rule: CreateWorktreeFromPR, which refuses before the worktree exists, and the header button, which greys out with the reason on its tooltip. Only a fork that withholds the permission is still refused, and both messages now name which of the two it is. Verified by hand against a fork PR: gh pr checkout inside a detached worktree sets branch.<name>.remote to the fork, so a push goes back to the branch the pull request is on. * fix(themes): address review feedback * fix(themes): save the template through the native dialog and tighten validation The template built its JSON in the page and pushed it through a synthetic anchor, so it landed wherever Chromium sends downloads — a window with no download shelf, and a toast that claimed success without knowing it. It is now themes/template.json, embedded beside the bundled pair and written by themes.SaveTemplate to the path the native save dialog returned, so the toast can name where the file went. Import already went through the picker; this puts both directions on the same seam. Color validation moves from a denylist to what each consumer actually parses. App values take hex, a CSS color name or a color function, which leaves out url(), image-set(), var() and attr() by construction rather than by blocklist. Terminal values must be hex: xterm parses those directly, and everything else goes through a round-trip that fails on translucency and is swallowed into a silent fallback, applying a theme halfway with no error shown. A theme id can no longer be a Windows device name. It names a file, and con.json still addresses the console. The id regex leaves the frontend. reconcileThemeSelections already checks the only thing that decides anything — membership — once the theme list lands, so the shape check ahead of it changed no outcome while keeping a third copy of a rule the backend owns. A Select renders its raw value until the Root is handed the value to label map, because items register themselves only once the popup has been opened. Both theme pickers pass it now, so a closed trigger reads System, not system. Also: the documented example and the template answer to the real validator in tests, findTheme is inlined into resolveTheme, and normalizeTheme only clones. --------- Co-authored-by: martelo <125408621+omartelo@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: omartelo <meopedevts@proton.me>
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.
Moves the
[Unreleased]entries under av0.23.0heading and refreshes thecompare links. No code changes.
What lands
Nine pull requests since v0.22.0 (#123, #124, #125, #126, #127, #128, #129,
#130, #132). Every commit in the range maps to at least one changelog entry and
every entry maps to a commit — 4 Added, 1 Changed, 12 Fixed.
Minor rather than patch: four of the nine add something a user did not have —
reviewing a pull request inside the diff, review comments handed to the session
as one prompt, a dev-server port per worktree, and the session cost readout.
Deliberately not in this release
#131 feat: add custom themesis left out: it is unreviewed, and it conflictswith main, so it does not merge as it stands.
Test plan
gofmt -l .andgo vet ./...cleango test -race ./...green, no data racespnpm check,tsc --noEmit,vitest run(587 tests, 57 files),vite buildv0.23.0and watch the release pipeline🤖 Generated with Claude Code
https://claude.ai/code/session_01WVfeykRkJkTPTzSRWLYSKP