From 871687e5765aed5b3a56c23d093c85fd730feda5 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 11:40:40 +0000 Subject: [PATCH] fix: gstack-named exemption needs a release-shaped quad and a same-major range end; Chromium install time doesn't eat the render deadline (GST-1204 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/aside-render.ts | 4 +++- lib/redact-patterns.ts | 13 ++++++++----- test/aside-render.test.ts | 13 +++++++++++-- test/redact-context-exemptions.test.ts | 7 +++++++ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/lib/aside-render.ts b/lib/aside-render.ts index 6362b7e67..adef341e6 100644 --- a/lib/aside-render.ts +++ b/lib/aside-render.ts @@ -535,7 +535,7 @@ export async function renderWithBrowse(spec: RenderSpec, bin: string | null = re const target = pageTarget(spec); if (target.kind === 'error') return fail(target.error); - const deadline = Date.now() + (spec.timeoutMs ?? DEFAULT_TIMEOUT_MS); + let deadline = Date.now() + (spec.timeoutMs ?? DEFAULT_TIMEOUT_MS); const browseEnv = { ...process.env, BROWSE_SESSION: '' }; const run = async (args: string[]): Promise => { const r = await runProc(bin, args, Math.max(1_000, Math.min(120_000, deadline - Date.now())), browseEnv); @@ -577,7 +577,9 @@ export async function renderWithBrowse(spec: RenderSpec, bin: string | null = re const ensure = /Executable doesn't exist/.test(message) ? browserEnsureBin(bin) : null; if (ensure) { log.push('newtab: Chromium is not installed — running gstack-browser-ensure once'); + const installStart = Date.now(); const installed = await runProc('bash', [ensure, '--quiet'], 600_000, browseEnv); + deadline += Date.now() - installStart; const last = installed.stdout.trim().split('\n').at(-1) ?? ''; log.push(`${ensure} --quiet\n${installed.stdout}${installed.stderr}`.trim()); if (installed.error || installed.code !== 0) throw new Error(`${message} (gstack-browser-ensure: ${last || installed.error || `exit ${installed.code}`})`); diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 8591c3e29..192424519 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -398,18 +398,21 @@ const CHANGELOG_HEADING_BEFORE = /^#{1,6}[ \t]+\[$/; /** * gstack names its own release right before the quad: `gstack 1.94.4.0`, `upgraded gstack from 1.93.14.0`, and the * install check's trailer `gstack: ok 1.94.4.0 for=ship` or `gstack: fail claude,codex (1.93.37.0)`, which every lane - * report and PR body quotes (GST-1204). `gstack-proxy 1.128.1.0` or `gstack talks to 8.8.8.8` still report. + * report and PR body quotes (GST-1204). Only a quad shaped like a gstack release (major 0-2) counts, so `gstack 8.8.8.8`, + * `my-gstack 1.94.4.0`, `gstack-proxy 1.128.1.0` or `gstack talks to 8.8.8.8` still report. */ -const GSTACK_NAMED_BEFORE = /\bgstack(?:[ \t]*:[ \t]*(?:ok|fail)(?:[ \t]+[a-z0-9_,-]+)?[ \t]+\(?|[ \t]+(?:from[ \t]+)?)$/; -/** The second end of a range (`X to Y`, `X-Y`) whose first end is itself a declared version. */ +const GSTACK_RELEASE = /^[0-2]\./; +const GSTACK_NAMED_BEFORE = /(?&2; echo '\u2554\u2550 Looks like Playwright was just installed or updated. \u2557' >&2; echo \"launch: Executable doesn't exist at /x/chrome-headless-shell\" >&2; exit 1"; - const lazyCheckout = (ensureBody: string): { browse: string; ensureLog: string; marker: string } => { + const lazyCheckout = (ensureBody: string, installedNewtab = ''): { browse: string; ensureLog: string; marker: string } => { const root = fs.mkdtempSync(path.join(tmp, 'lazy-')); const dist = path.join(root, 'browse', 'dist'); fs.mkdirSync(dist, { recursive: true }); @@ -880,7 +880,7 @@ describe.skipIf(!HERMETIC)('aside-render: renderWithBrowse — daemon CLI contra const marker = path.join(root, 'chromium'); writeExecutable(path.join(root, 'bin', 'gstack-browser-ensure'), 'printf "%s\\n" "$*" >> ' + JSON.stringify(ensureLog) + '\n' + ensureBody.replace('MARKER', JSON.stringify(marker)) + '\n'); fs.rmSync(log, { force: true }); - const browse = writeFakeBrowse(dist, log, { newtab: 'if [ ! -f ' + JSON.stringify(marker) + ' ]; then ' + MISSING + '; fi; echo \'{"tabId":7}\'' }); + const browse = writeFakeBrowse(dist, log, { newtab: 'if [ ! -f ' + JSON.stringify(marker) + ' ]; then ' + MISSING + '; fi; ' + installedNewtab + ' echo \'{"tabId":7}\'' }); return { browse, ensureLog, marker }; }; @@ -893,6 +893,15 @@ describe.skipIf(!HERMETIC)('aside-render: renderWithBrowse — daemon CLI contra expect(r.stdout).toContain('newtab: Chromium is not installed'); }); + // Value: protects=a Chromium download longer than the render budget leaves the retried newtab its own time; fails_when=the retry runs on + // the 1 s floor and the first render on a fresh machine times out; why_new=GST-1204 review (outside voice + adversarial P2); seam=deadline + test('the install time does not count against the render deadline', async () => { + const { browse, ensureLog } = lazyCheckout("sleep 3; : > MARKER", 'sleep 1.5;'); + const r = await renderWithBrowse({ file: doc, timeoutMs: 2_500, steps: [{ kind: 'eval', expression: 'document.title' }] }, browse); + expectOk(r); + expect(readLines(ensureLog)).toEqual(['--quiet']); + }, 20_000); + test('when gstack-browser-ensure cannot install Chromium, the error names the launch failure, not the daemon banner', async () => { const { browse, ensureLog } = lazyCheckout("echo 'BROWSER_UNAVAILABLE offline'; exit 1"); const r = await renderWithBrowse({ file: doc, steps: [] }, browse); diff --git a/test/redact-context-exemptions.test.ts b/test/redact-context-exemptions.test.ts index 20b0c8316..2d4a47784 100644 --- a/test/redact-context-exemptions.test.ts +++ b/test/redact-context-exemptions.test.ts @@ -226,6 +226,13 @@ describe("pii.ip_public: a four-part version gstack names is not an address (GST "landed in 1.94.4.0", "gstack 1.94.4.0 via 8.8.8.8", "from 1.128.1.0 to 1.128.1.1", + "gstack 8.8.8.8", + "gstack: fail 8.8.8.8", + "upgraded gstack from 8.8.8.8", + "my-gstack 1.94.4.0", + "/opt/gstack 1.94.4.0", + "gstack 1.94.4.0 - 8.8.8.8", + "VERSION 1.2.3.4 to 8.8.8.8", ], ); });