Skip to content

fix(mobile): use desktop's setup config and trust rules when creating workspaces - #33942

Merged
AmoabaKelvin merged 5 commits into
mainfrom
mobile-setup-trust-parity
Oct 27, 2026
Merged

AmoabaKelvin merged 5 commits into
mainfrom
mobile-setup-trust-parity

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​116 $\color{#cf222e}{\Huge{\mathbf{−}}}$​168 $\color{#cf222e}{\Huge{\mathbf{−}}}$​52
Prod 29 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​267 $\color{#cf222e}{\Huge{\mathbf{−}}}$​281 $\color{#cf222e}{\Huge{\mathbf{−}}}$​14

ELI5

When you create a workspace from the phone, it now asks before running the commands in a project's orca.yaml, the same way the desktop does. Before, the phone only knew about the setup script. Commands listed under defaultTabs either ran with no trust prompt or were silently skipped.

What Changed

The phone now uses the desktop's rules for setup and trust when it creates a workspace:

  • orca.yaml defaultTabs commands (workspaces Fix Ctrl+Backspace not deleting word in terminal #2). Desktop's getSetupConfig moved from the renderer to src/shared/orca-hook-setup-config.ts. The host's repo.hooks reply now includes setupConfig (the command preview and its kind), worked out by that shared function. The composer and Tasks create on the phone gate the Run/Skip choice and the trust prompt on it.
    • Before: with a repo whose orca.yaml has defaultTabs commands and no scripts.setup, the phone sent inherit. Under "run by default" the host ran the commands with no trust prompt. Under "ask" it skipped them with no message.
    • After: the phone shows the commands, asks Run/Skip under "ask", and asks for trust before the first run, like the desktop.
  • Setup trust text and setupTrust (workspaces Fix Ctrl+Backspace not deleting word in terminal #2). There were three copies of the "setup plus defaultTabs" trust text: the host, ensure-hooks-confirmed, and inside getSetupConfig. They are now one shared getSetupTrustContent. The host builds setupTrust from the shared config, so a repo that is implicitly local-only (no policy set, plus a local script) no longer gets a trust prompt for yaml commands that won't run. The desktop already skipped that prompt.
  • Repo-wide "Always trust" (workspaces Add Orca logo and polish README header #13, tasks Fix useEffect anti-patterns across renderer #9). canUseRepoWideTrust (including the duplicate-repo-id guard) and the trust entry builders moved to src/shared/orca-hook-trust-entries.ts. The desktop store actions and the phone both use them.
    • Before: when a repo id was listed on two hosts, the phone applied "Always trust".
    • After: like the desktop, it asks again.
  • Setup run default (workspaces Improve update experience: toasts, graceful restart, sidebar indicator #14). One shared getDefaultSetupDecision(hasConfig, policy) replaces the policy ternaries in derived-composer-state, quick-submit-preparation, linked-item-lookup-effects, launch-work-item-direct-preflight and the phone's Tasks create. The automation path keeps its own rule on purpose (ask means skip there).
  • New-workspace project picker (workspaces Fix useEffect anti-patterns across renderer #9). The phone's mirror of src/shared/new-workspace-dialog-repo.ts is deleted. The picker now lists only getNewWorkspaceDialogEligibleRepos, so hidden runtime-owned SSH repos (per-workspace environment VMs) and repos with no path no longer appear. Host ids are compared through the shared getRepoExecutionHostId.
  • Desktop fix found in review. A run-both repo with a local setup script and no orca.yaml commands used to get no setup config. Under "ask" the host then refused the create ("Setup decision required"). The shared rule now returns the local script for that case, on both desktop and phone.

Why

Part of the mobile/desktop parity audit. Desktop logic is the reference. Each rule above now has one copy in src/shared, used by the desktop renderer, the host and the phone. The phone can't compute the setup config itself, because repo.hooks strips defaultTabs from hooks. So the host computes it with the shared function, and the phone reads the answer.

Linked Issue

Part of the mobile/desktop parity audit

Visual Proof

No device captures for this fleet. The change adopts desktop's existing behavior: getSetupConfig plus the setup trust prompt (ensure-hooks-confirmed), canUseRepoWideTrust, and getNewWorkspaceDialogEligibleRepos.

Testing

  • I manually tested these changes locally

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

  • Moved desktop's getSetupConfig tests to src/shared/orca-hook-setup-config.test.ts, plus a run-both local-only regression test.

  • Moved the trust entry builder tests to src/shared/orca-hook-trust-entries.test.ts. The duplicate-id guard stays covered by ensure-hooks-confirmed.test.ts.

  • Added a phone test for a defaultTabs-only setupConfig (use-new-workspace-setup-script.test.ts).

  • Deleted the phone tests that only pinned the deleted repo-selection mirror; the shared tests cover them.

  • Ran the focused desktop, host and mobile tests, the mobile RPC-recording suite, check:code-quality:changed and check:react-doctor:changed.

AI Disclosure

Review

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

  • Mixed versions. setupConfig is a new optional field on repo.hooks. An old phone ignores it. A new phone on an old host falls back to hooks.scripts.setup (the old behavior).
  • Removed mobile-only behavior:
    • The picker no longer lists path-less or runtime-owned SSH repos.
    • "Always trust" no longer applies when the repo id is on two hosts.
    • An unknown setupRunPolicy from a newer host now reads as skip-by-default in the Tasks path. It already skipped before, but the raw string is no longer kept.
  • Desktop behavior change: the run-both local-only fix above.
  • SSH repos: unchanged. repo.hooks still reports no hooks for them.
  • Folder repos: setupConfig is null and no setupTrust is sent, matching the desktop composer, which only computes setup for git repos.

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)

@AmoabaKelvin
AmoabaKelvin force-pushed the mobile-setup-trust-parity branch from 2da8d9c to c9c9ff9 Compare October 26, 2026 14:23
@AmoabaKelvin
AmoabaKelvin force-pushed the mobile-setup-trust-parity branch 2 times, most recently from 352c1d6 to a29a712 Compare October 27, 2026 01:08
@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 27, 2026 01:08
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The change moves setup-configuration and hook-trust helpers into shared modules and updates runtime, renderer, and mobile consumers. Hook replies can include setup configuration, and unknown mobile setup policies now parse as skip-by-default. Mobile repository listings use shared eligibility filtering and preferred-repository resolution.

Priority: Normal

Merge Risk: Low · up to ddd50

Affected automations may show or apply an unexpected setup decision, but the impact is limited to this configuration and remains straightforward to correct.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description follows the template's main sections: ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, Review, Notes, and Checklist. It explains the before and after behavior, the mechani… Add a linked issue number (the template requires one for outside contributors; a maintainer should confirm whether a pre-existing issue applies). Add before and after visual proof for the mobile behavior change, or state N/A with a reason t…
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title accurately names the main change: mobile workspace creation now uses desktop's setup config and trust rules. The changeset moves setup config and trust logic into shared code and has the mob…
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: Title check

Explanation

The title accurately names the main change: mobile workspace creation now uses desktop's setup config and trust rules. The changeset moves setup config and trust logic into shared code and has the mobile app consume it. The title is specific and concise, and it avoids vague wording.

Full details: Description check

Explanation

The description follows the template's main sections: ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, Review, Notes, and Checklist. It explains the before and after behavior, the mechanism (shared getSetupConfig, setupConfig on repo.hooks, shared trust helpers), and mixed-version handling. Several gaps remain. The Linked Issue section has no issue number ('Fixes #' is blank, and it only says 'Part of the mobile/desktop parity audit'). The AI Disclosure and Review sections are empty. Some checklist items are unchecked, including lint, typecheck, test, and build. The manual testing box is unchecked. The Visual Proof section gives a reason but no before/after capture, which the template requires for UI and behavior changes.

Resolution

Add a linked issue number (the template requires one for outside contributors; a maintainer should confirm whether a pre-existing issue applies). Add before and after visual proof for the mobile behavior change, or state N/A with a reason that the template accepts. Fill the AI Disclosure section or state that it does not apply. Complete the Review section if it is needed. Confirm the manual testing box, or describe the manual steps taken. Run pnpm lint, typecheck, test, and build locally, or note that CI covers them. Update the checklist to match.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 32 functions across 34 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 36ce4aa1-1818-4357-81d7-c3b8421f1e1e
Commits

Reviewing files that changed from the base of the PR and between 57aa892 and a29a712.

Files selected for processing: 39
  • mobile/src/components/NewWorktreeModal.tsx
  • mobile/src/components/new-workspace-reply-schema.ts
  • mobile/src/components/use-new-workspace-create-submit.ts
  • mobile/src/components/use-new-workspace-repositories.ts
  • mobile/src/components/use-new-workspace-setup-script.test.ts
  • mobile/src/components/use-new-workspace-setup-script.ts
  • mobile/src/tasks/mobile-tasks-dependencies.ts
  • mobile/src/tasks/setup-hook-trust.test.ts
  • mobile/src/tasks/setup-hook-trust.ts
  • mobile/src/tasks/use-mobile-tasks-client-settings-actions.tsx
  • mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx
  • mobile/src/tasks/use-mobile-tasks-workspace-ssh-state.tsx
  • mobile/src/tasks/workspace-source-reply-schema.test.ts
  • mobile/src/tasks/workspace-source-reply-schema.ts
  • mobile/src/test-support/rpc-recording/adapters/workspace-settings-mounts.ts
  • mobile/src/worktree/new-workspace-dialog-repo-selection.test.ts
  • mobile/src/worktree/new-workspace-dialog-repo-selection.ts
  • src/main/effective-hook-config.ts
  • src/main/ipc/hooks/worktree-hook-execution-host.test.ts
  • src/main/runtime/orca-runtime-test-mocks/setup.spec.ts
  • src/main/runtime/runtime-repository-hooks-commands.ts
  • src/renderer/src/components/automations/automation-setup-decision.ts
  • src/renderer/src/components/new-workspace/new-workspace-composer-card-props.ts
  • src/renderer/src/hooks/composer-state/derived-composer-state.ts
  • src/renderer/src/hooks/composer-state/derived-model.ts
  • src/renderer/src/hooks/composer-state/linked-item-lookup-effects.ts
  • src/renderer/src/hooks/composer-state/quick-submit-preparation.ts
  • src/renderer/src/lib/ensure-hooks-confirmed.ts
  • src/renderer/src/lib/launch-work-item-direct-preflight.ts
  • src/renderer/src/lib/launch-work-item-direct.test.ts
  • src/renderer/src/lib/new-workspace.test.ts
  • src/renderer/src/lib/new-workspace.ts
  • src/renderer/src/lib/orca-hook-trust.ts
  • src/renderer/src/store/slices/ui/ui-slice-trust-actions.ts
  • src/shared/new-workspace-dialog-repo.ts
  • src/shared/orca-hook-setup-config.test.ts
  • src/shared/orca-hook-setup-config.ts
  • src/shared/orca-hook-trust-entries.test.ts
  • src/shared/orca-hook-trust-entries.ts
Files with no reviewable changes: 5
  • src/renderer/src/lib/launch-work-item-direct.test.ts
  • src/main/runtime/orca-runtime-test-mocks/setup.spec.ts
  • src/renderer/src/lib/new-workspace.test.ts
  • src/main/effective-hook-config.ts
  • src/renderer/src/lib/new-workspace.ts

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

@AmoabaKelvin
AmoabaKelvin force-pushed the mobile-setup-trust-parity branch from 2cc44b6 to ddd506b Compare October 27, 2026 02:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Filter the run-both local fallback from automation setup… · automation-setup-decision.ts:24-34

src/renderer/src/components/automations/automation-setup-decision.ts:24-34
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Filter the run-both local fallback from automation setup resolution.

For commandSourcePolicy: 'run-both', a local setup script with no YAML setup or defaultTabs command now returns a local setupConfig. getAutomationSetupSource exposes that config, so new_per_run receives run or skip instead of no setup decision. With run-by-default, saving the automation also enters ensureHooksConfirmed.

The merge-base automation resolver returned no config for this case. Filter setupConfig.source === 'local' when the effective policy is explicitly run-both in getAutomationSetupSource. Keep the shared fallback for workspace creation.

Suggested fix
   if (!setupConfig) {
     return null
   }
+  if (setupConfig.source === 'local' && hookSettings?.commandSourcePolicy === 'run-both') {
+    return null
+  }
   return {

Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e89e3aa0-2bf2-493e-b47a-aca2e2198ebe
Commits

Reviewing files that changed from the base of the PR and between 2cc44b6 and ddd506b.

Files selected for processing: 3
  • mobile/src/components/NewWorktreeModal.tsx
  • src/renderer/src/lib/launch-work-item-direct-preflight.ts
  • src/renderer/src/lib/launch-work-item-direct.test.ts
Files with no reviewable changes: 1
  • src/renderer/src/lib/launch-work-item-direct.test.ts

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

@AmoabaKelvin
AmoabaKelvin merged commit 71b55ab into main Oct 27, 2026
43 checks passed
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