mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
Michał Pierzchała
ef2094c9d7
* feat(size): measure a base ref in one command (pnpm size --base <ref>) The Size workflow already compares base and PR builds; locally that needed a manual checkout, install, build, --json, and --compare dance, so budgets were negotiated late. --base <ref> does the workflow's recipe in a detached worktree under .tmp/size-base/<sha> (kept for reuse, other bases pruned) and compares against it: first run ~1-2 min, later runs against the same base ~3s. Documents the local caveat: npm tarball/unpacked rows compare a fresh base against a working tree that may carry locally built helper artifacts. * feat(tooling): pnpm pr:evidence — one paste-ready, SHA-stamped evidence block for PR bodies Composes what the repo already measures instead of hand-transcribing it after every rebase: exact merge-base and head, changed-file areas, the affected selector's plan (local vs GitHub-authoritative, fail-open summarized), the layering guard verdict, depgraph counts with a real delta against the base (a throwaway git worktree, no install — the script analyzes its cwd while its imports resolve from this checkout), and, behind flags, the changed-line coverage table and pnpm size --base. It claims nothing about CI: the last line links the head's checks. ~20s default tier. The pure model (grouping, report parsing, rendering) has node:test coverage registered as the pr-evidence-model gate, run in the Affected-check Selector job next to the selector it reads. * fix(tooling): pr:evidence measures pristine head/base worktrees from an os.tmpdir scratch; size --base gets a per-SHA lock, completion stamp, and non-destructive eviction Review (three P1s): - pr:evidence created its scratch under an untracked .tmp/ that a fresh checkout lacks (ENOENT). Scratch now lives under os.tmpdir(), which exists by construction; a real entrypoint regression runs the whole pipeline with --base HEAD (no origin/main needed) and asserts JSON shape plus cleanup of both worktrees and the scratch. - Untracked or uncommitted production files could move the layering/depgraph numbers the block labels as HEAD's. Head is now measured from a pristine worktree of the head commit exactly like base, and the affected plan takes the head SHA (the literal HEAD folds the working tree in). The dirty flag now counts untracked files and says they are not in the block. - size --base force-pruned other cached bases without locking and trusted a dist/src that could be half-built. Per-SHA .lock (pid, O_EXCL) held from before the worktree exists until the base report is read; a live lock on the same base fails fast, a stale one is replaced; eviction skips worktrees whose lock owner is alive; dist/.size-base-complete marks a finished build. Orchestration tests run the real script against a throwaway git repo with pnpm/npm shimmed on PATH (build once, reuse, live lock, stale lock, interrupted build, guarded vs idle eviction). Also fixes the /tmp → /private/tmp realpath mismatch those tests surfaced (git lists worktrees by real path, so the registration check removed a live worktree). * fix(tooling): symlink-identity locks with compare-then-unlink; evict under the victim's lock; pr:evidence registers worktrees on add and cleans up exhaustively Review (three P1s): - Lock creation/takeover races: the lock is now a symlink whose target is the owner identity (pid:nonce), created with its identity in one syscall (no empty-file window), taken over only by compare-then-unlink on the exact identity judged stale, and verified after creation; release unlinks only a link that still names this run. Real overlapping-process tests: two runs on one base (exactly one builds, the other fails fast), and a takeover race against a simulated other taker across delays straddling the acquire window (a live lock is never unlinked, both never proceed). - Cross-base eviction: a victim is removed only while holding its own lock, acquired through the same path, so a run wanting it after the check finds it locked rather than half-removed; a live-locked victim is skipped. - pr:evidence worktrees: withWorktrees registers each worktree the moment its add succeeds and sweeps every resource on the way out, collecting failures instead of stopping at the first; planted reds for both (second add fails → first removed; removal of the middle one throws → the others still go). * test(size): serialize the size --base orchestration file with the other real spawners Caught running the full unit suite on the rebased branch: the file passed in isolation but intermittently failed under broad file parallelism, where it took 14s versus ~5.5s alone. It spawns node scripts/size-report.mjs per case, which spawns git and the shimmed package managers under it — the SUBPROCESS_STUB_TESTS class exactly (starved spawns surface as a vitest test timeout instead of the orchestration assertion the case is about), so it joins that serialized project with its spawn named at the entry, per docs/agents/testing.md. No rerun layer is involved: the flake is removed, not retried. Two full-suite runs green after. * refactor(size): extract the base-cache claim protocol and make stale takeover atomic Review (P1 + architecture): Stale-claim removal was compare-then-unlink (readlink then unlink; lstat then rm for a stray file), so another taker could replace the observed entry with its live claim between the two syscalls and this run would delete the replacement. Removal now happens only while holding the entry's takeover mutex — an atomically created directory — and re-verifies the claim inside it. A replacement can appear only by creating one on a free path (the abandoned claim occupies it until the unlink) or by another takeover (needs the mutex), so removal cannot delete a replacement. A mutex leaked by a process killed inside its sub-millisecond critical section is reclaimed by age, and even a wrong reclamation is contained: both takers re-verify inside, and the winner is still decided by the atomic symlink() that follows. The protocol moves out of size-report.mjs into scripts/size-base-cache.mjs (AGENTS.md: extract past 500 LOC) — 719 → 536, with the entry lifecycle (claim → evict others → ensure worktree → build if unstamped → measure → release) owned by the module behind withPreparedBaseWorktree. Mirrored tests in scripts/__tests__/size-base-cache.test.ts plant every dangerous interleaving directly on the filesystem: replacement-after-observation, a takeover held by another run, age reclamation, release-after-retarget, and a stray non-symlink. They need no subprocess and run in 9ms, so the raced single-process case was dropped from the orchestration file, which keeps only what real processes can show. Planted red: removing the mutex makes the contended case delete the claim it must not touch. * ci(size): preserve the reporter's whole module graph, and gate that it stays whole The Size workflow measures the base commit with the PR's reporter, so it copies the reporter out of the tree before checking the base out. Extracting size-base-cache.mjs made the reporter a two-file graph while the step still copied one file, and the base measurement died with ERR_MODULE_NOT_FOUND — after every deterministic gate had passed, because nothing local reproduces that copy. The step now copies the scripts directory, so a further split cannot leave an import behind, and size-report-preserved-closure.test.ts holds it to the reporter's real relative-import closure and to running the preserved copy rather than the checked-out tree. Planted red: restoring the single-file copy fails both cases, naming scripts/size-base-cache.mjs. Verified by running the reporter from a copied directory exactly as the workflow does. * fix(size): the takeover mutex has one holder for life; split report publishing out of the reporter Review (P1 + architecture): Age-based reclamation of the takeover mutex reintroduced the split ownership the mutex exists to prevent: a holder that is merely slow — paused or SIGSTOPed past any threshold — could have its mutex force-removed and replaced, putting two takers inside the supposedly exclusive section, where either could unlink the claim the other had just created; the unconditional pathname-based release could also delete the replacement mutex. The mutex is now a symlink naming its holder, created in one syscall, never reclaimed at any age, and released only by the run that owns it. A mutex leaked by a process killed inside a three-syscall critical section wedges one cache entry with the path to clear in the message, rather than silently deleting another run's live claim. Planted red: restoring age reclamation displaces a day-old delayed holder, which the new case pins. Publishing the report to a PR is a separate question from measuring and formatting it, so it moves to scripts/size-report-comment.mjs with the marker and retry policy it owns; its existing regression drives it through the real script unchanged. scripts/size-report.mjs is 386 LOC — under the 500 tripwire and below the 512 it had on base. * test: prove delayed size cache holder is preserved * docs: keep size review in CI