Skip to content

app server request/response structure for sandbox nux#11792

Closed
iceweasel-oai wants to merge 1 commit intomainfrom
app-server-sandbox-nux
Closed

app server request/response structure for sandbox nux#11792
iceweasel-oai wants to merge 1 commit intomainfrom
app-server-sandbox-nux

Conversation

@iceweasel-oai
Copy link
Collaborator

No description provided.

@iceweasel-oai
Copy link
Collaborator Author

@codex review

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96557c4854

ℹ️ 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".

Comment on lines +564 to +579
if WindowsSandboxLevel::from_config(&config) == WindowsSandboxLevel::Elevated {
self.outgoing
.send_response(
request_id,
WindowsSandboxNuxStartResponse { accepted: false },
)
.await;
return;
}

self.outgoing
.send_response(
request_id.clone(),
WindowsSandboxNuxStartResponse { accepted: true },
)
.await;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Honor elevated NUX kill switch in app-server start flow

windows_sandbox_nux_start only rejects when sandbox mode is already elevated, then starts the elevated NUX flow unconditionally. It never checks ELEVATED_SANDBOX_NUX_ENABLED (documented as a global kill switch in core/src/windows_sandbox.rs). If that switch is set false for emergency rollback, app-server still runs the new flow instead of reverting behavior.

Useful? React with 👍 / 👎.

Comment on lines +596 to +603
tokio::spawn(async move {
let mut host = AppServerWindowsSandboxNuxHost {
outgoing,
surface: protocol_surface,
};
let outcome =
codex_core::windows_sandbox_nux::run_windows_sandbox_nux(&mut host, run_params)
.await;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent concurrent windowsSandbox/nuxStart workflows

Each windowsSandbox/nuxStart call spawns a detached NUX task with no in-flight guard or deduplication. Repeated requests can run multiple prompt/setup loops concurrently on the same connection, interleave windowsSandbox/nuxPrompt requests, and race config persistence. This can lead to inconsistent user actions and setup state.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant