Skip to content

fix: gate kapi approval after review check#158

Merged
devkade merged 1 commit into
devfrom
fix/kapi-approval-gate-check-run
May 16, 2026
Merged

fix: gate kapi approval after review check#158
devkade merged 1 commit into
devfrom
fix/kapi-approval-gate-check-run

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 16, 2026

Summary

  • Move the formal kapi-agent approval gate off early pull_request events.
  • Re-run the formal gate after kapi-agent/review completes via check_run, while keeping pull_request_review events for review submitted/edited/dismissed re-evaluation.
  • Add short polling for the kapi-agent/review state before failing, so race-prone missing/pending checks do not leave stale failed required checks.

Why

PR #150 exposed a race where require formal kapi-agent approval ran before kapi-agent/review existed on the current head, leaving stale failed required checks that blocked merge even after later success.

Test Plan

  • ruby -e 'require "yaml"; YAML.load_file(".github/workflows/kapi-agent-formal-approval-gate.yml")'
  • extracted GitHub Script wrapped in async function: node --check
  • git diff --check .github/workflows/kapi-agent-formal-approval-gate.yml
  • npm run verify

Follow-up to #150.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 16, 2026

@kapi-agent review

Revision explanation for current head 3361a1e:

What changed:

  • Removed the early pull_request trigger from .github/workflows/kapi-agent-formal-approval-gate.yml.
  • Added check_run: completed as the primary trigger and filtered it to kapi-agent/review.
  • Kept pull_request_review triggers so submitted/edited/dismissed formal reviews still re-evaluate the gate.
  • Added PR-number extraction for both pull_request_review and check_run payloads.
  • Added a bounded wait loop before failing on missing/pending kapi-agent/review evidence.

Why this is needed:

  • PR refactor: extract authored artifact metadata helpers #150 showed stale failed require formal kapi-agent approval runs caused by the formal gate firing before kapi-agent/review existed on the head.
  • The new flow makes the formal approval gate run after review/check evidence exists, reducing race-created stale failures while keeping the same required checks.

Verification rerun:

  • YAML parse passed via Ruby YAML.load_file.
  • Extracted GitHub Script passed node --check when wrapped as an async function.
  • git diff --check .github/workflows/kapi-agent-formal-approval-gate.yml passed.
  • npm run verify passed.

Copy link
Copy Markdown

@kapi-agent kapi-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal is to fix the formal kapi-agent approval gate race where the gate could run before kapi-agent/review existed for the current head, leaving stale failed required checks. The semantic scope is limited to .github/workflows/kapi-agent-formal-approval-gate.yml. There is no prior kapi-agent review on this PR, so no unresolved ping-pong feedback remains.

What changed

The workflow now:

  • Runs on check_run completion for kapi-agent/review.
  • Keeps pull_request_review events for review submitted/edited/dismissed re-evaluation.
  • Resolves the PR number from either review or check-run payloads.
  • Polls briefly for the current-head kapi-agent/review check/status before failing.
  • Continues to require a formal PR review by kapi-agent on the current head, not an approval-shaped comment.

Why this is correct

Moving the gate off early PR lifecycle events avoids evaluating before the review check exists. Re-triggering from the completed kapi-agent/review check aligns the formal gate with the artifact it depends on, while retaining review-event re-evaluation for approval state changes. The current-head review SHA comparison and check/status lookup preserve the intended safety contract.

Evidence

  • Verifier gate: PASS.
  • Size gate: PASS, 69 changed lines under the 200-line limit.
  • Revision explanation status: not required; explanation present.
  • Ilchul harness gate: PASS, neutral/comment with 0 blocking findings.
  • CI/local evidence provided: npm ci && npm run verify exited 0.
  • Inspected file: .github/workflows/kapi-agent-formal-approval-gate.yml.

Remaining risks and approval rationale

Remaining risk is low and mainly operational: GitHub event/check-run association behavior can be subtle, but the implementation is narrowly scoped and keeps explicit current-head validation. Approval is justified because all gates pass, the PR is small, and I found no blocking correctness, security, or workflow-contract issue.

Blocking issues

None.

Warnings / risks

  • .github/workflows/kapi-agent-formal-approval-gate.yml: the workflow still depends on check_run.pull_requests[0] being present for the kapi-agent/review check-run path. That is acceptable for the expected PR check flow, but if the app ever emits check runs without PR association, the gate will fail with a clear diagnostic.

Suggestions

  • Consider adding a short comment in the workflow explaining why pull_request events are intentionally omitted, since this is a race-prevention contract and may otherwise look accidental later.

Looks good

  • The gate still distinguishes formal PR reviews from approval-shaped comments.
  • The latest kapi-agent review is still required to be APPROVED and tied to the current head SHA.
  • Polling is bounded and only waits for missing/pending check state, avoiding an indefinite workflow hang.
  • The change is focused on the approval-gate workflow and does not broaden permissions.

Verification notes

  • Verifier gate status: PASS, npm ci && npm run verify exited 0.
  • Size gate status: PASS, 69 changed lines < 200.
  • Revision-explanation status: not required; found.
  • Ilchul review harness: PASS, no blocking findings.
  • Local inspection covered the workflow logic in .github/workflows/kapi-agent-formal-approval-gate.yml.

Engine: pi

@devkade devkade merged commit d312be1 into dev May 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant