Python: fix: add missing status field on function_call / function_call_output items

[ltwlf]

Fixes #4701

Hey! So the Responses API expects a `status` field on both `function_call` and `function_call_output` items, but we weren't always including it. This leads to 400 errors when the API rejects the request.

What changed

• `_responses_client.py` — `status` now defaults to `"completed"` for both `function_call` and `function_call_output` items. If someone explicitly sets it via `additional_properties`, that value is respected.
• `_message_adapters.py` — AG-UI media parts with a `filename` property now pass it through to the `Content` object via `additional_properties`, so binary attachments keep their original filenames.

Tests

Added 9 new tests covering:

• Default status behavior for function_call and function_call_output
• Explicit status override via additional_properties
• Filename passthrough for base64, URL, data URI, and binary ID media parts
• No filename case (ensures no spurious additional_properties)

All existing tests still pass. Ran ruff, pyright, and the full test suite — everything's clean.

[moonbox3]

Automated Code Review

Reviewers: 4 | Confidence: 86%

✓ Correctness

The diff adds filename passthrough for AG-UI media parts via additional_properties and fixes a latent bug where accessing content.additional_properties.get("status") would crash with AttributeError when additional_properties is None. The status field for function_call and function_call_output is now always emitted, defaulting to "completed". The changes and tests are correct.

✓ Security Reliability

The diff adds filename passthrough for multimodal media parts and fixes a latent AttributeError in the responses client where content.additional_properties.get('status') would crash when additional_properties is None. The fix correctly guards the attribute access. One minor concern is that filenames from untrusted input are passed through without sanitization, and the or "completed" pattern silently coerces any falsy status value (including an explicit empty string) to "completed", which may mask unexpected upstream values. No blocking security or reliability issues found.

✓ Test Coverage

Test coverage for both changes is thorough. The filename passthrough feature is tested across all major code paths (base64, URL, data URI, binary ID) plus the absent-filename case. The status-default change is covered with both default and explicit-status tests for function_call and function_call_output. One minor gap: the base64 strict-decode-failure fallback paths (lines 310–330) also thread additional_props through but have no filename-specific test, though the logic is mechanically identical to the tested paths.

✗ Design Approach

The filename passthrough via additional_properties is acceptable given that Content has no native filename field and additional_properties is the designed extension mechanism. The more significant concern is in _responses_client.py: the diff changes two independent behaviors in one stroke — (1) it fixes a real AttributeError bug (calling .get() on a possibly-None additional_properties) and (2) it hardcodes 'completed' as the default status for both function_call and function_call_output. The function_call_output case is particularly risky: previously status was never emitted for this type; now it is always emitted. If the OpenAI Responses API does not accept a status field on function_call_output objects, this silently changes a working serialization into one that may be rejected or misinterpreted. The null-safety fix and the behavioral default are conflated in a way that makes the change harder to reason about and revert.

Flagged Issues

• In _responses_client.py, the function_call_output serialization previously omitted status entirely; the diff now unconditionally emits "status": "completed". This behavioral change goes beyond the None-safety fix. If the OpenAI Responses API does not accept a status field on function_call_output (unlike function_call), this will silently introduce a field the API may reject or misinterpret. The null-safety fix should be separated from the decision to default to "completed", and the default should only be added once the API contract for function_call_output is confirmed to accept it.

Suggestions

• The filename extracted from user-supplied part dict is stored in additional_properties without validation. If downstream code ever uses this filename for filesystem operations, it could be susceptible to path-traversal attacks (e.g., ../../etc/passwd). Consider validating or sanitizing the filename (e.g., stripping path separators, limiting length) at this trust boundary.
• The or "completed" pattern on lines 1189 and 1237 coerces any falsy status value (empty string, None, 0) to "completed", silently masking unexpected upstream values. Consider using an explicit if status is None check instead for stricter semantics.
• For the function_call and function_result cases, consider extracting a _get_status(content) helper to avoid duplicating the status-extraction-and-fallback expression and to make the fallback policy easy to find and change.
• In _message_adapters.py, the string key "filename" used to stash the value in additional_properties is referenced only by convention. Defining a module-level constant (e.g., _FILENAME_KEY = "filename") would make the coupling between writer and reader explicit and refactor-safe.
• Consider adding a test for the base64 lenient-decode fallback path (strict decode fails, lenient succeeds) with a filename, to cover the additional_properties threading on lines 310–316. This is a low-risk gap since the pattern is identical to the tested paths.
• Consider a test with an empty-string filename (e.g., "filename": "") to verify it is treated the same as absent (the if filename guard filters it out, resulting in additional_properties=None).

---

Automated review by moonbox3's agents

[moonbox3]

Using or "completed" means any falsy status value (e.g., "") is silently replaced with "completed". Consider an explicit None check for stricter semantics.

Suggested change

	"type": "function_call",
	status = (
	content.additional_properties.get("status") if content.additional_properties else None
	)
	if status is None:
	status = "completed"

[moonbox3]

function_call_output did not previously emit a status field at all. Unconditionally adding "status": "completed" is a separate behavioral change from the None-safety fix, and the or "completed" pattern also silently coerces falsy values. Only emit status here if the API contract is confirmed to accept it and the field was explicitly supplied, and use an explicit None check rather than or.

Suggested change

	return {
	fn_result_status = content.additional_properties.get("status") if content.additional_properties else None
	result: dict[str, Any] = {
	"call_id": content.call_id,
	"type": "function_call_output",
	"output": output,
	}
	if fn_result_status:
	result["status"] = fn_result_status
	return result