mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
feat/flow-bash-scripts
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Mateusz Aliyev
|
6e9e45c9f0 |
feat: attach to devices offered by external providers (#735)
This pull request lets Argent drive a simulator or emulator another process is already running, attaching to its simulator-server rather than spawning a second one. A provider writes a JSON descriptor to `~/.argent/providers/` listing the devices it offers and the mechanisms Argent may use on each. They appear in `list-devices` with an `ext:` id and work with the existing tools. The file is re-read on every call, so a withdrawal or a narrowed grant applies immediately. `ios.additionalDeviceSets` (#600) already makes such a simulator reachable by UDID. What changes is ownership. `boot-device` and `stop-simulator-server` refuse these devices, anything the provider did not grant is refused with a message naming it and Argent uses only the endpoints its own simulator-server build serves. A grant binds to the device rather than to one of its names, so the real udid or serial is gated exactly like the `ext:` id. Android emulators are covered too and a device visible to both a provider and `adb`/`simctl` is listed once. The contract ships as `schemas/device-provider-v1.json`, validated by `argent providers check`, and is documented in `docs/reference/device-providers.mdx`. Includes unit tests and an e2e phase where the harness acts as its own provider. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for discovering and connecting to externally managed devices. * Added `argent providers` commands for listing, validating, publishing, withdrawing, and pruning providers. * Added provider-aware simulator, debugger, profiler, native tools, and device listings. * Added capability controls, endpoint validation, revocation handling, and provider-specific diagnostics. * Added automatic CLI discovery for provider integrations. * **Bug Fixes** * Improved service recovery and paused-runtime error reporting. * Improved simulator keyboard and paste command reliability. * **Documentation** * Documented provider descriptors, capabilities, lifecycle rules, and CLI usage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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>
|
||
|
Ignacy Łątka
|
8c8cdb885e |
fix(e2e): stop the harness reporting checks it never made (#595)
Found while running the full E2E harness against the packed 0.17.0 bundle for the release check. All of these are harness bugs, not product bugs. ## The core problem The harness reported results it had not actually measured. `assert_reject` looked for zod issues on **stdout**, but the CLI prints them on stderr and leaves stdout empty on a rejection. Both arms of the check called `pass()`, so the miss was invisible: the zod-path argument every case passes in was decorative, and a CLI that simply exited 1 for every input would have scored the same 88/88. **All 88 validation cases asserted nothing beyond "exit code was non-zero."** Once that check could fail, it exposed the rest: - **awk has no `\b` word boundary.** `/\bnumber\b/` matches nothing, so every flag kind resolved to `unknown`, `model_number_flags` was always empty, and **the bad-type matrix never emitted a single case** - while `_build_args` handed the string `"x"` to every required numeric/boolean/object field. - The describe parser matched `--[a-zA-Z0-9-]+`, **truncating every snake_case flag at the underscore** (`--device_id` → `device`), and kept the `-json` display suffix (`--selector-json`) that is not part of the schema key. Both produced payloads carrying keys no schema knows. - `missing-required` asserted `invalid_type` for enum flags, which zod reports as `invalid_value`. Parsed kinds went from **270 unknown / 21 enum / 1 array** to 131 string, 68 number, 28 boolean, 25 integer, 21 enum, 16 array, 3 object, 2 unknown (the last two are `any`, which is correct). ## Teardown and the exit code `run-e2e.sh` had no `trap`. An unbound variable inside a phase is fatal to the whole shell - `run_phase || warn` does not catch it - so an abort skipped the cleanup phase, the report, and `rm -rf $E2E_WORK`, leaking a ~500MB sandbox plus a tool-server, Electron, Metro and a fixture http server. Report generation, teardown and the exit code now live in an EXIT trap, with a completion flag: without it an aborted run exits **0** on "0 failures", which is verified below by mutation. Teardown runs *before* the sandbox is removed - `$E2E_HOME` holds the detached tool-server's pid file and that server is started with no idle timeout, so deleting the sandbox first strands a process `argent server stop` can no longer find. `90-cleanup.sh` gated `stop-all-simulator-servers` and `stop-metro` on `ARGENT_TOOLS_URL`, which the harness never sets (`ensure_server` deliberately does not pin it), so **neither ever ran** and every run left a simulator-server and Metro alive - while `teardown-complete` passed regardless. ## Other findings fixed - **`--android-avd` could never work**: `boot-device` was asked for 840s inside `run_tool`'s 120s timeout, and the booted serial stayed in a `local`, so the RN tier blanket-skipped all 24 debugger/profiler tools for want of a device that was right there. Nothing shut the emulator down either - the tool-server only reaps devices Lens booted, not ones `boot-device` started. - **The validation tier was byte-identical green against a dead server.** `argent run` does no client-side validation, so every call failed with a transport error that no one distinguished from a rejection. It refuses to run instead, and no longer counts `timeout(1)`'s 124-127 as a rejection. - **The post-uninstall restore** hardcoded `--omit=optional` and the sandbox prefix - dropping electron on a full run (the chromium tier then skipped itself as "electron not installed"), and under `--system` restoring the machine's real global `argent` into the sandbox instead of putting it back. - **The Android teardown claimed to leave an injected device alone** and then issued the same `stop-simulator-server` call as the self-booted branch, discarded the result, and recorded a skip saying it had not - tearing down the allocator's device service immediately before the RN tier drove that same serial. - **The chromium port-owner fallback** matched `:$port` as a substring and took the first pid anywhere in `ss -ltnp` output, so cleaning up `:1234` would kill a process listening on `:12345`. - **The RN tier's `stop-metro`** sat after every early return (a failed `debugger-connect` left Metro running) and ran unconditionally, killing a developer's own Metro on :8081 that the harness never started. - **The chromium display gate** accepted an installed `xvfb-run` as a substitute for `DISPLAY`, but nothing wraps the Electron spawn in it - so a headless box with it installed cleared the gate and failed `boot-device`, turning the gate red for exactly the environment it exists to skip. - A phase file with a syntax error silently re-ran the **previous** phase under the new phase's name; an unknown `--phase` selected nothing and exited 0; `-h` printed 7 lines of source code; a value-taking flag with no value died on an unbound `$2`; two runs in the same second shared one results file. - `flow-add-step` declares its own `args` property, which makes `--args` that field's value rather than the whole-payload escape hatch, so `run_tool` cannot address its fields. Skipped with that reason rather than failing on a payload the tool never received. ## Verification Against `swmansion-argent-0.17.0.tgz`, `introspection,validation`: | | before | after | |---|---|---| | totals | 177 pass / 0 fail / 5 skip | 221 pass / 0 fail / 6 skip | | rejection cases naming the field they test | **0 of 88** | **130 of 131** | | bad-type cases emitted | **0** | **43** | - `assert_reject` checked against right-field, wrong-field, wrong-code, no-path and unreachable-binary inputs. Every one of those cases passes on the un-fixed code. - Abort path: injected an unbound variable into a phase - cleanup phase runs, report written, sandbox removed, exit 1, `PARTIAL` reported. With the completion flag mutated to always-set, the same abort exits **0**, which is what pins it. - awk `\b`, the `-json` suffix, the underscore truncation, the `ss` port collision and the `timeout` 124-127 contract were each reproduced standalone before being fixed. ## Found by running against a live Android emulator The android tier executed end-to-end for the first time: **23 pass / 1 fail / 10 skip**, including the `screenshot-diff` pair that the original unbound-variable bug aborted on. Three further defects only surfaced by running: - **Nine `pass`/`fail`/`skip` calls passed their message as the third argument**, which `_record()` takes as the *case name* — so the detail column was empty and the case string changed every run. The chromium boot failure was the visible cost: it recorded `case=" "` with **no diagnostic whatsoever**. - **A call killed by `timeout(1)` writes nothing**, so `RT_OUT` is empty exactly when the operator most needs to know what happened. A hung `boot-device` was recorded as a failure with a blank detail. All nine ad-hoc detail formatters now share `rt_detail()`, which names the timeout (124) and a command `timeout` could not run (125-127). - **The chromium display gate tested that `$DISPLAY` was non-empty, not that it was usable.** A display-manager session switch on this box left `$DISPLAY` pointing at a server the process holds no cookie for; Electron then bound its CDP port and never answered on it (verified: the TCP connection is accepted, the HTTP response never arrives), so the tier hard-failed instead of skipping. Moving `profiler-stack-query` and `profiler-combined-report` into the native block also closed a hole their removal had opened: both were listed in `RN_TOOLS` and in the failure-arm skips but had no case on the success path, so a successful `react-profiler-start` left them with no pass, fail or skip at all. ## Not covered here - The **postinstall assertion** is left alone - #556 already fixes it, and this branch does not touch it. Until that merges the install phase still records that one failure. - The **RN tier still has not executed.** The emulator itself is fine — it cold-boots in ~24-30s and ran the whole android tier — but it dies from CPU starvation (`detected a hanging thread 'QEMU2 main loop'`) whenever this machine's load average climbs, which it does to 90-120 on 16 cores from concurrent work. Four attempts: it survived the android tier at load ~10, and was starved each time by a competing `npm install` or by Metro's own bundler startup. The four RN profiler fixes are verified against the tools' source contracts (the throw sites and their required params), not against a live run. - The **chromium tier still has not executed** past `boot-device`, for the display reason above. - Known remaining, deliberately left: `assert_ok` cannot fail for calls whose result is a constant `true`; the cookie/storage set→get pair never verifies the round-trip value; a tier-level skip writes one row so `report.py`'s coverage matrix silently drops the tools that tier would have recorded; `report.py` renders an empty or partial log as a clean green report. |
||
|
Ignacy Łątka
|
cd4190b80e |
Full tgz-only E2E harness (scripts/e2e-full) (#495)
Now verified end-to-end on the real Mac: install/introspection/validation, the android tier (against a booted emulator), and the chromium tier all pass from a locally packed tgz; the rn tier auto-skips without `~/dev/bluesky` as designed. ## What this is A release-gating end-to-end harness under `scripts/e2e-full/` that starts from **only a `swmansion-argent-*.tgz` bundle** and exercises the whole product: the install flow, every CLI command, all 70 tools' argument validation, and a happy-path run of every tool against real devices. Intended to be run on the real Linux box and the real Mac before a release. Everything runs under a throwaway `HOME` + npm prefix, so it never touches the machine's real `~/.argent`, editor MCP configs, or global packages (safe on a shared box). It emits a JSONL log + a markdown coverage report and exits non-zero if any hard assertion fails. ## Phases | phase | needs | covers | |---|---|---| | `install` | npm + network | `npm i -g <tgz>`, bundled binaries, `init` (global + `--local`), `update`, `uninstall`, telemetry, MCP-config generation | | `introspection` | — | `--version/--help`, `tools describe` for **all 70** tools, feature flags, `server start/status/logs/stop`, `link/unlink` | | `validation` | — | per tool: missing-required / bad-enum / bad-type rejection (schema-driven, no hardware) | | `android` | Android emulator | happy-path of every touch/gesture/screenshot/app-lifecycle tool | | `chromium` | Electron (bundled optional dep) + a display | boots a generated Electron app; drives CDP tools (scroll/drag/tabs/cookies/storage) | | `rn` | `~/dev/bluesky` + Android device | debugger + react/native profiler + network chain against the real Bluesky app | Tiers auto-skip with a recorded reason when prerequisites are missing. iOS / tvOS / Vega are intentionally out of scope for now. ## What I actually verified (real runs, Linux) - **install** 18✓/0✗, **introspection** 85✓/0✗ (all 70 `tools describe`), **validation** 81✓/0✗, **chromium** 24✓/0✗ (real Electron app + CDP). - Holistic run: **190✓ / 0✗ / 34∼, exit 0.** Failure-gate confirmed (non-zero on fail), `bash -n` clean on all files, all 72 android/rn JSON arg literals linted. ## What I did NOT verify live (please scrutinize) - **`android` and `rn` tiers were never run against a live device.** The Android emulator would not stay up on my box (memory-constrained shared machine; QEMU threads hung seconds after boot across ~7 attempts). The code is complete and uses the same machinery the `chromium` tier exercised live, but it has **not** been proven end-to-end. This is the main reason for the "not certain how well it works" caveat. - The RN tier's exact debugger/profiler arg shapes are derived from `tools describe`, not confirmed against a running Metro. ## Run it ```bash bash scripts/e2e-full/run-e2e.sh # everything applicable to this OS bash scripts/e2e-full/run-e2e.sh --skip-install --phase introspection,validation # fast offline core bash scripts/e2e-full/run-e2e.sh --phase android --android-serial emulator-5554 # inject a device ``` |