feat: triage variable to control auto-coding - #331
Conversation
Adds a new variable to the triage agent that allows you to control whether it automatically triggers the coding agent. With the variable set to true (default) you get the current behaviour: the issue is labelled as `ready-to-code`, which triggers the coding agent automatically. When the variable is set to false, issues will be marked with `triaged` instead, and a human (or alternate agent) can decide when to add the `ready-to-code` label or `/fs-code` comment. Fixes #1750. Signed-off-by: Tim deBoer <git@tdeboer.ca>
Functional tests did not runThe
|
PR Summary by QodoAdd TRIAGE_AUTO_CODE flag to gate ready-to-code promotion
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. Protected paths modified
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| # 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}" |
There was a problem hiding this comment.
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
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 1:20 PM UTC · Completed 1:35 PM UTC |
ReviewFindingsMedium
Low
Labels: PR modifies triage agent configuration and post-script behavior. |
fullsend-ai-review
Bot
added
requires-manual-review
The review agent flagged docs/code.md as an issue because it says the 'ready-to-code' label was "unconditionally applied" and this PR changes that. That's not really an issue b/c the triage agent already applied the label only in certain cases, but it does highlight that the code agent docs contain unnecessary details of triage (which will require changes when triage changes; agents and their docs should be self-sufficient) and internal details (post-script). This just simplifies to clean that up and only contain what the user needs to know. Signed-off-by: Tim deBoer <git@tdeboer.ca>
|
The [stale-doc] above wasn't accurate, but it did highlight that the code agent docs had an overly detailed and unnecessary reference to triage agent. Second commit added to fix that. |
rh-hemartin
left a comment
rh-hemartin
left a comment
There was a problem hiding this comment.
LGTM, but you need to resolve the last Qodo comment
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review findings from a review sweep (severity-ordered). Two of the six findings don't have a diff-anchored line available for an inline comment, so they're included here directly; the other four are posted as inline comments on this review.
[HIGH] "Fixes fullsend-ai/fullsend#1750" is incorrect and will auto-close an unrelated issue (PR description)
Finding: fullsend-ai/fullsend#1750 ("Triage agent off by default?", still open) is a per-repo dispatch-routing bug: the triage bot's own root-cause comment says the per-repo shim workflow pins reusable-dispatch.yml@v0, a tag cut before the issues/opened -> triage routing rule existed on main, and recommends advancing the v0 tag or cutting a release. A 2026-07-02 "Meeting update" comment records the resolution as keeping the triage agent default-on and documenting that behavior clearly — it never mentions a code-agent opt-out. Separately, the underlying version-skew defect appears to already be fixed independently: fullsend-ai/fullsend#2884 ("Implement ADR 62: inline stage workflows into reusable-dispatch.yml") is closed, and the current reusable-dispatch.yml on main cites "ADR 62" and describes exactly this @v0 skew problem. This PR's diff (docs/code.md, docs/triage.md, env/triage.env, harness/triage.yaml, scripts/post-triage.sh, scripts/post-triage-test.sh) touches none of the dispatch/routing files. Merging with the "Fixes" keyword will auto-close #1750 via GitHub's cross-repo closing mechanic even though this PR neither implements nor references its actual resolution.
Suggestion: Change "Fixes" to "Relates to" (or remove the reference), and separately flag to a maintainer that #1750 looks already resolved by ADR-62/#2884 and should be closed on its own with the correct reference.
[MEDIUM] docs/triage.md's Control labels table (line 40) was not updated for the new TRIAGE_AUTO_CODE gating (docs/triage.md:40, outside this PR's diff hunk)
Finding: The ready-to-code row (docs/triage.md line 40) still reads "...is fully specified and low-risk (bug, documentation, performance)... Triggers the code agent" with no mention of the new gate, and the triaged row (line 41) still says it applies only when the issue "is a feature or other category that requires human prioritization" — no longer accurate, since bug/documentation/performance now also land on triaged whenever TRIAGE_AUTO_CODE=false. The new "### Variables" section this PR adds further down (around lines 131-139) documents the gate, but the table above it — the first, more prominent reference a reader consults — isn't updated or cross-linked, so two sections of the same doc now disagree.
Suggestion: Update both rows to note the TRIAGE_AUTO_CODE condition and link to the Variables section, e.g. "...unless TRIAGE_AUTO_CODE=false (see Variables below), in which case triaged is applied instead."
| CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}") | ||
| echo "Category: ${CATEGORY}" | ||
| case "${CATEGORY}" in | ||
| bug) |
There was a problem hiding this comment.
[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.
| CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}") | ||
| echo "Category: ${CATEGORY}" | ||
| case "${CATEGORY}" in | ||
| bug) |
There was a problem hiding this comment.
[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.
|
|
||
| | 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` | |
There was a problem hiding this comment.
[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.
| @@ -1,2 +1,3 @@ | |||
| export GITHUB_ISSUE_URL="${GITHUB_ISSUE_URL}" | |||
| export GH_TOKEN=${GH_TOKEN} | |||
| export TRIAGE_AUTO_CODE="${TRIAGE_AUTO_CODE:-true}" | |||
There was a problem hiding this comment.
[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.
3 participants
Adds a new variable to the triage agent that allows you to control whether it automatically triggers the coding agent. With the variable set to true (default) you get the current behaviour: the issue is labelled as
ready-to-code, which triggers the coding agent automatically. When the variable is set to false, issues will be marked withtriagedinstead, and a human (or alternate agent) can decide when to add theready-to-codelabel or/fs-codecomment.Fixes fullsend-ai/fullsend#1750.