feat(review): opt-in, comment-driven merge-conflict resolution

review/README.md

@@ -13,3 +13,18 @@ mechanical non-semantic fixes. Logic changes, safety-sensitive code, lifecycle
 or termination paths, and test changes are suggestion/comment-only so a human
 author owns them. It sends a message on Slack when a PR is ready for your
 review or can merge the PR if you approve.
+
+## Resolving merge conflicts (opt-in)
+
+Comment **`@relay fix conflicts`** on a PR and the agent resolves its merge
+conflicts: cloud merges the base branch into the working tree, the agent
+resolves the conflict markers (preserving both sides' intent, never weakening
+tests or flipping safety defaults), verifies the merged tree against the repo's
+CI command, and cloud finalizes and pushes the merge commit. A conflict that
+needs human judgment is left in place and called out in a comment rather than
+guessed at — so a risky half-merge is never pushed.
+
+This never runs on its own; only an explicit directive comment triggers it, and
+only from the PR author or a login in `APPROVERS` / `REVIEW_AUTHORS` (when those
+are set). It is enabled by the `conflictResolve` capability in `persona.ts` and
+depends on cloud support for the merge-in-tree + finalize-push flow.

review/agent.ts

@@ -62,6 +62,12 @@ const MERGE_ON_GREEN_LABEL = 'merge-on-green';
 const AGENT_WORKFORCE_ORG = 'agentworkforce';
 const SLACK_THREAD_TAG = 'pr-reviewer:slack-thread';
 
+// The opt-in directive a commenter posts to ask for merge-conflict resolution.
+// Accepts "@relay fix conflicts" / "@relay-bot resolve conflict" (case- and
+// whitespace-insensitive). Deliberately narrow: it must be an explicit ask, not
+// any mention, so an ordinary "there's a conflict here" comment never fires it.
+const CONFLICT_DIRECTIVE_PATTERN = /@relay(?:-?bot)?\s+(?:fix|resolve)\s+conflicts?\b/i;
+
 function vfsClient(): IntegrationClientOptions {
   return { relayfileMountRoot: resolveMountRoot({}) };
 }
@@ -78,6 +84,9 @@ export default defineAgent({
       { on: 'check_run.completed' },
       { on: 'pull_request.synchronize' },
       { on: 'issues.labeled' },
+      // Opt-in merge-conflict resolution: a PR-conversation comment carrying the
+      // CONFLICT_DIRECTIVE phrase asks the agent to resolve conflict markers.
+      { on: 'issue_comment.created' }
     ],
     slack: [
       {
@@ -110,6 +119,30 @@ export default defineAgent({
     return;
   }
 
+  // A PR-conversation comment opts this PR into merge-conflict resolution, but
+  // ONLY when it carries the directive phrase and comes from an authorized
+  // commenter. Every other issue_comment is ignored here (ordinary review still
+  // runs off the pull_request / review_comment triggers).
+  if (event.type === 'github.issue_comment.created') {
+    if (!matchesConflictDirective(commentBody(data))) return;
+    const pr = readPr(data);
+    if (!pr) return;
+    const commander = commenterLogin(data);
+    if (!isAuthorizedConflictCommander(ctx, commander, pr)) {
+      ctx.log?.('info', 'pr-reviewer conflict directive ignored: commander not authorized', {
+        owner: pr.owner, repo: pr.repo, number: pr.number, commander: commander || 'unknown',
+      });
+      return;
+    }
+    const skip = await shouldSkipReview(ctx, pr);
+    if (skip) {
+      ctx.log?.('info', 'pr-reviewer conflict directive skipped', { owner: pr.owner, repo: pr.repo, number: pr.number, reason: skip.reason });
+      return;
+    }
+    await resolveConflicts(ctx, pr);
+    return;
+  }
+
   // A check run that finished without failing needs no action.
   if (event.type === 'github.check_run.completed' && !ciFailed(data)) return;
 
@@ -331,6 +364,32 @@ async function reviewAndFix(ctx: WorkforceCtx, pr: Pr): Promise<void> {
   }
 }
 
+// ── conflict resolution (opt-in, comment-driven) ────────────────────────────
+// Cloud has merged the base branch into the working tree before this runs, so
+// conflict markers sit in the conflicted files (listed in
+// `.workforce/conflicted-files.txt`). The harness resolves the markers; cloud
+// finalizes the merge commit and pushes it after the harness exits. The harness
+// itself never runs git — same boundary as ordinary review.
+async function resolveConflicts(ctx: WorkforceCtx, pr: Pr): Promise<void> {
+  const run = await ctx.harness.run({
+    cwd: ctx.sandbox.cwd,
+    prompt: conflictResolveHarnessPrompt(pr)
+  });
+
+  const exitCode = (run as { exitCode?: unknown }).exitCode;
+  if (typeof exitCode === 'number' && exitCode !== 0) {
+    await failReviewRun(ctx, pr, `The conflict-resolution harness exited with code ${exitCode}.`);
+  }
+
+  const body = (run.output ?? '').trim();
+  if (!body) {
+    await failReviewRun(ctx, pr, 'The conflict-resolution harness produced no output.');
+  }
+  if (body) {
+    await githubClient().comment({ owner: pr.owner, repo: pr.repo, number: pr.number }, body);
+  }
+}
+
 // Announce "ready for your review" at most once per head commit. Re-reviews
 // fire on many webhooks (a later check completing, a new bot comment) and the
 // PR is no more "ready" than the last time we said so — re-announcing is the
@@ -511,6 +570,42 @@ export function reviewHarnessPrompt(pr: { owner: string; repo: string; number: n
   ].join('\n');
 }
 
+export function conflictResolveHarnessPrompt(pr: { owner: string; repo: string; number: number }): string {
+  return [
+    `Resolve the merge conflicts on pull request #${pr.number} in ${pr.owner}/${pr.repo}. The PR is checked out in the`,
+    `current directory and cloud has already merged the base branch into the working tree, so conflict markers`,
+    `(<<<<<<<, =======, >>>>>>>) are present in the conflicted files. Read .workforce/conflicted-files.txt for the`,
+    `exact list, and .workforce/pr.diff plus .workforce/context.json to understand what this PR changed versus base.`,
+    `Resolve EVERY conflict with the smallest correct merge that preserves BOTH sides' intent: understand what each`,
+    `side changed and why, then combine them so neither change is silently dropped. Do not blindly pick one side.`,
+    `Remove every conflict marker you resolve — leave no <<<<<<<, =======, or >>>>>>> behind in any file you touch.`,
+    `Do NOT use git or the gh CLI. Cloud finalizes the merge commit and pushes it to the PR after you exit; your job`,
+    `is only to leave a correctly merged working tree. Do not claim the merge was committed or pushed — that is a`,
+    `post-harness action cloud reports separately.`,
+    `Stay strictly within resolving the conflicts. Do not refactor, "harden", or fold in unrelated changes while`,
+    `merging — a conflict resolution that smuggles in extra edits is how an unrelated build breaks.`,
+    `Never resolve a conflict by changing a semantic or safety default: do not turn a fail-closed state into a`,
+    `fail-open one (a "timeout"/"pending"/throw/undefined becoming "acked"/true/{}/a success path), do not swap a`,
+    `truthiness check for a presence check, and do not alter guard default values to make the merge simpler.`,
+    `Never touch lifecycle, termination, reaper, in-flight, dispatch, broker ownership, or process-cleanup code to`,
+    `resolve a conflict; if the conflict is in one of those areas, treat it as needing human judgment (below).`,
+    `Never weaken or delete a test to resolve a conflict: do not skip it, loosen an assertion, or replace a real`,
+    `assertion with a trivially-true one. When both sides changed a test, merge both expectations honestly.`,
+    `If a conflict genuinely needs human judgment — the two sides are semantically incompatible, combining them is`,
+    `ambiguous or risky, or it lands in safety-critical code — do NOT guess. Leave that file's conflict markers in`,
+    `place, and list the file under a "## Unresolved conflicts" heading with a one-line reason. Cloud aborts the`,
+    `merge and posts your explanation when any conflict is left unresolved, so a risky half-merge is never pushed.`,
+    `After resolving, verify the merged tree the way CI does — run the repo's canonical build/test/typecheck command`,
+    `end to end (read package.json / turbo.json / the CI workflow to find what CI runs; install dependencies if`,
+    `needed) so you catch breakage caused by combining the two sides, not just within one conflicted file. If you`,
+    `cannot make the merged tree pass and cannot fix it without human judgment, leave the remaining markers and`,
+    `record them under "## Unresolved conflicts" rather than pushing an unverified merge.`,
+    `Account for every conflicted file in your output: list each resolved file under a "## Resolved conflicts"`,
+    `heading with a one-line note on how you combined the two sides (e.g. "src/foo.ts — kept base's retry plus the`,
+    `PR's new timeout arg"), and list any you left for a human under "## Unresolved conflicts".`
+  ].join('\n');
+}
+
 async function verifyReadyForHumanReview(ctx: WorkforceCtx, pr: Pr): Promise<boolean> {
   try {
     const state = await readPrReviewState(pr);
@@ -1163,52 +1258,102 @@ function slackThreadTags(pr: Pr, channel: string): string[] {
 // ── parsing the github webhook payload ──────────────────────────────────────
 // The PR lives in different places per event: `pull_request` (opened /
 // synchronize / review / review_comment), `check_run.pull_requests[0]`
-// (check_run.completed), or the top-level `number`.
+// (check_run.completed), `issue` (issue_comment.created — the issue IS the PR
+// when `issue.pull_request` is present), or the top-level `number`.
 export function readPr(payload: unknown): Pr | undefined {
   const p = payload as {
     number?: number;
-    issue?: {
+    pull_request?: {
       number?: number;
       html_url?: string;
-      pull_request?: unknown;
+      user?: { login?: string };
+      head?: { sha?: string };
+      state?: string;
+      merged?: boolean;
+      draft?: boolean;
       labels?: unknown;
     };
-    pull_request?: {
+    issue?: {
       number?: number;
       html_url?: string;
       user?: { login?: string };
-      head?: { sha?: string };
       state?: string;
-      merged?: boolean;
       draft?: boolean;
       labels?: unknown;
+      // Present only when the issue is actually a pull request. Its absence is
+      // how a plain issue_comment is told apart from a PR comment.
+      pull_request?: unknown;
     };
     check_run?: { pull_requests?: Array<{ number?: number; html_url?: string; head_sha?: string }> };
     repository?: { name?: string; owner?: { login?: string } };
     sender?: { login?: string };
   } | null;
-  const issuePr = p?.issue?.pull_request ? p.issue : undefined;
-  const prRef = p?.pull_request ?? p?.check_run?.pull_requests?.[0] ?? issuePr;
+  // For issue_comment / issues.labeled, only treat the issue as a PR when
+  // GitHub marks it one (the `pull_request` field is present).
+  const prIssue = p?.issue?.pull_request != null ? p.issue : undefined;
+  const prRef = p?.pull_request ?? p?.check_run?.pull_requests?.[0] ?? prIssue;
   const number = prRef?.number ?? p?.number;
   const owner = p?.repository?.owner?.login;
   const repo = p?.repository?.name;
   // Validate `number` is a real integer — it's interpolated into a shell command.
   if (typeof number !== 'number' || !Number.isInteger(number) || !owner || !repo) return undefined;
   const headSha = p?.pull_request?.head?.sha ?? p?.check_run?.pull_requests?.[0]?.head_sha;
+  // On issue_comment, `issue.user.login` is the PR opener and `sender.login` is
+  // the commenter — prefer the opener so the author allowlist gates on the right
+  // person; fall back to sender only for PR-shaped payloads without an opener.
+  const author =
+    p?.pull_request?.user?.login ??
+    prIssue?.user?.login ??
+    ((p?.pull_request || prIssue) ? p?.sender?.login : undefined) ??
+    'unknown';
+  const state = p?.pull_request?.state ?? prIssue?.state;
+  const draft = typeof p?.pull_request?.draft === 'boolean' ? p.pull_request.draft : prIssue?.draft;
+  const labels = p?.pull_request?.labels ?? prIssue?.labels;
   return {
     owner,
     repo,
     number,
     url: prRef?.html_url ?? `https://github.com/${owner}/${repo}/pull/${number}`,
-    author: p?.pull_request?.user?.login ?? (p?.pull_request ? p?.sender?.login : undefined) ?? 'unknown',
+    author,
     ...(headSha ? { headSha } : {}),
-    ...(p?.pull_request?.state ? { state: p.pull_request.state } : {}),
+    ...(state ? { state } : {}),
     ...(typeof p?.pull_request?.merged === 'boolean' ? { merged: p.pull_request.merged } : {}),
-    ...(typeof p?.pull_request?.draft === 'boolean' ? { draft: p.pull_request.draft } : {}),
-    ...(p?.pull_request?.labels !== undefined ? { labels: p.pull_request.labels } : {}),
-    ...(p?.issue?.labels !== undefined ? { labels: p.issue.labels } : {})
+    ...(typeof draft === 'boolean' ? { draft } : {}),
+    ...(labels !== undefined ? { labels } : {})
   };
 }
+/** The body of an issue_comment webhook, or '' when absent. */
+export function commentBody(payload: unknown): string {
+  const body = (payload as { comment?: { body?: unknown } } | null)?.comment?.body;
+  return typeof body === 'string' ? body : '';
+}
+/** The login of whoever wrote the comment (NOT the PR author), lowercased. */
+export function commenterLogin(payload: unknown): string {
+  const login = (payload as { comment?: { user?: { login?: unknown } } } | null)?.comment?.user?.login;
+  return typeof login === 'string' ? login.trim().toLowerCase() : '';
+}
+/** Whether a comment body carries the opt-in conflict-resolution directive. */
+export function matchesConflictDirective(body: string): boolean {
+  return CONFLICT_DIRECTIVE_PATTERN.test(body);
+}
+
+/**
+ * Who may command a conflict resolution. Resolving conflicts force-updates the
+ * PR branch, so don't take the order from just anyone: a bot never qualifies
+ * (it's also how the loop is broken if a fix-bot echoes the phrase), and when
+ * APPROVERS/REVIEW_AUTHORS are configured the commenter must be the PR's own
+ * author or appear on one of those trust lists. With neither list set the
+ * persona stays open (matching its other allowlists' "unset → everyone").
+ */
+export function isAuthorizedConflictCommander(ctx: WorkforceCtx, commander: string, pr: Pr): boolean {
+  if (!commander || commander.endsWith('[bot]')) return false;
+  const approvers = (input(ctx, 'APPROVERS') ?? '').split(',').map((s) => s.trim().toLowerCase()).filter(Boolean);
+  const reviewAuthors = reviewAuthorAllowlist(ctx);
+  if (approvers.length === 0 && reviewAuthors.size === 0) return true;
+  if (commander === (pr.author ?? '').trim().toLowerCase()) return true;
+  return approvers.includes(commander) || reviewAuthors.has(commander);
+}
+
 function isApproval(payload: unknown): boolean {
   return (payload as { review?: { state?: string } } | null)?.review?.state?.toLowerCase() === 'approved';
 }

review/persona.ts

@@ -12,6 +12,20 @@ export default definePersona({
   description: 'Reviews new PRs, applies only lint/format/typo fixes, comments on logic or safety findings, pings you on Slack when ready, and merges once you approve.',
   cloud: true,
 
+  // Opt-in, comment-driven merge-conflict resolution. Distinct from cloud's
+  // deterministic `conflictAutofix` (which only does a clean rebase and aborts
+  // on real textual conflicts): `conflictResolve` is the LLM path — cloud merges
+  // the base branch into the working tree, the harness resolves the conflict
+  // markers (see conflictResolveHarnessPrompt in agent.ts), and cloud finalizes
+  // and pushes the merge commit. It engages ONLY when an authorized commenter
+  // posts the directive (see CONFLICT_DIRECTIVE_PATTERN), never on ordinary PR
+  // events. Inert until cloud models this capability + the merge-in-tree setup
+  // (tracked in AgentWorkforce/cloud): with no merge, the harness finds no
+  // markers and makes no change.
+  capabilities: {
+    conflictResolve: { directive: '@relay fix conflicts', resolveMarkers: true }
+  },
+
   integrations: {
     github: {},
     slack: {