Skip to content

Conversation

@szokeasaurusrex
Copy link
Member

Move the conditional PR template logic from SKILL.md instructions into a deterministic bash script.

Previously, the skill instructed the LLM to run gh repo view --json pullRequestTemplates and interpret whether a template exists. This relied on the LLM correctly handling empty/null output.

Now, a script handles this logic deterministically:

  • Fetches the repo's PR template via gh
  • Falls back to the default template if none exists
  • Outputs the appropriate template for the LLM to follow

This makes the template retrieval reliable and testable.


Stacked on #18

🤖 Generated with Claude Code

Move the conditional PR template logic from SKILL.md instructions into
a deterministic bash script. This makes the template retrieval reliable
and testable rather than depending on the LLM to interpret empty/null
output from the gh command.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Comment on lines +4 to +7
template=$(gh repo view --json pullRequestTemplates --jq '.pullRequestTemplates[0].body' 2>/dev/null)

if [ -n "$template" ]; then
echo "$template"
Copy link

Choose a reason for hiding this comment

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

Bug: The script incorrectly handles missing PR templates. jq outputs a literal "null" string, which the -n check considers non-empty, preventing the fallback template from being used.
Severity: MEDIUM

🔍 Detailed Analysis

The script at get-pr-template.sh uses jq to fetch a pull request template. When no template exists, jq without the -r flag outputs the literal string "null". The subsequent check if [ -n "$template" ] incorrectly evaluates to true because the string "null" is not empty. As a result, the script echoes the literal string null to the user, and the intended fallback logic to provide a default template is never executed. This occurs whenever a repository lacks a pull request template, defeating the purpose of the fallback.

💡 Suggested Fix

Modify the jq command to handle null values correctly. Either add the -r (raw output) flag, which converts null to an empty string, or use a jq filter that explicitly converts null to an empty string, such as .pullRequestTemplates[0].body // empty. This will ensure the [ -n "$template" ] check fails as expected, triggering the fallback logic.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: plugins/sentry-skills/skills/create-pr/scripts/get-pr-template.sh#L4-L7

Potential issue: The script at `get-pr-template.sh` uses `jq` to fetch a pull request
template. When no template exists, `jq` without the `-r` flag outputs the literal string
`"null"`. The subsequent check `if [ -n "$template" ]` incorrectly evaluates to true
because the string `"null"` is not empty. As a result, the script echoes the literal
string `null` to the user, and the intended fallback logic to provide a default template
is never executed. This occurs whenever a repository lacks a pull request template,
defeating the purpose of the fallback.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8536186

Base automatically changed from feat/pr-template-support to main January 13, 2026 13:20
@szokeasaurusrex
Copy link
Member Author

szokeasaurusrex commented Jan 13, 2026

Closing this PR - on further reflection, moving the fallback template to a script removes it from context when a repo has a template, which is undesirable. The original approach in #18 (keeping the fallback inline in SKILL.md) ensures the guidance is always available, and can also be considered when a template is in place.

The script doesn't add enough value to justify the extra file.

🤖 This comment is partially AI-generated

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