fix(konflux-build-triage): fix dispatcher dedup, filtering, and throttling - #29
fix(konflux-build-triage): fix dispatcher dedup, filtering, and throttling#29quay-devel wants to merge 1 commit into
Conversation
…tling Five shortcomings identified after the first live triage run: 1. Deduplication used session_name which ACP ignores (generates UUID names). Switch to displayName-based matching via acp_list_sessions(search="Fix:"). 2. Stale/EOL versions were triaged unnecessarily. Add SUPPORTED_VERSIONS allowlist and MAX_FAILURE_AGE_DAYS (default 30) to filter them out. 3. FBC components with empty branch fields caused sessions to default to main and fail immediately. check-build-health.sh now skips empty-branch components with a warning in --failed-only mode; CLAUDE.md adds a branch filter step before spawning. 4. 11 sessions spawned simultaneously with no prioritization. Add MAX_SESSIONS_PER_RUN cap (default 5) and sort failures by last_build recency so the most recent breakage gets triaged first. 5. Spawned sessions appeared stuck (totalMessages=0). Add post-spawn session health check guidance and log idle session IDs for investigation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughConfiguration and documentation for the Konflux build triage dispatcher are updated with environment-driven limits ( ChangesBuild Triage Workflow Configuration and Filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workflows/konflux-build-triage/CLAUDE.md`:
- Around line 120-122: Add a language identifier to the Markdown fenced code
blocks flagged by markdownlint (MD040) so they are not treated as unspecified
fences; specifically update the fences containing the strings "SKIP {component}
— empty branch (FBC or unresolved source ref)", "SKIP {component} (app:
{application}) — not in SUPPORTED_VERSIONS ({SUPPORTED_VERSIONS})", "SKIP
{component} — last build {last_build} older than {MAX_FAILURE_AGE_DAYS} days",
and "NOTE: Run cap reached. {C} failure(s) deferred to next run." by changing
their opening ``` to ```text (or another appropriate language), ensuring each
matching closing ``` remains unchanged.
In `@workflows/konflux-build-triage/scripts/check-build-health.sh`:
- Around line 215-218: The --failed-only empty-branch filter is only applied in
the JSON branch; update check-build-health.sh so components with an empty branch
are excluded whenever FAILED_ONLY=true and build_failed=true regardless of
output mode—either move the if check (using variables FAILED_ONLY, build_failed,
branch, comp_name, app) to run before the output-mode branching, or duplicate
the same condition into the table-mode flow so FBC entries with empty branch are
skipped for all --failed-only outputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 71e9d4f3-7362-4b36-827a-3e0080b3e1c9
📒 Files selected for processing (3)
workflows/konflux-build-triage/.ambient/ambient.jsonworkflows/konflux-build-triage/CLAUDE.mdworkflows/konflux-build-triage/scripts/check-build-health.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
workflows/konflux-build-triage/scripts/check-build-health.sh
📄 CodeRabbit inference engine (workflows/konflux-build-triage/CLAUDE.md)
Use bash script 'scripts/check-build-health.sh --failed-only' to assess build health via KubeArchive REST API
In check-build-health.sh: exclude components with empty 'branch' fields (FBC components) from --failed-only output
Use KubeArchive REST API with curl and bearer token auth to query latest on-push PipelineRuns per component
Files:
workflows/konflux-build-triage/scripts/check-build-health.sh
🪛 markdownlint-cli2 (0.22.1)
workflows/konflux-build-triage/CLAUDE.md
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 127-127: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 208-208: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
workflows/konflux-build-triage/.ambient/ambient.json (1)
10-13: Env var additions look consistent with dispatcher behavior changes.The new runtime knobs align with the documented filtering/throttling flow and have sane defaults.
workflows/konflux-build-triage/CLAUDE.md (1)
44-69: Dispatcher flow updates are clear and operationally stronger.The displayName-based dedup plus pre-spawn filtering and run-cap ordering materially improve determinism and reduce noisy triage.
Also applies to: 114-164, 198-210
| ``` | ||
| SKIP {component} — empty branch (FBC or unresolved source ref) | ||
| ``` |
There was a problem hiding this comment.
Add languages to fenced code blocks to satisfy markdownlint (MD040).
These new fences are missing a language identifier, which is currently flagged by static analysis.
Suggested fix
-```
+```text
SKIP {component} — empty branch (FBC or unresolved source ref)- +text
SKIP {component} (app: {application}) — not in SUPPORTED_VERSIONS ({SUPPORTED_VERSIONS})
-```
+```text
SKIP {component} — last build {last_build} older than {MAX_FAILURE_AGE_DAYS} days
- +text
NOTE: Run cap reached. {C} failure(s) deferred to next run.
Also applies to: 127-129, 134-136, 208-210
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflows/konflux-build-triage/CLAUDE.md` around lines 120 - 122, Add a
language identifier to the Markdown fenced code blocks flagged by markdownlint
(MD040) so they are not treated as unspecified fences; specifically update the
fences containing the strings "SKIP {component} — empty branch (FBC or
unresolved source ref)", "SKIP {component} (app: {application}) — not in
SUPPORTED_VERSIONS ({SUPPORTED_VERSIONS})", "SKIP {component} — last build
{last_build} older than {MAX_FAILURE_AGE_DAYS} days", and "NOTE: Run cap
reached. {C} failure(s) deferred to next run." by changing their opening ``` to
```text (or another appropriate language), ensuring each matching closing ```
remains unchanged.
| if [[ "$FAILED_ONLY" == "true" && "$build_failed" == "true" && -z "$branch" ]]; then | ||
| echo "WARNING: Skipping ${comp_name} (app: ${app}) — empty branch, cannot spawn fix session" >&2 | ||
| continue | ||
| fi |
There was a problem hiding this comment.
Empty-branch filter is only applied in JSON mode, not all --failed-only output.
This condition is scoped to the JSON branch, so --failed-only --table still emits FBC entries with empty branch. Please apply the same skip in table mode (or centralize filtering before output-mode branching).
Suggested fix
@@
while IFS=$'\t' read -r comp_name app source branch; do
@@
if [[ "$FAILED_ONLY" == "true" && "$build_failed" != "true" ]]; then
continue
fi
+
+ if [[ "$FAILED_ONLY" == "true" && "$build_failed" == "true" && -z "$branch" ]]; then
+ echo "WARNING: Skipping ${comp_name} (app: ${app}) — empty branch, cannot spawn fix session" >&2
+ continue
+ fi
@@
done < "$TMPDIR/components.tsv" | sort -t$'\t' -k1,1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflows/konflux-build-triage/scripts/check-build-health.sh` around lines
215 - 218, The --failed-only empty-branch filter is only applied in the JSON
branch; update check-build-health.sh so components with an empty branch are
excluded whenever FAILED_ONLY=true and build_failed=true regardless of output
mode—either move the if check (using variables FAILED_ONLY, build_failed,
branch, comp_name, app) to run before the output-mode branching, or duplicate
the same condition into the table-mode flow so FBC entries with empty branch are
skipped for all --failed-only outputs.
Summary
Five shortcomings found after the first live triage run are addressed here:
session_nameis ignored by ACP (generates UUID names). Switched todisplayName-based matching withacp_list_sessions(search="Fix:")— both dedup and triage-cap checks now key offdisplayName.SUPPORTED_VERSIONSallowlist andMAX_FAILURE_AGE_DAYS=30env var. A new Step 2.5 filters failures before spawning, with per-entry logging.check-build-health.shnow skips and warns on components with empty branch in--failed-onlymode. CLAUDE.md adds a branch filter as the first pre-spawn check.MAX_SESSIONS_PER_RUN=5cap. Failures are sorted bylast_buildrecency before the loop so the cap favors recent breakage.Files changed
workflows/konflux-build-triage/CLAUDE.mdworkflows/konflux-build-triage/.ambient/ambient.jsonMAX_SESSIONS_PER_RUN,SUPPORTED_VERSIONS,MAX_FAILURE_AGE_DAYSenv varsworkflows/konflux-build-triage/scripts/check-build-health.sh--failed-onlyJSON pathTest plan
SUPPORTED_VERSIONS=v3-18and confirm only matching applications are spawned🤖 Generated with Claude Code