Files
Jason Jean 1a9c037b29 chore(repo): rebuild the review sandbox image instead of probing that it exists (#36560)
## Current Behavior

`review-pr`'s pre-flight asks whether the sandbox image exists:

```bash
test -n "$(docker images -q "$SANDBOX_IMAGE" 2>/dev/null)" && echo "image OK" || echo "image MISSING"
```

An image built from **any** older revision answers that identically. So
a capability added to the Dockerfile never reaches an image that already
exists, and nothing surfaces it — the only symptom is a review that is
slower or quietly weaker.

That is not hypothetical. The pnpm-store warming (`pnpm fetch`) landed
in `889f4cd45d` on **2026-07-31**; the local image was built
**2026-07-16**. `docker history` showed no `pnpm fetch` layer at all and
`/root/.local/share/pnpm` was 0 bytes, so the "warm store" the skill
promises had never existed on that machine. Every review in the two
weeks between downloaded ~4200 packages instead of linking them — about
**25 minutes each** — and the skill's only hint was a symptom you had to
notice yourself (*"If it is unexpectedly slow, the image predates the
warm store"*).

`setup-review-sandbox` had the same gate, plus a manual *"check the
`created` date against the Dockerfile"* that nobody does.

## Expected Behavior

Build unconditionally via a shared
`tools/review-sandbox/build-image.sh`, and let Docker's layer cache
decide what that costs. Both skills call it, so the image is kept
current by every review rather than by remembering to re-run setup.

Measured on the real image:

| situation | cost |
| --- | --- |
| nothing changed | **0.66 s** — prints `sandbox image up to date` |
| missing store layer (the bug above) | **2 m 47 s** — apt/mise layers
stayed cached |
| resulting store | **2.6 G**, matching the documented figure |

A `pnpm-lock.yaml` change re-runs `pnpm fetch`, which is the point: it
keeps the warm store matching the lockfile reviews actually install
from.

### No lock, because BuildKit already has one

`review-prs` drives up to five parallel `/review-pr` panes, so the
obvious worry is five simultaneous multi-GB builds. Measured instead of
assumed — 5 concurrent identical builds of a Dockerfile with a 20 s
step:

```
pane3 done at 21s   pane2 done at 21s   pane5 done at 22s
pane1 done at 22s   pane4 done at 22s

$ docker run --rm bktest cat /marker.txt
slow step running at 1785862632781210805      <- one line, not five
```

The step ran **once** and all five returned in ~21 s rather than 100 s.
An external lock would only duplicate that.

### Notes

- The build script writes to `tmp/review-sandbox-ctx` (gitignored) and
keeps the same minimal five-entry context — never the repo root.
- `allowed-tools` updated in both skills, or every run prompts.
- Documentation/tooling only. No product code, no tests affected.

## Related Issue(s)

Follow-up to #36557, found while running the skill against #36370.

<!-- polygraph-session-start -->
---
<p><picture><source media="(prefers-color-scheme: dark)"
srcset="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-dark.svg"><img
src="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-light.svg"
width="16" height="22" align="middle" alt="Polygraph"></picture> <a
href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Rebuild-the-review-sandbox-image-instead-of-probing-that-it-exists-c14dad6f">View
session ↗</a></p>
<!-- polygraph-session-end -->
2026-08-04 15:58:32 -04:00
..