feat(forgejo): submitted reviews continue the implementer session on its own PR #51

Merged
lucas merged 4 commits from 45-forgejo-submitted-reviews-continue-the-implementer-session-on-its-own-pr into main 2026-10-21 15:39:27 +02:00
Collaborator

A review submitted on a pull request the agent authored now starts a turn in that PR's implementer session. Any verdict counts and no @mention is needed. The webhook accepts the three review event types. The dispatcher checks that the reviewer can push, using the API, because the review payload carries the PR author's permission. The turn's seed has the reviewer, the verdict, the review body, and the review id and line-comment count. If the PR has no implementer session, one is created in the PR's clone. It is scoped to the issue the PR closes, or to the PR alone, and its seed tells it to catch up first. Implementer scopes gain an optional pull, so resume restores the right clone and the session can't open a second pull request.

Decisions

Posted in #51 (comment):

  • Widened ForgejoTriggerSummary.event / .action in packages/protocol to include the review events. No migration is needed (the columns are TEXT).
  • be7e811 doesn't typecheck on its own; 57f2e55 fixes it. git_rebase can't squash or fixup.
  • reviewerCanWrite has no permanent unit test, since there's no ForgejoClient stub pattern. All seven paths were checked once with a throwaway script against a local HTTP stub.
  • The review is the reviewer's newest one (highest id) with the event's verdict. If none matches, the seed says so and the agent finds it.

Verification

  • bun test in apps/server: 850 pass, 0 fail. New tests cover the webhook classifier and route, the dispatcher review gate and routing (9 tests), and the implementer policy and orientation for PR-started scopes.
  • bun run typecheck (protocol, server, web): pass.
  • Throwaway check of reviewerCanWrite against a local HTTP stub: all 7 cases as expected.
  • I didn't run the manual end-to-end check against a live Forgejo.

Closes #45

A review submitted on a pull request the agent authored now starts a turn in that PR's implementer session. Any verdict counts and no @mention is needed. The webhook accepts the three review event types. The dispatcher checks that the reviewer can push, using the API, because the review payload carries the PR author's permission. The turn's seed has the reviewer, the verdict, the review body, and the review id and line-comment count. If the PR has no implementer session, one is created in the PR's clone. It is scoped to the issue the PR closes, or to the PR alone, and its seed tells it to catch up first. Implementer scopes gain an optional `pull`, so resume restores the right clone and the session can't open a second pull request. ## Decisions Posted in https://git.sclu1034.dev/lucas/omp-deck/pulls/51#issuecomment-16422: - Widened `ForgejoTriggerSummary.event` / `.action` in `packages/protocol` to include the review events. No migration is needed (the columns are `TEXT`). - `be7e811` doesn't typecheck on its own; `57f2e55` fixes it. `git_rebase` can't squash or fixup. - `reviewerCanWrite` has no permanent unit test, since there's no `ForgejoClient` stub pattern. All seven paths were checked once with a throwaway script against a local HTTP stub. - The review is the reviewer's newest one (highest id) with the event's verdict. If none matches, the seed says so and the agent finds it. ## Verification - `bun test` in `apps/server`: 850 pass, 0 fail. New tests cover the webhook classifier and route, the dispatcher review gate and routing (9 tests), and the implementer policy and orientation for PR-started scopes. - `bun run typecheck` (protocol, server, web): pass. - Throwaway check of `reviewerCanWrite` against a local HTTP stub: all 7 cases as expected. - I didn't run the manual end-to-end check against a live Forgejo. Closes #45
An optional `pull` on the implementer scope records the pull request the
session was started from: its clone is restored on resume, it counts as
the session's pull request, and a pull request that names no issue gets
its own orientation text. All implementer sessions are told that
submitted reviews arrive as turns.

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

Decisions and notes for review:

  • Widened the trigger summary types in packages/protocol. This wasn't in the plan. ForgejoTriggerSummary.event / .action now include the three review event types and reviewed. Without that the stored trigger rows don't typecheck. No DB migration: the columns are plain TEXT.
  • be7e811 doesn't typecheck on its own. The type fix is the separate commit 57f2e55 that follows it. git_rebase doesn't allow squash or fixup, so I couldn't fold it in.
  • reviewerCanWrite has no permanent unit test. There's no ForgejoClient stub pattern to build on. I checked all seven paths once with a throwaway script against a local Bun.serve stub, and didn't commit it: 200 write → true, 200 read → false, 403 + owner → true, 403 + team member → true, 403 + no matching team → false, 403 + teams 404 → false, 500 → error. The dispatcher tests use FakeApi for the gate.
  • Finding the review: the dispatcher takes the newest review (highest id) by the reviewer whose state matches the event (APPROVED / REQUEST_CHANGES / COMMENT). If none matches, or the API call fails, the seed tells the agent to find the newest review itself.
Decisions and notes for review: - **Widened the trigger summary types in `packages/protocol`.** This wasn't in the plan. `ForgejoTriggerSummary.event` / `.action` now include the three review event types and `reviewed`. Without that the stored trigger rows don't typecheck. No DB migration: the columns are plain `TEXT`. - **`be7e811` doesn't typecheck on its own.** The type fix is the separate commit `57f2e55` that follows it. `git_rebase` doesn't allow squash or fixup, so I couldn't fold it in. - **`reviewerCanWrite` has no permanent unit test.** There's no `ForgejoClient` stub pattern to build on. I checked all seven paths once with a throwaway script against a local `Bun.serve` stub, and didn't commit it: 200 write → true, 200 read → false, 403 + owner → true, 403 + team member → true, 403 + no matching team → false, 403 + teams 404 → false, 500 → error. The dispatcher tests use `FakeApi` for the gate. - **Finding the review:** the dispatcher takes the newest review (highest id) by the reviewer whose `state` matches the event (`APPROVED` / `REQUEST_CHANGES` / `COMMENT`). If none matches, or the API call fails, the seed tells the agent to find the newest review itself.
lucas force-pushed 45-forgejo-submitted-reviews-continue-the-implementer-session-on-its-own-pr from 96b7a1c9fe to 64f284079f 2026-10-20 14:34:16 +02:00 Compare
lucas approved these changes 2026-10-21 15:39:13 +02:00
lucas merged commit 76630b3dbc into main 2026-10-21 15:39:27 +02:00
lucas deleted branch 45-forgejo-submitted-reviews-continue-the-implementer-session-on-its-own-pr 2026-10-21 15:39:27 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
lucas/omp-deck!51
No description provided.