feat(labctl): include the PR discussion thread in pr view #192

Merged

dominik.polakovics merged 1 commit from afk/191 into main

2026-07-21 02:45:47 +02:00

dominik.polakovics commented

2026-07-21 02:38:40 +02:00

Owner

An agent catching up on a PR conversation (verdict findings, human replies) was blind: labctl pr view rendered everything but the comment thread, while the write path (pr comment) and the plumbing (tracker.PullComments, already read by the autoland poller) existed. This folds the thread into the existing read — no new verb, no new endpoint — per the grilled design on the issue.

What shipped

API — GET /agent/v1/prs/{n} (prDetailResponse) gains comments: author/body/created_at, the issue-view comment JSON shape, oldest first, content verbatim and unfiltered (autoland verdict markers included — a reject's body IS the findings a fix agent needs). Contract mirrors reviews: always marshals [], never null; a failed PullComments read fails the whole response through writeTrackerError (no partial detail).
Builtin carve-out — the built-in binding's PullComments wraps ErrUnsupported unconditionally (no CR comment thread yet, ADR-0048); handlePRGet treats exactly that sentinel as an empty thread instead of failing, so builtin pr view keeps answering 200 with comments: [] — the same shape a commentless forge PR yields — rather than 409ing. Any other error still fails whole. This is the one point where the implementation refines the issue's "whole response fails" rule; the alternative regressed every builtin pr view, and the carve-out stops firing on its own once the builtin tracker grows CR comments. The tracker seam is untouched, so the autoland poller's fail-closed read is unchanged.
CLI — PRDetail gains Comments []Comment (the existing issue-comment client type); printPR renders each comment after the reviews block, byte-for-byte printIssue's --- comment by <author> (<time>) convention, slice order (server returns oldest first). Usage line updated. Zero comments leaves output byte-identical to before.
Docs — the generated-context template (contextfile.md.tmpl) now documents labctl pr view <n> including reviews and comments; all three claude goldens updated byte-exact.
Scope held — discussion comments only (what PullComments returns); inline file/line review comments stay out, pr list stays PullRef-cheap.

Tests

TestPRViewComments_forge (new): two comments through in order with store.FormatTime timestamps; a verdict-marker comment passes verbatim; a PullComments failure → 502 with the backend's words, no partial detail.
TestPRViewAndList_forge / TestPRViewAndList_builtin (extended): comments present and [] non-nil on both bindings — the builtin assertion is the regression test for the carve-out.
TestPRViewWithComments (new, labctl): full CLI drive against a JSON server — reviews block first, then both comment blocks in order, verbatim bodies.
TestPRViewBuiltinRoundTrip (untouched, still green) covers the real builtin server → labctl path end-to-end.

Verification

CGO_ENABLED=0 go build ./..., go vet ./..., gofmt clean; go test ./... green except internal/tmuxx's integration tests, which fail identically on pristine main in this sandbox (tmux panes exit immediately — environment-only, package untouched). Also drove the freshly built labctl pr view against the live lab server: renders cleanly with the field absent (wire compat both directions).

Closes #191

🤖 Generated with Claude Code

An agent catching up on a PR conversation (verdict findings, human replies) was blind: `labctl pr view` rendered everything but the comment thread, while the write path (`pr comment`) and the plumbing (`tracker.PullComments`, already read by the autoland poller) existed. This folds the thread into the existing read — no new verb, no new endpoint — per the grilled design on the issue. ## What shipped - **API** — GET `/agent/v1/prs/{n}` (`prDetailResponse`) gains `comments`: author/body/created_at, the issue-view comment JSON shape, oldest first, content verbatim and unfiltered (autoland verdict markers included — a reject's body IS the findings a fix agent needs). Contract mirrors `reviews`: always marshals `[]`, never null; a failed `PullComments` read fails the whole response through `writeTrackerError` (no partial detail). - **Builtin carve-out** — the built-in binding's `PullComments` wraps `ErrUnsupported` unconditionally (no CR comment thread yet, ADR-0048); `handlePRGet` treats exactly that sentinel as an empty thread instead of failing, so builtin `pr view` keeps answering 200 with `comments: []` — the same shape a commentless forge PR yields — rather than 409ing. Any other error still fails whole. This is the one point where the implementation refines the issue's "whole response fails" rule; the alternative regressed every builtin `pr view`, and the carve-out stops firing on its own once the builtin tracker grows CR comments. The tracker seam is untouched, so the autoland poller's fail-closed read is unchanged. - **CLI** — `PRDetail` gains `Comments []Comment` (the existing issue-comment client type); `printPR` renders each comment after the reviews block, byte-for-byte `printIssue`'s `--- comment by <author> (<time>)` convention, slice order (server returns oldest first). Usage line updated. Zero comments leaves output byte-identical to before. - **Docs** — the generated-context template (`contextfile.md.tmpl`) now documents `labctl pr view <n>` including reviews and comments; all three claude goldens updated byte-exact. - **Scope held** — discussion comments only (what `PullComments` returns); inline file/line review comments stay out, `pr list` stays PullRef-cheap. ## Tests - `TestPRViewComments_forge` (new): two comments through in order with `store.FormatTime` timestamps; a verdict-marker comment passes verbatim; a `PullComments` failure → 502 with the backend's words, no partial detail. - `TestPRViewAndList_forge` / `TestPRViewAndList_builtin` (extended): `comments` present and `[]` non-nil on both bindings — the builtin assertion is the regression test for the carve-out. - `TestPRViewWithComments` (new, labctl): full CLI drive against a JSON server — reviews block first, then both comment blocks in order, verbatim bodies. - `TestPRViewBuiltinRoundTrip` (untouched, still green) covers the real builtin server → labctl path end-to-end. ## Verification `CGO_ENABLED=0 go build ./...`, `go vet ./...`, gofmt clean; `go test ./...` green except `internal/tmuxx`'s integration tests, which fail identically on pristine main in this sandbox (tmux panes exit immediately — environment-only, package untouched). Also drove the freshly built `labctl pr view` against the live lab server: renders cleanly with the field absent (wire compat both directions). Closes #191 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 1 commit

2026-07-21 02:38:40 +02:00

feat(labctl): include the PR discussion thread in pr view

ci / native (pull_request) Successful in 6m44s

Details

d1bfcfff6b

GET /agent/v1/prs/{n} gains a comments array — the PR's discussion
thread as tracker.PullComments returns it, oldest first, verbatim
(autoland verdict-marker comments included: a reject's body IS the
findings a fix agent needs). Same contract as reviews: always [],
never null, and a failed read fails the whole response (no partial
detail) — except the built-in binding's ErrUnsupported, which renders
as an empty thread rather than 409ing every builtin pr view.

labctl pr view renders the thread after the reviews block,
byte-for-byte printIssue's "--- comment by <author> (<time>)"
convention, restoring issue/PR read symmetry. The generated-context
template now documents pr view (goldens updated), so agents learn the
thread is one call away.

Closes #191

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

dominik.polakovics commented

2026-07-21 02:45:43 +02:00

Author

Owner

[autoland] verdict: pass

dominik.polakovics merged commit f6500cb5f1 into main

2026-07-21 02:45:47 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-21 02:45:49 +02:00

Merge pull request 'feat(labctl): include the PR discussion thread in pr view' (#192) from afk/191 into main