416 lines
17 KiB
Markdown
416 lines
17 KiB
Markdown
# Epic 12 — Technical Debt & Gap Closure
|
||
|
||
**status:** planned
|
||
**scope:** fix all known bugs, ambiguities, and structural gaps before building interfaces
|
||
**goal:** codebase is correct, documented, and stable before Epic 13 (interfaces) begins
|
||
|
||
---
|
||
|
||
## task 1: ModelDescriptor.capabilities
|
||
|
||
**problem:** `LlamaCppInferenceProvider.capabilities()` derives capabilities from model name string matching. fragile, wrong on rename, invisible failure.
|
||
|
||
**fix:** add `capabilities: Set<CapabilityScore>` to `ModelDescriptor`. `LlamaCppInferenceProvider.capabilities()` returns `descriptor.capabilities` directly.
|
||
|
||
**files:**
|
||
- `infrastructure/inference/.../ModelDescriptor.kt` — add field
|
||
- `infrastructure/inference/.../LlamaCppInferenceProvider.kt` — remove name matching, return descriptor field
|
||
|
||
**acceptance criteria:**
|
||
- capabilities declared per-descriptor, not derived from name
|
||
- no string matching logic in `capabilities()`
|
||
|
||
---
|
||
|
||
## task 2: DefaultInferenceRouter
|
||
|
||
**problem:** `InferenceRouter` interface exists, no implementation. orchestrator calls `route(stageId, emptySet())` with hardcoded empty capabilities.
|
||
|
||
**fix:**
|
||
```kotlin
|
||
class DefaultInferenceRouter(
|
||
private val registry: ProviderRegistry,
|
||
private val strategy: RoutingStrategy,
|
||
) : InferenceRouter {
|
||
override fun route(stageId: StageId, requiredCapabilities: Set<ModelCapability>): InferenceProvider {
|
||
val candidates = requiredCapabilities
|
||
.flatMap { registry.resolve(it) }
|
||
.distinctBy { it.id }
|
||
.ifEmpty { registry.listAll() }
|
||
return strategy.select(candidates, requiredCapabilities)
|
||
}
|
||
}
|
||
```
|
||
|
||
**files:**
|
||
- `core/inference/.../DefaultInferenceRouter.kt` — new
|
||
- `infrastructure/.../InfrastructureModule.kt` — wire `createInferenceRouter()`
|
||
|
||
**acceptance criteria:**
|
||
- routes by capability, not hardcoded
|
||
- `NoEligibleProviderException` on no candidates
|
||
- contract tests: match found, no match throws, empty capabilities returns any available
|
||
|
||
---
|
||
|
||
## task 3: StageConfig in WorkflowGraph
|
||
|
||
**problem:** `WorkflowGraph` holds `stages: Set<StageId>` only. orchestrator hardcodes context budget, generation config, capabilities.
|
||
|
||
**fix:**
|
||
```kotlin
|
||
data class StageConfig(
|
||
val requiredCapabilities: Set<ModelCapability> = emptySet(),
|
||
val tokenBudget: Int = 4096,
|
||
val generationConfig: GenerationConfig = GenerationConfig(),
|
||
val allowedTools: Set<String> = emptySet(),
|
||
val maxRetries: Int = 3,
|
||
)
|
||
|
||
data class WorkflowGraph(
|
||
val stages: Map<StageId, StageConfig>,
|
||
val transitions: Set<TransitionEdge>,
|
||
val start: StageId,
|
||
) {
|
||
val stageIds: Set<StageId> get() = stages.keys
|
||
init { require(start in stages) { "start stage must exist in stages" } }
|
||
}
|
||
```
|
||
|
||
**before dispatching:** run `grep -r "WorkflowGraph" --include="*.kt" -l` to know blast radius.
|
||
|
||
**files:**
|
||
- `core/transitions/.../StageConfig.kt` — new
|
||
- `core/transitions/.../WorkflowGraph.kt` — replace `Set<StageId>` with `Map<StageId, StageConfig>`
|
||
- `core/kernel/.../DefaultSessionOrchestrator.kt` — use stage config values
|
||
- all tests constructing `WorkflowGraph` — update constructor
|
||
|
||
**acceptance criteria:**
|
||
- orchestrator derives capabilities, budget, generationConfig from stage config
|
||
- existing transition resolution and cycle analysis tests pass
|
||
- no hardcoded values remain in orchestrator for stage-specific config
|
||
|
||
---
|
||
|
||
## task 4: core:risk module
|
||
|
||
**problem:** approval tier hardcoded to `T2`. no risk signal aggregation exists.
|
||
|
||
**new module:** `:core:risk`
|
||
**depends on:** `:core:events`, `:core:approvals`, `:core:validation`, `:core:transitions`
|
||
|
||
**types:**
|
||
```kotlin
|
||
enum class RiskLevel { LOW, MEDIUM, HIGH, CRITICAL }
|
||
enum class RiskAction { PROCEED, PROMPT_USER, BLOCK }
|
||
|
||
sealed class RiskSignal {
|
||
data class DestructiveOperation(val tier: Tier) : RiskSignal()
|
||
data class CycleWithoutExit(val cycleId: String) : RiskSignal()
|
||
data class RepeatedFailure(val reason: String, val count: Int) : RiskSignal()
|
||
data class ValidationErrors(val errorCount: Int) : RiskSignal()
|
||
data class InferenceTimeout(val elapsedMs: Long) : RiskSignal()
|
||
}
|
||
|
||
data class RiskSummary(
|
||
val level: RiskLevel,
|
||
val signals: List<RiskSignal>,
|
||
val recommendedAction: RiskAction,
|
||
)
|
||
|
||
fun RiskLevel.toApprovalTier(): Tier = when (this) {
|
||
RiskLevel.LOW -> Tier.T1
|
||
RiskLevel.MEDIUM -> Tier.T2
|
||
RiskLevel.HIGH -> Tier.T3
|
||
RiskLevel.CRITICAL -> Tier.T4
|
||
}
|
||
|
||
interface RiskAssessor {
|
||
fun assess(
|
||
report: ValidationReport,
|
||
outcome: StageOutcome,
|
||
state: OrchestrationState,
|
||
): RiskSummary
|
||
}
|
||
```
|
||
|
||
**DefaultRiskAssessor rules:**
|
||
- T3/T4 tool in outcome → `DestructiveOperation` → at least HIGH
|
||
- validation report has errors → `ValidationErrors` → at least MEDIUM
|
||
- `retryCount >= maxAttempts - 1` → `RepeatedFailure` → HIGH
|
||
- cycle without policy binding → `CycleWithoutExit` → MEDIUM
|
||
- recent `InferenceTimeoutEvent` → `InferenceTimeout` → MEDIUM
|
||
- no signals → LOW, PROCEED
|
||
|
||
**new event:** `RiskAssessedEvent(sessionId, stageId, riskSummaryId, level, action)` — register in `Serialization.kt`.
|
||
|
||
**files:**
|
||
- `core/risk/src/main/kotlin/.../` — all risk types + interface + default impl
|
||
- `core/events/.../RiskAssessedEvent.kt` — new
|
||
- `core/events/.../serialization/Serialization.kt` — register
|
||
- `core/kernel/.../DefaultSessionOrchestrator.kt` — replace hardcoded `Tier.T2`
|
||
|
||
**acceptance criteria:**
|
||
- `DefaultRiskAssessor` is pure (no IO, no coroutines)
|
||
- each signal type has a dedicated test
|
||
- `RiskAssessedEvent` serializable and registered
|
||
- orchestrator derives tier from risk summary
|
||
|
||
---
|
||
|
||
## task 5: stage timeout enforcement
|
||
|
||
**problem:** `OrchestrationConfig.stageTimeoutMs` configured but never enforced.
|
||
|
||
**fix:** wrap stage execution in `withTimeout(config.stageTimeoutMs)` in `DefaultSessionOrchestrator`. on `TimeoutCancellationException` emit `InferenceTimeoutEvent`, return `StageOutcome.InferenceFailure(retryable = true)`.
|
||
|
||
**files:**
|
||
- `core/kernel/.../DefaultSessionOrchestrator.kt`
|
||
|
||
**acceptance criteria:**
|
||
- stage exceeding timeout produces `InferenceFailure`
|
||
- `InferenceTimeoutEvent` emitted
|
||
- timeout is per-stage
|
||
|
||
---
|
||
|
||
## task 6: session type safety and FSM gaps
|
||
|
||
**problems:**
|
||
1. `DefaultSessionRepository.getSession(sessionId: String)` takes raw `String` — loses type safety
|
||
2. `SessionStatus.REPLAYED` has no documented FSM transitions — can sessions leave REPLAYED state?
|
||
3. `SessionEventMapper` may be dead code after Epic 3 replaced it with `SessionReducer`
|
||
|
||
**fixes:**
|
||
1. change signature to `getSession(sessionId: SessionId)`
|
||
2. define explicit FSM edges for REPLAYED — either terminal (no outgoing) or document valid transitions
|
||
3. grep codebase for `SessionEventMapper` usages — delete if unused, document if kept
|
||
|
||
**files:**
|
||
- `core/sessions/.../DefaultSessionRepository.kt`
|
||
- `core/sessions/.../SessionFsm.kt` (or equivalent)
|
||
|
||
**acceptance criteria:**
|
||
- `getSession` takes `SessionId`
|
||
- REPLAYED state transitions are explicit and tested
|
||
- `SessionEventMapper` either removed or documented with justification
|
||
|
||
---
|
||
|
||
## task 7: transition engine undefined behaviors
|
||
|
||
**problems:**
|
||
1. no-match behavior undefined — what happens when no transition matches current stage?
|
||
2. `StageId` origin unspecified — who generates it, orchestrator or transition engine?
|
||
|
||
**fixes:**
|
||
1. define explicit behavior: emit `StageFailedEvent` with reason `"no matching transition"`, return typed failure from resolver
|
||
2. document and enforce: `StageId` is always caller-generated (orchestrator). transition engine never creates `StageId` values.
|
||
|
||
**files:**
|
||
- `core/transitions/.../DefaultTransitionResolver.kt` — add no-match handling
|
||
- `core/transitions/.../TransitionDecision.kt` — add `NoMatch` case if not present
|
||
- CLAUDE.md — add StageId ownership rule
|
||
|
||
**acceptance criteria:**
|
||
- no-match produces deterministic typed failure, not silent null/hang
|
||
- test: resolver with no matching transition returns `NoMatch` decision
|
||
- `StageId` ownership documented
|
||
|
||
---
|
||
|
||
## task 8: CycleSignature edge inclusion
|
||
|
||
**problem:** `CycleSignature` is derived from node set only. two cycles with same nodes but different edge order are treated as identical. policy binding can silently apply to the wrong cycle.
|
||
|
||
**fix:** include normalized edge set in `CycleSignature`:
|
||
```kotlin
|
||
data class CycleSignature(
|
||
val nodes: SortedSet<StageId>,
|
||
val edges: SortedSet<Pair<StageId, StageId>>,
|
||
)
|
||
```
|
||
|
||
**files:**
|
||
- `core/transitions/.../CycleSignature.kt`
|
||
- `core/validation/.../SemanticValidator.kt` — update signature derivation
|
||
- affected tests
|
||
|
||
**acceptance criteria:**
|
||
- two cycles with same nodes but different edges produce different signatures
|
||
- existing single-cycle tests still pass
|
||
|
||
---
|
||
|
||
## task 9: Epic 5 resolution document
|
||
|
||
**problem:** Epic 5 (Approvals) has no resolution document. `ApprovalEngine` contract, tier evaluation logic, grant semantics, and `ApprovalMode` behavior are undocumented. Epic 13 (interfaces) builds approval interaction on top of this.
|
||
|
||
**fix:** write `docs/epics/epic-5-resolution.md` covering:
|
||
- `ApprovalEngine` interface contract
|
||
- tier evaluation logic per mode (PROMPT, AUTO, DENY, YOLO)
|
||
- grant extraction semantics from event stream
|
||
- `ApprovalMode.PROMPT` current behavior (evaluates existing grants, no interactive prompt yet)
|
||
- what's deferred to Epic 13
|
||
|
||
**files:**
|
||
- `docs/epics/epic-5-resolution.md` — new
|
||
|
||
**acceptance criteria:**
|
||
- document exists and covers all four approval modes
|
||
- current PROMPT behavior limitation explicitly stated
|
||
- Epic 13 approval interaction scope is clear
|
||
|
||
---
|
||
|
||
## task 10: artifact lifecycle enforcement
|
||
|
||
**problems:**
|
||
1. `ArtifactLifecyclePhase` transitions are undocumented and unenforced — no reducer or FSM
|
||
2. `ArtifactRelationshipAddedEvent.relationshipType: String` — typo produces silent invalid relationship
|
||
3. `validationResultIds: List<String>` type unpinned — unclear if these are `ValidationReportId` or something else
|
||
|
||
**fixes:**
|
||
1. introduce `ArtifactReducer` enforcing valid phase transitions. invalid transitions → `IllegalStateException` or typed error
|
||
2. validate `relationshipType` against `ArtifactRelationshipType.entries` at event creation
|
||
3. pin type: rename to `validationReportIds: List<ValidationReportId>` — breaking change, fix all usages
|
||
|
||
**files:**
|
||
- `core/artifacts/.../ArtifactReducer.kt` — new
|
||
- `core/artifacts/.../ArtifactProjector.kt` — wire reducer
|
||
- `core/events/.../ArtifactRelationshipAddedEvent.kt` — add validation
|
||
- `core/artifacts/.../ArtifactLineage.kt` — rename + retype field
|
||
|
||
**acceptance criteria:**
|
||
- invalid phase transition throws at reducer level
|
||
- all valid transitions tested
|
||
- `relationshipType` validated at event creation, test: invalid type throws
|
||
- `validationReportIds` typed as `List<ValidationReportId>`
|
||
|
||
---
|
||
|
||
## task 11: context engine edge cases
|
||
|
||
**problems:**
|
||
1. `buildingInProgress` stuck permanently if process crashes between `ContextBuildingStartedEvent` and completion
|
||
2. L0+L1 exceeding `TokenBudget.limit` has no defined overflow behavior
|
||
3. Conversation compression strategy "last N" — N is undocumented and presumably hardcoded
|
||
|
||
**fixes:**
|
||
1. add recovery: on replay, if `buildingInProgress = true` and no completion/failure event follows, treat as failed. add `ContextBuildingInterruptedEvent` or handle via timeout during rebuild.
|
||
2. define overflow behavior explicitly: if L0+L1 > limit, truncate L1 from oldest first. L0 is never truncated. document this as a hard rule.
|
||
3. make N configurable in `CompressionStrategy.Conversation(keepLast: Int = 10)`. document default.
|
||
|
||
**files:**
|
||
- `core/context/.../DefaultContextReducer.kt` — stuck state recovery
|
||
- `core/context/.../DefaultContextCompressor.kt` — L1 overflow handling
|
||
- `core/context/.../CompressionStrategy.kt` — add `keepLast` param
|
||
|
||
**acceptance criteria:**
|
||
- replaying a session with interrupted context build produces valid (failed) state
|
||
- L0+L1 overflow test: L1 truncated, L0 retained
|
||
- `keepLast` documented and tested
|
||
|
||
---
|
||
|
||
## task 12: inference contract gaps
|
||
|
||
**problems:**
|
||
1. `InferenceTimeout` and `InferenceCancellationToken` interaction undefined — potential double-cancellation race
|
||
2. stale provider snapshot: provider becomes unavailable between routing and `infer()` call
|
||
3. `CancellationException` swallow not enforced by contract test — any provider can silently break cooperative cancellation
|
||
|
||
**fixes:**
|
||
1. document interaction rule: timeout fires → cancels token → provider sees cancellation. token cancel does NOT fire timeout. add KDoc on both types.
|
||
2. add health check in `DefaultInferenceRouter.route()` — filter out `Unavailable` providers before selection. on `infer()` failure with `Unavailable` health, throw `ProviderUnavailableException` (not generic inference failure).
|
||
3. add contract test: cancel coroutine mid-inference, assert coroutine is actually cancelled within 500ms.
|
||
|
||
**files:**
|
||
- `core/inference/.../InferenceCancellationToken.kt` — KDoc
|
||
- `core/inference/.../InferenceTimeout.kt` — KDoc
|
||
- `core/inference/.../DefaultInferenceRouter.kt` — health check before selection
|
||
- `testing/contracts/.../InferenceProviderContractTest.kt` — cancellation enforcement test
|
||
|
||
**acceptance criteria:**
|
||
- timeout/token interaction documented
|
||
- unavailable provider filtered at routing, not discovered at inference time
|
||
- cancellation contract test fails if provider swallows `CancellationException`
|
||
|
||
---
|
||
|
||
## task 13: orchestrator ambiguities
|
||
|
||
**problems:**
|
||
1. `ReplayStrategy.SkipValidation` leaves artifacts stuck in `VALIDATING` phase permanently
|
||
2. `ValidationFailure.retryable` ownership unclear — validator sets it or orchestrator?
|
||
3. approval tier for validation-triggered approvals unspecified
|
||
|
||
**fixes:**
|
||
1. when `SkipValidation` active, emit synthetic `ArtifactValidatedEvent` for any artifact in `VALIDATING` phase. document this as replay-only behavior.
|
||
2. rule: validator sets `retryable` based on failure type. orchestrator only reads it, never overrides. document this ownership in KDoc on `ValidationFailure`.
|
||
3. validation-triggered approvals use `Tier.T2` explicitly. document in approval trigger logic. revisit when risk model is wired (task 4 feeds into this).
|
||
|
||
**files:**
|
||
- `core/kernel/.../ReplayOrchestrator.kt` — synthetic validation events
|
||
- `core/kernel/.../StageOutcome.kt` — KDoc on `ValidationFailure.retryable`
|
||
- `core/validation/.../ApprovalTrigger.kt` — explicit tier for validation approvals
|
||
|
||
**acceptance criteria:**
|
||
- replay with SkipValidation: no artifacts left in VALIDATING phase
|
||
- `retryable` ownership documented and tested
|
||
- validation approval tier explicit and documented
|
||
|
||
---
|
||
|
||
## task 14: tool execution gaps
|
||
|
||
**problems:**
|
||
1. `ToolResult.Success` has no `exitCode` field — shell tool exit codes lost in receipt
|
||
2. `affectedEntities` hardcoded to `emptyList()` in `SandboxedToolExecutor` — tool lineage permanently lost
|
||
|
||
**fixes:**
|
||
1. add `exitCode: Int = 0` to `ToolResult.Success`. `SandboxedToolExecutor.emitCompleted()` uses it. `ShellTool` sets actual exit code.
|
||
2. `SandboxedToolExecutor.emitCompleted()`: if tool implements `FileAffectingTool`, populate `affectedEntities` from `tool.affectedPaths(request)`.
|
||
|
||
**files:**
|
||
- `core/tools/.../ToolResult.kt` — add `exitCode`
|
||
- `infrastructure/tools/.../SandboxedToolExecutor.kt` — use exitCode + affectedPaths
|
||
- `infrastructure/tools/shell/.../ShellTool.kt` — set exitCode in result
|
||
- affected tests
|
||
|
||
**acceptance criteria:**
|
||
- shell tool non-zero exit captured in receipt
|
||
- file-affecting tools produce non-empty `affectedEntities`
|
||
- existing tool tests updated for new `ToolResult.Success` signature
|
||
|
||
---
|
||
|
||
## sequencing
|
||
|
||
```
|
||
task 1 (ModelDescriptor.capabilities) ← quick, unblocks task 2
|
||
task 2 (DefaultInferenceRouter) ← depends on task 1
|
||
task 3 (StageConfig) ← independent, high blast radius
|
||
task 4 (core:risk) ← depends on task 3
|
||
task 5 (stage timeout) ← depends on task 4
|
||
task 6 (session type safety + FSM) ← independent
|
||
task 7 (transition undefined behaviors) ← independent
|
||
task 8 (CycleSignature edges) ← independent
|
||
task 9 (Epic 5 resolution doc) ← independent, no code
|
||
task 10 (artifact lifecycle) ← independent
|
||
task 11 (context engine edge cases) ← independent
|
||
task 12 (inference contract gaps) ← depends on task 2
|
||
task 13 (orchestrator ambiguities) ← depends on task 4
|
||
task 14 (tool execution gaps) ← independent
|
||
```
|
||
|
||
tasks 6–11 and 14 can be parallelized if using subagents.
|
||
tasks 1→2→12 and 3→4→5→13 are the two critical chains.
|
||
|
||
---
|
||
|
||
## after this epic
|
||
|
||
- update `des-doc-v0.1.md` and `spec-v0.1.md` to reflect current architecture
|
||
- write module specs for: infrastructure tools, inference, sessions (impl-level), kernel
|
||
- then Epic 13: interfaces (TUI + CLI + Ktor server) |