feat: add generic refine agent#86
Conversation
Add a platform-agnostic refinement agent that decomposes work items into implementable children with acceptance criteria. Includes harness config, sandbox policy (read-only GitHub API), pre/post scripts, result schema, platform context docs (Jira/GitHub/GitLab), and test suite (29 tests). Depends on shared scripts from PR #11 (explore agent) — comment-helpers.sh, pre-explore.sh, and markdown-to-adf.py. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 5:46 PM UTC · Completed 5:57 PM UTC |
PR Summary by QodoAdd platform-agnostic refine agent with harness, policy, schema, and scripts
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1.
|
Review — Refine Agent (Re-review)PR: #86 · Author: ascerra · Base: main · Status: open (not draft) Changes since prior reviewThe fix commit (
6 findings from the prior review remain unaddressed. Findings🟡 Medium[secrets-handling]
Remediation: Mask at the top of the script: [missing-dependency]
Remediation: Coordinate merge ordering so PR #11 merges first. 🔵 Low[gha-command-injection]
Remediation: Compute [gha-command-injection]
Remediation: Use [test-coverage] The 29 tests cover core decision logic well but miss: (a) GitHub fallback routing where Remediation: Add tests for the GitHub fallback routing and data-sources footer construction at minimum. [logic-inconsistency] The script uses three different patterns to detect GitHub mode: (1) Remediation: Use Notes
Previous runReview — Refine Agent (Re-review)PR: #86 · Author: ascerra · Base: main · Status: open (not draft) The fix commit (
Initial reviewInitial review found 11 findings: 2 high (schema missing additionalProperties, missing dependencies on PR #11), 6 medium (label removal, Makefile, GHA injection, secrets masking, stale icon, disallowedTools bypass), 2 low (dead code, test coverage). Labels: PR adds a new agent with feature scope Previous runReview — Refine Agent (Re-review)PR: #86 · Author: ascerra · Base: main · Status: open (not draft) Changes since prior reviewThe fix commit (
7 findings from the prior review remain unaddressed. 5 new findings were identified. Findings🔴 High[schema-correctness] Two related issues: (1) The schema omits Remediation: Add [missing-dependency]
Remediation: Coordinate merge ordering so PR #11 merges first. Document the dependency in the PR description (already partially done). 🟡 Medium[missing-validation] Every other agent prompt in the repo ( Remediation: Add [logic-error] The script removes Remediation: Add [test-infrastructure] The Remediation: Add [secrets-handling]
Remediation: Mask at the top of the script: 🔵 Low[gha-command-injection] The Remediation: Sanitize [gha-command-injection]
Remediation: Use [schema-correctness] The Remediation: Add [test-coverage] The 29 tests cover core decision logic but miss: (a) GitHub fallback routing where Remediation: Add tests for the GitHub fallback routing and data-sources footer construction at minimum. [logic-inconsistency] The script uses three different patterns to detect GitHub mode: (1) Remediation: Use Notes
Previous run (2)Review — Refine AgentPR: #86 · Author: ascerra · Base: main · Status: open (not draft) SummaryThis PR adds a well-structured refine agent with comprehensive prompt design, platform-specific context docs, harness configuration, sandbox policy, output schema, pre/post scripts, and a 29-test suite. The overall architecture follows existing patterns closely. However, there are several issues that need resolution before merge. Findings🔴 High[schema-correctness] Every other schema in this repository ( Remediation: Add [missing-dependency] Line 4 of the script sources Remediation: Merge PR #11 first, or coordinate atomic merge. Document the required merge order in the PR description. [missing-dependency] Line 12 calls Remediation: Same as above — coordinate merge ordering with PR #11. 🟡 Medium[logic-error] The script removes Remediation: Add [test-infrastructure] The Remediation: Add [gha-command-injection] Line 278 emits Remediation: Sanitize [secrets-handling]
Remediation: Compute tokens once at the top of the script and mask immediately: [docs-staleness] Line 3 references Remediation: Either add [disallowedTools-bypass] The Remediation: Consider blocking 🔵 Low[dead-code] The Remediation: Either remove the [test-coverage] The 29 tests cover core decision logic well but miss: (a) the Remediation: Add tests for the routing fallback case and the data-sources footer construction at minimum. Notes
|
- Block all gh api calls in sandbox (match fix agent pattern) - Remove FULLSEND_OUTPUT_FILE override and GH_TOKEN from sandbox - Guard comment-helpers.sh dependency with clear PR #11 message - Sanitize GHA workflow command output to prevent injection - Check label mutation success before logging success notices - Document proposed_description body updates in user docs - Remove missing icon reference from docs Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 6:12 PM UTC · Completed 6:23 PM UTC |
- Fix SC2034/SC2129 shellcheck failures - Upgrade schema to draft/2020-12 with additionalProperties: false - Add fullsend-check-output after Phase 6 - Remove ready-to-refine trigger label after completion (GitHub + Jira) - Register post-refine-test.sh in Makefile script-test target Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 7:57 PM UTC · Completed 8:10 PM UTC |
Superseded by updated review
| echo "::notice::Added label 'ready-to-critique' to GitHub issue #${SAFE_GH_NUM}" | ||
| else | ||
| echo "::warning::Failed to add critique labels to GitHub issue #${GITHUB_ISSUE_NUMBER}" | ||
| fi |
There was a problem hiding this comment.
[low] gha-command-injection
sanitize_gha is applied on success-path ::notice:: commands but raw unsanitized GITHUB_ISSUE_NUMBER, ISSUE_KEY, and REVIEW_ROUND are used in ::warning:: commands on error paths.
Suggested fix: Compute SAFE_* variables before the if/else block so both branches use sanitized values.
| echo "PLATFORM_CONTEXT=$WORKSPACE/platform-context.md" >> "${GITHUB_ENV:-/dev/null}" | ||
| echo "::notice::Platform context loaded: ${ISSUE_SOURCE}" | ||
| else | ||
| echo "::warning::No platform context template found for source=${ISSUE_SOURCE}" |
There was a problem hiding this comment.
[low] gha-command-injection
ISSUE_SOURCE is sanitized as SAFE_ISSUE_SOURCE at line 37 but raw ISSUE_SOURCE is used in ::warning:: output at line 282. One-character fix — safe variable is already in scope.
Suggested fix: Use SAFE_ISSUE_SOURCE in the warning output.
There was a problem hiding this comment.
Same deal as the other PR, move this to the harness
rh-hemartin
left a comment
There was a problem hiding this comment.
I want to move some envs to the harness file.
Summary
comment-helpers.sh,pre-explore.sh, andmarkdown-to-adf.pyRelated Issue
Continuation of agent migration from
konflux-ci/refinementto generic agents repo.Changes
agents/refine.mdharness/refine.yamlenv.runner/env.sandboxformat, human-directive mountpolicies/refine.yamlschemas/refine-result.schema.jsonscripts/pre-refine.shscripts/post-refine.shscripts/post-refine-test.shscripts/platform-{jira,github,gitlab}.mdenv/refine.envdocs/refine.mdconfig.yamlharness/refine.yamlREADME.mdTesting
bash scripts/post-refine-test.sh— 29/29 tests passingChecklist
PUTverb blocked indisallowedToolsenv.runner/env.sandboxformatMade with Cursor