Skip to content

Pick a branch or an issue when creating a workspace - #35

Merged
FireSquid6 merged 8 commits into
mainfrom
12-better-create-workspace-issue
Aug 3, 2026
Merged

Pick a branch or an issue when creating a workspace#35
FireSquid6 merged 8 commits into
mainfrom
12-better-create-workspace-issue

Conversation

@FireSquid6

Copy link
Copy Markdown
Owner

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 with ls-remote rather than through 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 HTTP 200 carrying errors as a failure.
  • POST /workspaces accepts issueNumber in place of branch (exactly one, never both). The name comes from a new shared issueBranchName in fleet-protocol — shared so the client can preview the name the bridge will authoritatively pick.

Client (a4f10b8, 39208c9, 96a4a2b)

  • Branch combobox with a hand-rolled fuzzy matcher (no new dependency), showing On branch <x> or Creating new branch <x> beneath the field.
  • "Create from issue" checkbox swapping in an open-issue picker that previews the derived branch name.
  • 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.

Review

Each half was built, committed, then audited by three reviewers (adversarial, quality, functionality); a4c9aa8, f420884, 39208c9 and 96a4a2b are the resulting fixes. Things they caught that are worth knowing about:

  • ls-remote ran with no prompt suppression and no deadline. git opens /dev/tty directly 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.
  • Create-from-issue had no "already linked" path, so any failure after a successful link left that issue permanently uncreatable. Both duplicate shapes GitHub has been reported to return are now handled, and the fallback only ever resolves a branch of the name that was asked for.
  • Pressing Enter to submit a newly typed branch name silently replaced it with the top match — typing fix where fix/rate-limit exists created the workspace on the existing branch.
  • The dropdown was clipped by the modal; moving it into normal flow fixed that but made pressing Create miss the button, because the list closing reflowed the panel between mousedown and mouseup. It is now portalled and fixed, which is subject to neither.

Testing

bun run typecheck and bun run test are clean across all 11 packages. fleet-bridge 155 → 207 tests, fleet-client 69 → 138, fleet-protocol 21 → 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" (except notifier) 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

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.
@FireSquid6 FireSquid6 linked an issue Jul 27, 2026 that may be closed by this pull request
FireSquid6 and others added 2 commits August 2, 2026 23:18
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>
@FireSquid6
FireSquid6 merged commit 5b8e55b into main Aug 3, 2026
1 check failed
This was referenced Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better create workspace issue

1 participant