ship/fix desktop show native engine provisioning status#6546
Conversation
📝 WalkthroughWalkthroughThe changes add native engine progress reporting and a localized frontend overlay, convert preview-server execution into an explicit workflow dependency, normalize shell release artifacts, adjust release Rust flags, and add a non-debug interaction consistency no-op. ChangesRelease workflow updates
Native engine progress UI
Interaction debug build handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant NativeEngine
participant TauriEventBridge
participant NativeEngineProgressOverlay
participant App
NativeEngine->>TauriEventBridge: emit native-engine-progress
TauriEventBridge->>NativeEngineProgressOverlay: deliver progress payload
App->>NativeEngineProgressOverlay: mount global overlay
NativeEngineProgressOverlay-->>App: render localized phase and detail
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/deploy.ymlTraceback (most recent call last): .github/workflows/preview-server.ymlTraceback (most recent call last): .github/workflows/release.ymlTraceback (most recent call last):
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 @.github/workflows/deploy.yml:
- Around line 349-361: Replace the broad secrets inheritance on the
preview-server job with an explicit mapping for only CLOUDFLARE_API_TOKEN,
CLOUDFLARE_ACCOUNT_ID, and SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY, sourcing each
from the caller workflow’s corresponding secrets while leaving the existing
preview-server inputs unchanged.
In `@client/src/components/chrome/NativeEngineProgressOverlay.tsx`:
- Around line 75-84: Update the full-screen motion.div in
NativeEngineProgressOverlay to be a non-blocking status surface by adding
pointer-events-none to its styling, allowing pointer interaction with the
underlying app while preserving the existing role, live-region attributes, and
visual overlay behavior.
In `@client/src/services/nativeEngine.ts`:
- Around line 62-63: Update the native-engine progress flow around the listen
call and NativeEngineProgressOverlay so the latest resolving, ready, or failed
state is persisted or queryable and replayed when the overlay mounts. Ensure
subscription is established before provisioning begins where applicable, and
handle any progress event emitted during listener registration so no state is
lost.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ca8e1ae-d302-438a-a36b-7570f6754f8f
📒 Files selected for processing (17)
.github/workflows/deploy.yml.github/workflows/preview-server.yml.github/workflows/release.yml.github/workflows/shell-release.ymlclient/src-tauri/src/native_engine.rsclient/src/App.tsxclient/src/components/chrome/NativeEngineProgressOverlay.tsxclient/src/components/chrome/__tests__/NativeEngineProgressOverlay.test.tsxclient/src/i18n/locales/de/common.jsonclient/src/i18n/locales/en/common.jsonclient/src/i18n/locales/es/common.jsonclient/src/i18n/locales/fr/common.jsonclient/src/i18n/locales/it/common.jsonclient/src/i18n/locales/pl/common.jsonclient/src/i18n/locales/pt/common.jsonclient/src/services/nativeEngine.tscrates/engine/src/game/interaction.rs
| preview-server: | ||
| name: Publish Preview Server | ||
| needs: [card-data] | ||
| if: ${{ github.repository == 'phase-rs/phase' && (github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success') }} | ||
| uses: ./.github/workflows/preview-server.yml | ||
| with: | ||
| fingerprint: ${{ needs.card-data.outputs.engine_fingerprint }} | ||
| commit: ${{ github.event.workflow_run.head_sha || github.sha }} | ||
| card_data_sha256: ${{ needs.card-data.outputs.card_data_sha256 }} | ||
| card_data_hash16: ${{ needs.card-data.outputs.card_data_hash16 }} | ||
| draft_pools_sha256: ${{ needs.card-data.outputs.draft_pools_sha256 }} | ||
| draft_pools_hash16: ${{ needs.card-data.outputs.draft_pools_hash16 }} | ||
| secrets: inherit |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass only the secrets required by preview-server.yml.
secrets: inherit exposes every secret available to this workflow to the reusable workflow, although it only consumes CLOUDFLARE_API_TOKEN, CLOUDFLARE_ACCOUNT_ID, and SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY. Replace it with an explicit secret mapping to preserve least privilege.
Proposed fix
- secrets: inherit
+ secrets:
+ CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
+ CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
+ SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY: ${{ secrets.SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| preview-server: | |
| name: Publish Preview Server | |
| needs: [card-data] | |
| if: ${{ github.repository == 'phase-rs/phase' && (github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success') }} | |
| uses: ./.github/workflows/preview-server.yml | |
| with: | |
| fingerprint: ${{ needs.card-data.outputs.engine_fingerprint }} | |
| commit: ${{ github.event.workflow_run.head_sha || github.sha }} | |
| card_data_sha256: ${{ needs.card-data.outputs.card_data_sha256 }} | |
| card_data_hash16: ${{ needs.card-data.outputs.card_data_hash16 }} | |
| draft_pools_sha256: ${{ needs.card-data.outputs.draft_pools_sha256 }} | |
| draft_pools_hash16: ${{ needs.card-data.outputs.draft_pools_hash16 }} | |
| secrets: inherit | |
| preview-server: | |
| name: Publish Preview Server | |
| needs: [card-data] | |
| if: ${{ github.repository == 'phase-rs/phase' && (github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success') }} | |
| uses: ./.github/workflows/preview-server.yml | |
| with: | |
| fingerprint: ${{ needs.card-data.outputs.engine_fingerprint }} | |
| commit: ${{ github.event.workflow_run.head_sha || github.sha }} | |
| card_data_sha256: ${{ needs.card-data.outputs.card_data_sha256 }} | |
| card_data_hash16: ${{ needs.card-data.outputs.card_data_hash16 }} | |
| draft_pools_sha256: ${{ needs.card-data.outputs.draft_pools_sha256 }} | |
| draft_pools_hash16: ${{ needs.card-data.outputs.draft_pools_hash16 }} | |
| secrets: | |
| CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} | |
| CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} | |
| SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY: ${{ secrets.SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY }} |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 353-353: secrets unconditionally inherited by called workflow (secrets-inherit): this reusable workflow
(secrets-inherit)
🤖 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 @.github/workflows/deploy.yml around lines 349 - 361, Replace the broad
secrets inheritance on the preview-server job with an explicit mapping for only
CLOUDFLARE_API_TOKEN, CLOUDFLARE_ACCOUNT_ID, and
SERVER_ARTIFACT_MINISIGN_PRIVATE_KEY, sourcing each from the caller workflow’s
corresponding secrets while leaving the existing preview-server inputs
unchanged.
Source: Linters/SAST tools
| <motion.div | ||
| className="fixed inset-0 z-[80] flex items-center justify-center bg-black/60 p-4 backdrop-blur-sm" | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| role="status" | ||
| aria-live="polite" | ||
| aria-atomic="true" | ||
| aria-busy={progress.phase !== "ready" && progress.phase !== "failed"} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not block pointer users with a passive status region.
This full-screen element intercepts pointer input, but role="status" neither moves nor traps focus; keyboard users can still operate the obscured app while pointer users are blocked for up to five seconds. Make this a non-blocking status surface (pointer-events-none) or implement it as a proper modal dialog with focus handling.
🤖 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 `@client/src/components/chrome/NativeEngineProgressOverlay.tsx` around lines 75
- 84, Update the full-screen motion.div in NativeEngineProgressOverlay to be a
non-blocking status surface by adding pointer-events-none to its styling,
allowing pointer interaction with the underlying app while preserving the
existing role, live-region attributes, and visual overlay behavior.
| const { listen } = await import("@tauri-apps/api/event"); | ||
| return listen<NativeEngineProgress>("native-engine-progress", ({ payload }) => listener(payload)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make provisioning progress replayable. listen() only sees events after registration. If NativeEngineProgressOverlay mounts after the native side has already sent resolving, ready, or failed, that state is lost permanently. Persist/query the latest progress or subscribe before any provisioning call, and handle an event emitted during registration.
🤖 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 `@client/src/services/nativeEngine.ts` around lines 62 - 63, Update the
native-engine progress flow around the listen call and
NativeEngineProgressOverlay so the latest resolving, ready, or failed state is
persisted or queryable and replayed when the overlay mounts. Ensure subscription
is established before provisioning begins where applicable, and handle any
progress event emitted during listener registration so no state is lost.
Parse changes introduced by this PR✓ No card-parse changes detected. |
* fix(desktop): show native engine provisioning status * fix(release): recover main bundle and clarify desktop downloads --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests