diff --git a/.changeset/final-head-build.md b/.changeset/final-head-build.md new file mode 100644 index 0000000..556ca12 --- /dev/null +++ b/.changeset/final-head-build.md @@ -0,0 +1,5 @@ +--- +"uptide": patch +--- + +`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 552910c..85668ae 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,133 @@ it('reports a workspace build that fails instead of throwing', 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( + 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:*' }, + 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'); + const fixer = { + id: 'test', + fix: vi.fn(async () => { + throw new Error('no agent work expected'); + }), + }; + 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' }, + ]); + // 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 }])( '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..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; - const runBuild = async (): Promise => { - if (!needsBuild) return []; + // 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'); 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 +604,30 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise => { + if (!needsBuild) return []; + return keepClean(async () => { + try { + return await verify(`build (${names(workspaces)})`, () => + timedBuild( + 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 +718,19 @@ 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); + 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)); @@ -947,7 +979,19 @@ async function fixPackage(options: FixOptions, services: FixServices): 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(); + await rebuildConsumed(); let { tests: ran, after, @@ -972,23 +1016,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 +1144,8 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise services.diagnostics(root, workspaces)); ran = await verify('tests after feedback', () => @@ -1160,8 +1212,11 @@ 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;