Skip to content

fix: use jq for review body JSON to prevent double-escaped newlines#80

Merged
derekmisler merged 2 commits intodocker:mainfrom
derekmisler:fix-review-body-newlines
Mar 9, 2026
Merged

fix: use jq for review body JSON to prevent double-escaped newlines#80
derekmisler merged 2 commits intodocker:mainfrom
derekmisler:fix-review-body-newlines

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

@derekmisler derekmisler commented Mar 9, 2026

Summary

The PR review agent was constructing its GitHub API payload by manually building JSON strings with echo, which caused \n sequences to be double-escaped and rendered as literal text rather than real newlines in posted reviews. This fix updates the agent's instructions in review-pr/agents/pr-review.yaml to use jq for JSON construction, ensuring newlines are properly escaped.

Changes

  • review-pr/agents/pr-review.yaml: Replaces the echo-based JSON payload construction with a jq-based approach. The new instructions direct the agent to assign the review body using a heredoc/variable (with real newlines), build the comments array via jq -n --arg/--argjson, and assemble the final payload with jq --slurpfile before piping to gh api.

How to Test

  • Trigger a PR review using the agent and verify that the posted GitHub review body renders newlines correctly (e.g., ### Assessment: appears on its own line rather than \n### Assessment:).
  • Confirm inline comment bodies also render markdown formatting correctly without literal \n characters appearing in the GitHub UI.

@derekmisler
Copy link
Copy Markdown
Contributor Author

/describe

@docker-agent
Copy link
Copy Markdown
Contributor

docker-agent bot commented Mar 9, 2026

✅ PR description has been generated and updated!

@derekmisler derekmisler marked this pull request as ready for review March 9, 2026 20:37
@derekmisler derekmisler requested a review from a team as a code owner March 9, 2026 20:37
@derekmisler derekmisler self-assigned this Mar 9, 2026
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Review Summary

Assessment: 🔴 CRITICAL

Summary

This PR improves the agent instructions by replacing echo-based JSON construction with jq to prevent double-escaped newlines. However, the provided example code has a critical logic error that would cause it to lose all but the last comment when processing multiple findings.

Critical Issues Found

  1. HIGH: The jq comment-building pattern overwrites the temp file on each iteration
  2. MEDIUM: The example uses literal placeholder text without showing variable substitution

Recommendation

The fix is correct in principle (using jq instead of echo), but the example implementation needs revision to handle multiple findings correctly. Consider either:

  • Building all comments in a single jq invocation
  • Using append operations instead of overwrite (>)
  • Making the example more explicit about variable substitution

- Initialize empty JSON array, then append each finding with jq instead
  of overwriting the file on each iteration (loses all but last comment)
- Replace literal ISSUE/DETAILS placeholders with $file_path,
  $line_number, $comment_body variables to clarify substitution
@derekmisler
Copy link
Copy Markdown
Contributor Author

/review

Copy link
Copy Markdown
Contributor

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Review Summary

Assessment: 🟢 APPROVE

This PR improves JSON payload construction by replacing manual echo-based string building with jq-based construction, which correctly handles newline escaping. This is a documentation improvement that will help prevent double-escaped newlines in GitHub review posts.

Summary

The changes update agent instructions to use jq for JSON construction rather than manual string building. One minor documentation issue was found in the example code - the loop structure comment references variables without showing how they should be populated from previous steps.


1 finding | 0 critical | 0 high | 0 medium | 1 low

Copy link
Copy Markdown

@krissetto krissetto left a comment

Choose a reason for hiding this comment

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

we could probably use a custom tool definition to format things a bit more reliably compared to just prompting 🤔

@derekmisler derekmisler merged commit 4a1bda4 into docker:main Mar 9, 2026
11 checks passed
@derekmisler derekmisler deleted the fix-review-body-newlines branch March 9, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants