Skip to content

Commit 4cb635b

Browse files
Merge pull request #408 from re-cinq/298-architecture-audit
Architecture audit ADR - docs only
2 parents 7f4defa + c08edc2 commit 4cb635b

6 files changed

Lines changed: 426 additions & 5 deletions

File tree

.golangci.yml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,72 @@ linters:
77
- wastedassign
88
- gocritic
99
- nolintlint
10+
- depguard
1011
settings:
1112
nolintlint:
1213
require-explanation: true
1314
require-specific: true
15+
depguard:
16+
rules:
17+
# Rule 1: Domain and Infrastructure packages must not import Presentation packages.
18+
# Known violation excluded: doctor → onboarding (ADR-003).
19+
no-presentation-reverse:
20+
files:
21+
- "**/internal/pipeline/**"
22+
- "**/internal/adapter/**"
23+
- "**/internal/contract/**"
24+
- "**/internal/relay/**"
25+
- "**/internal/deliverable/**"
26+
- "**/internal/preflight/**"
27+
- "**/internal/recovery/**"
28+
- "**/internal/skill/**"
29+
- "**/internal/defaults/**"
30+
- "**/internal/suggest/**"
31+
- "**/internal/state/**"
32+
- "**/internal/workspace/**"
33+
- "**/internal/worktree/**"
34+
- "**/internal/forge/**"
35+
- "**/internal/github/**"
36+
deny:
37+
- pkg: "github.com/recinq/wave/internal/display"
38+
desc: "Layer violation: Domain/Infrastructure must not import Presentation (ADR-003)"
39+
- pkg: "github.com/recinq/wave/internal/tui"
40+
desc: "Layer violation: Domain/Infrastructure must not import Presentation (ADR-003)"
41+
- pkg: "github.com/recinq/wave/internal/webui"
42+
desc: "Layer violation: Domain/Infrastructure must not import Presentation (ADR-003)"
43+
- pkg: "github.com/recinq/wave/internal/onboarding"
44+
desc: "Layer violation: Domain/Infrastructure must not import Presentation (ADR-003)"
45+
# Rule 2: Infrastructure packages must not import Domain packages.
46+
infrastructure-no-domain:
47+
files:
48+
- "**/internal/state/**"
49+
- "**/internal/workspace/**"
50+
- "**/internal/worktree/**"
51+
- "**/internal/forge/**"
52+
- "**/internal/github/**"
53+
deny:
54+
- pkg: "github.com/recinq/wave/internal/pipeline"
55+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
56+
- pkg: "github.com/recinq/wave/internal/adapter"
57+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
58+
- pkg: "github.com/recinq/wave/internal/contract"
59+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
60+
- pkg: "github.com/recinq/wave/internal/relay"
61+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
62+
- pkg: "github.com/recinq/wave/internal/deliverable"
63+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
64+
- pkg: "github.com/recinq/wave/internal/preflight"
65+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
66+
- pkg: "github.com/recinq/wave/internal/recovery"
67+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
68+
- pkg: "github.com/recinq/wave/internal/skill"
69+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
70+
- pkg: "github.com/recinq/wave/internal/defaults"
71+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
72+
- pkg: "github.com/recinq/wave/internal/suggest"
73+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
74+
- pkg: "github.com/recinq/wave/internal/doctor"
75+
desc: "Layer violation: Infrastructure must not import Domain (ADR-003)"
1476
exclusions:
1577
presets:
1678
- comments

docs/adr/003-layered-architecture.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ An audit of the codebase reveals the following import relationships among intern
2626
| `event` | (none) |
2727
| `forge` | (none) |
2828
| `github` | (none) |
29-
| `manifest` | (none) |
30-
| `onboarding` | `manifest`, `tui` |
29+
| `manifest` | `skill` |
30+
| `onboarding` | `manifest`, `skill`, `tui` |
3131
| `pathfmt` | (none) |
32-
| `pipeline` | `adapter`, `audit`, `contract`, `deliverable`, `event`, `manifest`, `preflight`, `relay`, `security`, `skill`, `state`, `workspace`, `worktree` |
32+
| `pipeline` | `adapter`, `audit`, `contract`, `deliverable`, `event`, `forge`, `manifest`, `preflight`, `recovery`, `relay`, `security`, `skill`, `state`, `workspace`, `worktree` |
3333
| `preflight` | `skill` |
3434
| `recovery` | `contract`, `pathfmt`, `preflight`, `security` |
3535
| `relay` | (none) |
@@ -48,9 +48,9 @@ The presentation/backend separation is mostly intact — the event system proper
4848
- **`doctor``onboarding`**: Domain package importing a presentation package
4949
- **`defaults``pipeline`**: Cross-cutting package importing domain, creating a circular-risk dependency
5050

51-
The `pipeline/executor.go` god-object (2,493+ lines, 11+ responsibilities) is the primary structural concern, but that is addressed separately by [ADR-002](002-extract-step-executor.md). This ADR focuses on the inter-package layer boundaries, not intra-package decomposition.
51+
The `pipeline/executor.go` god-object (3,104 lines, 11+ responsibilities) is the primary structural concern, but that is addressed separately by [ADR-002](002-extract-step-executor.md). This ADR focuses on the inter-package layer boundaries, not intra-package decomposition.
5252

53-
This work builds on the architecture audit from [#298](https://github.com/re-cinq/wave/issues/298).
53+
This work builds on the [architecture audit](../architecture-audit.md) from [#298](https://github.com/re-cinq/wave/issues/298).
5454

5555
## Decision
5656

@@ -125,6 +125,7 @@ Presentation → Domain → Infrastructure
125125
| Presentation → Domain (direct adapter/workspace use) | `webui``adapter`, `workspace` | Medium | Refactor `webui` to use domain interfaces rather than concrete infrastructure types |
126126
| Domain → Presentation | `doctor``onboarding` | Medium | Extract the shared logic into a domain-level interface or move `onboarding` interaction behind an interface |
127127
| Cross-cutting → Domain | `defaults``pipeline` | Low | `defaults` embeds pipeline definitions — consider moving the pipeline type definitions it needs into `manifest` |
128+
| Cross-cutting → Domain | `manifest``skill` | Low | `manifest` imports `skill` for skill configuration types — consider extracting shared types into `manifest` or a shared types package |
128129

129130
## Options Considered
130131

docs/architecture-audit.md

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
# Wave Architecture Audit
2+
3+
**Date**: 2026-03-16
4+
**Scope**: Full inventory of `internal/` packages, dependency graph, design patterns, and structural concerns.
5+
**Purpose**: Baseline reference for [ADR-003](adr/003-layered-architecture.md) layered architecture transition.
6+
7+
## Package Inventory
8+
9+
Wave's `internal/` directory contains **25 Go packages** totaling ~48,655 lines of production code and ~62,552 lines of test code.
10+
11+
| Package | Prod Lines | Test Lines | Responsibility |
12+
|---------|-----------|------------|----------------|
13+
| `adapter` | 2,914 | 4,301 | Subprocess execution for Claude Code and other LLM CLIs |
14+
| `audit` | 133 | 709 | Audit logging with credential scrubbing |
15+
| `contract` | 4,030 | 3,498 | Output validation (JSON schema, TypeScript, test suites, markdown spec) |
16+
| `defaults` | 209 | 529 | Embedded default personas, pipelines, and contracts |
17+
| `deliverable` | 513 | 222 | Pipeline deliverable tracking and output formatting |
18+
| `display` | 5,294 | 5,605 | Terminal progress display and formatting |
19+
| `doctor` | 2,474 | 2,500 | Project health checking and optimization |
20+
| `event` | 200 | 642 | Progress event emission and monitoring (producer/consumer) |
21+
| `forge` | 209 | 341 | Git forge/hosting platform detection (GitHub, GitLab, etc.) |
22+
| `github` | 1,188 | 722 | GitHub API integration (issue enhancement, PR operations) |
23+
| `manifest` | 555 | 1,816 | Configuration loading and validation (`wave.yaml`) |
24+
| `onboarding` | 1,498 | 1,056 | Interactive `wave init` flow |
25+
| `pathfmt` | 22 | 66 | Path formatting and normalization utilities |
26+
| `pipeline` | 11,050 | 23,742 | Pipeline execution, DAG traversal, step management |
27+
| `preflight` | 288 | 493 | Pipeline dependency validation and auto-install |
28+
| `recovery` | 295 | 677 | Pipeline recovery hints and error guidance |
29+
| `relay` | 422 | 2,780 | Context compaction and summarization |
30+
| `security` | 955 | 1,195 | Security validation, path sanitization, permission enforcement |
31+
| `skill` | 1,841 | 4,030 | Skill discovery, provisioning, and command management |
32+
| `state` | 2,828 | 2,815 | SQLite persistence and state management |
33+
| `suggest` | 362 | 405 | Pipeline suggestion engine |
34+
| `tui` | 13,092 | 11,403 | Bubble Tea terminal UI (pipeline list, detail, live output) |
35+
| `webui` | 2,111 | 907 | Web operations dashboard (behind `//go:build webui` tag) |
36+
| `workspace` | 289 | 940 | Ephemeral workspace management |
37+
| `worktree` | 134 | 335 | Git worktree lifecycle for isolated workspaces |
38+
39+
## Internal Dependency Graph
40+
41+
Each row shows which internal packages are imported.
42+
43+
| Package | Internal Imports | Fan-out |
44+
|---------|-----------------|---------|
45+
| `adapter` | `github` | 1 |
46+
| `audit` | *(none)* | 0 |
47+
| `contract` | `pathfmt` | 1 |
48+
| `defaults` | `manifest`, `pipeline` | 2 |
49+
| `deliverable` | `pathfmt` | 1 |
50+
| `display` | `deliverable`, `event`, `pathfmt` | 3 |
51+
| `doctor` | `forge`, `github`, `manifest`, `onboarding`, `pipeline` | 5 |
52+
| `event` | *(none)* | 0 |
53+
| `forge` | *(none)* | 0 |
54+
| `github` | *(none)* | 0 |
55+
| `manifest` | `skill` | 1 |
56+
| `onboarding` | `manifest`, `skill`, `tui` | 3 |
57+
| `pathfmt` | *(none)* | 0 |
58+
| `pipeline` | `adapter`, `audit`, `contract`, `deliverable`, `event`, `forge`, `manifest`, `preflight`, `recovery`, `relay`, `security`, `skill`, `state`, `workspace`, `worktree` | 15 |
59+
| `preflight` | `skill` | 1 |
60+
| `recovery` | `contract`, `pathfmt`, `preflight`, `security` | 4 |
61+
| `relay` | *(none)* | 0 |
62+
| `security` | *(none)* | 0 |
63+
| `skill` | *(none)* | 0 |
64+
| `state` | *(none)* | 0 |
65+
| `suggest` | `doctor`, `forge` | 2 |
66+
| `tui` | `display`, `event`, `forge`, `github`, `manifest`, `pathfmt`, `pipeline`, `state` | 8 |
67+
| `webui` | `adapter`, `audit`, `display`, `event`, `manifest`, `pipeline`, `state`, `workspace` (behind `webui` build tag) | 8 |
68+
| `workspace` | *(none)* | 0 |
69+
| `worktree` | *(none)* | 0 |
70+
71+
### Fan-in (packages that depend on each package)
72+
73+
| Package | Depended On By | Fan-in |
74+
|---------|---------------|--------|
75+
| `manifest` | `defaults`, `doctor`, `onboarding`, `pipeline`, `tui`, `webui` | 6 |
76+
| `skill` | `manifest`, `onboarding`, `pipeline`, `preflight` | 4 |
77+
| `event` | `display`, `pipeline`, `tui`, `webui` | 4 |
78+
| `pathfmt` | `contract`, `deliverable`, `display`, `recovery`, `tui` | 5 |
79+
| `pipeline` | `defaults`, `doctor`, `tui`, `webui` | 4 |
80+
| `forge` | `doctor`, `pipeline`, `suggest`, `tui` | 4 |
81+
| `state` | `pipeline`, `tui`, `webui` | 3 |
82+
| `adapter` | `pipeline`, `webui` | 2 |
83+
| `security` | `pipeline`, `recovery` | 2 |
84+
| `display` | `tui`, `webui` | 2 |
85+
| `github` | `adapter`, `doctor`, `tui` | 3 |
86+
| `contract` | `pipeline`, `recovery` | 2 |
87+
| `workspace` | `pipeline`, `webui` | 2 |
88+
| `deliverable` | `display`, `pipeline` | 2 |
89+
| `audit` | `pipeline`, `webui` | 2 |
90+
| `preflight` | `pipeline`, `recovery` | 2 |
91+
| `doctor` | `suggest` | 1 |
92+
| `onboarding` | `doctor` | 1 |
93+
| `tui` | `onboarding` | 1 |
94+
| `worktree` | `pipeline` | 1 |
95+
| `relay` | `pipeline` | 1 |
96+
| `recovery` | `pipeline` | 1 |
97+
| `suggest` | *(none internal)* | 0 |
98+
| `defaults` | *(none internal)* | 0 |
99+
100+
### CLI Boundary
101+
102+
`cmd/wave/commands/` is the entry point connecting CLI to internal packages. It imports:
103+
`adapter`, `audit`, `defaults`, `display`, `doctor`, `event`, `forge`, `manifest`, `onboarding`, `pipeline`, `preflight`, `recovery`, `skill`, `state`, `suggest`, `tui`, `workspace`
104+
105+
`cmd/wave/main.go` imports: `commands`, `doctor`, `manifest`, `state`, `suggest`, `tui`
106+
107+
The CLI layer acts as the composition root, wiring together all internal packages.
108+
109+
## Key Design Patterns
110+
111+
### 1. Event System (Producer/Consumer Decoupling)
112+
113+
The `event` package provides a publish-subscribe mechanism that decouples pipeline execution from display rendering. The `pipeline` package emits structured progress events (`StepStarted`, `StepCompleted`, `ContractValidating`, etc.) and `display`/`tui` packages consume them without direct coupling to the pipeline internals.
114+
115+
- **Producer**: `pipeline/executor.go` emits events via `event.Emitter`
116+
- **Consumers**: `display/progress.go` renders terminal output, `tui/` renders Bubble Tea UI
117+
- **Benefit**: Adding new display modes (e.g., `webui`) requires no pipeline changes
118+
119+
### 2. Adapter Pattern (Subprocess Execution)
120+
121+
The `adapter` package abstracts LLM CLI execution behind an `Adapter` interface. The primary implementation (`ClaudeAdapter`) manages subprocess lifecycle, I/O streaming, and settings.json generation.
122+
123+
- **Interface**: `Adapter` with `Run(ctx, config) (Result, error)`
124+
- **Config**: `AdapterRunConfig` encapsulates workspace path, system prompt, permissions, timeout
125+
- **Testability**: `MockAdapter` supports deterministic testing without subprocess execution
126+
127+
### 3. Contract Validation
128+
129+
The `contract` package validates step outputs against declarative schemas before marking steps successful. Supported validators: `json_schema`, `typescript_interface`, `test_suite`, `markdown_spec`, `format`.
130+
131+
- **Hard failures**: Block step completion, prevent downstream execution
132+
- **Soft failures**: Log warnings, allow step to proceed
133+
- **Recovery**: `recovery` package provides hints for common contract failures
134+
135+
### 4. Workspace Isolation
136+
137+
The `workspace` package creates ephemeral directories for step execution. The `worktree` package manages git worktrees for full repository isolation.
138+
139+
- **Mount modes**: `readonly`, `readwrite` for shared workspace access
140+
- **Artifact injection**: Outputs from prior steps are injected into `.wave/artifacts/`
141+
- **Cleanup**: Workspaces are cleaned up after pipeline completion (configurable retention)
142+
143+
### 5. State Persistence
144+
145+
The `state` package provides SQLite-backed persistence for pipeline runs, step status, and resumption data.
146+
147+
- **Run tracking**: Pipeline run metadata, step status, timestamps
148+
- **Resume support**: Failed pipelines can resume from the last failed step
149+
- **Migration system**: Versioned schema migrations in `internal/state/`
150+
151+
### 6. CLAUDE.md Assembly
152+
153+
At each step boundary, a per-step CLAUDE.md is generated from four layers:
154+
1. Base protocol preamble (`.wave/personas/base-protocol.md`)
155+
2. Persona system prompt (role, responsibilities, constraints)
156+
3. Contract compliance section (auto-generated from step contract schema)
157+
4. Restriction section (denied/allowed tools, network domains)
158+
159+
This ensures fresh memory at every step boundary — no chat history inheritance.
160+
161+
## Structural Concerns
162+
163+
### 1. God Object: `executor.go` (3,104 lines)
164+
165+
`internal/pipeline/executor.go` is the largest single file, handling:
166+
- DAG traversal and topological sorting
167+
- Step lifecycle management
168+
- Workspace creation and cleanup
169+
- Artifact injection and extraction
170+
- CLAUDE.md assembly
171+
- Adapter invocation
172+
- Contract validation orchestration
173+
- Event emission
174+
- State persistence
175+
- Error recovery
176+
- Concurrent step execution
177+
178+
This is tracked in [ADR-002](adr/002-extract-step-executor.md) which proposes extracting a `StepExecutor` component.
179+
180+
### 2. High Fan-out: `pipeline` (15 internal imports)
181+
182+
The `pipeline` package imports 15 of the 25 internal packages (60%), making it a coupling hotspot. Any change to its dependencies risks cascading effects. This fan-out is partially inherent to its role as the orchestration core, but ADR-002's `StepExecutor` extraction would distribute some of these dependencies.
183+
184+
### 3. High Fan-out: `tui` and `webui` (8 internal imports each)
185+
186+
Both presentation packages import 8 internal packages. `tui` imports `pipeline` directly for type information and execution, coupling the terminal UI to orchestration internals. The `webui` package (behind a build tag) has similar coupling.
187+
188+
### 4. Layer Violations
189+
190+
Using ADR-003's four-layer model, the following cross-layer violations exist:
191+
192+
| Violation | Direction | Severity |
193+
|-----------|-----------|----------|
194+
| `doctor``onboarding` | Domain → Presentation | Medium |
195+
| `webui``adapter`, `workspace` | Presentation → Domain/Infrastructure | Medium |
196+
| `defaults``pipeline` | Domain → Domain (circular risk) | Low |
197+
| `manifest``skill` | Cross-cutting → Domain | Low |
198+
199+
The `manifest``skill` violation was not previously documented in ADR-003. The `manifest` package (cross-cutting) imports `skill` (domain) for skill configuration types.
200+
201+
### 5. Build Tag Isolation
202+
203+
The `webui` package is gated behind `//go:build webui`, meaning its dependency violations only materialize when building with that tag. This provides effective isolation in the default build but means standard `go list` analysis misses its imports.
204+
205+
### 6. Leaf Packages (Zero Internal Dependencies)
206+
207+
Nine packages have no internal imports: `audit`, `event`, `forge`, `github`, `pathfmt`, `relay`, `security`, `skill`, `state`, `workspace`, `worktree`. These are healthy leaf nodes that can be tested and reasoned about in isolation.
208+
209+
### 7. Test Coverage Distribution
210+
211+
Test-to-production ratio varies significantly:
212+
213+
| Package | Ratio (test:prod) | Notes |
214+
|---------|--------------------|-------|
215+
| `relay` | 6.6:1 | Well-tested relative to size |
216+
| `audit` | 5.3:1 | Well-tested |
217+
| `pipeline` | 2.1:1 | Adequate given complexity |
218+
| `manifest` | 3.3:1 | Well-tested |
219+
| `workspace` | 3.3:1 | Well-tested |
220+
| `tui` | 0.9:1 | Could use more test coverage |
221+
| `webui` | 0.4:1 | Low test coverage |
222+
| `deliverable` | 0.4:1 | Low test coverage |
223+
224+
## ADR-003 Discrepancies Found
225+
226+
During this audit, the following discrepancies with [ADR-003](adr/003-layered-architecture.md) were identified and corrected:
227+
228+
1. **`manifest``skill`**: Not listed in ADR-003's dependency table. `manifest` was shown with no internal imports, but it imports `skill` for skill configuration types. This is a cross-cutting → domain violation.
229+
230+
2. **`pipeline``forge`, `recovery`**: ADR-003's dependency table listed 13 internal imports for `pipeline`, but the actual count is 15. Missing: `forge` (infrastructure) and `recovery` (domain).
231+
232+
3. **`onboarding``skill`**: ADR-003 listed `onboarding` as importing `manifest`, `tui` but it also imports `skill`. This is an allowed import (presentation → domain) but was missing from the table.
233+
234+
4. **`executor.go` line count**: ADR-003 referenced "2,493+ lines" but the current count is 3,104 lines.

0 commit comments

Comments
 (0)