You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
- Moved host operations out of Electron main: `apps/code/src/main/services/focus/sync-service.ts` deleted; git/worktree/watch logic now lives in `packages/workspace-server/src/services/focus/{service,sync-service}.ts` behind one-line `focus.*` procedures in `packages/workspace-server/src/trpc.ts`.
35
+
- Moved orchestration out of the renderer: `apps/code/src/renderer/stores/sagas/focusSagas.ts` deleted; multi-step enable/disable/restore flow now lives in `packages/core/src/focus/service.ts` as `FocusController`, with dependencies injected as a pure interface.
36
+
- Renderer stays thin: `apps/code/src/renderer/stores/focusStore.ts` is now UI state plus one controller call per action. It adapts existing tRPC calls into the core dependency interface and no longer owns the flow graph.
37
+
- Main is a bridge, not the source of truth for focus logic: `apps/code/src/main/services/focus/service.ts` now persists the local session snapshot for Electron restarts, forwards mutations/queries to workspace-server through `WorkspaceClient`, and re-emits focus events to legacy main-router subscribers.
38
+
- Bridge retirement: delete the main `FocusService` shim and move persisted focus-session storage out of Electron once session restore/event subscribers can read directly from workspace-server (or the eventual shared persistence layer). At that point the main `focus` router can disappear with the bridge.
39
+
- Left as-is: restore still re-saves the validated session before starting workspace-server watchers so the server-side in-memory session map is repopulated after app restart. That is intentional coexistence glue, not the final architecture.
Copy file name to clipboardExpand all lines: REFACTOR.md
+77-5Lines changed: 77 additions & 5 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -92,6 +92,10 @@ The desktop **main process is not the home of business logic anymore.** It does
92
92
-**Source-smoothing belongs with the source, not in core.** Debouncing a noisy event stream, dedup, bulk-threshold throttling, filtering source-specific noise (irrelevant git dir events, etc.) — these are properties of the *event source*, not domain decisions. They live in the workspace-server procedure that owns the source, so every client gets the smoothed stream for free. Don't put them in core just because they look like "orchestration."
93
93
-**Hooks are pure react-query idioms.**`useQuery`, `useMutation`, `useSubscription` over a tRPC procedure — that's the whole hook. No `useEffect` constructing services. No `for-await` over async iterables in a hook body. No imperative subscribe/unsubscribe ceremony with a wrappers map. If you find yourself reaching for those, the orchestration is in the wrong place — push it to wherever the tRPC procedure lives (typically workspace-server) and the hook collapses to 5 lines.
94
94
-**`useState`, `useRef`, `useEffect` in a hook are usually a smell.** They mean the hook is holding application state or subscription bookkeeping that should live elsewhere — react-query's cache, a Zustand store, a workspace-server procedure, or just derivation from existing query data. The legitimate uses are narrow: `useRef` for DOM refs (focus, scroll, measurement), `useEffect` for synchronizing imperative browser APIs (event listeners on `window`, `ResizeObserver`, etc.). Anything else — caching a previous value, holding a subscription handle, stashing a callback ref to avoid re-renders, building a wrappers map — means the hook is doing work that belongs upstream.
95
+
-**Try framework primitives before reaching for core.** Before extracting a forbidden pattern into a new core module, ask: does react-query / tRPC / Zustand already do this? `useMutation` dedups by mutation key. `useQuery` dedups by query key. `useSubscription` handles lifecycle. tRPC subscriptions invalidate caches. Most "I need a state machine for this" cases dissolve into a single mutation + its `onSuccess`. **Delete the forbidden pattern and use the framework primitive** is the first move. Only reach for a core module when you can't express the orchestration as a mutation/query/subscription — typically a Saga (multi-step with rollback), a long-running protocol (OAuth dance with redirects), or coordination that crosses multiple queries with invariants.
96
+
-**Smallest change first.** Try deleting the offending code before introducing a new abstraction. Try moving side effects into an existing `onSuccess` before writing an event bus. Try inlining at the call site before extracting a helper. The refactor PR should land *less* code than it deletes whenever possible. If your change adds a net new package, a new singleton, or a new abstraction layer, justify the line count.
97
+
-**Validate the app actually runs.** Typecheck and tests pass on incomplete work all the time. For any user-visible change, open the app and exercise the feature. For background changes, watch logs through one real usage cycle. CI green ≠ feature works.
98
+
-**Some main services stay in main forever.** Single-instance lock, window manager, deep-link router, crash reporter, auto-updater, app-lifecycle, anything that *is* the Electron shell. Don't try to migrate these. Mark them explicitly as "host-only" in code comments or a service-categorization doc so nobody wastes time auditing them for a slice.
95
99
96
100
## Comment markers
97
101
@@ -115,9 +119,12 @@ Use these consistently. Grep targets matter — follow-up passes hunt for each m
| Renderer-consumed host capability (auth, notifications, integrations — anything in main that the renderer needs to query/mutate via electron-trpc) |`packages/platform/src/<capability>.ts` interface + `apps/code/src/renderer/platform-adapters/<capability>.ts` adapter that wraps `trpcClient.X.*`|
118
123
119
124
If the migrated feature is pure data-piping (server → useQuery → component), there's no row to core — that's expected, not a missed step.
120
125
126
+
**Platform adapters apply in both directions.** The existing 15 interfaces in `packages/platform/src/` are all main-process-consumed (main service calls `IClipboard.write`). The same pattern works for renderer-consumed capabilities: interface in `packages/platform/`, adapter in `apps/<host>/src/<process>/platform-adapters/`, ui/core consume via the interface. This is the path for features that live in main and need to be reachable from ui — there's no separate "electron-trpc-client" package needed; the adapter IS the bridge.
127
+
121
128
---
122
129
123
130
## Per-feature procedure
@@ -139,6 +146,64 @@ Do these in order. One feature at a time.
139
146
140
147
---
141
148
149
+
## Canonical shape for features with real orchestration
150
+
151
+
When a feature genuinely needs core (multi-step Saga, OAuth dance, cross-query invariants — not just "we already had a forbidden pattern there"), use this shape. The **focus** port is the worked example.
// ... thin actions: call controller, set state from result
178
+
}));
179
+
```
180
+
181
+
**Why this shape:**
182
+
183
+
-**Controller is stateless.** It orchestrates. Domain state lives where react can render it (store / react-query cache). The controller never holds `this.session` or `this.user` — those would be a second source of truth.
184
+
-**Module-scope `new Controller(...)` is fine** because the controller is stateless and its deps are trpc-bound (which is also a singleton). The forbidden "store owning a singleton with state" pattern doesn't apply.
185
+
-**Deps are feature-scoped, defined in core.** Not a global platform interface, not a re-export from the trpc client. ~20-30 narrow methods the controller actually uses. The renderer adapter is dumb one-line wraps over `trpcClient.X`.
186
+
-**Store actions are call-controller-then-set.** No multi-step flow in the store. No `let inFlight` dedup. No cross-store reach-ins (those move to the controller, or to mutation `onSuccess` if simple).
187
+
-**No event bus.** State changes via store updates after each action returns. React-query consumers react via cache invalidation (the store action can invalidate after success).
188
+
189
+
**When this shape applies:**
190
+
191
+
The feature has at least one of:
192
+
- A Saga (multi-step with rollback) — e.g., focus enable: stash, checkout, save session, on failure unstash and restore
193
+
- A long-running protocol — OAuth dance with redirects, multi-round handshake
194
+
- An invariant that spans multiple queries — e.g., "if A is true, B must also be refreshed"
195
+
- A state machine genuinely complex enough that expressing it as one mutation `onSuccess` is hostile
196
+
197
+
If none of those apply — if the orchestration is "call endpoint, set state from result" — the feature **doesn't need core**. Use `useMutation`/`useQuery` directly. Don't invent a controller for symmetry.
198
+
199
+
**When this shape does NOT apply:**
200
+
201
+
- Pure data-piping (server query → useQuery → render). No core. The hook is 5 lines of `useQuery` over the tRPC procedure.
202
+
- Source-smoothing (debounce, dedup of noisy events). Goes in the workspace-server procedure that owns the source, not in core.
203
+
- Plain auth state that's already served by `trpc.X.getState`. React-query's cache IS the state. Don't shadow it with a stateful core class.
204
+
205
+
---
206
+
142
207
## Coexistence and bridges
143
208
144
209
This codebase is heavily inter-coupled — most main-process services consume events from, or call methods on, other main-process services. A pure "one feature, one slice, delete the old" port is the exception, not the rule. Expect coexistence; design for it.
@@ -221,13 +286,20 @@ If you find debt that isn't a forbidden pattern and isn't a layering fix, **leav
221
286
222
287
## Recommended order
223
288
224
-
1.**Read-only, no subscriptions.**Done — diff-stats.
225
-
2.**Read-only, subscription-based** — done. file-watcher proved the SSE streaming transport (workspace-client `splitLink` + `httpSubscriptionLink`, hono server accepting `?secret=` query).
226
-
3.**Auth / api-client-adjacent.**Exercises the api-client path end-to-end. Next.
227
-
4.**Write paths**(focus mode, worktree ops).
289
+
1.**Read-only, no subscriptions**— done. diff-stats.
290
+
2.**Read-only, subscription-based** — done. file-watcher proved the SSE streaming transport (workspace-client `splitLink` + `httpSubscriptionLink`, hono server accepting `?secret=` query). Source-smoothing lives in workspace-server, hook is pure `useSubscription`.
291
+
3.**Write paths with Saga orchestration**— done. focus proved the [canonical core-bearing shape](#canonical-shape-for-features-with-real-orchestration): stateless `FocusController` in core with feature-scoped deps interface, thin store wraps `trpcClient.X.*` as deps adapter, store actions call controller and set state from result. This is the reference for any future feature that genuinely needs core.
292
+
4.**Renderer-side platform adapter**— next. Auth or notifications. Establishes the pattern for the ~25 host-capability services to follow: `packages/platform/src/<cap>.ts` interface + `apps/code/src/renderer/platform-adapters/<cap>.ts` adapter wrapping `trpcClient.X.*` + ui consumes via context. Unlocks the bulk of the remaining main services.
228
293
5.**Terminal / pty proxying.** Most ambitious. Tests the full pipeline including binary data.
229
294
230
-
The first two slices also surfaced two recurring patterns now baked into the ground rules: source-smoothing belongs with the source (not core), and hooks are pure react-query idioms (not useEffect wrappers). Apply them on every slice going forward.
295
+
Patterns now baked into the ground rules from prior slices:
296
+
- Source-smoothing belongs with the source (not core) — file-watcher.
297
+
- Hooks are pure react-query idioms — file-watcher.
298
+
- Stateless controller + thin store + dumb deps adapter for features that need core — focus.
299
+
- Try framework primitives before reaching for core; most "I need a state machine" cases dissolve into `useMutation` + `onSuccess`.
300
+
- Platform adapters apply in both directions; the existing 15 are main-consumed, the next ones are renderer-consumed.
0 commit comments