diff --git a/.github/workflows/aur-publish.yml b/.github/workflows/aur-publish.yml index 8ee64a792..fce4e0f11 100644 --- a/.github/workflows/aur-publish.yml +++ b/.github/workflows/aur-publish.yml @@ -366,10 +366,47 @@ jobs: HostName aur.archlinux.org User aur IdentityFile ~/.ssh/aur_key + # Offer this key and nothing else. Without it ssh walks whatever + # else it can find first, and AUR can refuse on a key that is not + # the one being diagnosed -- so the probe below would be reporting + # on a different identity than the push. It is also the standard + # cause of "permission denied" against AUR with more than one key. + IdentitiesOnly yes StrictHostKeyChecking yes UserKnownHostsFile ~/.ssh/aur_known_hosts SSHCONF + # "Permission denied (publickey)" is the same message for three different + # problems: a secret that is not a readable key, a key nobody registered + # on AUR, and a key registered to an account that does not maintain this + # package. v1.9.1, v1.9.2 and v1.9.5 all died here and none said which. + # + # This narrows it by elimination rather than proving the last one. A + # public key is public, so printing it costs nothing and lets what CI + # presents be compared against what is on the account. `help` is the + # documented way to test AUR auth without pushing: if it answers, the key + # parses AND is registered, so only authorization for ${PACKAGE} is left. + # Its reply also enumerates the commands the account may run, which is + # where to look for a repo-listing one if this needs to go further. + - name: Identify the key AUR sees + if: steps.aur_secret.outputs.configured == 'true' && !inputs.dry_run + continue-on-error: true + env: + PACKAGE: ${{ vars.AUR_PACKAGE_NAME }} + run: | + if ! ssh-keygen -y -f ~/.ssh/aur_key > /tmp/aur_key.pub 2>/tmp/aur_key.err; then + echo "::error::AUR_SSH_PRIVATE_KEY is not a readable private key: $(cat /tmp/aur_key.err)" + exit 0 + fi + echo "Public key this workflow presents:" + cat /tmp/aur_key.pub + ssh-keygen -lf /tmp/aur_key.pub || true + echo "--- what AUR says about it (auth only; NOT write access to ${PACKAGE}) ---" + # Bounded: a diagnostic must never be the thing that hangs a release. + # Exits non-zero by design; the message is the payload. + timeout -k 5 30 ssh -o BatchMode=yes -o ConnectTimeout=10 \ + aur@aur.archlinux.org help 2>&1 || true + - name: Commit and push if: steps.aur_secret.outputs.configured == 'true' && !inputs.dry_run working-directory: aur-repo diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9c789b0a3..71e95f54d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -957,6 +957,12 @@ jobs: publish-msstore: name: Publish to Microsoft Store runs-on: windows-latest + # The workflow-wide token is `contents: write` because publish-release needs + # it. This job only reads: it checks the tree out so the CLI can identify the + # project, downloads a same-run artifact (which uses the runtime token, not + # this one), and talks to Partner Center with its own Entra credentials. + permissions: + contents: read needs: - build-windows-store - publish-release @@ -999,6 +1005,24 @@ jobs: exit 1 fi + # `msstore publish` takes a PROJECT root, not a package: it detects the app + # type there (Electron, via package.json) and only then accepts the built + # package through `--inputDirectory`. This job used to check nothing out, so + # there was no project to point it at. Checkout runs before the artifact + # download on purpose — actions/checkout cleans the workspace, and would + # delete the package if it ran after. + - name: Check out the project + if: steps.store.outputs.enabled == 'true' + uses: actions/checkout@v7 + with: + # Nothing here pushes; the tree is only read so the CLI can see it is + # an Electron project. Left at the default, checkout writes the + # workflow's `contents: write` token into .git/config, where every + # later step can read it — including a third-party CLI action and the + # Store submission. See the job-level `permissions` above: same reason, + # other half. + persist-credentials: false + - name: Download Store package if: steps.store.outputs.enabled == 'true' uses: actions/download-artifact@v4 @@ -1031,7 +1055,14 @@ jobs: throw 'more than one .appx in the artifact — refusing to guess which one to submit' } Write-Output "Submitting $($appx.Name) to product $env:PRODUCT_ID" - msstore publish $appx.FullName -id $env:PRODUCT_ID + # The positional argument is the project root, NOT the package — passing + # the .appx there is what failed the first real run of this job on + # v1.9.5: "We could not find a project publisher for the project at + # ...Openscreen.Setup.1.9.5.appx". The package goes through the option + # below, which takes the DIRECTORY holding it — the CLI's own usage + # says `-i, --inputDirectory`, and rejects the `--inputFile` that + # Microsoft Learn documents. The binary wins. + msstore publish . --inputDirectory $appx.Directory.FullName --appId $env:PRODUCT_ID # Report what happened, not what was configured. Keyed off `enabled` alone # under always(), this claimed "Submitted to the Store" when `msstore diff --git a/.github/workflows/publish-msstore.yml b/.github/workflows/publish-msstore.yml new file mode 100644 index 000000000..6f0d135c9 --- /dev/null +++ b/.github/workflows/publish-msstore.yml @@ -0,0 +1,228 @@ +name: Publish to Microsoft Store (retry) + +# Submits an already-built appx to the Store, without rebuilding anything. +# +# build.yml's own publish-msstore job is the normal path. This exists because +# that job has no usable retry: re-running it replays the workflow definition +# frozen into the original run, so a fix landed afterwards is not picked up, and +# re-dispatching build.yml rebuilds every platform and re-uploads the release +# assets with `--clobber` — rewriting a published release to correct a Store +# submission. v1.9.5 hit exactly that dead end. +# +# So this takes the appx that build already produced and submits it. Nothing is +# rebuilt, no release asset is touched, and the flaky macOS legs are not in the +# way. + +on: + workflow_dispatch: + inputs: + release_tag: + description: "Stable tag whose appx should be submitted (e.g. v1.9.5)" + required: true + type: string + run_id: + description: "Build run to take the appx from. Leave empty to use the most recent build for the tag." + required: false + type: string + dry_run: + description: "Create the submission but leave it in draft (--noCommit). Use this to test without shipping." + required: false + type: boolean + default: false + +# Read-only: this checks the tree out so the CLI can identify the project, and +# reads a build artifact. Partner Center is reached with its own Entra +# credentials, not with this token. +permissions: + contents: read + actions: read + +concurrency: + group: publish-msstore-${{ inputs.release_tag }} + cancel-in-progress: false + +jobs: + submit: + name: Submit ${{ inputs.release_tag }} to the Store + runs-on: windows-latest + # No job-level `if` on MSSTORE_PRODUCT_ID, deliberately. build.yml can afford + # to skip: it is one job among many in an automatic release. This one is + # something a person asked for by hand, and a skipped job is green and + # silent — the exact shape that let Homebrew and WinGet report success while + # publishing nothing for eight releases. Missing configuration is checked + # below and fails loudly instead. + steps: + - name: Validate the tag + id: tag + shell: bash + env: + TAG: ${{ inputs.release_tag }} + run: | + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::error::Expected a stable tag like v1.9.5; got '${TAG}'. RCs must never reach the Store." + exit 1 + fi + echo "tag=$TAG" >> "$GITHUB_OUTPUT" + + # All-or-nothing, as in build.yml: a half-configured publisher is a + # misnamed secret, and failing loudly beats submitting nothing quietly. + - name: Resolve Store configuration + id: store + shell: bash + env: + MSSTORE_PRODUCT_ID: ${{ vars.MSSTORE_PRODUCT_ID }} + AZURE_AD_TENANT_ID: ${{ secrets.AZURE_AD_TENANT_ID }} + AZURE_AD_APPLICATION_CLIENT_ID: ${{ secrets.AZURE_AD_APPLICATION_CLIENT_ID }} + AZURE_AD_APPLICATION_SECRET: ${{ secrets.AZURE_AD_APPLICATION_SECRET }} + SELLER_ID: ${{ secrets.SELLER_ID }} + run: | + # MSSTORE_PRODUCT_ID is in here rather than in a job-level `if` so an + # unconfigured repository gets an error and a Summary line, not a + # silent skip on a run somebody triggered on purpose. + required=(MSSTORE_PRODUCT_ID AZURE_AD_TENANT_ID + AZURE_AD_APPLICATION_CLIENT_ID AZURE_AD_APPLICATION_SECRET + SELLER_ID) + missing=() + for name in "${required[@]}"; do + [[ -n "${!name}" ]] || missing+=("$name") + done + if [[ ${#missing[@]} -ne 0 ]]; then + echo "::error::Store configuration incomplete; missing: ${missing[*]}" + exit 1 + fi + + - name: Check out the tag + uses: actions/checkout@v7 + with: + # `msstore publish` takes a project root and detects the app type + # there; it is not given a package to introspect. Checking out the tag + # rather than the default branch keeps that project state matching the + # appx being submitted. + ref: ${{ steps.tag.outputs.tag }} + # Nothing here pushes, and the token would otherwise sit in .git/config + # for the third-party CLI action below to read. + persist-credentials: false + + - name: Resolve the build run + id: run + shell: bash + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.tag.outputs.tag }} + RUN_ID: ${{ inputs.run_id }} + run: | + if [[ -n "$RUN_ID" ]]; then + # A hand-typed run id is the one input that can quietly ship the + # wrong bytes: nothing downstream re-checks what is inside the + # artifact, so a transposed digit could submit another commit's + # package to the Store under this tag. Confirm it is a build.yml run + # and that it was built from the tag being published. + INFO="$(gh api "repos/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}" \ + --jq '{path: .path, sha: .head_sha}' 2>/dev/null)" || { + echo "::error::Run ${RUN_ID} not found in ${GITHUB_REPOSITORY}." + exit 1 + } + RUN_PATH="$(jq -r .path <<<"$INFO")" + RUN_SHA="$(jq -r .sha <<<"$INFO")" + TAG_SHA="$(git rev-parse HEAD)" + if [[ "$RUN_PATH" != ".github/workflows/build.yml" ]]; then + echo "::error::Run ${RUN_ID} is ${RUN_PATH}, not build.yml." + exit 1 + fi + if [[ "$RUN_SHA" != "$TAG_SHA" ]]; then + echo "::error::Run ${RUN_ID} built ${RUN_SHA}, but ${TAG} is ${TAG_SHA}." + exit 1 + fi + echo "Using run ${RUN_ID}: build.yml at ${RUN_SHA}" + else + # Deliberately not filtered on conclusion: the run this is most + # likely to be retrying is the one whose Store step failed, so + # requiring success would skip exactly the build we want. + RUN_ID="$(gh run list --workflow build.yml --branch "$TAG" \ + --limit 1 --json databaseId --jq '.[0].databaseId')" + if [[ -z "$RUN_ID" || "$RUN_ID" == "null" ]]; then + echo "::error::No build.yml run found for ${TAG}. Pass run_id explicitly." + exit 1 + fi + echo "Resolved the most recent build for ${TAG}: $RUN_ID" + fi + echo "id=$RUN_ID" >> "$GITHUB_OUTPUT" + + - name: Download the Store package + shell: bash + env: + GH_TOKEN: ${{ github.token }} + RUN_ID: ${{ steps.run.outputs.id }} + run: | + mkdir -p artifacts/store + gh run download "$RUN_ID" --name openscreen-windows-store --dir artifacts/store + + - name: Configure Microsoft Store CLI + uses: microsoft/microsoft-store-apppublisher@v1.1 + + - name: Submit the package to the Store + id: submit + shell: pwsh + env: + PRODUCT_ID: ${{ vars.MSSTORE_PRODUCT_ID }} + TENANT_ID: ${{ secrets.AZURE_AD_TENANT_ID }} + SELLER_ID: ${{ secrets.SELLER_ID }} + CLIENT_ID: ${{ secrets.AZURE_AD_APPLICATION_CLIENT_ID }} + CLIENT_SECRET: ${{ secrets.AZURE_AD_APPLICATION_SECRET }} + DRY_RUN: ${{ inputs.dry_run }} + run: | + # Secrets arrive through env, not through expression interpolation + # expanded straight into shell source. + msstore reconfigure ` + --tenantId $env:TENANT_ID ` + --sellerId $env:SELLER_ID ` + --clientId $env:CLIENT_ID ` + --clientSecret $env:CLIENT_SECRET + + $packages = @(Get-ChildItem artifacts/store -Recurse -Include '*.appx','*.msix','*.msixupload') + if ($packages.Count -eq 0) { throw 'no package in the downloaded artifact' } + if ($packages.Count -ne 1) { + throw "expected one package, found $($packages.Count): refusing to guess which to submit" + } + $pkg = $packages[0] + + # The positional argument is the project root, NOT the package: passing + # the package there is what failed v1.9.5 ("could not find a project + # publisher"). The package goes through the option below, which takes + # the DIRECTORY holding it: the CLI's own usage says + # `-i, --inputDirectory`, and rejects the `--inputFile` that Microsoft + # Learn documents. The v1.9.5 dry run is what caught that. + # Not $args: that is a PowerShell automatic variable. + $cmdArgs = @('publish', '.', '--inputDirectory', $pkg.Directory.FullName, '--appId', $env:PRODUCT_ID) + if ($env:DRY_RUN -eq 'true') { + # Leaves the submission in draft instead of sending it to + # certification: the only way to test this path without shipping. + $cmdArgs += '--noCommit' + Write-Output "DRY RUN: submitting $($pkg.Name) as a draft only" + } else { + Write-Output "Submitting $($pkg.Name) to product $env:PRODUCT_ID" + } + msstore @cmdArgs + + # Report what happened, not what was configured — the mistake that let + # v1.9.5's failed submission read as a success (see 1617c930). + - name: Summary + if: always() + shell: bash + env: + SUBMIT: ${{ steps.submit.outcome }} + TAG: ${{ inputs.release_tag }} + DRY_RUN: ${{ inputs.dry_run }} + run: | + case "$SUBMIT" in + success) + if [[ "$DRY_RUN" == "true" ]]; then + echo "Draft submission created for ${TAG}; nothing was sent to certification." >> "$GITHUB_STEP_SUMMARY" + else + echo "Submitted ${TAG} to the Store. Certification still has to pass before it goes live." >> "$GITHUB_STEP_SUMMARY" + fi + ;; + *) + echo "Store submission for ${TAG} did NOT happen (submit step: ${SUBMIT:-did not run}). The appx is unchanged; upload it by hand if this keeps failing." >> "$GITHUB_STEP_SUMMARY" + ;; + esac diff --git a/AGENTS.md b/AGENTS.md index d7b2ec8ab..91483965f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,7 +14,7 @@ OpenScreen is a free, open-source screen recorder and video editor (Electron + R - Format: `npm run format` (Biome, tabs, double quotes, 100-col) - i18n check: `npm run i18n:check` (validates the 13 locale files) -**Use npm, not bun/pnpm/yarn/Deno.** Not a style preference. The native Swift (macOS) and C++ (Windows) capture helpers are rebuilt against Electron's ABI by electron-builder + `@electron/rebuild`, which resolve the tree through `package-lock.json`. Another package manager writes a different lockfile, so that rebuild breaks. `packageManager` + `engines` in `package.json` pin the versions; CI installs with `npm ci`. +**Use npm, not bun/pnpm/yarn/Deno.** Not a style preference. Node native modules are rebuilt against Electron's ABI by electron-builder + `@electron/rebuild`, which resolve the tree through `package-lock.json`. Another package manager writes a different lockfile, so that rebuild breaks. `packageManager` + `engines` in `package.json` pin the versions; CI installs with `npm ci`. Note what this does *not* cover: the standalone Swift (macOS) and C++ (Windows) capture helpers are separate executables, built by `npm run build:native:` and only *copied* into the package as `extraResources` — `build:win` even passes `--config.npmRebuild=false`. Nothing in a normal build compiles them. ## Development principles @@ -74,29 +74,47 @@ every edit is the main way an agent turns a 5-minute task into a 30-minute one, every Windows and macOS machine — `electron/recording/webm-seek-index.test.ts` is the worked example. - E2E tests are in `tests/e2e/` (Playwright). Some specs are platform-specific (e.g. `windows-native-checklist.spec.ts`). +- **Playwright is not the end of the e2e story.** It drives the app through CDP, which cannot reach real capture, a real webcam, the tray, or the click-through HUD. Everything those miss is covered by a manual pass driven with computer-use — see [Desktop E2E testing with computer-use](#desktop-e2e-testing-with-computer-use) below, which is required for native changes and before promoting a release candidate. - Add a test for every new behavior in the same package as the code under test. - All tests must pass before opening a PR. CI runs `npm run test` on every PR. +- **Which kind of test to write, and where: [`technical-documentation/testing/writing-tests.md`](technical-documentation/testing/writing-tests.md).** ## Desktop E2E testing with computer-use -Unit/browser tests can't exercise real capture (native screen recording, a physical webcam, the tray). To verify a recording/editor feature end to end, drive the actual Electron app with the **computer-use** MCP (screenshot + click/type on the desktop). This is the required "manual smoke test on real Windows/macOS" for native changes. +**Computer-use is how the manual end-to-end pass is driven — all of it, not only the native parts.** Real capture is what forces it (native screen recording, a physical webcam, the tray: no unit or browser test reaches those), but once the app is up you drive everything the same way — editor, timeline, regions, transcript, export, settings, persistence. Screenshot and click/type on the desktop, through the **computer-use** MCP, against the actual Electron app. This is the required "manual smoke test on real Windows/macOS" for native changes, and the only mode in which the checklist below means anything. + +This section is the *mechanics*. **What to actually run is [`technical-documentation/testing/manual-e2e-checklist.md`](technical-documentation/testing/manual-e2e-checklist.md)** — the capture-to-export pass, per-platform sections, and a results log to append to. Run it before promoting a release candidate and after any change to native capture, preview or export. For cursor work specifically, [`native-cursor-diagnostics.md`](technical-documentation/testing/native-cursor-diagnostics.md) gets you sidecars and reports without a full record-edit-export cycle. The checklist links back here for the mechanics below; the pairing only works if you know both halves exist. **Launch the app** - Normal: `npm run dev` — Vite serves the renderer and `vite-plugin-electron` opens the Electron window. The main process logs `Global shortcut registered: CommandOrControl+Shift+O` when ready (Ctrl/Cmd+Shift+O toggles the HUD). +- **Set `OPENSCREEN_DISABLE_CONTENT_PROTECTION=1` in the environment you launch from, or the HUD is invisible in every screenshot you take.** It is a module-scope constant (`electron/windows.ts:20`), read once as the main process loads, so it cannot be turned on afterwards — you relaunch or you work blind. The main process prints `[content-protection] OFF for the HUD window` when it took effect; if that line is missing, stop and relaunch rather than hunting a HUD you will never see. What it does and when to unset it: the HUD notes below. - The app is single-instance through `app.requestSingleInstanceLock()`, which keys on the `userData` path. If a leftover Electron process still holds it, a new launch quits silently (exit 0, no window) — kill leftover `electron` processes before relaunching. The lock is held by the OS and dies with the process, so there is nothing to clean up on disk. A dev build and the installed `Openscreen` resolve different `userData` paths and can run side by side. - **From a git worktree** (no `node_modules`/native binaries): junction/symlink `node_modules` from the main checkout (deps are usually identical — check `package-lock.json`), and copy the prebuilt native capture binaries from `electron/native/bin//` (gitignored — rebuilding needs the full VS/Xcode toolchain). Then `npm run dev` works normally. +- **Those binaries are frozen at whenever someone last built them, and nothing warns you.** They are not rebuilt by `npm run dev` or `npm run build`, so a helper older than the native change you came to test will run happily and silently exercise the old code path — the recording succeeds, and the thing you wanted to see is simply absent. Before trusting any native result, date the binary against the commit and search it for a string the change introduced — from the repo root: + + ```powershell + # the string the change introduced — absent from a stale helper + findstr /M /C:"fragmented-mp4" electron\native\bin\win32-x64\wgc-capture.exe + # the control — present in every helper, stale or not + findstr /M /C:"encoder-selection" electron\native\bin\win32-x64\wgc-capture.exe + ``` + + Run **both**. Only the second tells "the binary is stale" apart from "my search is broken", and that distinction is not hypothetical: `findstr` handles binaries and ships with Windows, but Git Bash has **no `strings`**, so `strings … | grep` there returns nothing and reads as a confident *absent* for every binary you point it at. Measured against the two helpers this section is about — stale: no match, then HIT; current: HIT, HIT. A control that does not hit means you learned nothing about the binary. If it is stale, rebuild it with `npm run build:native:win` (or `:mac` / `:linux`) — that is the only thing that compiles a helper. Without the toolchain, test the CI-built artifact instead; a dev build cannot answer the question. +- **And it is the whole directory, not the one binary you came for.** `electron/native/bin//` also holds the compositor addon, the cursor sampler, the ffmpeg DLLs it dlopens, and the STT binaries — each frozen independently at whenever someone last ran a build. Refreshing only the helper leaves a mismatched set, and a mismatched set fails like a product bug: an export died on `open_input: -22 (Invalid argument)` from `compositor.exportMulti` purely because the addon was four days older than the av\* DLLs it was built against, while `ffmpeg` on the command line opened the very same file without complaint. If you are borrowing binaries from an installed build, copy the **entire** directory and diff it by hash afterwards — the last check turned up sixteen differing files and two missing outright. **Granting access** - `request_access` resolves names against installed apps. A **dev build runs as `electron.exe`** (or `Electron.app`), *not* the installed `Openscreen` — grant **`electron.exe`** or the dev window stays masked in screenshots. Non-allowlisted windows are masked (solid rectangles); the screenshot note lists their process names to add. +- **Start the app before asking for it.** `electron.exe` is not an installed app, so the resolver only finds it once the process exists *and* owns a window; ask any earlier and the call fails with `doesn't match any installed or running application` — and one unresolvable name short-circuits the whole request, including the names that would have resolved. Granting `Openscreen` instead is not a workaround: it resolves to `…\programs\openscreen\openscreen.exe`, so the dev window stays masked while the grant reports success. **The HUD widget** (recording controller) - **It is invisible in screenshots by default.** The HUD (and the Notes window) call `setContentProtection(true)` so the recording controls never end up baked into a recording — the same `SetWindowDisplayAffinity` that WGC honours also hides them from *your* screenshots. The window is there, and clicks land, but you are aiming blind at a rectangle you cannot see. Set **`OPENSCREEN_DISABLE_CONTENT_PROTECTION=1`** in the app's environment to turn it off for a session; every skipped window logs a warning. Unset it before recording anything real, or the HUD ends up in the video. - **On macOS 26+ content protection is auto-disabled, so the HUD *is* visible and screenshottable with no flag.** That OS never displays a content-protected window at all — not just absent from captures, but never painted, leaving a tray icon, a live renderer and nothing on screen (confirmed on macOS 26.5 / Electron 41.2.1). `applyContentProtection` therefore skips the call there and logs a warning per window; the trade-off is that the HUD can appear in recordings on that OS until the ScreenCaptureKit helper excludes our own windows via `SCContentFilter(excludingWindows:)`, which it currently passes as `[]`. `OPENSCREEN_FORCE_CONTENT_PROTECTION=1` re-enables it to re-test against a future Electron. - The HUD is what opens the editor (clapper icon, tooltip *Open Studio*), so without that flag a whole slice of the app is unreachable from automation: killing the app to redeploy a native addon leaves you unable to reopen a project. -- Frameless, transparent, always-on-top, `skipTaskbar`, centered at the **bottom of the primary display** (`createHudOverlayWindow`, 600×160). It is **click-through** (`setIgnoreMouseEvents(true, { forward: true })`): moving the real cursor over an interactive control makes that region clickable and shows its tooltip, so `mouse_move` → screenshot → `left_click` works; a blind click on empty HUD area passes through to the desktop. +- Frameless, transparent, always-on-top, `skipTaskbar`, centered at the **bottom of the primary display** (`createHudOverlayWindow`, 820×560 at construction, then resized to fit its content — measured 904×698 with the bar at the bottom and mostly empty reserve above it). It is **click-through** (`setIgnoreMouseEvents(ignore)`): moving the real cursor over an interactive control makes that region clickable and shows its tooltip, so `mouse_move` → screenshot → `left_click` works; a blind click on empty HUD area passes through to the desktop. +- **Only a real OS mouse move reaches the HUD — on macOS as much as on Windows.** While the window is input-transparent Chromium delivers it no pointer events at all, so the main process samples the OS cursor instead: the `hud-overlay-cursor` poll in `electron/windows.ts` reads `screen.getCursorScreenPoint()` while the HUD is click-through and pushes the window-relative point to the renderer, which hit-tests it with `elementFromPoint(…).closest("[data-hud-interactive='true']")`. One path, both platforms — there is no platform branch. **Linux is the exception** (`!enabled && !isLinuxHud` in `LaunchWindow.tsx`, where the call is a no-op), so it is the one platform where a blind click on the HUD simply lands. What the poll keys off is the OS cursor's position *relative to the window*, so a resize or re-anchor that slides the bar under a motionless pointer produces a fresh sample too. What it can never key off is synthesised input: Playwright's `.click()`, `javascript_tool`-dispatched pointer events and everything like them move no pointer at all, so they never put one on a control. They arrive *below* the OS hit-test, fire the DOM handler, and look like they worked — while the click-through path was never exercised at all. `tests/e2e/windows-native-checklist.spec.ts` does click HUD test IDs and stays green for exactly that reason; it proves renderer wiring, not reachability, and a macOS spec written the same way would prove no more. Use computer-use (`mouse_move` → `left_click`), and never conclude from a passing injected click that a user could have clicked it. (Until #385 the lift was Electron's `{ forward: true }` — a global `WH_MOUSE_LL` hook on Windows — which Windows can revoke without telling the app, leaving the HUD painted and permanently dead. The poll replaced it. The rule for you is unchanged, because both mechanisms key off the real cursor.) - Control row (left→right): layout preset, **source** button (`Screen`/`Window` → label becomes the picked source), system-audio toggle, mic toggle, **webcam toggle** (shows the detected camera name), cursor-highlight toggle, **record**, notes, open-editor, language, minimize, close. The record button is disabled until a source is chosen (tooltip: "Please select a source to record"). **The tray icon** (bottom-right notification area) diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index 9e1fb7d0d..789eb7e59 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -395,6 +395,9 @@ interface Window { hudOverlayHide: () => void; hudOverlayClose: () => void; setHudOverlayIgnoreMouseEvents: (ignore: boolean) => void; + /** Window-relative cursor position, pushed while the HUD is click-through and + * therefore receiving no pointer events of its own. Returns an unsubscribe. */ + onHudOverlayCursor: (callback: (x: number, y: number) => void) => () => void; /** Pins the overlay's current position as the origin for `dragHudOverlayTo`. */ beginHudOverlayDrag: () => void; /** Total pointer travel since `beginHudOverlayDrag`, not a per-frame delta. */ diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 14262b11a..d4c094869 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -73,6 +73,8 @@ import { findPipeWireCursorHelperPath } from "../native-bridge/cursor/recording/ import type { CursorRecordingSession } from "../native-bridge/cursor/recording/session"; import { toHelperRect } from "../native-bridge/helperCoordinates"; import { + isSalvageableFragmentedCapture, + NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES, terminateNativeWindowsCapture, waitForNativeWindowsCaptureStop, } from "../recording/nativeWindowsCaptureStop"; @@ -538,6 +540,12 @@ let nativeWindowsCursorRecordingStartMs = 0; let nativeWindowsPauseStartedAtMs: number | null = null; let nativeWindowsPauseRanges: Array<{ startMs: number; endMs: number }> = []; let nativeWindowsIsPaused = false; +/** + * The MP4 flavour the helper reported for THIS run, or null if it never said. + * Read at stop, not for reporting: it is what decides whether a capture that + * failed to finalize still left a playable file behind. + */ +let nativeWindowsCaptureContainer: string | null = null; /** Cuts a surviving helper's output loose so it cannot pollute the next recording. */ let nativeWindowsCaptureDrainCleanup: (() => void) | null = null; @@ -558,14 +566,17 @@ function resetNativeWindowsCaptureState() { nativeWindowsPauseStartedAtMs = null; nativeWindowsPauseRanges = []; nativeWindowsIsPaused = false; + nativeWindowsCaptureContainer = null; } -/** - * An MP4 the helper never indexed is a few bytes of header at most. Anything - * larger might be a real recording, and deleting one of those to tidy up after - * a failed stop is a far worse outcome than leaving a stray file behind. - */ -const NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES = 64 * 1024; +/** Reads the file, then defers the judgement to the tested predicate. */ +async function salvageNativeWindowsFragmentedCapture(screenVideoPath: string | null) { + if (!screenVideoPath) { + return false; + } + const stats = await fs.stat(screenVideoPath).catch(() => null); + return isSalvageableFragmentedCapture(nativeWindowsCaptureContainer, stats?.size ?? null); +} /** * Best-effort removal of the files a failed or discarded native Windows capture @@ -1344,6 +1355,13 @@ function readNativeWindowsEncoderSelection(output: string) { try { return JSON.parse(lastLine) as { video?: string; + // Which MP4 flavour the helper actually wrote, `fragmented-mp4` or + // `mp4`. It reports this because the fragmented sink degrades to the + // plain one rather than failing a recording, so the flavour is a + // per-run outcome and not a property of the version. This is the only + // thing that can answer "was this file supposed to survive a kill?", + // which is what `salvageNativeWindowsFragmentedCapture` asks. + container?: string; preferSoftwareEncoder?: boolean; }; } catch { @@ -2433,6 +2451,9 @@ export function registerIpcHandlers( : 0; const webcamFormat = readNativeWindowsWebcamFormat(nativeWindowsCaptureOutput); const encoderSelection = readNativeWindowsEncoderSelection(nativeWindowsCaptureOutput); + // Captured now because stop may have no helper left to ask. A helper + // killed mid-recording is exactly the case where this matters most. + nativeWindowsCaptureContainer = encoderSelection?.container ?? null; console.info("[native-wgc] capture started", { captureStartedAtMs, cursorOffsetMs: nativeWindowsCursorOffsetMs, @@ -2742,6 +2763,11 @@ export function registerIpcHandlers( } } + // Set when the helper failed its stop handshake but left a playable + // fragmented file. Reported so a bug report can tell a clean stop from a + // recovered one; the user-facing path is deliberately identical. + let recovered = false; + try { completeNativeWindowsCursorPauseRange(); const stopPromise = waitForNativeWindowsCaptureStop({ @@ -2763,35 +2789,62 @@ export function registerIpcHandlers( if (!stopResult.exited) { detachNativeWindowsCaptureOutputDrain(); } - await stopCursorRecording(); - // Same as the discard path. `startCursorRecording` clears this on - // the next recording anyway, so this is not what keeps the samples - // from being written next to someone else's video -- it just stops - // a lost take's telemetry from sitting in memory until then. - pendingCursorRecordingData = null; - // The helper never announced a finalized file, so what is on disk - // is almost certainly an unindexed stub, and leaving those behind - // just accumulates unplayable recordings the user cannot explain. - // Almost: size-gate it, because throwing away a recording to tidy - // up after a failed stop is the worse mistake of the two. - await removeNativeWindowsCaptureOutputs(preferredPath, preferredWebcamPath, { - onlyIfUnusable: true, - }); - // The helper log goes to console/diagnostics above, not into this - // string: it ends up in a toast, and pasting an entire capture log - // into the HUD tells the user nothing they can act on. - return { - success: false, - reason: stopResult.reason, - error: - stopResult.reason === "stop-timeout" - ? "Timed out waiting for native Windows capture to stop. The recording could not be saved." - : stopResult.message.split(/\r?\n/).filter(Boolean).at(-1) || - "Native Windows capture failed.", - }; + + // A failed stop stopped meaning a lost take when the helper started + // writing fragmented MP4. The file on disk is already playable, so + // the only thing standing between the user and their recording is + // this function deciding to throw it away and say so. Fall through + // into the normal save path instead: same manifest, same media + // links, same editor. From the user's side it simply worked, minus + // at most the last incomplete fragment. + // + // Only once the helper is actually dead. `exited: false` means it + // survived even the forced kill -- stuck somewhere `TerminateProcess` + // could not reach -- and on Windows such a process still holds the + // MP4 open and may still be appending to it. Handing that file to + // the editor trades an honest failure for a sharing violation on a + // file that is still moving, so a wedged helper keeps the old answer. + if (stopResult.exited && (await salvageNativeWindowsFragmentedCapture(preferredPath))) { + console.warn("[native-wgc] stop failed but the fragmented output is playable", { + reason: stopResult.reason, + path: preferredPath, + }); + recovered = true; + } else { + await stopCursorRecording(); + // Same as the discard path. `startCursorRecording` clears this on + // the next recording anyway, so this is not what keeps the samples + // from being written next to someone else's video -- it just stops + // a lost take's telemetry from sitting in memory until then. + pendingCursorRecordingData = null; + // Reaching here means the container was the plain one, whose only + // index is written by the `Finalize()` this stop never reached, so + // what is on disk really is an unindexed stub and leaving those + // behind just accumulates unplayable recordings the user cannot + // explain. Size-gate it anyway: throwing away a recording to tidy + // up after a failed stop is the worse mistake of the two, and the + // gate is the same one the salvage check above uses. + await removeNativeWindowsCaptureOutputs(preferredPath, preferredWebcamPath, { + onlyIfUnusable: true, + }); + // The helper log goes to console/diagnostics above, not into this + // string: it ends up in a toast, and pasting an entire capture log + // into the HUD tells the user nothing they can act on. + return { + success: false, + reason: stopResult.reason, + error: + stopResult.reason === "stop-timeout" + ? "Timed out waiting for native Windows capture to stop. The recording could not be saved." + : stopResult.message.split(/\r?\n/).filter(Boolean).at(-1) || + "Native Windows capture failed.", + }; + } } - const screenVideoPath = stopResult.screenVideoPath || preferredPath; + // Only a successful stop names the file; the salvage path above falls + // through with `ok: false` and nothing but the path we asked for. + const screenVideoPath = (stopResult.ok ? stopResult.screenVideoPath : null) || preferredPath; if (!screenVideoPath) { throw new Error("Native Windows capture did not return an output path."); } @@ -2833,7 +2886,10 @@ export function registerIpcHandlers( success: true, path: screenVideoPath, session, - message: "Native Windows recording session stored successfully", + recovered, + message: recovered + ? "Native Windows recording recovered from a failed stop" + : "Native Windows recording session stored successfully", }; } catch (error) { console.error("Failed to stop native Windows recording:", error); diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 42e764e3b..c5e191056 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -141,6 +141,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { private var audioMixer: AudioTrackMixer? private var didStartWriting = false private var didEmitRecordingStarted = false + private var didReportWriterFailure = false private var isStopping = false private var isPaused = false private var pauseStartedAt: CMTime? @@ -309,7 +310,8 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { } if videoInput.isReadyForMoreMediaData { - if videoInput.append(sampleBuffer), !didEmitRecordingStarted { + let appended = videoInput.append(sampleBuffer) + if appended, !didEmitRecordingStarted { didEmitRecordingStarted = true emit([ "event": "recording-started", @@ -318,10 +320,40 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { "height": outputHeight, "captureBounds": captureBoundsPayload(), ]) + } else if !appended { + reportWriterFailure("video append") } } } + /// A failed AVAssetWriter keeps accepting appends and keeps answering false, so + /// a recorder that discards that Bool records nothing while the HUD counts on. + /// That is how a two-minute take was already lost by its fourth second and only + /// said so at finishWriting(). The Windows helper checks every WriteSample + /// HRESULT and escalates; this is the macOS half of the same contract -- report + /// once, at the append that actually failed, carrying the live writer.error. + /// + /// Deliberately not the code finishWriter() emits, and the difference is load + /// bearing. That one is the terminal result of stopping, and the Electron side + /// settles its stop on exactly one of `recording-stopped` or `writer-failed`. + /// Give both sites the same code behind this one-shot guard and a writer that + /// died mid-capture emits nothing at all at stop, so the stop promise never + /// settles and every failure becomes the "Saving..." hang instead of an error. + /// This event answers "when did the writer die"; that one answers "did stopping + /// work". Two questions, two codes. + private func reportWriterFailure(_ stage: String) { + guard !didReportWriterFailure, let writer else { + return + } + didReportWriterFailure = true + emitError( + code: "writer-failed-during-capture", + message: "\(stage): " + + (writer.error.map { "\($0)" } + ?? "AVAssetWriter status \(writer.status.rawValue)"), + ) + } + private func ensureRequestedPermissions() throws { if !CGPreflightScreenCaptureAccess() { let granted = CGRequestScreenCaptureAccess() @@ -456,6 +488,33 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { AVVideoCompressionPropertiesKey: [ AVVideoAverageBitRateKey: request.video.bitrate ?? 18_000_000, AVVideoExpectedSourceFrameRateKey: request.video.fps, + // Without this the encoder defaults to B-frames, and a reordered + // stream needs a composition offset per sample. AVAssetWriter emits + // those in a version 0 `trun`, where ISO/IEC 14496-12 8.8.8.2 defines + // the field as UNSIGNED -- so a negative offset goes out as + // 0xFFFFFFF6 and the fragment writer refuses the fragment it is + // about to emit. That refusal is -11800 / -16341, raised from the + // single site in MediaToolbox that writes moof/traf/trun, which is + // why it appears if and only if movieFragmentInterval is set and + // lands exactly on a fragment boundary. + // + // Turning reordering off makes every offset zero and PTS == DTS, so + // the fragment stays representable. A screen recorder gives up + // nothing for it: B-frames buy compression on lookahead-friendly + // content and cost encode latency, which is the wrong trade for + // real-time capture. + // + // Measured on macOS 26.5 / M1, 1080p with system audio. How reliably + // the bug bites scales with append rate, so quote the rate with the + // result: at ~57 fps, the rate the app actually drives, reordering + // on dies at 13.0s while reordering off stops clean at 31.6s; at + // 30 fps it is intermittent, dying at 1.0s and 2.0s but once + // surviving 22.2s. That intermittency is why the byte-level evidence + // leads here and the run counts only corroborate: the offsets are + // out of spec in every fragmented file whether or not that + // particular run happened to die. Reordering off is 3/3 clean across + // both rates, and a SIGKILL at 25s still leaves 27 readable `moof`. + AVVideoAllowFrameReorderingKey: false, ], ] let input = AVAssetWriterInput(mediaType: .video, outputSettings: settings) diff --git a/electron/preload.ts b/electron/preload.ts index 8e018ed8e..66fe2af66 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -80,6 +80,11 @@ contextBridge.exposeInMainWorld("electronAPI", { setHudOverlayIgnoreMouseEvents: (ignore: boolean) => { ipcRenderer.send("hud-overlay-ignore-mouse-events", ignore); }, + onHudOverlayCursor: (callback: (x: number, y: number) => void) => { + const listener = (_e: Electron.IpcRendererEvent, x: number, y: number) => callback(x, y); + ipcRenderer.on("hud-overlay-cursor", listener); + return () => ipcRenderer.removeListener("hud-overlay-cursor", listener); + }, beginHudOverlayDrag: () => { ipcRenderer.send("hud-overlay-drag-start"); }, diff --git a/electron/recording/nativeWindowsCaptureStop.test.ts b/electron/recording/nativeWindowsCaptureStop.test.ts index 7ba34317c..900df9e16 100644 --- a/electron/recording/nativeWindowsCaptureStop.test.ts +++ b/electron/recording/nativeWindowsCaptureStop.test.ts @@ -3,6 +3,8 @@ import { EventEmitter } from "node:events"; import { PassThrough, Writable } from "node:stream"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + isSalvageableFragmentedCapture, + NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES, readStoppedPath, terminateNativeWindowsCapture, waitForNativeWindowsCaptureStop, @@ -77,6 +79,45 @@ describe("readStoppedPath", () => { }); }); +describe("isSalvageableFragmentedCapture", () => { + const big = NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES * 8; + + // The whole point of the fragmented container, and the case that used to be + // deleted-or-disowned while the file on disk played perfectly (#252). + it("keeps a fragmented capture whose stop never finalized", () => { + expect(isSalvageableFragmentedCapture("fragmented-mp4", big)).toBe(true); + }); + + // The ablation. Same size, same failed stop, no index anywhere in the file: + // this one really is lost, and saying otherwise would open an empty editor. + it("does not pretend a plain MP4 survived the same failure", () => { + expect(isSalvageableFragmentedCapture("mp4", big)).toBe(false); + }); + + it("rejects a fragmented file too small to hold a complete fragment", () => { + expect( + isSalvageableFragmentedCapture("fragmented-mp4", NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES - 1), + ).toBe(false); + }); + + it("takes the floor itself as salvageable", () => { + expect( + isSalvageableFragmentedCapture("fragmented-mp4", NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES), + ).toBe(true); + }); + + // A helper predating the fragmented sink reports no container at all. Absent + // is not fragmented -- guessing here would resurrect the total loss. + it("refuses to guess when the helper never reported a container", () => { + expect(isSalvageableFragmentedCapture(null, big)).toBe(false); + expect(isSalvageableFragmentedCapture(undefined, big)).toBe(false); + }); + + it("rejects a file that is not there at all", () => { + expect(isSalvageableFragmentedCapture("fragmented-mp4", null)).toBe(false); + }); +}); + describe("waitForNativeWindowsCaptureStop", () => { it("resolves with the path the helper reported", async () => { let output = "Recording started\n"; diff --git a/electron/recording/nativeWindowsCaptureStop.ts b/electron/recording/nativeWindowsCaptureStop.ts index 95da2c19a..badec52d0 100644 --- a/electron/recording/nativeWindowsCaptureStop.ts +++ b/electron/recording/nativeWindowsCaptureStop.ts @@ -33,6 +33,43 @@ export const NATIVE_WINDOWS_CAPTURE_STOP_TIMEOUT_MS = 60_000; /** How long a killed helper gets to actually die before we escalate. */ const NATIVE_WINDOWS_CAPTURE_KILL_GRACE_MS = 2_000; +/** What `mf_encoder.h`'s `kContainerFormatFragmentedMp4` puts on the wire. */ +export const NATIVE_WINDOWS_FRAGMENTED_CONTAINER = "fragmented-mp4"; + +/** + * An MP4 the helper never indexed is a few bytes of header at most. Anything + * larger might be a real recording, and deleting one of those to tidy up after + * a failed stop is a far worse outcome than leaving a stray file behind. + */ +export const NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES = 64 * 1024; + +/** + * Did a stop that failed its handshake still leave a recording worth opening? + * + * Only the fragmented container can. A plain MP4 writes its one index in + * `Finalize()`, so a helper that never reached it leaves bytes no demuxer can + * read — the total loss issues #252 / #292 / #327 reported. A fragmented one + * writes `moov` up front and a self-describing `moof`+`mdat` pair about every + * second, so the same file plays up to the last complete fragment with nothing + * else needed. Which one a run used is not a property of the version: the + * fragmented sink degrades to the plain one rather than failing a recording, + * which is exactly why the helper reports the flavour it settled on. + * + * The size floor is shared with the cleanup that deletes unusable leftovers, so + * the two agree by construction: nothing is recovered that the tidy-up would + * have judged a stub, and nothing is deleted that this would have called a + * recording. + */ +export function isSalvageableFragmentedCapture( + container: string | null | undefined, + sizeBytes: number | null, +): boolean { + if (container !== NATIVE_WINDOWS_FRAGMENTED_CONTAINER) { + return false; + } + return sizeBytes !== null && sizeBytes >= NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES; +} + const RECORDING_STOPPED_PATTERN = /Recording stopped\. Output path: (.+)/; const STOP_TIMEOUT_EVENT_PATTERN = /"event":"stop-timeout"[^\n]*"step":"([^"]+)"/; diff --git a/electron/windows.ts b/electron/windows.ts index 0b19d5b98..4b5ceb7fe 100644 --- a/electron/windows.ts +++ b/electron/windows.ts @@ -104,9 +104,75 @@ ipcMain.on("hud-overlay-hide", () => { } }); +// The cursor, sampled here and pushed to the renderer, because while the HUD is +// click-through nothing else can tell it where the pointer is. +// +// Chromium delivers no pointer event of any kind to a window it has made +// input-transparent — including the pointermove the renderer needs to ask for input +// back. Electron's `{ forward: true }` covered that with a global WH_MOUSE_LL hook +// that re-posts WM_MOUSEMOVE, and that hook was the ONLY route out: its install is +// unchecked (SetWindowsHookEx's return value is discarded), latched behind Electron's +// `forwarding_mouse_messages_` so it re-arms only after a setIgnoreMouseEvents(false) +// the renderer can no longer request, and Windows silently revokes any low-level hook +// whose callback overruns LowLevelHooksTimeout — "there is no way for the application +// to know whether the hook is removed". One hook that never installs or quietly dies +// and the HUD is painted, inert, forever, with the tray icon as the only way to quit +// the app. That is issue #266, and issue #385 after it: #266 was closed by moving +// *when* the hook is installed, which left the trapdoor exactly where it was. +// +// So the escape no longer runs on anything Windows can take away. getCursorScreenPoint +// is a plain positional read the main process can always make, the poll exists only +// while the window is click-through — the state it is there to escape — and the +// renderer re-derives the answer from scratch on every tick, so no dropped message, +// dead hook or stale flag can strand it. +const HUD_CURSOR_POLL_MS = 32; +let hudCursorPoll: ReturnType | null = null; +let hudLastPoint: { x: number; y: number } | null = null; + +function stopHudCursorPoll() { + if (hudCursorPoll) clearInterval(hudCursorPoll); + hudCursorPoll = null; + hudLastPoint = null; +} + +function pollHudCursor() { + const win = hudOverlayWindow; + if (!win || win.isDestroyed() || !win.isVisible() || win.isMinimized()) return; + + // getBounds() and getCursorScreenPoint() are both in DIP, and so is a renderer CSS + // pixel (the HUD is frameless, so the client area is the whole window). + const bounds = win.getBounds(); + const cursor = screen.getCursorScreenPoint(); + const x = cursor.x - bounds.x; + const y = cursor.y - bounds.y; + if (x < 0 || y < 0 || x >= bounds.width || y >= bounds.height) return; + + // Deduped on the WINDOW-RELATIVE point, not the cursor: "hud-overlay-set-size" + // re-anchors the window on every content change, so the bar can arrive under a + // cursor that never moved — and that changes the answer just as much. + if (hudLastPoint && hudLastPoint.x === x && hudLastPoint.y === y) return; + hudLastPoint = { x, y }; + + win.webContents.send("hud-overlay-cursor", x, y); +} + ipcMain.on("hud-overlay-ignore-mouse-events", (_event, ignore: boolean) => { - if (hudOverlayWindow && !hudOverlayWindow.isDestroyed()) { - hudOverlayWindow.setIgnoreMouseEvents(ignore, { forward: true }); + if (!hudOverlayWindow || hudOverlayWindow.isDestroyed()) { + return; + } + + // No `forward`: the poll above replaces it, and leaving it on would keep the app + // depending on a hook it cannot check for a transition it no longer needs. + hudOverlayWindow.setIgnoreMouseEvents(ignore); + + if (!ignore) { + // Input is live again; the document's own pointer events are cheaper and + // finer-grained than anything sampled at 32 ms. + stopHudCursorPoll(); + return; + } + if (!hudCursorPoll) { + hudCursorPoll = setInterval(pollHudCursor, HUD_CURSOR_POLL_MS); } }); @@ -270,16 +336,9 @@ export function createHudOverlayWindow(): BrowserWindow { // ready-to-show, so the two are ~85 ms apart — measured, not assumed). What that // leaves open is an invisible rectangle that can swallow one desktop click in // those 85 ms, right after the user launched the app — against what doing it here - // cost them: the whole app (issue #266). On Windows the `forward` option is a global - // WH_MOUSE_LL hook, and that hook is the only way out of the state, because - // Chromium sends no pointermove to a window it has made input-transparent — so - // the renderer can never ask to leave it on its own. Electron latches - // the install behind `forwarding_mouse_messages_` and retries only after a - // setIgnoreMouseEvents(false) — the very call a dead hook prevents. One refused - // or revoked hook (Windows drops any whose callback overruns the 300 ms - // LowLevelHooksTimeout — on this thread, still busy booting the app) and the HUD - // is painted, inert, forever. Asking later moves the install onto an IPC message, - // i.e. onto a main thread that is provably pumping. + // cost them: the whole app (issue #266). A window nothing ever asks for — a + // renderer that dies before mount — then stays clickable instead of becoming a + // ghost. See the "hud-overlay-cursor" poll above for the way back out. // Keep the recording controls out of the recording (see applyContentProtection). applyContentProtection(win, "HUD"); @@ -307,6 +366,7 @@ export function createHudOverlayWindow(): BrowserWindow { if (hudOverlayWindow === win) { hudOverlayWindow = null; hudDragOrigin = null; + stopHudCursorPoll(); } }); diff --git a/package-lock.json b/package-lock.json index 59efaa908..011d7b244 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.9.2", + "version": "1.9.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.9.2", + "version": "1.9.6", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index 47a386c2f..5612797d2 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.9.2", + "version": "1.9.6", "type": "module", "packageManager": "npm@10.9.4", "engines": { diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index 208942e27..4a69effe8 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -41,6 +41,7 @@ import { } from "./Modals"; import { Preview } from "./Preview"; import type { TrimTarget } from "./RightPanes"; +import { importPendingRecording } from "./recordingImport"; import v4 from "./v4/EditorShellV4.module.css"; import { type EditorMode, EditorTopBar } from "./v4/EditorTopBar"; import { type Facet, FloatingInspector } from "./v4/FloatingInspector"; @@ -91,7 +92,6 @@ export function NewEditorShell() { const projectId = useProjectStore((s) => s.projectId); const dirty = useProjectStore((s) => s.dirty); const createProject = useProjectStore((s) => s.createProject); - const addAsset = useProjectStore((s) => s.addAsset); const setCurrentTime = useProjectStore((s) => s.setCurrentTime); const setSourceDuration = useProjectStore((s) => s.setSourceDuration); const loadProject = useProjectStore((s) => s.loadProject); @@ -222,59 +222,45 @@ export function NewEditorShell() { void (async () => { if (!window.electronAPI) return; try { - const result = await window.electronAPI.getCurrentRecordingSession(); - if (!result.success || !result.session?.screenVideoPath) { - // ponytail: no active recording — try to restore the user's - // most recent project. The browser-shim's listProjects - // returns the seeded `browser-shim-projects` entries, so - // e2e tests can land directly in a populated editor; for - // real Electron users this is the expected "open last - // project on launch" UX. - try { - const projects = await nativeBridgeClient.aiEdition.listProjects(); - console.info("[editor] listProjects returned", projects); - if (projects.length > 0) { - console.info("[editor] auto-loading project", projects[0].id); - await loadProject(projects[0].id); - const state = useProjectStore.getState(); - console.info( - "[editor] post-loadProject status=", - state.status, - "error=", - JSON.stringify(state.error), - "doc=", - state.document ? "loaded" : "null", - ); - } - } catch (e) { - console.warn("[editor] auto-load failed", e); - } + if (await importPendingRecording()) { + toast.success("Recording added to a new project"); return; } - const screenPath = result.session.screenVideoPath; - const label = screenPath.split(/[\\/]/).pop() || "Recording"; - await createProject(`Recording ${new Date().toLocaleString()}`); - await addAsset(screenPath, label); - // ponytail: MediaRecorder WebMs ship with duration = NaN until - // fix-webm-duration patches the EBML header; until that flows - // through the asset, drop a default 60s clip into the timeline - // so the editor isn't stuck on "No clips yet" the moment the - // user lands in the project. Real duration overwrites this - // when handleLoadedMetadata fires with a finite value. - const doc = useProjectStore.getState().document; - if (doc && doc.timeline.clips.length === 0 && doc.assets.length > 0) { - await useProjectStore - .getState() - .replaceTimeline([{ startSec: 0, endSec: 60 }], "Auto-imported recording"); - } - toast.success("Recording added to a new project"); } catch (err) { toast.error("Could not auto-create project from recording", { description: err instanceof Error ? err.message : String(err), }); + return; + } + // ponytail: no recording waiting — restore the user's most recent + // project. The browser-shim's listProjects returns the seeded + // `browser-shim-projects` entries, so e2e tests can land directly in a + // populated editor; for real Electron users this is the expected "open + // last project on launch" UX — and, now that the recording hand-off is + // consumed on import, it is also what reopening the editor after a + // recording lands on: the project that recording went into, settings and + // all, instead of a second project on the same file. + try { + const projects = await nativeBridgeClient.aiEdition.listProjects(); + console.info("[editor] listProjects returned", projects); + if (projects.length > 0) { + console.info("[editor] auto-loading project", projects[0].id); + await loadProject(projects[0].id); + const state = useProjectStore.getState(); + console.info( + "[editor] post-loadProject status=", + state.status, + "error=", + JSON.stringify(state.error), + "doc=", + state.document ? "loaded" : "null", + ); + } + } catch (e) { + console.warn("[editor] auto-load failed", e); } })(); - }, [addAsset, createProject, loadProject]); + }, [loadProject]); // Warn on close when dirty useEffect(() => { diff --git a/src/components/ai-edition/recordingImport.test.ts b/src/components/ai-edition/recordingImport.test.ts new file mode 100644 index 000000000..2c9728d44 --- /dev/null +++ b/src/components/ai-edition/recordingImport.test.ts @@ -0,0 +1,91 @@ +// @vitest-environment jsdom +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; +import { importPendingRecording } from "./recordingImport"; + +// The store's own bridge calls are never reached — every action the import uses +// is stubbed below — but importing the store pulls the client in, so stub it. +vi.mock("@/native/client", () => ({ nativeBridgeClient: { aiEdition: {} } })); + +const createProject = vi.fn(async () => undefined); +const addAsset = vi.fn(async () => null); +const replaceTimeline = vi.fn(async () => undefined); + +/** Stands in for the main-process recording slot: one value, set and read. */ +function stubElectronApi(screenVideoPath: string | null) { + let session = screenVideoPath ? { screenVideoPath, createdAt: 0 } : null; + const api = { + getCurrentRecordingSession: vi.fn(async () => + session ? { success: true, session } : { success: false }, + ), + setCurrentRecordingSession: vi.fn(async (next: typeof session) => { + session = next; + return { success: true }; + }), + }; + // biome-ignore lint/suspicious/noExplicitAny: test-only stub of the contextBridge surface + (window as any).electronAPI = api; + return api; +} + +describe("importPendingRecording", () => { + beforeEach(() => { + vi.clearAllMocks(); + useProjectStore.setState({ + document: null, + // biome-ignore lint/suspicious/noExplicitAny: partial action stubs, the rest of the store is untouched + createProject: createProject as any, + // biome-ignore lint/suspicious/noExplicitAny: partial action stubs, the rest of the store is untouched + addAsset: addAsset as any, + replaceTimeline, + }); + }); + + it("does nothing when no recording is waiting", async () => { + stubElectronApi(null); + await expect(importPendingRecording()).resolves.toBe(false); + expect(createProject).not.toHaveBeenCalled(); + }); + + it("imports the recording into a new project and consumes the hand-off", async () => { + const api = stubElectronApi("C:\\recordings\\recording-1.mp4"); + + await expect(importPendingRecording()).resolves.toBe(true); + + expect(createProject).toHaveBeenCalledTimes(1); + expect(addAsset).toHaveBeenCalledWith("C:\\recordings\\recording-1.mp4", "recording-1.mp4"); + expect(api.setCurrentRecordingSession).toHaveBeenCalledWith(null); + }); + + // The regression: the editor window is destroyed and recreated on every open, + // so a session left in the slot was imported again — a second project on the + // same recording, at default settings, with the user's saved ones stranded in + // the first one. + it("imports one recording once, however often the editor mounts", async () => { + stubElectronApi("C:\\recordings\\recording-1.mp4"); + + await importPendingRecording(); + await expect(importPendingRecording()).resolves.toBe(false); + + expect(createProject).toHaveBeenCalledTimes(1); + expect(addAsset).toHaveBeenCalledTimes(1); + }); + + it("seeds a placeholder clip when the imported asset has none", async () => { + stubElectronApi("/recordings/recording-1.webm"); + addAsset.mockImplementationOnce(async () => { + useProjectStore.setState({ + // biome-ignore lint/suspicious/noExplicitAny: only the two fields the seed reads + document: { assets: [{ id: "a1" }], timeline: { clips: [] } } as any, + }); + return null; + }); + + await importPendingRecording(); + + expect(replaceTimeline).toHaveBeenCalledWith( + [{ startSec: 0, endSec: 60 }], + "Auto-imported recording", + ); + }); +}); diff --git a/src/components/ai-edition/recordingImport.ts b/src/components/ai-edition/recordingImport.ts new file mode 100644 index 000000000..0b9b656b0 --- /dev/null +++ b/src/components/ai-edition/recordingImport.ts @@ -0,0 +1,55 @@ +// Hand-off from the recorder to the editor. +// +// The HUD parks the recording it just finished in ONE main-process slot +// (`set/getCurrentRecordingSession`) and opens the editor, which imports it into +// a fresh project on mount. The slot has to be emptied once that project owns +// the file, because opening the editor destroys and recreates its window +// (`createEditorWindowWrapper` in electron/main.ts) — so a session left in place +// is imported AGAIN on the next open: a second project on the same recording, +// back at the default padding / roundness / wallpaper, while everything the user +// set and saved stays behind in the first project, which is no longer the one on +// screen. That reads exactly like "the editor forgot my settings" (#364). +// +// `setCurrentRecordingSession(null)` is the existing clear (it also drops the +// derived `currentVideoPath`); the only renderer that still needs the session +// after this point is the CLI runner, which lives in its own process. + +import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; + +/** + * Imports the recording the HUD handed over into a new project, and consumes the + * hand-off so it is imported exactly once. + * + * Returns false when there is nothing pending — the caller then falls back to + * reopening the most recent project. Throws if the import itself fails, leaving + * the session in place so a later mount can retry it. + */ +export async function importPendingRecording(): Promise { + const api = window.electronAPI; + if (!api) return false; + + const result = await api.getCurrentRecordingSession(); + const screenPath = result.success ? result.session?.screenVideoPath : undefined; + if (!screenPath) return false; + + const label = screenPath.split(/[\\/]/).pop() || "Recording"; + await useProjectStore.getState().createProject(`Recording ${new Date().toLocaleString()}`); + await useProjectStore.getState().addAsset(screenPath, label); + // Consumed: the recording now lives in a project. Cleared here rather than + // after the timeline seed below so a failure down there can't hand the same + // recording to the next editor window. + await api.setCurrentRecordingSession(null); + + // ponytail: MediaRecorder WebMs ship with duration = NaN until + // fix-webm-duration patches the EBML header; until that flows through the + // asset, drop a default 60s clip into the timeline so the editor isn't stuck + // on "No clips yet" the moment the user lands in the project. Real duration + // overwrites this when handleLoadedMetadata fires with a finite value. + const doc = useProjectStore.getState().document; + if (doc && doc.timeline.clips.length === 0 && doc.assets.length > 0) { + await useProjectStore + .getState() + .replaceTimeline([{ startSec: 0, endSec: 60 }], "Auto-imported recording"); + } + return true; +} diff --git a/src/components/launch/LaunchWindow.test.tsx b/src/components/launch/LaunchWindow.test.tsx index 76b6a6fe9..3fdb7caae 100644 --- a/src/components/launch/LaunchWindow.test.tsx +++ b/src/components/launch/LaunchWindow.test.tsx @@ -63,6 +63,7 @@ const recorderState = vi.hoisted(() => ({ }, })); +let hudCursorListeners: Array<(x: number, y: number) => void> = []; let selectedSourceChangedListeners: SelectedSourceChangedListener[] = []; let sourceSelectorClosedListeners: Array<() => void> = []; @@ -209,6 +210,12 @@ function stubElectronAPI(getSelectedSource: Window["electronAPI"]["getSelectedSo })), setHudOverlaySize: vi.fn(), setHudOverlayIgnoreMouseEvents: vi.fn(), + onHudOverlayCursor: vi.fn((callback) => { + hudCursorListeners.push(callback); + return () => { + hudCursorListeners = hudCursorListeners.filter((listener) => listener !== callback); + }; + }), beginHudOverlayDrag: vi.fn(), dragHudOverlayTo: vi.fn(), endHudOverlayDrag: vi.fn(), @@ -273,6 +280,7 @@ function resetLaunchMocks() { recorderState.value.webcamEnabled = false; recorderState.value.setWebcamEnabled.mockClear(); micDevicesState.value = []; + hudCursorListeners = []; selectedSourceChangedListeners = []; sourceSelectorClosedListeners = []; i18nState.value.systemLocaleSuggestion = null; @@ -409,6 +417,68 @@ describe("LaunchWindow record button", () => { expect(window.electronAPI.openSourceSelector).not.toHaveBeenCalled(); }); + // The #385 regression, and #266 before it. A HUD that has gone click-through + // receives no pointer event of any kind, so every DOM route back — pointerenter, + // pointerdown, pointermove — is unreachable by construction. This test therefore + // fires NO pointer events at all: it delivers only the cursor position the main + // process pushes, which is the one signal that survives input-transparency, and + // requires that to be enough to make the bar clickable again. + it("leaves click-through on a pushed cursor position alone, with no pointer event", async () => { + platformState.value = "win32"; + + renderLaunchWindow(); + + await waitFor(() => { + expect(window.electronAPI.setHudOverlayIgnoreMouseEvents).toHaveBeenLastCalledWith(true); + }); + expect(hudCursorListeners).not.toHaveLength(0); + + // jsdom has no layout and does not implement elementFromPoint at all, so it is + // defined here to return what each point resolves to in a browser. The assertion + // is that the pushed cursor drives the hit test, not that jsdom can hit-test. + const bar = document.querySelector("[data-hud-interactive='true']"); + expect(bar).not.toBeNull(); + const elementFromPoint = vi.fn((_x: number, _y: number): Element | null => document.body); + Object.defineProperty(document, "elementFromPoint", { + value: elementFromPoint, + configurable: true, + }); + const setIgnore = vi.mocked(window.electronAPI.setHudOverlayIgnoreMouseEvents); + + try { + // The transparent reserve goes FIRST, while the window is still click-through. + // Do it after the bar has claimed input back and the assertion is vacuous: the + // renderer dedupes, so a point that wrongly enabled input would send no IPC at + // all and "still false" would hold either way. Here a wrong answer is an IPC. + setIgnore.mockClear(); + for (const listener of hudCursorListeners) listener(10, 10); + expect(setIgnore).not.toHaveBeenCalled(); + + // And the bar hands input back. + elementFromPoint.mockReturnValue(bar); + for (const listener of hudCursorListeners) listener(410, 540); + + expect(elementFromPoint).toHaveBeenCalledWith(410, 540); + expect(setIgnore).toHaveBeenCalledWith(false); + } finally { + Reflect.deleteProperty(document, "elementFromPoint"); + } + }); + + it("unsubscribes from the pushed cursor when the HUD unmounts", async () => { + platformState.value = "win32"; + + const { unmount } = renderLaunchWindow(); + + await waitFor(() => { + expect(hudCursorListeners).not.toHaveLength(0); + }); + + unmount(); + + expect(hudCursorListeners).toHaveLength(0); + }); + it("keeps the HUD interactive on Linux so the drag handle can receive pointer events", async () => { platformState.value = "linux"; diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx index 71d9eda27..7639936f6 100644 --- a/src/components/launch/LaunchWindow.tsx +++ b/src/components/launch/LaunchWindow.tsx @@ -420,6 +420,25 @@ export function LaunchWindow() { setHudMouseEventsEnabled(isPopoverOpen); }, [isPopoverOpen, setHudMouseEventsEnabled]); + // The way back out of click-through. Every other route below — pointerenter and + // pointerdown on the bar, pointermove on the root — needs an event this document + // stops receiving the moment the window goes input-transparent, which is what left + // the HUD painted and permanently dead in #266 and again in #385. So the main + // process samples the OS cursor and pushes it here instead, and the hit test is the + // one `handleRootPointerMove` already runs, against the same layout: elementFromPoint + // honours pointer-events, so a point over the transparent reserve resolves to the + // root and correctly stays click-through. + // + // Only ever turns click-through OFF. Turning it back on is the DOM handlers' job, + // and they are reliable by then — the window is receiving real input again. + useEffect(() => { + return window.electronAPI?.onHudOverlayCursor?.((x, y) => { + if (document.elementFromPoint(x, y)?.closest("[data-hud-interactive='true']")) { + setHudMouseEventsEnabled(true); + } + }); + }, [setHudMouseEventsEnabled]); + const defaultSourceName = t("sourceSelector.defaultSourceName"); const [selectedSource, setSelectedSource] = useState(defaultSourceName); const [hasSelectedSource, setHasSelectedSource] = useState(false); diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 9724c27ef..a6da24d74 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -591,8 +591,10 @@ export function useScreenRecorder(): UseScreenRecorderReturn { // disagreeing about whether anything was recording: the HUD kept // showing a stop button, and pressing it sent a second stop that // came back "Native Windows capture is not running." (issue #252). - // The recording is already lost either way -- what the user needs - // is to be able to start a new one. + // Reaching here now means the take really is unreadable -- a failed + // stop that left a playable fragmented file comes back `success` + // with a session and takes the editor path below, so this branch no + // longer decides the fate of a recoverable recording. clearNativeRecordingState(); return true; } diff --git a/technical-documentation/engineering/release-and-secrets.md b/technical-documentation/engineering/release-and-secrets.md index 3451ece82..169d610ec 100644 --- a/technical-documentation/engineering/release-and-secrets.md +++ b/technical-documentation/engineering/release-and-secrets.md @@ -169,6 +169,20 @@ Two constraints from Microsoft's documentation: automated updates through GitHub `msstore submission updateMetadata` can also drive the Store listing text from a versioned `metadata.json`, which would replace the CSV export/import round-trip. Not wired up here. +**It has submitted nothing yet.** v1.9.5 was the job's first real run — it did not exist on the v1.9.1 or v1.9.2 builds — and it failed: `We could not find a project publisher for the project at …Openscreen.Setup.1.9.5.appx`. Credentials were fine; the CLI reported the configuration valid and resolved the product. The call was wrong. `msstore publish` takes a **project root** as its positional argument, detects the app type there, and only then accepts a built package through `--inputFile`; the job passed the `.appx` positionally and never checked the repo out, so there was no project to detect. Fixed by adding a checkout (before the artifact download — `actions/checkout` cleans the workspace) and calling `msstore publish . --inputFile --appId `. + +That failure was visible only because the same release carried the fix that reports the submission's real outcome instead of the configuration's. The prior version wrote "Submitted to the Store" whenever credentials resolved, under `always()` — so this exact failure would have shipped as a green success. + +**Still unverified, and the next thing likely to break:** `--inputFile` is documented for `.msix` and `.msixupload`, and `build:win:store` produces an `.appx` (`electron-builder --win appx`). Whether the CLI accepts that extension is untested. + +### Retrying a Store submission + +`publish-msstore.yml` submits an already-built appx on demand: `workflow_dispatch` with a stable `release_tag`, optionally a `run_id` (defaults to the most recent `build.yml` run for that tag), and a **`dry_run`** flag. + +It exists because `build.yml`'s own job has no usable retry. Re-running the failed job replays the workflow definition frozen into the original run, so a fix landed afterwards is never picked up; and re-dispatching `build.yml` rebuilds every platform and re-uploads the release assets with `--clobber`, rewriting a published release to correct a Store submission — and, if dispatched from `main` rather than the tag, rewriting it with binaries built from code that release never contained. v1.9.5 hit both walls. + +**`dry_run: true` is the only safe way to test this path.** It passes `-nc, --noCommit`, which creates the submission and leaves it in draft instead of sending it to certification. Without it — and this is what `build.yml` does — `msstore publish` commits, so a dispatch fired "just to see whether the `.appx` is accepted" puts a build in front of users. Validate with the dry run first; submit for real only once it comes back clean. + Rotate by issuing a new client secret on the Entra registration, updating `AZURE_AD_APPLICATION_SECRET`, publishing one release to confirm, then deleting the old secret. The tenant, client and seller IDs change only when the registration or account does. ## Discord secrets and variables diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index f1375ff73..a57209706 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -6,10 +6,23 @@ Sections marked **v1.8.0** cover what this release changed: chat-driven editing ## How to run this -1. Drive the real Electron app with computer-use, not a browser shim. Start a dev build with `npm run dev`, or launch the packaged build under test. +1. Drive the real Electron app with computer-use — real OS mouse and keyboard events. Start a dev build with `npm run dev`, or launch the packaged build under test. + + "Manual" here usually means an agent holding the mouse, so the tempting shortcut is not a browser shim: it is driving the real app through CDP instead. **Do not.** Playwright's `.click()`, `javascript_tool`-dispatched pointer events and anything else synthesised into the renderer arrive *below* the OS hit-test. On Windows and macOS the HUD is input-transparent until a real cursor move lifts it, so an injected click fires the DOM handler and comes back green while the path a user actually takes was never exercised at all. + + That trap is specific to the HUD and the countdown overlay — they are the only click-through windows; the editor is an ordinary one, and an injected click there does reach the handler a user would reach. The reason not to inject in the editor either is the first line of this file: this checklist covers what unit, browser and **Playwright** tests cannot. Drive it the way those tests already drive it and you have re-run the coverage you had, then written "passed" beside the parts nothing checked. 2. The app is single-instance per `userData` path. If a leftover Electron/OpenScreen process still holds the lock, stop that process before relaunching; a second launch can exit successfully without opening a window. The lock is held by the OS and is released when the process dies, so there is nothing to delete on disk. -3. From a worktree, link or junction `node_modules` to the main checkout and provide the prebuilt native capture binaries for the platform before starting the dev build. -4. Grant computer-use access to the process name that actually owns the window: `electron.exe` or `Electron.app` for a dev build, and `Openscreen.exe` or `Openscreen.app` for a packaged build. Do not grant access only to the installed app name when testing a dev build. +3. From a worktree, link or junction `node_modules` to the main checkout and provide the prebuilt native capture binaries for the platform before starting the dev build. **Date those binaries against the change you came to test.** Nothing rebuilds them, so an older helper runs the old code path in silence: the recording succeeds and the thing you were checking for is simply absent. See AGENTS.md for how to check one and how to rebuild it; when you cannot, test the CI-built artifact, because a dev build cannot answer a native question. +4. Grant computer-use access to the process name that actually owns the window: `electron.exe` or `Electron.app` for a dev build, and `Openscreen.exe` or `Openscreen.app` for a packaged build. Do not grant access only to the installed app name when testing a dev build — it resolves to the *installed* executable and reports success while the dev window stays masked. This is step 4 and not step 1 for a reason: a dev build is not an installed app, so the resolver cannot find it until it is running and owns a window, and one unresolvable name voids the whole request. + + **Ask for everything in ONE call — after the launches above, before the first check.** `request_access` takes a list, and once a grant is in place the rest of the pass runs without a single further prompt: a full capture-to-export run is dozens of clicks and none of them ask again. So the only thing keeping a human at the keyboard is *how many* dialogs you raise and *when*. Raise one, before the first check, and the operator can walk away for the rest of the run; discover a fourth app you need an hour in and they cannot. That is also why this cannot move earlier — the resolver needs the app running, and one unresolvable name voids the batch. Beyond the app under test, ask for: + + - the desktop shell — the tray icon and the native save dialogs live there, and the tray is the only reliable way back to the HUD; + - the OS settings app — needed to change display scaling, which is how DPI checks are run (see AGENTS.md; "the machine is at 100%" is not a reason to skip them). + + **Name them the way the Start menu does, in the system's own language.** The resolver matches installed-app display names, not executables: on a French Windows the shell is `Explorateur de fichiers` and `explorer.exe` fails outright — `notInstalled`, with a nonsense suggestion attached — which then voids every other name in the same call. On an English install it is `File Explorer`. When unsure, ask rather than guess; the tool lists the installed names it knows. + + There is no way to pre-approve any of this in config: the request has to be answered live. That is upstream ([claude-code#46907](https://github.com/anthropics/claude-code/issues/46907), closed stale), and `bypassPermissions` does not cover it either ([#43172](https://github.com/anthropics/claude-code/issues/43172)). Batching is the whole mitigation. 5. Read [AGENTS.md](../../AGENTS.md) for the computer-use mechanics, screenshot permissions, tray interaction, and cleanup procedure. Read one check, perform it, observe the result, then continue; close each modal or popover with `Esc` before the next check. 6. The recording HUD is protected from capture by default and is invisible in screenshots. For this session only, launch with `OPENSCREEN_DISABLE_CONTENT_PROTECTION=1`; this is the environment variable checked before `setContentProtection(true)`. Unset it before making any recording whose HUD must not appear in the video. 7. A preview screenshot is downscaled. Settle every pixel-level question by exporting a frame and measuring the exported frame, not by judging fine edges, corners, shadows, or alignment from the preview screenshot. @@ -406,5 +419,8 @@ The agent may only call the fixed tool set in [ai-agent.md](../architecture/ai-a | Date | Build / tag | Platform | Pass/fail | Notes | |------|-------------|----------|-----------|-------| | 2026-07-31 | dev build, `claude/e2e-tests-v1-8-0-474894` (e9578f09) | macOS 26.5, M1 | Partial — 1 defect | Ran launch/HUD, media, modifier anchoring, and export. **Defect: a dangling asset blanks the preview.** Modifier anchoring across a reorder verified in preview and in the exported frames. macOS export produced 1280×720 h264 + AAC at ~2× realtime. Chat sections skipped: no AI provider configured. HUD drag not runnable under computer-use (drop point is the desktop). | +| 2026-08-13 | installed `v1.9.5-rc.1` | Windows 11 26200, 1920×1080 @ 100% | Partial — 1 defect | Ran launch/HUD, source selection, recording, stop, editor open. Fragmented MP4 confirmed on the shipped artifact: 48 `moof`+`mdat` pairs over 47.6s, `mvex` present, `mfra` on clean stop. **Defect: a recording that survives a helper kill is thrown away by the app** — killing `wgc-capture.exe` mid-recording leaves a fully decodable 41s file (2460 packets, `ffmpeg -f null -` exit 0) with no `.session.json` and no `.cursor.json`, and stop answers "The recording could not be saved". Fixed in #363, re-verified end to end. Truncation ablation at 60%: plain MP4 unreadable, fragmented plays 29s. **A dev build cannot test any of this** — the prebuilt worktree helper predated the change and silently ran the old path. Editor/export/chat sections not run. | +| 2026-08-14 | `release/v1.9.5` @ `b1b81de5` (rc.2 candidate: dev TS + the CI-built rc.1 native payload, which is byte-identical since no native source changed) | Windows 11 26200, 1920×1080 @ 100% | Pass — no defect | Regression net across the 65 commits since **v1.9.2**, not just the rc.2 delta. Four recordings. Every one a fragmented MP4 (`mvex` + ~1 `moof`/s, `mfra` only on a clean stop). GPU DXGI path still correctly opt-in (`videoInput: cpu-rgb32`) — the #336 regression has not crept back. No capture-pacing drift: HUD `00:59` → 60.067s at 60/1. Waveform correct in both directions: absent with no audio track, rendered with one. Audio muxes into the fragmented container (AAC 48k stereo) with 15 ms A/V drift, under one frame. Compositor renders and exports with no camera declared. **#366**: reopening returns to the saved project with its settings (Blur BG on, padding 9%) and mints no second project — 167→168 across a whole new recording. **#363**: helper killed mid-recording → editor opens on the recovered take (46 `moof`, no `mfra`), all three sidecars written, imported once. Export MP4 1080p60 **from that recovered take**: 46.0s / 2760 packets, decodes clean, duration matches the source exactly. Tray refocus works. NOT covered: DPI scaling — **not re-run here, already validated when `60bb6d7c` / `71cc88d6` landed**; note that the display scale is a setting, so "this machine is at 100%" is never a reason a DPI bug cannot be tested (flip it to 150%, ~2 min). Also not covered: webcam PiP and the export webcam fixes, microphone, GIF, macOS/Linux, AI sections, packaging. | +| 2026-08-14 | installed `v1.9.5-rc.1`, macOS Apple Silicon DMG (CI-built, Developer ID signed). **rc.2 is not published** — only rc.1 exists on Releases; no native source changed between `v1.9.5-rc.1` and `origin/release/v1.9.5`, so this artifact already carries the rc.2 native payload, but #366 (cross-platform TS) is absent from it | macOS 26.5 (25F71), M1, 1920×1080 @ 2× | **Fail — 1 blocker** | **The plan's assertion-1 criterion does not hold on macOS, in both directions.** On a clean stop `AVAssetWriter.finishWriting()` collapses the fragments into a normal movie: `ftyp mdat moov`, `mvex` ABSENT, 0 `moof`, no `mfra` (45 s / 44.4 MB run). That is exactly the shape the plan calls the headline failure — and the pre-`a6795d23` control recording (2026-08-10) has the *same* shape — so **a clean-stop box walk cannot distinguish fragmented from plain on macOS; only the kill test can.** Fragmenting *is* active: the takes whose writer died mid-fragment retain `mvex` + ~1 `moof` per second of media (shipped-build writer-failure samples: 35 `moof`/36.0 s, 14/15.0 s, 3/4.0 s; plus 18 on a surviving-helper kill). The one kill on the shipped build is the exception that proves the scope — capture had already stalled ~12 s before the kill, so it carries `mvex` but **0 `moof`** and only 1.0 s. No macOS file, clean or killed, ever carried `mfra`. **Blocker: every app-driven recording truncates, then the app discards it.** (Root cause and fix reported in #375 — the fragments carry a negative composition offset in a version 0 `trun`, where ISO/IEC 14496-12 8.8.8.2 defines the field as unsigned, because frame reordering was left on; `AVVideoAllowFrameReorderingKey: false` clears it and restores the crash-resilience the fragmenting was for. Verified at helper level there; **this rc.1 run only reproduced the failure and validated nothing about the fix**. Re-run this section against a CI build carrying #375 before rc.2 ships.) 3/3 takes stopped writing early while the HUD kept counting — media 4.0 s / 36.0 s / 15.0 s against HUD `02:02` / `01:30` / `01:04`. Helper emits `{"event":"error","code":"writer-failed"}`; main log `AVFoundationErrorDomain Code=-11800 … (-16341)`. Stop then hangs ~30 s on "Saving…" and drops the take: no `.session.json`, no `.cursor.json`, no editor. The app *does* surface the raw error in a toast (confirmed by hand on the same machine at 13:28–13:35 — my automated runs screenshotted after it auto-dismissed, so an earlier draft of this row wrongly said there was none). 44,561,966 / 328,337,979 / 139,631,607 / 17,187,009 bytes decodable and thrown away (147 GB free — not disk). Reproduced standalone with the shipped helper at 1080p30/8 Mbps, 2/2 (~9 s, ~5 s), so it is not confined to the app's 4K60 path — but do not read that as load-independent: append rate demonstrably modulates how reliably it bites (#375 measures it reliable at ~57 fps and intermittent at 30 fps). **Reproduced by hand, no automation involved**, on six takes recording a YouTube page — and those six separate the trigger cleanly: **system audio ON → 3/3 died at ~1.0 s and minted 0 projects; system audio OFF → 3/3 survived (3.3 s, 7.4 s, 25.0 s) and minted 1 project each.** **Audio is not the condition, only an accelerant** — a controlled run with system audio off *and not one screenshot taken during the capture* (the screenshot layer hides non-allowlisted windows, so it was the last confound worth eliminating) died the same way: 8.008 s of video, 79,004,330 bytes then flat for 76 s with the helper still alive, 7 `moof`, 0 sidecars, 0 projects, same `-11800`/`-16341`. What audio changes is the window: with a track it is ~1 s, without one ~4–40 s. That reconciles the by-hand takes with mine — a take short enough to stop before the writer dies is clean, which is why 3.3 s and 7.4 s survived and 8.0 s did not, and why the 25.0 s one minted a project while still carrying `mvex` (never cleanly finalised). **Turning audio off is therefore not a safe workaround.** Untested here: microphone — this Mac has no input device, and whether a mic track triggers the same path is an inference, not a measurement. **Helper A/B narrows the with-audio path to the fragmentation line**: helper built twice from source identical to the rc.1 tag, differing only by `writer.movieFragmentInterval` (701 vs 700 lines) — with system audio at 1080p30, WITH the line `writer-failed` 2/2 (2.0 s, 1.0 s), WITHOUT it clean `recording-stopped` 3/3 (40.6 s, 37.9 s, 37.6 s). **Read those counts as a sample, not a law**: a later rebuild of the with-the-line arm survived 22.2 s at the same settings, so the failure is probabilistic and rate-dependent, and the byte-level evidence in #375 is what actually carries the case. The video-only local-vs-shipped gap (local survived 45 s, shipped failed 5/5) is explained by the same variable rather than by the released artifact — the shipped runs encoded at 56.6 fps against 29 fps locally. **Kill test** is confounded on the shipped build (capture already dead before the kill): 17.19 MB → only 1.0 s / 56 packets, 0 `moof`. On a helper that does not fail, a mid-write kill leaves 18 `moof`, decodes clean (`ffmpeg -v error -f null -` exit 0, 1373 packets) and no `mfra` — the shape the plan expects. **#363 gap confirmed, and on macOS it fires with no kill at all**: `writer-failed` alone loses the take; there is no app-side recovery. **Audio**: AAC 48 kHz stereo muxes into the fragmented container, video start `0.000000` vs audio `0.014479` → 14.5 ms drift, under one frame at 30 fps (measured on the 2.0 s written before the writer died). **Compositor + export pass**: preview renders with no camera declared; export MP4 1080p60 H.264+AAC via `h264_videotoolbox (zero-copy VT)`, 5,726,865 bytes, 318 packets, decodes clean, duration matches to within 7 ms — source 26.713 s minus trims 19.910 + 1.513 = 5.290 s expected vs 5.283 s measured, under one frame at 60 fps. **#366 not runnable as specified** (absent from rc.1, rc.2 unpublished, and record→editor never completes); adjacent behaviour measured on an existing project — close+reopen kept 19→19 projects, exactly ONE project references the recording, and Blur BG / padding survived (`showBlur=true`, `padding=16`). NOT covered: Windows-only DPI and wgc-capture, GIF, AI sections, packaging (per plan); webcam PiP and microphone — this Mac has neither (Device settings reports "No microphone found" / "No camera found"). | | | | | | | | | | | | | diff --git a/tests/e2e/windows-native-checklist.spec.ts b/tests/e2e/windows-native-checklist.spec.ts index 959b15f1d..22b378196 100644 --- a/tests/e2e/windows-native-checklist.spec.ts +++ b/tests/e2e/windows-native-checklist.spec.ts @@ -321,11 +321,27 @@ test.describe("Windows native checklist smoke tests", () => { }); // The HUD must reach click-through by *asking* for it from the renderer, never - // by being born that way. On Windows the `forward` option is a global - // WH_MOUSE_LL hook, and it is the only route back out: a HUD that is already - // input-transparent when the hook fails to install can never be clicked again, - // which is what bricked the app in issue #266. Both halves matter — that nothing - // asks during construction, and that the renderer still does after mount. + // by being born that way: a window born input-transparent whose renderer never + // mounts can never be clicked again, which is what bricked the app in issue #266. + // Both halves matter — that nothing asks during construction, and that the + // renderer still does after mount. + // + // The second assertion also pins `forward` OFF. It used to be the only route back + // out of click-through, via a global WH_MOUSE_LL hook that Windows can refuse or + // silently revoke — which is how #385 reproduced a dead HUD on a build that already + // carried the #266 fix. The way out is now the "hud-overlay-cursor" poll in + // electron/windows.ts, and asking for `forward` again would restore the dependency + // without restoring the need. + // + // Note what this test therefore cannot do, and what no test in this file can. + // Only a real OS cursor move drives a WH_MOUSE_LL hook; CDP-injected input + // arrives below the OS hit-test, so Playwright's own `.click()` on a HUD testid + // — above, and in the source-selector step of the checklist test — reaches the + // DOM handler whether or not click-through is installed, or even working. Those + // clicks assert renderer wiring and nothing else. The failure #266 actually shipped + // (a painted, permanently inert HUD) is invisible to injected input by construction, + // so it belongs on the manual computer-use checklist and cannot be regression-tested + // here. Do not read a green run as evidence that the HUD is clickable. test("the HUD asks for click-through instead of being born with it", async () => { const app = await launchApp(); @@ -373,7 +389,7 @@ test.describe("Windows native checklist smoke tests", () => { // And the renderer does ask, once it has mounted. await expect .poll(() => app.evaluate(() => globalThis.__hudTape ?? []), { timeout: 20_000 }) - .toContainEqual([true, { forward: true }]); + .toContainEqual([true]); } finally { await app.evaluate(({ BrowserWindow }) => { const original = globalThis.__hudSetIgnoreMouseEvents;