diff --git a/docs/web/control-ui.md b/docs/web/control-ui.md index 4d80e370b172..7a6474359ec9 100644 --- a/docs/web/control-ui.md +++ b/docs/web/control-ui.md @@ -430,7 +430,7 @@ The macOS app keeps its native link-browser sidebar for links clicked in the das - The session header shows a small facepile beside the workspace chip when other people are viewing the same session; it lists up to four viewer avatars with an overflow count and disappears when you are alone. - Consecutive duplicate text-only messages render as one bubble with a count badge. Messages that carry images, attachments, tool output, or canvas previews are left uncollapsed. - User-message bubbles carry transcript actions: a hover rewind button (confirm popover with a "Don't ask again" option) plus right-click **Rewind to here** and **Fork from here**. Rewind repoints the session to the state just before that message and returns its text to the composer for edit and resend (`sessions.rewind`, `operator.admin`); fork creates a new session from the active-path prefix before the message, opens it, and seeds its composer with the same text (`sessions.fork`, `operator.write`). Both actions disable with an explanatory tooltip while the agent is working, apply only to persisted user messages, and are rejected for sessions whose conversation is owned by an external agent harness. Rewind moves chat context only — files and other tool side effects are not reverted — and the pre-rewind transcript remains preserved in the append-only session store. When that store contains multiple transcript branches, the chat title bar shows a branch menu with each branch's latest message, message count, and recency; selecting an inactive branch switches the current session back to that preserved path (`sessions.branches.list`, `operator.read`; `sessions.branches.switch`, `operator.admin`). Branch switching is also unavailable while the agent is working, and selecting the already-active branch is a typed no-op error at the RPC boundary. The separate hide action on user bubbles hides a message in the current browser only; the message stays in the transcript and the agent still sees it. - - When a session's checkout sits on a non-default branch of a GitHub repository, the chat view pins pull request chips above the composer: PR number, repo, branch, diff counts, a CI pill, and draft/merged/closed state, each linking to the PR. The row shows at most two chips — live (open/draft) PRs first — and a "Show more" button reveals collapsed merged/closed history. The CI pill opens a small CI monitoring popover with passed/failed/running/skipped check counts and a link to the PR's checks page. Detection runs server-side through `controlUi.sessionPullRequests`, which reuses the Gateway's `GH_TOKEN`/`GITHUB_TOKEN` when set. When the GitHub API rate limit is hit, chips keep the last known status and show a warning that the status may be out of date; dismissing a chip hides it for that session in the current browser profile. Before any PR exists, the row shows the branch itself — repo, branch name, and the +/− size of the diff against the default-branch merge base (committed and uncommitted work). Once the pushed branch has commits to compare, the row adds a Create PR button that opens GitHub's new-pull-request page; before that, a session with changed files (committed, uncommitted, or untracked) still gets the row without the button. The row hides itself while an open or draft PR exists. The branch row comes from local git only, so it stays available while GitHub is rate limited and carries the same stale-status warning, since "no PR found" cannot be trusted until the limit resets. + - When a session's checkout sits on a non-default branch of a GitHub repository, the chat view pins pull request chips above the composer: PR number, repo, branch, diff counts, a CI pill, and draft/merged/closed state, each linking to the PR. The row shows at most two chips — live (open/draft) PRs first — and a "Show more" button reveals collapsed merged/closed history. The CI pill opens a small CI monitoring popover with passed/failed/running/skipped check counts and a link to the PR's checks page. Detection runs server-side through `controlUi.sessionPullRequests`, which reuses the Gateway's `GH_TOKEN`/`GITHUB_TOKEN` when set. When the GitHub API rate limit is hit, chips keep the last known status and show a warning that the status may be out of date; dismissing a chip hides it for that session in the current browser profile. Before any PR exists, the row shows the branch itself — repo, branch name, and the +/− size of the diff against the default-branch merge base (committed and uncommitted work). Once the pushed branch has commits to compare, the row adds a Create PR button that opens GitHub's new-pull-request page; before that, a session with changed files (committed, uncommitted, or untracked) still gets the row without the button. The row hides itself while an open or draft PR exists; once the branch's PR is merged and the pushed tip still matches the merged head, the row disappears too (returning without the Create PR button only when new local work appears, and with it once new commits are pushed past the merged head). The branch row comes from local git only, so it stays available while GitHub is rate limited and carries the same stale-status warning, since "no PR found" cannot be trusted until the limit resets. - The session diff panel shows what a session's checkout actually changed: the branch button in the workspace rail or chat title bar opens the detail panel with a per-file diff of branch, uncommitted, and untracked work against the checkout's default-branch merge base — status dot, rename arrow, per-file +/− counts, collapsible files, and "N unmodified lines" markers between hunks. Diffs are computed server-side through the `sessions.diff` Gateway method (`operator.read` scope); binary and oversized files degrade to stats-only entries, and the button only appears when the connected Gateway advertises `sessions.diff`. - Every Chat pane has a title bar. Click the session title to rename it; the workspace chip copies the checkout path or branch and can reveal local Gateway workspaces in the host file manager. Remote and exec-node sessions keep copy actions but hide reveal. - The thread workspace rail in each Chat pane lists thread files, project files, and artifacts. It docks to the pane's right edge by default; drag its header (or use the dock button) to move it to the bottom, and the choice is stored in the current browser profile. A collapsed rail takes no space at all: reopen it with ⇧⌘B or the files toggle in the title bar, which carries a changed-file count badge. The separate file, tool, and Canvas detail panel is unaffected. diff --git a/src/gateway/control-ui-session-prs-branch.test.ts b/src/gateway/control-ui-session-prs-branch.test.ts new file mode 100644 index 000000000000..381b88f89722 --- /dev/null +++ b/src/gateway/control-ui-session-prs-branch.test.ts @@ -0,0 +1,961 @@ +import { execFile } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { loadControlUiSessionPullRequests } from "./control-ui-session-prs.js"; +import { + evictPullRequestCache, + githubJson, + pullListItem, + routedFetch, + testGitContext as context, +} from "./control-ui-session-prs.test-support.js"; + +describe("session branch diff stats", () => { + const execFileAsync = promisify(execFile); + let root: string; + + const git = (...args: string[]) => + execFileAsync("git", ["-c", "user.email=test@openclaw.ai", "-c", "user.name=Test", ...args], { + cwd: root, + }); + + beforeEach(async () => { + root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-session-prs-"))); + }); + + afterEach(async () => { + await evictPullRequestCache(); + await fs.rm(root, { recursive: true, force: true }); + }); + + it("counts committed and uncommitted changes vs the origin default merge base", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\ntwo\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + // Stand in for the remote default branch without a real remote. + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.writeFile(path.join(root, "a.txt"), "one\nthree\n"); + await fs.writeFile(path.join(root, "b.txt"), "committed\n"); + await git("add", "a.txt", "b.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + // Uncommitted work counts too: the row sizes the PR the push would open. + await fs.appendFile(path.join(root, "b.txt"), "pending\n"); + // Untracked files count toward additions as well. + await fs.writeFile(path.join(root, "c.txt"), "brand new\n"); + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 4, + deletions: 1, + createUrl: "https://github.com/openclaw/openclaw/pull/new/feature", + }); + }); + + it("skips non-regular and binary untracked files without blocking", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + await fs.writeFile(path.join(root, "text.txt"), "alpha\nbeta\n"); + await fs.writeFile(path.join(root, "blob.bin"), Buffer.from([0x50, 0x00, 0x4b, 0x03])); + if (process.platform !== "win32") { + // A named pipe must not block the stats path until the git timeout. + await execFileAsync("mkfifo", [path.join(root, "pipe")]); + } + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // 1 committed line + 2 untracked text lines; binary and pipe count 0. + expect(result.branch).toMatchObject({ additions: 3, deletions: 0 }); + }); + + it("omits the branch payload when the remote branch has nothing to compare", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // origin/feature == origin/main: GitHub would answer "nothing to compare". + expect(result.branch).toBeUndefined(); + }); + + it("reports local changes without createUrl until the branch exists on origin", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "local only"); + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // GitHub's pull/new page 404s for unpushed branches, so no Create PR + // link — but the session's changed files still get a row. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + }); + }); + + it("reports uncommitted changes when the remote branch has nothing to compare", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + await fs.appendFile(path.join(root, "a.txt"), "pending\n"); + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // origin/feature == origin/main, so no Create PR link yet, but the dirty + // working tree is visible work the row must surface. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + }); + }); + + it("drops the Create PR row once the pushed tip is a merged PR's head", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "refs/remotes/origin/feature")).stdout.trim(); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + expect(result.pullRequests[0]?.state).toBe("merged"); + // A squash merge keeps origin/feature "ahead" of origin/main forever, but + // the landed tip must not resurrect a Create PR invitation to duplicate it. + expect(result.branch).toBeUndefined(); + }); + + it("sizes only post-merge work, without a create link, once the PR landed", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "refs/remotes/origin/feature")).stdout.trim(); + await fs.appendFile(path.join(root, "a.txt"), "follow-up\n"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The stale merge base would replay the merged +1 as pending; only the + // uncommitted follow-up line counts, and no Create PR link is offered. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + }); + }); + + it("keeps the Create PR row when the PR merged into a non-default base", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "refs/remotes/origin/feature")).stdout.trim(); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + base: { ref: "release", repo: { name: "openclaw", owner: { login: "openclaw" } } }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // Merging into release does not land the work on main; the affordance to + // open a PR against the default branch must survive. + expect(result.branch?.createUrl).toBe("https://github.com/openclaw/openclaw/pull/new/feature"); + }); + + it("suppresses the row via local HEAD when the merged remote ref was pruned", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + // No origin/feature ref: GitHub deleted the head branch on merge and a + // pruned fetch removed the remote-tracking ref. + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // Without this, the stale merge base replays the landed diff forever. + expect(result.branch).toBeUndefined(); + }); + + it("suppresses the row when the local checkout trails the merged remote tip", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + const staleHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // Another checkout pushed a final commit before the merge; this session's + // HEAD trails the merged remote tip. + await fs.appendFile(path.join(root, "a.txt"), "review fix\n"); + await git("add", "a.txt"); + await git("commit", "-m", "review fix"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("reset", "--hard", staleHead); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // Everything committed here is contained in the merge; a clean tree gets + // no row, and the stale merge base must not replay the landed subset. + expect(result.branch).toBeUndefined(); + }); + + it("restores Create PR for a branch rebased past the landing with new work", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // Squash-land on main, then reuse the branch: reset onto updated main, + // add new work, force-push. + await git("checkout", "main"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "squash land"); + const mergeCommit = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "feature"); + await git("reset", "--hard", "refs/remotes/origin/main"); + await fs.writeFile(path.join(root, "b.txt"), "second round\n"); + await git("add", "b.txt"); + await git("commit", "-m", "second PR work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + merge_commit_sha: mergeCommit, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The merge base contains the PR's merge commit, proving the rebase went + // past the landing: the new commit is a genuine second PR. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + createUrl: "https://github.com/openclaw/openclaw/pull/new/feature", + }); + }); + + it("counts a release-branch landing once its merge commit reaches main", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // The PR merged into a release branch whose squash later reached main. + await git("checkout", "main"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "release merge propagated"); + const mergeCommit = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "feature"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + merge_commit_sha: mergeCommit, + base: { ref: "release", repo: { name: "openclaw", owner: { login: "openclaw" } } }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The release landing propagated to main, so the branch is landed work; + // no Create PR row despite the non-default PR base. + expect(result.branch).toBeUndefined(); + }); + + it("restores Create PR atop a merge-commit landing without a rebase", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // A merge-commit landing keeps the head an ancestor of main. + await git("checkout", "main"); + await git("merge", "--no-ff", "feature", "-m", "merge PR"); + const mergeCommit = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "feature"); + await fs.writeFile(path.join(root, "b.txt"), "follow-up\n"); + await git("add", "b.txt"); + await git("commit", "-m", "follow-up work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + merge_commit_sha: mergeCommit, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The merged head is contained in the merge base, so a new PR's compare + // holds only the follow-up commit; no rebase is required here. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + createUrl: "https://github.com/openclaw/openclaw/pull/new/feature", + }); + }); + + it("keeps Create PR off while a newer squash landing is unincorporated", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + // PR1: squash-land, then the branch rebases past it. + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "pr1 work"); + const pr1Head = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("checkout", "main"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "squash pr1"); + const pr1Merge = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("checkout", "feature"); + await git("reset", "--hard", "main"); + // PR2 on the rebased branch: squash-lands on main, but the branch does + // not rebase again before the follow-up commit. + await fs.writeFile(path.join(root, "b.txt"), "pr2\n"); + await git("add", "b.txt"); + await git("commit", "-m", "pr2 work"); + const pr2Head = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("checkout", "main"); + await fs.writeFile(path.join(root, "b.txt"), "pr2\n"); + await git("add", "b.txt"); + await git("commit", "-m", "squash pr2"); + const pr2Merge = (await git("rev-parse", "HEAD")).stdout.trim(); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "feature"); + await fs.writeFile(path.join(root, "c.txt"), "follow-up\n"); + await git("add", "c.txt"); + await git("commit", "-m", "follow-up work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + number: 2, + state: "closed", + merged_at: "2026-07-02T00:00:00Z", + head: { sha: pr2Head }, + merge_commit_sha: pr2Merge, + }), + pullListItem({ + number: 1, + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: pr1Head }, + merge_commit_sha: pr1Merge, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // PR1's landing is in the merge base but PR2's is not: a new PR would + // replay PR2's diff, so only the follow-up shows and the link stays off. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + }); + }); + + it("offers no Create PR from a stale tracking ref behind the merged head", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + const staleSha = (await git("rev-parse", "HEAD")).stdout.trim(); + await fs.appendFile(path.join(root, "a.txt"), "final fix\n"); + await git("add", "a.txt"); + await git("commit", "-m", "final fix"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // Another clone pushed the final commit and merged; this checkout's + // tracking ref and HEAD still sit at the earlier commit. + await git("update-ref", "refs/remotes/origin/feature", staleSha); + await git("reset", "--hard", staleSha); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The stale ref is not proof of new pushed work; everything here is + // contained in the merged head, so no row and no Create PR invitation. + expect(result.branch).toBeUndefined(); + }); + + it("prefers the newer merge base after the default branch was merged back in", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // Squash-land on main, main moves on, then the session merges main back + // into the still-checked-out feature branch. + await git("checkout", "main"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "squash land"); + await fs.writeFile(path.join(root, "b.txt"), "unrelated\n"); + await git("add", "b.txt"); + await git("commit", "-m", "unrelated main work"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "feature"); + await git("merge", "refs/remotes/origin/main", "-m", "merge main"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The merge base (main's tip) already carries the landed content; sizing + // against the older merged head would replay main's progress as pending. + expect(result.branch).toBeUndefined(); + }); + + it("ignores the merged tip as a diff base when the branch was reset onto main", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "HEAD")).stdout.trim(); + // Squash-land the same content on main, then main moves on. + await git("checkout", "main"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "squash land"); + await fs.writeFile(path.join(root, "b.txt"), "unrelated\n"); + await git("add", "b.txt"); + await git("commit", "-m", "unrelated main work"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + // The session resets its branch onto updated main; origin/feature still + // points at the merged head. + await git("checkout", "feature"); + await git("reset", "--hard", "refs/remotes/origin/main"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // Diffing against the stale merged tip would report main's unrelated + // progress as pending feature work; the merge-base path reports nothing. + expect(result.branch).toBeUndefined(); + }); + + it("keeps post-merge commits as a stats-only row until the branch rebases", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("update-ref", "refs/remotes/origin/main", "HEAD"); + await git("checkout", "-b", "feature"); + await fs.appendFile(path.join(root, "a.txt"), "two\n"); + await git("add", "a.txt"); + await git("commit", "-m", "feature work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + const mergedHead = (await git("rev-parse", "refs/remotes/origin/feature")).stdout.trim(); + await fs.appendFile(path.join(root, "a.txt"), "three\n"); + await git("add", "a.txt"); + await git("commit", "-m", "post-merge work"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => + githubJson([ + pullListItem({ + state: "closed", + merged_at: "2026-07-01T00:00:00Z", + head: { sha: mergedHead }, + }), + ]), + }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + resolveGitContext: async () => ({ + ...context, + branch: "feature", + root, + defaultBranch: "main", + }), + }, + ); + + // The post-merge commit counts (not the landed diff), but GitHub's + // compare for this un-rebased tip would replay the landed changes, so the + // Create PR link stays off until the branch incorporates the landing. + expect(result.branch).toEqual({ + owner: "openclaw", + repo: "openclaw", + branch: "feature", + additions: 1, + deletions: 0, + }); + }); + + it("omits the branch payload when the default branch is unknown", async () => { + await git("init", "--initial-branch=main", "."); + await fs.writeFile(path.join(root, "a.txt"), "one\n"); + await git("add", "a.txt"); + await git("commit", "-m", "base"); + await git("checkout", "-b", "feature"); + await git("update-ref", "refs/remotes/origin/feature", "HEAD"); + + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson([]) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + const result = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl, + // No defaultBranch: origin/HEAD unresolvable in this checkout. + resolveGitContext: async () => ({ ...context, branch: "feature", root }), + }, + ); + + // Fail closed: without a default branch there is nothing to compare against. + expect(result.branch).toBeUndefined(); + }); +}); diff --git a/src/gateway/control-ui-session-prs.test-support.ts b/src/gateway/control-ui-session-prs.test-support.ts new file mode 100644 index 000000000000..30bffffecec5 --- /dev/null +++ b/src/gateway/control-ui-session-prs.test-support.ts @@ -0,0 +1,77 @@ +// Shared fixtures for the control-ui session PR tests; test-only module. +import { vi } from "vitest"; +import { loadControlUiSessionPullRequests } from "./control-ui-session-prs.js"; + +type GitContext = { owner: string; repo: string; branch: string }; + +export function githubJson(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +export function requestUrl(input: RequestInfo | URL | undefined): string { + if (typeof input === "string") { + return input; + } + if (input instanceof URL) { + return input.href; + } + return input?.url ?? ""; +} + +export function routedFetch( + routes: Array<{ match: string; response: () => Response | Promise }>, +) { + return vi.fn(async (input: RequestInfo | URL) => { + const url = requestUrl(input); + const route = routes.find((candidate) => url.includes(candidate.match)); + if (!route) { + throw new Error(`unexpected GitHub request: ${url}`); + } + return route.response(); + }) as unknown as typeof fetch & { mock: { calls: unknown[][] } }; +} + +export function pullListItem(overrides: Record = {}): Record { + return { + number: 103469, + title: "fix(macos): tighten the link-browser tab header", + html_url: "https://github.com/openclaw/openclaw/pull/103469", + state: "open", + draft: false, + merged_at: null, + head: { sha: "a".repeat(40) }, + base: { ref: "main", repo: { name: "openclaw", owner: { login: "openclaw" } } }, + ...overrides, + }; +} + +export const testGitContext: GitContext = { + owner: "openclaw", + repo: "openclaw", + branch: "claude/browser-tabs-tighter-header", +}; + +let cacheEvictionEpoch = 0; + +// The module-level branch cache would leak entries across tests; flooding it +// past CACHE_LIMIT evicts everything a test created. +export async function evictPullRequestCache(): Promise { + const epoch = (cacheEvictionEpoch += 1); + await Promise.all( + Array.from({ length: 101 }, (_, index) => + loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { + fetchImpl: async () => githubJson([]), + resolveGitContext: async () => ({ + ...testGitContext, + branch: `test/cache-eviction-${epoch}-${index}`, + }), + }, + ), + ), + ); +} diff --git a/src/gateway/control-ui-session-prs.test.ts b/src/gateway/control-ui-session-prs.test.ts index de8a0dafd809..d80b0c0d0c7a 100644 --- a/src/gateway/control-ui-session-prs.test.ts +++ b/src/gateway/control-ui-session-prs.test.ts @@ -1,70 +1,20 @@ -import { execFile } from "node:child_process"; -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { loadControlUiSessionPullRequests, parseControlUiSessionPullRequestsParams, } from "./control-ui-session-prs.js"; +import { + evictPullRequestCache, + githubJson, + pullListItem, + requestUrl, + routedFetch, + testGitContext as context, +} from "./control-ui-session-prs.test-support.js"; import { parseGitHubRemoteUrl } from "./github-remote.js"; -type GitContext = { owner: string; repo: string; branch: string }; - -function githubJson(body: unknown, status = 200): Response { - return new Response(JSON.stringify(body), { - status, - headers: { "Content-Type": "application/json" }, - }); -} - -function requestUrl(input: RequestInfo | URL | undefined): string { - if (typeof input === "string") { - return input; - } - if (input instanceof URL) { - return input.href; - } - return input?.url ?? ""; -} - -function routedFetch( - routes: Array<{ match: string; response: () => Response | Promise }>, -) { - return vi.fn(async (input: RequestInfo | URL) => { - const url = requestUrl(input); - const route = routes.find((candidate) => url.includes(candidate.match)); - if (!route) { - throw new Error(`unexpected GitHub request: ${url}`); - } - return route.response(); - }) as unknown as typeof fetch & { mock: { calls: unknown[][] } }; -} - -function pullListItem(overrides: Record = {}): Record { - return { - number: 103469, - title: "fix(macos): tighten the link-browser tab header", - html_url: "https://github.com/openclaw/openclaw/pull/103469", - state: "open", - draft: false, - merged_at: null, - head: { sha: "a".repeat(40) }, - base: { repo: { name: "openclaw", owner: { login: "openclaw" } } }, - ...overrides, - }; -} - -const context: GitContext = { - owner: "openclaw", - repo: "openclaw", - branch: "claude/browser-tabs-tighter-header", -}; - const resolveGitContext = async () => context; let cacheEpochMs = Date.now(); -let cacheEvictionEpoch = 0; describe("parseGitHubRemoteUrl", () => { it("parses https, scp-like, and ssh remotes", () => { @@ -83,24 +33,6 @@ describe("parseGitHubRemoteUrl", () => { }); }); -async function evictPullRequestCache(): Promise { - const epoch = (cacheEvictionEpoch += 1); - await Promise.all( - Array.from({ length: 101 }, (_, index) => - loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl: async () => githubJson([]), - resolveGitContext: async () => ({ - ...context, - branch: `test/cache-eviction-${epoch}-${index}`, - }), - }, - ), - ), - ); -} - describe("parseControlUiSessionPullRequestsParams", () => { it("requires a non-empty session key", () => { expect(parseControlUiSessionPullRequestsParams({ sessionKey: "agent:main:main" })).toEqual({ @@ -595,238 +527,3 @@ describe("loadControlUiSessionPullRequests", () => { ); }); }); - -describe("session branch diff stats", () => { - const execFileAsync = promisify(execFile); - let root: string; - - const git = (...args: string[]) => - execFileAsync("git", ["-c", "user.email=test@openclaw.ai", "-c", "user.name=Test", ...args], { - cwd: root, - }); - - beforeEach(async () => { - root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-session-prs-"))); - }); - - afterEach(async () => { - await evictPullRequestCache(); - await fs.rm(root, { recursive: true, force: true }); - }); - - it("counts committed and uncommitted changes vs the origin default merge base", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\ntwo\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - // Stand in for the remote default branch without a real remote. - await git("update-ref", "refs/remotes/origin/main", "HEAD"); - await git("checkout", "-b", "feature"); - await fs.writeFile(path.join(root, "a.txt"), "one\nthree\n"); - await fs.writeFile(path.join(root, "b.txt"), "committed\n"); - await git("add", "a.txt", "b.txt"); - await git("commit", "-m", "feature work"); - await git("update-ref", "refs/remotes/origin/feature", "HEAD"); - // Uncommitted work counts too: the row sizes the PR the push would open. - await fs.appendFile(path.join(root, "b.txt"), "pending\n"); - // Untracked files count toward additions as well. - await fs.writeFile(path.join(root, "c.txt"), "brand new\n"); - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - resolveGitContext: async () => ({ - ...context, - branch: "feature", - root, - defaultBranch: "main", - }), - }, - ); - - expect(result.branch).toEqual({ - owner: "openclaw", - repo: "openclaw", - branch: "feature", - additions: 4, - deletions: 1, - createUrl: "https://github.com/openclaw/openclaw/pull/new/feature", - }); - }); - - it("skips non-regular and binary untracked files without blocking", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - await git("update-ref", "refs/remotes/origin/main", "HEAD"); - await git("checkout", "-b", "feature"); - await fs.appendFile(path.join(root, "a.txt"), "two\n"); - await git("add", "a.txt"); - await git("commit", "-m", "feature work"); - await git("update-ref", "refs/remotes/origin/feature", "HEAD"); - await fs.writeFile(path.join(root, "text.txt"), "alpha\nbeta\n"); - await fs.writeFile(path.join(root, "blob.bin"), Buffer.from([0x50, 0x00, 0x4b, 0x03])); - if (process.platform !== "win32") { - // A named pipe must not block the stats path until the git timeout. - await execFileAsync("mkfifo", [path.join(root, "pipe")]); - } - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - resolveGitContext: async () => ({ - ...context, - branch: "feature", - root, - defaultBranch: "main", - }), - }, - ); - - // 1 committed line + 2 untracked text lines; binary and pipe count 0. - expect(result.branch).toMatchObject({ additions: 3, deletions: 0 }); - }); - - it("omits the branch payload when the remote branch has nothing to compare", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - await git("update-ref", "refs/remotes/origin/main", "HEAD"); - await git("checkout", "-b", "feature"); - await git("update-ref", "refs/remotes/origin/feature", "HEAD"); - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - resolveGitContext: async () => ({ - ...context, - branch: "feature", - root, - defaultBranch: "main", - }), - }, - ); - - // origin/feature == origin/main: GitHub would answer "nothing to compare". - expect(result.branch).toBeUndefined(); - }); - - it("reports local changes without createUrl until the branch exists on origin", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - await git("update-ref", "refs/remotes/origin/main", "HEAD"); - await git("checkout", "-b", "feature"); - await fs.appendFile(path.join(root, "a.txt"), "two\n"); - await git("add", "a.txt"); - await git("commit", "-m", "local only"); - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - resolveGitContext: async () => ({ - ...context, - branch: "feature", - root, - defaultBranch: "main", - }), - }, - ); - - // GitHub's pull/new page 404s for unpushed branches, so no Create PR - // link — but the session's changed files still get a row. - expect(result.branch).toEqual({ - owner: "openclaw", - repo: "openclaw", - branch: "feature", - additions: 1, - deletions: 0, - }); - }); - - it("reports uncommitted changes when the remote branch has nothing to compare", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - await git("update-ref", "refs/remotes/origin/main", "HEAD"); - await git("checkout", "-b", "feature"); - await git("update-ref", "refs/remotes/origin/feature", "HEAD"); - await fs.appendFile(path.join(root, "a.txt"), "pending\n"); - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - resolveGitContext: async () => ({ - ...context, - branch: "feature", - root, - defaultBranch: "main", - }), - }, - ); - - // origin/feature == origin/main, so no Create PR link yet, but the dirty - // working tree is visible work the row must surface. - expect(result.branch).toEqual({ - owner: "openclaw", - repo: "openclaw", - branch: "feature", - additions: 1, - deletions: 0, - }); - }); - - it("omits the branch payload when the default branch is unknown", async () => { - await git("init", "--initial-branch=main", "."); - await fs.writeFile(path.join(root, "a.txt"), "one\n"); - await git("add", "a.txt"); - await git("commit", "-m", "base"); - await git("checkout", "-b", "feature"); - await git("update-ref", "refs/remotes/origin/feature", "HEAD"); - - const fetchImpl = routedFetch([ - { match: "/pulls?head=", response: () => githubJson([]) }, - { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, - ]); - const result = await loadControlUiSessionPullRequests( - { sessionKey: "agent:main:main" }, - { - fetchImpl, - // No defaultBranch: origin/HEAD unresolvable in this checkout. - resolveGitContext: async () => ({ ...context, branch: "feature", root }), - }, - ); - - // Fail closed: without a default branch there is nothing to compare against. - expect(result.branch).toBeUndefined(); - }); -}); diff --git a/src/gateway/control-ui-session-prs.ts b/src/gateway/control-ui-session-prs.ts index 84f3f2bb86c8..6fc123d5956c 100644 --- a/src/gateway/control-ui-session-prs.ts +++ b/src/gateway/control-ui-session-prs.ts @@ -56,15 +56,31 @@ type PullListItem = { repo: string; state: ControlUiSessionPullRequest["state"]; headSha?: string; + baseRef?: string; + mergeCommitSha?: string; +}; + +/** Lowercased merged-PR head, the base it merged into, and its merge commit. */ +type MergedPullHead = { sha: string; baseRef?: string; mergeCommitSha?: string }; + +/** + * Cached GitHub snapshot plus the merged PRs' heads. The heads stay + * gateway-internal (stripped before responding): they only exist so branch + * resolution can tell a landed tip from real post-merge work. Kept as raw + * GitHub facts because the cache key carries no default branch; each + * checkout filters them against its own default at resolve time. + */ +type BranchPullRequestsSnapshot = ControlUiSessionPullRequests & { + mergedHeads: MergedPullHead[]; }; type CacheEntry = { expiresAt: number; - promise: Promise; + promise: Promise; refreshMode: "normal" | "forced" | null; // Survives refetch failures so rate-limited refreshes degrade to stale // chips instead of clearing the row. - lastGood?: ControlUiSessionPullRequest[]; + lastGood?: { pullRequests: ControlUiSessionPullRequest[]; mergedHeads: MergedPullHead[] }; }; const branchCache = new Map(); @@ -96,6 +112,15 @@ async function gitOutput(cwd: string, args: string[]): Promise { } } +async function isAncestor(root: string, ancestor: string, descendant: string): Promise { + try { + const result = await runGit(root, ["merge-base", "--is-ancestor", ancestor, descendant]); + return result.code === 0; + } catch { + return false; + } +} + /** * Resolves the GitHub repo + branch a session works on. Returns null for * unknown sessions, non-git roots, detached HEADs, non-GitHub remotes, and @@ -208,24 +233,16 @@ async function untrackedStats(root: string): Promise<{ additions: number; files: } /** - * Working-tree diff counts vs the merge base with the remote default branch, - * untracked files included: the size the PR would have if the current work - * were committed and pushed; changedFiles decides row visibility for - * unpushed branches. Unlike bare `git diff`, diffing against an explicit - * base counts unmerged (conflict) paths, so conflict-only trees still show. + * Working-tree diff counts vs an explicit base, untracked files included: + * the size the PR would have if the current work were committed and pushed; + * changedFiles decides row visibility for unpushed branches. Unlike bare + * `git diff`, diffing against an explicit base counts unmerged (conflict) + * paths, so conflict-only trees still show. */ -async function loadBranchDiffStats( +async function diffStatsAgainst( root: string, - defaultBranch: string, + base: string, ): Promise<{ additions: number; deletions: number; changedFiles: number } | null> { - const mergeBase = await gitOutput(root, [ - "merge-base", - `refs/remotes/origin/${defaultBranch}`, - "HEAD", - ]); - if (!mergeBase) { - return null; - } try { // --no-ext-diff/--no-textconv: checkout-configurable diff drivers must // never execute in the Gateway process (same guard as sessions-diff). @@ -234,7 +251,7 @@ async function loadBranchDiffStats( "--shortstat", "--no-ext-diff", "--no-textconv", - mergeBase, + base, ]); if (result.code !== 0) { return null; @@ -262,21 +279,17 @@ async function loadBranchDiffStats( async function branchHasCreatablePullRequest( root: string, context: SessionPullRequestGitContext, + pushedSha: string | null, ): Promise { // Fail closed without a resolvable default branch: a session sitting on the // actual default in a clone lacking origin/HEAD must not get a Create PR row. - if (!context.defaultBranch) { - return false; - } - const remoteRef = `refs/remotes/origin/${context.branch}`; - const pushed = await gitOutput(root, ["rev-parse", "--verify", "--quiet", remoteRef]); - if (!pushed) { + if (!context.defaultBranch || !pushedSha) { return false; } const ahead = await gitOutput(root, [ "rev-list", "--count", - `refs/remotes/origin/${context.defaultBranch}..${remoteRef}`, + `refs/remotes/origin/${context.defaultBranch}..refs/remotes/origin/${context.branch}`, ]); // A failed count keeps the row: rev-list errors must not hide a valid branch. return ahead === null || Number(ahead) > 0; @@ -284,13 +297,98 @@ async function branchHasCreatablePullRequest( async function resolveSessionBranch( context: SessionPullRequestGitContext, + mergedHeads: readonly MergedPullHead[], ): Promise { - // Stubbed test contexts without a root skip the local-git gates. - const creatable = !context.root || (await branchHasCreatablePullRequest(context.root, context)); - const stats = - context.root && context.defaultBranch - ? await loadBranchDiffStats(context.root, context.defaultBranch) - : null; + const root = context.root; + if (!root) { + // Stubbed test contexts without a root skip the local-git gates. + return { + owner: context.owner, + repo: context.repo, + branch: context.branch, + createUrl: branchCreateUrl(context), + }; + } + const pushedSha = await gitOutput(root, [ + "rev-parse", + "--verify", + "--quiet", + `refs/remotes/origin/${context.branch}`, + ]); + const headSha = await gitOutput(root, ["rev-parse", "HEAD"]); + // Only merges whose content reached this checkout's default branch prove + // the tip landed there: a direct default-base merge, or a landing through + // another branch (feature -> release -> main) whose merge commit is now + // contained in the default branch. A PR merged into an unpropagated + // release/staging branch must not hide Create PR. Filtered here, not in + // the cache, because the cache key has no default branch. + const defaultRef = context.defaultBranch ? `refs/remotes/origin/${context.defaultBranch}` : null; + const landedHeads: MergedPullHead[] = []; + for (const head of mergedHeads) { + if (head.baseRef === context.defaultBranch) { + landedHeads.push(head); + } else if ( + defaultRef && + head.mergeCommitSha && + (await isAncestor(root, head.mergeCommitSha, defaultRef)) + ) { + landedHeads.push(head); + } + } + const mergedHeadShas = landedHeads.map((head) => head.sha); + const mergeBase = context.defaultBranch + ? await gitOutput(root, ["merge-base", `refs/remotes/origin/${context.defaultBranch}`, "HEAD"]) + : null; + // The stats base is the newest commit whose content is known-published: + // the ordinary default-branch merge base, or a merged PR head related to + // HEAD by ancestry (a HEAD trailing the merged tip is fully landed, so + // HEAD itself is the baseline). Diffing against anything older would + // replay landed work as pending — the squash-merged commits are never + // ancestors of the default branch, so the merge base alone cannot see + // them. Ancestry is best-effort: a merged head never fetched locally + // cannot be proven related and falls back to the merge base. + const baselines: string[] = mergeBase ? [mergeBase] : []; + if (headSha) { + for (const merged of mergedHeadShas) { + if (await isAncestor(root, merged, headSha)) { + baselines.push(merged); + } else if (await isAncestor(root, headSha, merged)) { + baselines.push(headSha); + } + } + } + let statsBase: string | null = null; + for (const candidate of baselines) { + if (!statsBase || (await isAncestor(root, statsBase, candidate))) { + statsBase = candidate; + } + } + // Squash merges leave origin/ "ahead" of the default branch + // forever, so local git alone would keep offering Create PR after the work + // landed. Once merged PRs exist, the link returns only when the merge base + // provably contains EVERY known landing — a merge-commit landing leaves the + // head itself as an ancestor, a squash landing is visible only via its + // merge commit, and an older landing must not vouch for a newer one whose + // diff a new PR would replay. A tip merely descending from a squashed head + // or a fetch-stale tracking ref proves nothing, so those states keep the + // row stats-only until a rebase or fetch. + let provenNewPushedWork = false; + if (pushedSha && mergeBase && !mergedHeadShas.includes(pushedSha.toLowerCase())) { + provenNewPushedWork = landedHeads.length > 0; + for (const head of landedHeads) { + const incorporated = + (await isAncestor(root, head.sha, mergeBase)) || + (head.mergeCommitSha ? await isAncestor(root, head.mergeCommitSha, mergeBase) : false); + if (!incorporated) { + provenNewPushedWork = false; + break; + } + } + } + const creatable = + (mergedHeadShas.length === 0 || provenNewPushedWork) && + (await branchHasCreatablePullRequest(root, context, pushedSha)); + const stats = statsBase ? await diffStatsAgainst(root, statsBase) : null; // No createUrl until GitHub can compare, but local changes still get a row. if (!creatable && !(stats && stats.changedFiles > 0)) { return undefined; @@ -338,6 +436,8 @@ function parsePullListItem(value: unknown): PullListItem | null { repo, state: derivePullState(value), headSha: optionalString(head, "sha"), + baseRef: optionalString(base, "ref"), + mergeCommitSha: optionalString(value, "merge_commit_sha"), }; } @@ -489,11 +589,25 @@ async function finishPullRequest( }; } +function mergedHeadsOf(items: readonly PullListItem[]): MergedPullHead[] { + const heads: MergedPullHead[] = []; + for (const item of items) { + if (item.state === "merged" && item.headSha) { + heads.push({ + sha: item.headSha.toLowerCase(), + ...(item.baseRef ? { baseRef: item.baseRef } : {}), + ...(item.mergeCommitSha ? { mergeCommitSha: item.mergeCommitSha.toLowerCase() } : {}), + }); + } + } + return heads; +} + async function fetchBranchPullRequests( context: SessionPullRequestGitContext, fetchImpl: typeof fetch, token: string | undefined, -): Promise<{ pullRequests: ControlUiSessionPullRequest[]; rateLimited: boolean }> { +): Promise { const head = `${context.owner}:${context.branch}`; let items = parsePullList( await fetchGitHubJson(pullsByHeadUrl(context.owner, context.repo, head), fetchImpl, token), @@ -508,11 +622,12 @@ async function fetchBranchPullRequests( } } const capped = items.slice(0, MAX_PULL_REQUESTS); + const mergedHeads = mergedHeadsOf(capped); try { const pullRequests = await Promise.all( capped.map((item) => finishPullRequest(item, context.branch, fetchImpl, token)), ); - return { pullRequests, rateLimited: false }; + return { pullRequests, rateLimited: false, mergedHeads }; } catch (error) { if (!(error instanceof ControlUiGitHubError && error.statusCode === 429)) { throw error; @@ -531,6 +646,7 @@ async function fetchBranchPullRequests( state: item.state, })), rateLimited: true, + mergedHeads, }; } } @@ -539,14 +655,14 @@ async function refreshBranchPullRequests( context: SessionPullRequestGitContext, fetchImpl: typeof fetch, entry: CacheEntry, -): Promise { +): Promise { try { const result = await fetchBranchPullRequests(context, fetchImpl, githubApiToken()); // Degraded state-only chips still become lastGood: a later refresh that // rate-limits at the list fetch must serve the proven PRs, not an empty // list that would resurrect the Create PR row mid-outage. The shortened // expiry makes the next window retry full detail. - entry.lastGood = result.pullRequests; + entry.lastGood = { pullRequests: result.pullRequests, mergedHeads: result.mergedHeads }; if (result.rateLimited) { entry.expiresAt = Date.now() + RATE_LIMIT_CACHE_MS; } @@ -555,10 +671,10 @@ async function refreshBranchPullRequests( const rateLimited = error instanceof ControlUiGitHubError && error.statusCode === 429; entry.expiresAt = Date.now() + (rateLimited ? RATE_LIMIT_CACHE_MS : FAILURE_CACHE_MS); if (rateLimited) { - return { pullRequests: entry.lastGood ?? [], rateLimited: true }; + return { pullRequests: [], mergedHeads: [], ...entry.lastGood, rateLimited: true }; } if (entry.lastGood) { - return { pullRequests: entry.lastGood, rateLimited: false }; + return { ...entry.lastGood, rateLimited: false }; } throw error; } @@ -583,18 +699,22 @@ export async function loadControlUiSessionPullRequests( // Branch metadata is local git only, so it stays fresh per request (the // working-tree diff moves while the agent works) and keeps the pre-PR row // alive when GitHub is rate limited; only the GitHub fetch is cached. - const [branch, snapshot] = await Promise.all([ - resolveSessionBranch(context), - cachedBranchPullRequests(context, deps, params.refresh === true), - ]); + // Sequenced after the snapshot because branch resolution needs the merged + // head SHAs; the snapshot is usually a cache hit, so this costs little. + const { mergedHeads, ...snapshot } = await cachedBranchPullRequests( + context, + deps, + params.refresh === true, + ); + const branch = await resolveSessionBranch(context, mergedHeads); return branch ? { ...snapshot, branch } : snapshot; } function trackBranchRefresh( entry: CacheEntry, mode: "normal" | "forced", - load: () => Promise, -): Promise { + load: () => Promise, +): Promise { // Publish the replacement promise before any awaited work so later callers // cannot overtake a queued forced refresh with an older normal result. entry.expiresAt = Date.now() + SUCCESS_CACHE_MS; @@ -613,7 +733,7 @@ async function cachedBranchPullRequests( context: SessionPullRequestGitContext, deps: LoadSessionPullRequestDeps, refresh: boolean, -): Promise { +): Promise { const key = `${context.owner.toLowerCase()}/${context.repo.toLowerCase()}#${context.branch}`; const cached = branchCache.get(key); if (cached && cached.expiresAt > Date.now()) { @@ -640,7 +760,7 @@ async function cachedBranchPullRequests( } const entry: CacheEntry = cached ?? { expiresAt: 0, - promise: Promise.resolve({ pullRequests: [], rateLimited: false }), + promise: Promise.resolve({ pullRequests: [], rateLimited: false, mergedHeads: [] }), refreshMode: null, }; const promise = trackBranchRefresh(entry, refresh ? "forced" : "normal", () => diff --git a/ui/src/pages/chat/chat-state-events.ts b/ui/src/pages/chat/chat-state-events.ts index cd3d8ba60bd1..d93255d409cc 100644 --- a/ui/src/pages/chat/chat-state-events.ts +++ b/ui/src/pages/chat/chat-state-events.ts @@ -221,6 +221,20 @@ function terminalOwnsActiveChatStream( return typeof payload?.runId === "string" && payload.runId === state.chatRunId; } +function pullRequestLinksIn(text: unknown): string[] { + if (typeof text !== "string" || !text.includes("github.com")) { + return []; + } + const links: string[] = []; + for (const match of text.matchAll(GITHUB_URL_CANDIDATE)) { + const href = match[0].replace(/[.,;:!?]+$/u, ""); + if (isGitHubPullRequestLink(href)) { + links.push(href); + } + } + return links; +} + function finalAssistantReplyHasPullRequestLink( state: ChatPageHost, payload: ChatEventPayload | undefined, @@ -235,15 +249,49 @@ function finalAssistantReplyHasPullRequestLink( ...(state.chatStreamSegments ?? []).map((segment) => segment.text), ); } - return texts.some((text) => { - if (typeof text !== "string") { - return false; - } - return Array.from(text.matchAll(GITHUB_URL_CANDIDATE)).some((match) => { - const href = match[0].replace(/[.,;:!?]+$/u, ""); - return isGitHubPullRequestLink(href); - }); - }); + return texts.some((text) => pullRequestLinksIn(text).length > 0); +} + +// Bounds the refreshed-run set; clearing at worst re-fires one refresh per run. +const STREAM_PR_REFRESH_RUN_LIMIT = 200; +// Longest URL prefix worth carrying across delta chunks. GitHub caps owners at +// 39 and repos at 100 chars, so a maximal PR URL is ~175 chars; 256 covers it. +const STREAM_PR_LINK_TAIL_CHARS = 256; + +/** + * A PR created or merged mid-turn should surface a chip right away instead of + * waiting for the terminal reply or the minute poll, so the first streamed + * sighting of a PR link forces one chips refresh. At most one per run: the + * refresh reloads all of the branch's PRs regardless of which link fired it, + * so more links in the same run add GitHub quota cost without information, + * while a later run announcing a state change (created -> merged) refreshes + * again. Deltas are arbitrary fragments, so a short rolling tail rejoins URLs + * split across chunks; a link the tail still misses is caught by the + * final-reply trigger. That terminal trigger intentionally refreshes again + * even after a stream refresh — state often changes between the announcement + * and the end of the turn (created -> merged) — bounding forced refreshes at + * two per run, coalesced by the gateway while one is in flight. + */ +function refreshPullRequestsForStreamedLinks( + state: ChatPageHost, + payload: ChatEventPayload, + deltaText: string, +): void { + const scope = `${payload.sessionKey}|${payload.runId ?? ""}`; + const tail = state.streamPullRequestTail; + // The tail is scoped like the refresh: joining across runs would falsely + // complete split URLs. + const joined = (tail?.scope === scope ? tail.text : "") + deltaText; + state.streamPullRequestTail = { scope, text: joined.slice(-STREAM_PR_LINK_TAIL_CHARS) }; + const seen = (state.streamPullRequestRefreshKeys ??= new Set()); + if (seen.has(scope) || pullRequestLinksIn(joined).length === 0) { + return; + } + if (seen.size > STREAM_PR_REFRESH_RUN_LIMIT) { + seen.clear(); + } + seen.add(scope); + void state.refreshSessionPullRequests?.({ refresh: true }); } function hasVisibleFinalAssistantReply( @@ -307,6 +355,13 @@ export function handlePageGatewayEvent(state: ChatPageHost, event: GatewayEventF ) { state.observerDigest = null; } + if ( + payload?.state === "delta" && + typeof payload.deltaText === "string" && + chatScopedEventSessionMatches(state, payload.sessionKey, payload.agentId) + ) { + refreshPullRequestsForStreamedLinks(state, payload, payload.deltaText); + } const shouldCelebrateFirstReply = hasVisibleFinalAssistantReply(state, payload); const shouldRefreshPullRequests = shouldCelebrateFirstReply && finalAssistantReplyHasPullRequestLink(state, payload); diff --git a/ui/src/pages/chat/chat-state-host.ts b/ui/src/pages/chat/chat-state-host.ts index 89ad25f668ed..fed12b14cff4 100644 --- a/ui/src/pages/chat/chat-state-host.ts +++ b/ui/src/pages/chat/chat-state-host.ts @@ -93,6 +93,10 @@ export type ChatPageHost = ChatHost & planStatus: PlanStatus | null; observerDigest: SessionObserverDigest | null; knownAgentRunIds: Set; + /** `sessionKey|runId` scopes that already forced a PR-chips refresh mid-stream. */ + streamPullRequestRefreshKeys?: Set; + /** Rolling stream suffix so a PR URL split across delta chunks still matches. */ + streamPullRequestTail?: { scope: string; text: string }; waitingApprovalStatuses: Map; waitingApprovalResolvedIds: Set; chatRunStatus: ChatProps["runStatus"]; diff --git a/ui/src/pages/chat/chat-state.test.ts b/ui/src/pages/chat/chat-state.test.ts index 3d52d67a4c6b..c78a8f62dae5 100644 --- a/ui/src/pages/chat/chat-state.test.ts +++ b/ui/src/pages/chat/chat-state.test.ts @@ -297,6 +297,56 @@ describe("ChatStateController render lifecycle", () => { expect(requestUpdate).toHaveBeenCalledOnce(); }); + it("forces one PR-chips refresh per PR link seen in the live stream", () => { + vi.spyOn(globalThis, "requestAnimationFrame").mockImplementation(() => 1); + const refreshSessionPullRequests = vi.fn(() => Promise.resolve()); + const state = { + chatMessages: [], + chatMessagesBySession: new Map(), + chatRunId: "run-1", + chatStream: null, + chatStreamRenderFrame: null, + chatStreamStartedAt: 1, + lastError: null, + pendingSessionMessageReloadSessionKey: null, + refreshSessionPullRequests, + requestUpdate: vi.fn(), + sessionKey: "main", + } as unknown as ChatPageHost; + const delta = (deltaText: string, runId = "run-1") => + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { state: "delta", runId, sessionKey: "main", deltaText }, + }); + + delta("working on it "); + expect(refreshSessionPullRequests).not.toHaveBeenCalled(); + + // Issue links never carry chips. + delta("see https://github.com/openclaw/openclaw/issues/42 "); + expect(refreshSessionPullRequests).not.toHaveBeenCalled(); + + delta("opened https://github.com/openclaw/openclaw/pull/113840 for review "); + expect(refreshSessionPullRequests).toHaveBeenCalledTimes(1); + expect(refreshSessionPullRequests).toHaveBeenCalledWith({ refresh: true }); + + // One refresh reloads all of the branch's PRs; further links in the same + // run must not spend more GitHub quota. + delta("also https://github.com/openclaw/openclaw/pull/113900 "); + expect(refreshSessionPullRequests).toHaveBeenCalledTimes(1); + + // Streaming may split a URL across chunks; the rolling tail rejoins it. + delta("continuing https://github.com/openclaw/openclaw/pu", "run-2"); + expect(refreshSessionPullRequests).toHaveBeenCalledTimes(1); + delta("ll/113901 done", "run-2"); + expect(refreshSessionPullRequests).toHaveBeenCalledTimes(2); + + // A later run announcing the same PR (e.g. its merge) refreshes again. + delta("merged https://github.com/openclaw/openclaw/pull/113840 at last", "run-3"); + expect(refreshSessionPullRequests).toHaveBeenCalledTimes(3); + }); + it("requests a render before selecting the commit promise", async () => { let resolveCommit: (value: boolean) => void = () => {}; const nextCommit = new Promise((resolve) => {