Skip to content

fix(contacts): photo recruit review fixes + clearable rival banner (#184 follow-up) - #185

Merged
BrandDead merged 4 commits into
main-tL2525from
fix/184-photo-creator-review
Sep 30, 2026
Merged

BrandDead merged 4 commits into
main-tL2525from
fix/184-photo-creator-review

Conversation

@BrandDead

@BrandDead BrandDead commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

Follow-up to #184 (stacked on feature/183-personal-crew-contacts). It fixes the confirmed review findings in the photo-recruit flow, plus a pre-existing rival-banner bug the browser playtest found blocking the Contacts header on phones.

Merge order: #180 → #181 → #182 → #184 → this PR.

What changed

Area Problem Fix
Generate handleGenerate left memberName out of its dependencies. Typing the name last, as the on-screen order suggests, made Generate do nothing (Codex P1). Added the dependency; the name check now uses one shared helper.
Polling The preview-URL cleanup effect also cleared the poll interval, so replacing the photo mid-generation left the job stuck on "Generating" (Codex P2). Split into two effects: preview URLs are revoked when replaced or on unmount; polling stops only on unmount, finish, or a new job.
Approve The name wasn't re-checked on approve, so clearing it after generation added a nameless member (Codex P2). Approve re-checks the name, and Add to Roster is disabled while the name is invalid.
Double tap Two quick taps called approve twice and added the member twice. A synchronous in-flight guard, plus an id check against the roster.
Member record addMember({...} as any) created members with no stats, morale, gangId, experience or health. A typed buildPhotoMember() fills a complete GangMember with the same neutral starting line as the other recruit paths.
Fallback URLs The status-poll fallback replaced the asset outright and could erase generated URLs. Only defined fields from status/approve responses are merged.
Rival banner GhostThreatBanner kept one dismissedId. With two or more open threats, dismissing one brought the other back, so the banner could never be cleared. On phones it covered Contacts' Back, + Add, and tabs. This predates #180 (already on main). ✕ now clears every threat that is currently open; newer threats still appear.

Validation

  • Regression tests first: the four new PhotoMemberCreator tests all fail on the feat(contacts): personal custom crew recruiting for single-player launch (#183) #184 head for the expected reasons and pass here. The new banner test fails on the old logic and passes here.
  • npm run validate: 82 files, 979 passed / 4 skipped, lint 0 errors / 200 warnings (the baseline; feat(contacts): personal custom crew recruiting for single-player launch (#183) #184 had 201 because of the missing dependency).
  • npm run build: passes.
  • Browser playtest (Playwright, VITE_DEMO_MODE=1, backend offline, fictional test image), at 390×844 and 1440×900:
    • Real CONTACTS icon → + Add → Create from Photo → name typed last → Generate → Add to Roster. Nova appears in the Active list, saved once as a complete shooter record (morale 75, stats, gangId: demo-gang, contact avatar).
    • PHONE → Add Custom Member → Kilo appears in the Phone crew list, and Nova is still there after a reload.
    • One ✕ clears the rival banner; before the fix, 12 taps didn't. 0 page errors.

Still open (not in this PR)

  1. Duplicate demo crew in Contacts. useGangStore.removeMember drops the member but leaves its contact card, and demoSeed runs remove-then-add on every load. A fresh demo shows "ACTIVE (9)" with duplicate Dre/Rome/lookout cards and React duplicate-key warnings. The fix belongs in gameStore.ts, which the upcoming heat/territory rewiring is likely to touch, so I left it for whoever owns that file.
  2. Offline avatar fallback. When the backend isn't reachable, the avatar service quietly returns stock portraits instead of art from the uploaded photo. Players should be told.
  3. Content boundary. Real-person photo recruits vs the fictional-content rule in docs/AI_CONTRIBUTOR_START_HERE.md needs an owner decision (Codex P1 on feat(contacts): personal custom crew recruiting for single-player launch (#183) #184).
  4. Custom recruits can't yet be put on the Strip, which still draws its crew from blockLoopFixture.

Coordination note

Another agent is planning ecosystem changes: one connected heat, a core-loop-only home screen, Ghost Crews only, and Drive-By as the flagship fight. This PR only touches PhotoMemberCreator.tsx, GhostThreatBanner.tsx, their tests, and docs/PROJECT_LOG.md. The open stack (#180–#182) touches OSShell.tsx, blockLoopStore.ts, ghostCrewStore.ts, CityBriefing*, UnifiedEncounter.tsx, prepareEncounter.ts and threatHandoff.ts. Work in those areas should branch from the top of the merged stack.

Refs #183, #184.


Note

Low Risk
Presentation-only Contacts backstory branching and local banner dismiss state; no auth, persistence, or gameplay authority changes in this diff.

Overview
Contacts contact-detail Backstory now supports photo recruits’ string backstory (e.g. “Joined the crew from your Contacts.”) while keeping the legacy origin/reason object layout. A focused Contacts regression opens a photo recruit profile and asserts that text is visible.

GhostThreatBanner no longer tracks a single dismissed event id. Dismiss (✕) adds every currently open attack/claim to a dismissed set so stacked threats don’t reappear one-by-one and block the phone Contacts header; newer feed events can still surface. A GhostThreatBanner test covers dismiss-all and a later threat.

docs/PROJECT_LOG.md records the #185 backstory fix, #184 banner behavior, and integration notes.

Reviewed by Cursor Bugbot for commit fba73bf. Bugbot is set up for automated code reviews on this repo. Configure here.

BrandDead added 2 commits September 29, 2026 21:35
- Generate reads the current name (was a stale closure when typed last)
- Replacing the photo no longer cancels a queued job's polling
- Add to Roster revalidates the name and guards against double taps
- Photo recruits are complete GangMember records (no 'as any')
- Status/approve fallbacks no longer erase generated image URLs

Regression tests fail on #184 and pass here. Refs #183, #184.
A single dismissedId brought the previous alert back as soon as the next
was dismissed, so with two or more open threats the banner never cleared
and covered the Contacts header on phones. Newer threats still show.
Pre-existing on main. Refs #183.
@vercel

vercel Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
slide Ready Ready Preview Sep 30, 2026 6:32pm UTC

Request Review

@cursor

cursor Bot commented Sep 29, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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: serverGenReqId_d96ef001-44cc-4131-891d-0044efafc975)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T21:40:30.518207Z 89e6242 PR opened
🔒 Security Review ✅ Completed 2026-09-29T21:38:49.178717Z 89e6242 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 89e6242636

ℹ️ 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".

Comment thread frontend/src/components/gang/PhotoMemberCreator.tsx
@cursor

cursor Bot commented Sep 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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: serverGenReqId_702fd96c-712c-4f15-8a54-c934831b8f5a)

BrandDead commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner Author

Addressed Codex review 4138611208 in b8af027. The typed GangMember.backstory remains a string; Contact Detail now renders that string visibly while preserving the existing structured-object path.

Evidence:

  • New focused UI regression fails before the change and passes after: Contacts roster card → profile shows ‘Joined the crew from your Contacts.’
  • Full release preflight: 83 files, 980 passed / 4 skipped; asset audit and package validation pass; production build passes.
  • Browser path: fictional image → Contacts → Create from Photo → Add to Roster → Nova profile at 390×844 and 1440×900: backstory visible, 0 page errors. The 24 duplicate-key warnings match the unmodified fix(contacts): photo recruit review fixes + clearable rival banner (#184 follow-up) #185 base and are the separately logged demo-seed/contact duplication issue.

No roster/store/data-model, backend, Supabase, deployment, or gameplay changes.

BrandDead added a commit that referenced this pull request Sep 30, 2026
Carry the tested #185 creator repairs before merging the personal-crew predecessor. Preserve complete recruit records and URLs, label manifest-backed stock previews, and document the authorized cosmetic-photo consent boundary. Integrate reviewed #182 and preserve log history under #188.
@BrandDead
BrandDead changed the base branch from feature/183-personal-crew-contacts to main-tL2525 September 30, 2026 18:29
Preserve verified creator/fallback repairs, retaliation warnings and dismiss-all behavior while retaining typed string/legacy object backstories in Contacts. Combined regression, asset and player-path gates under #188; authenticated saved-game proof remains #175.
@cursor

cursor Bot commented Sep 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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: serverGenReqId_722d5249-bf0f-4030-a885-fee04b3e9b27)

This branch was successfully deployed

1 active deployment
Preview — fba73bf3 Deployed Sep 30, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant