From 2153ef94566588d77efcf57fc0df56f2e34cca04 Mon Sep 17 00:00:00 2001 From: e0_7 <134906805+Daily-AC@users.noreply.github.com> Date: Sat, 4 Jul 2026 03:52:55 +0800 Subject: [PATCH 1/4] Accept any hostname (and optional :port) in manual network address entry (#7223) * Allow arbitrary hostnames in manual network address entry parseManualNetworkAddress only accepted an IPv4 address or a Tailscale MagicDNS (*.ts.net) hostname, so users behind a dynamic residential IP who rely on a DDNS domain or self-hosted relay had no way to enter it in the desktop UI short of bypassing validation via DevTools/IPC. The main process already resolves any host: resolvePairingEndpoint and parsePairingAddressOverride in src/main/runtime/runtime-rpc.ts accept an arbitrary hostname and an optional host:port. This change brings the renderer-side validation in line with what pairing already supports: any RFC 1123 hostname (a superset that still covers *.ts.net), optionally suffixed with :port (1-65535). IPv4 validation is unchanged, including still rejecting malformed dotted-numeric input instead of silently treating it as an all-digit hostname. Updates the custom-address dialog copy in NetworkInterfacePicker.tsx to describe the wider grammar. * Polish manual-address takeover: fix bare-numeric guard, sync 5 locales, lint - Require a dot in the IPv4-typo guard so a bare numeric label (`123`) validates as a legal RFC 1123 hostname, matching the code's own comment and the main-process resolver; add coverage. - Update en.json + es/ja/ko/zh placeholder/hint to the broadened grammar (translate() reads en.json before the TSX fallback, so the copy change was previously inert; the other locales described the old ts.net-only rule). - Replace indexOf(...)!==-1 with includes() to satisfy oxlint. Co-authored-by: Orca * Keep validator a strict subset of the backend resolver Review surfaced two ways the renderer could accept an address the main process handles differently: - All-numeric hosts (bare `123` and dotted `256.0.0.1`) are now rejected. The WHATWG URL host parser downstream reinterprets a numeric host as IPv4 (`123` -> `0.0.0.123`), so accepting one would validate an address the pairing resolver silently dials as a different host. - Ports with leading zeros are rejected. `^[0-9]+$` let an arbitrarily long zero-padded string past the range check and inflate the returned address beyond the hostname length cap that the old whole-string check enforced. Co-authored-by: Orca * Reject any numeric final label, not just fully-numeric hosts WHATWG URL host parsing treats a host whose last label is numeric (`foo.123`, `foo.0x1`) as an IPv4 signal, so the pairing resolver would fail to parse it and silently dial a fallback host. Widen the ambiguous-IP guard to a single last-label check that subsumes the earlier all-numeric case, keeping the renderer a strict subset of what the backend resolves correctly. Normal hostnames whose last label merely contains digits (`host2.example.com`) are unaffected. Co-authored-by: Orca --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca --- .../mobile/NetworkInterfacePicker.tsx | 13 +- src/renderer/src/i18n/locales/en.json | 4 +- src/renderer/src/i18n/locales/es.json | 4 +- src/renderer/src/i18n/locales/ja.json | 4 +- src/renderer/src/i18n/locales/ko.json | 4 +- src/renderer/src/i18n/locales/zh.json | 4 +- src/shared/network/manual-address.test.ts | 115 +++++++++++++++++- src/shared/network/manual-address.ts | 71 +++++++++-- 8 files changed, 187 insertions(+), 32 deletions(-) diff --git a/src/renderer/src/components/mobile/NetworkInterfacePicker.tsx b/src/renderer/src/components/mobile/NetworkInterfacePicker.tsx index a668ccbc555..af45e066a3c 100644 --- a/src/renderer/src/components/mobile/NetworkInterfacePicker.tsx +++ b/src/renderer/src/components/mobile/NetworkInterfacePicker.tsx @@ -6,10 +6,11 @@ import type { MobileNetworkInterface } from '../settings/mobile-network-interfac // Why: MobileHero (mobile pairing screen) and MobileNetworkInterfaceSection // (Settings → Mobile) both need the same network selector. This wraps the -// generic AddressPicker with the mobile grammar (IPv4 / Tailscale *.ts.net) -// and copy. Discovered interfaces come from the OS; "Add custom address…" -// opens a dialog for an address the OS didn't surface — the only way to pair -// across networks. +// generic AddressPicker with the mobile grammar (IPv4, any RFC 1123 +// hostname — including Tailscale *.ts.net and DDNS domains — optionally +// with :port) and copy. Discovered interfaces come from the OS; "Add custom +// address…" opens a dialog for an address the OS didn't surface — the only +// way to pair across networks. export type NetworkInterfacePickerProps = { networkInterfaces: readonly MobileNetworkInterface[] @@ -81,11 +82,11 @@ export function NetworkInterfacePicker({ inputLabel: translate('auto.components.mobile.CustomNetworkAddressDialog.label', 'Address'), placeholder: translate( 'auto.components.mobile.CustomNetworkAddressDialog.placeholder', - 'my-mac.ts.net or 192.168.1.50' + 'my-mac.ts.net, home.example.com, or 192.168.1.50' ), hint: translate( 'auto.components.mobile.CustomNetworkAddressDialog.hint', - 'Enter an IP address or a Tailscale hostname (ends in .ts.net).' + 'Enter an IP address or a hostname — a Tailscale MagicDNS name, a DDNS domain, or a LAN hostname — optionally with :port.' ), cancel: translate('auto.components.mobile.CustomNetworkAddressDialog.cancel', 'Cancel'), confirm: translate('auto.components.mobile.CustomNetworkAddressDialog.use', 'Use address') diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 5611d8d73ce..32395a56253 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -10216,8 +10216,8 @@ "title": "Custom network address", "description": "Advertise an address your phone can reach when it is not on the same Wi-Fi — for example a Tailscale hostname or a static IP.", "label": "Address", - "placeholder": "my-mac.ts.net or 192.168.1.50", - "hint": "Enter an IP address or a Tailscale hostname (ends in .ts.net).", + "placeholder": "my-mac.ts.net, home.example.com, or 192.168.1.50", + "hint": "Enter an IP address or a hostname — a Tailscale MagicDNS name, a DDNS domain, or a LAN hostname — optionally with :port.", "cancel": "Cancel", "use": "Use address" }, diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index ebca92c506e..ea305d3abb7 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -10216,8 +10216,8 @@ "title": "Dirección de red personalizada", "description": "Anuncia una dirección que tu teléfono pueda alcanzar cuando no esté en la misma red Wi-Fi: por ejemplo, un nombre de host de Tailscale o una IP estática.", "label": "Dirección", - "placeholder": "my-mac.ts.net o 192.168.1.50", - "hint": "Introduce una dirección IP o un nombre de host de Tailscale (termina en .ts.net).", + "placeholder": "my-mac.ts.net, home.example.com o 192.168.1.50", + "hint": "Introduce una dirección IP o un nombre de host (un nombre MagicDNS de Tailscale, un dominio DDNS o un nombre de host de LAN), opcionalmente con :puerto.", "cancel": "Cancelar", "use": "Usar dirección" }, diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index c02e6f709d8..fc9795ca8c0 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -10216,8 +10216,8 @@ "title": "カスタムネットワークアドレス", "description": "同じ Wi-Fi に接続していないときにスマートフォンから到達できるアドレスを指定します。たとえば Tailscale のホスト名や静的 IP です。", "label": "アドレス", - "placeholder": "my-mac.ts.net または 192.168.1.50", - "hint": "IP アドレスまたは Tailscale のホスト名（.ts.net で終わる）を入力してください。", + "placeholder": "my-mac.ts.net、home.example.com、または 192.168.1.50", + "hint": "IP アドレスまたはホスト名（Tailscale の MagicDNS 名、DDNS ドメイン、LAN のホスト名など）を入力してください。必要に応じて :ポート を付けられます。", "cancel": "キャンセル", "use": "アドレスを使用" }, diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index ce2b4283fcc..5ab1eade73a 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -10216,8 +10216,8 @@ "title": "사용자 지정 네트워크 주소", "description": "같은 Wi-Fi에 있지 않을 때 휴대폰이 연결할 수 있는 주소를 알립니다. 예: Tailscale 호스트 이름 또는 고정 IP.", "label": "주소", - "placeholder": "my-mac.ts.net 또는 192.168.1.50", - "hint": "IP 주소 또는 Tailscale 호스트 이름(.ts.net으로 끝남)을 입력하세요.", + "placeholder": "my-mac.ts.net, home.example.com 또는 192.168.1.50", + "hint": "IP 주소 또는 호스트 이름(Tailscale MagicDNS 이름, DDNS 도메인 또는 LAN 호스트 이름)을 입력하세요. 필요하면 :포트를 붙일 수 있습니다.", "cancel": "취소", "use": "주소 사용" }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 719e1e9856b..5f9fff50376 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -10216,8 +10216,8 @@ "title": "自定义网络地址", "description": "指定当手机不在同一 Wi-Fi 时可以访问的地址，例如 Tailscale 主机名或静态 IP。", "label": "地址", - "placeholder": "my-mac.ts.net 或 192.168.1.50", - "hint": "请输入 IP 地址或 Tailscale 主机名（以 .ts.net 结尾）。", + "placeholder": "my-mac.ts.net、home.example.com 或 192.168.1.50", + "hint": "请输入 IP 地址或主机名（Tailscale MagicDNS 名称、DDNS 域名或 LAN 主机名），可选择附加 :端口。", "cancel": "取消", "use": "使用地址" }, diff --git a/src/shared/network/manual-address.test.ts b/src/shared/network/manual-address.test.ts index 3d1b48b43b2..fe6c1950f68 100644 --- a/src/shared/network/manual-address.test.ts +++ b/src/shared/network/manual-address.test.ts @@ -19,11 +19,18 @@ describe('parseManualNetworkAddress', () => { expect(parseManualNetworkAddress('255.255.255.255').ok).toBe(true) }) + it('accepts an IPv4 address with a port suffix', () => { + expect(parseManualNetworkAddress('192.168.1.24:8080')).toEqual({ + ok: true, + address: '192.168.1.24:8080' + }) + }) + it('rejects malformed IPv4', () => { for (const bad of ['', ' ', '1.2.3', '1.2.3.4.5', '256.0.0.1']) { expect(parseManualNetworkAddress(bad)).toEqual({ ok: false, - error: 'Enter an IPv4 address or Tailscale MagicDNS hostname' + error: 'Enter an IPv4 address or hostname, optionally with a :port suffix' }) } }) @@ -31,7 +38,7 @@ describe('parseManualNetworkAddress', () => { it('rejects leading zeros in octets', () => { expect(parseManualNetworkAddress('01.02.03.04')).toEqual({ ok: false, - error: 'Enter an IPv4 address or Tailscale MagicDNS hostname' + error: 'Enter an IPv4 address or hostname, optionally with a :port suffix' }) expect(parseManualNetworkAddress('0.0.0.0').ok).toBe(true) }) @@ -57,11 +64,107 @@ describe('parseManualNetworkAddress', () => { expect(parseManualNetworkAddress('MY-MAC.TS.NET').ok).toBe(true) }) - it('rejects non-Tailscale hostnames', () => { - for (const bad of ['my-mac', 'my-mac.ts.com', '-foo.ts.net', 'my-mac.com']) { + it('rejects a MagicDNS-shaped hostname with a malformed label', () => { + // Leading/trailing hyphens are invalid in any RFC 1123 label, + // regardless of the `.ts.net` suffix. + expect(parseManualNetworkAddress('-foo.ts.net').ok).toBe(false) + }) + }) + + describe('arbitrary hostnames (DDNS / self-hosted domains)', () => { + it('accepts a bare single-label hostname', () => { + expect(parseManualNetworkAddress('my-nas')).toEqual({ + ok: true, + address: 'my-nas' + }) + }) + + it('accepts a multi-label domain, e.g. a DDNS hostname', () => { + expect(parseManualNetworkAddress('home.example.com')).toEqual({ + ok: true, + address: 'home.example.com' + }) + expect(parseManualNetworkAddress('my-house.ddns.net').ok).toBe(true) + }) + + it('accepts a hostname with a port suffix', () => { + expect(parseManualNetworkAddress('home.example.com:8443')).toEqual({ + ok: true, + address: 'home.example.com:8443' + }) + }) + + it('is case-insensitive', () => { + expect(parseManualNetworkAddress('HOME.EXAMPLE.COM').ok).toBe(true) + }) + + it('rejects labels with leading or trailing hyphens', () => { + for (const bad of ['-foo.example.com', 'foo-.example.com', 'foo..com']) { expect(parseManualNetworkAddress(bad).ok).toBe(false) } }) + + it('rejects hostnames with characters outside the RFC 1123 grammar', () => { + for (const bad of ['my_host.example.com', 'my host.example.com', 'example.com/path']) { + expect(parseManualNetworkAddress(bad).ok).toBe(false) + } + }) + + it('rejects a host whose last label is numeric as an ambiguous IPv4', () => { + // The WHATWG URL parser downstream treats "ends in a number" as an IPv4 + // signal and would reinterpret or fail to resolve these (`123` -> + // `0.0.0.123`; `foo.123` -> no-op fallback), so accepting one here would + // validate an address the main process dials differently. + const bad = [ + '123', + '123:8080', + '1.2.3', + '1.2.3.4.5', + '256.0.0.1', + '999.999.999.999', + 'foo.123', + 'foo.123:8080', + 'foo.0x1' + ] + for (const value of bad) { + expect(parseManualNetworkAddress(value).ok).toBe(false) + } + }) + + it('still accepts a hostname whose last label merely contains digits', () => { + // Only a fully-numeric (or hex) final label is ambiguous; a label like + // `com` or `nas1` is a normal hostname. + expect(parseManualNetworkAddress('host2.example.com').ok).toBe(true) + expect(parseManualNetworkAddress('my-nas1').ok).toBe(true) + }) + }) + + describe('port suffix', () => { + it('accepts boundary port values', () => { + expect(parseManualNetworkAddress('example.com:1').ok).toBe(true) + expect(parseManualNetworkAddress('example.com:65535').ok).toBe(true) + }) + + it('rejects out-of-range ports', () => { + expect(parseManualNetworkAddress('example.com:0').ok).toBe(false) + expect(parseManualNetworkAddress('example.com:65536').ok).toBe(false) + expect(parseManualNetworkAddress('example.com:99999').ok).toBe(false) + }) + + it('rejects a non-numeric or empty port', () => { + expect(parseManualNetworkAddress('example.com:abc').ok).toBe(false) + expect(parseManualNetworkAddress('example.com:').ok).toBe(false) + }) + + it('rejects a leading-zero (non-canonical, unbounded-length) port', () => { + expect(parseManualNetworkAddress('example.com:0080').ok).toBe(false) + expect(parseManualNetworkAddress(`example.com:${'0'.repeat(1000)}8080`).ok).toBe(false) + }) + + it('rejects addresses with more than one colon (e.g. IPv6-shaped input)', () => { + expect(parseManualNetworkAddress('example.com:80:90').ok).toBe(false) + expect(parseManualNetworkAddress('::1').ok).toBe(false) + }) }) describe('length and whitespace', () => { @@ -77,5 +180,9 @@ describe('parseManualNetworkAddress', () => { address: '192.168.1.24' }) }) + + it('rejects whitespace inside the address', () => { + expect(parseManualNetworkAddress('my host.example.com').ok).toBe(false) + }) }) }) diff --git a/src/shared/network/manual-address.ts b/src/shared/network/manual-address.ts index 9f6a8421f78..81a8a28a26b 100644 --- a/src/shared/network/manual-address.ts +++ b/src/shared/network/manual-address.ts @@ -1,36 +1,83 @@ // Why: pure shared helper so the same validation runs in renderer // today and in any future CLI/main-process caller without duplicating -// the IPv4 + Tailscale MagicDNS grammar. +// the IPv4 + hostname + optional-port grammar. const IPV4_OCTET = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])' const IPV4 = `(?:${IPV4_OCTET}\\.){3}${IPV4_OCTET}` -// MagicDNS hostname: lowercase letters/digits/hyphens, dot-separated, ending in .ts.net. -// Labels may not start or end with a hyphen; max 63 chars per label (DNS limit). -const MAGICDNS_LABEL = '[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?' -const MAGICDNS = `(?:${MAGICDNS_LABEL}\\.)+ts\\.net` +const IPV4_REGEX = new RegExp(`^${IPV4}$`) + +// RFC 1123 hostname label: letters/digits/hyphens, 1-63 chars, may not +// start or end with a hyphen. This covers plain LAN hostnames, DDNS domains +// (e.g. `home.example.com`), and Tailscale MagicDNS names (`*.ts.net`) as a +// special case of the same grammar — no separate MagicDNS-only pattern needed. +const HOSTNAME_LABEL = '[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?' +const HOSTNAME = `(?:${HOSTNAME_LABEL}\\.)*${HOSTNAME_LABEL}` +const HOSTNAME_REGEX = new RegExp(`^${HOSTNAME}$`, 'i') const HOSTNAME_MAX_LENGTH = 253 -const ERROR_MESSAGE = 'Enter an IPv4 address or Tailscale MagicDNS hostname' +const MIN_PORT = 1 +const MAX_PORT = 65535 +const ERROR_MESSAGE = 'Enter an IPv4 address or hostname, optionally with a :port suffix' export type ParseManualAddressResult = { ok: true; address: string } | { ok: false; error: string } export function parseManualNetworkAddress(input: string): ParseManualAddressResult { const trimmed = input.trim() - if (trimmed === '' || trimmed.length > HOSTNAME_MAX_LENGTH) { + if (trimmed === '') { return { ok: false, error: ERROR_MESSAGE } } if (/\s/.test(trimmed)) { return { ok: false, error: ERROR_MESSAGE } } - const ipv4Regex = new RegExp(`^${IPV4}$`) - if (ipv4Regex.test(trimmed)) { - return { ok: true, address: trimmed } + const { host, port } = splitHostPort(trimmed) + if (host === '' || host.length > HOSTNAME_MAX_LENGTH) { + return { ok: false, error: ERROR_MESSAGE } + } + if (port !== null && !isValidPort(port)) { + return { ok: false, error: ERROR_MESSAGE } } - const magicRegex = new RegExp(`^(?:${MAGICDNS})$`, 'i') - if (magicRegex.test(trimmed)) { + if (IPV4_REGEX.test(host)) { + return { ok: true, address: trimmed } + } + // Why: reject a host whose last label is numeric — a bare `123`, a dotted + // `256.0.0.1`, or a hostname with a numeric final label (`foo.123`, `foo.0x1`). + // The WHATWG URL host parser that `resolvePairingEndpoint` feeds this into + // treats "ends in a number" as an IPv4 signal and tries to parse the whole + // host as IPv4. A real IPv4 was already accepted above, so anything reaching + // here would fail that parse and the main process would silently dial a + // different host — so treat these as mistyped IPs, not hostnames. + const lastLabel = host.split('.').at(-1) ?? '' + if (/^[0-9]+$/.test(lastLabel) || /^0x[0-9a-f]*$/i.test(lastLabel)) { + return { ok: false, error: ERROR_MESSAGE } + } + if (HOSTNAME_REGEX.test(host)) { return { ok: true, address: trimmed } } return { ok: false, error: ERROR_MESSAGE } } + +// Why: mirrors `parsePairingAddressOverride` in src/main/runtime/runtime-rpc.ts +// so the UI only accepts what the main process's pairing endpoint resolution +// can already handle. IPv6 stays out of scope (same as that function), so a +// second colon is left in `host` and fails the grammar checks below instead +// of being misparsed as a port. +function splitHostPort(value: string): { host: string; port: string | null } { + const firstColon = value.indexOf(':') + if (firstColon === -1 || value.includes(':', firstColon + 1)) { + return { host: value, port: null } + } + return { host: value.slice(0, firstColon), port: value.slice(firstColon + 1) } +} + +function isValidPort(port: string): boolean { + // Reject leading zeros: a canonical port has none, and allowing them lets an + // arbitrarily long zero-padded string (`00…08080`) slip past the range check + // and inflate the returned address past the hostname length cap. + if (!/^[1-9][0-9]*$/.test(port)) { + return false + } + const value = Number(port) + return value >= MIN_PORT && value <= MAX_PORT +} From 9aeaf0728f6e6173b969e473c63a785b37dda182 Mon Sep 17 00:00:00 2001 From: yukei_lee Date: Sat, 4 Jul 2026 03:56:38 +0800 Subject: [PATCH 2/4] Fix git worktree list fallback for older Git (#7235) * fix git worktree list fallback for localized errors * Polish worktree -z fallback: clarify comment, add localized + negative tests Co-authored-by: Orca * Extend -z fallback to relay/SSH worktree path; tighten comments; add relay tests Co-authored-by: Orca * Isolate regex-fallback path in relay test Co-authored-by: Orca --------- Co-authored-by: yuqili03 Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca --- src/main/git/worktree.test.ts | 98 +++++++++++++++++++++++++++++ src/main/git/worktree.ts | 10 ++- src/relay/git-handler-utils.test.ts | 30 +++++++++ src/relay/git-handler-utils.ts | 16 ++++- 4 files changed, 148 insertions(+), 6 deletions(-) diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 44679c14ed3..69ce1ed46c5 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -237,6 +237,12 @@ bare }) describe('listWorktreeGraph', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + gitExecFileSyncMock.mockReset() + translateWslOutputPathsMock.mockClear() + }) + it('returns the worktree graph without sparse-checkout annotation probes', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: `worktree /repo @@ -272,6 +278,98 @@ branch refs/heads/feature/test }) }) + it('falls back when older Git rejects worktree list -z with usage exit code', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce( + Object.assign(new Error('git usage error'), { + code: 129, + stderr: 'usage: git worktree list []\n' + }) + ) + .mockResolvedValueOnce({ + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-main-2 +HEAD def456 +branch refs/heads/main-2 +` + }) + + await expect(listWorktreeGraph('/repo')).resolves.toEqual([ + { + path: '/repo', + head: 'abc123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/repo-main-2', + head: 'def456', + branch: 'refs/heads/main-2', + isBare: false, + isMainWorktree: false + } + ]) + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }], + [['worktree', 'list', '--porcelain'], { cwd: '/repo' }] + ]) + }) + + it('falls back on a localized (non-English) older-Git usage error via exit code 129', async () => { + // Git under a non-English locale translates the usage text, so stderr + // matching alone misses it; the 129 exit code must still trigger fallback. + gitExecFileAsyncMock + .mockRejectedValueOnce( + Object.assign(new Error('git worktree list --porcelain -z'), { + code: 129, + stderr: 'Fehler: Unbekannter Schalter »z«\nAufruf: git worktree list []\n' + }) + ) + .mockResolvedValueOnce({ + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main +` + }) + + await expect(listWorktreeGraph('/repo')).resolves.toEqual([ + { + path: '/repo', + head: 'abc123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }], + [['worktree', 'list', '--porcelain'], { cwd: '/repo' }] + ]) + }) + + it('does not retry without -z for non-usage Git failures', async () => { + // A fatal error (exit 128) is not an unsupported-flag signal, so the -z + // command must not be silently re-run without it. + gitExecFileAsyncMock.mockRejectedValueOnce( + Object.assign(new Error('git worktree list --porcelain -z'), { + code: 128, + stderr: 'fatal: unable to read tree\n' + }) + ) + + await expect(listWorktreeGraph('/repo')).resolves.toEqual([]) + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }] + ]) + }) + it('returns an empty graph for paths Git reports as non-repositories', async () => { gitExecFileAsyncMock.mockRejectedValueOnce(new Error('fatal: not a git repository')) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index ed830d06d70..1dce3fff2ae 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -104,9 +104,13 @@ function isNotGitRepositoryError(error: unknown): boolean { } function isUnsupportedWorktreeListZError(error: unknown): boolean { - return /(?:unknown|invalid) (?:switch|option).*`?-z'?|(?:unknown|invalid) (?:switch|option).*`?z'?/i.test( - getErrorText(error) - ) + // `-z` is this command's only flag older Git (<2.36) lacks, so its usage + // exit 129 signals the rejection in any locale; stderr match is a fallback. + if (getErrorCode(error) === '129') { + return true + } + + return /(?:unknown|invalid|unrecognized) (?:switch|option).*`?-?z'?/i.test(getErrorText(error)) } function isUnsupportedRevParsePathFormatError(error: unknown): boolean { diff --git a/src/relay/git-handler-utils.test.ts b/src/relay/git-handler-utils.test.ts index 9dca8777834..03d0fe14aed 100644 --- a/src/relay/git-handler-utils.test.ts +++ b/src/relay/git-handler-utils.test.ts @@ -1,5 +1,35 @@ import { describe, expect, it } from 'vitest' import { parseStatusOutput } from './git-status-output-parser' +import { isUnsupportedWorktreeListZError } from './git-handler-utils' + +describe('isUnsupportedWorktreeListZError', () => { + it('detects an unknown-switch usage error from stderr when the exit code is absent', () => { + // Isolates the regex fallback: no numeric code, so only the stderr text + // (a runner that dropped the exit code) can classify the rejection. + const error = Object.assign(new Error('worktree list -z'), { + stderr: "error: unknown switch `z'\nusage: git worktree list []\n" + }) + expect(isUnsupportedWorktreeListZError(error)).toBe(true) + }) + + it('detects a localized (non-English) usage error via exit code 129', () => { + // The SSH remote may run under a non-English locale where the stderr text is + // translated; the numeric exit code must still classify the -z rejection. + const error = Object.assign(new Error('worktree list -z'), { + code: 129, + stderr: 'Fehler: Unbekannter Schalter »z«\nAufruf: git worktree list []\n' + }) + expect(isUnsupportedWorktreeListZError(error)).toBe(true) + }) + + it('does not classify a fatal (exit 128) error as an unsupported -z rejection', () => { + const error = Object.assign(new Error('fatal'), { + code: 128, + stderr: 'fatal: unable to read tree\n' + }) + expect(isUnsupportedWorktreeListZError(error)).toBe(false) + }) +}) describe('parseStatusOutput', () => { it('parses upstream ahead/behind from porcelain v2 branch headers', () => { diff --git a/src/relay/git-handler-utils.ts b/src/relay/git-handler-utils.ts index 5ddd15fd896..f0a395da27b 100644 --- a/src/relay/git-handler-utils.ts +++ b/src/relay/git-handler-utils.ts @@ -147,10 +147,20 @@ function getErrorText(error: unknown): string { return String(error) } +function getErrorCode(error: unknown): string | undefined { + return typeof error === 'object' && error !== null && 'code' in error + ? String((error as { code?: unknown }).code) + : undefined +} + export function isUnsupportedWorktreeListZError(error: unknown): boolean { - return /(?:unknown|invalid) (?:switch|option).*`?-z'?|(?:unknown|invalid) (?:switch|option).*`?z'?/i.test( - getErrorText(error) - ) + // `-z` is this command's only flag older Git (<2.36) lacks, so its usage exit + // 129 signals the rejection in any locale; key for SSH remotes on old Git. + if (getErrorCode(error) === '129') { + return true + } + + return /(?:unknown|invalid|unrecognized) (?:switch|option).*`?-?z'?/i.test(getErrorText(error)) } export function parseWorktreeList( From 273fee79fa3b37e1938e5f3a0ec1ffcad0b61184 Mon Sep 17 00:00:00 2001 From: Avichal Dwivedi <150160561+AvichalDwivedi2205@users.noreply.github.com> Date: Sat, 4 Jul 2026 01:36:43 +0530 Subject: [PATCH 3/4] Fix CDP Page.printToPDF support (#7044) * Fix CDP printToPDF proxy * Harden CDP PDF stream handling * Fix PDF margin units * refactor(cdp): extract printToPDF param mapping + stream store into cdp-print-to-pdf Move the CDP Page.printToPDF option translation and the ReturnAsStream handle store out of cdp-ws-proxy into a focused cdp-print-to-pdf module, mirroring the existing cdp-screenshot sibling. Keeps the over-budget proxy from growing further and gives the PDF logic direct unit coverage. No behavior change: all existing proxy tests pass unmodified. Co-authored-by: Orca * fix(cdp): drop print result if client disconnected during printToPDF A ReturnAsStream print that resolved after the requesting client disconnected (or was replaced) registered its buffer in pdfStreams after the disconnect's clear() already ran, orphaning a potentially large PDF until the 5-minute TTL. Re-check isActiveClient after the await, mirroring navigateWithLifecycle/reloadWithLifecycle. Co-authored-by: Orca --------- Co-authored-by: Neil Co-authored-by: Orca --- src/main/browser/cdp-print-to-pdf.test.ts | 172 +++++++++++++++++ src/main/browser/cdp-print-to-pdf.ts | 177 ++++++++++++++++++ src/main/browser/cdp-ws-proxy.test.ts | 213 ++++++++++++++++++++++ src/main/browser/cdp-ws-proxy.ts | 79 ++++++++ 4 files changed, 641 insertions(+) create mode 100644 src/main/browser/cdp-print-to-pdf.test.ts create mode 100644 src/main/browser/cdp-print-to-pdf.ts diff --git a/src/main/browser/cdp-print-to-pdf.test.ts b/src/main/browser/cdp-print-to-pdf.test.ts new file mode 100644 index 00000000000..e6f2163b5dd --- /dev/null +++ b/src/main/browser/cdp-print-to-pdf.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, vi, afterEach } from 'vitest' +import { buildPrintToPdfOptions, CdpPdfStreamStore } from './cdp-print-to-pdf' + +const DEFAULT_MARGIN_INCHES = 1 / 2.54 +const TTL_MS = 5 * 60 * 1000 + +describe('buildPrintToPdfOptions', () => { + it('returns empty options for empty params', () => { + expect(buildPrintToPdfOptions({})).toEqual({}) + }) + + it('maps every well-formed field', () => { + expect( + buildPrintToPdfOptions({ + landscape: true, + displayHeaderFooter: true, + printBackground: true, + preferCSSPageSize: true, + generateTaggedPDF: true, + generateDocumentOutline: false, + scale: 1.5, + paperWidth: 8.5, + paperHeight: 11, + marginTop: 0.25, + marginBottom: 0.5, + marginLeft: 0.75, + marginRight: 1, + pageRanges: '1-2', + headerTemplate: '', + footerTemplate: '' + }) + ).toEqual({ + landscape: true, + displayHeaderFooter: true, + printBackground: true, + preferCSSPageSize: true, + generateTaggedPDF: true, + generateDocumentOutline: false, + scale: 1.5, + pageSize: { width: 8.5, height: 11 }, + margins: { marginType: 'custom', top: 0.25, bottom: 0.5, left: 0.75, right: 1 }, + pageRanges: '1-2', + headerTemplate: '', + footerTemplate: '' + }) + }) + + it('defaults omitted margin sides to 1cm when any side is set', () => { + expect(buildPrintToPdfOptions({ marginTop: 0.25 }).margins).toEqual({ + marginType: 'custom', + top: 0.25, + bottom: DEFAULT_MARGIN_INCHES, + left: DEFAULT_MARGIN_INCHES, + right: DEFAULT_MARGIN_INCHES + }) + }) + + it('drops non-finite and out-of-range numeric options', () => { + const options = buildPrintToPdfOptions({ + scale: 0, + paperWidth: Number.POSITIVE_INFINITY, + paperHeight: 11, + marginTop: Number.NaN + }) + expect(options.scale).toBeUndefined() + expect(options.pageSize).toBeUndefined() + expect(options.margins).toBeUndefined() + }) + + it('requires both paper dimensions for pageSize', () => { + expect(buildPrintToPdfOptions({ paperWidth: 8.5 }).pageSize).toBeUndefined() + }) + + it('ignores wrong-typed options', () => { + expect(buildPrintToPdfOptions({ landscape: 'yes', pageRanges: 7 })).toEqual({}) + }) +}) + +describe('CdpPdfStreamStore', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('claims only its own handles', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('pdf')) + + expect(store.ownsHandle({ handle })).toBe(true) + expect(handle).toMatch(/^orca-pdf-[\da-f-]{36}-\d+$/) + expect(store.ownsHandle({ handle: 'trace-stream' })).toBe(false) + expect(store.ownsHandle({ handle: 'orca-pdf-forged-1' })).toBe(false) + expect(store.ownsHandle({})).toBe(false) + }) + + it('mints distinct handles per stream', () => { + const store = new CdpPdfStreamStore() + expect(store.create(Buffer.from('a'))).not.toBe(store.create(Buffer.from('b'))) + }) + + it('reads sequential chunks and reports eof', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abcdef')) + + expect(store.read({ handle, size: 2 })).toEqual({ + data: Buffer.from('ab').toString('base64'), + eof: false + }) + expect(store.read({ handle })).toEqual({ + data: Buffer.from('cdef').toString('base64'), + eof: true + }) + }) + + it('honors an explicit read offset', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abcdef')) + + expect(store.read({ handle, offset: 4 })).toEqual({ + data: Buffer.from('ef').toString('base64'), + eof: true + }) + }) + + it('returns an empty eof chunk when reading past the end', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abc')) + + expect(store.read({ handle, offset: 99 })).toEqual({ data: '', eof: true }) + }) + + it('returns null for unknown handles', () => { + const store = new CdpPdfStreamStore() + expect(store.read({ handle: 'nope' })).toBeNull() + expect(store.read({})).toBeNull() + }) + + it('drops a stream on close', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abc')) + + store.close({ handle }) + expect(store.read({ handle })).toBeNull() + }) + + it('drops all streams on clear', () => { + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abc')) + + store.clear() + expect(store.read({ handle })).toBeNull() + }) + + it('evicts an abandoned stream after the TTL', () => { + vi.useFakeTimers() + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abc')) + + vi.advanceTimersByTime(TTL_MS + 1) + expect(store.read({ handle })).toBeNull() + }) + + it('refreshes the TTL on each read', () => { + vi.useFakeTimers() + const store = new CdpPdfStreamStore() + const handle = store.create(Buffer.from('abcdef')) + + vi.advanceTimersByTime(TTL_MS - 1) + expect(store.read({ handle, size: 1 })).not.toBeNull() + vi.advanceTimersByTime(TTL_MS - 1) + expect(store.read({ handle, size: 1 })).not.toBeNull() + }) +}) diff --git a/src/main/browser/cdp-print-to-pdf.ts b/src/main/browser/cdp-print-to-pdf.ts new file mode 100644 index 00000000000..643013377d8 --- /dev/null +++ b/src/main/browser/cdp-print-to-pdf.ts @@ -0,0 +1,177 @@ +import { randomUUID } from 'node:crypto' +import type { PrintToPDFOptions } from 'electron' + +const PDF_DEFAULT_MARGIN_INCHES = 1 / 2.54 +const PDF_STREAM_CHUNK_BYTES = 1024 * 1024 +const PDF_STREAM_HANDLE_PREFIX = 'orca-pdf-' +const PDF_STREAM_TTL_MS = 5 * 60 * 1000 + +function finiteNumber(value: unknown): number | null { + return typeof value === 'number' && Number.isFinite(value) ? value : null +} + +/** + * Translate CDP `Page.printToPDF` params into Electron `printToPDF` options. + * Only well-formed values are forwarded so a malformed param can never smuggle + * NaN/Infinity into Electron; omitted margin sides default to CDP's 1cm. + */ +export function buildPrintToPdfOptions(params: Record): PrintToPDFOptions { + const options: PrintToPDFOptions = {} + + if (typeof params.landscape === 'boolean') { + options.landscape = params.landscape + } + if (typeof params.displayHeaderFooter === 'boolean') { + options.displayHeaderFooter = params.displayHeaderFooter + } + if (typeof params.printBackground === 'boolean') { + options.printBackground = params.printBackground + } + if (typeof params.preferCSSPageSize === 'boolean') { + options.preferCSSPageSize = params.preferCSSPageSize + } + if (typeof params.generateTaggedPDF === 'boolean') { + options.generateTaggedPDF = params.generateTaggedPDF + } + if (typeof params.generateDocumentOutline === 'boolean') { + options.generateDocumentOutline = params.generateDocumentOutline + } + + const scale = finiteNumber(params.scale) + if (scale !== null && scale > 0) { + options.scale = scale + } + + const paperWidth = finiteNumber(params.paperWidth) + const paperHeight = finiteNumber(params.paperHeight) + if (paperWidth !== null && paperHeight !== null && paperWidth > 0 && paperHeight > 0) { + options.pageSize = { width: paperWidth, height: paperHeight } + } + + const marginTop = finiteNumber(params.marginTop) + const marginBottom = finiteNumber(params.marginBottom) + const marginLeft = finiteNumber(params.marginLeft) + const marginRight = finiteNumber(params.marginRight) + if ([marginTop, marginBottom, marginLeft, marginRight].some((margin) => margin !== null)) { + // CDP and Electron printToPDF both use inches; omitted CDP sides default to 1cm. + options.margins = { + marginType: 'custom', + top: marginTop ?? PDF_DEFAULT_MARGIN_INCHES, + bottom: marginBottom ?? PDF_DEFAULT_MARGIN_INCHES, + left: marginLeft ?? PDF_DEFAULT_MARGIN_INCHES, + right: marginRight ?? PDF_DEFAULT_MARGIN_INCHES + } + } + + if (typeof params.pageRanges === 'string') { + options.pageRanges = params.pageRanges + } + if (typeof params.headerTemplate === 'string') { + options.headerTemplate = params.headerTemplate + } + if (typeof params.footerTemplate === 'string') { + options.footerTemplate = params.footerTemplate + } + + return options +} + +type PdfStream = { + data: Buffer + offset: number + cleanupTimer: ReturnType +} + +export type PdfStreamChunk = { + data: string + eof: boolean +} + +/** + * Holds the PDF buffers produced for CDP `transferMode: "ReturnAsStream"` and + * serves them back through `IO.read` / `IO.close`, the path Playwright's + * `page.pdf()` uses. Handles carry a per-instance random prefix so a CDP client + * cannot forge one or collide with a real Chromium IO stream, and each buffer is + * evicted after a TTL so an abandoned stream can never leak memory. + */ +export class CdpPdfStreamStore { + private readonly streams = new Map() + private readonly handlePrefix = `${PDF_STREAM_HANDLE_PREFIX}${randomUUID()}-` + private nextId = 0 + + /** True when `params.handle` names one of this store's streams. */ + ownsHandle(params: Record): boolean { + return typeof params.handle === 'string' && params.handle.startsWith(this.handlePrefix) + } + + create(data: Buffer): string { + const handle = `${this.handlePrefix}${++this.nextId}` + this.streams.set(handle, { + data, + offset: 0, + cleanupTimer: this.scheduleCleanup(handle) + }) + return handle + } + + /** Read the next chunk, or `null` if the handle is unknown/expired. */ + read(params: Record): PdfStreamChunk | null { + const handle = typeof params.handle === 'string' ? params.handle : '' + const stream = this.streams.get(handle) + if (!stream) { + return null + } + this.refreshCleanup(handle, stream) + + const offset = finiteNumber(params.offset) + if (offset !== null) { + stream.offset = Math.max(0, Math.floor(offset)) + } + const requestedSize = finiteNumber(params.size) + const size = + requestedSize !== null && requestedSize > 0 + ? Math.floor(requestedSize) + : PDF_STREAM_CHUNK_BYTES + const start = Math.min(stream.offset, stream.data.length) + const end = Math.min(start + size, stream.data.length) + const chunk = stream.data.subarray(start, end) + stream.offset = end + + return { data: chunk.toString('base64'), eof: end >= stream.data.length } + } + + close(params: Record): void { + const handle = typeof params.handle === 'string' ? params.handle : '' + this.delete(handle) + } + + clear(): void { + for (const stream of this.streams.values()) { + clearTimeout(stream.cleanupTimer) + } + this.streams.clear() + } + + private scheduleCleanup(handle: string): ReturnType { + const cleanupTimer = setTimeout(() => { + this.delete(handle) + }, PDF_STREAM_TTL_MS) + const maybeNodeTimer = cleanupTimer as { unref?: () => void } + maybeNodeTimer.unref?.() + return cleanupTimer + } + + private refreshCleanup(handle: string, stream: PdfStream): void { + clearTimeout(stream.cleanupTimer) + stream.cleanupTimer = this.scheduleCleanup(handle) + } + + private delete(handle: string): void { + const stream = this.streams.get(handle) + if (!stream) { + return + } + clearTimeout(stream.cleanupTimer) + this.streams.delete(handle) + } +} diff --git a/src/main/browser/cdp-ws-proxy.test.ts b/src/main/browser/cdp-ws-proxy.test.ts index 11fbcd772cf..15976e9c729 100644 --- a/src/main/browser/cdp-ws-proxy.test.ts +++ b/src/main/browser/cdp-ws-proxy.test.ts @@ -43,6 +43,7 @@ function createMockWebContents() { debugger: debuggerObj, isDestroyed: () => destroyed, focus: vi.fn(), + printToPDF: vi.fn(async () => Buffer.from('%PDF-test')), reload: vi.fn(), reloadIgnoringCache: vi.fn(), getTitle: vi.fn(() => 'Example'), @@ -109,6 +110,15 @@ describe('CdpWsProxy', () => { return calls.map((call) => call[0]) } + function expectPdfStreamHandle(response: Record): string { + const result = response.result as Record + expect(result.data).toBe('') + expect(result.stream).toEqual(expect.stringMatching(/^orca-pdf-[\da-f-]{36}-\d+$/)) + return result.stream as string + } + + const defaultPdfMarginInches = 1 / 2.54 + it('starts on a random port and returns ws:// URL', () => { expect(endpoint).toMatch(/^ws:\/\/127\.0\.0\.1:\d+$/) expect(proxy.getPort()).toBeGreaterThan(0) @@ -493,6 +503,209 @@ describe('CdpWsProxy', () => { client.close() }) + it('prints PDF data through native webContents printToPDF', async () => { + const client = await connect() + + const response = await sendAndReceive(client, { + id: 19, + method: 'Page.printToPDF', + params: { + landscape: true, + printBackground: true, + paperWidth: 8.5, + paperHeight: 11, + marginTop: 0.25, + marginBottom: 0.5, + marginLeft: 0.75, + marginRight: 1, + pageRanges: '1-2', + preferCSSPageSize: true + } + }) + + expect(response).toEqual({ + id: 19, + result: { data: Buffer.from('%PDF-test').toString('base64') } + }) + expect(mock.webContents.printToPDF).toHaveBeenCalledWith({ + landscape: true, + printBackground: true, + pageSize: { width: 8.5, height: 11 }, + margins: { + marginType: 'custom', + top: 0.25, + bottom: 0.5, + left: 0.75, + right: 1 + }, + pageRanges: '1-2', + preferCSSPageSize: true + }) + expect(getSendCommandMethods()).not.toContain('Page.printToPDF') + client.close() + }) + + it('keeps default PDF margins for omitted sides', async () => { + const client = await connect() + + await sendAndReceive(client, { + id: 20, + method: 'Page.printToPDF', + params: { + marginTop: 0.25 + } + }) + + expect(mock.webContents.printToPDF).toHaveBeenCalledWith({ + margins: { + marginType: 'custom', + top: 0.25, + bottom: defaultPdfMarginInches, + left: defaultPdfMarginInches, + right: defaultPdfMarginInches + } + }) + client.close() + }) + + it('supports streamed Page.printToPDF results for Playwright page.pdf', async () => { + mock.webContents.printToPDF.mockResolvedValueOnce(Buffer.from('abcdef')) + const client = await connect() + + const printResponse = await sendAndReceive(client, { + id: 21, + method: 'Page.printToPDF', + params: { transferMode: 'ReturnAsStream' } + }) + const handle = expectPdfStreamHandle(printResponse) + + const firstRead = await sendAndReceive(client, { + id: 22, + method: 'IO.read', + params: { handle, size: 2 } + }) + const secondRead = await sendAndReceive(client, { + id: 23, + method: 'IO.read', + params: { handle } + }) + const closeResponse = await sendAndReceive(client, { + id: 24, + method: 'IO.close', + params: { handle } + }) + const readAfterClose = await sendAndReceive(client, { + id: 25, + method: 'IO.read', + params: { handle } + }) + + expect(printResponse.id).toBe(21) + expect(firstRead).toEqual({ + id: 22, + result: { base64Encoded: true, data: Buffer.from('ab').toString('base64'), eof: false } + }) + expect(secondRead).toEqual({ + id: 23, + result: { base64Encoded: true, data: Buffer.from('cdef').toString('base64'), eof: true } + }) + expect(closeResponse).toEqual({ id: 24, result: {} }) + expect(readAfterClose).toEqual({ + id: 25, + error: { code: -32000, message: 'Invalid stream handle' } + }) + client.close() + }) + + it('clears streamed PDF data when the client disconnects', async () => { + mock.webContents.printToPDF.mockResolvedValueOnce(Buffer.from('abcdef')) + const client = await connect() + + const printResponse = await sendAndReceive(client, { + id: 26, + method: 'Page.printToPDF', + params: { transferMode: 'ReturnAsStream' } + }) + const handle = expectPdfStreamHandle(printResponse) + + expect(printResponse.id).toBe(26) + client.close() + await new Promise((resolve) => setTimeout(resolve, 10)) + + const nextClient = await connect() + const staleRead = await sendAndReceive(nextClient, { + id: 27, + method: 'IO.read', + params: { handle } + }) + + expect(staleRead).toEqual({ + id: 27, + error: { code: -32000, message: 'Invalid stream handle' } + }) + nextClient.close() + }) + + it('does not register a PDF stream when the client disconnects mid-print', async () => { + let resolvePrint: (buf: Buffer) => void = () => {} + mock.webContents.printToPDF.mockImplementationOnce( + () => + new Promise((resolve) => { + resolvePrint = resolve + }) + ) + const store = (proxy as unknown as { pdfStreams: { create: (b: Buffer) => string } }).pdfStreams + const createSpy = vi.spyOn(store, 'create') + + const client = await connect() + client.send( + JSON.stringify({ + id: 30, + method: 'Page.printToPDF', + params: { transferMode: 'ReturnAsStream' } + }) + ) + await new Promise((resolve) => setTimeout(resolve, 10)) + client.close() + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Print resolves only after the client is gone: no stream must be created. + resolvePrint(Buffer.from('%PDF-late')) + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(createSpy).not.toHaveBeenCalled() + createSpy.mockRestore() + }) + + it('forwards non-PDF IO streams to the debugger', async () => { + mock.webContents.debugger.sendCommand + .mockResolvedValueOnce({ data: 'trace-data', eof: false }) + .mockResolvedValueOnce({}) + const client = await connect() + + const readResponse = await sendAndReceive(client, { + id: 28, + method: 'IO.read', + params: { handle: 'trace-stream', size: 64 } + }) + const closeResponse = await sendAndReceive(client, { + id: 29, + method: 'IO.close', + params: { handle: 'trace-stream' } + }) + + expect(readResponse).toEqual({ id: 28, result: { data: 'trace-data', eof: false } }) + expect(closeResponse).toEqual({ id: 29, result: {} }) + expect(mock.webContents.debugger.sendCommand).toHaveBeenCalledWith('IO.read', { + handle: 'trace-stream', + size: 64 + }) + expect(mock.webContents.debugger.sendCommand).toHaveBeenCalledWith('IO.close', { + handle: 'trace-stream' + }) + client.close() + }) + // ── Page.frameNavigated interception ── // ── Cleanup ── diff --git a/src/main/browser/cdp-ws-proxy.ts b/src/main/browser/cdp-ws-proxy.ts index ec8de5be176..6c3a401d5b7 100644 --- a/src/main/browser/cdp-ws-proxy.ts +++ b/src/main/browser/cdp-ws-proxy.ts @@ -3,6 +3,7 @@ import { WebSocketServer, WebSocket } from 'ws' import { createServer, type Server, type IncomingMessage, type ServerResponse } from 'node:http' import type { WebContents } from 'electron' import { captureScreenshot } from './cdp-screenshot' +import { buildPrintToPdfOptions, CdpPdfStreamStore } from './cdp-print-to-pdf' import { ANTI_DETECTION_SCRIPT } from './anti-detection' import { acquireElectronDebugger, type ElectronDebuggerLease } from './electron-debugger-lease' @@ -20,6 +21,7 @@ export class CdpWsProxy { private attached = false // Why: agent-browser filters events by sessionId from Target.attachToTarget. private clientSessionId: string | undefined = undefined + private readonly pdfStreams = new CdpPdfStreamStore() constructor(private readonly webContents: WebContents) {} @@ -51,6 +53,7 @@ export class CdpWsProxy { const onClose = (): void => { detach() if (this.client === ws) { + this.pdfStreams.clear() this.client = null } } @@ -101,6 +104,7 @@ export class CdpWsProxy { this.detachClientListeners?.() this.detachClientListeners = null this.client = null + this.pdfStreams.clear() client?.close() } @@ -290,6 +294,30 @@ export class CdpWsProxy { this.handleScreenshot(client, clientId, msg.params) return } + // Why: CDP Page.printToPDF is not available for Electron webview guests. + // Electron's native printToPDF path is the reliable equivalent. + if (msg.method === 'Page.printToPDF') { + void this.handlePrintToPdf(client, clientId, msg.params ?? {}) + return + } + if (msg.method === 'IO.read') { + const params = msg.params ?? {} + if (this.pdfStreams.ownsHandle(params)) { + this.handleStreamRead(client, clientId, params) + return + } + this.forwardCommand(client, clientId, msg.method, params, msg.sessionId) + return + } + if (msg.method === 'IO.close') { + const params = msg.params ?? {} + if (this.pdfStreams.ownsHandle(params)) { + this.handleStreamClose(client, clientId, params) + return + } + this.forwardCommand(client, clientId, msg.method, params, msg.sessionId) + return + } // Why: Input.insertText can still require native focus in Electron webviews. // Do not auto-focus generic Runtime.evaluate/callFunctionOn traffic: wait // polling and read-only JS probes use those methods heavily, and focusing on @@ -439,6 +467,57 @@ export class CdpWsProxy { } } + private async handlePrintToPdf( + client: WebSocket, + clientId: number, + params: Record + ): Promise { + if (this.webContents.isDestroyed()) { + this.sendError(clientId, 'Browser tab is no longer available', client) + return + } + try { + const pdf = await this.webContents.printToPDF(buildPrintToPdfOptions(params)) + // Why: printToPDF can resolve after the client disconnected (or was + // replaced). Bail before registering a stream so its buffer isn't + // orphaned in pdfStreams past the disconnect's clear() until the TTL. + if (!this.isActiveClient(client)) { + return + } + const buffer = Buffer.isBuffer(pdf) ? pdf : Buffer.from(pdf) + if (params.transferMode === 'ReturnAsStream') { + const handle = this.pdfStreams.create(buffer) + this.sendResult(clientId, { data: '', stream: handle }, client) + return + } + this.sendResult(clientId, { data: buffer.toString('base64') }, client) + } catch (err) { + this.sendError(clientId, err instanceof Error ? err.message : String(err), client) + } + } + + private handleStreamRead( + client: WebSocket, + clientId: number, + params: Record + ): void { + const chunk = this.pdfStreams.read(params) + if (!chunk) { + this.sendError(clientId, 'Invalid stream handle', client) + return + } + this.sendResult(clientId, { base64Encoded: true, data: chunk.data, eof: chunk.eof }, client) + } + + private handleStreamClose( + client: WebSocket, + clientId: number, + params: Record + ): void { + this.pdfStreams.close(params) + this.sendResult(clientId, {}, client) + } + private handleScreenshot( client: WebSocket, clientId: number, From 02dfb4b334c5090a89f3e19b438493893d3ce327 Mon Sep 17 00:00:00 2001 From: Parker Rex Date: Fri, 3 Jul 2026 16:12:14 -0400 Subject: [PATCH 4/4] Fix Discord icon and CreateWorktree label spacing (#6505) * Fix landing create worktree label * Update Discord help menu icon --- src/renderer/src/components/Landing.tsx | 4 +- .../sidebar/SidebarSettingsHelpMenu.test.tsx | 67 ++++++++++++++++++- .../sidebar/SidebarSettingsHelpMenu.tsx | 4 +- 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/components/Landing.tsx b/src/renderer/src/components/Landing.tsx index 05d609f6d35..9e79671f25a 100644 --- a/src/renderer/src/components/Landing.tsx +++ b/src/renderer/src/components/Landing.tsx @@ -362,8 +362,8 @@ export default function Landing(): React.JSX.Element { onClick={() => openModal('new-workspace-composer', { telemetrySource: 'unknown' })} > - {translate('auto.components.Landing.76a95f7f47', 'Create')} - {createTargetLabel} + {translate('auto.components.Landing.76a95f7f47', 'Create')}{' '} + {createTargetLabel.toLowerCase()} diff --git a/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx b/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx index 8fba33ed07d..6f6ea7809ef 100644 --- a/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx +++ b/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx @@ -1,6 +1,9 @@ +// @vitest-environment happy-dom + +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' import { renderToStaticMarkup } from 'react-dom/server' -import type { ReactNode } from 'react' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { SidebarSettingsHelpMenu } from './SidebarSettingsHelpMenu' const mocks = vi.hoisted(() => ({ @@ -20,6 +23,7 @@ const mocks = vi.hoisted(() => ({ })) let updateStatus = { state: 'idle' } as const +const roots: Root[] = [] vi.mock('@/store', () => ({ useAppStore: (selector: (state: unknown) => unknown) => @@ -96,9 +100,48 @@ vi.mock('./SidebarFeedbackDialog', () => ({ SidebarFeedbackDialog: () =>
})) +function installWindowApi(): void { + Object.assign(window, { + api: { + app: { + restart: mocks.appRestart + }, + shell: { + openUrl: mocks.shellOpenUrl + }, + updater: { + check: mocks.updaterCheck + } + } + }) +} + +async function renderMenu(): Promise { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + roots.push(root) + + await act(async () => { + root.render() + }) + + return container +} + +function findMenuItem(container: HTMLElement, label: string): HTMLButtonElement { + const button = Array.from( + container.querySelectorAll('[data-testid="menu-item"]') + ).find((element) => element.textContent?.includes(label)) + expect(button).toBeDefined() + return button as HTMLButtonElement +} + describe('SidebarSettingsHelpMenu', () => { beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.clearAllMocks() + installWindowApi() mocks.useShortcutKeyDetails.mockReturnValue({ keys: ['⌘', ','], doubleTap: false }) updateStatus = { state: 'idle' } mocks.setupProgress = { @@ -109,6 +152,13 @@ describe('SidebarSettingsHelpMenu', () => { } }) + afterEach(() => { + roots.splice(0).forEach((root) => { + act(() => root.unmount()) + }) + document.body.replaceChildren() + }) + it('renders the help button with correct aria-label', () => { const html = renderToStaticMarkup() expect(html).toContain('Help') @@ -177,6 +227,19 @@ describe('SidebarSettingsHelpMenu', () => { it('renders Discord link', () => { const html = renderToStaticMarkup() expect(html).toContain('Discord') + expect(html).toContain('viewBox="0 0 20 20"') + expect(html).toContain('M16.0742 4.45014C14.9244 3.92097 13.7106 3.54556 12.4638 3.3335') + }) + + it('opens Discord invite through the shell bridge', async () => { + const container = await renderMenu() + const discordButton = findMenuItem(container, 'Discord') + + await act(async () => { + discordButton.click() + }) + + expect(mocks.shellOpenUrl).toHaveBeenCalledWith('https://discord.gg/fzjDKHxv8Q') }) it('renders X link', () => { diff --git a/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx b/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx index f5bf3250c6f..d74fcfb11aa 100644 --- a/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx +++ b/src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx @@ -46,8 +46,8 @@ function openExternalUrl(url: string): void { function DiscordIcon(): React.JSX.Element { return ( -