diff --git a/apps/docs/src/content/docs/reference/bridge-api.md b/apps/docs/src/content/docs/reference/bridge-api.md index 992749a..43281a7 100644 --- a/apps/docs/src/content/docs/reference/bridge-api.md +++ b/apps/docs/src/content/docs/reference/bridge-api.md @@ -20,7 +20,7 @@ adds ship management, a repo registry, and an aggregate system-resources view. | `GET /workspaces` | Same path. Merged across ships, deduped, each row gains `ship`. | | `GET /workspaces/:repo/:name` | Same path. Proxied live to the owning ship; response gains `ship` on **both** the `active` and `inactive` variants. | | `GET /workspaces/:repo/:name/diff` | Same path and query. Proxied verbatim. | -| `POST /workspaces` | Same path, **different body**: `{ship, repoName, name, branch}` instead of `{url, repoName, name, branch}`. The clone URL comes from the bridge's repo registry. Response gains `ship`. | +| `POST /workspaces` | Same path, **different body**: `{ship, repoName, name, branch \| issueNumber}` instead of `{url, repoName, name, branch}`. The clone URL comes from the bridge's repo registry, and the branch may be named outright or derived from an issue. Response gains `ship`. | | `POST /workspaces/:repo/:name/branch` | Same. | | `POST /workspaces/:repo/:name/activate` | Same. | | `POST /workspaces/:repo/:name/deactivate` | Same. | @@ -36,7 +36,8 @@ adds ship management, a repo registry, and an aggregate system-resources view. Bridge-only routes: `GET`/`POST /ships`, `DELETE /ships/:name`, `GET /ships/:ship/system-resources`, `GET`/`POST /repos`, -`DELETE /repos/:name`, `GET /armory/file`, `GET /armory/ships`. +`DELETE /repos/:name`, `GET /repos/:name/branches`, `GET /armory/file`, +`GET /armory/ships`. ## Routes at a glance @@ -50,6 +51,7 @@ Bridge-only routes: `GET`/`POST /ships`, `DELETE /ships/:name`, | GET | `/repos` | 200 | `Repo[]` | | POST | `/repos` | 201 | `Repo` | | DELETE | `/repos/:name` | 200 | `{ ok: true }` | +| GET | `/repos/:name/branches` | 200 | `RepoBranch[]` | | GET | `/armory` | 200 | `ArmoryManifest` | | GET | `/armory/file` | 200 | `ArmoryFile` | | GET | `/armory/ships` | 200 | `ShipArmoryState[]` | @@ -76,11 +78,11 @@ The status comes from the thrown `BridgeError`; anything else is a `500`. | Status | Raised when | | --- | --- | -| `400` | Invalid repo/workspace/ship identifier; `unknown ship: ` (create, or per-ship resources); `unknown repo: `; `invalid repo`. | +| `400` | Invalid repo/workspace/ship identifier; `unknown ship: ` (create, or per-ship resources); `unknown repo: `; `invalid repo`; a create naming both a `branch` and an `issueNumber`, neither, a blank `branch`, or an `issueNumber` that is not a positive integer. | | `404` | `workspace not found: /` — no ship in the ownership index holds it; `ship not found: `; `repo not found: `. | | `409` | `ship already registered: `; a registering ship holds workspaces already hosted elsewhere; `workspace already exists: /`; a create already in progress or of indeterminate outcome for that key; a ship removed mid-request. | | `422` | Elysia schema validation on the request body. | -| `502` | `ship at did not respond: ` (`POST /ships`); a ship returned no data, an invalid summary/status, or a workspace identity that was not requested. | +| `502` | `ship at did not respond: ` (`POST /ships`); a ship returned no data, an invalid summary/status, or a workspace identity that was not requested; `GET /repos/:name/branches` could not reach the remote. | | `503` | `ship "" hosting / is offline`; `ship "" is offline` (create, per-ship resources); `ship "" unreachable: `. | | ship's status | Any error the owning ship returned is passed through with the ship's own status and message. | @@ -218,6 +220,30 @@ Responds `{ ok: true }`. Deleting a repo does not touch any workspace already cloned from it. +### `GET /repos/:name/branches` + +The branches the repo's remote currently advertises, sorted by name. + +```ts +{ name: string; sha: string }[] +``` + +Answered with `git ls-remote --heads` against the registered clone URL, **not** +through the repo's provider: `provider` defaults to `"custom"`, for which no +provider exists, so a provider-backed listing would be unavailable for most +repos. `ls-remote` works against any git URL and needs no token. The probe runs +non-interactively — git never prompts for credentials or host keys, aborts an +http transfer that stalls for 15 s, and is given up on entirely after 20 s. + +`refs/heads/` is stripped from each name; tags and other refs are omitted, so a +tag the ship would happily clone does not appear here. + +| Status | Cause | +| --- | --- | +| `400` | Invalid repo identifier. | +| `404` | `repo not found: `. | +| `502` | `could not list branches for repo "": ` — unreachable, unauthenticated, or timed out. Credentials embedded in the repo URL are redacted from this message. | + ## Armory The read side of the [armory](/guides/the-armory/): the manifest of the bridge's @@ -374,8 +400,10 @@ text. ### `POST /workspaces` ```ts -// request body — all four fields required -{ ship: string; repoName: string; name: string; branch: string } +// request body — ship, repoName and name are required; +// exactly one of branch / issueNumber must be present +{ ship: string; repoName: string; name: string; + branch?: string; issueNumber?: number } ``` `ship` names the target host and `repoName` must be a **registered repo**; the @@ -387,10 +415,32 @@ bridge looks up its clone URL and calls the ship's `POST /workspaces` with created in the new workspace rather than rejected — see [ship API](/reference/ship-api/). +With `issueNumber` instead, the bridge resolves the branch itself before calling +the ship: + +1. it reads the issue through the repo's [provider](#repo-registry) — so this + form needs `provider: "github"` and a token with repo write scope; +2. it computes the issue's canonical branch name, `-` + capped at 60 characters (`12-better-create-workspace-issue`) — the same + function a client can use to preview the name; +3. it asks the provider to create that branch and record it as the issue's + linked development branch (GitHub's "Development → create a branch"); +4. the **name the provider returns** is what the ship is told to check out. Use + it rather than the computed name — a provider may hand back a different ref. + +Step 3 is safe to repeat. If the provider will not create the branch, the bridge +looks for one of the *same name* — first among the issue's linked branches, then +as a plain ref (someone may have pushed it by hand) — and uses that. A branch +linked to the issue under a **different** name is never substituted; that case +fails with `409`, since silently checking out a branch the caller never named is +worse than refusing. The branch is created before the clone and is not removed if +the clone then fails, so a retry reuses it. + | Status | Cause | | --- | --- | -| `422` | A body field is missing. | -| `400` | Invalid repo/workspace identifier; `unknown ship: `; `unknown repo: `. | +| `422` | `ship`, `repoName` or `name` is missing. | +| `400` | Invalid repo/workspace identifier; `unknown ship: `; `unknown repo: `; both `branch` and `issueNumber`, or neither; a blank `branch`; an `issueNumber` that is not a positive integer. | +| provider's status | Any error resolving or linking the issue is passed through with the provider's own status — e.g. `401` with no token, `403` for a token without repo write scope, `404` for an unknown issue, `409` when the branch could be neither created nor found under the requested name. | | `503` | `ship "" is offline`. | | `409` | `workspace already exists: /`; a create for that key is already in progress; the key's create outcome is indeterminate; the target ship was removed mid-request. | | `502` | The ship returned no data, an invalid summary, or a different workspace identity. | diff --git a/packages/fleet-bridge/src/api/repos.ts b/packages/fleet-bridge/src/api/repos.ts index 8d90d8b..fa18d3d 100644 --- a/packages/fleet-bridge/src/api/repos.ts +++ b/packages/fleet-bridge/src/api/repos.ts @@ -25,6 +25,7 @@ export function reposPlugin(manager: FleetManager) { return { ok: true as const }; }) .get("/repos/:name/info", ({ params }) => manager.repoInfo(params.name)) + .get("/repos/:name/branches", ({ params }) => manager.listRepoBranches(params.name)) .get( "/repos/:name/issues", ({ params, query }) => manager.listRepoIssues(params.name, { state: query.state }), diff --git a/packages/fleet-bridge/src/api/workspaces.ts b/packages/fleet-bridge/src/api/workspaces.ts index 3cc7a36..8d92b12 100644 --- a/packages/fleet-bridge/src/api/workspaces.ts +++ b/packages/fleet-bridge/src/api/workspaces.ts @@ -85,7 +85,10 @@ export function workspacesPlugin(manager: FleetManager) { ship: t.String(), repoName: t.String(), name: t.String(), - branch: t.String(), + // Either/or, enforced by the manager rather than the schema so the + // client gets a 400 with a reason instead of a shapeless 422. + branch: t.Optional(t.String()), + issueNumber: t.Optional(t.Numeric()), }), }, ) diff --git a/packages/fleet-bridge/src/fleet-manager.ts b/packages/fleet-bridge/src/fleet-manager.ts index d7010bf..df8aa87 100644 --- a/packages/fleet-bridge/src/fleet-manager.ts +++ b/packages/fleet-bridge/src/fleet-manager.ts @@ -3,6 +3,7 @@ import { ARMORY_DIRECTORY, CreateRepoInputSchema, FleetIdentifierSchema, + issueBranchName, ShipSchema, WorkspaceRefsSchema, WorkspaceSummarySchema, @@ -18,7 +19,7 @@ import { type WorkspaceStatus, type WorkspaceSummary, } from "fleet-protocol"; -import type { DiffOptions } from "git-bun"; +import { Git, GitError, type DiffOptions, type RemoteRef } from "git-bun"; import { TERMINAL_TAKEOVER_QUERY } from "webterm/protocol"; import { ShipConnection, toWsUrl, type ShipConnectionDeps } from "./ship-connection"; import { defaultPublicUrl, type BridgeConfig } from "./config"; @@ -27,6 +28,7 @@ import { type BridgeWorkspaceEvent, type BridgeWorkspaceStatus, type BridgeWorkspaceSummary, + type RepoBranch, type ShipArmoryState, type ShipInfo, type ShipSystemResources, @@ -69,14 +71,121 @@ export class BridgeError extends Error { /** How long to wait for a ship's first `sync` before treating it as offline. */ const SYNC_TIMEOUT_MS = 5000; -/** Body of `POST /workspaces` on the bridge (ship-targeted, names a registered repo). */ +/** + * Outer bound on a remote ref probe. Deliberately longer than the 15 s of + * stalled transfer `NON_INTERACTIVE_GIT_ENV` gives git, so that on a dead remote + * git aborts itself first and the client gets git's own diagnosis instead of a + * bare "timed out". This is the backstop for a git that somehow does neither. + */ +const LS_REMOTE_TIMEOUT_MS = 20000; + +/** + * Environment for every git invocation the bridge makes. Two failure modes, both + * of which would otherwise leave a git process alive after the HTTP request that + * spawned it has gone: + * + * - **Prompts.** git asks for credentials, ssh passphrases and unknown host keys + * by opening `/dev/tty` directly, bypassing the pipes it was given — and the + * bridge normally runs in an operator's foreground terminal, so that tty + * exists. A private repo would park git on a prompt nobody answers forever. + * - **Tarpits.** A remote that completes the TCP handshake and then says nothing + * holds git open indefinitely; git only inherits a deadline if it is given + * one. The low-speed pair makes git abort a transfer moving under 1 byte/s for + * 15 s, and `ConnectTimeout` bounds the ssh handshake. Both make git exit on + * its own, which is what actually reaps the process — killing the request that + * is waiting on it would not. + * + * `GIT_HTTP_LOW_SPEED_*` only covers the http(s) transport; an ssh remote that + * connects and then stalls is still only bounded by `LS_REMOTE_TIMEOUT_MS`. + */ +const NON_INTERACTIVE_GIT_ENV: Record = { + GIT_TERMINAL_PROMPT: "0", + GIT_ASKPASS: "", + GIT_SSH_COMMAND: "ssh -oBatchMode=yes -oConnectTimeout=10", + GIT_HTTP_LOW_SPEED_LIMIT: "1", + GIT_HTTP_LOW_SPEED_TIME: "15", +}; + +/** + * Reject with `message` if `promise` has not settled within `ms`. + * + * This bounds the *waiting*, not the work: `git-bun` exposes no handle to abort + * a running command, so a git process that outlives its deadline is left to exit + * on its own. That is why `NON_INTERACTIVE_GIT_ENV` configures git to give up by + * itself — this timer is the backstop, not the mechanism. + */ +async function withTimeout(promise: Promise, ms: number, message: string): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error(message)), ms); + }), + ]); + } finally { + clearTimeout(timer); + } +} + +/** A URL-shaped run of text; quotes and whitespace end it, as they do in git's messages. */ +const URL_IN_TEXT = /[a-z][a-z0-9+.-]*:\/\/[^\s'"`]+/gi; + +/** + * Blank out `user:secret@` userinfo in every URL a message carries. A repo may + * be registered with an embedded token, and git echoes the URL it was given back + * in the command line it reports on failure — this keeps that out of an API + * response. + * + * Parsing beats pattern-matching here: a password may itself contain `@` + * (`https://user:p@ssw0rd@host/r`), and userinfo runs to the *last* `@` before + * the path, which a leftmost-shortest regex gets wrong. `URL` implements that + * rule; the regex below is only for candidates it refuses to parse. + * + * Not covered, deliberately: credentials in a query string + * (`?access_token=…`) and scp-style `token@host:org/repo.git`, neither of which + * is a URL this code can recognize as one. + */ +function redactUrlCredentials(text: string): string { + return text.replace(URL_IN_TEXT, (candidate) => { + try { + const url = new URL(candidate); + if (!url.username && !url.password) return candidate; + url.username = "***"; + url.password = ""; + return url.toString(); + } catch { + return redactAuthority(candidate); + } + }); +} + +/** Fallback for an unparseable URL: strip through the last `@` of the authority. */ +function redactAuthority(candidate: string): string { + const parts = /^([a-z][a-z0-9+.-]*:\/\/)([^/?#]*)([\s\S]*)$/i.exec(candidate); + if (!parts) return candidate; + const [, scheme, authority = "", rest = ""] = parts; + const at = authority.lastIndexOf("@"); + if (at === -1) return candidate; + return `${scheme}***@${authority.slice(at + 1)}${rest}`; +} + +/** + * Body of `POST /workspaces` on the bridge (ship-targeted, names a registered + * repo). The branch comes either verbatim from `branch`, or from the issue that + * `issueNumber` identifies — exactly one of the two, never both. + */ export interface CreateWorkspaceInput { readonly ship: string; readonly repoName: string; readonly name: string; - readonly branch: string; + readonly branch?: string; + readonly issueNumber?: number; } +/** Which of the two mutually exclusive branch sources a create request chose. */ +type BranchSource = { readonly branch: string } | { readonly issueNumber: number }; + type EdenResult = { data: T | null; error: unknown }; interface CreateReservation { @@ -96,6 +205,10 @@ export class FleetManager { private readonly syncTimeoutMs: number; private readonly store: Store; private readonly makeProvider: (repo: Repo) => RepoProvider; + /** Probes a remote's refs; overridable so tests never shell out to git. */ + private readonly lsRemote: typeof Git.lsRemote; + /** Deadline for one `lsRemote` probe (overridable in tests). */ + private readonly lsRemoteTimeoutMs: number; private readonly armory: ArmoryService; constructor( @@ -105,6 +218,8 @@ export class FleetManager { syncTimeoutMs?: number; store?: Store; providerFor?: (repo: Repo) => RepoProvider; + lsRemote?: typeof Git.lsRemote; + lsRemoteTimeoutMs?: number; armory?: ArmoryService; }, ) { @@ -112,6 +227,8 @@ export class FleetManager { this.syncTimeoutMs = opts?.syncTimeoutMs ?? SYNC_TIMEOUT_MS; this.store = opts?.store ?? new Store(config.dataDirectory); this.makeProvider = opts?.providerFor ?? providerFor; + this.lsRemote = opts?.lsRemote ?? Git.lsRemote; + this.lsRemoteTimeoutMs = opts?.lsRemoteTimeoutMs ?? LS_REMOTE_TIMEOUT_MS; this.armory = opts?.armory ?? new ArmoryService(join(config.dataDirectory, ARMORY_DIRECTORY)); } @@ -313,6 +430,52 @@ export class FleetManager { if (!deleted) throw new BridgeError(`repo not found: ${name}`, 404); } + /** + * `GET /repos/:name/branches` — the branches the repo's remote advertises. + * + * Answered with `ls-remote` rather than through the repo's provider on purpose: + * `addRepo` defaults a repo to `provider: "custom"`, for which `providerFor` + * throws 501, so a provider-backed listing would be dead for most registered + * repos. `ls-remote` speaks to any git URL and needs no token. + */ + async listRepoBranches(name: string): Promise { + this.identifier(name, "repo"); + const repo = await this.store.getRepo(name); + if (!repo) throw new BridgeError(`repo not found: ${name}`, 404); + + let refs: RemoteRef[]; + try { + refs = await withTimeout( + this.lsRemote(repo.url, { + cwd: this.config.dataDirectory, + heads: true, + env: NON_INTERACTIVE_GIT_ENV, + }), + this.lsRemoteTimeoutMs, + `timed out after ${this.lsRemoteTimeoutMs}ms`, + ); + } catch (error) { + // Any failure here is the remote's or the network's, not the caller's — and + // a raw GitError must not reach the route, which would report it as a 500. + // git's own stderr is preferred over the GitError message because the + // message replays the command line, credentials in the URL included. + const detail = error instanceof GitError ? error.stderr.trim() || error.message : (error as Error).message; + throw new BridgeError( + redactUrlCredentials(`could not list branches for repo "${name}": ${detail}`), + 502, + ); + } + + const prefix = "refs/heads/"; + return refs + .filter((ref) => ref.ref.startsWith(prefix)) + .map((ref) => ({ name: ref.ref.slice(prefix.length), sha: ref.sha })) + // Plain codepoint order, not `localeCompare`: the listing must not reorder + // itself with the bridge host's locale. `--heads` hides HEAD, so which + // branch is the default one is not knowable here. + .sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + } + /** `ProviderError` from `fn` propagates unchanged so the API can surface its status. */ private async withProvider(name: string, fn: (provider: RepoProvider) => Promise): Promise { this.identifier(name, "repo"); @@ -551,10 +714,15 @@ export class FleetManager { return parsed.data; } - /** `POST /workspaces {ship,repoName,name,branch}` — clones a registered repo. */ + /** + * `POST /workspaces {ship,repoName,name,branch|issueNumber}` — clones a + * registered repo onto a branch, either named outright or derived from an + * issue (which also creates and links that branch on the provider). + */ async createWorkspace(input: CreateWorkspaceInput): Promise { this.identifier(input.repoName, "repo"); this.identifier(input.name, "workspace"); + const source = this.branchSource(input); const conn = this.connections.get(input.ship); if (!conn) throw new BridgeError(`unknown ship: ${input.ship}`, 400); @@ -583,12 +751,24 @@ export class FleetManager { let retainReservation = false; try { + // Resolving the issue happens under the reservation, so two concurrent + // creates of the same workspace cannot both ask the provider for a branch; + // the second is turned away with a 409 before it gets here. + // + // The branch outliving a failed create is accepted rather than undone: if + // the ship call below fails, the linked branch stays on the remote with no + // workspace behind it. Nothing is corrupted — the `finally` still clears + // the reservation, the error the user sees is the ship's, and a retry + // resolves the same branch because `linkBranchToIssue` is idempotent. + const branch = + "branch" in source ? source.branch : await this.branchForIssue(input.repoName, source.issueNumber); + const response = await this.call(conn, () => conn.client.workspaces.post({ url: repo.url, repoName: input.repoName, name: input.name, - branch: input.branch, + branch, }) as Promise>, { ambiguousEmptyResponse: true }, ); @@ -640,6 +820,56 @@ export class FleetManager { } } + /** + * Validate a create request's mutually exclusive branch source. The ship also + * rejects a blank branch, but doing it here saves the round trip and keeps the + * "one of the two" rule in a single place. + */ + private branchSource(input: CreateWorkspaceInput): BranchSource { + if (input.branch !== undefined && input.issueNumber !== undefined) { + throw new BridgeError("a workspace is created from a branch or an issue, not both", 400); + } + if (input.branch !== undefined) { + const branch = input.branch.trim(); + if (branch.length === 0) throw new BridgeError("branch must not be empty", 400); + return { branch }; + } + if (input.issueNumber !== undefined) { + // `t.Numeric()` admits 0, 1.5 and -3; rejecting them here costs nothing, + // where letting them through costs a provider round trip to learn that no + // such issue exists. + if (!Number.isSafeInteger(input.issueNumber) || input.issueNumber < 1) { + throw new BridgeError("issueNumber must be a positive integer", 400); + } + return { issueNumber: input.issueNumber }; + } + throw new BridgeError("a workspace needs either a branch or an issue to start from", 400); + } + + /** + * Turn an issue into the branch a new workspace sits on: read the issue, then + * have the provider create and link `-`. The name the provider + * returns wins over the computed one — it may have de-duplicated it. + */ + private async branchForIssue(repoName: string, issueNumber: number): Promise { + return this.withProvider(repoName, async (provider) => { + const issue = await provider.getIssue(issueNumber); + let computed: string; + try { + computed = issueBranchName(issue); + } catch (error) { + // The issue identity came from the provider, so a name that cannot be + // derived from it is an upstream fault, not a bad request. + throw new BridgeError( + `could not derive a branch name for issue ${issueNumber}: ${(error as Error).message}`, + 502, + ); + } + const linked = await provider.linkBranchToIssue(issueNumber, computed); + return linked.name; + }); + } + /** `POST /workspaces/:repo/:name/branch`. */ async switchBranch(repo: string, name: string, branch: string): Promise { const conn = this.routeFor(repo, name); diff --git a/packages/fleet-bridge/src/providers/github.ts b/packages/fleet-bridge/src/providers/github.ts index 0565b52..adc0d82 100644 --- a/packages/fleet-bridge/src/providers/github.ts +++ b/packages/fleet-bridge/src/providers/github.ts @@ -4,6 +4,7 @@ import type { Issue, IssueComment, IssueSummary, + LinkedBranch, ListOptions, PullRequest, PullRequestSummary, @@ -24,6 +25,42 @@ export interface GitHubProviderConfig { const DEFAULT_BASE_URL = "https://api.github.com"; +/** + * The only way to attach a branch to an issue: GitHub exposes the linkage + * through GraphQL alone, with no REST equivalent. The mutation creates the ref + * as well, so nothing has to push a branch beforehand. + */ +const CREATE_LINKED_BRANCH = `mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, $name: String!) { + createLinkedBranch(input: { issueId: $issueId, oid: $oid, name: $name }) { + linkedBranch { + ref { + name + target { oid } + } + } + } +}`; + +/** + * The branches already attached to an issue, used to make `linkBranchToIssue` + * idempotent. `first: 10` is generous: GitHub's own UI links one branch per + * issue, and only a human linking branches by hand pushes it past that. + */ +const LINKED_BRANCHES = `query LinkedBranches($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + issue(number: $number) { + linkedBranches(first: 10) { + nodes { + ref { + name + target { oid } + } + } + } + } + } +}`; + /** * Extract `{ owner, repo }` from a git clone URL. Handles the https web form * (with or without a `.git` suffix or trailing slash) and the `git@host:o/r.git` @@ -63,6 +100,8 @@ interface GitHubRepo { interface GitHubIssue { number: number; + /** GraphQL global id — the handle `createLinkedBranch` addresses the issue by. */ + node_id?: string; title: string; state: string; user: GitHubUser | null; @@ -129,6 +168,50 @@ interface GitHubJob { conclusion: string | null; } +interface GitHubRef { + object?: { sha?: string }; +} + +/** Envelope of a GraphQL reply; `errors` can be present alongside HTTP 200. */ +interface GraphQLResponse { + data?: T | null; + errors?: { message?: unknown }[]; +} + +/** The `{ name, target { oid } }` selection both linked-branch operations share. */ +interface GraphQLRef { + name?: string; + target?: { oid?: string } | null; +} + +interface CreateLinkedBranchResult { + createLinkedBranch?: { + linkedBranch?: { ref?: GraphQLRef | null } | null; + } | null; +} + +interface LinkedBranchesResult { + repository?: { + issue?: { + linkedBranches?: { nodes?: ({ ref?: GraphQLRef | null } | null)[] | null } | null; + } | null; + } | null; +} + +/** + * Statuses that mean the create was refused for a reason no amount of looking + * around will change. Everything else is treated as "the branch may already + * exist" — the safe default, since a wrong guess there costs one extra read + * while the opposite dead-ends the issue permanently. + */ +const REFUSALS_TO_SURFACE = new Set([401, 403, 404]); + +/** A ref selection that carries both fields, or `undefined` if it does not. */ +function toLinkedBranch(ref: GraphQLRef | null | undefined): LinkedBranch | undefined { + if (!ref?.name || !ref.target?.oid) return undefined; + return { name: ref.name, sha: ref.target.oid }; +} + export class GitHubProvider implements RepoProvider { private readonly owner: string; private readonly repo: string; @@ -160,8 +243,11 @@ export class GitHubProvider implements RepoProvider { async listIssues(options?: ListOptions): Promise { const state = options?.state ?? "open"; + // GitHub pages at 30 by default and says nothing about the truncation. The + // list feeds a search-over-all-issues picker, so 100 — the maximum a single + // page allows — is the difference between "no matches" and the issue. const issues = await this.request( - `/repos/${this.owner}/${this.repo}/issues?state=${state}`, + `/repos/${this.owner}/${this.repo}/issues?state=${state}&per_page=100`, ); // GitHub's `/issues` endpoint returns pull requests too. return issues @@ -288,6 +374,133 @@ export class GitHubProvider implements RepoProvider { return logs; } + /** + * Create `branch` off the default branch's head and record it as issue + * `issueNumber`'s linked development branch, in one mutation. Needs a token + * with repo write scope: it writes a ref and an issue timeline entry. + * + * Three reads precede the write because the mutation is addressed by GraphQL + * node id and a commit oid, neither of which the caller has: the issue's + * `node_id`, the repo's default branch, and that branch's head SHA. + * + * Idempotent as far as it can be — asking twice for the same branch is + * ordinary (a second workspace off one issue, a retry after a failed clone, or + * a branch someone already made with `gh issue develop`). What GitHub does + * with a duplicate is **not verified here**: it has been reported both as a + * null `linkedBranch` on an HTTP 200 and as a refusal, and the API docs + * describe de-duplicating the name instead. So rather than encoding a guess, + * anything short of a clear auth/permission/not-found failure is treated as + * "it may already be there" and handed to {@link existingBranch}, which + * decides on evidence. + */ + async linkBranchToIssue(issueNumber: number, branch: string): Promise { + this.requireToken(); + + const issue = await this.request( + `/repos/${this.owner}/${this.repo}/issues/${issueNumber}`, + ); + if (!issue.node_id) { + throw new ProviderError(`GitHub issue ${issueNumber} carried no node id`, 502); + } + + const { defaultBranch } = await this.getInfo(); + // Not URL-encoded: a default branch may legitimately contain "/", which is a + // path separator in the ref endpoint rather than data. + const base = await this.request( + `/repos/${this.owner}/${this.repo}/git/ref/heads/${defaultBranch}`, + ); + if (!base.object?.sha) { + throw new ProviderError(`GitHub returned no head commit for branch ${defaultBranch}`, 502); + } + + let result: CreateLinkedBranchResult; + try { + result = await this.graphql(CREATE_LINKED_BRANCH, { + issueId: issue.node_id, + oid: base.object.sha, + name: branch, + }); + } catch (error) { + // Refusals worth reporting as-is: no token, no scope, no such issue. They + // cannot mean "already there", and looking would only mask them. Anything + // else — including a refusal whose wording nothing here can anticipate — + // is checked against what exists before it is called a failure. + if (error instanceof ProviderError && REFUSALS_TO_SURFACE.has(error.status)) throw error; + return this.existingBranch(issueNumber, branch, error); + } + + // A `linkedBranch` that is explicitly null is GitHub declining to create + // one; a *missing* payload is a reply this code cannot read. Only the first + // means "look for what is already there" — conflating them would throw away + // a branch that had just been created and silently use an older one. + if (result.createLinkedBranch?.linkedBranch === null) { + return this.existingBranch(issueNumber, branch); + } + const created = toLinkedBranch(result.createLinkedBranch?.linkedBranch?.ref); + if (!created) { + throw new ProviderError( + `GitHub createLinkedBranch returned an unusable payload for issue ${issueNumber}`, + 502, + ); + } + return created; + } + + /** + * What to use when `createLinkedBranch` would not create anything, in order of + * how well it matches what the caller asked for: + * + * 1. a branch of exactly `requested` already linked to the issue — the retry + * and second-workspace cases, and the only outright happy answer; + * 2. a branch named `requested` that exists but was never linked — someone + * pushed it by hand; it is still the branch the caller named. + * + * A branch linked to the issue under some *other* name is deliberately not + * used: it is not what was asked for, and returning it would silently put a + * workspace on a branch the user never saw. That case fails instead, carrying + * `cause` so the reason the create was refused is not lost. + */ + private async existingBranch( + issueNumber: number, + requested: string, + cause?: unknown, + ): Promise { + const result = await this.graphql(LINKED_BRANCHES, { + owner: this.owner, + repo: this.repo, + number: issueNumber, + }); + const linked = (result.repository?.issue?.linkedBranches?.nodes ?? []) + .map((node) => toLinkedBranch(node?.ref)) + .find((ref) => ref?.name === requested); + if (linked) return linked; + + const unlinked = await this.branchRef(requested); + if (unlinked) return unlinked; + + const detail = cause instanceof Error ? `: ${cause.message}` : ""; + throw new ProviderError( + `GitHub would not create branch "${requested}" for issue ${issueNumber}, and no branch of that name exists${detail}`, + 409, + ); + } + + /** One branch by name, or `undefined` when the remote has no such ref. */ + private async branchRef(branch: string): Promise { + try { + const ref = await this.request( + `/repos/${this.owner}/${this.repo}/git/ref/heads/${branch}`, + ); + return ref.object?.sha ? { name: branch, sha: ref.object.sha } : undefined; + } catch (error) { + // Only a 404 carries information — it means the ref is genuinely absent. A + // 403 or a 502 says nothing about whether the branch exists, and reporting + // either as "no such branch" would be a lie. + if (error instanceof ProviderError && error.status === 404) return undefined; + throw error; + } + } + private async postComment(number: number, body: string): Promise { this.requireToken(); const comment = await this.request( @@ -369,6 +582,32 @@ export class GitHubProvider implements RepoProvider { return (await response.json()) as T; } + /** + * Run a GraphQL operation through `request`, so it shares the REST path's + * headers, auth and failure mapping. GraphQL reports *operation* failures with + * HTTP 200 and a populated `errors` array, so a transport success still has to + * be inspected before the payload can be trusted. + */ + private async graphql(query: string, variables: Record): Promise { + const payload = await this.request>("/graphql", { + method: "POST", + body: { query, variables }, + }); + + const first = payload.errors?.[0]; + if (first !== undefined) { + const message = typeof first.message === "string" ? first.message : "unknown GraphQL error"; + // A GraphQL error has no status of its own. Callers that care what kind of + // failure it was must decide from context — matching on the wording would + // break the first time GitHub rephrases it, or answers in another locale. + throw new ProviderError(`GitHub GraphQL request failed: ${message}`, 502); + } + if (payload.data === undefined || payload.data === null) { + throw new ProviderError("GitHub GraphQL response carried no data", 502); + } + return payload.data; + } + /** * Fetch a non-JSON body (Actions job logs). GitHub answers the logs endpoint * with a 302 to a pre-signed storage URL that rejects the `Authorization` diff --git a/packages/fleet-bridge/src/providers/index.ts b/packages/fleet-bridge/src/providers/index.ts index 507b22f..d86a2d3 100644 --- a/packages/fleet-bridge/src/providers/index.ts +++ b/packages/fleet-bridge/src/providers/index.ts @@ -33,6 +33,7 @@ export type { Issue, IssueComment, IssueSummary, + LinkedBranch, ListOptions, PullRequest, PullRequestSummary, diff --git a/packages/fleet-bridge/src/providers/provider.ts b/packages/fleet-bridge/src/providers/provider.ts index 7252c9a..4314caa 100644 --- a/packages/fleet-bridge/src/providers/provider.ts +++ b/packages/fleet-bridge/src/providers/provider.ts @@ -95,6 +95,12 @@ export interface ListOptions { export type ReviewEvent = "APPROVE" | "REQUEST_CHANGES" | "COMMENT"; +/** A branch the provider created and attached to an issue. */ +export interface LinkedBranch { + readonly name: string; + readonly sha: string; +} + export interface RepoProvider { getInfo(): Promise; listIssues(options?: ListOptions): Promise; @@ -108,4 +114,16 @@ export interface RepoProvider { listChecks(ref: string): Promise; /** Raw logs of the failed GitHub Actions jobs for a commit-ish. */ getFailedLogs(ref: string): Promise; + /** + * Create `branch` on the remote *and* record it as the issue's linked + * development branch (GitHub's "Development → create a branch for this issue" + * relationship). A write: it needs a token with repo write scope. + * + * Callers must use the returned `name`, never the one they passed in: a forge + * may hand back a different ref. What any given forge does when the name is + * already taken — de-duplicate it, refuse, or return the existing link — is + * not established here, so an implementation is expected to cope with the name + * coming back changed and to be safe to call twice for the same branch. + */ + linkBranchToIssue(issueNumber: number, branch: string): Promise; } diff --git a/packages/fleet-bridge/src/types.ts b/packages/fleet-bridge/src/types.ts index aa05986..e6c4252 100644 --- a/packages/fleet-bridge/src/types.ts +++ b/packages/fleet-bridge/src/types.ts @@ -32,6 +32,12 @@ export type BridgeWorkspaceEvent = readonly workspace: BridgeWorkspaceSummary; }; +/** A branch a registered repo's remote advertises — a row of `GET /repos/:name/branches`. */ +export interface RepoBranch { + readonly name: string; + readonly sha: string; +} + /** * One ship's entry in the aggregate `GET /system-resources`. `resources` is * present when the ship is online and responded; otherwise `error` explains why diff --git a/packages/fleet-bridge/tests/providers.test.ts b/packages/fleet-bridge/tests/providers.test.ts index fe52f55..6f920eb 100644 --- a/packages/fleet-bridge/tests/providers.test.ts +++ b/packages/fleet-bridge/tests/providers.test.ts @@ -216,7 +216,7 @@ describe("GitHubProvider", () => { expect(issues).toHaveLength(1); expect(issues[0]!.number).toBe(1); - expect(calls[0]!.url).toBe("https://api.github.com/repos/owner/repo/issues?state=open"); + expect(calls[0]!.url).toBe("https://api.github.com/repos/owner/repo/issues?state=open&per_page=100"); }); test("listIssues passes through an explicit state", async () => { @@ -225,7 +225,16 @@ describe("GitHubProvider", () => { await provider.listIssues({ state: "all" }); - expect(calls[0]!.url).toBe("https://api.github.com/repos/owner/repo/issues?state=all"); + expect(calls[0]!.url).toBe("https://api.github.com/repos/owner/repo/issues?state=all&per_page=100"); + }); + + test("listIssues asks for a full page, not GitHub's default 30", async () => { + const { fetch, calls } = fakeFetch(Response.json([])); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", fetch }); + + await provider.listIssues(); + + expect(new URL(calls[0]!.url).searchParams.get("per_page")).toBe("100"); }); test("a 404 upstream response surfaces as a ProviderError with status 404", async () => { @@ -447,6 +456,305 @@ describe("GitHubProvider", () => { } }); + /** The GraphQL operation a fake saw, so a test can answer per operation. */ + interface GraphQLCall { + query: string; + variables: Record; + } + + /** + * Drive every REST read `linkBranchToIssue` makes, and route each GraphQL + * operation to `graphql`. Refs other than `main` 404 unless `extraRefs` names + * them — that is how the "branch exists but was never linked" path is set up. + */ + function linkBranchFetch( + graphql: (call: GraphQLCall) => Response, + extraRefs: Record = {}, + ): { fetch: typeof fetch; calls: FetchCall[] } { + const calls: FetchCall[] = []; + const refs: Record = { main: "basesha", ...extraRefs }; + const fn = (async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const body = typeof init?.body === "string" ? init.body : undefined; + calls.push({ url, method: init?.method ?? "GET", headers: new Headers(init?.headers), body }); + + const issue = /\/issues\/(\d+)$/.exec(url)?.[1]; + if (issue !== undefined) { + return Response.json({ + number: Number(issue), + node_id: `I_issue${issue}`, + title: "a bug", + state: "open", + user: { login: "alice" }, + html_url: `https://github.com/owner/repo/issues/${issue}`, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + body: null, + comments: 0, + }); + } + if (url.endsWith("/repos/owner/repo")) return Response.json(repoPayload); + + const ref = /\/git\/ref\/heads\/(.+)$/.exec(url)?.[1]; + if (ref !== undefined) { + const sha = refs[ref]; + return sha === undefined + ? Response.json({ message: "Not Found" }, { status: 404 }) + : Response.json({ object: { sha } }); + } + if (url.endsWith("/graphql")) return graphql(JSON.parse(body!) as GraphQLCall); + throw new Error(`unexpected fetch: ${url}`); + }) as unknown as typeof globalThis.fetch; + return { fetch: fn, calls }; + } + + const isMutation = (call: GraphQLCall) => call.query.includes("createLinkedBranch("); + + const createdRef = (name: string, oid: string) => + Response.json({ data: { createLinkedBranch: { linkedBranch: { ref: { name, target: { oid } } } } } }); + + const linkedRefs = (...refs: { name: string; oid: string }[]) => + Response.json({ + data: { + repository: { + issue: { + linkedBranches: { + nodes: refs.map((ref) => ({ ref: { name: ref.name, target: { oid: ref.oid } } })), + }, + }, + }, + }, + }); + + /** The documented duplicate reply: HTTP 200, no `errors`, null `linkedBranch`. */ + const duplicateByNull = () => + Response.json({ data: { createLinkedBranch: { clientMutationId: null, issue: null, linkedBranch: null } } }); + + /** The other duplicate reply: a refusal. The wording is not what makes it one. */ + const duplicateByError = () => + Response.json({ errors: [{ message: "A ref named 12-a-bug already exists in the repository" }] }); + + test("linkBranchToIssue posts the mutation with the issue node id, base oid and name", async () => { + const { fetch, calls } = linkBranchFetch(() => createdRef("12-a-bug", "newsha")); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + const linked = await provider.linkBranchToIssue(12, "12-a-bug"); + + expect(linked).toEqual({ name: "12-a-bug", sha: "newsha" }); + + const mutation = calls.find((c) => c.url.endsWith("/graphql"))!; + expect(mutation.url).toBe("https://api.github.com/graphql"); + expect(mutation.method).toBe("POST"); + expect(mutation.headers.get("Authorization")).toBe("Bearer t0ken"); + const body = JSON.parse(mutation.body!) as { query: string; variables: Record }; + expect(body.query).toContain("createLinkedBranch"); + expect(body.variables).toEqual({ issueId: "I_issue12", oid: "basesha", name: "12-a-bug" }); + }); + + test("linkBranchToIssue returns the name GitHub actually created, not the requested one", async () => { + const { fetch } = linkBranchFetch(() => createdRef("12-a-bug-1", "newsha")); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(12, "12-a-bug")).toEqual({ name: "12-a-bug-1", sha: "newsha" }); + }); + + test("linkBranchToIssue throws ProviderError(401) without a token", async () => { + const { fetch, calls } = fakeFetch(Response.json({})); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect(error).toBeInstanceOf(ProviderError); + expect((error as ProviderError).status).toBe(401); + } + expect(calls).toHaveLength(0); + }); + + test("a GraphQL error on an HTTP 200 keeps its message when nothing can be resolved", async () => { + const { fetch } = linkBranchFetch(() => + Response.json({ data: null, errors: [{ message: "Resource not accessible by integration" }] }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect(error).toBeInstanceOf(ProviderError); + expect((error as ProviderError).status).toBe(502); + expect((error as ProviderError).message).toContain("Resource not accessible by integration"); + } + }); + + // A refused create may only mean the branch is already there, and GitHub has + // been seen reporting that both as a null `linkedBranch` on an HTTP 200 and as + // an outright refusal — so both go looking before failing. + test.each([ + ["a null linkedBranch with no errors", duplicateByNull], + ["a refusal", duplicateByError], + ])("%s resolves to the branch of the requested name linked to the issue", async (_label, duplicate) => { + const { fetch, calls } = linkBranchFetch((call) => + isMutation(call) ? duplicate() : linkedRefs({ name: "12-a-bug", oid: "existingsha" }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(12, "12-a-bug")).toEqual({ name: "12-a-bug", sha: "existingsha" }); + + const query = calls.filter((c) => c.url.endsWith("/graphql")).at(-1)!; + const body = JSON.parse(query.body!) as GraphQLCall; + expect(body.query).toContain("linkedBranches"); + expect(body.variables).toEqual({ owner: "owner", repo: "repo", number: 12 }); + }); + + test.each([ + ["a null linkedBranch with no errors", duplicateByNull], + ["a refusal", duplicateByError], + ])("%s ignores links under other names and takes the requested one", async (_label, duplicate) => { + const { fetch } = linkBranchFetch((call) => + isMutation(call) + ? duplicate() + : linkedRefs({ name: "12-something-else", oid: "othersha" }, { name: "12-a-bug", oid: "existingsha" }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(12, "12-a-bug")).toEqual({ name: "12-a-bug", sha: "existingsha" }); + }); + + test("a branch of the requested name beats an unrelated branch linked to the issue", async () => { + // Issue 42 was linked to "42-retries" by `gh issue develop`, and the name the + // caller previewed exists too. Answering with "42-retries" would put the + // workspace on a branch the user never saw. + const { fetch } = linkBranchFetch( + (call) => (isMutation(call) ? duplicateByNull() : linkedRefs({ name: "42-retries", oid: "OLDSHA" })), + { "42-add-retries-to-the-sync-loop": "requestedsha" }, + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(42, "42-add-retries-to-the-sync-loop")).toEqual({ + name: "42-add-retries-to-the-sync-loop", + sha: "requestedsha", + }); + }); + + test("a duplicate with no linked branch falls back to the branch of that name", async () => { + const { fetch } = linkBranchFetch( + (call) => (isMutation(call) ? duplicateByError() : linkedRefs()), + { "12-a-bug": "unlinkedsha" }, + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(12, "12-a-bug")).toEqual({ name: "12-a-bug", sha: "unlinkedsha" }); + }); + + test("a refusal whose wording nothing anticipated still resolves the existing branch", async () => { + const { fetch } = linkBranchFetch( + (call) => + isMutation(call) + ? Response.json({ errors: [{ message: "Referenz existiert bereits" }] }) + : linkedRefs({ name: "12-a-bug", oid: "existingsha" }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + expect(await provider.linkBranchToIssue(12, "12-a-bug")).toEqual({ name: "12-a-bug", sha: "existingsha" }); + }); + + test("a refused create with no branch of that name is a 409 carrying the reason", async () => { + const { fetch } = linkBranchFetch((call) => + isMutation(call) ? duplicateByError() : linkedRefs({ name: "12-something-else", oid: "othersha" }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect(error).toBeInstanceOf(ProviderError); + expect((error as ProviderError).status).toBe(409); + expect((error as ProviderError).message).toContain("12-a-bug"); + // The branch linked under another name is reported, never returned. + expect((error as ProviderError).message).toContain("already exists in the repository"); + } + }); + + test.each([ + ["a payload with no createLinkedBranch", { data: { createLinkedBranch: null } }], + [ + "a created ref missing its target oid", + { data: { createLinkedBranch: { linkedBranch: { ref: { name: "12-a-bug" } } } } }, + ], + ])("%s is a ProviderError(502), not a duplicate", async (_label, payload) => { + const { fetch, calls } = linkBranchFetch((call) => + isMutation(call) ? Response.json(payload) : linkedRefs({ name: "12-older-link", oid: "oldsha" }), + ); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect(error).toBeInstanceOf(ProviderError); + expect((error as ProviderError).status).toBe(502); + } + // An unreadable reply must not be answered with some older branch: the + // mutation may well have created the one that was asked for. + expect(calls.filter((c) => c.url.endsWith("/graphql"))).toHaveLength(1); + }); + + test.each([401, 403, 404])( + "an HTTP %i from the mutation is surfaced without looking for an existing branch", + async (status) => { + const { fetch, calls } = linkBranchFetch(() => Response.json({ message: "nope" }, { status })); + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect((error as ProviderError).status).toBe(status); + } + expect(calls.filter((c) => c.url.endsWith("/graphql"))).toHaveLength(1); + }, + ); + + test("a non-404 from the branch lookup is surfaced instead of 'no such branch'", async () => { + const { fetch } = linkBranchFetch((call) => { + if (isMutation(call)) return duplicateByNull(); + return linkedRefs(); + }); + // Re-wrap so the ref lookup 403s rather than 404s. + const forbidding = (async (input: string | URL | Request, init?: RequestInit) => { + if (/\/git\/ref\/heads\/12-a-bug$/.test(String(input))) { + return Response.json({ message: "Resource not accessible" }, { status: 403 }); + } + return fetch(input as string, init); + }) as unknown as typeof globalThis.fetch; + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch: forbidding }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect((error as ProviderError).status).toBe(403); + } + }); + + test("linkBranchToIssue reports an issue with no node id as a 502", async () => { + const fetch = (async (input: string | URL | Request) => { + if (String(input).endsWith("/issues/12")) return Response.json({ number: 12, title: "a bug" }); + throw new Error(`unexpected fetch: ${String(input)}`); + }) as unknown as typeof globalThis.fetch; + const provider = new GitHubProvider({ owner: "owner", repo: "repo", token: "t0ken", fetch }); + + try { + await provider.linkBranchToIssue(12, "12-a-bug"); + throw new Error("expected linkBranchToIssue to throw"); + } catch (error) { + expect((error as ProviderError).status).toBe(502); + } + }); + test("getFailedLogs returns [] when nothing failed and there are no Actions runs", async () => { const fetch = (async (input: string | URL | Request) => { const url = String(input); diff --git a/packages/fleet-bridge/tests/repo-branches.test.ts b/packages/fleet-bridge/tests/repo-branches.test.ts new file mode 100644 index 0000000..fb9e132 --- /dev/null +++ b/packages/fleet-bridge/tests/repo-branches.test.ts @@ -0,0 +1,203 @@ +/** + * repo-branches.test.ts — `GET /repos/:name/branches` driven in-process against + * a fake `Git.lsRemote`, so the route/manager mapping is exercised without a git + * binary or a reachable remote. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { GitError, type RemoteRef } from "git-bun"; +import { FleetManager } from "../src/fleet-manager"; +import { createApp } from "../src/api"; +import { Store } from "../src/store/store"; +import { makeDeps } from "./helpers"; + +/** What the fake `lsRemote` was asked, and what it answers with. */ +interface LsRemoteStub { + calls: { url: string; cwd: string; heads?: boolean; env?: Record }[]; + answer: () => RemoteRef[] | Promise; +} + +describe("GET /repos/:name/branches", () => { + let dir: string; + let manager: FleetManager; + let app: ReturnType; + let lsRemote: LsRemoteStub; + + async function call(method: string, path: string, body?: unknown) { + const res = await app.handle( + new Request(`http://bridge${path}`, { + method, + headers: body === undefined ? undefined : { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }), + ); + const text = await res.text(); + return { status: res.status, body: text ? JSON.parse(text) : undefined }; + } + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "fleet-bridge-branches-")); + lsRemote = { + calls: [], + answer: () => [ + { sha: "sha-main", ref: "refs/heads/main" }, + { sha: "sha-feature", ref: "refs/heads/feature/login" }, + { sha: "sha-alpha", ref: "refs/heads/alpha" }, + ], + }; + const config = { dataDirectory: dir, port: 4901, name: "bridge" }; + const store = new Store(dir); + await store.load(); + manager = new FleetManager(config, makeDeps(new Map()), { + syncTimeoutMs: 50, + store, + lsRemoteTimeoutMs: 50, + lsRemote: async (url, options) => { + lsRemote.calls.push({ url, cwd: options.cwd, heads: options.heads, env: options.env }); + return lsRemote.answer(); + }, + }); + await manager.init(); + app = createApp(manager); + expect((await call("POST", "/repos", { name: "repo1", url: "git@fake/repo1.git" })).status).toBe(201); + }); + afterEach(async () => { + manager.shutdown(); + await rm(dir, { recursive: true, force: true }); + }); + + test("maps refs/heads/* to {name, sha}, sorted ascending", async () => { + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.status).toBe(200); + expect(res.body).toEqual([ + { name: "alpha", sha: "sha-alpha" }, + { name: "feature/login", sha: "sha-feature" }, + { name: "main", sha: "sha-main" }, + ]); + }); + + test("probes the repo's url from the bridge's data directory, branches only", async () => { + await call("GET", "/repos/repo1/branches"); + + expect(lsRemote.calls).toHaveLength(1); + expect(lsRemote.calls[0]).toMatchObject({ url: "git@fake/repo1.git", cwd: dir, heads: true }); + }); + + test("runs git non-interactively and under its own deadlines", async () => { + await call("GET", "/repos/repo1/branches"); + + // Without the first three git opens /dev/tty for the prompt — the bridge runs + // in an operator's terminal, so the request would block until someone typed. + // The last two make git abandon a stalled transfer itself, which is the only + // thing that reaps the process: giving up on the promise does not. + expect(lsRemote.calls[0]!.env).toEqual({ + GIT_TERMINAL_PROMPT: "0", + GIT_ASKPASS: "", + GIT_SSH_COMMAND: "ssh -oBatchMode=yes -oConnectTimeout=10", + GIT_HTTP_LOW_SPEED_LIMIT: "1", + GIT_HTTP_LOW_SPEED_TIME: "15", + }); + }); + + test("a probe that never settles is abandoned as a 502 instead of hanging", async () => { + lsRemote.answer = () => new Promise(() => {}); + + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.status).toBe(502); + expect(res.body.error).toContain("timed out"); + expect(res.body.error).toContain("repo1"); + }); + + test("drops refs that are not branches", async () => { + lsRemote.answer = () => [ + { sha: "sha-main", ref: "refs/heads/main" }, + { sha: "sha-tag", ref: "refs/tags/v1.0.0" }, + { sha: "sha-pull", ref: "refs/pull/7/head" }, + { sha: "sha-head", ref: "HEAD" }, + ]; + + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.body).toEqual([{ name: "main", sha: "sha-main" }]); + }); + + test("an unregistered repo returns 404", async () => { + expect((await call("GET", "/repos/ghost/branches")).status).toBe(404); + expect(lsRemote.calls).toHaveLength(0); + }); + + test("an invalid repo identifier returns 400", async () => { + expect((await call("GET", "/repos/..%2Fescape/branches")).status).toBe(400); + }); + + test("a token embedded in the repo url never reaches the error response", async () => { + const url = "https://x-access-token:ghp_SECRET@github.com/acme/private.git"; + expect((await call("POST", "/repos", { name: "private", url })).status).toBe(201); + lsRemote.answer = () => { + // git replays the command line — token and all — in the GitError message, + // and redacts it only in its own stderr. + throw new GitError(["ls-remote", "--heads", "--", url], { + stdout: "", + stderr: "fatal: Authentication failed for 'https://github.com/acme/private.git/'", + exitCode: 128, + }); + }; + + const res = await call("GET", "/repos/private/branches"); + + expect(res.status).toBe(502); + expect(res.body.error).not.toContain("ghp_SECRET"); + expect(res.body.error).toContain("Authentication failed"); + }); + + test("a password containing @ is redacted whole, not up to its first @", async () => { + lsRemote.answer = () => { + throw new GitError(["ls-remote"], { + stdout: "", + stderr: "fatal: could not read from 'https://user:p@ssw0rd@github.com/o/r.git'", + exitCode: 128, + }); + }; + + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.body.error).not.toContain("ssw0rd"); + expect(res.body.error).toContain("https://***@github.com/o/r.git"); + }); + + test("credentials git echoes back in its own stderr are redacted too", async () => { + lsRemote.answer = () => { + throw new GitError(["ls-remote"], { + stdout: "", + stderr: "fatal: could not read from 'https://user:ghp_SECRET@github.com/acme/private.git'", + exitCode: 128, + }); + }; + + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.body.error).not.toContain("ghp_SECRET"); + expect(res.body.error).toContain("https://***@github.com/acme/private.git"); + }); + + test("an unreachable remote surfaces as 502 naming the repo", async () => { + lsRemote.answer = () => { + throw new GitError(["ls-remote", "--heads", "--", "git@fake/repo1.git"], { + stdout: "", + stderr: "fatal: repository not found", + exitCode: 128, + }); + }; + + const res = await call("GET", "/repos/repo1/branches"); + + expect(res.status).toBe(502); + expect(res.body.error).toContain("repo1"); + expect(res.body.error).toContain("repository not found"); + }); +}); diff --git a/packages/fleet-bridge/tests/repo-provider-api.test.ts b/packages/fleet-bridge/tests/repo-provider-api.test.ts index 14d7363..2a330a2 100644 --- a/packages/fleet-bridge/tests/repo-provider-api.test.ts +++ b/packages/fleet-bridge/tests/repo-provider-api.test.ts @@ -166,6 +166,11 @@ describe("repo provider API", () => { recorder.failedLogsRef = ref; return [failedLog]; }, + // Unused by these routes, but the interface requires it. + async linkBranchToIssue(_issueNumber: number, branch: string) { + guard(); + return { name: branch, sha: "sha-of-linked-branch" }; + }, }; } diff --git a/packages/fleet-bridge/tests/workspace-from-issue.test.ts b/packages/fleet-bridge/tests/workspace-from-issue.test.ts new file mode 100644 index 0000000..896a878 --- /dev/null +++ b/packages/fleet-bridge/tests/workspace-from-issue.test.ts @@ -0,0 +1,249 @@ +/** + * workspace-from-issue.test.ts — `POST /workspaces` driven in-process with both + * a fake ship and a fake provider, covering the `issueNumber` branch source: + * which provider calls it makes, what branch the ship ends up being handed, and + * the validation and reservation behaviour around a provider failure. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { Repo } from "fleet-protocol"; +import { FleetManager } from "../src/fleet-manager"; +import { createApp } from "../src/api"; +import { Store } from "../src/store/store"; +import { ProviderError, type Issue, type RepoProvider } from "../src/providers"; +import { FakeSocket, makeDeps, type FakeShip } from "./helpers"; + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +/** Records what the fake provider was asked to do. */ +interface Recorder { + getIssueNumber?: number; + linkBranch?: { issueNumber: number; branch: string }; + linkCalls: number; +} + +const issue: Issue = { + number: 12, + title: "Better create workspace issue", + state: "open", + author: "octocat", + url: "https://github.com/acme/repo1/issues/12", + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-02T00:00:00.000Z", + body: "details", + comments: 1, +}; + +describe("POST /workspaces from an issue", () => { + let dir: string; + let manager: FleetManager; + let app: ReturnType; + let ships: Map; + let recorder: Recorder; + /** When set, `linkBranchToIssue` throws this instead of succeeding. */ + let linkFailsWith: ProviderError | undefined; + /** Name the provider claims to have created; defaults to the requested one. */ + let linkReturns: string | undefined; + /** When set, `linkBranchToIssue` reports arrival and blocks until released. */ + let linkGate: { entered: () => void; wait: Promise } | undefined; + + function makeProvider(_repo: Repo): RepoProvider { + const unused = () => { + throw new Error("not used by these tests"); + }; + return { + getInfo: unused, + listIssues: unused, + listPullRequests: unused, + getPullRequest: unused, + commentOnIssue: unused, + commentOnPullRequest: unused, + reviewPullRequest: unused, + listChecks: unused, + getFailedLogs: unused, + async getIssue(number: number) { + recorder.getIssueNumber = number; + return issue; + }, + async linkBranchToIssue(issueNumber: number, branch: string) { + recorder.linkBranch = { issueNumber, branch }; + recorder.linkCalls += 1; + linkGate?.entered(); + await linkGate?.wait; + if (linkFailsWith) throw linkFailsWith; + return { name: linkReturns ?? branch, sha: "sha-of-linked-branch" }; + }, + }; + } + + async function call(method: string, path: string, body?: unknown) { + const res = await app.handle( + new Request(`http://bridge${path}`, { + method, + headers: body === undefined ? undefined : { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }), + ); + const text = await res.text(); + return { status: res.status, body: text ? JSON.parse(text) : undefined }; + } + + /** The branch the fake ship recorded for a workspace it was asked to create. */ + function branchTheShipReceived(name: string): string | undefined { + return ships.get("http://ship-a")!.workspaces.find((w) => w.name === name)?.branch; + } + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "fleet-bridge-issue-ws-")); + FakeSocket.byBase.clear(); + recorder = { linkCalls: 0 }; + linkFailsWith = undefined; + linkReturns = undefined; + linkGate = undefined; + ships = new Map([["http://ship-a", { name: "ship-a", workspaces: [] }]]); + const config = { dataDirectory: dir, port: 4902, name: "bridge" }; + const store = new Store(dir); + await store.load(); + await store.createShip({ name: "ship-a", url: "http://ship-a" }); + manager = new FleetManager(config, makeDeps(ships), { + syncTimeoutMs: 50, + store, + providerFor: makeProvider, + }); + await manager.init(); + app = createApp(manager); + expect( + (await call("POST", "/repos", { name: "repo1", url: "https://github.com/acme/repo1", provider: "github" })) + .status, + ).toBe(201); + }); + afterEach(async () => { + manager.shutdown(); + await rm(dir, { recursive: true, force: true }); + }); + + test("derives the branch name from the issue and links it before cloning", async () => { + const res = await call("POST", "/workspaces", { + ship: "ship-a", + repoName: "repo1", + name: "twelve", + issueNumber: 12, + }); + + expect(res.status).toBe(201); + expect(recorder.getIssueNumber).toBe(12); + expect(recorder.linkBranch).toEqual({ issueNumber: 12, branch: "12-better-create-workspace-issue" }); + expect(branchTheShipReceived("twelve")).toBe("12-better-create-workspace-issue"); + expect(res.body).toMatchObject({ + repoName: "repo1", + name: "twelve", + branch: "12-better-create-workspace-issue", + ship: "ship-a", + }); + }); + + test("hands the ship the name the provider returned, not the computed one", async () => { + linkReturns = "12-better-create-workspace-issue-1"; + + const res = await call("POST", "/workspaces", { + ship: "ship-a", + repoName: "repo1", + name: "twelve", + issueNumber: 12, + }); + + expect(res.status).toBe(201); + expect(recorder.linkBranch!.branch).toBe("12-better-create-workspace-issue"); + expect(branchTheShipReceived("twelve")).toBe("12-better-create-workspace-issue-1"); + expect(res.body.branch).toBe("12-better-create-workspace-issue-1"); + }); + + test("a plain branch create never touches the provider", async () => { + const res = await call("POST", "/workspaces", { + ship: "ship-a", + repoName: "repo1", + name: "plain", + branch: "main", + }); + + expect(res.status).toBe(201); + expect(res.body.branch).toBe("main"); + expect(recorder).toEqual({ linkCalls: 0 }); + }); + + test.each([ + ["both a branch and an issue", { branch: "main", issueNumber: 12 }], + ["neither", {}], + ["a blank branch", { branch: " " }], + ["a non-integer issue number", { issueNumber: 1.5 }], + ["a zero issue number", { issueNumber: 0 }], + ["an issue number past the safe integer range", { issueNumber: 1e21 }], + ])("rejects %s with 400", async (_label, extra) => { + const res = await call("POST", "/workspaces", { + ship: "ship-a", + repoName: "repo1", + name: "rejected", + ...extra, + }); + + expect(res.status).toBe(400); + expect(recorder).toEqual({ linkCalls: 0 }); + expect(ships.get("http://ship-a")!.createCalls).toBeUndefined(); + }); + + test("a failed link surfaces its status and leaves no reservation behind", async () => { + linkFailsWith = new ProviderError("authentication required", 401); + const body = { ship: "ship-a", repoName: "repo1", name: "twelve", issueNumber: 12 }; + + const first = await call("POST", "/workspaces", body); + expect(first.status).toBe(401); + expect(ships.get("http://ship-a")!.createCalls).toBeUndefined(); + + // A retry must fail the same way, not 409 on a reservation the first attempt left. + const second = await call("POST", "/workspaces", body); + expect(second.status).toBe(401); + expect(second.body.error).not.toContain("already in progress"); + }); + + test("a ship failure after a successful link clears the reservation too", async () => { + ships.get("http://ship-a")!.errorResponse = { status: 409, message: "clone destination exists" }; + const body = { ship: "ship-a", repoName: "repo1", name: "twelve", issueNumber: 12 }; + + expect((await call("POST", "/workspaces", body)).status).toBe(409); + + ships.get("http://ship-a")!.errorResponse = undefined; + const retry = await call("POST", "/workspaces", body); + expect(retry.status).toBe(201); + expect(retry.body.branch).toBe("12-better-create-workspace-issue"); + }); + + test("the reservation is taken before the provider call, so a concurrent create links once", async () => { + const entered = deferred(); + const release = deferred(); + linkGate = { entered: entered.resolve, wait: release.promise }; + const body = { ship: "ship-a", repoName: "repo1", name: "twelve", issueNumber: 12 }; + + const first = call("POST", "/workspaces", body); + await entered.promise; + const second = await call("POST", "/workspaces", body); + + expect(second.status).toBe(409); + expect(second.body.error).toContain("already in progress"); + // The second request must have been turned away before reaching the provider: + // two linked branches for one workspace is exactly what the ordering prevents. + expect(recorder.linkCalls).toBe(1); + + release.resolve(); + expect((await first).status).toBe(201); + expect(recorder.linkCalls).toBe(1); + }); +}); diff --git a/packages/fleet-client/src/App.tsx b/packages/fleet-client/src/App.tsx index 6d1bc66..15efc57 100644 --- a/packages/fleet-client/src/App.tsx +++ b/packages/fleet-client/src/App.tsx @@ -1,6 +1,6 @@ import "./index.css"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { BrowserRouter, Route, Routes } from "react-router-dom"; import { FleetProvider } from "./data/FleetContext"; import { Shell } from "./layouts/Shell"; @@ -17,6 +17,12 @@ export function App() { const [theme, setTheme] = useState("dark"); const toggleTheme = () => setTheme((t) => (t === "dark" ? "light" : "dark")); + // On the document root rather than a wrapper element so that content + // portalled to `document.body` — the combobox list — is themed too. + useEffect(() => { + document.documentElement.classList.toggle("dark", theme === "dark"); + }, [theme]); + return ( diff --git a/packages/fleet-client/src/components/CreateWorkspaceModal.tsx b/packages/fleet-client/src/components/CreateWorkspaceModal.tsx index f176435..54eacc5 100644 --- a/packages/fleet-client/src/components/CreateWorkspaceModal.tsx +++ b/packages/fleet-client/src/components/CreateWorkspaceModal.tsx @@ -1,10 +1,19 @@ -import { useState } from "react"; +import { useEffect, useState, type FormEvent, type ReactNode } from "react"; import { useFleet } from "@/data/FleetContext"; +import type { RepoBranch, RepoIssue } from "@/data/types"; import { Modal } from "@/components/ui/modal"; import { Input } from "@/components/ui/input"; +import { Checkbox } from "@/components/ui/checkbox"; +import { Combobox, highlight, splitRanges } from "@/components/ui/combobox"; +import { cn } from "@/lib/utils"; +import { branchState, createWorkspaceInput, issueBranchPreview, issueText } from "@/lib/create-workspace"; import { Field, ModalActions } from "@/routes/ReposRoute"; import { useSubmitAction } from "@/lib/useSubmitAction"; +// Module-level so the pickers' memoised filtering is not invalidated every render. +const branchName = (branch: RepoBranch) => branch.name; +const issueKey = (issue: RepoIssue) => String(issue.number); + interface Props { repoName: string; /** When set, the ship is fixed (Bridge cell entry); otherwise a dropdown is shown. */ @@ -12,22 +21,93 @@ interface Props { onClose: () => void; } +/** + * The create-workspace form. The branch can be picked two ways: by name, out of + * the repo's remote branches (or typed, which creates it), or by open issue, in + * which case the bridge derives and links the branch and the client only previews + * the name it will pick. + * + * Both lists are allowed to be missing — `GET /repos/:name/branches` needs to + * reach the remote and the issues route needs a provider and a token — so + * neither is on the critical path: a repo whose branches cannot be listed falls + * back to the plain text field this form used to be, and issues are not even + * requested until the checkbox is ticked, since for a `custom` repo that request + * is a guaranteed 501. + */ export function CreateWorkspaceModal({ repoName, ship, onClose }: Props) { - const { ships, createWorkspace } = useFleet(); + const { ships, createWorkspace, listRepoBranches, listRepoIssues } = useFleet(); const [name, setName] = useState(""); const [branch, setBranch] = useState("main"); const [selectedShip, setSelectedShip] = useState(ship ?? ships[0]?.name ?? ""); + // null while unknown — loading or failed. `branchState` reads it as "cannot say". + const [branches, setBranches] = useState(null); + const [branchesError, setBranchesError] = useState(null); + + const [fromIssue, setFromIssue] = useState(false); + const [issues, setIssues] = useState([]); + const [issuesLoaded, setIssuesLoaded] = useState(false); + const [issuesLoading, setIssuesLoading] = useState(false); + const [issuesError, setIssuesError] = useState(null); + const [issueQuery, setIssueQuery] = useState(""); + const [issue, setIssue] = useState(null); + const shipName = ship ?? selectedShip; - const { error, pending, submit } = useSubmitAction( - () => createWorkspace({ ship: shipName, repoName, name: name.trim(), branch: branch.trim() }), - onClose, - ); + useEffect(() => { + let cancelled = false; + void (async () => { + try { + const loaded = await listRepoBranches(repoName); + if (!cancelled) setBranches(loaded); + } catch (e) { + if (!cancelled) setBranchesError((e as Error).message); + } + })(); + return () => { + cancelled = true; + }; + }, [listRepoBranches, repoName]); + + // Lazy on purpose: see the component's note about `custom` repos. A failed load + // leaves `issuesLoaded` false, so unticking and re-ticking retries. + useEffect(() => { + if (!fromIssue || issuesLoaded) return; + let cancelled = false; + setIssuesLoading(true); + setIssuesError(null); + void (async () => { + try { + const loaded = await listRepoIssues(repoName); + if (cancelled) return; + setIssues(loaded); + setIssuesLoaded(true); + } catch (e) { + if (!cancelled) setIssuesError((e as Error).message); + } finally { + if (!cancelled) setIssuesLoading(false); + } + })(); + return () => { + cancelled = true; + }; + }, [fromIssue, issuesLoaded, listRepoIssues, repoName]); + + const state = branchState(branch, branches); + const input = createWorkspaceInput({ ship: shipName, repoName, name, fromIssue, branch, issue }); + + const { error, pending, submit } = useSubmitAction(() => createWorkspace(input!), onClose); + + const onSubmit = (e: FormEvent) => { + e.preventDefault(); + // `input` being null is what disables the Create button, but implicit + // submission can still reach here. + if (input && !pending) void submit(); + }; return ( -
+
▣ {repoName}
 @@ -53,18 +133,108 @@ export function CreateWorkspaceModal({ repoName, ship, onClose }: Props) { setName(e.target.value)} placeholder="feature-x" autoFocus /> - - setBranch(e.target.value)} placeholder="main" /> - + + + Create from issue + + + {fromIssue ? ( + + { + setIssueQuery(value); + // Editing the text abandons the selection: what the field says and + // what would be submitted must never disagree. + setIssue(null); + }} + items={issues} + toText={issueText} + toKey={issueKey} + renderItem={(i, ranges) => } + onSelect={(i) => { + setIssue(i); + setIssueQuery(issueText(i)); + }} + placeholder="Search open issues" + disabled={issuesError !== null} + // No message while the issues are still on their way: the list + // would claim nothing matches over the "listing issues…" below it. + emptyMessage={issuesLoading ? undefined : "No open issue matches."} + /> + {issuesError ? ( + Issues could not be listed: {issuesError} + ) : issuesLoading ? ( + listing issues… + ) : issue ? ( + + ) : null} + + ) : ( + + setBranch(b.name)} + placeholder="main" + allowFreeText + /> + {state.kind === "existing" && On branch {state.branch}} + {state.kind === "new" && Creating new branch {state.branch}} + {state.kind === "unknown" && ( + + {branchesError ? "Branches could not be listed — type a branch name." : "listing branches…"} + + )} + + )} {error &&

{error}

} - +
); } + +/** One line of small mono text under a field. */ +function Note({ children, className }: { children: ReactNode; className: string }) { + return {children}; +} + +function IssueRow({ issue, ranges }: { issue: RepoIssue; ranges: [number, number][] }) { + const text = issueText(issue); + const pivot = `#${issue.number}`.length; + const [numberRanges, titleRanges] = splitRanges(ranges, pivot); + return ( + + {highlight(text.slice(0, pivot), numberRanges)} + {highlight(text.slice(pivot), titleRanges)} + {issue.author && @{issue.author}} + + ); +} + +function SelectedIssue({ issue }: { issue: RepoIssue }) { + const preview = issueBranchPreview(issue); + return ( + + + {issueText(issue)} ↗ + + {preview && ( + + Creating new branch {preview}{" "} + (the provider may adjust the name) + + )} + + ); +} diff --git a/packages/fleet-client/src/components/ui/checkbox.tsx b/packages/fleet-client/src/components/ui/checkbox.tsx new file mode 100644 index 0000000..9a7d3ed --- /dev/null +++ b/packages/fleet-client/src/components/ui/checkbox.tsx @@ -0,0 +1,40 @@ +import { cn } from "@/lib/utils"; + +/** + * ui/checkbox.tsx — a real `` with a label, styled to the + * Bridge design language. Native rather than a composed widget: `accent-color` + * is enough to tint the box, and the browser's own control keeps keyboard and + * screen-reader behaviour for free. + */ +export function Checkbox({ + checked, + onChange, + disabled, + children, + className, +}: { + checked: boolean; + onChange: (checked: boolean) => void; + disabled?: boolean; + children: React.ReactNode; + className?: string; +}) { + return ( + + ); +} diff --git a/packages/fleet-client/src/components/ui/combobox.tsx b/packages/fleet-client/src/components/ui/combobox.tsx new file mode 100644 index 0000000..7d2b526 --- /dev/null +++ b/packages/fleet-client/src/components/ui/combobox.tsx @@ -0,0 +1,416 @@ +import { + useCallback, + useEffect, + useId, + useMemo, + useRef, + useState, + type CSSProperties, + type KeyboardEvent, + type MouseEvent, + type ReactNode, +} from "react"; +import { createPortal } from "react-dom"; +import { cn } from "@/lib/utils"; +import { fuzzySearch } from "@/lib/fuzzy"; +import { Input } from "@/components/ui/input"; + +/** + * ui/combobox.tsx — a controlled "type to filter, pick from a list" input. + * + * Generic over the item because the create-workspace modal needs the same + * behaviour twice, over branches and over issues. Filtering is + * {@link fuzzySearch}, done once per render and handed to `renderItem` as the + * ranges to highlight, so the list rendering never re-derives the match. + * + * The list is **portalled into `document.body` and positioned `fixed`** against + * the input's viewport rect. Its only home is inside a `Modal`, and the two + * simpler options each break there: absolutely positioned inside the panel it is + * clipped, because the panel is `overflow-hidden` and its body `overflow-y-auto` + * and any non-`visible` overflow ancestor clips an out-of-flow descendant; laid + * out in normal flow it is not clipped, but then its *presence* sets the height + * of everything below it, so unmounting it on blur reflows the panel between a + * mousedown and its mouseup and the click lands somewhere else entirely — on the + * backdrop, in the worst case, discarding the form. Portalled and fixed, it is + * neither clipped nor part of any layout. + * + * Two consequences of the portal, both relied upon: + * - React routes events through the *React* tree, not the DOM tree, so a click on + * a row still passes through `Modal`'s panel handler and is stopped there — it + * cannot reach the backdrop's `onClose`. + * - The rows are no longer inside the `