fix(coding-agent): stop updating packages on startup closes #1963
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- Changed package startup behavior so installed unpinned packages are no longer checked or updated during startup. Use `pi update` to apply npm/git package updates, while interactive mode now checks for available package updates in the background and notifies you when updates are available ([#1963](https://github.com/badlogic/pi-mono/issues/1963))
|
||||
|
||||
## [0.59.0] - 2026-03-17
|
||||
|
||||
### New Features
|
||||
|
||||
@@ -10,6 +10,7 @@ import { type GitSource, parseGitUrl } from "../utils/git.js";
|
||||
import type { PackageSource, SettingsManager } from "./settings-manager.js";
|
||||
|
||||
const NETWORK_TIMEOUT_MS = 10000;
|
||||
const UPDATE_CHECK_CONCURRENCY = 4;
|
||||
|
||||
function isOfflineModeEnabled(): boolean {
|
||||
const value = process.env.PI_OFFLINE;
|
||||
@@ -48,6 +49,13 @@ export interface ProgressEvent {
|
||||
|
||||
export type ProgressCallback = (event: ProgressEvent) => void;
|
||||
|
||||
export interface PackageUpdate {
|
||||
source: string;
|
||||
displayName: string;
|
||||
type: "npm" | "git";
|
||||
scope: Exclude<SourceScope, "temporary">;
|
||||
}
|
||||
|
||||
export interface PackageManager {
|
||||
resolve(onMissing?: (source: string) => Promise<MissingSourceAction>): Promise<ResolvedPaths>;
|
||||
install(source: string, options?: { local?: boolean }): Promise<void>;
|
||||
@@ -886,6 +894,71 @@ export class DefaultPackageManager implements PackageManager {
|
||||
}
|
||||
}
|
||||
|
||||
async checkForAvailableUpdates(): Promise<PackageUpdate[]> {
|
||||
if (isOfflineModeEnabled()) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const globalSettings = this.settingsManager.getGlobalSettings();
|
||||
const projectSettings = this.settingsManager.getProjectSettings();
|
||||
const allPackages: Array<{ pkg: PackageSource; scope: SourceScope }> = [];
|
||||
for (const pkg of projectSettings.packages ?? []) {
|
||||
allPackages.push({ pkg, scope: "project" });
|
||||
}
|
||||
for (const pkg of globalSettings.packages ?? []) {
|
||||
allPackages.push({ pkg, scope: "user" });
|
||||
}
|
||||
|
||||
const packageSources = this.dedupePackages(allPackages);
|
||||
const checks = packageSources
|
||||
.filter(
|
||||
(entry): entry is { pkg: PackageSource; scope: Exclude<SourceScope, "temporary"> } =>
|
||||
entry.scope !== "temporary",
|
||||
)
|
||||
.map((entry) => async (): Promise<PackageUpdate | undefined> => {
|
||||
const source = typeof entry.pkg === "string" ? entry.pkg : entry.pkg.source;
|
||||
const parsed = this.parseSource(source);
|
||||
if (parsed.type === "local" || parsed.pinned) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (parsed.type === "npm") {
|
||||
const installedPath = this.getNpmInstallPath(parsed, entry.scope);
|
||||
if (!existsSync(installedPath)) {
|
||||
return undefined;
|
||||
}
|
||||
const hasUpdate = await this.npmHasAvailableUpdate(parsed, installedPath);
|
||||
if (!hasUpdate) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
source,
|
||||
displayName: parsed.name,
|
||||
type: "npm",
|
||||
scope: entry.scope,
|
||||
};
|
||||
}
|
||||
|
||||
const installedPath = this.getGitInstallPath(parsed, entry.scope);
|
||||
if (!existsSync(installedPath)) {
|
||||
return undefined;
|
||||
}
|
||||
const hasUpdate = await this.gitHasAvailableUpdate(installedPath);
|
||||
if (!hasUpdate) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
source,
|
||||
displayName: `${parsed.host}/${parsed.path}`,
|
||||
type: "git",
|
||||
scope: entry.scope,
|
||||
};
|
||||
});
|
||||
|
||||
const results = await this.runWithConcurrency(checks, UPDATE_CHECK_CONCURRENCY);
|
||||
return results.filter((result): result is PackageUpdate => result !== undefined);
|
||||
}
|
||||
|
||||
private async resolvePackageSources(
|
||||
sources: Array<{ pkg: PackageSource; scope: SourceScope }>,
|
||||
accumulator: ResourceAccumulator,
|
||||
@@ -920,7 +993,9 @@ export class DefaultPackageManager implements PackageManager {
|
||||
|
||||
if (parsed.type === "npm") {
|
||||
const installedPath = this.getNpmInstallPath(parsed, scope);
|
||||
const needsInstall = !existsSync(installedPath) || (await this.npmNeedsUpdate(parsed, installedPath));
|
||||
const needsInstall =
|
||||
!existsSync(installedPath) ||
|
||||
(parsed.pinned && !(await this.installedNpmMatchesPinnedVersion(parsed, installedPath)));
|
||||
if (needsInstall) {
|
||||
const installed = await installMissing();
|
||||
if (!installed) continue;
|
||||
@@ -1063,31 +1138,34 @@ export class DefaultPackageManager implements PackageManager {
|
||||
return { type: "local", path: source };
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if an npm package needs to be updated.
|
||||
* - For unpinned packages: check if registry has a newer version
|
||||
* - For pinned packages: check if installed version matches the pinned version
|
||||
*/
|
||||
private async npmNeedsUpdate(source: NpmSource, installedPath: string): Promise<boolean> {
|
||||
private async installedNpmMatchesPinnedVersion(source: NpmSource, installedPath: string): Promise<boolean> {
|
||||
const installedVersion = this.getInstalledNpmVersion(installedPath);
|
||||
if (!installedVersion) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const { version: pinnedVersion } = this.parseNpmSpec(source.spec);
|
||||
if (!pinnedVersion) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return installedVersion === pinnedVersion;
|
||||
}
|
||||
|
||||
private async npmHasAvailableUpdate(source: NpmSource, installedPath: string): Promise<boolean> {
|
||||
if (isOfflineModeEnabled()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const installedVersion = this.getInstalledNpmVersion(installedPath);
|
||||
if (!installedVersion) return true;
|
||||
|
||||
const { version: pinnedVersion } = this.parseNpmSpec(source.spec);
|
||||
if (pinnedVersion) {
|
||||
// Pinned: check if installed matches pinned (exact match for now)
|
||||
return installedVersion !== pinnedVersion;
|
||||
if (!installedVersion) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Unpinned: check registry for latest version
|
||||
try {
|
||||
const latestVersion = await this.getLatestNpmVersion(source.name);
|
||||
return latestVersion !== installedVersion;
|
||||
} catch {
|
||||
// If we can't check registry, assume it's fine
|
||||
return false;
|
||||
}
|
||||
}
|
||||
@@ -1113,6 +1191,92 @@ export class DefaultPackageManager implements PackageManager {
|
||||
return data.version;
|
||||
}
|
||||
|
||||
private async gitHasAvailableUpdate(installedPath: string): Promise<boolean> {
|
||||
if (isOfflineModeEnabled()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
const localHead = await this.runCommandCapture("git", ["rev-parse", "HEAD"], {
|
||||
cwd: installedPath,
|
||||
timeoutMs: NETWORK_TIMEOUT_MS,
|
||||
});
|
||||
const remoteHead = await this.getRemoteGitHead(installedPath);
|
||||
return localHead.trim() !== remoteHead.trim();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private async getRemoteGitHead(installedPath: string): Promise<string> {
|
||||
const upstreamRef = await this.getGitUpstreamRef(installedPath);
|
||||
if (upstreamRef) {
|
||||
const remoteHead = await this.runGitRemoteCommand(installedPath, ["ls-remote", "origin", upstreamRef]);
|
||||
const match = remoteHead.match(/^([0-9a-f]{40})\s+/m);
|
||||
if (match?.[1]) {
|
||||
return match[1];
|
||||
}
|
||||
}
|
||||
|
||||
const remoteHead = await this.runGitRemoteCommand(installedPath, ["ls-remote", "origin", "HEAD"]);
|
||||
const match = remoteHead.match(/^([0-9a-f]{40})\s+HEAD$/m);
|
||||
if (!match?.[1]) {
|
||||
throw new Error("Failed to determine remote HEAD");
|
||||
}
|
||||
return match[1];
|
||||
}
|
||||
|
||||
private async getGitUpstreamRef(installedPath: string): Promise<string | undefined> {
|
||||
try {
|
||||
const upstream = await this.runCommandCapture("git", ["rev-parse", "--abbrev-ref", "@{upstream}"], {
|
||||
cwd: installedPath,
|
||||
timeoutMs: NETWORK_TIMEOUT_MS,
|
||||
});
|
||||
const trimmed = upstream.trim();
|
||||
if (!trimmed.startsWith("origin/")) {
|
||||
return undefined;
|
||||
}
|
||||
const branch = trimmed.slice("origin/".length);
|
||||
return branch ? `refs/heads/${branch}` : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
private runGitRemoteCommand(installedPath: string, args: string[]): Promise<string> {
|
||||
return this.runCommandCapture("git", args, {
|
||||
cwd: installedPath,
|
||||
timeoutMs: NETWORK_TIMEOUT_MS,
|
||||
env: {
|
||||
GIT_TERMINAL_PROMPT: "0",
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
private async runWithConcurrency<T>(tasks: Array<() => Promise<T>>, limit: number): Promise<T[]> {
|
||||
if (tasks.length === 0) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const results: T[] = new Array(tasks.length);
|
||||
let nextIndex = 0;
|
||||
const workerCount = Math.max(1, Math.min(limit, tasks.length));
|
||||
|
||||
const worker = async () => {
|
||||
while (true) {
|
||||
const index = nextIndex;
|
||||
nextIndex += 1;
|
||||
if (index >= tasks.length) {
|
||||
return;
|
||||
}
|
||||
results[index] = await tasks[index]();
|
||||
}
|
||||
};
|
||||
|
||||
await Promise.all(Array.from({ length: workerCount }, () => worker()));
|
||||
return results;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a unique identity for a package, ignoring version/ref.
|
||||
* Used to detect when the same package is in both global and project settings.
|
||||
@@ -1803,6 +1967,54 @@ export class DefaultPackageManager implements PackageManager {
|
||||
};
|
||||
}
|
||||
|
||||
private runCommandCapture(
|
||||
command: string,
|
||||
args: string[],
|
||||
options?: { cwd?: string; timeoutMs?: number; env?: Record<string, string> },
|
||||
): Promise<string> {
|
||||
return new Promise((resolvePromise, reject) => {
|
||||
const child = spawn(command, args, {
|
||||
cwd: options?.cwd,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
shell: process.platform === "win32",
|
||||
env: options?.env ? { ...process.env, ...options.env } : process.env,
|
||||
});
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
let timedOut = false;
|
||||
const timeout =
|
||||
typeof options?.timeoutMs === "number"
|
||||
? setTimeout(() => {
|
||||
timedOut = true;
|
||||
child.kill();
|
||||
}, options.timeoutMs)
|
||||
: undefined;
|
||||
|
||||
child.stdout?.on("data", (data) => {
|
||||
stdout += data.toString();
|
||||
});
|
||||
child.stderr?.on("data", (data) => {
|
||||
stderr += data.toString();
|
||||
});
|
||||
child.on("error", (error) => {
|
||||
if (timeout) clearTimeout(timeout);
|
||||
reject(error);
|
||||
});
|
||||
child.on("exit", (code) => {
|
||||
if (timeout) clearTimeout(timeout);
|
||||
if (timedOut) {
|
||||
reject(new Error(`${command} ${args.join(" ")} timed out after ${options?.timeoutMs}ms`));
|
||||
return;
|
||||
}
|
||||
if (code === 0) {
|
||||
resolvePromise(stdout.trim());
|
||||
return;
|
||||
}
|
||||
reject(new Error(`${command} ${args.join(" ")} failed with code ${code}: ${stderr || stdout}`));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
private runCommand(command: string, args: string[], options?: { cwd?: string }): Promise<void> {
|
||||
return new Promise((resolvePromise, reject) => {
|
||||
const child = spawn(command, args, {
|
||||
|
||||
@@ -38,6 +38,7 @@ import {
|
||||
import { spawn, spawnSync } from "child_process";
|
||||
import {
|
||||
APP_NAME,
|
||||
getAgentDir,
|
||||
getAuthPath,
|
||||
getDebugLogPath,
|
||||
getShareViewerUrl,
|
||||
@@ -57,6 +58,7 @@ import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/
|
||||
import { type AppAction, KeybindingsManager } from "../../core/keybindings.js";
|
||||
import { createCompactionSummaryMessage } from "../../core/messages.js";
|
||||
import { findExactModelReferenceMatch, resolveModelScope } from "../../core/model-resolver.js";
|
||||
import { DefaultPackageManager } from "../../core/package-manager.js";
|
||||
import type { ResourceDiagnostic } from "../../core/resource-loader.js";
|
||||
import { type SessionContext, SessionManager } from "../../core/session-manager.js";
|
||||
import { BUILTIN_SLASH_COMMANDS } from "../../core/slash-commands.js";
|
||||
@@ -512,6 +514,13 @@ export class InteractiveMode {
|
||||
}
|
||||
});
|
||||
|
||||
// Start package update check asynchronously
|
||||
this.checkForPackageUpdates().then((updates) => {
|
||||
if (updates.length > 0) {
|
||||
this.showPackageUpdateNotification(updates);
|
||||
}
|
||||
});
|
||||
|
||||
// Check tmux keyboard setup asynchronously
|
||||
this.checkTmuxKeyboardSetup().then((warning) => {
|
||||
if (warning) {
|
||||
@@ -593,6 +602,24 @@ export class InteractiveMode {
|
||||
}
|
||||
}
|
||||
|
||||
private async checkForPackageUpdates(): Promise<string[]> {
|
||||
if (process.env.PI_OFFLINE) {
|
||||
return [];
|
||||
}
|
||||
|
||||
try {
|
||||
const packageManager = new DefaultPackageManager({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
settingsManager: this.settingsManager,
|
||||
});
|
||||
const updates = await packageManager.checkForAvailableUpdates();
|
||||
return updates.map((update) => update.displayName);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
private async checkTmuxKeyboardSetup(): Promise<string | undefined> {
|
||||
if (!process.env.TMUX) return undefined;
|
||||
|
||||
@@ -2887,6 +2914,24 @@ export class InteractiveMode {
|
||||
this.ui.requestRender();
|
||||
}
|
||||
|
||||
showPackageUpdateNotification(packages: string[]): void {
|
||||
const action = theme.fg("accent", `${APP_NAME} update`);
|
||||
const updateInstruction = theme.fg("muted", "Package updates are available. Run ") + action;
|
||||
const packageLines = packages.map((pkg) => `- ${pkg}`).join("\n");
|
||||
|
||||
this.chatContainer.addChild(new Spacer(1));
|
||||
this.chatContainer.addChild(new DynamicBorder((text) => theme.fg("warning", text)));
|
||||
this.chatContainer.addChild(
|
||||
new Text(
|
||||
`${theme.bold(theme.fg("warning", "Package Updates Available"))}\n${updateInstruction}\n${theme.fg("muted", "Packages:")}\n${packageLines}`,
|
||||
1,
|
||||
0,
|
||||
),
|
||||
);
|
||||
this.chatContainer.addChild(new DynamicBorder((text) => theme.fg("warning", text)));
|
||||
this.ui.requestRender();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all queued messages (read-only).
|
||||
* Combines session queue and compaction queue.
|
||||
|
||||
@@ -1321,23 +1321,84 @@ export default function(api) { api.registerTool({ name: "test", description: "te
|
||||
expect(refreshTemporaryGitSourceSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should not call fetch in npmNeedsUpdate when offline", async () => {
|
||||
process.env.PI_OFFLINE = "1";
|
||||
const installedPath = join(tempDir, "installed-package");
|
||||
mkdirSync(installedPath, { recursive: true });
|
||||
writeFileSync(join(installedPath, "package.json"), JSON.stringify({ version: "1.0.0" }));
|
||||
it("should not fetch npm registry during resolve for installed unpinned packages", async () => {
|
||||
const installedPath = join(tempDir, ".pi", "npm", "node_modules", "example");
|
||||
mkdirSync(join(installedPath, "extensions"), { recursive: true });
|
||||
writeFileSync(join(installedPath, "package.json"), JSON.stringify({ name: "example", version: "1.0.0" }));
|
||||
writeFileSync(join(installedPath, "extensions", "index.ts"), "export default function() {};");
|
||||
settingsManager.setProjectPackages(["npm:example"]);
|
||||
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
|
||||
const needsUpdate = await (packageManager as any).npmNeedsUpdate(
|
||||
{ type: "npm", spec: "example", name: "example", pinned: false },
|
||||
installedPath,
|
||||
);
|
||||
|
||||
expect(needsUpdate).toBe(false);
|
||||
const result = await packageManager.resolve();
|
||||
expect(result.extensions.some((r) => pathEndsWith(r.path, "extensions/index.ts") && r.enabled)).toBe(true);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should reinstall pinned npm packages when installed version does not match", async () => {
|
||||
const installedPath = join(tempDir, ".pi", "npm", "node_modules", "example");
|
||||
mkdirSync(installedPath, { recursive: true });
|
||||
writeFileSync(join(installedPath, "package.json"), JSON.stringify({ name: "example", version: "1.0.0" }));
|
||||
settingsManager.setProjectPackages(["npm:example@2.0.0"]);
|
||||
|
||||
const installParsedSourceSpy = vi
|
||||
.spyOn(packageManager as any, "installParsedSource")
|
||||
.mockResolvedValue(undefined);
|
||||
|
||||
await packageManager.resolve();
|
||||
expect(installParsedSourceSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should not check package updates when offline", async () => {
|
||||
process.env.PI_OFFLINE = "1";
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
|
||||
const updates = await packageManager.checkForAvailableUpdates();
|
||||
expect(updates).toEqual([]);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should report updates for installed unpinned npm packages", async () => {
|
||||
const installedPath = join(tempDir, ".pi", "npm", "node_modules", "example");
|
||||
mkdirSync(installedPath, { recursive: true });
|
||||
writeFileSync(join(installedPath, "package.json"), JSON.stringify({ name: "example", version: "1.0.0" }));
|
||||
settingsManager.setProjectPackages(["npm:example"]);
|
||||
|
||||
const fetchMock = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ version: "1.2.3" }),
|
||||
});
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const updates = await packageManager.checkForAvailableUpdates();
|
||||
expect(updates).toEqual([
|
||||
{
|
||||
source: "npm:example",
|
||||
displayName: "example",
|
||||
type: "npm",
|
||||
scope: "project",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("should skip pinned packages when checking for updates", async () => {
|
||||
const installedNpmPath = join(tempDir, ".pi", "npm", "node_modules", "example");
|
||||
mkdirSync(installedNpmPath, { recursive: true });
|
||||
writeFileSync(join(installedNpmPath, "package.json"), JSON.stringify({ name: "example", version: "1.0.0" }));
|
||||
const parsedGitSource = (packageManager as any).parseSource("git:github.com/example/repo@v1");
|
||||
const installedGitPath = (packageManager as any).getGitInstallPath(parsedGitSource, "project") as string;
|
||||
mkdirSync(installedGitPath, { recursive: true });
|
||||
settingsManager.setProjectPackages(["npm:example@1.0.0", "git:github.com/example/repo@v1"]);
|
||||
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
const gitUpdateSpy = vi.spyOn(packageManager as any, "gitHasAvailableUpdate");
|
||||
|
||||
const updates = await packageManager.checkForAvailableUpdates();
|
||||
expect(updates).toEqual([]);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(gitUpdateSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should pass an AbortSignal timeout when fetching npm latest version", async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
|
||||
Reference in New Issue
Block a user