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

[fullsend-ai-coder]

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)

[fullsend-ai-review]

🤖 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]

Risk Assessment: moderate (2/5)

Details

Re-review anchoring applied: all Tier 1 signals are identical to the prior assessment (PROTECTED_PATH_COUNT=6, TEST_FILE_RATIO=0.17, bot author, no security exposure), and Tier 2 high-churn and multi-author signals remain repository-wide characteristics already noted in the prior rationale; Tier 3 issue alignment is unchanged and strong, keeping the composite at 2 (moderate).

Previous run

Risk Assessment: moderate (2/5)

Details

Moderate risk: Tier 1 signals unchanged from prior assessment — bot-authored additive feature (new schema fields, post-scripts, eval case) with clear issue alignment and no security exposure, offset by 6 protected-path touches and a test-file ratio of 0.17; Tier 2 flags persistent high churn on hot files and 13 distinct authors with fix/revert touches in 90 days, but these are repository-wide characteristics already present in the prior assessment, and the low-risk Tier 3 issue alignment keeps the composite at 2.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Moderate risk: signals are unchanged from prior assessment — bot-authored additive feature (new schema fields, post-scripts, eval case) with clear issue alignment and no security exposure, offset by 6 protected-path touches, meaningful repository churn (71 commits/30d, 13 authors/90d), and a test-file ratio below the preferred 0.30 threshold.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Moderate risk: signals are unchanged from prior assessment - bot-authored additive feature (new schema fields, post-scripts, eval case) with clear issue alignment and no security exposure, offset by 6 protected-path touches, meaningful repository churn (71 commits/30d, 13 authors/90d), and a test-file ratio below the preferred 0.30 threshold.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Moderate risk: additive, bot-authored feature (new schema fields, post-scripts, eval case) with clear issue alignment and no security exposure, offset by 6 protected-path touches, meaningful repository churn (71 commits/30d, 13 authors/90d), and a test-file ratio below the preferred 0.30 threshold.

Previous run (5)

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 (6)

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 (7)

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]

Review

Findings

Medium

• [protected-path] agents/, scripts/, skills/ — 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

• [schema-constraint-relaxation] schemas/code-result.schema.json:45 — target_branch is no longer unconditionally required. The base schema declared it at the top level; the PR moves it to the else branch of an allOf/if/then/else conditional. The sole known consumer (post-code.sh) is updated in this PR to handle the early-exit path.

• [schema-addition] schemas/code-result.schema.json:40 — The if condition in the allOf block contains "required": ["needs_input"] alongside the const: true check. This is the correct guard against vacuous truth — if accidentally removed, objects without needs_input would match the if vacuously, triggering then (requiring needs_input_reason) instead of else (requiring target_branch).

• [test-adequacy] scripts/post-code-test.sh — Tests use local helper functions 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.

• [test-coverage-gap] scripts/validate-code-output-test.sh — Missing test case for needs_input explicitly false without target_branch: {"needs_input":false} should fail validation (else branch fires, target_branch required). The existing schema-missing-target-branch test covers the needs_input absent path, but the needs_input=false path exercises a subtly different JSON Schema evaluation (const mismatch vs required miss).

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

• [naming-abstraction] scripts/post-code.src.sh:415 — The needs_input handler calls _post_failure_ensure_token inside a path the comment declares is "an expected path — exit 0, not a failure." The function is pure token setup with no failure-specific logic; the naming mismatch is pre-existing.

• [label-naming-convention] scripts/post-code.src.sh:430 — Label fs-code-needs-input introduces an fs-code- prefix not seen in other flat-named labels. Namespaced labels have precedent (e.g., risk/* in post-review.src.sh), and issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this label name.

• [architectural-coherence] scripts/post-code.sh:2451 — The post-script reads needs_input_reason with a // "No reason provided" fallback. This path is unreachable: the schema enforces needs_input_reason via allOf/if-then when needs_input is true, and the validation loop skips the post-script when schema validation fails. The fallback provides defense-in-depth.

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.

• [technical-documentation-accuracy] skills/code-implementation/SKILL.md:1125 — The text states "target_branch (required) and optionally pr_body, closes_issue, needs_input, and needs_input_reason" but the schema now uses conditional validation: target_branch is only required when needs_input is NOT true, and needs_input_reason is required (not optional) when needs_input IS true. This contradicts the schema and could lead the agent to omit needs_input_reason when signaling needs_input: true, causing schema validation failure. The same inaccuracy exists in agents/code.md line 86.
  Remediation: Rewrite the summary to reflect the conditional requirements: target_branch is required for normal runs; needs_input_reason is required when needs_input is true.

Low

• [schema-constraint-relaxation] schemas/code-result.schema.json:35 — target_branch is no longer unconditionally required. The base schema declared it at the top level; the PR moves it to the else branch of an allOf/if/then/else conditional. The sole known consumer (post-code.sh) is updated in this PR to handle the early-exit path.
  Remediation: Consider documenting the conditional requirement in the schema's description field for future consumers.

• [schema-addition] schemas/code-result.schema.json:24 — The if condition in the allOf block contains "required": ["needs_input"] alongside the const: true check. This is the correct guard against vacuous truth — if accidentally removed, objects without needs_input would match the if vacuously, triggering then (requiring needs_input_reason) instead of else (requiring target_branch).
  Remediation: Add a $comment field on the if subschema explaining that required: ["needs_input"] guards against vacuous truth.

• [test-naming-convention] scripts/validate-code-output-test.sh:117 — Test name schema-valid-needs-input-reason-only implies only needs_input_reason is present, but the input JSON also includes target_branch. The established -only suffix convention in this file means the named field is the sole content.
  Remediation: Rename to schema-valid-needs-input-reason-without-flag or similar.

• [test-adequacy] scripts/post-code-test.sh — Tests use local helper functions 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.

• [architectural-coherence] scripts/post-code.sh:2451 — The post-script reads needs_input_reason with a // "No reason provided" fallback. This path is unreachable: the schema enforces needs_input_reason via allOf/if-then when needs_input is true, and the validation loop skips the post-script when schema validation fails. The fallback provides defense-in-depth.

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

• [naming-abstraction] scripts/post-code.src.sh:415 — The needs_input handler calls _post_failure_ensure_token inside a path the comment declares is "an expected path — exit 0, not a failure." The function is pure token setup with no failure-specific logic; the naming mismatch is pre-existing.

• [label-naming-convention] scripts/post-code.src.sh:430 — Label fs-code-needs-input introduces an fs-code- prefix not seen in other flat-named labels. Namespaced labels have precedent (e.g., risk/* in post-review.src.sh), and issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this label name.

---

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

Previous run (2)

Review

Findings

Medium

• [intent-alignment] schemas/code-result.schema.json:7 — target_branch is unconditionally required by the schema, but the needs_input signal can be triggered before a target branch is determined — for example, when the issue is immediately uninterpretable (planning stage) or when scan-secrets is missing (before any code work begins). If an agent emits a needs_input file without target_branch, schema validation fails and the validation loop skips the post-script entirely. The needs_input signal is then silently swallowed and the fs-code-needs-input label is never applied. The validation loop's retry mechanism mitigates this (the agent gets feedback to add target_branch), but if retries are exhausted the signal is permanently lost.
  Remediation: Either (a) add a second allOf condition that makes target_branch optional when needs_input is true, or (b) add explicit guidance in agents/code.md and the skill's needs_input instructions that target_branch must be set to a valid default even when signaling needs_input.

• [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

• [intent-alignment] schemas/code-result.schema.json:28 — The schema's needs_input property description says "No code changes are expected", a normative assertion. agents/code.md was updated this cycle to use advisory language ("The agent should not commit code changes before signaling needs_input, but the post-script does not enforce this"). The schema description is now the only place with the normative form, creating an inconsistency.
  Remediation: Update the schema description to match the agents/code.md advisory language.

• [variable-naming-convention] scripts/post-code.src.sh:414 — The needs_input block introduces four lowercase variables at the top-level script scope: safe_issue_number, run_url, sanitized_reason, needs_input_body. The overwhelming majority of top-level assignments in this file use UPPER_SNAKE_CASE. The same applies to the compiled post-code.sh.
  Remediation: Rename to SAFE_ISSUE_NUMBER, RUN_URL, SANITIZED_REASON, NEEDS_INPUT_BODY in the top-level block (both post-code.src.sh and post-code.sh).

• [architectural-coherence] scripts/post-code.sh:2451 — The post-script reads needs_input_reason with a // "No reason provided" fallback, implying it treats schema-invalid output as a valid path. In practice this path is unreachable: the schema enforces needs_input_reason via allOf/if-then when needs_input is true, and the validation loop skips the post-script when schema validation fails. The fallback provides defense-in-depth for potential file corruption or future schema changes.

• [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 a 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. Namespaced labels have precedent (e.g., risk/* in post-review.src.sh), and issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this exact label name.

• [naming-abstraction] scripts/post-code.sh:2460 — The needs_input handler calls _post_failure_ensure_token inside a path the comment explicitly declares is "an expected path — exit 0, not a failure." The function itself is pure token setup with no failure-specific logic, and the naming mismatch is pre-existing (post_noop_comment also calls it). See also: [variable-naming-convention] finding at this location.

• [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.

---

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

Previous run (3)

Review

Findings

Medium

• [logic-error] scripts/post-code.src.sh:405 — The needs_input handler does not check for EXTERNAL_WORK_ITEM=true before using ISSUE_NUMBER for forge operations. When the code agent is invoked for an external work item (tracker differs from forge), ISSUE_NUMBER may be empty. This causes forge_add_label and forge_post_issue_comment to make malformed API calls that fail silently due to || true and 2>/dev/null, so the script exits 0 but the needs_input signal is silently lost — no label applied, no comment posted. The analogous post_noop_comment function already guards against this with an EXTERNAL_WORK_ITEM check.
  Remediation: Add an EXTERNAL_WORK_ITEM guard at the start of the needs_input block, matching the pattern in post_noop_comment. Apply to both post-code.src.sh and post-code.sh.

• [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

• [emphasis-convention] skills/code-implementation/SKILL.md:850 — The file uses **STOP** (bold, uppercase) as the established convention for non-negotiable halt instructions — at line 45 and line 761. The two new emit-needs_input instructions added in this PR at lines 514–515 and 850 use lowercase 'stop', inconsistent with the convention the PR itself preserves at line 45.
  Remediation: Change the lowercase 'stop' at lines 514–515 and 850 to **STOP** to match the established halt-instruction convention.

• [intent-alignment] agents/code.md:124 — 'No code changes are expected' in the needs_input exit-state description is a documentation convention, not a post-script invariant. If an agent creates commits and then later signals needs_input, those commits are silently orphaned. The post-script exits 0 without validating the no-commit assertion.
  Remediation: Either strengthen the language to clarify this is a convention rather than a guarantee, or add a post-script guard.

• [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.

• [naming-abstraction] scripts/post-code.sh:2455 — The needs_input handler calls _post_failure_ensure_token inside a path the comment explicitly declares is "an expected path — exit 0, not a failure." The function itself is pure token setup with no failure-specific logic, and the naming mismatch is pre-existing (post_noop_comment also calls it). See also: [architectural-coherence] finding at this location.
  Remediation: Consider extracting the token-ensure logic into a neutrally-named helper (e.g., _ensure_forge_token) shared by both the failure and needs_input paths.

• [architectural-coherence] scripts/post-code.sh:2451 — The post-script reads needs_input_reason with a // "No reason provided" fallback, implying it considers schema-invalid output a valid path. In practice this path is unreachable: the schema enforces needs_input_reason via allOf/if-then, and the validation_loop skips the post-script when schema validation fails. The fallback provides defense-in-depth for potential file corruption or future schema changes. See also: [naming-abstraction] finding at this location.

---

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

Previous run (4)

Review

Findings

Medium

• [logic-error] scripts/post-code.src.sh:405 — The needs_input handler does not check for EXTERNAL_WORK_ITEM=true before using ISSUE_NUMBER for forge operations. When the code agent is invoked for an external work item (tracker differs from forge), ISSUE_NUMBER may be empty. This causes forge_add_label and forge_post_issue_comment to make malformed API calls that fail silently due to || true and 2>/dev/null, so the script exits 0 but the needs_input signal is silently lost — no label applied, no comment posted. The analogous post_noop_comment function already guards against this with an EXTERNAL_WORK_ITEM check.
  Remediation: Add an EXTERNAL_WORK_ITEM guard at the start of the needs_input block, matching the pattern in post_noop_comment. Apply to both post-code.src.sh and post-code.sh.

• [stale-doc] eval/README.md:112 — The Test case structure section describes annotations.yaml as containing labels, review expectations, max_turns, and max_cost_usd. This PR introduces a new annotation key expect_needs_input and a new eval judge needs_input_signal that branches on this key. Neither is mentioned in the eval README. A developer writing a new code-agent test case for the needs_input path would have to discover the annotation by reading existing case files rather than the documented reference.
  Remediation: Add expect_needs_input to the annotations.yaml field list in the Test case structure section.

• [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.

• [naming-abstraction] scripts/post-code.sh:2455 — The needs_input handler calls _post_failure_ensure_token inside a path the comment explicitly declares is "an expected path — exit 0, not a failure." The function itself is pure token setup with no failure-specific logic, and the naming mismatch is pre-existing (post_noop_comment also calls it), but adds one more call site that reinforces the semantic confusion.
  Remediation: Consider extracting the token-ensure logic into a neutrally-named helper (e.g., _ensure_forge_token) shared by both the failure and needs_input paths.

---

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

Previous run (5)

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 (6)

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 (7)

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-coder]

🤖 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

[fullsend-ai-coder]

🔧 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]

🤖 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

Superseded by updated review

[ralphbean]

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

[fullsend-ai-coder]

🤖 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

[fullsend-ai-coder]

🔧 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]

🤖 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]

🤖 Finished Review · ✅ Success · Started 5:01 PM UTC · Completed 5:22 PM UTC

Commit: c907ec3 · View workflow run →

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

[fullsend-ai-coder]

🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/agents/agents/.fullsend/.fullsend-cache/resources/sha256/f0b37e364cfe852baffb406950fba12d481781d1ec6d4ec0d4d22f581fa19dd8/scripts/post-fix.sh failed: exit status 1) · Started 5:24 PM UTC · Completed 5:31 PM UTC

Commit: c907ec3 · View workflow run →

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

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 5:25 PM UTC · Ended 5:46 PM UTC

Commit: 252d7b4 · View workflow run →

[fullsend-ai-coder]

⚠️ Post-fix script failed — Push rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/agents/actions/runs/34256691147

Details:
To https://github.com/fullsend-ai/agents.git
! [rejected] agent/677-code-agent-needs-input -> agent/677-code-agent-needs-input (fetch first)
error: failed to push some refs to 'https://github.com/fullsend-ai/agents.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally. This is usually caused by another repository pushing to
hint: the same ref. If you want to integrate the remote changes, use
hint: 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
To https://github.com/fullsend-ai/agents.git
! [rejected] agent/677-code-agent-needs-input -> agent/677-code-agent-needs-input (stale info)
error: failed to push some refs to 'https://github.com/fullsend-ai/agents.git'
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:25 PM UTC · Completed 5:46 PM UTC

Commit: 252d7b4 · View workflow run →

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

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 5:47 PM UTC · Completed 5:55 PM UTC

Commit: 252d7b4 · View workflow run →

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

[fullsend-ai-coder]

🔧 Fix agent — iteration 3 (bot-triggered)

Fixed three review findings: added EXTERNAL_WORK_ITEM guard to prevent silent needs_input signal loss with external work items, normalized halt-instruction emphasis to STOP convention, and clarified needs_input documentation as convention rather than enforced invariant. Six informational findings disagreed with as out-of-scope or by-design.

Fixed (3):

1. The needs_input handler does not check for EXTERNAL_WORK_ITEM=true before using ISSUE_NUMBER for forge operations (scripts/post-code.src.sh): Added EXTERNAL_WORK_ITEM guard at the start of the needs_input block, matching the pattern in post_noop_comment. Applied to both post-code.src.sh and post-code.sh.
2. The two new emit-needs_input instructions use lowercase stop, inconsistent with the established STOP convention (skills/code-implementation/SKILL.md): Changed lowercase 'stop' to STOP (bold uppercase) at lines 515 and 850 to match the established halt-instruction convention at line 45.
3. No code changes are expected in the needs_input exit-state description is a documentation convention, not a post-script invariant (agents/code.md): Clarified the language to state this is a convention the post-script does not enforce, rather than implying it is a guaranteed invariant.

Disagreed (6):

1. This PR modifies files under protected paths (agents/code.md, scripts/, skills/). Human approval is always required for protected-path changes.: Informational finding — the PR inherently modifies protected paths as part of the feature. Human approval is required and expected; no code fix can address this.
2. Tests use local helper functions that reimplement detection and comment logic rather than driving the shipped script end-to-end: The reviewer acknowledges this is consistent with all other test helpers in the file and is an architectural choice rather than a correctness bug.
3. New block labeled # 0 uses 0-based numbering while existing sections use 1-based: The reviewer notes this has direct precedent in sibling post-fix.src.sh which uses # 0 for its pre-pipeline guard.
4. Label fs-code-needs-input introduces an fs-code- prefix not seen in other flat-named labels: The reviewer acknowledges the codebase already uses namespaced labels and issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this label name as a deliberate design choice.
5. The needs_input handler calls _post_failure_ensure_token inside a path declared as expected — not a failure: Renaming _post_failure_ensure_token to a neutral name would be a refactoring of pre-existing code outside this PR's scope. The reviewer notes the naming mismatch is pre-existing (post_noop_comment also calls it).
6. The post-script reads needs_input_reason with a fallback, implying schema-invalid output is a valid path: The reviewer describes this as defense-in-depth for potential file corruption or future schema changes. No action needed.

Tests: passed

Decision points

• Chose to add EXTERNAL_WORK_ITEM guard inline rather than extracting a shared helper (alternatives: Extract a shared helper function for the guard pattern, Add guard inline matching post_noop_comment; rationale: The inline guard matches the established pattern in post_noop_comment and keeps the change minimal.)

---

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]

🤖 Finished Review · ✅ Success · Started 5:57 PM UTC · Completed 6:18 PM UTC

Commit: f4acf3d · View workflow run →

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

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:20 PM UTC · Completed 6:28 PM UTC

Commit: f4acf3d · View workflow run →

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

[fullsend-ai-coder]

🔧 Fix agent — iteration 4 (bot-triggered)

Addressed three actionable findings: (1) restructured schema to make target_branch conditional on needs_input, (2) aligned schema description with advisory language, (3) renamed variables to UPPER_SNAKE_CASE. Six informational findings recorded as disagreements with rationale. Strategy escalation: prior iterations (1-3) addressed doc/skill files but not the schema constraint or variable naming directly — this iteration fixes the root causes in the schema and scripts.

Fixed (3):

1. [intent-alignment] target_branch unconditionally required conflicts with needs_input signal (schemas/code-result.schema.json): Moved target_branch requirement into the allOf else clause so it is only required when needs_input is not true. Added test case for needs_input without target_branch.
2. [intent-alignment] Schema description uses normative 'No code changes are expected' vs advisory language in agents/code.md (schemas/code-result.schema.json): Updated needs_input description to match the advisory language already used in agents/code.md.
3. [variable-naming-convention] Lowercase variables in top-level needs_input handler (scripts/post-code.src.sh): Renamed safe_issue_number, run_url, sanitized_reason, needs_input_body to UPPER_SNAKE_CASE in both post-code.src.sh and post-code.sh.

Disagreed (6):

1. [protected-path] PR modifies files under protected paths: Acknowledged — human approval is always required for protected-path changes. This is an informational finding, not actionable by the fix agent.
2. [architectural-coherence] needs_input_reason fallback provides defense-in-depth: Reviewer acknowledges this is defense-in-depth for potential file corruption or future schema changes, not a bug.
3. [section-numbering] 0-based numbering for needs_input section: Reviewer acknowledges direct precedent in sibling post-fix.src.sh which uses '# 0. Check for agent commits'.
4. [label-naming-convention] fs-code-needs-input prefix: Reviewer acknowledges namespaced labels have precedent and issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this exact label name.
5. [naming-abstraction] _post_failure_ensure_token called from non-failure path: Reviewer acknowledges the naming mismatch is pre-existing (post_noop_comment also calls it). The function is pure token setup with no failure-specific logic.
6. [test-adequacy] Test helpers reimplement logic rather than driving end-to-end: Reviewer acknowledges this is consistent with all other test helpers in the file and is an architectural choice rather than a correctness bug.

Tests: passed

Strategy change: Prior iterations addressed documentation in agents/code.md and skills/SKILL.md but did not modify the schema constraint or script variable naming. This iteration directly fixes the schema (allOf restructure) and scripts (variable casing), addressing root causes rather than documentation.

Decision points

• Schema approach for target_branch + needs_input interaction (alternatives: Option (a): Make target_branch optional via schema allOf/else when needs_input is true, Option (b): Add documentation guidance that target_branch must always be set; rationale: Chose option (a) because it prevents the validation loop from silently swallowing the needs_input signal when the agent has no target branch to report. Documentation-only fixes were the approach in prior iterations and did not resolve the underlying schema constraint.)

---

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]

🤖 Finished Review · ✅ Success · Started 6:30 PM UTC · Completed 6:49 PM UTC

Commit: 8e63339 · View workflow run →

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

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:51 PM UTC · Completed 6:59 PM UTC

Commit: 8e63339 · View workflow run →

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

[fullsend-ai-coder]

🔧 Fix agent — iteration 5 (bot-triggered)

Addressed all actionable findings: rewrote docs in SKILL.md and agents/code.md to accurately describe conditional schema requirements, added $comment to schema explaining vacuous truth guard, documented conditional requirement in target_branch description, renamed misleading test. Recorded reasoned disagreements for 6 informational/pre-existing/deliberate-design findings.

Fixed (4):

1. [technical-documentation-accuracy] SKILL.md:1125 and agents/code.md:86 — docs describe target_branch as unconditionally required (skills/code-implementation/SKILL.md): Rewrote structured-output documentation in SKILL.md to list conditional requirements explicitly: target_branch required for normal runs, needs_input_reason required when needs_input is true. Applied matching fix to agents/code.md.
2. [schema-constraint-relaxation] schemas/code-result.schema.json:35 — document conditional requirement in description (schemas/code-result.schema.json): Updated target_branch description to note it is required for normal runs and optional when needs_input is true.
3. [schema-addition] schemas/code-result.schema.json:24 — add $comment explaining vacuous truth guard (schemas/code-result.schema.json): Added $comment to the allOf/if subschema explaining that required: [needs_input] guards against vacuous truth.
4. [test-naming-convention] validate-code-output-test.sh:117 — misleading test name schema-valid-needs-input-reason-only (scripts/validate-code-output-test.sh): Renamed test from schema-valid-needs-input-reason-only to schema-valid-needs-input-reason-without-flag to accurately reflect that target_branch is also present.

Disagreed (6):

1. [protected-path] scripts/, skills/, agents/ — PR modifies protected paths: Protected-path changes are expected for this feature PR (issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677). Human approval is required and cannot be addressed by the fix agent.
2. [test-adequacy] post-code-test.sh — tests use local helper functions rather than end-to-end: Reviewer explicitly notes this is consistent with all other test helpers in the file and is an architectural choice, not a correctness bug.
3. [architectural-coherence] post-code.sh:2451 — unreachable fallback for needs_input_reason: Defense-in-depth is intentional. The reviewer acknowledges the path is unreachable due to schema enforcement and characterizes the fallback as defense-in-depth.
4. [section-numbering] post-code.src.sh:393 — 0-based numbering: Reviewer notes this has direct precedent in sibling post-fix.src.sh which uses '# 0.' for its own pre-pipeline guard.
5. [naming-abstraction] post-code.src.sh:415 — _post_failure_ensure_token naming mismatch: Pre-existing naming issue. The function is pure token setup with no failure-specific logic; the naming mismatch is pre-existing and out of scope for this PR.
6. [label-naming-convention] post-code.src.sh:430 — fs-code- prefix: Issue Code agent needs a structured way to say 'needs human input' instead of silently no-oping #677 explicitly specifies this label name. Namespaced labels have precedent (e.g., risk/* in post-review.src.sh).

Tests: passed

Strategy change: Iteration 5 (above escalation threshold): rewrote doc sections comprehensively instead of minimal single-line patches that left residual inaccuracies.

Decision points

• Strategy escalation at iteration 5: rewrote docs comprehensively instead of minimal patching (alternatives: Minimally patch the specific lines mentioned, Rewrite the documentation sections to fully reflect conditional schema logic; rationale: Previous iterations patched individual sentences but left inaccuracies. A comprehensive rewrite of the structured-output documentation sections ensures the conditional requirements are clearly communicated.)

---

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]

🤖 Finished Review · ✅ Success · Started 7:01 PM UTC · Completed 7:22 PM UTC

Commit: ef42079 · View workflow run →

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

Superseded by updated review