Skip to content

Resolve and configure Source windows for remote hosts - #941

Merged
thesiti92 merged 5 commits into
mainfrom
remote/8-source-windows-trust
Oct 14, 2026
Merged

thesiti92 merged 5 commits into
mainfrom
remote/8-source-windows-trust

Conversation

@thesiti92

@thesiti92 thesiti92 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Stacked PR 8 of 9 for remote reviews (base: remote/7-remote-ask; #934, the rename, is merged). Review in order; each PR builds and passes its suites on its own.

Manual test plan

Purpose. Builds what a remote Source window needs before anything opens one: the whiteboard+<serverId> resolver over the review channel, the read-only pin and title for every Source window, and their registration in the navigator. A Source window layers settings as stock VS Code does, but no workspace file, folder .vscode/settings.json or host machine setting can lower its read-only rule. There is no UI entry point for a remote Source window yet (PR 9), so the remote half of the read-only check runs at PR 9's head.

Setup. Worktree at origin/remote/8-source-windows-trust. No host is needed for steps 1–3.

Steps.

  1. (cd apps/review-desktop && TSX_TSCONFIG_PATH=tsconfig.test.json node --import tsx --test --test-force-exit code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.test.ts code-oss/src/vs/review/services/configuration/reviewSourceWindowConfiguration.test.ts code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts) → all pass.
  2. Local read-only pin. The window shows the review's own checkout of the reviewed commit, so the settings file must be committed: W='{"files.readonlyInclude":{"**/*":false},"files.readonlyExclude":{"**/*":true},"window.title":"wb hostile title","editor.fontSize":40}'; awk '/^const fixture = String.raw/{f=1;next} /^;/{f=0} f' apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs | sed "s#^ git add \.\$# mkdir -p .vscode; echo '$W' > .vscode/settings.json; git add .#" > /tmp/wb-source.sh; mkdir -p $H/laptop $H/bin; printf '#!/bin/sh\nexec node %s/packages/review/dist/cli.js "$@"\n' "$PWD" > $H/bin/whiteboard; chmod +x $H/bin/whiteboard. With Desktop running: HOME=$H/laptop DEV_REVIEW_HOME=$H DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 PATH=$H/bin:$PATH bash -s -- Local-source Local-second < /tmp/wb-source.sh → a laptop review whose committed .vscode/settings.json tries to lift read-only.
  3. Open Local-source → Diff → Open file on f.ts → Source window, not titled wb hostile title; typing in f.ts changes nothing and shows the read-only notice.
  4. With a host online with language features, api /reviews-api → its entries still have available.sourceWindows: false, and the remote review's Diff view offers no Open file.
  5. At origin/remote/9-source-windows (needs PR 9's entry point), the same against a host, driven as remote-source-window drives it: S wb-test-a 'bash -s -- Remote-source Second-checkout' < /tmp/wb-source.sh (the step 2 script, so the checkout commits the same .vscode/settings.json), and echo "$W" | S wb-test-a 'mkdir -p ~/.dev/whiteboard-remote/server/data/Machine && cat > ~/.dev/whiteboard-remote/server/data/Machine/settings.json' for the host's machine settings. Add the host; Open file on f.ts → the window is titled Remote-source — Source — Whiteboard, its explorer shows .vscode, f.ts is read-only and typing shows the notice. Other settings layer as stock VS Code's: the host's editor.fontSize 40 applies.

Covered by unit tests.

  • reviewWindowAuthorityResolver.test.ts: waits up to 60 s for a connecting host, then fails <alias> is offline; re-resolves a moved forward; other authorities pass through.
  • reviewSourceWindowConfiguration.test.ts: the workspace file, a folder's settings and the host's machine settings cannot lower files.readonlyInclude/files.readonlyExclude.
  • reviewRemoteHosts.test.ts: removing a machine's last alias closes only its windows.

Cleanup. Quit Desktop; rm -rf $H /tmp/wb-source.sh; if steps 4–5 used a host, common cleanup.

Reviewer notes

  • Remote hosts are trusted the way VS Code Remote trusts them: a Source window layers settings and registers host extension contributions as stock VS Code does; only the read-only pin and the title stay Whiteboard's (docs/superpowers/plans/2026-10-05-adopt-vscode-trust-model.md, rulings T2, T3, T5).
  • Not a defect: after a first success the resolver waits on a host that is connecting (the stage-4 ruling); removing the host closes its windows, which ends the wait.
  • Not a defect: the resolver does not override getConnectionData; the base service caches it per authority.
Common setup for all plans

Common setup (read once)

  • Test each PR in a worktree at its head branch: git -C <core> worktree add --detach ../review-prN origin/<head>, then pnpm install there. Run everything below from that worktree root.
  • Helpers (bash or zsh):
R() { node apps/review-desktop/scripts/e2e/remote/remote.mjs "$@"; }
export WB_TEST_RUN=manual-$$                  # before the first `R up`
S() { ssh -F /tmp/wbt.$WB_TEST_RUN/ssh_config "$@"; }   # host aliases are wb-test-<name>
H=/tmp/wb-home-$WB_TEST_RUN                   # isolated Desktop home
mkdir -p $H/review-desktop/state/user-data/User
echo '{"review.experimental.remoteHosts.enabled": true}' > $H/review-desktop/state/user-data/User/settings.json
awk '/createRemoteReview = String.raw`/{f=1;next} /^`;/{f=0} f' apps/review-desktop/scripts/e2e/journeys/remote-host.mjs > /tmp/wb-review.sh
sed -e 's/open\\":false/open\\":true/' -e 's/wbrepo/wbrepo2/g' /tmp/wb-review.sh > /tmp/wb-push.sh
edit() { printf '{"sessionId":"%s","edit":{"type":"insert","content":{"type":"markdown","markdown":"%s"}}}' "$2" "$3" | S "$1" 'whiteboard api session_edit -'; }
  • Launch (dev build only; a packaged build ignores both env vars): DEV_REVIEW_HOME=$H DEV_FAST_REVIEW_SSH_CONFIG=/tmp/wbt.$WB_TEST_RUN/ssh_config pnpm dev (prefix DEV_FAST_REVIEW_DESKTOP_BACKGROUND=1 to keep it from taking focus).
  • Desktop API: D=$(ls $H/review-desktop/instances/dev-*.json); URL=$(node -p "require('$D').url"); TOK=$(node -p "require('$D').token"); api() { curl -s -H "x-review-token: $TOK" "$URL$1"; }
  • A remote review: ID=$(S wb-test-a 'bash -s -- Remote-order' < /tmp/wb-review.sh | head -1) (repo ~/wbrepo, review "Remote-order" with a prose line and a code peek of f.ts). /tmp/wb-push.sh is the same with open: true in ~/wbrepo2.
  • Settings: ⌘, → region Remote hosts → type the alias in SSH alias → Add.
  • Fork unit tests: (cd apps/review-desktop && TSX_TSCONFIG_PATH=tsconfig.test.json node --import tsx --test --test-force-exit <files>). Package tests: pnpm --filter @dev.fast/whiteboard test <files>.
  • Journeys: stage a runtime per apps/review-desktop/scripts/e2e/TESTING.md ("Staging the runtime"), then node apps/review-desktop/scripts/e2e/run.mjs --runtime "$REVIEW_E2E_RUNTIME" --journey <name>.
  • The harness has pause/resume (there is no unpause).
  • Cleanup, every plan: quit Desktop first (its ssh masters count as leftovers), then R down --all; R verify-clean; rm -rf $H /tmp/wb-*.sh. Without an aws sso login session verify-clean also prints AWS could not be checked and exits 1; no other line may appear.

Also runs Desktop's CI tests after the other packages: #939's remote tarball test starved the concurrent browser tests of CPU, so test files failed to import or hung until the 60-minute timeout (was #1132).

@thesiti92
thesiti92 added this pull request to stack #943 October 5, 2026 19:37
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from c2ef751 to ee2a783 Compare October 5, 2026 19:38
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch 2 times, most recently from 6437262 to a845e8c Compare October 5, 2026 20:19
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from a845e8c to c03c240 Compare October 5, 2026 20:34
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch 2 times, most recently from 37fd82d to c03c240 Compare October 5, 2026 20:41
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from c03c240 to f94f9ee Compare October 5, 2026 21:24
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from f94f9ee to 2092b59 Compare October 5, 2026 21:38
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 2092b59 to dd8c379 Compare October 5, 2026 21:47
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from dd8c379 to 4c9ade0 Compare October 5, 2026 23:08
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 4c9ade0 to 0515a89 Compare October 6, 2026 01:08
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 0515a89 to 915abf7 Compare October 6, 2026 01:58
@thesiti92 thesiti92 changed the title Guard a Source window bound to a remote host Resolve and configure Source windows for remote hosts Oct 6, 2026
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 915abf7 to 45a5e72 Compare October 6, 2026 02:46
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 45a5e72 to 13435fb Compare October 6, 2026 03:46
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from c424793 to 97aa0ee Compare October 11, 2026 13:34
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 97aa0ee to 326d918 Compare October 13, 2026 14:44
@thesiti92
thesiti92 removed this pull request from stack #943 October 13, 2026 15:38
@thesiti92
thesiti92 added this pull request to stack #1105 October 13, 2026 15:38
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch 2 times, most recently from 79698fa to 96f9e03 Compare October 13, 2026 17:22
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 96f9e03 to c94be8a Compare October 13, 2026 17:38
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from c94be8a to e37276a Compare October 13, 2026 21:49
@thesiti92
thesiti92 removed this pull request from stack #1105 October 13, 2026 22:42
@thesiti92
thesiti92 added this pull request to stack #1117 October 13, 2026 22:42
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch 2 times, most recently from 8465cc9 to 8b4e947 Compare October 13, 2026 22:54
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch from 8b4e947 to 9346c74 Compare October 14, 2026 00:50
@thesiti92
thesiti92 force-pushed the remote/8-source-windows-trust branch 3 times, most recently from 2b9a05a to 62fa8af Compare October 14, 2026 07:13
Desktop's remote tarball test (#939) builds Whiteboard, bundles, installs
and compresses on the same 4-vCPU runner as the Chromium browser tests.
Starved of CPU, Chromium drops a test file's module fetch: the file fails
with "Failed to fetch dynamically imported module", or its iframe never
reports ready and the job hangs until the 60-minute timeout.
getRemoteHostState returns the gateway's entry, detail included, and the window reads main's IPC answers with typed calls.
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.

2 participants