Skip to content

No sanctioned completion path when the final rework cycle's review still fails #522

Description

@pulkit-noon

Problem

When an automated review cycle fails and the rework budget is exhausted (fab/project/code-review.md's Max cycles), every documented continuation path in _pipeline.md / fab-continue.md re-enters automated review (fail → reset → apply → re-review, or the interactive rework menu). There is no sanctioned path for "a human directly verified the outstanding findings and wants to proceed without another automated cycle."

What happened

On a real change (kube-all 260804-a9dp-1argo-drift-check), the final (3rd) cycle review returned fail on one must-fix + three should-fix items. The must-fix required a cross-repo infra change; once that landed, a human verified all four findings directly (script runs, live IAM checks, gcloud/practice-matrix checks) rather than re-running review, and the user explicitly chose to proceed to hydrate/ship on that basis.

Because there's no supported way to record this, the operator drove the state machine directly (fab status finish <change> review) instead of going through fail. This skipped the fail event entirely — no fail was ever written for that cycle. The result:

  • .status.yaml now shows review: done, iterations: 3 — indistinguishable from three genuinely passing cycles.
  • The PR's own ## Meta block (rendered by fab pr-meta) shows Review: ✓ 3 cycles, which reads as "all three cycles passed" when the last one did not.

This is a state-integrity divergence: the recorded history no longer matches what actually happened, and there's no way to represent "human-verified override" as a distinct, honest state.

Suggested direction

A distinct terminal state for review (e.g. review: overridden or a human_verified: true flag alongside done), settable only when the rework budget is exhausted and a human explicitly confirms proceeding — surfaced differently in fab pr-meta (e.g. ⚠ 3 cycles, cycle 3 overridden) so the PR's own provenance stays honest. Happy to discuss the exact shape — this report is the gap, not a prescribed fix.

Workaround used

Manually corrected the affected PR's body text after the fact; no change made to the fab CLI's own state files.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions