Commit Graph

6 Commits

Author SHA1 Message Date
Ignacy Łątka a5c6730d05 feat(ios-profiler): attributable memory leaks via malloc_stack_logging (#351)
## What

Opt-in `malloc_stack_logging` flag on `native-profiler-start` that makes
iOS memory leaks **attributable** — they come back with a real
responsible frame + library instead of `<Call stack limit reached>`.

## Why

`native-profiler-start` attaches to the already-running app. iOS records
malloc allocation backtraces only when `MallocStackLogging` is set **at
process launch**, so an attached app has none: Instruments' Leaks
scanner finds the leaked blocks but can't attribute them and emits the
placeholder `<Call stack limit reached>` (that string is Instruments',
not ours).

## How

When `malloc_stack_logging: true`, the profiler cold-launches the app
**under** xctrace instead of attaching:

```
xctrace record --template <Argent.tracetemplate> --device <udid> \
  --env MallocStackLogging=1 --output <…> --no-prompt \
  --launch -- <App.app>
```

- `.app` path resolved via `simctl get_app_container`; the running
instance is terminated first for a clean cold start.
- The existing Leaks export / parser / render path is reused
**unchanged** — no parser changes.
- Default behaviour is untouched: attach to the running app (no
relaunch, no overhead). The flag is opt-in because it restarts the app
(loses state) and adds allocator overhead — for leak-attribution passes,
not CPU/hang work.
- The report relabels unattributable leaks with a hint to re-run with
`malloc_stack_logging: true`, instead of surfacing the raw placeholder.

## Evidence (real iPhone 16 sim, same scroll workload)

Before (attach) — every leak row:
```
responsible-frame="<Call stack limit reached>" responsible-library=""
```
After (`--env MallocStackLogging=1 --launch`) — same Leaks export:

| leaked-object | responsible-frame | library |
|---|---|---|
| Malloc 1008 B ×3 | `itanium_demangle::OutputBuffer::grow(...)` |
libc++abi |
| Malloc 48 B ×1 | `hermes::vm::JSTypedArrayBase::createBuffer(...)` |
hermes |

## Tests

- `test/ios-instruments/malloc-stack-logging.test.ts` — the malloc
launch argv (`--env MallocStackLogging=1`, `--launch -- <app>` last,
with and without `--notify-tracing-started`, `--no-prompt`) vs default
`--attach`; terminate-first / get_app_container ordering; failed-start
behavior; per-test `ARGENT_IOS_CAPTURE` scrubbing.
- `test/ios-instruments/leak-attribution-render.test.ts` —
unattributable vs attributed leak rendering, capture-mode-aware hints.
- `test/ios-instruments/load-freshness.test.ts` — per-capture residue
clearing on `profiler-load`, live/recovery-pending load refusals.
- `test/ios-instruments/stop-recovery.test.ts` — capture-mode pairing at
stop (clean and recovery paths).
- `test/profiler-combined-leaks.test.ts`,
`test/profiler-leak-stacks.test.ts`, `test/ios-leak-pipeline.test.ts` —
shared caveat line, evidence-first hints, attribution-normalized
grouping.
- Full `test/ios-instruments/` suite green (9 files / 70 tests); full
tool-server suite green; `tsc --noEmit` + `tsc -p tsconfig.test.json`
clean.

## Notes

E2E'd live twice during review (real iPhone 16 Pro / iPhone 17 Pro sims
on Xcode 26.6): guard refusals fire before the app is touched, the
`--device` cold launch's deadlock on degraded Xcode is confirmed (so the
guard is warranted), and the malloc argv/terminate/relaunch behavior
checked out on-device. Docs updated: `IOS_PROFILER_REFERENCE.md` and the
`argent-native-profiler` skill.

## Review follow-ups (post-approval hardening)

Later commits address the remaining review threads and several
subsequent multi-lens review passes:

- **Capture-mode is a real signal, end to end.** The session records the
mode of the in-flight recording at start
(`recordingMallocStackLogging`), pairs it with `exportedFiles` at stop
(`mallocStackLogging`), and freezes it into `parsedData` at analyze/load
— so the analyze report, the combined report, and the `leak_stacks`
drill-down all name the capture mode of the data they actually render
instead of inferring it (or mislabeling it after a newer capture
starts). One shared `renderUnattributedLeaksNote()` keeps the wording
identical across reports, and attribution evidence outranks the flag: a
resolved frame proves the target ran under malloc stack logging however
it was launched.
- **`profiler-load` restores a clean slate.** Loading clears all
per-capture residue (capture mode, `cpuFilterPid`, `wallClockStartMs`,
`traceFile`) so a loaded trace is never analyzed with a previous live
capture's PID filter, freshness anchor, or report path — and refuses to
load while a recording is in flight or a crashed capture awaits
`native-profiler-stop`'s partial-trace recovery, which the load would
otherwise orphan.
- **Failed starts are non-destructive.** Per-capture descriptors — and
the recovery flags that gate `native-profiler-stop`'s partial-trace
export — are stamped/reset only on a successful start, so a failed
attempt leaves the previous capture's still-loaded exports fully
described and a pending abnormal-end recovery intact.
- **Guards keep session state coherent.** `profiler-load`,
`native-profiler-analyze`, and `profiler-combined-report` all refuse
while a recording is in flight or a crashed capture awaits recovery, via
one shared `isCaptureInFlight`/`inFlightGuardMessage` helper whose
message distinguishes recording vs 10-min-cap timeout vs unexpected exit
(so the stated cause matches what `native-profiler-stop` reports next).
Proceeding would otherwise wedge the session (load) or render one
capture's frozen data under another's live fields (analyze/combined).
- **The combined report anchors on a frozen clock.** The recording's
start time is frozen into `parsedData` at analyze, and the iOS combined
report anchors its hangs to that frozen value rather than the live
session field — so a capture started after analyze can never shift a
real hang↔commit correlation into "unmatched". Android keeps its live
anchor (it re-derives hangs from the live trace). The combined-report
guard's recovery advice routes through `native-profiler-analyze`, the
only step that rebuilds `parsedData`.
- **Leak grouping is attribution-normalized.** `aggregateLeaks` keys on
`(objectType, frame-normalized-the-way-isLeakAttributed-matches)`, so
mixed unattributed spellings (`Unknown`, the sentinel, whitespace
variants) can't split one group per type.
- **The malloc launch target is unambiguous.** Since the cold launch
terminates the resolved app, `app_process` resolution prefers an exact
`CFBundleExecutable` match and, when builds still tie (dev + prod
sharing an executable/display name), refuses and points at the
globally-unique `CFBundleIdentifier` — which it also accepts as an
`app_process` — instead of killing whichever app the plist enumerates
first.
- **Report tables escape `|`.** Demangled C++ frames (`operator|`
included) are real headline content under malloc mode; every free-form
cell across the leak, CPU-hotspot, function caller/callee, and
thread-breakdown tables — symbol *and* thread/queue-name columns — is
escaped so a stray pipe can't shift the row's columns.
- **Android start is non-destructive too.** `startNativeProfilerAndroid`
now starts perfetto before mutating session state, mirroring iOS, so a
failed start no longer burns a prior capped capture's pending
partial-trace recovery.
2026-07-12 13:42:23 +02:00
filip131311 c8f6b4baf7 feat(ios-profiler): pluggable capture strategy with --all-processes fallback for the xctrace --device deadlock (#380)
## What

Encapsulates the iOS native CPU capture behind an `IosCaptureStrategy`
interface with **two interchangeable implementations**, selected per
environment. This is a drop-in refactor: the default path is
byte-identical to today's behaviour; a new fallback path is used only on
broken Xcode versions.

## Why

On **Xcode 26.4–27.0**, `xctrace record --device <sim>` **deadlocks at
the recording-start handshake** (a host-side xctrace regression: the
in-sim `DTServiceHub` waits forever for a `recording-started` reply the
regressed xctrace never sends). Capture never starts, so there's nothing
to recover — `native-profiler-start` just hangs until the cap. Older
xctrace (≤ 26.3) works fine, which is how we know the bug is in
xctrace's host side, not the simulator.

The only hang-free, Apple-entitled path that still profiles a simulator
app is to record the **host** with `--all-processes` (no `--device`).
Simulator apps run as ordinary host processes on the shared host kernel,
so host-wide kperf sampling captures the target app's threads too — at
the same ~1 kHz PET rate, with fully symbolicated user callstacks — and
the host path finalizes cleanly.

## How

- **`utils/ios-profiler/capture-strategy/`** (new): `IosCaptureStrategy`
interface + two impls:
- **`device`** (default): `xctrace --device <sim> --attach <pid|name>` —
record args are identical to the current code, so healthy environments
are unaffected. No post-filter (already scoped).
- **`all-processes`** (fallback): `xctrace --all-processes` (host-wide),
then filter the export to the app PID.
- **`select.ts`**: picks `device` vs `all-processes` from the active
Xcode version (26.4–27.0 ⇒ fallback), with an
`ARGENT_IOS_CAPTURE=device|all-processes` env override to force either
direction. So the new path runs **only on degraded environments** unless
explicitly overridden.
- **`platforms/ios.ts`**: builds the xctrace argv via
`strategy.buildRecordArgs(...)`, stores `strategy.cpuFilterPid(target)`
on the session, gates the cold-start retry on `strategy.attachesByName`,
and guards that the all-processes path has a running app PID to scope
by.
- **Pipeline**: `runIosProfilerPipeline(files, { cpuFilterPid })` →
`parseCpuFile/parseCpuXml(xml, targetPid)`. When a filter PID is set,
`time-profile` rows are kept only for that PID using the export's
`<process>`/`<pid>` id/ref table, preserving the existing two-pass
row↔sample alignment. No filtering when unset (device path), so existing
behaviour is unchanged.

Lifecycle (startup readiness, stop/finalize, export) and all analyses
are shared between strategies.

## Fidelity

`--all-processes` data fully supports the analyses the tool-server
provides — **hotspots, native call trees, per-thread breakdown, app
call-chains, hangs** (verified against the export schema and Argent's
parser). The only thing absent is **kernel callstacks**
(`cp-kernel-callstack` is empty for simulator processes) — **no Argent
analysis consumes them, and the `--device` path lacks them for
simulators too, so there is no regression.**

## Caveats / notes

- All-processes capture is system-wide; samples are filtered to the app
PID post-export. Trace files are larger and export takes a few extra
seconds.
- The target app must be on-CPU during capture to be sampled (true of
any sampling profiler).
- The Xcode-version range in `select.ts` is the currently-known-broken
set; revisit when Apple ships a fix (or just rely on the env override).

## Testing

- New `capture-strategy` module typechecks clean in isolation;
full-package `tsc` shows no errors in any changed file.
- Verified the `device` strategy emits the exact current `record` argv,
and `--all-processes` host capture finalizes hang-free with per-sample
symbolicated user stacks filterable to one PID.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-06-19 21:43:57 +02:00
filip131311 c01f41cd5f feat(remote): input-side file boundary — flows, screenshot-diff, profiler reports, workspace tools (#319)
## Why

PR #278 fixed the **output** half of the remote file boundary: files a
tool *produces* travel to the client as `ArtifactHandle`s. But seven
tools still break when the tool-server runs on a different machine than
the agent, in one (or both) of two ways:

- **Output-side leftovers** — `native-profiler-analyze` and
`screenshot-diff` still returned raw host paths (and embedded them in
report/summary prose), so the agent can't open them remotely.
- **Input-side gap** — nothing moved a file the *agent* owns onto the
tool-server. Any tool taking a client-local path as a parameter
(`screenshot-diff` baselines, the three flow tools' `project_root`,
`react-profiler-component-source`, `gather-workspace-data`) silently
read/wrote the wrong machine's disk. Flows were fully broken in remote
mode: the YAML landed on the server's disk under the agent's path.

## What

### Output side (apply the existing pattern)
- **native-profiler-analyze** — `reportFile` is now an `ArtifactHandle`
(mirrors `react-profiler-analyze`'s `fileArtifact()`); the inline report
now says *"use the Read tool on the `reportFile` path in this result"*
instead of embedding the server path.
- **screenshot-diff** — `diffPath` / `contextDiffPath` are now
`ArtifactHandle`s; the MCP adapter renders the context-diff image from
materialized bytes (legacy string-path results from older servers still
render co-located). The summary references the result fields instead of
embedding host paths.

### Input side (new, symmetric mechanism)
A declarative **file-input boundary**, mirroring the artifact gate in
the opposite direction:

- A tool declares `fileInputs: [{ target, path: "${param}…", kind }]` on
its `ToolDefinition`; the declaration ships to the client via `GET
/tools`.
- Before sending a call, the client (`@argent/tools-client`, shared by
CLI and MCP) interpolates each template from the args and replaces the
target arg with a wrapper: `{ __argentFileInput, path, size, mtimeMs,
content? }`. File bytes are base64-inlined **only when routed to an
external tool-server** (`argent link` / `ARGENT_TOOLS_URL`) — unlinked
local calls never pay for encoding.
- The server resolves wrappers **before zod validation**, so tools
always execute against a plain local path:
- path matches on the server's own filesystem (existence for dirs,
size+mtime for files) → used **in place**, zero copies — the exact dual
of the artifact materializer's co-located gate;
- otherwise `kind: "file"` content is materialized into a temp file
(with size-integrity check, 32 MB cap);
- `kind: "directory"` (trees that can't ride in a call) → 422 with
actionable remote-mode guidance;
- `kind: "probe"` → never fails; the tool learns host-presence via
`ctx.fileInputs` and adapts.
- The reverse channel: a tool whose output belongs in the **agent's
project** returns a `__argentClientFile` directive (path + content); the
client writes it and rewrites the directive to the path. Writes are
constrained to `**/.argent/flows/<safe-name>.yaml` so a tool-server
can't direct writes anywhere else on the client machine.

### Tool migrations
| Tool | Boundary use |
|---|---|
| `flow-start-recording` | `probe` on `project_root`: present → host
persistence exactly as before; absent (remote) → recording kept in
server memory, YAML returned as a client-write directive |
| `flow-add-step` / `flow-add-echo` / `flow-finish-recording` | follow
the session's persist mode; every mutation returns `savedTo` (path
locally, directive remotely) so the agent-side file is always current |
| `flow-execute` / `flow-read-prerequisite` | `file` input derived from
`${project_root}/.argent/flows/${name}.yaml` into an internal
`flow_file` param — read in place locally, uploaded when remote |
| `screenshot-diff` | `baselinePath`/`currentPath` as `file` inputs;
`outputDir` now **optional** (temp default) and probe-ignored when
absent on the host |
| `react-profiler-component-source`, `gather-workspace-data` |
`directory` gate — unchanged locally; clear error remotely instead of a
silent empty AST index / all-nulls snapshot |

### Version skew
Both halves degrade to today's behavior: an old client sends plain
strings (the resolver passes them through, tools take their legacy
paths), and an old server doesn't advertise `fileInputs` (the new client
never wraps). Wrappers on undeclared params fail the tool's own schema
validation, so nothing can be smuggled through free-form args.

## Tests

**Unit** (all green: registry 68, tool-server 992, tools-client 107,
argent-mcp 66, argent-cli 117):
- `tool-server/test/file-inputs.test.ts` — in-place gate, stat-mismatch
→ upload, truncated-upload rejection, directory gate, probe semantics,
undeclared-target smuggling.
- `argent-tools-client/test/file-inputs.test.ts` — wrapping (stat-only
vs inlined content), multi-param template derivation, explicit-override
respect, directive writes incl. path-validation refusals (relative,
traversal, outside `.argent/flows`, bad charset/extension).
- `tool-server/test/flows/flow-remote-recording.test.ts` — remote
recording stays in memory + returns directives, nothing written on the
host, replay/read from a boundary-resolved `flow_file`.
- Updated existing screenshot-diff / native-profiler tests to the
artifact-handle contract.

**Local / co-located (unlink scenario)** — isolated tool-server + CLI on
an iPhone 17 Pro simulator: full flow
record→finish→read-prerequisite→execute loop (YAML written via the same
path as before, in-place gate hit, no byte copies); screenshot-diff with
and without `outputDir`; wire-level checks of the unlinked client shape
(wrapper without content → in-place read) and the legacy plain-string
shape; missing-file → 422 with a precise message.

**Remote (Lima Linux VM as the agent machine, tool-server on the host
Mac, `--host` bind + bearer auth)** — via both the CLI and the real MCP
stdio server:
- flow recording from the VM: YAML lands **in the VM's project**,
`savedTo` rewritten to the VM path, *no* agent-path directories created
on the host; replay from the VM uploads the YAML and drives the host
simulator.
- `screenshot` materializes into the VM temp cache; `screenshot-diff`
round-trips both directions (VM baseline uploaded, live capture on host,
diff artifacts downloaded back; MCP returns `image,text` content with
the context diff inlined).
- `gather-workspace-data` / `react-profiler-component-source` with
VM-only paths → the new actionable 422.

## Notes
- `native-profiler-analyze` was not exercised live (xctrace export is
unusable on this machine's iOS 26.4 simulators — known, unrelated); its
change is the same one-line `fileArtifact` pattern
`react-profiler-analyze` already ships, covered by unit tests.
- `react-profiler-analyze`'s own `project_root` (AST resolve stage) is
left as-is deliberately: it degrades gracefully (report still renders)
and gating it would break currently-working remote analysis. Tracked as
follow-up alongside nested invocations (`run-sequence`/`flow-execute`
steps bypass the HTTP boundary by design).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-12 17:14:04 +02:00
Paweł Fornagiel 3ccd89f333 feat: add native android profiling (#275)
# Android native profiling via in-process Perfetto WASM

Connected with:
https://github.com/software-mansion/argent-private/pull/16

## TL;DR

Adds **Android** to the native profiler (previously iOS-only) and does
it with a
brand-new engine: instead of shipping Perfetto's `trace_processor_shell`
native
binary and forking it per query, we now run **Perfetto's trace-processor
compiled
to WebAssembly, in-process** inside the tool-server.

- **One ~13 MB `trace_processor.wasm`** runs on every OS/arch — no
per-platform
  binary, no subprocess, fully offline.
- The native profiler is refactored from an iOS-only implementation into
a
**platform-dispatch facade** (`platforms/ios.ts` +
`platforms/android.ts`) over
a new **`profiler-shared/`** layer, so iOS and Android share the
aggregator,
  types, lifecycle, and time-alignment code.
- New **release/pack pipeline** fetches + sha256-verifies the WASM
bundle from
`argent-private-releases`, and bundles it (plus SQL queries and the
Perfetto
  TraceConfig) into the published `@swmansion/argent` package.

End-to-end on a 76 MB trace with 1013 jank rows: **6.3 s** total analyze
(the older per-hang-subprocess approach was projected at ~47 minutes and
never
finished within the tool-call deadline).

---

## TESTING MANUALLY

To test manually run the following packing command checked-out on this
branch:
```
npm run pack:mcp -- argent-@pFornagiel-native-android-devtools-perfetto
```

This will bundle the package with the new binaries. After this PR is
fixed and the binaries are resolved from the main release, the
`argent-@pFornagiel-native-android-devtools-perfetto` release can be
deleted.

## Why

Android needed native CPU/hang/memory profiling to reach parity with
iOS. The
natural engine is Perfetto's trace-processor, but the obvious ways to
ship it are
both bad:

- **`trace_processor_shell` binary** — per-platform (a mac-arm64 pack
shipped a
mac-arm64-only tarball), pinned an OS/arch matrix, and cost a fresh ~1.3
s
subprocess fork **per query**. With one subprocess forked per detected
hang, a
  large trace projected to ~47 minutes.
- **`get.perfetto.dev/trace_processor` Python launcher** — adds a
runtime Python 3
dependency, resolves "latest" at first run (defeats version pinning),
and makes
  the first analyze slow via a network fetch.

The WASM engine is one cross-platform artifact, in-process and
warm-cached: the
trace is parsed **once** and reused across the whole analyze run plus
every
drill-down query.

---

## The three big pieces

### 1. In-process Perfetto WASM engine

New package surface in `@argent/native-devtools-android`:

| File | Role |
| --- | --- |
| `src/wasm-trace-processor.ts` (+476) | The whole engine: boot, glue
patching, warm-cache, fault containment, chunked parse, cell decoding |
| `src/perfetto-engine.d.ts` (+49) | Hand-written types for the
version-agnostic RPC decoder (imported by path, so tsc can't see its own
`.d.ts`) |
| `src/errors.ts` (+68) | `TraceProcessorUnavailableError`
(`wasm_load_failed` / `wasm_path_invalid`) |
| `src/bundled-meta.ts` (+12) | `PERFETTO_VERSION` stamp, regenerated at
pack time |
| `src/index.ts` | Re-exports + package-root accessors
(`traceProcessorQueriesDir`, `traceConfigPath`) |

Public surface: `resolveTraceProcessorAssets()`,
`queryWarm<Row>(tracePath, sql)`,
`ensureTraceProcessorReady(tracePath)`, `disposeWarmEngine(tracePath)`.

Highlights of how it works:

- **Boots Google's prebuilt web/worker wasm under Node.** Installs a
minimal
worker scope (`self`, `WorkerGlobalScope`, `location`), reads the wasm
bytes,
patches the Emscripten glue (`useMemory64=false`, hand the bytes in
directly,
expose the bridge), and drives it over a `MessageChannel` + `EngineBase`
subclass. Glue patches are applied **by unique string anchors that throw
if they
  don't match** — the canary for glue-format drift on a Perfetto bump.
- **Warm-engine cache, one per trace path.** LRU with `MAX_WARM_ENGINES
= 3`
(bounds resident memory at ~26–76 MB trace + wasm heap each), 5-min idle
dispose
  (timer `unref()`'d), no failed-promise caching.
- **Fault containment (`fatal` promise).** `EngineBase.fail()` throws
synchronously
inside the `MessagePort.onmessage` handler (e.g. an RPC framing error);
left
unhandled it becomes a Node `uncaughtException` and would **kill the
entire
tool-server**, wiping every device + profiler session. Each engine
exposes a
`fatal: Promise<never>`; the handler try/catches, rejects `fatal`,
evicts the
engine, and every load/query is `Promise.race`'d against it — so a
contained
  fault is a clean rejection and the server stays up.
- **Chunked trace parse.** Perfetto's RPC ring buffer rejects any frame
> 64 MiB,
  so the trace is fed in 32 MiB chunks before `notifyEof()`.

### 2. Android native profiler + the shared/dispatch refactor

**Capture** (`utils/android-profiler/capture.ts`, `detect-app.ts`):
spawns
`adb shell perfetto --txt -c - --background-wait -o
/data/.../argent-<ts>.pftrace`,
piping the TraceConfig on **stdin** (SELinux denies `shell:s0` writes
under
`/data/misc/perfetto-traces/`, so the config can't be staged as a file),
waits for
the PID on stdout, then on stop sends `kill -TERM`, polls `/proc/<pid>`,
and
`adb pull`s the trace.

**Pipeline** (`utils/android-profiler/pipeline/`): PerfettoSQL does the
parsing and
aggregation, so the Android pipeline is intentionally **two files, not
iOS's four**.
The per-hang annotation pass is **batched** — `runBatchedHangFolds`
inlines all
hang windows as a `VALUES` table into one SQL script (two derived views
unioned in
a single terminal SELECT, because the engine returns only the final
statement's
rows) and runs it once against the warm engine.

**The platform-dispatch refactor** —
`native-profiler-start/stop/analyze` shrank
from monolithic iOS files (~160–330 lines each) into **thin routers**
(~60–110
lines) that branch on `api.platform`:

```
native-profiler-start ─┬─ ios?    → platforms/ios.ts      (xctrace)
                        └─ android → platforms/android.ts  (adb perfetto)
```

A new **`utils/profiler-shared/`** layer holds everything both backends
use:

| File | What was hoisted |
| --- | --- |
| `types.ts` | Unified `Bottleneck = CpuHotspot \| UiHang \| MemoryLeak
\| MemoryRssGrowth`, each now carrying a `platform` field; shared
`NativeProfilerAnalyzeResult`; `RECORDING_CAP_MS` |
| `aggregate.ts` | Generic `aggregateCpuHotspots(AggregatorInputRow[])`
— burst windowing (`BURST_GAP_MS=500`), severity banding (RED >15%,
YELLOW 3–15%, <3% dropped). iOS feeds it via a pre-pass; Android feeds
it precomputed SQL rows |
| `lifecycle.ts` | `shutdownChild` SIGINT→SIGTERM→SIGKILL ladder +
`waitForChildExit` (no PID-reuse hazard) |
| `thread.ts` | `normalizeThreadName` for both xctrace and Perfetto
thread names |
| `time-align.ts` | New `buildPerfettoAnchor()` + `windowsOverlap()`
alongside the existing iOS/React anchors |
| `format.ts` | `formatBytes` (moved out of the combined report) |

iOS was refactored to fit this shape: `ios-profiler/lifecycle.ts` and
`types.ts`
become thin re-exports; `pipeline/02-aggregate.ts` shrank from ~200 to
~60 lines
(keeps its iOS-only dominant-function pre-pass, delegates
grouping/bursting/
severity to the shared aggregator). **No iOS behavior change** — same
outputs,
shared internals.

**Session blueprint** (`native-profiler-session.ts`): generic
`capturePid` /
`captureProcess` (was `xctracePid` / `xctraceProcess`), new `platform`
and
`androidOnDeviceTracePath` fields, and **platform-branched dispose**
(iOS SIGKILLs
xctrace; Android `kill -KILL`s perfetto, `rm`s the device trace, and
disposes the
warm WASM engine).

**Drill-down tools**: `profiler-load.ts` learns to restore Android
`.pftrace`
sessions (with an optional metadata sidecar / `app_process` override);
`profiler-stack-query.ts` splits into iOS and Android branches. Unlike
iOS (which
caches `parsedData` in memory), Android **re-queries the `.pftrace`**
per
drill-down — the warm engine is the cache, so each query costs only SQL
execution
time.

### 3. Packaging & release pipeline

The WASM bundle's three third-party artifacts (`trace_processor.wasm`,
`engine_bundle.node.js`, `engine.mjs`) + `LICENSE` are **never
committed**
(`.gitignore` lists the generated files but not the dir). They're built
and
sha256-checksummed in `argent-private` CI, published to
`argent-private-releases`, and on our side:

- **`scripts/download-trace-processor.sh`** (+88, new) — `gh release
download` →
verify tarball sha256 (**FATAL on mismatch** — the blobs are unsigned) →
extract
  → re-verify via `SHA256SUMS` → assert all four present.
- **`scripts/pack-mcp.cjs`** (+60, new) — single pack entry point:
downloads
simulator-server, native binaries, **and** the trace-processor bundle,
then
builds and `npm pack`s `@swmansion/argent`. `package.json`'s `pack` /
`pack:mcp`
  now call this script.
- **`packages/argent/scripts/bundle-tools.cjs`** (heavily reworked,
+574/-...) —
copies `assets/trace-processor/`, `assets/queries/`, and
`argent.tracecfg.pbtxt`
into `packages/argent/assets/`, and regenerates `bundled-meta.ts` to
stamp the
  Perfetto version.
- **`publish.yml` / `publish-next.yml`** — run the download step and
then **verify**:
  all four WASM files present, the wasm magic-byte check
(`head -c4 … | grep $'\x00asm'`), plus the queries dir, TraceConfig,
manifest,
  and Android helper APK.

Asset reorg: `native-devtools-android` now ships `dist/`, `bin/`, and
`assets/`
(SQL queries, TraceConfig, manifest moved under `assets/`); `.gitignore`
swaps the
old per-file `packages/argent/manifest.json` entry for the whole
`packages/argent/assets/` dir.

---

## Testing

15 new test files under `packages/tool-server/test/android-perfetto/`
cover the
engine and pipeline, including:

- `dispatch.test.ts` — start/stop/analyze route to the right platform
backend.
- `aggregate-shared.test.ts` — the shared aggregator on both iOS and
Android-shaped input.
- `hang-folds-batched.test.ts` / `hang-fold.test.ts` — the batched
per-hang SQL fold.
- `hang-severity.test.ts` — jank-reason → RED/YELLOW classification.
- `dispose.test.ts` / `dispose-warm-engine.test.ts` — session dispose
kills the daemon, rms the device trace, and tears down the warm engine.
- `stop-recovery.test.ts` — `profilingActive` is forced false even if
`adb pull` fails (so a retry-start doesn't wedge).
- `run-tp.test.ts` — `renderSqlTemplate` token rendering + the "unused
substitution" guard.
- `manifest-hint.test.ts` — zero-callstack detection emits the manifest
hint.
- `profiler-load.test.ts` — `.pftrace` session restore with/without the
metadata sidecar.

Plus `native-profiler-analyze-failure.test.ts` for the
engine-unavailable banner,
and updated iOS tests for the shared-lifecycle refactor.
`@argent/native-devtools-android` gains its own vitest setup
(`vitest.config.ts`, `tsconfig.test.json`, `test/errors.test.ts`).

---

## Notes & follow-ups

- **Manifest requirement.** Perfetto's `linux.perf` data source needs
`/proc/<pid>/mem` access, granted only to a **debuggable** or
`<profileable
android:shell="true"/>` app. A release build without either silently
produces
zero callstacks; the pipeline detects this and emits a **manifest hint**
instead
  of an "All clear" report.
- **Version pinning lives in the `argent-private` submodule**
(`PERFETTO_VERSION`
/ `PERFETTO_WASM_TAG` / `LYNX_TRACE_PROCESSOR_VERSION`). Bumping is
documented in
`argent-private/docs/ANDROID_PERFETTO.md`. JS↔engine version skew is
structurally
  impossible (version-agnostic decoder + vendored wasm).
- **`disposeWarmEngine` is exported but not wired into session-stop** —
warm
engines are reclaimed only by the idle timer + LRU cap. Acceptable today
(bounded
memory); wire it into session dispose if explicit teardown is ever
needed.
- **Submodule bump.** This PR moves `packages/argent-private` to
`22e55f90` — see the companion `argent-private` PR description,
including its
merge-hygiene note (that branch should pick up latest `argent-private`
main
  before the pointer is finalized).

### Companion docs added in this branch
- `utils/android-profiler/ANDROID_PROFILER_REFERENCE.md` — stack,
capture, queries, manifest requirement.
- `utils/android-profiler/PIPELINE_DESIGN.md` — the "why" (two-file
pipeline, shared-aggregator hoist, re-query-don't-cache, batched fold).

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 16:06:24 +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
Ignacy Łątka 837d088744 test: cover #148 (android) (#186)
This PR adds tests for #148.

## Summary

Moves the Android emulator branch's test coverage out of #148 so that PR
can be reviewed without the test churn. This branch adds the test files
back on top of the (now test-stripped) base branch.

Tests inseparable from source changes (`describe-tool`,
`workspace-reader`, `workspace-reader-integration`) and the
`boot-simulator.test.ts` deletion stayed on #148 - base PR's CI passes
on its own.

## Test plan

- [x] `npx tsc --build` — clean
- [x] `npx prettier --check .` — clean
- [x] `npm test --workspaces --if-present` — 585/585 tool-server tests
pass; all other workspaces green

---------

Co-authored-by: Paweł Fornagiel <pawel.fornagiel2@gmail.com>
2026-05-12 14:21:09 +02:00