diff --git a/.agent/docs/architecture/agent-orchestrator.md b/.agent/docs/architecture/agent-orchestrator.md index d544ee96..9c53a21d 100644 --- a/.agent/docs/architecture/agent-orchestrator.md +++ b/.agent/docs/architecture/agent-orchestrator.md @@ -145,7 +145,12 @@ the same compact table style while preserving their hidden durable markers. If terminal child metadata is found but rejected by trust checks or cannot be safely updated, the dispatcher posts a compact stop comment on the current terminal issue or PR with a hidden dedupe marker. Ordinary terminal PR stops -without sub-orchestrator metadata remain silent. +without sub-orchestrator metadata finalize one visible note. The dispatcher +merges the outcome into the current planner progress comment when available; +otherwise it updates or creates a trusted comment carrying the existing +`sepo-agent-orchestrate-stop` marker. The note includes the planner summary, +source outcome, target, round, reason, run ID, and a requester mention only for +human GitHub logins. If the resumed parent planner decides there is no next child or action, the parent run posts a terminal stop comment on the parent issue with the source conclusion, target, round, reason, and hidden `sepo-agent-orchestrate-stop` @@ -153,6 +158,12 @@ marker. Exact trusted duplicates are skipped on reruns. When the planner returns `blocked` with `user_message` or `clarification_request`, that same terminal comment surfaces the planner's question directly and the chain pauses without dispatching an `answer` route. +After a validated terminal `review`/`SHIP`, `agent-self-approve`/`approved`, or +`agent-self-merge`/`merged` or `auto_merge_enabled` outcome, the dispatcher +best-effort minimizes older trusted review synthesis, rubrics review, fix-pr, +and completed handoff comments from the PR conversation. It keeps the final +note and pending handoffs visible, skips cleanup for every other outcome, and +warns without hiding the final note when cleanup fails. Initial user-launched `/orchestrate` requests validate that the requester has access to the delegated route capability set before dispatching work. When diff --git a/.agent/docs/usage/supported-workflows.md b/.agent/docs/usage/supported-workflows.md index d89a6113..97341939 100644 --- a/.agent/docs/usage/supported-workflows.md +++ b/.agent/docs/usage/supported-workflows.md @@ -74,6 +74,10 @@ memory and rubrics read-only so automated control-flow planning can use steering context without mutating those state branches. Orchestration stops when target state indicates no safe next action, a route fails, a duplicate handoff marker is found, the planner stops or blocks, or the max-round budget is exhausted. +Terminal PR stops reuse the current run's progress comment when available, or +update one trusted `sepo-agent-orchestrate-stop` marker comment otherwise. The +final note includes the planner summary and outcome metadata and mentions the +original requester only when it is a human GitHub login. When a child issue reaches a terminal stop, the handoff dispatcher resolves the trusted child metadata from the issue body or an agent-authored child issue @@ -122,6 +126,10 @@ HTML markers for robust matching, with heading/text fallbacks for older comments. Rubrics reviews match the `## Rubrics Review` heading, and orchestrator handoffs match their hidden handoff marker. This keeps the latest generated status prominent while leaving older generated comments expandable. +Successful terminal PR orchestration also best-effort minimizes older matching +conversation comments while leaving the final note and pending handoffs +visible. Blocked, clarification, failed, malformed-planner, and other +non-success outcomes never trigger this terminal cleanup. Set `AGENT_COLLAPSE_OLD_REVIEWS=false` to skip this cleanup and leave prior generated comments visible. diff --git a/.agent/src/__tests__/orchestrate-handoff-cli.test.ts b/.agent/src/__tests__/orchestrate-handoff-cli.test.ts index a7656baa..5adf10b2 100644 --- a/.agent/src/__tests__/orchestrate-handoff-cli.test.ts +++ b/.agent/src/__tests__/orchestrate-handoff-cli.test.ts @@ -52,6 +52,10 @@ if [ "\${1-}" = "pr" ] && [ "\${2-}" = "view" ]; then exit 1 fi if [[ "$*" == *"body"* ]]; then + if [ "\${FAKE_PR_BODY_MODE-}" = "error" ]; then + printf 'pull request body unavailable\\n' >&2 + exit 1 + fi printf '{"body":"%s"}\\n' "\${FAKE_PR_BODY-}" exit 0 fi @@ -90,6 +94,15 @@ if [ "\${1-}" = "api" ] && [ "\${2-}" = "--paginate" ] && [ "\${3-}" = "--slurp" exit 0 fi +if [ "\${1-}" = "api" ] && [[ "\${2-}" == repos/*/issues/comments/* ]] && [ "\${3-}" = "--jq" ] && [ "\${4-}" = ".body" ]; then + if [ "\${FAKE_PROGRESS_BODY_MODE-}" = "error" ]; then + printf 'progress comment unavailable\\n' >&2 + exit 1 + fi + printf '%s\\n' "\${FAKE_PROGRESS_BODY-}" + exit 0 +fi + if [ "\${1-}" = "api" ] && [ "\${2-}" = "--paginate" ] && [[ "\${3-}" == repos/*/issues/*/sub_issues ]]; then if [ "\${FAKE_SUB_ISSUES_MODE-}" = "error" ]; then printf 'sub-issues unavailable\\n' >&2 @@ -151,6 +164,14 @@ if [ "\${1-}" = "api" ] && [ "\${2-}" = "--method" ] && [ "\${3-}" = "PATCH" ] & exit 0 fi +if [ "\${1-}" = "pr" ] && [ "\${2-}" = "comment" ]; then + if [ "\${FAKE_PR_COMMENT_MODE-}" = "error" ]; then + printf 'pull request comment unavailable\\n' >&2 + exit 1 + fi + exit 0 +fi + if [ "\${1-}" = "api" ] && [ "\${2-}" = "-X" ] && [ "\${3-}" = "POST" ] && [[ "\${4-}" == repos/*/actions/workflows/*/dispatches ]]; then cat > "$FAKE_DISPATCH_PAYLOAD" exit 0 @@ -1501,7 +1522,7 @@ test("heuristics parent orchestrate stops do not post final comments", () => { assert.equal(run.dispatchPayload, null); }); -test("agent parent orchestrate stops for pull requests do not post final comments", () => { +test("agent parent orchestrate stops for pull requests publish a final note", () => { const run = runOrchestrateHandoff({ SOURCE_ACTION: "orchestrate", SOURCE_CONCLUSION: "done", @@ -1517,12 +1538,291 @@ test("agent parent orchestrate stops for pull requests do not post final comment assert.equal(run.status, 0, run.stderr || run.stdout); assert.equal(run.outputs.get("decision"), "stop"); assert.equal(run.outputs.get("reason"), "pull request is closed"); - assert.doesNotMatch(run.ghLog, /api --method POST repos\/self-evolving\/repo\/issues\/76\/comments/); - assert.doesNotMatch(run.ghLog, //); + assert.match(run.ghLog, /pr comment 76/); + assert.match(run.ghLog, /Sepo orchestration stopped after `orchestrate` concluded `done`\./); + assert.match(run.ghLog, /> Requested by @lolipopshock\./); + assert.match(run.ghLog, //); assert.doesNotMatch(run.ghLog, /actions\/workflows\//); assert.equal(run.dispatchPayload, null); }); +test("parent resolution failures do not suppress the terminal PR note", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "heuristics", + AGENT_COLLAPSE_OLD_REVIEWS: "false", + FAKE_PR_BODY_MODE: "error", + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.match(run.stderr, /Failed to report terminal sub-orchestration state/); + assert.match(run.ghLog, /pr comment 128/); + assert.match(run.ghLog, //); +}); + +test("terminal PR publication failures fail the handoff step", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "heuristics", + AGENT_COLLAPSE_OLD_REVIEWS: "false", + FAKE_PR_COMMENT_MODE: "error", + }); + + assert.equal(run.status, 1, run.stderr || run.stdout); + assert.match(run.stderr, /Failed to publish terminal pull request note/); +}); + +test("successful terminal PR stop merges its summary into the progress note", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "agent", + AUTOMATION_CURRENT_ROUND: "4", + AUTOMATION_MAX_ROUNDS: "8", + SOURCE_RUN_ID: "review-run-123", + AGENT_COLLAPSE_OLD_REVIEWS: "false", + AGENT_PROGRESS_COMMENT_ID: "777", + AGENT_PROGRESS_FINAL_COMMENT_MODE: "merge", + MODEL_DISPLAY: "`codex` | `gpt-test`", + FAKE_PROGRESS_BODY: [ + "### Sepo is working", + "", + "Latest planner activity", + "", + "", + ].join("\n"), + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "The reviewed implementation is ready.", + user_message: "Implementation and review completed successfully.", + }), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.equal(run.outputs.get("decision"), "stop"); + assert.match(run.ghLog, /api repos\/self-evolving\/repo\/issues\/comments\/777 --jq \.body/); + assert.match(run.ghLog, /api --method PATCH repos\/self-evolving\/repo\/issues\/comments\/777/); + assert.match(run.ghLog, /Sepo orchestration finished successfully after `review` concluded `SHIP`\./); + assert.match(run.ghLog, /Implementation and review completed successfully\./); + assert.match(run.ghLog, /> Requested by @lolipopshock\./); + assert.match(run.ghLog, //); + assert.match(run.ghLog, //); + assert.match(run.ghLog, /`codex` \| `gpt-test`/); + assert.doesNotMatch(run.ghLog, /pr comment 128/); +}); + +test("terminal PR fallback updates the trusted marker note and suppresses bot mentions", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "agent", + AUTOMATION_CURRENT_ROUND: "4", + AGENT_COLLAPSE_OLD_REVIEWS: "false", + REQUESTED_BY: "sepo-agent-app[bot]", + FAKE_ISSUE_COMMENTS_JSON: JSON.stringify([{ + id: "existing-final", + body: "Old final note\n\n", + created_at: "2026-08-10T00:00:00Z", + user: { login: "app/sepo-agent-app" }, + }]), + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "The reviewed implementation is ready.", + user_message: "Implementation and review completed successfully.", + }), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.match(run.ghLog, /api --method PATCH repos\/self-evolving\/repo\/issues\/comments\/existing-final/); + assert.doesNotMatch(run.ghLog, /pr comment 128/); + assert.doesNotMatch(run.ghLog, /Requested by/); + assert.match(run.stdout, /Updated orchestrator final comment\./); +}); + +test("successful terminal PR cleanup minimizes only older conversation artifacts", () => { + const generated = (id: string, databaseId: number, body: string) => ({ + id, + databaseId, + body, + isMinimized: false, + author: { login: "sepo-agent-app[bot]" }, + }); + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "agent", + AGENT_PROGRESS_COMMENT_ID: "777", + AGENT_PROGRESS_FINAL_COMMENT_MODE: "merge", + FAKE_PROGRESS_BODY: "### Sepo is working\n\n", + FAKE_GRAPHQL_PR_COMMENTS: JSON.stringify([ + generated("review-summary", 700, "## AI Review Synthesis\nold"), + generated("rubrics-review", 701, "## Rubrics Review\nold"), + generated("fix-status", 702, ""), + generated("handoff", 703, ""), + generated("pending", 704, ""), + generated("old-final", 705, "Old final\n"), + generated("current-final", 777, "Current final\n"), + generated("newer-review", 800, "## AI Review Synthesis\nnewer"), + { ...generated("human", 706, "## AI Review Synthesis\nhuman"), author: { login: "lolipopshock" } }, + ]), + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "The reviewed implementation is ready.", + user_message: "Implementation and review completed successfully.", + }), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + for (const id of ["review-summary", "rubrics-review", "fix-status", "handoff", "old-final"]) { + assert.match(run.ghLog, new RegExp(`id=${id} -f classifier=OUTDATED`)); + } + assert.doesNotMatch(run.ghLog, /id=pending -f classifier=OUTDATED/); + assert.doesNotMatch(run.ghLog, /id=current-final -f classifier=OUTDATED/); + assert.doesNotMatch(run.ghLog, /id=newer-review -f classifier=OUTDATED/); + assert.doesNotMatch(run.ghLog, /id=human -f classifier=OUTDATED/); + assert.doesNotMatch(run.ghLog, /PullRequestReviewSummaries/); + assert.match(run.stdout, /Collapsed 5 previous orchestration artifact comment\(s\)\./); +}); + +for (const [sourceAction, sourceConclusion] of [ + ["agent-self-approve", "approved"], + ["agent-self-merge", "merged"], + ["agent-self-merge", "auto_merge_enabled"], +] as const) { + test(`${sourceAction} ${sourceConclusion} is eligible for terminal PR cleanup`, () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: sourceAction, + SOURCE_CONCLUSION: sourceConclusion, + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "heuristics", + AGENT_PROGRESS_COMMENT_ID: "777", + AGENT_PROGRESS_FINAL_COMMENT_MODE: "merge", + FAKE_PROGRESS_BODY: "### Sepo is working\n\n", + FAKE_GRAPHQL_PR_COMMENTS: "[]", + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.match(run.ghLog, /PullRequestReviewSummaryComments/); + }); +} + +for (const [name, env] of [ + ["blocked planner", { + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + AUTOMATION_MODE: "agent", + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "blocked", + reason: "Need maintainer input.", + clarification_request: "Should this ship now?", + }), + }], + ["planner stop without summary", { + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + AUTOMATION_MODE: "agent", + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "No cumulative summary was produced.", + }), + }], + ["malformed planner", { + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + AUTOMATION_MODE: "agent", + FAKE_PLANNER_RESPONSE: "not json", + }], + ["failed source", { + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "failed", + AUTOMATION_MODE: "agent", + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "The review failed.", + user_message: "Review did not complete.", + }), + }], +] as const) { + test(`${name} does not trigger terminal PR cleanup`, () => { + const run = runOrchestrateHandoff({ + ...env, + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AGENT_PROGRESS_COMMENT_ID: "777", + AGENT_PROGRESS_FINAL_COMMENT_MODE: "merge", + FAKE_PROGRESS_BODY: "### Sepo is working\n\n", + FAKE_GRAPHQL_PR_COMMENTS: JSON.stringify([{ + id: "review-summary", + body: "## AI Review Synthesis\nold", + isMinimized: false, + author: { login: "sepo-agent-app[bot]" }, + }]), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.doesNotMatch(run.ghLog, /classifier=OUTDATED/); + }); +} + +test("enabled downstream automation prevents terminal cleanup on a budget stop", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "heuristics", + AUTOMATION_CURRENT_ROUND: "5", + AUTOMATION_MAX_ROUNDS: "5", + AGENT_ALLOW_SELF_APPROVE: "true", + FAKE_GRAPHQL_PR_COMMENTS: JSON.stringify([{ + id: "review-summary", + body: "## AI Review Synthesis\nold", + isMinimized: false, + author: { login: "sepo-agent-app[bot]" }, + }]), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.equal(run.outputs.get("reason"), "automation round budget exhausted"); + assert.doesNotMatch(run.ghLog, /classifier=OUTDATED/); +}); + +test("terminal PR cleanup failure is visible and non-fatal", () => { + const run = runOrchestrateHandoff({ + SOURCE_ACTION: "review", + SOURCE_CONCLUSION: "SHIP", + TARGET_KIND: "pull_request", + TARGET_NUMBER: "128", + AUTOMATION_MODE: "agent", + AGENT_PROGRESS_COMMENT_ID: "777", + AGENT_PROGRESS_FINAL_COMMENT_MODE: "merge", + FAKE_PROGRESS_BODY: "### Sepo is working\n\n", + FAKE_GRAPHQL_MODE: "error", + FAKE_PLANNER_RESPONSE: JSON.stringify({ + decision: "stop", + reason: "The reviewed implementation is ready.", + user_message: "Implementation and review completed successfully.", + }), + }); + + assert.equal(run.status, 0, run.stderr || run.stdout); + assert.match(run.ghLog, /api --method PATCH repos\/self-evolving\/repo\/issues\/comments\/777/); + assert.match(run.stderr, /Failed to collapse previous orchestration artifacts/); +}); + test("terminal child result reports to parent and preserves terminal reruns", () => { const childBody = ""; const run = runOrchestrateHandoff({ diff --git a/.agent/src/cli/orchestrate-handoff.ts b/.agent/src/cli/orchestrate-handoff.ts index c9cd0d79..e7854731 100644 --- a/.agent/src/cli/orchestrate-handoff.ts +++ b/.agent/src/cli/orchestrate-handoff.ts @@ -4,12 +4,19 @@ // GITHUB_REPOSITORY, DEFAULT_BRANCH, REQUESTED_BY, REQUEST_TEXT, // SESSION_BUNDLE_MODE, SOURCE_RUN_ID, PLANNER_RESPONSE_FILE, TARGET_KIND, // BASE_BRANCH, BASE_PR, AGENT_COLLAPSE_OLD_REVIEWS, AGENT_ALLOW_SELF_APPROVE, -// AGENT_ALLOW_SELF_MERGE +// AGENT_ALLOW_SELF_MERGE, AGENT_HANDLE, AGENT_PROGRESS_COMMENT_ID, +// AGENT_PROGRESS_FINAL_COMMENT_MODE, MODEL_DISPLAY import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { createIssueComment, dispatchWorkflow, gh, updateIssueComment } from "../github.js"; +import { + createIssueComment, + dispatchWorkflow, + gh, + updateIssueComment, + upsertPrCommentByMarker, +} from "../github.js"; import { setOutput } from "../output.js"; import { type HandoffDecision, @@ -25,7 +32,12 @@ import { parseHandoffMarker, } from "../handoff.js"; import { initialOrchestrateCapabilityStopReason } from "../orchestrator-capabilities.js"; -import { collapsePreviousHandoffComments } from "../review-summary-minimize.js"; +import { tryMergeProgressFinalComment } from "../progress-final-comment.js"; +import { + collapsePreviousHandoffComments, + collapsePreviousPrConversationArtifacts, +} from "../review-summary-minimize.js"; +import { appendRunDisplayFooter } from "../response.js"; import { extractClosingIssueNumber, formatSubOrchestrationIssueBody, @@ -841,6 +853,10 @@ const isPublicRepo = String(process.env.REPOSITORY_PRIVATE || "").trim().toLower const targetNumber = process.env.TARGET_NUMBER || ""; const requestedBy = process.env.REQUESTED_BY || ""; const requestText = process.env.REQUEST_TEXT || ""; +const agentHandle = process.env.AGENT_HANDLE || "@sepo-agent"; +const modelDisplay = process.env.MODEL_DISPLAY || process.env.AGENT_RUN_DISPLAY || ""; +const progressFinalCommentMode = process.env.AGENT_PROGRESS_FINAL_COMMENT_MODE || ""; +const progressCommentId = process.env.AGENT_PROGRESS_COMMENT_ID || process.env.PROGRESS_COMMENT_ID || ""; const sessionBundleMode = process.env.SESSION_BUNDLE_MODE || ""; const baseBranch = process.env.BASE_BRANCH || ""; const basePr = process.env.BASE_PR || ""; @@ -968,16 +984,16 @@ function commentOnTerminalSubOrchestrationRejection(rejection: TerminalSubOrches })); } -function reportTerminalToParent(decision: HandoffDecision): void { +function reportTerminalToParent(decision: HandoffDecision): boolean { const childResolution = resolveChildIssueForTerminal(); - if (childResolution.kind === "none") return; + if (childResolution.kind === "none") return false; if (childResolution.kind === "rejected") { commentOnTerminalSubOrchestrationRejection(childResolution.rejection); - return; + return true; } const childIssue = childResolution.issue; const marker = childIssue.subOrchestrator.marker; - if (!["running", "done", "blocked", "failed"].includes(marker.state)) return; + if (!["running", "done", "blocked", "failed"].includes(marker.state)) return true; const resultState = marker.state === "running" ? resultStateFromTerminal({ sourceAction, @@ -996,7 +1012,7 @@ function reportTerminalToParent(decision: HandoffDecision): void { const existingProgress = progressComments[progressComments.length - 1]; const progressWasDispatched = String(existingProgress?.body || "").includes(dispatchedProgressMarker); if (marker.state !== "running" && progressWasDispatched) { - return; + return true; } let progressCommentId = existingProgress?.id ? String(existingProgress.id) : ""; const writeProgress = (progressMarker: string): void => { @@ -1046,6 +1062,7 @@ function reportTerminalToParent(decision: HandoffDecision): void { if (updatedChildMarkerBody !== childIssue.subOrchestrator.body) { updateTrustedSubOrchestratorMarker(repo, childIssue, updatedChildMarkerBody); } + return true; } function pushUniqueMarkdownBlock(lines: string[], value: string | undefined): void { @@ -1054,6 +1071,64 @@ function pushUniqueMarkdownBlock(lines: string[], value: string | undefined): vo lines.push(text); } +function requestedByHumanLine(): string { + if (normalizeToken(sourceTargetKind) !== "pull_request") return ""; + const raw = requestedBy.trim(); + if (!raw || /^app\//i.test(raw) || /\[bot\]$/i.test(raw)) return ""; + + const login = raw.replace(/^@+/, ""); + if (!/^[a-z\d](?:[a-z\d-]{0,38})$/i.test(login)) return ""; + + const normalizedLogin = normalizeActorLogin(login); + const normalizedHandle = normalizeActorLogin(agentHandle.replace(/^@+/, "")); + if ( + normalizedLogin === normalizedHandle || + normalizedLogin === "sepo-agent" || + normalizedLogin === "sepo-agent-app" || + normalizedLogin === "github-actions" + ) { + return ""; + } + return `> Requested by @${login}.`; +} + +function startOrchestrationComment(heading: string): string[] { + const lines = [heading]; + const requester = requestedByHumanLine(); + if (requester) lines.push("", requester); + return lines; +} + +function isSuccessfulPrSourceOutcome(): boolean { + const action = normalizeToken(sourceAction); + const conclusion = normalizeToken(sourceConclusion); + const recommendedNextStep = normalizeToken(sourceRecommendedNextStep); + if (action === "review") { + return ( + conclusion === "ship" && + !allowSelfApprove && + (!recommendedNextStep || recommendedNextStep === "no_automated_action") + ); + } + if (action === "agent_self_approve") { + return conclusion === "approved" && !allowSelfMerge; + } + return action === "agent_self_merge" && ["merged", "auto_merge_enabled"].includes(conclusion); +} + +function isValidatedSuccessfulPrStop(decision: HandoffDecision): boolean { + if ( + decision.decision !== "stop" || + normalizeToken(sourceTargetKind) !== "pull_request" || + !isSuccessfulPrSourceOutcome() || + Boolean(String(decision.clarificationRequest || "").trim()) + ) { + return false; + } + if (automationMode !== "agent") return true; + return decision.plannerDecisionKind === "stop" && Boolean(String(decision.userMessage || "").trim()); +} + function formatPlannerClarificationComment(decision: HandoffDecision): string | null { if (decision.plannerDecisionKind !== "blocked") { return null; @@ -1068,8 +1143,10 @@ function formatPlannerClarificationComment(decision: HandoffDecision): string | return null; } - const lines = [ + const lines = startOrchestrationComment( "Sepo orchestration needs clarification before it can continue.", + ); + lines.push( "", ...messageLines.flatMap((message, index) => index === 0 ? [message] : ["", message]), "", @@ -1078,7 +1155,7 @@ function formatPlannerClarificationComment(decision: HandoffDecision): string | `- Target: \`${sourceTargetKind || "unknown"} #${targetNumber || "unknown"}\``, `- Round: \`${currentRound}/${maxRounds}\``, `- Reason: ${decision.reason}`, - ]; + ); if (sourceRunId) { lines.push(`- Source run ID: \`${sourceRunId}\``); @@ -1101,8 +1178,8 @@ function formatPlannerAnswerComment(decision: HandoffDecision): string | null { const message = String(decision.userMessage || "").trim(); if (!message) return null; - const lines = [ - "Sepo answered this orchestration request.", + const lines = startOrchestrationComment("Sepo answered this orchestration request."); + lines.push( "", message, "", @@ -1111,7 +1188,7 @@ function formatPlannerAnswerComment(decision: HandoffDecision): string | null { `- Target: \`${sourceTargetKind || "unknown"} #${targetNumber || "unknown"}\``, `- Round: \`${currentRound}/${maxRounds}\``, `- Reason: ${decision.reason}`, - ]; + ); if (sourceRunId) { lines.push(`- Source run ID: \`${sourceRunId}\``); @@ -1131,15 +1208,24 @@ function formatOrchestrateStopComment(decision: HandoffDecision): string { return answerComment; } - const lines = [ - `Sepo orchestration stopped after \`${sourceAction || "unknown"}\` concluded \`${sourceConclusion || "unknown"}\`.`, + const successful = isValidatedSuccessfulPrStop(decision); + const lines = startOrchestrationComment( + successful + ? `Sepo orchestration finished successfully after \`${sourceAction || "unknown"}\` concluded \`${sourceConclusion || "unknown"}\`.` + : `Sepo orchestration stopped after \`${sourceAction || "unknown"}\` concluded \`${sourceConclusion || "unknown"}\`.`, + ); + const userMessage = normalizeToken(sourceTargetKind) === "pull_request" + ? String(decision.userMessage || "").trim() + : ""; + if (userMessage) lines.push("", userMessage); + lines.push( "", `- Source action: \`${sourceAction || "unknown"}\``, `- Source conclusion: \`${sourceConclusion || "unknown"}\``, `- Target: \`${sourceTargetKind || "unknown"} #${targetNumber || "unknown"}\``, `- Round: \`${currentRound}/${maxRounds}\``, `- Reason: ${decision.reason}`, - ]; + ); if (sourceRunId) { lines.push(`- Source run ID: \`${sourceRunId}\``); @@ -1147,13 +1233,50 @@ function formatOrchestrateStopComment(decision: HandoffDecision): string { lines.push( "", - "No follow-up workflow was dispatched. Inspect the source action status comment and workflow logs before retrying or continuing manually.", + successful + ? "No further workflow was needed." + : "No follow-up workflow was dispatched. Inspect the source action status comment and workflow logs before retrying or continuing manually.", "", ORCHESTRATE_STOP_MARKER, ); return lines.join("\n"); } +function currentFinalCommentId(prNumber: number, knownCommentId: string): string { + if (/^\d+$/.test(knownCommentId.trim())) return knownCommentId.trim(); + const comments = fetchIssueComments(repo, prNumber).filter((comment) => ( + comment.id && + String(comment.body || "").includes(ORCHESTRATE_STOP_MARKER) && + isTrustedActorLogin(comment.authorLogin || "") + )); + return String(comments[comments.length - 1]?.id || ""); +} + +function collapseSuccessfulPrArtifacts( + prNumber: number, + decision: HandoffDecision, + knownCommentId: string, +): void { + if (!collapseOldReviews || !isValidatedSuccessfulPrStop(decision)) return; + try { + const finalCommentId = currentFinalCommentId(prNumber, knownCommentId); + if (!finalCommentId) return; + const collapsed = collapsePreviousPrConversationArtifacts({ + repo, + prNumber, + finalBodyMarker: ORCHESTRATE_STOP_MARKER, + currentCommentDatabaseId: finalCommentId, + }); + if (collapsed > 0) { + console.log(`Collapsed ${collapsed} previous orchestration artifact comment(s).`); + } + } catch (err: unknown) { + console.warn(`Failed to collapse previous orchestration artifacts for ${repo}#${prNumber}: ${errorText(err)}`); + } +} + +let orchestrateStopCommentHandled = false; + function hasMatchingOrchestrateStopComment(repoSlug: string, issueNumber: number, body: string): boolean { try { const expectedBody = body.trim(); @@ -1172,15 +1295,40 @@ function hasMatchingOrchestrateStopComment(repoSlug: string, issueNumber: number } function createOrchestrateStopComment(decision: HandoffDecision): void { + if (orchestrateStopCommentHandled) return; const target = parsePositiveTargetNumber(targetNumber); - if (!repo || !target || !["issue", "pull_request"].includes(normalizeToken(sourceTargetKind))) { + const targetKind = normalizeToken(sourceTargetKind); + if (!repo || !target || !["issue", "pull_request"].includes(targetKind)) { return; } const body = formatOrchestrateStopComment(decision); + if (targetKind === "pull_request") { + const merged = tryMergeProgressFinalComment({ + repo, + commentId: progressCommentId, + mode: progressFinalCommentMode, + finalBody: body, + footer: modelDisplay, + }); + if (!merged) { + const action = upsertPrCommentByMarker( + target, + repo, + ORCHESTRATE_STOP_MARKER, + appendRunDisplayFooter(body, modelDisplay), + ); + console.log(`${action === "updated" ? "Updated" : "Created"} orchestrator final comment.`); + } + orchestrateStopCommentHandled = true; + collapseSuccessfulPrArtifacts(target, decision, merged ? progressCommentId : ""); + return; + } if (hasMatchingOrchestrateStopComment(repo, target, body)) { + orchestrateStopCommentHandled = true; return; } createIssueComment(repo, target, body); + orchestrateStopCommentHandled = true; } function commentOnInitialOrchestrateStop(decision: HandoffDecision): void { @@ -1245,6 +1393,17 @@ function commentOnTerminalMetaOrchestratorStop(decision: HandoffDecision): void createOrchestrateStopComment(decision); } +function commentOnTerminalPullRequestStop(decision: HandoffDecision, reportedToParent: boolean): void { + if ( + decision.decision !== "stop" || + reportedToParent || + normalizeToken(sourceTargetKind) !== "pull_request" + ) { + return; + } + createOrchestrateStopComment(decision); +} + function decideManualOrchestration(): HandoffDecision { const nextRound = currentRound + 1; if (currentRound >= maxRounds) { @@ -1373,15 +1532,22 @@ setOutput("marker_comment_id", ""); if (decision.decision !== "dispatch" && decision.decision !== "delegate_issue") { console.log(`Handoff ${decision.decision}: ${decision.reason}`); + let reportedToParent = false; try { commentOnPlannerClarificationStop(decision); commentOnInitialOrchestrateStop(decision); commentOnUnsatisfactoryActionStop(decision); - reportTerminalToParent(decision); + reportedToParent = reportTerminalToParent(decision); commentOnTerminalMetaOrchestratorStop(decision); } catch (err: unknown) { console.warn(`Failed to report terminal sub-orchestration state: ${errorText(err)}`); } + try { + commentOnTerminalPullRequestStop(decision, reportedToParent); + } catch (err: unknown) { + console.error(`Failed to publish terminal pull request note: ${errorText(err)}`); + process.exit(1); + } process.exit(0); } diff --git a/.agent/src/review-summary-minimize.ts b/.agent/src/review-summary-minimize.ts index f974614b..06b101a1 100644 --- a/.agent/src/review-summary-minimize.ts +++ b/.agent/src/review-summary-minimize.ts @@ -13,6 +13,7 @@ type PageInfo = { type ReviewSummaryNode = { id?: string | null; + databaseId?: number | null; body?: string | null; isMinimized?: boolean | null; author?: { @@ -61,6 +62,11 @@ type CollapsePreviousReviewSummariesOptions = { client?: GraphQLClient; }; +type CollapsePreviousPrConversationArtifactsOptions = CollapsePreviousReviewSummariesOptions & { + finalBodyMarker?: string; + currentCommentDatabaseId?: string | number; +}; + type CollapsePreviousHandoffCommentsOptions = { repo: string; targetNumber: number; @@ -92,6 +98,7 @@ const COMMENTS_QUERY = ` comments(first: 100, after: $after) { nodes { id + databaseId body isMinimized author { @@ -300,6 +307,7 @@ function collapsePreviousMatchingReviewComments( function collapsePreviousMatchingPrComments( options: CollapsePreviousReviewSummariesOptions, bodyMatcher: ReviewBodyMatcher, + beforeDatabaseId?: number, ): number { const client = options.client || createGhGraphqlClient(); const repo = parseRepo(options.repo); @@ -313,7 +321,14 @@ function collapsePreviousMatchingPrComments( viewerLogin, bodyMatcher, ); - const uniqueNodeIds = Array.from(new Set(nodes.map((node) => node.id).filter(Boolean))) as string[]; + const uniqueNodeIds = Array.from(new Set( + nodes + .filter((node) => !beforeDatabaseId || ( + Number.isFinite(Number(node.databaseId)) && Number(node.databaseId) < beforeDatabaseId + )) + .map((node) => node.id) + .filter(Boolean), + )) as string[]; for (const id of uniqueNodeIds) { client.graphql(MINIMIZE_COMMENT_MUTATION, { @@ -325,6 +340,16 @@ function collapsePreviousMatchingPrComments( return uniqueNodeIds.length; } +function isTerminalPrConversationArtifact(body: string): boolean { + const handoffMarker = parseAnyHandoffMarker(body); + return ( + isReviewSynthesisBody(body) || + isRubricsReviewBody(body) || + isFixPrStatusBody(body) || + Boolean(handoffMarker && handoffMarker.state !== "pending") + ); +} + function collapsePreviousMatchingHandoffComments( options: CollapsePreviousHandoffCommentsOptions, ): number { @@ -445,6 +470,24 @@ export function collapsePreviousFixPrComments( return collapsePreviousMatchingPrComments(options, isFixPrStatusBody); } +/** + * Collapses trusted generated PR conversation comments after terminal success. + * Formal review objects are intentionally left to the review workflow. + */ +export function collapsePreviousPrConversationArtifacts( + options: CollapsePreviousPrConversationArtifactsOptions, +): number { + const finalBodyMarker = String(options.finalBodyMarker || ""); + const currentCommentDatabaseId = Number(options.currentCommentDatabaseId); + if (!Number.isSafeInteger(currentCommentDatabaseId) || currentCommentDatabaseId <= 0) { + return 0; + } + return collapsePreviousMatchingPrComments(options, (body) => ( + isTerminalPrConversationArtifact(body) || + Boolean(finalBodyMarker && body.includes(finalBodyMarker)) + ), currentCommentDatabaseId); +} + /** * Collapses older orchestrator handoff marker comments after a fresh dispatch. */ diff --git a/.github/prompts/agent-orchestrator.md b/.github/prompts/agent-orchestrator.md index 86c1ed8f..0b8129a0 100644 --- a/.github/prompts/agent-orchestrator.md +++ b/.github/prompts/agent-orchestrator.md @@ -74,7 +74,7 @@ rubrics. Then return exactly one JSON object and nothing else: "next_action": "implement | review | fix-pr | agent-self-approve | agent-self-merge", "reason": "Short explanation for logs and the handoff marker.", "handoff_context": "Actionable instructions for the next action, especially fix-pr.", - "user_message": "Optional user-facing message to post when decision is answer or blocked.", + "user_message": "Optional user-facing summary or message to post when decision is stop, answer, or blocked.", "clarification_request": "Optional focused question to post when decision is blocked.", "child_stage": "Short child issue stage name when decision is delegate_issue.", "child_instructions": "Concrete child issue task instructions when decision is delegate_issue.", @@ -114,6 +114,9 @@ Rules: and are safe for an automated agent to apply. - Use `stop` when the task appears complete, the result is unsupported, or the next step should be left to a human. +- For `stop`, include a concise `user_message` summarizing the cumulative work + and terminal outcome from the current target's orchestration history. If no + substantive work completed, explain the remaining human action or blocker. - Stop instead of handing off when the remaining items are metadata-only (for example PR title/body/labels/comments), optional suggestions, `INFO` / `FOLLOW_UP` findings, style or naming preferences, already-fixed findings, or diff --git a/.github/workflows/agent-orchestrator.yml b/.github/workflows/agent-orchestrator.yml index 9c88521f..85e98468 100644 --- a/.github/workflows/agent-orchestrator.yml +++ b/.github/workflows/agent-orchestrator.yml @@ -207,6 +207,9 @@ jobs: AUTOMATION_MAX_ROUNDS: ${{ inputs.automation_max_rounds }} AUTOMATION_MODE: ${{ inputs.automation_mode }} AGENT_COLLAPSE_OLD_REVIEWS: ${{ vars.AGENT_COLLAPSE_OLD_REVIEWS }} + AGENT_HANDLE: ${{ vars.AGENT_HANDLE || '@sepo-agent' }} + AGENT_PROGRESS_COMMENT_ID: ${{ steps.planner.outputs.progress_comment_id }} + AGENT_PROGRESS_FINAL_COMMENT_MODE: merge BASE_BRANCH: ${{ inputs.base_branch }} BASE_PR: ${{ inputs.base_pr }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} @@ -229,4 +232,5 @@ jobs: TARGET_KIND: ${{ inputs.target_kind || (inputs.source_action == 'implement' && 'issue' || 'pull_request') }} TARGET_NUMBER: ${{ inputs.target_number }} PLANNER_RESPONSE_FILE: ${{ steps.planner.outputs.response_file }} + MODEL_DISPLAY: ${{ steps.planner.outputs.model_display }} run: node .agent/dist/cli/orchestrate-handoff.js