From e115e7cb0e866488ae0f513581de8dfef626be68 Mon Sep 17 00:00:00 2001 From: Daniel Polito Date: Mon, 20 Jul 2026 10:39:42 -0300 Subject: [PATCH] feat: improve PR feedback handling - independently assess review comments before applying fixes - weight project-member intent using GitHub author metadata - confirm clean PR state after CI with a final full load --- packages/core/agents/reviewer.md | 2 + packages/core/commands/index.ts | 4 +- packages/core/commands/pr/fix.md | 20 +++++- packages/core/commands/pr/fix/loop.md | 29 +++++++-- packages/core/commands/pr/review.md | 2 +- packages/core/components/pr-fix.md | 33 +++++++--- packages/core/test/commands.test.ts | 33 ++++++++++ packages/core/test/pr-load.test.ts | 29 ++++++--- packages/core/tools/pr-load.ts | 8 +++ .../opencode/.opencode/agents/reviewer.md | 2 + .../opencode/.opencode/commands/pr/fix.md | 53 ++++++++++++---- .../.opencode/commands/pr/fix/loop.md | 61 ++++++++++++++----- .../opencode/.opencode/commands/pr/review.md | 2 +- .../web/src/components/CommandShowcase.astro | 11 ++-- .../docs/docs/reference/commands/index.mdx | 4 +- .../docs/reference/commands/pr-fix-loop.mdx | 6 +- .../docs/docs/reference/commands/pr-fix.mdx | 8 ++- .../docs/docs/reference/components/index.mdx | 2 +- .../docs/reference/tools/pr-load-review.mdx | 1 + .../docs/docs/reference/tools/pr-load.mdx | 6 +- 20 files changed, 246 insertions(+), 70 deletions(-) diff --git a/packages/core/agents/reviewer.md b/packages/core/agents/reviewer.md index 62ebaef..28c463a 100644 --- a/packages/core/agents/reviewer.md +++ b/packages/core/agents/reviewer.md @@ -48,6 +48,8 @@ Before reviewing, always check repository guidance: ## Review Stability +- Treat prior review comments as context, not ground truth; independently verify each concern against the current code and diff before repeating or relying on it +- Use available author type and repository association as weighting, not proof of whether feedback was human-authored; project members have greater authority on intent, while technical claims from every source still require evidence - Aim for stable outcomes across reruns on the same effective diff - When prior review context is provided, treat it as the baseline for consistency - If no relevant code changed since the last review, do not introduce new findings unless there is concrete new evidence or a clearly missed material defect diff --git a/packages/core/commands/index.ts b/packages/core/commands/index.ts index ce7a82d..08975ce 100644 --- a/packages/core/commands/index.ts +++ b/packages/core/commands/index.ts @@ -69,7 +69,7 @@ export const commandDefinitions: Record = { subtask: false, }, "pr/fix/loop": { - description: "Watch PR CI and comments, repeatedly fixing both without approval prompts", + description: "Assess PR CI and comments, fix valid issues, and reply without approval prompts", agent: "worker", templatePath: "commands/pr/fix/loop.md", }, @@ -90,7 +90,7 @@ export const commandDefinitions: Record = { subtask: false, }, "pr/fix": { - description: "Fix PR feedback or CI failures, push updates, and reply", + description: "Assess PR feedback and CI failures, fix valid issues, push, and reply", agent: "worker", templatePath: "commands/pr/fix.md", }, diff --git a/packages/core/commands/pr/fix.md b/packages/core/commands/pr/fix.md index fa15951..2317208 100644 --- a/packages/core/commands/pr/fix.md +++ b/packages/core/commands/pr/fix.md @@ -1,6 +1,6 @@ ## Goal -Address feedback or CI failures on a pull request, validate the fixes, push them, and respond. +Independently assess PR feedback and CI failures, implement valid fixes, push them, and respond to each current concern. ## Additional Context @@ -19,6 +19,7 @@ $ARGUMENTS - Store automatic completion requests as `` = `auto`; otherwise use `review` - Store a PR number or URL as `` and remaining guidance as `` - Leave `` undefined when absent +- Initialize `` as an empty set ### Load PR Context @@ -30,16 +31,29 @@ $ARGUMENTS ### Output -When fixes are complete, display: +When `` is `complete`, display: ``` PR fix complete for # - Changes made: files modified - Base update: -- Threads resolved: +- Feedback actionable: +- Feedback declined: +- Replies posted: - Validation passing: - Validation details: - Pushed: No additional steps are required. ``` + +When `` is `waiting for clarification`, display: +``` +PR fix waiting for clarification for # + +- Changes made: files modified +- Feedback declined: +- Feedback awaiting clarification: +- Replies posted: +- Pushed: +``` diff --git a/packages/core/commands/pr/fix/loop.md b/packages/core/commands/pr/fix/loop.md index e4c0cb9..2f976da 100644 --- a/packages/core/commands/pr/fix/loop.md +++ b/packages/core/commands/pr/fix/loop.md @@ -1,6 +1,6 @@ ## Goal -Continuously watch a pull request, fix CI failures and new review feedback, push, reply, and repeat until clean. +Continuously watch a pull request, independently assess CI failures and review feedback, fix valid issues, reply, and repeat until clean or clarification is needed. ## Additional Context @@ -20,6 +20,7 @@ $ARGUMENTS - Store a PR number or URL as `` and remaining guidance as `` - Initialize `` as `0` +- Initialize `` as an empty set ### Load Initial PR Context @@ -43,19 +44,26 @@ $ARGUMENTS - Call `<%= it.config.tools.pr_load_review.name %>` with `pr: ` and `since: ` - Store the result as `` and advance `` to `` - Merge new reviews, issue comments, and whole changed threads into ``, deduplicating by stable IDs -- Combine open actionable feedback and `` into `` -- If there is no actionable feedback and no actionable CI failure, continue to `### Output` +- Combine open candidate feedback and `` into `` without presuming that reviewer requests are valid; exclude comment IDs already handled by this workflow unless new thread context or code changes materially reopen the concern + +### Confirm Clean PR Context + +- When `` is successful or no checks are configured and `` is empty, call `<%= it.config.tools.pr_load.name %>` with `pr: ` for a final complete snapshot +- Store the result as `` and replace `` with its reviews, issue comments, and threads +- Recompute `` from the complete snapshot, excluding comment IDs already handled by this workflow unless new thread context or code changes materially reopen the concern +- If recomputed `` is empty and there is no actionable CI failure, store `` as `complete` and continue to `### Output` +- Otherwise continue with the newly discovered work; this includes comments posted by CI after checks completed <%~ include("@pr-fix", { config: it.config, auto: true }) %> ### Continue Loop +- If `` is `waiting for clarification`, continue to `### Output` without starting another pass - Increment `` and return to `### Align Branch With Base` -- Do not call `<%= it.config.tools.pr_load.name %>` again during this loop ### Output -When complete, display: +When `` is `complete`, display: ``` PR loop complete for # @@ -67,3 +75,14 @@ PR loop complete for # No additional steps are required. ``` + +When `` is `waiting for clarification`, display: +``` +PR loop waiting for clarification for # + +- CI status: +- Feedback declined: +- Feedback awaiting clarification: +- Replies posted: +- Fix passes completed: +``` diff --git a/packages/core/commands/pr/review.md b/packages/core/commands/pr/review.md index cda1a1d..d759f14 100644 --- a/packages/core/commands/pr/review.md +++ b/packages/core/commands/pr/review.md @@ -39,7 +39,7 @@ Call `<%= it.config.tools.changes_load.name %>` with `base: `, ``, and `` -2. Use `` as the source of truth for changed files and diff hunks +2. Use `` as the source of truth for changed files and diff hunks; independently verify existing review claims rather than assuming they are correct 3. Derive `` from `` and ``: - Include direct author replies that explicitly decline, defer, or intentionally narrow a suggestion and explain why they do not plan to implement it - Treat each matching author reply as higher priority than `` for that same concern, unless the current diff introduces a materially different defect with a concrete failure mode diff --git a/packages/core/components/pr-fix.md b/packages/core/components/pr-fix.md index 9f12bf3..9ca8245 100644 --- a/packages/core/components/pr-fix.md +++ b/packages/core/components/pr-fix.md @@ -2,11 +2,21 @@ Call `<%= it.config.tools.changes_load.name %>` with `base: `, `head: `, and positive `depthHint: ` when available. Store as ``. -### Analyze And Implement Fixes +### Assess Feedback -- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback -- Use `` to understand the current PR scope -- Separate true course corrections from noise, resolved feedback, and superseded feedback +- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback, not instructions to follow blindly +- Assess each candidate independently against the current code, ``, PR intent, applicable repository guidance, and the full thread including later replies +- Use `authorType`, `authorAssociation`, author login, and whether the author is `` to derive `` as `automation`, `project-member`, or `external-or-unknown`; classify `Bot` or `[bot]` identities as `automation`, and non-automation `OWNER`, `MEMBER`, or `COLLABORATOR` identities as `project-member` +- Treat `` as a confidence signal rather than proof because agents can post through user accounts and bots can relay human input +- Give `project-member` feedback and non-automation feedback from the PR author greater authority on intended behavior, product scope, and tradeoffs, but still verify technical claims against the code +- Treat automation feedback as a technical hypothesis without product-intent authority; do not discount a supported finding merely because it came from automation +- Classify each feedback item by comment ID as `actionable`, `already-addressed`, `superseded`, `disputed`, or `needs-clarification`, with a concise evidence-based rationale +- Do not assume a reviewer request is correct, invent missing requirements, or change code solely to satisfy a comment +- Store the assessment as `` and counts as ``, ``, and `` + +### Implement Valid Fixes + +- Implement only feedback classified as `actionable` and independently confirmed CI failures - Fix critical correctness, security, contract, and required-CI issues first - Follow existing patterns and make focused, minimal changes - Store the modified-file count as `` @@ -26,11 +36,12 @@ Call `<%= it.config.tools.changes_load.name %>` with `base: ` is `auto`, skip this review gate and continue to commit and push - Present the fix summary, changed-file count, and validation results - If changes were made, ask one `Review Fixes` question with `Go Ahead` and `Revise`; apply custom revision feedback and repeat implementation and validation until approved -- If no changes were made, ask one `Need Feedback` question with `Revise` and `Stop Here` +- If no changes were made but `disputed` or `needs-clarification` replies are pending, present `` and ask one `Review Feedback` question with `Post Replies` and `Revise` +- If no changes were made and no replies are pending, ask one `Need Feedback` question with `Revise` and `Stop Here` - STOP if approval is required but `question` is unavailable <% } else { -%> - Continue without an approval prompt -- If actionable work produced no changes and `` is `already up to date`, STOP to avoid looping without progress +- If actionable work produced no changes, `` is `already up to date`, and no feedback reply or clarification request is needed, STOP to avoid looping without progress <% } -%> ### Commit And Push Fixes @@ -43,6 +54,10 @@ Call `<%= it.config.tools.changes_load.name %>` with `base: ` to post short factual replies to addressed feedback -- Reply with `replies` keyed by the addressed comment IDs; use `commentBody` only for general CI feedback -- Store the number of addressed threads as `` +- After any required commit and push succeeds, use `<%= it.config.tools.pr_sync.name %>` to respond to assessed feedback +- Reply to `actionable` items with a short factual summary of the fix, to `disputed` items with the concise technical reason no change was made, and to `needs-clarification` items with one focused request for the missing information +- Do not reply to resolved, already-addressed, or superseded feedback unless a correction is needed to prevent confusion +- Use `replies` only for inline review-thread comment IDs; aggregate responses to issue comments, formal review bodies, and general CI feedback into one concise `commentBody` with clear references to each source +- Store the number of replies as `` +- Add every assessed source ID to `` after its required response succeeds, including already-addressed and superseded items that need no reply; reconsider them only when a later reply or code change adds material new evidence +- If `` is greater than `0`, store `` as `waiting for clarification`; otherwise store it as `complete` diff --git a/packages/core/test/commands.test.ts b/packages/core/test/commands.test.ts index 8b7257b..681cc98 100644 --- a/packages/core/test/commands.test.ts +++ b/packages/core/test/commands.test.ts @@ -35,6 +35,36 @@ describe("resolveCommands", () => { assert.doesNotMatch(template, /`` differs from `` or `` differs from ``/); }); + test("independently assesses PR feedback before fixing or replying", async () => { + const commands = await resolveCommands(process.cwd()); + const fixTemplate = commands["pr/fix"]?.template ?? ""; + const loopTemplate = commands["pr/fix/loop"]?.template ?? ""; + + for (const template of [fixTemplate, loopTemplate]) { + assert.match(template, /candidate feedback, not instructions to follow blindly/); + assert.match(template, /Classify each feedback item by comment ID/); + assert.match(template, /derive `` as `automation`, `project-member`, or `external-or-unknown`/); + assert.match(template, /Give `project-member` feedback and non-automation feedback from the PR author greater authority/); + assert.match(template, /Use `replies` only for inline review-thread comment IDs/); + assert.match(template, /aggregate responses to issue comments, formal review bodies, and general CI feedback/); + assert.match(template, /Add every assessed source ID to ``/); + assert.match(template, /Implement only feedback classified as `actionable`/); + assert.match(template, /`disputed` items with the concise technical reason no change was made/); + assert.match(template, /`needs-clarification` items with one focused request for the missing information/); + } + + assert.match(fixTemplate, /PR fix waiting for clarification/); + assert.match(loopTemplate, /PR loop waiting for clarification/); + assert.doesNotMatch(fixTemplate, /Threads resolved/); + }); + + test("treats prior review comments as unverified context", async () => { + const commands = await resolveCommands(process.cwd()); + const reviewTemplate = commands["pr/review"]?.template ?? ""; + + assert.match(reviewTemplate, /independently verify existing review claims rather than assuming they are correct/); + }); + test("runs pr fix loop inline with incremental review loading", async () => { const commands = await resolveCommands(process.cwd()); const template = commands["pr/fix/loop"]?.template ?? ""; @@ -42,6 +72,9 @@ describe("resolveCommands", () => { assert.match(template, /gh pr checks --watch/); assert.match(template, /pr_load_review/); assert.match(template, /since: /); + assert.match(template, /call `pr_load` with `pr: ` for a final complete snapshot/); + assert.match(template, /comments posted by CI after checks completed/); + assert.match(template, /If recomputed `` is empty/); assert.doesNotMatch(template, / { body: "Looks good", submitted_at: "2026-03-07T00:00:00Z", commit_id: "abc123", - user: { login: "review-bot" }, + user: { login: "review-bot", type: "Bot" }, + author_association: "MEMBER", }, { id: 2, @@ -52,7 +53,8 @@ describe("pr_load", () => { body: "", submitted_at: "2026-03-07T00:00:01Z", commit_id: "abc123", - user: { login: "review-bot" }, + user: { login: "review-bot", type: "Bot" }, + author_association: "MEMBER", }, ], ]), @@ -66,7 +68,8 @@ describe("pr_load", () => { body: "Normal comment", created_at: "2026-03-07T00:00:02Z", updated_at: "2026-03-07T00:00:02Z", - user: { login: "octo" }, + user: { login: "octo", type: "User" }, + author_association: "OWNER", html_url: "https://example.test/comment", }, ], @@ -95,7 +98,8 @@ describe("pr_load", () => { nodes: [ { databaseId: 12345, - author: { login: "review-bot" }, + author: { login: "review-bot", __typename: "Bot" }, + authorAssociation: "MEMBER", body: "Please simplify this block", createdAt: "2026-03-07T00:00:03Z", updatedAt: "2026-03-07T00:00:03Z", @@ -130,6 +134,8 @@ describe("pr_load", () => { id: 1, state: "COMMENTED", author: "review-bot", + authorType: "Bot", + authorAssociation: "MEMBER", body: "Looks good", submittedAt: "2026-03-07T00:00:00Z", commitId: "abc123", @@ -138,6 +144,8 @@ describe("pr_load", () => { id: 2, state: "APPROVED", author: "review-bot", + authorType: "Bot", + authorAssociation: "MEMBER", submittedAt: "2026-03-07T00:00:01Z", commitId: "abc123", }, @@ -146,6 +154,8 @@ describe("pr_load", () => { { id: 10, author: "octo", + authorType: "User", + authorAssociation: "OWNER", createdAt: "2026-03-07T00:00:02Z", updatedAt: "2026-03-07T00:00:02Z", body: "Normal comment", @@ -163,6 +173,8 @@ describe("pr_load", () => { { id: 12345, author: "review-bot", + authorType: "Bot", + authorAssociation: "MEMBER", body: "Please simplify this block", createdAt: "2026-03-07T00:00:03Z", updatedAt: "2026-03-07T00:00:03Z", @@ -401,14 +413,14 @@ describe("pr_load", () => { { contains: "gh repo view --json nameWithOwner", stdout: JSON.stringify({ nameWithOwner: "acme/repo" }) }, { contains: "gh api user", stdout: JSON.stringify({ login: "review-bot" }) }, { contains: "pulls/7/reviews?per_page=100", stdout: JSON.stringify([[{ - id: 1, state: "COMMENTED", body: "new", submitted_at: "2026-03-07T00:00:02Z", user: { login: "octo" }, + id: 1, state: "COMMENTED", body: "new", submitted_at: "2026-03-07T00:00:02Z", user: { login: "octo", type: "User" }, author_association: "MEMBER", }]]) }, { contains: "issues/7/comments?per_page=100&since=2026-03-07T00%3A00%3A00Z", stdout: JSON.stringify([[{ - id: 2, body: "updated", created_at: "2026-03-06T00:00:00Z", updated_at: "2026-03-07T00:00:03Z", user: { login: "octo" }, + id: 2, body: "updated", created_at: "2026-03-06T00:00:00Z", updated_at: "2026-03-07T00:00:03Z", user: { login: "octo", type: "User" }, author_association: "MEMBER", }]]) }, { contains: "gh api graphql", stdout: JSON.stringify({ data: { repository: { pullRequest: { reviewThreads: { pageInfo: { hasNextPage: false, endCursor: null }, - nodes: [{ id: "thread-1", isResolved: false, isOutdated: false, path: "x.ts", line: 1, comments: { nodes: [{ databaseId: 3, body: "new", createdAt: "2026-03-07T00:00:04Z", updatedAt: "2026-03-07T00:00:04Z", author: { login: "octo" } }] } }], + nodes: [{ id: "thread-1", isResolved: false, isOutdated: false, path: "x.ts", line: 1, comments: { nodes: [{ databaseId: 3, body: "new", createdAt: "2026-03-07T00:00:04Z", updatedAt: "2026-03-07T00:00:04Z", author: { login: "octo", __typename: "User" }, authorAssociation: "MEMBER" }] } }], } } } } }) }, ]); @@ -421,6 +433,9 @@ describe("pr_load", () => { assert.equal(result.reviews.length, 1); assert.equal(result.issueComments.length, 1); assert.equal(result.threads.length, 1); + assert.equal(result.reviews[0].authorType, "User"); + assert.equal(result.issueComments[0].authorAssociation, "MEMBER"); + assert.equal(result.threads[0].comments[0].authorType, "User"); assert.ok(result.loadedAt); }); }); diff --git a/packages/core/tools/pr-load.ts b/packages/core/tools/pr-load.ts index cabf09f..aa1e62a 100644 --- a/packages/core/tools/pr-load.ts +++ b/packages/core/tools/pr-load.ts @@ -122,7 +122,9 @@ const REVIEW_THREADS_QUERY = `query($owner: String!, $repo: String!, $number: In databaseId author { login + __typename } + authorAssociation body createdAt updatedAt @@ -159,6 +161,8 @@ export function simplifyReviews(reviews: any[]) { id: review.id, state: review.state, author: review.user?.login, + authorType: review.user?.type, + authorAssociation: review.author_association, ...(typeof review.body === "string" && review.body.trim().length > 0 ? { body: review.body } : {}), submittedAt: review.submitted_at, commitId: review.commit_id, @@ -170,6 +174,8 @@ export function simplifyIssueComments(comments: any[]) { return comments.map((comment) => ({ id: comment.id, author: comment.user?.login, + authorType: comment.user?.type, + authorAssociation: comment.author_association, createdAt: comment.created_at, updatedAt: comment.updated_at, body: comment.body, @@ -190,6 +196,8 @@ export function simplifyThreads(threads: any[]) { ? thread.comments.nodes.map((comment: any) => ({ id: comment.databaseId, author: comment.author?.login, + authorType: comment.author?.__typename, + authorAssociation: comment.authorAssociation, body: comment.body, createdAt: comment.createdAt, updatedAt: comment.updatedAt, diff --git a/packages/opencode/.opencode/agents/reviewer.md b/packages/opencode/.opencode/agents/reviewer.md index 0d4d30b..3b74a2e 100644 --- a/packages/opencode/.opencode/agents/reviewer.md +++ b/packages/opencode/.opencode/agents/reviewer.md @@ -56,6 +56,8 @@ Before reviewing, always check repository guidance: ## Review Stability +- Treat prior review comments as context, not ground truth; independently verify each concern against the current code and diff before repeating or relying on it +- Use available author type and repository association as weighting, not proof of whether feedback was human-authored; project members have greater authority on intent, while technical claims from every source still require evidence - Aim for stable outcomes across reruns on the same effective diff - When prior review context is provided, treat it as the baseline for consistency - If no relevant code changed since the last review, do not introduce new findings unless there is concrete new evidence or a clearly missed material defect diff --git a/packages/opencode/.opencode/commands/pr/fix.md b/packages/opencode/.opencode/commands/pr/fix.md index 2001746..6c7c61d 100644 --- a/packages/opencode/.opencode/commands/pr/fix.md +++ b/packages/opencode/.opencode/commands/pr/fix.md @@ -1,11 +1,11 @@ --- -description: Fix PR feedback or CI failures, push updates, and reply +description: Assess PR feedback and CI failures, fix valid issues, push, and reply agent: worker --- ## Goal -Address feedback or CI failures on a pull request, validate the fixes, push them, and respond. +Independently assess PR feedback and CI failures, implement valid fixes, push them, and respond to each current concern. ## Additional Context @@ -24,6 +24,7 @@ $ARGUMENTS - Store automatic completion requests as `` = `auto`; otherwise use `review` - Store a PR number or URL as `` and remaining guidance as `` - Leave `` undefined when absent +- Initialize `` as an empty set ### Load PR Context @@ -56,11 +57,21 @@ $ARGUMENTS Call `kompass_changes_load` with `base: `, `head: `, and positive `depthHint: ` when available. Store as ``. -### Analyze And Implement Fixes +### Assess Feedback -- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback -- Use `` to understand the current PR scope -- Separate true course corrections from noise, resolved feedback, and superseded feedback +- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback, not instructions to follow blindly +- Assess each candidate independently against the current code, ``, PR intent, applicable repository guidance, and the full thread including later replies +- Use `authorType`, `authorAssociation`, author login, and whether the author is `` to derive `` as `automation`, `project-member`, or `external-or-unknown`; classify `Bot` or `[bot]` identities as `automation`, and non-automation `OWNER`, `MEMBER`, or `COLLABORATOR` identities as `project-member` +- Treat `` as a confidence signal rather than proof because agents can post through user accounts and bots can relay human input +- Give `project-member` feedback and non-automation feedback from the PR author greater authority on intended behavior, product scope, and tradeoffs, but still verify technical claims against the code +- Treat automation feedback as a technical hypothesis without product-intent authority; do not discount a supported finding merely because it came from automation +- Classify each feedback item by comment ID as `actionable`, `already-addressed`, `superseded`, `disputed`, or `needs-clarification`, with a concise evidence-based rationale +- Do not assume a reviewer request is correct, invent missing requirements, or change code solely to satisfy a comment +- Store the assessment as `` and counts as ``, ``, and `` + +### Implement Valid Fixes + +- Implement only feedback classified as `actionable` and independently confirmed CI failures - Fix critical correctness, security, contract, and required-CI issues first - Follow existing patterns and make focused, minimal changes - Store the modified-file count as `` @@ -78,7 +89,8 @@ Call `kompass_changes_load` with `base: `, `head: ` is `auto`, skip this review gate and continue to commit and push - Present the fix summary, changed-file count, and validation results - If changes were made, ask one `Review Fixes` question with `Go Ahead` and `Revise`; apply custom revision feedback and repeat implementation and validation until approved -- If no changes were made, ask one `Need Feedback` question with `Revise` and `Stop Here` +- If no changes were made but `disputed` or `needs-clarification` replies are pending, present `` and ask one `Review Feedback` question with `Post Replies` and `Revise` +- If no changes were made and no replies are pending, ask one `Need Feedback` question with `Revise` and `Stop Here` - STOP if approval is required but `question` is unavailable ### Commit And Push Fixes @@ -91,22 +103,39 @@ Call `kompass_changes_load` with `base: `, `head: ` +- After any required commit and push succeeds, use `kompass_pr_sync` to respond to assessed feedback +- Reply to `actionable` items with a short factual summary of the fix, to `disputed` items with the concise technical reason no change was made, and to `needs-clarification` items with one focused request for the missing information +- Do not reply to resolved, already-addressed, or superseded feedback unless a correction is needed to prevent confusion +- Use `replies` only for inline review-thread comment IDs; aggregate responses to issue comments, formal review bodies, and general CI feedback into one concise `commentBody` with clear references to each source +- Store the number of replies as `` +- Add every assessed source ID to `` after its required response succeeds, including already-addressed and superseded items that need no reply; reconsider them only when a later reply or code change adds material new evidence +- If `` is greater than `0`, store `` as `waiting for clarification`; otherwise store it as `complete` ### Output -When fixes are complete, display: +When `` is `complete`, display: ``` PR fix complete for # - Changes made: files modified - Base update: -- Threads resolved: +- Feedback actionable: +- Feedback declined: +- Replies posted: - Validation passing: - Validation details: - Pushed: No additional steps are required. ``` + +When `` is `waiting for clarification`, display: +``` +PR fix waiting for clarification for # + +- Changes made: files modified +- Feedback declined: +- Feedback awaiting clarification: +- Replies posted: +- Pushed: +``` diff --git a/packages/opencode/.opencode/commands/pr/fix/loop.md b/packages/opencode/.opencode/commands/pr/fix/loop.md index 18d3e56..99c260e 100644 --- a/packages/opencode/.opencode/commands/pr/fix/loop.md +++ b/packages/opencode/.opencode/commands/pr/fix/loop.md @@ -1,11 +1,11 @@ --- -description: Watch PR CI and comments, repeatedly fixing both without approval prompts +description: Assess PR CI and comments, fix valid issues, and reply without approval prompts agent: worker --- ## Goal -Continuously watch a pull request, fix CI failures and new review feedback, push, reply, and repeat until clean. +Continuously watch a pull request, independently assess CI failures and review feedback, fix valid issues, reply, and repeat until clean or clarification is needed. ## Additional Context @@ -25,6 +25,7 @@ $ARGUMENTS - Store a PR number or URL as `` and remaining guidance as `` - Initialize `` as `0` +- Initialize `` as an empty set ### Load Initial PR Context @@ -69,18 +70,35 @@ $ARGUMENTS - Call `kompass_pr_load_review` with `pr: ` and `since: ` - Store the result as `` and advance `` to `` - Merge new reviews, issue comments, and whole changed threads into ``, deduplicating by stable IDs -- Combine open actionable feedback and `` into `` -- If there is no actionable feedback and no actionable CI failure, continue to `### Output` +- Combine open candidate feedback and `` into `` without presuming that reviewer requests are valid; exclude comment IDs already handled by this workflow unless new thread context or code changes materially reopen the concern + +### Confirm Clean PR Context + +- When `` is successful or no checks are configured and `` is empty, call `kompass_pr_load` with `pr: ` for a final complete snapshot +- Store the result as `` and replace `` with its reviews, issue comments, and threads +- Recompute `` from the complete snapshot, excluding comment IDs already handled by this workflow unless new thread context or code changes materially reopen the concern +- If recomputed `` is empty and there is no actionable CI failure, store `` as `complete` and continue to `### Output` +- Otherwise continue with the newly discovered work; this includes comments posted by CI after checks completed ### Load PR Changes Call `kompass_changes_load` with `base: `, `head: `, and positive `depthHint: ` when available. Store as ``. -### Analyze And Implement Fixes +### Assess Feedback + +- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback, not instructions to follow blindly +- Assess each candidate independently against the current code, ``, PR intent, applicable repository guidance, and the full thread including later replies +- Use `authorType`, `authorAssociation`, author login, and whether the author is `` to derive `` as `automation`, `project-member`, or `external-or-unknown`; classify `Bot` or `[bot]` identities as `automation`, and non-automation `OWNER`, `MEMBER`, or `COLLABORATOR` identities as `project-member` +- Treat `` as a confidence signal rather than proof because agents can post through user accounts and bots can relay human input +- Give `project-member` feedback and non-automation feedback from the PR author greater authority on intended behavior, product scope, and tradeoffs, but still verify technical claims against the code +- Treat automation feedback as a technical hypothesis without product-intent authority; do not discount a supported finding merely because it came from automation +- Classify each feedback item by comment ID as `actionable`, `already-addressed`, `superseded`, `disputed`, or `needs-clarification`, with a concise evidence-based rationale +- Do not assume a reviewer request is correct, invent missing requirements, or change code solely to satisfy a comment +- Store the assessment as `` and counts as ``, ``, and `` -- Treat unresolved review threads, review state changes, ``, and CI details in `` as candidate feedback -- Use `` to understand the current PR scope -- Separate true course corrections from noise, resolved feedback, and superseded feedback +### Implement Valid Fixes + +- Implement only feedback classified as `actionable` and independently confirmed CI failures - Fix critical correctness, security, contract, and required-CI issues first - Follow existing patterns and make focused, minimal changes - Store the modified-file count as `` @@ -96,7 +114,7 @@ Call `kompass_changes_load` with `base: `, `head: ` is `already up to date`, STOP to avoid looping without progress +- If actionable work produced no changes, `` is `already up to date`, and no feedback reply or clarification request is needed, STOP to avoid looping without progress ### Commit And Push Fixes @@ -108,18 +126,22 @@ Call `kompass_changes_load` with `base: `, `head: ` +- After any required commit and push succeeds, use `kompass_pr_sync` to respond to assessed feedback +- Reply to `actionable` items with a short factual summary of the fix, to `disputed` items with the concise technical reason no change was made, and to `needs-clarification` items with one focused request for the missing information +- Do not reply to resolved, already-addressed, or superseded feedback unless a correction is needed to prevent confusion +- Use `replies` only for inline review-thread comment IDs; aggregate responses to issue comments, formal review bodies, and general CI feedback into one concise `commentBody` with clear references to each source +- Store the number of replies as `` +- Add every assessed source ID to `` after its required response succeeds, including already-addressed and superseded items that need no reply; reconsider them only when a later reply or code change adds material new evidence +- If `` is greater than `0`, store `` as `waiting for clarification`; otherwise store it as `complete` ### Continue Loop +- If `` is `waiting for clarification`, continue to `### Output` without starting another pass - Increment `` and return to `### Align Branch With Base` -- Do not call `kompass_pr_load` again during this loop ### Output -When complete, display: +When `` is `complete`, display: ``` PR loop complete for # @@ -131,3 +153,14 @@ PR loop complete for # No additional steps are required. ``` + +When `` is `waiting for clarification`, display: +``` +PR loop waiting for clarification for # + +- CI status: +- Feedback declined: +- Feedback awaiting clarification: +- Replies posted: +- Fix passes completed: +``` diff --git a/packages/opencode/.opencode/commands/pr/review.md b/packages/opencode/.opencode/commands/pr/review.md index d339ac0..fcb4bfe 100644 --- a/packages/opencode/.opencode/commands/pr/review.md +++ b/packages/opencode/.opencode/commands/pr/review.md @@ -55,7 +55,7 @@ Call `kompass_changes_load` with `base: `, `head: `, ``, and `` -2. Use `` as the source of truth for changed files and diff hunks +2. Use `` as the source of truth for changed files and diff hunks; independently verify existing review claims rather than assuming they are correct 3. Derive `` from `` and ``: - Include direct author replies that explicitly decline, defer, or intentionally narrow a suggestion and explain why they do not plan to implement it - Treat each matching author reply as higher priority than `` for that same concern, unless the current diff introduces a materially different defect with a concrete failure mode diff --git a/packages/web/src/components/CommandShowcase.astro b/packages/web/src/components/CommandShowcase.astro index 8dfc2be..08ce90c 100644 --- a/packages/web/src/components/CommandShowcase.astro +++ b/packages/web/src/components/CommandShowcase.astro @@ -331,21 +331,21 @@ const scenarios: CommandScenario[] = [ label: '/pr/fix', command: '/pr/fix', agentName: 'Worker', - task: 'Fix review feedback, then push and reply', + task: 'Assess review feedback, fix valid issues, then reply', group: 'pr', steps: [ - { id: 'thinking', phase: 'thinking', content: 'Update the PR branch from its base when needed, address the feedback, validate the fixes, then wait for approval before pushing or replying.' }, + { id: 'thinking', phase: 'thinking', content: 'Update the PR branch, verify each comment against the code and PR intent, implement valid fixes, and explain any disagreement before pushing or replying.' }, { id: 'tools', phase: 'tool_calls', toolCalls: [ { tool: 'pr_load', args: 'current PR', status: 'complete' }, { tool: 'bash', args: 'fetch and merge PR base when behind', status: 'complete' }, - { tool: 'apply_patch', args: 'implement requested fixes', status: 'complete' }, + { tool: 'apply_patch', args: 'implement validated fixes', status: 'complete' }, { tool: 'question', args: 'Review Fixes', status: 'complete' }, { tool: 'bash', args: 'git push', status: 'complete' }, { tool: 'pr_sync', args: 'reply to addressed threads', status: 'complete' } ] }, { id: 'output', phase: 'output', output: [ 'PR fix complete for #51', - 'Threads resolved: 2', + 'Replies posted: 2', 'Pushed: yes' ] } ] @@ -440,11 +440,12 @@ const scenarios: CommandScenario[] = [ task: 'Watch PR CI and comments, then fix both hands-off', group: 'orchestration', steps: [ - { id: 'thinking', phase: 'thinking', content: 'Load the PR once, wait for checks, incrementally load review activity, apply fixes inline, then repeat until clean.' }, + { id: 'thinking', phase: 'thinking', content: 'Wait for checks, assess feedback with author context, then confirm the clean state with a final full PR snapshot before finishing.' }, { id: 'tools', phase: 'tool_calls', toolCalls: [ { tool: 'pr_load', args: 'PR #51', status: 'complete' }, { tool: 'bash', args: 'gh pr checks 51 --watch', status: 'complete' }, { tool: 'pr_load_review', args: 'since last checkpoint', status: 'complete' }, + { tool: 'pr_load', args: 'final complete snapshot', status: 'complete' }, { tool: 'pr_sync', args: 'reply to addressed feedback', status: 'complete' } ] }, { id: 'output', phase: 'output', output: [ diff --git a/packages/web/src/content/docs/docs/reference/commands/index.mdx b/packages/web/src/content/docs/docs/reference/commands/index.mdx index d8bde3b..562dad9 100644 --- a/packages/web/src/content/docs/docs/reference/commands/index.mdx +++ b/packages/web/src/content/docs/docs/reference/commands/index.mdx @@ -182,7 +182,7 @@ Creates the PR in the invoking session while retaining an authoritative committe ### `/pr/fix` -Addresses PR feedback or CI failures by making fixes and responding to review threads. +Independently assesses PR feedback and CI failures, weighs project-member and PR-author intent more heavily than automation, implements valid fixes, and responds with either the fix, a reasoned disagreement, or a clarification request. - usage: `/pr/fix [pr-or-auto-or-context]` - arguments: optional PR ref, `auto`, or additional fix guidance @@ -190,7 +190,7 @@ Addresses PR feedback or CI failures by making fixes and responding to review th ### `/pr/fix/loop` -Watches PR CI and incrementally loads review activity, then repeatedly applies the shared non-interactive fix phase until no actionable work remains. +Watches PR CI and incrementally loads review activity, then independently assesses each item and applies valid fixes until no actionable work remains or clarification is needed. A final complete PR load catches comments posted as CI finishes. - usage: `/pr/fix/loop [pr-or-context]` - arguments: optional PR ref, CI guidance, or fix scope constraints diff --git a/packages/web/src/content/docs/docs/reference/commands/pr-fix-loop.mdx b/packages/web/src/content/docs/docs/reference/commands/pr-fix-loop.mdx index b1312cb..50a7bdc 100644 --- a/packages/web/src/content/docs/docs/reference/commands/pr-fix-loop.mdx +++ b/packages/web/src/content/docs/docs/reference/commands/pr-fix-loop.mdx @@ -1,6 +1,6 @@ --- title: /pr/fix/loop -description: Watch PR CI and comments, repeatedly fixing both without approval prompts. +description: Watch PR CI and comments, independently assess each item, and fix valid issues without approval prompts. --- Use `/pr/fix/loop` when a PR should be advanced hands-off until both CI and review feedback are clean. @@ -14,9 +14,11 @@ Use `/pr/fix/loop` when a PR should be advanced hands-off until both CI and revi - loads the PR and stores the PR URL and number - watches `gh pr checks --watch` until the latest checks finish - loads review activity after the previous `loadedAt` checkpoint -- applies the shared non-interactive PR-fix phase inline +- independently classifies feedback before applying the shared non-interactive PR-fix phase inline +- after CI is green and incremental feedback appears clean, loads one final complete PR snapshot to catch late CI-generated comments - updates the PR branch from its base when it is behind - repeats after each push until no actionable CI failures or PR feedback remain +- replies with a reason when feedback is disputed and stops explicitly when reviewer clarification is needed - stops on failing validation, failed push, failed PR replies, or no-change feedback loops ## Related diff --git a/packages/web/src/content/docs/docs/reference/commands/pr-fix.mdx b/packages/web/src/content/docs/docs/reference/commands/pr-fix.mdx index 9f5e0b8..f46907f 100644 --- a/packages/web/src/content/docs/docs/reference/commands/pr-fix.mdx +++ b/packages/web/src/content/docs/docs/reference/commands/pr-fix.mdx @@ -1,6 +1,6 @@ --- title: /pr/fix -description: Fix PR feedback or CI failures with a review-first approval loop by default, then push and reply. +description: Assess PR feedback and CI failures, implement valid fixes with a review-first approval loop, then push and reply. --- ## Purpose @@ -17,11 +17,13 @@ Use `/pr/fix` when a PR already exists and review feedback or CI failures need a - load the PR and existing review state - check out the PR branch, fetch its base, and merge and push the base when the branch is behind -- implement the requested review or CI fixes on the current branch +- independently assess each comment against the current code, PR intent, repository guidance, and full thread +- give project members more authority on intent and tradeoffs while still verifying every technical claim; automation identity is treated as a signal rather than proof +- implement only validated review or CI fixes on the current branch - run relevant validation - in default mode, show the fix summary and wait for user approval before committing or pushing fix changes or replying; required base alignment is applied first - skip that approval loop only with `/pr/fix auto` -- push the updated branch and reply in the relevant review threads after commit succeeds +- push the updated branch, then reply with the fix, a technical reason for declining the request, or a focused clarification question ## Common tools diff --git a/packages/web/src/content/docs/docs/reference/components/index.mdx b/packages/web/src/content/docs/docs/reference/components/index.mdx index f3064c1..9f1ec7b 100644 --- a/packages/web/src/content/docs/docs/reference/components/index.mdx +++ b/packages/web/src/content/docs/docs/reference/components/index.mdx @@ -61,7 +61,7 @@ Checks out the PR branch, fetches its base, confirms base ancestry, and merges a ### `pr-fix` -Analyzes feedback, applies and validates fixes, commits, pushes, and responds after the caller aligns the PR branch. Callers select interactive or automatic review behavior. +Independently classifies feedback with author-type and repository-association weighting, applies and validates only supported fixes, commits, pushes, and responds with fixes, disagreements, or clarification requests. Callers select interactive or automatic review behavior. ### `push` diff --git a/packages/web/src/content/docs/docs/reference/tools/pr-load-review.mdx b/packages/web/src/content/docs/docs/reference/tools/pr-load-review.mdx index a1983a0..d282da7 100644 --- a/packages/web/src/content/docs/docs/reference/tools/pr-load-review.mdx +++ b/packages/web/src/content/docs/docs/reference/tools/pr-load-review.mdx @@ -15,6 +15,7 @@ Use `pr_load_review` after an initial `pr_load` when a long-running workflow onl - filters formal reviews by submission time - asks GitHub for issue comments updated since the checkpoint - returns a whole review thread when any comment in it changed after the checkpoint +- preserves author type and repository association so callers can distinguish automation signals from project-member intent - returns `loadedAt` for the next call GitHub does not expose a timestamp filter for review-thread resolution changes, so resolution-only changes without comment activity are not detectable incrementally. diff --git a/packages/web/src/content/docs/docs/reference/tools/pr-load.mdx b/packages/web/src/content/docs/docs/reference/tools/pr-load.mdx index 5ade503..ebedaf7 100644 --- a/packages/web/src/content/docs/docs/reference/tools/pr-load.mdx +++ b/packages/web/src/content/docs/docs/reference/tools/pr-load.mdx @@ -16,9 +16,9 @@ Use `pr_load` when a workflow needs one grounded PR context payload instead of s - `repo` - `viewerLogin` - `pr` metadata including title, body, URL, refs, commit count, and author -- `reviews` -- `issueComments` -- `threads` +- `reviews`, including author type and repository association when GitHub provides them +- `issueComments`, including author type and repository association when GitHub provides them +- `threads`, with the same metadata on each comment ## Useful for