Skip to content

feat(retro): post evidence comments on related issues #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rh-hemartin wants to merge 1 commit into from
Closed

feat(retro): post evidence comments on related issues #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions agents/retro.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,20 @@ After gathering findings from subagents:

Write a single JSON file to `$FULLSEND_OUTPUT_DIR/agent-result.json`.

The top-level object must have **exactly two properties** — no others:
The top-level object has two required properties and one optional:

```json
{
"summary": "...",
"proposals": [...]
"proposals": [...],
"evidence_comments": [...]
}
```

- `summary` (required) — Markdown summary posted as a comment on the originating PR/issue.
- `proposals` (required) — Improvement proposals, each filed as a GitHub issue.
- `evidence_comments` (optional) — Comments posted on existing open issues when this retro found corroborating evidence. See the `retro-analysis` skill for the evidence comment schema.

The schema enforces `"additionalProperties": false`. Any extra top-level key (e.g., `timeline`, `workflow_quality`, `originating_url`, `metadata`) will fail validation.

See the `retro-analysis` skill for the proposal object schema and writing guidance.
Expand Down
24 changes: 24 additions & 0 deletions schemas/retro-result.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,33 @@
"maxItems": 3,
"items": { "$ref": "#/$defs/proposal" },
"description": "List of improvement proposals. Each becomes a GitHub issue."
},
"evidence_comments": {
"type": "array",
"maxItems": 5,
"items": { "$ref": "#/$defs/evidence_comment" },
"description": "Comments to post on existing open issues when this retro found corroborating evidence."
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
},
"$defs": {
"evidence_comment": {
"type": "object",
"additionalProperties": false,
"required": ["issue_url", "body"],
"properties": {
"issue_url": {
"type": "string",
"pattern": "^https://github\\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/issues/[0-9]+$",
"description": "Full GitHub URL of the existing issue to comment on."
},
"body": {
"type": "string",
"minLength": 1,
"maxLength": 8192,
"description": "Markdown comment body with the specific evidence found, what was analyzed, and a link to the originating PR."
}
}
},
"proposal": {
"type": "object",
"additionalProperties": false,
Expand Down
130 changes: 126 additions & 4 deletions scripts/post-retro-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ MOCK_BIN="${TMPDIR}/bin"
mkdir -p "${MOCK_BIN}"
cat > "${MOCK_BIN}/gh" <<'MOCKEOF'
#!/usr/bin/env bash
# Consume stdin if --input - is passed, to avoid SIGPIPE under pipefail.
# Capture stdin if --input - is passed (also avoids SIGPIPE under pipefail).
for arg in "$@"; do
if [[ "${arg}" == "--input" ]]; then
cat > /dev/null
cat >> "${GH_LOG}.body"
break
fi
done
Expand All @@ -48,9 +48,15 @@ if [[ "$1" == "issue" && "$2" == "create" ]]; then
exit 0
fi

# Comment posting via gh api — controlled by GH_MOCK_COMMENT_FAIL.
# Comment posting via gh api — controlled by GH_MOCK_COMMENT_FAIL (summary)
# and GH_MOCK_EVIDENCE_FAIL (evidence comments on other issues).
if [[ "$1" == "api" && "$2" == *"/comments" ]]; then
case "${GH_MOCK_COMMENT_FAIL:-}" in
# Determine which failure mode to use based on the endpoint.
FAIL_MODE="${GH_MOCK_COMMENT_FAIL:-}"
if [[ "$2" != "repos/test-org/test-repo/issues/10/comments" && -n "${GH_MOCK_EVIDENCE_FAIL:-}" ]]; then
FAIL_MODE="${GH_MOCK_EVIDENCE_FAIL}"
fi
case "${FAIL_MODE}" in
403)
echo "HTTP 403: Resource not accessible by integration" >&2
exit 1
Expand Down Expand Up @@ -108,12 +114,62 @@ FIXTURE_NO_PROPOSALS='{
"proposals": []
}'

# Fixture: a valid agent result with evidence comments.
FIXTURE_WITH_EVIDENCE='{
"summary": "Found evidence corroborating existing issue #42.",
"proposals": [],
"evidence_comments": [
{
"issue_url": "https://github.com/test-org/target-repo/issues/42",
"body": "### Evidence from retro of PR #10\n\nWidget service crashed again on empty input.\n\n_Source: https://github.com/test-org/test-repo/pull/10_"
}
]
}'

# Fixture: proposals + evidence comments together.
FIXTURE_PROPOSALS_AND_EVIDENCE='{
"summary": "Found one improvement and evidence for #42.",
"proposals": [
{
"target_repo": "test-org/target-repo",
"title": "Improve error handling in widget service",
"what_happened": "The widget service crashed on empty input.",
"what_could_go_better": "Input validation should reject empty payloads.",
"proposed_change": "Add a nil check at the entry point.",
"validation_criteria": "Widget service returns 400 on empty input."
}
],
"evidence_comments": [
{
"issue_url": "https://github.com/test-org/target-repo/issues/42",
"body": "### Evidence from retro of PR #10\n\nMore evidence here.\n\n_Source: https://github.com/test-org/test-repo/pull/10_"
}
]
}'

# Fixture: multiple evidence comments targeting different issues.
FIXTURE_MULTI_EVIDENCE='{
"summary": "Found evidence corroborating two existing issues.",
"proposals": [],
"evidence_comments": [
{
"issue_url": "https://github.com/test-org/target-repo/issues/42",
"body": "### Evidence from retro\n\nFirst issue evidence.\n\n_Source: https://github.com/test-org/test-repo/pull/10_"
},
{
"issue_url": "https://github.com/test-org/other-repo/issues/7",
"body": "### Evidence from retro\n\nSecond issue evidence.\n\n_Source: https://github.com/test-org/test-repo/pull/10_"
}
]
}'

run_test() {
local test_name="$1"
local json_content="$2"
local expected_pattern="$3"
local expect_failure="${4:-false}"
local comment_fail="${5:-}"
local evidence_fail="${6:-}"

# Create iteration output structure.
local run_dir="${TMPDIR}/run-${test_name}"
Expand All @@ -122,7 +178,9 @@ run_test() {

# Clear gh call log.
: > "${GH_LOG}"
: > "${GH_LOG}.body"
export GH_MOCK_COMMENT_FAIL="${comment_fail}"
export GH_MOCK_EVIDENCE_FAIL="${evidence_fail}"

# Run the post-script.
local exit_code=0
Expand Down Expand Up @@ -162,12 +220,15 @@ run_test_stdout() {
local expected_stdout="$3"
local expect_failure="${4:-false}"
local comment_fail="${5:-}"
local evidence_fail="${6:-}"

local run_dir="${TMPDIR}/run-${test_name}"
mkdir -p "${run_dir}/iteration-1/output"
echo "${json_content}" > "${run_dir}/iteration-1/output/agent-result.json"
: > "${GH_LOG}"
: > "${GH_LOG}.body"
export GH_MOCK_COMMENT_FAIL="${comment_fail}"
export GH_MOCK_EVIDENCE_FAIL="${evidence_fail}"

local exit_code=0
(cd "${run_dir}" && bash "${POST_SCRIPT}") > "${TMPDIR}/stdout.log" 2>&1 || exit_code=$?
Expand Down Expand Up @@ -274,6 +335,67 @@ run_test_stdout "complete-message" \
"${FIXTURE_ONE_PROPOSAL}" \
"Post-retro complete."

# Evidence comments: happy path — gh api called on the referenced issue.
run_test "evidence-comment-posted" \
"${FIXTURE_WITH_EVIDENCE}" \
"repos/test-org/target-repo/issues/42/comments"

# Evidence comments: verify the posted body matches the fixture.
run_test "evidence-body-posted" \
"${FIXTURE_WITH_EVIDENCE}" \
"repos/test-org/target-repo/issues/42/comments"
# The body log should contain the evidence comment text sent via --input.
if ! grep -qF "Widget service crashed again on empty input" "${GH_LOG}.body"; then
echo "FAIL: evidence-body-posted — expected evidence body not found in captured stdin"
echo "Captured body:"
cat "${GH_LOG}.body"
FAILURES=$((FAILURES + 1))
else
echo "PASS: evidence-body-posted (body verified)"
fi

# Evidence comments: no evidence_comments field — no extra API calls.
run_test_stdout "no-evidence-field" \
"${FIXTURE_NO_PROPOSALS}" \
"Post-retro complete."
Comment on lines +357 to +360

@qodo-code-review qodo-code-review Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

5. No-evidence test doesn't assert 🐞 Bug ⚙ Maintainability

The new "no-evidence-field" test claims to ensure no extra API calls occur when evidence_comments is
absent, but it only checks stdout for "Post-retro complete." and never inspects the gh call log.
This test can pass even if evidence-comment gh api calls are accidentally made.
Agent Prompt 
## Issue description
The `no-evidence-field` test is intended to validate that no evidence-comment API calls are made, but it uses `run_test_stdout`, which does not check `${GH_LOG}` at all.

## Issue Context
- `run_test()` verifies `${GH_LOG}` patterns.
- `run_test_stdout()` only greps stdout, so it cannot validate side effects like extra `gh api .../comments` calls.

## Fix Focus Areas
- scripts/post-retro-test.sh[200-251]
- scripts/post-retro-test.sh[325-329]

### Suggested implementation direction
- Either:
  - Add a new helper that asserts `${GH_LOG}` does *not* contain a substring (e.g., `target-repo/issues/42/comments`), or
  - Extend `run_test_stdout` to accept an optional `forbidden_pattern` (or expected call count) and assert against `${GH_LOG}`.
- Update `no-evidence-field` to assert the gh log contains only the originating summary comment call and no other `/comments` endpoints.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


# Evidence comments: proposals and evidence together both work.
run_test "evidence-with-proposals" \
"${FIXTURE_PROPOSALS_AND_EVIDENCE}" \
"repos/test-org/target-repo/issues/42/comments"

# Evidence comments: 403 on evidence comment is non-fatal.
run_test_stdout "evidence-403-non-fatal" \
"${FIXTURE_WITH_EVIDENCE}" \
"::warning::Could not post evidence comment" \
"false" \
"" \
"403"

# Evidence comments: 401 on evidence comment is non-fatal.
run_test_stdout "evidence-401-non-fatal" \
"${FIXTURE_WITH_EVIDENCE}" \
"::warning::Could not post evidence comment" \
"false" \
"" \
"401"

# Evidence comments: 500 on evidence comment is fatal.
run_test_stdout "evidence-500-fatal" \
"${FIXTURE_WITH_EVIDENCE}" \
"ERROR: failed to post evidence comment" \
"true" \
"" \
"500"

# Evidence comments: multiple entries — both are posted.
run_test "multi-evidence-first" \
"${FIXTURE_MULTI_EVIDENCE}" \
"repos/test-org/target-repo/issues/42/comments"
run_test "multi-evidence-second" \
"${FIXTURE_MULTI_EVIDENCE}" \
"repos/test-org/other-repo/issues/7/comments"

# --- Results ---

if [[ ${FAILURES} -gt 0 ]]; then
Expand Down
58 changes: 58 additions & 0 deletions scripts/post-retro.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,64 @@ for i in $(seq 0 $((PROPOSAL_COUNT - 1))); do
"
done

# Post evidence comments on referenced issues.
EVIDENCE_COUNT=$(jq '.evidence_comments // [] | length' "${RESULT_FILE}")
if [[ "${EVIDENCE_COUNT}" -gt 0 ]]; then
for i in $(seq 0 $((EVIDENCE_COUNT - 1))); do
EU=$(jq -r ".evidence_comments[$i].issue_url" "${RESULT_FILE}")
if [[ ! "${EU}" =~ ^https://github\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/issues/[0-9]+$ ]]; then
echo "ERROR: evidence_comments[$i].issue_url does not match expected pattern: ${EU}" >&2
exit 1
fi
EB=$(jq -r ".evidence_comments[$i].body // empty" "${RESULT_FILE}")
if [[ -z "${EB}" ]]; then
echo "ERROR: evidence_comments[$i].body is missing or empty" >&2
exit 1
fi
done
echo "All ${EVIDENCE_COUNT} evidence comment(s) validated"

echo "Posting ${EVIDENCE_COUNT} evidence comment(s)"
for i in $(seq 0 $((EVIDENCE_COUNT - 1))); do
ISSUE_URL=$(jq -r ".evidence_comments[$i].issue_url" "${RESULT_FILE}")
EVIDENCE_BODY=$(jq -r ".evidence_comments[$i].body" "${RESULT_FILE}")

[[ "${ISSUE_URL}" =~ ^https://github\.com/([a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+)/issues/([0-9]+)$ ]]
EVIDENCE_REPO="${BASH_REMATCH[1]}"
EVIDENCE_NUMBER="${BASH_REMATCH[2]}"

echo " Commenting on ${EVIDENCE_REPO}#${EVIDENCE_NUMBER}"
EVIDENCE_OUTPUT=""
EVIDENCE_EXIT=0
EVIDENCE_OUTPUT=$(jq -nc --arg body "${EVIDENCE_BODY}" '{body: $body}' | gh api \
"repos/${EVIDENCE_REPO}/issues/${EVIDENCE_NUMBER}/comments" \
--input - 2>&1) || EVIDENCE_EXIT=$?

if [[ ${EVIDENCE_EXIT} -ne 0 ]]; then
if echo "${EVIDENCE_OUTPUT}" | grep -qE "HTTP (401|403)"; then
SAFE_OUTPUT="${EVIDENCE_OUTPUT//::/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0A/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0a/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0D/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0d/}"
SAFE_REPO="${EVIDENCE_REPO//::/}"
SAFE_NUMBER="${EVIDENCE_NUMBER//::/}"
echo "::warning::Could not post evidence comment on ${SAFE_REPO}#${SAFE_NUMBER}: insufficient permissions (${SAFE_OUTPUT}). Skipping."
else
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
SAFE_OUTPUT="${EVIDENCE_OUTPUT//::/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0A/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0a/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0D/}"
SAFE_OUTPUT="${SAFE_OUTPUT//%0d/}"
SAFE_REPO="${EVIDENCE_REPO//::/}"
SAFE_NUMBER="${EVIDENCE_NUMBER//::/}"
echo "ERROR: failed to post evidence comment on ${SAFE_REPO}#${SAFE_NUMBER}: ${SAFE_OUTPUT}"
exit 1
fi
fi
done
fi

Comment on lines +125 to +182

@qodo-code-review qodo-code-review Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Protected paths modified 📜 Skill insight § Compliance

This PR modifies protected governance/infrastructure paths (scripts/ and skills/), which require
explicit human review and must not be auto-approved. Even with an issue link, this should be
surfaced as a protected-path change needing manual approval.
Agent Prompt 
## Issue description
Protected governance/infrastructure paths were modified and must be explicitly flagged for human review.

## Issue Context
This PR changes automation scripts and agent skill instructions.

## Fix Focus Areas
- scripts/post-retro.sh[125-165]
- skills/retro-analysis/SKILL.md[121-183]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

# Post summary comment on the originating PR/issue.
# Uses REST API (not gh issue comment) for consistency. Note: despite being
# an "issues" endpoint, GitHub requires pull_requests:write when the target
Expand Down
16 changes: 13 additions & 3 deletions skills/retro-analysis/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,13 @@ Use multiple searches with different keyword combinations if the first returns n
- **Skip the proposal** if a recently closed issue addressed the same problem (closed in the last 90 days) — the fix may already be in flight.
- **Include the proposal** only if you are confident no existing issue covers it, or if your proposal meaningfully refines an existing one in a way that warrants a new issue.

**Do not file "evidence for" issues.** When your analysis produces evidence that supports or corroborates an existing open issue, put it in your `summary` field — not in a new proposal. Do not title proposals "Evidence for #XXXX" or use any other framing that makes a duplicate look like a new issue. The summary is posted as a comment on the originating PR or issue, which preserves the data point. Filing evidence as a separate proposal creates noise that compounds across retro runs.
When your analysis produces evidence that supports or corroborates an existing open issue, add an entry to the `evidence_comments` array. The post-script posts each evidence comment directly on the referenced issue, giving issue owners a growing body of data points.

When skipping, note the duplicate in your `summary` field — include the issue number and what specific evidence this retro found, so the human understands what was filtered and why. Keep evidence notes concise — one sentence per existing issue with the issue number and a brief description of the new evidence. The summary field has a schema length limit; prioritize the most impactful evidence if space is constrained.
Each `evidence_comments` entry needs:
- `issue_url` — the full GitHub URL of the existing issue (e.g. `https://github.com/owner/repo/issues/42`).
- `body` — a markdown comment with: what PR/workflow this retro analyzed, the specific evidence found (concrete data points, not just "more evidence"), and a link back to the originating PR (`$ORIGINATING_URL`) for full context.

Also note the evidence briefly in your `summary` field — include the issue number so the human reading the originating PR understands that evidence was generated.

## Localization guidance

Expand Down Expand Up @@ -165,11 +169,17 @@ Write a single JSON file to `$FULLSEND_OUTPUT_DIR/agent-result.json` with this s
"proposed_change": "Specific change description...",
"validation_criteria": "How to verify the improvement..."
}
],
"evidence_comments": [
{
"issue_url": "https://github.com/owner/repo/issues/42",
"body": "### Evidence from retro of PR #10\n\nSpecific evidence found...\n\n_Source: https://github.com/owner/repo/pull/10_"
}
]
}
```

**Schema is strict.** The top-level object allows ONLY `summary` and `proposals` — no additional properties. Each proposal object allows ONLY the six fields shown above. The harness validates against `$FULLSEND_OUTPUT_SCHEMA` with `"additionalProperties": false` at both levels. Do not add fields like `timeline`, `metadata`, `workflow_quality`, or `originating_url`.
**Schema is strict.** The top-level object allows ONLY `summary`, `proposals`, and `evidence_comments` — no additional properties. Each proposal object allows ONLY the six fields shown above. Each evidence comment allows ONLY `issue_url` and `body`. The harness validates against `$FULLSEND_OUTPUT_SCHEMA` with `"additionalProperties": false` at all levels. Do not add fields like `timeline`, `metadata`, `workflow_quality`, or `originating_url`. The `evidence_comments` array is optional — omit it when no corroborating evidence was found.

After writing the file, validate it before exiting:

Expand Down
Loading