mirror of
https://github.com/openai/codex.git
synced 2026-09-15 12:08:01 +00:00
Merge d2cb604b7d into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -8,9 +8,9 @@ pushd "$SCRIPT_DIR/.." >> /dev/null || {
|
||||
echo "Error: Failed to change directory to $SCRIPT_DIR/.."
|
||||
exit 1
|
||||
}
|
||||
npm install
|
||||
npm run build
|
||||
pnpm install
|
||||
pnpm run build
|
||||
rm -rf ./dist/openai-codex-*.tgz
|
||||
npm pack --pack-destination ./dist
|
||||
pnpm pack --pack-destination ./dist
|
||||
mv ./dist/openai-codex-*.tgz ./dist/codex.tgz
|
||||
docker build -t codex -f "./Dockerfile" .
|
||||
|
||||
@@ -470,15 +470,8 @@ export default function TerminalChatInput({
|
||||
setInput("");
|
||||
|
||||
try {
|
||||
// Dynamically import dependencies to avoid unnecessary bundle size
|
||||
const [{ default: open }, os] = await Promise.all([
|
||||
import("open"),
|
||||
import("node:os"),
|
||||
]);
|
||||
|
||||
// Lazy import CLI_VERSION to avoid circular deps
|
||||
const os = await import("node:os");
|
||||
const { CLI_VERSION } = await import("../../utils/session.js");
|
||||
|
||||
const { buildBugReportUrl } = await import(
|
||||
"../../utils/bug-report.js"
|
||||
);
|
||||
@@ -492,10 +485,6 @@ export default function TerminalChatInput({
|
||||
.join(" | "),
|
||||
});
|
||||
|
||||
// Open the URL in the user's default browser
|
||||
await open(url, { wait: false });
|
||||
|
||||
// Inform the user in the chat history
|
||||
setItems((prev) => [
|
||||
...prev,
|
||||
{
|
||||
@@ -505,13 +494,13 @@ export default function TerminalChatInput({
|
||||
content: [
|
||||
{
|
||||
type: "input_text",
|
||||
text: "📋 Opened browser to file a bug report. Please include any context that might help us fix the issue!",
|
||||
text: `🔗 Bug report URL: ${url}`,
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
} catch (error) {
|
||||
// If anything went wrong, notify the user
|
||||
// If anything went wrong, notify the user.
|
||||
setItems((prev) => [
|
||||
...prev,
|
||||
{
|
||||
|
||||
@@ -31,6 +31,7 @@ import DiffOverlay from "../diff-overlay.js";
|
||||
import HelpOverlay from "../help-overlay.js";
|
||||
import HistoryOverlay from "../history-overlay.js";
|
||||
import ModelOverlay from "../model-overlay.js";
|
||||
import chalk from "chalk";
|
||||
import { Box, Text } from "ink";
|
||||
import { spawn } from "node:child_process";
|
||||
import OpenAI from "openai";
|
||||
@@ -575,7 +576,7 @@ export default function TerminalChat({
|
||||
providers={config.providers}
|
||||
currentProvider={provider}
|
||||
hasLastResponse={Boolean(lastResponseId)}
|
||||
onSelect={(newModel) => {
|
||||
onSelect={(allModels, newModel) => {
|
||||
log(
|
||||
"TerminalChat: interruptAgent invoked – calling agent.cancel()",
|
||||
);
|
||||
@@ -585,6 +586,20 @@ export default function TerminalChat({
|
||||
agent?.cancel();
|
||||
setLoading(false);
|
||||
|
||||
if (!allModels?.includes(newModel)) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
chalk.bold.red(
|
||||
`Model "${chalk.yellow(
|
||||
newModel,
|
||||
)}" is not available for provider "${chalk.yellow(
|
||||
provider,
|
||||
)}".`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
setModel(newModel);
|
||||
setLastResponseId((prev) =>
|
||||
prev && newModel !== model ? null : prev,
|
||||
|
||||
@@ -53,7 +53,8 @@ export default function HelpOverlay({
|
||||
<Text color="cyan">/clearhistory</Text> – clear command history
|
||||
</Text>
|
||||
<Text>
|
||||
<Text color="cyan">/bug</Text> – file a bug report with session log
|
||||
<Text color="cyan">/bug</Text> – generate a prefilled GitHub issue URL
|
||||
with session log
|
||||
</Text>
|
||||
<Text>
|
||||
<Text color="cyan">/diff</Text> – view working tree git diff
|
||||
|
||||
@@ -19,7 +19,7 @@ type Props = {
|
||||
currentProvider?: string;
|
||||
hasLastResponse: boolean;
|
||||
providers?: Record<string, { name: string; baseURL: string; envKey: string }>;
|
||||
onSelect: (model: string) => void;
|
||||
onSelect: (allModels: Array<string>, model: string) => void;
|
||||
onSelectProvider?: (provider: string) => void;
|
||||
onExit: () => void;
|
||||
};
|
||||
@@ -153,7 +153,12 @@ export default function ModelOverlay({
|
||||
}
|
||||
initialItems={items}
|
||||
currentValue={currentModel}
|
||||
onSelect={onSelect}
|
||||
onSelect={() =>
|
||||
onSelect(
|
||||
items?.map((m) => m.value),
|
||||
currentModel,
|
||||
)
|
||||
}
|
||||
onExit={onExit}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -23,7 +23,10 @@ export const SLASH_COMMANDS: Array<SlashCommand> = [
|
||||
{ command: "/help", description: "Show list of commands" },
|
||||
{ command: "/model", description: "Open model selection panel" },
|
||||
{ command: "/approval", description: "Open approval mode selection panel" },
|
||||
{ command: "/bug", description: "Generate a prefilled GitHub bug report" },
|
||||
{
|
||||
command: "/bug",
|
||||
description: "Generate a prefilled GitHub issue URL with session log",
|
||||
},
|
||||
{
|
||||
command: "/diff",
|
||||
description:
|
||||
|
||||
130
codex-cli/tests/terminal-chat-model-selection.test.tsx
Normal file
130
codex-cli/tests/terminal-chat-model-selection.test.tsx
Normal file
@@ -0,0 +1,130 @@
|
||||
/* eslint-disable no-console */
|
||||
import { renderTui } from "./ui-test-helpers.js";
|
||||
import React from "react";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import chalk from "chalk";
|
||||
import ModelOverlay from "src/components/model-overlay.js";
|
||||
|
||||
// Mock the necessary dependencies
|
||||
vi.mock("../src/utils/logger/log.js", () => ({
|
||||
log: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("chalk", () => ({
|
||||
default: {
|
||||
bold: {
|
||||
red: vi.fn((msg) => `[bold-red]${msg}[/bold-red]`),
|
||||
},
|
||||
yellow: vi.fn((msg) => `[yellow]${msg}[/yellow]`),
|
||||
},
|
||||
}));
|
||||
|
||||
describe("Model Selection Error Handling", () => {
|
||||
// Create a console.error spy with proper typing
|
||||
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should display error with chalk formatting when selecting unavailable model", () => {
|
||||
// Setup
|
||||
const allModels = ["gpt-4", "gpt-3.5-turbo"];
|
||||
const currentModel = "gpt-4";
|
||||
const unavailableModel = "gpt-invalid";
|
||||
const currentProvider = "openai";
|
||||
|
||||
renderTui(
|
||||
<ModelOverlay
|
||||
currentModel={currentModel}
|
||||
providers={{ openai: { name: "OpenAI", baseURL: "", envKey: "test" } }}
|
||||
currentProvider={currentProvider}
|
||||
hasLastResponse={false}
|
||||
onSelect={(models, newModel) => {
|
||||
if (!models?.includes(newModel)) {
|
||||
console.error(
|
||||
chalk.bold.red(
|
||||
`Model "${chalk.yellow(
|
||||
newModel,
|
||||
)}" is not available for provider "${chalk.yellow(
|
||||
currentProvider,
|
||||
)}".`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
}}
|
||||
onSelectProvider={() => {}}
|
||||
onExit={() => {}}
|
||||
/>,
|
||||
);
|
||||
|
||||
const onSelectHandler = vi.fn((models, newModel) => {
|
||||
if (!models?.includes(newModel)) {
|
||||
console.error(
|
||||
chalk.bold.red(
|
||||
`Model "${chalk.yellow(
|
||||
newModel,
|
||||
)}" is not available for provider "${chalk.yellow(
|
||||
currentProvider,
|
||||
)}".`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
});
|
||||
|
||||
onSelectHandler(allModels, unavailableModel);
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
expect(chalk.bold.red).toHaveBeenCalled();
|
||||
expect(chalk.yellow).toHaveBeenCalledWith(unavailableModel);
|
||||
expect(chalk.yellow).toHaveBeenCalledWith(currentProvider);
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
`[bold-red]Model "[yellow]${unavailableModel}[/yellow]" is not available for provider "[yellow]${currentProvider}[/yellow]".[/bold-red]`,
|
||||
);
|
||||
});
|
||||
|
||||
it("should not proceed with model change when model is unavailable", () => {
|
||||
const mockSetModel = vi.fn();
|
||||
const mockSetLastResponseId = vi.fn();
|
||||
const mockSaveConfig = vi.fn();
|
||||
const mockSetItems = vi.fn();
|
||||
const mockSetOverlayMode = vi.fn();
|
||||
|
||||
const onSelectHandler = vi.fn((allModels, newModel) => {
|
||||
if (!allModels?.includes(newModel)) {
|
||||
console.error(
|
||||
chalk.bold.red(
|
||||
`Model "${chalk.yellow(
|
||||
newModel,
|
||||
)}" is not available for provider "${chalk.yellow("openai")}".`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
mockSetModel(newModel);
|
||||
mockSetLastResponseId(null);
|
||||
mockSaveConfig({});
|
||||
mockSetItems((prev: Array<unknown>) => [...prev, {}]);
|
||||
mockSetOverlayMode("none");
|
||||
});
|
||||
|
||||
onSelectHandler(["gpt-4", "gpt-3.5-turbo"], "gpt-invalid");
|
||||
|
||||
expect(mockSetModel).not.toHaveBeenCalled();
|
||||
expect(mockSetLastResponseId).not.toHaveBeenCalled();
|
||||
expect(mockSaveConfig).not.toHaveBeenCalled();
|
||||
expect(mockSetItems).not.toHaveBeenCalled();
|
||||
expect(mockSetOverlayMode).not.toHaveBeenCalled();
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -868,6 +868,7 @@ async fn handle_function_call(
|
||||
sandbox_type,
|
||||
&roots_snapshot,
|
||||
sess.ctrl_c.clone(),
|
||||
sess.sandbox_policy,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -952,11 +953,14 @@ async fn handle_function_call(
|
||||
|
||||
let retry_roots = { sess.writable_roots.lock().unwrap().clone() };
|
||||
|
||||
// This is an escalated retry; the policy will not be
|
||||
// examined and the sandbox has been set to `None`.
|
||||
let retry_output_result = process_exec_tool_call(
|
||||
params.clone(),
|
||||
SandboxType::None,
|
||||
&retry_roots,
|
||||
sess.ctrl_c.clone(),
|
||||
sess.sandbox_policy,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -15,8 +15,10 @@ use tokio::sync::Notify;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result;
|
||||
use crate::error::SandboxErr;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
|
||||
/// Maximum we keep for each stream (100 KiB).
|
||||
/// TODO(ragona) this should be reduced
|
||||
const MAX_STREAM_OUTPUT: usize = 100 * 1024;
|
||||
|
||||
const DEFAULT_TIMEOUT_MS: u64 = 10_000;
|
||||
@@ -55,8 +57,9 @@ async fn exec_linux(
|
||||
params: ExecParams,
|
||||
writable_roots: &[PathBuf],
|
||||
ctrl_c: Arc<Notify>,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
crate::linux::exec_linux(params, writable_roots, ctrl_c).await
|
||||
crate::linux::exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
@@ -64,6 +67,7 @@ async fn exec_linux(
|
||||
_params: ExecParams,
|
||||
_writable_roots: &[PathBuf],
|
||||
_ctrl_c: Arc<Notify>,
|
||||
_sandbox_policy: SandboxPolicy,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
Err(CodexErr::Io(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
@@ -76,6 +80,7 @@ pub async fn process_exec_tool_call(
|
||||
sandbox_type: SandboxType,
|
||||
writable_roots: &[PathBuf],
|
||||
ctrl_c: Arc<Notify>,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
) -> Result<ExecToolCallOutput> {
|
||||
let start = Instant::now();
|
||||
|
||||
@@ -98,7 +103,9 @@ pub async fn process_exec_tool_call(
|
||||
)
|
||||
.await
|
||||
}
|
||||
SandboxType::LinuxSeccomp => exec_linux(params, writable_roots, ctrl_c).await,
|
||||
SandboxType::LinuxSeccomp => {
|
||||
exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
|
||||
}
|
||||
};
|
||||
let duration = start.elapsed();
|
||||
match raw_output_result {
|
||||
@@ -199,9 +206,17 @@ pub async fn exec(
|
||||
if let Some(dir) = &workdir {
|
||||
cmd.current_dir(dir);
|
||||
}
|
||||
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
|
||||
cmd.kill_on_drop(true);
|
||||
cmd.spawn()?
|
||||
|
||||
// Do not create a file descriptor for stdin because otherwise some
|
||||
// commands may hang forever waiting for input. For example, ripgrep has
|
||||
// a heuristic where it may try to read from stdin as explained here:
|
||||
// https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103
|
||||
cmd.stdin(Stdio::null());
|
||||
|
||||
cmd.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.kill_on_drop(true)
|
||||
.spawn()?
|
||||
};
|
||||
|
||||
let stdout_handle = tokio::spawn(read_capped(
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::error::SandboxErr;
|
||||
use crate::exec::exec;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec::RawExecToolCallOutput;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
|
||||
use landlock::Access;
|
||||
use landlock::AccessFs;
|
||||
@@ -33,6 +34,7 @@ pub async fn exec_linux(
|
||||
params: ExecParams,
|
||||
writable_roots: &[PathBuf],
|
||||
ctrl_c: Arc<Notify>,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
// Allow READ on /
|
||||
// Allow WRITE on /dev/null
|
||||
@@ -47,34 +49,12 @@ pub async fn exec_linux(
|
||||
.expect("Failed to create runtime");
|
||||
|
||||
rt.block_on(async {
|
||||
let abi = ABI::V5;
|
||||
let access_rw = AccessFs::from_all(abi);
|
||||
let access_ro = AccessFs::from_read(abi);
|
||||
|
||||
let mut ruleset = Ruleset::default()
|
||||
.set_compatibility(CompatLevel::BestEffort)
|
||||
.handle_access(access_rw)?
|
||||
.create()?
|
||||
.add_rules(landlock::path_beneath_rules(&["/"], access_ro))?
|
||||
.add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))?
|
||||
.set_no_new_privs(true);
|
||||
|
||||
if !writable_roots_copy.is_empty() {
|
||||
ruleset = ruleset.add_rules(landlock::path_beneath_rules(
|
||||
&writable_roots_copy,
|
||||
access_rw,
|
||||
))?;
|
||||
if sandbox_policy.is_network_restricted() {
|
||||
install_network_seccomp_filter_on_current_thread()?;
|
||||
}
|
||||
|
||||
let status = ruleset.restrict_self()?;
|
||||
|
||||
// TODO(wpt): Probably wanna expand this more generically and not warn every time.
|
||||
if status.ruleset == landlock::RulesetStatus::NotEnforced {
|
||||
return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict));
|
||||
}
|
||||
|
||||
if let Err(e) = install_network_seccomp_filter() {
|
||||
return Err(CodexErr::Sandbox(e));
|
||||
if sandbox_policy.is_file_write_restricted() {
|
||||
install_filesystem_landlock_rules_on_current_thread(writable_roots_copy)?;
|
||||
}
|
||||
|
||||
exec(params, ctrl_c_copy).await
|
||||
@@ -92,7 +72,33 @@ pub async fn exec_linux(
|
||||
}
|
||||
}
|
||||
|
||||
fn install_network_seccomp_filter() -> std::result::Result<(), SandboxErr> {
|
||||
fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec<PathBuf>) -> Result<()> {
|
||||
let abi = ABI::V5;
|
||||
let access_rw = AccessFs::from_all(abi);
|
||||
let access_ro = AccessFs::from_read(abi);
|
||||
|
||||
let mut ruleset = Ruleset::default()
|
||||
.set_compatibility(CompatLevel::BestEffort)
|
||||
.handle_access(access_rw)?
|
||||
.create()?
|
||||
.add_rules(landlock::path_beneath_rules(&["/"], access_ro))?
|
||||
.add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))?
|
||||
.set_no_new_privs(true);
|
||||
|
||||
if !writable_roots.is_empty() {
|
||||
ruleset = ruleset.add_rules(landlock::path_beneath_rules(&writable_roots, access_rw))?;
|
||||
}
|
||||
|
||||
let status = ruleset.restrict_self()?;
|
||||
|
||||
if status.ruleset == landlock::RulesetStatus::NotEnforced {
|
||||
return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> {
|
||||
// Build rule map.
|
||||
let mut rules: BTreeMap<i64, Vec<SeccompRule>> = BTreeMap::new();
|
||||
|
||||
@@ -156,6 +162,7 @@ mod tests_linux {
|
||||
use crate::exec::process_exec_tool_call;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec::SandboxType;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use std::sync::Arc;
|
||||
use tempfile::NamedTempFile;
|
||||
use tokio::sync::Notify;
|
||||
@@ -172,6 +179,7 @@ mod tests_linux {
|
||||
SandboxType::LinuxSeccomp,
|
||||
writable_roots,
|
||||
Arc::new(Notify::new()),
|
||||
SandboxPolicy::NetworkAndFileWriteRestricted,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -238,6 +246,7 @@ mod tests_linux {
|
||||
SandboxType::LinuxSeccomp,
|
||||
&[],
|
||||
Arc::new(Notify::new()),
|
||||
SandboxPolicy::NetworkRestricted,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -100,6 +100,30 @@ pub enum SandboxPolicy {
|
||||
DangerousNoRestrictions,
|
||||
}
|
||||
|
||||
impl SandboxPolicy {
|
||||
pub fn is_dangerous(&self) -> bool {
|
||||
match self {
|
||||
SandboxPolicy::NetworkRestricted => false,
|
||||
SandboxPolicy::FileWriteRestricted => false,
|
||||
SandboxPolicy::NetworkAndFileWriteRestricted => false,
|
||||
SandboxPolicy::DangerousNoRestrictions => true,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_network_restricted(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
SandboxPolicy::NetworkRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
|
||||
)
|
||||
}
|
||||
|
||||
pub fn is_file_write_restricted(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
SandboxPolicy::FileWriteRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
|
||||
)
|
||||
}
|
||||
}
|
||||
/// User input
|
||||
#[non_exhaustive]
|
||||
#[derive(Debug, Clone, Deserialize, Serialize)]
|
||||
|
||||
Reference in New Issue
Block a user