Commit Graph

6 Commits

Author SHA1 Message Date
filip131311 ee97c443b7 feat(tool-server): make the update reminder configurable and release-age aware (#276)
## 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>
2026-06-01 17:15:12 +02:00
Ignacy Łątka 2180dd8e6a ci: typecheck test files in CI (#202)
We've had lots of type errors in our tests, scripts and other non-core
code despite all CI being green. This PR fixes that.

This PR also removes some redundant features like `@ts-check` inline
directives which have been replaced by proper ts config files.

<details>

## AI Summary

Audit of every CI workflow against every package's `package.json` and
`tsconfig` surfaced several gaps where CI did partial coverage. This PR
closes them and fixes every pre-existing type error the new gates
surfaced.

## Gaps closed

| # | Gap | Fix |
|---|---|---|
| 1 | `tsc --build` only covers `src/**`; test files (`test/**`,
`tests/**`) never typecheck. Vitest transforms tests with esbuild —
strips types instead of checking them. | New `tsconfig.test.json` per
test-bearing package (`composite: false`, `noEmit`, `rootDir: "."`,
includes `src/**` + `test/**` (or `tests/**`) + `vitest.config.ts`). New
`typecheck:tests` npm script per package. New CI step `npm run
typecheck:tests --workspaces --if-present`. |
| 2 | Build/publish scripts (`scripts/*.{cjs,mjs}`,
`packages/argent/scripts/*.cjs`, `packages/skills/scripts/install.js`) —
JS, never typechecked. `sync-readme.cjs` runs in `prepack`;
`postinstall.cjs` runs on every install; `bundle-tools.cjs` is
build-critical. A typo only surfaces at runtime. | Root
`tsconfig.scripts.json` with `allowJs` + `checkJs` + `noImplicitReturns`
covers all script paths. Root `typecheck:scripts` npm script. New CI
step. Per-file `// @ts-check` comments removed — tsconfig drives
inclusion. |
| 3 | `argent-cli` had no `typecheck:tests` script. When tests get added
there they'd silently bypass the gate. | Added `tsconfig.test.json` +
`typecheck:tests` script. `--if-present` would have skipped it anyway,
so this is future-proofing. |
| 4 | Local `prettier --check .` walked into the `argent-private`
submodule and reported violations there. CI didn't see this because
`actions/checkout@v4` doesn't init submodules — so local `prettier
--check` did not match CI behavior. | New `.prettierignore` excludes the
submodule plus `dist/`, `node_modules/`, lock files, `tsbuildinfo`.
No-op in CI; aligns local with remote. |

## Test type errors fixed

These were already present. The `npm test` passed because esbuild
stripped the offending types:

- **argent-installer**: widen `scope` literal type for
dead-code-elimination guard; non-null `addAllowlist!`/`removeAllowlist!`
(optional methods on adapter).
- **argent-mcp**: add `.js` extensions for NodeNext module resolution.
- **registry**: broaden `StaticBlueprintResult` blueprint api from `{
id; deps? }` to `Record<string, unknown>` so existing tests can pass
arbitrary api shapes.
- **tool-server**:
- non-null `zodSchema!` on tool defs that always carry one
(`launchAppTool`, `restartAppTool`, etc.)
- cast sentinel `"ignored"` strings to `DeviceInfo` in factory-rejection
tests
- match the 4-arg `dispatchByPlatform<IosServices, AndroidServices,
Params, Result>` signature (tests still passed the old 3-arg shape)
- `assertFlowRunResult` type guard for the `FlowRunResult |
FlowPrerequisiteNotice` union before `.steps` access
- swap broken `typeof import("supertest").default` for static `import
supertest` (supertest is `export = supertest`)
- fill in the 4 fields missing from a `NativeProfilerSessionApi` mock
(`xctraceProcess`, `recordingTimedOut`, `recordingExitedUnexpectedly`,
`lastExitInfo`)
- cast a vitest `Mock` to a plain function at one direct call site where
the `Procedure | Constructable` union loses callability
- fix `update-checker.test.ts` reading `../../package.json` (off-by-one
— went past tool-server; was relying on a Vite resolver quirk)
- replace one cross-package `../../../registry/src/index` import with
`@argent/registry` so registry's private `services` field isn't compared
between src and dist declarations

All fixes are type-level. Full `npm test --workspaces --if-present`
still reports **991 passed across 87 test files**.

</details>
2026-05-20 17:03:34 +02:00
Filip131311 f7348886d2 fix: new describe crashes 2026-04-07 19:09:12 +02:00
Ignacy Łątka 445bc044c1 feat: Add update system to Argent (#78)
* feat: add update notification and update-argent tool

Injects an update note into HTTP responses when a newer version of Argent
is available, forwards the note through the MCP server so agents see it
prepended to tool results, and adds an update-argent tool that agents can
call (with explicit user consent) to apply the pending update.

* test: add edge-case tests for update note injection and update-argent tool

* fix: guard update-argent against double-spawn and no-op calls, use argent binary

* feat: auto-suppress update note for 30min after delivery, add dismiss-update tool

* fix: correct npm package name to @software-mansion/argent

* fix: use semver ordering for version comparison, guard double-start, handle response stream errors

* feat: add update notification and update-argent tool

Injects an update note into HTTP responses when a newer version of Argent
is available, forwards the note through the MCP server so agents see it
prepended to tool results, and adds an update-argent tool that agents can
call (with explicit user consent) to apply the pending update.

* test: add edge-case tests for update note injection and update-argent tool

* fix: guard update-argent against double-spawn and no-op calls, use argent binary

* feat: auto-suppress update note for 30min after delivery, add dismiss-update tool

* fix: correct npm package name to @software-mansion/argent

* fix: use semver ordering for version comparison, guard double-start, handle response stream errors

* fix unresolved conflicts

* add yarn to gitignore

* cleanup typecasting

* isolate update note builder

* remove unused import

* fix package name

* change update rate to once per day

* add note against using permanent storage mediums

* update tool descriptions

* fix tests
2026-04-07 17:42:59 +02:00
Filip131311 c8e8669c56 Revert "feat: add update-argent tool and per-response update notification (#74)"
This reverts commit c9a673e97a.
2026-04-07 10:00:10 +02:00
Ignacy Łątka c9a673e97a feat: add update-argent tool and per-response update notification (#74)
* feat: add update notification and update-argent tool

Injects an update note into HTTP responses when a newer version of Argent
is available, forwards the note through the MCP server so agents see it
prepended to tool results, and adds an update-argent tool that agents can
call (with explicit user consent) to apply the pending update.

* test: add edge-case tests for update note injection and update-argent tool

* fix: guard update-argent against double-spawn and no-op calls, use argent binary

* feat: auto-suppress update note for 30min after delivery, add dismiss-update tool

* fix: correct npm package name to @software-mansion/argent

* fix: use semver ordering for version comparison, guard double-start, handle response stream errors
2026-04-06 20:49:06 +02:00