diff --git a/changelog.d/gst-1204-ship-tooling.md b/changelog.d/gst-1204-ship-tooling.md new file mode 100644 index 000000000..b9d4c13d4 --- /dev/null +++ b/changelog.d/gst-1204-ship-tooling.md @@ -0,0 +1,11 @@ +**Three /ship tooling fixes from the gbrain-evals F1 rollout (GST-1204).** Version strings quoted in lane reports no longer trip the secret scan, `gstack-pr-shots` renders on a fresh Capy machine, and `gstack-models` explains why it has no outside-voice role. + +### Fixed +- **Secret scan (`pii.ip_public`):** a four-part version that gstack names right before it is not read as a public IPv4 address. That covers `gstack 1.94.4.0`, `upgraded gstack from 1.93.14.0`, and the install check's trailers `gstack: ok 1.94.4.0 for=ship` and `gstack: fail claude,codex (1.93.37.0)`. It also covers the far end of a range whose first end is a declared version (`gstack 1.94.4.0 to 1.94.8.0`). Before this, every push and PR-body post that quoted those lines stopped on a MEDIUM finding. Real addresses still report: `gstack talks to 8.8.8.8`, `gstack-proxy 1.128.1.0`, `landed in 1.94.4.0`, and a range with no declared first end. +- **`gstack-pr-shots` and every renderer that uses gstack's own browser:** when Playwright's Chromium isn't installed (the Capy install defers it to first use), the renderer now runs the checkout's `bin/gstack-browser-ensure` once and retries. Before, it stopped with `browse newtab failed: [browse] Starting server...`, which hid the cause. When the install fails, the error names the launch failure and the installer's last line instead of the daemon's banner. +- **`gstack-models resolve --role outside-voice`:** it still exits as a usage error, but now says the outside voice follows the harness, not the model policy, and points to docs/model-policy.md. + +### Tests +- `test/redact-context-exemptions.test.ts`: six gstack-named version shapes stay clean; five address controls still report MEDIUM. +- `test/aside-render.test.ts`: a missing Chromium runs gstack-browser-ensure once and the render proceeds; a failed install names the launch error; the existing newtab cases are unchanged. +- `test/model-policy-cli.test.ts`: the outside-voice hint. diff --git a/lib/aside-render.ts b/lib/aside-render.ts index 66a617d85..6362b7e67 100644 --- a/lib/aside-render.ts +++ b/lib/aside-render.ts @@ -519,6 +519,13 @@ function screenshotName(i: number, step: ScreenshotStep): string { * Aside path, one CLI call per action, artifacts staged under /tmp and copied * to the caller's paths. The tab is closed in a finally; the daemon stays up. */ +/** The checkout's bin/gstack-browser-ensure next to a browse/dist/browse binary; it installs gstack's Chromium on first use. */ +function browserEnsureBin(bin: string): string | null { + const root = /^(.*)[\\/]browse[\\/]dist[\\/]browse(?:\.exe)?$/.exec(bin)?.[1]; + const ensure = root ? path.join(root, 'bin', 'gstack-browser-ensure') : null; + return ensure && fs.existsSync(ensure) ? ensure : null; +} + export async function renderWithBrowse(spec: RenderSpec, bin: string | null = resolveBrowseBin()): Promise { const outputs: string[] = []; const evals: Record = {}; @@ -534,7 +541,8 @@ export async function renderWithBrowse(spec: RenderSpec, bin: string | null = re const r = await runProc(bin, args, Math.max(1_000, Math.min(120_000, deadline - Date.now())), browseEnv); log.push(`$ browse ${args.join(' ').slice(0, 300)}\n${r.stdout}${r.stderr}`.trim()); if (r.error || r.code !== 0) { - const first = (r.stderr || r.stdout || r.error || '').trim().split('\n')[0]; + const lines = (r.stderr || r.stdout || r.error || '').split('\n').map((l) => l.trim()).filter(Boolean); + const first = lines.find((l) => /Executable doesn't exist/.test(l)) ?? lines.find((l) => !/^\[browse\] (Starting server|Server failed to start)/.test(l) && !/^[\u2554\u2551\u255a]/.test(l)) ?? lines[0] ?? ''; if (/JS execution blocked/.test(`${r.stderr}${r.stdout}`)) { // After `$B cookie-import` the daemon refuses page JS on every other // origin, 127.0.0.1 included; a local-HTML render cannot proceed in it. @@ -565,10 +573,21 @@ export async function renderWithBrowse(spec: RenderSpec, bin: string | null = re try { opened = await openTab(); } catch (e) { - if (!/Unable to connect/.test((e as Error).message)) throw e; - log.push('newtab: daemon not up yet — retrying once'); - await Bun.sleep(1_500); - opened = await openTab(); + const message = (e as Error).message; + 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 installed = await runProc('bash', [ensure, '--quiet'], 600_000, browseEnv); + 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}`})`); + opened = await openTab(); + } else { + if (!/Unable to connect/.test(message)) throw e; + log.push('newtab: daemon not up yet — retrying once'); + await Bun.sleep(1_500); + opened = await openTab(); + } } tab = opened ? JSON.parse(opened).tabId : undefined; if (typeof tab !== 'number') throw new Error('browse newtab --json returned no tabId'); diff --git a/lib/model-policy-cli.ts b/lib/model-policy-cli.ts index 30694d11d..f9aed6f31 100644 --- a/lib/model-policy-cli.ts +++ b/lib/model-policy-cli.ts @@ -298,7 +298,7 @@ export function modelsMain(args: string[], env: StateRootEnv = process.env, plat const providerArg = options['--provider']; const provider = parseProvider(providerArg); if (command === 'resolve') { - if (role !== 'plan-review' && role !== 'implementation' && role !== 'eval-arms' && role !== 'agent') return usage(role === undefined ? 'resolve needs --role plan-review|implementation|eval-arms|agent' : `unknown role '${role}': use plan-review, implementation, eval-arms or agent`, json); + if (role !== 'plan-review' && role !== 'implementation' && role !== 'eval-arms' && role !== 'agent') return usage(role === undefined ? 'resolve needs --role plan-review|implementation|eval-arms|agent' : role === 'outside-voice' ? "unknown role 'outside-voice': the outside voice follows the harness, not the model policy (docs/model-policy.md#your-model-preference); use plan-review, implementation, eval-arms or agent" : `unknown role '${role}': use plan-review, implementation, eval-arms or agent`, json); if (role === 'agent') { if (providerArg !== undefined) return usage('resolve --role agent takes no --provider; the preference names its provider', json); return agentResult(env, platform, options['--request'], json); diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 5bac73526..8591c3e29 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -395,12 +395,27 @@ export function isCiRunIdentifier(span: string, match: RegExpExecArray): boolean const VERSION_DECLARATION_BEFORE = /(?:\b[Vv]ersion|\bVERSION|[a-z0-9_]Version|[_-][Vv]ersion|_VERSION)["'\]]?(?:[ \t]*[:=(>][ \t]*|[ \t]+)["']?$/; const CHANGELOG_HEADING_BEFORE = /^#{1,6}[ \t]+\[$/; -export function isDeclaredVersion(match: RegExpExecArray): boolean { - const input = match.input ?? ""; - const { start, end } = spanBounds(match); +/** + * 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. + */ +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 RANGE_END_BEFORE = /(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})[ \t]*(?:to|through|-|\u2013)[ \t]*$/; +function declaredAt(input: string, start: number, end: number, range: boolean): boolean { const lineStart = input.lastIndexOf("\n", start - 1) + 1; - if (VERSION_DECLARATION_BEFORE.test(input.slice(Math.max(lineStart, start - 48), start))) return true; - return CHANGELOG_HEADING_BEFORE.test(input.slice(lineStart, start)) && input[end] === "]"; + const before = input.slice(Math.max(lineStart, start - 48), start); + if (VERSION_DECLARATION_BEFORE.test(before) || GSTACK_NAMED_BEFORE.test(before)) return true; + if (CHANGELOG_HEADING_BEFORE.test(input.slice(lineStart, start)) && input[end] === "]") return true; + const first = range ? RANGE_END_BEFORE.exec(before) : null; + if (!first) return false; + const firstStart = start - before.length + first.index; + return declaredAt(input, firstStart, firstStart + first[1]!.length, false); +} +export function isDeclaredVersion(match: RegExpExecArray): boolean { + const { start, end } = spanBounds(match); + return declaredAt(match.input ?? "", start, end, true); } // ── Placeholder suppression (per-matched-span, NOT per-line) ───────────────── diff --git a/test/aside-render.test.ts b/test/aside-render.test.ts index 67dc8bfd4..8b2b03316 100644 --- a/test/aside-render.test.ts +++ b/test/aside-render.test.ts @@ -867,6 +867,42 @@ describe.skipIf(!HERMETIC)('aside-render: renderWithBrowse — daemon CLI contra expect(r.evals[0]).toBe('Fake Title'); }); + // Value: protects=a lazy Capy install (Chromium skipped until first use) renders on the first call: Playwright's missing-browser launch + // error runs the checkout's gstack-browser-ensure once and retries newtab; fails_when=the render stops on "browse newtab failed: [browse] + // Starting server..." with the cause hidden; why_new=GST-1204 (gbrain-evals#317: gstack-pr-shots on a Capy machine); seam=bin/gstack-browser-ensure + const MISSING = "echo '[browse] Starting server...' >&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 root = fs.mkdtempSync(path.join(tmp, 'lazy-')); + const dist = path.join(root, 'browse', 'dist'); + fs.mkdirSync(dist, { recursive: true }); + fs.mkdirSync(path.join(root, 'bin')); + const ensureLog = path.join(root, 'ensure.log'); + 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}\'' }); + return { browse, ensureLog, marker }; + }; + + test('a missing Playwright Chromium runs gstack-browser-ensure once, then the render proceeds', async () => { + const { browse, ensureLog } = lazyCheckout(": > MARKER; echo 'BROWSER_OK playwright 140.0'"); + const r = await renderWithBrowse({ file: doc, steps: [{ kind: 'eval', expression: 'document.title' }] }, browse); + expectOk(r); + expect(readLines(ensureLog)).toEqual(['--quiet']); + expect(readLines(log).filter((l) => l === 'newtab --json').length).toBe(2); + expect(r.stdout).toContain('newtab: Chromium is not installed'); + }); + + 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); + expect(r.ok).toBe(false); + expect(r.error).toContain("browse newtab failed: launch: Executable doesn't exist"); + expect(r.error).toContain('BROWSER_UNAVAILABLE offline'); + expect(readLines(ensureLog).length).toBe(1); + expect(readLines(log).filter((l) => l === 'newtab --json').length).toBe(1); + }); + test('any other newtab failure is not retried', async () => { const r = await renderWithBrowse({ file: doc, steps: [] }, fake({ newtab: `echo 'browser launch failed: no display' >&2; exit 1` })); expect(r.ok).toBe(false); diff --git a/test/model-policy-cli.test.ts b/test/model-policy-cli.test.ts index db9996244..a76dd72ad 100644 --- a/test/model-policy-cli.test.ts +++ b/test/model-policy-cli.test.ts @@ -137,6 +137,7 @@ describe('gstack-models inspector', () => { [['bogus'], "unknown command 'bogus'"], [['resolve'], 'resolve needs --role'], [['resolve', '--role', 'coding'], "unknown role 'coding'"], + [['resolve', '--role', 'outside-voice'], "the outside voice follows the harness, not the model policy (docs/model-policy.md"], [['resolve', '--role', 'plan-review'], 'needs --provider'], [['resolve', '--role', 'plan-review', '--provider', 'claude'], "unknown provider 'claude'"], [['resolve', '--role', 'implementation', '--provider', 'OpenAI'], "unknown provider 'OpenAI'"], diff --git a/test/redact-context-exemptions.test.ts b/test/redact-context-exemptions.test.ts index a5134b7f3..20b0c8316 100644 --- a/test/redact-context-exemptions.test.ts +++ b/test/redact-context-exemptions.test.ts @@ -208,6 +208,28 @@ describe("pii.ip_public: a four-part version needs a version declaration (#2784) ); }); +describe("pii.ip_public: a four-part version gstack names is not an address (GST-1204)", () => { + table( + "pii.ip_public", + "MEDIUM", + [ + "gstack 1.94.4.0", + "gstack: ok 1.94.4.0 for=ship", + "gstack: fail claude,codex (1.93.37.0)", + "upgraded gstack from 1.93.14.0 to 1.94.4.0", + "seen on gstack 1.94.4.0 to 1.94.8.0", + "(gstack 1.94.4.0-1.94.8.0)", + ], + [ + "gstack talks to 8.8.8.8", + "gstack-proxy 1.128.1.0", + "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", + ], + ); +}); + describe("db.url_with_password: postgres:postgres only on a loopback host (#2913)", () => { // Assembled from parts so this file's own source never carries a // credential-bearing URL for the repo's secret scan to block.