mirror of
https://github.com/garrytan/gstack.git
synced 2026-07-07 08:37:52 +02:00
Merge remote-tracking branch 'origin/main' into garrytan/fix-for-gbrowser
# Conflicts: # CHANGELOG.md # VERSION
This commit is contained in:
@@ -29,17 +29,20 @@ describe('shouldEnableChromiumSandbox', () => {
|
||||
const origPlatform = process.platform;
|
||||
const origCI = process.env.CI;
|
||||
const origContainer = process.env.CONTAINER;
|
||||
const origNoSandbox = process.env.GSTACK_CHROMIUM_NO_SANDBOX;
|
||||
const origGetuid = process.getuid;
|
||||
|
||||
beforeEach(() => {
|
||||
delete process.env.CI;
|
||||
delete process.env.CONTAINER;
|
||||
delete process.env.GSTACK_CHROMIUM_NO_SANDBOX;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(process, 'platform', { value: origPlatform });
|
||||
if (origCI === undefined) delete process.env.CI; else process.env.CI = origCI;
|
||||
if (origContainer === undefined) delete process.env.CONTAINER; else process.env.CONTAINER = origContainer;
|
||||
if (origNoSandbox === undefined) delete process.env.GSTACK_CHROMIUM_NO_SANDBOX; else process.env.GSTACK_CHROMIUM_NO_SANDBOX = origNoSandbox;
|
||||
process.getuid = origGetuid;
|
||||
});
|
||||
|
||||
@@ -90,6 +93,31 @@ describe('shouldEnableChromiumSandbox', () => {
|
||||
const { shouldEnableChromiumSandbox } = await import('../src/browser-manager');
|
||||
expect(shouldEnableChromiumSandbox()).toBe(false);
|
||||
});
|
||||
|
||||
// #1562 — Ubuntu/AppArmor opt-in override
|
||||
it('linux + GSTACK_CHROMIUM_NO_SANDBOX=1 → false (Ubuntu/AppArmor opt-out)', async () => {
|
||||
setPlatform('linux');
|
||||
process.env.GSTACK_CHROMIUM_NO_SANDBOX = '1';
|
||||
process.getuid = (() => 1000) as typeof process.getuid;
|
||||
const { shouldEnableChromiumSandbox } = await import('../src/browser-manager');
|
||||
expect(shouldEnableChromiumSandbox()).toBe(false);
|
||||
});
|
||||
|
||||
it('darwin + GSTACK_CHROMIUM_NO_SANDBOX=1 → false (env override wins on any platform)', async () => {
|
||||
setPlatform('darwin');
|
||||
process.env.GSTACK_CHROMIUM_NO_SANDBOX = '1';
|
||||
process.getuid = (() => 501) as typeof process.getuid;
|
||||
const { shouldEnableChromiumSandbox } = await import('../src/browser-manager');
|
||||
expect(shouldEnableChromiumSandbox()).toBe(false);
|
||||
});
|
||||
|
||||
it('GSTACK_CHROMIUM_NO_SANDBOX=0 → does NOT trigger override (must be exactly "1")', async () => {
|
||||
setPlatform('linux');
|
||||
process.env.GSTACK_CHROMIUM_NO_SANDBOX = '0';
|
||||
process.getuid = (() => 1000) as typeof process.getuid;
|
||||
const { shouldEnableChromiumSandbox } = await import('../src/browser-manager');
|
||||
expect(shouldEnableChromiumSandbox()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── resolveDisconnectCause ──────────────────────────────────────
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
/**
|
||||
* Coverage for #1612 — macOS/Linux server must survive sandboxed-shell
|
||||
* harnesses by becoming its own session leader (setsid).
|
||||
*
|
||||
* Pre-#1612, Bun.spawn().unref() removed the child from Bun's event loop
|
||||
* but did NOT call setsid(). When the CLI ran inside Claude Code's
|
||||
* per-command sandbox, Conductor, or CI step runners, the session leader's
|
||||
* exit sent SIGHUP to every PID in the session, killing the bun server.
|
||||
*
|
||||
* The fix routes macOS/Linux spawn through Node's child_process.spawn with
|
||||
* detached:true, which calls setsid() so the server becomes its own session
|
||||
* leader (PPID=1 on Linux, similar reparenting on Darwin).
|
||||
*
|
||||
* The actual setsid syscall is hard to assert in a unit test without a
|
||||
* real spawn — testing here is static: the cli.ts source must use the
|
||||
* Node spawn path on macOS/Linux, with detached:true and .unref(). If a
|
||||
* future refactor reverts to Bun.spawn().unref() on the macOS/Linux branch
|
||||
* the regression returns and these tests fail.
|
||||
*/
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, "..", "..");
|
||||
const CLI = path.join(ROOT, "browse", "src", "cli.ts");
|
||||
|
||||
function read(): string {
|
||||
return fs.readFileSync(CLI, "utf-8");
|
||||
}
|
||||
|
||||
describe("#1612 macOS/Linux daemonize via Node setsid path", () => {
|
||||
test("cli.ts imports nodeSpawn from child_process (Node spawn alias)", () => {
|
||||
const body = read();
|
||||
// The fix relies on Node's child_process.spawn (which calls setsid on
|
||||
// detached:true), aliased to avoid name collision with Bun.spawn. Match
|
||||
// either `nodeSpawn` or `spawn as nodeSpawn` to be flexible to the
|
||||
// exact import style.
|
||||
expect(body).toMatch(/(spawn as nodeSpawn|nodeSpawn\s*[,}])/);
|
||||
expect(body).toMatch(/from\s+['"]child_process['"]/);
|
||||
});
|
||||
|
||||
test("non-Windows branch uses nodeSpawn(...).unref() with detached:true", () => {
|
||||
const body = read();
|
||||
// Find the non-Windows branch and assert it uses the Node spawn alias
|
||||
// with detached:true. Match the pattern `nodeSpawn(...) ... detached:true`.
|
||||
expect(body).toMatch(/nodeSpawn\([\s\S]{0,500}detached:\s*true/);
|
||||
expect(body).toMatch(/nodeSpawn\([\s\S]{0,500}\.unref\(\)/);
|
||||
});
|
||||
|
||||
test("non-Windows branch comment documents setsid/SIGHUP root cause", () => {
|
||||
const body = read();
|
||||
// The comment block must mention setsid() so a future refactor sees the
|
||||
// why before changing the spawn call.
|
||||
expect(body).toMatch(/setsid/);
|
||||
expect(body).toMatch(/SIGHUP/);
|
||||
});
|
||||
|
||||
test("the spawn call on macOS/Linux is nodeSpawn, not Bun.spawn", () => {
|
||||
const body = read();
|
||||
// Strip line comments before regex matching, so the "Bun.spawn().unref()"
|
||||
// mentions inside the explanatory comment don't trigger false positives.
|
||||
const codeOnly = body
|
||||
.split("\n")
|
||||
.filter((line) => !line.trim().startsWith("//"))
|
||||
.join("\n");
|
||||
// Find the non-Windows branch. The `} else {` block following the
|
||||
// Windows branch. We then require its first ~400 chars contain a
|
||||
// nodeSpawn() call and NOT a Bun.spawn() call (excluding the comment).
|
||||
const nonWindowsStart = codeOnly.indexOf("nodeSpawn('bun'");
|
||||
expect(nonWindowsStart).toBeGreaterThan(-1);
|
||||
const slice = codeOnly.slice(nonWindowsStart, nonWindowsStart + 400);
|
||||
expect(slice).toMatch(/nodeSpawn\(/);
|
||||
expect(slice).not.toMatch(/Bun\.spawn\(/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user