Skip to content

refactor(runtime-host): fold the desktop E2E candidate into the real main - #3106

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/host-fold-e2e-candidate-main
Aug 16, 2026
Merged

refactor(runtime-host): fold the desktop E2E candidate into the real main#3106
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/host-fold-e2e-candidate-main

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

desktop-e2e-execution-candidate-main.ts was a second copy of candidate startup, selected by module name from Desktop boot under MAKA_E2E=1. It already drifted from the real main (no startup-failure classification, no policy bootstrap).

There is now one candidate entrypoint:

  • Delete desktop-e2e-execution-candidate-main.ts and its @maka/runtime-host/desktop-e2e-execution-candidate-main export
  • Desktop boot always resolves @maka/runtime-host/execution-candidate-main and passes desktopE2e: true when the isolated E2E profile is on
  • The launcher forwards --desktop-e2e 1; the real main applies FakeBackend + /compact checkpoint, OAuth stubs, bootstrapRuntimePolicy: false, idleGraceMs: 500, and the Electron parent watch
  • E2E startup failures now go through the same classifier as production

Production composition is unchanged: the flag is only sent from Desktop E2E.

Fixes #3085

Verification

  • Grep: no remaining desktop-e2e-execution-candidate-main callers or export
  • npx biome check on the changed files — clean
  • npm --workspace @maka/runtime-host run typecheck — clean
  • npm --workspace @maka/desktop run typecheck — clean
  • Focused host tests (candidate CLI, desktop E2E backend, startup failure, owned candidate) — 16/16
  • Official npm --workspace @maka/runtime-host run test:dist943/944. The one failure is owned Host exits promptly after its first connection closes under full-suite load (failed vs connected with a 2s election window). Isolated 3/3; not this change
  • Official npm --workspace @maka/desktop run test:dist862/862
  • Playwright send-message + slash-command-menu5/5 (FakeBackend + /compact still work)

Not run: full desktop e2e suite

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the fold and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…main

Remove desktop-e2e-execution-candidate-main and its package export.
Desktop boot always launches execution-candidate-main and passes
--desktop-e2e 1 so the same startup path classifies failures and
applies the FakeBackend composition, OAuth stubs, idle grace, and
parent watch.

Fixes apache#3085

Generated-by: Grok
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 977131db-87b1-44e9-a944-e184c0e72e54

📥 Commits

Reviewing files that changed from the base of the PR and between 21c8de9 and a17df93.

📒 Files selected for processing (2)
  • packages/runtime-host/src/__tests__/execution-candidate-main.test.ts
  • packages/runtime-host/src/execution-candidate-main.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/runtime-host/src/execution-candidate-main.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Problem solved

Desktop E2E startup now uses @maka/runtime-host/execution-candidate-main instead of a separate entrypoint. Desktop passes --desktop-e2e 1 for isolated E2E profiles.

The shared entrypoint applies the Desktop E2E composition:

  • FakeBackend
  • Deterministic /compact checkpoints
  • OAuth stubs
  • Disabled runtime policy bootstrap
  • idleGraceMs: 500
  • Electron parent-process watching
  • Production startup error classification

Production composition remains unchanged.

Design assessment

This PR extends the existing candidate main. It does not create a parallel startup path.

The change is the smallest coherent solution. The mode flag and its propagation through the CLI, launcher, connection layer, and Desktop boot are required to select the shared composition.

The deleted desktop-e2e-execution-candidate-main.ts entrypoint and package export should remain removed. No further deletion is apparent without reducing regression coverage. The added tests cover flag validation, invalid startup input, deterministic checkpoint behavior, and Desktop E2E composition.

Validation

Reported validation includes:

  • Clean formatting
  • Typechecks
  • Focused runtime-host tests
  • Desktop tests
  • Selected Playwright tests
  • Runtime-host suite: 943/944 tests reported
  • One unrelated load-sensitive runtime-host failure
  • Full Desktop E2E suite not run

Required-check status is not independently verified from the provided information.

Review-relevant risks

The removed package export changes a public package contract. Material changes in public contracts require independent human review under repository policy.

Desktop startup behavior now depends on mode-flag propagation and shared composition selection. A propagation or startup-classification regression could affect Desktop E2E behavior. Material changes in user-visible startup behavior require independent human review under repository policy.

The person performing the merge must review the final diff. A maintainer makes the final determination.

Walkthrough

Desktop E2E startup now uses the standard runtime host candidate entrypoint. The desktopE2e flag propagates through CLI parsing and candidate launch. Desktop-specific backend, OAuth, idle-grace, and parent-process lifecycle behavior are injected conditionally.

Changes

Desktop E2E runtime host

Layer / File(s) Summary
Desktop E2E flag propagation
packages/runtime-host/src/candidate-cli.ts, packages/runtime-host/src/client/*, apps/desktop/src/main/runtime-host-*.ts, packages/runtime-host/src/__tests__/candidate-cli.test.ts
The CLI accepts and validates --desktop-e2e 1. Candidate launch and desktop runtime host wiring forward the flag to the standard execution candidate entrypoint.
Desktop E2E execution lifecycle
packages/runtime-host/src/desktop-e2e-execution.ts, packages/runtime-host/src/execution-candidate-main.ts, packages/runtime-host/src/__tests__/desktop-e2e-execution.test.ts, packages/runtime-host/src/__tests__/execution-candidate-main.test.ts
The standard entrypoint injects deterministic Desktop E2E dependencies, applies the idle grace period, monitors the parent process, and cleans up on exit. Tests cover checkpoint recording, missing recorder errors, and invalid flag values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to a17df

The PR consolidates desktop E2E startup onto the production candidate entrypoint while keeping production composition unchanged; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant DesktopApp
  participant CandidateLauncher
  participant ExecutionCandidateMain
  participant DesktopE2eBackend

  DesktopApp->>CandidateLauncher: Launch with desktopE2e enabled
  CandidateLauncher->>ExecutionCandidateMain: Pass --desktop-e2e 1
  ExecutionCandidateMain->>DesktopE2eBackend: Create desktop E2E dependencies
  DesktopE2eBackend-->>ExecutionCandidateMain: Provide deterministic backend and OAuth behavior
  ExecutionCandidateMain->>DesktopApp: Close runtime when parent exits
Loading

Possibly related issues

  • maka-agent/maka-agent#2704 — The issue also concerns Desktop E2E runtime host setup and fake backend behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai Use Disclosure ⚠️ Warning The PR discloses Grok-authored material code, but the third PR commit a17df93 adds startup logic and a test without a Generated-by trailer; the first two commits use Generated-by: Grok. Add Generated-by: Grok to a17df93, review CONTRIBUTING.md's Human ownership and AI attribution section, and ensure the trailer survives squash or amend.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes consolidating the Desktop E2E candidate into the real runtime-host entrypoint.
Description check ✅ Passed The description follows the template and documents scope, verification results, AI use, checklist status, and the known unrelated test failure.
Linked Issues check ✅ Passed The changes remove the duplicate entrypoint, use the real main with an E2E flag, preserve FakeBackend behavior, and include relevant validation for issue #3085.
Out of Scope Changes check ✅ Passed The changed files and tests directly support consolidating the Desktop E2E candidate and do not show unrelated scope.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/runtime-host/src/__tests__/desktop-e2e-execution.test.ts (1)

51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the duplicate version assertion.

Line 50 already asserts that checkpoint.version is 2. Line 51 adds no coverage.

Proposed simplification
-  assert.ok(recorded[0]?.checkpoint.version === 2);

As per path instructions, flag concrete code that can be deleted or simplified.

Source: Path instructions


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc6c16f9-9214-4ee3-b709-5000451dfa1e

📥 Commits

Reviewing files that changed from the base of the PR and between a032beb and 2f09c8b.

📒 Files selected for processing (11)
  • apps/desktop/src/main/runtime-host-boot.ts
  • apps/desktop/src/main/runtime-host-desktop-candidate.ts
  • packages/runtime-host/package.json
  • packages/runtime-host/src/__tests__/candidate-cli.test.ts
  • packages/runtime-host/src/__tests__/desktop-e2e-execution.test.ts
  • packages/runtime-host/src/candidate-cli.ts
  • packages/runtime-host/src/client/connect-or-spawn.ts
  • packages/runtime-host/src/client/launcher.ts
  • packages/runtime-host/src/desktop-e2e-execution-candidate-main.ts
  • packages/runtime-host/src/desktop-e2e-execution.ts
  • packages/runtime-host/src/execution-candidate-main.ts
💤 Files with no reviewable changes (2)
  • packages/runtime-host/src/desktop-e2e-execution-candidate-main.ts
  • packages/runtime-host/package.json

Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.

Keep a single version assertion and a type-narrowing return so
summary is checked without a second version check.

Generated-by: Grok
@1625567290

1625567290 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

The extra assert.ok(checkpoint.version === 2) was a TypeScript narrow so .summary type-checks on the HistoryCompactCheckpoint union. Coverage-wise it looked like a duplicate of the assert.equal on the line above.

Replaced both with one version assertion and an early return in 21c8de9 so the summary check stays typed without a second version assertion.

Generated-by: Grok

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for folding the E2E candidate into the real main — the drift was real and the deletion is thorough. I traced the full flag pipeline and checked for leftovers.

Conclusion: PASS. One P2 (suggest a launcher-level test), optional P3 nits.

Verified:

  • desktop-e2e-execution-candidate-main has zero references left (src, package.json exports, docs, workflows).
  • The flag pipeline (boot → desktop-candidate → connect-or-spawn → launcher → candidate-cli → main) is complete, and every export of the new desktop-e2e-execution.ts has a consumer.
  • The flag-based approach is the right call over an env var: an env var would silently flip a CLI run into the fake backend whenever the parent shell exports MAKA_E2E; an explicit flag can't.

Findings:

  • P2: launcher.ts:89 appending --desktop-e2e 1 has no unit test — it's the single point where E2E mode becomes a process argument. If it regresses, the E2E candidate silently runs in production mode (real backend, policy bootstrap, real provider calls from the e2e workspace), and the only backstop is a ~5-minute CI suite failing vaguely. Suggest a ~10-line test asserting the args, or explicitly defer with the e2e suite as the backstop.
  • P3: desktop-e2e-execution.test.ts:52if (checkpoint.version !== 2) return; is dead at runtime (the assert.equal above guarantees it); it exists only for TS narrowing. Fine to keep, but worth a comment.
  • P3: the third test in candidate-cli.test.ts:33-45 (rejects 'true') overlaps with the end-to-end coverage of the same path in execution-candidate-main.test.ts; could be trimmed.
  • P3: desktop-e2e-execution.test.ts:53 asserts an exact summary string — implementation detail, acceptable.

AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on ollama-cloud/deepseek-v4-flash, read-only). The subagent checked references on the PR head and compared old/new startup behavior line by line; I verified the zero-reference claim and the launcher flag line myself. Failure-mode predictions are analysis, not observed behavior. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS,无 P0/P1,可合并。删除彻底(旧 candidate 全仓 0 残留),flag 管道完整,新模块导出均有消费者;用显式 flag 而非 env 是正确的(env 会被父 shell 意外继承导致静默切 fake backend)。1 个 P2:launcher.ts:89 追加 --desktop-e2e 1 无单元测试,若回归会静默以生产模式运行 e2e,建议补约 10 行断言或显式延后。P3 可选:测试里死代码的 TS 收窄、重复的 CLI 测试、精确字符串断言。

@Astro-Han
Astro-Han merged commit a79f691 into apache:main Aug 16, 2026
13 checks passed
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.

refactor(runtime-host): fold the desktop E2E candidate main into the real entrypoint

2 participants