feat: add PhasePreset schema contract#200
Conversation
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the current review scope:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR adds the first bounded runtime contract slice for issue #198: a PhasePreset schema/type surface, validator, default lifecycle preset catalog, and regression tests. The semantic scope is intentionally limited to schema definition and fixture validation, not RunState replay, migration, or gate-engine execution. There were no prior kapi-agent reviews on this PR, so no fix ping-pong was required.
What changed
- Added
src/domain/phase-preset.tswith:- Phase, gate, evidence, branch, artifact, policy hint, blocker message, and preset IO types.
validatePhasePreset(...)structural validation.DEFAULT_PHASE_PRESETSfor all eight lifecycle phases.
- Added
test/phase-preset.test.tscovering:- Default preset lifecycle order and validation success.
- Failure cases for weak transition contracts.
- Graph execution expectations around
task_graph,policy_selection, and repair branching.
Why this is correct
The change creates a runtime-owned typed boundary for phase preset data without prematurely coupling it to replay or execution internals. The validator fails closed for the key transition-contract weaknesses this slice claims to enforce: missing inputs/outputs, blank/duplicate IDs, missing gate conditions/blocker messages, missing recovery branches for repair/human-decision failures, and non-advisory policy hints.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 122 changed lines, below the 200-line semantic review threshold.
- Revision explanation status: not required; found in PR body.
- Ilchul review harness gate: PASS — neutral/comment result with 0 blocking findings.
- Inspected files:
src/domain/phase-preset.ts,test/phase-preset.test.ts,package.json.
Remaining risks and approval rationale
Remaining risks are limited to the intentionally deferred integration work: RunState replay, gate evaluation serialization, and migration policy are not implemented here. That is acceptable because the PR clearly uses Refs #198, describes this as a bounded first slice, and includes tests for the new contract surface. Approval is justified because all gates pass and I found no blocking correctness, safety, workflow-contract, or regression issue in the current scope.
Blocking issues
- None.
Warnings / risks
src/domain/phase-preset.ts: the validator is structural rather than semantic/executable. It does not yet validate cross-reference integrity between blocker messages, gates, evidence, and branches beyond the current checks. This matches the stated slice, but the follow-up runtime integration should tighten those invariants before using presets as authoritative execution input.src/domain/phase-preset.ts: default gate conditions are human-readable strings, not executable predicates. Future gate-engine work should avoid treating these strings as trusted executable logic.
Suggestions
- Consider adding cross-reference validation in a follow-up, e.g.
blockerMessages.relatedGateIdmust point to an existing gate and branch evidence IDs must be declared or intentionally inline. - Consider formatting the interfaces over multiple lines if this module grows; the current compact style is acceptable for this small contract but may become harder to review as fields expand.
- When wiring this into RunState/replay, add tests that assert serialized presets remain stable across round trips.
Looks good
- The PR keeps the implementation focused and avoids mixing schema work with runtime replay/migration behavior.
- The default preset catalog covers the full lifecycle in a deterministic order.
- Tests cover both positive fixture validation and representative failure modes.
- The graph-execution test protects the important boundary that TaskGraph remains a runtime primitive rather than a policy hint.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifycompleted successfully. - Size gate status: PASS — 122 changed lines, below threshold.
- Revision-explanation status: not required for this opened PR; explanatory PR body is present.
- Additional evidence: Ilchul harness reported PASS with no blocking findings; PR body lists focused local QA commands and follow-up risks.
Engine: pi
Summary
Linked issue
Refs #198
This is a bounded first slice, not the full issue closeout. Remaining #198 work includes wiring these schemas into RunState replay / gate evaluation serialization and any migration policy once the runtime integration point is selected.
Problem
Issue #198 defines the desired PhasePreset schema in prose, but the repo did not yet have a machine-readable runtime contract or validation fixture set for the default lifecycle catalog. That leaves follow-up DAG/runtime work without a shared typed boundary for phase inputs, outputs, gates, evidence, artifacts, policy hints, and recovery branches.
Options considered
Selected approach
Selected option: option 3 — compact runtime schema/type module plus focused tests.
Why this one:
Risks/trade-offs:
Refs #198and leaves the issue open.Implementation by file/surface
src/domain/phase-preset.tsvalidatePhasePreset(...)for structural checks: required inputs/outputs, duplicate/blank IDs, gate condition/blocker messages, recovery branches for repair/human-decision failures, and advisory-only policy hints.DEFAULT_PHASE_PRESETSforstandard-intake,objective-approval,policy-selection,graph-execution,objective-evaluation,gated-integration,record-and-calibrate, andevidence-sealed-close.test/phase-preset.test.tsgraph-executiontreats TaskGraph as the runtime primitive and keeps repair branching visible.Why this fixes it
The issue’s prose schema now has a runtime-owned TypeScript contract and validator-backed examples. Future runtime slices can import this module instead of reinterpreting issue text, and tests protect the base catalog from losing required inputs, outputs, evidence freshness, advisory policy semantics, or graph-execution task-graph boundaries.
QA / Verification
npm test -- test/phase-preset.test.ts— pass; package script also ran the existingtest/*.test.tssuite because of the current script shape.npm run check— pass.npm run check:unused— pass.npm run quality:budgets— pass.git diff --check— pass.git diff --cached --shortstat origin/devbefore commit reported2 files changed, 122 insertions(+).Anomalies observed
npm ciinstalled dependencies..ilchul/runtime-prompts/.manual-attach-required/ prompt dispatch failed), so I manually supplied the readiness response and assignment, then continued this bounded slice directly from the clean supervisor worktree to avoid blocking the scheduled run.npm ciproduced recurring mode-only churn onsrc/cli/kapi-review-cli.ts; it was reset before staging.Risks / Follow-up
PhasePresetvalidation to RunState replay and gate evaluation tests.kapi-agent review expectations and current-head merge gate
kapi-agentAPPROVED review on the currentheadRefOid, successfulkapi-agent/reviewcheck, passing required checks, mergeable state, and supervisor verification evidence above.