Repository navigation
Conversation
Invoke the installed TypeScript entrypoint and unchanged test ratchet through Node, preserving the existing compiler parallelism and background join.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe mobile workflow now runs the production TypeScript check through Node and runs the test-typecheck ratchet script directly, replacing the pnpm commands. New tests check the workflow commands and step ordering. They also run both checks in temporary fixtures and verify exit codes, compiler arguments and environment, installer behavior, and ratchet output. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The mobile typecheck change is ready to merge after normal CI checks; no outstanding issue was identified. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Direct compiler entrypoints in mobile CI —
.github/workflows/mobile.ymlnow runsnode node_modules/typescript/bin/tsc --noEmitfor the backgroundedTypecheckstep andnode scripts/check-tests-typecheck-ratchet.mjsfor the typecheck ratchet, replacing two concurrentpnpmwrapper calls. Since pnpm'sverifyDepsBeforeRundefaults toinstallon the pinnedpnpm@12.0.0, eachpnpm <script>performs a dependency check that can re-run install (and the mobilepostinstallwebview builds), so the two parallel typechecks could both mutatenode_modulesat once. Both direct commands target real JS entries, so no pnpm-provided PATH or env behavior is lost. - New workflow contract test —
config/scripts/mobile-typecheck-read-only-workflow.test.mjspins the step ordering, background flag,waitbarrier, and the exact command strings, then executes both shell commands in a temp fixture with a stubbedpnpmon PATH to prove zero installer invocations, identical args/cwd/env, and the production-exit / ratchet-exit outcomes. The executable half genuinely fails against stock: the stubbedpnpmwould log an installer and exit 73.
Verified locally: the new suite passes 4/4 and the related mobile/workflow suites pass 96/96; check:code-quality:changed and oxlint are clean.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
|
Superseded by #24776 (commit77ad467ebb83f16e8754abe7cfab7cb6d8078d5d): main already uses the direct Node compiler and ratchet commands and now deliberately waits for production types before the ratchet. Applying this older branch would undo that barrier and remove the restore-pnpm-verification path trigger. Closing preserves the newer main behavior; no remaining production optimization needs merging. |
ELI5
Two type checks should read the installed tools. They should not both try to reinstall those tools at the same time.
What Changed
Two real CI attempts failed while concurrent pnpm compiler wrappers tried to modify the same dependency directories before the checks ran. The workflow now calls the same installed TypeScript compiler and unchanged test ratchet through Node. The complete frozen install, compiler arguments, test baseline, parallel execution and wait before test workers remain intact.
Why
The job has already completed its frozen dependency install. Direct entrypoints remove two installer-capable wrapper calls while keeping the compilers parallel and preserving all checks. CI logs show overlapping installs; the precise pnpm freshness trigger was not logged.
Linked Issue
Internal performance audit; no pre-existing issue.
Visual Proof
N/A — cleanup and internal work only; live UI, output and interaction behavior are unchanged.
Testing
Five root contract suites pass 84 tests; existing mobile ratchet tests pass 21. An extracted actual workflow-shell fixture verifies zero pnpm calls versus two on stock, identical arguments/cwd/Node/environment, production exit 13, test-ratchet exit 1 and the background join. Stock fails four wrapper-count contracts. Independent reviewer reproduced these results and ran both real installed compilers concurrently: both exit zero. The installed compiler shim separately gives identical empty diagnostics. Root changed-code quality, full typecheck and build pass locally.
All test launches used ORCA_BACKGROUND_LAUNCH=1.
Review
Independent audit_renderer review approved the frozen file hashes and reproduced the original regression and final compatibility checks.
Agent skill upstream boundary
Notes
CI currently rejects three unchanged terminal write-spy fixture lines on main. #24600 repairs that separate fixture; rerun against the repaired base before merging this PR.
CI-only change: no mobile runtime, wire, SSH, workspace, credentials, dependency policy or application behavior changes. Linux ARM64 workflow remains the target; executable fixtures run on macOS, with structural checks available on Windows. A published green CI run is still required. The earlier project-matching PR has an independently reproduced installation failure; this separate fix does not silently change its head or bypass checks.
Checklist