fix(scout-for-lol): route a match by its own id, not by who still tracks it by shepherdjerred · Pull Request #3009 · shepherdjerred/monorepo

shepherdjerred · 2026-09-20T19:59:07Z

Why

The V2 match-context resolver raised a non-retryable failure when no currently tracked account played on the match's platform. Every per-match Activity resolves that context and archiving is the first of them, so the failure landed before anything else could run.

A failing child breaks the discovery loop, so the run stopped, the cursor never advanced past that match, and every later match queued behind it. One account deregistering after a match was observed could stall post-match processing indefinitely, and reconciliation restarts hit the same failure every time. It also fired during exactly those restarts, which is where the deliberate roster precondition was designed to stand down.

This is the most severe of three findings in the same family, all recorded on the tracking issue: the V2 core consults the present in places where it has already recorded the past.

What

The route now comes from the match id's own prefix. The resolver was already deriving that platform, in order to search the roster for an account whose region mapped back to it, and then handing that region to a fetcher that mapped it forward again through a function which accepts either spelling. The round trip was ceremony, and the failure guarded a step that needs no guard.

The fetcher's parameter widens to accept either spelling. That is convergence rather than invention: the sibling timeline fetcher directly below it already took both, so the match fetcher was the odd one out.

The region spelling was strictly narrower, not merely redundant. ME1 is a platform no Region maps to, so a region-derived route could never express a match played there, at any roster size.

This does not weaken the tracked-account precondition. That gate is separate, deliberate and untouched: it lives in the observation commit and has its own documented escape for reconciliation restarts.

Removing the wedge does not relocate the stall. The cursor advance is the last Activity of the per-match workflow, and the report render runs in a fan-out child started after it, so a render failure cannot hold the cursor. This one could, because it ran in the first Activity.

Also removes the context's whole-roster field, which no V2 consumer reads. It was verified unread rather than assumed, and a live field of the same name in the v1 post-match path, populated by v1's own callers, is untouched. The match fetch now happens before the roster query, which makes their independence visible rather than merely true.

Verification

Source and focused local checks only. No CI, image, ArgoCD or runtime layer is claimed.

bunx turbo run build typecheck test lint --filter=scout-for-lol \
  --filter=@scout-for-lol/backend --filter=@scout-for-lol/temporal \
  --filter=@scout-for-lol/domain --filter=@scout-for-lol/app \
  --filter=@scout-for-lol/activity --filter=@scout-for-lol/data --force
44 tasks successful, 482 backend test files

Incremental caches were cleared first, and the run was forced, so nothing was replayed.

Nothing pinned this before. No test named the failure and the module had no test file at all, for a non-retryable failure in the first Activity of every match. The new test covers both the reported case and the platform no Region can express.

Mutation proof: against the previous resolver both cases fail with No tracked account plays on KR and No tracked account plays on ME1. The file was restored and verified byte-identical afterwards.

🤖 Generated with Claude Code

https://claude.ai/code/session_013qRdG6fzJr3THzYzAJwTZB

shepherdjerred · 2026-09-20T19:59:11Z

This change is part of the following stack:

fix(scout-for-lol): route a match by its own id, not by who still tracks it #3009 ◀

_{Change managed by git-spice.}

chatgpt-codex-connector · 2026-09-20T19:59:24Z

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-21T00:00:36.837777Z	`1c50529`	New commits

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d98720be46

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

shepherdjerred · 2026-09-20T23:53:41Z

Review findings dismissed by an operator

Each entry was marked resolved in the review comment after being verified fixed at this head or incorrect. The gate honours the resolved state, so this comment is the record of why.

thread PRRT_kwDOHf4r4c6kLqDL — Fixed, and the convention it cites is near-universal rather than merely common: of the fourteen V2 integration suites that create a test database client, thirteen disconnect in an afterAll hook and this new one was the only exception. It now does the same. The finding is also right about the consequence rather than just the omission: this suite runs inside the large backend suite, so a pool left open for the worker's lifetime draws down a shared connection budget, and the failure that eventually produces would surface in some unrelated suite rather than in this one.
thread PRRT_kwDOHf4r4c6kNG0H — Fixed, and the finding was right about the outcome and wrong about the mechanism, which changed what the fix had to be. The reported sequence cannot occur: deregistration hard-deletes the account row, and the mint's audience comes from an account-by-puuid join, so after a full deregistration the minter mints nothing at all and no render is ever owed. A fix built to that stated mechanism would have been built for a case that cannot happen. The real cause is that the live roster is narrowed by a read of the Discord gateway's guild cache, which returns no filter at all in a process that owns no gateway; the mint runs on the realtime queue and the render on background, so they are different worker pools and can hold different cache state. The rows all still exist and are merely filtered out, so the roster depends on which process asks, not only on when. That helper's own comment justifies its fallback as more work and never the wrong work, which is true for the polling filter it was written for and exactly false here, where the filter narrows a match's roster. What this change does: two resolvers split on the observation commit rather than two fields on one type, because the distinction is temporal. The existing one keeps the present roster for the three consumers that run before the commit, which are the snapshot's producer, the discovery-source precondition, and the archive's alias metadata which runs before any snapshot exists. A new observed resolver serves settlement, progression, the render and the mint, carrying what the observation recorded alongside the configs those identities hydrate to, keyed by identity with no guild filter. A missing observation fails loudly because that is a caller reaching for a snapshot before it is written; an observation recording nobody returns empty because the route fix made that reachable. Worth noting that progression was the worst consumer rather than the render: its attested account count is written into a receipt, so a narrowed roster became durable evidence stating a wrong number. The mint was already correct and its comment already argued this case, so the render now agrees with reasoning that was already written down. Your second scenario does survive by another route and is deliberately not fixed here: fan-out plans children from standing drivable intent rows, so an intent minted before a deregistration and left pending is re-driven later and still fails. No observation-time config is reconstructible once the row is deleted, and preserving one would mean keeping a Discord identity for a user who asked to be removed, which is not an acceptable fix. Retiring an intent whose audience no longer exists is the right answer and is a notification-lifecycle change with its own evidence. This is also an instance of a class: the same filter narrows sets in the prematch context and reads and in v1 ingestion, all recorded separately and not fixed here.

…cks it Why: the V2 match-context resolver raised a non-retryable failure when no currently tracked account played on the match's platform. Every per-match Activity resolves that context, and archiving is the first of them, so the failure landed before anything else could run. A failing child breaks the discovery loop, so the run stopped, the cursor never advanced past that match, and every later match queued behind it. One account deregistering after a match was observed could stall post-match processing indefinitely, and reconciliation restarts hit the same failure every time. It also fired during exactly those restarts, which is where the deliberate roster precondition was designed to stand down. What: the route now comes from the match id's own prefix. The resolver was already deriving that platform in order to search the roster for an account whose region mapped back to it, then handing that region to a fetcher which mapped it forward again through a function that accepts either spelling. The round trip was ceremony, and the failure guarded a step that needs no guard. The fetcher's parameter is widened to accept either spelling, which is convergence rather than invention: the sibling timeline fetcher directly below it already took both, so the match fetcher was the odd one out. The region spelling was also strictly narrower, not merely redundant. ME1 is a platform no Region maps to, so a region-derived route could never express a match played there at any roster size. This is not a weakening of the tracked-account precondition, which is separate, deliberate and untouched: it lives in the observation commit and has its own documented escape for reconciliation restarts. Removing the wedge does not relocate the stall. The cursor advance is the last Activity of the per-match workflow and the report render runs in a fan-out child started after it, so a render failure cannot hold the cursor. This one could, because it ran in the first Activity. Also removes the context's whole-roster field, which no V2 consumer reads. It was verified unread rather than assumed, and a live field of the same name in the v1 post-match path, populated by v1's own callers, is untouched. The match fetch now happens before the roster query, which makes their independence visible rather than merely true. Verification: bunx turbo run build typecheck test lint across all seven Scout filters, forced, with the incremental caches cleared first -- 44 tasks successful, 482 backend test files. Nothing pinned this before: no test named the failure and the module had no test file, for a non-retryable failure in the first Activity of every match. The new test covers both the reported case and the platform no Region can express. Mutation proof: against the previous resolver both cases fail with "No tracked account plays on KR" and "No tracked account plays on ME1" respectively; the file was restored and verified byte-identical afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013qRdG6fzJr3THzYzAJwTZB

Why: the new suite created a dedicated test database client and never released it. It runs inside the large backend suite, so a pool left open for the worker's lifetime draws down a shared connection budget, and the exhaustion that eventually causes would surface in some unrelated suite rather than here. What: release it in an afterAll hook, as the other V2 database integration suites do. Thirteen of the fourteen that create a client already did; this was the only exception. Verification: the suite passes with the hook in place, 2 of 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013qRdG6fzJr3THzYzAJwTZB

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c50529be6

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

Why: routing a match by its own id removed a non-retryable failure that stalled the per-match pipeline at its first Activity. That made previously unreachable code reachable, and the review gate found the consequence. The gate named a mechanism that cannot produce it. It reported that a resumed match whose last participant deregistered mints an owed intent and then fails to render. Deregistration hard-deletes the account row, and the mint's audience comes from an account-by-puuid join, so after a full deregistration the minter mints nothing and no render is ever owed. The real path is fan-out, which plans notification children from STANDING drivable intent rows: an intent minted before the deregistration and left pending by an outage is re-driven later, and that render has nobody to render for. The gate was right about the outcome and wrong about the mechanism, and the difference matters, because a fix built to its stated mechanism would have been built for a case that cannot occur. The cause is not that the roster changed. The live roster is narrowed by a read of the Discord gateway's guild cache, which returns no filter at all in a process that owns no gateway. The mint runs on the realtime queue and the render on background, so they are different worker pools and can hold different cache state. The roster therefore depends on WHICH PROCESS ASKS, not only on when. That helper's own comment says its fallback means "more work, never the wrong work". True for the polling filter it was written for, where widening only costs effort. Exactly false here, where the filter NARROWS a match's roster, so a gateway-owning worker does less work and the wrong work. A justification that holds for the use it was written for, applied to a use it does not cover, is how the comment became the part everyone trusted. The general rule: a fallback that fails open is safe where the set decides how much work to do, and unsafe where the set defines who a durable fact is about. An audience, a settlement roster and an attested count are all the second kind. Failing open in one process and closed in another is not a performance trade, it is two answers to one question. This is an instance of a class. The same filter narrows sets at prematch-reads.ts:155, at prematch-context.ts:142 -- where the shared-derivation comment argues against precisely the drift its own process-dependent derivation permits -- and at temporal-match-ingestion.ts:30. Those are recorded separately and are NOT fixed here. A reader who fixes this one and believes the class is closed is worse off than one who knows three more exist. What: two resolvers, split on the observation commit rather than two fields on one type, because the distinction is temporal and a separately named function tells the reader which side of the commit they are on. The existing resolver keeps the present roster for the three consumers that run BEFORE the observation commits: the snapshot's own producer, the discovery-source precondition, and the archive's alias metadata, which runs before any snapshot exists and is receipt-gated on resume. A new observed resolver serves those that run after: settlement, progression, the render, and the mint. It carries what the observation recorded and the configs those identities hydrate to, which are two facts rather than two rosters: the second is shorter exactly when a registration has been hard-deleted since. Hydration is keyed by identity with no guild filter. A match with no observation fails loudly, because that is a caller reaching for a snapshot before it is written. An observation that recorded nobody returns empty, because the route fix made that case reachable. Those two must not collapse. Progression is the worst of the three consumers: its attested account count is written into a receipt, so a narrowed roster became durable evidence stating a wrong number that no later reader could distinguish from the truth. The mint was already correct and is the precedent; its comment argued this case in full, and the render now agrees with reasoning that was already written down. Not fixed here, deliberately: an intent minted before a deregistration and left pending is re-driven by fan-out and still fails, because the account row is gone and no observation-time config is reconstructible from what remains. Preserving one would mean keeping a Discord identity for a user who asked to be removed, which is not an acceptable fix. Retiring an intent whose audience no longer exists is the right answer and is a notification-lifecycle change with its own evidence. Verification: bunx turbo run build typecheck test lint over the seven Scout workspaces, run five times by the implementing lane and once more independently. The backend package, the only one this change touches, passed 4562 of 4562 in every run without exception, and both new tests passed in every run. Three runs were otherwise green; two carried a single 5000ms test timeout apiece, in the temporal package and the root package, neither of which this diff touches. Those two suites were re-run in isolation on an uncontended machine and both passed, 18 tasks of 18. The flaky pair share one signature, a bare 5000ms timeout in a suite that builds a heavy fixture, where the backend package sets 20000; that is recorded separately as a tooling defect. One pre-existing failure is unrelated and unfixed: the data package's generate step reports that the committed Dare paraphrase schema no longer matches its generator. It is a drift check rather than a build error, it executed and exited non-zero on its own rather than being skipped, it masked no sibling task, and it fails identically on a tree this branch does not touch. Duplication ratchet passed with the baseline unmodified and no new clones. Dependency check clean. Layers verified: source and focused local checks only. CI, published artifacts, ArgoCD reconciliation and runtime are unverified. Four mutations, each broken, observed failing, restored and re-asserted green: the observed resolver rebuilt from the live roster; a missing observation returning empty instead of failing loudly, where the other cases still passed so the two empty cases are provably distinguished; an observation recording nobody made to throw; and the mint using hydrated configs instead of the recorded identities. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013qRdG6fzJr3THzYzAJwTZB

chatgpt-codex-connector · 2026-09-22T02:27:38Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

shepherdjerred · 2026-09-22T02:28:45Z

@codex review

_{Automated re-review request for 216bdca from the CI review gate.}

chatgpt-codex-connector · 2026-09-22T02:28:57Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

shepherdjerred · 2026-09-22T02:43:58Z

@codex review

_{Automated re-review request for 216bdca (attempt 2 of 2) from the CI review gate.}

chatgpt-codex-connector · 2026-09-22T02:44:08Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Why: The branch forked before main landed id-based match routing (#3009), the champion-mastery snapshot layer (#2983, #3037) and the observed-roster split, so PR #3023 was CONFLICTING and its verify lane failed the jscpd ratchet. What: - match-context: extract `canonicalMatchData`, so the client-first payload resolution and main's route-by-match-id fix are one path. Both the live and observed resolvers use it and both now carry `matchDataSource`, which `match-effects` reads off the observed context. Riot is routed by `platformRouteOf`, never by a region derived from the live roster. `allPlayerConfigs` stays dropped from the context type, as main dropped it and nothing reads it back. - riot-player-tools: `currentMastery` keeps preferring a fresh paired-client capture and now reaches Riot through `getChampionMasterySnapshot`, so the Explore tool shares main's cache, freshness and metrics instead of re-fetching behind them. It returns null rather than throwing when neither source answers, and the tool reports source, capturedAt, fetchedAt and freshness. - match-data-fetcher: keep main's widened `PlatformRoute | Region` route alongside the branch's `failureMode`. - player-snapshots: drop the omit of `chestGranted`, which main removed from the mastery schema. - lobby-binding.test: build the two-sided lobby from one helper, removing the clone the jscpd ratchet rejected. Verification: - tsc --noEmit on @scout-for-lol/backend: clean - vitest, 397 passing across src/scout-client, src/temporal/v2, src/betting/dares, src/progression - eslint and prettier clean on every file resolved here - jscpd: the flagged lobby-binding clone is gone and the corpus drops 525 -> 524 clones - NOT run locally: Buildkite verify, integration suites needing Postgres, and any deployed behaviour. Six backend test files cannot load on Windows for pre-existing reasons unrelated to this merge (`new URL().pathname` yielding `/C:/...`, and `@scout-for-lol/data` failing to resolve from an unbuilt workspace dep). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Sep 20, 2026

View reviewed changes

Comment thread packages/scout-for-lol/packages/backend/src/temporal/v2/match-context.integration.test.ts

shepherdjerred and others added 2 commits September 20, 2026 16:55

shepherdjerred force-pushed the scout-v2-observed-context branch from d98720b to 1c50529 Compare September 20, 2026 23:55

chatgpt-codex-connector Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread packages/scout-for-lol/packages/backend/src/temporal/v2/match-context.ts

shepherdjerred merged commit a0d1b15 into main Sep 22, 2026
7 of 9 checks passed

shepherdjerred deleted the scout-v2-observed-context branch September 22, 2026 04:40

Navigation Menu

fix(scout-for-lol): route a match by its own id, not by who still tracks it - #3009

fix(scout-for-lol): route a match by its own id, not by who still tracks it#3009
shepherdjerred merged 3 commits into
mainfrom
scout-v2-observed-context

shepherdjerred commented Sep 20, 2026

Uh oh!

shepherdjerred commented Sep 20, 2026

Uh oh!

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

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

shepherdjerred commented Sep 20, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

shepherdjerred commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

shepherdjerred commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

shepherdjerred commented Sep 20, 2026

Why

What

Verification

Uh oh!

shepherdjerred commented Sep 20, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

shepherdjerred commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review findings dismissed by an operator

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

shepherdjerred commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

shepherdjerred commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

edited

Loading

shepherdjerred commented Sep 20, 2026 •

edited

Loading