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
9 changes: 9 additions & 0 deletions .changeset/agent-phase-wall-time.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"uptide": patch
---

`uptide fix` spends less wall time on test runs and dependency copies:

- The agent's own `run_tests` check runs only its runner's related tests when the `test` script builds first and then runs a script that is just the runner (`pnpm run build && pnpm run test:unit`). The gate after the agent's edits and the final verification run exactly what they ran before.
- The upgrade's install moves the installed `node_modules` out of its temporary worktree instead of copying it.
- Each phase of the fix report lists the repository's commands it ran (installs, generators, builds, type-checks, tests, lint, format) with their count and summed wall time. An agent-led run's clean install has its own phase.
31 changes: 31 additions & 0 deletions packages/core/src/fix/fix.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,37 @@ describe('test detection', () => {
]);
});

it("scopes only the agent's own check to the runner a test script chain ends with", () => {
const root = mkdtempSync(join(scratch, 'chain-'));
const write = (file: string, text: string, mode?: number) => {
mkdirSync(join(root, file, '..'), { recursive: true });
writeFileSync(join(root, file), text, mode ? { mode } : {});
};
write('node_modules/.bin/vitest', '#!/bin/sh\n', 0o755);
const test = 'pnpm run build && pnpm run test:unit';
write(
'package.json',
JSON.stringify({ name: 'app', scripts: { build: 'tsc', test, 'test:unit': 'vitest run' } }),
);
write('src/a.ts', '');
const files = ['src/a.ts'];
expect(planTests(root, ['.'], files, { related: true, agentCheck: true })).toEqual([
{
workspace: '.',
covers: ['.'],
cwd: root,
command: 'vitest related --run --passWithNoTests src/a.ts',
scope:
'vitest tests related to 1 changed file (its `test` script ends with `test:unit`, the runner; the steps before it did not run)',
},
]);
// The gate and verification run the whole script, build first, as before.
expect(planTests(root, ['.'], files, { related: true }).map((p) => p.command)).toEqual([test]);
expect(planTests(root, ['.'], files, { fullSuite: true }).map((p) => p.command)).toEqual([
test,
]);
});

it('knows jest, prefers a workspace script, then the root script, and says when nothing covers a workspace', () => {
const jest = monorepo('jest');
expect(
Expand Down
96 changes: 96 additions & 0 deletions packages/core/src/fix/generic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ describe('fix for a dependency without a pack', () => {
'feedback',
]);
expect(result.phases?.find((p) => p.name === 'agent')?.costUsd).toBe(0.6);
// The repository's commands each phase ran, counted where they ran.
expect(result.phases?.find((p) => p.name === 'baseline')?.runs?.types?.count).toBe(1);
expect(result.phases?.find((p) => p.name === 'baseline')?.runs?.tests?.count).toBe(1);
const total = (result.phases ?? []).reduce((ms, p) => ms + p.ms, 0);
expect(total).toBeLessThanOrEqual(result.timingMs);
expect(prBody(result)).toMatch(/Phases: analyze \d+s · baseline \d+s · .* agent \d+s \$0\.60/);
Expand Down Expand Up @@ -382,6 +385,99 @@ describe('fix for a dependency without a pack', () => {
);
}, 60000);

it.each([
[
'ends by running the runner',
'pnpm run build && pnpm run test:unit',
['full', 'check', 'gate', 'full'],
false,
],
// The check is the gate's own run, whose result the gate reuses on the same tree.
['is the runner', 'vitest run', ['full', 'gate', 'full'], false],
// Agent-led: the forced feedback round's check and its after-round run are the full
// suite, and the branch must still install from scratch (UPT-102).
[
'ends by running the runner, agent-led',
'pnpm run build && pnpm run test:unit',
['full', 'check', 'gate', 'full', 'full', 'full'],
true,
],
])(
"narrows only the agent's own check, where the test script %s; the gate and the final verification run as before",
async (_, test, expected, agentLed) => {
const { root, services, fixer } = paintFixture();
const manifest = JSON.parse(readFileSync(join(root, 'package.json'), 'utf8'));
writeFileSync(
join(root, 'package.json'),
JSON.stringify({ ...manifest, scripts: { build: 'tsc', test, 'test:unit': 'vitest run' } }),
);
mkdirSync(join(root, 'node_modules/.bin'), { recursive: true });
writeFileSync(join(root, 'node_modules/.bin/vitest'), '#!/bin/sh\n', { mode: 0o755 });
git(root, 'commit', '-qam', 'test scripts');
const calls: {
files: string[];
related?: boolean;
agentCheck?: boolean;
fullSuite?: boolean;
}[] = [];
const result = await fix(
{
cwd: root,
only: 'paint',
fixer: fixer(0.01),
agent: 'sdk',
...(agentLed ? { agentLed: ['a peer blocker the agent continues past'] } : {}),
},
{
...services,
tests: async (_root, _workspaces, _timeout, files = [], options = {}) => {
calls.push({ files, ...options });
return [{ workspace: '.', status: 'passed', output: '' }];
},
agentRunner: async ({ root, checks }) => {
writeFileSync(
join(root, 'wall1.ts'),
"import { fill } from 'paint';\nfill({ name: 'red' });\n",
);
await checks.tests();
return {
turns: 2,
inputTokens: 1,
outputTokens: 1,
costUsd: 0.01,
durationMs: 1,
toolCalls: {},
summary: {
changed: [],
newFiles: [],
decisions: [],
assumptions: [],
couldNotDo: [],
},
};
},
},
);
expect(result.verification.passed).toBe(true);
expect(result.sites.map((s) => s.outcome)).toEqual(['agent']);
const kind = (c: (typeof calls)[number]) =>
c.agentCheck ? 'check' : c.fullSuite ? 'full' : c.related ? 'gate' : 'other';
// Baseline in full; the agent's check; its gate as before; then the full suite.
expect(calls.map(kind)).toEqual(expected);
expect(calls.at(-1)).toMatchObject({ fullSuite: true });
expect(calls.at(-1)?.related).toBeUndefined();
// The final verification still builds, type-checks and tests the head the agent left.
const verify = result.phases?.find((p) => p.name === 'verify');
expect(verify?.runs?.build?.count).toBe(1);
expect(verify?.runs?.types?.count).toBe(1);
expect(verify?.runs?.tests?.count).toBe(1);
const clean = result.phases?.find((p) => p.name === 'clean-install');
expect(clean?.runs?.install?.count).toBe(agentLed ? 1 : undefined);
expect(result.cleanInstall?.passed).toBe(agentLed ? true : undefined);
},
30000,
);

it('stops before any agent call when the estimate exceeds the limit', async () => {
for (const [costSetting, raise] of [
[undefined, 'Run again with --max-cost 0.18.'],
Expand Down
40 changes: 39 additions & 1 deletion packages/core/src/fix/managers/isolated-install.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import {
existsSync,
mkdirSync,
mkdtempSync,
readFileSync,
readlinkSync,
rmSync,
statSync,
symlinkSync,
writeFileSync,
} from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterAll, expect, it } from 'vitest';
Expand Down Expand Up @@ -69,6 +79,34 @@ it('installs in a detached worktree, promotes lockfile bytes unchanged and clean
expect(existsSync(temporary)).toBe(false);
expect(git(root, 'worktree', 'list')).not.toContain('uptide-install-');
});
it('moves the installed tree out of the worktree, links pointing into it relocated', async () => {
const root = fixture();
let installed = 0;
await isolatedUpgrade(root, upgrade, async (dir) => {
mkdirSync(join(dir, 'node_modules/.bin'), { recursive: true });
writeFileSync(join(dir, 'node_modules/target'), 'new');
installed = statSync(join(dir, 'node_modules/target')).ino;
symlinkSync('../target', join(dir, 'node_modules/.bin/relative'));
symlinkSync(join(dir, 'node_modules/target'), join(dir, 'node_modules/.bin/absolute'));
writeFileSync(join(dir, 'package-lock.json'), '{"lockfileVersion":3,"packages":{}}\n');
return {
file: 'package-lock.json',
manager: 'npm',
added: ['target'],
removed: [],
changed: [],
allowed: ['target'],
};
});
// The same file the install wrote, not a copy of it.
expect(statSync(join(root, 'node_modules/target')).ino).toBe(installed);
expect(existsSync(join(root, 'node_modules/baseline'))).toBe(false);
expect(readlinkSync(join(root, 'node_modules/.bin/relative'))).toBe('../target');
expect(readlinkSync(join(root, 'node_modules/.bin/absolute'))).toBe(
join(root, 'node_modules/target'),
);
expect(readFileSync(join(root, 'node_modules/.bin/absolute'), 'utf8')).toBe('new');
});
it('failed/out-of-scope installs never replace the branch lockfile or dependencies', async () => {
const root = fixture(),
before = readFileSync(join(root, 'package-lock.json'), 'utf8');
Expand Down
15 changes: 14 additions & 1 deletion packages/core/src/fix/managers/isolated-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ function relocateLinks(dir: string, from: string, to: string): void {
}
}
}
/**
* The installed tree moves out of the worktree, which is removed anyway, rather than being
* copied: a copy of a large node_modules cost seconds (enki: 7.5 s) and gave the same tree.
* Across filesystems it is copied as before.
*/
function moveTree(source: string, target: string): void {
try {
renameSync(source, target);
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== 'EXDEV') throw error;
cpSync(source, target, { recursive: true, verbatimSymlinks: true });
}
}
/** Promote only a validated lockfile and its already-installed dependencies. No install runs on the branch. */
export async function isolatedUpgrade(
root: string,
Expand Down Expand Up @@ -71,7 +84,7 @@ export async function isolatedUpgrade(
if (!existsSync(source) && !existsSync(target)) continue;
const staged = existsSync(source) ? join(staging, `new-${i}`) : undefined;
if (staged) {
cpSync(source, staged, { recursive: true, verbatimSymlinks: true });
moveTree(source, staged);
relocateLinks(staged, checkout, root);
}
modules.push({
Expand Down
67 changes: 60 additions & 7 deletions packages/core/src/fix/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,15 @@ import { describeRootPin, planRootPins, writeRootPins } from './root-pins.js';
import { assistWithSdk, sdkInstalled, type TaskRunner } from './sdk-agent.js';
import { selectedFindings } from './select.js';
import { applyFollowUps, commit, type Followed, safeFile, settle } from './settle.js';
import { lintFiles } from './style.js';
import { formatFiles, lintFiles } from './style.js';
import { type ResolvedTarget, resolveTarget } from './target.js';
import type {
BuildResult,
FixDiagnostic,
Fixer,
FixPhase,
FixReport,
FixRunKind,
FixSite,
LintResult,
TestResult,
Expand All @@ -82,6 +83,7 @@ import {
gitLineMoves,
markPreexisting,
newDiagnostics,
planTests,
type TestOptions,
testArtifacts,
testWorkspaces,
Expand Down Expand Up @@ -266,14 +268,37 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
// Each phase ends where the next begins, so together they are the run's wall time.
const phases: FixPhase[] = [];
let lapped = started;
let runs: NonNullable<FixPhase['runs']> = {};
const lap = (name: FixPhase['name'], costUsd?: number): void => {
const now = Date.now();
phases.push({ name, ms: now - lapped, ...(costUsd !== undefined ? { costUsd } : {}) });
phases.push({
name,
ms: now - lapped,
...(costUsd !== undefined ? { costUsd } : {}),
...(Object.keys(runs).length ? { runs } : {}),
});
lapped = now;
runs = {};
};
// The repository's commands each phase runs, counted where they are called.
const timed =
<A extends unknown[], R>(kind: FixRunKind, work: (...args: A) => R) =>
(...args: A): R => {
const start = Date.now();
const done = (): void => {
const entry = runs[kind] ?? { count: 0, ms: 0 };
entry.count++;
entry.ms += Date.now() - start;
runs[kind] = entry;
};
const result = work(...args);
if (result instanceof Promise) return result.finally(done) as R;
done();
return result;
};
// 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 timedBuild: typeof buildWorkspaces = timed('build', async (...args) => {
const start = Date.now();
try {
const results = await (services.build ?? buildWorkspaces)(...args);
Expand All @@ -282,7 +307,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
} finally {
buildRuns.ms += Date.now() - start;
}
};
});
const root = realpathSync(resolve(options.cwd));
if (
options.maxCostUsd !== undefined &&
Expand Down Expand Up @@ -383,6 +408,16 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
const branch = `uptide/${pack.name}-${target}`;
packageManager(root);
if (services.install === install) await assertManagerAvailable(root);
const defaultDiagnostics = services.diagnostics === diagnostics;
services = {
...services,
install: timed('install', services.install),
diagnostics: timed('types', services.diagnostics),
tests: timed('tests', services.tests),
lint: timed('lint', services.lint ?? lintFiles),
format: timed('format', services.format ?? formatFiles),
generate: timed('generate', services.generate ?? generateClients),
};
// Generated clients and consumed workspace builds, before the planner compiles anything.
const prepared = await prepareWorkspace(root, workspacePackagesOf(root), {
build: timedBuild,
Expand Down Expand Up @@ -731,9 +766,10 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
generated.push(...rebuilt.generated.map((g) => ({ ...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));
const targetChecker = sdkAgent && defaultDiagnostics ? typeChecker(root, workspaces) : undefined;
const checkTarget = targetChecker
? timed('types', targetChecker.check)
: () => services.diagnostics(root, workspaces);
const targetErrors = await verify(`types (${names(workspaces)})`, checkTarget);
lap('install');
const sites: FixSite[] = [];
Expand Down Expand Up @@ -902,6 +938,21 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
warning: true,
detail: `Agent ${describeEstimate(estimate)} (${estimate.setting === 'maxCost' ? 'maxCost in uptide.yml' : '--max-cost'})`,
});
// The agent's own run_tests, where a test script runs more than its runner's related tests
// (`pnpm run build && pnpm run test:unit`): those tests alone. Its gate, and the final
// verification after it, run what they always ran. Elsewhere the check is the gate's run,
// whose result the gate reuses on the same tree.
const testOptions = { withServices: options.withServices === true, related: true };
const commands = (o: TestOptions) =>
planTests(root, workspaces, affected, o)
.map((p) => p.command)
.join('\n');
const narrowCheck = commands({ ...testOptions, agentCheck: true }) !== commands(testOptions);
const agentCheckTests = (files: string[]) =>
services.tests(root, workspaces, options.testTimeoutMs, files, {
...testOptions,
agentCheck: true,
});
const llm = await progress(options.onProgress, { phase: 'assist', package: pack.name }, () =>
sdkAgent
? assistWithSdk({
Expand All @@ -919,6 +970,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
withServices: options.withServices === true,
related: true,
}),
...(narrowCheck ? { checkTests: agentCheckTests } : {}),
...(services.lint
? {
lint: (files: string[], b?: LintResult[]) =>
Expand Down Expand Up @@ -1160,6 +1212,7 @@ async function fixPackage(options: FixOptions, services: FixServices): Promise<F
const tests = markPreexisting(ran, baselineTests);
// An agent-led draft must still install from scratch, as the repository's CI would (UPT-102).
const cleanInstall = fallback.length ? await cleanInstallCheck(root, services) : undefined;
if (cleanInstall) lap('clean-install');
const pending = git(root, 'status', '--porcelain', '--untracked-files=all');
const newErrors = newDiagnostics(baseline, after, gitLineMoves(root, baseHead));
const behavior = pack.name === 'zod' ? behaviorCheck(root, originals, sites) : undefined;
Expand Down
Loading
Loading