From 903022f72abd573c4807d1e10d5c80f6eacd192e Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 01:07:16 -0500 Subject: [PATCH 01/46] feat(opencode): add GitHub PR backend - VCS routes, PR creation, merge, and comment handling - Extend Vcs namespace with PrInfo, GithubCapability types and enriched VcsInfo - Add pr.ts for PR creation/merge/update operations via gh CLI - Add pr-comments.ts for fetching and addressing PR review comments - Extract VCS routes into dedicated routes/vcs.ts with full PR endpoints --- packages/opencode/src/project/pr-comments.ts | 176 +++++++ packages/opencode/src/project/pr.ts | 233 ++++++++ packages/opencode/src/project/vcs.ts | 528 ++++++++++++++++--- packages/opencode/src/server/routes/vcs.ts | 242 +++++++++ packages/opencode/src/server/server.ts | 25 +- 5 files changed, 1109 insertions(+), 95 deletions(-) create mode 100644 packages/opencode/src/project/pr-comments.ts create mode 100644 packages/opencode/src/project/pr.ts create mode 100644 packages/opencode/src/server/routes/vcs.ts diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts new file mode 100644 index 000000000000..e436337943ef --- /dev/null +++ b/packages/opencode/src/project/pr-comments.ts @@ -0,0 +1,176 @@ +import { $ } from "bun" +import z from "zod" +import { Log } from "@/util/log" +import { Instance } from "./instance" +import { Vcs } from "./vcs" +import { PR } from "./pr" + +const log = Log.create({ service: "pr-comments" }) + +export namespace PrComments { + export const ReviewComment = z + .object({ + id: z.number(), + author: z.string(), + body: z.string(), + path: z.string(), + line: z.number().nullable(), + diffHunk: z.string().optional(), + }) + .meta({ ref: "ReviewComment" }) + export type ReviewComment = z.infer + + export const ReviewThread = z + .object({ + id: z.string(), + isResolved: z.boolean(), + path: z.string(), + line: z.number().nullable(), + comments: ReviewComment.array(), + }) + .meta({ ref: "ReviewThread" }) + export type ReviewThread = z.infer + + export const CommentsResponse = z + .object({ + threads: ReviewThread.array(), + promptBlock: z.string(), + unresolvedCount: z.number(), + }) + .meta({ ref: "PrCommentsResponse" }) + export type CommentsResponse = z.infer + + function buildQuery(owner: string, name: string, prNumber: number, cursor: string | null): string { + return `query($owner: String!, $name: String!, $prNumber: Int!, $cursor: String) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 50) { nodes { databaseId author { login } body path line: originalLine diffHunk } } } } } } }` + } + + export async function fetch(): Promise { + const info = await Vcs.info() + if (!info.pr) { + throw new PR.PrError("NO_PR", "No pull request found for current branch") + } + if (!info.github?.authenticated || !info.github.repo) { + throw new PR.PrError("GH_NOT_AUTHENTICATED", "GitHub CLI is not authenticated") + } + + const { owner, name } = info.github.repo + const prNumber = info.pr.number + const cwd = Instance.worktree + + const allThreads: ReviewThread[] = [] + let cursor: string | null = null + + do { + const query = buildQuery(owner, name, prNumber, cursor) + const args = [ + "gh", + "api", + "graphql", + "-f", + `query=${query}`, + "-f", + `owner=${owner}`, + "-f", + `name=${name}`, + "-F", + `prNumber=${prNumber}`, + ] + if (cursor) { + args.push("-f", `cursor=${cursor}`) + } + const cmd = await $`${args}`.quiet().nothrow().cwd(cwd) + + const stdout = cmd.stdout.toString() + const stderr = cmd.stderr.toString() + + if (cmd.exitCode !== 0) { + log.error("gh api graphql failed", { exitCode: cmd.exitCode, stderr }) + throw new PR.PrError( + "COMMENTS_FETCH_FAILED", + stderr.trim() || `gh api graphql exited with code ${cmd.exitCode}`, + ) + } + + if (!stdout.trim()) { + log.error("gh api graphql returned empty response") + throw new PR.PrError("COMMENTS_FETCH_FAILED", "GitHub API returned empty response") + } + + let parsed: Record + try { + parsed = JSON.parse(stdout) + } catch { + log.error("gh api graphql returned invalid JSON", { stdout: stdout.slice(0, 200) }) + throw new PR.PrError("COMMENTS_FETCH_FAILED", "GitHub API returned invalid response") + } + + if (parsed.errors) { + const gqlErrors = parsed.errors as Array<{ message?: string }> + const msg = gqlErrors.map((e) => e.message).join("; ") + log.error("GraphQL errors", { errors: parsed.errors }) + throw new PR.PrError("COMMENTS_FETCH_FAILED", msg || "GraphQL query failed") + } + + const reviewThreads = (parsed.data as Record | undefined)?.repository as + | Record + | undefined + const prData = reviewThreads?.pullRequest as Record | undefined + const threads = prData?.reviewThreads as + | { pageInfo?: { hasNextPage?: boolean; endCursor?: string }; nodes?: Array> } + | undefined + + if (!threads) break + + for (const node of threads.nodes ?? []) { + const commentsNodes = (node.comments as { nodes?: Array> })?.nodes ?? [] + allThreads.push({ + id: node.id as string, + isResolved: node.isResolved as boolean, + path: (node.path as string) ?? "", + line: node.line as number | null, + comments: commentsNodes.map((c) => ({ + id: c.databaseId as number, + author: (c.author as { login?: string })?.login ?? "unknown", + body: (c.body as string) ?? "", + path: (c.path as string) ?? "", + line: (c.line as number | null) ?? null, + diffHunk: c.diffHunk as string | undefined, + })), + }) + } + + cursor = threads.pageInfo?.hasNextPage ? (threads.pageInfo.endCursor ?? null) : null + } while (cursor) + + const unresolved = allThreads.filter((t) => !t.isResolved) + const promptBlock = formatPromptBlock(unresolved) + + return { + threads: unresolved, + promptBlock, + unresolvedCount: unresolved.length, + } + } + + function formatPromptBlock(threads: ReviewThread[]): string { + if (threads.length === 0) return "No unresolved review comments." + + const blocks = threads.map((thread) => { + if (thread.comments.length === 0) return "" + + const lines = [`### File: ${thread.path}${thread.line ? ` (line ${thread.line})` : ""}`] + for (const comment of thread.comments) { + lines.push(`**@${comment.author}** (comment ID: ${comment.id}): ${comment.body}`) + if (comment.diffHunk) { + const contextLine = comment.diffHunk.split("\n").pop() + if (contextLine) { + lines.push(`> Code context: \`${contextLine.replace(/^[+-]/, "").trim()}\``) + } + } + } + return lines.join("\n") + }) + + return blocks.filter(Boolean).join("\n\n") + } +} diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts new file mode 100644 index 000000000000..8d8aa9a7533f --- /dev/null +++ b/packages/opencode/src/project/pr.ts @@ -0,0 +1,233 @@ +import { $ } from "bun" +import z from "zod" +import { Log } from "@/util/log" +import { Instance } from "./instance" +import { Vcs } from "./vcs" + +const log = Log.create({ service: "pr" }) + +export namespace PR { + export const ErrorCode = z.enum([ + "GH_NOT_INSTALLED", + "GH_NOT_AUTHENTICATED", + "NO_REPO", + "NO_PR", + "UPSTREAM_MISSING", + "CREATE_FAILED", + "MERGE_FAILED", + "DELETE_BRANCH_FAILED", + "COMMENTS_FETCH_FAILED", + ]) + export type ErrorCode = z.infer + + export class PrError extends Error { + constructor( + public code: ErrorCode, + message: string, + ) { + super(message) + this.name = "PrError" + } + } + + export const CreateInput = z + .object({ + title: z.string(), + body: z.string(), + base: z.string().optional(), + draft: z.boolean().optional(), + }) + .meta({ ref: "PrCreateInput" }) + export type CreateInput = z.infer + + export const MergeInput = z + .object({ + strategy: z.enum(["merge", "squash", "rebase"]).optional(), + deleteBranch: z.boolean().optional(), + }) + .meta({ ref: "PrMergeInput" }) + export type MergeInput = z.infer + + export const DeleteBranchInput = z + .object({ + branch: z.string(), + }) + .meta({ ref: "PrDeleteBranchInput" }) + export type DeleteBranchInput = z.infer + + export const PrErrorResponse = z + .object({ + code: ErrorCode, + message: z.string(), + }) + .meta({ ref: "PrErrorResponse" }) + + async function ensureGithub() { + const info = await Vcs.info() + const github = info.github + if (!github?.available) { + throw new PrError("GH_NOT_INSTALLED", "GitHub CLI (gh) is not installed") + } + if (!github.authenticated) { + throw new PrError("GH_NOT_AUTHENTICATED", "Run `gh auth login` to authenticate") + } + return { info, github: github as Required } + } + + export async function get(): Promise { + const info = await Vcs.info() + return info.pr + } + + export async function create(input: CreateInput): Promise { + const { info } = await ensureGithub() + const cwd = Instance.worktree + + if (info.pr) { + return info.pr + } + + const push = await $`git push -u origin HEAD`.quiet().nothrow().cwd(cwd) + if (push.exitCode !== 0) { + const errorOutput = + push.stderr?.toString().trim() || push.stdout?.toString().trim() || "Failed to push branch automatically" + log.error("push failed", { output: errorOutput }) + throw new PrError("CREATE_FAILED", errorOutput) + } + + const args = ["gh", "pr", "create", "--title", input.title] + args.push("--body", input.body) + if (input.base) args.push("--base", input.base) + if (input.draft) args.push("--draft") + + const cmd = await $`${args}` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + + const result = cmd.stdout.toString() + const errorOut = cmd.stderr.toString() + + if (cmd.exitCode !== 0 || !result.trim()) { + log.error("pr create failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) + throw new PrError("CREATE_FAILED", errorOut.trim() || result.trim() || "Failed to create pull request") + } + + await Vcs.refresh() + const updated = await Vcs.info() + if (updated.pr) { + return updated.pr + } + + const prUrl = result.trim().split("\n").pop() ?? "" + const numberMatch = prUrl.match(/\/pull\/(\d+)/) + return { + number: numberMatch ? parseInt(numberMatch[1], 10) : 0, + url: prUrl, + title: input.title, + state: "OPEN", + headRefName: info.branch, + baseRefName: input.base ?? info.defaultBranch ?? "main", + isDraft: input.draft ?? false, + mergeable: "UNKNOWN", + reviewDecision: null, + checksState: null, + } + } + + export async function merge(input: MergeInput): Promise { + const { info } = await ensureGithub() + const cwd = Instance.worktree + + const currentPr = await get() + if (!currentPr) { + throw new PrError("NO_PR", "No pull request found for the current branch") + } + + const github = info.github + if (!github?.repo) { + throw new PrError("MERGE_FAILED", "Unable to determine repository information") + } + + const strategy = input.strategy ?? "squash" + const mergeMethod = strategy === "squash" ? "squash" : strategy === "merge" ? "merge" : "rebase" + + const args = [ + "gh", + "api", + `repos/${github.repo.owner}/${github.repo.name}/pulls/${currentPr.number}/merge`, + "-X", + "PUT", + "-f", + `merge_method=${mergeMethod}`, + ] + + if (input.deleteBranch !== false) { + args.push("-f", "delete_branch=true") + } + + const cmd = await $`${args}` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + + const responseText = cmd.stdout.toString().trim() + const errorOut = cmd.stderr.toString().trim() + + if (cmd.exitCode !== 0 || !responseText) { + let errorMessage = errorOut || responseText || "Failed to merge pull request" + try { + const errorJson = JSON.parse(errorOut || responseText) + if (errorJson.message) { + errorMessage = errorJson.message + } + } catch {} + log.error("pr merge failed", { stderr: errorOut, stdout: responseText, exitCode: cmd.exitCode }) + throw new PrError("MERGE_FAILED", errorMessage) + } + + await Vcs.refresh() + const updated = await Vcs.info() + return updated.pr ?? { ...currentPr, state: "MERGED" } + } + + export async function deleteBranch(input: DeleteBranchInput): Promise { + await ensureGithub() + const cwd = Instance.worktree + + // Delete remote branch + const remote = await $`git push origin --delete ${input.branch}` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + + if (remote.exitCode !== 0) { + const errorOut = remote.stderr.toString().trim() + // Ignore "remote ref does not exist" — branch may already be deleted + if (!errorOut.includes("remote ref does not exist")) { + log.error("delete remote branch failed", { stderr: errorOut }) + throw new PrError("DELETE_BRANCH_FAILED", errorOut || "Failed to delete remote branch") + } + } + + // Clean up local branch if it exists and isn't the current one + const currentBranch = await $`git rev-parse --abbrev-ref HEAD` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + if (currentBranch.trim() !== input.branch) { + await $`git branch -D ${input.branch}` + .quiet() + .nothrow() + .cwd(cwd) + .catch(() => {}) + } + + await Vcs.refresh() + } +} diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index dea25b91b43b..3ef218591de9 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -1,16 +1,50 @@ -import { Effect, Layer, ServiceMap } from "effect" -import { Bus } from "@/bus" import { BusEvent } from "@/bus/bus-event" -import { InstanceState } from "@/effect/instance-state" -import { makeRunPromise } from "@/effect/run-service" -import { FileWatcher } from "@/file/watcher" +import { Bus } from "@/bus" +import { $ } from "bun" +import z from "zod" import { Log } from "@/util/log" -import { git } from "@/util/git" import { Instance } from "./instance" -import z from "zod" +import { FileWatcher } from "@/file/watcher" + +const log = Log.create({ service: "vcs" }) export namespace Vcs { - const log = Log.create({ service: "vcs" }) + export const PrInfo = z + .object({ + number: z.number(), + url: z.string(), + title: z.string(), + state: z.enum(["OPEN", "CLOSED", "MERGED"]), + headRefName: z.string(), + baseRefName: z.string(), + isDraft: z.boolean(), + mergeable: z.enum(["MERGEABLE", "CONFLICTING", "UNKNOWN"]), + reviewDecision: z.enum(["APPROVED", "CHANGES_REQUESTED", "REVIEW_REQUIRED"]).nullable().optional(), + checksState: z.enum(["SUCCESS", "FAILURE", "PENDING"]).nullable().optional(), + checksUrl: z.string().optional(), + checksSummary: z + .object({ + total: z.number(), + passed: z.number(), + failed: z.number(), + pending: z.number(), + skipped: z.number(), + }) + .optional(), + unresolvedCommentCount: z.number().optional(), + }) + .meta({ ref: "PrInfo" }) + export type PrInfo = z.infer + + export const GithubCapability = z + .object({ + available: z.boolean(), + authenticated: z.boolean(), + repo: z.object({ owner: z.string(), name: z.string() }).optional(), + host: z.string().optional(), + }) + .meta({ ref: "GithubCapability" }) + export type GithubCapability = z.infer export const Event = { BranchUpdated: BusEvent.define( @@ -19,93 +53,445 @@ export namespace Vcs { branch: z.string().optional(), }), ), + Updated: BusEvent.define( + "vcs.updated", + z.object({ + branch: z.string().optional(), + defaultBranch: z.string().optional(), + branches: z.array(z.string()).optional(), + dirty: z.number().optional(), + pr: PrInfo.optional(), + github: GithubCapability.optional(), + }), + ), } export const Info = z .object({ branch: z.string(), + defaultBranch: z.string().optional(), + branches: z.array(z.string()).optional(), + dirty: z.number().optional(), + pr: PrInfo.optional(), + github: GithubCapability.optional(), }) .meta({ ref: "VcsInfo", }) export type Info = z.infer - export interface Interface { - readonly init: () => Effect.Effect - readonly branch: () => Effect.Effect + async function currentBranch() { + return $`git rev-parse --abbrev-ref HEAD` + .quiet() + .nothrow() + .cwd(Instance.worktree) + .text() + .then((x) => x.trim()) + .catch(() => undefined) } - interface State { - current: string | undefined + export async function detectGithubCapability(): Promise { + const cwd = Instance.worktree + const authCmd = await $`gh auth status` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const authText = + typeof authCmd === "string" ? authCmd : (authCmd.stdout?.toString() ?? "") + (authCmd.stderr?.toString() ?? "") + const available = !authText.includes("not found") && !authText.includes("command not found") + if (!available) { + return { available: false, authenticated: false } + } + const authenticated = authText.includes("Logged in") || (typeof authCmd !== "string" && authCmd.exitCode === 0) + if (!authenticated) { + return { available: true, authenticated: false } + } + const repoCmd = await $`gh repo view --json owner,name,url` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + + let repo: { owner: string; name: string } | undefined + let host: string | undefined + if (repoCmd.exitCode !== 0) { + log.warn("gh repo view failed", { error: repoCmd.stderr.toString() }) + } else { + try { + const parsed = JSON.parse(repoCmd.stdout.toString()) + const ownerLogin = typeof parsed.owner === "string" ? parsed.owner : parsed.owner?.login + if (ownerLogin && parsed.name) { + repo = { owner: ownerLogin, name: parsed.name } + } + if (parsed.url) { + host = new URL(parsed.url).hostname + } + } catch (e) { + log.warn("gh repo view json parse failed", { error: String(e) }) + } + } + + return { available: true, authenticated: true, repo, host } } - export class Service extends ServiceMap.Service()("@opencode/Vcs") {} + export async function fetchDefaultBranch(): Promise { + const cwd = Instance.worktree + const result = await $`git rev-parse --abbrev-ref origin/HEAD` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + const trimmed = result.trim() + if (trimmed && !trimmed.includes("fatal") && trimmed !== "origin/HEAD") { + return trimmed.replace("origin/", "") + } + const ghResult = await $`gh repo view --json defaultBranchRef --jq .defaultBranchRef.name` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + const ghTrimmed = ghResult.trim() + if (ghTrimmed && !ghTrimmed.includes("error")) { + return ghTrimmed + } + return undefined + } - export const layer = Layer.effect( - Service, - Effect.gen(function* () { - const state = yield* InstanceState.make( - Effect.fn("Vcs.state")((ctx) => - Effect.gen(function* () { - if (ctx.project.vcs !== "git") { - return { current: undefined } - } + export async function fetchBranches(): Promise { + const cwd = Instance.worktree + const result = await $`git branch -r --format=${"%(refname:short)"}` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + const branches = result + .split("\n") + .map((b) => b.trim()) + .filter((b) => b && !b.includes("->")) + .map((b) => b.replace(/^origin\//, "")) + .filter((b, i, arr) => arr.indexOf(b) === i) + branches.sort((a, b) => a.localeCompare(b)) + return branches + } - const getCurrentBranch = async () => { - const result = await git(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: ctx.worktree, - }) - if (result.exitCode !== 0) return undefined - const text = result.text().trim() - return text || undefined - } + async function fetchUnresolvedCommentCount( + owner: string, + name: string, + prNumber: number, + ): Promise { + const cwd = Instance.worktree + const query = `query($owner: String!, $name: String!, $prNumber: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100) { nodes { isResolved } } } } }` + const result = await $`gh api graphql -f query=${query} -f owner=${owner} -f name=${name} -F prNumber=${prNumber}` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + try { + const parsed = JSON.parse(result) + const threads = parsed?.data?.repository?.pullRequest?.reviewThreads?.nodes + if (!Array.isArray(threads)) return undefined + return threads.filter((t: { isResolved: boolean }) => !t.isResolved).length + } catch { + return undefined + } + } + + export async function fetchPrForBranch(repo?: { owner: string; name: string }): Promise { + const cwd = Instance.worktree + const result = + await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + if (!result.trim() || result.includes("no pull requests found")) { + return undefined + } + try { + const parsed = JSON.parse(result) + if (!parsed.number) return undefined + + let checksState: "SUCCESS" | "FAILURE" | "PENDING" | null = null + let checksUrl: string | undefined + let checksSummary: { total: number; passed: number; failed: number; pending: number; skipped: number } | undefined + if (parsed.statusCheckRollup && Array.isArray(parsed.statusCheckRollup)) { + const checks = parsed.statusCheckRollup as Array<{ + __typename?: string + conclusion?: string + status?: string + state?: string + detailsUrl?: string + }> + if (checks.length > 0) { + checksUrl = parsed.url ? `${parsed.url}/checks` : checks[0]?.detailsUrl?.replace(/\/runs\/.*/, "") + // StatusContext entries (e.g. CodeRabbit) use `state` instead of `conclusion`/`status` + const isSuccess = (c: (typeof checks)[0]) => + c.conclusion === "SUCCESS" || c.conclusion === "success" || c.state === "SUCCESS" || c.state === "success" + const isFailure = (c: (typeof checks)[0]) => + c.conclusion === "FAILURE" || + c.conclusion === "failure" || + c.state === "FAILURE" || + c.state === "failure" || + c.state === "ERROR" || + c.state === "error" + const isSkipped = (c: (typeof checks)[0]) => + c.conclusion === "SKIPPED" || + c.conclusion === "skipped" || + c.conclusion === "NEUTRAL" || + c.conclusion === "neutral" + const failed = checks.filter(isFailure).length + const passed = checks.filter(isSuccess).length + const skipped = checks.filter(isSkipped).length + const total = checks.length + const pending = total - passed - failed - skipped + checksSummary = { total, passed, failed, pending, skipped } + if (failed > 0) checksState = "FAILURE" + else if (pending === 0) checksState = "SUCCESS" + else checksState = "PENDING" + } + } + + const stateMap: Record = { + OPEN: "OPEN", + CLOSED: "CLOSED", + MERGED: "MERGED", + } + + const mergeableMap: Record = { + MERGEABLE: "MERGEABLE", + CONFLICTING: "CONFLICTING", + UNKNOWN: "UNKNOWN", + } + + const pr: PrInfo = { + number: parsed.number, + url: parsed.url, + title: parsed.title, + state: stateMap[parsed.state] ?? "OPEN", + headRefName: parsed.headRefName, + baseRefName: parsed.baseRefName, + isDraft: parsed.isDraft ?? false, + mergeable: mergeableMap[parsed.mergeable] ?? "UNKNOWN", + reviewDecision: parsed.reviewDecision || null, + checksState, + checksUrl, + checksSummary, + } + + // Fetch unresolved comment count via lightweight GraphQL query + if (repo) { + pr.unresolvedCommentCount = await fetchUnresolvedCommentCount(repo.owner, repo.name, pr.number) + } + + return pr + } catch { + return undefined + } + } + + async function fetchDirtyCount(): Promise { + const cwd = Instance.worktree + const result = await $`git status --porcelain` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + return result.split("\n").filter((l) => l.trim()).length + } + + async function fetchLocalInfo(): Promise> { + const branch = await currentBranch() + return { branch: branch ?? "", dirty: branch ? await fetchDirtyCount() : undefined } + } + + async function fetchRemoteInfo(_branch: string): Promise> { + const [github, defaultBranch, branches] = await Promise.all([ + detectGithubCapability(), + fetchDefaultBranch(), + fetchBranches(), + ]) + + let pr: PrInfo | undefined + if (github.authenticated) { + pr = await fetchPrForBranch(github.repo) + } + + return { + defaultBranch, + branches: branches.length > 0 ? branches : undefined, + pr, + github, + } + } + + async function fetchFullInfo(): Promise { + const local = await fetchLocalInfo() + if (!local.branch) return { branch: "" } + const remote = await fetchRemoteInfo(local.branch) + return { ...local, ...remote } + } + + export async function commit(message: string) { + const cwd = Instance.worktree + const add = await $`git add -A`.quiet().nothrow().cwd(cwd) + if (add.exitCode !== 0) { + throw new Error("git add failed: " + add.stderr.toString()) + } + const result = await $`git commit -m ${message}`.quiet().nothrow().cwd(cwd) + if (result.exitCode !== 0) { + throw new Error("git commit failed: " + result.stderr.toString()) + } + await refresh() + } + + export const POLL_INTERVAL_MS = 120_000 + export const POLL_INTERVAL_NO_PR_MULTIPLIER = 2 + export const LOCAL_DEBOUNCE_MS = 500 + export const REF_DEBOUNCE_MS = 2_000 + export const POLL_JITTER_MS = 10_000 + + function isGitRefChange(file: string): boolean { + return ( + file.endsWith("HEAD") || + file.includes(".git/refs/") || + file.endsWith("MERGE_HEAD") || + file.endsWith("COMMIT_EDITMSG") || + file.includes(".git/packed-refs") + ) + } + + const state = Instance.state( + async () => { + if (Instance.project.vcs !== "git") { + return { info: async () => ({ branch: "" }), refresh: async () => {}, unsubscribe: undefined } + } + let current = await fetchFullInfo() + log.info("initialized", { branch: current.branch, pr: current.pr?.number }) - const value = { - current: yield* Effect.promise(() => getCurrentBranch()), + let localDebounce: ReturnType | undefined + let refDebounce: ReturnType | undefined + let pollTimer: ReturnType | undefined + let hasActivePr = !!current.pr + let pollIntervalMs = hasActivePr ? POLL_INTERVAL_MS : POLL_INTERVAL_MS * POLL_INTERVAL_NO_PR_MULTIPLIER + + const restartPollTimer = () => { + if (pollTimer) clearInterval(pollTimer) + pollTimer = setInterval( + async () => { + try { + await refreshFull() + } catch (e) { + log.error("poll refresh failed", { error: e }) } - log.info("initialized", { branch: value.current }) - - yield* Effect.acquireRelease( - Effect.sync(() => - Bus.subscribe( - FileWatcher.Event.Updated, - Instance.bind(async (evt) => { - if (!evt.properties.file.endsWith("HEAD")) return - const next = await getCurrentBranch() - if (next !== value.current) { - log.info("branch changed", { from: value.current, to: next }) - value.current = next - Bus.publish(Event.BranchUpdated, { branch: next }) - } - }), - ), - ), - (unsubscribe) => Effect.sync(unsubscribe), - ) - - return value - }), - ), - ) - - return Service.of({ - init: Effect.fn("Vcs.init")(function* () { - yield* InstanceState.get(state) - }), - branch: Effect.fn("Vcs.branch")(function* () { - return yield* InstanceState.use(state, (x) => x.current) - }), + }, + pollIntervalMs + Math.random() * POLL_JITTER_MS, + ) + } + + const publish = () => { + Bus.publish(Event.Updated, { + branch: current.branch, + defaultBranch: current.defaultBranch, + branches: current.branches, + dirty: current.dirty, + pr: current.pr, + github: current.github, + }) + } + + const refreshLocal = async () => { + const local = await fetchLocalInfo() + const branchChanged = local.branch !== current.branch + current = { ...current, ...local } + if (branchChanged) { + Bus.publish(Event.BranchUpdated, { branch: local.branch }) + // Branch changed — also refresh remote info + if (local.branch) { + const remote = await fetchRemoteInfo(local.branch) + current = { ...current, ...remote } + } + } + publish() + } + + const refreshFull = async () => { + const next = await fetchFullInfo() + const branchChanged = next.branch !== current.branch + const prChanged = next.pr?.number !== current.pr?.number + current = next + if (branchChanged) { + Bus.publish(Event.BranchUpdated, { branch: next.branch }) + } + + const hasPr = !!next.pr + if (hasPr !== hasActivePr || prChanged) { + hasActivePr = hasPr + pollIntervalMs = hasActivePr ? POLL_INTERVAL_MS : POLL_INTERVAL_MS * POLL_INTERVAL_NO_PR_MULTIPLIER + restartPollTimer() + } + + publish() + } + + const unsubscribeWatcher = Bus.subscribe(FileWatcher.Event.Updated, async (evt) => { + const file = evt.properties.file + if (isGitRefChange(file)) { + // Git ref change (branch switch, commit, etc) — debounce a full refresh + if (refDebounce) clearTimeout(refDebounce) + refDebounce = setTimeout(refreshFull, REF_DEBOUNCE_MS) + return + } + // Regular file change — only refresh local (dirty count, branch) + if (localDebounce) clearTimeout(localDebounce) + localDebounce = setTimeout(refreshLocal, LOCAL_DEBOUNCE_MS) }) - }), + + restartPollTimer() + + let onFocus: (() => void) | undefined + if (typeof window !== "undefined") { + onFocus = () => refreshFull() + window.addEventListener("focus", onFocus) + } + + return { + info: async () => current, + refresh: refreshFull, + unsubscribe: () => { + unsubscribeWatcher() + if (localDebounce) clearTimeout(localDebounce) + if (refDebounce) clearTimeout(refDebounce) + if (pollTimer) clearInterval(pollTimer) + if (onFocus) window.removeEventListener("focus", onFocus) + }, + } + }, + async (state) => { + state.unsubscribe?.() + }, ) - const runPromise = makeRunPromise(Service, layer) + export async function init() { + return state() + } + + export async function branch() { + return await state().then((s) => s.info().then((i) => i.branch)) + } - export function init() { - return runPromise((svc) => svc.init()) + export async function info(): Promise { + return await state().then((s) => s.info()) } - export function branch() { - return runPromise((svc) => svc.branch()) + export async function refresh() { + const s = await state() + await s.refresh() } } diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts new file mode 100644 index 000000000000..15b31e569157 --- /dev/null +++ b/packages/opencode/src/server/routes/vcs.ts @@ -0,0 +1,242 @@ +import { Hono } from "hono" +import { describeRoute, validator, resolver } from "hono-openapi" +import z from "zod" +import { Vcs } from "../../project/vcs" +import { PR } from "../../project/pr" +import { PrComments } from "../../project/pr-comments" +import { errors } from "../error" +import { lazy } from "../../util/lazy" + +const CommitInput = z + .object({ + message: z.string(), + }) + .meta({ ref: "VcsCommitInput" }) + +export const VcsRoutes = lazy(() => + new Hono() + .get( + "/", + describeRoute({ + summary: "Get VCS info", + description: + "Retrieve version control system (VCS) information for the current project, including branch, PR, and GitHub capability.", + operationId: "vcs.get", + responses: { + 200: { + description: "VCS info", + content: { + "application/json": { + schema: resolver(Vcs.Info), + }, + }, + }, + }, + }), + async (c) => { + const data = await Vcs.info() + return c.json(data) + }, + ) + .get( + "/branches", + describeRoute({ + summary: "List remote branches", + description: "List all remote branches for the current repository.", + operationId: "vcs.branches", + responses: { + 200: { + description: "Branch list", + content: { + "application/json": { + schema: resolver(z.array(z.string())), + }, + }, + }, + }, + }), + async (c) => { + const branches = await Vcs.fetchBranches() + return c.json(branches) + }, + ) + .get( + "/pr", + describeRoute({ + summary: "Get current PR", + description: "Retrieve the pull request associated with the current branch, if any.", + operationId: "vcs.pr.get", + responses: { + 200: { + description: "PR info or null", + content: { + "application/json": { + schema: resolver(Vcs.PrInfo.nullable()), + }, + }, + }, + }, + }), + async (c) => { + const pr = await PR.get() + return c.json(pr ?? null) + }, + ) + .post( + "/pr", + describeRoute({ + summary: "Create PR", + description: "Create a pull request for the current branch. Idempotent — returns existing PR if one exists.", + operationId: "vcs.pr.create", + responses: { + 200: { + description: "Created or existing PR", + content: { + "application/json": { + schema: resolver(Vcs.PrInfo), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", PR.CreateInput), + async (c) => { + try { + const input = c.req.valid("json") + const pr = await PR.create(input) + return c.json(pr) + } catch (e) { + if (e instanceof PR.PrError) { + return c.json({ code: e.code, message: e.message }, { status: 400 }) + } + throw e + } + }, + ) + .get( + "/pr/comments", + describeRoute({ + summary: "Get unresolved review comments", + description: + "Fetch unresolved review threads for the current PR, with a pre-formatted prompt block for the agent.", + operationId: "vcs.pr.comments", + responses: { + 200: { + description: "Unresolved review threads and prompt block", + content: { + "application/json": { + schema: resolver(PrComments.CommentsResponse), + }, + }, + }, + ...errors(400), + }, + }), + async (c) => { + try { + const data = await PrComments.fetch() + return c.json(data) + } catch (e) { + if (e instanceof PR.PrError) { + return c.json({ code: e.code, message: e.message }, { status: 400 }) + } + const message = e instanceof Error ? e.message : String(e) + return c.json({ code: "COMMENTS_FETCH_FAILED", message }, { status: 400 }) + } + }, + ) + .post( + "/pr/merge", + describeRoute({ + summary: "Merge PR", + description: "Merge the pull request for the current branch.", + operationId: "vcs.pr.merge", + responses: { + 200: { + description: "Merged PR", + content: { + "application/json": { + schema: resolver(Vcs.PrInfo), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", PR.MergeInput), + async (c) => { + try { + const input = c.req.valid("json") + const pr = await PR.merge(input) + return c.json(pr) + } catch (e) { + if (e instanceof PR.PrError) { + return c.json({ code: e.code, message: e.message }, { status: 400 }) + } + throw e + } + }, + ) + .post( + "/pr/delete-branch", + describeRoute({ + summary: "Delete branch", + description: "Delete a remote (and local) branch after a PR has been merged.", + operationId: "vcs.pr.deleteBranch", + responses: { + 200: { + description: "Branch deleted", + content: { + "application/json": { + schema: resolver(z.object({ ok: z.boolean() })), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", PR.DeleteBranchInput), + async (c) => { + try { + const input = c.req.valid("json") + await PR.deleteBranch(input) + return c.json({ ok: true }) + } catch (e) { + if (e instanceof PR.PrError) { + return c.json({ code: e.code, message: e.message }, { status: 400 }) + } + throw e + } + }, + ) + .post( + "/commit", + describeRoute({ + summary: "Commit all changes", + description: "Stage all changes and commit with the given message.", + operationId: "vcs.commit", + responses: { + 200: { + description: "Commit succeeded", + content: { + "application/json": { + schema: resolver(z.object({ ok: z.boolean() })), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", CommitInput), + async (c) => { + try { + const input = c.req.valid("json") + await Vcs.commit(input.message) + return c.json({ ok: true }) + } catch (e: any) { + return c.json({ code: "COMMIT_FAILED", message: e?.message ?? "Commit failed" }, { status: 400 }) + } + }, + ), +) diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index 7ead4df8a3cb..f91d571bee2f 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -249,6 +249,7 @@ export namespace Server { .route("/provider", ProviderRoutes()) .route("/", FileRoutes()) .route("/", EventRoutes()) + .route("/vcs", VcsRoutes()) .route("/mcp", McpRoutes()) .route("/tui", TuiRoutes()) .post( @@ -312,30 +313,6 @@ export namespace Server { }) }, ) - .get( - "/vcs", - describeRoute({ - summary: "Get VCS info", - description: "Retrieve version control system (VCS) information for the current project, such as git branch.", - operationId: "vcs.get", - responses: { - 200: { - description: "VCS info", - content: { - "application/json": { - schema: resolver(Vcs.Info), - }, - }, - }, - }, - }), - async (c) => { - const branch = await Vcs.branch() - return c.json({ - branch, - }) - }, - ) .get( "/command", describeRoute({ From 5ac07c2bb5d5c3fc0b55d85f3944f796a389e741 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 01:07:25 -0500 Subject: [PATCH 02/46] feat(sdk): regenerate SDK types and client for PR/VCS endpoints --- packages/opencode/src/server/server.ts | 2 +- packages/sdk/js/src/v2/gen/sdk.gen.ts | 335 ++++++++++++++++++++--- packages/sdk/js/src/v2/gen/types.gen.ts | 337 +++++++++++++++++++++--- 3 files changed, 605 insertions(+), 69 deletions(-) diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index f91d571bee2f..ba0c0302a0d5 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -11,7 +11,6 @@ import { LSP } from "../lsp" import { Format } from "../format" import { TuiRoutes } from "./routes/tui" import { Instance } from "../project/instance" -import { Vcs } from "../project/vcs" import { Agent } from "../agent/agent" import { Skill } from "../skill" import { Auth } from "../auth" @@ -31,6 +30,7 @@ import { ConfigRoutes } from "./routes/config" import { ExperimentalRoutes } from "./routes/experimental" import { ProviderRoutes } from "./routes/provider" import { EventRoutes } from "./routes/event" +import { VcsRoutes } from "./routes/vcs" import { InstanceBootstrap } from "../project/bootstrap" import { NotFoundError } from "../storage/db" import type { ContentfulStatusCode } from "hono/utils/http-status" diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index b6821322e2f3..cd0e0d59a9fa 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -76,6 +76,9 @@ import type { PermissionRespondErrors, PermissionRespondResponses, PermissionRuleset, + PrCreateInput, + PrDeleteBranchInput, + PrMergeInput, ProjectCurrentResponses, ProjectInitGitResponses, ProjectListResponses, @@ -172,7 +175,20 @@ import type { TuiSelectSessionResponses, TuiShowToastResponses, TuiSubmitPromptResponses, + VcsBranchesResponses, + VcsCommitErrors, + VcsCommitInput, + VcsCommitResponses, VcsGetResponses, + VcsPrCommentsErrors, + VcsPrCommentsResponses, + VcsPrCreateErrors, + VcsPrCreateResponses, + VcsPrDeleteBranchErrors, + VcsPrDeleteBranchResponses, + VcsPrGetResponses, + VcsPrMergeErrors, + VcsPrMergeResponses, WorktreeCreateErrors, WorktreeCreateInput, WorktreeCreateResponses, @@ -2877,6 +2893,283 @@ export class Event extends HeyApiClient { } } +export class Pr extends HeyApiClient { + /** + * Get current PR + * + * Retrieve the pull request associated with the current branch, if any. + */ + public get( + parameters?: { + directory?: string + workspace?: string + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + ], + }, + ], + ) + return (options?.client ?? this.client).get({ + url: "/vcs/pr", + ...options, + ...params, + }) + } + + /** + * Create PR + * + * Create a pull request for the current branch. Idempotent — returns existing PR if one exists. + */ + public create( + parameters?: { + directory?: string + workspace?: string + prCreateInput?: PrCreateInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "prCreateInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/pr", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } + + /** + * Get unresolved review comments + * + * Fetch unresolved review threads for the current PR, with a pre-formatted prompt block for the agent. + */ + public comments( + parameters?: { + directory?: string + workspace?: string + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + ], + }, + ], + ) + return (options?.client ?? this.client).get({ + url: "/vcs/pr/comments", + ...options, + ...params, + }) + } + + /** + * Merge PR + * + * Merge the pull request for the current branch. + */ + public merge( + parameters?: { + directory?: string + workspace?: string + prMergeInput?: PrMergeInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "prMergeInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/pr/merge", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } + + /** + * Delete branch + * + * Delete a remote (and local) branch after a PR has been merged. + */ + public deleteBranch( + parameters?: { + directory?: string + workspace?: string + prDeleteBranchInput?: PrDeleteBranchInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "prDeleteBranchInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/pr/delete-branch", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } +} + +export class Vcs extends HeyApiClient { + /** + * Get VCS info + * + * Retrieve version control system (VCS) information for the current project, including branch, PR, and GitHub capability. + */ + public get( + parameters?: { + directory?: string + workspace?: string + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + ], + }, + ], + ) + return (options?.client ?? this.client).get({ + url: "/vcs", + ...options, + ...params, + }) + } + + /** + * List remote branches + * + * List all remote branches for the current repository. + */ + public branches( + parameters?: { + directory?: string + workspace?: string + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + ], + }, + ], + ) + return (options?.client ?? this.client).get({ + url: "/vcs/branches", + ...options, + ...params, + }) + } + + /** + * Commit all changes + * + * Stage all changes and commit with the given message. + */ + public commit( + parameters?: { + directory?: string + workspace?: string + vcsCommitInput?: VcsCommitInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "vcsCommitInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/commit", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } + + private _pr?: Pr + get pr(): Pr { + return (this._pr ??= new Pr({ client: this.client })) + } +} + export class Auth2 extends HeyApiClient { /** * Remove MCP OAuth @@ -3663,38 +3956,6 @@ export class Path extends HeyApiClient { } } -export class Vcs extends HeyApiClient { - /** - * Get VCS info - * - * Retrieve version control system (VCS) information for the current project, such as git branch. - */ - public get( - parameters?: { - directory?: string - workspace?: string - }, - options?: Options, - ) { - const params = buildClientParams( - [parameters], - [ - { - args: [ - { in: "query", key: "directory" }, - { in: "query", key: "workspace" }, - ], - }, - ], - ) - return (options?.client ?? this.client).get({ - url: "/vcs", - ...options, - ...params, - }) - } -} - export class Command extends HeyApiClient { /** * List commands @@ -3986,6 +4247,11 @@ export class OpencodeClient extends HeyApiClient { return (this._event ??= new Event({ client: this.client })) } + private _vcs?: Vcs + get vcs(): Vcs { + return (this._vcs ??= new Vcs({ client: this.client })) + } + private _mcp?: Mcp get mcp(): Mcp { return (this._mcp ??= new Mcp({ client: this.client })) @@ -4006,11 +4272,6 @@ export class OpencodeClient extends HeyApiClient { return (this._path ??= new Path({ client: this.client })) } - private _vcs?: Vcs - get vcs(): Vcs { - return (this._vcs ??= new Vcs({ client: this.client })) - } - private _command?: Command get command(): Command { return (this._command ??= new Command({ client: this.client })) diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index f7aab687e663..019c13cd808d 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -882,13 +882,6 @@ export type EventSessionError = { } } -export type EventVcsBranchUpdated = { - type: "vcs.branch.updated" - properties: { - branch?: string - } -} - export type EventWorkspaceReady = { type: "workspace.ready" properties: { @@ -957,6 +950,57 @@ export type EventWorktreeFailed = { } } +export type EventVcsBranchUpdated = { + type: "vcs.branch.updated" + properties: { + branch?: string + } +} + +export type PrInfo = { + number: number + url: string + title: string + state: "OPEN" | "CLOSED" | "MERGED" + headRefName: string + baseRefName: string + isDraft: boolean + mergeable: "MERGEABLE" | "CONFLICTING" | "UNKNOWN" + reviewDecision?: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null + checksState?: "SUCCESS" | "FAILURE" | "PENDING" | null + checksUrl?: string + checksSummary?: { + total: number + passed: number + failed: number + pending: number + skipped: number + } + unresolvedCommentCount?: number +} + +export type GithubCapability = { + available: boolean + authenticated: boolean + repo?: { + owner: string + name: string + } + host?: string +} + +export type EventVcsUpdated = { + type: "vcs.updated" + properties: { + branch?: string + defaultBranch?: string + branches?: Array + dirty?: number + pr?: PrInfo + github?: GithubCapability + } +} + export type Event = | EventInstallationUpdated | EventInstallationUpdateAvailable @@ -994,7 +1038,6 @@ export type Event = | EventSessionDeleted | EventSessionDiff | EventSessionError - | EventVcsBranchUpdated | EventWorkspaceReady | EventWorkspaceFailed | EventPtyCreated @@ -1003,6 +1046,8 @@ export type Event = | EventPtyDeleted | EventWorktreeReady | EventWorktreeFailed + | EventVcsBranchUpdated + | EventVcsUpdated export type GlobalEvent = { directory: string @@ -1851,6 +1896,58 @@ export type File = { status: "added" | "deleted" | "modified" } +export type VcsInfo = { + branch: string + defaultBranch?: string + branches?: Array + dirty?: number + pr?: PrInfo + github?: GithubCapability +} + +export type PrCreateInput = { + title: string + body: string + base?: string + draft?: boolean +} + +export type ReviewComment = { + id: number + author: string + body: string + path: string + line: number | null + diffHunk?: string +} + +export type ReviewThread = { + id: string + isResolved: boolean + path: string + line: number | null + comments: Array +} + +export type PrCommentsResponse = { + threads: Array + promptBlock: string + unresolvedCount: number +} + +export type PrMergeInput = { + strategy?: "merge" | "squash" | "rebase" + deleteBranch?: boolean +} + +export type PrDeleteBranchInput = { + branch: string +} + +export type VcsCommitInput = { + message: string +} + export type McpStatusConnected = { status: "connected" } @@ -1888,10 +1985,6 @@ export type Path = { directory: string } -export type VcsInfo = { - branch: string -} - export type Command = { name: string description?: string @@ -4248,6 +4341,207 @@ export type EventSubscribeResponses = { export type EventSubscribeResponse = EventSubscribeResponses[keyof EventSubscribeResponses] +export type VcsGetData = { + body?: never + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs" +} + +export type VcsGetResponses = { + /** + * VCS info + */ + 200: VcsInfo +} + +export type VcsGetResponse = VcsGetResponses[keyof VcsGetResponses] + +export type VcsBranchesData = { + body?: never + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/branches" +} + +export type VcsBranchesResponses = { + /** + * Branch list + */ + 200: Array +} + +export type VcsBranchesResponse = VcsBranchesResponses[keyof VcsBranchesResponses] + +export type VcsPrGetData = { + body?: never + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr" +} + +export type VcsPrGetResponses = { + /** + * PR info or null + */ + 200: PrInfo | null +} + +export type VcsPrGetResponse = VcsPrGetResponses[keyof VcsPrGetResponses] + +export type VcsPrCreateData = { + body?: PrCreateInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr" +} + +export type VcsPrCreateErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrCreateError = VcsPrCreateErrors[keyof VcsPrCreateErrors] + +export type VcsPrCreateResponses = { + /** + * Created or existing PR + */ + 200: PrInfo +} + +export type VcsPrCreateResponse = VcsPrCreateResponses[keyof VcsPrCreateResponses] + +export type VcsPrCommentsData = { + body?: never + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr/comments" +} + +export type VcsPrCommentsErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrCommentsError = VcsPrCommentsErrors[keyof VcsPrCommentsErrors] + +export type VcsPrCommentsResponses = { + /** + * Unresolved review threads and prompt block + */ + 200: PrCommentsResponse +} + +export type VcsPrCommentsResponse = VcsPrCommentsResponses[keyof VcsPrCommentsResponses] + +export type VcsPrMergeData = { + body?: PrMergeInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr/merge" +} + +export type VcsPrMergeErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrMergeError = VcsPrMergeErrors[keyof VcsPrMergeErrors] + +export type VcsPrMergeResponses = { + /** + * Merged PR + */ + 200: PrInfo +} + +export type VcsPrMergeResponse = VcsPrMergeResponses[keyof VcsPrMergeResponses] + +export type VcsPrDeleteBranchData = { + body?: PrDeleteBranchInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr/delete-branch" +} + +export type VcsPrDeleteBranchErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrDeleteBranchError = VcsPrDeleteBranchErrors[keyof VcsPrDeleteBranchErrors] + +export type VcsPrDeleteBranchResponses = { + /** + * Branch deleted + */ + 200: { + ok: boolean + } +} + +export type VcsPrDeleteBranchResponse = VcsPrDeleteBranchResponses[keyof VcsPrDeleteBranchResponses] + +export type VcsCommitData = { + body?: VcsCommitInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/commit" +} + +export type VcsCommitErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsCommitError = VcsCommitErrors[keyof VcsCommitErrors] + +export type VcsCommitResponses = { + /** + * Commit succeeded + */ + 200: { + ok: boolean + } +} + +export type VcsCommitResponse = VcsCommitResponses[keyof VcsCommitResponses] + export type McpStatusData = { body?: never path?: never @@ -4833,25 +5127,6 @@ export type PathGetResponses = { export type PathGetResponse = PathGetResponses[keyof PathGetResponses] -export type VcsGetData = { - body?: never - path?: never - query?: { - directory?: string - workspace?: string - } - url: "/vcs" -} - -export type VcsGetResponses = { - /** - * VCS info - */ - 200: VcsInfo -} - -export type VcsGetResponse = VcsGetResponses[keyof VcsGetResponses] - export type CommandListData = { body?: never path?: never From ccabed57798d1e9de36f6ce299abff678d77ab64 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 01:07:34 -0500 Subject: [PATCH 03/46] feat(app): add PR UI - create/merge/address-comments dialogs, PR button, and session commands - Add PR button component with status display in sidebar - Add dialogs for creating PRs, merging, and addressing review comments - Wire up VCS updated event in global sync for PR state tracking - Add PR-related session commands and i18n strings - Add git-pull-request icon and dialog styling updates --- .../components/dialog-address-comments.tsx | 291 +++++++++++++++ .../app/src/components/dialog-create-pr.tsx | 267 ++++++++++++++ .../app/src/components/dialog-merge-pr.tsx | 159 +++++++++ packages/app/src/components/pr-button.tsx | 335 ++++++++++++++++++ .../src/components/session/session-header.tsx | 2 + .../src/context/global-sync/child-store.ts | 2 +- .../src/context/global-sync/event-reducer.ts | 20 +- packages/app/src/i18n/en.ts | 72 ++++ .../src/pages/layout/sidebar-workspace.tsx | 22 +- .../pages/session/use-session-commands.tsx | 57 +++ packages/app/src/utils/pr-style.ts | 25 ++ packages/ui/src/components/dialog.css | 20 +- packages/ui/src/components/icon.tsx | 3 + 13 files changed, 1261 insertions(+), 14 deletions(-) create mode 100644 packages/app/src/components/dialog-address-comments.tsx create mode 100644 packages/app/src/components/dialog-create-pr.tsx create mode 100644 packages/app/src/components/dialog-merge-pr.tsx create mode 100644 packages/app/src/components/pr-button.tsx create mode 100644 packages/app/src/utils/pr-style.ts diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx new file mode 100644 index 000000000000..39ab370d9566 --- /dev/null +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -0,0 +1,291 @@ +import { Button } from "@opencode-ai/ui/button" +import { Checkbox } from "@opencode-ai/ui/checkbox" +import { useDialog } from "@opencode-ai/ui/context/dialog" +import { Dialog } from "@opencode-ai/ui/dialog" +import { Icon } from "@opencode-ai/ui/icon" +import { Spinner } from "@opencode-ai/ui/spinner" +import { TextField } from "@opencode-ai/ui/text-field" +import { createMemo, For, onMount, Show } from "solid-js" +import { createStore } from "solid-js/store" +import { useLanguage } from "@/context/language" +import { usePrompt } from "@/context/prompt" +import { useSDK } from "@/context/sdk" +import { useSync } from "@/context/sync" +import type { ReviewThread } from "@opencode-ai/sdk/v2" + +export function AddressCommentsDialog() { + const dialog = useDialog() + const sdk = useSDK() + const sync = useSync() + const prompt = usePrompt() + const language = useLanguage() + + const vcs = createMemo(() => sync.data.vcs) + const pr = createMemo(() => vcs()?.pr) + const github = createMemo(() => vcs()?.github) + + const [store, setStore] = createStore({ + loading: true, + error: undefined as string | undefined, + threads: [] as ReviewThread[], + selected: {} as Record, + instructions: "", + }) + + onMount(async () => { + try { + const result = await sdk.client.vcs.pr.comments({ directory: sdk.directory }) + const data = result.data + if (!data) { + setStore("error", "No response from server") + setStore("loading", false) + return + } + const selected: Record = {} + for (const thread of data.threads) { + selected[thread.id] = true + } + setStore({ + threads: data.threads, + selected, + loading: false, + }) + } catch (e: unknown) { + if (import.meta.env.DEV) { + console.error("Fetch comments error:", e) + } + const err = (e as { error?: { code?: string; message?: string } })?.error ?? e + const errObj = err as { code?: string; message?: string } + const msg = errObj?.message ?? errObj?.code ?? "Failed to fetch review comments" + setStore("error", String(msg)) + setStore("loading", false) + } + }) + + const selectedCount = createMemo(() => Object.values(store.selected).filter(Boolean).length) + const allSelected = createMemo(() => store.threads.length > 0 && selectedCount() === store.threads.length) + + const toggleAll = () => { + const next = !allSelected() + const updated: Record = {} + for (const thread of store.threads) { + updated[thread.id] = next + } + setStore("selected", updated) + } + + const toggleThread = (id: string) => { + setStore("selected", id, !store.selected[id]) + } + + const handleSubmit = () => { + const prNumber = pr()?.number + const repo = github()?.repo + const owner = repo?.owner ?? "" + const repoName = repo?.name ?? "" + + const selectedThreads = store.threads.filter((t) => store.selected[t.id]) + + let text = `Address the following unresolved review comments on PR #${prNumber}.\n\n` + text += `## Comments to Address\n\n` + + for (const thread of selectedThreads) { + if (thread.comments.length === 0) continue + text += `### File: ${thread.path}${thread.line ? ` (line ${thread.line})` : ""}\n` + for (const comment of thread.comments) { + text += `**@${comment.author}** (comment ID: ${comment.id}): ${comment.body}\n` + } + text += "\n" + } + + text += `## Instructions\n\n` + text += `1. Read each comment above and decide whether to fix it or intentionally skip it\n` + text += `2. For fixes: make the code change\n` + text += `3. For skips: explain the design rationale in your reply\n` + text += `4. After addressing all comments, reply to each one on GitHub using the comment ID shown above:\n` + text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="" -F in_reply_to=\`\n` + text += `5. Do NOT merge, rebase, or force-push\n` + + if (store.instructions.trim()) { + text += `\n${store.instructions.trim()}\n` + } + + dialog.close() + requestAnimationFrame(() => { + prompt.set([ + { + type: "text", + content: text, + start: 0, + end: text.length, + }, + ]) + }) + } + + return ( + +
+ + +
+ } + > + +
+ + {language.t("pr.comments.error.title")} + {store.error} +
+ + + 0} + fallback={ +
+ + {language.t("pr.comments.none")} +
+ } + > + {/* Header with count and select all toggle */} +
+ + {store.threads.length === 1 + ? language.t("pr.comments.count.one") + : language.t("pr.comments.count", { count: String(store.threads.length) })} + + +
+ + {/* Comment thread list */} +
+ + {(thread) => { + const firstComment = () => thread.comments[0] + const replies = () => thread.comments.slice(1) + const isChecked = () => !!store.selected[thread.id] + + return ( +
toggleThread(thread.id)} + > +
+ +   + +
+
+ {/* File path + line */} +
+ + {thread.path} + + + : + {thread.line} + + +
+ + {/* First comment (the review comment) */} + + {(comment) => ( +
+
+
+ @{comment().author} + {comment().author.includes("bot") && ( + + Bot + + )} +
+ + e.stopPropagation()} + class="text-icon-weak hover:text-text-strong transition-colors shrink-0 cursor-pointer" + title="View on GitHub" + > + + + +
+

+ {comment().body} +

+
+ )} + + + {/* Reply count */} + 0}> +
+ ↳ + + {replies().length} {replies().length === 1 ? "reply" : "replies"} + +
+ +
+
+ ) + }} + +
+ + {/* Additional instructions */} +
+ + setStore("instructions", e.currentTarget.value)} + placeholder={language.t("pr.comments.instructions.placeholder")} + /> +
+ + + + + {/* Footer */} +
+
+ 0 && !store.error}> + + {language.t("pr.comments.selected", { + selected: String(selectedCount()), + total: String(store.threads.length), + })} + + +
+
+ + 0 && !store.error}> + + +
+
+ + + ) +} diff --git a/packages/app/src/components/dialog-create-pr.tsx b/packages/app/src/components/dialog-create-pr.tsx new file mode 100644 index 000000000000..3923348edba9 --- /dev/null +++ b/packages/app/src/components/dialog-create-pr.tsx @@ -0,0 +1,267 @@ +import { Button } from "@opencode-ai/ui/button" +import { useDialog } from "@opencode-ai/ui/context/dialog" +import { Dialog } from "@opencode-ai/ui/dialog" +import { Icon } from "@opencode-ai/ui/icon" +import { Select } from "@opencode-ai/ui/select" +import { Switch } from "@opencode-ai/ui/switch" +import { TextField } from "@opencode-ai/ui/text-field" +import { showToast } from "@opencode-ai/ui/toast" +import { createEffect, createMemo, createSignal, Show } from "solid-js" +import { createStore } from "solid-js/store" +import { useLanguage } from "@/context/language" +import { useSDK } from "@/context/sdk" +import { useSync } from "@/context/sync" + +export function CreatePrDialog() { + const dialog = useDialog() + const sdk = useSDK() + const sync = useSync() + const language = useLanguage() + + const vcs = createMemo(() => sync.data.vcs) + const branch = createMemo(() => vcs()?.branch ?? "") + const defaultBranch = createMemo(() => vcs()?.defaultBranch ?? "main") + const dirty = createMemo(() => vcs()?.dirty) + + const isPushed = createMemo(() => { + const current = branch() + const branches = vcs()?.branches ?? [] + return branches.includes(current) + }) + + const branchTitle = createMemo(() => { + const b = branch() + if (!b) return "" + return b.replace(/[-_]/g, " ").replace(/\b\w/g, (c) => c.toUpperCase()) + }) + + const baseOptions = createMemo(() => { + const remote = vcs()?.branches + if (remote && remote.length > 0) return remote + return [defaultBranch()] + }) + + const [store, setStore] = createStore({ + title: branchTitle(), + body: "", + base: defaultBranch(), + draft: false, + submitting: false, + committing: false, + commitMessage: branchTitle(), + showCommitInput: false, + error: undefined as string | undefined, + }) + + const [titleEdited, setTitleEdited] = createSignal(false) + const [baseEdited, setBaseEdited] = createSignal(false) + + createEffect(() => { + const t = branchTitle() + if (t && !titleEdited()) { + setStore("title", t) + setStore("commitMessage", t) + } + }) + + createEffect(() => { + const b = defaultBranch() + if (b && !baseEdited()) { + setStore("base", b) + } + }) + + const handleCommit = async () => { + if (!store.commitMessage.trim() || store.committing) return + setStore("committing", true) + setStore("error", undefined) + + try { + await sdk.client.vcs.commit({ + directory: sdk.directory, + vcsCommitInput: { + message: store.commitMessage.trim(), + }, + }) + + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.commit.success"), + }) + setStore("showCommitInput", false) + } catch (e: unknown) { + const err = (e as { error?: { message?: string } })?.error ?? e + setStore("error", (err as { message?: string })?.message ?? "Commit failed") + } finally { + setStore("committing", false) + } + } + + const handleSubmit = async () => { + if (!store.title.trim() || store.submitting) return + setStore("submitting", true) + setStore("error", undefined) + + try { + await sdk.client.vcs.pr.create({ + directory: sdk.directory, + prCreateInput: { + title: store.title.trim(), + body: store.body.trim() || store.title.trim(), + base: store.base || undefined, + draft: store.draft, + }, + }) + + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.created"), + }) + dialog.close() + } catch (e: unknown) { + if (import.meta.env.DEV) { + console.error("PR creation error:", e) + } + + let errorMsg: string + const err = + (e as { error?: { code?: string; message?: string } })?.error ?? (e as { code?: string; message?: string }) + if (err?.code) { + // Dynamic lookup: pr.error.gh_not_authenticated, pr.error.no_repo, etc. + const errorKey = `pr.error.${err.code.toLowerCase()}` + const translated = language.t(errorKey as Parameters[0]) + errorMsg = translated !== errorKey ? translated : err.message || "Failed to create PR" + } else { + errorMsg = err?.message ?? (typeof err === "string" ? err : "Failed to create PR") + } + + setStore("error", errorMsg) + } finally { + setStore("submitting", false) + } + } + + return ( + +
+ {/* Branch flow indicator */} +
+ + {branch()} + → + {store.base || defaultBranch()} +
+ + 0}> +
+
+ + + {language.t("pr.commit.warning", { count: String(dirty()) })} + + + + +
+ +
+ setStore("commitMessage", e.currentTarget.value)} + placeholder={language.t("pr.commit.placeholder")} + class="flex-1" + autofocus + onKeyDown={(e: KeyboardEvent) => { + if (e.key === "Enter" && !e.shiftKey) { + e.preventDefault() + handleCommit() + } + }} + /> + +
+ +
+ +
+ + { + setTitleEdited(true) + setStore("title", e.currentTarget.value) + }} + autofocus + /> +
+
+ + setStore("body", e.currentTarget.value)} + placeholder={language.t("pr.create.field.body.placeholder")} + style={{ "min-height": "80px" }} + /> +
+
+ + setStore("strategy", (v as MergeStrategy) ?? "squash")} + variant="secondary" + triggerVariant="settings" + triggerStyle={{ + width: "100%", + "justify-content": "space-between", + background: "var(--input-base)", + border: "1px solid var(--border-weak-base)", + "border-radius": "var(--radius-md)", + }} + > + {(item) => {item ? strategyLabel(item) : ""}} + +
+ + {/* Delete branch checkbox */} + setStore("deleteBranch", checked)}> + {language.t("pr.merge.delete_branch")} + + + {/* Error display */} + +
+ + {store.error} +
+ + + {/* Actions */} +
+ + +
+
+ + ) +} diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx new file mode 100644 index 000000000000..3e37063167cd --- /dev/null +++ b/packages/app/src/components/pr-button.tsx @@ -0,0 +1,335 @@ +import { Button } from "@opencode-ai/ui/button" +import { DropdownMenu } from "@opencode-ai/ui/dropdown-menu" +import { Icon } from "@opencode-ai/ui/icon" +import { IconButton } from "@opencode-ai/ui/icon-button" +import { Tooltip } from "@opencode-ai/ui/tooltip" +import { showToast } from "@opencode-ai/ui/toast" +import { useDialog } from "@opencode-ai/ui/context/dialog" +import { createMemo, Show } from "solid-js" +import { createStore } from "solid-js/store" +import { useLanguage } from "@/context/language" +import { usePlatform } from "@/context/platform" +import { useSDK } from "@/context/sdk" +import { useServer } from "@/context/server" +import { useSync } from "@/context/sync" +import { getPrButtonContainerStyle, getPrButtonDividerStyle } from "@/utils/pr-style" +import { CreatePrDialog } from "./dialog-create-pr" +import { MergePrDialog } from "./dialog-merge-pr" +import { AddressCommentsDialog } from "./dialog-address-comments" + +export function PrButton() { + const sync = useSync() + const sdk = useSDK() + const server = useServer() + const platform = usePlatform() + const language = useLanguage() + const dialog = useDialog() + + const [menu, setMenu] = createStore({ open: false }) + + const vcs = createMemo(() => sync.data.vcs) + const pr = createMemo(() => vcs()?.pr) + const github = createMemo(() => vcs()?.github) + const defaultBranch = createMemo(() => vcs()?.defaultBranch) + const branch = createMemo(() => vcs()?.branch) + + const isOnDefaultBranch = createMemo(() => { + const b = branch() + const db = defaultBranch() + if (!b || !db) return false + return b === db + }) + + const hidden = createMemo(() => { + if (!vcs()) return true + if (!github()?.available) return true + if (isOnDefaultBranch()) return true + return false + }) + + const canMutate = createMemo(() => { + return server.isLocal() && github()?.authenticated + }) + + const tooltipText = createMemo(() => { + if (!github()?.authenticated) return language.t("pr.tooltip.not_authenticated") + if (!server.isLocal()) return language.t("pr.tooltip.not_local") + return undefined + }) + + const isOpen = createMemo(() => { + const p = pr() + return p?.state === "OPEN" + }) + + const isMerged = createMemo(() => pr()?.state === "MERGED") + const isClosed = createMemo(() => pr()?.state === "CLOSED") + + const containerStyle = createMemo(() => getPrButtonContainerStyle(pr())) + const dividerStyle = createMemo(() => getPrButtonDividerStyle(pr())) + + const ciLabel = createMemo(() => { + const p = pr() + if (!p?.checksState) return undefined + const summary = p.checksSummary + if (p.checksState === "SUCCESS") return language.t("pr.menu.ci.success") + if (p.checksState === "FAILURE" && summary) { + return language.t("pr.menu.ci.failure", { + failed: String(summary.failed), + total: String(summary.total), + }) + } + if (p.checksState === "PENDING" && summary) { + return language.t("pr.menu.ci.pending", { + pending: String(summary.pending), + plural: summary.pending === 1 ? "" : "s", + }) + } + return language.t("pr.menu.ci") + }) + + const ciIconName = createMemo(() => { + const p = pr() + if (p?.checksState === "SUCCESS") return "circle-check" as const + if (p?.checksState === "FAILURE") return "circle-x" as const + return "loader" as const + }) + + const ciIconColor = createMemo(() => { + const p = pr() + if (p?.checksState === "SUCCESS") return "text-icon-success-base" + if (p?.checksState === "FAILURE") return "text-icon-critical-base" + return "text-icon-warning-base" + }) + + const ciIconSpin = createMemo(() => pr()?.checksState === "PENDING") + + const commentLabel = createMemo(() => { + const p = pr() + if (!p) return language.t("pr.menu.comments") + if (p.unresolvedCommentCount === undefined) return language.t("pr.menu.comments") + if (p.unresolvedCommentCount === 0) return language.t("pr.menu.comments.none") + if (p.unresolvedCommentCount === 1) return language.t("pr.menu.comments.one") + return language.t("pr.menu.comments.count", { count: String(p.unresolvedCommentCount) }) + }) + + const openPr = () => { + const p = pr() + if (!p?.url) return + platform.openLink(p.url) + } + + const copyLink = () => { + const p = pr() + if (!p?.url) return + navigator.clipboard + .writeText(p.url) + .then(() => { + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.copied"), + description: p.url, + }) + }) + .catch(() => {}) + } + + const viewCI = () => { + const p = pr() + if (!p?.checksUrl) return + platform.openLink(p.checksUrl) + } + + const openCreateDialog = () => { + dialog.show(() => ) + } + + const openAddressDialog = () => { + dialog.show(() => ) + } + + const openMergeDialog = () => { + dialog.show(() => ) + } + + const handleDeleteBranch = async () => { + const p = pr() + if (!p?.headRefName) return + try { + await sdk.client.vcs.pr.deleteBranch({ + directory: sdk.directory, + prDeleteBranchInput: { branch: p.headRefName }, + }) + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.branch_deleted"), + }) + } catch { + showToast({ + variant: "error", + icon: "circle-x", + title: language.t("pr.error.delete_branch_failed"), + }) + } + } + + return ( + +
+
+ 0}> +
+ +
+ + + + } + > + {(currentPr) => ( + <> + +
+ setMenu("open", open)} + > + + + + + + {/* Copy link — always visible */} + { + setMenu("open", false) + copyLink() + }} + > + + {language.t("pr.menu.copy")} + + + {/* CI status — shown when checks exist */} + + + { + setMenu("open", false) + viewCI() + }} + > + + {ciLabel()} + + + + {/* Merge — only for open, non-draft PRs */} + + + + { + setMenu("open", false) + openMergeDialog() + }} + > + + {language.t("pr.menu.merge")} + + + + + {/* Address comments — only for open/draft PRs */} + + + + { + setMenu("open", false) + openAddressDialog() + }} + > + + {commentLabel()} + + + + + {/* Status info for merged/closed */} + + + + + + {isMerged() ? language.t("pr.menu.status.merged") : language.t("pr.menu.status.closed")} + + + + + {/* Delete branch — only for merged PRs */} + + { + setMenu("open", false) + handleDeleteBranch() + }} + > + + {language.t("pr.menu.delete_branch")} + + + + + + + )} + +
+
+
+ + ) +} diff --git a/packages/app/src/components/session/session-header.tsx b/packages/app/src/components/session/session-header.tsx index 495b32340580..4fe63e4dc8cd 100644 --- a/packages/app/src/components/session/session-header.tsx +++ b/packages/app/src/components/session/session-header.tsx @@ -23,6 +23,7 @@ import { useSessionLayout } from "@/pages/session/session-layout" import { messageAgentColor } from "@/utils/agent" import { decode64 } from "@/utils/base64" import { Persist, persisted } from "@/utils/persist" +import { PrButton } from "../pr-button" import { StatusPopover } from "../status-popover" const OPEN_APPS = [ @@ -414,6 +415,7 @@ export function SessionHeader() {
+
diff --git a/packages/app/src/context/global-sync/child-store.ts b/packages/app/src/context/global-sync/child-store.ts index 70668350ec20..28309ee74c7b 100644 --- a/packages/app/src/context/global-sync/child-store.ts +++ b/packages/app/src/context/global-sync/child-store.ts @@ -126,7 +126,7 @@ export function createChildStoreManager(input: { if (!children[directory]) { const vcs = runWithOwner(input.owner, () => persisted( - Persist.workspace(directory, "vcs", ["vcs.v1"]), + Persist.workspace(directory, "vcs", ["vcs.v2"]), createStore({ value: undefined as VcsInfo | undefined }), ), ) diff --git a/packages/app/src/context/global-sync/event-reducer.ts b/packages/app/src/context/global-sync/event-reducer.ts index b8eda0573f7b..8d8fafb8ef58 100644 --- a/packages/app/src/context/global-sync/event-reducer.ts +++ b/packages/app/src/context/global-sync/event-reducer.ts @@ -10,6 +10,7 @@ import type { Session, SessionStatus, Todo, + VcsInfo, } from "@opencode-ai/sdk/v2/client" import type { State, VcsCache } from "./types" import { trimSessions } from "./session-trim" @@ -267,10 +268,27 @@ export function applyDirectoryEvent(input: { ) break } + case "vcs.updated": { + const props = event.properties as { + branch?: string + defaultBranch?: string + dirty?: number + pr?: VcsInfo["pr"] + github?: VcsInfo["github"] + } + const next: VcsInfo = { + ...input.store.vcs, + ...props, + branch: props.branch ?? input.store.vcs?.branch ?? "", + } + input.setStore("vcs", next) + if (input.vcsCache) input.vcsCache.setStore("value", next) + break + } case "vcs.branch.updated": { const props = event.properties as { branch: string } if (input.store.vcs?.branch === props.branch) break - const next = { branch: props.branch } + const next = { ...input.store.vcs, branch: props.branch } input.setStore("vcs", next) if (input.vcsCache) input.vcsCache.setStore("value", next) break diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 8efd9d3bc9f0..39bc76f68341 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -85,6 +85,78 @@ export const dict = { "command.session.compact.description": "Summarize the session to reduce context size", "command.session.fork": "Fork from message", "command.session.fork.description": "Create a new session from a previous message", + "command.category.pr": "Pull Request", + "command.pr.open": "Open Pull Request", + "command.pr.create": "Create Pull Request", + "command.pr.copy": "Copy Pull Request Link", + "command.pr.comments": "Address Review Comments", + + "pr.button.open": "PR #{{number}}", + "pr.button.create": "Create PR", + "pr.menu.copy": "Copy PR Link", + "pr.menu.ci": "View CI Status", + "pr.menu.ci.success": "All checks passed", + "pr.menu.ci.failure": "{{failed}} of {{total}} checks failed", + "pr.menu.ci.pending": "{{pending}} check{{plural}} in progress", + "pr.menu.comments": "Address Review Comments", + "pr.menu.comments.one": "Address 1 Comment", + "pr.menu.comments.count": "Address {{count}} Comments", + "pr.menu.comments.none": "No Review Comments", + "pr.menu.status.merged": "Merged", + "pr.menu.status.closed": "Closed", + "pr.create.title": "Create Pull Request", + "pr.create.field.title": "Title", + "pr.create.field.body": "Description", + "pr.create.field.body.placeholder": "Leave empty to use title as description", + "pr.create.field.base": "Base Branch", + "pr.create.field.draft": "Create as Draft", + "pr.create.submit": "Create", + "pr.create.submitPush": "Push & Create", + "pr.create.submitting": "Creating...", + "pr.commit.warning": "{{count}} uncommitted changes", + "pr.commit.action": "Commit changes", + "pr.commit.placeholder": "Commit message", + "pr.commit.submit": "Commit", + "pr.commit.committing": "Committing...", + "pr.commit.success": "Changes committed", + "pr.comments.title": "Address Review Comments", + "pr.comments.count": "{{count}} unresolved comments", + "pr.comments.count.one": "1 unresolved comment", + "pr.comments.selected": "{{selected}} of {{total}} selected", + "pr.comments.none": "No unresolved comments", + "pr.comments.error.title": "Unable to fetch comments", + "pr.comments.instructions": "Additional instructions", + "pr.comments.instructions.placeholder": "Any additional instructions for the agent...", + "pr.comments.submit": "Prefill Prompt", + "pr.comments.select.all": "Select all", + "pr.comments.select.none": "Deselect all", + "pr.toast.copied": "PR link copied", + "pr.toast.created": "Pull request created", + "pr.error.gh_not_installed": "GitHub CLI (gh) is not installed", + "pr.error.gh_not_authenticated": "Run `gh auth login` to authenticate", + "pr.error.upstream_missing": "Push your branch first", + "pr.error.create_failed": "Failed to create pull request", + "pr.error.no_pr": "No pull request found", + "pr.error.comments_fetch_failed": "Failed to fetch review comments", + "pr.menu.merge": "Merge Pull Request", + "pr.menu.delete_branch": "Delete Branch", + "pr.merge.title": "Merge Pull Request", +"pr.merge.strategy": "Merge Strategy", + "pr.merge.strategy.squash": "Squash and merge", + "pr.merge.strategy.merge": "Create a merge commit", + "pr.merge.strategy.rebase": "Rebase and merge", + "pr.merge.delete_branch": "Delete branch after merge", + "pr.merge.submit": "Merge", + "pr.merge.submitting": "Merging...", + "pr.merge.conflict": "This branch has conflicts that must be resolved", + "pr.merge.checks_failing": "Some checks are failing", + "pr.toast.merged": "Pull request merged", + "pr.toast.branch_deleted": "Branch deleted", + "pr.error.merge_failed": "Failed to merge pull request", + "pr.error.delete_branch_failed": "Failed to delete branch", + "pr.tooltip.not_authenticated": "Run `gh auth login` to enable", + "pr.tooltip.not_local": "PR actions require local server", + "command.session.share": "Share session", "command.session.share.description": "Share this session and copy the URL to clipboard", "command.session.unshare": "Unshare session", diff --git a/packages/app/src/pages/layout/sidebar-workspace.tsx b/packages/app/src/pages/layout/sidebar-workspace.tsx index 3bf00ea424d6..a6c6aee8e129 100644 --- a/packages/app/src/pages/layout/sidebar-workspace.tsx +++ b/packages/app/src/pages/layout/sidebar-workspace.tsx @@ -12,7 +12,8 @@ import { Icon } from "@opencode-ai/ui/icon" import { IconButton } from "@opencode-ai/ui/icon-button" import { Spinner } from "@opencode-ai/ui/spinner" import { Tooltip } from "@opencode-ai/ui/tooltip" -import { type Session } from "@opencode-ai/sdk/v2/client" +import { type Session, type PrInfo } from "@opencode-ai/sdk/v2/client" +import { getPrPillStyle } from "@/utils/pr-style" import { type LocalProject } from "@/context/layout" import { useGlobalSync } from "@/context/global-sync" import { useLanguage } from "@/context/language" @@ -83,6 +84,8 @@ export const WorkspaceDragOverlay = (props: { ) } +const prPillStyle = getPrPillStyle + const WorkspaceHeader = (props: { local: Accessor busy: Accessor @@ -90,6 +93,7 @@ const WorkspaceHeader = (props: { directory: string language: ReturnType branch: Accessor + pr: Accessor workspaceValue: Accessor workspaceEditActive: Accessor InlineEditor: WorkspaceSidebarContext["InlineEditor"] @@ -130,7 +134,19 @@ const WorkspaceHeader = (props: { openOnDblClick={false} /> -
+ + {(currentPr) => { + const pillStyle = () => prPillStyle(currentPr()) + return ( + + PR #{currentPr().number} + + ) + }} + +
@@ -353,6 +369,7 @@ export const SortableWorkspace = (props: { directory={props.directory} language={language} branch={() => workspaceStore.vcs?.branch} + pr={() => workspaceStore.vcs?.pr} workspaceValue={workspaceValue} workspaceEditActive={workspaceEditActive} InlineEditor={props.ctx.InlineEditor} @@ -382,6 +399,7 @@ export const SortableWorkspace = (props: { "opacity-50 pointer-events-none": busy(), }} > +
{ const permission = usePermission() const prompt = usePrompt() const sdk = useSDK() + const platform = usePlatform() + const server = useServer() const sync = useSync() const terminal = useTerminal() const layout = useLayout() @@ -126,6 +132,7 @@ export const useSessionCommands = (actions: SessionCommandContext) => { const mcpCommand = withCategory(language.t("command.category.mcp")) const agentCommand = withCategory(language.t("command.category.agent")) const permissionsCommand = withCategory(language.t("command.category.permissions")) + const prCommand = withCategory(language.t("command.category.pr")) const isAutoAcceptActive = () => { const sessionID = params.id @@ -489,6 +496,56 @@ export const useSessionCommands = (actions: SessionCommandContext) => { disabled: !params.id || visibleUserMessages().length === 0, onSelect: () => dialog.show(() => ), }), + ...(function () { + const vcs = sync.data.vcs + if (!vcs?.github?.available) return [] + + const hasPr = !!vcs.pr + const canMutate = server.isLocal() && vcs.github.authenticated + + if (hasPr) { + return [ + prCommand({ + id: "pr.open", + title: language.t("command.pr.open"), + onSelect: () => { + if (vcs.pr?.url) platform.openLink(vcs.pr.url) + }, + }), + prCommand({ + id: "pr.copy", + title: language.t("command.pr.copy"), + onSelect: () => { + if (!vcs.pr?.url) return + navigator.clipboard.writeText(vcs.pr.url).then(() => { + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.copied"), + }) + }) + }, + }), + prCommand({ + id: "pr.comments", + title: language.t("command.pr.comments"), + disabled: !canMutate, + onSelect: () => dialog.show(() => ), + }), + ] + } + + if (vcs.branch === vcs.defaultBranch) return [] + + return [ + prCommand({ + id: "pr.create", + title: language.t("command.pr.create"), + disabled: !canMutate, + onSelect: () => dialog.show(() => ), + }), + ] + })(), ...share, ] }) diff --git a/packages/app/src/utils/pr-style.ts b/packages/app/src/utils/pr-style.ts new file mode 100644 index 000000000000..bc55019511c6 --- /dev/null +++ b/packages/app/src/utils/pr-style.ts @@ -0,0 +1,25 @@ +import type { PrInfo } from "@opencode-ai/sdk/v2/client" + +export function getPrPillStyle(pr: PrInfo): string { + if (pr.state === "MERGED") return "border-[#8957e5]/60 bg-[#8957e5]/20 text-[#a371f7]" + if (pr.state === "CLOSED") return "border-[#da3633]/60 bg-[#da3633]/20 text-[#f85149]" + if (pr.isDraft) return "border-[#768390]/60 bg-[#768390]/20 text-[#768390]" + if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[#da3633]/60 bg-[#da3633]/20 text-[#f85149]" + return "border-[#238636]/60 bg-[#238636]/20 text-[#3fb950]" +} + +export function getPrButtonContainerStyle(pr: PrInfo | undefined): string { + if (!pr) return "border-border-weak-base bg-surface-panel" + if (pr.state === "MERGED") return "border-[#8957e5]/60 bg-[#8957e5]/20" + if (pr.state === "CLOSED") return "border-[#da3633]/60 bg-[#da3633]/20" + if (pr.isDraft) return "border-[#768390]/60 bg-[#768390]/20" + return "border-[#238636]/60 bg-[#238636]/20" +} + +export function getPrButtonDividerStyle(pr: PrInfo | undefined): string { + if (!pr) return "bg-border-weak-base" + if (pr.state === "MERGED") return "bg-[#8957e5]/60" + if (pr.state === "CLOSED") return "bg-[#da3633]/60" + if (pr.isDraft) return "bg-[#768390]/60" + return "bg-[#238636]/60" +} diff --git a/packages/ui/src/components/dialog.css b/packages/ui/src/components/dialog.css index 1e74763ae2d8..7588db1928da 100644 --- a/packages/ui/src/components/dialog.css +++ b/packages/ui/src/components/dialog.css @@ -114,16 +114,6 @@ } } - &[data-fit] { - [data-slot="dialog-container"] { - height: auto; - - [data-slot="dialog-content"] { - min-height: 0; - } - } - } - &[data-size="large"] [data-slot="dialog-container"] { width: min(calc(100vw - 32px), 800px); height: min(calc(100vh - 32px), 600px); @@ -133,6 +123,16 @@ width: min(calc(100vw - 32px), 960px); height: min(calc(100vh - 32px), 600px); } + + &[data-fit] { + [data-slot="dialog-container"] { + height: auto; + + [data-slot="dialog-content"] { + min-height: 0; + } + } + } } [data-component="dialog"][data-transition] [data-slot="dialog-content"] { diff --git a/packages/ui/src/components/icon.tsx b/packages/ui/src/components/icon.tsx index e2eaf107a675..f727dd9629cf 100644 --- a/packages/ui/src/components/icon.tsx +++ b/packages/ui/src/components/icon.tsx @@ -78,6 +78,8 @@ const icons = { "layout-bottom-full": ``, "dot-grid": ``, "circle-check": ``, + "circle-check-fill": ``, + "circle-fill": ``, copy: ``, check: ``, photo: ``, @@ -102,6 +104,7 @@ const icons = { link: ``, providers: ``, models: ``, + loader: ``, } export interface IconProps extends ComponentProps<"svg"> { From be4713170d0a76ad08719668958cc5f8874b0e8d Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 12:26:16 -0500 Subject: [PATCH 04/46] fix(pr): improve type safety, error handling, and code organization - Remove `as any` casts in session commands and route handlers - Extract shared API error parsing into pr-errors.ts utility - Add GraphQL response type interface for pr-comments.ts - Fix non-null assertions in dialog-address-comments.tsx - Add branch name validation regex to DeleteBranchInput - Remove dead `typeof authCmd === "string"` code path in vcs.ts - Standardize catch blocks with proper error typing - Add i18n keys for reply count strings - Move fetchPrForBranch and fetchUnresolvedCommentCount from vcs.ts to pr.ts --- .../components/dialog-address-comments.tsx | 49 ++++--- .../app/src/components/dialog-create-pr.tsx | 20 +-- .../app/src/components/dialog-merge-pr.tsx | 18 +-- packages/app/src/components/pr-button.tsx | 4 +- packages/app/src/i18n/en.ts | 2 + packages/app/src/utils/pr-errors.ts | 35 +++++ packages/opencode/src/project/pr-comments.ts | 70 ++++++---- packages/opencode/src/project/pr.ts | 129 +++++++++++++++++- packages/opencode/src/project/vcs.ts | 126 +---------------- packages/opencode/src/server/routes/vcs.ts | 10 +- 10 files changed, 256 insertions(+), 207 deletions(-) create mode 100644 packages/app/src/utils/pr-errors.ts diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 39ab370d9566..a7d168314545 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -11,6 +11,7 @@ import { useLanguage } from "@/context/language" import { usePrompt } from "@/context/prompt" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" +import { resolveApiErrorMessage } from "@/utils/pr-errors" import type { ReviewThread } from "@opencode-ai/sdk/v2" export function AddressCommentsDialog() { @@ -51,13 +52,8 @@ export function AddressCommentsDialog() { loading: false, }) } catch (e: unknown) { - if (import.meta.env.DEV) { - console.error("Fetch comments error:", e) - } - const err = (e as { error?: { code?: string; message?: string } })?.error ?? e - const errObj = err as { code?: string; message?: string } - const msg = errObj?.message ?? errObj?.code ?? "Failed to fetch review comments" - setStore("error", String(msg)) + if (import.meta.env.DEV) console.error("Fetch comments error:", e) + setStore("error", resolveApiErrorMessage(e, "Failed to fetch review comments", (k) => language.t(k as Parameters[0]))) setStore("loading", false) } }) @@ -212,18 +208,29 @@ export function AddressCommentsDialog() { )}
- - e.stopPropagation()} - class="text-icon-weak hover:text-text-strong transition-colors shrink-0 cursor-pointer" - title="View on GitHub" - > - - - + {(() => { + const g = github() + const p = pr() + const href = g?.repo && p?.number + ? `https://github.com/${g.repo.owner}/${g.repo.name}/pull/${p.number}#discussion_r${comment().id}` + : undefined + return ( + + {(url) => ( + e.stopPropagation()} + class="text-icon-weak hover:text-text-strong transition-colors shrink-0 cursor-pointer" + title="View on GitHub" + > + + + )} + + ) + })()}

{comment().body} @@ -237,7 +244,9 @@ export function AddressCommentsDialog() {

↳ - {replies().length} {replies().length === 1 ? "reply" : "replies"} + {replies().length === 1 + ? language.t("pr.comments.replies.one") + : language.t("pr.comments.replies.count", { count: String(replies().length) })}
diff --git a/packages/app/src/components/dialog-create-pr.tsx b/packages/app/src/components/dialog-create-pr.tsx index 3923348edba9..0a18e5d92a02 100644 --- a/packages/app/src/components/dialog-create-pr.tsx +++ b/packages/app/src/components/dialog-create-pr.tsx @@ -11,6 +11,7 @@ import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" +import { resolveApiErrorMessage } from "@/utils/pr-errors" export function CreatePrDialog() { const dialog = useDialog() @@ -121,23 +122,8 @@ export function CreatePrDialog() { }) dialog.close() } catch (e: unknown) { - if (import.meta.env.DEV) { - console.error("PR creation error:", e) - } - - let errorMsg: string - const err = - (e as { error?: { code?: string; message?: string } })?.error ?? (e as { code?: string; message?: string }) - if (err?.code) { - // Dynamic lookup: pr.error.gh_not_authenticated, pr.error.no_repo, etc. - const errorKey = `pr.error.${err.code.toLowerCase()}` - const translated = language.t(errorKey as Parameters[0]) - errorMsg = translated !== errorKey ? translated : err.message || "Failed to create PR" - } else { - errorMsg = err?.message ?? (typeof err === "string" ? err : "Failed to create PR") - } - - setStore("error", errorMsg) + if (import.meta.env.DEV) console.error("PR creation error:", e) + setStore("error", resolveApiErrorMessage(e, "Failed to create PR", (k) => language.t(k as Parameters[0]))) } finally { setStore("submitting", false) } diff --git a/packages/app/src/components/dialog-merge-pr.tsx b/packages/app/src/components/dialog-merge-pr.tsx index bf605517acd3..ff6eb1fab749 100644 --- a/packages/app/src/components/dialog-merge-pr.tsx +++ b/packages/app/src/components/dialog-merge-pr.tsx @@ -10,6 +10,7 @@ import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" +import { resolveApiErrorMessage } from "@/utils/pr-errors" type MergeStrategy = "squash" | "merge" | "rebase" @@ -59,21 +60,8 @@ export function MergePrDialog() { }) dialog.close() } catch (e: unknown) { - if (import.meta.env.DEV) { - console.error("PR merge error:", e) - } - - let errorMsg: string - const err = (e as { error?: { code?: string; message?: string } })?.error ?? (e as { code?: string; message?: string }) - if (err?.code) { - const errorKey = `pr.error.${err.code.toLowerCase()}` - const translated = language.t(errorKey as Parameters[0]) - errorMsg = translated !== errorKey ? translated : err.message || "Failed to merge PR" - } else { - errorMsg = err?.message ?? (typeof err === "string" ? err : "Failed to merge PR") - } - - setStore("error", errorMsg) + if (import.meta.env.DEV) console.error("PR merge error:", e) + setStore("error", resolveApiErrorMessage(e, "Failed to merge PR", (k) => language.t(k as Parameters[0]))) } finally { setStore("submitting", false) } diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 3e37063167cd..38403420cdad 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -132,7 +132,9 @@ export function PrButton() { description: p.url, }) }) - .catch(() => {}) + .catch(() => { + // Clipboard write can fail in non-secure contexts; not actionable + }) } const viewCI = () => { diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 39bc76f68341..a5e6659b5bbf 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -128,6 +128,8 @@ export const dict = { "pr.comments.instructions": "Additional instructions", "pr.comments.instructions.placeholder": "Any additional instructions for the agent...", "pr.comments.submit": "Prefill Prompt", + "pr.comments.replies.one": "1 reply", + "pr.comments.replies.count": "{{count}} replies", "pr.comments.select.all": "Select all", "pr.comments.select.none": "Deselect all", "pr.toast.copied": "PR link copied", diff --git a/packages/app/src/utils/pr-errors.ts b/packages/app/src/utils/pr-errors.ts new file mode 100644 index 000000000000..f1df2787d830 --- /dev/null +++ b/packages/app/src/utils/pr-errors.ts @@ -0,0 +1,35 @@ +interface ApiError { + code?: string + message?: string +} + +export function parseApiError(e: unknown): ApiError { + if (e && typeof e === "object") { + if ("error" in e) { + const inner = (e as { error?: unknown }).error + if (inner && typeof inner === "object") { + return inner as ApiError + } + } + if ("code" in e || "message" in e) { + return e as ApiError + } + } + if (e instanceof Error) return { message: e.message } + if (typeof e === "string") return { message: e } + return {} +} + +export function resolveApiErrorMessage( + e: unknown, + fallback: string, + translate?: (key: string) => string, +): string { + const err = parseApiError(e) + if (err.code && translate) { + const key = `pr.error.${err.code.toLowerCase()}` + const translated = translate(key) + if (translated !== key) return translated + } + return err.message ?? fallback +} diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts index e436337943ef..3867411ef18d 100644 --- a/packages/opencode/src/project/pr-comments.ts +++ b/packages/opencode/src/project/pr-comments.ts @@ -40,6 +40,35 @@ export namespace PrComments { .meta({ ref: "PrCommentsResponse" }) export type CommentsResponse = z.infer + interface GqlReviewThreadsResponse { + data?: { + repository?: { + pullRequest?: { + reviewThreads?: { + pageInfo?: { hasNextPage?: boolean; endCursor?: string } + nodes?: Array<{ + id: string + isResolved: boolean + path: string + line: number | null + comments: { + nodes: Array<{ + databaseId: number + author: { login: string } + body: string + path: string + line: number | null + diffHunk?: string + }> + } + }> + } + } + } + } + errors?: Array<{ message?: string }> + } + function buildQuery(owner: string, name: string, prNumber: number, cursor: string | null): string { return `query($owner: String!, $name: String!, $prNumber: Int!, $cursor: String) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 50) { nodes { databaseId author { login } body path line: originalLine diffHunk } } } } } } }` } @@ -104,37 +133,30 @@ export namespace PrComments { throw new PR.PrError("COMMENTS_FETCH_FAILED", "GitHub API returned invalid response") } - if (parsed.errors) { - const gqlErrors = parsed.errors as Array<{ message?: string }> - const msg = gqlErrors.map((e) => e.message).join("; ") - log.error("GraphQL errors", { errors: parsed.errors }) + const response = parsed as GqlReviewThreadsResponse + + if (response.errors) { + const msg = response.errors.map((e) => e.message).join("; ") + log.error("GraphQL errors", { errors: response.errors }) throw new PR.PrError("COMMENTS_FETCH_FAILED", msg || "GraphQL query failed") } - const reviewThreads = (parsed.data as Record | undefined)?.repository as - | Record - | undefined - const prData = reviewThreads?.pullRequest as Record | undefined - const threads = prData?.reviewThreads as - | { pageInfo?: { hasNextPage?: boolean; endCursor?: string }; nodes?: Array> } - | undefined - + const threads = response.data?.repository?.pullRequest?.reviewThreads if (!threads) break for (const node of threads.nodes ?? []) { - const commentsNodes = (node.comments as { nodes?: Array> })?.nodes ?? [] allThreads.push({ - id: node.id as string, - isResolved: node.isResolved as boolean, - path: (node.path as string) ?? "", - line: node.line as number | null, - comments: commentsNodes.map((c) => ({ - id: c.databaseId as number, - author: (c.author as { login?: string })?.login ?? "unknown", - body: (c.body as string) ?? "", - path: (c.path as string) ?? "", - line: (c.line as number | null) ?? null, - diffHunk: c.diffHunk as string | undefined, + id: node.id, + isResolved: node.isResolved, + path: node.path ?? "", + line: node.line, + comments: (node.comments.nodes ?? []).map((c) => ({ + id: c.databaseId, + author: c.author?.login ?? "unknown", + body: c.body ?? "", + path: c.path ?? "", + line: c.line ?? null, + diffHunk: c.diffHunk, })), }) } diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 8d8aa9a7533f..ed4a6812c68d 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -50,7 +50,7 @@ export namespace PR { export const DeleteBranchInput = z .object({ - branch: z.string(), + branch: z.string().regex(/^[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name"), }) .meta({ ref: "PrDeleteBranchInput" }) export type DeleteBranchInput = z.infer @@ -62,6 +62,123 @@ export namespace PR { }) .meta({ ref: "PrErrorResponse" }) + async function fetchUnresolvedCommentCount( + owner: string, + name: string, + prNumber: number, + ): Promise { + const cwd = Instance.worktree + const query = `query($owner: String!, $name: String!, $prNumber: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100) { nodes { isResolved } } } } }` + const result = await $`gh api graphql -f query=${query} -f owner=${owner} -f name=${name} -F prNumber=${prNumber}` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + try { + const parsed = JSON.parse(result) + const threads = parsed?.data?.repository?.pullRequest?.reviewThreads?.nodes + if (!Array.isArray(threads)) return undefined + return threads.filter((t: { isResolved: boolean }) => !t.isResolved).length + } catch { + return undefined + } + } + + export async function fetchForBranch(repo?: { owner: string; name: string }): Promise { + const cwd = Instance.worktree + const result = + await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => "") + if (!result.trim() || result.includes("no pull requests found")) { + return undefined + } + try { + const parsed = JSON.parse(result) + if (!parsed.number) return undefined + + let checksState: "SUCCESS" | "FAILURE" | "PENDING" | null = null + let checksUrl: string | undefined + let checksSummary: { total: number; passed: number; failed: number; pending: number; skipped: number } | undefined + if (parsed.statusCheckRollup && Array.isArray(parsed.statusCheckRollup)) { + const checks = parsed.statusCheckRollup as Array<{ + __typename?: string + conclusion?: string + status?: string + state?: string + detailsUrl?: string + }> + if (checks.length > 0) { + checksUrl = parsed.url ? `${parsed.url}/checks` : checks[0]?.detailsUrl?.replace(/\/runs\/.*/, "") + // StatusContext entries (e.g. CodeRabbit) use `state` instead of `conclusion`/`status` + const isSuccess = (c: (typeof checks)[0]) => + c.conclusion === "SUCCESS" || c.conclusion === "success" || c.state === "SUCCESS" || c.state === "success" + const isFailure = (c: (typeof checks)[0]) => + c.conclusion === "FAILURE" || + c.conclusion === "failure" || + c.state === "FAILURE" || + c.state === "failure" || + c.state === "ERROR" || + c.state === "error" + const isSkipped = (c: (typeof checks)[0]) => + c.conclusion === "SKIPPED" || + c.conclusion === "skipped" || + c.conclusion === "NEUTRAL" || + c.conclusion === "neutral" + const failed = checks.filter(isFailure).length + const passed = checks.filter(isSuccess).length + const skipped = checks.filter(isSkipped).length + const total = checks.length + const pending = total - passed - failed - skipped + checksSummary = { total, passed, failed, pending, skipped } + if (failed > 0) checksState = "FAILURE" + else if (pending === 0) checksState = "SUCCESS" + else checksState = "PENDING" + } + } + + const stateMap: Record = { + OPEN: "OPEN", + CLOSED: "CLOSED", + MERGED: "MERGED", + } + + const mergeableMap: Record = { + MERGEABLE: "MERGEABLE", + CONFLICTING: "CONFLICTING", + UNKNOWN: "UNKNOWN", + } + + const pr: Vcs.PrInfo = { + number: parsed.number, + url: parsed.url, + title: parsed.title, + state: stateMap[parsed.state] ?? "OPEN", + headRefName: parsed.headRefName, + baseRefName: parsed.baseRefName, + isDraft: parsed.isDraft ?? false, + mergeable: mergeableMap[parsed.mergeable] ?? "UNKNOWN", + reviewDecision: parsed.reviewDecision || null, + checksState, + checksUrl, + checksSummary, + } + + // Fetch unresolved comment count via lightweight GraphQL query + if (repo) { + pr.unresolvedCommentCount = await fetchUnresolvedCommentCount(repo.owner, repo.name, pr.number) + } + + return pr + } catch { + return undefined + } + } + async function ensureGithub() { const info = await Vcs.info() const github = info.github @@ -104,7 +221,7 @@ export namespace PR { .quiet() .nothrow() .cwd(cwd) - .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) const result = cmd.stdout.toString() const errorOut = cmd.stderr.toString() @@ -202,7 +319,7 @@ export namespace PR { .quiet() .nothrow() .cwd(cwd) - .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) if (remote.exitCode !== 0) { const errorOut = remote.stderr.toString().trim() @@ -219,13 +336,15 @@ export namespace PR { .nothrow() .cwd(cwd) .text() - .catch(() => "") + .catch((_e: unknown) => "") if (currentBranch.trim() !== input.branch) { await $`git branch -D ${input.branch}` .quiet() .nothrow() .cwd(cwd) - .catch(() => {}) + .catch(() => { + // Local branch cleanup is best-effort + }) } await Vcs.refresh() diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 3ef218591de9..f2568862f2db 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -97,13 +97,12 @@ export namespace Vcs { .nothrow() .cwd(cwd) .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) - const authText = - typeof authCmd === "string" ? authCmd : (authCmd.stdout?.toString() ?? "") + (authCmd.stderr?.toString() ?? "") + const authText = (authCmd.stdout?.toString() ?? "") + (authCmd.stderr?.toString() ?? "") const available = !authText.includes("not found") && !authText.includes("command not found") if (!available) { return { available: false, authenticated: false } } - const authenticated = authText.includes("Logged in") || (typeof authCmd !== "string" && authCmd.exitCode === 0) + const authenticated = authText.includes("Logged in") || authCmd.exitCode === 0 if (!authenticated) { return { available: true, authenticated: false } } @@ -178,123 +177,6 @@ export namespace Vcs { return branches } - async function fetchUnresolvedCommentCount( - owner: string, - name: string, - prNumber: number, - ): Promise { - const cwd = Instance.worktree - const query = `query($owner: String!, $name: String!, $prNumber: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100) { nodes { isResolved } } } } }` - const result = await $`gh api graphql -f query=${query} -f owner=${owner} -f name=${name} -F prNumber=${prNumber}` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch(() => "") - try { - const parsed = JSON.parse(result) - const threads = parsed?.data?.repository?.pullRequest?.reviewThreads?.nodes - if (!Array.isArray(threads)) return undefined - return threads.filter((t: { isResolved: boolean }) => !t.isResolved).length - } catch { - return undefined - } - } - - export async function fetchPrForBranch(repo?: { owner: string; name: string }): Promise { - const cwd = Instance.worktree - const result = - await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch(() => "") - if (!result.trim() || result.includes("no pull requests found")) { - return undefined - } - try { - const parsed = JSON.parse(result) - if (!parsed.number) return undefined - - let checksState: "SUCCESS" | "FAILURE" | "PENDING" | null = null - let checksUrl: string | undefined - let checksSummary: { total: number; passed: number; failed: number; pending: number; skipped: number } | undefined - if (parsed.statusCheckRollup && Array.isArray(parsed.statusCheckRollup)) { - const checks = parsed.statusCheckRollup as Array<{ - __typename?: string - conclusion?: string - status?: string - state?: string - detailsUrl?: string - }> - if (checks.length > 0) { - checksUrl = parsed.url ? `${parsed.url}/checks` : checks[0]?.detailsUrl?.replace(/\/runs\/.*/, "") - // StatusContext entries (e.g. CodeRabbit) use `state` instead of `conclusion`/`status` - const isSuccess = (c: (typeof checks)[0]) => - c.conclusion === "SUCCESS" || c.conclusion === "success" || c.state === "SUCCESS" || c.state === "success" - const isFailure = (c: (typeof checks)[0]) => - c.conclusion === "FAILURE" || - c.conclusion === "failure" || - c.state === "FAILURE" || - c.state === "failure" || - c.state === "ERROR" || - c.state === "error" - const isSkipped = (c: (typeof checks)[0]) => - c.conclusion === "SKIPPED" || - c.conclusion === "skipped" || - c.conclusion === "NEUTRAL" || - c.conclusion === "neutral" - const failed = checks.filter(isFailure).length - const passed = checks.filter(isSuccess).length - const skipped = checks.filter(isSkipped).length - const total = checks.length - const pending = total - passed - failed - skipped - checksSummary = { total, passed, failed, pending, skipped } - if (failed > 0) checksState = "FAILURE" - else if (pending === 0) checksState = "SUCCESS" - else checksState = "PENDING" - } - } - - const stateMap: Record = { - OPEN: "OPEN", - CLOSED: "CLOSED", - MERGED: "MERGED", - } - - const mergeableMap: Record = { - MERGEABLE: "MERGEABLE", - CONFLICTING: "CONFLICTING", - UNKNOWN: "UNKNOWN", - } - - const pr: PrInfo = { - number: parsed.number, - url: parsed.url, - title: parsed.title, - state: stateMap[parsed.state] ?? "OPEN", - headRefName: parsed.headRefName, - baseRefName: parsed.baseRefName, - isDraft: parsed.isDraft ?? false, - mergeable: mergeableMap[parsed.mergeable] ?? "UNKNOWN", - reviewDecision: parsed.reviewDecision || null, - checksState, - checksUrl, - checksSummary, - } - - // Fetch unresolved comment count via lightweight GraphQL query - if (repo) { - pr.unresolvedCommentCount = await fetchUnresolvedCommentCount(repo.owner, repo.name, pr.number) - } - - return pr - } catch { - return undefined - } - } - async function fetchDirtyCount(): Promise { const cwd = Instance.worktree const result = await $`git status --porcelain` @@ -320,7 +202,9 @@ export namespace Vcs { let pr: PrInfo | undefined if (github.authenticated) { - pr = await fetchPrForBranch(github.repo) + // Dynamic import to avoid circular dependency (pr.ts → vcs.ts) + const { PR } = await import("./pr") + pr = await PR.fetchForBranch(github.repo) } return { diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index 15b31e569157..387069ef0225 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -141,8 +141,7 @@ export const VcsRoutes = lazy(() => if (e instanceof PR.PrError) { return c.json({ code: e.code, message: e.message }, { status: 400 }) } - const message = e instanceof Error ? e.message : String(e) - return c.json({ code: "COMMENTS_FETCH_FAILED", message }, { status: 400 }) + throw e } }, ) @@ -234,8 +233,11 @@ export const VcsRoutes = lazy(() => const input = c.req.valid("json") await Vcs.commit(input.message) return c.json({ ok: true }) - } catch (e: any) { - return c.json({ code: "COMMIT_FAILED", message: e?.message ?? "Commit failed" }, { status: 400 }) + } catch (e) { + if (e instanceof Error) { + return c.json({ code: "COMMIT_FAILED", message: e.message }, { status: 400 }) + } + throw e } }, ), From 25bc63ba5f350693e4a95441f0f5ecf37f6cf748 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 15:04:24 -0500 Subject: [PATCH 05/46] fix(app): improve address-comments prompt to commit, push, and reply per comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent prompt now walks through each review comment individually: fix → commit → push → reply with SHA, or skip → reply with rationale. --- .../components/dialog-address-comments.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index a7d168314545..1dd2d45a5414 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -95,12 +95,19 @@ export function AddressCommentsDialog() { } text += `## Instructions\n\n` - text += `1. Read each comment above and decide whether to fix it or intentionally skip it\n` - text += `2. For fixes: make the code change\n` - text += `3. For skips: explain the design rationale in your reply\n` - text += `4. After addressing all comments, reply to each one on GitHub using the comment ID shown above:\n` - text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="" -F in_reply_to=\`\n` - text += `5. Do NOT merge, rebase, or force-push\n` + text += `Work through each comment above **one at a time**. The comment ID for each is shown above next to the author. For each comment:\n\n` + text += `1. Read the comment and decide whether to fix it or intentionally skip it\n` + text += `2. **If fixing:**\n` + text += ` - Make the code change\n` + text += ` - Commit with a descriptive message following the repository's commit conventions\n` + text += ` - \`git push\`\n` + text += ` - Get the commit SHA with \`git rev-parse HEAD\`\n` + text += ` - Reply on GitHub referencing the commit:\n` + text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="Fixed in : " -F in_reply_to=\`\n` + text += `3. **If skipping:**\n` + text += ` - Reply on GitHub explaining the design rationale:\n` + text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="" -F in_reply_to=\`\n` + text += `4. Do NOT merge, rebase, or force-push\n` if (store.instructions.trim()) { text += `\n${store.instructions.trim()}\n` From 1283ebead378a45d9907b742aeb334c17efd7362 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 1 Mar 2026 15:38:01 -0500 Subject: [PATCH 06/46] update(app): mute sidebar PR badge colors slightly --- packages/app/src/utils/pr-style.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/app/src/utils/pr-style.ts b/packages/app/src/utils/pr-style.ts index bc55019511c6..b8de41fef672 100644 --- a/packages/app/src/utils/pr-style.ts +++ b/packages/app/src/utils/pr-style.ts @@ -1,11 +1,11 @@ import type { PrInfo } from "@opencode-ai/sdk/v2/client" export function getPrPillStyle(pr: PrInfo): string { - if (pr.state === "MERGED") return "border-[#8957e5]/60 bg-[#8957e5]/20 text-[#a371f7]" - if (pr.state === "CLOSED") return "border-[#da3633]/60 bg-[#da3633]/20 text-[#f85149]" - if (pr.isDraft) return "border-[#768390]/60 bg-[#768390]/20 text-[#768390]" - if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[#da3633]/60 bg-[#da3633]/20 text-[#f85149]" - return "border-[#238636]/60 bg-[#238636]/20 text-[#3fb950]" + if (pr.state === "MERGED") return "border-[#8957e5]/40 bg-[#8957e5]/15 text-[#a371f7]" + if (pr.state === "CLOSED") return "border-[#da3633]/40 bg-[#da3633]/15 text-[#f85149]" + if (pr.isDraft) return "border-[#768390]/40 bg-[#768390]/15 text-[#768390]" + if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[#da3633]/40 bg-[#da3633]/15 text-[#f85149]" + return "border-[#238636]/40 bg-[#238636]/15 text-[#3fb950]" } export function getPrButtonContainerStyle(pr: PrInfo | undefined): string { From ea1894491fb1b9197c3b3288adcd79c9d31930d5 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 13:14:12 -0500 Subject: [PATCH 07/46] update(app): match PR badge style to sidebar unread badge Use the same size, color, and positioning as the project unread indicator for visual consistency. --- packages/app/src/components/pr-button.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 38403420cdad..3e2996902d39 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -182,7 +182,7 @@ export function PrButton() {
0}> -
+
Date: Mon, 2 Mar 2026 13:55:10 -0500 Subject: [PATCH 08/46] update(app): refine address-comments dialog card design Soften comment card borders using border-weaker-base/border-weak-base, switch to rounded cards with gap spacing, add circular checkboxes with interactive colors, use authorIsBot flag, and improve typography/spacing. --- .../components/dialog-address-comments.tsx | 107 ++++++++++-------- 1 file changed, 61 insertions(+), 46 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 1dd2d45a5414..8a8764d1fcdd 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -53,7 +53,12 @@ export function AddressCommentsDialog() { }) } catch (e: unknown) { if (import.meta.env.DEV) console.error("Fetch comments error:", e) - setStore("error", resolveApiErrorMessage(e, "Failed to fetch review comments", (k) => language.t(k as Parameters[0]))) + setStore( + "error", + resolveApiErrorMessage(e, "Failed to fetch review comments", (k) => + language.t(k as Parameters[0]), + ), + ) setStore("loading", false) } }) @@ -170,7 +175,7 @@ export function AddressCommentsDialog() {
{/* Comment thread list */} -
+
{(thread) => { const firstComment = () => thread.comments[0] @@ -179,21 +184,27 @@ export function AddressCommentsDialog() { return (
toggleThread(thread.id)} >
- +  
-
+
{/* File path + line */}
- {thread.path} + {thread.path} : @@ -204,51 +215,55 @@ export function AddressCommentsDialog() { {/* First comment (the review comment) */} - {(comment) => ( -
-
-
- @{comment().author} - {comment().author.includes("bot") && ( - - Bot - - )} + {(comment) => { + const isBot = comment().authorIsBot + return ( +
+
+
+ @{comment().author} + {isBot && ( + + Bot + + )} +
+ {(() => { + const g = github() + const p = pr() + const href = + g?.repo && p?.number + ? `https://github.com/${g.repo.owner}/${g.repo.name}/pull/${p.number}#discussion_r${comment().id}` + : undefined + return ( + + {(url) => ( + e.stopPropagation()} + class="text-icon-weaker hover:text-text-weak transition-colors shrink-0 cursor-pointer" + title="View on GitHub" + > + + + )} + + ) + })()}
- {(() => { - const g = github() - const p = pr() - const href = g?.repo && p?.number - ? `https://github.com/${g.repo.owner}/${g.repo.name}/pull/${p.number}#discussion_r${comment().id}` - : undefined - return ( - - {(url) => ( - e.stopPropagation()} - class="text-icon-weak hover:text-text-strong transition-colors shrink-0 cursor-pointer" - title="View on GitHub" - > - - - )} - - ) - })()} +

+ {comment().body} +

-

- {comment().body} -

-
- )} + ) + }} {/* Reply count */} 0}> -
+
↳ {replies().length === 1 From 6c91f8600f45ee0f7ccf04a602342b1de9994cc8 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 13:57:15 -0500 Subject: [PATCH 09/46] update(app): remove additional instructions field from address-comments dialog The field only pre-fills text into the prompt input, which users can edit directly after the prompt is generated. Removing it simplifies the dialog without losing any functionality. --- .../src/components/dialog-address-comments.tsx | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 8a8764d1fcdd..5e8459d603b0 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -4,7 +4,6 @@ import { useDialog } from "@opencode-ai/ui/context/dialog" import { Dialog } from "@opencode-ai/ui/dialog" import { Icon } from "@opencode-ai/ui/icon" import { Spinner } from "@opencode-ai/ui/spinner" -import { TextField } from "@opencode-ai/ui/text-field" import { createMemo, For, onMount, Show } from "solid-js" import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" @@ -30,7 +29,6 @@ export function AddressCommentsDialog() { error: undefined as string | undefined, threads: [] as ReviewThread[], selected: {} as Record, - instructions: "", }) onMount(async () => { @@ -114,10 +112,6 @@ export function AddressCommentsDialog() { text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="" -F in_reply_to=\`\n` text += `4. Do NOT merge, rebase, or force-push\n` - if (store.instructions.trim()) { - text += `\n${store.instructions.trim()}\n` - } - dialog.close() requestAnimationFrame(() => { prompt.set([ @@ -279,16 +273,6 @@ export function AddressCommentsDialog() {
- {/* Additional instructions */} -
- - setStore("instructions", e.currentTarget.value)} - placeholder={language.t("pr.comments.instructions.placeholder")} - /> -
From e9396c089a49061678fcc2896c5f9510abd99124 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 13:59:59 -0500 Subject: [PATCH 10/46] fix(ui): pass class prop through to Checkbox root element The class prop was being split out but not forwarded to the Kobalte root, preventing consumers from applying custom styles. --- packages/ui/src/components/checkbox.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/checkbox.tsx b/packages/ui/src/components/checkbox.tsx index 7187e4ac300b..51323e775286 100644 --- a/packages/ui/src/components/checkbox.tsx +++ b/packages/ui/src/components/checkbox.tsx @@ -11,7 +11,7 @@ export interface CheckboxProps extends ParentProps + From 94f0a991fcc149fddd64385a7deca1630201a4a2 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 14:00:46 -0500 Subject: [PATCH 11/46] feat(pr): add authorIsBot field to review comments Query __typename from GitHub GraphQL to detect bot authors and expose as a boolean on ReviewComment, replacing the fragile string-based check. --- packages/opencode/src/project/pr-comments.ts | 6 ++++-- packages/sdk/js/src/v2/gen/types.gen.ts | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts index 3867411ef18d..c64d78d5338f 100644 --- a/packages/opencode/src/project/pr-comments.ts +++ b/packages/opencode/src/project/pr-comments.ts @@ -12,6 +12,7 @@ export namespace PrComments { .object({ id: z.number(), author: z.string(), + authorIsBot: z.boolean(), body: z.string(), path: z.string(), line: z.number().nullable(), @@ -54,7 +55,7 @@ export namespace PrComments { comments: { nodes: Array<{ databaseId: number - author: { login: string } + author: { login: string; __typename?: string } body: string path: string line: number | null @@ -70,7 +71,7 @@ export namespace PrComments { } function buildQuery(owner: string, name: string, prNumber: number, cursor: string | null): string { - return `query($owner: String!, $name: String!, $prNumber: Int!, $cursor: String) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 50) { nodes { databaseId author { login } body path line: originalLine diffHunk } } } } } } }` + return `query($owner: String!, $name: String!, $prNumber: Int!, $cursor: String) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 50) { nodes { databaseId author { login __typename } body path line: originalLine diffHunk } } } } } } }` } export async function fetch(): Promise { @@ -153,6 +154,7 @@ export namespace PrComments { comments: (node.comments.nodes ?? []).map((c) => ({ id: c.databaseId, author: c.author?.login ?? "unknown", + authorIsBot: c.author?.__typename === "Bot", body: c.body ?? "", path: c.path ?? "", line: c.line ?? null, diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 019c13cd808d..8717cf85143e 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -1915,6 +1915,7 @@ export type PrCreateInput = { export type ReviewComment = { id: number author: string + authorIsBot: boolean body: string path: string line: number | null From 387468326c9e719b838b8f42692f2ca3f32bbf0f Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 14:34:37 -0500 Subject: [PATCH 12/46] fix(pr): fix branch deletion, dead code, and race conditions - Fix merge branch deletion: remove no-op `delete_branch` API field, call `deleteBranch()` after successful merge instead - Guard `handleSubmit` against undefined PR/repo to prevent "PR #undefined" - Remove unused params from `buildQuery()` in pr-comments - Throw on unparseable PR number in `create()` instead of returning 0 - Remove dead `UPSTREAM_MISSING` error code - Remove dead "no pull requests found" string check (stderr, not stdout) - Add refresh mutex to prevent interleaved `refreshLocal`/`refreshFull` --- .../components/dialog-address-comments.tsx | 9 ++- packages/opencode/src/project/pr-comments.ts | 4 +- packages/opencode/src/project/pr.ts | 24 +++++--- packages/opencode/src/project/vcs.ts | 58 +++++++++++-------- 4 files changed, 58 insertions(+), 37 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 5e8459d603b0..558645ffd0f4 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -78,10 +78,13 @@ export function AddressCommentsDialog() { } const handleSubmit = () => { - const prNumber = pr()?.number + const currentPr = pr() const repo = github()?.repo - const owner = repo?.owner ?? "" - const repoName = repo?.name ?? "" + if (!currentPr || !repo) return + + const prNumber = currentPr.number + const owner = repo.owner + const repoName = repo.name const selectedThreads = store.threads.filter((t) => store.selected[t.id]) diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts index c64d78d5338f..0cfd83e9b9b2 100644 --- a/packages/opencode/src/project/pr-comments.ts +++ b/packages/opencode/src/project/pr-comments.ts @@ -70,7 +70,7 @@ export namespace PrComments { errors?: Array<{ message?: string }> } - function buildQuery(owner: string, name: string, prNumber: number, cursor: string | null): string { + function buildQuery(): string { return `query($owner: String!, $name: String!, $prNumber: Int!, $cursor: String) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 50) { nodes { databaseId author { login __typename } body path line: originalLine diffHunk } } } } } } }` } @@ -91,7 +91,7 @@ export namespace PrComments { let cursor: string | null = null do { - const query = buildQuery(owner, name, prNumber, cursor) + const query = buildQuery() const args = [ "gh", "api", diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index ed4a6812c68d..4116d9377362 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -12,7 +12,6 @@ export namespace PR { "GH_NOT_AUTHENTICATED", "NO_REPO", "NO_PR", - "UPSTREAM_MISSING", "CREATE_FAILED", "MERGE_FAILED", "DELETE_BRANCH_FAILED", @@ -94,7 +93,7 @@ export namespace PR { .cwd(cwd) .text() .catch(() => "") - if (!result.trim() || result.includes("no pull requests found")) { + if (!result.trim()) { return undefined } try { @@ -239,8 +238,11 @@ export namespace PR { const prUrl = result.trim().split("\n").pop() ?? "" const numberMatch = prUrl.match(/\/pull\/(\d+)/) + if (!numberMatch) { + throw new PrError("CREATE_FAILED", "Pull request was created but could not determine PR number from output") + } return { - number: numberMatch ? parseInt(numberMatch[1], 10) : 0, + number: parseInt(numberMatch[1], 10), url: prUrl, title: input.title, state: "OPEN", @@ -280,10 +282,6 @@ export namespace PR { `merge_method=${mergeMethod}`, ] - if (input.deleteBranch !== false) { - args.push("-f", "delete_branch=true") - } - const cmd = await $`${args}` .quiet() .nothrow() @@ -306,6 +304,18 @@ export namespace PR { } await Vcs.refresh() + + if (input.deleteBranch !== false) { + const branchToDelete = currentPr.headRefName + if (branchToDelete) { + try { + await deleteBranch({ branch: branchToDelete }) + } catch (e) { + log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) + } + } + } + const updated = await Vcs.info() return updated.pr ?? { ...currentPr, state: "MERGED" } } diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index f2568862f2db..d12da6b63bfc 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -290,38 +290,46 @@ export namespace Vcs { }) } + let refreshLock: Promise = Promise.resolve() + const refreshLocal = async () => { - const local = await fetchLocalInfo() - const branchChanged = local.branch !== current.branch - current = { ...current, ...local } - if (branchChanged) { - Bus.publish(Event.BranchUpdated, { branch: local.branch }) - // Branch changed — also refresh remote info - if (local.branch) { - const remote = await fetchRemoteInfo(local.branch) - current = { ...current, ...remote } + refreshLock = refreshLock.then(async () => { + const local = await fetchLocalInfo() + const branchChanged = local.branch !== current.branch + current = { ...current, ...local } + if (branchChanged) { + Bus.publish(Event.BranchUpdated, { branch: local.branch }) + // Branch changed — also refresh remote info + if (local.branch) { + const remote = await fetchRemoteInfo(local.branch) + current = { ...current, ...remote } + } } - } - publish() + publish() + }) + await refreshLock } const refreshFull = async () => { - const next = await fetchFullInfo() - const branchChanged = next.branch !== current.branch - const prChanged = next.pr?.number !== current.pr?.number - current = next - if (branchChanged) { - Bus.publish(Event.BranchUpdated, { branch: next.branch }) - } + refreshLock = refreshLock.then(async () => { + const next = await fetchFullInfo() + const branchChanged = next.branch !== current.branch + const prChanged = next.pr?.number !== current.pr?.number + current = next + if (branchChanged) { + Bus.publish(Event.BranchUpdated, { branch: next.branch }) + } - const hasPr = !!next.pr - if (hasPr !== hasActivePr || prChanged) { - hasActivePr = hasPr - pollIntervalMs = hasActivePr ? POLL_INTERVAL_MS : POLL_INTERVAL_MS * POLL_INTERVAL_NO_PR_MULTIPLIER - restartPollTimer() - } + const hasPr = !!next.pr + if (hasPr !== hasActivePr || prChanged) { + hasActivePr = hasPr + pollIntervalMs = hasActivePr ? POLL_INTERVAL_MS : POLL_INTERVAL_MS * POLL_INTERVAL_NO_PR_MULTIPLIER + restartPollTimer() + } - publish() + publish() + }) + await refreshLock } const unsubscribeWatcher = Bus.subscribe(FileWatcher.Event.Updated, async (evt) => { From b5e022789a52aa5a5edadda7f23b6983fbdc6300 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 15:04:21 -0500 Subject: [PATCH 13/46] update(ui): mute deselected cards in address-comments dialog --- packages/app/src/components/dialog-address-comments.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 558645ffd0f4..38c54017f054 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -181,10 +181,10 @@ export function AddressCommentsDialog() { return (
toggleThread(thread.id)} > From a8f24bfbabebf8cf94cb84154886c92420276b3c Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 15:13:21 -0500 Subject: [PATCH 14/46] fix(pr): prevent PR creation with uncommitted changes Disable the Push & Create button when there are uncommitted changes, auto-expand the commit input when the dialog opens with dirty files, and show a helper text to guide users to commit first. --- .../app/src/components/dialog-create-pr.tsx | 20 +++++++++++++++++-- packages/app/src/i18n/en.ts | 3 ++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/dialog-create-pr.tsx b/packages/app/src/components/dialog-create-pr.tsx index 0a18e5d92a02..7a7a1b9d4bfd 100644 --- a/packages/app/src/components/dialog-create-pr.tsx +++ b/packages/app/src/components/dialog-create-pr.tsx @@ -57,6 +57,12 @@ export function CreatePrDialog() { const [titleEdited, setTitleEdited] = createSignal(false) const [baseEdited, setBaseEdited] = createSignal(false) + createEffect(() => { + if ((dirty() ?? 0) > 0) { + setStore("showCommitInput", true) + } + }) + createEffect(() => { const t = branchTitle() if (t && !titleEdited()) { @@ -123,7 +129,10 @@ export function CreatePrDialog() { dialog.close() } catch (e: unknown) { if (import.meta.env.DEV) console.error("PR creation error:", e) - setStore("error", resolveApiErrorMessage(e, "Failed to create PR", (k) => language.t(k as Parameters[0]))) + setStore( + "error", + resolveApiErrorMessage(e, "Failed to create PR", (k) => language.t(k as Parameters[0])), + ) } finally { setStore("submitting", false) } @@ -239,7 +248,11 @@ export function CreatePrDialog() { -
+ 0 && !store.showCommitInput}> +

{language.t("pr.commit.requiredHint")}

+
) diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index a5e6659b5bbf..50d2b9868f39 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -119,6 +119,7 @@ export const dict = { "pr.commit.submit": "Commit", "pr.commit.committing": "Committing...", "pr.commit.success": "Changes committed", + "pr.commit.requiredHint": "Commit your changes before creating a PR", "pr.comments.title": "Address Review Comments", "pr.comments.count": "{{count}} unresolved comments", "pr.comments.count.one": "1 unresolved comment", @@ -143,7 +144,7 @@ export const dict = { "pr.menu.merge": "Merge Pull Request", "pr.menu.delete_branch": "Delete Branch", "pr.merge.title": "Merge Pull Request", -"pr.merge.strategy": "Merge Strategy", + "pr.merge.strategy": "Merge Strategy", "pr.merge.strategy.squash": "Squash and merge", "pr.merge.strategy.merge": "Create a merge commit", "pr.merge.strategy.rebase": "Rebase and merge", From af5de6d9a7453f536e0a9b7e262c3b896cf3f89f Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 16:18:23 -0500 Subject: [PATCH 15/46] feat(pr): add mark as ready for review action - Add backend PR.ready() function and API endpoint - Add dropdown menu item for draft PRs to mark as ready - Show loading spinner on dropdown trigger while action is in progress - Add success/error toast notifications --- packages/app/src/components/pr-button.tsx | 44 +++++++++++++++++++++- packages/app/src/i18n/en.ts | 3 ++ packages/opencode/src/project/pr.ts | 36 ++++++++++++++++++ packages/opencode/src/server/routes/vcs.ts | 32 ++++++++++++++++ packages/sdk/js/src/v2/gen/sdk.gen.ts | 40 ++++++++++++++++++++ packages/sdk/js/src/v2/gen/types.gen.ts | 32 ++++++++++++++++ 6 files changed, 185 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 3e2996902d39..132c03ba1cbb 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -5,7 +5,7 @@ import { IconButton } from "@opencode-ai/ui/icon-button" import { Tooltip } from "@opencode-ai/ui/tooltip" import { showToast } from "@opencode-ai/ui/toast" import { useDialog } from "@opencode-ai/ui/context/dialog" -import { createMemo, Show } from "solid-js" +import { createMemo, createSignal, Show } from "solid-js" import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { usePlatform } from "@/context/platform" @@ -26,6 +26,7 @@ export function PrButton() { const dialog = useDialog() const [menu, setMenu] = createStore({ open: false }) + const [readyLoading, setReadyLoading] = createSignal(false) const vcs = createMemo(() => sync.data.vcs) const pr = createMemo(() => vcs()?.pr) @@ -177,6 +178,29 @@ export function PrButton() { } } + const handleMarkReady = () => { + setReadyLoading(true) + sdk.client.vcs.pr + .ready({ directory: sdk.directory }) + .then(() => { + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.ready"), + }) + }) + .catch(() => { + showToast({ + variant: "error", + icon: "circle-x", + title: language.t("pr.error.ready_failed"), + }) + }) + .finally(() => { + setReadyLoading(false) + }) + } + return (
@@ -228,7 +252,12 @@ export function PrButton() { class="rounded-none h-full w-[24px] p-0 border-none shadow-none data-[expanded]:bg-surface-raised-base-active relative group" aria-label={language.t("common.moreOptions")} > - + } + > + + @@ -261,6 +290,17 @@ export function PrButton() { + {/* Mark as ready — only for draft PRs */} + + + + + + {language.t("pr.menu.ready")} + + + + {/* Merge — only for open, non-draft PRs */} diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 50d2b9868f39..c164a37a58ac 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -142,6 +142,7 @@ export const dict = { "pr.error.no_pr": "No pull request found", "pr.error.comments_fetch_failed": "Failed to fetch review comments", "pr.menu.merge": "Merge Pull Request", + "pr.menu.ready": "Mark as ready for review", "pr.menu.delete_branch": "Delete Branch", "pr.merge.title": "Merge Pull Request", "pr.merge.strategy": "Merge Strategy", @@ -154,8 +155,10 @@ export const dict = { "pr.merge.conflict": "This branch has conflicts that must be resolved", "pr.merge.checks_failing": "Some checks are failing", "pr.toast.merged": "Pull request merged", + "pr.toast.ready": "Pull request marked as ready for review", "pr.toast.branch_deleted": "Branch deleted", "pr.error.merge_failed": "Failed to merge pull request", + "pr.error.ready_failed": "Failed to mark PR as ready", "pr.error.delete_branch_failed": "Failed to delete branch", "pr.tooltip.not_authenticated": "Run `gh auth login` to enable", "pr.tooltip.not_local": "PR actions require local server", diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 4116d9377362..f8f8a1270793 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -16,6 +16,7 @@ export namespace PR { "MERGE_FAILED", "DELETE_BRANCH_FAILED", "COMMENTS_FETCH_FAILED", + "READY_FAILED", ]) export type ErrorCode = z.infer @@ -54,6 +55,9 @@ export namespace PR { .meta({ ref: "PrDeleteBranchInput" }) export type DeleteBranchInput = z.infer + export const ReadyInput = z.object({}).meta({ ref: "PrReadyInput" }) + export type ReadyInput = z.infer + export const PrErrorResponse = z .object({ code: ErrorCode, @@ -255,6 +259,38 @@ export namespace PR { } } + export async function ready(_input: ReadyInput): Promise { + const { info } = await ensureGithub() + const cwd = Instance.worktree + + const currentPr = await get() + if (!currentPr) { + throw new PrError("NO_PR", "No pull request found for the current branch") + } + + if (!currentPr.isDraft) { + return currentPr + } + + const cmd = await $`gh pr ready` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + + const result = cmd.stdout.toString() + const errorOut = cmd.stderr.toString() + + if (cmd.exitCode !== 0) { + log.error("pr ready failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) + throw new PrError("READY_FAILED", errorOut.trim() || result.trim() || "Failed to mark PR as ready") + } + + await Vcs.refresh() + const updated = await Vcs.info() + return updated.pr ?? { ...currentPr, isDraft: false } + } + export async function merge(input: MergeInput): Promise { const { info } = await ensureGithub() const cwd = Instance.worktree diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index 387069ef0225..d6322adbc91e 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -177,6 +177,38 @@ export const VcsRoutes = lazy(() => } }, ) + .post( + "/pr/ready", + describeRoute({ + summary: "Mark PR as ready", + description: "Mark a draft pull request as ready for review.", + operationId: "vcs.pr.ready", + responses: { + 200: { + description: "PR marked as ready", + content: { + "application/json": { + schema: resolver(Vcs.PrInfo), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", PR.ReadyInput), + async (c) => { + try { + const input = c.req.valid("json") + const pr = await PR.ready(input) + return c.json(pr) + } catch (e) { + if (e instanceof PR.PrError) { + return c.json({ code: e.code, message: e.message }, { status: 400 }) + } + throw e + } + }, + ) .post( "/pr/delete-branch", describeRoute({ diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index cd0e0d59a9fa..76c4fe6fc4a2 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -90,6 +90,7 @@ import type { ProviderOauthAuthorizeResponses, ProviderOauthCallbackErrors, ProviderOauthCallbackResponses, + PrReadyInput, PtyConnectErrors, PtyConnectResponses, PtyCreateErrors, @@ -189,6 +190,8 @@ import type { VcsPrGetResponses, VcsPrMergeErrors, VcsPrMergeResponses, + VcsPrReadyErrors, + VcsPrReadyResponses, WorktreeCreateErrors, WorktreeCreateInput, WorktreeCreateResponses, @@ -3028,6 +3031,43 @@ export class Pr extends HeyApiClient { }) } + /** + * Mark PR as ready + * + * Mark a draft pull request as ready for review. + */ + public ready( + parameters?: { + directory?: string + workspace?: string + prReadyInput?: PrReadyInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "prReadyInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/pr/ready", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } + /** * Delete branch * diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 8717cf85143e..4dd5ae7187ff 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -1941,6 +1941,10 @@ export type PrMergeInput = { deleteBranch?: boolean } +export type PrReadyInput = { + [key: string]: unknown +} + export type PrDeleteBranchInput = { branch: string } @@ -4483,6 +4487,34 @@ export type VcsPrMergeResponses = { export type VcsPrMergeResponse = VcsPrMergeResponses[keyof VcsPrMergeResponses] +export type VcsPrReadyData = { + body?: PrReadyInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr/ready" +} + +export type VcsPrReadyErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrReadyError = VcsPrReadyErrors[keyof VcsPrReadyErrors] + +export type VcsPrReadyResponses = { + /** + * PR marked as ready + */ + 200: PrInfo +} + +export type VcsPrReadyResponse = VcsPrReadyResponses[keyof VcsPrReadyResponses] + export type VcsPrDeleteBranchData = { body?: PrDeleteBranchInput path?: never From 95c783b1b8377ba26984c8f0c63b3994d109d045 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 16:56:51 -0500 Subject: [PATCH 16/46] Switch to build mode when pre-filling prompt from addressed comments dialog --- .../app/src/components/dialog-address-comments.tsx | 10 ++++++++-- packages/app/src/i18n/en.ts | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 38c54017f054..ba743efd7180 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -7,6 +7,7 @@ import { Spinner } from "@opencode-ai/ui/spinner" import { createMemo, For, onMount, Show } from "solid-js" import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" +import { useLocal } from "@/context/local" import { usePrompt } from "@/context/prompt" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" @@ -17,6 +18,7 @@ export function AddressCommentsDialog() { const dialog = useDialog() const sdk = useSDK() const sync = useSync() + const local = useLocal() const prompt = usePrompt() const language = useLanguage() @@ -115,6 +117,9 @@ export function AddressCommentsDialog() { text += ` \`gh api repos/${owner}/${repoName}/pulls/${prNumber}/comments --method POST -f body="" -F in_reply_to=\`\n` text += `4. Do NOT merge, rebase, or force-push\n` + if (local.agent.current()?.name !== "build") { + local.agent.set("build") + } dialog.close() requestAnimationFrame(() => { prompt.set([ @@ -275,7 +280,6 @@ export function AddressCommentsDialog() { }}
- @@ -298,7 +302,9 @@ export function AddressCommentsDialog() { 0 && !store.error}>
diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index c164a37a58ac..5f95e6ebcdd6 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -129,6 +129,7 @@ export const dict = { "pr.comments.instructions": "Additional instructions", "pr.comments.instructions.placeholder": "Any additional instructions for the agent...", "pr.comments.submit": "Prefill Prompt", + "pr.comments.submitWithSwitch": "Prefill Prompt & Switch to Build", "pr.comments.replies.one": "1 reply", "pr.comments.replies.count": "{{count}} replies", "pr.comments.select.all": "Select all", From 944e6f11f38bdbd1ab1dde21c81008997f8571b7 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 17:46:11 -0500 Subject: [PATCH 17/46] tweak: change the button label to let user know mode will switch --- packages/app/src/components/pr-button.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 132c03ba1cbb..3f9bcf8d9645 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -66,6 +66,13 @@ export function PrButton() { const isMerged = createMemo(() => pr()?.state === "MERGED") const isClosed = createMemo(() => pr()?.state === "CLOSED") + const remoteBranchExists = createMemo(() => { + const branchName = pr()?.headRefName + const branches = vcs()?.branches + if (!branchName || !branches) return true + return branches.includes(branchName) + }) + const containerStyle = createMemo(() => getPrButtonContainerStyle(pr())) const dividerStyle = createMemo(() => getPrButtonDividerStyle(pr())) @@ -350,8 +357,8 @@ export function PrButton() { - {/* Delete branch — only for merged PRs */} - + {/* Delete branch — only for merged PRs with branch still existing */} + { From cd4cfa38b6442d2894109730eb93cd0c4113b349 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 18:29:18 -0500 Subject: [PATCH 18/46] feat(pr): redesign delete branch dialog with critical styling - Switch warning box from yellow/warning to red/critical tokens - Move branch name inside warning box as cohesive info block - Use fit dialog to eliminate excess bottom whitespace - Use proper theme tokens for destructive button styling - Remove local branch cleanup from backend (user is typically on the branch) - Simplify i18n to single warning message about remote-only deletion --- .../src/components/dialog-delete-branch.tsx | 83 +++++++++++++++++++ packages/app/src/i18n/en.ts | 5 ++ packages/opencode/src/project/pr.ts | 17 ---- 3 files changed, 88 insertions(+), 17 deletions(-) create mode 100644 packages/app/src/components/dialog-delete-branch.tsx diff --git a/packages/app/src/components/dialog-delete-branch.tsx b/packages/app/src/components/dialog-delete-branch.tsx new file mode 100644 index 000000000000..b4f46f561614 --- /dev/null +++ b/packages/app/src/components/dialog-delete-branch.tsx @@ -0,0 +1,83 @@ +import { Button } from "@opencode-ai/ui/button" +import { useDialog } from "@opencode-ai/ui/context/dialog" +import { Dialog } from "@opencode-ai/ui/dialog" +import { Icon } from "@opencode-ai/ui/icon" +import { showToast } from "@opencode-ai/ui/toast" +import { createMemo } from "solid-js" +import { createStore } from "solid-js/store" +import { useLanguage } from "@/context/language" +import { useSDK } from "@/context/sdk" +import { useSync } from "@/context/sync" + +export function DeleteBranchDialog() { + const dialog = useDialog() + const sdk = useSDK() + const sync = useSync() + const language = useLanguage() + + const pr = createMemo(() => sync.data.vcs?.pr) + + const [store, setStore] = createStore({ + submitting: false, + }) + + const handleDelete = async () => { + const p = pr() + if (!p?.headRefName || store.submitting) return + + setStore("submitting", true) + + try { + await sdk.client.vcs.pr.deleteBranch({ + directory: sdk.directory, + prDeleteBranchInput: { branch: p.headRefName }, + }) + + showToast({ + variant: "success", + icon: "circle-check", + title: language.t("pr.toast.branch_deleted"), + }) + dialog.close() + } catch { + showToast({ + variant: "error", + icon: "circle-x", + title: language.t("pr.error.delete_branch_failed"), + }) + } finally { + setStore("submitting", false) + } + } + + return ( + +
+
+ +
+
+ + {pr()?.headRefName} +
+ {language.t("pr.delete.warning")} +
+
+ +
+ + +
+
+ + ) +} diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 5f95e6ebcdd6..5511b4e7e900 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -161,6 +161,11 @@ export const dict = { "pr.error.merge_failed": "Failed to merge pull request", "pr.error.ready_failed": "Failed to mark PR as ready", "pr.error.delete_branch_failed": "Failed to delete branch", + "pr.delete.title": "Delete Branch", + "pr.delete.warning": + "This will permanently delete the remote branch. It can be restored from the pull request page on GitHub.", + "pr.delete.submit": "Delete Branch", + "pr.delete.submitting": "Deleting...", "pr.tooltip.not_authenticated": "Run `gh auth login` to enable", "pr.tooltip.not_local": "PR actions require local server", diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index f8f8a1270793..f5dde143b25c 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -376,23 +376,6 @@ export namespace PR { } } - // Clean up local branch if it exists and isn't the current one - const currentBranch = await $`git rev-parse --abbrev-ref HEAD` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch((_e: unknown) => "") - if (currentBranch.trim() !== input.branch) { - await $`git branch -D ${input.branch}` - .quiet() - .nothrow() - .cwd(cwd) - .catch(() => { - // Local branch cleanup is best-effort - }) - } - await Vcs.refresh() } } From 4abab9e77809968d76a04fee230f11d1118d46e8 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 18:30:47 -0500 Subject: [PATCH 19/46] refactor(pr): wire delete branch action to confirmation dialog Move inline delete branch handler in pr-button to open the new DeleteBranchDialog instead of executing the deletion directly. --- packages/app/src/components/pr-button.tsx | 25 ++++------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 3f9bcf8d9645..0f15fa38ebc1 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -15,6 +15,7 @@ import { useSync } from "@/context/sync" import { getPrButtonContainerStyle, getPrButtonDividerStyle } from "@/utils/pr-style" import { CreatePrDialog } from "./dialog-create-pr" import { MergePrDialog } from "./dialog-merge-pr" +import { DeleteBranchDialog } from "./dialog-delete-branch" import { AddressCommentsDialog } from "./dialog-address-comments" export function PrButton() { @@ -163,26 +164,8 @@ export function PrButton() { dialog.show(() => ) } - const handleDeleteBranch = async () => { - const p = pr() - if (!p?.headRefName) return - try { - await sdk.client.vcs.pr.deleteBranch({ - directory: sdk.directory, - prDeleteBranchInput: { branch: p.headRefName }, - }) - showToast({ - variant: "success", - icon: "circle-check", - title: language.t("pr.toast.branch_deleted"), - }) - } catch { - showToast({ - variant: "error", - icon: "circle-x", - title: language.t("pr.error.delete_branch_failed"), - }) - } + const openDeleteDialog = () => { + dialog.show(() => ) } const handleMarkReady = () => { @@ -363,7 +346,7 @@ export function PrButton() { disabled={!canMutate()} onSelect={() => { setMenu("open", false) - handleDeleteBranch() + openDeleteDialog() }} > From 02b0cd12286328517b699f5d3c444feff3e0dfa3 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 18:32:41 -0500 Subject: [PATCH 20/46] fix(ui): ensure dropdown menu items fill full width --- packages/ui/src/components/dropdown-menu.css | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ui/src/components/dropdown-menu.css b/packages/ui/src/components/dropdown-menu.css index edc2eee9a400..01ce51f60ab6 100644 --- a/packages/ui/src/components/dropdown-menu.css +++ b/packages/ui/src/components/dropdown-menu.css @@ -47,6 +47,7 @@ line-height: var(--line-height-large); letter-spacing: var(--letter-spacing-normal); color: var(--text-strong); + width: 100%; &[data-highlighted] { background: var(--surface-raised-base-hover); From a4130a540d6044bcbb1fa9e0ac3c9fe61c302b0e Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:07:44 -0500 Subject: [PATCH 21/46] feat(app): display a noty baddge on PR tag when associated PR has unresolved comments --- .../app/src/pages/layout/sidebar-workspace.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/app/src/pages/layout/sidebar-workspace.tsx b/packages/app/src/pages/layout/sidebar-workspace.tsx index a6c6aee8e129..7b3db117f313 100644 --- a/packages/app/src/pages/layout/sidebar-workspace.tsx +++ b/packages/app/src/pages/layout/sidebar-workspace.tsx @@ -137,12 +137,18 @@ const WorkspaceHeader = (props: { {(currentPr) => { const pillStyle = () => prPillStyle(currentPr()) + const hasUnresolvedComments = () => (currentPr().unresolvedCommentCount ?? 0) > 0 return ( - - PR #{currentPr().number} - +
+ + PR #{currentPr().number} + + +
+ +
) }} From ff96edee0511cc04c2aec482b4acda49cfe76e87 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:22:14 -0500 Subject: [PATCH 22/46] feat(pr): add "ask agent to fix conflicts" action to merge dialog When a PR has merge conflicts, the merge dialog now shows an inline button that pre-fills the prompt with conflict resolution instructions and switches to build mode, matching the address-comments pattern. --- .../app/src/components/dialog-merge-pr.tsx | 41 ++++++++++++++++++- packages/app/src/i18n/en.ts | 1 + 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/dialog-merge-pr.tsx b/packages/app/src/components/dialog-merge-pr.tsx index ff6eb1fab749..b7df727abc0d 100644 --- a/packages/app/src/components/dialog-merge-pr.tsx +++ b/packages/app/src/components/dialog-merge-pr.tsx @@ -8,6 +8,8 @@ import { showToast } from "@opencode-ai/ui/toast" import { createMemo, Show } from "solid-js" import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" +import { useLocal } from "@/context/local" +import { usePrompt } from "@/context/prompt" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" import { resolveApiErrorMessage } from "@/utils/pr-errors" @@ -20,6 +22,8 @@ export function MergePrDialog() { const dialog = useDialog() const sdk = useSDK() const sync = useSync() + const local = useLocal() + const prompt = usePrompt() const language = useLanguage() const pr = createMemo(() => sync.data.vcs?.pr) @@ -39,6 +43,36 @@ export function MergePrDialog() { return language.t(key as Parameters[0]) } + const handleFixConflicts = () => { + const currentPr = pr() + if (!currentPr) return + + let text = `Fix the merge conflicts on this branch.\n\n` + text += `## Context\n\n` + text += `Branch \`${currentPr.headRefName}\` has conflicts with \`${currentPr.baseRefName}\`.\n\n` + text += `## Instructions\n\n` + text += `1. Fetch the latest changes from the base branch \`${currentPr.baseRefName}\`\n` + text += `2. Merge \`${currentPr.baseRefName}\` into \`${currentPr.headRefName}\` and resolve all conflicts\n` + text += `3. Commit the resolved changes\n` + text += `4. \`git push\`\n` + text += `5. Do NOT force-push or rewrite history\n` + + if (local.agent.current()?.name !== "build") { + local.agent.set("build") + } + dialog.close() + requestAnimationFrame(() => { + prompt.set([ + { + type: "text", + content: text, + start: 0, + end: text.length, + }, + ]) + }) + } + const handleSubmit = async () => { if (store.submitting || hasConflict()) return setStore("submitting", true) @@ -80,9 +114,12 @@ export function MergePrDialog() { {/* Conflict warning */} -
- +
+ {language.t("pr.merge.conflict")} +
diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 5511b4e7e900..fadf0f616acc 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -154,6 +154,7 @@ export const dict = { "pr.merge.submit": "Merge", "pr.merge.submitting": "Merging...", "pr.merge.conflict": "This branch has conflicts that must be resolved", + "pr.merge.conflict.fix": "Ask agent to fix conflicts", "pr.merge.checks_failing": "Some checks are failing", "pr.toast.merged": "Pull request merged", "pr.toast.ready": "Pull request marked as ready for review", From fa9483f06bf3d22a08d4a878e522dcc36a953ab6 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:31:43 -0500 Subject: [PATCH 23/46] fix(app): use else if in applyOptimisticAdd to avoid stale variable check --- packages/app/src/context/sync.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/src/context/sync.tsx b/packages/app/src/context/sync.tsx index 0f2008723456..62eece0cf20b 100644 --- a/packages/app/src/context/sync.tsx +++ b/packages/app/src/context/sync.tsx @@ -117,11 +117,11 @@ export function mergeOptimisticPage(page: MessagePage, items: OptimisticItem[]) export function applyOptimisticAdd(draft: OptimisticStore, input: OptimisticAddInput) { const messages = draft.message[input.sessionID] - if (messages) { + if (!messages) { + draft.message[input.sessionID] = [input.message] + } else if (messages) { const result = Binary.search(messages, input.message.id, (m) => m.id) messages.splice(result.index, 0, input.message) - } else { - draft.message[input.sessionID] = [input.message] } draft.part[input.message.id] = sortParts(input.parts) } From 0b0c3198bd9a3c42b8061ba6e3bddd321d8f28ed Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:35:49 -0500 Subject: [PATCH 24/46] fix: remove dead window code from backend vcs module --- packages/opencode/src/project/vcs.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index d12da6b63bfc..b371683155a5 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -347,12 +347,6 @@ export namespace Vcs { restartPollTimer() - let onFocus: (() => void) | undefined - if (typeof window !== "undefined") { - onFocus = () => refreshFull() - window.addEventListener("focus", onFocus) - } - return { info: async () => current, refresh: refreshFull, @@ -361,7 +355,6 @@ export namespace Vcs { if (localDebounce) clearTimeout(localDebounce) if (refDebounce) clearTimeout(refDebounce) if (pollTimer) clearInterval(pollTimer) - if (onFocus) window.removeEventListener("focus", onFocus) }, } }, From d1ca5e4fa377b27348d4756072a750d507e51cf8 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:35:59 -0500 Subject: [PATCH 25/46] refactor(app): simplify applyOptimisticAdd with else instead of else if --- packages/app/src/context/sync.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/context/sync.tsx b/packages/app/src/context/sync.tsx index 62eece0cf20b..372b2ee44f54 100644 --- a/packages/app/src/context/sync.tsx +++ b/packages/app/src/context/sync.tsx @@ -119,7 +119,7 @@ export function applyOptimisticAdd(draft: OptimisticStore, input: OptimisticAddI const messages = draft.message[input.sessionID] if (!messages) { draft.message[input.sessionID] = [input.message] - } else if (messages) { + } else { const result = Binary.search(messages, input.message.id, (m) => m.id) messages.splice(result.index, 0, input.message) } From 568b4b416059a49d7347dc5753c38a1e57b43561 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:40:57 -0500 Subject: [PATCH 26/46] fix(pr): disallow path traversal in branch name regex --- packages/opencode/src/project/pr.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index f5dde143b25c..8876eeac3a66 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -50,7 +50,7 @@ export namespace PR { export const DeleteBranchInput = z .object({ - branch: z.string().regex(/^[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name"), + branch: z.string().regex(/^(?![^/]*\.\.)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name"), }) .meta({ ref: "PrDeleteBranchInput" }) export type DeleteBranchInput = z.infer From 97d45e9849e2faaeb5ad1f8633c34993b9351898 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:43:49 -0500 Subject: [PATCH 27/46] fix(pr): sync commit message to PR title while editing One-way sync from commit message to PR title while the commit input is visible. Stops when user manually edits the PR title. --- packages/app/src/components/dialog-create-pr.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/app/src/components/dialog-create-pr.tsx b/packages/app/src/components/dialog-create-pr.tsx index 7a7a1b9d4bfd..507cae0c9e82 100644 --- a/packages/app/src/components/dialog-create-pr.tsx +++ b/packages/app/src/components/dialog-create-pr.tsx @@ -63,9 +63,15 @@ export function CreatePrDialog() { } }) + createEffect(() => { + if (store.showCommitInput && !titleEdited()) { + setStore("title", store.commitMessage) + } + }) + createEffect(() => { const t = branchTitle() - if (t && !titleEdited()) { + if (t && !titleEdited() && !store.showCommitInput) { setStore("title", t) setStore("commitMessage", t) } From 1f86f35c0efc7f11b4f80c2524bce67fd480642a Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:46:20 -0500 Subject: [PATCH 28/46] fix(vcs): only stage tracked files in commit() --- packages/opencode/src/project/vcs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index b371683155a5..5cba124ab5d6 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -224,7 +224,7 @@ export namespace Vcs { export async function commit(message: string) { const cwd = Instance.worktree - const add = await $`git add -A`.quiet().nothrow().cwd(cwd) + const add = await $`git add -u`.quiet().nothrow().cwd(cwd) if (add.exitCode !== 0) { throw new Error("git add failed: " + add.stderr.toString()) } From 4cb43c30065f49613c9722b1272153732e84caca Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 19:52:34 -0500 Subject: [PATCH 29/46] fix: use pr.url for GitHub comment links to support Enterprise --- packages/app/src/components/dialog-address-comments.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index ba743efd7180..66b84cfd47e8 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -234,9 +234,7 @@ export function AddressCommentsDialog() { const g = github() const p = pr() const href = - g?.repo && p?.number - ? `https://github.com/${g.repo.owner}/${g.repo.name}/pull/${p.number}#discussion_r${comment().id}` - : undefined + g?.repo && p?.url ? `${p.url}#discussion_r${comment().id}` : undefined return ( {(url) => ( From 467f3c335247647892c78be65e8272e6d2e140f2 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 20:05:03 -0500 Subject: [PATCH 30/46] fix(pr): correct delete-branch route description Route said "remote (and local) branch" but only remote deletion is implemented. --- packages/opencode/src/server/routes/vcs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index d6322adbc91e..659f14ba7158 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -213,7 +213,7 @@ export const VcsRoutes = lazy(() => "/pr/delete-branch", describeRoute({ summary: "Delete branch", - description: "Delete a remote (and local) branch after a PR has been merged.", + description: "Delete the remote branch after a PR has been merged.", operationId: "vcs.pr.deleteBranch", responses: { 200: { From 5962ff256b379f07b5804f8664082ea03ff09ac1 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 20:05:08 -0500 Subject: [PATCH 31/46] fix(vcs): use recursive setTimeout for true poll jitter --- packages/opencode/src/project/vcs.ts | 30 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 5cba124ab5d6..347d6789d6da 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -261,22 +261,28 @@ export namespace Vcs { let localDebounce: ReturnType | undefined let refDebounce: ReturnType | undefined - let pollTimer: ReturnType | undefined + let pollTimer: ReturnType | undefined let hasActivePr = !!current.pr let pollIntervalMs = hasActivePr ? POLL_INTERVAL_MS : POLL_INTERVAL_MS * POLL_INTERVAL_NO_PR_MULTIPLIER const restartPollTimer = () => { - if (pollTimer) clearInterval(pollTimer) - pollTimer = setInterval( - async () => { - try { - await refreshFull() - } catch (e) { - log.error("poll refresh failed", { error: e }) - } - }, - pollIntervalMs + Math.random() * POLL_JITTER_MS, - ) + if (pollTimer) clearTimeout(pollTimer) + + const scheduleNext = () => { + pollTimer = setTimeout( + async () => { + try { + await refreshFull() + } catch (e) { + log.error("poll refresh failed", { error: e }) + } + scheduleNext() + }, + pollIntervalMs + Math.random() * POLL_JITTER_MS, + ) + } + + scheduleNext() } const publish = () => { From c2015b9f4f296c1103ef37540ca22a4758eab19e Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 20:07:01 -0500 Subject: [PATCH 32/46] fix(pr): stop swallowing gh errors, make deleteBranch opt-in --- packages/opencode/src/project/pr.ts | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 8876eeac3a66..6bdd79075fe0 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -90,17 +90,16 @@ export namespace PR { export async function fetchForBranch(repo?: { owner: string; name: string }): Promise { const cwd = Instance.worktree - const result = - await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch(() => "") - if (!result.trim()) { - return undefined - } try { + const result = + await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` + .quiet() + .nothrow() + .cwd(cwd) + .text() + if (!result.trim()) { + return undefined + } const parsed = JSON.parse(result) if (!parsed.number) return undefined @@ -341,7 +340,7 @@ export namespace PR { await Vcs.refresh() - if (input.deleteBranch !== false) { + if (input.deleteBranch === true) { const branchToDelete = currentPr.headRefName if (branchToDelete) { try { From 607bb559473d6fdc5da47724134150b935d096c8 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 20:10:14 -0500 Subject: [PATCH 33/46] fix(pr): add log.warn to silent catch blocks in fetchForBranch and fetchUnresolvedCommentCount --- packages/opencode/src/project/pr.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 6bdd79075fe0..35316e9b6ae3 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -83,7 +83,8 @@ export namespace PR { const threads = parsed?.data?.repository?.pullRequest?.reviewThreads?.nodes if (!Array.isArray(threads)) return undefined return threads.filter((t: { isResolved: boolean }) => !t.isResolved).length - } catch { + } catch (e) { + log.warn("fetchUnresolvedCommentCount failed", { error: e }) return undefined } } @@ -176,7 +177,8 @@ export namespace PR { } return pr - } catch { + } catch (e) { + log.warn("fetchForBranch failed", { error: e }) return undefined } } From d9fbc2a5f29ff509feed988099a161e1d9375c43 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 22:41:23 -0500 Subject: [PATCH 34/46] fix(pr): harden validation, merge guard, and HTTP status codes - Reject '..' anywhere in branch name regex (path traversal) - Require non-empty title on CreateInput via .min(1) - Check mergeable status before calling GitHub merge API - Return 404 instead of 400 for NO_PR errors in route handlers --- packages/opencode/src/project/pr.ts | 8 ++++++-- packages/opencode/src/server/routes/vcs.ts | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 35316e9b6ae3..b77e170842d4 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -32,7 +32,7 @@ export namespace PR { export const CreateInput = z .object({ - title: z.string(), + title: z.string().min(1), body: z.string(), base: z.string().optional(), draft: z.boolean().optional(), @@ -50,7 +50,7 @@ export namespace PR { export const DeleteBranchInput = z .object({ - branch: z.string().regex(/^(?![^/]*\.\.)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name"), + branch: z.string().regex(/^(?!.*\.\.)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name"), }) .meta({ ref: "PrDeleteBranchInput" }) export type DeleteBranchInput = z.infer @@ -301,6 +301,10 @@ export namespace PR { throw new PrError("NO_PR", "No pull request found for the current branch") } + if (currentPr.mergeable === "CONFLICTING") { + throw new PrError("MERGE_FAILED", "PR has merge conflicts that must be resolved first") + } + const github = info.github if (!github?.repo) { throw new PrError("MERGE_FAILED", "Unable to determine repository information") diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index 659f14ba7158..a9b22c67c174 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -130,7 +130,7 @@ export const VcsRoutes = lazy(() => }, }, }, - ...errors(400), + ...errors(400, 404), }, }), async (c) => { @@ -139,7 +139,8 @@ export const VcsRoutes = lazy(() => return c.json(data) } catch (e) { if (e instanceof PR.PrError) { - return c.json({ code: e.code, message: e.message }, { status: 400 }) + const status = e.code === "NO_PR" ? 404 : 400 + return c.json({ code: e.code, message: e.message }, { status }) } throw e } @@ -160,7 +161,7 @@ export const VcsRoutes = lazy(() => }, }, }, - ...errors(400), + ...errors(400, 404), }, }), validator("json", PR.MergeInput), @@ -171,7 +172,8 @@ export const VcsRoutes = lazy(() => return c.json(pr) } catch (e) { if (e instanceof PR.PrError) { - return c.json({ code: e.code, message: e.message }, { status: 400 }) + const status = e.code === "NO_PR" ? 404 : 400 + return c.json({ code: e.code, message: e.message }, { status }) } throw e } @@ -192,7 +194,7 @@ export const VcsRoutes = lazy(() => }, }, }, - ...errors(400), + ...errors(400, 404), }, }), validator("json", PR.ReadyInput), @@ -203,7 +205,8 @@ export const VcsRoutes = lazy(() => return c.json(pr) } catch (e) { if (e instanceof PR.PrError) { - return c.json({ code: e.code, message: e.message }, { status: 400 }) + const status = e.code === "NO_PR" ? 404 : 400 + return c.json({ code: e.code, message: e.message }, { status }) } throw e } @@ -224,7 +227,7 @@ export const VcsRoutes = lazy(() => }, }, }, - ...errors(400), + ...errors(400, 404), }, }), validator("json", PR.DeleteBranchInput), @@ -235,7 +238,8 @@ export const VcsRoutes = lazy(() => return c.json({ ok: true }) } catch (e) { if (e instanceof PR.PrError) { - return c.json({ code: e.code, message: e.message }, { status: 400 }) + const status = e.code === "NO_PR" ? 404 : 400 + return c.json({ code: e.code, message: e.message }, { status }) } throw e } From e7f12090edbe6ee63fd3944812218a0d9dee50f7 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 22:43:46 -0500 Subject: [PATCH 35/46] fix(pr): narrow ensureGithub type, fix timer cleanup, add missing event field, sanitize error output - Remove unsafe Required cast in ensureGithub(); use narrowed intersection type instead so repo/host remain optional - Use clearTimeout instead of clearInterval for setTimeout handle - Add missing branches field to vcs.updated event type in reducer - Redact GitHub token prefixes (ghp_, github_pat_, etc.) and truncate subprocess error output before surfacing in PrError messages --- .../app/src/context/global-sync/event-reducer.ts | 1 + packages/opencode/src/project/pr.ts | 16 ++++++++++------ packages/opencode/src/project/vcs.ts | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/app/src/context/global-sync/event-reducer.ts b/packages/app/src/context/global-sync/event-reducer.ts index 8d8fafb8ef58..3b959be64644 100644 --- a/packages/app/src/context/global-sync/event-reducer.ts +++ b/packages/app/src/context/global-sync/event-reducer.ts @@ -272,6 +272,7 @@ export function applyDirectoryEvent(input: { const props = event.properties as { branch?: string defaultBranch?: string + branches?: string[] dirty?: number pr?: VcsInfo["pr"] github?: VcsInfo["github"] diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index b77e170842d4..f17e6668aef6 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -183,6 +183,10 @@ export namespace PR { } } + function sanitizeOutput(output: string): string { + return output.replace(/(ghp_|github_pat_|gho_|ghu_|ghs_|ghr_)[a-zA-Z0-9_]+/g, "").slice(0, 500) + } + async function ensureGithub() { const info = await Vcs.info() const github = info.github @@ -192,7 +196,7 @@ export namespace PR { if (!github.authenticated) { throw new PrError("GH_NOT_AUTHENTICATED", "Run `gh auth login` to authenticate") } - return { info, github: github as Required } + return { info, github: github as Vcs.GithubCapability & { available: true; authenticated: true } } } export async function get(): Promise { @@ -213,7 +217,7 @@ export namespace PR { const errorOutput = push.stderr?.toString().trim() || push.stdout?.toString().trim() || "Failed to push branch automatically" log.error("push failed", { output: errorOutput }) - throw new PrError("CREATE_FAILED", errorOutput) + throw new PrError("CREATE_FAILED", sanitizeOutput(errorOutput)) } const args = ["gh", "pr", "create", "--title", input.title] @@ -232,7 +236,7 @@ export namespace PR { if (cmd.exitCode !== 0 || !result.trim()) { log.error("pr create failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) - throw new PrError("CREATE_FAILED", errorOut.trim() || result.trim() || "Failed to create pull request") + throw new PrError("CREATE_FAILED", sanitizeOutput(errorOut.trim() || result.trim() || "Failed to create pull request")) } await Vcs.refresh() @@ -284,7 +288,7 @@ export namespace PR { if (cmd.exitCode !== 0) { log.error("pr ready failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) - throw new PrError("READY_FAILED", errorOut.trim() || result.trim() || "Failed to mark PR as ready") + throw new PrError("READY_FAILED", sanitizeOutput(errorOut.trim() || result.trim() || "Failed to mark PR as ready")) } await Vcs.refresh() @@ -341,7 +345,7 @@ export namespace PR { } } catch {} log.error("pr merge failed", { stderr: errorOut, stdout: responseText, exitCode: cmd.exitCode }) - throw new PrError("MERGE_FAILED", errorMessage) + throw new PrError("MERGE_FAILED", sanitizeOutput(errorMessage)) } await Vcs.refresh() @@ -377,7 +381,7 @@ export namespace PR { // Ignore "remote ref does not exist" — branch may already be deleted if (!errorOut.includes("remote ref does not exist")) { log.error("delete remote branch failed", { stderr: errorOut }) - throw new PrError("DELETE_BRANCH_FAILED", errorOut || "Failed to delete remote branch") + throw new PrError("DELETE_BRANCH_FAILED", sanitizeOutput(errorOut || "Failed to delete remote branch")) } } diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 347d6789d6da..c37605ee0d85 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -360,7 +360,7 @@ export namespace Vcs { unsubscribeWatcher() if (localDebounce) clearTimeout(localDebounce) if (refDebounce) clearTimeout(refDebounce) - if (pollTimer) clearInterval(pollTimer) + if (pollTimer) clearTimeout(pollTimer) }, } }, From 0731d65896512098dc6cd20a38d5a9406dca1226 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 23:05:38 -0500 Subject: [PATCH 36/46] fix(pr): refactor PrError to NamedError, add gh timeouts, surface branch delete failure - Replace hand-rolled PrError class with NamedError.create() convention - Add PrError to global onError handler; remove per-route try/catch blocks - Wrap all gh CLI subprocess calls with withTimeout (30s default, 60s for create/merge) - Add branchDeleteFailed field to PrInfo so callers know when post-merge cleanup fails --- packages/opencode/src/cli/cmd/pr.ts | 22 +--- packages/opencode/src/project/pr-comments.ts | 21 ++-- packages/opencode/src/project/pr.ts | 112 ++++++++++--------- packages/opencode/src/project/vcs.ts | 43 ++++--- packages/opencode/src/server/routes/vcs.ts | 67 +++-------- packages/opencode/src/server/server.ts | 2 + 6 files changed, 119 insertions(+), 148 deletions(-) diff --git a/packages/opencode/src/cli/cmd/pr.ts b/packages/opencode/src/cli/cmd/pr.ts index 8826fe343ea0..d0ea3ae02295 100644 --- a/packages/opencode/src/cli/cmd/pr.ts +++ b/packages/opencode/src/cli/cmd/pr.ts @@ -3,6 +3,8 @@ import { cmd } from "./cmd" import { Instance } from "@/project/instance" import { Process } from "@/util/process" import { git } from "@/util/git" +import { withTimeout } from "@/util/timeout" +import { $ } from "bun" export const PrCommand = cmd({ command: "pr ", @@ -28,12 +30,7 @@ export const PrCommand = cmd({ UI.println(`Fetching and checking out PR #${prNumber}...`) // Use gh pr checkout with custom branch name - const result = await Process.run( - ["gh", "pr", "checkout", `${prNumber}`, "--branch", localBranchName, "--force"], - { - nothrow: true, - }, - ) + const result = await withTimeout($`gh pr checkout ${prNumber} --branch ${localBranchName} --force`.nothrow(), 30_000) if (result.code !== 0) { UI.error(`Failed to checkout PR #${prNumber}. Make sure you have gh CLI installed and authenticated.`) @@ -41,16 +38,9 @@ export const PrCommand = cmd({ } // Fetch PR info for fork handling and session link detection - const prInfoResult = await Process.text( - [ - "gh", - "pr", - "view", - `${prNumber}`, - "--json", - "headRepository,headRepositoryOwner,isCrossRepository,headRefName,body", - ], - { nothrow: true }, + const prInfoResult = await withTimeout( + $`gh pr view ${prNumber} --json headRepository,headRepositoryOwner,isCrossRepository,headRefName,body`.nothrow(), + 30_000, ) let sessionId: string | undefined diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts index 0cfd83e9b9b2..a08c9719cfc8 100644 --- a/packages/opencode/src/project/pr-comments.ts +++ b/packages/opencode/src/project/pr-comments.ts @@ -1,6 +1,7 @@ import { $ } from "bun" import z from "zod" import { Log } from "@/util/log" +import { withTimeout } from "@/util/timeout" import { Instance } from "./instance" import { Vcs } from "./vcs" import { PR } from "./pr" @@ -77,10 +78,10 @@ export namespace PrComments { export async function fetch(): Promise { const info = await Vcs.info() if (!info.pr) { - throw new PR.PrError("NO_PR", "No pull request found for current branch") + throw new PR.PrError({ code: "NO_PR", message: "No pull request found for current branch" }) } if (!info.github?.authenticated || !info.github.repo) { - throw new PR.PrError("GH_NOT_AUTHENTICATED", "GitHub CLI is not authenticated") + throw new PR.PrError({ code: "GH_NOT_AUTHENTICATED", message: "GitHub CLI is not authenticated" }) } const { owner, name } = info.github.repo @@ -108,22 +109,22 @@ export namespace PrComments { if (cursor) { args.push("-f", `cursor=${cursor}`) } - const cmd = await $`${args}`.quiet().nothrow().cwd(cwd) + const cmd = await withTimeout($`${args}`.quiet().nothrow().cwd(cwd), 30_000) const stdout = cmd.stdout.toString() const stderr = cmd.stderr.toString() if (cmd.exitCode !== 0) { log.error("gh api graphql failed", { exitCode: cmd.exitCode, stderr }) - throw new PR.PrError( - "COMMENTS_FETCH_FAILED", - stderr.trim() || `gh api graphql exited with code ${cmd.exitCode}`, - ) + throw new PR.PrError({ + code: "COMMENTS_FETCH_FAILED", + message: stderr.trim() || `gh api graphql exited with code ${cmd.exitCode}`, + }) } if (!stdout.trim()) { log.error("gh api graphql returned empty response") - throw new PR.PrError("COMMENTS_FETCH_FAILED", "GitHub API returned empty response") + throw new PR.PrError({ code: "COMMENTS_FETCH_FAILED", message: "GitHub API returned empty response" }) } let parsed: Record @@ -131,7 +132,7 @@ export namespace PrComments { parsed = JSON.parse(stdout) } catch { log.error("gh api graphql returned invalid JSON", { stdout: stdout.slice(0, 200) }) - throw new PR.PrError("COMMENTS_FETCH_FAILED", "GitHub API returned invalid response") + throw new PR.PrError({ code: "COMMENTS_FETCH_FAILED", message: "GitHub API returned invalid response" }) } const response = parsed as GqlReviewThreadsResponse @@ -139,7 +140,7 @@ export namespace PrComments { if (response.errors) { const msg = response.errors.map((e) => e.message).join("; ") log.error("GraphQL errors", { errors: response.errors }) - throw new PR.PrError("COMMENTS_FETCH_FAILED", msg || "GraphQL query failed") + throw new PR.PrError({ code: "COMMENTS_FETCH_FAILED", message: msg || "GraphQL query failed" }) } const threads = response.data?.repository?.pullRequest?.reviewThreads diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index f17e6668aef6..91480c9b4458 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -1,6 +1,8 @@ import { $ } from "bun" import z from "zod" +import { NamedError } from "@opencode-ai/util/error" import { Log } from "@/util/log" +import { withTimeout } from "@/util/timeout" import { Instance } from "./instance" import { Vcs } from "./vcs" @@ -20,15 +22,8 @@ export namespace PR { ]) export type ErrorCode = z.infer - export class PrError extends Error { - constructor( - public code: ErrorCode, - message: string, - ) { - super(message) - this.name = "PrError" - } - } + export const PrError = NamedError.create("PrError", z.object({ code: ErrorCode, message: z.string() })) + export type PrError = InstanceType export const CreateInput = z .object({ @@ -58,12 +53,7 @@ export namespace PR { export const ReadyInput = z.object({}).meta({ ref: "PrReadyInput" }) export type ReadyInput = z.infer - export const PrErrorResponse = z - .object({ - code: ErrorCode, - message: z.string(), - }) - .meta({ ref: "PrErrorResponse" }) + export const PrErrorResponse = PrError.Schema async function fetchUnresolvedCommentCount( owner: string, @@ -72,12 +62,15 @@ export namespace PR { ): Promise { const cwd = Instance.worktree const query = `query($owner: String!, $name: String!, $prNumber: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $prNumber) { reviewThreads(first: 100) { nodes { isResolved } } } } }` - const result = await $`gh api graphql -f query=${query} -f owner=${owner} -f name=${name} -F prNumber=${prNumber}` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch(() => "") + const result = await withTimeout( + $`gh api graphql -f query=${query} -f owner=${owner} -f name=${name} -F prNumber=${prNumber}` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => ""), + 30_000, + ) try { const parsed = JSON.parse(result) const threads = parsed?.data?.repository?.pullRequest?.reviewThreads?.nodes @@ -92,12 +85,14 @@ export namespace PR { export async function fetchForBranch(repo?: { owner: string; name: string }): Promise { const cwd = Instance.worktree try { - const result = - await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` + const result = await withTimeout( + $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` .quiet() .nothrow() .cwd(cwd) - .text() + .text(), + 30_000, + ) if (!result.trim()) { return undefined } @@ -191,10 +186,10 @@ export namespace PR { const info = await Vcs.info() const github = info.github if (!github?.available) { - throw new PrError("GH_NOT_INSTALLED", "GitHub CLI (gh) is not installed") + throw new PrError({ code: "GH_NOT_INSTALLED", message: "GitHub CLI (gh) is not installed" }) } if (!github.authenticated) { - throw new PrError("GH_NOT_AUTHENTICATED", "Run `gh auth login` to authenticate") + throw new PrError({ code: "GH_NOT_AUTHENTICATED", message: "Run `gh auth login` to authenticate" }) } return { info, github: github as Vcs.GithubCapability & { available: true; authenticated: true } } } @@ -217,7 +212,7 @@ export namespace PR { const errorOutput = push.stderr?.toString().trim() || push.stdout?.toString().trim() || "Failed to push branch automatically" log.error("push failed", { output: errorOutput }) - throw new PrError("CREATE_FAILED", sanitizeOutput(errorOutput)) + throw new PrError({ code: "CREATE_FAILED", message: sanitizeOutput(errorOutput) }) } const args = ["gh", "pr", "create", "--title", input.title] @@ -225,18 +220,21 @@ export namespace PR { if (input.base) args.push("--base", input.base) if (input.draft) args.push("--draft") - const cmd = await $`${args}` - .quiet() - .nothrow() - .cwd(cwd) - .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const cmd = await withTimeout( + $`${args}` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })), + 60_000, + ) const result = cmd.stdout.toString() const errorOut = cmd.stderr.toString() if (cmd.exitCode !== 0 || !result.trim()) { log.error("pr create failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) - throw new PrError("CREATE_FAILED", sanitizeOutput(errorOut.trim() || result.trim() || "Failed to create pull request")) + throw new PrError({ code: "CREATE_FAILED", message: sanitizeOutput(errorOut.trim() || result.trim() || "Failed to create pull request") }) } await Vcs.refresh() @@ -248,7 +246,7 @@ export namespace PR { const prUrl = result.trim().split("\n").pop() ?? "" const numberMatch = prUrl.match(/\/pull\/(\d+)/) if (!numberMatch) { - throw new PrError("CREATE_FAILED", "Pull request was created but could not determine PR number from output") + throw new PrError({ code: "CREATE_FAILED", message: "Pull request was created but could not determine PR number from output" }) } return { number: parseInt(numberMatch[1], 10), @@ -270,25 +268,28 @@ export namespace PR { const currentPr = await get() if (!currentPr) { - throw new PrError("NO_PR", "No pull request found for the current branch") + throw new PrError({ code: "NO_PR", message: "No pull request found for the current branch" }) } if (!currentPr.isDraft) { return currentPr } - const cmd = await $`gh pr ready` - .quiet() - .nothrow() - .cwd(cwd) - .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const cmd = await withTimeout( + $`gh pr ready` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })), + 30_000, + ) const result = cmd.stdout.toString() const errorOut = cmd.stderr.toString() if (cmd.exitCode !== 0) { log.error("pr ready failed", { stdout: result, stderr: errorOut, exitCode: cmd.exitCode }) - throw new PrError("READY_FAILED", sanitizeOutput(errorOut.trim() || result.trim() || "Failed to mark PR as ready")) + throw new PrError({ code: "READY_FAILED", message: sanitizeOutput(errorOut.trim() || result.trim() || "Failed to mark PR as ready") }) } await Vcs.refresh() @@ -302,16 +303,16 @@ export namespace PR { const currentPr = await get() if (!currentPr) { - throw new PrError("NO_PR", "No pull request found for the current branch") + throw new PrError({ code: "NO_PR", message: "No pull request found for the current branch" }) } if (currentPr.mergeable === "CONFLICTING") { - throw new PrError("MERGE_FAILED", "PR has merge conflicts that must be resolved first") + throw new PrError({ code: "MERGE_FAILED", message: "PR has merge conflicts that must be resolved first" }) } const github = info.github if (!github?.repo) { - throw new PrError("MERGE_FAILED", "Unable to determine repository information") + throw new PrError({ code: "MERGE_FAILED", message: "Unable to determine repository information" }) } const strategy = input.strategy ?? "squash" @@ -327,11 +328,14 @@ export namespace PR { `merge_method=${mergeMethod}`, ] - const cmd = await $`${args}` - .quiet() - .nothrow() - .cwd(cwd) - .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const cmd = await withTimeout( + $`${args}` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e: unknown) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })), + 60_000, + ) const responseText = cmd.stdout.toString().trim() const errorOut = cmd.stderr.toString().trim() @@ -345,10 +349,12 @@ export namespace PR { } } catch {} log.error("pr merge failed", { stderr: errorOut, stdout: responseText, exitCode: cmd.exitCode }) - throw new PrError("MERGE_FAILED", sanitizeOutput(errorMessage)) + throw new PrError({ code: "MERGE_FAILED", message: sanitizeOutput(errorMessage) }) } await Vcs.refresh() + const updated = await Vcs.info() + const result: Vcs.PrInfo = updated.pr ?? { ...currentPr, state: "MERGED" } if (input.deleteBranch === true) { const branchToDelete = currentPr.headRefName @@ -357,12 +363,12 @@ export namespace PR { await deleteBranch({ branch: branchToDelete }) } catch (e) { log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) + result.branchDeleteFailed = true } } } - const updated = await Vcs.info() - return updated.pr ?? { ...currentPr, state: "MERGED" } + return result } export async function deleteBranch(input: DeleteBranchInput): Promise { @@ -381,7 +387,7 @@ export namespace PR { // Ignore "remote ref does not exist" — branch may already be deleted if (!errorOut.includes("remote ref does not exist")) { log.error("delete remote branch failed", { stderr: errorOut }) - throw new PrError("DELETE_BRANCH_FAILED", sanitizeOutput(errorOut || "Failed to delete remote branch")) + throw new PrError({ code: "DELETE_BRANCH_FAILED", message: sanitizeOutput(errorOut || "Failed to delete remote branch") }) } } diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index c37605ee0d85..dcddfede9f7a 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -3,6 +3,7 @@ import { Bus } from "@/bus" import { $ } from "bun" import z from "zod" import { Log } from "@/util/log" +import { withTimeout } from "@/util/timeout" import { Instance } from "./instance" import { FileWatcher } from "@/file/watcher" @@ -32,6 +33,7 @@ export namespace Vcs { }) .optional(), unresolvedCommentCount: z.number().optional(), + branchDeleteFailed: z.boolean().optional(), }) .meta({ ref: "PrInfo" }) export type PrInfo = z.infer @@ -92,11 +94,14 @@ export namespace Vcs { export async function detectGithubCapability(): Promise { const cwd = Instance.worktree - const authCmd = await $`gh auth status` - .quiet() - .nothrow() - .cwd(cwd) - .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const authCmd = await withTimeout( + $`gh auth status` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })), + 30_000, + ) const authText = (authCmd.stdout?.toString() ?? "") + (authCmd.stderr?.toString() ?? "") const available = !authText.includes("not found") && !authText.includes("command not found") if (!available) { @@ -106,11 +111,14 @@ export namespace Vcs { if (!authenticated) { return { available: true, authenticated: false } } - const repoCmd = await $`gh repo view --json owner,name,url` - .quiet() - .nothrow() - .cwd(cwd) - .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })) + const repoCmd = await withTimeout( + $`gh repo view --json owner,name,url` + .quiet() + .nothrow() + .cwd(cwd) + .catch((e) => ({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from(String(e)) })), + 30_000, + ) let repo: { owner: string; name: string } | undefined let host: string | undefined @@ -146,12 +154,15 @@ export namespace Vcs { if (trimmed && !trimmed.includes("fatal") && trimmed !== "origin/HEAD") { return trimmed.replace("origin/", "") } - const ghResult = await $`gh repo view --json defaultBranchRef --jq .defaultBranchRef.name` - .quiet() - .nothrow() - .cwd(cwd) - .text() - .catch(() => "") + const ghResult = await withTimeout( + $`gh repo view --json defaultBranchRef --jq .defaultBranchRef.name` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => ""), + 30_000, + ) const ghTrimmed = ghResult.trim() if (ghTrimmed && !ghTrimmed.includes("error")) { return ghTrimmed diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index a9b22c67c174..6b5736c7bc8e 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -102,16 +102,9 @@ export const VcsRoutes = lazy(() => }), validator("json", PR.CreateInput), async (c) => { - try { - const input = c.req.valid("json") - const pr = await PR.create(input) - return c.json(pr) - } catch (e) { - if (e instanceof PR.PrError) { - return c.json({ code: e.code, message: e.message }, { status: 400 }) - } - throw e - } + const input = c.req.valid("json") + const pr = await PR.create(input) + return c.json(pr) }, ) .get( @@ -134,16 +127,8 @@ export const VcsRoutes = lazy(() => }, }), async (c) => { - try { - const data = await PrComments.fetch() - return c.json(data) - } catch (e) { - if (e instanceof PR.PrError) { - const status = e.code === "NO_PR" ? 404 : 400 - return c.json({ code: e.code, message: e.message }, { status }) - } - throw e - } + const data = await PrComments.fetch() + return c.json(data) }, ) .post( @@ -166,17 +151,9 @@ export const VcsRoutes = lazy(() => }), validator("json", PR.MergeInput), async (c) => { - try { - const input = c.req.valid("json") - const pr = await PR.merge(input) - return c.json(pr) - } catch (e) { - if (e instanceof PR.PrError) { - const status = e.code === "NO_PR" ? 404 : 400 - return c.json({ code: e.code, message: e.message }, { status }) - } - throw e - } + const input = c.req.valid("json") + const pr = await PR.merge(input) + return c.json(pr) }, ) .post( @@ -199,17 +176,9 @@ export const VcsRoutes = lazy(() => }), validator("json", PR.ReadyInput), async (c) => { - try { - const input = c.req.valid("json") - const pr = await PR.ready(input) - return c.json(pr) - } catch (e) { - if (e instanceof PR.PrError) { - const status = e.code === "NO_PR" ? 404 : 400 - return c.json({ code: e.code, message: e.message }, { status }) - } - throw e - } + const input = c.req.valid("json") + const pr = await PR.ready(input) + return c.json(pr) }, ) .post( @@ -232,17 +201,9 @@ export const VcsRoutes = lazy(() => }), validator("json", PR.DeleteBranchInput), async (c) => { - try { - const input = c.req.valid("json") - await PR.deleteBranch(input) - return c.json({ ok: true }) - } catch (e) { - if (e instanceof PR.PrError) { - const status = e.code === "NO_PR" ? 404 : 400 - return c.json({ code: e.code, message: e.message }, { status }) - } - throw e - } + const input = c.req.valid("json") + await PR.deleteBranch(input) + return c.json({ ok: true }) }, ) .post( diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index ba0c0302a0d5..cef4b80136c9 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -10,6 +10,7 @@ import { NamedError } from "@opencode-ai/util/error" import { LSP } from "../lsp" import { Format } from "../format" import { TuiRoutes } from "./routes/tui" +import { PR } from "../project/pr" import { Instance } from "../project/instance" import { Agent } from "../agent/agent" import { Skill } from "../skill" @@ -65,6 +66,7 @@ export namespace Server { else if (err instanceof Provider.ModelNotFoundError) status = 400 else if (err.name === "ProviderAuthValidationFailed") status = 400 else if (err.name.startsWith("Worktree")) status = 400 + else if (PR.PrError.isInstance(err)) status = err.data.code === "NO_PR" ? 404 : 400 else status = 500 return c.json(err.toObject(), { status }) } From 2d6d555c292b16854a4fd4d40f7a71b7d9c4f38a Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 23:09:50 -0500 Subject: [PATCH 37/46] fix(pr): improve error visibility, pagination bounds, and UI consistency - Log stderr from gh pr view instead of silently returning undefined (#10) - Cap pr-comments pagination at 10 pages to prevent runaway requests (#18) - Use resolveApiErrorMessage in delete-branch dialog for consistent error UX (#19) - Default remoteBranchExists to false while loading to avoid UI flash (#20) --- packages/app/src/components/dialog-delete-branch.tsx | 7 +++++-- packages/app/src/components/pr-button.tsx | 2 +- packages/opencode/src/project/pr-comments.ts | 6 ++++++ packages/opencode/src/project/pr.ts | 12 ++++++++---- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/app/src/components/dialog-delete-branch.tsx b/packages/app/src/components/dialog-delete-branch.tsx index b4f46f561614..5393acd3bc18 100644 --- a/packages/app/src/components/dialog-delete-branch.tsx +++ b/packages/app/src/components/dialog-delete-branch.tsx @@ -8,6 +8,7 @@ import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { useSDK } from "@/context/sdk" import { useSync } from "@/context/sync" +import { resolveApiErrorMessage } from "@/utils/pr-errors" export function DeleteBranchDialog() { const dialog = useDialog() @@ -39,11 +40,13 @@ export function DeleteBranchDialog() { title: language.t("pr.toast.branch_deleted"), }) dialog.close() - } catch { + } catch (e: unknown) { showToast({ variant: "error", icon: "circle-x", - title: language.t("pr.error.delete_branch_failed"), + title: resolveApiErrorMessage(e, language.t("pr.error.delete_branch_failed"), (k) => + language.t(k as Parameters[0]), + ), }) } finally { setStore("submitting", false) diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 0f15fa38ebc1..9d7638223a3b 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -70,7 +70,7 @@ export function PrButton() { const remoteBranchExists = createMemo(() => { const branchName = pr()?.headRefName const branches = vcs()?.branches - if (!branchName || !branches) return true + if (!branchName || !branches) return false return branches.includes(branchName) }) diff --git a/packages/opencode/src/project/pr-comments.ts b/packages/opencode/src/project/pr-comments.ts index a08c9719cfc8..1e19e26ff636 100644 --- a/packages/opencode/src/project/pr-comments.ts +++ b/packages/opencode/src/project/pr-comments.ts @@ -90,8 +90,14 @@ export namespace PrComments { const allThreads: ReviewThread[] = [] let cursor: string | null = null + const MAX_PAGES = 10 + let page = 0 do { + if (++page > MAX_PAGES) { + log.warn("pr-comments: max pages reached, truncating") + break + } const query = buildQuery() const args = [ "gh", diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 91480c9b4458..1f916515cbc6 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -85,15 +85,19 @@ export namespace PR { export async function fetchForBranch(repo?: { owner: string; name: string }): Promise { const cwd = Instance.worktree try { - const result = await withTimeout( + const cmd = await withTimeout( $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` .quiet() .nothrow() - .cwd(cwd) - .text(), + .cwd(cwd), 30_000, ) - if (!result.trim()) { + if (cmd.exitCode !== 0) { + log.warn("gh pr view failed", { stderr: cmd.stderr.toString().trim() }) + return undefined + } + const result = cmd.stdout.toString().trim() + if (!result) { return undefined } const parsed = JSON.parse(result) From a6a1d87c7e9e16802101427ad803fb79e2969fed Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 23:14:41 -0500 Subject: [PATCH 38/46] fix(pr): refresh before create, validate base branch, fence comment bodies - Refresh VCS state before idempotency check in create() to avoid TOCTOU race (#4, #12) - Use refreshed branch/defaultBranch in fallback PR return (#9) - Validate base branch exists before attempting gh pr create (#22) - Wrap review comment bodies in fenced blocks for structural separation (#8) --- .../app/src/components/dialog-address-comments.tsx | 2 +- packages/opencode/src/project/pr.ts | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/dialog-address-comments.tsx b/packages/app/src/components/dialog-address-comments.tsx index 66b84cfd47e8..eb56d8552e40 100644 --- a/packages/app/src/components/dialog-address-comments.tsx +++ b/packages/app/src/components/dialog-address-comments.tsx @@ -97,7 +97,7 @@ export function AddressCommentsDialog() { if (thread.comments.length === 0) continue text += `### File: ${thread.path}${thread.line ? ` (line ${thread.line})` : ""}\n` for (const comment of thread.comments) { - text += `**@${comment.author}** (comment ID: ${comment.id}): ${comment.body}\n` + text += `**@${comment.author}** (comment ID: ${comment.id}):\n\`\`\`\n${comment.body}\n\`\`\`\n` } text += "\n" } diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 1f916515cbc6..66e07afdc27f 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -204,6 +204,7 @@ export namespace PR { } export async function create(input: CreateInput): Promise { + await Vcs.refresh() const { info } = await ensureGithub() const cwd = Instance.worktree @@ -219,6 +220,13 @@ export namespace PR { throw new PrError({ code: "CREATE_FAILED", message: sanitizeOutput(errorOutput) }) } + if (input.base) { + const branches = info.branches ?? (await Vcs.fetchBranches()) + if (!branches.includes(input.base)) { + throw new PrError({ code: "CREATE_FAILED", message: `Base branch '${input.base}' does not exist` }) + } + } + const args = ["gh", "pr", "create", "--title", input.title] args.push("--body", input.body) if (input.base) args.push("--base", input.base) @@ -257,8 +265,8 @@ export namespace PR { url: prUrl, title: input.title, state: "OPEN", - headRefName: info.branch, - baseRefName: input.base ?? info.defaultBranch ?? "main", + headRefName: updated.branch || info.branch, + baseRefName: input.base ?? updated.defaultBranch ?? info.defaultBranch ?? "main", isDraft: input.draft ?? false, mergeable: "UNKNOWN", reviewDecision: null, From f711033d2ae617f91b1d550c86013cdc5eea7177 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Mon, 2 Mar 2026 23:26:38 -0500 Subject: [PATCH 39/46] fix(pr): hide unread-comments badge on workspace hover The unresolved-comments dot badge remained visible when hovering over the workspace name, appearing as a stray purple dot after the PR pill animated away. --- packages/app/src/pages/layout/sidebar-workspace.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/pages/layout/sidebar-workspace.tsx b/packages/app/src/pages/layout/sidebar-workspace.tsx index 7b3db117f313..c8194bd2c1ed 100644 --- a/packages/app/src/pages/layout/sidebar-workspace.tsx +++ b/packages/app/src/pages/layout/sidebar-workspace.tsx @@ -146,7 +146,7 @@ const WorkspaceHeader = (props: { PR #{currentPr().number} -
+
) From 99364828940183dc63d3cb52609ffe09ddddddfe Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Tue, 3 Mar 2026 21:19:19 -0500 Subject: [PATCH 40/46] refactor(pr): extract hardcoded GitHub PR state colors into CSS custom properties Define --pr-color-{open,merged,closed,draft} tokens in index.css and consume them via var() in pr-style.ts, giving a single source of truth for PR state colors that can be overridden per-theme if needed. --- packages/app/src/index.css | 11 +++++++++++ packages/app/src/utils/pr-style.ts | 26 +++++++++++++------------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/app/src/index.css b/packages/app/src/index.css index 9e231e2d2858..7147944ab8bf 100644 --- a/packages/app/src/index.css +++ b/packages/app/src/index.css @@ -1,5 +1,16 @@ @import "@opencode-ai/ui/styles/tailwind"; +:root { + --pr-color-open: #238636; + --pr-color-open-text: #3fb950; + --pr-color-merged: #8957e5; + --pr-color-merged-text: #a371f7; + --pr-color-closed: #da3633; + --pr-color-closed-text: #f85149; + --pr-color-draft: #768390; + --pr-color-draft-text: #768390; +} + @layer components { [data-component="getting-started"] { container-type: inline-size; diff --git a/packages/app/src/utils/pr-style.ts b/packages/app/src/utils/pr-style.ts index b8de41fef672..de920700395b 100644 --- a/packages/app/src/utils/pr-style.ts +++ b/packages/app/src/utils/pr-style.ts @@ -1,25 +1,25 @@ import type { PrInfo } from "@opencode-ai/sdk/v2/client" export function getPrPillStyle(pr: PrInfo): string { - if (pr.state === "MERGED") return "border-[#8957e5]/40 bg-[#8957e5]/15 text-[#a371f7]" - if (pr.state === "CLOSED") return "border-[#da3633]/40 bg-[#da3633]/15 text-[#f85149]" - if (pr.isDraft) return "border-[#768390]/40 bg-[#768390]/15 text-[#768390]" - if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[#da3633]/40 bg-[#da3633]/15 text-[#f85149]" - return "border-[#238636]/40 bg-[#238636]/15 text-[#3fb950]" + if (pr.state === "MERGED") return "border-[var(--pr-color-merged)]/40 bg-[var(--pr-color-merged)]/15 text-[var(--pr-color-merged-text)]" + if (pr.state === "CLOSED") return "border-[var(--pr-color-closed)]/40 bg-[var(--pr-color-closed)]/15 text-[var(--pr-color-closed-text)]" + if (pr.isDraft) return "border-[var(--pr-color-draft)]/40 bg-[var(--pr-color-draft)]/15 text-[var(--pr-color-draft-text)]" + if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[var(--pr-color-closed)]/40 bg-[var(--pr-color-closed)]/15 text-[var(--pr-color-closed-text)]" + return "border-[var(--pr-color-open)]/40 bg-[var(--pr-color-open)]/15 text-[var(--pr-color-open-text)]" } export function getPrButtonContainerStyle(pr: PrInfo | undefined): string { if (!pr) return "border-border-weak-base bg-surface-panel" - if (pr.state === "MERGED") return "border-[#8957e5]/60 bg-[#8957e5]/20" - if (pr.state === "CLOSED") return "border-[#da3633]/60 bg-[#da3633]/20" - if (pr.isDraft) return "border-[#768390]/60 bg-[#768390]/20" - return "border-[#238636]/60 bg-[#238636]/20" + if (pr.state === "MERGED") return "border-[var(--pr-color-merged)]/60 bg-[var(--pr-color-merged)]/20" + if (pr.state === "CLOSED") return "border-[var(--pr-color-closed)]/60 bg-[var(--pr-color-closed)]/20" + if (pr.isDraft) return "border-[var(--pr-color-draft)]/60 bg-[var(--pr-color-draft)]/20" + return "border-[var(--pr-color-open)]/60 bg-[var(--pr-color-open)]/20" } export function getPrButtonDividerStyle(pr: PrInfo | undefined): string { if (!pr) return "bg-border-weak-base" - if (pr.state === "MERGED") return "bg-[#8957e5]/60" - if (pr.state === "CLOSED") return "bg-[#da3633]/60" - if (pr.isDraft) return "bg-[#768390]/60" - return "bg-[#238636]/60" + if (pr.state === "MERGED") return "bg-[var(--pr-color-merged)]/60" + if (pr.state === "CLOSED") return "bg-[var(--pr-color-closed)]/60" + if (pr.isDraft) return "bg-[var(--pr-color-draft)]/60" + return "bg-[var(--pr-color-open)]/60" } From cf482904dd952ff38e2a5bdd626b4c542be4892f Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 15 Mar 2026 16:00:47 -0400 Subject: [PATCH 41/46] fix(vcs): clear stale state and stage untracked files --- PR-REVIEW.md | 499 ++++++++++++++++++ .../context/global-sync/event-reducer.test.ts | 56 ++ .../src/context/global-sync/event-reducer.ts | 17 +- packages/opencode/src/project/vcs.ts | 22 +- 4 files changed, 577 insertions(+), 17 deletions(-) create mode 100644 PR-REVIEW.md diff --git a/PR-REVIEW.md b/PR-REVIEW.md new file mode 100644 index 000000000000..e1b0262e0ce9 --- /dev/null +++ b/PR-REVIEW.md @@ -0,0 +1,499 @@ +# Code Review: feat/github-pr-integration + +**Branch:** `feat/github-pr-integration` (based off `dev`) +**Scope:** ~3,300 lines added across 25 files — backend PR operations, SDK generation, and frontend UI. + +--- + +## Issues + +### 1. ~~Branch name regex allows path traversal in later segments~~ COMPLETED + +**Severity:** High | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:53` + +The `DeleteBranchInput` regex `^(?![^/]*\.\.)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$` uses a negative lookahead that only checks the first path segment (before the first `/`). Multi-segment paths containing `..` pass validation: + +``` +"a/b/../c" → passes (should fail) +"a/b/../../etc/passwd" → passes (should fail) +``` + +While Bun's `$` template literal prevents command injection and git treats refnames literally, the stated intent is to block `..` traversal and the regex fails to do so. + +**Fix:** Use a regex that rejects `..` anywhere: + +```ts +branch: z.string().regex(/^(?!.*\.\.)(?!.*\/\/)(?!\/)(?!.*\/$)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name") +``` + +Or use a simpler `.refine()` check: `.refine(s => !s.includes('..'), "Invalid branch name")`. + +--- + +### 2. ~~`ensureGithub()` returns `Required` but `repo` can be undefined~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:195` + +`ensureGithub()` casts the return to `Required`, claiming `repo` and `host` are always present. But `detectGithubCapability()` can return `{ available: true, authenticated: true }` without `repo` when `gh repo view` fails (e.g., detached worktree, network issues, non-GitHub remote). + +The `merge()` function safely re-checks `github?.repo` at line 305, but `create()` at line 204 destructures `{ info }` and never checks `info.github.repo` — it doesn't use `repo` directly, so it's safe by accident. Future code relying on the `Required<>` type won't be safe. + +**Fix:** Don't cast to `Required<>`. Instead, return the narrowed type: + +```ts +return { info, github: github as Vcs.GithubCapability & { available: true; authenticated: true } } +``` + +Or add a `repo` check to `ensureGithub` and return a properly narrowed type. + +--- + +### 3. ~~`CreateInput` schema lacks `min(1)` on title — empty string passes validation~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:35` + +`title: z.string()` allows empty strings. The frontend guards against this (`!store.title.trim()`), but the API endpoint doesn't. A direct API call with `title: ""` would attempt to create a PR with an empty title. + +**Fix:** Add a `min(1)` constraint and reasonable max limits (GitHub's API doesn't document explicit limits, but practical limits exist): + +```ts +title: z.string().min(1).max(1024), +body: z.string().optional(), +``` + +The `min(1)` on `title` is the essential fix. Max limits are a safeguard against accidental abuse. + +--- + +### 4. ~~Race condition in `PR.create()` — TOCTOU on `info.pr`~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:207-209` + +`create()` checks `info.pr` to short-circuit if a PR already exists, but `info` comes from cached VCS state. Two concurrent `create()` calls could both see `info.pr` as `undefined`, push to origin, and both call `gh pr create`. The second call would fail with a `gh` error (GitHub rejects duplicate PRs), which is handled, but the push would have already happened. + +The `POST /pr` route is documented as "Idempotent — returns existing PR if one exists" but this is only true at the state cache level, not at the git/GitHub level. + +**Fix:** This is low-risk since `gh` is the final arbiter, but if true idempotency is desired, refresh VCS state before the check: + +```ts +await Vcs.refresh() +const { info } = await ensureGithub() +``` + +--- + +### 5. ~~Event reducer omits `branches` field from `vcs.updated` event type~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/app/src/context/global-sync/event-reducer.ts:262-268` + +The `vcs.updated` event type cast omits `branches`: + +```ts +const props = event.properties as { + branch?: string + defaultBranch?: string + dirty?: number + pr?: VcsInfo["pr"] + github?: VcsInfo["github"] + // missing: branches?: string[] +} +``` + +The server publishes `branches` in the event (`vcs.ts:293`). At runtime the `...props` spread will include it, but the explicit type annotation is incomplete and misleading for future maintainers. + +**Fix:** Add `branches?: string[]` to the type annotation. + +--- + +### 6. ~~Route error handling diverges from project convention~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/server/routes/vcs.ts` (all handlers) + +Every other route file in the project (session, project, file, config, etc.) lets errors propagate to the global `onError` handler in `server.ts`. The VCS routes use local try/catch blocks instead. This is because `PrError` extends `Error` (not `NamedError`), so the global handler would return 500 for what should be 400. + +**Fix:** `NamedError` uses a static `create()` factory (not a constructable base class), so the integration pattern is different from a simple `extends`. Create a named error via the factory and add a status mapping to the global handler: + +```ts +// In pr.ts — create a NamedError subclass via the factory +export const PrError = NamedError.create("PrError", z.object({ code: ErrorCode, message: z.string() })) +export type PrError = InstanceType +``` + +Then in `server.ts`, add a clause to the global `onError` handler: + +```ts +else if (err instanceof PrError) status = err.data.code === "NO_PR" ? 404 : 400 +``` + +This lets you remove all try/catch blocks from the VCS route handlers, aligning with the project's error handling convention. + +**Note:** This changes how `PrError` is constructed — all `throw new PrError(code, msg)` calls would become `throw new PrError({ code, message: msg })`. + +--- + +### 7. ~~`clearInterval` used on `setTimeout` handle~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/vcs.ts:364` + +The cleanup function uses `clearInterval(pollTimer)` but `pollTimer` is set via `setTimeout` (line 273). While this works in practice (both clear the same timer queue in Bun/Node), it's semantically incorrect. + +**Fix:** Change to `clearTimeout(pollTimer)`. + +--- + +### 8. ~~`address-comments` prompt includes unsanitized comment body text~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/app/src/components/dialog-address-comments.tsx:100` + +Review comment bodies from GitHub are interpolated directly into the agent prompt: + +```ts +text += `**@${comment.author}** (comment ID: ${comment.id}): ${comment.body}\n` +``` + +A malicious review comment could contain text designed to manipulate the agent (prompt injection). This is inherent to the feature design (the whole point is to pass comments to the agent), but worth noting that there's no sanitization or structural separation between the instruction portion and the user-controlled content. + +**Fix:** Consider wrapping user-controlled content in a fenced block: + +```ts +text += `**@${comment.author}** (comment ID: ${comment.id}):\n\`\`\`\n${comment.body}\n\`\`\`\n` +``` + +This doesn't prevent prompt injection but provides structural separation. + +--- + +### 9. ~~`PR.create()` returns `info.branch` which may be stale~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:254` + +In the fallback return (lines 249-261), `headRefName` is set to `info.branch` — the branch name from the cached VCS state snapshot taken _before_ the push and PR creation. If the branch name somehow changed between the state read and the PR creation, this would be wrong. + +This is extremely unlikely in practice, but for consistency with the rest of the function (which refreshes state via `Vcs.refresh()`), the branch should come from the refreshed state. + +**Fix:** Use `(await Vcs.info()).branch` or move the fallback before the `Vcs.refresh()` call. + +--- + +### 10. ~~`fetchForBranch` silently returns `undefined` on `gh pr view` stderr errors~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:96-101` + +`gh pr view` is called with `.nothrow()`, and if the command writes to stderr (e.g., auth errors, rate limiting), the output goes to `result` only if it's on stdout. The function checks `if (!result.trim()) return undefined` — but on error, `gh` may write nothing to stdout and everything to stderr, causing a silent `undefined` return with no logging for what might be a transient error. + +The outer `catch` at line 180-183 does log, but this path (empty stdout, non-zero exit code) hits the early return at line 101-102, not the catch. + +**Fix:** Remove `.text()` to get the full result object, then check `exitCode` before parsing: + +```ts +const result = + await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` + .quiet() + .nothrow() + .cwd(cwd) +if (result.exitCode !== 0) { + log.warn("gh pr view failed", { stderr: result.stderr.toString() }) + return undefined +} +const text = result.stdout.toString().trim() +if (!text) return undefined +const parsed = JSON.parse(text) +``` + +--- + +### 11. Hardcoded GitHub status colors bypass design system tokens + +**Severity:** Low +**File:** `packages/app/src/utils/pr-style.ts:4-8` + +The PR pill and button styles use hardcoded hex colors (`#8957e5`, `#da3633`, `#238636`, `#768390`) rather than design system tokens. This will clash with custom themes and doesn't respect the existing `text-icon-success-base` / `text-icon-critical-base` token pattern used elsewhere (e.g., `pr-button.tsx:109-111`). + +**Fix:** Use the project's semantic tokens: + +```ts +if (pr.state === "MERGED") return "border-border-info-base/40 bg-surface-info-weak text-text-info" +if (pr.state === "CLOSED") return "border-border-critical-base/40 bg-surface-critical-weak text-text-danger" +``` + +--- + +### 12. ~~`PR.create()` idempotency check doesn't refresh before reading~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:207` + +The idempotency check `if (info.pr) return info.pr` reads from cached state without refreshing. If a PR was created outside opencode (e.g., on github.com) since the last poll, the cached state won't know about it, and `create()` will attempt to create a duplicate (which `gh` will reject). + +This is the same issue as #4 but from the perspective of external PR creation. Low severity because `gh pr create` would return a clear error. + +**Fix:** Call `await Vcs.refresh()` before the idempotency check, or document that the idempotency is best-effort. + +--- + +### 13. ~~Merge doesn't verify PR is mergeable before attempting~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:295-310` + +The `merge()` function doesn't check `currentPr.mergeable` before calling the GitHub merge API. The frontend shows a conflict warning and disables the button, but the backend doesn't enforce it. A direct API call can attempt to merge a PR with conflicts. + +```ts +const currentPr = await get() +if (!currentPr) { + throw new PrError("NO_PR", "...") +} +// No check for mergeable === "CONFLICTING" +const strategy = input.strategy ?? "squash" +``` + +**Fix:** Add server-side validation: + +```ts +if (currentPr.mergeable === "CONFLICTING") { + throw new PrError("MERGE_FAILED", "PR has merge conflicts that must be resolved first") +} +``` + +--- + +### 14. ~~Wrong HTTP status code for `NO_PR` errors~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/server/routes/vcs.ts:109-114` + +All `PrError` codes return 400, but `NO_PR` is semantically a 404 (resource not found). Also, `GH_NOT_INSTALLED` and `GH_NOT_AUTHENTICATED` are client-side prerequisite failures — 400 is acceptable (the request is malformed given the current environment), though 424 (Failed Dependency) could also fit. + +**Fix:** Map error codes to appropriate statuses: + +```ts +const status = e.code === "NO_PR" ? 404 : 400 +return c.json({ code: e.code, message: e.message }, { status }) +``` + +--- + +### 15. ~~No timeout on GitHub CLI commands~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** Multiple (`pr.ts`, `pr-comments.ts`, `vcs.ts`) + +All `gh` subprocess calls have no timeout. On slow networks, GitHub Enterprise with rate limiting, or when `gh` hangs waiting for input, these commands block indefinitely. + +```ts +const result = await $`gh pr view --json ...`.quiet().nothrow().cwd(cwd).text() +``` + +**Fix:** Bun's `$` shell does **not** have a `.timeout()` method. Use `Promise.race()` instead: + +```ts +function withTimeout(promise: Promise, ms: number): Promise { + return Promise.race([promise, new Promise((_, reject) => setTimeout(() => reject(new Error("timeout")), ms))]) +} + +const result = await withTimeout($`gh pr view --json ...`.quiet().nothrow().cwd(cwd).text(), 30_000) +``` + +Alternatively, consider using `Bun.spawn()` which supports a `timeout` option, though that would require a larger refactor. + +--- + +### 16. ~~Error messages may expose sensitive data~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:214-216, 233-235` + +Raw stderr/stdout from `git push` and `gh pr create` is passed directly into `PrError` messages and returned to clients. These may contain file paths, environment variables, or token fragments depending on git configuration and hooks. + +```ts +const errorOutput = push.stderr?.toString().trim() || push.stdout?.toString().trim() +throw new PrError("CREATE_FAILED", errorOutput) +``` + +**Fix:** Truncate and sanitize error output: + +```ts +function sanitize(output: string): string { + return output.replace(/(ghp_|github_pat_)[a-zA-Z0-9_]+/g, "").slice(0, 500) +} +``` + +--- + +### 17. ~~Branch deletion after merge — silent failure and ordering risk~~ COMPLETED + +**Severity:** Medium | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:345-354` + +If merge succeeds but branch deletion fails (network timeout, permissions), the error is silently swallowed with only a `log.warn`. The user gets a success response but the branch isn't cleaned up, and there's no UI indication of the partial failure. + +```ts +try { + await deleteBranch({ branch: branchToDelete }) +} catch (e) { + log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) +} +``` + +**Fix:** Return a response that indicates partial success, or surface the branch deletion failure to the client: + +```ts +const result: Vcs.PrInfo & { branchDeleteFailed?: boolean } = updated.pr ?? { ...currentPr, state: "MERGED" } +try { + await deleteBranch({ branch: branchToDelete }) +} catch (e) { + log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) + result.branchDeleteFailed = true +} +``` + +**Note:** The `branchDeleteFailed` field would also need to be added to the `Vcs.PrInfo` Zod schema (or returned as a separate response type) to be properly typed and serialized through the API. + +--- + +### 18. ~~PrComments pagination has no upper bound~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/pr-comments.ts:93-167` + +The `do...while` pagination loop has no page limit. On PRs with hundreds of review threads, this could issue many sequential GraphQL requests with no cap. + +**Fix:** Add a maximum page count: + +```ts +const MAX_PAGES = 10 +let page = 0 +do { + if (++page > MAX_PAGES) { + log.warn("pr-comments: max pages reached, truncating") + break + } + // ... +} while (cursor) +``` + +--- + +### 19. ~~Inconsistent error handling in delete-branch dialog~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/app/src/components/dialog-delete-branch.tsx:42-47` + +The delete-branch dialog catches errors with a generic toast, unlike every other dialog which uses `resolveApiErrorMessage` to extract structured error context. + +```ts +} catch { + showToast({ + variant: "error", + icon: "circle-x", + title: language.t("pr.error.delete_branch_failed"), + }) +} +``` + +**Fix:** Use consistent error handling matching the other dialogs: + +```ts +} catch (e: unknown) { + showToast({ + variant: "error", + icon: "circle-x", + title: resolveApiErrorMessage(e, language.t("pr.error.delete_branch_failed"), (k) => + language.t(k as Parameters[0]) + ), + }) +} +``` + +--- + +### 20. ~~`remoteBranchExists` defaults to `true` while data is loading~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/app/src/components/pr-button.tsx:70-75` + +```ts +const remoteBranchExists = createMemo(() => { + const branchName = pr()?.headRefName + const branches = vcs()?.branches + if (!branchName || !branches) return true // defaults to true + return branches.includes(branchName) +}) +``` + +When branch data hasn't loaded yet, this defaults to `true`, which could briefly show the "Delete branch" menu item for a merged PR even if the branch is already gone. + +**Fix:** Default to `undefined` or `false` when data isn't available, and guard the UI accordingly. + +--- + +### 21. Stale PR data in delete branch dialog + +**Severity:** Low +**File:** `packages/app/src/components/dialog-delete-branch.tsx:26` + +The dialog reads `pr()` from the sync context, which may be stale (e.g., user has multiple tabs, VCS poll hasn't fired). The `headRefName` used for deletion could theoretically reference a branch that no longer matches the current state. + +**Fix:** Fetch fresh VCS state on dialog mount, or show the branch name prominently with a confirmation step (the dialog already shows the branch name, so this is partially addressed). + +--- + +### 22. ~~No validation that base branch exists before PR creation~~ COMPLETED + +**Severity:** Low | **Status:** Fixed +**File:** `packages/opencode/src/project/pr.ts:221` + +```ts +if (input.base) args.push("--base", input.base) +``` + +If a user specifies a non-existent base branch, they get an opaque error from `gh pr create` rather than a clear validation error. + +**Fix:** Validate against known branches: + +```ts +if (input.base) { + const branches = await Vcs.fetchBranches() + if (!branches.includes(input.base)) { + throw new PrError("CREATE_FAILED", `Base branch '${input.base}' does not exist`) + } + args.push("--base", input.base) +} +``` + +--- + +## Summary + +The implementation is well-structured and follows most project conventions. 22 issues identified: + +| Severity | Count | Key items | +| -------- | ----- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| High | 1 | Branch name regex (#1) | +| Medium | 10 | Type safety (#2), input validation (#3), race conditions (#4, #5), conventions (#6), merge validation (#13), HTTP status (#14), timeouts (#15), error exposure (#16), merge+delete ordering (#17) | +| Low | 11 | Timer mismatch (#7), prompt injection (#8), stale state (#9, #12, #21), silent errors (#10), hardcoded colors (#11), pagination bounds (#18), inconsistent error handling (#19), loading defaults (#20), base branch validation (#22) | + +**Must fix before merge:** ~~#1 (regex)~~, ~~#3 (input validation)~~, ~~#13 (merge validation)~~, ~~#14 (HTTP status codes)~~ — all done. +**Should fix:** ~~#2~~, ~~#5~~, ~~#6~~, ~~#15~~, ~~#16~~, ~~#17~~ — all done. +**Can follow up:** ~~#7~~, ~~#8~~, ~~#9~~, ~~#10~~, #11, ~~#12~~, ~~#18~~, ~~#19~~, ~~#20~~, #21, ~~#22~~ — 2 remaining (#11, #21). + +--- + +## Verification Notes + +All 22 fix recommendations were verified against the source code. **4 corrections were applied:** + +1. **Fix #3** — Removed specific GitHub title/body max limits (256/65536) as they are not documented in the GitHub REST API. Changed to recommend `min(1)` (essential) with reasonable safeguard max. +2. **Fix #6** — Rewrote the `NamedError` integration. `NamedError` is an abstract class with a `static create()` factory pattern, not a direct `extends` target. The fix now shows the correct `NamedError.create()` pattern. +3. **Fix #14** — Removed 503 suggestion for `GH_NOT_INSTALLED`/`GH_NOT_AUTHENTICATED`. These are client-side prerequisite failures, not server unavailability. Changed to 400 (or 424 as alternative). +4. **Fix #15** — Bun's `$` shell does **not** have a `.timeout()` method (confirmed at runtime). Changed to recommend `Promise.race()` pattern or `Bun.spawn()` which does support timeout. diff --git a/packages/app/src/context/global-sync/event-reducer.test.ts b/packages/app/src/context/global-sync/event-reducer.test.ts index cf2da135cbbc..e921ab5ca63c 100644 --- a/packages/app/src/context/global-sync/event-reducer.test.ts +++ b/packages/app/src/context/global-sync/event-reducer.test.ts @@ -515,6 +515,62 @@ describe("applyDirectoryEvent", () => { expect(cacheStore.value).toEqual({ branch: "feature/test" }) }) + test("clears vcs fields when update payload sends null markers", () => { + const [store, setStore] = createStore( + baseState({ + vcs: { + branch: "feature/test", + defaultBranch: "dev", + branches: ["dev", "feature/test"], + dirty: 2, + pr: { + number: 12, + url: "https://github.com/acme/repo/pull/12", + title: "Test", + state: "OPEN", + headRefName: "feature/test", + baseRefName: "dev", + isDraft: false, + mergeable: "MERGEABLE", + }, + github: { + available: true, + authenticated: true, + repo: { owner: "acme", name: "repo" }, + }, + }, + }), + ) + const [cacheStore, setCacheStore] = createStore({ value: store.vcs }) + + applyDirectoryEvent({ + event: { + type: "vcs.updated", + properties: { + branch: "feature/next", + defaultBranch: null, + branches: null, + dirty: null, + pr: null, + github: null, + }, + }, + store, + setStore, + push() {}, + directory: "/tmp", + loadLsp() {}, + vcsCache: { + store: cacheStore, + setStore: setCacheStore, + ready: () => true, + }, + }) + + expect(store.vcs).toEqual({ branch: "feature/next" }) + expect(cacheStore.value).toEqual({ branch: "feature/next" }) + }) + test("routes disposal and lsp events to side-effect handlers", () => { const [store, setStore] = createStore(baseState()) const pushes: string[] = [] diff --git a/packages/app/src/context/global-sync/event-reducer.ts b/packages/app/src/context/global-sync/event-reducer.ts index 3b959be64644..ec459e771977 100644 --- a/packages/app/src/context/global-sync/event-reducer.ts +++ b/packages/app/src/context/global-sync/event-reducer.ts @@ -271,16 +271,21 @@ export function applyDirectoryEvent(input: { case "vcs.updated": { const props = event.properties as { branch?: string - defaultBranch?: string - branches?: string[] - dirty?: number - pr?: VcsInfo["pr"] - github?: VcsInfo["github"] + defaultBranch?: string | null + branches?: string[] | null + dirty?: number | null + pr?: VcsInfo["pr"] | null + github?: VcsInfo["github"] | null } const next: VcsInfo = { ...input.store.vcs, - ...props, + ...Object.fromEntries(Object.entries(props).filter(([key]) => key !== "branch")), branch: props.branch ?? input.store.vcs?.branch ?? "", + defaultBranch: props.defaultBranch === null ? undefined : props.defaultBranch ?? input.store.vcs?.defaultBranch, + branches: props.branches === null ? undefined : props.branches ?? input.store.vcs?.branches, + dirty: props.dirty === null ? undefined : props.dirty ?? input.store.vcs?.dirty, + pr: props.pr === null ? undefined : props.pr ?? input.store.vcs?.pr, + github: props.github === null ? undefined : props.github ?? input.store.vcs?.github, } input.setStore("vcs", next) if (input.vcsCache) input.vcsCache.setStore("value", next) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index dcddfede9f7a..d34e8510e065 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -59,11 +59,11 @@ export namespace Vcs { "vcs.updated", z.object({ branch: z.string().optional(), - defaultBranch: z.string().optional(), - branches: z.array(z.string()).optional(), - dirty: z.number().optional(), - pr: PrInfo.optional(), - github: GithubCapability.optional(), + defaultBranch: z.string().nullable().optional(), + branches: z.array(z.string()).nullable().optional(), + dirty: z.number().nullable().optional(), + pr: PrInfo.nullable().optional(), + github: GithubCapability.nullable().optional(), }), ), } @@ -235,7 +235,7 @@ export namespace Vcs { export async function commit(message: string) { const cwd = Instance.worktree - const add = await $`git add -u`.quiet().nothrow().cwd(cwd) + const add = await $`git add -A`.quiet().nothrow().cwd(cwd) if (add.exitCode !== 0) { throw new Error("git add failed: " + add.stderr.toString()) } @@ -299,11 +299,11 @@ export namespace Vcs { const publish = () => { Bus.publish(Event.Updated, { branch: current.branch, - defaultBranch: current.defaultBranch, - branches: current.branches, - dirty: current.dirty, - pr: current.pr, - github: current.github, + defaultBranch: current.defaultBranch ?? null, + branches: current.branches ?? null, + dirty: current.dirty ?? null, + pr: current.pr ?? null, + github: current.github ?? null, }) } From 66e89586aeacfbb9ed27e16866e195e2e4cc8d0f Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 15 Mar 2026 16:07:25 -0400 Subject: [PATCH 42/46] feat(pr): generate drafts and improve create flow --- PR-15785-review.md | 70 ++++++ .../app/src/components/dialog-create-pr.tsx | 44 +++- packages/app/src/components/pr-button.tsx | 4 +- packages/app/src/i18n/en.ts | 1 + .../src/pages/layout/sidebar-workspace.tsx | 6 +- packages/app/src/utils/pr-style.ts | 7 +- packages/opencode/src/project/pr.ts | 209 +++++++++++++++++- packages/opencode/src/project/vcs.ts | 46 +++- packages/opencode/src/server/routes/vcs.ts | 25 +++ packages/sdk/js/src/v2/gen/sdk.gen.ts | 42 +++- packages/sdk/js/src/v2/gen/types.gen.ts | 160 +++++++++----- 11 files changed, 551 insertions(+), 63 deletions(-) create mode 100644 PR-15785-review.md diff --git a/PR-15785-review.md b/PR-15785-review.md new file mode 100644 index 000000000000..7da60e7ddfc2 --- /dev/null +++ b/PR-15785-review.md @@ -0,0 +1,70 @@ +# PR 15785 Review + +Scope reviewed: +- GitHub integration backend: [`packages/opencode/src/project/pr.ts`](packages/opencode/src/project/pr.ts), [`packages/opencode/src/project/pr-comments.ts`](packages/opencode/src/project/pr-comments.ts), [`packages/opencode/src/project/vcs.ts`](packages/opencode/src/project/vcs.ts), [`packages/opencode/src/server/routes/vcs.ts`](packages/opencode/src/server/routes/vcs.ts) +- Solid app integration: [`packages/app/src/components/pr-button.tsx`](packages/app/src/components/pr-button.tsx), [`packages/app/src/components/dialog-create-pr.tsx`](packages/app/src/components/dialog-create-pr.tsx), [`packages/app/src/components/dialog-address-comments.tsx`](packages/app/src/components/dialog-address-comments.tsx), [`packages/app/src/context/global-sync/event-reducer.ts`](packages/app/src/context/global-sync/event-reducer.ts) +- Adjacent UI changes that shipped in the same PR: [`packages/app/src/pages/session/message-timeline.tsx`](packages/app/src/pages/session/message-timeline.tsx) + +Validation: +- `packages/opencode`: `bun run typecheck` +- `packages/app`: `bun run typecheck` +- `packages/ui`: `bun run typecheck` + +## Findings + +### 1. High: `vcs.updated` cannot clear stale PR/GitHub state on the client +- Files: + - [`packages/app/src/context/global-sync/event-reducer.ts:261`](packages/app/src/context/global-sync/event-reducer.ts#L261) + - [`packages/opencode/src/server/server.ts:500`](packages/opencode/src/server/server.ts#L500) +- `vcs.updated` is reduced as a partial merge (`{ ...input.store.vcs, ...props }`), but SSE payloads are serialized with `JSON.stringify(event)`. Any field that becomes `undefined` on the server is dropped from the wire entirely. +- That means transitions like "branch with PR" -> "branch without PR", or "authenticated GitHub repo" -> "repo not detected", never clear `pr`, `github`, `branches`, etc. on the app side. The old values remain cached until a hard refresh/bootstrap replaces the whole snapshot. +- User-visible result: stale PR pills/buttons/badges can remain visible after a branch switch or after the PR disappears. + +### 2. High: the commit step in the create-PR dialog does not stage new files +- Files: + - [`packages/opencode/src/project/vcs.ts:237`](packages/opencode/src/project/vcs.ts#L237) + - [`packages/app/src/components/dialog-create-pr.tsx:87`](packages/app/src/components/dialog-create-pr.tsx#L87) +- The new PR flow blocks creation while the worktree is dirty, then offers an in-dialog commit action. That action calls `git add -u`, which only stages tracked-file changes. +- If the user created new files, the commit either omits them or fails to clear the dirty state. The dialog then continues to block PR creation because `dirty > 0`, so the happy path breaks on a common workflow. +- This needs `git add -A` or equivalent behavior if the dialog is going to be presented as "commit current changes, then create PR". + +### 3. Medium: branch push detection and PR creation break for non-`origin` remotes +- Files: + - [`packages/opencode/src/project/vcs.ts:174`](packages/opencode/src/project/vcs.ts#L174) + - [`packages/app/src/components/dialog-create-pr.tsx:27`](packages/app/src/components/dialog-create-pr.tsx#L27) + - [`packages/opencode/src/project/pr.ts:206`](packages/opencode/src/project/pr.ts#L206) +- `fetchBranches()` lists every remote branch, but only strips the `origin/` prefix. A branch tracked on `alice/feature-x` stays `alice/feature-x`, so `isPushed()` compares `feature-x` against `alice/feature-x` and reports "not pushed" even when it already is. +- `PR.create()` then hardcodes `git push -u origin HEAD` instead of respecting the existing upstream/remote. In multi-remote or fork setups this can fail outright or push to the wrong remote. +- This is especially risky because the same PR adds explicit fork-handling elsewhere (`opencode pr `), so the integration now supports multi-remote workflows in one path and breaks them in another. + +### 4. Medium: unresolved comment badges/counts silently undercount large PRs +- File: + - [`packages/opencode/src/project/pr.ts:58`](packages/opencode/src/project/pr.ts#L58) +- `fetchUnresolvedCommentCount()` only queries `reviewThreads(first: 100)` and never paginates. +- The UI uses this count for attention badges, comment menu labels, and workspace indicators. Once a PR crosses 100 review threads, the product starts reporting an incomplete count while still looking authoritative. +- The dialog fetch path already paginates threads; the lightweight count path should do the same or avoid showing an exact count. + +### 5. Medium: timeline staging work was accidentally bypassed +- Files: + - [`packages/app/src/pages/session/message-timeline.tsx:257`](packages/app/src/pages/session/message-timeline.tsx#L257) + - [`packages/app/src/pages/session/message-timeline.tsx:684`](packages/app/src/pages/session/message-timeline.tsx#L684) +- `createTimelineStaging()` is still constructed, but the render loop now iterates `rendered()` instead of `staging.messages()`. +- That removes the defer-mount behavior described by the helper and brings back full DOM mounting for older turns. +- It is not directly part of the GitHub feature, but it ships in the same PR and looks like an accidental regression introduced during the active/queued message work. + +## Testing Gaps + +- I did not run app-level interaction tests or builds. +- There is no targeted automated coverage in this PR for: + - `vcs.updated` state clearing after merge/branch switch + - create-PR flow with untracked files + - fork/multi-remote PR creation + - unresolved comment counts above 100 threads + +## Suggested Fix Order + +1. Fix `vcs.updated` so fields can be cleared deterministically. +2. Fix the commit/create flow to handle untracked files. +3. Make push/branch detection upstream-aware instead of assuming `origin`. +4. Paginate or relax the unresolved-comment count badge. +5. Restore `staging.messages()` in the timeline render path. diff --git a/packages/app/src/components/dialog-create-pr.tsx b/packages/app/src/components/dialog-create-pr.tsx index 507cae0c9e82..127afc1ab554 100644 --- a/packages/app/src/components/dialog-create-pr.tsx +++ b/packages/app/src/components/dialog-create-pr.tsx @@ -47,6 +47,7 @@ export function CreatePrDialog() { body: "", base: defaultBranch(), draft: false, + draftLoading: false, submitting: false, committing: false, commitMessage: branchTitle(), @@ -55,7 +56,9 @@ export function CreatePrDialog() { }) const [titleEdited, setTitleEdited] = createSignal(false) + const [bodyEdited, setBodyEdited] = createSignal(false) const [baseEdited, setBaseEdited] = createSignal(false) + let draftRequest = 0 createEffect(() => { if ((dirty() ?? 0) > 0) { @@ -84,6 +87,35 @@ export function CreatePrDialog() { } }) + createEffect(() => { + const currentBranch = branch() + const base = store.base || defaultBranch() + const dirtyCount = dirty() ?? 0 + if (!currentBranch || !base) return + if (titleEdited() && bodyEdited()) return + + const request = ++draftRequest + setStore("draftLoading", true) + + sdk.client.vcs.pr + .draft({ + directory: sdk.directory, + prDraftInput: { base }, + }) + .then((result) => { + if (request !== draftRequest) return + const draft = result.data + if (!draft) return + if (!titleEdited()) setStore("title", draft.title) + if (!bodyEdited()) setStore("body", draft.body) + }) + .catch(() => {}) + .finally(() => { + if (request !== draftRequest) return + setStore("draftLoading", false) + }) + }) + const handleCommit = async () => { if (!store.commitMessage.trim() || store.committing) return setStore("committing", true) @@ -155,6 +187,13 @@ export function CreatePrDialog() { {store.base || defaultBranch()}
+ +
+ + {language.t("pr.create.generating")} +
+ + 0}>
@@ -215,7 +254,10 @@ export function CreatePrDialog() { setStore("body", e.currentTarget.value)} + onInput={(e) => { + setBodyEdited(true) + setStore("body", e.currentTarget.value) + }} placeholder={language.t("pr.create.field.body.placeholder")} style={{ "min-height": "80px" }} /> diff --git a/packages/app/src/components/pr-button.tsx b/packages/app/src/components/pr-button.tsx index 9d7638223a3b..86e6b0e32733 100644 --- a/packages/app/src/components/pr-button.tsx +++ b/packages/app/src/components/pr-button.tsx @@ -12,7 +12,7 @@ import { usePlatform } from "@/context/platform" import { useSDK } from "@/context/sdk" import { useServer } from "@/context/server" import { useSync } from "@/context/sync" -import { getPrButtonContainerStyle, getPrButtonDividerStyle } from "@/utils/pr-style" +import { getPrButtonContainerStyle, getPrButtonDividerStyle, prRequiresAttention } from "@/utils/pr-style" import { CreatePrDialog } from "./dialog-create-pr" import { MergePrDialog } from "./dialog-merge-pr" import { DeleteBranchDialog } from "./dialog-delete-branch" @@ -195,7 +195,7 @@ export function PrButton() {
- 0}> +
{(currentPr) => { const pillStyle = () => prPillStyle(currentPr()) - const hasUnresolvedComments = () => (currentPr().unresolvedCommentCount ?? 0) > 0 + const requiresAttention = () => prRequiresAttention(currentPr()) return (
PR #{currentPr().number} - +
diff --git a/packages/app/src/utils/pr-style.ts b/packages/app/src/utils/pr-style.ts index de920700395b..bd6a0194edfc 100644 --- a/packages/app/src/utils/pr-style.ts +++ b/packages/app/src/utils/pr-style.ts @@ -4,7 +4,6 @@ export function getPrPillStyle(pr: PrInfo): string { if (pr.state === "MERGED") return "border-[var(--pr-color-merged)]/40 bg-[var(--pr-color-merged)]/15 text-[var(--pr-color-merged-text)]" if (pr.state === "CLOSED") return "border-[var(--pr-color-closed)]/40 bg-[var(--pr-color-closed)]/15 text-[var(--pr-color-closed-text)]" if (pr.isDraft) return "border-[var(--pr-color-draft)]/40 bg-[var(--pr-color-draft)]/15 text-[var(--pr-color-draft-text)]" - if (pr.state === "OPEN" && pr.checksState === "FAILURE") return "border-[var(--pr-color-closed)]/40 bg-[var(--pr-color-closed)]/15 text-[var(--pr-color-closed-text)]" return "border-[var(--pr-color-open)]/40 bg-[var(--pr-color-open)]/15 text-[var(--pr-color-open-text)]" } @@ -23,3 +22,9 @@ export function getPrButtonDividerStyle(pr: PrInfo | undefined): string { if (pr.isDraft) return "bg-[var(--pr-color-draft)]/60" return "bg-[var(--pr-color-open)]/60" } + +export function prRequiresAttention(pr: PrInfo | undefined): boolean { + if (!pr) return false + if (pr.state === "MERGED" || pr.state === "CLOSED") return false + return (pr.unresolvedCommentCount ?? 0) > 0 || pr.checksState === "FAILURE" +} diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index 66e07afdc27f..a8dea8057fde 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -1,6 +1,12 @@ import { $ } from "bun" +import { generateObject, streamObject, type ModelMessage } from "ai" import z from "zod" import { NamedError } from "@opencode-ai/util/error" +import { Auth } from "@/auth" +import { Plugin } from "@/plugin" +import { Provider } from "@/provider/provider" +import { ProviderTransform } from "@/provider/transform" +import { SystemPrompt } from "@/session/system" import { Log } from "@/util/log" import { withTimeout } from "@/util/timeout" import { Instance } from "./instance" @@ -19,6 +25,7 @@ export namespace PR { "DELETE_BRANCH_FAILED", "COMMENTS_FETCH_FAILED", "READY_FAILED", + "DRAFT_FAILED", ]) export type ErrorCode = z.infer @@ -53,8 +60,140 @@ export namespace PR { export const ReadyInput = z.object({}).meta({ ref: "PrReadyInput" }) export type ReadyInput = z.infer + export const DraftInput = z + .object({ + base: z.string().optional(), + }) + .meta({ ref: "PrDraftInput" }) + export type DraftInput = z.infer + + export const DraftOutput = z + .object({ + title: z.string().min(1), + body: z.string().min(1), + }) + .meta({ ref: "PrDraftOutput" }) + export type DraftOutput = z.infer + export const PrErrorResponse = PrError.Schema + const DraftSchema = z.object({ + title: z.string(), + body: z.string(), + }) + + function fallbackTitle(branch: string) { + return branch + .split("/") + .at(-1) + ?.replace(/[-_]/g, " ") + .replace(/\b\w/g, (c) => c.toUpperCase()) + .trim() || "Update branch" + } + + function fallbackBody(title: string) { + return `## Summary +- ${title} + +## Testing +- Not run` + } + + function cleanDraftTitle(title: string, branch: string) { + const line = title + .replace(/[\s\S]*?<\/think>\s*/g, "") + .split("\n") + .map((line) => line.trim()) + .find((line) => line.length > 0) + const next = line?.replace(/[.]+$/g, "").trim() + if (!next) return fallbackTitle(branch) + return next.length > 100 ? next.slice(0, 97).trimEnd() + "..." : next + } + + function cleanDraftBody(body: string, title: string) { + const next = body.replace(/[\s\S]*?<\/think>\s*/g, "").trim() + if (!next) return fallbackBody(title) + return next + } + + async function selectDraftModel() { + const defaultModel = await Provider.defaultModel() + return ( + (await Provider.getSmallModel(defaultModel.providerID).catch(() => undefined)) ?? + (await Provider.getModel(defaultModel.providerID, defaultModel.modelID)) + ) + } + + async function generateDraftObject(input: { branch: string; base: string; commits: string; diffStat: string; diffPatch: string }) { + const model = await selectDraftModel() + const language = await Provider.getLanguage(model) + const system = [ + [ + "You write GitHub pull request drafts.", + "Return a JSON object with keys: title, body.", + "Rules:", + "- title must be concise, specific, and written for a pull request", + "- body must be markdown", + "- body must include headings '## Summary' and '## Testing'", + "- under Summary, use short bullet points", + "- under Testing, use bullet points with concrete checks or 'Not run'", + "- do not invent requirements, screenshots, or test results", + ].join("\n"), + ] + await Plugin.trigger("experimental.chat.system.transform", { model }, { system }) + + const prompt = [ + `Head branch: ${input.branch}`, + `Base branch: ${input.base}`, + "", + "Commits:", + input.commits || "(none)", + "", + "Diff stat:", + input.diffStat || "(none)", + "", + "Diff patch:", + input.diffPatch || "(none)", + ].join("\n") + + const messages: ModelMessage[] = [ + ...system.map((content) => ({ + role: "system" as const, + content, + })), + { + role: "user", + content: prompt, + }, + ] + + const isCodex = model.providerID === "openai" && (await Auth.get(model.providerID))?.type === "oauth" + const params = { + model: language, + schema: DraftSchema, + temperature: 0.2, + messages, + } satisfies Parameters[0] + + if (isCodex) { + const result = streamObject({ + ...params, + providerOptions: ProviderTransform.providerOptions(model, { + instructions: SystemPrompt.instructions(), + store: false, + }), + onError: () => {}, + }) + for await (const part of result.fullStream) { + if (part.type === "error") throw part.error + } + return await result.object + } + + const result = await generateObject(params) + return result.object + } + async function fetchUnresolvedCommentCount( owner: string, name: string, @@ -203,16 +342,84 @@ export namespace PR { return info.pr } + export async function draft(input: DraftInput): Promise { + await Vcs.refresh() + const info = await Vcs.info() + const branch = info.branch + if (!branch) { + throw new PrError({ code: "DRAFT_FAILED", message: "No current branch found" }) + } + + const base = input.base ?? info.defaultBranch ?? "main" + const cwd = Instance.worktree + + const [commits, diffStat, diffPatch] = await Promise.all([ + withTimeout( + $`git log --oneline ${base}..HEAD` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => ""), + 30_000, + ), + withTimeout( + $`git diff --stat ${base}...HEAD` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => ""), + 30_000, + ), + withTimeout( + $`git diff --minimal ${base}...HEAD` + .quiet() + .nothrow() + .cwd(cwd) + .text() + .catch(() => ""), + 30_000, + ), + ]) + + try { + const generated = await generateDraftObject({ + branch, + base, + commits: commits.trim().slice(0, 20_000), + diffStat: diffStat.trim().slice(0, 20_000), + diffPatch: diffPatch.trim().slice(0, 60_000), + }) + const title = cleanDraftTitle(generated.title, branch) + const body = cleanDraftBody(generated.body, title) + return { title, body } + } catch (e) { + log.error("pr draft failed", { error: e }) + throw new PrError({ code: "DRAFT_FAILED", message: "Failed to generate pull request draft" }) + } + } + export async function create(input: CreateInput): Promise { await Vcs.refresh() const { info } = await ensureGithub() const cwd = Instance.worktree + const branch = info.branch if (info.pr) { return info.pr } - const push = await $`git push -u origin HEAD`.quiet().nothrow().cwd(cwd) + if (!branch) { + throw new PrError({ code: "CREATE_FAILED", message: "No current branch found" }) + } + + const remote = await Vcs.resolvePushRemote(branch) + if (!remote) { + throw new PrError({ code: "CREATE_FAILED", message: "No remote configured for current branch" }) + } + + const push = await $`git push -u ${remote} HEAD`.quiet().nothrow().cwd(cwd) if (push.exitCode !== 0) { const errorOutput = push.stderr?.toString().trim() || push.stdout?.toString().trim() || "Failed to push branch automatically" diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index d34e8510e065..ecd028807e56 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -92,6 +92,50 @@ export namespace Vcs { .catch(() => undefined) } + function splitRemoteRef(ref: string) { + const i = ref.indexOf("/") + if (i <= 0) return undefined + return { + remote: ref.slice(0, i), + branch: ref.slice(i + 1), + } + } + + async function gitText(args: string[]) { + return $`${args}` + .quiet() + .nothrow() + .cwd(Instance.worktree) + .text() + .then((x) => x.trim()) + .catch(() => "") + } + + export async function currentUpstream() { + const ref = await gitText(["git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"]) + if (!ref || ref.includes("fatal")) return + return splitRemoteRef(ref) + } + + export async function resolvePushRemote(branch: string) { + const upstream = await currentUpstream() + if (upstream?.remote) return upstream.remote + + const branchRemote = await gitText(["git", "config", `branch.${branch}.pushRemote`]) + if (branchRemote) return branchRemote + + const remote = await gitText(["git", "config", `branch.${branch}.remote`]) + if (remote && remote !== ".") return remote + + const pushDefault = await gitText(["git", "config", "remote.pushDefault"]) + if (pushDefault) return pushDefault + + const remotes = await gitText(["git", "remote"]) + const list = remotes.split("\n").map((x) => x.trim()).filter(Boolean) + if (list.length === 1) return list[0] + if (list.includes("origin")) return "origin" + } + export async function detectGithubCapability(): Promise { const cwd = Instance.worktree const authCmd = await withTimeout( @@ -182,7 +226,7 @@ export namespace Vcs { .split("\n") .map((b) => b.trim()) .filter((b) => b && !b.includes("->")) - .map((b) => b.replace(/^origin\//, "")) + .map((b) => splitRemoteRef(b)?.branch ?? b) .filter((b, i, arr) => arr.indexOf(b) === i) branches.sort((a, b) => a.localeCompare(b)) return branches diff --git a/packages/opencode/src/server/routes/vcs.ts b/packages/opencode/src/server/routes/vcs.ts index 6b5736c7bc8e..d1aacf77eeee 100644 --- a/packages/opencode/src/server/routes/vcs.ts +++ b/packages/opencode/src/server/routes/vcs.ts @@ -131,6 +131,31 @@ export const VcsRoutes = lazy(() => return c.json(data) }, ) + .post( + "/pr/draft", + describeRoute({ + summary: "Generate PR draft", + description: "Generate an AI draft for the current branch's pull request title and body.", + operationId: "vcs.pr.draft", + responses: { + 200: { + description: "Draft PR content", + content: { + "application/json": { + schema: resolver(PR.DraftOutput), + }, + }, + }, + ...errors(400), + }, + }), + validator("json", PR.DraftInput), + async (c) => { + const input = c.req.valid("json") + const draft = await PR.draft(input) + return c.json(draft) + }, + ) .post( "/pr/merge", describeRoute({ diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index 76c4fe6fc4a2..6d734cabf961 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -78,6 +78,7 @@ import type { PermissionRuleset, PrCreateInput, PrDeleteBranchInput, + PrDraftInput, PrMergeInput, ProjectCurrentResponses, ProjectInitGitResponses, @@ -187,6 +188,8 @@ import type { VcsPrCreateResponses, VcsPrDeleteBranchErrors, VcsPrDeleteBranchResponses, + VcsPrDraftErrors, + VcsPrDraftResponses, VcsPrGetResponses, VcsPrMergeErrors, VcsPrMergeResponses, @@ -2994,6 +2997,43 @@ export class Pr extends HeyApiClient { }) } + /** + * Generate PR draft + * + * Generate an AI draft for the current branch's pull request title and body. + */ + public draft( + parameters?: { + directory?: string + workspace?: string + prDraftInput?: PrDraftInput + }, + options?: Options, + ) { + const params = buildClientParams( + [parameters], + [ + { + args: [ + { in: "query", key: "directory" }, + { in: "query", key: "workspace" }, + { key: "prDraftInput", map: "body" }, + ], + }, + ], + ) + return (options?.client ?? this.client).post({ + url: "/vcs/pr/draft", + ...options, + ...params, + headers: { + "Content-Type": "application/json", + ...options?.headers, + ...params.headers, + }, + }) + } + /** * Merge PR * @@ -3071,7 +3111,7 @@ export class Pr extends HeyApiClient { /** * Delete branch * - * Delete a remote (and local) branch after a PR has been merged. + * Delete the remote branch after a PR has been merged. */ public deleteBranch( parameters?: { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 4dd5ae7187ff..2ca7d359eb54 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -882,6 +882,58 @@ export type EventSessionError = { } } +export type EventVcsBranchUpdated = { + type: "vcs.branch.updated" + properties: { + branch?: string + } +} + +export type PrInfo = { + number: number + url: string + title: string + state: "OPEN" | "CLOSED" | "MERGED" + headRefName: string + baseRefName: string + isDraft: boolean + mergeable: "MERGEABLE" | "CONFLICTING" | "UNKNOWN" + reviewDecision?: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null + checksState?: "SUCCESS" | "FAILURE" | "PENDING" | null + checksUrl?: string + checksSummary?: { + total: number + passed: number + failed: number + pending: number + skipped: number + } + unresolvedCommentCount?: number + branchDeleteFailed?: boolean +} + +export type GithubCapability = { + available: boolean + authenticated: boolean + repo?: { + owner: string + name: string + } + host?: string +} + +export type EventVcsUpdated = { + type: "vcs.updated" + properties: { + branch?: string + defaultBranch?: string | null + branches?: Array | null + dirty?: number | null + pr?: PrInfo | null + github?: GithubCapability | null + } +} + export type EventWorkspaceReady = { type: "workspace.ready" properties: { @@ -950,57 +1002,6 @@ export type EventWorktreeFailed = { } } -export type EventVcsBranchUpdated = { - type: "vcs.branch.updated" - properties: { - branch?: string - } -} - -export type PrInfo = { - number: number - url: string - title: string - state: "OPEN" | "CLOSED" | "MERGED" - headRefName: string - baseRefName: string - isDraft: boolean - mergeable: "MERGEABLE" | "CONFLICTING" | "UNKNOWN" - reviewDecision?: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null - checksState?: "SUCCESS" | "FAILURE" | "PENDING" | null - checksUrl?: string - checksSummary?: { - total: number - passed: number - failed: number - pending: number - skipped: number - } - unresolvedCommentCount?: number -} - -export type GithubCapability = { - available: boolean - authenticated: boolean - repo?: { - owner: string - name: string - } - host?: string -} - -export type EventVcsUpdated = { - type: "vcs.updated" - properties: { - branch?: string - defaultBranch?: string - branches?: Array - dirty?: number - pr?: PrInfo - github?: GithubCapability - } -} - export type Event = | EventInstallationUpdated | EventInstallationUpdateAvailable @@ -1038,6 +1039,8 @@ export type Event = | EventSessionDeleted | EventSessionDiff | EventSessionError + | EventVcsBranchUpdated + | EventVcsUpdated | EventWorkspaceReady | EventWorkspaceFailed | EventPtyCreated @@ -1046,8 +1049,6 @@ export type Event = | EventPtyDeleted | EventWorktreeReady | EventWorktreeFailed - | EventVcsBranchUpdated - | EventVcsUpdated export type GlobalEvent = { directory: string @@ -1936,6 +1937,15 @@ export type PrCommentsResponse = { unresolvedCount: number } +export type PrDraftOutput = { + title: string + body: string +} + +export type PrDraftInput = { + base?: string +} + export type PrMergeInput = { strategy?: "merge" | "squash" | "rebase" deleteBranch?: boolean @@ -4446,6 +4456,10 @@ export type VcsPrCommentsErrors = { * Bad request */ 400: BadRequestError + /** + * Not found + */ + 404: NotFoundError } export type VcsPrCommentsError = VcsPrCommentsErrors[keyof VcsPrCommentsErrors] @@ -4459,6 +4473,34 @@ export type VcsPrCommentsResponses = { export type VcsPrCommentsResponse = VcsPrCommentsResponses[keyof VcsPrCommentsResponses] +export type VcsPrDraftData = { + body?: PrDraftInput + path?: never + query?: { + directory?: string + workspace?: string + } + url: "/vcs/pr/draft" +} + +export type VcsPrDraftErrors = { + /** + * Bad request + */ + 400: BadRequestError +} + +export type VcsPrDraftError = VcsPrDraftErrors[keyof VcsPrDraftErrors] + +export type VcsPrDraftResponses = { + /** + * Draft PR content + */ + 200: PrDraftOutput +} + +export type VcsPrDraftResponse = VcsPrDraftResponses[keyof VcsPrDraftResponses] + export type VcsPrMergeData = { body?: PrMergeInput path?: never @@ -4474,6 +4516,10 @@ export type VcsPrMergeErrors = { * Bad request */ 400: BadRequestError + /** + * Not found + */ + 404: NotFoundError } export type VcsPrMergeError = VcsPrMergeErrors[keyof VcsPrMergeErrors] @@ -4502,6 +4548,10 @@ export type VcsPrReadyErrors = { * Bad request */ 400: BadRequestError + /** + * Not found + */ + 404: NotFoundError } export type VcsPrReadyError = VcsPrReadyErrors[keyof VcsPrReadyErrors] @@ -4530,6 +4580,10 @@ export type VcsPrDeleteBranchErrors = { * Bad request */ 400: BadRequestError + /** + * Not found + */ + 404: NotFoundError } export type VcsPrDeleteBranchError = VcsPrDeleteBranchErrors[keyof VcsPrDeleteBranchErrors] From 29e09d26c7922258722ce2f7f5ab31fab40fb0ad Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 15 Mar 2026 17:58:55 -0400 Subject: [PATCH 43/46] chore: remove temporary PR review notes --- PR-15785-review.md | 70 ------- PR-REVIEW.md | 499 --------------------------------------------- 2 files changed, 569 deletions(-) delete mode 100644 PR-15785-review.md delete mode 100644 PR-REVIEW.md diff --git a/PR-15785-review.md b/PR-15785-review.md deleted file mode 100644 index 7da60e7ddfc2..000000000000 --- a/PR-15785-review.md +++ /dev/null @@ -1,70 +0,0 @@ -# PR 15785 Review - -Scope reviewed: -- GitHub integration backend: [`packages/opencode/src/project/pr.ts`](packages/opencode/src/project/pr.ts), [`packages/opencode/src/project/pr-comments.ts`](packages/opencode/src/project/pr-comments.ts), [`packages/opencode/src/project/vcs.ts`](packages/opencode/src/project/vcs.ts), [`packages/opencode/src/server/routes/vcs.ts`](packages/opencode/src/server/routes/vcs.ts) -- Solid app integration: [`packages/app/src/components/pr-button.tsx`](packages/app/src/components/pr-button.tsx), [`packages/app/src/components/dialog-create-pr.tsx`](packages/app/src/components/dialog-create-pr.tsx), [`packages/app/src/components/dialog-address-comments.tsx`](packages/app/src/components/dialog-address-comments.tsx), [`packages/app/src/context/global-sync/event-reducer.ts`](packages/app/src/context/global-sync/event-reducer.ts) -- Adjacent UI changes that shipped in the same PR: [`packages/app/src/pages/session/message-timeline.tsx`](packages/app/src/pages/session/message-timeline.tsx) - -Validation: -- `packages/opencode`: `bun run typecheck` -- `packages/app`: `bun run typecheck` -- `packages/ui`: `bun run typecheck` - -## Findings - -### 1. High: `vcs.updated` cannot clear stale PR/GitHub state on the client -- Files: - - [`packages/app/src/context/global-sync/event-reducer.ts:261`](packages/app/src/context/global-sync/event-reducer.ts#L261) - - [`packages/opencode/src/server/server.ts:500`](packages/opencode/src/server/server.ts#L500) -- `vcs.updated` is reduced as a partial merge (`{ ...input.store.vcs, ...props }`), but SSE payloads are serialized with `JSON.stringify(event)`. Any field that becomes `undefined` on the server is dropped from the wire entirely. -- That means transitions like "branch with PR" -> "branch without PR", or "authenticated GitHub repo" -> "repo not detected", never clear `pr`, `github`, `branches`, etc. on the app side. The old values remain cached until a hard refresh/bootstrap replaces the whole snapshot. -- User-visible result: stale PR pills/buttons/badges can remain visible after a branch switch or after the PR disappears. - -### 2. High: the commit step in the create-PR dialog does not stage new files -- Files: - - [`packages/opencode/src/project/vcs.ts:237`](packages/opencode/src/project/vcs.ts#L237) - - [`packages/app/src/components/dialog-create-pr.tsx:87`](packages/app/src/components/dialog-create-pr.tsx#L87) -- The new PR flow blocks creation while the worktree is dirty, then offers an in-dialog commit action. That action calls `git add -u`, which only stages tracked-file changes. -- If the user created new files, the commit either omits them or fails to clear the dirty state. The dialog then continues to block PR creation because `dirty > 0`, so the happy path breaks on a common workflow. -- This needs `git add -A` or equivalent behavior if the dialog is going to be presented as "commit current changes, then create PR". - -### 3. Medium: branch push detection and PR creation break for non-`origin` remotes -- Files: - - [`packages/opencode/src/project/vcs.ts:174`](packages/opencode/src/project/vcs.ts#L174) - - [`packages/app/src/components/dialog-create-pr.tsx:27`](packages/app/src/components/dialog-create-pr.tsx#L27) - - [`packages/opencode/src/project/pr.ts:206`](packages/opencode/src/project/pr.ts#L206) -- `fetchBranches()` lists every remote branch, but only strips the `origin/` prefix. A branch tracked on `alice/feature-x` stays `alice/feature-x`, so `isPushed()` compares `feature-x` against `alice/feature-x` and reports "not pushed" even when it already is. -- `PR.create()` then hardcodes `git push -u origin HEAD` instead of respecting the existing upstream/remote. In multi-remote or fork setups this can fail outright or push to the wrong remote. -- This is especially risky because the same PR adds explicit fork-handling elsewhere (`opencode pr `), so the integration now supports multi-remote workflows in one path and breaks them in another. - -### 4. Medium: unresolved comment badges/counts silently undercount large PRs -- File: - - [`packages/opencode/src/project/pr.ts:58`](packages/opencode/src/project/pr.ts#L58) -- `fetchUnresolvedCommentCount()` only queries `reviewThreads(first: 100)` and never paginates. -- The UI uses this count for attention badges, comment menu labels, and workspace indicators. Once a PR crosses 100 review threads, the product starts reporting an incomplete count while still looking authoritative. -- The dialog fetch path already paginates threads; the lightweight count path should do the same or avoid showing an exact count. - -### 5. Medium: timeline staging work was accidentally bypassed -- Files: - - [`packages/app/src/pages/session/message-timeline.tsx:257`](packages/app/src/pages/session/message-timeline.tsx#L257) - - [`packages/app/src/pages/session/message-timeline.tsx:684`](packages/app/src/pages/session/message-timeline.tsx#L684) -- `createTimelineStaging()` is still constructed, but the render loop now iterates `rendered()` instead of `staging.messages()`. -- That removes the defer-mount behavior described by the helper and brings back full DOM mounting for older turns. -- It is not directly part of the GitHub feature, but it ships in the same PR and looks like an accidental regression introduced during the active/queued message work. - -## Testing Gaps - -- I did not run app-level interaction tests or builds. -- There is no targeted automated coverage in this PR for: - - `vcs.updated` state clearing after merge/branch switch - - create-PR flow with untracked files - - fork/multi-remote PR creation - - unresolved comment counts above 100 threads - -## Suggested Fix Order - -1. Fix `vcs.updated` so fields can be cleared deterministically. -2. Fix the commit/create flow to handle untracked files. -3. Make push/branch detection upstream-aware instead of assuming `origin`. -4. Paginate or relax the unresolved-comment count badge. -5. Restore `staging.messages()` in the timeline render path. diff --git a/PR-REVIEW.md b/PR-REVIEW.md deleted file mode 100644 index e1b0262e0ce9..000000000000 --- a/PR-REVIEW.md +++ /dev/null @@ -1,499 +0,0 @@ -# Code Review: feat/github-pr-integration - -**Branch:** `feat/github-pr-integration` (based off `dev`) -**Scope:** ~3,300 lines added across 25 files — backend PR operations, SDK generation, and frontend UI. - ---- - -## Issues - -### 1. ~~Branch name regex allows path traversal in later segments~~ COMPLETED - -**Severity:** High | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:53` - -The `DeleteBranchInput` regex `^(?![^/]*\.\.)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$` uses a negative lookahead that only checks the first path segment (before the first `/`). Multi-segment paths containing `..` pass validation: - -``` -"a/b/../c" → passes (should fail) -"a/b/../../etc/passwd" → passes (should fail) -``` - -While Bun's `$` template literal prevents command injection and git treats refnames literally, the stated intent is to block `..` traversal and the regex fails to do so. - -**Fix:** Use a regex that rejects `..` anywhere: - -```ts -branch: z.string().regex(/^(?!.*\.\.)(?!.*\/\/)(?!\/)(?!.*\/$)[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/, "Invalid branch name") -``` - -Or use a simpler `.refine()` check: `.refine(s => !s.includes('..'), "Invalid branch name")`. - ---- - -### 2. ~~`ensureGithub()` returns `Required` but `repo` can be undefined~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:195` - -`ensureGithub()` casts the return to `Required`, claiming `repo` and `host` are always present. But `detectGithubCapability()` can return `{ available: true, authenticated: true }` without `repo` when `gh repo view` fails (e.g., detached worktree, network issues, non-GitHub remote). - -The `merge()` function safely re-checks `github?.repo` at line 305, but `create()` at line 204 destructures `{ info }` and never checks `info.github.repo` — it doesn't use `repo` directly, so it's safe by accident. Future code relying on the `Required<>` type won't be safe. - -**Fix:** Don't cast to `Required<>`. Instead, return the narrowed type: - -```ts -return { info, github: github as Vcs.GithubCapability & { available: true; authenticated: true } } -``` - -Or add a `repo` check to `ensureGithub` and return a properly narrowed type. - ---- - -### 3. ~~`CreateInput` schema lacks `min(1)` on title — empty string passes validation~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:35` - -`title: z.string()` allows empty strings. The frontend guards against this (`!store.title.trim()`), but the API endpoint doesn't. A direct API call with `title: ""` would attempt to create a PR with an empty title. - -**Fix:** Add a `min(1)` constraint and reasonable max limits (GitHub's API doesn't document explicit limits, but practical limits exist): - -```ts -title: z.string().min(1).max(1024), -body: z.string().optional(), -``` - -The `min(1)` on `title` is the essential fix. Max limits are a safeguard against accidental abuse. - ---- - -### 4. ~~Race condition in `PR.create()` — TOCTOU on `info.pr`~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:207-209` - -`create()` checks `info.pr` to short-circuit if a PR already exists, but `info` comes from cached VCS state. Two concurrent `create()` calls could both see `info.pr` as `undefined`, push to origin, and both call `gh pr create`. The second call would fail with a `gh` error (GitHub rejects duplicate PRs), which is handled, but the push would have already happened. - -The `POST /pr` route is documented as "Idempotent — returns existing PR if one exists" but this is only true at the state cache level, not at the git/GitHub level. - -**Fix:** This is low-risk since `gh` is the final arbiter, but if true idempotency is desired, refresh VCS state before the check: - -```ts -await Vcs.refresh() -const { info } = await ensureGithub() -``` - ---- - -### 5. ~~Event reducer omits `branches` field from `vcs.updated` event type~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/app/src/context/global-sync/event-reducer.ts:262-268` - -The `vcs.updated` event type cast omits `branches`: - -```ts -const props = event.properties as { - branch?: string - defaultBranch?: string - dirty?: number - pr?: VcsInfo["pr"] - github?: VcsInfo["github"] - // missing: branches?: string[] -} -``` - -The server publishes `branches` in the event (`vcs.ts:293`). At runtime the `...props` spread will include it, but the explicit type annotation is incomplete and misleading for future maintainers. - -**Fix:** Add `branches?: string[]` to the type annotation. - ---- - -### 6. ~~Route error handling diverges from project convention~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/server/routes/vcs.ts` (all handlers) - -Every other route file in the project (session, project, file, config, etc.) lets errors propagate to the global `onError` handler in `server.ts`. The VCS routes use local try/catch blocks instead. This is because `PrError` extends `Error` (not `NamedError`), so the global handler would return 500 for what should be 400. - -**Fix:** `NamedError` uses a static `create()` factory (not a constructable base class), so the integration pattern is different from a simple `extends`. Create a named error via the factory and add a status mapping to the global handler: - -```ts -// In pr.ts — create a NamedError subclass via the factory -export const PrError = NamedError.create("PrError", z.object({ code: ErrorCode, message: z.string() })) -export type PrError = InstanceType -``` - -Then in `server.ts`, add a clause to the global `onError` handler: - -```ts -else if (err instanceof PrError) status = err.data.code === "NO_PR" ? 404 : 400 -``` - -This lets you remove all try/catch blocks from the VCS route handlers, aligning with the project's error handling convention. - -**Note:** This changes how `PrError` is constructed — all `throw new PrError(code, msg)` calls would become `throw new PrError({ code, message: msg })`. - ---- - -### 7. ~~`clearInterval` used on `setTimeout` handle~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/vcs.ts:364` - -The cleanup function uses `clearInterval(pollTimer)` but `pollTimer` is set via `setTimeout` (line 273). While this works in practice (both clear the same timer queue in Bun/Node), it's semantically incorrect. - -**Fix:** Change to `clearTimeout(pollTimer)`. - ---- - -### 8. ~~`address-comments` prompt includes unsanitized comment body text~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/app/src/components/dialog-address-comments.tsx:100` - -Review comment bodies from GitHub are interpolated directly into the agent prompt: - -```ts -text += `**@${comment.author}** (comment ID: ${comment.id}): ${comment.body}\n` -``` - -A malicious review comment could contain text designed to manipulate the agent (prompt injection). This is inherent to the feature design (the whole point is to pass comments to the agent), but worth noting that there's no sanitization or structural separation between the instruction portion and the user-controlled content. - -**Fix:** Consider wrapping user-controlled content in a fenced block: - -```ts -text += `**@${comment.author}** (comment ID: ${comment.id}):\n\`\`\`\n${comment.body}\n\`\`\`\n` -``` - -This doesn't prevent prompt injection but provides structural separation. - ---- - -### 9. ~~`PR.create()` returns `info.branch` which may be stale~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:254` - -In the fallback return (lines 249-261), `headRefName` is set to `info.branch` — the branch name from the cached VCS state snapshot taken _before_ the push and PR creation. If the branch name somehow changed between the state read and the PR creation, this would be wrong. - -This is extremely unlikely in practice, but for consistency with the rest of the function (which refreshes state via `Vcs.refresh()`), the branch should come from the refreshed state. - -**Fix:** Use `(await Vcs.info()).branch` or move the fallback before the `Vcs.refresh()` call. - ---- - -### 10. ~~`fetchForBranch` silently returns `undefined` on `gh pr view` stderr errors~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:96-101` - -`gh pr view` is called with `.nothrow()`, and if the command writes to stderr (e.g., auth errors, rate limiting), the output goes to `result` only if it's on stdout. The function checks `if (!result.trim()) return undefined` — but on error, `gh` may write nothing to stdout and everything to stderr, causing a silent `undefined` return with no logging for what might be a transient error. - -The outer `catch` at line 180-183 does log, but this path (empty stdout, non-zero exit code) hits the early return at line 101-102, not the catch. - -**Fix:** Remove `.text()` to get the full result object, then check `exitCode` before parsing: - -```ts -const result = - await $`gh pr view --json number,url,title,state,headRefName,baseRefName,isDraft,mergeable,reviewDecision,statusCheckRollup` - .quiet() - .nothrow() - .cwd(cwd) -if (result.exitCode !== 0) { - log.warn("gh pr view failed", { stderr: result.stderr.toString() }) - return undefined -} -const text = result.stdout.toString().trim() -if (!text) return undefined -const parsed = JSON.parse(text) -``` - ---- - -### 11. Hardcoded GitHub status colors bypass design system tokens - -**Severity:** Low -**File:** `packages/app/src/utils/pr-style.ts:4-8` - -The PR pill and button styles use hardcoded hex colors (`#8957e5`, `#da3633`, `#238636`, `#768390`) rather than design system tokens. This will clash with custom themes and doesn't respect the existing `text-icon-success-base` / `text-icon-critical-base` token pattern used elsewhere (e.g., `pr-button.tsx:109-111`). - -**Fix:** Use the project's semantic tokens: - -```ts -if (pr.state === "MERGED") return "border-border-info-base/40 bg-surface-info-weak text-text-info" -if (pr.state === "CLOSED") return "border-border-critical-base/40 bg-surface-critical-weak text-text-danger" -``` - ---- - -### 12. ~~`PR.create()` idempotency check doesn't refresh before reading~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:207` - -The idempotency check `if (info.pr) return info.pr` reads from cached state without refreshing. If a PR was created outside opencode (e.g., on github.com) since the last poll, the cached state won't know about it, and `create()` will attempt to create a duplicate (which `gh` will reject). - -This is the same issue as #4 but from the perspective of external PR creation. Low severity because `gh pr create` would return a clear error. - -**Fix:** Call `await Vcs.refresh()` before the idempotency check, or document that the idempotency is best-effort. - ---- - -### 13. ~~Merge doesn't verify PR is mergeable before attempting~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:295-310` - -The `merge()` function doesn't check `currentPr.mergeable` before calling the GitHub merge API. The frontend shows a conflict warning and disables the button, but the backend doesn't enforce it. A direct API call can attempt to merge a PR with conflicts. - -```ts -const currentPr = await get() -if (!currentPr) { - throw new PrError("NO_PR", "...") -} -// No check for mergeable === "CONFLICTING" -const strategy = input.strategy ?? "squash" -``` - -**Fix:** Add server-side validation: - -```ts -if (currentPr.mergeable === "CONFLICTING") { - throw new PrError("MERGE_FAILED", "PR has merge conflicts that must be resolved first") -} -``` - ---- - -### 14. ~~Wrong HTTP status code for `NO_PR` errors~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/server/routes/vcs.ts:109-114` - -All `PrError` codes return 400, but `NO_PR` is semantically a 404 (resource not found). Also, `GH_NOT_INSTALLED` and `GH_NOT_AUTHENTICATED` are client-side prerequisite failures — 400 is acceptable (the request is malformed given the current environment), though 424 (Failed Dependency) could also fit. - -**Fix:** Map error codes to appropriate statuses: - -```ts -const status = e.code === "NO_PR" ? 404 : 400 -return c.json({ code: e.code, message: e.message }, { status }) -``` - ---- - -### 15. ~~No timeout on GitHub CLI commands~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** Multiple (`pr.ts`, `pr-comments.ts`, `vcs.ts`) - -All `gh` subprocess calls have no timeout. On slow networks, GitHub Enterprise with rate limiting, or when `gh` hangs waiting for input, these commands block indefinitely. - -```ts -const result = await $`gh pr view --json ...`.quiet().nothrow().cwd(cwd).text() -``` - -**Fix:** Bun's `$` shell does **not** have a `.timeout()` method. Use `Promise.race()` instead: - -```ts -function withTimeout(promise: Promise, ms: number): Promise { - return Promise.race([promise, new Promise((_, reject) => setTimeout(() => reject(new Error("timeout")), ms))]) -} - -const result = await withTimeout($`gh pr view --json ...`.quiet().nothrow().cwd(cwd).text(), 30_000) -``` - -Alternatively, consider using `Bun.spawn()` which supports a `timeout` option, though that would require a larger refactor. - ---- - -### 16. ~~Error messages may expose sensitive data~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:214-216, 233-235` - -Raw stderr/stdout from `git push` and `gh pr create` is passed directly into `PrError` messages and returned to clients. These may contain file paths, environment variables, or token fragments depending on git configuration and hooks. - -```ts -const errorOutput = push.stderr?.toString().trim() || push.stdout?.toString().trim() -throw new PrError("CREATE_FAILED", errorOutput) -``` - -**Fix:** Truncate and sanitize error output: - -```ts -function sanitize(output: string): string { - return output.replace(/(ghp_|github_pat_)[a-zA-Z0-9_]+/g, "").slice(0, 500) -} -``` - ---- - -### 17. ~~Branch deletion after merge — silent failure and ordering risk~~ COMPLETED - -**Severity:** Medium | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:345-354` - -If merge succeeds but branch deletion fails (network timeout, permissions), the error is silently swallowed with only a `log.warn`. The user gets a success response but the branch isn't cleaned up, and there's no UI indication of the partial failure. - -```ts -try { - await deleteBranch({ branch: branchToDelete }) -} catch (e) { - log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) -} -``` - -**Fix:** Return a response that indicates partial success, or surface the branch deletion failure to the client: - -```ts -const result: Vcs.PrInfo & { branchDeleteFailed?: boolean } = updated.pr ?? { ...currentPr, state: "MERGED" } -try { - await deleteBranch({ branch: branchToDelete }) -} catch (e) { - log.warn("post-merge branch deletion failed", { branch: branchToDelete, error: e }) - result.branchDeleteFailed = true -} -``` - -**Note:** The `branchDeleteFailed` field would also need to be added to the `Vcs.PrInfo` Zod schema (or returned as a separate response type) to be properly typed and serialized through the API. - ---- - -### 18. ~~PrComments pagination has no upper bound~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/pr-comments.ts:93-167` - -The `do...while` pagination loop has no page limit. On PRs with hundreds of review threads, this could issue many sequential GraphQL requests with no cap. - -**Fix:** Add a maximum page count: - -```ts -const MAX_PAGES = 10 -let page = 0 -do { - if (++page > MAX_PAGES) { - log.warn("pr-comments: max pages reached, truncating") - break - } - // ... -} while (cursor) -``` - ---- - -### 19. ~~Inconsistent error handling in delete-branch dialog~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/app/src/components/dialog-delete-branch.tsx:42-47` - -The delete-branch dialog catches errors with a generic toast, unlike every other dialog which uses `resolveApiErrorMessage` to extract structured error context. - -```ts -} catch { - showToast({ - variant: "error", - icon: "circle-x", - title: language.t("pr.error.delete_branch_failed"), - }) -} -``` - -**Fix:** Use consistent error handling matching the other dialogs: - -```ts -} catch (e: unknown) { - showToast({ - variant: "error", - icon: "circle-x", - title: resolveApiErrorMessage(e, language.t("pr.error.delete_branch_failed"), (k) => - language.t(k as Parameters[0]) - ), - }) -} -``` - ---- - -### 20. ~~`remoteBranchExists` defaults to `true` while data is loading~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/app/src/components/pr-button.tsx:70-75` - -```ts -const remoteBranchExists = createMemo(() => { - const branchName = pr()?.headRefName - const branches = vcs()?.branches - if (!branchName || !branches) return true // defaults to true - return branches.includes(branchName) -}) -``` - -When branch data hasn't loaded yet, this defaults to `true`, which could briefly show the "Delete branch" menu item for a merged PR even if the branch is already gone. - -**Fix:** Default to `undefined` or `false` when data isn't available, and guard the UI accordingly. - ---- - -### 21. Stale PR data in delete branch dialog - -**Severity:** Low -**File:** `packages/app/src/components/dialog-delete-branch.tsx:26` - -The dialog reads `pr()` from the sync context, which may be stale (e.g., user has multiple tabs, VCS poll hasn't fired). The `headRefName` used for deletion could theoretically reference a branch that no longer matches the current state. - -**Fix:** Fetch fresh VCS state on dialog mount, or show the branch name prominently with a confirmation step (the dialog already shows the branch name, so this is partially addressed). - ---- - -### 22. ~~No validation that base branch exists before PR creation~~ COMPLETED - -**Severity:** Low | **Status:** Fixed -**File:** `packages/opencode/src/project/pr.ts:221` - -```ts -if (input.base) args.push("--base", input.base) -``` - -If a user specifies a non-existent base branch, they get an opaque error from `gh pr create` rather than a clear validation error. - -**Fix:** Validate against known branches: - -```ts -if (input.base) { - const branches = await Vcs.fetchBranches() - if (!branches.includes(input.base)) { - throw new PrError("CREATE_FAILED", `Base branch '${input.base}' does not exist`) - } - args.push("--base", input.base) -} -``` - ---- - -## Summary - -The implementation is well-structured and follows most project conventions. 22 issues identified: - -| Severity | Count | Key items | -| -------- | ----- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| High | 1 | Branch name regex (#1) | -| Medium | 10 | Type safety (#2), input validation (#3), race conditions (#4, #5), conventions (#6), merge validation (#13), HTTP status (#14), timeouts (#15), error exposure (#16), merge+delete ordering (#17) | -| Low | 11 | Timer mismatch (#7), prompt injection (#8), stale state (#9, #12, #21), silent errors (#10), hardcoded colors (#11), pagination bounds (#18), inconsistent error handling (#19), loading defaults (#20), base branch validation (#22) | - -**Must fix before merge:** ~~#1 (regex)~~, ~~#3 (input validation)~~, ~~#13 (merge validation)~~, ~~#14 (HTTP status codes)~~ — all done. -**Should fix:** ~~#2~~, ~~#5~~, ~~#6~~, ~~#15~~, ~~#16~~, ~~#17~~ — all done. -**Can follow up:** ~~#7~~, ~~#8~~, ~~#9~~, ~~#10~~, #11, ~~#12~~, ~~#18~~, ~~#19~~, ~~#20~~, #21, ~~#22~~ — 2 remaining (#11, #21). - ---- - -## Verification Notes - -All 22 fix recommendations were verified against the source code. **4 corrections were applied:** - -1. **Fix #3** — Removed specific GitHub title/body max limits (256/65536) as they are not documented in the GitHub REST API. Changed to recommend `min(1)` (essential) with reasonable safeguard max. -2. **Fix #6** — Rewrote the `NamedError` integration. `NamedError` is an abstract class with a `static create()` factory pattern, not a direct `extends` target. The fix now shows the correct `NamedError.create()` pattern. -3. **Fix #14** — Removed 503 suggestion for `GH_NOT_INSTALLED`/`GH_NOT_AUTHENTICATED`. These are client-side prerequisite failures, not server unavailability. Changed to 400 (or 424 as alternative). -4. **Fix #15** — Bun's `$` shell does **not** have a `.timeout()` method (confirmed at runtime). Changed to recommend `Promise.race()` pattern or `Bun.spawn()` which does support timeout. From 61be64bd092bb46a18a4a286600afaa619a639d6 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 15 Mar 2026 22:02:22 -0400 Subject: [PATCH 44/46] fix(vcs): ignore detached head during bootstrap --- packages/opencode/src/project/vcs.ts | 10 +- packages/opencode/test/project/vcs.test.ts | 130 +++------------------ 2 files changed, 19 insertions(+), 121 deletions(-) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index ecd028807e56..f29a18ddeb57 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -83,13 +83,9 @@ export namespace Vcs { export type Info = z.infer async function currentBranch() { - return $`git rev-parse --abbrev-ref HEAD` - .quiet() - .nothrow() - .cwd(Instance.worktree) - .text() - .then((x) => x.trim()) - .catch(() => undefined) + const text = await gitText(["git", "rev-parse", "--abbrev-ref", "HEAD"]) + if (!text || text === "HEAD") return + return text } function splitRemoteRef(ref: string) { diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts index 11463b795023..65a3ad7bf806 100644 --- a/packages/opencode/test/project/vcs.test.ts +++ b/packages/opencode/test/project/vcs.test.ts @@ -1,125 +1,27 @@ -import { $ } from "bun" import { afterEach, describe, expect, test } from "bun:test" -import fs from "fs/promises" -import path from "path" -import { Effect, Layer, ManagedRuntime } from "effect" -import { tmpdir } from "../fixture/fixture" -import { watcherConfigLayer, withServices } from "../fixture/instance" -import { FileWatcher } from "../../src/file/watcher" +import { $ } from "bun" import { Instance } from "../../src/project/instance" -import { GlobalBus } from "../../src/bus/global" import { Vcs } from "../../src/project/vcs" +import { tmpdir } from "../fixture/fixture" -// Skip in CI — native @parcel/watcher binding needed -const describeVcs = FileWatcher.hasNativeBinding() && !process.env.CI ? describe : describe.skip - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -function withVcs( - directory: string, - body: (rt: ManagedRuntime.ManagedRuntime) => Promise, -) { - return withServices( - directory, - Layer.merge(FileWatcher.layer, Vcs.layer), - async (rt) => { - await rt.runPromise(FileWatcher.Service.use((s) => s.init())) - await rt.runPromise(Vcs.Service.use((s) => s.init())) - await Bun.sleep(500) - await body(rt) - }, - { provide: [watcherConfigLayer] }, - ) -} - -type BranchEvent = { directory?: string; payload: { type: string; properties: { branch?: string } } } - -/** Wait for a Vcs.Event.BranchUpdated event on GlobalBus, with retry polling as fallback */ -function nextBranchUpdate(directory: string, timeout = 10_000) { - return new Promise((resolve, reject) => { - let settled = false - - const timer = setTimeout(() => { - if (settled) return - settled = true - GlobalBus.off("event", on) - reject(new Error("timed out waiting for BranchUpdated event")) - }, timeout) - - function on(evt: BranchEvent) { - if (evt.directory !== directory) return - if (evt.payload.type !== Vcs.Event.BranchUpdated.type) return - if (settled) return - settled = true - clearTimeout(timer) - GlobalBus.off("event", on) - resolve(evt.payload.properties.branch) - } - - GlobalBus.on("event", on) - }) -} - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describeVcs("Vcs", () => { - afterEach(async () => { - await Instance.disposeAll() - }) - - test("branch() returns current branch name", async () => { - await using tmp = await tmpdir({ git: true }) - - await withVcs(tmp.path, async (rt) => { - const branch = await rt.runPromise(Vcs.Service.use((s) => s.branch())) - expect(branch).toBeDefined() - expect(typeof branch).toBe("string") - }) - }) - - test("branch() returns undefined for non-git directories", async () => { - await using tmp = await tmpdir() - - await withVcs(tmp.path, async (rt) => { - const branch = await rt.runPromise(Vcs.Service.use((s) => s.branch())) - expect(branch).toBeUndefined() - }) - }) +afterEach(async () => { + await Instance.disposeAll() +}) - test("publishes BranchUpdated when .git/HEAD changes", async () => { +describe("Vcs.info", () => { + test("returns no branch data when git is detached at HEAD", async () => { await using tmp = await tmpdir({ git: true }) - const branch = `test-${Math.random().toString(36).slice(2)}` - await $`git branch ${branch}`.cwd(tmp.path).quiet() + await $`git checkout --detach HEAD`.cwd(tmp.path).quiet() - await withVcs(tmp.path, async () => { - const pending = nextBranchUpdate(tmp.path) - - const head = path.join(tmp.path, ".git", "HEAD") - await fs.writeFile(head, `ref: refs/heads/${branch}\n`) - - const updated = await pending - expect(updated).toBe(branch) + const info = await Instance.provide({ + directory: tmp.path, + fn: async () => Vcs.info(), }) - }) - - test("branch() reflects the new branch after HEAD change", async () => { - await using tmp = await tmpdir({ git: true }) - const branch = `test-${Math.random().toString(36).slice(2)}` - await $`git branch ${branch}`.cwd(tmp.path).quiet() - - await withVcs(tmp.path, async (rt) => { - const pending = nextBranchUpdate(tmp.path) - const head = path.join(tmp.path, ".git", "HEAD") - await fs.writeFile(head, `ref: refs/heads/${branch}\n`) - - await pending - const current = await rt.runPromise(Vcs.Service.use((s) => s.branch())) - expect(current).toBe(branch) - }) + expect(info.branch).toBe("") + expect(info.defaultBranch).toBeUndefined() + expect(info.branches).toBeUndefined() + expect(info.github).toBeUndefined() + expect(info.pr).toBeUndefined() }) }) From 5216cf6111da90aebe5ab4f2bab9da5e46ac41b6 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sun, 15 Mar 2026 22:06:24 -0400 Subject: [PATCH 45/46] fix(db): preserve channel database path --- packages/opencode/src/index.ts | 2 +- packages/opencode/src/storage/db.ts | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index b3d1db7eb0cb..beff492f8b42 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -84,7 +84,7 @@ let cli = yargs(hideBin(process.argv)) args: process.argv.slice(2), }) - const marker = path.join(Global.Path.data, "opencode.db") + const marker = Database.Path if (!(await Filesystem.exists(marker))) { const tty = process.stderr.isTTY process.stderr.write("Performing one time database migration, may take a few minutes..." + EOL) diff --git a/packages/opencode/src/storage/db.ts b/packages/opencode/src/storage/db.ts index 1bb8c1a69bf3..86c72ef7e14f 100644 --- a/packages/opencode/src/storage/db.ts +++ b/packages/opencode/src/storage/db.ts @@ -32,11 +32,18 @@ export namespace Database { if (path.isAbsolute(Flag.OPENCODE_DB)) return Flag.OPENCODE_DB return path.join(Global.Path.data, Flag.OPENCODE_DB) } + const legacy = path.join(Global.Path.data, "opencode.db") const channel = Installation.CHANNEL - if (["latest", "beta"].includes(channel) || Flag.OPENCODE_DISABLE_CHANNEL_DB) - return path.join(Global.Path.data, "opencode.db") const safe = channel.replace(/[^a-zA-Z0-9._-]/g, "-") - return path.join(Global.Path.data, `opencode-${safe}.db`) + const next = path.join(Global.Path.data, `opencode-${safe}.db`) + const preview = Installation.VERSION.startsWith("0.0.0-") + if (Flag.OPENCODE_DISABLE_CHANNEL_DB) return legacy + if (["latest", "beta", "local", "dev"].includes(channel) || preview) { + if (existsSync(legacy) || !existsSync(next)) return legacy + return next + } + if (existsSync(next) || !existsSync(legacy)) return next + return legacy }) export type Transaction = SQLiteTransaction<"sync", void> From e6ffbd3494aa674cf57e4cdd761117be778f73f2 Mon Sep 17 00:00:00 2001 From: Tamara Zuk Date: Sat, 21 Mar 2026 17:49:03 -0400 Subject: [PATCH 46/46] fix(pr): restore rebase type safety --- packages/opencode/src/cli/cmd/pr.ts | 6 +++--- packages/opencode/src/project/pr.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/cli/cmd/pr.ts b/packages/opencode/src/cli/cmd/pr.ts index d0ea3ae02295..532512943da0 100644 --- a/packages/opencode/src/cli/cmd/pr.ts +++ b/packages/opencode/src/cli/cmd/pr.ts @@ -32,7 +32,7 @@ export const PrCommand = cmd({ // Use gh pr checkout with custom branch name const result = await withTimeout($`gh pr checkout ${prNumber} --branch ${localBranchName} --force`.nothrow(), 30_000) - if (result.code !== 0) { + if (result.exitCode !== 0) { UI.error(`Failed to checkout PR #${prNumber}. Make sure you have gh CLI installed and authenticated.`) process.exit(1) } @@ -45,8 +45,8 @@ export const PrCommand = cmd({ let sessionId: string | undefined - if (prInfoResult.code === 0) { - const prInfoText = prInfoResult.text + if (prInfoResult.exitCode === 0) { + const prInfoText = prInfoResult.stdout.toString() if (prInfoText.trim()) { const prInfo = JSON.parse(prInfoText) diff --git a/packages/opencode/src/project/pr.ts b/packages/opencode/src/project/pr.ts index a8dea8057fde..e69a37ac5b42 100644 --- a/packages/opencode/src/project/pr.ts +++ b/packages/opencode/src/project/pr.ts @@ -179,7 +179,7 @@ export namespace PR { const result = streamObject({ ...params, providerOptions: ProviderTransform.providerOptions(model, { - instructions: SystemPrompt.instructions(), + instructions: system.join("\n"), store: false, }), onError: () => {},