Skip to content

[codex] Refactor shared and SSH Effect services#3206

Open
juliusmarminge wants to merge 26 commits into
codex/redact-dpop-request-targetfrom
codex/effect-service-shared-ssh
Open

[codex] Refactor shared and SSH Effect services#3206
juliusmarminge wants to merge 26 commits into
codex/redact-dpop-request-targetfrom
codex/effect-service-shared-ssh

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • standardize NetService, SshPasswordPrompt, SshEnvironmentManager, and the desktop SSH environment on inline service contracts and Service["Service"] references
  • export concrete-service make factories and canonical layer constructors
  • keep RelayClient abstract while preserving the implementation-specific makeCloudflaredRelayClient and layerCloudflared exports
  • split materially distinct shared/SSH and relay-install failures into concrete Schema.TaggedErrorClass errors, with category Schema.Union exports and diagnostic attributes preserved
  • use namespace imports only at service boundaries; retain named imports for contracts, errors, configuration, and helper-only APIs

Review fixes

  • split reason/operation-driven message switches into concrete error types while preserving category predicates and wire-level reason codes
  • preserve implementation-specific makeCloudflaredRelayClient and layerCloudflared naming for the abstract relay client port
  • retain sanitized readiness targets plus URL lengths while stripping credentials, query strings, and fragments from direct errors and logs\n- replace raw SSH command arrays, stdout/stderr, launch output, and pairing output with executable names, argument counts, exit codes, and byte counts; exact underlying causes remain chained

Scope

Orchestration and MCP modules are intentionally untouched. Existing suites changed only for service references and focused behavior/error-chain regressions; this PR adds no broad refactor-only test suite.

Validation

  • final focused post-review shared, SSH, and desktop suites: 8 files / 40 tests passed
  • vp check (passes with 20 existing warnings in files outside this PR)
  • vp run typecheck
  • git diff --check origin/main...HEAD
  • zero-diff audit for orchestration and MCP paths

Note

High Risk
Large cross-package error-model and SSH/desktop IPC boundary changes affect authentication, remote pairing, relay installation, and command resolution; regressions would surface as misclassified or stripped errors at the renderer.

Overview
Replaces broad Data.TaggedError types across SSH, shared (Net, shell, relayClient), desktop, contracts, and server with Schema.TaggedErrorClass unions, computed message getters, and structured fields (targets, byte counts, platform, sanitized URL diagnostics) instead of embedding raw stdout/stderr or full command lines in errors.

SSH splits monolithic failures into typed variants (SshCommandSpawnError, SshCommandExitError, password-prompt subclasses, launch/pairing/readiness/HTTP-bridge errors); desktop IPC maps prompt tags explicitly and keeps structural SSH errors at the invoke boundary. Relay install uses many concrete install errors server-side, maps them to wire reason on RelayClientInstallFailedError (internal cause not serialized). Auth/authorization drops redundant message fields from schemas (EnvironmentAuthorizationError, remote request unions).

Resilience: desktop legacy user-data and asset exists probes log warnings and fall back instead of failing; resolveCommandPath skips failed PATH **stat**s when a later candidate works; loopback port reservation gets distinct listen/address/release errors.

Tests shift to Schema.is, logging assertions, IPC cause identity, and trimmed generator suites.

Reviewed by Cursor Bugbot for commit 9ed4bf4. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Refactor SSH and shared Effect services to use structured, typed error classes

  • Replaces generic Data.TaggedError error classes across SSH, Net, relay client, and desktop services with Schema.TaggedErrorClass-based variants carrying structured fields (e.g. target, stdoutBytes, cause, platform) and standardized message getters.
  • SSH tunnel, command, and auth failures now emit specific typed errors (SshTunnelSpawnError, SshCommandExitError, SshAuthenticationHelperError, etc.) instead of a single SshCommandError; raw stdout/stderr are replaced by byte-length metrics in error payloads.
  • reserveLoopbackPort in Net.ts now distinguishes listen, address-unavailable, and release failures with dedicated error classes; resolveCommandPathForPlatform in shell.ts continues past per-candidate PermissionDenied errors and preserves the first failure as cause.
  • Desktop asset and user-data path probes no longer expose an error channel — probe failures are logged as warnings and fall back gracefully instead of propagating errors.
  • DesktopSshEnvironmentRequestError, RelayClientInstallFailedError, and EnvironmentAuthorizationError are migrated to schema-backed classes with computed message getters; encoded forms no longer include a message field.
  • Risk: all call sites consuming the removed generic error classes (SshCommandError, SshLaunchError, SshPairingError, SshHttpBridgeError, SshReadinessError, SshPasswordPromptError, NetError, RelayClientInstallError, CommandResolutionError) must be updated to handle the new typed variants.

Macroscope summarized 9ed4bf4.

@juliusmarminge juliusmarminge marked this pull request as ready for review June 20, 2026 02:07
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f91b532e-fe72-4065-8317-db135842eea7

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/effect-service-shared-ssh

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL 500-999 changed lines (additions + deletions). labels Jun 20, 2026
@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

🚀 Expo continuous deployment is ready!

  • Project → t3-code
  • Platforms → android, ios
  • Scheme → t3code-preview
  🤖 Android 🍎 iOS
Fingerprint fe5a51f2e189da69dfc4c2cd458e6cfb5fdff2ea ae3bd597809dfd7771d0898f735d172973d4c1c8
Build Details Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview:dev
Runtime version: fe5a51f2e189da69dfc4c2cd458e6cfb5fdff2ea
App version: 0.1.0
Git commit: cf1b582b1ac4b58a8fa629631da293314f81ca86
Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview:dev
Runtime version: ae3bd597809dfd7771d0898f735d172973d4c1c8
App version: 0.1.0
Git commit: d5d0b2332c4f1477d040363277fa6c2639bb84b8
Update Details Update Permalink
DetailsBranch: pr-3206
Runtime version: fe5a51f2e189da69dfc4c2cd458e6cfb5fdff2ea
Git commit: cf1b582b1ac4b58a8fa629631da293314f81ca86
Update Permalink
DetailsBranch: pr-3206
Runtime version: ae3bd597809dfd7771d0898f735d172973d4c1c8
Git commit: cf1b582b1ac4b58a8fa629631da293314f81ca86
Update QR

Comment thread packages/ssh/src/tunnel.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This major refactor converts error handling from Data.TaggedError to Schema.TaggedErrorClass across SSH, networking, and authentication code, changing runtime behavior in several places (e.g., fallback instead of fail on probe errors). The scope across 30+ files and changes to sensitive authentication/IPC code warrant careful human review.

You can customize Macroscope's approvability policy. Learn more.

@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch 2 times, most recently from 0b6d15f to b04704c Compare June 20, 2026 02:50
Comment thread packages/ssh/src/errors.ts Outdated
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch 2 times, most recently from 7106351 to a4af44e Compare June 20, 2026 03:58
@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Jun 20, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch from a4af44e to 91f873b Compare June 20, 2026 04:25
@macroscopeapp macroscopeapp Bot dismissed their stale review June 20, 2026 04:25

Dismissing prior approval to re-evaluate 91f873b

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch 4 times, most recently from 470e248 to c20caa0 Compare June 20, 2026 05:58
@macroscopeapp macroscopeapp Bot dismissed their stale review June 20, 2026 05:58

Dismissing prior approval to re-evaluate c20caa0

@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch from c20caa0 to 6999c3a Compare June 20, 2026 06:01
@juliusmarminge juliusmarminge marked this pull request as draft June 20, 2026 06:08
@juliusmarminge juliusmarminge marked this pull request as ready for review June 20, 2026 06:08
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch 4 times, most recently from 799b906 to bd827ee Compare June 20, 2026 06:47
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch from 2d458d2 to 0cc0afd Compare June 21, 2026 01:44
@juliusmarminge juliusmarminge force-pushed the codex/redact-dpop-request-target branch from 8d916bc to 7670d78 Compare June 21, 2026 01:51
@juliusmarminge juliusmarminge force-pushed the codex/effect-service-shared-ssh branch from 0cc0afd to 683a20e Compare June 21, 2026 01:51
@juliusmarminge juliusmarminge force-pushed the codex/redact-dpop-request-target branch from 7670d78 to 1ed6271 Compare June 21, 2026 02:03
juliusmarminge and others added 26 commits June 20, 2026 20:17
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant