Skip to content

fix(desktop): Chrome login mirror no longer blocks startup - #545

Merged
lukemaj merged 3 commits into
mainfrom
fix/541-mirror-startup
Oct 19, 2026
Merged

lukemaj merged 3 commits into
mainfrom
fix/541-mirror-startup

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What: Chrome observation is opt-in per Mac, native Chromium key reads run in a worker, and the desktop bundle regression covers every configured worker.
Why: The original mirror could block startup on Keychain consent; its new worker also exposed a stale four-worker bundle-test fixture.
So what: The planner reviews head 2a511fa8e900b28fdc949d6b03305b7348296b59 on current main, checks its new CI, and owns any approved rollout and Rocky live proof.

Refs #541 (closes after the live Rocky startup proof). Refs #520 and #530.

Rebased merge-free onto origin/main at 1e83a3169f77d5dbbd6727c929ffb8b0c1118754. The sole conflict was concurrent appended paths in scripts/fork-upstream-edits.txt; both main's secret/OAuth/research paths and #541's entries are retained. The existing implementation commits otherwise replay unchanged. No version bump.

A nonempty chromeCookieKeyCommand enables the Chrome source on that machine. Missing, invalid and empty commands leave it off; desktop-to-environment mirroring remains available. Shared Chromium key resolution runs native Keychain access in a worker for both one-off imports and the mirror fallback. The argv command already runs in a child process. No protocol, server or web-component behavior changed.

CI repair: Test job 118154631974 failed with [UNRESOLVED_ENTRY] Cannot resolve entry module src/preview/BrowserImport/ChromiumKeychainWorker.ts. The build config declared the new worker, but the synthetic fixture only created four older workers. The Check job merely propagated test: failure; it has no separate defect. The fixture now derives worker paths from the real bundle configuration, creates their nested directories and computes the shared-module import relative to each path. It loads every emitted worker and still asserts desktop startup occurs exactly once. The new touched upstream test is allowlisted and assigned to the existing mirror owner.

Comparison: before repair, the rebased candidate reproduces the same unresolved-entry failure (1 failed / 1 passed). The byte-identical unmodified test and bundle config from 1e83a31 pass (2/2); main's CI also passes. After repair the bundle regression passes 2/2. This CI failure is introduced by this PR, not pre-existing on main.

Final-source proof, all exit 0:

vp test run apps/desktop/scripts/main-process-bundle.test.mjs \
  apps/desktop/src/preview/BrowserImport \
  apps/desktop/src/preview/ChromeCookieKey.test.ts \
  apps/desktop/src/preview/ChromeCookieSource.test.ts \
  apps/desktop/src/preview/CookieMirror.test.ts \
  apps/desktop/src/preview/CookieMirrorMetadata.test.ts \
  apps/desktop/src/preview/CookieMirrorSocket.test.ts \
  apps/desktop/src/preview/DesktopCookieMirror.test.ts \
  apps/desktop/src/preview/DesktopCookieMirror.startup.test.ts \
  apps/desktop/src/settings/DesktopAppSettings.test.ts
# 21 files / 192 tests PASS
vp run --filter @t3tools/desktop typecheck
# PASS, five existing unrelated suggestions
vp run knip:check
# PASS, both file/dependency and exports phases, 0/2 cache hit
(cd apps/desktop && vp pack)
# PASS, existing optional x11/import-meta warnings
scripts/fork-check.sh
node scripts/fork-features.mjs check
git diff --check
# PASS, including merge-free stack and 55 registered features

Scoped vp lint on all 10 changed JavaScript/TypeScript files and vp fmt --check on all 15 supported touched files pass. No web component changed, so React Compiler coverage and screenshots do not apply. Website/SEO impact: none; desktop startup and existing guidance only. The unresolved-key regression uses actual DesktopApp.program and BackendManager with synthetic process/HTTP/window boundaries; native Rocky acceptance remains unverified.

The original upstream check inspected ChromiumKeys blob 0d91ebe5a3746b0e47ddc7da47efa34d3d05681c; upstream #21028 covers a different safeStorage path and no upstream fix was ported. The original three failing-before source/worker regressions and synthetic blocked-worker artifact probe remain recorded in the earlier #541 handoff. This follow-up used existing cached dependencies with an explicit Electron path, with no installs, launches, restarts, dev servers or live Keychain/cookie access. Full Knip now passes without changing dependency declarations; #544's earlier setup-limited findings no longer block this candidate.

Range-diff maps 749e3369e1 to 4be08dfb65 with only allowlist context changed, and 0727249d45 to identical patch 2265c34feb; 2a511fa8e9 adds only the bundle-fixture repair and its fork ownership. Pushed with an explicit lease against remote 0727249d45c20bcb044b80054c835390d2aae7c1.

Opus review/independent success covered the old head 0727249d45, not this new candidate. Fresh independent exact-head review and CI results remain planner-owned. No merge, production artifact, installation or live rollout is claimed; retain the task worktree until the planner merges and retires it.

Requirements and who asked: The planner requests rebase onto current main, resolution of both failing checks, preserved worker/startup isolation, focused proof, vp pack and a lease push.
Deleted: The stale hand-maintained worker fixture list, assumptions that successful production bundling also proved the synthetic bundle test, and a separate fix for the aggregate Check job.
Bottleneck: The synthetic bundle fixture failed before exercising the worker/startup isolation assertion, blocking CI and fleet rollout.
Checked myself: Both complete failed-job logs, rebased failure, main test/config comparison and green CI, corrected bundle test, 192 focused tests, actual vp pack, desktop typecheck, scoped lint/format, full Knip, range-diff and fork checks.

Implemented with GPT-6.1 Sol through the Codex harness.

@github-actions github-actions Bot added size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Oct 19, 2026
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent review at 0727249d45: PASS (Claude Opus 5.5). Grok not available (402 balance exhausted).

  • A fresh install with no key command never selects the Chrome source, reads a key or spawns anything (DesktopCookieMirror.ts:174); the mirror starts after whenReady.
  • Every Keychain read goes through the worker (ChromiumKeys.ts:72-107), for both the mirror key cache and the one-off import (ChromiumCookies.ts:290); the key command is an async child process with a 15 s timeout.
  • vp pack emits dist-electron/preview/BrowserImport/ChromiumKeychainWorker.cjs at the path main.cjs resolves; @napi-rs/keyring stays external.
  • Head passes 7 files / 35 tests; restoring pre-fix ChromiumKeys.ts and DesktopCookieMirror.ts fails 9 tests across 4 files.

Unproven: a worker_threads worker loading the native keyring addon from the signed macOS app; Rocky's live run settles it.
Optional: quitting during a pending one-off import prompt may wait for it; a cancelled import leaves its prompt on screen; the null-command Keychain branch in ChromeCookieKey.ts:51 is now dead.

@lukemaj
lukemaj marked this pull request as ready for review October 19, 2026 15:49
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 1e83a31 · PR result: 2a511fa · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj
lukemaj force-pushed the fix/541-mirror-startup branch from 0727249 to 2a511fa Compare October 19, 2026 22:41
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #545 is rebased onto 1e83a3169f77d5dbbd6727c929ffb8b0c1118754 and lease-pushed at 2a511fa8e900b28fdc949d6b03305b7348296b59.
Why: The stale four-worker synthetic bundle fixture caused Test to fail; Check only aggregated that failure.
So what: The planner reviews the new exact head and its fresh CI, then owns any approved rollout; no installs or restarts were performed in this follow-up.

Current PR body with complete commands, failure logs and comparisons.

One rebase conflict: scripts/fork-upstream-edits.txt. Retained both main's secret/OAuth/research additions and #541's paths. Range-diff: 749e3369e1 -> 4be08dfb65 differs only in allowlist context; 0727249d45 -> 2265c34feb is identical; 2a511fa8e9 adds the fixture repair and fork metadata. No merges or version bump.

Read both failing jobs: Test 118154631974 reports [UNRESOLVED_ENTRY] Cannot resolve entry module src/preview/BrowserImport/ChromiumKeychainWorker.ts; aggregate Check 118158145217 reports test: failure. The fixture now derives worker entries from the actual build configuration, creates nested worker paths and uses the correct relative shared import. Every worker still executes in the bundle harness and the final desktop-startup count must stay exactly one.

Comparison: rebased candidate before repair reproduces 1 failure / 1 pass; the byte-identical test and config from current main pass 2/2, and main CI run 39334964453 is green. Corrected fixture: 2/2 PASS. This is our PR regression, not a pre-existing main defect.

Final-source commands/results:

  • Focused vp test run selection in the PR, now including apps/desktop/scripts/main-process-bundle.test.mjs: 21 files / 192 tests PASS.
  • vp run --filter @t3tools/desktop typecheck: PASS, five existing suggestions.
  • vp lint on all 10 changed JS/TS files: PASS.
  • vp fmt --check on all 15 supported touched files: PASS.
  • vp run knip:check: both phases PASS, 0/2 cache hit; no dependency declarations changed.
  • (cd apps/desktop && vp pack): PASS, existing optional x11/import-meta warnings only.
  • scripts/fork-check.sh, node scripts/fork-features.mjs check, git diff --check: PASS.

Explicit lease was against remote 0727249d45c20bcb044b80054c835390d2aae7c1. The old Opus review status does not cover this new exact SHA. Fresh review/CI, native Rocky acceptance and rollout remain planner-owned. Existing cached dependencies were reused with an explicit Electron path; no installer, app launch/restart, dev server or live Keychain/cookie read was used. Keep this task worktree for review and retirement at merge.

Requirements and who asked: Planner: rebase, fix both failing checks, preserve worker/startup isolation, rerun focused gates and vp pack, and push with a lease.
Deleted: Stale fixture inventory, treating a production bundle pass as synthetic bundle-test proof, and a separate patch for the aggregate Check job.
Bottleneck: Fixture compilation failed before reaching its desktop-startup isolation assertions.
Checked myself: Both failed logs, red/main/green bundle comparison, 192 focused tests, real bundle output, all scoped checks, full Knip, range-diff, fork ownership and lease target.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent review at 2a511fa8e9 (rebased on 1e83a31): PASS (Claude Opus 5.5). Fresh review; the 0727249d45 PASS was not reused.

  • Nothing under apps/desktop changed between e5936c2 and 1e83a31; the diff is the previous candidate plus the bundle-fixture repair.
  • scripts/fork-upstream-edits.txt conflict: additive only, main's entries kept, four fix(desktop): Chrome login mirror hangs startup on Macs that are not the source #541 paths appended.
  • Bundle test oracle: making the Keychain worker fixture import src/main.ts fails with expected [ 42, 42 ] to deeply equal [ 42 ].
  • vp pack emits dist-electron/preview/BrowserImport/ChromiumKeychainWorker.cjs at the path main.cjs resolves; main.cjs has no @napi-rs/keyring reference.
  • Chrome source only with a nonempty key command (DesktopCookieMirror.ts:174); the only native read is in the worker (ChromiumKeys.ts:72-107).
  • 15 files / 138 focused tests pass.

Unproven: native signed-app worker load; the Rocky live run settles it.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost at least $0.18 · 11 responses · 16 sessions · 4.8 h wall time

Model Responses Tokens Estimated cost
grok-4.7 (grok, high) 4 8,279,687 unknown
grok-4.7 (grok, medium) 1 833,703 unknown
gpt-6.1-sol (codex, medium) 2 421,045 $0.07
claude-opus-5-5 (claude, medium) 4 359,208 $0.12

Flags: 2 human corrections · 4 large tool outputs · 8 repeated commands · 4 repeated failures · 19 repeated reads · 9 repeated skill loads · 1 test edited after a failure · 5 unpriced responses

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/chromeria#545: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: fix(desktop): Chrome login mirror no longer blocks startup #545
  • Snapshot 168fe740b1917f85ca82b5400513d83bda9f1312a01e75d36f58eb6e8ffe2ae8, records up to 2026-10-19 22:28 UTC.
  • 16 sessions on claude, codex, grok; AgentsMD 17.4.1.
  • The total includes work shared with 3 other tasks, counted whole in each, so sums across PRs overlap.
  • Not counted: 833 responses ($47.09) in sessions shared with other PRs that worked in no single PR's checkout.
  • 224 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: yes.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-10-18, schedule 414551dda4c47258113dcfa46a5dfa5aa512406e3dfcadcca3381421595c5588. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
    • Model price sources: T3 local rate table (path withheld).
    • Unpriced: unknown model (5).
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

  • grok-4.7 (grok, high), grok counters: input includes cached, output includes reasoning: input 8,141,105; cache-read 7,637,760; cache-write 0; output 138,582; reasoning 122,233; harness total 8,279,687.
  • grok-4.7 (grok, medium), grok counters: input includes cached, output includes reasoning: input 821,203; cache-read 733,696; cache-write 0; output 12,500; reasoning 10,424; harness total 833,703.
  • gpt-6.1-sol (codex, medium), codex counters: input includes cached, output includes reasoning: input 419,535; cache-read 415,232; cache-write 0; output 1,510; reasoning 736; harness total 421,045.
  • claude-opus-5-5 (claude, medium), claude counters: input excludes cache, output includes thinking: input 8; cache-read 355,057; cache-write 3,079; output 1,064; reasoning 119; harness total 359,208.

Generated tokens (reasoning and other output are disjoint; other output includes code, tool calls and replies):

Model Reasoning Other output Reasoning share Split known for
grok-4.7 (grok, high) unknown unknown unknown 0 of 4
grok-4.7 (grok, medium) unknown unknown unknown 0 of 1
gpt-6.1-sol (codex, medium) 736 774 48.7% 2 of 2
claude-opus-5-5 (claude, medium) 119 945 11.2% 4 of 4

Usage by work phase (this task's own responses):

Phase Model Responses Input Cache read Cache write Output Reasoning Total Priced part
unclassified gpt-6.1-sol (codex, medium) 2 419,535 415,232 0 1,510 736 421,045 $0.065229
unclassified claude-opus-5-5 (claude, medium) 4 8 355,057 3,079 1,064 119 359,208 $0.116955

Recorded activity by phase (observations, not separate token bills):

Phase Tool calls MCP results Reads File changes Failed tool results
unclassified 91 3 43 0 18

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning
claude-opus-5-5 all 4 0.2 5m: 5; 1h: 8 20 20
gpt-6.1-sol all 2 0.1 2.5 10 10
grok-4.7 all unknown unknown unknown unknown unknown

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

@lukemaj
lukemaj merged commit 5898397 into main Oct 19, 2026
26 of 27 checks passed
@lukemaj
lukemaj deleted the fix/541-mirror-startup branch October 19, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant