Skip to content

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

Closed
10 of 10 issues completed
Closed

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 desktop — main.ts, preload, desktop+ui mechanical batch (batch last; it shares app-shell.tsx import-line churn with nothing else).
  • Lane runtime/compaction — context-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/other — tool-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 headless — harbor-cell.ts, prompt-candidate-loop.ts, task-agent-controller.ts, fixed-prompt-controller.ts: disjoint.
  • Lane core/storage — settings.ts, settings-store.ts, local-memory.ts, core+runtime mechanical batch.
  • Lane cli/cua — pi-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.

Activity

  1. Nyvo-io commented on Jul 18, 2026

    @Nyvo-io
    Contributor

    I would like to claim the apps/desktop/src/preload/preload.ts bridge-contract slice from #1084.

    I will first audit the current window.maka declarations, the object exposed through contextBridge, every preload consumer, and the browser-API attachment encoder that currently crosses the preload/main dependency boundary.

    Planned scope:

    • define the window.maka bridge shape once in a preload-safe shared module;
    • make the runtime object in preload.ts satisfy that contract and link global.d.ts to the same type;
    • relocate the browser attachment encoder out of src/main/ without changing its wire format or IPC channel behavior;
    • preserve existing imports and the public window.maka surface;
    • co-migrate source-contract tests where paths or declaration shapes move.

    This will remain behavior- and visually-neutral. It will not rename IPC channels, widen renderer capabilities, add preload-to-main imports, or redesign the bridge.

    Planned verification:

    • focused preload/IPC contract tests;
    • Desktop build, typecheck, and test suite;
    • repository lint and git diff --check;
    • explicit comparison of the exposed runtime keys against the shared contract.

    I will wait for maintainer confirmation of this slice before creating a branch or starting implementation.

  2. Nyvo-io commented on Jul 18, 2026

    @Nyvo-io
    Contributor

    Implemented the claimed preload bridge-contract slice in PR #1204: #1204

    The PR centralizes the MakaBridge contract under src/preload, makes the exposed bridge satisfy it, moves attachment encoding out of src/main, and adds dedicated preload typechecking. IPC channels and attachment wire format are unchanged.

  3. xuruiray commented on Jul 18, 2026

    @xuruiray
    Contributor

    Implemented the tool-runtime.ts provider-error classification slice in PR #1214: #1214

    It extracts the taxonomy and evidence normalization into a dedicated dependency-free module, preserves existing runtime behavior, and adds focused classification and boundary tests. Typecheck, test, and e2e CI checks are green.

  4. Astro-Han commented on Jul 19, 2026

    @Astro-Han
    ContributorAuthor

    I'd like to claim the context-budget.ts split from the runtime/compaction lane, and the following ai-sdk-backend.ts orchestrator extraction as the same owner (per the lane's "sequence them under one owner to avoid import churn" note).

    PR1 — context-budget.ts: extract the synthesis-cache subsystem into synthesis-cache.ts (pairing the existing synthesis-cache-artifacts.ts) and the history-compact block engine into history-compact.ts (pairing history-compact-checkpoint.ts / compaction-boundary.ts); move the cross-block shared pure helpers into a context-budget-helpers.ts leaf; keep budget shaping + orchestration hooks in context-budget.ts as the compatibility re-export surface. Behavior- and visually-neutral; public imports preserved via re-exports; no reverse imports from new leaves; co-migrate source-reading contract tests.

    PR2 (after PR1 lands) — ai-sdk-backend.ts: extract the ~1800-line history-compaction/context-budget orchestrator (build*PrepareStep family, mid-turn compaction, overflow recovery, compact/synthesis-cache load/write) into a dedicated collaborator; AiSdkBackend keeps only streamText↔event/permission adaptation.

    Will wait for maintainer confirmation before opening a branch.

  5. 26 remaining items

  6. added theissue type on Aug 29, 2026
  7. added
    staleNo qualifying activity within the lifecycle policy window
    on Sep 2, 2026
  8. github-actions commented on Sep 2, 2026

    @github-actions

    This issue has had no human activity for 30 days and has been marked stale. It will be closed in 30 days unless someone comments.

    If the issue is still current, please confirm it against the latest main and add any information that would help move it forward. Assigned issues and issues labelled pinned are exempt from this policy.

  9. added
    trackingTracking or umbrella issue
    and removed
    staleNo qualifying activity within the lifecycle policy window
    on Sep 17, 2026
  10. added
    staleNo qualifying activity within the lifecycle policy window
    on Sep 18, 2026
  11. github-actions commented on Sep 18, 2026

    @github-actions

    This issue has had no human activity for 30 days and has been marked stale. It will be closed in 30 days unless someone comments.

    If the issue is still current, please confirm it against the latest main and add any information that would help move it forward. Assigned issues and issues labelled pinned are exempt from this policy.

  12. github-actions commented on Oct 18, 2026

    @github-actions

    This issue has been closed because it received no human response during the 30-day grace period. This is not a judgement on the value of the report.

    If the issue still applies to the latest main, please comment with updated evidence so a maintainer can reopen it, or open a follow-up issue with the missing information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    staleNo qualifying activity within the lifecycle policy windowtrackingTracking or umbrella issue

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions