# Akmatori Deep Refactoring Plan

## Context

The Akmatori codebase has accumulated structural debt across all three stacks. Several god objects (1,000+ line files with 19-34 methods), 393 unstructured `log.Printf` calls, dead feature code, and duplicated React patterns are slowing development, making testing painful (handlers at 10.2% coverage), and causing unexpected side effects from tight coupling. This refactoring addresses all of it in a single sweep.

**Decisions**: Full stack | Big bang | slog (stdlib) | Custom hooks only (no new deps) | Remove dead features completely

---

## Phase 0: Dead Code Removal

Remove unimplemented features and legacy code first to reduce the surface area for subsequent phases.

- [x] Delete `device_auth_service.go`, `device_auth_service_test.go`, `device_auth_service_edge_test.go`
- [x] Remove `codexWSHandler` and `deviceAuthService` fields from `api.go`
- [x] Remove 4 device auth handlers from `api_settings.go`
- [x] Remove device auth types, fields, and methods from `codex_ws.go`
- [x] Remove `runIncidentLocal` from `api_incidents.go`
- [x] Remove `runSlackChannelInvestigationLocal` from `alert.go`
- [x] Remove `OpenAISettings` struct and methods from `models.go`
- [x] Remove OpenAI DB functions from `db.go`
- [x] Remove OpenAI benchmarks from `models_test.go`
- [x] Update test files referencing `deviceAuthService`
- [x] Verify: `make verify`

---

## Phase 1: Structured Logging Migration (slog)

Replace 393 `log.Printf/Fatalf/Println` calls across 28 files with `log/slog`.

- [x] Create `internal/logging/logging.go` with slog initialization
- [x] Migrate all log.Printf/Fatalf/Println calls across 28 files to slog
- [x] Add `logging.Init()` call in `cmd/akmatori/main.go`
- [x] Update `mcp-gateway/` (8 occurrences across 2 files)
- [x] Verify: `make verify` + grep returns 0 log.Printf calls

---

## Phase 2: Go Backend — Split God Objects

- [x] 2A: Split `internal/database/models.go` (588 lines → 6 files)
- [x] 2B: Split `internal/services/skill_service.go` (1,129 lines → 4 files)
- [x] 2C: Split `internal/handlers/alert.go` (1,272 lines → 4 files)
- [x] 2D: Split `internal/handlers/slack.go` (909 lines → 3 files)
- [x] 2E: Split `internal/handlers/api_settings.go` (586 lines → 4 files)
- [x] Verify after each sub-phase: `make verify`

### 2A: Split `internal/database/models.go` (588 lines → 6 files)

| New file | Content |
|----------|---------|
| `models.go` | Keep only `JSONB` type + Scan/Value |
| `models_settings.go` | SlackSettings, LLMSettings, LLMProvider, ThinkingLevel, ProxySettings, GeneralSettings, APIKeySettings, APIKeyEntry |
| `models_skills.go` | Skill, ToolType, ToolInstance, SkillTool, EventSource, EventSourceType |
| `models_alerts.go` | AlertSourceType, AlertSourceInstance, AlertSeverity, AlertStatus, GetSeverityEmoji |
| `models_incidents.go` | Incident, IncidentStatus constants |
| `models_context.go` | ContextFile, Runbook |

All files: `package database`. No import changes anywhere — purely file-level split.

### 2B: Split `internal/services/skill_service.go` (1,129 lines → 4 files)

| New file | Methods moved |
|----------|-------------|
| `skill_file_sync.go` | EnsureSkillDirectories, EnsureSkillScriptsDir, SyncSkillAssets, ClearSkillScripts, ListSkillScripts, GetSkillScript, UpdateSkillScript, DeleteSkillScript, SyncSkillsFromFilesystem, RegenerateAllSkillMds |
| `skill_prompt_service.go` | generateSkillMd, generateToolUsageExample, extractToolDetails, sshAllHostsAllowWrite, GetSkillPrompt, UpdateSkillPrompt, stripAutoGeneratedSections, SkillFrontmatter, truncateString |
| `incident_service.go` | IncidentContext, SubagentSummaryInput, SpawnIncidentManager, generateIncidentAgentsMd, UpdateIncidentStatus, UpdateIncidentComplete, UpdateIncidentLog, GetIncident, SummarizeSubagentForContext, AppendSubagentLog |
| `skill_service.go` (keep) | SkillService struct, NewSkillService, CRUD ops, AssignTools, validation |

All methods stay on `*SkillService` receiver — same package, just different files.

### 2C: Split `internal/handlers/alert.go` (1,272 lines → 4 files)

| New file | Methods moved |
|----------|-------------|
| `alert_processor.go` | processAlert, processSlackChannelAlert, buildAlertTask, buildAlertHeader |
| `alert_slack.go` | postSlackThreadReply, postSlackThreadReplyGetTS, updateSlackThreadMessage, updateSlackChannelReactions, buildSlackResponse, buildSlackNotification, formatSlackAlertMessage, truncateLogForSlack |
| `alert_aggregation.go` | checkForExistingIncident, tryAttachToIncident, buildAggregationContext |
| `alert.go` (keep) | AlertHandler struct, NewAlertHandler, RegisterAdapter, HandleWebhook, ServeHTTP, runInvestigation |

### 2D: Split `internal/handlers/slack.go` (909 lines → 3 files)

| New file | Content |
|----------|---------|
| `slack_processor.go` | Business logic extracted from event handling methods |
| `slack_formatting.go` | Message formatting utilities |
| `slack.go` (keep) | SlackHandler struct, NewSlackHandler, HandleSocketMode, event dispatch |

### 2E: Split `internal/handlers/api_settings.go` (586 lines → 4 files)

| New file | Content |
|----------|---------|
| `api_settings_llm.go` | handleLLMSettings, ModelConfigs |
| `api_settings_slack.go` | handleSlackSettings, maskToken |
| `api_settings_proxy.go` | handleProxySettings, GetProxySettings, UpdateProxySettings, maskProxyURL, isValidURL |
| `api_settings_general.go` | handleGeneralSettings, handleGetAggregationSettings, handleUpdateAggregationSettings |

Delete `api_settings.go` after all content moved.

**Verify after each sub-phase**: `make verify`

---

## Phase 3: Interface Extraction for Testability

- [x] Create `internal/services/interfaces.go` with manager interfaces
- [x] Update `APIHandler` and `AlertHandler` to use interfaces
- [x] Verify: `make verify`

### CREATE:
- **`internal/services/interfaces.go`** — Define interfaces:
  - `SkillManager` — skill CRUD + lifecycle
  - `IncidentManager` — incident spawn/update/get
  - `ToolManager` — tool instance CRUD
  - `AlertManager` — alert source operations
  - `RunbookManager` — runbook CRUD

### MODIFY:
- **`internal/handlers/api.go`** — Change `APIHandler` struct fields from `*services.XxxService` to `services.XxxManager` interfaces
- **`internal/handlers/alert.go`** — Change `AlertHandler` fields similarly
- **`cmd/akmatori/main.go`** — No changes needed (concrete types satisfy interfaces via Go structural typing)

This unblocks future handler unit tests with mock services.

**Verify**: `make verify`

---

## Phase 4: React Frontend Refactoring

- [x] 4A: Create shared hooks (useAsync, useFormState)
- [x] 4B: Split Tools.tsx (993 lines)
- [x] 4C: Split Settings.tsx (792 lines)
- [x] 4D: Split AlertSourcesManager.tsx (595 lines)
- [x] 4E: Create shared components (StatusBadge, LoadingError)
- [x] Verify: `make test-agent` + manual browser check

### 4A: Create shared hooks

| New file | Purpose |
|----------|---------|
| `web/src/hooks/useAsync.ts` | Generic loading/error/data hook — replaces duplicated pattern in 11+ components |
| `web/src/hooks/useFormState.ts` | Consolidated form state management |

### 4B: Split `web/src/pages/Tools.tsx` (993 lines)

| New file | Content |
|----------|---------|
| `web/src/hooks/useToolManagement.ts` | Tool CRUD state + handlers |
| `web/src/hooks/useSSHKeyManagement.ts` | SSH key state + handlers |
| `web/src/components/tools/ToolFormSection.tsx` | Tool create/edit form |
| `web/src/components/tools/SSHKeysSection.tsx` | SSH key management UI |
| `web/src/components/tools/SSHHostsSection.tsx` | SSH host config UI |

`Tools.tsx` becomes thin orchestrator importing these.

### 4C: Split `web/src/pages/Settings.tsx` (792 lines)

| New file | Content |
|----------|---------|
| `web/src/components/settings/LLMSettingsSection.tsx` | Provider, model, API key config |
| `web/src/components/settings/SlackSettingsSection.tsx` | Slack token management |
| `web/src/components/settings/GeneralSettingsSection.tsx` | Base URL, general config |

### 4D: Split `web/src/components/AlertSourcesManager.tsx` (595 lines)

| New file | Content |
|----------|---------|
| `web/src/hooks/useAlertSourceManagement.ts` | Alert source CRUD + webhook logic |
| `web/src/components/alerts/AlertSourceForm.tsx` | Create/edit form |

### 4E: Shared components

| New file | Content |
|----------|---------|
| `web/src/components/shared/StatusBadge.tsx` | Unified status/severity badge (currently duplicated in 3+ files) |
| `web/src/components/shared/LoadingError.tsx` | Loading spinner + error message compound component |

**Verify**: `make test-agent` + manual browser check

---

## Phase 5: Agent Worker Refactoring

- [x] Create `agent-worker/src/tool-output-formatter.ts`
- [x] Update `agent-runner.ts` to import and delegate to formatter
- [x] Verify: `cd agent-worker && npm test`

### CREATE:
- **`agent-worker/src/tool-output-formatter.ts`** — Extract from `agent-runner.ts`:
  - `formatToolArgs`, `formatToolOutput`, `extractToolText`, `collectTextParts`, `collectContentText`, `safeJSONStringify`
  - `ToolExecutionTrace` interface

### MODIFY:
- **`agent-worker/src/agent-runner.ts`** — Import formatter, delegate, remove ~130 lines

**Verify**: `cd agent-worker && npm test`

---

## Phase 6: Final Validation

- [x] Remove any remaining `//lint:ignore U1000` or `//nolint:unused` on deleted code
- [x] Run full suite: `make verify`, `make test-all`, `golangci-lint run`
- [x] Build all Docker containers: `docker-compose build`
- [x] Manual smoke test

1. Remove any remaining `//lint:ignore U1000` or `//nolint:unused` on deleted code
2. Run full suite:
   ```bash
   make verify          # go vet + Go tests
   make test-all        # includes agent-worker
   golangci-lint run    # lint pass
   ```
3. Build all Docker containers:
   ```bash
   docker-compose build
   ```
4. Manual smoke test: start stack, create incident, verify Slack flow

---

## Execution Order & Dependencies

```
Phase 0 (dead code) → Phase 1 (slog) → Phase 2 (file splits) → Phase 3 (interfaces)
                                                                       ↓
                                                    Phase 4 (frontend) + Phase 5 (agent)  [parallel]
                                                                       ↓
                                                                 Phase 6 (validation)
```

Phases 4 and 5 are independent and can run in parallel. Everything else is sequential.

---

## Risk Mitigation

| Risk | Mitigation |
|------|------------|
| File split breaks compilation | All splits stay within same Go package — no import changes |
| Interface extraction breaks wiring | Go structural typing = concrete types auto-satisfy interfaces |
| slog changes log output format | JSON is more parseable; any log grep scripts need updating |
| Frontend split breaks UI state | No logic changes — only extraction; manual browser verification |
| Dead code removal hits live reference | `make verify` catches immediately; grep verified all references |

**Rollback**: Each phase is a separate commit. `git revert` any phase if needed. No database migrations involved.
