mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use released DotSlash package for argument-comment lint (#15199)
## Why The argument-comment lint now has a packaged DotSlash artifact from [#15198](https://github.com/openai/codex/pull/15198), so the normal repo lint path should use that released payload instead of rebuilding the lint from source every time. That keeps `just clippy` and CI aligned with the shipped artifact while preserving a separate source-build path for people actively hacking on the lint crate. The current alpha package also exposed two integration wrinkles that the repo-side prebuilt wrapper needs to smooth over: - the bundled Dylint library filename includes the host triple, for example `@nightly-2025-09-18-aarch64-apple-darwin`, and Dylint derives `RUSTUP_TOOLCHAIN` from that filename - on Windows, Dylint's driver path also expects `RUSTUP_HOME` to be present in the environment Without those adjustments, the prebuilt CI jobs fail during `cargo metadata` or driver setup. This change makes the checked-in prebuilt wrapper normalize the packaged library name to the plain `nightly-2025-09-18` channel before invoking `cargo-dylint`, and it teaches both the wrapper and the packaged runner source to infer `RUSTUP_HOME` from `rustup show home` when the environment does not already provide it. After the prebuilt Windows lint job started running successfully, it also surfaced a handful of existing anonymous literal callsites in `windows-sandbox-rs`. This PR now annotates those callsites so the new cross-platform lint job is green on the current tree. ## What Changed - checked in the current `tools/argument-comment-lint/argument-comment-lint` DotSlash manifest - kept `tools/argument-comment-lint/run.sh` as the source-build wrapper for lint development - added `tools/argument-comment-lint/run-prebuilt-linter.sh` as the normal enforcement path, using the checked-in DotSlash package and bundled `cargo-dylint` - updated `just clippy` and `just argument-comment-lint` to use the prebuilt wrapper - split `.github/workflows/rust-ci.yml` so source-package checks live in a dedicated `argument_comment_lint_package` job, while the released lint runs in an `argument_comment_lint_prebuilt` matrix on Linux, macOS, and Windows - kept the pinned `nightly-2025-09-18` toolchain install in the prebuilt CI matrix, since the prebuilt package still relies on rustup-provided toolchain components - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` to normalize host-qualified nightly library filenames, keep the `rustup` shim directory ahead of direct toolchain `cargo` binaries, and export `RUSTUP_HOME` when needed for Windows Dylint driver setup - updated `tools/argument-comment-lint/src/bin/argument-comment-lint.rs` so future published DotSlash artifacts apply the same nightly-filename normalization and `RUSTUP_HOME` inference internally - fixed the remaining Windows lint violations in `codex-rs/windows-sandbox-rs` by adding the required `/*param*/` comments at the reported callsites - documented the checked-in DotSlash file, wrapper split, archive layout, nightly prerequisite, and Windows `RUSTUP_HOME` requirement in `tools/argument-comment-lint/README.md`
This commit is contained in:
Michael Bolin
committed by
GitHub
Unverified
GitHub
parent
96a86710c3
commit
fa2a2f0be9
@@ -1363,18 +1363,18 @@ impl App {
|
||||
let windows_sandbox_level = WindowsSandboxLevel::from_config(&self.config);
|
||||
self.app_event_tx.send(AppEvent::CodexOp(
|
||||
AppCommand::override_turn_context(
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
/*cwd*/ None,
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
/*sandbox_policy*/ None,
|
||||
#[cfg(target_os = "windows")]
|
||||
Some(windows_sandbox_level),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
)
|
||||
.into_core(),
|
||||
));
|
||||
@@ -3785,7 +3785,7 @@ impl App {
|
||||
Ok(()) => {
|
||||
session_telemetry.counter(
|
||||
"codex.windows_sandbox.elevated_setup_success",
|
||||
1,
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
AppEvent::EnableWindowsSandboxForAgentMode {
|
||||
@@ -3815,7 +3815,7 @@ impl App {
|
||||
codex_core::windows_sandbox::elevated_setup_failure_metric_name(
|
||||
&err,
|
||||
),
|
||||
1,
|
||||
/*inc*/ 1,
|
||||
&tags,
|
||||
);
|
||||
tracing::error!(
|
||||
@@ -3856,7 +3856,7 @@ impl App {
|
||||
) {
|
||||
session_telemetry.counter(
|
||||
"codex.windows_sandbox.legacy_setup_preflight_failed",
|
||||
1,
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tracing::warn!(
|
||||
@@ -3881,7 +3881,7 @@ impl App {
|
||||
self.chat_widget
|
||||
.add_to_history(history_cell::new_info_event(
|
||||
format!("Granting sandbox read access to {path} ..."),
|
||||
None,
|
||||
/*hint*/ None,
|
||||
));
|
||||
|
||||
let policy = self.config.permissions.sandbox_policy.get().clone();
|
||||
@@ -3956,11 +3956,13 @@ impl App {
|
||||
match builder.apply().await {
|
||||
Ok(()) => {
|
||||
if elevated_enabled {
|
||||
self.config.set_windows_sandbox_enabled(false);
|
||||
self.config.set_windows_elevated_sandbox_enabled(true);
|
||||
self.config.set_windows_sandbox_enabled(/*value*/ false);
|
||||
self.config
|
||||
.set_windows_elevated_sandbox_enabled(/*value*/ true);
|
||||
} else {
|
||||
self.config.set_windows_sandbox_enabled(true);
|
||||
self.config.set_windows_elevated_sandbox_enabled(false);
|
||||
self.config.set_windows_sandbox_enabled(/*value*/ true);
|
||||
self.config
|
||||
.set_windows_elevated_sandbox_enabled(/*value*/ false);
|
||||
}
|
||||
self.chat_widget.set_windows_sandbox_mode(
|
||||
self.config.permissions.windows_sandbox_mode,
|
||||
@@ -3972,18 +3974,18 @@ impl App {
|
||||
{
|
||||
self.app_event_tx.send(AppEvent::CodexOp(
|
||||
AppCommand::override_turn_context(
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
/*cwd*/ None,
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
/*sandbox_policy*/ None,
|
||||
#[cfg(target_os = "windows")]
|
||||
Some(windows_sandbox_level),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
)
|
||||
.into(),
|
||||
));
|
||||
@@ -3998,18 +4000,18 @@ impl App {
|
||||
} else {
|
||||
self.app_event_tx.send(AppEvent::CodexOp(
|
||||
AppCommand::override_turn_context(
|
||||
None,
|
||||
/*cwd*/ None,
|
||||
Some(preset.approval),
|
||||
Some(self.config.approvals_reviewer),
|
||||
Some(preset.sandbox.clone()),
|
||||
#[cfg(target_os = "windows")]
|
||||
Some(windows_sandbox_level),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
)
|
||||
.into(),
|
||||
));
|
||||
|
||||
@@ -4699,7 +4699,7 @@ impl ChatWidget {
|
||||
|
||||
self.session_telemetry.counter(
|
||||
"codex.windows_sandbox.setup_elevated_sandbox_command",
|
||||
1,
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
self.app_event_tx
|
||||
@@ -8707,8 +8707,11 @@ impl ChatWidget {
|
||||
return;
|
||||
}
|
||||
|
||||
self.session_telemetry
|
||||
.counter("codex.windows_sandbox.elevated_prompt_shown", 1, &[]);
|
||||
self.session_telemetry.counter(
|
||||
"codex.windows_sandbox.elevated_prompt_shown",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
|
||||
let mut header = ColumnRenderable::new();
|
||||
header.push(*Box::new(
|
||||
@@ -8727,7 +8730,11 @@ impl ChatWidget {
|
||||
name: "Set up default sandbox (requires Administrator permissions)".to_string(),
|
||||
description: None,
|
||||
actions: vec![Box::new(move |tx| {
|
||||
accept_otel.counter("codex.windows_sandbox.elevated_prompt_accept", 1, &[]);
|
||||
accept_otel.counter(
|
||||
"codex.windows_sandbox.elevated_prompt_accept",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::BeginWindowsSandboxElevatedSetup {
|
||||
preset: preset.clone(),
|
||||
});
|
||||
@@ -8739,7 +8746,11 @@ impl ChatWidget {
|
||||
name: "Use non-admin sandbox (higher risk if prompt injected)".to_string(),
|
||||
description: None,
|
||||
actions: vec![Box::new(move |tx| {
|
||||
legacy_otel.counter("codex.windows_sandbox.elevated_prompt_use_legacy", 1, &[]);
|
||||
legacy_otel.counter(
|
||||
"codex.windows_sandbox.elevated_prompt_use_legacy",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::BeginWindowsSandboxLegacySetup {
|
||||
preset: legacy_preset.clone(),
|
||||
});
|
||||
@@ -8751,7 +8762,11 @@ impl ChatWidget {
|
||||
name: "Quit".to_string(),
|
||||
description: None,
|
||||
actions: vec![Box::new(move |tx| {
|
||||
quit_otel.counter("codex.windows_sandbox.elevated_prompt_quit", 1, &[]);
|
||||
quit_otel.counter(
|
||||
"codex.windows_sandbox.elevated_prompt_quit",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::Exit(ExitMode::ShutdownFirst));
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
@@ -8801,7 +8816,11 @@ impl ChatWidget {
|
||||
let otel = self.session_telemetry.clone();
|
||||
let preset = elevated_preset;
|
||||
move |tx| {
|
||||
otel.counter("codex.windows_sandbox.fallback_retry_elevated", 1, &[]);
|
||||
otel.counter(
|
||||
"codex.windows_sandbox.fallback_retry_elevated",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::BeginWindowsSandboxElevatedSetup {
|
||||
preset: preset.clone(),
|
||||
});
|
||||
@@ -8817,7 +8836,11 @@ impl ChatWidget {
|
||||
let otel = self.session_telemetry.clone();
|
||||
let preset = legacy_preset;
|
||||
move |tx| {
|
||||
otel.counter("codex.windows_sandbox.fallback_use_legacy", 1, &[]);
|
||||
otel.counter(
|
||||
"codex.windows_sandbox.fallback_use_legacy",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::BeginWindowsSandboxLegacySetup {
|
||||
preset: preset.clone(),
|
||||
});
|
||||
@@ -8830,7 +8853,11 @@ impl ChatWidget {
|
||||
name: "Quit".to_string(),
|
||||
description: None,
|
||||
actions: vec![Box::new(move |tx| {
|
||||
quit_otel.counter("codex.windows_sandbox.fallback_prompt_quit", 1, &[]);
|
||||
quit_otel.counter(
|
||||
"codex.windows_sandbox.fallback_prompt_quit",
|
||||
/*inc*/ 1,
|
||||
&[],
|
||||
);
|
||||
tx.send(AppEvent::Exit(ExitMode::ShutdownFirst));
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
@@ -8870,11 +8897,12 @@ impl ChatWidget {
|
||||
// While elevated sandbox setup runs, prevent typing so the user doesn't
|
||||
// accidentally queue messages that will run under an unexpected mode.
|
||||
self.bottom_pane.set_composer_input_enabled(
|
||||
false,
|
||||
/*enabled*/ false,
|
||||
Some("Input disabled until setup completes.".to_string()),
|
||||
);
|
||||
self.bottom_pane.ensure_status_indicator();
|
||||
self.bottom_pane.set_interrupt_hint_visible(false);
|
||||
self.bottom_pane
|
||||
.set_interrupt_hint_visible(/*visible*/ false);
|
||||
self.set_status(
|
||||
"Setting up sandbox...".to_string(),
|
||||
Some("Hang tight, this may take a few minutes".to_string()),
|
||||
@@ -8890,7 +8918,8 @@ impl ChatWidget {
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub(crate) fn clear_windows_sandbox_setup_status(&mut self) {
|
||||
self.bottom_pane.set_composer_input_enabled(true, None);
|
||||
self.bottom_pane
|
||||
.set_composer_input_enabled(/*enabled*/ true, /*placeholder*/ None);
|
||||
self.bottom_pane.hide_status_indicator();
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
@@ -1003,8 +1003,10 @@ async fn enter_with_only_remote_images_does_not_submit_when_input_disabled() {
|
||||
|
||||
let remote_url = "https://example.com/remote-only.png".to_string();
|
||||
chat.set_remote_image_urls(vec![remote_url.clone()]);
|
||||
chat.bottom_pane
|
||||
.set_composer_input_enabled(false, Some("Input disabled for test.".to_string()));
|
||||
chat.bottom_pane.set_composer_input_enabled(
|
||||
/*enabled*/ false,
|
||||
Some("Input disabled for test.".to_string()),
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
|
||||
@@ -352,7 +352,7 @@ mod tests {
|
||||
StatusDetailsCapitalization::CapitalizeFirst,
|
||||
STATUS_DETAILS_DEFAULT_MAX_LINES,
|
||||
);
|
||||
w.set_interrupt_hint_visible(false);
|
||||
w.set_interrupt_hint_visible(/*visible*/ false);
|
||||
|
||||
// Freeze time-dependent rendering (elapsed + spinner) to keep the snapshot stable.
|
||||
w.is_paused = true;
|
||||
|
||||
Reference in New Issue
Block a user