diff --git a/.github/workflows/reusable-code.yml b/.github/workflows/reusable-code.yml index 2eb72770b0..c9c30841e8 100644 --- a/.github/workflows/reusable-code.yml +++ b/.github/workflows/reusable-code.yml @@ -179,7 +179,7 @@ jobs: REPO_FULL_NAME: ${{ inputs.source_repo }} PUSH_TOKEN: ${{ steps.app-token.outputs.token }} PUSH_TOKEN_SOURCE: github-app - TARGET_BRANCH: main + CODE_ALLOWED_TARGET_BRANCHES: '' with: agent: code version: ${{ inputs.fullsend_version }} diff --git a/docs/ADRs/0053-agent-driven-branch-targeting.md b/docs/ADRs/0053-agent-driven-branch-targeting.md new file mode 100644 index 0000000000..206355c1d7 --- /dev/null +++ b/docs/ADRs/0053-agent-driven-branch-targeting.md @@ -0,0 +1,180 @@ +--- +title: "53. Agent-driven branch targeting for the code agent" +status: Accepted +relates_to: + - agent-architecture +topics: + - code-agent + - post-script + - branch-targeting + - structured-output +--- + +# 53. Agent-driven branch targeting for the code agent + +Date: 2026-06-17 + +## Status + +Accepted + +## Context + +The code agent's `reusable-code.yml` hardcodes `TARGET_BRANCH: main` in the +workflow step env. The post-script (`post-code.sh`) uses this value for +`git merge-base` and `gh pr create --base`. For any repository whose default +branch is not `main`, PR creation fails silently. + +The fix agent already handles this correctly by resolving the PR's base +branch dynamically from the existing PR metadata. The code agent has no +equivalent — it creates new PRs from issues, so there is no existing PR to +query. + +Beyond the immediate bug, the current design has two deeper problems: + +1. **The agent understands the target better than the workflow.** When an + issue says "set up builds on the 3.18 branch," the agent reads and + understands that context. Hardcoding the branch in an env var forces the + workflow to guess what the agent already knows. + +2. **Business logic in GitHub Actions workflows is not portable.** Fullsend + aims to run agents in environments beyond GitHub Actions (e.g., Kubernetes + pods). Branch-targeting logic embedded in workflow YAML becomes technical + debt that must be disentangled during that migration. + +The post-script is the right place for branch policy enforcement. It already +serves as the security boundary — it runs on the runner (not in the sandbox), +holds the write token, and performs secret scanning and pre-commit validation +before any push. Adding branch validation here follows the established +security model. + +## Options + +### Option A: Workflow-level auto-detection + +Add a step in `reusable-code.yml` that queries the GitHub API for the repo's +default branch and replaces the hardcoded `main`. Optionally parse +`--branch ` from the `/fs-code` comment. + +**Pros:** Smallest change (one file). Fixes the immediate bug. +**Cons:** Business logic in the workflow. Not portable. The agent still +cannot choose a branch different from the default without comment-level +syntax. + +### Option B: Agent-driven targeting with post-script policy gate (recommended) + +The agent writes its chosen `target_branch` to a structured output file +(`code-result.json`). The post-script reads the agent's choice, validates it +against a `CODE_ALLOWED_TARGET_BRANCHES` env var, and falls back to the +auto-detected repo default branch when no output is provided. + +**Pros:** Agent decides intent based on issue context. Post-script enforces +policy. No business logic in the workflow. Follows the existing structured +output pattern used by fix, triage, and review agents. Backward compatible. +**Cons:** Requires a new output schema for the code agent. + +### Option C: Git commit trailer convention + +The agent adds a `Target-Branch: ` trailer to its commit message. The +post-script parses it from `git log`. + +**Pros:** No new schema. +**Cons:** Fragile parsing. No validation tooling. Agent may forget the +trailer. Does not follow established structured output patterns. + +## Decision + +Adopt Option B: agent-driven targeting with post-script policy enforcement. + +### Code agent output schema + +Add `schemas/code-result.schema.json`: + +```json +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "Code Agent Result", + "type": "object", + "required": ["target_branch"], + "additionalProperties": false, + "properties": { + "target_branch": { + "type": "string", + "description": "Branch the PR should target.", + "pattern": "^[a-zA-Z0-9._/-]+$" + } + } +} +``` + +The agent determines the target branch from the issue context and writes +`code-result.json` to `$FULLSEND_OUTPUT_DIR`. + +### Post-script policy gate + +Replace the `TARGET_BRANCH="${TARGET_BRANCH:-main}"` line in `post-code.sh` +with branch resolution logic: + +1. Read `target_branch` from `code-result.json` (agent's choice). +2. Auto-detect the repo's default branch via `gh api`. +3. If the agent specified a branch, validate it against + `CODE_ALLOWED_TARGET_BRANCHES` (comma-separated list, or `*` for any). + When unset, only the auto-detected default branch is allowed. +4. If the agent did not write `code-result.json`, use the auto-detected default. +5. Fall back to `main` if the API call fails. + +### Harness changes + +Update `harness/code.yaml`: + +- Add `FULLSEND_OUTPUT_SCHEMA` and `FULLSEND_OUTPUT_FILE` to `runner_env` + (wiring up structured output for the code agent). +- Add `CODE_ALLOWED_TARGET_BRANCHES` to `runner_env` (policy enforcement). +- Remove `TARGET_BRANCH` from `runner_env` (replaced by post-script logic). + +### Workflow changes + +Remove `TARGET_BRANCH: main` from the "Run code agent" step env in +`reusable-code.yml`. No replacement env var is needed in the workflow — the +post-script handles all branch logic. Repos that want to restrict allowed +branches configure `CODE_ALLOWED_TARGET_BRANCHES` via their harness override +(`runner_env`), not in the workflow YAML. + +## Consequences + +**What becomes easier:** + +- Repos with non-`main` default branches work out of the box. No + configuration required — the post-script auto-detects the default branch. +- Agents can target the correct branch based on issue context (e.g., "set up + builds on redhat-3.18" results in a PR targeting `redhat-3.18`). +- Repos can restrict which branches agents may target by setting + `CODE_ALLOWED_TARGET_BRANCHES` in their harness override. +- Branch-targeting logic lives in the portable post-script, not in + GitHub-Actions-specific YAML. + +**What becomes harder or changes:** + +- The code agent now has a structured output contract. Agent definitions must + be updated to instruct the agent to write `code-result.json`. +- Repos that override `harness/code.yaml` via `.fullsend/customized/` must + update their override to include the new `runner_env` fields. Specifically, + replace `TARGET_BRANCH: "${TARGET_BRANCH}"` with the new keys: + ```yaml + runner_env: + CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" + FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/code-result.schema.json + FULLSEND_OUTPUT_FILE: code-result.json + ``` + Customized harnesses that still reference `${TARGET_BRANCH}` will fail + harness validation because the workflow no longer provides the variable. +- The `TARGET_BRANCH` env var is removed. Any tooling that reads it directly + (outside the post-script) must be updated. + +**Backward compatibility:** + +- If the agent does not write `code-result.json` (e.g., older agent + definitions), the post-script falls back to the auto-detected default + branch. This is strictly better than the current `main` hardcode. +- If `CODE_ALLOWED_TARGET_BRANCHES` is not set, only the auto-detected + default branch is allowed. Safe by default. diff --git a/docs/architecture.md b/docs/architecture.md index 5549cf9799..bc1148c1b3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -96,6 +96,11 @@ The harness draws its configuration from the adopting organization's **`.fullsen mechanisms (`.env` files, `runner_env`). Each agent documents its config vars in `docs/agents/.md` ([ADR 0049](ADRs/0049-agent-configuration-env-var-convention.md)). +- Agent-driven branch targeting: the code agent writes its chosen target + branch to structured output. The post-script validates the choice against + an allowlist and falls back to the repo's auto-detected default branch. + Branch-targeting logic lives in the portable post-script, not in workflow + YAML ([ADR 0053](ADRs/0053-agent-driven-branch-targeting.md)). **Open questions:** diff --git a/docs/guides/user/running-agents-locally.md b/docs/guides/user/running-agents-locally.md index e8f1ec5575..98c384187e 100644 --- a/docs/guides/user/running-agents-locally.md +++ b/docs/guides/user/running-agents-locally.md @@ -172,7 +172,7 @@ PUSH_TOKEN_SOURCE=github-app GITHUB_ISSUE_URL=https://github.com/{org}/{repo}/issues/{issue_num} REPO_FULL_NAME={org}/{repo} ISSUE_NUMBER={issue_num} -TARGET_BRANCH=main +CODE_ALLOWED_TARGET_BRANCHES=main REPO_DIR=/tmp/repo-dir GITHUB_WORKSPACE=/tmp/ ``` diff --git a/internal/harness/scaffold_integration_test.go b/internal/harness/scaffold_integration_test.go index 519355f036..795b08df16 100644 --- a/internal/harness/scaffold_integration_test.go +++ b/internal/harness/scaffold_integration_test.go @@ -301,7 +301,7 @@ func TestResolveForge_ScaffoldRunnerEnvMerge(t *testing.T) { }, { file: "code.yaml", - topLevelKeys: []string{"TARGET_BRANCH"}, + topLevelKeys: []string{"CODE_ALLOWED_TARGET_BRANCHES", "FULLSEND_OUTPUT_SCHEMA", "FULLSEND_OUTPUT_FILE"}, forgeGithubKeys: []string{"PUSH_TOKEN", "PUSH_TOKEN_SOURCE", "REPO_FULL_NAME", "ISSUE_NUMBER", "REPO_DIR"}, }, { diff --git a/internal/scaffold/fullsend-repo/harness/code.yaml b/internal/scaffold/fullsend-repo/harness/code.yaml index ac6bd9f16f..1c40b820be 100644 --- a/internal/scaffold/fullsend-repo/harness/code.yaml +++ b/internal/scaffold/fullsend-repo/harness/code.yaml @@ -42,7 +42,9 @@ plugins: # Environment variables available to pre/post scripts on the runner. # These are expanded from the runner environment and NEVER enter the sandbox. runner_env: - TARGET_BRANCH: "${TARGET_BRANCH}" + CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" + FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/code-result.schema.json + FULLSEND_OUTPUT_FILE: code-result.json timeout_minutes: 35 diff --git a/internal/scaffold/fullsend-repo/schemas/code-result.schema.json b/internal/scaffold/fullsend-repo/schemas/code-result.schema.json new file mode 100644 index 0000000000..f4b5fbf268 --- /dev/null +++ b/internal/scaffold/fullsend-repo/schemas/code-result.schema.json @@ -0,0 +1,16 @@ +{ + "$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.", + "type": "object", + "required": ["target_branch"], + "additionalProperties": false, + "properties": { + "target_branch": { + "type": "string", + "description": "Branch the PR should target. Determined by the agent from issue context.", + "pattern": "^[a-zA-Z0-9._/-]+$" + } + } +} diff --git a/internal/scaffold/fullsend-repo/scripts/post-code.sh b/internal/scaffold/fullsend-repo/scripts/post-code.sh index 87c9dcafc8..09de04297f 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-code.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code.sh @@ -24,6 +24,10 @@ # # Optional environment variables: # PUSH_TOKEN_SOURCE — "github-app" (for logging; default: unknown) +# CODE_ALLOWED_TARGET_BRANCHES +# — comma-separated list of branches the agent may target, +# or "*" for any. When unset, only the repo's default +# branch is allowed. (default: auto-detected) # # Exit codes: # 0 — branch pushed and PR created, OR agent determined nothing to do @@ -45,6 +49,7 @@ UV_SHA256="f3b623eb0e6141a7053d571d59a0bdc341e0f238ea8f5f0b4815ddbec9a2a296" # Setup # --------------------------------------------------------------------------- REPO_DIR="${REPO_DIR:-repo}" +RUN_DIR="$(pwd)" if [ "${REPO_DIR}" != "." ]; then if [ ! -d "${REPO_DIR}" ]; then @@ -57,7 +62,45 @@ fi : "${PUSH_TOKEN:?PUSH_TOKEN is required}" : "${REPO_FULL_NAME:?REPO_FULL_NAME is required}" : "${ISSUE_NUMBER:?ISSUE_NUMBER is required}" -TARGET_BRANCH="${TARGET_BRANCH:-main}" +# --------------------------------------------------------------------------- +# Resolve target branch (ADR 0053) +# +# Priority: agent output > allowed-list validation > auto-detect default +# The agent writes its chosen branch to code-result.json. The post-script +# validates it against CODE_ALLOWED_TARGET_BRANCHES (comma-separated list +# or "*" for any). When unset, only the auto-detected default branch is +# allowed. Falls back to "main" if the API call fails. +# --------------------------------------------------------------------------- +AGENT_TARGET="" +RESULT_FILE="" +for dir in "${RUN_DIR}"/iteration-*/output; do + if [ -f "${dir}/code-result.json" ]; then + RESULT_FILE="${dir}/code-result.json" + fi +done +if [ -n "${RESULT_FILE}" ]; then + AGENT_TARGET="$(jq -r '.target_branch // empty' "${RESULT_FILE}" 2>/dev/null || true)" +fi +if [[ -n "${AGENT_TARGET}" && ! "${AGENT_TARGET}" =~ ^[a-zA-Z0-9._/-]+$ ]]; then + echo "Error: invalid branch name from agent output: '${AGENT_TARGET}'" + exit 1 +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" + else + echo "Error: agent requested branch '${AGENT_TARGET}' but allowed branches are: ${ALLOWED}" + exit 1 + fi +else + TARGET_BRANCH="${DEFAULT_BRANCH}" + echo "No agent branch preference — using repo default: ${TARGET_BRANCH}" +fi echo "::add-mask::${PUSH_TOKEN}" diff --git a/internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md b/internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md index dfc18fb5e4..cad0a766b9 100644 --- a/internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md +++ b/internal/scaffold/fullsend-repo/skills/code-implementation/SKILL.md @@ -178,13 +178,30 @@ From these files, determine: missing, but matching the repo's expected format directly is preferred. - **Branch conventions** — naming patterns, target branch -If a `TARGET_BRANCH` environment variable is set, use it. Otherwise, determine -the default branch: +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: ```bash 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: + +```bash +mkdir -p "${FULLSEND_OUTPUT_DIR}" +cat > "${FULLSEND_OUTPUT_DIR}/${FULLSEND_OUTPUT_FILE}" <" +} +RESULT +``` + +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. + ### 4. Check for existing branch Before creating a new branch, check whether a branch already exists for this diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index 0ca8f6c0df..95ab7fbd96 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -650,7 +650,7 @@ func TestHarnessForgeRunnerEnvMerge(t *testing.T) { }, { file: "code.yaml", - topLevelKeys: []string{"TARGET_BRANCH"}, + topLevelKeys: []string{"CODE_ALLOWED_TARGET_BRANCHES", "FULLSEND_OUTPUT_SCHEMA", "FULLSEND_OUTPUT_FILE"}, forgeGithubKeys: []string{"PUSH_TOKEN", "PUSH_TOKEN_SOURCE", "REPO_FULL_NAME", "ISSUE_NUMBER", "REPO_DIR"}, }, {