diff --git a/frontend/openapi/openapi.yaml b/frontend/openapi/openapi.yaml index fec589a3f..2fded8c74 100644 --- a/frontend/openapi/openapi.yaml +++ b/frontend/openapi/openapi.yaml @@ -476,6 +476,9 @@ components: message: description: First line of commit message type: string + pushed: + description: Whether the commit is reachable from the workspace branch's upstream tracking ref; false means it has not been pushed. Omitted when push status is unknown, such as pull request commits. + type: boolean sha: description: Full commit SHA type: string diff --git a/internal/apiclient/generated/client.gen.go b/internal/apiclient/generated/client.gen.go index e7649d277..98cdaf7a2 100644 --- a/internal/apiclient/generated/client.gen.go +++ b/internal/apiclient/generated/client.gen.go @@ -1,6 +1,6 @@ // Package generated provides primitives to interact with the openapi HTTP API. // -// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.7.0 DO NOT EDIT. +// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.7.1 DO NOT EDIT. package generated import ( @@ -598,6 +598,9 @@ type CommitResponse struct { // Message First line of commit message Message string `json:"message"` + // Pushed Whether the commit is reachable from the workspace branch's upstream tracking ref; false means it has not been pushed. Omitted when push status is unknown, such as pull request commits. + Pushed *bool `json:"pushed,omitempty"` + // Sha Full commit SHA Sha string `json:"sha"` } diff --git a/internal/server/api_types.go b/internal/server/api_types.go index b8fe50b88..190d9e1d3 100644 --- a/internal/server/api_types.go +++ b/internal/server/api_types.go @@ -446,6 +446,7 @@ type commitResponse struct { Message string `json:"message" doc:"First line of commit message"` AuthorName string `json:"author_name" doc:"Commit author display name"` AuthoredAt time.Time `json:"authored_at" doc:"Commit author date (RFC3339)"` + Pushed *bool `json:"pushed,omitempty" doc:"Whether the commit is reachable from the workspace branch's upstream tracking ref; false means it has not been pushed. Omitted when push status is unknown, such as pull request commits."` } type commitsResponse struct { diff --git a/internal/server/huma_routes.go b/internal/server/huma_routes.go index ee961b549..fa3594c03 100644 --- a/internal/server/huma_routes.go +++ b/internal/server/huma_routes.go @@ -5477,14 +5477,37 @@ func (s *Server) getWorkspaceCommits( "commits not available for this workspace", nil) } + // Annotate each commit with whether it has reached the branch's upstream so + // the UI can flag local-only commits. Push status is an enhancement over the + // commit list, so a probe failure degrades to omitting the flag rather than + // failing the request. When the branch has no upstream we cannot tell pushed + // from unpushed (a fork PR head has no upstream yet already exists on its + // remote), so the flag is omitted rather than guessed. + unpushed, hasUpstream, pushErr := workspace.WorktreeUnpushedSHAs( + ctx, req.Summary.WorktreePath, + ) + if pushErr != nil { + slog.Warn( + "failed to determine unpushed workspace commits", + "workspace_id", input.ID, + "err", pushErr, + ) + } + resp := commitsResponse{Commits: make([]commitResponse, len(commits))} for i, c := range commits { - resp.Commits[i] = commitResponse{ + cr := commitResponse{ SHA: c.SHA, Message: c.Message, AuthorName: c.AuthorName, AuthoredAt: c.AuthoredAt.UTC(), } + if pushErr == nil && hasUpstream { + _, isUnpushed := unpushed[c.SHA] + pushed := !isUnpushed + cr.Pushed = &pushed + } + resp.Commits[i] = cr } return &getWorkspaceCommitsOutput{Body: resp}, nil } diff --git a/internal/server/workspacetest/workspace_test.go b/internal/server/workspacetest/workspace_test.go index 477728040..b052fcf76 100644 --- a/internal/server/workspacetest/workspace_test.go +++ b/internal/server/workspacetest/workspace_test.go @@ -191,6 +191,73 @@ func TestWorkspaceRuntimeAttachSpecUsesStoredTmuxSessionE2E(t *testing.T) { assert.True(spec.RequiresLocalHost) } +func TestWorkspaceCommitsFlagsUnpushedCommitsE2E(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + fixture := setupWorkspaceServerFixture(t, nil) + ctx := t.Context() + ws := createReadyWorkspace(t, ctx, fixture.client) + require.NotEmpty(ws.WorktreePath) + + // Commit locally without pushing. A brand-new commit is unpushed no matter + // how the worktree tracks its upstream, so the commits endpoint must flag + // it - this proves the push status reaches the wire for a real workspace. + runGit(t, ws.WorktreePath, "config", "user.email", "ws@test.com") + runGit(t, ws.WorktreePath, "config", "user.name", "Workspace") + require.NoError(os.WriteFile( + filepath.Join(ws.WorktreePath, "local-only.txt"), + []byte("local\n"), 0o644, + )) + runGit(t, ws.WorktreePath, "add", ".") + runGit(t, ws.WorktreePath, "commit", "-m", "local only commit") + localSHA := testGitSHA(t, ws.WorktreePath, "HEAD") + + resp, err := fixture.client.HTTP.GetWorkspaceCommitsWithResponse(ctx, ws.Id) + require.NoError(err) + require.Equal(http.StatusOK, resp.StatusCode()) + require.NotNil(resp.JSON200) + require.NotNil(resp.JSON200.Commits) + require.NotEmpty(*resp.JSON200.Commits) + + top := (*resp.JSON200.Commits)[0] + assert.Equal(localSHA, top.Sha, "newest commit should be the local-only commit") + require.NotNil(top.Pushed, "workspace commits should carry push status") + assert.False(*top.Pushed, "freshly committed local commit must be unpushed") +} + +func TestWorkspaceCommitsOmitsPushStatusWithoutUpstreamE2E(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + fixture := setupWorkspaceServerFixture(t, nil) + ctx := t.Context() + ws := createReadyWorkspace(t, ctx, fixture.client) + require.NotEmpty(ws.WorktreePath) + require.NotEmpty(ws.GitHeadRef) + + // Drop the branch's upstream so the worktree mimics a fork PR head: the + // commits exist but @{upstream} no longer resolves. Push status is then + // unknowable, so the endpoint must omit it rather than report every commit + // as unpushed (the false "Not pushed to remote" regression roborev flagged). + runGit(t, ws.WorktreePath, + "config", "--unset", "branch."+ws.GitHeadRef+".remote") + runGit(t, ws.WorktreePath, + "config", "--unset", "branch."+ws.GitHeadRef+".merge") + + resp, err := fixture.client.HTTP.GetWorkspaceCommitsWithResponse(ctx, ws.Id) + require.NoError(err) + require.Equal(http.StatusOK, resp.StatusCode()) + require.NotNil(resp.JSON200) + require.NotNil(resp.JSON200.Commits) + require.NotEmpty(*resp.JSON200.Commits) + + for _, c := range *resp.JSON200.Commits { + assert.Nil(c.Pushed, + "push status must be omitted when the branch has no upstream") + } +} + func writeWorkspaceRuntimeTmuxProbe( t *testing.T, expectedSession string, diff --git a/internal/workspace/divergence.go b/internal/workspace/divergence.go index e535faaf7..65b5a38ff 100644 --- a/internal/workspace/divergence.go +++ b/internal/workspace/divergence.go @@ -93,6 +93,62 @@ func WorktreeDivergence( return Divergence{Ahead: ahead, Behind: behind}, true, nil } +// WorktreeUnpushedSHAs returns the set of commit SHAs reachable from the +// worktree's HEAD but not from its `@{upstream}` tracking branch — the commits +// that have not been pushed yet. +// +// The second return value is false when the branch has no upstream configured. +// Callers cannot infer push status in that case: a fresh branch has pushed +// nothing, but a fork PR head also has no upstream while already existing on a +// remote. The error return is reserved for unexpected git failures. +func WorktreeUnpushedSHAs( + ctx context.Context, dir string, +) (map[string]struct{}, bool, error) { + if dir == "" { + return nil, false, errors.New("empty worktree dir") + } + + cmd := workspaceGitCommand( + ctx, dir, "rev-list", "@{upstream}..HEAD", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + // Like WorktreeDivergence, the commit picker can fan this probe out per + // workspace, so gate it on the shared git subprocess limiter. We keep + // stderr separate (rather than using procutil's combined-output helpers) + // so the no-upstream branch below can recognize git's "no upstream" + // message exactly as the divergence probe does. + release, err := procutil.TryAcquire( + ctx, "git unpushed subprocess capacity", + ) + if err != nil { + return nil, false, err + } + defer release() + + if err := cmd.Run(); err != nil { + stderrText := stderr.String() + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && isNoUpstreamMessage(stderrText) { + return nil, false, nil + } + return nil, false, fmt.Errorf( + "git rev-list: %w: %s", err, strings.TrimSpace(stderrText), + ) + } + + // rev-list emits one full SHA per line and never embeds whitespace, so + // Fields cleanly tokenizes the output regardless of trailing newlines. + shas := strings.Fields(stdout.String()) + unpushed := make(map[string]struct{}, len(shas)) + for _, sha := range shas { + unpushed[sha] = struct{}{} + } + return unpushed, true, nil +} + // isNoUpstreamMessage matches the stderr texts git uses when the // current branch has no `@{upstream}` configured. func isNoUpstreamMessage(stderr string) bool { diff --git a/internal/workspace/divergence_test.go b/internal/workspace/divergence_test.go index c8cee27ba..9a20e4d59 100644 --- a/internal/workspace/divergence_test.go +++ b/internal/workspace/divergence_test.go @@ -3,6 +3,7 @@ package workspace import ( "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -124,3 +125,69 @@ func TestWorktreeDivergenceWithoutUpstream(t *testing.T) { assert.New(t).False(ok, "expected ok=false for branch without upstream") assert.New(t).Equal(Divergence{}, div) } + +func TestWorktreeUnpushedSHAsInSync(t *testing.T) { + work := setupDivergenceWorktree(t) + + unpushed, ok, err := WorktreeUnpushedSHAs(t.Context(), work) + require := require.New(t) + require.NoError(err) + require.True(ok) + assert.New(t).Empty(unpushed) +} + +func TestWorktreeUnpushedSHAsAheadOfRemote(t *testing.T) { + require := require.New(t) + work := setupDivergenceWorktree(t) + pushedHead := revParseTestSHA(t, work, "HEAD") + + require.NoError(os.WriteFile( + filepath.Join(work, "f.txt"), []byte("ahead-1\n"), 0o644, + )) + runWorkspaceTestGit(t, work, "add", ".") + runWorkspaceTestGit(t, work, "commit", "-m", "ahead 1") + first := revParseTestSHA(t, work, "HEAD") + + require.NoError(os.WriteFile( + filepath.Join(work, "f.txt"), []byte("ahead-2\n"), 0o644, + )) + runWorkspaceTestGit(t, work, "add", ".") + runWorkspaceTestGit(t, work, "commit", "-m", "ahead 2") + second := revParseTestSHA(t, work, "HEAD") + + unpushed, ok, err := WorktreeUnpushedSHAs(t.Context(), work) + require.NoError(err) + require.True(ok) + assert := assert.New(t) + assert.Len(unpushed, 2) + assert.Contains(unpushed, first) + assert.Contains(unpushed, second) + assert.NotContains(unpushed, pushedHead, + "commit already on the upstream must not be reported as unpushed") +} + +func TestWorktreeUnpushedSHAsWithoutUpstream(t *testing.T) { + require := require.New(t) + root := t.TempDir() + work := filepath.Join(root, "work") + runWorkspaceTestGit(t, root, "init", "--initial-branch=main", work) + runWorkspaceTestGit(t, work, "config", "user.email", "t@test.com") + runWorkspaceTestGit(t, work, "config", "user.name", "Test") + require.NoError(os.WriteFile( + filepath.Join(work, "x.txt"), []byte("x\n"), 0o644, + )) + runWorkspaceTestGit(t, work, "add", ".") + runWorkspaceTestGit(t, work, "commit", "-m", "init") + + unpushed, ok, err := WorktreeUnpushedSHAs(t.Context(), work) + require.NoError(err) + assert.New(t).False(ok, "expected ok=false for branch without upstream") + assert.New(t).Nil(unpushed) +} + +func revParseTestSHA(t *testing.T, dir, ref string) string { + t.Helper() + return strings.TrimSpace(string( + runWorkspaceTestGit(t, dir, "rev-parse", ref), + )) +} diff --git a/packages/ui/src/api/generated/schema.ts b/packages/ui/src/api/generated/schema.ts index cec6c73b5..a23d8cb06 100644 --- a/packages/ui/src/api/generated/schema.ts +++ b/packages/ui/src/api/generated/schema.ts @@ -4340,6 +4340,8 @@ export interface components { authored_at: string; /** @description First line of commit message */ message: string; + /** @description Whether the commit is reachable from the workspace branch's upstream tracking ref; false means it has not been pushed. Omitted when push status is unknown, such as pull request commits. */ + pushed?: boolean; /** @description Full commit SHA */ sha: string; }; diff --git a/packages/ui/src/api/types.ts b/packages/ui/src/api/types.ts index bb7ec1557..eafe7c404 100644 --- a/packages/ui/src/api/types.ts +++ b/packages/ui/src/api/types.ts @@ -154,6 +154,10 @@ export interface CommitInfo { message: string; author_name: string; authored_at: string; + // True when the commit is reachable from the workspace branch's upstream + // tracking ref; false means it is local-only. Absent when push status is + // unknown, such as pull request commits. + pushed?: boolean; } export interface WorkspaceHost { diff --git a/packages/ui/src/components/diff/CommitListItem.svelte b/packages/ui/src/components/diff/CommitListItem.svelte index 62d3f11f3..5f629d33a 100644 --- a/packages/ui/src/components/diff/CommitListItem.svelte +++ b/packages/ui/src/components/diff/CommitListItem.svelte @@ -37,6 +37,14 @@ onclick={handleClick} title={commit.message} > + {commit.sha.slice(0, 7)} {commit.message} {relativeDate(commit.authored_at)} @@ -67,6 +75,18 @@ color: var(--text-primary); } + .commit-item__push { + width: 6px; + height: 6px; + border-radius: 50%; + flex-shrink: 0; + background: transparent; + } + + .commit-item__push--unpushed { + background: var(--accent-amber); + } + .commit-item__sha { font-family: var(--font-mono); font-size: var(--font-size-2xs); diff --git a/packages/ui/src/components/diff/CommitListItem.test.ts b/packages/ui/src/components/diff/CommitListItem.test.ts new file mode 100644 index 000000000..d4ae1466e --- /dev/null +++ b/packages/ui/src/components/diff/CommitListItem.test.ts @@ -0,0 +1,42 @@ +import { cleanup, render, screen } from "@testing-library/svelte"; +import { afterEach, describe, expect, it } from "vite-plus/test"; + +import type { CommitInfo } from "../../api/types.js"; +import CommitListItem from "./CommitListItem.svelte"; + +function makeCommit(overrides: Partial = {}): CommitInfo { + return { + sha: "a5a7f26abcdef0123456789", + message: "Document analyze precedence", + author_name: "Alice", + authored_at: "2026-06-27T00:00:00Z", + ...overrides, + }; +} + +function renderItem(commit: CommitInfo): void { + render(CommitListItem, { + props: { commit, active: false, onclick: () => {} }, + }); +} + +describe("CommitListItem", () => { + afterEach(() => { + cleanup(); + }); + + it("flags commits that have not been pushed", () => { + renderItem(makeCommit({ pushed: false })); + expect(screen.getByRole("img", { name: "Not pushed to remote" })).toBeTruthy(); + }); + + it("does not flag pushed commits", () => { + renderItem(makeCommit({ pushed: true })); + expect(screen.queryByLabelText("Not pushed to remote")).toBeNull(); + }); + + it("does not flag commits with unknown push status", () => { + renderItem(makeCommit()); + expect(screen.queryByLabelText("Not pushed to remote")).toBeNull(); + }); +});