Skip to content

fix(evals): make specs placement-portable — host workspace root, sandbox-aware ports, profile pruning - #3388

Merged
benjaminshafii merged 24 commits into
devfrom
feat/eval-multi-sandbox-connector
Aug 1, 2026
Merged

fix(evals): make specs placement-portable — host workspace root, sandbox-aware ports, profile pruning#3388
benjaminshafii merged 24 commits into
devfrom
feat/eval-multi-sandbox-connector

Conversation

@benjaminshafii

@benjaminshafii benjaminshafii commented Jul 31, 2026

Copy link
Copy Markdown
Member

Status update (2026-08-01)

Everything below the rule was written when mode B first ran; the PR has since grown the missing pieces and they are proven:

  • The two-member connector spec IS now in this PR (org-connector-two-members.slow.test.ts) and is green twice on two separate Daytona sandboxes — 133.0s / 135.2s, driver outside all sandboxes. Both members OAuth-connect their own account and call the connector's tool from the composer; the mock witnesses two distinct bearer fingerprints. Full evidence: the run report.
  • Mode B is green (two-daytona-desktops.slow.test.ts, plus the connector runs above). The renderer-over-proxy hang was renderer JS starvation (workspace scan + full disk), not the proxy — fixed by the /tmp workspaces and profile pruning already in this PR.
  • Of the "What's left" table: Idea: Rename into Labori #1 resolved via startMockMcp({ publicUrl }) (the seam existed but was dead code — health-check and stale-log bugs fixed here); Minor issue in README: audience description is an incorrect generalization #2 unnecessary (the eval image's real browser completes the OAuth hop on-sandbox); Add engine doctor + guided OpenCode install #3 landed (args + per-credential tokenId, asserted by the spec); the model knob exists (OPENWORK_EVAL_MODEL); CA trust/TLS not needed (Daytona preview URLs are real HTTPS).
  • The earlier "blocked on a product defect (feat(connections): show real provider logos across the connect flow #3375)" claim is retracted in the spec header itself; the spec stays opt-in because it needs provisioned placement (two sandboxes, reachable Den, published mock), which nightly does not have.

Groundwork for the two-member connector spec (two Electrons in two sandboxes, den + connector reachable from both). I set out to write that spec; driving mode B for the first time surfaced three real bugs that had to land first. The spec itself is not in this PR — see "What's left".

Three bugs, each found by running

1. Specs assumed the driver's filesystem is the app's. app-smoke passed process.cwd() as a workspace path. Correct only when driver and app share a disk. Driving a sandbox from a laptop, the app was told to open a directory that exists only on the laptop — the symptom was onboarding hanging 120s on "Power your first task" with no error, reading like a broken app.

Host.workspaceRoot now states it (repoRoot locally, /workspace on Daytona) and DesktopHandle re-exposes it, for specs that genuinely need the repo (skills-local reads .opencode/skills).

2. Daytona surface ports were allocated from a local counter. The local host finds a free port by binding one; the Daytona host cannot, since the port must be free on another machine. It guessed from { primary: 9825, next: 9830 } — and the OpenCode sidecar, which picks its own port at boot, was observed holding 9825, the CDP primary. Electron's debugger never bound; the preview URL timed out after 180s naming nothing. Now it asks the sandbox which ports are listening and skips them.

3. Stale surface profiles filled the disk. Disposal removes a profile on the happy path; every killed or failed run leaks one at ~50MB. 27 of them took a 10GB sandbox to 99% full. The symptom was not "disk full" — it was the renderer failing Runtime.evaluate for 240s. That cost four debug cycles. clearStaleSurfaces already pkills by rootDir and so already assumes exclusive ownership; it now deletes the directories it orphans.

A regression I caught and reverted

Routing app-smoke through workspaceRoot made it open the whole monorepo (/workspace, node_modules included) instead of the smaller /workspace/evals that process.cwd() happened to give. Engine scan blocked the renderer past 240s. It now uses a fresh /tmp/openwork-app-smoke-<stamp> — small, and valid on any host. Only caught because I re-ran mode A after each change; worth keeping that habit.

Proof

Mode A (in-sandbox), after the disk was freed:

Spec Result
app-smoke PASS — 59.1s, then 58.3s
skills-local PASS — 79.1s

Pruning verified: profiles-left=1 (the current run only) where 27 had accumulated; disk steady at 84%.

tsc -p evals clean. pnpm --dir evals test 95 pass / 0 fail. Three Host fakes in runner tests needed workspaceRoot — the type system caught them, which is the seam working.

Mode B status, honestly

Mode B (vitest on a laptop, Electron in a sandbox, CDP over a preview URL) went from hanging at 120s to completing workspace creation in 68.5s. It is not yet green end to end. Two things remain, both documented rather than fixed:

  • The driver needs the vision key. validate() runs driver-side, so OPENAI_API_KEY/ANTHROPIC_API_KEY must be on the driver — daytona-spec.sh sources /daytona-secrets/*.env inside the sandbox, which does nothing for mode B. Same family as bugs 1 and 3: something moved sides.
  • Renderer responsiveness over the proxy during engine boot still needs work.

What's left for the connector spec

# Gap
1 startMockMcp({ host }) — labs are local-only. publicUrl already exists as an attach hatch, so a generic Host.startService() is the clean shape.
2 captureOpenedUrls({ host }) — the xdg-open shim lands on the driver, not where Electron runs
3 Mock MCP call log: record tool arguments and token identity (it already logs toolNames + authorized)
4–7 CA trust, TLS terminator, requires([...]) capabilities, model knob

Note this spec cannot run in nightly — nightly is blacksmith-4vcpu-ubuntu-2204 with no Daytona access. That is why the capability model matters: without it, it would skip silently and read green.

…he driver's

A spec passing process.cwd() as a workspace path is only correct when the
driver and the app share a filesystem. Driving a sandbox from a laptop, the
app was asked to open a directory that exists only on the laptop — observed
as onboarding hanging 120s on 'Power your first task' with no error, which
reads like a broken app rather than a bad path.

Host.workspaceRoot states it (repoRoot locally, /workspace on Daytona) and
DesktopHandle re-exposes it, so app-smoke asks the app's own host.
…andbox

The local host finds a free port by binding one; the Daytona host cannot,
because the port must be free on another machine. It allocated from a local
counter instead, so the OpenCode sidecar — which picks its own port at boot —
was observed holding 9825, the CDP primary. Electron's debugger never bound
and the preview URL timed out after 180s with nothing naming the cause.

Ask the sandbox which ports are listening and skip those.
Routing app-smoke through Host.workspaceRoot made it open the whole monorepo
(/workspace, node_modules included). The engine scan then blocked the renderer
past 240s and the spec failed reading location.hash — a regression versus the
old process.cwd() (which happened to be the smaller /workspace/evals).

workspaceRoot stays the right primitive for specs that genuinely need the repo
(skills-local reads .opencode/skills); a workspace path just needs to be small
and valid on the app's host.
Disposal removes a profile on the happy path, but every killed or failed run
leaks one at ~50MB. Twenty-seven filled a 10GB sandbox to 99%, and the symptom
was not 'disk full' — it was the renderer failing to answer Runtime.evaluate
for 240s, indistinguishable from a broken app. Cost four debug cycles to find.

clearStaleSurfaces already pkills by rootDir and so already assumes exclusive
ownership; deleting the directories it just orphaned is the matching half.
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
openwork-app Ready Ready Preview Aug 1, 2026 9:03am
openwork-den Ready Ready Preview Aug 1, 2026 9:03am
openwork-den-worker-proxy Ready Ready Preview Aug 1, 2026 9:03am
openwork-diagnostics Ready Ready Preview Aug 1, 2026 9:03am
openwork-landing Ready Ready Preview, v0 Aug 1, 2026 9:03am

…nnector and call its tools

Two SEPARATE desktops, own profiles, signed in as different members. A single
desktop clearing localStorage only simulates per-member isolation; two make it
real — the load-bearing assertion is that member B still needs their own
sign-in after member A connected.

The connector is the authority on use: mock MCP now records tool ARGUMENTS and
a sha256 fingerprint of the caller's bearer token, so the spec asserts on calls
actually served and on DISTINCT credentials rather than the app's own
'Connected' text.

Placement: both desktops use the ambient host; moving either to its own sandbox
is desktop({ host: daytonaSandbox(id) }) once mode B is green.
…ws is the test

ensureMemberSession threw when mark-verified failed, even though the member had
just been created successfully and could sign in. On the eval sandbox the helper
uses -uroot with no password while den's MariaDB root has one, so bootstrapping
a second member aborted the whole spec at 2s.

Warn and continue; if the subsequent sign-in fails, report both causes.
Comment thread scripts/mock-oauth-mcp-server.mjs Fixed
waitForText reads innerText, which is render-aware: a badge styled with CSS
text-transform reads NEEDS YOUR SIGN-IN on screen while textContent holds
'Needs your sign-in'. The reveal could never agree with the wait.
@benjaminshafii

Copy link
Copy Markdown
Member Author

Status: 26 of ~29 steps green, including the headline assertion

I was wrong about the product regression — retracting it

I claimed /settings/extensions/connections was broken by #3375. It is not. A probe found the surface renders fine: 31 buttons, 557 chars of text, zero errors, zero console errors.

The real cause was mine. The app canonicalises the route#/workspace/<id>/settings/extensions/connections becomes #/workspace/<id>/extensions/connections. My steer-back loop checked for the pre-rewrite form, so it re-navigated every iteration, fighting the rewrite until the surface never settled. That produced the blank page I blamed on dev. #3375 is exonerated; nobody should bisect it on my account.

What now passes

Everything through member B connecting their own account:

  • arrangement: connector, two members, org connection created and readable per-member over the API
  • member A: desktop boots, signs in, connections surface, card renders, NEEDS YOUR SIGN-IN, detail opens, OAuth required
  • member A connects: authorize observed at the connector, Connected with your own account., API flips to connectedForMe: true
  • member B still shows NEEDS YOUR SIGN-IN while A is connected — the per-member isolation assertion, the whole reason two desktops exist
  • member B connects their own account
  • three vision frames pass

What is still blocked, and why it argues for the original design

The tool-call phase (steps 27–29) never starts: go(.../session) cannot execute because appA's renderer is wedged. Not a burst — it survived a 15s probe retried across 120s.

This is resource exhaustion, not product behaviour. Two Electrons, two OpenCode engines, den and MySQL on one 9GB sandbox. Which is an argument for the two-sandbox topology this branch was groundwork for: one desktop and one engine per sandbox removes the ceiling entirely.

Cheapest next step without more hardware: reorder so only one desktop is heavily active — A connects → A's tool call → then create B's desktop → B sees NEEDS YOUR SIGN-IN (isolation still valid, B is created after A connected) → B connects → B's tool call.

Resilience fixes landed while getting here

  • CDP timeouts now name the expression. The rejection fires from a timer, so the caller's stack is gone; CDP call Runtime.evaluate timed out alone can't say which of dozens of evaluations blocked. This one change turned a guessing game into a one-line fix twice.
  • go() retries. Setting a hash is idempotent; a bare 20s evaluate was the most common way a long journey died near its end.
  • Frames wait for paint, not DOM. waitForText proves the DOM; the detail panel paints slightly later, so the vision check passed one run and failed the next.
  • Reveal matches rendered text. innerText is render-aware, so a badge styled text-transform: uppercase reads NEEDS YOUR SIGN-IN while textContent holds Needs your sign-in — the reveal could never agree with the wait.

The spec stays opt-in (OPENWORK_EVAL_CONNECTOR_SPEC=1) until the tool-call phase runs. app-smoke, skills-local, and first-run-cloud-share remain green on this branch.

publicUrl was a dead seam: waitForHealth read child?.exitCode on the null
child and declared the (never-spawned) server exited, and the request log
of a long-lived mock satisfied atLeast with a PREVIOUS run's tool calls,
returning before this run's calls arrived. Guard the health check to only
fail on a spawned child that exited, carry the mock's timestamp on each
tool call, and let callers scope toolCalls with sinceIso.
… sandbox

Two desktops plus two engines starve renderers on one 9GB sandbox — that
ceiling, not any product defect, is what kept the tool-call phase red, so
the header's retracted defect story goes too. OPENWORK_EVAL_DAYTONA_SANDBOX_A/_B
now place each member via desktop({ host: daytonaSandbox(id) }), the mock
publishes through OPENWORK_EVAL_CONNECTOR_MOCK_PUBLIC_URL so Den, both
desktop browsers, and the driver can all reach it, and this run's tool
calls are scoped with sinceIso.

Green twice end to end (133.0s, 135.2s): both members OAuth-connect their
own account, call mock_echo from the composer, and the connector witnesses
two distinct bearer fingerprints.
@benjaminshafii

Copy link
Copy Markdown
Member Author

org-connector-two-members: green on two Daytona sandboxes — twice

The last opt-in spec now completes its tool-call phase. The fix was placement, as predicted: each member's desktop gets its own sandbox via desktop({ host: daytonaSandbox(id) }), so two Electrons + two engines never contend for one 9GB box.

Runs (driver on a Mac outside all sandboxes, mode B):

run result duration
1 green 133.2s
2 (final code) green 133.0s
3 (final code) green 135.2s
✓ specs/org-connector-two-members.slow.test.ts  two members each connect their own
  account to one org connector and call its tools  133042ms / 135211ms

Topology: Den server sandbox (test-server-on-daytona.sh, seeded demo org) + mock OAuth MCP on that sandbox with ISSUER=<public preview URL> + desktop A and desktop B on separate openwork-eval-vnc sandboxes. Env: OPENWORK_EVAL_DAYTONA_SANDBOX_A/_B, OPENWORK_EVAL_CONNECTOR_MOCK_PUBLIC_URL, OPENWORK_EVAL_MODEL=big-pickle (signed-in org desktops only offer the org's Zen models — local openai/* is filtered by desktop policy).

The connector's own witness log (the authority, not the UI): each run served both markers under distinct bearer fingerprints —

08:52:19 mock_echo {"text":"a-1785574331698"} token: a7d2f7da4e15
08:52:26 mock_echo {"text":"b-1785574334716"} token: 4787b0fe26b7
08:54:52 mock_echo {"text":"a-1785574483795"} token: 952a847864c1
08:54:57 mock_echo {"text":"b-1785574486615"} token: 8744fda9ad6e

Two real bugs found by running it:

  1. startMockMcp({ publicUrl }) was a dead seam — child?.exitCode !== null on the null child declared the never-spawned server "exited". Never exercised until now.
  2. A long-lived mock's request log satisfied toolCalls({ atLeast: 2 }) with the previous run's calls. toolCalls now takes sinceIso (and each call carries the mock's timestamp), which also protects the token-distinctness assertion from cross-run contamination.

Vision-validated frames (run 2, evals/results/rolls/2026-08-01T08-50-30-689Z-org-connector-two-members/):

member A must connect member A connected member B NOT inherited member B's tool call ran

Also ran: pnpm exec tsc -p evals clean; evals unit tests 95 pass / 0 fail.

The spec header's earlier "blocked on a product defect (#3375)" claim is now retracted in the code itself; the honest reason it stays opt-in is that it needs provisioned placement (two sandboxes + a reachable Den + a published mock), not that anything product-side is broken.

Repro:

OPENWORK_EVAL_APP_SPECS=1 OPENWORK_EVAL_CONNECTOR_SPEC=1 \
OPENWORK_EVAL_DEN_API_URL=<den api url> OPENWORK_EVAL_DEN_WEB_URL=<den web url> \
OPENWORK_EVAL_DAYTONA_SANDBOX_A=<id-a> OPENWORK_EVAL_DAYTONA_SANDBOX_B=<id-b> \
OPENWORK_EVAL_CONNECTOR_MOCK_PUBLIC_URL=<mock url> OPENWORK_EVAL_MODEL=big-pickle \
pnpm --dir evals exec vitest run --config vitest.config.ts --project nightly specs/org-connector-two-members.slow.test.ts

CodeQL: js/polynomial-redos — ^Bearer\s+(.+)$ backtracks polynomially on
'Bearer' followed by header spam. One shared linear parse for the auth
check and the fingerprint.
@benjaminshafii

Copy link
Copy Markdown
Member Author

Addendum: CodeQL flagged a polynomial-ReDoS in the mock's bearer-header regex (js/polynomial-redos, high) — replaced with a shared linear-time parse in 2b62df0. Redeployed the mock at that tip and re-ran the spec end to end: green, 127.9s, fresh witness log:

09:04:51 mock_echo {"text":"a-1785575083225"} token: 187d39be4aac
09:04:57 mock_echo {"text":"b-1785575086025"} token: 78c0feb69a8c

CodeQL, openwork-tests (ubuntu + macos), and both Analyze jobs now pass on the tip. That makes four greens for this spec today (133.2s, 133.0s, 135.2s, 127.9s).

@benjaminshafii

Copy link
Copy Markdown
Member Author

@OmarMcAdam @src-opn — requesting your approval; auto-merge (squash) is armed, so one approval ships it.

30-second review guide:

  • Scope: evals-only + scripts/mock-oauth-mcp-server.mjs. No app/server/Den code.
  • What it does: makes eval specs placement-portable (host-owned workspace roots, sandbox-aware ports, profile pruning) and lands the two-member org-connector spec, proven green 4x with each member's desktop on its own Daytona sandbox — evidence with frames and the connector's own token-fingerprint log is in this comment.
  • Risk: nightly/CI unaffected — the new spec self-skips without its opt-in env (nightly has no Daytona). All checks green, incl. CodeQL after fixing a ReDoS it caught in the mock.

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.

3 participants