feat: DI container + event bus foundation (Phase 1)

[TonyCasey]

Summary

• Awilix DI container — createContainer() factory replaces manual NotesService → MemoryRepository → Service chains across all 8 entry points (4 CLI commands + 4 MCP tools), removing 72 lines of duplicated wiring
• Domain event types — ISessionStartEvent, ISessionStopEvent, IPromptSubmitEvent with discriminated union HookEvent for future Claude Code hooks integration
• EventBus — infrastructure pub/sub implementation with error isolation (a failing handler doesn't block others) and typed IEventBus, IEventHandler, IEventResult interfaces

Changes

Commit	Description
chore: install awilix	Add awilix ^12.1.0 dependency
feat: domain events	HookEvents types, IEventBus/IEventHandler/IEventResult interfaces
feat: EventBus	Infrastructure EventBus with error isolation and logging
feat: DI container	createContainer() with ICradle type, IContainerOptions, CLASSIC injection mode
refactor: entry points	CLI commands + MCP tools use createContainer() instead of manual instantiation

Test plan

• All 161 existing tests pass (pure refactor — no functional changes)
• npm run type-check passes
• npm run lint passes (pre-existing no-require-imports in cli.ts only)
• Manual smoke test: git mem remember "test" && git mem recall
• Manual smoke test: MCP server tools via Claude Code

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Adds a hook event system for session lifecycle and prompt submissions.
  • Adds a publish/subscribe event bus with handler result reporting.

• Chores

  • Centralizes dependency wiring via a typed DI container; commands/tools now initialize via the container.
  • Optional enrichment and scoped logging are container-driven.
  • Adds a new runtime dependency for the DI system.

• Tests

  • Comprehensive unit tests for the DI container and event bus.

• Documentation

  • Updated bootstrap docs showing DI usage.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an Awilix-based DI composition root and types, replaces manual service construction in CLI commands and MCP tools with container resolution, introduces HookEvent types plus a typed EventBus and related interfaces, adds unit tests for DI and EventBus, and adds awilix to package.json.

Changes

Cohort / File(s)	Summary
Package package.json	Added dependency awilix (^12.1.0).
DI container & types src/infrastructure/di/container.ts, src/infrastructure/di/types.ts, src/infrastructure/di/index.ts	New DI composition root createContainer and typed cradle/options (ICradle, IContainerOptions); registers logger, clients, repos, event bus, and app services; supports scoped containers and optional LLM (enrich).
Commands (wired to DI) src/commands/context.ts, src/commands/liberate.ts, src/commands/recall.ts, src/commands/remember.ts	Replaced manual instantiation with createContainer({ logger, scope }); services and logger resolved from container.cradle; adjusted LLM/enrich validation and logging sourcing.
MCP tools (wired to DI) src/mcp/tools/context.ts, src/mcp/tools/liberate.ts, src/mcp/tools/recall.ts, src/mcp/tools/remember.ts	Same DI migration for MCP tools; liberate passes enrich into container to control LLM wiring.
Domain event types & re-exports src/domain/events/HookEvents.ts, src/domain/events/index.ts	Added ISessionStartEvent, ISessionStopEvent, IPromptSubmitEvent, discriminated union HookEvent, and HookEventType; re-exported via events index.
Event bus interfaces src/domain/interfaces/IEventBus.ts, src/domain/interfaces/IEventHandler.ts, src/domain/interfaces/IEventResult.ts	New typed event-bus contract: IEventBus (on/emit/registeredEvents), IEventHandler, and IEventResult to capture per-handler outcomes.
Event bus implementation src/infrastructure/events/EventBus.ts	New EventBus class implementing typed pub/sub; emit runs handlers asynchronously, collects IEventResult[], isolates handler errors and logs warnings via optional logger.
Tests tests/unit/infrastructure/di/container.test.ts, tests/unit/infrastructure/events/EventBus.test.ts	Added unit tests for DI container (resolution, singleton behavior, logger/LLM options, service wiring) and EventBus (handler ordering, error isolation, registeredEvents reporting).
Docs / CLI tweak CLAUDE.md, src/cli.ts	Documentation updated to describe DI bootstrapping; minor ESLint directive change in src/cli.ts (no behavioral change).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Caller as rgba(20,116,255,0.5) Publisher/Command
participant Bus as rgba(0,153,68,0.5) EventBus
participant HandlerA as rgba(255,153,51,0.5) Handler A
participant HandlerB as rgba(255,102,102,0.5) Handler B
Caller->>Bus: emit(HookEvent)
Bus->>HandlerA: handlerA.handle(event)
HandlerA-->>Bus: Promise (success)
Bus->>HandlerB: handlerB.handle(event)
HandlerB-->>Bus: Promise (error captured)
Bus-->>Caller: Promise<IEventResult[]>

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Fix hook logging + broken integration/e2e tests lisa#108: Overlaps in DI/bootstrap and service composition changes; likely touches the same composition root and wiring.
• Refactor: Consolidate legacy services.ts with DI container lisa#106: Similar migration from ad-hoc service construction to centralized DI across commands/tools.
• feat: add logging infrastructure and wire into all services #18: Related changes to logger wiring and how loggers are provided to services/commands; intersects with container-provided logger usage.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main changes: introducing a DI container and event bus foundation as Phase 1 of a broader refactoring effort.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch tony/git-39-install-awilix-dependency

---

No actionable comments were generated in the recent review. 🎉

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/commands/recall.ts (1)

5-19: ⚠️ Potential issue | 🟠 Major

Replace DI container usage with manual wiring in command handlers.
The command now uses createContainer (Line 17), but command handlers must construct their dependency chain directly per invocation (NotesService → MemoryRepository → MemoryService). Please revert to manual wiring here.

As per coding guidelines: src/commands/**/*.ts: Use manual dependency construction instead of a DI container. Each command/tool handler must construct its dependency chain fresh per invocation: NotesService → MemoryRepository → MemoryService.

src/commands/context.ts (1)

5-17: ⚠️ Potential issue | 🟠 Major

Replace DI container usage with manual wiring in command handlers.
The command now uses createContainer (Line 15), but command handlers must construct their dependency chain directly per invocation (NotesService → MemoryRepository → MemoryService). Please revert to manual wiring here.

As per coding guidelines: src/commands/**/*.ts: Use manual dependency construction instead of a DI container. Each command/tool handler must construct its dependency chain fresh per invocation: NotesService → MemoryRepository → MemoryService.

src/commands/remember.ts (1)

5-22: ⚠️ Potential issue | 🟠 Major

Commands must keep manual wiring (no DI container).
This now uses createContainer, but command handlers are required to construct dependencies per invocation (NotesService → MemoryRepository → MemoryService). Please revert to explicit wiring in this file.

As per coding guidelines: “src/commands/**/*.ts: Use manual dependency construction instead of a DI container. Each command/tool handler must construct its dependency chain fresh per invocation: NotesService → MemoryRepository → MemoryService”.

🤖 Fix all issues with AI agents

In `@src/commands/liberate.ts`:
- Around line 17-25: Replace the DI container usage (createContainer and
subsequent container.cradle destructuring) with manual construction of the
command's dependency chain: instantiate each dependency in order (e.g., create
MemoryRepository, then MemoryService, then NotesService, then LiberateService),
wire the llm client/anthropic client similarly so options.enrich checks
reference that explicit llmClient instance, and keep the existing logger usage
by passing the logger into the constructed services (replace references to
container.cradle.liberateService, container.cradle.llmClient, and
container.cradle.logger/log with the newly created instances). Ensure the logic
that logs the warning when enrich is enabled still checks the explicit llmClient
and that log.info is called on the passed logger instance after construction.

In `@src/infrastructure/di/types.ts`:
- Around line 1-7: Update the file-level docblock to mention both exported
interfaces — ICradle and IContainerOptions — rather than only ICradle; describe
that this file defines the typed shape of the DI container (ICradle) and the
configuration/options shape (IContainerOptions), note that all types reference
interfaces not concrete implementations, and keep wording concise so readers
immediately understand the purpose of both exports.

In `@src/infrastructure/events/EventBus.ts`:
- Around line 37-44: The handler name is obtained via handler.constructor.name
which can be mangled or empty in production; change the API to require/accept an
explicit stable name instead. Add a required name property to the IEventHandler
interface (or accept a name when registering handlers), update EventBus.emit and
any registration logic to use handler.name (or the supplied registration name)
when populating IEventResult.handler, and update all call sites that
construct/register handlers so they provide a meaningful name instead of relying
on handler.constructor.name.
- Around line 20-24: Change on() to return an unsubscribe function so callers
can remove handlers: update the IEventBus.on() signature to return () => void,
have EventBus.on<T extends HookEvent>(eventType: T['type'], handler:
IEventHandler<T>) add the handler to this.handlers (as it currently does) but
then return a function that removes that specific handler from the handlers map
for eventType (cleaning up the array and deleting the map entry if empty).
Ensure you reference and remove the same handler instance (IEventHandler<T> /
IEventHandler<HookEvent> casting as needed) and keep generics intact so typings
remain correct.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9448f26a0d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[TonyCasey]

Addressing Review Comments

1. coderabbitai — Commands should manually wire dependencies (liberate.ts, recall.ts, context.ts, remember.ts)
This PR intentionally replaces the old manual wiring pattern with a DI container — that's the purpose of GIT-42/GIT-43. CLAUDE.md bootstrapping docs updated in 92be41a to reflect the new createContainer() pattern.

2. coderabbitai — Docblock only describes ICradle (types.ts)
Fixed in 92be41a — docblock now describes both ICradle and IContainerOptions.

3. coderabbitai — Return unsubscribe from on() (EventBus.ts:24)
Valid suggestion but not needed now — handlers are registered once at startup and never removed. Can add if dynamic handler management is needed later.

4. coderabbitai — constructor.name unreliable (EventBus.ts:44)
Not applicable — git-mem is a Node.js CLI/MCP server, never minified. constructor.name is stable in Node.js.

5. codex-connector — CLASSIC mode with destructuring (container.ts:63)
Good catch — fixed in 92be41a. Converted asFunction registrations from destructuring ({ gitClient }: ICradle) to plain named parameters (gitClient: IGitClient) for unambiguous CLASSIC mode compatibility.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@CLAUDE.md`:
- Around line 45-50: Add a blank line before the opening fenced code block and a
blank line after the closing ``` so the block is separated from surrounding
text; update the section that shows the createContainer example (the lines
containing "const container = createContainer({ logger, scope: 'remember' });"
and "const { memoryService } = container.cradle;") to have an empty line above
the triple-backtick and an empty line below the closing triple-backtick.

In `@src/infrastructure/di/container.ts`:
- Around line 68-70: The container is using asClass with InjectionMode.CLASSIC
which requires constructor parameter names to match registration keys (ICradle)
— this is fragile; update src/infrastructure/di/container.ts by either adding a
clear comment above the registrations (near memoryService, contextService,
liberateService) noting the CLASSIC-name coupling, or switch the container to
InjectionMode.PROXY / use a different registration strategy, or add a small
runtime/assertion test that verifies constructor parameter names match the
ICradle keys; reference asClass, InjectionMode.CLASSIC, ICradle, and the
registration names memoryService, contextService, liberateService when making
the change.