Repository navigation
refactor: third-pass sink-file and boundary governance #1404
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 ofbots/dingtalk-bridge.tscopy the same WebSocket gateway lifecycle wholesale (opcodes, heartbeat/seq/session, connect/reconnect/backoff — the backoff constants are byte-identical).bots/simple-bridge.tsadditionally carries dead Feishu/Discord paths superseded by the dedicated bridges and only actually runs Telegram.- Five
apps/desktop/src/main/oauth/*-subscription-service.tsfiles 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 intosubscription-ipc-main.ts(7 same-shape handlers per provider) andoauth-model-connections-main.ts. - Two full 5-field cron parsers exist across packages:
core/plan-reminders.ts(parsePlanReminderCronExpression/cronExpressionMatches, ~L541–656) andruntime/automation-state.ts(expandCronField/normalizeCronExpression/computeNextCronFire/matchesCronField, ~L521–696) — and insideautomation-state.tsthe same field grammar is parsed twice with already-divergent edge handling. - Security-relevant:
main/rive-cli.tsships a private redaction implementation (redactRiveText/redactRiveValue, ~L160–178) parallel to@maka/core's authoritativeredactSecrets, 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 sharedreconnectBackoffMs. Follow-up in the same lane: deletesimple-bridge.ts's dead Feishu/Discord branches and rename ittelegram-bridgeto match its real responsibility. - Desktop subscription-OAuth base — extract a
SubscriptionOAuthServiceBase(or composable helpers) for the five services; extractstartLoopbackAuthCallbackServershared by codex/antigravity; then collapse the per-provider handler blocks insubscription-ipc-main.tsand the repeated needs_reauth blocks inoauth-model-connections-main.ts. 2–3 PRs, one owner. - Cron unification — single cron-expression module in
@maka/core;plan-reminders.tsandruntime/automation-state.tsconsume it; inside it, one field-grammar parser (todayexpandCronFieldvalidates strictly whilematchesCronFieldsilently skips invalid parts). -
main/rive-cli.tsredaction (security) — deleteredactRiveText/redactRiveValue, use@maka/coreredactSecrets(thin deep-recursive wrapper if needed).
packages/runtime
- Compaction helper dedup —
history-compact-checkpoint.ts(~L516–546) andsemantic-compact.ts(~L1515–1578) re-inline primitives their own domain authoritycontext-budget-helpers.tsalready exports (estimateTokens/sha256/nonEmpty/escapeAttribute/uniqueSorted/finiteRatio); theescapeAttributecopy has already diverged (extra>escaping). Also collapse thestableStringifycopies (active-full-compact-facts.ts:558et al.) and the ~9 three-linesha256wrappers. Coordinate with refactor: reuse shared result and hashing helpers #1374 (hash reuse, approved). Caution: the localuniqueSortedvariant keeps empty strings — a contract difference to preserve or consciously resolve. - Sandbox-denial predicate —
isLikelySandboxDenialis verbatim inshell-tools.ts:400andbuiltin-tools.ts:1085, with a third variant inshell-run-tool-result.ts:169; extract one primitive over{stdout, stderr, sandboxed}.
packages/headless
-
canonicalJson— five copies (one already diverged onundefinedhandling); single serializer next tobuildRunManifestFingerprint. -
harbor-cli.tsconnection reading — stop re-parsing workspacellm-connections.json(schema + per-platform path, ~L770–843); route through the shared workspace-path + connection reader /ConnectionStoreauthority.
packages/core
- Text-sanitize pipeline —
foreign-session.tssanitizeForeignText(~L94–106) copiessession-name.ts's pipeline and has drifted; extractsanitizeUnicodeText(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–229et al.); extract one shared interface and makeisTokenUsageFieldsfollow it.
packages/storage
- JSONL base batch — shared
readJsonlLines(path, {strict, tolerateTornTail, decode})(today 5+ hand-rolled readers with silently divergent torn-tail policies); sharedwriteFileAtomic(6+ temp+rename copies;credential-store's hardened variant stays independent by design); collapsesession-store.ts's near-verbatim double message-parse loop (~L787–902); low: reuseisSafeSessionId/assertSafeSessionIdfromsession-storein the other stores.
packages/ui + apps/desktop
-
chat-turn.tsxclipboard feedback —TurnFooterActionshand-rolls the copy-feedback state machine its same-file sibling already gets fromuseClipboardCopyFeedback; 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); callcreateBotConversationSessionfrom both. -
capability-snapshot.ts— the macOS TCC/OS permission-probing subsystem (~L347–459) is an independently evolving concern; split intopermission-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;
sleepcopies in bots. - ui + cli + desktop: extract
DeepResearchProgressPanelfromchat-view.tsx(and re-point the desktop source-regex test at behavior); hoist the motion-collapse predicate thatreadStreamSnap/useStreamSnapduplicate fromscroll-motion-policy.ts; mergesmooth-stream.ts'sresolveLiveBacklogCps/resolveCompletionCps; collapse CLIruntime-bootstrap.ts's 4 verbatim subscription-cloak modelFetch blocks into one helper; movemain-window.ts's embedded smoke-diagnostic (~L502–585) to its own file; dedupeoffice-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 test: replace source-regex extraction contracts with behavioral assertions #1403.
- A large
session-manager.tschild-orchestration dedup (feat(runtime): migrate agent tools to child sessions #1385 follow-up: four copied child-run drive loops, three copied resume-lineage validations) is confirmed but will be filed separately once refactor(runtime): extract session branching into session-branch.ts #1253 lands, to avoid churn in the same file. - Everything already tracked under refactor: second-pass sink-file and boundary governance #1084 and its sub-issues.
References
- refactor: split sink files before visual polish #363 (first pass) → refactor: second-pass sink-file and boundary governance #1084 (second pass) → this issue.
- refactor: reuse shared result and hashing helpers #1374 (in-flight hash/Result reuse, coordinate), refactor(runtime): extract session branching into session-branch.ts #1253 (session-manager branch extraction, in flight), test: replace source-regex extraction contracts with behavioral assertions #1403 (extraction-contract test governance).
Activity
chinawch007 commentedon Jul 23, 2026
Hi, I’d like to take the "Sandbox-denial predicate" slice.
My plan is: extract one shared isLikelySandboxDenial({ stdout, stderr, sandboxed }) primitive (plus the regex it shares) into a new packages/runtime/src/sandbox/detect.ts, re-exported from sandbox/index.ts. The three copies differ only in input shape: two are semantically identical (same regex, same text shape, same boolean result — the 'sandboxed' in result guard in shell-tools.ts is just TS narrowing because BoundedShellResult lacks the field), and the shell-run-tool-result.ts variant additionally handles a PTY branch and checks sandboxExecution.enforced separately via its sandboxDenialForRecord wrapper. I'll lift the text-flattening (pipes-vs-pty) into the wrapper and pass a { stdout, stderr, sandboxed } triple into the new primitive, preserving the original behavior.
Would it be okay for me to work on this?
chinawch007 commentedon Jul 24, 2026
Implemented the sandbox-denial predicate slice in PR #1438 : deduped the three isLikelySandboxDenial copies into one {stdout, stderr, sandboxed} primitive in sandbox/detect.ts; each call site keeps its own input shaping (including the pty flattening in the variant), regex and behavior unchanged.
Also a heads-up: this is my first contribution to the repo, so the CI workflows on the PR are awaiting maintainer approval to run. Thanks!
test(runtime): add direct tests for sandbox-denial predicate and PTY …
refactor(runtime): dedupe isLikelySandboxDenial into sandbox/detect l… (
me2seeks commentedon Aug 3, 2026
I'd like to take the Cron unification slice.
I re-audited current
main@6a4b4344:packages/core/src/plan-reminders.tsandpackages/runtime/src/automation-state.tsstill own independent five-field cron parsers, and there is no open PR touching either file.My implementation boundary will be:
@maka/core, with parsing, matching, and next-occurrence calculation consuming the same compiled representation;0/7, Vixie day-of-month/day-of-week OR semantics, impossible dates, local-time DST behavior, malformed input, and the bounded search limit;mainand no Runtime Host or scheduler ownership changes.I'll keep the commits reviewable: shared Core contract and tests first, followed by the two consumer migrations.
chinawch007 commentedon Aug 3, 2026
I'd like to take the canonicalJson slice. Plan:
Promote ab-manifest.ts's copy as the single source — export function canonicalJson, unchanged. Replace the 4 identical copies + divergent one with imports. Collapse hand-rolled sha256:...update(canonicalJson(...)) expressions into buildRunManifestFingerprint.
Update the 3 test fixtures to import the authoritative copy.
harness-oracle-registry.ts:534 is missing the .filter(([, v]) => v !== undefined), producing invalid JSON on {b: undefined}. Traced all 6 call sites — none can receive an undefined field (literals / JSON.parse / null-not-undefined), so the fix is behavior-neutral today and removes a latent footgun.
refactor(headless): unify canonicalJson into single serializer (apach…
refactor(headless): unify canonicalJson into single serializer (apach…
chinawch007 commentedon Aug 3, 2026
Implemented task in #2005.
Consolidated all six canonicalJson copies in @maka/headless into a single exported implementation in ab-manifest.ts, next to buildRunManifestFingerprint. Fixed the divergent copy in harness-oracle-registry.ts (missing undefined filtering — verified behavior-neutral across all call sites) and collapsed the hand-rolled sha256:...update(canonicalJson(...)) expressions into buildRunManifestFingerprint. Added behavior tests pinning the determinism invariants.
docs(headless): clarify canonicalJson scope + add registry divergence…
refactor(headless): unify canonicalJson into single serializer (apach…
docs(headless): clarify canonicalJson scope + add registry divergence…
chinawch007 commentedon Aug 4, 2026
Forgot claiming and just completed the task...😓
Implemented the harbor-cli.ts slice in #2102 . harbor-cli.ts was re-deriving both the per-platform workspace path and the llm-connections.json schema/parse, and both copies had drifted from their authorities.
Two changes:
Moved the workspace-path authority (resolveMakaWorkspaceRoot) into @maka/storage, next to createConnectionStore, and made resolveDefaultConnectionsPath delegate to it (also fixes a latent win32 path-separator bug).
Routed applyConnectionDefaults through ConnectionStore (getDefault/get) instead of its inline JSON.parse + hand-written schema, dropping the redundant normalizeProviderType patch the old code needed to paper over the drift.
8 remaining items
refactor(headless): route applyConnectionDefaults through ConnectionS…
refactor(headless): route Harbor CLI connection reading through share…
chinawch007 commentedon Aug 16, 2026
Implemented slice Bot gateway bridge base in #3076
Extracted a shared
WsBridgeBase(connect, close policy, reconnect backoff, stop — including the sharedreconnectBackoffMs) with aGatewayBridgeBaseon top (opcodes, heartbeat/seq/session, identify/resume); QQ and Discord now override only the auth scheme, identify/resume payloads, dispatch event names, and send routes. DingTalk's Stream protocol has no opcode/heartbeat machinery, so it extends the WS base directly instead of the gateway layer. Follow-up done too:simple-bridge.ts's dead Feishu/Discord branches are removed and the file is renamedtelegram-bridge.ts.99Gaoxiaoqi commentedon Sep 6, 2026
I am taking only the
chat-turn.tsxclipboard-feedback slice of #1404. The change reusesuseClipboardCopyFeedbackinTurnFooterActions, preserving raw-text copying, pending-write deduplication, success/failure feedback, the 1400 ms reset, and unmount cleanup. It does not claim the other slices in this tracking issue.Automated message by OpenAI Codex on behalf of @99Gaoxiaoqi.
refactor(ui): reuse clipboard feedback for turn footer (#4891)
seekskyworld commentedon Sep 6, 2026
认领 Rive CLI 脱敏切片,具体范围与验收已拆到 #4925;只修改 rive-cli.ts / 相关测试,不占用其他成员的文件范围。
seekskyworld commentedon Sep 6, 2026
另认领存储 safe-ID 一致性切片,已拆为新 issue #4926;避开 agent-run-store、message-admission-store 及其现有 PR。
HuYellow commentedon Sep 15, 2026
I am taking only the remaining compaction-helper deduplication slice: reuse the existing context-budget SHA-256 helper in ai-sdk-compaction and history-compact-checkpoint, and share the identical non-empty-string guard used by checkpoint validation and the Codex history compactor. I verified these copies still exist on main at dd15b63; #1374 is merged. Serializer, escaping, and token-estimation policies with different edge semantics will stay unchanged. This does not claim the other #1404 slices.
Automated message by OpenAI Codex on behalf of @HuYellow.
refactor(runtime): reuse shared compaction helpers (#5362)
github-actions commentedon Oct 16, 2026
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
mainand add any information that would help move it forward. Assigned issues and issues labelledpinnedare exempt from this policy.