Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: af4506e2-7778-47fb-b430-a40a1cb0a2e8) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9783ee8e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!member || member.status !== 'active') { | ||
| return reject('Choose an active crew member to receive this item.'); |
There was a problem hiding this comment.
Handle rejected transfers in Contacts
When npcRetaliationEngine marks a dealer injured, only the GangMember status changes while its Contact remains active, so Contacts.tsx still exposes “Equip from Stash.” This new status check returns false, but the caller at Contacts.tsx:591-592 ignores the result and unconditionally announces “Product Equipped”; the inventory remains unchanged while the player sees success. Check the returned boolean in that caller and only emit success when the transfer commits.
Useful? React with 👍 / 👎.
| if (!member || member.status !== 'active') { | ||
| return reject('Choose an active crew member to receive this item.'); |
There was a problem hiding this comment.
Align the transfer check with Trap's deployed recipients
For persisted crew records whose status is deployed, Trap explicitly includes the member in its selectable recipient list (TrapApp.tsx:111-112), but this new check rejects every status other than active. Selecting such a displayed member therefore always produces an error, whereas the previous transfer accepted the recipient. Either accept the legacy deployed status here or stop presenting those members as valid targets.
Useful? React with 👍 / 👎.
Player outcome
TRAP → choose a product/armor stack → choose an active eligible member → GIVE now transfers recorded items safely, reports actual success or visible retryable denial, and keeps matching demo-owned carried inventory after a real reload. It no longer throws the browser
require is not definederror, inflates inventory through invalid quantities, strips metadata, consumes the wrong same-item batch, or submits one selected give twice on same-frame taps.Refs #208. Local repair only; #175 remains the authenticated saved-game/external-beta gate.
Scope and existing authority
Reserved before coding:
gameStoretransfer action/type/imports;TrapAppgive/key/feedback; narrowly scopeddemoSeedcarried-inventory preservation; three new regression files; one append-only project-log entry and versioned evidence. No new economy/combat store or authority. Optional source-stack ID preserves three-argument caller compatibility. Existing role rules remain in force; unclassified legacyweaponrecords remain explicitly unsupported, not arbitrarily classified for DRIVE.No runtime-file overlap with the ten open PRs audited today. No other contributor branch, Contacts/PhotoMemberCreator/identity contract, ATTACK/Nightfall/DRIVE runtime, API/auth/backend, Supabase/migration, asset/resolver, production config, scheduler, payments, protected branch or release tag changed. Preserve the concurrent PR log entries during later integration.
Regression-first and validation
Base regression and real browser path failed before implementation. Follow-up red cases proved wrong second-batch consumption, absent inline failure feedback, and lost carried inventory on demo reseed/actual reload. Final focused suite: 35 passed (29 new plus six existing adjacent seeder tests). Independent read-only reviewer: no material findings.
Full local preflight: 1,062 frontend passed / 4 existing intentional skips / 94 files, lint 0 errors / unchanged 200 baseline warnings, typecheck and production build passed, 112-asset audit 0 errors/warnings at 7.70 MB / 20 MB, five-package/four-schema validation passed, diff hygiene passed. Isolated backend: 107 passed with remote database/Supabase credentials blank. Existing bundle-size warnings remain.
A separately built non-production demo passed desktop → TRAP → real give → role denial → actual reload at 390×844, 375×812, 1440×900. Stash 50→49, exactly one retained carried unit and complete persisted record, no false success for shooter product, visible denial and reachable GIVE target. Zero uncaught page exceptions.
Evidence and remaining limits
Full verification record, raw red/green/full gates, source hashes, browser JSON, portable runner and screenshots.
This is not clean-console or launch approval: built-preview console output retains proven baseline 404s for
/favicon.svg,/assets/game_logo.png,/assets/generated/ui/icons/icon_news_v001.png(5/4/4 occurrences including reload), reported under #190. No new page exception or unexpected network/console failure was accepted. These missing files/references are unchanged from protected main.Contacts still ignores the transfer result and can show its old unconditional success message on rejection; that overlapping file needs a separate coordinated follow-up. The inventory is still the existing local store, not an authenticated server-authoritative economy. Two-user Auth/RLS/direct-write-denial, canonical Flask exact-once receipt and second-session proof remain #175; migration 007 is already applied and must not be replayed. Do not infer live PvP readiness or deploy/merge from this PR alone.
Integration and rollback
Source/test SHA-256 values match the tested candidate. Integration owner should validate the combined candidate and preserve all append-only log entries. Revert this focused change through a reviewed revert PR if needed; no schema or migration rollback is required. Nothing merged or deployed to production in this task.
Note
Medium Risk
Changes local economy/inventory mutation paths and demo persistence; mistakes could duplicate or lose items, though scope is frontend-only with heavy regression coverage and no API/schema changes.
Overview
Repairs crew stash → member transfers for TRAP and the economy store:
transferItemToMembernow uses ESM role checks, returns boolean success/failure, rejects invalid quantities and inactive recipients, optionally targets a specific stash stack, copies full item metadata with new carried IDs, and blocks legacy unclassified weapons instead of using browser-breakingrequire.TRAP only celebrates successful gives, shows an inline role denial alert, guards against double same-frame GIVE taps, and keys/selects stash rows by stack identity when multiple batches share an
itemId. Demo reseed keeps carried inventory across reload for the demo player and matching demo gang only.Adds regression tests plus a project-log entry and verification evidence (browser runner, gate logs); Contacts’ ignored transfer result is explicitly left for a follow-up.
Reviewed by Cursor Bugbot for commit e9783ee. Bugbot is set up for automated code reviews on this repo. Configure here.