Pick a branch or an issue when creating a workspace - #35
Merged
Conversation
Backend half of issue #12. The web client's create-workspace form needs real data behind its branch field and a way to start a workspace from an issue, so the bridge gains: - `GET /repos/:name/branches`, answered with `ls-remote` rather than the repo's provider: repos default to `provider: "custom"`, for which `providerFor` throws 501, so a provider-backed listing would be dead for most registered repos. - `RepoProvider.linkBranchToIssue`, which creates a branch and records it as the issue's linked development branch. GitHub exposes that linkage through GraphQL alone, so the provider grows a small `graphql` helper that also treats a 200 carrying `errors` as a failure. - `POST /workspaces` accepting `issueNumber` in place of `branch`, with the branch name derived by the new shared `issueBranchName` — shared so the client can preview the name the bridge will authoritatively pick. Pre-review: the review panel runs against this commit.
Review fixes for 03a08ec. The two that matter were each found by two reviewers independently: - `ls-remote` ran with no prompt suppression and no deadline. git opens /dev/tty directly for credentials, ssh passphrases and unknown host keys, bypassing the pipes it was handed, and the bridge normally runs in an operator's foreground terminal — so a private repo parked the probe on a prompt nobody answers and hung the request behind it for good. Probes now run non-interactively under a 15s deadline. - `linkBranchToIssue` had no "already linked" path, so the retry the create-site comment promised did not exist: any failure after a successful link left that issue permanently uncreatable. GitHub signals a duplicate two different ways — a populated `errors` array, or HTTP 200 with a null `linkedBranch` — and both are now read as "already there", falling back to whatever the issue is linked to, or to the branch of that name. Only a genuine absence is an error, and it is a 409 naming the collision rather than a 502. Also: pin the reservation-before-provider ordering with a gated concurrency test (hoisting the provider call now fails it), raise the issue listing off GitHub's 30-item default, keep credentials embedded in a repo URL out of the 502 body, and correct the bridge API reference, which this feature had made false.
Client half of issue #12. The branch field was a bare text box; it is now a type-to-filter combobox over the repo's remote branches, and it says `On branch <x>` or `Creating new branch <x>` underneath so the outcome is visible before you commit to it. A "Create from issue" checkbox swaps it for a picker over the repo's open issues, previewing the branch name the bridge will derive and link. Both lists are allowed to be missing, and neither is on the critical path: a repo whose remote refuses to list branches falls back to exactly the text field this form used to be, and issues are not requested until the checkbox is ticked, since for a `custom` repo that request is a guaranteed 501. When the list is unknown the form says nothing about the branch rather than guessing, so it cannot claim "Creating new branch" for one that already exists. The fuzzy matcher is hand-rolled — the client carries no search dependency and this is the only place that wants one. It and the form's branch-state logic are pure and tested; the repo has no DOM harness, so interaction behaviour is not under test. Pre-review: the review panel runs against this commit.
Second review round on the create-from-issue path. The idempotency fallback preferred any branch linked to the issue over one actually named as requested, so an issue linked to `42-retries` by an earlier `gh issue develop` would quietly put the workspace on that stale branch instead of the name the form previewed. The fallback is now exact-linked-match, then a ref of that name, then a 409 — a branch the caller never asked for is no longer a candidate at all. Which failure means "already there" is no longer guessed from an English substring of GitHub's error text, which a rewording or another locale would defeat. Anything short of 401/403/404 is now checked against what exists before it is called a failure, and a null `linkedBranch` is distinguished structurally from an unreadable payload, so a malformed reply can no longer discard a branch that was just created. `GIT_HTTP_LOW_SPEED_*` and ssh's `ConnectTimeout` make git abort a stalled probe by itself, which is what actually reaps the process — the outer timer never could, and 8 probes at a tarpit previously left 16 git processes alive. The outer bound moves to 20s so git's own diagnosis wins the race. Credential redaction now parses rather than pattern-matches, since a password may itself contain `@`.
Review fixes for a4f10b8. The dropdown was absolutely positioned inside a modal whose panel is `overflow-hidden` and whose body is `overflow-y-auto`, and a non-visible overflow ancestor always clips an out-of-flow descendant — so most of an eight-branch list was unreachable, and arrowing down dragged the input off the top. The list is now in normal flow: there is nothing out of flow left to clip, and it simply lengthens a modal body that already scrolls. Pressing Enter to submit a newly typed branch name silently replaced it with the top fuzzy match, because the list pre-highlighted its first row — typing `fix` where `fix/rate-limit` exists then created the workspace on the existing branch, which is precisely the case the new-branch flow is for. Nothing is highlighted now until the user arrows into the list, so Enter commits what they typed. Matching restarts at every occurrence of the query's first character and keeps the best alignment, because pure greedy scanning got the headline case wrong: `main` consumed the `m` of `chore/re*m*ove-main-shim` and ranked that branch below one merely containing the letters scattered. The form's payload choice moves into a single pure function that returns the request or null, so "what gets sent" and "is it submittable" cannot disagree — the one decision here with a 400 behind it, previously the only one no test could reach.
The list has now failed in two places, for opposite reasons. Absolutely positioned inside the panel it was clipped: the panel is `overflow-hidden` and its body `overflow-y-auto`, and a non-visible overflow ancestor always clips an out-of-flow descendant, so most of an eight-branch list was unreachable. In normal flow it was not clipped, but its presence set the height of everything below it — pressing Create closed the list on blur, the panel re-centred, and the button moved out from under the pointer before mouseup, so the click either missed or landed on the backdrop and discarded the form. Portalled into `document.body` and positioned `fixed`, it is subject to neither: no overflow ancestor to clip it, and out of flow so mounting and unmounting move nothing. Placement is a pure function, so the flip and clamp are unit-tested rather than reasoned about. React routes portal events through the React tree, so a row click still meets the panel's `stopPropagation` on its way up. Rows now select on click rather than mousedown — the focus guard keeps the list mounted through mouseup — and hover reacts only to genuine pointer movement, so a list opening under a resting pointer can no longer capture the Enter that was meant to submit a newly typed branch name.
main's code-reduction pass landed while this branch was open. Three of the five conflicts are that pass meeting new code: - `mapErrorHook` replaced the per-route `try`/`catch`, so the branches route drops its own mapping and lets the plugin hook do it. - `useSubmitAction` replaced the hand-rolled submit state. The form keeps its guard — the Create button is disabled without a valid input, but implicit submission reaches `onSubmit` regardless — so the guard moves there and the hook owns `error`/`pending`. - Comments the tightened policy no longer allows are dropped from the conflicted regions. `createApp` also lost its `config` argument on main; the two new bridge test files drop it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `.dark` class carrying every dark token value sat on a `div` inside the Shell, so the custom properties only inherited down the app's own subtree. The combobox list is portalled to `document.body`, which is outside it, and resolved the `:root` light values instead — a white dropdown over the dark modal. Toggling the class on `document.documentElement` puts every token above both the app and any portal. `@custom-variant dark (&:is(.dark *))` is unaffected: `html` is an ancestor of everything the variant targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
Merged
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.
Closes #12.
The create-workspace form's branch field was a plain text box. It is now a fuzzy-searchable picker over the repo's real branches that says what it is about to do, plus a "Create from issue" mode that starts a workspace from an open issue and links the branch to that issue on the provider.
Depends on #10 and #11, both merged.
What changed
Backend (
03a08ec,a4c9aa8,f420884)GET /repos/:name/branches, answered withls-remoterather than through the repo's provider — repos default toprovider: "custom", for whichproviderForthrows 501, so a provider-backed listing would be dead for most registered repos.RepoProvider.linkBranchToIssue, which creates a branch and records it as the issue's linked development branch. GitHub exposes that linkage through GraphQL alone, so the provider grows a smallgraphqlhelper that also treats HTTP 200 carryingerrorsas a failure.POST /workspacesacceptsissueNumberin place ofbranch(exactly one, never both). The name comes from a new sharedissueBranchNameinfleet-protocol— shared so the client can preview the name the bridge will authoritatively pick.Client (
a4f10b8,39208c9,96a4a2b)On branch <x>orCreating new branch <x>beneath the field.customrepo that request is a guaranteed 501. When the list is unknown the form says nothing about the branch rather than guessing.Review
Each half was built, committed, then audited by three reviewers (adversarial, quality, functionality);
a4c9aa8,f420884,39208c9and96a4a2bare the resulting fixes. Things they caught that are worth knowing about:ls-remoteran with no prompt suppression and no deadline. git opens/dev/ttydirectly for credentials, bypassing the pipes it was handed, and the bridge runs in an operator's foreground terminal — so a private repo hung the request for good. 8 probes at a tarpit left 16 git processes alive. Probes now run non-interactively and git aborts them itself.fixwherefix/rate-limitexists created the workspace on the existing branch.fixed, which is subject to neither.Testing
bun run typecheckandbun run testare clean across all 11 packages.fleet-bridge155 → 207 tests,fleet-client69 → 138,fleet-protocol21 → 28.Known gap: this repo has no DOM or React test harness, so none of the combobox's interaction behaviour is under automated test. The pure parts — the matcher, placement, the keyboard state machine, and the request the form amounts to — are extracted and tested hard, but dropdown positioning, the flip threshold, and stacking against the modal backdrop have been reasoned about rather than seen. Those want a human's eyes in a browser before merge.
Two smaller judgement calls, both deliberate: the mock's seed repos now say
provider: "github"(exceptnotifier) so both the working issue picker and the 501 degradation path are reachable in mock mode, which changes the PROVIDER column on the mock Repos page; and what GitHub actually does with a duplicate linked-branch name is not verified against the live API — settling it would mean creating a branch on a real repo — so the code handles every reported behaviour and the docs no longer assert any of them as fact.🤖 Generated with Claude Code