refactor(core): break the dependency cycle + extract shared core (WS2)#499
Merged
Conversation
Phase K breaks the 18-cycle dependency cluster (madge-confirmed; back-edge is registry.ts→getHookRegistry) and the core→workflows-plugin boundary violation; Phase D kills 4 server-side duplications; Phase G adds the architecture guards. Finding (8) (gate runtime.config.get/replace) split to its own follow-up PR per decision — it's adapter-API design, not layering/dedup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getHookRegistry + the __bakinHookRegistry globalThis cell move from src/lib/plugin-registry.ts to a dependency-free leaf, @bakin/core/hooks/hook-registry-singleton. Every consumer — the soon-moved exec-tool registry, the plugin loader, the per-request plugin context (which dropped its raw globalThis pokes), and all core modules — imports it from the leaf, not from the loader. This removes the registry.ts → plugin-registry back-edge that anchors the dependency cycle (K2/K5 finish the break). Singleton stays single-homed (globalThis-backed, one owner). Test mocks of getHookRegistry repointed from the plugin-registry module to the leaf. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…/core/exec-tools scripts/lib/registry.ts is the production exec-tool registry (globalThis Map + PluginToolContext builder), not build tooling — moved to src/core/exec-tools/registry.ts. Its 5 production importers (mcp-server, bakin-skill, reload-pipeline, plugin-registry, the exec-tools API route), the 5 scripts/lib self-registering tool peers, and all test importers are repointed; the dedicated test moves to tests/core/exec-tools/. Combined with K1, this collapses the dependency cycle from 18 → 6: the entire scripts/lib/registry cluster (13 cycles) is gone. Relocating the 5 tool peers out of scripts/ entirely is a noted WS7-tooling follow-up. No behavior change (exec-tool registration unchanged — mcp-server tests green). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plugin loader's buildContext and the per-request catch-all's buildCtx were two copies of the same context builder that had drifted: the per-request updateSettings silently skipped the onSettingsChange notification. Both now delegate to one buildPluginContext factory (src/lib/plugin-context-factory.ts) that owns the shared dynamic surfaces (storage/runtime/tasks/assets/settings/activity/search/hooks + the permission wrap); callers pass the registration behaviour (real registrars at activate-time, noopRegistrars per-request) plus the per-context knobs (skipFileBackedWiring, auditSource, scoped logger, onSettingsChange). Convergence fix: the per-request path now fires onSettingsChange too. New unit test locks it. activate-time audit call shape preserved (channel arg omitted when unset). No other behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…he last runtime cycle) The remaining dependency cycle was closed by workflow-skill-drift.ts importing getPluginSkills from the plugin loader. The plugin-skill map + getPluginSkills/removePluginSkillsByPlugin/clearPluginSkills move to a dependency-free leaf (@bakin/core/skills/plugin-skill-registry, globalThis-backed like the hook singleton); the workflows plugin's skill consumers and the reload pipeline import it from there, and plugin-registry re-exports it for the existing surface. madge now reports 18 → 4 cycles: both WS2-targeted runtime clusters (scripts/lib registry + workflow registries) are gone; the 4 left are pre-existing type-only cycles (core docs/plugin-types and the WS1 SDK split). The workflow-registry boundary move (core importing plugins/workflows internals) is deferred to WS6, where the workflows-plugin split happens — they're intertwined. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plugin-settings JSON read (parse-with-empty-fallback) and write (mkdir + writeFile) mechanics were hand-rolled in 5 places. They now go through packages/core/src/plugins/settings-store (readPluginSettings/writePluginSettings/mergePluginSettings): the unified PluginContext factory, the plugin-settings + agents REST routes, and the team plugin all use it. Change-notification policy stays with each caller (the ctx fires onSettingsChange; the REST route notifies the registry + SSE) — only the storage mechanics are shared. Caught + fixed a latent bug in the K3 factory along the way: writing the merge behind `onSettingsChange?.(merge(...))` short-circuited the persist when no notifier was set (harmless in production — both paths set one — but wrong). Persist now runs unconditionally, then notifies. New factory test covers it. Tests gained the packages/core content-dir mock the store requires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rolled JSON writers
atomicWriteJson moves from install-core to a neutral home,
packages/core/src/storage/atomic-write (with a writeTextAtomic sibling and
a trailingNewline option); install-core re-exports it for its 4 original
callers. The hand-rolled tmp-file+rename JSON writers in
memory/offsets, assets/manifest, workflows/approval-store, and
models/models-cache now use it ({ trailingNewline: false } preserves their
exact output — none wrote a trailing newline). Non-JSON atomic writers
(log rotation, markdown/scoped storage, secret-store, self-update) are
untouched. No behavior change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…residue) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… WS2 The exec-tool registry moved scripts/lib → src/core/exec-tools; the hook singleton + getHookRegistry moved to the dependency-free leaf hook-registry-singleton. CLAUDE.md, plugin-system.md, and plugin-lifecycle.md updated (incl. the now-correct getHookRegistry import example). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ---\n…\n--- split regex was copy-pasted into 10 files, parseSkillFile verbatim into 3, and the line-based lesson parser into 4. packages/core/src/format/frontmatter now owns all three reads: splitFrontmatter (raw split), parseFrontmatter (YAML, lenient — replaces the 3 parseSkillFile copies), and parseLessonFrontmatter (title/ defaultEnabled/tags line parser — replaces the lesson copies in lesson-files/integrity/retrieval/toggle, team/index, package-integrity). The memory tier's parseSkillFile (a different MemoryRow chunker) and the workflows health-check's variant regex are left as-is. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 3-line HealthCheckResult constructors were copy-pasted into 13 files with two divergent signatures: (check, message, autoFixable?) in the six plugin health-checks, and (message) with a hardcoded check name in the seven system-checks. healthOk/healthWarn/healthError/healthFixed now live in @makinbakin/sdk/utils (pure; HealthCheckResult is single-homed in the SDK). The plugin files alias-import them (call sites unchanged); the system-checks' fixed-name (message) wrappers now delegate to them (preserving their ergonomics). No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
packages/sdk/src was missing from SCAN_ROOTS, so the published SDK was never checked against the no-@antfly/no-openclaw-internals/etc. rules. Added it; the SDK is clean. (The cross-plugin + core→plugin import guards land in WS6, once K5-boundary + K6 fix the remaining violations.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
markhayden
changed the title
main advanced past WS2's branch point (WS3 #501 usePluginEvent, #500 models cost). Conflicts + semantic fixes resolved: - tasks/plan.md + tasks/todo.md: textual conflicts (each workstream rewrites these). Archived WS2's as tasks/{plan,todo}-ws2-core-extractions.md (matches the plan-ws1-contract-types.md convention); kept main's active WS3 plan/todo. - #500 added two NEW getHookRegistry consumers on the OLD import path that WS2's K1 moved to the leaf module: plugins/health/lib/system-checks/budget.ts and src/core/agent-cost.ts → repointed both to @bakin/core/hooks/hook-registry-singleton (relative for the plugin file). - #500's tests (agent-cost, budget-gate, health/budget) mocked getHookRegistry only on the legacy facade; added the leaf mock (K1 partial-mock sweep) so all three exercise the real import site. Verified: bun run typecheck clean; bun run test 5072 pass / 0 fail; madge shows 6 type-only cycles (the 4 WS2 documented + 2 docs/ cycles inherited from main, all erased at compile) — none route through scripts/lib/registry, so WS2's runtime cycle break holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WS2 of the audit refactor — moves runtime infrastructure to its correct layer and kills server-side duplication. No behavior change (one internal convergence + one latent bug fix, both test-covered). Verified end-to-end on the dockerized OpenClaw rig.
Phase K — break the dependency cycle (keystone)
madge --circular: 18 → 4 cycles; both runtime clusters the audit flagged are gone (the 4 left are pre-existing type-only cycles — 3 core docs/types, 1 from the WS1 SDK split).getHookRegistry→ dependency-free leafscripts/lib/registry.ts → src/core/exec-tools/(collapses the 13-cycle cluster)PluginContextbuilders into onebuildPluginContextfactory (converged the per-requestonSettingsChangedrift)Phase D — kill server-side duplication
settings-storefor plugin-settings read/write (5 copies → 1). Caught + fixed a latent persist bug in the K3 factory.atomicWriteJsonpromoted tostorage/atomic-write(+writeTextAtomic,trailingNewline); 4 hand-rolled JSON writers deduped.format/frontmattermodule —splitFrontmatter/parseFrontmatter/parseLessonFrontmatter(regex ×10,parseSkillFile×3, lesson parser ×6 → 1 each).healthOk/healthWarn/healthError/healthFixedin the SDK (13 copies, two divergent signatures → one).Phase G
packages/sdk/srcadded to the architecture-test scan roots (SDK is clean).Scope deferred (recorded in tasks/plan.md)
runtime.config.get/replace) → its own follow-up (adapter-API design).Verification
bun run test(4,998 pass / 0 fail) +typecheck+lintgreen on every commitmadge --circular: 18 → 4 (runtime clusters eliminated)bun run build(3 binaries)🤖 Generated with Claude Code