skills/software-mansion__argent
1
0
Fork 0
mirror of https://github.com/software-mansion/argent.git synced 2026-09-14 19:27:14 +08:00
Code Issues Packages Projects Releases Wiki Activity
1,119 Commits 225 Branches 38 Tags
stack/721-base-legacy
Commit Graph

42 Commits

Author SHA1 Message Date
Jakub Piasecki d4a712c401 refactor(gesture): rename the momentum-free flag to momentum (#732) 
`settle: true` collided with the runner's other `settle` — waiting for
the tree to quiesce — which is the very thing a swipe step does after
every dispatch, whichever way the flag is set. One sentence of the flow
skill used both senses at once.

This names the physics instead: `momentum: false` is the momentum-free
gesture, and the natural fling is the default the prose already
described.

### Renamed

| surface | before | after |
| --- | --- | --- |
| `swipe` flow directive | `settle: true` | `momentum: false` |
| `gesture-swipe` param | `settle` | `momentum` |
| `gesture-drag` param | `settle` | `momentum` |

The directive passes the flag straight through to the tools, so all
three move together. `momentum: true` is the default and normalizes to
absent, mirroring what `settle: false` did — parse/serialize stay exact
inverses.

### Also

- Recording summary label `(settle)` → `(momentum-free)`
- Internal constants → `MOMENTUM_FREE_EASE_EXPONENT`,
`MOMENTUM_FREE_MIN_STEPS`
- `flow-run` tool description and `argent-create-flow` skill updated

### Testing

`tsc -b`, eslint (`--max-warnings 0`) and prettier clean; `test/flows` +
`test/tools` pass 671/671, including a new case covering the
legacy-spelling rejection.
 2026-08-28 09:55:47 +02:00
Hubert Gancarczyk 2740a3ec38 chore(knip): clear the parked dead-code backlog and drop the ceiling to 0 (#770) 
> Built on top of #769, so this PR carries that commit too — the
workflows only run for PRs targeting `main`, which is why it is based
there rather than on the branch. #769 on its own is the minimal,
low-risk fix for the currently-red job; if it merges first this shrinks
to its own commit, and if this one merges first it closes #769 as well.

## What this does

#572 landed the gate with 215 findings parked under `--max-issues`. That
bounded the backlog but left it in place, and the first thing to touch
it (#663) pushed the total to 218 and turned the job red. This clears
the backlog and sets the ceiling to **0**, so the next unused export
fails the job instead of consuming headroom nobody knew was there.

| | before | after |
| --- | ---: | ---: |
| Unused exports | 47 | 0 |
| Unused exported types | 156 | 0 |
| Unused exported class members | 12 | 0 |
| `--max-issues` | 215 | **0** |

## Scope check first

Every workspace holding a finding is `private: true`.
`@swmansion/argent` — the one published package — had **zero**, so
nothing here moves a public API.

## Two shapes of fix

**Still referenced inside its own file → drop `export`** (146 types, 36
exports). The declaration stays put; it just leaves the module's public
surface. `tsc` keeps emitting these into the `.d.ts` as local
declarations that the exported signatures reference.

For the 146 **types** that is the whole story: they are erased, so the
emitted `.js` is byte-identical. For the 36 **values** it is not.
Dropping `export` from a `const`/`function`/`class` removes its
`exports.X` binding from the emitted `dist/*.js` — 36 declarations
across 26 files here. `packages/tool-server/dist/utils/device-info.js`
no longer carries `exports.REMOTE_PREFIX` or
`exports.VEGA_SERIAL_PREFIX`; `telemetry/dist/posthog.js` loses
`exports.POSTHOG_PROJECT_TOKEN`; in the ESM-emitting `argent-installer`,
`isTempRunnerPath` / `resolveLocalArgentDir` / `PACKAGE_ROOT` leave the
module namespace object entirely.

Nothing in the repo or in the `packages/argent-private` submodule at the
pinned revision reads any of them by path, so nothing breaks. Flagging
it because a `require()` of a built `dist/*.js` loses a dropped `export`
exactly the way it lost the deleted `__resetAndroidDevtoolsInstallCache`
this branch had to restore — so this half of the diff does deserve
runtime scrutiny, not a skim.

**Referenced nowhere → delete.** Orphaned test seams (`__test`,
`_testValidators`), stale compatibility re-exports, and a few real
orphans. The ones worth a reviewer's eye:

- **`localSimctl`** — the `SimctlBackend` strategy is only ever
constructed with `remoteSimctl`. What keeps the local iOS impl off it is
the handler signature: `buildIosLaunchHandler` /
`buildIosRestartHandler` read `services.nativeDevtools`, and
`ios-remote` is the only platform that declares that service, so the
local impl — which resolves native-devtools per device inside its own
handler — has nothing to hand them and shells out to `xcrun` itself.
- **`sendCharInsert`** — removing the `__sendCharInsert` test alias
exposed the underlying helper as unreferenced. It is superseded, not
unwired: the Chromium keyboard impl dispatches its own `type: "char"`
event inline (`keyboard/platforms/chromium.ts:58`), which is exactly
what this helper did. Chromium typing is unaffected.
- **Error metadata fields** — `ServiceNotFoundError.serviceId`,
`ToolNotFoundError.toolId` and their two siblings were never read. The
`e.serviceId` in `registry-error-events.test.ts` is a field on the event
object the handler pushes, not on the error class; the `err.toolId` in
`http.ts` is `NotImplementedOnPlatformError`, a different class that
keeps its fields. Flagging these in case they are wanted as deliberate
diagnostics.

Deleting `SourceMapsRegistry.toGeneratedPosition` / `findMatchingSource`
cascaded into their only helper (`buildSourceCandidates`) and then into
`projectRoot`, which had no remaining reader — so the constructor
parameter goes too. `StubSourceMapsRegistry` existed only to pass
`super("")`, so it goes with it.

## The three cross-workspace members

`ArtifactStore.register`, `ArtifactStore.list` and
`TypedEventEmitter.off` are all called from other workspaces — the
artifact route and screenshot tools in `tool-server`, and
`registry-listener` in `telemetry`. They are reported unused anyway, and
the reason is the **unbuilt tree** the gate runs against: every
workspace resolves `main`/`types` to a `dist/` that does not exist, so
`@argent/registry` resolves to nothing and the cross-workspace edge
never forms. Build the tree and all three findings disappear on their
own; a control member that really is dead is still reported in the built
run, so the pass is live either way. Nothing about this is specific to
`classMembers` — every cross-workspace reference is invisible the same
way.

Each is exempted at the declaration with a `@public` JSDoc tag rather
than by name in `knip.jsonc`, for one reason: **scope**. `ignoreMembers`
matches a name across the whole workspace, so it would also hide a
future dead `register`/`list`/`off` on any other class in
`packages/registry`; `@public` binds to the member it is written on.

What `@public` does **not** buy is a staleness check, and neither does
`ignoreMembers`. Measured on a clean unbuilt worktree: a bogus
`ignoreMembers` name and a redundant `@public` each leave the run at
exit 0 with no hint, while a bogus `ignoreDependencies` name exits 1
with `Remove from ignoreDependencies`. `treatConfigHintsAsErrors` is
live and simply has no producer for either exemption style — so when one
of the three stops being earned, nothing says so, and the JSDoc at each
member is the only record a re-audit has. Each one names its callers,
and `off` records how to re-derive them (rename the member and read the
`Property 'off' does not exist` errors; grepping `.off(` over-reports,
since 14 of the 38 call-site hits are Node `EventEmitter`s). One
correction to the commit message on that change: it says 35 hits where
the exact count is 38 — 14 Node `EventEmitter`, 24 `TypedEventEmitter`
(21 production, 3 in a test). The 14 and the 21 are right.

`knip.jsonc`, `CONTRIBUTING.md` step 5 and the workflow's
failure-explanation step all described a parked backlog and a ceiling to
stay under. All three are rewritten for a gate that must simply come
back empty, and all three now name the `@public` escape hatch — the
remedy for a symbol whose only caller is out of knip's reach.

## Verification

Run against a clean worktree with **no build output**, the way CI
counts:

- `npm run knip` — **exit 0, prints nothing**. Both passes empty, no
config hints.
- `npx tsc --build` — clean.
- `npm run lint` (`eslint . --max-warnings 0`) — clean.
- `npx prettier --check .` — clean.
- `npm test --workspaces` — tool-server 329 files / 3857 passed;
registry, telemetry, update-core, tools-client, mcp, cli green.
- `npm run typecheck:tests --workspaces` — 13 workspaces define the
script and all 13 are clean, but **the command itself exits 1**: npm
errors `Missing script: "typecheck:tests"` for the three that do not
define it rather than skipping them. Worth knowing before re-running
this bullet and reading the exit code as a failure.

End-to-end on a tool-server built from this branch, driven over its HTTP
API:

- **Chromium** (Electron smoke app) — `describe`, `screenshot`,
`gesture-tap` (counter advanced 0 → 1), `POST /api/clipboard/text` read
back out of the renderer, the WS `clipboardSync` command, and
`debugger-connect` / `debugger-status` / `debugger-evaluate`
(`sourceMapReady: true` with `StubSourceMapsRegistry` gone).
- **iOS** — `boot-device`, `launch-app`, `describe` via ax-service,
`screenshot`.
- **tvOS** — `boot-device`, `launch-app` on an Apple TV udid,
focus-driven `describe`. The registry snapshot then carries
`NativeDevtools:<tvOS udid>`, confirming injection **is** resolved on
tvOS through the local impl.
- **Android** — `boot-device` on a dedicated AVD, `describe` via
android-devtools, `gesture-tap`.
- **Metro source maps** — the registry driven against a real 12.5 MB
Metro `.map` over loopback, the way `Debugger.scriptParsed` drives it:
allowlist passes and `waitForPending()` resolves on both, while retained
heap after gc drops from 17.3 MB to 3.6 MB.


## Review follow-ups

Five commits on top, one per finding. The base is back on `main`, so the
workflows run again.

- **`docs(launch-app)`** — the services comment explained the empty
shape through `LaunchAppAndroidServices`, deleted here, and skipped
`LaunchAppVegaServices`, which is still there. Same edit in
`restart-app/types.ts`.
- **`docs(registry)`** — the `@public` tag on `ArtifactStore.register`
named 2 of 7 callers. Derived the full set the way the sibling tag on
`off` prescribes: renamed the member, read the 16 `TS2339` errors `tsc
--build` reports across flow-visual (5), screenshot (4),
native-profiler-stop (2), screenshot-diff (2), native-profiler-analyze,
react-profiler-analyze and screen-recording-stop. The re-derivation
recipe is recorded too — `.register(` greps as badly as `.off(`.
- **`docs(knip)`** — "the build, the tests and this gate all stay green
on a wrong delete" held for argent-private and not for the
cross-workspace case that justifies three of the four `@public` tags.
Deleting `ArtifactStore.register` gives 16 `TS2339` errors and `9 failed
| 2 passed` in `test/artifacts.test.ts`; only the gate stays green.
Split in the workflow comment, the failure annotation and
`CONTRIBUTING.md` step 5.
- **`perf(debugger)`** — `doRegister`'s `data:` branch still
base64-decoded and `JSON.parse`d a payload it dropped, inside a `catch`
that swallows. Both arms returned the same `void`, so the branch
collapses to an early return; `scriptUrl` / `scriptId` go with it, since
their last reader was the deleted `this.maps.push(...)` and `doRegister`
is private.
- **`test(debugger)`** — the allowlist check moved to a top-level early
return with nothing pinning it: delete the line and both SSRF files
still pass 17/17. Two tests now assert `doRegister` consults it — four
rejected URLs reach no `fetch`, and the loopback `*.map` Metro emits
still does.

Re-verified after: `npm run knip` exit 0 and empty on a `tsc --build
--clean` tree, `tsc --build`, `eslint . --max-warnings 0`, `prettier
--check .` all clean, and `npm test --workspaces` with tool-server at
329 files / 3854 passed. The one red test, `argent-installer`'s
`globalPath returns ~/.config/opencode/opencode.json`, fails identically
with these commits stashed — it reads the real `~/.config/opencode`,
which holds a `.jsonc` on this machine.

End-to-end on a tool-server run from this branch over its HTTP API, on
every platform:

- **Chromium** — `launch-app`, `describe`, `screenshot` (returned an
artifact handle, `GET /artifacts/<id>` served the 137542-byte PNG back),
`gesture-tap` (counter 0 → 1), `debugger-connect` / `debugger-status`
(`sourceMapReady: true`) / `debugger-evaluate` (read `taps: 1` out of
the page).
- **iOS** — `launch-app`, `describe` via native-devtools, `restart-app`,
`screenshot`.
- **tvOS** — `launch-app`, focus-driven `describe` (12 focusables in
TVSettings), `tv-remote`, `restart-app`.
- **Android** — `launch-app`, `describe` via android-devtools,
`restart-app`, `screenshot`.
- **Vega** — `boot-device` on the `tv` VVD, `describe` via the
automation toolkit.
- **Metro** — the real `debugger-connect` / `debugger-status` /
`debugger-evaluate` tools against a stand-in Metro (HTTP `/json/list` +
CDP WebSocket) emitting three `Debugger.scriptParsed` events: a loopback
`*.map` on a counting server, a 1.4 MB inline `data:` map, and
`http://169.254.169.254/latest.map`. `sourceMapReady: true`,
`loadedScripts: 3`, evaluate 42, the loopback map fetched exactly once
and the metadata URL never. A malformed inline payload gives
byte-identical output, which is the finding.

### Merge with `main`

Putting the base back on `main` made CI resolve the merge, and the Unit
Tests job went red on a conflict no side could see on its own: `#771`
fixed the same red gate this branch's first commit did, the other way
round — it made `component-names.test.ts` import `StrippedName`,
`ComponentAnnotation` and `ComponentNameResolution` instead of dropping
their `export`. Merged `main` in, reproduced it (`tsc --noEmit -p
tsconfig.test.json` → three `TS2459`s) and took main's resolution: the
three exports are back. The gate stays green because knip treats test
files as entry points, so a type a test imports is not an unused export.

All nine workflows are green on the head commit.
 2026-08-25 17:27:12 +02:00
Ignacy Łątka 2e19c97de0 fix(cli): accept --help after a telemetry subcommand (#959) 
Found while differentially regression-testing the 0.22.1 release against
a bundle built from `v0.22.0`.

## The problem

`argent telemetry status --help` exits 2 with `Error: Unknown flag:
--help`. So do `-h`, and the same after `enable` / `disable`. On 0.22.0
all of them exited 0.

```
$ argent telemetry status --help
0.22.0   telemetry: / state: enabled / device id: not created     (rc=0)
0.22.1   Error: Unknown flag: --help                              (rc=2)
```

`telemetry` is the only subcommand that behaves this way. Checked
against every sibling on both versions — `server status|logs|start`,
`config list|get|set`, `flow list|run`, `secrets`, `flags`, `enable`,
`link`, `unlink` all return 0 on both. And the top-level help tells the
user to run exactly this: "Run `argent <command> --help` for
command-specific help."

## Cause

`TELEMETRY_OPTIONS` declares only `scope`, and
`parseCommandArgs(args.slice(1), …)` reads everything after the
subcommand — so `--help` arrives as an unknown flag. The `case "--help"`
arm in the switch only fires when help is `args[0]`, which is why bare
`argent telemetry --help` still works.

Introduced when `telemetry` was converted to the declarative parser
(#952, extended by #953).

## The fix

Hoist the help check above the parser, matching what `config.ts:220` and
`flags.ts:92` already do; `server`, `link` and `lens` instead declare
`help` in their option spec. Telemetry did neither. Ordering of
`telemetryInit` / `telemetryShutdown` around the help path is unchanged,
so a help invocation behaves exactly as `argent telemetry --help` did
before.

## Tests

Six cases added to `telemetry-command.test.ts` covering `--help` / `-h`,
bare and after each subcommand, asserting usage is printed and no config
file is written.

Mutation-checked: with the source change reverted, 4 of the 6 fail (the
2 bare-help cases pass either way, since those already worked) — so the
test pins the actual defect rather than the behaviour around it.

`@argent/cli`: 498 passed, 3 skipped. `tsc --build` clean, prettier
clean.

## Docs

No docs update needed — this restores previously documented behaviour
rather than changing a documented surface.

Co-authored-by: Ignacy Łątka <f.kaminski2000@gmail.com>
 2026-08-25 17:05:57 +02:00
filip131311 26405e68c7 refactor(cli): one declarative option parser for every hand-written subcommand (#953) 
Follows #952 (merged); rebased onto main.

## Why

`argent-cli` had **seven** copies of the same argv loop — `server
start`, `link`/`unlink`, `flow run`, `lens`, `config`,
`enable`/`disable`, and (from #952) `telemetry` — each hand-rolling `--x
value` / `--x=value` / `-x` / unknown-flag / missing-value handling with
slightly different messages and gaps (e.g. `lens` silently ignored
unknown flags). Adding an option meant another `if (tok === "--x") …
else if (tok.startsWith("--x="))` pair.

## What

`packages/argent-cli/src/command-args.ts` (introduced in #952) now
covers the whole subcommand surface, and every command declares its
options as a spec:

```ts
const START_OPTIONS = {
  help: { kind: "boolean", alias: "h" },
  port: { kind: "value", alias: "p" },
  …
} as const satisfies OptionSpecs;
```

Parser features: `--name value`, `--name=value`, single-letter aliases,
boolean switches, `--` end-of-options, `choices` validation, "don't
swallow a following flag as a value", and the guard against a stray
`true`/`false` word after a switch (previously duplicated in `flow` and
`lens`). Value *semantics* (port ranges, host wildcards, flag-name
charset, link targets) stay in each command — the parser only shapes
argv.

`flag-parser.ts` is untouched: it is the schema-driven `argent run
<tool>` payload builder (JSON Schema → tool args, `--args`/stdin
hatches), a different job.

## Behavior changes (bad-input paths only)

| Command | Before | After |
|---|---|---|
| `lens --bogus` / `lens extra` | silently ignored | `lens: Unknown
flag: --bogus`, exit 2 |
| `lens --terminal x` | `--terminal expects "iterm" or "terminal", got
"x"` | `--terminal must be "iterm" or "terminal", got "x"` |
| `flow run --platfrom=ios` | `unknown flag --platfrom=ios` | `Unknown
flag: --platfrom=ios` |
| `unlink foo` | `Unknown flag: foo` | `Unexpected argument "foo"` |
| `config … --scope x` | `--scope must be "global" or "project", got
"x"` | unchanged wording |
| `enable/disable`, `server start`, `link` | — | unchanged wording
(tests pinned) |

Happy paths are byte-identical; `server`/`link` still throw
`StartFlagError`, `flow` still throws `FlagParseException` (wrapped from
`UsageError`), so callers and tests keep their contracts.

## Tests

`argent-cli` 489/489 (26 files): rewritten `command-args.test.ts`
(aliases, `-` positional, empty-token value, true/false guard, choices
message), pinned-string updates only where the table above says the
wording changed (`flow.test.ts` ×4, `parse-link-flags.test.ts` ×1). `tsc
--build`, `typecheck:tests`, eslint, prettier, knip green. Smoke-tested
every command's error path through the built
`packages/argent/dist/cli.js` (see the "Behavior changes" table — all
exit 2 with usage where the command prints one).

## Docs

No docs update needed: the pages document flags and their meaning, not
error strings; no flag was added, removed or renamed.

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

https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu
 2026-08-25 13:32:36 +02:00
filip131311 c2324f052d feat(telemetry): project-scope opt-out (restrictive merge) + init --local --no-telemetry (#952) 
## Why

Teams that must not send usage data to a third party (a
strict-compliance customer asked) had no way to turn telemetry off for a
repository. `telemetry.enabled` was global-only
(`~/.argent/config.json`), so a committed local install (`init --local`)
still defaulted every teammate to **on** — `--no-telemetry` only opted
out the machine that ran `init`.

## What

- **`telemetry.enabled` accepts the project scope.** `consent.ts` now
reads both `~/.argent/config.json` and
`<project-root>/.argent/config.json` (root resolved from `cwd`, per-file
cache keyed by path) and merges **restrictively**: `false` in either
document wins. A committed project opt-out holds on every clone, and a
project file can never re-enable a global opt-out. Env overrides
(`DO_NOT_TRACK`, `ARGENT_TELEMETRY`) still outrank both.
- **`argent telemetry enable|disable --scope global|project`** (default
`global`). `status` names the document that decided (`config.json
(project)`, `(global)`, `(project and global)`). `enable` at one scope
while the other still opts out reports that telemetry stays disabled
instead of claiming success. Unknown scope / argument → exit 2, nothing
written.
- **`argent init --local --no-telemetry`** also writes
`telemetry.enabled: false` to `.argent/config.json`, and the "commit
these files" checklist lists it. Global mode is unchanged.
- `argent config set/unset telemetry.enabled` stays delegated to `argent
telemetry` (`manageCommand`), so the lifecycle work (draining the live
client) is still done by the dedicated command.

## Tests

Unit: `telemetry` (+8 project-scope cases: single-scope opt-out, both
restrictive directions, both-off, env precedence, live re-read of the
project file, malformed project file), `configuration-core` (schema
scopes), new `argent-cli/test/telemetry-command.test.ts` (real-fs, 6
cases), `argent-installer` — all green; eslint, prettier, knip,
`docusaurus build` green.

Live, from a real `npm pack` tarball of this branch (sandboxed `HOME`):

1. Fresh project → `init --local --yes --no-telemetry --from <tgz>`:
writes `.argent/config.json` `{telemetry:{enabled:false}}` + global
opt-out; summary lists `.argent/config.json` to commit.
2. `telemetry status` → `disabled`, source `config.json (project and
global)`. `telemetry enable` (global) → "stays disabled: config.json
(project) still opts out"; from `~` (no project) → `enabled`. `enable
--scope project` → enabled; `disable --scope project` → disabled.
`--scope team` → exit 2.
3. **Teammate simulation**: copied only the committed files to a fresh
clone with a fresh `HOME` (no global opt-out), bundle built with a dummy
OTLP token, `ARGENT_TELEMETRY_DEBUG=1`, drove `argent mcp` (initialize →
tools/list → `list-devices` call). Control clone without
`.argent/config.json`: tool-server emitted `toolserver:start`,
`tool:invoke`, `tool:complete`, `toolserver:stop`. Clone with the
committed opt-out: **0 events**, and the first-run telemetry notice is
not printed.

## Caveat (documented)

The project file is resolved from the working directory of the process.
Editors start the MCP server in the workspace root, so it and the
tool-server it launches read it. The tool-server is shared by every
editor on the machine and reads the project file of the workspace that
started it; the global opt-out or `DO_NOT_TRACK` still covers the
machine-wide case.

## Docs

Updated in this PR: `reference/configuration.mdx` (scopes + merge
wording), `reference/telemetry.mdx` (new "Opt out for a whole
repository" section), `reference/cli.mdx` (`init`, `telemetry` rows),
`fundamentals/installation.mdx` (`--no-telemetry` in local mode).

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

https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu
 2026-08-25 11:24:02 +02:00
Hubert Gancarczyk 50c9580e30 fix(tools): make a schema failure name the parameter that was sent (#718) 
> Stacked on #717.

## The problem

An invalid tool call returned Zod's raw issue array. Someone who wrote
`flow_name` instead of `name` got back, depending on the tool:

```
Invalid params for tool "flow-start-recording": [{"expected":"string","code":"invalid_type","path":["name"]}]
Invalid params for tool "flow-execute":         [{"code":"custom","path":["flow_path"],"message":"Pass exactly one flow source: name or flow_path."}]
```

The first names the parameter the tool **wanted** and never the one the
caller **sent**. The second — flow-execute's, whose `name` is optional
so `flow_path` can stand in — names neither: it is anchored on the one
source field the caller had no reason to send. Either way the mistake is
invisible, and working it out costs a whole turn.

## What changes

**1. `describeParamIssues` — one sentence per bad parameter.**

Missing fields read as `` `name` is required (string) and was not
provided`` rather than as a type error about `undefined`. Unknown keys
are named and path-qualified, so a key nested in `selector` is not
reported as a bare name that contradicts the top-level list printed
beside it. The message closes with the caller's own keys — `You sent:
\`flow_name\`, \`project_root\`.` — which is what makes the mistake
self-evident.

Key names only, never values: this string reaches logs, telemetry and
the agent transcript, and a params object can carry a secret.

Wired into both entry points: the registry's `invokeTool` and the HTTP
boundary.

**2. `flow-execute` accepts `flow_name` as an alias for `name`.**

The tool is called `flow-execute`, so "the flow's name" spells itself
that way; this is a mistake worth absorbing rather than rejecting. The
name is resolved in code rather than by the schema — a Zod `required`
failure would once again name only the field it wanted — and the error
text names the spellings that are silently discarded (`flowName`,
`flow`, `flow_file`), because by then Zod has already stripped them and
the caller has no other way to learn their key was dropped.

**3. A flow DIRECTIVE passed as `command` points at the tool that
records it.**

`command: "echo"` used to come back as a bare "Tool not found". It now
names `flow-add-echo` — and says to call it **directly**, since routing
it through the recorder would write the echo *and* a `tool:
flow-add-echo` step that fails on every replay.

The hint distinguishes directives the recorder **rewrites** from those
it stores **raw** for the polish pass, because promising a rewrite that
does not happen sends the author looking for a directive that is not in
the file. `wait` and `long-press` get their own answers: neither has a
recording tool at all.

Only a genuine not-found is rewritten — a tool that ran and failed still
reports its own error.

**4. A recorder tool as `command` is refused.**

`flow-add-echo`, `flow-add-step`, `flow-start-recording`,
`flow-finish-recording` each mutate the recording itself, and nesting
one reported success either way. Each is refused for its own reason:
`flow-add-echo` would write the echo **and** a replay-breaking `tool:`
step, `flow-start-recording` would **erase** this flow at replay,
`flow-finish-recording` would end the take it is a step in, and
`flow-add-step` cannot record itself. Now nothing is executed and
nothing is written.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-08-25 10:48:49 +02:00
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>
 2026-08-24 12:16:20 +02:00
schicaelmott dd5d823833 feat(flow): stream run progress as NDJSON (#855) 
## Summary

- add `argent flow run --json-stream`
- stream flow progress as JSON lines
- finish with a JSON result or error

## Why

`--json` prints only after a flow finishes. `--json-stream` lets scripts
and CI see live progress.

## Why this is a separate PR

This is useful without the custom-tools feature in #856.

## Validation

- `npm test -w @argent/cli -- flow.test.ts` — 106 tests passed
- `npm run typecheck:tests -w @argent/cli`
- `npm run build -w @argent/cli`

---------

Co-authored-by: Filip131311 <contact@filipkaminski.com>
 2026-08-21 12:14:34 +02:00
Jakub Piasecki abe5ff3c56 refactor(flow): generalize when's block machinery so a second block directive can't miss a site (#748) 
`when` is the only directive with a sibling `steps:` list, and four
runner sites asked `kind === "when"` directly to decide whether a step
has authored children to expand as skip lines. A second block directive
would have had to remember all four, and a forgotten one silently drops
a whole block from the report — exactly the invariant
`reportBlockSkipped` exists to hold.

This introduces `blockSteps(step)` as the single runtime predicate and
`BLOCK_DIRECTIVE_KEYS` as its parse-time counterpart, and extracts the
parse-side block plumbing (`assertBlockDepth`, `parseBlockSteps`) that a
second directive must share — notably the depth cap, renamed
`MAX_BLOCK_DEPTH`, which has to be ONE counter across directives or an
alternating nesting chain evades it and reopens the cyclic-YAML-alias
hole it was added to close.

No behavior change, with one deliberate exception: the depth-cap message
now reads `block directives nest deeper than 20 levels` where it said
`when blocks nest deeper than 20 levels` — the cap stopped being
`when`'s the moment it became shared, and a message naming only `when`
would misdirect an author who hit it through a different directive.
Every other error message, their ordering, and the report shapes are
untouched.

559 flow tests pass unchanged.

---

**Stack**

1. **#748 — you are here** · `refactor(flow)`: generalize when's block
machinery → `main`
2. #749 · `feat(flow)`: add `repeat:` blocks → #748
 2026-08-13 21:51:23 +02:00
Hubert Gancarczyk 96ea94f87a feat(flow): add an idle condition that waits for the screen to stop moving (#728) 
> Stacked on #574. Replaces the route-fingerprint approach that #727 and
#729 carried; both are closed.
>
> Rebased off the #715-#726 chain so it can merge on #574 alone. That
chain owns `DirectiveOutcome.warning` / `StepReport.warning` and the
`indeterminate` scoring rule; the small amount of each that this
condition needs is carried here, scoped to `idle`, and will conflict
textually with #724/#725 for whichever lands second.

## One check a selector cannot express

```yaml
- await: { idle: true }
```

Has the screen stopped moving? Flows were substituting fixed `wait:`
steps for it, which either wait too long on every run or too little on
the run that mattered.

## Why it reads pixels as well as the tree

Each signal is blind to what the other sees.

The tree cannot see presentation-layer motion. An iOS push or modal
dismissal commits its hierarchy up front and then animates a layer for a
few hundred milliseconds; a cross-fade or a scrim moves no node at all.
Measured on a real Android emulator: a page animating a large element
continuously reads as a single static `WebView` node, and the existing
`await-screen-idle` tool called it settled in **465 ms, five times out
of five**. On a deterministic Chromium page whose DOM is byte-identical
across the animation, that tool returned `settled: true` in **407 ms**
while this condition failed 8/8.

Pixels alone would not do either: they cannot see a tree still churning
behind an unchanged surface, and anything animated forever (a video, a
shimmer) would make a pixel-only settle unsatisfiable on a screen the
tree calls ready.

Unlike that tool, this cannot be silently stepped over. Readiness is not
an acceptance criterion — the verdict belongs to the identity and
outcome checks around it — so a screen that never settles still
**passes**, carrying a `warning` that says what the green actually
bought. That is what makes it safe to persist in a flow: it reports on
every run without ever failing one over a video or a shimmer. The only
thing that stops a run is a tree source that cannot be read.

## The rule the design follows

The absence of evidence is never evidence.

- A capture that did not arrive cannot complete the hold.
- Neither can a single agreeing pair. Every animation that reverses has
a turning point, and two samples straddling it come back identical while
the screen is moving. On a live 3 s cross-fade that passed a
default-shaped step on roughly **one run in three**; settling now takes
two consecutive still intervals, so `minStableMs: 0` still means three
reads.
- A round is never started without the budget to observe it with. A
round begun with nothing left neither captures nor reads, and both
absences were otherwise recorded as facts about the device.
- No verdict comes from a latch. A screen that settles and then moves
again, or goes blank, has not settled.

## `timeout:` is a real bound

No describe path takes an abort signal, so the tree read is raced
against what is left of the budget. Without that, a wedged ViewInspector
RPC ran 2.25 s past an 8000 ms budget, and a busy Android screen ran
5.4-6.0 s against a 3000 ms one. A read that runs out of step budget is
the step ending, not the source failing, and only the latter is reported
as an environment problem.

## Failure modes stay apart

They call for opposite responses. A screen that never stopped moving is
a verdict about the app. A tree that could not be read is not, and it
names the foreground check first, because a backgrounded app reads
identically to an uninstrumented one and relaunching is the wrong
repair. A run whose captures never produced a comparable pair still
settles on the tree and says so in a warning, rather than passing off
half a proof as the whole one.

## Capture routing and tolerance

Captures route exactly as the `screenshot` tool routes them, so tvOS and
Vega, which have no simulator-server backend but are perfectly
screenshottable through `xcrun` and the emulator console, are covered
rather than written off.

The pixel comparison owns its tolerance rather than borrowing
`screenshot-diff`'s. That one holds a baseline stored across sessions,
machines and OS versions against a live capture and must absorb real
drift; this one holds two captures one poll apart from a single session,
where a static screen reads back byte-identical (measured: zero changed
pixels across five consecutive pairs on an idle simulator). The margin
is load-bearing, because uniform change is all-or-nothing: at the
baseline tolerance any cross-fade slower than ~2 s counted zero changed
pixels and read as settled mid-animation.

## Not a screen check

A dropped tap leaves the source screen perfectly idle. It belongs after
the element check that names the destination, never instead of one. #730
documents the pairing.

## Verification

- 3307 tests pass; typecheck, `typecheck:tests`, lint and prettier
clean. Eleven mutations applied to the source, all eleven caught by
tests.
- Driven end to end against a real iOS simulator, a real Android
emulator, and a Chromium app through a tool-server built from this
branch: static screens pass with no warning (proving captures ran),
animating screens pass carrying the warning that names the motion, and
the pre-existing tool disagrees exactly where it should.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-08-10 11:50:16 +02:00
Jakub Piasecki 6d907cc86b feat(flow): boot a Chromium instance per launch step (#585) 
A `launch:` on Chromium now starts its own Electron instance, so nested
e2e flows and mid-flow relaunches work as they do on iOS/Android.
Previously the runner booted one app per run and rejected any second
chromium launch.

A chromium device id _is_ its CDP port, so a launch that starts an app
from scratch has to boot one and move the run onto it: `state.device`
became reassignable (steps already read it per call), and `state.owned`
tracks what the runner booted, torn down in reverse at run end. An
instance the runner did not boot — pinned via `--device`, or
auto-detected — is attached to, never killed. Relaunching the same app
kills its instance and waits for the process to actually exit, or the
replacement loses a race against the app's single-instance lock.

The runner needs a device before step 1, so `resolveRunDevice` follows a
leading `run:` chain to find the launch the run _begins_ with: a
fragment composing a chromium e2e flow now boots instead of failing with
"No booted chromium device found". A launch declaring no app id for the
run's platform stays an error — the run's platform is fixed start to
finish.

Also fixes a pre-existing teardown leak: the spawned handle is the npm
`electron` node wrapper, not Electron, so signalling it left the app and
its helpers running (reparented to launchd, still in the dock). Teardown
now sweeps the child's process group.

**Behavior change:** a fragment leading with `run:` into a chromium e2e
flow now boots a fresh instance where it previously attached to a
running one; pass `--device` to keep attaching.

## Tests

Cover the boot/attach/settle three-way, teardown ordering and the group
sweep, device rebinding after a nested launch, same-app retire ordering,
cancellation mid-boot, both "no chromium app declared" guards, and the
leading-`run:` chain (multi-hop, cyclic, no-launch). The guard,
exit-wait and sweep tests are mutation-checked.
 2026-08-06 07:28:27 +02:00
Jakub Piasecki 22033e1418 feat(flow): allow to run every flow in a directory from flow run (#576) 
`argent flow run` now accepts a directory and runs every flow in it
sequentially:

```
argent flow run .argent/flows --recursive
```

Discovery uses the same acceptance rules as `flow list` (lowercase
`.yaml`, safe stem, readable file), sorted for a deterministic run
order. `-r`/`--recursive` walks subdirectories too, skipping
dot-directories and `node_modules` and never following directory
symlinks. A directory named `foo.yaml` still batches (stat wins over
extension).

Batch output is failures-only: each flow prints a `[i/n] <path>` header,
its failing/errored steps — numbered exactly as a single-mode rerun
would number them — and a per-flow summary; the run ends with a
`PASS/FAIL — N flows: X passed, Y failed, Z skipped` verdict. `--json`
prints one aggregate object with per-flow reports. Exit code reflects
the batch (0 pass, 1 any failure, 2 discovery/config errors).

`--output` exports keep the single-run `<output>/<flow>/` layout, keyed
by subdirectory in recursive runs so same-stem flows in different
directories can't clobber each other's snapshots.

**Failure semantics:** a flow failing its steps — or one the tool-server
rejects as invalid (bad YAML, unparseable step) — fails alone and the
batch continues. An infra error (transport death, unclassified failure)
stops the batch and counts the remaining flows skipped. To make that
distinction possible, tool-error responses now carry the server's
failure signal (`error_code`/`error_kind`) on the wire — on the
streaming terminal error line and on JSON error bodies — and the tools
client surfaces server-answered errors as a typed `ToolInvocationError`.
Against a pre-signal server this degrades to stopping on any throw.

Not breaking: single-file runs behave exactly as before.
 2026-08-05 14:51:42 +02:00
Jakub Piasecki cc2f907ce9 feat(flows): resolve run: directives from explicit YAML paths (#567) 
A `run:` value is now a YAML path resolved against the directory of the
flow file that contains the step, matching the CLI cutover:

```yaml
- run: login.yaml
- run: ../shared/login.yaml   # cross-directory composition now works
```

Bare names fail at parse with a migration hint (`run: login` → `run:
login.yaml`). The filename stem stays the fragment's report attribution;
reports also carry the as-written path as `target`. Cycle detection
compares realpath-canonical paths (same stem in two dirs ≠ cycle; same
file via symlink/spelling = cycle). Snapshot baselines are unchanged —
keyed by and stored beside the root flow.

An uploaded root flow with `run:` steps is now rejected up front with a
contract error instead of a mid-run temp-dir ENOENT (its referenced
files stayed on the client, and relative resolution in the temp dir
could reach host files); same for a relative chromium app path. The
recorder emits the sibling form (`run: <name>.yaml`).

**Breaking:** existing `run: <name>` directives fail at parse until
renamed to `run: <name>.yaml`. Snapshot baselines for a saved flow whose
YAML is a symlink now anchor beside the real file (the symlink target's
directory) — move any committed `.argent/flows/__baselines__/<flow>/`
set beside the real file once, or the next run fails with a missing
baseline. A `run:` target must also resolve inside the project root or
beside/below the flow file that references it (arbitrary host YAML is no
longer reachable), and an uploaded flow with a `snapshot` step is
rejected up front (its baselines cannot anchor to the per-call temp
dir).
 2026-08-05 14:51:42 +02:00
Jakub Piasecki a644d52d39 feat(flows): run CLI flows from explicit paths (#568) 
`argent flow run` now takes an explicit YAML path instead of a
saved-flow name:

```
argent flow run .argent/flows/checkout.yaml --platform ios
```

The path may point anywhere on the local filesystem; the filename stem
names the run's report/artifacts, so it must be `[A-Za-z0-9_-]+` with a
lowercase `.yaml` extension. A bare name fails with the exact
replacement command, and `argent flow list` prints paths instead of
names.

Because sibling `run:` files and `__baselines__` resolve beside the
YAML, the CLI now requires the auto-started local tool server and
refuses `ARGENT_TOOLS_URL` / `argent link` routing. The `flow-execute`
tool stays remotely callable via `name` + `project_root`, and gains a
`flow_path` parameter (exactly one of `name`/`flow_path`) that is only
honored when the file-input boundary verified it as a real host path —
uploads and boundary bypasses are rejected, so it can't execute
arbitrary server YAML.

Also: the flow-name pattern is now a single shared export from
`@argent/registry`, and the recorder composes a `flow_path` call into a
`run:` step when it targets a sibling of the recording.
 2026-08-05 14:51:42 +02:00
filip131311 19782f4e1a feat(secrets): resolve {{secret:…}} from .env files, not just the environment (#734) 
## Why

A credential could only reach a typing tool through an
`ARGENT_SECRET_<NAME>` environment variable. That is the right CI
contract — and stays the highest-priority source — but interactively it
is poor ergonomics: an editor-hosted tool-server inherits the
environment its editor was launched with, so changing a credential means
restarting the editor, and there is nowhere to write one down that
survives a reboot. "Is loading these from a `.env` file on the roadmap?"
is the recurring ask.

## What

`{{secret:NAME}}` now resolves through an ordered chain, first match
wins, re-read on every resolution — so editing a file applies to the
next keystroke with **no server restart** (the same runtime-read design
`ios.additionalDeviceSets` already uses).

| # | Source | Which keys it exposes |
| - | ------ | --------------------- |
| 1 | `ARGENT_SECRET_<NAME>` in the environment | prefixed variables
only — unchanged, still wins |
| 2 | `<project>/.argent/secrets.env` | every key (gitignore it) |
| 3 | `<project>/.env.local`, then `<project>/.env` | **only
`ARGENT_SECRET_`-prefixed keys** |
| 4 | `~/.argent/secrets.env` | every key, any project |

Covers `keyboard`, `paste`, `run-sequence` keyboard steps, and flow
`type:` steps — all four already funnel through the same resolution
point, so nothing else changed.

Also adds `argent secrets [list] [--json]` so a setup is verifiable
without an agent: names only, never values, shadowed names marked, and a
warning when a linked remote tool-server means resolution happens on a
different machine.

## The one design decision worth reviewing

**Row 3 is why this isn't just "read the `.env`".** An agent composes
the tool call, so anything reachable through `{{secret:…}}` is
exfiltratable by a prompt-injected one — type it into a field, then read
the field back. "Which values may argent type" has to stay an allowlist
the user wrote deliberately.

So: a *dedicated* argent secrets file exists for no other purpose and
therefore **is** the allowlist; a file the app *shares* keeps the prefix
as the allowlist, exactly as the process environment does. Your
`STRIPE_SECRET_KEY` sitting in the same `.env` stays unreachable —
verified live, not just asserted. Adding `ARGENT_SECRET_` to one line is
the deliberate act that exposes it.

Two smaller calls, both reversible:

- The chain lives in `@argent/configuration-core`, not the tool-server,
so the CLI reads the same code; the tool-server keeps only the
placeholder engine.
- The project scope is discovered by the **cwd walk-up every other
project-scoped configuration uses**. A per-request project root (header
→ `ToolContext`) was considered and rejected as new protocol surface
across two client paths; reusing flows' module-global
`setActiveProjectRoot` was rejected as a cross-project leak on a reused
server. The cost is that a tool-server whose cwd sits outside the
project skips project files — mitigated by the failure now listing every
path it consulted, and by `~/.argent/secrets.env` needing no project at
all.

## Verification

Unit + integration: tool-server 3122, configuration-core 122 (19 new),
argent-cli 370 (6 new). Lint and prettier clean. All four publish
bundles build.

Live, driving this branch's tool-server on :3001 against iPhone 17 Pro /
iOS 26.4 and the Electron testbed:

- global `~/.argent/secrets.env` → typed correctly; the response echoed
only the placeholder
- prefixed key in the project `.env` → typed; **unprefixed
`STRIPE_SECRET_KEY` in that same file → rejected**
- `.argent/secrets.env` beat `.env`; the environment variable beat both
- edited a secrets file mid-session → applied on the very next call,
same server pid
- a chromium flow with `type: { text: "{{secret:APP_PASSWORD}}" }` → 3/3
pass, asserting on the greeting the app rendered, so the *correct value*
provably landed
- an undefined name inside a flow surfaces the full source list in the
step report

Sample of the failure output (no value ever appears):

```
Unknown secret "LOGIN_PASSWORD" — no source on the machine running the tool-server defines
LOGIN_PASSWORD. Available secrets: APP_USER.
Sources consulted, first match wins:
  1. environment (ARGENT_SECRET_*) — 0 secrets
  2. /path/to/project/.argent/secrets.env — found, 1 secret
  3. /path/to/project/.env.local — not found
  4. /path/to/project/.env — found, 1 secret
  5. /Users/me/.argent/secrets.env — not found
To make it available, ask the user to add `LOGIN_PASSWORD=…` to …
```

## Out of scope

No general flow variable/parameter system — a non-secret external value
(a test account's email, a staging tenant id) can use the same
placeholder, but argent treats every such value as a secret: redacted
from errors, never echoed, so it is not for something a report should
show. No config key pointing at an arbitrary secrets file. Remote
(`argent link`) still resolves on the remote host, which the CLI now
says out loud.
 2026-08-04 15:23:12 +02:00
filip131311 2278115028 fix(flow): run a device-less flow without a device, and say why its counts are zero (#649) 
Fixes #644.

## Before

```
$ argent flow run echo-only          # .argent/flows/echo-only.yaml: two `echo` steps
[Tool:flow-execute] No booted device found. Pass a device id or platform explicitly.
Available devices: 18BE573F-… (ios, Shutdown), <70 more>
```

and when a device *was* booted:

```
PASS on 18BE573F-… — 0 passed, 0 failed, 0 errored, 0 skipped
```

## After

```
$ argent flow run echo-only          # nothing booted
Flow "echo-only"
  › first step
  › second step

PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps)
```

## Symptom 1 — the device

Whether a device is needed is decided from the flow's own steps, before
resolution.

Classification is **per step kind and defaults to needing one**, so a
kind added later inherits
today's behaviour instead of silently running against no device — and
the `never` binding makes
leaving a new kind unclassified a compile error, backed by a test keyed
on the step union so it
fails to build rather than to run.

Two classifications worth calling out:

- **`when` always needs a device**, whatever its body contains, because
the guard reads one itself
(its platform, or its view tree). A `when: {platform: ios}` block
containing only narration still
  resolves.
- **`run` needs one without the fragment being read here.** The named
flow is resolved at run time;
resolving it a second time would duplicate that lookup and could
disagree with it if the file
changed in between. A deliberate over-approximation: composing a
narration-only fragment still
  resolves a device. Stated in the code and in the tool description.

For `tool:` steps the tool's own schema decides, via a superset of the
keys the runner injects —
`device` is included because a nested `flow-execute` step drives a
device without being handed the
run's own. A tool that declares no input at all
(`stop-all-simulator-servers`) is device-free, and
reading its absent schema must not throw — it previously would have.

`ExecState.device` is nullable, which is the safety mechanism rather
than just typing: the compiler
enumerates every site that acts on a device, each of which now says so
via one `deviceEnv()`
narrowing. `flow-actions.ts`, `flow-visual.ts` and `ActionEnv` are
untouched. The executor also
re-checks each step against the **same predicate**, so if the scan and
the executor ever disagree
the step reports it instead of failing obscurely deeper in.

A run that resolved no device reports `device: ""`, and neither renderer
claims it ran somewhere.

## Symptom 2 — the counters

Reproducing this showed the issue's framing is slightly off: `--json`
doesn't only disagree with the
text, it disagrees with **itself** — counters all zero while its own
`steps[]` marks both steps
`"pass"`. So the CLI text was a faithful rendering of the server's
counters.

**The counters are right and stay as they are.** Narration is
deliberately excluded, and it should
be: after a hard stop every remaining step *including narration* is
reported skipped, so counting it
would inflate `skipped` on every real failing run — a worse and far more
common lie than the vacuous
one being reported here. It would also contradict the JUnit model in
#578, where echo is
`<system-out>` and not a `<testcase>`.

What was missing was saying *why* zero. Both the CLI and the MCP
renderer now append
`(no test steps)`, and only on a pass — a cancelled run can be a FAIL
with all four counters zero,
where "no test steps" would read as though the failure had no cause.

No wire format changes: `steps[].kind === "echo"` already distinguishes
counted from uncounted steps,
so the JSON is self-describing without widening the status enum.

## Conflicts, for whoever lands second

This is a hot area — seven open PRs touch `flow-run.ts`.

- **#578** rewrites the CLI summary line I also change (one hunk, one
7-line function, semantically
independent — resolution is keep-both), and its
`attachFailureDiagnostics` dereferences
`env.device.platform`. **It will need a null-device guard**, or every
failure in a device-less flow
silently degrades to `read-failed` diagnostics. Happy to rebase onto it
rather than the reverse.
- **#585** rewrites `resolveRunDevice` at the same seam and edits the
same description sentence.
- **#538** rewrites `flow-device.ts`. Worth noting it makes
`resolveFlowDevice` reject a physical
iPhone for flows — skipping resolution means a narration-only flow now
passes on a
physical-iPhone-only host, which I think is desirable but is a second
behaviour change on this seam.

All new tests are in new files, since every existing flow test file in
the blast radius is rewritten
by an open PR.

## Behaviour changes a reviewer should weigh

- A device-free flow no longer prints `PASS on <id>` **even when a
device is booted**. That is the
point: otherwise an identical flow reports differently in CI (no
simulator) than on a laptop with
  one open. An explicit `--device` is still honoured and still named.
- `--platform` is ignored for a device-free flow, where it previously
errored with nothing booted.
- A 0.18.1 CLI against a newer server would print `Flow "x" on ` —
cosmetic version skew.

## Verification

Live against the built CLI: device-less flow with nothing booted (the
reported bug) and with a device
booted (identical output, no ambient attribution); `--json` showing
`device: ""`; explicit `--device`
still naming it; a mixed flow and a composed flow both still demanding a
device; a mixed flow with a
device booted still reporting `PASS on <id> — 1 passed`.

24 new tests. tool-server 3103 passed, CLI 284 passed, MCP 78 passed —
no existing test modified.

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

---

> **#697 is stacked on this branch** — it conflicts with this one in
`flow-run.ts`'s import block, so it was rebased on top. Merge this
first.
 2026-08-03 16:14:30 +02:00
filip131311 81de9f66a6 fix(config): say what a rejected value should have been, and prune the group it left behind (#650) 
Fixes #643.

## Before / after

```
$ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project
Error: Invalid value for config key "ios.additionalDeviceSets".        # ← the whole message
```
```
$ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project
Error: Invalid value for config key "ios.additionalDeviceSets": expected an array of strings.
Did you mean: argent config set ios.additionalDeviceSets '["~/Library/Developer/Radon/Devices"]' --scope project
Run `argent config list` to see each key's expected value.
```
```
$ argent config unset ios.additionalDeviceSets --scope project
$ cat .argent/config.json
{ "ios": {} }        →        {}
```

## Symptom 1 — the message

The issue names three gaps: no expected type, no example, no pointer to
`config list`. All three are
closed, plus one the issue didn't ask for and I think is the actual fix.

**The suggestion carries the user's own value.** A generic `Example: …
'["~/DeviceSets/ci"]'` would
hand someone a path they have never heard of and expect them to spot the
difference. Where the
correction is mechanical — a single item offered to a list-valued key —
the error offers back what
they typed, wrapped, shell-quoted, and **on the scope they asked for**.
Pasting it works; verified
live that it lands in project scope, not global.

Nothing is coerced. The issue floated accepting a bare string and
storing it wrapped; I didn't,
because `config set` persists, gets committed, and is read by other
machines, so a value silently
stored in a shape the user didn't type is expensive to debug and
`coerceCliValue` already has one
silent fallback. Showing the corrected command costs the user one
keystroke and keeps the store
honest. If a reviewer prefers coercion, this is the smaller change to
build on.

**The expected shape comes from the validator, not a per-key string** —
`asStringArray` *is* "an
array of strings", so swapping a key's `parse` swaps its description
with it and the two cannot
drift. A bespoke validator can override via a new optional `expected`,
and degrades to today's exact
message if it says nothing.

**`config list` states it too.** The issue notes `list` doesn't close
the gap either — its
description describes the *elements* but never says the value is a list.
It now shows
`value: an array of strings, e.g. ["~/DeviceSets/ci"]`, and `list
--json` carries both fields.

`telemetry.enabled` deliberately gets no example: `setConfigValue`
refuses it before validating
(it's managed by `argent telemetry`), so a "here's how to set it" hint
would point at a command you
may not run. A schema guard test keeps that honest as keys are added —
and asserts every example
**actually round-trips through its own validator**, so an example can
never ship that reproduces the
error it exists to fix.

## Symptom 2 — the empty group

Deleting a key now removes any container the delete emptied, stopping at
the first ancestor that
still holds something.

**This is file hygiene, not a behaviour change.** I traced every raw
reader of the document —
`readConfigObject`, and telemetry's `consent.ts` and `notice.ts`, which
each do their own read — and
an empty parent is already indistinguishable from an absent one:
`getAtPath` returns `undefined`
either way, before `parse`, so the merge layer never sees a container at
all. The only observable
difference is the bytes on disk, which is exactly the complaint.

Guards worth noting:

- **The root always survives** — an emptied config is `{}`, never a
deleted file. `.argent/` is
itself a project-root marker, so removing the file wouldn't restore the
prior state anyway, and
`updateConfig` is a shared publish path with a lock/rename protocol that
has no defined meaning for
  file removal.
- **A no-op unset still doesn't touch the file.** That fast path exists
so an unset that removes
nothing can't materialise a project config and dirty git status; pruning
lives inside the mutate
callback, which the fast path returns before reaching. So a group
someone deliberately left empty
by hand is never tidied away — pinned by a test asserting the file stays
byte-identical.
- Siblings at any level stop the unwind, including an empty array value,
which is preserved.

**Two existing tests asserted the old behaviour and now assert the new
one** —
`config-access.test.ts:47` and `config.test.ts:109` (the latter is
`clearRememberedAgent`'s
sibling-preservation test, so it doubles as proof siblings survive).

## Known same-class case, not fixed here

`resetFirstRunNotice` (`packages/telemetry/src/notice.ts`) does its own
inline delete and still
leaves `{"notices":{}}`. It doesn't go through `deleteAtPath`, so it
doesn't inherit this. It's a
one-line change to route it through the same function — happy to do it
here or as a follow-up, but
it pulls the telemetry package into a config PR so I left it out by
default.

## Verification

Live against the built CLI: the reported case; pasting the suggestion
back and confirming it lands in
**project** scope; unset leaving `{}`; a sibling key surviving; a
non-array key correctly getting the
generic example and *no* bogus "Did you mean"; `config list` output;
`telemetry.enabled` still
delegating unchanged.

configuration-core 103 passed, argent-cli 285 passed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-08-03 14:56:40 +02:00
filip131311 3fff176a32 fix(cli): let a boolean flag take the value you typed (#699) 
Fixes #586.

## The bug

`--includeChildren false` left the flag **true** and silently dropped
the word, so the tool ran with the opposite of what was asked. The
reporter spent a while chasing a filter bug that did not exist — the
output was simply the unfiltered kind.

Reproduced on 0.18.1, device-free, with a discriminator that shows the
flag's value directly:

```
$ argent run screenshot-diff --udid U --baselinePath A.png --currentPath B.png --captureCurrent=false
{ "summary": "Screenshot diff summary … pixel_mismatch: 1.79% …"      <-- false, diff runs

$ argent run screenshot-diff --udid U --baselinePath A.png --currentPath B.png --captureCurrent false
[Tool:screenshot-diff] Provide either currentPath or captureCurrent, not both.   <-- stayed TRUE
```

That message is the whole problem in one line: the user typed `false`,
and the tool refuses on the grounds that they asked to capture.

## Why the old behaviour existed, and why it does not need to

The parser deliberately declined to look ahead:

> `// Bare boolean flag: true. Don't consume next argv to avoid
surprising behavior when a positional follows.`

That protects a case which does not exist. `parseFlags` has exactly one
caller, and **`positional` is never read anywhere in the repo** — every
bare token is collected and dropped, for booleans and for every other
type. There is no positional to steal. The module header had documented
the space form as legal all along:

```
//   --name value          (string / number / integer / boolean-with-value)
```

This matters because several tool booleans **default to true**, so
`false` is the only way to say what you mean:
`screenshot.includeImageInContext`, `network-request.includeBody`,
`debugger-component-tree.onScreenOnly`,
`debugger-inspect-element.resolveSourceMaps`.

## What changed

A boolean flag consumes a following token when it is exactly the word
`true`/`false`, case-insensitively. `1`/`0` and everything else are left
alone — a bare number after a switch is more likely an unrelated
argument than a value, and guessing at an ambiguous token is what caused
this bug. `--flag -- false` still keeps the word positional.

Three consequences are handled here rather than left to bite later:

- **`argent run` now reports arguments it ignored.** This is what stops
`--flag 0` and `--flag yes` being the same silent inversion, in a syntax
users are now being taught works. stderr, so `--json` stays parseable.
- **`--no-flag true` / `--no-flag false` now name the positive form**
instead of quietly meaning false. One is a contradiction, the other a
double negative.
- **`--flag=True` used to throw** while `--flag True` would now work —
the inline coercion was case-sensitive and did not trim. Both paths now
share one helper, so a single word cannot get two answers one function
apart.

The **flow** and **lens** parsers take switches that accept no value at
all, and would have silently ignored the syntax their users just learned
elsewhere. They now refuse it — in the flow parser that also stops
`false` being swallowed as the flow name, since its first bare token is
the flow to run.

`--help` gains **one legend line** for tools with a boolean field,
rather than widening the flag column on every field of every tool:

```
  Booleans: --flag or --flag true sets true; --flag false, --flag=false, or --no-flag sets false.
```

It deliberately does not begin with `--`.
`scripts/e2e-full/lib/discover-tools.sh` reads any such line in the
`Flags:` section as a flag row and takes the first `--token` as its
name, so a legend starting with a flag would inject a phantom flag into
every tool model it builds. **There is a test pinning that shape**,
verified live: running the harness's awk over the new help output yields
exactly the real flags and no legend row.

## Verified live

```
--captureCurrent false  -> diff runs (was: "Provide either currentPath or captureCurrent, not both.")
--captureCurrent=false  -> unchanged
--captureCurrent 0      -> "Note: ignoring unused argument(s): 0. Pass values as --flag <value> or --flag=<value>."
screenshot --help       -> legend present; harness awk parses only the real flags
```

25 new tests; 8 confirmed to fail before and pass after. The rest pin
what deliberately did *not* change: bare flag still true, a following
flag or non-boolean word left alone, `1`/`0` still positional in the
space form, `--` escape, string and unknown fields untouched,
`--no-flag` semantics, boolean arrays, last-write-wins. The parser had
**zero** boolean coverage before — which is why this survived. Full CLI
suite 298 passing; lint, prettier and `typecheck:tests` clean.

## Scope and neighbours

Only `flag-parser.ts` (the schema-driven parser for `argent run`)
changes semantics. The other CLI parsers take fixed, default-off
switches where `false` has no coherent meaning; three of them already
reject a stray token outright, and the two that did not now do.
`flow.ts`'s deliberate rejection of `--flag=value` is untouched.

**#648** also edits `flag-parser.ts` (rewrites `renderFlagName`, inserts
helpers at the same anchor). Behaviourally compatible — it keeps boolean
→ bare `--name` — so this is a textual conflict only, and either order
works.

## Found while here, not fixed

`scripts/e2e-full/lib/discover-tools.sh:60-64` detects flag kinds with
`\bboolean\b` in awk, where `\b` is **backspace**, not a word boundary —
so it never matches and every non-enum flag is typed `unknown`. That
makes the `bad-type` validation case downstream unreachable.
Pre-existing and unrelated to this fix; filing separately.

---

> **Stacked on #648** (`filip/run-cli-validation-messages`). Both
rewrite the same region of `packages/argent-cli/src/flag-parser.ts` —
#648 factors out `flagNameFor`/`isJsonField` and rewrites
`renderFlagName`, this one adds `booleanWord` beside them and a boolean
legend to `formatSchemaUsage`. Merging them independently conflicts;
this branch is rebased on #648, so review only the top commit. GitHub
retargets it to `main` when #648 merges.

---------

Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>
 2026-08-03 12:18:11 +02:00
filip131311 a6f1bef632 fix(cli): stop argent mcp --help starting the server, and argent tools --help dumping the catalogue (#656) 
Fixes #638.

## The report understated it

The issue says `argent mcp --help` "does not hang and does not start the
server — it simply prints
nothing". Reproducing it showed the opposite:

```
$ argent mcp --help < /dev/null ; echo $?
0                          # 0 bytes  ← what the reporter saw
$ argent mcp --help        # stdin open (a pipe, an editor, a CI step)
                           # HANGS — I hit a 300s timeout
```

`--help` **starts the MCP stdio server**, which blocks reading JSON-RPC
from stdin. The "prints
nothing, exit 0" is just the server getting an immediate EOF from a
closed stdin. So this is the same
"`--help` runs the real command" bug that was fixed for the installers —
`mcp` was missed.

`startMcpServer` takes no argv at all (`mcp-server.ts:96` — its only
parameter is `{ paths }`), so a
help flag was *structurally* unreachable; there was nowhere downstream
that could have handled it.

## Fix

`mcp` joins the existing guarded set rather than growing a second help
path. That set's defining
property is "the handler never inspects its argv", and `mcp` qualifies
more strongly than the
installers do — it isn't merely ignoring `rest`, it is never handed it.

**It cannot fire on a real launch.** The guard tests `rest`, which is
`[]` for every generated editor
config (all four shapes end at `mcp`, and `isArgentManagedEntry`
requires exactly that), and
`[].some(...)` evaluates no predicate. This is pinned in both
directions, because a guard that *did*
fire here would print usage onto the JSON-RPC channel and break every
editor at handshake — the
highest-cost regression in this file, and it was untested.

The top-level table's `mcp` row now reads the same metadata as its own
help, so the two cannot drift.

**No rename.** An earlier draft renamed the module and six identifiers
(`installer-help` →
`command-help`, etc.) on the grounds that its comments had become false.
They hadn't quite: fixing
`mcp` *and* `tools` makes "every other subcommand parses `--help`
itself" true again — I checked all
ten siblings. Since #530 and #568 both edit `cli.ts`, a rename would
have forced them through churn
for no behavioural gain. Prose-only.

## Second commit: `argent tools --help`

The issue mentions it in passing; it's a different mechanism and a
genuinely dead branch.
`argv.filter((a) => !a.startsWith("--"))` dropped `--help` before the
subcommand was read, so it fell
through to listing all 77 tools — **contacting the tool-server, starting
one if none was running**,
to do it. Only `-h` ever reached the existing help branch.

Scoped to the subcommand position on purpose: a global check would have
broken
`argent tools describe <name> --help`, which today prints *that tool's*
flags. It still does — there
is a test.

Separate commit so it can be reverted alone.

## Verification

Live, against the built CLI:

| | before | after |
|---|---|---|
| `mcp --help`, stdin open | hangs indefinitely | usage, returns
immediately |
| `mcp --help`, stdin closed | 0 bytes, exit 0 | 849 bytes of help, exit
0 |
| `mcp` (real launch) | works | works — verified with an actual
`initialize` handshake, server replies with its capabilities |
| `mcp --metro-port 8082` | starts server | starts server (guard keys on
help, not on "any argv") |
| `tools --help` | 77-line catalogue | usage |
| `tools describe gesture-tap --help` | tool's flags | tool's flags |
| `argent --help` `mcp` row | — | byte-identical |

argent 80 passed, argent-cli 284 passed.

## Note on the help text

Every claim in it is sourced (stdio/JSON-RPC transport, stdout
ownership, the `~/.argent/mcp-calls.log`
default and `ARGENT_MCP_LOG` override, auto-spawn vs
`ARGENT_TOOLS_URL`/`argent link`). It deliberately
does **not** suggest running `argent mcp` by hand to check an install —
that is the reported hang — and
instead says it will look like it has hung and why.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-08-03 12:14:24 +02:00
filip131311 d08c3a3cfc fix(cli): name the missing flag instead of printing a Zod dump (#648) 
Fixes #645.

## Before

```
$ argent run gesture-tap
[
  { "expected": "string", "code": "invalid_type", "path": ["udid"],
    "message": "Invalid input: expected string, received undefined" },
  ... two more
]
```

## After

```
$ argent run gesture-tap
Error: missing required flags --udid, --x, --y

argent run gesture-tap [flags]
...full per-flag help...
```

## Scope

The issue names the missing-flag case. Reproducing it showed the same
raw dump has a second
trigger: any constraint the tool itself rejects, which the CLI cannot
know in advance.

| invocation | before | after |
|---|---|---|
| `gesture-tap` (no flags) | raw issue array | `missing required flags
--udid, --x, --y` |
| `screenshot --scale 99` | raw issue array | `--scale Too big: expected
number to be <=1` |
| `gesture-scroll --udid X --x .5 --y .5` (zero deltas) | raw issue
array | `Pass a non-zero deltaX and/or deltaY …` |
| `run-sequence --udid X` | raw issue array | `missing required flag
--steps-json` |
| `gesture-tap --x abc` | already good | unchanged |
| runtime failure | plain message, exit 1 | unchanged |

Fixing only the first row would have left the CLI's error voice less
predictable than before, so
both go through the same renderer.

## How it works

Two sources, one output. Missing flags are found locally from the tool's
published schema — a
**presence** check only, never a type or range judgement, which stays
exclusively the server's.
That is sound because the schema generator excludes defaulted fields
from `required`
(`debugger-connect` requires `device_id`, not `port`), so it cannot
reject a call the server would
have accepted. Everything else is read off the rejection the tool-server
already returns.

Both produce one report rendered by one function and reported through
one call site, so the
wording, the output channel and the exit code cannot drift between them.

Recognition of a rejection is **structural** — the shape of the issue
list plus the tool's own
schema — never the wording of a message, which is not ours to depend on
and changes between Zod
releases. Four gates must all hold, and any failure leaves the existing
handling untouched.

Details worth a reviewer's attention:

- **A rule spanning the whole payload reports no field.**
`gesture-scroll` and `gesture-rotate`
validate at the object level, and `screenshot-diff` is `.strict()`.
Treating those as
field-scoped would print `Error: --undefined …`; they are stated on
their own instead.
- **A rule firing on an *optional* field is not called a missing
required flag.**
`await-ui-element` refines on `expectedText`, which it does not require
— reporting that as
missing would contradict the help block printed directly underneath,
where the field carries no
  `(required)` marker.
- **The check runs after `--args` is merged**, so `argent run
gesture-tap --args '{...}'` is not
falsely reported as missing everything. Verified live: it reaches the
device and taps.
- Presence uses own-key semantics, so a required field named e.g.
`constructor` is not treated as
  supplied by the prototype chain.

## Behaviour changes a reviewer should weigh

- **A tool-rejected value now exits 2 instead of 1.** `run.ts` already
uses 2 for bad input and 1
for a failed run; a Zod 400 means the tool never ran. No in-repo
consumer tests for a specific
code — `scripts/e2e-full` only checks zero/non-zero. Note this does
**not** make 2 universal:
  an unknown tool name still exits 1, which is untouched here.
- **`--json` gains a defined failure shape** (it had none — the blob
printed regardless). One
object on stderr, nothing on stdout, so `--json | jq` on a failed run
reads an empty stream and
  a non-zero status:
  ```json
  { "error": "missing required flags --udid, --x, --y",
    "missing": ["--udid", "--x", "--y"],
    "issues": [] }
  ```
`issues` carries the tool's list verbatim, which is the raw data the
issue asked to keep.
- **One adjacent fix**, called out because it is not the reported bug:
the global-option parse ran
*outside* the error handler, so `argent run screenshot --out` printed a
raw stack trace. Same
  bad-input-to-ugly-output defect, one line above the fix.

## Not done

- The unknown-flag hole (`argent run gesture-tap --bogusflag 1` is
silently accepted and the tool
runs). The pass-through is deliberate — see the comment in
`flag-parser.ts` — and belongs with
  #586, the same CLI-argument family.
- `argent flow` builds its own payload and has its own required check;
it cannot reach this path.
- The server still returns the issue list verbatim, so MCP and other
clients are unaffected. All
  prettifying is CLI-side.

## Verification

Live against the built CLI for every row of the table above, plus:
`debugger-connect` asks only
for `--device_id` and never `--port`; `--args '{...}'` reaches the
device and taps; a thrown
runtime error still prints its bare message with no `Error:` prefix, no
help block, exit 1.

43 new tests. `run-telemetry.test.ts` passes **unedited** — it already
asserted the non-Zod
passthrough, which makes it the regression guard for the untouched path.
Full CLI suite 320
passed, registry 87 passed.

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

---

> **#699 is stacked on this branch** — it conflicts with this one in
`flag-parser.ts`, so it was rebased on top rather than left to collide
at merge time. Merge this first.
 2026-08-03 11:42:26 +02:00
Ignacy Łątka 2251c7cf46 feat(screen-recording): screen-recording-start/stop tools with per-call stop reminders (#517) 
## What

Adds a `screen-recording-start` / `screen-recording-stop` tool pair
(naming follows `native-profiler-start/stop`) that records the device
screen to an mp4 and returns it as a downloadable artifact.

Capture runs through **simulator-server** — the same backend
`screenshot` and every interaction tool already drive. simulator-server
publishes the device screen as an MJPEG stream (`/stream.mjpeg`), so
recording needs no platform-specific recorder: the tool-server
subscribes to that stream, paces the frames onto a fixed 30 fps
timeline, and pipes them into a single ffmpeg process that encodes — and
watermarks — straight to the final mp4. The stream is multi-subscriber,
so a preview window can stay open on the same device while recording.

- **One path for every device simulator-server drives** — iOS
simulators, Android emulators, and physical Android devices.
- **Native resolution, constant 30 fps** — h264, `+faststart`, no
post-processing pass.
- Session state lives in a per-device `ScreenRecordingSession` blueprint
(mirrors the native-profiler session): time-limit cap with
auto-finalize, unexpected-exit recovery (a capture that ended on its own
is still retrievable via stop), always-startable state reset on stop,
and dispose-on-shutdown cleanup through the registry.

## Why the frames are paced host-side

A device only emits a frame when the screen **changes**. Pointing ffmpeg
straight at the stream therefore produces a video whose length is the
sum of the moments something moved — a still screen collapses to a
fraction of a second — and an ffmpeg blocked on a silent socket does not
answer a stop signal promptly either (measured: 12 s late). Re-emitting
the last frame on a wall-clock schedule fixes both: **video duration
always equals real elapsed time**, and stop is a stdin EOF that
finalizes in ~60 ms. Duplicate frames cost almost nothing once encoded
(a 10 s recording of a static screen is ~1 MB).

## Finishing pass folded into capture

The Argent watermark (logo + `By @swmansion`, bottom-left, sized
relative to the frame) is stamped by the same ffmpeg invocation that
encodes the capture, so there is no second encode and no temp
intermediate. Its colour is chosen **per pixel** from the background
luma underneath it — the white logo over dark areas, a runtime-derived
near-black copy over light areas — so it keeps maximum contrast against
whatever is behind it. It sits at 20 % opacity and is opt-out via
`argent disable video-watermark` (read when the recording starts). Frame
geometry comes from the first JPEG's SOF header, so no `ffprobe` is
needed.

ffmpeg is now the recorder rather than an optional finisher, so a
missing ffmpeg fails `screen-recording-start` up front with an install
hint instead of silently degrading the output at stop time.

## Reminder plumbing

While a recording is running — or ended on its own but was never
retrieved — **every** tool result carries a `note` telling the agent the
recording is still going and how to stop it. It rides the same `note`
field as the update reminder but is per-call and unsuppressed; both
notes compose when they coincide. The note text switches to "already
ended, retrieve the file" once the cap fires or the capture dies.

## Skill

New `argent-screen-recording` skill (+ routing stanza in
`rules/argent.md`): documents the start → interact → stop workflow and
instructs the agent to schedule its own reminder/wakeup for the expected
end of the recording immediately after starting one (harness reminder
tool if available, otherwise a background `sleep` whose completion
notification pulls it back). Scores 10/10 on `scripts/grade-skills.mjs`.

## Verification

- Unit: 45 tests across `screen-recording.test.ts`,
`mjpeg-stream.test.ts`, `watermark.test.ts`,
`http-recording-note.test.ts`; full tool-server suite **2808 tests / 280
files green**; `tsc --noEmit -p tsconfig.test.json`, root `tsc --build`,
eslint, prettier all clean.
- E2E on a live tool-server (`POST /tools/...`), driving real
interactions between start and stop:
- **iOS sim** (iPhone 16 Pro Max, iOS 18.5): 1320×2868 h264,
`r_frame_rate=30/1`, 338 frames, container duration 11.27 s vs reported
`durationMs` 11314 — **within 50 ms of wall clock**. Extracted frames
show real screen content (mid app-switcher animation) with the watermark
rendered light-over-dark.
- **Android emulator** (Pixel 3a, API 34): 1080×2220 h264, 30 fps, 353
frames, duration 11.77 s vs `durationMs` 11793 — within 30 ms. 512 KB
for a mostly-static home screen.
- **Static screen** (no interaction at all, 10 s): 300 frames, duration
exactly 10.000 s — the case the previous `recordVideo` implementation
reduced to ~0.03 s.
- **Long silence then activity** (15 s untouched, then an app launch):
stream survived, no warning, frames after the silence show the new
screen. This caught a real bug during review — the connect timeout was
also acting as a socket *inactivity* timeout and tore the stream down at
10 s of stillness; fixed and covered by a regression test that fails
without the fix.
- **Time-limit cap**: `timeLimitSeconds: 5` self-finalized at 4.97 s /
149 frames, the per-call note flipped to "already ended (it hit its 5s
time limit)", and a stop issued 11 s later still returned the video with
the matching warning.

## Not supported (capability-gated with clear errors)

tvOS simulators, physical iPhones, Chromium apps, Vega, and remote
(`remote:`-prefixed) simulators — none of them expose a readable frame
stream.

---------

Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com>
 2026-07-23 21:14:37 +02:00
filip131311 8d2a499eaf Project-scope configuration with schema-driven merge + argent config (#534) 
## Summary

Adds a general, **scoped** configuration system on top of the shared
`config.json`, plus a universal `argent config` command to manage it.
Motivated by the discussion on #401 (iOS device set): config that needs
to work naturally with the MCP server should be read off disk at runtime
like telemetry consent is — not threaded through CLI flags/env and a
kill-and-respawn dance. This lands the reusable machinery; the
device-set becomes a one-line schema entry + a runtime reader on top of
it.

Four pieces, matching the requirements:

### 1. Project-scope config files
`config.json` now resolves at two scopes:
- `~/.argent/config.json` (**global**, default)
- `<project-root>/.argent/config.json` (**project**, resolved by the
same `.argent`/`.git`/`package.json` marker walk as feature flags)

`configFilePath` / `readConfigObject` / `updateConfig` take an optional
`scope` (default `global`), so **every existing caller is unchanged**
(telemetry, notices, lens). Per-scope lock + atomic write are preserved.

### 2. Schema-driven merged reads
`getConfigValue(def)` reads each scope the value's schema entry allows,
merges them under the entry's policy, and falls back to the entry's
default. This is the reader consumers should call instead of poking at a
single global file. The existing **lens** getters (`getRememberedAgent`
etc.) are migrated onto it as the first live consumer — a repo can now
pin its lens agent, falling back to the user's global choice.

### 3. A registration file for configurations (`config-schema.ts`)
Adding a configuration is a one-liner: `key`, `description`, allowed
`scopes`, a `parse` validator, and a `merge` policy. Merge presets:
- `prioritize-local` (project wins) · `prioritize-global` (global wins)
- `prioritize-restrictive` (booleans/numbers — opt-out/stricter wins)
- `union` / `intersection` (arrays)
- …plus an escape hatch: pass your **own** `(inputs) => value` function
for a bespoke rule.

### 4. `argent config` — one universal command
`argent config <list | get | set | unset>` with `--scope global|project`
and `--json`. New configs are added as **schema entries**, not new
top-level `argent` commands, to avoid bloat.

**Telemetry stays backward-compatible** on its dedicated `argent
telemetry` command (which also drains/resets the live client). It's
surfaced *read-only* under `argent config` via a `manageCommand` guard —
so `argent config set telemetry.enabled` refuses and points at `argent
telemetry`, and a committed project file can never re-enable a global
opt-out (`prioritize-restrictive`, global-only).

## Testing

- **Unit / integration**: 71 configuration-core tests (merge presets +
custom fn, dotted-path + prototype-pollution guards, scope merge,
scope/validation/manage-command errors, `listConfig`, `coerceCliValue`)
and 11 real-filesystem CLI tests driving the actual `config()` entry
point across sandboxed HOME + project cwd. Full suites green:
configuration-core 71, argent-cli 268, telemetry 281, argent 60.
- **Live, on the built binary** (`node packages/argent/dist/cli.js …`):
set global + project (from a deep subdir) → effective get returns the
project value; per-scope gets return each file; both files land in the
right place; `argent telemetry disable/enable` writes `config.json` and
`argent config get telemetry.enabled` reflects it read-only; unknown-key
/ invalid-value / bad-scope errors exit non-zero with hints.
- `tsc --build` across the workspace, prettier, and eslint
(`--max-warnings 0`) all clean.

## Notes / follow-ups
- Config-file reads are not yet fingerprint-cached the way telemetry's
consent read is; a long-lived tool-server re-reads on each
`getConfigValue`. Fine for CLI/low-frequency use; a hot runtime consumer
(e.g. a future device-set reader) may want the same mtime/inode cache.
 2026-07-22 12:47:42 +02:00
Jakub Piasecki a0d1bbb1e7 feat(flow): indent nested steps in reports by block depth (#524) 
Flow reports rendered every step flat, hiding which steps ran inside a
`when:` block or a `run:` fragment. The tool-server now stamps
`StepReport.depth` (+1 per block directive, omitted at top level) and
the CLI/MCP renderers indent step labels and under-step lines by it:

```
  ✓  1 launch
  ✓  2 when visible "What's new" — condition met
  ✓  3   tap "Skip"
  ✓  4   run dismiss [dismiss]
  ✓  5     tap "OK" [dismiss]
```

Invariants:

- **Wire-compatible both ways** — a flow with no block directives
reports byte-identical to the pre-depth shape; a pre-depth server
renders flat as before.
- **Skipped ≡ entered** — skipped/errored/aborted blocks report the same
depths as entered ones, keeping reports comparable run-to-run.
- **Untrusted wire data** — renderers clamp depth (display cap, not a
producer bound): hostile values render flat or at the cap, never throw
or allocate huge strings.

Also fixes a pre-existing bug: under-step warning/artifact lines now
stay aligned past 99 steps.

Tests pin depth on every producer path (entered/skipped/errored/aborted
blocks, `run:` success/failure markers, `run:`-in-`when:`) and renderer
alignment incl. the clamp boundary.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-21 07:28:48 +02:00
Jakub Piasecki 3f602a7a9b feat(flow): conditional when blocks (#498) 
## What

A `when:` block runs a step list only when one condition holds — a UI
condition (`exists`/`visible`/`hidden`/`text`, the await/assert shapes)
or a static `platform` test. **No else**, by design: a block exists to
restore determinism (dismiss an interstitial, reconverge), never to
test two paths. Tap-if-present is a one-step block:
`when: { visible: "Got it" }` + `steps: [tap: "Got it"]`.

No per-step `optional:` — `when:` covers every directive once. The key
is rejected at parse with a pointer to `when:`, not silently dropped (a
Maestro habit that would leave a "can't fail" step hard-stopping the
flow).

## How

- The guard reuses the assert engine at the 1s grace (`timeout` is a
parse error), so a skipped block barely costs a clean run.
- Verdicts are evidence-based: no trustworthy read in the window →
indeterminate → the step **errors** ("could not evaluate" is not
"condition false"); a transient read failure after trusted reads showed
the condition false stays a clean skip.
- `platform` folds `ios-remote` into `ios` — the parser rejects that
spelling, so the fold is the only way a guard matches a remote sim.
- A skipped block reports one line per authored step, same shape for
unmet guard, hard stop, or cancellation; everything goes through
`pushReport`, so when lines stream live. Failures inside an entered
block hard-stop as usual.
- Results carry an explicit `aborted` flag; `ok` keys off
fail/error/cancel, not skip lines.
- Nesting is capped at 20, so a cyclic YAML alias is a structured parse
error, not a stack overflow.

## Testing

`vitest` 2546 pass (17 when-specific); `tsc --noEmit` (tool-server,
argent-mcp, argent-cli), eslint, prettier clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-20 10:26:34 +02:00
filip131311 07427917e2 fix(launcher,lens): drop the npm postinstall — its jobs move to runtime (#510) 
## Why

Shipping *any* `postinstall` makes pnpm 10+ wall the install behind its
build gate: `ERR_PNPM_IGNORED_BUILDS`, `pnpm install` exits 1, and `pnpm
exec argent` is unusable until the user runs `pnpm approve-builds` —
their first experience of argent is a bricked CLI. The same script is
also silently skipped by `--ignore-scripts`, Yarn PnP, and locked-down
CI, so anything correctness-relevant in it was already unreliable (the
codebase says as much in several places, e.g. `runTrustingDisk`).

Live-verified during the 0.15.0 QA: with **all builds declined** in pnpm
11, argent installs, inits, and serves MCP fine — the postinstall was
never load-bearing for install correctness. Removing it means pnpm users
never see the approve-builds wall for argent itself.

## What the postinstall did, and where it went

| Job | New home |
|---|---|
| SIGTERM a tool-server stranded by an upgrade that replaced its bundle
dir (pnpm/yarn version-pinned layouts) | `sweepDeadStateFiles` in the
launcher's slow path now retires **live** servers whose recorded bundle
is gone from disk, with the same pid-identity guard as
`killToolServerForInstallDir` (never signal a recycled pid; keep
unidentifiable live records reachable for `server stop`). Same-bundle
in-place bumps were already self-healed by the disk-authoritative
version gate. |
| `chmod +x` node-pty's macOS `spawn-helper` prebuilds (shipped without
the exec bit; first `pty.spawn` otherwise fails `posix_spawnp`) |
`ensureSpawnHelperExecutable()` in `lens-pty.ts`, run when the module
loads, best-effort with the AppleScript fallback intact. |
| Print the `argent init` hint | Dropped — init flows drive the install,
and the hint never printed on pnpm/PnP anyway. |

`ARGENT_SKIP_POSTINSTALL` in `update.ts` is kept deliberately:
downgrades and pinned installs fetch older versions whose postinstall
still exists and honors it.

## Testing

- New `launcher-sweep.test.ts` (6 tests) exercises the sweep against
**real processes**: live-server-with-deleted-bundle is terminated and
its record unlinked; live server with intact bundle untouched;
recycled-pid record kept, not signalled; dead-pid record unlinked;
legacy slot untouched; a SIGTERM-trapping orphan doesn't hold the sweep
(and the spawn lock above it) hostage for its kill grace window — the
record is unlinked and the SIGKILL escalation runs in the background.
- New `ensureSpawnHelperExecutable` tests (real fs, darwin-gated) verify
the +x restore and silent failure; a regression test pins that
`loadNodePty` tolerates req seams without `resolve`.
- Full suites green in all touched packages: tools-client 154, installer
535, cli 248, mcp 76, argent 60.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-07-15 20:26:39 +02:00
Jakub Piasecki 551c781fb0 feat(flow): support running end-to-end flows without agent (#429) 
# Flow E2E

This PR turns flows from replayable tool-call recordings into a
declarative, portable e2e test system that runs headlessly via `argent
flow run <name>` — no LLM in the loop, non-zero exit on failure,
suitable for CI.

Agents can still replay flows via `flow-execute` and get the full output
from each step after it finishes.

## Flow format

Flows are YAML files in `.argent/flows/`, now built from **directives**
interpreted by the runner rather than raw tool calls:

- `launch` — terminate + relaunch the app under test (per-platform app
id map supported); its presence marks a flow as **e2e** (controls its
own start state) vs. a **fragment** (runs against current state,
declares an `executionPrerequisite`, composable via `run:` with
cycle/depth guards).
- `tap`, `type` (focus handshake + optional submit), `scroll-to`
(momentum-free, increments by half the container, `within:` for nested
scrollers), `await`/`assert` (`visible`/`exists`/`hidden`/`text`
conditions), `wait`, `snapshot`, `echo`, and raw `tool:` as escape hatch
— still fully supported, so previously recorded flows keep working
unchanged.
- **Selectors** are `{ text, identifier, role }`; a bare string is a
loose selector resolving identifier-first, then visible text. Any
directive failure hard-stops the flow; later steps report `skip`.

## Execution engine

- Flows store no device id — the runner binds a device at replay
(explicit `--device`/`--platform` or the single booted one).
- Selector resolution runs against **platform-unified UI trees**: iOS
native view hierarchy (full testID coverage, gated on native-devtools
connecting), Android full accessibility hierarchy (full resource-id
coverage), Chromium CDP DOM, and the Vega automation toolkit. A shared
flatten/text-hoisting pass lets `text` assertions read what an
identified container visibly shows. Vega is remote-driven, so touch
directives fail with guidance to use `tv-remote`/`keyboard`.
- Steps wait for the tree to **settle** (two consecutive identical
reads) before acting; `launch` additionally gates on platform
tree-source readiness instead of silently degrading.

## Visual snapshots

`snapshot` diffs a full-resolution screenshot against baselines in
`.argent/flows/__baselines__/<flow>/`, keyed by platform + resolution,
with configurable `maxMismatch` and `--update-baselines`. The status bar
is pinned for the whole run (iOS `simctl status_bar`, Android demo mode)
so clocks/battery don't drive diffs. Baseline/current/diff images are
returned as artifacts on failure (and the written baseline on first
capture/update); a clean pass carries none, keeping reports and agent
context lean.

## Recording

The record-live workflow (`flow-start-recording` → `flow-add-step` →
`flow-finish-recording`) now emits directives automatically: coordinate
taps are rewritten to portable selector `tap:` steps (selector captured
from the pre-tap tree), a leading `restart-app` becomes `launch:`,
`flow-execute` of a sibling fragment becomes `run:`, and device ids are
stripped. The skill instructs the agent to polish the saved YAML
afterwards (fold tap+keyboard into `type:`, swipes-to-reach into
`scroll-to:`, awaits into sugar form).

## Also included

- `argent flow run` / `argent flow list` CLI with per-step report
rendering, artifact materialization, and `--json` output.
- New structured failure codes for flow validation, device resolution,
composition, and devtools readiness.
- Extensive unit test coverage (tree adapters, selector matching,
scrolling, composition, type focus, snapshots, status bar).
 2026-07-13 17:15:01 +02:00
filip131311 7bd017a796 feat(installer): committable repo-local (--local) install mode (#428) 
## Summary

Adds an opt-in **committable / repo-local** install mode so a team can
version Argent _with the repo_. `argent init --local` installs
`@swmansion/argent` as a project **devDependency** and writes MCP
configs that launch the project-local copy — so after `git clone && npm
install`, every teammate has the same Argent version and MCP wiring with
**no global install and no per-developer `argent init`**.

**Global mode stays the default** and is unchanged; everything here is
additive and backward-compatible.

Motivation: the workflow some users already do by hand (#207).
Supersedes the earlier, since-closed #232 — this version is Windows-safe
(`node`, not the `.bin` shim), handles Yarn PnP, and closes the
cross-install tool-server hazards rather than leaving them.

## What changed

**`init`**
- `--local` / `--global` flags + an interactive "How should argent be
installed?" prompt. Mutually exclusive; non-interactive (`--yes`)
defaults to **global**.
- Local mode installs the devDependency with the package manager
detected from the project's **lockfile** (run with `cwd = projectRoot`).
- MCP entry points at the local copy: `node
node_modules/@swmansion/argent/dist/cli.js mcp` (the bin path is derived
from the installed `package.json` `bin` and existence-checked — forward
slashes, committable, Windows-safe). Yarn PnP → `yarn argent mcp`; an
`npx --no-install argent mcp` fallback is used only when the bin can't
be resolved.
- Records the mode in a committed `<root>/.argent/install.json`.
- Drops global-only adapters (Windsurf/Hermes) in local mode; prints
team-setup / commit guidance.

**`update`** — detects the project's mode; in local mode reads the
installed version from `node_modules` (not the global binary / npx
cache) and bumps the **devDependency + lockfile**, then re-emits the
local MCP command.

**`uninstall`** — scoped to the project's mode: a local-mode uninstall
removes the **local devDependency only and never the shared global
install**, and prunes `.argent/install.json`. Global-mode behavior is
unchanged.

**Cross-install hardening (the gate that makes per-project pinning
safe)**
- `launcher`: a tool-server is reused only when its `bundlePath` matches
the caller's — a repo-local pin never silently runs against (or kills) a
different version's server.
- `postinstall`: only stops a tool-server spawned from *this package's
own* bundle, so a devDependency install can't tear down an unrelated
editor session.

**Telemetry** — adds an `install_mode` dimension to
init/update/uninstall events.

## Testing

- New unit tests: install-mode helpers + record (`utils`), mode-aware
`getMcpEntry`/`resolveLocalCommandMode` incl. the opencode array form
(`mcp-configs`), local-only uninstall guard (`uninstall`), and
bundlePath-gated tool-server reuse (`launcher`).
- All existing suites green (installer 346, tools-client 115, telemetry
191, registry 83, argent 13, tool-server 1621). Build + lint clean.
- **Live end-to-end** in an isolated sandbox: `init --local` → real MCP
stdio handshake serves **69 tools from `node_modules`**; `update
--local` reads the local version; `uninstall --local` removes the local
install and **leaves the global install intact**; `init --global`
unchanged; flag conflict rejected.

## Notes / known limitations

- Recommended on mainstream platforms (macOS, Linux x64, Windows x64)
with prebuilt native binaries; Linux arm64 / Windows arm need a C/C++
toolchain to build `tree-sitter` on `npm install` (documented in the
README).
- Concurrent sessions of two *different* Argent versions on one machine
each spawn their own tool-server; an orphaned one idle-times-out (no
session is ever broken).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-07-08 12:24:19 +02:00
Hubert Gancarczyk 3393d22edd feat(telemetry): derive distinct_id from a stable host fingerprint (#444) 
## Summary

Replaces the random-UUID telemetry `distinct_id` with a stable
per-machine id derived from the host fingerprint, so PostHog's native
"Unique users" counts machines (funnels / retention / breakdowns work
with no HogQL). Every Argent entry point - installer, CLI, tool-server,
MCP - derives the id from one shared code path, so a given machine
reports a single identity across every event and across install /
uninstall / reinstall.

## What changed

- `distinct_id = uuidv5(hostFingerprint, NAMESPACE)`, where the
fingerprint comes from `simulator-server fingerprint`. Deterministic,
keeps the UUID shape, and never stores the raw hardware value in PostHog
(only the SHA-1-derived UUID leaves the process).
- The fingerprint is resolved in `buildPayload` - the single `capture()`
path shared by every entry point - so installer / CLI / tool-server /
MCP all produce the same id. Resolved lazily, once per process,
memoized.
- One-time, local-only migration: the first time a fingerprint resolves,
`~/.argent/telemetry-id` is rewritten from the legacy random UUID to the
derived id. No PostHog `alias` / `$identify` event is emitted.
- Fallback: if the fingerprint can't be resolved (unsupported platform,
binary absent), the stored or a fresh random UUID is used - identically
for every entry point on that machine, self-healing to the derived id on
the next successful resolve.
- Hardening: never throws; the fingerprint is validated (exactly 64 hex,
case-insensitive) before hashing; corrupt / empty / unreadable id files
self-heal; the binary is spawned at most once per process.

## Why the id is stable across install -> uninstall -> reinstall

Uninstall deletes `~/.argent/telemetry-id`. Because the id is
`uuidv5(fingerprint)` and the fingerprint is deterministic per machine,
a fresh install recomputes the identical id. Previously only the
tool-server resolved the fingerprint while the installer emitted under a
random UUID, so this cycle produced a different random id each time.

## Bundling note for reviewers

`@argent/telemetry` now depends on `@argent/native-devtools-ios` to
locate the binary. That package computes its bin dir from `__dirname` at
module load, which is undefined in an ES module. The installer / CLI /
MCP ship as ESM bundles, so `bundle-tools.cjs`'s ESM banner is extended
to shim `__dirname` / `__filename` (the CJS tool-server bundle already
has them natively). Without this shim those bundles would crash on load,
so the banner change is load-bearing.

## Testing

- Unit: 219 telemetry tests - RFC 4122 v5 vectors, migration,
idempotency, case/length validation, corrupt-file self-heal,
spawn-at-most-once, and all fallbacks.
- E2E on a real machine + booted iPhone 16 Pro: installer, CLI, and
tool-server runtimes all emit the same derived id; the id is stable
across a delete/recreate (uninstall/reinstall) cycle; a legacy v4 id
migrates locally with no alias/identify emitted; verified from an ESM
bundle placed in the real install's `dist/` so the binary resolves via
the bundled `__dirname` with no env override; tool calls against the
device carry the same id.
 2026-07-07 16:28:29 +02:00
Ignacy Łątka c5bab2a83d fix(cli): let a tool's own --args field win over the whole-payload flag (#458) 
## Bug

`argent run flow-add-step` declares its own `args` field — a JSON string
holding the recorded step's tool arguments. But the CLI flag parser
hardcoded `--args` as a whole-payload escape hatch, so `--args` shadowed
the tool's own `args` field. The documented per-flag form

```
argent run flow-add-step --command gesture-tap --args '{"udid":"...","x":0.5,"y":0.5}'
```

silently dropped the step arguments: the `--args` JSON was consumed as
the whole payload and the tool's `args` field stayed `undefined`,
producing a misleading downstream validation error. Only the nested
full-payload form worked.

## Fix

Make `--args` schema-aware in `parseFlags()`:

- When the tool's schema declares a property named `args`
(`schema.properties.args`), `--args <value>` is routed through the
normal per-field handling and coerced by its declared type (a `string`
for flow-add-step, so the raw JSON string passes straight into the
`args` field). Both `--args value` and `--args=value` forms are
supported.
- When the schema does NOT declare `args`, the existing whole-payload
behavior is preserved exactly, including the `--args -` stdin sentinel.

`printToolHelp()` no longer prints the global whole-payload `--args
<json>` / `--args -` lines for tools that declare their own `args`
field, so the help stops contradicting the per-field `--args` shown in
the schema flags.

The change is general: any tool with an `args` field benefits, with no
special-casing by tool name.

## Verification

- `npm run build` (root tsc) — clean.
- `npm test -w @argent/cli` — 172 passed, including 6 new `parseFlags`
cases covering schema-with-`args` (space-separated + inline forms, plus
sibling fields) and schema-without-`args` (whole-payload preserved,
`--args -` sentinel, no-schema fallback).
- Prettier + ESLint (`--max-warnings 0`) clean on the touched files.

Full CLI end-to-end (running `flow-add-step` against a live tool-server
with an active flow recording) was not exercised here; the added unit
tests prove the parser behavior.

Fixes #452
---

### CLI end-to-end verification

Ran the documented per-flag form against a live tool-server with an
active flow recording:

- **Before (global argent 0.13.0):** `argent run flow-add-step --command
gesture-tap --args '{"udid":"...","x":0.5,"y":0.35}'` → `Invalid params
for tool "gesture-tap": udid expected string, received undefined ...`
(args dropped).
- **After (this branch):** same command → `{ "message": "Step added to
\"colltest\" flow", "toolResult": { "tapped": true } }`, and the
recorded YAML contains the step **with** its args (`udid`/`x`/`y`). The
per-flag form now works as documented.
 2026-07-07 13:37:09 +02:00
Ignacy Łątka a7722df8a2 fix(cli): reject empty numeric flags and array/-json interleave crash (#432) 
Fixes two related defects in `packages/argent-cli/src/flag-parser.ts`,
the argv-to-JSON flag parser used by `argent run`. The package
previously had no dedicated test file for it; this PR adds one covering
both fixes.

## Defect A: empty/whitespace numeric values coerced to 0

`coerceScalar` validates number/integer fields with `Number.isNaN` /
`Number.isInteger`, but `Number("")` and `Number(" ")` both evaluate to
`0`, so those guards pass. An empty or whitespace-only value was
therefore silently accepted as `0` instead of being rejected.

Reachable via `--x=` (empty inline), `--x ""`, and `--x "   "`.

- Observed: `parseFlags(["--x="], { type: "object", properties: { x: {
type: "number" } } })` returned `{ x: 0 }`.
- Expected: rejected, the same way a non-numeric string like `"abc"`
already is.

Fix: reject an empty or whitespace-only value up front for
number/integer types, throwing the same `FlagParseException` the
function already throws for `"abc"` (`--<flag> expected a
number/integer, got "<raw>"`). Valid numbers are unaffected.

## Defect B: array + `-json` interleave throws a raw TypeError

`seenArrayFields` is only populated by the array branch, but the
`--field-json` escape hatch writes `args[fieldName]` directly and can
set it to a non-array. If a field was already seen as an array, a later
array-form occurrence ran `(args[flag] as unknown[]).push(...)` on a
non-array value and threw a raw `TypeError`. `run.ts` only catches
`FlagParseException` and re-throws anything else, so the user got an
uncaught stack trace instead of a clean error.

Repro:

```
parseFlags(
  ["--tags", "a", "--tags-json", '"b"', "--tags", "c"],
  { type: "object", properties: { tags: { type: "array", items: { type: "string" } } } }
)
```

- Observed: throws `TypeError: args[flag].push is not a function`
(uncaught, surfaces as a stack trace through `run.ts`).
- Expected: a clean validation error.

Fix: in the array branch, when `args[flag]` is not actually an array
(because `--field-json` replaced it), throw a `FlagParseException`
explaining the `--field` and `--field-json` forms cannot be mixed for
the same field, never a raw `TypeError`.

## Tests

Adds `packages/argent-cli/test/flag-parser.test.ts`. The new tests fail
before the fix (4 failures) and pass after; the full `argent-cli` suite
stays green (90 passed). Behavior for valid inputs is unchanged.
 2026-07-03 16:19:22 +02:00
Ignacy Łątka a92c859d07 feat(argent-lens): argent lens — bind a claude session to the preview window (#416) 
## What

Adds `argent lens` — open Argent Lens bound to a fresh `claude` session.

## Why

The Lens preview window and the agent communicate through a shared,
process-wide tool-server that carries **no per-agent identity** (only a
coarse telemetry header). So when the user requests changes in the
window, there's no reliable way to route them to the *specific* agent
that opened it.

`argent lens` sidesteps the problem by inverting ownership: the human
launches Lens, and Lens spawns (and therefore owns) exactly one `claude`
terminal. The binding is 1:1 by construction — no identity plumbing
required.

## How it works

1. `argent lens` ensures a tool-server and **begins a CLI Lens
session**, which opens the preview window up front (no
`await_user_selection` needed) and keeps it open across rounds.
2. It spawns a tracked `claude` terminal in the current directory — a
TypeScript port of the macOS applet's `osascript` spawn/track mechanism
— seeded to propose variants without blocking.
3. A watcher polls `GET /preview/outcome`; on each submitted round it
types a one-line feedback summary into that terminal, queuing it to the
agent as its next prompt. Requesting changes therefore "talks back" to
the bound agent instead of closing the window.

macOS only (drives Terminal/iTerm via `osascript`); off-darwin it exits
with a clear message.

## Changes

- **CLI** `lens-terminal.ts` — `osascript` spawn/track + a new
write-into-running-session path (the "outputting" channel; injects input
into the running TUI, which a tty write cannot do).
- **CLI** `lens.ts` — the command + feedback formatter.
- **tool-server** — a `cliSession` flag in the variant store (opens the
window, suppresses auto-close on submit); `POST /preview/cli-session`
and `GET /preview/outcome` routes.
- **UI** — relabels the submit action to "Request changes" while a CLI
session is active.

Behind the existing `argent-lens` feature flag.

## Verification

- Build + typecheck across the workspace; ESLint (`--max-warnings 0`) +
Prettier clean.
- 34 new unit tests; full `@argent/cli` suite **118 passing**,
tool-server variant/preview suites **46 passing**.
- **E2E (observed on this machine):**
- **tool-server HTTP** — `cli-session` toggles `cliSession` true/false;
submitted feedback round-trips through `/preview/outcome`.
- **terminal injection** — spawned a window running `cat`, confirmed
`writeToSession` lands text in the running program's **stdin** (both
iTerm and Terminal), with embedded newlines flattened.
- **full relay** — the real `watchAndRelay` loop against a real
tool-server typed two distinct submission rounds' feedback into a
stand-in agent terminal (dedup on `completedAt` verified).
- `argent lens --help` and the feature-flag gate (exit 1 with guidance,
no spawn).

Note: a real end-to-end with a live `claude` agent was not run (it would
drive a paid interactive session); the agent-facing channel is proven by
injecting into a stand-in process's stdin via the exact same code path.

---------

Co-authored-by: Filip131311 <contact@filipkaminski.com>
 2026-07-01 19:57:56 +02:00
Hubert Gancarczyk 6bc5376746 fix(tool-server): stop list-devices from hanging - wedged-VVD stall and CLI socket leak (#419) 
## Summary

`list-devices` (an `alwaysLoad` tool called at session start and
frequently after)
could appear to **hang**. Investigation found **two independent
causes**, both fixed here:

1. **Server-side (Vega):** a wedged Vega VVD turned `list-devices` into
a ~40s stall
   and leaked `vega` CLI subprocesses.
2. **Client-side (CLI):** every `argent run …` lingered a constant **~6s
after it had
already printed its result**, because a health-check `fetch` never
drained its
response body and kept an undici keep-alive socket ref'd until the
server closed it.

Net effect for `argent run list-devices`: **6.15s → 0.91s**, and the
worst-case
wedged-VVD stall drops from ~40s to a bounded short call.

---

## Problem 1 — wedged Vega VVD stalls `list-devices` (~40s) and leaks
processes

On a host with the Vega SDK installed and a **running-but-unresponsive
VVD**,
`list-devices`' Vega discovery path made **two serial 20s CLI calls**
that both hung:

- `vega device list` → SIGKILL at 20s
- then (recovery) `vega device info` → SIGKILL at another 20s → **~40s
total**

Three compounding defects:

- `execFileAsync` resolves on stdio **close**, not child **exit**. The
`vega` launcher
forks a `python3 → node → vda` worker tree; an orphaned worker holds the
stdout pipe
open, so the call stayed pending for the full timeout even after the
launcher was
  killed.
- The `device info` recovery fired even when `device list` had **hung**,
doubling the
  cost and spawning a second hanging subprocess.
- On timeout we killed only the **direct child**, so the `python3 → node
→ vda` worker
tree was **orphaned, not reaped** — leaking a process tree per call (a
wedged host had
  accumulated ~269 of them).

### Fix

- **`runVega` (`vega-cli.ts`):** switched from `execFileAsync` to
`spawn` with
**`detached: true`**, making the launcher its own **process-group
leader**, and on
timeout SIGKILL the **whole process group** (`process.kill(-pid)`) —
which reaps the
entire `python3 → node → vda` tree, **not just the launcher**. We also
destroy our
stdio stream ends so a pipe-holding worker can't keep the call pending.
(`execFile`
silently drops `detached`, which is why the switch to `spawn` was
required.)
- **Success path doesn't depend on stdio EOF either:** we prefer `close`
(fully
drained), but if a grandchild holds the stdout pipe open after a **clean
exit**, we
fall back to `exit` + a short (~1s) drain grace and resolve with the
captured
output — so a finished call can't be stalled into the timeout and then
rejected,
discarding valid data. The output cap is measured in real UTF-8 bytes.
For that
guarantee to hold at the boundary, the `exit` handler **disarms the main
timeout
timer**: once the child has terminated the wall-clock deadline is moot,
so a clean
exit landing within the drain grace of the deadline resolves with its
output instead
of being mis-rejected as a `timeout` (which would also wrongly suppress
the
    `listVegaDevices` recovery for a still-running VVD).
- **Belt-and-suspenders for a worker that escapes the group:** an
earlier
investigation reported the worker tree `setsid()`-ing into its own
session (which
would defeat a group-only SIGKILL). To be safe regardless of which is
true, on
timeout we **snapshot the launcher's descendants from the process table
*before*
killing** (a setsid'd worker keeps its `ppid` pointing at the launcher
until the
launcher dies, so a `ppid` tree-walk still reaches it) and SIGKILL those
pids
individually after the group kill. The snapshot is **precise** — only
this
launcher's own descendants, by pid, so a concurrent `vega` call's
workers are never
touched — and bounded, and the group kill runs regardless of whether it
succeeds.
- A real-subprocess test confirms **0 orphans** after a timeout,
including a worker
deliberately spawned into its **own** process group (so the group kill
alone cannot
    reach it — the test fails without the descendant sweep).
- **`listVegaDevices` (`vega-devices.ts`):** skip the `device info`
recovery only when
  `device list` **timed out** (a wedged agent — keyed off the failure's
`error_kind: "timeout"`), so a hung list no longer pays for a second
timeout wait. A
*fast* (non-timeout) list failure still recovers a genuinely-running VVD
rather than
mis-reporting it as stopped. Tightened discovery timeouts (20s → 6s/4s —
a generous
multiple of the ~1s healthy time so a cold-start `vega` launcher isn't
spuriously
timed out, with their sum (10s) kept under the 15s branch deadline with
margin).
- **`list-devices.ts`:** added a 15s `withDeadline` backstop around the
Android and Vega
branches so no single branch can stall the whole fan-out (returns
partial results +
logs a note). Sized to sit ~5s above the Vega branch's ~10s worst case,
so the
backstop never truncates a slow-but-completing branch (which would drop
a real
  device); an invariant test guards that ordering.
- **`readAvdName` (`adb.ts`):** run the two `getprop` probes
concurrently so a wedged
  emulator costs 5s, not 10s.

---

## Problem 2 — `argent run` hangs ~6s after printing output (CLI socket
leak)

Independent of Vega. The tool-server returns `list-devices` in <1s, but
the CLI process
took a **constant ~6s** to exit. Any path that calls `process.exit()`
was instant; any
path that returned naturally (`argent run …`, `argent run … --help`,
`argent tools`)
lingered ~6s.

**Root cause:** `isToolsServerHealthy` (`launcher.ts`) did
`const res = await fetch('/tools'); return res.ok;` and **never drained
the body**.
`/tools` is a large (~112KB) payload, so undici keeps the keep-alive
socket **ref'd**
until the tool-server's idle `keepAliveTimeout` (~5s) closes it.
`ensureToolsServer`
runs this health check on every invocation, and `run()`'s success path
has no
`process.exit(0)`, so the ref'd socket holds the event loop open.
(Confirmed
telemetry-independent — `DO_NOT_TRACK` made no difference.) The same
undrained-body
pattern existed in `link.ts` (`argent link`).

### Fix

Drain the response body before returning in both health-check-style
fetches:

```ts
await res.body?.cancel().catch(() => {});
return res.ok;
```

(`fetchTools`/`callTool` were already fine — they `await res.json()`.)

---

## Testing

**Empirical, end-to-end** (on a machine reproducing the wedged VVD):

| Command | Before | After |
| --- | --- | --- |
| `argent run list-devices` | 6.15s | **0.91s** |
| `argent run list-devices --help` | 6.14s | **0.16s** |
| `argent tools` | 6.15s | **0.16s** |
| handle probe after run | 2 sockets ref'd for ~6s | **none — immediate
exit** |

- Raw `POST /tools/list-devices` was ~0.7s throughout — confirming the
~5.4s was pure
  CLI teardown, not tool work.
- **Process-group reap proven empirically:** the live `vega → bash →
python3 → node`
tree all shares the detached launcher's PGID; a group-SIGKILL left **0
survivors**.
A real-subprocess test (unmocked `runVega` vs. a fake hanging `vega` on
`PATH`) confirms
  it settles at its deadline **and** leaves **0 orphaned processes**.

**Automated:**

- `vega-cli-timeout.test.ts` (real-subprocess test) — `runVega` rejects
on its deadline,
**reaps the entire worker tree (0 orphans)** — the fake's hung worker is
spawned into its
**own** process group, so the test fails without the descendant sweep —
and returns
normally when the CLI responds. Also covers the boundary case: a **clean
exit whose
drain grace outlasts the deadline** resolves with its captured output
rather than being
rejected as a `timeout` (guards the `exit`-handler timer disarm above).
- `vega-adb-connect-recovery.test.ts` — a *timed-out* `device list` does
not trigger a
second `device info` call (no stacking); a *fast, non-timeout* failure
**still** recovers
  a running VVD.
- `list-devices-deadline.test.ts` — `withDeadline` returns the branch
value on the happy
path and the fallback (with a logged note) when a branch exceeds the
deadline.
- `list-devices.test.ts` — added a `spawn` mock (vega moved off
`execFile`).
- `launcher-helpers.test.ts` — `isToolsServerHealthy` cancels (drains)
its response body.
- Suites green: **tool-server (1610), argent-tools-client (109),
argent-cli (84)**.
- eslint (`--max-warnings 0`), prettier, and `tsc` clean.

---
 2026-07-01 11:02:33 +02:00
filip131311 5f9dd48e2a fix(tool-server): prevent duplicate tool-servers after an nvm node switch (#385) 
## Problem

Switching Node versions with **nvm** can leave **two (or more)
`tool-server` processes alive at once** while
`~/.argent/tool-server.json` tracks only one. The extra servers are
orphaned (untracked, on leaked ports) until their 30‑minute idle
timeout.

This was reproduced live: a machine had **three** `node
…/dist/tool-server.cjs start` processes running, all spawned within the
same minute, with state tracking only the last one.

## Root cause

`@argent/tools-client` `ensureToolsServer()` ran a non-atomic
`read state → check alive+healthy → clearState → spawn → writeState`
sequence with:

1. **No cross-process serialization.** An nvm switch relaunches `argent
mcp` under the new Node while the *previous* MCP's 30s health monitor
reconnects — several launchers run `ensureToolsServer()` at nearly the
same time, all observe "no healthy server", and each spawns its own
**detached** server on a fresh free port. Last‑writer‑wins on the state
file orphans the rest.
2. **No kill-before-respawn.** When the tracked server was alive but
unhealthy, the old code just `clearState()`d and spawned a new one,
leaving the old detached process running untracked.

## Fix (`packages/argent-tools-client/src/launcher.ts`)

- **O_EXCL spawn lock** (`~/.argent/tool-server.lock`) serialises the
spawn decision across processes: per-acquisition nonce, stale‑steal
(dead holder / aged lock), bounded wait, and a hot‑spin‑proof reclaim
loop. Inside the lock, `ensureToolsServer` **double‑checks** for a
peer‑spawned healthy server before spawning, so a burst of concurrent
launches collapses to a single server.
- **Kill-before-respawn** terminates a wedged server we own before
spawning its replacement, with three guards so it can never signal the
wrong process:
- `managed === "autospawn"` — a user's `argent server start` server is
now tagged `managed: "cli"` and is **never** killed by the auto‑spawn
path;
- a **structural** `node <bundlePath> start` command-line identity match
(not a bare substring);
- a re-check of that identity immediately before each signal (covers PID
reuse across the SIGTERM grace window).
- **`spawnToolsServer` reaps its detached child on every reject path**,
so a readiness timeout can no longer leak a child that binds its port
moments later as an untracked second server.

The lock-free fast path (reuse a healthy tracked server) is unchanged,
so steady‑state behaviour and latency are unaffected; the lock is only
taken when a spawn is actually needed.

## Tests

New
`packages/argent-tools-client/test/launcher-duplicate-spawn.test.ts`:

- a 5× concurrent launch burst collapses to **one** server;
- an alive‑but‑unhealthy `autospawn` server is terminated (not orphaned)
on respawn;
- a recycled pid whose command line doesn't match is **never**
signalled;
- a `managed: "cli"` server is left alive instead of being killed;
- a readiness timeout reaps the spawned child (no orphan).

Each case was verified to **fail against the pre-fix code** and pass
with the fix. Existing `launcher-spawn` / `launcher-state` suites are
unchanged and green.

> Note: the `@swmansion/argent` `dispatcher › forwards SIGTERM to the
child binary` unit test is a pre-existing flake (alternates pass/fail
independent of this change, which doesn't touch that package).

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

Co-authored-by: Radoniusz Cloudiusz <radoniusz@radoniusz.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-06-29 17:20:37 +02:00
Hubert Gancarczyk 8483fe23f9 feat: add telemetry (#274) 
## Summary

Adds anonymous, opt-out telemetry for Argent installer, update/uninstall
flows, CLI telemetry controls, tool-server lifecycle, and tool
invocation health signals.
## What Changed
- Added a new private `@argent/telemetry` package
- Introduced typed telemetry events with runtime property
allowlisting/sanitization.
- Added lazy anonymous identity creation in `~/.argent/telemetry-id`.
- Added consent handling via: `argent telemetry enable`, `argent
telemetry disable`, `argent telemetry status`
- Added local debug logging with `ARGENT_TELEMETRY_DEBUG=1`.
- Tracks installer funnel events for init, global install/update
decisions, editor selection, allowlist decisions, skill installation,
rules/agents copy, update, and uninstall.
- Tracks tool-server start/stop and tool invocation/complete/fail
events.
- Adds per-tool invocation IDs so request metadata can be correlated
without relying on ordering.
- Records only coarse platform metadata (`ios` / `android`) for device
tools
- Ensures short-lived commands drain telemetry with bounded shutdown
before exit.
- Wires telemetry package into bundling and injects full Argent CLI
version metadata at build time.
- Added central `FAILURE_CODES` and `FailureSignal` helpers in
`@argent/registry`.
- Attached structured, sanitized failure metadata across registry
errors, tool-server tools/services, HTTP validation/dependency gates,
installer flows, and `argent run`.
- Added telemetry allowlisting for safe failure fields only: static
error codes/stages, coarse area/kind/command, bounded exit codes,
allowlisted signals/spawn codes, and coarse network failure classes.
- Split CLI-local `argent run` failures into `cli:run_fail` so they are
not double-counted as tool failures.
- Added installer telemetry for failed package actions, update/uninstall
failures, rules/agents copy results, and skill refresh results.
- Made telemetry consent writes atomic and kept telemetry status
side-effect free via `peekAnonId()`.
- Adds a coarse, anonymous signal to tool telemetry: which AI coding
tool is driving the MCP server (Claude Code, Codex, Cursor, Gemini, VS
Code, Windsurf, Zed, opencode, Copilot, or other).

## Tests Added / Updated

- Telemetry unit coverage for consent precedence, identity creation,
base props, sanitization, PostHog config, debug logging, shutdown,
opt-out, and forget behavior.
- Registry/tool-server tests for invocation IDs, platform metadata
boundaries, startup failure flushing, and tool lifecycle telemetry.
- Installer tests for final telemetry drain, update action
classification, and uninstall identity cleanup ordering.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-06-19 18:22:34 +02:00
Ignacy Łątka acfe80ab6b chore: add ESLint with typescript-eslint (#349) 
## What

Adds type-aware linting to the workspace. Until now the only static gate
on JS/TS was Prettier (formatting) and `tsc` — there was no linter, so
the whole class of bugs `tsc` doesn't catch (floating promises, misused
promises, throwing non-Errors, unsafe stringification, dead code) went
unguarded across ~450 TS files.

- **`eslint.config.mjs`** — flat config. `recommendedTypeChecked` runs
on the TS sources via an explicit `project` list (each package keeps
tests in a separate `tsconfig.test.json` that `projectService` can't
auto-discover, so it's listed explicitly). Type-checked rules are
disabled for the loosely-typed plain-JS build/dev scripts. The
`argent-private` submodule and build artifacts are ignored (mirrors
`.prettierignore`).
- **`lint` / `lint:fix`** npm scripts.
- **`.github/workflows/lint.yml`** — builds the workspace (type-aware
rules need the referenced projects' declarations) then runs `eslint .`.
Node 24 (ESLint 10 requires `^20.19 || ^22.13 || >=24`).

## Rule calibration

This is an adoption PR, so it's deliberately calibrated rather than
maximal:

- **Kept on (bug-catchers):** `no-floating-promises`,
`no-misused-promises` (with `checksVoidReturn: false` to drop the noisy
callback variant), `only-throw-error`, `no-base-to-string`,
`prefer-promise-reject-errors`, `no-unused-vars` (`_`-prefix escape
hatch), plus the ESLint recommended set.
- **Parked off as documented debt:** the high-volume style/`any`-family
rules — `no-unsafe-*`, `require-await`, `no-unnecessary-type-assertion`,
`restrict-template-expressions`, `unbound-method`, `no-explicit-any`,
etc. These flag broad classes of pre-existing code and are out of scope
here; each is commented as a ratchet target for follow-up passes.

All findings from the kept-on rules are fixed in this PR (dead
code/imports, error-cause chaining, **3 genuine floating promises**,
non-Error throws, unsafe stringification). Stale unused `eslint-disable`
directives left over in the tree were removed by `--fix`.

## Verification

Locally, all green: `eslint .` (0 errors), `tsc --build`,
`typecheck:tests`/`typecheck:scripts`, the full `vitest` suite (no
behavior change from the fixes), and `prettier --check`.

## Not included (intentionally)

Other static-analysis gates discussed (CodeQL, dependency-audit,
ShellCheck, actionlint) are out of scope for this PR.
 2026-06-17 12:20:55 +02:00
Paweł Fornagiel 92cc69a75c feat: extract configuration-core and migrate auto-screenshot to feature flag (#295) 
## Summary

- Introduces `@argent/configuration-core` as the shared source of truth
for feature-flag storage, the registry, and `isFlagEnabled`, so MCP and
CLI can import the same primitives without coupling runtime code to
CLI-only modules.
- Replaces `ARGENT_AUTO_SCREENSHOT` with the off-by-default
`disable-auto-screenshot` flag (auto-screenshot stays on unless enabled
via `argent enable disable-auto-screenshot`); renames
`ARGENT_AUTO_SHUTDOWN` to `ARGENT_TOOL_SERVER_SHUTDOWN_ON_MCP_EXIT`.
- Keeps `@argent/cli` as the command layer (`enable` / `disable` /
`flags`) with backward-compat re-exports, colored flag state output
(picocolors), and `--help` listings of available flags.

---------

Co-authored-by: Filip131311 <contact@filipkaminski.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
 2026-06-10 15:01:25 +02:00
filip131311 20cb452fd4 feat(remote): artifact transport for files produced by a remote tool-server (#278) 
## Why

Files produced on the **tool-server host** (screenshots, profiler
exports) were surfaced to the agent as host paths / `127.0.0.1` media
URLs, which the MCP client (`argent-mcp`) then fetched a *second* time
to get the bytes. When the tool-server runs **remotely**, that localhost
URL resolves to the client's own machine — nothing there — so
screenshots fail with `(Screenshot unavailable: no valid PNG …)`.
Profiler exports have the same shape: paths that only exist on the
remote host.

## What

A reusable **artifact boundary layer** so any file-producing tool works
regardless of where the tool-server runs:

**Foundation**
- **tool-server** (`src/artifacts.ts`, `http.ts`): in-process artifact
registry + `GET /artifacts/:id` streaming route.
`registerArtifact(hostPath)` returns a wire handle (`{ __argentArtifact,
id, filename, mimeType, size }`) instead of a raw path.
- **argent-mcp** (`src/artifacts.ts`, `content.ts`, `mcp-server.ts`): a
client materializer deep-walks tool results for handles, downloads each
over the remote-aware `TOOLS_URL`, writes it into a **structured
cache**, and rewrites the result to the **local** path (+ inline image
rendering).

**Cache layout** (override root with `ARGENT_ARTIFACTS_DIR`):
```
~/.argent/artifacts/<project>/<session>/<device>/<filename>
```
- `project` = `basename(cwd)` + short hash of the full path (readable +
collision-safe)
- `session` = minted once per argent-mcp process (groups a run, easy GC)
- `device` = udid/serial when device-scoped, omitted otherwise

**Consumers wired**
- **screenshot** → returns an image artifact; rendered inline from
downloaded bytes with **no second `127.0.0.1` fetch**. Fixes
`auto-screenshot` and `flow-execute` screenshots via the same path.
- **native-profiler-stop** → `exportedFiles` (XML) become downloadable
artifacts materialized client-side.

## Deferred (follow-ups)
- **`.trace` bundle** — it's an Instruments bundle (a directory);
streaming needs archiving (no archiver dep yet). `native-profiler-stop`
now returns a `traceFileNote` explaining it stays on the profiling host.
- **Project source on remote** — `react-profiler-component-source` /
`react-profiler-analyze` read the user's source tree on the tool-server
host. The profiler analysis chains already work remotely
(server-internal); tracked separately.

## Compatibility
Legacy `{ url, path }` screenshot rendering is retained as a fallback
when no artifact context is present (older tool-servers / unit tests).

## Tests
- tool-server: artifact registry (infer/override), `GET /artifacts/:id`
(200/404/410), updated `stop-recovery` assertions.
- argent-mcp: materializer
(download/rewrite/nested/failure/device-scoping), artifact-aware
`toMcpContent`.
- argent-mcp typecheck + full suite green; tool-server artifact/profiler
tests green. (Note: a full local tool-server typecheck trips a
pre-existing zod-v4-vs-v3 mismatch in `run-sequence` that's unrelated to
this change and resolves on this branch's own lockfile.)

## Notes
Routes around the `argent-private` native simulator-server binary (which
emits the `127.0.0.1` media URL) rather than changing it — no submodule
change.

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

---------

Co-authored-by: pFornagiel <pawel.fornagiel2@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Paweł Fornagiel <85541429+pFornagiel@users.noreply.github.com>
 2026-06-10 12:04:30 +02:00
filip131311 8efd7c6346 feat: add argent server start for long-lived tool-server (#182) 
## Human summary: 

This adds `argent server start` command to enable us to spawn tools
server in arbitrary location. we want this to enable using remote
tools-server via our mcp not only local ones.

## Summary

- Adds `argent server start` to spawn a tool-server that does **not**
auto-shutdown on idle, with configurable bind port and host — the
missing piece for running tool-server on a remote/dedicated host.
- Foreground by default so process supervisors (systemd, Docker,
supervisord) own the lifecycle; `--detach` keeps the existing
daemon-spawn behavior. Refuses to clobber a healthy running server
unless `--force`.
- Phase 2 (consumer-side `--tools-url` flag pointing the MCP / CLI at a
remote tool-server) is now a one-flag change away — `tools-client.ts`
and `mcp-server.ts` already honor `ARGENT_TOOLS_URL`. Out of scope for
this PR.

## What changed

- **`tool-server`** — read `HOST` env var (default `127.0.0.1` so the
existing auto-spawn path is unaffected), bind to it, attach an `error`
listener on `app.listen` so `EADDRINUSE` / `EACCES` exit cleanly instead
of routing through `uncaughtException`.
- **`argent-tools-client`** — broaden the readiness regex to match any
host; extend `buildToolsServerEnv` with optional `{ host,
idleTimeoutMinutes }`; record `host` in `~/.argent/tool-server.json`
(backward-compat: missing field defaults to `127.0.0.1`); export
`spawnToolsServer`, `findFreePort`, `isToolsServerHealthy`,
`isToolsServerProcessAlive`, the state read/write/clear helpers, and
`formatToolsServerUrl` so the CLI can reuse them.
- **`argent-cli`** — new `server start [flags]`: `--port`/`-p` (default
3001, `0` = pick free), `--host` (default `127.0.0.1`), `--idle-timeout`
(default `0` = never), `--detach`/`-d`, `--force`, `--help`. Warns when
host is non-loopback. Foreground mode forwards `SIGINT`/`SIGTERM` to the
child and writes the state file so local `argent run` / `argent mcp` can
attach.
- **`argent` dispatcher** — pass `BUNDLED_RUNTIME_PATHS` to `server()`
so the new subcommand can find the bundled tool-server.

## Test plan

- [x] `argent server --help` lists the new `start` subcommand
- [x] `argent server start --help` shows full flag reference
- [x] `argent server start --detach --port 0` → status alive/healthy,
`/tools` reachable, state file populated with `host` field
- [x] Second `argent server start --detach` refuses (`exit 1`) with
helpful message
- [x] `argent server start --detach --force` kills the old PID and
starts a new one
- [x] Foreground `argent server start --port 0` runs with inherited
stdio; child PID is the actual tool-server (not the wrapper); `argent
server stop` from a sibling shell tears down both
- [x] Existing auto-spawn path (`argent tools`) still works — verifies
the regex broadening + default-host change is backward-compatible
- [x] `tool-server` (444 tests) + `argent-tools-client` (1) + `argent`
dispatcher (1) suites pass
- [ ] Manual smoke on `--host 0.0.0.0` — could not exercise locally
(sandbox declined the public-interface bind); the warning + bind path is
identical to loopback besides the listen address

---------

Co-authored-by: pFornagiel <pawel.fornagiel2@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Paweł Fornagiel <85541429+pFornagiel@users.noreply.github.com>
 2026-06-03 17:44:15 +02:00
Ignacy Łątka 74c2065ca0 fix: multiple minor bugs (#194) 
## Human Summary

This PR aims to fix bugs that are benign for now, could become serious
in the future:

- multiple CSRF via CORS misconfiguration on `tool-server`
- multiple SSRF that were possible due to the CSRF
- path traversal via flow creator
- multiple shell injections
- lack of auth on `MCP/CLI <-> tool-server` path (bug details discussed
in person). After the other fixes this is somewhat redundant, but
doesn't hurt and protects against theoretical access attempts from
within other processes running on the user's device.

## AI Summary

<details>
Bundles six independent fixes that together close a CSRF cluster on the
local tool-server. The vulnerabilities chain because the HTTP API has no
auth and serves wildcard CORS, so any web origin (Firefox
unconditionally; Chrome via PNA-disabled or loopback-served) can drive
the tool-server from a cross-origin tab. Each individual fix lives on
its own branch (linked below) and stands alone — this PR exists so the
cluster can be reviewed and shipped as one piece.

### Fixes

1. **`fix(http): require auth token + drop wildcard CORS`** —
per-process 32-byte hex Bearer token, generated by the launcher,
persisted in `~/.argent/tool-server.json` with mode `0600`, required on
every route except `/preview`. Constant-time compare. Wildcard CORS
dropped. Threaded through MCP server + CLI + tools-client.
Backwards-compat: when `ARGENT_AUTH_TOKEN` is unset (e.g. `npm run
dev`), the server logs a one-shot warning and skips the gate.
2. **`fix(http): reject non-loopback Host headers`** — middleware
(before the auth gate) returns 403 for any request whose Host is not
`127.0.0.1`, `localhost`, or `::1`. Closes DNS-rebinding bypass.
3. **`fix(ios-profiler-start): prevent shell injection via device_id`**
— `detectRunningApp(udid)` no longer interpolates `params.device_id`
into `execSync` template strings. Both `xcrun simctl spawn ${udid} …`
and the piped `xcrun simctl listapps ${udid} | plutil …` are rewritten
to `execFileSync` array form, with the pipe to `plutil` happening in JS
(input option) rather than the shell.
4. **`fix(flows): validate name to close path-traversal in
getFlowPath`** — `getFlowPath()` requires `^[A-Za-z0-9_\\-]+$` and
verifies `path.relative(getFlowsDir(), filePath)` does not start with
`..`. Centralizing the check in `getFlowPath` covers every caller
(start-recording, add-step, insert-echo, finish-recording, run,
read-prerequisite) without per-tool churn.
5. **`fix(source-resolver): reject paths outside projectRoot +
non-source extensions`** — `readSourceFragment()` previously accepted
any `file` field on the SourceLocation it was given (the field traces
back to a React fiber's `_debugSource.fileName`, which is
attacker-controllable). Now: containment check via
`path.relative(projectRoot, abs)` plus an extension allowlist
(`.js/.jsx/.ts/.tsx/.mjs/.cjs/.json`).
6. **`fix(source-maps): reject non-loopback URLs (close SSRF)`** —
`doRegister()` validates `sourceMapURL` before fetching: only
`http://`/`https://` URLs whose hostname is loopback. Closes a
blind-SSRF probe of the host network (cloud metadata at
`169.254.169.254`, intranet pages, etc.).

### Individual branches

- `fix/auth-token-drop-cors`
- `fix/host-header-validation`
- `fix/profiler-command-injection`
- `fix/flow-name-validation`
- `fix/source-resolver-traversal`
- `fix/source-map-ssrf`

If you want to land them as separate PRs, the integration branch can be
dropped after each lands.

## Test plan

Unit tests (already in CI):

- [x] `npm run build` passes (workspace tsc clean)
- [x] `npm run test -w @argent/tool-server` — **510 passed (was 473 on
`main`)**, +37 new tests across:
- `test/http-auth.test.ts` — 10 tests (auth, CORS removal, dev-mode
escape hatch)
- `test/http-host-validation.test.ts` — 7 tests (loopback allow,
DNS-rebinding reject)
- `test/flows/flow-utils.test.ts` — +5 tests (path-traversal name
rejection)
- `test/metro/source-resolver.test.ts` — +5 tests (containment +
extension allowlist)
- `test/metro/source-maps-ssrf.test.ts` — 10 tests (loopback-only allow)

Live end-to-end (run locally against this branch's bundled artifacts):

- [x] Launcher spawns tool-server; `~/.argent/tool-server.json` contains
a 64-char hex token and is mode `0600`
- [x] `GET /tools` without `Authorization` → **401**
- [x] `GET /tools` with wrong token → **401**
- [x] `GET /tools` with correct token → **200** (61 tools)
- [x] Request with `Host: attacker.example` → **403**
- [x] No `Access-Control-Allow-Origin` header on responses
- [x] Cross-origin command-injection probe via `device_id=\"x;touch
/tmp/marker;#\"` does NOT create the marker file (Vuln 4 closed)
- [x] `flow-start-recording` with `name=\"../../../tmp/escape\"`
rejected with `Invalid flow name`
- [x] `flow-start-recording` with a valid name still works
- [x] `tools-client.fetchTools()` and
`tools-client.callTool('list-simulators')` succeed via the auth-token
threading (the path the MCP server uses internally)
- [x] MCP-stdio entry point (`startMcpServer`) end-to-end: `initialize`
→ `tools/list` (61 tools) → `tools/call('list-simulators')` returns
content. Confirms the launcher → token → fetch → tool-server flow is
intact for the production `argent mcp` path.

## Notes for reviewers

- `/preview` is intentionally exempt from the auth gate (it was not
gated before; tightening it deserves a separate PR).
- The `ARGENT_AUTH_TOKEN`-unset escape hatch keeps `npm run dev`
workable; production `argent mcp` always passes a token via env, so the
warning never fires there.
- The auth bundle changes the public return shape of `ensureToolsServer`
from `Promise<string>` to `Promise<{ url, token }>`. The two in-tree
callers (`argent-mcp/mcp-server.ts`,
`argent-tools-client/tools-client.ts`) were updated; external consumers
using `ARGENT_TOOLS_URL` should also set `ARGENT_AUTH_TOKEN` to match
the server.

</details>
 2026-06-03 15:57:42 +02:00
Ignacy Łątka c5c3f7af11 feat: feature flags (#260) 
## Summary

- New top-level CLI commands `argent enable <flag>` / `argent disable
<flag>` / `argent flags`.
- Storage is a small JSON file at `~/.argent/flags.json` by default;
`--scope project` writes to `<project-root>/.argent/flags.json` instead.
- `enable` writes `true`. `disable` removes the entry at the chosen
scope (and deletes the file + empty `.argent/` when it becomes the last
flag).
- `isFlagEnabled` walks project → global, so a project entry shadows the
same key in the global file. To opt a single project out of a
globally-enabled flag, hand-edit `<project>/.argent/flags.json` to
`{"flags":{"name":false}}` — there is no CLI for an explicit override.
- The dispatcher routes the new commands to `@argent/cli` exports;
bundling needs no config changes (only `node:fs|path|os`).

## Behaviour

- `argent enable foo` → writes `{ "flags": { "foo": true } }` to global
file.
- `argent disable foo` → removes `foo` from the global file (clean round
trip with `enable`).
- `argent enable foo --scope project` → writes to
`<project-root>/.argent/flags.json`.
- `argent flags` → lists effective state with the winning scope;
`--json` for scripting.
- Project root is the nearest ancestor with `.argent`, `.git`, or
`package.json` — falls back to cwd when none found.
- POSIX `--` separator: `argent enable -- weird-name` is supported.
- Help (`--help` / `-h`), invalid `--scope`, invalid flag names, missing
arg, unknown flags all produce a clear stderr error and `exit 2`.
- Flag-name validation: `/^[a-zA-Z][a-zA-Z0-9._-]*$/`.
- Writes are atomic (tmp file + rename) so concurrent invocations cannot
tear the JSON.

## Reader API

`isFlagEnabled(name)` is exported from `@argent/cli` for future
consumers; it walks project → global and returns `false` when unset.

## Files

- `packages/argent-cli/src/flags.ts` (new) — storage + CLI handlers
- `packages/argent-cli/src/index.ts` — re-exports `enable` / `disable` /
`flags` / `isFlagEnabled` / `FlagScope`
- `packages/argent-cli/test/flags.test.ts` (new) — 41 tests
- `packages/argent-cli/vitest.config.ts` (new)
- `packages/argent-cli/package.json` — description bump
- `packages/argent/src/cli.ts` — dispatcher routes + help text
- `README.md`, `CONTRIBUTING.md` — command tables

## Test plan

- [x] `npm test --workspaces --if-present` → 1085 tests pass (41 in
`@argent/cli`).
- [x] `npm run typecheck:tests --workspaces --if-present` → clean.
- [x] `npm run build` (root `tsc --build`) → clean.
- [x] `npm run build -w @swmansion/argent` (dispatcher + esbuild
bundles) → clean.
- [x] `npx prettier --check .` → clean.
- [x] End-to-end against the built dispatcher in a tmpdir:
- `argent enable my-feature-flag` (global) writes to
`~/.argent/flags.json`.
- `argent enable proj-only --scope project` writes to
`<proj>/.argent/flags.json`.
- `argent disable my-feature-flag` removes the entry; round trip leaves
a clean tree (file + empty `.argent/` removed when no other state lives
there).
  - `argent disable foo` preserves sibling flags in the same file.
  - `argent flags` lists each flag with the winning scope.
- `argent enable --` errors cleanly; `argent enable -- name-after`
accepts the literal.
- `argent enable foo --scope project` walks up to `.git` /
`package.json` / `.argent`; stray editor configs (`.claude`, `.cursor`,
…) do NOT count as project roots.
- Error paths (`--scope bogus`, no name, `1bad-name`) exit `2` with a
clear message.
 2026-06-01 23:22:10 +02:00
Ignacy Łątka d017ba340c feat: Android emulator support (#148) 
### For reviewers - what to focus on:

The diff is ~6k LoC; this breakdown should help triage where to spend
attention:

| Bucket | Lines | Effort |
| --- | --- | --- |
| **Net-new Android logic**: `boot-device.ts` (660) + `adb.ts` (399) +
`uiautomator-parser.ts` (292) | 1351 | **Read carefully** |
| Blueprint `DeviceInfo` refactor: `simulator-server`, `ax-service`,
`native-devtools` | 365 | Read |
| Tool `index.ts` / `types.ts` sprawl across ~20 tools (capability
gates, `services()`, descriptions) | ~965 | Skim — watch for cross-file
consistency |
| `platforms/android.ts` impls (stub → real implementation) | ~426 |
Skim |
| Skill/docs prose, profiler rename ripple | ~1000 | Skim |
| **`platforms/ios.ts` deletions** — 12 files, 0 additions; logic moved
into unified handlers | 684 | **Skip — mechanical** |

## What works, what doesn't:

### Just works thanks to either sim server or metro:
- `gesture-*`
- `button`, `keyboard`
- `screenshot`
- `run-sequence`, `flow-*`
- `debugger-*`
- `react-profiler-*`

### Adjusted to work on android too:

| tool | android | ios |
| --- | --- | --- |
| `describe` | `uiautomator dump` → `DescribeNode` | `AXServiceApi` |
| `launch-app` | `cmd package resolve-activity` + `am start -W` |
`simctl launch` |
| `restart-app` | `cmd package resolve-activity` + `am force-stop` + `am
start -W` | `simctl terminate` + `simctl launch` |
| `reinstall-app` | `adb install -r` (+`-g`/`-d`) | `simctl uninstall` +
`simctl install` |
| `open-url` | `am start -a VIEW -d <url>` | `simctl openurl` |
| `list-devices` | `adb devices` + `emulator -list-avds` | `xcrun simctl
list --json` |
| `boot-device` | snapshot probe → `-force-snapshot-load` hot boot with
screencap-alive guard, falls back to cold boot on any failure | `simctl
boot` + `bootstatus -b` |

### Will not work on android for now:
- `native-devtools-*` (7 total)
- `native-profiler-*` (3 total — was `ios-profiler-*`)
- `profiler-stack-query`, `profiler-combined-report` (depend on
`native-profiler-*`)
- `paste` — unregistered on both platforms for now.

---------

Co-authored-by: Paweł Fornagiel <pawel.fornagiel2@gmail.com>
 2026-05-11 11:39:10 +02:00
filip131311 6355620c4d refactor(mcp): split mcp package into argent, argent-cli, argent-installer, argent-mcp, argent-tools-client (#162) 
## Summary

- Splits the monolithic `packages/mcp` package into five focused
packages:
- `argent` — the user-facing CLI entry (`argent` binary,
init/uninstall/update flows, bundled-paths, scripts).
- `argent-cli` — CLI runner internals (flag-parser, run, server, tools).
- `argent-installer` — `init`/`update`/`uninstall` flows, MCP config
adapters, skills installer.
- `argent-mcp` — MCP server itself (mcp-server, tool-mapping,
auto-screenshot, content adapters).
  - `argent-tools-client` — HTTP client + launcher for the tool-server.
- Adds Gemini agent definitions, VSCode MCP config, and per-package
`tsconfig.json` / `vitest.config.ts`.
- Updates root `tsconfig.json`, `scripts/dev.cjs`, CI workflows, and
`package.json` to wire the new packages together.

## Test plan

- [ ] `npm install` resolves the new workspaces cleanly
- [ ] `npm run build` succeeds across all packages
- [ ] `npm test` passes (renamed test files keep their assertions)
- [ ] `npx argent init` still produces the expected MCP config in a
sample project
- [ ] `npx argent update` / `argent uninstall` exercise the installer
package
- [ ] CI workflows (`publish.yml`, `unit-tests.yml`) run on this branch
 2026-04-28 08:36:24 +02:00