fix(desktop,host-service): address unreviewed findings from the sentry fix wave

Merged
SA
3mo ago
+91-28

Problem

The sentry-fix wave (#6191, #6218, #6258, #6259, #6260, #6316–#6320) was merged on 2026-08-13 after sitting open for 3–8 days. Reviewing the merge afterwards turned up something worth recording: not one of those PRs had a human review, and none had CHANGES_REQUESTED. They were not declined, they were never looked at. But the automated reviewers did leave findings, and nobody acted on them.

This PR fixes the four that verify as real against the code. Two others were checked and dismissed: the Windows-separator concern on #6218 (git emits / on every platform, so changeset paths never contain \) and the "preserve the original message" concern on #6259 (that substitution was the pre-existing behaviour and is deliberate).

Root cause and fix, per finding

1. Cloud credential could reach Sentry (#6191, flagged Major). The coordinator attaches a tail of the child's stdout/stderr to crash reports and redacted instance.secret from it — but it also injects config.authToken, the cloud credential, into the same child's env. Anything the child logged that embedded that token (a request header, an env dump inside a stack trace) would have been captured verbatim.

Replaced the single .split(secret).join(...) with a redactions: string[] on the instance, populated at spawn from both secrets and at adopt from the manifest token. A list also means the next secret added to the child env is one array entry away from being covered, rather than another chained replacement someone has to remember.

The test helper that stands up a running instance for the exit handler was constructing it through a loosely-typed internals map, so it silently omitted the new field; it now sets it. Production code stays strict rather than defaulting the field, so the type system keeps telling the truth.

2. Any git failure reported as a detached HEAD (#6258, flagged P2). getCurrentBranch() returns null for a detached HEAD and for every other git failure — not a repository, permission denied, git not on PATH — because both of its lookups swallow their errors. #6258 turned that null into BAD_REQUEST / { kind: "DETACHED_HEAD" }, so those users are now told to "checkout a branch", and, because the code is no longer a 500, the underlying breakage stopped reaching Sentry entirely. That is a net loss of signal on top of a misleading message.

Added readCurrentBranch(), which returns a discriminated { kind: "branch" } | { kind: "detached" } and lets a genuine git failure propagate. getCurrentBranch now delegates to it and keeps its existing string | null contract for its other callers, so nothing else changes.

A first attempt added a separate isDetachedHead() helper, which CI correctly rejected: it constructed a second simple-git client on the Electron main process, and no-main-process-blocking.test.ts ratchets that count downward. Bumping the allowlist would have defeated the point of the ratchet. Reading HEAD once through one client keeps the count unchanged at 21 and is the better shape anyway.

3. WebGL probe leaked a context per mount (#6318, flagged P2). supportsWebgl() created a canvas and acquired a context on every mount and never released it. Browsers cap concurrent WebGL contexts, so enough remounts starve the real gradient and leave otherwise-capable browsers on the static fallback permanently — the fix quietly causing the failure it was written to prevent. Now releases the probe via WEBGL_lose_context and caches the answer, which cannot change over a page's lifetime.

4. Directory guard accepted a regular file (#6316, flagged P2). existsSync is true for a file. Switched to statSync(..., { throwIfNoEntry: false })?.isDirectory() and corrected the message, which claimed the directory was missing.

Verification

bunx biome check on the six changed files:

Checked 6 files in 47ms. No fixes applied.

Typecheck:

desktop typecheck=0
host-service typecheck=0

Tests:

apps/desktop        752 pass, 0 fail (48 files)
packages/host-service 161 pass, 0 fail (12 files)

Not fixed here

spawn git ENOENT is still unclassified in the host-service git classifier (flagged P1 on #6260). Deliberately skipped: Node reports the same ENOENT whether the working directory vanished or git is not installed, and collapsing those would repeat exactly the over-classification this PR fixes in finding 2. It needs a cwd existence check at the classification site to be done correctly.

Refs DESKTOP-JM

Summary by cubic

Fixes four unreviewed findings from the Sentry fix wave to prevent credential leakage, restore accurate git failure signal, release WebGL probe contexts, and correctly require a directory for project repos. Aligns with DESKTOP-JM by tightening telemetry redaction and error classification.

  • apps/desktop workspaces/create: Use a new readCurrentBranch that returns { kind: "branch" | "detached" }. Only genuine detachment is BAD_REQUEST; other git failures throw and reach Sentry. Reads HEAD once via a single git client.

  • apps/desktop host-service coordinator: Replace single-secret replacement with per-instance redactions applied to crash-report outputTail, covering both the child secret and the cloud authToken. Tests set redactions.

  • packages/ui: Release the WebGL probe via WEBGL_lose_context and cache support for the page lifetime. Old: leaked one context per mount.

  • packages/host-service: Guard project repos with statSync(...).isDirectory() and fix the error message. Old: regular files passed the check.

  • No API or config changes; no migration required.

  • Expect accurate Sentry signal for git failures and no credentials in crash reports; the WebGL decision is cached for the page lifetime.