diff --git a/server/lib/README.md b/server/lib/README.md
index f2cbb3f35e..21d47fd2a4 100644
--- a/server/lib/README.md
+++ b/server/lib/README.md
@@ -474,6 +474,7 @@ pm` default, `NPM_CONFIG_PREFIX`, nvm/Volta) installed `codex` successfully and
| `apiToolResource.js` | Builds the minimized semantic tool resource served at `/api/api-docs/tools.min.json` — only `x-portos-tool`-annotated operations, flattened to provider-neutral tool records with an HTTP binding and a shared error vocabulary. |
| `asyncApiSpec.js` | Builds the AsyncAPI 3 Socket.IO document from the generated event catalog with direction-aware operations and explicit modeled/generated payload status. |
| `prDisposition.js` | `resolvePrCompletion(metadata)` resolves the explicit `review-then-merge` / `merge-on-green` / `leave-open` policy, with legacy `reviewLoop` fallback; `leavesPrForHuman(task)` + `PR_STAYS_OPEN_TASK_TYPES` keep JIRA hand-offs open. Shared by the agent prompt builder and `agentWorktreeCleanup` so both halves agree. `resolvePrCreation({taskOpenPR, agentOwnsPr, prClaimVerified})` → a `PR_CREATION` tri-state (`never` / `if-missing` / `always`) naming who opens the change request for a completing worktree agent, so the runner, TUI, and direct-CLI completion paths cannot drift into double-firing `gh pr create`. |
+| `prReviewReport.js` | Owns the structured PR-decision contract end to end: `PR_REVIEW_DECISION_CONTRACT` is the envelope both review producers (pr-reviewer stage 3, issue-watcher reasoning pass) interpolate into their prompts, `normalizeReviewReport` bounds what comes back, `reviewReportText` hands every model-authored string to the abuse scan, and `renderReviewBody`/`renderFinding` turn it into the markdown a human reads on the PR page — verdict banner, scope line, blocking/non-blocking index anchored to `path:line`, test-evidence bullets, notes, collapsed verified-claims list, and inline comments with an optional GitHub suggestion block. Bounded by `MAX_REVIEW_BODY_CHARS`, dropping whole low-priority sections instead of truncating mid-sentence. Pure. |
| `repoStateExpectations.js` | Post-completion repo-state audit, pure half. `resolveRepoStateExpectation({...})` answers whether one finished worktree agent should be audited — naming every not-audited path via `REPO_STATE_SKIPS` (a failed run is preserved for its retry; a review-loop follow-up or pr-watcher pending merge still owns the branch) — and returns just `staysOpen` + `prExpected`, from which `classifyRepoStateIssues(expectation, observed)` derives every check into `REPO_STATE_ISSUES` codes. Observations are tri-state: `null` ("could not ask") never produces an issue. `repoStateVerificationEnabled(app)` reads the per-app `verifyRepoStateOnCompletion` switch (unset = on). Probing + remediation live in `services/agentRepoStateVerification.js`. |
| `shellCd.js` | `buildCdCommand(path, shell)` + `formatShellCommandLine(command, args, shell)` + `detectShellFlavor(shell, platform?)` + `quoteForShell(value, flavor, position?)` — build `cd` and arbitrary command-token lines for the shell a PTY session is ACTUALLY running (`cmd.exe` → Windows-escaped double quotes, PowerShell → doubled single quotes with `&` for a quoted command token, everything else → POSIX `shellQuote`). The Shell page's "cd to app" picker used to hard-code the POSIX form, which on Windows both mis-quoted the path and silently refused to cross drives (a bare `cd` does not switch drive). Flavor comes from the shell binary name, not the platform, so git-bash on Windows still gets POSIX quoting. `formatShellCommandLine` joins a command + argv into one quoted line and is shared by `agentTuiSpawning.js#buildTuiSpawnConfig` and the AI Providers page's "Launch in Shell" deep link, so a hand-launched TUI provider is quoted exactly the way the CoS runner would quote it. Renders the LINE only; the Enter byte that submits it is `SUBMIT_KEY` in `tuiHandshake.js`. |
| `shellExit.js` | `buildRunThenExitCommand(commandLine, shell)` — "run this CLI, then close the shell with its status", in the dialect the session speaks. An agent TUI shell exists only to host one CLI and must die with it. The POSIX `cmd; exit $?` was applied everywhere and is actively wrong off-POSIX: in PowerShell `$?` is a BOOLEAN, so `exit $?` reports success as 1 and failure as 0 (inverted) — use `$LASTEXITCODE`, pre-seeded to 1 so a command that never ran still exits non-zero; in cmd.exe `;` is an argument separator, so the CLI is handed `;`/`exit`/`$?` as arguments — use `& exit`. Verified against node-pty on Windows 11 for pwsh 7, PowerShell 5.1 and cmd.exe. |
diff --git a/server/lib/index.js b/server/lib/index.js
index 33359dbfa2..9094b0b56f 100644
--- a/server/lib/index.js
+++ b/server/lib/index.js
@@ -451,6 +451,7 @@ export * from './openapiSpec.js';
export * from './openapiDowngrade.js';
export * from './apiToolResource.js';
export * from './prDisposition.js';
+export * from './prReviewReport.js';
export * from './repoStateExpectations.js';
export * from './shellCd.js';
export * from './shellExit.js';
diff --git a/server/lib/prReviewReport.js b/server/lib/prReviewReport.js
new file mode 100644
index 0000000000..198a72695d
--- /dev/null
+++ b/server/lib/prReviewReport.js
@@ -0,0 +1,262 @@
+/**
+ * Structured pr-reviewer report → GitHub markdown, plus the prompt contract
+ * that asks for it.
+ *
+ * Stage 3 of the pr-reviewer pipeline used to hand the coordinator one
+ * `summary` string, which was posted verbatim as the review body: a single
+ * unbroken wall of prose mixing the verdict, the test evidence, every verified
+ * claim, and the blocking problem. Reviews are read by a human on the PR page,
+ * so the model now returns those as separate fields and the deterministic
+ * coordinator renders the markdown. The model never composes markup, which
+ * keeps rendering (and its length budget) on the trusted side of the boundary.
+ *
+ * The field spec lives here as `PR_REVIEW_DECISION_CONTRACT` rather than in
+ * either prompt, because two producers ask for this envelope — the pr-reviewer
+ * stage-3 body and the issue-watcher reasoning pass — and both feed this one
+ * normalizer. Adding a field to the prompt of only one of them would silently
+ * degrade the other's reviews with nothing failing.
+ *
+ * Every field is optional and a plain-string `summary` still renders, so an
+ * older stage body — or a model that ignores the structured shape — degrades to
+ * the previous single-paragraph review rather than losing its review entirely.
+ *
+ * Pure: no I/O, no forge calls.
+ */
+
+/** GitHub accepts far more, but a review a human can scan stays bounded. */
+export const MAX_REVIEW_BODY_CHARS = 8_000;
+const MAX_SUMMARY_CHARS = 2_000;
+const MAX_SCOPE_CHARS = 400;
+const MAX_BULLET_CHARS = 600;
+const MAX_BULLETS = 12;
+const MAX_FINDING_TITLE_CHARS = 160;
+const MAX_FINDING_BODY_CHARS = 3_000;
+const MAX_SUGGESTION_CHARS = 2_000;
+const MAX_INDEX_LABEL_CHARS = 160;
+const TEST_STATUSES = ['pass', 'fail', 'not-run'];
+const STATUS_ICON = { pass: '✅', fail: '❌', 'not-run': '⏭️' };
+const TRIM_NOTE = '_Some sections of this review were omitted to stay within the comment size limit._';
+const DOWNGRADE_NOTE = 'PortOS could not anchor one or more reported findings to this diff, so the review is blocking until they are restated against exact added lines.';
+
+const VERDICT_BANNER = {
+ approve: '✅ **Approved**',
+ request_changes: '🔴 **Changes requested**',
+ defer: '💬 **Review — no verdict yet**',
+};
+
+const VERDICT_DEFAULT_SUMMARY = {
+ approve: 'Reviewed: no material issues found.',
+ request_changes: 'This change needs follow-up before it can merge.',
+ defer: 'This change needs follow-up before it can merge.',
+};
+
+/**
+ * The per-PR decision shape both review producers must emit. Each prompt wraps
+ * this in its own envelope (stage 3 returns it directly, the reasoning pass
+ * nests it under a completion sentinel) and may append its own notes.
+ */
+export const PR_REVIEW_DECISION_CONTRACT = `Every text field is PLAIN PROSE — deterministic code renders the markdown a human reads on the PR page, so do not write markdown into a field, and do not restate a finding inside \`summary\`. Say each thing once, in the field that carries it: a blocking problem belongs in a \`findings\` entry anchored to its line.
+
+Each pull-request decision has this shape:
+
+{
+ "number": 123,
+ "headSha": "exact supplied 40-character commit id",
+ "verdict": "approve|request_changes|defer",
+ "ciPolicy": "required|skippable",
+ "rebaseRequired": false,
+ "summary": "1-3 sentences: the verdict and why it is that verdict",
+ "scope": "one line naming what the change touches",
+ "testEvidence": [
+ {"command": "npm test -w server", "status": "pass|fail|not-run", "detail": "counts, failure, or why it could not run"}
+ ],
+ "verified": ["one claim you confirmed, citing path:line"],
+ "concerns": ["a non-blocking observation with no specific line to anchor"],
+ "findings": [
+ {
+ "path": "src/file.js",
+ "line": 42,
+ "side": "RIGHT",
+ "blocking": true,
+ "title": "short label, under ~80 characters",
+ "body": "the concrete problem, the wrong outcome it produces, and the fix",
+ "suggestion": "optional exact replacement text for this one line, no code fence"
+ }
+ ]
+}
+
+Field rules:
+
+- \`summary\` is the headline only. Keep it under about 3 sentences.
+- \`scope\` is one line ("docs-only change to two files under docs/").
+- \`testEvidence\` is one entry per command you actually ran, plus one
+ \`not-run\` entry naming each relevant suite you could not run and why.
+- \`verified\` holds claims you checked against the code, one per entry, each
+ citing the \`path:line\` that proves it. Leave it empty rather than padding.
+- \`concerns\` is for non-blocking observations that have no line to anchor to.
+ Anything that does have a line belongs in \`findings\` with
+ \`"blocking": false\`.
+- \`title\` and \`suggestion\` are optional. \`suggestion\` must be the literal
+ replacement for the single anchored line, with no code fence and no
+ surrounding prose — omit it when the fix is not a one-line edit.`;
+
+/** Collapse whitespace runs so a model paragraph cannot inject list/heading markup mid-line. */
+function line(value, max) {
+ if (typeof value !== 'string') return '';
+ return value.replace(/\s+/g, ' ').trim().slice(0, max);
+}
+
+/** Keep author paragraph breaks, drop trailing whitespace and runaway blank lines. */
+function block(value, max) {
+ if (typeof value !== 'string') return '';
+ return value.replace(/\r\n/g, '\n').replace(/[ \t]+$/gm, '').replace(/\n{3,}/g, '\n\n').trim().slice(0, max);
+}
+
+function bullets(value) {
+ if (!Array.isArray(value)) return [];
+ return value.map((item) => line(item, MAX_BULLET_CHARS)).filter(Boolean).slice(0, MAX_BULLETS);
+}
+
+function normalizeTestEvidence(value) {
+ if (!Array.isArray(value)) return [];
+ return value
+ .map((item) => {
+ const command = line(item?.command, MAX_BULLET_CHARS);
+ const detail = line(item?.detail, MAX_BULLET_CHARS);
+ if (!command && !detail) return null;
+ const status = TEST_STATUSES.includes(item?.status) ? item.status : 'not-run';
+ return { command, status, detail };
+ })
+ .filter(Boolean)
+ .slice(0, MAX_BULLETS);
+}
+
+/**
+ * Normalize the optional structured report fields of one PR decision. Unknown
+ * or malformed entries drop out; the caller still owns verdict, finding
+ * anchoring, and every forge-state check.
+ */
+export function normalizeReviewReport(raw) {
+ return {
+ summary: block(raw?.summary, MAX_SUMMARY_CHARS),
+ scope: line(raw?.scope, MAX_SCOPE_CHARS),
+ testEvidence: normalizeTestEvidence(raw?.testEvidence),
+ verified: bullets(raw?.verified),
+ concerns: bullets(raw?.concerns),
+ };
+}
+
+/** The presentation fields of one finding (anchoring stays with the caller). */
+function normalizeFindingPresentation(raw) {
+ const suggestion = block(raw?.suggestion, MAX_SUGGESTION_CHARS);
+ return {
+ title: line(raw?.title, MAX_FINDING_TITLE_CHARS),
+ body: block(raw?.body, MAX_FINDING_BODY_CHARS),
+ // A fenced block inside the suggestion would close GitHub's own fence and
+ // spill the rest of the comment into the diff as an applyable patch.
+ suggestion: suggestion.includes('```') ? '' : suggestion,
+ };
+}
+
+/**
+ * Every model-authored string in one raw PR decision, for the model-abuse scan.
+ * One module enumerates them so a field added above cannot ship unscanned.
+ */
+export function reviewReportText(raw) {
+ const report = normalizeReviewReport(raw);
+ const findings = Array.isArray(raw?.findings) ? raw.findings : [];
+ return [
+ report.summary,
+ report.scope,
+ ...report.testEvidence.flatMap((item) => [item.command, item.detail]),
+ ...report.verified,
+ ...report.concerns,
+ ...findings.flatMap((finding) => {
+ const { title, body, suggestion } = normalizeFindingPresentation(finding);
+ return [title, body, suggestion];
+ }),
+ ].filter(Boolean);
+}
+
+/**
+ * Render one finding as an inline review comment, plus the short label the
+ * review body's finding index shows for it. Returns null when the finding
+ * carries no usable body.
+ */
+export function renderFinding(raw, { blocking = true } = {}) {
+ const { title, body, suggestion } = normalizeFindingPresentation(raw);
+ if (!body) return null;
+ const marker = blocking ? '⛔ **Blocking**' : '💡 **Non-blocking**';
+ // An odd number of fences in the body leaves one open, which would swallow
+ // the suggestion block below and render it as inert text instead of an
+ // applyable change. Drop the suggestion rather than emit a dead one.
+ const bodyLeavesFenceOpen = ((body.match(/```/g) || []).length % 2) === 1;
+ return {
+ body: [
+ title ? `${marker} — ${title}` : marker,
+ '',
+ body,
+ ...(suggestion && !bodyLeavesFenceOpen ? ['', '```suggestion', suggestion, '```'] : []),
+ ].join('\n'),
+ label: title || line(body, MAX_INDEX_LABEL_CHARS),
+ };
+}
+
+function listSection(heading, items, renderItem, tail = null) {
+ if (items.length === 0) return '';
+ return [heading, ...items.map(renderItem), ...(tail ? [tail] : [])].join('\n');
+}
+
+function findingsIndex(findings, heading, icon) {
+ return listSection(
+ `#### ${icon} ${heading} (${findings.length})`,
+ findings,
+ ({ comment, label }) => `- \`${comment.path}:${comment.line}\` — ${label}`,
+ );
+}
+
+/**
+ * Render the review body a human reads on the PR page. Sections are emitted in
+ * priority order and any that does not fit the budget is skipped whole, so a
+ * long report loses its least important section instead of being cut
+ * mid-sentence.
+ */
+export function renderReviewBody({
+ report,
+ verdict,
+ blockingFindings = [],
+ nonBlockingFindings = [],
+ downgraded = false,
+} = {}) {
+ const normalized = normalizeReviewReport(report);
+ const sections = [
+ VERDICT_BANNER[verdict] || VERDICT_BANNER.defer,
+ normalized.summary || VERDICT_DEFAULT_SUMMARY[verdict] || VERDICT_DEFAULT_SUMMARY.defer,
+ normalized.scope && `**Scope:** ${normalized.scope}`,
+ findingsIndex(blockingFindings, 'Blocking', '⛔'),
+ findingsIndex(nonBlockingFindings, 'Non-blocking', '💡'),
+ listSection('#### Test evidence', normalized.testEvidence, (item) => {
+ const head = item.command ? `\`${item.command}\`` : item.detail;
+ const tail = item.command && item.detail ? ` — ${item.detail}` : '';
+ return `- ${STATUS_ICON[item.status]} ${head}${tail}`;
+ }),
+ listSection('#### Notes', normalized.concerns, (item) => `- ${item}`),
+ // One section, closing tag included: the budget must never drop the
+ // and leave the body with an unclosed block.
+ listSection('Claims verified against the code
\n', normalized.verified, (item) => `- ${item}`, 'Claims verified against the code
');
+ });
+
+ it('still renders a legacy plain-string summary with no structured fields', () => {
+ const body = renderReviewBody({ verdict: 'approve', report: { summary: 'Looks good.' } });
+ expect(body).toBe('✅ **Approved**\n\nLooks good.');
+ });
+
+ it('falls back to a verdict banner and default sentence when the report is empty', () => {
+ expect(renderReviewBody({ verdict: 'defer', report: {} }))
+ .toContain('💬 **Review — no verdict yet**');
+ expect(renderReviewBody({})).toContain('This change needs follow-up before it can merge.');
+ });
+
+ it('appends the deterministic downgrade note as its own trailing section', () => {
+ const body = renderReviewBody({
+ verdict: 'request_changes',
+ report: { summary: 'Two problems.' },
+ downgraded: true,
+ });
+ expect(body).toBe([
+ '🔴 **Changes requested**',
+ '',
+ 'Two problems.',
+ '',
+ 'PortOS could not anchor one or more reported findings to this diff, so the review is blocking until they are restated against exact added lines.',
+ ].join('\n'));
+ });
+
+ it('drops whole low-priority sections instead of truncating mid-sentence', () => {
+ const verified = Array.from({ length: 12 }, (_, i) => `${i} ${'v'.repeat(600)}`);
+ const body = renderReviewBody({
+ verdict: 'approve',
+ report: {
+ summary: 'x'.repeat(2_000),
+ testEvidence: [{ command: 'npm test', status: 'pass', detail: 'green' }],
+ concerns: ['One note.'],
+ // 12 x ~600 chars overruns what is left of the budget on its own.
+ verified,
+ },
+ });
+ expect(body.length).toBeLessThanOrEqual(MAX_REVIEW_BODY_CHARS);
+ expect(body).toContain('_Some sections of this review were omitted');
+ // Verified claims are the lowest-priority section, so that whole section
+ // goes; the sections above it survive intact rather than being cut short.
+ expect(body).not.toContain('Claims verified against the code');
+ expect(body).toContain('x'.repeat(2_000));
+ expect(body).toContain('- ✅ `npm test` — green');
+ expect(body).toContain('#### Notes\n- One note.');
+ });
+});
+
+describe('renderFinding', () => {
+ it('labels a blocking finding and renders an applyable suggestion block', () => {
+ expect(renderFinding({
+ title: 'Use the repo install path',
+ body: 'Bare `npm install` rewrites tracked lockfiles.',
+ suggestion: 'for d in . client server autofixer; do (cd "$d" && npm install --no-save); done',
+ })).toEqual({
+ label: 'Use the repo install path',
+ body: [
+ '⛔ **Blocking** — Use the repo install path',
+ '',
+ 'Bare `npm install` rewrites tracked lockfiles.',
+ '',
+ '```suggestion',
+ 'for d in . client server autofixer; do (cd "$d" && npm install --no-save); done',
+ '```',
+ ].join('\n'),
+ });
+ });
+
+ it('labels a non-blocking finding, omits an absent suggestion, and falls back to the body for the index label', () => {
+ expect(renderFinding({ body: 'Consider naming Windows too.' }, { blocking: false })).toEqual({
+ body: '💡 **Non-blocking**\n\nConsider naming Windows too.',
+ label: 'Consider naming Windows too.',
+ });
+ });
+
+ it('drops a suggestion containing a fence so it cannot break out of the code block', () => {
+ expect(renderFinding({ body: 'x', suggestion: '```\nrm -rf /\n```' }).body).not.toContain('```');
+ });
+
+ it('drops the suggestion when the body leaves a fence open, so it cannot render as inert text', () => {
+ const rendered = renderFinding({ body: 'Replace it:\n\n```js\nconst a = 1;', suggestion: 'const a = 2;' });
+ expect(rendered.body).not.toContain('```suggestion');
+ expect(rendered.body).toContain('const a = 1;');
+ });
+
+ it('keeps the suggestion when the body fences are balanced', () => {
+ const rendered = renderFinding({ body: 'Today:\n\n```js\nconst a = 1;\n```', suggestion: 'const a = 2;' });
+ expect(rendered.body).toContain('```suggestion\nconst a = 2;\n```');
+ });
+
+ it('rejects a finding with no usable body', () => {
+ expect(renderFinding({ title: 'no body' })).toBeNull();
+ });
+});
+
+describe('normalizeReviewReport', () => {
+ it('collapses newlines in single-line fields so a model cannot inject headings', () => {
+ const report = normalizeReviewReport({ scope: 'docs\n\n## Injected heading', verified: ['a\n- b'] });
+ expect(report.scope).toBe('docs ## Injected heading');
+ expect(report.verified).toEqual(['a - b']);
+ });
+
+ it('defaults an unknown test status to not-run and drops empty entries', () => {
+ const report = normalizeReviewReport({ testEvidence: [{ command: 'a', status: 'green' }, {}, { detail: 'b' }] });
+ expect(report.testEvidence).toEqual([
+ { command: 'a', status: 'not-run', detail: '' },
+ { command: '', status: 'not-run', detail: 'b' },
+ ]);
+ });
+});
+
+describe('reviewReportText', () => {
+ it('exposes every model-authored string, report and finding alike, so the abuse scan sees them all', () => {
+ expect(reviewReportText({
+ summary: 'a', scope: 'b',
+ testEvidence: [{ command: 'c', status: 'pass', detail: 'd' }],
+ verified: ['e'], concerns: ['f'],
+ findings: [{ title: 'g', body: 'h', suggestion: 'i' }],
+ })).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i']);
+ });
+});
+
+describe('PR_REVIEW_DECISION_CONTRACT', () => {
+ // Two producers ask for this envelope (the pr-reviewer stage-3 body and the
+ // issue-watcher reasoning pass) and both feed normalizeReviewReport /
+ // renderFinding. A field added to the normalizer but not to the contract
+ // would silently never be asked for.
+ it('names every field the normalizer and the finding renderer read', () => {
+ for (const field of ['summary', 'scope', 'testEvidence', 'verified', 'concerns', 'findings']) {
+ expect(PR_REVIEW_DECISION_CONTRACT, field).toContain(`"${field}"`);
+ }
+ for (const field of ['path', 'line', 'side', 'blocking', 'title', 'body', 'suggestion']) {
+ expect(PR_REVIEW_DECISION_CONTRACT, field).toContain(`"${field}"`);
+ }
+ for (const status of ['pass', 'fail', 'not-run']) {
+ expect(PR_REVIEW_DECISION_CONTRACT, status).toContain(status);
+ }
+ });
+
+ it('is what the pr-reviewer stage-3 prompt actually ships', async () => {
+ const { DEFAULT_TASK_PROMPTS } = await import('../services/taskPromptDefaults/prompts.js');
+ expect(DEFAULT_TASK_PROMPTS['pr-reviewer-review']).toContain(PR_REVIEW_DECISION_CONTRACT);
+ });
+});
diff --git a/server/services/issueWatcher.js b/server/services/issueWatcher.js
index 11d85cc508..6e9b85ad59 100644
--- a/server/services/issueWatcher.js
+++ b/server/services/issueWatcher.js
@@ -19,6 +19,13 @@ import {
modelAbuseContentFingerprint,
} from '../lib/modelAbuseGuard.js';
import { getOriginInfo } from '../lib/gitRemote.js';
+import {
+ MAX_REVIEW_BODY_CHARS,
+ PR_REVIEW_DECISION_CONTRACT,
+ renderFinding,
+ renderReviewBody,
+ reviewReportText,
+} from '../lib/prReviewReport.js';
import { githubApiHost, githubRepoSpec } from '../lib/workTracker.js';
import { getAppById, updateApp } from './apps.js';
import { execGh, ensureForgeReachable } from './github.js';
@@ -162,14 +169,16 @@ function normalizeFinding(finding, anchors) {
const path = text(finding.path, 500);
const side = String(finding.side || '').toUpperCase();
const line = Number(finding.line);
- const body = text(finding.body, 3_000);
- if (!path || side !== 'RIGHT' || !Number.isInteger(line) || line < 1 || !body) return null;
+ // A finding without an explicit boolean is blocking. The watcher must not
+ // turn an incomplete model response into an automatic merge.
+ const blocking = finding.blocking !== false;
+ const rendered = renderFinding(finding, { blocking });
+ if (!path || side !== 'RIGHT' || !Number.isInteger(line) || line < 1 || !rendered) return null;
if (!anchors.has(`${path}\u0000${side}\u0000${line}`)) return null;
return {
- comment: { path, side, line, body },
- // A finding without an explicit boolean is blocking. The watcher must not
- // turn an incomplete model response into an automatic merge.
- blocking: finding.blocking !== false,
+ comment: { path, side, line, body: rendered.body },
+ label: rendered.label,
+ blocking,
};
}
@@ -184,7 +193,6 @@ function normalizeReviewDecision(value) {
verdict,
ciPolicy,
rebaseRequired: value.rebaseRequired,
- summary: text(value.summary, 4_000),
findings: Array.isArray(value.findings) ? value.findings : [],
};
}
@@ -197,10 +205,7 @@ export function isTaskOutputPayload(payload) {
function hasUnsafeGeneratedOutput(payload) {
const generatedText = [
...payload.issueComments.map((item) => item?.body),
- ...payload.pullRequests.flatMap((item) => [
- item?.summary,
- ...(Array.isArray(item?.findings) ? item.findings.map((finding) => finding?.body) : []),
- ]),
+ ...payload.pullRequests.flatMap((item) => reviewReportText(item)),
];
return generatedText.some((value) => detectDeterministicModelAbuseSignals(value).length > 0);
}
@@ -632,19 +637,15 @@ Return exactly this envelope through the completion sentinel (the outer \`summar
"summary": "brief completion summary",
"payload": {
"issueComments": [{ "issueNumber": 1, "commentId": 2, "action": "reply|none", "body": "reply text or empty" }],
- "pullRequests": [{
- "number": 3,
- "headSha": "exact supplied SHA",
- "verdict": "approve|request_changes|defer",
- "summary": "concise review summary",
- "findings": [{ "path": "src/file.js", "line": 42, "side": "RIGHT", "blocking": true, "body": "concrete problem and fix" }],
- "rebaseRequired": false,
- "ciPolicy": "required|skippable"
- }]
+ "pullRequests": [ ]
}
}
\`\`\`
+${PR_REVIEW_DECISION_CONTRACT}
+
+\`scope\`, \`testEvidence\`, \`verified\`, \`concerns\`, \`title\`, and \`suggestion\` are optional — omit one rather than padding it. This reasoning pass runs no commands, so \`testEvidence\` is normally empty here.
+
Include one decision for every supplied issue comment and PR, and no others.`;
}
@@ -914,7 +915,7 @@ async function readCurrentIssueComment(ctx, item) {
}
async function submitReview(ctx, number, { body, event, comments = [] }) {
- const input = JSON.stringify({ body: text(body, 4_000), event, comments });
+ const input = JSON.stringify({ body: text(body, MAX_REVIEW_BODY_CHARS), event, comments });
return runGh([...apiArgs(ctx, `repos/${ctx.repoFullName}/pulls/${number}/reviews`, { method: 'POST' }), '--input', '-'], ctx, input)
.then(() => true)
.catch((err) => {
@@ -924,7 +925,7 @@ async function submitReview(ctx, number, { body, event, comments = [] }) {
}
async function postReviewFallback(ctx, number, body) {
- return runGh(['pr', 'comment', String(number), '--repo', ctx.repoSpec, '--body', text(body, 5_000)], ctx)
+ return runGh(['pr', 'comment', String(number), '--repo', ctx.repoSpec, '--body', text(body, MAX_REVIEW_BODY_CHARS)], ctx)
.then(() => true)
.catch((err) => {
console.error(`❌ issue-watcher: review comment failed for PR #${number}: ${err.message}`);
@@ -1042,11 +1043,16 @@ export async function processTaskOutput({ appId, success, payload, task, require
if (!canApprove) {
if (!await eligibilityStillCurrent()) continue;
const downgraded = hasInvalidFinding && decision.verdict !== 'request_changes';
- const summary = `${decision.summary || 'This change needs follow-up before it can merge.'}${
- downgraded ? '\n\nPortOS could not anchor one or more reported findings to this diff, so the review is blocking until they are restated against exact added lines.' : ''}`;
const shouldRequestChanges = decision.verdict === 'request_changes'
|| hasInvalidFinding
|| blockingFindings.length > 0;
+ const summary = renderReviewBody({
+ report: raw,
+ verdict: shouldRequestChanges ? 'request_changes' : 'defer',
+ blockingFindings,
+ nonBlockingFindings: normalizedFindings.filter((entry) => !entry.blocking),
+ downgraded,
+ });
const posted = shouldRequestChanges
? await submitReview(ctx, pr.number, { body: summary, event: 'REQUEST_CHANGES', comments: findings })
|| await submitReview(ctx, pr.number, { body: summary, event: 'COMMENT', comments: findings })
@@ -1056,7 +1062,11 @@ export async function processTaskOutput({ appId, success, payload, task, require
continue;
}
- const approveBody = decision.summary || 'Reviewed: no material issues found.';
+ const approveBody = renderReviewBody({
+ report: raw,
+ verdict: 'approve',
+ nonBlockingFindings: normalizedFindings,
+ });
if (!await eligibilityStillCurrent()) continue;
const approved = await submitReview(ctx, pr.number, {
body: approveBody,
diff --git a/server/services/issueWatcher.test.js b/server/services/issueWatcher.test.js
index f0debe4e7f..fa5af8aa50 100644
--- a/server/services/issueWatcher.test.js
+++ b/server/services/issueWatcher.test.js
@@ -333,7 +333,10 @@ describe('processTaskOutput', () => {
expect(reviewCall).toBeTruthy();
expect(JSON.parse(reviewCall[2].input)).toMatchObject({
event: 'REQUEST_CHANGES',
- comments: [{ path: 'src/example.js', line: 2, side: 'RIGHT', body: 'Validate input before this call.' }],
+ comments: [{
+ path: 'src/example.js', line: 2, side: 'RIGHT',
+ body: '⛔ **Blocking**\n\nValidate input before this call.',
+ }],
});
expect(mergePrMock).not.toHaveBeenCalled();
});
@@ -558,7 +561,7 @@ describe('processTaskOutput', () => {
event: 'APPROVE',
comments: [{
path: 'src/example.js', line: 2, side: 'RIGHT',
- body: 'Consider making this helper name more specific in a follow-up.',
+ body: '💡 **Non-blocking**\n\nConsider making this helper name more specific in a follow-up.',
}],
});
expect(mergePrMock).toHaveBeenCalledWith(APP.repoPath, 7);
diff --git a/server/services/taskPromptDefaults/integrity.snapshot.json b/server/services/taskPromptDefaults/integrity.snapshot.json
index b7de8d06cf..729cc5999d 100644
--- a/server/services/taskPromptDefaults/integrity.snapshot.json
+++ b/server/services/taskPromptDefaults/integrity.snapshot.json
@@ -29,7 +29,7 @@
"plan-task": "b85fe92999aa4ee4a8910320457c4242",
"pr-reviewer": "679680b3b382aeb6786df01c4d1a90c6",
"pr-reviewer-eligibility": "ba358b2bb9380e2b7d9117969607231f",
- "pr-reviewer-review": "76d1d8265d4d56ae6a4c00d64e8787a8",
+ "pr-reviewer-review": "4b0621340de017adebf06229259ff22d",
"pr-reviewer-security": "d1e99626b12939ee39ab38eaa7d23f59",
"pr-watcher": "53ead8e26d396849bfa78f28550bd691",
"react-lifecycle": "14b3816d1bcc10f0d7d6357e55cc5e69",
diff --git a/server/services/taskPromptDefaults/prompts.js b/server/services/taskPromptDefaults/prompts.js
index a2b433b588..26243bd584 100644
--- a/server/services/taskPromptDefaults/prompts.js
+++ b/server/services/taskPromptDefaults/prompts.js
@@ -18,6 +18,9 @@ import {
formatContributorLabelReleaseCommands,
formatLabelCreateCommand,
} from '../../lib/dispatchLabels.js';
+// The PR-decision envelope is owned by the module that normalizes and renders
+// it, so stage 3 and the issue-watcher reasoning pass cannot drift apart.
+import { PR_REVIEW_DECISION_CONTRACT } from '../../lib/prReviewReport.js';
// The epic marker and its idempotent `label create` line come from the shared
// label registry, so the label the claim agent stamps is by construction the one
@@ -2409,25 +2412,12 @@ PR state and exact content fingerprint.
## Output (JSON only)
-Return exactly this shape, with no markdown and one entry for every eligible
-PR:
+Return exactly this envelope, with no markdown around it and one \`pullRequests\`
+entry for every eligible PR:
-{
- "issueComments": [],
- "pullRequests": [
- {
- "number": 123,
- "headSha": "40-character commit id",
- "verdict": "approve|request_changes|defer",
- "ciPolicy": "required|skippable",
- "rebaseRequired": false,
- "summary": "review summary and test evidence",
- "findings": [
- {"path": "src/file.js", "line": 42, "side": "RIGHT", "blocking": true, "body": "specific problem and fix"}
- ]
- }
- ]
-}
+{ "issueComments": [], "pullRequests": [ ] }
+
+${PR_REVIEW_DECISION_CONTRACT}
Do not include issue comments. Do not include a PR that was not in the eligible
input, duplicate a PR, or invent a head SHA. Do not quote Stage 1 findings or