mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix: version strings gstack names aren't public IPs; renderers install Chromium on first use; outside-voice role hint (GST-1204) (#3941)
* 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> * 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> * docs: changelog fragment and troubleshooting entry name the tightened gstack-version exemption (GST-1204) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
9b8000a642
commit
0b19a366ca
8 files changed
+139
-12
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, shaped like a gstack release (major 0 to 2), 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 with the same major (`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 8.8.8.8`, `gstack talks to 8.8.8.8`, `gstack-proxy 1.128.1.0`, `my-gstack 1.94.4.0`, `landed in 1.94.4.0`, `gstack 1.94.4.0 - 8.8.8.8`, 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; twelve address controls still report MEDIUM.
|
||||
- `test/aside-render.test.ts`: a missing Chromium runs gstack-browser-ensure once and the render proceeds; the install time doesn't count against the render deadline; a failed install names the launch error; the existing newtab cases are unchanged.
|
||||
- `test/model-policy-cli.test.ts`: the outside-voice hint.
|
||||
@@ -2706,6 +2706,8 @@ exempt wherever they appear, so there the finding means the quad is not a
|
||||
release of this repository (or the post ran outside the checkout).
|
||||
|
||||
**Fix.** Write it as `version: 1.2.3.4`, `"version": "..."` or `v1.2.3.4`.
|
||||
A gstack release named right after `gstack` (`gstack 1.94.4.0`, the install
|
||||
check's `gstack: ok 1.94.4.0 for=ship`) is exempt too, but only for majors 0 to 2.
|
||||
MEDIUM findings do not block pushes.
|
||||
|
||||
<a id="redact-run-id-as-phone"></a>
|
||||
|
||||
+27
-6
@@ -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> = {};
|
||||
@@ -528,13 +535,14 @@ 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<string> => {
|
||||
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,23 @@ 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 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}`})`);
|
||||
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);
|
||||
|
||||
+23
-5
@@ -406,12 +406,30 @@ 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). 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_RELEASE = /^[0-2]\./;
|
||||
const GSTACK_NAMED_BEFORE = /(?<![\w./-])gstack(?:[ \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 a declared version with the same major. */
|
||||
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);
|
||||
const span = input.slice(start, end);
|
||||
if (VERSION_DECLARATION_BEFORE.test(before) || (GSTACK_NAMED_BEFORE.test(before) && GSTACK_RELEASE.test(span))) 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 || first[1]!.split(".")[0] !== span.split(".")[0]) 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,51 @@ 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, 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 });
|
||||
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; ' + installedNewtab + ' 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');
|
||||
});
|
||||
|
||||
// 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);
|
||||
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,35 @@ 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",
|
||||
"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",
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
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