Skip to content

refactor: second-pass sink-file and boundary governance #1084

Description

@Astro-Han

Context

First-pass sink-file governance (#363) split components.tsx, SettingsModal.tsx, ProvidersPanel.tsx, renderer/main.tsx, and main/main.ts, and closed with the instruction to track a fresh second-pass scan in a follow-up issue. This is that issue.

A full-repo scan (38 area/file audit agents + per-finding adversarial verification against current main, calibrated with #363's rules) confirmed ~40 findings. As in #363, the problem is never file length by itself: every finding below is about unrelated responsibilities sharing one module, a broken verification boundary, a parallel implementation that can silently diverge, or a name/location that misleads. Large single-responsibility data/fixture/generated files were explicitly excluded.

Headline evidence

  • apps/desktop/src/main/main.ts regrew from 1663 lines (post refactor(desktop): split main process orchestration #385) to 2412. registerIpc() is ~800 lines inlining ≥8 feature domains, while 11 sibling *-ipc-main.ts files already follow a register*Ipc(deps) pattern — the seam exists but was applied inconsistently. Its own contract test regex-matches raw source text because registerIpc runs as a module side effect and cannot be invoked in isolation: the verification boundary is already broken.
  • packages/headless/src/harbor-cell.ts (1810 lines) fuses session orchestration, a ~600-line env→ContextBudgetPolicy compiler, subprocess/tool-executor plumbing, and credential resolution. runtime-policy-ab-run.ts imports one env-key list from it and drags in node:child_process, runtime backends, and credentials.json reading. 44 commits have touched this file for unrelated reasons.
  • Security-relevant duplication: the filesystem-containment guards (isContainedPath/isSafeSkillId, isInside/toRelative) exist as byte-identical private copies in packages/runtime/src/skills.ts, apps/desktop/src/main/explore-agent-tool.ts, and apps/desktop/src/main/managed-skill-sources.ts. A future fix to the containment check must currently be applied in multiple places.

PR mapping

  • High and medium slices: one file, one PR — each checkbox below is one independently reviewable, independently revertible PR.
  • Low-severity mechanical items: batched into two cleanup PRs (one per area) instead of one PR per one-line change.
  • Every PR follows the worktree/branch convention; flat off main, no stacking.

Order of work

  1. The two high-severity sinks first (main.ts, harbor-cell.ts) plus the security-relevant guard dedup.
  2. Remaining splits grouped by package, in parallel lanes (see Parallelization).
  3. The two mechanical cleanup batches last (or whenever a lane frees up — they conflict with nothing outside their own files).

Invariants

Same as #363: behavior- and visually-neutral; preserve public imports (keep re-export compatibility where a moved symbol is public); no reverse imports from new leaf files; co-migrate source-reading contract tests; flat PRs off main; proportional verification (typecheck/build + targeted contract tests for pure moves).

Slices

High

  • apps/desktop/src/main/main.ts — migrate the un-migrated registerIpc() domains (sessions, onboarding, settings/bots/network, permissions+capabilities+health, quickChat+expertTeam, gateway, workspaceInstructions, git, workspace search) into register*Ipc(deps) modules following the existing 11-file pattern; registerIpc becomes pure composition. The backing logic modules (onboarding-service.ts, quick-chat.ts, expert-team-start.ts, permissions-actions.ts, capability-snapshot.ts) already exist.
  • packages/headless/src/harbor-cell.ts — four-way split: context-budget env compiler → harbor-cell-context-budget-env.ts; tool executors + subprocess helpers → harbor-cell-tool-executor.ts; provider/credential resolution → merge into the existing provider-env.ts seam; harbor-cell.ts keeps orchestration + backend wiring. Also dedupe booleanEnv/positiveIntEnv/numericEnv into headless-run-env.ts (a third private copy lives in harbor-task-runner.ts).
  • Containment-guard dedup (security) — single shared home for isContainedPath/isSafeSkillId (runtime) and isInside/toRelative (desktop); managed-skill-sources.ts and explore-agent-tool.ts import instead of keeping byte-for-byte copies.

packages/runtime

  • context-budget.ts (3001) — extract the history-compact block engine into history-compact-*.ts siblings and the full synthesis-cache subsystem into synthesis-cache.ts (pairing existing synthesis-cache-artifacts.ts); keep only budget shaping + orchestration hooks, with context-budget.ts as the compatibility export surface.
  • ai-sdk-backend.ts (4347) — extract the ~1800-line history-compaction/context-budget orchestrator (the build*PrepareStep family, mid-turn compaction, overflow recovery, compact/synthesis-cache load/write methods) into a dedicated collaborator; AiSdkBackend keeps the streamText↔event/permission adaptation. Land after the context-budget.ts split to avoid import churn.
  • runtime-kernel.ts — extract the history-compact checkpoint cache/CAS-write/cleanup coordinator into a sibling next to history-compact-checkpoint.ts; resolve the private AsyncEventQueue duplicate against async-queue.ts (reuse or rename with documented contract difference).
  • tool-runtime.ts (done: refactor(runtime): extract provider error classification #1214) — extract the ~270-line provider-error classification taxonomy (classifyError, regex tables, evidence normalization) into its own module.
  • session-manager.ts (in flight: refactor(runtime): extract session branching into session-branch.ts #1253, needs rebase after feat(runtime): migrate agent tools to child sessions #1385) — extract branch-session creation + runtime-ledger cloning (~250 lines, own verification boundary) into session-branch.ts; import headerLineage from agent-run-recovery.ts instead of the verbatim copy.
  • agent-run.ts — extract the prior-run context assembly (buildPriorRuntimeContext + repair/backfill helpers) into prior-run-context.ts; move the pure status-projection helpers into session-projection-helpers.ts where their siblings already live.
  • skills.ts (1189) — split along the package's existing convention into discovery / scan / metadata / (remaining) modules, keeping the @maka/runtime public exports stable.
  • active-full-compact.ts (done: refactor(runtime): extract active compact facts #1259) — extract the ~380-line regex fact-extraction engine (summary fields only) into a sibling file.
  • computer-use-tools.ts (1763) — extract the wire-protocol codec (zod schemas, adaptToCuAction, snapshotComputerParams) mirroring openai-computer-codec.ts; move the CuDispatchBackend/CuRunResult/… contract types into a dedicated types module so adapters stop importing the implementation file.

packages/headless

  • prompt-candidate-loop.ts (1007) — extract the git safety/rollback wrapper and the trajectory/tool-failure digest extraction into siblings; dedupe the two hand-rolled "held-in task set hash" implementations (rsi-round-analysis.ts re-implements it and the outputs are compared with strict equality across the WAL boundary).
  • task-agent-controller.ts — extract the permission-intervention decision engine into permission-intervention.ts beside permission-grants.ts.
  • fixed-prompt-controller.ts (in flight: refactor(headless): extract fixed prompt WAL types #1261, needs rebase after feat(headless): add createPierTaskRunner for DeepSWE via Pier #1352) — extract the ~340 lines of WAL event schema types into fixed-prompt-wal-types.ts.

packages/core + storage

  • core/settings.ts (done: refactor(core): split settings into domain owners #1265) — extract the bot-chat channel subsystem (types, defaults, ~250-line readiness state machine) into bot-chat-settings.ts; move the web-search credential-reconciliation logic into web-search.ts; resolve the NetworkSettings name collision with settings/network-settings.ts.
  • storage/settings-store.ts (done: refactor(storage): extract usage stats store #1266) — extract the usage-stats JSONL aggregation engine into a sibling usage-stats-store.ts that SettingsStore delegates to.
  • core/local-memory.ts (blocked on feat(memory): add measurement foundation and scoped read gates #820 — coordinate before starting) — replace the embedded hand-rolled SHA-256 with the same digest-injection seam oauth-subscription.ts uses; collapse the duplicated meta-value sanitizer onto normalizeMetaValue.

apps/desktop + cli + computer-use

  • preload/preload.ts (done: refactor(desktop): centralize preload bridge contract #1204) — define the window.maka bridge shape once and link preload.ts (satisfies) and global.d.ts to it (today they are two hand-maintained declarations with no compiler link); relocate the browser-API attachment encoder out of src/main/ (preload's own header forbids main-process deps).
  • cli/pi-tui-runner.ts — extract the shell-run hydration/buffering state machine into shell-run-hydration.ts.
  • computer-use/cua-driver-backend.ts (2701) — extract the TOCTOU target staleness/occlusion re-verification into cua-driver-target-resolution.ts, mirroring cua-driver-snapshot.ts.

Mechanical cleanup batches (low severity, one PR each)

  • Batch: core + runtime mechanical (in flight: refactor: reuse shared result and hashing helpers #1374, approved) — semantic-compact.ts reuses request-shape.ts's exported stableHash/stableStringify instead of its local sortJson/sha256 trio; move the generic Result<T> helper from core/settings/result.ts to @maka/core/result.
  • Batch: desktop + ui mechanical — rename OnboardingHero.tsx, FirstRunChecklist.tsx, SettingsModal.tsx, ProvidersPanel.tsx to kebab-case; replace app-shell.tsx's hand-rolled pending-set with the useKeyedPendingRegistry hook it already uses four times; move the four authored Markdown skill bodies out of main/skills.ts into content files; extract composer.tsx's self-contained workspace/branch-picker row into composer-workspace-row.tsx (mirroring chat-model-switcher.tsx) (done).

Parallelization

All slices touch disjoint files except two ordered pairs, so the work splits into independent lanes that different contributors can pick up concurrently (one worktree + branch per slice):

  • Lane desktopmain.ts, preload, desktop+ui mechanical batch (batch last; it shares app-shell.tsx import-line churn with nothing else).
  • Lane runtime/compactioncontext-budget.ts then ai-sdk-backend.ts (same feature family; sequence them under one owner to avoid import churn); runtime-kernel.ts is same-family but disjoint files, can go to a second owner.
  • Lane runtime/othertool-runtime.ts, session-manager.ts, agent-run.ts, skills.ts, active-full-compact.ts, computer-use-tools.ts: six disjoint files, up to six owners.
  • Lane headlessharbor-cell.ts, prompt-candidate-loop.ts, task-agent-controller.ts, fixed-prompt-controller.ts: disjoint.
  • Lane core/storagesettings.ts, settings-store.ts, local-memory.ts, core+runtime mechanical batch.
  • Lane cli/cuapi-tui-runner.ts, cua-driver-backend.ts.

Ordering constraints: (1) context-budget.ts split lands before the ai-sdk-backend.ts orchestrator extraction; (2) the containment-guard dedup and the runtime/skills.ts split both involve skills.ts — they compose cleanly as long as the @maka/runtime public exports stay stable, but prefer one owner for both.

Borderline (evidence confirmed, worth-a-PR disputed — decide per slice, default skip)

  • cli/pi-transcript.ts: pure transcript reducer vs ANSI rendering layer separation. Declined after re-verification — the toggle reducers and renderer are bidirectionally coupled through shared render geometry (entryFirstLine/viewportTop); no clean boundary.
  • core/settings.ts UsageRequestLog/UsageSummary vs usage-stats/types.ts type overlap. Moved out of scope — two generations of the usage data model (Settings page v1 vs telemetry v2); needs its own migration issue, not a mechanical dedup.
  • headless/fixed-prompt-controller.ts: second, production-unused Harbor result parser (readHarborTaskRunOutput). Suggested: fold into refactor(headless): extract fixed prompt WAL types #1261 — demote to test-private or delete.
  • headless/ahe-evidence-export.ts: evidence export vs target-snapshot building split. Declined after re-verification — export and target-snapshot building share one result type; split value is low.

Out of scope

Same as #363: visual redesign, breaking public imports, splitting large single-purpose data files for line count, rewriting runtime behavior while moving files, new dependency/import-cycle tooling.

References

Status update (2026-07-23)

A full re-verification against current main confirmed the remaining unclaimed slices are still valid; refreshed scopes now live in per-slice sub-issues (#1405#1414) so they can be claimed individually. In-flight: #1253 and #1261 need a rebase; #1374 is approved. A third-pass full-repo scan found a new set of verified findings, tracked in #1404; source-regex extraction-contract test governance is tracked in #1403.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions