From bd8561dcbfb03e8b3120d48133d2c9108659231b Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 09:13:28 +0000 Subject: [PATCH] fix: pre-landing review fixes (wave browse): scoped newtab keeps a concurrent tab switch; shadow validate catches a deleted record; a failed shadow publish is journaled Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- browse/src/browser-manager.ts | 2 +- browse/test/session-tabs.test.ts | 28 ++++++++++++++++++++++++++++ lib/autoplan-shadow-reader.ts | 11 ++++++++--- lib/autoplan-shadow.ts | 2 ++ test/autoplan-shadow-driver.test.ts | 5 +++++ test/autoplan-shadow.test.ts | 14 +++++++++++++- 6 files changed, 57 insertions(+), 5 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index dfa27158e..058590a71 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -1231,7 +1231,7 @@ export class BrowserManager { const scope = this.tabScope.getStore(); if (scope) { scope.tabId = id; - this.activeTabId = activeBefore; + if (this.activeTabId === id) this.activeTabId = activeBefore; } else this.activeTabId = id; // Record tab ownership for multi-agent isolation diff --git a/browse/test/session-tabs.test.ts b/browse/test/session-tabs.test.ts index 0273a4d23..94254cded 100644 --- a/browse/test/session-tabs.test.ts +++ b/browse/test/session-tabs.test.ts @@ -250,6 +250,28 @@ const rootTab = bm.getActiveTabId(); rootStatus: (await run('url')).status }; } +{ // an unsessioned tab switch made while a session opens a tab survives the open; closing a pinned tab that is also the global tab moves the global tab + const base = bm.getActiveTabId(); + const other = JSON.parse((await run('newtab', ['--json'])).result).tabId; + bm.switchTab(base, { bringToFront: false }); + const ctx = bm.context; + const newPage = ctx.newPage; + let release, entered; + const gate = new Promise(r => { release = r; }); + const inside = new Promise(r => { entered = r; }); + ctx.newPage = async (...a) => { entered(); await gate; return newPage.apply(ctx, a); }; + const opening = run('newtab', [], { session: 'race' }); + await inside; + bm.switchTab(other, { bringToFront: false }); + release(); + await opening; + ctx.newPage = newPage; + const raceActive = bm.activeTabId; + bm.switchTab(other, { bringToFront: false }); + const closed = await run('closetab', [], { tabId: other }); + steps.race = { raceActive, other, closed: closed.status, activeLive: bm.pages.has(bm.activeTabId) }; +} + await Bun.write(${JSON.stringify(path.join(root, 'steps.json'))}, JSON.stringify(steps)); fixture.stop(true); await Promise.race([bm.close(), Bun.sleep(3000)]); @@ -311,6 +333,12 @@ describe('four sessions at once (#1961)', () => { }); describe('session lifecycle', () => { + test('an unsessioned tab switch during a session newtab is kept', () => { + expect(steps.race.raceActive).toBe(steps.race.other); + }); + test('closing a pinned tab that is the global active tab moves the global tab to a live one', () => { + expect(steps.race).toMatchObject({ closed: 200, activeLive: true }); + }); test('a read on a closed session tab fails with the last URL; goto reopens it', () => { expect(steps.reset.textStatus).not.toBe(200); expect(steps.reset.text).toContain('BROWSE_SESSION_TAB_RESET'); diff --git a/lib/autoplan-shadow-reader.ts b/lib/autoplan-shadow-reader.ts index 32f1934eb..a6a072766 100644 --- a/lib/autoplan-shadow-reader.ts +++ b/lib/autoplan-shadow-reader.ts @@ -82,15 +82,20 @@ export function renderShadowExport(r: ShadowExport): string[] { /** The private check behind gstack-artifact validate: each shadow record matches its journaled sha256 and its rows the schema. */ export function validateShadow(runDir: string): ValidationError[] { const dir = shadowDir(runDir); + const journal = readJournal(runDir); + const missing = (present: Set): ValidationError[] => [...new Set(journal.filter(r => r.event === 'shadow_recorded').map(r => Number(r.gate_rev)))] + .filter(rev => !present.has(rev)) + .map(rev => ({ code: 'ARTIFACT_STALE', path: `shadow/${path.basename(recordFile(runDir, rev))}`, message: `the journal records gate_rev ${rev}, but its shadow record is missing` })); let st: fs.Stats; - try { st = fs.lstatSync(dir); } catch { return []; } + try { st = fs.lstatSync(dir); } catch { return missing(new Set()); } if (!st.isDirectory()) return [{ code: 'ARTIFACT_PATH_ESCAPE', path: 'shadow', message: 'the shadow folder is a symlink or not a directory' }]; const errors: ValidationError[] = []; - const journal = readJournal(runDir); + const present = new Set(); for (const name of fs.readdirSync(dir)) { const m = RECORD_RE.exec(name); if (!m) continue; const rev = Number(m[1]); + present.add(rev); const rel = `shadow/${name}`; const bytes = fs.readFileSync(path.join(dir, name)); const recorded = journal.filter(r => r.event === 'shadow_recorded' && r.gate_rev === rev).at(-1); @@ -104,7 +109,7 @@ export function validateShadow(runDir: string): ValidationError[] { if (problem) errors.push({ code: 'ARTIFACT_SCHEMA', path: `${rel}:${i + 1}`, message: problem }); }); } - return errors; + return [...errors, ...missing(present)]; } function rowProblem(r: Record, rev: number): string | undefined { diff --git a/lib/autoplan-shadow.ts b/lib/autoplan-shadow.ts index 0cf988551..dbc1752e1 100644 --- a/lib/autoplan-shadow.ts +++ b/lib/autoplan-shadow.ts @@ -417,6 +417,8 @@ function publish(out: string, claim: ShadowClaim, r: { rows?: Array { diff --git a/test/autoplan-shadow.test.ts b/test/autoplan-shadow.test.ts index 5784fa857..d7ce8275a 100644 --- a/test/autoplan-shadow.test.ts +++ b/test/autoplan-shadow.test.ts @@ -14,7 +14,7 @@ import * as path from 'node:path'; import { SHADOW_PROMPT_SHA256, SHADOW_PROMPT_VERSION, auditMark, buildPage, claimShadow, parsePage, recordFile, runShadow, shadowPromptFile, shadowSwitch } from '../lib/autoplan-shadow'; import { callModel } from '../lib/outside-voice-api'; import { gateCost, renderGateCost } from '../lib/autoplan-gate-cost'; -import { readState } from '../lib/autoplan-state'; +import { readJournal, readState } from '../lib/autoplan-state'; import { reserve, settle, readLedger } from '../lib/spend-ledger'; import type { GateList } from '../lib/gate-list'; import { ROOT, readRun, runPhase, startSmallRun } from './helpers/autoplan-run-fixture'; @@ -280,6 +280,18 @@ describe('runShadow: retry, truncation, failures and the deadline', async () => expect(slow.ms).toBeLessThan(2500); expect(readLedger(path.join(slow.out, 'spend.json')).settled.filter(s => s.label === 'shadow').map(s => s.usd)).toEqual(['unknown']); }); + test('a publish that throws journals shadow_failed and drops the lease, so a later claim pays no second call', async () => { + const f = await fake('publish-throws', [{ mode: 'valid' }]); + const out = gateRun(); + const { claim } = claimShadow(out); + fs.mkdirSync(path.join(out, 'shadow', 'ceo-only.r1.jsonl.stuck.tmp'), { recursive: true }); + const outcome = await runShadow(out, claim!, { env: { ...process.env, ...f.env } }); + expect(outcome).toMatchObject({ status: 'failed', code: 'internal' }); + expect(readJournal(out).filter(r => r.event === 'shadow_failed' && r.gate_rev === 1).map(r => r.code)).toEqual(['internal']); + expect(fs.existsSync(path.join(out, 'shadow', 'r1.pending'))).toBe(false); + expect(claimShadow(out)).toEqual({}); + expect(f.requests().length).toBe(1); + }); test('a record already published makes a later claim a no-op: no second call is paid', async () => { const { f, out } = await shadowed('noop', [{ mode: 'valid' }]); const before = fs.readFileSync(recordFile(out, 1));