Skip to content

refactor(settings): move workflows out of transport adapters - #595

Merged
ewen-poch merged 5 commits into
mainfrom
refactor/settings-workflows-adapters
Aug 1, 2026
Merged

refactor(settings): move workflows out of transport adapters#595
ewen-poch merged 5 commits into
mainfrom
refactor/settings-workflows-adapters

Conversation

@ewen-poch

Copy link
Copy Markdown
Member

Problem

Settings mutations were persisted by SettingsService, but cross-module follow-up effects still lived in the Electron/Web transport adapter. That made settings/ipc.ts own provider reconnects, framework switches, Skill reloads, Connector permission cleanup, derived Connector Skill documents, and native icon application. The same composition also kept Connector runtime projection state and the shared provider/install id counter close to the facade instead of with explicit owners.

Proposed change

  • Split transport-free Settings workflows into narrow Runtime/Auth, Skills, Connectors, and Appearance owners with required effect ports.
  • Make Electron and local Web route through the same Settings handlers and workflow instances.
  • Move the live Connector snapshot and generated Skill document synchronization into ConnectorRuntimeSettingsProjection.
  • Serialize the complete Connector refresh pipeline so a slow older refresh cannot overwrite newer snapshot/doc state; a failed refresh remains contained and does not poison the queue.
  • Move the process-local monotonic provider/install suffix allocator into its own closure owner.
  • Narrow transitional ACP/window Settings capabilities without changing the underlying repository or public surface.
flowchart LR
  E["Electron IPC"] --> A["Settings transport adapter"]
  W["Local Web RPC"] --> A
  R["Remote Web policy gate"] --> A
  A --> RT["Runtime/Auth workflows"]
  A --> SK["Skill workflows"]
  A --> CO["Connector workflows"]
  A --> AP["Appearance workflows"]
  RT --> S["SettingsService / repository"]
  SK --> S
  CO --> S
  AP --> S
  CO --> P["Serialized Connector runtime projection"]
Loading

Scope and non-goals

  • No IPC channel, preload contract, generated Web API map, Task/SDK/CLI API, persistence schema, data relationship, migration, renderer, or user-interaction change.
  • Preserve the existing capability asymmetry: Specialist remains Electron-only; Permission remains complete on Electron/Web with the existing Task subset; Compute remains complete on Electron/local Web, remote Web still rejects download/reveal, and CLI has no direct Compute management API.
  • Preserve existing success/failure/cancel/stale behavior and await versus fire-and-forget ordering.
  • Keep #458 forward-compatible only; this PR adds no orchestration state, session tree, budget, event, tool, or public method.

Acceptance criteria and validation

All checks below ran after the last material edit.

  • Runtime/Auth, Skill, Connector, Appearance effect ordering; Connector concurrency and failure recovery; Electron/local Web shared routing -> npm test -- --run src/main/settings/workflows.test.ts src/main/settings/ipc.test.ts src/main/connectors/runtime-settings-projection.test.ts src/main/connector-reload.test.ts -> 4 files passed, 77 tests passed.
  • Settings persistence, ACP integration, Connector projection and local HTTP behavior -> npm test -- --run src/main/settings src/main/acp src/main/connectors src/main/connector-reload.test.ts -> 125 files passed, 1 skipped; 2,226 tests passed, 42 skipped.
  • Full application and cross-surface regression coverage -> npm test -> 641 files passed, 15 skipped; 9,446 tests passed, 184 skipped.
  • Node and Web contracts -> npm run typecheck -> passed.
  • Transport allowlist/generated map -> npm run check:web-api-map -> passed.
  • Repository lint -> npm run lint -> 0 errors; 23 unchanged latest-main warnings.
  • Patch integrity -> git diff --check -> passed.
  • Independent Standards and Spec/compatibility reviews -> no P0-P2 findings.

Uncovered risk: there is no new manual UI/E2E run because this PR does not change UI or public interaction. Production effect composition is covered through the real workflow/projection tests, transport registration tests, full suite, and independent wiring review rather than a dedicated src/main/ipc.ts composition integration test; CI E2E remains the final gate.

Review focus

  • Required narrow effect ports and the four workflow ownership boundaries.
  • Connector refresh serialization and the preserved fire-and-forget mutation boundary.
  • Custom-server permission prune/security guard ordering and rollback semantics.
  • Electron/local Web shared handler routing and unchanged remote Web/CLI/Task authority boundaries.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codex Review

Verdict: mergeable

No actionable findings.

Summary: Static inspection found no concrete merge-blocking defect introduced by this pull request. No tests, lint, typecheck, build, or dependency commands were run.

@github-actions github-actions Bot added the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 1, 2026
@ewen-poch
ewen-poch merged commit 88493e7 into main Aug 1, 2026
25 checks passed
@ewen-poch
ewen-poch deleted the refactor/settings-workflows-adapters branch August 1, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge All completed AI reviewers found this pull request mergeable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant