Repository navigation
Generate game-owned Devvit starter targets - #9
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a game-owned Reddit Devvit app root template ( ChangesReddit Devvit App Root Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Express as Devvit Express Server
participant Handler as handleBridgeRequest
participant Redis
participant Reddit as Devvit Reddit API
Client->>Express: POST /api/mpgd/bridge
Express->>Express: assertBridgeRequest(body)
Express->>Handler: dispatch(method, payload)
alt leaderboard.submitScore
Handler->>Redis: acquire lock, zScore, WATCH/MULTI/EXEC
Redis-->>Handler: submitted/failed result
else storage.load / storage.save
Handler->>Redis: GET/SET stored JSON
Redis-->>Handler: value / saved status
else identity.getPlayer
Handler->>Reddit: getCurrentUsername()
Reddit-->>Handler: displayName / fallback
end
Handler-->>Express: BridgeResponse
Express-->>Client: response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/cli/src/index.ts (1)
23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate target list instead of reusing
defaultMatrixTargets.
defaultMatrixTargets(Line 23) was updated to'web,ait,reddit', but the "Next steps" guidance at Line 595 hardcodes the same list as a separate string literal. Future changes todefaultMatrixTargets(adding/removing a target) won't automatically propagate here, risking guidance drifting out of sync with actual CLI defaults.♻️ Reuse the constant
- `--targets-file ${path.join(appDir, 'mpgd.targets.json')}`, - '--targets web,ait,reddit', + `--targets-file ${path.join(appDir, 'mpgd.targets.json')}`, + `--targets ${defaultMatrixTargets}`,Also applies to: 592-596
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/index.ts` around lines 23 - 24, The “Next steps” guidance is duplicating the target list instead of reusing the shared default, so update the logic in index.ts to reference defaultMatrixTargets wherever the matrix targets are shown or composed. In the section that builds the guidance text near the Next steps message, replace the hardcoded web,ait,reddit string with the defaultMatrixTargets constant so future target changes stay in sync. Keep the fix localized to the existing Next steps/help text path and the defaultMatrixTargets symbol.packages/cli/templates/phaser-game/apps/target-devvit/src/server/index.ts (1)
609-634: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant parse→stringify→parse round trip for the oRPC path.
request.bodyis already parsed byexpress.json()and then re-serialized here viaJSON.stringify(request.body ?? null)just to be re-parsed inside the oRPC fetch handler. This is a minor inefficiency on a hot path but not incorrect; could be avoided by using a raw body parser for this specific endpoint and forwarding the raw bytes instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/templates/phaser-game/apps/target-devvit/src/server/index.ts` around lines 609 - 634, The oRPC request conversion in expressRequestToFetchRequest is doing an unnecessary parse→stringify→parse round trip by re-serializing request.body after express.json() already parsed it. Update the server path that builds the Request to preserve and forward the raw request bytes for this endpoint instead of using JSON.stringify(request.body ?? null), so the fetch handler can consume the original body directly without re-parsing.packages/cli/templates/phaser-game/package.json (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving
@mpgd/clitodevDependencies.
@mpgd/cliis only used for thebuild:devvit*/smoke:devvitscripts (build/test tooling), not runtime game code. As per README guidance that Target builds use the mpgd CLI because it knows how to turn this game into platform-specific artifacts, this is a dev-time tool; placing it independenciesrather thandevDependenciesis a minor convention deviation that could affect production install footprint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/templates/phaser-game/package.json` at line 39, Move `@mpgd/cli` from dependencies to devDependencies in the phaser-game package.json template, since it is only referenced by the build:devvit* and smoke:devvit tooling. Update the package manifest entry so the CLI remains available for build-time scripts while avoiding treating it as a runtime dependency, and keep the existing script references unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/templates/phaser-game/agent/brief.md`:
- Line 19: Replace the kit-specific checkout example in the Phaser agent brief
with the standalone project flow used by the README. Update the command shown in
the brief to use the mpgd CLI via pnpm exec with --kit-path instead of pnpm
--dir ../mpgd-kit, and keep the rest of the build-all arguments aligned with the
existing target/variant example.
In `@packages/cli/templates/phaser-game/apps/target-devvit/src/server/index.ts`:
- Around line 337-359: The custom parseBridgeRequest logic is bypassing the
canonical BridgeRequest validation and fabricating default payload/meta values,
so bridge traffic can diverge from the oRPC path. Remove or replace this manual
parser in server/index.ts and route requests through
createBridgeRpcRouter/createBridgeRpcFetchHandler so assertBridgeRequest handles
runtime validation and invalid inputs become INVALID_BRIDGE_REQUEST responses.
Keep handleBridgeRequest as the downstream handler, but ensure it only receives
validated BridgeRequest data from the shared bridge schema path.
- Around line 575-589: `currentDisplayName` is reading `context.username`, but
that field is not available on the exposed Devvit context; update this helper to
use the Reddit client instead. Make `currentDisplayName` async, fetch the name
via `reddit.getCurrentUsername()`, and fall back to the passed display name or
player id only when the Reddit username is empty. Keep `currentPlayerId`
unchanged and make sure any callers of `currentDisplayName` are updated to await
the async result.
---
Nitpick comments:
In `@packages/cli/src/index.ts`:
- Around line 23-24: The “Next steps” guidance is duplicating the target list
instead of reusing the shared default, so update the logic in index.ts to
reference defaultMatrixTargets wherever the matrix targets are shown or
composed. In the section that builds the guidance text near the Next steps
message, replace the hardcoded web,ait,reddit string with the
defaultMatrixTargets constant so future target changes stay in sync. Keep the
fix localized to the existing Next steps/help text path and the
defaultMatrixTargets symbol.
In `@packages/cli/templates/phaser-game/apps/target-devvit/src/server/index.ts`:
- Around line 609-634: The oRPC request conversion in
expressRequestToFetchRequest is doing an unnecessary parse→stringify→parse round
trip by re-serializing request.body after express.json() already parsed it.
Update the server path that builds the Request to preserve and forward the raw
request bytes for this endpoint instead of using JSON.stringify(request.body ??
null), so the fetch handler can consume the original body directly without
re-parsing.
In `@packages/cli/templates/phaser-game/package.json`:
- Line 39: Move `@mpgd/cli` from dependencies to devDependencies in the
phaser-game package.json template, since it is only referenced by the
build:devvit* and smoke:devvit tooling. Update the package manifest entry so the
CLI remains available for build-time scripts while avoiding treating it as a
runtime dependency, and keep the existing script references unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ec8c94a-e819-46b4-9643-cf05fcfc145f
📒 Files selected for processing (20)
.sampo/changesets/game-owned-devvit-template.mdREADME.mddocs/AGENTIC_GAME_WORKFLOW.mddocs/GAME_DEVELOPMENT.mddocs/PUBLIC_RELEASE_CHECKLIST.mdpackages/cli/src/index.tspackages/cli/templates/phaser-game/.gitignorepackages/cli/templates/phaser-game/README.mdpackages/cli/templates/phaser-game/agent/acceptance.mdpackages/cli/templates/phaser-game/agent/brief.mdpackages/cli/templates/phaser-game/agent/game-manifest.jsonpackages/cli/templates/phaser-game/apps/target-devvit/README.mdpackages/cli/templates/phaser-game/apps/target-devvit/devvit.jsonpackages/cli/templates/phaser-game/apps/target-devvit/package.jsonpackages/cli/templates/phaser-game/apps/target-devvit/src/server/index.tspackages/cli/templates/phaser-game/apps/target-devvit/tsconfig.jsonpackages/cli/templates/phaser-game/apps/target-devvit/vite.server.config.tspackages/cli/templates/phaser-game/mpgd.targets.jsonpackages/cli/templates/phaser-game/package.jsonpackages/cli/templates/phaser-game/pnpm-workspace.yaml
![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: 56cc05a8ea
ℹ️ 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".
| @@ -0,0 +1,12 @@ | |||
| node_modules/ | |||
There was a problem hiding this comment.
When @mpgd/cli is packed, npm omits files named .gitignore even though templates is listed in files (checked with npm pack --dry-run --json from packages/cli, and this file was absent). As a result, starters created from the published package will not get these ignore rules, so the new Devvit build outputs under apps/*/dist, artifacts, and .mpgd.targets.generated.json can be accidentally committed despite the template adding protections here. Consider templating this as a non-dotfile name and writing it out as .gitignore during generation.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,40 @@ | |||
| { | |||
| "$schema": "https://developers.reddit.com/schema/config-file.v1.json", | |||
| "name": "__GAME_NAME__", | |||
There was a problem hiding this comment.
For generated games whose directory basename is shorter than 3 or longer than 16 characters, this placeholder is filled with a value that passes assertValidGameName but violates Devvit's required name constraints, so pnpm devvit:init/upload rejects the app before playtest; the Devvit config docs require the app name to be 3-16 characters and match the lowercase/hyphen slug pattern (https://developers.reddit.com/docs/capabilities/devvit-web/devvit_web_configuration#required-properties). Please validate the game basename against the Devvit limit or derive a separate valid Devvit slug for this field.
Useful? React with 👍 / 👎.
| ] | ||
| }, | ||
| "scripts": { | ||
| "dev": "pnpm dev:prepare && pnpm build:server -- --watch", |
There was a problem hiding this comment.
pnpm devvit:playtest relies on Devvit's scripts.dev command; the Devvit config docs describe this as the command devvit playtest uses to build or watch artifacts (https://developers.reddit.com/docs/capabilities/devvit-web/devvit_web_configuration#scripts-configuration). This command runs the full client build only once via dev:prepare and then watches only vite.server.config.ts, so editing the generated game's Phaser client under src/** during an active playtest leaves dist/client stale until the user stops and restarts playtest. Please run a client vite build --watch or equivalent alongside the server watcher.
Useful? React with 👍 / 👎.
|
Addressed the latest review feedback in
Validation run:
Re-review requested when you have a moment. |
|
@coderabbitai review Review feedback has been addressed in |
|
@codex review Please re-review the latest head ( |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a48c589a
ℹ️ 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".
| "scripts": { | ||
| "dev": "devvit playtest", | ||
| "dev:prepare": "pnpm --dir ../.. build:target reddit staging", | ||
| "dev:client": "APP_TARGET=reddit APP_VERSION=0.0.0-dev BUILD_ID=devvit-watch pnpm --dir ../.. exec vite build --mode staging --watch --outDir apps/target-devvit/dist/client", |
There was a problem hiding this comment.
In the in-repo Devvit target, ../.. is the repository root, not the Phaser app; the Vite config and index.html for this target live under apps/game-phaser. When apps/target-devvit/devvit.json invokes pnpm dev:watch, this command runs Vite from the root and either fails to resolve an entry or watches/builds the wrong directory, so Devvit playtest no longer rebuilds the game client after the initial dev:prepare.
Useful? React with 👍 / 👎.
| "version": "0.0.0", | ||
| "type": "module", | ||
| "scripts": { | ||
| "init": "devvit init --config devvit.json", |
There was a problem hiding this comment.
For generated games following the new README's pnpm devvit:init step, this script runs devvit init --config devvit.json. I checked the Devvit 0.13 CLI docs: they document devvit new for creating projects and devvit upload [--bump ...] for uploading/registering app versions, and Reddit's own Bolt starter implements its devvit:init script as devvit upload --copy-paste; there is no documented init --config path, so first-time generated starters can fail before users can playtest or upload. Use the supported upload/link flow from the target app directory instead.
Useful? React with 👍 / 👎.
|
Addressed the latest Devvit playtest review feedback in
Validation run:
@codex review Please re-review the latest head when available. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/target-devvit/package.json (1)
11-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
dev:watchmasks watcher failures.
sh -c '... & ... & wait'returns exit status 0 even ifdev:clientordev:servercrashes (barewaitignores child exit codes), so a failed watcher goes unnoticed while the other keeps running. Consider a runner likeconcurrently/npm-run-all -pthat propagates failures and terminates siblings on exit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/target-devvit/package.json` at line 11, The dev:watch script in package.json currently backgrounds dev:client and dev:server with a bare wait, which can hide watcher crashes and leave the other process running. Replace the shell-based parallel execution with a process runner such as concurrently or npm-run-all in the dev:watch script so failures from either dev:client or dev:server are propagated and sibling processes are terminated on exit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/target-devvit/package.json`:
- Line 11: The dev:watch script in package.json currently backgrounds dev:client
and dev:server with a bare wait, which can hide watcher crashes and leave the
other process running. Replace the shell-based parallel execution with a process
runner such as concurrently or npm-run-all in the dev:watch script so failures
from either dev:client or dev:server are propagated and sibling processes are
terminated on exit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1c1a20c-c2aa-4731-9081-cd1a4b43c123
📒 Files selected for processing (11)
apps/target-devvit/devvit.jsonapps/target-devvit/package.jsonapps/target-devvit/src/server/index.tspackages/cli/src/index.tspackages/cli/templates/phaser-game/README.mdpackages/cli/templates/phaser-game/agent/brief.mdpackages/cli/templates/phaser-game/apps/target-devvit/README.mdpackages/cli/templates/phaser-game/apps/target-devvit/devvit.jsonpackages/cli/templates/phaser-game/apps/target-devvit/package.jsonpackages/cli/templates/phaser-game/apps/target-devvit/src/server/index.tspackages/cli/templates/phaser-game/gitignore
💤 Files with no reviewable changes (1)
- packages/cli/templates/phaser-game/gitignore
✅ Files skipped from review due to trivial changes (3)
- packages/cli/templates/phaser-game/apps/target-devvit/README.md
- packages/cli/templates/phaser-game/agent/brief.md
- packages/cli/templates/phaser-game/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/templates/phaser-game/apps/target-devvit/package.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60a725a947
ℹ️ 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".
|
|
||
| response.status(200).json({ | ||
| showToast: { | ||
| text: `Created __GAME_TITLE__ post ${post.id}.`, |
There was a problem hiding this comment.
When a user runs mpgd game create with a title containing a backtick or ${...}, assertValidGameTitle still accepts it, and the raw title is rendered into this Devvit server template literal. That produces generated TypeScript with an unterminated template string or unintended interpolation, so an otherwise accepted starter title can make pnpm check/Devvit builds fail or change the success toast at runtime. Escape the title for TypeScript output or avoid embedding the placeholder inside a template literal here.
Useful? React with 👍 / 👎.
|
Addressed the latest watcher failure-propagation feedback in
Validation run:
@codex review Please re-review the latest head when available. |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Addressed the newly surfaced Devvit title escaping feedback in
Validation run:
@codex review Please re-review the latest head when available. |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
apps/target-devvitapp root in Phaser starters, includingdevvit.json, server bridge, scripts, README, and workspace install metadata.@mpgd/*package ranges from the released@mpgd/cliversion, add a Sampo changeset, and document AIT/Capacitor reference-shell ownership limits.Validation
pnpm create:game /tmp/mpgd-template-smoke-workspace --title "Workspace Game" --workspace --kit-path /Users/imjlk/repos/imjlk/mpgd-kitpnpm install --filter . --filter ./apps/target-devvitin the generated workspace starterpnpm check,pnpm build,pnpm build:devvit,pnpm smoke:devvit,pnpm --dir apps/target-devvit run checkin the generated workspace starterpnpm exec mpgd target build-all --targets-file ./mpgd.targets.json --targets web,ait,reddit --ait-variant wrapper --kit-path /Users/imjlk/repos/imjlk/mpgd-kitin the generated starterpnpm exec mpgd target smoke-all --targets-file ./mpgd.targets.json --targets web,ait,reddit --kit-path /Users/imjlk/repos/imjlk/mpgd-kitin the generated starterpnpm create:game /tmp/mpgd-template-smoke-nonworkspace --title "Smoke Game"pnpm create:game /tmp/mpgd-template-smoke-autodetect --title "Auto Detect Game" --workspacepnpm validate:starter-workflowpnpm --filter @mpgd/cli checkpnpm checkpnpm graph:preflightpnpm pack:packagesocr review --audience agent --background "Update mpgd create-game/CLI templates so generated Phaser games own apps/target-devvit, add Devvit init/playtest scripts/docs, derive starter @mpgd dependency pins from the @mpgd/cli release version, and document AIT/Capacitor as kit reference shells for smoke only." --commit cdef365ocr review --audience agent --background "Follow-up fix after OCR: in create-game workspace mode, generated Devvit scripts should use the auto-detected mpgd-kit path fallback when --kit-path is omitted." --commit 56cc05aNotes
pnpm sampo:release:dry-runwas attempted but is intentionally blocked on this branch: Sampo releases are configured formainonly.Summary by CodeRabbit
New Features
Bug Fixes