ci: require formal kapi-agent approval#124
Conversation
|
Revision note after initial workflow run:
Verification already run locally on this focused branch:
|
![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: APPROVE
Final approval summary
Review journey
Original goal: add a GitHub-enforced gate so Kapi PRs cannot merge based only on an approval-shaped comment. The semantic scope is limited to one CI workflow plus documentation for requiring formal current-head kapi-agent approval and the kapi-agent/review signal. No prior kapi-agent review existed for this PR, so there was no feedback/fix ping-pong to verify.
What changed
- Added
.github/workflows/kapi-agent-formal-approval-gate.yml. - Updated
README.mdto point branch/ruleset enforcement at both required checks. - Added
docs/kapi-agent-approval-gate.mdexplaining the formal approval contract and PR #110 incident rule.
Why this is correct
The workflow checks the latest formal PR review by kapi-agent, requires it to be APPROVED, ties it to the current head SHA, requires kapi-agent/review success, rejects drafts, and explicitly ignores approval-shaped comments. That matches the intended artifact/workflow contract: comments can explain reviews, but only GitHub review state plus the bot check can satisfy merge enforcement.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 114 changed lines, below the 200-line limit.
- Revision explanation status: not required; no prior kapi-agent review existed.
- Inspected files:
.github/workflows/kapi-agent-formal-approval-gate.yml,README.md, anddocs/kapi-agent-approval-gate.md.
Remaining risks and approval rationale
Remaining risk is operational: the repository ruleset must actually require both kapi-agent/review and require formal kapi-agent approval after merge. The code and docs correctly define the gate, the PR is small, verification passed, and no blocking issue remains, so approval is justified.
Blocking issues
None.
Warnings / risks
.github/workflows/kapi-agent-formal-approval-gate.yml: enforcement depends on branch protection/rulesets requiring this new check pluskapi-agent/review; without that repository setting, the workflow can report failure but cannot block merges..github/workflows/kapi-agent-formal-approval-gate.yml: if GitHub represents the app reviewer login differently in this repo, for example as a bot-suffixed login, thereviewer = 'kapi-agent'constant may need adjustment. The current implementation is consistent with the documented contract.
Suggestions
- Consider adding a future small test/harness fixture for the github-script logic if this gate grows more complex.
- Consider documenting the expected exact GitHub review actor login if it ever differs from
kapi-agent.
Looks good
- The workflow checks formal PR review state rather than parsing comment text as approval.
- The review is tied to
pull.head.sha, preventing stale approvals after synchronize. - The required
kapi-agent/reviewcheck/status remains part of the contract. - Documentation clearly explains the incident rule: approval-shaped comments are not merge approval.
Verification notes
Verifier gate status: PASS — npm ci && npm run verify exited 0.
Size gate status: PASS — 114 changed lines, below the 200-line limit.
Revision-explanation status: not required; no prior kapi-agent review exists for this PR.
PR evidence reviewed: workflow file, README update, approval-gate documentation, and provided CI/verifier output.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Revision explanation gate failed. This PR was updated after a prior kapi-agent review, but the PR body or author comments do not explain what changed and why for this current revision. Per policy, please add a review response explaining what changed since the prior kapi-agent approval and why, then request re-review.
Warnings / risks
- None blocking from the inspected diff itself. The workflow/documentation scope remains small and focused.
Suggestions
- In the revision response, include the head SHA or a concise bullet list of the synchronize changes so reviewers can distinguish the current revision from the already-approved one.
Looks good
- The size gate passes: 133 changed lines, under the 200-line limit.
- The formal approval gate design is directionally sound: it requires a formal
kapi-agentPR review on the current head, checkskapi-agent/review, rejects drafts, and treats approval-shaped comments as non-authoritative. - Documentation in
README.mdanddocs/kapi-agent-approval-gate.mdclearly describes the branch/ruleset requirements and the comment-vs-review distinction.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 133 changed lines < 200.
- Revision-explanation status: FAIL — required now, but not found for the current synchronized revision.
- PR evidence inspected: workflow addition
.github/workflows/kapi-agent-formal-approval-gate.yml, README update, approval-gate documentation, prior review history, and provided verification output.
Engine: pi
|
Revision note:
Expected current state: this push invalidated the prior approval by changing the head SHA, so |
|
Revision explanation for current head
Verification after this revision:
|
|
@kapi-agent review |
kapi-agent reviewVerdict: REQUEST_CHANGES Blocking issues
Warnings / risks
Suggestions
Looks good
Verification notes
Engine: kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
.github/workflows/kapi-agent-formal-approval-gate.yml: approval-shaped comments are not actually ignored. The workflow counts matching comments and then appends that count todiagnostics; any diagnostic causescore.setFailed(...). That means a PR with a valid formal current-headkapi-agentapproval and successfulkapi-agent/reviewcan still be blocked forever merely because an old comment contains:This contradicts the stated contract in the PR and docs: comments should be non-authoritative, not a failing condition. Please remove this from failing diagnostics, or report it only in the summary without failing the gate.## kapi-agent review **Verdict:** APPROVE
Warnings / risks
- The workflow only fetches
reviews(last: 100)andcomments(last: 100). This is probably fine for normal PRs, but if the repository ever has very noisy review history, pagination would be more robust.
Suggestions
- Consider adding a small fixture/unit-style test for the gate script logic if this workflow grows further, especially for: valid approval + approval-shaped comment should pass.
Looks good
- Size gate passes: 133 changed lines, under the 200-line limit.
- Revision explanation was provided and clearly explains the REST-to-GraphQL change and current-head approval invariant.
- The main enforcement model is sound: require latest formal
kapi-agentreview to beAPPROVED, tie it topull.head.sha, requirekapi-agent/reviewsuccess, and reject drafts. - Documentation clearly distinguishes formal GitHub review state from comment text.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 133 changed lines, below the 200-line limit.
- Revision-explanation status: PASS — current-head revision explanation found.
- Local inspection focused on
.github/workflows/kapi-agent-formal-approval-gate.yml,README.md, anddocs/kapi-agent-approval-gate.md.
Engine: pi
|
@kapi-agent review Revision explanation: What changed: fixed the blocking issue from the latest kapi-agent review. Approval-shaped comments are still counted and shown in the GitHub Actions summary, but they are no longer appended to failing diagnostics and therefore cannot block a PR that otherwise has a valid current-head formal approval plus successful Why: the intended contract is that comments are non-authoritative. They should be ignored for approval decisions, not treated as a failure condition. Verification: |
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Revision explanation gate failed. This PR was updated after prior kapi-agent reviews, including a prior approval and a later changes-requested review, but the current PR body or author comments do not explain what changed and why for this latest synchronized revision. Per policy, please add a revision response explaining the current-head changes and why they close the prior feedback, then request re-review.
Warnings / risks
.github/workflows/kapi-agent-formal-approval-gate.yml: the remaining workflow risk is pagination/noise sensitivity. It reads onlyreviews(last: 100)andcomments(last: 100). That is probably acceptable for this small repo, but very noisy PRs could hide older relevant data if this grows.
Suggestions
- In the revision response, include the current head SHA and a short bullet list of what changed since the last kapi-agent review, especially that approval-shaped comments are now only reported in the summary and no longer fail the gate.
Looks good
- The previous blocking workflow issue appears addressed in the provided current file: approval-shaped comments are counted and appended to
summaryItems, but they are not added todiagnostics, so they should not fail an otherwise valid formal approval gate. - The workflow continues to enforce the important contract: latest formal
kapi-agentreview must beAPPROVED, tied to the current head SHA, withkapi-agent/reviewsuccessful, and draft PRs blocked. - Documentation in
README.mdanddocs/kapi-agent-approval-gate.mdclearly states that comment verdicts are not approval and that branch/ruleset enforcement should require both checks.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 136 changed lines, under the 200-line limit.
- Revision-explanation status: FAIL — required now, but not found for the current synchronized revision.
- Local inspection note: the checkout available to this review did not expose the full PR diff, so this review relies on the provided PR context plus the verification evidence.
Engine: pi
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
|
@kapi-agent review |
kapi-agent reviewVerdict: APPROVE Final approval summaryReview journeyOriginal goal: add a GitHub-enforced merge gate so Kapi PRs cannot merge from approval-shaped comments alone. The semantic scope is limited to one formal approval GitHub Actions workflow plus README/docs updates describing the required checks and PR #110 incident rule. There was review/fix ping-pong:
What changed
Why this is correctThe workflow now requires:
Approval-shaped comments are only reported in the Actions summary and are not appended to failing diagnostics, which closes the prior blocking feedback. Evidence
Remaining risks and approval rationaleRemaining risk: the workflow reads the last 100 reviews/comments/check runs/statuses and does not paginate. That is acceptable for this small focused gate, but could become brittle on extremely noisy PRs. Approval is justified because all required gates pass, the PR is small and focused, the prior blocking issue is fixed, and no blocking correctness/security/workflow-contract issue remains. Blocking issuesNone. Warnings / risks
Suggestions
Looks good
Verification notes
Engine: kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
Original goal: add a GitHub-enforced merge gate so Kapi PRs cannot merge from approval-shaped comments alone. The semantic scope is limited to one formal approval GitHub Actions workflow plus README/docs updates describing the required checks and PR #110 incident rule.
There was review/fix ping-pong:
- An earlier revision was approved.
- A later revision required changes because approval-shaped comments were still treated as failing diagnostics.
- The current revision explains that comments are now counted only for summary visibility and no longer block the gate.
What changed
- Added
.github/workflows/kapi-agent-formal-approval-gate.yml. - Updated
README.mdto document requiring bothkapi-agent/reviewandrequire formal kapi-agent approval. - Added
docs/kapi-agent-approval-gate.mddocumenting the formal review contract and that comment verdicts are non-authoritative.
Why this is correct
The workflow now requires:
- latest formal
kapi-agentPR review exists, - latest formal review is
APPROVED, - review commit OID matches the current PR head SHA,
kapi-agent/reviewcheck/status issuccess,- PR is not draft.
Approval-shaped comments are only reported in the Actions summary and are not appended to failing diagnostics, which closes the prior blocking feedback.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 136 changed lines, below the 200-line limit.
- Revision-explanation status: PASS — author explained what changed and why after prior kapi-agent feedback.
- Inspected files:
.github/workflows/kapi-agent-formal-approval-gate.yml,README.md,docs/kapi-agent-approval-gate.md.
Remaining risks and approval rationale
Remaining risk: the workflow reads the last 100 reviews/comments/check runs/statuses and does not paginate. That is acceptable for this small focused gate, but could become brittle on extremely noisy PRs.
Approval is justified because all required gates pass, the PR is small and focused, the prior blocking issue is fixed, and no blocking correctness/security/workflow-contract issue remains.
Blocking issues
None.
Warnings / risks
.github/workflows/kapi-agent-formal-approval-gate.yml: pagination remains a low-risk limitation. Very noisy PRs or refs with more than 100 relevant reviews/check runs/statuses could require pagination later.
Suggestions
- If this gate grows further, consider extracting the script into a testable module or adding fixture coverage for “valid formal approval + approval-shaped comment still passes.”
Looks good
- The workflow correctly uses formal PR review state rather than comment text as the approval authority.
- Current-head binding via
latestReview.commit?.oid !== pull.head.shaprotects against stale approval after synchronize. - The prior issue is fixed: approval-shaped comments are summarized but no longer fail the gate.
- Documentation clearly explains the required branch/ruleset checks and the comment-vs-review distinction.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 136 changed lines < 200.
- Revision-explanation status: PASS — required and found.
- Local/PR evidence inspected: workflow file, README update, approval-gate docs, prior kapi-agent feedback, and current author revision explanation.
Engine: pi
1 participant
Summary
require formal kapi-agent approvalkapi-agentPR review isAPPROVEDon the current head andkapi-agent/reviewis successCloses #122.
Verification
npm cigit diff --checknpm run verifyRequired GitHub settings after merge
Require both checks in branch protection/rulesets for
dev:kapi-agent/reviewrequire formal kapi-agent approvalComment verdicts such as
## kapi-agent review/**Verdict:** APPROVEare ignored by the workflow.