Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds Convex no-op z-wrappers, a PlayerDocument Zod schema and DB playerBans table, new REST endpoints (GET/PUT/DELETE) for player documents, Convex internal queries/mutations to save/delete player documents and query bans, and plugin-side model, serializers, ApiService, PlayerDocumentService, and DI bindings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Player
participant Plugin
participant PDS as PlayerDocumentService
participant API as ApiService
participant HTTP as Backend HTTP (/convex)
participant Convex as Convex
participant DB as DB
Player->>Plugin: join event
Plugin->>PDS: onJoin(player)
PDS->>API: GET /players/:uuid/document?joinEvent=true
API->>HTTP: GET /players/:uuid/document
HTTP->>Convex: internalQuery getPlayerByUuid / getPlayerBanByUuid (or create)
Convex->>DB: read/upsert players & playerBans
DB-->>Convex: records
Convex-->>HTTP: PlayerDocument
HTTP-->>API: 200 PlayerDocument
API-->>PDS: PlayerDocument
PDS->>PDS: cache document
note over PDS: periodic flush every 5m
PDS->>API: PUT /players/:uuid/document (cached)
API->>HTTP: PUT /players/:uuid/document
HTTP->>Convex: zInternalMutation savePlayerDocument
Convex->>DB: patch player, upsert banData
DB-->>Convex: OK
Convex-->>HTTP: OK
HTTP-->>API: 200
sequenceDiagram
autonumber
actor Player
participant Plugin
participant PDS
participant API
participant HTTP
Player->>Plugin: quit event
Plugin->>PDS: onQuit(player)
alt cached doc exists
PDS->>API: PUT /players/:uuid/document (save)
API->>HTTP: PUT /players/:uuid/document
HTTP-->>API: 200 OK
else no cache
PDS-->>Plugin: noop
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt (1)
111-117: Add teardown to flush docs and unregister.Call service teardown during shutdown.
override suspend fun onDisableAsync() { HubService.teardown() WorldService.teardown() DebugService.teardown() + PlayerDocumentService.teardown() stopKoin() logger.info("SSMB shutting down") }packages/backend/convex/players.ts (1)
55-65: Add returns validator to internal queryConvex guideline: always specify
returns. Use a permissive validator if full shape is noisy.export const getPlayerByUuid = internalQuery({ args: { uuid: v.string(), }, + returns: v.optional(v.any()), handler: async (ctx, args) => { return await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); }, });
🧹 Nitpick comments (6)
plugin/.env.example (1)
2-4: Drop quotes and add a trailing newline for dotenv hygiene.Unquoted empty values avoid linter warnings; also end the file with a newline.
-AXIOM_DATASET_NAME="" -HTTP_AUTH_TOKEN="" -HTTP_BASE_URL="" +AXIOM_DATASET_NAME= +HTTP_AUTH_TOKEN= +HTTP_BASE_URL= +plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (2)
64-72: Simplify null check.Minor cleanup.
- fun getPlayerDocument(player: Player): PlayerDocument { - val document = documents[player] - - if (document == null) { - throw IllegalStateException("Player document for ${player.name} not found in cache.") - } else { - return document - } - } + fun getPlayerDocument(player: Player): PlayerDocument = + documents[player] ?: error("Player document for ${player.name} not found in cache.")
21-73: Optional: add a flush in teardown to persist any remaining docs.If Manageable.teardown() is intended for cleanup, override it to flush once.
Add:
override fun teardown() { val entries = documents.entries.toList() runBlocking { entries.forEach { (player, doc) -> withContext(Dispatchers.IO) { runCatching { api.playersSetDocumentAsync(player, doc) } .onFailure { plugin.logger.warning("Flush failed for ${player.name}: ${it.message}") } } } } documents.clear() }plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt (1)
18-19: Prefer MutableMap over concrete HashMap in public modelUse interface type to avoid leaking implementation and ease testing.
- val stats: HashMap<String, JsonElement>, + val stats: MutableMap<String, JsonElement>,packages/backend/convex/players.ts (2)
98-105: Add timeout to external fetch to avoid hung actionsPrevent long‑lived actions when playerdb.co is slow/unreachable.
- const response = await fetch(`https://playerdb.co/api/player/minecraft/${args.uuid}`); + const controller = new AbortController(); + const t = setTimeout(() => controller.abort(), 10_000); + const response = await fetch( + `https://playerdb.co/api/player/minecraft/${args.uuid}`, + { signal: controller.signal }, + ).finally(() => clearTimeout(t));
155-181: Add returns validatorMutation returns
null; declare it.export const updatePlayerJoin = internalMutation({ args: { uuid: v.string(), }, + returns: v.null(), handler: async (ctx, args) => { const player = await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); @@ - return null; + return null; }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/backend/convex/_generated/api.d.tsis excluded by!**/_generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/backend/convex/common.ts(1 hunks)packages/backend/convex/http.ts(2 hunks)packages/backend/convex/players.ts(3 hunks)packages/backend/convex/schema.ts(1 hunks)packages/backend/convex/schemas.ts(1 hunks)packages/backend/package.json(1 hunks)plugin/.env.example(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt(2 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/package.json
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
Always add
@types/nodeto yourpackage.jsonif any Node.js built-in modules are used in the project.
Files:
packages/backend/package.json
**/convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/convex/**/*.ts: ALWAYS use the new function syntax for Convex functions using function builders like query, mutation, internalQuery, internalMutation, action, and internalAction.
ALWAYS include argument and return validators for all Convex functions (query,internalQuery,mutation,internalMutation,action,internalAction). If a function does not return anything, includereturns: v.null()as its output validator.
UseinternalQuery,internalMutation, andinternalActionto register internal (private) functions, and usequery,mutation, andactionto register public functions. Do not use these to register sensitive internal functions as public.
You CANNOT register a function through theapiorinternalobjects; always use the appropriate function builder (query, mutation, etc.).
When usingctx.runQuery,ctx.runMutation, orctx.runActionto call a function in the same file, specify a type annotation on the return value to work around TypeScript circularity limitations.
Use theapiobject fromconvex/_generated/api.tsto call public functions and theinternalobject to call internal functions; respect function privacy and file-based routing.
Usectx.runQueryto call a query,ctx.runMutationto call a mutation, andctx.runActionfor actions. Do NOT try to pass the callee directly; always provide aFunctionReference.
Do NOT usefilterin Convex queries. Instead, define an index in the schema and usewithIndexin queries.
Convex queries do NOT support.delete(). Instead,.collect()the results and callctx.db.delete(row._id)for each.
Use.unique()to get a single document from a query; it throws if multiple matches exist.
When using async iteration on Convex queries, usefor await (const row of query)—do not use.collect()or.take(n).
Usectx.db.replacefor a full document replacement andctx.db.patchfor shallow merging. Both throw if the document does not exist.
Always add '"use node";' to the ...
Files:
packages/backend/convex/schemas.tspackages/backend/convex/common.tspackages/backend/convex/schema.tspackages/backend/convex/http.tspackages/backend/convex/players.ts
**/convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/convex/schema.ts: Always define your schema inconvex/schema.tsand always import schema definition functions fromconvex/server.
Always include all index fields in the index name (e.g., index on["field1", "field2"]should be named "by_field1_and_field2").
Index fields must be queried in the same order they are defined. If you need different orders, define separate indexes.
Usev.record()for defining record types in schema;v.map()andv.set()are not supported.
Files:
packages/backend/convex/schema.ts
🧠 Learnings (8)
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use the Convex-supported set of validators for argument and schema validation; avoid unsupported or deprecated ones as listed.
Applied to files:
packages/backend/package.jsonpackages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/schema.ts : Always define your schema in `convex/schema.ts` and always import schema definition functions from `convex/server`.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Define all Convex database tables using `defineTable` and specify indexes using `.index()` in schema definition files.
Applied to files:
packages/backend/convex/schemas.tspackages/backend/convex/schema.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use `internalQuery`, `internalMutation`, and `internalAction` to register internal (private) functions, and use `query`, `mutation`, and `action` to register public functions. Do not use these to register sensitive internal functions as public.
Applied to files:
packages/backend/convex/common.tspackages/backend/convex/players.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : ALWAYS use the new function syntax for Convex functions using function builders like query, mutation, internalQuery, internalMutation, action, and internalAction.
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : ALWAYS include argument and return validators for all Convex functions (`query`, `internalQuery`, `mutation`, `internalMutation`, `action`, `internalAction`). If a function does not return anything, include `returns: v.null()` as its output validator.
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use `ctx.runQuery` to call a query, `ctx.runMutation` to call a mutation, and `ctx.runAction` for actions. Do NOT try to pass the callee directly; always provide a `FunctionReference`.
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : You CANNOT register a function through the `api` or `internal` objects; always use the appropriate function builder (query, mutation, etc.).
Applied to files:
packages/backend/convex/common.ts
🧬 Code graph analysis (2)
packages/backend/convex/http.ts (1)
packages/backend/convex/schemas.ts (2)
PlayerDocument(19-19)playerDocumentSchema(3-17)
packages/backend/convex/players.ts (3)
packages/backend/convex/common.ts (1)
zInternalMutation(11-11)packages/backend/convex/schemas.ts (1)
playerDocumentSchema(3-17)packages/backend/convex/_generated/server.js (4)
internalMutation(59-59)internalMutation(59-59)internalQuery(39-39)internalQuery(39-39)
🪛 dotenv-linter (3.3.0)
plugin/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🔇 Additional comments (4)
packages/backend/convex/schema.ts (1)
20-26: Schema addition looks good; confirm index suffices for your read patterns.by_uuid is appropriate. If you’ll frequently fetch “active” bans (not expired) or latest ban, consider adding a composite index (e.g., by_uuid_and_bannedAt or by_uuid_and_expiresAt) to avoid scanning.
If queries already exist, please confirm they use withIndex("by_uuid") and don’t rely on filter.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (1)
55-61: Quit handler is fine; keep writes to cache on main thread.Removal happens on main; the network call is offloaded to IO. LGTM.
packages/backend/convex/common.ts (1)
1-11: Wrapper setup LGTM.Exports establish standardized zod-enabled builders for public and internal ops.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt (1)
44-59: Endpoints look consistent; minor nit: contentType already set in defaultRequest.Leaving as-is is fine.
| app.put("/players/:uuid/document", zValidator("json", playerDocumentSchema), async (c) => { | ||
| const uuid = c.req.param("uuid"); | ||
| const document = c.req.valid("json"); | ||
|
|
||
| await c.env.runMutation(internal.players.savePlayerDocument, { | ||
| uuid: uuid as any, | ||
| document, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Remove unsafe casts; return a response on PUT
as any masks a real type mismatch. Also, the handler returns nothing; send a success response.
-app.put("/players/:uuid/document", zValidator("json", playerDocumentSchema), async (c) => {
+app.put("/players/:uuid/document", zValidator("json", playerDocumentSchema), async (c) => {
const uuid = c.req.param("uuid");
const document = c.req.valid("json");
await c.env.runMutation(internal.players.savePlayerDocument, {
- uuid: uuid as any,
+ uuid,
document,
});
+ return c.json({ message: "Player document saved" });
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/backend/convex/http.ts around lines 112 to 120, remove the unsafe
"as any" cast on uuid and ensure the types passed to runMutation match the
mutation signature (convert/validate uuid to the expected type, e.g., string or
UUID type) by explicitly typing or parsing the param and using the validated
document type from zValidator; after awaiting runMutation return an appropriate
HTTP response (e.g., JSON success or 204) with correct status so the PUT handler
does not return nothing.
| app.delete("/players/:uuid/document", async (c) => { | ||
| const uuid = c.req.param("uuid"); | ||
| await c.env.runMutation(internal.players.deletePlayerDocument, { uuid: uuid as any }); | ||
|
|
||
| return c.json({ message: "Player data deleted successfully" }); | ||
| }); |
There was a problem hiding this comment.
Remove unsafe cast on DELETE
Aligns with delete mutation accepting external UUID.
app.delete("/players/:uuid/document", async (c) => {
const uuid = c.req.param("uuid");
- await c.env.runMutation(internal.players.deletePlayerDocument, { uuid: uuid as any });
+ await c.env.runMutation(internal.players.deletePlayerDocument, { uuid });
return c.json({ message: "Player data deleted successfully" });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app.delete("/players/:uuid/document", async (c) => { | |
| const uuid = c.req.param("uuid"); | |
| await c.env.runMutation(internal.players.deletePlayerDocument, { uuid: uuid as any }); | |
| return c.json({ message: "Player data deleted successfully" }); | |
| }); | |
| app.delete("/players/:uuid/document", async (c) => { | |
| const uuid = c.req.param("uuid"); | |
| await c.env.runMutation(internal.players.deletePlayerDocument, { uuid }); | |
| return c.json({ message: "Player data deleted successfully" }); | |
| }); |
🤖 Prompt for AI Agents
In packages/backend/convex/http.ts around lines 122 to 127, the DELETE handler
currently casts uuid using "as any" before passing it to
internal.players.deletePlayerDocument; remove the unsafe cast and pass the uuid
with the correct type expected by the mutation (e.g., string or ExternalUuid
type). If necessary, perform a runtime null/undefined check and/or
convert/validate the param to the expected type before calling runMutation so
the call is type-safe and aligns with the delete mutation's external UUID
parameter.
| export const getPlayerBanByUuid = internalQuery({ | ||
| args: { | ||
| uuid: v.string(), | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| return await ctx.db | ||
| .query("playerBans") | ||
| .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) | ||
| .unique(); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add returns validator to ban query
Same rationale as above.
export const getPlayerBanByUuid = internalQuery({
args: {
uuid: v.string(),
},
+ returns: v.optional(v.any()),
handler: async (ctx, args) => {
return await ctx.db
.query("playerBans")
.withIndex("by_uuid", (q) => q.eq("uuid", args.uuid))
.unique();
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getPlayerBanByUuid = internalQuery({ | |
| args: { | |
| uuid: v.string(), | |
| }, | |
| handler: async (ctx, args) => { | |
| return await ctx.db | |
| .query("playerBans") | |
| .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) | |
| .unique(); | |
| }, | |
| }); | |
| export const getPlayerBanByUuid = internalQuery({ | |
| args: { | |
| uuid: v.string(), | |
| }, | |
| returns: v.optional(v.any()), | |
| handler: async (ctx, args) => { | |
| return await ctx.db | |
| .query("playerBans") | |
| .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) | |
| .unique(); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/backend/convex/players.ts around lines 67 to 77, the internalQuery
definition for getPlayerBanByUuid is missing a returns validator; add a returns
validator that reflects that unique() can return either a playerBan object or
null (e.g., returns: v.nullable(v.type("playerBan")) or the appropriate nullable
object schema used elsewhere) so the query validator matches the actual return
shape.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt
Outdated
Show resolved
Hide resolved
| private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN") | ||
| private val httpBaseUrl = System.getenv("HTTP_BASE_URL") | ||
|
|
||
| @PublishedApi | ||
| internal val convexClient = | ||
| HttpClient(CIO) { | ||
| defaultRequest { | ||
| if (!httpAuthToken.isNullOrBlank()) { | ||
| header("Authorization", "Bearer $httpAuthToken") | ||
| } | ||
| contentType(ContentType.Application.Json) | ||
| url("$httpBaseUrl/convex") | ||
| } | ||
| install(ContentNegotiation) { |
There was a problem hiding this comment.
Base URL handling can produce invalid URLs when HTTP_BASE_URL is unset or has stray slashes.
Fail fast if missing and normalize the base before appending /convex.
- private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN")
- private val httpBaseUrl = System.getenv("HTTP_BASE_URL")
+ private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN")
+ private val httpBaseUrl = System.getenv("HTTP_BASE_URL")?.trim()?.trimEnd('/')
+ private val convexBase =
+ (httpBaseUrl ?: error("HTTP_BASE_URL is required (e.g., http://localhost:3000)")) + "/convex"
@@
- defaultRequest {
+ defaultRequest {
if (!httpAuthToken.isNullOrBlank()) {
header("Authorization", "Bearer $httpAuthToken")
}
contentType(ContentType.Application.Json)
- url("$httpBaseUrl/convex")
+ url(convexBase)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN") | |
| private val httpBaseUrl = System.getenv("HTTP_BASE_URL") | |
| @PublishedApi | |
| internal val convexClient = | |
| HttpClient(CIO) { | |
| defaultRequest { | |
| if (!httpAuthToken.isNullOrBlank()) { | |
| header("Authorization", "Bearer $httpAuthToken") | |
| } | |
| contentType(ContentType.Application.Json) | |
| url("$httpBaseUrl/convex") | |
| } | |
| install(ContentNegotiation) { | |
| private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN") | |
| private val httpBaseUrl = System.getenv("HTTP_BASE_URL")?.trim()?.trimEnd('/') | |
| private val convexBase = | |
| (httpBaseUrl ?: error("HTTP_BASE_URL is required (e.g., http://localhost:3000)")) + "/convex" | |
| @PublishedApi | |
| internal val convexClient = | |
| HttpClient(CIO) { | |
| defaultRequest { | |
| if (!httpAuthToken.isNullOrBlank()) { | |
| header("Authorization", "Bearer $httpAuthToken") | |
| } | |
| contentType(ContentType.Application.Json) | |
| url(convexBase) | |
| } | |
| install(ContentNegotiation) { |
🤖 Prompt for AI Agents
In plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt
around lines 20 to 33, the current code builds the client URL directly from
HTTP_BASE_URL which can be null/blank or contain stray slashes; update
initialization to fail fast if HTTP_BASE_URL is null or blank (throw
IllegalStateException with a clear message) and normalize the value by trimming
whitespace and removing trailing slashes before appending "/convex" so the final
url is formed as normalizedBase + "/convex".
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt
Show resolved
Hide resolved
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt
Show resolved
Hide resolved
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt
Show resolved
Hide resolved
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
plugin/.env.example (3)
2-4: Drop quotes and add brief guidance to satisfy dotenv-linter and avoid confusion.Quoted empty strings trigger lint warnings and sometimes confuse loaders. Also clarify token format and base URL expectations.
Apply this diff:
-AXIOM_DATASET_NAME="" -HTTP_AUTH_TOKEN="" -HTTP_BASE_URL="" +# Axiom dataset (example: ssm-brawl) +AXIOM_DATASET_NAME= +# API auth token (do not prefix with "Bearer ") +HTTP_AUTH_TOKEN= +# Base URL for API (no trailing slash), e.g., https://api.example.com +HTTP_BASE_URL=Note: For consistency, consider also removing quotes from AXIOM_API_TOKEN on Line 1.
4-4: Add a trailing newline.dotenv-linter reports “No blank line at the end of the file”. Add a final EOL to silence it.
-HTTP_BASE_URL= +HTTP_BASE_URL= +
3-4: Clarify token format and normalize base URL (no trailing slash)
- HTTP_AUTH_TOKEN — do NOT include the "Bearer " prefix; the code adds it when setting Authorization (plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt:28). AxiomLoggerHandler does the same for AXIOM_API_TOKEN (plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/AxiomLoggerHandler.kt:51). Update .env.example to state "token only (no 'Bearer ')".
- HTTP_BASE_URL — currently used as "$httpBaseUrl/convex" (plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt:31). Either document that the env value must not end with a trailing slash, or make the code robust (e.g., httpBaseUrl.trimEnd('/') or use a URL builder) to avoid producing "//convex".
packages/backend/convex/schema.ts (1)
20-26: Consider using consistent date field types and add missing indicesThe playerBans table uses string fields for dates while other tables may benefit from a consistent approach. Also, consider adding an index on
expiresAtfor efficient expiration queries.playerBans: defineTable({ uuid: v.string(), reason: v.string(), expiresAt: v.optional(v.string()), bannedAt: v.string(), bannedBy: v.string(), - }).index("by_uuid", ["uuid"]), + }).index("by_uuid", ["uuid"]) + .index("by_expiration", ["expiresAt"]),Also consider adding a compound index for efficient ban checking:
+ .index("by_uuid_and_expiration", ["uuid", "expiresAt"])plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (1)
64-72: Consider returning nullable instead of throwing exceptionThe current implementation throws an exception when a document isn't found, which could crash calling code. Consider returning null or using a Result type for safer error handling.
-fun getPlayerDocument(player: Player): PlayerDocument { - val document = documents[player] - - if (document == null) { - throw IllegalStateException("Player document for ${player.name} not found in cache.") - } else { - return document - } -} +fun getPlayerDocument(player: Player): PlayerDocument? { + return documents[player] +} + +fun requirePlayerDocument(player: Player): PlayerDocument { + return documents[player] + ?: throw IllegalStateException("Player document for ${player.name} not found in cache.") +}This gives callers the choice between safe nullable access and explicit exception-based access.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt (1)
59-73: setStat doesn’t need reified/inline; simplify signature.No type checks rely on T; use Any and keep runtime guards.
- /** Type-safe setter for stats values. Mutates the stats map */ - inline fun <reified T> setStat(key: String, value: T) { + /** Type-safe setter for stats values. Mutates the stats map */ + fun setStat(key: String, value: Any) { val jsonElement = when (value) { is String -> JsonPrimitive(value) is Number -> JsonPrimitive(value) is Boolean -> JsonPrimitive(value) else -> throw IllegalArgumentException( "Stats only support String, Number, or Boolean types" ) } stats[key] = jsonElement }packages/backend/convex/players.ts (2)
55-66: Add returns validator to getPlayerByUuid.Matches possible null result from unique().
export const getPlayerByUuid = internalQuery({ args: { uuid: v.string(), }, + returns: v.nullable(v.any()), handler: async (ctx, args) => { return await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); }, });
155-181: Add returns validator to updatePlayerJoin.Function returns null explicitly; declare it.
export const updatePlayerJoin = internalMutation({ args: { uuid: v.string(), }, + returns: v.null(), handler: async (ctx, args) => { @@ - return null; + return null; }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/backend/convex/_generated/api.d.tsis excluded by!**/_generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/backend/convex/common.ts(1 hunks)packages/backend/convex/http.ts(2 hunks)packages/backend/convex/players.ts(3 hunks)packages/backend/convex/schema.ts(1 hunks)packages/backend/convex/schemas.ts(1 hunks)packages/backend/package.json(1 hunks)plugin/.env.example(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt(2 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Define all Convex database tables using `defineTable` and specify indexes using `.index()` in schema definition files.
Applied to files:
packages/backend/convex/schema.tspackages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use the Convex-supported set of validators for argument and schema validation; avoid unsupported or deprecated ones as listed.
Applied to files:
packages/backend/package.jsonpackages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use `internalQuery`, `internalMutation`, and `internalAction` to register internal (private) functions, and use `query`, `mutation`, and `action` to register public functions. Do not use these to register sensitive internal functions as public.
Applied to files:
packages/backend/convex/common.tspackages/backend/convex/players.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : ALWAYS use the new function syntax for Convex functions using function builders like query, mutation, internalQuery, internalMutation, action, and internalAction.
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : ALWAYS include argument and return validators for all Convex functions (`query`, `internalQuery`, `mutation`, `internalMutation`, `action`, `internalAction`). If a function does not return anything, include `returns: v.null()` as its output validator.
Applied to files:
packages/backend/convex/common.tspackages/backend/convex/players.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use `ctx.runQuery` to call a query, `ctx.runMutation` to call a mutation, and `ctx.runAction` for actions. Do NOT try to pass the callee directly; always provide a `FunctionReference`.
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : You CANNOT register a function through the `api` or `internal` objects; always use the appropriate function builder (query, mutation, etc.).
Applied to files:
packages/backend/convex/common.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/schema.ts : Always define your schema in `convex/schema.ts` and always import schema definition functions from `convex/server`.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : When returning a null value from a query or mutation, always use the `v.null()` validator.
Applied to files:
packages/backend/convex/players.ts
🪛 dotenv-linter (3.3.0)
plugin/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🔇 Additional comments (15)
plugin/.env.example (1)
2-2: Reordering looks fine.Moving AXIOM_DATASET_NAME next to AXIOM_API_TOKEN improves discoverability.
packages/backend/convex/common.ts (1)
1-11: Good abstraction for Convex custom functionsThe NoOp wrappers provide a clean abstraction layer for Convex queries and mutations, maintaining consistency across the codebase.
packages/backend/package.json (1)
16-16: Fix Zod version mismatch and verify @hono/zod-validator compatibilityThe backend code imports from "zod/v3" but package.json declares zod ^4.0.2. Additionally, @hono/zod-validator@0.7.3 may not be published (latest is 0.7.2).
Apply this diff to use the correct published version:
- "@hono/zod-validator": "^0.7.3", + "@hono/zod-validator": "^0.7.2",plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt (1)
49-50: PlayerDocumentService requires initialization after registrationThe service is registered with DI but its
setup()method is never called, so listeners and tasks won't be registered.Add initialization after the DI setup:
startKoin { modules( module { single { this@SuperSmashMobsBrawl } single<JavaPlugin> { this@SuperSmashMobsBrawl } single { this@SuperSmashMobsBrawl.logger } single { ApiService } single { PlayerDocumentService } single(createdAtStart = true) { DataService() } single { MinigameService() } single { KitService } single(createdAtStart = true) { LangService(this@SuperSmashMobsBrawl) } single { WorldService } single { HubService } } ) } + // Initialize PlayerDocumentService to register listeners and tasks + PlayerDocumentService.setup()packages/backend/convex/schemas.ts (1)
1-19: Update to Zod v4 and use built-in validators for better type safetyThe import uses "zod/v3" while package.json declares v4. Also, leverage Zod v4's built-in validators for URLs and date-time strings.
-import z from "zod/v3"; +import { z } from "zod"; export const playerDocumentSchema = z.object({ isFirstTimeOnServer: z.boolean(), - avatarUrl: z.string(), - lastJoinTime: z.string(), + avatarUrl: z.string().url(), + lastJoinTime: z.string().datetime(), stats: z.record(z.string(), z.union([z.string(), z.number(), z.boolean()])), banData: z.nullable( z.object({ isBanned: z.boolean(), reason: z.string(), - expiresAt: z.optional(z.string()), - bannedAt: z.string(), + expiresAt: z.optional(z.string().datetime()), + bannedAt: z.string().datetime(), bannedBy: z.string(), }), ), });plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (3)
29-37: Protect against concurrent modification in periodic flushThe iteration over the documents map can throw ConcurrentModificationException if modified during iteration. Also add error handling to prevent task failure.
runnables.add( repeatingTask(5.minutes.ticks) { - documents.forEach { player, document -> + val snapshot = documents.entries.toList() + snapshot.forEach { (player, document) -> plugin.launch { withContext(Dispatchers.IO) { - api.playersSetDocumentAsync(player, document) + runCatching { + api.playersSetDocumentAsync(player, document) + }.onFailure { e -> + plugin.logger.warning("Failed to save document for ${player.name}: ${e.message}") + } } } } } )
41-50: Remove runBlocking to avoid server thread stallsUsing
runBlockingin a player join event can freeze the server thread. The coroutine launch is sufficient, and the map mutation should happen on the main thread.listeners.add( event<PlayerJoinEvent>(priority = EventPriority.LOWEST) { - runBlocking { - plugin.launch { - withContext(Dispatchers.IO) { - val document = api.playersGetDocumentAsync(player, true) - documents[player] = document - } + plugin.launch { + val document = withContext(Dispatchers.IO) { + api.playersGetDocumentAsync(player, true) } + // Write to map on main thread + documents[player] = document } } )
25-25: Consider using ConcurrentHashMap for thread safetyThe documents map is accessed from multiple coroutine contexts (join event, quit event, periodic flush). While most operations happen on the main thread after your fixes, using a thread-safe map provides additional safety.
-private val documents = hashMapOf<Player, PlayerDocument>() +private val documents = java.util.concurrent.ConcurrentHashMap<Player, PlayerDocument>()packages/backend/convex/players.ts (2)
67-77: Add returns validator to getPlayerBanByUuid.unique() can return null.
export const getPlayerBanByUuid = internalQuery({ args: { uuid: v.string(), }, + returns: v.nullable(v.any()), handler: async (ctx, args) => { return await ctx.db .query("playerBans") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); }, });
46-53: Delete uses Convex Id; accept external uuid and resolve internally; add returns.-export const deletePlayerDocument = internalMutation({ - args: { - uuid: v.id("players"), - }, - handler: async (ctx, args) => { - return await ctx.db.delete(args.uuid); - }, -}); +export const deletePlayerDocument = internalMutation({ + args: { uuid: v.string() }, + returns: v.null(), + handler: async (ctx, args) => { + const player = await ctx.db + .query("players") + .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) + .unique(); + if (!player) throw new Error("Player not found"); + await ctx.db.delete(player._id); + return null; + }, +});plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt (1)
20-33: Fail fast and normalize HTTP_BASE_URL; avoid "null/convex" and double slashes.Currently "$httpBaseUrl/convex" can become "null/convex" or include duplicate slashes. Normalize and error early.
- private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN") - private val httpBaseUrl = System.getenv("HTTP_BASE_URL") + private val httpAuthToken = System.getenv("HTTP_AUTH_TOKEN") + private val httpBaseUrl = System.getenv("HTTP_BASE_URL")?.trim()?.trimEnd('/') + private val convexBase = + (httpBaseUrl ?: error("HTTP_BASE_URL is required (e.g., http://localhost:3000)")) + "/convex" @@ - url("$httpBaseUrl/convex") + url(convexBase)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt (1)
81-88: Make banData.expiresAt nullable to match backend (can be absent).Prevents deserialization failures when no expiry exists.
data class PlayerBanData( val isBanned: Boolean, val reason: String, - val expiresAt: String, + val expiresAt: String?, val bannedAt: String, val bannedBy: String, )#!/bin/bash # Find usages assuming non-null expiresAt. rg -nP '\bexpiresAt\b(?!\s*\?)' -C2packages/backend/convex/http.ts (3)
81-86: Refresh player data after join update to return fresh lastJoinTime/joinCount.- } else if (isJoinEvent) { - await c.env.runMutation(internal.players.updatePlayerJoin, { uuid }); - } + } else if (isJoinEvent) { + await c.env.runMutation(internal.players.updatePlayerJoin, { uuid }); + // Re-fetch to return updated values + playerData = await c.env.runQuery(internal.players.getPlayerByUuid, { uuid }); + }
112-120: Remove unsafe cast and return a response on PUT.Aligns types and avoids silent 200-without-body.
app.put("/players/:uuid/document", zValidator("json", playerDocumentSchema), async (c) => { const uuid = c.req.param("uuid"); const document = c.req.valid("json"); await c.env.runMutation(internal.players.savePlayerDocument, { - uuid: uuid as any, + uuid, document, }); + return c.json({ message: "Player document saved" }); });
122-127: Remove unsafe cast in DELETE.Let the mutation resolve external UUID → internal id.
app.delete("/players/:uuid/document", async (c) => { const uuid = c.req.param("uuid"); - await c.env.runMutation(internal.players.deletePlayerDocument, { uuid: uuid as any }); + await c.env.runMutation(internal.players.deletePlayerDocument, { uuid }); return c.json({ message: "Player data deleted successfully" }); });
| export const savePlayerDocument = zInternalMutation({ | ||
| args: { | ||
| uuid: zid("players"), | ||
| document: playerDocumentSchema, | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| if (args.document.banData) { | ||
| const existingBan = await ctx.db | ||
| .query("playerBans") | ||
| .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) | ||
| .unique(); | ||
|
|
||
| if (existingBan) { | ||
| await ctx.db.patch(existingBan._id, { | ||
| reason: args.document.banData.reason, | ||
| expiresAt: args.document.banData.expiresAt, | ||
| }); | ||
| } else { | ||
| await ctx.db.insert("playerBans", { | ||
| uuid: args.uuid, | ||
| reason: args.document.banData.reason, | ||
| expiresAt: args.document.banData.expiresAt, | ||
| bannedAt: args.document.banData.bannedAt, | ||
| bannedBy: args.document.banData.bannedBy, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return await ctx.db.patch(args.uuid, { | ||
| lastJoinedAt: args.document.lastJoinTime, | ||
| stats: args.document.stats, | ||
| avatarUrl: args.document.avatarUrl, | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Fix uuid type mismatch (external UUID), patch by internal _id, add returns, and handle unban.
Current code treats external UUID as a Convex Id and never unbans.
-export const savePlayerDocument = zInternalMutation({
- args: {
- uuid: zid("players"),
- document: playerDocumentSchema,
- },
- handler: async (ctx, args) => {
- if (args.document.banData) {
- const existingBan = await ctx.db
- .query("playerBans")
- .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid))
- .unique();
-
- if (existingBan) {
- await ctx.db.patch(existingBan._id, {
- reason: args.document.banData.reason,
- expiresAt: args.document.banData.expiresAt,
- });
- } else {
- await ctx.db.insert("playerBans", {
- uuid: args.uuid,
- reason: args.document.banData.reason,
- expiresAt: args.document.banData.expiresAt,
- bannedAt: args.document.banData.bannedAt,
- bannedBy: args.document.banData.bannedBy,
- });
- }
- }
-
- return await ctx.db.patch(args.uuid, {
- lastJoinedAt: args.document.lastJoinTime,
- stats: args.document.stats,
- avatarUrl: args.document.avatarUrl,
- });
- },
-});
+export const savePlayerDocument = zInternalMutation({
+ args: {
+ uuid: v.string(), // external Minecraft UUID
+ document: playerDocumentSchema,
+ },
+ returns: v.null(),
+ handler: async (ctx, args) => {
+ const player = await ctx.db
+ .query("players")
+ .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid))
+ .unique();
+ if (!player) throw new Error("Player not found");
+
+ const existingBan = await ctx.db
+ .query("playerBans")
+ .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid))
+ .unique();
+
+ if (args.document.banData) {
+ if (existingBan) {
+ await ctx.db.patch(existingBan._id, {
+ reason: args.document.banData.reason,
+ expiresAt: args.document.banData.expiresAt,
+ });
+ } else {
+ await ctx.db.insert("playerBans", {
+ uuid: args.uuid,
+ reason: args.document.banData.reason,
+ expiresAt: args.document.banData.expiresAt,
+ bannedAt: args.document.banData.bannedAt,
+ bannedBy: args.document.banData.bannedBy,
+ });
+ }
+ } else if (existingBan) {
+ await ctx.db.delete(existingBan._id); // unban
+ }
+
+ await ctx.db.patch(player._id, {
+ lastJoinedAt: args.document.lastJoinTime,
+ stats: args.document.stats,
+ avatarUrl: args.document.avatarUrl,
+ });
+ return null;
+ },
+});🏁 Script executed:
#!/bin/bash
# Show type mismatch usage across HTTP and convex layers.
rg -n 'savePlayerDocument' -C2
rg -n 'zid\("players"\)' packages/backend/convex/players.tsLength of output: 683
Fix UUID type mismatch: accept external UUID, patch by internal _id, return null, and handle unban.
http.ts is calling internal.players.savePlayerDocument with an external UUID (packages/backend/convex/http.ts:116–118 — uuid: uuid as any) while players.ts validates uuid as a Convex id (zid("players")) and calls ctx.db.patch(args.uuid); this treats an external UUID as an internal Id and prevents unban logic. Change players.ts to accept an external string UUID (e.g. uuid: v.string()), add returns: v.null(), resolve the player by the external UUID (.query("players").withIndex("by_uuid", q => q.eq("uuid", args.uuid)).unique()), patch the found player’s internal _id, and when banData is falsy delete the existing playerBans row (ctx.db.delete(existingBan._id)). Also remove the uuid as any cast in http.ts and pass a proper string.
🤖 Prompt for AI Agents
In packages/backend/convex/players.ts around lines 10 to 44, fix the UUID type
mismatch and unban handling by changing the mutation to accept an external UUID
string (uuid: v.string()) and declare returns: v.null(), then resolve the
internal player row via ctx.db.query("players").withIndex("by_uuid", q =>
q.eq("uuid", args.uuid)).unique(); if no player is found return null; when a
player is found use ctx.db.patch(player._id, ...) to update
lastJoinedAt/stats/avatarUrl; for ban handling, if args.document.banData is
truthy insert or patch the playerBans row (by querying by uuid) as before, but
if args.document.banData is falsy delete the existing playerBans row via
ctx.db.delete(existingBan._id); finally remove the uuid as any cast in
packages/backend/convex/http.ts and pass the actual external UUID string.
| suspend inline fun <reified T> kvGetAsync(key: String): T? { | ||
| val response: T = convexClient.get("/kv/$key").body() | ||
|
|
||
| return response | ||
| } | ||
|
|
||
| suspend inline fun kvSetAsync(key: String, value: Any) { | ||
| convexClient.put("/kv/$key") { | ||
| contentType(ContentType.Application.Json) | ||
| setBody(value) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
KV client: fix leading “/”, match server response shape, and drop unnecessary inline.
Server returns { key, value }; client deserializes T directly and uses a leading “/” which resets the base path.
- suspend inline fun <reified T> kvGetAsync(key: String): T? {
- val response: T = convexClient.get("/kv/$key").body()
-
- return response
- }
+ suspend inline fun <reified T> kvGetAsync(key: String): T? {
+ // 404 -> exception; return null in that case
+ return runCatching { convexClient.get("kv/$key").body<KvGetResponse<T>>() }
+ .getOrNull()
+ ?.value
+ }
- suspend inline fun kvSetAsync(key: String, value: Any) {
- convexClient.put("/kv/$key") {
+ suspend fun kvSetAsync(key: String, value: Any) {
+ convexClient.put("kv/$key") {
contentType(ContentType.Application.Json)
setBody(value)
}
}Add this DTO in the same file (top-level) to support deserialization:
@kotlinx.serialization.Serializable
data class KvGetResponse<T>(val key: String, val value: T? = null)🏁 Script executed:
#!/bin/bash
# Confirm server KV shape and spot leading slashes in client.
rg -n 'app.get\("/kv/:key"' -C2 packages/backend/convex/http.ts
rg -n 'get\("?/kv/' plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.ktLength of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== rg getKv in repo ==="
rg -n --hidden 'getKv' || true
echo
echo "=== rg 'api.kv' in repo ==="
rg -n --hidden 'api\.kv' || true
echo
echo "=== Show backend convex/http.ts (around app.get) ==="
sed -n '1,240p' packages/backend/convex/http.ts || true
echo
echo "=== Search for implementation of kv handlers (kv.* in backend) ==="
rg -n --hidden 'kv' packages/backend -S || true
echo
echo "=== Show plugin ApiService.kt ==="
sed -n '1,240p' plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt || true
echo
echo "=== Search for convexClient definition and HttpClient creation ==="
rg -n --hidden 'convexClient' -S || true
rg -n --hidden 'HttpClient\\(' -S plugin || true
rg -n --hidden 'defaultRequest\\s*\\{' -S plugin || trueLength of output: 9089
KV client: remove leading '/', deserialize server {key,value}, and drop unnecessary inline.
Server returns { key, value } (packages/backend/convex/http.ts) and convexClient.defaultRequest sets url(.../convex); the leading '/' resets the base path and deserializing into T will fail. Fix ApiService to call "kv/$key", parse the DTO and return its .value; remove the unnecessary inline on kvSetAsync.
File: plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt (lines ~61–72)
- suspend inline fun <reified T> kvGetAsync(key: String): T? {
- val response: T = convexClient.get("/kv/$key").body()
-
- return response
- }
+ suspend inline fun <reified T> kvGetAsync(key: String): T? {
+ // 404 -> exception; return null in that case
+ return runCatching { convexClient.get("kv/$key").body<KvGetResponse<T>>() }
+ .getOrNull()
+ ?.value
+ }
- suspend inline fun kvSetAsync(key: String, value: Any) {
- convexClient.put("/kv/$key") {
+ suspend fun kvSetAsync(key: String, value: Any) {
+ convexClient.put("kv/$key") {
contentType(ContentType.Application.Json)
setBody(value)
}
}Add this DTO (top-level) to support deserialization:
@kotlinx.serialization.Serializable
data class KvGetResponse<T>(val key: String, val value: T? = null)🤖 Prompt for AI Agents
In plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt
around lines 61–72, the GET uses a leading '/' which resets the base path and
tries to deserialize the server response directly into T; change the request
path to "kv/$key", deserialize into a top-level DTO KvGetResponse<T> (add
@kotlinx.serialization.Serializable data class KvGetResponse<T>(val key: String,
val value: T? = null) in the same package), and return the DTO.value from
kvGetAsync; also remove the unnecessary inline modifier from kvSetAsync and keep
the put call as-is (no leading '/'). Ensure you import
kotlinx.serialization.Serializable and adjust generics/return type to T?.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/backend/convex/players.ts (2)
66-76: Add returns validator togetPlayerByUuidPer project Convex guidelines, include a
returnsvalidator. If you don’t have a concrete object schema here, use an optional any as a stopgap.Apply:
export const getPlayerByUuid = internalQuery({ args: { uuid: v.string(), }, + returns: v.optional(v.any()), handler: async (ctx, args) => { return await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); }, });
171-197: Add returns validator toupdatePlayerJoinFunction returns
nullbut lacksreturns.Apply:
export const updatePlayerJoin = internalMutation({ args: { uuid: v.string(), }, + returns: v.null(), handler: async (ctx, args) => { const player = await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); @@ await ctx.db.patch(player._id, { lastJoinDate: new Date().toISOString(), stats: { ...player.stats, joinCount: currentJoinCount + 1, }, }); return null; }, });
🧹 Nitpick comments (8)
plugin/.env.example (3)
3-4: Document semantics for new HTTP_ vars (scheme, trailing slash, bearer) and provide safe placeholders.*Add brief inline guidance to prevent misconfiguration and accidental secret leaks; suggest a sample base URL and note that tokens are secrets and must not be logged.
Apply this diff:
-HTTP_AUTH_TOKEN= -HTTP_BASE_URL= +## HTTP client config +# Base URL must include scheme; prefer no trailing slash (code should not require it) +HTTP_BASE_URL=https://api.example.com +# Bearer token only; do not commit real values and avoid logging this env var +HTTP_AUTH_TOKEN=
1-2: Use explicit placeholders/comments for Axiom settings.Blank values are easy to miss. Provide examples and clarify that leaving them empty disables Axiom locally.
-AXIOM_API_TOKEN= -AXIOM_DATASET_NAME= +## Observability (Axiom) — leave empty to disable locally +AXIOM_API_TOKEN= # e.g., axiom_xxx +AXIOM_DATASET_NAME= # e.g., ssm-brawl-dev
1-4: Add trailing newline to satisfy dotenv-linter.Resolves EndingBlankLine warning.
AXIOM_API_TOKEN= AXIOM_DATASET_NAME= HTTP_AUTH_TOKEN= HTTP_BASE_URL= +plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/InstantIsoSerializer.kt (1)
21-24: Redundant @OptIn(ExperimentalTime) on method.You already opt-in at the object level; drop the method-level opt-in.
- @OptIn(ExperimentalTime::class) override fun deserialize(decoder: Decoder): Instant { return Instant.parse(decoder.decodeString()) }plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/BannedBySerializer.kt (1)
10-28: Consider case-insensitive "system" and basic UUID shape validation.Small hardening to avoid surprises from "System"/"SYSTEM" and obvious non-UUID typos.
- return if (string == "system") { + return if (string.equals("system", ignoreCase = true)) { BannedBy.System } else { BannedBy.Moderator(string) }Optionally validate
stringagainst a simple UUID regex before accepting asModerator.packages/backend/convex/schemas.ts (1)
3-17: Tighten schema: disallow unknown keys.Add
.strict()to the root and nested objects to catch payload typos early. Keeping Zod v3 as required for Convex.-export const playerDocumentSchema = z.object({ +export const playerDocumentSchema = z + .object({ isFirstTimeOnServer: z.boolean(), avatarUrl: z.string(), lastJoinDate: z.string(), stats: z.record(z.string(), z.union([z.string(), z.number(), z.boolean()])), - banData: z.nullable( - z.object({ - isBanned: z.boolean(), - reason: z.string(), - expiresAt: z.optional(z.string()), - bannedAt: z.string(), - bannedBy: z.string(), - }), - ), -}); + banData: z.nullable( + z + .object({ + isBanned: z.boolean(), + reason: z.string(), + expiresAt: z.optional(z.string()), + bannedAt: z.string(), + bannedBy: z.string(), + }) + .strict(), + ), + }) + .strict();packages/backend/convex/http.ts (2)
12-16: Remove double non-null assertion; rely on runtime check.
!!here is unnecessary and confusing. Let the throw enforce presence.-const token = process.env.HTTP_AUTH_TOKEN!!; +const token = process.env.HTTP_AUTH_TOKEN; if (!token) { throw new Error("HTTP_AUTH_TOKEN is not set"); }
123-131: Avoid returning raw error objects in JSON.They may leak internals. Log server-side and return a sanitized message.
- return c.json( - { - message: "Player document was not able to be fetched", - error, - }, - { status: 500 }, - ); + c.env.log?.error?.(error); + return c.json({ message: "Player document was not able to be fetched" }, { status: 500 });Apply similarly to save/delete handlers.
Also applies to: 146-152, 163-169
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.env(0 hunks).env.example(0 hunks)packages/backend/convex/http.ts(3 hunks)packages/backend/convex/players.ts(5 hunks)packages/backend/convex/schema.ts(1 hunks)packages/backend/convex/schemas.ts(1 hunks)plugin/.env.example(1 hunks)plugin/build.gradle.kts(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt(3 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/BannedBySerializer.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/InstantIsoSerializer.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt(1 hunks)plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt(1 hunks)
💤 Files with no reviewable changes (2)
- .env
- .env.example
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt
- packages/backend/convex/schema.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/convex/**/*.ts: ALWAYS use the new function syntax for Convex functions using function builders like query, mutation, internalQuery, internalMutation, action, and internalAction.
ALWAYS include argument and return validators for all Convex functions (query,internalQuery,mutation,internalMutation,action,internalAction). If a function does not return anything, includereturns: v.null()as its output validator.
UseinternalQuery,internalMutation, andinternalActionto register internal (private) functions, and usequery,mutation, andactionto register public functions. Do not use these to register sensitive internal functions as public.
You CANNOT register a function through theapiorinternalobjects; always use the appropriate function builder (query, mutation, etc.).
When usingctx.runQuery,ctx.runMutation, orctx.runActionto call a function in the same file, specify a type annotation on the return value to work around TypeScript circularity limitations.
Use theapiobject fromconvex/_generated/api.tsto call public functions and theinternalobject to call internal functions; respect function privacy and file-based routing.
Usectx.runQueryto call a query,ctx.runMutationto call a mutation, andctx.runActionfor actions. Do NOT try to pass the callee directly; always provide aFunctionReference.
Do NOT usefilterin Convex queries. Instead, define an index in the schema and usewithIndexin queries.
Convex queries do NOT support.delete(). Instead,.collect()the results and callctx.db.delete(row._id)for each.
Use.unique()to get a single document from a query; it throws if multiple matches exist.
When using async iteration on Convex queries, usefor await (const row of query)—do not use.collect()or.take(n).
Usectx.db.replacefor a full document replacement andctx.db.patchfor shallow merging. Both throw if the document does not exist.
Always add '"use node";' to the ...
Files:
packages/backend/convex/schemas.tspackages/backend/convex/http.tspackages/backend/convex/players.ts
🧠 Learnings (7)
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/schema.ts : Always define your schema in `convex/schema.ts` and always import schema definition functions from `convex/server`.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use the Convex-supported set of validators for argument and schema validation; avoid unsupported or deprecated ones as listed.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Define all Convex database tables using `defineTable` and specify indexes using `.index()` in schema definition files.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-21T01:47:24.597Z
Learnt from: BetrixDev
PR: BetrixDev/ssm-brawl#136
File: packages/backend/convex/schemas.ts:1-19
Timestamp: 2025-09-21T01:47:24.597Z
Learning: Convex projects must use Zod v3 (imported from "zod/v3") rather than Zod v4, as Convex currently only supports Zod v3. Do not suggest upgrading to Zod v4 features for Convex codebases.
Applied to files:
packages/backend/convex/schemas.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : Use `internalQuery`, `internalMutation`, and `internalAction` to register internal (private) functions, and use `query`, `mutation`, and `action` to register public functions. Do not use these to register sensitive internal functions as public.
Applied to files:
packages/backend/convex/players.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : When returning a null value from a query or mutation, always use the `v.null()` validator.
Applied to files:
packages/backend/convex/players.ts
📚 Learning: 2025-09-20T02:04:15.025Z
Learnt from: CR
PR: BetrixDev/ssm-brawl#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-09-20T02:04:15.025Z
Learning: Applies to **/convex/**/*.ts : ALWAYS include argument and return validators for all Convex functions (`query`, `internalQuery`, `mutation`, `internalMutation`, `action`, `internalAction`). If a function does not return anything, include `returns: v.null()` as its output validator.
Applied to files:
packages/backend/convex/players.ts
🧬 Code graph analysis (3)
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (1)
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/extensions/events.kt (1)
event(7-14)
packages/backend/convex/http.ts (1)
packages/backend/convex/schemas.ts (2)
PlayerDocument(19-19)playerDocumentSchema(3-17)
packages/backend/convex/players.ts (3)
packages/backend/convex/common.ts (1)
zInternalMutation(11-11)packages/backend/convex/schemas.ts (1)
playerDocumentSchema(3-17)packages/backend/convex/_generated/server.js (4)
internalMutation(59-59)internalMutation(59-59)internalQuery(39-39)internalQuery(39-39)
🪛 dotenv-linter (3.3.0)
plugin/.env.example
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: build-and-test
🔇 Additional comments (13)
plugin/build.gradle.kts (1)
46-46: Confirm kotlinx-datetime compatibility with Kotlin 2.2.0 and serialization 1.9.0.Looks right, but please verify no transitive conflicts and that we actually use kotlinx-datetime vs kotlin.time in plugin code.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/SuperSmashMobsBrawl.kt (1)
49-51: Good: DI bindings and explicit PlayerDocumentService.setup().Registration + setup call looks correct.
Ensure any buffered document writes flush on shutdown (e.g., add a
PlayerDocumentService.teardown()inonDisableAsync()if applicable).Also applies to: 66-66
packages/backend/convex/http.ts (3)
139-145: Remove unsafe cast; return is already correct.
as anydrops type safety; the mutation should accept the external UUID string.await c.env.runMutation(internal.players.savePlayerDocument, { - uuid: uuid as any, + uuid, document, });
156-161: Remove unsafe cast on DELETE.Same as PUT.
- await c.env.runMutation(internal.players.deletePlayerDocument, { uuid: uuid as any }); + await c.env.runMutation(internal.players.deletePlayerDocument, { uuid });
83-98: Re-fetch after join update; current manual patch risks stale data and has a precedence bug.Prefer fetching the updated player; if you keep the patch, fix
((joinCount || 0) + 1)parentheses.} else if (isJoinEvent) { await c.env.runMutation(internal.players.updatePlayerJoin, { uuid }); - - if (playerData) { - playerData = { - ...playerData, - lastJoinDate: new Date().toISOString(), - stats: { - ...playerData.stats, - joinCount: (playerData.stats.joinCount as number) || 0 + 1, - }, - }; - } + // Re-fetch to return fresh data + playerData = await c.env.runQuery(internal.players.getPlayerByUuid, { uuid }); }If you must keep the local patch:
-joinCount: (playerData.stats.joinCount as number) || 0 + 1, +joinCount: ((Number(playerData.stats.joinCount) || 0) + 1),plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/InstantIsoSerializer.kt (1)
41-48: Bug: nullable Instant deserialization returns decoder.decodeNull() instead of null.This will not return a proper null and can misbehave. Return null after
decodeNull().Apply:
- override fun deserialize(decoder: Decoder): Instant? { - return if (decoder.decodeNotNullMark()) { - Instant.parse(decoder.decodeString()) - } else { - decoder.decodeNull() - } - } + override fun deserialize(decoder: Decoder): Instant? { + if (decoder.decodeNotNullMark()) { + return Instant.parse(decoder.decodeString()) + } + decoder.decodeNull() + return null + }Likely an incorrect or invalid review comment.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt (2)
39-51: Nice: iterate over a snapshot during periodic flushCopying
documents.entries.toList()avoids concurrent modification and you added per-entry error logging. Good.
56-67: Don’t block the server thread; removerunBlockingfrom join handler
runBlockingcan stall the main thread and it doesn’t even wait for the innerlaunch. Launch on the plugin scope, do IO off-thread, then write back on main.Apply:
- listeners.add( - event<PlayerJoinEvent>(priority = EventPriority.LOWEST) { - runBlocking { - plugin.launch { - val document = - withContext(Dispatchers.IO) { - api.playersGetDocumentAsync(player, true) - } - - documents[player] = document - } - } - } - ) + listeners.add( + event<PlayerJoinEvent>(priority = EventPriority.LOWEST) { + plugin.launch { + val document = withContext(Dispatchers.IO) { + api.playersGetDocumentAsync(player, true) + } + documents[player] = document + } + } + )packages/backend/convex/players.ts (4)
10-44: Fix UUID type mismatch, add returns validator, patch by internal_id, and implement unban handling
- Args incorrectly validate
uuidas Convex Id while you compare against external UUIDs.- Missing
returnsviolates Convex guidelines.- You patch
args.uuidinstead of the player’s internal_id.- Unban path is missing when
banDatais absent or explicitly not banned.Apply:
export const savePlayerDocument = zInternalMutation({ args: { - uuid: zid("players"), + uuid: v.string(), // external player UUID document: playerDocumentSchema, }, + returns: v.null(), handler: async (ctx, args) => { - if (args.document.banData) { + // Resolve internal player doc by external UUID + const player = await ctx.db + .query("players") + .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) + .unique(); + if (!player) throw new Error("Player not found"); + + const existingBan = await ctx.db + .query("playerBans") + .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) + .unique(); + + const shouldBan = !!args.document.banData?.isBanned; + if (shouldBan) { - const existingBan = await ctx.db - .query("playerBans") - .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) - .unique(); - if (existingBan) { await ctx.db.patch(existingBan._id, { reason: args.document.banData.reason, expiresAt: args.document.banData.expiresAt, }); } else { await ctx.db.insert("playerBans", { uuid: args.uuid, reason: args.document.banData.reason, expiresAt: args.document.banData.expiresAt, bannedAt: args.document.banData.bannedAt, bannedBy: args.document.banData.bannedBy, }); } - } + } else if (existingBan) { + await ctx.db.delete(existingBan._id); // unban + } - return await ctx.db.patch(args.uuid, { + await ctx.db.patch(player._id, { lastJoinDate: args.document.lastJoinDate, stats: args.document.stats, avatarUrl: args.document.avatarUrl, }); + return null; }, });
46-64: Delete mutation should accept external UUID string and return null
args.uuidis validated asv.id("players")but then compared to the external"uuid"field, so the lookup will fail. Also missingreturns.Apply:
export const deletePlayerDocument = internalMutation({ args: { - uuid: v.id("players"), + uuid: v.string(), }, + returns: v.null(), handler: async (ctx, args) => { const player = await ctx.db .query("players") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); if (!player) { throw new Error("Player not found"); } await ctx.db.delete(player._id); - return null; + return null; }, });
78-88: Add returns validator togetPlayerBanByUuidSame rationale as above.
Apply:
export const getPlayerBanByUuid = internalQuery({ args: { uuid: v.string(), }, + returns: v.optional(v.any()), handler: async (ctx, args) => { return await ctx.db .query("playerBans") .withIndex("by_uuid", (q) => q.eq("uuid", args.uuid)) .unique(); }, });
3-3: Incorrect — keepzod/v3imports for ConvexConvex requires Zod v3 and importing from
"zod/v3"is correct; the code already uses"zod/v3"(packages/backend/convex/schemas.ts, packages/backend/convex/players.ts). Root package.json has no zod — ensure zod v3 is declared in the package's package.json or workspace deps.Likely an incorrect or invalid review comment.
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt (1)
6-8: Incorrect — repository consistently uses kotlin.time.Instant; do not switch to kotlinx.datetime herePlayerDocument.kt and InstantIsoSerializer.kt (which also contains NullableInstantIsoSerializer) import and use kotlin.time.Instant and @OptIn(ExperimentalTime::class); applying the suggested change would create type mismatches. Either keep the current kotlin.time.* usage or convert serializers and all call sites to kotlinx.datetime.Instant in one coordinated change.
Files: plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/models/player/PlayerDocument.kt, plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/serializers/InstantIsoSerializer.kt.
Likely an incorrect or invalid review comment.
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), 10000); | ||
|
|
||
| const response = await fetch(`https://playerdb.co/api/player/minecraft/${args.uuid}`, { | ||
| signal: controller.signal, | ||
| }).finally(() => clearTimeout(timeout)); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Good timeout/abort handling around fetch
The AbortController with a 10s timeout and cleanup via finally(clearTimeout) is solid. 4xx vs 5xx handling is also clear.
Check for any lingering casts like uuid as any in the HTTP layer that depended on the old validator:
Also applies to: 114-121
🏁 Script executed:
#!/bin/bash
rg -nP 'uuid\s+as\s+any|playersSaveDocument|savePlayerDocument' -S packagesLength of output: 382
Good timeout/abort handling around fetch — remove uuid as any casts in HTTP layer
AbortController/timeout are fine; the HTTP layer still bypasses the new validator with uuid as any—remove those casts and pass a properly-validated/typed uuid to the internal mutations.
- packages/backend/convex/http.ts — lines 139–140: call to internal.players.savePlayerDocument using
uuid: uuid as any. - packages/backend/convex/http.ts — line 159: call to internal.players.deletePlayerDocument using
uuid: uuid as any. - packages/backend/convex/players.ts — line 10:
export const savePlayerDocument = zInternalMutation({ ... })(verify mutation signature/validator).
Remove the as any casts and align the HTTP inputs with the mutation's validated types.
| AXIOM_API_TOKEN= | ||
| AXIOM_DATASET_NAME= | ||
| HTTP_AUTH_TOKEN= | ||
| HTTP_BASE_URL= No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Ensure env validation and usage are updated across the codebase.
If you validate env with Zod (recommended), add these keys and enforce formats (URL, non-empty token).
Example (outside this file):
import { z } from "zod";
export const EnvSchema = z.object({
AXIOM_API_TOKEN: z.string().optional(), // empty disables
AXIOM_DATASET_NAME: z.string().optional(),
HTTP_BASE_URL: z.string().url(), // include scheme
HTTP_AUTH_TOKEN: z.string().min(1), // non-empty when required
});Run to verify references and headers:
🏁 Script executed:
#!/bin/bash
# Find usages and check Authorization header assembly
rg -n -C2 -g '!**/dist/**' -e 'process\.env\.(HTTP_BASE_URL|HTTP_AUTH_TOKEN)' -e 'Authorization' -e 'Bearer 'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Top-level files:"
ls -la | sed -n '1,120p' || true
echo
echo "Searching repository (no ignores, include hidden/binary) for env keys, Authorization header, and 'Bearer ':"
rg -uuu -n -C2 \
-e 'process\.env\.(HTTP_BASE_URL|HTTP_AUTH_TOKEN|AXIOM_API_TOKEN|AXIOM_DATASET_NAME)' \
-e 'HTTP_BASE_URL' -e 'HTTP_AUTH_TOKEN' -e 'AXIOM_API_TOKEN' -e 'AXIOM_DATASET_NAME' \
-e 'Authorization' -e 'Bearer ' || true
echo
echo "Searching for zod / env validation / dotenv usage:"
rg -uuu -n -C2 -e '\bzod\b' -e 'EnvSchema' -e 'z\.object' -e 'dotenv' || true
echo
echo "List .env* files found:"
rg -uuu --files -g '*.env*' || true
echo
echo "Show plugin/.env.example (first 120 lines) if present:"
if [ -f plugin/.env.example ]; then sed -n '1,120p' plugin/.env.example || true; else echo "plugin/.env.example not found"; fi
echo
echo "Show package.json lines mentioning zod or dotenv (if package.json exists):"
if [ -f package.json ]; then rg -n '"zod"|"dotenv"' package.json || true; else echo "package.json not found"; fiLength of output: 19444
Enforce and centralize env validation for both Node and Kotlin runtimes
-
Findings (where these vars are used): plugin/.env.example; packages/backend/convex/http.ts (process.env.HTTP_AUTH_TOKEN — throws if missing); plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/ApiService.kt (System.getenv HTTP_AUTH_TOKEN & HTTP_BASE_URL — errors if base URL missing; conditionally adds Authorization); plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/AxiomLoggerHandler.kt (AXIOM_API_TOKEN, AXIOM_DATASET_NAME — conditional Authorization); docker-compose.yml (passes AXIOM_* into plugin).
-
Actionable changes:
- Node (packages/backend): add a Zod EnvSchema and parse at startup to fail fast and enforce formats. Minimal example:
import { z } from "zod";
export const EnvSchema = z.object({
AXIOM_API_TOKEN: z.string().optional(),
AXIOM_DATASET_NAME: z.string().optional(),
HTTP_BASE_URL: z.string().url().optional(),
HTTP_AUTH_TOKEN: z.string().min(1),
});
export const env = EnvSchema.parse(process.env); - Kotlin (plugin): add explicit startup validation (in ApiService or application entry) — require/validate HTTP_BASE_URL (must include scheme) and enforce token non-empty when the runtime expects to use it; treat AXIOM_* as optional but validate when present.
- Update plugin/.env.example and docs to mark which vars are required vs optional and their expected formats.
- Verify docker-compose/CI provide required vars or the service fails fast with clear messages.
- Node (packages/backend): add a Zod EnvSchema and parse at startup to fail fast and enforce formats. Minimal example:
-
Rationale: some checks exist but are inconsistent across runtimes; centralizing/standardizing validation prevents runtime surprises and inconsistent header behavior.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
plugin/.env.example around lines 1 to 4: the repository lacks centralized,
consistent env validation across Node and Kotlin runtimes leading to runtime
surprises; add explicit validations: in Node (packages/backend) create and parse
a Zod EnvSchema at startup that enforces HTTP_AUTH_TOKEN as non-empty,
HTTP_BASE_URL as a valid URL (optional if allowed), and AXIOM_* as optional
strings; in Kotlin (plugin) add startup checks (in ApiService or app entry) that
validate HTTP_BASE_URL includes a scheme and HTTP_AUTH_TOKEN is non-empty when
required, and validate AXIOM_* only if present; update plugin/.env.example and
docs to mark required vs optional vars and expected formats; ensure
docker-compose.yml and CI supply required vars or the service fails fast with
clear error messages.
| private val documents = hashMapOf<Player, PlayerDocument>() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Consider keying the cache by UUID instead of Player objects
Avoids retaining Player instances and simplifies lookups across reloads; safer keys for maps.
Example refactor sketch:
- private val documents = hashMapOf<Player, PlayerDocument>()
+ private val documents = hashMapOf<java.util.UUID, PlayerDocument>()
@@
- documents[player] = document
+ documents[player.uniqueId] = document
@@
- documents.remove(player)?.let {
+ documents.remove(player.uniqueId)?.let {
plugin.launch {
- withContext(Dispatchers.IO) { api.playersSetDocumentAsync(player, it) }
+ withContext(Dispatchers.IO) { api.playersSetDocumentAsync(player, it) }
}
}
@@
- val entries = documents.entries.toList()
+ val entries = documents.entries.toList()
@@
- fun getPlayerDocument(player: Player): PlayerDocument =
- documents[player] ?: error("Player document for ${player.name} not found in cache.")
+ fun getPlayerDocument(player: Player): PlayerDocument =
+ documents[player.uniqueId]
+ ?: error("Player document for ${player.name} not found in cache.")Scan for other runBlocking or direct documents[...] mutations off main:
Also applies to: 70-77, 81-94, 96-97
🏁 Script executed:
#!/bin/bash
rg -n 'runBlocking|documents\\[' plugin/src/main/kotlin -S -C2Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Show PlayerDocumentService.kt (first 300 lines) ==="
sed -n '1,300p' plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt || true
echo
echo "=== occurrences of 'documents[' and 'documents.' in that file ==="
rg -n -C2 --hidden -S 'documents\[' plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt || true
rg -n -C2 --hidden -S '\bdocuments\.' plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt || true
echo
echo "=== occurrences of 'documents[' across plugin/src/main/kotlin ==="
rg -n -C2 -S 'documents\[' plugin/src/main/kotlin || true
echo
echo "=== occurrences of runBlocking across plugin/src/main/kotlin ==="
rg -n -C2 -S 'runBlocking' plugin/src/main/kotlin || true
echo
echo "=== search for map mutation methods on 'documents' (put/remove/entries/get) across plugin ==="
rg -n -C2 -S 'documents\.(put|remove|entries|get)\b' plugin/src/main/kotlin || trueLength of output: 10872
Key documents by UUID and update usages in PlayerDocumentService.kt
Avoid retaining Player instances; change the cache to use UUID keys and update all local usages in PlayerDocumentService.kt.
- Use a thread-safe map keyed by UUID (e.g. ConcurrentHashMap<UUID, PlayerDocument>).
- Replace mutations/lookups: documents[player] -> documents[player.uniqueId] (setup + PlayerJoinEvent + PlayerQuitEvent removal).
- Update periodic persistence and teardown loops to iterate (uuid, doc) and resolve uuid -> Player via plugin.server.getPlayer(uuid) (skip if null) before calling api.playersSetDocumentAsync — or adjust the API to accept UUIDs if available.
- Keep getPlayerDocument(player: Player) signature but lookup by player.uniqueId and preserve the error message.
- Ensure thread-safety (ConcurrentHashMap or single-threaded dispatcher) because the map is mutated from coroutines.
PlayerDocumentService.kt: update the setup population, repeatingTask persistence loop, PlayerJoinEvent, PlayerQuitEvent, teardown flush, and getPlayerDocument accordingly.
🤖 Prompt for AI Agents
In
plugin/src/main/kotlin/dev/betrix/superSmashMobsBrawl/services/PlayerDocumentService.kt
around lines 25-26, the documents cache currently keys by Player and must be
changed to key by UUID to avoid retaining Player objects and to be thread-safe;
replace hashMapOf<Player, PlayerDocument>() with a ConcurrentHashMap<UUID,
PlayerDocument>, update every access from documents[player] to
documents[player.uniqueId], populate the map on setup using player.uniqueId,
change PlayerJoinEvent and PlayerQuitEvent to insert/remove by uniqueId, update
the periodic persistence and teardown loops to iterate entries as (uuid, doc)
and resolve the live Player via plugin.server.getPlayer(uuid) (skip if null)
before calling api.playersSetDocumentAsync, keep the getPlayerDocument(player:
Player) signature but perform lookup by player.uniqueId and preserve the
existing error message, and ensure all mutations use the ConcurrentHashMap (or a
single-threaded dispatcher) to guarantee thread-safety from coroutines.
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores