diff --git a/announcement_tip.toml b/announcement_tip.toml index 2473258368..79c0daea76 100644 --- a/announcement_tip.toml +++ b/announcement_tip.toml @@ -4,20 +4,14 @@ # version_regex matches against the CLI version (env!("CARGO_PKG_VERSION")); omit to apply to all versions. # target_app specify which app should display the announcement (cli, vsce, ...). -[[announcements]] -content = "Welcome to Codex! Check out the new onboarding flow." -from_date = "2024-10-01" -to_date = "2024-10-15" -target_app = "cli" - -# Test announcement only for local build version until 2026-01-10 excluded (past) +# Test announcement only for local build version until 2027-05-10 excluded [[announcements]] content = "This is a test announcement" version_regex = "^0\\.0\\.0$" -to_date = "2026-05-10" +to_date = "2027-05-10" [[announcements]] -content = "**BREAKING NEWS**: `gpt-5.3-codex` is out! Upgrade to `0.98.0` for a faster, smarter, more steerable agent." -from_date = "2026-02-01" -to_date = "2026-02-16" -version_regex = "^0\\.(?:[0-9]|[1-8][0-9]|9[0-7])\\." +content = "Update Required - This version will no longer be supported starting May 8th. Please upgrade to the latest version (https://github.com/openai/codex/releases/latest) using your preferred package manager." +# Matches 0.x.y versions from 0.0.y through 0.119.y; excludes 0.120.0 and newer. +version_regex = "^0\\.(?:[0-9]|[1-9][0-9]|1[01][0-9])\\." +to_date = "2026-05-08" diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5e54b772c1..68e0ec092f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1461,6 +1461,7 @@ dependencies = [ "codex-sandboxing", "codex-shell-command", "codex-state", + "codex-thread-store", "codex-tools", "codex-utils-absolute-path", "codex-utils-cargo-bin", @@ -1938,6 +1939,7 @@ dependencies = [ "codex-state", "codex-terminal-detection", "codex-test-binary-support", + "codex-thread-store", "codex-tools", "codex-utils-absolute-path", "codex-utils-cache", @@ -2841,9 +2843,17 @@ version = "0.0.0" dependencies = [ "async-trait", "chrono", + "codex-git-utils", "codex-protocol", + "codex-rollout", + "codex-state", + "pretty_assertions", "serde", + "serde_json", + "tempfile", "thiserror 2.0.18", + "tokio", + "uuid", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index b57a464d5e..4bb8837211 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -166,6 +166,7 @@ codex-state = { path = "state" } codex-stdio-to-uds = { path = "stdio-to-uds" } codex-terminal-detection = { path = "terminal-detection" } codex-test-binary-support = { path = "test-binary-support" } +codex-thread-store = { path = "thread-store" } codex-tools = { path = "tools" } codex-tui = { path = "tui" } codex-utils-absolute-path = { path = "utils/absolute-path" } diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index eb3d984965..35f9068712 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -28,6 +28,7 @@ use std::time::Duration; pub use codex_app_server::in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; pub use codex_app_server::in_process::InProcessServerEvent; use codex_app_server::in_process::InProcessStartArgs; +use codex_app_server::in_process::LogDbLayer; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; @@ -354,6 +355,8 @@ pub struct InProcessClientStartArgs { pub cloud_requirements: CloudRequirementsLoader, /// Feedback sink used by app-server/core telemetry and logs. pub feedback: CodexFeedback, + /// SQLite tracing layer used to flush recently emitted logs before feedback upload. + pub log_db: Option, /// Environment manager used by core execution and filesystem operations. pub environment_manager: Arc, /// Startup warnings emitted after initialize succeeds. @@ -405,6 +408,7 @@ impl InProcessClientStartArgs { loader_overrides: self.loader_overrides, cloud_requirements: self.cloud_requirements, feedback: self.feedback, + log_db: self.log_db, environment_manager: self.environment_manager, config_warnings: self.config_warnings, session_source: self.session_source, @@ -982,6 +986,7 @@ mod tests { loader_overrides: LoaderOverrides::default(), cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), + log_db: None, environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), config_warnings: Vec::new(), session_source, @@ -1994,6 +1999,7 @@ mod tests { loader_overrides: LoaderOverrides::default(), cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), + log_db: None, environment_manager: environment_manager.clone(), config_warnings: Vec::new(), session_source: SessionSource::Exec, diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index e5287e1c65..9941054c7e 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -253,6 +262,199 @@ } ] }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "NetworkApprovalContext": { "properties": { "host": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index ac8d5c4010..d05a6d332f 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,199 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "RequestPermissionProfile": { "additionalProperties": false, "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json index 7b0c2b1a3b..eb3c65283a 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,199 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "GrantedPermissionProfile": { "properties": { "fileSystem": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index b31e69f203..f6c2908806 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -586,6 +595,199 @@ ], "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "McpElicitationArrayType": { "enum": [ "array" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index d0ce368b78..3e0674bee6 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/v2/AbsolutePathBuf" @@ -2179,6 +2188,199 @@ "title": "FileChangeRequestApprovalResponse", "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/v2/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "FuzzyFileSearchMatchType": { "enum": [ "file", diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts new file mode 100644 index 0000000000..ccaa07a8bf --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts @@ -0,0 +1,12 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Access mode for a filesystem entry. + * + * When two equally specific entries target the same path, we compare these by + * conflict precedence rather than by capability breadth: `none` beats + * `write`, and `write` beats `read`. + */ +export type FileSystemAccessMode = "read" | "write" | "none"; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts new file mode 100644 index 0000000000..3b022f1744 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "./AbsolutePathBuf"; +import type { FileSystemSpecialPath } from "./FileSystemSpecialPath"; + +export type FileSystemPath = { "type": "path", path: AbsolutePathBuf, } | { "type": "special", value: FileSystemSpecialPath, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts new file mode 100644 index 0000000000..f37cd0d63e --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { FileSystemAccessMode } from "./FileSystemAccessMode"; +import type { FileSystemPath } from "./FileSystemPath"; + +export type FileSystemSandboxEntry = { path: FileSystemPath, access: FileSystemAccessMode, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts new file mode 100644 index 0000000000..8ba7b433a6 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "current_working_directory" } | { "kind": "project_roots", subpath?: string, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath?: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 7bbb417fdc..50277418a3 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -16,6 +16,10 @@ export type { ExecCommandApprovalParams } from "./ExecCommandApprovalParams"; export type { ExecCommandApprovalResponse } from "./ExecCommandApprovalResponse"; export type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type { FileChange } from "./FileChange"; +export type { FileSystemAccessMode } from "./FileSystemAccessMode"; +export type { FileSystemPath } from "./FileSystemPath"; +export type { FileSystemSandboxEntry } from "./FileSystemSandboxEntry"; +export type { FileSystemSpecialPath } from "./FileSystemSpecialPath"; export type { ForcedLoginMethod } from "./ForcedLoginMethod"; export type { FunctionCallOutputBody } from "./FunctionCallOutputBody"; export type { FunctionCallOutputContentItem } from "./FunctionCallOutputContentItem"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts index 0ed8a1b12f..be84d3a093 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; +import type { FileSystemSandboxEntry } from "../FileSystemSandboxEntry"; -export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, }; +export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, entries: Array | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 3d6bf9d872..509482613a 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -2039,6 +2039,7 @@ mod tests { file_system: Some(v2::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }), }), proposed_execpolicy_amendment: None, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 5897625de1..249001aeac 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -46,6 +46,9 @@ use codex_protocol::openai_models::ModelAvailabilityNux as CoreModelAvailability use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::default_input_modalities; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; +use codex_protocol::permissions::FileSystemAccessMode as CoreFileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath as CoreFileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry as CoreFileSystemSandboxEntry; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; use codex_protocol::protocol::AgentStatus as CoreAgentStatus; @@ -1129,22 +1132,46 @@ impl From for NetworkApprovalContext { pub struct AdditionalFileSystemPermissions { pub read: Option>, pub write: Option>, + pub entries: Option>, } impl From for AdditionalFileSystemPermissions { fn from(value: CoreFileSystemPermissions) -> Self { + let entries = value + .entries + .iter() + .any(|entry| { + entry.access == CoreFileSystemAccessMode::None + || !matches!(entry.path, CoreFileSystemPath::Path { .. }) + }) + .then_some(value.entries.clone()); + let read = value + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == CoreFileSystemAccessMode::Read).then_some(path.clone()) + }) + .collect::>(); + let write = value + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == CoreFileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); Self { - read: value.read, - write: value.write, + read: (!read.is_empty()).then_some(read), + write: (!write.is_empty()).then_some(write), + entries, } } } impl From for CoreFileSystemPermissions { fn from(value: AdditionalFileSystemPermissions) -> Self { - Self { - read: value.read, - write: value.write, + match value.entries { + Some(entries) if !entries.is_empty() => CoreFileSystemPermissions { entries }, + Some(_) | None => { + CoreFileSystemPermissions::from_read_write_roots(value.read, value.write) + } } } } @@ -6727,6 +6754,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6737,16 +6765,16 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } @@ -6820,6 +6848,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6830,20 +6859,102 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } + #[test] + fn additional_file_system_permissions_empty_entries_fall_back_to_read_write_roots() { + let read_only_path = if cfg!(windows) { + r"C:\tmp\read-only" + } else { + "/tmp/read-only" + }; + let read_write_path = if cfg!(windows) { + r"C:\tmp\read-write" + } else { + "/tmp/read-write" + }; + + assert_eq!( + CoreFileSystemPermissions::from(AdditionalFileSystemPermissions { + read: Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) + .expect("path must be absolute"), + ]), + write: Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) + .expect("path must be absolute"), + ]), + entries: Some(Vec::new()), + }), + CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) + .expect("path must be absolute"), + ]), + Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) + .expect("path must be absolute"), + ]), + ) + ); + } + + #[test] + fn additional_permission_profile_preserves_canonical_file_system_entries() { + let deny_path = if cfg!(windows) { + r"C:\tmp\secret.txt" + } else { + "/tmp/secret.txt" + }; + let file_system = CoreFileSystemPermissions { + entries: vec![ + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Root, + }, + access: CoreFileSystemAccessMode::Write, + }, + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Path { + path: AbsolutePathBuf::try_from(PathBuf::from(deny_path)) + .expect("path must be absolute"), + }, + access: CoreFileSystemAccessMode::None, + }, + ], + }; + let permissions = CorePermissionProfile { + file_system: Some(file_system.clone()), + ..Default::default() + }; + + let additional_permissions = AdditionalPermissionProfile::from(permissions.clone()); + assert_eq!( + additional_permissions.file_system, + Some(AdditionalFileSystemPermissions { + read: None, + write: None, + entries: Some(file_system.entries), + }) + ); + assert_eq!( + CorePermissionProfile::from(additional_permissions), + permissions + ); + } + #[test] fn permissions_request_approval_response_defaults_scope_to_turn() { let response = serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 5dc3a31485..408f39fafe 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -54,6 +54,7 @@ codex-rmcp-client = { workspace = true } codex-rollout = { workspace = true } codex-sandboxing = { workspace = true } codex-state = { workspace = true } +codex-thread-store = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-json-to-toml = { workspace = true } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 4fddee4a11..7a4be202f5 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -118,6 +118,8 @@ use codex_protocol::ThreadId; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem as CoreDynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse as CoreDynamicToolResponse; use codex_protocol::items::parse_hook_prompt_message; +use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; +use codex_protocol::models::PermissionProfile as CorePermissionProfile; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo; use codex_protocol::protocol::Event; @@ -138,7 +140,6 @@ use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequest use codex_protocol::request_permissions::RequestPermissionsResponse as CoreRequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; -use codex_sandboxing::policy_transforms::intersect_permission_profiles; use codex_shell_command::parse_command::shlex_join; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; @@ -2566,9 +2567,12 @@ async fn on_request_permissions_response( let response = receiver.await; resolve_server_request_on_thread_listener(&thread_state, pending_request_id).await; drop(request_permissions_guard); - let Some(response) = - request_permissions_response_from_client_result(requested_permissions, response) - else { + let cwd = conversation.cwd().await; + let Some(response) = request_permissions_response_from_client_result( + requested_permissions, + cwd.as_path(), + response, + ) else { return; }; @@ -2585,6 +2589,7 @@ async fn on_request_permissions_response( fn request_permissions_response_from_client_result( requested_permissions: CoreRequestPermissionProfile, + cwd: &Path, response: std::result::Result, ) -> Option { let value = match response { @@ -2615,15 +2620,41 @@ fn request_permissions_response_from_client_result( } }); Some(CoreRequestPermissionsResponse { - permissions: intersect_permission_profiles( - requested_permissions.into(), - response.permissions.into(), - ) - .into(), + permissions: granted_request_permissions_within_requested_scope( + requested_permissions, + CorePermissionProfile::from(response.permissions).into(), + cwd, + ), scope: response.scope.to_core(), }) } +fn granted_request_permissions_within_requested_scope( + requested: CoreRequestPermissionProfile, + granted: CoreRequestPermissionProfile, + cwd: &Path, +) -> CoreRequestPermissionProfile { + let requested: CorePermissionProfile = requested.into(); + let granted: CorePermissionProfile = granted.into(); + let requested_file_system_policy = requested.file_system_sandbox_policy(); + CorePermissionProfile { + network: granted.network.filter(|network| { + requested.network_sandbox_policy().is_enabled() && network.enabled.unwrap_or(false) + }), + file_system: granted + .file_system + .map(|file_system| CoreFileSystemPermissions { + entries: file_system + .entries + .into_iter() + .filter(|entry| requested_file_system_policy.covers_entry_with_cwd(entry, cwd)) + .collect(), + }) + .filter(|file_system| !file_system.is_empty()), + } + .into() +} + const REVIEW_FALLBACK_MESSAGE: &str = "Reviewer failed to output a response."; fn render_review_output_text(output: &ReviewOutputEvent) -> String { @@ -3673,6 +3704,7 @@ mod tests { let response = request_permissions_response_from_client_result( CoreRequestPermissionProfile::default(), + Path::new("/tmp"), Ok(Err(error)), ); @@ -3703,10 +3735,10 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), }; let cases = vec![ ( @@ -3733,10 +3765,10 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: None, - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), @@ -3751,10 +3783,10 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), @@ -3763,6 +3795,7 @@ mod tests { for (granted_permissions, expected_permissions) in cases { let response = request_permissions_response_from_client_result( requested_permissions.clone(), + Path::new("/tmp"), Ok(Ok(serde_json::json!({ "permissions": granted_permissions, }))), @@ -3783,6 +3816,7 @@ mod tests { fn request_permissions_response_preserves_session_scope() { let response = request_permissions_response_from_client_result( CoreRequestPermissionProfile::default(), + Path::new("/tmp"), Ok(Ok(serde_json::json!({ "scope": "session", "permissions": {}, @@ -3799,6 +3833,94 @@ mod tests { ); } + #[test] + fn request_permissions_response_accepts_explicit_child_grant_for_requested_cwd_scope() { + let cwd = TempDir::new().expect("create temp dir"); + let child = + AbsolutePathBuf::try_from(cwd.path().join("child.txt")).expect("absolute child path"); + let requested_permissions = CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions { + entries: vec![codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }], + }), + ..CoreRequestPermissionProfile::default() + }; + + let response = request_permissions_response_from_client_result( + requested_permissions, + cwd.path(), + Ok(Ok(serde_json::json!({ + "permissions": { + "fileSystem": { + "write": [child], + }, + }, + }))), + ) + .expect("response should be accepted"); + + assert_eq!( + response, + CoreRequestPermissionsResponse { + permissions: CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..CoreRequestPermissionProfile::default() + }, + scope: CorePermissionGrantScope::Turn, + } + ); + } + + #[test] + fn request_permissions_response_ignores_broader_cwd_grant_for_requested_child_path() { + let cwd = TempDir::new().expect("create temp dir"); + let child = + AbsolutePathBuf::try_from(cwd.path().join("child.txt")).expect("absolute child path"); + let requested_permissions = CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..CoreRequestPermissionProfile::default() + }; + + let response = request_permissions_response_from_client_result( + requested_permissions, + cwd.path(), + Ok(Ok(serde_json::json!({ + "permissions": { + "fileSystem": { + "entries": [{ + "path": { + "type": "special", + "value": { + "kind": "current_working_directory", + }, + }, + "access": "write", + }], + }, + }, + }))), + ) + .expect("response should be accepted"); + + assert_eq!( + response, + CoreRequestPermissionsResponse { + permissions: CoreRequestPermissionProfile::default(), + scope: CorePermissionGrantScope::Turn, + } + ); + } + #[test] fn collab_resume_begin_maps_to_item_started_resume_agent() { let event = CollabResumeBeginEvent { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 513837c7ce..05ce2d7974 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -203,7 +203,6 @@ use codex_chatgpt::connectors; use codex_cloud_requirements::cloud_requirements_loader; use codex_config::types::McpServerTransportConfig; use codex_core::CodexThread; -use codex_core::Cursor as RolloutCursor; use codex_core::ForkSnapshot; use codex_core::NewThread; use codex_core::RolloutRecorder; @@ -211,7 +210,6 @@ use codex_core::SessionMeta; use codex_core::SteerInputError; use codex_core::ThreadConfigSnapshot; use codex_core::ThreadManager; -use codex_core::ThreadSortKey as CoreThreadSortKey; use codex_core::append_thread_name; use codex_core::config::Config; use codex_core::config::ConfigOverrides; @@ -232,7 +230,6 @@ use codex_core::find_archived_thread_path_by_id_str; use codex_core::find_thread_name_by_id; use codex_core::find_thread_names_by_ids; use codex_core::find_thread_path_by_id_str; -use codex_core::parse_cursor; use codex_core::path_utils; use codex_core::plugins::MarketplaceAddError; use codex_core::plugins::MarketplaceError; @@ -322,6 +319,12 @@ use codex_state::StateRuntime; use codex_state::ThreadMetadata; use codex_state::ThreadMetadataBuilder; use codex_state::log_db::LogDbLayer; +use codex_thread_store::ListThreadsParams as StoreListThreadsParams; +use codex_thread_store::LocalThreadStore; +use codex_thread_store::StoredThread; +use codex_thread_store::ThreadSortKey as StoreThreadSortKey; +use codex_thread_store::ThreadStore; +use codex_thread_store::ThreadStoreError; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_json_to_toml::json_to_toml; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; @@ -3819,15 +3822,15 @@ impl CodexMessageProcessor { .map(|value| value as usize) .unwrap_or(THREAD_LIST_DEFAULT_LIMIT) .clamp(1, THREAD_LIST_MAX_LIMIT); - let core_sort_key = match sort_key.unwrap_or(ThreadSortKey::CreatedAt) { - ThreadSortKey::CreatedAt => CoreThreadSortKey::CreatedAt, - ThreadSortKey::UpdatedAt => CoreThreadSortKey::UpdatedAt, + let store_sort_key = match sort_key.unwrap_or(ThreadSortKey::CreatedAt) { + ThreadSortKey::CreatedAt => StoreThreadSortKey::CreatedAt, + ThreadSortKey::UpdatedAt => StoreThreadSortKey::UpdatedAt, }; let (summaries, next_cursor) = match self .list_threads_common( requested_page_size, cursor, - core_sort_key, + store_sort_key, ThreadListFilters { model_providers, source_kinds, @@ -5071,7 +5074,7 @@ impl CodexMessageProcessor { &self, requested_page_size: usize, cursor: Option, - sort_key: CoreThreadSortKey, + sort_key: StoreThreadSortKey, filters: ThreadListFilters, ) -> Result<(Vec, Option), JSONRPCErrorError> { let ThreadListFilters { @@ -5081,16 +5084,7 @@ impl CodexMessageProcessor { cwd, search_term, } = filters; - let mut cursor_obj: Option = match cursor.as_ref() { - Some(cursor_str) => { - Some(parse_cursor(cursor_str).ok_or_else(|| JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid cursor: {cursor_str}"), - data: None, - })?) - } - None => None, - }; + let mut cursor_obj = cursor; let mut last_cursor = cursor_obj.clone(); let mut remaining = requested_page_size; let mut items = Vec::with_capacity(requested_page_size); @@ -5109,54 +5103,26 @@ impl CodexMessageProcessor { let fallback_provider = self.config.model_provider_id.clone(); let (allowed_sources_vec, source_kind_filter) = compute_source_filters(source_kinds); let allowed_sources = allowed_sources_vec.as_slice(); - let state_db_ctx = get_state_db(&self.config).await; + let store = LocalThreadStore::new(codex_rollout::RolloutConfig::from_view(&self.config)); while remaining > 0 { let page_size = remaining.min(THREAD_LIST_MAX_LIMIT); - let page = if archived { - RolloutRecorder::list_archived_threads( - &self.config, + let page = store + .list_threads(StoreListThreadsParams { page_size, - cursor_obj.as_ref(), + cursor: cursor_obj.clone(), sort_key, - allowed_sources, - model_provider_filter.as_deref(), - fallback_provider.as_str(), - search_term.as_deref(), - ) + allowed_sources: allowed_sources.to_vec(), + model_providers: model_provider_filter.clone(), + archived, + search_term: search_term.clone(), + }) .await - .map_err(|err| JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to list threads: {err}"), - data: None, - })? - } else { - RolloutRecorder::list_threads( - &self.config, - page_size, - cursor_obj.as_ref(), - sort_key, - allowed_sources, - model_provider_filter.as_deref(), - fallback_provider.as_str(), - search_term.as_deref(), - ) - .await - .map_err(|err| JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to list threads: {err}"), - data: None, - })? - }; + .map_err(thread_store_list_error)?; let mut filtered = Vec::with_capacity(page.items.len()); for it in page.items { - let Some(summary) = summary_from_thread_list_item( - it, - fallback_provider.as_str(), - state_db_ctx.as_ref(), - ) - .await + let Some(summary) = summary_from_stored_thread(it, fallback_provider.as_str()) else { continue; }; @@ -5176,12 +5142,8 @@ impl CodexMessageProcessor { items.extend(filtered); remaining = requested_page_size.saturating_sub(items.len()); - // Encode RolloutCursor into the JSON-RPC string form returned to clients. let next_cursor_value = page.next_cursor.clone(); - next_cursor = next_cursor_value - .as_ref() - .and_then(|cursor| serde_json::to_value(cursor).ok()) - .and_then(|value| value.as_str().map(str::to_owned)); + next_cursor = next_cursor_value.clone(); if remaining == 0 { break; } @@ -9421,67 +9383,52 @@ fn set_thread_name_from_title(thread: &mut Thread, title: String) { thread.name = Some(title); } -async fn summary_from_thread_list_item( - it: codex_core::ThreadItem, - fallback_provider: &str, - state_db_ctx: Option<&StateDbHandle>, -) -> Option { - if let Some(thread_id) = it.thread_id { - let timestamp = it.created_at.clone(); - let updated_at = it.updated_at.clone().or_else(|| timestamp.clone()); - let model_provider = it - .model_provider - .clone() - .unwrap_or_else(|| fallback_provider.to_string()); - let cwd = it.cwd?; - let cli_version = it.cli_version.unwrap_or_default(); - let source = with_thread_spawn_agent_metadata( - it.source - .unwrap_or(codex_protocol::protocol::SessionSource::Unknown), - it.agent_nickname.clone(), - it.agent_role.clone(), - ); - return Some(ConversationSummary { - conversation_id: thread_id, - path: it.path, - preview: it.first_user_message.unwrap_or_default(), - timestamp, - updated_at, - model_provider, - cwd, - cli_version, - source, - git_info: if it.git_sha.is_none() - && it.git_branch.is_none() - && it.git_origin_url.is_none() - { - None - } else { - Some(ConversationGitInfo { - sha: it.git_sha, - branch: it.git_branch, - origin_url: it.git_origin_url, - }) - }, - }); +fn thread_store_list_error(err: ThreadStoreError) -> JSONRPCErrorError { + match err { + ThreadStoreError::InvalidRequest { message } => JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message, + data: None, + }, + err => JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!("failed to list threads: {err}"), + data: None, + }, } - if let Some(thread_id) = thread_id_from_rollout_path(it.path.as_path()) { - return read_summary_from_state_db_context_by_thread_id(state_db_ctx, thread_id).await; - } - None } -fn thread_id_from_rollout_path(path: &Path) -> Option { - let file_name = path.file_name()?.to_str()?; - let stem = file_name.strip_suffix(".jsonl")?; - if stem.len() < 37 { - return None; - } - let uuid_start = stem.len().saturating_sub(36); - if !stem[..uuid_start].ends_with('-') { - return None; - } - ThreadId::from_string(&stem[uuid_start..]).ok() +fn summary_from_stored_thread( + thread: StoredThread, + fallback_provider: &str, +) -> Option { + let path = thread.rollout_path?; + let source = with_thread_spawn_agent_metadata( + thread.source, + thread.agent_nickname.clone(), + thread.agent_role.clone(), + ); + let git_info = thread.git_info.map(|git| ConversationGitInfo { + sha: git.commit_hash.map(|sha| sha.0), + branch: git.branch, + origin_url: git.repository_url, + }); + Some(ConversationSummary { + conversation_id: thread.thread_id, + path, + preview: thread.first_user_message.unwrap_or(thread.preview), + timestamp: Some(thread.created_at.to_rfc3339_opts(SecondsFormat::Secs, true)), + updated_at: Some(thread.updated_at.to_rfc3339_opts(SecondsFormat::Secs, true)), + model_provider: if thread.model_provider.is_empty() { + fallback_provider.to_string() + } else { + thread.model_provider + }, + cwd: thread.cwd, + cli_version: thread.cli_version, + source, + git_info, + }) } #[allow(clippy::too_many_arguments)] diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 9ac0e6cf42..84dfd282cd 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -82,6 +82,7 @@ use codex_exec_server::EnvironmentManager; use codex_feedback::CodexFeedback; use codex_login::AuthManager; use codex_protocol::protocol::SessionSource; +pub use codex_state::log_db::LogDbLayer; use tokio::sync::mpsc; use tokio::sync::oneshot; use tokio::time::timeout; @@ -117,6 +118,8 @@ pub struct InProcessStartArgs { pub cloud_requirements: CloudRequirementsLoader, /// Feedback sink used by app-server/core telemetry and logs. pub feedback: CodexFeedback, + /// SQLite tracing layer used to flush recently emitted logs before feedback upload. + pub log_db: Option, /// Environment manager used by core execution and filesystem operations. pub environment_manager: Arc, /// Startup warnings emitted after initialize succeeds. @@ -395,7 +398,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { loader_overrides: args.loader_overrides, cloud_requirements: args.cloud_requirements, feedback: args.feedback, - log_db: None, + log_db: args.log_db, config_warnings: args.config_warnings, session_source: args.session_source, auth_manager, @@ -728,6 +731,7 @@ mod tests { loader_overrides: LoaderOverrides::default(), cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), + log_db: None, environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), config_warnings: Vec::new(), session_source, diff --git a/codex-rs/app-server/src/transport/mod.rs b/codex-rs/app-server/src/transport/mod.rs index 75a4971905..a71fd0b6f1 100644 --- a/codex-rs/app-server/src/transport/mod.rs +++ b/codex-rs/app-server/src/transport/mod.rs @@ -780,6 +780,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, @@ -842,6 +843,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, @@ -868,7 +870,8 @@ mod tests { "network": null, "fileSystem": { "read": [allowed_path], - "write": null, + "write": null, + "entries": null, }, }) ); diff --git a/codex-rs/app-server/src/transport/remote_control/protocol.rs b/codex-rs/app-server/src/transport/remote_control/protocol.rs index 857855f2a0..dcb79e5c35 100644 --- a/codex-rs/app-server/src/transport/remote_control/protocol.rs +++ b/codex-rs/app-server/src/transport/remote_control/protocol.rs @@ -48,9 +48,8 @@ pub enum ClientEvent { message: JSONRPCMessage, }, /// Backend-generated acknowledgement for all server envelopes addressed to - /// `client_id` whose envelope `seq_id` is less than or equal to this ack's - /// `seq_id`. This cursor is client-scoped, not stream-scoped, so receivers - /// must not use `stream_id` to partition acks. + /// `client_id` and `stream_id` whose envelope `seq_id` is less than or equal + /// to this ack's `seq_id`. This cursor is stream-scoped. Ack, Ping, ClientClosed, @@ -65,7 +64,7 @@ pub(crate) struct ClientEnvelope { pub(crate) client_id: ClientId, #[serde(rename = "stream_id", skip_serializing_if = "Option::is_none")] pub(crate) stream_id: Option, - /// For `Ack`, this is the backend-generated per-client cursor over + /// For `Ack`, this is the backend-generated per-stream cursor over /// `ServerEnvelope.seq_id`. #[serde(rename = "seq_id", skip_serializing_if = "Option::is_none")] pub(crate) seq_id: Option, diff --git a/codex-rs/app-server/src/transport/remote_control/tests.rs b/codex-rs/app-server/src/transport/remote_control/tests.rs index 6dd6978fa5..2dd687a3d1 100644 --- a/codex-rs/app-server/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server/src/transport/remote_control/tests.rs @@ -5,6 +5,7 @@ use super::enroll::update_persisted_remote_control_enrollment; use super::protocol::ClientEnvelope; use super::protocol::ClientEvent; use super::protocol::ClientId; +use super::protocol::StreamId; use super::protocol::normalize_remote_control_url; use super::websocket::REMOTE_CONTROL_PROTOCOL_VERSION; use super::*; @@ -163,7 +164,7 @@ async fn remote_control_transport_manages_virtual_clients_and_routes_messages() json!({ "type": "pong", "client_id": "client-1", - "seq_id": 0, + "seq_id": 1, "status": "unknown", }) ); @@ -369,7 +370,7 @@ async fn remote_control_transport_manages_virtual_clients_and_routes_messages() json!({ "type": "pong", "client_id": "client-1", - "seq_id": 3, + "seq_id": 1, "status": "unknown", }) ); @@ -618,12 +619,13 @@ async fn remote_control_transport_clears_outgoing_buffer_when_backend_acks() { )) .await .expect("remote writer should accept outgoing message"); + let (server_event, stream_id) = read_server_event_with_stream_id(&mut first_websocket).await; assert_eq!( - read_server_event(&mut first_websocket).await, + server_event, json!({ "type": "server_message", "client_id": "client-1", - "seq_id": 0, + "seq_id": 1, "message": { "method": "configWarning", "params": { @@ -639,8 +641,8 @@ async fn remote_control_transport_clears_outgoing_buffer_when_backend_acks() { ClientEnvelope { event: ClientEvent::Ack, client_id: client_id.clone(), - stream_id: None, - seq_id: Some(0), + stream_id: Some(stream_id), + seq_id: Some(1), cursor: None, }, ) @@ -854,7 +856,7 @@ async fn remote_control_http_mode_enrolls_before_connecting() { json!({ "type": "server_message", "client_id": backend_client_id.0.clone(), - "seq_id": 0, + "seq_id": 1, "message": { "id": 11, "result": { @@ -882,7 +884,7 @@ async fn remote_control_http_mode_enrolls_before_connecting() { json!({ "type": "server_message", "client_id": backend_client_id.0.clone(), - "seq_id": 1, + "seq_id": 2, "message": { "method": "configWarning", "params": { @@ -1356,6 +1358,12 @@ async fn send_client_event( } async fn read_server_event(websocket: &mut WebSocketStream) -> serde_json::Value { + read_server_event_with_stream_id(websocket).await.0 +} + +async fn read_server_event_with_stream_id( + websocket: &mut WebSocketStream, +) -> (serde_json::Value, StreamId) { loop { let frame = timeout(Duration::from_secs(5), websocket.next()) .await @@ -1366,13 +1374,15 @@ async fn read_server_event(websocket: &mut WebSocketStream) -> serde_ tungstenite::Message::Text(text) => { let mut event: serde_json::Value = serde_json::from_str(text.as_ref()).expect("server event should deserialize"); - if let Some(stream_id) = event + let stream_id = event .as_object_mut() .and_then(|event| event.remove("stream_id")) - { - assert!(stream_id.is_string(), "stream_id should be a string"); - } - return event; + .expect("stream_id should be present"); + let stream_id = stream_id + .as_str() + .expect("stream_id should be a string") + .to_string(); + return (event, StreamId(stream_id)); } tungstenite::Message::Ping(payload) => { websocket diff --git a/codex-rs/app-server/src/transport/remote_control/websocket.rs b/codex-rs/app-server/src/transport/remote_control/websocket.rs index d168ec0e18..82e5f35b33 100644 --- a/codex-rs/app-server/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server/src/transport/remote_control/websocket.rs @@ -14,6 +14,7 @@ use super::protocol::ClientEvent; use super::protocol::ClientId; use super::protocol::RemoteControlTarget; use super::protocol::ServerEnvelope; +use super::protocol::StreamId; use axum::http::HeaderValue; use base64::Engine; use codex_core::util::backoff; @@ -25,8 +26,8 @@ use futures::SinkExt; use futures::StreamExt; use futures::stream::SplitSink; use futures::stream::SplitStream; -use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::VecDeque; use std::io; use std::io::ErrorKind; use std::sync::Arc; @@ -57,10 +58,7 @@ const REMOTE_CONTROL_ACCOUNT_ID_RETRY_INTERVAL: std::time::Duration = std::time::Duration::from_secs(1); struct BoundedOutboundBuffer { - // Remote-control acks are generated by the backend at client scope, so - // retransmit retention is keyed by client_id only. stream_id stays on each - // envelope for routing, but it is not part of the ack cursor. - buffer_by_client: HashMap>, + buffer_by_stream: HashMap<(ClientId, StreamId), VecDeque>, used_tx: watch::Sender, } @@ -68,46 +66,50 @@ impl BoundedOutboundBuffer { fn new() -> (Self, watch::Receiver) { let (used_tx, used_rx) = watch::channel(0); let buffer = Self { - buffer_by_client: HashMap::new(), + buffer_by_stream: HashMap::new(), used_tx, }; (buffer, used_rx) } fn insert(&mut self, server_envelope: &ServerEnvelope) { - self.buffer_by_client - .entry(server_envelope.client_id.clone()) + self.buffer_by_stream + .entry(( + server_envelope.client_id.clone(), + server_envelope.stream_id.clone(), + )) .or_default() - .insert(server_envelope.seq_id, server_envelope.clone()); + .push_back(server_envelope.clone()); self.used_tx.send_modify(|used| *used += 1); } - fn ack(&mut self, client_id: &ClientId, acked_seq_id: u64) { - let Some(buffer) = self.buffer_by_client.get_mut(client_id) else { + fn ack(&mut self, client_id: &ClientId, stream_id: &StreamId, acked_seq_id: u64) { + let key = (client_id.clone(), stream_id.clone()); + let Some(buffer) = self.buffer_by_stream.get_mut(&key) else { return; }; - while let Some(seq_id) = buffer.first_key_value().map(|(seq_id, _)| seq_id) - && *seq_id <= acked_seq_id + while let Some(server_envelope) = buffer.front() + && server_envelope.seq_id <= acked_seq_id { - buffer.pop_first(); + buffer.pop_front(); self.used_tx.send_modify(|used| *used -= 1); } if buffer.is_empty() { - self.buffer_by_client.remove(client_id); + self.buffer_by_stream.remove(&key); } } fn server_envelopes(&self) -> impl Iterator { - self.buffer_by_client + self.buffer_by_stream .values() - .flat_map(|buffer| buffer.values()) + .flat_map(|buffer| buffer.iter()) } } struct WebsocketState { outbound_buffer: BoundedOutboundBuffer, subscribe_cursor: Option, - next_seq_id: u64, + next_seq_id_by_stream: HashMap<(ClientId, StreamId), u64>, } pub(crate) struct RemoteControlWebsocket { @@ -162,7 +164,7 @@ impl RemoteControlWebsocket { state: Arc::new(Mutex::new(WebsocketState { outbound_buffer, subscribe_cursor: None, - next_seq_id: 0, + next_seq_id_by_stream: HashMap::new(), })), server_event_rx: Arc::new(Mutex::new(server_event_rx)), used_rx, @@ -445,10 +447,16 @@ impl RemoteControlWebsocket { } } }; - let (server_envelope, write_complete_tx) = { + let (payload, write_complete_tx) = { let mut state = state.lock().await; - let seq_id = state.next_seq_id; - state.next_seq_id = state.next_seq_id.saturating_add(1); + let seq_key = ( + queued_server_envelope.client_id.clone(), + queued_server_envelope.stream_id.clone(), + ); + let seq_id = *state + .next_seq_id_by_stream + .entry(seq_key.clone()) + .or_insert(1); let server_envelope = ServerEnvelope { event: queued_server_envelope.event, @@ -456,17 +464,19 @@ impl RemoteControlWebsocket { seq_id, stream_id: queued_server_envelope.stream_id, }; + let payload = match serde_json::to_string(&server_envelope) { + Ok(payload) => payload, + Err(err) => { + error!("failed to serialize remote-control server event: {err}"); + continue; + } + }; + state + .next_seq_id_by_stream + .insert(seq_key, seq_id.saturating_add(1)); state.outbound_buffer.insert(&server_envelope); - (server_envelope, queued_server_envelope.write_complete_tx) - }; - - let payload = match serde_json::to_string(&server_envelope) { - Ok(payload) => payload, - Err(err) => { - error!("failed to serialize remote-control server event: {err}"); - continue; - } + (payload, queued_server_envelope.write_complete_tx) }; tokio::select! { @@ -590,10 +600,13 @@ impl RemoteControlWebsocket { } if let ClientEvent::Ack = &client_envelope.event && let Some(acked_seq_id) = client_envelope.seq_id + && let Some(stream_id) = client_envelope.stream_id.as_ref() { - websocket_state - .outbound_buffer - .ack(&client_envelope.client_id, acked_seq_id); + websocket_state.outbound_buffer.ack( + &client_envelope.client_id, + stream_id, + acked_seq_id, + ); } drop(websocket_state); @@ -1220,7 +1233,7 @@ mod tests { let state = Arc::new(Mutex::new(WebsocketState { outbound_buffer, subscribe_cursor: None, - next_seq_id: 0, + next_seq_id_by_stream: HashMap::new(), })); let (_server_event_tx, server_event_rx) = mpsc::channel(super::super::CHANNEL_CAPACITY); let server_event_rx = Arc::new(Mutex::new(server_event_rx)); @@ -1248,6 +1261,83 @@ mod tests { .expect("writer should stop cleanly"); } + #[tokio::test] + async fn run_server_writer_inner_assigns_contiguous_seq_ids_per_stream() { + let (client_stream, mut server_stream) = connected_websocket_pair().await; + let (websocket_writer, _websocket_reader) = client_stream.split(); + let (outbound_buffer, used_rx) = BoundedOutboundBuffer::new(); + let state = Arc::new(Mutex::new(WebsocketState { + outbound_buffer, + subscribe_cursor: None, + next_seq_id_by_stream: HashMap::new(), + })); + let (server_event_tx, server_event_rx) = mpsc::channel(super::super::CHANNEL_CAPACITY); + let server_event_rx = Arc::new(Mutex::new(server_event_rx)); + let shutdown_token = CancellationToken::new(); + let writer_task = tokio::spawn(RemoteControlWebsocket::run_server_writer_inner( + state, + server_event_rx, + used_rx, + websocket_writer, + Duration::from_secs(60), + shutdown_token.clone(), + )); + + let client_id = ClientId("client-1".to_string()); + let first_stream = StreamId("stream-1".to_string()); + let second_stream = StreamId("stream-2".to_string()); + for stream_id in [&first_stream, &second_stream, &first_stream] { + server_event_tx + .send(super::super::QueuedServerEnvelope { + event: ServerEvent::Pong { + status: crate::transport::remote_control::protocol::PongStatus::Active, + }, + client_id: client_id.clone(), + stream_id: stream_id.clone(), + write_complete_tx: None, + }) + .await + .expect("server event should queue"); + } + + assert_eq!( + read_server_text_event(&mut server_stream).await, + serde_json::json!({ + "type": "pong", + "client_id": "client-1", + "stream_id": "stream-1", + "seq_id": 1, + "status": "active", + }) + ); + assert_eq!( + read_server_text_event(&mut server_stream).await, + serde_json::json!({ + "type": "pong", + "client_id": "client-1", + "stream_id": "stream-2", + "seq_id": 1, + "status": "active", + }) + ); + assert_eq!( + read_server_text_event(&mut server_stream).await, + serde_json::json!({ + "type": "pong", + "client_id": "client-1", + "stream_id": "stream-1", + "seq_id": 2, + "status": "active", + }) + ); + + shutdown_token.cancel(); + writer_task + .await + .expect("writer task should join") + .expect("writer should stop cleanly"); + } + #[tokio::test] async fn run_websocket_reader_inner_times_out_without_pong_frames() { let (client_stream, _server_stream) = connected_websocket_pair().await; @@ -1256,7 +1346,7 @@ mod tests { let state = Arc::new(Mutex::new(WebsocketState { outbound_buffer, subscribe_cursor: None, - next_seq_id: 0, + next_seq_id_by_stream: HashMap::new(), })); let (server_event_tx, _server_event_rx) = mpsc::channel(super::super::CHANNEL_CAPACITY); let (transport_event_tx, _transport_event_rx) = @@ -1287,70 +1377,32 @@ mod tests { } #[test] - fn outbound_buffer_acks_by_client_id_across_stream_ids() { + fn outbound_buffer_acks_by_stream_id() { let (mut outbound_buffer, used_rx) = BoundedOutboundBuffer::new(); let client_1 = ClientId("client-1".to_string()); let client_2 = ClientId("client-2".to_string()); + let stream_1 = StreamId("stream-1".to_string()); outbound_buffer.insert(&server_envelope( &client_1, "stream-1", - /*seq_id*/ 0, + /*seq_id*/ 1, "first-client-old-stream", )); outbound_buffer.insert(&server_envelope( &client_2, "stream-1", - /*seq_id*/ 1, + /*seq_id*/ 2, "second-client", )); outbound_buffer.insert(&server_envelope( &client_1, "stream-2", - /*seq_id*/ 2, + /*seq_id*/ 3, "first-client-new-stream", )); - outbound_buffer.ack(&client_1, /*acked_seq_id*/ 2); - - let mut retained = outbound_buffer - .server_envelopes() - .map(|server_envelope| { - ( - server_envelope.client_id.0.as_str(), - server_envelope.stream_id.0.as_str(), - server_envelope.seq_id, - ) - }) - .collect::>(); - retained.sort_unstable(); - assert_eq!(retained, vec![("client-2", "stream-1", 1)]); - assert_eq!(*used_rx.borrow(), 1); - } - - #[test] - fn outbound_buffer_retains_unacked_messages_until_ack_advances() { - let (mut outbound_buffer, used_rx) = BoundedOutboundBuffer::new(); - let client_1 = ClientId("client-1".to_string()); - let client_2 = ClientId("client-2".to_string()); - - outbound_buffer.insert(&server_envelope( - &client_1, - "stream-1", - /*seq_id*/ 0, - "first-old", - )); - outbound_buffer.insert(&server_envelope( - &client_1, - "stream-2", - /*seq_id*/ 1, - "first-new", - )); - outbound_buffer.insert(&server_envelope( - &client_2, "stream-1", /*seq_id*/ 2, "second", - )); - - outbound_buffer.ack(&client_1, /*acked_seq_id*/ 0); + outbound_buffer.ack(&client_1, &stream_1, /*acked_seq_id*/ 3); let mut retained = outbound_buffer .server_envelopes() @@ -1365,7 +1417,50 @@ mod tests { retained.sort_unstable(); assert_eq!( retained, - vec![("client-1", "stream-2", 1), ("client-2", "stream-1", 2)] + vec![("client-1", "stream-2", 3), ("client-2", "stream-1", 2)] + ); + assert_eq!(*used_rx.borrow(), 2); + } + + #[test] + fn outbound_buffer_retains_unacked_messages_until_ack_advances() { + let (mut outbound_buffer, used_rx) = BoundedOutboundBuffer::new(); + let client_1 = ClientId("client-1".to_string()); + let client_2 = ClientId("client-2".to_string()); + let stream_1 = StreamId("stream-1".to_string()); + + outbound_buffer.insert(&server_envelope( + &client_1, + "stream-1", + /*seq_id*/ 1, + "first-old", + )); + outbound_buffer.insert(&server_envelope( + &client_1, + "stream-2", + /*seq_id*/ 2, + "first-new", + )); + outbound_buffer.insert(&server_envelope( + &client_2, "stream-1", /*seq_id*/ 3, "second", + )); + + outbound_buffer.ack(&client_1, &stream_1, /*acked_seq_id*/ 1); + + let mut retained = outbound_buffer + .server_envelopes() + .map(|server_envelope| { + ( + server_envelope.client_id.0.as_str(), + server_envelope.stream_id.0.as_str(), + server_envelope.seq_id, + ) + }) + .collect::>(); + retained.sort_unstable(); + assert_eq!( + retained, + vec![("client-1", "stream-2", 2), ("client-2", "stream-1", 3)] ); assert_eq!(*used_rx.borrow(), 2); } @@ -1450,6 +1545,20 @@ mod tests { (client_stream, server_stream) } + async fn read_server_text_event( + server_stream: &mut WebSocketStream, + ) -> serde_json::Value { + let message = timeout(Duration::from_secs(5), server_stream.next()) + .await + .expect("server event should arrive in time") + .expect("server websocket should stay open") + .expect("server event should read"); + let tungstenite::Message::Text(text) = message else { + panic!("expected text event, got {message:?}"); + }; + serde_json::from_str(text.as_ref()).expect("server event should deserialize") + } + async fn respond_with_status_and_headers( mut stream: TcpStream, status: &str, diff --git a/codex-rs/app-server/tests/suite/v2/request_permissions.rs b/codex-rs/app-server/tests/suite/v2/request_permissions.rs index 5a0679415d..05a45011de 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -93,6 +93,7 @@ async fn request_permissions_round_trip() -> Result<()> { file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { read: None, write: Some(vec![requested_writes[0].clone()]), + entries: None, }), }, scope: PermissionGrantScope::Turn, diff --git a/codex-rs/app-server/tests/suite/v2/thread_list.rs b/codex-rs/app-server/tests/suite/v2/thread_list.rs index cf1ac4ff11..8fd4e47303 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_list.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_list.rs @@ -563,15 +563,34 @@ sqlite = true /*git_info*/ None, )?; - // `thread/list` only applies `search_term` on the sqlite path. In this test we - // create rollouts manually, so we must also create the sqlite DB and mark backfill - // complete; otherwise app-server will permanently use filesystem fallback. + // `thread/list` applies `search_term` on the sqlite fast path. This test creates + // rollouts manually, so mark the DB backfill complete and then run an unsearched + // list large enough to repair every rollout the searched list should find. let state_db = codex_state::StateRuntime::init(codex_home.path().to_path_buf(), "mock_provider".into()) .await?; state_db .mark_backfill_complete(/*last_watermark*/ None) .await?; + let rollout_config = codex_rollout::RolloutConfig { + codex_home: codex_home.path().to_path_buf(), + sqlite_home: codex_home.path().to_path_buf(), + cwd: codex_home.path().to_path_buf(), + model_provider_id: "mock_provider".to_string(), + generate_memories: false, + }; + let repaired_page = codex_core::RolloutRecorder::list_threads( + &rollout_config, + /*page_size*/ 10, + /*cursor*/ None, + codex_core::ThreadSortKey::CreatedAt, + &[], + /*model_providers*/ None, + "mock_provider", + /*search_term*/ None, + ) + .await?; + assert_eq!(repaired_page.items.len(), 3); let mut mcp = init_mcp(codex_home.path()).await?; let request_id = mcp diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index 0ad2b13979..7c36c67fdd 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -12,6 +12,7 @@ use codex_protocol::error::RetryLimitReachedError; use codex_protocol::error::UnexpectedResponseError; use codex_protocol::error::UsageLimitReachedError; use http::HeaderMap; +use http::HeaderValue; use serde::Deserialize; use serde_json::Value; @@ -200,11 +201,16 @@ impl CoreAuthProvider { } impl ApiAuthProvider for CoreAuthProvider { - fn bearer_token(&self) -> Option { - self.token.clone() - } - - fn account_id(&self) -> Option { - self.account_id.clone() + fn add_auth_headers(&self, headers: &mut HeaderMap) { + if let Some(token) = self.token.as_ref() + && let Ok(header) = HeaderValue::from_str(&format!("Bearer {token}")) + { + let _ = headers.insert(http::header::AUTHORIZATION, header); + } + if let Some(account_id) = self.account_id.as_ref() + && let Ok(header) = HeaderValue::from_str(account_id) + { + let _ = headers.insert("ChatGPT-Account-ID", header); + } } } diff --git a/codex-rs/codex-api/src/api_bridge_tests.rs b/codex-rs/codex-api/src/api_bridge_tests.rs index 71d3889915..50247c1314 100644 --- a/codex-rs/codex-api/src/api_bridge_tests.rs +++ b/codex-rs/codex-api/src/api_bridge_tests.rs @@ -141,3 +141,24 @@ fn core_auth_provider_reports_when_auth_header_will_attach() { assert!(auth.auth_header_attached()); assert_eq!(auth.auth_header_name(), Some("authorization")); } + +#[test] +fn core_auth_provider_adds_auth_headers() { + let auth = CoreAuthProvider::for_test(Some("access-token"), Some("workspace-123")); + let mut headers = HeaderMap::new(); + + crate::AuthProvider::add_auth_headers(&auth, &mut headers); + + assert_eq!( + headers + .get(http::header::AUTHORIZATION) + .and_then(|value| value.to_str().ok()), + Some("Bearer access-token") + ); + assert_eq!( + headers + .get("ChatGPT-Account-ID") + .and_then(|value| value.to_str().ok()), + Some("workspace-123") + ); +} diff --git a/codex-rs/codex-api/src/auth.rs b/codex-rs/codex-api/src/auth.rs index f649062db1..a7b1e69d14 100644 --- a/codex-rs/codex-api/src/auth.rs +++ b/codex-rs/codex-api/src/auth.rs @@ -1,33 +1,10 @@ -use codex_client::Request; use http::HeaderMap; -use http::HeaderValue; -/// Provides bearer and account identity information for API requests. +/// Adds authentication headers to API requests. /// /// Implementations should be cheap and non-blocking; any asynchronous /// refresh or I/O should be handled by higher layers before requests /// reach this interface. pub trait AuthProvider: Send + Sync { - fn bearer_token(&self) -> Option; - fn account_id(&self) -> Option { - None - } -} - -pub(crate) fn add_auth_headers_to_header_map(auth: &A, headers: &mut HeaderMap) { - if let Some(token) = auth.bearer_token() - && let Ok(header) = HeaderValue::from_str(&format!("Bearer {token}")) - { - let _ = headers.insert(http::header::AUTHORIZATION, header); - } - if let Some(account_id) = auth.account_id() - && let Ok(header) = HeaderValue::from_str(&account_id) - { - let _ = headers.insert("ChatGPT-Account-ID", header); - } -} - -pub(crate) fn add_auth_headers(auth: &A, mut req: Request) -> Request { - add_auth_headers_to_header_map(auth, &mut req.headers); - req + fn add_auth_headers(&self, headers: &mut HeaderMap); } diff --git a/codex-rs/codex-api/src/endpoint/compact.rs b/codex-rs/codex-api/src/endpoint/compact.rs index 44a56a11a7..748ac35558 100644 --- a/codex-rs/codex-api/src/endpoint/compact.rs +++ b/codex-rs/codex-api/src/endpoint/compact.rs @@ -90,9 +90,7 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } #[test] diff --git a/codex-rs/codex-api/src/endpoint/memories.rs b/codex-rs/codex-api/src/endpoint/memories.rs index 5cb2a65b1f..3047c859db 100644 --- a/codex-rs/codex-api/src/endpoint/memories.rs +++ b/codex-rs/codex-api/src/endpoint/memories.rs @@ -103,9 +103,7 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } #[derive(Clone)] diff --git a/codex-rs/codex-api/src/endpoint/models.rs b/codex-rs/codex-api/src/endpoint/models.rs index 97781ac419..17342d6f99 100644 --- a/codex-rs/codex-api/src/endpoint/models.rs +++ b/codex-rs/codex-api/src/endpoint/models.rs @@ -132,9 +132,7 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } fn provider(base_url: &str) -> Provider { diff --git a/codex-rs/codex-api/src/endpoint/realtime_call.rs b/codex-rs/codex-api/src/endpoint/realtime_call.rs index 4d0bcfa7bf..a9a8b963cf 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_call.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_call.rs @@ -284,8 +284,11 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn bearer_token(&self) -> Option { - Some("test-token".to_string()) + fn add_auth_headers(&self, headers: &mut HeaderMap) { + headers.insert( + http::header::AUTHORIZATION, + HeaderValue::from_static("Bearer test-token"), + ); } } diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index a60d188bda..d2b775cdd5 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -1,5 +1,4 @@ use crate::auth::AuthProvider; -use crate::auth::add_auth_headers_to_header_map; use crate::common::ResponseEvent; use crate::common::ResponseStream; use crate::common::ResponsesWsRequest; @@ -310,7 +309,7 @@ impl ResponsesWebsocketClient { let mut headers = merge_request_headers(&self.provider.headers, extra_headers, default_headers); - add_auth_headers_to_header_map(&self.auth, &mut headers); + self.auth.add_auth_headers(&mut headers); let (stream, server_reasoning_included, models_etag, server_model) = connect_websocket(ws_url, headers, turn_state.clone()).await?; diff --git a/codex-rs/codex-api/src/endpoint/session.rs b/codex-rs/codex-api/src/endpoint/session.rs index 00919a0c54..e4a470ceee 100644 --- a/codex-rs/codex-api/src/endpoint/session.rs +++ b/codex-rs/codex-api/src/endpoint/session.rs @@ -1,5 +1,4 @@ use crate::auth::AuthProvider; -use crate::auth::add_auth_headers; use crate::error::ApiError; use crate::provider::Provider; use crate::telemetry::run_with_request_telemetry; @@ -56,7 +55,8 @@ impl EndpointSession { if let Some(body) = body { req.body = Some(RequestBody::Json(body.clone())); } - add_auth_headers(&self.auth, req) + self.auth.add_auth_headers(&mut req.headers); + req } pub(crate) async fn execute( diff --git a/codex-rs/codex-api/src/files.rs b/codex-rs/codex-api/src/files.rs index 6fad5b62f5..ebe35af5ba 100644 --- a/codex-rs/codex-api/src/files.rs +++ b/codex-rs/codex-api/src/files.rs @@ -256,17 +256,14 @@ fn authorized_request( method: reqwest::Method, url: &str, ) -> reqwest::RequestBuilder { + let mut headers = http::HeaderMap::new(); + auth.add_auth_headers(&mut headers); + let client = build_reqwest_client(); - let mut request = client + client .request(method, url) - .timeout(OPENAI_FILE_REQUEST_TIMEOUT); - if let Some(token) = auth.bearer_token() { - request = request.bearer_auth(token); - } - if let Some(account_id) = auth.account_id() { - request = request.header("chatgpt-account-id", account_id); - } - request + .timeout(OPENAI_FILE_REQUEST_TIMEOUT) + .headers(headers) } fn build_reqwest_client() -> reqwest::Client { diff --git a/codex-rs/codex-api/tests/clients.rs b/codex-rs/codex-api/tests/clients.rs index b11c6f9d6b..d82fcc14ce 100644 --- a/codex-rs/codex-api/tests/clients.rs +++ b/codex-rs/codex-api/tests/clients.rs @@ -91,9 +91,7 @@ impl HttpTransport for RecordingTransport { struct NoAuth; impl AuthProvider for NoAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } #[derive(Clone)] @@ -112,12 +110,14 @@ impl StaticAuth { } impl AuthProvider for StaticAuth { - fn bearer_token(&self) -> Option { - Some(self.token.clone()) - } - - fn account_id(&self) -> Option { - Some(self.account_id.clone()) + fn add_auth_headers(&self, headers: &mut HeaderMap) { + let token = &self.token; + if let Ok(header) = HeaderValue::from_str(&format!("Bearer {token}")) { + headers.insert(http::header::AUTHORIZATION, header); + } + if let Ok(header) = HeaderValue::from_str(&self.account_id) { + headers.insert("ChatGPT-Account-ID", header); + } } } diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index 91a8477a92..fab135e0ab 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -24,9 +24,7 @@ use wiremock::matchers::path; struct DummyAuth; impl AuthProvider for DummyAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } fn provider(base_url: &str) -> Provider { diff --git a/codex-rs/codex-api/tests/sse_end_to_end.rs b/codex-rs/codex-api/tests/sse_end_to_end.rs index b15de296a8..4d32e82242 100644 --- a/codex-rs/codex-api/tests/sse_end_to_end.rs +++ b/codex-rs/codex-api/tests/sse_end_to_end.rs @@ -53,9 +53,7 @@ impl HttpTransport for FixtureSseTransport { struct NoAuth; impl AuthProvider for NoAuth { - fn bearer_token(&self) -> Option { - None - } + fn add_auth_headers(&self, _headers: &mut HeaderMap) {} } fn provider(name: &str) -> Provider { diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index a371966079..78dfd3c070 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -57,6 +57,7 @@ codex-rmcp-client = { workspace = true } codex-sandboxing = { workspace = true } codex-state = { workspace = true } codex-terminal-detection = { workspace = true } +codex-thread-store = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cache = { workspace = true } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index d5d832d6cb..9280895b5e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -523,6 +523,9 @@ }, "web_search_request": { "type": "boolean" + }, + "workspace_dependencies": { + "type": "boolean" } }, "type": "object" @@ -2380,6 +2383,9 @@ }, "web_search_request": { "type": "boolean" + }, + "workspace_dependencies": { + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 16f743943a..bd83c81f0d 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -32,6 +32,7 @@ use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use codex_api::ApiError; +use codex_api::AuthProvider; use codex_api::CompactClient as ApiCompactClient; use codex_api::CompactionInput as ApiCompactionInput; use codex_api::Compression; @@ -83,7 +84,6 @@ use futures::StreamExt; use http::HeaderMap as ApiHeaderMap; use http::HeaderValue; use http::StatusCode as HttpStatusCode; -use http::header::AUTHORIZATION; use reqwest::StatusCode; use std::time::Duration; use std::time::Instant; @@ -277,16 +277,7 @@ pub(crate) struct RealtimeWebrtcCallStart { /// `api.openai.com` sideband path. fn sideband_websocket_auth_headers(api_auth: &CoreAuthProvider) -> ApiHeaderMap { let mut headers = ApiHeaderMap::new(); - if let Some(token) = api_auth.token.as_ref() - && let Ok(value) = HeaderValue::from_str(&format!("Bearer {token}")) - { - headers.insert(AUTHORIZATION, value); - } - if let Some(account_id) = api_auth.account_id.as_ref() - && let Ok(value) = HeaderValue::from_str(account_id) - { - headers.insert("ChatGPT-Account-ID", value); - } + api_auth.add_auth_headers(&mut headers); headers } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b0f1cae1c4..4529c640d7 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -135,9 +135,11 @@ use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; +use codex_rollout::RolloutConfig; use codex_rollout::state_db; use codex_shell_command::parse_command::parse_command; use codex_terminal_detection::user_agent; +use codex_thread_store::LocalThreadStore; use codex_tools::filter_tool_suggest_discoverable_tools_for_client; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_stream_parser::AssistantTextChunk; @@ -1454,6 +1456,12 @@ impl Session { per_turn_config.service_tier = session_configuration.service_tier; per_turn_config.personality = session_configuration.personality; per_turn_config.approvals_reviewer = session_configuration.approvals_reviewer; + per_turn_config.permissions.approval_policy = session_configuration.approval_policy.clone(); + per_turn_config.permissions.sandbox_policy = session_configuration.sandbox_policy.clone(); + per_turn_config.permissions.file_system_sandbox_policy = + session_configuration.file_system_sandbox_policy.clone(); + per_turn_config.permissions.network_sandbox_policy = + session_configuration.network_sandbox_policy; let resolved_web_search_mode = resolve_web_search_mode_for_turn( &per_turn_config.web_search_mode, session_configuration.sandbox_policy.get(), @@ -2126,6 +2134,7 @@ impl Session { network_proxy, network_approval: Arc::clone(&network_approval), state_db: state_db_ctx.clone(), + thread_store: LocalThreadStore::new(RolloutConfig::from_view(config.as_ref())), model_client: ModelClient::new( Some(Arc::clone(&auth_manager)), conversation_id, @@ -3554,6 +3563,11 @@ impl Session { ts.granted_permissions() } + pub(crate) async fn cwd(&self) -> AbsolutePathBuf { + let state = self.state.lock().await; + state.session_configuration.cwd.clone() + } + pub(crate) async fn granted_session_permissions(&self) -> Option { let state = self.state.lock().await; state.granted_permissions() diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index a408fcbba3..eaa2099098 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2919,6 +2919,9 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { network_proxy: None, network_approval: Arc::clone(&network_approval), state_db: None, + thread_store: codex_thread_store::LocalThreadStore::new( + codex_rollout::RolloutConfig::from_view(config.as_ref()), + ), model_client: ModelClient::new( Some(auth_manager.clone()), conversation_id, @@ -3776,6 +3779,9 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( network_proxy: None, network_approval: Arc::clone(&network_approval), state_db: None, + thread_store: codex_thread_store::LocalThreadStore::new( + codex_rollout::RolloutConfig::from_view(config.as_ref()), + ), model_client: ModelClient::new( Some(Arc::clone(&auth_manager)), conversation_id, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index fa9f835181..398cf1aca5 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -79,6 +79,10 @@ impl CodexThread { self.codex.shutdown_and_wait().await } + pub async fn cwd(&self) -> AbsolutePathBuf { + self.codex.session.cwd().await + } + #[doc(hidden)] pub async fn ensure_rollout_materialized(&self) { self.codex.session.ensure_rollout_materialized().await; diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 59fb45454d..eaf1d92109 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -58,7 +58,7 @@ pub(crate) enum InitialContextInjection { } pub(crate) fn should_use_remote_compact_task(provider: &ModelProviderInfo) -> bool { - provider.is_openai() + provider.supports_remote_compaction() } pub(crate) async fn run_inline_auto_compact_task( diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index cecf9ce9d7..f7a8d1dfff 100644 --- a/codex-rs/core/src/compact_tests.rs +++ b/codex-rs/core/src/compact_tests.rs @@ -1,4 +1,6 @@ use super::*; +use codex_model_provider_info::ModelProviderInfo; +use codex_model_provider_info::WireApi; use pretty_assertions::assert_eq; async fn process_compacted_history_with_test_session( @@ -185,6 +187,30 @@ fn build_token_limited_compacted_history_appends_summary_message() { assert_eq!(summary, summary_text); } +#[test] +fn should_use_remote_compact_task_for_azure_provider() { + let provider = ModelProviderInfo { + name: "Azure".into(), + base_url: Some("https://example.com/openai".into()), + env_key: Some("AZURE_OPENAI_API_KEY".into()), + env_key_instructions: None, + experimental_bearer_token: None, + auth: None, + wire_api: WireApi::Responses, + query_params: None, + http_headers: None, + env_http_headers: None, + request_max_retries: None, + stream_max_retries: None, + stream_idle_timeout_ms: None, + websocket_connect_timeout_ms: None, + requires_openai_auth: false, + supports_websockets: false, + }; + + assert!(should_use_remote_compact_task(&provider)); +} + #[tokio::test] async fn process_compacted_history_replaces_developer_messages() { let compacted_history = vec![ diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index b2931e1055..461c434b15 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -48,6 +48,7 @@ use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID; use codex_model_provider_info::OLLAMA_OSS_PROVIDER_ID; use codex_model_provider_info::WireApi; use codex_models_manager::bundled_models_response; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -627,6 +628,13 @@ fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::io::Re }, ]), ); + assert_eq!( + config.permissions.runtime_permission_profile(), + PermissionProfile::from_runtime_permissions( + &config.permissions.file_system_sandbox_policy, + config.permissions.network_sandbox_policy, + ) + ); assert_eq!( config.permissions.sandbox_policy.get(), &SandboxPolicy::WorkspaceWrite { @@ -1245,6 +1253,14 @@ exclude_slash_tmp = true NetworkSandboxPolicy::from(sandbox_policy), "case `{name}` should preserve network semantics from legacy config" ); + assert_eq!( + config.permissions.runtime_permission_profile(), + PermissionProfile::from_runtime_permissions( + &config.permissions.file_system_sandbox_policy, + config.permissions.network_sandbox_policy, + ), + "case `{name}` should populate canonical permission profile from runtime policies" + ); assert_eq!( config .permissions diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index b58acbf651..523880b537 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -73,6 +73,8 @@ use codex_protocol::config_types::Verbosity; use codex_protocol::config_types::WebSearchConfig; use codex_protocol::config_types::WebSearchMode; use codex_protocol::config_types::WindowsSandboxLevel; +#[cfg(test)] +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::permissions::FileSystemSandboxPolicy; @@ -212,6 +214,16 @@ pub struct Permissions { pub windows_sandbox_private_desktop: bool, } +impl Permissions { + #[cfg(test)] + pub(crate) fn runtime_permission_profile(&self) -> PermissionProfile { + PermissionProfile::from_runtime_permissions( + &self.file_system_sandbox_policy, + self.network_sandbox_policy, + ) + } +} + /// Application configuration loaded from disk and merged with overrides. #[derive(Debug, Clone, PartialEq)] pub struct Config { diff --git a/codex-rs/core/src/personality_migration.rs b/codex-rs/core/src/personality_migration.rs index 52cabf55de..4b53a2629e 100644 --- a/codex-rs/core/src/personality_migration.rs +++ b/codex-rs/core/src/personality_migration.rs @@ -1,14 +1,10 @@ use crate::config::edit::ConfigEditsBuilder; -use crate::rollout::ARCHIVED_SESSIONS_SUBDIR; -use crate::rollout::SESSIONS_SUBDIR; -use crate::rollout::list::ThreadListConfig; -use crate::rollout::list::ThreadListLayout; -use crate::rollout::list::ThreadSortKey; -use crate::rollout::list::get_threads_in_root; use codex_config::config_toml::ConfigToml; use codex_protocol::config_types::Personality; -use codex_protocol::protocol::SessionSource; -use codex_rollout::state_db; +use codex_thread_store::ListThreadsParams; +use codex_thread_store::LocalThreadStore; +use codex_thread_store::ThreadSortKey; +use codex_thread_store::ThreadStore; use std::io; use std::path::Path; use tokio::fs::OpenOptions; @@ -64,57 +60,33 @@ pub async fn maybe_migrate_personality( } async fn has_recorded_sessions(codex_home: &Path, default_provider: &str) -> io::Result { - let allowed_sources: &[SessionSource] = &[]; + let store = LocalThreadStore::new(codex_rollout::RolloutConfig { + codex_home: codex_home.to_path_buf(), + sqlite_home: codex_home.to_path_buf(), + cwd: codex_home.to_path_buf(), + model_provider_id: default_provider.to_string(), + generate_memories: false, + }); + if has_threads(&store, /*archived*/ false).await? { + return Ok(true); + } + has_threads(&store, /*archived*/ true).await +} - if let Some(state_db_ctx) = state_db::open_if_present(codex_home, default_provider).await - && let Some(ids) = state_db::list_thread_ids_db( - Some(state_db_ctx.as_ref()), - codex_home, - /*page_size*/ 1, - /*cursor*/ None, - ThreadSortKey::CreatedAt, - allowed_sources, - /*model_providers*/ None, - /*archived_only*/ false, - "personality_migration", - ) +async fn has_threads(store: &LocalThreadStore, archived: bool) -> io::Result { + store + .list_threads(ListThreadsParams { + page_size: 1, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived, + search_term: None, + }) .await - && !ids.is_empty() - { - return Ok(true); - } - - let sessions = get_threads_in_root( - codex_home.join(SESSIONS_SUBDIR), - /*page_size*/ 1, - /*cursor*/ None, - ThreadSortKey::CreatedAt, - ThreadListConfig { - allowed_sources, - model_providers: None, - default_provider, - layout: ThreadListLayout::NestedByDate, - }, - ) - .await?; - if !sessions.items.is_empty() { - return Ok(true); - } - - let archived_sessions = get_threads_in_root( - codex_home.join(ARCHIVED_SESSIONS_SUBDIR), - /*page_size*/ 1, - /*cursor*/ None, - ThreadSortKey::CreatedAt, - ThreadListConfig { - allowed_sources, - model_providers: None, - default_provider, - layout: ThreadListLayout::Flat, - }, - ) - .await?; - Ok(!archived_sessions.items.is_empty()) + .map(|page| !page.items.is_empty()) + .map_err(io::Error::other) } async fn create_marker(marker_path: &Path) -> io::Result<()> { diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index de1070ad34..4aef53a5c4 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -7,6 +7,8 @@ use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::UserMessageEvent; +use codex_rollout::ARCHIVED_SESSIONS_SUBDIR; +use codex_rollout::SESSIONS_SUBDIR; use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::io::AsyncWriteExt; @@ -25,6 +27,16 @@ async fn write_session_with_user_event(codex_home: &Path) -> io::Result<()> { .join("2025") .join("01") .join("01"); + write_rollout_with_user_event(&dir, thread_id).await +} + +async fn write_archived_session_with_user_event(codex_home: &Path) -> io::Result<()> { + let thread_id = ThreadId::new(); + let dir = codex_home.join(ARCHIVED_SESSIONS_SUBDIR); + write_rollout_with_user_event(&dir, thread_id).await +} + +async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::Result<()> { tokio::fs::create_dir_all(&dir).await?; let file_path = dir.join(format!("rollout-{TEST_TIMESTAMP}-{thread_id}.jsonl")); let mut file = tokio::fs::File::create(&file_path).await?; @@ -85,6 +97,22 @@ async fn applies_when_sessions_exist_and_no_personality() -> io::Result<()> { Ok(()) } +#[tokio::test] +async fn applies_when_only_archived_sessions_exist_and_no_personality() -> io::Result<()> { + let temp = TempDir::new()?; + write_archived_session_with_user_event(temp.path()).await?; + + let config_toml = ConfigToml::default(); + let status = maybe_migrate_personality(temp.path(), &config_toml).await?; + + assert_eq!(status, PersonalityMigrationStatus::Applied); + assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists()); + + let persisted = read_config_toml(temp.path()).await?; + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); + Ok(()) +} + #[tokio::test] async fn skips_when_marker_exists() -> io::Result<()> { let temp = TempDir::new()?; diff --git a/codex-rs/core/src/realtime_context.rs b/codex-rs/core/src/realtime_context.rs index 6e9815f562..30cbd6f636 100644 --- a/codex-rs/core/src/realtime_context.rs +++ b/codex-rs/core/src/realtime_context.rs @@ -4,8 +4,10 @@ use crate::event_mapping::is_contextual_user_message_content; use chrono::Utc; use codex_git_utils::resolve_root_git_project_for_trust; use codex_protocol::models::ResponseItem; -use codex_state::SortKey; -use codex_state::ThreadMetadata; +use codex_thread_store::ListThreadsParams; +use codex_thread_store::StoredThread; +use codex_thread_store::ThreadSortKey; +use codex_thread_store::ThreadStore; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::truncate_text; use dirs::home_dir; @@ -98,7 +100,7 @@ pub(crate) async fn build_realtime_startup_context( } if let Some(section) = format_section( "Notes", - Some("Built at realtime startup from the current thread history, persisted thread metadata in the state DB, and a bounded local workspace scan. This excludes repo memory instructions, AGENTS files, project-doc prompt blends, and memory summaries.".to_string()), + Some("Built at realtime startup from the current thread history, local thread metadata, and a bounded local workspace scan. This excludes repo memory instructions, AGENTS files, project-doc prompt blends, and memory summaries.".to_string()), NOTES_SECTION_TOKEN_BUDGET, ) { parts.push(section); @@ -117,33 +119,31 @@ pub(crate) async fn build_realtime_startup_context( Some(context) } -async fn load_recent_threads(sess: &Session) -> Vec { - let Some(state_db) = sess.services.state_db.as_ref() else { - return Vec::new(); - }; - - match state_db - .list_threads( - MAX_RECENT_THREADS, - /*anchor*/ None, - SortKey::UpdatedAt, - &[], - /*model_providers*/ None, - /*archived_only*/ false, - /*search_term*/ None, - ) +async fn load_recent_threads(sess: &Session) -> Vec { + match sess + .services + .thread_store + .list_threads(ListThreadsParams { + page_size: MAX_RECENT_THREADS, + cursor: None, + sort_key: ThreadSortKey::UpdatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: false, + search_term: None, + }) .await { Ok(page) => page.items, Err(err) => { - warn!("failed to load realtime startup threads from state db: {err}"); + warn!("failed to load realtime startup threads from thread store: {err}"); Vec::new() } } } -fn build_recent_work_section(cwd: &Path, recent_threads: &[ThreadMetadata]) -> Option { - let mut groups: HashMap> = HashMap::new(); +fn build_recent_work_section(cwd: &Path, recent_threads: &[StoredThread]) -> Option { + let mut groups: HashMap> = HashMap::new(); for entry in recent_threads { let group = resolve_root_git_project_for_trust(&entry.cwd).unwrap_or_else(|| entry.cwd.clone()); @@ -446,7 +446,7 @@ fn format_section(title: &str, body: Option, budget_tokens: usize) -> Op fn format_thread_group( current_group: &Path, group: &Path, - entries: Vec<&ThreadMetadata>, + entries: Vec<&StoredThread>, ) -> Option { let latest = entries.first()?; let group_label = if resolve_root_git_project_for_trust(latest.cwd.as_path()).is_some() { @@ -461,8 +461,9 @@ fn format_thread_group( ]; if let Some(git_branch) = latest - .git_branch - .as_deref() + .git_info + .as_ref() + .and_then(|git| git.branch.as_deref()) .filter(|git_branch| !git_branch.is_empty()) { lines.push(format!("Latest branch: {git_branch}")); diff --git a/codex-rs/core/src/realtime_context_tests.rs b/codex-rs/core/src/realtime_context_tests.rs index 7495161d46..7e1e82476f 100644 --- a/codex-rs/core/src/realtime_context_tests.rs +++ b/codex-rs/core/src/realtime_context_tests.rs @@ -3,20 +3,31 @@ use super::build_recent_work_section; use super::build_workspace_section_with_user_root; use chrono::TimeZone; use chrono::Utc; +use codex_git_utils::GitSha; use codex_protocol::ThreadId; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; -use codex_state::ThreadMetadata; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::GitInfo; +use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::protocol::SessionSource; +use codex_thread_store::StoredThread; use pretty_assertions::assert_eq; use std::fs; use std::path::PathBuf; use std::process::Command; use tempfile::TempDir; -fn thread_metadata(cwd: &str, title: &str, first_user_message: &str) -> ThreadMetadata { - ThreadMetadata { - id: ThreadId::new(), - rollout_path: PathBuf::from("/tmp/rollout.jsonl"), +fn stored_thread(cwd: &str, title: &str, first_user_message: &str) -> StoredThread { + StoredThread { + thread_id: ThreadId::new(), + rollout_path: Some(PathBuf::from("/tmp/rollout.jsonl")), + forked_from_id: None, + preview: first_user_message.to_string(), + name: (!title.is_empty()).then(|| title.to_string()), + model_provider: "test-provider".to_string(), + model: Some("gpt-5".to_string()), + reasoning_effort: None, created_at: Utc .timestamp_opt(1_709_251_100, 0) .single() @@ -25,24 +36,23 @@ fn thread_metadata(cwd: &str, title: &str, first_user_message: &str) -> ThreadMe .timestamp_opt(1_709_251_200, 0) .single() .expect("valid timestamp"), - source: "cli".to_string(), - agent_path: None, - agent_nickname: None, - agent_role: None, - model_provider: "test-provider".to_string(), - model: Some("gpt-5".to_string()), - reasoning_effort: None, + archived_at: None, cwd: PathBuf::from(cwd), cli_version: "test".to_string(), - title: title.to_string(), - sandbox_policy: "workspace-write".to_string(), - approval_mode: "never".to_string(), - tokens_used: 0, + source: SessionSource::Cli, + agent_nickname: None, + agent_role: None, + agent_path: None, + git_info: Some(GitInfo { + commit_hash: Some(GitSha::new("abcdef")), + branch: Some("main".to_string()), + repository_url: None, + }), + approval_mode: AskForApproval::Never, + sandbox_policy: SandboxPolicy::new_read_only_policy(), + token_usage: None, first_user_message: Some(first_user_message.to_string()), - archived_at: None, - git_sha: None, - git_branch: Some("main".to_string()), - git_origin_url: None, + history: None, } } @@ -224,17 +234,17 @@ fn recent_work_section_groups_threads_by_cwd() { fs::create_dir_all(&outside).expect("create outside dir"); let recent_threads = vec![ - thread_metadata( + stored_thread( workspace_a.to_string_lossy().as_ref(), "Investigate realtime startup context", "Log the startup context before sending it", ), - thread_metadata( + stored_thread( workspace_b.to_string_lossy().as_ref(), "Trim websocket startup payload", "Remove memories from the realtime startup context", ), - thread_metadata(outside.to_string_lossy().as_ref(), "", "Inspect flaky test"), + stored_thread(outside.to_string_lossy().as_ref(), "", "Inspect flaky test"), ]; let current_cwd = workspace_a; let repo = fs::canonicalize(repo).expect("canonicalize repo"); diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index 26e3a84d5c..4913f957ea 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -46,11 +46,7 @@ impl codex_rollout::RolloutConfigView for Config { } pub(crate) mod list { - pub use codex_rollout::ThreadListConfig; - pub use codex_rollout::ThreadListLayout; - pub use codex_rollout::ThreadSortKey; pub use codex_rollout::find_thread_path_by_id_str; - pub use codex_rollout::get_threads_in_root; } pub(crate) mod metadata { diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 4b6c5b30f5..5db38f7b72 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -24,6 +24,7 @@ use codex_mcp::McpConnectionManager; use codex_models_manager::manager::ModelsManager; use codex_otel::SessionTelemetry; use codex_rollout::state_db::StateDbHandle; +use codex_thread_store::LocalThreadStore; use std::path::PathBuf; use tokio::sync::Mutex; use tokio::sync::RwLock; @@ -59,6 +60,7 @@ pub(crate) struct SessionServices { pub(crate) network_proxy: Option, pub(crate) network_approval: Arc, pub(crate) state_db: Option, + pub(crate) thread_store: LocalThreadStore, /// Session-scoped model client shared across turns. pub(crate) model_client: ModelClient, pub(crate) code_mode_service: CodeModeService, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 8e932c3ad7..59b464ea2f 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -82,10 +82,10 @@ fn write_permissions_for_paths( .ok()?; let permissions = (!write_paths.is_empty()).then_some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(write_paths), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(write_paths), + )), ..Default::default() })?; diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 86e05fb8a1..d99f158e66 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -85,7 +85,13 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs(); assert_eq!( - permissions.and_then(|profile| profile.file_system.and_then(|fs| fs.write)), - Some(vec![expected_outside]) + permissions, + Some(PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![expected_outside]), + )), + ..Default::default() + }) ); } diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 022e02c39e..0a17138d85 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -18,9 +18,9 @@ mod tool_suggest; pub(crate) mod unified_exec; mod view_image; -use codex_sandboxing::policy_transforms::intersect_permission_profiles; use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use codex_sandboxing::policy_transforms::permission_profile_is_preapproved; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; use serde::Deserialize; @@ -187,13 +187,18 @@ pub(super) async fn apply_granted_turn_permissions( additional_permissions.as_ref(), granted_permissions.as_ref(), ); - let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) { - (Some(effective_permissions), Some(granted_permissions)) => { - intersect_permission_profiles(effective_permissions.clone(), granted_permissions) - == *effective_permissions - } - _ => false, - }; + let session_cwd = session.cwd().await; + let permissions_preapproved = + match (effective_permissions.as_ref(), granted_permissions.as_ref()) { + (Some(effective_permissions), Some(granted_permissions)) => { + permission_profile_is_preapproved( + effective_permissions, + granted_permissions, + session_cwd.as_path(), + ) + } + _ => false, + }; let sandbox_permissions = if effective_permissions.is_some() && !sandbox_permissions.uses_additional_permissions() { @@ -235,12 +240,12 @@ mod tests { fn file_system_permissions(path: &std::path::Path) -> PermissionProfile { PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ AbsolutePathBuf::from_absolute_path(path).expect("absolute path"), ]), - }), + )), ..Default::default() } } diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index b641814516..1f52445a5b 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -188,10 +188,10 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() - assert_eq!( args.additional_permissions, Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![expected_write.abs()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![expected_write.abs()]), + )), ..Default::default() }) ); diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 0ba3e131af..f838a43a7a 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -82,10 +82,10 @@ fn file_system_sandbox_context_uses_active_attempt() { .abs(); let additional_permissions = PermissionProfile { network: None, - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(Vec::new()), + )), }; let req = ApplyPatchRequest { action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()), diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index f11050f72b..bbbcd82aaa 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -250,12 +250,12 @@ fn map_exec_result_preserves_stdout_and_stderr() { #[test] fn shell_request_escalation_execution_is_explicit() { let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), ]), - }), + )), ..Default::default() }; let sandbox_policy = SandboxPolicy::WorkspaceWrite { diff --git a/codex-rs/core/tests/suite/realtime_conversation.rs b/codex-rs/core/tests/suite/realtime_conversation.rs index add4065f7b..ecaaeb8cba 100644 --- a/codex-rs/core/tests/suite/realtime_conversation.rs +++ b/codex-rs/core/tests/suite/realtime_conversation.rs @@ -194,10 +194,16 @@ async fn seed_recent_thread( let db = test.codex.state_db().context("state db enabled")?; let thread_id = ThreadId::new(); let updated_at = Utc::now(); + let rollout_path = test + .codex_home_path() + .join(format!("rollout-{thread_id}.jsonl")); + // This helper seeds SQLite metadata directly. Local listing drops stale metadata rows whose + // rollout path no longer exists, so create the placeholder path that the test metadata points + // at without exercising rollout writing in this realtime-context test. + std::fs::write(&rollout_path, "")?; let mut metadata_builder = codex_state::ThreadMetadataBuilder::new( thread_id, - test.codex_home_path() - .join(format!("rollout-{thread_id}.jsonl")), + rollout_path, updated_at, SessionSource::Cli, ); diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 2cfd1cf6f7..1ab44593d8 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -7,6 +7,7 @@ use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; @@ -293,20 +294,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } @@ -343,10 +344,10 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let call_id = "request_permissions_skip_approval"; let command = "touch requested-dir/requested-but-unused.txt"; let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?; @@ -521,10 +522,10 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul let call_id = "request_permissions_relative_workdir"; let command = "touch relative-write.txt"; let expected_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![absolute_path(&nested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![absolute_path(&nested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_raw_request_permissions( @@ -624,10 +625,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd "cwd-widened", unrequested_write, unrequested_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -725,10 +726,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp "tmp-widened", tmp_write, tmp_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -824,19 +825,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "outside-cwd-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..RequestPermissionProfile::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -926,19 +927,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul "should-not-write", outside_write, outside_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -1028,19 +1029,19 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul "sticky-grant-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let responses = mount_sse_sequence( @@ -1492,35 +1493,35 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ absolute_path(first_dir.path()), absolute_path(second_dir.path()), ]), - }), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..RequestPermissionProfile::default() }; let granted_permissions = normalized_directory_write_permissions(first_dir.path())?; let second_dir_permissions = requested_directory_write_permissions(second_dir.path()); let merged_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..Default::default() }; @@ -1584,16 +1585,31 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let approval_file_system = approval_permissions .file_system .unwrap_or_else(|| panic!("expected filesystem permissions")); - assert!(approval_file_system.read.as_ref().is_none_or(Vec::is_empty)); - let mut approval_writes = approval_file_system.write.unwrap_or_default(); + assert_eq!( + approval_file_system + .explicit_path_entries() + .filter(|(_, access)| *access == FileSystemAccessMode::Read) + .count(), + 0 + ); + + let mut approval_writes = approval_file_system + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); approval_writes.sort_by_key(|path| path.display().to_string()); let mut expected_writes = merged_permissions .file_system .unwrap_or_else(|| panic!("expected merged filesystem permissions")) - .write - .unwrap_or_default(); + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); expected_writes.sort_by_key(|path| path.display().to_string()); assert_eq!(approval_writes, expected_writes); diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index 14506f4a41..0578441e99 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -81,20 +81,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 7b0ac256d4..f2f0c52d8d 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -3,6 +3,9 @@ use std::collections::HashMap; use codex_app_server_protocol::JSONRPCErrorError; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::ReadOnlyAccess; @@ -121,16 +124,23 @@ impl FileSystemSandboxRunner { match additional_permissions.and_then(|permissions| permissions.file_system.clone()) { Some(mut file_system) => { if let Some(helper_read_root) = &helper_read_root { - let read_paths = file_system.read.get_or_insert_with(Vec::new); - if !read_paths.contains(helper_read_root) { - read_paths.push(helper_read_root.clone()); + let helper_read_entry = FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: helper_read_root.clone(), + }, + access: FileSystemAccessMode::Read, + }; + if !file_system.entries.contains(&helper_read_entry) { + file_system.entries.push(helper_read_entry); } } Some(file_system) } - None => helper_read_root.map(|helper_read_root| FileSystemPermissions { - read: Some(vec![helper_read_root]), - write: None, + None => helper_read_root.map(|helper_read_root| { + FileSystemPermissions::from_read_write_roots( + Some(vec![helper_read_root]), + /*write*/ None, + ) }), }; @@ -289,6 +299,7 @@ mod tests { use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; + use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -373,27 +384,32 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![writable.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![writable.clone()]), + )), })); assert_eq!(permissions.network, None); - assert_eq!( - permissions - .file_system - .as_ref() - .and_then(|fs| fs.write.clone()), - Some(vec![writable]) - ); - assert_eq!( - permissions - .file_system - .as_ref() - .and_then(|fs| fs.read.clone()), - Some(vec![readable]) - ); + let file_system = permissions + .file_system + .expect("helper permissions should include filesystem access"); + let mut read_paths = file_system + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Read).then_some(path.clone()) + }) + .collect::>(); + read_paths.sort_by(|left, right| left.as_path().cmp(right.as_path())); + let mut write_paths = file_system + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); + write_paths.sort_by(|left, right| left.as_path().cmp(right.as_path())); + assert_eq!(read_paths, vec![readable]); + assert_eq!(write_paths, vec![writable]); } #[test] @@ -415,10 +431,10 @@ mod tests { assert_eq!(permissions.network, None); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![readable]), - write: None, - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![readable]), + /*write*/ None, + )) ); } } diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 41dd3f8600..9e044322e3 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -472,10 +472,10 @@ async fn file_system_sandboxed_write_allows_additional_write_root(use_remote: bo let mut sandbox = read_only_sandbox(readable_dir); sandbox.additional_permissions = Some(PermissionProfile { network: None, - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![absolute_path(writable_dir)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![absolute_path(writable_dir)]), + )), }); file_system diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 758da9f43d..123038ffaa 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -481,6 +481,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result loader_overrides: run_loader_overrides, cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), + log_db: None, environment_manager: std::sync::Arc::new(EnvironmentManager::from_env_with_runtime_paths( Some(local_runtime_paths), )), diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 18daae1b1a..10e79f492a 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -191,6 +191,8 @@ pub enum Feature { ResponsesWebsocketsV2, /// Use the agent identity registration flow for ChatGPT-authenticated sessions. UseAgentIdentity, + /// Enable workspace dependency support. + WorkspaceDependencies, } impl Feature { @@ -672,11 +674,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::MemoryTool, key: "memories", - stage: Stage::Experimental { - name: "Memories", - menu_description: "Allow Codex to create new memories from conversations and bring relevant memories into new conversations.", - announcement: "NEW: Codex can now generate and uses memories. Try is now with `/memories`", - }, + stage: Stage::UnderDevelopment, default_enabled: false, }, FeatureSpec { @@ -929,6 +927,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::WorkspaceDependencies, + key: "workspace_dependencies", + stage: Stage::Stable, + default_enabled: true, + }, ]; pub fn unstable_features_warning_event( diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index 367527ba15..f55578dbd6 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -183,6 +183,16 @@ fn use_agent_identity_is_under_development() { assert_eq!(Feature::UseAgentIdentity.default_enabled(), false); } +#[test] +fn workspace_dependencies_is_stable_and_enabled_by_default() { + assert_eq!(Feature::WorkspaceDependencies.stage(), Stage::Stable); + assert_eq!(Feature::WorkspaceDependencies.default_enabled(), true); + assert_eq!( + feature_for_key("workspace_dependencies"), + Some(Feature::WorkspaceDependencies) + ); +} + #[test] fn collab_is_legacy_alias_for_multi_agent() { assert_eq!(feature_for_key("multi_agent"), Some(Feature::Collab)); diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index f39ff0c7fe..80fb705605 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -7,6 +7,7 @@ use codex_api::Provider as ApiProvider; use codex_api::RetryConfig as ApiRetryConfig; +use codex_api::is_azure_responses_provider; use codex_app_server_protocol::AuthMode; use codex_protocol::config_types::ModelProviderAuthInfo; use codex_protocol::error::CodexErr; @@ -300,6 +301,10 @@ impl ModelProviderInfo { self.name == OPENAI_PROVIDER_NAME } + pub fn supports_remote_compaction(&self) -> bool { + self.is_openai() || is_azure_responses_provider(&self.name, self.base_url.as_deref()) + } + pub fn has_command_auth(&self) -> bool { self.auth.is_some() } diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index bdc2e73b1a..6cd1ef2366 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -129,6 +129,61 @@ supports_websockets = true assert_eq!(provider.websocket_connect_timeout_ms, Some(15_000)); } +#[test] +fn test_supports_remote_compaction_for_openai() { + let provider = ModelProviderInfo::create_openai_provider(/*base_url*/ None); + + assert!(provider.supports_remote_compaction()); +} + +#[test] +fn test_supports_remote_compaction_for_azure_name() { + let provider = ModelProviderInfo { + name: "Azure".into(), + base_url: Some("https://example.com/openai".into()), + env_key: Some("AZURE_OPENAI_API_KEY".into()), + env_key_instructions: None, + experimental_bearer_token: None, + auth: None, + wire_api: WireApi::Responses, + query_params: None, + http_headers: None, + env_http_headers: None, + request_max_retries: None, + stream_max_retries: None, + stream_idle_timeout_ms: None, + websocket_connect_timeout_ms: None, + requires_openai_auth: false, + supports_websockets: false, + }; + + assert!(provider.supports_remote_compaction()); +} + +#[test] +fn test_supports_remote_compaction_for_non_openai_non_azure_provider() { + let provider = ModelProviderInfo { + name: "Example".into(), + base_url: Some("https://example.com/v1".into()), + env_key: Some("API_KEY".into()), + env_key_instructions: None, + experimental_bearer_token: None, + auth: None, + wire_api: WireApi::Responses, + query_params: None, + http_headers: None, + env_http_headers: None, + request_max_retries: None, + stream_max_retries: None, + stream_idle_timeout_ms: None, + websocket_connect_timeout_ms: None, + requires_openai_auth: false, + supports_websockets: false, + }; + + assert!(!provider.supports_remote_compaction()); +} + #[test] fn test_deserialize_provider_auth_config_defaults() { let base_dir = tempdir().unwrap(); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 0f18b68393..ec5ed25ac9 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io; use std::path::Path; use std::sync::LazyLock; @@ -14,6 +15,12 @@ use ts_rs::TS; use crate::config_types::ApprovalsReviewer; use crate::config_types::CollaborationMode; use crate::config_types::SandboxMode; +use crate::permissions::FileSystemAccessMode; +use crate::permissions::FileSystemPath; +use crate::permissions::FileSystemSandboxEntry; +use crate::permissions::FileSystemSandboxPolicy; +use crate::permissions::FileSystemSpecialPath; +use crate::permissions::NetworkSandboxPolicy; use crate::protocol::AskForApproval; use crate::protocol::COLLABORATION_MODE_CLOSE_TAG; use crate::protocol::COLLABORATION_MODE_OPEN_TAG; @@ -80,15 +87,119 @@ impl SandboxPermissions { } } -#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)] pub struct FileSystemPermissions { - pub read: Option>, - pub write: Option>, + pub entries: Vec, } impl FileSystemPermissions { pub fn is_empty(&self) -> bool { - self.read.is_none() && self.write.is_none() + self.entries.is_empty() + } + + pub fn from_read_write_roots( + read: Option>, + write: Option>, + ) -> Self { + let mut entries = Vec::new(); + if let Some(read) = read { + entries.extend(read.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Read, + })); + } + if let Some(write) = write { + entries.extend(write.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + })); + } + Self { entries } + } + + pub fn explicit_path_entries( + &self, + ) -> impl Iterator { + self.entries.iter().filter_map(|entry| match &entry.path { + FileSystemPath::Path { path } => Some((path, entry.access)), + FileSystemPath::Special { .. } => None, + }) + } + + fn as_legacy_permissions(&self) -> Option { + let mut read = Vec::new(); + let mut write = Vec::new(); + + for entry in &self.entries { + let FileSystemPath::Path { path } = &entry.path else { + return None; + }; + match entry.access { + FileSystemAccessMode::Read => read.push(path.clone()), + FileSystemAccessMode::Write => write.push(path.clone()), + FileSystemAccessMode::None => return None, + } + } + + Some(LegacyFileSystemPermissions { + read: (!read.is_empty()).then_some(read), + write: (!write.is_empty()).then_some(write), + }) + } +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct LegacyFileSystemPermissions { + #[serde(default, skip_serializing_if = "Option::is_none")] + read: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + write: Option>, +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct CanonicalFileSystemPermissions { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + entries: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +enum FileSystemPermissionsDe { + Canonical(CanonicalFileSystemPermissions), + Legacy(LegacyFileSystemPermissions), +} + +impl Serialize for FileSystemPermissions { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + if let Some(legacy) = self.as_legacy_permissions() { + legacy.serialize(serializer) + } else { + CanonicalFileSystemPermissions { + entries: self.entries.clone(), + } + .serialize(serializer) + } + } +} + +impl<'de> Deserialize<'de> for FileSystemPermissions { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + match FileSystemPermissionsDe::deserialize(deserializer)? { + FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions { entries }) => { + Ok(Self { entries }) + } + FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => { + Ok(Self::from_read_write_roots(read, write)) + } + } } } @@ -113,6 +224,80 @@ impl PermissionProfile { pub fn is_empty(&self) -> bool { self.network.is_none() && self.file_system.is_none() } + + pub fn from_runtime_permissions( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + ) -> Self { + Self { + network: Some(network_sandbox_policy.into()), + file_system: Some(file_system_sandbox_policy.into()), + } + } + + pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { + Self::from_runtime_permissions( + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, cwd), + NetworkSandboxPolicy::from(sandbox_policy), + ) + } + + pub fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy { + self.file_system.as_ref().map_or_else( + || FileSystemSandboxPolicy::restricted(Vec::new()), + FileSystemSandboxPolicy::from, + ) + } + + pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy { + if self + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + } + } + + pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result { + self.file_system_sandbox_policy() + .to_legacy_sandbox_policy(self.network_sandbox_policy(), cwd) + } +} + +impl From for NetworkPermissions { + fn from(value: NetworkSandboxPolicy) -> Self { + Self { + enabled: Some(value.is_enabled()), + } + } +} + +impl From<&FileSystemSandboxPolicy> for FileSystemPermissions { + fn from(value: &FileSystemSandboxPolicy) -> Self { + let entries = match value.kind { + crate::permissions::FileSystemSandboxKind::Restricted => value.entries.clone(), + crate::permissions::FileSystemSandboxKind::Unrestricted + | crate::permissions::FileSystemSandboxKind::ExternalSandbox => { + vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }] + } + }; + Self { entries } + } +} + +impl From<&FileSystemPermissions> for FileSystemSandboxPolicy { + fn from(value: &FileSystemPermissions) -> Self { + FileSystemSandboxPolicy::restricted(value.entries.clone()) + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] @@ -1436,6 +1621,10 @@ mod tests { use std::path::PathBuf; use tempfile::tempdir; + fn absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(path).expect("path must be absolute") + } + #[test] fn sandbox_permissions_helpers_match_documented_semantics() { let cases = [ @@ -1546,6 +1735,58 @@ mod tests { assert_eq!(permission_profile.is_empty(), false); } + #[test] + fn file_system_permissions_deserialize_legacy_read_write_shape() { + let file_system = serde_json::from_value::(serde_json::json!({ + "read": [absolute_path("/tmp/read-only")], + "write": [absolute_path("/tmp/read-write")], + })) + .expect("deserialize legacy filesystem permissions"); + + assert_eq!( + file_system, + FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/read-write")]), + ) + ); + } + + #[test] + fn file_system_permissions_serialize_explicit_paths_as_legacy_read_write_shape() { + let file_system = FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/read-write")]), + ); + + let value = serde_json::to_value(file_system).expect("serialize filesystem permissions"); + + assert_eq!( + value, + serde_json::json!({ + "read": [absolute_path("/tmp/read-only")], + "write": [absolute_path("/tmp/read-write")], + }) + ); + } + + #[test] + fn file_system_permissions_round_trip_special_entries_through_canonical_shape() { + let file_system = FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + }; + + let value = serde_json::to_value(&file_system).expect("serialize filesystem permissions"); + let reparsed = serde_json::from_value::(value) + .expect("deserialize filesystem permissions"); + + assert_eq!(reparsed, file_system); + } #[test] fn convert_mcp_content_to_items_builds_data_urls_when_missing_prefix() { let contents = vec![serde_json::json!({ diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index f101f8bef0..cc5327136e 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -46,6 +46,7 @@ impl NetworkSandboxPolicy { Copy, PartialEq, Eq, + Hash, PartialOrd, Ord, Serialize, @@ -72,7 +73,7 @@ impl FileSystemAccessMode { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "kind", rename_all = "snake_case")] #[ts(tag = "kind")] pub enum FileSystemSpecialPath { @@ -115,7 +116,7 @@ impl FileSystemSpecialPath { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] pub struct FileSystemSandboxEntry { pub path: FileSystemPath, pub access: FileSystemAccessMode, @@ -156,7 +157,7 @@ struct FileSystemSemanticSignature { unreadable_roots: Vec, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] pub enum FileSystemPath { @@ -370,6 +371,50 @@ impl FileSystemSandboxPolicy { self.resolve_access_with_cwd(path, cwd).can_write() } + pub fn covers_entry_with_cwd(&self, entry: &FileSystemSandboxEntry, cwd: &Path) -> bool { + match &entry.path { + FileSystemPath::Path { path } => match entry.access { + FileSystemAccessMode::Read => self.can_read_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::Write => self.can_write_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::None => { + self.resolve_access_with_cwd(path.as_path(), cwd) == FileSystemAccessMode::None + } + }, + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => match entry.access { + FileSystemAccessMode::Read => self.has_full_disk_read_access(), + FileSystemAccessMode::Write => self.has_full_disk_write_access(), + FileSystemAccessMode::None => { + !self.has_full_disk_read_access() && !self.has_full_disk_write_access() + } + }, + FileSystemSpecialPath::Minimal => match entry.access { + FileSystemAccessMode::Read => { + self.has_full_disk_read_access() || self.include_platform_defaults() + } + FileSystemAccessMode::Write => self.has_full_disk_write_access(), + FileSystemAccessMode::None => { + !self.has_full_disk_read_access() && !self.include_platform_defaults() + } + }, + _ => resolve_file_system_special_path( + value, + AbsolutePathBuf::from_absolute_path(cwd).ok().as_ref(), + ) + .is_some_and(|path| match entry.access { + FileSystemAccessMode::Read => self.can_read_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::Write => { + self.can_write_path_with_cwd(path.as_path(), cwd) + } + FileSystemAccessMode::None => { + self.resolve_access_with_cwd(path.as_path(), cwd) + == FileSystemAccessMode::None + } + }), + }, + } + } + pub fn with_additional_readable_roots( mut self, cwd: &Path, diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 4f88ff1a21..ab1ae0a5bf 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -279,6 +279,10 @@ impl RolloutRecorder { let codex_home = config.codex_home(); let state_db_ctx = state_db::get_state_db(config).await; + // Search is the SQLite-optimized path and assumes a DB marked backfill-complete is + // actually populated enough to answer the query. If unmigrated rollout files still exist + // on disk, the repair path below may or may not run and catch them depending on whether + // SQLite already has another matching search hit. if search_term.is_some() && let Some(db_page) = state_db::list_threads_db( state_db_ctx.as_deref(), diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index f4e019ee28..e869736294 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -130,10 +130,10 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }), }, policy: &SandboxPolicy::ExternalSandbox { @@ -183,10 +183,10 @@ fn transform_additional_permissions_preserves_denied_entries() { cwd: cwd.clone(), env: HashMap::new(), additional_permissions: Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![allowed_path.clone()]), + )), ..Default::default() }), }, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 73ecf81c9b..714a7fd90e 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -1,7 +1,6 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; -use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxKind; @@ -13,6 +12,7 @@ use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; use std::collections::HashSet; +use std::path::Path; #[derive(Debug, Clone, PartialEq, Eq)] pub struct EffectiveSandboxPermissions { @@ -45,13 +45,26 @@ pub fn normalize_additional_permissions( let file_system = additional_permissions .file_system .map(|file_system| { - let read = file_system - .read - .map(|paths| normalize_permission_paths(paths, "file_system.read")); - let write = file_system - .write - .map(|paths| normalize_permission_paths(paths, "file_system.write")); - FileSystemPermissions { read, write } + let mut entries = Vec::with_capacity(file_system.entries.len()); + for entry in file_system.entries { + let path = match entry.path { + FileSystemPath::Path { path } => FileSystemPath::Path { + path: canonicalize_preserving_symlinks(path.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) + .unwrap_or(path), + }, + FileSystemPath::Special { value } => FileSystemPath::Special { value }, + }; + let normalized_entry = FileSystemSandboxEntry { + path, + access: entry.access, + }; + if !entries.contains(&normalized_entry) { + entries.push(normalized_entry); + } + } + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); Ok(PermissionProfile { @@ -89,8 +102,7 @@ pub fn merge_permission_profiles( }; let file_system = match (base.file_system.as_ref(), permissions.file_system.as_ref()) { (Some(base), Some(permissions)) => Some(FileSystemPermissions { - read: merge_permission_paths(base.read.as_ref(), permissions.read.as_ref()), - write: merge_permission_paths(base.write.as_ref(), permissions.write.as_ref()), + entries: merge_permission_entries(&base.entries, &permissions.entries), }) .filter(|file_system| !file_system.is_empty()), (Some(base), None) => Some(base.clone()), @@ -115,12 +127,13 @@ pub fn intersect_permission_profiles( let file_system = requested .file_system .map(|requested_file_system| { - let granted_file_system = granted.file_system.unwrap_or_default(); - let read = - intersect_permission_paths(requested_file_system.read, granted_file_system.read); - let write = - intersect_permission_paths(requested_file_system.write, granted_file_system.write); - FileSystemPermissions { read, write } + let granted_entries = granted.file_system.unwrap_or_default().entries; + let entries = requested_file_system + .entries + .into_iter() + .filter(|entry| granted_entries.contains(entry)) + .collect(); + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); let network = match (requested.network, granted.network) { @@ -143,67 +156,46 @@ pub fn intersect_permission_profiles( } } -fn intersect_permission_paths( - requested: Option>, - granted: Option>, -) -> Option> { - requested.and_then(|requested_paths| { - if requested_paths.is_empty() { - return granted.map(|_| Vec::new()); +pub fn permission_profile_is_preapproved( + requested: &PermissionProfile, + granted: &PermissionProfile, + cwd: &Path, +) -> bool { + let network_preapproved = !requested + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + || granted + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false); + let file_system_preapproved = match requested.file_system.as_ref() { + Some(requested_file_system) => { + let granted_policy = granted.file_system_sandbox_policy(); + requested_file_system + .entries + .iter() + .all(|entry| granted_policy.covers_entry_with_cwd(entry, cwd)) } + None => true, + }; - let granted_paths = granted.unwrap_or_default(); - Some( - requested_paths - .into_iter() - .filter(|path| granted_paths.contains(path)) - .collect::>(), - ) - .filter(|paths| !paths.is_empty()) - }) + network_preapproved && file_system_preapproved } -fn normalize_permission_paths( - paths: Vec, - _permission_kind: &str, -) -> Vec { - let mut out = Vec::with_capacity(paths.len()); - let mut seen = HashSet::new(); - - for path in paths { - let canonicalized = canonicalize_preserving_symlinks(path.as_path()) - .ok() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) - .unwrap_or(path); - if seen.insert(canonicalized.clone()) { - out.push(canonicalized); +fn merge_permission_entries( + base: &[FileSystemSandboxEntry], + permissions: &[FileSystemSandboxEntry], +) -> Vec { + let mut merged = Vec::with_capacity(base.len() + permissions.len()); + for entry in base.iter().chain(permissions.iter()) { + if !merged.contains(entry) { + merged.push(entry.clone()); } } - - out -} - -fn merge_permission_paths( - base: Option<&Vec>, - permissions: Option<&Vec>, -) -> Option> { - match (base, permissions) { - (Some(base), Some(permissions)) => { - let mut merged = Vec::with_capacity(base.len() + permissions.len()); - let mut seen = HashSet::with_capacity(base.len() + permissions.len()); - - for path in base.iter().chain(permissions.iter()) { - if seen.insert(path.clone()) { - merged.push(path.clone()); - } - } - - Some(merged).filter(|paths| !paths.is_empty()) - } - (Some(base), None) => Some(base.clone()), - (None, Some(permissions)) => Some(permissions.clone()), - (None, None) => None, - } + merged } fn dedup_absolute_paths(paths: Vec) -> Vec { @@ -217,7 +209,7 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { out } -fn additional_permission_roots( +fn additional_permission_explicit_path_roots( additional_permissions: &PermissionProfile, ) -> (Vec, Vec) { ( @@ -225,14 +217,24 @@ fn additional_permission_roots( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.read.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_read().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), dedup_absolute_paths( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.write.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_write().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), ) @@ -240,28 +242,14 @@ fn additional_permission_roots( fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, - extra_reads: Vec, - extra_writes: Vec, + additional_permissions: &FileSystemPermissions, ) -> FileSystemSandboxPolicy { match file_system_policy.kind { FileSystemSandboxKind::Restricted => { let mut merged_policy = file_system_policy.clone(); - for path in extra_reads { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Read, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); - } - } - for path in extra_writes { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Write, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); + for entry in &additional_permissions.entries { + if !merged_policy.entries.contains(entry) { + merged_policy.entries.push(entry.clone()); } } merged_policy @@ -280,14 +268,15 @@ pub fn effective_file_system_sandbox_policy( return file_system_policy.clone(); }; - let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - if extra_reads.is_empty() && extra_writes.is_empty() { + let Some(file_system_permissions) = additional_permissions.file_system.as_ref() else { + return file_system_policy.clone(); + }; + if file_system_permissions.is_empty() { file_system_policy.clone() } else { merge_file_system_policy_with_additional_permissions( file_system_policy, - extra_reads, - extra_writes, + file_system_permissions, ) } } @@ -347,7 +336,11 @@ fn sandbox_policy_with_additional_permissions( return sandbox_policy.clone(); } - let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); + // Legacy SandboxPolicy remains a best-effort projection during the + // migration to PermissionProfile-backed thread permissions. The direct + // filesystem sandbox policy carries the full permission shape. + let (extra_reads, extra_writes) = + additional_permission_explicit_path_roots(additional_permissions); match sandbox_policy { SandboxPolicy::DangerFullAccess => SandboxPolicy::DangerFullAccess, diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index f33d312f15..8b404bf440 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -2,6 +2,7 @@ use super::effective_file_system_sandbox_policy; use super::intersect_permission_profiles; use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; +use super::permission_profile_is_preapproved; use super::sandbox_policy_with_additional_permissions; use super::should_require_platform_sandbox; use codex_protocol::models::FileSystemPermissions; @@ -106,10 +107,10 @@ fn normalize_additional_permissions_preserves_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path.clone()]), + )), }) .expect("permissions"); @@ -121,10 +122,10 @@ fn normalize_additional_permissions_preserves_network() { ); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path]), - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path]), + )) ); } @@ -141,23 +142,23 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() { let link_write_dir = AbsolutePathBuf::from_absolute_path(link_root.join("write")).expect("link write dir"); let permissions = normalize_additional_permissions(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![link_write_dir]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![link_write_dir]), + )), ..Default::default() }) .expect("permissions"); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::from_absolute_path(link_root.join("write")) - .expect("link write dir") + .expect("link write dir"), ]), - }) + )) ); } @@ -165,50 +166,22 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() { fn normalize_additional_permissions_drops_empty_nested_profiles() { let permissions = normalize_additional_permissions(PermissionProfile { network: Some(NetworkPermissions { enabled: None }), - file_system: Some(FileSystemPermissions { - read: None, - write: None, - }), + file_system: Some(FileSystemPermissions::default()), }) .expect("permissions"); assert_eq!(permissions, PermissionProfile::default()); } -#[test] -fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); - let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), - ..Default::default() - }; - let granted = requested.clone(); - - assert_eq!( - intersect_permission_profiles(requested.clone(), granted), - requested - ); -} - #[test] fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: None, - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(Vec::from(["/tmp/requested" + .try_into() + .expect("absolute path")])), + /*write*/ None, + )), ..Default::default() }; @@ -219,24 +192,65 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { } #[test] -fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() { +fn permission_profile_is_preapproved_when_granted_parent_directory_covers_requested_child() { let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( + let parent = AbsolutePathBuf::from_absolute_path( canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); + let child = AbsolutePathBuf::from_absolute_path(parent.as_path().join("child")) + .expect("absolute child path"); + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![parent]), + )), + ..Default::default() + }; let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), ..Default::default() }; - assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default()), - PermissionProfile::default() - ); + assert!(permission_profile_is_preapproved( + &requested, + &granted, + temp_dir.path(), + )); +} + +#[test] +fn permission_profile_is_preapproved_when_granted_cwd_covers_requested_child() { + let temp_dir = TempDir::new().expect("create temp dir"); + let child = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("child")) + .expect("absolute child path"); + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + }), + ..Default::default() + }; + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + }; + + assert!(permission_profile_is_preapproved( + &requested, + &granted, + temp_dir.path(), + )); } #[test] @@ -258,10 +272,10 @@ fn read_only_additional_permissions_can_enable_network_without_writes() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(Vec::new()), + )), }, ); @@ -292,10 +306,10 @@ fn external_sandbox_additional_permissions_can_enable_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }, ); @@ -331,8 +345,10 @@ fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roo access: FileSystemAccessMode::None, }, ]), - vec![allowed_path.clone()], - Vec::new(), + &FileSystemPermissions::from_read_write_roots( + Some(vec![allowed_path.clone()]), + Some(Vec::new()), + ), ); assert_eq!( @@ -402,10 +418,10 @@ fn effective_file_system_sandbox_policy_merges_additional_write_roots() { }, ]); let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![allowed_path.clone()]), + )), ..Default::default() }; diff --git a/codex-rs/thread-store/Cargo.toml b/codex-rs/thread-store/Cargo.toml index a8f22fbfe5..328d9a4a7f 100644 --- a/codex-rs/thread-store/Cargo.toml +++ b/codex-rs/thread-store/Cargo.toml @@ -14,6 +14,16 @@ workspace = true [dependencies] async-trait = { workspace = true } chrono = { workspace = true, features = ["serde"] } +codex-git-utils = { workspace = true } codex-protocol = { workspace = true } +codex-rollout = { workspace = true } serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } thiserror = { workspace = true } + +[dev-dependencies] +codex-state = { workspace = true } +pretty_assertions = { workspace = true } +tempfile = { workspace = true } +tokio = { workspace = true, features = ["macros"] } +uuid = { workspace = true } diff --git a/codex-rs/thread-store/src/lib.rs b/codex-rs/thread-store/src/lib.rs index 5525db1394..a64bf023be 100644 --- a/codex-rs/thread-store/src/lib.rs +++ b/codex-rs/thread-store/src/lib.rs @@ -5,12 +5,14 @@ //! any other backing store. mod error; +mod local; mod recorder; mod store; mod types; pub use error::ThreadStoreError; pub use error::ThreadStoreResult; +pub use local::LocalThreadStore; pub use recorder::ThreadRecorder; pub use store::ThreadStore; pub use types::AppendThreadItemsParams; diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs new file mode 100644 index 0000000000..cc15033cd9 --- /dev/null +++ b/codex-rs/thread-store/src/local/mod.rs @@ -0,0 +1,565 @@ +use async_trait::async_trait; +use chrono::DateTime; +use chrono::Utc; +use codex_git_utils::GitSha; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::GitInfo; +use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::protocol::SessionSource; +use codex_rollout::RolloutConfig; +use codex_rollout::RolloutRecorder; +use codex_rollout::ThreadItem; +use codex_rollout::parse_cursor; + +use crate::AppendThreadItemsParams; +use crate::ArchiveThreadParams; +use crate::CreateThreadParams; +use crate::ListThreadsParams; +use crate::LoadThreadHistoryParams; +use crate::ReadThreadParams; +use crate::ResumeThreadRecorderParams; +use crate::SetThreadNameParams; +use crate::StoredThread; +use crate::StoredThreadHistory; +use crate::ThreadPage; +use crate::ThreadRecorder; +use crate::ThreadSortKey; +use crate::ThreadStore; +use crate::ThreadStoreError; +use crate::ThreadStoreResult; +use crate::UpdateThreadMetadataParams; + +/// Local filesystem/SQLite-backed implementation of [`ThreadStore`]. +#[derive(Clone, Debug)] +pub struct LocalThreadStore { + config: RolloutConfig, +} + +impl LocalThreadStore { + /// Create a local store from the rollout configuration used by existing local persistence. + pub fn new(config: RolloutConfig) -> Self { + Self { config } + } +} + +#[async_trait] +impl ThreadStore for LocalThreadStore { + async fn create_thread( + &self, + _params: CreateThreadParams, + ) -> ThreadStoreResult> { + unsupported("create_thread") + } + + async fn resume_thread_recorder( + &self, + _params: ResumeThreadRecorderParams, + ) -> ThreadStoreResult> { + unsupported("resume_thread_recorder") + } + + async fn append_items(&self, _params: AppendThreadItemsParams) -> ThreadStoreResult<()> { + unsupported("append_items") + } + + async fn load_history( + &self, + _params: LoadThreadHistoryParams, + ) -> ThreadStoreResult { + unsupported("load_history") + } + + async fn read_thread(&self, _params: ReadThreadParams) -> ThreadStoreResult { + unsupported("read_thread") + } + + async fn list_threads(&self, params: ListThreadsParams) -> ThreadStoreResult { + let cursor = params + .cursor + .as_deref() + .map(|cursor| { + parse_cursor(cursor).ok_or_else(|| ThreadStoreError::InvalidRequest { + message: format!("invalid cursor: {cursor}"), + }) + }) + .transpose()?; + let sort_key = match params.sort_key { + ThreadSortKey::CreatedAt => codex_rollout::ThreadSortKey::CreatedAt, + ThreadSortKey::UpdatedAt => codex_rollout::ThreadSortKey::UpdatedAt, + }; + let page = list_rollout_threads(&self.config, ¶ms, cursor.as_ref(), sort_key).await?; + + let next_cursor = page + .next_cursor + .as_ref() + .and_then(|cursor| serde_json::to_value(cursor).ok()) + .and_then(|value| value.as_str().map(str::to_owned)); + let items = page + .items + .into_iter() + .filter_map(|item| { + stored_thread_from_rollout_item( + item, + params.archived, + self.config.model_provider_id.as_str(), + ) + }) + .collect::>(); + + Ok(ThreadPage { items, next_cursor }) + } + + async fn set_thread_name(&self, _params: SetThreadNameParams) -> ThreadStoreResult<()> { + unsupported("set_thread_name") + } + + async fn update_thread_metadata( + &self, + _params: UpdateThreadMetadataParams, + ) -> ThreadStoreResult { + unsupported("update_thread_metadata") + } + + async fn archive_thread(&self, _params: ArchiveThreadParams) -> ThreadStoreResult<()> { + unsupported("archive_thread") + } + + async fn unarchive_thread( + &self, + _params: ArchiveThreadParams, + ) -> ThreadStoreResult { + unsupported("unarchive_thread") + } +} + +fn unsupported(operation: &str) -> ThreadStoreResult { + Err(ThreadStoreError::Internal { + message: format!("local thread store does not implement {operation} in this slice"), + }) +} + +async fn list_rollout_threads( + config: &RolloutConfig, + params: &ListThreadsParams, + cursor: Option<&codex_rollout::Cursor>, + sort_key: codex_rollout::ThreadSortKey, +) -> ThreadStoreResult { + let page = if params.archived { + RolloutRecorder::list_archived_threads( + config, + params.page_size, + cursor, + sort_key, + params.allowed_sources.as_slice(), + params.model_providers.as_deref(), + config.model_provider_id.as_str(), + params.search_term.as_deref(), + ) + .await + } else { + RolloutRecorder::list_threads( + config, + params.page_size, + cursor, + sort_key, + params.allowed_sources.as_slice(), + params.model_providers.as_deref(), + config.model_provider_id.as_str(), + params.search_term.as_deref(), + ) + .await + }; + page.map_err(|err| ThreadStoreError::Internal { + message: format!("failed to list threads: {err}"), + }) +} + +fn stored_thread_from_rollout_item( + item: ThreadItem, + archived: bool, + default_provider: &str, +) -> Option { + let thread_id = item + .thread_id + .or_else(|| thread_id_from_rollout_path(item.path.as_path()))?; + let created_at = parse_rfc3339(item.created_at.as_deref()).unwrap_or_else(Utc::now); + let updated_at = parse_rfc3339(item.updated_at.as_deref()).unwrap_or(created_at); + let archived_at = archived.then_some(updated_at); + let git_info = git_info_from_parts( + item.git_sha.clone(), + item.git_branch.clone(), + item.git_origin_url.clone(), + ); + let source = item.source.unwrap_or(SessionSource::Unknown); + let preview = item.first_user_message.clone().unwrap_or_default(); + + Some(StoredThread { + thread_id, + rollout_path: Some(item.path), + forked_from_id: None, + preview, + name: None, + model_provider: item + .model_provider + .filter(|provider| !provider.is_empty()) + .unwrap_or_else(|| default_provider.to_string()), + model: None, + reasoning_effort: None, + created_at, + updated_at, + archived_at, + cwd: item.cwd.unwrap_or_default(), + cli_version: item.cli_version.unwrap_or_default(), + source, + agent_nickname: item.agent_nickname, + agent_role: item.agent_role, + agent_path: None, + git_info, + approval_mode: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_read_only_policy(), + token_usage: None, + first_user_message: item.first_user_message, + history: None, + }) +} + +fn parse_rfc3339(value: Option<&str>) -> Option> { + DateTime::parse_from_rfc3339(value?) + .ok() + .map(|dt| dt.with_timezone(&Utc)) +} + +fn git_info_from_parts( + sha: Option, + branch: Option, + origin_url: Option, +) -> Option { + if sha.is_none() && branch.is_none() && origin_url.is_none() { + return None; + } + Some(GitInfo { + commit_hash: sha.as_deref().map(GitSha::new), + branch, + repository_url: origin_url, + }) +} + +fn thread_id_from_rollout_path(path: &std::path::Path) -> Option { + let file_name = path.file_name()?.to_str()?; + let stem = file_name.strip_suffix(".jsonl")?; + if stem.len() < 37 { + return None; + } + let uuid_start = stem.len().saturating_sub(36); + if !stem[..uuid_start].ends_with('-') { + return None; + } + codex_protocol::ThreadId::from_string(&stem[uuid_start..]).ok() +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::io::Write; + use std::path::Path; + use std::path::PathBuf; + + use codex_protocol::ThreadId; + use codex_protocol::protocol::SessionSource; + use codex_rollout::ARCHIVED_SESSIONS_SUBDIR; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + use uuid::Uuid; + + use super::*; + + fn test_config(codex_home: &Path) -> RolloutConfig { + RolloutConfig { + codex_home: codex_home.to_path_buf(), + sqlite_home: codex_home.to_path_buf(), + cwd: codex_home.to_path_buf(), + model_provider_id: "test-provider".to_string(), + generate_memories: true, + } + } + + fn write_session_file(root: &Path, ts: &str, uuid: Uuid) -> std::io::Result { + write_session_file_with( + root, + root.join("sessions/2025/01/03"), + ts, + uuid, + "Hello from user", + Some("test-provider"), + ) + } + + fn write_archived_session_file(root: &Path, ts: &str, uuid: Uuid) -> std::io::Result { + write_session_file_with( + root, + root.join(ARCHIVED_SESSIONS_SUBDIR), + ts, + uuid, + "Archived user message", + Some("test-provider"), + ) + } + + fn write_session_file_with( + root: &Path, + day_dir: PathBuf, + ts: &str, + uuid: Uuid, + first_user_message: &str, + model_provider: Option<&str>, + ) -> std::io::Result { + fs::create_dir_all(&day_dir)?; + let path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl")); + let mut file = fs::File::create(&path)?; + let meta = serde_json::json!({ + "timestamp": ts, + "type": "session_meta", + "payload": { + "id": uuid, + "timestamp": ts, + "cwd": root, + "originator": "test_originator", + "cli_version": "test_version", + "source": "cli", + "model_provider": model_provider, + "git": { + "commit_hash": "abcdef", + "branch": "main", + "repository_url": "https://example.com/repo.git" + } + }, + }); + writeln!(file, "{meta}")?; + let user_event = serde_json::json!({ + "timestamp": ts, + "type": "event_msg", + "payload": { + "type": "user_message", + "message": first_user_message, + "kind": "plain", + }, + }); + writeln!(file, "{user_event}")?; + Ok(path) + } + + #[tokio::test] + async fn list_threads_uses_default_provider_when_rollout_omits_provider() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + write_session_file_with( + home.path(), + home.path().join("sessions/2025/01/03"), + "2025-01-03T12-00-00", + Uuid::from_u128(102), + "Hello from user", + /*model_provider*/ None, + ) + .expect("session file"); + + let page = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: false, + search_term: None, + }) + .await + .expect("thread listing"); + + assert_eq!(page.items.len(), 1); + assert_eq!(page.items[0].model_provider, "test-provider"); + } + + #[tokio::test] + async fn list_threads_preserves_sqlite_title_search_results() { + let home = TempDir::new().expect("temp dir"); + let config = test_config(home.path()); + let store = LocalThreadStore::new(config.clone()); + let uuid = Uuid::from_u128(103); + let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id"); + let rollout_path = home.path().join("rollout-title-search.jsonl"); + fs::write(&rollout_path, "").expect("placeholder rollout file"); + + let runtime = codex_state::StateRuntime::init( + home.path().to_path_buf(), + config.model_provider_id.clone(), + ) + .await + .expect("state db should initialize"); + runtime + .mark_backfill_complete(/*last_watermark*/ None) + .await + .expect("backfill should be complete"); + let created_at = Utc::now(); + let mut builder = codex_state::ThreadMetadataBuilder::new( + thread_id, + rollout_path, + created_at, + SessionSource::Cli, + ); + builder.model_provider = Some(config.model_provider_id.clone()); + builder.cwd = home.path().to_path_buf(); + builder.cli_version = Some("test_version".to_string()); + let mut metadata = builder.build(config.model_provider_id.as_str()); + metadata.title = "needle title".to_string(); + metadata.first_user_message = Some("plain preview".to_string()); + runtime + .upsert_thread(&metadata) + .await + .expect("state db upsert should succeed"); + + let page = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: false, + search_term: Some("needle".to_string()), + }) + .await + .expect("thread listing"); + + let ids = page + .items + .iter() + .map(|item| item.thread_id) + .collect::>(); + assert_eq!(ids, vec![thread_id]); + assert_eq!( + page.items[0].first_user_message.as_deref(), + Some("plain preview") + ); + } + + #[tokio::test] + async fn list_threads_selects_active_or_archived_collection() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + let active_uuid = Uuid::from_u128(105); + let archived_uuid = Uuid::from_u128(106); + write_session_file(home.path(), "2025-01-03T12-00-00", active_uuid) + .expect("active session file"); + write_archived_session_file(home.path(), "2025-01-03T13-00-00", archived_uuid) + .expect("archived session file"); + + let active = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: false, + search_term: None, + }) + .await + .expect("active listing"); + let archived = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: true, + search_term: None, + }) + .await + .expect("archived listing"); + + let active_id = ThreadId::from_string(&active_uuid.to_string()).expect("valid thread id"); + let archived_id = + ThreadId::from_string(&archived_uuid.to_string()).expect("valid thread id"); + assert_eq!( + active + .items + .iter() + .map(|item| item.thread_id) + .collect::>(), + vec![active_id] + ); + assert_eq!( + archived + .items + .iter() + .map(|item| item.thread_id) + .collect::>(), + vec![archived_id] + ); + assert_eq!(active.items[0].archived_at, None); + assert_eq!( + archived.items[0].archived_at, + Some(archived.items[0].updated_at) + ); + } + + #[tokio::test] + async fn list_threads_returns_local_rollout_summary() { + let home = TempDir::new().expect("temp dir"); + let config = test_config(home.path()); + let store = LocalThreadStore::new(config); + let uuid = Uuid::from_u128(101); + let path = + write_session_file(home.path(), "2025-01-03T12-00-00", uuid).expect("session file"); + + let page = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: None, + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: vec![SessionSource::Cli], + model_providers: Some(vec!["test-provider".to_string()]), + archived: false, + search_term: None, + }) + .await + .expect("thread listing"); + + let thread_id = + codex_protocol::ThreadId::from_string(&uuid.to_string()).expect("valid thread id"); + assert_eq!(page.next_cursor, None); + assert_eq!(page.items.len(), 1); + assert_eq!(page.items[0].thread_id, thread_id); + assert_eq!(page.items[0].rollout_path, Some(path)); + assert_eq!(page.items[0].preview, "Hello from user"); + assert_eq!( + page.items[0].first_user_message.as_deref(), + Some("Hello from user") + ); + assert_eq!(page.items[0].model_provider, "test-provider"); + assert_eq!(page.items[0].cli_version, "test_version"); + assert_eq!(page.items[0].source, SessionSource::Cli); + } + + #[tokio::test] + async fn list_threads_rejects_invalid_cursor() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + + let err = store + .list_threads(ListThreadsParams { + page_size: 10, + cursor: Some("not-a-cursor".to_string()), + sort_key: ThreadSortKey::CreatedAt, + allowed_sources: Vec::new(), + model_providers: None, + archived: false, + search_term: None, + }) + .await + .expect_err("invalid cursor should fail"); + + assert!(matches!(err, ThreadStoreError::InvalidRequest { .. })); + } +} diff --git a/codex-rs/thread-store/src/types.rs b/codex-rs/thread-store/src/types.rs index 73f983a466..8d489ea6cb 100644 --- a/codex-rs/thread-store/src/types.rs +++ b/codex-rs/thread-store/src/types.rs @@ -135,6 +135,8 @@ pub struct ThreadPage { pub struct StoredThread { /// Thread id. pub thread_id: ThreadId, + /// Local rollout path when the backing store is filesystem-based. + pub rollout_path: Option, /// Source thread id when this thread was forked from another thread. pub forked_from_id: Option, /// Best available user-facing preview, usually the first user message. @@ -155,6 +157,8 @@ pub struct StoredThread { pub archived_at: Option>, /// Working directory captured for the thread. pub cwd: PathBuf, + /// CLI version captured for the thread. + pub cli_version: String, /// Runtime source for the thread. pub source: SessionSource, /// Optional random nickname for thread-spawn sub-agents. diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index b30e0bffc2..4a3ccc01ae 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -85,6 +85,7 @@ pub use responses_api::ResponsesApiNamespace; pub use responses_api::ResponsesApiNamespaceTool; pub use responses_api::ResponsesApiTool; pub use responses_api::ToolSearchOutputTool; +pub(crate) use responses_api::default_namespace_description; pub use responses_api::dynamic_tool_to_responses_api_tool; pub use responses_api::mcp_tool_to_deferred_responses_api_tool; pub use responses_api::mcp_tool_to_responses_api_tool; diff --git a/codex-rs/tools/src/responses_api.rs b/codex-rs/tools/src/responses_api.rs index 205c075105..5317a082f1 100644 --- a/codex-rs/tools/src/responses_api.rs +++ b/codex-rs/tools/src/responses_api.rs @@ -55,6 +55,10 @@ pub struct ResponsesApiNamespace { pub tools: Vec, } +pub(crate) fn default_namespace_description(namespace_name: &str) -> String { + format!("Tools in the {namespace_name} namespace.") +} + #[derive(Debug, Clone, Serialize, PartialEq)] #[serde(tag = "type")] pub enum ResponsesApiNamespaceTool { diff --git a/codex-rs/tools/src/tool_discovery.rs b/codex-rs/tools/src/tool_discovery.rs index e35da5735c..b773623edf 100644 --- a/codex-rs/tools/src/tool_discovery.rs +++ b/codex-rs/tools/src/tool_discovery.rs @@ -5,6 +5,7 @@ use crate::ResponsesApiTool; use crate::ToolName; use crate::ToolSearchOutputTool; use crate::ToolSpec; +use crate::default_namespace_description; use crate::mcp_tool_to_deferred_responses_api_tool; use codex_app_server_protocol::AppInfo; use serde::Deserialize; @@ -225,6 +226,8 @@ pub fn collect_tool_search_output_tools<'a>( let description = first_tool .connector_description + .map(str::trim) + .filter(|description| !description.is_empty()) .map(str::to_string) .or_else(|| { first_tool @@ -232,12 +235,6 @@ pub fn collect_tool_search_output_tools<'a>( .map(str::trim) .filter(|connector_name| !connector_name.is_empty()) .map(|connector_name| format!("Tools for working with {connector_name}.")) - }) - .or_else(|| { - Some(format!( - "Tools from the {} MCP server.", - first_tool.server_name - )) }); let tools = tools @@ -251,7 +248,8 @@ pub fn collect_tool_search_output_tools<'a>( results.push(ToolSearchOutputTool::Namespace(ResponsesApiNamespace { name: tool_namespace.to_string(), - description: description.unwrap_or_default(), + description: description + .unwrap_or_else(|| default_namespace_description(tool_namespace)), tools, })); } diff --git a/codex-rs/tools/src/tool_discovery_tests.rs b/codex-rs/tools/src/tool_discovery_tests.rs index 10e108fe04..b745f756cc 100644 --- a/codex-rs/tools/src/tool_discovery_tests.rs +++ b/codex-rs/tools/src/tool_discovery_tests.rs @@ -253,7 +253,7 @@ fn collect_tool_search_output_tools_preserves_search_order_while_grouping_by_nam }), ToolSearchOutputTool::Namespace(ResponsesApiNamespace { name: "mcp__docs__".to_string(), - description: "Tools from the docs MCP server.".to_string(), + description: "Tools in the mcp__docs__ namespace.".to_string(), tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { name: "search".to_string(), description: "Search docs.".to_string(), @@ -272,7 +272,7 @@ fn collect_tool_search_output_tools_preserves_search_order_while_grouping_by_nam } #[test] -fn collect_tool_search_output_tools_falls_back_to_connector_name_description() { +fn collect_tool_search_output_tools_ignores_blank_connector_description() { let gmail_batch_read_email = mcp_tool("gmail-batch-read-email", "Read multiple emails."); let tools = collect_tool_search_output_tools([ToolSearchResultSource { @@ -281,7 +281,7 @@ fn collect_tool_search_output_tools_falls_back_to_connector_name_description() { tool_name: "_batch_read_email", tool: &gmail_batch_read_email, connector_name: Some("Gmail"), - connector_description: None, + connector_description: Some(" "), }]) .expect("collect tool search output tools"); diff --git a/codex-rs/tools/src/tool_registry_plan.rs b/codex-rs/tools/src/tool_registry_plan.rs index cec938962c..b5da9afb25 100644 --- a/codex-rs/tools/src/tool_registry_plan.rs +++ b/codex-rs/tools/src/tool_registry_plan.rs @@ -55,6 +55,7 @@ use crate::create_wait_agent_tool_v2; use crate::create_wait_tool; use crate::create_web_search_tool; use crate::create_write_stdin_tool; +use crate::default_namespace_description; use crate::dynamic_tool_to_responses_api_tool; use crate::mcp_tool_to_responses_api_tool; use crate::request_permissions_tool_description; @@ -485,11 +486,20 @@ pub fn build_tool_registry_plan( for (namespace, mut entries) in namespace_entries { entries.sort_by_key(|tool| tool.name.name.clone()); - let description = params + let tool_namespace = params .tool_namespaces - .and_then(|namespaces| namespaces.get(&namespace)) - .and_then(|namespace| namespace.description.clone()) - .unwrap_or_default(); + .and_then(|namespaces| namespaces.get(&namespace)); + let description = tool_namespace + .and_then(|namespace| namespace.description.as_deref()) + .map(str::trim) + .filter(|description| !description.is_empty()) + .map(str::to_string) + .unwrap_or_else(|| { + let namespace_name = tool_namespace + .map(|namespace| namespace.name.as_str()) + .unwrap_or(namespace.as_str()); + default_namespace_description(namespace_name) + }); let mut tools = Vec::new(); for tool in entries { match mcp_tool_to_responses_api_tool(&tool.name, tool.tool) { diff --git a/codex-rs/tools/src/tool_registry_plan_tests.rs b/codex-rs/tools/src/tool_registry_plan_tests.rs index 7b9e8ed9ac..a643543ee7 100644 --- a/codex-rs/tools/src/tool_registry_plan_tests.rs +++ b/codex-rs/tools/src/tool_registry_plan_tests.rs @@ -1151,6 +1151,46 @@ fn test_build_specs_mcp_tools_converted() { ); } +#[test] +fn test_build_specs_mcp_namespace_description_falls_back_when_missing() { + let model_info = model_info(); + let mut features = Features::with_defaults(); + features.enable(Feature::UnifiedExec); + let available_models = Vec::new(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &features, + image_generation_tool_auth_allowed: true, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + sandbox_policy: &SandboxPolicy::DangerFullAccess, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + let (tools, _) = build_specs( + &tools_config, + Some(HashMap::from([( + ToolName::namespaced("test_server/", "do_something_cool"), + mcp_tool( + "do_something_cool", + "Do something cool", + serde_json::json!({"type": "object"}), + ), + )])), + /*deferred_mcp_tools*/ None, + &[], + ); + + let namespace_tool = find_tool(&tools, "test_server/"); + let ToolSpec::Namespace(namespace) = &namespace_tool.spec else { + panic!("expected namespace tool"); + }; + assert_eq!( + namespace.description, + "Tools in the test_server/ namespace." + ); +} + #[test] fn test_build_specs_mcp_tools_sorted_by_name() { let model_info = model_info(); diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 026bd05d7c..bef1fe1b83 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -6351,7 +6351,7 @@ async fn fetch_plugins_list( ) -> Result { let cwd = AbsolutePathBuf::try_from(cwd).wrap_err("plugin list cwd must be absolute")?; let request_id = RequestId::String(format!("plugin-list-{}", Uuid::new_v4())); - request_handle + let mut response = request_handle .request_typed(ClientRequest::PluginList { request_id, params: PluginListParams { @@ -6360,7 +6360,17 @@ async fn fetch_plugins_list( }, }) .await - .wrap_err("plugin/list failed in TUI") + .wrap_err("plugin/list failed in TUI")?; + hide_cli_only_plugin_marketplaces(&mut response); + Ok(response) +} + +const CLI_HIDDEN_PLUGIN_MARKETPLACES: &[&str] = &["openai-bundled"]; + +fn hide_cli_only_plugin_marketplaces(response: &mut PluginListResponse) { + response + .marketplaces + .retain(|marketplace| !CLI_HIDDEN_PLUGIN_MARKETPLACES.contains(&marketplace.name.as_str())); } async fn fetch_plugin_detail( @@ -6538,6 +6548,7 @@ mod tests { use codex_app_server_protocol::NetworkPolicyRuleAction as AppServerNetworkPolicyRuleAction; use codex_app_server_protocol::NonSteerableTurnKind as AppServerNonSteerableTurnKind; use codex_app_server_protocol::PermissionsRequestApprovalParams; + use codex_app_server_protocol::PluginMarketplaceEntry; use codex_app_server_protocol::RequestId as AppServerRequestId; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; @@ -6596,6 +6607,41 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path") } + #[test] + fn hide_cli_only_plugin_marketplaces_removes_openai_bundled() { + let mut response = PluginListResponse { + marketplaces: vec![ + PluginMarketplaceEntry { + name: "openai-bundled".to_string(), + path: test_absolute_path("/marketplaces/openai-bundled"), + interface: None, + plugins: Vec::new(), + }, + PluginMarketplaceEntry { + name: "openai-curated".to_string(), + path: test_absolute_path("/marketplaces/openai-curated"), + interface: None, + plugins: Vec::new(), + }, + ], + marketplace_load_errors: Vec::new(), + remote_sync_error: None, + featured_plugin_ids: Vec::new(), + }; + + hide_cli_only_plugin_marketplaces(&mut response); + + assert_eq!( + response.marketplaces, + vec![PluginMarketplaceEntry { + name: "openai-curated".to_string(), + path: test_absolute_path("/marketplaces/openai-curated"), + interface: None, + plugins: Vec::new(), + }] + ); + } + #[test] fn normalize_harness_overrides_resolves_relative_add_dirs() -> Result<()> { let temp_dir = tempdir()?; @@ -8833,10 +8879,12 @@ guardian_approval = true network: Some(AdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(AdditionalFileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(AdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + ), + )), }); params.proposed_network_policy_amendments = Some(vec![AppServerNetworkPolicyAmendment { host: "example.com".to_string(), @@ -8868,10 +8916,10 @@ guardian_approval = true network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + )), }) ); assert_eq!( @@ -8940,10 +8988,12 @@ guardian_approval = true network: Some(AdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(AdditionalFileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(AdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + ), + )), }, }, }; @@ -8964,10 +9014,10 @@ guardian_approval = true network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + )), } ); } diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index b1393e95e0..a9210fc97b 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -393,10 +393,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path(read_path)]), - write: Some(vec![absolute_path(write_path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(read_path)]), + Some(vec![absolute_path(write_path)]), + )), }, scope: codex_protocol::request_permissions::PermissionGrantScope::Session, }, @@ -412,10 +412,12 @@ mod tests { network: Some(AdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(AdditionalFileSystemPermissions { - read: Some(vec![absolute_path(read_path)]), - write: Some(vec![absolute_path(write_path)]), - }), + file_system: Some(AdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(read_path)]), + Some(vec![absolute_path(write_path)]), + ), + )), }, scope: PermissionGrantScope::Session, } diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index 8d2335eb7d..700cd1e946 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -35,12 +35,7 @@ pub(crate) fn granted_permission_profile_from_request( network: value.network.map(|network| AdditionalNetworkPermissions { enabled: network.enabled, }), - file_system: value - .file_system - .map(|file_system| AdditionalFileSystemPermissions { - read: file_system.read, - write: file_system.write, - }), + file_system: value.file_system.map(AdditionalFileSystemPermissions::from), } } @@ -82,19 +77,23 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/read-only")]), - write: Some(vec![absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/write")]), + )), }), codex_app_server_protocol::GrantedPermissionProfile { network: Some(codex_app_server_protocol::AdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { - read: Some(vec![absolute_path("/tmp/read-only")]), - write: Some(vec![absolute_path("/tmp/write")]), - }), + file_system: Some( + codex_app_server_protocol::AdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/write")]), + ), + ) + ), } ); } diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 656e812d66..7864036085 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -20,6 +20,9 @@ use codex_features::Features; use codex_protocol::ThreadId; use codex_protocol::mcp::RequestId; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; @@ -747,22 +750,20 @@ pub(crate) fn format_additional_permissions_rule( parts.push("network".to_string()); } if let Some(file_system) = additional_permissions.file_system.as_ref() { - if let Some(read) = file_system.read.as_ref() { - let reads = read - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + if let Some(reads) = format_file_system_permissions(file_system, FileSystemAccessMode::Read) + { parts.push(format!("read {reads}")); } - if let Some(write) = file_system.write.as_ref() { - let writes = write - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + if let Some(writes) = + format_file_system_permissions(file_system, FileSystemAccessMode::Write) + { parts.push(format!("write {writes}")); } + if let Some(denies) = + format_file_system_permissions(file_system, FileSystemAccessMode::None) + { + parts.push(format!("deny {denies}")); + } } if parts.is_empty() { None @@ -771,6 +772,40 @@ pub(crate) fn format_additional_permissions_rule( } } +fn format_file_system_permissions( + file_system: &codex_protocol::models::FileSystemPermissions, + access: FileSystemAccessMode, +) -> Option { + let values = file_system + .entries + .iter() + .filter(|entry| entry.access == access) + .map(|entry| format!("`{}`", format_file_system_path(&entry.path))) + .collect::>(); + (!values.is_empty()).then(|| values.join(", ")) +} + +fn format_file_system_path(path: &FileSystemPath) -> String { + match path { + FileSystemPath::Path { path } => path.display().to_string(), + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => ":root".to_string(), + FileSystemSpecialPath::Minimal => ":minimal".to_string(), + FileSystemSpecialPath::CurrentWorkingDirectory => ":cwd".to_string(), + FileSystemSpecialPath::ProjectRoots { subpath } => subpath.as_ref().map_or_else( + || ":project_roots".to_string(), + |subpath| format!(":project_roots/{}", subpath.display()), + ), + FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(), + FileSystemSpecialPath::SlashTmp => "/tmp".to_string(), + FileSystemSpecialPath::Unknown { path, subpath } => subpath.as_ref().map_or_else( + || path.clone(), + |subpath| format!("{path}/{}", subpath.display()), + ), + }, + } +} + pub(crate) fn format_requested_permissions_rule( permissions: &RequestPermissionProfile, ) -> Option { @@ -852,6 +887,10 @@ mod tests { use crate::app_event::AppEvent; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::protocol::NetworkPolicyAmendment; @@ -884,6 +923,7 @@ mod tests { [ (absolute_path("/tmp/readme.txt"), "/tmp/readme.txt"), (absolute_path("/tmp/out.txt"), "/tmp/out.txt"), + (absolute_path("/tmp/secret.txt"), "/tmp/secret.txt"), ] .into_iter() .fold(rendered, |rendered, (path, normalized)| { @@ -914,10 +954,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }, } } @@ -1194,10 +1234,10 @@ mod tests { #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), ..Default::default() }; let options = exec_options( @@ -1275,10 +1315,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; @@ -1325,10 +1365,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; @@ -1339,6 +1379,46 @@ mod tests { ); } + #[test] + fn additional_permissions_special_entries_prompt_snapshot() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let exec_request = ApprovalRequest::Exec { + thread_id: ThreadId::new(), + thread_label: None, + id: "test".into(), + command: vec!["cat".into(), "/tmp/readme.txt".into()], + reason: Some("need broader filesystem access".into()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], + network_approval_context: None, + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: absolute_path("/tmp/secret.txt"), + }, + access: FileSystemAccessMode::None, + }, + ], + }), + ..Default::default() + }), + }; + + let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults()); + assert_snapshot!( + "approval_overlay_additional_permissions_special_entries_prompt", + normalize_snapshot_paths(render_overlay_lines(&view, /*width*/ 120)) + ); + } + #[test] fn permissions_prompt_snapshot() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 29da003dc2..8bef8ddbc8 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -293,7 +293,7 @@ mod tests { CommandItem::Builtin(cmd) => cmd.command(), }) .collect(); - assert_eq!(cmds, vec!["model", "memories", "mention", "mcp"]); + assert_eq!(cmds, vec!["model", "mention", "mcp"]); } #[test] diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap new file mode 100644 index 0000000000..0e868386cc --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/bottom_pane/approval_overlay.rs +expression: "normalize_snapshot_paths(render_overlay_lines(&view, 120))" +--- + Would you like to run the following command? + + Reason: need broader filesystem access + + Permission rule: write `:root`; deny `/tmp/secret.txt` + + $ cat /tmp/readme.txt + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 512b415926..06cb4ef2a9 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -110,10 +110,12 @@ fn app_server_exec_approval_request_preserves_permissions_context() { network: Some(AppServerAdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(AppServerAdditionalFileSystemPermissions { - read: Some(vec![read_path.clone()]), - write: Some(vec![write_path.clone()]), - }), + file_system: Some(AppServerAdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![read_path.clone()]), + Some(vec![write_path.clone()]), + ), + )), }), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, @@ -135,10 +137,10 @@ fn app_server_exec_approval_request_preserves_permissions_context() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![read_path]), - write: Some(vec![write_path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![read_path]), + Some(vec![write_path]), + )), }) ); } @@ -159,10 +161,12 @@ fn app_server_request_permissions_preserves_file_system_permissions() { network: Some(AppServerAdditionalNetworkPermissions { enabled: Some(true), }), - file_system: Some(AppServerAdditionalFileSystemPermissions { - read: Some(vec![read_path.clone()]), - write: Some(vec![write_path.clone()]), - }), + file_system: Some(AppServerAdditionalFileSystemPermissions::from( + FileSystemPermissions::from_read_write_roots( + Some(vec![read_path.clone()]), + Some(vec![write_path.clone()]), + ), + )), }, }); @@ -172,10 +176,10 @@ fn app_server_request_permissions_preserves_file_system_permissions() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![read_path]), - write: Some(vec![write_path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![read_path]), + Some(vec![write_path]), + )), } ); } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 46ac561dff..cf938545c6 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -67,10 +67,12 @@ use std::future::Future; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use tracing::Level; use tracing::error; use tracing::warn; use tracing_appender::non_blocking; use tracing_subscriber::EnvFilter; +use tracing_subscriber::filter::Targets; use tracing_subscriber::prelude::*; use url::Url; use uuid::Uuid; @@ -237,6 +239,7 @@ pub use public_widgets::composer_input::ComposerAction; pub use public_widgets::composer_input::ComposerInput; // (tests access modules directly within the crate) +#[allow(clippy::too_many_arguments)] async fn start_embedded_app_server( arg0_paths: Arg0DispatchPaths, config: Config, @@ -244,6 +247,7 @@ async fn start_embedded_app_server( loader_overrides: LoaderOverrides, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, + log_db: Option, environment_manager: Arc, ) -> color_eyre::Result { start_embedded_app_server_with( @@ -253,6 +257,7 @@ async fn start_embedded_app_server( loader_overrides, cloud_requirements, feedback, + log_db, environment_manager, InProcessAppServerClient::start, ) @@ -369,6 +374,7 @@ async fn start_app_server( loader_overrides: LoaderOverrides, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, + log_db: Option, environment_manager: Arc, ) -> color_eyre::Result { match target { @@ -379,6 +385,7 @@ async fn start_app_server( loader_overrides, cloud_requirements, feedback, + log_db, environment_manager, ) .await @@ -403,6 +410,7 @@ pub(crate) async fn start_app_server_for_picker( LoaderOverrides::default(), CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), + /*log_db*/ None, environment_manager, ) .await?; @@ -429,6 +437,7 @@ async fn start_embedded_app_server_with( loader_overrides: LoaderOverrides, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, + log_db: Option, environment_manager: Arc, start_client: F, ) -> color_eyre::Result @@ -453,6 +462,7 @@ where loader_overrides, cloud_requirements, feedback, + log_db, environment_manager, config_warnings, session_source: codex_protocol::protocol::SessionSource::Cli, @@ -969,9 +979,10 @@ pub async fn run_main( let otel_tracing_layer = otel.as_ref().and_then(|o| o.tracing_layer()); - let log_db_layer = get_state_db(&config) - .await - .map(|db| log_db::start(db).with_filter(env_filter())); + let log_db = get_state_db(&config).await.map(log_db::start); + let log_db_layer = log_db + .clone() + .map(|layer| layer.with_filter(Targets::new().with_default(Level::TRACE))); let _ = tracing_subscriber::registry() .with(file_layer) @@ -993,6 +1004,7 @@ pub async fn run_main( cli_kv_overrides, cloud_requirements, feedback, + log_db, remote_url, remote_auth_token, environment_manager, @@ -1013,6 +1025,7 @@ async fn run_ratatui_app( cli_kv_overrides: Vec<(String, toml::Value)>, mut cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, + log_db: Option, remote_url: Option, remote_auth_token: Option, environment_manager: Arc, @@ -1071,6 +1084,7 @@ async fn run_ratatui_app( loader_overrides.clone(), cloud_requirements.clone(), feedback.clone(), + log_db.clone(), environment_manager.clone(), ) .await @@ -1409,6 +1423,7 @@ async fn run_ratatui_app( loader_overrides, cloud_requirements.clone(), feedback.clone(), + log_db.clone(), environment_manager.clone(), ) .await @@ -1772,6 +1787,7 @@ mod tests { LoaderOverrides::default(), CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), + /*log_db*/ None, Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), ) .await @@ -2163,6 +2179,7 @@ mod tests { LoaderOverrides::default(), CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), + /*log_db*/ None, Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), |_args| async { Err(std::io::Error::other("boom")) }, ) diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 57bd58bf5a..aee909cea1 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -988,6 +988,7 @@ mod tests { "https://chatgpt.com/backend-api/".to_string(), ), feedback: codex_feedback::CodexFeedback::new(), + log_db: None, environment_manager: Arc::new(codex_app_server_client::EnvironmentManager::new( /*exec_server_url*/ None, )), diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 759bedbad7..70aed459ea 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -195,6 +195,7 @@ impl SlashCommand { SlashCommand::SandboxReadRoot => cfg!(target_os = "windows"), SlashCommand::Copy => !cfg!(target_os = "android"), SlashCommand::Rollout | SlashCommand::TestApproval => cfg!(debug_assertions), + SlashCommand::Memories => false, _ => true, } }