Files
software-mansion__argent/.github/workflows/publish-npm.yml
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

315 lines
15 KiB
YAML

name: Publish npm package
# One file, because npm trusted publishing registers a single workflow filename
# as the package's trusted publisher.
#
# Modes (derived from the trigger + the ref's version):
#
# push to main → canary: version derived + stamped (<base>-next.<k>),
# published under the `next` dist-tag. Gated behind the
# PUBLISH_NPM_UNIFIED repository variable.
# workflow_dispatch → publishes the ref's package.json version verbatim, on
# the dist-tag derived from that version. `dry_run`
# rehearses only this path — the canary machinery
# (stamping, --version check, collision retry) is never
# exercised by it.
#
# Trusted publishing: THIS file is registered on npmjs.com as the npm Trusted
# Publisher for @swmansion/argent, so the OIDC exchange authenticates the
# publish and no npm token is involved. Renaming or moving it breaks publishing
# until that registration is updated to match. NPM_TOKEN survives as a repo
# secret only for sync-next-tag.yml, which mutates dist-tags — OIDC covers
# publishing only.
on:
push:
branches:
- main
workflow_dispatch:
inputs:
ref:
description: "Branch, tag, or SHA to publish (e.g. v0.17.0 or v0.17.0-next.0)"
required: true
type: string
channel:
description: "Expected dist-tag; 'auto' derives it from the version (prerelease → next, stable → latest), an explicit value makes the run fail on a mismatch"
required: false
default: auto
type: choice
options:
- auto
- latest
- next
dry_run:
description: "Build and verify the bundle, but skip npm publish"
required: false
default: false
type: boolean
concurrency:
# Serialize canary publishes: the per-base counter is the next index after the
# highest published `<base>-next.<k>`, so overlapping runs would read the same
# index and collide on npm. Manual dispatches get their own lane, and an
# in-flight publish is never cancelled.
group: publish-npm-${{ github.event_name }}
cancel-in-progress: false
jobs:
publish:
runs-on: ubuntu-latest
# Unsetting PUBLISH_NPM_UNIFIED is the canary kill-switch; manual dispatches
# always run.
if: github.event_name == 'workflow_dispatch' || vars.PUBLISH_NPM_UNIFIED == '1'
permissions:
contents: read
id-token: write # npm provenance attestation + trusted-publishing OIDC exchange
steps:
- uses: actions/checkout@v7
with:
ref: ${{ github.event_name == 'workflow_dispatch' && inputs.ref || github.sha }}
- uses: actions/setup-node@v6
with:
node-version: "24"
registry-url: "https://registry.npmjs.org"
scope: "@swmansion"
# Trusted publishing needs npm >= 11.5.1; don't rely on whatever the
# runner image bundles.
- name: Ensure npm supports trusted publishing
run: |
npm install -g npm@^11.5.1
npm --version
# inputs.ref reaches the script through env, never template interpolation —
# this file is the trusted publisher, so a crafted ref name must stay data.
- name: Resolve publish mode
id: mode
env:
REF: ${{ inputs.ref }}
CHANNEL: ${{ inputs.channel }}
run: |
if [ "${{ github.event_name }}" = "push" ]; then
# argent-main is the floating main build of the signed binaries.
echo "mode=canary" >> "$GITHUB_OUTPUT"
echo "dist_tag=next" >> "$GITHUB_OUTPUT"
echo "binaries_tag=argent-main" >> "$GITHUB_OUTPUT"
else
VERSION=$(node -p "require('./packages/argent/package.json').version")
case "$VERSION" in
*-*) DERIVED=next ;;
*) DERIVED=latest ;;
esac
if [ "$CHANNEL" != "auto" ] && [ "$CHANNEL" != "$DERIVED" ]; then
echo "::error::Version '$VERSION' at ref '$REF' belongs on the '$DERIVED' dist-tag, but channel '$CHANNEL' was requested. Pick the ref matching the channel you meant (or leave channel on 'auto')." >&2
exit 1
fi
echo "Resolved version: $VERSION → dist-tag '$DERIVED'"
echo "mode=dispatch" >> "$GITHUB_OUTPUT"
echo "dist_tag=$DERIVED" >> "$GITHUB_OUTPUT"
echo "binaries_tag=argent-${REF//\//-}" >> "$GITHUB_OUTPUT"
fi
- name: Download signed simulator-server binary
run: bash scripts/download-simulator-server.sh
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Download signed native binaries (dylibs, ax-service, Android helper APK)
run: bash scripts/download-native-binaries.sh "$BINARIES_TAG"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BINARIES_TAG: ${{ steps.mode.outputs.binaries_tag }}
- name: Download Perfetto trace-processor WASM artifacts
run: bash scripts/download-trace-processor.sh "$BINARIES_TAG"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BINARIES_TAG: ${{ steps.mode.outputs.binaries_tag }}
- run: npm ci
- run: npx tsc --build
# Sequenced after `npm ci` (the checked-out lockfile still matches the
# original version at install time) and before the bundle build, which
# bakes the version into the telemetry payload as a string literal — a
# build that ran first would stamp every canary's events with the previous
# release's version.
- name: Compute and stamp canary version
if: steps.mode.outputs.mode == 'canary'
id: canary
run: |
VERSION="$(node scripts/next-canary-version.mjs --write)"
echo "Canary version: ${VERSION}"
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
- run: npm run build -w @swmansion/argent
env:
# Release-only: build-package-artifact.yml omits it so test tarballs
# stay inert. An unset secret bakes in an empty token, which disables
# telemetry rather than failing the build.
ARGENT_OTEL_INGEST_TOKEN: ${{ secrets.ARGENT_OTEL_INGEST_TOKEN }}
# darwin is always required; the Linux keys (x86_64 "linux", arm64
# "linux-arm64") and win32 stay gated until radon's artifacts for them are
# reliably present — flip the matching env var to "1" then. The inventory
# always logs every platform so a silently-missing binary shows up in the
# run.
- name: Verify per-platform simulator-server binaries
env:
ARGENT_REQUIRE_LINUX_BINARY: "0"
ARGENT_REQUIRE_WINDOWS_BINARY: "0"
run: |
set -e
missing=0
echo "simulator-server inventory:"
for plat in darwin linux linux-arm64 win32; do
bin="simulator-server"
if [[ "$plat" == "win32" ]]; then bin="simulator-server.exe"; fi
p="packages/argent/bin/${plat}/${bin}"
if [[ -x "$p" ]]; then
size=$(wc -c <"$p")
echo " ✓ ${plat}: ${p} (${size} bytes; $(file -b "$p"))"
else
echo " ✗ ${plat}: ${p} — missing"
if [[ "$plat" == "darwin" ]]; then missing=1; fi
if [[ "$plat" == linux* && "${ARGENT_REQUIRE_LINUX_BINARY}" == "1" ]]; then missing=1; fi
if [[ "$plat" == "win32" && "${ARGENT_REQUIRE_WINDOWS_BINARY}" == "1" ]]; then missing=1; fi
fi
done
[[ "$missing" == "0" ]] || { echo "::error::required simulator-server binary missing"; exit 1; }
- run: test -x packages/argent/bin/darwin/ax-service
- run: test -f packages/argent/dylibs/libNativeDevtoolsIos.dylib
- run: test -f packages/argent/dylibs/libKeyboardPatch.dylib
- run: test -f packages/argent/dylibs/libArgentInjectionBootstrap.dylib
# tvOS (Apple TV) binaries — missing ones would silently ship a package
# whose TV tooling crashes at runtime.
- run: test -x packages/argent/bin/darwin/tvos-ax-service
- run: test -x packages/argent/bin/darwin/tvos-hid-daemon
- run: test -f packages/argent/dylibs/tvos/libNativeDevtoolsIos.dylib
- run: test -f packages/argent/dylibs/tvos/libKeyboardPatch.dylib
- run: test -f packages/argent/dylibs/tvos/libArgentInjectionBootstrap.dylib
# TCP-transport (sim-remote / ios-remote) binaries — the ax-service and
# native-devtools variants the remote code path spawns/injects over a
# sim-remote-tunnelled TCP socket. bin/tcp is platform-NEUTRAL:
# axServiceBinaryPathTcp() resolves bin/tcp on every host, never a
# bin/<platform>/tcp subdir.
- run: test -x packages/argent/bin/tcp/ax-service
- run: test -f packages/argent/dylibs/tcp/libNativeDevtoolsIos.dylib
- run: test -f packages/argent/dylibs/tcp/libKeyboardPatch.dylib
- run: test -f packages/argent/dylibs/tcp/libArgentInjectionBootstrap.dylib
- run: test -f packages/argent/assets/argent.tracecfg.pbtxt
- run: test -d packages/argent/assets/queries
- run: test -f packages/argent/assets/manifest.json
- run: test -f packages/argent/assets/trace-processor/trace_processor.wasm
- run: test -f packages/argent/assets/trace-processor/engine_bundle.node.js
- run: test -f packages/argent/assets/trace-processor/engine.mjs
- run: test -f packages/argent/assets/trace-processor/LICENSE
- run: head -c4 packages/argent/assets/trace-processor/trace_processor.wasm | grep -q $'\x00asm'
- run: ls packages/argent/bin/argent-android-devtools-*.apk
# The shipped CLI reads its version from package.json at runtime, so this
# is exactly what `argent --version` prints after install.
- name: Verify argent --version reports the canary version
if: steps.mode.outputs.mode == 'canary'
run: |
REPORTED="$(node packages/argent/dist/cli.js --version)"
EXPECTED="${{ steps.canary.outputs.version }}"
echo "argent --version → ${REPORTED} (expected ${EXPECTED})"
[ "${REPORTED}" = "${EXPECTED}" ] || { echo "::error::argent --version (${REPORTED}) != published version (${EXPECTED})"; exit 1; }
# `argent --version` reads package.json at runtime, so it stays correct
# however the build is sequenced and cannot catch a stale *baked* version.
# Telemetry's cli_version is substituted into each bundle at build time, so
# it is only right while the stamp precedes the build — assert the literal
# every bundle actually carries.
- name: Verify the bundled telemetry version matches the canary version
if: steps.mode.outputs.mode == 'canary'
env:
EXPECTED: ${{ steps.canary.outputs.version }}
run: |
node -e '
const fs = require("node:fs");
const path = require("node:path");
const expected = process.env.EXPECTED;
const dir = "packages/argent/dist";
// esbuild folds the ARGENT_CLI_VERSION define into readCliVersion.
const re = /function readCliVersion\(\)\s*\{[^}]*?return\s*"([^"]*)"/s;
let checked = 0;
let failed = false;
for (const file of fs.readdirSync(dir).filter((f) => /\.(mjs|cjs)$/.test(f))) {
const m = re.exec(fs.readFileSync(path.join(dir, file), "utf8"));
// Bundles that do not carry telemetry (e.g. mcp-server) bake no version.
if (!m) continue;
checked++;
const ok = m[1] === expected;
console.log(`${ok ? "ok " : "FAIL"} ${file}: baked cli_version ${m[1]} (expected ${expected})`);
if (!ok) failed = true;
}
if (checked === 0) {
console.log(`::error::no bundle in ${dir} bakes a telemetry cli_version — the check can no longer see what ships`);
process.exit(1);
}
if (failed) {
console.log("::error::bundled telemetry cli_version != published version — the canary stamp must run before the bundle build");
process.exit(1);
}
console.log(`Verified the baked telemetry version in ${checked} bundle(s).`);
'
- run: npm pack --dry-run -w @swmansion/argent
- name: Dry run — skipping publish
if: steps.mode.outputs.mode == 'dispatch' && inputs.dry_run
run: |
VERSION=$(node -p "require('./packages/argent/package.json').version")
echo "dry_run: would have published ${VERSION} under dist-tag '${{ steps.mode.outputs.dist_tag }}'"
echo "Dry run — **not** published: \`@swmansion/argent@${VERSION}\` (dist-tag \`${{ steps.mode.outputs.dist_tag }}\`)" >> "$GITHUB_STEP_SUMMARY"
- name: Publish
if: steps.mode.outputs.mode == 'dispatch' && !inputs.dry_run
run: npm publish -w @swmansion/argent --tag "$DIST_TAG" --provenance
env:
DIST_TAG: ${{ steps.mode.outputs.dist_tag }}
# Serialization makes concurrent index collisions impossible, but npm's
# read-after-write lag can still hand a back-to-back run a stale version
# list, so retry a version-exists collision by recomputing + restamping.
# Any other publish failure is real and fails fast.
- name: Publish canary
if: steps.mode.outputs.mode == 'canary'
env:
DIST_TAG: ${{ steps.mode.outputs.dist_tag }}
run: |
for attempt in 1 2 3; do
VERSION="$(node -p "require('./packages/argent/package.json').version")"
echo "Publishing ${VERSION} (attempt ${attempt})"
if npm publish -w @swmansion/argent --tag "$DIST_TAG" --provenance; then
echo "Published ${VERSION}"
exit 0
fi
if npm view "@swmansion/argent@${VERSION}" version >/dev/null 2>&1; then
echo "::warning::${VERSION} already on npm (lost a race / stale read) — recomputing"
node scripts/next-canary-version.mjs --write
sleep 10
else
echo "::error::npm publish failed for ${VERSION} (not a version collision)"
exit 1
fi
done
echo "::error::canary publish still colliding after 3 attempts"
exit 1
- name: Publish summary
if: success() && !(steps.mode.outputs.mode == 'dispatch' && inputs.dry_run)
run: |
VERSION=$(node -p "require('./packages/argent/package.json').version")
echo "Published \`@swmansion/argent@${VERSION}\` under dist-tag \`${{ steps.mode.outputs.dist_tag }}\` (with provenance)" >> "$GITHUB_STEP_SUMMARY"
# `npm publish --tag` already tags this release. Re-pointing `next` at the
# highest published version (and repairing it if stale) is the
# "Sync 'next' dist-tag" workflow, which runs automatically once this one
# succeeds.