A pocket that holds one of each: per-kind grenades, and the rule slot 4 draws by #142

Approved. This is the refactor #130 asked for, it is player-invisible today, and it holds up under a real run. (This forge's review API only takes an anchored comment, so the verdict is here, on the line the one substantive note is about. An earlier probe of mine left a stray comment reading p on world.ts:447 that the API gives me no way to delete — ignore it.)

What I checked at d61fe6d

• tsc --noEmit && bun test on the head tree: types clean, 571 pass / 0 fail across 58 files. (The pull body says 544 — a stale count, not a problem.)
• Drove the world myself rather than trusting the suite: buy an HE gives grenadeCount 1 and grenadeSlot "he", the second buy is refused, and 400 s of ticks through five rounds and half time leaves every player's pocket {} — so clearGrenades works on the one path that has no test of its own.
• Turning a number into an object is the real risk in a diff like this, and it is clean: at the head, grep finds no call site left outside world.ts, hud.ts and their tests. bots.ts, observe.ts, report.ts, summary.ts and the server never touched it, observe.ts's 32 inputs are untouched so the shipped policy's numbers still mean what they meant, and no player object is shallow-copied anywhere, so one player's pocket can never alias another's.
• throwGrenade reading p.weapon as the kind is safe: shoot only reaches it when currentWeapon(p).kind === "grenade", and takeGrenade returns false without writing when the count is 0. main.ts passing a null from grenadeSlot lands exactly where the old hardcoded "he" was refused by holds.
• Security: nothing server-side is in this diff — no identity, no request body, no rate limit, no SQL, no escaping. I booted this exact head anyway: / answers 200 in 7 ms with the floor intact (CSP, X-Content-Type-Options, Referrer-Policy, HttpOnly SameSite=Lax session). No new dependency, no dead code, no TODO, and each new export names one concept.

Notes, none of them blocking

1. This line. The issue's first box asks for "the total ceiling stated as a constant"; this makes MAX_GRENADES explicitly per-kind and adds no cap on the sum of kinds. Invisible with one kind, and it matches the pull's title — but it is a deliberate deviation from what was asked and the "Left out" paragraph does not mention it. Say it there, and add the total when #37 lands.
2. "a second kind is refused too: MAX_GRENADES caps every kind, not just HE" only exercises HE — it sets { he: MAX_GRENADES } and proves nothing the test above it does not. There is no second kind to test yet, so name it for what it checks.
3. hud.ts's multi-kind join cannot be reached by a test, since grenadeKinds filters GRENADE_IDS and a placeholder id never gets in. It gets its test with the first real kind — and check that line's width at 360px then, because it grows sideways.
4. clearGrenades could be p.grenades = {}; iterating GRENADE_IDS only matters for a key addGrenade cannot write.

Preview: https://cs--142--forge-trevato-apps.open-platform.sh answers 404 not found from the edge on five tries over ten minutes — not the app's own 404. The forge's pictures at this head show the landing page rendering fine on phone and desktop, and a local boot of d61fe6d is healthy, so I read that as preview infrastructure rather than the change. Worth a glance once it lands.

p

Approved. All four notes from my last pass are answered, and the change holds up at f943369. (This forge's review API only takes an anchored comment, so the verdict rides here, on the line note 1 was about.)

The notes, closed

1. The per-kind ceiling now says out loud that there is deliberately no total cap, and that #37 makes that call when a second kind lands — better placed in the code than in the pull body, where I asked for it.
2. The redundant test is renamed to what it actually checks: the cap is read off the count, not off having bought before.
3. The untestable HUD join carries its reason and the 360px warning for whoever adds the second kind.
4. clearGrenades is p.grenades = {}.

What I checked at this head

• tsc --noEmit && bun test on the branch tree: types clean, 618 pass / 0 fail across 61 files.
• The branch is three commits behind main (#137 and #140 landed after its merge point), so I did the merge myself rather than trust mergeable: true: git merge-tree against 3162f4b gives a clean tree, and the full check on that tree is 649 pass / 0 fail, types clean. Nothing in #140's saved.ts reaches the pocket — SavedPlayer carries no grenades field — so the type change cannot touch the snapshot schema.
• The branch's own diff against its merge base is exactly the six files listed and nothing else; merging main in smuggled nothing.
• Drove the world again: buying gives grenadeCount 1 and grenadeSlot "he", the second buy is refused, and 400 s across five rounds and half time leaves every pocket {} — so the simplified clearGrenades still fires on the one path that has no test of its own.
• The object-for-number swap is still clean: no p.grenades access survives outside world.ts's helpers; bots.ts, observe.ts, plan.ts, saved.ts and the server never touch it, observe.ts's 32 policy inputs are untouched, and no player object is shallow-copied anywhere, so one player's pocket cannot alias another's. throwGrenade reading p.weapon as the kind stays safe: shoot only reaches it when the held weapon is a grenade, and takeGrenade returns false without writing at zero.
• Security: nothing server-side is in this diff — no identity, no request body, no rate limit, no SQL, no escaping. Booted the head anyway: / answers 200 in 15 ms with the floor intact (CSP, X-Content-Type-Options, Referrer-Policy, HttpOnly SameSite=Lax session). No new dependency, no dead code, no TODO.

Two small things, neither worth another round

• The pull body still says "544 tests (4 new)", and its "Left out" paragraph still does not mention the absent total cap. The code says both correctly now; the description is just stale.
• The preview at https://cs--142--forge-trevato-apps.open-platform.sh still answers 404 not found from the edge — that is not the app's own 404. The forge's pictures at this head render the landing page fine on phone and desktop (now with the Map select from #136), and a local boot of f943369 is healthy, so I read this as preview infrastructure rather than the change. Worth one look after it lands.

Merge it.

Approved, and nothing has changed that would move that. (The forge's review API only takes an anchored comment, so the verdict rides here on the per-kind ceiling.)

e611e80 is a merge of main and nothing else: the branch's diff against its merge base is byte-identical to the one I approved at f943369 — the same six files, the same 161/38 lines. #137 and #140 came in with the merge; the pocket is untouched by them.

Re-checked at this head anyway

• tsc --noEmit && bun test on the head tree: types clean, 649 pass / 0 fail across 62 files.
• The branch still trails main by one (#145 landed after its merge point), so I merged it myself rather than trust mergeable: true: git merge-tree against 83014ee gives a clean tree, and the full check on that tree is 664 pass / 0 fail, types clean. That is the number that matters for landing.
• Behaviour probe again: buying gives grenadeCount 1 and grenadeSlot "he", the second buy is refused, and 400 s across five rounds and half time leaves every pocket {}.
• Booted e611e80: / answers 200 in 15 ms with all three floor headers and the HttpOnly SameSite=Lax session. Nothing server-side is in the diff — no identity, no request body, no rate limit, no SQL, no escaping — and no new dependency.

Two notes for whoever merges, neither blocking

• The phone picture still shows the leaderboard's wins column running off 360px. That is not this pull: #145 fixed it on main after this branch's merge base, so the fix arrives with the merge rather than being missing from it.
• The preview at https://cs--142--forge-trevato-apps.open-platform.sh has answered 404 not found from the edge at every head I have reviewed, across three rounds and some fifteen attempts — that is not the app's own 404, and the forge's own pictures at each head render fine. A local boot of every head has been healthy, so I read it as preview infrastructure rather than the change; it is worth someone chasing separately, because a pull whose preview never answers cannot be tried by hand by anyone.

Merge it.