Skip to content
Merged
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
95 changes: 95 additions & 0 deletions scripts/post-code-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,62 @@ run_noop_comment_test() {
echo "PASS: ${test_name}"
}

# ---------------------------------------------------------------------------
# Test helper — reimplements the branch validation logic from post-code.src.sh
# to test the auto-correct vs hard-fail behavior. Given an agent target, a
# default branch, and an optional allowed list, returns the decision.
# ---------------------------------------------------------------------------
validate_target_branch() {
local agent_target="$1"
Comment thread
rh-hemartin marked this conversation as resolved.
local default_branch="$2"
Comment thread
rh-hemartin marked this conversation as resolved.
local allowed_list="$3" # empty string means unset

if [ -n "${agent_target}" ]; then
if [ -n "${allowed_list}" ]; then
# Explicit allowed list — hard-fail if not in it.
if [ "${allowed_list}" = "*" ] \
|| echo ",${allowed_list}," | grep -qF ",${agent_target},"; then
echo "accept:${agent_target}"
else
echo "reject:${agent_target}:allowed=${allowed_list}"
fi
else
# No explicit list — auto-correct to default when mismatched.
if [ "${agent_target}" = "${default_branch}" ]; then
echo "accept:${agent_target}"
else
echo "auto-correct:${default_branch}"
fi
fi
else
echo "default:${default_branch}"
fi
}

run_branch_validation_test() {
local test_name="$1"
local agent_target="$2"
local default_branch="$3"
local allowed_list="$4"
local expected_prefix="$5"

local actual
actual="$(validate_target_branch "${agent_target}" "${default_branch}" "${allowed_list}")"

if [[ "${actual}" != ${expected_prefix}* ]]; then
echo "FAIL: ${test_name}"
echo " agent_target: '${agent_target}'"
echo " default_branch: '${default_branch}'"
echo " allowed_list: '${allowed_list}'"
echo " expected prefix: '${expected_prefix}'"
echo " actual: '${actual}'"
FAILURES=$((FAILURES + 1))
return
fi

echo "PASS: ${test_name}"
}

# --- No-op comment test cases ---

# Comment should include the reason for no feature branch
Expand Down Expand Up @@ -1170,6 +1226,45 @@ else
echo "PASS: script-has-noop-comment"
fi

# --- Branch validation test cases ---

# Auto-correct: agent writes main, default is master, no allowed list → corrected
run_branch_validation_test "auto-correct-to-default" \
"main" "master" "" "auto-correct:master"

# Explicit list enforced: agent writes main, allowed=release-1,release-2 → reject
run_branch_validation_test "explicit-list-rejects-mismatch" \
"main" "master" "release-1,release-2" "reject:main"

# Match: agent matches default, no allowed list → accepted
run_branch_validation_test "agent-matches-default" \
"main" "main" "" "accept:main"

# Wildcard: allowed=*, agent writes develop → accepted
run_branch_validation_test "wildcard-allows-any" \
"develop" "main" "*" "accept:develop"

# No agent target: falls back to default
run_branch_validation_test "no-agent-target-uses-default" \
"" "master" "" "default:master"

# Agent matches explicit list
run_branch_validation_test "explicit-list-accepts-match" \
"release-1" "main" "release-1,release-2" "accept:release-1"

# Agent matches default with explicit list that also includes default
run_branch_validation_test "explicit-list-includes-default" \
"main" "main" "main,develop" "accept:main"

# No agent target with explicit list still uses default
run_branch_validation_test "no-agent-target-ignores-allowed-list" \
"" "main" "release-1,release-2" "default:main"

# Substring mismatch: agent writes "release" but only "release-1","release-2"
# are allowed — comma-wrapping must reject the partial match.
run_branch_validation_test "substring-not-accepted" \
"release" "main" "release-1,release-2" "reject:release"

# --- Summary ---

echo ""
Comment thread
rh-hemartin marked this conversation as resolved.
Expand Down
24 changes: 18 additions & 6 deletions scripts/post-code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -744,13 +744,25 @@ fi
DEFAULT_BRANCH="$(GH_TOKEN="${PUSH_TOKEN}" gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo 'main')"

if [ -n "${AGENT_TARGET}" ]; then
ALLOWED="${CODE_ALLOWED_TARGET_BRANCHES:-${DEFAULT_BRANCH}}"
if [ "${ALLOWED}" = "*" ] || echo ",${ALLOWED}," | grep -qF ",${AGENT_TARGET},"; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — allowed"
if [ -n "${CODE_ALLOWED_TARGET_BRANCHES:-}" ]; then
# Explicit allowed list — hard-fail if agent's choice is not in it.
if [ "${CODE_ALLOWED_TARGET_BRANCHES}" = "*" ] \
|| echo ",${CODE_ALLOWED_TARGET_BRANCHES}," | grep -qF ",${AGENT_TARGET},"; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — allowed"
else
post_fail_to_issue branch-validation \
"Agent requested branch '${AGENT_TARGET}' but allowed branches are: ${CODE_ALLOWED_TARGET_BRANCHES}"
fi
else
post_fail_to_issue branch-validation \
"Agent requested branch '${AGENT_TARGET}' but allowed branches are: ${ALLOWED}"
# No explicit list — auto-correct to API-discovered default when mismatched.
if [ "${AGENT_TARGET}" = "${DEFAULT_BRANCH}" ]; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — matches default"
else
TARGET_BRANCH="${DEFAULT_BRANCH}"
gha_echo warning "Agent requested branch '${AGENT_TARGET}' but default branch is '${DEFAULT_BRANCH}' — auto-correcting"
fi
fi
else
TARGET_BRANCH="${DEFAULT_BRANCH}"
Expand Down
24 changes: 18 additions & 6 deletions scripts/post-code.src.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,25 @@ fi
DEFAULT_BRANCH="$(GH_TOKEN="${PUSH_TOKEN}" gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo 'main')"

if [ -n "${AGENT_TARGET}" ]; then
ALLOWED="${CODE_ALLOWED_TARGET_BRANCHES:-${DEFAULT_BRANCH}}"
if [ "${ALLOWED}" = "*" ] || echo ",${ALLOWED}," | grep -qF ",${AGENT_TARGET},"; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — allowed"
if [ -n "${CODE_ALLOWED_TARGET_BRANCHES:-}" ]; then
# Explicit allowed list — hard-fail if agent's choice is not in it.
if [ "${CODE_ALLOWED_TARGET_BRANCHES}" = "*" ] \
|| echo ",${CODE_ALLOWED_TARGET_BRANCHES}," | grep -qF ",${AGENT_TARGET},"; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — allowed"
else
post_fail_to_issue branch-validation \
"Agent requested branch '${AGENT_TARGET}' but allowed branches are: ${CODE_ALLOWED_TARGET_BRANCHES}"
fi
else
post_fail_to_issue branch-validation \
"Agent requested branch '${AGENT_TARGET}' but allowed branches are: ${ALLOWED}"
# No explicit list — auto-correct to API-discovered default when mismatched.
Comment thread
rh-hemartin marked this conversation as resolved.
if [ "${AGENT_TARGET}" = "${DEFAULT_BRANCH}" ]; then
TARGET_BRANCH="${AGENT_TARGET}"
echo "Agent requested branch '${TARGET_BRANCH}' — matches default"
else
TARGET_BRANCH="${DEFAULT_BRANCH}"
gha_echo warning "Agent requested branch '${AGENT_TARGET}' but default branch is '${DEFAULT_BRANCH}' — auto-correcting"
fi
fi
else
TARGET_BRANCH="${DEFAULT_BRANCH}"
Expand Down
29 changes: 25 additions & 4 deletions skills/code-implementation/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,24 +188,45 @@ From these files, determine:

Determine the correct target branch from the issue context. If the issue
references a specific branch (e.g., "set up builds on the 3.18 branch"),
use that branch. Otherwise, determine the repo's default branch:
use that branch. Otherwise, determine the repo's default branch by trying
these commands in order until one succeeds:

```bash
git rev-parse --abbrev-ref origin/HEAD | cut -d/ -f2
# Try each discovery method; use the first that returns a non-empty value.
DEFAULT_BRANCH=""
DEFAULT_BRANCH="$(gh repo view --json defaultBranchRef \
--jq '.defaultBranchRef.name' 2>/dev/null)" || true
if [ -z "${DEFAULT_BRANCH}" ]; then
DEFAULT_BRANCH="$(git rev-parse --abbrev-ref origin/HEAD 2>/dev/null \
| sed 's|^origin/||')" || true
fi
if [ -z "${DEFAULT_BRANCH}" ] || [ "${DEFAULT_BRANCH}" = "HEAD" ]; then
DEFAULT_BRANCH="$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null \
Comment thread
rh-hemartin marked this conversation as resolved.
| sed 's|^refs/remotes/origin/||')" || true
fi
```

**Do not skip discovery and assume `"main"`.** If all discovery methods
fail, `${DEFAULT_BRANCH:-main}` provides a last-resort fallback — but
the post-script will auto-correct it to the API-discovered default branch
when no explicit allowed list is configured. Getting discovery right here
avoids an unnecessary correction and the warning that goes with it.

Write the structured output file with the target branch now. Write only
`target_branch` at this stage — `pr_body` is added after implementation
(step 10d) so a timeout never leaks placeholder text as the PR description.

```bash
mkdir -p "${FULLSEND_OUTPUT_DIR}"

jq -n --arg tb "<branch-name>" '{target_branch: $tb}' \
jq -n --arg tb "${DEFAULT_BRANCH:-main}" '{target_branch: $tb}' \
> "${FULLSEND_OUTPUT_DIR}/agent-result.json"
```

The post-script validates `target_branch` against allowed branches.
The post-script validates `target_branch` against allowed branches. When
no explicit `CODE_ALLOWED_TARGET_BRANCHES` list is configured, the
post-script auto-corrects to the API-discovered default branch if the
agent's value does not match.

### 4. Check for existing branch

Expand Down
Loading