feat: define agent adapter contract#229
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:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR defines the agent adapter contract slice for issue #188. The semantic scope is bounded: it adds a typed domain contract, a default Codex/Pi/Claude Code compatibility matrix, validation tests, and a small README note. It does not implement live worker dispatch, heartbeat loops, or CLI-specific runtime behavior. There was no prior kapi-agent blocking review for this head.
What changed
- Added
src/domain/agent-adapter.tswith:AgentAdapterContract- capability/substrate/report/readiness support types
- fail-closed
validateAgentAdapterContract DEFAULT_AGENT_ADAPTER_MATRIX
- Added
test/agent-adapter.test.tscovering valid defaults and missing guarantee failures. - Updated
README.mdto document the adapter-neutral worker execution boundary.
Why this is correct
The change makes the adapter boundary executable without coupling the domain layer to a specific CLI. Required readiness, health, report, substrate, and compatibility expectations are now explicit and validated. The implementation stays product-neutral and avoids pulling runtime/process/GitHub-specific behavior into the domain layer.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 145 changed lines, below the 200-line semantic review threshold.
- Revision-explanation status: not required, but found.
- Ilchul review harness: PASS — STANDARD risk, no blocking findings.
- Inspected files:
src/domain/agent-adapter.ts,test/agent-adapter.test.ts,README.md.
Remaining risks and approval rationale
Remaining risk is limited to future integration: this contract is not yet wired into live adapter launch/dispatch/heartbeat flows. That is explicitly documented as follow-up and is appropriate for this bounded contract PR. Approval is justified because the gates pass, the scope is reviewable, tests cover the new fail-closed behavior, and no blocking correctness or safety issue remains.
Blocking issues
None.
Warnings / risks
src/domain/agent-adapter.ts: compatibility validation is intentionally lightweight text matching. That is acceptable for this contract slice, but future live adapter registration may need stronger capability negotiation than notes-based validation.- This does not implement live adapter behavior; later runtime phases must ensure concrete adapters actually satisfy this contract.
Suggestions
- Consider adding future tests for duplicate/unknown report field handling if runtime consumers depend on exact report schemas.
- Consider formatting the long
compatibilityNotesarrays across multiple lines in a follow-up cleanup for readability.
Looks good
- The domain layer remains free of GitHub/Ragna/kapi-agent coupling.
- Validation fails closed for missing readiness nonce, health behavior, report fields, substrates, and compatibility notes.
- Tests are focused and map directly to the new contract behavior.
- README is updated alongside the behavior/documented boundary change.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 145 changed lines.
- Revision-explanation status: not required; explanation found.
- Ilchul harness gate: PASS — STANDARD risk, no blocking findings.
- PR evidence also reports targeted test, typecheck, unused check, quality budgets, verify, and
git diff --checkpassing.
Engine: pi
588a362 to
e074d69
Compare
|
@kapi-agent review --force 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 agent adapter contract slice for issue #188. The semantic scope remains bounded: it adds a typed domain contract, a default Codex/Pi/Claude Code compatibility matrix, validation tests, and a small README note. It does not implement live dispatch, heartbeat loops, or CLI-specific adapter runtime behavior. kapi-agent previously approved this change; the current revision rebases the same approved slice onto the updated dev base.
What changed
- Added
src/domain/agent-adapter.tswithAgentAdapterContract, substrate/capability/report/readiness types, fail-closed validation, and the default adapter matrix. - Added
test/agent-adapter.test.tsfor valid default contracts and missing guarantee failures. - Updated
README.mdto document the adapter-neutral worker execution boundary. - Current revision explanation states the head was rebased after PR #228 with no intentional source changes beyond replaying this slice.
Why this is correct
The contract makes readiness, health, report, substrate, and compatibility expectations explicit without coupling the domain layer to a specific CLI. The validation fails closed for missing core guarantees, including nonce-equivalent readiness, health behavior, report fields, and compatibility notes. The implementation stays domain-level and product-neutral, matching the stated bounded goal.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 145 changed lines, below the 200-line semantic review threshold.
- Ilchul review harness: PASS — STANDARD risk, neutral conclusion, no blocking findings.
- Revision explanation status: not required now, and found.
- Inspected files:
src/domain/agent-adapter.ts,test/agent-adapter.test.ts, and README diff/context.
Remaining risks and approval rationale
Remaining risks are limited and non-blocking: this is a contract/matrix slice, not live adapter probing or runtime dispatch. Later phases still need to wire this contract into worker execution and validate concrete adapter behavior. Approval is justified because the change is bounded, tested, documented, passes all gates, and no blocking correctness or safety issue remains.
Blocking issues
- None.
Warnings / risks
src/domain/agent-adapter.ts: compatibility validation is intentionally lightweight text matching. That is acceptable for this contract slice, but future live adapter registration should prefer structured compatibility declarations over regex note checks.- Runtime integration remains follow-up work; this PR does not prove actual Codex/Pi/Claude Code launch, capture, readiness, or report behavior.
Suggestions
- In a later integration PR, add concrete adapter registration/probing tests that validate real readiness markers and worker report parsing.
- Consider replacing compatibility-note regex checks with structured compatibility fields once the matrix is consumed by runtime code.
Looks good
- The domain contract is explicit and avoids importing runtime/process/GitHub-specific concerns.
- Validation fails closed for the important execution guarantees.
- Tests cover both the default matrix and a deliberately invalid adapter.
- README is aligned with the new boundary and keeps implementation expectations clear.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 145 changed lines < 200.
- Revision-explanation status: not required now; explanation found.
- Ilchul review harness gate: PASS — STANDARD risk, no blocking findings.
- Author-provided evidence also reports
npm run check,npm run verify, andgit diff --checkpassing after rebase.
Engine: pi
Summary
AgentAdapterContractfor portable worker adapters.Linked issue
Closes #188
Refs #167
Problem
The objective-driven learning runtime needs to run different worker tools through one substrate boundary, but the contract for adapter capabilities, readiness proof, health behavior, and worker reports was not explicit. Without this, future runtime phases could accidentally couple core scheduling to a single CLI or accept workers that cannot produce enough evidence for task completion.
Options considered
Selected approach
Implementation by file/surface
src/domain/agent-adapter.tsAgentAdapterContract.DEFAULT_AGENT_ADAPTER_MATRIXfor Codex, Pi, and Claude Code compatible workers.test/agent-adapter.test.tsREADME.mdWhy this fixes it
#188 asks for an AgentAdapter contract, ExecutionSubstrate contract, capability matching/readiness/health expectations, structured worker report contract, and compatibility notes for Codex/Pi/Claude Code. This PR makes those constraints executable in the domain layer and documents the public boundary, while leaving concrete worker dispatch implementation to later runtime phases.
QA / Verification
npm ci— passnpx tsx --test test/agent-adapter.test.ts— pass, 2 testsnpm run check— passnpm run check:unused— passnpm run quality:budgets— pass with existing non-failing warnings:code_smells=51 > 20; coverage not configurednpm run verify— pass, 507 passed / 11 skipped; same existing non-failing quality warninggit diff --check— passAnomalies observed
quality:budgetswarning remains:code_smells=51 > 20; this PR does not change that budget.Risks / Follow-up
kapi-agent review expectations and current-head merge gate
kapi-agentapproval and successfulkapi-agent/reviewcheck before merge.