Skip to content

feat: publish inline reviews with forgejo#295

Merged
mariusvniekerk merged 8 commits into
mainfrom
inline-review-forgejo
May 26, 2026
Merged

feat: publish inline reviews with forgejo#295
mariusvniekerk merged 8 commits into
mainfrom
inline-review-forgejo

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented May 11, 2026

Copy link
Copy Markdown
Collaborator

Adds Forgejo inline review support on top of the shared draft flow:

  • Publish local inline draft comments through Forgejo pull-review APIs.
  • Ingest provider review comments back into timeline review-thread metadata during publish and sync.
  • Keep range support aligned with Forgejo's current single-line publishing behavior.
  • Tighten range validation and stale draft handling around publish, refresh, and navigation.

@mariusvniekerk

mariusvniekerk commented May 11, 2026

Copy link
Copy Markdown
Collaborator Author

@roborev-ci

roborev-ci Bot commented May 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (0a05603)

No Medium, High, or Critical findings were reported.

All review agents found no actionable issues at or above medium severity.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci

roborev-ci Bot commented May 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (52ebd12)

No Medium, High, or Critical issues found across the reviews.

All agents reported the code is clean.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 52ebd12 to 79ab0f9 Compare May 11, 2026 12:42
@roborev-ci

roborev-ci Bot commented May 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (79ab0f9)

All reviewed agents report no Medium, High, or Critical issues.

No findings to include.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from f1614aa to 699cada Compare May 11, 2026 13:47
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 79ab0f9 to 008f993 Compare May 11, 2026 13:47
@roborev-ci

roborev-ci Bot commented May 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (008f993)

No Medium, High, or Critical issues were found.

The only reported finding was Low severity and is omitted per the review rules.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk marked this pull request as ready for review May 12, 2026 00:00
@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 699cada to 20e4437 Compare May 15, 2026 22:39
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 008f993 to 9dacaa0 Compare May 15, 2026 22:40
@roborev-ci

roborev-ci Bot commented May 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (9dacaa0)

Medium-risk issue found.

Medium

  • internal/platform/forgejo/diff_review.go:83 - ListMergeRequestReviewThreads dereferences review.ID without checking whether review is nil. A Forgejo-compatible server could return a reviews array containing null, causing a panic during sync or PR detail refresh and creating a remote denial-of-service risk against the local dashboard.

    Suggested fix: validate each review before dereferencing it, either by skipping nil entries or returning a provider error.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci

roborev-ci Bot commented May 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (ddf3e90)

No Medium, High, or Critical findings were reported.

The only finding was Low severity and has been omitted per instructions.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 20e4437 to 4c2446f Compare May 15, 2026 23:56
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from ddf3e90 to 4716152 Compare May 15, 2026 23:57
@roborev-ci

roborev-ci Bot commented May 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (4716152)

No Medium, High, or Critical findings were reported.

The only reported issue was Low severity, so it is omitted per the review rules.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 4c2446f to f3c2ab9 Compare May 16, 2026 01:01
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 4716152 to 5fa8c5a Compare May 16, 2026 01:01
@roborev-ci

roborev-ci Bot commented May 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (5fa8c5a)

No Medium, High, or Critical issues found.

Both completed reviews report the code is clean.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci

roborev-ci Bot commented May 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (5fa8c5a)

No Medium, High, or Critical findings were reported.

All reviewed agents found the changes clean.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from f3c2ab9 to 61d6509 Compare May 16, 2026 23:46
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 5fa8c5a to 9a56ce9 Compare May 16, 2026 23:46
@roborev-ci

roborev-ci Bot commented May 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (9a56ce9)

High severity issue found: Forgejo review publishing can apply approvals to an unreviewed head commit.

High

  • internal/platform/forgejo/diff_review.go:31
    Forgejo review publication only sets CommitID from the first inline comment. If a maintainer publishes an approve/request-changes review with no inline comments, commitID remains empty and CreatePullReview is called without pinning the review to the reviewed head SHA.

    A PR author could push or force-push after the maintainer reviewed the diff but before approval publication, causing the approval to apply to a newer unreviewed commit.

    Remediation: require and pass the expected PR head SHA for every Forgejo review publish, including body-only reviews. Reject publication when the stored draft/diff head no longer matches the current PR head SHA.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk added the stacked part of a stacked diff label May 17, 2026
@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 61d6509 to 7968fff Compare May 17, 2026 14:15
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 9a56ce9 to 03884ca Compare May 17, 2026 14:16
@roborev-ci

roborev-ci Bot commented May 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (03884ca)

No Medium, High, or Critical issues found.

All reviewers reported the changes as clean.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci

roborev-ci Bot commented May 22, 2026

Copy link
Copy Markdown

roborev: Combined Review (bfc6890)

No Medium, High, or Critical findings were reported.

Security review found no issues; the other reviews did not provide actionable findings above the reporting threshold.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 0c64b4d to 3f1acd5 Compare May 23, 2026 22:17
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from bfc6890 to 3688fbe Compare May 23, 2026 22:17
@roborev-ci

roborev-ci Bot commented May 23, 2026

Copy link
Copy Markdown

roborev: Combined Review (3688fbe)

Clean: no Medium, High, or Critical findings were reported.

Security review found no issues; the skipped review produced no findings to merge.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Note: gemini review skipped (agent quota exhausted)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 3f1acd5 to 6a82108 Compare May 25, 2026 14:50
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 3688fbe to c7efae1 Compare May 25, 2026 14:50
@roborev-ci

roborev-ci Bot commented May 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (c7efae1)

Summary verdict: No medium-or-higher issues were reported.

All review agents found the code clean or reported no actionable findings at Medium severity or above.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-baseline branch from 6a82108 to bfb745f Compare May 25, 2026 20:51
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from c7efae1 to 49f413a Compare May 25, 2026 20:51
@roborev-ci

roborev-ci Bot commented May 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (49f413a)

Summary verdict: No actionable Medium, High, or Critical findings were reported.

No Medium-or-higher findings to include.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 49f413a to 1c5fbbc Compare May 26, 2026 11:45
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (1c5fbbc)

No Medium, High, or Critical findings were reported.

All review agents found no reportable issues.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Base automatically changed from inline-review-baseline to main May 26, 2026 16:51
mariusvniekerk and others added 3 commits May 26, 2026 12:53
Adds Forgejo/Gitea inline review publishing, provider range validation, review thread recovery, and regression coverage for Forgejo-backed review flows.
Mutations invalidate pending draft loads. Clear the loading flag at the same boundary so failed mutations or successful discards cannot leave the inline review draft store permanently loading while the stale request skips cleanup.
Use the current go.kenn.io module path in Forgejo inline review files after rebasing the stack onto the renamed module.
@mariusvniekerk mariusvniekerk force-pushed the inline-review-forgejo branch from 1c5fbbc to 19e0f03 Compare May 26, 2026 16:54
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (19e0f03)

No Medium, High, or Critical findings were reported.

All agents agree the reviewed code is clean.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Update the Forgejo review-thread API assertions to use the provider-neutral diff_thread response shape introduced by the landed baseline branch.
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (c2541eb)

No medium-or-higher issues were found across the reviews.

All agents reported the reviewed diff as clean for reportable findings.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Poll the workspace PR monitor in the association test so race-enabled CI does not fail when the workspace branch and MR metadata are not observed on the first pass.
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (7998775)

Verdict: No Medium, High, or Critical findings were reported.

All review outputs are clean or contain no actionable findings above the requested severity threshold.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Resend the resize and size probe while waiting for tmux-backed runtime terminal output so CI does not fail when startup output arrives before the helper sees the probe.
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (b1c5ae1)

No Medium, High, or Critical findings were reported.

All review agents provided no actionable findings at or above the requested severity threshold.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Have the tmux-backed terminal helper report its size periodically instead of relying on binary input being consumed before tmux echoes it during CI startup.
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (0d3d217)

Medium issue found: the new review capabilities are wired for Forgejo but not the analogous Gitea path.

Medium

  • internal/platform/forgejo/client.go:131
    The new inline review draft and review thread capabilities are enabled only for Forgejo, even though Gitea already uses the same gitealike pull review surface in this repo. This leaves Gitea users without the new review publishing/thread recovery path, contrary to the provider support rule that new features work across supported providers where the API allows.

    Fix: Move the shared review draft/thread implementation into the gitealike layer, or add equivalent Gitea client support and capability wiring. Only disable it per provider if Gitea lacks a required endpoint.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Accept the workspace PR association when it has already been persisted by the background monitor before the explicit test monitor pass returns an update.
@roborev-ci

roborev-ci Bot commented May 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (fee8f60)

No actionable Medium, High, or Critical findings were reported.

All review outputs are clean or contain no file/line findings to include.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk merged commit 25b9c6f into main May 26, 2026
18 of 28 checks passed
@mariusvniekerk mariusvniekerk deleted the inline-review-forgejo branch May 26, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stacked part of a stacked diff

Development

Successfully merging this pull request may close these issues.

1 participant