feat(mt#1341): Add suggestion field to ReviewComment for GitHub suggestion blocks#906
Merged
Conversation
…stion blocks - Add optional to ReviewComment interface - Validate suggestion line count matches anchored range in validateReviewComment - Map suggestion to fenced suggestion block in submitReview body - Extend MCP schema in sessionPrReviewSubmitCommandParams with suggestion, startLine, and startSide fields - Add 17 new unit tests covering validation and body mapping
![minsky-reviewer[bot]](https://avatars.githubusercontent.com/u/52695?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
conclude_review call. Event derived from severity counts: REQUEST_CHANGES (1 BLOCKING / 3 NON-BLOCKING / 0 PRE-EXISTING findings). Executive summary unavailable.
Findings
- [BLOCKING] src/domain/repository/github-pr-review.ts:205 — Suggestion fence is always appended with a trailing newline — may produce unintended extra blank line in GitHub rendering
When buildingresolvedBody, the code constructs${c.body}\n\n\``suggestion\n${c.suggestion}\n```regardless of whetherc.suggestionalready ends with a newline. If the suggestion text already contains a final newline (which your validator tolerates), this produces two trailing newlines inside the fence (...\n\n````).
While GitHub's suggestion parser typically tolerates this, the rendering may include an extra blank line or could, in edge cases, miscount lines if there are platform-specific line endings. To keep mapping deterministic and aligned with the validator, normalizesuggestionbefore appending (e.g., strip all trailing newlines and then add exactly one) and mirror the same normalization in tests. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:112 — Only a single trailing newline is stripped from
suggestionwhen counting lines
In validateReviewComment(), the line count logic removes at most one trailing newline:comment.suggestion.endsWith("\n") ? comment.suggestion.slice(0, -1) : comment.suggestion.
If the provided suggestion text inadvertently has multiple trailing newlines (e.g., generated content or copy-paste), the extra newlines will be counted as additional blank lines and cause a false mismatch against the anchored range.
Consider trimming all trailing newlines (e.g., using a regex like/\n+$/or a loop) before splitting to make the validator resilient to accidental extra trailing newlines. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:101 — No guard against backticks inside
suggestion— may break fenced block rendering
The mapping wrapssuggestioninside a Markdown fenced block using triple backticks. If the suggestion text itself contains the sequence ``` (triple backticks), the fence will be prematurely terminated and GitHub may fail to render the suggestion (or render malformed Markdown). Consider either validating and rejecting suggestions that contain triple backticks, or escaping/transforming them (e.g., replacing with indented code or using a longer fence with more backticks) to preserve a valid fence. - [NON-BLOCKING] src/adapters/shared/commands/session/session-parameters.ts:741 — Session params schema accepts
suggestionbut does not document line-ending normalization or backtick constraints
You added thesuggestionoptional field to the comments schema with a brief description about line-count matching. Given the stricter rules in the domain validator (line count must match; GitHub requires exact counts; fencing with triple backticks), operators may be confused when validations fail due to trailing newlines or backticks inside the suggestion. Consider elaborating in the.describe(...)text: mention that trailing newlines are trimmed to 1, that suggestions should avoid triple backticks, and that the count is based on\nseparators after normalization.
Inline comments
- src/domain/repository/github-pr-review.test.ts:153 — These mapping tests cover the happy path, but there’s no test asserting behavior when the suggestion already includes a trailing newline. Since the validator tolerates a trailing
\n, a test should verify the body mapping normalizes to exactly one newline inside the fence (or at least that it doesn’t produce an extra blank line). - src/domain/repository/github-pr-review.test.ts:137 — Edge case to consider: suggestion strings containing triple backticks (```), which would break the fenced block. It may be worth adding a validator rejection or escaping strategy and a corresponding test.
…lding Strip all trailing newlines from c.suggestion before inserting into the fenced suggestion block, preventing double-blank-lines when the input ends with multiple newlines. Mirror the same normalization in validateReviewComment line-count logic (was stripping at most one trailing newline; now strips all). Add two unit tests: one for the mapping path and one for the validator path.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Solid feature addition with good tests, but I found multiple robustness gaps. The validator and mapper don’t normalize CRLF line endings, leading to off-by-one line counts and stray \r in fences. The body fence uses fixed triple backticks; if the suggestion contains ``` it will prematurely close and break GitHub rendering. The MCP schema permits invalid combinations (multi-line suggestions without a multi-line anchor), pushing avoidable errors to runtime. Tests mirror the production logic instead of the actual mapping, risking false confidence if both share the same bug. Please address these issues; minor spacing before the fence can also be tightened.
Findings
- [BLOCKING] src/domain/repository/github-pr-review.ts:108 — Suggestion line-count normalization ignores CRLF (\r\n) line endings, causing off-by-one and residual \r characters
In validateReviewComment(), trailing newline stripping and line counting usecomment.suggestion.replace(/\n+$/, "")andsplit("\n")(around lines 108–116). This fails for CRLF inputs: e.g., a suggestion ending with "a\r\n\r\n" becomes "a\r\n\r" (only the\nare stripped), andsplit("\n")yields an extra empty segment with residual\r. This can produce incorrectsuggestionLineCountand spurious validation failures or passes depending on content, and will also leak stray\rinto the fenced block. Fix by normalizing Windows line endings before counting and stripping, e.g.: - Normalize:
const text = comment.suggestion.replace(/\r\n/g, "\n"); - Strip:
text.replace(/\n+$/, "") - Count with
text.split("\n").length(or split on/\r?\n/). - [BLOCKING] src/adapters/shared/commands/session/session-parameters.ts:700 — MCP schema allows suggestion on multi-line ranges without requiring startLine/startSide; validator will accept, but GitHub may reject (no range with multi-line suggestion)
The CLI/MCP schema (sessionPrReviewSubmitCommandParams) adds optional fields startLine, startSide, and suggestion (lines ~700–715), but does not assert that when suggestion spans multiple lines, startLine must be present to form a multi-line anchor. The domain validator only enforces line-count equality if startLine is provided; it does not prevent providing a multi-line suggestion for a single-line anchor — it will throw, but the schema allows constructing such payloads. This is a UX/API contract gap: clients can produce invalid combinations that will always error at runtime. Consider schema-level refinement: ifsuggestioncontains more than one line (after normalizing trailing newlines), requirestartLine, and optionally requirestartSideto matchsidewhen present. - [BLOCKING] src/domain/repository/github-pr-review.ts:199 — Suggestion fence is not robust when suggestion text contains triple backticks; fence will prematurely terminate and break GitHub suggestion parsing
In submitReview() when buildingresolvedBody, the code hardcodes a triple-backtick suggestion fence:${c.body}\n\nsuggestion\n${normalizedSuggestion}\nnormalizedSuggestioncontains a line with "```" (common in Markdown files or code snippets), the inner triple backticks will close the fence early, producing malformed Markdown and likely preventing GitHub from rendering the suggestion button. To fix, choose a fence delimiter that is guaranteed not to appear in the content: detect the longest run of backticks innormalizedSuggestionand use a backtick fence longer than that (e.g., start with "````suggestion" and end with the same), or use tildes ("~~~~suggestion"). Add corresponding tests for suggestions containing backticks. - [NON-BLOCKING] src/domain/repository/github-pr-review.test.ts:170 — Tests mirror production mapping logic; increases risk of false confidence if both share the same flaw
The test suite’s buildApiComment() re-implements the production mapping (lines ~156–188) rather than invoking the actual submitReview mapping. This makes the tests susceptible to mirror-drift (or mirroring the same bug), as evidenced by both using identicalreplace(/\n+$/, "")and simple triple-backtick fences. Consider extracting the mapping into a shared pure function exported from the module and importing it in tests, or using an integration test that exercises submitReview with a mocked Octokit to validate the exact payload constructed. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:200 — Potential extra blank line before closing fence if body ends with trailing whitespace/newlines
resolvedBody prefixes the suggestion fence with"\n\n"(line ~200). Ifc.bodyalready ends with a newline, this yields three consecutive newlines before the fence, adding unintended spacing in rendering. Prior review flagged trailing-newline handling inside the fence; you handled trailing newlines in suggestion, but not before the fence. Consider trimming only trailing newlines fromc.bodywhen appending the fence or document the intended spacing.
Inline comments
- src/domain/repository/github-pr-review.ts:205 — What happens if
suggestioncontains a line with triple backticks (```)? The fence will close early and likely break the suggestion. Consider choosing a fence length that exceeds any backtick run in the content (or use tildes) and add a test case covering this. - src/domain/repository/github-pr-review.ts:112 — Line counting strips only
\nat the end and splits on\n. CRLF (\r\n) inputs will miscount and leave stray\rcharacters. Recommend normalizing line endings first:const s = suggestion.replace(/\r\n/g, "\n").replace(/\n+$/, "")thens.split("\n").length. - src/adapters/shared/commands/session/session-parameters.ts:711 — Schema allows any string for
suggestion. To reduce runtime rejections, consider a refinement: ifsuggestionhas >1 line (after trimming trailing newlines), requirestartLineto be present; ifstartLineis present, also assertline > startLine. You could also enforce side/startSide equality at schema level.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Feature addition is solid with comprehensive tests, but two blocking robustness issues remain. The validator and mapper only normalize '\n' and ignore CRLF, leaving stray '\r' and risking line-count/render anomalies on Windows-origin suggestions. The fixed triple-backtick fence is unsafe if suggestion content contains ``` and can prematurely close the block. Non-blocking: add Zod refinements to fail earlier on invalid combos, avoid mirroring production mapping in tests to prevent shared-bug masking, and consider trimming excessive newlines between body and fence. Address CRLF normalization and fence delimiter safety before merging.
Findings
- [BLOCKING] src/domain/repository/github-pr-review.ts:100 — CRLF not normalized: validator and mapper only strip "\n", leaving stray "\r" and miscount risk on Windows line endings
In validateReviewComment, the suggestion normalization usescomment.suggestion.replace(/\n+$/, "")and counts lines viasplit("\n"). This ignores\r\nendings. For Windows-style input, the trailing\nis removed but\rremains at the end of the last line, and any interior lines retain\rcharacters. This can leak\rinto the fenced block and, depending on how input is constructed (e.g., suggestion ending with only\r\n), can cause off-by-one when suggestions are assembled or rendered by GitHub. The same issue exists in the submitReview mapper wherenormalizedSuggestionis built with the same regex before embedding in the fence. Please normalize line endings first (e.g.,text.replace(/\r\n?/g, "\n")) and then strip trailing newlines before counting and embedding. - [BLOCKING] src/domain/repository/github-pr-review.ts:205 — Unescaped fenced code block delimiter: suggestion content containing
will prematurely close the suggestion block In submitReview's body mapping (and mirrored in tests), the fence is hard-coded as triple backticks: `````suggestion` followed by `${normalizedSuggestion}` and a closing ````` on their own lines. If `c.suggestion` contains a line with three backticks (), GitHub Markdown will terminate the fence early, breaking the suggestion rendering or exposing raw fence syntax. This is a known pitfall when embedding arbitrary content in backtick fences. Consider choosing a fence delimiter that avoids conflict (e.g., use a longer sequence of backticks based on the maximum run in the content, or use GitHub's suggestion syntax with tildes), or escape/transform the content to ensure the fence is not prematurely closed. Add tests where suggestion includes "```" to verify correct rendering. - [NON-BLOCKING] src/adapters/shared/commands/session/session-parameters.ts:604 — MCP schema allows suggestion without enforcing multi-line range consistency at parameter layer
The Zod schema adds optionalstartLine,startSide, andsuggestion, but it does not constrain combinations (e.g., suggestion withstartLineabsent implies single-line; multi-line requires startLine < line; startSide must match side). These constraints are enforced at runtime byvalidateReviewComment, but pushing some invariants down to the schema (via refinements) would yield earlier, clearer CLI/MCP errors and better DX. Consider refining: (1) ifstartLineis present, requirestartLine < line; (2) ifstartSideandsideare both present, require equality; (3) ifsuggestionis present andstartLineabsent, enforce single-line suggestion (no\n) to fail fast. - [NON-BLOCKING] src/domain/repository/github-pr-review.test.ts:116 — Test helper mirrors production mapping, risking tautology and shared-bug masking
The localbuildApiCommentin tests replicates the production suggestion mapping, including the same normalization (replace(/\n+$/, "")) and backtick fence format. If both share a bug (e.g., CRLF mishandling or fence delimiter collision), tests will pass despite incorrect behavior. Better to test via submitReview mapping or keep helper logic minimal and assert the exact strings only from the production function, possibly by extracting a pure function from github-pr-review.ts and importing it, to avoid duplication. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:205 — Potential extra blank line before closing fence when suggestion already ends with newline(s) — partially addressed but not documented
You normalize trailing newlines inside the fence (replace(/\n+$/, "")) so the interior ends with exactly one newline before the closing backticks. That addresses double-blank-lines inside. However, the outer${c.body}\n\n...always inserts two blank lines between body and fence; ifbodyitself ends with newline(s), this results in 3+ consecutive newlines before the fence. While GitHub generally collapses whitespace, it's worth either trimmingbodyor documenting that callers should avoid trailing newlines inbodyto keep rendering tight. Consider normalizingbodysimilarly or adding a small test.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| Add suggestion?: string field to ReviewComment with JSDoc | Met | src/domain/repository/github-pr-review.ts:51-79 adds suggestion?: string with detailed JSDoc about constraints and behavior. |
Owner
|
@minsky-reviewer please re-review — fixes pushed addressing prior round's findings. |
…h for suggestions BLOCKING 1: In validateReviewComment and the submitReview mapper, apply text.replace followed by stripping trailing newlines before line-counting or embedding the suggestion. This prevents carriage returns from leaking into fenced blocks and fixes miscounts when suggestion uses Windows-style line endings. BLOCKING 2: In the submitReview mapper and the test-local buildApiComment helper, compute the longest run of backticks in the suggestion content and use a fence delimiter of max(3, longestRun+1) backticks. This prevents early fence termination when suggestion contains triple-backtick sequences. New tests added: - validateReviewComment: 3 CRLF/CR normalization cases - Octokit payload mapping: CRLF no-leak case, dynamic fence length case
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Solid addition of suggestion support with good normalization and dynamic fence handling; prior CRLF and fence-length issues appear resolved and are covered by tests. However, two concerns remain. First, the implementation allows suggestions on LEFT/base-side anchors, which GitHub typically cannot apply; this should be validated and rejected early with tests. Second, empty/whitespace-only suggestions are currently accepted as valid single-line suggestions, which is likely unintended; consider rejecting them explicitly. I’ve also noted a schema-level refinement gap and a brittle test expectation as non-blocking. Address the blocking items before merging.
Findings
- [BLOCKING] src/domain/repository/github-pr-review.ts:100 — No guard against suggestions on LEFT (base) side comments; GitHub suggestions generally only apply on RIGHT/head side
validateReviewComment enforces range and side consistency and validates suggestion line counts, but it does not reject or warn when a suggestion is attached to a LEFT-side (base/deleted) anchor. See src/domain/repository/github-pr-review.ts:83-123. GitHub’s apply-suggestion feature only works when the suggestion can patch the current head; comments anchored on the LEFT/base side (including deletions) typically won’t render an “Apply suggestion” button and/or will be rejected. This change enables constructing such invalid suggestions silently. Please add a validation such as: if suggestion is present, require the effective side (side ?? startSide ?? "RIGHT") to be "RIGHT" (and for multi-line, that startSide, when provided, is also RIGHT). Also add a unit test that attempts a LEFT-side suggestion and expects a MinskyError. - [BLOCKING] src/domain/repository/github-pr-review.ts:102 — Empty or whitespace-only suggestions are allowed and will pass validation as a 1-line suggestion
In validateReviewComment (src/domain/repository/github-pr-review.ts:83-123), the guard only checkscomment.suggestion !== undefined. An empty string "" normalizes to "" andsplit("\n").lengthis 1, so a single-line anchor will accept an empty suggestion. Similarly, a whitespace-only suggestion (e.g., " \t\n") is normalized to an empty last line and may be miscounted or accepted unexpectedly. GitHub suggestions that replace code with an empty string are typically invalid or at least surprising and may be rejected or render poorly. Please add a validation thatsuggestion.trim().length > 0(after line-ending normalization) and reject empty/whitespace-only suggestions with a descriptive MinskyError. Add tests covering empty and whitespace-only suggestion inputs. - [NON-BLOCKING] src/adapters/shared/commands/session/session-parameters.ts:704 — Schema does not constrain suggestion/range invariants at parameter layer
The Zod schema now includes startLine, startSide, and suggestion (src/adapters/shared/commands/session/session-parameters.ts:704-724), but it does not refine combinations. For better DX and earlier feedback, consider adding refinements: (1) if startLine is present, require startLine < line; (2) if both side and startSide are present, require equality; (3) if suggestion is present, require that (a) effective side is RIGHT and (b) startLine is present when a multi-line suggestion is intended; (4) forbid empty/whitespace-only suggestion. These validations currently occur (partially) at runtime in validateReviewComment; duplicating some at the schema layer would surface errors before hitting the network. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:200 — [NEEDS VERIFICATION] Suggestions on LEFT/base side may not be applicable in GitHub UI
The mapper allows suggestion fences regardless of diff side by computingresolvedSide = c.side ?? c.startSide ?? "RIGHT"and embedding the fence whenc.suggestionis present (src/domain/repository/github-pr-review.ts:200-239). GitHub’s one-click suggestion apply typically operates on the head (RIGHT) side; suggestions anchored to the LEFT/base side (including deletions) may fail to render as apply-able or be rejected. This is an external API/UX behavior; please confirm against GitHub docs or a real PR. If LEFT-side suggestions are unsupported, add a validator to reject them early with a clear error and a unit test. [NEEDS VERIFICATION: external GitHub behavior]
Inline comments
- src/domain/repository/github-pr-review.test.ts:247 — The multi-line suggestion fence test hardcodes a triple-backtick fence in the expected string. Since buildApiComment now computes a dynamic fence length based on backticks in content, this expectation can become brittle if suggestion text contains backticks (even by future refactor). Consider asserting the fence structure more generally (e.g., regex that captures the opening fence and reuses it to check the closing fence, as you did later for the backtick-containing case).
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Good progress: CRLF normalization and dynamic backtick fences address prior blockers, and tests cover many cases. However, the current implementation strips all trailing newlines from suggestions in both validation and mapping, which prevents representing a replacement whose final line is intentionally blank. This alters semantics and can cause false validation failures for legitimate suggestions; please preserve meaningful trailing blank lines. Additionally, empty suggestions currently pass validation for single-line anchors and produce empty fenced blocks; clarify and enforce intended semantics. Once trailing blank-line handling is fixed, this should be in solid shape.
Findings
- [BLOCKING] src/domain/repository/github-pr-review.ts:106 — Stripping all trailing newlines prevents representing suggestions whose final replacement line is intentionally blank
Both validateReviewComment and the submitReview mapper normalize and then strip trailing newlines via.replace(/\n+$/, "")before counting and embedding the suggestion (validator: lines 106–114; mapper: lines 212–224). This makes it impossible to propose a replacement where the last line is intentionally empty. Example: for a 2-line range where the intended replacement is ["A", ""], callers would naturally pass"A\n\n". After stripping, this becomes"A", the validator computes 1 line (mismatch) and the mapper would embed only "A", changing semantics. GitHub suggestions support trailing blank lines as part of the replacement; the implementation should count and preserve an intentionally blank final line rather than unconditionally stripping it. Consider stripping at most one terminal newline for single-line cases, or tracking line breaks separately (e.g., split into lines without discarding the final empty element) and preserving content in the fence. - [BLOCKING] src/domain/repository/github-pr-review.ts:212 — Mapper also strips all trailing newlines from suggestion, altering intended replacements with a final blank line
In submitReview's API mapping,normalizedSuggestionis built with.replace(/\n+$/, "")(around lines 212–220) and then embedded in the fenced block. This mirrors the validator behavior and discards an intentionally empty final line (e.g., a 2-line replacement where the second line is blank, passed as "A\n\n"). The result is the body suggesting only "A", changing semantics and likely failing GitHub's line-count requirements. Preserve an intentional trailing blank line in the content and ensure the fence includes the correct final newline without collapsing meaningful trailing empties. - [NON-BLOCKING] src/domain/repository/github-pr-review.ts:100 — Validation permits empty suggestion content after normalization, likely producing an empty fenced block
In validateReviewComment (around lines 100–123),suggestionTextis normalized and trailing newlines stripped, then line count is computed viasplit("\n").length. For an input likesuggestion: ""or"\n\n",suggestionTextbecomes an empty string, andsplit("\n").lengthyields 1, so a single-line anchor passes. The mapper will then produce a fenced suggestion block with no replacement content. GitHub suggestions typically require meaningful replacement text; an empty suggestion is ambiguous (delete vs. no-op) and may be rejected or confusing to users. Consider enforcingsuggestionText.length > 0after normalization (and for multi-line anchors, ensuring it contains anchoredLineCount lines, including intentional blank ones) and/or documenting intended semantics for deletions.
edobry
added a commit
that referenced
this pull request
May 1, 2026
Resolves the merge conflict between this branch and main. Main has advanced with PRs #896 (mt#1343 review threads), #898 (mt#1342 thread resolve), #906 (mt#1341 suggestion blocks), and #907 (mt#1348 review state labels). All four touch overlapping surface (github-pr-review.ts, github.ts, repository index, session-parameters, workflow-commands, session.ts, pr-subcommands) — and #898 also added new files this branch needs (pr-review-thread-resolve subcommand and adapter). This commit copies main main content for the new and updated files so the in-progress submitCheckRun additions in this branch can co-exist without the no-marker abort path of session_update (mt#1303). Files added or replaced: - diff-anchor-validator.ts (new from #887 mt#1347) - review-state-labels.ts (new from #907 mt#1348) - github-pr-review.ts (updated by #898 #906 #907) - github-pr-operations.ts (updated by #896) - pr-review-context-subcommand.ts (updated by #896) - pr-review-thread-resolve-subcommand.ts (new from #898) - pr-review-thread-resolve-command.ts (new from #898) - github-pr-review.test.ts (updated by #906) - github-pr-operations-review-threads.test.ts (new from #896) - github-pr-review-threads.test.ts (new from #898) - review-state-labels.test.ts (new from #907) Files merged in place to keep this branch additions plus main additions: - github.ts (review wrapper now has resolveReviewThread plus unresolveReviewThread plus submitCheckRun) - repository index.ts (ReviewOperations interface adds the same three optional methods) - session-parameters.ts (PrReviewSubmit comments schema gains startLine startSide suggestion; adds PrReviewThreadResolveCommandParams alongside PrCheckRunSubmitCommandParams) - session.ts (registers PrReviewThreadResolve and PrCheckRunSubmit) - workflow-commands.ts (re-exports the same) - pr-subcommands.ts (barrel exports the same) Typecheck clean. 123 of 123 tests pass across the 6 reviewer-output test files. Next step is session_update to fast-forward this branch to main now that all conflicts are pre-resolved by content match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional
suggestionfield toReviewCommentthat GitHub renders as a one-click "Apply suggestion" button. When present, the forge mapping wraps the comment body with a fenced suggestion block. A pre-flight validator enforces that the suggestion's line count matches the anchored range.Motivation & Context
GitHub renders
suggestionfenced blocks inside PR review comments with a one-click apply button, making it easy for authors to accept suggested fixes. The reviewer bot (mt#1340) can now emit suggestions with exact replacement code rather than prose-only comments.Key Changes
src/domain/repository/github-pr-review.tssuggestion?: stringfield toReviewCommentinterface with full JSDocvalidateReviewComment()with suggestion line-count check: single-line anchor requires 1-line suggestion; multi-line range (startLine..line) requires(line - startLine + 1)lines; trailing newline stripped before countingsubmitReview()body mapping, whensuggestionis present the body sent to GitHub is${body}\n\n\``suggestion\n${suggestion}\n````src/adapters/shared/commands/session/session-parameters.tscomments[]schema insessionPrReviewSubmitCommandParamsto includestartLine,startSide, andsuggestionfields (the first two were missing from the MCP schema even though the domain type already supported them)src/domain/repository/github-pr-review.test.tsvalidateReviewCommenttests covering suggestion pass/fail/error-message casesOctokit payload mappingtests covering body unchanged without suggestion, single-line fence, and multi-line fencebuildApiCommenthelper to mirror the production suggestion mappingTesting
All 27 tests pass (
bun teston the test file). The 17 new tests cover: