Forgejo: review requests and PR review sessions #32

omp-deck · 2026-10-16T14:55:27+02:00

omp-deck commented

2026-10-16 14:55:27 +02:00

Adds a forgejo_pull_request session trigger kind, now used for every triggered session on a PR (addressed comments and review requests). Its tool surface is the conversation set plus forgejo_post_review, which is scoped to the PR and denied when the agent authored it. The webhook receiver now accepts review_requested deliveries for the agent, using a whole-body dedupe hash so each genuine re-request becomes a new review. The dispatcher gates the requester on write access. On review requests for the agent's own PR it posts OWN_PR_REVIEW instead of starting a session. Otherwise it seeds the session with the new buildReviewRequestSeed. Issue comments still start forgejo_conversation sessions.

Decisions

Widened ForgejoTriggerSummary.event/action in the protocol to include pull_request_review_request/pull_request and review_requested, and documented that commentAuthor holds the requester for review requests. The DB has no CHECK on these columns, so no migration is needed.
The exhaustive never switches in session-policy.ts now bind trigger itself rather than trigger.kind. Once every kind is handled, the trigger narrows to never, so .kind no longer typechecks.
Did not check the review-request header and payload shape against a real delivery: no local server was running with the webhook subscribed. The classifier follows the Gitea-lineage shape from the plan and also accepts Event-Type pull_request as a fallback.
ownPr for comments is read from issue.user.login in the issue_comment payload.
The public failure comment now says "failed to handle this event".

Verification

bun test apps/server/src/bridge/session-policy.test.ts apps/server/src/bridge/permission-filter.test.ts apps/server/src/forgejo/ apps/server/src/routes-forgejo-webhook.test.ts: 236 pass, 0 fail. This includes a new webhook.test.ts and new policy, seed and dispatcher cases.
bun test (full suite): 999 pass, 0 fail.
bun run typecheck: protocol, server and web all exit 0.
Not run: the end-to-end check against the real Forgejo instance. It needs the webhook subscribed to Pull request review requested events.

Closes #30

Adds a `forgejo_pull_request` session trigger kind, now used for every triggered session on a PR (addressed comments and review requests). Its tool surface is the conversation set plus `forgejo_post_review`, which is scoped to the PR and denied when the agent authored it. The webhook receiver now accepts `review_requested` deliveries for the agent, using a whole-body dedupe hash so each genuine re-request becomes a new review. The dispatcher gates the requester on write access. On review requests for the agent's own PR it posts `OWN_PR_REVIEW` instead of starting a session. Otherwise it seeds the session with the new `buildReviewRequestSeed`. Issue comments still start `forgejo_conversation` sessions. ## Decisions - Widened `ForgejoTriggerSummary.event`/`action` in the protocol to include `pull_request_review_request`/`pull_request` and `review_requested`, and documented that `commentAuthor` holds the requester for review requests. The DB has no CHECK on these columns, so no migration is needed. - The exhaustive `never` switches in `session-policy.ts` now bind `trigger` itself rather than `trigger.kind`. Once every kind is handled, the trigger narrows to `never`, so `.kind` no longer typechecks. - Did not check the review-request header and payload shape against a real delivery: no local server was running with the webhook subscribed. The classifier follows the Gitea-lineage shape from the plan and also accepts Event-Type `pull_request` as a fallback. - `ownPr` for comments is read from `issue.user.login` in the `issue_comment` payload. - The public failure comment now says "failed to handle this event". ## Verification - `bun test apps/server/src/bridge/session-policy.test.ts apps/server/src/bridge/permission-filter.test.ts apps/server/src/forgejo/ apps/server/src/routes-forgejo-webhook.test.ts`: 236 pass, 0 fail. This includes a new `webhook.test.ts` and new policy, seed and dispatcher cases. - `bun test` (full suite): 999 pass, 0 fail. - `bun run typecheck`: protocol, server and web all exit 0. - Not run: the end-to-end check against the real Forgejo instance. It needs the webhook subscribed to *Pull request review requested* events. Closes #30

lucas requested changes

2026-10-16 15:11:08 +02:00

Dismissed

apps/server/src/bridge/session-policy.ts Outdated

					
				@ -117,0 +160,4 @@

							`- Reply only on #${number}: \`forgejo_create_pr_comment\` for the conversation, \`forgejo_create_pr_thread_comment\` to answer an inline review comment in its thread.`,

							ownPr

								? "- You authored this pull request. Never review it: if you are asked to, reply with a comment explaining that you don't review your own pull requests."

								: "- When a comment asks you to review (or the session was started by a review request), do a full review: `forgejo_read_pr`, `forgejo_get_pr_diff`, `forgejo_read_file_at_ref` for surrounding code, `forgejo_list_pr_comments_and_reviews` for earlier feedback, then post exactly one `forgejo_post_review`. Put every line-specific remark in `comments`; keep `body` to a short summary. Choose COMMENT, REQUEST_CHANGES or APPROVE by your findings. Otherwise just reply.",

Don't mention forgejo_read_file_at_ref explicitly. The next fee steps and changes will make a local checkout and normal read possible, and the agent should decide for itself.

Don't mention `forgejo_read_file_at_ref` explicitly. The next fee steps and changes will make a local checkout and normal `read` possible, and the agent should decide for itself.

Done in 1e4e04e. The review instruction now just says to read the pull request, its diff, as much surrounding code as needed, and earlier feedback. It doesn't name the tools for that anymore; only forgejo_post_review is still named, as the way to post the result.

Done in 1e4e04e. The review instruction now just says to read the pull request, its diff, as much surrounding code as needed, and earlier feedback. It doesn't name the tools for that anymore; only `forgejo_post_review` is still named, as the way to post the result.

apps/server/src/forgejo/conversation-detect.ts Outdated

					
				@ -171,1 +171,4 @@

				}

				/** A code fence longer than any backtick run in `text`, so the text cannot close it. */

				function fenceFor(text: string): string {

I'm pretty sure I've seen the same Markdown parsing in a different file already.

The copy you've seen is probably the one on main: this exact fence computation was inline in buildConversationSeed. This PR moves it into fenceFor so the conversation seed and the new review-request seed share it. It's the only copy left in apps/server/src and packages.

The closest other code is the fence handling in kb-service.ts, but that one finds existing fenced blocks so wikilinks inside them are skipped. It doesn't build a fence to enclose arbitrary text, so I left it separate. If you had a different place in mind, point me to it and I'll merge the two.

The copy you've seen is probably the one on `main`: this exact fence computation was inline in `buildConversationSeed`. This PR moves it into `fenceFor` so the conversation seed and the new review-request seed share it. It's the only copy left in `apps/server/src` and `packages`. The closest other code is the fence handling in `kb-service.ts`, but that one finds existing fenced blocks so wikilinks inside them are skipped. It doesn't build a fence to enclose arbitrary text, so I left it separate. If you had a different place in mind, point me to it and I'll merge the two.

lucas force-pushed 30-forgejo-explicit-review-request-and-pr-review-sessions-plan-step-13 from 53bb57a529 to 519124ecdc

2026-10-16 15:36:08 +02:00

Compare

omp-deck added 1 commit

2026-10-16 15:36:45 +02:00

fix(server): let PR review sessions choose how to read code 1e4e04e9cd

The review instruction no longer names the tools for reading code, so
the agent can pick whatever suits it, including a local checkout once
one is available.

Co-Authored-By: Claude Opus 5.5 via omp-deck

lucas approved these changes

2026-10-16 15:37:54 +02:00

lucas merged commit 3c565e42fe into main

2026-10-16 15:38:02 +02:00

lucas deleted branch 30-forgejo-explicit-review-request-and-pr-review-sessions-plan-step-13

2026-10-16 15:38:02 +02:00

lucas referenced this pull request from a commit

2026-10-16 15:38:03 +02:00

Merge pull request 'Forgejo: review requests and PR review sessions' (#32) from 30-forgejo-explicit-review-request-and-pr-review-sessions-plan-step-13 into main

Sign in to join this conversation.

No reviewers