mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 23:37:53 +01:00
fix: version strings gstack names aren't public IPs; renderers install Chromium on first use; outside-voice role hint (GST-1204)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
bf2bcf14b0
commit
0694040f98
7 files changed
+115
-11
No files matched your search
@@ -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.
|
||||
+24
-5
@@ -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<RenderResult> {
|
||||
const outputs: string[] = [];
|
||||
const evals: Record<number, string> = {};
|
||||
@@ -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');
|
||||
|
||||
@@ -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);
|
||||
|
||||
+20
-5
@@ -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) ─────────────────
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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'"],
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in new issue
Block a user