diff --git a/.github/workflows/adhoc-mac-build.yml b/.github/workflows/adhoc-mac-build.yml index 03d2c76ccc0..147e43c19dc 100644 --- a/.github/workflows/adhoc-mac-build.yml +++ b/.github/workflows/adhoc-mac-build.yml @@ -92,6 +92,11 @@ jobs: # HOURLY_RELEASE_APP_PRIVATE_KEY into it, then pin its deployment branch # policy to main so only main's copy of this file can read them. environment: adhoc-mac-build + outputs: + tag: ${{ steps.release.outputs.tag }} + version: ${{ steps.adhoc.outputs.version }} + head_sha: ${{ steps.adhoc.outputs.head_sha }} + published: ${{ steps.publish_live.outcome == 'success' && 'true' || 'false' }} runs-on: blacksmith-6vcpu-macos-15 # Why 150: it must exceed the worst case the retry budgets below can produce # (install 3x10 + publish 2x45 = 120, plus ~25 for checkout/build/verify), or @@ -290,16 +295,23 @@ jobs: --repo "$ADHOC_REPO" \ --title "$NAME" \ --draft \ - --notes "Adhoc macOS dev build of \`$REF\` at commit \`$short_sha\`. + --notes "Adhoc macOS and Windows dev build of \`$REF\` at commit \`$short_sha\`. Built from [\`stablyai/orca@$short_sha\`](https://github.com/stablyai/orca/commit/$SHA), cut by @$ACTOR. **Unlanded and unvetted.** This is somebody's branch, not main. No tests - ran. Signed and notarized like a release, so it installs through Orca's - in-app updater and opens without a Gatekeeper prompt — but the branch may - never merge, and this build is deleted after $ADHOC_RETAIN_DAYS days." + ran. The macOS build is signed and notarized like a release, so it installs + through Orca's in-app updater and opens without a Gatekeeper prompt — but the + branch may never merge, and this build is deleted after $ADHOC_RETAIN_DAYS days. + + **Windows builds are unsigned.** They install and update normally once you + are on one, but a signed Stable or RC build cannot install one through the + in-app updater — download \`orca-windows-setup.exe\` below and run it once + (SmartScreen will warn about an unknown publisher). Every later switch, + including back to Stable, works in-app from there." echo "tag=$TAG" >>"$GITHUB_OUTPUT" + - name: Publish adhoc macOS artifacts uses: nick-fields/retry@v4 with: @@ -416,3 +428,27 @@ jobs: gh release delete "$tag" --repo "$ADHOC_REPO" --yes --cleanup-tag || \ echo "::warning::Could not prune $tag" done <<<"$stale" + + # Why this runs after the mac leg rather than beside it: the tag and version + # are computed inside that job, so until it has run nothing else can name the + # release to upload into. The cost is small enough not to matter — the Windows + # leg measures ~7.5 min (install 2m45, build 35s, NSIS package 3m) against a + # ~9.5 min mac run, which keeps a adhoc run far inside its interval. + # + # Why `./` rather than a pinned `@main`: `uses:` resolves against the ref this + # file itself came from, which for an ordinary dispatch is main. Someone who + # deliberately points the Actions "Use workflow from" picker at a branch + # already gets that branch's copy of this entire file, so this follows the same + # rule instead of inventing a second one. + build-adhoc-win: + needs: build-adhoc-mac + # Only once the mac release is actually live: there is no release to upload + # into otherwise, and the Windows workflow refuses to create one. + if: needs.build-adhoc-mac.outputs.published == 'true' + uses: ./.github/workflows/dev-channel-win-build.yml + secrets: inherit + with: + channel: adhoc + tag: ${{ needs.build-adhoc-mac.outputs.tag }} + ref: ${{ needs.build-adhoc-mac.outputs.head_sha }} + version: ${{ needs.build-adhoc-mac.outputs.version }} diff --git a/.github/workflows/daily-mac-build.yml b/.github/workflows/daily-mac-build.yml index 3a99577bf74..e445045e67c 100644 --- a/.github/workflows/daily-mac-build.yml +++ b/.github/workflows/daily-mac-build.yml @@ -73,6 +73,8 @@ jobs: build-daily-mac: if: github.repository == 'stablyai/orca' outputs: + tag: ${{ steps.release.outputs.tag }} + version: ${{ steps.daily.outputs.version }} head_sha: ${{ steps.freshness.outputs.head_sha }} published: ${{ steps.publish_live.outcome == 'success' && 'true' || 'false' }} runs-on: blacksmith-6vcpu-macos-15 @@ -274,13 +276,19 @@ jobs: # exactly. One source, written once, read twice. notes_file="$RUNNER_TEMP/daily-release-notes.md" cat >"$notes_file" <>"$GITHUB_OUTPUT" echo "notes_file=$notes_file" >>"$GITHUB_OUTPUT" + - name: Publish daily macOS artifacts if: steps.freshness.outputs.should_build == 'true' uses: nick-fields/retry@v4 @@ -505,3 +514,27 @@ jobs: [[ "$attempt" -eq 3 ]] || sleep "$((attempt * 5))" done echo "::warning::Failed to dispatch post-daily E2E for $SHA after 3 attempts." + + # Why this runs after the mac leg rather than beside it: the tag and version + # are computed inside that job, so until it has run nothing else can name the + # release to upload into. The cost is small enough not to matter — the Windows + # leg measures ~7.5 min (install 2m45, build 35s, NSIS package 3m) against a + # ~9.5 min mac run, which keeps a daily run far inside its interval. + # + # Why `./` rather than a pinned `@main`: `uses:` resolves against the ref this + # file itself came from, which for an ordinary dispatch is main. Someone who + # deliberately points the Actions "Use workflow from" picker at a branch + # already gets that branch's copy of this entire file, so this follows the same + # rule instead of inventing a second one. + build-daily-win: + needs: build-daily-mac + # Only once the mac release is actually live: there is no release to upload + # into otherwise, and the Windows workflow refuses to create one. + if: needs.build-daily-mac.outputs.published == 'true' + uses: ./.github/workflows/dev-channel-win-build.yml + secrets: inherit + with: + channel: daily + tag: ${{ needs.build-daily-mac.outputs.tag }} + ref: ${{ needs.build-daily-mac.outputs.head_sha }} + version: ${{ needs.build-daily-mac.outputs.version }} diff --git a/.github/workflows/dev-channel-win-build.yml b/.github/workflows/dev-channel-win-build.yml new file mode 100644 index 00000000000..efb35628a28 --- /dev/null +++ b/.github/workflows/dev-channel-win-build.yml @@ -0,0 +1,304 @@ +name: Dev Channel Windows Build + +# Why its own file rather than steps inlined into hourly/daily/adhoc: one copy of +# the Windows leg instead of three, and it stays dispatchable on its own so a +# Windows artifact can be rebuilt for an existing tag without paying for the mac +# leg's packaging and notarization again. +# +# Each mac workflow calls this as a `needs:`-gated job once its release is live, +# passing the tag it created. Both legs land in that one release, so a tag +# carries every platform it managed to build. Measured cost of the Windows leg is +# ~7.5 min (install 2m45, build 35s, NSIS package 3m) against a mac run of ~9.5 +# min, so running it after the mac job keeps an hourly well inside its cron. +# +# Why unsigned: Windows release installers are signed by SignPath *after* +# packaging, and release-cut budgets 1h + 4h for those approval waits. That does +# not fit an hourly cadence and it does not fit "dispatch an adhoc build and go +# get coffee". So dev-channel Windows builds ship unsigned, which has one real +# consequence, handled in `src/shared/release-channel.ts`: +# +# electron-updater Authenticode-verifies every installer it downloads against +# the publisherName baked into the *installed* app's app-update.yml. Stable and +# RC carry 'SignPath Foundation', so they reject an unsigned dev installer and +# no future build can fix the copies already installed. Dev builds omit the +# name (config/electron-builder.config.cjs), so verification is skipped there. +# +# Net effect: the way *into* a dev channel on Windows is a one-time manual +# installer run. Every way out — to another dev build, or back to Stable — works +# through the in-app updater. The picker offers a download for exactly that jump. +# +# Called as a job by each mac workflow once its release is live, and separately +# dispatchable by hand to rebuild a Windows artifact for an existing tag without +# re-running the mac leg's twenty minutes of packaging and notarization: +# +# gh workflow run dev-channel-win-build.yml --ref main \ +# -f channel=adhoc -f tag=v1.4.178-adhoc.20260819010203 \ +# -f ref= -f version=1.4.178-adhoc.20260819010203 + +on: + # Why the inputs are duplicated: workflow_call does not accept `type: choice`, + # and workflow_dispatch wants it so the Actions UI offers a menu instead of a + # free-text box. The channel allowlist below is what actually enforces the set, + # since a workflow_call caller can pass any string. + workflow_call: + inputs: + channel: + description: Dev channel whose release this build uploads into + required: true + type: string + tag: + description: Existing release tag in the channel repo + required: true + type: string + ref: + description: Commit SHA to build — must be the exact commit the mac leg built + required: true + type: string + version: + description: Version to package, without the leading v + required: true + type: string + workflow_dispatch: + inputs: + channel: + description: Dev channel whose release this build uploads into + required: true + type: choice + options: + - hourly + - daily + - adhoc + tag: + description: Existing release tag in the channel repo (e.g. v1.4.178-adhoc.20260819010203) + required: true + type: string + ref: + description: Commit SHA to build — must be the exact commit the mac leg built + required: true + type: string + version: + description: Version to package, without the leading v + required: true + type: string + +permissions: + contents: read + +concurrency: + # Keyed on the tag: re-dispatching the same tag must not race two uploads into + # one release, but two different channels (or two adhoc branches) are the + # ordinary case and must not wait on each other. + group: dev-channel-win-build-${{ inputs.tag }} + cancel-in-progress: false + +jobs: + build-win: + if: github.repository == 'stablyai/orca' + # Why the same environment as the mac workflows: this needs the App token + # that can write to the dev-channel repos, and it should be reachable from + # exactly the same place those secrets already live. + environment: adhoc-mac-build + # Why windows-2022 and not windows-latest: windows-latest moved to the + # Windows 2025 / VS 2026 image before node-gyp could detect VS 18, breaking + # native dependency install. release-cut pins the same image. + runs-on: windows-2022 + timeout-minutes: 90 + env: + NODE_OPTIONS: --max-old-space-size=4096 + CHANNEL: ${{ inputs.channel }} + TAG: ${{ inputs.tag }} + VERSION: ${{ inputs.version }} + + steps: + # Why vet before checkout: everything after this runs the checked-out code + # with a token that can write to a release repo. The mac leg already vetted + # the ref it resolved, but this workflow is dispatchable on its own, so it + # re-derives the same guarantee rather than trusting its caller. + - name: Vet the requested inputs + id: vetted + shell: bash + env: + REQUESTED_SHA: ${{ inputs.ref }} + REPO_URL: https://github.com/${{ github.repository }} + run: | + set -euo pipefail + # workflow_call takes channel as a free-text string, so the set is + # enforced here rather than by the input type. + case "$CHANNEL" in + hourly|daily|adhoc) ;; + *) + echo "::error::Unknown dev channel '$CHANNEL'; expected hourly, daily, or adhoc." + exit 1 + ;; + esac + if [[ ! "$REQUESTED_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::ref must be a full 40-character commit SHA, got '$REQUESTED_SHA'. The dispatching workflow passes the commit it resolved." + exit 1 + fi + # The tag must name the version being packaged, or the artifacts would + # land in a release describing a different build. + if [[ "$TAG" != "v$VERSION" ]]; then + echo "::error::tag '$TAG' does not match version '$VERSION'." + exit 1 + fi + # And the version must carry the channel's own prerelease identifier, + # so an hourly artifact can never be uploaded into an adhoc release. + if [[ "$VERSION" != *"-$CHANNEL."* ]]; then + echo "::error::version '$VERSION' is not a $CHANNEL version." + exit 1 + fi + # Reachability is the trust test: GitHub serves PR-only commits by SHA, + # so resolving the object is not proof a branch or tag of this repo + # reaches it. Bare + tree:0 keeps this to the commit graph. + scratch="$RUNNER_TEMP/vet-requested-ref" + git init -q --bare "$scratch" + git -C "$scratch" fetch -q --filter=tree:0 "$REPO_URL" '+refs/heads/*:refs/heads/*' '+refs/tags/*:refs/tags/*' + if ! git -C "$scratch" rev-parse --verify --quiet "$REQUESTED_SHA^{commit}" >/dev/null; then + echo "::error::Commit $REQUESTED_SHA is not in stablyai/orca." + exit 1 + fi + if [[ -z "$(git -C "$scratch" for-each-ref --contains "$REQUESTED_SHA" refs/heads refs/tags | head -1)" ]]; then + echo "::error::Commit $REQUESTED_SHA is not reachable from any branch or tag of stablyai/orca; refusing to build it." + exit 1 + fi + echo "Vetted $CHANNEL $TAG at $REQUESTED_SHA" + + - name: Checkout the built commit + uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref }} + # This job only reads stablyai/orca and never pushes; every write goes + # to the dev-channel repo through a minted App token passed by env + # (zizmor: artipacked). + persist-credentials: false + + # Why a guard and not just a build: the workflow file comes from the + # dispatch ref, but the packaging config comes from the *built* commit. A + # branch cut before Windows dev builds landed has a config that ignores + # ORCA_WIN_*, which would resolve publish.repo to the main repo. Say that + # in one sentence here rather than failing deep inside electron-builder. + - name: Resolve the dev-channel packaging identity + shell: bash + run: | + set -euo pipefail + upper="$(printf '%s' "$CHANNEL" | tr '[:lower:]' '[:upper:]')" + echo "ORCA_WIN_${upper}=1" >>"$GITHUB_ENV" + echo "ORCA_${upper}_BUILD_VERSION=${VERSION}" >>"$GITHUB_ENV" + if [[ ! -f config/scripts/verify-dev-channel-packaging.mjs ]]; then + echo "::error::$TAG was built from a commit with no config/scripts/verify-dev-channel-packaging.mjs; that commit predates Windows dev builds, so it cannot produce one." + exit 1 + fi + + # pnpm must be on PATH before setup-node so setup-node can locate the store. + - name: Setup pnpm + uses: pnpm/action-setup@v6 + with: + run_install: false + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version-file: package.json + cache: pnpm + + # Caches the Electron binary and electron-builder's tool downloads (nsis, + # winCodeSign). Same key shape as release-cut's Windows leg. + - name: Cache electron-builder downloads + uses: actions/cache@v5 + with: + path: | + ~\AppData\Local\electron\Cache + ~\AppData\Local\electron-builder\Cache + key: electron-builder-win-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: | + electron-builder-win- + + # Why retried: pnpm install triggers electron's postinstall, which pulls the + # Electron binary from GitHub release assets, and that CDN returns transient + # 504s often enough to lose a build to it. + - name: Install dependencies + uses: nick-fields/retry@v4 + with: + timeout_minutes: 10 + max_attempts: 3 + retry_wait_seconds: 30 + command: pnpm install --frozen-lockfile + + # Why the packaging check runs before the 20-minute build: it only needs + # node_modules, and a stale config should cost seconds rather than a build. + - name: Verify dev-channel packaging identity + shell: bash + run: node config/scripts/verify-dev-channel-packaging.mjs --channel="$CHANNEL" --platform=win32 + + - name: Build app + shell: bash + run: pnpm build:release + env: + # Why unset ORCA_BUILD_IDENTITY: telemetry's transport gate accepts only + # 'stable' or 'rc', so leaving it unset keeps dev builds silent — which + # is correct for unvetted artifacts. Same as the mac dev channels. + ORCA_DIAGNOSTICS_TOKEN_URL: https://www.onorca.dev/diagnostics/token + + # Why the token is minted here and not at the top: installation tokens live + # one hour and nothing before this point writes anything. + - name: Mint dev channel repo token + id: app_token + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ secrets.HOURLY_RELEASE_APP_ID }} + private-key: ${{ secrets.HOURLY_RELEASE_APP_PRIVATE_KEY }} + owner: stablyai + repositories: orca-${{ inputs.channel }} + + # Why: electron-builder's publisher creates a release when it cannot find + # the tag ("publish: always"). If the mac leg failed and discarded its draft + # while this was building, that would mint a fresh, untitled, Windows-only + # release. Check first and fail instead. + - name: Confirm the target release still exists + shell: bash + env: + GH_TOKEN: ${{ steps.app_token.outputs.token }} + run: | + set -euo pipefail + if ! gh release view "$TAG" --repo "stablyai/orca-$CHANNEL" --json tagName >/dev/null 2>&1; then + echo "::error::Release $TAG no longer exists in stablyai/orca-$CHANNEL; the mac leg most likely failed and discarded it. Not creating a Windows-only release." + exit 1 + fi + + - name: Publish Windows artifacts + uses: nick-fields/retry@v4 + with: + # 30 is the pack + upload budget; there is no notary queue on this leg. + timeout_minutes: 30 + max_attempts: 2 + retry_wait_seconds: 30 + command: node config/scripts/ensure-native-runtime.mjs --runtime=electron; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }; pnpm exec electron-builder --config config/electron-builder.config.cjs --win --publish always + env: + GH_TOKEN: ${{ steps.app_token.outputs.token }} + ORCA_BUILD_COMMIT: ${{ inputs.ref }} + # Why: electron-publish refuses to upload into a release published more + # than two hours ago (gitHubPublisher.getOrCreateRelease). The mac leg + # publishes the draft live as soon as *it* finishes, so a slow notary + # queue plus a slow Windows build can cross that line and silently drop + # every Windows asset. This is the documented escape hatch. + EP_GH_IGNORE_TIME: 'true' + + # Why: the updater resolves a tag, then fetches latest.yml from it. A + # release carrying the installer but not the manifest is one the picker + # offers and the update 404s on, so assert both. + - name: Verify Windows update manifest published + shell: bash + env: + GH_TOKEN: ${{ steps.app_token.outputs.token }} + run: | + set -euo pipefail + assets="$(gh release view "$TAG" --repo "stablyai/orca-$CHANNEL" --json assets --jq '.assets[].name')" + echo "Assets on $TAG:" + echo "$assets" + for required in latest.yml orca-windows-setup.exe; do + if ! grep -qx "$required" <<<"$assets"; then + echo "::error::$TAG is missing $required; Windows could not install this build." + exit 1 + fi + done + echo "Windows artifacts verified on $TAG." diff --git a/.github/workflows/hourly-mac-build.yml b/.github/workflows/hourly-mac-build.yml index 7d4ecc44131..16e2f05579c 100644 --- a/.github/workflows/hourly-mac-build.yml +++ b/.github/workflows/hourly-mac-build.yml @@ -62,6 +62,11 @@ env: jobs: build-hourly-mac: if: github.repository == 'stablyai/orca' + outputs: + tag: ${{ steps.release.outputs.tag }} + version: ${{ steps.hourly.outputs.version }} + head_sha: ${{ steps.freshness.outputs.head_sha }} + published: ${{ steps.publish_live.outcome == 'success' && 'true' || 'false' }} runs-on: blacksmith-6vcpu-macos-15 # Why 150: it must exceed the worst case the retry budgets below can produce # (install 3x10 + publish 2x45 = 120, plus ~25 for checkout/build/verify), or @@ -270,15 +275,22 @@ jobs: --repo "$HOURLY_REPO" \ --title "$NAME" \ --draft \ - --notes "Automated hourly macOS dev build from commit \`$short_sha\`. + --notes "Automated hourly macOS and Windows dev build from commit \`$short_sha\`. Built from [\`stablyai/orca@$short_sha\`](https://github.com/stablyai/orca/commit/$SHA). - **Unvetted.** No tests ran. Signed and notarized like a release, so it - installs through Orca's in-app updater and opens from a manual download - without a Gatekeeper prompt — but nothing here has been reviewed." + **Unvetted.** No tests ran. The macOS build is signed and notarized like a + release, so it installs through Orca's in-app updater and opens from a manual + download without a Gatekeeper prompt — but nothing here has been reviewed. + + **Windows builds are unsigned.** They install and update normally once you + are on one, but a signed Stable or RC build cannot install one through the + in-app updater — download \`orca-windows-setup.exe\` below and run it once + (SmartScreen will warn about an unknown publisher). Every later switch, + including back to Stable, works in-app from there." echo "tag=$TAG" >>"$GITHUB_OUTPUT" + - name: Publish hourly macOS artifacts if: steps.freshness.outputs.should_build == 'true' uses: nick-fields/retry@v4 @@ -430,3 +442,27 @@ jobs: gh release delete "$tag" --repo "$HOURLY_REPO" --yes --cleanup-tag || \ echo "::warning::Could not prune $tag" done <<<"$stale" + + # Why this runs after the mac leg rather than beside it: the tag and version + # are computed inside that job, so until it has run nothing else can name the + # release to upload into. The cost is small enough not to matter — the Windows + # leg measures ~7.5 min (install 2m45, build 35s, NSIS package 3m) against a + # ~9.5 min mac run, which keeps a hourly run far inside its interval. + # + # Why `./` rather than a pinned `@main`: `uses:` resolves against the ref this + # file itself came from, which for an ordinary dispatch is main. Someone who + # deliberately points the Actions "Use workflow from" picker at a branch + # already gets that branch's copy of this entire file, so this follows the same + # rule instead of inventing a second one. + build-hourly-win: + needs: build-hourly-mac + # Only once the mac release is actually live: there is no release to upload + # into otherwise, and the Windows workflow refuses to create one. + if: needs.build-hourly-mac.outputs.published == 'true' + uses: ./.github/workflows/dev-channel-win-build.yml + secrets: inherit + with: + channel: hourly + tag: ${{ needs.build-hourly-mac.outputs.tag }} + ref: ${{ needs.build-hourly-mac.outputs.head_sha }} + version: ${{ needs.build-hourly-mac.outputs.version }} diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index f1a536bed5a..33e31aa2605 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -22,14 +22,27 @@ const { verifySkillsCliRuntime } = require('./scripts/verify-skills-cli-runtime. const isMacHourly = process.env.ORCA_MAC_HOURLY === '1' const isMacDaily = process.env.ORCA_MAC_DAILY === '1' const isMacAdhoc = process.env.ORCA_MAC_ADHOC === '1' +// Why a second set of variables rather than making the mac ones platform-neutral: +// the mac ones gate `isMacRelease` below, which turns on hardened runtime, +// notarization, and root-level `forceCodeSigning`. A Windows dev build that +// reused them would fail packaging outright for want of a cert it is +// deliberately not using. +const isWinHourly = process.env.ORCA_WIN_HOURLY === '1' +const isWinDaily = process.env.ORCA_WIN_DAILY === '1' +const isWinAdhoc = process.env.ORCA_WIN_ADHOC === '1' +const isWinDevChannel = isWinHourly || isWinDaily || isWinAdhoc const isMacRelease = process.env.ORCA_MAC_RELEASE === '1' || isMacHourly || isMacDaily || isMacAdhoc const isLinuxArm64Release = process.env.ORCA_LINUX_ARM64_RELEASE === '1' -const localBuildVersion = isMacRelease ? undefined : process.env.ORCA_LOCAL_BUILD_VERSION -const devChannelBuildVersion = isMacHourly +const localBuildVersion = + isMacRelease || isWinDevChannel ? undefined : process.env.ORCA_LOCAL_BUILD_VERSION +const isHourlyChannel = isMacHourly || isWinHourly +const isDailyChannel = isMacDaily || isWinDaily +const isAdhocChannel = isMacAdhoc || isWinAdhoc +const devChannelBuildVersion = isHourlyChannel ? process.env.ORCA_HOURLY_BUILD_VERSION - : isMacDaily + : isDailyChannel ? process.env.ORCA_DAILY_BUILD_VERSION - : isMacAdhoc + : isAdhocChannel ? process.env.ORCA_ADHOC_BUILD_VERSION : undefined // Why each dev channel gets its own repo rather than tagging into the main one: @@ -38,11 +51,11 @@ const devChannelBuildVersion = isMacHourly // to install. Keeping adhoc/daily separate from hourly too means a branch build // or a once-a-day cut cannot be picked up by someone who only meant to ride // main's hourlies. -const devChannelRepo = isMacHourly +const devChannelRepo = isHourlyChannel ? 'orca-hourly' - : isMacDaily + : isDailyChannel ? 'orca-daily' - : isMacAdhoc + : isAdhocChannel ? 'orca-adhoc' : null const appId = 'com.stablyai.orca' @@ -304,9 +317,18 @@ module.exports = { executableName: 'Orca', // Why: Windows installers are signed after electron-builder packaging by // SignPath, so the packager cannot infer the updater publisherName. - signtoolOptions: { - publisherName: 'SignPath Foundation' - }, + // + // Why dev channels drop it instead: they ship unsigned, because SignPath's + // approval waits are budgeted in hours and cannot fit an hourly cadence. + // electron-updater Authenticode-verifies every installer it downloads + // against the publisherName baked into the *installed* app's app-update.yml + // (NsisUpdater.verifySignature), and skips verification entirely when that + // name is absent. An unsigned build that still claimed 'SignPath Foundation' + // would therefore reject its own channel's next build — and its way back to + // stable with it. Dropping it is what makes dev→dev and dev→stable work. + ...(isWinDevChannel + ? { verifyUpdateCodeSignature: false } + : { signtoolOptions: { publisherName: 'SignPath Foundation' } }), extraResources: [ ...commonExtraResources, ...createPackagedRuntimeNodeModuleResources('win32'), diff --git a/config/scripts/dev-channel-windows-workflow-contract.test.mjs b/config/scripts/dev-channel-windows-workflow-contract.test.mjs new file mode 100644 index 00000000000..9133e1a9156 --- /dev/null +++ b/config/scripts/dev-channel-windows-workflow-contract.test.mjs @@ -0,0 +1,176 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' + +const projectDir = resolve(import.meta.dirname, '../..') + +const readWorkflow = (relativePath) => parse(readFileSync(join(projectDir, relativePath), 'utf8')) + +const MAC_WORKFLOWS = [ + ['hourly', '.github/workflows/hourly-mac-build.yml', 'build-hourly-mac'], + ['daily', '.github/workflows/daily-mac-build.yml', 'build-daily-mac'], + ['adhoc', '.github/workflows/adhoc-mac-build.yml', 'build-adhoc-mac'] +] + +const winWorkflow = () => readWorkflow('.github/workflows/dev-channel-win-build.yml') +const winSteps = () => winWorkflow().jobs['build-win'].steps +const stepNamed = (steps, name) => steps.find((step) => step.name === name) + +describe('dev-channel Windows build wiring', () => { + it.each(MAC_WORKFLOWS)( + 'calls the shared Windows workflow from %s with its own channel', + (channel, path, jobName) => { + const jobs = readWorkflow(path).jobs + const winJob = jobs[`build-${channel}-win`] + + expect(winJob).toBeDefined() + expect(winJob.uses).toBe('./.github/workflows/dev-channel-win-build.yml') + expect(winJob.with.channel).toBe(channel) + // Why `./` and not a pinned @main: `uses:` resolves against the ref this + // workflow file came from, which for an ordinary dispatch is main. A branch + // deliberately selected in the Actions picker already supplies its own copy + // of the whole file, so this follows that rule rather than adding a second. + expect(winJob.uses.startsWith('./')).toBe(true) + expect(winJob.needs).toBe(jobName) + expect(winJob.secrets).toBe('inherit') + } + ) + + // Why gated on published: the Windows leg uploads into a release the mac leg + // created, and refuses to create one itself. Without this it would run for a + // tag that was never published — or, for hourly, for a run that skipped. + it.each(MAC_WORKFLOWS)( + 'only runs the %s Windows leg once the release is live', + (channel, path, jobName) => { + expect(readWorkflow(path).jobs[`build-${channel}-win`].if).toBe( + `needs.${jobName}.outputs.published == 'true'` + ) + } + ) + + // The Windows job names the release by consuming these; a renamed step id + // would silently resolve them to empty strings and upload nowhere. + it.each(MAC_WORKFLOWS)( + 'exposes the %s tag, version and commit as job outputs', + (_c, path, jobName) => { + const job = readWorkflow(path).jobs[jobName] + const stepIds = job.steps.map((step) => step.id).filter(Boolean) + + for (const key of ['tag', 'version', 'head_sha', 'published']) { + expect(job.outputs[key]).toBeTruthy() + } + // Every referenced step id must actually exist in the job. + for (const expression of Object.values(job.outputs)) { + const referenced = [...expression.matchAll(/steps\.([A-Za-z0-9_-]+)\./g)].map((m) => m[1]) + for (const id of referenced) { + expect(stepIds).toContain(id) + } + } + } + ) + + // Why this is asserted: the previous design dispatched a sibling run, which + // needed actions:write on a job holding macOS signing credentials. Calling the + // workflow directly removes that, and it must not creep back. + it.each(MAC_WORKFLOWS)( + 'does not widen the %s signing job to actions:write', + (_c, path, jobName) => { + expect(readWorkflow(path).jobs[jobName].permissions?.actions).toBeUndefined() + } + ) +}) + +describe('dev-channel Windows build workflow', () => { + it('is both callable by the mac workflows and dispatchable on its own', () => { + const triggers = winWorkflow().on ?? winWorkflow()[true] + + expect(Object.keys(triggers)).toEqual( + expect.arrayContaining(['workflow_call', 'workflow_dispatch']) + ) + // workflow_call cannot express `type: choice`, so the channel set has to be + // enforced in the job itself. + expect(stepNamed(winSteps(), 'Vet the requested inputs').run).toContain('hourly|daily|adhoc') + }) + + // Why windows-2022: windows-latest moved to the Windows 2025 / VS 2026 image + // before node-gyp could detect VS 18, breaking native dependency install. + it('pins the same Windows image release-cut builds on', () => { + expect(winWorkflow().jobs['build-win']['runs-on']).toBe('windows-2022') + }) + + // The guard against a branch whose electron-builder config predates Windows + // dev builds: without it, publish.repo resolves to the main repo. + it('verifies the dev-channel packaging identity before building', () => { + const steps = winSteps() + const names = steps.map((step) => step.name) + const verify = names.indexOf('Verify dev-channel packaging identity') + const build = names.indexOf('Build app') + + expect(verify).toBeGreaterThanOrEqual(0) + expect(build).toBeGreaterThan(verify) + expect(stepNamed(steps, 'Verify dev-channel packaging identity').run).toContain( + 'verify-dev-channel-packaging.mjs' + ) + }) + + // Why: electron-publish creates a release when it cannot find the tag under + // `--publish always`. If the mac leg discarded its draft while this was + // building, that would mint an untitled Windows-only release. + it('confirms the target release exists before publishing into it', () => { + const names = winSteps().map((step) => step.name) + const confirm = names.indexOf('Confirm the target release still exists') + const publish = names.indexOf('Publish Windows artifacts') + + expect(confirm).toBeGreaterThanOrEqual(0) + expect(publish).toBeGreaterThan(confirm) + }) + + // electron-publish refuses to upload into a release published more than two + // hours ago. The mac leg publishes as soon as it finishes, so a slow notary + // queue plus a slow Windows build crosses that line and drops every asset. + it('opts out of the publisher two-hour upload window', () => { + expect(stepNamed(winSteps(), 'Publish Windows artifacts').env.EP_GH_IGNORE_TIME).toBe('true') + }) + + it('packages Windows unsigned through the shared electron-builder config', () => { + const publish = stepNamed(winSteps(), 'Publish Windows artifacts') + + expect(publish.with.command).toContain( + 'electron-builder --config config/electron-builder.config.cjs --win --publish always' + ) + // No signing env: SignPath's approval waits cannot fit a dev cadence, which + // is the entire reason these builds are unsigned. + expect(Object.keys(publish.env)).not.toContain('SIGNPATH_API_TOKEN') + }) + + // Why both: a release carrying the installer but not the manifest is a row the + // picker offers and the in-app update 404s on. + it('requires the manifest and the installer before calling the build good', () => { + const verify = stepNamed(winSteps(), 'Verify Windows update manifest published') + + expect(verify.run).toContain('latest.yml') + expect(verify.run).toContain('orca-windows-setup.exe') + }) + + // Why: this is callable and separately dispatchable, and workflow_call takes + // its inputs as free text, so it re-derives the ref guarantee rather than + // trusting whoever called it. + it('vets the requested commit before checking it out', () => { + const names = winSteps().map((step) => step.name) + const vet = names.indexOf('Vet the requested inputs') + const checkout = names.indexOf('Checkout the built commit') + + expect(vet).toBe(0) + expect(checkout).toBeGreaterThan(vet) + expect(stepNamed(winSteps(), 'Vet the requested inputs').run).toContain('--contains') + }) + + // Telemetry's transport gate accepts only 'stable' or 'rc'; leaving the build + // identity unset is what keeps unvetted artifacts silent. + it('never stamps an official telemetry build identity', () => { + const build = stepNamed(winSteps(), 'Build app') + + expect(Object.keys(build.env ?? {})).not.toContain('ORCA_BUILD_IDENTITY') + }) +}) diff --git a/config/scripts/verify-dev-channel-packaging.mjs b/config/scripts/verify-dev-channel-packaging.mjs new file mode 100644 index 00000000000..7f2067552d5 --- /dev/null +++ b/config/scripts/verify-dev-channel-packaging.mjs @@ -0,0 +1,125 @@ +#!/usr/bin/env node +// Why this exists: the dev-channel workflows run from main, but they build (and +// therefore read `config/electron-builder.config.cjs` from) whatever ref was +// asked for. A branch cut before Windows dev builds landed has a config that +// ignores ORCA_WIN_*, which would resolve `publish.repo` to the *main* repo and +// leave the release identity signed-looking. Publishing would then fail deep +// inside electron-builder with a 404 from a token scoped to the dev repo — or, +// worse, succeed against a repo it was never meant to touch. +// +// So: load the config exactly as electron-builder will, and assert the identity +// it produced matches the channel and platform the workflow believes it is +// building. Runs before packaging, fails with a sentence someone can act on. + +import { createRequire } from 'node:module' +import { resolve } from 'node:path' +import { pathToFileURL } from 'node:url' + +const CHANNEL_REPOS = { + hourly: 'orca-hourly', + daily: 'orca-daily', + adhoc: 'orca-adhoc' +} + +const CHANNEL_VERSION_ENV = { + hourly: 'ORCA_HOURLY_BUILD_VERSION', + daily: 'ORCA_DAILY_BUILD_VERSION', + adhoc: 'ORCA_ADHOC_BUILD_VERSION' +} + +export function collectDevChannelPackagingProblems({ channel, platform, config, env }) { + const problems = [] + const expectedRepo = CHANNEL_REPOS[channel] + if (!expectedRepo) { + return [ + `Unknown dev channel "${channel}"; expected one of ${Object.keys(CHANNEL_REPOS).join(', ')}.` + ] + } + + if (config.publish?.repo !== expectedRepo) { + problems.push( + `publish.repo is "${config.publish?.repo}" but this ${channel} build must publish to "${expectedRepo}". ` + + `The checked-out ref's electron-builder config does not understand this channel on ${platform} — rebase it onto a main that does.` + ) + } + + if (config.publish?.releaseType !== 'prerelease') { + problems.push( + `publish.releaseType is "${config.publish?.releaseType}" but dev-channel builds must publish as "prerelease".` + ) + } + + // Why version too: `extraMetadata.version` is what stamps the tag the workflow + // already created. A config that dropped it would package package.json's + // version and upload into the wrong release entirely. + const expectedVersion = env[CHANNEL_VERSION_ENV[channel]] + if (expectedVersion && config.extraMetadata?.version !== expectedVersion) { + problems.push( + `extraMetadata.version is "${config.extraMetadata?.version}" but the workflow computed "${expectedVersion}".` + ) + } + + if (platform === 'win32') { + // The one that silently breaks updates rather than failing the build: a dev + // build that advertises a publisherName can never install its own channel's + // next build, because electron-updater verifies against the name baked into + // the installed app. + if (config.win?.verifyUpdateCodeSignature !== false) { + problems.push( + 'win.verifyUpdateCodeSignature must be false for unsigned dev builds, or electron-updater will Authenticode-verify every installer this build downloads and reject all of them.' + ) + } + if (config.win?.signtoolOptions?.publisherName != null) { + problems.push( + `win.signtoolOptions.publisherName is set to "${config.win.signtoolOptions.publisherName}" on an unsigned dev build; it must be absent.` + ) + } + } + + return problems +} + +function parseArgs(argv) { + const args = {} + for (const entry of argv) { + const match = /^--([^=]+)=(.*)$/.exec(entry) + if (match) { + args[match[1]] = match[2] + } + } + return args +} + +function main() { + const { channel, platform = process.platform } = parseArgs(process.argv.slice(2)) + if (!channel) { + console.error( + 'Usage: verify-dev-channel-packaging.mjs --channel= [--platform=win32]' + ) + process.exit(1) + } + const require = createRequire(import.meta.url) + const config = require(resolve(import.meta.dirname, '../electron-builder.config.cjs')) + const problems = collectDevChannelPackagingProblems({ + channel, + platform, + config, + env: process.env + }) + if (problems.length > 0) { + for (const problem of problems) { + console.error(`::error::${problem}`) + } + process.exit(1) + } + console.log( + `Dev-channel packaging verified: ${channel} on ${platform} → stablyai/${CHANNEL_REPOS[channel]} @ ${config.extraMetadata?.version}` + ) +} + +// Why the guard: the test imports the pure collector without running the CLI. +// pathToFileURL, not a `file://` template: this also runs on Windows, where a +// drive-letter path does not concatenate into a valid URL. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main() +} diff --git a/config/scripts/verify-dev-channel-packaging.test.mjs b/config/scripts/verify-dev-channel-packaging.test.mjs new file mode 100644 index 00000000000..63e1c7d5b0c --- /dev/null +++ b/config/scripts/verify-dev-channel-packaging.test.mjs @@ -0,0 +1,174 @@ +import { createRequire } from 'node:module' +import { resolve } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { collectDevChannelPackagingProblems } from './verify-dev-channel-packaging.mjs' + +const require = createRequire(import.meta.url) +const CONFIG_PATH = resolve(import.meta.dirname, '../electron-builder.config.cjs') + +/** The config reads process.env at require time, so each identity needs a fresh load. */ +function loadConfigWithEnv(env) { + const saved = { ...process.env } + for (const key of Object.keys(process.env)) { + if (key.startsWith('ORCA_')) { + delete process.env[key] + } + } + Object.assign(process.env, env) + try { + delete require.cache[require.resolve(CONFIG_PATH)] + return require(CONFIG_PATH) + } finally { + process.env = saved + delete require.cache[require.resolve(CONFIG_PATH)] + } +} + +const WIN_ADHOC_ENV = { + ORCA_WIN_ADHOC: '1', + ORCA_ADHOC_BUILD_VERSION: '1.4.178-adhoc.20260819010203' +} + +afterEach(() => { + delete require.cache[require.resolve(CONFIG_PATH)] +}) + +describe('electron-builder dev-channel identity', () => { + it('keeps the SignPath publisherName on stable Windows builds', () => { + const config = loadConfigWithEnv({}) + + expect(config.win.signtoolOptions.publisherName).toBe('SignPath Foundation') + expect(config.win.verifyUpdateCodeSignature).toBeUndefined() + expect(config.publish.repo).toBe('orca') + expect(config.publish.releaseType).toBe('release') + }) + + // The whole point of the change: an unsigned build that advertised a + // publisherName would Authenticode-verify — and reject — every installer it + // ever downloaded, including its own way back to stable. + it('drops the publisherName and disables update signature checks on Windows dev builds', () => { + const config = loadConfigWithEnv(WIN_ADHOC_ENV) + + expect(config.win.signtoolOptions?.publisherName).toBeUndefined() + expect(config.win.verifyUpdateCodeSignature).toBe(false) + }) + + it.each([ + ['hourly', { ORCA_WIN_HOURLY: '1' }, 'orca-hourly'], + ['daily', { ORCA_WIN_DAILY: '1' }, 'orca-daily'], + ['adhoc', { ORCA_WIN_ADHOC: '1' }, 'orca-adhoc'] + ])('publishes %s Windows builds to its own repo as a prerelease', (_channel, env, repo) => { + const config = loadConfigWithEnv(env) + + expect(config.publish.repo).toBe(repo) + expect(config.publish.releaseType).toBe('prerelease') + }) + + // Why: ORCA_MAC_* gates hardened runtime, notarization, and root-level + // forceCodeSigning. If the Windows variables leaked into that, the Windows job + // would fail packaging for want of a cert it deliberately does not use. + it('leaves mac release signing off for Windows dev builds', () => { + const config = loadConfigWithEnv(WIN_ADHOC_ENV) + + expect(config.forceCodeSigning).toBe(false) + expect(config.mac.notarize).toBe(false) + expect(config.mac.hardenedRuntime).toBe(false) + }) + + it('still notarizes mac dev builds', () => { + const config = loadConfigWithEnv({ + ORCA_MAC_ADHOC: '1', + ORCA_ADHOC_BUILD_VERSION: '1.4.178-adhoc.20260819010203' + }) + + expect(config.mac.notarize).toBe(true) + expect(config.publish.repo).toBe('orca-adhoc') + }) +}) + +describe('collectDevChannelPackagingProblems', () => { + const goodWinConfig = { + publish: { repo: 'orca-adhoc', releaseType: 'prerelease' }, + extraMetadata: { version: '1.4.178-adhoc.20260819010203' }, + win: { verifyUpdateCodeSignature: false } + } + const env = { ORCA_ADHOC_BUILD_VERSION: '1.4.178-adhoc.20260819010203' } + + it('accepts a correctly configured Windows dev build', () => { + expect( + collectDevChannelPackagingProblems({ + channel: 'adhoc', + platform: 'win32', + config: goodWinConfig, + env + }) + ).toEqual([]) + }) + + // The failure this script exists for: a branch predating Windows dev builds + // resolves publish.repo to the main repo. + it('rejects a config that resolved the main repo', () => { + const problems = collectDevChannelPackagingProblems({ + channel: 'adhoc', + platform: 'win32', + config: { ...goodWinConfig, publish: { repo: 'orca', releaseType: 'release' } }, + env + }) + + expect(problems.join('\n')).toContain('must publish to "orca-adhoc"') + expect(problems.join('\n')).toContain('rebase it onto a main that does') + }) + + it('rejects a Windows dev build that still advertises a publisherName', () => { + const problems = collectDevChannelPackagingProblems({ + channel: 'adhoc', + platform: 'win32', + config: { + ...goodWinConfig, + win: { signtoolOptions: { publisherName: 'SignPath Foundation' } } + }, + env + }) + + expect(problems.join('\n')).toContain('verifyUpdateCodeSignature must be false') + expect(problems.join('\n')).toContain('publisherName is set to "SignPath Foundation"') + }) + + it('rejects a build packaging a version other than the tag the workflow created', () => { + const problems = collectDevChannelPackagingProblems({ + channel: 'adhoc', + platform: 'win32', + config: { ...goodWinConfig, extraMetadata: { version: '1.4.178' } }, + env + }) + + expect(problems.join('\n')).toContain('but the workflow computed') + }) + + // macOS builds are signed, so the Windows-only assertions must not fire there. + it('does not apply Windows signature rules to a mac dev build', () => { + expect( + collectDevChannelPackagingProblems({ + channel: 'adhoc', + platform: 'darwin', + config: { + publish: { repo: 'orca-adhoc', releaseType: 'prerelease' }, + extraMetadata: { version: '1.4.178-adhoc.20260819010203' }, + win: { signtoolOptions: { publisherName: 'SignPath Foundation' } } + }, + env + }) + ).toEqual([]) + }) + + it('rejects an unknown channel', () => { + expect( + collectDevChannelPackagingProblems({ + channel: 'nightly', + platform: 'win32', + config: goodWinConfig, + env + }) + ).toEqual(['Unknown dev channel "nightly"; expected one of hourly, daily, adhoc.']) + }) +}) diff --git a/mobile/src/tasks/worktree-create-retry.test.ts b/mobile/src/tasks/worktree-create-retry.test.ts index 74e7806a64f..30249d206a6 100644 --- a/mobile/src/tasks/worktree-create-retry.test.ts +++ b/mobile/src/tasks/worktree-create-retry.test.ts @@ -1,24 +1,87 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' +import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' import { LogicalClientCutoverError } from '../transport/logical-client-cutover-error' -import { createWorktreeWithNameRetry } from './worktree-create-retry' +import type { ConnectionState } from '../transport/types' +import { WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../../src/shared/new-workspace/worktree-create-retry-policy' +import { + createWorktreeWithNameRetry, + WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS, + WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS +} from './worktree-create-retry' +import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout' type Attempt = { method: string; params: Record } +// Drives the transport state a replay has to wait on. Production couples the two: +// a socket close rejects the pending frame AND drops the client off 'connected'. +function connectionController(): { + getState: () => ConnectionState + onStateChange: (listener: (state: ConnectionState) => void) => () => void + set: (next: ConnectionState) => void +} { + let state: ConnectionState = 'connected' + const listeners = new Set<(next: ConnectionState) => void>() + return { + getState: () => state, + onStateChange: (listener) => { + listeners.add(listener) + return () => listeners.delete(listener) + }, + set: (next) => { + state = next + for (const listener of listeners) { + listener(next) + } + } + } +} + +// Lets a parked replay reach its state wait before the test resumes the transport. +async function flush(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)) +} + // A client whose per-call outcome is scripted: return an id, a server error // message, or throw (transport-level rejection, e.g. a connection-migration // cutover). Records every call so tests can assert on the clientMutationId. function scriptedClient( - outcomes: Array<{ id: string } | { errorMessage: string } | { throws: unknown }>, - attempts: Attempt[] + outcomes: Array< + | { id: string } + | { errorMessage: string } + // takesMs models how long the ambiguity took to SURFACE — a clean close is + // instant, a half-open socket waits out the liveness watchdog or the timeout. + // reconnectsAfterMs brings the transport back so the next replay can run. + | { throws: unknown; dropsConnection?: boolean; takesMs?: number; reconnectsAfterMs?: number } + >, + attempts: Attempt[], + connection?: ReturnType, + // Wall-clock stamp of the last frame that really arrived. Absent = a transport that + // can't vouch for one, which must fall back to the send; a function models a live + // socket whose stamp keeps advancing. + lastInboundAt?: number | (() => number) ): RpcClient { let call = 0 return { + getState: () => connection?.getState() ?? 'connected', + getLastInboundAt: () => + (typeof lastInboundAt === 'function' ? lastInboundAt() : lastInboundAt) ?? null, + onStateChange: (listener: (state: ConnectionState) => void) => + connection?.onStateChange(listener) ?? (() => {}), sendRequest: async (method: string, params?: unknown) => { attempts.push({ method, params: (params ?? {}) as Record }) const outcome = outcomes[Math.min(call, outcomes.length - 1)]! call += 1 if ('throws' in outcome) { + if (outcome.takesMs !== undefined) { + await new Promise((resolve) => setTimeout(resolve, outcome.takesMs)) + } + if (outcome.dropsConnection) { + connection?.set('reconnecting') + if (outcome.reconnectsAfterMs !== undefined) { + setTimeout(() => connection?.set('connected'), outcome.reconnectsAfterMs) + } + } throw outcome.throws } if ('errorMessage' in outcome) { @@ -117,6 +180,8 @@ describe('createWorktreeWithNameRetry', () => { expect(attempts).toHaveLength(6) }) + // Also the delivery-ambiguity guard: an unmarked error is a DEFINITE failure, so + // neither the cutover nor the replay path may re-issue it. it('does not treat an ordinary transport error as a cutover', async () => { const attempts: Attempt[] = [] const client = scriptedClient([{ throws: new Error('Request timed out') }], attempts) @@ -188,4 +253,407 @@ describe('createWorktreeWithNameRetry', () => { expect(attempts).toHaveLength(1) expect(attempts[0]!.params.clientMutationId).toBeUndefined() }) + + it('replays a delivery-ambiguous socket drop with the SAME key once the transport returns', async () => { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true + }, + { id: 'wt-drop' } + ], + attempts, + connection + ) + + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'urchin', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-drop' + }) + + await flush() + // Parked on the reconnect, not resent onto a dead socket. + expect(attempts).toHaveLength(1) + + connection.set('connected') + await expect(pending).resolves.toEqual({ worktreeId: 'wt-drop', name: 'urchin' }) + expect(attempts).toHaveLength(2) + // Idempotency: the host dedupes the replay against the create it may already + // have finished, instead of building a second worktree. + expect(attempts[1]!.params.clientMutationId).toBe('key-drop') + expect(attempts[1]!.params.name).toBe('urchin') + }) + + it('does not replay a delivery-ambiguous drop when the host lacks idempotency support', async () => { + const attempts: Attempt[] = [] + const client = scriptedClient( + [{ throws: markRpcDeliveryUnknown(new Error('Connection interrupted')) }], + attempts + ) + await expect( + createWorktreeWithNameRetry({ + client, + baseName: 'limpet', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: false, + mintMutationId: () => 'must-not-be-used' + }) + ).rejects.toThrow('Connection interrupted') + expect(attempts).toHaveLength(1) + expect(attempts[0]!.params.clientMutationId).toBeUndefined() + }) + + it('gives up after the delivery-ambiguity replay budget, still inside the dedupe TTL', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true, + reconnectsAfterMs: 1_000 + } + ], + attempts, + connection + ) + const startedAt = Date.now() + let settledAt = -1 + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'barnacle', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-budget' + }).catch((error: unknown) => { + settledAt = Date.now() + throw error + }) + const settled = expect(pending).rejects.toThrow('Connection interrupted') + await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_DEDUPE_TTL_MS) + await settled + // Initial attempt + 2 replays. + expect(attempts).toHaveLength(3) + // The budget is a count, but what keeps a replay reconciling instead of building + // a second worktree is wall clock: every attempt has to land while the host still + // holds the record, with the watchdog's detection latency already deducted. + expect(settledAt - startedAt).toBeLessThanOrEqual(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('surfaces the original ambiguity when the transport never comes back', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true + } + ], + attempts, + connection + ) + + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'anemone', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-stuck' + }) + const settled = expect(pending).rejects.toThrow('Connection interrupted') + // A phone that never reconnects must not leave the Create spinner parked. + await vi.advanceTimersByTimeAsync(60_000) + await settled + expect(attempts).toHaveLength(1) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('does not replay an ambiguity that surfaced while the transport stayed connected', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + [ + { + // A silently dropped response frame leaves the socket alive, so nothing + // rejects until the request timeout — by then the host resolved at an + // unknowable point and its dedupe record is very likely gone. + throws: markRpcDeliveryUnknown(new Error('Request timed out: worktree.create')), + takesMs: WORKTREE_CREATE_TIMEOUT_MS + }, + { id: 'wt-duplicate' } + ], + attempts, + connection, + // Watchdog probes keep answering throughout, so the replay window looks wide + // open on arrival: only the still-connected guard can stop this replay. + () => Date.now() + ) + + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'cuttlefish', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-expired' + }) + const settled = expect(pending).rejects.toThrow('Request timed out') + await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_TIMEOUT_MS) + await settled + // Never left 'connected', so no drop was ever detected and the staleness of the + // ambiguity is unbounded. Re-sending would miss the record and build a SECOND + // worktree — for a folder workspace, one with the very same name. + expect(connection.getState()).toBe('connected') + expect(attempts).toHaveLength(1) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('clamps a later reconnect wait to what is left of the replay window', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + [ + { + // Most of the window is already gone by the time the drop is reported, so the + // remainder — not the full wait — is what the second wait gets. + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true, + takesMs: 30_000, + reconnectsAfterMs: 1_000 + }, + { + // Second drop, and this time the phone never comes back. + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true + } + ], + attempts, + connection + ) + + const startedAt = Date.now() + let settledAt = -1 + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'nautilus', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-clamped' + }).catch((error: unknown) => { + settledAt = Date.now() + throw error + }) + const settled = expect(pending).rejects.toThrow('Connection interrupted') + await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS) + await settled + expect(attempts).toHaveLength(2) + // The window is anchored at the FIRST ambiguity, so the second wait gets only the + // remainder — it must not restart a full wait and carry the replay past the record. + expect(settledAt - startedAt).toBe(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('refuses a replay when the ambiguity surfaced long after the last inbound frame', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const startedAt = Date.now() + const client = scriptedClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true, + reconnectsAfterMs: 1_000, + // The phone was backgrounded: the OS suspended JS and killed the socket, and + // nothing observed the drop until the app came back ten minutes later. + takesMs: 600_000 + } + ], + attempts, + connection, + startedAt + 2_000 + ) + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'limpet', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-stale' + }) + const settled = expect(pending).rejects.toThrow('Connection interrupted') + await vi.advanceTimersByTimeAsync(700_000) + await settled + // The host forgot this create ~9 minutes ago. A replay would not reconcile — it + // would build a second worktree — so the create fails instead. + expect(attempts).toHaveLength(1) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('still replays a long-running create whose socket stayed live until it dropped', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const startedAt = Date.now() + const client = scriptedClient( + [ + { + // Three minutes of clone/fetch with the socket answering probes throughout, + // then it drops and the watchdog reports it four seconds later. + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true, + reconnectsAfterMs: 1_000, + takesMs: 184_000 + }, + { id: 'wt-long' } + ], + attempts, + connection, + startedAt + 180_000 + ) + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'kelp', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-long' + }) + await vi.advanceTimersByTimeAsync(250_000) + // Anchoring at the send would refuse this — the create outran the TTL long before it + // went ambiguous — but the record only starts ticking when the host RESOLVES, and the + // live socket proves our knowledge was current. + expect(await pending).toEqual({ worktreeId: 'wt-long', name: 'kelp' }) + expect(attempts).toHaveLength(2) + expect(attempts[0]!.params.clientMutationId).toBe('key-long') + expect(attempts[1]!.params.clientMutationId).toBe('key-long') + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('does not extend the replay window when the replacement session reports fresher inbound activity', async () => { + vi.useFakeTimers() + try { + const attempts: Attempt[] = [] + const connection = connectionController() + const startedAt = Date.now() + // The replacement session starts answering probes, so its inbound stamp is far + // newer than anything that bears on when the ORIGINAL create resolved host-side. + let lastInboundAt = startedAt + 1_000 + setTimeout(() => { + lastInboundAt = startedAt + 45_000 + }, 45_000) + const client = scriptedClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection lost')), + dropsConnection: true, + takesMs: 40_000, + reconnectsAfterMs: 1_000 + }, + { + throws: markRpcDeliveryUnknown(new Error('Connection lost')), + dropsConnection: true, + takesMs: 20_000, + reconnectsAfterMs: 1_000 + }, + { id: 'wt-third' } + ], + attempts, + connection, + () => lastInboundAt + ) + + const pending = createWorktreeWithNameRetry({ + client, + baseName: 'urchin', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-anchored' + }) + const settled = expect(pending).rejects.toThrow('Connection lost') + await vi.advanceTimersByTimeAsync(120_000) + await settled + // The deadline is fixed at the FIRST ambiguity. Re-reading it here would buy another + // 50s of window off a stamp that says nothing about the original request. + expect(attempts).toHaveLength(2) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('does not replay a dropped transport error that was never marked delivery-unknown', async () => { + const attempts: Attempt[] = [] + const connection = connectionController() + const client = scriptedClient( + // Unmarked: the frame is known NOT to have reached the wire, so the host never saw + // it and a replay would be a second create, not a reconciliation. The transport + // still leaves 'connected', so only the delivery-unknown check stops this. + [{ throws: new Error('Socket closed before send'), dropsConnection: true }], + attempts, + connection + ) + await expect( + createWorktreeWithNameRetry({ + client, + baseName: 'urchin', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-unmarked' + }) + ).rejects.toThrow('Socket closed before send') + expect(attempts).toHaveLength(1) + }) + + it('keeps the replay window strictly inside the host dedupe TTL', () => { + // The window is measured from a lower bound on when the host could have resolved, so + // it has to leave the record room for the replay to still be in flight. Widening it + // to the whole TTL would let the last replay land on a record that just expired. + expect(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS).toBeGreaterThan(0) + expect(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS).toBeLessThan(WORKTREE_CREATE_DEDUPE_TTL_MS) + // A single wait must not be able to consume the window on its own. + expect(WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS).toBeLessThan( + WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS + ) + }) }) diff --git a/mobile/src/tasks/worktree-create-retry.ts b/mobile/src/tasks/worktree-create-retry.ts index 646c14f69ff..9f7be3bb94f 100644 --- a/mobile/src/tasks/worktree-create-retry.ts +++ b/mobile/src/tasks/worktree-create-retry.ts @@ -1,11 +1,14 @@ import type { RpcClient } from '../transport/rpc-client' import type { RpcResponse, RpcSuccess } from '../transport/types' +import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import { waitForRpcClientReconnected } from '../transport/rpc-client-reconnect-wait' import { isLogicalClientCutoverError } from '../transport/logical-client-cutover-error' import { CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS, getClientWorktreeCreateCandidate, getGeneratedWorktreeCreateRetryCandidate, - isRetryableWorktreeCreateConflict + isRetryableWorktreeCreateConflict, + WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../../src/shared/new-workspace/worktree-create-retry-policy' import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout' @@ -24,6 +27,29 @@ export type WorktreeCreateResult = { worktreeId: string; name: string } | { erro // worktree silently created. const WORKTREE_CREATE_CUTOVER_MAX_RETRIES = 5 +// Why: a connection migration is not the only way a create goes delivery-ambiguous. +// A plain socket close (cellular flap, relay drop, the phone backgrounding and the +// supervisor suspending a billed relay splice) rejects the in-flight frame as +// delivery-unknown with no generation bump, and create holds the longest budget of +// any mobile RPC — 10 minutes of clone/fetch/setup — so it is the likeliest request +// to be caught by one. Surfacing that as a failure is wrong: the host may well have +// finished the worktree. Replay on the same clientMutationId instead. +const WORKTREE_CREATE_AMBIGUOUS_MAX_RETRIES = 2 + +// Why: the host keeps a settled create's dedupe record for WORKTREE_CREATE_DEDUPE_TTL_MS +// after it resolves; a replay that lands later misses it and the host's suffix loop +// builds a SECOND worktree — and for folder workspaces a second one with the SAME name +// and no collision check at all. So a replay is bounded by how stale our knowledge of the +// host is, and that has to be measured in WALL CLOCK: iOS/Android suspend JS timers while +// the app is backgrounded, so any ceiling derived from timer intervals — a watchdog probe +// budget, a request timeout — can be arbitrarily wrong across a background cycle, which is +// exactly when a create sits ambiguous for minutes. +const WORKTREE_CREATE_REPLAY_FLIGHT_MARGIN_MS = 10_000 +export const WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS = + WORKTREE_CREATE_DEDUPE_TTL_MS - WORKTREE_CREATE_REPLAY_FLIGHT_MARGIN_MS +// Bounded so the Create spinner doesn't sit for the whole window; the deadline still caps it. +export const WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS = 20_000 + export type CreateWorktreeWithNameRetryArgs = { client: RpcClient baseName: string @@ -78,32 +104,87 @@ export async function createWorktreeWithNameRetry( return { error: lastError ?? 'Failed to create workspace' } } -// Sends worktree.create, re-issuing on a connection-migration cutover only. The -// shared clientMutationId in `params` keeps the retry idempotent host-side. +// Sends worktree.create, re-issuing whenever the request went delivery-ambiguous — +// the frame reached the wire but no response came back, so the host may already have +// built the worktree. The shared clientMutationId in `params` keeps the retry +// idempotent host-side. A definite failure (never sent, or a server error response) +// is returned to the caller untouched. async function sendWorktreeCreateResilient( client: RpcClient, params: Record, supportsIdempotentCutoverRetry: boolean ): Promise { - for (let migrationRetry = 0; ; migrationRetry += 1) { + let migrationRetry = 0 + let ambiguousRetry = 0 + const firstSentAt = Date.now() + let replayDeadlineAt: number | null = null + for (;;) { try { return await client.sendRequest('worktree.create', params, { timeoutMs: WORKTREE_CREATE_TIMEOUT_MS }) } catch (error) { + if (!supportsIdempotentCutoverRetry) { + throw error + } + if (isLogicalClientCutoverError(error)) { + if (migrationRetry >= WORKTREE_CREATE_CUTOVER_MAX_RETRIES) { + throw error + } + migrationRetry += 1 + // Why: LogicalClientCutoverError is raised only after migrateTo installs an + // authenticated replacement, so retry immediately instead of adding UI lag. + continue + } + if (!isRpcDeliveryUnknown(error) || ambiguousRetry >= WORKTREE_CREATE_AMBIGUOUS_MAX_RETRIES) { + throw error + } + // Why: every transport path that reports a *drop* leaves 'connected' before the + // rejection reaches us (rpc-client.ts:675/695/1213 set state first or reject via + // queueMicrotask; the relay's fail() publishes synchronously). So still being + // 'connected' here means the socket was healthy the whole time and only the + // response went missing — the request-timeout path, which surfaces after + // WORKTREE_CREATE_TIMEOUT_MS. That says nothing about when the host actually + // resolved, so the dedupe record may be long gone and a replay would build a + // second worktree instead of reconciling. Fail the create instead. + if (client.getState() === 'connected') { + throw error + } + // Computed once: a later ambiguity reads a fresher lastInboundAt from the + // replacement session, which would push the deadline past the record it respects. + replayDeadlineAt ??= resolveReplayDeadline(client, firstSentAt) + const remainingWindowMs = replayDeadlineAt - Date.now() + if (remainingWindowMs <= 0) { + throw error + } + ambiguousRetry += 1 + // Why: unlike a cutover, no replacement session exists yet — resending now + // would just hit the dead one, so wait for the transport to come back and + // surface the original ambiguity if it does not. Clamped to the window so the + // wait itself cannot carry the replay past the host's record. if ( - !supportsIdempotentCutoverRetry || - !isLogicalClientCutoverError(error) || - migrationRetry >= WORKTREE_CREATE_CUTOVER_MAX_RETRIES + !(await waitForRpcClientReconnected( + client, + Math.min(WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS, remainingWindowMs) + )) ) { throw error } - // Why: LogicalClientCutoverError is raised only after migrateTo installs an - // authenticated replacement, so retry immediately instead of adding UI lag. } } } +// Latest instant the host's dedupe record is still guaranteed to exist, from the earliest +// point the create could have resolved. lastInboundAt stamps a frame that really arrived, +// so it stays honest across a suspension in a way a timer budget cannot; the send is the +// fallback, and a sound floor either way, because the host cannot resolve a create it has +// not yet received. +function resolveReplayDeadline(client: RpcClient, firstSentAt: number): number { + const lastInboundAt = client.getLastInboundAt?.() ?? null + const anchor = lastInboundAt !== null && lastInboundAt > firstSentAt ? lastInboundAt : firstSentAt + return anchor + WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS +} + function defaultWorktreeCreateMutationId(): string { const randomPart = Math.random().toString(36).slice(2, 10) return `worktree-create:${Date.now().toString(36)}:${randomPart}` diff --git a/mobile/src/transport/mobile-relay-rpc-session.ts b/mobile/src/transport/mobile-relay-rpc-session.ts index f182b7381ed..24a66f6d804 100644 --- a/mobile/src/transport/mobile-relay-rpc-session.ts +++ b/mobile/src/transport/mobile-relay-rpc-session.ts @@ -114,6 +114,7 @@ export function connectMobileRelayRpcSession(args: { getState: () => state, getReconnectAttempt: () => 0, getLastConnectedAt: () => lastConnectedAt, + getLastInboundAt: () => livenessWatchdog.getLastInboundAt() || null, onStateChange(listener) { stateListeners.add(listener) return () => stateListeners.delete(listener) diff --git a/mobile/src/transport/rpc-client-contract.ts b/mobile/src/transport/rpc-client-contract.ts index 9461743950d..6104174cce7 100644 --- a/mobile/src/transport/rpc-client-contract.ts +++ b/mobile/src/transport/rpc-client-contract.ts @@ -36,6 +36,10 @@ export type RpcClient = { getState: () => ConnectionState getReconnectAttempt: () => number getLastConnectedAt: () => number | null + // Wall-clock stamp of the last inbound frame on the current session, or null when the + // transport can't vouch for one. Optional so older/foreign RpcClient shapes stay valid; + // callers must treat absent/null as "unknown" and fall back to a safe bound. + getLastInboundAt?: () => number | null onStateChange: (listener: (state: ConnectionState) => void) => () => void notifyForeground: (reason?: ForegroundNudgeReason) => void restartAfterStructuredBackground?: () => void diff --git a/mobile/src/transport/rpc-client-reconnect-wait.test.ts b/mobile/src/transport/rpc-client-reconnect-wait.test.ts new file mode 100644 index 00000000000..db87817f821 --- /dev/null +++ b/mobile/src/transport/rpc-client-reconnect-wait.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from './rpc-client' +import type { ConnectionState } from './types' +import { waitForRpcClientReconnected } from './rpc-client-reconnect-wait' + +// notifyDuringSubscribe models a client that fires the listener synchronously from +// onStateChange — the branch that has to survive `unsubscribe` not being assigned yet. +function fakeClient( + initial: ConnectionState, + options: { notifyDuringSubscribe?: ConnectionState } = {} +): { client: RpcClient; set: (next: ConnectionState) => void; listenerCount: () => number } { + let state = initial + const listeners = new Set<(next: ConnectionState) => void>() + const client = { + getState: () => state, + onStateChange: (listener: (next: ConnectionState) => void) => { + listeners.add(listener) + if (options.notifyDuringSubscribe) { + state = options.notifyDuringSubscribe + listener(state) + } + return () => listeners.delete(listener) + } + } as unknown as RpcClient + return { + client, + set: (next) => { + state = next + // Safe to delete during iteration: finish() unsubscribes the current listener. + for (const listener of listeners) { + listener(next) + } + }, + listenerCount: () => listeners.size + } +} + +describe('waitForRpcClientReconnected', () => { + it('resolves immediately when the transport is already connected', async () => { + const { client, listenerCount } = fakeClient('connected') + await expect(waitForRpcClientReconnected(client, 10_000)).resolves.toBe(true) + // No subscription at all on the fast path, so nothing to leak. + expect(listenerCount()).toBe(0) + }) + + it('resolves once the transport comes back, and unsubscribes', async () => { + const { client, set, listenerCount } = fakeClient('reconnecting') + const pending = waitForRpcClientReconnected(client, 10_000) + expect(listenerCount()).toBe(1) + set('connected') + await expect(pending).resolves.toBe(true) + expect(listenerCount()).toBe(0) + }) + + it('gives up at the timeout and unsubscribes', async () => { + vi.useFakeTimers() + try { + const { client, listenerCount } = fakeClient('reconnecting') + const pending = waitForRpcClientReconnected(client, 10_000) + await vi.advanceTimersByTimeAsync(10_000) + await expect(pending).resolves.toBe(false) + expect(listenerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('does not wait out the timeout when the pairing is already revoked', async () => { + const { client, listenerCount } = fakeClient('auth-failed') + await expect(waitForRpcClientReconnected(client, 10_000)).resolves.toBe(false) + expect(listenerCount()).toBe(0) + }) + + it('does not wait out the timeout when the pairing is revoked mid-wait', async () => { + vi.useFakeTimers() + try { + const { client, set, listenerCount } = fakeClient('reconnecting') + const pending = waitForRpcClientReconnected(client, 10_000) + set('auth-failed') + await expect(pending).resolves.toBe(false) + expect(listenerCount()).toBe(0) + // Nothing left armed that could fire after the caller moved on. + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) + + it('tears down cleanly when the state change fires synchronously during subscribe', async () => { + vi.useFakeTimers() + try { + const { client, listenerCount } = fakeClient('reconnecting', { + notifyDuringSubscribe: 'connected' + }) + await expect(waitForRpcClientReconnected(client, 10_000)).resolves.toBe(true) + expect(listenerCount()).toBe(0) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + // Generous: advanceTimersByTimeAsync yields through REAL macrotasks between ticks, + // so vitest's default 5s real-time budget is reachable on a loaded runner. + }, 30_000) +}) diff --git a/mobile/src/transport/rpc-client-reconnect-wait.ts b/mobile/src/transport/rpc-client-reconnect-wait.ts new file mode 100644 index 00000000000..72702eae4e0 --- /dev/null +++ b/mobile/src/transport/rpc-client-reconnect-wait.ts @@ -0,0 +1,55 @@ +import type { RpcClient } from './rpc-client' + +// Why: after a socket drop the direct client parks a send in waitForConnected, but a +// relay session rejects it outright ('relay session not connected') until the supervisor +// migrates a replacement in. A replay that fires the instant the drop lands therefore +// burns its budget on the dead session, so callers that mean to re-issue an idempotent +// request wait for the transport to come back first. +// +// Unlike waitForAuthenticated, intermediate 'disconnected'/'reconnecting' states are the +// expected path here, not a failure. Only a revoked pairing and the timeout end the wait +// unsuccessfully. +export function waitForRpcClientReconnected( + client: RpcClient, + timeoutMs: number +): Promise { + const state = client.getState() + if (state === 'connected') { + return Promise.resolve(true) + } + if (state === 'auth-failed') { + return Promise.resolve(false) + } + return new Promise((resolve) => { + let settled = false + let unsubscribe: (() => void) | null = null + // Armed before subscribing so a synchronous notification during registration + // still finds a timer to clear. + const timer = setTimeout(() => finish(false), timeoutMs) + unsubscribe = client.onStateChange((state) => { + if (state === 'connected') { + finish(true) + } else if (state === 'auth-failed') { + // Revoked pairing never reaches 'connected', so waiting out the timeout only + // delays the error the caller is going to surface anyway. + finish(false) + } + }) + if (settled) { + // The notification fired inside onStateChange, before we held the handle. + unsubscribe() + unsubscribe = null + } + + function finish(reconnected: boolean): void { + if (settled) { + return + } + settled = true + clearTimeout(timer) + unsubscribe?.() + unsubscribe = null + resolve(reconnected) + } + }) +} diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index be8efd6da24..c0a7ab83848 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -1128,6 +1128,10 @@ export function connect( return lastConnectedAt }, + getLastInboundAt(): number | null { + return livenessWatchdog.getLastInboundAt() || null + }, + onStateChange(listener: (state: ConnectionState) => void): () => void { stateListeners.add(listener) return () => stateListeners.delete(listener) diff --git a/mobile/src/transport/rpc-session-liveness-watchdog.ts b/mobile/src/transport/rpc-session-liveness-watchdog.ts index 2563f799819..e819dbb6edf 100644 --- a/mobile/src/transport/rpc-session-liveness-watchdog.ts +++ b/mobile/src/transport/rpc-session-liveness-watchdog.ts @@ -52,6 +52,13 @@ export class RpcSessionLivenessWatchdog { this.armIdle(identity) } + // Wall-clock stamp of the last frame that actually arrived; 0 before the first + // start(). Unlike a timer deadline this survives a JS suspension, so callers can + // tell how stale their knowledge of the peer really is. + getLastInboundAt(): number { + return this.lastInboundAt + } + noteAuthenticatedInbound(identity: RpcSessionIdentity): void { if (this.identity !== identity) { return diff --git a/mobile/src/transport/stable-logical-rpc-client.ts b/mobile/src/transport/stable-logical-rpc-client.ts index 48eb4d43d58..dc14e170405 100644 --- a/mobile/src/transport/stable-logical-rpc-client.ts +++ b/mobile/src/transport/stable-logical-rpc-client.ts @@ -147,6 +147,7 @@ export function createStableLogicalRpcClient( getState: () => state, getReconnectAttempt: () => connectionPath.reconnectAttempt(activeSession.getReconnectAttempt()), getLastConnectedAt: () => activeSession.getLastConnectedAt(), + getLastInboundAt: () => activeSession.getLastInboundAt?.() ?? null, onStateChange(listener) { stateListeners.add(listener) return () => stateListeners.delete(listener) diff --git a/src/cli/args.ts b/src/cli/args.ts index 68ae1cae913..c4c373a814f 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -206,6 +206,7 @@ export function isCommandGroup(commandPath: string[]): boolean { 'artifacts', 'automations', 'project', + 'host', 'repo', 'worktree', 'terminal', diff --git a/src/cli/execution-host-flag.test.ts b/src/cli/execution-host-flag.test.ts index a51b69f5d38..d0239b22d43 100644 --- a/src/cli/execution-host-flag.test.ts +++ b/src/cli/execution-host-flag.test.ts @@ -20,13 +20,19 @@ vi.mock('./runtime-client', () => ({ })) import { + assertEnvironmentSelectorResolvable, hostFilterMatchesHostId, parseHostFlag, resolveHostFlagEnvironmentId } from './execution-host-flag' import { parseExecutionHostId } from '../shared/execution-host' -const NO_SELECTION = { pairingCode: null, environmentSelector: null } +const listSshTargetsMock = vi.fn(async () => [] as { id: string; label: string }[]) +const NO_SELECTION = { + listSshTargets: listSshTargetsMock, + pairingCode: null, + environmentSelector: null +} function flags(entries: Record): Map { return new Map(Object.entries(entries)) @@ -84,15 +90,17 @@ describe('resolveHostFlagEnvironmentId', () => { await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-missing' }), NO_SELECTION) - ).rejects.toThrow('no paired environment has id env-missing') + ).rejects.toThrow('no paired Orca server is named or has id env-missing') }) - it('matches by environment id only, never by environment name', async () => { + // Why: the name is what a person or agent actually knows; requiring the raw uuid made the + // obvious spelling fail against a server that is sitting right there. + it('accepts the environment name as well as its id', async () => { listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')]) await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:gpu' }), NO_SELECTION) - ).rejects.toThrow('no paired environment has id gpu') + ).resolves.toBe('env-1') }) it('decodes percent-encoded environment ids', async () => { @@ -108,6 +116,7 @@ describe('resolveHostFlagEnvironmentId', () => { await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-1' }), { + listSshTargets: listSshTargetsMock, pairingCode: 'orca://pair?x', environmentSelector: null }) @@ -120,6 +129,7 @@ describe('resolveHostFlagEnvironmentId', () => { await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-1' }), { + listSshTargets: listSshTargetsMock, pairingCode: null, environmentSelector: { value: 'other', label: '--environment' } }) @@ -132,6 +142,7 @@ describe('resolveHostFlagEnvironmentId', () => { await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-1' }), { + listSshTargets: listSshTargetsMock, pairingCode: null, environmentSelector: { value: 'staging', label: 'ORCA_ENVIRONMENT' } }) @@ -142,7 +153,7 @@ describe('resolveHostFlagEnvironmentId', () => { listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')]) await expect( - resolveHostFlagEnvironmentId(flags({ host: 'runtime:gpu' }), NO_SELECTION) + resolveHostFlagEnvironmentId(flags({ host: 'runtime:missing' }), NO_SELECTION) ).rejects.toMatchObject({ code: 'invalid_argument', data: { knownEnvironments: [{ id: 'env-1', name: 'gpu' }] } @@ -155,6 +166,7 @@ describe('resolveHostFlagEnvironmentId', () => { await expect( resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-1' }), { + listSshTargets: listSshTargetsMock, pairingCode: null, environmentSelector: { value: 'gpu', label: '--environment' } }) @@ -187,3 +199,78 @@ describe('hostFilterMatchesHostId', () => { expect(hostFilterMatchesHostId(runtimeHost, null)).toBe(false) }) }) + +describe('ambiguous environment names', () => { + it('refuses --host runtime: when two servers share that name', async () => { + listEnvironmentsMock.mockReturnValue([ + environment('env-1', 'awin'), + environment('env-2', 'awin') + ]) + + await expect( + resolveHostFlagEnvironmentId(flags({ host: 'runtime:awin' }), NO_SELECTION) + ).rejects.toThrow('2 paired servers are named awin') + }) + + it('refuses --environment when two servers share that name', async () => { + listEnvironmentsMock.mockReturnValue([ + environment('env-1', 'awin'), + environment('env-2', 'awin') + ]) + + await expect(assertEnvironmentSelectorResolvable('awin', listSshTargetsMock)).rejects.toThrow( + '2 paired servers are named awin' + ) + }) + + it('still routes an exact id past a colliding name', async () => { + listEnvironmentsMock.mockReturnValue([ + environment('env-1', 'awin'), + environment('env-2', 'awin') + ]) + + await expect( + resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-2' }), NO_SELECTION) + ).resolves.toBe('env-2') + }) +}) + +describe('assertEnvironmentSelectorResolvable', () => { + it('accepts a paired server by name or id', async () => { + listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')]) + listSshTargetsMock.mockClear() + + await expect( + assertEnvironmentSelectorResolvable('awin', listSshTargetsMock) + ).resolves.toBeUndefined() + await expect( + assertEnvironmentSelectorResolvable('env-1', listSshTargetsMock) + ).resolves.toBeUndefined() + expect(listSshTargetsMock).not.toHaveBeenCalled() + }) + + // Why: the inverse of the --host case, and the one the report actually hit — a name that is + // an SSH target dead-ended with a bare "unknown environment". + it('names the SSH target when the selector is one', async () => { + listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')]) + listSshTargetsMock.mockResolvedValue([{ id: 'ssh-1-a', label: 'openclaw' }]) + + await expect( + assertEnvironmentSelectorResolvable('openclaw', listSshTargetsMock) + ).rejects.toMatchObject({ + code: 'invalid_argument', + data: { + nextSteps: expect.arrayContaining([expect.stringContaining('--host ssh:ssh-1-a')]) + } + }) + }) + + it('still points at host list when the name is on neither axis', async () => { + listEnvironmentsMock.mockReturnValue([]) + listSshTargetsMock.mockResolvedValue([]) + + await expect( + assertEnvironmentSelectorResolvable('nowhere', listSshTargetsMock) + ).rejects.toThrow('no paired Orca server is named or has id nowhere') + }) +}) diff --git a/src/cli/execution-host-flag.ts b/src/cli/execution-host-flag.ts index 36dc080e329..8b30807b816 100644 --- a/src/cli/execution-host-flag.ts +++ b/src/cli/execution-host-flag.ts @@ -2,11 +2,23 @@ import { LOCAL_EXECUTION_HOST_ID, normalizeExecutionHostId, parseExecutionHostId, + toSshExecutionHostId, type ParsedExecutionHost } from '../shared/execution-host' +import { + ambiguousEnvironments, + crossKindNextSteps, + findEnvironmentByName, + resolveSshHostTargetId, + type SshTargetSummary +} from './host-selector-alternatives' +import type { RuntimeClient } from './runtime-client' import { RuntimeClientError } from './runtime/types' export type HostFlagRoutingSelection = { + // Why: SSH targets live in the running Orca host, not on disk, so enumerating them needs a + // client. Injected as a thunk so the lookup only happens on the error path we are explaining. + listSshTargets: () => Promise pairingCode: string | null // Why: an ambient ORCA_ENVIRONMENT counts as a selection too, so carry the label to // name the real source in the conflict message. @@ -51,19 +63,29 @@ export async function resolveHostFlagEnvironmentId( ]) const userDataPath = getDefaultUserDataPath() const known = listEnvironments(userDataPath) - const environment = known.find((candidate) => candidate.id === host.environmentId) + // Why: --environment has always taken a name or an id, and the name is what people and agents + // actually know. Requiring the raw uuid here made the obvious spelling fail; accept either and + // canonicalize to the id so stored host ids still compare correctly downstream. + const environment = findEnvironmentByName( + known.map((candidate) => ({ id: candidate.id, name: candidate.name })), + host.environmentId + ) if (!environment) { // Why: `runtime:` is a host id that also appears in stored rows, so it resolves by id // only — unlike --environment, which also accepts a name. Say so, and hand back the ids an // agent can retry with instead of making it scrape the sentence. + const environments = known.map((candidate) => ({ id: candidate.id, name: candidate.name })) + assertEnvironmentNameUnambiguous(environments, host.environmentId, `--host ${host.id}`) + const sshTargets = await selection.listSshTargets() throw new RuntimeClientError( 'invalid_argument', - `Unknown Orca server in --host ${host.id}: no paired environment has id ${host.environmentId}. --host runtime: matches environment ids only, never names. Run \`orca environment list\` to see paired servers, or pass --host local.`, + `Unknown Orca server in --host ${host.id}: no paired Orca server is named or has id ${host.environmentId}.`, { - knownEnvironments: known.map((candidate) => ({ id: candidate.id, name: candidate.name })), + knownEnvironments: environments, + knownSshTargets: sshTargets, nextSteps: [ + ...crossKindNextSteps(host.environmentId, { environments, sshTargets }, 'environment'), 'Run `orca environment list` to see paired Orca servers.', - 'Use the environment id, not its name: --host runtime:.', 'Use --host local to target this machine.' ] } @@ -100,3 +122,86 @@ export function hostFilterMatchesHostId( } return filter.kind === 'runtime' && candidate === LOCAL_EXECUTION_HOST_ID } + +// Why: `ssh:` reaches a machine the connected runtime owns, so it can only be checked against +// that runtime — unlike `runtime:`, which is resolved from this machine's pairing store before a +// client exists. Callers that act on a --host value run this so an unknown target fails loudly +// instead of quietly filtering to nothing. +export async function resolveHostFlagTarget( + flags: Map, + client: RuntimeClient +): Promise { + const host = parseHostFlag(flags) + if (host?.kind !== 'ssh') { + return host + } + const [{ listEnvironments }, { getDefaultUserDataPath }] = await Promise.all([ + import('./runtime/environments.js'), + import('./runtime-client.js') + ]) + const environments = listEnvironments(getDefaultUserDataPath()).map((candidate) => ({ + id: candidate.id, + name: candidate.name + })) + const targetId = await resolveSshHostTargetId(client, host.targetId, environments) + return parseExecutionHostId(toSshExecutionHostId(targetId)) ?? host +} + +// Why: the store refuses an ambiguous environment name rather than guessing which server was +// meant. Resolving one here would put the guess back, in the flag whose whole purpose is to stop +// a command reaching a machine the caller did not choose. +function assertEnvironmentNameUnambiguous( + environments: readonly { id: string; name: string }[], + name: string, + flag: string +): void { + const ambiguous = ambiguousEnvironments(environments, name) + if (ambiguous.length === 0) { + return + } + throw new RuntimeClientError( + 'invalid_argument', + `Ambiguous Orca server in ${flag}: ${ambiguous.length} paired servers are named ${name}. Use the environment id.`, + { + knownEnvironments: ambiguous, + nextSteps: ambiguous.map( + (candidate) => `Use --host runtime:${candidate.id} for the server named ${candidate.name}.` + ) + } + ) +} + +// Why: the inverse of the --host case. `--environment openclaw` failed with a bare "Unknown +// environment", when openclaw is very often an SSH target — a different axis, not a typo. The +// store's own error cannot carry the hint (translateStoreError forwards code and message only, +// dropping data), so resolve the selector here where the payload survives. +export async function assertEnvironmentSelectorResolvable( + selector: string, + listSshTargetsForSuggestion: () => Promise +): Promise { + const [{ listEnvironments }, { getDefaultUserDataPath }] = await Promise.all([ + import('./runtime/environments.js'), + import('./runtime-client.js') + ]) + const environments = listEnvironments(getDefaultUserDataPath()).map((candidate) => ({ + id: candidate.id, + name: candidate.name + })) + if (findEnvironmentByName(environments, selector)) { + return + } + assertEnvironmentNameUnambiguous(environments, selector, `--environment ${selector}`) + const sshTargets = await listSshTargetsForSuggestion() + throw new RuntimeClientError( + 'invalid_argument', + `Unknown Orca server in --environment ${selector}: no paired Orca server is named or has id ${selector}.`, + { + knownEnvironments: environments, + knownSshTargets: sshTargets, + nextSteps: [ + ...crossKindNextSteps(selector, { environments, sshTargets }, 'environment'), + 'Run `orca host list` to see every machine you can target and the flag for each.' + ] + } + ) +} diff --git a/src/cli/format.ts b/src/cli/format.ts index ebe30e68305..2b6b67f05e9 100644 --- a/src/cli/format.ts +++ b/src/cli/format.ts @@ -175,6 +175,26 @@ function localCliErrorData(error: unknown, context: CliErrorContext): unknown { return undefined } +export type HostListEntry = { + kind: 'local' | 'ssh' | 'environment' + name: string + id: string + selector: string +} + +// Why: the selector column is the point of this command — the name alone is what callers already +// had, and passing it on the wrong axis is the mistake this output exists to prevent. +export function formatHostList(result: { hosts: HostListEntry[] }): string { + const kindLabel: Record = { + local: 'local', + ssh: 'ssh target', + environment: 'orca server' + } + return result.hosts + .map((host) => `${kindLabel[host.kind].padEnd(11)} ${host.name} -> ${host.selector}`) + .join('\n') +} + export function formatCliStatus(status: CliStatusResult): string { return [ ...(status.target && status.target.kind === 'environment' diff --git a/src/cli/handler-group-manifest.ts b/src/cli/handler-group-manifest.ts index 7a5a24fbe99..0907905e5e3 100644 --- a/src/cli/handler-group-manifest.ts +++ b/src/cli/handler-group-manifest.ts @@ -194,7 +194,13 @@ export const HANDLER_GROUPS: readonly HandlerGroup[] = [ }, { name: 'environment', - keys: ['environment add', 'environment list', 'environment show', 'environment rm'], + keys: [ + 'host list', + 'environment add', + 'environment list', + 'environment show', + 'environment rm' + ], load: async () => (await import('./handlers/environment.js')).ENVIRONMENT_HANDLERS }, { diff --git a/src/cli/handlers/environment.ts b/src/cli/handlers/environment.ts index 7827f91347d..2a433021769 100644 --- a/src/cli/handlers/environment.ts +++ b/src/cli/handlers/environment.ts @@ -1,5 +1,6 @@ import type { CommandHandler } from '../dispatch' -import { formatEnvironment, formatEnvironmentList, printResult } from '../format' +import { formatEnvironment, formatEnvironmentList, formatHostList, printResult } from '../format' +import { listSshTargets } from '../host-selector-alternatives' import { getDefaultUserDataPath, RuntimeClientError } from '../runtime-client' import type { RuntimeRpcSuccess } from '../runtime-client' import { redactRuntimeEnvironment } from '../../shared/runtime-environments' @@ -29,6 +30,29 @@ export const ENVIRONMENT_HANDLERS: Record = { `Saved environment ${result.environment.name} (${result.environment.id}).` ) }, + // Why: an agent told "run it on " had nowhere to look. `orca environment list` showed + // paired servers only, and nothing in the CLI listed SSH targets at all, so the wrong-axis + // guess was the only move available. This is the one place that answers both. + 'host list': async ({ client, json }) => { + const environments = listEnvironments(getDefaultUserDataPath()).map((environment) => ({ + kind: 'environment' as const, + name: environment.name, + id: environment.id, + selector: `--environment ${environment.name}` + })) + const sshTargets = (await listSshTargets(client)).map((target) => ({ + kind: 'ssh' as const, + name: target.label, + id: target.id, + selector: `--host ssh:${target.id}` + })) + const hosts = [ + { kind: 'local' as const, name: 'this machine', id: 'local', selector: '--host local' }, + ...sshTargets, + ...environments + ] + printResult(localSuccess({ hosts }), json, formatHostList) + }, 'environment list': async ({ json }) => { const environments = listEnvironments(getDefaultUserDataPath()).map(redactRuntimeEnvironment) printResult(localSuccess({ environments }), json, formatEnvironmentList) diff --git a/src/cli/handlers/project.ts b/src/cli/handlers/project.ts index 83c13a93448..2b0a8d8ada2 100644 --- a/src/cli/handlers/project.ts +++ b/src/cli/handlers/project.ts @@ -22,7 +22,11 @@ import { formatProjectList, printResult } from '../format' -import { hostFilterMatchesHostId, parseHostFlag } from '../execution-host-flag' +import { + hostFilterMatchesHostId, + parseHostFlag, + resolveHostFlagTarget +} from '../execution-host-flag' import { getOptionalStringFlag, getRequiredStringFlag } from '../flags' import { resolveRepoPathArgument } from '../repo-path-arguments' import { RuntimeClientError, type RuntimeRpcSuccess } from '../runtime-client' @@ -53,6 +57,20 @@ async function callProjectHostSetup( } } +async function getResolvedHostId( + flags: Map, + client: HandlerContext['client'] +): Promise { + const host = await resolveHostFlagTarget(flags, client) + if (!host) { + throw new RuntimeClientError('invalid_argument', 'Missing required --host') + } + return host.id +} + +// Why: the setup paths keep the unresolved id on purpose. The runtime rejects every `ssh:` host +// for those operations regardless of whether it exists, so resolving first would answer "no such +// target" and imply the command would have worked with the right id. function getRequiredHostId(flags: Map): ExecutionHostId { const host = parseHostFlag(flags) if (!host) { @@ -79,7 +97,7 @@ export const PROJECT_HANDLERS: Record = { }, 'project setups': async ({ flags, client, json }) => { const projectFilter = getOptionalStringFlag(flags, 'project') - const hostFilter = parseHostFlag(flags) + const hostFilter = await resolveHostFlagTarget(flags, client) const result = await callProjectHostSetup<{ setups: ProjectHostSetup[] }>( client, 'projectHostSetup.list' @@ -132,7 +150,12 @@ export const PROJECT_HANDLERS: Record = { const path = getOptionalStringFlag(flags, 'path') const args: ProjectHostSetupCreateArgs = { projectId: getRequiredStringFlag(flags, 'project'), - hostId: getRequiredHostId(flags), + // Why: unlike the setup paths below, the runtime does not reject `ssh:` here — this records + // independent metadata — so an unknown target would persist a row pointing at a machine + // that does not exist. Resolving catches that. `local` and `runtime:` pass through + // untouched, because this is also the provisioning path and a runtime host legitimately + // may not exist yet when its metadata is written. + hostId: await getResolvedHostId(flags, client), setupId: getOptionalStringFlag(flags, 'setup-id'), path: path === undefined diff --git a/src/cli/help.ts b/src/cli/help.ts index 31a0eded615..afabbc7e155 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -30,6 +30,9 @@ Skills: skills install Install bundled Orca skills globally via the community skills CLI skills update Update already-installed Orca skills via the community skills CLI +Hosts: + host list List targetable machines and how to name each one + Environments: environment add Save a remote Orca runtime from a pairing code environment list List saved remote Orca runtimes @@ -225,6 +228,7 @@ Common Commands: orca agent-context [--json] orca account add [--agent claude|codex] [--json] orca account list [--json] + orca host list [--json] orca environment add --name --pairing-code [--json] orca environment list [--json] orca environment show --environment [--json] diff --git a/src/cli/host-selector-alternatives.test.ts b/src/cli/host-selector-alternatives.test.ts new file mode 100644 index 00000000000..bc4fadd05c1 --- /dev/null +++ b/src/cli/host-selector-alternatives.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it, vi } from 'vitest' +import { + ambiguousEnvironments, + ambiguousSshTargets, + crossKindNextSteps, + findEnvironmentByName, + findSshTargetByName, + listSshTargets, + resolveSshHostTargetId +} from './host-selector-alternatives' +import type { RuntimeClient } from './runtime-client' + +const SSH_TARGETS = [{ id: 'ssh-1755000000000-a1b2c3', label: 'openclaw' }] +const ENVIRONMENTS = [{ id: '03ef704c-b180-4b10-998d-e28fbd5de9a3', name: 'awin' }] + +function clientReturning(targets: { id: string; label: string }[]): RuntimeClient { + return { + call: vi.fn(async () => ({ result: { targets } })) + } as unknown as RuntimeClient +} + +describe('findSshTargetByName', () => { + // Why: ids are machine-generated `ssh--`, so the label is the only name a + // person or agent ever has. Matching ids alone reports "not found" for a target that exists. + it('matches the human label as well as the generated id', () => { + expect(findSshTargetByName(SSH_TARGETS, 'openclaw')?.id).toBe('ssh-1755000000000-a1b2c3') + expect(findSshTargetByName(SSH_TARGETS, 'ssh-1755000000000-a1b2c3')?.label).toBe('openclaw') + expect(findSshTargetByName(SSH_TARGETS, 'OpenClaw')?.label).toBe('openclaw') + }) + + it('does not invent a match', () => { + expect(findSshTargetByName(SSH_TARGETS, 'awin')).toBeUndefined() + }) +}) + +describe('findEnvironmentByName', () => { + it('matches a paired server by name or id', () => { + expect(findEnvironmentByName(ENVIRONMENTS, 'awin')?.id).toBe(ENVIRONMENTS[0]!.id) + expect(findEnvironmentByName(ENVIRONMENTS, ENVIRONMENTS[0]!.id)?.name).toBe('awin') + }) +}) + +describe('crossKindNextSteps', () => { + const alternatives = { sshTargets: SSH_TARGETS, environments: ENVIRONMENTS } + + it('points an SSH name at the SSH flag when a server was asked for', () => { + const steps = crossKindNextSteps('openclaw', alternatives, 'environment') + + expect(steps.join('\n')).toContain('--host ssh:ssh-1755000000000-a1b2c3') + expect(steps.join('\n')).toContain('is an SSH target') + }) + + it('points a server name at --environment when an SSH target was asked for', () => { + const steps = crossKindNextSteps('awin', alternatives, 'ssh') + + expect(steps.join('\n')).toContain('--environment awin') + expect(steps.join('\n')).toContain('is a paired Orca server') + }) + + it('says nothing when the name exists on neither axis', () => { + expect(crossKindNextSteps('nowhere', alternatives, 'ssh')).toEqual([]) + }) + + // Why: suggesting the axis the caller already used would be noise, not help. + it('does not suggest the axis that was already requested', () => { + expect(crossKindNextSteps('openclaw', alternatives, 'ssh')).toEqual([]) + expect(crossKindNextSteps('awin', alternatives, 'environment')).toEqual([]) + }) +}) + +describe('resolveSshHostTargetId', () => { + it('resolves a label to the target id', async () => { + await expect( + resolveSshHostTargetId(clientReturning(SSH_TARGETS), 'openclaw', ENVIRONMENTS) + ).resolves.toBe('ssh-1755000000000-a1b2c3') + }) + + // Why: this used to answer ok:true with an empty list — the same silent wrong-machine result + // that unknown runtime ids gave before they were rejected. + it('rejects an unknown target instead of letting it filter to nothing', async () => { + await expect( + resolveSshHostTargetId(clientReturning(SSH_TARGETS), 'nowhere', ENVIRONMENTS) + ).rejects.toThrow('no SSH target named or with id nowhere') + }) + + it('names the paired server when an SSH target was asked for by a server name', async () => { + await expect( + resolveSshHostTargetId(clientReturning(SSH_TARGETS), 'awin', ENVIRONMENTS) + ).rejects.toMatchObject({ + data: { + nextSteps: expect.arrayContaining([expect.stringContaining('--environment awin')]) + } + }) + }) + + it('says so plainly when the host has no SSH targets at all', async () => { + await expect( + resolveSshHostTargetId(clientReturning([]), 'openclaw', ENVIRONMENTS) + ).rejects.toMatchObject({ + data: { nextSteps: expect.arrayContaining(['This Orca host has no SSH targets registered.']) } + }) + }) +}) + +describe('listSshTargets', () => { + // Why: an older host answers listTargets but not listTargetSummaries. Treating that as "no + // targets" would reject an ssh id that is valid on that host. + it('falls back to the older listing when the newer method is absent', async () => { + const { RuntimeClientError } = await import('./runtime/types.js') + const call = vi.fn(async (method: string) => { + if (method === 'ssh.listTargetSummaries') { + throw new RuntimeClientError('method_not_found', 'Unknown method') + } + return { result: { targets: SSH_TARGETS } } + }) + + await expect(listSshTargets({ call } as unknown as RuntimeClient)).resolves.toEqual(SSH_TARGETS) + expect(call).toHaveBeenCalledWith('ssh.listTargets') + }) + + // Why: this only ever runs to enrich an error we are already reporting; a failure here must + // not replace that error with a confusing one about SSH enumeration. + it('returns nothing rather than masking the error it was enriching', async () => { + const client = { + call: vi.fn(async () => { + throw new Error('boom') + }) + } + + await expect(listSshTargets(client as unknown as RuntimeClient)).resolves.toEqual([]) + }) +}) + +describe('ambiguous names never resolve silently', () => { + const twoOpenclaw = [ + { id: 'ssh-1-a', label: 'openclaw' }, + { id: 'ssh-2-b', label: 'openclaw' } + ] + const twoAwin = [ + { id: 'env-1', name: 'awin' }, + { id: 'env-2', name: 'awin' } + ] + + // Why: picking the first would choose a machine on the caller's behalf — the exact failure the + // whole selector path exists to prevent. + it('refuses to guess between two ssh targets sharing a label', () => { + expect(findSshTargetByName(twoOpenclaw, 'openclaw')).toBeUndefined() + expect(ambiguousSshTargets(twoOpenclaw, 'openclaw')).toHaveLength(2) + }) + + it('refuses to guess between two servers sharing a name', () => { + expect(findEnvironmentByName(twoAwin, 'awin')).toBeUndefined() + expect(ambiguousEnvironments(twoAwin, 'awin')).toHaveLength(2) + }) + + // An exact id is never ambiguous, even when labels collide. + it('still resolves an exact id past a colliding label', () => { + expect(findSshTargetByName(twoOpenclaw, 'ssh-2-b')?.id).toBe('ssh-2-b') + expect(findEnvironmentByName(twoAwin, 'env-2')?.id).toBe('env-2') + }) + + it('reports no ambiguity for a unique name', () => { + expect(ambiguousSshTargets(SSH_TARGETS, 'openclaw')).toEqual([]) + expect(ambiguousEnvironments(ENVIRONMENTS, 'awin')).toEqual([]) + }) + + it('names both candidates when an ssh label is ambiguous', async () => { + await expect( + resolveSshHostTargetId(clientReturning(twoOpenclaw), 'openclaw', ENVIRONMENTS) + ).rejects.toMatchObject({ + data: { + nextSteps: expect.arrayContaining([ + expect.stringContaining('ssh-1-a'), + expect.stringContaining('ssh-2-b') + ]) + } + }) + }) +}) diff --git a/src/cli/host-selector-alternatives.ts b/src/cli/host-selector-alternatives.ts new file mode 100644 index 00000000000..fd5abecbc18 --- /dev/null +++ b/src/cli/host-selector-alternatives.ts @@ -0,0 +1,159 @@ +import type { RuntimeClient } from './runtime-client' + +export type SshTargetSummary = { id: string; label: string } +export type EnvironmentSummary = { id: string; name: string } + +export type HostAlternatives = { + sshTargets: readonly SshTargetSummary[] + environments: readonly EnvironmentSummary[] +} + +// Why: SSH target ids are machine-generated (`ssh--`), so the name a person +// or an agent actually knows is the label. Looking a name up as an id alone reports "not found" +// for a target that is sitting right there. +export function findSshTargetByName( + targets: readonly SshTargetSummary[], + name: string +): SshTargetSummary | undefined { + const byId = targets.find((target) => target.id === name) + if (byId) { + return byId + } + const byLabel = matchesByLabel(targets, name) + // Why: two targets can share a label. Picking the first would silently choose a machine for + // the caller — the failure this whole selector path exists to prevent — so an ambiguous name + // resolves to nothing and the caller is told to use an id. + return byLabel.length === 1 ? byLabel[0] : undefined +} + +function matchesByLabel(targets: readonly SshTargetSummary[], name: string): SshTargetSummary[] { + const wanted = name.trim().toLowerCase() + return targets.filter((target) => target.label.trim().toLowerCase() === wanted) +} + +export function ambiguousSshTargets( + targets: readonly SshTargetSummary[], + name: string +): SshTargetSummary[] { + const byLabel = matchesByLabel(targets, name) + return byLabel.length > 1 ? byLabel : [] +} + +export function findEnvironmentByName( + environments: readonly EnvironmentSummary[], + name: string +): EnvironmentSummary | undefined { + const byId = environments.find((environment) => environment.id === name) + if (byId) { + return byId + } + const byName = matchesByEnvironmentName(environments, name) + // Why: the environment store itself refuses an ambiguous name rather than guessing; resolving + // one here would quietly reintroduce the guess it exists to prevent. + return byName.length === 1 ? byName[0] : undefined +} + +function matchesByEnvironmentName( + environments: readonly EnvironmentSummary[], + name: string +): EnvironmentSummary[] { + const wanted = name.trim().toLowerCase() + return environments.filter((environment) => environment.name.trim().toLowerCase() === wanted) +} + +export function ambiguousEnvironments( + environments: readonly EnvironmentSummary[], + name: string +): EnvironmentSummary[] { + const byName = matchesByEnvironmentName(environments, name) + return byName.length > 1 ? byName : [] +} + +// Why: a paired Orca server and an SSH target are different machines reached different ways, but +// a caller only knows "the machine called X". When X misses on one axis, the useful answer is +// almost always that it exists on the other — so say which, and give the exact flag. +export function crossKindNextSteps( + name: string, + alternatives: HostAlternatives, + requested: 'environment' | 'ssh' +): string[] { + const steps: string[] = [] + const ssh = findSshTargetByName(alternatives.sshTargets, name) + const environment = findEnvironmentByName(alternatives.environments, name) + if (requested !== 'ssh' && ssh) { + steps.push( + `"${name}" is an SSH target on this Orca host, not a paired server. Use --host ssh:${ssh.id}.` + ) + } + if (requested !== 'environment' && environment) { + steps.push( + `"${name}" is a paired Orca server, not an SSH target. Use --environment ${environment.name}.` + ) + } + return steps +} + +// Why: only display identity crosses this boundary — the RPC deliberately withholds addresses +// and credentials — and an enumeration failure must never mask the error we are explaining. +export async function listSshTargets(client: RuntimeClient): Promise { + try { + const result = await client.call<{ targets: SshTargetSummary[] }>('ssh.listTargetSummaries') + return result.result.targets + } catch (error) { + // Why: hosts predating listTargetSummaries still answer listTargets, and both are served by + // the same summariser. Without this an old host looks like one with no SSH targets at all, + // which would reject a target id that is actually valid there. + if (error instanceof Error && 'code' in error && error.code === 'method_not_found') { + try { + const legacy = await client.call<{ targets: SshTargetSummary[] }>('ssh.listTargets') + return legacy.result.targets + } catch { + return [] + } + } + return [] + } +} + +// Why: `--host ssh:` was never validated, so an unknown target answered ok:true with an +// empty list — the same silent wrong-machine answer that `runtime:` ids used to give. And since +// target ids are machine-generated (`ssh--`), the label a caller actually +// knows never matches one, so this fires on the common spelling rather than a rare typo. +// Resolving the label and naming the alternative is what makes the failure recoverable. +export async function resolveSshHostTargetId( + client: RuntimeClient, + targetId: string, + environments: readonly EnvironmentSummary[] +): Promise { + const targets = await listSshTargets(client) + const matched = findSshTargetByName(targets, targetId) + if (matched) { + return matched.id + } + const { RuntimeClientError } = await import('./runtime/types.js') + const ambiguous = ambiguousSshTargets(targets, targetId) + if (ambiguous.length > 0) { + throw new RuntimeClientError( + 'invalid_argument', + `Ambiguous SSH target in --host ssh:${targetId}: ${ambiguous.length} targets share that label. Use the target id.`, + { + knownSshTargets: ambiguous, + nextSteps: ambiguous.map((target) => `Use --host ssh:${target.id} for ${target.label}.`) + } + ) + } + throw new RuntimeClientError( + 'invalid_argument', + `Unknown SSH target in --host ssh:${targetId}: this Orca host has no SSH target named or with id ${targetId}.`, + { + knownSshTargets: targets, + knownEnvironments: environments, + nextSteps: [ + ...crossKindNextSteps(targetId, { sshTargets: targets, environments }, 'ssh'), + ...(targets.length > 0 + ? [`Known SSH targets: ${targets.map((target) => target.label).join(', ')}.`] + : ['This Orca host has no SSH targets registered.']) + ] + } + ) +} diff --git a/src/cli/index-project-setup.test.ts b/src/cli/index-project-setup.test.ts index f5efead6a80..068c246ba7c 100644 --- a/src/cli/index-project-setup.test.ts +++ b/src/cli/index-project-setup.test.ts @@ -220,9 +220,10 @@ describe('orca cli worktree awareness', () => { await main(['project', 'setups', '--host', 'runtime:not-a-real-env', '--json'], '/tmp/repo') - expect(callMock).not.toHaveBeenCalled() + // The command itself never reached a runtime; only the suggestion lookup did. + expect(callMock).not.toHaveBeenCalledWith('projectHostSetup.list') const printed = [...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n') - expect(printed).toContain('no paired environment has id not-a-real-env') + expect(printed).toContain('no paired Orca server is named or has id not-a-real-env') // An agent reads the code and the retry candidates, not the prose. expect(JSON.parse(printed).error.code).toBe('invalid_argument') expect(JSON.parse(printed).error.data.knownEnvironments).toEqual([]) @@ -231,6 +232,124 @@ describe('orca cli worktree awareness', () => { process.exitCode = priorExitCode }) + // Why: ssh: was never validated, so an unknown target answered ok:true with an empty list — + // the same silent wrong-machine answer unknown runtime ids used to give. + it('rejects an unknown ssh host instead of answering empty', async () => { + queueFixtures(callMock, okFixture('req_ssh_targets', { targets: [] })) + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const priorExitCode = process.exitCode + + await main(['project', 'setups', '--host', 'ssh:openclaw', '--json'], '/tmp/repo') + + expect(callMock).not.toHaveBeenCalledWith('projectHostSetup.list') + expect([...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n')).toContain( + 'no SSH target named or with id openclaw' + ) + expect(process.exitCode).toBe(1) + + process.exitCode = priorExitCode + }) + + it('tells a caller reaching for a paired server by ssh that it is an environment', async () => { + pairRuntimeEnvironment(listEnvironmentsMock, 'awin') + queueFixtures(callMock, okFixture('req_ssh_targets', { targets: [] })) + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const priorExitCode = process.exitCode + + await main(['project', 'setups', '--host', 'ssh:awin', '--json'], '/tmp/repo') + + expect([...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n')).toContain( + '--environment awin' + ) + expect(process.exitCode).toBe(1) + + process.exitCode = priorExitCode + }) + + it('resolves an ssh label to its target id before filtering', async () => { + queueFixtures( + callMock, + okFixture('req_ssh_targets', { targets: [{ id: 'ssh-123-abc', label: 'openclaw' }] }), + okFixture('req_project_setups', { + setups: [ + { + id: 'setup-openclaw', + projectId: 'github:stablyai/orca', + hostId: 'ssh:ssh-123-abc', + repoId: 'repo-openclaw', + path: '/home/me/orca', + displayName: 'Orca', + setupState: 'ready', + setupMethod: 'legacy-repo', + createdAt: 1, + updatedAt: 1 + } + ] + }) + ) + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main(['project', 'setups', '--host', 'ssh:openclaw'], '/tmp/repo') + + expect(logSpy.mock.calls[0]?.[0]).toContain('setup-openclaw') + }) + + // Why: `runtime:` is a persisted token — it lands in ProjectHostSetup.hostId and is + // embedded in generated setup ids. Accepting a name is only safe because it is canonicalized + // to the id before anything downstream sees it; this pins that. + it('never lets an environment name reach a persisted host id', async () => { + pairRuntimeEnvironment(listEnvironmentsMock, 'env-uuid-1', 'awin') + queueFixtures( + callMock, + okFixture('req_project_setup_create', { + result: { + project: { + id: 'github:stablyai/orca', + displayName: 'Orca', + badgeColor: '#7c3aed', + sourceRepoIds: [], + createdAt: 1, + updatedAt: 1 + }, + setup: { + id: 'setup-awin', + projectId: 'github:stablyai/orca', + hostId: 'local', + repoId: '', + path: '', + displayName: 'awin', + setupState: 'setting-up', + setupMethod: 'provisioned', + createdAt: 1, + updatedAt: 2 + } + } + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + [ + 'project', + 'setup-create', + '--project', + 'github:stablyai/orca', + '--host', + 'runtime:awin', + '--json' + ], + '/tmp/repo' + ) + + expect(runtimeClientConstructorMock).toHaveBeenCalledWith(null, 'env-uuid-1') + expect(callMock).toHaveBeenCalledWith( + 'projectHostSetup.create', + expect.objectContaining({ hostId: 'runtime:env-uuid-1' }) + ) + }) + it('rejects a malformed --host value before contacting any runtime', async () => { const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) diff --git a/src/cli/index.ts b/src/cli/index.ts index c6585e79247..26cfb8eea49 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -9,7 +9,11 @@ import { validateCommandAndFlags } from './args' import { dispatch } from './dispatch' -import { resolveHostFlagEnvironmentId } from './execution-host-flag' +import { + assertEnvironmentSelectorResolvable, + resolveHostFlagEnvironmentId +} from './execution-host-flag' +import { listSshTargets } from './host-selector-alternatives' import { reportCliError } from './format' import { printHelp } from './help' import type { RuntimeClient } from './runtime-client' @@ -90,6 +94,15 @@ export async function main( const ignoreRemoteSelection = shouldIgnoreRemoteSelection(parsed.commandPath) const pairingCode = ignoreRemoteSelection ? null : parsed.flags.get('pairing-code') const environmentSelector = ignoreRemoteSelection ? null : parsed.flags.get('environment') + // Why: only the explicit flag is asserted eagerly. An ambient ORCA_ENVIRONMENT is background + // config, and failing local-only commands because of a stale one would be a regression; the + // explicit flag means the caller named that machine, so a bad name should fail immediately + // with the cross-kind hint rather than a bare store error at first use. + const listSshTargetsForSuggestion = async (): Promise<{ id: string; label: string }[]> => + listSshTargets(new RuntimeClientClass(undefined, undefined, null, null)) + if (typeof environmentSelector === 'string') { + await assertEnvironmentSelectorResolvable(environmentSelector, listSshTargetsForSuggestion) + } // Why: --host runtime: names a paired server, not a filter over this // runtime's rows, so it has to pick the connection before the client exists. // An ambient ORCA_ENVIRONMENT is checked for disagreement too — silently @@ -99,6 +112,9 @@ export async function main( const hostEnvironmentId = ignoreRemoteSelection ? null : await resolveHostFlagEnvironmentId(parsed.flags, { + // Why: only consulted when the name missed, and against this machine's own runtime — + // SSH targets are registered there, not in the paired server we failed to find. + listSshTargets: listSshTargetsForSuggestion, pairingCode: typeof pairingCode === 'string' ? pairingCode : null, environmentSelector: typeof environmentSelector === 'string' @@ -107,6 +123,12 @@ export async function main( ? { value: process.env.ORCA_ENVIRONMENT, label: 'ORCA_ENVIRONMENT' } : null }) + // Why: --host runtime: is canonicalized to the environment's id so downstream host-id + // comparisons against stored rows still match; rewrite the flag once, here, rather than + // resolving the name again at every consumer. + if (hostEnvironmentId !== null) { + parsed.flags.set('host', `runtime:${hostEnvironmentId}`) + } // Why: pass `null` (not `undefined`) when remote selection is suppressed // so the RuntimeClient default parameter does not re-activate the // ORCA_PAIRING_CODE / ORCA_ENVIRONMENT env-var fallback for commands diff --git a/src/cli/runtime-client-deferral.test.ts b/src/cli/runtime-client-deferral.test.ts index d5a1f421213..9345283431c 100644 --- a/src/cli/runtime-client-deferral.test.ts +++ b/src/cli/runtime-client-deferral.test.ts @@ -11,6 +11,16 @@ const { constructorArgsMock, callMock, getCliStatusMock } = vi.hoisted(() => ({ // Why: `main` reaches RuntimeClient through `await import('./runtime-client.js')` // now. Mocking the same specifier the eager import used proves the dynamic // import still resolves to the module the 10 existing vi.mock suites target. +// Why: --environment is now resolved against the paired-environment store before the client is +// constructed, so a forwarding assertion needs an environment that actually resolves. +vi.mock('./runtime/environments', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + listEnvironments: () => [{ id: 'env-1', name: 'env-1' }] + } +}) + vi.mock('./runtime-client', () => { class RuntimeClient { call = callMock @@ -21,7 +31,7 @@ vi.mock('./runtime-client', () => { constructorArgsMock(...args) } } - return { RuntimeClient } + return { RuntimeClient, getDefaultUserDataPath: () => '/tmp/orca-user-data' } }) import { main } from './index' diff --git a/src/cli/specs/environment.ts b/src/cli/specs/environment.ts index 7c4aecbc31a..d6ceb795028 100644 --- a/src/cli/specs/environment.ts +++ b/src/cli/specs/environment.ts @@ -2,6 +2,18 @@ import type { CommandSpec } from '../args' import { GLOBAL_FLAGS } from '../args' export const ENVIRONMENT_COMMAND_SPECS: CommandSpec[] = [ + { + path: ['host', 'list'], + summary: 'List every machine this Orca host can target, and how to name each one', + usage: 'orca host list [--json]', + allowedFlags: [...GLOBAL_FLAGS], + notes: [ + 'Answers "what can I target and what do I pass" in one place: this machine, the SSH targets registered on it, and the Orca servers paired with it.', + 'The three kinds are reached differently. A paired Orca server is a connection, selected with --environment . An SSH target is a machine the connected Orca host reaches, selected with --host ssh:. Passing one where the other belongs is the most common way to get an empty or missing-host answer.', + "SSH targets are read from the Orca host you are currently connected to, so this lists that host's targets and not another server's." + ], + examples: ['orca host list', 'orca host list --json'] + }, { path: ['environment', 'add'], summary: 'Save a remote Orca runtime environment from a pairing code', diff --git a/src/cli/worktree-project-target.ts b/src/cli/worktree-project-target.ts index 66d94aa0b1a..6dfd169fccc 100644 --- a/src/cli/worktree-project-target.ts +++ b/src/cli/worktree-project-target.ts @@ -1,6 +1,6 @@ import { normalizeExecutionHostId, type ParsedExecutionHost } from '../shared/execution-host' import type { ProjectHostSetup } from '../shared/project-types' -import { hostFilterMatchesHostId, parseHostFlag } from './execution-host-flag' +import { hostFilterMatchesHostId, resolveHostFlagTarget } from './execution-host-flag' import type { RuntimeClient } from './runtime-client' import { RuntimeClientError } from './runtime-client' @@ -73,7 +73,7 @@ export async function resolveProjectCreateTarget( ): Promise { const projectHostSetupId = getPresentStringFlag(flags, 'project-host-setup') const projectId = getPresentStringFlag(flags, 'project') - const host = parseHostFlag(flags) + const host = await resolveHostFlagTarget(flags, client) if (!projectHostSetupId && !projectId && !host) { return undefined } diff --git a/src/main/agent-hooks/windows-hook-post-interpreter.test.ts b/src/main/agent-hooks/windows-hook-post-interpreter.test.ts new file mode 100644 index 00000000000..0e79c125c2e --- /dev/null +++ b/src/main/agent-hooks/windows-hook-post-interpreter.test.ts @@ -0,0 +1,149 @@ +// Why (#15117): an agent holding a private copy of the shared post command missed the move to +// curl for three months, invisible to per-agent tests. Assert the invariant across every agent +// at once: a managed Windows .cmd hook posts through curl.exe and spawns no interpreter. +// Generated under a mocked win32 platform, not executed, so the POSIX CI legs guard it too. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type * as osModule from 'node:os' + +let isolatedUserDataDir = '' +let previousUserDataPath: string | undefined + +const { homedirMock } = vi.hoisted(() => ({ + homedirMock: vi.fn<() => string>() +})) + +vi.mock('electron', () => ({ + app: { + getPath: () => '/tmp/orca-user-data' + } +})) + +vi.mock('os', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + homedir: homedirMock.mockImplementation(actual.homedir) + } +}) + +import { AntigravityHookService } from '../antigravity/hook-service' +import { ClaudeHookService } from '../claude/hook-service' +import { CodexHookService } from '../codex/hook-service' +import { CommandCodeHookService } from '../command-code/hook-service' +import { CursorHookService } from '../cursor/hook-service' +import { DevinHookService } from '../devin/hook-service' +import { DroidHookService } from '../droid/hook-service' +import { GeminiHookService } from '../gemini/hook-service' +import { GrokHookService } from '../grok/hook-service' +import { openClaudeHookService } from '../openclaude/hook-service' + +// Why: only agents whose managed Windows script is a .cmd batch file. Copilot's hook is a +// `.ps1` — PowerShell is its interpreter, not a child process it spawns per event — and Kimi's +// is a Git Bash `.sh`, so neither is subject to this invariant. +const BATCH_SCRIPT_INSTALLERS = [ + { agent: 'antigravity', install: () => new AntigravityHookService().install() }, + { agent: 'claude', install: () => new ClaudeHookService().install() }, + { agent: 'openclaude', install: () => openClaudeHookService.install() }, + { agent: 'codex', install: () => new CodexHookService().install() }, + { agent: 'command-code', install: () => new CommandCodeHookService().install() }, + { agent: 'cursor', install: () => new CursorHookService().install() }, + { agent: 'devin', install: () => new DevinHookService().install() }, + { agent: 'droid', install: () => new DroidHookService().install() }, + { agent: 'gemini', install: () => new GeminiHookService().install() }, + { agent: 'grok', install: () => new GrokHookService().install() } +] as const + +function withPlatform(platform: NodeJS.Platform, run: () => T): T { + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return run() + } finally { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + } +} + +describe('Windows managed hook post interpreter', () => { + let home = '' + + beforeEach(() => { + previousUserDataPath = process.env.ORCA_USER_DATA_PATH + isolatedUserDataDir = mkdtempSync(join(tmpdir(), 'orca-hook-interpreter-user-data-')) + // Why: Orca-managed Codex hooks resolve through ORCA_USER_DATA_PATH before the mocked + // home; an inherited live path would let this test rewrite the developer's own hooks. + process.env.ORCA_USER_DATA_PATH = isolatedUserDataDir + home = mkdtempSync(join(tmpdir(), 'orca-hook-interpreter-')) + homedirMock.mockReturnValue(home) + }) + + afterEach(() => { + if (previousUserDataPath === undefined) { + delete process.env.ORCA_USER_DATA_PATH + } else { + process.env.ORCA_USER_DATA_PATH = previousUserDataPath + } + rmSync(isolatedUserDataDir, { recursive: true, force: true }) + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + rmSync(home, { recursive: true, force: true }) + home = '' + }) + + it('posts through curl.exe from every managed batch script, spawning no interpreter', () => { + const scripts = withPlatform('win32', () => { + for (const entry of BATCH_SCRIPT_INSTALLERS) { + expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + } + const hooksDir = join(home, '.orca', 'agent-hooks') + return readdirSync(hooksDir) + .filter((name) => name.endsWith('.cmd')) + .map((name) => ({ name, body: readFileSync(join(hooksDir, name), 'utf8') })) + }) + + // Why: a silent zero would make this suite pass while asserting nothing. + expect(scripts.length).toBeGreaterThanOrEqual(BATCH_SCRIPT_INSTALLERS.length) + + // Why: assert this across every .cmd before narrowing. Filtering to posting scripts first + // hides the failure that matters — a script that swapped its curl POST for an interpreter + // no longer matches the filter, so the suite fails on an opaque count instead. + for (const script of scripts) { + // Why: PowerShell costs ~300ms of interpreter startup per event and recodes the UTF-8 + // payload through the console code page. curl.exe (Win10 1803+) does neither. + expect(script.body, `${script.name} must not spawn an interpreter`).not.toMatch( + /powershell(\.exe)?/i + ) + } + + // Why: `%~dp0` marks an event wrapper that only sets env and delegates to the core script. + const isWrapper = (body: string): boolean => body.includes('%~dp0') + const posts = (body: string): boolean => body.includes('127.0.0.1:%ORCA_AGENT_HOOK_PORT%') + + // Why: name the script that stopped posting rather than failing on a bare count. + expect( + scripts.filter((s) => !isWrapper(s.body) && !posts(s.body)).map((s) => s.name), + 'every non-wrapper script must post to the hook port' + ).toEqual([]) + + const posting = scripts.filter((script) => posts(script.body)) + expect(posting.length).toBeGreaterThanOrEqual(BATCH_SCRIPT_INSTALLERS.length) + + for (const script of posting) { + // Why: fully qualified so a repo-local curl.exe cannot intercept hook payloads. + expect(script.body, `${script.name} must post through curl.exe`).toContain( + '"%SystemRoot%\\System32\\curl.exe"' + ) + } + + // Why: hook events pipe via stdin to keep multi-KB tool output off the command line (EDR + // oversized-command-line rules). The statusline script stages a temp file instead. + for (const script of posting.filter((s) => s.body.includes('/hook/'))) { + expect(script.body, `${script.name} must pipe the payload via stdin`).toContain( + '--data-urlencode "payload@-"' + ) + } + }) +}) diff --git a/src/main/antigravity/hook-script.ts b/src/main/antigravity/hook-script.ts index 724bffbcd86..0645416c005 100644 --- a/src/main/antigravity/hook-script.ts +++ b/src/main/antigravity/hook-script.ts @@ -4,13 +4,23 @@ import { buildWindowsHookStdinDrainEpilogue, WINDOWS_HOOK_STDIN_DRAIN_COMMAND } from '../agent-hooks/hook-stdin-contract' +import { buildWindowsAgentHookPostCommand } from '../agent-hooks/installer-utils' import { ANTIGRAVITY_PRE_TOOL_USE_DECISION } from './hook-events' +// Why (#15117): PowerShell cost ~300ms of startup per event, which is what made the console +// the agent allocates for each hook last long enough to see. +const WINDOWS_ANTIGRAVITY_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('antigravity', [ + // Why: Antigravity alone takes its event name from the wrapper's env, not the piped payload. + ' --data-urlencode "hook_event_name=%ORCA_ANTIGRAVITY_EVENT%" ^' +]) + export function getManagedScript(target: 'local' | 'posix' = 'local'): string { if (target === 'local' && process.platform === 'win32') { return [ '@echo off', - 'setlocal', + // Why (#9358/#9941): inherited delayed expansion eats `!` out of the percent-expanded + // curl args, mangling paneKey and dropping worktreeId. `!` is legal in a Windows path. + 'setlocal DisableDelayedExpansion', 'if /I "%ORCA_ANTIGRAVITY_EVENT%"=="Stop" (', ' echo {"decision":""}', ') else if /I "%ORCA_ANTIGRAVITY_EVENT%"=="PreToolUse" (', @@ -20,7 +30,7 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { ')', 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', ...buildWindowsHookEnvironmentGuardLines(), - buildWindowsAntigravityHookPostCommand(), + WINDOWS_ANTIGRAVITY_HOOK_POST_COMMAND, 'exit /b 0', ...buildWindowsHookStdinDrainEpilogue(), '' @@ -96,10 +106,3 @@ export function getWindowsWrapperScript(eventName: string): string { '' ].join('\r\n') } - -function buildWindowsAntigravityHookPostCommand(): string { - // Why: Antigravity hooks are best-effort status updates; do not let a stalled - // local listener hold the agent process open. Qualify PowerShell so a - // worktree-local powershell.exe cannot hijack hook payloads. - return `"%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" -NoProfile -ExecutionPolicy Bypass -Command "$utf8=[System.Text.UTF8Encoding]::new($false); [Console]::InputEncoding=$utf8; [Console]::OutputEncoding=$utf8; $inputData=[Console]::In.ReadToEnd(); try { $payload=if ([string]::IsNullOrWhiteSpace($inputData)) { @{} } else { $inputData | ConvertFrom-Json }; $body=@{ paneKey=$env:ORCA_PANE_KEY; launchToken=$env:ORCA_AGENT_LAUNCH_TOKEN; tabId=$env:ORCA_TAB_ID; worktreeId=$env:ORCA_WORKTREE_ID; env=$env:ORCA_AGENT_HOOK_ENV; version=$env:ORCA_AGENT_HOOK_VERSION; hook_event_name=$env:ORCA_ANTIGRAVITY_EVENT; payload=$payload } | ConvertTo-Json -Depth 100 -Compress; $bodyBytes=$utf8.GetBytes($body); Invoke-WebRequest -UseBasicParsing -Method Post -Uri ('http://127.0.0.1:' + $env:ORCA_AGENT_HOOK_PORT + '/hook/antigravity') -ContentType 'application/json; charset=utf-8' -Headers @{ 'X-Orca-Agent-Hook-Token'=$env:ORCA_AGENT_HOOK_TOKEN } -Body $bodyBytes -TimeoutSec 2 | Out-Null } catch {}"` -} diff --git a/src/main/antigravity/hook-service.test.ts b/src/main/antigravity/hook-service.test.ts index e758f348a4b..b9f69b03cc1 100644 --- a/src/main/antigravity/hook-service.test.ts +++ b/src/main/antigravity/hook-service.test.ts @@ -98,10 +98,12 @@ describe('AntigravityHookService', () => { ) expect(script).toContain('/hook/antigravity') if (process.platform === 'win32') { - expect(script).toContain('%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe') - expect(script).toContain('hook_event_name=$env:ORCA_ANTIGRAVITY_EVENT') - expect(script).toContain('[string]::IsNullOrWhiteSpace($inputData)) { @{} }') - expect(script).not.toContain('[string]::IsNullOrWhiteSpace($inputData)) { exit 0 }') + expect(script).not.toContain('powershell.exe') + expect(script).toContain('%SystemRoot%\\System32\\curl.exe') + expect(script).toContain('hook_event_name=%ORCA_ANTIGRAVITY_EVENT%') + expect(script).toContain('--data-urlencode "payload@-"') + // Why (#9358/#9941): delayed expansion eats `!` out of percent-expanded curl args. + expect(script).toContain('setlocal DisableDelayedExpansion') } else { expect(script).toContain('hook_event_name=${ORCA_ANTIGRAVITY_EVENT}') expect(script).toContain(`payload=$(${POSIX_HOOK_STDIN_READER})`) @@ -275,10 +277,10 @@ describe('AntigravityHookService', () => { 'utf8' ) expect(script).toContain('/hook/antigravity') - expect(script).toContain('%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe') - expect(script).toContain('hook_event_name=$env:ORCA_ANTIGRAVITY_EVENT') - expect(script).toContain('[string]::IsNullOrWhiteSpace($inputData)) { @{} }') - expect(script).not.toContain('[string]::IsNullOrWhiteSpace($inputData)) { exit 0 }') + expect(script).not.toContain('powershell.exe') + expect(script).toContain('%SystemRoot%\\System32\\curl.exe') + expect(script).toContain('hook_event_name=%ORCA_ANTIGRAVITY_EVENT%') + expect(script).toContain('setlocal DisableDelayedExpansion') }) }) diff --git a/src/main/antigravity/windows-hook-payload-delivery.test.ts b/src/main/antigravity/windows-hook-payload-delivery.test.ts new file mode 100644 index 00000000000..8261cc3ce91 --- /dev/null +++ b/src/main/antigravity/windows-hook-payload-delivery.test.ts @@ -0,0 +1,287 @@ +// Why (#15117): shape assertions cannot catch a curl line that posts nothing, so this suite +// pipes a real payload through the installed wrappers and follows it to a live listener. +import { afterEach, describe, expect, it, vi } from 'vitest' +import { spawn } from 'node:child_process' +import { createServer, type Server } from 'node:http' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type * as osModule from 'node:os' + +const { homedirMock } = vi.hoisted(() => ({ + homedirMock: vi.fn<() => string>() +})) + +vi.mock('electron', () => ({ + app: { + getPath: () => '/tmp/orca-user-data' + } +})) + +vi.mock('os', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + homedir: homedirMock.mockImplementation(actual.homedir) + } +}) + +import { AntigravityHookService } from './hook-service' +import { ANTIGRAVITY_EVENTS, ANTIGRAVITY_PRE_TOOL_USE_DECISION } from './hook-events' +import { getManagedScript } from './hook-script' + +// Why (#9358/#9941): `!` is legal in a Windows path and in a pane key. Under inherited +// delayed expansion cmd eats it out of a percent-expanded curl argument, so bake one into +// every value the script forwards rather than asserting the `setlocal` line alone. +const PANE_KEY = 'tab-1:leaf-1!bang' +const WORKTREE_ID = 'repo-1::C:\\Users\\alice\\orca\\feature!branch' +const HOOK_TOKEN = 'antigravity-payload-delivery-token' +// Why: exercise the sizes and bytes a real hook carries — a multi-KB body crosses the pipe +// in several chunks, and non-ASCII catches a launcher that recodes stdin through a code page. +const PAYLOAD = JSON.stringify({ + session_id: 'a3f9c1e0-antigravity-delivery', + tool_name: 'Bash', + tool_input: { command: 'echo "café 日本語 😀" & echo %PATH% | more' }, + transcript: 'y'.repeat(4096) +}) + +type HookPost = { + payload: string | null + paneKey: string | null + worktreeId: string | null + hookEventName: string | null + token: string | null + contentType: string | null +} + +async function startHookListener(): Promise<{ + server: Server + port: number + posts: HookPost[] +}> { + const posts: HookPost[] = [] + const server = createServer((req, res) => { + let body = '' + req.setEncoding('utf8') + req.on('data', (chunk) => { + body += chunk + }) + req.on('end', () => { + const form = new URLSearchParams(body) + posts.push({ + payload: form.get('payload'), + paneKey: form.get('paneKey'), + worktreeId: form.get('worktreeId'), + hookEventName: form.get('hook_event_name'), + token: (req.headers['x-orca-agent-hook-token'] as string | undefined) ?? null, + contentType: (req.headers['content-type'] as string | undefined) ?? null + }) + res.writeHead(200, { 'content-type': 'application/json' }) + res.end('{}') + }) + }) + const port = await new Promise((resolve) => { + server.listen(0, '127.0.0.1', () => { + const address = server.address() + resolve(typeof address === 'object' && address ? address.port : 0) + }) + }) + return { server, port, posts } +} + +type HookRun = { exitCode: number | null; stdout: string; stderr: string; timedOut: boolean } + +function runWrapper( + wrapperPath: string, + env: NodeJS.ProcessEnv, + // Why: `null` abandons stdin instead of closing it — the shape a caller outside an Orca + // pane produces, and the only way to prove the env guard exits before reading (#11549). + stdinPayload: string | null = PAYLOAD +): Promise { + return new Promise((resolve, reject) => { + // Why: mirror how Antigravity spawns the hook — `cmd /c `, the exact + // chain in the bug report's process trace. + const child = spawn('cmd.exe', ['/d', '/c', wrapperPath], { + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + env + }) + let stdout = '' + let stderr = '' + let timedOut = false + const timer = setTimeout(() => { + timedOut = true + child.kill('SIGKILL') + }, 15_000) + child.on('error', (error) => { + clearTimeout(timer) + reject(error) + }) + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString() + }) + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString() + }) + child.on('close', (exitCode) => { + clearTimeout(timer) + // Why: curl exits once the POST is written; give the listener a beat to finish reading it. + setTimeout(() => resolve({ exitCode, stdout, stderr, timedOut }), 250) + }) + // Why: an abandoned pipe raises EPIPE once the child exits; swallow it so the run still + // resolves on the child's own terms. + child.stdin.on('error', () => {}) + if (stdinPayload !== null) { + child.stdin.end(Buffer.from(stdinPayload, 'utf8')) + } + }) +} + +function hookEnvironment(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const base = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) + ) + return { ...base, ...extra } +} + +function expectedStdout(eventName: string): string { + if (eventName === 'PreToolUse') { + return ANTIGRAVITY_PRE_TOOL_USE_DECISION + } + return eventName === 'Stop' ? '{"decision":""}' : '{}' +} + +// Why: runs on every platform — the live delivery suite below is Windows-only, so this +// keeps a POSIX-only CI leg from letting the interpreter back into the hot path. +describe('Antigravity Windows hook post command', () => { + it('posts through curl.exe rather than a PowerShell interpreter', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const script = getManagedScript('local') + + expect(script).not.toMatch(/powershell/i) + expect(script).toContain('"%SystemRoot%\\System32\\curl.exe" -sS -X POST') + expect(script).toContain('http://127.0.0.1:%ORCA_AGENT_HOOK_PORT%/hook/antigravity') + expect(script).toContain('--data-urlencode "hook_event_name=%ORCA_ANTIGRAVITY_EVENT%"') + // Why: keep the payload off the command line so multi-KB tool output cannot trip an + // EDR oversized-command-line rule. + expect(script).toContain('--data-urlencode "payload@-"') + expect(script).toContain('setlocal DisableDelayedExpansion') + vi.restoreAllMocks() + }) +}) + +describe.skipIf(process.platform !== 'win32')('Antigravity Windows hook payload delivery', () => { + let home = '' + let server: Server | null = null + + afterEach(() => { + server?.close() + server = null + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + if (home) { + rmSync(home, { recursive: true, force: true }) + home = '' + } + }) + + it('delivers every event wrapper payload to the listener without spawning PowerShell', async () => { + home = mkdtempSync(join(tmpdir(), 'orca-antigravity-hook-')) + homedirMock.mockReturnValue(home) + expect(new AntigravityHookService().install().state).toBe('installed') + + const hooksDir = join(home, '.orca', 'agent-hooks') + const coreScript = readFileSync(join(hooksDir, 'antigravity-hook.cmd'), 'utf8') + expect(coreScript).not.toMatch(/powershell/i) + + const listener = await startHookListener() + server = listener.server + const env = hookEnvironment({ + USERPROFILE: home, + HOME: home, + ORCA_AGENT_HOOK_PORT: String(listener.port), + ORCA_AGENT_HOOK_TOKEN: HOOK_TOKEN, + ORCA_PANE_KEY: PANE_KEY, + ORCA_WORKTREE_ID: WORKTREE_ID + }) + + for (const event of ANTIGRAVITY_EVENTS) { + const label = event.eventName + const before = listener.posts.length + const result = await runWrapper(join(hooksDir, event.windowsWrapperFileName), env) + + expect(result.timedOut, `${label} timed out`).toBe(false) + expect(result.exitCode, `${label} exit code`).toBe(0) + expect(result.stderr, `${label} stderr`).toBe('') + // Why: Antigravity reads silence on PreToolUse as deny (#2426), so the gate answer + // must survive the transport change. + expect(result.stdout.trim(), `${label} stdout`).toBe(expectedStdout(label)) + + const posts = listener.posts.slice(before) + expect(posts, `${label} posted exactly one hook`).toHaveLength(1) + // Why: byte-exact, not "non-empty" — PowerShell recoded this body through the console + // code page, and a silently corrupted payload still looks posted. + expect(posts[0].payload, `${label} payload`).toBe(PAYLOAD) + expect(posts[0].hookEventName, `${label} hook_event_name`).toBe(label) + // Why: the `!` in both values is the delayed-expansion regression guard. + expect(posts[0].paneKey, `${label} paneKey`).toBe(PANE_KEY) + expect(posts[0].worktreeId, `${label} worktreeId`).toBe(WORKTREE_ID) + expect(posts[0].token, `${label} token`).toBe(HOOK_TOKEN) + expect(posts[0].contentType, `${label} content-type`).toContain( + 'application/x-www-form-urlencoded' + ) + } + // Why: five wrapper launches plus a real install can overrun the default under load. + }, 60_000) + + // Why (#15117): Antigravity fires some events with no stdin at all. PowerShell substituted + // `{}` before posting; curl forwards the empty body, so prove the post still happens — the + // listener's matching allowance is covered in agent-hook-listener-antigravity.test.ts. + it('still posts a status event when the agent supplies no payload', async () => { + home = mkdtempSync(join(tmpdir(), 'orca-antigravity-hook-')) + homedirMock.mockReturnValue(home) + expect(new AntigravityHookService().install().state).toBe('installed') + + const listener = await startHookListener() + server = listener.server + const result = await runWrapper( + join(home, '.orca', 'agent-hooks', 'antigravity-pre-invocation.cmd'), + hookEnvironment({ + USERPROFILE: home, + HOME: home, + ORCA_AGENT_HOOK_PORT: String(listener.port), + ORCA_AGENT_HOOK_TOKEN: HOOK_TOKEN, + ORCA_PANE_KEY: PANE_KEY + }), + '' + ) + + expect(result.timedOut).toBe(false) + expect(result.exitCode).toBe(0) + expect(listener.posts).toHaveLength(1) + // Why: curl drops a `--data-urlencode name@-` field entirely when stdin is empty, so the + // event reaches the listener with no `payload` key — not an empty one. + expect(listener.posts[0].payload).toBeNull() + expect(listener.posts[0].hookEventName).toBe('PreInvocation') + }, 30_000) + + it('exits without reading stdin when the pane env is missing', async () => { + home = mkdtempSync(join(tmpdir(), 'orca-antigravity-hook-')) + homedirMock.mockReturnValue(home) + expect(new AntigravityHookService().install().state).toBe('installed') + + const listener = await startHookListener() + server = listener.server + // Why (#11549): outside an Orca pane the caller may abandon stdin rather than close it, + // so the guard must exit before the read — otherwise the console lingers indefinitely. + const result = await runWrapper( + join(home, '.orca', 'agent-hooks', 'antigravity-pre-tool-use.cmd'), + hookEnvironment({ USERPROFILE: home, HOME: home }), + null + ) + + expect(result.timedOut).toBe(false) + expect(result.exitCode).toBe(0) + expect(result.stdout.trim()).toBe(ANTIGRAVITY_PRE_TOOL_USE_DECISION) + expect(listener.posts).toHaveLength(0) + }, 30_000) +}) diff --git a/src/main/persistence-ui-state.test.ts b/src/main/persistence-ui-state.test.ts index d6690712e72..ccafa4e3fab 100644 --- a/src/main/persistence-ui-state.test.ts +++ b/src/main/persistence-ui-state.test.ts @@ -95,6 +95,31 @@ describe('Store', () => { ]) }) + // The RPC now strips manualRepoOrder, so every paired-client ui.set reaches the store without + // the key. Absent has to mean preserve: if it read as clear, the strip would erase the desktop's + // order on the first unrelated setting a phone or web client changes. + // Absent-means-preserve is what makes the pairing-local strip safe: a client's ui.set arrives + // without these fields, so the desktop's own values must survive the update. + it('updateUI preserves the manual repo and host-section order when an update omits them', async () => { + const store = await createStore() + store.updateUI({ + manualRepoOrder: [ + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'bravo' } + ] as never, + workspaceHostOrder: ['ssh:box', 'local'] as never + }) + + store.updateUI({ sidebarWidth: 400 }) + + expect(store.getUI().manualRepoOrder).toEqual([ + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'bravo' } + ]) + expect(store.getUI().workspaceHostOrder).toEqual(['ssh:box', 'local']) + expect(store.getUI().sidebarWidth).toBe(400) + }) + it('updateUI persists sanitized per-worktree dotfile visibility', async () => { const store = await createStore() store.updateUI({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index c0da9e8cf7e..ec923855a56 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1128,6 +1128,7 @@ import { isGeneratedWorktreeCreateName, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' +import { WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../shared/new-workspace/worktree-create-retry-policy' import { failedWorktreeCreationNeedsRetirement, getRetiredNameRegistryForRepo, @@ -2074,8 +2075,9 @@ function getAgentLaunchPlatformForRepo( const MOBILE_TERMINAL_CREATE_RESULT_TTL_MS = 60_000 // Why: same idempotency window for worktree.create — a phone whose create was // interrupted by a connection migration retries with the same clientMutationId -// and reuses the just-created worktree instead of spawning a duplicate. -const WORKTREE_CREATE_RESULT_TTL_MS = 60_000 +// and reuses the just-created worktree instead of spawning a duplicate. Shared with +// the mobile client so its replay budget is sized against the real window. +const WORKTREE_CREATE_RESULT_TTL_MS = WORKTREE_CREATE_DEDUPE_TTL_MS const FOREGROUND_AGENT_WRAPPER_RETRY_INTERVAL_MS = 150 const FOREGROUND_AGENT_WRAPPER_RETRY_TIMEOUT_MS = 6_500 const BRACKETED_PASTE_BEGIN = '\x1b[200~' @@ -17219,7 +17221,10 @@ export class OrcaRuntimeService { // in passive desktop-watch mode. this.setMobileDisplayMode(ptyId, 'auto') if (this.hasRemoteDesktopLayoutState(ptyId)) { - return this.applyRemoteDesktopLayout(ptyId) + // Why: the lock is already released above, so this re-layout is + // best-effort. Reporting its `ok` would tell the desktop "nothing was + // reclaimed" and cost the caller its post-take-back refit and focus. + await this.applyRemoteDesktopLayout(ptyId) } return true } @@ -17235,14 +17240,14 @@ export class OrcaRuntimeService { clearTimeout(softLeaver.timer) this.pendingSoftLeavers.delete(ptyId) } - const priorDriver = this.getDriver(ptyId) + // Why: applyRemoteDesktopLayout no-ops while the driver still reads mobile. this.setDriver(ptyId, { kind: 'idle' }) - const converged = await this.applyRemoteDesktopLayout(ptyId) - if (!converged) { - this.setDriver(ptyId, priorDriver) - return false - } - this.setDriver(ptyId, { kind: 'desktop' }) + // Why: best-effort, like the local held branch below. A host whose resize + // keeps failing (dropped SSH/WSL provider, exited PTY) would otherwise + // roll the lock back and leave the banner stranded, making every retry a + // no-op — the one branch that broke this method's release guarantee. + await this.applyRemoteDesktopLayout(ptyId) + this.releaseDesktopTakeBack(ptyId) this.setMobileDisplayMode(ptyId, 'auto') return true } @@ -18046,8 +18051,10 @@ export class OrcaRuntimeService { // Returns the post-condition "no fit-override remains held" (#7588): `true` // when it cleared a held override OR nothing was held to begin with, `false` // only when a restore was attempted and the resize failed (override rolled - // back, still held). reclaimTerminalForDesktop gates its driver/mode - // transitions on this; other callers ignore it. + // back, still held). Informational for every caller today — + // reclaimTerminalForDesktop deliberately does NOT gate on it, because an + // explicit take-back must drop the lock even when the resize cannot + // converge. Do not reinstate a convergence gate there. async applyMobileDisplayMode(ptyId: string): Promise { const mode = this.getMobileDisplayMode(ptyId) const inner = this.mobileSubscribers.get(ptyId) @@ -36739,7 +36746,10 @@ export class OrcaRuntimeService { private isPtyKnownExited(ptyId: string): boolean { const pty = this.ptysById.get(ptyId) if (pty) { - return !pty.connected + // Why: `!connected` is an inference, not proof. The liveness sweep clears it with no + // exit code for every PTY of a dropped relay, so reading that as an exit retires the + // lease of a process still running on the host — 'unknown' must keep watching. + return getPtyTerminalState(pty) === 'exited' } return this.getLeavesForPty(ptyId).some((leaf) => getTerminalState(leaf) === 'exited') } diff --git a/src/main/runtime/remote-desktop-driver.test.ts b/src/main/runtime/remote-desktop-driver.test.ts index 06eea32df00..bf2370ee924 100644 --- a/src/main/runtime/remote-desktop-driver.test.ts +++ b/src/main/runtime/remote-desktop-driver.test.ts @@ -84,11 +84,16 @@ function createRuntime(mobileAutoRestoreFitMs: number | null = 5_000) { const resizeCalls: { ptyId: string; cols: number; rows: number }[] = [] const driverEvents: { ptyId: string; driver: { kind: string; clientId?: string } }[] = [] const fitOverrideEvents: { ptyId: string; mode: string; cols: number; rows: number }[] = [] + let resizeSucceeds = true runtime.setPtyController({ write: () => true, kill: () => true, getForegroundProcess: async () => null, resize: (ptyId, cols, rows) => { + // Why: the production controller returns false when the provider throws — a dropped SSH/WSL provider or an exited PTY. + if (!resizeSucceeds) { + return false + } resizeCalls.push({ ptyId, cols, rows }) ptySizes.set(ptyId, { cols, rows }) return true @@ -116,7 +121,10 @@ function createRuntime(mobileAutoRestoreFitMs: number | null = 5_000) { runtime, driverEvents, fitOverrideEvents, - resizeCalls + resizeCalls, + setResizeSucceeds: (next: boolean) => { + resizeSucceeds = next + } } } @@ -519,4 +527,66 @@ describe('remote desktop viewer width driver', () => { await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 100, 40) expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) }) + + it('held phone-fit take-back releases the lock when the remote reclaim resize fails', async () => { + // Why: the local held branch releases unconditionally (mobile-presence-lock.test.ts scenario 4). + // The remote branch rolled the lock back instead, so the banner survived and every retry was a no-op. + const { runtime, fitOverrideEvents, setResizeSucceeds } = createRuntime(null) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + runtime.onClientDisconnected('phone-A') + await vi.advanceTimersByTimeAsync(10_000) + expect(runtime.getTerminalFitOverride('pty-1')).toMatchObject({ mode: 'mobile-fit' }) + + setResizeSucceeds(false) + const notifierBefore = fitOverrideEvents.length + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) + expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) + // A second click must not be needed, and must stay idempotent. + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(false) + }) + + it('driving take-back reports success when the trailing remote layout cannot converge', async () => { + // Why: the lock is already released here, so returning false told the desktop renderer + // "nothing was reclaimed" and it skipped the post-take-back refit and focus. + const { runtime, setResizeSucceeds } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + + setResizeSucceeds(false) + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + }) + + it('take-back during the resubscribe grace still resizes the PTY off the phone grid', async () => { + // Why: both take-back tests above force the resize to fail, so nothing pinned that a + // converging remote reclaim reaches the host. A phone that unsubscribes cleanly holds + // driver=mobile for the 250ms resubscribe grace, and applyRemoteDesktopLayout no-ops on + // a mobile driver — so without the idle flip the lock drops and `true` is still reported + // while the PTY stays stranded at the phone grid. + const { runtime, resizeCalls } = createRuntime(null) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + runtime.handleMobileUnsubscribe('pty-1', 'phone-A') + // Inside the grace window on purpose: the driver still reads mobile here. + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + expect(runtime.getTerminalFitOverride('pty-1')).toMatchObject({ mode: 'mobile-fit' }) + const resizesBefore = resizeCalls.length + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) + expect(resizeCalls.slice(resizesBefore)).toContainEqual({ ptyId: 'pty-1', cols: 100, rows: 30 }) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 100, rows: 30 }) + }) }) diff --git a/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts b/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts new file mode 100644 index 00000000000..d2bcec6a7f3 --- /dev/null +++ b/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultUIState } from '../../../../shared/constants' +import { PAIRING_LOCAL_UI_FIELDS } from '../../../../shared/pairing-local-ui-fields' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { RpcRequest } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { CLIENT_UI_METHODS } from './client-ui' + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +// Both directions of the pairing boundary for the fields in PAIRING_LOCAL_UI_FIELDS: a client's +// value must never be persisted by the host, and the host's must never be returned to a client. +describe('client UI RPC pairing-local field seams', () => { + it('drops a paired client manualRepoOrder while forwarding the rest of the payload', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => getDefaultUIState()) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + // A paired web client restamps every repo onto its own runtime:web-* pseudo-host, so its + // overlay names hosts this profile will never own; persisting it erases the desktop order. + const response = await dispatcher.dispatch( + makeRequest('ui.set', { + sidebarWidth: 280, + manualRepoOrder: [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' }, + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-b' } + ] + }) + ) + + expect(response).toMatchObject({ ok: true }) + expect(runtime.updateUIState).toHaveBeenCalledWith({ sidebarWidth: 280 }) + }) + + // Driven off the census so a field added to PAIRING_LOCAL_UI_FIELDS without wiring a seam + // fails here rather than shipping. Sample values are what a paired web client actually sends. + const pairingLocalSamples: Record<(typeof PAIRING_LOCAL_UI_FIELDS)[number], unknown> = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' } + ], + workspaceHostOrder: ['runtime:web-11111111-2222-3333-4444-555555555555', 'local'] + } + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'ui.set never persists the pairing-local field %s', + async (field) => { + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => getDefaultUIState()) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('ui.set', { sidebarWidth: 280, [field]: pairingLocalSamples[field] }) + ) + + expect(response).toMatchObject({ ok: true }) + const forwarded = vi.mocked(runtime.updateUIState).mock.calls[0]?.[0] ?? {} + expect(Object.keys(forwarded)).not.toContain(field) + expect(forwarded).toMatchObject({ sidebarWidth: 280 }) + } + ) + + // The read path matters as much as the write path: a host that echoes these back overwrites the + // client's own value, and on a profile poisoned before the write strip the echoed + // runtime:web-* keys match the very client that minted them. + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'ui.get never returns the pairing-local field %s', + async (field) => { + const runtime = { + getRuntimeId: () => 'test-runtime', + getUIState: vi.fn(() => ({ ...getDefaultUIState(), [field]: pairingLocalSamples[field] })) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const response = await dispatcher.dispatch(makeRequest('ui.get')) + + const ui = (response as { result: { ui: Record } }).result.ui + expect(Object.keys(ui)).not.toContain(field) + expect(ui).toMatchObject({ sidebarWidth: getDefaultUIState().sidebarWidth }) + } + ) + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'the ui.set and ui.recordFeatureInteraction responses omit the pairing-local field %s', + async (field) => { + const stored = { ...getDefaultUIState(), [field]: pairingLocalSamples[field] } + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => stored), + recordFeatureInteraction: vi.fn(() => stored) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const setResponse = await dispatcher.dispatch(makeRequest('ui.set', { sidebarWidth: 280 })) + const interactionResponse = await dispatcher.dispatch( + makeRequest('ui.recordFeatureInteraction', 'tasks') + ) + + for (const response of [setResponse, interactionResponse]) { + const ui = (response as { result: { ui: Record } }).result.ui + expect(Object.keys(ui)).not.toContain(field) + } + } + ) +}) diff --git a/src/main/runtime/rpc/methods/client-ui.test.ts b/src/main/runtime/rpc/methods/client-ui.test.ts index a1f24c28a9f..3bfb7303bd0 100644 --- a/src/main/runtime/rpc/methods/client-ui.test.ts +++ b/src/main/runtime/rpc/methods/client-ui.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { getDefaultUIState } from '../../../../shared/constants' +import { omitPairingLocalUiFields } from '../../../../shared/pairing-local-ui-fields' import { MAX_QUICK_COMMAND_AGENT_PROMPT_LENGTH, MAX_QUICK_COMMAND_ID_LENGTH, @@ -375,7 +376,7 @@ describe('client UI RPC methods', () => { const response = await dispatcher.dispatch(makeRequest('ui.get')) expect(runtime.getUIState).toHaveBeenCalledTimes(1) - expect(response).toMatchObject({ ok: true, result: { ui } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(ui) } }) }) it('persists UI updates on the runtime host and returns the updated state', async () => { @@ -415,7 +416,7 @@ describe('client UI RPC methods', () => { hideAutomationGeneratedWorkspaces: true, filterRepoIds: ['repo-1'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('lets a paired client clear the OSC 52 default-on notice', async () => { @@ -560,11 +561,12 @@ describe('client UI RPC methods', () => { } const response = await dispatcher.dispatch(makeRequest('ui.set', payload)) + const { manualRepoOrder: _desktopOwnedOrder, ...forwarded } = payload expect(runtime.updateUIState).toHaveBeenCalledWith({ - ...payload, + ...forwarded, worktreeCardProperties: ['status', 'unread', 'branch', 'automation', 'inline-agents'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) // Why one case per field: the schema is strict, so a single unlisted key makes @@ -659,7 +661,7 @@ describe('client UI RPC methods', () => { const response = await dispatcher.dispatch(makeRequest('ui.recordFeatureInteraction', 'tasks')) expect(runtime.recordFeatureInteraction).toHaveBeenCalledWith('tasks') - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('rejects unknown and malformed UI update fields', async () => { @@ -721,7 +723,7 @@ describe('client UI RPC methods', () => { expect(runtime.updateUIState).toHaveBeenCalledWith({ worktreeCardProperties: ['status', 'unread', 'jira-issue'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('accepts every worktree card property the shared union defines', async () => { @@ -801,7 +803,7 @@ describe('client UI RPC methods', () => { expect(runtime.updateUIState).toHaveBeenCalledWith({ worktreeCardProperties: ['status', 'unread', 'ci', 'issue', 'pr'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('rejects each star-nag persisted state mutation field from remote clients', async () => { diff --git a/src/main/runtime/rpc/methods/client-ui.ts b/src/main/runtime/rpc/methods/client-ui.ts index 70ad9e9df57..c62328ebc64 100644 --- a/src/main/runtime/rpc/methods/client-ui.ts +++ b/src/main/runtime/rpc/methods/client-ui.ts @@ -1,3 +1,4 @@ +import { omitPairingLocalUiFields } from '../../../../shared/pairing-local-ui-fields' import type { PersistedUIState } from '../../../../shared/persisted-ui-state-types' import { defineMethod, type RpcMethod } from '../core' import { @@ -50,22 +51,24 @@ export const CLIENT_UI_METHODS: RpcMethod[] = [ defineMethod({ name: 'ui.get', params: null, - handler: (_params, { runtime }) => ({ ui: runtime.getUIState() }) + handler: (_params, { runtime }) => ({ ui: omitPairingLocalUiFields(runtime.getUIState()) }) }), defineMethod({ name: 'ui.set', params: UiUpdate, - handler: (params, { runtime }) => { - const { hideWorkspacesFromOtherDevices: _clientLocalFilter, ...hostUpdates } = params - void _clientLocalFilter - return { ui: runtime.updateUIState(hostUpdates as Partial) } - } + // Why the fields are dropped here rather than removed from the schema: UiUpdate is strict, so + // an unlisted key would make the dispatcher reject an old client's ENTIRE payload. + handler: (params, { runtime }) => ({ + ui: omitPairingLocalUiFields( + runtime.updateUIState(omitPairingLocalUiFields(params) as Partial) + ) + }) }), defineMethod({ name: 'ui.recordFeatureInteraction', params: FeatureInteractionIdParam, handler: (params, { runtime }) => ({ - ui: runtime.recordFeatureInteraction(params) + ui: omitPairingLocalUiFields(runtime.recordFeatureInteraction(params)) }) }) ] diff --git a/src/main/runtime/rpc/terminal-subscribe-relay-drop-lease-survival.test.ts b/src/main/runtime/rpc/terminal-subscribe-relay-drop-lease-survival.test.ts new file mode 100644 index 00000000000..5cfab233011 --- /dev/null +++ b/src/main/runtime/rpc/terminal-subscribe-relay-drop-lease-survival.test.ts @@ -0,0 +1,118 @@ +// Why: the stale-handle survival suite doubles subscribeToPtyExit, so it pins the wiring but +// not the predicate that decides whether a PTY counts as exited. This wires that one call to +// a real OrcaRuntimeService, because a relay drop reaches the phone through the predicate. +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from '../orca-runtime' +import type { RpcRequest } from './core' +import { RpcDispatcher } from './dispatcher' +import { TERMINAL_METHODS } from './methods/terminal' +import { createSubscriptionRegistryDouble } from './subscription-registry-test-double' + +const PTY_ID = 'pty-1' + +const subscriptionCases = [ + { + name: 'lease-only', + params: { + terminal: 'terminal-1', + client: { id: 'phone-1', type: 'mobile' }, + capabilities: { terminalBinaryStream: 1, mobileInputLeaseOnly: 1 } + } + }, + { + name: 'legacy JSON', + params: { terminal: 'terminal-1', client: { id: 'desktop-1', type: 'desktop' } } + }, + { + name: 'binary', + params: { + terminal: 'terminal-1', + client: { id: 'phone-1', type: 'mobile' }, + capabilities: { terminalBinaryStream: 1 } + } + } +] as const + +type PtyInternals = { + recordPtyWorktree: (ptyId: string, worktreeId: string, state?: { connected?: boolean }) => unknown + ptysById: Map +} + +/** A PTY whose relay dropped: connection lost, no exit code, process possibly still running. */ +function makeRelayDroppedRuntime(): OrcaRuntimeService { + const real = new OrcaRuntimeService() + const internals = real as unknown as PtyInternals + internals.recordPtyWorktree(PTY_ID, 'wt-1', { connected: true }) + const pty = internals.ptysById.get(PTY_ID)! + pty.connected = false + expect(pty.lastExitCode).toBeNull() + return real +} + +function createRuntime(real: OrcaRuntimeService): { + registry: ReturnType + runtime: OrcaRuntimeService +} { + const registry = createSubscriptionRegistryDouble() + const runtime = { + getRuntimeId: () => 'test-runtime', + requestRendererTerminalTabMount: () => false, + resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: PTY_ID }), + handleMobileSubscribe: vi.fn().mockResolvedValue(true), + handleMobileUnsubscribe: vi.fn(), + registerRemoteTerminalViewSubscriber: vi.fn(() => vi.fn()), + subscribeToTerminalData: vi.fn(() => vi.fn()), + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi + .fn() + .mockResolvedValue({ data: 'snapshot', cols: 80, rows: 24, seq: 1 }), + getTerminalSize: vi.fn().mockReturnValue({ cols: 80, rows: 24 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + isTerminalAlternateScreen: vi.fn().mockReturnValue(false), + subscribeToTerminalResize: vi.fn(() => vi.fn()), + subscribeToFitOverrideChanges: vi.fn(() => vi.fn()), + registerSubscriptionCleanup: vi.fn(registry.registerSubscriptionCleanup), + registerOwnedSubscriptionCleanup: vi.fn(registry.registerOwnedSubscriptionCleanup), + cleanupSubscription: vi.fn(registry.cleanupSubscription), + waitForTerminal: vi.fn(() => Promise.reject(new Error('unexpected handle waiter'))), + // The one call under test: the real exit predicate, not a double's opinion of it. + subscribeToPtyExit: (ptyId: string, listener: () => void) => + real.subscribeToPtyExit(ptyId, listener) + } as unknown as OrcaRuntimeService + return { registry, runtime } +} + +function makeRequest(params: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method: 'terminal.subscribe', params } +} + +describe('terminal subscribe when the relay dropped without an exit code', () => { + it.each(subscriptionCases)( + 'does not end $name for a process that may still be running', + async ({ params }) => { + const real = makeRelayDroppedRuntime() + const { registry, runtime } = createRuntime(real) + const messages: string[] = [] + const types = (): unknown[] => messages.map((message) => JSON.parse(message).result?.type) + + void new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + .dispatchStreaming(makeRequest(params), (message) => messages.push(message), { + connectionId: 'conn-1', + sendBinary: vi.fn(), + registerBinaryStreamHandler: vi.fn(() => vi.fn()) + }) + .catch(() => undefined) + + const subscriptionId = `terminal-1:${params.client.id}` + await vi.waitFor(() => expect(registry.peekCleanup(subscriptionId)).toBeDefined()) + expect(types()).not.toContain('end') + + // The host comes back and reports the real exit; only now may the lease retire. + real.onPtyExit(PTY_ID, 0) + + await vi.waitFor(() => expect(registry.peekCleanup(subscriptionId)).toBeUndefined()) + expect(types()).toContain('end') + } + ) +}) diff --git a/src/main/runtime/terminal-pty-exit-waiter.test.ts b/src/main/runtime/terminal-pty-exit-waiter.test.ts index 5f10449f34d..8b6daa3c2b0 100644 --- a/src/main/runtime/terminal-pty-exit-waiter.test.ts +++ b/src/main/runtime/terminal-pty-exit-waiter.test.ts @@ -4,6 +4,7 @@ import { OrcaRuntimeService } from './orca-runtime' type RuntimeInternals = { recordPtyWorktree: (ptyId: string, worktreeId: string, state?: { connected?: boolean }) => unknown ptyExitListenersByPtyId: Map> + ptysById: Map } function internals(runtime: OrcaRuntimeService): RuntimeInternals { @@ -56,3 +57,56 @@ describe('PTY exit subscription', () => { expect(internals(runtime).ptyExitListenersByPtyId.has('pty-1')).toBe(false) }) }) + +// Why: the liveness sweep clears `connected` with no exit code for every PTY behind a +// dropped relay, so a record in that state is a lost connection to a process that may +// still be running. Reading it as an exit retires a live lease and emits `end`, which the +// phone rearms against three times and then locks the composer on "Waiting for terminal…". +describe('PTY exit subscription demands proof of exit, not loss of connection', () => { + function dropTheConnection(runtime: OrcaRuntimeService): void { + const pty = internals(runtime).ptysById.get('pty-1')! + pty.connected = false + // Precondition: onPtyExit is the only writer of lastExitCode, and it never ran. + expect(pty.lastExitCode).toBeNull() + } + + it('does not fire for a disconnected PTY that never reported an exit code', () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime) + dropTheConnection(runtime) + const listener = vi.fn() + + runtime.subscribeToPtyExit('pty-1', listener) + + expect(listener).not.toHaveBeenCalled() + expect(internals(runtime).ptyExitListenersByPtyId.get('pty-1')).toHaveLength(1) + }) + + it('still fires once the host comes back and reports the real exit', () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime) + dropTheConnection(runtime) + const listener = vi.fn() + + runtime.subscribeToPtyExit('pty-1', listener) + expect(listener).not.toHaveBeenCalled() + + // Why: subscribing during the outage must not cost the subscriber the later exit. + runtime.onPtyExit('pty-1', 0) + + expect(listener).toHaveBeenCalledOnce() + }) + + it('fires immediately for a disconnected PTY that did report an exit code', () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime) + const pty = internals(runtime).ptysById.get('pty-1')! + pty.connected = false + pty.lastExitCode = 0 + const listener = vi.fn() + + runtime.subscribeToPtyExit('pty-1', listener) + + expect(listener).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/updater-release-builds.test.ts b/src/main/updater-release-builds.test.ts index 5e7b55e9f75..30188d73de2 100644 --- a/src/main/updater-release-builds.test.ts +++ b/src/main/updater-release-builds.test.ts @@ -13,11 +13,23 @@ function jsonResponse(body: unknown, init: { ok?: boolean; status?: number } = { } } +/** Every platform's manifest by default, so a case that is not about asset + * filtering stays readable and stays green whatever platform is passed. */ +const allPlatformAssets = [ + { name: 'latest-mac.yml' }, + { name: 'orca-macos-arm64.dmg' }, + { name: 'latest.yml' }, + { name: 'orca-windows-setup.exe' }, + { name: 'latest-linux.yml' }, + { name: 'orca-linux.AppImage' } +] + const release = (tag: string, extra: Record = {}) => ({ tag_name: tag, draft: false, published_at: '2026-07-28T14:00:00Z', html_url: `https://github.com/stablyai/orca/releases/tag/${tag}`, + assets: allPlatformAssets, ...extra }) @@ -35,7 +47,7 @@ describe('listReleaseBuilds', () => { ]) ) - const builds = await listReleaseBuilds('hourly') + const builds = await listReleaseBuilds('hourly', 'darwin') expect(fetchMock.mock.calls[0][0]).toContain('stablyai/orca-hourly') expect(builds.map((build) => build.version)).toEqual([ @@ -54,7 +66,7 @@ describe('listReleaseBuilds', () => { ]) ) - const builds = await listReleaseBuilds('daily') + const builds = await listReleaseBuilds('daily', 'darwin') expect(fetchMock.mock.calls[0][0]).toContain('stablyai/orca-daily') expect(builds.map((build) => build.version)).toEqual([ @@ -71,16 +83,16 @@ describe('listReleaseBuilds', () => { jsonResponse([release('v1.4.160-rc.2'), release('v1.4.159'), release('v1.4.158')]) ) - await expect(listReleaseBuilds('stable').then((b) => b.map((x) => x.version))).resolves.toEqual( - ['1.4.159', '1.4.158'] - ) + await expect( + listReleaseBuilds('stable', 'darwin').then((b) => b.map((x) => x.version)) + ).resolves.toEqual(['1.4.159', '1.4.158']) fetchMock.mockResolvedValue( jsonResponse([release('v1.4.160-rc.2'), release('v1.4.159'), release('v1.4.158')]) ) - await expect(listReleaseBuilds('rc').then((b) => b.map((x) => x.version))).resolves.toEqual([ - '1.4.160-rc.2' - ]) + await expect( + listReleaseBuilds('rc', 'darwin').then((b) => b.map((x) => x.version)) + ).resolves.toEqual(['1.4.160-rc.2']) }) // Why: a draft release has no downloadable assets; offering it makes the @@ -95,7 +107,7 @@ describe('listReleaseBuilds', () => { ]) ) - const builds = await listReleaseBuilds('stable') + const builds = await listReleaseBuilds('stable', 'darwin') expect(builds.map((build) => build.version)).toEqual(['1.4.159']) }) @@ -113,7 +125,7 @@ describe('listReleaseBuilds', () => { ]) ) - const builds = await listReleaseBuilds('hourly') + const builds = await listReleaseBuilds('hourly', 'darwin') expect(builds.map((build) => build.name)).toEqual([ '1.4.163 • 01 • 07-31 13:54 • e698241', null, @@ -122,14 +134,89 @@ describe('listReleaseBuilds', () => { ]) }) + // Why: the mac and Windows legs publish into one release independently, and + // either can fail. Offering a row the running platform has no artifact for + // sends the user into a download that 404s after they commit to it. + it('hides builds that published no artifact for this platform', async () => { + fetchMock.mockResolvedValue( + jsonResponse([ + release('v1.4.163-hourly.202607312054'), + release('v1.4.163-hourly.202607311933', { + assets: [{ name: 'latest-mac.yml' }, { name: 'orca-macos-arm64.dmg' }] + }) + ]) + ) + + await expect( + listReleaseBuilds('hourly', 'win32').then((builds) => builds.map((build) => build.version)) + ).resolves.toEqual(['1.4.163-hourly.202607312054']) + }) + + // The mac-only releases every dev channel published before Windows builds + // existed must simply not appear on Windows, rather than erroring. + it('returns an empty list when no build has this platform artifact', async () => { + fetchMock.mockResolvedValue( + jsonResponse([ + release('v1.4.163-hourly.202607312054', { assets: [{ name: 'latest-mac.yml' }] }) + ]) + ) + + await expect(listReleaseBuilds('hourly', 'win32')).resolves.toEqual([]) + }) + + it('keeps mac builds visible on macOS regardless of the Windows leg', async () => { + fetchMock.mockResolvedValue( + jsonResponse([ + release('v1.4.163-hourly.202607312054', { assets: [{ name: 'latest-mac.yml' }] }) + ]) + ) + + await expect( + listReleaseBuilds('hourly', 'darwin').then((builds) => builds.map((build) => build.version)) + ).resolves.toEqual(['1.4.163-hourly.202607312054']) + }) + + // Why: on Windows a signed stable cannot reach a dev channel through the + // updater, so the picker needs a direct download to hand the user instead. + it('resolves the platform installer download url', async () => { + fetchMock.mockResolvedValue(jsonResponse([release('v1.4.163-hourly.202607312054')])) + + const [build] = await listReleaseBuilds('hourly', 'win32') + + expect(build.installerUrl).toBe( + 'https://github.com/stablyai/orca-hourly/releases/download/v1.4.163-hourly.202607312054/orca-windows-setup.exe' + ) + }) + + it('leaves the installer url null when the release published no installer', async () => { + fetchMock.mockResolvedValue( + jsonResponse([release('v1.4.163-hourly.202607312054', { assets: [{ name: 'latest.yml' }] })]) + ) + + const [build] = await listReleaseBuilds('hourly', 'win32') + + expect(build.installerUrl).toBeNull() + }) + + it('tolerates a release whose assets are missing or malformed', async () => { + fetchMock.mockResolvedValue( + jsonResponse([ + release('v1.4.163-hourly.202607312054', { assets: undefined }), + release('v1.4.163-hourly.202607311933', { assets: [null, { name: 7 }] }) + ]) + ) + + await expect(listReleaseBuilds('hourly', 'win32')).resolves.toEqual([]) + }) + it('surfaces a rate limit as an actionable message', async () => { fetchMock.mockResolvedValue(jsonResponse(null, { ok: false, status: 403 })) - await expect(listReleaseBuilds('hourly')).rejects.toThrow(/rate limit/i) + await expect(listReleaseBuilds('hourly', 'darwin')).rejects.toThrow(/rate limit/i) }) it('reports a missing hourly repo distinctly', async () => { fetchMock.mockResolvedValue(jsonResponse(null, { ok: false, status: 404 })) - await expect(listReleaseBuilds('hourly')).rejects.toThrow(/No releases repository/i) + await expect(listReleaseBuilds('hourly', 'darwin')).rejects.toThrow(/No releases repository/i) }) }) diff --git a/src/main/updater-release-builds.ts b/src/main/updater-release-builds.ts index f4f694facf4..01478535018 100644 --- a/src/main/updater-release-builds.ts +++ b/src/main/updater-release-builds.ts @@ -1,7 +1,9 @@ import { net } from 'electron' import { + findInstallerAssetName, getReleaseRepoForChannel, getVersionChannel, + hasInstallableArtifactForPlatform, normalizeTagToVersion, sortReleaseBuildsNewestFirst, type ReleaseBuild, @@ -26,9 +28,23 @@ type GitHubReleaseEntry = { draft?: unknown published_at?: unknown html_url?: unknown + assets?: unknown } -function parseReleaseEntry(entry: GitHubReleaseEntry, repo: string): ReleaseBuild | null { +function readAssetNames(assets: unknown): string[] { + if (!Array.isArray(assets)) { + return [] + } + return assets + .map((asset) => (asset as { name?: unknown })?.name) + .filter((name): name is string => typeof name === 'string') +} + +function parseReleaseEntry( + entry: GitHubReleaseEntry, + repo: string, + platform: NodeJS.Platform +): ReleaseBuild | null { if (typeof entry.tag_name !== 'string' || entry.draft === true) { return null } @@ -38,6 +54,15 @@ function parseReleaseEntry(entry: GitHubReleaseEntry, repo: string): ReleaseBuil if (!isValidVersion(version) || !channel) { return null } + // Why filter on assets rather than on a per-channel platform table: a release + // is published as soon as one platform's leg finishes, and a leg can fail + // outright. Asking what the release actually carries covers both without the + // picker ever offering a row whose download 404s. + const assetNames = readAssetNames(entry.assets) + if (!hasInstallableArtifactForPlatform(platform, assetNames)) { + return null + } + const installerAsset = findInstallerAssetName(platform, assetNames) // Why null when it merely repeats the tag: GitHub titles an untitled release // with its tag name, and hourlies predating the naming change were created that // way too. Neither says anything the version beside it does not. @@ -51,7 +76,10 @@ function parseReleaseEntry(entry: GitHubReleaseEntry, repo: string): ReleaseBuil releaseUrl: typeof entry.html_url === 'string' ? entry.html_url - : `https://github.com/${repo}/releases/tag/${encodeURIComponent(tag)}` + : `https://github.com/${repo}/releases/tag/${encodeURIComponent(tag)}`, + installerUrl: installerAsset + ? `${getReleaseDownloadUrlForRepo(repo, tag)}/${encodeURIComponent(installerAsset)}` + : null } } @@ -64,7 +92,10 @@ function parseReleaseEntry(entry: GitHubReleaseEntry, repo: string): ReleaseBuil * yesterday's hourly". This runs only on explicit dev interaction, so its * unauthenticated rate limit never touches background checks. */ -export async function listReleaseBuilds(channel: ReleaseChannel): Promise { +export async function listReleaseBuilds( + channel: ReleaseChannel, + platform: NodeJS.Platform = process.platform +): Promise { const repo = getReleaseRepoForChannel(channel) const res = await net.fetch(getReleasesApiUrl(repo), { headers: { Accept: 'application/vnd.github+json' }, @@ -84,7 +115,7 @@ export async function listReleaseBuilds(channel: ReleaseChannel): Promise parseReleaseEntry(entry as GitHubReleaseEntry, repo)) + .map((entry) => parseReleaseEntry(entry as GitHubReleaseEntry, repo, platform)) .filter((build): build is ReleaseBuild => build !== null) // Why: the main repo serves both stable and rc, so filter to the asked-for channel. .filter((build) => build.channel === channel) diff --git a/src/main/updater.build-channel-selection.test.ts b/src/main/updater.build-channel-selection.test.ts index d4f85c419e1..9e036cc2b9e 100644 --- a/src/main/updater.build-channel-selection.test.ts +++ b/src/main/updater.build-channel-selection.test.ts @@ -33,11 +33,23 @@ describe('updater', () => { }) it.each([ - ['hourly', 'v1.4.160-hourly.202607281400', 'Hourly builds are produced only for macOS.'], - ['daily', 'v1.4.160-daily.202607281300', 'Daily builds are produced only for macOS.'], - ['adhoc', 'v1.4.160-adhoc.20260728140533', 'Adhoc builds are produced only for macOS.'] + [ + 'hourly', + 'v1.4.160-hourly.202607281400', + 'Hourly builds are produced only for macOS and Windows.' + ], + [ + 'daily', + 'v1.4.160-daily.202607281300', + 'Daily builds are produced only for macOS and Windows.' + ], + [ + 'adhoc', + 'v1.4.160-adhoc.20260728140533', + 'Adhoc builds are produced only for macOS and Windows.' + ] ] as const)( - 'uses the display label in the mac-only %s pinned-build error', + 'uses the display label in the unsupported-platform %s pinned-build error', async (channel, tag, message) => { const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('linux') try { @@ -61,6 +73,65 @@ describe('updater', () => { } ) + // Why this refuses rather than trying: electron-updater would download the + // whole installer and then fail it with a raw ERR_UPDATER_INVALID_SIGNATURE, + // because a signed build verifies every installer against the publisherName + // baked into its own app-update.yml. The picker disables this, but IPC is + // reachable regardless. + it('refuses to pin a Windows dev build from a signed build, and says what to do', async () => { + const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + appMock.getVersion.mockReturnValue('1.4.160') + const send = vi.fn() + const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu({ channel: 'adhoc', targetTag: 'v1.4.160-adhoc.20260728140533' }) + + expect(send).toHaveBeenCalledWith('updater:status', { + state: 'error', + message: expect.stringContaining('Download the installer from the release page'), + userInitiated: true + }) + expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + } finally { + platformSpy.mockRestore() + } + }) + + // The way out of a dev channel must stay in-app: an unsigned build carries no + // publisherName, so electron-updater skips verification entirely. + it.each([ + ['another dev build', 'adhoc', 'v1.4.160-adhoc.20260728140533'], + ['back to stable', 'stable', 'v1.4.160'] + ] as const)( + 'still pins %s from an unsigned Windows dev build', + async (_label, channel, targetTag) => { + const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + appMock.getVersion.mockReturnValue('1.4.160-hourly.202607281400') + const send = vi.fn() + const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu({ channel, targetTag }) + + expect(send).not.toHaveBeenCalledWith('updater:status', { + state: 'error', + message: expect.stringContaining('Download the installer'), + userInitiated: true + }) + expect(autoUpdaterMock.allowDowngrade).toBe(true) + } finally { + platformSpy.mockRestore() + } + } + ) + it.runIf(process.platform === 'darwin')( 'allows a validated local build to downgrade through the normal updater lifecycle', async () => { diff --git a/src/main/updater.ts b/src/main/updater.ts index f89c00331a7..640906197c8 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -74,9 +74,12 @@ import { import type { LocalBuildFeed } from './local-builds/local-build-feed-server' import { listReleaseBuilds, resolveTargetBuild } from './updater-release-builds' import { + DEV_CHANNEL_PLATFORM_LABEL, + getVersionChannel, hasDedicatedReleaseRepo, isChannelSupportedOnPlatform, RELEASE_CHANNEL_LABELS, + requiresManualDevChannelInstall, type ReleaseBuild, type ReleaseChannel } from '../shared/release-channel' @@ -1737,11 +1740,29 @@ async function checkForPinnedBuild(channel: ReleaseChannel, tag: string): Promis return } // Why here as well as in the picker: the renderer disables the option, but IPC - // is reachable regardless, and there is no artifact to install off-macOS. + // is reachable regardless, and there is no artifact to install on a platform + // the dev workflows do not build for. if (!isChannelSupportedOnPlatform(channel, process.platform)) { sendStatus({ state: 'error', - message: `${RELEASE_CHANNEL_LABELS[channel]} builds are produced only for macOS.`, + message: `${RELEASE_CHANNEL_LABELS[channel]} builds are produced only for ${DEV_CHANNEL_PLATFORM_LABEL}.`, + userInitiated: true + }) + return + } + // Why: electron-updater would otherwise take this all the way to a download + // and fail it with a raw ERR_UPDATER_INVALID_SIGNATURE. Say what to do instead + // — the installer is run by hand once, and in-app updates work from there on. + if ( + requiresManualDevChannelInstall({ + platform: process.platform, + runningChannel: getVersionChannel(app.getVersion()), + targetChannel: channel + }) + ) { + sendStatus({ + state: 'error', + message: `${RELEASE_CHANNEL_LABELS[channel]} builds are unsigned, and this signed build only installs updates signed by Orca's publisher. Download the installer from the release page and run it once — updates work normally from there, including back to Stable.`, userInitiated: true }) return diff --git a/src/renderer/src/components/settings/ReleaseChannelSection.tsx b/src/renderer/src/components/settings/ReleaseChannelSection.tsx index bfe2321bd32..cd0a8484cfe 100644 --- a/src/renderer/src/components/settings/ReleaseChannelSection.tsx +++ b/src/renderer/src/components/settings/ReleaseChannelSection.tsx @@ -1,6 +1,6 @@ import type React from 'react' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' -import { AlertTriangle, Apple, Loader2, RefreshCw } from 'lucide-react' +import { AlertTriangle, Download, Loader2, RefreshCw } from 'lucide-react' import { toast } from 'sonner' import { useAppStore } from '../../store' import { Button } from '../ui/button' @@ -10,12 +10,14 @@ import { Badge } from '../ui/badge' import { translate } from '@/i18n/i18n' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { + DEV_CHANNEL_PLATFORM_LABEL, RELEASE_CHANNELS, RELEASE_CHANNEL_LABELS, getVersionChannel, hasDedicatedReleaseRepo, isChannelSupportedOnPlatform, parseDevBuildStamp, + requiresManualDevChannelInstall, type DedicatedRepoChannel, type ReleaseBuild, type ReleaseChannel @@ -24,10 +26,10 @@ import { const CHANNEL_DESCRIPTIONS: Record = { stable: 'Shipped releases. What everyone else is running.', rc: 'Release candidates cut ahead of each stable.', - hourly: 'macOS only. Unvetted builds from main, built every hour. No tests.', + hourly: 'macOS and Windows. Unvetted builds from main, built every hour. No tests.', daily: - 'macOS only. Unvetted builds from main, cut once a day at 14:15 UTC (early morning Pacific). No tests.', - adhoc: 'macOS only. One-off builds cut from a branch to try a feature before it lands.' + 'macOS and Windows. Unvetted builds from main, cut once a day at 14:15 UTC (early morning Pacific). No tests.', + adhoc: 'macOS and Windows. One-off builds cut from a branch to try a feature before it lands.' } const DEDICATED_CHANNEL_WARNINGS: Record = @@ -35,17 +37,17 @@ const DEDICATED_CHANNEL_WARNINGS: Record { + // Falls back to the release page: better to land somewhere with the assets + // listed than to do nothing because the artifact was named unexpectedly. + void window.api.shell.openUrl(build.installerUrl ?? build.releaseUrl) + } return (
@@ -215,28 +232,21 @@ export function ReleaseChannelSection(): React.JSX.Element { const supported = isChannelSupportedOnPlatform(channel, platform) return { value: channel, - label: supported ? ( - RELEASE_CHANNEL_LABELS[channel] - ) : ( - - {RELEASE_CHANNEL_LABELS[channel]} - - ), + label: RELEASE_CHANNEL_LABELS[channel], disabled: !supported, ariaLabel: supported ? undefined : translate( - 'auto.components.settings.ReleaseChannelSection.devChannelMacOnlyAria', - '{{value0}} (macOS only)', - { value0: RELEASE_CHANNEL_LABELS[channel] } + 'auto.components.settings.ReleaseChannelSection.devChannelUnsupportedAria', + '{{value0}} ({{value1}} only)', + { value0: RELEASE_CHANNEL_LABELS[channel], value1: DEV_CHANNEL_PLATFORM_LABEL } ), tooltip: supported ? undefined : translate( - 'auto.components.settings.ReleaseChannelSection.devChannelMacOnly', - '{{value0}} builds are produced only for macOS. Linux and Windows stay on Stable or RC.', - { value0: RELEASE_CHANNEL_LABELS[channel] } + 'auto.components.settings.ReleaseChannelSection.devChannelUnsupported', + '{{value0}} builds are produced only for {{value1}}. Linux stays on Stable or RC.', + { value0: RELEASE_CHANNEL_LABELS[channel], value1: DEV_CHANNEL_PLATFORM_LABEL } ) } })} @@ -310,10 +320,27 @@ export function ReleaseChannelSection(): React.JSX.Element { size="sm" type="button" disabled={!selectedBuild || busy || isRunningBuild} - onClick={() => selectedBuild && handleSwitchTo(selectedBuild)} + onClick={() => { + if (!selectedBuild) { + return + } + if (needsManualInstall) { + handleDownloadInstaller(selectedBuild) + return + } + handleSwitchTo(selectedBuild) + }} > {busy ? ( + ) : needsManualInstall ? ( + + ) : ( translate( 'auto.components.settings.ReleaseChannelSection.switchTo', @@ -325,6 +352,14 @@ export function ReleaseChannelSection(): React.JSX.Element { {loadError ? (

{loadError}

+ ) : needsManualInstall && selectedBuild ? ( +

+ {translate( + 'auto.components.settings.ReleaseChannelSection.manualInstallHint', + '{{value0}} builds are unsigned on Windows, so the in-app updater cannot install one over a signed build. Run the downloaded installer once — Windows will warn about an unknown publisher — and every later switch, including back to Stable, works from here.', + { value0: RELEASE_CHANNEL_LABELS[activeChannel] } + )} +

) : isRunningBuild ? (

{translate( diff --git a/src/renderer/src/components/settings/SettingsFormControls.segmented-control.test.tsx b/src/renderer/src/components/settings/SettingsFormControls.segmented-control.test.tsx index 00e1b728748..55fd12e9119 100644 --- a/src/renderer/src/components/settings/SettingsFormControls.segmented-control.test.tsx +++ b/src/renderer/src/components/settings/SettingsFormControls.segmented-control.test.tsx @@ -18,8 +18,8 @@ function renderControl(onChange = vi.fn()): { onChange: ReturnType value: 'hourly', label: 'Hourly', disabled: true, - ariaLabel: 'Hourly (macOS only)', - tooltip: 'Hourly builds are produced only for macOS.' + ariaLabel: 'Hourly (macOS and Windows only)', + tooltip: 'Hourly builds are produced only for macOS and Windows.' } ]} /> @@ -34,20 +34,20 @@ describe('SettingsSegmentedControl unavailable options', () => { it('marks unavailable options aria-disabled so their tooltip stays reachable', async () => { renderControl() - const hourly = screen.getByRole('radio', { name: 'Hourly (macOS only)' }) + const hourly = screen.getByRole('radio', { name: 'Hourly (macOS and Windows only)' }) expect(hourly.hasAttribute('disabled')).toBe(false) expect(hourly.getAttribute('aria-disabled')).toBe('true') fireEvent.focus(hourly) expect( - await screen.findAllByText('Hourly builds are produced only for macOS.') + await screen.findAllByText('Hourly builds are produced only for macOS and Windows.') ).not.toHaveLength(0) }) it('ignores clicks on unavailable options', () => { const { onChange } = renderControl() - fireEvent.click(screen.getByRole('radio', { name: 'Hourly (macOS only)' })) + fireEvent.click(screen.getByRole('radio', { name: 'Hourly (macOS and Windows only)' })) expect(onChange).not.toHaveBeenCalled() fireEvent.click(screen.getByRole('radio', { name: 'Stable' })) diff --git a/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts b/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts index 4aa654cf33a..cc13aa983aa 100644 --- a/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts +++ b/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts @@ -113,6 +113,59 @@ describe('createDirectSshHostHydration', () => { ]) }) + it('keeps the manual cross-host order when the host catalog is republished', async () => { + const owner = authority('box') + const store = createStore(() => + state({ + repos: [repo('bravo', 'box'), repo('alpha', null), repo('delta', 'box')], + manualRepoOrder: [ + { hostId: 'ssh:box', repoId: 'bravo' }, + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'delta' } + ] + }) + ) + const hydration = createDirectSshHostHydration({ + store, + listRepos: vi.fn(async () => + hostSnapshot(owner, [repo('bravo', 'box'), repo('delta', 'box')]) + ), + listLineage: vi.fn(), + isCurrentAuthority: () => true + }) + + await hydration.capturePreparationInput(owner, 'reconnect') + + expect(store.getState().repos.map((entry) => entry.id)).toEqual(['bravo', 'alpha', 'delta']) + }) + + // A repo added after the last drag has no overlay entry. Ranked rows must keep their order and + // the newcomer sinks to the tail, rather than the overlay being discarded for being incomplete. + it('keeps ranked rows in order and appends unranked ones when the overlay is partial', async () => { + const owner = authority('box') + const store = createStore(() => + state({ + repos: [repo('bravo', 'box'), repo('alpha', null), repo('delta', 'box')], + manualRepoOrder: [ + { hostId: 'ssh:box', repoId: 'delta' }, + { hostId: 'local', repoId: 'alpha' } + ] + }) + ) + const hydration = createDirectSshHostHydration({ + store, + listRepos: vi.fn(async () => + hostSnapshot(owner, [repo('bravo', 'box'), repo('delta', 'box')]) + ), + listLineage: vi.fn(), + isCurrentAuthority: () => true + }) + + await hydration.capturePreparationInput(owner, 'reconnect') + + expect(store.getState().repos.map((entry) => entry.id)).toEqual(['delta', 'alpha', 'bravo']) + }) + it('rejects a mismatched host response without publishing', async () => { const owner = authority() const store = createStore(() => state({ repos: [repo('cached', 'target-a')] })) diff --git a/src/renderer/src/hooks/direct-ssh-host-hydration.ts b/src/renderer/src/hooks/direct-ssh-host-hydration.ts index 069b3b3d323..419dc134075 100644 --- a/src/renderer/src/hooks/direct-ssh-host-hydration.ts +++ b/src/renderer/src/hooks/direct-ssh-host-hydration.ts @@ -2,6 +2,7 @@ import type { StoreApi } from 'zustand' import { getRepoExecutionHostId, toSshExecutionHostId } from '../../../shared/execution-host' import type { HostLineageSnapshot } from '../../../shared/host-lineage-contract' import type { HostRepoCatalogSnapshot } from '../../../shared/host-repo-catalog-contract' +import { applyManualRepoOrder } from '../../../shared/manual-repo-order' import type { DirectSshAuthority } from '../../../shared/ssh-types' import { isWorkspaceKey } from '../../../shared/workspace-scope' import type { AppState } from '../store/types' @@ -63,12 +64,14 @@ function mergeExactHostCatalog(state: AppState, snapshot: HostRepoCatalogSnapsho return state } const hostId = snapshot.authority.executionHostId + // Why re-apply the overlay: re-appending the host's rows puts them at the tail, which would + // undo the user's manual cross-host order on every connect. return { ...state, - repos: [ - ...state.repos.filter((repo) => getRepoExecutionHostId(repo) !== hostId), - ...snapshot.repos - ] + repos: applyManualRepoOrder( + [...state.repos.filter((repo) => getRepoExecutionHostId(repo) !== hostId), ...snapshot.repos], + state.manualRepoOrder + ) } } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f2f3e4567b6..66e994e3b7f 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -11071,9 +11071,9 @@ "description": "Switch update channels or jump to any published build, including older ones. Downgrades are allowed and unvetted builds can be broken.", "devOnly": "Dev only", "channelAriaLabel": "Update channel", - "hourlyWarning": "Hourly builds are macOS-only and ship straight from main with no test gate. Keep a stable build handy.", - "dailyWarning": "Daily builds are macOS-only and ship straight from main with no test gate. Keep a stable build handy.", - "adhocWarning": "Adhoc builds are macOS-only and come from a branch that has not landed. Whoever cut one may abandon it — keep a stable build handy.", + "hourlyWarning": "Hourly builds ship straight from main with no test gate, and the Windows ones are unsigned. Keep a stable build handy.", + "dailyWarning": "Daily builds ship straight from main with no test gate, and the Windows ones are unsigned. Keep a stable build handy.", + "adhocWarning": "Adhoc builds come from a branch that has not landed, and the Windows ones are unsigned. Whoever cut one may abandon it — keep a stable build handy.", "loadingBuilds": "Loading builds…", "noBuilds": "No builds found", "refresh": "Refresh build list", @@ -11081,8 +11081,10 @@ "alreadyRunning": "This is the build you are running.", "willSwitch": "{{value0}} → {{value1}}", "webUnavailable": "Switching builds is only available in the desktop app.", - "devChannelMacOnlyAria": "{{value0}} (macOS only)", - "devChannelMacOnly": "{{value0}} builds are produced only for macOS. Linux and Windows stay on Stable or RC." + "devChannelUnsupportedAria": "{{value0}} ({{value1}} only)", + "devChannelUnsupported": "{{value0}} builds are produced only for {{value1}}. Linux stays on Stable or RC.", + "downloadInstaller": "Download installer", + "manualInstallHint": "{{value0}} builds are unsigned on Windows, so the in-app updater cannot install one over a signed build. Run the downloaded installer once — Windows will warn about an unknown publisher — and every later switch, including back to Stable, works from here." }, "VoiceMicrophoneSetting": { "systemDefault": "System default", diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts index 8128e9ae8a0..0a24c776c62 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -8,6 +8,7 @@ import { type IssueCommandReadResult } from '@/runtime/runtime-hooks-client' import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner' +import { MODAL_DISMISSED_KEY } from '@/store/slices/modal-slot-dismissal' import { getRepoExecutionHostId, parseExecutionHostId, @@ -128,6 +129,14 @@ async function confirmScriptContent( const previouslyApproved = Boolean(existingHash) return new Promise<'run' | 'skip'>((resolve) => { + let settled = false + const settle = (decision: 'run' | 'skip'): void => { + if (settled) { + return + } + settled = true + resolve(decision) + } state.openModal('confirm-orca-yaml-hooks', { repoId, repoName, @@ -135,7 +144,9 @@ async function confirmScriptContent( scriptContent, contentHash, previouslyApproved, - onResolve: (decision: 'run' | 'skip') => resolve(decision) + onResolve: settle, + // Why: eviction must fail closed without stranding the singleton trust queue. + [MODAL_DISMISSED_KEY]: () => settle('skip') }) }) } diff --git a/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts b/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts new file mode 100644 index 00000000000..56bd061af6b --- /dev/null +++ b/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { StoreApi } from 'zustand/vanilla' +import type { AppState } from '@/store/types' +import { createUIStore } from '@/store/slices/ui-slice-test-harness' +import { __resetTrustPromptChainForTests, ensureHooksConfirmed } from './ensure-hooks-confirmed' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '@/runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '@/runtime/runtime-rpc-client' + +const hooksCheckMock = vi.fn() +const readIssueCommandMock = vi.fn() +const runtimeEnvironmentTransportCallMock = vi.fn() + +// Why: a hand-rolled openModal double would decide the eviction outcome under test. +function createStateBackedByRealModalSlot(): { + store: StoreApi + state: AppState +} { + const store = createUIStore() + store.setState({ + repos: [{ id: 'repo-1', displayName: 'Repo One' }], + trustedOrcaHooks: {} + } as unknown as Partial) + return { store, state: store.getState() } +} + +async function settleOrReport(promise: Promise): Promise { + return Promise.race([ + promise, + new Promise<'never-settled'>((resolve) => setTimeout(() => resolve('never-settled'), 100)) + ]) +} + +describe('orca.yaml trust prompt evicted from the modal slot', () => { + beforeEach(() => { + hooksCheckMock.mockReset() + readIssueCommandMock.mockReset() + runtimeEnvironmentTransportCallMock.mockReset() + runtimeEnvironmentTransportCallMock.mockImplementation((args: RuntimeEnvironmentCallRequest) => + createCompatibleRuntimeStatusResponseIfNeeded(args) + ) + clearRuntimeCompatibilityCacheForTests() + vi.stubGlobal('window', { + api: { + hooks: { check: hooksCheckMock, readIssueCommand: readIssueCommandMock }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCallMock } + } + }) + __resetTrustPromptChainForTests() + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'pnpm install' } }, + mayNeedUpdate: false + }) + }) + + it('resolves the pending decision as skip when another modal takes the slot', async () => { + const { store, state } = createStateBackedByRealModalSlot() + const runSetup = vi.fn() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + store.getState().openModal('worktree-palette') + + const result = await settleOrReport(decision) + if (result === 'run') { + runSetup() + } + + expect(result).toBe('skip') + expect(runSetup).not.toHaveBeenCalled() + expect(store.getState().trustedOrcaHooks).toEqual({}) + }) + + it('resolves the pending decision as skip when the slot is closed outright', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + store.getState().closeModal() + + await expect(settleOrReport(decision)).resolves.toBe('skip') + }) + + it('keeps later trust prompts working after one is evicted', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const evicted = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + store.getState().openModal('worktree-palette') + await settleOrReport(evicted) + + const next = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + const onResolve = store.getState().modalData.onResolve as (d: 'run' | 'skip') => void + onResolve('run') + + await expect(settleOrReport(next)).resolves.toBe('run') + }) + + it('leaves the answered decision alone when the dialog closes the slot itself', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + // Mirrors the trust dialog: resolve first, then vacate the slot. + ;(store.getState().modalData.onResolve as (d: 'run' | 'skip') => void)('run') + store.getState().closeModal() + + await expect(settleOrReport(decision)).resolves.toBe('run') + }) +}) diff --git a/src/renderer/src/store/slices/modal-slot-dismissal.ts b/src/renderer/src/store/slices/modal-slot-dismissal.ts new file mode 100644 index 00000000000..d98a7e50f56 --- /dev/null +++ b/src/renderer/src/store/slices/modal-slot-dismissal.ts @@ -0,0 +1,9 @@ +// Modal owners with pending work use this hook to settle when the singleton slot evicts them. +export const MODAL_DISMISSED_KEY = 'onModalDismissed' + +export function settleEvictedModalData(evicted: Record): void { + const onDismissed = evicted[MODAL_DISMISSED_KEY] + if (typeof onDismissed === 'function') { + ;(onDismissed as () => void)() + } +} diff --git a/src/renderer/src/store/slices/ui-modal-slot-dismissal.test.ts b/src/renderer/src/store/slices/ui-modal-slot-dismissal.test.ts new file mode 100644 index 00000000000..c858e0d507e --- /dev/null +++ b/src/renderer/src/store/slices/ui-modal-slot-dismissal.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it, vi } from 'vitest' +import { createUIStore } from './ui-slice-test-harness' +import { MODAL_DISMISSED_KEY } from './modal-slot-dismissal' + +function dismissalData(onDismissed: () => void): Record { + return { [MODAL_DISMISSED_KEY]: onDismissed } +} + +describe('UI modal slot dismissal', () => { + it('dismisses replaced and closed entries once', () => { + const store = createUIStore() + const firstDismissed = vi.fn() + const secondDismissed = vi.fn() + + store.getState().openModal('worktree-palette', dismissalData(firstDismissed)) + store.getState().openModal('quick-open', dismissalData(secondDismissed)) + store.getState().closeModal() + store.getState().closeModal() + + expect(firstDismissed).toHaveBeenCalledOnce() + expect(secondDismissed).toHaveBeenCalledOnce() + }) + + it('dismisses each entry across consecutive replacements', () => { + const store = createUIStore() + const firstDismissed = vi.fn() + const secondDismissed = vi.fn() + const thirdDismissed = vi.fn() + + store.getState().openModal('worktree-palette', dismissalData(firstDismissed)) + store.getState().openModal('quick-open', dismissalData(secondDismissed)) + store.getState().openModal('add-repo', dismissalData(thirdDismissed)) + store.getState().closeModal() + + expect(firstDismissed).toHaveBeenCalledOnce() + expect(secondDismissed).toHaveBeenCalledOnce() + expect(thirdDismissed).toHaveBeenCalledOnce() + }) + + it('updates the slot before notifying the evicted entry', () => { + const store = createUIStore() + const observedModal = vi.fn(() => store.getState().activeModal) + + store.getState().openModal('worktree-palette', dismissalData(observedModal)) + store.getState().openModal('quick-open') + + expect(observedModal).toHaveReturnedWith('quick-open') + }) + + it('ignores modal data without a callable dismissal hook', () => { + const store = createUIStore() + + store.getState().openModal('worktree-palette', { [MODAL_DISMISSED_KEY]: 'invalid' }) + + expect(() => store.getState().openModal('quick-open')).not.toThrow() + expect(store.getState().activeModal).toBe('quick-open') + }) + + it('settles reentrant replacement and close paths once', () => { + const store = createUIStore() + const replacementDismissed = vi.fn() + const reentrantDismissed = vi.fn() + const firstDismissed = vi.fn(() => { + store.getState().openModal('quick-open', dismissalData(reentrantDismissed)) + }) + + store.getState().openModal('worktree-palette', dismissalData(firstDismissed)) + store.getState().openModal('add-repo', dismissalData(replacementDismissed)) + store.getState().closeModal() + + expect(firstDismissed).toHaveBeenCalledOnce() + expect(replacementDismissed).toHaveBeenCalledOnce() + expect(reentrantDismissed).toHaveBeenCalledOnce() + expect(store.getState().activeModal).toBe('none') + }) + + it('performs one bounded notification per transition under repeated replacement', () => { + const store = createUIStore() + const dismissed = new Set() + const transitionCount = 1_000 + + for (let id = 0; id < transitionCount; id += 1) { + store.getState().openModal( + 'worktree-palette', + dismissalData(() => { + expect(dismissed.has(id)).toBe(false) + dismissed.add(id) + }) + ) + } + store.getState().closeModal() + + expect(dismissed.size).toBe(transitionCount) + expect(store.getState().modalData).toEqual({}) + }) +}) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index e89e990719d..e06734e4923 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -2,6 +2,7 @@ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import { normalizeRightSidebarRoute } from '../right-sidebar-route' +import { settleEvictedModalData } from './modal-slot-dismissal' import { findPrevLiveNonTaskStackHistoryIndex, findPrevLiveWorktreeHistoryIndex @@ -1621,12 +1622,18 @@ export const createUISlice: StateCreator = (set, get) if (modal === 'add-repo' || modal === 'create-worktree') { get().recordFeatureInteraction?.('workspace-creation') } + const evicted = get().modalData set({ activeModal: modal, modalData: data }) + settleEvictedModalData(evicted) + }, + closeModal: () => { + const evicted = get().modalData + set({ activeModal: 'none', modalData: {} }) + settleEvictedModalData(evicted) }, - closeModal: () => set({ activeModal: 'none', modalData: {} }), featureTipsSeenIds: [], markFeatureTipsSeen: (ids) => set((s) => { diff --git a/src/renderer/src/web/web-preload-api-ui.test.ts b/src/renderer/src/web/web-preload-api-ui.test.ts index e809210aac1..ce5e9572f9b 100644 --- a/src/renderer/src/web/web-preload-api-ui.test.ts +++ b/src/renderer/src/web/web-preload-api-ui.test.ts @@ -1,6 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { FeatureInteractionState } from '../../../shared/feature-interactions' +import { + PAIRING_LOCAL_UI_FIELDS, + type PairingLocalUiField +} from '../../../shared/pairing-local-ui-fields' +import type { PersistedUIState } from '../../../shared/persisted-ui-state-types' import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' +import type { ManualRepoOrderEntry } from '../../../shared/ui-chrome-types' import { installApi, installBrowserGlobals, @@ -327,6 +333,181 @@ describe('web UI preload API', () => { }) }) + it('keeps the manual repo order browser-local instead of overwriting the host order', async () => { + const runtimeCalls: { method: string; params: unknown }[] = [] + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string, params?: unknown): Promise> { + runtimeCalls.push({ method, params }) + return Promise.resolve({ + id: method, + ok: true, + result: { ui: {} }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + // The web client keys its order to its own runtime:web-* host, so a host that predates the + // RPC-side strip would persist it over the desktop's local/ssh-keyed order. + const manualRepoOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-b' }, + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' } + ] + await globals.window.api.ui.set({ manualRepoOrder, sidebarWidth: 280 }) + + expect(runtimeCalls[0]).toEqual({ method: 'ui.set', params: { sidebarWidth: 280 } }) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject({ + manualRepoOrder, + sidebarWidth: 280 + }) + }) + + // Why cover a host that still sends the field: clients and hosts update independently, so a new + // web client meets hosts predating the response omission. On a profile an old web client + // poisoned, those echoed runtime:web-* entries match this browser's own repos and would beat + // every drag it has made since. + it.each([ + [ + 'this browser own stale runtime:web-* entries', + [ + { hostId: 'runtime:web-env-1', repoId: 'repo-c' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + ], + [ + 'the desktop local/ssh-keyed order', + [ + { hostId: 'local', repoId: 'repo-a' }, + { hostId: 'ssh:box', repoId: 'repo-b' } + ] + ], + ['an empty overlay from a desktop that never dragged', []] + ])('keeps the browser-local manual repo order when ui.get returns %s', async (_label, hosted) => { + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string): Promise> { + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { manualRepoOrder: hosted } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const webOwnOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-env-1', repoId: 'repo-b' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify({ manualRepoOrder: webOwnOrder })) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await expect(globals.window.api.ui.get()).resolves.toMatchObject({ + manualRepoOrder: webOwnOrder + }) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject({ + manualRepoOrder: webOwnOrder + }) + }) + + it('keeps the browser-local manual repo order when a new host omits it entirely', async () => { + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string): Promise> { + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { sidebarWidth: 320 } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const webOwnOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-env-1', repoId: 'repo-b' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify({ manualRepoOrder: webOwnOrder })) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await expect(globals.window.api.ui.get()).resolves.toMatchObject({ + manualRepoOrder: webOwnOrder, + sidebarWidth: 320 + }) + }) + + // Census-driven, matching the host-side seam tests: a field added to PAIRING_LOCAL_UI_FIELDS + // without wiring the web read seam fails here rather than shipping. The host sample differs from + // the browser's for every field, so only the pin makes this pass. + const browserLocalUiSamples: Record = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [{ hostId: 'runtime:web-env-1', repoId: 'repo-b' }], + workspaceHostOrder: ['runtime:web-env-1', 'local'] + } + const hostUiSamples: Record = { + hideWorkspacesFromOtherDevices: false, + manualRepoOrder: [{ hostId: 'local', repoId: 'repo-a' }], + workspaceHostOrder: ['local', 'ssh:box'] + } + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'keeps the browser-local %s and never sends it to the host', + async (field) => { + const runtimeCalls: { method: string; params: unknown }[] = [] + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string, params?: unknown): Promise> { + runtimeCalls.push({ method, params }) + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { [field]: hostUiSamples[field] } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const browserLocal = { [field]: browserLocalUiSamples[field] } as Partial + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify(browserLocal)) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await globals.window.api.ui.set({ ...browserLocal, sidebarWidth: 280 }) + + expect(runtimeCalls[0]).toEqual({ method: 'ui.set', params: { sidebarWidth: 280 } }) + await expect(globals.window.api.ui.get()).resolves.toMatchObject(browserLocal) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject( + browserLocal + ) + } + ) + it('union-merges local contextual tour seen ids when ui.get returns stale host state', async () => { vi.doMock('./web-runtime-client', () => ({ WebRuntimeClient: class { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index bc590675289..a4c62a584cf 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -32,6 +32,11 @@ import type { } from '../../../shared/global-settings-types' import type { OnboardingState } from '../../../shared/onboarding-state-types' import type { PersistedUIState } from '../../../shared/persisted-ui-state-types' +import { + omitPairingLocalUiFields, + type PairedUiState, + type PairingLocalUiField +} from '../../../shared/pairing-local-ui-fields' import type { MemorySnapshot, StatsSummary } from '../../../shared/process-stats-types' import type { Repo } from '../../../shared/repo-types' import type { @@ -2714,11 +2719,7 @@ function createWebUiApi(): NonNullable['ui']> { return { get: async () => { try { - const result = await callRuntimeResult<{ ui: PersistedUIState }>( - 'ui.get', - undefined, - 15_000 - ) + const result = await callRuntimeResult<{ ui: PairedUiState }>('ui.get', undefined, 15_000) const local = readLocalWebUIState() const next = { ...mergeHostWebUIState(local, result.ui), @@ -2743,9 +2744,9 @@ function createWebUiApi(): NonNullable['ui']> { const next = mergeWebUIState(readLocalWebUIState(), updates) writeJson(UI_STORAGE_KEY, next) zoomLevel = next.uiZoomLevel - const { hideWorkspacesFromOtherDevices: _clientLocalWorkspaceFilter, ...hostUpdates } = - updates - void _clientLocalWorkspaceFilter + // Why strip here too when the host also strips: an old host predating that strip would + // otherwise persist this browser's runtime:web-* keys over the desktop profile's order. + const hostUpdates = omitPairingLocalUiFields(updates) try { await callRuntimeResult('ui.set', hostUpdates, 15_000) } catch { @@ -2767,7 +2768,7 @@ function createWebUiApi(): NonNullable['ui']> { }) writeJson(UI_STORAGE_KEY, optimistic) try { - const result = await callRuntimeResult<{ ui: PersistedUIState }>( + const result = await callRuntimeResult<{ ui: PairedUiState }>( 'ui.recordFeatureInteraction', id, 15_000 @@ -4178,14 +4179,18 @@ function mergeWebUIState( } } -function mergeHostWebUIState( - local: PersistedUIState, - incoming: PersistedUIState -): PersistedUIState { - return { - ...mergeWebUIState(local, incoming), - hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true - } +// Why pin instead of trusting the host's strip: an old host still returns these fields, and for a +// profile a pre-fix drag poisoned they echo back the same runtime:web-* keys this browser minted, +// which then match its own repos and beat every newer drag. +function mergeHostWebUIState(local: PersistedUIState, incoming: PairedUiState): PersistedUIState { + // Why `satisfies Record<...>` rather than a `Pick<...>` annotation: every member is optional in + // PersistedUIState, so Pick would accept a literal that silently skipped a newly added member. + const pinned = { + hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true, + manualRepoOrder: local.manualRepoOrder, + workspaceHostOrder: local.workspaceHostOrder + } satisfies Record & Partial + return { ...mergeWebUIState(local, incoming), ...pinned } } function mergeFeatureInteractionState( diff --git a/src/shared/agent-hook-listener-antigravity.test.ts b/src/shared/agent-hook-listener-antigravity.test.ts index da3c813b3b1..9eac0d75593 100644 --- a/src/shared/agent-hook-listener-antigravity.test.ts +++ b/src/shared/agent-hook-listener-antigravity.test.ts @@ -23,6 +23,81 @@ describe('shared agent-hook-listener', () => { vi.unstubAllEnvs() }) + // Why (#15117): the Windows script posts stdin verbatim through curl, so an event that + // fires without a payload arrives as `payload=""` rather than the `{}` the POSIX script + // substitutes. Both transports must still produce the status transition. + it('accepts an Antigravity event whose payload is empty or whitespace-only', () => { + for (const payload of ['', ' ', '\r\n']) { + const started = normalizeHookPayload( + state, + 'antigravity', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + hook_event_name: 'PreInvocation', + payload + }, + 'production' + ) + expect(started?.payload, `payload ${JSON.stringify(payload)}`).toMatchObject({ + state: 'working', + agentType: 'antigravity' + }) + } + }) + + // Why (#15117): this is the shape Windows actually produces — curl omits the form field + // entirely when the agent supplies no stdin, so the key is absent rather than empty. + it('accepts an Antigravity event that carries no payload key at all', () => { + const started = normalizeHookPayload( + state, + 'antigravity', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + hook_event_name: 'PreInvocation' + }, + 'production' + ) + expect(started?.payload).toMatchObject({ state: 'working', agentType: 'antigravity' }) + }) + + it('still drops an Antigravity event whose payload is malformed rather than empty', () => { + expect( + normalizeHookPayload( + state, + 'antigravity', + { + paneKey: PANE_KEY, + hook_event_name: 'PreInvocation', + payload: '{"prompt":' + }, + 'production' + ) + ).toBeNull() + }) + + // Why: the empty-payload allowance is Antigravity's documented policy, not a global one — + // every other source must keep rejecting a body it cannot parse. + it('does not extend the empty-payload allowance to other sources', () => { + for (const payload of ['', undefined]) { + expect( + normalizeHookPayload( + state, + 'gemini', + { + paneKey: PANE_KEY, + hook_event_name: 'BeforeAgent', + ...(payload === undefined ? {} : { payload }) + }, + 'production' + ) + ).toBeNull() + } + }) + it('normalizes Antigravity invocation and tool hooks', () => { const started = normalizeHookPayload( state, diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 609fa0e08d4..76f767d2567 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -4366,8 +4366,16 @@ export function normalizeHookPayload( const paneKey = typeof record.paneKey === 'string' ? record.paneKey.trim() : '' const parsedPaneKey = parsePaneKey(paneKey) const rawPayload = record.payload - const hookPayload = - typeof rawPayload === 'string' + // Why (#15117): some Antigravity events fire with no stdin at all, yet still carry a status + // transition in `hook_event_name`. The POSIX script substitutes `{}` before posting; on + // Windows curl omits the form field entirely when stdin is empty, so the event arrives with + // no `payload` key at all. Accept both shapes rather than dropping the transition. + const antigravityPayloadAbsent = + source === 'antigravity' && + (rawPayload === undefined || (typeof rawPayload === 'string' && rawPayload.trim() === '')) + const hookPayload = antigravityPayloadAbsent + ? {} + : typeof rawPayload === 'string' ? (() => { try { return parseAgentHookJson(rawPayload) diff --git a/src/shared/new-workspace/worktree-create-retry-policy.ts b/src/shared/new-workspace/worktree-create-retry-policy.ts index e45da0d5530..2d3438b0218 100644 --- a/src/shared/new-workspace/worktree-create-retry-policy.ts +++ b/src/shared/new-workspace/worktree-create-retry-policy.ts @@ -65,3 +65,10 @@ export function getGeneratedWorktreeCreateRetryCandidate( export function isRetryableWorktreeCreateConflict(message: string): boolean { return RETRYABLE_WORKTREE_CREATE_CONFLICT_PATTERNS.some((pattern) => pattern.test(message)) } + +// Why: the host drops a *settled* worktree.create dedupe record this long after it +// resolves, so a client replaying an idempotent create must land inside the window +// or the replay misses the record and the host's suffix loop builds a second +// worktree. Shared so the client's replay budget can be asserted against it instead +// of assuming a value that lives in another process. +export const WORKTREE_CREATE_DEDUPE_TTL_MS = 60_000 diff --git a/src/shared/pairing-local-ui-fields.test.ts b/src/shared/pairing-local-ui-fields.test.ts new file mode 100644 index 00000000000..41545abb378 --- /dev/null +++ b/src/shared/pairing-local-ui-fields.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { omitPairingLocalUiFields, PAIRING_LOCAL_UI_FIELDS } from './pairing-local-ui-fields' + +describe('pairing-local UI fields', () => { + // Why a census: the seams that honor this set are spread across the host RPC and the web + // preload, so a field added here without wiring every seam would otherwise ship silently. + it('census: the set is exactly the fields no pairing may exchange', () => { + expect([...PAIRING_LOCAL_UI_FIELDS]).toEqual([ + 'hideWorkspacesFromOtherDevices', + 'manualRepoOrder', + 'workspaceHostOrder' + ]) + }) + + it('omits every member and keeps everything else', () => { + const state = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [{ hostId: 'local' as const, repoId: 'repo-a' }], + workspaceHostOrder: ['local' as const], + sidebarWidth: 280, + activeView: 'tasks' as const + } + + expect(omitPairingLocalUiFields(state)).toEqual({ sidebarWidth: 280, activeView: 'tasks' }) + }) + + it('leaves a payload that carries no member untouched', () => { + const state = { sidebarWidth: 280 } + + expect(omitPairingLocalUiFields(state)).toEqual(state) + }) + + // A member explicitly set to undefined must still be removed, not forwarded as a present key + // that a receiver would read as "cleared". + it('drops a member present with an undefined value', () => { + expect( + Object.keys(omitPairingLocalUiFields({ manualRepoOrder: undefined, sidebarWidth: 280 })) + ).toEqual(['sidebarWidth']) + }) +}) diff --git a/src/shared/pairing-local-ui-fields.ts b/src/shared/pairing-local-ui-fields.ts new file mode 100644 index 00000000000..453a16eb0ee --- /dev/null +++ b/src/shared/pairing-local-ui-fields.ts @@ -0,0 +1,28 @@ +import type { PersistedUIState } from './persisted-ui-state-types' + +// UI state each side of a pairing owns for itself. These fields describe a client's own view — +// which workspaces it hides, what order it puts repos and host sections in — and are keyed to hosts +// only that side knows about, so a value copied across the boundary in either direction overwrites +// the receiver's answer with one computed for somebody else. They must be stripped on the way to a +// host (ui.set) and pinned to the local value on the way back (ui.get). +export const PAIRING_LOCAL_UI_FIELDS = [ + 'hideWorkspacesFromOtherDevices', + 'manualRepoOrder', + 'workspaceHostOrder' +] as const satisfies readonly (keyof PersistedUIState)[] + +export type PairingLocalUiField = (typeof PAIRING_LOCAL_UI_FIELDS)[number] + +// What a paired client actually receives over the UI RPCs, so reading a pairing-local field off a +// host response is a compile error rather than a silent undefined. +export type PairedUiState = Omit + +const PAIRING_LOCAL_UI_FIELD_SET: ReadonlySet = new Set(PAIRING_LOCAL_UI_FIELDS) + +// Accepts any object, not just Partial: the ui.set seam passes the zod-inferred +// update type, whose optionality differs from the persisted shape. +export function omitPairingLocalUiFields(state: T): Omit { + return Object.fromEntries( + Object.entries(state).filter(([key]) => !PAIRING_LOCAL_UI_FIELD_SET.has(key)) + ) as Omit +} diff --git a/src/shared/release-channel.test.ts b/src/shared/release-channel.test.ts index fc7172efbf6..a2d66242d93 100644 --- a/src/shared/release-channel.test.ts +++ b/src/shared/release-channel.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + findInstallerAssetName, formatAdhocVersion, formatDailyVersion, formatHourlyVersion, @@ -7,6 +8,7 @@ import { getReleaseRepoForChannel, getVersionChannel, hasDedicatedReleaseRepo, + hasInstallableArtifactForPlatform, isAdhocVersion, isChannelSupportedOnPlatform, isDailyVersion, @@ -16,8 +18,10 @@ import { parseDailyVersionStamp, parseDevBuildStamp, parseHourlyVersionStamp, + requiresManualDevChannelInstall, sortReleaseBuildsNewestFirst, - type ReleaseBuild + type ReleaseBuild, + type ReleaseChannel } from './release-channel' import { compareAppVersions } from './app-version' @@ -155,17 +159,85 @@ describe('release channel', () => { expect(parseDevBuildStamp('1.4.160')).toBeNull() }) - // Why: all dev workflows are macOS-only, so the channels have no artifact to - // offer elsewhere. Both the picker and the main-process check read this, so a - // regression here would silently re-expose an uninstallable channel. - it('offers the dev channels only on macOS', () => { + // Why: the dev workflows build macOS and Windows but not Linux, so a Linux + // install has no artifact to offer. Both the picker and the main-process check + // read this, so a regression here would silently expose an uninstallable + // channel. + it('offers the dev channels on macOS and Windows but not Linux', () => { for (const channel of ['hourly', 'daily', 'adhoc'] as const) { expect(isChannelSupportedOnPlatform(channel, 'darwin')).toBe(true) + expect(isChannelSupportedOnPlatform(channel, 'win32')).toBe(true) expect(isChannelSupportedOnPlatform(channel, 'linux')).toBe(false) - expect(isChannelSupportedOnPlatform(channel, 'win32')).toBe(false) } }) + // The whole Windows story in one test. electron-updater verifies a downloaded + // installer against the publisherName baked into the *installed* app, so a + // signed stable/RC rejects an unsigned dev installer and no future build can + // fix the copies already out there. Dev builds carry no publisherName, so + // everything leaving a dev channel — including the way back to stable — works. + it('requires a manual install only when entering a dev channel from a signed Windows build', () => { + const manual = (runningChannel: ReleaseChannel | null, targetChannel: ReleaseChannel) => + requiresManualDevChannelInstall({ platform: 'win32', runningChannel, targetChannel }) + + expect(manual('stable', 'adhoc')).toBe(true) + expect(manual('rc', 'hourly')).toBe(true) + expect(manual('stable', 'daily')).toBe(true) + // Unparseable version: assume signed, which sends the user to a download + // that works rather than an update that fails on a signature error. + expect(manual(null, 'adhoc')).toBe(true) + + // Already unsigned — the updater skips verification entirely from here. + expect(manual('adhoc', 'hourly')).toBe(false) + expect(manual('hourly', 'adhoc')).toBe(false) + expect(manual('hourly', 'stable')).toBe(false) + expect(manual('adhoc', 'rc')).toBe(false) + + // Not a dev channel at all. + expect(manual('stable', 'rc')).toBe(false) + expect(manual('rc', 'stable')).toBe(false) + }) + + // Why: macOS dev builds are signed and notarized like a release, so the + // updater installs them over a stable build with no manual step. + it('never requires a manual install off Windows', () => { + for (const platform of ['darwin', 'linux'] as const) { + expect( + requiresManualDevChannelInstall({ + platform, + runningChannel: 'stable', + targetChannel: 'adhoc' + }) + ).toBe(false) + } + }) + + it('detects an installable artifact from the platform update manifest', () => { + expect(hasInstallableArtifactForPlatform('win32', ['latest.yml'])).toBe(true) + expect(hasInstallableArtifactForPlatform('win32', ['latest-mac.yml'])).toBe(false) + expect(hasInstallableArtifactForPlatform('darwin', ['latest-mac.yml'])).toBe(true) + expect(hasInstallableArtifactForPlatform('darwin', ['latest.yml'])).toBe(false) + expect(hasInstallableArtifactForPlatform('linux', ['latest-linux-arm64.yml'])).toBe(true) + expect(hasInstallableArtifactForPlatform('linux', [])).toBe(false) + // An unknown platform must not hide every build; a download-time error is a + // better failure than an empty picker with no explanation. + expect(hasInstallableArtifactForPlatform('freebsd', [])).toBe(true) + }) + + it('finds the directly runnable installer for a platform', () => { + const assets = [ + 'latest.yml', + 'orca-windows-setup.exe', + 'orca-macos-arm64.dmg', + 'orca-linux.AppImage' + ] + expect(findInstallerAssetName('win32', assets)).toBe('orca-windows-setup.exe') + expect(findInstallerAssetName('darwin', assets)).toBe('orca-macos-arm64.dmg') + expect(findInstallerAssetName('linux', assets)).toBe('orca-linux.AppImage') + expect(findInstallerAssetName('win32', ['latest.yml'])).toBeNull() + expect(findInstallerAssetName('freebsd', assets)).toBeNull() + }) + it('offers stable and rc on every platform', () => { for (const platform of ['darwin', 'linux', 'win32'] as const) { expect(isChannelSupportedOnPlatform('stable', platform)).toBe(true) @@ -192,7 +264,8 @@ describe('release channel', () => { channel: 'hourly', name: null, publishedAt: null, - releaseUrl: `https://github.com/stablyai/orca-hourly/releases/tag/v${version}` + releaseUrl: `https://github.com/stablyai/orca-hourly/releases/tag/v${version}`, + installerUrl: null }) const sorted = sortReleaseBuildsNewestFirst([ build('1.4.160-hourly.202607280900'), @@ -237,7 +310,8 @@ describe('release channel', () => { channel: 'adhoc', name: null, publishedAt: null, - releaseUrl: `https://github.com/stablyai/orca-adhoc/releases/tag/v${version}` + releaseUrl: `https://github.com/stablyai/orca-adhoc/releases/tag/v${version}`, + installerUrl: null }) const sorted = sortReleaseBuildsNewestFirst([ build('1.4.160-adhoc.20260728140502'), diff --git a/src/shared/release-channel.ts b/src/shared/release-channel.ts index 74b08f8eea5..056152ffcc9 100644 --- a/src/shared/release-channel.ts +++ b/src/shared/release-channel.ts @@ -53,21 +53,65 @@ export function hasDedicatedReleaseRepo(channel: ReleaseChannel): channel is Ded return (DEDICATED_REPO_CHANNELS as readonly ReleaseChannel[]).includes(channel) } +/** + * The platforms each dev channel actually builds for. Split from + * `hasDedicatedReleaseRepo` because "published to its own repo" and "built for + * this OS" stopped coinciding once the dev workflows gained a Windows job — + * Linux has no dev-channel artifact yet, so it still falls back to stable/RC. + */ +const DEV_CHANNEL_PLATFORMS: Readonly> = { + hourly: ['darwin', 'win32'], + daily: ['darwin', 'win32'], + adhoc: ['darwin', 'win32'] +} + +/** Human-readable list of where dev builds exist, for picker copy that would + * otherwise have to restate the table above and drift from it. */ +export const DEV_CHANNEL_PLATFORM_LABEL = 'macOS and Windows' + /** * Shared so the picker, the main-process check, and any future surface cannot * drift on where a channel is available. - * - * Why this rides on the dev-channel list: all dev channels are produced only by - * macOS workflows, so none has an artifact to offer elsewhere. If one ever - * gains a Windows or Linux job, split the two concepts apart — they coincide - * today, but "published to its own repo" and "built for macOS only" are not the - * same claim. */ export function isChannelSupportedOnPlatform( channel: ReleaseChannel, platform: NodeJS.Platform ): boolean { - return !hasDedicatedReleaseRepo(channel) || platform === 'darwin' + if (!hasDedicatedReleaseRepo(channel)) { + return true + } + return DEV_CHANNEL_PLATFORMS[channel].includes(platform) +} + +/** + * True when the running build cannot reach `targetChannel` through the in-app + * updater and the installer has to be run by hand instead. + * + * Windows dev builds ship unsigned, because SignPath's approval waits are + * budgeted in hours and cannot fit an hourly cadence. electron-updater + * Authenticode-verifies every installer it downloads against the publisherName + * baked into the *installed* app's app-update.yml, so a signed stable or RC + * rejects an unsigned dev installer outright with ERR_UPDATER_INVALID_SIGNATURE + * — and no change to a future build can fix the copies already installed. + * + * Dev builds omit that name, so verification is skipped there and every route + * *out* of a dev channel, including back to stable, still works in-app. Only + * the way in is blocked, and only on Windows. + * + * A null `runningChannel` (an unparseable version) is treated as signed: the + * conservative answer sends someone to a working download rather than to an + * update that fails with a signature error. + */ +export function requiresManualDevChannelInstall(options: { + platform: NodeJS.Platform + runningChannel: ReleaseChannel | null + targetChannel: ReleaseChannel +}): boolean { + const { platform, runningChannel, targetChannel } = options + if (platform !== 'win32' || !hasDedicatedReleaseRepo(targetChannel)) { + return false + } + return runningChannel === null || !hasDedicatedReleaseRepo(runningChannel) } export function getReleaseRepoForChannel(channel: ReleaseChannel): string { @@ -206,6 +250,58 @@ export function getReleaseNotesUrlForVersion(version: string | null): string { : `https://github.com/${repo}/releases` } +/** + * The electron-updater manifest each platform's updater fetches before it can + * install anything. A release without one has nothing that platform can use, + * which is how a build whose Windows leg failed — or one whose Windows leg is + * still running — stays out of that platform's picker instead of becoming a row + * that 404s on download. + */ +const PLATFORM_UPDATE_MANIFESTS: Partial> = { + darwin: ['latest-mac.yml'], + win32: ['latest.yml'], + // Both, because one release carries x64 and arm64 and either makes it installable. + linux: ['latest-linux.yml', 'latest-linux-arm64.yml'] +} + +export function getUpdateManifestNamesForPlatform(platform: NodeJS.Platform): readonly string[] { + return PLATFORM_UPDATE_MANIFESTS[platform] ?? [] +} + +/** True when the release carries an artifact this platform's updater can install. */ +export function hasInstallableArtifactForPlatform( + platform: NodeJS.Platform, + assetNames: readonly string[] +): boolean { + const manifests = getUpdateManifestNamesForPlatform(platform) + // Why permissive on an unknown platform: a filter that hides every build is a + // worse failure than one that offers a build the download step will report on. + if (manifests.length === 0) { + return true + } + return manifests.some((manifest) => assetNames.includes(manifest)) +} + +/** Matches the electron-builder `artifactName` for each platform's directly + * runnable installer — the file someone downloads when the in-app updater + * cannot make the jump. */ +const PLATFORM_INSTALLER_PATTERNS: Partial> = { + darwin: /\.dmg$/i, + win32: /windows-setup\.exe$/i, + linux: /\.AppImage$/i +} + +export function findInstallerAssetName( + platform: NodeJS.Platform, + assetNames: readonly string[] +): string | null { + const pattern = PLATFORM_INSTALLER_PATTERNS[platform] + if (!pattern) { + return null + } + return assetNames.find((name) => pattern.test(name)) ?? null +} + export type ReleaseBuild = { tag: string version: string @@ -215,6 +311,10 @@ export type ReleaseBuild = { name: string | null publishedAt: string | null releaseUrl: string + /** Direct download for this platform's installer, or null when the release + * published none. Drives the Windows bootstrap path, where a signed build + * cannot reach a dev channel through the updater. */ + installerUrl: string | null } /** Newest first, so the picker's first row is always the channel's current tip. */