Skip to content

feat(#677): add structured needs_input signal for code agent - #1058

Open
fullsend-ai-coder[bot] wants to merge 3 commits into
mainfrom
agent/677-code-agent-needs-input
Open

feat(#677): add structured needs_input signal for code agent#1058
fullsend-ai-coder[bot] wants to merge 3 commits into
mainfrom
agent/677-code-agent-needs-input

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Adds a structured needs_input signal so the code agent can explicitly say "I cannot proceed — a human needs to act" instead of silently no-oping.

Problem: When the code agent encounters broken tooling, a missing scan-secrets binary, or a genuinely uninterpretable issue, it currently produces no informative signal — the post-script falls through to a generic "No changed files" no-op comment with no explanation.

Solution: Three coordinated changes give the agent a structured way to push back:

  • Schema (schemas/code-result.schema.json): New optional needs_input (boolean) and needs_input_reason (string) properties
  • Skill (skills/code-implementation/SKILL.md): Steps 8 (uninterpretable issue), 9a (missing scan-secrets), and 9c (broken build tooling) now reference a new "Emitting needs_input" procedure section
  • Post-script (scripts/post-code.src.sh): Checks for needs_input before the feature branch check — applies fs-code-needs-input label to the issue, posts an explanatory comment with the agent's reason, and exits cleanly (exit 0, no PR)

A single label (fs-code-needs-input) is reused for all causes — the comment body carries the specifics.

Testing

  • 6 detection logic tests (true/false/absent, with/without reason, specific blockers)
  • 7 comment body tests (reason, issue number, retry instruction, run URL, header, no leaks)
  • 2 bundled script grep checks (needs_input handling, label creation)
  • All existing tests pass (post-code-test.sh, validate-code-output-test.sh)
  • Schema validated with jsonschema for new properties, existing properties, and additionalProperties enforcement
  • Bundle verified via make check-bundle

Closes #677

Post-script verification

  • Branch is not main/master (agent/677-code-agent-needs-input)
  • Secret scan passed (gitleaks — 2fbb7da338097ad4c3d659820444c7523a0f1e63..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

When the code agent cannot proceed without human intervention (broken
tooling, missing scan-secrets, uninterpretable issue), it now emits a
structured needs_input signal in agent-result.json instead of silently
no-oping.

Changes:
- Schema: add needs_input (boolean) and needs_input_reason (string)
  properties to code-result.schema.json
- Skill: update SKILL.md steps 8/9a/9c to reference the new
  "Emitting needs_input" procedure section
- Post-script: handle needs_input before the feature branch check —
  apply fs-code-needs-input label, post explanatory comment, exit 0
- Tests: add detection logic tests, comment body tests, and bundled
  script grep checks to post-code-test.sh
- Bundle: rebuild post-code.sh from post-code.src.sh

The fs-code-needs-input label is created on first use (yellow, like
requires-manual-review) and reused for all causes — the comment body
carries the specifics.

Closes #677
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 26, 2026 20:40
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 26, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:43 PM UTC · Completed 9:02 PM UTC

Commit: 5a2e515 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.02

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 26, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Moderate risk: well-scoped additive feature (new schema fields, post-script handling, skill updates, eval case) with bot authorship and clear issue alignment, offset by protected path modifications in scripts/ and skills/.

Previous run

Risk Assessment: moderate (2/5)

Details

Moderate risk: well-scoped additive feature (new schema fields, post-script handling, skill updates) with bot authorship and clear issue alignment, offset by protected path modifications in scripts/ and skills/.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Moderate risk: a well-scoped additive feature (new schema, scripts, and skill updates) with bot authorship and clear issue alignment, offset by high churn and multi-author contention on the heavily-modified post-code scripts.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] scripts/, skills/, agents/ — This PR modifies files under protected paths (agents/code.md, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/validate-code-output-test.sh, skills/code-implementation/SKILL.md). The PR links to issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-adequacy] scripts/post-code-test.sh — Tests use local helper functions (detect_needs_input, build_needs_input_comment) that reimplement detection and comment logic rather than driving the shipped script end-to-end. This is consistent with all other test helpers in the file and is an architectural choice rather than a correctness bug.

  • [section-numbering] scripts/post-code.src.sh — New block labeled "# 0. Check for needs_input signal" uses 0-based numbering while existing sections in this file use 1-based (# 1 through # 9). The pattern has direct precedent in sibling post-fix.src.sh which uses "# 0. Check for agent commits" for its own pre-pipeline guard.

  • [label-naming-convention] scripts/post-code.src.sh — Label fs-code-needs-input introduces an fs-code- prefix not seen in other flat-named labels. However, the codebase already uses namespaced labels (e.g., risk/* in post-review.src.sh), so prefixed naming is not without precedent. The linked issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this exact label name as a deliberate design choice.

Previous run

Review

Findings

Medium

  • [protected-path] scripts/, skills/, agents/ — This PR modifies files under protected paths (agents/code.md, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/validate-code-output-test.sh, skills/code-implementation/SKILL.md). The PR links to issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-adequacy] scripts/post-code-test.sh — Tests use local helper functions (detect_needs_input, build_needs_input_comment) that reimplement detection and comment logic rather than driving the shipped script end-to-end. This is consistent with all other test helpers in the file and is an architectural choice rather than a correctness bug.

  • [section-numbering] scripts/post-code.src.sh — New block labeled "# 0. Check for needs_input signal" uses 0-based numbering while existing sections in this file use 1-based (# 1 through # 9). The pattern has precedent in sibling post-fix.src.sh which uses "# 0. Check for agent commits" for its pre-pipeline guard.

  • [label-naming-convention] scripts/post-code.src.sh — Label fs-code-needs-input introduces an fs-code- prefix not seen in existing labels (needs-human, needs-info, ready-to-code, ready-for-review, fullsend-no-fix). The prefix namespaces to the code agent, which is defensible but inconsistent with the flat naming convention used elsewhere.

Previous run (2)

Review

Findings

High

  • [stale-doc] skills/code-implementation/SKILL.md:1076 — The schema compliance note states "Only target_branch, pr_body, and closes_issue are allowed. Any other fields will cause validation to fail." This PR adds needs_input and needs_input_reason to the schema and adds an "Emitting needs_input" section instructing the agent to write those fields. The allowlist statement is now factually incorrect and directly contradicts the new procedure section in the same file — an agent following these instructions would believe the new fields will fail validation and avoid using the feature.
    Remediation: Update the schema compliance note to include needs_input and needs_input_reason in the list of allowed fields.

Medium

  • [api-contract] schemas/code-result.schema.json:32 — The schema description for needs_input_reason states "Required when needs_input is true", but the JSON Schema has no if/then conditional to enforce this constraint. The triage-result.schema.json in this codebase uses allOf/if/then for identical conditional requirement patterns. The post-script compensates with a fallback ("No reason provided"), so this is not a runtime crash risk, but the schema's stated contract is unenforced.
    Remediation: Add a conditional requirement using JSON Schema if/then, or change the description from "Required" to "Recommended".

  • [stale-doc] agents/code.md:87 — The Structured output section says the JSON file has "target_branch (required) and optionally pr_body for the PR description." It does not mention the new needs_input or needs_input_reason fields added to the schema.
    Remediation: Add a sentence mentioning needs_input and needs_input_reason fields.

  • [stale-doc] docs/code.md:34 — The Control labels table lists ready-to-code and ready-for-review but does not include the new fs-code-needs-input label introduced by this PR. Users consulting this documentation will not know about this label.
    Remediation: Add a row for fs-code-needs-input to the Control labels table.

  • [protected-path] scripts/, skills/ — This PR modifies files under protected paths (scripts/post-code.src.sh, scripts/post-code.sh, scripts/post-code-test.sh, skills/code-implementation/SKILL.md). The PR links to issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [stale-doc] agents/code.md:104 — The Failure handling section describes only two exit states (clean commit → PR, no commit → failure report). The PR introduces a third path (needs_input → label + comment + exit 0) that is not documented.

  • [section-numbering] scripts/post-code.src.sh — The new block is labeled "# 0. Check for needs_input signal" while existing sections use 1-based numbering. The "0." numbering communicates pre-check intent but is inconsistent with the established convention.

  • [test-adequacy] scripts/post-code-test.sh:1441 — Tests reimplement detection and comment logic in local helper functions. No integration test for the full needs_input path (result file → label created → comment posted → exit 0 without PR).

  • [test-coverage] scripts/validate-code-output-test.sh — Not updated with test cases for the new needs_input and needs_input_reason schema fields.

  • [label-naming-convention] scripts/post-code.src.sh — The label fs-code-needs-input uses an fs-code- prefix not seen in existing labels (e.g., ready-for-review, needs-human). The prefix namespaces to the code agent, which is defensible, but inconsistent with the flat naming convention used elsewhere.


Labels: PR modifies code agent post-script and skill behavior


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:04 PM UTC · Completed 9:12 PM UTC

Commit: 5a2e515 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.97

- Update SKILL.md schema compliance note to include needs_input and
  needs_input_reason in the allowed fields list (stale-doc, high)
- Add JSON Schema if/then conditional requiring needs_input_reason
  when needs_input is true, matching the triage schema pattern
  (api-contract, medium)
- Update agents/code.md structured output section to mention
  needs_input and needs_input_reason fields (stale-doc, medium)
- Add third exit state (needs_input) to agents/code.md failure
  handling section (stale-doc, low)
- Add fs-code-needs-input label to docs/code.md control labels
  table (stale-doc, medium)
- Add schema validation test cases for needs_input fields in
  validate-code-output-test.sh (test-coverage, low)

Addresses review feedback on #1058
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed 6 of 10 review findings with code changes. Fixed the high-severity stale allowlist in SKILL.md, added if/then schema enforcement for needs_input_reason, updated agents/code.md with new fields and exit state, added fs-code-needs-input to docs/code.md control labels table, and added 5 schema validation test cases. Disagreed with 4 findings: protected-path (informational), section-numbering (intentional), test-adequacy (environment constraint), label-naming (intentional namespacing).

Fixed (6):

  1. stale-doc: SKILL.md schema compliance allowlist excludes needs_input fields (skills/code-implementation/SKILL.md): Updated the schema compliance note and field list to include needs_input and needs_input_reason alongside existing allowed fields
  2. api-contract: needs_input_reason described as required but not enforced by schema (schemas/code-result.schema.json): Added allOf/if/then conditional requiring needs_input_reason when needs_input is true, matching the existing pattern in triage-result.schema.json
  3. stale-doc: agents/code.md structured output section missing needs_input fields (agents/code.md): Updated structured output section to mention needs_input and needs_input_reason fields and the fs-code-needs-input label behavior
  4. stale-doc: docs/code.md control labels table missing fs-code-needs-input (docs/code.md): Added fs-code-needs-input row to the control labels table with description of its purpose and retry instructions
  5. stale-doc: agents/code.md failure handling missing needs_input exit state (agents/code.md): Added third exit state documenting the needs_input path (label + comment + exit 0 without PR creation)
  6. test-coverage: validate-code-output-test.sh missing needs_input schema test cases (scripts/validate-code-output-test.sh): Added 5 test cases covering needs_input=true with reason (pass), needs_input=false (pass), needs_input absent (pass), needs_input=true without reason (fail via if/then), and needs_input_reason without needs_input (pass)

Disagreed (4):

  1. protected-path: PR modifies files under scripts/ and skills/: This is an informational note that human approval is required for protected-path changes, not an actionable fix request. The PR already documents this requirement.
  2. section-numbering: post-code.src.sh uses 0-based numbering for needs_input check: The '0.' numbering is intentional — it communicates that this is a pre-check that runs before all numbered sections. Renumbering to '1.' and shifting all subsequent sections would be a larger, riskier change for cosmetic consistency.
  3. test-adequacy: no integration test for full needs_input path: Integration tests for the full needs_input path require network access and GitHub API calls which are not available in the test environment. The unit tests correctly validate the detection logic and comment construction in isolation, and the bundled script grep checks verify the handling is present.
  4. label-naming-convention: fs-code-needs-input uses fs-code- prefix inconsistent with flat naming: The fs-code- prefix is intentional namespacing to scope the label to the code agent, distinguishing it from the existing needs-human label which is agent-agnostic. As the platform adds more agent-specific labels, namespacing prevents collisions.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:14 PM UTC · Completed 9:35 PM UTC

Commit: f052c48 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.87

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 26, 2026 21:35

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 26, 2026
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix add an functional-test eval case for this in eval/

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:12 PM UTC · Completed 9:20 PM UTC

Commit: f052c48 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.64

Add eval case 002-needs-input-uninterpretable that exercises the
needs_input signal end-to-end: an issue with redacted context and no
actionable scope should cause the code agent to emit needs_input: true,
triggering the post-script to apply the fs-code-needs-input label and
post a blocker comment instead of creating a PR.

Update eval judges (pr_created, expected_files) to handle the
expect_needs_input annotation, and add a needs_input_signal judge
that verifies the label and comment are present.

Addresses #1058
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Added eval/code/cases/002-needs-input-uninterpretable with judges to test the needs_input signal end-to-end: uninterpretable issue → agent emits needs_input → post-script applies label and posts comment → no PR created

Fixed (3):

  1. Add a functional-test eval case for needs_input in eval/ (eval/code/cases/002-needs-input-uninterpretable/input.yaml): Created eval case with a genuinely uninterpretable issue fixture (redacted context, external ticket reference) that should trigger the code agent to emit needs_input: true
  2. Add a functional-test eval case for needs_input in eval/ (eval/code/cases/002-needs-input-uninterpretable/annotations.yaml): Added annotations with expect_needs_input: true, forbidden ready-for-review label, and code_expectations documenting the expected needs_input path
  3. Add a functional-test eval case for needs_input in eval/ (eval/code/eval.yaml): Updated pr_created and expected_files judges to handle expect_needs_input annotation (no PR is the pass condition); added needs_input_signal judge checking for fs-code-needs-input label and blocker comment; added threshold

Tests: passed

Decision points
  • Chose 'genuinely uninterpretable issue' as the needs_input trigger scenario (alternatives: Missing scan-secrets (requires sandbox modification), Broken build tooling (requires infrastructure changes), Uninterpretable issue (achievable via fixture content alone); rationale: An uninterpretable issue can be tested purely through fixture content without modifying the sandbox image or build infrastructure, making it the most reliable and portable eval case)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 1, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:22 PM UTC · Completed 9:42 PM UTC

Commit: 9b451ec · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.79

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread scripts/post-code.src.sh
fi
}

# ---------------------------------------------------------------------------

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] section-numbering

New block labeled '# 0. Check for needs_input signal' uses 0-based numbering while existing sections in this file use 1-based (# 1 through # 9). The pattern has direct precedent in sibling post-fix.src.sh which uses '# 0. Check for agent commits' for its own pre-pipeline guard.

Comment thread scripts/post-code.src.sh
forge_create_label "fs-code-needs-input" "Code agent needs human input to proceed" "FBCA04"
forge_add_label "fs-code-needs-input"

if ! forge_post_issue_comment "${needs_input_body}"; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] label-naming-convention

Label fs-code-needs-input introduces an fs-code- prefix not seen in other flat-named labels. However, the codebase already uses namespaced labels (e.g., risk/* in post-review.src.sh), so prefixed naming is not without precedent. The linked issue #677 explicitly specifies this exact label name as a deliberate design choice.

guyoron1 added a commit to guyoron1/agents that referenced this pull request Sep 6, 2026
The 800/700 pair was derived for two cases, but fullsend-ai#1058 adds a third to the
same parallelism: 1 leg, and whichever of the two PRs merges second would
land a budget that no longer holds: 3 x (120 + 800 + 180 + 30) = 3390s
against a 2700s cap. 420/320 gives 3 x 750 = 2250s and keeps a 450s setup
allowance — the same margin the two-case derivation reserved, and enough
for podman's own ~190s plus the rest. Two cases at this value are 1500s,
so merging before fullsend-ai#1058 is safe either way. 500 (the value whose
arithmetic reaches 2490s) leaves 210s, which is inside podman alone.

The window is a job-budget quotient, not a measured fit: 003 has never
run, so if its first real run needs more than 320s the code cases need
their own matrix legs rather than a larger share of this one. Said the
same thing plainly in 003's budget comment, which called the ceiling
"unmeasured" without saying the harness has never executed the case —
every Functional Tests run on this branch was gated off as unauthorized.

after_each's derivation picks up run_go_checks' 5s kill grace: ~143s
worst case, not ~135s.

Signed-off-by: guy oron <goron@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-agent ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code agent needs a structured way to say 'needs human input' instead of silently no-oping

1 participant