Skip to content

fix(evals): saved-app creation opens its verification return tab as a person's tab, not agent browser control - #4779

Merged
yomgui merged 9 commits into
devfrom
e2e/saved-app-browser-requesting-conversation
Sep 11, 2026
Merged

yomgui merged 9 commits into
devfrom
e2e/saved-app-browser-requesting-conversation

Conversation

@yomgui

@yomgui yomgui commented Sep 10, 2026

Copy link
Copy Markdown
Member

Root cause

Triage: /Users/guillaume/OpenWork Chat/reports/e2e-red-2026-09-09/triage.md — saved-app-creation, failure at worlds/saved-apps.ts:163 (returnVerification) with
Error invoking remote method 'openwork:browser:openUrl': Browser control requires a requesting conversation. (dev nightly @ 52a0288, PR #4757 run @ 564e17c).

The spec's world channel, not the product, lacked the origin. returnVerification created its vehicle tab through the agent entry point — window.__OPENWORK_ELECTRON__.browser.openUrl("about:blank", "builtin"), the same IPC behind the browser.open_url affordance — without a requesting conversation. Since #4434 (1d2fc5b) the browser task host correctly refuses that (browser-task.mjs missing_session), and it never accepted an about:blank destination either (browserTaskUrl only allows http(s)); stamping an origin would just have moved the failure to invalid_url plus a consent prompt.

The tab is only a stand-in for the person's own browser: containers have no OS protocol registration, so the world navigates the real openwork://den-reauth return link in an Electron tab and the main process intercepts it on did-start-navigation for any tab, owner or not. The honest channel for that is the person's new-tab path (browser.createTab, a null-owner "shared" tab) — already used by worlds/browser-panel.ts as "explicit human-created initial state, not an automation navigation". No product change; the requesting-conversation requirement is not bypassed or weakened.

What changed

  • evals/worlds/saved-apps.ts returnVerification: create the return tab with browser.createTab("about:blank") and find its CDP target by diffing page targets (that IPC returns only tabId). Navigation, deep-link interception and tab close are unchanged.
  • Merged origin/e2e/fresh-session-send-gate (564e17c) so the spec gets past the send step (Cluster 1); those commits are not part of this fix.

Proof

pnpm evals:e2e saved-app-creation --daytona with OPENWORK_EVAL_REF=8ced4002cc82eb18fb24c66e844b4f67f988538b (source verified 8ced4002c in the sandbox log). Placement line: selection: engine=legacy surface=legacy case=all; placement: daytona (--daytona).

Three runs (each provisioned its own Den + two desktop sandboxes and deleted all three):

run steps 1–11 (incl. both returnVerification calls in "share dashboard apps with a teammate") step 12 "return from browser verification…" (3rd returnVerification) outcome
04:37Z ok failed on the Den web page: Verify password produced Better-Auth "Provider not found" (a social sign-in fired) 370 ms after a client-side route change 0 passed / 1 failed / 0 skipped
04:49Z ok ok 0 passed / 1 failed / 0 skipped — failed at step 13 (below)
04:59Z ok ok 0 passed / 1 failed / 0 skipped — failed at step 13 (below)

The cluster claim is proven: every returnVerification call now completes and the subsequent assertions (unrelated callback rejected, verified link shared 1 app, late callback left #/dashboard, browser-return grant gave the recipient 200) pass. The whole spec is not green yet: verdict Failed at later steps that have different root causes, so this PR does not claim saved-app-creation Passed.

Noticed, not touched

  1. Step 13 "an admin cancels deletion in the app and confirms it on the dashboard" fails deterministically (2/2) at saved-app-creation.e2e.test.ts:523: after world.open("/dashboard") (go() only sets the hash and returns) the click("App options for Team briefing") resolves in ~110 ms — most likely the still-mounted app-page header button — and then Delete Team briefing is never found; after 30 s the only visible buttons are the shell rail (New session, Dashboard, Library, …), i.e. the main content renders nothing. Looks like a product issue on route change with an open header menu, not just spec ordering; needs a screenshot/error-boundary look.
  2. Round-2 web verification flake (1/3): on /reauth/desktop with an existing cookie session, type Password + click("Verify password") within ~0.4 s of the client-side nonce change ended in "Provider not found" — a social button received the click. Testkit text click vs. dialog layout race, or the dialog should not shift while opening.
  3. pnpm --dir evals typecheck is red on dev independent of this change (unbuilt @openwork-ee/den-db/@openwork/email outputs, Promise.withResolvers lib target); worlds/saved-apps.ts itself has no errors and check:browser passes.

…immediately

A freshly created session had no cached snapshot, so deriveSessionRenderModel
reported "switching" while the first snapshot round trip ran and the composer
disabled Run task. On Daytona that window is ~0.9 s and a click or Enter inside
it was silently dropped with the draft intact. Seed the snapshot cache from the
create response (stale, so the real fetch still runs as a background refresh).
…ad of wedging

A prompt_async answered with 5xx/408 is an uncertain admission (#4635), but
the hold it created could never end: the acceptance check treated absence as
no evidence, Stop refused while the admission was unknown, and the failure body
was dropped, so the person saw a paused composer with no explanation.

- Keep the settled response body on PromptAdmissionUnknownError and show it as
  the session error card right away; the hold still prevents any resend.
- readPromptAdmission: after the exact-message GET misses, list the idle
  conversation; when native lists it without the message the POST is settled
  and did not admit, so the drain receives admission_rejected and halts like
  any definite failure (explicit retry only, never an automatic resend).
- On release the pending row is removed and the submitted composer is kept as
  an unsent message to restore; on confirmed acceptance the stale failure card
  is cleared.
Lexical keeps one empty paragraph in an empty editor, which innerText reports
as "\n". readComposerState now drops that trailing line break so draftText
matches what the person sees; user.see("composer", { text }) already trimmed.
…ssion button

The sidebar button was renamed from "New task" to "New session" in #4491
(Sep 5) and the "new" scenario added in #4631 (Sep 8) still targeted the old
label, so it could never find the button. Same user action, current label.
…o e2e/saved-app-browser-requesting-conversation
… new tab

returnVerification drove the agent browser entry point (browser.openUrl, the
IPC behind browser.open_url) with no requesting conversation. Since #4434 the
task host refuses that, and it never accepted an about:blank destination.
The tab only stands in for the person's own browser so the openwork:// return
link can be navigated without OS protocol registration, so create it the way a
person opens a new tab and find its CDP target by diffing the page targets.
@vercel

vercel Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
openwork-app Ready Ready Preview Sep 10, 2026 9:38pm UTC
openwork-den Ready Ready Preview Sep 10, 2026 9:38pm UTC
openwork-diagnostics Ready Ready Preview Sep 10, 2026 9:38pm UTC
openwork-landing Ready Ready Preview, v0 Sep 10, 2026 9:38pm UTC

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Warden review summary — Clear

Native findings: 0. Blocking policy matches before consolidation: 0. Full finding text remains in Warden's native checks and review threads. This comment retains only native IDs, severity, skill attribution, and safe locations. Analysis run

Reported trigger coverage (not proof of complete repository or context coverage): diff-security-review (82461 ms), confidentiality-review (206220 ms), spec-provenance-review (92582 ms), desktop-den-sync-review (131630 ms).

Blockers

  • None.

Advisories

  • None.

Needs recheck

  • None.

Observed review-thread metrics

  • Unique observed run attempts in retained window: 2
  • Distinct heads in retained window: 2
  • Current unresolved Warden threads: 0 of 0 observed
  • Observed resolved/unresolved transitions in retained snapshots: 0 / 0
  • First observed resolved/unresolved (state at first collection, not event time): 0 / 0
  • Precision: unavailable (no adjudications; no TP/FP values are inferred)
  • Observation window: 2/20 run attempts retained; truncated: false
  • Transition snapshot: 0/0 current attributed threads retained (limit 500); truncated: false

Thread states are read-only collection snapshots. These metrics are neither lifetime totals nor exact resolution times, and this summary is not clearance authority.

diff-warden[bot]
diff-warden Bot previously approved these changes Sep 10, 2026

@diff-warden diff-warden Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warden security clearance: clear. No blocking findings (security or desktop↔den sync) in this diff (8ced4002cc82eb18fb24c66e844b4f67f988538b). Automated clearance satisfies the required-review gate only — a human still reviews and merges. Analysis run

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Passed · 12/12 tests · 18/18 assertions · 8 images

Commit 1afdc2757bae3c517d03951dcdaa58860b2199da · selected evidence

Open review report

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Deploying openwork with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1afdc27
Status: ✅  Deploy successful!
Preview URL: https://a80aba17.openwork.pages.dev
Branch Preview URL: https://e2e-saved-app-browser-reques.openwork.pages.dev

View logs

@diff-warden diff-warden Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warden security clearance: clear. No blocking findings (security or desktop↔den sync) in this diff (1afdc2757bae3c517d03951dcdaa58860b2199da). Automated clearance satisfies the required-review gate only — a human still reviews and merges. Analysis run

@yomgui

yomgui commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Resync onto dev after the #4757 conflict resolution

Proof at the pushed head

OPENWORK_EVAL_REF=1afdc2757bae3c517d03951dcdaa58860b2199da pnpm evals:e2e saved-app-creation --daytona (selection: engine=legacy surface=legacy case=all; placement: daytona (--daytona); source verified 1afdc2757… in all three sandboxes).

test result
create, preview, save and reopen an app without changing already-open results (this PR's spec) ✓ passed (331.9 s) — all steps ok, including every returnVerification call
APP-ISOLATION embedded MCP Apps isolate siblings… (added to this file by #4829 today) × failed in world provisioning after 53 s, before any assertion: Mock issuer gate failed: health reported https://3979-…, expected https://3979-…

"passed":1,"failed":1,"skipped":0 → file verdict Failed, but the red test is not this PR's: isolatedMcpApps seeds two seed.mock(...) without explicit ports into one appWeb sandbox, so both get daytonaPort: 3979 (evals/packages/env/src/mock.ts:56); bootRemoteMocks starts them sequentially on the same sandbox and the second mock's pkill -f mock-oauth-mcp-server + rebind on the same port makes the health gate see the first mock's issuer. Reproduced identically 2/2 at this sha (first run also hit a hung mock process cleanup exec on the Den sandbox → 600 s timeout; re-run per policy). #4829 proved that test with --local --engine v1 --case APP-ISOLATION only, never --daytona; the co-located Daytona path is not exercised locally. Attribute to #4829 — give second a distinct port (e.g. 3980). This PR does not touch that world/spec code.

Sandboxes: all created by run 2 deleted by the runner; run 1's leaked openwork-server-20260910-213758-… deleted manually.

@yomgui
yomgui merged commit 23a29a5 into dev Sep 11, 2026
29 checks passed

This branch was successfully deployed

4 active deployments
Preview – openwork-landing — 1afdc275 Deployed Sep 10, 2026 by vercel[bot]
Preview – openwork-app — 1afdc275 Deployed Sep 10, 2026 by vercel[bot]
Preview – openwork-den — 1afdc275 Deployed Sep 10, 2026 by vercel[bot]
Preview – openwork-diagnostics — 1afdc275 Deployed Sep 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant