fix(retro): prohibit filing "evidence for" issues - #3638
Closed
rh-hemartin wants to merge 2 commits into
Closed
Conversation
Teach the code agent to discover and use PR templates when writing
pr_body for the structured output file. The post-script uses pr_body
verbatim as the PR description, falling back to the commit body when
absent.
Address review feedback:
- Annotate ADR 0053 with pr_body schema amendment
- Use printf instead of echo for pr_body piping (flag-safe)
- Tighten sed to only strip GitHub close-reference lines, not prose
- Add maxLength: 65536 to pr_body schema (GitHub API limit)
- Fix wrong cross-reference ("step 6" → "structured output block")
- Replace heredoc examples with jq -n pattern (safe from shell expansion)
- Wrap examples in if/else to prevent running both branches
- Align pr_body description style with target_branch (direct assertion)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
The retro agent's dedup check tells it to skip proposals covered by existing issues, but the LLM invented a workaround: filing "Evidence for #XXXX" issues that pass schema validation by stuffing evidence into the standard proposal fields. This produced 336 noise issues in 3 days (69% of all retro output since July 5). Add explicit prohibition to the retro-analysis skill. Evidence for existing issues goes in the summary field (posted as a PR comment), not as a new proposal. The same change must be applied to fullsend-ai/agents independently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 2:20 PM UTC · Completed 2:31 PM UTC |
2 tasks
Site previewPreview: https://99a76a7c-site.fullsend-ai.workers.dev Commit: |
PR Summary by QodoSupport template-aware PR bodies and ban retro "Evidence for" issue spam
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
Context used✅ Tickets:
🎫 Retro agent files "Evidence for" issues instead of skipping duplicates 🎫 Review agent should deduplicate findings across re-review iterations on the same PR✅ Compliance rules (platform):
61 rules✅ Skills:
writing-user-docs, writing-adrs 1. pr_body lacks requirement link
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+105
to
+109
| }, | ||
| "pr_body": { | ||
| "type": "string", | ||
| "maxLength": 65536, | ||
| "description": "PR description used as PR body instead of commit body." |
There was a problem hiding this comment.
2. Pr description misses adr 📘 Rule violation § Compliance
An Accepted ADR was modified in this PR, but the PR description does not explicitly call out the ADR filename/identifier and summarize what changed. This reduces auditability of architectural decision changes.
Agent Prompt
## Issue description
The PR modifies an accepted ADR but the PR description does not explicitly mention the ADR and summarize the change.
## Issue Context
Compliance requires PR descriptions to call out edits to accepted ADRs.
## Fix Focus Areas
- docs/ADRs/0053-agent-driven-branch-targeting.md[1-20]
- docs/ADRs/0053-agent-driven-branch-targeting.md[85-124]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+445
to
+455
| # Read pr_body from agent output. Fall back to commit body if absent. | ||
| PR_BODY_FROM_RESULT="" | ||
| if [ -n "${RESULT_FILE}" ]; then | ||
| PR_BODY_FROM_RESULT="$(jq -r '.pr_body // empty' "${RESULT_FILE}" 2>/dev/null || true)" | ||
| fi | ||
|
|
||
| if [ -n "${PR_BODY_FROM_RESULT}" ]; then | ||
| # Agent provided pr_body (template-aware or best-effort). | ||
| # Strip Signed-off-by and Closes lines so the script appends them once. | ||
| COMMIT_BODY="$(printf '%s\n' "${PR_BODY_FROM_RESULT}" | sed '/^Signed-off-by:/d' | sed '/^Closes #/d; /^Closes [a-zA-Z0-9_.-]*\/[a-zA-Z0-9_.-]*#/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')" | ||
| else |
There was a problem hiding this comment.
3. pr_body lacks requirement link 📘 Rule violation § Compliance
The PR introduces new behavior (pr_body support) in the code-agent contract and post-script, but this PR is scoped/described as a retro-agent fix (#3617) and does not reference a tracked requirement for the new PR-body feature. Untracked functionality increases scope creep and weakens change control.
Agent Prompt
## Issue description
New behavior (`pr_body` support) was added but is not tied to an explicit tracked requirement in this PR’s scope.
## Issue Context
Compliance requires new public behaviors to map to an explicit tracked requirement (issue/RFC/etc.) and not be introduced as unrelated scope.
## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[445-465]
- internal/scaffold/fullsend-repo/schemas/code-result.schema.json[4-19]
- internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md[167-234]
- docs/ADRs/0053-agent-driven-branch-targeting.md[115-124]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+105
to
+109
| }, | ||
| "pr_body": { | ||
| "type": "string", | ||
| "maxLength": 65536, | ||
| "description": "PR description used as PR body instead of commit body." |
There was a problem hiding this comment.
4. Adr inlines json schema 📜 Skill insight ⚙ Maintainability
The ADR embeds the code-result JSON Schema inline (and expands it in this PR) instead of linking to a versioned normative spec. Inline contracts are harder to audit/version and should live under docs/normative/<topic>/v<major>/... with the ADR linking to them.
Agent Prompt
## Issue description
The ADR includes inline JSON Schema content instead of linking to a versioned normative spec location.
## Issue Context
Compliance requires detailed contracts/JSON schemas to be stored in `docs/normative/<topic>/v<major>/...` and referenced from the ADR.
## Fix Focus Areas
- docs/ADRs/0053-agent-driven-branch-targeting.md[89-124]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+456
to
+463
| # Fall back to unwrapped commit body (legacy path) | ||
| COMMIT_BODY_RAW="$(git log -1 --format='%b' HEAD | sed '/^Signed-off-by:/d' | sed '/^Closes #/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')" | ||
| COMMIT_BODY="$(echo "${COMMIT_BODY_RAW}" | awk ' | ||
| /^$/ { if (buf) print buf; print; buf=""; next } | ||
| /^[-*#>]|^ / { if (buf) print buf; buf=""; print; next } | ||
| /^Closes / { if (buf) print buf; buf=""; print; next } | ||
| { buf = (buf ? buf " " $0 : $0) } | ||
| END { if (buf) print buf } |
There was a problem hiding this comment.
5. Extra closes directives retained 🐞 Bug ≡ Correctness
In post-code.sh, the legacy commit-body path only strips lines starting with Closes # but can
retain other Closes ... lines (e.g. Closes owner/repo#123). Because the script always appends
its own Closes #${ISSUE_NUMBER} footer, the resulting PR body can contain multiple close
directives, contradicting the “append once” intent and risking unintended side effects.
Agent Prompt
### Issue description
`post-code.sh` strips `Closes` lines more completely for the new `pr_body` path than for the legacy commit-body path. The legacy path can therefore leave additional `Closes ...` directives in the final PR body even though the script appends its own `Closes #<issue>` footer.
### Issue Context
- The new `pr_body` path deletes both `^Closes #` and `^Closes owner/repo#...`.
- The legacy path deletes only `^Closes #`, leaving other `Closes ...` lines intact.
### Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[451-465]
- internal/scaffold/fullsend-repo/scripts/post-code-test.sh[127-337]
### Suggested fix
- Make the legacy commit-body sanitization match the `pr_body` path by deleting all `Closes ...` lines (at minimum the cross-repo form) before assembling `COMMIT_BODY`.
- Add a regression test in `post-code-test.sh` where the commit body includes `Closes owner/repo#123` and assert it does not appear in the final body (and that the appended `Closes #<issue>` appears exactly once).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🤖 Finished Retro · ✅ Success · Started 2:26 PM UTC · Completed 2:32 PM UTC |
|
Review skipped — this PR is already closed. The Posted by fullsend post-review check |
Retro: PR #3638 — closed without mergePR #3638 was a human-authored PR by Issue #3617 remains open and undelivered. The retro agent continues to file "Evidence for" issues — 354 are currently open. Observations
Proposals filed
|
rh-hemartin
added a commit
to fullsend-ai/agents
that referenced
this pull request
Jul 8, 2026
The retro agent's dedup check tells it to skip proposals covered by existing issues, but the LLM invented a workaround: filing "Evidence for #XXXX" issues that pass schema validation. This produced 336 noise issues in 3 days (69% of all retro output since July 5). Add explicit prohibition to the retro-analysis skill. Evidence for existing issues goes in the summary field (posted as a PR comment), not as a new proposal. Mirrors fullsend-ai/fullsend#3638. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skills/retro-analysis/SKILL.mdsummaryfield (posted as PR comment), not as a new proposalfullsend-ai/agentsseparatelyThe retro agent's dedup check says "skip proposals covered by existing issues" but the LLM invented a workaround: titling proposals "Evidence for #XXXX" and stuffing the evidence narrative into the standard
what_happened/proposed_changefields. Schema validation passes because it only checks field names and types, not semantic intent.Transcript analysis (runs 28948580111, 28941534472) shows the agent fighting the schema to get evidence filed — in one case burning 4 validation-loop iterations trying
{"type": "evidence", "target_issue": 2959}before discovering it can just use "Evidence for #2959" as the title with standard fields.336 of 485 retro-filed issues since July 5 (69%) are evidence filings. On peak day (July 7), 156 of 230.
Fixes #3617
Test plan
🤖 Generated with Claude Code