Skip to content

test(evals): harness hygiene — complete locate() miss diagnostics and route-commit wait in world open() - #4810

Merged
yomgui merged 48 commits into
devfrom
e2e/harness-hygiene-locate-workspaces-open
Sep 11, 2026
Merged

yomgui merged 48 commits into
devfrom
e2e/harness-hygiene-locate-workspaces-open

Conversation

@yomgui

@yomgui yomgui commented Sep 10, 2026

Copy link
Copy Markdown
Member

Stacked on origin/e2e/permission-ask-and-orphans-after-abort (#4798, which carries #4786, #4747, #4757, #4723) and origin/e2e/dashboard-delete-no-buttons (#4794, which carries #4779) via merge commits ae3533bdc and 20a3aa597; none of their files are modified here. Net change over the merges: two commits, both in evals/packages/cdp (af2b46ae3, 7a31c4917). Depends on #4798 and #4794 landing first. Evals-only; no product code.

Context: 2026-09-09 E2E triage wave-4 follow-ups. The brief originally carried a third item (make the four remaining splitPaneQuestions worlds create their own workspace); that item was reassigned mid-session to another session that will open its own PR, so this PR does not touch evals/worlds/chat.ts and nothing about it was committed or needed reverting.

1. locate() miss diagnostics (af2b46ae3, 7a31c4917)

Root cause. cdp/src/input.ts#locate reported a miss with the first 8 rendered buttons/links in DOM order. The shell rail always comes first, so every miss read as "the page has no buttons", which sent #4779's diagnosis after a dashboard bug that did not exist (#4794 "Noticed, not touched" item 1).

What changed. No change to matching. On a miss the page-side callback now returns:

  • every rendered element of the requested role (for role-less targets, every rendered button/link as before) — no DOM-order cap in the browser;
  • the current location.hash (or pathname);
  • whether [data-app-header] and [data-dashboard-page] are mounted.

The Node side renders that as, e.g. Could not locate {...}. Route #/dashboard. Page roots: appHeader=true dashboardPage=false. Visible menuitem candidates (2): menuitem "Remove Team briefing from dashboard", menuitem "Delete Team briefing". The list is capped at 40 (MISS_CANDIDATE_LIMIT) and says (showing first 40 of N) when truncated; an empty list says No visible <role> candidates. Older/partial reports (mocked surfaces) still produce a usable message.

Unit proof. node --test packages/cdp/test/input.test.ts → 11/11. New tests: message rendering for role/route/roots; the 40-cap with truncation note and the empty-list wording; and the serialized page callback itself run in a vm context against a fake DOM (9 shell-rail buttons + 2 dashboard menu items): a menuitem miss lists both menu items and appHeader=false dashboardPage=true, a bare miss lists all 9 buttons (the old 8-cap is gone). pnpm --dir evals run check:browser → 832 callbacks clean.

2. Route-commit wait in other worlds' open()/go() — audit, no change

#4794 made the saved-apps world's open() wait for the destination's own root after go() because go() only sets location.hash; the page being left stays mounted until the router commits, and the dashboard and app page share control labels. I audited every hash-only navigation in evals/worlds/**, packages/behaviors, and packages/env, looking for the same shape: hash set → immediate click/read that the predecessor page can satisfy with the same label/text. None of the other worlds has that race, so per the brief ("only touch worlds where you can name the race") nothing was changed. Inventory:

Site Next action Why it is not the race
worlds/first-run.ts:1291-1292, :1346-1347 (backgroundUpdateWorld, revokedUpdateWorld openSettings/openWorkspace) — used by background-desktop-update openSettings() → click("Check now") / click(switch "Check automatically"); openWorkspace() → see/notSee("Restart to update") "Check now" and the switch exist only on Settings › Updates, so click blocks until the destination renders. "Restart to update" is the shell titlebar button, identical on both pages, and at every notSee after openWorkspace() the download has not finished, so the text is absent app-wide; the outcome cannot differ by which page is mounted. Structurally notSee-right-after-go is the weak spot; listed, not changed.
worlds/first-run.ts:296 (artifactCodeBrowserWorld) writes files via the local server, then spec click("Select tab: overflow-tab-12.md") The tab strip belongs to the seeded session only (destination-only control).
worlds/first-run.ts:536, :675 (alphaUpdateEligibilityWorld, updaterChannelWorld) return Not used by any spec. updaterChannelWorld.relaunch (:692-698) already waits for [aria-label="Release channel"] after its go().
worlds/first-run.ts:1155 (openMcpSettings) return Not used by any spec.
worlds/library.ts:209, 236, 297, 426, 443, 462, 489, 531, 546, 572, 582, 595 (setup-time location.hash = …/settings/general or /extensions) world returns; the two live specs (connect-readiness-preseeded, library-add-connector-discovery) next click("Library") (shell rail; the click's own navigation is ordered after the queued hashchange) or go(/extensions) + waitFor(Add button) Destination-only or order-safe. The other ten worlds are not imported by any spec.
worlds/session-shell.ts:1529 polls sidebar rows / route state up to 60 s Already waits on the destination's own state.
behaviors/desktop-boot.ts:92 (waitForTaskUi) waitFor(route matches && Run task/hero) Already waits.
behaviors/desktop.ts:320, :386 (waitForConnectionCard, openConnectionsSurface) steer-back polling loops Already wait on /extensions + destination text.
env/desktop-app.ts:109 location.reload() then interactive-probe poll Reload, not a hash-only race.

Spec-level go() call sites (not worlds; listed for completeness, not touched): workspace-engine-upgrade, opencode-v2-chat-routing, engine-v2-preview-flag, responsive-session-layout, cross-workspace-split-view, active-session-workspace-storm, workspace-storm-auth-coherence, library-add-connector-discovery, cloud-provider-local-credential-fallback, org-team-lifecycle-critical-path all follow go() with a destination-only waitFor/see/model-picker read. The one I would flag: cloud-provider-sync-contract.e2e.test.ts:602 goes from session/<id> to /session and immediately selectModel(...) — the model picker is on both pages, so the pick can land on the previous session's picker; it sets the global default so the outcome is the same today, but it is the shape of the race. Out of scope for this PR.

Proof

pnpm evals:e2e saved-app-creation --daytona with OPENWORK_EVAL_REF=7a31c49175f9666732ee138c3bcc6dee4e10b532 exported (==> source verified 7a31c49175f9… in both sandboxes), fresh Den + admin desktop + web sandboxes, all three deleted by the run:

selection: engine=legacy surface=legacy case=all; placement: daytona (--daytona)
✓ create, preview, save and reopen an app without changing already-open results  316522ms
"passed":1,"failed":0,"skipped":0,"skips":[],"verdict":"passed"

All 14 steps ok, including step 13 "an admin cancels deletion in the app and confirms it on the dashboard" (the #4794 fix) and step 14 "Dashboard Add opens a creation conversation". This is the spec whose failure mode motivated item 1 and whose world carries the reference open() wait for item 2; it proves the harness change is inert on a green path (the miss branch only runs on a miss, which the unit tests cover). No spec or world file was touched, so there is no other "touched spec" to run. Verdict: Passed.

Noticed, not touched

The eval.task_activity.seed hook rendered an in-flight child under a parent
whose thread status stayed "ready": it only marked the activity store busy.
Since #4635 MessageList reads that as a stopped parent and the unobserved
child shows "Waiting for task result" instead of the Working shimmer, so
task-activity-shimmer and parent-child-permission-approval lost their
[data-subagent-activity="shimmer"] row. A delegating parent is mid-run;
mark the eval thread streaming like the chat-loading seed already does.
…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.
The emptySession world declared /tmp/openwork-empty-session-<ts> but
seed.workspace() adopted the existing first-launch default workspace
instead ("OpenWork Chat" under the dev profile home). On Daytona that
profile lives inside the repo checkout, so the engine merged
/workspace/.opencode/opencode.json ("permission": "allow") over the
workspace's own catch-all and composer-run-mode never saw the workspace
MCP rule take effect.
…ce is server-owned

Workspaces created from the app live only in the server registry, so a
renderer reload reaches startServerWithoutDesktopWorkspace and restarted
the embedded server (and its engine) although it was healthy and already
serving that workspace. Reuse it when it is ready with the preferred
remote-access mode; only restart when nothing usable is running.
…o e2e/saved-app-browser-requesting-conversation
…d 'origin/e2e/fresh-session-send-gate' into e2e/child-checklist-after-stop
… 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.
… edge

An interrupted question tool leaves the engine's pending list through its
finalizer without a question.rejected event, so Stop on a delegated child
left its question (and option buttons) in the parent view. Reconcile
questions alongside permissions on terminal edges, for both engines.
OpenCode v1 runs tools on detached fibers: aborting a session marks the
question tool part as aborted but leaves its request in GET /question, so a
parent that stopped a delegated child kept showing the child's question and
its option buttons. After Stop confirms the tree idle, reject the pending
questions owned by the stopped sessions; other roots are left untouched.
…into e2e/permission-ask-and-orphans-after-abort

Stacks on #4786 (which itself carries #4723 and #4757) so test 2 of
parent-child-permission-approval is reachable. Their files are not modified here.
…-conversation' into e2e/dashboard-delete-no-buttons
… into e2e/permission-ask-and-orphans-after-abort

Stacks on #4747: creating the world's declared tmp workspace needs its
desktop-runtime-boot fix so user.reload() reuses the running server instead
of restarting the engine (which would drop every pending request). Its files
are not modified here.
…lared tmp path

Test 2 of parent-child-permission-approval ran bash without asking although the
world wrote permission.bash="ask" to the workspace opencode.json. The world
called seed.workspace() without create, so it adopted the first-launch default
workspace under the dev profile, which on Daytona lives inside the repo
checkout; the engine merges /workspace/.opencode/opencode.json
("permission": "allow") after the workspace file and the agent ruleset ends
as [* allow, bash ask, * allow] (last match wins; permissions/effective reported
shell allow, source engine, workspace file .../home/OpenWork Chat/opencode.jsonc).

Same trap and same remedy as #4747 for emptySession; opt-in so the other
splitPaneQuestions worlds keep their arrangement.
…after Stop

OpenCode v1 runs tools on detached fibers: aborting a session marks the tool
part as aborted but leaves its Permission.ask pending in /permission, exactly
as #4786 found for questions. The stopped tree would keep showing Allow once /
Deny in every pane and after reload although nothing can consume the answer.

After stopForegroundTree confirms every stopped session idle, also list pending
permissions and reject the ones owned by the stopped tree; other roots are
never touched and each withdrawal kind is best-effort on its own.
…fter-abort' into e2e/harness-hygiene-locate-workspaces-open
… into e2e/harness-hygiene-locate-workspaces-open
…ed role plus route and page roots

A miss used to quote only the first 8 visible buttons in DOM order, which
are always the shell rail, so every miss read as "the page has no
buttons" (#4794's diagnosis detour). The browser-side report now lists
every rendered element of the requested role (buttons/links for
role-less targets), the current route/hash, and whether the
[data-app-header] / [data-dashboard-page] roots are mounted. The Node
side caps the list at 40 and says how many the page really had.
Matching is unchanged.
… page DOM

Proves the browser-side branch the way Chromium will execute it: the
report lists every rendered menuitem on a dashboard whose shell rail has
nine buttons, names the route and mounted page roots, and a role-less
miss lists all nine buttons instead of the first eight.
@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 11, 2026 4:10am UTC
3 Skipped Deployments
Project Deployment Actions Updated
openwork-den Ignored Ignored Preview Sep 11, 2026 4:10am UTC
openwork-diagnostics Ignored Ignored Preview Sep 11, 2026 4:10am UTC
openwork-landing Ignored Ignored Preview Sep 11, 2026 4:10am UTC

Comment thread evals/packages/cdp/src/input.ts
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

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

Commit 4f0f2798207aed990fd590660077c4808faff432 · selected evidence

Open review report

@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 (34111 ms), confidentiality-review (36137 ms).

Blockers

  • None.

Advisories

  • None.

Needs recheck

  • None.

Observed review-thread metrics

  • Unique observed run attempts in retained window: 6
  • Distinct heads in retained window: 6
  • Current unresolved Warden threads: 0 of 2 observed
  • Observed resolved/unresolved transitions in retained snapshots: 2 / 0
  • First observed resolved/unresolved (state at first collection, not event time): 0 / 2
  • Precision: unavailable (no adjudications; no TP/FP values are inferred)
  • Observation window: 6/20 run attempts retained; truncated: false
  • Transition snapshot: 2/2 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.

@vercel
vercel Bot temporarily deployed to Preview – openwork-den September 10, 2026 23:40 Inactive
@vercel
vercel Bot temporarily deployed to Preview – openwork-landing September 10, 2026 23:40 Inactive
@vercel
vercel Bot temporarily deployed to Preview – openwork-diagnostics September 10, 2026 23:40 Inactive
@vercel
vercel Bot temporarily deployed to Preview – openwork-app September 10, 2026 23:40 Inactive
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 (6d94944450f45856537eec87c8b7f10bc1b1e463). Automated clearance satisfies the required-review gate only — a human still reviews and merges. Analysis run

@yomgui

yomgui commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Resync onto dev after the #4757 / #4786 resolution — and the #4747 revert

Merges (all SSH-signed G, pushed without force):

  1. Updated origin/e2e/permission-ask-and-orphans-after-abort (fix(app): apply per-session bash 'ask' permission and withdraw orphaned permission requests after Stop #4798) → 4bc56648d. One textual conflict in evals/packages/cdp/test/input.test.ts (import line: this PR adds MISS_CANDIDATE_LIMIT, dev's fix(app): restore OpenCode v2 thinking and effort selection #4853 adds pressKey) — kept both.
  2. Updated origin/e2e/dashboard-delete-no-buttons (test(evals): saved-app world.open waits for the destination page before the spec acts on it #4794 @ 05b752efc) → af407dfb0; origin/dev (e8bc90f9f) → 984f36ede.
  3. After Revert "fix(server): make workspace permission changes reach the engine's effective permissions (composer-run-mode)" (#4747) #4870 landed: origin/dev (858414c41) → 854b875a6; then fix(app): apply per-session bash 'ask' permission and withdraw orphaned permission requests after Stop #4798's post-revert head 614ebb795 → 6d9494445 (current head).

#4747's product commit 561b5c03a is gone from this branch. It arrived here through #4798 → #4747; dev reverted #4747 in #4870 (active-session-workspace-storm regression: stale per-workspace provider config after a switch). Merging origin/dev with the revert inverted that hunk — git revert --no-edit 561b5c03a afterwards was a no-op ("nothing to commit") — and git diff origin/dev...HEAD -- apps/app/src/react-app/shell/desktop-runtime-boot.ts is empty. #4747's evals-only commits stay. This PR's own spec (saved-app-creation) never depended on it.

pnpm --filter @openwork/app typecheck → exit 0. node --test packages/cdp/test/input.test.ts → 13/13 (this PR's 11 + dev's 2 from #4853). pnpm --dir evals run check:browser is red on evals/specs/library-mcp-servers-from-config.e2e.test.ts:21 (raw browser code string) — that file is byte-identical to dev (#4552) and fails the same way on a clean control tree; pre-existing, not this PR's.

Proof at 6d9494445

OPENWORK_EVAL_REF=6d94944450f45856537eec87c8b7f10bc1b1e463 pnpm evals:e2e saved-app-creation --daytona (placement: daytona, source verified 6d9494445… in all sandboxes; fresh Den + admin desktop + web sandboxes, all deleted by the runner):

test result
create, preview, save and reopen an app without changing already-open results (this PR's proof spec; all 14 steps incl. step 13 delete-from-dashboard and step 14 Dashboard Add) ✓ passed (424.9 s)
APP-ISOLATION embedded MCP Apps isolate siblings… (#4829) × world provisioning: Mock issuer gate failed: health reported https://3979-…, expected https://3979-…
APP-ARCHIVE archived conversations render Apps without actions (#4863, new today) × same Mock issuer gate failed signature

"passed":1,"failed":2,"skipped":0. Both red cases are appWeb worlds seeding two seed.mock(...) without explicit ports into one sandbox, so both default to daytonaPort: 3979 (evals/packages/env/src/mock.ts:56) and the second mock's pkill + rebind trips the issuer gate — now 5/5 across #4779, #4794 and this branch; #4829 and #4863 proved those cases with --local only. Attribute to #4829 / #4863 (give the second mock a distinct port). This PR does not touch that code; its harness change is inert on the green path (the miss branch is unit-tested).

Squash diff shrinks to af2b46ae3 + 7a31c4917 once #4798 and #4794 (and their ancestors) land.

diff-warden[bot]
diff-warden Bot previously approved these changes Sep 11, 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 (489ffd7192d7f953e1ce991e634ba31dfef55fcc). Automated clearance satisfies the required-review gate only — a human still reviews and merges. Analysis run

@yomgui

yomgui commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Restacked after #4786 landed: merged the updated #4798 (0912f4b5d, which resolved its opencode-interruption.ts / opencode-session-native.test.ts conflicts against the #4786 squash) → e6a8205c5, then origin/dev 82dfff729 → 489ffd719 (signed, clean, no conflicts). git diff origin/dev is exactly #4798's three files + #4794's evals/worlds/saved-apps.ts + this PR's two evals/packages/cdp files; desktop-runtime-boot.ts has no diff vs dev (the #4747 revert survived). Typecheck exit 0; node --test packages/cdp/test/input.test.ts 13/13; opencode-session-native.test.ts 42/42. MERGEABLE. No re-proof of saved-app-creation for this restack: the delta vs the proven 6d9494445 is #4798's helper hunks (unit-tested, not on this spec's path) and dev commits; the earlier green own-test at 6d9494445 stands as the harness-change proof.

@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 (4f0f2798207aed990fd590660077c4808faff432). Automated clearance satisfies the required-review gate only — a human still reviews and merges. Analysis run

@yomgui

yomgui commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Restack after #4770 landed (222fde99e)

Merged origin/dev → 4f0f27982 (signed G, plain merge, no rebase/force/stash). One additive conflict in evals/packages/cdp/test/input.test.ts: only the import line (this PR's MISS_CANDIDATE_LIMIT vs #4770's clickTarget, DisabledTargetError) — unioned; every test body from both sides merged automatically. The file now holds 17 tests = this branch's 13 (11 own + 2 from #4853) + #4770's 4; nothing dropped.

Proof at 4f0f27982

OPENWORK_EVAL_REF=4f0f2798207aed990fd590660077c4808faff432 pnpm evals:e2e saved-app-creation --daytona (placement: daytona, source verified 4f0f27982… in every sandbox; all sandboxes deleted by the runner):

test result
create, preview, save and reopen an app without changing already-open results (this PR's proof spec, all 14 steps) ✓ passed (342.1 s)
APP-ISOLATION … (#4829) / APP-ARCHIVE … (#4863) × both in world provisioning: Mock issuer gate failed — the two-mocks-on-daytonaPort 3979 collision in those appWeb worlds (6/6 reproductions across #4779/#4794/#4810 now); not this PR's code

"passed":1,"failed":1+1,"skipped":0; the harness change is inert on the green path and its miss branch is unit-tested. MERGEABLE.

@yomgui
yomgui merged commit 71d22ac into dev Sep 11, 2026
27 checks passed

This branch was successfully deployed

2 active (1 outdated) and 2 inactive (outdated) deployments
Preview – openwork-app — 4f0f2798 Deployed Sep 11, 2026 by vercel[bot]
Preview – openwork-den — 489ffd71 Deployed Sep 11, 2026 by vercel[bot]
Preview – openwork-diagnostics — 6d949444 Deployed Sep 10, 2026 by vercel[bot]
Preview – openwork-landing — 6d949444 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