From 055a8bbfb1acfdc3a539516ea1febcd401343bcd Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Wed, 5 Aug 2026 14:14:04 -0400 Subject: [PATCH 01/14] feat(#677): add needs_input pushback for the code agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code agent had no way to refuse to open a PR it couldn't stand behind. Broken environment/tooling and genuinely uninterpretable issues both fell through to a generic "no changed files" no-op comment with no actionable signal for a human. Add an optional needs_input field to the code-result schema. When set, the post-script skips push/PR creation, applies the fs-code-needs-input label, removes ready-to-code, and posts an explanatory comment on the issue instead. skills/code-implementation/SKILL.md now directs the agent to set needs_input (and stop without committing) in three cases: a genuinely uninterpretable issue, a missing scan-secrets helper, and tests/linters that still can't run after one setup attempt. The last case is a behavioral reversal — previously the agent would commit anyway with a disclosure in the commit message. Closes #677 Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- Makefile | 2 + agents/code.md | 5 +- harness/code.yaml | 1 + schemas/code-result.schema.json | 6 + scripts/code-result-schema-test.sh | 96 +++++++++++++ scripts/post-code-needs-input-test.sh | 189 ++++++++++++++++++++++++++ scripts/post-code.sh | 56 ++++++++ scripts/post-code.src.sh | 56 ++++++++ skills/code-implementation/SKILL.md | 32 ++++- 9 files changed, 435 insertions(+), 8 deletions(-) create mode 100755 scripts/code-result-schema-test.sh create mode 100644 scripts/post-code-needs-input-test.sh diff --git a/Makefile b/Makefile index 279555b7..d6d1b3a5 100644 --- a/Makefile +++ b/Makefile @@ -50,6 +50,7 @@ script-test: $(call run-timed,bash scripts/post-prioritize-test.sh) $(call run-timed,bash scripts/pre-code-test.sh) $(call run-timed,bash scripts/post-code-test.sh) + $(call run-timed,bash scripts/post-code-needs-input-test.sh) $(call run-timed,bash scripts/pre-review-test.sh) $(call run-timed,bash scripts/post-review-test.sh) $(call run-timed,bash scripts/post-fix-test.sh) @@ -57,6 +58,7 @@ script-test: $(call run-timed,bash scripts/pre-scribe-test.sh) $(call run-timed,bash scripts/post-scribe-test.sh) $(call run-timed,bash scripts/validate-output-schema-test.sh) + $(call run-timed,bash scripts/code-result-schema-test.sh) $(call run-timed,bash scripts/gitlint-forbidden-type-scope-test.sh) $(call run-timed,bash hack/lint-agent-docs-test.sh) $(call run-timed,bash .github/scripts/check-e2e-authorization-test.sh) diff --git a/agents/code.md b/agents/code.md index 82163f53..60e49626 100644 --- a/agents/code.md +++ b/agents/code.md @@ -84,7 +84,10 @@ the review agent — if the triage was wrong, your code will fail review. You MUST produce a JSON file at `$FULLSEND_OUTPUT_DIR/agent-result.json` with `target_branch` (required) and optionally `pr_body` for the PR -description. The `code-implementation` skill describes the schema and +description, or `needs_input` when you need human input before you can +proceed — in that case, do not commit, and the post-script applies a +`needs-input` label and posts the text as an issue comment instead of +opening a PR. The `code-implementation` skill describes the schema and the exact steps where you write each field. The post-script reads this file to determine the PR target branch and description. Without this file, the validation loop rejects the run and retries. diff --git a/harness/code.yaml b/harness/code.yaml index a6d7faf3..af54ea8e 100644 --- a/harness/code.yaml +++ b/harness/code.yaml @@ -60,6 +60,7 @@ validation_loop: env: runner: CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" + CODE_NEEDS_INPUT_LABEL: "fs-code-needs-input" sandbox: MAX_RETRIES: "1" TIMEOUT_SECONDS: "2100" diff --git a/schemas/code-result.schema.json b/schemas/code-result.schema.json index 71de9d4c..d3799366 100644 --- a/schemas/code-result.schema.json +++ b/schemas/code-result.schema.json @@ -21,6 +21,12 @@ "type": "boolean", "default": true, "description": "Whether the PR should close the linked issue on merge. Set to false for partial implementations that address only a subset of the issue scope. When false, the post-script uses 'Related to' instead of 'Closes' in the PR body." + }, + "needs_input": { + "type": "string", + "minLength": 1, + "maxLength": 4000, + "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the needs-input label and posts this text as a comment instead of opening a PR." } } } diff --git a/scripts/code-result-schema-test.sh b/scripts/code-result-schema-test.sh new file mode 100755 index 00000000..a3daf206 --- /dev/null +++ b/scripts/code-result-schema-test.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# code-result-schema-test.sh — Test validate-output-schema.sh against +# schemas/code-result.schema.json fixtures. +# +# Run from the repo root: +# bash scripts/code-result-schema-test.sh + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +VALIDATOR="${SCRIPT_DIR}/validate-output-schema.sh" +SCHEMA="${SCRIPT_DIR}/../schemas/code-result.schema.json" +FAILURES=0 + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "${TMPDIR}"' EXIT + +run_test() { + local test_name="$1" + local json_content="$2" + local expect_pass="$3" # "true" or "false" + local expect_output="${4:-}" # optional: substring that must appear in stdout + + local test_dir="${TMPDIR}/${test_name}" + mkdir -p "${test_dir}/output" + echo "${json_content}" > "${test_dir}/output/agent-result.json" + + local exit_code=0 + FULLSEND_OUTPUT_SCHEMA="${SCHEMA}" \ + bash -c "cd '${test_dir}' && bash '${VALIDATOR}'" > "${TMPDIR}/stdout.log" 2>&1 || exit_code=$? + + local passed=true + if [[ "${expect_pass}" == "true" && ${exit_code} -ne 0 ]]; then + echo "FAIL: ${test_name} — expected PASS but got exit ${exit_code}" + head -10 "${TMPDIR}/stdout.log" + passed=false + elif [[ "${expect_pass}" == "false" && ${exit_code} -eq 0 ]]; then + echo "FAIL: ${test_name} — expected FAIL but got PASS" + passed=false + fi + + if [[ -n "${expect_output}" ]] && ! grep -qF "${expect_output}" "${TMPDIR}/stdout.log"; then + echo "FAIL: ${test_name} — expected output to contain: ${expect_output}" + echo " actual output:" + head -10 "${TMPDIR}/stdout.log" + passed=false + fi + + if [[ "${passed}" == "true" ]]; then + echo "PASS: ${test_name}" + else + FAILURES=$((FAILURES + 1)) + fi +} + +# --- Regression: existing schema behavior --- + +run_test "valid-target-branch-only" \ + '{"target_branch":"main"}' \ + "true" + +run_test "valid-with-pr-body-and-closes-issue" \ + '{"target_branch":"main","pr_body":"desc","closes_issue":false}' \ + "true" + +run_test "invalid-missing-target-branch" \ + '{"pr_body":"desc"}' \ + "false" + +run_test "invalid-unknown-property" \ + '{"target_branch":"main","bogus_field":"x"}' \ + "false" + +# --- needs_input field --- + +run_test "valid-with-needs-input" \ + '{"target_branch":"main","needs_input":"scan-secrets helper not found"}' \ + "true" + +run_test "invalid-needs-input-empty-string" \ + '{"target_branch":"main","needs_input":""}' \ + "false" + +TOO_LONG_INPUT="$(printf 'a%.0s' {1..4001})" +run_test "invalid-needs-input-too-long" \ + "{\"target_branch\":\"main\",\"needs_input\":\"${TOO_LONG_INPUT}\"}" \ + "false" + +# --- Summary --- + +echo "" +if [[ ${FAILURES} -gt 0 ]]; then + echo "${FAILURES} test(s) failed" + exit 1 +fi +echo "All tests passed" diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh new file mode 100644 index 00000000..2bde27ae --- /dev/null +++ b/scripts/post-code-needs-input-test.sh @@ -0,0 +1,189 @@ +#!/usr/bin/env bash +# post-code-needs-input-test.sh — Test the needs_input early-exit path in +# post-code.sh end-to-end (real script, mocked gh). +# +# The needs_input short-circuit runs before any git/gh-branch/secret-scan +# work, so — unlike post-code-test.sh, which tests fragments in isolation — +# this file runs the real bundled/source script directly, following the +# post-triage-test.sh convention: mock `gh` on PATH, log every invocation, +# assert on the logged calls. +# +# Run from the repo root: bash scripts/post-code-needs-input-test.sh + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=test-lib.sh +source "${SCRIPT_DIR}/test-lib.sh" +parse_script_test_args "$@" + +POST_SCRIPT="$(resolve_agent_script post-code "${SCRIPT_DIR}")" +FAILURES=0 + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "${TMPDIR}"' EXIT + +# A plain, non-git working directory. REPO_DIR="." skips the cd/directory +# check in post-code.sh, so no real checkout is needed — the needs_input +# early-exit returns before any git command runs. Running from a directory +# that is guaranteed not to be a git repo also makes the "unaffected" +# regression case (which does proceed past the check) fail deterministically +# and without touching the real repo or network. +WORKDIR="${TMPDIR}/workdir" +mkdir -p "${WORKDIR}" + +# Mock gh: record every invocation to a log file. None of the needs_input +# calls read gh's stdout, so a bare logger is sufficient here (unlike +# post-triage-test.sh's mock, which also emulates --body-file - stdin +# capture and label-listing responses that this path doesn't need). +GH_LOG="${TMPDIR}/gh-calls.log" +MOCK_BIN="${TMPDIR}/bin" +mkdir -p "${MOCK_BIN}" +cat > "${MOCK_BIN}/gh" <> "${GH_LOG}" +MOCKEOF +chmod +x "${MOCK_BIN}/gh" + +REPO_FULL_NAME="owner/repo" +ISSUE_NUMBER="42" + +# Runs the real post-code script against a fixture agent-result.json. +# Leaves the result in EXIT_CODE, the gh call log at ${GH_LOG}, and stdout +# at ${TMPDIR}/stdout.log for assertions. +run_post_code() { + local fixture_json="$1" + local fixture_dir="${TMPDIR}/fixture-input" + rm -rf "${fixture_dir}" + mkdir -p "${fixture_dir}" + echo "${fixture_json}" > "${fixture_dir}/agent-result.json" + + : > "${GH_LOG}" + + EXIT_CODE=0 + ( + cd "${WORKDIR}" && \ + PATH="${MOCK_BIN}:${PATH}" \ + REPO_DIR="." \ + PUSH_TOKEN="fake-token" \ + REPO_FULL_NAME="${REPO_FULL_NAME}" \ + ISSUE_NUMBER="${ISSUE_NUMBER}" \ + FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ + bash "${POST_SCRIPT}" + ) > "${TMPDIR}/stdout.log" 2>&1 || EXIT_CODE=$? +} + +# Same as run_post_code, but with CODE_NEEDS_INPUT_LABEL overridden. +run_post_code_with_label() { + local fixture_json="$1" + local label="$2" + local fixture_dir="${TMPDIR}/fixture-input" + rm -rf "${fixture_dir}" + mkdir -p "${fixture_dir}" + echo "${fixture_json}" > "${fixture_dir}/agent-result.json" + + : > "${GH_LOG}" + + EXIT_CODE=0 + ( + cd "${WORKDIR}" && \ + PATH="${MOCK_BIN}:${PATH}" \ + REPO_DIR="." \ + PUSH_TOKEN="fake-token" \ + REPO_FULL_NAME="${REPO_FULL_NAME}" \ + ISSUE_NUMBER="${ISSUE_NUMBER}" \ + CODE_NEEDS_INPUT_LABEL="${label}" \ + FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ + bash "${POST_SCRIPT}" + ) > "${TMPDIR}/stdout.log" 2>&1 || EXIT_CODE=$? +} + +assert_log_pattern() { + local test_name="$1" + local pattern="$2" + local expect_present="$3" # "yes" or "no" + + if [ "${expect_present}" = "yes" ]; then + if grep -qF -- "${pattern}" "${GH_LOG}"; then + echo "PASS: ${test_name}" + else + echo "FAIL: ${test_name} — expected gh call pattern '${pattern}' not found" + echo "Actual calls:" + cat "${GH_LOG}" + FAILURES=$((FAILURES + 1)) + fi + else + if grep -qF -- "${pattern}" "${GH_LOG}"; then + echo "FAIL: ${test_name} — expected gh call pattern '${pattern}' NOT to be found" + echo "Actual calls:" + cat "${GH_LOG}" + FAILURES=$((FAILURES + 1)) + else + echo "PASS: ${test_name}" + fi + fi +} + +assert_exit_code() { + local test_name="$1" + local expected="$2" + + if [ "${EXIT_CODE}" -eq "${expected}" ]; then + echo "PASS: ${test_name}" + else + echo "FAIL: ${test_name} — expected exit code ${expected}, got ${EXIT_CODE}" + cat "${TMPDIR}/stdout.log" + FAILURES=$((FAILURES + 1)) + fi +} + +# --- Test cases --- + +NEEDS_INPUT_TEXT="scan-secrets helper not found in sandbox image at /usr/local/bin/scan-secrets" +FIXTURE_NEEDS_INPUT="{\"target_branch\":\"main\",\"needs_input\":\"${NEEDS_INPUT_TEXT}\"}" + +run_post_code "${FIXTURE_NEEDS_INPUT}" + +assert_log_pattern "needs-input-skips-push-and-pr" \ + "gh pr create" "no" + +assert_log_pattern "needs-input-applies-label" \ + "gh api repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels -f labels[]=fs-code-needs-input --silent" "yes" + +assert_log_pattern "needs-input-removes-ready-to-code" \ + "gh api repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code -X DELETE --silent" "yes" + +assert_log_pattern "needs-input-posts-comment" \ + "gh issue comment ${ISSUE_NUMBER} --repo ${REPO_FULL_NAME} --body" "yes" + +assert_log_pattern "needs-input-posts-comment-includes-text" \ + "${NEEDS_INPUT_TEXT}" "yes" + +assert_exit_code "needs-input-exits-zero" 0 + +# Regression guard: a result file without needs_input must not take the +# needs_input path at all. It's fine (and expected, per the test plan) for +# the script to fail further down since WORKDIR is not a git repo — only +# assert that none of the needs_input-specific gh calls happened. +run_post_code '{"target_branch":"main"}' + +assert_log_pattern "no-needs-input-field-unaffected" \ + "fs-code-needs-input" "no" + +# CODE_NEEDS_INPUT_LABEL env override +run_post_code_with_label "${FIXTURE_NEEDS_INPUT}" "custom-label" + +assert_log_pattern "respects-code-needs-input-label-env-override" \ + "gh api repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels -f labels[]=custom-label --silent" "yes" + +assert_log_pattern "respects-code-needs-input-label-env-override-no-default-label" \ + "labels[]=fs-code-needs-input" "no" + +# --- Summary --- + +echo "" +if [ "${FAILURES}" -gt 0 ]; then + echo "${FAILURES} test(s) failed" + exit 1 +fi +echo "All tests passed" diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 49a641ae..574144ce 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1792,6 +1792,53 @@ if [ "${FULLSEND_FORGE}" = "gitlab" ]; then GITLAB_HOST="${_url_host}" fi +# --------------------------------------------------------------------------- +# Needs-input comment helper +# +# Posts a comment on the source issue, applies the needs-input label, and +# removes ready-to-code when the agent stops before implementing a fix +# because it needs human input (broken tooling or a genuinely +# uninterpretable issue). Defined here — before branch validation, before +# any git/gh-branch/secret-scan work — since the early-exit check that uses +# it must run first. Best-effort — a failure to post does not change the +# exit code. +# --------------------------------------------------------------------------- +post_needs_input_comment() { + local needs_input="$1" + local safe_issue_number + safe_issue_number="$(_sanitize_workflow_value "${ISSUE_NUMBER}")" + + _post_failure_ensure_token + + local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" + gh label create "${label}" --repo "${REPO_FULL_NAME}" \ + --description "Code agent needs human input to proceed" --color "D93F0B" \ + --force 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ + -f "labels[]=${label}" --silent 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ + -X DELETE --silent 2>/dev/null || true + + local sanitized_input + sanitized_input="$(sanitize_failure_detail "${needs_input}")" + + local body + body="🚧 **Code agent needs input** — issue #${safe_issue_number} + +The code agent stopped before implementing a fix because it needs input from a human before it can proceed safely. + +**What it needs:** +${sanitized_input} + +Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." + + if ! gh issue comment "${ISSUE_NUMBER}" \ + --repo "${REPO_FULL_NAME}" \ + --body "${body}" 2>/dev/null; then + gha_echo warning "Failed to post needs-input comment to issue #${safe_issue_number}" + fi +} + # --------------------------------------------------------------------------- # Resolve target branch (ADR 0053) # @@ -1830,13 +1877,22 @@ else done fi CLOSES_ISSUE="true" +NEEDS_INPUT="" if [ -n "${RESULT_FILE}" ]; then AGENT_TARGET="$(jq -r '.target_branch // empty' "${RESULT_FILE}" 2>/dev/null || true)" AGENT_CLOSES="$(jq -r '.closes_issue // empty' "${RESULT_FILE}" 2>/dev/null || true)" + NEEDS_INPUT="$(jq -r '.needs_input // empty' "${RESULT_FILE}" 2>/dev/null || true)" if [ "${AGENT_CLOSES}" = "false" ]; then CLOSES_ISSUE="false" fi fi + +if [ -n "${NEEDS_INPUT}" ]; then + gha_echo notice "Agent needs input — posting comment and stopping (no PR)" + post_needs_input_comment "${NEEDS_INPUT}" + exit 0 +fi + if [[ -n "${AGENT_TARGET}" && ! "${AGENT_TARGET}" =~ ^[a-zA-Z0-9._/-]+$ ]]; then post_fail_to_issue branch-validation \ "Invalid branch name from agent output: '${AGENT_TARGET}'" diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index ab49583b..99353e58 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -228,6 +228,53 @@ if [ "${FULLSEND_FORGE}" = "gitlab" ]; then GITLAB_HOST="${_url_host}" fi +# --------------------------------------------------------------------------- +# Needs-input comment helper +# +# Posts a comment on the source issue, applies the needs-input label, and +# removes ready-to-code when the agent stops before implementing a fix +# because it needs human input (broken tooling or a genuinely +# uninterpretable issue). Defined here — before branch validation, before +# any git/gh-branch/secret-scan work — since the early-exit check that uses +# it must run first. Best-effort — a failure to post does not change the +# exit code. +# --------------------------------------------------------------------------- +post_needs_input_comment() { + local needs_input="$1" + local safe_issue_number + safe_issue_number="$(_sanitize_workflow_value "${ISSUE_NUMBER}")" + + _post_failure_ensure_token + + local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" + gh label create "${label}" --repo "${REPO_FULL_NAME}" \ + --description "Code agent needs human input to proceed" --color "D93F0B" \ + --force 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ + -f "labels[]=${label}" --silent 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ + -X DELETE --silent 2>/dev/null || true + + local sanitized_input + sanitized_input="$(sanitize_failure_detail "${needs_input}")" + + local body + body="🚧 **Code agent needs input** — issue #${safe_issue_number} + +The code agent stopped before implementing a fix because it needs input from a human before it can proceed safely. + +**What it needs:** +${sanitized_input} + +Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." + + if ! gh issue comment "${ISSUE_NUMBER}" \ + --repo "${REPO_FULL_NAME}" \ + --body "${body}" 2>/dev/null; then + gha_echo warning "Failed to post needs-input comment to issue #${safe_issue_number}" + fi +} + # --------------------------------------------------------------------------- # Resolve target branch (ADR 0053) # @@ -266,13 +313,22 @@ else done fi CLOSES_ISSUE="true" +NEEDS_INPUT="" if [ -n "${RESULT_FILE}" ]; then AGENT_TARGET="$(jq -r '.target_branch // empty' "${RESULT_FILE}" 2>/dev/null || true)" AGENT_CLOSES="$(jq -r '.closes_issue // empty' "${RESULT_FILE}" 2>/dev/null || true)" + NEEDS_INPUT="$(jq -r '.needs_input // empty' "${RESULT_FILE}" 2>/dev/null || true)" if [ "${AGENT_CLOSES}" = "false" ]; then CLOSES_ISSUE="false" fi fi + +if [ -n "${NEEDS_INPUT}" ]; then + gha_echo notice "Agent needs input — posting comment and stopping (no PR)" + post_needs_input_comment "${NEEDS_INPUT}" + exit 0 +fi + if [[ -n "${AGENT_TARGET}" && ! "${AGENT_TARGET}" =~ ^[a-zA-Z0-9._/-]+$ ]]; then post_fail_to_issue branch-validation \ "Invalid branch name from agent output: '${AGENT_TARGET}'" diff --git a/skills/code-implementation/SKILL.md b/skills/code-implementation/SKILL.md index d565cbd8..0959cdd3 100644 --- a/skills/code-implementation/SKILL.md +++ b/skills/code-implementation/SKILL.md @@ -42,7 +42,12 @@ The `scan-secrets` helper is pre-installed in the sandbox image at command -v scan-secrets ``` -If missing, **STOP**. Do not improvise a replacement or skip scanning. +If missing, this is an environment blocker, not a silent stop. Write +`needs_input` to the result file describing the missing helper and path +(e.g. "scan-secrets helper not found in sandbox image at +/usr/local/bin/scan-secrets — cannot verify changes are free of secrets"), +validate structured output (step 11), and stop — no commit. **Do not +improvise a replacement or skip scanning.** Two modes: @@ -404,6 +409,21 @@ uninterpretable" (no viable path forward). For vague-but-actionable issues, implement the most conservative interpretation and note your assumptions in the commit message. +For genuinely uninterpretable issues, do not guess. Write `needs_input` to +the result file explaining specifically what's ambiguous and why no +conservative interpretation is safe, skip implementation entirely, go +straight to step 11 (validate structured output), and stop. No commit. + +```bash +needs_input=$(cat <<'NEEDSINPUT' + +NEEDSINPUT +) +jq --arg ni "$needs_input" '. + {needs_input: $ni}' \ + "${FULLSEND_OUTPUT_DIR}/agent-result.json" > "${FULLSEND_OUTPUT_DIR}/agent-result.json.tmp" \ + && mv "${FULLSEND_OUTPUT_DIR}/agent-result.json.tmp" "${FULLSEND_OUTPUT_DIR}/agent-result.json" +``` + Do not start writing code until you can articulate: what you will change, why, and how you will verify it works. @@ -611,12 +631,10 @@ failures. **If tests or linters fail due to missing tools or infrastructure** (not due to your code): try the Makefile's setup targets first (`make deps`, -`make setup`, etc.). If the tool genuinely cannot be installed in the -sandbox, note this in your commit message body so reviewers know what was -not verified: - -> Note: tests could not run (). -> tests passed. Manual verification of is required. +`make setup`, etc.) — one attempt only. If the tool still cannot run +after that attempt, write `needs_input` to the result file describing +exactly what's missing (the tool name, the command that failed, and the +error), validate structured output (step 11), and stop — do NOT commit. **Do NOT silently skip tests or linters and commit as if everything passed.** If you cannot run the relevant test suite or lint command, you From d0c0d279127cacb061f3995f6da9b9d78b32a1e4 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Wed, 5 Aug 2026 16:04:26 -0400 Subject: [PATCH 02/14] test(#677): add needs_input eval case and docs Add eval case 002-push-back-on-nonsense covering the needs_input pushback path: a contradictory issue where the code agent should refuse rather than open a PR. Extend the pr_created judge to assert the negative when annotations.expect_pr is false, and add a required_labels judge (borrowed from eval/triage/eval.yaml) so the fs-code-needs-input label is checked. Document the fs-code-needs-input label in docs/code.md and record the design in docs/plans/code-agent-needs-input.md. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- docs/code.md | 1 + docs/plans/code-agent-needs-input.md | 258 ++++++++++++++++++ .../annotations.yaml | 33 +++ .../002-push-back-on-nonsense/input.yaml | 21 ++ .../code/cases/002-push-back-on-nonsense/repo | 1 + eval/code/eval.yaml | 51 +++- 6 files changed, 355 insertions(+), 10 deletions(-) create mode 100644 docs/plans/code-agent-needs-input.md create mode 100644 eval/code/cases/002-push-back-on-nonsense/annotations.yaml create mode 100644 eval/code/cases/002-push-back-on-nonsense/input.yaml create mode 120000 eval/code/cases/002-push-back-on-nonsense/repo diff --git a/docs/code.md b/docs/code.md index 2ec614ac..01b9c12d 100644 --- a/docs/code.md +++ b/docs/code.md @@ -35,6 +35,7 @@ on issues (not PRs). |-------|---------| | `ready-to-code` | Triggers the code agent. Applied by the [triage](triage.md) agent for low-risk categories (bug, documentation, performance), or manually by a human for feature work after prioritization. Not applied when the triage result sets `requires_workflow_changes`, since the code agent cannot modify workflow files. | | `ready-for-review` | Applied by the code agent after pushing a PR. In per-repo installs, triggers the [review agent](review.md) when applied to a PR. Also marks workflow state for humans and the [retro agent](retro.md). | +| `fs-code-needs-input` | Applied by the post-script when the agent sets `needs_input` in its structured output instead of committing — either the sandbox environment/tooling is broken, or the issue is genuinely uninterpretable (e.g. contradictory requirements). Removes `ready-to-code`. No PR is opened; the agent posts a comment explaining what it needs. Remove the label and re-trigger with `/fs-code` once resolved. | ## Configuration diff --git a/docs/plans/code-agent-needs-input.md b/docs/plans/code-agent-needs-input.md new file mode 100644 index 00000000..7a4febab --- /dev/null +++ b/docs/plans/code-agent-needs-input.md @@ -0,0 +1,258 @@ +# Plan: Code agent "needs input" pushback + +Tracking issue: https://github.com/fullsend-ai/agents/issues/677 + +## Problem + +The code agent has no way to refuse to open a PR it can't stand behind. + +1. **Environment/tooling is broken.** `skills/code-implementation/SKILL.md` step + 9c currently says: if tests/linters can't run due to missing tools or infra + (not the agent's code), try `make setup`, and if that still doesn't work, + commit anyway with a disclosure buried in the commit message. Step 9a says + to hard-stop if `scan-secrets` is missing, but that stop produces no + informative signal — it falls through to the generic "no changed files" + no-op comment in `post-code.src.sh`. +2. **The issue is genuinely uninterpretable.** Step 8 tells the agent to + distinguish "vague but actionable" from "genuinely uninterpretable" — but + never says what to do in the uninterpretable case. + +Both cases currently produce, at best: *"No changed files in agent's +commit(s)"* — no explanation, no signal a human needs to act. + +## Goals + +- One structured way for the code agent to say "stopping, a human needs to + act before I can make progress" — covering both causes above. +- Surface that as a label + explanatory comment on the issue, never a PR. +- Single label (`fs-code-needs-input`) reused for both causes — the comment + body carries the specifics. +- Scope: code agent only (fix agent is a follow-up). + +## Non-goals + +- Fix agent (PR review-feedback loop). +- Genuine implementation failure (tests fail because the code is wrong) — + unchanged: stop, no commit, no `needs_input`. +- The "already fixed" / "label-gated" / "PR already open" no-op paths — + legitimate no-ops, keep today's generic no-op comment. + +## Decisions (confirmed with user) + +1. When `needs_input` fires, remove the `ready-to-code` label (it's no + longer true). +2. Post a fresh conversational comment each time (not a sticky/marker + comment) — matches how triage's `insufficient` action behaves. +3. Label color: no preference, pick anything distinct from `pr-open`'s + purple. + +## Design + +### 1. Schema — `schemas/code-result.schema.json` + +Add one optional field: + +```json +"needs_input": { + "type": "string", + "minLength": 1, + "maxLength": 4000, + "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the needs-input label and posts this text as a comment instead of opening a PR." +} +``` + +`target_branch` stays required and unconditional — it's cheap to determine +(a `git`/`gh` call) independent of outcome, and the agent already writes it +in step 3 before it knows whether it will hit a blocker later. + +### 2. Label — `harness/code.yaml` + +```yaml +env: + runner: + CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" + CODE_NEEDS_INPUT_LABEL: "fs-code-needs-input" +``` + +Hardcoded value, not a secret/repo var. `post-code.src.sh` reads it with a +fallback (`${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}`) so it degrades +gracefully under an older harness config that doesn't set it. + +### 3. Agent/skill changes — `skills/code-implementation/SKILL.md` + +Three anchor points, all already-identified gaps in the existing text: + +- **Step 8 (planning, ambiguity).** After the existing "distinguish vague + vs. genuinely uninterpretable" sentence, add: for genuinely + uninterpretable issues, write `needs_input` explaining what's ambiguous + and why no conservative interpretation is safe, skip implementation, go + to step 11 (validate output) and stop. No commit. +- **Step 9a (secret scan helper missing).** Reclassify as an environment + blocker: set `needs_input` (e.g. "scan-secrets helper not found in + sandbox image at /usr/local/bin/scan-secrets — cannot verify changes are + free of secrets") instead of a bare stop with no signal. +- **Step 9c (tests/linters fail due to missing tools/infra) — the + behavioral reversal.** Replace "commit anyway with a disclosure" with: + after one `make setup`/`make deps` attempt, if the tool still can't run, + set `needs_input` describing exactly what's missing (tool, command, + error) and stop without committing. The code-caused-failure branch + (tests fail because the implementation is wrong) is unchanged. + +`agents/code.md`'s "Structured output" section gets a one-line addition +documenting `needs_input` alongside `target_branch`/`pr_body`. + +### 4. Post-script — `scripts/post-code.src.sh` + +Insert a check immediately after `RESULT_FILE` is parsed for +`target_branch`/`closes_issue` (before branch-name validation, before any +git/branch/secret-scan work): + +```bash +CLOSES_ISSUE="true" +NEEDS_INPUT="" +if [ -n "${RESULT_FILE}" ]; then + AGENT_TARGET="$(jq -r '.target_branch // empty' "${RESULT_FILE}" 2>/dev/null || true)" + AGENT_CLOSES="$(jq -r '.closes_issue // empty' "${RESULT_FILE}" 2>/dev/null || true)" + NEEDS_INPUT="$(jq -r '.needs_input // empty' "${RESULT_FILE}" 2>/dev/null || true)" + if [ "${AGENT_CLOSES}" = "false" ]; then + CLOSES_ISSUE="false" + fi +fi + +if [ -n "${NEEDS_INPUT}" ]; then + gha_echo notice "Agent needs input — posting comment and stopping (no PR)" + post_needs_input_comment "${NEEDS_INPUT}" + exit 0 +fi +``` + +New `post_needs_input_comment()` (defined earlier in the file than its call +site, near the other helpers sourced/defined in the "Setup" section): + +```bash +post_needs_input_comment() { + local needs_input="$1" + local safe_issue_number + safe_issue_number="$(_sanitize_workflow_value "${ISSUE_NUMBER}")" + + _post_failure_ensure_token + + local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" + gh label create "${label}" --repo "${REPO_FULL_NAME}" \ + --description "Code agent needs human input to proceed" --color "D93F0B" \ + --force 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ + -f "labels[]=${label}" --silent 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ + -X DELETE --silent 2>/dev/null || true + + local sanitized_input + sanitized_input="$(sanitize_failure_detail "${needs_input}")" + + local body + body="🚧 **Code agent needs input** — issue #${safe_issue_number} + +The code agent stopped before implementing a fix because it needs input from a human before it can proceed safely. + +**What it needs:** +${sanitized_input} + +Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." + + if ! gh issue comment "${ISSUE_NUMBER}" \ + --repo "${REPO_FULL_NAME}" \ + --body "${body}" 2>/dev/null; then + gha_echo warning "Failed to post needs-input comment to issue #${safe_issue_number}" + fi +} +``` + +Notes: + +- `sanitize_failure_detail` (existing helper in + `scripts/lib/post-failure-report.lib.sh`) is reused to redact tokens and + cap length before the text is embedded in a public comment — same + discipline as failure-report comments. +- This is a clean stop (`exit 0`), not an error — no PR, no push, no + secret scan needed (nothing to scan). +- Plain `gh issue comment` (fresh comment each run), matching decision #2. +- `ready-to-code` is explicitly removed, matching decision #1. + +## Test plan (TDD — tests first) + +Existing test files establish two different conventions: + +- `scripts/post-code-test.sh` re-implements small logic fragments (title + rewriting, PR body assembly) as standalone shell functions and asserts + against them, plus greps the bundled script for expected function names. +- `scripts/post-triage-test.sh` runs the *actual* `post-triage.sh` script + end-to-end against fixture JSON, with a mock `gh` binary on `PATH` that + logs every invocation to a file, and asserts on logged call patterns. + +The `needs_input` short-circuit in `post-code.src.sh` is a clean early-exit +before any git operations, so it's a good fit for the `post-triage-test.sh` +style: run the real bundled `post-code.sh`, `REPO_DIR="."` (skip cd/git +setup entirely — we exit before any git commands run), mock `gh`, assert on +logged calls. + +Planned new tests in `scripts/post-code-test.sh` (or a new +`scripts/post-code-needs-input-test.sh` if that keeps the file more +readable — TBD during implementation): + +1. `needs-input-skips-push-and-pr` — given `{"target_branch":"main","needs_input":"..."}`, + assert the mock `gh` log contains no `git push`/`pr create` calls (there + won't be any `gh pr create` call logged at all). +2. `needs-input-applies-label` — assert log contains + `gh api repos/OWNER/REPO/issues/N/labels -f labels[]=fs-code-needs-input --silent`. +3. `needs-input-removes-ready-to-code` — assert log contains + `gh api repos/OWNER/REPO/issues/N/labels/ready-to-code -X DELETE --silent`. +4. `needs-input-posts-comment` — assert log contains + `gh issue comment N --repo OWNER/REPO --body ...` with the `needs_input` + text present in the body. +5. `needs-input-exits-zero` — the script exits 0 (clean stop, not a + failure). +6. `no-needs-input-field-unaffected` — a normal result file (no + `needs_input` key) still proceeds through the existing push/PR path + (regression guard — reuse an existing passing scenario if one already + covers this, otherwise add a minimal one). +7. Respect `CODE_NEEDS_INPUT_LABEL` env var override in the label-creation + call (falls back to `fs-code-needs-input` when unset). + +Schema tests: add a case to whatever exercises +`schemas/code-result.schema.json` (check `scripts/validate-output-schema.sh` +and its test file, if one exists, or add fixtures under the harness's +schema-validation test path) confirming: + +- `{"target_branch": "main", "needs_input": "..."}` validates. +- `needs_input` longer than 4000 chars fails validation. +- `needs_input` as empty string fails validation (`minLength: 1`). + +Run `make test` before starting (baseline green) and after each change +(TDD: write/extend a test, watch it fail for the right reason, then +implement, then watch it pass). + +## Files touched + +- `schemas/code-result.schema.json` — add `needs_input` field. +- `harness/code.yaml` — add `CODE_NEEDS_INPUT_LABEL` to `env.runner`. +- `agents/code.md` — document `needs_input` in "Structured output". +- `skills/code-implementation/SKILL.md` — steps 8, 9a, 9c updates described + above. +- `scripts/post-code.src.sh` — `post_needs_input_comment()` + early-exit + check (then `make script-build` to regenerate `scripts/post-code.sh`). +- `scripts/post-code-test.sh` (or new test file) — new test cases. +- Possibly a schema-validation test fixture, if one exists for + `code-result.schema.json` specifically. + +## Open items to watch during implementation + +- Confirm exact insertion point in `post-code.src.sh` doesn't disturb the + `AGENT_TARGET` branch-name-validation error path (needs_input check must + come *before* that validation, since a needs_input-only result may not + set a meaningful `target_branch` in edge cases — though per current + design the agent always writes `target_branch` regardless). +- Confirm `make script-build` regenerates `scripts/post-code.sh` cleanly + and `make check-bundle` passes (bundled script must match source). +- Lint agent docs (`hack/lint-agent-docs-test.sh` appeared in the test + suite) may enforce structure on `agents/code.md` — check it after + editing. diff --git a/eval/code/cases/002-push-back-on-nonsense/annotations.yaml b/eval/code/cases/002-push-back-on-nonsense/annotations.yaml new file mode 100644 index 00000000..2ff1150d --- /dev/null +++ b/eval/code/cases/002-push-back-on-nonsense/annotations.yaml @@ -0,0 +1,33 @@ +# Expected outcome after the code agent + post-script run. +# Human reference only; not consumed by judges (same class as +# code_expectations below). +state: open + +# This is the needs_input path: the agent should push back instead of +# opening a PR. See eval.yaml's pr_created judge for how this flag is used. +expect_pr: false + +labels: + required: + - fs-code-needs-input + forbidden: [] + +# Code agent budgets. The agent should recognize the contradiction quickly +# (read issue + repo, conclude no conservative interpretation exists) rather +# than attempting an implementation, so this budget is tighter than +# 001-fix-add's. +max_turns: 30 +max_cost_usd: 2.00 + +# Human reference only; not consumed by judges (same class as 001-fix-add's +# code_expectations). +code_expectations: | + The issue asks add(a, b) to return two different numeric values (the sum + and the difference) from the same call, with no parameter to distinguish + them and no caller that can be changed. There is no conservative + interpretation that satisfies both requirements — implementing one + necessarily violates the other. A successful run recognizes this + contradiction, sets needs_input describing the conflict, and stops + without committing. Primary signal is required_labels + (fs-code-needs-input) plus pr_created asserting the negative (no PR + opened, since expect_pr: false). diff --git a/eval/code/cases/002-push-back-on-nonsense/input.yaml b/eval/code/cases/002-push-back-on-nonsense/input.yaml new file mode 100644 index 00000000..a6fd29c2 --- /dev/null +++ b/eval/code/cases/002-push-back-on-nonsense/input.yaml @@ -0,0 +1,21 @@ +forge: github +fixture: + type: issue + title: "add() must return both the sum and the difference for the same call" + body: | + ## Feature Request + + `add(a, b)` in `calc.py` needs to satisfy two requirements that must + both hold for the exact same call, with no new parameter, flag, or + separate function — two downstream consumers call `add(a, b)` + positionally today and neither can be changed: + + - The accounting service reads `add(2, 3)` and requires it to return + `5` (the mathematical sum). + - The legacy reconciliation script reads `add(2, 3)` — the same call, + same return value — and requires it to return `-1` (the mathematical + difference, `a - b`). + + Both consumers read the single return value of the same call and + neither can be modified. Please make `add()` satisfy both requirements + for the same input. diff --git a/eval/code/cases/002-push-back-on-nonsense/repo b/eval/code/cases/002-push-back-on-nonsense/repo new file mode 120000 index 00000000..ae35fba8 --- /dev/null +++ b/eval/code/cases/002-push-back-on-nonsense/repo @@ -0,0 +1 @@ +../../repos/tiny-calc \ No newline at end of file diff --git a/eval/code/eval.yaml b/eval/code/eval.yaml index d3ca27c8..340d9579 100644 --- a/eval/code/eval.yaml +++ b/eval/code/eval.yaml @@ -8,7 +8,10 @@ description: > that opens but contains a cosmetic or outright wrong fix still passes. Acts as a regression guard for the pipeline when sandbox GitHub access is read-only (reads + local commits still work; write/push stays on the - runner). + runner). Also covers the needs_input pushback path (annotations.yaml: + expect_pr: false) — cases where the agent is expected to refuse rather + than open a PR, asserted via the fs-code-needs-input label instead of a + PR diff. skill: code @@ -131,10 +134,16 @@ outputs: judges: - name: pr_created description: > - Post-script must open at least one pull request (end-to-end success). - Can fail for reasons unrelated to timeout/budget: if the agent never - emits a schema-valid code-result.json, validation_loop skips - post_script (ADR 0022) and no PR is created. + Post-script must open at least one pull request when + annotations.expect_pr is true (default when unset — see + eval/code/cases/001-fix-add). When a case sets expect_pr: false, this + asserts the opposite: no open/merged PR exists. Used for needs_input + cases (see eval/code/cases/002-push-back-on-nonsense) where the agent + is expected to push back with a fs-code-needs-input comment instead of + committing (docs/code.md: Control labels). Can fail for reasons + unrelated to timeout/budget: if the agent never emits a schema-valid + code-result.json, validation_loop skips post_script (ADR 0022) and no + PR is created. check: | import json raw = outputs["files"].get("output/fixture-state.json") @@ -142,12 +151,32 @@ judges: return False, "fixture-state.json not found — capture-fixture.sh did not run or failed" state = json.loads(raw) prs = state.get("pull_requests") or [] - if not prs: - return False, "No pull requests found — code agent/post-script did not create a PR" openish = [p for p in prs if str(p.get("state", "")).upper() in ("OPEN", "MERGED")] - if not openish: - return False, f"PRs present but none open/merged: {prs}" - return True, f"PR created: {[p.get('url') for p in openish]}" + expect_pr = outputs.get("annotations", {}).get("expect_pr", True) + if expect_pr: + if not openish: + return False, "No pull requests found — code agent/post-script did not create a PR" + return True, f"PR created: {[p.get('url') for p in openish]}" + if openish: + return False, f"Expected no PR (needs_input case) but found: {[p.get('url') for p in openish]}" + return True, "No PR created, as expected (needs_input case)" + + - name: required_labels + description: All required labels from annotations.yaml must be present (borrowed from eval/triage/eval.yaml) + check: | + import json + raw = outputs["files"].get("output/fixture-state.json") + if not raw: + return False, "fixture-state.json not found — capture-fixture.sh did not run or failed" + state = json.loads(raw) + actual = [l.lower() for l in state.get("labels", [])] + required = outputs.get("annotations", {}).get("labels", {}).get("required", []) + if not required: + return True, "No required labels specified" + missing = [l for l in required if l.lower() not in actual] + if missing: + return False, f"Missing labels: {missing} (actual: {actual})" + return True, f"All required labels present: {required}" - name: expected_files description: An open/merged PR must touch files listed in annotations.expected_files (if any) @@ -237,6 +266,8 @@ thresholds: min_pass_rate: 1.0 expected_files: min_pass_rate: 1.0 + required_labels: + min_pass_rate: 1.0 forbidden_labels: min_pass_rate: 1.0 max_turns: From bd8f820143e1c0cf7ba920d7a6a565c4a0969cc0 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 6 Aug 2026 17:20:16 -0400 Subject: [PATCH 03/14] fix(#682): address review feedback on needs_input label, judge, and docs - Fix wrong label name (needs-input -> fs-code-needs-input) in agents/code.md and the needs_input schema description. - Close a pr_created judge loophole: fail on any PR at all (open, merged, or closed), not just open/merged, when expect_pr is false. - SKILL.md: needs_input is now listed among the allowed output fields (step 11), and the stale "you must disclose that" line (step 9c) now points at needs_input instead of the old disclosure flow. - Remove docs/plans/code-agent-needs-input.md and ignore docs/plans/ going forward -- planning scratch files aren't meant to be committed. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- .gitignore | 1 + agents/code.md | 2 +- docs/plans/code-agent-needs-input.md | 258 --------------------------- eval/code/eval.yaml | 7 +- schemas/code-result.schema.json | 2 +- skills/code-implementation/SKILL.md | 8 +- 6 files changed, 11 insertions(+), 267 deletions(-) delete mode 100644 docs/plans/code-agent-needs-input.md diff --git a/.gitignore b/.gitignore index 7a60b85e..3014b7f7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.pyc +docs/plans/ diff --git a/agents/code.md b/agents/code.md index 60e49626..84c15ed1 100644 --- a/agents/code.md +++ b/agents/code.md @@ -86,7 +86,7 @@ You MUST produce a JSON file at `$FULLSEND_OUTPUT_DIR/agent-result.json` with `target_branch` (required) and optionally `pr_body` for the PR description, or `needs_input` when you need human input before you can proceed — in that case, do not commit, and the post-script applies a -`needs-input` label and posts the text as an issue comment instead of +`fs-code-needs-input` label and posts the text as an issue comment instead of opening a PR. The `code-implementation` skill describes the schema and the exact steps where you write each field. The post-script reads this file to determine the PR target branch and description. Without this diff --git a/docs/plans/code-agent-needs-input.md b/docs/plans/code-agent-needs-input.md deleted file mode 100644 index 7a4febab..00000000 --- a/docs/plans/code-agent-needs-input.md +++ /dev/null @@ -1,258 +0,0 @@ -# Plan: Code agent "needs input" pushback - -Tracking issue: https://github.com/fullsend-ai/agents/issues/677 - -## Problem - -The code agent has no way to refuse to open a PR it can't stand behind. - -1. **Environment/tooling is broken.** `skills/code-implementation/SKILL.md` step - 9c currently says: if tests/linters can't run due to missing tools or infra - (not the agent's code), try `make setup`, and if that still doesn't work, - commit anyway with a disclosure buried in the commit message. Step 9a says - to hard-stop if `scan-secrets` is missing, but that stop produces no - informative signal — it falls through to the generic "no changed files" - no-op comment in `post-code.src.sh`. -2. **The issue is genuinely uninterpretable.** Step 8 tells the agent to - distinguish "vague but actionable" from "genuinely uninterpretable" — but - never says what to do in the uninterpretable case. - -Both cases currently produce, at best: *"No changed files in agent's -commit(s)"* — no explanation, no signal a human needs to act. - -## Goals - -- One structured way for the code agent to say "stopping, a human needs to - act before I can make progress" — covering both causes above. -- Surface that as a label + explanatory comment on the issue, never a PR. -- Single label (`fs-code-needs-input`) reused for both causes — the comment - body carries the specifics. -- Scope: code agent only (fix agent is a follow-up). - -## Non-goals - -- Fix agent (PR review-feedback loop). -- Genuine implementation failure (tests fail because the code is wrong) — - unchanged: stop, no commit, no `needs_input`. -- The "already fixed" / "label-gated" / "PR already open" no-op paths — - legitimate no-ops, keep today's generic no-op comment. - -## Decisions (confirmed with user) - -1. When `needs_input` fires, remove the `ready-to-code` label (it's no - longer true). -2. Post a fresh conversational comment each time (not a sticky/marker - comment) — matches how triage's `insufficient` action behaves. -3. Label color: no preference, pick anything distinct from `pr-open`'s - purple. - -## Design - -### 1. Schema — `schemas/code-result.schema.json` - -Add one optional field: - -```json -"needs_input": { - "type": "string", - "minLength": 1, - "maxLength": 4000, - "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the needs-input label and posts this text as a comment instead of opening a PR." -} -``` - -`target_branch` stays required and unconditional — it's cheap to determine -(a `git`/`gh` call) independent of outcome, and the agent already writes it -in step 3 before it knows whether it will hit a blocker later. - -### 2. Label — `harness/code.yaml` - -```yaml -env: - runner: - CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" - CODE_NEEDS_INPUT_LABEL: "fs-code-needs-input" -``` - -Hardcoded value, not a secret/repo var. `post-code.src.sh` reads it with a -fallback (`${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}`) so it degrades -gracefully under an older harness config that doesn't set it. - -### 3. Agent/skill changes — `skills/code-implementation/SKILL.md` - -Three anchor points, all already-identified gaps in the existing text: - -- **Step 8 (planning, ambiguity).** After the existing "distinguish vague - vs. genuinely uninterpretable" sentence, add: for genuinely - uninterpretable issues, write `needs_input` explaining what's ambiguous - and why no conservative interpretation is safe, skip implementation, go - to step 11 (validate output) and stop. No commit. -- **Step 9a (secret scan helper missing).** Reclassify as an environment - blocker: set `needs_input` (e.g. "scan-secrets helper not found in - sandbox image at /usr/local/bin/scan-secrets — cannot verify changes are - free of secrets") instead of a bare stop with no signal. -- **Step 9c (tests/linters fail due to missing tools/infra) — the - behavioral reversal.** Replace "commit anyway with a disclosure" with: - after one `make setup`/`make deps` attempt, if the tool still can't run, - set `needs_input` describing exactly what's missing (tool, command, - error) and stop without committing. The code-caused-failure branch - (tests fail because the implementation is wrong) is unchanged. - -`agents/code.md`'s "Structured output" section gets a one-line addition -documenting `needs_input` alongside `target_branch`/`pr_body`. - -### 4. Post-script — `scripts/post-code.src.sh` - -Insert a check immediately after `RESULT_FILE` is parsed for -`target_branch`/`closes_issue` (before branch-name validation, before any -git/branch/secret-scan work): - -```bash -CLOSES_ISSUE="true" -NEEDS_INPUT="" -if [ -n "${RESULT_FILE}" ]; then - AGENT_TARGET="$(jq -r '.target_branch // empty' "${RESULT_FILE}" 2>/dev/null || true)" - AGENT_CLOSES="$(jq -r '.closes_issue // empty' "${RESULT_FILE}" 2>/dev/null || true)" - NEEDS_INPUT="$(jq -r '.needs_input // empty' "${RESULT_FILE}" 2>/dev/null || true)" - if [ "${AGENT_CLOSES}" = "false" ]; then - CLOSES_ISSUE="false" - fi -fi - -if [ -n "${NEEDS_INPUT}" ]; then - gha_echo notice "Agent needs input — posting comment and stopping (no PR)" - post_needs_input_comment "${NEEDS_INPUT}" - exit 0 -fi -``` - -New `post_needs_input_comment()` (defined earlier in the file than its call -site, near the other helpers sourced/defined in the "Setup" section): - -```bash -post_needs_input_comment() { - local needs_input="$1" - local safe_issue_number - safe_issue_number="$(_sanitize_workflow_value "${ISSUE_NUMBER}")" - - _post_failure_ensure_token - - local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" - gh label create "${label}" --repo "${REPO_FULL_NAME}" \ - --description "Code agent needs human input to proceed" --color "D93F0B" \ - --force 2>/dev/null || true - gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ - -f "labels[]=${label}" --silent 2>/dev/null || true - gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ - -X DELETE --silent 2>/dev/null || true - - local sanitized_input - sanitized_input="$(sanitize_failure_detail "${needs_input}")" - - local body - body="🚧 **Code agent needs input** — issue #${safe_issue_number} - -The code agent stopped before implementing a fix because it needs input from a human before it can proceed safely. - -**What it needs:** -${sanitized_input} - -Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." - - if ! gh issue comment "${ISSUE_NUMBER}" \ - --repo "${REPO_FULL_NAME}" \ - --body "${body}" 2>/dev/null; then - gha_echo warning "Failed to post needs-input comment to issue #${safe_issue_number}" - fi -} -``` - -Notes: - -- `sanitize_failure_detail` (existing helper in - `scripts/lib/post-failure-report.lib.sh`) is reused to redact tokens and - cap length before the text is embedded in a public comment — same - discipline as failure-report comments. -- This is a clean stop (`exit 0`), not an error — no PR, no push, no - secret scan needed (nothing to scan). -- Plain `gh issue comment` (fresh comment each run), matching decision #2. -- `ready-to-code` is explicitly removed, matching decision #1. - -## Test plan (TDD — tests first) - -Existing test files establish two different conventions: - -- `scripts/post-code-test.sh` re-implements small logic fragments (title - rewriting, PR body assembly) as standalone shell functions and asserts - against them, plus greps the bundled script for expected function names. -- `scripts/post-triage-test.sh` runs the *actual* `post-triage.sh` script - end-to-end against fixture JSON, with a mock `gh` binary on `PATH` that - logs every invocation to a file, and asserts on logged call patterns. - -The `needs_input` short-circuit in `post-code.src.sh` is a clean early-exit -before any git operations, so it's a good fit for the `post-triage-test.sh` -style: run the real bundled `post-code.sh`, `REPO_DIR="."` (skip cd/git -setup entirely — we exit before any git commands run), mock `gh`, assert on -logged calls. - -Planned new tests in `scripts/post-code-test.sh` (or a new -`scripts/post-code-needs-input-test.sh` if that keeps the file more -readable — TBD during implementation): - -1. `needs-input-skips-push-and-pr` — given `{"target_branch":"main","needs_input":"..."}`, - assert the mock `gh` log contains no `git push`/`pr create` calls (there - won't be any `gh pr create` call logged at all). -2. `needs-input-applies-label` — assert log contains - `gh api repos/OWNER/REPO/issues/N/labels -f labels[]=fs-code-needs-input --silent`. -3. `needs-input-removes-ready-to-code` — assert log contains - `gh api repos/OWNER/REPO/issues/N/labels/ready-to-code -X DELETE --silent`. -4. `needs-input-posts-comment` — assert log contains - `gh issue comment N --repo OWNER/REPO --body ...` with the `needs_input` - text present in the body. -5. `needs-input-exits-zero` — the script exits 0 (clean stop, not a - failure). -6. `no-needs-input-field-unaffected` — a normal result file (no - `needs_input` key) still proceeds through the existing push/PR path - (regression guard — reuse an existing passing scenario if one already - covers this, otherwise add a minimal one). -7. Respect `CODE_NEEDS_INPUT_LABEL` env var override in the label-creation - call (falls back to `fs-code-needs-input` when unset). - -Schema tests: add a case to whatever exercises -`schemas/code-result.schema.json` (check `scripts/validate-output-schema.sh` -and its test file, if one exists, or add fixtures under the harness's -schema-validation test path) confirming: - -- `{"target_branch": "main", "needs_input": "..."}` validates. -- `needs_input` longer than 4000 chars fails validation. -- `needs_input` as empty string fails validation (`minLength: 1`). - -Run `make test` before starting (baseline green) and after each change -(TDD: write/extend a test, watch it fail for the right reason, then -implement, then watch it pass). - -## Files touched - -- `schemas/code-result.schema.json` — add `needs_input` field. -- `harness/code.yaml` — add `CODE_NEEDS_INPUT_LABEL` to `env.runner`. -- `agents/code.md` — document `needs_input` in "Structured output". -- `skills/code-implementation/SKILL.md` — steps 8, 9a, 9c updates described - above. -- `scripts/post-code.src.sh` — `post_needs_input_comment()` + early-exit - check (then `make script-build` to regenerate `scripts/post-code.sh`). -- `scripts/post-code-test.sh` (or new test file) — new test cases. -- Possibly a schema-validation test fixture, if one exists for - `code-result.schema.json` specifically. - -## Open items to watch during implementation - -- Confirm exact insertion point in `post-code.src.sh` doesn't disturb the - `AGENT_TARGET` branch-name-validation error path (needs_input check must - come *before* that validation, since a needs_input-only result may not - set a meaningful `target_branch` in edge cases — though per current - design the agent always writes `target_branch` regardless). -- Confirm `make script-build` regenerates `scripts/post-code.sh` cleanly - and `make check-bundle` passes (bundled script must match source). -- Lint agent docs (`hack/lint-agent-docs-test.sh` appeared in the test - suite) may enforce structure on `agents/code.md` — check it after - editing. diff --git a/eval/code/eval.yaml b/eval/code/eval.yaml index 340d9579..ba6d849f 100644 --- a/eval/code/eval.yaml +++ b/eval/code/eval.yaml @@ -137,7 +137,8 @@ judges: Post-script must open at least one pull request when annotations.expect_pr is true (default when unset — see eval/code/cases/001-fix-add). When a case sets expect_pr: false, this - asserts the opposite: no open/merged PR exists. Used for needs_input + asserts the opposite: no PR was created at all (open, merged, or + closed). Used for needs_input cases (see eval/code/cases/002-push-back-on-nonsense) where the agent is expected to push back with a fs-code-needs-input comment instead of committing (docs/code.md: Control labels). Can fail for reasons @@ -157,8 +158,8 @@ judges: if not openish: return False, "No pull requests found — code agent/post-script did not create a PR" return True, f"PR created: {[p.get('url') for p in openish]}" - if openish: - return False, f"Expected no PR (needs_input case) but found: {[p.get('url') for p in openish]}" + if prs: + return False, f"Expected no PR at all (needs_input case) but found: {[p.get('url') for p in prs]}" return True, "No PR created, as expected (needs_input case)" - name: required_labels diff --git a/schemas/code-result.schema.json b/schemas/code-result.schema.json index d3799366..0e01a979 100644 --- a/schemas/code-result.schema.json +++ b/schemas/code-result.schema.json @@ -26,7 +26,7 @@ "type": "string", "minLength": 1, "maxLength": 4000, - "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the needs-input label and posts this text as a comment instead of opening a PR." + "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the fs-code-needs-input label and posts this text as a comment instead of opening a PR." } } } diff --git a/skills/code-implementation/SKILL.md b/skills/code-implementation/SKILL.md index 0959cdd3..0e5a2fe2 100644 --- a/skills/code-implementation/SKILL.md +++ b/skills/code-implementation/SKILL.md @@ -637,8 +637,8 @@ exactly what's missing (the tool name, the command that failed, and the error), validate structured output (step 11), and stop — do NOT commit. **Do NOT silently skip tests or linters and commit as if everything -passed.** If you cannot run the relevant test suite or lint command, you -must disclose that. +passed.** If you cannot run the relevant test suite or lint command, use +`needs_input` as described above — do not commit. **If tests or linters fail due to your code:** @@ -914,8 +914,8 @@ optionally `pr_body` and `closes_issue`: ``` **Schema compliance:** The schema uses `additionalProperties: false`. -Only `target_branch`, `pr_body`, and `closes_issue` are allowed. Any -other fields will cause validation to fail. +Only `target_branch`, `pr_body`, `closes_issue`, and `needs_input` are +allowed. Any other fields will cause validation to fail. Validate the output against the schema: From 100f3519341dacac18c12f599e0ca21408ffb567 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 6 Aug 2026 17:20:26 -0400 Subject: [PATCH 04/14] fix(#682): harden post-code needs_input path per review feedback - Warn (instead of silently swallowing) label create/apply/remove failures in post_needs_input_comment, matching the existing comment-post failure pattern. - Stop truncating the needs_input comment from the tail -- it's forward, human-authored prose already length-capped by the schema (maxLength 4000), not command/log output where tail-ing makes sense. Truncating from the tail dropped the opening context of longer explanations. - Guard against a needs_input contract violation: warn (in both the workflow log and the posted comment) if the agent committed local work before setting needs_input, since that work is silently discarded, and check for an already-open PR on the branch to avoid posting a "no PR" comment alongside a real one. Adds a regression test for the truncation fix and two git-repo-backed tests for the new contract-violation guards. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- scripts/post-code-needs-input-test.sh | 100 ++++++++++++++++++++++++-- scripts/post-code.sh | 52 ++++++++++++-- scripts/post-code.src.sh | 52 ++++++++++++-- 3 files changed, 187 insertions(+), 17 deletions(-) diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index 2bde27ae..ffce8332 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -32,22 +32,31 @@ trap 'rm -rf "${TMPDIR}"' EXIT WORKDIR="${TMPDIR}/workdir" mkdir -p "${WORKDIR}" -# Mock gh: record every invocation to a log file. None of the needs_input -# calls read gh's stdout, so a bare logger is sufficient here (unlike -# post-triage-test.sh's mock, which also emulates --body-file - stdin -# capture and label-listing responses that this path doesn't need). +REPO_FULL_NAME="owner/repo" +ISSUE_NUMBER="42" + +# Mock gh: record every invocation to a log file. Most needs_input calls +# don't read gh's stdout, but the contract-violation guards (existing PR / +# default branch lookups) do, so this mock emulates those two responses via +# case-matching on the call — unlike post-triage-test.sh's mock, which +# emulates --body-file stdin capture and label-listing instead. GH_LOG="${TMPDIR}/gh-calls.log" MOCK_BIN="${TMPDIR}/bin" mkdir -p "${MOCK_BIN}" cat > "${MOCK_BIN}/gh" <> "${GH_LOG}" +case "\$*" in + *"repos/${REPO_FULL_NAME} --jq .default_branch"*) + echo "main" + ;; + *"pr list --repo ${REPO_FULL_NAME} --head"*"--json url"*) + echo "\${MOCK_EXISTING_PR_URL:-}" + ;; +esac MOCKEOF chmod +x "${MOCK_BIN}/gh" -REPO_FULL_NAME="owner/repo" -ISSUE_NUMBER="42" - # Runs the real post-code script against a fixture agent-result.json. # Leaves the result in EXIT_CODE, the gh call log at ${GH_LOG}, and stdout # at ${TMPDIR}/stdout.log for assertions. @@ -170,6 +179,21 @@ run_post_code '{"target_branch":"main"}' assert_log_pattern "no-needs-input-field-unaffected" \ "fs-code-needs-input" "no" +# Regression guard: a needs_input longer than POST_FAILURE_DETAIL_MAX_LINES +# (default 30) must not be truncated from the start. sanitize_failure_detail +# defaults to tail-based truncation (recent lines of command/log output); +# needs_input is forward, human-authored prose and must be posted in full. +NEEDS_INPUT_LONG_TEXT="opening context that must not be dropped" +for i in $(seq 1 35); do + NEEDS_INPUT_LONG_TEXT="${NEEDS_INPUT_LONG_TEXT}\nline ${i} of a long explanation" +done +FIXTURE_NEEDS_INPUT_LONG="{\"target_branch\":\"main\",\"needs_input\":\"${NEEDS_INPUT_LONG_TEXT}\"}" + +run_post_code "${FIXTURE_NEEDS_INPUT_LONG}" + +assert_log_pattern "needs-input-comment-not-truncated-from-start" \ + "opening context that must not be dropped" "yes" + # CODE_NEEDS_INPUT_LABEL env override run_post_code_with_label "${FIXTURE_NEEDS_INPUT}" "custom-label" @@ -179,6 +203,68 @@ assert_log_pattern "respects-code-needs-input-label-env-override" \ assert_log_pattern "respects-code-needs-input-label-env-override-no-default-label" \ "labels[]=fs-code-needs-input" "no" +# --- Contract-violation guard tests --- +# needs_input should mean "stop before implementing" — no local commits, no +# open PR. Unlike the tests above (a plain, non-git WORKDIR so the guard's +# `git branch --show-current` is always empty and the guard is a no-op), +# these use a real git repo with a feature branch ahead of a fake +# `origin/main` ref, so the guard's checks actually run. +GIT_WORKDIR="${TMPDIR}/git-workdir" + +setup_git_workdir_with_commits_ahead() { + rm -rf "${GIT_WORKDIR}" + git init -q -b main "${GIT_WORKDIR}" + git -C "${GIT_WORKDIR}" config user.email "test@example.com" + git -C "${GIT_WORKDIR}" config user.name "Test" + git -C "${GIT_WORKDIR}" commit --allow-empty -m "init" -q + # A local-only ref standing in for a fetched remote-tracking branch — no + # actual remote needed for the guard's merge-base-style comparison. + git -C "${GIT_WORKDIR}" update-ref refs/remotes/origin/main HEAD + git -C "${GIT_WORKDIR}" checkout -q -b feature/needs-input + git -C "${GIT_WORKDIR}" commit --allow-empty -m "agent work" -q +} + +run_post_code_in_git_workdir() { + local fixture_json="$1" + local fixture_dir="${TMPDIR}/fixture-input" + rm -rf "${fixture_dir}" + mkdir -p "${fixture_dir}" + echo "${fixture_json}" > "${fixture_dir}/agent-result.json" + + : > "${GH_LOG}" + + EXIT_CODE=0 + ( + cd "${GIT_WORKDIR}" && \ + PATH="${MOCK_BIN}:${PATH}" \ + REPO_DIR="." \ + PUSH_TOKEN="fake-token" \ + REPO_FULL_NAME="${REPO_FULL_NAME}" \ + ISSUE_NUMBER="${ISSUE_NUMBER}" \ + FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ + bash "${POST_SCRIPT}" + ) > "${TMPDIR}/stdout.log" 2>&1 || EXIT_CODE=$? +} + +setup_git_workdir_with_commits_ahead +run_post_code_in_git_workdir "${FIXTURE_NEEDS_INPUT}" + +assert_log_pattern "needs-input-warns-on-discarded-commits" \ + "were not pushed and will be discarded" "yes" + +assert_exit_code "needs-input-with-commits-still-exits-zero" 0 + +# Same git state, but gh pr list reports an already-open PR for the branch — +# the "existing PR" caveat should win over the "discarded commits" one. +MOCK_EXISTING_PR_URL="https://github.com/${REPO_FULL_NAME}/pull/7" \ + run_post_code_in_git_workdir "${FIXTURE_NEEDS_INPUT}" + +assert_log_pattern "needs-input-warns-on-existing-pr" \ + "An open PR already exists for branch" "yes" + +assert_log_pattern "needs-input-existing-pr-caveat-omits-discarded-commits" \ + "were not pushed and will be discarded" "no" + # --- Summary --- echo "" diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 574144ce..67a379a3 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1813,14 +1813,56 @@ post_needs_input_comment() { local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" gh label create "${label}" --repo "${REPO_FULL_NAME}" \ --description "Code agent needs human input to proceed" --color "D93F0B" \ - --force 2>/dev/null || true + --force 2>/dev/null || gha_echo warning "Failed to create/update label '${label}' on ${REPO_FULL_NAME}" gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ - -f "labels[]=${label}" --silent 2>/dev/null || true + -f "labels[]=${label}" --silent 2>/dev/null || \ + gha_echo warning "Failed to apply label '${label}' to issue #${safe_issue_number}" gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ - -X DELETE --silent 2>/dev/null || true + -X DELETE --silent 2>/dev/null || \ + gha_echo warning "Failed to remove 'ready-to-code' label from issue #${safe_issue_number}" + + # Guard against a contract violation: needs_input means "stop before + # implementing," so there should be no local commits and no open PR for + # this branch. Check anyway — cheaply — so a violation surfaces to the + # human instead of silently discarding the agent's work or leaving + # contradictory state (an open PR alongside a "no PR" comment). + local caveat="" + local current_branch + current_branch="$(git branch --show-current 2>/dev/null || true)" + if [ -n "${current_branch}" ]; then + local existing_pr_url + existing_pr_url="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${current_branch}" \ + --json url --jq '.[0].url // empty' 2>/dev/null || true)" + if [ -n "${existing_pr_url}" ]; then + caveat="⚠️ An open PR already exists for branch \`${current_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." + gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" + else + local default_branch commits_ahead + default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo main)" + if [ "${current_branch}" != "${default_branch}" ]; then + commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" + if [ "${commits_ahead}" -gt 0 ]; then + caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${current_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." + gha_echo warning "needs_input set but ${commits_ahead} local commit(s) exist on branch '${current_branch}' — discarding" + fi + fi + fi + fi local sanitized_input - sanitized_input="$(sanitize_failure_detail "${needs_input}")" + # max_lines=0 disables tail-based truncation: needs_input is forward, + # human-authored prose already length-capped by the schema (maxLength + # 4000), not command/log output where tail-ing to recent lines makes + # sense. Truncating from the tail would silently drop the opening + # framing of a long explanation. + sanitized_input="$(sanitize_failure_detail "${needs_input}" 0)" + + local caveat_block="" + if [ -n "${caveat}" ]; then + caveat_block=" +${caveat} +" + fi local body body="🚧 **Code agent needs input** — issue #${safe_issue_number} @@ -1829,7 +1871,7 @@ The code agent stopped before implementing a fix because it needs input from a h **What it needs:** ${sanitized_input} - +${caveat_block} Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." if ! gh issue comment "${ISSUE_NUMBER}" \ diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index 99353e58..d6386801 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -249,14 +249,56 @@ post_needs_input_comment() { local label="${CODE_NEEDS_INPUT_LABEL:-fs-code-needs-input}" gh label create "${label}" --repo "${REPO_FULL_NAME}" \ --description "Code agent needs human input to proceed" --color "D93F0B" \ - --force 2>/dev/null || true + --force 2>/dev/null || gha_echo warning "Failed to create/update label '${label}' on ${REPO_FULL_NAME}" gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ - -f "labels[]=${label}" --silent 2>/dev/null || true + -f "labels[]=${label}" --silent 2>/dev/null || \ + gha_echo warning "Failed to apply label '${label}' to issue #${safe_issue_number}" gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels/ready-to-code" \ - -X DELETE --silent 2>/dev/null || true + -X DELETE --silent 2>/dev/null || \ + gha_echo warning "Failed to remove 'ready-to-code' label from issue #${safe_issue_number}" + + # Guard against a contract violation: needs_input means "stop before + # implementing," so there should be no local commits and no open PR for + # this branch. Check anyway — cheaply — so a violation surfaces to the + # human instead of silently discarding the agent's work or leaving + # contradictory state (an open PR alongside a "no PR" comment). + local caveat="" + local current_branch + current_branch="$(git branch --show-current 2>/dev/null || true)" + if [ -n "${current_branch}" ]; then + local existing_pr_url + existing_pr_url="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${current_branch}" \ + --json url --jq '.[0].url // empty' 2>/dev/null || true)" + if [ -n "${existing_pr_url}" ]; then + caveat="⚠️ An open PR already exists for branch \`${current_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." + gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" + else + local default_branch commits_ahead + default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo main)" + if [ "${current_branch}" != "${default_branch}" ]; then + commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" + if [ "${commits_ahead}" -gt 0 ]; then + caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${current_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." + gha_echo warning "needs_input set but ${commits_ahead} local commit(s) exist on branch '${current_branch}' — discarding" + fi + fi + fi + fi local sanitized_input - sanitized_input="$(sanitize_failure_detail "${needs_input}")" + # max_lines=0 disables tail-based truncation: needs_input is forward, + # human-authored prose already length-capped by the schema (maxLength + # 4000), not command/log output where tail-ing to recent lines makes + # sense. Truncating from the tail would silently drop the opening + # framing of a long explanation. + sanitized_input="$(sanitize_failure_detail "${needs_input}" 0)" + + local caveat_block="" + if [ -n "${caveat}" ]; then + caveat_block=" +${caveat} +" + fi local body body="🚧 **Code agent needs input** — issue #${safe_issue_number} @@ -265,7 +307,7 @@ The code agent stopped before implementing a fix because it needs input from a h **What it needs:** ${sanitized_input} - +${caveat_block} Once this is resolved, remove the \`${label}\` label and re-trigger with \`/fs-code\`." if ! gh issue comment "${ISSUE_NUMBER}" \ From 38c1981e05c625ca57592908fffd02e44252570d Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 6 Aug 2026 17:33:12 -0400 Subject: [PATCH 05/14] fix(#682): tighten case 002 eval budget from an observed CI baseline max_turns/max_cost_usd were plausibility-based guesses. Update them using the one CI run we have (21 turns / $0.64, run 31042840745), applying the same headroom multipliers as 001-fix-add (~1.7x turns, ~2x cost) since we only have a single observation so far. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- .../code/cases/002-push-back-on-nonsense/annotations.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/eval/code/cases/002-push-back-on-nonsense/annotations.yaml b/eval/code/cases/002-push-back-on-nonsense/annotations.yaml index 2ff1150d..313bc42a 100644 --- a/eval/code/cases/002-push-back-on-nonsense/annotations.yaml +++ b/eval/code/cases/002-push-back-on-nonsense/annotations.yaml @@ -16,8 +16,12 @@ labels: # (read issue + repo, conclude no conservative interpretation exists) rather # than attempting an implementation, so this budget is tighter than # 001-fix-add's. -max_turns: 30 -max_cost_usd: 2.00 +# Observed baseline: 21 turns / $0.64 (CI run 31042840745). Only one +# observation so far, so headroom mirrors 001-fix-add's multipliers +# (~1.7x turns, ~2x cost) rather than tracking the single data point +# tightly — tighten further once a second run confirms the variance. +max_turns: 35 +max_cost_usd: 1.25 # Human reference only; not consumed by judges (same class as 001-fix-add's # code_expectations). From 7d3e64e745b9e78908f19bb4a44dec00a011504c Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Mon, 10 Aug 2026 12:19:40 -0400 Subject: [PATCH 06/14] fix(#682): warn instead of silently assuming 'main' as default branch post_needs_input_comment's discarded-commits check silently fell back to "main" when the gh api call for the repo's default branch failed. If the actual default branch differs, the subsequent git rev-list comparison silently reports zero commits ahead, dropping the discarded-commits caveat this check exists to surface. Now it warns via gha_echo when the API call fails, so the inaccuracy is visible in the workflow log. Addresses review feedback from waynesun09 on PR #682. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- scripts/post-code.sh | 5 ++++- scripts/post-code.src.sh | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 67a379a3..5b7ed628 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1838,7 +1838,10 @@ post_needs_input_comment() { gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" else local default_branch commits_ahead - default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo main)" + if ! default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null)"; then + default_branch="main" + gha_echo warning "Failed to determine default branch for ${REPO_FULL_NAME}; assuming 'main' — the discarded-commits check may be inaccurate" + fi if [ "${current_branch}" != "${default_branch}" ]; then commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" if [ "${commits_ahead}" -gt 0 ]; then diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index d6386801..b23be266 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -274,7 +274,10 @@ post_needs_input_comment() { gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" else local default_branch commits_ahead - default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null || echo main)" + if ! default_branch="$(gh api "repos/${REPO_FULL_NAME}" --jq '.default_branch' 2>/dev/null)"; then + default_branch="main" + gha_echo warning "Failed to determine default branch for ${REPO_FULL_NAME}; assuming 'main' — the discarded-commits check may be inaccurate" + fi if [ "${current_branch}" != "${default_branch}" ]; then commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" if [ "${commits_ahead}" -gt 0 ]; then From af293c72ed8cdbc79dcc9d46e9e051b826201dae Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Mon, 10 Aug 2026 12:19:49 -0400 Subject: [PATCH 07/14] docs(#682): soften hardcoded setup-retry count in SKILL.md "one attempt only" for Makefile setup-target retries was an unsubstantiated specific number. Softened to "a reasonable number of attempts (typically one, more only if the failure looks transient)" per review feedback, so the agent has room to judge transient vs. persistent failures rather than following a hardcoded count with no cited basis. Addresses review feedback from waynesun09 on PR #682. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- skills/code-implementation/SKILL.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/skills/code-implementation/SKILL.md b/skills/code-implementation/SKILL.md index 0e5a2fe2..835ca6ea 100644 --- a/skills/code-implementation/SKILL.md +++ b/skills/code-implementation/SKILL.md @@ -631,10 +631,11 @@ failures. **If tests or linters fail due to missing tools or infrastructure** (not due to your code): try the Makefile's setup targets first (`make deps`, -`make setup`, etc.) — one attempt only. If the tool still cannot run -after that attempt, write `needs_input` to the result file describing -exactly what's missing (the tool name, the command that failed, and the -error), validate structured output (step 11), and stop — do NOT commit. +`make setup`, etc.) — a reasonable number of attempts (typically one, +more only if the failure looks transient). If the tool still cannot run, +write `needs_input` to the result file describing exactly what's missing +(the tool name, the command that failed, and the error), validate +structured output (step 11), and stop — do NOT commit. **Do NOT silently skip tests or linters and commit as if everything passed.** If you cannot run the relevant test suite or lint command, use From 1b6c4165393f50c2eefc7d64f629000f57fb60f9 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Mon, 10 Aug 2026 15:10:45 -0400 Subject: [PATCH 08/14] fix(#682): honor CODE_NEEDS_INPUT_LABEL env var and sanitize branch name in comments harness/code.yaml hardcoded CODE_NEEDS_INPUT_LABEL to the default label instead of passing through the runner env var, silently defeating the operator override the script and its tests expect. post-code.src.sh interpolated the raw current_branch (chosen by the code agent while processing potentially adversarial issue content) into a public GitHub comment wrapped only in backticks. Git ref names permit backticks, so a malicious branch name could break out of the markdown code span and inject content into a comment posted with the bot's write token. Validate current_branch against the same safe-charset regex used for AGENT_TARGET and substitute a redacted placeholder when it fails, while still using the real branch name for the underlying gh/git checks. Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- scripts/post-code-needs-input-test.sh | 59 +++++++++++++++++++++++++++ scripts/post-code.sh | 13 +++++- scripts/post-code.src.sh | 13 +++++- 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index ffce8332..f3f1cb48 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -133,6 +133,39 @@ assert_log_pattern() { fi } +# Like assert_log_pattern, but scoped to just the "gh issue comment" call — +# i.e. the actual posted comment body — rather than the whole gh call log +# (which also legitimately contains raw values like the branch name from +# earlier "gh pr list --head" lookup calls). +assert_comment_body_pattern() { + local test_name="$1" + local pattern="$2" + local expect_present="$3" # "yes" or "no" + # The mocked gh call is logged as-is, including embedded newlines from a + # multi-line --body value, so pull every line from the "gh issue comment" + # call up to (not including) the next top-level "gh " invocation. + local comment_line + comment_line="$(awk '/^gh issue comment/{p=1} p && /^gh / && !/^gh issue comment/{exit} p' "${GH_LOG}")" + + if [ "${expect_present}" = "yes" ]; then + if grep -qF -- "${pattern}" <<<"${comment_line}"; then + echo "PASS: ${test_name}" + else + echo "FAIL: ${test_name} — expected comment body pattern '${pattern}' not found" + echo "Actual comment call: ${comment_line}" + FAILURES=$((FAILURES + 1)) + fi + else + if grep -qF -- "${pattern}" <<<"${comment_line}"; then + echo "FAIL: ${test_name} — expected comment body pattern '${pattern}' NOT to be found" + echo "Actual comment call: ${comment_line}" + FAILURES=$((FAILURES + 1)) + else + echo "PASS: ${test_name}" + fi + fi +} + assert_exit_code() { local test_name="$1" local expected="$2" @@ -265,6 +298,32 @@ assert_log_pattern "needs-input-warns-on-existing-pr" \ assert_log_pattern "needs-input-existing-pr-caveat-omits-discarded-commits" \ "were not pushed and will be discarded" "no" +# Branch names are chosen by the code agent and git ref names permit +# backticks — a branch name containing one must never be interpolated raw +# into the posted comment body, since it would break out of the markdown +# code span it's wrapped in. +BACKTICK_BRANCH='feature/`whoami`-needs-input' + +setup_git_workdir_with_backtick_branch() { + rm -rf "${GIT_WORKDIR}" + git init -q -b main "${GIT_WORKDIR}" + git -C "${GIT_WORKDIR}" config user.email "test@example.com" + git -C "${GIT_WORKDIR}" config user.name "Test" + git -C "${GIT_WORKDIR}" commit --allow-empty -m "init" -q + git -C "${GIT_WORKDIR}" update-ref refs/remotes/origin/main HEAD + git -C "${GIT_WORKDIR}" checkout -q -b "${BACKTICK_BRANCH}" + git -C "${GIT_WORKDIR}" commit --allow-empty -m "agent work" -q +} + +setup_git_workdir_with_backtick_branch +MOCK_EXISTING_PR_URL="" run_post_code_in_git_workdir "${FIXTURE_NEEDS_INPUT}" + +assert_comment_body_pattern "needs-input-backtick-branch-omitted-from-comment" \ + "${BACKTICK_BRANCH}" "no" + +assert_comment_body_pattern "needs-input-backtick-branch-placeholder-used" \ + "branch name omitted" "yes" + # --- Summary --- echo "" diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 5b7ed628..9d84473c 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1830,11 +1830,20 @@ post_needs_input_comment() { local current_branch current_branch="$(git branch --show-current 2>/dev/null || true)" if [ -n "${current_branch}" ]; then + # current_branch is chosen by the code agent inside the sandbox while + # processing potentially adversarial issue content, and git ref names + # permit backticks — never interpolate it raw into the comment body + # below. Same safe-charset check already applied to AGENT_TARGET. + local display_branch="${current_branch}" + if [[ ! "${current_branch}" =~ ^[a-zA-Z0-9._/-]+$ ]]; then + display_branch="(branch name omitted — contains unexpected characters, see workflow log)" + gha_echo warning "needs_input set on a branch with unexpected characters in its name; omitting the raw name from the issue comment" + fi local existing_pr_url existing_pr_url="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${current_branch}" \ --json url --jq '.[0].url // empty' 2>/dev/null || true)" if [ -n "${existing_pr_url}" ]; then - caveat="⚠️ An open PR already exists for branch \`${current_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." + caveat="⚠️ An open PR already exists for branch \`${display_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" else local default_branch commits_ahead @@ -1845,7 +1854,7 @@ post_needs_input_comment() { if [ "${current_branch}" != "${default_branch}" ]; then commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" if [ "${commits_ahead}" -gt 0 ]; then - caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${current_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." + caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${display_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." gha_echo warning "needs_input set but ${commits_ahead} local commit(s) exist on branch '${current_branch}' — discarding" fi fi diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index b23be266..a2f3601b 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -266,11 +266,20 @@ post_needs_input_comment() { local current_branch current_branch="$(git branch --show-current 2>/dev/null || true)" if [ -n "${current_branch}" ]; then + # current_branch is chosen by the code agent inside the sandbox while + # processing potentially adversarial issue content, and git ref names + # permit backticks — never interpolate it raw into the comment body + # below. Same safe-charset check already applied to AGENT_TARGET. + local display_branch="${current_branch}" + if [[ ! "${current_branch}" =~ ^[a-zA-Z0-9._/-]+$ ]]; then + display_branch="(branch name omitted — contains unexpected characters, see workflow log)" + gha_echo warning "needs_input set on a branch with unexpected characters in its name; omitting the raw name from the issue comment" + fi local existing_pr_url existing_pr_url="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${current_branch}" \ --json url --jq '.[0].url // empty' 2>/dev/null || true)" if [ -n "${existing_pr_url}" ]; then - caveat="⚠️ An open PR already exists for branch \`${current_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." + caveat="⚠️ An open PR already exists for branch \`${display_branch}\`: ${existing_pr_url}. The agent set \`needs_input\` on this run — check whether that PR is still current." gha_echo warning "needs_input set but an open PR already exists for branch '${current_branch}': ${existing_pr_url}" else local default_branch commits_ahead @@ -281,7 +290,7 @@ post_needs_input_comment() { if [ "${current_branch}" != "${default_branch}" ]; then commits_ahead="$(git rev-list --count "origin/${default_branch}..HEAD" 2>/dev/null || echo 0)" if [ "${commits_ahead}" -gt 0 ]; then - caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${current_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." + caveat="⚠️ The agent made ${commits_ahead} local commit(s) on branch \`${display_branch}\` before setting \`needs_input\` — these were not pushed and will be discarded." gha_echo warning "needs_input set but ${commits_ahead} local commit(s) exist on branch '${current_branch}' — discarding" fi fi From f2e68f72f4ea19eb1a2aee9f0f3541a1c8a67bf1 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:14:28 +0000 Subject: [PATCH 09/14] fix(#682): mask PUSH_TOKEN before needs_input early-exit path Move `echo "::add-mask::${PUSH_TOKEN}"` from after branch resolution to immediately after PUSH_TOKEN validation, so the token is masked in GHA logs even when the needs_input early-exit path is taken (which previously exited before the masking line ran). Addresses review feedback on #682 --- scripts/post-code.sh | 9 ++++----- scripts/post-code.src.sh | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 9d84473c..72298ded 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1755,6 +1755,10 @@ REPO_DIR="${REPO_DIR:-repo}" RUN_DIR="$(pwd)" : "${PUSH_TOKEN:?PUSH_TOKEN is required}" +echo "::add-mask::${PUSH_TOKEN}" +if [ -n "${GITLAB_TOKEN:-}" ]; then + echo "::add-mask::${GITLAB_TOKEN}" +fi : "${REPO_FULL_NAME:?REPO_FULL_NAME is required}" : "${ISSUE_NUMBER:?ISSUE_NUMBER is required}" trap 'report_post_failure_to_issue' ERR @@ -1980,11 +1984,6 @@ else echo "No agent branch preference — using repo default: ${TARGET_BRANCH}" fi -echo "::add-mask::${PUSH_TOKEN}" -if [ -n "${GITLAB_TOKEN:-}" ]; then - echo "::add-mask::${GITLAB_TOKEN}" -fi - # --------------------------------------------------------------------------- # No-op comment helper # diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index a2f3601b..e5445363 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -191,6 +191,10 @@ REPO_DIR="${REPO_DIR:-repo}" RUN_DIR="$(pwd)" : "${PUSH_TOKEN:?PUSH_TOKEN is required}" +echo "::add-mask::${PUSH_TOKEN}" +if [ -n "${GITLAB_TOKEN:-}" ]; then + echo "::add-mask::${GITLAB_TOKEN}" +fi : "${REPO_FULL_NAME:?REPO_FULL_NAME is required}" : "${ISSUE_NUMBER:?ISSUE_NUMBER is required}" trap 'report_post_failure_to_issue' ERR @@ -416,11 +420,6 @@ else echo "No agent branch preference — using repo default: ${TARGET_BRANCH}" fi -echo "::add-mask::${PUSH_TOKEN}" -if [ -n "${GITLAB_TOKEN:-}" ]; then - echo "::add-mask::${GITLAB_TOKEN}" -fi - # --------------------------------------------------------------------------- # No-op comment helper # From cf3d453c288095d4d15a122cc376c3cdd5792218 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:41:38 +0000 Subject: [PATCH 10/14] =?UTF-8?q?fix(#682):=20address=20review=20feedback?= =?UTF-8?q?=20=E2=80=94=20gitleaks=20scan,=20eval=20fixture,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add gitleaks secret scan for needs_input text before posting as an issue comment, matching the pr_body scanning pattern. Falls back to a generic redacted message if secrets are detected. - Create eval/code/repos/tiny-calc-neutral/ with a correct add() implementation (no BUG comment, passing tests) and update case 002 to use it, removing environmental bias that favored one side of the contradictory requirement. - Document CODE_NEEDS_INPUT_LABEL in post-code script headers and docs/code.md Variables table. - Remove stale "prefer committing with a disclosed issue" sentence from SKILL.md that contradicted the needs_input contract. - Update Step 11 prose to list needs_input alongside pr_body and closes_issue as an allowed optional field. Addresses review feedback on #682 --- docs/code.md | 1 + .../code/cases/002-push-back-on-nonsense/repo | 2 +- eval/code/repos/tiny-calc-neutral/README.md | 8 ++++++ eval/code/repos/tiny-calc-neutral/calc.py | 6 ++++ eval/code/repos/tiny-calc-neutral/conftest.py | 0 .../tiny-calc-neutral/tests/test_calc.py | 11 ++++++++ scripts/post-code.sh | 28 +++++++++++++++++++ scripts/post-code.src.sh | 28 +++++++++++++++++++ skills/code-implementation/SKILL.md | 5 ++-- 9 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 eval/code/repos/tiny-calc-neutral/README.md create mode 100644 eval/code/repos/tiny-calc-neutral/calc.py create mode 100644 eval/code/repos/tiny-calc-neutral/conftest.py create mode 100644 eval/code/repos/tiny-calc-neutral/tests/test_calc.py diff --git a/docs/code.md b/docs/code.md index 01b9c12d..27c10e75 100644 --- a/docs/code.md +++ b/docs/code.md @@ -50,6 +50,7 @@ See [Customizing with AGENTS.md](https://fullsend.sh/docs/guides/user/customizin | `FULLSEND_FORGE` | Forge platform. Set automatically by the harness `forge..env` section. | (set by harness) | `"github"`, `"gitlab"` | | `CODE_AUTO_MERGE` | Set to `"true"` to enable auto-merge on PRs/MRs created by the code agent. On GitHub, uses `gh pr merge --auto`; on GitLab, uses `merge_when_pipeline_succeeds`. Requires branch protection with required reviews or status checks on the target branch. Read directly from the runner environment (not declared in `env.runner`). | `""` (disabled) | `"true"` to enable | | `CODE_AUTO_MERGE_METHOD` | Merge method for auto-merge: `"squash"`, `"rebase"`, or `"merge"`. When unset, auto-detected from the repo's allowed merge methods (prefers squash). Omitted automatically when the target branch uses a merge queue. Ignored unless `CODE_AUTO_MERGE` is `"true"`. | Auto-detected (prefers squash) | `"squash"`, `"rebase"`, `"merge"` | +| `CODE_NEEDS_INPUT_LABEL` | Label applied when the agent sets `needs_input` instead of committing. Set via `env.runner` in `harness/code.yaml` as a hardcoded value (not forwarded from the runner environment). | `fs-code-needs-input` | Any valid GitHub label name | ## How the agent works diff --git a/eval/code/cases/002-push-back-on-nonsense/repo b/eval/code/cases/002-push-back-on-nonsense/repo index ae35fba8..3532ba49 120000 --- a/eval/code/cases/002-push-back-on-nonsense/repo +++ b/eval/code/cases/002-push-back-on-nonsense/repo @@ -1 +1 @@ -../../repos/tiny-calc \ No newline at end of file +../../repos/tiny-calc-neutral \ No newline at end of file diff --git a/eval/code/repos/tiny-calc-neutral/README.md b/eval/code/repos/tiny-calc-neutral/README.md new file mode 100644 index 00000000..307684ee --- /dev/null +++ b/eval/code/repos/tiny-calc-neutral/README.md @@ -0,0 +1,8 @@ +# tiny-calc-neutral + +Minimal Python calculator used by the code agent functional eval. + +Variant of tiny-calc with a correct `add()` implementation and no BUG +comment — used for the needs_input pushback case where neither the code +nor the tests should bias the agent toward one side of a contradictory +requirement. diff --git a/eval/code/repos/tiny-calc-neutral/calc.py b/eval/code/repos/tiny-calc-neutral/calc.py new file mode 100644 index 00000000..8ab7e620 --- /dev/null +++ b/eval/code/repos/tiny-calc-neutral/calc.py @@ -0,0 +1,6 @@ +# Tiny calculator — neutral implementation for the needs_input eval case. + + +def add(a: int, b: int) -> int: + """Return the sum of a and b.""" + return a + b diff --git a/eval/code/repos/tiny-calc-neutral/conftest.py b/eval/code/repos/tiny-calc-neutral/conftest.py new file mode 100644 index 00000000..e69de29b diff --git a/eval/code/repos/tiny-calc-neutral/tests/test_calc.py b/eval/code/repos/tiny-calc-neutral/tests/test_calc.py new file mode 100644 index 00000000..3895873b --- /dev/null +++ b/eval/code/repos/tiny-calc-neutral/tests/test_calc.py @@ -0,0 +1,11 @@ +"""Tests for calc module.""" + +from calc import add + + +def test_add() -> None: + assert add(2, 3) == 5 + + +def test_add_negative() -> None: + assert add(-1, -2) == -3 diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 72298ded..92c53f78 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -34,6 +34,11 @@ # — 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) +# CODE_NEEDS_INPUT_LABEL +# — label applied when the agent sets needs_input instead +# of committing. Set via env.runner in harness/code.yaml +# as a hardcoded value (not forwarded from the runner +# environment). (default: fs-code-needs-input) # POST_FAILURE_DETAIL_MAX_LINES # — max lines of failure detail in issue/PR comments (default: 30) # CODE_AUTO_MERGE — "true" to enable auto-merge on the PR/MR after @@ -1873,6 +1878,29 @@ post_needs_input_comment() { # framing of a long explanation. sanitized_input="$(sanitize_failure_detail "${needs_input}" 0)" + # Secret-scan needs_input — same category of free-form, agent-authored, + # out-of-git-tree prose as pr_body, also posted as a public issue comment. + # Run gitleaks to catch secrets that sanitize_failure_detail's fixed + # pattern set does not cover (e.g. AWS keys, DB passwords). + local ni_tmp gl_stderr gl_rc + ni_tmp="$(mktemp)" + printf '%s\n' "${sanitized_input}" > "${ni_tmp}" + gl_stderr="$(mktemp)" + gl_rc=0 + gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? + if [ -s "${gl_stderr}" ]; then + sed 's/^/::debug::gitleaks: /' "${gl_stderr}" + fi + rm -f "${gl_stderr}" + if [ "${gl_rc}" -eq 1 ]; then + gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" + sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" + elif [ "${gl_rc}" -gt 1 ]; then + gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" + fi + rm -f "${ni_tmp}" + local caveat_block="" if [ -n "${caveat}" ]; then caveat_block=" diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index e5445363..d62de167 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -33,6 +33,11 @@ # — 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) +# CODE_NEEDS_INPUT_LABEL +# — label applied when the agent sets needs_input instead +# of committing. Set via env.runner in harness/code.yaml +# as a hardcoded value (not forwarded from the runner +# environment). (default: fs-code-needs-input) # POST_FAILURE_DETAIL_MAX_LINES # — max lines of failure detail in issue/PR comments (default: 30) # CODE_AUTO_MERGE — "true" to enable auto-merge on the PR/MR after @@ -309,6 +314,29 @@ post_needs_input_comment() { # framing of a long explanation. sanitized_input="$(sanitize_failure_detail "${needs_input}" 0)" + # Secret-scan needs_input — same category of free-form, agent-authored, + # out-of-git-tree prose as pr_body, also posted as a public issue comment. + # Run gitleaks to catch secrets that sanitize_failure_detail's fixed + # pattern set does not cover (e.g. AWS keys, DB passwords). + local ni_tmp gl_stderr gl_rc + ni_tmp="$(mktemp)" + printf '%s\n' "${sanitized_input}" > "${ni_tmp}" + gl_stderr="$(mktemp)" + gl_rc=0 + gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? + if [ -s "${gl_stderr}" ]; then + sed 's/^/::debug::gitleaks: /' "${gl_stderr}" + fi + rm -f "${gl_stderr}" + if [ "${gl_rc}" -eq 1 ]; then + gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" + sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" + elif [ "${gl_rc}" -gt 1 ]; then + gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" + fi + rm -f "${ni_tmp}" + local caveat_block="" if [ -n "${caveat}" ]; then caveat_block=" diff --git a/skills/code-implementation/SKILL.md b/skills/code-implementation/SKILL.md index 835ca6ea..a9f313b9 100644 --- a/skills/code-implementation/SKILL.md +++ b/skills/code-implementation/SKILL.md @@ -657,8 +657,7 @@ passed.** If you cannot run the relevant test suite or lint command, use The retry limit is read from the `MAX_RETRIES` environment variable (default: 1 if unset). The harness may also enforce a hard timeout independently — if the harness kills the session, your retry count is -irrelevant. Prefer committing with a disclosed issue over burning time -on additional retry iterations. +irrelevant. If the retry limit is reached and tests or linters still fail, do not commit. Validate structured output, then stop: @@ -905,7 +904,7 @@ cat "${FULLSEND_OUTPUT_DIR}/agent-result.json" ``` The file must be valid JSON with `target_branch` (required) and -optionally `pr_body` and `closes_issue`: +optionally `pr_body`, `closes_issue`, and `needs_input`: ```json { From bd5ce677cacb873861b5633b801a4a2eff5035be Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:45:01 +0000 Subject: [PATCH 11/14] fix(#682): address remaining review findings from waynesun09 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - harness/code.yaml: change CODE_NEEDS_INPUT_LABEL from hardcoded "fs-code-needs-input" to "${CODE_NEEDS_INPUT_LABEL}" passthrough, matching the pattern used by every sibling env.runner entry - schemas/code-result.schema.json: make target_branch conditionally required — only when needs_input is absent — so a broken-tooling scenario where target_branch cannot be determined still surfaces the needs_input signal instead of failing schema validation silently - post-code.src.sh: apply fs-code-needs-input-conflict label when the contract-violation guard detects local commits or an existing PR alongside needs_input, providing a machine-queryable signal - Update docs/code.md and script headers to reflect the passthrough - Add tests for all three changes Addresses review feedback on #682 --- docs/code.md | 2 +- schemas/code-result.schema.json | 9 ++++++++- scripts/code-result-schema-test.sh | 4 ++++ scripts/post-code-needs-input-test.sh | 9 +++++++++ scripts/post-code.sh | 16 +++++++++++++--- scripts/post-code.src.sh | 16 +++++++++++++--- 6 files changed, 48 insertions(+), 8 deletions(-) diff --git a/docs/code.md b/docs/code.md index 27c10e75..08aa0eef 100644 --- a/docs/code.md +++ b/docs/code.md @@ -50,7 +50,7 @@ See [Customizing with AGENTS.md](https://fullsend.sh/docs/guides/user/customizin | `FULLSEND_FORGE` | Forge platform. Set automatically by the harness `forge..env` section. | (set by harness) | `"github"`, `"gitlab"` | | `CODE_AUTO_MERGE` | Set to `"true"` to enable auto-merge on PRs/MRs created by the code agent. On GitHub, uses `gh pr merge --auto`; on GitLab, uses `merge_when_pipeline_succeeds`. Requires branch protection with required reviews or status checks on the target branch. Read directly from the runner environment (not declared in `env.runner`). | `""` (disabled) | `"true"` to enable | | `CODE_AUTO_MERGE_METHOD` | Merge method for auto-merge: `"squash"`, `"rebase"`, or `"merge"`. When unset, auto-detected from the repo's allowed merge methods (prefers squash). Omitted automatically when the target branch uses a merge queue. Ignored unless `CODE_AUTO_MERGE` is `"true"`. | Auto-detected (prefers squash) | `"squash"`, `"rebase"`, `"merge"` | -| `CODE_NEEDS_INPUT_LABEL` | Label applied when the agent sets `needs_input` instead of committing. Set via `env.runner` in `harness/code.yaml` as a hardcoded value (not forwarded from the runner environment). | `fs-code-needs-input` | Any valid GitHub label name | +| `CODE_NEEDS_INPUT_LABEL` | Label applied when the agent sets `needs_input` instead of committing. Forwarded from the runner environment via `env.runner` in `harness/code.yaml`. The script defaults to `fs-code-needs-input` when unset. | `fs-code-needs-input` | Any valid GitHub label name | ## How the agent works diff --git a/schemas/code-result.schema.json b/schemas/code-result.schema.json index 0e01a979..d1aee819 100644 --- a/schemas/code-result.schema.json +++ b/schemas/code-result.schema.json @@ -4,7 +4,6 @@ "title": "Code Agent Result", "description": "Structured output from the code agent documenting the target branch and PR body for PR creation.", "type": "object", - "required": ["target_branch"], "additionalProperties": false, "properties": { "target_branch": { @@ -28,5 +27,13 @@ "maxLength": 4000, "description": "Set when the agent cannot proceed without human input — either the environment/tooling is broken (can't verify changes) or the issue is genuinely uninterpretable. Explain specifically what is needed. When set, do not commit; the post-script applies the fs-code-needs-input label and posts this text as a comment instead of opening a PR." } + }, + "if": { + "not": { + "required": ["needs_input"] + } + }, + "then": { + "required": ["target_branch"] } } diff --git a/scripts/code-result-schema-test.sh b/scripts/code-result-schema-test.sh index a3daf206..a438c6ea 100755 --- a/scripts/code-result-schema-test.sh +++ b/scripts/code-result-schema-test.sh @@ -77,6 +77,10 @@ run_test "valid-with-needs-input" \ '{"target_branch":"main","needs_input":"scan-secrets helper not found"}' \ "true" +run_test "valid-needs-input-without-target-branch" \ + '{"needs_input":"sandbox tooling broken — cannot determine target branch"}' \ + "true" + run_test "invalid-needs-input-empty-string" \ '{"target_branch":"main","needs_input":""}' \ "false" diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index f3f1cb48..d06f6d5a 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -201,6 +201,9 @@ assert_log_pattern "needs-input-posts-comment" \ assert_log_pattern "needs-input-posts-comment-includes-text" \ "${NEEDS_INPUT_TEXT}" "yes" +assert_log_pattern "needs-input-no-conflict-label-on-clean-path" \ + "fs-code-needs-input-conflict" "no" + assert_exit_code "needs-input-exits-zero" 0 # Regression guard: a result file without needs_input must not take the @@ -285,6 +288,9 @@ run_post_code_in_git_workdir "${FIXTURE_NEEDS_INPUT}" assert_log_pattern "needs-input-warns-on-discarded-commits" \ "were not pushed and will be discarded" "yes" +assert_log_pattern "needs-input-conflict-label-applied-on-discarded-commits" \ + "labels[]=fs-code-needs-input-conflict" "yes" + assert_exit_code "needs-input-with-commits-still-exits-zero" 0 # Same git state, but gh pr list reports an already-open PR for the branch — @@ -295,6 +301,9 @@ MOCK_EXISTING_PR_URL="https://github.com/${REPO_FULL_NAME}/pull/7" \ assert_log_pattern "needs-input-warns-on-existing-pr" \ "An open PR already exists for branch" "yes" +assert_log_pattern "needs-input-conflict-label-applied-on-existing-pr" \ + "labels[]=fs-code-needs-input-conflict" "yes" + assert_log_pattern "needs-input-existing-pr-caveat-omits-discarded-commits" \ "were not pushed and will be discarded" "no" diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 92c53f78..17b27656 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -36,9 +36,9 @@ # branch is allowed. (default: auto-detected) # CODE_NEEDS_INPUT_LABEL # — label applied when the agent sets needs_input instead -# of committing. Set via env.runner in harness/code.yaml -# as a hardcoded value (not forwarded from the runner -# environment). (default: fs-code-needs-input) +# of committing. Forwarded from the runner environment +# via env.runner in harness/code.yaml. The script +# defaults when unset. (default: fs-code-needs-input) # POST_FAILURE_DETAIL_MAX_LINES # — max lines of failure detail in issue/PR comments (default: 30) # CODE_AUTO_MERGE — "true" to enable auto-merge on the PR/MR after @@ -1906,6 +1906,16 @@ post_needs_input_comment() { caveat_block=" ${caveat} " + # Apply a machine-queryable conflict label so dashboards/automation can + # distinguish "clean needs_input" from "agent violated the needs_input + # contract" without reading comment prose. + local conflict_label="${label}-conflict" + gh label create "${conflict_label}" --repo "${REPO_FULL_NAME}" \ + --description "Code agent set needs_input but left local commits or an open PR" --color "E4E669" \ + --force 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ + -f "labels[]=${conflict_label}" --silent 2>/dev/null || \ + gha_echo warning "Failed to apply conflict label '${conflict_label}' to issue #${safe_issue_number}" fi local body diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index d62de167..9b637ed1 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -35,9 +35,9 @@ # branch is allowed. (default: auto-detected) # CODE_NEEDS_INPUT_LABEL # — label applied when the agent sets needs_input instead -# of committing. Set via env.runner in harness/code.yaml -# as a hardcoded value (not forwarded from the runner -# environment). (default: fs-code-needs-input) +# of committing. Forwarded from the runner environment +# via env.runner in harness/code.yaml. The script +# defaults when unset. (default: fs-code-needs-input) # POST_FAILURE_DETAIL_MAX_LINES # — max lines of failure detail in issue/PR comments (default: 30) # CODE_AUTO_MERGE — "true" to enable auto-merge on the PR/MR after @@ -342,6 +342,16 @@ post_needs_input_comment() { caveat_block=" ${caveat} " + # Apply a machine-queryable conflict label so dashboards/automation can + # distinguish "clean needs_input" from "agent violated the needs_input + # contract" without reading comment prose. + local conflict_label="${label}-conflict" + gh label create "${conflict_label}" --repo "${REPO_FULL_NAME}" \ + --description "Code agent set needs_input but left local commits or an open PR" --color "E4E669" \ + --force 2>/dev/null || true + gh api "repos/${REPO_FULL_NAME}/issues/${ISSUE_NUMBER}/labels" \ + -f "labels[]=${conflict_label}" --silent 2>/dev/null || \ + gha_echo warning "Failed to apply conflict label '${conflict_label}' to issue #${safe_issue_number}" fi local body From f47f8c34fe2793ca8e61ad57431d757667fab7f9 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 00:13:40 +0000 Subject: [PATCH 12/14] fix(#682): mock gitleaks in needs-input test, add eval env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-code-needs-input-test.sh script runs the real post-code.src.sh which calls gitleaks inside post_needs_input_comment() — but gitleaks is installed later by install_gitleaks(), well past the needs_input early-exit path. On CI runners where gitleaks is not pre-installed, the scan fails with exit 127 and the content is replaced with a generic redacted message, breaking test assertions that check for the original needs_input text. - Add a mock gitleaks binary (exit 0, no secrets) to MOCK_BIN in the test, matching the existing gh mock pattern - Add CODE_NEEDS_INPUT_LABEL to eval/scripts/run-fullsend.sh env file for code/fix agents (empty = use default), matching the pattern of CODE_ALLOWED_TARGET_BRANCHES Addresses review feedback on #682 --- eval/scripts/run-fullsend.sh | 2 ++ scripts/post-code-needs-input-test.sh | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/eval/scripts/run-fullsend.sh b/eval/scripts/run-fullsend.sh index fc160e84..a13c2755 100755 --- a/eval/scripts/run-fullsend.sh +++ b/eval/scripts/run-fullsend.sh @@ -163,6 +163,8 @@ install -m 0600 /dev/null "$ENV_FILE" # Empty matches production reusable-code.yml: post-code.sh treats unset/empty # as fallback to the repo default branch (not "allow all"; use * for any). emit_env "CODE_ALLOWED_TARGET_BRANCHES" "" + # Empty lets post-code.sh fall back to "fs-code-needs-input" default. + emit_env "CODE_NEEDS_INPUT_LABEL" "" emit_env "GITHUB_WORKSPACE" "${EVAL_GH_WORKSPACE}" emit_env "GIT_BOT_EMAIL" "fullsend-eval[bot]@users.noreply.github.com" ;; diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index d06f6d5a..c8635434 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -57,6 +57,18 @@ esac MOCKEOF chmod +x "${MOCK_BIN}/gh" +# Mock gitleaks: the real gitleaks is installed later in the post-code script +# (by install_gitleaks) — well past the needs_input early-exit path. On CI +# runners, gitleaks is not pre-installed, so the scan in +# post_needs_input_comment would fail with exit 127, causing the content to be +# replaced with a generic redacted message and breaking test assertions. +cat > "${MOCK_BIN}/gitleaks" <<'GLEOF' +#!/usr/bin/env bash +# No secrets found — exit 0. +exit 0 +GLEOF +chmod +x "${MOCK_BIN}/gitleaks" + # Runs the real post-code script against a fixture agent-result.json. # Leaves the result in EXIT_CODE, the gh call log at ${GH_LOG}, and stdout # at ${TMPDIR}/stdout.log for assertions. From 5a0270fbf734f1abc8e22b29f6526307c21ffbb5 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:57:31 +0000 Subject: [PATCH 13/14] fix(#682): call install_gitleaks before scanning needs_input text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit post_needs_input_comment() calls gitleaks detect to scan the needs_input text, but install_gitleaks was only called in step 3 — after the needs_input early-exit. On CI runners without a pre-installed gitleaks binary, the scan failed with exit 127 and the content was silently replaced with a generic redacted message. Fix: call install_gitleaks inside post_needs_input_comment before the gitleaks detect call (no-op when already on PATH). Remove the gitleaks mock from post-code-needs-input-test.sh since the script now properly ensures the binary is available. Addresses review feedback on #682 --- scripts/post-code-needs-input-test.sh | 14 ++------ scripts/post-code.sh | 47 +++++++++++++++++---------- scripts/post-code.src.sh | 45 +++++++++++++++---------- 3 files changed, 60 insertions(+), 46 deletions(-) diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index c8635434..fcb68903 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -57,17 +57,9 @@ esac MOCKEOF chmod +x "${MOCK_BIN}/gh" -# Mock gitleaks: the real gitleaks is installed later in the post-code script -# (by install_gitleaks) — well past the needs_input early-exit path. On CI -# runners, gitleaks is not pre-installed, so the scan in -# post_needs_input_comment would fail with exit 127, causing the content to be -# replaced with a generic redacted message and breaking test assertions. -cat > "${MOCK_BIN}/gitleaks" <<'GLEOF' -#!/usr/bin/env bash -# No secrets found — exit 0. -exit 0 -GLEOF -chmod +x "${MOCK_BIN}/gitleaks" +# No gitleaks mock needed: post_needs_input_comment now calls +# install_gitleaks itself before scanning, so gitleaks is available on +# all platforms (sandbox images pre-install it; CI runners download it). # Runs the real post-code script against a fixture agent-result.json. # Leaves the result in EXIT_CODE, the gh call log at ${GH_LOG}, and stdout diff --git a/scripts/post-code.sh b/scripts/post-code.sh index 17b27656..3f51b97b 100755 --- a/scripts/post-code.sh +++ b/scripts/post-code.sh @@ -1882,24 +1882,35 @@ post_needs_input_comment() { # out-of-git-tree prose as pr_body, also posted as a public issue comment. # Run gitleaks to catch secrets that sanitize_failure_detail's fixed # pattern set does not cover (e.g. AWS keys, DB passwords). - local ni_tmp gl_stderr gl_rc - ni_tmp="$(mktemp)" - printf '%s\n' "${sanitized_input}" > "${ni_tmp}" - gl_stderr="$(mktemp)" - gl_rc=0 - gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? - if [ -s "${gl_stderr}" ]; then - sed 's/^/::debug::gitleaks: /' "${gl_stderr}" - fi - rm -f "${gl_stderr}" - if [ "${gl_rc}" -eq 1 ]; then - gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" - sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" - elif [ "${gl_rc}" -gt 1 ]; then - gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" - sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" - fi - rm -f "${ni_tmp}" + # + # install_gitleaks is a no-op when gitleaks is already on PATH (sandbox + # images pre-install it). On CI runners without a pre-installed binary + # it downloads and verifies the pinned release — the same function the + # main secret-scan step (step 3) calls later, but that step is past the + # needs_input early-exit, so we must ensure the binary is available here. + if ! install_gitleaks; then + gha_echo warning "Failed to install gitleaks for needs_input scan; replacing content with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation could not run because gitleaks installation failed. Check the workflow log for details.)" + else + local ni_tmp gl_stderr gl_rc + ni_tmp="$(mktemp)" + printf '%s\n' "${sanitized_input}" > "${ni_tmp}" + gl_stderr="$(mktemp)" + gl_rc=0 + gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? + if [ -s "${gl_stderr}" ]; then + sed 's/^/::debug::gitleaks: /' "${gl_stderr}" + fi + rm -f "${gl_stderr}" + if [ "${gl_rc}" -eq 1 ]; then + gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" + sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" + elif [ "${gl_rc}" -gt 1 ]; then + gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" + fi + rm -f "${ni_tmp}" + fi local caveat_block="" if [ -n "${caveat}" ]; then diff --git a/scripts/post-code.src.sh b/scripts/post-code.src.sh index 9b637ed1..eff540fb 100755 --- a/scripts/post-code.src.sh +++ b/scripts/post-code.src.sh @@ -318,24 +318,35 @@ post_needs_input_comment() { # out-of-git-tree prose as pr_body, also posted as a public issue comment. # Run gitleaks to catch secrets that sanitize_failure_detail's fixed # pattern set does not cover (e.g. AWS keys, DB passwords). - local ni_tmp gl_stderr gl_rc - ni_tmp="$(mktemp)" - printf '%s\n' "${sanitized_input}" > "${ni_tmp}" - gl_stderr="$(mktemp)" - gl_rc=0 - gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? - if [ -s "${gl_stderr}" ]; then - sed 's/^/::debug::gitleaks: /' "${gl_stderr}" - fi - rm -f "${gl_stderr}" - if [ "${gl_rc}" -eq 1 ]; then - gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" - sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" - elif [ "${gl_rc}" -gt 1 ]; then - gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" - sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" + # + # install_gitleaks is a no-op when gitleaks is already on PATH (sandbox + # images pre-install it). On CI runners without a pre-installed binary + # it downloads and verifies the pinned release — the same function the + # main secret-scan step (step 3) calls later, but that step is past the + # needs_input early-exit, so we must ensure the binary is available here. + if ! install_gitleaks; then + gha_echo warning "Failed to install gitleaks for needs_input scan; replacing content with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation could not run because gitleaks installation failed. Check the workflow log for details.)" + else + local ni_tmp gl_stderr gl_rc + ni_tmp="$(mktemp)" + printf '%s\n' "${sanitized_input}" > "${ni_tmp}" + gl_stderr="$(mktemp)" + gl_rc=0 + gitleaks detect --source "${ni_tmp}" --no-git --redact 2>"${gl_stderr}" || gl_rc=$? + if [ -s "${gl_stderr}" ]; then + sed 's/^/::debug::gitleaks: /' "${gl_stderr}" + fi + rm -f "${gl_stderr}" + if [ "${gl_rc}" -eq 1 ]; then + gha_echo warning "BLOCKED — secret detected in needs_input text; replacing with generic message" + sanitized_input="(Content redacted — the agent's explanation contained a potential secret. Check the workflow log for details.)" + elif [ "${gl_rc}" -gt 1 ]; then + gha_echo warning "gitleaks scan of needs_input failed (exit ${gl_rc}); replacing with generic message" + sanitized_input="(Content redacted — secret scan of the agent's explanation failed. Check the workflow log for details.)" + fi + rm -f "${ni_tmp}" fi - rm -f "${ni_tmp}" local caveat_block="" if [ -n "${caveat}" ]; then From 63d22294b53caff5207ba19a9e6ef01c8282da4f Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:37:54 +0000 Subject: [PATCH 14/14] fix(#682): add FULLSEND_FORGE to needs-input test after multi-forge rebase The rebase onto main picked up the multi-forge change (PR #813) which requires FULLSEND_FORGE to be set. The needs-input test was missing this env var, causing all test invocations to fail with "invalid FULLSEND_FORGE" at script load time. Also resolves merge conflicts from the rebase: - harness/code.yaml: kept both CODE_NEEDS_INPUT_LABEL (PR) and sandbox env vars (main) - scripts/post-code.sh, post-code.src.sh: merged GITLAB_TOKEN masking (main) with early PUSH_TOKEN masking (PR) Addresses review feedback on #682 --- scripts/post-code-needs-input-test.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/post-code-needs-input-test.sh b/scripts/post-code-needs-input-test.sh index fcb68903..b6112a71 100644 --- a/scripts/post-code-needs-input-test.sh +++ b/scripts/post-code-needs-input-test.sh @@ -81,6 +81,7 @@ run_post_code() { PUSH_TOKEN="fake-token" \ REPO_FULL_NAME="${REPO_FULL_NAME}" \ ISSUE_NUMBER="${ISSUE_NUMBER}" \ + FULLSEND_FORGE="github" \ FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ bash "${POST_SCRIPT}" ) > "${TMPDIR}/stdout.log" 2>&1 || EXIT_CODE=$? @@ -105,6 +106,7 @@ run_post_code_with_label() { PUSH_TOKEN="fake-token" \ REPO_FULL_NAME="${REPO_FULL_NAME}" \ ISSUE_NUMBER="${ISSUE_NUMBER}" \ + FULLSEND_FORGE="github" \ CODE_NEEDS_INPUT_LABEL="${label}" \ FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ bash "${POST_SCRIPT}" @@ -281,6 +283,7 @@ run_post_code_in_git_workdir() { PUSH_TOKEN="fake-token" \ REPO_FULL_NAME="${REPO_FULL_NAME}" \ ISSUE_NUMBER="${ISSUE_NUMBER}" \ + FULLSEND_FORGE="github" \ FULLSEND_VALIDATED_ITERATION_DIR="${fixture_dir}" \ bash "${POST_SCRIPT}" ) > "${TMPDIR}/stdout.log" 2>&1 || EXIT_CODE=$?