mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 17:28:23 +01:00
fix(flaky): brain-sync auth check ignores the remote URL, browse binds its port after launch, three paid-eval clarity repairs (GST-1438) (#4035)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
6278ee4fb7
commit
8a51aeb2e5
12 files changed
+117
-8
No files matched your search
@@ -1472,8 +1472,11 @@ subcmd_once() {
|
||||
# Push. On reject, fetch + merge (merge driver handles JSONL) + retry once.
|
||||
local push_err
|
||||
push_err=$(git -C "$GSTACK_HOME" push origin HEAD 2>&1 >/dev/null) || {
|
||||
# Check if this is an auth error first — no point retrying.
|
||||
if echo "$push_err" | grep -qiE "auth|permission|403|401|forbidden"; then
|
||||
# Check if this is an auth error first — no point retrying. The remote's
|
||||
# URL or path (git's "To <url>" line and every quoted '<url>') is not
|
||||
# evidence: a remote under /tmp/x403y or named author-notes would read as
|
||||
# an auth failure and skip the fetch-and-merge retry.
|
||||
if printf '%s\n' "$push_err" | grep -v '^To ' | sed "s/'[^']*'//g" | grep -qiE "auth|permission|403|401|forbidden"; then
|
||||
local hint
|
||||
hint=$(remote_auth_hint)
|
||||
write_status "push_failed" "push failed: auth error; commit retained locally, will retry next run. fix: $hint"
|
||||
|
||||
@@ -1858,9 +1858,6 @@ export async function start() {
|
||||
safeUnlink(NETWORK_LOG_PATH);
|
||||
safeUnlink(DIALOG_LOG_PATH);
|
||||
|
||||
const port = await findPort();
|
||||
LOCAL_LISTEN_PORT = port;
|
||||
|
||||
// ─── Proxy config (D8 + codex F5) ──────────────────────────────
|
||||
// The CLI resolves the proxy (--proxy or env, #915). SOCKS5 with auth runs a
|
||||
// local 127.0.0.1 bridge to the authenticated upstream (Chromium can't do
|
||||
@@ -1946,6 +1943,11 @@ export async function start() {
|
||||
|
||||
const startTime = Date.now();
|
||||
|
||||
// The port is probed free and then bound; pick it after the browser launch so the seconds Chromium takes to start are not
|
||||
// a window in which another process can take it (Free Tests 41951644962: "Failed to start server. Is port 43780 in use?").
|
||||
const port = await findPort();
|
||||
LOCAL_LISTEN_PORT = port;
|
||||
|
||||
// ─── Build the request handlers via buildFetchHandler factory ───
|
||||
// CLI path passes env-derived values; no beforeRoute hook. Phoenix uses
|
||||
// the same factory with its own cfg + overlay hook.
|
||||
|
||||
@@ -77,4 +77,19 @@ describe('terminal-agent uses the shared allocator (static tripwire)', () => {
|
||||
expect(src).toContain("from './port-allocator'");
|
||||
expect(src).toMatch(/findAvailablePort\(BROWSE_PORT\)/);
|
||||
});
|
||||
|
||||
// Value: protects=the daemon probes its port after Chromium has launched, so the launch's seconds are not a window in which
|
||||
// another process takes it (Free Tests 41951644962: "Failed to start server. Is port 43780 in use?"); fails_when=findPort()
|
||||
// moves back ahead of the launch; why_new=nothing pinned the order; seam=none; no_claim=the probe-to-bind gap that remains
|
||||
test('start() picks its port after the browser launch, right before the bind', () => {
|
||||
const src = fs.readFileSync(SERVER_TS, 'utf-8');
|
||||
const body = src.slice(src.indexOf('export async function start()'));
|
||||
const launch = body.indexOf('await browserManager.launch();');
|
||||
const pick = body.indexOf('const port = await findPort();');
|
||||
const bind = body.indexOf('const server = Bun.serve({');
|
||||
expect(launch).toBeGreaterThan(0);
|
||||
expect(pick).toBeGreaterThan(launch);
|
||||
expect(bind).toBeGreaterThan(pick);
|
||||
expect(body.slice(pick, bind)).not.toMatch(/\bawait\b(?! findPort\(\))/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,6 @@
|
||||
### Fixed
|
||||
|
||||
- **`gstack-brain-sync` no longer reports a rejected push as an auth error when the remote's URL or path happens to contain `auth`, `permission`, `401`, `403` or `forbidden`.** It searched git's whole error text, including the `To <url>` line and the quoted URL, so a remote such as `github.com/author/notes` or a path under `/tmp/x403y` skipped the fetch-and-merge retry, and the sync stayed `push_failed` until someone pushed by hand. The check now ignores the URL. A real auth refusal (for example `The requested URL returned error: 403`) is still reported as one. In CI, the free shard's temporary folder name turned eight brain-sync tests red at once (Free Tests 41847262176).
|
||||
- **The browse daemon picks its port after Chromium has launched, right before it binds.** It used to check a random port first and bind it only after the browser launch, which takes seconds. Under load another process could take the port in that time, and the daemon failed with "Failed to start server. Is port … in use?" (Free Tests 41951644962).
|
||||
- **`test/gate-reuse.test.ts`**: the fixture's bare origin turns off auto-maintenance, as the pregate fixture already does. Each push started a detached `git maintenance run --auto` that could remove an object folder while a later push was writing into it (Free Tests 41980850843).
|
||||
- **Paid evals**: the ship doc-sync interface now says how to create the private folder it allows (one `mkdir` as its own call), after two `ship-docsync-store` runs each joined a no-op mkdir stand-in to a read line (41850017482, 41987975678). The coverage-audit prompt asks for the two file reads after the skill files, so the skill's dedicated-read step is seen before the files are read (`plan-eng-coverage-audit`, 41932813292). The QA-only cleanup names the terminal record, port file, daemon and daemon-log state when it can't confirm the terminal's identity (`qa-only-no-fix`, 41962523474).
|
||||
@@ -1015,6 +1015,41 @@ describe('gstack-brain-sync per-file hold', () => {
|
||||
expect(statusJson().message).not.toContain('unmerged');
|
||||
expect(statusJson().message).toContain(`Merge conflict in ${SHARED}`);
|
||||
});
|
||||
|
||||
// Value: protects=a rejected push still fetches and merges when the remote's path reads like an auth failure (8 tests red at
|
||||
// once in Free Tests 41847262176 shard 3: the shard's TMPDIR named it); fails_when=the classifier greps git's "To <url>" line
|
||||
// or a quoted '<url>'; why_new=no test put an auth word in the remote path; seam=none; no_claim=real network auth failures
|
||||
test('a rejected push to a remote whose path reads like an auth failure still merges', () => {
|
||||
const parent = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-sync-oauth-403-permission-'));
|
||||
fs.rmSync(bareRemote, { recursive: true, force: true });
|
||||
bareRemote = path.join(parent, 'author-401-forbidden.git');
|
||||
try {
|
||||
expect(spawnSync('git', ['init', '--bare', '-q', '-b', 'main', bareRemote], { timeout: 30_000 }).status).toBe(0);
|
||||
setup();
|
||||
peerCommit(SHARED, '{"note":"shared from the peer"}\n');
|
||||
write(CLEAN, '# a plan\n');
|
||||
expect(drain().status).toBe(0);
|
||||
expect(statusJson().status).toBe('ok');
|
||||
expect(remoteFiles()).toContain(CLEAN);
|
||||
expect(remoteShow(SHARED).stdout).toBe('{"note":"shared from the peer"}\n');
|
||||
} finally {
|
||||
fs.rmSync(parent, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a real auth refusal still stops before the retry merge, with the keyword after git's quoted URL;
|
||||
// fails_when=the URL strip also drops the rest of git's line; why_new=the auth branch had no test; seam=git PATH shim;
|
||||
// no_claim=which wording each forge prints, the shim stands in for the server
|
||||
test('a push refused for auth is still reported as an auth error', () => {
|
||||
setup();
|
||||
write(CLEAN, '# a plan\n');
|
||||
const shim = gitShim(`case " $* " in *" push "*) echo "fatal: unable to access '${bareRemote}/': The requested URL returned error: 403" >&2; exit 128;; esac`);
|
||||
expect(drain(shim.env).status).toBe(0);
|
||||
const s = statusJson();
|
||||
expect(s.status).toBe('push_failed');
|
||||
expect(s.message).toContain('push failed: auth error');
|
||||
expect(shim.calls().some(call => /(^| )fetch( |$)/.test(call))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
test('a git add failure is reported, never swallowed', () => {
|
||||
|
||||
@@ -229,6 +229,7 @@ mock.module(path.join(root, 'test/helpers/session-runner.ts'), () => ({ runSkill
|
||||
expect(opts.allowedTools).toEqual(['Bash','Read','Write','Edit','Glob','Grep']);
|
||||
expect(opts.signal).toBeInstanceOf(AbortSignal);
|
||||
expect(opts.prompt).not.toContain('Step 4.75'); expect(opts.prompt).not.toContain('Step 3.4'); expect(opts.prompt).not.toContain('coverage-read-evidence:');
|
||||
if (opts.testName !== 'ship-coverage-audit') expect(opts.prompt).toContain("Read those two files only after the skill files above, and read them the way the skill's own read step says.");
|
||||
if (opts.testName === 'review-coverage-audit') expect(opts.prompt).toContain('review/specialists/testing.md');
|
||||
else if (opts.testName === 'plan-eng-coverage-audit') { expect(opts.prompt).toContain('plan-eng-review/sections/review-sections.md'); expect(opts.prompt).toContain('3. Test review'); }
|
||||
else { expect(opts.testName).toBe('ship-coverage-audit'); expect(opts.prompt).toContain('Step 7'); expect(opts.prompt).toContain('ship/sections/test-coverage.md'); }
|
||||
|
||||
@@ -706,3 +706,21 @@ test.skipIf(process.platform === 'win32')('the captured private-directory mkdir
|
||||
} finally { fs.rmSync(outside, { recursive: true, force: true }); }
|
||||
} finally { fixture.clean(); }
|
||||
});
|
||||
|
||||
// Value: protects=the interface says how the private directory it offers is made, the form the observer admits; fails_when=the
|
||||
// sentence stops naming the own-call mkdir; why_new=ship-docsync-store went red twice (#3997 screen 41850017482 slice 2, census
|
||||
// 41987975678 slice 4) on one read line each that carried a no-op mkdir stand-in; seam=none; no_claim=that the model now never
|
||||
// probes, which only paid runs measure
|
||||
test('the interface names its one mkdir form, and both captured mkdir stand-ins stay refused', () => {
|
||||
const fixture = fixtureDocs('store');
|
||||
try {
|
||||
const publish = path.join(fixture.home, 'fixture-publish.ts');
|
||||
const sentence = docsNativeInterface(fixture, [publish]).split(/(?<=\.)\s+/).find(part => part.startsWith('A private JSON/Markdown artifact'));
|
||||
expect(sentence).toContain(`(with one mkdir ${fixture.home}/<name>, optionally -p, as its own Bash call: the only mkdir this interface admits`);
|
||||
expect(docsCommandAllowed(`mkdir ${fixture.home}/docs-store`, fixture, [publish])).toBe(true);
|
||||
for (const line of [
|
||||
`git rev-parse HEAD; git merge-base main HEAD; git rev-parse origin/main; mkdir -p /dev/null 2>/dev/null; ls ${fixture.home}/state`,
|
||||
`git tag --list; git log --oneline --all; mkdir_check=1; ls ${fixture.home}`,
|
||||
]) expect(docsCommandAllowed(line, fixture, [publish]), line).toBe(false);
|
||||
} finally { fixture.clean(); }
|
||||
});
|
||||
@@ -57,6 +57,8 @@ beforeAll(() => {
|
||||
origin = path.join(scratch, 'origin.git');
|
||||
git(scratch, 'init', '-q', '--bare', origin);
|
||||
git(origin, 'config', 'uploadpack.allowAnySHA1InWant', 'true');
|
||||
git(origin, 'config', 'maintenance.auto', 'false');
|
||||
git(origin, 'config', 'gc.auto', '0');
|
||||
const work = path.join(scratch, 'work');
|
||||
git(scratch, 'clone', '-q', origin, work);
|
||||
const policy = fs.readFileSync(path.join(ROOT, '.gstack/ship-policy.json'), 'utf8');
|
||||
@@ -184,6 +186,23 @@ function find(head: string, runs: WorkflowRun[], bindings: Record<number, GateBi
|
||||
return { ...result, out, text: result.lines.join('\n') };
|
||||
}
|
||||
|
||||
describe('fixture origin', () => {
|
||||
// Value: protects=a push into the bare origin starts no detached auto-maintenance there, so a later push never writes into an
|
||||
// object directory a repack just removed (unable to write ./objects/3f/…, Free Tests 41980850843 shard 5); fails_when=the origin
|
||||
// loses maintenance.auto=false; why_new=local transport drops the runner's GIT_CONFIG_COUNT backstop; seam=none;
|
||||
// no_claim=which maintenance task removed the directory on the CI runner's git
|
||||
test('a push into the origin starts no auto-maintenance even when the runner backstop is dropped', () => {
|
||||
const trace = path.join(scratch, 'trace2.txt');
|
||||
const backstop = { GIT_CONFIG_COUNT: '2', GIT_CONFIG_KEY_0: 'maintenance.auto', GIT_CONFIG_VALUE_0: 'false', GIT_CONFIG_KEY_1: 'gc.auto', GIT_CONFIG_VALUE_1: '0' };
|
||||
const push = spawnSync('git', ['push', '-q', 'origin', 'HEAD:refs/heads/maintenance-probe'],
|
||||
{ cwd: path.join(scratch, 'work'), encoding: 'utf8', timeout: 30_000, env: { ...process.env, ...backstop, GIT_TRACE2: trace } });
|
||||
expect(push.status, push.stderr).toBe(0);
|
||||
const events = fs.readFileSync(trace, 'utf8');
|
||||
expect(events).toContain('receive-pack');
|
||||
expect(events.split('\n').filter(line => line.includes('child_start') && line.includes('maintenance'))).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('find: the positive case', () => {
|
||||
test('a restamp of a GREEN head on the same base yields a candidate with the receipt', () => {
|
||||
const { candidate, out, text } = find(sha.restamp!, [run(300, sha.gated!)], { 300: binding() });
|
||||
|
||||
@@ -21,7 +21,7 @@ then
|
||||
${DOCS_AUDIT_RESET}
|
||||
and the cap gate, which reads origin/main's policy and whose DOCS_CAP lines and exit status decide it
|
||||
${docsCapsCommand(skills)}
|
||||
(no --json, no other base, no other gstack-ship-receipt subcommand). ${readOnlyInterfaceText(fixture.home)} The Git reads above may join such a line too, but not in a line with cd. So may the installed ${skills}/bin/gstack-docs-candidate --help, alone or after snapshot or compare, because it only prints usage (the one exception); neither a joined Git read nor that --help may be fed by a pipe. A for loop over literal words (a lowercase loop variable; words without quotes, globs or $) whose body uses only such reads counts as those reads, one pass per word, wherever it sits in the line. Every read stays off actor-state.json and off ${docsConfigDir(fixture.home)} except its settings.json: that directory holds Claude's credentials and every agent's session transcript, so judge a child only by the output it returns (its echoed SESSION_KIND line included) and by the repository. The additional scripts below also run each as its own Bash call; no other own-call command ever joins such a line. Otherwise no shell composition, custom interpreters, arbitrary scripts, inline eval or memory-mapped writes. The only additional scripts are ${scripts.length ? scripts.join(', ') : 'none'}.${insert ? ` To insert one saved private Markdown artifact into another (a saved section file into a report), Bash may also run the single literal command cat SOURCE.md >> TARGET.md with absolute paths under ${fixture.home}.` : ''} Read/Glob/Grep remain available; Read skill and section files with Read (offset/limit for ranges), because Bash output over 30KB becomes a truncated preview. A private JSON/Markdown artifact (the snapshot --out file, saved child output, a section or report file) lives directly under ${fixture.home} or in a new directory you create there, never inside the repository, ${skills}, ${docsConfigDir(fixture.home)}, ${state} (gstack's own state directory, not a place for your files) or ${fixture.home}/remote.git. Use Write/Edit for permitted docs, those private artifacts and Step 14.5's receipt record ${fixture.repo}/${DOCS_AUDIT_RECORD} (the only permitted write under the repository besides the document, excluded from git by the line above); do not rewrite installed skills, config, actor state or scripts. No effects outside the owned fixture. The owner preserves evidence and cleans up. Missing observer coverage blocks acceptance; the Linux kernel monitor covers syscall writes in the product tree, not hostile processes or arbitrary external destinations.
|
||||
(no --json, no other base, no other gstack-ship-receipt subcommand). ${readOnlyInterfaceText(fixture.home)} The Git reads above may join such a line too, but not in a line with cd. So may the installed ${skills}/bin/gstack-docs-candidate --help, alone or after snapshot or compare, because it only prints usage (the one exception); neither a joined Git read nor that --help may be fed by a pipe. A for loop over literal words (a lowercase loop variable; words without quotes, globs or $) whose body uses only such reads counts as those reads, one pass per word, wherever it sits in the line. Every read stays off actor-state.json and off ${docsConfigDir(fixture.home)} except its settings.json: that directory holds Claude's credentials and every agent's session transcript, so judge a child only by the output it returns (its echoed SESSION_KIND line included) and by the repository. The additional scripts below also run each as its own Bash call; no other own-call command ever joins such a line. Otherwise no shell composition, custom interpreters, arbitrary scripts, inline eval or memory-mapped writes. The only additional scripts are ${scripts.length ? scripts.join(', ') : 'none'}.${insert ? ` To insert one saved private Markdown artifact into another (a saved section file into a report), Bash may also run the single literal command cat SOURCE.md >> TARGET.md with absolute paths under ${fixture.home}.` : ''} Read/Glob/Grep remain available; Read skill and section files with Read (offset/limit for ranges), because Bash output over 30KB becomes a truncated preview. A private JSON/Markdown artifact (the snapshot --out file, saved child output, a section or report file) lives directly under ${fixture.home} or in a new directory you create there (with one mkdir ${fixture.home}/<name>, optionally -p, as its own Bash call: the only mkdir this interface admits, so no other mkdir, not even a no-op one, and nothing joined to it), never inside the repository, ${skills}, ${docsConfigDir(fixture.home)}, ${state} (gstack's own state directory, not a place for your files) or ${fixture.home}/remote.git. Use Write/Edit for permitted docs, those private artifacts and Step 14.5's receipt record ${fixture.repo}/${DOCS_AUDIT_RECORD} (the only permitted write under the repository besides the document, excluded from git by the line above); do not rewrite installed skills, config, actor state or scripts. No effects outside the owned fixture. The owner preserves evidence and cleans up. Missing observer coverage blocks acceptance; the Linux kernel monitor covers syscall writes in the product tree, not hostile processes or arbitrary external destinations.
|
||||
|
||||
The working directory for parent and child Bash calls is already ${fixture.repo}. Run Git reads directly, for example: git status, git diff --cached, git merge-base main HEAD, git rev-parse HEAD. Do not use Git global options such as -C, -c, --git-dir or --work-tree, and do not prepend cd or another shell wrapper. The literal git subcommand must immediately follow git; an absolute owned repository path does not make git -C an allowed command.
|
||||
|
||||
|
||||
@@ -53,7 +53,16 @@ async function settleOwnedBrowser(directory: string, deadlineEpochMs: number): P
|
||||
|| typeof state.instanceId !== 'string' || !state.instanceId) fail('invalid daemon identity');
|
||||
const agentRaw = fs.existsSync(agentFile) ? fs.readFileSync(agentFile, 'utf8') : undefined;
|
||||
const agent = readAgentRecord(stateDir);
|
||||
if (!agentRaw || !agent) fail('terminal identity is unavailable');
|
||||
if (!agentRaw || !agent) {
|
||||
// Qa-only-no-fix (census 41962523474) failed here once with nothing else to go on; name what was there instead.
|
||||
const log = path.join(stateDir, 'browse-server.log');
|
||||
const agentLines = fs.existsSync(log) && fs.lstatSync(log).isFile()
|
||||
? fs.readFileSync(log, 'utf8').split('\n').filter(line => /terminal-agent/i.test(line)).slice(-3).map(line => line.slice(0, 160)) : [];
|
||||
fail(`terminal identity is unavailable (record ${agentRaw === undefined ? 'absent' : `unparsed, ${agentRaw.length} bytes`}; `
|
||||
+ `terminal-port ${fs.existsSync(path.join(stateDir, 'terminal-port')) ? 'present' : 'absent'}; daemon ${state.pid} `
|
||||
+ `${isProcessAlive(state.pid) ? 'alive' : 'exited'}, state written ${Math.round((Date.now() - fs.statSync(stateFile).mtimeMs) / 1000)}s ago; `
|
||||
+ `daemon log terminal-agent lines: ${agentLines.length ? agentLines.join(' | ') : 'none'})`);
|
||||
}
|
||||
if (!isProcessAlive(state.pid)) fail('daemon exited before its owned processes could be identified');
|
||||
const daemonStart = readPidStartTime(state.pid);
|
||||
const command = readPidCmdline(state.pid);
|
||||
|
||||
@@ -113,7 +113,7 @@ Bun.spawnSync = (...args) => {
|
||||
const started = performance.now();
|
||||
const errors = { 'ack-only': 'settlement deadline exceeded', 'replaced-state': 'daemon state was replaced',
|
||||
'terminal-mismatch': 'terminal ownership is unconfirmed', 'chromium-mismatch': 'Chromium ownership is unconfirmed',
|
||||
'missing-terminal': 'terminal identity is unavailable', 'blocked-identity': 'owned worker settlement deadline exceeded' };
|
||||
'missing-terminal': 'terminal identity is unavailable (record absent; terminal-port', 'blocked-identity': 'owned worker settlement deadline exceeded' };
|
||||
await expect(stopQaOnlyBrowser(dir, 1000)).rejects.toThrow(errors[scenario]);
|
||||
expect(performance.now() - started).toBeLessThan(2000);
|
||||
expect(daemon.exitCode).toBeNull();
|
||||
|
||||
@@ -63,6 +63,7 @@ You are on the feature/billing branch. The base branch is main.
|
||||
This is a test project — there is no remote, no PR to create.
|
||||
The source code is in ${cwd}/src/billing.ts.
|
||||
Existing tests are in ${cwd}/test/billing.test.ts.
|
||||
Read those two files only after the skill files above, and read them the way the skill's own read step says.
|
||||
|
||||
Produce the ASCII coverage diagram showing which code paths are tested and which have gaps.
|
||||
Output the diagram directly, name both billing functions, and include a coverage summary.
|
||||
|
||||
Reference in new issue
Block a user