Repository navigation
feat(api): publish roster and analytics contracts - #67
MagicTheDev wants to merge 23 commits into
Conversation
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. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
clashking-api | e27a9e4 | Commit Preview URL Branch Preview URL |
Sep 25 2026, 04:37 PM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74decf4378
ℹ️ 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".
| const hook = yield* rosterWebhook(input.bindings.DISCORD_CLIENT_ID, serverId, String(body.channelId)) | ||
| const ids: string[] = [] | ||
| for (const publication of publications) { | ||
| const message = yield* discord.request(`${hook.path}?wait=true&${hook.query}`, { method: "POST", body: { ...publication, ...hook.identity } }) |
There was a problem hiding this comment.
When a client retries after Discord accepted the first POST but the API response was lost, this sends another message because the validated body.nonce is never included in the Discord payload. The database then tracks only the newer message, leaving the original interactive roster post active and unmanageable; pass the nonce with duplicate enforcement or reconcile nonce-matched messages before posting again.
Useful? React with 👍 / 👎.
| const offset = body.event_offset_days ?? current.event_offset_days | ||
| const scheduledAt = offset === null ? body.scheduled_at ?? current.scheduled_at : yield* relativeAutomationTime(sql, serverId, targets.rosterId, targets.groupId, offset) |
There was a problem hiding this comment.
Once a rule has a non-null event_offset_days, every update inherits that value and recomputes scheduled_at, so supplying a new absolute scheduled_at is silently ignored. The update contract also does not accept null, leaving clients no way to switch a relative rule back to an absolute schedule without deleting and recreating it.
Useful? React with 👍 / 👎.
| if (!item || typeof item !== "object" || typeof item.id !== "string" || !/^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/u.test(item.id) | ||
| || ["account", "account_selector", "player", "player_selector"].includes(item.id.toLowerCase()) || ids.has(item.id) |
There was a problem hiding this comment.
This accepts IDs containing uppercase letters or hyphens and IDs up to 64 characters, but both queryMetric and validateRosterViewSpec still require ^[a-z][a-z0-9_]{0,47}$ for signup.answer.questionId. A questionnaire created with an otherwise valid ID such as Question-1 can collect answers, but those answers can never be queried or used in a saved roster view.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d505043cb
ℹ️ 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".
| const roster = (yield* sql<{ roster_role_id: string | null; min_townhall: number | null; max_townhall: number | null; | ||
| signup_scope: "clan-only" | "family-wide"; clan_tag: string | null; server_id: string }>` | ||
| signup_scope: "clan-only" | "family-only" | "anyone"; clan_tag: string | null; server_id: string }>` | ||
| SELECT roster_role_id, min_townhall, max_townhall, signup_scope, clan_tag, server_id FROM rosters WHERE id = ${row.roster_id}::uuid`)[0]! |
There was a problem hiding this comment.
When a roster has require_verified=true, signups submitted through the new Discord publication buttons still reach confirmRosterOperation, but this locked roster read never selects or checks require_verified; only the HTTP submitSignup path performs that check. Consequently, any previously linked but unverified account can join a verification-only roster through the button flow, so this confirmation path must apply the same verification guard before inserting members.
Useful? React with 👍 / 👎.
| const roster = (yield* sql<{ roster_role_id: string | null; min_townhall: number | null; max_townhall: number | null; | ||
| signup_scope: "clan-only" | "family-wide"; clan_tag: string | null; server_id: string }>` | ||
| signup_scope: "clan-only" | "family-only" | "anyone"; clan_tag: string | null; server_id: string }>` | ||
| SELECT roster_role_id, min_townhall, max_townhall, signup_scope, clan_tag, server_id FROM rosters WHERE id = ${row.roster_id}::uuid`)[0]! |
There was a problem hiding this comment.
When max_signups is lower than the legacy capacity, the Discord button flow can overbook the roster: this path eventually calls the separate roster-interaction-membership.ts guard, which checks only rosters.capacity, while the Dashboard signup paths enforce max_signups. Since the published signup buttons use this confirmation path, select and enforce max_signups here as part of the locked final-state validation.
Useful? React with 👍 / 👎.
| const response = yield* effect | ||
| if (["updateRoster", "submitSignup", "submitBatchSignup", "withdrawSignup"].includes(operation)) { | ||
| yield* database("Unable to mark roster publication pending", sql`UPDATE roster_discord_publications SET needs_sync = true WHERE roster_id = ${input.params.rosterId ?? ""}::uuid`) |
There was a problem hiding this comment.
When staff add or remove members through existing operations such as manageMembers, removeMember, refreshRosters, or refreshMember, those operations change data rendered in the Discord roster post but are absent from this synchronization list. The database mutation succeeds while needs_sync remains false and the published member list stays stale until someone manually refreshes it; include every operation that changes publication-visible roster membership or member data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78e2ee6150
ℹ️ 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".
| return yield* database(sql.withTransaction(Effect.gen(function* () { | ||
| yield* lockAuthenticatedUser(userId) | ||
| const rows = yield* sql`WITH target AS (SELECT share_code FROM army_compositions WHERE share_code=${shareCode}), inserted AS ( | ||
| INSERT INTO user_saved_armies(user_id,share_code) SELECT ${userId},share_code FROM target |
There was a problem hiding this comment.
When a user saves an army, this creates user-owned data in user_saved_armies, but exportAccount has no query or response section for that table (unlike user_saved_bases). Consequently, /v2/auth/export silently omits every saved army from the user's privacy export; add a safe saved-armies projection to exportQueries and PrivacyExportResponse.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c26f3b88a
ℹ️ 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".
| WHERE battle.player_tag=$1 AND battle.battle_mode=2 AND battle.direction=1 | ||
| AND battle.battle_time >= $2::timestamptz AND battle.battle_time < $3::timestamptz | ||
| GROUP BY member.family_id ORDER BY attack_count DESC,member.family_id LIMIT 1 |
There was a problem hiding this comment.
When the player uses a different army heavily during the current unfinished Legend day, this CTE can select that family because its upper bound is now, while the subsequent covered CTE explicitly excludes the unfinished day. The response can therefore compare a newly selected family only against older days—or omit the army block entirely if it has no older observations—even though another family dominates the finalized sample. Select the dominant family from the same finalized-day window used by covered and personal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83ef2b787a
ℹ️ 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".
| ...optional("discord_avatar_url", row.discord_user_id && row.cached_discord ? row.cached_discord.user?.avatar | ||
| ? `https://cdn.discordapp.com/avatars/${row.discord_user_id}/${row.cached_discord.user.avatar}.png` | ||
| : `https://cdn.discordapp.com/embed/avatars/${Number((BigInt(row.discord_user_id) >> 22n) % 6n)}.png` : null), |
There was a problem hiding this comment.
When a Discord member has a server-specific avatar, cached_discord.avatar contains that guild avatar, but this response uses only cached_discord.user.avatar and otherwise generates a default avatar. The roster UI therefore shows the global or default avatar instead of the member's active server avatar; construct the guild CDN URL from the top-level cached avatar before falling back to the global avatar.
Useful? React with 👍 / 👎.
| id: Schema.String, | ||
| name: Schema.String, | ||
| minTownhall: Schema.optionalKey(Schema.NullOr(Schema.Number)), | ||
| requireVerified: Schema.optionalKey(Schema.Boolean), |
There was a problem hiding this comment.
When a public roster has require_verified=true, the newly exposed field is always absent because publicRoster neither selects roster.require_verified nor includes requireVerified in its response. Public clients consequently cannot indicate that only verified accounts may sign up and learn the restriction only after a rejected submission; return the stored value from the public handler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bd28b691b
ℹ️ 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 (["updateRoster", "submitSignup", "submitBatchSignup", "withdrawSignup", "manageMembers", "updateMember", "removeMember", "refreshMember"].includes(operation)) { | ||
| yield* database("Unable to mark roster publication pending", sql`UPDATE roster_discord_publications SET needs_sync = true WHERE roster_id = ${input.params.rosterId ?? ""}::uuid`) |
There was a problem hiding this comment.
The fresh evidence after the earlier synchronization fix is that applyMembershipChanges still bypasses this block because it is absent from transactionalOperations, and it is also absent from this synchronization allowlist. When an approved proposal adds, moves, or removes members, the database roster changes but needs_sync is never set and the Discord publication remains stale; route this operation through the publication synchronization flow or mark every affected roster within its own transaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4314b0ccf
ℹ️ 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".
| } else { | ||
| yield* prepareStaticMetadata(bindings, ["heroes"]) |
There was a problem hiding this comment.
For every roster operation outside the five-item mutation list—including publicRoster, signupForm, automation CRUD, and publication actions—this branch reads and decodes static_data/heroes.json even though those handlers do not use hero metadata. A missing or temporarily unavailable ASSETS object now turns otherwise independent roster requests, including the public roster endpoint, into 503 responses, while successful requests incur an unnecessary R2 read; load heroes only for handlers that serialize memberJson or otherwise calculate hero maxima.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e27a9e4124
ℹ️ 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".
| { type: 2, style: 3, label: input.joinLabel, custom_id: `ck:roster:join:${input.id}` }, | ||
| { type: 2, style: 4, label: input.leaveLabel, custom_id: `ck:roster:leave:${input.id}` }, |
There was a problem hiding this comment.
For every roster published in signup mode, these buttons emit join and leave, but verifyRosterInteraction only accepts the board actions signup, remove, sub, and refresh; both clicks therefore fail immediately with wrong_message before the signup/removal flow runs. Emit the canonical signup and remove action names used by the interaction runtime.
Useful? React with 👍 / 👎.
1 participant
Summary
Publishes the local API work for canonical Clash layout links and personal army storage, then extends the Worker and shared contracts for roster signup/publication, CWL participation, daily army setups, legend metadata, and wider war-stat archives. Server account links now require verified in-game token proof for every new link.
The account privacy export includes safe saved-army references (
share_code,saved_at) and account deletion removes only the user's save rows, retaining shared canonical compositions and other users' saves.The local development changes support isolated roster testing, beta Discord credentials, controlled workspace origins, and explicitly scoped read-only production static assets. This PR includes the corresponding unit, PostgreSQL, script, and contract tests, and pins the exact reviewed DevKit migration commit for CI.
Legend player comparisons now choose the dominant army only from finalized days, matching the cohort and personal comparison window; a disposable Goose-042 regression covers a different army overtaking it during the unfinished day.
Public roster snapshots now include the stored
requireVerifiedflag, and roster member avatars prefer a cached guild-specific avatar before the global/default fallback. TherequireVerifiedresponse field was already optional in the shared contract, so no schema shape changed for that fix.Approved multi-roster membership changes now mark every affected interactive publication pending inside the same SQL transaction, then synchronize each Discord post after commit. A disposable Goose-043 test moves a member between two published rosters and verifies both posts are patched.
Roster dispatch now loads static hero metadata only for operations that serialize a legacy roster/member projection with derived hero maxima; unrelated public, automation, publication, and signup-form routes no longer depend on the heroes asset. A disposable Goose-044 test proves the public route survives an ASSETS failure while a derived roster route still fails explicitly.
Dependency and validation
c5fc3ec4db2a6502ee36ac56d8f399b356e4d50e. Merge/apply its forward Goose migrations through 044 before deploying these Worker queries.npm run typecheck,npm run lint,npm test(1,463 tests),npm run test:scripts(41 tests), andgit diff --check. Active roster admission passed 15 PostgreSQL cases, including two concurrent requests for one remaining signup seat. The account-export regression from the removeduser_saved_bases.kindcolumn passed its three PostgreSQL cases.Roster publication reserves each
(roster_id, nonce)before sending through Discord's webhook API, which does not support message nonces. Exact retries return the recorded message ID; a pending reservation returns HTTP 409 withreason: publication_pendingand requires manual reconciliation before any intentional new post. A worker interruption after Discord accepts a message but before its ID is recorded, or partway through a multi-message static post, can leave untracked messages; automatic retry with a fresh nonce would risk duplicates.The separate deferred Bot roster-confirmation reference implementation is not registered in the current API router and requires runtime tables not present in the retained Goose schema. Current Bot roster commands call the active
/v2/server/:serverId/rosters/:rosterId/submissionsroutes. Deferred-only fixtures are deliberately outside the retained PostgreSQL CI inventory; this PR does not claim to ship that deferred route.