Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/final-head-build.md
Original file line number Diff line number Diff line change
@@ -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.
130 changes: 129 additions & 1 deletion packages/core/src/fix/fix.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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) => {
Expand Down
140 changes: 98 additions & 42 deletions packages/core/src/fix/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,18 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
phases.push({ name, ms: now - lapped, ...(costUsd !== undefined ? { costUsd } : {}) });
lapped = now;
};
// Every build the run executes, preparation and verification, with its wall time.
const buildRuns = { count: 0, ms: 0 };
const timedBuild: typeof buildWorkspaces = async (...args) => {
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 &&
Expand Down Expand Up @@ -373,7 +385,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
if (services.install === install) await assertManagerAvailable(root);
// Generated clients and consumed workspace builds, before the planner compiles anything.
const prepared = await prepareWorkspace(root, workspacePackagesOf(root), {
build: services.build ?? buildWorkspaces,
build: timedBuild,
generate: services.generate ?? generateClients,
buildScript: options.buildScript,
onProgress: options.onProgress,
Expand Down Expand Up @@ -572,34 +584,50 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
list.map((w) => (w === '.' ? 'root' : (w.split('/').pop() ?? w))).join(', ');
lap('analyze');
const needsBuild = generic || pack.requiresBuild === true;
const runBuild = async (): Promise<BuildResult[]> => {
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 <T>(work: () => Promise<T>): Promise<T> => {
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', '--', '.');
git(root, 'clean', '-fdq');
}
}
};
const runBuild = async (): Promise<BuildResult[]> => {
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),
Expand Down Expand Up @@ -690,15 +718,19 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
[...bump.files, join(root, packageManager(root).lockfile)],
);
// The install replaced node_modules, and what was generated into it with them.
const rebuilt = await prepareWorkspace(root, workspaces, {
build: services.build ?? buildWorkspaces,
generate: services.generate ?? generateClients,
const consumedBuilds = {
build: timedBuild,
moved: [pack.name, ...companions.map((c) => 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));
Expand Down Expand Up @@ -947,7 +979,19 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
}
// Release cached programs before final verification builds independent ones.
targetChecker?.clear();
// 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<void> => {
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,
Expand All @@ -972,23 +1016,29 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
});
const unverified = typeResolutionFailure(root, baseline);
if (unverified && canAssist) fallback.push(`Unknown baseline: ${unverified}`);
// A workspace build that could not prepare consumed declarations is reported, not thrown.
for (const b of canAssist ? workspaceBuilds : [])
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'),
// 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,
Expand Down Expand Up @@ -1094,6 +1144,8 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
for (const file of Object.keys(task.diffs ?? {}))
if (!affected.includes(file)) affected.push(file);
build = await runBuild();
await rebuildConsumed();
buildFallback();
lint = await (services.lint ?? lintFiles)(root, affected, baselineLint);
after = await verify('types after feedback', () => services.diagnostics(root, workspaces));
ran = await verify('tests after feedback', () =>
Expand Down Expand Up @@ -1160,8 +1212,11 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
newErrors,
baselineTests,
tests,
...(needsBuild || workspaceBuilds.length
? { build: [...workspaceBuilds, ...build], ...(needsBuild ? { baselineBuild } : {}) }
...(needsBuild || workspaceBuilds.length || consumed.length
? {
build: [...workspaceBuilds, ...consumed, ...build],
...(needsBuild ? { baselineBuild } : {}),
}
: {}),
...(pack.requiresBuild ? { buildRequired: true } : {}),
...(lint.length ? { lint } : {}),
Expand All @@ -1186,6 +1241,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
...(behavior ? { behavior } : {}),
timingMs: Date.now() - started,
phases,
buildRuns,
prBody: join(root, '.uptide/pr-body.md'),
notes: [
...pins.map((pin) => describeRootPin(pin).replace('it will be updated', 'updated')),
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/fix/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading