feat(review): add applicable code suggestions

lambadalambda commented

2026-08-26 11:36:12 +00:00

Owner

Summary

backport Forgejo applicable review suggestions from upstream PR forgejo/forgejo#13023
support suggestion creation, preview, single apply, and atomic batch apply
support same-repository and fork pull requests

Pleroma hardening

bind apply requests to the exact comment content versions that were previewed
reject ambiguous legacy comments containing multiple suggestion blocks
require an exact stored preimage before applying multiline suggestions
preserve private author, committer, and co-author email settings
validate suggestion invariants at service boundaries and preflight API review batches

Testing

focused Go tests for Markdown parsing, preview generation, file splicing, preimage validation, and private attribution
SQLite integration suite for suggestion rendering, single and batch apply, stale/edit races, forks, permissions, and quotas
all suggestion integration tests pass

A full services/repository/files package run also encountered the sandbox's unrelated TestGetDiffPreview process restriction (operation not permitted); focused suggestion tests and the complete suggestion integration suite pass.

## Summary - backport Forgejo applicable review suggestions from upstream PR [forgejo/forgejo#13023](https://codeberg.org/forgejo/forgejo/pulls/13023) - support suggestion creation, preview, single apply, and atomic batch apply - support same-repository and fork pull requests ## Pleroma hardening - bind apply requests to the exact comment content versions that were previewed - reject ambiguous legacy comments containing multiple suggestion blocks - require an exact stored preimage before applying multiline suggestions - preserve private author, committer, and co-author email settings - validate suggestion invariants at service boundaries and preflight API review batches ## Testing - focused Go tests for Markdown parsing, preview generation, file splicing, preimage validation, and private attribution - SQLite integration suite for suggestion rendering, single and batch apply, stale/edit races, forks, permissions, and quotas - all suggestion integration tests pass A full `services/repository/files` package run also encountered the sandbox's unrelated `TestGetDiffPreview` process restriction (`operation not permitted`); focused suggestion tests and the complete suggestion integration suite pass.

lambadalambda added 4 commits

2026-08-26 11:36:12 +00:00

feat: allow user to create suggestion block in a comment 758f8b5725

feat: allow user to apply a single suggestion 46a0e86065

feat: allow user to batch apply suggestions 3c5678f7a8

fix(review): harden suggestion application

opencode/review OpenCode execution completed

Details

c861bdc5cf

lambadalambda commented

2026-08-26 11:44:18 +00:00

Author

Owner

/oc review

lambadalambda added 2 commits

2026-08-26 12:46:33 +00:00

fix(review): hide controls for pending suggestions 5c85e34f3b

fix(review): preserve dominant line endings

opencode/review OpenCode execution completed

Details

c7399227b1

lambadalambda commented

2026-08-26 12:47:25 +00:00

Author

Owner

Addressed both OpenCode P2 findings:

Pending review suggestions now render as plain code without apply/batch controls, matching apply-time eligibility (5c85e34f3b). Added an integration regression test.
Mixed-EOL files now use the dominant line ending when applying suggestions, with LF on ties (c7399227b1). Added LF-majority, CRLF-majority, and tie unit coverage.

Verification:

go test -tags 'sqlite sqlite_unlock_notify' ./services/repository/files -run '^Test(SuggestionCommitIdentityRespectsEmailPrivacy|SuggestionRangePreimage|SuggestionLines|SpliceLines)$' -count=1
go test -tags 'sqlite sqlite_unlock_notify' ./services/gitdiff -count=1
Complete suggestion SQLite integration suite (render, single/batch apply, fork, and quota cases)

Addressed both OpenCode P2 findings: - Pending review suggestions now render as plain code without apply/batch controls, matching apply-time eligibility (`5c85e34f3b`). Added an integration regression test. - Mixed-EOL files now use the dominant line ending when applying suggestions, with LF on ties (`c7399227b1`). Added LF-majority, CRLF-majority, and tie unit coverage. Verification: - `go test -tags 'sqlite sqlite_unlock_notify' ./services/repository/files -run '^Test(SuggestionCommitIdentityRespectsEmailPrivacy|SuggestionRangePreimage|SuggestionLines|SpliceLines)$' -count=1` - `go test -tags 'sqlite sqlite_unlock_notify' ./services/gitdiff -count=1` - Complete suggestion SQLite integration suite (render, single/batch apply, fork, and quota cases)

lambadalambda commented

2026-08-26 12:48:42 +00:00

Author

Owner

/oc review

opencode-bot commented

2026-08-26 13:12:46 +00:00

Owner

OpenCode Review

Execution completed for pleroma/forgejo-custom at c7399227b1ee (job #48).

Rating: 5/5

The PR adds GitHub-style review suggestions: a ```suggestion fenced block with a server-rendered before/after preview in code comments, plus single and batch apply that commits to the PR head branch (fork-aware) with author/committer attribution and Co-authored-by trailers. Verification at head c739922: both previously reported P2s are fixed and covered by tests — pending-review comments no longer render or accept apply controls (SuggestionDiffs and resolveSuggestionEdit both gate on review state, with render and rejection integration tests), and apply now preserves the file's dominant line ending with LF on ties (unit coverage for LF/CRLF/tie). Independent checks: all touched Go packages build and vet cleanly; markdown, gitdiff, files, models/issues, services/pull and router unit tests pass; the complete new suggestion integration suites (render, single apply, batch, overlap, fork, quota, permissions, closed/merged/AGit gating) pass under sqlite; ESLint clean and webpack build succeeds. Security review confirms CSRF via the web route group, sign-in and repo assignment, comment-to-PR binding, head-branch write via CanMaintainerWriteToBranch, quota charged to the head-repo owner, LFS/binary/oversize rejection, exact preimage guards for multi-line writes, sound overlap and batch-limit handling, and archived-repo/closed-PR fail-closed behavior. Remaining observations (a no-op suggestion toolbar button when editing a code comment from the Conversation tab, uncached per-comment git check-attr during render, rare concurrent-apply race surfacing as a 500) are polish-level and non-blocking. Ready to merge.

No findings were reported.

Production-ready: both prior P2 findings verified fixed at head with regression coverage; builds, vet, unit tests, integration suites, lint and frontend build all pass locally; security and data-integrity gating on the new apply path is sound. No blocking or medium-impact defects found.

No companion fix was needed because the fresh review found no fixable findings.

Last reviewed commit: c7399227b1ee

## OpenCode Review _Execution completed for `pleroma/forgejo-custom` at `c7399227b1ee` (job #48)._ Rating: **5/5** The PR adds GitHub-style review suggestions: a ```suggestion fenced block with a server-rendered before/after preview in code comments, plus single and batch apply that commits to the PR head branch (fork-aware) with author/committer attribution and Co-authored-by trailers. Verification at head c739922: both previously reported P2s are fixed and covered by tests — pending-review comments no longer render or accept apply controls (SuggestionDiffs and resolveSuggestionEdit both gate on review state, with render and rejection integration tests), and apply now preserves the file's dominant line ending with LF on ties (unit coverage for LF/CRLF/tie). Independent checks: all touched Go packages build and vet cleanly; markdown, gitdiff, files, models/issues, services/pull and router unit tests pass; the complete new suggestion integration suites (render, single apply, batch, overlap, fork, quota, permissions, closed/merged/AGit gating) pass under sqlite; ESLint clean and webpack build succeeds. Security review confirms CSRF via the web route group, sign-in and repo assignment, comment-to-PR binding, head-branch write via CanMaintainerWriteToBranch, quota charged to the head-repo owner, LFS/binary/oversize rejection, exact preimage guards for multi-line writes, sound overlap and batch-limit handling, and archived-repo/closed-PR fail-closed behavior. Remaining observations (a no-op suggestion toolbar button when editing a code comment from the Conversation tab, uncached per-comment git check-attr during render, rare concurrent-apply race surfacing as a 500) are polish-level and non-blocking. Ready to merge. No findings were reported. Production-ready: both prior P2 findings verified fixed at head with regression coverage; builds, vet, unit tests, integration suites, lint and frontend build all pass locally; security and data-integrity gating on the new apply path is sound. No blocking or medium-impact defects found. _No companion fix was needed because the fresh review found no fixable findings._ --- _Last reviewed commit: `c7399227b1ee`_

lambadalambda commented

2026-08-26 13:41:18 +00:00

Author

Owner

/oc rerun