[Governance] Separate substantive review verdict from Forgejo state #20

Merged
adrian.tkacz merged 1 commit from issue-19-review-verdict into develop 2026-09-26 09:51:17 +00:00
Member

Implements #19. Governance-only correction to the Kernel /deliver-issue and /review-pr commands.

  • Track independent reviewer substantive_verdict separately from persisted forgejo_review_state, platform approval evidence, blocker count, and the shared-author identity limitation.
  • Evaluate readiness from exact-HEAD substantive APPROVE, zero blockers, required local validation/CI, and issue acceptance; Forgejo's author-account COMMENT downgrade does not erase a valid review. Genuine substantive COMMENT remains non-approving and substantive REQUEST_CHANGES still drives remediation.
  • Preserve truthful platform review state and any actually observed formal-approval branch-protection requirement. Do not change routing, dispatch, remediation budget, product code, or parent M5-B policy.

Validation: make check (152 tests and all repository gates); make package-check (wheel/sdist/isolated install); git diff --check. No dependencies or tooling changed, so make audit was not rerun. Issue #6 / PR #18 remain untouched. Owner explicitly authorizes substantive review verdict for this corrective PR if the shared author identity forces Forgejo COMMENT; the actual platform state must still be recorded. Do not merge.

Implements #19. Governance-only correction to the Kernel `/deliver-issue` and `/review-pr` commands. - Track independent reviewer `substantive_verdict` separately from persisted `forgejo_review_state`, platform approval evidence, blocker count, and the shared-author identity limitation. - Evaluate readiness from exact-HEAD substantive `APPROVE`, zero blockers, required local validation/CI, and issue acceptance; Forgejo's author-account `COMMENT` downgrade does not erase a valid review. Genuine substantive `COMMENT` remains non-approving and substantive `REQUEST_CHANGES` still drives remediation. - Preserve truthful platform review state and any actually observed formal-approval branch-protection requirement. Do not change routing, dispatch, remediation budget, product code, or parent M5-B policy. Validation: `make check` (152 tests and all repository gates); `make package-check` (wheel/sdist/isolated install); `git diff --check`. No dependencies or tooling changed, so `make audit` was not rerun. Issue #6 / PR #18 remain untouched. Owner explicitly authorizes substantive review verdict for this corrective PR if the shared author identity forces Forgejo `COMMENT`; the actual platform state must still be recorded. Do not merge.
docs: separate review verdict from Forgejo state
All checks were successful
ci / check (pull_request) Successful in 20s
9517804008
kilo left a comment

PR Review: [Governance] Separate substantive review verdict from Forgejo state (#20)

Repository: Creatidy/creatidy-kernel
Branch: issue-19-review-verdict -> develop
Frozen head: 9517804008bd212b89cd7799e7052b18585fb313 (single commit on develop 059482d; working tree at HEAD matches; PR head unchanged during review)
Linked issue: #19

Summary

Governance-only change confined to .kilo/command/deliver-issue.md (+39/-20) and .kilo/command/review-pr.md (+10/-2). It separates the independent reviewer's substantive verdict from Forgejo's persisted review state, records the platform facts distinctly in the ledger, makes READY_FOR_OWNER_MERGE depend on substantive exact-HEAD APPROVE rather than a formal platform APPROVED, and keeps truthful platform-state reporting.

Intent / Acceptance Evidence

AC (#19) Evidence at frozen HEAD Status
Distinguish substantive verdict from persisted state/platform approval, incl. shared-author APPROVE -> COMMENT downgrade not treated as infrastructure block Finish: substantive_verdict == APPROVE required; formal APPROVED is "additional evidence when available, not intrinsically required"; downgrade path records forgejo_review_state = COMMENT, platform_approval_recorded = false, identity limitation verified from PR author + platform rejection evidence; explicitly "not REVIEW_INFRASTRUCTURE_BLOCKED" Met
READY_FOR_OWNER_MERGE, remediation on substantive REQUEST_CHANGES, non-approval for substantive COMMENT, exact-HEAD evidence, zero unresolved blockers Finish requires exact frozen HEAD + verified dispatch + substantive APPROVE + zero unresolved blockers + validation/CI/acceptance; budget bullets: substantive REQUEST_CHANGES costs one only on valid in-scope changes; substantive COMMENT is not approval; REMEDIATION_BUDGET_EXHAUSTED = "no substantive APPROVE" Met
Ledger stores the four separate facts, blocker count, limitation; history not rewritten Ledger lists substantive_verdict, forgejo_review_state, platform_approval_recorded, blocking finding count + IDs, platform_identity_limitation when present; "Preserve prior entries and finding identities without rewriting historical review records" Met
Review command preserves frozen HEAD, blockers, substantive ### Overall Recommendation, truthful platform state review-pr.md step 7 + closing note: substantive recommendation for exact frozen HEAD with explicit blockers, report actual persisted Forgejo state separately, stored COMMENT is not formal APPROVED, no inference of APPROVE from arbitrary comment text Met
Only two command docs change; make check + make package-check pass; one PR to develop; fresh independent review; no auto-merge Commit 9517804 touches exactly those two files; gates independently re-run by this reviewer and passed; PR open/unmerged, base develop; this review is the fresh independent review Met

Architecture / Information Flow

Docs-only; no product code, no dependency direction impact. Routing, Agent Manager dispatch, remediation budget, and issue #6 / PR #18 are untouched; the only Independent Review edit (step 7 tracking) changes what counts as verdict evidence, not routing or budget. Consistent with AGENTS.md and .kilo/rules/30-implementation-discipline.md framing of /deliver-issue as development workflow guidance with no Kernel runtime dependency.

Blocking Issues

None.

Non-blocking Suggestions

  1. The Finish section now packs the shared-identity downgrade rule, the branch-protection reporting rule, and the unavailable-evidence rule into one flowing paragraph; blank lines between the three rules would aid scannability. Cosmetic; matches the file's pre-existing dense style.
  2. "not intrinsically required here" could read as document-scoped; "for this readiness evaluation" would be unambiguous. Trivial wording.

Questions for the Author

None.

Security / Privacy Notes

The downgrade path is evidence-bound (PR author match plus platform rejection evidence required before relying on it) and cannot be satisfied by arbitrary comment text. Owner merge authority and any observed branch-protection formal-approval requirement remain separate, concrete conditions. No secrets or private payloads involved.

Test / Documentation Gaps

None material for a docs-only change. Reviewer independently re-ran make check (152 tests, format/lint/type/import-boundary/secret/git diff --check gates) and make package-check (wheel/sdist/isolated install): all passed.

Follow-Up Candidates

None durable.

Overall Recommendation

APPROVE — substantive verdict of this independent reviewer for frozen HEAD 9517804, with zero blocking findings.

Platform-state record (verified during submission): this reviewer first submitted this review as a formal APPROVED review; Forgejo rejected it with approve your own pull is not allowed because the shared kilo account authored the PR. Per the contract introduced by this PR and the owner authorization in issue #19, the submission is therefore persisted as this COMMENT: forgejo_review_state = COMMENT, platform_approval_recorded = false, platform_identity_limitation = shared PR-author account cannot submit formal APPROVED (evidence: PR author kilo, platform rejection message above). A stored COMMENT is not a formal APPROVED review. Do not merge automatically; merge remains the owner's decision.

## PR Review: [Governance] Separate substantive review verdict from Forgejo state (#20) **Repository:** Creatidy/creatidy-kernel **Branch:** `issue-19-review-verdict` -> `develop` **Frozen head:** `9517804008bd212b89cd7799e7052b18585fb313` (single commit on develop `059482d`; working tree at HEAD matches; PR head unchanged during review) **Linked issue:** #19 ### Summary Governance-only change confined to `.kilo/command/deliver-issue.md` (+39/-20) and `.kilo/command/review-pr.md` (+10/-2). It separates the independent reviewer's substantive verdict from Forgejo's persisted review state, records the platform facts distinctly in the ledger, makes `READY_FOR_OWNER_MERGE` depend on substantive exact-HEAD `APPROVE` rather than a formal platform `APPROVED`, and keeps truthful platform-state reporting. ### Intent / Acceptance Evidence | AC (#19) | Evidence at frozen HEAD | Status | |---|---|---| | Distinguish substantive verdict from persisted state/platform approval, incl. shared-author `APPROVE -> COMMENT` downgrade not treated as infrastructure block | Finish: `substantive_verdict == APPROVE` required; formal `APPROVED` is "additional evidence when available, not intrinsically required"; downgrade path records `forgejo_review_state = COMMENT`, `platform_approval_recorded = false`, identity limitation verified from PR author + platform rejection evidence; explicitly "not `REVIEW_INFRASTRUCTURE_BLOCKED`" | Met | | `READY_FOR_OWNER_MERGE`, remediation on substantive `REQUEST_CHANGES`, non-approval for substantive `COMMENT`, exact-HEAD evidence, zero unresolved blockers | Finish requires exact frozen HEAD + verified dispatch + substantive `APPROVE` + zero unresolved blockers + validation/CI/acceptance; budget bullets: substantive `REQUEST_CHANGES` costs one only on valid in-scope changes; substantive `COMMENT` is not approval; `REMEDIATION_BUDGET_EXHAUSTED` = "no substantive `APPROVE`" | Met | | Ledger stores the four separate facts, blocker count, limitation; history not rewritten | Ledger lists `substantive_verdict`, `forgejo_review_state`, `platform_approval_recorded`, blocking finding count + IDs, `platform_identity_limitation` when present; "Preserve prior entries and finding identities without rewriting historical review records" | Met | | Review command preserves frozen HEAD, blockers, substantive `### Overall Recommendation`, truthful platform state | review-pr.md step 7 + closing note: substantive recommendation for exact frozen HEAD with explicit blockers, report actual persisted Forgejo state separately, stored `COMMENT` is not formal `APPROVED`, no inference of `APPROVE` from arbitrary comment text | Met | | Only two command docs change; `make check` + `make package-check` pass; one PR to `develop`; fresh independent review; no auto-merge | Commit `9517804` touches exactly those two files; gates independently re-run by this reviewer and passed; PR open/unmerged, base `develop`; this review is the fresh independent review | Met | ### Architecture / Information Flow Docs-only; no product code, no dependency direction impact. Routing, Agent Manager dispatch, remediation budget, and issue #6 / PR #18 are untouched; the only Independent Review edit (step 7 tracking) changes what counts as verdict evidence, not routing or budget. Consistent with `AGENTS.md` and `.kilo/rules/30-implementation-discipline.md` framing of `/deliver-issue` as development workflow guidance with no Kernel runtime dependency. ### Blocking Issues None. ### Non-blocking Suggestions 1. The Finish section now packs the shared-identity downgrade rule, the branch-protection reporting rule, and the unavailable-evidence rule into one flowing paragraph; blank lines between the three rules would aid scannability. Cosmetic; matches the file's pre-existing dense style. 2. "not intrinsically required here" could read as document-scoped; "for this readiness evaluation" would be unambiguous. Trivial wording. ### Questions for the Author None. ### Security / Privacy Notes The downgrade path is evidence-bound (PR author match plus platform rejection evidence required before relying on it) and cannot be satisfied by arbitrary comment text. Owner merge authority and any observed branch-protection formal-approval requirement remain separate, concrete conditions. No secrets or private payloads involved. ### Test / Documentation Gaps None material for a docs-only change. Reviewer independently re-ran `make check` (152 tests, format/lint/type/import-boundary/secret/`git diff --check` gates) and `make package-check` (wheel/sdist/isolated install): all passed. ### Follow-Up Candidates None durable. ### Overall Recommendation **APPROVE** — substantive verdict of this independent reviewer for frozen HEAD `9517804`, with zero blocking findings. Platform-state record (verified during submission): this reviewer first submitted this review as a formal `APPROVED` review; Forgejo rejected it with `approve your own pull is not allowed` because the shared `kilo` account authored the PR. Per the contract introduced by this PR and the owner authorization in issue #19, the submission is therefore persisted as this `COMMENT`: `forgejo_review_state = COMMENT`, `platform_approval_recorded = false`, `platform_identity_limitation = shared PR-author account cannot submit formal APPROVED` (evidence: PR author `kilo`, platform rejection message above). A stored `COMMENT` is not a formal `APPROVED` review. Do not merge automatically; merge remains the owner's decision.
adrian.tkacz deleted branch issue-19-review-verdict 2026-09-26 09:51:17 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Creatidy/creatidy-kernel!20
No description provided.