Repository navigation
feat(target): add native release identity preflight - #118
Conversation
|
Warning Review limit reached
Next review available in: 53 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 Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds native Android/iOS release identity preflight checks, records native version evidence in release manifests, wires validation into Capacitor builds, and documents multi-target release versioning and promotion rules. ChangesNative release versioning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BuildTarget
participant IdentityValidator
participant NativeShell
participant ReleaseManifest
BuildTarget->>IdentityValidator: Validate platform identity and release environment
IdentityValidator->>NativeShell: Read configured native identity values
NativeShell-->>IdentityValidator: Return Android or iOS settings
IdentityValidator-->>BuildTarget: Allow build or report mismatch
BuildTarget->>ReleaseManifest: Write native version evidence
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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
🤖 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 `@docs/MULTI_TARGET_RELEASE_VERSIONING.md`:
- Around line 45-52: Update the Android and iOS release variable assignments in
the versioning documentation to use exported environment variables, ensuring a
subsequent mpgd build:target process receives them for preflight and manifest
generation.
In `@tools/target/native-release-identity.ts`:
- Around line 54-75: The native identity validation flow currently returns
undefined when both version variables are absent, allowing production releases
to bypass preflight. Add and propagate an explicit required-production flag from
the build caller into the relevant validation function, reject missing Android
and iOS version pairs when that flag is set, and add coverage for the release
failure case while preserving the optional behavior for non-production builds.
- Around line 123-128: Update the SemVer validation in requireFinalSemVer in
tools/target/native-release-identity.ts (123-128) to require each numeric
component to be either zero or a nonzero digit followed by digits, rejecting
leading zeros; apply the identical pattern in
tools/target/generate-release-manifest.ts (413-418) before recording manifest
evidence.
🪄 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 Plus
Run ID: 83f9925a-428e-41e0-b0a8-7d581c5d8988
📒 Files selected for processing (7)
docs/MULTI_TARGET_RELEASE_VERSIONING.mdpackage.jsonpackages/cli/templates/phaser-game/README.mdtools/target/build-target.tstools/target/generate-release-manifest.tstools/target/native-release-identity.test.tstools/target/native-release-identity.ts
![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: 9be05bb022
ℹ️ 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".
| versionCode: requirePositiveInteger(versionCode, 'MPGD_TARGET_VERSION_CODE'), | ||
| versionName: requireFinalSemVer(versionName, 'MPGD_TARGET_VERSION_NAME'), |
There was a problem hiding this comment.
When APP_VERSION is 1.5.0 but the native environment variables and shell still contain 1.4.0, this preflight succeeds because it validates only syntax and shell equality. The generated manifest then records gameVersion: 1.5.0 alongside versionName or marketingVersion: 1.4.0, violating the documented single-version release model and allowing the wrong store version to be packaged; compare the native version with APP_VERSION before accepting it.
Useful? React with 👍 / 👎.
| assertSetting(source, /\bapplicationId\s*(?:=\s*)?["']([^"']+)["']/u, expected.packageId, file); | ||
| assertSetting(source, /\bversionCode\s*(?:=\s*)?(\d+)/u, expected.versionCode, file); | ||
| assertSetting(source, /\bversionName\s*(?:=\s*)?["']([^"']+)["']/u, expected.versionName, file); |
There was a problem hiding this comment.
For a Gradle release build that sets applicationIdSuffix or versionNameSuffix, these expressions validate only the base applicationId and versionName, so the preflight passes even though bundleRelease emits an AAB with different final identity values. Resolve the Release variant's effective Gradle properties, or explicitly reject identity-transforming release configuration, instead of treating these textual base assignments as the artifact identity.
Useful? React with 👍 / 👎.
| For Sampo private-workspace releases, Devvit RC/final separation, and native | ||
| store-version provenance, see the | ||
| [multi-target release versioning guide](https://github.com/imjlk/mpgd-kit/blob/main/docs/MULTI_TARGET_RELEASE_VERSIONING.md). |
There was a problem hiding this comment.
This new link changes the generated README shipped in @mpgd/cli (the package includes templates), but the commit contains no .sampo/changesets/ entry. Without a CLI changeset, Sampo will not version and release this user-facing template contract; add a focused changeset for @mpgd/cli.
AGENTS.md reference: AGENTS.md:L51-L55
Useful? React with 👍 / 👎.
| throw new Error(`Native release preflight could not find ${expression.source} in ${file}.`); | ||
| } | ||
|
|
||
| if (values.some((value) => value !== expected)) { |
There was a problem hiding this comment.
When the Xcode project contains an extension, test target, or Debug configuration with its own bundle identifier or version, values.some(...) rejects the build even if the App target's Release configuration—the one archived by build-target.ts—matches exactly. Such additional identifiers are valid and often required for extensions, so the preflight should inspect only the effective settings for the archived App scheme/configuration rather than every matching assignment in the project file.
Useful? React with 👍 / 👎.
| Synchronize `package.json.version` with `mpgd.game.json.game.version` before | ||
| every production target build. Use `sampo release` for version/changelog |
There was a problem hiding this comment.
The generated game template has no game.version field in mpgd.game.json, and the CLI release environment reads only package.json.version in createGameOwnedReleaseEnv. Consequently, users following this instruction either cannot find the value or add an inert property that is never checked; document the actual version source or implement validation for the claimed field.
Useful? React with 👍 / 👎.
| if (versionName === undefined && versionCode === undefined) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
When neither native version variable is set, this early return disables the new identity check entirely, so an Android or iOS production build can still package a game-owned shell without validating its store identifier or recording any native version evidence. The guide labels these variables required for native releases; pass the build profile into this preflight and reject missing values for production while retaining the opt-out for staging builds.
Useful? React with 👍 / 👎.
| | Android | `versionName` | `versionCode` | | ||
| | iOS | `MARKETING_VERSION` | `CURRENT_PROJECT_VERSION` | |
There was a problem hiding this comment.
The repository exposes pnpm build:target and the published CLI exposes mpgd target build, but there is no mpgd build:target command. A developer following this release guide therefore fails before reaching the new preflight; replace the example with the appropriate CLI invocation, including its target and production profile.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function assertSetting(source: string, expression: RegExp, expected: string, file: string): void { | ||
| const values = [...source.matchAll(expression)].map((match) => match[1]?.trim()); |
There was a problem hiding this comment.
Whenever native version variables enable this preflight, the first assertSetting call throws TypeError: String.prototype.matchAll called with a non-global RegExp argument because every caller supplies a /u expression without the g flag. As a result, even a shell whose identity exactly matches the requested values cannot build; add the global flag to these expressions or use a matching API that accepts non-global regular expressions.
Useful? React with 👍 / 👎.
|
Addressed all three actionable review findings in a76b329: exported the documented variables, require native version evidence for production builds, and reject leading-zero SemVer components. undefined |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR branch / follow-up commit =='
git status --short --branch
git show --no-ext-diff --format=fuller --stat a76b329 -- 2>&1 || true
echo
echo '== Relevant changed paths in the follow-up =='
git diff-tree --no-commit-id --name-status -r a76b329 2>&1 || true
echo
echo '== package.json scripts =='
if [ -f package.json ]; then
node -e '
const p = JSON.parse(require("fs").readFileSync("package.json", "utf8"));
for (const [key, value] of Object.entries(p.scripts ?? {})) {
if (key.includes("native-release") || key === "test:prepared") console.log(`${key}: ${value}`);
}
' 2>&1 || true
fi
echo
echo '== Candidate target files =='
fd -a -t f '^(build-target|generate-release-manifest|native-release-identity)(\.test)?\.ts$' tools/target 2>/dev/null || true
echo
echo '== Symbol outlines =='
for f in tools/target/build-target.ts tools/target/generate-release-manifest.ts tools/target/native-release-identity.ts tools/target/native-release-identity.test.ts; do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --view expanded 2>&1 || true
fi
done
echo
echo '== Relevant implementation and test references =='
rg -n -C 5 --glob 'package.json' --glob 'tools/target/*.ts' \
'smoke:native-release-identity|MPGD_TARGET_(BUILD_NUMBER|MARKETING_VERSION|VERSION_CODE|VERSION_NAME)|requireFinalSemVer|requirePositiveInteger|assertNativeReleaseIdentity|production' \
package.json tools/target 2>&1 || trueLength of output: 41521 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR diff from merge base =='
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main 2>/dev/null || true)"
if [ -n "$base" ]; then
echo "merge-base: $base"
git diff --no-ext-diff --unified=50 "$base"...HEAD -- \
package.json \
tools/target/build-target.ts \
tools/target/generate-release-manifest.ts \
tools/target/native-release-identity.ts \
tools/target/native-release-identity.test.ts \
docs/MULTI_TARGET_RELEASE_VERSIONING.md
else
echo 'Could not resolve a main merge base; showing commit range instead.'
git show --no-ext-diff --unified=50 HEAD -- \
package.json \
tools/target/build-target.ts \
tools/target/generate-release-manifest.ts \
tools/target/native-release-identity.ts \
tools/target/native-release-identity.test.ts \
docs/MULTI_TARGET_RELEASE_VERSIONING.md
fi
echo
echo '== Build environment and manifest call sites =='
sed -n '100,280p' tools/target/build-target.ts
sed -n '470,530p' tools/target/build-target.ts
sed -n '590,625p' tools/target/build-target.ts
echo
echo '== Native identity implementation =='
sed -n '1,180p' tools/target/native-release-identity.ts
echo
echo '== Release manifest native evidence implementation =='
sed -n '54,165p' tools/target/generate-release-manifest.ts
sed -n '246,292p' tools/target/generate-release-manifest.ts
echo
echo '== Native identity test =='
sed -n '1,130p' tools/target/native-release-identity.test.ts
echo
echo '== All release-manifest writer call sites =='
rg -n -C 4 '\b(writeCapturedReleaseManifest|createReleaseManifestWriter|writeReleaseManifest)\s*\(' \
--glob '*.ts' \
tools packagesLength of output: 72180
I found no further actionable issues in this pass. The root If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
Validation note (run from the kit worktree): |
Summary
Why
Native store artifacts must prove their package or bundle identity and platform build number before packaging. The target layer now validates those values without embedding any game-specific product data.
Validation
pnpm smoke:native-release-identitypnpm exec ttsc --noEmit -p tsconfig.tools.jsonNo game-specific product, identity, or store data is included.