[brennanb2025]

ELI5

One E2E test still expected the pre-#19542 world where mail to a bare terminal handle from a sender in no Run was quietly filed under the legacy Run. #19542 made that a hard refusal, so the test now pins the refusal instead.

What Changed

• tests/e2e/orchestration-idle-mail-delivery.spec.ts: the test keeps unbound direct mail durable without pointing to an unsafe check is rewritten as refuses unbound direct mail from a sender in no Run instead of pushing it, keeping its position and the spec's structure. It now asserts, in order:
  1. orchestration.send from the non-Run test client to the pane's bare handle rejects with a RuntimeRpcFailureError whose code is runtime_error and whose message contains Run is required.
  2. Its data carries the client's own durable-mutation stamp (orchestrationRequestId, from src/cli/runtime/client-error-recovery.ts) and has none of the server-side orchestrationSkillRecoveryData() keys (effectsApplied, guide, nextCommandArgs, nextSteps) — pinning that the server attached no recovery data, without freezing the client stamp's exact shape (an exact toEqual would go red with no product change if orchestration-recovery-command.ts ever mapped orchestration.send to an originalCommand).
  3. readMailbox(userDataDir, pane.handle) is [] immediately after the rejection.
  4. After walking a busy→idle edge on the pane (the push trigger) and letting NO_DELIVERY_SETTLE_MS elapse so the push window (microtask + 500 ms Enter delay) is fully covered, the mailbox is still [] and the fake agent's stdin ledger is byte-identical to its pre-send snapshot — no pointer, no synthesized Enter.
• No product code changed. Neighbouring tests untouched.

Why

#19542 (98fdbc4ade, "file federated worker mail under the coordinator Run") deleted every ?? LEGACY_RUN_ID write fallback; its body states the intended contract directly:

All four ?? LEGACY_RUN_ID write fallbacks are deleted (message-insert.ts, task-store.ts, decision-gate-store.ts, check-worker.ts). A missing Run now throws instead of misfiling.

On the send path, recipient-routing.ts resolves a bare handle with a live pane but no Run/Dispatch to runId: params.senderRunId; the E2E client is in no Run, so insertMessage throws Run is required (src/main/runtime/orchestration/db/messages/message-insert.ts). The old test asserted the pre-#19542 outcome (row stored with read: 0, delivered_at: null), so it fails with that error at sendMail. #19542 touched no E2E file and e2e.yml only runs on schedule/dispatch, which is how it slipped past PR CI.

It is the only remaining red spec on main's scheduled E2E and on the 1.4.199 release branch: runs 34284171625 and 34298315442.

The new test keeps the title's original intent — unbound direct mail must never be pushed into an idle pane — and states the mechanism that now guarantees it: nothing is stored, so the idle scan has nothing to point at.

Linked Issue

N/A — follow-up to #19542; no separate issue filed.

Visual Proof

N/A — test-only change to an E2E spec; no UI or interaction change.

Testing

All run locally on macOS (darwin arm64) in a fresh worktree (origin/main 210ac1f77c + these commits), pnpm install from scratch.

Second commit 238d2d797a (review tightenings: absence checks on the recovery keys, settle window before the ledger read):

Command	Result
SKIP_BUILD=1 ORCA_BACKGROUND_LAUNCH=1 npx playwright test tests/e2e/orchestration-idle-mail-delivery.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1 -g "unbound direct mail"	1 passed (8.9s), exit 0
pnpm run check:code-quality:changed	passed: 0 new findings across 1 changed file
pre-commit hook (oxlint, oxlint react-doctor, oxfmt --write)	passed; oxfmt made no changes
Not re-run at this commit: whole spec file, typecheck:e2e	the second commit only edits the same test body (no new imports or symbols); the whole-file 12/12 below is from the first commit

First commit b09788541a:

Command	Result
SKIP_BUILD=1 ORCA_BACKGROUND_LAUNCH=1 npx playwright test tests/e2e/orchestration-idle-mail-delivery.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1 (whole file)	12 passed (2.5m), 0 failed, exit 0
same, -g "unbound direct mail" (the rewritten test alone; first run included pnpm run ensure:electron-runtime + the globalSetup build)	1 passed, exit 0
same single test, first draft of the assertion (message: 'Run is required' exact, data: undefined)	failed — real wire shape is message suffixed with Orchestration mutation request ID: <uuid>. and data: { orchestrationRequestId } from the client's durable-mutation recovery; assertion corrected to that observed shape (this is what the committed test pins)
pnpm run check:code-quality:changed	passed: 0 new findings (code quality, type-aware, React Doctor) across 1 changed file
pnpm run typecheck:e2e	exit 1 with 197 errors, all pre-existing in other files; 0 in the changed spec — config/tsconfig.e2e.json documents this project as not yet enforced (~198 known errors)
git diff --check	clean
pre-commit hook (oxlint, oxlint react-doctor, oxfmt --write)	passed; oxfmt made no changes (committed file is byte-identical to what the e2e run executed)
Not run: pnpm tc (node/cli/web)	no src/ change, so nothing for those projects to see; the E2E-specific project above covers the changed file
Not run: Linux / Windows / SSH	E2E suite is not in PR CI; e2e.yml runs on schedule/dispatch — the next scheduled main run and a re-run on the 1.4.199 release branch are the real gate

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

Follow-up (not done here, product code deliberately untouched)

• The refusal surfaces as a bare Error('Run is required') from insertMessage, which mapRuntimeError (src/main/runtime/rpc/errors.ts) flattens to wire code runtime_error with no recovery data. The sibling no-recipient path a few lines earlier in send-methods.ts throws OrchestrationError('run_required', ..., orchestrationSkillRecoveryData()) (effectsApplied, guide, nextSteps). If the unbound-handle refusal should be equally actionable for agents (run_required + recovery data), that is a product change; this test's code/data assertions are the ones to update with it. Same applies to the other three writers fix(orchestration): file federated worker mail under the coordinator Run #19542 hardened (task-store.ts, decision-gate-store.ts, check-worker.ts).

Review

Agent skill upstream boundary

N/A

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 3d713c59-caee-4418-aa58-b9d0964c90b1

📥 Commits

Reviewing files that changed from the base of the PR and between b097885 and 238d2d7.

📒 Files selected for processing (1)

• tests/e2e/orchestration-idle-mail-delivery.spec.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The end-to-end test verifies that direct mail from a sender in no Run is rejected. It checks for a RuntimeRpcFailureError with code runtime_error, a message containing “Run is required”, and an orchestration request identifier. It also verifies that recovery data is absent, the recipient mailbox remains empty across a busy-to-idle transition, and pane stdin is unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 238d2

This change updates E2E coverage so unbound direct mail is rejected without creating mailbox state or changing pane input. The expected failure behavior and side-effect boundaries are covered, with no current merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the orchestration test change and the Run-required contract for unbound direct mail.
Description check	✅ Passed	The description clearly covers the change, reason, testing, visual-proof status, and scope. It is mostly complete, although the required Linked Issue section is marked N/A despite referencing #19542 e…

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Rewrites the unbound-mail E2E spec to pin the Run-required refusal — the test now asserts that orchestration.send to a bare handle from a sender in no Run rejects with code: 'runtime_error', a Run is required message, and data of exactly { orchestrationRequestId }, and that the mailbox stays empty through a busy→idle edge with no pointer or synthesized Enter reaching the agent.

I traced the full contract and it holds: insertMessage throws a plain Error('Run is required'), mapRuntimeError flattens it to runtime_error with no recovery data, and the client's durable-mutation recovery stamps only orchestrationRequestId (no originalCommand, since orchestration.send is not a worker command). The toEqual on data is deliberately tight — it would catch a future regression where the server starts attaching recovery data or the client starts populating originalCommand — and the inline comment documents exactly why runtime_error rather than run_required, so it is pinning behavior, not theatre.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}