Safe outputs: blank optional fields (e.g. create_pull_request stack_position: "") get MCP "success" but are rejected by the collector, dropping the output #66716

Summary

When an agent sends an empty string for an optional safe-output field, the safeoutputs MCP server accepts the call and returns success. The collector rejects the same item later. Because the agent already received success, it cannot correct and retry. The output is dropped and the run ends report_incomplete.

In the observed run, create_pull_request was called with stack_position: "". The pull request was never opened, and the patch exists only in the run artifact.

The bug is in engine-agnostic safe-output code, so it is not specific to Pi.

Observed run

• Run: https://github.com/githubnext/gh-aw-routing-sandbox/actions/runs/37703103348 (private sandbox; workflow_dispatch)
• Engine: pi, model copilot/gpt-5.6-sol; compiler dev-303b402810; setup action 303b4028
• Job results: agent, detection, and safe_outputs succeeded; conclusion failed
• Collector error: Line 1: create_pull_request 'stack_position' must be a valid positive integer (got: )
• Agent output was replaced by report_incomplete with reason: invalid_safe_outputs and failureCause: request_rejection. Its details say "Agent finished without emitting a terminal safe output". That wording is misleading: the agent did emit one, and the collector rejected it.

Sequence

1. Pi presented the create_pull_request tool schema to the model unchanged: only title and body were required, and strict mode was not used. The model's reasoning said it would "err on the side of caution and fill everything in". It sent every optional property and left the unused ones blank: stack_position: "", stack_root: "", dependencies: [], labels: [].
2. The safeoutputs MCP server accepted the call. The schema type for stack_position is number | string, so "" passes schema validation. The handler returned {"result":"success"} with the patch and bundle paths, and the agent ended its turn.
3. During collection, safe_output_type_validator.cjs ran validateOptionalPositiveInteger. That function treats only undefined as absent, so "" was parsed as a number, became NaN, and the item was rejected. It was the only item, so the run had no terminal safe output.

Local reproduction (fixture, same commit)

I replayed the exact NDJSON line from the run through validateItem and got the same error. When only the empty-string properties are removed, the same item validates cleanly. Downstream, stacked_pull_requests.cjs already treats a blank stack_root as absent, which matches the intended semantics.

Root cause

1. Optional validators treat "" as a supplied value. Several optional-field paths in validateField check only for undefined/null: optional positive integer, optional issue-number-or-temporary-ID, pattern, enum, min-length, and number/boolean/array type checks. An empty string is the most common way a model expresses "not applicable", yet it fails these checks.
2. MCP-time and collection-time validation disagree. The MCP layer validates only against the JSON Schema. The stricter type validator runs only in the collector. Any value that passes the first check but fails the second is reported to the agent as success and then silently dropped. This is the same kind of contract mismatch as dismiss-pull-request-review sample contract contradicts ingestion validation #55177.

Can other engines hit this? Yes

• The safeoutputs MCP server, the CLI bridge, and the collector are shared by all engines. Whether a run fails depends on the model filling in optional fields, not on the engine. Pi did nothing unusual here: it forwarded the schema faithfully. Copilot, Claude, Codex, Gemini, OpenCode and other engines reach the same code whenever their model blank-fills optional parameters.
• CLI-proxy path: mcp_cli_bridge.cjs converts --stack_position "" to 0 for number-typed properties. The collector then rejects it with (got: 0). This is the same bug with a different symptom.
• Scope across all safe-output types (local probe at 303b402810, sending "" to every optional field through the real validators):
  • 78 of 217 optional fields across 66 types reject "" at collection time.
  • 29 of them, across about 20 types, are in the silent class: the MCP layer accepts "" and returns success, and the collector rejects it later:
    • Number/ID fields: close_issue.issue_number, close_pull_request.pull_request_number, close_discussion.discussion_number, add_comment.comment_id, create_pull_request.stack_position, create_pull_request_review_comment.pull_request_number, create_pull_request_review_comment.start_line, reply_to_pull_request_review_comment.pull_request_number, dismiss_pull_request_review.review_id, create_code_scanning_alert.column, add_labels.item_number, remove_labels.item_number, replace_label.item_number, assign_milestone.milestone_number, assign_to_agent.issue_number, assign_to_agent.pull_number, update_issue.milestone, update_project.content_number
    • Pattern-constrained strings: add_comment.temporary_id, create_pull_request.temporary_id, create_project.temporary_id, update_project.temporary_id, upload_artifact.temporary_id, ledger_append.temp_id, update_project.draft_issue_id, update_project.target_repo, create_code_scanning_alert.ruleIdSuffix, create_project_status_update.start_date, create_project_status_update.target_date
  • The other 49 are rejected at MCP call time, so the agent sees an error and can retry. That is less severe, but it still wastes turns on a value that only means "not set".
• Recurrence: this is the only observed occurrence. The other three recent failed Task runs in the same repository (all Copilot) failed for unrelated reasons.

Proposed solution

1. Normalize at MCP ingress. The safe-outputs argument normalizer already remaps parameter synonyms. Extend it to drop optional (non-required) properties whose value is an empty or whitespace-only string, before schema validation and before the item is written to NDJSON. Limit this to properties where an empty string cannot be meaningful: non-string or mixed types, and string properties with a pattern, enum, or format constraint. Free-text string properties should keep "", so intentional clears in update-style tools still work. Required properties are unchanged and still produce the existing "requires a … field" error.
2. Apply the same rule in the collector. In validateField, treat an empty or whitespace-only string on a non-required field as absent, before any kind-specific check, using the same scoping as step 1. Omit the field from the normalized item. This also covers NDJSON written by paths other than MCP, and items emitted by older runtimes.
3. CLI bridge. For an optional property, an empty flag value should leave the property out instead of being converted to 0 or kept as "".
4. Prevent future MCP-vs-collector drift. Add a conformance test that iterates safe_outputs_tools.json against the validation config and checks that a blank value on any optional field is either accepted by both layers or rejected by both. Longer term, consider running the collector's type validation in the MCP handler before appending the item, so agents always get an actionable error they can retry instead of a false success (see dismiss-pull-request-review sample contract contradicts ingestion validation #55177).
5. Clearer diagnostics (optional). When the collector rejects every item, the report_incomplete details should say the agent emitted outputs that failed validation, not "finished without emitting a terminal safe output".

Acceptance criteria

• Replaying the NDJSON line from this run produces a valid create_pull_request item that omits stack_position and stack_root.
• A table-driven test covers every optional field in the validation config with "": each one is either treated as absent or rejected consistently at both the MCP and collector layers. None reports success at the MCP layer and is then dropped.
• --stack_position "" through the CLI bridge results in the property being absent, not 0.
• Required fields still reject "" with the existing messages, and free-text optional strings still accept "".