feat: define runtime state schema#230
Conversation
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
kapi-agent review completedThe formal GitHub PR review and required check were submitted separately. This request comment is kept concise to avoid duplicating the full review body. kapi-agent request lifecycle:
|
![kapi-agent[bot]](https://avatars.githubusercontent.com/in/3661577?s=60&v=4){kind=small}
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
-
src/domain/runtime-state.ts:parseRuntimeState()is not yet a reliable fail-closed runtime schema boundary. It checks only a few top-level fields, then casts the input toRuntimeState. As a result, malformed persisted state can be accepted as valid, including:- unsupported/invalid local versions such as
schemaVersion: 0,-1, or1.5; - invalid enum values such as
status: "done"or task/worker/evaluation statuses outside the declared unions; - malformed nested records in
tasks,workers,evaluations,rewards,integrationCandidates,objective, andpolicySelection; - malformed nested artifact refs inside task/evaluation/reward/policy-selection structures.
Since this PR’s contract is specifically “versioned schema + fail-closed parsing,” accepting malformed nested state defeats the main safety boundary. Please either add validation for the declared schema surfaces or explicitly narrow the parser’s documented responsibility and tests so it is not presented as validating
RuntimeState. - unsupported/invalid local versions such as
Warnings / risks
src/domain/runtime-state.ts:validateArtifactRef()only checks thatkindis a nonblank string, not that it is one ofRuntimeArtifactKind. This can leak unknown artifact kinds throughparseRuntimeState()and makesbuildRunContractRuntimeRefs()dependent on partially trusted casted data.test/runtime-state.test.ts: current tests cover happy-path representation and a few top-level failures, but do not assert rejection of invalid nested task/worker/evaluation/reward/integration shapes. That leaves the primary schema contract under-tested.
Suggestions
- Add small enum guard helpers, e.g.
isRuntimeStatus,isRuntimeTaskStatus,isRuntimeArtifactKind, and use them during parsing. - Consider validating
schemaVersion === RUNTIME_STATE_SCHEMA_VERSIONfor now, unless older-version migrations are intentionally supported. - Add focused tests for:
- invalid
schemaVersionvalues below/current but malformed; - invalid top-level
status; - non-array or malformed
dependsOn,evidenceRefs,evaluationRefs; - malformed nested artifact refs;
- invalid
RuntimeArtifactKind.
- invalid
Looks good
- The change is appropriately bounded to the runtime-state schema slice, plus focused tests and README documentation.
- The schema types are product-neutral and do not introduce GitHub/Ragna/kapi-agent semantics into the domain layer.
- Unknown newer schema versions are rejected, which is the right compatibility posture.
buildRunContractRuntimeRefs()cleanly exposes only objective, policy-selection, and evaluation artifact refs.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 190 changed lines, under the 200-line semantic threshold.
- Revision-explanation status: not required for this review; explanation found.
- Ilchul review harness gate: PASS — STANDARD risk, neutral conclusion, no blocking findings.
- Local inspection reviewed
src/domain/runtime-state.ts,test/runtime-state.test.ts, and usage ofparseRuntimeState(). The blocking issue is semantic contract coverage, not CI failure.
Engine: pi
devkade
force-pushed
the
feat/issue-185-runstate-schema
branch
from
d9d9f5b to
5785391
Compare
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
kapi-agent review completedThe formal GitHub PR review and required check were submitted separately. This request comment is kept concise to avoid duplicating the full review body. kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
-
src/domain/runtime-state.ts:createRuntimeState()does not actually reject a blankrunId. It callsneedText(input.runId, "runId", []), but the collected issue is discarded, socreateRuntimeState({ runId: "", now })returns an invalidRuntimeState. Since this PR establishes the executable schema boundary, the constructor should not create state that the parser would later reject. Please make this throw or otherwise fail closed, and add a focused test. -
src/domain/runtime-state.ts:parseRuntimeState()still does not validate declared optional fields when present:RuntimeWorkerSchema.readinessNonce?: stringEvaluationResultSchema.taskId?: string
A persisted state with
workers: [{ ..., readinessNonce: 123 }]orevaluations: [{ ..., taskId: {} }]is accepted and cast toRuntimeState. This keeps a gap in the “fail-closed parsing for declared schema surfaces” contract from the prior review. Please validate optional fields when present and cover them with tests.
Warnings / risks
src/domain/runtime-state.ts: timestamp validation usesDate.parse(), which accepts some non-ISO/date-like strings. If the intended persisted contract is ISO timestamps, this should be tightened now or documented as intentionally accepting any JavaScript-parseable timestamp.
Suggestions
- Consider replacing
needText(..., [])style validation in constructors with a helper that throws directly, so future constructor validation cannot silently discard issues.
Looks good
- The revised parser now validates schema version shape, rejects newer schema versions, validates top-level runtime status, validates nested task/worker/evaluation/integration enum fields, and validates nested artifact refs.
- The scope remains bounded to the runtime-state schema/versioning slice and does not mix in event replay or worker execution behavior.
- README documentation was updated to mention the separate runtime state version boundary and RunContract-facing artifact refs.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 172 changed lines, below the 200-line semantic review threshold.
- Revision-explanation status: PASS — current-head revision explanation was provided.
- Ilchul review harness gate: PASS — STANDARD risk profile, no blocking findings.
- Local/deeper inspection found the remaining schema-boundary issues above despite the automated gates passing.
Engine: pi
devkade
force-pushed
the
feat/issue-185-runstate-schema
branch
from
5785391 to
70f0316
Compare
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
kapi-agent review completedThe formal GitHub PR review and required check were submitted separately. This request comment is kept concise to avoid duplicating the full review body. kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR defines the versioned RuntimeState schema boundary for the objective-driven runtime. The semantic scope is bounded to schema/types, fail-closed parsing, RunContract-facing runtime artifact refs, focused tests, and README documentation. It intentionally does not implement event replay, worker execution, storage migration, or runtime mutation.
There were two prior kapi-agent change requests:
- First:
parseRuntimeState()validated too little and cast malformed nested data asRuntimeState. - Second:
createRuntimeState()allowed blankrunId, and optional declared fields were not validated when present.
The current revision includes an author explanation and addresses those prior findings.
What changed
Important paths:
src/domain/runtime-state.ts- Adds
RuntimeState.schemaVersion: 1. - Defines runtime objective, policy, task, worker, artifact ref, evaluation, reward, and integration candidate schema types.
- Adds
createRuntimeState(),parseRuntimeState(), andbuildRunContractRuntimeRefs(). - Validates top-level fields, enum values, nested arrays/records, nested artifact refs, optional fields, and newer unsupported schema versions.
- Adds
test/runtime-state.test.ts- Adds initialization, fail-closed parsing, malformed nested field, blank constructor
runId, optional-field, RunContract-ref, success, and repair-required examples.
- Adds initialization, fail-closed parsing, malformed nested field, blank constructor
README.md- Documents the separately versioned runtime state boundary and RunContract artifact-ref exposure.
Why this is correct
The previous blocking gaps are closed:
createRuntimeState()now callsrequireText(input.runId, "runId"), so it cannot construct a blank-run-id state that the parser would later reject.parseRuntimeState()now validates declared nested schema surfaces rather than accepting a shallow object and casting.- Optional fields called out in the prior review are validated when present:
RuntimeWorkerSchema.readinessNonceEvaluationResultSchema.taskId
- Unknown newer schema versions fail closed with
unsupported-newer-schema, matching the stated compatibility contract.
Evidence
- Verifier gate: PASS.
- Size gate: PASS, 178 changed lines under the 200-line semantic threshold.
- Ilchul review harness gate: PASS, STANDARD risk profile, no blocking findings.
- Revision explanation status: required and found.
- Automated verification evidence:
npm ci && npm run verifyexited 0.- Full verifier included tests, type checks, unused checks, and quality budgets.
- Inspected files:
src/domain/runtime-state.tstest/runtime-state.test.tsREADME.md
Remaining risks and approval rationale
Remaining risks are non-blocking:
- Timestamp validation uses
Date.parse(), which is permissive; acceptable for this slice unless strict RFC3339/ISO timestamp semantics become part of the persisted-state contract. - Extra unknown object fields are currently tolerated. That is compatible with an additive schema posture, but future storage/migration work should document whether unknown fields are preserved, ignored, or rejected.
Approval is justified because all configured gates pass, prior blocking feedback is addressed, tests cover the corrected behavior, and the implementation matches the narrow versioned-schema contract described by the PR.
Blocking issues
- None.
Warnings / risks
src/domain/runtime-state.ts:Date.parse()accepts some loose date strings. If runtime state needs strict interoperable persistence, consider replacing it with a stricter timestamp validator in a follow-up.src/domain/runtime-state.ts: unknown extra fields are accepted after validation. This is reasonable for additive compatibility, but should remain an explicit policy as migrations/event replay are added.
Suggestions
- Consider adding table-driven negative tests later for each schema subsection as the runtime state grows.
- Consider extracting validators into a small reusable schema-validation helper if more domain schemas adopt this pattern.
Looks good
- The parser now fails closed on invalid schema versions, invalid enum values, malformed required fields, malformed nested arrays, malformed artifact refs, and the optional fields raised in the previous review.
buildRunContractRuntimeRefs()keeps the RunContract-facing artifact surface narrow.- The README update matches the implemented behavior without overclaiming event replay or execution semantics.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 178 changed lines, below threshold.
- Revision-explanation status: required and found.
- Ilchul review harness: PASS — STANDARD risk profile, no blocking findings.
- Local inspection confirmed the current head contains the fixes described in the author’s revision explanation.
Engine: pi
|
@kapi-agent review --force Revision explanation for current head What changed
Why this closes the prior feedback
Verification
|
devkade
force-pushed
the
feat/issue-185-runstate-schema
branch
from
70f0316 to
3268659
Compare
kapi-agent review completedThe formal GitHub PR review and required check were submitted separately. This request comment is kept concise to avoid duplicating the full review body. kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR adds a versioned RuntimeState schema boundary for the objective-driven runtime. The semantic scope is bounded to schema/types, fail-closed parsing, RunContract-facing runtime artifact references, focused tests, and README documentation.
Prior kapi-agent feedback requested stronger fail-closed validation for nested state, invalid enum/schema versions, constructor-created invalid runIds, and optional declared fields. Those issues were addressed in earlier revisions and approved. The current revision rebases the already-approved implementation onto current dev and preserves the README adjacency after related PRs landed.
What changed
Important paths:
src/domain/runtime-state.ts- Defines
RuntimeState.schemaVersion: 1. - Adds runtime objective, policy, task, worker, artifact ref, evaluation, reward, and integration candidate schema types.
- Adds
createRuntimeState(),parseRuntimeState(), andbuildRunContractRuntimeRefs(). - Validates top-level and nested records, enum values, timestamps, required strings, optional declared fields, arrays, and artifact refs.
- Defines
test/runtime-state.test.ts- Covers initialization, unsupported newer schemas, malformed timestamps/refs, invalid enums, malformed optional fields, blank constructor
runId, RunContract-visible refs, and representative success/repair states.
- Covers initialization, unsupported newer schemas, malformed timestamps/refs, invalid enums, malformed optional fields, blank constructor
README.md- Documents the separately versioned runtime state boundary and fail-closed newer-version behavior.
Why this is correct
The parser now acts as a real schema boundary rather than a shallow cast: malformed declared schema surfaces fail closed, unknown newer schema versions are not interpreted optimistically, and the constructor no longer creates state that the parser would reject. The RunContract helper exposes only the documented objective, policy-selection, and evaluation artifact refs.
The latest rebase does not appear to alter the approved runtime-state behavior; it only resolves branch freshness and README adjacency.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 178 changed lines, below the configured 200-line review threshold.
- Ilchul review harness gate: PASS — STANDARD risk, no blocking findings.
- Revision explanation: present for the current head
32686594f675996dfc215ee1775e3d5611e9b754. - Inspected files:
src/domain/runtime-state.ts,test/runtime-state.test.ts,README.md. - PR evidence also reports
npm run check,npm run verify, andgit diff --checkpassing after the rebase.
Remaining risks and approval rationale
Remaining risks are follow-up scope rather than blockers: event replay, worker execution, storage migration, and runtime mutation are intentionally not implemented in this slice. Approval is justified because the PR’s bounded schema/versioning contract is implemented, tested, documented, and the prior kapi-agent blocking issues are closed.
Blocking issues
None.
Warnings / risks
src/domain/runtime-state.ts: timestamp validation still usesDate.parse(), which is permissive. This is acceptable for the current schema boundary, but stricter ISO timestamp validation may be worth adding before persisted runtime state becomes externally produced or migration-sensitive.- Runtime behavior beyond schema validation remains follow-up work: event replay, worker lifecycle mutation, storage migration, and live execution are not covered by this PR.
Suggestions
- Consider adding a small future test for rejecting non-ISO-but-parseable timestamps if the project wants a stricter timestamp contract.
- As the schema grows, consider splitting validators into smaller schema modules or table-driven validators to keep
runtime-state.tsmaintainable.
Looks good
- The parser now validates nested declared surfaces instead of relying on unsafe casts.
- Unsupported newer schema versions fail closed with a distinct reason.
createRuntimeState()enforces requiredrunIdand timestamp inputs.- RunContract-facing refs are intentionally narrowed to objective, policy-selection, and evaluation artifacts.
- Tests directly cover the previously requested regression cases.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 178 changed lines < 200.
- Revision-explanation status: PASS — current-head explanation was provided.
- Ilchul review harness: PASS — STANDARD risk, no blocking findings.
- Local/CI evidence supplied:
npm run check,npm run verify, andgit diff --checkpass.
Engine: pi
1 participant
Summary
RuntimeStateschema for the objective-driven learning runtime.Linked issue
Closes #185
Refs #167
Problem
The objective-driven learning runtime roadmap needs a concrete state contract before event replay, worker execution, repair, and reward loops can safely build on it. Without a versioned schema and compatibility policy, later runtime slices could persist incompatible state or let unknown newer data be interpreted optimistically.
Options considered
Selected approach
Implementation by file/surface
src/domain/runtime-state.tsRuntimeState.schemaVersion: 1and runtime schema interfaces.createRuntimeState,parseRuntimeState, andbuildRunContractRuntimeRefs.test/runtime-state.test.tsREADME.mdWhy this fixes it
#185 asks for schema list, version/migration policy, RunContract-to-runtime artifact refs, unknown newer schema behavior, and backward compatibility expectations. This PR encodes those as a versioned additive schema with a parser that rejects unsupported newer versions rather than guessing.
QA / Verification
npm ci— passnpx tsx --test test/runtime-state.test.ts— pass, 4 testsnpm run check— passnpm run check:unused— passnpm run quality:budgets— pass with existing non-failing warning:code_smells=55 > 20; coverage not configurednpm run verify— pass, 509 passed / 11 skipped; same existing non-failing quality warninggit diff --check— passAnomalies observed
quality:budgetswarning remains; this branch reportscode_smells=55 > 20and coverage is not configured.Risks / Follow-up
kapi-agent review expectations and current-head merge gate
kapi-agentapproval and successfulkapi-agent/reviewcheck before merge.