Attach Windows sandbox log to feedback reports#24623
Conversation
iceweasel-oai
force-pushed
the
codex/windows-sandbox-feedback-log
branch
2 times, most recently
from
719f929 to
e40c947
Compare
![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: 4ebbb28945
ℹ️ 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".
| @@ -93,6 +93,9 @@ tracing = { workspace = true, features = ["log"] } | |||
| tracing-subscriber = { workspace = true, features = ["env-filter", "fmt", "json"] } | |||
| uuid = { workspace = true, features = ["serde", "v7"] } | |||
|
|
|||
| [target.'cfg(windows)'.dependencies] | |||
| codex-windows-sandbox = { workspace = true } | |||
There was a problem hiding this comment.
This adds a new Cargo dependency, but MODULE.bazel.lock is not updated in the commit. The repo’s dependency-change instructions require just bazel-lock-update, and just bazel-lock-check runs bazel mod deps --lockfile_mode=error, so CI will fail on this lockfile drift until the regenerated MODULE.bazel.lock is committed with this Cargo.toml/Cargo.lock change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I checked this with Bazelisk from the repo root:
bazel mod deps --lockfile_mode=updatebazel mod deps --lockfile_mode=error
No MODULE.bazel.lock diff was produced, and the error-mode check passes. This dependency edge is a first-party workspace crate dependency, so there does not appear to be lockfile drift to commit here.
| if let Some(sandbox_log_attachment) = | ||
| windows_sandbox_log_attachment(&self.config.codex_home) |
There was a problem hiding this comment.
On Windows, consenting to upload logs now also sends windows-sandbox.log, but the TUI consent dialog still says “The following files will be sent” and only lists codex-logs.log, the doctor report, rollouts, and diagnostics (codex-rs/tui/src/bottom_pane/feedback_view.rs lines 500-528). In the /feedback flow, users who choose “Yes” therefore upload an extra daily sandbox log without seeing it in the consent file list, so this attachment should be surfaced in that consent UI before being added here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in eee9b2add9: the TUI consent copy now says the listed files “may be sent” and includes windows-sandbox.log (Windows only, if present). I also updated the affected TUI snapshots.
| if let Some(sandbox_log_attachment) = | ||
| windows_sandbox_log_attachment(&self.config.codex_home) |
There was a problem hiding this comment.
When a Windows user has run sandboxed commands in another Codex thread or process earlier the same UTC day, this attaches the shared daily $codex_home/.sandbox/sandbox.YYYY-MM-DD.log resolved only from codex_home; the sandbox logger writes command previews from all launches into that one file, with no thread/session partition. A feedback upload for one conversation can therefore include commands and paths from unrelated sessions, so the attachment should be scoped or filtered to the session being reported before adding it automatically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a real limitation, but it matches the existing privacy/scope model for other feedback logs such as desktop app logs: they are app-wide diagnostics, not conversation-scoped artifacts. The sandbox log currently has no thread/session identifier to filter on, so truly scoping this would require adding new correlation data to the sandbox log writer and then filtering during feedback assembly.
For this PR, I’m treating the sandbox log as another app-wide diagnostic attachment and relying on the now-updated consent UI disclosure. We can do session-scoped sandbox logs as a separate follow-up if we decide that is required.
iceweasel-oai
force-pushed
the
codex/windows-sandbox-feedback-log
branch
3 times, most recently
from
693a515 to
d978e74
Compare
iceweasel-oai
force-pushed
the
codex/windows-sandbox-feedback-log
branch
from
d978e74 to
8e9d82a
Compare
iceweasel-oai
force-pushed
the
codex/windows-sandbox-feedback-log
branch
from
8e9d82a to
9a30e47
Compare
2 participants
Why
Windows sandbox diagnostics are currently hard to recover from
/feedbackeven though they are often the most useful artifact when debugging sandbox behavior. Now that sandbox logging uses daily rolling files, feedback can safely include the current day's sandbox log without uploading the old ever-growing legacysandbox.log.What changed
codex-windows-sandboxhelper that resolves the current daily sandbox log fromcodex_home.windows-sandbox.log, independent of the dated on-disk filename.extra_log_filesbehavior unchanged for rollout and desktop log attachments.Verification
cargo fmt -p codex-app-server -p codex-windows-sandboxcargo test -p codex-windows-sandbox current_log_file_path_for_codex_home_uses_sandbox_dircargo test -p codex-app-server windows_sandbox_log_attachment_uses_current_log/feedbacktest confirmed Sentry receivedwindows-sandbox.log.