From 806cdd68e312f8cb80efe367c661bea0bd3ee537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Hou=C5=A1ka?= Date: Mon, 2 Nov 2026 21:49:07 +0100 Subject: [PATCH] fix(coding-agent): only report packages as up to date when pi update checked them (#11711) ## Problem #11707 prints `Packages are already up to date` whenever no package changed. In two cases that claim is false. **1. npm left the old version installed.** Extension updates install `@latest` or the configured range. If npm resolves stale registry metadata, it reinstalls the old version and exits 0. The existing check warns, then the summary contradicts it: ``` Updating npm:foo... npm:foo is still 1.2.0 (registry reports 1.3.0); npm registry metadata may be stale, try again in a few minutes Packages are already up to date ``` **2. Offline mode.** With `--offline` or `PI_OFFLINE=1`, `updateConfiguredSources()` returns before checking anything, and the CLI still prints `Packages are already up to date`. ## Change The CLI only prints "already up to date" when the update ran and printed no warning: | Situation | Output | |---|---| | Packages changed | `Updated N packages:` and the list (unchanged) | | Nothing changed, no warnings | `Packages are already up to date` (unchanged) | | Nothing changed, a warning was printed | `No packages were updated, see the warnings above` (or ` was not updated, ...`) | | Offline mode | `Skipped package updates (offline mode)` | The CLI counts `progress` update events, which are always warnings, and does not try to map them to packages. That is deliberate: the git warning about untracked files that could not be cleaned is printed even though the commit did change, so a warning does not mean "this package was not updated". In offline mode, `update()` still runs, so `pi update ` still fails with "No matching package". `isOfflineModeEnabled()` is now exported from `core/package-manager.ts` (not from the package index) so the CLI uses the same check. The changelog entry from #11707 is adjusted, since it is unreleased. ## Tests `test/package-command-paths.test.ts`, both through `main()`: - npm `view` reports 1.3.0 but `install` leaves 1.2.0: the stale-metadata warning is printed, then `No packages were updated, see the warnings above`, and no "up to date". - `PI_OFFLINE=1`: `Skipped package updates (offline mode)`, no "up to date". Both fail without the fix. `cycles project package overrides in config local mode`, `prints a pnpm metadata hint when self-update fails` and `should expand ~ in exclusion and exact patterns` fail locally on Windows, on `origin/main` as well. --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/core/package-manager.ts | 3 +- .../coding-agent/src/package-manager-cli.ts | 18 ++++++- .../test/package-command-paths.test.ts | 48 +++++++++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 839fe6ae4..c987db53b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- `pi update` now lists each updated package with its npm version or git commit change, for example `npm:foo 1.2.0 -> 1.3.0`, and prints `Packages are already up to date` when nothing changed. `PackageManager.update()` reports each changed package as a new `result` progress event whose `message` holds that line ([#11707](https://github.com/earendil-works/pi/pull/11707)) +- `pi update` now lists each updated package with its npm version or git commit change, for example `npm:foo 1.2.0 -> 1.3.0`, and prints `Packages are already up to date` when nothing changed. When nothing changed but an update printed a warning, for example because npm left the old version installed after resolving stale registry metadata, it prints `No packages were updated, see the warnings above` instead; in offline mode it prints `Skipped package updates (offline mode)`. `PackageManager.update()` reports each changed package as a new `result` progress event whose `message` holds that line ([#11707](https://github.com/earendil-works/pi/pull/11707), [#11711](https://github.com/earendil-works/pi/pull/11711)) ## [1.5.0] - 2026-11-02 diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index b83381a9e..5fd93754e 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -60,7 +60,8 @@ const NETWORK_TIMEOUT_MS = 10000; const UPDATE_CHECK_CONCURRENCY = 4; const GIT_UPDATE_CONCURRENCY = 4; -function isOfflineModeEnabled(): boolean { +/** Whether PI_OFFLINE is set; package installs and updates then skip all network access. */ +export function isOfflineModeEnabled(): boolean { const value = process.env.PI_OFFLINE; if (!value) return false; return value === "1" || value.toLowerCase() === "true" || value.toLowerCase() === "yes"; diff --git a/packages/coding-agent/src/package-manager-cli.ts b/packages/coding-agent/src/package-manager-cli.ts index 911d0c84f..93a8fd4f8 100644 --- a/packages/coding-agent/src/package-manager-cli.ts +++ b/packages/coding-agent/src/package-manager-cli.ts @@ -32,7 +32,7 @@ import { } from "./config.ts"; import type { InlineExtension } from "./core/extensions/types.ts"; import { ModelRuntime } from "./core/model-runtime.ts"; -import { DefaultPackageManager } from "./core/package-manager.ts"; +import { DefaultPackageManager, isOfflineModeEnabled } from "./core/package-manager.ts"; import { type AppMode, resolveProjectTrusted } from "./core/project-trust.ts"; import { DefaultResourceLoader, formatPackageInstallWarning, isBuiltinExtension } from "./core/resource-loader.ts"; import { SettingsManager } from "./core/settings-manager.ts"; @@ -1029,12 +1029,15 @@ export async function handlePackageCommand( // Update result events describe a changed package, e.g. "npm:foo 1.2.0 -> 1.3.0". const updatedPackages: string[] = []; + // Progress messages are warnings, e.g. npm reinstalled the old version from stale registry metadata. + let warningCount = 0; packageManager.setProgressCallback((event) => { if (event.type === "result" && event.action === "update" && event.message) { updatedPackages.push(event.message); } else if (event.type === "start") { process.stdout.write(chalk.dim(`${event.message}\n`)); } else if (event.type === "progress" && event.message) { + warningCount++; process.stdout.write(chalk.yellow(`${event.message}\n`)); } }); @@ -1103,7 +1106,18 @@ export async function handlePackageCommand( if (updateTargetIncludesExtensions(target)) { const updateSource = target.type === "extensions" ? target.source : undefined; await packageManager.update(updateSource); - if (updatedPackages.length === 0) { + // Only claim "up to date" when the update actually checked and nothing warned. + if (isOfflineModeEnabled()) { + console.log(chalk.yellow("Skipped package updates (offline mode)")); + } else if (updatedPackages.length === 0 && warningCount > 0) { + console.log( + chalk.yellow( + updateSource + ? `${updateSource} was not updated, see the warnings above` + : "No packages were updated, see the warnings above", + ), + ); + } else if (updatedPackages.length === 0) { console.log( chalk.green( updateSource ? `${updateSource} is already up to date` : "Packages are already up to date", diff --git a/packages/coding-agent/test/package-command-paths.test.ts b/packages/coding-agent/test/package-command-paths.test.ts index a12acd856..711cf3f96 100644 --- a/packages/coding-agent/test/package-command-paths.test.ts +++ b/packages/coding-agent/test/package-command-paths.test.ts @@ -369,6 +369,54 @@ if (process.platform !== "win32") fs.chmodSync(piPath, 0o755); } }); + /** Install `npm:fake-package` at `installedVersion` with an npm whose registry reports `registryVersion`. */ + function prepareStaleNpmPackage(installedVersion: string, registryVersion: string): void { + const installedPath = join(agentDir, "npm", "node_modules", "fake-package"); + mkdirSync(installedPath, { recursive: true }); + writeFileSync( + join(installedPath, "package.json"), + JSON.stringify({ name: "fake-package", version: installedVersion }), + ); + // `npm view` reports the newer version, but `npm install` succeeds without changing anything. + const fakeNpmPath = join(tempDir, "fake-stale-npm.cjs"); + writeFileSync( + fakeNpmPath, + `if (process.argv.includes("view")) console.log(${JSON.stringify(JSON.stringify(registryVersion))});`, + ); + writeFileSync( + join(agentDir, "settings.json"), + JSON.stringify({ packages: ["npm:fake-package"], npmCommand: [originalExecPath, fakeNpmPath] }), + ); + } + + it("does not report packages as up to date when npm left an older version installed", async () => { + prepareStaleNpmPackage("1.2.0", "1.3.0"); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + + await expect(main(["update", "--extensions"])).resolves.toBeUndefined(); + + const progress = stdoutSpy.mock.calls.map(([chunk]) => String(chunk)).join(""); + const stdout = logSpy.mock.calls.map(([message]) => String(message)).join("\n"); + expect(progress).toContain("npm:fake-package is still 1.2.0 (registry reports 1.3.0)"); + expect(stdout).toContain("No packages were updated, see the warnings above"); + expect(stdout).not.toContain("up to date"); + }); + + it("does not report packages as up to date in offline mode", async () => { + prepareStaleNpmPackage("1.2.0", "1.3.0"); + // main() writes both variables in offline mode; stubbing them lets afterEach restore them. + vi.stubEnv("PI_OFFLINE", "1"); + vi.stubEnv("PI_SKIP_VERSION_CHECK", "1"); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(main(["update", "--extensions"])).resolves.toBeUndefined(); + + const stdout = logSpy.mock.calls.map(([message]) => String(message)).join("\n"); + expect(stdout).toContain("Skipped package updates (offline mode)"); + expect(stdout).not.toContain("up to date"); + }); + it("uses saved project trust during update", async () => { mkdirSync(join(projectDir, ".pi"), { recursive: true }); const fakeNpmPath = join(tempDir, "fake-trusted-project-npm.cjs");