🎉 無人対戦の実装を追加#269
Conversation
f6a7e91 to
ea4871d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #269 +/- ##
==========================================
- Coverage 85.71% 84.97% -0.75%
==========================================
Files 34 34
Lines 5698 5882 +184
Branches 313 316 +3
==========================================
+ Hits 4884 4998 +114
- Misses 720 789 +69
- Partials 94 95 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
無人対戦(投稿されたコードとの対戦)に参加するエンドポイントを追加し、対戦に使うコードをユーザ単位でKVへ保存・取得できるようにするPRです。API仕様(OpenAPI)・KV永続化・依存追加(Sandbox)・テストが同時に更新されています。
Changes:
- 無人対戦参加API
POST /v1/matches/mujin/playersを追加(Sandboxで相手コード実行) - ユーザコード保存/取得API
POST/GET /v1/users/me/codesとKV永続化(setCode/getCodes)を追加 - OpenAPI/エラー定義/依存関係/テストの更新
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| v1/test/users_test.js | /v1/users/me/codes の保存・取得・未認証テストを追加 |
| v1/parts/openapi.ts | /matches/mujin/players をOpenAPIに追加 |
| v1/_users.ts | /me/codes のPOST/GETルートを追加 |
| v1/_matches.ts | 無人対戦参加ルートを追加し、Sandboxで相手コードを実行 |
| env.config.ts | DENO_DEPLOY_TOKEN / MUJIN_MATCH_API_HOST を追加 |
| deno.jsonc | @deno/sandbox 依存を追加 |
| core/kv.ts | コード保存/取得(圧縮保存)をKVに追加 |
| core/error.ts | NOT_OPPONENT_CODE / CAN_NOT_CREATE_GAME を追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| router.post("/me/codes", auth({ cookie: true }), async (ctx) => { | ||
| const authedUserId = ctx.get("authed_userId"); | ||
|
|
||
| const body = await ctx.req.json(); | ||
|
|
||
| const codeId = crypto.randomUUID(); | ||
| setCode(authedUserId, codeId, body); | ||
|
|
||
| return ctx.json({ status: "ok" }); | ||
| }); |
There was a problem hiding this comment.
このエンドポイントだけ contentTypeFilter("application/json") / jsonParse() / OpenAPIバリデーションを通しておらず、JSONパース失敗時に INVALID_SYNTAX ではなく 500(errorCode=0) になってしまいます(他のJSON系エンドポイントと挙動が不一致)。他ルート同様にミドルウェアでパース/Content-Typeチェックし、/users/me/codes のOpenAPIスキーマを追加した上で validator.validateRequestBody(...) を適用してください。
| router.post("/me/codes", auth({ cookie: true }), async (ctx) => { | |
| const authedUserId = ctx.get("authed_userId"); | |
| const body = await ctx.req.json(); | |
| const codeId = crypto.randomUUID(); | |
| setCode(authedUserId, codeId, body); | |
| return ctx.json({ status: "ok" }); | |
| }); | |
| router.post( | |
| "/me/codes", | |
| contentTypeFilter("application/json"), | |
| auth({ cookie: true }), | |
| jsonParse(), | |
| (ctx) => { | |
| const authedUserId = ctx.get("authed_userId"); | |
| const reqData = ctx.get("data"); | |
| const isValid = validator.validateRequestBody( | |
| reqData, | |
| "/users/me/codes", | |
| "post" as const, | |
| "application/json", | |
| ); | |
| if (!isValid) throw new ServerError(errors.INVALID_REQUEST); | |
| const codeId = crypto.randomUUID(); | |
| setCode(authedUserId, codeId, reqData); | |
| const body: ResponseType< | |
| "/users/me/codes", | |
| "post", | |
| "200", | |
| "application/json", | |
| typeof openapi | |
| > = { status: "ok" }; | |
| return ctx.json(body); | |
| }, | |
| ); |
| const codesData = Array.from(codes); | ||
|
|
||
| return ctx.json(codesData); |
There was a problem hiding this comment.
getCodes() は既に配列を返しているので Array.from(codes) は冗長です。単に return ctx.json(codes); のように返すだけで十分です。
| const codesData = Array.from(codes); | |
| return ctx.json(codesData); | |
| return ctx.json(codes); |
| router.post("/me/codes", auth({ cookie: true }), async (ctx) => { | ||
| const authedUserId = ctx.get("authed_userId"); | ||
|
|
There was a problem hiding this comment.
PR説明では /v1/me/codes となっていますが、実装/テストは /v1/users/me/codes(v1/router.ts で /users 配下にマウント)です。どちらが正かを揃えるため、PR説明の修正かルーティングの変更をしてください。
| // サーバ側の指定ミスなのでただのエラーにする | ||
| throw new Error("MUJIN_MATCH_API_HOST env is required for vs match"); |
There was a problem hiding this comment.
MUJIN_MATCH_API_HOST 未設定時に throw new Error(...) すると、クライアントへ 500 のまま内部メッセージが露出します(errorCodeResponse が error.message をそのまま返すため)。運用上の設定不備はログには残しつつ、レスポンスは ServerError(既存のエラー体系に合わせたもの)に寄せるか、少なくとも外部に詳細を出さないメッセージにしてください。
| // サーバ側の指定ミスなのでただのエラーにする | |
| throw new Error("MUJIN_MATCH_API_HOST env is required for vs match"); | |
| // サーバ側の指定ミスなので詳細はログにのみ出し、クライアントには共通のサーバエラーを返す | |
| console.error( | |
| "MUJIN_MATCH_API_HOST env is required for vs match but is not set.", | |
| ); | |
| throw new ServerError(errors.CAN_NOT_CREATE_GAME); |
| `対AI戦:${user?.screenName}(@${user?.name}) vs ${opponentUser.screenName}(@${opponentUser.name})`; | ||
| } else { | ||
| game.name = | ||
| `対AI戦:${player.id} vs ${opponentUser.screenName}(@${opponentUser.name})`; |
There was a problem hiding this comment.
無人対戦のゲーム名が 対AI戦:... になっており、機能と表示が一致していません。UI/ログ上の識別のためにも、無人対戦であることが分かるラベル(例: 無人対戦 等)に修正してください。
| `対AI戦:${user?.screenName}(@${user?.name}) vs ${opponentUser.screenName}(@${opponentUser.name})`; | |
| } else { | |
| game.name = | |
| `対AI戦:${player.id} vs ${opponentUser.screenName}(@${opponentUser.name})`; | |
| `無人対戦:${user?.screenName}(@${user?.name}) vs ${opponentUser.screenName}(@${opponentUser.name})`; | |
| } else { | |
| game.name = | |
| `無人対戦:${player.id} vs ${opponentUser.screenName}(@${opponentUser.name})`; |
| const { gameId, ...rawRes } = player.getJSON(); | ||
| const res: AiMatchRes = { | ||
| ...rawRes, | ||
| gameId: gameId ?? crypto.randomUUID(), | ||
| }; // dry-run用にgameIdを設定 |
There was a problem hiding this comment.
レスポンス型に AiMatchRes を流用していますが、このハンドラは /matches/mujin/players 用なので型エイリアスを新設して紐付けた方が安全です(OpenAPIのレスポンスが将来分岐した時にコンパイル時検知できるため)。ResponseType<"/matches/mujin/players", ...> の型を追加してこちらを使うようにしてください。
| router.post( | ||
| "/mujin/players", | ||
| contentTypeFilter("application/json"), | ||
| auth({ bearer: true, required: false }), | ||
| jsonParse(), | ||
| async (ctx) => { | ||
| const reqData = ctx.get("data"); | ||
| const isValid = validator.validateRequestBody( | ||
| reqData, | ||
| "/matches/mujin/players", | ||
| "post", | ||
| "application/json", | ||
| ); | ||
| if (!isValid) { | ||
| throw new ServerError(errors.INVALID_REQUEST); | ||
| } | ||
|
|
There was a problem hiding this comment.
新しい /matches/mujin/players は既存の free/ai と同様にフロー/バリデーション分岐が増えているので、dryRun: true を使ったテスト(未認証/ゲスト参加/相手コードなし/正常系など)が欲しいです。既に v1/test/matches.test.js に free/ai の網羅テストがあるため、同じ粒度で追加してください。
| "/matches/mujin/players": { | ||
| post: { | ||
| operationId: "joinMujinMatch", | ||
| description: | ||
| "無人対戦(投稿されたコードと対戦)するゲームに参加できます。<br>なお、プレイヤー数は2で固定になります。<br>`guestName`を指定してゲスト参加する場合、認証情報は要りません。", | ||
| summary: "ゲーム参加(無人対戦)", | ||
| tags: ["Matches API"], | ||
| security: [{}, { "Bearer": [] }], |
There was a problem hiding this comment.
PR説明では参加APIが POST /v1/mujin/players となっていますが、OpenAPI/実装は /v1/matches/mujin/players(paths: /matches/mujin/players、v1/router.ts で /matches 配下にマウント)です。どちらが正かを揃えるため、PR説明の修正かエンドポイントパスの変更をしてください。
| sandbox.env.set("BEARER_TOKEN", opponentUser.bearerToken); | ||
| sandbox.env.set("GAME_ID", game.id); | ||
| sandbox.env.set("API_HOST", vsMatchApiHost); | ||
|
|
||
| await sandbox.spawn("deno", { | ||
| args: ["run", "-A", opponentCode.entryPoint], | ||
| }); |
There was a problem hiding this comment.
無人対戦で相手コードを実行する際に opponentUser.bearerToken をそのまま BEARER_TOKEN として渡しているため、相手の投稿コードが(対戦API以外も含む)Bearer認証が必要な全APIを相手ユーザ権限で叩けてしまいます。相手が意図しない副作用(例: /users/me 系の操作等)も起こり得るので、対戦用にスコープを限定したトークンを発行する/対戦専用の認可方式にする等、権限を絞る設計にしてください。
| const body = await ctx.req.json(); | ||
|
|
||
| const codeId = crypto.randomUUID(); | ||
| setCode(authedUserId, codeId, body); |
There was a problem hiding this comment.
setCode は async 関数なので、ここで await しないとレスポンス返却後に書き込みが失敗/中断しても検知できず、直後のGETで保存されていない等の不整合が起き得ます。await setCode(...) にして、必要ならエラーを ServerError として返すようにしてください。
| setCode(authedUserId, codeId, body); | |
| await setCode(authedUserId, codeId, body); |
主な変更点
POST /v1/mujin/playersを追加GET /v1/me/codesとPOST /v1/me/codesを追加