fix(csa-session): preserve user result schema in result.toml pipeline#308
fix(csa-session): preserve user result schema in result.toml pipeline#308RyderFreeman4Logos merged 4 commits intomainfrom
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a20b242853
ℹ️ 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".
crates/csa-session/src/manager.rs
Outdated
| match key { | ||
| "status" | "summary" | "tool" | "started_at" | "completed_at" => value.is_str(), | ||
| "exit_code" | "events_count" => value.is_integer(), | ||
| "artifacts" => value.is_array(), |
There was a problem hiding this comment.
value_matches_runtime_schema treats artifacts as runtime-compatible whenever the value is any array, so malformed/custom artifact payloads (for example artifacts = [1, 2] or tables without path) are classified as non-custom and save_result_in skips output/user-result.toml snapshotting before overwrite. Since SessionResult artifact deserialization only accepts string paths or SessionArtifact objects, this path silently drops user-authored non-runtime result content that this change is intended to preserve.
Useful? React with 👍 / 👎.
|
Addressed Codex P2 in commit 3b28c7a: artifacts schema validation now checks per-entry shape before deciding whether to skip user-result sidecar snapshot; added regression test for malformed artifacts arrays. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b28c7a902
ℹ️ 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 has_custom_schema { | ||
| let Some(contents) = existing_contents.as_deref() else { |
There was a problem hiding this comment.
save_result_in only adds output/user-result.toml to artifacts inside the if has_custom_schema branch. For sessions whose original custom schema is entirely conflicting keys (for example only [tool]), the first save snapshots to sidecar and removes those conflicting keys, so later saves no longer satisfy has_custom_schema; then the runtime merge removes artifacts and replaces it with the caller-provided list, which drops the sidecar reference. This makes downstream consumers that rely on result.toml artifacts (rather than scanning output/) lose visibility of the preserved user snapshot even though the file still exists.
Useful? React with 👍 / 👎.
|
Addressed follow-up P2 in commit 6c6771c: if output/user-result.toml exists, save_result_in now always keeps its artifact entry so later saves cannot drop visibility. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ 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". |
1 participant
Summary
output/user-result.tomlevents_count,artifacts) from leaking across savesValidation
Fixes #307