2 Commits

Author	SHA1	Message	Date
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
Paweł Fornagiel	8a3b5a1ff7	fix(react-profiler): self-bootstrap DevTools backend when no client is attached (#205) ... ## Summary `react-profiler-start` failed in any RN bridgeless dev build with no external React DevTools client (Fusebox React tab / `npx react-devtools`) connected — returning the misleading error *"No React renderer interface attached yet. Wait for the app to render its first commit and retry."* even when the app had rendered. This PR makes the tool self-bootstrap the DevTools backend so profiling works out of the box, and replaces the one-size-fits-all error with mode-specific actionable messages. Companion to #199 (schema fix). With both landed, `react-profiler-start` is end-to-end usable from MCP clients against a vanilla RN 0.76 dev app with zero extra setup. > [!IMPORTANT] > The bootstrap path mutates global runtime state (Bridge + Agent + hook subscriptions via `connectWithCustomMessagingProtocol`). Unit tests cover every reason branch, but the live behavior — especially coexistence with a later-attached `npx react-devtools` client and the absence of subscription leaks across repeated start/stop cycles — must be **smoke-tested against a working RN application** before this merges. ## Root cause argent's profiler delegates React commit capture to the in-app DevTools backend via `rendererInterface.startProfiling(...)`. The two maps on `__REACT_DEVTOOLS_GLOBAL_HOOK__` look similar but are populated by different actors: | Map | Populated by | When | |---|---|---| | `hook.renderers` | React itself via `hook.inject(renderer)` | First render | | `hook.rendererInterfaces` | DevTools backend's `attach(hook, id, renderer, global)` (in `react-devtools-shared/src/backend/renderer.js`) | When `initBackend(hook, agent, global)` runs | In an RN 0.76 bridgeless dev build, `setUpReactDevTools.js` runs at bundle load and calls `react-devtools-core.connectToDevTools({ host: 'localhost', port: 8097, websocket })`. That opens a WebSocket; `initBackend` only runs on `ws.onopen`. With no DevTools client listening on 8097, the connection never opens, `initBackend` never runs, `rendererInterfaces` stays empty — but `hook.renderers` is populated because React doesn't care about the backend. So `rendererInterfaces.size === 0` while `renderers.size === 2` is the steady-state of a dev build with no DevTools client. argent's pre-flight check saw the empty map and threw with a misleading message ("wait for first commit") that conflated three distinct failure modes: 1. App hasn't rendered yet (legitimate wait). 2. App rendered but no DevTools backend attached (this PR — the common case). 3. Production build / no `react-devtools-core` in bundle (different recovery). Direct CDP verification of the hook from the bug state: ```js { rIds: [1, 2], riIds: [], reactDevtoolsAgent: undefined } ``` ## Fix When the pre-flight detects `rendererInterfaceFound: false` with the hook present, `react-profiler-start` now runs a self-bootstrap script (`BOOTSTRAP_DEVTOOLS_BACKEND_SCRIPT`) that: 1. **Looks up `react-devtools-core` in the Metro module registry** by scanning `__r.getModules()` entries for a `verboseName` matching `react-devtools-core/dist/backend`. Handles both `Map<id, meta>` (modern Metro) and array-of-meta (older Metro) shapes — mirrors the pattern in `debugger-component-tree`. 2. **Calls `connectWithCustomMessagingProtocol`** (rdt-core ≥5.1) with no-op `onSubscribe` / `onUnsubscribe` / `onMessage` handlers. The function internally constructs a `Bridge` + `Agent` and runs `initBackend(hook, agent, window)`, which iterates `hook.renderers` and calls `attach()` on each — exactly the call path that populates `rendererInterfaces`. The fake wall discards every outbound message, which is fine because argent calls into the rendererInterface directly and never reads from the bridge. 3. **Returns one of seven distinct reasons**, each mapping (via `bootstrapFailureMessage` in `utils/react-profiler/devtools-bootstrap.ts`) to its own actionable error message: - `no-hook` → "production build" guidance - `no-renderers` → "wait for first commit" - `no-metro-modules` → bundle isn't Metro-style - `no-rdt-module` → react-devtools-core stripped from bundle (production) - `unsupported-rdt-version` → rdt-core <5.1 (RN <0.74) → fall back to `npx react-devtools` - `metro-scan-error` / `bootstrap-threw` / `bootstrap-no-effect` → unexpected, with verbatim error text After a successful bootstrap, `REACT_NATIVE_PROFILER_SETUP_SCRIPT` is **re-run** to install argent's `__argent_startWrapped__` wrappers on the freshly-attached interfaces. Without this re-run, `buildStartScript`'s post-start `__argent_isProfiling__` check would falsely report `startProfiling-threw` (the wrapper sets the flag; an unwrapped interface leaves it false even on a successful start). ### Idempotency / leak protection `initBackend` adds subscriptions to `hook.sub('renderer', ...)` on every call, so naive re-invocation would leak listeners. The script short-circuits with `reason: 'already-attached'` when `rendererInterfaces.size > 0`, so repeated `react-profiler-start` calls within a session never re-bootstrap. Verified empirically: listener counts stay flat across multiple start/stop cycles. ### Coexistence with external DevTools If `npx react-devtools` is later started while argent has already bootstrapped, RN's bundled reconnect path calls `connectToDevTools` again, which goes through `attachRenderer` → sees `rendererInterface != null` (argent's already there) → re-emits `renderer-attached` without re-attaching. Both backends coexist; argent talks to the renderer interface directly, so the active "agent" is irrelevant to profiling. ## Files changed - `packages/tool-server/src/utils/react-profiler/scripts.ts` — adds `BOOTSTRAP_DEVTOOLS_BACKEND_SCRIPT` (the injected IIFE that does the Metro scan + connect call). - `packages/tool-server/src/utils/react-profiler/devtools-bootstrap.ts` *(new)* — `BootstrapResult` type, `BootstrapReason` union, and the `bootstrapFailureMessage()` helper that maps each reason to its agent-facing error. Mirrors the `session-ownership.ts` convention (types + pure helpers as a sibling of `scripts.ts`, unit-testable without CDP / vitest mocking). - `packages/tool-server/src/tools/profiler/react/react-profiler-start.ts` — calls bootstrap on empty `rendererInterfaces`, re-runs setup, throws via the imported `bootstrapFailureMessage`. Also replaces two pre-existing error strings ("hook not present", `how_to_reclaim`) with the same agent-friendly phrasing used elsewhere in this PR. - `packages/tool-server/test/react-profiler/bootstrap-devtools-backend.test.ts` *(new)* — 12 unit tests covering every reason branch (Map + array Metro registries, require() throws, getModules throws, connect throws, connect is a no-op, idempotent already-attached path). ## Manual verification (iPhone Air, RN 0.76.7 bridgeless, no external DevTools) 1. Reload bundle → confirm `rIds: [1,2], riIds: []` (bug state). 2. Pre-fix: `react-profiler-start` returns *"No React renderer interface attached yet…"* 3. Post-fix: `react-profiler-start` returns `{ started_at, hermes_version: "for RN 0.76.7", detected_architecture: "bridgeless" }`. Hook now shows `riIds: [1,2]`, `__argent_startWrapped__: true` on each, `reactDevtoolsAgent` set, `__argent_isProfiling__: true`. 4. Forced a React state-hook dispatch during the session and stopped: `total_react_commits: 2`, `unattributed_ms: 8.41` — real commit data captured through the bootstrapped backend. 5. Second `react-profiler-start` in same session returns success via `already-attached` early-exit (verified directly against the script). Hook listener counts (`renderer`, `renderer-attached`, `react-devtools`) stable across multiple cycles — no subscription leak. 6. Simulated `no-renderers` by clearing `h.renderers` → got *"React has not rendered yet, so there is nothing to profile. Ask the user to wait until the app shows React content…"*	2026-05-15 16:52:56 +02:00