Repository navigation
fix: safely process only selected staged art (#198) - #199
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ba6a62e9-964c-4414-bf57-70010b17beab) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 144ddd2e08
ℹ️ 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,43 @@ | |||
| import { test } from 'node:test'; | |||
There was a problem hiding this comment.
This standalone node:test file is never exercised by the repository's normal validation: vitest.config.ts includes only src/**/*.{test,spec}.{ts,tsx}, npm run validate invokes only Vitest, and the inspected .github/workflows/ci.yml does not run node --test. Consequently, both new --only regressions are skipped by CI and can silently regress; move the test into the configured suite or add an explicit command to the validation script.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
| // node scripts/assets/process.mjs --only=generated/environments/street/new_plate.png | ||
| // node scripts/assets/process.mjs --write --only=generated/environments/street/new_plate.png |
There was a problem hiding this comment.
The new scoped command contradicts the repository's required asset-integration workflow: .agents/skills/slide-asset-integration-qa/SKILL.md:20 and its references/runtime-contracts-and-gates.md:56-69 still state that process.mjs has no per-file scope, document only the global commands, and instruct contributors to stop when unrelated inputs appear. Because contributors are required to use that skill, they will not discover or use this fix in precisely the blocked-import scenario it addresses; update the canonical workflow and reference alongside the CLI.
AGENTS.md reference: AGENTS.md:L10-L16
Useful? React with 👍 / 👎.
Outcome
Exact-source
--only=<relative image>allows new car art to enter the existing manifest/asset budget pipeline without consuming 23 unrelated legacy image files; default global behavior remains unchanged.Scope
In: asset processor CLI, subprocess regression, append-only log. Out: new images, gameplay, production database, credentials. Reserved: issue #198 comment.
Contracts preserved
Retains and validates every previous manifest entry and total runtime budget; rejects absolute/traversal/runtime/package/duplicate/symlink/missing sources before mutation. Global no-flag scan remains untouched.
Verification
Node 24: scoped CLI tests 2/2; full
npm run validate(including asset audit/package checks),npm run build, and backend pytest 95 passed. Local dry-run with no--onlystill fails on unrelated corrupt legacy icon, unchanged by this PR. #197 will exercise a scoped write with two generated plates after this PR is merged.Operational impact
None; no source art was processed in this PR. No migrations or deployment.
Integration notes
Merge before #197. Existing unrelated legacy icon remains intact.
Note
Low Risk
Build-time CLI and tests only; no runtime gameplay, shipped assets, or production deployment changes.
Overview
Adds repeatable
--only=<relative-image-path>to the build-time asset processor so operators can dry-run or write one staged source through the existing manifest and 20 MB budget pipeline without scanning every legacy file underpublic/assets.Invalid selections (absolute paths, traversal,
runtime//packages/, duplicates, symlinks, missing files) fail before any copy or manifest mutation; with--only, the run still retains and validates all registered manifest entries and global budget rules. Default full-tree behavior is unchanged.New subprocess regression tests cover a scoped dry run (unrelated corrupt legacy art is not touched; manifest and runtime stay unchanged) and unsafe
--onlyvalues.docs/PROJECT_LOG.mdrecords the change for upcoming #197 car-loadout art.Reviewed by Cursor Bugbot for commit 144ddd2. Bugbot is set up for automated code reviews on this repo. Configure here.