mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/flow-dir-export-throw
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 filip131311
|
b232114422 |
docs: audit every comment in src against the code it describes (#931)
Audits every comment in `src/` and `scripts/` against the code it
describes.
**462 files, 57 commits, net −8,194 lines.** Comment text only — the
whole branch is code-identical to `main`.
## Method
One subagent per file, strictly sequential. Scope was `src/` +
`scripts/` (459 files); three more files were added at the end because
they carried dead references of the same kind — two test headers citing
design docs that do not exist, and `publish-npm.yml` citing a retired
workflow. Each agent verified every comment — line, block, JSDoc, file
header, trailing — against the surrounding code and the rest of the
repo, following identifiers, paths, tool ids, config keys, env vars and
issue links to see whether they still exist and still behave as
described. Rules:
- **A false or misleading comment is deleted, not reworded.** If a claim
could not be confirmed by reading the source, it went. That is why the
deletion count is so much larger than the rewrite count.
- Survivors are cut to the shortest form carrying something the code
does not already say. Restatement, preamble, hedging, changelog prose
and ASCII banners are gone; the non-obvious *why* stays.
- Preserved byte-identical: license headers, pragmas and directives
(`@ts-*`, `eslint-disable`, shebangs, `/// <reference>`), JSDoc tag
tokens, everything inside a string or template literal, and the sole
comment inside an otherwise empty block (ESLint `no-empty` counts a
comment-bearing block as non-empty).
## Verification
Every file passed two independent gates before being recorded as done:
1. `comments-only` — the required check.
2. A second comment-stripping comparator with a proper mode stack,
written for this pass because `comments-only`'s flat scanner desyncs on
nested template literals and quote-bearing regex literals and then
reports comment lines as code changes. Two files hit that false FAIL
(`utils/android-profiler/pipeline/index.ts`,
`scripts/extract-tools.mjs`); in both the "changed code" it printed was
literally `//` lines, and the second checker confirmed the code was
byte-identical.
After the last file, all 462 changed files were re-checked against
`main` with the same comparator, rather than trusting any agent's
self-report. **459 code-identical; 2 are non-code (`.svg`, `.md`); 1
intentional.**
The intentional one is `packages/argent/scripts/bundle-tools.cjs`: the
changed template literal *is* the comment header of the file it
generates, `packages/native-devtools-android/src/bundled-meta.ts`.
Fixing only the generated file would have been reverted by the next
build, so the generator changed too — and it has been verified to
reproduce the committed generated file byte-for-byte.
## Representative false claims removed
Not wording nits — statements a reader would have acted on:
- **Reversed directions.** `proxyStart`'s JSDoc had the tunnel backwards
(it is a reverse tunnel: the host binds first and the simulator dials
in). A `paste()` doc had the pasteboard copy direction reversed.
- **Contradicted by the code below it.** A timeout budget multiplied by
three where the probes run concurrently — the same comment said so six
lines later. A "warn once" that warns on every call. A "binary search"
that is a linear scan.
- **Named things that do not exist.** A `vega-fast-cli` binary, a
`finish-recording.ts`, a `publish-next.yml` workflow, two
`profiler-react19-*.md` design docs, a `DebuggerTarget.ts`, a commit
hash git does not know, two tool ids, an `ensureEnv` cycle.
- **Wrong by construction.** "Welford accumulators" across four files
where the code keeps naive `n`/`sum`/`sumSq`; `sum`/`sumSq` documented
over `actualDuration` when reduce sums `selfDuration`; a strict-mode
halving written `n/2` where the code ceils; field docs listing enum
values the producers never emit.
- **Guarantees the code does not make.** A validation matrix claiming to
cover "EVERY tool" that skips flagless ones; a Pareto cutoff that
`slice(0, 20)` makes inert; an idempotence claim where the real rule is
at-or-ahead; a capability note describing a clean 400 the shape-based
device resolver can never produce.
- **Unverifiable assertions** about prebuilt binaries, external CLIs and
the cloud SDK — deleted rather than kept as folklore, since nothing in
the repo can confirm them.
- **Stale numbers**: invented Android tool versions, hard-coded tool
counts and description lengths that had drifted.
## Review
A Fable agent reviewed both halves adversarially for over-deletion,
misread code, `no-empty` hazards and byte-identity violations.
Second-half verdict: **SHIP**, with two one-line restores, both applied
in the final commit — the `npm view ""` rationale behind a blank-token
guard, and the note that `argent-mcp` keeps a copy of
`SECRET_PLACEHOLDER_MARKER` it cannot import.
## Code issues surfaced but deliberately not fixed
This pass changes comments only. Eight genuine findings are logged for a
follow-up:
1. `telemetry/src/consent.ts` — a non-ENOENT read error returns null and
falls through to the default-on path, so file errors *can* silently flip
telemetry on.
2. `chromium-server/navigation.ts` — `navigate()` is reachable from
`POST /api/navigate` with only a `typeof === "string"` check; open-url's
schema is a bare `z.string()`, so the "already validated by zod" premise
never held.
3. `http.ts` — `constantTimeEqual` returns early on a length mismatch,
so the auth token's length is observable.
4. `describe/index.ts:~114` — the ios-remote branch passes `{ isTvOs:
false }` unconditionally, so a remote tvOS simulator takes the iOS
ax-service path, though `isRemoteTvOsSimulator` exists and shake/paste
do use it.
5. `devices/boot-device.ts` — `-crash-report-mode never` is passed
unconditionally *and* appended again by the feature-detecting path, so
every emulator spawn passes it twice.
6. `react-profiler/pipeline/04-rank.ts` — `PARETO_THRESHOLD_PCT` is
dead: `slice(0, 20)` always wins.
7. `reaped-sessions.ts` — a user-facing hint string tells the agent that
`react-profiler-start { force: true }` disposes the debugger and
profiler session; it does not. Left byte-identical because it is a
string literal, not a comment.
8. `utils/simctl-backend.ts` — `localSimctl` is exported with no
importers anywhere.
## Docs
No documentation change is needed: this pass touches only source
comments, and no user-facing capability, tool, CLI flag, config key or
flow-file behaviour changed.
---------
Co-authored-by: filip131311 <f.kaminski2000@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Kacper Kapuściak
|
36aa14a5fc |
docs: add Argent documentation (#814)
Adds the Argent documentation site, published to [docs.swmansion.com/argent](https://docs.swmansion.com/argent): a branded Docusaurus project under `packages/docs/`, the full first set of pages, and the CI that checks and deploys it. <img width="1512" height="945" alt="Screenshot 2026-08-21 at 13 05 04" src="https://github.com/user-attachments/assets/4607e143-4359-49a8-93a2-aaa3af4c9534" /> ## Site A standalone Docusaurus 3.9 project, built on the shared Software Mansion docs theme ([`@swmansion/t-rex-ui`](https://www.npmjs.com/package/@swmansion/t-rex-ui)) and restyled for Argent: - **Palette** from [argent.swmansion.com](https://argent.swmansion.com): dark `#0D0F26`, blue `#99DAFF`, lavender `#E4E1FF`, light `#FEFEFE`, mist `#F1F1F1`. The `--swm-*` token names stay, since the shared theme resolves against them. - **Typography**: DM Sans and DM Mono, matching the landing page. - **Sidebar** with lucide icons on every page entry (`sidebar_custom_props.icon` in the front matter, registered in `src/theme/SidebarIcon`), the logo kept on the page background and the panel inset from the edge. - **Video component** for embedded screen recordings, with `scripts/encode-video.sh` producing a web-sized MP4 and a poster frame for each clip in `static/video/`. - **Copy page button** and a few theme fixes: inline code badges in tables, paginator hover. - **No landing page.** That stays at argent.swmansion.com. The root route redirects to Getting started. ## Content Three sections in `docs/`: - **Fundamentals**: getting started, installation, supported platforms. - **Features**: interacting with apps, flows, network, screen recording, lens, visual regression, profiling, debugging. Conceptual overviews with recordings, each linking to the tools reference. - **Reference**: tools, CLI, configuration, flow YAML, editors, telemetry. Prose follows Simplified Technical English. The conventions (style, front matter, icons, checks) are written down in `packages/docs/CLAUDE.md`, and a root `CLAUDE.md` adds a checklist so that code changes to tools, CLI, configuration or flows update the matching docs page in the same pull request. ## CI and deploy - `Docs build` runs `format:check`, `lint`, `typecheck` and `build` on every pull request that touches `packages/docs/`. The build has `onBrokenLinks: "throw"`, so it catches links left dangling by a moved page. - `Docs publish` deploys `packages/docs/build` through the GitHub Actions Pages source on every push to `main` that touches the docs. The repository Pages source needs to be set to **GitHub Actions** for the first run. ## Notes for review - The site is excluded from the root `packages/*` workspaces (`!packages/docs`) and keeps its own `package.json` and `package-lock.json`, so its dependency tree stays out of the toolkit's lockfile. Root Prettier, ESLint and knip ignore it; it formats and lints itself with the repo `.prettierrc` and its own `eslint.config.mjs`. `check-workspace-versions.mjs` skips it so its `0.0.0` version is not read as drift. - `webpack` is pinned to `5.105.4` and `@docusaurus/plugin-content-docs` / `theme-common` to `3.9.2` via `overrides`. Newer webpack fails Docusaurus 3.9's ProgressPlugin option validation, and a hoisted `plugin-content-docs@3.10.2` produced a duplicate React context that crashed SSR. - **Search is not wired up yet.** The shared theme always mounts a DocSearch bar, so an `algolia` block has to be present. It reads `ALGOLIA_APP_ID` / `ALGOLIA_API_KEY` / `ALGOLIA_INDEX_NAME` from the environment and the bar stays hidden until Argent has its own DocSearch application. Follow-up PR. ## Testing - `npm run format:check`, `npm run lint`, `npm run typecheck` and `npm run build` pass in `packages/docs/`. - Root `prettier --check`, `eslint` and `npm run knip` pass. - Walked the served production build in the browser in light and dark themes while iterating: root redirect, docs pages, sidebar icons, videos, TOC, footer. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ignacy Łątka
|
7d51d880af |
fix(scripts): stop the version guard passing a workspace it failed to read (#608)
Follow-up to #598, which merged while these were still being verified.
Three defects in the version guard it added, plus the test gaps that let
them through.
### An unreadable `packages/*` manifest was silently dropped
The scan's `catch { continue; }` swallowed every `readFileSync` +
`JSON.parse` failure, not just a missing file, so a manifest that is
*present but unparseable* left the lockstep comparison entirely — and
the run then affirmed a version it never checked:
```console
$ node scripts/check-workspace-versions.mjs # BOM on a drifted packages/registry manifest
All workspace packages and server.json are at 0.18.0.
exit=0
```
Control: identical tree with the BOM removed → `exit=1`, `0.17.0:
@argent/registry`.
A BOM is the trigger that matters. A trailing comma is caught upstream
(`npm ci` fails `EJSONPARSE`, and `prettier --check .` flags the file),
and `chmod 000` is a workstation-only state since `actions/checkout`
writes 644. A BOM is invisible to both — `prettier --check` reports the
file clean and `npm install` parses it — and `repo-hygiene.yml` runs the
guard on a bare checkout with no `npm ci` in that job, so nothing in the
pipeline sees it.
Now only `ENOENT` is skipped, which is what the comment (`// directory
without a package.json`) ever meant to cover; anything else fails naming
the file. This half is pre-existing — the same `catch` sits on the merge
base and fails open there identically — but #598 added `readTrackedJson`
precisely so a missing/malformed tracked JSON file fails on-message, and
applied it to two files while the other sixteen manifests kept the
silent skip.
The same fail-open had a second, subtler window: a manifest whose *parse
succeeds* but returns a non-object (`null`, a bare scalar, or an array).
`JSON.parse` accepts all three, so none of them ever reached the
`catch`, and the `manifest?.version` read then skipped the package as if
it carried no version — a present-but-unusable manifest still leaving
the comparison and affirming a version it never read. That shape is
exactly what `readTrackedJson`'s shape check rejects for `server.json`
and `packages/argent`, so the scan now rejects it the same way
(on-message, naming the file) with tests for the `null` and array forms.
### The failure report told you to edit the wrong file
Three mismatch lines report a problem in `packages/argent/package.json`
(no `version`, no `name`, no `mcpName`), but the trailer read
"server.json is not under `packages/*` … edit it by hand to match
`packages/argent/package.json`" — the opposite of the fix:
```console
$ node scripts/check-workspace-versions.mjs
server.json is out of sync with packages/argent/package.json:
packages/argent/package.json has no mcpName — the registry reads it off the published tarball to prove the namespace
server.json is not under packages/*, so a workspace bump leaves it behind — edit it
by hand to match packages/argent/package.json.
```
Every mismatch line already begins with the file it concerns, so the
trailer now says that instead of naming one file for all of them.
### An array `packages[]` entry bypassed the shape guard
`typeof [] === "object"`, so `["@swmansion/argent", "0.18.0"]` fell
through to the field comparisons and came out as two `undefined`
mismatches rather than the wrong shape it is.
### Test gaps
Each of the following was mutated and survived the whole suite plus
`eslint` and `tsc` before this change, and is killed by exactly the test
added for it now:
| mutation | was |
| --- | --- |
| `catch` swallows every manifest read error | 27/27 green |
| `readTrackedJson(argentManifestPath)` → raw `JSON.parse` | 27/27 green
|
| drop `names.sort()` in the drift report | 27/27 green |
| `manifest.name ?? entry.name` → always the manifest name (drops the
`entry.name` fallback) | 27/27 green |
| version-less manifest joins the version map | 27/27 green |
| either failure report's guidance trailer deleted | 27/27 green |
| drop the array arm of the entry shape guard | 27/27 green |
`readTrackedJson`'s malformed-JSON arm is unreachable from the
`packages/argent` side — the scan reads the same file first and stops
there — so the two tests added for that call site cover the arms that
are reachable (missing, and not-an-object).
The `names.sort()` gap was a fixture artifact: the only two-package
fixture used directories `alpha`/`argent` holding
`@argent/alpha`/`@swmansion/argent`, already in sorted order, so the
sort was a no-op. The fixture now puts `@zzz/scanned-first` in directory
`aaa`, so scan order and sorted order disagree.
Same story on the name-fallback row: the `manifest.name ?? entry.name`
*fallback* arm (a versioned manifest carrying no `name`) had no fixture
at all, so writing it as `manifest.name` (dropping the fallback → prints
`0.16.0: undefined`) survived the whole suite. The reverse mutation —
`entry.name` only — was never the gap: the merge base's fixture in
directory `registry` named `@argent/registry` already asserts the
manifest name, so `@argent/registry`-expecting lines fail under it. The
new fixture (a versioned manifest with no name) is what pins the
fallback arm.
### Verification
- `test:scripts` 91/91, `eslint --max-warnings 0`, `tsc -p
tsconfig.scripts.json`, `prettier --check` — all clean on top of `main`
at
|
||
|
Ignacy Łątka
|
7e4ece697d |
chore: add server.json for the official MCP registry (#598)
Adds the two files the official MCP registry
(`registry.modelcontextprotocol.io`) needs in order to list argent.
Argent is not in that registry today
(`?search=io.github.software-mansion` returns 0 servers).
The registry is metadata-only — it points at the npm package rather than
hosting anything.
### `server.json`
Describes the server and how a client starts it: `npx @swmansion/argent
mcp`. The `mcp` positional is the dispatcher case in
`packages/argent/src/cli.ts:137`, and it is the same subcommand `argent
init` writes into every client config —
`packages/argent-installer/src/mcp-configs.ts:130-141` spells it four
ways (`node <path> mcp`, `yarn argent mcp`, `npx --no-install argent
mcp`, and the global `{ command: MCP_BINARY_NAME, args: ["mcp"] }`,
where `MCP_BINARY_NAME` is `"argent"`). None of them is literally the
npx-by-package-name form in `server.json`, which is the form the
registry expects a client to construct from `identifier` + `runtimeHint`
— so it is verified below by running it, not by matching a string.
`repository.id` is the GitHub repo id (`1164885639`); the schema
recommends it so a registry can detect a delete-and-recreate of the
repo.
### `mcpName` in `packages/argent/package.json`
The ownership proof. The registry fetches the npm package and requires
`mcpName` to equal the server name in `server.json` before it will
accept a publish under `io.github.software-mansion`. It is the field the
registry's own quickstart names for npm packages. Nothing in the publish
path drops it: `scripts/pack-mcp.cjs` shells out to `npm pack` without
rewriting the manifest, and the canary path's
`scripts/next-canary-version.mjs --write` round-trips the JSON and only
sets `version`.
### Namespace
`io.github.software-mansion/argent` — GitHub OAuth auth. The registry
grants an org namespace only to org **Owners**; plain membership is no
longer enough. If no Owner is available to run the publish, the
alternative is DNS auth via an apex TXT record on `swmansion.com`, which
changes the name to `com.swmansion/argent` (and the `mcpName` value with
it).
### Version sync
`server.json` sits at the repo root, outside `packages/*`, so a release
bump would have left its two `0.18.0`s untouched — and the registry
rejects a publish whose `packages[].version` names a version npm has
never seen. `scripts/check-workspace-versions.mjs` (already run by the
Static checks job) also holds `server.json` to the version in
`packages/argent/package.json` — the package it names — and pins the two
cross-file links a publish depends on: `name` == `mcpName`, `identifier`
== the published package name. Both halves are reported in one run, so a
half-finished bump doesn't cost a CI round-trip per file.
### Not in this PR
- **Publishing.** Nothing has been published; this is just the manifest.
Publishing is `mcp-publisher login github && mcp-publisher publish`, and
it can only succeed **after** a release whose npm tarball carries
`mcpName` — i.e. not against `0.18.0`, which is `latest` (published
2026-07-30) and predates `mcpName` — `curl
registry.npmjs.org/@swmansion%2Fargent/0.18.0 | jq .mcpName` → `null`.
The first publishable release is the next bump.
- **`github.com/mcp`.** A separate, manual step. Onboarding a new server
there is hand-curated: per
[github/github-mcp-server#1257](https://github.com/github/github-mcp-server/discussions/1257)
(trent-j, a collaborator on that repo, 19 May 2026) "onboarding a new
server is still a manual curation process today. Once a server has been
onboarded, newly published versions from the OSS registry should sync
from there." The nomination channel is `partnerships@github.com`, from
GitHub's [registry how-to
post](https://github.blog/ai-and-ml/generative-ai/how-to-find-install-and-manage-mcp-servers-with-the-github-mcp-registry/)
(24 Oct 2025) — that address does not appear anywhere in the discussion
thread.
### Verification
- `mcp-publisher validate` (v1.8.0, which posts the file to the live
registry) → `server.json is valid`. Mutation-checked what that actually
covers: removing `transport` fails it and removing `$schema` is a 422,
but a foreign namespace (`io.github.anthropics/argent`), a nonexistent
npm identifier and a `99.0.0` package version all still pass — so it
checks shape only, and the ownership/existence checks happen at publish
time. Also validated offline against the pinned `2025-12-11` schema.
- Ran the entrypoint the file declares. `npx -y @swmansion/argent@latest
mcp` fed an `initialize` over stdio answered
`{"protocolVersion":"2024-11-05","capabilities":{"tools":{}},"serverInfo":{"name":"argent","version":"0.18.0"},"instructions":"…"}`,
with stderr empty.
- Mutation-checked the new version guard: drifting `server.json`
`version`, drifting `packages[0].version`, deleting `mcpName`, and
typo-ing `identifier` each fail `check-workspace-versions.mjs` with the
specific line; the pre-existing packages/* drift check still fires on
its own.
- Asserted across the two files: `mcpName` == `server.json` `name`,
`identifier` == package `name`, all three versions equal, description 89
chars (registry limit is 100).
- `prettier --check` and `eslint` clean on both changed files.
- Mutation-checked the guard as a whole — 22 mutants, 1 survivor.
Killed: reverting the self-invocation guard to a raw `argv[1]` compare
and making it never match; `exit(1)` → `exit(0)` on the failure path;
dropping either `failed = true`; exiting early on `packages/*` drift
instead of collecting; `packages.slice(0, 1)`; hardcoding `packages[0]`
in either message; dropping the no-packages-array guard, the
non-object-entry guard, the non-object-JSON guard, and the presence
checks for `version`, `name` and `mcpName`; `continue` → `break` in the
`packages/*` scan (both arms) and in the entry loop; dropping the
on-message read, the `git checkout` hint, and the per-version name
accumulator. The survivor is the `packages/*` readdir sort: it fixes the
scan order rather than any output, so no single run can observe it — it
is there so a scan that stops early drops the same packages on every
machine instead of a filesystem-dependent set.
- Reproduced the symlink no-op on the real repo: with
`packages/registry` at `9.9.9` and `server.json` at `0.17.0`, `node
/tmp/argent-alias/scripts/check-workspace-versions.mjs` now exits 1 and
prints both problems, where before it exited 0 with no output at all.
- Closed the vacuous-pass class in `serverJsonMismatches`: all four
comparisons it makes were bare `!==`, so a field absent from *both*
sides compared equal and passed. A `packages/argent/package.json` with
no `version` printed `All workspace packages and server.json are at
undefined.` and exited 0; with no `name`, a `server.json` naming nothing
installable was reported in sync. `version` and `name` now require
presence, as `mcpName` already did.
- `readTrackedJson` guarantees an object, not just parseable bytes — a
`server.json` holding `null` used to reach the field reads and throw,
and `"0.18.0"` or `[]` produced mismatch lines that never said the file
was the wrong shape.
- Full CI parity locally: `test:scripts` 85/85, `tsc -p
tsconfig.scripts.json`, `eslint --max-warnings 0`, `prettier --check`.
|
||
|
Ignacy Łątka
|
dc0930d50c |
ci: prevent package-lock drift + add repo-hygiene checks (#362)
## Why We hit a mismatch between a locally-generated `package-lock.json` and the one on `main`. `npm ci` validates the lockfile loosely and accepts a stale lock that a fresh `npm install` would rewrite, so the drift slipped through. This adds CI to catch that, plus a few related common-mistake guards. ## What **`Lockfile` workflow** (Node 20) — runs `npm install --package-lock-only --ignore-scripts` and fails if `package-lock.json` changes. This is the gap `npm ci` leaves: it asserts the lock is exactly what npm produces, not merely "consistent enough". **`Repo Hygiene` workflow** — three fast static guards: - **Merge-conflict markers** — fail on any `<<<<<<<` / `=======` / `>>>>>>>` left in a tracked file. - **Focused tests** — fail on `describe/it/test/bench.only` in `*.test.ts(x)` / `*.spec.ts(x)`. Uses a POSIX-ERE word-boundary pattern so it actually fires under `git grep -E` (a `\b` pattern silently matches nothing there). - **Workspace version sync** — fail when `packages/*` versions drift, via `scripts/check-workspace-versions.mjs` (also `npm run check:versions`). To make versions consistent, `@argent/ui` is bumped `0.0.1 → 0.12.0` (it is `private`, so no publish impact). ## Verification (Node 20 / npm 10.8.2, matching CI) - Lockfile check: no false positive on `main`; idempotent; catches a tampered lock. - Merge-marker & `.only` greps: proven to catch real violations and reject lookalikes (`await.only`, `audit.only`, `foo.it.only`); clean across the repo. - `check-workspace-versions.mjs`: reports all packages at `0.12.0`; passes `eslint` and `typecheck:scripts`. - All new/changed files pass `prettier --check`; both workflow YAMLs parse. ## Note: npm version coupling The check runs on Node 20 (npm 10.8.2), matching the committed lock's format. npm 11 (Node 24) regenerates the same lock minus cosmetic `"peer": true` markers — no resolution change, but a contributor who regenerates on npm 11 would trip the strict diff. The repo pins no toolchain today (CI mixes Node 20 and 24). Pinning npm/Node repo-wide would fully close this; left out of scope here — happy to add it as a follow-up. |