Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/code.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ on issues (not PRs). The code agent is also triggered automatically when the

| Label | Meaning |
|-------|---------|
| `ready-to-code` | Triggers the code agent. Applied by the [triage](triage.md) post-script for low-risk categories (bug, documentation, performance), or manually by a human for feature work after prioritization. |
| `ready-to-code` | Triggers the code agent. May be applied by the [triage](triage.md) agent, or manually by a human for feature work after prioritization. |
| `ready-for-review` | Applied by the code agent's post-script after pushing a PR. In per-repo installs, triggers review when applied to a PR; also marks workflow state for humans and the retro agent. |

## Configuration and extension
Expand Down
10 changes: 10 additions & 0 deletions docs/triage.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ This gives the triage agent the subtlety it needs to distinguish between
controller-runtime code, without adding label documentation to `AGENTS.md`
where every agent would pay the context cost.

### Variables

| Variable | Description | Default | Valid values |
|----------|-------------|---------|--------------|
| `TRIAGE_AUTO_CODE` | When `true`, low-risk categories (bug, documentation, performance) automatically receive the `ready-to-code` label, triggering the [code agent](code.md). When `false`, these categories receive `triaged` instead, requiring human review before coding begins. | `true` | `true`, `false` |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] TRIAGE_AUTO_CODE's design doesn't appear to have been requested or confirmed in the issue this PR claims to fix

Finding: The full fullsend-ai/fullsend#1750 thread (root-cause comment, reporter follow-up, RICE-score comment, and the 2026-07-02 meeting-update comment) doesn't discuss or request a mechanism to gate auto-promotion to ready-to-code. The meeting-update decision is scoped to whether the triage agent should run by default (yes) — a different question from whether a successful triage should auto-trigger the code agent, which is what TRIAGE_AUTO_CODE actually controls. Unlike the neighboring code comment in the same block, which cites a real discussion ("#561") for the existing feature/non-feature split, this new gate has no comparable paper trail.

Suggestion: Get explicit maintainer/reporter sign-off on the requirement (a comment on a genuinely relevant issue, or a fresh issue capturing the need) before merging, or reframe the PR description as an independent enhancement not tied to closing #1750.


Set this in the CI workflow `env:` block. The env file passes it to the
sandbox automatically, and the post-script reads it from the runner
environment directly — no separate configuration is needed.

## Source

[`harness/triage.yaml`](../harness/triage.yaml)
1 change: 1 addition & 0 deletions env/triage.env
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export GITHUB_ISSUE_URL="${GITHUB_ISSUE_URL}"
export GH_TOKEN=${GH_TOKEN}
export TRIAGE_AUTO_CODE="${TRIAGE_AUTO_CODE:-true}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] env/triage.env ships TRIAGE_AUTO_CODE into the sandbox using a fallback syntax the harness's expansion doesn't support

Finding: env/triage.env is a host_files entry (expand: true) copied to /sandbox/workspace/.env.d/triage.env, seeding the sandboxed agent's environment. agents/triage.md (the sandboxed prompt, untouched here) never references TRIAGE_AUTO_CODE — only the runner-side scripts/post-triage.sh reads it, already served by the separate env.runner.TRIAGE_AUTO_CODE entry this PR adds in harness/triage.yaml, which is correctly absent from env.sandbox. The expand: true substitution mechanism (fullsend-ai/fullsend's internal/cli/run.go, shellSafeExpandEnv) is os.Expand(text, func(key) { return escapeForDoubleQuotes(os.Getenv(key)) }) — Go's brace expansion treats everything between ${ and } as one literal variable name, with no bash :-default support. So ${TRIAGE_AUTO_CODE:-true} is expanded by looking up an env var literally named TRIAGE_AUTO_CODE:-true (which never exists), always yielding an empty string — the file actually written into the sandbox is export TRIAGE_AUTO_CODE="", never "true", regardless of the runner's value. This contradicts docs/triage.md's claim that "The env file passes it to the sandbox automatically... no separate configuration is needed," and breaks the pattern used elsewhere in this repo (harness/code.yaml documents its analogous runner-only var CODE_ALLOWED_TARGET_BRANCHES as never entering the sandbox, correctly absent from env/code.env). Currently harmless only because nothing in the sandbox reads the variable.

Suggestion: Drop the TRIAGE_AUTO_CODE line from env/triage.env (it serves no purpose in the sandbox today) and correct docs/triage.md's claim about automatic sandbox delivery. If a sandbox-side consumer is ever needed, add it to harness/triage.yaml's env.sandbox block (matching the GITHUB_ISSUE_URL/GH_TOKEN pattern) rather than relying on bash-default syntax the harness doesn't honor.

1 change: 1 addition & 0 deletions harness/triage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ forge:
runner:
GITHUB_ISSUE_URL: ${GITHUB_ISSUE_URL}
GH_TOKEN: ${GH_TOKEN}
TRIAGE_AUTO_CODE: ${TRIAGE_AUTO_CODE}
sandbox:
GITHUB_ISSUE_URL: "${GITHUB_ISSUE_URL}"
GH_TOKEN: "${GH_TOKEN}"
72 changes: 72 additions & 0 deletions scripts/post-triage-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,78 @@ run_test "label-category-consistent-passes" \
'{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady.","label_actions":{"reason":"Area label applies.","actions":[{"action":"add","label":"area/api"}]}}' \
"gh api repos/test-org/test-repo/issues/42/labels -f labels[]=area/api --silent"

# --- TRIAGE_AUTO_CODE=false tests ---

# When TRIAGE_AUTO_CODE=false, bug category should get "triaged" instead of "ready-to-code".
run_test_auto_code_disabled() {
local test_name="$1"
local json_content="$2"
local expected_pattern="$3"
local forbidden_pattern="${4:-}"

local run_dir="${TMPDIR}/run-${test_name}"
mkdir -p "${run_dir}/iteration-1/output"
echo "${json_content}" > "${run_dir}/iteration-1/output/agent-result.json"
: > "${GH_LOG}"

local exit_code=0
(cd "${run_dir}" && TRIAGE_AUTO_CODE=false bash "${POST_SCRIPT}") > "${TMPDIR}/stdout.log" 2>&1 || exit_code=$?

if [[ ${exit_code} -ne 0 ]]; then
echo "FAIL: ${test_name} — exit code ${exit_code}"
cat "${TMPDIR}/stdout.log"
FAILURES=$((FAILURES + 1))
return
fi

if ! grep -qF "${expected_pattern}" "${GH_LOG}"; then
echo "FAIL: ${test_name} — expected '${expected_pattern}' not found"
echo "Actual calls:"
cat "${GH_LOG}"
FAILURES=$((FAILURES + 1))
return
fi

if [[ -n "${forbidden_pattern}" ]] && grep -qF "${forbidden_pattern}" "${GH_LOG}"; then
echo "FAIL: ${test_name} — forbidden '${forbidden_pattern}' was found"
echo "Actual calls:"
cat "${GH_LOG}"
FAILURES=$((FAILURES + 1))
return
fi

echo "PASS: ${test_name}"
}

SUFFICIENT_BUG='{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady."}'

SUFFICIENT_DOCS='{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Update docs","severity":"low","category":"documentation","problem":"Outdated docs","root_cause_hypothesis":"Not updated","reproduction_steps":["step 1"],"environment":"Linux","impact":"Contributors","recommended_fix":"Update README","proposed_test_case":"test_docs"},"comment":"## Triage Summary\n\nDocs issue."}'

SUFFICIENT_PERF='{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Slow query","severity":"medium","category":"performance","problem":"Slow","root_cause_hypothesis":"Missing index","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Add index","proposed_test_case":"test_query_speed"},"comment":"## Triage Summary\n\nPerf issue."}'

run_test_auto_code_disabled "auto-code-disabled-bug-gets-triaged" \
"${SUFFICIENT_BUG}" \
"labels[]=triaged" \
"labels[]=ready-to-code"

run_test_auto_code_disabled "auto-code-disabled-bug-still-gets-bug-label" \
"${SUFFICIENT_BUG}" \
"labels[]=bug"

run_test_auto_code_disabled "auto-code-disabled-docs-gets-triaged" \
"${SUFFICIENT_DOCS}" \
"labels[]=triaged" \
"labels[]=ready-to-code"

run_test_auto_code_disabled "auto-code-disabled-docs-still-gets-documentation-label" \
"${SUFFICIENT_DOCS}" \
"labels[]=documentation"

run_test_auto_code_disabled "auto-code-disabled-perf-gets-triaged" \
"${SUFFICIENT_PERF}" \
"labels[]=triaged" \
"labels[]=ready-to-code"

# --- Summary ---

echo ""
Expand Down
32 changes: 26 additions & 6 deletions scripts/post-triage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -312,24 +312,44 @@ ${FAILED_CREATES}"
# ready-to-code, which triggers the code agent. Feature work and anything
# else receives the triaged label and waits for human prioritization
# (per #561, only feature issues should require human review before coding).
#
Comment thread
rh-hemartin marked this conversation as resolved.
# Set TRIAGE_AUTO_CODE=false to disable the automatic ready-to-code
# promotion. When disabled, low-risk categories receive "triaged" instead,
# requiring human review before the code agent runs.
TRIAGE_AUTO_CODE="${TRIAGE_AUTO_CODE:-true}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Unvalidated auto-code flag 🐞 Bug ☼ Reliability

scripts/post-triage.sh treats any TRIAGE_AUTO_CODE value other than the exact string "false" as
enabled, so typos/case variants (e.g., "FALSE", "0", "fasle") will still apply "ready-to-code" and
can unintentionally trigger the coding agent.
Agent Prompt
### Issue description
`TRIAGE_AUTO_CODE` is documented as a boolean (`true`/`false`) but is only checked via `[[ "${TRIAGE_AUTO_CODE}" == "false" ]]`. Any other value (including common misconfigurations like `FALSE`) silently behaves like `true`, potentially auto-applying `ready-to-code`.

### Issue Context
- Docs state valid values are strictly `true` and `false`.
- The post-triage script currently defaults to enabled for all non-exact-false values.

### Fix Focus Areas
- scripts/post-triage.sh[319-352]
- docs/triage.md[131-139]

### Proposed fix
1. Normalize the value (e.g., lowercase) and validate explicitly:
   - `TRIAGE_AUTO_CODE="${TRIAGE_AUTO_CODE:-true}"`
   - `TRIAGE_AUTO_CODE="${TRIAGE_AUTO_CODE,,}"`
   - `case "${TRIAGE_AUTO_CODE}" in true|false) ;; *) echo "ERROR: TRIAGE_AUTO_CODE must be true|false"; exit 1 ;; esac`
2. Keep the existing branching logic, but now it can rely on a known-good value.
3. (Optional) Mention in docs that values are case-sensitive unless normalization is implemented.

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

CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}")
echo "Category: ${CATEGORY}"
case "${CATEGORY}" in
bug)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[HIGH] Branch is stale and will conflict with the already-merged workflow-changes guard (#325) on these lines

Finding: This branch is significantly behind main (diverged, roughly 86 commits behind / 2 ahead). Current main's post-triage.sh already implements a REQUIRES_WORKFLOW/WORKFLOW_BLOCKED guard (from issue #325, "Triage should detect likely GHA workflow modifications and skip automatic code agent") that computes WORKFLOW_BLOCKED once before the case "${CATEGORY}" block and gates each of the bug/documentation/performance branches' DEFERRED_LABEL="ready-to-code" on [[ "${WORKFLOW_BLOCKED}" != "true" ]]. This PR's branch predates that change and instead adds its own per-branch if/else on TRIAGE_AUTO_CODE in the same three case arms, rewriting the same comment block above the case statement. Both branches also independently edited the same docs/code.md ready-to-code table row. Rebasing/merging will produce real textual conflicts in both files, and a naive per-hunk resolution (accepting only one side) would silently drop one of the two safety gates — neither test suite exercises the other's condition.

Suggestion: Rebase onto current main before merging. Reconcile by computing one combined boolean ahead of the case statement (e.g. AUTO_CODE_ALLOWED=true; [[ "${WORKFLOW_BLOCKED}" == "true" ]] && AUTO_CODE_ALLOWED=false; [[ "${TRIAGE_AUTO_CODE}" == "false" ]] && AUTO_CODE_ALLOWED=false), gate DEFERRED_LABEL="ready-to-code" on it in each branch, and update docs/code.md's ready-to-code row to describe both gates. Add a test combining requires_workflow_changes: true with TRIAGE_AUTO_CODE=false to confirm the merged logic resolves to triaged exactly once.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] Identical TRIAGE_AUTO_CODE if/else block duplicated across three category branches

Finding: The same six-line block (if [[ "${TRIAGE_AUTO_CODE}" == "false" ]]; then ... add_label "triaged" ... else ... DEFERRED_LABEL="ready-to-code" ... fi) is repeated verbatim at the bug, documentation, and performance case arms (lines 323, 334, 345), differing only in the category name interpolated into the echo strings. Standing alone this is a simplification nit, but it compounds the stale-branch risk noted in the other comment on this line: main has already added a second, structurally different triplicated guard (WORKFLOW_BLOCKED, from #325) across the identical three branches, so reconciling the two features means hand-editing 3+3 near-identical branches — exactly the setup where a fix applied to one copy and missed in the other two would silently produce inconsistent outcomes between categories, uncaught by either test suite.

Suggestion: Extract a helper, e.g. apply_ready_to_code_or_triaged "${CATEGORY}", containing the shared branch logic, and call it once per case arm — ideally as part of the main-rebase reconciliation so the merged guard logic lives in one place.

echo "Applying bug label..."
add_label "bug"
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
if [[ "${TRIAGE_AUTO_CODE}" == "false" ]]; then
echo "TRIAGE_AUTO_CODE=false — applying triaged label instead of ready-to-code (${CATEGORY})"
add_label "triaged"
else
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
fi
;;
documentation)
echo "Applying documentation label..."
add_label "documentation"
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
if [[ "${TRIAGE_AUTO_CODE}" == "false" ]]; then
echo "TRIAGE_AUTO_CODE=false — applying triaged label instead of ready-to-code (${CATEGORY})"
add_label "triaged"
else
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
fi
;;
performance)
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
if [[ "${TRIAGE_AUTO_CODE}" == "false" ]]; then
echo "TRIAGE_AUTO_CODE=false — applying triaged label instead of ready-to-code (${CATEGORY})"
add_label "triaged"
else
echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..."
DEFERRED_LABEL="ready-to-code"
fi
;;
feature)
echo "Applying feature + triaged labels..."
Expand Down
Loading