Repository navigation
Conversation
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — Client-host environments now automatically use local editor execution instead of remote links or an unavailable action. A human must confirm this default routing change, including its precedence over SSH and advertised remote routes. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
binjamshid
left a comment
There was a problem hiding this comment.
Using the same client-host test as chat file links for "Open in" makes sense; for a desktop renderer whose host environment id matches, the service is on this machine, so local exec is correct even when an SSH profile also exists. Behaviour for non-desktop renderers is unchanged because isClientHostEnvironment only returns true there for a loopback PrimaryConnectionTarget, which the resolver already treated as local.
- low,
apps/web/src/remoteOpen.ts:129: this re-derives the flag inline with the same four arguments thatclientHostEnvironmentAtom(environmentId)instate/clientHostEnvironment.tsalready computes. Usingget(clientHostEnvironmentAtom(environmentId))would keep the two call sites from drifting if the host test changes later. - low: the new tests cover
resolveRemoteOpenStatewith the flag passed in, but nothing covers the atom wiring (host id arriving asynchronously fromgetHostEnvironmentId, so the state starts asremote-unavailableand flips tolocal-exec). Not a blocker givenremoteOpenStatesEqualhandles the flip, but worth a sentence in the PR about having checked it in the desktop app, since there is no before/after evidence of the actual "Open in" menu.
|
Thanks for this. Main has the change since #21661. Compared both changed files and all four added resolver cases with main: matching desktop-host bearer and SSH environments now resolve to local execution before SSH routing, while other environments retain their unavailable or SSH-link behavior. Main therefore already provides this PR's behavior. Closing as superseded. |
Summary
A desktop app paired with a background service on the same machine keeps that service as a plain
BearerConnectionTargetin the catalog.resolveRemoteOpenStateonly returnedlocal-execfor a primary connection target or a desktop-managed local backend, so a paired service with no SSH profile and no advertised hosts fell through toremote-unavailableand "Open in" dead-ended at "No SSH route to …".This makes editor resolution use the same "this machine" notion as chat file links:
resolveRemoteOpenStatetakes an optionalisClientHostEnvironmentand returnslocal-execafter the null-target check.remoteOpenResolutionAtomcomputes that flag withisClientHostEnvironment(environmentId, hostEnvironmentIdAtom, entry.target, isDesktopRenderer).Testing
pnpm installin the worktree.vp test run apps/web/src/remoteOpen.test.ts— 18 passed.vp test run apps/web/src/remoteOpen.test.ts apps/web/src/state/primaryEnvironment.test.ts— 22 passed.vp run --filter @t3tools/web typecheck— passed (EffectschemaClassMissingBrandsuggestions only, all pre-existing).vp run check:changed— format, lint, knip, typecheck, tests, and React compiler checks all passed.