Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions docs/ADRs/0053-agent-driven-branch-targeting.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ Add `schemas/code-result.schema.json`:
"type": "string",
"description": "Branch the PR should target.",
"pattern": "^[a-zA-Z0-9._/-]+$"
},
"pr_body": {
"type": "string",
"maxLength": 65536,
"description": "PR description used as PR body instead of commit body."
}
}
}
Expand All @@ -110,6 +115,13 @@ Add `schemas/code-result.schema.json`:
The agent determines the target branch from the issue context and writes
`code-result.json` to `$FULLSEND_OUTPUT_DIR`.

**Amendment (PR #2979):** Added optional `pr_body` field so the agent can
provide a PR description independently of the commit body. When the repo has
a PR template, the agent structures `pr_body` to match. The post-script uses
`pr_body` verbatim as the PR description when present, falling back to the
commit body otherwise. `pr_body` is optional — omitting it preserves the
original commit-body-as-description behavior.

### Post-script policy gate

Replace the `TARGET_BRANCH="${TARGET_BRANCH:-main}"` line in `post-code.sh`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "code-result.schema.json",
"title": "Code Agent Result",
"description": "Structured output from the code agent documenting the target branch for PR creation.",
"description": "Structured output from the code agent documenting the target branch and PR body for PR creation.",
"type": "object",
"required": ["target_branch"],
"additionalProperties": false,
Expand All @@ -11,6 +11,11 @@
"type": "string",
"description": "Branch the PR should target. Determined by the agent from issue context.",
"pattern": "^[a-zA-Z0-9._/-]+$"
},
"pr_body": {
"type": "string",
"maxLength": 65536,
"description": "PR description used as PR body instead of commit body. Supports markdown formatting and template-compliant structure without gitlint line-length constraints."
}
}
}
87 changes: 86 additions & 1 deletion internal/scaffold/fullsend-repo/scripts/post-code-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,14 @@ build_pr_body() {
local issue_number="$2"
local branch="$3"
local scan_range="$4"
local pr_body_from_result="${5:-}" # optional: agent-provided pr_body

local description
if [ -z "${commit_body}" ]; then
if [ -n "${pr_body_from_result}" ]; then
# Agent provided pr_body (new path).
# Strip Signed-off-by and Closes lines so script appends them once.
description="$(printf '%s\n' "${pr_body_from_result}" | sed '/^Signed-off-by:/d' | sed '/^Closes #/d; /^Closes [a-zA-Z0-9_.-]*\/[a-zA-Z0-9_.-]*#/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')"
elif [ -z "${commit_body}" ]; then
description="Automated implementation for issue #${issue_number}."
else
description="${commit_body}"
Expand Down Expand Up @@ -250,6 +255,86 @@ count_closes_test "single-closes-with-body" \
count_closes_test "single-closes-empty-body" \
"" "99"

# Verify pr_body path strips Closes lines (agent may include them)
count_closes_pr_body_test() {
local test_name="$1"
local pr_body="$2"
local issue_number="$3"

local actual
actual="$(build_pr_body "" "${issue_number}" "agent/${issue_number}-fix" "abc123..def456" "${pr_body}")"

local count
count=$(echo "${actual}" | grep -c "Closes #${issue_number}" || true)

if [ "${count}" -ne 1 ]; then
echo "FAIL: ${test_name}"
echo " expected exactly 1 'Closes #${issue_number}', found ${count}"
echo " in body:"
echo "${actual}" | sed 's/^/ /'
FAILURES=$((FAILURES + 1))
return
fi

echo "PASS: ${test_name}"
}

count_closes_pr_body_test "single-closes-pr-body-with-closes" \
"## Summary

Implemented widget rendering.

Closes #42" "42"

# --- pr_body path test cases ---

# Helper for pr_body tests (fifth arg is pr_body from result file)
run_pr_body_test() {
local test_name="$1"
local pr_body="$2"
local issue_number="$3"
local branch="$4"
local check_pattern="$5"
local expect_present="$6" # "yes" or "no"

local actual
actual="$(build_pr_body "" "${issue_number}" "${branch}" "abc123..def456" "${pr_body}")"

if [ "${expect_present}" = "yes" ]; then
if ! echo "${actual}" | grep -qF "${check_pattern}"; then
echo "FAIL: ${test_name}"
echo " expected to find: '${check_pattern}'"
echo " in body:"
echo "${actual}" | sed 's/^/ /'
FAILURES=$((FAILURES + 1))
return
fi
else
if echo "${actual}" | grep -qF "${check_pattern}"; then
echo "FAIL: ${test_name}"
echo " expected NOT to find: '${check_pattern}'"
echo " in body:"
echo "${actual}" | sed 's/^/ /'
FAILURES=$((FAILURES + 1))
return
fi
fi

echo "PASS: ${test_name}"
}

# pr_body provided by agent should appear in final PR body
run_pr_body_test "pr-body-from-result" \
$'## Summary\n\nAdded widget rendering.\n\n## Testing\n\nManual test.' \
"42" "agent/42-widget" \
"Added widget rendering." "yes"

# pr_body should NOT be word-wrapped (it's verbatim)
run_pr_body_test "pr-body-verbatim" \
$'## Summary\n\nThis is a very long line that would normally be word-wrapped by the legacy commit-body awk logic but should remain intact when coming from pr_body.' \
"42" "agent/42-widget" \
"This is a very long line that would normally be word-wrapped by the legacy commit-body awk logic but should remain intact when coming from pr_body." "yes"

# ---------------------------------------------------------------------------
# Test helper — reimplements the no-op detection logic from post-code.sh
# so we can test it without a git repo or network access.
Expand Down
31 changes: 22 additions & 9 deletions internal/scaffold/fullsend-repo/scripts/post-code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,28 @@ fi
echo "Creating PR..."

COMMIT_SUBJECT="$(git log -1 --format='%s' HEAD)"
COMMIT_BODY_RAW="$(git log -1 --format='%b' HEAD | sed '/^Signed-off-by:/d' | sed '/^Closes #/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')"

COMMIT_BODY="$(echo "${COMMIT_BODY_RAW}" | awk '
/^$/ { if (buf) print buf; print; buf=""; next }
/^[-*#>]|^ / { if (buf) print buf; buf=""; print; next }
/^Closes / { if (buf) print buf; buf=""; print; next }
{ buf = (buf ? buf " " $0 : $0) }
END { if (buf) print buf }
')"

# Read pr_body from agent output. Fall back to commit body if absent.
PR_BODY_FROM_RESULT=""
if [ -n "${RESULT_FILE}" ]; then
PR_BODY_FROM_RESULT="$(jq -r '.pr_body // empty' "${RESULT_FILE}" 2>/dev/null || true)"
fi

if [ -n "${PR_BODY_FROM_RESULT}" ]; then
# Agent provided pr_body (template-aware or best-effort).
# Strip Signed-off-by and Closes lines so the script appends them once.
COMMIT_BODY="$(printf '%s\n' "${PR_BODY_FROM_RESULT}" | sed '/^Signed-off-by:/d' | sed '/^Closes #/d; /^Closes [a-zA-Z0-9_.-]*\/[a-zA-Z0-9_.-]*#/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')"
else
# Fall back to unwrapped commit body (legacy path)
COMMIT_BODY_RAW="$(git log -1 --format='%b' HEAD | sed '/^Signed-off-by:/d' | sed '/^Closes #/d' | sed -e :a -e '/^\n*$/{ $d; N; ba; }')"
COMMIT_BODY="$(echo "${COMMIT_BODY_RAW}" | awk '
/^$/ { if (buf) print buf; print; buf=""; next }
/^[-*#>]|^ / { if (buf) print buf; buf=""; print; next }
/^Closes / { if (buf) print buf; buf=""; print; next }
{ buf = (buf ? buf " " $0 : $0) }
END { if (buf) print buf }
')"
fi
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Comment on lines +445 to +465

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Duplicate closes lines 🐞 Bug ≡ Correctness

When code-result.json contains pr_body, post-code.sh uses it verbatim as DESCRIPTION but still
appends its own “Closes #${ISSUE_NUMBER}” footer, so any pr_body that already includes a Closes line
produces duplicates in the PR body. This contradicts the updated skill examples and the
post-code-test.sh expectation of exactly one footer Closes line.
Agent Prompt
## Issue description
When `pr_body` is present, `post-code.sh` sets `COMMIT_BODY` directly from it and later appends its own `Closes #${ISSUE_NUMBER}` footer. Because the skill examples for `pr_body` include a `Closes #<issue-number>` line, PRs will frequently end up with duplicated close directives.

## Issue Context
Legacy behavior stripped `Closes #...` lines from the commit body before building the PR body, ensuring a single footer close line. The new `pr_body` path bypasses that stripping.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[445-464]
- internal/scaffold/fullsend-repo/scripts/post-code.sh[485-503]
- internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md[199-215]
- internal/scaffold/fullsend-repo/scripts/post-code-test.sh[188-245]

## Implementation guidance
- Option A (recommended): strip `^Closes ` / `^Closes #` (and optionally `^Signed-off-by:`) from `PR_BODY_FROM_RESULT` the same way the legacy commit-body path does, so the script remains the single source of truth for the close footer.
- Option B: stop appending the footer `Closes #${ISSUE_NUMBER}` when `pr_body` is provided (or only append it if it’s not already present).
- Update skill examples accordingly (if keeping the footer append, remove `Closes #...` from `pr_body` examples).

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


# ---------------------------------------------------------------------------
# Ensure PR title includes an issue reference.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,16 @@ and `Glob` to inspect project configuration:
`check-pr-title` action with a regex). If the repo requires a specific
format like `type(TICKET): description`, note the convention — you will
use it when writing the commit subject in step 10.
5. **Check for PR template.** Look for `.github/pull_request_template.md`,
`.github/PULL_REQUEST_TEMPLATE.md`, or `docs/pull_request_template.md`.
If a directory `.github/PULL_REQUEST_TEMPLATE/` exists with multiple
templates, use the one matching the issue context (e.g., `bug_report.md`
for bug fixes, `feature_request.md` for enhancements), or fall back to
the first/default template. If found, read it and note sections/prompts
— skip HTML comments (`<!-- ... -->`), extract only the visible headings
and prompts. You will structure the `pr_body` field in the result file
to match template sections (see the structured output block below) — the post-script uses
`pr_body` as the PR description.

From these files, determine:

Expand All @@ -186,21 +196,42 @@ use that branch. Otherwise, determine the repo's default branch:
git rev-parse --abbrev-ref origin/HEAD | cut -d/ -f2
```

Write your chosen branch to the structured output file so the post-script
knows which branch to target the PR against:
Write the structured output file with target branch and PR body. The
post-script uses pr_body as the PR description (no gitlint line-length
constraints) and automatically appends `Closes #<issue-number>` — do not
include it in pr_body. If PR template found in step 3 item 5, structure
pr_body per template. Otherwise, write best-effort description.

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] documentation-coherence

Two separate heredoc examples for with-template and without-template cases are presented sequentially with only comments distinguishing them. A literal-minded agent could run both (second overwrites first).

Use `jq -n` to build JSON safely — avoids shell expansion of `$` and
backticks in pr_body content (common in markdown code blocks). Run exactly
one of the two branches below depending on whether a PR template was found:

```bash
mkdir -p "${FULLSEND_OUTPUT_DIR}"
cat > "${FULLSEND_OUTPUT_DIR}/${FULLSEND_OUTPUT_FILE}" <<RESULT
{
"target_branch": "<branch-name>"
}
RESULT

if [ -n "${pr_template}" ]; then
# PR template found — structure pr_body to match template sections
jq -n \
--arg tb "<branch-name>" \
--arg pb "$(printf '## Summary\n\n<what changed and why>\n\n## Testing\n\n<test approach>')" \
'{target_branch: $tb, pr_body: $pb}' \
> "${FULLSEND_OUTPUT_DIR}/${FULLSEND_OUTPUT_FILE}"
else
# No PR template — best-effort description
jq -n \
--arg tb "<branch-name>" \
--arg pb "<What changed and why. Testing approach. Any caveats.>" \
'{target_branch: $tb, pr_body: $pb}' \
> "${FULLSEND_OUTPUT_DIR}/${FULLSEND_OUTPUT_FILE}"
fi
```

Write this output early (during planning, after determining the target
branch) so it is available even if the agent hits a timeout or error later.
The post-script validates this against the repo's allowed branches.
Write `target_branch` early (during planning) so it's available if the
agent times out later. You may write a draft `pr_body` at planning time
(template headings with placeholder text) and update it after
implementation with actual details. The post-script validates
target_branch against allowed branches and uses pr_body as PR
description.

### 4. Check for existing branch

Expand Down Expand Up @@ -670,10 +701,12 @@ that fits.
**Body line length — comply with the repo's gitlint config:**

If `.gitlint` has a `[body-max-line-length]` rule (e.g. `line-length=72`),
you **MUST** hard-wrap body text at that limit. This is enforced by CI.
The post-script will unwrap the body when building the PR description,
so your hard-wrapped commit body will still render as nice prose on
GitHub.
you **MUST** hard-wrap commit body text at that limit. This is enforced
by CI. The post-script will unwrap the commit body when building the PR
description (legacy path), so your hard-wrapped commit body will still
render as nice prose on GitHub. When you provide `pr_body` in the result
file, the post-script uses it verbatim (no unwrapping), so `pr_body` is
not subject to gitlint line-length constraints.

Hard-wrap guidelines when a limit is configured:
- Break lines at word boundaries before hitting the limit
Expand All @@ -695,6 +728,9 @@ The commit body should:
- Summarize which files/functions were modified and the approach
- Note any trade-offs, assumptions, or edge cases

Keep commit body concise (respects gitlint line-length limits). PR body
in code-result.json holds the template-structured description if needed.

```bash
git commit -m "<type>(#<number>): <short-description>

Expand Down
Loading