apache / apache/maka

refactor: third-pass sink-file and boundary governance

Open
#1,404 14 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
5.4k
Forks
502
Avg merge
1d 2h
Merged PRs (30d)
715

Description

## Context

Second-pass governance (#1084) is winding down: 10 of its slices are merged and the remaining ones are tracked as sub-issues there. This issue tracks the third pass: ~150 source files ≥350 lines (everything not already covered by #1084) audited against the same four criteria, with per-finding adversarial verification against current `main`. 32 findings survived verification; 18 were refuted — plausible-but-justified coupling was explicitly rejected (e.g. `model-adapter.ts` usage normalization, `permission.ts` shell classification, and `app-shell.tsx` mode management are all fine as-is).

The dominant pattern this round is different from #1084: 25 of 32 findings are **silently-diverging parallel implementations** (copy-paste that must be kept in sync by hand), not mixed responsibilities. Several copies have already drifted.

## Headline evidence

- `bots/qq-bridge.ts`, `bots/discord-bridge.ts`, and most of `bots/dingtalk-bridge.ts` copy the same WebSocket gateway lifecycle wholesale (opcodes, heartbeat/seq/session, connect/reconnect/backoff — the backoff constants are byte-identical). `bots/simple-bridge.ts` additionally carries dead Feishu/Discord paths superseded by the dedicated bridges and only actually runs Telegram.
- Five `apps/desktop/src/main/oauth/*-subscription-service.ts` files copy one service shell (pending/logout/refresh/getAccessToken/failure handling); the codex and antigravity services duplicate the loopback callback server nearly verbatim. The missing base leaks downstream into `subscription-ipc-main.ts` (7 same-shape handlers per provider) and `oauth-model-connections-main.ts`.
- Two full 5-field cron parsers exist across packages: `core/plan-reminders.ts` (`parsePlanReminderCronExpression`/`cronExpressionMatches`, ~L541–656) and `runtime/automation-state.ts` (`expandCronField`/`normalizeCronExpression`/`computeNextCronFire`/`matchesCronField`, ~L521–696) — and inside `automation-state.ts` the same field grammar is parsed twice with already-divergent edge handling.
- Security-relevant: `main/rive-cli.ts` ships a private redaction implementation (`redactRiveText`/`redactRiveValue`, ~L160–178) parallel to `@maka/core`'s authoritative `redactSecrets`, with narrower coverage (misses GitHub/Google/Slack token shapes) — a future hardening of the authoritative rules silently won't apply here.

## Slices

Each High/medium checkbox is one independently reviewable, independently revertible PR unless noted; low-severity items are batched.

### High

- [ ] **Bot gateway bridge base** — extract `GatewayBridgeBase` (opcodes, heartbeat/seq/session, connect/reconnect/backoff, close policy); qq/discord/dingtalk override only auth header, identify/resume payload, dispatch event names, send route; includes the shared `reconnectBackoffMs`. Follow-up in the same lane: delete `simple-bridge.ts`'s dead Feishu/Discord branches and rename it `telegram-bridge` to match its real responsibility.
- [ ] **Desktop subscription-OAuth base** — extract a `SubscriptionOAuthServiceBase` (or composable helpers) for the five services; extract `startLoopbackAuthCallbackServer` shared by codex/antigravity; then collapse the per-provider handler blocks in `subscription-ipc-main.ts` and the repeated needs_reauth blocks in `oauth-model-connections-main.ts`. 2–3 PRs, one owner.
- [ ] **Cron unification** — single cron-expression module in `@maka/core`; `plan-reminders.ts` and `runtime/automation-state.ts` consume it; inside it, one field-grammar parser (today `expandCronField` validates strictly while `matchesCronField` silently skips invalid parts).
- [ ] **`main/rive-cli.ts` redaction (security)** — delete `redactRiveText`/`redactRiveValue`, use `@maka/core` `redactSecrets` (thin deep-recursive wrapper if needed).

### packages/runtime

- [ ] **Compaction helper dedup** — `history-compact-checkpoint.ts` (~L516–546) and `semantic-compact.ts` (~L1515–1578) re-inline primitives their own domain authority `context-budget-helpers.ts` already exports (`estimateTokens`/`sha256`/`nonEmpty`/`escapeAttribute`/`uniqueSorted`/`finiteRatio`); the `escapeAttribute` copy has already diverged (extra `>` escaping). Also collapse the `stableStringify` copies (`active-full-compact-facts.ts:558` et al.) and the ~9 three-line `sha256` wrappers. Coordinate with #1374 (hash reuse, approved). Caution: the local `uniqueSorted` variant keeps empty strings — a contract difference to preserve or consciously resolve.
- [ ] **Sandbox-denial predicate** — `isLikelySandboxDenial` is verbatim in `shell-tools.ts:400` and `builtin-tools.ts:1085`, with a third variant in `shell-run-tool-result.ts:169`; extract one primitive over `{stdout, stderr, sandboxed}`.

### packages/headless

- [ ] **`canonicalJson`** — five copies (one already diverged on `undefined` handling); single serializer next to `buildRunManifestFingerprint`.
- [ ] **`harbor-cli.ts` connection reading** — stop re-parsing workspace `llm-connections.json` (schema + per-platform path, ~L770–843); route through the shared workspace-path + connection reader / `ConnectionStore` authority.

### packages/core

- [ ] **Text-sanitize pipeline** — `foreign-session.ts` `sanitizeForeignText` (~L94–106) copies `session-name.ts`'s pipeline and has drifted; extract `sanitizeUnicodeText(text, {maxCodePoints})`.
- [ ] **`TokenUsageFields`** — the same token-usage field bundle is declared in three places with no shared base and has already drifted (`runtime-event.ts:204–229` et al.); extract one shared interface and make `isTokenUsageFields` follow it.

### packages/storage

- [ ] **JSONL base batch** — shared `readJsonlLines(path, {strict, tolerateTornTail, decode})` (today 5+ hand-rolled readers with silently divergent torn-tail policies); shared `writeFileAtomic` (6+ temp+rename copies; `credential-store`'s hardened variant stays independent by design); collapse `session-store.ts`'s near-verbatim double message-parse loop (~L787–902); low: reuse `isSafeSessionId`/`assertSafeSessionId` from `session-store` in the other stores.

### packages/ui + apps/desktop

- [ ] **`chat-turn.tsx` clipboard feedback** — `TurnFooterActions` hand-rolls the copy-feedback state machine its same-file sibling already gets from `useClipboardCopyFeedback`; converge.
- [ ] **`bot-incoming-main.ts`** — the new-bot-conversation sequence (limit check + rate-limit + getReadyConnection + createSession + set + emit + ensureSend) is copied twice (~L192–219, 266–300); call `createBotConversationSession` from both.
- [ ] **`capability-snapshot.ts`** — the macOS TCC/OS permission-probing subsystem (~L347–459) is an independently evolving concern; split into `permission-snapshot.ts`.

### Mechanical cleanup batches (low, one PR per area)

- [ ] **runtime + headless**: git-exec wrapper (4 copies, drifted); harbor-cli's inline autonomous decision ladder → reuse the exported default policy; `sleep` copies in bots.
- [ ] **ui + cli + desktop**: extract `DeepResearchProgressPanel` from `chat-view.tsx` (and re-point the desktop source-regex test at behavior); hoist the motion-collapse predicate that `readStreamSnap`/`useStreamSnap` duplicate from `scroll-motion-policy.ts`; merge `smooth-stream.ts`'s `resolveLiveBacklogCps`/`resolveCompletionCps`; collapse CLI `runtime-bootstrap.ts`'s 4 verbatim subscription-cloak modelFetch blocks into one helper; move `main-window.ts`'s embedded smoke-diagnostic (~L502–585) to its own file; dedupe `office-document-tool.ts`'s path-validation preamble.

## Invariants

Same as #1084: behavior- and visually-neutral; preserve public imports (re-export compatibility where a moved symbol is public); no reverse imports from new leaf files; co-migrate tests; flat PRs off `main`; proportional verification (typecheck/build + targeted tests for pure moves). Per #1403, extraction PRs should pin their result with typed interface assertions and behavior tests, not new source-regex contracts.

## Parallelization

All slices touch disjoint files except the compaction-helper batch (coordinate with #1374) and the OAuth lane (one owner, 2–3 ordered PRs). Before starting a slice, post a short claim comment here to avoid conflicts.

## Out of scope

- The usage v1 → telemetry-v2 data-model migration (needs its own issue).
- The desktop source-regex behavioral test family — tracked in #1403.
- A large `session-manager.ts` child-orchestration dedup (#1385 follow-up: four copied child-run drive loops, three copied resume-lineage validations) is confirmed but will be filed separately once #1253 lands, to avoid churn in the same file.
- Everything already tracked under #1084 and its sub-issues.

## References

- #363 (first pass) → #1084 (second pass) → this issue.
- #1374 (in-flight hash/Result reuse, coordinate), #1253 (session-manager branch extraction, in flight), #1403 (extraction-contract test governance).

Contributor guide

Open the contributing guide

Research direction

Choose one unchecked slice, then read its named files and the relevant invariants before claiming it; examples include the bot bridge files, OAuth subscription services, cron implementations, or main/rive-cli.ts. Run the targeted tests plus typecheck/build, and consider the slice done when behavior and public imports remain unchanged, duplication is removed, and the result is covered by appropriate behavior or interface checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.