Skip to content

fix(deps): root and mobile resolve one zod, and CI fails if they drift - #31260

Merged
nwparker merged 5 commits into
mainfrom
nwparker/align-zod-root-mobile
Oct 21, 2026
Merged

nwparker merged 5 commits into
mainfrom
nwparker/align-zod-root-mobile

Conversation

@nwparker

@nwparker nwparker commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​91 $\color{#cf222e}{\Huge{\mathbf{−}}}$​29 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​62
Prod 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1376 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1310 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​66

ELI5

The desktop code and the phone app each install their own copy of zod, the library that describes the shape of data. The phone app had an older patch release (4.6.5) than the desktop code (4.6.7). Some type checks read phone code and shared desktop code together. When they did, TypeScript treated the two copies as unrelated libraries and lost the real types. It then reported errors that came and went depending on which CI job ran. This PR puts both on the same release and adds a CI check that fails if they drift apart again.

What Changed

Before: the e2e typecheck ratchet only failed on PRs whose CI installed mobile dependencies (any PR touching mobile/). It failed with errors like tests/e2e/helpers/phone-feed-list.ts(154,9): error TS2322: Type '{} | null' is not assignable to type 'Platform | null'. That PR's code was fine. Mobile schemas that wrap src/shared helpers built on the root's zod (for example salvagedOptional) inferred {}, because TypeScript only merges two installed copies of a package when their versions match. PR #31217 hit this and added a type guard to work around it.

After: root and mobile lock the same zod, so TypeScript merges the copies and the inferred types are correct no matter which dependencies a job installs.

The change:

  • mobile/package.json: zod ~4.6.5 → ~4.6.7, matching the root. mobile/pnpm-lock.yaml was regenerated with the pinned pnpm 12.14.0 (pnpm install --lockfile-only), not edited by hand.
  • New config/scripts/check-mobile-shared-dependency-versions.mjs, run in pnpm lint and as a step in the static-analysis job of pr.yml. It reads the version each lockfile resolves and fails if they differ, or if either side stops declaring the package. It covers the packages src/shared imports that mobile/tsconfig.json remaps to mobile's own copy: zod, tweetnacl, @noble/hashes and @types/ws. Only zod differed; the other three already match.
  • lockfileRootImporter and lockedVersion moved out of check-node-runtime-pin.mjs into config/scripts/pnpm-lockfile-root-importer.mjs, so both checks share them. Importing them no longer loads the Node-pin .ts module.

About the lockfile diff (~2,580 lines): the set of locked packages changes in exactly one entry, zod@4.6.5 → zod@4.6.7. settings, overrides and patchedDependencies are unchanged. The rest is pnpm 12.14 re-resolving a lockfile that an older pnpm last resolved in full:

  • debug's optional supports-color peer now resolves to the already-locked 8.1.1, which adds an (supports-color@8.1.1) suffix to every snapshot that reaches debug. The root lockfile already has the same style with supports-color@7.2.0. The only effect is that dev-tool debug output may now be coloured.
  • @typescript-eslint/parser's typescript peer goes from 6.0.3 to the 5.9.3 that its dependents (eslint-config-universe, eslint-plugin) were already keyed on, which fixes an inconsistency in the old lockfile. Mobile lint is oxlint, so this doesn't affect it.

Packages whose resolved version changed: exactly one, zod 4.6.5 → 4.6.7. Comparing every packages: key and every importer version (ignoring peer suffixes) between the old and new lockfile shows no other version change.

Why the peer suffixes changed, and why I couldn't shrink the diff:

  • It isn't the pnpm version. I used the pinned 12.14.0 (the same as packageManager and the lockfile's packageManagerDependencies), with no resolution-mode or dedupe overrides set. Running the same bump under pnpm 12.8.1, the version the lockfile was created with in feat(orchestration): a native chat gets the orchestration pointer a CLI agent gets, through the same send as your messages #25078, produces a byte-identical 2,580-line diff.
  • It isn't local state. Moving mobile/node_modules aside gives the same result.
  • It is pnpm's peer-resolution pass. On main, a re-resolve that keeps zod at the same version (pinning "zod": "4.6.5") changes only the specifier line. Any real version change runs the full peer pass, and on today's graph that pass resolves debug's optional supports-color peer to the already-locked 8.1.1. The lockfile has never contained those suffixes. Every mobile lockfile change since feat(orchestration): a native chat gets the orchestration pointer a CLI agent gets, through the same send as your messages #25078 left debug's existing peer resolution untouched, so the next real version bump to any mobile dependency will produce the same rewrite.
  • pnpm add, pnpm update zod --lockfile-only and pnpm install --lockfile-only all produce it, and the last gives the smallest diff. A smaller diff would mean hand-editing the lockfile, which I didn't do.

Why

  • Why not a pnpm catalog or a shared override? mobile/ is a separate pnpm project, with its own pnpm-workspace.yaml and lockfile, kept out of the root graph on purpose (see the comment in the root pnpm-workspace.yaml). A catalog or override in one project can't reach the other, and the repo has no cross-project mechanism for other shared deps either: react is ~19.2.8 at the root and 19.2.8 in mobile, aligned by hand. So the single source of truth here is the check. It compares the versions the two lockfiles actually resolve, because two tilde ranges can match today and drift on the next install.
  • Why the lockfile and not node_modules? It's offline, needs no mobile install, takes milliseconds, and the static job already runs for any change to mobile/package.json, mobile/pnpm-lock.yaml, package.json or pnpm-lock.yaml.
  • Why not keep the fix(orcad): voice dictation on headless orca serve hosts #31217 type guard? It hides one call site. The next wrapper around a shared zod helper would fail the same way.

Linked Issue

None. This came up as an intermittent failure on #31217.

Visual Proof

N/A: dependency and CI change with no UI.

Testing

All runs were local on macOS, with root and mobile dependencies installed as CI does for mobile PRs.

  • Original failure reproduced on main (f812c67a052, which does not include fix(orcad): voice dictation on headless orca serve hosts #31217's guard): pnpm typecheck:e2e reports tests/e2e/helpers/phone-feed-list.ts(154,9): error TS2322 ... '{} | null' ... 'Platform | null', and check:e2e-typecheck-ratchet fails on that file. 337 diagnostics in total.

  • With this change: that error is gone, total diagnostics drop to 255, and the ratchet passes (222 grandfathered).

  • Root pnpm tc passes.

  • Mobile scripts/typecheck-mobile.mjs and scripts/check-tests-typecheck-ratchet.mjs pass.

  • Mobile vitest over the zod-heavy folders (src/transport, src/session, src/tasks, src/mobile-web-shell/bridge, src/host-screen): 554 files and 5,331 tests pass.

  • Mobile bundling: pnpm build:mobile-web passes (build and verify). Expo iOS and Android Hermes bundles also build.

  • New tests in check-mobile-shared-dependency-versions.test.mjs cover a match, a mismatch, peer and patch suffixes, a missing side (either side), the default package list, and the committed lockfiles. The lockfileRootImporter test moved to pnpm-lockfile-root-importer.test.mjs, with lockedVersion tests added. check-node-runtime-pin.test.mjs still passes.

  • The full static-analysis job was run locally, step by step.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

  • Codex (gpt-6.1-sol, high) against pinned base f812c67a052: no findings.
  • Independent Opus review: no P0-P2 findings in two rounds. Its four P3s (a wider guard list, a stale line-number comment, tests left behind when code moved, and two untested branches) were all fixed in the second commit.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

No runtime behaviour change on desktop, SSH or remote hosts. The mobile app gets a zod patch release (4.6.5 → 4.6.7) that the desktop already ships. The check uses path.join and only reads files, so it works on every OS.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⛔ Files ignored due to path filters: 1
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 901c4866-155b-4034-86bb-0a5a5b1b44cc
📥 Commits

Reviewing files that changed from the base of the PR and between 9486657 and 13b06df.

⛔ Files ignored due to path filters: 1
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35dde920-ef35-43e0-9b56-470882c91987
📥 Commits

Reviewing files that changed from the base of the PR and between 2083c05baa924c1ae7098ccb1814943e7c901c87 and 9486657.

⛔ Files ignored due to path filters: 1
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing: 1
  • config/scripts/pr-preflight-gates.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The changes add shared pnpm lockfile helpers and a check for matching versions of configured dependencies in the root and mobile projects. The check runs through the package lint command and pull request preflight. The mobile zod dependency constraint changes from ~4.6.5 to ~4.6.7.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 94866

No actionable merge-blocking risk is established for this change.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary change: aligning root and mobile zod versions and adding CI protection against dependency drift.
Description check Passed The description is detailed and covers the required sections, including the problem, changes, rationale, testing, review, compatibility notes, and checklist. The Linked Issue section states "None" ins…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and covers the required sections, including the problem, changes, rationale, testing, review, compatibility notes, and checklist. The Linked Issue section states "None" instead of providing an issue link, which is a template gap, but the description is otherwise mostly complete.

✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Mobile type-checks src/shared zod schemas. With mobile on zod 4.6.5 and the
root on 4.6.7, TypeScript treated them as unrelated packages, so wrapped
schemas inferred {} (tests/e2e/helpers/phone-feed-list.ts:154) whenever a
job installed mobile deps.
@nwparker
nwparker force-pushed the nwparker/align-zod-root-mobile branch from 2083c05 to 9486657 Compare October 20, 2026 23:38
@nwparker
nwparker merged commit a308277 into main Oct 21, 2026
55 checks passed
nwparker added a commit that referenced this pull request Oct 21, 2026
…platform guard now zod is aligned

launch.ts went one line past max-lines with main; resolveAppRoot,
resolveForegroundOrcaExecutable, getExecutableAppArgs and
stripElectronRunAsNode move to orca-app-executable.ts (re-exported, so
callers are unchanged). With mobile and root on one zod (#31260), the
host.platform schema no longer needs the extra narrowing step.
nwparker added a commit that referenced this pull request Oct 21, 2026
* fix(orcad): voice dictation on headless orca serve hosts

Mobile dictation against an orca serve host answered speech_unavailable
since serve moved to orcad: orcad never installed the speech service
factories, and the model manager and download imported Electron.

The model download now takes a host request opener (Chromium net on the
desktop, Node https on orcad, with redirects mapped to the same event),
the STT worker ships in the orcad slot, and orca serve hands orcad the
installed app's on-device recognizer. A host without a recognizer (an
SSH or WSL managed slot) lists only cloud models and sends a phone that
picked an on-device model to the setup sheet. Mobile names the fix when
an older server still has no speech at all.

The parity test now requires both roots to install speech.

Reported-by: delorean2020 (@delorean2020)

* fix(orcad): retry Node download drops, stage the recognizer on Windows, review fixes

- Node https errors (ECONNRESET, ETIMEDOUT, ...) are retryable, so an
  interrupted orcad model download resumes (Codex P2).
- On Windows orca serve hands orcad a per-version copy of the recognizer
  in the profile, so a loaded addon never locks the app's files against
  an update (Opus).
- Drop a recognizer candidate path that could never match; one SAFETY
  helper per test file instead of repeated casts; comments.

* fix(orcad): cloud speech is ready only with a key this host can decrypt; prune superseded recognizer copies

Codex round 2: a desktop-sealed OpenAI key exists on disk but orcad's
secret store cannot open it, so the catalog reported cloud models ready
and dictation failed at the end. orcad's model manager now proves the
key decrypts. The desktop keeps its existence check (no keychain prompt).

* fix(orcad): never prune staged recognizers before the profile lock; copy linked packages as real files

Codex round 3. ref-oss check: versioned copies are cleaned only by the
process that owns them, so the pre-lock prune is dropped rather than
patched.

* fix(orcad): remove a partial recognizer copy when staging fails (Codex round 4)

* fix(speech): never hand out another store's ciphertext as the OpenAI key (Codex round 5)

* refactor(orcad): no recognizer copy on Windows; Windows orcad keeps cloud dictation only

Design check after review round 5: every remaining finding came from the
Windows copy that dodges addon file locks. Dropping it shrinks the
surface to a path lookup; Linux and macOS serve keep on-device models.

* fix(speech): name the fix when an old server refuses a mic tap; always set the recognizer env; retry ENOTFOUND (final Opus review)

* fix(managed-server): probe install completeness relative to the slot dir

CI: shipping stt-worker.js added one more absolute slot path to the
install-complete probe, pushing it further past the exec-size budget.
Testing each artifact relative to the slot (inside a subshell cd) cuts
the probe from ~12.2 KB to ~3.6 KB, so it drops off the over-budget
list instead of raising a ceiling.

* fix(mobile): pin host.platform's read to a platform type, independent of which zod copy resolves

With mobile's own dependencies installed (which CI does for any PR that
touches mobile/), the e2e program resolves two zod copies, and the
salvaged platform arm from src/shared inferred as {}. The schema now
narrows through an explicit platform guard.

* fix(mobile): keep the host.platform read line intact for its RPC mutant; narrow in a second step

Mobile verify: the recorded-RPC mutant anchors on the read line, which
the previous guard rewrote, so the mutant could not apply.

* refactor(cli): app-executable resolution in its own module; drop the platform guard now zod is aligned

launch.ts went one line past max-lines with main; resolveAppRoot,
resolveForegroundOrcaExecutable, getExecutableAppArgs and
stripElectronRunAsNode move to orca-app-executable.ts (re-exported, so
callers are unchanged). With mobile and root on one zod (#31260), the
host.platform schema no longer needs the extra narrowing step.

* refactor(cli): import app-executable helpers from their module; no re-exports from launch.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant