refactor(desktop): drop unused bridge endpoints and the e2e starter - #3102
Conversation
Remove Desktop preload, contract, and main IPC for memory proposal/entry CRUD, artifacts:get, skills:details, skills.createStarter, onboarding:clearMilestone, mcp:reconnect, and git-review:mutate. None of those had a renderer, e2e, or test consumer. The skill draft e2e now opens the Skills module for the fixture-seeded workspace skill instead of calling createStarter. Fixes apache#3090 Generated-by: Grok
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughSummaryThis PR removes unused Desktop bridge and IPC endpoints with no renderer, E2E, or test consumers. It removes memory proposal and entry mutations, The draft-lifecycle E2E test now uses the installed Design assessment
Risks and validationThe main risk is an accidental caller of a removed bridge method. The E2E changes also require validation that the real Skills UI preserves draft-lifecycle coverage. Reported validation passed:
The full Desktop E2E suite was not run. The repository search did not provide direct evidence for these result claims. Required-check status therefore remains unverified. Review-relevant risksThe change removes public preload bridge and bridge-contract APIs, including memory mutations, artifact lookup, skill details and starter creation, onboarding milestone clearing, MCP reconnect, and Git review mutation. Any material public-contract change requires independent human review under repository policy. The change moves the E2E path from No protected-area effect was identified for licensing, releases, security, or governance in the current diff. The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughDesktop removes unused IPC endpoints across main, preload, and bridge contracts. It updates MCP fixtures and migrates skill lifecycle and slash-command E2E tests. ChangesDesktop endpoint cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR removes unused desktop bridge and IPC endpoints while updating the affected end-to-end flow; the reported checks pass, and no actionable merge-blocking risk remains after normal review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed the deletion of the unused Desktop bridge endpoints and the e2e starter.
The removed channels have no remaining renderer, e2e, or test consumers on the PR branch. The two skill e2e cases now use the seeded invocable catalog instead of skills.createStarter, which is the product path. Desktop-side deletion looks complete.
Non-blocking follow-ups, not in this PR's boundary: mutateGitReview is now test-only in the same package; runtime-host still has create_starter and the memory proposal mutations with no Desktop client.
Approve.
AI-assisted review: Grok 4.6 and opencode-go/deepseek-v4-flash:max each reviewed the PR independently. A third pass on ark-coding-plan/glm-5.3 hit a quota limit and did not finish. I grepped the deleted symbols on the PR branch and checked the rewritten e2e specs. Unverified by me: I did not rerun desktop e2e locally.
|
Fast-path merge This change qualifies for the self-merge fast path: it is low impact and easy to reverse (drops Desktop bridge/IPC endpoints with no renderer, e2e, or test consumers, plus the e2e-only Human contributor @yuhan reviewed the final diff and chose the fast path. |
Astro-Han
left a comment
There was a problem hiding this comment.
Approving the change direction (removing unused Desktop bridge/IPC endpoints and the e2e-only skills.createStarter).
Note: this PR currently has merge conflicts with main and needs a rebase/merge before it can be merged. Conflicting files:
apps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/main/onboarding-service.tsapps/desktop/src/main/runtime-host-boot.ts
The preload conflicts are the endpoints this PR removes (memory proposal ops, git-review:mutate, onboarding:clearMilestone, artifacts:get, etc.) still present on main; resolve by keeping the removals and retaining main's DesktopTargetScope / getState(sessionId?) changes from #3097.
Keep the live memory getState(sessionId) signature from main. Continue to omit the unused proposal, artifact get, git-review mutate, and onboarding clearMilestone preload endpoints.
522893b to
5f9261c
Compare
After a steering send, fill('/') can land on an unfocused composer so
the command list stays empty. /compact then looks absent and the /side
click times out in CI.
Summary
Six Desktop bridge + IPC surfaces had no renderer, e2e, or test consumers, and
skills.createStarterexisted only forskill-draft-lifecyclee2e.Removed across preload,
bridge-contract, and the matching main handlers:memory:propose/remember/listProposals/approveProposal/rejectProposal/archiveEntry/restoreEntryartifacts:getskills:detailsonboarding:clearMilestonemcp:reconnectgit-review:mutateskills:createStarterLive memory document ops (
getState/save/reset/openFile), artifact list/read/delete, and skills list/enable/pin/delete stay.The draft-lifecycle e2e now opens the Skills module for the fixture-seeded Workspace Only skill instead of calling
createStarter. Disable/enable during a revision still uses the livesetEnabledbridge so the composer is not unmounted.Fixes #3090
Verification
npx biome checkon the 13 changed files — cleannpm --workspace @maka/desktop run typecheck— cleannpm --workspace @maka/desktop test— 862/862playwright test e2e/skill-draft-lifecycle.spec.ts— 2/2Not run: full desktop e2e suite
AI use
Select exactly one:
Tool(s) and scope:
Grok authored the deletion, e2e rewrite, and this PR description.
Checklist
Does this PR entail a change in behavior?