test(eval): add fix agent human /fs-fix functional eval#183
Conversation
|
🤖 Finished Review · ✅ Success · Started 3:18 PM UTC · Completed 3:30 PM UTC |
PR Summary by QodoAdd code/fix functional eval suites with PR-head checkout and capture
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Looks good to me Previous runReview — approvePR: #183 — Re-review deltaPrior review (at AnalysisCorrectness: No issues found. All five judges (new_commit, expected_files, forbidden_labels, max_turns, max_cost) correctly handle null/missing data with appropriate type coercions. The Security: No issues found. Intent & coherence: All 7 changed files trace directly to issue #182's acceptance criteria. No scope creep detected. The Style/conventions: Code follows all established conventions: Documentation: No staleness found. The eval system has no existing documentation (pre-existing gap across all eval agents, not attributable to this PR). Internal env vars (HUMAN_INSTRUCTION, PRE_AGENT_HEAD, etc.) are correctly omitted from user-facing docs. Positive observations
Previous run (2)Looks good to me Previous runReview — approvePR: #183 — Re-review deltaPrior review (at
This commit replaces the inline
This is a net security improvement over the prior delete-only approach. AnalysisCorrectness: The scrub regex correctly handles all 6 GitHub token prefixes and credential URL patterns. The Security: The script is a clear improvement. The perl scrub patterns are comprehensive for GitHub-issued tokens. The Intent & coherence: The scrub script goes slightly beyond issue #182's explicit acceptance criteria but directly supports the eval infrastructure being added. The commit type Style: The script follows all established conventions: Documentation: No staleness found. The eval system has no existing documentation files that need updating. Observations1. Verification regex narrower than scrub regex —
|
| Prior finding | Severity | Resolution |
|---|---|---|
PR-head checkout applies to all pull_request fixtures |
medium | Fixed — scoped to $AGENT == "fix" && $FIXTURE_TYPE == "pull_request" |
fetch_pr_files null-safety gap |
medium | Fixed — jq expression now uses [(.files // [])[].path] |
eval/code/ additions exceed issue #182 scope |
medium | Resolved — PR base is eval/code-functional-test, so eval/code/ files are in the base, not this PR's diff |
Hardcoded HUMAN_INSTRUCTION in shared runner |
low | Fixed — instruction now lives in case input.yaml, propagated through setup-fixture.sh hook outputs |
emit_env name parameter not validated |
low | Fixed — regex validation ^[A-Za-z_][A-Za-z0-9_]*$ added |
All prior findings have been addressed.
Previous run (4)
Review — approve
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts (run-fullsend.sh, capture-fixture.sh, setup-fixture.sh) for fix-specific env, PR-head checkout, and enriched fixture state capture.
Re-review delta
Prior review (at ae8fbe0) raised 5 findings. Three commits were pushed to address them:
| Prior finding | Severity | Resolution |
|---|---|---|
PR-head checkout applies to all pull_request fixtures |
medium | Fixed — scoped to $AGENT == "fix" && $FIXTURE_TYPE == "pull_request" |
fetch_pr_files null-safety gap |
medium | Fixed — jq expression now uses [(.files // [])[].path] |
eval/code/ additions exceed issue #182 scope |
medium | Resolved — PR base is eval/code-functional-test, so eval/code/ files are in the base, not this PR's diff |
Hardcoded HUMAN_INSTRUCTION in shared runner |
low | Fixed — instruction now lives in case input.yaml, propagated through setup-fixture.sh hook outputs |
emit_env name parameter not validated |
low | Fixed — regex validation ^[A-Za-z_][A-Za-z0-9_]*$ added |
All prior findings have been addressed. The additional commits (49eb3a1, 0889e1d) also fix the branch checkout strategy (use real PR head ref instead of local alias) and add git refs API polling in capture-fixture.sh to handle headRefOid lag after pushes.
Analysis
Correctness: No issues found. The producer/consumer contract for PRE_AGENT_HEAD (env var + sidecar file fallback) is sound. Judge logic handles edge cases (missing keys, null values, type coercion). The polling loop in capture-fixture.sh (up to 21s total) is appropriate for GitHub ref propagation delays. The TRIGGER_SOURCE=eval-human value correctly triggers the human-trigger code path in the fix agent's pre/post scripts.
Security: The PR is a net security improvement. emit_env() replaces raw echo KEY=*** writes with name-format and newline/CR validation, closing a pre-existing dotenv injection surface. Input shape checks for FIXTURE_URL, FIXTURE_NUMBER, and EPHEMERAL_REPOadd defense-in-depth.HEAD_REF validation (^[A-Za-z0-9._/-]+$) is appropriate for eval-generated branch names. HUMAN_INSTRUCTIONis written to.hook-outputs.yamlviayq -iwithstrenv()`, which handles YAML special characters safely.
Intent & coherence: All 7 changed files trace directly to issue #182's acceptance criteria. The stacking on PR #177 is appropriate — after #177 merges to main, rebase and retarget. No scope creep detected.
Style: The new eval/fix/ structure follows the established pattern from eval/code/ (same base branch), including parallelism: 1, EVAL_TIMEOUT in execution env, and case directory naming (001-human-fs-fix-add).
Documentation: No staleness found. The eval system's documentation is inline in script comments and eval.yaml schemas.
Positive observations
emit_env()with name and value validation is a genuine security hardening that benefits all eval agents, not just fix.- The graceful
pr_fetch_failedfallback incapture-fixture.shensures judges fail with clear messages rather than hook aborts. - The git refs API + polling approach for
head_shacapture is a thoughtful fix for a real race condition with GitHub's PR API lag. - The
HUMAN_INSTRUCTIONlifecycle (input.yaml → setup-fixture → hook-outputs → env file → agent) keeps test content with the test case rather than hardcoded in shared scripts.
Labels: PR adds fix agent eval infrastructure and hardens shared eval scripts.
Previous run (5)
Review — comment
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts (run-fullsend.sh, capture-fixture.sh), and includes eval/code/ additions from PR #177.
Findings
1. PR-head checkout applies to all pull_request fixtures — medium
File: eval/scripts/run-fullsend.sh (line ~42)
Category: behavioral-change
The new PR-head checkout block runs unconditionally for all pull_request fixtures, not just for the fix agent:
if [[ "$FIXTURE_TYPE" == "pull_request" ]]; then
git -C "$TARGET_DIR" fetch origin "pull/${FIXTURE_NUMBER}/head:eval-pr-head"
git -C "$TARGET_DIR" checkout eval-pr-head
fiThe review eval case 001-clean-approve uses fixture.type: pull_request. Previously the clone stayed on main; now it switches to the PR branch. The comment says "Fix (and any PR-driven agent)" suggesting this is intentional, but it's an untested behavioral change for the review pipeline. If the review agent reads source files from --target-repo, it now sees PR-branch content instead of base-branch content.
Remediation: Either scope to agents that need it (if [[ "$AGENT" =~ ^(fix|code)$ ]]; then) or confirm the review eval passes with this change and add a test note documenting the intent.
2. fetch_pr_files null-safety gap — medium
File: eval/scripts/capture-fixture.sh (line ~36)
Category: error-handling
fetch_pr_files uses gh pr view ... --jq '[.files[].path]'. If gh succeeds but .files is null (possible for very recently opened PRs before GitHub computes the file list), the jq expression [.files[].path] errors on stderr (suppressed by 2>/dev/null), and gh may still exit 0. The caller would record files_fetch_failed=false with a non-array value, which is misleading — it looks like the PR has zero changed files rather than "could not determine files."
Remediation: Use a null-safe jq expression: --jq '[(.files // [])[].path]'. This produces [] when .files is null, which is a valid empty result.
3. eval/code additions exceed issue #182 scope — medium
File: eval/code/ (multiple files)
Category: scope-authorization
Issue #182 authorizes eval/fix/ with a human /fs-fix case and shared script extensions. The PR also includes the full eval/code/ directory (eval.yaml, case 001-fix-add, shared tiny-calc repo) which is authorized by issue #180 / PR #177. The PR body transparently acknowledges this: "This branch was cut from eval/code-functional-test (PR #177)... Prefer merging #177 first then rebasing, or close #177 as superseded."
Per AGENTS.md: "Every changed line in your diff must trace directly to the issue scope." The eval/code/ additions trace to #180/#177, not #182.
Remediation: Either merge #177 first and rebase this PR (removing the duplicate eval/code/ commits), or close #177 as superseded and update #182's scope to explicitly authorize landing both evals.
4. Hardcoded HUMAN_INSTRUCTION in shared runner — low
File: eval/scripts/run-fullsend.sh (line ~125)
Category: maintainability
The AGENT=fix block hardcodes the instruction text, TRIGGER_SOURCE, FIX_ITERATION, and TARGET_BRANCH specific to case 001-human-fs-fix-add. Adding a second fix case with a different instruction or trigger mode requires modifying this shared script.
Remediation: Acceptable for v1 with a single case. Consider adding a TODO comment noting the coupling. Future cases should read fix-specific env from the case's input.yaml.
5. emit_env name parameter not validated — low
File: eval/scripts/run-fullsend.sh (line ~63)
Category: defense-in-depth
emit_env() validates values for newlines/CRs but does not validate the name parameter. A name containing = or whitespace would produce a malformed env file line. All current callers use safe hardcoded literals, so this is not exploitable today.
Remediation: Add name validation: if [[ ! "$name" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then echo "ERROR: invalid env name" >&2; exit 1; fi
Positive Observations
- The
emit_env()function with newline validation is a good defense-in-depth improvement over the rawechopattern in the base code. - Input validation for
FIXTURE_URL,FIXTURE_NUMBER, andEPHEMERAL_REPOstrengthens the eval infrastructure. - The producer/consumer contract for
PRE_AGENT_HEAD(sidecar file + env fallback) is well-designed. - Judge logic in both eval.yaml files correctly handles edge cases (missing fields, null values, type coercion).
- The overall architecture (mirroring
eval/code/foreval/fix/, shared hooks/scripts) is clean and consistent with the existingeval/review/andeval/triage/patterns.
Labels: PR adds fix and code agent eval infrastructure.
Previous run (6)
Looks good to me
Previous run (7)
Review — approve
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts for fix-specific env, PR-head checkout, and enriched fixture state capture. Adds scrub-eval-results.sh for comprehensive secret redaction before artifact upload.
Re-review delta
Prior review (at 0889e1d) approved the PR with all earlier findings resolved. One commit was pushed since:
| Commit | Description |
|---|---|
8b088ff |
fix(eval): scrub ::add-mask:: tokens from functional-test artifacts |
This commit replaces the inline find ... -delete in .github/workflows/functional-tests.yml with a dedicated eval/scripts/scrub-eval-results.sh script that:
- Deletes
.eval-envfiles (preserving prior behavior) - Scrubs all 6 GitHub token prefixes (
ghp_,gho_,ghu_,ghs_,ghr_,github_pat_) andx-access-token:credential URLs from text files - Runs a fail-closed verification pass to detect surviving tokens
This is a net security improvement over the prior delete-only approach.
Analysis
Correctness: The scrub regex correctly handles all 6 GitHub token prefixes and credential URL patterns. The \S+ match for ::add-mask:: payloads is appropriate since tokens contain no whitespace. Edge cases are handled: empty directories are skipped ([[ -d "$root" ]] || continue), empty files are skipped ([[ -s "$f" ]] || return 0), and the find targets only text file extensions. The workflow change preserves the original .eval-env deletion behavior while adding the broader scrub. GHA steps are strictly sequential, so no race between scrub and upload.
Security: The script is a clear improvement. The perl scrub patterns are comprehensive for GitHub-issued tokens. The x-access-token pattern correctly handles credential URLs. The fail-closed verification pass adds defense-in-depth. One observation: the verification regex (leak_pat) covers fewer standalone patterns than the scrub regex (see finding below), but this is a defense-in-depth gap — the scrub itself is correct for all patterns.
Intent & coherence: The scrub script goes slightly beyond issue #182's explicit acceptance criteria but directly supports the eval infrastructure being added. The commit type fix(eval) correctly signals this as a bug fix (tokens leaking to artifacts). Architecturally well-placed in eval/scripts/.
Style: The script follows all established conventions: #!/usr/bin/env bash, set -euo pipefail, em-dash comment header, uppercase variable names, lowercase function names, stderr error output.
Documentation: No staleness found. The eval system has no existing documentation files that need updating.
Observations
1. Verification regex narrower than scrub regex — low
File: eval/scripts/scrub-eval-results.sh (line 50)
Category: defense-in-depth
The leak_pat verification regex has standalone \b<prefix>_[A-Za-z0-9_]{20,} patterns for ghp_, gho_, and github_pat_ only. The ghu_, ghs_, and ghr_ prefixes are only checked when preceded by ::add-mask::. Similarly, the x-access-token: pattern is not verified. The scrub regex correctly handles all patterns — this gap is only in the verification/alert layer. Practical risk is low: if a bare ghs_ token survived perl scrubbing (unlikely given the regex matches), it would not trigger the fail-closed alert, but the scrub itself would have already redacted it.
Suggested follow-up: Add \bghu_[A-Za-z0-9_]{20,}, \bghs_[A-Za-z0-9_]{20,}, and \bghr_[A-Za-z0-9_]{20,} to leak_pat for parity with the scrub regex.
Positive observations
- The script is a significant security upgrade over the prior inline
find ... -deletewhich only removed.eval-envfiles but left tokens in log/JSON output. - The fail-closed design (exit 1 on detected leaks) is the right pattern for secret scrubbing.
- GHA
::add-mask::runtime masking provides additional defense for any tokens that reach step logs. - The
--binary-files=without-matchflag and extension-based file selection prevent corruption of binary artifacts.
Labels: PR adds fix agent eval infrastructure, extends shared eval scripts, and hardens eval artifact secret scrubbing.
Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.Protected files in this PR:
.github/workflows/functional-tests.yml
Previous run (8)
Review — approve
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts (run-fullsend.sh, capture-fixture.sh, setup-fixture.sh) for fix-specific env, PR-head checkout, and enriched fixture state capture.
Re-review delta
Prior review (at ae8fbe0) raised 5 findings. Three commits were pushed to address them:
| Prior finding | Severity | Resolution |
|---|---|---|
PR-head checkout applies to all pull_request fixtures |
medium | Fixed — scoped to $AGENT == "fix" && $FIXTURE_TYPE == "pull_request" |
fetch_pr_files null-safety gap |
medium | Fixed — jq expression now uses [(.files // [])[].path] |
eval/code/ additions exceed issue #182 scope |
medium | Resolved — PR base is eval/code-functional-test, so eval/code/ files are in the base, not this PR's diff |
Hardcoded HUMAN_INSTRUCTION in shared runner |
low | Fixed — instruction now lives in case input.yaml, propagated through setup-fixture.sh hook outputs |
emit_env name parameter not validated |
low | Fixed — regex validation ^[A-Za-z_][A-Za-z0-9_]*$ added |
All prior findings have been addressed.
Previous run (9)
Review — approve
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts (run-fullsend.sh, capture-fixture.sh, setup-fixture.sh) for fix-specific env, PR-head checkout, and enriched fixture state capture.
Re-review delta
Prior review (at ae8fbe0) raised 5 findings. Three commits were pushed to address them:
| Prior finding | Severity | Resolution |
|---|---|---|
PR-head checkout applies to all pull_request fixtures |
medium | Fixed — scoped to $AGENT == "fix" && $FIXTURE_TYPE == "pull_request" |
fetch_pr_files null-safety gap |
medium | Fixed — jq expression now uses [(.files // [])[].path] |
eval/code/ additions exceed issue #182 scope |
medium | Resolved — PR base is eval/code-functional-test, so eval/code/ files are in the base, not this PR's diff |
Hardcoded HUMAN_INSTRUCTION in shared runner |
low | Fixed — instruction now lives in case input.yaml, propagated through setup-fixture.sh hook outputs |
emit_env name parameter not validated |
low | Fixed — regex validation ^[A-Za-z_][A-Za-z0-9_]*$ added |
All prior findings have been addressed. The additional commits (49eb3a1, 0889e1d) also fix the branch checkout strategy (use real PR head ref instead of local alias) and add git refs API polling in capture-fixture.sh to handle headRefOid lag after pushes.
Analysis
Correctness: No issues found. The producer/consumer contract for PRE_AGENT_HEAD (env var + sidecar file fallback) is sound. Judge logic handles edge cases (missing keys, null values, type coercion). The polling loop in capture-fixture.sh (up to 21s total) is appropriate for GitHub ref propagation delays. The TRIGGER_SOURCE=eval-human value correctly triggers the human-trigger code path in the fix agent's pre/post scripts.
Security: The PR is a net security improvement. emit_env() replaces raw echo KEY=*** writes with name-format and newline/CR validation, closing a pre-existing dotenv injection surface. Input shape checks for FIXTURE_URL, FIXTURE_NUMBER, and EPHEMERAL_REPOadd defense-in-depth.HEAD_REF validation (^[A-Za-z0-9._/-]+$) is appropriate for eval-generated branch names. HUMAN_INSTRUCTIONis written to.hook-outputs.yamlviayq -iwithstrenv()`, which handles YAML special characters safely.
Intent & coherence: All 7 changed files trace directly to issue #182's acceptance criteria. The stacking on PR #177 is appropriate — after #177 merges to main, rebase and retarget. No scope creep detected.
Style: The new eval/fix/ structure follows the established pattern from eval/code/ (same base branch), including parallelism: 1, EVAL_TIMEOUT in execution env, and case directory naming (001-human-fs-fix-add).
Documentation: No staleness found. The eval system's documentation is inline in script comments and eval.yaml schemas.
Positive observations
emit_env()with name and value validation is a genuine security hardening that benefits all eval agents, not just fix.- The graceful
pr_fetch_failedfallback incapture-fixture.shensures judges fail with clear messages rather than hook aborts. - The git refs API + polling approach for
head_shacapture is a thoughtful fix for a real race condition with GitHub's PR API lag. - The
HUMAN_INSTRUCTIONlifecycle (input.yaml → setup-fixture → hook-outputs → env file → agent) keeps test content with the test case rather than hardcoded in shared scripts.
Labels: PR adds fix agent eval infrastructure and hardens shared eval scripts.
Previous run (10)
Review — comment
PR: #183 — test(eval): add fix agent human /fs-fix functional eval
Scope: Adds eval/fix/ functional eval for the fix agent pipeline (human /fs-fix trigger), extends shared eval scripts (run-fullsend.sh, capture-fixture.sh), and includes eval/code/ additions from PR #177.
Findings
1. PR-head checkout applies to all pull_request fixtures — medium
File: eval/scripts/run-fullsend.sh (line ~42)
Category: behavioral-change
The new PR-head checkout block runs unconditionally for all pull_request fixtures, not just for the fix agent:
if [[ "$FIXTURE_TYPE" == "pull_request" ]]; then
git -C "$TARGET_DIR" fetch origin "pull/${FIXTURE_NUMBER}/head:eval-pr-head"
git -C "$TARGET_DIR" checkout eval-pr-head
fiThe review eval case 001-clean-approve uses fixture.type: pull_request. Previously the clone stayed on main; now it switches to the PR branch. The comment says "Fix (and any PR-driven agent)" suggesting this is intentional, but it's an untested behavioral change for the review pipeline. If the review agent reads source files from --target-repo, it now sees PR-branch content instead of base-branch content.
Remediation: Either scope to agents that need it (if [[ "$AGENT" =~ ^(fix|code)$ ]]; then) or confirm the review eval passes with this change and add a test note documenting the intent.
2. fetch_pr_files null-safety gap — medium
File: eval/scripts/capture-fixture.sh (line ~36)
Category: error-handling
fetch_pr_files uses gh pr view ... --jq '[.files[].path]'. If gh succeeds but .files is null (possible for very recently opened PRs before GitHub computes the file list), the jq expression [.files[].path] errors on stderr (suppressed by 2>/dev/null), and gh may still exit 0. The caller would record files_fetch_failed=false with a non-array value, which is misleading — it looks like the PR has zero changed files rather than "could not determine files."
Remediation: Use a null-safe jq expression: --jq '[(.files // [])[].path]'. This produces [] when .files is null, which is a valid empty result.
3. eval/code additions exceed issue #182 scope — medium
File: eval/code/ (multiple files)
Category: scope-authorization
Issue #182 authorizes eval/fix/ with a human /fs-fix case and shared script extensions. The PR also includes the full eval/code/ directory (eval.yaml, case 001-fix-add, shared tiny-calc repo) which is authorized by issue #180 / PR #177. The PR body transparently acknowledges this: "This branch was cut from eval/code-functional-test (PR #177)... Prefer merging #177 first then rebasing, or close #177 as superseded."
Per AGENTS.md: "Every changed line in your diff must trace directly to the issue scope." The eval/code/ additions trace to #180/#177, not #182.
Remediation: Either merge #177 first and rebase this PR (removing the duplicate eval/code/ commits), or close #177 as superseded and update #182's scope to explicitly authorize landing both evals.
4. Hardcoded HUMAN_INSTRUCTION in shared runner — low
File: eval/scripts/run-fullsend.sh (line ~125)
Category: maintainability
The AGENT=fix block hardcodes the instruction text, TRIGGER_SOURCE, FIX_ITERATION, and TARGET_BRANCH specific to case 001-human-fs-fix-add. Adding a second fix case with a different instruction or trigger mode requires modifying this shared script.
Remediation: Acceptable for v1 with a single case. Consider adding a TODO comment noting the coupling. Future cases should read fix-specific env from the case's input.yaml.
5. emit_env name parameter not validated — low
File: eval/scripts/run-fullsend.sh (line ~63)
Category: defense-in-depth
emit_env() validates values for newlines/CRs but does not validate the name parameter. A name containing = or whitespace would produce a malformed env file line. All current callers use safe hardcoded literals, so this is not exploitable today.
Remediation: Add name validation: if [[ ! "$name" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then echo "ERROR: invalid env name" >&2; exit 1; fi
Positive Observations
- The
emit_env()function with newline validation is a good defense-in-depth improvement over the rawechopattern in the base code. - Input validation for
FIXTURE_URL,FIXTURE_NUMBER, andEPHEMERAL_REPOstrengthens the eval infrastructure. - The producer/consumer contract for
PRE_AGENT_HEAD(sidecar file + env fallback) is well-designed. - Judge logic in both eval.yaml files correctly handles edge cases (missing fields, null values, type coercion).
- The overall architecture (mirroring
eval/code/foreval/fix/, shared hooks/scripts) is clean and consistent with the existingeval/review/andeval/triage/patterns.
Labels: PR adds fix and code agent eval infrastructure.
fullsend-ai-review
Bot
added
requires-manual-review
Scope PR-head checkout to fix only, move HUMAN_INSTRUCTION into case input.yaml, harden PR capture with retries/null-safety, validate emit_env names, and drop superpowers plan/spec docs from the PR. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Review follow-up (
|
| Finding | Action |
|---|---|
| PR-head checkout for all PR fixtures | Scoped to AGENT=fix only — review stays on main |
fetch_pr_files null-safety |
[(.files // [])[].path] + single retried PR view |
| PR capture lacks retries | retry_cmd + one consolidated gh pr view; failure writes pr_fetch_failed state |
Hardcoded HUMAN_INSTRUCTION / agent-prompt compliance |
Moved to case input.yaml (human_instruction); setup forwards via hook-outputs |
emit_env name validation |
Added ^[A-Za-z_][A-Za-z0-9_]*$ check |
eval/code/ scope vs #182 |
PR base is eval/code-functional-test (#177) — fix-only commits; rebase onto main after #177 merges |
| Superpowers plan/spec in PR | Removed from the branch |
|
🤖 Review · |
CI follow-up —
|
|
🤖 Review · |
CI follow-up 2 — stale
|
|
🤖 Review · ❌ Terminated · Started 4:12 PM UTC · Ended 4:23 PM UTC |
Functional-tests (fix) greenRun: https://github.com/fullsend-ai/agents/actions/runs/29431325042 |
![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
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 4:12 PM UTC · Completed 4:23 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 4:54 PM UTC · Completed 5:06 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Review · |
|
🤖 Finished Review · ✅ Success · Started 5:12 PM UTC · Completed 5:26 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
92135ac to
54929aa
Compare
Scope PR-head checkout to fix only, move HUMAN_INSTRUCTION into case input.yaml, harden PR capture with retries/null-safety, validate emit_env names, and drop superpowers plan/spec docs from the PR. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
1540d4f to
353e640
Compare
|
🤖 Finished Review · ✅ Success · Started 6:17 PM UTC · Completed 6:30 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
maruiz93
left a comment
maruiz93
left a comment
There was a problem hiding this comment.
Added some minor improvements
a0ecb06 to
c62ca8a
Compare
Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Scope PR-head checkout to fix only, move HUMAN_INSTRUCTION into case input.yaml, harden PR capture with retries/null-safety, validate emit_env names, and drop superpowers plan/spec docs from the PR. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Local branch alias eval-pr-head caused post-fix.sh to push a new remote branch instead of the PR head, so new_commit always failed. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
gh pr view headRefOid can lag behind a successful post-fix push, which made new_commit fail despite the branch tip moving. Prefer git refs API and briefly poll when the tip still equals pre_agent_head. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Rebase is already on #177 tip. Retries for fix headRefName fetch; extract resolve_head_sha with head_ref validation and poll budget comment; harden retry_cmd (stderr on final attempt); wrap issue-branch gh calls; stagger EVAL_TIMEOUT; drop redundant mktemp truncate; document shared judges and fix_expectations. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
353e640 to
ec1d206
Compare
|
🤖 Finished Review · ✅ Success · Started 7:48 PM UTC · Completed 8:01 PM UTC |
2 participants
Summary
eval/fix/with001-human-fs-fix-add(existing PR +human_instructionin case input)Closes #182
Stacked on #177
Base is
eval/code-functional-test(PR #177) so this PR is fix-only. After #177 merges tomain, rebase this branch ontomainand retarget the PR base tomain.Test plan
./eval/lint-cases.sh fixnew_commit+expected_filespassFunctional test proof
CI run: https://github.com/fullsend-ai/agents/actions/runs/29431325042
Job: functional-tests (fix) — pass
Ephemeral fixture (torn down after case)
halfsend/eval-001-human-fs-fix-add-eaf3b3caeval-pr-1784131907-3665c2eab6f→e85c316Judge results (
summary.yamlfrom artifacteval-results-fix)new_commitc2eab6f…→e85c316…expected_files['calc.py']forbidden_labelsmax_turnsmax_costCaptured fixture state (excerpt)
{ "fixture_type": "pull_request", "fixture_url": "https://github.com/halfsend/eval-001-human-fs-fix-add-eaf3b3ca/pull/1", "head_sha": "e85c3164bcd15b5a5d67d3553a882c37c8ea650f", "pre_agent_head": "c2eab6f4006228091cfe6bb4f4da3ece8c479d28", "head_ref": "eval-pr-1784131907-3665", "files": ["calc.py", "tests/test_calc.py"] }Note: the halfsend eval repo is deleted by
teardown-fixture.shafter capture, so the PR URL above is historical proof from the artifact, not a live page.