What happened
PR #124 (#124) went through 4 automated fullsend-ai-review rounds. The final round (workflow run https://github.com/redhat-et/ProtoBot/actions/runs/35204502556, posted 2026-09-17T09:43:05Z) reviewed commit 6cc9a55 — the exact commit that was merged, with no further commits after — and posted 3 Medium + 1 Low findings as unresolved inline review comments. None were replied to, fixed, or tracked; the PR merged 2026-09-17T16:08:56Z (~6h15m later) via a bare 'APPROVED' from JohnStrunk with no comments addressing them. I read the merged files directly (not just the bot's claims) and confirmed two of the Medium findings are real: (a) docs/architecture/agent-harness/adapter-contract.md guard rule 6 (lines 631-640) only checks that a shell command's ////<title> placeholders match recorded project state; it does not check whether .protobot/ already existed, so the initialization-branch form git switch -c <prefix>00001-project-init <default> still matches rule 6 after setup is complete, even though the shell-operations table (line 406) and guard rule 1 (lines 581-593) describe that form as valid only before .protobot/ exists — the enforcement mechanism (rule 6) doesn't encode the constraint the prose asserts. (b) docs/architecture/agent-harness/opencode.md's native bash-rule block (line 196) is "ears-manager *": allow with no restriction on flag values, while adapter-contract.md (lines 324-428) states the core guard refuses --content-file/--impact-file with any value but -, and even ships a guard-vector fixture for --content-file ~/.netrc (line 1163, a credential-exfiltration vector) — but nothing in opencode.md's native layer mirrors that restriction, so if OpenCode's guard hook were ever skipped (a fail-open case the adapter-contract explicitly discusses elsewhere), the native wildcard alone would let the credential-exfiltration form through. This is the same failure pattern as PR #123, where retro filed issue #129 (closed) for four leftover findings that also went unaddressed at merge — i.e. this is a recurring pattern, not a one-off.
What could go better
High confidence on findings (a) and (b): I traced the cross-references myself across adapter-contract.md, opencode.md, and the shell-operations table rather than trusting the bot's summary alone, and both describe a genuine inconsistency between stated intent and the literal enforcement text. Lower confidence on the other two findings from that same round (a claude-code.md fail-open nuance on shell exit 126/127, and an 'cannot change what the agent can do' overstated-claim finding in adapter-contract.md): reading claude-code.md lines 127-132 in the merged file, the fail-open condition for exit 126/127 already appears explicitly documented ('a shim the shell cannot run (status 126 or 127 ...) yields no status 2 ... H8 records the gap'), so that finding may already be substantially addressed or may be a narrower re-flag of already-acknowledged behavior — I'd defer to a human or implementer to confirm rather than asserting it's unresolved. This PR is explicitly a design/spec document that defers full implementation-proving to issue #33/#77, and several other findings across earlier rounds were legitimately closed as 'recorded gap, no change requested' — but (a) and (b) describe the guard/native-rule mechanics contradicting their own stated guarantees, which is a different class of problem than an acknowledged future gap.
Proposed change
Open a follow-up PR against redhat-et/ProtoBot's docs/architecture/agent-harness/ documents that: (1) in adapter-contract.md guard rule 6 (~line 631), add an explicit check that the initialization form is refused whenever .protobot/ already exists (not merely implied by rule 1's prose), and add a guard test vector that runs git switch -c cs/00001-project-init main against a fixture clone where .protobot/ exists and expects a refusal; (2) in opencode.md's native bash-rule block (~line 196), replace the unconditional "ears-manager *": allow with a rule that denies --content-file/--impact-file when followed by anything other than -, or, if OpenCode's rule syntax can't express that, explicitly document (matching the style used elsewhere for Claude Code/Codex gaps) that this specific restriction relies solely on the guard hook and the native layer provides no defense-in-depth here; (3) explicitly confirm and close out, one way or the other, whether the claude-code.md fail-open finding and the adapter-contract.md overstated-claim finding are already covered by the existing 'H8 records the gap' language.
Validation criteria
A follow-up PR addressing points 1-2 merges, and the next fullsend-ai-review pass on that PR does not re-raise the guard-rule-6 lifetime-constraint or the opencode.md permission-expansion finding. When issue #77 (OpenCode Drafting Table MVP) is implemented, its guard fixture includes both new test vectors described above.
Generated by retro agent from #124
What happened
PR #124 (#124) went through 4 automated fullsend-ai-review rounds. The final round (workflow run https://github.com/redhat-et/ProtoBot/actions/runs/35204502556, posted 2026-09-17T09:43:05Z) reviewed commit 6cc9a55 — the exact commit that was merged, with no further commits after — and posted 3 Medium + 1 Low findings as unresolved inline review comments. None were replied to, fixed, or tracked; the PR merged 2026-09-17T16:08:56Z (~6h15m later) via a bare 'APPROVED' from JohnStrunk with no comments addressing them. I read the merged files directly (not just the bot's claims) and confirmed two of the Medium findings are real: (a) docs/architecture/agent-harness/adapter-contract.md guard rule 6 (lines 631-640) only checks that a shell command's ////<title> placeholders match recorded project state; it does not check whether
.protobot/already existed, so the initialization-branch formgit switch -c <prefix>00001-project-init <default>still matches rule 6 after setup is complete, even though the shell-operations table (line 406) and guard rule 1 (lines 581-593) describe that form as valid only before.protobot/exists — the enforcement mechanism (rule 6) doesn't encode the constraint the prose asserts. (b) docs/architecture/agent-harness/opencode.md's native bash-rule block (line 196) is"ears-manager *": allowwith no restriction on flag values, while adapter-contract.md (lines 324-428) states the core guard refuses--content-file/--impact-filewith any value but-, and even ships a guard-vector fixture for--content-file ~/.netrc(line 1163, a credential-exfiltration vector) — but nothing in opencode.md's native layer mirrors that restriction, so if OpenCode's guard hook were ever skipped (a fail-open case the adapter-contract explicitly discusses elsewhere), the native wildcard alone would let the credential-exfiltration form through. This is the same failure pattern as PR #123, where retro filed issue #129 (closed) for four leftover findings that also went unaddressed at merge — i.e. this is a recurring pattern, not a one-off.What could go better
High confidence on findings (a) and (b): I traced the cross-references myself across adapter-contract.md, opencode.md, and the shell-operations table rather than trusting the bot's summary alone, and both describe a genuine inconsistency between stated intent and the literal enforcement text. Lower confidence on the other two findings from that same round (a claude-code.md fail-open nuance on shell exit 126/127, and an 'cannot change what the agent can do' overstated-claim finding in adapter-contract.md): reading claude-code.md lines 127-132 in the merged file, the fail-open condition for exit 126/127 already appears explicitly documented ('a shim the shell cannot run (status 126 or 127 ...) yields no status 2 ... H8 records the gap'), so that finding may already be substantially addressed or may be a narrower re-flag of already-acknowledged behavior — I'd defer to a human or implementer to confirm rather than asserting it's unresolved. This PR is explicitly a design/spec document that defers full implementation-proving to issue #33/#77, and several other findings across earlier rounds were legitimately closed as 'recorded gap, no change requested' — but (a) and (b) describe the guard/native-rule mechanics contradicting their own stated guarantees, which is a different class of problem than an acknowledged future gap.
Proposed change
Open a follow-up PR against redhat-et/ProtoBot's docs/architecture/agent-harness/ documents that: (1) in adapter-contract.md guard rule 6 (~line 631), add an explicit check that the initialization form is refused whenever
.protobot/already exists (not merely implied by rule 1's prose), and add a guard test vector that runsgit switch -c cs/00001-project-init mainagainst a fixture clone where.protobot/exists and expects a refusal; (2) in opencode.md's native bash-rule block (~line 196), replace the unconditional"ears-manager *": allowwith a rule that denies--content-file/--impact-filewhen followed by anything other than-, or, if OpenCode's rule syntax can't express that, explicitly document (matching the style used elsewhere for Claude Code/Codex gaps) that this specific restriction relies solely on the guard hook and the native layer provides no defense-in-depth here; (3) explicitly confirm and close out, one way or the other, whether the claude-code.md fail-open finding and the adapter-contract.md overstated-claim finding are already covered by the existing 'H8 records the gap' language.Validation criteria
A follow-up PR addressing points 1-2 merges, and the next fullsend-ai-review pass on that PR does not re-raise the guard-rule-6 lifetime-constraint or the opencode.md permission-expansion finding. When issue #77 (OpenCode Drafting Table MVP) is implemented, its guard fixture includes both new test vectors described above.
Generated by retro agent from #124