mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
ee97c443b7
## Why The tools-server shows an update reminder (a `note` appended to every tool response) once a newer `@swmansion/argent` lands on npm. Two pain points: 1. **No way to turn it off.** Suppression is in-memory only (`dismiss-update`, 30-min auto), so it comes back on every server restart. 2. **It nags about versions you can't install yet.** Registries with a **minimum-release-age** (package-cooldown) security policy refuse to install a version until it has been public for N days. Until then the reminder repeats every day for the whole cooldown window with nothing the user can do about it — exactly the "reminded for 5+ days" situation. ## What's in this PR **1. A config option to disable the reminder** - New env var **`ARGENT_DISABLE_UPDATE_NOTIFICATIONS=1`** (`"1"` or `"true"`). - When set, `startUpdateChecker()` is a no-op: **no registry request** is made and **no note** is ever attached. - The opt-out is also surfaced inside the reminder text itself, so the agent can tell the user how to silence it permanently. **2. Minimum-release-age awareness** (answers "detect the policy and check the update is actually installable first") - The checker now fetches the **npm packument** (`registry.npmjs.org/@swmansion/argent`) instead of `/latest`, because only the packument carries the `time` map of version → publish timestamp. - It detects the machine's policy and only reminds once the latest version has **aged past** it. Notification is gated on a new **`updateInstallable`** flag (newer **and** old enough) rather than `updateAvailable`. - `updateAvailable` keeps its pure version-comparison meaning; with no policy, `updateInstallable === updateAvailable` (no behavior change for users without a policy). ### Is detecting the user's min-age policy possible? Yes — partially. Each package manager stores it differently, and the tools-server has no reliable signal for *which* PM installed argent, so we probe every PM present and take the **most restrictive** (largest) value. Erring toward over-waiting is intentional: a slightly late reminder beats nagging about a forbidden version. | PM | Config key | Unit | Detect via | Min PM version | |----|-----------|------|-----------|----------------| | npm | `min-release-age` | days | `npm config get min-release-age` | 11.10.0 | | pnpm | `minimumReleaseAge` | minutes | `pnpm config get minimumReleaseAge` | 10.16 | | yarn (berry) | `npmMinimalAgeGate` | minutes | `yarn config get npmMinimalAgeGate` | 4.10 | | bun | `minimumReleaseAge` (bunfig.toml `[install]`) | seconds | no `config get` — not auto-probed | 1.3 | | yarn classic | — (unsupported) | — | — | — | Because bun has no `config get` for this and auto-detection is best-effort, there's also an explicit override: - **`ARGENT_MIN_RELEASE_AGE_DAYS`** — positive number of days; wins outright and skips probing (useful for bun users, locked-down CI, or pinning the value). When a policy holds an update back, a single stderr line explains the silence; no reminder reaches the agent. ## Conservative edge handling - Network/registry failure → state unchanged (never crashes the server). - Policy in effect but publish time missing/unparseable → treated as **not** installable (stay silent rather than nag). - PM probe errors / PM absent → treated as no policy (0). - Probes shell out via PATH (`exec`, 3s timeout, constant commands only — no caller input) so Windows `.cmd`/`.ps1` shims resolve. ## Tests - `test/min-release-age.test.ts` (new): unit-level parsing, override precedence, unit normalization, most-restrictive-wins, PM-absent. - `test/update-checker.test.ts`: rewritten for the packument shape; new cases for held-by-policy, aged-past-policy, unknown publish time, and the disable flag (probe mocked so tests never spawn a real PM). - `test/http-update-note.test.ts`: gate now keys off `updateInstallable`; added a "available but held by policy → no note" case. - Full tool-server suite green (**732 tests**), `tsc --noEmit` and `typecheck:tests` clean. ## Follow-up (not in this PR) The env-var reference lives in the `argent-private` submodule (`docs/environment-variables.md`), which is a separate repo — needs a companion PR. Suggested rows for the tools-server table: ``` | `ARGENT_DISABLE_UPDATE_NOTIFICATIONS` | `"1"`/`"true"` or unset | disabled | When truthy, disables the update reminder entirely (no registry check, no note). | | `ARGENT_MIN_RELEASE_AGE_DAYS` | number | _(auto-detect)_ | Minimum age (days) a newer version must reach before it is advertised. Overrides auto-detection of the npm/pnpm/yarn minimum-release-age policy. | ``` ## Open questions for review - **Most-restrictive-across-PMs** vs. detecting a single PM — is "max wins" the behavior we want, or should it follow `npm_config_user_agent`? - Should an update held by policy stay fully silent (current), or emit a softened "available, installable in ~N days" note? --------- Co-authored-by: Cursor <cursoragent@cursor.com>
77 lines
2.2 KiB
TypeScript
77 lines
2.2 KiB
TypeScript
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
|
import https from "node:https";
|
|
import { EventEmitter } from "node:events";
|
|
|
|
vi.mock("node:https");
|
|
|
|
import { fetchRegistryInfo } from "../src/registry";
|
|
|
|
function createMockResponse(statusCode: number, body: string) {
|
|
const res = new EventEmitter() as EventEmitter & {
|
|
statusCode: number;
|
|
setEncoding: ReturnType<typeof vi.fn>;
|
|
resume: ReturnType<typeof vi.fn>;
|
|
};
|
|
res.statusCode = statusCode;
|
|
res.setEncoding = vi.fn();
|
|
res.resume = vi.fn();
|
|
|
|
process.nextTick(() => {
|
|
res.emit("data", body);
|
|
res.emit("end");
|
|
});
|
|
|
|
return res;
|
|
}
|
|
|
|
function mockResponseBody(statusCode: number, body: string) {
|
|
const mockGet = vi.mocked(https.get);
|
|
mockGet.mockImplementation((_url: unknown, _opts: unknown, cb: unknown) => {
|
|
const callback = cb as (res: ReturnType<typeof createMockResponse>) => void;
|
|
callback(createMockResponse(statusCode, body));
|
|
return new EventEmitter() as ReturnType<typeof https.get>;
|
|
});
|
|
return mockGet;
|
|
}
|
|
|
|
const URL = "https://registry.npmjs.org/@swmansion/argent";
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
describe("fetchRegistryInfo", () => {
|
|
it("parses the packument into latest + times", async () => {
|
|
mockResponseBody(
|
|
200,
|
|
JSON.stringify({
|
|
"dist-tags": { latest: "2.0.0" },
|
|
"time": { "1.0.0": "2026-01-01T00:00:00Z", "2.0.0": "2026-05-01T00:00:00Z" },
|
|
})
|
|
);
|
|
|
|
const info = await fetchRegistryInfo(URL);
|
|
expect(info?.latest).toEqual({ version: "2.0.0", publishedAt: "2026-05-01T00:00:00Z" });
|
|
expect(info?.times["1.0.0"]).toBe("2026-01-01T00:00:00Z");
|
|
});
|
|
|
|
it("returns null when the registry responds with a non-200 status", async () => {
|
|
mockResponseBody(503, "");
|
|
expect(await fetchRegistryInfo(URL)).toBeNull();
|
|
});
|
|
|
|
it("returns null when the body has no latest dist-tag", async () => {
|
|
mockResponseBody(200, JSON.stringify({ time: {} }));
|
|
expect(await fetchRegistryInfo(URL)).toBeNull();
|
|
});
|
|
|
|
it("returns null on invalid JSON", async () => {
|
|
mockResponseBody(200, "not json");
|
|
expect(await fetchRegistryInfo(URL)).toBeNull();
|
|
});
|
|
});
|