From ee19d00a678a70de52141501f36d6fa117814e53 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 21 Aug 2026 09:46:55 +0200 Subject: [PATCH 1/3] ci(nix): let the job finish, and stop it blaming export for a capture failure Two things the first runs on main showed, neither of which is about the derivation. cancel-in-progress killed every run before it finished. The job takes half an hour and main takes merges minutes apart, so each merge evicted the one before it: 21m38s, 25s, 7m05s, all cancelled, and the run that finally reported did so only because the merges happened to stop overnight. A cancelled run is not a failure either, so nothing went red while the workflow reported nothing at all. That reading is what the concurrency key was already half-fixed for -- separating the schedule from pushes -- and push-versus-push is the same mechanism, missed at the time. Queueing costs at most one pending run, since GitHub keeps a single pending entry per group and replaces it. And the failure annotation named the wrong component. Every run so far has died inside `record`, with export never executing, while the message read "the compositor addon is packaged but the export path does not work" -- an accusation aimed at the one thing the run had not reached, and the addon is precisely what this step exists to vouch for. Track whether record ever produced a project, and say which stage actually failed. The reason is already in the scratch file the run writes, so surface it rather than leaving it there. Co-Authored-By: Claude --- .github/workflows/nix-build.yml | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index 4cd79af1b..428e00beb 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -43,7 +43,15 @@ concurrency: # Keying on the event as well keeps a merge train collapsing to its latest push # without letting it cancel the schedule. group: nix-build-${{ github.event_name }}-${{ github.ref }} - cancel-in-progress: true + # false, where a PR-branch check would want true. This job takes half an hour + # and main takes merges minutes apart, so cancel-in-progress meant each merge + # evicted the previous run before it finished: the first four runs on main were + # cancelled at 21m38s, 25s, 7m05s and then one that only survived because the + # merges happened to stop. A cancelled run is not a failure either, so the + # branch showed nothing while the workflow reported nothing. Queueing instead + # costs at most one pending run -- GitHub keeps a single pending entry per group + # and replaces it -- and every merge eventually gets an answer. + cancel-in-progress: false jobs: build: @@ -294,6 +302,13 @@ jobs: # capture failures are a separate problem, already reported above. echo "--- record then export ---" EXPORTED="" + # Tracked apart from EXPORTED so the verdict can name the stage that + # actually failed. Every run so far has died in record without export + # ever executing, while the annotation said "the export path does not + # work" -- an accusation aimed at the one component the run never + # reached, and the compositor addon is precisely what this step exists + # to vouch for. + RECORDED=0 for i in 1 2 3; do echo "=== export attempt $i/3 ===" rm -f /tmp/demo.openscreen /tmp/demo.mp4 @@ -302,8 +317,12 @@ jobs: if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then echo "record failed (rc=$RC); last lines:" tail -5 "/tmp/rec.$i.out" || true + # The bound inside get-sources names its own failure; surface it + # rather than leaving the reason five lines up in a scratch file. + grep -a "get-sources\]" "/tmp/rec.$i.out" | tail -3 || true continue fi + RECORDED=1 echo "recorded. project:" head -c 200 /tmp/demo.openscreen; echo @@ -319,8 +338,10 @@ jobs: done EXPORT_OK=0 - if [ -z "$EXPORTED" ]; then - echo "::error::No attempt produced an MP4. The compositor addon is packaged but the export path does not work." + if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then + echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." + elif [ -z "$EXPORTED" ]; then + echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." else SIZE=$(wc -c < "$EXPORTED") # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or @@ -350,7 +371,7 @@ jobs: # per-attempt warnings above keep that flakiness visible without letting # it decide the build; tighten this to $ATTEMPTS once the capture failure # is understood and fixed. - echo "=== verdict: enumeration $OK/$ATTEMPTS ok, export $EXPORT_OK ===" + echo "=== verdict: enumeration $OK/$ATTEMPTS ok, record $RECORDED, export $EXPORT_OK ===" if [ "$EXPORT_OK" -ne 1 ] || [ "$OK" -eq 0 ]; then exit 1 fi From dda5d3e6c47ba813b08e3d093239606c84a51a5a Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 21 Aug 2026 09:47:00 +0200 Subject: [PATCH 2/3] feat(diagnostics): time desktop source enumeration under the diagnostic flag The bound added to get-sources turned an indefinite hang into a named failure, which is where the question starts rather than ends. On a headless runner `openscreen sources` gets an answer within 20s four times in five, while `record` -- the same call, the same options -- exceeds 30s on all three attempts, and the log has nothing to say about the difference beyond which side of the threshold each landed on. A duration on both paths is what separates "slow here, fast there" from "never returns here", and neither a threshold nor a stack trace can. Gated on the flag that already exists, so a normal run is unchanged; main-process console is rerouted to stderr in CLI mode, so it lands beside the milestones. Co-Authored-By: Claude --- electron/ipc/handlers.ts | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index b6916e355..f8b40c4bd 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -52,7 +52,7 @@ import { import type { CursorTelemetryReader } from "../ai-edition/deep-agent/service"; import { DocumentService } from "../ai-edition/document-service"; import { LlmConfigStore } from "../ai-edition/llm-config-store"; -import { mainLogBuffer } from "../diagnostics/main-log-buffer"; +import { isDiagnosticModeEnabled, mainLogBuffer } from "../diagnostics/main-log-buffer"; import { mainT } from "../i18n"; import { getInstallChannel } from "../install-channel"; import { RECORDINGS_DIR } from "../main"; @@ -1716,12 +1716,35 @@ export function registerIpcHandlers( // await it, and a renderer-side race would only stop *waiting* while this // keeps running and its reply goes to nobody. Rejecting is what turns an // indefinite spinner into the pickers' existing error branch. - const sources = await withDeadline( - desktopCapturer.getSources(opts), - GET_SOURCES_TIMEOUT_MS, - `Desktop source enumeration did not return within ${GET_SOURCES_TIMEOUT_MS}ms. ` + - "This usually means the display or GPU stack cannot be reached — check that a display server is available.", - ); + // How long it actually took, under the existing diagnostic flag. The bound + // above turned an indefinite hang into a named failure, which is where the + // open question starts rather than ends: on a headless runner `openscreen + // sources` gets an answer within 20s four times in five while `record` -- + // the same call with the same options -- exceeds 30s every time. A duration + // on both paths is what tells those apart; a threshold alone cannot. + const startedAt = Date.now(); + const diagnostic = isDiagnosticModeEnabled(); + let sources: Awaited>; + try { + sources = await withDeadline( + desktopCapturer.getSources(opts), + GET_SOURCES_TIMEOUT_MS, + `Desktop source enumeration did not return within ${GET_SOURCES_TIMEOUT_MS}ms. ` + + "This usually means the display or GPU stack cannot be reached — check that a display server is available.", + ); + } catch (error) { + if (diagnostic) { + console.info( + `[get-sources] gave up after ${Date.now() - startedAt}ms (types=${(opts?.types ?? []).join(",")})`, + ); + } + throw error; + } + if (diagnostic) { + console.info( + `[get-sources] returned ${sources.length} source(s) in ${Date.now() - startedAt}ms (types=${(opts?.types ?? []).join(",")})`, + ); + } lastEnumeratedSources = new Map(sources.map((source) => [source.id, source])); return sources.map((source) => ({ id: source.id, From 849e992749d53bb4f0df35471729b8b3d5e666a9 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 21 Aug 2026 12:44:43 +0200 Subject: [PATCH 3/3] fix: correct the concurrency claim and stop calling every rejection a timeout Both from the review, and the first is a factual error of mine. "Every merge eventually gets an answer" is not what cancel-in-progress: false buys. GitHub holds one pending entry per group and a newer push replaces it, so merges landing while a run is in flight still go unbuilt. What the setting buys is that a run which has started finishes -- the affordable half, and the one that was missing. Verifying every merge would need a queue this workflow does not have, and is not worth it for a half-hour drift check. And the diagnostic logged "gave up after Nms" for anything the catch saw, including a getSources that rejected on its own well inside the deadline. For a commit whose whole purpose is to say what happened, asserting a timeout that did not occur points the next reader at the wrong thing. Log the error instead; the deadline error already carries its own wording. Co-Authored-By: Claude --- .github/workflows/nix-build.yml | 11 ++++++++--- electron/ipc/handlers.ts | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index 428e00beb..b8994692f 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -48,9 +48,14 @@ concurrency: # evicted the previous run before it finished: the first four runs on main were # cancelled at 21m38s, 25s, 7m05s and then one that only survived because the # merges happened to stop. A cancelled run is not a failure either, so the - # branch showed nothing while the workflow reported nothing. Queueing instead - # costs at most one pending run -- GitHub keeps a single pending entry per group - # and replaces it -- and every merge eventually gets an answer. + # branch showed nothing while the workflow reported nothing. + # + # What this buys is that a started run finishes, not that every merge is + # verified: GitHub holds a single pending entry per group and a newer push + # replaces it, so merges landing while a run is in flight still go unbuilt. + # That is the affordable half. Verifying each merge would need a queue this + # workflow does not have, and is not worth it for a half-hour job whose purpose + # is catching drift rather than gating a commit. cancel-in-progress: false jobs: diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index f8b40c4bd..8596dfafd 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1734,8 +1734,13 @@ export function registerIpcHandlers( ); } catch (error) { if (diagnostic) { + // The reason, not an assumption about it: this catch also sees a + // getSources that rejected on its own, well inside the deadline, and + // calling that a timeout would point the next reader at the wrong thing. + // The deadline error carries its own wording. + const reason = error instanceof Error ? error.message : String(error); console.info( - `[get-sources] gave up after ${Date.now() - startedAt}ms (types=${(opts?.types ?? []).join(",")})`, + `[get-sources] failed after ${Date.now() - startedAt}ms (types=${(opts?.types ?? []).join(",")}): ${reason}`, ); } throw error;