fix: an oversized PR body is shortened, never a lost run (UPT-106)

.changeset/pr-body-never-too-large.md

@@ -0,0 +1,7 @@
+---
+"uptide": patch
+---
+
+A migration whose evidence is larger than GitHub's PR description limit no longer stops `fix` with "Required migration evidence exceeds the PR body budget" and loses the run. Long required lists get a compact form when they do not fit. Lockfile churn shows counts per kind of change and the first packages. Manual sites are grouped by rule, with counts and the first locations. Remaining type errors show the first entries. Every full list stays in report.json and, in the Uptide Action, in the job summary, which the description links to. If even the compact form does not fit, the description is a short summary that says so at the top: the verdict, the checks, the counts and where the full report is. The branch and report.json are always kept.
+
+An agent's decisions under "Decisions for you" name the files the agent changed, five at most, and count the rest, instead of every file in the task's scope. A verification-feedback task scoped to the whole repository made each of its decisions a list of hundreds of files.

packages/core/src/fix/budget.test.ts

@@ -27,7 +27,7 @@ it('keeps rank 0, then lower ranks first, and never cuts inside a block', () =>
   const out = fitPieces(
     [...pieces, { kind: 'text', text: 'low priority '.repeat(100), rank: 9 }],
     172,
-  );
+  ) as string;
   expect(out.length).toBeLessThanOrEqual(172);
   expect(out).toContain('# Title');
   expect(out).toContain('long diff');
@@ -52,12 +52,10 @@ it('falls back to one line per piece when almost nothing fits', () => {
 });
 
 it('keeps a complete document unchanged when it fits, even if omission notices are longer', () => {
-  const body = fitPieces(pieces);
+  const body = fitPieces(pieces) as string;
   expect(fitPieces(pieces, body.length)).toBe(body);
 });
 
-it('refuses oversized required evidence instead of silently dropping it', () => {
-  expect(() => fitPieces([must('required'.repeat(20))], 100)).toThrow(
-    'Required migration evidence',
-  );
+it('returns nothing for oversized required evidence instead of silently dropping it', () => {
+  expect(fitPieces([must('required'.repeat(20))], 100)).toBeUndefined();
 });

packages/core/src/fix/budget.ts

@@ -3,7 +3,8 @@
  * GitHub refuses a PR body over 65,536 characters. A piece is dropped whole (replaced by its
  * `alt`, usually one line saying where the content is), a list loses items from its end and
  * says how many; nothing is ever cut inside a block, so no `<details>` or code fence is left
- * open. Lower ranks are kept first; rank 0 is never dropped.
+ * open. Lower ranks are kept first; rank 0 is never dropped. When rank 0 alone does not fit,
+ * there is no document: the caller renders a shorter one (`undefined`).
  */
 export type Piece =
   | { kind: 'text'; text: string; rank: number; alt?: string }
@@ -20,7 +21,7 @@ export type Piece =
 
 export const must = (text: string): Piece => ({ kind: 'text', text, rank: 0 });
 
-export function fitPieces(pieces: Piece[], budget?: number): string {
+export function fitPieces(pieces: Piece[], budget?: number): string | undefined {
   const complete = pieces.map((p) => full(p)).join('\n');
   if (budget === undefined || complete.length <= budget) return complete;
   // What each piece contributes at minimum: a text's alt, a list's "all hidden" line.
@@ -34,12 +35,8 @@ export function fitPieces(pieces: Piece[], budget?: number): string {
         : '';
   const chosen = pieces.map(floor);
   let size = chosen.reduce((n, text) => n + text.length, 0) + Math.max(0, chosen.length - 1);
-  // Arbitrarily large required evidence cannot fit a finite field. Refuse instead of
-  // silently deleting decisions/diffs or handing GitHub an oversized description.
-  if (size > budget)
-    throw new Error(
-      'Required migration evidence exceeds the PR body budget; review the full report.json and narrow the migration before publishing.',
-    );
+  // Arbitrarily large required evidence cannot fit a finite field: nothing is silently cut.
+  if (size > budget) return undefined;
   const order = pieces
     .map((p, i) => ({ p, i }))
     .filter(({ p }) => p.rank > 0)

packages/core/src/fix/isolate.ts

@@ -336,8 +336,8 @@ function finish(
   const dir = storedRunDir(source, result.branch);
   mkdirSync(dir, { recursive: true });
   result.prBody = join(dir, 'pr-body.md');
-  writeFileSync(result.prBody, prBody(result));
   writeFileSync(join(dir, 'report.json'), JSON.stringify(result, null, 2));
+  writeFileSync(result.prBody, prBody(result));
   return result;
 }
 

packages/core/src/fix/pr-body.test.ts

@@ -87,15 +87,14 @@ it('applies the same budget to a stored many-site preview and the submitted body
   expect(readFileSync(m.run, 'utf8')).toBe(JSON.stringify(m.report));
 });
 
-it('refuses oversized required evidence before editing a PR and leaves the stored run intact', async () => {
+it('sends a shortened description for oversized required evidence and leaves the stored run intact', async () => {
   const m = mock();
   m.report.decisions = ['- Owner decision. '.repeat(PR_BODY_BUDGET)];
   writeFileSync(m.run, JSON.stringify(m.report));
-  await expect(updatePrBody({ cwd: temp, pr: '82', run: m.run }, m.io)).rejects.toThrow(
-    'Required migration evidence',
-  );
-  expect(m.sent()).toBe('');
-  expect(m.calls.flat()).not.toContain('edit');
+  const updated = await updatePrBody({ cwd: temp, pr: '82', run: m.run }, m.io);
+  expect(updated.body.length).toBeLessThanOrEqual(PR_BODY_BUDGET);
+  expect(updated.body).toContain('**Shortened description.**');
+  expect(m.sent()).toBe(updated.body);
   expect(readFileSync(m.run, 'utf8')).toBe(JSON.stringify(m.report));
 });
 it('falls back to the REST endpoint when gh pr edit trips over retired classic projects', async () => {

packages/core/src/fix/report-budget.test.ts

@@ -196,3 +196,114 @@ it('keeps the Action report and PR body portable without an npm invocation', asy
     vi.unstubAllEnvs();
   }
 });
+
+/** ~300 manual sites and ~100 churned lockfile entries: required lists far past the limit (UPT-106). */
+function overflowingReport(): FixReport {
+  const report = manySitesReport();
+  report.sites = report.sites.slice(0, 300).map((s, i) => ({
+    ...s,
+    outcome: 'manual' as const,
+    rule: `rule-${i % 3}`,
+    reason: `Needs a person: ${'the call site passes options the rule cannot rewrite safely; '.repeat(3)}${i}`,
+  }));
+  const churn = (prefix: string) =>
+    Array.from({ length: 100 }, (_, i) => ({
+      name: `${prefix}-${i}`,
+      ...(i % 4 === 0 ? {} : { from: '1.0.0' }),
+      ...(i % 4 === 1 ? {} : { to: i % 4 === 2 ? '1.0.0' : '1.1.0' }),
+    }));
+  report.lockfile = {
+    manager: 'npm',
+    file: 'package-lock.json',
+    added: [],
+    removed: [],
+    changed: [],
+    allowed: [],
+    outOfScope: churn('outside'),
+    parentChurn: churn('parent'),
+  };
+  return report;
+}
+
+it('renders 300 manual sites and 100 churned lockfile entries under the limit, every section represented', () => {
+  const report = overflowingReport();
+  report.runUrl = 'https://github.com/acme/app/actions/runs/42';
+  expect(migrationBody(report).length).toBeGreaterThan(PR_BODY_BUDGET);
+  const body = prBody(report);
+  expect(body.length).toBeLessThanOrEqual(PR_BODY_BUDGET);
+  for (const section of [
+    '### Lockfile changes outside the upgrade: review',
+    '### Lockfile churn the parent commit already has',
+    '| **Risk** |',
+    '### What changed',
+    '### Worth a look',
+    '### Decisions for you',
+    'Verification details',
+    'Run details',
+  ])
+    expect(body).toContain(section);
+  // Manual sites: by rule, with counts and the first locations.
+  expect(body).toContain('- Manual: 300 sites to change by hand');
+  expect(body).toContain('- `rule-0` (100): `src/billing-0.ts:1`, `src/billing-3.ts:4`');
+  expect(body).toContain('and 95 more');
+  expect(body).toContain('[job summary](https://github.com/acme/app/actions/runs/42)');
+  expect(body).toContain('- parent-99 1.0.0 → 1.1.0');
+  balanced(body);
+  // Tighter still, lockfile churn keeps its counts per kind of change and the first packages.
+  const tight = renderMigration(report, 'full', 8_000);
+  expect(tight.length).toBeLessThanOrEqual(8_000);
+  expect(tight).toContain('### Lockfile churn the parent commit already has');
+  expect(tight).toContain(
+    '100 entries: 25 added, 25 removed, 25 re-resolved at the same version, 25 version changed',
+  );
+  expect(tight).toContain('- parent-9 ');
+  expect(tight).not.toContain('- parent-10 ');
+  expect(tight).toContain('- Manual: 300 sites to change by hand');
+  balanced(tight);
+});
+
+it('falls back to a minimal body when even the compact sections do not fit', () => {
+  const report = overflowingReport();
+  report.decisions = ['- Owner decision. '.repeat(PR_BODY_BUDGET / 4)];
+  const body = prBody(report);
+  expect(body.length).toBeLessThanOrEqual(PR_BODY_BUDGET);
+  expect(body).toMatch(/^## Upgrade example-sdk [^\n]+\n\n> ⚠️ \*\*Shortened description\.\*\*/);
+  expect(body).toContain('Verification: PASS.');
+  expect(body).toContain('300 manual');
+  expect(body).toContain('report.json');
+  expect(body).not.toContain('Owner decision');
+});
+
+it('names the files an agent decision changed, not the task scope (UPT-106)', () => {
+  const report = manySitesReport();
+  const scope = Array.from({ length: 317 }, (_, i) => `src/module-${i}.ts`);
+  report.llm.tasks = [
+    {
+      id: 'verification-1',
+      rule: 'verification',
+      files: scope,
+      sites: 0,
+      fixed: 0,
+      turns: 3,
+      inputTokens: 0,
+      outputTokens: 0,
+      costUsd: 0,
+      durationMs: 0,
+      toolCalls: {},
+      gate: 'accepted',
+      diffs: { 'src/module-7.ts': '+x' },
+      summary: {
+        changed: [],
+        decisions: ['Kept the ref nullable'],
+        assumptions: [],
+        couldNotDo: [],
+        newFiles: [],
+      },
+    },
+  ];
+  expect(prBody(report)).toContain(
+    '- `src/module-7.ts`: Kept the ref nullable. Confirm or change.',
+  );
+  delete (report.llm.tasks[0] as { diffs?: unknown }).diffs;
+  expect(prBody(report)).toContain('`src/module-4.ts` and 312 more files: Kept the ref nullable.');
+});