feat: expose per-ref diff statuses - #29082
DanielleMaywood wants to merge 2 commits into
Conversation
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
9169278 to
5cb2c01
Compare
5cb2c01 to
064d2a7
Compare
064d2a7 to
f7f3b28
Compare
f7f3b28 to
7def130
Compare
e55688b to
eae5fbe
Compare
eae5fbe to
c66b95b
Compare
c66b95b to
d86382c
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10e9ca4d69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| remoteOrigin := status.GitRemoteOrigin | ||
| result.RemoteOrigin = &remoteOrigin |
There was a problem hiding this comment.
Redact credentials from exposed remote origins
When a Git remote uses an HTTPS URL with embedded credentials, agentgit.getRepoChanges reads remote.origin.url verbatim and gitsync persists that value, so returning it unchanged here exposes the password or token through chat GET/list responses. This also discloses the credential to users granted read access through the chat ACL, even if they cannot access the workspace. Expose a credential-free normalized origin or redact URL userinfo instead of serializing the database key directly.
Useful? React with 👍 / 👎.
| // A selector that matches nothing is an error only when the | ||
| // chat tracks other refs. A chat with no refs stays on inline | ||
| // resolution. | ||
| return result, xerrors.Errorf("no diff status for ref %s/%s", selector.RemoteOrigin, selector.GitBranch) |
There was a problem hiding this comment.
Return a client error for unknown refs
When origin and branch do not exactly match a tracked ref, including when a caller supplies only one of the independently optional query parameters, this new error reaches getChatDiffContents and is always returned as HTTP 500. A malformed or stale selector is a client-side condition, so returning 500 misclassifies expected input errors and can trigger retries or server-error alerts; validate incomplete selectors and map unmatched refs to an appropriate 4xx response.
Useful? React with 👍 / 👎.
4c58154 to
e81202d
Compare
e81202d to
5344c6c
Compare
5344c6c to
fdbbdf2
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdbbdf2d98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RemoteOrigin *string `json:"remote_origin,omitempty"` | ||
| GitBranch *string `json:"git_branch,omitempty"` |
There was a problem hiding this comment.
Compare ref identity when merging diff-status events
When the primary moves between two refs whose other status fields match, such as two GitLab branches with no discovered PR, site/src/api/queries/chats.ts:489-514 still considers the statuses equal because it does not compare the newly exposed remote_origin or git_branch. mergeWatchedChatSummary can therefore treat the watch event as a no-op and retain the previous primary ref while the server and selector-free diff endpoint use the new one; include both identity fields in diffStatusEqual.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b505d99b1: diffStatusEqual now compares remote_origin and git_branch before the other fields, so a primary move between two refs whose remaining fields coincide (e.g. two no-PR GitLab branches) no longer passes the merge no-op guard with a stale primary.
| // @Param origin query string false "Remote origin selecting the ref to diff" | ||
| // @Param branch query string false "Git branch selecting the ref to diff" |
There was a problem hiding this comment.
Expose the ref selector through the Go SDK
The endpoint now accepts origin and branch, but a repo-wide search shows that codersdk.Client.GetChatDiffContents still always requests the bare /diff URL. Consequently Go SDK consumers cannot retrieve the diff for any non-primary entry returned in DiffStatuses; add a URL-encoding SDK method or options parameter for both selector fields while preserving the existing selector-free call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b505d99b1: GetChatDiffContents now accepts opts ...RequestOption, and WithChatDiffStatusRef(ref) sets the origin and branch query parameters through the SDK's existing WithQueryParam. Selector-free calls are unchanged. A test exercises the option against a chat with two tracked refs.
fdbbdf2 to
7b505d9
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
The API now exposes the per-ref diff statuses that part 1 stores. The old
diff_statusfield keeps its meaning, so no client breaks.This is part 2 of 3. It stacks on the database change in part 1. Part 3 adds the git panel views.
Fix
Chat.diff_statusesis a new list of every ref the chat tracks, next to the existingdiff_status.diff_statusis the first row of theupdated_at DESCordering. The selector-free diff GET and the git panel use the same row, so every surface agrees on which PR represents the chat.remote_originandgit_branch.diff_status_changeevents have a newchanged_diff_statusfield that names the one ref that changed. The chat inside the event keeps the primary indiff_status.GET /chats/{chat}/diffaccepts optionaloriginandbranchquery parameters. Without them it behaves as before.Notes for review
Generated by Coder Agents.