Fix all open Codex recovery-capture findings on PR #4

.github/workflows/ci.yml

@@ -52,3 +52,6 @@ jobs:
       - name: Recovery capture isolation and late frames
         if: always() && !cancelled()
         run: timeout 120s node test/recovery-capture.smoke.mjs
+      - name: Environment retention across dispatch and handoff
+        if: always() && !cancelled()
+        run: timeout 90s node test/environment-retention.smoke.mjs

package.json

@@ -26,7 +26,7 @@
     "test:python": "python3 -m unittest discover -s worker/tests -v",
     "check": "npm run typecheck && npm test && npm run test:python",
     "prepack": "npm run build",
-    "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs"
+    "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs && node test/environment-retention.smoke.mjs"
   },
   "dependencies": {
     "@effect/platform-node": "4.0.0-rc.116",

src/page-worker.ts

@@ -57,6 +57,7 @@ export async function checkOne(page: Page, c: Check): Promise<Data> {
   else {
     const count = await locator.count();
     if (c.kind === "count") actual = String(count);
+    else if (c.kind === "visible" && count === 0) actual = "false";
     else if (count !== 1) { actual = `<${count} matching elements>`; available = false; }
     else {
       const value = await locator.evaluate(readElement, c);
@@ -93,12 +94,28 @@ async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; loca
   }
   return { frame, locator: frame.locator(css(s.selector ?? "body", s.shadow)) };
 }
+function suspendRecoveryCapture(journal: Journal): void {
+  if (!journal.events().some(e => e.type === "recovery.capture_suspended")) {
+    journal.append("recovery.capture_suspended", {
+      reason: "Environment-backed input may be moved or transformed by the page. Automatic recovery-value capture is disabled for this run; prior snapshots and explicit non-secret input intents remain available.",
+    });
+  }
+}
 async function captureFields(page: Page, journal: Journal, save = true): Promise<void> {
   const task = journal.meta().task;
   const fields = task.recovery?.fields ?? [];
   if (fields.length === 0) return;
-  const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])]
-    .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined);
+  const steps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])];
+  const events = journal.events();
+  const suspended = events.some(e => e.type === "recovery.capture_suspended");
+  // Old receipts cannot prove which environment-input boundary was crossed.
+  const legacy = steps.some(s => s.valueFromEnv !== undefined) && events.some(e =>
+    e.type === "action.start" && e.data.engine === "playwright" && e.data.recoveryGuarded !== true);
+  if (suspended || legacy) {
+    if (!suspended) suspendRecoveryCapture(journal);
+    save = false; // Still reject known protected aliases, but never read new page values.
+  }
+  const protectedSteps = steps.filter(s => s.valueFromEnv !== undefined && s.selector !== undefined);
   // These values never enter browser evaluation arguments. A page may redefine
   // its prototypes; equality against environment secrets belongs in this process.
   const protectedValues = new Set(protectedSteps
@@ -223,8 +240,12 @@ async function runSteps(page: Page, journal: Journal, job: PageJob, snap: () =>
     let actionId: string | undefined;
     if (s.kind !== "checkpoint") {
       if (remaining(journal.state()).actions <= 0) throw new BrowserError({ code: "budget_exhausted", reason: "Action budget exhausted." });
+      // Selectors, node identities and exact-value matching cannot follow an
+      // arbitrary formatter/rerender. Persist this boundary before dispatch so
+      // resume, reconstruction and a Jev/Midscene handoff cannot forget it.
+      if (s.valueFromEnv) suspendRecoveryCapture(journal);
       actionId = randomUUID();
-      journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright" });
+      journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright", recoveryGuarded: true });
     }
     switch (s.kind) {
       case "navigate": {

test/environment-retention.smoke.mjs

@@ -0,0 +1,117 @@
+import assert from 'node:assert/strict';
+import * as fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { execFileSync } from 'node:child_process';
+import { fixture } from './support/chrome.mjs';
+import { Cdp } from '../dist/cdp.js';
+import { createRun, atomic } from '../dist/journal.js';
+import { attachPage, pageJob } from '../dist/page-worker.js';
+
+const key = 'FE2E_RERENDER_SECRET', secret = 'synthetic-private-123';
+const formatted = [...secret].join(' '), previous = process.env[key];
+process.env[key] = secret;
+const f = await fixture(), raw = await Cdp.open(f.session), passed = [];
+const worker = fileURLToPath(new URL('../worker', import.meta.url));
+function assertPrivate(journal) {
+  for (const entry of fs.readdirSync(journal.dir, { recursive: true })) {
+    const file = path.join(journal.dir, entry);
+    if (fs.statSync(file).isFile()) {
+      const text = fs.readFileSync(file, 'utf8');
+      assert.equal(text.includes(secret), false, entry);
+      assert.equal(text.includes(formatted), false, entry);
+    }
+  }
+}
+function checkJevHandoff(journal) {
+  // A fresh Python adapter must honor the durable Node suspension even without
+  // the original environment. Any capture evaluation would fail this test.
+  const env = { ...process.env }; delete env[key];
+  execFileSync('python3', ['-c', `import sys
+sys.path.insert(0, sys.argv[1])
+from jev_runner import Journal, capture
+class Page:
+    def evaluate(self, expression):
+        raise AssertionError('Suspended Jev recovery must not evaluate the page')
+capture(Page(), Journal(sys.argv[2], sys.argv[3]))
+`, worker, f.directory, journal.runId], { env, timeout: 10_000 });
+}
+async function scenario(name, variant, reconstruct, check) {
+  const { targetId } = await raw.send('Target.createTarget', { url: 'about:blank' });
+  const connection = await attachPage(f.session, targetId);
+  try {
+    const page = connection.page;
+    await page.goto(f.url, { waitUntil: 'load' });
+    await page.setContent(`<input id="public" class="active-input" value="safe-before"><input id="token"><input id="safe" value="safe-before">
+      <script>document.querySelector('#token').oninput=event=>{
+        const node=event.target,value=[...node.value].join(' ');
+        if(${JSON.stringify(variant)}==='mirror') document.querySelector('#public').value=value;
+        else {
+          document.querySelector('#public').classList.remove('active-input');
+          const next=${JSON.stringify(variant)}==='replace'?node.cloneNode(true):node;
+          next.id='renamed';next.classList.add('active-input');next.value=value;
+          if(next!==node)node.replaceWith(next);
+        }
+      };</script>`);
+    const step = { kind: 'fill', selector: '#token', valueFromEnv: key };
+    const task = { url: f.url, goal: name, steps: [step], recovery: {
+      fields: [{ key: 'public', selector: '.active-input' }, { key: 'safe', selector: '#safe' }],
+      ...(reconstruct ? { reconstruct: [{ kind: 'navigate', value: f.url, safeToRepeat: true }, { ...step, safeToRepeat: true }] } : {}),
+    } };
+    const { journal } = createRun(f.directory, task, false, f.profile);
+    journal.append('target', { targetId, browserId: f.session.browserId });
+    atomic(path.join(f.directory, 'targets', f.session.namespace, `${targetId}.json`), { targetId, browserId: f.session.browserId });
+    const invoke = (operation = 'capture', extra = {}) => pageJob({
+      root: f.directory, runId: journal.runId, session: f.session, targetId,
+      operation, steps: [step], timeoutMs: 20_000, ...extra,
+    }, new AbortController().signal);
+    await invoke();
+    await check({ page, journal, step, invoke });
+    assertPrivate(journal); checkJevHandoff(journal);
+    passed.push(name);
+  } finally {
+    await connection.browser.close(); await raw.send('Target.closeTarget', { targetId });
+  }
+}
+try {
+  for (const variant of ['retag', 'replace', 'mirror']) for (const reconstruct of [false, true]) {
+    await scenario(`Formatted ${variant} value is not retained; reconstruct=${reconstruct}`, variant, reconstruct,
+      async ({ page, journal, invoke }) => {
+        await invoke('steps', reconstruct ? { saveProgress: false } : {});
+        assert.equal(await page.locator('.active-input').inputValue(), formatted);
+        const retained = journal.recoveryFields();
+        assert.equal(retained.public.value, 'safe-before');
+        assert.equal(retained.public.source, 'observed');
+        const events = journal.events();
+        const boundary = events.find(e => e.type === 'recovery.capture_suspended');
+        assert.ok(boundary);
+        assert.ok(boundary.seq < events.find(e => e.type === 'action.start').seq);
+        // Reattachment, a later capture and loss of the env value cannot undo suspension.
+        delete process.env[key];
+        try { await invoke(); await invoke(); } finally { process.env[key] = secret; }
+        assert.deepEqual(journal.recoveryFields(), retained);
+        assert.equal(journal.events().filter(e => e.type === 'recovery.capture_suspended').length, 1);
+      });
+  }
+  await scenario('Explicit public inputs remain recoverable after secret dispatch', 'mirror', false,
+    async ({ journal, step, invoke }) => {
+      await invoke('steps', { steps: [step, { kind: 'fill', selector: '#safe', value: 'user-provided-public' }] });
+      assert.equal(journal.recoveryFields().safe.value, 'user-provided-public');
+      assert.equal(journal.recoveryFields().safe.source, 'intent');
+      assert.equal(journal.recoveryFields().public.value, 'safe-before');
+    });
+  await scenario('Legacy dispatch without provenance suspends capture on resume', 'mirror', false,
+    async ({ page, journal, invoke }) => {
+      journal.append('action.start', { id: 'legacy', kind: 'fill', selector: '#token', step: 0, engine: 'playwright' });
+      await page.locator('#token').fill(secret);
+      journal.append('action.end', { id: 'legacy' });
+      delete process.env[key];
+      try { await invoke(); } finally { process.env[key] = secret; }
+      assert.equal(journal.recoveryFields().public.value, 'safe-before');
+      assert.ok(journal.events().some(e => e.type === 'recovery.capture_suspended'));
+    });
+} finally {
+  await raw.close(); await f.cleanup();
+  if (previous === undefined) delete process.env[key]; else process.env[key] = previous;
+}
+console.log(JSON.stringify({ suite: 'environment retention across dispatch and handoff', passed, paidModelCalls: 0 }));

test/recovery-capture.smoke.mjs

@@ -169,6 +169,32 @@ try {
   }
   await scenario('Required assertion scopes remain strict', { steps: [] }, '<h1>No iframe</h1>', async ({ page }) => {
     await assert.rejects(() => checkOne(page, { kind: 'count', frames: ['#missing'], selector: 'input', value: '0' }), error => error.code === 'unsupported_scope');
+    assert.equal((await checkOne(page, { kind: 'visible', selector: '#missing', value: 'false' })).passed, true);
+  });
+
+  const publicFields = Array.from({ length: 8 }, (_, i) => ({ key: `public${i}`, selector: `#public${i}` }));
+  await scenario('Protected frame traversal is shared within, not across, snapshots', {
+    steps: Array.from({ length: 20 }, (_, i) => ({ kind: 'fill', valueFromEnv: key,
+      selector: i % 2 ? '#nestedName' : '#frameName', frames: i % 2 ? ['#remote', '#nested'] : ['#remote'] })),
+    recovery: { fields: publicFields },
+  }, remote => publicFields.map((_, i) => `<input id="public${i}" value="public-${i}">`).join('') + `<iframe id="remote" src="${remote}"></iframe>`, async ({ page, journal, invoke }) => {
+    // Count real frame-path lookups, not timing: added fields/duplicate steps
+    // must not multiply browser round trips. The second snapshot must be fresh.
+    const prototype = Object.getPrototypeOf(page.mainFrame()), original = prototype.locator;
+    let queries = 0;
+    prototype.locator = function (selector, ...options) {
+      if (selector === 'css:light=#remote' || selector === 'css:light=#nested') queries++;
+      return Reflect.apply(original, this, [selector, ...options]);
+    };
+    try {
+      for (let attempt = 0; attempt < 2; attempt++) {
+        queries = 0;
+        await invoke();
+        assert.equal(queries, 3, 'Resolve the two distinct paths once each per snapshot');
+        assert.equal(Object.keys(journal.recoveryFields()).length, 8);
+      }
+    } finally { prototype.locator = original; }
+    assertNotRetained(journal);
   });
 } finally {
   await raw.close(); await f.cleanup();

worker/jev_runner.py

@@ -72,9 +72,23 @@ def ids(t):
 def capture(browser, journal, action=None, text=None):
     task = journal.meta["task"]
     fields = task.get("recovery", {}).get("fields", [])
+    if not fields:
+        return
+    steps = [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])]
+    events = journal.events()
+    # A scoped secret fill can hand off to Jev after the page has formatted or
+    # moved its value. Never infer that it became public from current selectors.
+    if any(e["type"] == "recovery.capture_suspended" for e in events):
+        return
+    if any(s.get("valueFromEnv") is not None for s in steps) and any(
+        e["type"] == "action.start" and e["data"].get("engine") == "playwright"
+        and e["data"].get("recoveryGuarded") is not True for e in events
+    ):
+        journal.append("recovery.capture_suspended", reason="Legacy dispatch cannot establish the environment-input boundary. Keep existing recovery inputs; do not collect new page values.")
+        return
     # Capture also runs after a scoped/environment-backed step hands off to Jev.
     # Protect aliases in the top-level document without reading secret values.
-    protected = [s["selector"] for s in [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])]
+    protected = [s["selector"] for s in steps
                  if s.get("valueFromEnv") is not None and s.get("selector") and not s.get("frames")]
     for field in fields:
         if field.get("frames") or field.get("shadow") == "open":

worker/tests/test_jev_retention.py

@@ -0,0 +1,66 @@
+"""A fresh managed-Jev process must honor the shared capture boundary."""
+import json
+from pathlib import Path
+import sys
+import tempfile
+import time
+import unittest
+from unittest.mock import Mock
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+from jev_runner import Journal, capture
+
+
+class RecoveryBoundaryTests(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory()
+        self.addCleanup(self.temp.cleanup)
+        self.run_id = 'r_' + 'a' * 32
+        self.directory = Path(self.temp.name) / 'runs' / self.run_id
+        self.directory.mkdir(parents=True)
+        self.task = {
+            'steps': [{'kind': 'fill', 'selector': '#token', 'valueFromEnv': 'SYNTHETIC_INPUT'}],
+            'recovery': {'fields': [{'key': 'public', 'selector': '#public'}]},
+        }
+        self.write_meta()
+        (self.directory / 'events.jsonl').write_text('')
+        self.journal = Journal(self.temp.name, self.run_id)
+        self.page = Mock()
+        self.page.evaluate.return_value = {'value': 'public', 'matches': False}
+
+    def write_meta(self):
+        (self.directory / 'task.json').write_text(json.dumps({
+            'version': 2, 'expiresAt': time.time() * 1000 + 60_000, 'task': self.task,
+        }))
+
+    def test_suspension_survives_new_journal_and_skips_model_generated_intents(self):
+        self.journal.append('recovery.capture_suspended', reason='environment boundary')
+        fresh = Journal(self.temp.name, self.run_id)
+        capture(self.page, fresh)
+        capture(self.page, fresh, {'kind': 'fill', 'node': 1}, 'model-generated')
+        self.page.evaluate.assert_not_called()
+        self.assertFalse((self.directory / 'recovery.json').exists())
+
+    def test_legacy_playwright_receipt_is_not_assumed_public(self):
+        self.journal.append('action.start', engine='playwright', id='old')
+        capture(self.page, self.journal)
+        capture(self.page, Journal(self.temp.name, self.run_id))
+        self.page.evaluate.assert_not_called()
+        self.assertEqual(sum(e['type'] == 'recovery.capture_suspended' for e in self.journal.events()), 1)
+
+    def test_guarded_public_dispatch_does_not_disable_capture(self):
+        self.journal.append('action.start', engine='playwright', id='public', recoveryGuarded=True)
+        capture(self.page, self.journal)
+        self.page.evaluate.assert_called_once()
+        self.assertEqual(json.loads((self.directory / 'recovery.json').read_text())['public']['value'], 'public')
+
+    def test_legacy_run_without_environment_steps_still_captures(self):
+        self.task['steps'] = [{'kind': 'click', 'selector': '#open'}]
+        self.write_meta()
+        self.journal.append('action.start', engine='playwright', id='old-public')
+        capture(self.page, Journal(self.temp.name, self.run_id))
+        self.page.evaluate.assert_called_once()
+
+
+if __name__ == '__main__':
+    unittest.main()