16 Commits

Author	SHA1	Message	Date
Ignacy Łątka	16ad44bb0e	test(telemetry): pin the wire format and what a dead collector costs (#736) ... Two test files that look at what #570's exporter actually *does*, plus the configuration holes they exposed. The suite on #570 proves the exporter is **configured** right - the url, the headers, the timeouts, all read off the constructor. Nothing looked at the request that comes out of it, or at what happens when the collector does not answer. Both gaps are invisible from inside Argent by design: a failed export never reaches the caller, and an OTLP exporter counts any 2xx as delivered. | | | |---|---| | **`otel-wire.test.ts`** | Captures the real exporter's request off a loopback socket: uncompressed OTLP/JSON on `POST /v1/logs`, where each field the ingestion schema reads actually sits, the batch-size cap, and connection reuse. | | **`otel-unreachable.test.ts`** | Drives the same exporter at a silent socket, a refused port, a 503, a 401 and a 204, and asserts each drain finishes inside its deadline and what the collector really received. | | **`src/otel.ts`** | Two holes the tests exposed: `compression` and the six OTLP certificate variables are now settled in code instead of left to the environment. | **No docs update needed** - no user-facing capability, CLI surface, config key, flow file or MCP tool changed. <details> <summary><b>The two source fixes</b></summary> **`compression` was the one OTLP knob still left to the environment.** With `OTEL_EXPORTER_OTLP_COMPRESSION=gzip` (or its `_LOGS_` variant) set - and a machine already running OpenTelemetry has it set for its own collector - argent gzips its batches. Nothing downstream notices: a gzipped 2xx is a delivered batch, while the request this suite and the deploy-time smoke test are written against quietly stops being the request argent sends. `createExporter` now passes `compression` explicitly, which beats both variables. Unlike the header channel, where the SDK *merges* whatever the code does not set and the variables have to be cleared outright, one explicit value settles this. **The certificate variables could hang the command outright.** The SDK resolves `OTEL_EXPORTER_OTLP_{,LOGS_}{CERTIFICATE,CLIENT_CERTIFICATE,CLIENT_KEY}` with a synchronous `fs.readFileSync` while the exporter is constructed, and only then discards the https agent it built from them in favour of the explicit `httpAgentOptions`. The file is read for nothing - and a path that never answers hangs the command that emitted the event, uninterruptibly, on `getClient()`'s path. Measured with a fifo that has no writer: `createExporter` never returned (killed at 12s); with the variables cleared, 1.0 ms. They join the header pair in the list cleared across the constructor and restored on the way out. Some source prose was also wrong about the SDK and is corrected: it does **not** reject a null attribute - it serializes one as an empty OTLP value, `{"key":"cloud_agent","value":{}}` with `droppedAttributesCount: 0`, stored as and unrecoverable from a property that really was empty. That is the actual reason `toAttributes` drops the key. </details> <details> <summary><b>What the tests pin, and what they deliberately do not</b></summary> The wire test **mirrors** `OtelClient.emit` rather than calling it: `getClient()` resolves its endpoint from the hard-coded `OTLP_LOGS_ENDPOINT`, and that being unredirectable is the anti-exfiltration property the client is meant to have, so there is deliberately no seam to aim it at a test server. `otel-endpoint.test.ts` covers the other half against the real emit path - `SERVICE_NAME`, the logger name, event-name-as-body, the dropping of null/undefined properties - so a rename is caught there rather than in the mirror. What enforces the drain budgets is the exporter's `timeoutMillis`, *not* the processor's `exportTimeoutMillis`, which `forceFlush()` awaits straight past. Measured against a silent socket: `exportTimeout` 200 / `timeoutMillis` 5000 drains in 5012 ms, the reverse in 202 ms. 503 is the one collector state where the batch is sent twice, and the request count is the only thing that tells a retried failure from a rejected one - the SDK reports the class, not the code. The retry itself is only reachable while the next backoff fits in what is left of `timeoutMillis`, so the jitter is pinned to its low draw and the assertion turns on the round trip alone. `httpAgentOptions.timeout` is the one option with no behavioural coverage: it covers a socket stuck in *connect*, which needs an address whose packets are dropped and is not portably reproducible here. It stays pinned at the constructor and the docstring says so. Its `keepAlive` half **is** covered, on the two batches of a single drain. Real deadlines cost real time: `otel-unreachable.test.ts` waits ~5.1 s, taking the package suite from ~0.9 s to ~5.3 s. Inherent to driving real deadlines against real sockets. </details> <details> <summary><b>Verification</b></summary> - `npm test -w @argent/telemetry` - **336 passed**, 19 files - `typecheck:tests`, `eslint --max-warnings 0`, `prettier --check`, `knip`, `tsc --build` all clean; CI green - **Mutation-tested**, whole-suite counts rather than one test each: | mutation in `src/otel.ts` | tests red | |---|---| | `compression` pin dropped | 2 | | `compression` pinned to `"gzip"` | 5 | | any one of the 8 cleared env-var names dropped | 1 each | | env-clearing loop deleted | 2 | | the `finally` that restores it deleted | 1 | | `timeoutMillis` dropped | 5 | | `httpAgentOptions` dropped, or just its `timeout` | 1 | | `keepAlive` flipped to `false` | 2 | | `installDiagLogger`'s `!isDebugEnabled()` guard removed | 1 | | `maxExportBatchSize` 20 → 19 or 25; `scheduledDelayMillis` changed | 2 | | `SERVICE_NAME` renamed | 2 | | `LOGGER_NAME` changed | 1 | | record body no longer the event name | 4 | | `severityText` INFO → WARN | 1 | | null/undefined property skip removed | 2 | | `url` given a suffix the config did not set | 9 | - **Absence assertions carry positive controls.** The certificate test reads its answer out of a `Failed to read ` prefix that lives in a transitive dependency, and the 204 case asserts an empty diag channel; both would pass just as well against a dead observer. Each now proves its own observer live first. - **Flake-tested.** The 503 case's exact two-request assertion used to turn on the jitter draw: standing a 500 ms collector delay in for a loaded runner made it fail 20/20 (13 on the request count, 7 on `Timeout` masking the error class). With the jitter pinned, 20/20 pass. </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved telemetry export reliability by preventing unintended compression and certificate-related environment settings from affecting exporter configuration. - Ensured telemetry attributes with null or undefined values are omitted rather than serialized as misleading empty values. - Improved handling of unreachable or unresponsive telemetry collectors with bounded failures and appropriate diagnostic reporting. - Preserved connection reuse and batching behavior for more efficient OTLP delivery. - **Tests** - Added comprehensive coverage for OTLP formatting, authorization, batching, compression, certificates, connection handling, and failure scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:39:35 +02:00
Ignacy Łątka	86a2668fb0	test: stop the suite and the process environment leaking into each other (#1058) ... Test env leakage, both directions. **Out** — six `argent-tools-client` suites pointed `HOME`/`USERPROFILE` at a temp dir, deleted the dir, and never restored, so a `--no-isolate` run handed every later file a home that no longer exists. Lifted the redirect into a shared `test/helpers/home-redirect.ts`; the three suites that already hand-rolled the save/restore moved onto it too. **In** — `packages/telemetry` had no `setupFiles` at all, so its unit tests read the developer's shell. Added `test/setup/clear-telemetry-env.ts` (ARGENT_ prefix sweep + `DO_NOT_TRACK` + the cloud-agent detector's markers), wired via `setupFiles`, following the `@argent/tool-server` precedent — and dropped the per-file guards it replaces. **Pinned, not just fixed** — a `test/setup/assert-env-restored.ts` in each of the three packages fails any suite that leaves a watched variable changed, so dropping a restore can no longer pass. Each watches `HOME` and `USERPROFILE`, plus `PATH` (`argent-tools-client`, `tool-server`) or `CI` and the five `OTEL_EXPORTER_OTLP_*` (`telemetry`) — the ones the sweep deliberately leaves ambient and individual tests then set by hand. Each of the three `clear-*-env` setup files has a pin test that re-imports it against planted sentinels; `packages/tool-server`'s existing copy gains the mixed-case one it was missing. `packages/tool-server` is in the diff because it has the same two gaps: it originated the prefix sweep (both pre-existing copies were case-sensitive, so they are fixed together) and it has the largest redirect surface of the three — 12 files hand-rolling the redirect plus 17 going through `scopeTempHome` — with no guard. It is already green under one; deleting `scopeTempHome`'s restore turns 17 files red. Two of its suites restored `HOME` by assigning back a saved value that can be `undefined`, writing the string `"undefined"`; the guard caught both on a host with no ambient `HOME`, and they are fixed here. `windows-e2e.yml` runs seven `tool-server` files through the config this branch edits, but neither that config nor `test/setup/**` was in its `paths:` filter — so the new setup file would have reached windows-latest unverified. Added both. No docs change: test infrastructure only, no user-facing behaviour. Fixes #1028 Fixes #839 Same-class problems found while reviewing this and filed rather than folded in: #1068 (`argent-cli` fails under `DO_NOT_TRACK=1`), #1069 (the `ios-device-runner` suites pin only `HOME`, so the redirect is inert on Windows — its second half, the restore that wrote the string `"undefined"`, is fixed here because the new guard caught it), #1070 (`useTempHome` shares one saved-`HOME` slot, so a nested `scopeHome()` restores a deleted dir), #1071 (`tool-server`'s sweep is prefix-only, so an exported `ADB_SERVER_SOCKET` reddens its suite), #1072 (`external-devices.test.ts` reads the developer's real `~/.argent/providers`) and #1073 (`flow-script-environment.test.ts` fails with no ambient `HOME`). <details> <summary>#1028 — probe results, six files</summary> `test/zz-home-probe.test.ts` from the issue body, run per suite. Vitest's sequencer orders by cached duration and falls back to file size, so the cache is cleared before each run to keep the tiny probe last: ``` rm -rf node_modules/.vite/vitest ../../node_modules/.vite/vitest npx vitest run test/<suite>.test.ts test/zz-home-probe.test.ts --no-isolate --no-file-parallelism ``` | suite | before | after | | --- | --- | --- | | `launcher-state` | 1 failed \| 23 passed — `argent-state-test-… exists=false` | 24 passed | | `launcher-spawn` | 1 failed \| 8 passed — `argent-spawn-test-… exists=false` | 9 passed | | `launcher-version-gate` | 1 failed \| 7 passed — `argent-version-gate-test-… exists=false` | 8 passed | | `launcher-duplicate-spawn` | 1 failed \| 10 passed — `argent-dup-test-… exists=false` | 11 passed | | `launcher-exit` | 1 failed \| 1 passed — `argent-exit-test-… exists=false` | 2 passed | | `link-config` | 1 failed \| 43 passed — `argent-link-config-test-… exists=false` | 44 passed | | `launcher-sweep` (control, fixed in #713) | passed | passed | | `launcher-ps-guard` (control, fixed in #713) | passed | passed | The probe itself is a diagnostic and is not part of the diff — `test/setup/assert-env-restored.ts` is the durable guard, and it needs no `--no-isolate` to fire. </details> <details> <summary>#839 — the stated repro is stale; here is what is actually left</summary> The issue was filed 2026-08-18. #714 landed 2026-08-19 and hand-enumerated the variables inside `index.test.ts` (a `CONSENT_ENV_KEYS` `beforeEach`) and `base-props.test.ts` (eight `delete`s). On `aa90873b5` all three named repros are green: ``` $ DO_NOT_TRACK=1 npx vitest run --root packages/telemetry → 319 passed $ ARGENT_TELEMETRY=0 npx vitest run --root packages/telemetry → 319 passed $ CURSOR_AGENT_WORKER_ID=x npx vitest run --root packages/telemetry → 319 passed ``` What remains is the structural gap the issue names: the protection is per-file and per-variable, nothing pins it, and `vitest.config.ts` still has no `setupFiles`. This PR moves the property to the setup file and deletes the two hand-rolled guards, so the setup file is what holds it — see the mutation section, where removing the `setupFiles` line reproduces the issue's 17 and 1 exactly. Post-change sweep, one hostile value per run, 329 tests each: `DO_NOT_TRACK=1`, `ARGENT_TELEMETRY=0`, `ARGENT_TELEMETRY_DEBUG=1`, `CLAUDE_CODE_ENVIRONMENT_KIND=byoc`, `CLAUDE_CODE_ENTRYPOINT=remote`, `CLAUDE_CODE_REMOTE_SESSION_ID=x`, `CURSOR_AGENT_WORKER_ID=x`, `CURSOR_WORKER_POOL_NAME=x`, `GITHUB_ACTIONS=true`, `GITHUB_ACTOR=copilot`, `GITHUB_WORKFLOW_REF=…copilot-swe-agent.yml@refs/heads/main`, `REPLIT_AGENT=1`, `ARGENT_HOST`/`ARGENT_PORT`/`ARGENT_AUTH_TOKEN`, `CI=1`, `CI=false`, `OTEL_EXPORTER_OTLP_ENDPOINT`, `OTEL_EXPORTER_OTLP_HEADERS`, the real CI combination, and all eleven at once — 329 passed every time. `GITHUB_ACTIONS` is a ci-info vendor input as well as a copilot-agent marker, so clearing it does move `isCi()` — but only its GitHub Actions branch, and an Actions run also sets `CI`, which `isCi()` answers ahead of any vendor check. `GITHUB_ACTOR` and `GITHUB_WORKFLOW_REF` appear in neither `ci-info/vendors.json` nor `ci-detect.ts`'s generic list, so they cost nothing. The rest of the ci-info surface and `OTEL_*` are deliberately left alone; the setup file says so. </details> <details> <summary>Mutation testing — every guard, applied by hand, verified to fail, reverted</summary> | mutation | result | | --- | --- | | drop `setupFiles` from `packages/telemetry/vitest.config.ts` | `DO_NOT_TRACK=1` → 19 failed (**17 in `index.test.ts`**, matching the issue, + the two registration pins) | | drop `CURSOR_WORKER_POOL_NAME` from `CLEARED_ENV_VARS` | drift test red | | move an `env.` read into `src/detect/vendor.ts` | drift test red, naming `detect/vendor.ts` | | rewrite a read as `env["X"]`, `const { X } = env`, `const { X }: NodeJS.ProcessEnv = env` or `const { X = "d" } = env` | drift test red | | add `cloud-agent-detect.ts` to `UNSWEPT_SRC_FILES` | drift test red | | revert `toUpperCase()` in any of the three sweeps | that package's mixed-case case red | | drop `restoreHome()` from one suite / from all nine | 1 file / 9 files red, each naming its deleted dir | | drop `scopeTempHome()`'s restore in `tool-server` | 17 files red | | drop the hand-rolled `CI` restore in `base-props.test.ts` | red, naming that file (green before; under `--no-isolate`, 7 unrelated failures in `otel-endpoint.test.ts`) | | narrow either guard's filter to variables that were already set | `assert-env-restored.test.ts` red | | replace the leaked half of the guard's message with a constant | `assert-env-restored.test.ts` red | | rename one function's `env` parameter in `cloud-agent-detect.ts` | drift test red — the read is found under the new binding | | narrow any guard's filter to a subset of `SCOPED_ENV_VARS` | `assert-env-restored.test.ts` red — the pin plants every entry | | drop the hand-rolled OTLP restore in `otel-endpoint-live.test.ts` | red, naming all four leaked variables | | move the guard's `AMBIENT` snapshot inside `assertEnvRestored` | `assert-env-restored.test.ts` red in all three packages | | delete the guard's `afterAll(...)` registration | `assert-env-restored.test.ts` red in all three packages | | drop `restoreHome()` from `artifacts.test.ts`'s two `afterEach` | red — the file-level guard catches a per-test omission | | drop ps-guard's `PATH` restore | red, naming the file | | empty `leakedEnvVars`, or drop a name from `SCOPED_ENV_VARS` | `assert-env-restored.test.ts` red | | replace the helper's delete branch with a plain assignment | `home-redirect.test.ts` red | | drop `USERPROFILE` from the helper | `home-redirect.test.ts` red | </details> <details> <summary>Local CI parity</summary> ``` npx vitest run --root packages/argent-tools-client 213 passed | 1 skipped npx vitest run --root packages/argent-tools-client --no-isolate 213 passed | 1 skipped npx vitest run --root packages/telemetry 329 passed npx vitest run --root packages/telemetry --no-isolate 329 passed npx vitest run --root packages/tool-server 6166 passed | 1 skipped npx tsc --noEmit -p packages/{argent-tools-client,telemetry,tool-server}/tsconfig.test.json clean npm run format / npm run lint / npm run knip clean ``` </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Improved test isolation by consistently restoring temporary environment settings after test runs. - Added safeguards that detect leaked environment settings and flag incomplete cleanup. - Added coverage to ensure telemetry-related environment variables are cleared before tests and do not affect results. - Added cross-platform checks for environment variable handling, including mixed-case names. - Expanded validation for home-directory redirection and restoration behavior. - Added checks confirming environment cleanup runs in the correct order across test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:38:51 +02:00
filip131311	c573e9c524	feat(telemetry): report device_kind next to platform on tool events (#1049) ... ## Why Telemetry cannot tell a physical iPhone from a simulator: `platform` is `ios` for both, and the tool-server's `kind` never reaches the events. Physical-iOS support shipped in 0.24.0 with no way to count its adoption except through `ios_device_*` failure stages on `tool:fail`. Physical Android phones have the same gap. ## What `tool:invoke` / `tool:complete` / `tool:fail` gain `device_kind: simulator | emulator | vvd | device | app`, derived from the same device id as `platform` (single classifier in `telemetry-platform.ts`, which `debugger:tool_outcome` and `lens:*` also go through for their platform). `platform: ios` + `device_kind: device` is a physical iPhone. The kind is emitted only for positively recognised id shapes, so the metric is not polluted by ids that are not devices: | id shape | platform | device_kind | |---|---|---| | simulator UUID / `remote:` | ios / ios-remote (tvos when the cache is warm) | simulator | | physical iPhone UDID | ios | device | | `emulator-NNNN` | android (android-tv when warm) | emulator | | USB serial, 6–20 alnum with a digit and a letter | android | device | | non-loopback `ip:port` | android | device (best effort) | | loopback `127.0.0.1:port` / `localhost:port` / `::1:port` | android | emulator | | anything else non-iOS-shaped (Metro 40-hex handle, `booted`, `Pixel_7`, IPv6, mDNS) | android | omitted | | `chromium-cdp-<port>` / `amazon-…` | chromium / vega | app / vvd | | `avdName`-only boot | android | omitted | A child sub-tool that names its own device replaces the parent's platform, kind and provider label wholesale, so an `avdName`-only child never reports the parent's `simulator` next to its own `android`. A logical-keyed Metro session strips the kind. `unknown` (in the registry `DeviceKind`) is rejected by the sanitizer like `platform: "unknown"`; no registry change. ## Querying - Physical iPhone adoption: `tool:invoke` where `platform = 'ios' and device_kind = 'device'`, distinct users over it. `isIosPhysicalUdid` is a positive match, so invoke is clean. - Physical Android: count on `tool:complete` (the device answered). `tool:invoke` / `tool:fail` still carry residual mistakes that happen to look like a serial (`Pixel7`), and a non-loopback `ip:port` may be a networked virtual device. ## Docs - `Telemetry.md` → 1.04, effective date 7 September 2026 (adjust to the release date on merge). Discloses the new category and, while touching that section, the external-provider label that #735 added without a notice line — happy to drop that second bullet if it should ship separately. - No docs-site change: `docs/reference/telemetry.mdx` links to `Telemetry.md` and has no per-property list. ## Notes - Pre-existing, unchanged: HTTP-edge failures carry no `tool_invocation_id`; `stop-all-simulator-servers` declares no capability, so its invoke row has neither field while its fail row has both. `device_kind` follows `platform` in each site. - Legacy 40-hex iPhone UDIDs already classify as `android` today; they now get no kind rather than a wrong one. - Design went through three adversarial review rounds before implementation and one on the diff; the fallback-bucket and child-attribution rules above came out of those. ## Verification - `packages/telemetry`: 319 tests, `packages/tool-server`: full suite 5670 passed / 1 skipped; `typecheck:tests` both packages; `eslint --max-warnings 0`; `knip`; prettier. - New: `test/telemetry-device-kind.test.ts` (the shape table above), plus cases in `http-tools-meta`, `http-platform-alias`, `sanitize`, `registry-listener`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BSGpeAwvoCv2JAhbLsuto2 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Telemetry now records a device category—such as simulator, emulator, physical device, virtual device, or desktop app—alongside the platform. * Device categories are inferred from device information and may include a short label for external device providers. * Device category information is included in tool invocation, completion, and failure telemetry when available. * Device identifiers themselves continue to be excluded from telemetry. * **Documentation** * Updated the Argent Privacy Notice (Telemetry) to version 1.04, effective 7 September 2026. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-07 11:55:57 +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
Ignacy Łątka	5d8fc10799	test: stop tests binding machine-wide resources they do not own (#714) ... ## Why Three flaky or failing tests were diagnosed separately this week, and all three turned out to be one defect wearing different clothes: **a test binding a machine-wide resource it does not own**, so a second concurrent run — another checkout, a CI matrix cell, a developer's own app — perturbs it. Those three are fixed in #707, #709 and #711. Each of those was found by chasing a symptom, and each audit covered only its own resource type. This is the systematic sweep: all **448 test files across 13 packages**, plus the 4 `node --test` scripts. (The sweep itself ran over the 400 files that existed when it started; merging `main` brought the count to 448, and every category below was re-run over the full set — 25 files assign `env.HOME` and none omit `USERPROFILE`, one test file calls `pkill`/`pgrep` (`vega-cli-timeout.test.ts`, scoped out to #711 below), and no `listen()` names a fixed port.) ## Findings | Category | Files checked | Real findings | |---|---|---| | Network endpoints (bind / probe / unix socket) | 448 | 4 | | Filesystem — shared temp namespace (`os.tmpdir()` and literal `/tmp`) | 448 | 8 | | Filesystem — real `$HOME` / `~/.argent` | 448 | 4 (28 files) | | Ambient environment | 448 + all `src` readers | 12 | | Device / emulator / SDK state | 448 | 2 | | Process table (`pgrep` / `pkill` / fixed-pattern sweeps) | 448 | **0** | | In-process globals leaking across files | 13 packages × 13 modes | **0** | Every finding was reproduced with a concrete instrument before being touched, and the instrument re-run after. Representative: - **`stop-tools.test.ts`** ran the real `stop-metro` against port 59999, which `lsof`s and **SIGTERMs whatever it finds**. With a squatter there: `16 passed` → `1 failed`, and the squatter logged `received SIGTERM #1`. Now port 1. - **`bind-failure-telemetry` / `lens-relay-telemetry`** were gated on the real `~/.argent/flags.json`. Enabling the documented `tool-server-event-log` flag: `4 passed` → **`4 failed`**, and records were appended to the developer's real event log. - **LogFileWriter (12 files)** — its constructor `mkdir -p`s `os.homedir()/.argent/tmp`. Instrument: run each of the 16 test files that reference a writer, or a blueprint that builds one, under a scratch `HOME`/`USERPROFILE`, then test `$HOME/.argent/tmp` for existence. 4 created it; the 12 that actually construct a writer now all call `scopeTempHome`, and none creates it. - **`screen-recording.test.ts`** — the device ids are fixtures and `vi.useFakeTimers()` seeds its clock from the real time, so only the millisecond a run starts in separates two runs' `.mp4` paths, and each test deletes the path it derived. Self-concurrent: **2 of 3 rounds failed** → 0 of 4. Scoping `TMPDIR` removes the window rather than narrowing it. - **`DO_NOT_TRACK=1`** → telemetry `282 passed` → **`13 failed`**. **`npm_config_user_agent=pnpm|yarn|bun`** → installer `535 passed` → 4/5/3 failed. **19 tests required a real `adb`** — PATH shadow + empty HOME: `30 passed` → **`19 failed`**. - A fixed `/tmp/argent-nd-<udid8>.sock` **destroyed a concurrent holder's live socket** (proven by inode) in **3** files that run the native-devtools factory. Two of them also fail outright: `native-devtools-app-state` lost **3 of 3** rounds of two concurrent runs on `expected 'unregistered' to be 'connected'`. All three now derive the UDID from `process.pid`. - `flow-visual`, `react-profiler/dump`, `file-inputs`, `http-upload` — shared `/tmp` paths and prefix scans, each reproduced with a squatter. In `http-upload` the collision surfaced *as a timeout*, inside `vi.waitFor`. (`flow-chromium-boot` was fixed on this branch too, but main's #585 rewrote that file afterwards and the merge kept main's copy, so it is byte-identical to `origin/main` here and carries no fix.) ## Clean, with the evidence - **Process table** — an `LD_PRELOAD` exec interposer over a full run (507 execs) plus all 12 other suites. Excluding `vega-cli-timeout.test.ts`, which is #711's and does sweep the table (`pkill -f '^sleep 6910[0-9]$'` at line 134): no `pgrep`/`pkill`/`killall`/fixed-pattern sweep anywhere, and **no test signals a pid it did not spawn**. - **Real device state** — **zero** exec attempts of real `adb` / `simctl` / `emulator` / `avdmanager` / `vega`; no test asserts a device list. That list was too narrow: `workspace-reader-integration` spawned the real `yarn` / `pnpm` / `bun` / `pod` / `eas` / `expo`, which is the second finding in this row. - **Cross-file in-process leakage** — structurally impossible: vitest 4.1.9 resolves `pool: "forks"` / `isolate: true`, confirmed by a scratch experiment (distinct pids, no env leak, module `Map` = 1,1,1). 120 default-mode runs across 12 packages produced 1 failure. - **`scripts/`** — 4 files, 86 tests: `mkdtemp` throughout, no ports, no sweeps, writes nothing. ## Verification Each fix was mutated back and killed by a **named** test under its instrument — e.g. `× reads a plain absolute path`, `× returns null when only the extensionless binary exists`, `× pins both names os.homedir() consults, at the same directory`. Restores verified with `git diff` / `git status`, never a substring grep. **Pinned by an instrument only:** the socket fix for `ios-only-blueprint-gate`, the `~/.argent/tmp` and `~/.argent/flags.json` fixes, the `/tmp` fixture cleanup, the `stop-metro` port change (the test asserts `stopped: false` / `pids: []`, which hold for either port whenever nothing is listening) and the `workspace-reader` `PATH` pin (only `node` is asserted; the other seven probes are not) are not pinned by any suite assertion — reverting them still passes, and only the external squatter / scratch-`HOME` instruments catch them. They are pollution fixes. The `native-devtools-app-state` socket fix is the exception in the other direction: reverting it turns two concurrent runs red. Gates, unpiped and read by exit code: `eslint .` 0, `prettier --check .` 0, `tsc --build` 0, `typecheck:tests` 0. Suite, run as six shards: tool-server **4287 passed, 1 skipped, 0 failed**; argent-cli 462; telemetry 306; argent-tools-client 191; configuration-core 122; argent-mcp 80; argent-installer 555. All other packages and `test:scripts` green, identical to baseline. `lens-tools-platform-gate.test.ts` times out at the 5s default under full-suite load and passes at `--testTimeout=30000` (24.7s, almost all of it transform). It predates this branch and is untouched by it. ## Scope This branch is **test-only** — no `src/` file is touched. Both production defects the sweep turned up are separated out, so each can be reviewed as the user-facing fix it is: `ps` without `-ww` orphaning live tool-servers is #713, and #852 is `ensureToolsServer` respawning on every call when `ARGENT_HOST` is exported — `buildToolsServerEnv` passes the inherited value to the child while the state records the literal `127.0.0.1` the health check then probes. The second surfaced here as eight stranded `fake-tool-server` processes, and pinning the environment in this branch removes that signal, which is why it is filed rather than merely worked around. A third, outside this branch entirely, is #853: ten `tool-server` test servers omit the host on `listen(0)`, binding every interface where twenty-two sibling sites pass `127.0.0.1`. It is hardening, not a live exposure — `createHttpApp`'s Host allow-list answers a LAN-addressed request with 403, and only a raw client forging `Host: 127.0.0.1` reaches the stub registry behind it, inside the ~100 ms one `it()` holds the socket. It is also not a collision risk: ephemeral ports are kernel-assigned and never reissued while in use, which is why the fixed-port audit above is unaffected. All ten predate this branch. Left untouched on purpose: `boot-electron-spawn-error.test.ts`, `chromium-discovery.test.ts` and `vega-cli-timeout.test.ts` — owned by the still-open #709 and #711, and all three still carrying their defects on this base. (#707 has since merged into this base, so `boot-device-hotboot.test.ts` and `vitest.config.ts` carry its fix rather than a defect; `test/setup/clear-argent-env.ts` is its work, and this branch relies on it.) **Not fixed, deliberately** — an adjacent class (test-*order* dependence, not machine-global resources), reproducible if wanted: `link-config.test.ts` (deterministic: `-t "returns null when required field"` → 4 failed), `network/network-integration.test.ts` and `android-perfetto/dispatch.test.ts` (fail under `--sequence.shuffle` seeds). **Also observed, not addressed:** one green tool-server run leaves **56** entries behind in a scratch `TMPDIR` — **30** in the `argent-screenshot-diff*` family, 4 `argent-events-*`, 3 `argent-flow-crop-*`, 3 `argent-tar-upload-*`, 2 `argent-flow-diff-*`, 2 `argent-file-input-*`, and 12 assorted singletons (`ast-index-*`, `component-source-*`, `argent-profiler-cwd`, `argent-chromium-media`, …). Measured per run against a scoped temp dir, so it ranks the contributors rather than counting one machine's accumulated history — screenshot-diff, not flow-crop, is the dominant one. A separate leak. That instrument is blind to a *literal* path, which is how it missed the two `/tmp/native-profiler-*-report.md` writers the fourth pass found. Re-run across all 13 packages, `argent-mcp` is the only one outside `tool-server` that leaves anything — two entries per run, filed as #854. ## Second pass A review of this branch re-ran every category above with its own instrument rather than reading the diff, and found the sweep had pinned four resources in one file while leaving a sibling that does the same thing live. Fixed in `4289e3d8f` and `8164955b4`; the table and bullets above are the corrected totals. - **`tool-server-event-log`** was mocked off in 2 of the 4 files that call the real `start()`. With the flag on, `startup-telemetry` and `tool-fail-invalid-params-emission` failed 3 tests in `attachRegistryEventLogger` and truncated the real `~/.argent/tool-server-events.jsonl` to 0 bytes first. `ARGENT_EVENT_LOG` cannot redirect that: `test/setup/clear-argent-env.ts` strips every `ARGENT_*` var before the module graph loads. - **`scopeTempHome`** reached 8 files; 4 more built a real `LogFileWriter` through the JS-runtime-debugger blueprint. - **`os.homedir()` on Windows** reads `USERPROFILE`, not `HOME`. `android-binary-windows` and `adb-resolve-avd-path` pinned only `HOME`, and both run on a real windows-latest host. Proven by running the resolver under a `node:os` mock that reproduces the Windows lookup: with `HOME` alone the stock Studio SDK under the profile resolves; with both pinned it misses. `update-checker` had the same gap. - **`getConsentState`** honours a falsy `ARGENT_TELEMETRY` at the same precedence as `DO_NOT_TRACK`, ahead of the config override the suite writes. Only `DO_NOT_TRACK` was pinned: `ARGENT_TELEMETRY=0` gave `17 failed | 6 passed`. - **`workspace-reader-integration`** let the version probe spawn the real package managers off `PATH`; their shims self-install, so the file pulled **~44 MB** into `~/.cache/node/corepack` and `~/Library/Caches/eas-cli`. `PATH` is now a directory holding only `node`, the one version the test asserts — 44 MB → 4 KB. - **`argent-mcp`** snapshotted `ARGENT_AUTO_SCREENSHOT_DELAY_MS` without clearing it, and that package has no `ARGENT_*`-stripping setup file, so 3 tests asserted the developer's exported value. Smaller items in the same commits: the `/tmp` socket fixtures now remove the directories they create per run; `http-upload`'s pre-scan snapshot is gone, since scoping `TMPDIR` to a fresh directory made it permanently empty and its filter inert, as its two siblings already did; and two `detectProjectPackageManager` assertions that enumerated the entire return union are pinned to a single value. The confirmation pass also caught a defect the first round of fixes introduced: pinning `HOME`/`USERPROFILE` to the fixture root in `android-binary-windows` let `<home>/Android/Sdk` shadow the `%LOCALAPPDATA%` root the two positive tests are named after, so deleting `roots.push(join(localAppData, …))` from the resolver left all three green. Home has its own directory in `6aee3ee7a`, and that mutation now fails both tests. A third pass, running the nearest-twin move repo-wide, found the `os.homedir()`-on-Windows gap in one more package: eight `argent-tools-client` test files redirect only `HOME`, and four of them recursively delete `STATE_DIR` — two before each test, two after — which on Windows is the developer's real `%USERPROFILE%\.argent`. Fixed in `139447d4a`, together with the same gap in `avd-snapshot`. Nothing runs that package on Windows CI today, so no current run fires it. ## Fourth pass A fourth pass audited the three fix commits and re-ran the sweep with two instruments the earlier ones did not have. Both holes were in the *measurement*, not the diff — which is why three passes missed what they found. Fixed in `a67c43f8d`. - **`os.tmpdir()` does not read the same variable on every platform.** POSIX takes `TMPDIR`, `TMP`, `TEMP` in that order; Windows takes `TEMP`, then `TMP`, and never looks at `TMPDIR`. The five pins this branch added set `TMPDIR` alone, so on Windows production still materialized into the machine-wide `%TEMP%` while the test scanned a scratch directory nothing ever wrote to — and `file-inputs`, `http-upload` and `flow-visual` each end in a leak assertion over that listing, which then passes whatever the code does. Against `origin/main`, which diffed a before/after listing of the real tmpdir, that is a regression rather than a wash. `redirectTmpdir` (`test/helpers/tmpdir-env.ts`) sets all three and returns the restore; the **7** files that scope a tmpdir use it, including the two `android-perfetto` ones that predate this branch. `test/tmpdir-env.test.ts` pins both lookup chains — reverting the helper to `TMPDIR` alone fails two of its three tests. All three leak assertions were re-confirmed live on POSIX by mutating the production cleanup each one covers. - **A hardcoded absolute path is invisible to a `TMPDIR`-scoped instrument.** `session-correctness` and `ios-instruments/analyze-freshness` passed a literal `/tmp/native-profiler-{x,ios}.trace` as the session's trace file, and the renderer writes its report beside it (`deriveReportPath` = `dirname(traceFile)/<base>-report.md`). Every run left `native-profiler-x-report.md` and `native-profiler-ios-report.md` in the shared `/tmp`; nothing removed them, and two concurrent runs write the same names. Reproduced with `TMPDIR` pointed at a scratch dir — the files still landed in `/tmp`, which is exactly why the `os.tmpdir()` row and the leftover census below both read clean here. Both tests now own the directory. - **A prefix-scanned env var is invisible to a grep for variable names.** `secretSources` enumerates `process.env` by the `ARGENT_SECRET_` prefix and ranks it *above* every file the test writes, so `secrets-command.test.ts` — which already pins `HOME`, `USERPROFILE` and cwd — still read the developer's shell. `ARGENT_SECRET_FOO=bar`, argent's own documented way to supply a secret: `6 passed` → **`3 failed | 3 passed`**, deterministic. `argent-cli` has no `ARGENT_*`-stripping setup file, so the prefix is cleared per test. Its sibling `configuration-core/tests/secrets.test.ts` was already immune — it threads an explicit `env: NO_ENV` into every call. - **`android-binary` and `vega-cli-fallback`** were the last two files redirecting `HOME` without `USERPROFILE` — the former the direct twin of `android-binary-windows`, in the same directory, driving the same resolver. Under a `node:os` mock reproducing the Windows lookup with an SDK planted in the profile, it loses **5 of 11** tests. No test file in the repo redirects `HOME` alone any more. (`6fbb66ce1` later skipped both of this file's describes on win32 — its fixtures are extensionless and the win32 resolver only takes `.exe` — so its own `USERPROFILE` pin is now symmetry with the sibling files that do run there, not a live guard.) Two smaller items in the same commit. `native-devtools-socket-bind`'s ENOENT fixture still rooted at `os.tmpdir()` while the comment three screens above explains why the others do not: with a 142-character ambient `$TMPDIR` that path exceeds `sun_path` and `listen` rejects it for *that* reason, which the assertions cannot tell from the missing-directory one they were written for. And `<home>\AppData\Local\Android\Sdk` — the resolver's fallback for a GUI-spawned server that never inherited `%LOCALAPPDATA%` — could be deleted with the whole suite green; it now has a test, and deleting either win32 root fails exactly the tests named for it. ## Fifth pass A fifth pass took the platform-branch shape the fourth pass exposed and ran it as its own move: *for every function that branches on `process.platform`, does the test's guard cover every arm, or only the one the author's machine takes?* Fixed in `e08b466b6`. - **`init-stale-config.test.ts` deletes the developer's real VS Code MCP config on Windows.** The file mocks `os.homedir()` and says it does so "so hidden-scope probes (`~/.claude.json`, VS Code user profile) never touch the real home directory". `vscodeUserDirs()` branches `darwin → homedir()/Library/Application Support`, `win32 → %APPDATA%`, `else → homedir()/.config` — the win32 arm never calls `homedir()`, so the mock is inert there, and the test's own `userMcpJsonPath()` mirrors the same branch. Both user-profile tests then write a one-entry fixture over `%APPDATA%\Code\User\mcp.json` with a whole-file `writeFileSync`, and assert that `remove()` deleted it. Reproduced by forcing `process.platform` to `win32` with a two-server `mcp.json` planted at a probe `%APPDATA%`: **`3 passed`, and the file and its directory were gone.** With `APPDATA` pinned, the whole file passes under the same instrument — 31 tests — and the planted config is byte-identical. - **`argent-tools-client` hands its ambient `ARGENT_*` to a real spawned child.** The launcher spawns an actual tool-server, and `buildToolsServerEnv` overrides only the keys it is given — everything else rides in on `...baseEnv`. `ARGENT_HOST` is the one that bites: the fixture binds the host it names while the launcher records and health-checks `127.0.0.1`, so reuse misses and a second server is spawned. With six overrides exported: **`10 failed | 181 passed`, and eight stray `fake-tool-server` processes still holding TCP listen sockets after the run.** With the prefix stripped: `191 passed`, none surviving. The package now carries the same setup file `tool-server` has, for the same reason — and it is the second of the two packages whose missing `ARGENT_*` strip this review found. ## Later passes A sixth pass audited the fix commits themselves and bounded the SDK probe (`6fbb66ce1`); a seventh ran the moves against the diff one more time and turned up the two below, plus the `listen(0)` defect filed as #853. - **The `PATH` pin made `workspace-reader-integration` POSIX-only without the guard its siblings got.** The shim it puts on `PATH` is an extensionless `node`, and `symlink()` to a file needs an elevated process or Developer Mode on Windows, so `beforeAll` throws `EPERM` there before any assertion runs; a copy in place of the symlink would not rescue it either, since a bare command name resolves through `PATHEXT`. This sweep already skips `android-binary` and `native-devtools-socket-bind` on win32 for exactly this class, and the Windows job runs a curated list of seven `tool-server` files that excludes this one, so nothing was catching it. Fixed in `095d75bd8` — verified POSIX `2 passed`, and under a forced win32 platform `2 skipped` with the top-level `beforeAll` never entered. - **A rationale that rested on a case the file excludes.** `android-binary`'s header justified pinning `USERPROFILE` by what `os.homedir()` reads on Windows, two sentences before noting that both describes skip on win32. Fixed in `f37b3605e`; the variable stays in the snapshot list, the justification no longer claims a platform this file never reaches. A seventh pass also re-ran the sweep against a hostile machine: every feature flag on in a scratch `~/.argent/flags.json`, no `adb` on `PATH`, and a `TMPDIR` long enough to overrun the 104-byte `sun_path` limit. It found one resource this branch had not closed. - **A red `launcher-version-gate` run strands a real tool-server.** `ensureToolsServer` spawns a fixture server, but its pid only reached the reaper after three or four assertions on the state it returned; any of those throwing left the pid unrecorded, and the same `afterEach` then deleted `STATE_DIR`, the only other record of what to kill. **Eight such processes were alive on the development machine**, spread from 14:33 to 18:23, every one from an `argent-version-gate-test-*` fixture and none from any other family — each still holding a TCP listen socket with its fixture directory long gone. Forcing one assertion to fail took the `tool-server.cjs start` count from 10 to 11. The nearest twin, `launcher-duplicate-spawn`, guards this twice: it records the pid straight after the null check and sets `FAKE_TTL_MS` so an escaped server self-exits. Fixed in `26c2c60ec` with both, the TTL also covering the window between the spawn and the state read where no pid exists yet — the same forced failure now leaves **zero** strays. An eighth pass audited the seventh's own fixes, and found the version-gate fix had been applied to one file when the defect was a property of four. - **Three more files strand a real tool-server on a red run.** `launcher-spawn` and `launcher-exit` set `FAKE_TTL_MS` nowhere; `launcher-duplicate-spawn` — the file `26c2c60ec` cites as the model — armed it inside one test's `try/finally`, so eight of its nine spawns ran without it and the `finally` disarmed it for the remainder of that test. Reproduced per file by forcing a failure before the pid is recorded: a detached node process survives, reparented to init, holding a listening socket, with its `TEST_HOME` removed and its state record already cleared. Fixed in `3bb69ded6` by moving the net to a file-wide `beforeEach` in all three. Verified end to end — the child carries `FAKE_TTL_MS=60000`, and a stranded server spawned at 21:28:36 exited by itself at 21:29:36 with nothing signalling it; three consecutive green runs stay at 191 passed. - **A 29th file writes the real `~/.argent`.** `boot-electron-spawn-error`'s success-path test completes a boot, and a completed boot calls `trackChromiumPort`, persisting to `os.homedir()/.argent/chromium-cdp-ports.json`. The real file on the development machine had accumulated **106 dead ephemeral ports**. Its two siblings in the same directory already mock `chromium-discovery` for exactly this reason; this was the third, with no mock. Fixed in `3306b6f40`. - **`scopeTempHome` was itself unpinned.** Deleting its `USERPROFILE` line — the Windows half, the same property the sibling helper's test pins — left all twelve consumers green at 48 passed. `3306b6f40` adds the missing test; it kills that mutation plus dropping the `HOME` pin and replacing the per-test `mkdtemp` with one shared directory. Two findings from that pass were refuted on verification and are recorded rather than acted on: the tool-server tmpdir leftovers (mechanism real, but they are the separate leak this body already scopes out) and a claim that `26c2c60ec`'s message overstated its twin. Four comments were also corrected against their code: `deriveReportPath` joins `<base>-report.md` *inside* the trace's directory rather than appending to the directory's name, which is what makes the `rm(traceDir)` added in `a67c43f8d` cover it; two `android-perfetto` comments still named `TMPDIR` as the mechanism after the switch to `redirectTmpdir`; and `android-binary`'s header claimed the home pin lets its two positive tests resolve "on either platform", though its fixtures are extensionless and the win32 resolver only accepts `.exe`.	2026-08-19 23:06:48 +02:00
Ignacy Łątka	0a6dbec87c	feat(telemetry): export events via OpenTelemetry OTLP logs instead of PostHog (#570) ... ## What Swaps the `posthog-node` transport for an OpenTelemetry Logs exporter. Everything above the transport stays as it is - opt-out consent, per-machine identity and fingerprint, the sanitizer allowlist, the typed event surface, crash diagnostics. Only the ~3-method client boundary changes. One analytics event becomes one OTLP/HTTP log record: | PostHog | OpenTelemetry | | --- | --- | | `capture({ event, distinctId, properties })` | `logger.emit({ body: event, attributes: { distinct_id, "event.name", ...props } })` | | event name | log-record `body` | | `distinctId` | `distinct_id` attribute | | event/base properties | log-record attributes | | `client.shutdown()` | `LoggerProvider.shutdown()` (force-flush + teardown) | Records go to a hard-coded Software Mansion collector. The endpoint and the authorization header are passed to the exporter explicitly in code, and an explicit value beats the corresponding `OTEL_EXPORTER_OTLP_*` variable - the same anti-exfiltration property the fixed PostHog host had. ## Changes - `packages/telemetry/src/posthog.ts` becomes `otel.ts`: `LoggerProvider` + `BatchLogRecordProcessor` + `OTLPLogExporter` (HTTP/JSON). Queue size 20 and a 10 s flush interval mirror the old client; the per-export deadline is 1.5 s, kept at or below `index.ts`'s drain budget so a stalled export can't hold a short-lived command open. - Dependencies: drop `posthog-node`, add `@opentelemetry/{api,api-logs,sdk-logs,exporter-logs-otlp-http,resources}`. They inline cleanly into the esbuild bundles. - Build-time ingest token via esbuild `define` (`ARGENT_OTEL_INGEST_TOKEN`), same mechanism as `ARGENT_CLI_VERSION`. Unset in a dev build leaves the client unconstructed and telemetry inert. - Base props: drop `$process_person_profile` (a PostHog directive, meaningless to a collector) and rename `$session_id` to `session_id`. Null-valued attributes are dropped, since OTel rejects them - `cloud_agent` is null on the common path, and absence means the same thing. - `Telemetry.md`: the "who we disclose to" and "international transfers" sections describe a Software Mansion-operated OTLP collector instead of PostHog as a third-party processor. ## A hang this uncovered `timeoutMillis` only bounds a request once its socket is connected, because the exporter applies it with `req.setTimeout()`. Against a collector address that drops packets rather than refusing them - a corporate egress filter, a dead host behind a firewall - the socket sits in the connecting state where no export deadline reaches it, and the process stays alive for the OS connect timeout: 75 s past a `shutdown()` that had already resolved at 1.75 s. Pairing the export deadline with an agent-level socket timeout, armed at socket creation so it covers connection establishment too, fixes it: | collector state | before | after | | --- | --- | --- | | healthy | 0.18 s, delivered | 0.18 s, delivered | | connection refused | 0.94 s | 1.03 s | | blackholed (http and https) | 75.0 s | 1.6 s | | connected but never responds | - | 1.6 s | `posthog-node` exited in 10.6 s under the same blackhole (undici aborts a connecting socket), so this was a regression as well as a hang. A unit test that fails without the fix guards it. ## Found in review Each of these was reproduced before it was changed, and each is pinned by a test that fails without the fix. - **The environment's OTLP headers were forwarded to the collector.** The SDK merges `OTEL_EXPORTER_OTLP_HEADERS` / `_LOGS_HEADERS` into every request, keeping any key the code does not set itself. Out in the world that variable holds the developer's *own* observability credential - a Honeycomb team key, a Dynatrace `Api-Token`, Grafana Cloud basic auth - so any machine already running OpenTelemetry shipped that third-party secret here on every batch, without it passing the sanitizer or appearing in `Telemetry.md`. Reproduced by watching `x-honeycomb-team: hcaik_…` arrive at a capture server. The variables are now cleared across the exporter constructor (where the SDK resolves its header set) and restored after. - **`httpAgentOptions` silently disabled connection reuse.** Supplying it replaces the agent the SDK would build, whose default is `keepAlive: true`. Measured on loopback: three batches shared one socket by default, and opened three sockets with `httpAgentOptions` - a fresh TCP+TLS handshake per 10 s batch in the long-lived tool-server. `keepAlive` is restated alongside the timeout. - **`markDisabled()`'s drain had no grace period.** It raced flat against `SHORT_FLUSH_TIMEOUT_MS` while `shutdown()` allowed `timeoutMs + 250`. Since the exporter's own deadline equals that budget, the flat race fired at the same instant and always abandoned the batch instead of letting it finish. Both call sites share one `raceDrain()` now. - **Export failures reached nobody.** The batch processor hands errors to OpenTelemetry's global handler, so `LoggerProvider.shutdown()` resolves identically for a delivered batch and a rejected token - making `shutdown()`'s `catch` unreachable. The SDK's diagnostics are now routed into the existing debug channel under `ARGENT_TELEMETRY_DEBUG=1`; a run without the flag stays silent so no host application's diag logger is disturbed. - **The build-time token now beats the `__ARGENT_OTEL_TOKEN_TEST` seam** rather than losing to it, so the seam reaches unbundled source only. Verified with esbuild across all three build modes. - **The endpoint guard test mocked the exporter it guards.** It could only show that the code passes a `url`; an SDK upgrade that changed the precedence would have kept it green. `otel-endpoint-live.test.ts` drives the real exporter against loopback servers - dropping the explicit `url`, the explicit `authorization`, or the header clearing each fails it. - **The wire shape had no pins at all.** Bypassing the sanitizer entirely, sending an explicit `null`, renaming `service.name` and dropping `event.name` all left the suite green. Four tests now fail on those mutations. The sanitizer bypass was unpinned on `main` too. ## Before this ships 1. ~~**Collector URL**~~ - done in 8bca962f. `OTLP_LOGS_ENDPOINT` is `https://argent-otel.swmansion.com/v1/logs`. That host serves OTLP as of 2026-08-12: a `POST` answers the OTLP-shaped `401` (`{"code":16,"message":"provided authorization does not match expected scheme or token"}`), where it previously `301`d to `/v1/logs/` and `404`d there. It is a distinct host from the test collector (`167.235.245.1` vs `46.224.172.171`, its own certificate). 2. **Ingest token** - `ARGENT_OTEL_INGEST_TOKEN` has to be set in the release build environment. Nothing under `.github/` sets it today, so every published artifact resolves an empty token and stays inert - silently: with no token the client is never constructed, so a run emits nothing and writes no `telemetry-debug.log` even under `ARGENT_TELEMETRY_DEBUG=1`. It must be the **production** host's token: `argent-otel.swmansion.com` runs its own token list and rejects the test collector's. 3. **IP geolocation** - PostHog's `disableGeoip: true` set `$geoip_disable` on every event, which is what stopped location being derived from the source IP. There is no in-repo equivalent for a collector, so that suppression has to be configured collector-side to keep `Telemetry.md`'s "what we collect" list accurate. 4. **Legal** - the privacy-notice edits and the version/date bump to 1.03 are drafted to match the new architecture, but Legal should review and finalize them. Note the notice actually served at `TELEMETRY_DETAILS_URL` is still Version 1.01 and names PostHog as the processor. ## Verification Unit and integration: - `packages/telemetry`: 293 tests pass, including the rewritten `index.test.ts`, `otel-endpoint.test.ts` and the new `otel-endpoint-live.test.ts`. - Every workspace suite green on the merged branch: argent-cli 276, argent-installer 535, argent-mcp 78, argent 60, tools-client 178, configuration-core 83, registry 87, tool-server 3070, update-core 31, archive 11. - `bundle-tools.cjs` leaves no `posthog` in any of the four bundles. OTel is inlined into the three that emit - `cli-cmds.mjs`, `installer.mjs`, `tool-server.cjs` - each carrying the endpoint and the token define as string literals. `mcp-server.mjs` carries none: it is a stdio shim over the tool-server, which is the process that reports. - `lint`, `prettier --check`, `tsc --build`, the `tsconfig.test.json` gate and `npm install --package-lock-only` (idempotent on Node 24) all clean.	2026-08-17 21:25:51 +02:00
filip131311	4975ff490c	fix(debugger): cut debugger tool fail rate — structured not-connected results, classified CDP failures, narrow recovery (#610) ... ## Why PostHog (30d) shows the debugger tool family is argent's least reliable surface: | tool | fail rate | dominant causes | |---|---|---| | `debugger-status` | **37%** | METRO_NO_TARGETS / SERVICE_INITIALIZATION_FAILED / zod | | `debugger-connect` | **24%** | SERVICE_INITIALIZATION_FAILED (~7k, >1k users), METRO_NO_TARGETS | | `debugger-log-registry` | 41.7% raw / 4.2% excl. storms | one user retry-looped it **12,742×** on NO_TARGETS | Three design issues drive this: (1) "app/Metro not ready yet" — an expected precondition — is reported as a tool *error*, which both inflates fail rates and invites agent retry storms; (2) every CDP init/lifecycle fault lands in telemetry as the catch-all `REGISTRY_SERVICE_INITIALIZATION_FAILED` with `error_kind=unknown`; (3) ~600 users/30d hit zod validation failures with zero visibility into which parameter was wrong. ## What **1. Classified failures** (`d71efa43`) — six former plain-`Error` sites in `cdp-client.ts` / `target-selection.ts` now carry dedicated `DEBUGGER_CDP_*` / `DEBUGGER_TARGET_DEVICE_MISMATCH` signals (messages byte-identical). The registry's two causeless "terminating window" `ServiceInitializationError`s get their own `REGISTRY_SERVICE_TERMINATING` code. **2. Narrow recovery** (`ff9c1955`) — `recoverable()` on both debugger blueprints, scoped to windows where retry is provably safe: Metro recovers only `DEBUGGER_CDP_NOT_CONNECTED` (request never left the host; a detected socket death already self-heals for the *next* call via the terminated cascade — recovering `CONNECTION_CLOSED` there would be dead code, see the blueprint comment). Chromium also recovers `CONNECTION_CLOSED` for the tab-switch reconnect window. `REQUEST_TIMEOUT` stays unrecoverable (double-execution risk). A reachability test proves the retry actually fires for a RUNNING node. **3. Structured not-connected results** (`48b91ffd`) — `debugger-status` and `debugger-log-registry` become factories that return `{ status: "not_connected", reason, detail, guidance }` for classified preconditions (`metro_not_running | no_app_connected | device_mismatch | cdp_unreachable | stale_connection | reconnecting`) and still throw on unexpected faults. `detail` preserves the original error text for string-matching agents. `debugger-status` gains a socket-state gate (Metro: dispose stale node; Chromium: report transient reconnect, no dispose — the client belongs to the ChromiumCdp dependency). log-registry keeps **no** gate on purpose: captured logs are readable over a dead socket, and disposal would destroy the post-crash log file. Flows/run-sequence map a not_connected result to a **failed step** (mirrors `isUnmetUiWaitResult`) so recorded connectivity gates don't silently pass. `debugger-evaluate` re-codes agent-expression throws as `DEBUGGER_EVALUATE_EXPRESSION_THREW`. **4. Telemetry + guidance + docs** (`7602a70d`) — `tool:fail.invalid_params` (schema-declared names only; unknown keys → literal `"unrecognized_keys"`; capped emit-side at 16), new `debugger:tool_outcome` event (coded enum; joins `tool:invoke`/`complete` via `tool_invocation_id` — `ai_client`/`duration` live on the joined row), anti-retry-loop guidance appended to the Metro discovery errors (first sentences verbatim — skills match on them), six skills updated, Telemetry.md bumped to v1.03 with the new coded diagnostics disclosed. ## Dashboard migration note `debugger-status` / `debugger-log-registry` precondition failures stop emitting `tool:fail` — track them via `debugger:tool_outcome` (`outcome != 'connected'`). Per-AI-client breakdowns of those outcomes need a join on `tool_invocation_id` against `tool:complete`. ## Testing - Full suites green: tool-server 3,142, telemetry 290, registry 87. New coverage: recoverable truth tables (incl. non-Error inputs), status not-connected matrix (Metro/Chromium/terminating-window/stale/dispose-side-effect/fall-through-rethrow), log-registry rows, zod `invalid_params` derivation + emission-level guard, flow gate, evaluate rewrap, sanitize-level enum checks. - Live-tested against a booted iOS simulator + RN app and an Electron target (details in PR comments). Plan hardened through 3 adversarial review rounds (46 confirmed findings addressed) before implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-10 11:51:45 +02:00
filip131311	7f85f996aa	fix(tool-server): defer to an existing server on EADDRINUSE instead of crash-looping (#533) ... ## Summary Fixes **bucket B** of the 0.16.0 crash telemetry — the `toolserver_bind` / `EADDRINUSE` restart loop. It's the largest crash **volume** (~817 events, one machine alone = 785) even though it's a narrow user set (10). Companion to #531 (which fixed the widespread multi-user bucket A). ## Root cause (reproduced live) The main HTTP listener's `server.on("error")` handler routed **every** bind error through `crashShutdown` (exit 1). When a second tool-server starts on an already-owned port, that exit just feeds a supervisor respawn → re-bind → crash, forever. The telemetry signature was puzzling: all these crashes are `crash_phase=serving` with ~1s uptime and **0 tool calls** — i.e. the listener *bound successfully* and then died. I reproduced it: on macOS, a **second tool-server process's** `listen()` momentarily **succeeds** (the `listening` callback runs → `listening=true` → phase `serving`), and **then** the server emits an asynchronous `EADDRINUSE`. So the redundant instance binds, immediately errors, crashes, and the supervisor loops it (~1s each, 785× on one machine). Live A/B on the compiled `dist` (isolated port, telemetry disabled), server A up then a second server B on the same port: ``` buggy (current 0.16.0): B: "Tools server listening ..." → "Failed to bind ... EADDRINUSE" → exit 1 (→ respawn loop) fixed: B: "Tools server listening ..." → "deferring to it (redundant instance)" → exit 0 ``` A survives in both. ## Fix Handle `EADDRINUSE` specifically instead of crash-looping: - Probe the port for a healthy argent tool-server (`GET /tools` → 200, or 401 when a token is configured). - **If a healthy peer answers**, this instance is redundant → shut down **cleanly** (`shutdown(0)`) and defer to it. This removes it from the `reason:"crash"` restart-loop population entirely. - **If nothing healthy owns the port** (a foreign holder, or a wedged peer that doesn't answer within the probe timeout) → still crash, so a genuinely stuck port stays visible. - `EACCES` and all other server errors crash exactly as before. It also corrects the now-false comment claiming these crashes are phased `startup` ("the listen callback never ran"). Telemetry and the live repro prove the bind succeeds first, so `listening=true` and the phase is genuinely `serving`. ## Testing - **New unit tests** for `probeArgentToolServer`: peer answering 200 or 401 → `true`; foreign 500, connection-refused, and wedged/timeout → `false`. - Full tool-server suite **2731 green**; `tsc` + `eslint` clean. - **Live A/B** above, on the built dist. ## Note on scope This is a robustness fix that breaks the loop for the redundant-instance case (the dominant one). The precise reason macOS lets a second process's bind succeed-then-error asynchronously (vs. failing immediately, as same-process contention does) is a kernel/loopback timing detail; the fix does not depend on it — any `EADDRINUSE`, immediate or deferred, now defers-or-crashes deterministically rather than hot-looping.	2026-07-22 16:56:54 +02:00
filip131311	2196cc18f0	feat(telemetry): anonymous crash diagnostics on toolserver:stop (#494) ... ## Why PostHog shows **~9,785 `toolserver:stop` crashes over 90 days**, but every one collapses into a single bucket — `error_kind=crash`, `error_code=TOOLSERVER_UNCAUGHT_EXCEPTION`. We know *that* the tool server died on a top-level throw, never *which* throw. The population is also two very different things we can't tell apart today: - **~99%** crash at **<5s uptime with 0 tool calls** — startup faults, and heavily concentrated per-user (top 10 users ≈ 31% of all crashes; one user logged 1,027), i.e. **boot → crash → relaunch loops**. - **~1,000** crash after **>10 min** of real serving — genuine runtime bugs. Without the actual exception, all 9,785 rows are unrankable and unfixable. ## What Four **crash-only, provably-anonymous** fields on `toolserver:stop`, enforced by the existing coded-allowlist sanitizer so **no free text can ever reach PostHog**: | field | value | why | |---|---|---| | `error_name` | error class name (`TypeError`, …) | "what kind" | | `error_syscall` | Node's `err.code` (`EADDRINUSE`, `ECONNREFUSED`, …) | the key discriminator for the startup-loop population (port/socket conflicts) | | `crash_fingerprint` | 16-hex hash of the **de-identified** top stack frames | clusters the same crash across users; input carries no identity, output is opaque | | `crash_phase` | `"startup"` \| `"serving"` | splits the two populations directly | Turns 9,785 opaque rows into a handful of rankable clusters. ## Anonymity (the hard requirement) - **No error message, no raw stack.** Those routinely embed `/Users/<name>/…`, URLs, argv, hostnames, interpolated values — the highest-PII surface and impossible to reliably scrub. The fingerprint replaces the message for clustering; `error_name` + `error_syscall` give the "what kind" without free text. - **Fingerprint input is de-identified before hashing** (defense-in-depth on top of the hash): first-party frames → basename only (drops the home-dir/project prefix), `node_modules` frames → package-relative tail (drops the user prefix; also collapses the pnpm `.pnpm/pkg@ver/` layout), `node:` internals kept verbatim, column dropped. So the *same crash on two machines produces the same fingerprint*, and no username/path survives even into the hash input. - **Sanitizer is the final gate.** `error_name` `^[A-Za-z][A-Za-z0-9_]{0,63}$`, `error_syscall` `^E[A-Z0-9_]{1,31}$`, `crash_fingerprint` `^[0-9a-f]{16}$`, `crash_phase` a 2-value enum. Any path/message/lowercase/oversize value is dropped before it can leave the machine. ## Scope & safety (no leaking crashes out of scope) - Fields are populated **only** in `crashShutdown` (which forces `reason="crash"`), so clean idle/signal stops carry none. Verified by the existing startup-telemetry test (the pre-listener watcher-rejection crash path stays the bare `{reason,uptime_ms,total_tool_calls}` shape). - The four keys exist only on `ToolserverStopProps` and only in the `toolserver:stop` allowlist entry — they can't ride any other event. - `describeCrash` is **total**: every field extraction is isolated, so a hostile throwing getter on `.name`/`.code`/`.stack` yields the phase (and whatever else is readable) instead of turning crash reporting into a second crash. The crash/exit guarantee — re-entrancy guard, force-exit timer, `finalExitCode` escalation — is untouched. ## Tests - New `crash-diagnostics.test.ts`: class-name/constructor-name/primitive extraction, syscall capture, cross-user determinism (incl. **Windows** drive-letter paths and **pnpm** layout), node_modules tail, line-sensitivity, no-stack omission, hostile-getter resilience, and "never emits a message/stack" confinement. - `sanitize.test.ts`: crash fields pass when well-formed; dropped for message-as-name, path/lowercase syscall, wrong-length/uppercase fingerprint, unknown phase; `ERR_*` accepted. - Full telemetry suite (278) + tool-server startup test green; lint/prettier clean. ## Reviewed Ran an adversarial review pass (anonymity leak-hunt, determinism, crash-path safety, scope). Ship-ready; all findings were Low/Nit and the actionable ones (hostile-getter hardening, Windows/pnpm test coverage, `error_syscall` doc clarity) are folded in. ### Deliberately deferred (documented, not blockers) - Sanitizer doesn't *cross-gate* the crash fields on `reason==="crash"` — the per-field validator design has no sibling access; coupling is enforced in emit code. The fields are non-identifying regardless, so a hypothetical stray `crash_phase` on an idle stop is a data nit, not a leak. - First-party frames reduce to basename; if `dist` ever ships unbundled with duplicate basenames, two sites at the same line could collide. Non-issue for the bundled tool-server.	2026-07-13 22:16:33 +02:00
Hubert Gancarczyk	f002fc9540	feat(telemetry): track Argent Lens design-review funnel (#478) ... ## What Adds three privacy-safe telemetry events for the Argent Lens design-review flow, capturing the human-decision funnel that the generic `tool:*` path can't see (submission is an HTTP POST, and the `await_user_selection` tool is hidden in `argent lens` CLI sessions): - **`lens:preview_opened`** — a human loaded the preview UI (`GET /preview/`), deduped per round. - **`lens:round_completed`** — the human submitted their picks (once per round, never on a resubmit). - **`lens:round_abandoned`** — a staged round was discarded before submit (window closed, `argent lens` exited mid-review, or the round superseded). ## Privacy Payloads carry only aggregate counts, booleans, durations, and the device `platform` enum — never element names, comment text, variant code, file paths, or raw device ids. The store emits privacy-safe aggregate structs that are decoupled from the telemetry package; structural assignability to the event props is enforced at the `track()` call site (drift is a compile error). ## Event fields - **`preview_opened`**: `round, element_count, variant_count, is_cli_session, platform` - **`round_completed`**: `round, element_count, variant_count, annotation_count, element_comment_count, skipped_comment_count, has_global_comment, is_cli_session, had_parked_await, round_duration_ms, platform` - **`round_abandoned`**: `round, element_count, variant_count, had_parked_await, is_cli_session, platform` ## Testing - Unit tests for all three events: per-round dedup, exactly-once completion (no resubmit re-emit), abandoned vs happy-path roll, CLI up-front open, and content-agnostic privacy assertions. - Verified end-to-end against a live tool-server: all three events fire with correct aggregate values and zero content leakage through the real sanitize pipeline. --------- Co-authored-by: Ignacy Łątka <latkaignacy@gmail.com>	2026-07-13 16:56:49 +02:00
Stanisław Chmiela	a7775e4ba6	feat(tool-server): add tool-server event log (#367) ... We want to display a nicely parsed and rendered log of events that happened over a span of an Argent session. Current log file is not designed to be machine-readable, so adding a JSONL-formatted log of events (gated by `tool-server-event-log`). I eventually want this 😋 https://x.com/j3nnings/status/2069072177584849192 When enabled, the tool server writes Bunyan-compatible records to `ARGENT_EVENT_LOG` or `~/.argent/tool-server-events.jsonl`. The log captures tool-server lifecycle events and registry lifecycle events, including tool invocations and others. To make messages nicer, added `interaction` object to tool definition. This allows tools to define a nicer message in log (print "Screenshot capture completed." instead of "Tool `screenshot` completed.") Adding these to the existing tools is a follow up PR matter I think.	2026-07-10 11:52:28 +02:00
Hubert Gancarczyk	f38f0e3876	feat(telemetry): distinguish tvOS and Android TV from iOS/Android (#477) ... ## Summary Telemetry inferred the device platform purely from device-id **shape** on the per-tool-call hot path, so a tvOS simulator (same 8-4-4-4-12 UUID as an iPhone) and an Android TV emulator (same `emulator-NNNN` serial as a phone) were both reported as `ios` / `android` — TV usage was invisible in analytics. This adds `tvos` and `android-tv` to the **telemetry** `PLATFORMS` enum and refines the inference so a TV target is attributed correctly. ## Approach - The runtime kind (`mobile` / `tv`) is already resolved and memoized in two module-level caches (`runtimeKindCache` in `ios-devices.ts`, `androidRuntimeKindCache` in `adb.ts`), warmed by the usual TV-path tools (describe, screenshot, keyboard, launch/restart-app, the streaming blueprint, tv-control). - New synchronous, **cache-only** readers (`getCachedSimulatorRuntimeKind`, `getCachedAndroidRuntimeKind`) let `refineTvPlatform` in `http.ts` map `ios → tvos` / `android → android-tv` with **no new `simctl`/`adb` round-trip** on the hot path. - The registry **device** `Platform` (`@argent/registry`) is intentionally left unchanged — a TV stays a `runtimeKind`, not its own device platform, so capability gating and dispatch remain TV-agnostic. The two new values are confined to the telemetry boundary (`tool:invoke` / `tool:complete` / `tool:fail` and child sub-tool attribution). ## Behavior notes - The **first** tool call on a device may land before the cache is warm and reports the coarse base platform (`ios`/`android`); subsequent calls, once describe/screenshot/streaming have run, report the TV variant. This is best-effort coarse telemetry by design. - For an `emulator-NNNN` slot reclaimed by a different-form-factor AVD within one tool-server lifetime, the synchronous reader can momentarily report the prior occupant's kind until an async, avdName-checked path re-probes. Documented inline; no crash and no dispatch impact. ## Verification - **Live E2E** against real booted devices (real `simctl`/`adb` → real cache → real http telemetry recording): Apple TV → `tvos` (cold-cache `ios`, warm `tvos`); iPhone → `ios`; Android phone → `android`. The `android-tv` path is symmetric and covered by unit/integration tests with real leanback-feature probing. - Unit + integration tests added: sanitize allowlist, http platform inference (cold/warm/TV/child), and cache readers. Telemetry suite 192/192; tool-server affected suites green. - `tsc` build, `typecheck:tests`, eslint (`--max-warnings 0`), and prettier all clean.	2026-07-07 16:29:22 +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
filip131311	1aaa94d118	feat(telemetry): detect cloud/remote AI-agent environment and tag every event (#418) ... ## What Adds a **`cloud_agent`** field to the base telemetry props — alongside `is_ci` — naming the **cloud/remote AI coding-agent environment** the process is running in, or `null`. Since it lives in the base props, it's attached to **every** telemetry event automatically. Crucially this is **cloud-only**: it fires for vendor-hosted / remote autonomous runs, **not** for a developer running an agent CLI locally. A laptop `cursor-agent`/`claude` session returns `null`; only a genuine remote environment resolves to a slug. (Generic cloud/CI execution stays `is_ci`; this names the specific agent.) ## How - **New `packages/telemetry/src/cloud-agent-detect.ts`** — `detectCloudAgent(env?)` returns a canonical slug or `null`. Modelled on `ci-detect.ts`: takes `env` lazily for tests, runs nothing at import time, and only ever emits a fixed allowlist slug — a raw env value is never copied into telemetry. - **`base-props.ts`** — adds `cloud_agent: CloudAgent | null` next to `is_ci`, computed once in the memoized invariant block (no per-event cost in the long-lived tool-server). ## Signals (only high-confidence remote markers; local CLI use → `null`) | Slug | Cloud marker | Basis | |---|---|---| | `claude_code` | `CLAUDE_CODE_ENVIRONMENT_KIND ∈ {byoc, anthropic_cloud}`, a `remote_*`/`claude-in-teams` entrypoint, or `CLAUDE_CODE_REMOTE_SESSION_ID` | Verified against the Claude Code binary's own internal cloud check | | `cursor` | `CURSOR_AGENT_WORKER_ID` / `CURSOR_WORKER_POOL_NAME` (worker/pool exec) | Verified `cursor-agent` reads it from env; the local CLI sets `CURSOR_AGENT` + `CURSOR_AGENT_CLI_LOCAL_MODE` instead | | `copilot` | `GITHUB_ACTIONS` + `copilot-swe-agent` actor/workflow | Confirmed actor identity (also `is_ci`) | | `replit` | `REPLIT_AGENT` | Agent-specific (the broad `REPL_ID` fires in any Repl and is **not** used) | | `devin` | `/opt/.devin` VM marker | Confirmed (`@vercel/detect-agent`) | | `jules` | `/opt/environment_summary.sh` VM marker | Confirmed (official Jules docs) | ### Deliberately excluded - **Codex cloud** — no reliable cloud-specific marker exists (`CODEX_SANDBOX` is the *local* sandbox signal; OpenAI declined an `AGENT=codex` flag in `openai/codex#13416`). Not detected rather than guessed. - **Local agent CLIs** — `CURSOR_AGENT`, `CLAUDECODE` with `entrypoint=cli`, plain `REPL_ID` workspaces: intentionally `null`. ## Privacy Reads only a fixed allowlist of env-var **names** plus two filesystem markers (`/opt/.devin`, `/opt/environment_summary.sh`); emits only a fixed slug. The filesystem checks are injectable and guarded so a permission error can never break telemetry. ## Tests - New `test/cloud-agent-detect.test.ts` — every vendor signal, plus the key **negatives** (local Claude `entrypoint=cli`, local `cursor-agent`, plain Repl `REPL_ID`, normal CI run) asserting `null`. - Updated `test/base-props.test.ts` for the renamed key. - Full telemetry suite **191 passing**, typecheck + build clean. Live check confirms this local Claude Code session resolves to `null` while a simulated `byoc`/remote env resolves to `claude_code`. --- > Scope note: started as a broad "is an agent in the loop" flag (`agent_env`); narrowed to cloud-only per review — a human using an agent locally shouldn't look like a cloud run.	2026-06-26 16:33:32 +02:00
Hubert Gancarczyk	0edefebe06	fix(telemetry): orchestator sub-tools invocations was missing ai_client and platform attributes (#400) ... ## Summary Fixes telemetry attribution for tools dispatched by orchestrators. `run-sequence`, `flow-execute`, and `flow-add-step` run their steps via `registry.invokeTool()` directly, which minted a fresh, unregistered invocation id per nested call — so the `ai_client` / `platform` the HTTP layer captured for the outer request never reached the nested gestures, and they were recorded as **anonymous** in telemetry. Remove `opt_out` event that is a leftover. ## Changes - Add an optional `recordChildInvocation` recorder to `InvokeToolOptions` / `ToolContext`, bound by the HTTP layer to the outer request's attribution. - New `invokeSubTool` helper: registers each sub-tool under a minted id inheriting the request's AI client, with the platform re-derived from the sub-tool's **own** device arg (`udid` / `device_id` / `avdName`); forwards the recorder so propagation survives arbitrary nesting (e.g. `flow-execute → run-sequence → gesture-tap`). - Route the three orchestrators' sub-dispatch through - Refactor platform inference in the HTTP layer into a shared `platformFromArgs` helper (also hardened to not throw on a malformed device arg). - Remove opt_out event leftover Why re-derive platform per child: an orchestrator like `flow-execute` has no platform of its own and a single flow can target several devices, so the child's device arg is the only correct platform source; the parent's platform is the fallback.	2026-06-24 14:03:57 +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