diff --git a/docs/implementation/update-check.md b/docs/implementation/update-check.md index 011487ae..ee696db4 100644 --- a/docs/implementation/update-check.md +++ b/docs/implementation/update-check.md @@ -3,9 +3,8 @@ ## Purpose The CLI warns users when their installed `githits` package is behind the npm -`latest` dist-tag. This supports sticky installs such as `npm i -g githits` -without introducing forced updates, backend policy, or automatic package-manager -execution. +`latest` dist-tag. It also uses npm deprecation metadata as a blunt kill switch +for versions that become backend-incompatible or unsafe. ## Background @@ -19,6 +18,23 @@ That path asks the package manager for the latest published version at startup. The update checker targets long-lived local/global installs where the binary stays on the installed version until the user updates it. +## Support Policy + +GitHits supports the current and recent CLI versions during normal operation; +the target support window is about seven days. Older versions may continue to +work, but are not guaranteed. + +When a version must stop running, maintainers deprecate it on npm with a clear +reason: + +```sh +npm deprecate 'githits@<0.3.0' 'Backend protocol changed' +``` + +The CLI treats npm deprecation as runtime policy once observed. npm or network +failure does not break commands unless the installed version is already cached as +deprecated. + ## Behavior The check is advisory only: @@ -30,6 +46,17 @@ The check is advisory only: - every eligible invocation reports the stale version once a newer npm `latest` is known +Required-update enforcement is separate: + +- background refresh records whether the installed version is npm-deprecated +- the current command is not blocked by newly fetched deprecation metadata +- the next eligible invocation blocks from cached metadata only +- help/version and ephemeral package-runner invocations are not blocked +- CI, non-TTY, and MCP stdio invocations can be blocked because compatibility + matters there too +- successful non-deprecated metadata clears a cached block +- fetch failures preserve any cached deprecated status and otherwise fail open + The notice format is: ```text @@ -37,6 +64,21 @@ Update available: githits 0.2.0 -> 0.3.0 Run: npm i -g githits@latest ``` +The required-update format is: + +```text +Update required: Backend protocol changed + +Installed githits 0.2.0 is no longer supported. +Latest known version: 0.3.0 +Update with: + npm i -g githits@latest +``` + +The `Latest known version` line is omitted when the advisory latest-version cache +is missing. Required-update enforcement does not fetch npm just to render this +line. + ## Registry Source The checker fetches npm dist-tags directly: @@ -45,9 +87,17 @@ The checker fetches npm dist-tags directly: https://registry.npmjs.org/-/package/githits/dist-tags ``` -Only the `latest` field is used. The CLI does not shell out to `npm info`, -because subprocess behavior depends on the user's package-manager setup and is -slower than one HTTPS request. +Only the `latest` field is used for advisory update notices. Required-update +refresh uses the installed package-version metadata endpoint: + +```text +https://registry.npmjs.org/githits/ +``` + +The optional `deprecated` string is sanitized, cached, and later displayed as the +required-update reason. The CLI does not shell out to `npm info`, because +subprocess behavior depends on the user's package-manager setup and is slower +than direct HTTPS requests. ## Cache @@ -71,7 +121,12 @@ Cache shape: ```json { "checkedAt": "2026-04-28T12:00:00.000Z", - "latestVersion": "0.3.0" + "latestVersion": "0.3.0", + "currentVersionStatus": { + "version": "0.2.0", + "checkedAt": "2026-04-28T12:00:00.000Z", + "deprecatedReason": "Backend protocol changed" + } } ``` @@ -79,8 +134,8 @@ The CLI checks npm at most once every 24 hours. If the cached latest version is newer than the running CLI, the notice is printed on every eligible invocation. When the cache is stale, the CLI refreshes npm first and falls back to the cached notice if the refresh fails. A missing or malformed cache is treated as stale. -Concurrent writes use last-writer-wins semantics, and redundant fetches from -racing processes are acceptable. +Concurrent writes use atomic write-then-rename with last-writer-wins semantics, +and redundant fetches from racing processes are acceptable. ## Eligibility @@ -96,6 +151,11 @@ Checks are skipped for: - likely ephemeral package-runner invocations (`npx`, `bunx`, npm/bun `exec`) - MCP stdio server invocations +Required-update refresh/enforcement uses broader eligibility: it skips +help/version and ephemeral package runners, but not CI, non-TTY, disabled +advisory checks, or MCP stdio. This keeps compatibility enforcement effective +for automation and MCP clients while keeping advisory output conservative. + MCP has two forms: - `githits mcp start` always starts the stdio server and is skipped @@ -109,7 +169,7 @@ Key modules: | File | Purpose | |---|---| | `src/services/update-check-service.ts` | Registry fetch, cache handling, eligibility helpers, notice formatting | -| `src/cli/update-check.ts` | Cancellable background task and post-command stderr notice | +| `src/cli/update-check.ts` | Cancellable background tasks, post-command advisory notice, cached required-update enforcement | | `src/cli.ts` | Starts and flushes the update-check task around Commander parsing | | `src/services/update-check-service.test.ts` | Service and eligibility coverage | | `src/cli/update-check.test.ts` | CLI orchestration coverage | @@ -118,11 +178,22 @@ The service accepts injected dependencies for the current version, fetcher, clock, and file-system service. Tests should mock those dependencies rather than patch global state. +## Backend Compatibility Signals + +All backend requests already include `x-githits-client-version` and a +`User-Agent`, so backend telemetry can identify old clients. Until the backend +has a structured `CLIENT_UPDATE_REQUIRED` error, the CLI maps GraphQL schema +validation failures such as `Cannot query field` to an `UPDATE_REQUIRED` error +because they are strong evidence that the client query is incompatible with the +backend schema. + ## Future Work -This mechanism is not a hard compatibility gate. Future phases can add: +Future phases can add: - CDN-hosted version policy with `recommended` and `minimumSupported` - recurring checks for long-running MCP servers - `githits update` - backend `426 Upgrade Required` enforcement +- structured GraphQL `CLIENT_UPDATE_REQUIRED` extensions with minimum version + and upgrade instructions diff --git a/src/cli.ts b/src/cli.ts index ccecf369..f29d149c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -2,7 +2,9 @@ import { Command } from "commander"; import { version } from "../package.json"; import { + enforceCachedRequiredUpdateForInvocation, runWithUpdateCheckFlush, + startRequiredUpdateRefreshTaskForInvocation, startUpdateCheckTaskForInvocation, } from "./cli/update-check.js"; import { @@ -38,17 +40,32 @@ const commandSpans = new WeakMap< Command, ReturnType >(); +const createUpdateCheckService = () => + new NpmRegistryUpdateCheckService({ + currentVersion: version, + fileSystemService: new FileSystemServiceImpl(), + }); + +await enforceCachedRequiredUpdateForInvocation({ + args: argv, + env: process.env, + createService: createUpdateCheckService, + stderr: process.stderr, + exit: process.exit as (code: number) => never, +}); + const updateCheckTask = startUpdateCheckTaskForInvocation({ args: argv, env: process.env, stderrIsTTY: process.stderr.isTTY === true, stdinIsTTY: process.stdin.isTTY === true, stdoutIsTTY: process.stdout.isTTY === true, - createService: () => - new NpmRegistryUpdateCheckService({ - currentVersion: version, - fileSystemService: new FileSystemServiceImpl(), - }), + createService: createUpdateCheckService, +}); +const requiredUpdateRefreshTask = startRequiredUpdateRefreshTaskForInvocation({ + args: argv, + env: process.env, + createService: createUpdateCheckService, }); if (isTelemetryEnabled()) { @@ -138,7 +155,7 @@ try { await runWithUpdateCheckFlush( () => withTelemetrySpan("cli.parse", () => program.parseAsync()), updateCheckTask, - { stderr: process.stderr }, + { stderr: process.stderr, requiredUpdateRefreshTask }, ); } catch (error) { if (isUserFacingError(error)) { diff --git a/src/cli/update-check.test.ts b/src/cli/update-check.test.ts index 44dbb849..10e13526 100644 --- a/src/cli/update-check.test.ts +++ b/src/cli/update-check.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it, mock } from "bun:test"; import { createMockUpdateCheckService } from "../services/test-helpers.js"; import { + enforceCachedRequiredUpdateForInvocation, + flushRequiredUpdateRefresh, flushUpdateCheckNotice, runWithUpdateCheckFlush, + startRequiredUpdateRefreshTask, + startRequiredUpdateRefreshTaskForInvocation, startUpdateCheckTask, startUpdateCheckTaskForInvocation, } from "./update-check.js"; @@ -97,6 +101,125 @@ describe("update-check CLI orchestration", () => { expect(createService).not.toHaveBeenCalled(); }); + it("starts required update refresh task for MCP stdio invocations", () => { + const createService = mock(() => createMockUpdateCheckService()); + + const task = startRequiredUpdateRefreshTaskForInvocation({ + args: ["mcp", "start"], + env: {}, + createService, + }); + + expect(task).toBeDefined(); + expect(createService).toHaveBeenCalled(); + task?.abort(); + }); + + it("flushes required update refresh task without writing notices", async () => { + const service = createMockUpdateCheckService({ + refreshRequiredUpdateStatus: mock(() => Promise.resolve()), + }); + const task = startRequiredUpdateRefreshTask(service); + + await flushRequiredUpdateRefresh(task, { timeoutMs: 50 }); + + expect(service.refreshRequiredUpdateStatus).toHaveBeenCalledWith( + expect.any(AbortSignal), + ); + }); + + it("enforces cached required update with terminal output", async () => { + const service = createMockUpdateCheckService({ + getRequiredUpdateNotice: mock(() => + Promise.resolve({ + currentVersion: "0.2.0", + latestKnownVersion: "0.3.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }), + ), + }); + const stderr = { write: mock(() => undefined) }; + const exit = mock((code: number) => { + throw new Error(`exit ${code}`); + }) as (code: number) => never; + + await expect( + enforceCachedRequiredUpdateForInvocation({ + args: ["example", "query"], + env: {}, + createService: () => service, + stderr, + exit, + }), + ).rejects.toThrow("exit 1"); + + expect(stderr.write).toHaveBeenCalledWith( + "Update required: Backend protocol changed\n\nInstalled githits 0.2.0 is no longer supported.\nLatest known version: 0.3.0\nUpdate with:\n npm i -g githits@latest\n", + ); + }); + + it("enforces cached required update with JSON output", async () => { + const service = createMockUpdateCheckService({ + getRequiredUpdateNotice: mock(() => + Promise.resolve({ + currentVersion: "0.2.0", + latestKnownVersion: "0.3.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }), + ), + }); + const stderr = { write: mock(() => undefined) }; + const exit = mock((code: number) => { + throw new Error(`exit ${code}`); + }) as (code: number) => never; + + await expect( + enforceCachedRequiredUpdateForInvocation({ + args: ["pkg", "info", "npm:express", "--json"], + env: {}, + createService: () => service, + stderr, + exit, + }), + ).rejects.toThrow("exit 1"); + + const payload = JSON.parse( + (stderr.write as ReturnType).mock.calls[0]?.[0] as string, + ); + expect(payload).toEqual({ + error: "Update required: Backend protocol changed", + code: "UPDATE_REQUIRED", + retryable: false, + details: { + currentVersion: "0.2.0", + latestKnownVersion: "0.3.0", + updateCommand: "npm i -g githits@latest", + reason: "Backend protocol changed", + }, + }); + }); + + it("does not enforce cached required update for help", async () => { + const createService = mock(() => createMockUpdateCheckService()); + const stderr = { write: mock(() => undefined) }; + const exit = mock(() => { + throw new Error("exit"); + }) as (code: number) => never; + + await enforceCachedRequiredUpdateForInvocation({ + args: ["--help"], + env: {}, + createService, + stderr, + exit, + }); + + expect(createService).not.toHaveBeenCalled(); + expect(stderr.write).not.toHaveBeenCalled(); + }); + it("flushes update notices when the wrapped action throws", async () => { const service = createMockUpdateCheckService({ checkForUpdate: mock(() => diff --git a/src/cli/update-check.ts b/src/cli/update-check.ts index 4ea6cf27..22db366e 100644 --- a/src/cli/update-check.ts +++ b/src/cli/update-check.ts @@ -1,14 +1,25 @@ import type { + RequiredUpdateNotice, UpdateCheckNotice, UpdateCheckService, } from "../services/index.js"; -import { formatUpdateNotice, shouldRunUpdateCheck } from "../services/index.js"; +import { + formatRequiredUpdateNotice, + formatUpdateNotice, + shouldRunRequiredUpdateEnforcement, + shouldRunUpdateCheck, +} from "../services/index.js"; export interface UpdateCheckTask { promise: Promise; abort: () => void; } +export interface RequiredUpdateRefreshTask { + promise: Promise; + abort: () => void; +} + export interface UpdateCheckOutput { write(chunk: string): unknown; } @@ -23,6 +34,16 @@ export function startUpdateCheckTask( }; } +export function startRequiredUpdateRefreshTask( + service: UpdateCheckService, +): RequiredUpdateRefreshTask { + const controller = new AbortController(); + return { + promise: service.refreshRequiredUpdateStatus(controller.signal), + abort: () => controller.abort(), + }; +} + export function startUpdateCheckTaskForInvocation(options: { args: string[]; env: Record; @@ -46,21 +67,93 @@ export function startUpdateCheckTaskForInvocation(options: { return startUpdateCheckTask(options.createService()); } +export function startRequiredUpdateRefreshTaskForInvocation(options: { + args: string[]; + env: Record; + createService: () => UpdateCheckService; +}): RequiredUpdateRefreshTask | undefined { + if ( + !shouldRunRequiredUpdateEnforcement({ + args: options.args, + env: options.env, + }) + ) { + return undefined; + } + + return startRequiredUpdateRefreshTask(options.createService()); +} + +export async function enforceCachedRequiredUpdateForInvocation(options: { + args: string[]; + env: Record; + createService: () => UpdateCheckService; + stderr: UpdateCheckOutput; + exit: (code: number) => never; +}): Promise { + if ( + !shouldRunRequiredUpdateEnforcement({ + args: options.args, + env: options.env, + }) + ) { + return; + } + + const notice = await options.createService().getRequiredUpdateNotice(); + if (!notice) { + return; + } + + if (isJsonInvocation(options.args)) { + options.stderr.write(`${JSON.stringify(requiredUpdateEnvelope(notice))}\n`); + } else { + options.stderr.write(`${formatRequiredUpdateNotice(notice)}\n`); + } + options.exit(1); +} + export async function runWithUpdateCheckFlush( action: () => Promise, task: UpdateCheckTask | undefined, options: { stderr: UpdateCheckOutput; timeoutMs?: number; + requiredUpdateRefreshTask?: RequiredUpdateRefreshTask; }, ): Promise { try { return await action(); } finally { + await flushRequiredUpdateRefresh(options.requiredUpdateRefreshTask, { + timeoutMs: options.timeoutMs, + }); await flushUpdateCheckNotice(task, options); } } +export async function flushRequiredUpdateRefresh( + task: RequiredUpdateRefreshTask | undefined, + options: { timeoutMs?: number } = {}, +): Promise { + if (!task) { + return; + } + + const timeoutMs = options.timeoutMs ?? 50; + let timeout: ReturnType | undefined; + const timeoutPromise = new Promise<"timeout">((resolve) => { + timeout = setTimeout(() => resolve("timeout"), timeoutMs); + }); + const result = await Promise.race([task.promise, timeoutPromise]); + if (timeout) { + clearTimeout(timeout); + } + if (result === "timeout") { + task.abort(); + } +} + export async function flushUpdateCheckNotice( task: UpdateCheckTask | undefined, options: { @@ -94,3 +187,33 @@ export async function flushUpdateCheckNotice( options.stderr.write(`${formatUpdateNotice(result)}\n`); } + +function isJsonInvocation(args: string[]): boolean { + return args.includes("--json"); +} + +function requiredUpdateEnvelope(notice: RequiredUpdateNotice): { + error: string; + code: "UPDATE_REQUIRED"; + retryable: false; + details: { + currentVersion: string; + latestKnownVersion?: string; + updateCommand: string; + reason: string; + }; +} { + return { + error: `Update required: ${notice.reason}`, + code: "UPDATE_REQUIRED", + retryable: false, + details: { + currentVersion: notice.currentVersion, + ...(notice.latestKnownVersion + ? { latestKnownVersion: notice.latestKnownVersion } + : {}), + updateCommand: notice.updateCommand, + reason: notice.reason, + }, + }; +} diff --git a/src/commands/code/code-nav-cli-helpers.ts b/src/commands/code/code-nav-cli-helpers.ts index 900a18c6..38bf8f09 100644 --- a/src/commands/code/code-nav-cli-helpers.ts +++ b/src/commands/code/code-nav-cli-helpers.ts @@ -13,6 +13,7 @@ import type { CodeNavigationTarget, } from "../../services/index.js"; import { + formatMappedErrorForTerminal, type MappedError, mapCodeNavigationError, } from "../../shared/code-navigation-error-map.js"; @@ -122,6 +123,9 @@ export function parseIntCliOption( * share the same indexing-retry story. */ export function formatIndexingError(mapped: MappedError): string { + if (mapped.code === "UPDATE_REQUIRED") { + return formatMappedErrorForTerminal(mapped); + } if (mapped.code !== "INDEXING") return mapped.message; const detail = mapped.details ?? {}; const lines = [mapped.message]; @@ -148,6 +152,9 @@ export function formatIndexingError(mapped: MappedError): string { * class of failure. */ export function formatFileErrorWithFilesHint(mapped: MappedError): string { + if (mapped.code === "UPDATE_REQUIRED") { + return formatMappedErrorForTerminal(mapped); + } if (mapped.code === "FILE_NOT_FOUND") { return `${mapped.message}\n Use \`code files\` to list available paths.`; } diff --git a/src/commands/docs/list.ts b/src/commands/docs/list.ts index 8fb5c71f..ee5a57fc 100644 --- a/src/commands/docs/list.ts +++ b/src/commands/docs/list.ts @@ -6,6 +6,7 @@ import { buildListPackageDocsParams, buildListPackageDocsSuccessPayload, formatListPackageDocsTerminal, + formatMappedErrorForTerminal, InvalidPackageSpecError, mapPackageIntelligenceError, parsePackageSpec, @@ -99,7 +100,7 @@ function handleDocsListError(error: unknown, json: boolean): never { }), ); } else { - console.error(mapped.message); + console.error(formatMappedErrorForTerminal(mapped)); } process.exit(1); diff --git a/src/commands/docs/read.ts b/src/commands/docs/read.ts index 30d24aec..dd1fe878 100644 --- a/src/commands/docs/read.ts +++ b/src/commands/docs/read.ts @@ -4,6 +4,7 @@ import type { PackageIntelligenceService } from "../../services/index.js"; import { buildReadPackageDocParams, buildReadPackageDocSuccessPayload, + formatMappedErrorForTerminal, formatReadPackageDocTerminal, InvalidPackageSpecError, mapPackageIntelligenceError, @@ -80,7 +81,7 @@ function handleDocsReadError(error: unknown, json: boolean): never { }), ); } else { - console.error(mapped.message); + console.error(formatMappedErrorForTerminal(mapped)); } process.exit(1); diff --git a/src/commands/pkg/changelog.ts b/src/commands/pkg/changelog.ts index fd6d5002..44a61b66 100644 --- a/src/commands/pkg/changelog.ts +++ b/src/commands/pkg/changelog.ts @@ -3,6 +3,7 @@ import { createContainer } from "../../container.js"; import type { PackageIntelligenceService } from "../../services/index.js"; import { shouldUseColors } from "../../shared/colors.js"; import { + formatMappedErrorForTerminal, InvalidPackageSpecError, type MappedError, mapPackageIntelligenceError, @@ -146,6 +147,9 @@ function handlePkgChangelogCommandError(error: unknown, json: boolean): never { * these from `fromVersion` / `toVersion` when `version` wasn't set). */ function formatChangelogTerminalError(mapped: MappedError): string { + if (mapped.code === "UPDATE_REQUIRED") { + return formatMappedErrorForTerminal(mapped); + } if (mapped.code !== "VERSION_NOT_FOUND") return mapped.message; const detail = mapped.details ?? {}; const pkg = typeof detail.package === "string" ? detail.package : undefined; diff --git a/src/commands/pkg/deps.ts b/src/commands/pkg/deps.ts index 86227a72..9fd0d4a3 100644 --- a/src/commands/pkg/deps.ts +++ b/src/commands/pkg/deps.ts @@ -3,6 +3,7 @@ import { createContainer } from "../../container.js"; import type { PackageIntelligenceService } from "../../services/index.js"; import { shouldUseColors } from "../../shared/colors.js"; import { + formatMappedErrorForTerminal, InvalidPackageSpecError, type MappedError, mapPackageIntelligenceError, @@ -161,6 +162,9 @@ function handlePkgDepsCommandError(error: unknown, json: boolean): never { * provides one. */ function formatDepsTerminalError(mapped: MappedError): string { + if (mapped.code === "UPDATE_REQUIRED") { + return formatMappedErrorForTerminal(mapped); + } if (mapped.code !== "VERSION_NOT_FOUND") return mapped.message; const detail = mapped.details ?? {}; const pkg = typeof detail.package === "string" ? detail.package : undefined; diff --git a/src/commands/pkg/info.ts b/src/commands/pkg/info.ts index d7015bd4..b2a65422 100644 --- a/src/commands/pkg/info.ts +++ b/src/commands/pkg/info.ts @@ -3,6 +3,7 @@ import { createContainer } from "../../container.js"; import type { PackageIntelligenceService } from "../../services/index.js"; import { shouldUseColors } from "../../shared/colors.js"; import { + formatMappedErrorForTerminal, InvalidPackageSpecError, mapPackageIntelligenceError, parsePackageSpec, @@ -93,7 +94,7 @@ function handlePkgInfoCommandError(error: unknown, json: boolean): never { // Bare mapped message. Domain messages (`Package 'npm:foo' not found.`, // `pkg info always returns the latest version; omit @4.18.0.`) are // already caller-readable. - console.error(mapped.message); + console.error(formatMappedErrorForTerminal(mapped)); process.exit(1); } diff --git a/src/commands/pkg/vulns.ts b/src/commands/pkg/vulns.ts index 9e809b56..f908012d 100644 --- a/src/commands/pkg/vulns.ts +++ b/src/commands/pkg/vulns.ts @@ -3,6 +3,7 @@ import { createContainer } from "../../container.js"; import type { PackageIntelligenceService } from "../../services/index.js"; import { shouldUseColors } from "../../shared/colors.js"; import { + formatMappedErrorForTerminal, InvalidPackageSpecError, type MappedError, mapPackageIntelligenceError, @@ -108,6 +109,9 @@ function handlePkgVulnsCommandError(error: unknown, json: boolean): never { * ("No matching version found") omits crucial context. */ function formatVulnsTerminalError(mapped: MappedError): string { + if (mapped.code === "UPDATE_REQUIRED") { + return formatMappedErrorForTerminal(mapped); + } if (mapped.code !== "VERSION_NOT_FOUND") return mapped.message; const detail = mapped.details ?? {}; const pkg = typeof detail.package === "string" ? detail.package : undefined; diff --git a/src/services/client-update-required-error.ts b/src/services/client-update-required-error.ts new file mode 100644 index 00000000..11095425 --- /dev/null +++ b/src/services/client-update-required-error.ts @@ -0,0 +1,40 @@ +import { version } from "../../package.json"; + +export const CLIENT_UPDATE_REQUIRED_REASON = "Backend protocol changed"; + +export class ClientUpdateRequiredError extends Error { + constructor( + message = `Update required: ${CLIENT_UPDATE_REQUIRED_REASON}`, + public readonly reason = CLIENT_UPDATE_REQUIRED_REASON, + public readonly currentVersion = version, + ) { + super(message); + this.name = "ClientUpdateRequiredError"; + } +} + +export function isClientUpdateRequiredGraphQLError(input: { + message: string; + code?: string; +}): boolean { + if (input.code === "CLIENT_UPDATE_REQUIRED") { + return true; + } + + const message = input.message; + if (!isGraphQLSchemaMismatchMessage(message)) { + return false; + } + + return ( + !input.code || + input.code === "GRAPHQL_VALIDATION_FAILED" || + input.code === "BAD_USER_INPUT" + ); +} + +function isGraphQLSchemaMismatchMessage(message: string): boolean { + return /Cannot query field|Field .* does not exist|Unknown argument|Unknown type|Unknown field/i.test( + message, + ); +} diff --git a/src/services/code-navigation-service.test.ts b/src/services/code-navigation-service.test.ts index 2f28dc1b..38096ed4 100644 --- a/src/services/code-navigation-service.test.ts +++ b/src/services/code-navigation-service.test.ts @@ -7,6 +7,7 @@ import { mock, spyOn, } from "bun:test"; +import { ClientUpdateRequiredError } from "./client-update-required-error.js"; import { CodeNavigationBackendError, CodeNavigationFileNotFoundError, @@ -687,6 +688,35 @@ describe("CodeNavigationServiceImpl", () => { } }); + it("classifies GraphQL schema mismatch as ClientUpdateRequiredError", async () => { + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: 'Cannot query field "search" on type "Query".', + extensions: { code: "GRAPHQL_VALIDATION_FAILED" }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + await expect( + service.search({ + targets: [{ registry: "NPM", packageName: "express" }], + query: "middleware", + }), + ).rejects.toBeInstanceOf(ClientUpdateRequiredError); + }); + it("sends grepRepo variables with the correct shape", async () => { const fn = mockFetch(() => Promise.resolve( diff --git a/src/services/code-navigation-service.ts b/src/services/code-navigation-service.ts index b75c2b54..e0996ad4 100644 --- a/src/services/code-navigation-service.ts +++ b/src/services/code-navigation-service.ts @@ -6,6 +6,10 @@ import { postPkgseerGraphql, } from "../shared/pkgseer-graphql.js"; import type { PkgseerRegistry } from "../shared/pkgseer-registry.js"; +import { + ClientUpdateRequiredError, + isClientUpdateRequiredGraphQLError, +} from "./client-update-required-error.js"; import { executeWithTokenRefresh } from "./execute-with-token-refresh.js"; import { AuthenticationError } from "./githits-service.js"; import type { TokenProvider } from "./token-manager.js"; @@ -1472,6 +1476,10 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { : undefined; const indexingRef = getGraphQLIndexingRef(errors); + if (isClientUpdateRequiredGraphQLError({ message, code })) { + return new ClientUpdateRequiredError(); + } + // Direct dispatch on extensions.code — the April 2026 backend // contract populates this on every error. Fall back to message // heuristics below for older backend builds that haven't diff --git a/src/services/index.ts b/src/services/index.ts index c40e6cd2..0c45b392 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -34,6 +34,11 @@ export { ChunkingKeyringService, WINDOWS_MAX_ENTRY_SIZE, } from "./chunking-keyring-service.js"; +export { + CLIENT_UPDATE_REQUIRED_REASON, + ClientUpdateRequiredError, + isClientUpdateRequiredGraphQLError, +} from "./client-update-required-error.js"; export type { AvailableVersion, CodeNavigationRegistry, @@ -184,13 +189,17 @@ export { RefreshingGitHitsService } from "./refreshing-githits-service.js"; export type { TokenProvider } from "./token-manager.js"; export { refreshExpiredToken, TokenManager } from "./token-manager.js"; export type { + RequiredUpdateNotice, UpdateCheckFetcher, UpdateCheckNotice, UpdateCheckService, } from "./update-check-service.js"; export { + formatRequiredUpdateNotice, + formatUpdateCommand, formatUpdateNotice, NpmRegistryUpdateCheckService, resolveConfigHome, + shouldRunRequiredUpdateEnforcement, shouldRunUpdateCheck, } from "./update-check-service.js"; diff --git a/src/services/package-intelligence-service.test.ts b/src/services/package-intelligence-service.test.ts index e0eadbcb..618c711e 100644 --- a/src/services/package-intelligence-service.test.ts +++ b/src/services/package-intelligence-service.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"; +import { ClientUpdateRequiredError } from "./client-update-required-error.js"; import { AuthenticationError } from "./githits-service.js"; import { MalformedPackageIntelligenceResponseError, @@ -424,6 +425,30 @@ describe("PackageIntelligenceServiceImpl", () => { ).rejects.toBeInstanceOf(PackageIntelligenceValidationError); }); + it("classifies GraphQL schema mismatch as ClientUpdateRequiredError", async () => { + const fetchFn = mock(() => + Promise.resolve( + jsonResponse({ + errors: [ + { + message: 'Cannot query field "packageSummary" on type "Query".', + extensions: { code: "GRAPHQL_VALIDATION_FAILED" }, + }, + ], + }), + ), + ); + const service = new PackageIntelligenceServiceImpl( + ENDPOINT, + createMockTokenProvider(), + asFetchFn(fetchFn), + ); + + await expect( + service.packageSummary({ registry: "NPM", packageName: "x" }), + ).rejects.toBeInstanceOf(ClientUpdateRequiredError); + }); + it("classifies 5xx plain-text body via parseDetail as PackageIntelligenceBackendError", async () => { const fetchFn = mock(() => Promise.resolve( diff --git a/src/services/package-intelligence-service.ts b/src/services/package-intelligence-service.ts index ca14e09b..30c5a3b9 100644 --- a/src/services/package-intelligence-service.ts +++ b/src/services/package-intelligence-service.ts @@ -24,6 +24,10 @@ import { } from "../shared/pkgseer-graphql.js"; import type { PkgseerRegistry } from "../shared/pkgseer-registry.js"; import { withTelemetrySpan } from "../shared/telemetry.js"; +import { + ClientUpdateRequiredError, + isClientUpdateRequiredGraphQLError, +} from "./client-update-required-error.js"; import { executeWithTokenRefresh } from "./execute-with-token-refresh.js"; import { AuthenticationError } from "./githits-service.js"; import { promoteGenericVersionNotFound } from "./promote-version-not-found.js"; @@ -1397,6 +1401,10 @@ export class PackageIntelligenceServiceImpl ? extensions.retryable : undefined; + if (isClientUpdateRequiredGraphQLError({ message, code })) { + return new ClientUpdateRequiredError(); + } + switch (code) { case "NOT_FOUND": case "PACKAGE_NOT_FOUND": diff --git a/src/services/test-helpers.ts b/src/services/test-helpers.ts index 10002d30..08191e2b 100644 --- a/src/services/test-helpers.ts +++ b/src/services/test-helpers.ts @@ -236,6 +236,8 @@ export function createMockUpdateCheckService( ): UpdateCheckService { return { checkForUpdate: mock(() => Promise.resolve(undefined)), + refreshRequiredUpdateStatus: mock(() => Promise.resolve()), + getRequiredUpdateNotice: mock(() => Promise.resolve(undefined)), ...impl, }; } diff --git a/src/services/update-check-service.test.ts b/src/services/update-check-service.test.ts index f6c92291..b0282ada 100644 --- a/src/services/update-check-service.test.ts +++ b/src/services/update-check-service.test.ts @@ -1,9 +1,12 @@ import { describe, expect, it, mock } from "bun:test"; import { createMockFileSystemService } from "./test-helpers.js"; import { + formatRequiredUpdateNotice, + formatUpdateCommand, formatUpdateNotice, NpmRegistryUpdateCheckService, resolveConfigHome, + shouldRunRequiredUpdateEnforcement, shouldRunUpdateCheck, type UpdateCheckFetcher, } from "./update-check-service.js"; @@ -41,10 +44,9 @@ describe("NpmRegistryUpdateCheckService", () => { "/home/test/.config/githits", 0o700, ); - expect(fs.writeFile).toHaveBeenCalledWith( + expect(fs.atomicWriteFile).toHaveBeenCalledWith( "/home/test/.config/githits/update-check.json", expect.any(String), - 0o600, ); }); @@ -64,10 +66,9 @@ describe("NpmRegistryUpdateCheckService", () => { await service.checkForUpdate(); expect(fs.ensureDir).toHaveBeenCalledWith("/tmp/xdg/githits", 0o700); - expect(fs.writeFile).toHaveBeenCalledWith( + expect(fs.atomicWriteFile).toHaveBeenCalledWith( "/tmp/xdg/githits/update-check.json", expect.any(String), - 0o600, ); }); @@ -94,7 +95,10 @@ describe("NpmRegistryUpdateCheckService", () => { const notice = await service.checkForUpdate(); - expect(fetcher).not.toHaveBeenCalled(); + expect(fetcher).toHaveBeenCalledWith( + "https://registry.npmjs.org/githits/0.2.0", + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); expect(notice?.latestVersion).toBe("0.3.0"); }); @@ -122,7 +126,10 @@ describe("NpmRegistryUpdateCheckService", () => { const notice = await service.checkForUpdate(); - expect(fetcher).not.toHaveBeenCalled(); + expect(fetcher).toHaveBeenCalledWith( + "https://registry.npmjs.org/githits/0.2.0", + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); expect(notice?.latestVersion).toBe("0.3.0"); }); @@ -261,6 +268,199 @@ describe("NpmRegistryUpdateCheckService", () => { expect(fetcher).toHaveBeenCalled(); expect(notice?.latestVersion).toBe("0.3.0"); }); + + it("refreshes and caches deprecated current version status", async () => { + const fetcher = createRouteFetcher({ + "https://registry.npmjs.org/githits/0.2.0": { + deprecated: "Backend protocol changed", + }, + }); + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(true)), + readFile: mock(() => + Promise.resolve( + JSON.stringify({ + checkedAt: FRESH, + latestVersion: "0.3.0", + }), + ), + ), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher, + now: () => NOW, + env: {}, + }); + + await service.refreshRequiredUpdateStatus(); + + expect(fetcher).toHaveBeenCalledWith( + "https://registry.npmjs.org/githits/0.2.0", + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + expect(getWrittenCache(fs).currentVersionStatus).toEqual({ + version: "0.2.0", + checkedAt: NOW.toISOString(), + deprecatedReason: "Backend protocol changed", + }); + }); + + it("returns cached required update notice without fetching", async () => { + const fetcher = createJsonFetcher({ latest: "0.4.0" }); + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(true)), + readFile: mock(() => + Promise.resolve( + JSON.stringify({ + checkedAt: FRESH, + latestVersion: "0.3.0", + currentVersionStatus: { + version: "0.2.0", + checkedAt: FRESH, + deprecatedReason: "Backend protocol changed", + }, + }), + ), + ), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher, + now: () => NOW, + env: {}, + }); + + await expect(service.getRequiredUpdateNotice()).resolves.toEqual({ + currentVersion: "0.2.0", + latestKnownVersion: "0.3.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }); + expect(fetcher).not.toHaveBeenCalled(); + }); + + it("ignores cached required update notice for a different current version", async () => { + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(true)), + readFile: mock(() => + Promise.resolve( + JSON.stringify({ + currentVersionStatus: { + version: "0.1.0", + checkedAt: FRESH, + deprecatedReason: "Backend protocol changed", + }, + }), + ), + ), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher: createJsonFetcher({}), + now: () => NOW, + env: {}, + }); + + await expect(service.getRequiredUpdateNotice()).resolves.toBeUndefined(); + }); + + it("preserves cached deprecated status when refresh fails", async () => { + const fetcher = mock(() => Promise.reject(new Error("network down"))); + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(true)), + readFile: mock(() => + Promise.resolve( + JSON.stringify({ + checkedAt: FRESH, + latestVersion: "0.3.0", + currentVersionStatus: { + version: "0.2.0", + checkedAt: STALE, + deprecatedReason: "Backend protocol changed", + }, + }), + ), + ), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher, + now: () => NOW, + env: {}, + }); + + await service.refreshRequiredUpdateStatus(); + + expect(fs.atomicWriteFile).not.toHaveBeenCalled(); + await expect(service.getRequiredUpdateNotice()).resolves.toMatchObject({ + reason: "Backend protocol changed", + }); + }); + + it("clears cached deprecated status after successful non-deprecated metadata", async () => { + const fetcher = createRouteFetcher({ + "https://registry.npmjs.org/githits/0.2.0": { version: "0.2.0" }, + }); + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(true)), + readFile: mock(() => + Promise.resolve( + JSON.stringify({ + checkedAt: FRESH, + latestVersion: "0.3.0", + currentVersionStatus: { + version: "0.2.0", + checkedAt: STALE, + deprecatedReason: "Backend protocol changed", + }, + }), + ), + ), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher, + now: () => NOW, + env: {}, + }); + + await service.refreshRequiredUpdateStatus(); + + expect(getWrittenCache(fs).currentVersionStatus).toEqual({ + version: "0.2.0", + checkedAt: NOW.toISOString(), + }); + }); + + it("sanitizes deprecated reason before caching", async () => { + const fetcher = createRouteFetcher({ + "https://registry.npmjs.org/githits/0.2.0": { + deprecated: "\u001b[31mBackend\nprotocol\tchanged\u001b[0m", + }, + }); + const fs = createMockFileSystemService({ + exists: mock(() => Promise.resolve(false)), + }); + const service = new NpmRegistryUpdateCheckService({ + currentVersion: "0.2.0", + fileSystemService: fs, + fetcher, + now: () => NOW, + env: {}, + }); + + await service.refreshRequiredUpdateStatus(); + + expect(getWrittenCache(fs).currentVersionStatus).toMatchObject({ + deprecatedReason: "[31mBackend protocol changed [0m", + }); + }); }); describe("resolveConfigHome", () => { @@ -383,6 +583,32 @@ describe("shouldRunUpdateCheck", () => { }); }); +describe("shouldRunRequiredUpdateEnforcement", () => { + it("allows CI, non-TTY, and disabled advisory update checks", () => { + expect( + shouldRunRequiredUpdateEnforcement({ + args: ["example", "query"], + env: { CI: "1", GITHITS_DISABLE_UPDATE_CHECK: "1" }, + }), + ).toBe(true); + }); + + it("skips help, version, and ephemeral package-runner invocations", () => { + expect( + shouldRunRequiredUpdateEnforcement({ args: ["--help"], env: {} }), + ).toBe(false); + expect(shouldRunRequiredUpdateEnforcement({ args: ["-V"], env: {} })).toBe( + false, + ); + expect( + shouldRunRequiredUpdateEnforcement({ + args: ["example", "query"], + env: { npm_lifecycle_event: "npx" }, + }), + ).toBe(false); + }); +}); + describe("formatUpdateNotice", () => { it("formats stderr-only update notice text", () => { expect( @@ -397,6 +623,48 @@ describe("formatUpdateNotice", () => { }); }); +describe("formatRequiredUpdateNotice", () => { + it("formats blocking required update text", () => { + expect( + formatRequiredUpdateNotice({ + currentVersion: "0.2.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }), + ).toBe( + "Update required: Backend protocol changed\n\nInstalled githits 0.2.0 is no longer supported.\nUpdate with:\n npm i -g githits@latest", + ); + }); + + it("includes latest known version when present", () => { + expect( + formatRequiredUpdateNotice({ + currentVersion: "0.2.0", + latestKnownVersion: "0.3.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }), + ).toBe( + "Update required: Backend protocol changed\n\nInstalled githits 0.2.0 is no longer supported.\nLatest known version: 0.3.0\nUpdate with:\n npm i -g githits@latest", + ); + }); +}); + +describe("formatUpdateCommand", () => { + it("uses package-manager hints when present", () => { + expect(formatUpdateCommand({ npm_config_user_agent: "pnpm/9.0.0" })).toBe( + "pnpm add -g githits@latest", + ); + expect(formatUpdateCommand({ npm_execpath: "/bin/yarn.js" })).toBe( + "yarn global add githits@latest", + ); + expect(formatUpdateCommand({ npm_config_user_agent: "bun/1.3.0" })).toBe( + "bun add -g githits@latest", + ); + expect(formatUpdateCommand({})).toBe("npm i -g githits@latest"); + }); +}); + function createService(options: { currentVersion?: string; body?: unknown; @@ -421,9 +689,22 @@ function createJsonFetcher( ) as UpdateCheckFetcher & ReturnType; } +function createRouteFetcher( + routes: Record, +): UpdateCheckFetcher & ReturnType { + return mock((input: string | URL | Request) => { + const url = typeof input === "string" ? input : input.toString(); + const body = routes[url]; + if (body === undefined) { + return Promise.resolve(new Response("not found", { status: 404 })); + } + return Promise.resolve(Response.json(body)); + }) as UpdateCheckFetcher & ReturnType; +} + function getWrittenCache( fs: ReturnType, ): Record { - const calls = (fs.writeFile as ReturnType).mock.calls; + const calls = (fs.atomicWriteFile as ReturnType).mock.calls; return JSON.parse(calls.at(-1)?.[1] as string) as Record; } diff --git a/src/services/update-check-service.ts b/src/services/update-check-service.ts index d65a8f40..540f1ea3 100644 --- a/src/services/update-check-service.ts +++ b/src/services/update-check-service.ts @@ -3,11 +3,12 @@ import type { FileSystemService } from "./filesystem-service.js"; const NPM_DIST_TAGS_URL = "https://registry.npmjs.org/-/package/githits/dist-tags"; +const NPM_PACKAGE_VERSION_URL = "https://registry.npmjs.org/githits"; const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; const FETCH_TIMEOUT_MS = 1000; const DIR_MODE = 0o700; -const FILE_MODE = 0o600; const UPDATE_COMMAND = "npm i -g githits@latest"; +const MAX_DEPRECATION_REASON_LENGTH = 200; export interface UpdateCheckNotice { currentVersion: string; @@ -15,8 +16,17 @@ export interface UpdateCheckNotice { updateCommand: string; } +export interface RequiredUpdateNotice { + currentVersion: string; + latestKnownVersion?: string; + reason: string; + updateCommand: string; +} + export interface UpdateCheckService { checkForUpdate(signal?: AbortSignal): Promise; + refreshRequiredUpdateStatus(signal?: AbortSignal): Promise; + getRequiredUpdateNotice(): Promise; } export type UpdateCheckFetcher = ( @@ -35,8 +45,15 @@ export interface UpdateCheckServiceOptions { } interface UpdateCheckCache { + checkedAt?: string; + latestVersion?: string; + currentVersionStatus?: CurrentVersionStatus; +} + +interface CurrentVersionStatus { + version: string; checkedAt: string; - latestVersion: string; + deprecatedReason?: string; } export interface UpdateCheckEligibilityInput { @@ -47,6 +64,11 @@ export interface UpdateCheckEligibilityInput { stdoutIsTTY?: boolean; } +export interface RequiredUpdateEligibilityInput { + args: string[]; + env?: Record; +} + export class NpmRegistryUpdateCheckService implements UpdateCheckService { private readonly currentVersion: string; private readonly fs: FileSystemService; @@ -77,24 +99,95 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { signal?: AbortSignal, ): Promise { const cache = await this.loadCache(); + const latestDue = !cache || this.isCheckDue(cache); + const currentVersionStatusDue = this.isCurrentVersionStatusDue( + cache?.currentVersionStatus, + ); + const currentVersionStatus = await this.refreshCurrentVersionStatusIfDue( + cache?.currentVersionStatus, + signal, + ); + const currentVersionStatusChanged = + currentVersionStatusDue && + !sameCurrentVersionStatus( + currentVersionStatus, + cache?.currentVersionStatus, + ); - if (cache && !this.isCheckDue(cache)) { + if (cache && !latestDue) { + if (currentVersionStatusChanged) { + await this.saveCache({ + ...cache, + currentVersionStatus, + }); + } return this.noticeFromLatest(cache.latestVersion); } const latestVersion = await this.fetchLatestVersion(signal); if (!latestVersion) { + if (currentVersionStatusChanged) { + await this.saveCache({ + ...cache, + currentVersionStatus, + }); + } return this.noticeFromLatest(cache?.latestVersion); } await this.saveCache({ checkedAt: this.now().toISOString(), latestVersion, + currentVersionStatus, }); return this.noticeFromLatest(latestVersion); } + async refreshRequiredUpdateStatus(signal?: AbortSignal): Promise { + const cache = await this.loadCache(); + const currentVersionStatusDue = this.isCurrentVersionStatusDue( + cache?.currentVersionStatus, + ); + const currentVersionStatus = await this.refreshCurrentVersionStatusIfDue( + cache?.currentVersionStatus, + signal, + ); + if ( + currentVersionStatusDue && + !sameCurrentVersionStatus( + currentVersionStatus, + cache?.currentVersionStatus, + ) + ) { + await this.saveCache({ + ...cache, + currentVersionStatus, + }); + } + } + + async getRequiredUpdateNotice(): Promise { + const cache = await this.loadCache(); + const status = cache?.currentVersionStatus; + if ( + !status || + status.version !== this.currentVersion || + !status.deprecatedReason + ) { + return undefined; + } + + return { + currentVersion: this.currentVersion, + ...(cache.latestVersion + ? { latestKnownVersion: cache.latestVersion } + : {}), + reason: status.deprecatedReason, + updateCommand: formatUpdateCommand(this.env), + }; + } + private noticeFromLatest( latestVersion: string | undefined, ): UpdateCheckNotice | undefined { @@ -115,6 +208,9 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { } private isCheckDue(cache: UpdateCheckCache): boolean { + if (!cache.checkedAt || !cache.latestVersion) { + return true; + } const checkedAtMs = Date.parse(cache.checkedAt); if (Number.isNaN(checkedAtMs)) { return true; @@ -122,6 +218,37 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { return this.now().getTime() - checkedAtMs >= this.checkIntervalMs; } + private isCurrentVersionStatusDue( + status: CurrentVersionStatus | undefined, + ): boolean { + if (!status || status.version !== this.currentVersion) { + return true; + } + const checkedAtMs = Date.parse(status.checkedAt); + if (Number.isNaN(checkedAtMs)) { + return true; + } + return this.now().getTime() - checkedAtMs >= this.checkIntervalMs; + } + + private async refreshCurrentVersionStatusIfDue( + cached: CurrentVersionStatus | undefined, + signal: AbortSignal | undefined, + ): Promise { + const reusableCached = + cached?.version === this.currentVersion ? cached : undefined; + if (!this.isCurrentVersionStatusDue(reusableCached)) { + return reusableCached; + } + + const remote = await this.fetchCurrentVersionStatus(signal); + if (!remote) { + return reusableCached; + } + + return remote; + } + private async fetchLatestVersion( signal: AbortSignal | undefined, ): Promise { @@ -150,6 +277,42 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { } } + private async fetchCurrentVersionStatus( + signal: AbortSignal | undefined, + ): Promise { + try { + const timeoutSignal = AbortSignal.timeout(this.fetchTimeoutMs); + const response = await this.fetcher( + `${NPM_PACKAGE_VERSION_URL}/${this.currentVersion}`, + { + signal: signal + ? AbortSignal.any([signal, timeoutSignal]) + : timeoutSignal, + }, + ); + if (!response.ok) { + return undefined; + } + const body = await response.json(); + if (!body || typeof body !== "object") { + return undefined; + } + const rawDeprecated = (body as { deprecated?: unknown }).deprecated; + const deprecatedReason = + typeof rawDeprecated === "string" + ? sanitizeDeprecationReason(rawDeprecated) + : undefined; + + return { + version: this.currentVersion, + checkedAt: this.now().toISOString(), + ...(deprecatedReason ? { deprecatedReason } : {}), + }; + } catch { + return undefined; + } + } + private async loadCache(): Promise { try { if (!(await this.fs.exists(this.cachePath))) { @@ -157,15 +320,23 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { } const raw = await this.fs.readFile(this.cachePath); const parsed = JSON.parse(raw) as Partial; - if ( - typeof parsed.checkedAt !== "string" || - typeof parsed.latestVersion !== "string" - ) { + const currentVersionStatus = parseCurrentVersionStatus( + parsed.currentVersionStatus, + ); + const hasLatest = + typeof parsed.checkedAt === "string" && + typeof parsed.latestVersion === "string"; + if (!hasLatest && !currentVersionStatus) { return undefined; } return { - checkedAt: parsed.checkedAt, - latestVersion: parsed.latestVersion, + ...(hasLatest + ? { + checkedAt: parsed.checkedAt, + latestVersion: parsed.latestVersion, + } + : {}), + currentVersionStatus, }; } catch { return undefined; @@ -175,10 +346,17 @@ export class NpmRegistryUpdateCheckService implements UpdateCheckService { private async saveCache(cache: UpdateCheckCache): Promise { try { await this.fs.ensureDir(this.configDir, DIR_MODE); + if (typeof this.fs.atomicWriteFile === "function") { + await this.fs.atomicWriteFile( + this.cachePath, + `${JSON.stringify(cache, null, 2)}\n`, + ); + return; + } await this.fs.writeFile( this.cachePath, `${JSON.stringify(cache, null, 2)}\n`, - FILE_MODE, + 0o600, ); } catch { // Update checks must never break the real command. @@ -230,10 +408,110 @@ export function shouldRunUpdateCheck( return true; } +export function shouldRunRequiredUpdateEnforcement( + input: RequiredUpdateEligibilityInput, +): boolean { + if (isHelpOrVersionInvocation(input.args)) { + return false; + } + + const env = input.env ?? process.env; + if (isLikelyEphemeralPackageRunner(env)) { + return false; + } + + return true; +} + export function formatUpdateNotice(notice: UpdateCheckNotice): string { return `Update available: githits ${notice.currentVersion} -> ${notice.latestVersion}\nRun: ${notice.updateCommand}`; } +export function formatRequiredUpdateNotice( + notice: RequiredUpdateNotice, +): string { + const lines = [ + `Update required: ${notice.reason}`, + "", + `Installed githits ${notice.currentVersion} is no longer supported.`, + ]; + if (notice.latestKnownVersion) { + lines.push(`Latest known version: ${notice.latestKnownVersion}`); + } + lines.push("Update with:", ` ${notice.updateCommand}`); + return [...lines].join("\n"); +} + +export function formatUpdateCommand( + env: Record = process.env, +): string { + const userAgent = env.npm_config_user_agent ?? ""; + const execPath = env.npm_execpath ?? ""; + const signal = `${userAgent} ${execPath}`.toLowerCase(); + + if (signal.includes("pnpm")) { + return "pnpm add -g githits@latest"; + } + if (signal.includes("yarn")) { + return "yarn global add githits@latest"; + } + if (signal.includes("bun")) { + return "bun add -g githits@latest"; + } + return UPDATE_COMMAND; +} + +function parseCurrentVersionStatus( + value: unknown, +): CurrentVersionStatus | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + const status = value as Partial; + if (typeof status.version !== "string") { + return undefined; + } + if (typeof status.checkedAt !== "string") { + return undefined; + } + const deprecatedReason = + typeof status.deprecatedReason === "string" + ? sanitizeDeprecationReason(status.deprecatedReason) + : undefined; + return { + version: status.version, + checkedAt: status.checkedAt, + ...(deprecatedReason ? { deprecatedReason } : {}), + }; +} + +function sameCurrentVersionStatus( + left: CurrentVersionStatus | undefined, + right: CurrentVersionStatus | undefined, +): boolean { + return ( + left?.version === right?.version && + left?.checkedAt === right?.checkedAt && + left?.deprecatedReason === right?.deprecatedReason + ); +} + +function sanitizeDeprecationReason(value: string): string | undefined { + const sanitized = Array.from(value) + .map((character) => (isControlCharacter(character) ? " " : character)) + .join("") + .replace(/\s+/g, " ") + .trim() + .slice(0, MAX_DEPRECATION_REASON_LENGTH) + .trim(); + return sanitized.length > 0 ? sanitized : undefined; +} + +function isControlCharacter(value: string): boolean { + const code = value.charCodeAt(0); + return code <= 0x1f || (code >= 0x7f && code <= 0x9f); +} + function isHelpOrVersionInvocation(args: string[]): boolean { return ( args.length === 0 || diff --git a/src/shared/code-navigation-error-map.test.ts b/src/shared/code-navigation-error-map.test.ts index 0be71f3d..d958cc3e 100644 --- a/src/shared/code-navigation-error-map.test.ts +++ b/src/shared/code-navigation-error-map.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import { ClientUpdateRequiredError } from "../services/client-update-required-error.js"; import { CodeNavigationAccessError, CodeNavigationBackendError, @@ -31,6 +32,23 @@ class UnsupportedRegistryError extends Error { } describe("mapCodeNavigationError", () => { + it("classifies ClientUpdateRequiredError as UPDATE_REQUIRED", () => { + expect( + mapCodeNavigationError( + new ClientUpdateRequiredError(undefined, undefined, "0.2.0"), + ), + ).toEqual({ + code: "UPDATE_REQUIRED", + message: "Update required: Backend protocol changed", + retryable: false, + details: { + currentVersion: "0.2.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }, + }); + }); + it("classifies CodeNavigationTargetNotFoundError as NOT_FOUND", () => { const err = new CodeNavigationTargetNotFoundError("Package not found", [ { version: "5.2.1", ref: "v5.2.1" }, diff --git a/src/shared/code-navigation-error-map.ts b/src/shared/code-navigation-error-map.ts index d248c160..600b5191 100644 --- a/src/shared/code-navigation-error-map.ts +++ b/src/shared/code-navigation-error-map.ts @@ -1,3 +1,7 @@ +import { + CLIENT_UPDATE_REQUIRED_REASON, + ClientUpdateRequiredError, +} from "../services/client-update-required-error.js"; import { type AvailableVersion, CodeNavigationAccessError, @@ -30,6 +34,7 @@ export type MappedErrorCode = | "TIMEOUT" | "RATE_LIMITED" | "PROTOCOL_ERROR" + | "UPDATE_REQUIRED" | "UNKNOWN"; export interface MappedErrorDetails { @@ -49,6 +54,12 @@ export interface MappedErrorDetails { package?: string; /** The file path the caller asked for (for `FILE_NOT_FOUND`). */ filePath?: string; + /** Installed CLI version when an update is required. */ + currentVersion?: string; + /** Suggested package-manager command when an update is required. */ + updateCommand?: string; + /** Human-readable update reason. */ + reason?: string; } export interface MappedError { @@ -92,6 +103,9 @@ export function mapCodeNavigationError(error: unknown): MappedError { } function classify(error: unknown): MappedError { + if (error instanceof ClientUpdateRequiredError) { + return buildUpdateRequiredError(error.reason, error.currentVersion); + } if (error instanceof CodeNavigationVersionNotFoundError) { const details: MappedErrorDetails = {}; if (error.packageName) details.package = error.packageName; @@ -207,6 +221,34 @@ function classify(error: unknown): MappedError { return { code: "UNKNOWN", message: "Unknown error", retryable: false }; } +export function buildUpdateRequiredError( + reason = CLIENT_UPDATE_REQUIRED_REASON, + currentVersion?: string, +): MappedError { + return { + code: "UPDATE_REQUIRED", + message: `Update required: ${reason}`, + retryable: false, + details: { + reason, + updateCommand: "npm i -g githits@latest", + ...(currentVersion ? { currentVersion } : {}), + }, + }; +} + +export function formatMappedErrorForTerminal(mapped: MappedError): string { + if (mapped.code !== "UPDATE_REQUIRED") { + return mapped.message; + } + const detail = mapped.details ?? {}; + const updateCommand = + typeof detail.updateCommand === "string" + ? detail.updateCommand + : "npm i -g githits@latest"; + return [mapped.message, "", "Update with:", ` ${updateCommand}`].join("\n"); +} + /** * Dispatch on `CodeNavigationBackendError.graphqlCode` to produce * TIMEOUT / RATE_LIMITED / BACKEND_ERROR with the correct retryable diff --git a/src/shared/index.ts b/src/shared/index.ts index e4a2599b..8da15639 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -16,6 +16,7 @@ export { MAX_WAIT_TIMEOUT_MS, } from "./code-navigation-defaults.js"; export { + formatMappedErrorForTerminal, type MappedError, type MappedErrorCode, mapCodeNavigationError, diff --git a/src/shared/package-intelligence-error-map.test.ts b/src/shared/package-intelligence-error-map.test.ts index ccced1e4..c8083b1e 100644 --- a/src/shared/package-intelligence-error-map.test.ts +++ b/src/shared/package-intelligence-error-map.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { ClientUpdateRequiredError } from "../services/client-update-required-error.js"; import { AuthenticationError } from "../services/githits-service.js"; import { MalformedPackageIntelligenceResponseError, @@ -14,6 +15,23 @@ import { import { mapPackageIntelligenceError } from "./package-intelligence-error-map.js"; describe("mapPackageIntelligenceError", () => { + it("maps ClientUpdateRequiredError to UPDATE_REQUIRED", () => { + const mapped = mapPackageIntelligenceError( + new ClientUpdateRequiredError(undefined, undefined, "0.2.0"), + ); + + expect(mapped).toEqual({ + code: "UPDATE_REQUIRED", + message: "Update required: Backend protocol changed", + retryable: false, + details: { + currentVersion: "0.2.0", + reason: "Backend protocol changed", + updateCommand: "npm i -g githits@latest", + }, + }); + }); + it("maps PackageIntelligenceTargetNotFoundError to NOT_FOUND", () => { const mapped = mapPackageIntelligenceError( new PackageIntelligenceTargetNotFoundError("Package not found"), diff --git a/src/shared/package-intelligence-error-map.ts b/src/shared/package-intelligence-error-map.ts index 546d36e4..c5930d04 100644 --- a/src/shared/package-intelligence-error-map.ts +++ b/src/shared/package-intelligence-error-map.ts @@ -8,6 +8,7 @@ * single debug line under the `pkg-intel` area. */ +import { ClientUpdateRequiredError } from "../services/client-update-required-error.js"; import { AuthenticationError } from "../services/githits-service.js"; import { MalformedPackageIntelligenceResponseError, @@ -26,6 +27,7 @@ import type { MappedErrorCode, MappedErrorDetails, } from "./code-navigation-error-map.js"; +import { buildUpdateRequiredError } from "./code-navigation-error-map.js"; import { debugLog } from "./debug-log.js"; // Re-export for caller convenience — callers of @@ -51,6 +53,9 @@ export function mapPackageIntelligenceError(error: unknown): MappedError { } function classify(error: unknown): MappedError { + if (error instanceof ClientUpdateRequiredError) { + return buildUpdateRequiredError(error.reason, error.currentVersion); + } if ( error instanceof PackageIntelligenceTargetNotFoundError || error instanceof PackageIntelligenceChangelogSourceNotFoundError