From b2ef0035f0d96aeccf09482a4371c72c08d84330 Mon Sep 17 00:00:00 2001 From: Lucas Braum Date: Sat, 17 Oct 2026 18:24:27 -0700 Subject: [PATCH 1/2] fix(fix): judge consumed builds on the final head, not after the bump (UPT-104) The workspace builds prepareWorkspace runs right after the bump were reported as the run's build and turned into "Build cannot run" fallback reasons, even when the edits fixed them. That forced a no-op feedback round, an "Agent-led draft" banner and "Build failed" in the PR body. Rebuild them on the head before the verdict and after each feedback round, and recompute the build fallback reasons from those builds. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Lucas Braum --- .changeset/final-head-build.md | 5 ++ packages/core/src/fix/fix.test.ts | 80 +++++++++++++++++++++- packages/core/src/fix/run.ts | 108 +++++++++++++++++++----------- 3 files changed, 152 insertions(+), 41 deletions(-) create mode 100644 .changeset/final-head-build.md diff --git a/.changeset/final-head-build.md b/.changeset/final-head-build.md new file mode 100644 index 0000000..dfdb4b0 --- /dev/null +++ b/.changeset/final-head-build.md @@ -0,0 +1,5 @@ +--- +"uptide": patch +--- + +`uptide fix` rebuilds consumed workspaces on the final head before the verdict and before each feedback decision. A build that failed right after the bump and passes once the edits land no longer starts a feedback round, adds an "Agent-led draft" reason or shows "Build failed" in the PR body. diff --git a/packages/core/src/fix/fix.test.ts b/packages/core/src/fix/fix.test.ts index 552910c..c9c97fb 100644 --- a/packages/core/src/fix/fix.test.ts +++ b/packages/core/src/fix/fix.test.ts @@ -1495,7 +1495,8 @@ it('records baseline builds and rebuilds affected output after the upgrade throu }, ); expect(upgraded).toBe(true); - expect(build).toHaveBeenCalledTimes(2); + // Baseline, right after the bump (the agent's declarations), and again on the final head. + expect(build).toHaveBeenCalledTimes(3); expect(build).toHaveBeenNthCalledWith( 1, realpathSync(root), @@ -1565,6 +1566,83 @@ it('reports a workspace build that fails instead of throwing', async () => { ); }); +it('judges workspace builds on the final head, not right after the bump (UPT-104)', async () => { + const { root, services } = fixture(); + const internal = join(root, 'packages/internal'); + cpSync( + resolve(import.meta.dirname, '../../../../fixtures/workspaces/build-types/packages/internal'), + internal, + { recursive: true }, + ); + const internalManifest = join(internal, 'package.json'); + writeFileSync( + internalManifest, + JSON.stringify({ + ...JSON.parse(readFileSync(internalManifest, 'utf8')), + dependencies: { zod: '*' }, + scripts: { 'build:ci': 'node build.cjs' }, + }), + ); + const file = join(root, 'package.json'); + const pkg = JSON.parse(readFileSync(file, 'utf8')); + writeFileSync( + file, + JSON.stringify({ + ...pkg, + workspaces: ['packages/*'], + dependencies: { ...pkg.dependencies, internal: 'workspace:*' }, + }), + ); + writeFileSync(join(root, '.gitignore'), 'node_modules\ndist\n'); + git(root, 'add', '.'); + git(root, 'commit', '-m', 'workspace build fixture'); + let upgraded = false; + // Fails while the bumped tree still has the old call, as a consumer's build would. + const build: typeof buildWorkspaces = async (...args) => + upgraded && readFileSync(join(args[0], 'index.ts'), 'utf8').includes('required_error') + ? [ + { + workspace: 'packages/internal', + status: 'failed', + command: 'node build.cjs', + script: 'build:ci', + output: "index.ts(2,30): error TS2353: 'required_error' does not exist", + }, + ] + : buildWorkspaces(...args); + const fixer = { + id: 'test', + fix: vi.fn(async () => { + throw new Error('no agent work expected'); + }), + }; + const result = await fix( + { cwd: root, only: 'zod', pack: zodPack, buildScript: 'build:ci', fixer }, + { + ...services, + build, + install: async (...args) => { + await services.install(...args); + upgraded = true; + }, + }, + ); + expect(readFileSync(join(result.repo, 'index.ts'), 'utf8')).not.toContain('required_error'); + expect(result.agentFallback).toBeUndefined(); + expect(result.llm.feedback?.roundsUsed).toBe(0); + expect(fixer.fix).not.toHaveBeenCalled(); + expect(result.verification.build).toMatchObject([ + { phase: 'baseline', workspace: 'packages/internal', status: 'passed' }, + { workspace: 'packages/internal', status: 'passed' }, + ]); + const body = prBody(result); + expect(body).toContain( + 'Build after the bump in `packages/internal` (`build:ci` script): `node build.cjs` — passed', + ); + expect(body).not.toContain('Agent-led draft'); + expect(body).not.toMatch(/build failed/i); +}); + it.each([{ updatePins: true }, { pr: true }, { forPublication: true }])( 'updates approved root pins in the bump commit and PR: %j', async (consent) => { diff --git a/packages/core/src/fix/run.ts b/packages/core/src/fix/run.ts index d8b5a89..ccdb5f1 100644 --- a/packages/core/src/fix/run.ts +++ b/packages/core/src/fix/run.ts @@ -572,27 +572,11 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise (w === '.' ? 'root' : (w.split('/').pop() ?? w))).join(', '); lap('analyze'); const needsBuild = generic || pack.requiresBuild === true; - const runBuild = async (): Promise => { - if (!needsBuild) return []; + // A build leaves a clean tree as it found it. + const keepClean = async (work: () => Promise): Promise => { const clean = !git(root, 'status', '--porcelain', '--untracked-files=all'); try { - return await verify(`build (${names(workspaces)})`, () => - (services.build ?? buildWorkspaces)( - root, - workspaces, - undefined, - options.buildScript ?? configuredBuildScript(root), - ), - ); - } catch (error) { - return [ - { - workspace: '.', - status: 'failed', - cause: 'build could not run', - output: error instanceof Error ? error.message : String(error), - }, - ]; + return await work(); } finally { if (clean && git(root, 'status', '--porcelain', '--untracked-files=all')) { git(root, 'checkout', '--', '.'); @@ -600,6 +584,30 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise => { + if (!needsBuild) return []; + return keepClean(async () => { + try { + return await verify(`build (${names(workspaces)})`, () => + (services.build ?? buildWorkspaces)( + root, + workspaces, + undefined, + options.buildScript ?? configuredBuildScript(root), + ), + ); + } catch (error) { + return [ + { + workspace: '.', + status: 'failed', + cause: 'build could not run', + output: error instanceof Error ? error.message : String(error), + }, + ]; + } + }); + }; const baselineBuild = await runBuild(); const baseline = await verify(`types (${names(workspaces)})`, () => services.diagnostics(root, workspaces), @@ -690,15 +698,17 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise c.name), ...removals.map((r) => r.name)], buildScript: options.buildScript, onProgress: options.onProgress, + }; + const rebuilt = await prepareWorkspace(root, workspaces, { + ...consumedBuilds, + generate: services.generate ?? generateClients, }); generated.push(...rebuilt.generated.map((g) => ({ ...g, phase: 'bump' as const }))); - workspaceBuilds.push(...rebuilt.builds); const targetChecker = sdkAgent && services.diagnostics === diagnostics ? typeChecker(root, workspaces) : undefined; const checkTarget = targetChecker?.check ?? (() => services.diagnostics(root, workspaces)); @@ -947,7 +957,14 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise => + rebuilt.builds.length + ? keepClean(async () => (await prepareWorkspace(root, workspaces, consumedBuilds)).builds) + : []; let build = await runBuild(); + let consumed = await rebuildConsumed(); let { tests: ran, after, @@ -972,23 +989,29 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise - p.workspace === b.workspace && - p.command === b.command && - (p.status === 'failed' || p.status === 'timeout'), + // Build reasons are the head's: recomputed after each build, so a fixed build drops its own. + const headIndependent = fallback.length; + const buildFallback = (): void => { + fallback.length = headIndependent; + // A workspace build that could not prepare consumed declarations is reported, not thrown. + for (const b of canAssist ? [...workspaceBuilds, ...consumed] : []) + if (b.status !== 'passed') + fallback.push(`Build cannot run (${b.workspace}): ${b.cause ?? trimOutput(b.output)}`); + for (const b of canAssist ? build : []) { + if ( + baselineBuild.some( + (p) => + p.workspace === b.workspace && + p.command === b.command && + (p.status === 'failed' || p.status === 'timeout'), + ) ) - ) - continue; - if ((pack.requiresBuild && b.status === 'missing') || b.cause || b.status === 'timeout') - fallback.push(`Build cannot run (${b.workspace}): ${b.cause ?? trimOutput(b.output)}`); - } + continue; + if ((pack.requiresBuild && b.status === 'missing') || b.cause || b.status === 'timeout') + fallback.push(`Build cannot run (${b.workspace}): ${b.cause ?? trimOutput(b.output)}`); + } + }; + buildFallback(); const feedbackFinding: Finding = { severity: 'breaking', confidence: 1, @@ -1094,6 +1117,8 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise services.diagnostics(root, workspaces)); ran = await verify('tests after feedback', () => @@ -1160,8 +1185,11 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise Date: Sat, 17 Oct 2026 18:56:53 -0700 Subject: [PATCH 2/2] fix(fix): rebuild consumed workspaces only when the tree changed (UPT-104) The final-head rebuild ran even when nothing changed after a passing after-bump build, and beside a verification build that already built the same workspaces. Keep the last result with the tree state it was taken on and build again only when that state changed; take the verification build's results when it covers those workspaces. The report's buildRuns counts every build the run executed and their wall time, so runs can be compared. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Lucas Braum --- .changeset/final-head-build.md | 2 +- packages/core/src/fix/fix.test.ts | 128 +++++++++++++++++++++--------- packages/core/src/fix/run.ts | 50 +++++++++--- packages/core/src/fix/types.ts | 2 + 4 files changed, 131 insertions(+), 51 deletions(-) diff --git a/.changeset/final-head-build.md b/.changeset/final-head-build.md index dfdb4b0..556ca12 100644 --- a/.changeset/final-head-build.md +++ b/.changeset/final-head-build.md @@ -2,4 +2,4 @@ "uptide": patch --- -`uptide fix` rebuilds consumed workspaces on the final head before the verdict and before each feedback decision. A build that failed right after the bump and passes once the edits land no longer starts a feedback round, adds an "Agent-led draft" reason or shows "Build failed" in the PR body. +`uptide fix` judges consumed workspace builds on the final head, not on the tree right after the bump. A build that failed after the bump and passes once the edits land no longer starts a feedback round, adds an "Agent-led draft" reason or shows "Build failed" in the PR body. The builds run again only when the tree changed since their last result, and not at all when the verification build already built those workspaces. The fix report's `buildRuns` gives the number of builds the run executed and their wall time. diff --git a/packages/core/src/fix/fix.test.ts b/packages/core/src/fix/fix.test.ts index c9c97fb..85668ae 100644 --- a/packages/core/src/fix/fix.test.ts +++ b/packages/core/src/fix/fix.test.ts @@ -1566,7 +1566,8 @@ it('reports a workspace build that fails instead of throwing', async () => { ); }); -it('judges workspace builds on the final head, not right after the bump (UPT-104)', async () => { +/** The zod fixture, whose root consumes `packages/internal`'s built declarations. */ +function consumedWorkspace(index?: string) { const { root, services } = fixture(); const internal = join(root, 'packages/internal'); cpSync( @@ -1591,56 +1592,105 @@ it('judges workspace builds on the final head, not right after the bump (UPT-104 ...pkg, workspaces: ['packages/*'], dependencies: { ...pkg.dependencies, internal: 'workspace:*' }, + scripts: { 'build:ci': 'node -e 0' }, }), ); + if (index) writeFileSync(join(root, 'index.ts'), index); writeFileSync(join(root, '.gitignore'), 'node_modules\ndist\n'); git(root, 'add', '.'); git(root, 'commit', '-m', 'workspace build fixture'); - let upgraded = false; - // Fails while the bumped tree still has the old call, as a consumer's build would. - const build: typeof buildWorkspaces = async (...args) => - upgraded && readFileSync(join(args[0], 'index.ts'), 'utf8').includes('required_error') - ? [ - { - workspace: 'packages/internal', - status: 'failed', - command: 'node build.cjs', - script: 'build:ci', - output: "index.ts(2,30): error TS2353: 'required_error' does not exist", - }, - ] - : buildWorkspaces(...args); const fixer = { id: 'test', fix: vi.fn(async () => { throw new Error('no agent work expected'); }), }; - const result = await fix( - { cwd: root, only: 'zod', pack: zodPack, buildScript: 'build:ci', fixer }, - { - ...services, - build, - install: async (...args) => { - await services.install(...args); - upgraded = true; + return { root, services, fixer }; +} + +describe('consumed workspace builds on the final head (UPT-104)', () => { + it('judges a build that failed after the bump by the head the edits left', async () => { + const { root, services, fixer } = consumedWorkspace(); + let upgraded = false; + // Fails while the bumped tree still has the old call, as a consumer's build would. + const build: typeof buildWorkspaces = async (...args) => + upgraded && readFileSync(join(args[0], 'index.ts'), 'utf8').includes('required_error') + ? [ + { + workspace: 'packages/internal', + status: 'failed', + command: 'node build.cjs', + script: 'build:ci', + output: "index.ts(2,30): error TS2353: 'required_error' does not exist", + }, + ] + : buildWorkspaces(...args); + const result = await fix( + { cwd: root, only: 'zod', pack: zodPack, buildScript: 'build:ci', fixer }, + { + ...services, + build, + install: async (...args) => { + await services.install(...args); + upgraded = true; + }, }, - }, - ); - expect(readFileSync(join(result.repo, 'index.ts'), 'utf8')).not.toContain('required_error'); - expect(result.agentFallback).toBeUndefined(); - expect(result.llm.feedback?.roundsUsed).toBe(0); - expect(fixer.fix).not.toHaveBeenCalled(); - expect(result.verification.build).toMatchObject([ - { phase: 'baseline', workspace: 'packages/internal', status: 'passed' }, - { workspace: 'packages/internal', status: 'passed' }, - ]); - const body = prBody(result); - expect(body).toContain( - 'Build after the bump in `packages/internal` (`build:ci` script): `node build.cjs` — passed', - ); - expect(body).not.toContain('Agent-led draft'); - expect(body).not.toMatch(/build failed/i); + ); + expect(readFileSync(join(result.repo, 'index.ts'), 'utf8')).not.toContain('required_error'); + expect(result.agentFallback).toBeUndefined(); + expect(result.llm.feedback?.roundsUsed).toBe(0); + expect(fixer.fix).not.toHaveBeenCalled(); + expect(result.verification.build).toMatchObject([ + { phase: 'baseline', workspace: 'packages/internal', status: 'passed' }, + { workspace: 'packages/internal', status: 'passed' }, + ]); + // Baseline, after the bump, and once on the edited head. + expect(result.buildRuns?.count).toBe(3); + const body = prBody(result); + expect(body).toContain( + 'Build after the bump in `packages/internal` (`build:ci` script): `node build.cjs` — passed', + ); + expect(body).not.toContain('Agent-led draft'); + expect(body).not.toMatch(/build failed/i); + }); + + it('does not build again when nothing changed after a passing after-bump build', async () => { + const { root, services, fixer } = consumedWorkspace( + "import { z } from 'zod';\nexport const schema = z.string();\n", + ); + const build = vi.fn(buildWorkspaces); + const result = await fix( + { cwd: root, only: 'zod', pack: zodPack, buildScript: 'build:ci', fixer }, + { ...services, build }, + ); + expect(result.changedFiles).not.toContain('index.ts'); + expect(build).toHaveBeenCalledTimes(2); + expect(result.buildRuns?.count).toBe(2); + expect(result.verification.build).toMatchObject([ + { phase: 'baseline', workspace: 'packages/internal', status: 'passed' }, + { workspace: 'packages/internal', status: 'passed' }, + ]); + }); + + it('reuses the verification build when it builds the consumed workspace on the head', async () => { + const { root, services, fixer } = consumedWorkspace(); + const build = vi.fn(buildWorkspaces); + const result = await fix( + { + cwd: root, + only: 'zod', + pack: { ...zodPack, requiresBuild: true }, + buildScript: 'build:ci', + fixer, + }, + { ...services, build }, + ); + // Preparation before the baseline, the baseline build, after the bump, the final build. + expect(build).toHaveBeenCalledTimes(4); + const after = result.verification.build?.filter((b) => b.phase !== 'baseline') ?? []; + expect(after.filter((b) => b.workspace === 'packages/internal')).toHaveLength(1); + expect(after.every((b) => b.status === 'passed')).toBe(true); + }); }); it.each([{ updatePins: true }, { pr: true }, { forPublication: true }])( diff --git a/packages/core/src/fix/run.ts b/packages/core/src/fix/run.ts index ccdb5f1..3f14053 100644 --- a/packages/core/src/fix/run.ts +++ b/packages/core/src/fix/run.ts @@ -271,6 +271,18 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise { + const start = Date.now(); + try { + const results = await (services.build ?? buildWorkspaces)(...args); + buildRuns.count += results.filter((b) => b.command).length; + return results; + } finally { + buildRuns.ms += Date.now() - start; + } + }; const root = realpathSync(resolve(options.cwd)); if ( options.maxCostUsd !== undefined && @@ -373,7 +385,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise (w === '.' ? 'root' : (w.split('/').pop() ?? w))).join(', '); lap('analyze'); const needsBuild = generic || pack.requiresBuild === true; + // ponytail: what a build sees, as HEAD's tree plus the diff and untracked names over it; an + // untracked file edited in place goes unseen. + const treeState = (): string => + [ + git(root, 'rev-parse', 'HEAD^{tree}'), + git(root, 'diff', 'HEAD'), + git(root, 'status', '--porcelain', '--untracked-files=all'), + ].join('\0'); // A build leaves a clean tree as it found it. const keepClean = async (work: () => Promise): Promise => { const clean = !git(root, 'status', '--porcelain', '--untracked-files=all'); @@ -589,7 +609,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise { try { return await verify(`build (${names(workspaces)})`, () => - (services.build ?? buildWorkspaces)( + timedBuild( root, workspaces, undefined, @@ -699,7 +719,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise c.name), ...removals.map((r) => r.name)], buildScript: options.buildScript, onProgress: options.onProgress, @@ -709,6 +729,8 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise ({ ...g, phase: 'bump' as const }))); + let consumed = rebuilt.builds; + let consumedAt = treeState(); const targetChecker = sdkAgent && services.diagnostics === diagnostics ? typeChecker(root, workspaces) : undefined; const checkTarget = targetChecker?.check ?? (() => services.diagnostics(root, workspaces)); @@ -957,14 +979,19 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise => - rebuilt.builds.length - ? keepClean(async () => (await prepareWorkspace(root, workspaces, consumedBuilds)).builds) - : []; + // The consumed builds ran right after the bump: what is reported, and what feedback and the + // fallback decide on, is the head the edits left (UPT-104). They run again only when the tree + // changed since, and never beside a verification build that built the same workspaces. + const rebuildConsumed = async (): Promise => { + if (consumed.every((c) => build.some((b) => b.workspace === c.workspace))) consumed = []; + if (!consumed.length || treeState() === consumedAt) return; + consumed = await keepClean( + async () => (await prepareWorkspace(root, workspaces, consumedBuilds)).builds, + ); + consumedAt = treeState(); + }; let build = await runBuild(); - let consumed = await rebuildConsumed(); + await rebuildConsumed(); let { tests: ran, after, @@ -1117,7 +1144,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise services.diagnostics(root, workspaces)); @@ -1214,6 +1241,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise describeRootPin(pin).replace('it will be updated', 'updated')), diff --git a/packages/core/src/fix/types.ts b/packages/core/src/fix/types.ts index b44e972..73ef0f4 100644 --- a/packages/core/src/fix/types.ts +++ b/packages/core/src/fix/types.ts @@ -326,6 +326,8 @@ export interface FixReport { timingMs: number; /** Wall time and agent spend per phase, in run order; together about `timingMs`. */ phases?: FixPhase[]; + /** The builds the run executed (workspace preparation and verification) and their wall time. */ + buildRuns?: { count: number; ms: number }; prBody: string; /** The migration report as a page, written by the CLI next to the stored run. */ html?: string;