From dbb4b5064c3757354231f8dbb65de5b476949e2a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 10 Nov 2026 07:55:42 -0800 Subject: [PATCH] Fix six test and eval checks that misread correct work or raced their own fixtures (GST-2338) (#4284) * Fix six test and eval checks that misread correct work or raced their own fixtures (GST-2338) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Credit a one-sentence defer offer again and limit the /dev/null goto credit to file:// targets with no || before it Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/specialist-batched#1 reader=specialist-batched Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Refuse the /dev/null goto credit only when || sits directly before the goto's pipeline Review-Finding: p2/red-team#3 reader=red-team 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> --- changelog.d/fix-flaky-checks-2026-11-10.md | 1 + .../test/helpers/session-runner.ts.json | 2 +- test/cso-registry-socket.test.ts | 4 +- test/cso-snapshot-state.test.ts | 2 +- test/docsync-command-grammar.test.ts | 23 ++++++++++ test/helpers/docsync-interface.ts | 2 +- test/helpers/docsync-observer.ts | 4 +- test/helpers/session-runner.test.ts | 34 +++++++++++++++ test/helpers/session-runner.ts | 42 ++++++++++++++++--- test/helpers/third-party-actions.ts | 22 ++++++++-- test/skill-e2e-skillify.test.ts | 2 +- test/third-party-actions.test.ts | 21 +++++++++- 12 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 changelog.d/fix-flaky-checks-2026-11-10.md diff --git a/changelog.d/fix-flaky-checks-2026-11-10.md b/changelog.d/fix-flaky-checks-2026-11-10.md new file mode 100644 index 000000000..6ba4b6534 --- /dev/null +++ b/changelog.d/fix-flaky-checks-2026-11-10.md @@ -0,0 +1 @@ +- **Six test and eval checks that misread correct work, or raced their own fixtures, now read it right (2026-11-10 daily flaky pass, GST-2338).** ship-docsync-store's fixture interface now declares the `gstack-docs-candidate attempt --state ` call that /ship's documentation step has asked for since #4192, so a model that follows the step is no longer stopped by the fixture (census 42748129558). tpa-absent-darwin no longer counts a gstack-browser option as an Aside drive just because it says Aside isn't available. tpa-present's defer check counts a "the step stays blocked on you" status line only when the option puts the step off ("later", "not yet", "not now"), so a status line after another instruction no longer passes as a defer offer. scrape-prototype-path also credits `$B goto >/dev/null && $B html` when the call proves the goto exited 0. Two free tests stop racing themselves: the CSO lease contender publishes its result file by rename, so the test never reads it empty, and the registry-socket deadline test sets a deadline that expires after the watchdog's startup handshake instead of inside it. No threshold, trial count, case kind or retry changed. diff --git a/scripts/harness-pins/test/helpers/session-runner.ts.json b/scripts/harness-pins/test/helpers/session-runner.ts.json index b1dc4ddd3..c81f216e3 100644 --- a/scripts/harness-pins/test/helpers/session-runner.ts.json +++ b/scripts/harness-pins/test/helpers/session-runner.ts.json @@ -1 +1 @@ -{"blob":"0d5f36f91c31bf32d4997cde0c267f96caf57c7a","since":6,"nonBehavioral":"adds helpers called only by the changed scrape-prototype-path, codex-review and ship-coverage-value case files and turns off git auto-maintenance in the AUQ capture fixture; no existing caller's behavior changes (GST-2131)"} +{"blob":"6a0fad8792221a97dd27c01837af368be81d5bf5","since":6,"nonBehavioral":"widens browseNavigatedTo, whose only caller is the changed scrape-prototype-path case file, to credit a goto with stdout sent to /dev/null when the call proves it exited 0 (a file:// target, no || directly before the goto's pipeline, only && after it); no other caller or case changes (GST-2338)"} diff --git a/test/cso-registry-socket.test.ts b/test/cso-registry-socket.test.ts index 02352e3a0..a311d6441 100644 --- a/test/cso-registry-socket.test.ts +++ b/test/cso-registry-socket.test.ts @@ -80,7 +80,9 @@ posixDescribe('short supervised CSO registry sockets', () => { }, 15_000); test('deadline cleanup removes the whole per-call socket root without a global temp sweep', async () => { - const doomed = await superviseRegistrySocket({ watchdogPath, ownerPid: process.pid, deadline: Date.now() + 150 }); + // The watchdog takes whole seconds and cleans as soon as the deadline passes, so a deadline inside the ~1 s startup + // handshake fails it on a slow start (Free Tests 42772387147): expire it after the handshake instead. + const doomed = await superviseRegistrySocket({ watchdogPath, ownerPid: process.pid, deadline: Date.now() + 2_000 }); const survivor = await superviseRegistrySocket({ watchdogPath, ownerPid: process.pid, deadline: Date.now() + 10_000 }); const broker = new RegistryEgressBroker(doomed.socketPath, ['registry.npmjs.org'], Date.now() + 10_000, 1024 * 1024); try { diff --git a/test/cso-snapshot-state.test.ts b/test/cso-snapshot-state.test.ts index a18799a73..3357e8b54 100644 --- a/test/cso-snapshot-state.test.ts +++ b/test/cso-snapshot-state.test.ts @@ -17,7 +17,7 @@ const read=fs.readdirSync.bind(fs);spyOn(fsModule,'readdirSync').mockImplementat spyOn(fsModule,'writeFileSync').mockImplementation(((target,data,...args)=>{const name=path.basename(String(target));if(name.includes('.decision.tmp.')){let decision;try{decision=JSON.parse(String(data));}catch{}if(decision?.kind==='ticket'&&decision.token===token){try{mark('choice-'+id,decision.ticket);}catch{}if(mode==='pause-before-decision'&&!triggered){triggered=true;mark('paused-'+id);process.kill(process.pid,'SIGSTOP');}}if(decision?.kind==='withdraw'&&mode==='pause-before-withdraw'&&!triggered){triggered=true;mark('paused-'+id);process.kill(process.pid,'SIGSTOP');}}return write(target,data,...args);}) as typeof fs.writeFileSync); const link=fs.linkSync.bind(fs);spyOn(fsModule,'linkSync').mockImplementation(((source,target)=>{const name=path.basename(String(target));if(mode==='wait-active'&&!triggered&&name.includes('.active.')){triggered=true;mark('doorway-'+id);waitFor('paused-H');}return link(source,target);}) as typeof fs.linkSync); const unlink=fs.unlinkSync.bind(fs);spyOn(fsModule,'unlinkSync').mockImplementation(((target)=>{const result=unlink(target),name=path.basename(String(target)),candidateToken=name.endsWith('.json')?name.slice(0,-5):'';if((mode==='pause-decision'||mode==='pause-ticket')&&!triggered&&name.includes('.decision.tmp.')&&(!token||name.startsWith(token+'.decision.tmp.'))){triggered=true;mark('paused-'+id);process.kill(process.pid,'SIGSTOP');}if((mode==='pause-after-candidate-cleanup'||mode==='pause-withdraw-cleanup')&&!triggered&&/^[a-f0-9]{32}$/.test(candidateToken)&&(!token||candidateToken===token)&&fs.existsSync(path.join(leases,candidateToken+'.decision'))){triggered=true;mark('paused-'+id);process.kill(process.pid,'SIGSTOP');}return result;}) as typeof fs.unlinkSync); -const {withLock}=await import(${JSON.stringify(path.resolve(import.meta.dir,'../lib/cso/state.ts'))});let outcome='success';try{withLock(dir,()=>{const active=path.join(barrier,'work-active');try{write(active,id,{flag:'wx'});}catch{fs.appendFileSync(path.join(barrier,'overlap'),'overlap\\n');}mark('entered-'+id);if(process.env.RACE_HOLD==='1')waitFor('release-'+id);try{if(fs.readFileSync(active,'utf8')===id)fs.unlinkSync(active);}catch{}});}catch(error){outcome=String(error?.code??error);}write(path.join(barrier,'result-'+id),outcome);`;} +const {withLock}=await import(${JSON.stringify(path.resolve(import.meta.dir,'../lib/cso/state.ts'))});let outcome='success';try{withLock(dir,()=>{const active=path.join(barrier,'work-active');try{write(active,id,{flag:'wx'});}catch{fs.appendFileSync(path.join(barrier,'overlap'),'overlap\\n');}mark('entered-'+id);if(process.env.RACE_HOLD==='1')waitFor('release-'+id);try{if(fs.readFileSync(active,'utf8')===id)fs.unlinkSync(active);}catch{}});}catch(error){outcome=String(error?.code??error);}const result=path.join(barrier,'result-'+id);write(result+'.tmp',outcome);fs.renameSync(result+'.tmp',result);`;} describe('CSO dirty snapshot boundary',()=>{ test('captures tracked modifications and nonignored untracked source without changing Git state',async()=>{const p=repo();fs.writeFileSync(path.join(p,'tracked.txt'),'dirty\n');fs.writeFileSync(path.join(p,'new.txt'),'untracked\n');fs.writeFileSync(path.join(p,'.gitignore'),'ignored.txt\n');fs.writeFileSync(path.join(p,'ignored.txt'),'ignored\n');const before=git(p,'status','--porcelain=v1','-z'),run=newRun(p),m=await capture(p,run.dir,'HEAD');expect(fs.readFileSync(path.join(run.dir,'snapshot','tracked.txt'),'utf8')).toBe('dirty\n');expect(fs.readFileSync(path.join(run.dir,'snapshot','new.txt'),'utf8')).toBe('untracked\n');expect(fs.existsSync(path.join(run.dir,'snapshot','ignored.txt'))).toBe(false);expect(git(p,'status','--porcelain=v1','-z')).toBe(before);expect(m.changedPaths).toContain('new.txt');}); test('records an untracked nested repository as one boundary entry instead of failing the run',async()=>{const p=repo(),inner=path.join(p,'inner');fs.mkdirSync(inner);git(inner,'init','-q');git(inner,'config','user.email','fixture@example.test');git(inner,'config','user.name','Fixture');fs.writeFileSync(path.join(inner,'f.txt'),'x\n');git(inner,'add','.');git(inner,'commit','-qm','inner');const run=newRun(p),manifest=await capture(p,run.dir,'HEAD');assertSnapshot(run.dir,manifest);const entry=manifest.entries.find(e=>e.path==='inner');expect(entry?.transformation).toBe(NESTED_REPOSITORY);expect(entry?.originalHash).toBe('not-read');expect(manifest.entries.map(e=>e.path).filter(p=>p==='tracked.txt'||p.startsWith('inner'))).toEqual(['inner','tracked.txt']);expect(manifest.changedPaths).toContain('inner');expect(fs.existsSync(path.join(run.dir,'snapshot','inner'))).toBe(false);expect(fs.existsSync(path.join(run.dir,'readable','inner'))).toBe(false);}); diff --git a/test/docsync-command-grammar.test.ts b/test/docsync-command-grammar.test.ts index 0a26c7266..ad9743cff 100644 --- a/test/docsync-command-grammar.test.ts +++ b/test/docsync-command-grammar.test.ts @@ -258,6 +258,29 @@ test('the docs-candidate result command is admitted only with a private output f expect(docsNativeInterface(fixture)).toContain(`${fixture.skills.split(path.sep).join('/')}/bin/gstack-docs-candidate result --audit-id --out-dir <`); }); +// Census 42748129558 (ship-docsync-store): Step 14.5 counts each launch with `gstack-docs-candidate attempt` (#4192), which +// this interface did not declare, so the parent correctly stopped before launching any audit. +test('the docs-candidate attempt command is admitted only with a private child output and a private --state .json', () => { + const helper = path.join(fixture.skills, 'bin/gstack-docs-candidate'); + const output = path.join(fixture.home, 'audit-1-output.md'); + const state = path.join(fixture.home, 'docs-attempts.json'); + for (const command of [`${helper} attempt ${output} --state ${state}`, `${helper} attempt --help`]) + expect(docsCommandAllowed(command, fixture), command).toBe(true); + for (const command of [ + `${helper} attempt ${output} --state ${path.join(fixture.repo, '.gstack/tmp/docs-attempts.json')}`, + `${helper} attempt ${output} --state ${path.join(fixture.home, 'docs-attempts.md')}`, + `${helper} attempt ${output} --state ${path.join(fixture.skills, 'bin/docs-attempts.json')}`, + `${helper} attempt ${path.join(fixture.repo, 'out.md')} --state ${state}`, + `${helper} attempt ${output}`, + `${helper} attempt --state ${state} ${output}`, + `${helper} attempt ${output} --state ${state} --exec rm`, + `${helper} attempt ${output} --out ${state}`, + `${helper} attempt --help extra`, + `~/.claude/skills/gstack/bin/gstack-docs-candidate attempt ${output} --state ${state}`, + ]) expect(docsCommandAllowed(command, fixture), command).toBe(false); + expect(docsNativeInterface(fixture)).toContain(`${fixture.skills.split(path.sep).join('/')}/bin/gstack-docs-candidate attempt --state and attempt --help`); +}); + // Train gate 39904966815 and GSTA-63's focused run joined the candidate's --help onto a line of reads; v1.91.185.0 moved the // own-call rule into the sentence naming --help and the slip came back four times (JOINED_READ_LINES below). The --help only // prints usage, so it now joins a line of reads like a Git read; every other own-call command stays alone. diff --git a/test/helpers/docsync-interface.ts b/test/helpers/docsync-interface.ts index 90113471f..485fa3bec 100644 --- a/test/helpers/docsync-interface.ts +++ b/test/helpers/docsync-interface.ts @@ -15,7 +15,7 @@ export const docsCapsCommand = (skills: string) => `${skills}/bin/gstack-ship-re export function docsNativeInterface(fixture: Pick, 'home' | 'repo' | 'skills'> & Partial, 'env'>>, scripts: string[] = [], transport = false, insert = false, file?: string): string { const skills = fixture.skills.split(path.sep).join('/'); const state = fixture.env?.GSTACK_HOME ?? path.join(fixture.home, 'state'); - return `Fixture observation interface (applies to parent and every child; ${file ? `saved at ${file}: each child prompt tells the child to Read that file first instead of copying this text` : 'include this interface in child prompts'}): Bash may execute only separate literal pwd, ls, cat, stat, sha256sum and Git read commands (status, diff, show, log, ls-files, ls-tree, check-ignore, rev-parse, merge-base, hash-object without -w, branch listings with no operand and only --show-current, -a/--all, -r/--remotes, -v/--verbose, -l/--list, --no-color or --no-abbrev (no --merged, --sort, --format or --contains), tag listings with no operand and only -l/--list, describe with only --tags, --always, --abbrev= or --exact-match and at most one revision). Bash may also run each of the following as its own Bash call, with nothing joined before or after it by ;, &&, || or a pipe (one exception follows the read grammar below): the exact generated Preamble block with its spawned prefix, literal installed gstack-skill-start/gstack-skill-end commands for document-release (start requires GSTACK_SESSION_KIND=spawned) or their own --help, or the literal installed ${skills}/bin/gstack-docs-candidate snapshot (its --out a private .json under ${fixture.home}), compare and --help commands, the literal installed ${skills}/bin/gstack-docs-candidate result --audit-id --out-dir <${fixture.home} or a new directory directly under it> and result --help, and /ship Step 14.5's three own-tree housekeeping commands exactly as written, each on its own line: the shared .gstack/tmp exclude line (the one &&-joined housekeeping line this interface admits) + return `Fixture observation interface (applies to parent and every child; ${file ? `saved at ${file}: each child prompt tells the child to Read that file first instead of copying this text` : 'include this interface in child prompts'}): Bash may execute only separate literal pwd, ls, cat, stat, sha256sum and Git read commands (status, diff, show, log, ls-files, ls-tree, check-ignore, rev-parse, merge-base, hash-object without -w, branch listings with no operand and only --show-current, -a/--all, -r/--remotes, -v/--verbose, -l/--list, --no-color or --no-abbrev (no --merged, --sort, --format or --contains), tag listings with no operand and only -l/--list, describe with only --tags, --always, --abbrev= or --exact-match and at most one revision). Bash may also run each of the following as its own Bash call, with nothing joined before or after it by ;, &&, || or a pipe (one exception follows the read grammar below): the exact generated Preamble block with its spawned prefix, literal installed gstack-skill-start/gstack-skill-end commands for document-release (start requires GSTACK_SESSION_KIND=spawned) or their own --help, or the literal installed ${skills}/bin/gstack-docs-candidate snapshot (its --out a private .json under ${fixture.home}), compare and --help commands, the literal installed ${skills}/bin/gstack-docs-candidate result --audit-id --out-dir <${fixture.home} or a new directory directly under it> and result --help, the literal installed ${skills}/bin/gstack-docs-candidate attempt --state and attempt --help, and /ship Step 14.5's three own-tree housekeeping commands exactly as written, each on its own line: the shared .gstack/tmp exclude line (the one &&-joined housekeeping line this interface admits) ${DOCS_TMP_EXCLUDE} then ${DOCS_AUDIT_RESET} diff --git a/test/helpers/docsync-observer.ts b/test/helpers/docsync-observer.ts index 0630b1fac..00ac6a56a 100644 --- a/test/helpers/docsync-observer.ts +++ b/test/helpers/docsync-observer.ts @@ -414,8 +414,10 @@ function literalDocsCommandAllowed(text: string, fixture: ReturnType !!file && path.isAbsolute(file) && /\.json$/i.test(file) && docsPrivateArtifact(file, fixture, scripts); if (rest.length === 1 && rest[0] === '--help') return true; - if (rest.length === 2 && (rest[0] === 'snapshot' || rest[0] === 'compare' || rest[0] === 'result') && rest[1] === '--help') return true; + if (rest.length === 2 && ['snapshot', 'compare', 'result', 'attempt'].includes(rest[0]) && rest[1] === '--help') return true; if (rest[0] === 'compare') return rest.length === 2 && privateJson(rest[1]); + if (rest[0] === 'attempt') return rest.length === 4 && path.isAbsolute(rest[1]) && docsPrivateArtifact(rest[1], fixture, scripts) && + rest[2] === '--state' && privateJson(rest[3]); if (rest[0] === 'result') { if (rest.length !== 6 || !path.isAbsolute(rest[1]) || !docsPrivateArtifact(rest[1], fixture, scripts)) return false; const flags = [rest[2], rest[4]]; diff --git a/test/helpers/session-runner.test.ts b/test/helpers/session-runner.test.ts index 2c67252e6..dd24d7bfd 100644 --- a/test/helpers/session-runner.test.ts +++ b/test/helpers/session-runner.test.ts @@ -282,4 +282,38 @@ describe('browseNavigatedTo', () => { expect(browseNavigatedTo(bash(`$B goto ${url}`, `Error: page.goto: net::ERR_FILE_NOT_FOUND at ${url}`), url)).toBe(false); expect(browseNavigatedTo(bash(`$B goto ${url}`, `Navigated to ${url} (404)`), url)).toBe(false); }); + + test('a goto with its stdout sent to /dev/null is credited only when the call proves it exited 0 (PR #4223 outside-voice#1)', () => { + const { url } = trials[0]!; + const cwd = url.replace(/^file:\/\//, '').replace(/\/fixture\.html$/, ''); + const html = '
  • Alpha 12
'; + const credited = (command: string, output = html) => browseNavigatedTo(bash(command, output), url, cwd); + for (const command of ['B=/x/browse; $B goto "file://$PWD/fixture.html" >/dev/null && $B html', + `$B goto ${url} >/dev/null 2>&1 && $B text`, `"$B" goto '${url}' > /dev/null &&\n $B html`, + '/x/browse/dist/browse goto "file://${PWD}/fixture.html" &>/dev/null && /x/browse/dist/browse html']) + expect(credited(command), command).toBe(true); + expect(browseNavigatedTo(bash(`cd ${cwd} && $B goto file://$PWD/fixture.html >/dev/null && $B html`, html), url)).toBe(true); + expect(browseNavigatedTo([...bash(`cd ${cwd}`, ''), ...bash('$B goto "file://$PWD/fixture.html" >/dev/null && $B html', html)], url)) + .toBe(true); + for (const command of ['$B goto "file://$PWD/fixture.html" >/dev/null; $B html', + '$B goto "file://$PWD/fixture.html" >/dev/null || $B html', '$B goto "file://$PWD/fixture.html" >/dev/null & $B html', + '$B goto "file://$PWD/fixture.html" >/dev/null && $B html; echo done', '$B goto "file://$PWD/fixture.html" && $B html', + "$B goto 'file://$PWD/fixture.html' >/dev/null && $B html", '$B goto "file://$PWD/other.html" >/dev/null && $B html', + 'cd /tmp/elsewhere && $B goto "file://$PWD/fixture.html" >/dev/null && $B html', + 'cd "$HOME" && $B goto "file://$PWD/fixture.html" >/dev/null && $B html', + `echo "$B goto ${url}" >/dev/null && $B html`, `$B text goto ${url} >/dev/null && $B html`, + '$B status >/dev/null || $B goto "file://$PWD/fixture.html" >/dev/null && $B html', + `cd ${cwd} && $B status || $B goto "file://$PWD/fixture.html" >/dev/null && $B html`, + 'true || echo | $B goto "file://$PWD/fixture.html" >/dev/null && $B html']) + expect(credited(command), command).toBe(false); + for (const command of [`test -d ${cwd} || exit 1; $B goto "file://$PWD/fixture.html" >/dev/null && $B html`, + '[ -x "$B" ] || B=/x/browse && $B goto "file://$PWD/fixture.html" >/dev/null && $B html']) + expect(credited(command), command).toBe(true); + const http = 'http://127.0.0.1:9/missing'; + expect(browseNavigatedTo(bash(`$B goto ${http} >/dev/null && $B html`, 'Not Found'), http)).toBe(false); + const goto = '$B goto "file://$PWD/fixture.html" >/dev/null && $B html'; + expect(browseNavigatedTo(bash(goto, html), url)).toBe(false); + for (const output of [`Exit code 1\n${html}`, `Error: page.goto: net::ERR_FILE_NOT_FOUND at ${url}`, 'denied']) + expect(credited(goto, output), output).toBe(false); + }); }); diff --git a/test/helpers/session-runner.ts b/test/helpers/session-runner.ts index 0d5f36f91..6a0fad879 100644 --- a/test/helpers/session-runner.ts +++ b/test/helpers/session-runner.ts @@ -92,13 +92,45 @@ export function scanBrowseErrors(text: string): string[] { }); } -/** True when a Bash `goto` printed browse's success line for exactly this URL with a 2xx status. */ -export function browseNavigatedTo(toolCalls: Array<{ tool: string; input: any; output: string }>, url: string): boolean { - return toolCalls.some(call => call.tool === 'Bash' && /\bgoto\b/.test(String(call.input?.command ?? '')) - && String(call.output ?? '').split('\n').some(line => { +/** + * True when a Bash `goto` reached exactly this URL: it printed browse's success line for the URL with a 2xx status, or + * it sent its stdout to /dev/null (`$B goto "" >/dev/null && $B html`) and the call proves it exited 0. That proof + * is a call that did not fail (no leading `Exit code N` or tool error, no `Error` or `net::ERR_` line) where no `||` + * directly before the goto's pipeline could skip it and every command after the goto is joined by `&&`, so none of them ran + * unless it succeeded. Only a file:// URL qualifies: goto exits 0 on any HTTP status, so exit 0 proves no 2xx load of + * an http URL. The goto's target must be the URL + * itself, or `$PWD`/`${PWD}` under a known working directory (`cwd`, followed through literal `cd ` commands in + * call order; any other `cd` makes it unknown). The goto must be the segment's own browse command (`$B goto`, + * `${B} goto` or `/browse goto`), not text inside another command. + */ +export function browseNavigatedTo(toolCalls: Array<{ tool: string; input: any; output: string }>, url: string, cwd?: string): boolean { + let pwd = cwd; + for (const call of toolCalls) { + if (call.tool !== 'Bash') continue; + const command = String(call.input?.command ?? ''); + const output = String(call.output ?? ''); + if (/\bgoto\b/.test(command) && output.split('\n').some(line => { const match = line.trim().match(/^Navigated to (\S+) \((\d+)\)$/); return match?.[1] === url && match[2]!.startsWith('2'); - })); + })) return true; + const failed = /^\s*(?:Exit code [1-9]|)/.test(output) || /^\s*Error\b|net::ERR_/m.test(output); + const parts = command.replace(/\\\n/g, ' ').replace(/(&&|\|\|)\s*\n/g, '$1 ').trim().split(/(&&|\|\||(?&])&(?![>&])|[;|\n])/); + for (let i = 0; i < parts.length; i += 2) { + const segment = parts[i]!.trim(); + const cd = segment.match(/^cd\s+(\S+)$/); + if (cd) pwd = /^\/[^$`'"]*$/.test(cd[1]!) ? cd[1] : undefined; + const goto = segment.match(/^(?:\w+=\S*\s+)*(?:"?\$\{?B\}?"?|\S*\bbrowse)\s+goto\s+("[^"]*"|'[^']*'|[^\s'"]+)(.*)$/); + if (failed || !goto || !url.startsWith('file://') || !/(?:^|\s)(?:1?>|&>)\s*\/dev\/null(?:\s|$)/.test(goto[2]!)) continue; + if (parts.slice(i + 1).some((separator, j) => j % 2 === 0 && separator !== '&&')) continue; + const before = parts.slice(0, i).filter((_, j) => j % 2 === 1); + if (before[before.findLastIndex(separator => separator !== '|')] === '||') continue; + const raw = goto[1]!; + const target = raw.startsWith("'") ? raw.slice(1, -1) + : (raw.startsWith('"') ? raw.slice(1, -1) : raw).replace(/\$\{PWD\}|\$PWD\b/g, () => pwd ?? '\u0000'); + if (target === url) return true; + } + } + return false; } // --- Testable NDJSON parser --- diff --git a/test/helpers/third-party-actions.ts b/test/helpers/third-party-actions.ts index 8220e897c..426c60202 100644 --- a/test/helpers/third-party-actions.ts +++ b/test/helpers/third-party-actions.ts @@ -34,6 +34,13 @@ export function letteredOptions(text: string): string[] { return options; } +/** + * A mention that says Aside is missing ("since Aside isn't available", "Aside was not detected", "with no Aside + * installed") names the browser the option does not use, so it never makes a drive option an Aside drive (gate + * 42748129558: "I drive it in gstack's own visible browser (recommended, since Aside isn't available)"). + */ +const ASIDE_UNAVAILABLE = /\bAside(?:['’]s)?(?:\s+(?:browser|app))?\s+(?:is|was)(?:n['’]t|\s+not)\b|\bAside(?:\s+(?:browser|app))?\s+(?:is|was)\s+(?:unavailable|missing|absent)\b|\b(?:no|without)\s+(?:the\s+|your\s+)?Aside\b/gi; + /** Inspect lettered consent options, not narration that quotes an unavailable option. */ export function asideDriveOptions(text: string): string[] { return letteredOptions(text).filter(option => { @@ -41,7 +48,7 @@ export function asideDriveOptions(text: string): string[] { /\b(?:I|we)(?:['’]ll| will) (?:re[- ]?ask|ask again)(?: (?:you|this question))? with (?:the )?(?:"[^"]+"|“[^”]+”|[\w -]+?) option(?: included)?\b/gi, '', ); - return /\bAside\b/.test(option) && (/\b(?:drive|driving|browse|browsing|navigate|click)\b/i.test(currentOffer) + return /\bAside\b/.test(option.replace(ASIDE_UNAVAILABLE, '')) && (/\b(?:drive|driving|browse|browsing|navigate|click)\b/i.test(currentOffer) || /\bI(?:['’]ll| will)? (?:do|handle|create|complete) (?:it|this)\b/i.test(currentOffer)); }); } @@ -65,13 +72,22 @@ export const offersDriveOption = (text: string): boolean => letteredOptions(text * waiting on you", "I refuse to mark this step as blocked on you", "otherwise I'll mark this step as blocked on you", * "if you skip it, I'll mark this step as blocked on you", "the step is not blocked on you" and "nothing stays blocked * on you" do not count, and neither does "waiting on you to approve it". + * + * Marking the step is what choosing the option does, so that clause counts on its own. The stays/remains/is predicate + * only describes the step's state, which any option can mention, so it counts only after an earlier part of the same + * option puts the step off ("later", "not yet", "not now"): "Later. This step remains waiting on you." counts, but "Sign + * in yourself. Until then the step stays blocked on you.", "Approve it in the dashboard. The step is waiting on you." + * and "Do it now, not later. The step stays blocked on you." do not. */ -const DEFER_OUTCOME = /(?:^|[.!?;:]\s+|\s-+\s+|[\u2013\u2014]\s*)(?:(?!\b(?:not|never|no|nothing|nobody|none|neither|nor|without|cannot|refus\w*|declin\w*|avoid\w*|rather|instead|if|unless|otherwise|else)\b|n['\u2019]t\b)[^.!?;:\u2013\u2014])*?\b(?:(?:I|we)(?:['\u2019]ll| will) |I |we )?(?:(?:mark|leave|keep)\w* (?:this|it|the)(?: [\w-]+)?(?: step)? (?:as )?|(?:stays|remains|is) )(?:blocked|waiting) on you(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/i; +const DEFER_OUTCOME = /(?:^|[.!?;:]\s+|\s-+\s+|[\u2013\u2014]\s*)(?:(?!\b(?:not|never|no|nothing|nobody|none|neither|nor|without|cannot|refus\w*|declin\w*|avoid\w*|rather|instead|if|unless|otherwise|else)\b|n['\u2019]t\b)[^.!?;:\u2013\u2014])*?\b(?:(?:I|we)(?:['\u2019]ll| will) |I |we )?(?:(?:mark|leave|keep)\w* (?:this|it|the)(?: [\w-]+)?(?: step)? (?:as )?|(?(?:stays|remains|is) ))(?:blocked|waiting) on you(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/gi; +const PUT_OFF = /(? [...option.matchAll(DEFER_OUTCOME)] + .some(match => !match.groups?.status || PUT_OFF.test(option.slice(0, match.index + match[0].lastIndexOf(match.groups.status)))); export const offersDefer = (text: string): boolean => letteredOptions(text).some(option => /\b(?:defer\w*|postpone\w*|put (?:this|it)(?: step)? off|skip (?:this|it)(?: step)? for now|come back to (?:this|it)(?: step)? later|leave (?:this|it)(?: step)? for (?:now|later))\b/i.test(option) || !/drive|browse|Aside/i.test(option) && (/(?:^|[.!?;:]\s+)(?:(?:we|I|let['\u2019]s) )?(?:wait|hold off)(?: on (?:this|it)(?: step)?)? for now\b(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/i.test(option) - || !offersManualSteps(option) && DEFER_OUTCOME.test(option))); + || !offersManualSteps(option) && offersDeferOutcome(option))); /** The manual option: "manual", or one sentence offering instructions or steps the user follows themselves. */ export function offersManualSteps(text: string): boolean { diff --git a/test/skill-e2e-skillify.test.ts b/test/skill-e2e-skillify.test.ts index d5847f970..acdf4d647 100644 --- a/test/skill-e2e-skillify.test.ts +++ b/test/skill-e2e-skillify.test.ts @@ -280,7 +280,7 @@ This run is non-interactive; AskUserQuestion is unavailable.`, logCost('scrape-prototype-path', result); const cmds = bashCommands(result); - const wentToFixture = browseNavigatedTo(result.toolCalls, fileUrl); + const wentToFixture = browseNavigatedTo(result.toolCalls, fileUrl, workDir); const fetchedHtml = cmds.some(c => /\bgoto\b|\bhtml\b|\btext\b/.test(c)); const surface = fullSurface(result); const mentionsSkillify = /skillify/i.test(surface); diff --git a/test/third-party-actions.test.ts b/test/third-party-actions.test.ts index c597fe4f1..50e68a4d9 100644 --- a/test/third-party-actions.test.ts +++ b/test/third-party-actions.test.ts @@ -72,6 +72,16 @@ D) Defer expect(asideDriveOptions('A) Open the Aside app; if READY, I drive the dashboard.')).toHaveLength(1); }); + test('a drive option that names Aside only as missing is not an Aside drive (gate 42748129558)', () => { + for (const option of ["B) I drive it in gstack's own visible browser (recommended, since Aside isn't available). I open the dashboard in a visible browser window and stop whenever you need to act.", + "B) Aside isn't available, so I drive it in gstack's own browser.", "B) Aside was not detected; I drive it in gstack's own browser.", + "B) Since the Aside browser is unavailable, I drive it in gstack's own browser.", "B) With no Aside installed, I drive it in gstack's browser."]) + expect(asideDriveOptions(option), option).toEqual([]); + for (const option of ["A) Aside isn't open yet, so I open Aside and drive the dashboard.", + 'A) I drive it in your Aside browser; Aside is not the only option.', "A) Aside wasn't running, but now I drive it in your Aside browser."]) + expect(asideDriveOptions(option), option).toHaveLength(1); + }); + test.each(recoveryFixture.responses)('native recovery response $attempt defers drive consent to a new question', ({ text }) => { expect(asideDriveOptions(text)).toEqual([]); expect(asideDriveOptions(text.replace('option included.', 'option included; then I drive in your Aside browser.'))).toHaveLength(1); @@ -577,8 +587,9 @@ describe('Opus consent renders (gate captures)', () => { test('the defer outcome counts in any words, and a refusal of it never does', () => { // Gate 42431616220 slice 15: the red tpa-present render's D) option, missed by the spelling list. for (const option of ["D) Do it later. I'll mark this step as waiting on you.", 'D) Not yet: the step stays blocked on you.', - 'D) Later. This step remains waiting on you.', "D) Do it later \u2014 I'll leave this step as blocked on you.", - "D) Do it later - I'll leave this step as blocked on you."]) + 'D) Later. This step remains waiting on you.', 'D) Later, so this step remains waiting on you.', "D) Do it later \u2014 I'll leave this step as blocked on you.", + "D) Do it later - I'll leave this step as blocked on you.", 'D) Not now. The step stays blocked on you.', + 'D) Sign in yourself later. The step is waiting on you.']) expect(offersDefer(option), option).toBe(true); for (const text of ['D) Continue now. I will not mark this step as blocked on you.', "D) Continue now. I won't mark this step as waiting on you.", 'D) Continue now. I never mark this step as blocked on you.', @@ -598,6 +609,12 @@ describe('Opus consent renders (gate captures)', () => { "B) I drive it in gstack's own browser and mark this step as waiting on you.", "C) Manual instructions; I'll mark this step as blocked on you.", 'C) I give you the steps to do it yourself. The step stays blocked on you.', + // /ship note at d98f9bb5 (PR #4223): a state line after a non-defer instruction is not a defer offer. + 'D) Sign in yourself. Until then the step stays blocked on you.', + 'D) Approve the token in the dashboard. The step is waiting on you.', + 'D) Continue now. The step is waiting on you.', + 'D) Do it now, not later. The step stays blocked on you.', + 'D) The step is waiting on you. Do it later.', "I'll mark this step as waiting on you.\nA) I drive it in your Aside browser\nB) Manual instructions"]) expect(offersDefer(text), text).toBe(false); });