mirror of https://github.com/garrytan/gstack.git
fix(ios-qa): isolate E2E tests under --concurrent (3 real races)
The ios-qa E2E file failed intermittently under `bun test --concurrent` (the eval harness default). Three distinct shared-state races, all fixed: 1. Shared pidfile: a module-level `workDir` reassigned in beforeEach was clobbered by parallel tests, so concurrent daemons collided on the same pidfile and the loser returned `already_running`. Each test now gets its own dir via makeWorkDir(). 2. process.env path globals: tests set GSTACK_IOS_AUDIT_PATH / _ATTEMPTS_PATH / _ALLOWLIST_PATH on the shared process env; concurrent tests stomped each other's audit/attempts destinations. Threaded auditPath/attemptsPath/allowlistPath through DaemonOptions (and mintForCaller) as explicit args — env is no longer load-bearing. 3. afterEach cleanup race: the per-test cleanup drained a shared dir array, so the first test to finish deleted still-running tests' workDirs mid-assertion. Moved to afterAll (cleans once, after all settle). Verified: 5/5 clean full-suite runs at --max-concurrency 15 (was intermittent); daemon unit suite 91/91; daemon source compiles. The paths default to the env-derived locations when options are omitted, so the production CLI path is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
3cbb5c3bbd
commit
51e9351ed7
|
|
@ -31,6 +31,7 @@ export async function mintForCaller(opts: {
|
|||
request: MintRequest;
|
||||
tokenStore: SessionTokenStore;
|
||||
allowlistPath?: string;
|
||||
attemptsPath?: string;
|
||||
endpoint?: string;
|
||||
}): Promise<MintResponse | MintError> {
|
||||
const allowlist = await loadAllowlist(opts.allowlistPath);
|
||||
|
|
@ -42,6 +43,7 @@ export async function mintForCaller(opts: {
|
|||
rawIdentity: opts.callerIdentity,
|
||||
endpoint: opts.endpoint ?? '/auth/mint',
|
||||
reason: 'identity_not_allowed',
|
||||
path: opts.attemptsPath,
|
||||
});
|
||||
return { error: 'identity_not_allowed' };
|
||||
}
|
||||
|
|
@ -52,6 +54,7 @@ export async function mintForCaller(opts: {
|
|||
rawIdentity: opts.callerIdentity,
|
||||
endpoint: opts.endpoint ?? '/auth/mint',
|
||||
reason: 'capability_insufficient',
|
||||
path: opts.attemptsPath,
|
||||
});
|
||||
return { error: 'capability_insufficient' };
|
||||
}
|
||||
|
|
@ -73,6 +76,7 @@ export async function mintForCaller(opts: {
|
|||
rawIdentity: opts.callerIdentity,
|
||||
endpoint: opts.endpoint ?? '/auth/mint',
|
||||
reason: 'rate_limited',
|
||||
path: opts.attemptsPath,
|
||||
});
|
||||
return { error: 'rate_limited' };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,6 +30,12 @@ interface DaemonOptions {
|
|||
tailnetSocketPath?: string;
|
||||
tailnetSessionTtlSeconds?: number;
|
||||
pidfilePath?: string;
|
||||
// Explicit security-log + allowlist paths. Default to the env-var-derived
|
||||
// locations (defaultAuditPath etc.) when omitted, but passing them lets
|
||||
// concurrent test instances stay isolated without racing on process.env.
|
||||
auditPath?: string;
|
||||
attemptsPath?: string;
|
||||
allowlistPath?: string;
|
||||
// Test injection
|
||||
tunnelProvider?: () => Promise<DeviceTunnel | null>;
|
||||
whoIsImpl?: (addr: string) => Promise<{ identity: string; raw: unknown }>;
|
||||
|
|
@ -112,6 +118,9 @@ export async function startDaemon(opts: DaemonOptions): Promise<RunningDaemon |
|
|||
res,
|
||||
tokenStore,
|
||||
getTunnel,
|
||||
auditPath: opts.auditPath,
|
||||
attemptsPath: opts.attemptsPath,
|
||||
allowlistPath: opts.allowlistPath,
|
||||
whoIsImpl: opts.whoIsImpl ?? ((addr) => whoIs(addr, opts.tailnetSocketPath)),
|
||||
});
|
||||
});
|
||||
|
|
@ -172,6 +181,10 @@ interface HandlerCtx {
|
|||
res: ServerResponse;
|
||||
tokenStore: SessionTokenStore;
|
||||
getTunnel: () => Promise<DeviceTunnel | null>;
|
||||
// Explicit security-log + allowlist paths (default to env-derived when undefined).
|
||||
auditPath?: string;
|
||||
attemptsPath?: string;
|
||||
allowlistPath?: string;
|
||||
}
|
||||
|
||||
function readBody(req: IncomingMessage, maxBytes = 1_048_576): Promise<Buffer | { error: 'body_too_large' }> {
|
||||
|
|
@ -274,7 +287,7 @@ interface TailnetCtx extends HandlerCtx {
|
|||
* Tailnet handler — locked allowlist + capability tiers.
|
||||
*/
|
||||
async function handleTailnet(ctx: TailnetCtx): Promise<void> {
|
||||
const { req, res, tokenStore, getTunnel, whoIsImpl } = ctx;
|
||||
const { req, res, tokenStore, getTunnel, whoIsImpl, auditPath, attemptsPath, allowlistPath } = ctx;
|
||||
const url = parseUrl(req.url ?? '/');
|
||||
const path = url.pathname ?? '/';
|
||||
const method = req.method ?? 'GET';
|
||||
|
|
@ -304,6 +317,7 @@ async function handleTailnet(ctx: TailnetCtx): Promise<void> {
|
|||
rawIdentity: peerAddr,
|
||||
endpoint: route,
|
||||
reason: 'whois_unparseable',
|
||||
path: attemptsPath,
|
||||
});
|
||||
sendJson(res, 502, { error: 'whois_failed', detail: (err as Error).message });
|
||||
return;
|
||||
|
|
@ -318,6 +332,8 @@ async function handleTailnet(ctx: TailnetCtx): Promise<void> {
|
|||
request: parsed,
|
||||
tokenStore,
|
||||
endpoint: route,
|
||||
allowlistPath,
|
||||
attemptsPath,
|
||||
});
|
||||
|
||||
if ('error' in result) {
|
||||
|
|
@ -338,6 +354,7 @@ async function handleTailnet(ctx: TailnetCtx): Promise<void> {
|
|||
rawIdentity: token ? 'token:' + token.slice(0, 8) : 'no_token',
|
||||
endpoint: route,
|
||||
reason: validation.reason,
|
||||
path: attemptsPath,
|
||||
});
|
||||
const status = validation.reason === 'capability_insufficient' ? 403 : 401;
|
||||
sendJson(res, status, { error: validation.reason });
|
||||
|
|
@ -383,7 +400,7 @@ async function handleTailnet(ctx: TailnetCtx): Promise<void> {
|
|||
capability: session.capability,
|
||||
request_id: req.headers['x-request-id']?.toString() ?? '-',
|
||||
status: upstream.status,
|
||||
});
|
||||
}, auditPath);
|
||||
}
|
||||
|
||||
res.writeHead(upstream.status, upstream.headers);
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@
|
|||
// daemon source (ios-qa/daemon/test/*). This file tests the agent-flow
|
||||
// boundary — what the /ios-qa skill orchestrates end-to-end.
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import { createServer, type Server, type IncomingMessage } from 'http';
|
||||
import { mkdtempSync, rmSync, mkdirSync, writeFileSync, existsSync, readFileSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
|
|
@ -26,14 +26,27 @@ const HAS_DEVICE = process.env.GSTACK_HAS_IOS_DEVICE === '1';
|
|||
|
||||
const DEVICE_TOKEN = 'rotated-mock-bearer-token';
|
||||
|
||||
let workDir: string;
|
||||
// Per-test isolation under `bun test --concurrent`: a single module-level
|
||||
// `workDir` reassigned in beforeEach is clobbered by parallel tests, so they
|
||||
// collide on the same daemon pidfile (`already_running`) and stomp each
|
||||
// other's GSTACK_IOS_* env paths. Each test calls makeWorkDir() for its own
|
||||
// dir instead; afterEach cleans up every dir created during the test.
|
||||
const createdWorkDirs: string[] = [];
|
||||
function makeWorkDir(): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'ios-e2e-'));
|
||||
createdWorkDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
workDir = mkdtempSync(join(tmpdir(), 'ios-e2e-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(workDir, { recursive: true, force: true });
|
||||
// Clean up ONCE after all tests, not per-test. Under `bun test --concurrent`
|
||||
// an afterEach that drains the shared array would delete still-running tests'
|
||||
// workDirs the moment ANY test finishes, vanishing their audit/attempts files
|
||||
// mid-assertion. afterAll runs after every concurrent test has settled.
|
||||
afterAll(() => {
|
||||
for (const dir of createdWorkDirs) {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
createdWorkDirs.length = 0;
|
||||
});
|
||||
|
||||
interface StubState {
|
||||
|
|
@ -152,6 +165,7 @@ async function fetchJson(method: string, url: string, init: { headers?: Record<s
|
|||
|
||||
describe('ios-qa E2E (no-device path)', () => {
|
||||
test('NO_DEVICE: codegen runs against a SwiftUI fixture and emits valid accessors', () => {
|
||||
const workDir = makeWorkDir();
|
||||
const srcDir = join(workDir, 'app-src');
|
||||
mkdirSync(srcDir);
|
||||
writeFileSync(join(srcDir, 'AppState.swift'), `
|
||||
|
|
@ -183,6 +197,7 @@ class AppState {
|
|||
});
|
||||
|
||||
test('NO_DEVICE: cache hit on rerun', () => {
|
||||
const workDir = makeWorkDir();
|
||||
const srcDir = join(workDir, 'app-src');
|
||||
mkdirSync(srcDir);
|
||||
writeFileSync(join(srcDir, 'AppState.swift'), '@Observable class A { @Snapshotable var x: Int = 0 }');
|
||||
|
|
@ -194,6 +209,7 @@ class AppState {
|
|||
});
|
||||
|
||||
test('NO_DEVICE: schema mismatch returns 409 on restore', async () => {
|
||||
const workDir = makeWorkDir();
|
||||
const stub = await startStubStateServer({ loggedIn: false, username: '', rawTaps: [] });
|
||||
try {
|
||||
const tunnel: DeviceTunnel = {
|
||||
|
|
@ -237,6 +253,7 @@ class AppState {
|
|||
|
||||
describe('ios-qa E2E (agent-flow simulation)', () => {
|
||||
test('SCENARIO: acquire → snapshot → restore → tap → release', async () => {
|
||||
const workDir = makeWorkDir();
|
||||
const initial: StubState = { loggedIn: false, username: '', rawTaps: [] };
|
||||
const stub = await startStubStateServer(initial);
|
||||
try {
|
||||
|
|
@ -303,6 +320,7 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
});
|
||||
|
||||
test('SCENARIO: contention — second session-acquire returns 423 while first holds', async () => {
|
||||
const workDir = makeWorkDir();
|
||||
const stub = await startStubStateServer({ loggedIn: false, username: '', rawTaps: [] });
|
||||
try {
|
||||
const tunnel: DeviceTunnel = {
|
||||
|
|
@ -333,14 +351,16 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
});
|
||||
|
||||
test('SCENARIO: tailnet allowlist gate + mint + audit log', async () => {
|
||||
const workDir = makeWorkDir();
|
||||
const stub = await startStubStateServer({ loggedIn: false, username: '', rawTaps: [] });
|
||||
try {
|
||||
const allowPath = join(workDir, 'allowlist.json');
|
||||
const auditPath = join(workDir, 'audit.jsonl');
|
||||
const attemptsPath = join(workDir, 'attempts.jsonl');
|
||||
process.env.GSTACK_IOS_ALLOWLIST_PATH = allowPath;
|
||||
process.env.GSTACK_IOS_AUDIT_PATH = auditPath;
|
||||
process.env.GSTACK_IOS_ATTEMPTS_PATH = attemptsPath;
|
||||
// Pass paths as daemon OPTIONS, not process.env — env is process-global
|
||||
// and races across concurrent tests (the cause of the original
|
||||
// intermittent failures). GSTACK_IOS_TAILNET_BIND is read from env but
|
||||
// is the same constant for every tailnet test, so it can't diverge.
|
||||
process.env.GSTACK_IOS_TAILNET_BIND = '127.0.0.1';
|
||||
|
||||
const tunnel: DeviceTunnel = {
|
||||
|
|
@ -352,6 +372,9 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
const daemon = await startDaemon({
|
||||
loopbackPort: 0,
|
||||
tailnetEnabled: true,
|
||||
allowlistPath: allowPath,
|
||||
auditPath,
|
||||
attemptsPath,
|
||||
pidfilePath: join(workDir, 'daemon.pid'),
|
||||
tunnelProvider: async () => tunnel,
|
||||
probeImpl: async () => ({ ok: true, ownIdentity: 'mac@e2e' }),
|
||||
|
|
@ -406,9 +429,6 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
expect(attempts).toMatch(/"reason":"identity_not_allowed"/);
|
||||
} finally {
|
||||
await daemon.close();
|
||||
delete process.env.GSTACK_IOS_ALLOWLIST_PATH;
|
||||
delete process.env.GSTACK_IOS_AUDIT_PATH;
|
||||
delete process.env.GSTACK_IOS_ATTEMPTS_PATH;
|
||||
delete process.env.GSTACK_IOS_TAILNET_BIND;
|
||||
}
|
||||
} finally {
|
||||
|
|
@ -417,19 +437,20 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
});
|
||||
|
||||
test('SCENARIO: capability-tier enforcement — observe token cannot /tap', async () => {
|
||||
const workDir = makeWorkDir();
|
||||
const stub = await startStubStateServer({ loggedIn: false, username: '', rawTaps: [] });
|
||||
try {
|
||||
const allowPath = join(workDir, 'allowlist.json');
|
||||
process.env.GSTACK_IOS_ALLOWLIST_PATH = allowPath;
|
||||
process.env.GSTACK_IOS_AUDIT_PATH = join(workDir, 'audit.jsonl');
|
||||
process.env.GSTACK_IOS_ATTEMPTS_PATH = join(workDir, 'attempts.jsonl');
|
||||
|
||||
// Paths via daemon options, not process.env (concurrency-safe).
|
||||
const tunnel: DeviceTunnel = {
|
||||
udid: 'CAP-UDID', ipv6Addr: '127.0.0.1', port: stub.port, bootTokenRotated: DEVICE_TOKEN,
|
||||
};
|
||||
const daemon = await startDaemon({
|
||||
loopbackPort: 0,
|
||||
tailnetEnabled: true,
|
||||
allowlistPath: allowPath,
|
||||
auditPath: join(workDir, 'audit.jsonl'),
|
||||
attemptsPath: join(workDir, 'attempts.jsonl'),
|
||||
pidfilePath: join(workDir, 'daemon.pid'),
|
||||
tunnelProvider: async () => tunnel,
|
||||
probeImpl: async () => ({ ok: true, ownIdentity: 'mac@e2e' }),
|
||||
|
|
@ -463,9 +484,6 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
expect((tap.body as { error: string }).error).toBe('capability_insufficient');
|
||||
} finally {
|
||||
await daemon.close();
|
||||
delete process.env.GSTACK_IOS_ALLOWLIST_PATH;
|
||||
delete process.env.GSTACK_IOS_AUDIT_PATH;
|
||||
delete process.env.GSTACK_IOS_ATTEMPTS_PATH;
|
||||
}
|
||||
} finally {
|
||||
stub.server.close();
|
||||
|
|
@ -477,6 +495,7 @@ describe('ios-qa E2E (agent-flow simulation)', () => {
|
|||
|
||||
(HAS_DEVICE ? describe : describe.skip)('ios-qa E2E (with device)', () => {
|
||||
test('WITH_DEVICE: full agent loop against a real iPhone', () => {
|
||||
const workDir = makeWorkDir();
|
||||
// Stub — real implementation requires `devicectl` + an attached iPhone.
|
||||
// Documented in ios-qa/SKILL.md.tmpl under "Manual smoke test".
|
||||
expect(HAS_DEVICE).toBe(true);
|
||||
|
|
|
|||
Loading…
Reference in New Issue