Skip to content

feat: expose review comment permalinks in review context - #1

Merged
sdirix merged 2 commits into
mainfrom
feat/review-comment-permalinks
Aug 5, 2026
Merged

sdirix merged 2 commits into
mainfrom
feat/review-comment-permalinks

Conversation

@sdirix

@sdirix sdirix commented Aug 3, 2026

Copy link
Copy Markdown
Member

Review comments now carry their GitHub permalink as url, so a review agent can link an earlier discussion when it refers to one. Review summaries and conversation comments already exposed htmlUrl.

The permalink is selected in every comment query and mutation, so pending review comments and the add/modify results carry it too. A thread's permalink is its first comment's url.

Review comments now carry their GitHub permalink as `url`, so a review
agent can link an earlier discussion when it refers to one. Review
summaries and conversation comments already exposed `htmlUrl`.

The permalink is selected in every comment query and mutation, so
pending review comments and the add/modify results carry it too. A
thread's permalink is its first comment's `url`.

@EclipseSourceAI EclipseSourceAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Autonomous AI review.

This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. Noting why helps, since replies are read and taken into account in follow-up reviews.

Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.

To get an updated review after pushing changes, a maintainer may re-request a review from this account.

Running in Eclipse Enclave, submitted via review-guard-mcp

Focused change: ReviewComment gains a url permalink, selected in every GraphQL query and mutation that feeds mapGraphqlReviewComment, so an agent can link back to an earlier discussion. I checked all six call sites of that mapper and the coverage is complete, the safety boundary (submit/resolve/scope gates) is untouched, and there are no drive-by changes.

Toolchain is green locally: 72 tests pass, tsc --noEmit, ESLint and Prettier are clean. (npm itself refuses to run in this environment because devEngines.packageManager wants npm >= 12.)

Two points deserve a maintainer decision: nothing on the agent-facing surface (tool descriptions, README tools table) mentions permalinks, so a model may never discover the field, and the pending-review paths now return a #discussion_r link for comments nobody can open until the review is submitted. The rest is naming consistency (url vs the sibling htmlUrl fields) and one suggestion on test coverage. Details inline.

Comment thread AGENTS.md Outdated
Comment thread src/github.ts
Comment thread src/github.ts
pageInfo { hasNextPage endCursor }
nodes {
id
url

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path (plus the add/update mutations) now hands out a #discussion_r link for comments in a PENDING review, which nobody can open until the review is submitted. Document that on the field so an agent doesn't quote a draft link into a comment body, and a maintainer should confirm the real behaviour via the integration suite.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not hold up. The user of the review-guard can see them and once the review is submitted, everyone can. So it's safe to use.

Comment thread test/github.test.ts
Name the permalink `url` on every output type instead of mixing `url` and
`htmlUrl` depending on whether the field came from GraphQL or REST, since the
output shape should be more consistent than the GitHub API is. Review summaries
and conversation comments therefore rename `htmlUrl` to `url`.

Mention permalinks on the agent-facing surface, the `get_pr_review_context`
description and the README tools table, so a model can discover the field.

Document what a pending comment's permalink means, on the `ReviewComment.url`
doc comment and on the descriptions of the three tools that hand back pending
comments. A draft comment already carries its final permalink and every comment
of a review goes live the moment the review is submitted, so an agent can
cross-link its own findings while drafting. The link just does not resolve yet
while the review is pending.

Cover the pending review path in the unit tests, where a dropped `url`
selection would otherwise degrade to null silently, plus the two renamed
REST-sourced fields. The integration suite asserts that the permalink of a
draft comment is unchanged after the review is submitted.
@sdirix
sdirix requested a review from EclipseSourceAI August 5, 2026 14:56

@EclipseSourceAI EclipseSourceAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Autonomous AI review.

This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.

Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.

To get an updated review after pushing changes, a maintainer may re-request a review from this account.

Running in Eclipse Enclave, submitted via review-guard-mcp

The chore: address review feedback commit takes care of everything from my earlier pass: url naming is now consistent across ReviewComment, ReviewSummary and PullRequestConversationComment (no more htmlUrl/url split), AGENTS.md/README/tool descriptions now mention permalinks, and the pending-review path got its own doc comment plus a dedicated test asserting the permalink survives on the draft path.

On the discussion about pending comments handing out a #discussion_r link before submission: that's been settled the other way, the link is documented as safe and the integration suite now asserts it survives submission unchanged, so I'm not reopening that.

I didn't find anything new to flag in this round. Diff is scoped tightly to the naming/docs cleanup, tests cover the gap I called out last time, nothing else changed.

@sdirix
sdirix merged commit 5df36bf into main Aug 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants