Whole-system audit: five real defects, 519 lines of dead code - #214
Merged
Conversation
A pass over everything rather than recent work: every API route traced to a
caller, every RTDB path to its writers and readers, every number in the
player-facing docs checked against the code that produces it.
1. CONVERTING TO A WALLET THREW THE WEEK AWAY ← the serious one
/api/guest/migrate moved Point, materials, gear, tiers, blueprints and
elixir, and NONE of the time-boxed records. A guest who spent the week
opening containers arrived at their new wallet with ZERO Vault Fragments —
losing the entire path to that week's USDT at the exact moment they
converted. Connecting a wallet is the single thing we most want a player to
do, and it was the thing that cost them the most. The login streak, today's
contracts and the weekly item claims went the same way.
Now lib/server/guestMigrate.ts, with a rule per record rather than one
blanket policy, because the records mean different things:
fragments ADD two part-filled bars are one fuller bar
claims FILL so the week's one-per-wallet cap survives and no second
Old Paper is granted; a wallet that claimed keeps its own
contracts MAX summing would pay a free contract for converting
streak MAX plus the bigger best-ever, which is a trophy not a balance
2. THE SEASON PAYOUT COMMAND I GENERATED DID NOT RUN
payoutCommands() emitted season-deposit without --token, and resolveToken()
in deposit-reward.js DIES rather than defaulting — so the second line handed
to the owner failed the moment it was pasted. The entire point of preparing
automatically is that paying is a paste, and my own comment said as much.
It survived because the test checked the publish command's flags against the
CLI and simply did not check the other one. Both are checked now.
3. game-mechanics.md said Common items cannot be burned. They can, for 1-5
Point — nothing in the engine, the Rewards screen or the burn route
filters them.
4. Two docs disagreed on the season token: §3 said USDm, OWNER-RUNBOOK.md and
rewards-system.md say USDT, and the runbook is what gets executed. §3
corrected; the token is now a named constant the command reads.
5. OWNER-RUNBOOK.md still said "work out the top 3 yourself" — I built the
automation and never told the document the owner actually follows.
DELETED, all verified unreferenced: USDmDisplay, GameUI, TokenBalanceWidget,
CustomCursor, LiveStatsTicker, SectionDivider, useScrollReveal, and
/api/vault/status whose only caller had already gone. 519 lines. Lint
baseline 34 -> 33.
VERIFIED CORRECT: the money path connects end to end (fragments ->
paperClaims/goldenKeyClaims -> the records /api/vault/submit gates on); every
number in game-mechanics.md matches its source, including rarity odds that
compute to 58.8/26.5/10.6/3.5/0.6 against a documented 59/26/11/3.5/0.6.
KNOWN AND LEFT ALONE, both written down in §9b: energy refills on a rolling
24h while contracts and the streak reset at 00:00 UTC — two meanings of "a
day" on one status bar; and nothing prunes the daily/weekly buckets, which is
harmless for correctness and about 36 MB a year at 1,000 daily players.
npm run test:migrate (14) and an extended test:season (45).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017A764RdnwpyWnG7uCNhMiQ
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A pass over everything, not just recent work: every API route traced to a caller, every RTDB path to its writers and readers, every number in the player-facing docs checked against the code that produces it.
No smart contract, ABI, or contract address is touched.
1. Converting to a wallet threw the week away — the serious one
/api/guest/migratemoved Point, materials, gear, tiers, blueprints and elixir. It moved none of the time-boxed records:vaultFragments/{week}paperClaims/goldenKeyClaimsdailyContracts/{day}loginStreakA guest who spent the week opening containers arrived at their new wallet with zero Vault Fragments — losing the entire path to that week's USDT at the exact moment they converted.
Connecting a wallet is the single thing we most want a player to do, and it was the thing that cost them the most.
Now in
lib/server/guestMigrate.ts, with a rule per record rather than one blanket policy, because the records mean different things:2. The season payout command I generated did not run
payoutCommands()emittedseason-depositwithout--token, andresolveToken()indeposit-reward.jsdies rather than defaulting. The second line handed to the owner failed the moment it was pasted.The entire point of preparing automatically is that paying is a paste — my own comment in that file says so. It survived because the test checked the publish command's flags against the CLI and simply did not check the other one. Both are checked now, against the script's own argument validation.
3–5. Documentation that had drifted from the code
game-mechanics.mdsaid Common items cannot be burned. They can, for 1–5 Point — nothing in the engine, the Rewards screen or the burn route filters them.OWNER-RUNBOOK.mdandrewards-system.mdsay USDT, and the runbook is what gets executed. §3 corrected; the token is now a named constant the generated command reads.OWNER-RUNBOOK.mdstill said "work out the top 3 yourself." I built the automation last PR and never told the document the owner actually follows. Rewritten around/api/season/status, the/statsindicator and the mark-paid call.Deleted — 519 lines, all verified unreferenced
USDmDisplay·GameUI·TokenBalanceWidget·CustomCursor·LiveStatsTicker·SectionDivider·useScrollReveal·/api/vault/status(its only caller had already been removed).Lint baseline 34 → 33 — the dead code was carrying an error.
Verified correct (no change needed)
paperClaims/goldenKeyClaims→ the exact records/api/vault/submitgates on.game-mechanics.mdmatches its source. Rarity odds compute to 58.8 / 26.5 / 10.6 / 3.5 / 0.6 against a documented 59 / 26 / 11 / 3.5 / 0.6; burn ranges, fragment thresholds, the streak ladder, the bunker table and 5 free runs/day all agree.firebaseAuth.tsreads as unreferenced to a naive grep but is loaded viaawait import()in three places — a false positive in my own scan, worth naming so nobody deletes it.Known, left alone, written down (§9b)
Verification
npm run test:migrate(new)npm run test:seasontest:play·test:raid·test:contracts·test:floors·test:audio·test:streak-ui·test:bunkers-ui·test:season-routestest:streak·test:seeded·test:bunkers·test:inventorytest:traversableaudit·check:copy·check:market·check:cssvars·check:attributiontsc --noEmitlintbuildGenerated by Claude Code