redhat-et / redhat-et/ProtoBot
Fix three unresolved Medium findings from PR #138's final review round: add-requirement blocked-dependency mechanism, un-revoked Gate approval on supersede, and resolution-replay field contradiction
@JohnStrunk is already working on this.
Since Sep 18, 2026.
- Dominant language
- Go
- Stars
- 5
- Forks
- 6
- Avg merge
- 23h 24m
- Merged PRs (30d)
- 66
Description
What happened
PR #138 (https://github.com/redhat-et/ProtoBot/pull/138, merged 2026-09-18T20:09:17Z) added docs/architecture/drafting-table-wms.md and its golden fixture. fullsend-ai-review's final pass (workflow run https://github.com/redhat-et/ProtoBot/actions/runs/35355464689, posted as https://github.com/redhat-et/ProtoBot/pull/138#issuecomment-5717965681 at 2026-09-18T14:42:01Z, reviewing head 81685ea9 — the exact commit that was merged, with no further commits after) posted 3 Medium findings that were unresolved: (1) api-contract-violation at drafting-table-wms.md:358 — add-requirement's prerequisite requires Materializer processing to create/refresh a dependency on the work item while it stays blocked, but sibling validation-rules.md has no same-state blocked mutation or command naming this; flagged since at least the round-3 review (originally at line 347) and never resolved across 4+ rounds. (2) api-contract-violation at drafting-table-wms.md:381 — a superseding submission leaves the prior submission's Gate approval 'unused' rather than revoked/consumed, so a role-valid Materializer could later apply a withdrawn approval via resolve-block; the golden fixture never exercises this path. This was a new defect introduced by this same revision's fix to a different, previously-flagged finding (the 'reserved' approval-state gap) — i.e. fixing one finding introduced another. (3) logic-error in the golden fixture at drafting-table-wms-golden.jsonl:27 (step resolution-replay) — an exact idempotency-key replay of resolution-001 returns submission_status: superseded (a different field name, reflecting live post-supersede state) instead of replaying the original applied result verbatim (submission: accepted, revision 1), contradicting the contract's own idempotent-replay rule. I independently verified all three are still present in main by reading the merged files directly (not just trusting the bot): drafting-table-wms.md lines 358-361 and 375-381, and golden.jsonl lines 19 vs 27. The PR merged via JohnStrunk's uncommented APPROVED review with no discussion of these findings — consistent with the repo's known, deliberate 'no re-approval required' tradeoff (see closed issue #116). This mirrors exactly what happened on PR #124, where a prior retro run filed issue #139 to track that PR's unresolved final-round findings; no equivalent issue exists yet for PR #138.
What could go better
I am highly confident these three findings are genuine (not false positives): I read the merged spec text and fixture directly and confirmed the described contradictions exist verbatim, and the review agent's remediation guidance for each is specific and technically sound (e.g., naming the exact Validation Rules command gap, the exact fixture step/field mismatch). What I'm less certain about is severity in practice — this is a documentation-only repo where the 'implementation' is the spec itself, so these are design ambiguities rather than runtime bugs, but per AGENTS.md's own rules ('governed sibling specs must be reconciled') they represent real non-compliance with the project's own hierarchy-consistency requirements. I'm not proposing any change to the merge/approval policy itself, since #116 shows the maintainers already made an explicit, informed choice to prioritize velocity over blocking merges on unresolved bot findings — re-litigating that would be a wasted proposal.
Proposed change
File a new issue in redhat-et/ProtoBot (following the format and precedent of issue #139) that: (1) quotes the three unresolved Medium findings above with their exact file/line locations and the review agent's remediation suggestions from https://github.com/redhat-et/ProtoBot/pull/138#issuecomment-5717965681; (2) asks for docs/architecture/drafting-table-wms.md to either name the Validation Rules command/precondition that legally records the add-requirement dependency on a blocked item (or restructure so no same-state work-item mutation is implied), to specify that a superseded submission's Gate approval is revoked/terminalized (or that resolve-block must name and validate against the currently-active resolution_submission_id), and to fix the golden fixture's resolution-replay step (line 27) to return the frozen original result verbatim on exact-key replay; (3) notes the several unresolved Low findings from the same review round (missing cross-references in git-integration.md and adapter-contract.md, delegated-principal binding gaps on acknowledge/submit-resolution, missing approval-consumption semantics on refine) so they can be swept up in the same pass. Do not propose a branch-protection or re-approval-requirement change — that is a settled, deliberate maintainer decision per closed issue #116.
Validation criteria
The new issue is created and, once resolved by a follow-up PR, docs/architecture/drafting-table-wms.md no longer describes an unnamed same-state blocked dependency mutation, explicitly states the fate of a superseded submission's Gate approval, and the golden fixture's resolution-replay step returns the same submission/revision fields and values as the original apply at step 19. A subsequent fullsend-ai-review pass on that follow-up PR should not re-flag any of these three specific findings.
Generated by retro agent from https://github.com/redhat-et/ProtoBot/pull/138
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.