2 Commits

Author	SHA1	Message	Date
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
Ignacy Łątka	cd4190b80e	Full tgz-only E2E harness (scripts/e2e-full) (#495) ... Now verified end-to-end on the real Mac: install/introspection/validation, the android tier (against a booted emulator), and the chromium tier all pass from a locally packed tgz; the rn tier auto-skips without `~/dev/bluesky` as designed. ## What this is A release-gating end-to-end harness under `scripts/e2e-full/` that starts from **only a `swmansion-argent-*.tgz` bundle** and exercises the whole product: the install flow, every CLI command, all 70 tools' argument validation, and a happy-path run of every tool against real devices. Intended to be run on the real Linux box and the real Mac before a release. Everything runs under a throwaway `HOME` + npm prefix, so it never touches the machine's real `~/.argent`, editor MCP configs, or global packages (safe on a shared box). It emits a JSONL log + a markdown coverage report and exits non-zero if any hard assertion fails. ## Phases | phase | needs | covers | |---|---|---| | `install` | npm + network | `npm i -g <tgz>`, bundled binaries, `init` (global + `--local`), `update`, `uninstall`, telemetry, MCP-config generation | | `introspection` | — | `--version/--help`, `tools describe` for **all 70** tools, feature flags, `server start/status/logs/stop`, `link/unlink` | | `validation` | — | per tool: missing-required / bad-enum / bad-type rejection (schema-driven, no hardware) | | `android` | Android emulator | happy-path of every touch/gesture/screenshot/app-lifecycle tool | | `chromium` | Electron (bundled optional dep) + a display | boots a generated Electron app; drives CDP tools (scroll/drag/tabs/cookies/storage) | | `rn` | `~/dev/bluesky` + Android device | debugger + react/native profiler + network chain against the real Bluesky app | Tiers auto-skip with a recorded reason when prerequisites are missing. iOS / tvOS / Vega are intentionally out of scope for now. ## What I actually verified (real runs, Linux) - **install** 18✓/0✗, **introspection** 85✓/0✗ (all 70 `tools describe`), **validation** 81✓/0✗, **chromium** 24✓/0✗ (real Electron app + CDP). - Holistic run: **190✓ / 0✗ / 34∼, exit 0.** Failure-gate confirmed (non-zero on fail), `bash -n` clean on all files, all 72 android/rn JSON arg literals linted. ## What I did NOT verify live (please scrutinize) - **`android` and `rn` tiers were never run against a live device.** The Android emulator would not stay up on my box (memory-constrained shared machine; QEMU threads hung seconds after boot across ~7 attempts). The code is complete and uses the same machinery the `chromium` tier exercised live, but it has **not** been proven end-to-end. This is the main reason for the "not certain how well it works" caveat. - The RN tier's exact debugger/profiler arg shapes are derived from `tools describe`, not confirmed against a running Metro. ## Run it ```bash bash scripts/e2e-full/run-e2e.sh # everything applicable to this OS bash scripts/e2e-full/run-e2e.sh --skip-install --phase introspection,validation # fast offline core bash scripts/e2e-full/run-e2e.sh --phase android --android-serial emulator-5554 # inject a device ```	2026-07-20 16:43:34 +02:00