feat(#1754): add TRIAGE_AUTO_CODE configuration for ready-to-code - #567
Conversation
PR Summary by QodoAdd TRIAGE_AUTO_CODE to control ready-to-code auto-promotion in triage
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
|
🤖 Review · |
Code Review by Qodo
1. Protected harness/scripts changes
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
908eaeb to
5ac6c31
Compare
|
🤖 Finished Review · ✅ Success · Started 4:23 PM UTC · Completed 4:37 PM UTC |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Next steps:
Previous run (5)ReviewFindingsMedium
Low
Next steps:
Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)ReviewFindingsMedium
Low
Previous run (9)ReviewFindingsMedium
Low
Previous run (10)ReviewFindingsMedium
Low
Previous run (11)ReviewFindingsMedium
Low
Previous run (12)ReviewFindingsMedium
Low
Labels: PR modifies the triage agent post-script and harness configuration. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 4:40 PM UTC · Completed 4:57 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Additional review findings (verified against head commit 780c846, deduplicated against existing threads).
|
🤖 Finished Review · ✅ Success · Started 5:28 PM UTC · Completed 5:44 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 6:05 PM UTC · Completed 6:24 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 6:40 PM UTC · Completed 6:57 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Review · |
Add TRIAGE_AUTO_CODE env var to control whether triage auto-applies the ready-to-code label (which triggers the code agent). Three modes: - on (default): auto-promote bug/documentation/performance — preserves existing behavior. - off: never auto-promote; always apply triaged instead, requiring human review before coding starts. - category: auto-promote only categories listed in TRIAGE_AUTO_CODE_CATEGORIES (comma-separated, default: bug,documentation,performance). Closes fullsend-ai/fullsend#1754 Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Suppress SC2163 shellcheck warning in post-triage-test.sh where
export "${kv}" is intentionally used to export KEY=VALUE pairs from
word-split strings.
Remove ${VAR:-default} syntax from harness/triage.yaml forge env vars —
the fullsend Go binary does not support shell-style defaults in YAML
env expansion. The post-triage.sh script already handles defaults
internally.
Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Strip whitespace from the comma-separated categories string before matching, so "bug, documentation" works the same as "bug,documentation". Add a test case for whitespace tolerance. Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Normalize TRIAGE_AUTO_CODE to lowercase for case-insensitive matching - Emit (#325) workflow-change warning independently of auto_code_allowed so the operational signal is always visible when REQUIRES_WORKFLOW is true - Fix run_test_with_env to use newline-delimited parsing so values with spaces (like "bug, documentation, performance") survive intact - Switch whitespace-tolerant test to use documentation fixture to actually exercise multi-item category matching - Reorder docs table to match Variable | Description | Default | Valid values convention from review.md and code.md Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
The harness file is the single source of truth for env var defaults.
Move TRIAGE_AUTO_CODE and TRIAGE_AUTO_CODE_CATEGORIES from post-script
shell defaults (:-on, :-bug,...) to top-level env.runner literals in
harness/triage.yaml. End users override via base-derived harness
configs instead of workflow env vars.
This also fixes the functional test failures — fullsend validates all
forge.github.env.runner ${VAR} refs against the host environment, so
the previous approach required every caller (eval, production workflow,
end users) to have these vars pre-set even though they're optional
with defaults.
Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Add :- defaults to TRIAGE_AUTO_CODE and TRIAGE_AUTO_CODE_CATEGORIES so the script doesn't crash under set -euo pipefail when the env vars are unset (e.g. running outside the harness). Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
a0ab900 to
896f6d8
Compare
|
🤖 Review · |
|
🤖 Finished Review · ✅ Success · Started 7:22 PM UTC · Completed 7:38 PM UTC |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review-only sweep — 1 new finding (ADR 0080 merged since the last sweep and now requires override docs that docs/triage.md still lacks).
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Approving — no functional blockers. Outstanding item: docs/triage.md still needs the per-repo/org override instructions ADR 0080 mandates for this exact PR (merged 2026-08-03, cites this PR by name). Small, doc-only gap — trusting this gets closed before merge. Two other pre-existing MEDIUM notes (harness env-var pass-through convention, TRIAGE_AUTO_CODE_CATEGORIES doc wording) are non-blocking style/doc items.
|
I created a very similar PR a couple weeks ago: #331. I've just closed it as a duplicate of this PR but one nit: the code agent docs (docs/code.md) include what conditions/categories the triage agent will auto-triage and this will make it incorrect. That doc should be cleaned up, preferably but removing unnecessary details of the triage agent. However, the main reason I didn't push on my PR is that I started to get uncomfortable with having a triage-specific way to disable triggering the code agent. First, it's kind of backwards (we want coder to not trigger automatically, not triage to behave differently) and architecturally odd (triage agent shouldn't know details of code agent or care if it's human), but mostly it's because I think every agent should have an 'installed but not triggered automatically' mode. i.e. triage, review, or retro only happens when a human asks. Some of these may be unlikely to be used (who really cares if triage adds a comment?) but I've been leaning towards a platform-level agent setting or CEL for 'only trigger on human request' being a better way to handle this. It would provide a simple, consistent way to handle this without complexity in any agent, plus optional softer onboarding for all agents. (TRIAGE_AUTO_CODE_CATEGORIES should still be here as triage config. One could argue whether it should be the triage agent deciding when the coding agent has enough details to do well vs code agent deciding what it can fix, but I think the time/$$/simplicity means triage will always be the one to decide) |
|
Yeah, CEL expressions are probably the way for most of your "act on labels or only act on explicit commands" configuration. There's an epic tracking this: fullsend-ai/fullsend#2888 (per-repo harness CEL dispatch) — moving each default agent's trigger logic off the bash router and onto CEL expressions one at a time. |
"category" and "on" were already 100% identical in auto_code_allowed() after the in-script default list was dropped -- both just call category_in_auto_code_list(). Drop the redundant enum value and update docs/triage.md to describe the actual two-value behavior, plus add the override-path note ADR 0080 requires for this variable. Addresses PR review feedback. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 6:42 PM UTC · Completed 6:59 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 7:04 PM UTC · Completed 7:14 PM UTC |
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review findings below (inline comments cover 3 of them; one more that doesn't anchor to a specific line is here in the summary).
[MEDIUM] PR description still advertises a third "category" mode that this PR's own final commit removed
The PR description currently reads: "Three modes: on (default, preserves existing behavior), off (never auto-promote), category (auto-promote only listed categories via TRIAGE_AUTO_CODE_CATEGORIES)." The current head commit collapsed the category enum value into on (docs/triage.md's Variables table now lists only on/off as valid values, and on's description already reads as category-gated auto-promotion). The description was never updated to match, so it now describes a design that contradicts the shipped implementation as of the commit under review. This is separate from the earlier resolved thread about the "closes #1754" auto-close scoping claim — that thread never touched this "three modes" text.
Suggestion: update the description to describe the final two-value design (on/off, with on scoped by TRIAGE_AUTO_CODE_CATEGORIES) before merge, since this text is the durable record of how #1754 was resolved.
| # the new action. Every terminal action below resets its own set of control | ||
| # labels, but "triaged" is only ever re-applied (never removed) by the | ||
| # handlers themselves, so it must be cleared up front rather than per-branch. | ||
| remove_label "triaged" |
There was a problem hiding this comment.
[HIGH] remove_label("triaged") runs before per-action input validation, mutating GitHub state on failing runs
remove_label "triaged" (line 122) executes unconditionally immediately before case "${ACTION}" in (line 124) — i.e. before every branch's own validation, e.g. insufficient's [[ -z "${COMMENT}" ]] guard (lines 126-129), and before the final *) echo "ERROR: unknown action..."; exit 1 fallback (lines 522-524) for a malformed/unrecognized ACTION.
remove_label() issues a live gh api ... -X DELETE against the issue's labels with errors swallowed (2>/dev/null || true), so this is a real, unguarded side effect. Every other mutation in this script follows validate-then-mutate (each terminal action checks its required fields before calling add_label/remove_label). This new call breaks that invariant: a malformed agent-result.json or a not-yet-handled future ACTION value will now strip the triaged label from the live issue before the script aborts, leaving the issue with no control label and no explanatory comment despite the run being reported as failed.
This ordering concern wasn't raised in the earlier thread that introduced this single pre-dispatch call — that thread fixed a coverage gap (stale label only cleared on sufficient) but didn't flag the new validation-ordering regression.
Suggestion: move the remove_label "triaged" call inside each terminal action's block (after that action's own validation/guard, alongside its other remove_label calls), or gate it behind a check that ACTION is a known, already-validated terminal action, so no label mutation happens before a validation failure short-circuits the script.
| # the new action. Every terminal action below resets its own set of control | ||
| # labels, but "triaged" is only ever re-applied (never removed) by the | ||
| # handlers themselves, so it must be cleared up front rather than per-branch. | ||
| remove_label "triaged" |
There was a problem hiding this comment.
[MEDIUM] Stale-"triaged"-label removal is test-covered for only 2 of 7 action branches
The case "${ACTION}" in dispatch has 7 branches (insufficient, duplicate, prerequisites, in-progress, sufficient, question, not-planned). The comment above this shared remove_label "triaged" call states it must fire for every terminal action, but scripts/post-triage-test.sh only has run_test "insufficient-clears-stale-triaged-label" and run_test "sufficient-clears-stale-triaged-label" — duplicate, prerequisites, in-progress, question, and not-planned have zero regression coverage for this DELETE call.
Because the fix is currently a single shared call, the immediate risk is low, but there's no test to catch a future refactor (e.g. reintroducing per-branch logic, floated elsewhere in review as a follow-up for the auto-code triplication) from silently dropping the removal on 5 of the 7 branches.
Suggestion: add one run_test "<action>-clears-stale-triaged-label" case per remaining branch (duplicate, prerequisites, in-progress, question, not-planned), mirroring the existing insufficient/sufficient tests in scripts/post-triage-test.sh.
| | `TRIAGE_AUTO_CODE` | Controls whether triage auto-applies `ready-to-code`. `on` — auto-promote categories listed in `TRIAGE_AUTO_CODE_CATEGORIES`. `off` — never auto-promote; always apply `triaged`. | `on` | `on`, `off` | | ||
| | `TRIAGE_AUTO_CODE_CATEGORIES` | Comma-separated list of categories to auto-promote when `TRIAGE_AUTO_CODE=on`. | `bug,documentation,performance` | `bug`, `documentation`, `performance` | | ||
|
|
||
| To override these defaults per repo or org, create a custom harness for the |
There was a problem hiding this comment.
[MEDIUM] New override guidance doesn't match the SHA-pinned base: pattern docs/code.md actually requires, and a bare relative path won't resolve cross-repo
This new paragraph tells operators to override the two new variables by creating .fullsend/triage.yaml with "a base: pointing at harness/triage.yaml" — a repo-relative markdown link, with no YAML example. But docs/code.md's own "How to configure" section requires base: to be a full SHA-pinned URL with an integrity hash — base: https://raw.githubusercontent.com/fullsend-ai/agents/<SHA>/harness/code.yaml#sha256=<sha256sum> — obtained via a documented curl/sha256sum recipe, not a bare relative path.
Checking fullsend's harness composition (loadBaseChain), a non-https:// base value is resolved relative to the child harness file's own directory on disk. That only works when the overriding .fullsend/triage.yaml lives in the same checkout as harness/triage.yaml (local/monorepo composition). An external org's .fullsend/triage.yaml lives in a different repository, where no harness/triage.yaml file exists, so the guidance as literally written wouldn't resolve there.
This substance hasn't been confirmed anywhere in the thread — the "Does that cover it?" question after this text was pushed has no reply yet.
Suggestion: give a concrete YAML example mirroring code.md's SHA-pinned base: URL (with the same curl/sha256sum snippet, substituted for triage.yaml), and drop or clarify the bare relative-path phrasing so it isn't read as the literal value to place in base:.
Retro: PR #567 — TRIAGE_AUTO_CODE configurationTimeline: PR #567, authored by ralphbean, added Key findingsReview quality gap — human reviewers significantly outperformed the review agent on this PR. The review agent's correctness sub-agent missed the PR's highest-severity bugs, all caught by human reviewer waynesun09:
Meanwhile, the review agent spent significant token budget repeating the Human reviewer maruiz93 raised architectural concerns about the config override mechanism (CI env blocks vs harness composition) that led to ADRs 0080/0081 — a class of design-level feedback outside the review agent's current capability. Token cost — 13 full review runs on a single PR is excessive. Most runs produced near-identical findings on unchanged code. The agent does have re-review narrowing (severity anchoring, dimension skipping), but it still dispatched full review cycles on every push. Evidence for existing open issues
Proposals filedOne proposal targeting the review agent's correctness sub-agent test integrity checks — a novel gap not covered by existing issues. Proposals filed
|
remove_label "triaged" previously only ran on the sufficient branch, so an issue that acquired triaged under a prior TRIAGE_AUTO_CODE=off run kept it alongside needs-info/duplicate/blocked/question/pr-open/ not-planned on any later re-triage. Move the reset to a single point before the action dispatch so every terminal action gets the same cleanup. Also add coverage for the TRIAGE_AUTO_CODE=category default-category fallback with TRIAGE_AUTO_CODE_CATEGORIES genuinely unset — the existing "default" test inherited the harness-level export instead of exercising the script's own bash default. Addresses review feedback on #567. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…efault The bug/documentation/performance allowlist was independently expressed in three places inside post-triage.sh (the "on" branch, the "*" fallback branch, and the "category" branch's own env-var default) plus docs/triage.md and harness/triage.yaml. Rather than extract a shared constant, remove the script-level default entirely: "on" and "category" now both read the list from TRIAGE_AUTO_CODE_CATEGORIES via a single category_in_auto_code_list() helper, with an absent/unset var treated as an empty allowlist. The "bug,documentation,performance" default now lives only in harness/triage.yaml (which always sets it) and docs/triage.md. Updates test coverage accordingly: the tests that previously locked in the script's own default now assert the var is required. Addresses review feedback on #567. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
remove_label "triaged" previously only ran on the sufficient branch, so an issue that acquired triaged under a prior TRIAGE_AUTO_CODE=off run kept it alongside needs-info/duplicate/blocked/question/pr-open/ not-planned on any later re-triage. Move the reset to a single point before the action dispatch so every terminal action gets the same cleanup. Also add coverage for the TRIAGE_AUTO_CODE=category default-category fallback with TRIAGE_AUTO_CODE_CATEGORIES genuinely unset — the existing "default" test inherited the harness-level export instead of exercising the script's own bash default. Addresses review feedback on fullsend-ai#567. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…efault The bug/documentation/performance allowlist was independently expressed in three places inside post-triage.sh (the "on" branch, the "*" fallback branch, and the "category" branch's own env-var default) plus docs/triage.md and harness/triage.yaml. Rather than extract a shared constant, remove the script-level default entirely: "on" and "category" now both read the list from TRIAGE_AUTO_CODE_CATEGORIES via a single category_in_auto_code_list() helper, with an absent/unset var treated as an empty allowlist. The "bug,documentation,performance" default now lives only in harness/triage.yaml (which always sets it) and docs/triage.md. Updates test coverage accordingly: the tests that previously locked in the script's own default now assert the var is required. Addresses review feedback on fullsend-ai#567. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
4 participants
Summary
TRIAGE_AUTO_CODEenv var to control whether triage auto-appliesready-to-code(which triggers the code agent). Three modes:on(default, preserves existing behavior),off(never auto-promote),category(auto-promote only listed categories viaTRIAGE_AUTO_CODE_CATEGORIES).harness/triage.yamlunderenv.runner.docs/triage.md.This delivers the category-allowlist slice of fullsend-ai/fullsend#1754. Clarity-score and label-based conditional criteria are deferred to a follow-up.
Relates to fullsend-ai/fullsend#1754
Test plan
make test)make test)post-triage.shTRIAGE_AUTO_CODE=offagainstappdumpster/test-repo#40(bug category) — confirmed it receivedbug+triagedinstead ofbug+ready-to-code🤖 Generated with Claude Code