Skip to content

fix: escape HTML comments in review skill, add output schema to agent def #1111

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
waynesun09 wants to merge 3 commits into from
Closed

fix: escape HTML comments in review skill, add output schema to agent def #1111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8e802f
fix: escape Head SHA HTML comment to pass context scan
waynesun09 May 18, 2026
db6878a
docs: add pipeline output JSON schema to review agent definition
waynesun09 May 18, 2026
5efb0ee
fix: address review squad findings on review docs
waynesun09 May 18, 2026
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
47 changes: 46 additions & 1 deletion internal/scaffold/fullsend-repo/agents/review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,52 @@ approve the PR and mark concrete follow-up work as `actionable: true`
in the structured result so the post-script can create tracking issues.

The `code-review` skill defines the finding structure. The `pr-review`
skill defines the GitHub review comment format.
skill defines the review comment format and procedure.

### Pipeline output JSON schema

In pipeline mode (`$FULLSEND_OUTPUT_DIR` set), the agent writes
`agent-result.json`. The harness validates this against
`schemas/review-result.schema.json` (source of truth) before the
post-script runs. The JSON must conform to the following schema:

**Top-level object** (`additionalProperties: false`):

| Field | Type | Always required | Description |
|-------------|---------|-----------------|--------------------------------------------------|
| `action` | string | yes | One of: `approve`, `request-changes`, `comment`, `reject`, `failure` |
| `pr_number` | integer | yes | PR number (minimum 1) |
| `repo` | string | yes | `owner/repo` format (pattern: `^[^/]+/[^/]+$`) |
| `head_sha` | string | conditional | Commit SHA (min 7 chars) |
| `body` | string | conditional | Markdown review comment (min 1 char) |
| `findings` | array | conditional | Array of finding objects (min 1 item when present)|
| `reason` | string | conditional | One of: `tool-failure`, `missing-context`, `ambiguous-findings`, `token-limit` |

**Required fields per action:**

| Action | Required fields |
|-------------------|------------------------------------------|

@fullsend-ai-review fullsend-ai-review Bot May 18, 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.

[low] incomplete-fix

The literal HTML comment pattern inside the Failure output code fence was not escaped. The PR fixes this pattern in SKILL.md but misses this instance in agents/review.md, which is also modified in this PR. If the scanner does not respect code fences, this will still trigger a hidden_html_comment finding.

Suggested fix: Apply the same [open]/[close] workaround used in SKILL.md step 6 to the failure output example in agents/review.md.

| `approve` | `body`, `head_sha` |
| `request-changes` | `body`, `head_sha`, `findings` |
| `comment` | `body`, `head_sha` |
| `reject` | `body`, `head_sha`, `findings` |
| `failure` | `reason` |

**Finding object** (`additionalProperties: false`):

| Field | Type | Required | Description |
|---------------|---------|----------|-----------------------------------------------|
| `severity` | string | yes | One of: `critical`, `high`, `medium`, `low`, `info` |
| `category` | string | yes | Finding category (min 1 char) |
| `file` | string | yes | File path (min 1 char) |
| `line` | integer | no | Line number (minimum 1) |
| `description` | string | yes | Finding description (min 1 char) |
| `remediation` | string | no | Suggested fix |
| `actionable` | boolean | no | When true on low/info findings in an `approve` result, the post-script creates follow-up issues |

Schema validation failures trigger a harness retry iteration. Ensure
every `agent-result.json` includes all required fields for its action
and contains no extra properties.

## Exit code contract

Expand Down
14 changes: 11 additions & 3 deletions internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,17 @@ and re-evaluate the overall outcome.

Compose the review comment using this structure:

```markdown
<!-- **Head SHA:** <sha> -->
The first line must be an HTML comment embedding the head SHA.
Construct it by concatenating: the HTML comment open delimiter,
a space, `**Head SHA:**`, a space, the SHA value, a space, and
the HTML comment close delimiter. For example, if the SHA were
`abc123`, the line would read (with no line break):

[open] **Head SHA:** abc123 [close]

where `[open]` = `<` + `!--` and `[close]` = `--` + `>`.

```markdown
## Review

### Findings
Expand Down Expand Up @@ -422,7 +430,7 @@ wins.
skills first.** Partial reviews miss context and produce unreliable
verdicts.
- **Always include the PR head SHA in a hidden HTML comment.** The
SHA must appear as `<!-- **Head SHA:** <sha> -->` so the re-review
SHA must appear in the format described in step 6 so the re-review
anchoring script can extract it, but it must not be visible to
reviewers.
- **Report failure rather than posting a partial review.** If you cannot
Expand Down
Loading