fix: redact function tool trace span errors#3111
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
seratch
left a comment
There was a problem hiding this comment.
Thanks for the fix. The direction looks right, but I think this still needs one more redaction path before we merge.
This PR covers the failure_error_function=None path where the tool exception is re-raised, but the default/handled function-tool error path can still put the raw exception message into the function span error even when RunConfig(trace_include_sensitive_data=False) is set.
I verified this on the PR head with a small probe:
- define a normal
@function_toolwithoutfailure_error_function=None - have it raise
ValueError("secret-token-123") - run with
RunConfig(trace_include_sensitive_data=False) - inspect the exported function span
The exported span still includes:
{
"message": "Error running tool (non-fatal)",
"data": {
"tool_name": "error_tool",
"error": "secret-token-123"
}
}The leaking path appears to be the handled-error reporter in src/agents/tool.py, which currently stores str(error) in SpanError.data["error"] without checking the active run config's trace_include_sensitive_data value.
Could you update that path as well and add a regression test for the default @function_tool error handling case, not only the failure_error_function=None case?
|
Thanks for the detailed catch. Agreed — I’ll add regression coverage for this path and update the implementation accordingly. |
|
Updated based on the review feedback. This now covers the default Added/updated regression coverage for:
Re-ran the full local verification successfully:
|
|
I checked the latest head ( Local verification: uv run pytest tests/test_run_step_execution.py -k 'function_tool_error_trace_respects_sensitive_data_setting or default_function_tool_error_trace_respects_sensitive_data_setting or cancelled_function_tool_error_trace_respects_sensitive_data_setting' -q
# 3 passed, 78 deselected
uv run ruff check src/agents/run_internal/tool_execution.py src/agents/tool.py src/agents/util/_tool_errors.py tests/test_run_step_execution.py
# All checks passedI did not find another unredacted function-span error path in the touched flow. |
Summary
trace_include_sensitive_data=FalseTest plan
uv run pytest tests/test_run_step_execution.py::test_function_tool_error_trace_respects_sensitive_data_setting -qbash .agents/skills/code-change-verification/scripts/run.shIssue number
Closes #3110
Checks
make lintandmake format