Repository navigation
feat(cli): generate Microsoft Store PWA packages - #100
Conversation
|
Warning Review limit reached
Next review available in: 12 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 selected for processing (4)
📝 WalkthroughWalkthroughAdds Microsoft Store PWA package generation with validated evidence, manifest scope checks, bounded downloads, ZIP safety validation, atomic archive and provenance outputs, CLI wiring, smoke coverage, and updated release guidance. ChangesMicrosoft Store package generation
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Filesystem
participant PWA
participant Generator
CLI->>Filesystem: Read submission evidence and pinned assets
CLI->>PWA: Verify manifest and icon hashes
CLI->>Generator: POST package-generation request
Generator-->>CLI: Return ZIP archive
CLI->>Filesystem: Validate and atomically write ZIP and provenance
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5a64d7f0f
ℹ️ 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".
b5a64d7 to
8c949a1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c949a14fc
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/microsoft-store-package-generation-download.ts`:
- Around line 134-140: Replace the existsSync check followed by renameSync in
the package publication flow with an atomic no-clobber operation, such as
creating a hard link from temporaryFile to input.outputFile. Preserve the
new-output contract by handling an EEXIST failure as the existing-output error,
and remove the racy pre-check.
In `@packages/cli/src/microsoft-store-package-generation-input.ts`:
- Around line 34-38: Update the submission evidence flow around
hashMicrosoftStoreFileSnapshot and readSubmissionEvidence so the exact bytes
represented by submissionBefore are validated before parsing, following the
manifest reader’s validation pattern. Ensure submissionEvidence and
submissionEvidenceSha256 are derived from the same verified source bytes,
including the corresponding later flow noted in the comment.
In `@packages/cli/src/microsoft-store-package-generation-zip.ts`:
- Around line 87-108: Update the local-header validation near localHeaderOffset
to read the complete 30-byte fixed header, then read its filename using the
local header’s filename length and run assertSafeZipEntryName on it. Require the
raw local filename bytes to exactly match fileNameBytes from the central
directory, rejecting mismatches before continuing while preserving the existing
signature and offset checks.
In `@tools/smoke/cli-microsoft-store-package-generation.ts`:
- Line 13: Update the temporary-archive assertion in the Microsoft Store package
generation smoke test to use the imported path basename utility instead of
splitting outputFile on '/'. Preserve the existing archive-name validation while
making it work with both Windows and POSIX paths.
🪄 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: a4d5237c-4b10-45d5-ab63-d219ae541ed8
📒 Files selected for processing (16)
.sampo/changesets/microsoft-store-package-generation.mdREADME.mddocs/PUBLIC_RELEASE_CHECKLIST.mdpackage.jsonpackages/cli/src/index.tspackages/cli/src/microsoft-store-package-generation-contract.tspackages/cli/src/microsoft-store-package-generation-download.tspackages/cli/src/microsoft-store-package-generation-input.tspackages/cli/src/microsoft-store-package-generation-integrity.tspackages/cli/src/microsoft-store-package-generation-manifest.tspackages/cli/src/microsoft-store-package-generation-zip.tspackages/cli/src/microsoft-store-package-generation.tspackages/cli/templates/phaser-game/README.mdpackages/cli/templates/phaser-game/gitignorepackages/cli/templates/phaser-game/package.jsontools/smoke/cli-microsoft-store-package-generation.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e473c008b4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13781abf12
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a5a4b60de
ℹ️ 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".
|
@codex review the exact current head cf5c330. Please review the complete PR at this SHA, including the public-only DNS-pinned fetch boundary, transactional evidence writes, the --package-version CLI path, and repeated manifest-icon file reuse. Report every actionable finding; if none remain, mark this exact head clean. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/index.ts (1)
1206-1231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnsure output directories exist before writing generation artifacts.
Unlike the
preflightandaccept-packagecommands, this subcommand does not create theoutputDirbefore proceeding. Furthermore, theoutputFileresides in an input directory (release-input/microsoft-store) that may not exist on a clean run. Missing directories will likely causeENOENTerrors when writing evidence files or saving the ZIP archive.Consider extracting
outputFileand explicitly creating both directories before calling the generation routine.🛠️ Proposed fix
const outputDir = resolveGameRelativePath( gameRoot, readOptionalString(ctx.values['output-dir']) ?? 'release-output/microsoft-store', ); + const outputFile = resolveGameRelativePath( + gameRoot, + readOptionalString(ctx.values['output-file']) ?? 'release-input/microsoft-store/pwabuilder-package.zip', + ); assertGameOwnedOutputDirectory(gameRoot, outputDir); + mkdirSync(outputDir, { recursive: true }); + mkdirSync(path.dirname(outputFile), { recursive: true }); resolveMicrosoftStoreArtifactRoot(targetsFile); const evidence = await runMicrosoftStorePackageGeneration({ gameRoot, submissionEvidenceFile: resolveGameRelativePath( gameRoot, readOptionalString(ctx.values['submission-evidence']) ?? 'release-output/microsoft-store/submission-preflight.json', ), pwaUrl: readOptionalString(ctx.values['pwa-url']) ?? '', manifestUrl: readOptionalString(ctx.values['manifest-url']) ?? '', modernVersion: readOptionalString(ctx.values['package-version']) ?? '', classicVersion: readOptionalString(ctx.values['classic-version']) ?? '', - outputFile: resolveGameRelativePath( - gameRoot, - readOptionalString(ctx.values['output-file']) - ?? 'release-input/microsoft-store/pwabuilder-package.zip', - ), + outputFile, jsonFile: path.join(outputDir, 'package-generation.json'), markdownFile: path.join(outputDir, 'package-generation.md'), });🤖 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 1206 - 1231, In the Microsoft Store package-generation flow, update the code around outputDir and runMicrosoftStorePackageGeneration to resolve the outputFile path once, create both outputDir and its parent directory recursively before generation, and pass the resolved outputFile to the routine. Preserve the existing defaults and artifact paths.
🤖 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/src/microsoft-store-package-generation-download.ts`:
- Around line 79-85: Update the fetch wrapper around the `undiciFetch` call to
pass the original `input` directly instead of extracting `input.url`, preserving
`Request` properties such as method, headers, and body while retaining the
dispatcher override in `init`.
---
Outside diff comments:
In `@packages/cli/src/index.ts`:
- Around line 1206-1231: In the Microsoft Store package-generation flow, update
the code around outputDir and runMicrosoftStorePackageGeneration to resolve the
outputFile path once, create both outputDir and its parent directory recursively
before generation, and pass the resolved outputFile to the routine. Preserve the
existing defaults and artifact paths.
🪄 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: 4461990e-e55c-47a7-882b-4f4c62faab91
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.sampo/changesets/microsoft-store-package-generation.mdREADME.mdpackages/cli/package.jsonpackages/cli/src/index.tspackages/cli/src/microsoft-store-package-generation-contract.tspackages/cli/src/microsoft-store-package-generation-download.tspackages/cli/src/microsoft-store-package-generation-input.tspackages/cli/src/microsoft-store-package-generation.tspackages/cli/templates/phaser-game/README.mdtools/smoke/cli-microsoft-store-package-generation.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .sampo/changesets/microsoft-store-package-generation.md
- README.md
- packages/cli/src/microsoft-store-package-generation.ts
- packages/cli/src/microsoft-store-package-generation-contract.ts
- packages/cli/src/microsoft-store-package-generation-input.ts
- tools/smoke/cli-microsoft-store-package-generation.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf5c330b8b
ℹ️ 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".
|
Follow-up on the outside-diff clean-run output-directory note from the latest automated review: no patch is needed for that item. The package-generation input preparation resolves the requested ZIP path, rejects an existing target, creates its parent recursively, then canonicalizes the parent and re-checks that it remains inside the game root before the download begins ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f29cb0887
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 585143e6bc
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/src/microsoft-store-package-generation.ts`:
- Around line 254-272: Update the publication loop around the file placement
operations and the corresponding flow at the additional location: immediately
after each successful linkSync or renameSync, record the file’s rollback
ownership identity in publishedIdentity before unlinkSync, lstatSync, or any
other fallible operation. Preserve the existing identity validation,
temporary-file state updates, and rollback behavior, and add fault-injection
coverage for failures after both linkSync and renameSync.
🪄 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: 9b2984f4-c8d1-4fd2-bf33-d1840d6793c5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
docs/PUBLIC_RELEASE_CHECKLIST.mdpackage.jsonpackages/cli/src/microsoft-store-package-generation-download.tspackages/cli/src/microsoft-store-package-generation.tstools/smoke/cli-microsoft-store-package-generation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/PUBLIC_RELEASE_CHECKLIST.md
- packages/cli/src/microsoft-store-package-generation-download.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18a32b70af
ℹ️ 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".
|
Local OpenCodeReview completed for exact head 43bb5ff (session db152bc8-f48c-4497-b714-87a00dbff7b8): 4 files completed, 0 failed, 0 LLM failures, aborted=false. The sole Low maintainability note asks to remove the icon URL undefined guard. I am retaining it: indexed access remains string-or-undefined under the project TypeScript settings, the explicit guard avoids a non-null assertion, and it preserves fail-closed behavior if this private helper is later passed a sparse or concurrently-derived array. No High or Medium findings remain. |
|
@codex review exact head 43bb5ff. Please re-review the complete current PR, including absolute verified manifest icon URLs before request serialization/hash, immediate ZIP rollback ownership after hard-link placement, and repository-level generated package ignore coverage. Report every actionable issue still present on this exact head; if none remain, mark it clean. |
|
Codex Review: Didn't find any major issues. Nice work! 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
Validation
pnpm smoke:cli-microsoft-store-submissionpnpm smoke:cli-microsoft-store-package-generationpnpm smoke:cli-microsoft-store-package-acceptancepnpm checkpnpm lintpnpm build:packagespnpm build:microsoft-storepnpm smoke:target microsoft-storegit diff --checkSampo release dry-run is intentionally blocked on feature branches because releases are configured for main only; the changeset uses the existing Added syntax.
Boundaries
The remote API and deployed icon URLs are mutable external resources, so evidence marks this as a best-effort external-service contract and pins the reviewed upstream source revision. Before/after probes detect changes but cannot make remote resources content-addressed. Redirects fail closed to preserve exact evidence URL binding. This command only downloads and records the ZIP; package inspection and acceptance remain a separate required stage before Partner Center submission.
Official sources:
Summary by CodeRabbit
New Features
generate-package microsoft-store).Documentation
Tests