Files
fusion/packages/engine/src/pr-comment-handler.ts
gsxdsm 0e3d2a2265 refactor: delete meta-task auto-archive and automated recovery follow-ups (#2461)
Deletes two pieces of automated "meta" machinery that filed and
garbage-collected cards restating state already on the task that failed.
Net **-1015 lines**.

## Why

**Automated recovery follow-ups.** `createAutomatedFollowup` and its
dedup engine (289 lines of signature matching, 1h recurrence
rate-limiting, 24h supersedes windows) existed to file recovery cards
for verification-cap and merge-conflict give-ups. In both cases the
parent is *already* parked `failed` with a descriptive `error` and a log
entry carrying the failing command, branch, and output — the card was a
second copy of that.

**Meta-task auto-archive.** The sweeps that garbage-collected those
cards were worse than redundant: the regex classifier matched ordinary
feature work, and its positional fallback bound cards to unrelated
tasks, so **live work could be archived**.

They are removed together, because the auto-archive sweeps only existed
to clean up after the follow-up engine.

## What changed

### Deleted
- `packages/engine/src/verification-followup-dedup.ts` in full —
`createAutomatedFollowup`, `decideAutomatedFollowup`,
`AutomatedFollowupKind`, `computeVerificationFailureSignature`,
`extractFailingTestFiles`.
- `findActiveRecoveryFollowUp` — dead code, defined and never called
(`tsc` independently flagged it `6133 declared but its value is never
read`).
- The meta-task auto-archive sweeps `autoArchiveResolvedMetaTasks` /
`autoArchiveStalledMetaTasks` and helpers `classifyMetaTask` /
`resolveMetaTargetTaskId` / `computeMetaChainDepth` / `archiveMetaTask`
/ `evaluateMetaAutoArchiveGuards`, plus settings
`metaTaskStallAutoCloseMs` and `metaTaskActiveExecutionGraceMs`.
- Run-audit types `task:auto-archived-meta-resolved`,
`task:auto-archived-meta-stalled`,
`task:auto-archive-meta-resolved-skipped`,
`task:auto-archive-meta-stalled-skipped`,
`verification:followup-created`, `verification:followup-deduped`.

The two signature helpers were **deleted rather than relocated** — once
the three call sites went they were provably unreachable:
`buildVerificationFailureSignature` had exactly one caller, and it was
the only caller of `extractFailingTestFiles`.

### Call sites 1 and 2 — park kept, card dropped
Verification-cap and merge-conflict give-ups keep their park, audit
event, operator comment, and log entry. Site 1's `error` string was
reworded off `"See follow-up task for investigation."` (no follow-up
will exist) to carry the guidance itself. `autoResolveDisabled` was
**kept** — it still drives the outer park guard and the `reason` string;
only the inner branch that guarded card creation is gone.

### Call site 3 — autostash orphan, replaced not deleted
This one is a genuine data-loss guard, so it keeps a durable trail. A
`live`-classified orphan is a merger stash holding **real uncommitted
work**, and unlike sites 1–2 there is no parked parent — the parent may
already be `done` and merged, so nothing else on the board would ever
mention the stash.

The card is replaced by a `logEntry` **and** an `addTaskComment` on the
parent, preserving every fact the old description carried: the sha,
`record.label` (the handle `git stash` recovery needs),
`record.detectedByTaskId`, and `sourcePhase`. New truthful run-audit
event `task:autostash-orphan-live-detected` replaces the borrowed
`verification:followup-*` name, with ids/outcomes-only metadata per
AGENTS.md.

### Kept unchanged: the two real product features
Eval follow-ups (`eval-followups.ts`) and PR-comment follow-ups
(`pr-comment-handler.ts`) only borrowed the shared engine for its dedup
pass. Both keep their exact behavior, column, priority, `sourceType`,
and log lines, with dedup inlined as a `listTasks` scan on
`suggestionId` / `prNumber` respectively. Both fail open (create) if the
listing throws, matching the old engine.

## Test changes — read this one

Two tests asserted the *deleted* engine's rate-limited `"[verification
recurrence]"` logEntry. Those assertions were removed, **not loosened**:
both tests still assert no duplicate card is created, and the eval test
still asserts the existing id is reported back. No coverage of surviving
behavior was weakened. The three `meta-*` test files were deleted along
with the sweeps they covered.

## Verification

```
$ pnpm test:gate
 Test Files  2 passed (2)     Tests   10 passed (10)    # core
 Test Files  16 passed (16)   Tests  299 passed (299)   # engine-core
 Test Files  1 passed (1)     Tests   70 passed (70)    # ci-shape
GATE_EXIT=0

$ pnpm --filter @fusion/engine --filter @fusion/core exec tsc --noEmit -p tsconfig.json
TSC_EXIT=0   (no output)
```

Plus a file-scoped run over the touched surfaces (`eval-followups`,
`pr-comment-handler`, `merger-autostash-orphan-surface`,
`merger-autostash-cleanup`, `run-audit`, `run-audit-secret-taxonomy`,
`project-engine`, `project-engine-manager`): **213/213 passed**.

A repo-wide grep confirms no surviving references to any deleted symbol,
module, or audit event.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Failed tasks now retain recovery and verification details directly on
the original task instead of generating separate follow-up cards.
* Live autostash issues now preserve stash information in task comments
and activity logs.
* Existing evaluation and pull-request follow-ups continue to be reused
when appropriate.

* **Changes**
  * Removed automatic archival of meta-tasks.
  * Removed obsolete meta-task timing settings.

* **Documentation**
* Updated architecture and settings documentation to reflect these
workflow changes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-26 22:38:58 -07:00

344 lines
11 KiB
TypeScript

import type { TaskStore } from "@fusion/core";
import type { PrInfo } from "@fusion/core";
import { prMonitorLog } from "./logger.js";
/*
FNXC:PullRequestReview 2026-07-26-00:00:
The PR-feedback follow-up card is a real product feature, but it used to borrow the shared automated-recovery follow-up engine (`createAutomatedFollowup` in verification-followup-dedup.ts) purely for its dedup pass. That engine was deleted along with the recovery follow-up cards it existed to file, so the one dedup rule this feature needs is inlined below: never file a second card for the same PR number under the same parent while one is still open. Closed columns (done/archived) are excluded so a later close/reopen of the same PR can legitimately file a fresh card.
*/
const CLOSED_FOLLOWUP_COLUMNS = new Set(["done", "archived"]);
interface PrComment {
id: number;
body: string;
user: { login: string };
created_at: string;
updated_at: string;
html_url: string;
}
/**
* Analyzes PR comments for actionable feedback and creates
* steering comments or follow-up tasks.
*/
export class PrCommentHandler {
// Keywords that suggest actionable feedback
private readonly ACTION_KEYWORDS = [
"fix",
"change",
"update",
"remove",
"add",
"should",
"need to",
"needs to",
"please",
"consider",
"suggest",
"recommend",
];
// Non-actionable patterns to filter out
private readonly NON_ACTIONABLE_PATTERNS = [
/^\s*lgtm\s*$/i,
/^\s*looks? good\s*$/i,
/^\s*thanks?\s*$/i,
/^\s*thank you\s*$/i,
/^\s*nice\s*$/i,
/^\s*great\s*$/i,
/^\s*awesome\s*$/i,
/^\s*👍\s*$/,
/^\s*✅\s*$/,
];
constructor(private store: TaskStore) {}
/**
* Process new PR comments for a task.
* Called by PrMonitor when new comments are detected.
*/
async handleNewComments(
taskId: string,
prInfo: PrInfo,
comments: PrComment[]
): Promise<void> {
for (const comment of comments) {
await this.processComment(taskId, prInfo, comment);
}
}
private async processComment(
taskId: string,
prInfo: PrInfo,
comment: PrComment
): Promise<void> {
// Skip non-actionable comments
if (this.isNonActionable(comment.body)) {
prMonitorLog.log(`Skipping non-actionable comment #${comment.id}`);
return;
}
// Check if comment contains actionable feedback
const isActionable = this.isActionable(comment.body);
const hasCodeSuggestions = this.hasCodeBlock(comment.body);
if (!isActionable && !hasCodeSuggestions) {
prMonitorLog.log(`Comment #${comment.id} does not contain actionable feedback`);
return;
}
// Build comment text
const text = this.buildCommentText(prInfo, comment, hasCodeSuggestions);
try {
await this.store.addTaskComment(taskId, text, "agent");
await this.upsertReviewItem(taskId, prInfo, comment, "queued");
prMonitorLog.log(`Added comment for PR review #${comment.id}`);
} catch (err) {
prMonitorLog.error(`Failed to add comment for ${taskId}:`, err);
}
}
/**
* Check if a comment is non-actionable (LGTM, thanks, etc.)
*/
private isNonActionable(body: string): boolean {
const trimmed = body.trim();
return this.NON_ACTIONABLE_PATTERNS.some((pattern) => pattern.test(trimmed));
}
/**
* Check if a comment contains actionable feedback keywords.
*/
private isActionable(body: string): boolean {
const lowerBody = body.toLowerCase();
return this.ACTION_KEYWORDS.some((keyword) => lowerBody.includes(keyword));
}
/**
* Check if a comment contains code blocks suggesting changes.
*/
private hasCodeBlock(body: string): boolean {
// Look for code blocks (``` or `code`)
return /```[\s\S]*?```/.test(body) || /`[^`]+`/.test(body);
}
/**
* Build comment text from PR review comment.
*/
private buildCommentText(
prInfo: PrInfo,
comment: PrComment,
hasCodeSuggestions: boolean
): string {
const lines: string[] = [];
lines.push(`**PR Review Feedback** from @${comment.user.login}`);
lines.push(`**PR:** #${prInfo.number} (${prInfo.status})`);
if (prInfo.status !== "open") {
lines.push(`**Note:** This PR is already ${prInfo.status}. Treat the feedback as follow-up work.`);
}
lines.push("");
// Truncate comment body if too long
const maxBodyLength = 500;
let body = comment.body.trim();
if (body.length > maxBodyLength) {
body = body.slice(0, maxBodyLength) + "...";
}
lines.push(body);
lines.push("");
if (hasCodeSuggestions) {
lines.push("💡 This comment contains code suggestions. Please review and apply if appropriate.");
}
lines.push(`[View on GitHub](${comment.html_url})`);
return lines.join("\n");
}
/**
* Handle "changes requested" PR review state.
* Moves the task back to in-progress with reviewer feedback as a steering comment,
* closing the feedback loop so the agent can address the requested changes.
*/
async handleChangesRequested(
taskId: string,
prInfo: PrInfo,
reviewerLogin: string,
reviewBody: string,
): Promise<void> {
try {
const task = await this.store.getTask(taskId);
if (task.column !== "in-review") {
prMonitorLog.log(`Task ${taskId} not in-review (${task.column}), skipping changes-requested handling`);
return;
}
// Add reviewer feedback as a steering comment
const feedbackText = [
`**Changes Requested** by @${reviewerLogin} on PR #${prInfo.number}`,
"",
reviewBody ? reviewBody.slice(0, 800) : "(no review body)",
"",
"Please address the requested changes and update the PR.",
].join("\n");
await this.store.addTaskComment(taskId, feedbackText, "agent");
await this.upsertReviewItem(
taskId,
prInfo,
{
id: Date.now(),
body: reviewBody || "(no review body)",
user: { login: reviewerLogin },
created_at: new Date().toISOString(),
updated_at: new Date().toISOString(),
html_url: prInfo.url,
},
"queued",
);
await this.store.moveTask(taskId, "in-progress");
await this.store.logEntry(
taskId,
`PR #${prInfo.number}: changes requested by @${reviewerLogin} — moved back to in-progress`,
);
prMonitorLog.log(`Task ${taskId} moved to in-progress after changes requested on PR #${prInfo.number}`);
} catch (err) {
prMonitorLog.error(`Failed to handle changes-requested for ${taskId}:`, err);
}
}
/**
* Create a follow-up task when a PR is closed with unaddressed feedback.
* This is called when a PR is merged or closed.
*/
async createFollowUpTask(
originalTaskId: string,
prInfo: PrInfo,
unaddressedComments: PrComment[]
): Promise<void> {
if (unaddressedComments.length === 0) return;
const summary = unaddressedComments
.map((c) => `- @${c.user.login}: ${c.body.slice(0, 100).trim()}${c.body.length > 100 ? "..." : ""}`)
.join("\n");
const description = `Follow-up for ${originalTaskId}
PR #${prInfo.number} was ${prInfo.status} with unaddressed feedback:
${summary}
Please review the PR comments and address any remaining issues.`;
try {
const openTasks = await this.store.listTasks({ slim: true }).catch(() => []);
const existing = openTasks.find(
(task) =>
task.id !== originalTaskId &&
!CLOSED_FOLLOWUP_COLUMNS.has(task.column) &&
task.sourceParentTaskId === originalTaskId &&
task.sourceMetadata?.prNumber === prInfo.number,
);
if (existing) {
prMonitorLog.log(`Reused follow-up task ${existing.id} for PR #${prInfo.number}`);
return;
}
const task = await this.store.createTask({
title: `Follow-up: Address PR #${prInfo.number} feedback`,
description,
column: "triage",
dependencies: [originalTaskId],
source: {
sourceType: "api",
sourceParentTaskId: originalTaskId,
sourceMetadata: { prNumber: prInfo.number, prUrl: prInfo.url },
},
});
prMonitorLog.log(`Created follow-up task ${task.id} for PR #${prInfo.number}`);
} catch (err) {
prMonitorLog.error(`Failed to create follow-up task:`, err);
}
}
private async upsertReviewItem(
taskId: string,
prInfo: PrInfo,
comment: PrComment,
status: "queued" | "in-progress" | "addressed" | "failed",
): Promise<void> {
const task = await this.store.getTask(taskId);
const now = new Date().toISOString();
const current = task.review ?? {
mode: "pull-request",
source: "github-pr",
decision: "pending",
items: [],
selectedItemIds: [],
};
const itemId = `gh-comment-${comment.id}`;
const existingIndex = current.items.findIndex((item: { id: string }) => item.id === itemId);
const nextItem = {
id: itemId,
source: "github-pr" as const,
status,
summary: comment.body.trim().slice(0, 160) || `Feedback from @${comment.user.login}`,
body: comment.body,
reviewer: comment.user.login,
commentUrl: comment.html_url,
createdAt: comment.created_at,
updatedAt: now,
};
const nextItems = [...current.items];
if (existingIndex >= 0) {
nextItems[existingIndex] = { ...nextItems[existingIndex], ...nextItem };
} else {
nextItems.push(nextItem);
}
const currentReviewState = task.reviewState ?? {
source: "pull-request" as const,
items: [],
addressing: [],
};
const existingReviewStateIndex = currentReviewState.items.findIndex((item) => item.id === itemId);
const nextReviewStateItem = {
id: itemId,
githubCommentId: comment.id,
body: comment.body,
author: { login: comment.user.login },
createdAt: comment.created_at,
updatedAt: now,
htmlUrl: comment.html_url,
source: "github-pr" as const,
};
const nextReviewStateItems = [...currentReviewState.items];
if (existingReviewStateIndex >= 0) {
nextReviewStateItems[existingReviewStateIndex] = { ...nextReviewStateItems[existingReviewStateIndex], ...nextReviewStateItem };
} else {
nextReviewStateItems.push(nextReviewStateItem);
}
await this.store.updateTask(taskId, {
review: {
...current,
mode: "pull-request",
source: "github-pr",
summary: `PR #${prInfo.number} feedback items: ${nextItems.length}`,
latestRefreshAt: now,
items: nextItems,
},
reviewState: {
...currentReviewState,
source: "pull-request",
summary: currentReviewState.summary,
items: nextReviewStateItems,
},
});
}
}