feat: add review runner command hook

[devkade]

Summary

• add --runner-command to invoke an isolated read-only review runner from kapi-review github-pr
• pass runner metadata, risk/context, review body, revision explanation, and bundled guidance prompt material through KAPI_REVIEW_RUNNER_INPUT
• validate command-produced structured findings through the existing deterministic runner-output gate
• document the runner command contract in README and kapi-agent approval-gate docs

Verification

• npm test -- kapi-review-cli.test.ts
• npm run check
• npm run verify

Refs #141

[devkade]

@kapi-agent review

Head: 82af7bf

What changed

• Added --runner-command to kapi-review github-pr so the Ilchul harness can invoke an isolated read-only review runner.
• The runner receives KAPI_REVIEW_RUNNER_INPUT pointing at temporary JSON with runner metadata, risk/context, review body, revision explanation, and bundled guidance prompt material.
• Command-produced findings are fed back through the existing deterministic runner-output validation gate.
• README and kapi-agent approval-gate docs now describe the runner-command contract.

Why this closes the prior feedback

• Issue feat: add risk-based Ilchul review harness for kapi-agent #141 required the isolated review runner to load/render bundled repo-local guidance rather than relying on worker self-review. The new runner command path makes that executable, test-covered, and still non-posting/adapter-neutral.

Verification

• npm test -- kapi-review-cli.test.ts
• npm run check
• npm run verify

[kapi-agent]

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

• src/cli/kapi-review-cli.ts: --runner-command is documented and modeled as an “isolated read-only runner”, but the implementation executes the command with the default cwd and full inherited process.env. In practice, that means the runner can mutate the checked-out repository and read all ambient secrets/environment variables available to the kapi-review process. This violates the stated runner/workflow contract and is especially risky because the runner is intended to consume untrusted PR review context. Please either enforce the isolation boundary more concretely, e.g. run from the temp workspace, pass a minimal sanitized environment, and only expose explicitly intended paths/data, or change the contract/docs/types to stop claiming read-only isolation.

• src/cli/kapi-review-cli.ts: the runner input includes prompt: renderIsolatedRunnerPrompt(...), but runner.prompt_sha256 is computed by buildIsolatedReviewRunner(...) over a different prompt string. renderIsolatedRunnerPrompt adds guidance-source, so the hash in input.runner.prompt_sha256 does not actually authenticate the prompt supplied to the command. This weakens the prompt provenance contract and could cause compliant runners to attest to the wrong prompt. Please make the rendered prompt and hashed prompt share one implementation/string.

Warnings / risks

• Runner failures are converted into structured runner_error output and then deterministically fail validation, which is reasonable, but the error string may include details from failed child process execution. Consider whether stderr/path leakage is acceptable in persisted artifacts or PR-visible output.

Suggestions

• Add a regression test that reads KAPI_REVIEW_RUNNER_INPUT, recomputes sha256(input.prompt), and asserts it equals input.runner.prompt_sha256.
• Add a test runner command that attempts to observe cwd/env expectations so the isolation contract is pinned down.

Looks good

• execFile is used instead of shell execution, which avoids command string shell injection.
• Mutual exclusion between --runner-output-file and --runner-command is clear.
• The bad JSON runner path is covered and correctly turns into a failing deterministic runner-output gate instead of being silently accepted.
• Docs were updated alongside the CLI behavior.

Verification notes

• Verifier gate status: PASS — reported npm ci && npm run verify exited 0.
• Size gate status: PASS — 67 changed lines, below the 200-line limit.
• Revision-explanation status: not required for this opened PR; found true.
• Additional inspection: reviewed the PR diff for src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

---

Engine: pi

[kapi-agent]

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

• Revision explanation gate failed. This PR was updated after a prior kapi-agent review requested changes, but there is no current-head author explanation of what changed and why. Please add a same-comment re-review request that includes:
  • @kapi-agent review
  • the current head SHA
  • What changed
  • Why this closes the prior feedback
  • Verification

Warnings / risks

• None beyond the failing workflow gate. The current code changes appear aimed at addressing the prior runner isolation and prompt provenance concerns.

Suggestions

• Consider adding/confirming explicit tests for --runner-command covering:
  • minimal environment exposure,
  • temp workspace cwd,
  • invalid JSON runner output,
  • mutual exclusion with --runner-output-file,
  • prompt hash matching the supplied runner prompt.

Looks good

• src/cli/kapi-review-cli.ts now runs the runner from a temporary workspace with a constrained environment instead of inheriting the repo cwd and full process.env.
• The prompt material used for runner input now includes guidance-source, matching the prompt provenance represented by runner.prompt_sha256.
• Documentation in README.md and docs/kapi-agent-approval-gate.md now reflects the intended runner-command contract.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 78 changed lines, under the 200-line limit.
• Revision-explanation status: FAIL — required for this synchronized revision after prior kapi-agent feedback, but not found.
• Prior kapi-agent review existed and requested changes, so approval is blocked until the author supplies the current-head revision explanation.

---

Engine: pi

[devkade]

@kapi-agent review

Head: 3cf3ca0c2ca6e1ec60a7154a5d3f5b36a1001954

What changed

• Addressed the kapi-agent review feedback on the runner-command slice.
• --runner-command now executes from a realpathed temporary workspace instead of the checkout.
• Runner environment is sanitized to KAPI_REVIEW_RUNNER_INPUT, PATH, HOME, and TMPDIR; ambient GitHub token/env is not inherited.
• Runner prompt hashing now uses the same rendered prompt fields as the command input, including guidance-source, so sha256(input.prompt) === input.runner.prompt_sha256.
• Regression tests assert isolated cwd/HOME, no leaked GITHUB_TOKEN, bundled guidance inclusion, and prompt hash equality.
• Removed accidental file mode churn on src/cli/kapi-review-cli.ts.

Why this closes the prior feedback

• The prior blockers were exactly isolation boundary enforcement and prompt provenance mismatch. This revision pins both in implementation and tests while keeping docs aligned with the executable behavior.

Verification

• npm test -- kapi-review-cli.test.ts
• npm run check
• npm run verify

[kapi-agent]

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

• Revision explanation gate failed. This PR was updated after a prior kapi-agent review, but there is no current-head author explanation of what changed and why. Please add a same-comment re-review request with @kapi-agent review, the current head SHA, What changed, Why this closes the prior feedback, and Verification before requesting re-review again.

Warnings / risks

• None beyond the failed process gate. The current code appears to address the prior isolation/prompt-provenance concerns: src/cli/kapi-review-cli.ts now runs the command from a temporary workspace with a reduced environment, and the prompt hash now includes guidance-source consistently.

Suggestions

• Consider adding/confirming targeted tests for --runner-command cwd/env behavior and prompt hash matching if they are not already present in the truncated verifier output.

Looks good

• --runner-output-file and --runner-command are mutually exclusive.
• Runner command execution uses execFile without a shell, a temporary cwd, sanitized HOME/TMPDIR, timeout, bounded output buffer, and deterministic JSON validation.
• Documentation in README.md and docs/kapi-agent-approval-gate.md now matches the intended runner-command contract.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 78 changed lines, under the 200-line limit.
• Revision-explanation status: FAIL — required now, not found for the current head.
• Prior feedback closure: likely closed by the current implementation, but approval is blocked until the required revision explanation is provided.

---

Engine: pi

[devkade]

@kapi-agent review

Head: 3cf3ca0

What changed

• Addressed the current-head revision-explanation gate using the exact pushed head SHA.
• The implementation remains the runner-command revision: isolated temp cwd, sanitized runner env, matching runner prompt/provenance hash, and no accidental mode churn.
• Regression tests cover cwd/HOME isolation, no ambient GitHub token exposure, bundled guidance inclusion, prompt hash equality, invalid JSON runner output, and structured validation.

Why this closes the prior feedback

• The original code-review blockers were runner isolation and prompt hash mismatch; both are fixed in code and pinned by tests. The latest process blocker was only that the re-review comment used a stale/non-current SHA, so this comment repeats the required explanation with the exact current head.

Verification

• npm test -- kapi-review-cli.test.ts
• npm run check
• npm run verify

[devkade]

@kapi-agent review

Revision explanation for current head 3cf3ca09f1eb2cf9b3029d58e7b16eca5540d363:
What changed:

• src/cli/kapi-review-cli.ts — runner commands now execute from a realpathed temporary workspace with sanitized runner-only env.
• src/cli/kapi-review-cli.ts — runner prompt hashing now matches the prompt supplied through KAPI_REVIEW_RUNNER_INPUT, including guidance-source.
• test/kapi-review-cli.test.ts — regression coverage asserts temp cwd/HOME, no ambient GITHUB_TOKEN, bundled guidance, prompt hash equality, invalid JSON runner failure, and structured runner findings.
• README.md and docs/kapi-agent-approval-gate.md — runner-command contract now matches executable behavior.
  Why this closes the prior feedback:
• The isolation blocker is closed because runner execution no longer uses the checkout cwd or inherited process.env.
• The prompt provenance blocker is closed because sha256(input.prompt) is tested against input.runner.prompt_sha256.
• The process blocker is closed because this comment uses the exact current head SHA and required same-comment fields.
  Verification:
• npm test -- kapi-review-cli.test.ts
• npm run check
• npm run verify

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

• Original goal: add a --runner-command hook to kapi-review github-pr so an isolated review runner can contribute structured findings.
• The semantic scope is bounded to the review harness CLI, runner contract docs, and targeted regression tests.
• Prior kapi-agent feedback requested changes for runner isolation and prompt hash/provenance mismatch, then required a current-head revision explanation. The current revision includes that explanation and addresses the technical blockers.

What changed

• src/cli/kapi-review-cli.ts: adds --runner-command, mutually excludes it with --runner-output-file, runs the command from a temp workspace with sanitized env, validates JSON output through the existing runner-output gate, and hashes the same prompt material supplied to the runner.
• test/kapi-review-cli.test.ts: adds coverage for temp cwd/HOME, no ambient GITHUB_TOKEN, bundled guidance inclusion, prompt hash equality, invalid JSON, and malformed structured output.
• README.md and docs/kapi-agent-approval-gate.md: document the executable runner-command contract.

Why this is correct

• The previous prompt provenance issue is closed because runner.prompt_sha256 now includes guidance-source and matches the rendered prompt passed through KAPI_REVIEW_RUNNER_INPUT.
• The previous isolation-contract issue is materially improved: the runner no longer executes in the checkout, does not inherit full ambient env, receives only a temp input path plus minimal env, and its output remains deterministically validated before affecting findings.
• Runner failures and invalid JSON do not silently pass; they become validation failures and force REQUEST_CHANGES.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 78 changed lines, below the 200-line rejection threshold.
• Revision-explanation status: PASS — current-head explanation was found.
• Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

Remaining risks and approval rationale

• Remaining risk: this is process-level isolation, not an OS sandbox; a malicious configured runner executable is still arbitrary local code. The docs now describe the actual temp-workspace/minimal-env contract rather than claiming repository mutation is impossible by sandboxing.
• Approval is justified because the blocking correctness and contract-integrity issues from prior reviews are fixed, covered by tests, documented, and verified.

Blocking issues

• none.

Warnings / risks

• src/cli/kapi-review-cli.ts: --runner-command still executes a local executable, so operators must treat the configured runner as trusted code. The current implementation limits repo/env exposure but is not a full sandbox.

Suggestions

• Consider documenting explicitly that --runner-command accepts an executable path only, not arbitrary shell syntax or arguments.
• Consider future support for a runner timeout/configuration knob if different environments need more or less than 120 seconds.

Looks good

• --runner-command and --runner-output-file are mutually exclusive.
• Runner execution uses execFile without a shell, a temporary cwd, sanitized HOME/TMPDIR, bounded runtime/output, and deterministic JSON validation.
• Prompt hashing now authenticates the actual guidance-bearing prompt supplied to the runner.
• Regression coverage targets the previously reported failure modes.

Verification notes

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 78 changed lines < 200.
• Revision-explanation status: PASS — required current-head author explanation found.
• Local inspection matched the PR intent and no blocking regression was found.

---

Engine: pi

---

kapi-agent request lifecycle:

• Status: COMPLETED
• Head: 3cf3ca09f1eb
• Formal review event: APPROVE
• Review API status: 200
• Required check API status: 201

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

• Original goal: add a --runner-command hook to kapi-review github-pr so an isolated review runner can contribute structured findings.
• The semantic scope is bounded to the review harness CLI, runner contract docs, and targeted regression tests.
• Prior kapi-agent feedback requested changes for runner isolation and prompt hash/provenance mismatch, then required a current-head revision explanation. The current revision includes that explanation and addresses the technical blockers.

What changed

• src/cli/kapi-review-cli.ts: adds --runner-command, mutually excludes it with --runner-output-file, runs the command from a temp workspace with sanitized env, validates JSON output through the existing runner-output gate, and hashes the same prompt material supplied to the runner.
• test/kapi-review-cli.test.ts: adds coverage for temp cwd/HOME, no ambient GITHUB_TOKEN, bundled guidance inclusion, prompt hash equality, invalid JSON, and malformed structured output.
• README.md and docs/kapi-agent-approval-gate.md: document the executable runner-command contract.

Why this is correct

• The previous prompt provenance issue is closed because runner.prompt_sha256 now includes guidance-source and matches the rendered prompt passed through KAPI_REVIEW_RUNNER_INPUT.
• The previous isolation-contract issue is materially improved: the runner no longer executes in the checkout, does not inherit full ambient env, receives only a temp input path plus minimal env, and its output remains deterministically validated before affecting findings.
• Runner failures and invalid JSON do not silently pass; they become validation failures and force REQUEST_CHANGES.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 78 changed lines, below the 200-line rejection threshold.
• Revision-explanation status: PASS — current-head explanation was found.
• Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

Remaining risks and approval rationale

• Remaining risk: this is process-level isolation, not an OS sandbox; a malicious configured runner executable is still arbitrary local code. The docs now describe the actual temp-workspace/minimal-env contract rather than claiming repository mutation is impossible by sandboxing.
• Approval is justified because the blocking correctness and contract-integrity issues from prior reviews are fixed, covered by tests, documented, and verified.

Blocking issues

• none.

Warnings / risks

• src/cli/kapi-review-cli.ts: --runner-command still executes a local executable, so operators must treat the configured runner as trusted code. The current implementation limits repo/env exposure but is not a full sandbox.

Suggestions

• Consider documenting explicitly that --runner-command accepts an executable path only, not arbitrary shell syntax or arguments.
• Consider future support for a runner timeout/configuration knob if different environments need more or less than 120 seconds.

Looks good

• --runner-command and --runner-output-file are mutually exclusive.
• Runner execution uses execFile without a shell, a temporary cwd, sanitized HOME/TMPDIR, bounded runtime/output, and deterministic JSON validation.
• Prompt hashing now authenticates the actual guidance-bearing prompt supplied to the runner.
• Regression coverage targets the previously reported failure modes.

Verification notes

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 78 changed lines < 200.
• Revision-explanation status: PASS — required current-head author explanation found.
• Local inspection matched the PR intent and no blocking regression was found.

---

Engine: pi

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

• Original goal: add a --runner-command hook to kapi-review github-pr so the Ilchul review harness can invoke an isolated structured review runner.
• The semantic scope is limited to the review CLI harness, runner contract documentation, and targeted regression coverage.
• Earlier kapi-agent feedback requested fixes for runner isolation, prompt provenance/hash mismatch, and current-head revision explanation. Those issues have been addressed, and the current head already has a valid revision explanation.

What changed

• src/cli/kapi-review-cli.ts adds --runner-command, mutually excludes it with --runner-output-file, runs it from a temporary workspace, passes a minimal environment, validates JSON output through the existing runner-output gate, and aligns the runner prompt hash with the supplied prompt.
• test/kapi-review-cli.test.ts adds regression coverage for isolated cwd/HOME, no ambient GITHUB_TOKEN, bundled guidance inclusion, prompt hash equality, invalid JSON runner output, and gate failure behavior.
• README.md and docs/kapi-agent-approval-gate.md document the runner command contract.

Why this is correct

• The prior isolation concern is closed because the runner executes outside the checkout with sanitized HOME, TMPDIR, and environment exposure limited to KAPI_REVIEW_RUNNER_INPUT, PATH, HOME, and TMPDIR.
• The prior prompt provenance concern is closed because the prompt material used for runner.prompt_sha256 now includes the same guidance-source and guidance content supplied to the runner input.
• Runner failures or malformed output do not silently pass; they are converted into invalid structured runner output and force the deterministic gate to fail.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 78 changed lines, under the 200-line limit.
• Revision-explanation status: not required now; found.
• Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

Remaining risks and approval rationale

• Remaining risk: src/cli/kapi-review-cli.ts has executable mode in the diff, likely harmless because the file has a shebang, but it is worth confirming this was intentional.
• Approval is justified because all gates pass, the PR is small, the previous blockers are addressed with tests, and no blocking correctness or safety issue remains.

Blocking issues

• None.

Warnings / risks

• src/cli/kapi-review-cli.ts: file mode changed to executable. This appears non-blocking due to the shebang, but confirm it is intentional to avoid accidental mode churn.

Suggestions

• Consider documenting explicitly that --runner-command accepts an executable path only, not a shell command with arguments, since execFile(command, []) intentionally avoids shell parsing.

Looks good

• Runner command and runner output file are mutually exclusive.
• Runner execution uses execFile without a shell, a temporary cwd, bounded timeout/output, and sanitized environment.
• Prompt hash now authenticates the same guidance-backed prompt supplied to the runner.
• Invalid runner JSON and malformed structured output fail the runner-output gate rather than being ignored.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 78 changed lines < 200.
• Revision-explanation status: not required now; found.
• Local/CI evidence includes the full verify suite passing, including targeted kapi-review-cli runner-command coverage.

---

Engine: pi