fix(tracing): detach serialized trace metadata - #4498
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53acf0a0bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if exported is None: | ||
| return None | ||
| payload = dict(exported) | ||
| payload = copy.deepcopy(exported) |
There was a problem hiding this comment.
Avoid deep-copying arbitrary trace metadata
When trace metadata contains an object that cannot be deep-copied, such as a lock or a JSON-compatible container with a custom failing __deepcopy__, this now raises synchronously during every normal run: Runner calls RunState.set_trace() at startup, which reaches TraceState.from_trace() and this line. Previously such metadata only traveled through the non-fatal tracing/export path, so an observability detail can now prevent the application run from starting; detach the supported container structure without requiring every arbitrary metadata value to support deepcopy, or handle copy failures without failing the run.
AGENTS.md reference: AGENTS.md:L162-L162
Useful? React with 👍 / 👎.
|
I don't think deep copying here is necessary. |
Summary
Trace.to_json()The previous top-level
dict()copy left nested metadata shared, so mutating a serialized payload could silently mutate the trace object it came from.Test plan
tests/test_trace_json_isolation.pyIssue number
N/A