feat(agents): add action-hints footer to review, triage, and fix comments

[rh-hemartin]

Summary

• Add "Next steps" footer to review comments on request-changes outcomes, showing /fs-fix, /fs-fix <instruction>, push, and /fs-fix-stop
• Add "Next steps" footer to triage comments on sufficient action, showing /fs-code and /fs-code <instruction>
• Add "Next steps" footer to fix agent summary comments, showing /fs-review, /fs-fix <instruction>, and push
• Add test for fix agent footer presence

Closes fullsend-ai/fullsend#1789

Test plan

• process-fix-result-test.py passes (27 tests)
• Deploy to staging, trigger a review with request-changes outcome — verify footer appears
• Trigger a triage with sufficient action — verify footer appears
• Trigger a fix agent run — verify footer appears
• Verify footer does NOT appear on approve, comment, reject, insufficient, duplicate, prerequisites, question outcomes

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 6:37 AM UTC · Ended 6:46 AM UTC
Commit: e8381e3 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Add “Next steps” action-hints footers to review/triage specs and fix summaries

✨ Enhancement 📝 Documentation 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add “Next steps” footers to review/triage specs to make bot commands discoverable.
• Introduce a fix-result processor that posts fix-agent summaries with follow-up commands.
• Add unit tests covering footer presence, schema validation, and comment truncation.

Diagram

graph TD
  RA["Review agent"] --> PS["Prompt specs"] --> GPR["GitHub PR comment"]
  TA["Triage agent"] --> PS --> GPR
  FA["Fix agent"] --> FR[/"fix-result.json"/] --> PFR["process-fix-result.py"] --> GH["gh pr comment"] --> GPR
  subgraph Legend
    direction LR
    _agent["Agent"] ~~~ _spec["Spec/Doc"] ~~~ _file[/"File"/] ~~~ _tool["CLI tool"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize footer templates in a shared module/config

• ➕ Avoids footer text drift across review/triage/fix surfaces
• ➕ Makes it easy to update command names/descriptions once
• ➖ Requires plumbing shared assets into markdown specs and scripts
• ➖ Slightly more abstraction for a small amount of text

2. Inject footers in a single “comment posting” layer

• ➕ Guarantees consistent behavior regardless of agent prompt variations
• ➕ Easier to enforce “only on certain outcomes” rules programmatically
• ➖ Needs outcome/action metadata available at posting time
• ➖ More refactoring across agent pipelines to route structured signals

Recommendation: The current approach is reasonable for quick discoverability wins: specs clearly document when to show the footer, and the fix-agent summary footer is enforced in code. If these footers expand to more agents or commands change frequently, consider centralizing the footer content (or injecting it in a shared posting layer) to reduce duplication and drift.

Files changed (4) +666 / -2

Enhancement (1) +198 / -0

process-fix-result.pyAdd fix-result processor that posts summary comments with “Next steps” footer +198/-0

Add fix-result processor that posts summary comments with “Next steps” footer

• Adds a new script to read 'fix-result.json', validate it against 'schemas/fix-result.schema.json', render a markdown summary (fixed/disagreed/decision points/tests), and post it to the PR via 'gh pr comment'. Includes comment-length truncation handling and distinct exit codes for validation vs posting failures.

scripts/process-fix-result.py

Tests (1) +445 / -0

process-fix-result-test.pyAdd unit tests for fix-result summary formatting and posting behavior +445/-0

Add unit tests for fix-result summary formatting and posting behavior

• Introduces a unittest suite covering summary rendering (fixes, disagreements, decision points, strategy change), footer presence, schema-validation failure cases, truncation behavior, and error handling when comment posting fails.

scripts/process-fix-result-test.py

Documentation (2) +23 / -2

triage.mdDocument action-hints footer for sufficient triage outcomes +8/-0

Document action-hints footer for sufficient triage outcomes

• Extends triage guidance to append a standardized “Next steps” markdown footer when 'action' is 'sufficient'. Explicitly requires omitting the footer for other triage actions.

agents/triage.md

SKILL.mdAllow action-hints footer only for request-changes review outcomes +15/-2

Allow action-hints footer only for request-changes review outcomes

• Updates review-skill instructions to keep reviews footer-free by default, but add a standardized “Next steps” footer for 'request-changes' outcomes only. Explicitly forbids the footer for approve/comment/reject outcomes.

skills/pr-review/SKILL.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (2)

Context used

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

1. Missing jsonschema dependency ✓ Resolved 🐞 Bug ☼ Reliability

Description

scripts/process-fix-result.py unconditionally imports and uses jsonschema, but the fix
post-script invokes it on the GitHub Actions runner without ensuring jsonschema is installed. This
can crash the post-fix step (after a successful push), preventing the fix summary comment from being
posted and potentially failing the workflow.

Code

scripts/process-fix-result.py[R16-22]

+import json
+import os
+import subprocess
+import sys
+
+import jsonschema
+

Relevance

⭐⭐⭐ High

Team enforces explicit jsonschema installs/pinning in CI; indicates dependency must be managed (PR
#37).

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new script imports and uses jsonschema during execution; the fix post-script calls it with
python3 but does not install/verify jsonschema, while the repo’s schema validation tooling
indicates jsonschema is an explicit dependency (not assumed present). The only explicit
installation of jsonschema in-repo is in the script-test workflow, not in the fix pipeline runner
scripts.

scripts/process-fix-result.py[16-22]
scripts/process-fix-result.py[162-179]
scripts/post-fix.sh[317-362]
scripts/validate-output-schema.sh[50-55]
.github/workflows/script-test.yml[19-28]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/process-fix-result.py` imports `jsonschema` unconditionally and calls `jsonschema.validate(...)`. The fix pipeline’s post-script (`scripts/post-fix.sh`) runs this script on the Actions runner, but does not install or verify the presence of the `jsonschema` package. If the runner environment doesn’t already have `jsonschema`, the post-fix step will fail with `ModuleNotFoundError` after pushing commits, and the summary comment won’t be posted.

## Issue Context
- The harness already validates agent output against a schema during the validation loop, so the extra validation in `process-fix-result.py` is defense-in-depth but introduces a runtime dependency.
- The repo’s own schema-validation helper explicitly checks for `jsonschema` and fails if it’s missing, implying it is not guaranteed to be present by default.

## Fix Focus Areas
Choose one (or combine):
1) **Remove the runtime dependency**: drop JSON Schema validation in `process-fix-result.py` (or replace with minimal manual validation of required fields/types using stdlib only), since the harness validation loop already enforces the schema.
2) **Make validation best-effort**: wrap the `jsonschema` import in `try/except ImportError`; if missing, log a warning and continue posting the summary (or exit with code `2` so `post-fix.sh` treats it as non-fatal).
3) **Install dependency in runner**: update `scripts/post-fix.sh` to install `jsonschema` if missing (similar to how it installs `pre-commit`).

- scripts/process-fix-result.py[16-22]
- scripts/process-fix-result.py[162-179]
- scripts/post-fix.sh[317-362]

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

---

2. Protected paths changed 📜 Skill insight § Compliance

Description

This PR modifies files under protected governance/infrastructure paths (agents/, scripts/,
skills/), which must not be auto-approved and require explicit human review. Even with an issue
link, these paths require elevated scrutiny due to their ability to affect agent behavior and
automation.

Code

skills/pr-review/SKILL.md[R845-859]

+- **No general footer.** Do not repeat the outcome or include
+  boilerplate about pushes clearing the review. **Exception:** for
+  `request-changes` outcomes only, append an action-hints footer
+  after the findings:
+
+  ```markdown
+  ---
+  **Next steps:**
+  - `/fs-fix` — agent addresses review findings automatically
+  - `/fs-fix <your instruction>` — agent fixes with your specific guidance
+  - Push commits directly — review re-runs automatically on push
+  - `/fs-fix-stop` — disable automatic fix runs for this PR
+  ```
+
+  Omit this footer for `approve`, `comment`, and `reject` outcomes.

Relevance

⭐⭐ Medium

Governance/protected-path scrutiny discussed but outcomes unclear; CODEOWNERS added for review
gating (PR #27/#29).

PR-#27
PR-#29

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538392 requires raising a finding whenever protected governance/infrastructure
paths are modified. This PR changes agents/triage.md, adds/modifies scripts under scripts/, and
modifies skills/pr-review/SKILL.md, all of which are explicitly listed protected paths.

agents/triage.md[318-325]
scripts/process-fix-result.py[1-198]
skills/pr-review/SKILL.md[845-859]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Protected governance/infrastructure paths were modified. These changes must be treated as requiring explicit human review (not auto-approval), and the PR should include clear justification/authorization.

## Issue Context
Protected-path changes include updates to agent/skill instructions and automation scripts.

## Fix Focus Areas
- agents/triage.md[318-325]
- scripts/process-fix-result.py[1-198]
- skills/pr-review/SKILL.md[845-859]

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

---

3. ::warning:: prints unsanitized e.stderr ✓ Resolved 📜 Skill insight ⛨ Security

Description

The new process-fix-result.py emits GitHub Actions workflow commands (::warning::, ::error::)
while interpolating untrusted/variable data (e.g., e.stderr, result_file, schema validation
messages) without sanitization. This can enable workflow-command injection or log spoofing via ::
sequences and newlines in interpolated values.

Code

scripts/process-fix-result.py[R132-135]

+        print(
+            f"::warning::Failed to post PR summary: {e.stderr}",
+            file=sys.stderr,
+        )

Relevance

⭐ Low

Similar “sanitize GitHub Actions workflow command output” suggestion was explicitly rejected by
reviewers (PR #37).

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538382 requires sanitizing every interpolated variable used in GitHub Actions
workflow commands. The added script prints multiple workflow commands while directly interpolating
exception messages, file paths, and other variable content without any sanitization step.

scripts/process-fix-result.py[116-118]
scripts/process-fix-result.py[132-136]
scripts/process-fix-result.py[159-160]
scripts/process-fix-result.py[172-173]
scripts/process-fix-result.py[178-179]
scripts/process-fix-result.py[186-186]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The script prints GitHub Actions workflow commands (e.g., `::warning::`, `::error::`) with interpolated values that are not sanitized. GitHub Actions requires escaping/sanitizing to prevent command injection via `::`, `%0A/%0D`, `%`, ANSI/control characters.

## Issue Context
This PR introduces multiple `print()` statements that emit workflow commands while including exception text, filenames, schema paths, and user-controlled JSON fields (like action `type`). These values must be sanitized individually before interpolation.

## Fix Focus Areas
- scripts/process-fix-result.py[116-118]
- scripts/process-fix-result.py[159-160]
- scripts/process-fix-result.py[172-173]
- scripts/process-fix-result.py[178-179]
- scripts/process-fix-result.py[132-136]
- scripts/process-fix-result.py[186-186]

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

---

4. /fs-* commands embedded 📜 Skill insight ⛨ Security

Description

The PR introduces agent-instruction patterns (e.g., /fs-review, /fs-fix, /fs-code,
/fs-fix-stop) inside string literals and markdown content. This violates the requirement to avoid
agent-instruction-like patterns in committed text, which can be abused for prompt injection or
unintended triggering.

Code

scripts/process-fix-result.py[R91-97]

+    sections.append(
+        "\n---\n"
+        "**Next steps:**\n"
+        "- `/fs-review` — request a re-review of the changes\n"
+        "- `/fs-fix <your instruction>` — run another fix pass with specific guidance\n"
+        "- Push commits directly — review re-runs automatically on push"
+    )

Relevance

⭐ Low

Repo already documents/uses /fs-<agent> commands (PR #12); no historical evidence of banning these
strings.

PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538322 prohibits agent-instruction patterns in committed comments/string
literals/config values. The PR adds multiple /fs-* command strings in newly added/modified content
across the repository, including a Python string literal used to generate PR comments and markdown
documentation.

scripts/process-fix-result.py[91-97]
agents/triage.md[318-325]
skills/pr-review/SKILL.md[845-859]
Skill: code-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The change adds agent instruction patterns like `/fs-fix`, `/fs-review`, `/fs-code`, and `/fs-fix-stop` in committed markdown/text and in a generated comment body string. The compliance rule disallows these patterns in comments/string literals/config values.

## Issue Context
These command-like strings are present in:
- the fix-agent PR comment footer builder
- the triage agent instructions doc
- the pr-review skill doc

If these commands must be displayed to users, represent them in a non-literal form (e.g., escape the leading slash, HTML entity `/`, insert a zero-width space after `/`, or otherwise ensure the raw pattern is not present in repository text).

## Fix Focus Areas
- scripts/process-fix-result.py[91-97]
- agents/triage.md[318-325]
- skills/pr-review/SKILL.md[845-859]

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

---

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:47 AM UTC · Completed 7:02 AM UTC
Commit: a2c10f3 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] scripts/post-review.sh, scripts/post-review-test.sh, scripts/post-triage.sh, scripts/post-triage-test.sh, scripts/process-fix-result.py, scripts/process-fix-result-test.py, skills/pr-review/SKILL.md — 7 of 8 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

• [scope-creep] scripts/post-triage.sh — The PR adds an action-hints footer to triage sufficient comments, but the linked issue (feat(review): add action hints to review bot comments fullsend#1789) only authorizes adding footers to review bot "changes requested" outcomes. The issue title says "add action hints to review bot comments" and the body says the footer should "only appear on 'changes requested' outcomes." Triage is a different agent with a different outcome type.

• [scope-creep] scripts/process-fix-result.py — The PR adds an action-hints footer to fix agent comments via a new 211-line script, but the linked issue only authorizes footers for review bot comments. The fix agent footer lists different commands (/fs-review, /fs-fix <instruction>, push) than the issue's proposed review footer (/fs-fix, /fs-fix <instruction>, push, /fs-fix-stop).

Low

• [test-inadequate] scripts/process-fix-result-test.py — Tests in TestUnknownActionType, TestSchemaValidation, and TestMain.test_empty_actions_fails_schema_validation depend on the jsonschema library being installed. When jsonschema is absent, main() skips validation and returns 0 instead of 1, causing 8 test methods to produce failures rather than skips. Consider adding @unittest.skipUnless(jsonschema is not None, 'requires jsonschema') guards.

• [edge-case] scripts/post-triage.sh — No test covers the combined label_actions + action-hints footer case for triage. When a sufficient outcome has both label actions and the action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested. Note: the review path now tests this combination (label-actions-plus-action-hints-two-hrs in post-review-test.sh).

---

Labels: PR modifies post-scripts and result processing for review, triage, and fix agents.

Previous run

Review

Findings

Medium

• [protected-path] scripts/post-review.sh, scripts/post-review-test.sh, scripts/post-triage.sh, scripts/post-triage-test.sh, scripts/process-fix-result.py, scripts/process-fix-result-test.py, skills/pr-review/SKILL.md — 7 of 8 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

Low

• [test-inadequate] scripts/process-fix-result-test.py — Tests in TestUnknownActionType and TestSchemaValidation depend on the jsonschema library being installed. Without it, main() skips validation and returns 0 instead of 1, causing these tests to fail rather than skip. Consider adding @unittest.skipUnless(jsonschema is not None, 'requires jsonschema') guards.

• [edge-case] scripts/post-triage.sh — No test covers the combined label_actions + action-hints footer case for triage. When a sufficient outcome has both label actions and the action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested on the triage path. Note: the post-review side now tests this combination (label-actions-plus-action-hints-two-hrs).

• [incomplete-doc] docs/review.md, docs/triage.md, docs/fix.md — The three agent doc files describe their post-scripts posting comments/reviews but don't mention the new action-hints footers. The docs are not incorrect but are incomplete about the new feature.

Previous run (2)

Review

Findings

Medium

• [protected-path] scripts/post-review.sh, scripts/post-triage-test.sh, scripts/post-triage.sh, scripts/process-fix-result-test.py, scripts/process-fix-result.py, skills/pr-review/SKILL.md — 6 of 7 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

• [test-inadequate] scripts/post-review-test.sh — The new action-hints footer logic in post-review.sh (appending a "Next steps" footer when ACTION is request-changes) has no test coverage in post-review-test.sh. No test verifies that: (1) the footer is appended for request-changes, (2) the footer is NOT appended for other actions (approve, comment, reject), or (3) the footer content is correct. The triage side has a corresponding test (sufficient-appends-action-hints-footer), making this gap more conspicuous.

Low

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:180 — The atype variable from fix-result.json is interpolated into a ::warning:: GHA workflow command without any sanitization (print(f"::warning::Unknown action type '{atype}' — ignored", ...)), while e.stderr (line 130) and e.message (line 147) both apply .replace('\r', '').replace('\n', ' ').replace('::', ': :'). Apply the same sanitization for consistency.

• [test-inadequate] scripts/process-fix-result-test.py:198 — Tests in TestUnknownActionType and TestSchemaValidation depend on the jsonschema library being installed. Without it, main() skips validation and returns 0 instead of 1, causing tests to fail. Consider adding @unittest.skipUnless guards.

• [edge-case] scripts/post-triage.sh:438 — No test covers the combined label_actions + action-hints footer case. When a sufficient outcome has both label actions and the new action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested.

• [incomplete-doc] docs/fix.md, docs/review.md, docs/triage.md — The three agent doc files describe their post-scripts posting comments/reviews but don't mention the new action-hints footers. The docs are not incorrect but are incomplete about the new feature.

Previous run (3)

Review

Findings

Medium

• [protected-path] scripts/post-review.sh, scripts/post-triage-test.sh, scripts/post-triage.sh, scripts/process-fix-result-test.py, scripts/process-fix-result.py, skills/pr-review/SKILL.md — 6 of 7 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

• [test-inadequate] scripts/post-review-test.sh — The new action-hints footer logic in post-review.sh (appending a "Next steps" footer when ACTION is request-changes) has no test coverage in post-review-test.sh. No test verifies that: (1) the footer is appended for request-changes, (2) the footer is NOT appended for other actions (approve, comment, reject), or (3) the footer content is correct. The triage side has a corresponding test (sufficient-appends-action-hints-footer), making this gap more conspicuous.

Low

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:180 — The atype variable from fix-result.json is interpolated into a ::warning:: GHA workflow command without any sanitization (print(f"::warning::Unknown action type '{atype}' — ignored", ...)), while e.stderr (line 130) and e.message (line 147) both apply .replace('\r', '').replace('\n', ' ').replace('::', ': :'). Apply the same sanitization for consistency.

• [test-inadequate] scripts/process-fix-result-test.py:198 — Tests in TestUnknownActionType and TestSchemaValidation depend on the jsonschema library being installed. Without it, main() skips validation and returns 0 instead of 1, causing tests to fail. Consider adding @unittest.skipUnless guards.

• [edge-case] scripts/post-triage.sh:438 — No test covers the combined label_actions + action-hints footer case. When a sufficient outcome has both label actions and the new action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested.

• [incomplete-doc] docs/fix.md, docs/review.md, docs/triage.md — The three agent doc files describe their post-scripts posting comments/reviews but don't mention the new action-hints footers. The docs are not incorrect but are incomplete about the new feature.

---

Labels: PR modifies protected infrastructure scripts and agent skill definitions to add action-hints footers.

Previous run (4)

Review

Findings

Medium

• [protected-path] scripts/post-triage.sh, scripts/post-triage-test.sh, scripts/process-fix-result.py, scripts/process-fix-result-test.py, skills/pr-review/SKILL.md — 5 of 6 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

Low

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:180 — The atype variable from fix-result.json is interpolated into a ::warning:: GHA workflow command without any sanitization (print(f"::warning::Unknown action type '{atype}' — ignored", ...)), while e.stderr (line 130) and e.message (line 147) both apply .replace('\r', '').replace('\n', ' ').replace('::', ': :'). Apply the same sanitization for consistency.

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:147, :130 — Schema validation error and gh CLI stderr sanitization covers \r, \n, and :: but not %0A/%0D URL-encoded newlines. Practical impact is limited (failure paths only, ::set-env:: disabled by default). Remediation: extend sanitization with .replace('%0A', '').replace('%0D', '').replace('%0a', '').replace('%0d', '').

• [edge-case] scripts/post-triage.sh:426 — No test covers the combined label_actions + action-hints footer case. When a sufficient outcome has both label actions and the new action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested.

• [api-contract] skills/pr-review/SKILL.md:845 — The review agent's action-hints footer relies on the LLM following the SKILL.md prompt instruction rather than programmatic enforcement. Unlike the triage footer (appended in post-triage.sh) and the fix footer (appended in process-fix-result.py), no post-script adds or verifies the review footer. This is consistent with the standard pattern for all SKILL.md formatting instructions.

• [code-organization] Makefile:28 — All 8 existing entries in script-test use bash as the interpreter. The new entry uses python3 scripts/process-fix-result-test.py, which is functionally correct but breaks the all-bash convention.

• [test-inadequate] scripts/process-fix-result-test.py:198, :268 — Tests in TestUnknownActionType and TestSchemaValidation depend on the jsonschema library being installed. Without it, main() skips validation and returns 0 instead of 1, causing tests to fail. Consider adding @unittest.skipUnless(jsonschema is not None, 'jsonschema not installed') guards.

• [edge-case] scripts/post-triage-test.sh:206 — The new sufficient-appends-action-hints-footer test omits the information_gaps field entirely, while all other sufficient tests include it (even as an empty array). This exercises the jq // [] fallback path, which is valid but inconsistent with existing test conventions.

---

Labels: All modified files are in protected paths requiring manual review. The PR adds new agent functionality.

Previous run (5)

Review

Findings

Medium

• [protected-path] scripts/post-triage.sh, scripts/post-triage-test.sh, scripts/process-fix-result.py, scripts/process-fix-result-test.py, skills/pr-review/SKILL.md — 5 of 6 modified files are under protected paths (scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments"). Note: Makefile is not a protected path.

Low

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:180 — The atype variable from fix-result.json is interpolated into a ::warning:: GHA workflow command without any sanitization (print(f"::warning::Unknown action type '{atype}' — ignored", ...)), while e.stderr (line 130) and e.message (line 147) both apply .replace('\r', '').replace('\n', ' ').replace('::', ': :'). Apply the same sanitization for consistency.

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:147, :130 — Schema validation error and gh CLI stderr sanitization covers \r, \n, and :: but not %0A/%0D URL-encoded newlines. Practical impact is limited (failure paths only, ::set-env:: disabled by default). Remediation: extend sanitization with .replace('%0A', '').replace('%0D', '').replace('%0a', '').replace('%0d', '').

• [edge-case] scripts/post-triage.sh:426 — No test covers the combined label_actions + action-hints footer case. When a sufficient outcome has both label actions and the new action-hints footer, the comment body will contain two --- horizontal rules with different footers (**Labels:** then **Next steps:**). The behavior is correct but the combination is untested.

• [api-contract] skills/pr-review/SKILL.md:845 — The review agent's action-hints footer relies on the LLM following the SKILL.md prompt instruction rather than programmatic enforcement. Unlike the triage footer (appended in post-triage.sh) and the fix footer (appended in process-fix-result.py), no post-script adds or verifies the review footer. This is consistent with the standard pattern for all SKILL.md formatting instructions.

• [code-organization] Makefile:28 — All 8 existing entries in script-test use bash as the interpreter. The new entry uses python3 scripts/process-fix-result-test.py, which is functionally correct but breaks the all-bash convention.

• [test-inadequate] scripts/process-fix-result-test.py:198, :268 — Tests in TestUnknownActionType and TestSchemaValidation depend on the jsonschema library being installed. Without it, main() skips validation and returns 0 instead of 1, causing tests to fail. Consider adding @unittest.skipUnless(jsonschema is not None, 'jsonschema not installed') guards.

• [edge-case] scripts/post-triage-test.sh:206 — The new sufficient-appends-action-hints-footer test omits the information_gaps field entirely, while all other sufficient tests include it (even as an empty array). This exercises the jq // [] fallback path, which is valid but inconsistent with existing test conventions.

Previous run (6)

Review

Findings

Medium

• [protected-path] agents/triage.md, scripts/process-fix-result.py, scripts/process-fix-result-test.py, skills/pr-review/SKILL.md — All 4 modified files are under protected paths (agents/, scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. Authorization traced to cross-repo issue feat(review): add action hints to review bot comments fullsend#1789 ("feat(review): add action hints to review bot comments").

Low

• [logic-error] agents/triage.md:318 — The action-hints footer instruction is appended to the general "Comment content rules" section rather than within the sufficient action definition (lines 243–281). The agent will still see it, but placing it inside the sufficient section would improve locality and discoverability.

• [test-inadequate] scripts/process-fix-result-test.py:160 — test_action_hints_footer_present only tests with fix actions. No test covers the disagree-only case or verifies all three footer items (/fs-review, /fs-fix, "Push commits directly").

• [edge-case] scripts/process-fix-result-test.py:175 — test_long_body_truncated verifies the dry-run output path but does not assert that the body was actually truncated to MAX_COMMENT_LENGTH.

• [edge-case] scripts/process-fix-result-test.py:175 — sys.stdout/sys.stderr capture uses direct assignment without try/finally. If post_summary raises, the stream stays redirected for the rest of the test suite. Consider contextlib.redirect_stdout.

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:147 — Schema validation error e.message is interpolated into a ::error:: workflow command without sanitizing newlines or :: sequences. The jsonschema library includes invalid values in error messages, and fix-result.json content could be influenced by prompt injection. Practical impact is limited (only triggers on the failure path; ::set-env:: is disabled by default).
  Remediation: sanitized = e.message.replace('\r', '').replace('\n', ' ').replace('::', ': :')

• [GHA-workflow-cmd-injection] scripts/process-fix-result.py:108 — e.stderr from the gh CLI subprocess is interpolated into ::warning:: without sanitization. GitHub API error responses could reflect user-controlled content.
  Remediation: Same sanitization approach as above.

• [naming-convention] scripts/process-fix-result.py — Kebab-case naming follows the directory convention (consistent with post-fix.sh, pre-code.sh, etc.) but prevents Python module imports. Acceptable since the script is a CLI entry point invoked via python3, never imported.

• [file-permissions] scripts/process-fix-result.py, scripts/process-fix-result-test.py — New files are mode 100644 (not executable). All 22 existing scripts in scripts/ are executable. Run chmod +x for consistency, even though post-fix.sh invokes the script via python3 explicitly.

---

Labels: All modified files are in protected paths requiring manual review. The PR adds new agent functionality.

[waynesun09]

Multi-agent review — #53

Agents: 5 (Claude ×3, Gemini, Codex) | Findings: 14 total, 5 unique MEDIUM+ after dedup and verification against the PR branch

Context: extraction gap

The existing jsonschema dependency finding (already posted by qodo) is not new to this PR — it's a consequence of post-fix.sh being extracted from fullsend with the process-fix-result.py call baked in, while the script itself wasn't extracted. This PR fills that gap. The fix should still land here since this is the PR introducing the script, but it's not a regression.

High (1) — not inline

Tests not wired into CI
scripts/process-fix-result-test.py adds 27 tests but is not listed in the Makefile script-test target. The PR branch Makefile is identical to main. CI runs make script-test, which only runs shell-based test files — these Python tests will never execute in CI.

Suggested fix: Add to the Makefile script-test target:

$(call run-timed,python3 scripts/process-fix-result-test.py)

The CI workflow already installs jsonschema==4.23.0 via uv pip install --system, so the dependency is available in the test path.

Medium (2) — posted inline

1. Truncation silently destroys the footer (process-fix-result.py:91) — 2/5 agents consensus
2. stdout/stderr capture lacks try/finally (process-fix-result-test.py:170) — 3/5 agents consensus

Already posted (skipped)

• jsonschema dependency (qodo, line 22) — already covered
• GHA workflow command injection ×2 (lines 108, 147) — already covered as low

Assisted-by: Claude (review), Gemini (review), Codex (review)

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 7:59 AM UTC · Ended 8:03 AM UTC
Commit: e8381e3 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:04 AM UTC · Completed 8:21 AM UTC
Commit: a57e3b5 · View workflow run →

[waynesun09]

Review squad pass (3 agents: Claude, Claude, Gemini) on the rebased state. My two prior MEDIUM findings (truncation destroying the footer, missing try/finally guards) are confirmed fixed. Posting the 4 new unique MEDIUM findings inline — all relate to the same theme: the footer feature was hardened for triage/fix but the review-agent path and the "don't suggest commands for uninstalled agents" request were left unaddressed, plus one unrelated finding on the comment-length constant.

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 7:56 AM UTC · Ended 7:58 AM UTC
Commit: 5cd495a · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 7:59 AM UTC · Ended 8:00 AM UTC
Commit: 5cd495a · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:00 AM UTC · Completed 8:19 AM UTC
Commit: f2db9e2 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:58 AM UTC · Completed 8:13 AM UTC
Commit: 61fa4f5 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:39 AM UTC · Completed 9:54 AM UTC
Commit: e44fc6a · View workflow run →

[fullsend-ai-review]

See the review comment for full details.