feat(#1250): add discretionary ready-to-code promotion - #1251
Conversation
|
🤖 Finished Review · ✅ Success · Started 6:07 PM UTC · Completed 6:28 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $5.01 |
|
Risk Assessment: moderate (2/5) DetailsTier 1 signals are unchanged from the prior review (same 9 files, 6 protected paths, no dependency/CI changes, same bot author); Tier 2 continues to show only ambient churn on shared triage infrastructure rather than PR-specific risk; Tier 3 confirms the PR closely matches a well-scoped, opt-in, no-risk-label issue with no unresolved discussion — anchoring preserves the prior score of 2 (moderate). Previous runRisk Assessment: moderate (2/5) DetailsIndependent full re-evaluation still lands on score 2: Tier 1 is a moderate, backward-compatible, bot-authored, no-dependency change with high protected-path count offset by no security-sensitive paths and no CI changes; Tier 2 shows high churn/author-contention on these shared triage files, reflecting busy shared infrastructure rather than PR-specific risk; Tier 3 shows the PR closely tracks a well-scoped, discretionary-only, opt-in-by-default issue with no risk labels. Weighted composite rounds to 2, matching the prior score. Previous run (2)Risk Assessment: moderate (2/5) DetailsRe-review anchoring applies: Tier 1 signals (9 files, 6 protected paths, no dependency changes, bot author) remain unchanged, and the sole delta since the prior reviewed SHA is a +6/-1 prompt-text edit to agents/triage.md with no schema/script/harness changes, introducing no new risk; the prior score of 2 (moderate) is preserved. Previous run (3)Risk Assessment: moderate (2/5) DetailsRe-review anchoring applies: Tier 1 signals (9 files, 6 protected paths, no dependency changes, bot author) are unchanged from the prior assessment, and the only delta since the prior SHA is two doc-only edits (docs/code.md, docs/triage.md) with no schema/script/harness changes; the prior score of 2 (moderate) is preserved. Previous run (4)Risk Assessment: moderate (2/5) DetailsTouches protected-path files (agents/, harness/, scripts/) with historically high churn, but the change is small, backward-compatible, opt-in by default, has explicit new test coverage, comes from a track-recorded bot author, and closes a well-scoped issue with no open discussion. |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Previous run (2)ReviewFindingsMedium
Previous run (3)ReviewFindingsMedium
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Next steps:
|
![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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 6:30 PM UTC · Completed 6:37 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.77 |
🔧 Fix agent — iteration 1 (bot-triggered)Fixed both non-protected doc findings (docs/code.md and docs/triage.md) to accurately describe TRIAGE_AUTO_CODE-gated ready-to-code promotion. The api-contract fix in agents/triage.md, the test-organization fix in scripts/validate-output-schema-test.sh, and the protected-path finding itself all touch protected paths (agents/, harness/, scripts/) that this bot cannot edit without explicit human instruction; recorded as disagreements with verification notes so a human can apply them via /fs-fix or direct approval. Fixed (2):
Disagreed (3):
Tests: passed Decision points
Next steps:
|
…E-gated Addresses review feedback on PR #1251: - docs/code.md: note that ready-to-code auto-promotion is also subject to TRIAGE_AUTO_CODE (on/off/discretionary), linking to the canonical Ready-to-code promotion section in docs/triage.md. - docs/triage.md: bug/documentation rows now note they apply alongside ready-to-code OR triaged, matching post-triage.sh's unconditional category-label application ahead of the ready-to-code/triaged branch. Three findings from the same review are not addressed here because the affected files are under this repo's protected paths (agents/, harness/, scripts/), which this bot cannot modify without explicit human instruction: - agents/triage.md:369 (api-contract): the sufficient JSON example should include promote_to_ready_to_code alongside requires_workflow_changes. Verified correct against the file. - scripts/validate-output-schema-test.sh:73 (code-organization): the invalid-promote-not-boolean test should move to the Structural failures section. Verified correct against the file. - The protected-path finding itself, which correctly notes this PR's changes to agents/, harness/, and scripts/ require human approval. Note: pre-commit could not fetch its hook repositories (network 403), so hooks were run directly per the configured .pre-commit-config.yaml entries; all applicable hooks (end-of-file-fixer, trailing-whitespace, detect-private-key, check-merge-conflict, mixed-line-ending, gitleaks, and the local lint-agent-docs hook) passed. make lint could not run (uvx is unavailable in this sandbox) and make test has 9 pre-existing failures in scripts/harness-jira-test.sh due to a missing yq binary, unrelated to this docs-only change; scripts/post-triage-test.sh, the suite most relevant to this PR, passed in full. Addresses #1251
|
🤖 Finished Review · ✅ Success · Started 6:38 PM UTC · Completed 6:52 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.45 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 6:54 PM UTC · Completed 6:59 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.37 |
🔧 Fix agent — iteration 2 (bot-triggered)Both current review findings (the protected-path notice and the api-contract gap in agents/triage.md's discretionary example) were verified against the code and confirmed accurate, but both resolve to protected-path files that this bot-triggered run is not authorized to modify. No code changes were made. A human should either approve the existing protected-path changes and/or fix the missing promote_to_ready_to_code field in the JSON example directly, or issue a /fs-fix instruction explicitly authorizing this agent to make that one-line edit. Disagreed (2):
Tests: passed Decision points
Next steps:
|
|
/fs-fix go ahead and address the api contract gap |
|
🤖 Finished Fix · ✅ Success · Started 8:26 PM UTC · Completed 8:31 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.53 |
The `sufficient`-action JSON example ended `triage_summary` at `requires_workflow_changes`, omitting `promote_to_ready_to_code`. Since the post-script fail-closes on absence in discretionary mode, an agent copying the example verbatim would silently never promote. Add the field next to `requires_workflow_changes` using the same union-style placeholder as `severity`/`category`, per human /fs-fix instruction on PR #1251. Addresses #1251
🔧 Fix agent — iteration 2 (human-triggered)Added the missing promote_to_ready_to_code field to the sufficient-action JSON example in agents/triage.md, closing the api-contract gap the review agent flagged across two prior runs. This was previously blocked by the protected-path policy on bot-triggered runs; the human's explicit /fs-fix instruction authorized the edit this iteration. Fixed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:33 PM UTC · Completed 8:45 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.32 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
requires-manual-review
|
/fs-fix rebase on main and resolve merge conflicts |
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/agents/agents/.fullsend/.fullsend-cache/resources/sha256/47106004ec3cf7c0276a1222d251ee87ee3a18a76d0f02142777f77912edc34a/scripts/post-fix.sh failed: exit status 1) · Started 1:45 PM UTC · Completed 1:52 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.65 |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/agents/actions/runs/34851094859 Details: |
TRIAGE_AUTO_CODE was a strict on/off switch, so backlog grooming of sufficient bugs auto-dispatched the code agent. Add a discretionary mode in which the triage agent records promote_to_ready_to_code on the sufficient result and the post-script honors that field. on/always still auto-promote listed categories; off/never still never promote. The new field is ignored in those modes so existing harnesses keep mechanical behavior. Discretionary withholds when the field is absent or false, and still respects the category allowlist and workflow-change guard. Note: `pre-commit run` could not fetch hook repos (HTTP 403). Hooks were executed directly: shellcheck, gitleaks/scan-secrets, lint-agent-docs, check-json, check-yaml, and whitespace/conflict checks. pinact and actionlint do not apply to these files. Closes #1250
…E-gated Addresses review feedback on PR #1251: - docs/code.md: note that ready-to-code auto-promotion is also subject to TRIAGE_AUTO_CODE (on/off/discretionary), linking to the canonical Ready-to-code promotion section in docs/triage.md. - docs/triage.md: bug/documentation rows now note they apply alongside ready-to-code OR triaged, matching post-triage.sh's unconditional category-label application ahead of the ready-to-code/triaged branch. Three findings from the same review are not addressed here because the affected files are under this repo's protected paths (agents/, harness/, scripts/), which this bot cannot modify without explicit human instruction: - agents/triage.md:369 (api-contract): the sufficient JSON example should include promote_to_ready_to_code alongside requires_workflow_changes. Verified correct against the file. - scripts/validate-output-schema-test.sh:73 (code-organization): the invalid-promote-not-boolean test should move to the Structural failures section. Verified correct against the file. - The protected-path finding itself, which correctly notes this PR's changes to agents/, harness/, and scripts/ require human approval. Note: pre-commit could not fetch its hook repositories (network 403), so hooks were run directly per the configured .pre-commit-config.yaml entries; all applicable hooks (end-of-file-fixer, trailing-whitespace, detect-private-key, check-merge-conflict, mixed-line-ending, gitleaks, and the local lint-agent-docs hook) passed. make lint could not run (uvx is unavailable in this sandbox) and make test has 9 pre-existing failures in scripts/harness-jira-test.sh due to a missing yq binary, unrelated to this docs-only change; scripts/post-triage-test.sh, the suite most relevant to this PR, passed in full. Addresses #1251
The `sufficient`-action JSON example ended `triage_summary` at `requires_workflow_changes`, omitting `promote_to_ready_to_code`. Since the post-script fail-closes on absence in discretionary mode, an agent copying the example verbatim would silently never promote. Add the field next to `requires_workflow_changes` using the same union-style placeholder as `severity`/`category`, per human /fs-fix instruction on PR #1251. Addresses #1251
3fc5a4e to
c284ed5
Compare
|
🤖 Finished Review · ✅ Success · Started 12:48 AM UTC · Completed 1:07 AM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $4.69 |
Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 12:18 AM UTC · Completed 12:31 AM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.83 |
| "proposed_test_case": "Conceptual description of a test that would verify the fix — what to test, expected vs actual behavior, and edge cases to cover. Do not assume a specific test framework or file layout.", | ||
| "requires_workflow_changes": false | ||
| "requires_workflow_changes": false, | ||
| "promote_to_ready_to_code": "true | false — required when TRIAGE_AUTO_CODE is discretionary, omit otherwise" |
There was a problem hiding this comment.
[low] logic-error
The sufficient-action JSON example sets promote_to_ready_to_code to a string ("true | false — required when TRIAGE_AUTO_CODE is discretionary, omit otherwise") while schemas/triage-result.schema.json types the field as boolean and scripts/validate-output-schema-test.sh (invalid-promote-not-boolean) rejects non-booleans. The adjacent optional boolean in the same object is shown as a real boolean (requires_workflow_changes: false). Pipe-separated placeholders for severity/category in this same example are an established pattern, but those fields are typed as strings — a literal copy stays type-valid (only enum-invalid). A literal copy of this new field is a type violation. validation_loop.max_iterations is 2, so the agent has one retry before the run fails. The prose immediately below the example correctly instructs setting true/false or omitting the field, which limits real-world impact.
Suggested fix: Use a boolean in the example, matching requires_workflow_changes — e.g. "promote_to_ready_to_code": false — and keep the required-when-discretionary / omit-otherwise guidance in the Discretionary promotion paragraph (already present).
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ✅ Success · Started 12:33 AM UTC · Completed 12:41 AM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $1.10 |
|
🤖 Finished Retro · ✅ Success · Started 12:37 AM UTC · Completed 12:54 AM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $3.47 |
🔧 Fix agent — iteration 3 (bot-triggered)Fixed the low-severity logic-error finding by changing the promote_to_ready_to_code placeholder string in the sufficient-action JSON example to a literal boolean (false), matching the schema and the adjacent requires_workflow_changes example. Recorded a disagreement (no-op) on the protected-path finding since it is the standing merge-gate notice and prescribes no content change — human approval is still required before merge. Fixed (1):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
PR #1251 (discretionary TRIAGE_AUTO_CODE mode, closes #1250) took 11 days and 5 review cycles to merge, but the pipeline itself worked reasonably well: the review agent correctly and repeatedly flagged a protected-path human-approval requirement, an api-contract gap (fixed on human request), and — in its final pass — a real Low-severity logic-error (a JSON example in agents/triage.md typed Two notable defects surfaced with concrete, verifiable evidence:
One new, previously unreported root cause was found and is filed below: a human-requested rebase fix (2026-09-14) resolved a real merge conflict correctly inside its sandbox but never set the The long tail of protected-path/rework-rate issues already has extensive open-issue coverage (agents-repo#1293, #1294, #1297, #1321, #1328, #1331, #1348, #1362, #1374, #1384; fullsend#2190, #2524, #2579, #2794, #3628, #7186, #7339) — no new proposals filed on that front; this PR's 11-day, mostly-idle timeline is additional evidence for fullsend#7186's staleness-nudge idea specifically. Proposals filed |
1 participant
Summary
Adds a
discretionarymode toTRIAGE_AUTO_CODEso the triage agent canpromote a sufficient result to
ready-to-codeor leave ittriagedforhuman prioritization.
Existing values stay backward compatible:
on/always— auto-promote categories inTRIAGE_AUTO_CODE_CATEGORIESoff/never— never auto-promotediscretionary— honortriage_summary.promote_to_ready_to_codeThe new field is consulted only in discretionary mode. Legacy modes ignore
its presence or absence, so existing harnesses keep mechanical behavior.
Omitted or
falsewithholds promotion; the category allowlist andrequires_workflow_changesguard still apply.Testing
bash scripts/post-triage-test.sh— promote/withhold under discretionarymode,
always/neveraliases, and legacyon/offignoring the new fieldbash scripts/validate-output-schema-test.sh— optional boolean accepted;non-boolean rejected
make check-bundleandmake lintCloses #1250
Post-script verification
agent/1250-discretionary-auto-code)883141b9508c657c3c4235c8cd7c86577f0b47a9..HEAD)