Skip to content

fix(debates): allow requests with pending inbound challenges (GEO-3027) - #2567

Draft
jwalkingjew wants to merge 14 commits into
masterfrom
preston/geo-3027-pending-inbound-debate-requests
Draft

jwalkingjew wants to merge 14 commits into
masterfrom
preston/geo-3027-pending-inbound-debate-requests

Conversation

@jwalkingjew

@jwalkingjew jwalkingjew commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • allow a new outbound debate request while inbound requests remain pending
  • retain simultaneous inbound and outbound person challenges even though the activity response exposes only one challenge at a time
  • keep Not now snoozed through request-list/activity transitions so the inbound popup does not reopen
  • show outbound person requests immediately across People, Requests, Explore, Lobby, and Positions
  • apply one shared outbound gate in both directions: claim requests block person requests, and person requests block claim requests
  • preserve inbound requests in Received so they can still be accepted or rejected
  • centralize person-request role, expiry, and outbound resolution so all matchmaking surfaces use the same state
  • reconcile the optimistic outbound row after the server propagation window, preventing rejected requests from blocking the UI until expiry

Linear: GEO-3027

Review notes

  • rebased onto the latest master
  • removed duplicate challenge-state logic from People and Requests
  • fixed space filtering for claimless inbound and outbound requests
  • kept the PR as a draft for product review

Testing

  • 377 focused Vitest tests passed across Claims, People, Requests, Positions, activity reconciliation, coordinator popups, claim cards, and request gating
  • ESLint passed on every reviewed file
  • Prettier and git diff --check passed

Local environment note

  • Full bun run typecheck remains blocked in this worktree by existing shared dependency issues (missing AI SDK, LiveKit, Upstash, and Tiptap modules plus duplicate viem installations). None of the changed files report TypeScript errors.

@vercel

vercel Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
geogenesis Ready Ready Preview Sep 24, 2026 11:53pm UTC

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Cold-cache creation and remote rejection during an inbound challenge can leave outbound request state missing or stale.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Enables outbound debate requests alongside pending inbound challenges and centralizes challenge-state handling.

Changes:

  • Retains and displays simultaneous inbound/outbound challenges.
  • Applies shared outbound request gating across matchmaking surfaces.
  • Preserves snoozed requests and adds propagation reconciliation tests.
File Description
apps/​web/​core/​debates/​request-gate.ts Adds shared outbound-block reason.
apps/​web/​core/​debates/​request-debate-control.tsx Adds native blocked-reason title.
apps/​web/​core/​debates/​matchmaking/​use-outbound-debate-challenge.ts Centralizes challenge role and expiry resolution.
apps/​web/​core/​debates/​matchmaking/​requests-tab.tsx Shows simultaneous sent and received challenges.
apps/​web/​core/​debates/​matchmaking/​requests-tab.test.tsx Tests challenge coexistence and filtering.
apps/​web/​core/​debates/​matchmaking/​people-tab.tsx Allows requests during inbound challenges and shares mutation state.
apps/​web/​core/​debates/​matchmaking/​people-tab.test.tsx Tests outbound gating and shared mutation behavior.
apps/​web/​core/​debates/​matchmaking/​matches-list.tsx Displays outbound person challenges in Lobby.
apps/​web/​core/​debates/​matchmaking/​matches-list.test.tsx Tests Lobby challenge display and gating.
apps/​web/​core/​debates/​matchmaking/​claims-tab.tsx Displays outbound person challenges in Explore.
apps/​web/​core/​debates/​matchmaking/​claims-tab.test.tsx Tests Explore challenge display.
apps/​web/​core/​debates/​hooks.ts Retains and reconciles optimistic outbound challenges.
apps/​web/​core/​debates/​hooks-query-network.test.tsx Tests activity-cache reconciliation.
apps/​web/​core/​debates/​debate-coordinator.tsx Preserves snoozes across activity transitions.
apps/​web/​core/​debates/​debate-coordinator.test.tsx Tests popup coexistence and snoozing.
apps/​web/​core/​debates/​api.ts Adds client-retained outbound challenge state.
apps/​web/​core/​claims/​browse/​use-claim-matchup.ts Gates claim requests on outbound person challenges.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/web/core/debates/hooks.ts Outdated
Comment thread apps/web/core/debates/hooks.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Client/server clock skew can invalidate the propagation grace period and retain or discard outbound challenges incorrectly.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread apps/web/core/debates/hooks.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The propagation grace expires without triggering reconciliation, allowing rejected requests to block longer than intended.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Schedule refetch when optimistic grace period expires

apps/​web/​core/​debates/​hooks.ts:1298

The grace deadline is only evaluated when queryFn runs, but this success path no longer invalidates or schedules another read. If the gateway refetch arrives during the first 10 seconds, it retains the optimistic row and no event fires when the grace expires, so a rejected request can continue blocking until the 30-second poll (or the next focus). Schedule an activity refetch at the grace boundary.

@jwalkingjew

Copy link
Copy Markdown
Collaborator Author

Addressed Copilot review 5310312379’s Previously missed: Schedule refetch when optimistic grace period expires finding. GitHub did not create an inline thread for this finding, so I am recording the disposition here. The finding was correct: the gateway could refetch during the 10-second protection window, retain the optimistic row, and then leave it untouched until the 30-second poll. Fixed in be15302 by scheduling one activity invalidation exactly when the propagation grace expires. I audited every retention/deadline path added by this PR; this was the only grace without a boundary reconciliation. The regression test was added first and failed against the old code because the activity query was never invalidated after 10 seconds, then passed after the fix. This changes only stale/null reconciliation timing from up to 30 seconds to 10 seconds; confirmed server requests and simultaneous inbound/outbound requests remain retained.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Activity cache replacement can discard retained outbound state, and Requests can hide that state behind an unrelated query status.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Availability updates discard retained outbound challenge state

apps/​web/​core/​debates/​hooks.ts:1293

The retained outbound state can still be discarded by the availability mutation. useUpdateDebateAvailability replaces this activity cache with the wire response on success (hooks.ts:753-755), and its rollback can restore a snapshot taken before this challenge was created (hooks.ts:742-751). Because the wire shape has no outbound_challenge, toggling availability after creation—or completing an overlapping toggle—removes this field; when challenge is the simultaneous inbound request, the outbound card and gate disappear. Preserve the latest client-only outbound fields when applying either availability result, or centralize activity cache writes through the same reconciliation logic.

Medium severity Request query states hide the cancellable outbound challenge

apps/​web/​core/​debates/​matchmaking/​requests-tab.tsx:107

Although this now derives an optimistic outbound challenge independently of the request list, it is rendered inside HubQueryState, whose loading/error branches replace all children (hub-states.tsx:132-185). On a cold Requests load—or whenever the unrelated claim-request query fails—the person request is therefore hidden and cannot be cancelled here even though activity already contains it. Keep the challenge section outside the request-list query state, as the scheduled section and the other tabs’ outbound cards are.

@jwalkingjew

Copy link
Copy Markdown
Collaborator Author

Addressed both Previously missed findings from Copilot review 5310432184. GitHub did not create inline threads for either finding, so the dispositions are recorded here.

  1. Availability updates discard retained outbound challenge state — correct, fixed in 097bc8c. I enumerated both full-replacement paths: the availability success response and an error rollback to a pre-mutation snapshot. Both now overlay the latest client-only outbound_challenge and monotonic cache timestamp instead of discarding them. Separate regressions for success and rollback failed against the prior code, then passed after the fix.

  2. Request query states hide the cancellable outbound challenge — correct, fixed in 097bc8c. I checked both activity-backed directions, not just the reported outbound case. Outbound person challenges remain cancellable while the unrelated claim-request query loads, and inbound person challenges remain actionable if that query errors. The normal content state still uses one shared section renderer, so there is no duplicate Sent/Received implementation. Separate loading/outbound and error/inbound regressions failed against the prior code, then passed after the fix.

These changes preserve behavior already tested on-device; they prevent unrelated availability and claim-request transitions from hiding or erasing that state. Validation: 105 focused tests plus ESLint, Prettier, and git diff --check.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The outbound gate still permits concurrent submissions before mutations settle and adds per-card account-level timers and effects.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (1)

Comment thread apps/web/core/claims/browse/use-claim-matchup.ts Outdated
Comment thread apps/web/core/claims/browse/use-claim-matchup.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Claim-request creation releases the shared gate before the outbound cache reconciliation finishes.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread apps/web/core/debates/matchmaking/hooks.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The profile control can remain blocked after the authoritative request list clears a stale outbound request.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment on lines +33 to +34
requests?.outbound ||
activity?.outbound_request ||

This branch was successfully deployed

1 active deployment
Preview — ed7716af Deployed Sep 24, 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.

2 participants