diff --git a/package.json b/package.json index dc34c3358..0a58fe0f1 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "codeg", "private": true, - "version": "0.21.8", + "version": "0.21.9", "packageManager": "pnpm@11.9.0", "scripts": { "dev": "next dev --turbopack", diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index ee01c432f..6afb950c6 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1014,7 +1014,7 @@ checksum = "c3e64b0cc0439b12df2fa678eae89a1c56a529fd067a9115f7827f1fffd22b32" [[package]] name = "codeg" -version = "0.21.8" +version = "0.21.9" dependencies = [ "aes-gcm", "agent-client-protocol-schema", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index a68821f79..6aee8ccc2 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeg" -version = "0.21.8" +version = "0.21.9" description = "Agent Code Generation App" authors = ["feitao"] edition = "2021" diff --git a/src-tauri/src/acp/file_system_runtime.rs b/src-tauri/src/acp/file_system_runtime.rs index 2524f1479..d7ae37133 100644 --- a/src-tauri/src/acp/file_system_runtime.rs +++ b/src-tauri/src/acp/file_system_runtime.rs @@ -226,8 +226,11 @@ fn extra_write_roots(runtime_env: &BTreeMap) -> Vec { /// — hence `canonical_root` on every entry). fn temp_roots() -> Vec { let mut roots = vec![std::env::temp_dir()]; - #[cfg(unix)] - { + // The shared unix temp dirs are extra roots on top of the per-user one; + // Windows has none to add. Gated with a runtime `cfg!` rather than + // `#[cfg(unix)]` so the binding is still mutated on Windows — a `#[cfg]` + // block leaves `mut` unused there, which `-D warnings` rejects. + if cfg!(unix) { roots.push(PathBuf::from("/tmp")); roots.push(PathBuf::from("/var/tmp")); } @@ -946,6 +949,24 @@ mod tests { path } + /// Drive prefix that makes a rooted path ABSOLUTE on this platform. On + /// Windows `\tmp\x` is rooted but not absolute (it is relative to the + /// current drive), so a literal unix path would be dropped by the very + /// relative-path guards these tests exercise. + #[cfg(windows)] + const ABS_PREFIX: &str = "C:"; + #[cfg(not(windows))] + const ABS_PREFIX: &str = ""; + + /// An absolute path for the HOST platform, built from unix-style segments. + /// `absolute_path("")` is the filesystem root itself (`/`, or `C:/`). + /// + /// None of these paths are ever opened — they only have to be absolute, so + /// the synthetic drive letter needs no counterpart on disk. + fn absolute_path(segments: &str) -> PathBuf { + PathBuf::from(format!("{ABS_PREFIX}/{segments}")) + } + #[tokio::test(flavor = "current_thread")] async fn read_honors_line_and_limit() { let workspace = temp_workspace(); @@ -1297,7 +1318,7 @@ mod tests { /// fallback) are covered by `relocation_suffixes_match_the_agents_own_resolvers`. #[test] fn agent_data_roots_honor_runtime_env_relocation() { - let relocated = PathBuf::from("/tmp/codeg-relocated-agent-home"); + let relocated = absolute_path("tmp/codeg-relocated-agent-home"); let cases = [ (AgentType::Grok, "GROK_HOME"), (AgentType::ClaudeCode, "CLAUDE_CONFIG_DIR"), @@ -1327,7 +1348,7 @@ mod tests { /// writable, which for `GEMINI_CLI_HOME=$HOME` is the whole home directory. #[test] fn relocation_suffixes_match_the_agents_own_resolvers() { - let base = PathBuf::from("/tmp/codeg-relocation-base"); + let base = absolute_path("tmp/codeg-relocation-base"); let cases = [ (AgentType::Gemini, "GEMINI_CLI_HOME", ".gemini"), (AgentType::OpenCode, "XDG_DATA_HOME", "opencode"), @@ -1407,7 +1428,7 @@ mod tests { /// inactive tree holds config and credentials, so it must not stay writable. #[test] fn relocation_excludes_the_inactive_default_root() { - let relocated = PathBuf::from("/tmp/codeg-isolated-profile"); + let relocated = absolute_path("tmp/codeg-isolated-profile"); let cases = [ (AgentType::Codex, "CODEX_HOME"), (AgentType::Grok, "GROK_HOME"), @@ -1504,14 +1525,18 @@ mod tests { /// not to the masked key's inherited value. #[test] fn blank_runtime_value_falls_through_to_the_next_candidate() { + let xdg = absolute_path("tmp/codeg-xdg"); let roots = agent_data_roots( AgentType::Cursor, &BTreeMap::from([ ("CURSOR_CONFIG_DIR".to_string(), String::new()), - ("XDG_CONFIG_HOME".to_string(), "/tmp/codeg-xdg".to_string()), + ( + "XDG_CONFIG_HOME".to_string(), + xdg.to_string_lossy().to_string(), + ), ]), ); - assert_eq!(roots, vec![PathBuf::from("/tmp/codeg-xdg/cursor")]); + assert_eq!(roots, vec![xdg.join("cursor")]); } /// `child_env_value` must distinguish the ways a var reaches (or fails to @@ -1723,16 +1748,17 @@ mod tests { /// Same hazard through the user-facing knob. #[test] fn relative_extra_roots_are_dropped() { + let absolute = absolute_path("tmp/codeg-abs-extra"); let runtime_env = BTreeMap::from([( FS_EXTRA_ROOTS_ENV.to_string(), - std::env::join_paths([Path::new("."), Path::new("/tmp/codeg-abs-extra")]) + std::env::join_paths([Path::new("."), absolute.as_path()]) .expect("join paths") .to_string_lossy() .to_string(), )]); let roots = extra_write_roots(&runtime_env); - assert_eq!(roots, vec![PathBuf::from("/tmp/codeg-abs-extra")]); + assert_eq!(roots, vec![absolute]); } /// The extra-roots list must be read VERBATIM. `split_paths` does not trim @@ -1740,20 +1766,22 @@ mod tests { /// `" / "` into `/` and hand out the entire filesystem. #[test] fn whitespace_padded_extra_root_is_not_trimmed_into_filesystem_root() { - for padded in [" / ", "\t/", "/ "] { - let runtime_env = - BTreeMap::from([(FS_EXTRA_ROOTS_ENV.to_string(), padded.to_string())]); + let root = absolute_path(""); + let raw = root.to_string_lossy().to_string(); + + for padded in [format!(" {raw} "), format!("\t{raw}"), format!("{raw} ")] { + let runtime_env = BTreeMap::from([(FS_EXTRA_ROOTS_ENV.to_string(), padded.clone())]); let roots = extra_write_roots(&runtime_env); assert!( - !roots.contains(&PathBuf::from("/")), + !roots.contains(&root), "{padded:?} must not become the filesystem root, got {roots:?}" ); } - // An entry that IS exactly `/` stays honored — widening is this knob's - // declared purpose, so that is the user's explicit choice. - let runtime_env = BTreeMap::from([(FS_EXTRA_ROOTS_ENV.to_string(), "/".to_string())]); - assert_eq!(extra_write_roots(&runtime_env), vec![PathBuf::from("/")]); + // An entry that IS exactly the filesystem root stays honored — widening + // is this knob's declared purpose, so that is the user's explicit choice. + let runtime_env = BTreeMap::from([(FS_EXTRA_ROOTS_ENV.to_string(), raw)]); + assert_eq!(extra_write_roots(&runtime_env), vec![root]); } /// Trimming a relocation value can WIDEN the policy, not just fail to match: @@ -1761,27 +1789,34 @@ mod tests { /// filesystem root and would make everything writable. #[test] fn whitespace_padded_root_never_becomes_the_filesystem_root() { + let root = absolute_path(""); + let padded = format!(" {} ", root.to_string_lossy()); + for (agent_type, key) in [ (AgentType::Codex, "CODEX_HOME"), (AgentType::Grok, "GROK_HOME"), (AgentType::Pi, "PI_CODING_AGENT_DIR"), ] { - let runtime_env = BTreeMap::from([(key.to_string(), " / ".to_string())]); + let runtime_env = BTreeMap::from([(key.to_string(), padded.clone())]); let roots = agent_data_roots(agent_type, &runtime_env); assert!( - !roots.contains(&PathBuf::from("/")), - "{agent_type:?}: {key}=\" / \" must not resolve to the filesystem root, \ - got {roots:?}" + !roots.contains(&root), + "{agent_type:?}: {key}={padded:?} must not resolve to the filesystem \ + root, got {roots:?}" ); } - // And the same value must not slip through the write gate either. + // And the same value must not slip through the write gate either. Compared + // against the CANONICAL root because that is the form `permissive` stores — + // on Windows `canonicalize("C:/")` is the verbatim `\\?\C:\`, so comparing + // the raw spelling would make this assertion unfalsifiable there. let workspace = temp_workspace(); - let runtime_env = BTreeMap::from([("CODEX_HOME".to_string(), " / ".to_string())]); + let runtime_env = BTreeMap::from([("CODEX_HOME".to_string(), padded)]); let policy = FsAccessPolicy::permissive(&workspace, AgentType::Codex, &runtime_env); assert!( - !policy.write_roots.contains(&PathBuf::from("/")), - "write roots must not include /: {:?}", + !policy.write_roots.contains(&canonical_root(&root)), + "write roots must not include {}: {:?}", + root.display(), policy.write_roots ); @@ -1853,16 +1888,20 @@ mod tests { /// first match must win rather than both being admitted. #[test] fn cursor_relocation_respects_resolver_precedence() { + let config_dir = absolute_path("tmp/codeg-cursor"); let runtime_env = BTreeMap::from([ ( "CURSOR_CONFIG_DIR".to_string(), - "/tmp/codeg-cursor".to_string(), + config_dir.to_string_lossy().to_string(), + ), + ( + "XDG_CONFIG_HOME".to_string(), + absolute_path("tmp/codeg-xdg").to_string_lossy().to_string(), ), - ("XDG_CONFIG_HOME".to_string(), "/tmp/codeg-xdg".to_string()), ]); let roots = agent_data_roots(AgentType::Cursor, &runtime_env); - assert_eq!(roots, vec![PathBuf::from("/tmp/codeg-cursor")]); + assert_eq!(roots, vec![config_dir]); } /// pi relocates its session store independently of its agent home, so a @@ -1870,7 +1909,7 @@ mod tests { /// roots on its own — the process-env-only resolver cannot see it. #[test] fn pi_session_dir_relocation_is_honored() { - let sessions = PathBuf::from("/tmp/codeg-pi-sessions-elsewhere"); + let sessions = absolute_path("tmp/codeg-pi-sessions-elsewhere"); let runtime_env = BTreeMap::from([( "PI_CODING_AGENT_SESSION_DIR".to_string(), sessions.to_string_lossy().to_string(), diff --git a/src-tauri/src/acp/registry.rs b/src-tauri/src/acp/registry.rs index f22f84509..42d5eb5a7 100644 --- a/src-tauri/src/acp/registry.rs +++ b/src-tauri/src/acp/registry.rs @@ -300,34 +300,34 @@ pub fn get_agent_meta(agent_type: AgentType) -> AcpAgentMeta { name: "OpenCode", description: "The open source coding agent", distribution: AgentDistribution::Binary { - version: "1.18.4", + version: "1.18.5", cmd: "opencode", args: &["acp"], env: &[], platforms: &[ PlatformBinary { platform: "darwin-aarch64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-darwin-arm64.zip", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-darwin-arm64.zip", }, PlatformBinary { platform: "darwin-x86_64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-darwin-x64.zip", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-darwin-x64.zip", }, PlatformBinary { platform: "linux-aarch64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-linux-arm64.tar.gz", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-linux-arm64.tar.gz", }, PlatformBinary { platform: "linux-x86_64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-linux-x64.tar.gz", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-linux-x64.tar.gz", }, PlatformBinary { platform: "windows-aarch64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-windows-arm64.zip", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-windows-arm64.zip", }, PlatformBinary { platform: "windows-x86_64", - url: "https://github.com/anomalyco/opencode/releases/download/v1.18.4/opencode-windows-x64.zip", + url: "https://github.com/anomalyco/opencode/releases/download/v1.18.5/opencode-windows-x64.zip", }, ], dir_entry: None, @@ -432,8 +432,8 @@ pub fn get_agent_meta(agent_type: AgentType) -> AcpAgentMeta { // leading `KEY=value` argv and sacp's `parse_env_var` only accepts // `[A-Za-z0-9_]` env names, which npm's `@scope:registry` key is not.) distribution: AgentDistribution::Npx { - version: "0.2.111", - package: "@xai-official/grok@0.2.111", + version: "0.2.112", + package: "@xai-official/grok@0.2.112", cmd: "grok", // Only the ACP subcommand lives here. Grok's ROOT-level launch // flags (`--no-auto-update` always, `--permission-mode ` @@ -444,7 +444,7 @@ pub fn get_agent_meta(agent_type: AgentType) -> AcpAgentMeta { // args rather than appending after. args: &["agent", "stdio"], env: &[], - // `@xai-official/grok@0.2.111` declares `engines.node: ">=20"`; + // `@xai-official/grok@0.2.112` declares `engines.node: ">=20"`; // surface that in preflight so Node 18 isn't silently accepted. node_required: Some("20.0.0"), }, @@ -671,11 +671,11 @@ mod tests { assert_npx_version(AgentType::Pi, "0.0.32", "pi-acp@0.0.32", Some("22.0.0")); assert_npx_version( AgentType::Grok, - "0.2.111", - "@xai-official/grok@0.2.111", + "0.2.112", + "@xai-official/grok@0.2.112", Some("20.0.0"), ); - assert_binary_version(AgentType::OpenCode, "1.18.4", "/releases/download/v1.18.4/"); + assert_binary_version(AgentType::OpenCode, "1.18.5", "/releases/download/v1.18.5/"); assert_uvx_version( AgentType::Hermes, "0.19.0", diff --git a/src-tauri/src/acp/types.rs b/src-tauri/src/acp/types.rs index bb4d67b05..6db403b54 100644 --- a/src-tauri/src/acp/types.rs +++ b/src-tauri/src/acp/types.rs @@ -665,6 +665,10 @@ pub struct AcpAgentInfo { /// list) round-tripped into the settings editor. Only populated for /// `AgentType::Codex`, and only in api-key mode (no bound provider). pub codex_model_catalog: Option, + /// Parsed sandbox / approval keys from `~/.codex/config.toml` backing the + /// Codex panel's structured controls. Only populated for `AgentType::Codex`. + /// Derived from `codex_config_toml`. + pub codex_sandbox_settings: Option, pub cline_secrets_json: Option, /// Raw `~/.hermes/config.yaml` text, attached for the Hermes settings panel's /// advanced editor. Only populated for `AgentType::Hermes`. @@ -688,6 +692,151 @@ pub struct AcpAgentInfo { pub model_provider_id: Option, } +/// The `~/.codex/config.toml` sandbox / approval keys surfaced as structured +/// controls in the Codex settings panel. +/// +/// ## Why these keys matter even though the composer already has a preset +/// +/// codex-acp attaches `approvalPolicy` + `sandboxPolicy` to EVERY normal turn +/// (`runTurn`), sourced from the composer's mode preset — so for ordinary +/// prompts these config keys are overridden per turn and invisible. `/goal` is +/// different: `thread/goal/set` only records the objective and the turn is then +/// started SERVER-side with no policy attached (same for `/review` and +/// `/compact`), so those turns fall back to the thread defaults — i.e. exactly +/// these config.toml keys. Without them a user who picked "Agent (full access)" +/// still gets `on-request` + `workspace-write` + no network inside `/goal`. +/// +/// Vocabulary is pinned to codex-cli 0.145.0: `AskForApproval` +/// (codex-rs/protocol/src/protocol.rs) and `SandboxMode` +/// (codex-rs/protocol/src/config_types.rs). +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct CodexSandboxSettings { + /// Root `approval_policy` when it is one of the plain string variants + /// (`untrusted` / `on-request` / `never`). The legacy `on-failure` spelling + /// is a serde ALIAS of `on-request` upstream, so it is normalized to + /// `on-request` on read. `None` when the key is absent or when the granular + /// table form is in use — the enum is externally tagged, so a value is + /// either a string or the table below, never both. + pub approval_policy: Option, + /// The `approval_policy = { granular = { … } }` variant. + pub granular: Option, + /// Root `sandbox_mode` — `read-only` / `workspace-write` / `danger-full-access`. + /// `None` = absent, in which case codex falls back to `workspace-write` for + /// any directory carrying a `[projects]` trust decision, else `read-only` + /// (and on Windows without the experimental sandbox, `workspace-write` is + /// further downgraded to `read-only`). + pub sandbox_mode: Option, + /// `[sandbox_workspace_write]`. + pub workspace_write: CodexWorkspaceWrite, + /// `default_permissions` is set, which makes codex resolve permissions + /// through the profile pipeline and ignore `sandbox_mode` entirely + /// (`resolve_permission_config_syntax` evaluates `default_permissions` after + /// `sandbox_mode` within a layer, so it wins). Verified against 0.145: + /// `default_permissions = ":read-only"` alongside + /// `sandbox_mode = "danger-full-access"` yields a read-only sandbox. The + /// panel disables the sandbox controls and says why. + pub shadowed_by_default_permissions: bool, + /// A `[permissions]` profile table exists. Combined with an absent + /// `default_permissions` that is a hard startup error upstream ("config + /// defines `[permissions]` profiles but does not set `default_permissions`"), + /// so the panel surfaces it instead of writing into a config that cannot + /// load. + pub has_permissions_table: bool, +} + +/// `approval_policy = { granular = { … } }` — `GranularApprovalConfig` upstream. +/// +/// Field names stay snake_case on BOTH the read projection and the write payload +/// (unlike the camelCase parent payload) so one type serves both directions. +/// +/// `sandbox_approval`, `rules` and `mcp_elicitations` carry no `#[serde(default)]` +/// upstream: omitting any of them makes codex refuse to load the config +/// (verified — `thread/start` fails with "missing field `sandbox_approval`"), so +/// all five keys are always written together. +#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize)] +pub struct CodexGranularApproval { + /// Shell command approval requests, including inline + /// `with_additional_permissions` / `require_escalated` escalations. + pub sandbox_approval: bool, + /// Prompts triggered by execpolicy `prompt` rules. + pub rules: bool, + /// Prompts triggered by skill script execution. + pub skill_approval: bool, + /// Prompts triggered by the `request_permissions` tool. + pub request_permissions: bool, + /// MCP elicitation prompts. + pub mcp_elicitations: bool, +} + +/// `[sandbox_workspace_write]` — only consulted when the effective sandbox mode +/// is `workspace-write`. Every field defaults to false/empty upstream, so an +/// absent key and an explicit `false` are equivalent; codeg writes only the +/// non-default ones to keep the file tidy. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct CodexWorkspaceWrite { + /// Extra writable folders beyond cwd. Upstream these are `AbsolutePathBuf`, + /// but a RELATIVE entry is not rejected — codex resolves it against + /// `CODEX_HOME` (verified: `"rel/dir"` became `~/.codex/rel/dir`). codeg + /// therefore refuses to write relative entries rather than let a user + /// silently grant write access inside `~/.codex`. + pub writable_roots: Vec, + /// Allow outbound network access from inside the sandbox. + pub network_access: bool, + /// Drop the per-user `TMPDIR` from the default writable roots. + pub exclude_tmpdir_env_var: bool, + /// Drop `/tmp` from the default writable roots (UNIX). + pub exclude_slash_tmp: bool, +} + +/// `absent` vs `null` for a nullable field: serde folds both into `None` on a +/// plain `Option`, so a field that must distinguish "not sent" from "sent as +/// null" needs `Option>` plus this deserializer. +fn double_option<'de, D, T>(deserializer: D) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de>, +{ + Option::deserialize(deserializer).map(Some) +} + +/// The structured-control values the Codex settings panel sends on save. Merged +/// format-preservingly (via `toml_edit`) onto the current `~/.codex/config.toml` +/// so comments and unmanaged keys survive. camelCase on the wire to match the +/// enclosing request body, except the nested `granular` object (see +/// [`CodexGranularApproval`]). +/// +/// **This is a per-field PATCH, not a snapshot.** An absent field leaves its key +/// exactly as the merge base has it. That matters because the settings panel +/// sends the raw config.toml text alongside this patch and the patch is applied +/// last: if it carried the whole group, any key the user had hand-edited in the +/// raw editor — a surface the panel never parses back into its controls — would +/// be silently reverted by the panel's stale value for that key. Sending only +/// what the user actually moved keeps the two surfaces from fighting. +/// +/// Field semantics: +/// - `approval_policy` / `granular`: move as a PAIR (the upstream enum is one +/// externally tagged key, either a string or a table). Both absent leaves the +/// key untouched; both `Some(None)` removes it; exactly one carrying a value +/// writes that form; both carrying values is rejected. +/// - `sandbox_mode`: absent leaves, `Some(None)` removes, `Some(Some(v))` sets. +/// - workspace-write fields: absent leaves; a value sets it, and `false` / an +/// empty list removes the key (identical to codex's own defaults). A section +/// left with no keys is removed wholesale. +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CodexSandboxStructuredConfig { + #[serde(default, deserialize_with = "double_option")] + pub approval_policy: Option>, + #[serde(default, deserialize_with = "double_option")] + pub granular: Option>, + #[serde(default, deserialize_with = "double_option")] + pub sandbox_mode: Option>, + pub writable_roots: Option>, + pub network_access: Option, + pub exclude_tmpdir_env_var: Option, + pub exclude_slash_tmp: Option, +} + /// The subset of `~/.grok/config.toml` keys surfaced as structured controls in /// the Grok settings panel. Each field mirrors one documented key (see /// docs.x.ai/build/settings/reference); `None` means the key is absent. diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index a462f2bb6..34062b7ad 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -15,8 +15,10 @@ use crate::acp::preflight::{self, PreflightResult}; use crate::acp::registry; use crate::acp::types::{ AcpAgentInfo, AgentDiagnosticsReport, AgentSkillContent, AgentSkillItem, AgentSkillLayout, - AgentSkillLocation, AgentSkillScope, AgentSkillsListResult, ConfigStaleKind, ConnectionStatus, - DiagCheck, DiagLevel, DiagSection, DiagnosticsVerdict, GrokSettings, GrokStructuredConfig, + AgentSkillLocation, AgentSkillScope, AgentSkillsListResult, CodexGranularApproval, + CodexSandboxSettings, CodexSandboxStructuredConfig, CodexWorkspaceWrite, ConfigStaleKind, + ConnectionStatus, DiagCheck, DiagLevel, DiagSection, DiagnosticsVerdict, GrokSettings, + GrokStructuredConfig, }; #[cfg(feature = "tauri-runtime")] use crate::acp::types::{ConnectionInfo, ForkResultInfo, PromptInputBlock}; @@ -2621,6 +2623,39 @@ fn codex_config_projection_from_toml(raw_toml: &str) -> serde_json::Map Result { + let path = codex_config_toml_path(); + match fs::read_to_string(&path) { + Ok(text) => Ok(text), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(String::new()), + Err(e) => Err(AcpError::protocol(format!( + "read codex config.toml failed: {e}" + ))), + } +} + +/// The plain-string `approval_policy` values codex 0.145 accepts. +/// `AskForApproval` also has a `Granular(GranularApprovalConfig)` variant, which +/// is a TOML *table* rather than a string and is handled separately. +const CODEX_APPROVAL_POLICIES: &[&str] = &["untrusted", "on-request", "never"]; + +/// `SandboxMode` — the complete upstream vocabulary. +const CODEX_SANDBOX_MODES: &[&str] = &["read-only", "workspace-write", "danger-full-access"]; + +/// Parse the sandbox / approval keys backing the Codex panel's structured +/// controls from a raw `~/.codex/config.toml`. Read-only; uses the `toml` crate +/// so inline tables (`approval_policy = { granular = { … } }`), dotted keys and +/// arrays all read correctly. A malformed file yields defaults — the panel then +/// shows "unset" and the raw editor below it is where the user fixes the syntax. +fn parse_codex_sandbox_settings(raw_toml: &str) -> CodexSandboxSettings { + let Ok(table) = raw_toml.parse::() else { + return CodexSandboxSettings::default(); + }; + + // `approval_policy` is an externally tagged enum: a bare string for the unit + // variants, or a single-key table for `granular`. `on-failure` is a serde + // alias of `on-request` upstream, so it is folded here rather than shown as + // a fourth option the panel would have to round-trip. + let approval_item = table.get("approval_policy"); + let approval_policy = approval_item + .and_then(toml::Value::as_str) + .map(str::trim) + .map(|value| match value { + "on-failure" => "on-request", + other => other, + }) + .filter(|value| CODEX_APPROVAL_POLICIES.contains(value)) + .map(str::to_string); + let granular = approval_item + .and_then(toml::Value::as_table) + .and_then(|policy| policy.get("granular")) + .and_then(toml::Value::as_table) + .map(|granular| { + let flag = |key: &str| { + granular + .get(key) + .and_then(toml::Value::as_bool) + .unwrap_or(false) + }; + CodexGranularApproval { + sandbox_approval: flag("sandbox_approval"), + rules: flag("rules"), + skill_approval: flag("skill_approval"), + request_permissions: flag("request_permissions"), + mcp_elicitations: flag("mcp_elicitations"), + } + }); + + let sandbox_mode = table + .get("sandbox_mode") + .and_then(toml::Value::as_str) + .map(str::trim) + .filter(|value| CODEX_SANDBOX_MODES.contains(value)) + .map(str::to_string); + + let ws = table + .get("sandbox_workspace_write") + .and_then(toml::Value::as_table); + let ws_flag = |key: &str| { + ws.and_then(|t| t.get(key)) + .and_then(toml::Value::as_bool) + .unwrap_or(false) + }; + let writable_roots = ws + .and_then(|t| t.get("writable_roots")) + .and_then(toml::Value::as_array) + .map(|entries| { + entries + .iter() + .filter_map(toml::Value::as_str) + .map(str::trim) + .filter(|entry| !entry.is_empty()) + .map(str::to_string) + .collect() + }) + .unwrap_or_default(); + + CodexSandboxSettings { + approval_policy, + granular, + sandbox_mode, + workspace_write: CodexWorkspaceWrite { + writable_roots, + network_access: ws_flag("network_access"), + exclude_tmpdir_env_var: ws_flag("exclude_tmpdir_env_var"), + exclude_slash_tmp: ws_flag("exclude_slash_tmp"), + }, + shadowed_by_default_permissions: table.contains_key("default_permissions"), + has_permissions_table: table + .get("permissions") + .and_then(toml::Value::as_table) + .is_some_and(|profiles| !profiles.is_empty()), + } +} + +/// A `writable_roots` entry codex will accept as-is. Upstream types the field as +/// `AbsolutePathBuf`, but a relative entry does NOT error — it is resolved +/// against `CODEX_HOME`, so `"rel/dir"` silently becomes `~/.codex/rel/dir`. +/// Rejecting it here is the only way the user learns their path was not what +/// they meant. Both POSIX (`/x`) and Windows (`C:\x`, `\\server\share`) shapes +/// are accepted regardless of the host, since the config file is portable. +fn is_absolute_config_path(value: &str) -> bool { + let value = value.trim(); + if value.starts_with('/') || value.starts_with("\\\\") { + return true; + } + let mut chars = value.chars(); + match (chars.next(), chars.next(), chars.next()) { + (Some(drive), Some(':'), Some('\\' | '/')) => drive.is_ascii_alphabetic(), + _ => false, + } +} + +/// Apply the Codex panel's sandbox / approval PATCH to the raw config.toml text, +/// format-preservingly (comments and unmanaged keys are kept). Values are +/// validated against the upstream vocabularies first, so a UI bug can never +/// write a config codex refuses to load. +/// +/// Only the fields the patch actually carries are touched — see +/// [`CodexSandboxStructuredConfig`] for why that matters. Within a carried +/// field the removal rules match codex's own defaults: an unset approval/sandbox +/// drops its key; a `false` flag or empty `writable_roots` drops that key; and a +/// `[sandbox_workspace_write]` left with no keys at all drops the section. +fn apply_codex_sandbox_config( + base_toml: &str, + settings: &CodexSandboxStructuredConfig, +) -> Result { + let mut doc = base_toml + .parse::() + .map_err(|e| AcpError::protocol(format!("invalid codex config.toml: {e}")))?; + + // `approval_policy` is one externally tagged key, so the preset and the + // granular table travel together: either both absent (leave the key alone) + // or both present with at most one carrying a value. + let approval = settings + .approval_policy + .as_ref() + .map(|policy| policy.as_deref()); + let granular = settings.granular; + if approval.is_some() || granular.is_some() { + let preset = approval.flatten(); + let granular = granular.flatten(); + if preset.is_some() && granular.is_some() { + return Err(AcpError::protocol( + "approval_policy cannot be both a preset and a granular table", + )); + } + match (preset, granular) { + (Some(policy), _) => { + let policy = policy.trim(); + if !CODEX_APPROVAL_POLICIES.contains(&policy) { + return Err(AcpError::protocol(format!( + "unsupported codex approval_policy: {policy}" + ))); + } + // Assigning a value over an existing `[approval_policy.granular]` + // table replaces the whole item, which is what switching away + // from granular must do — an emptied table would fail to + // deserialize. + doc["approval_policy"] = toml_edit::value(policy); + } + (None, Some(granular)) => { + // All five keys are always written: `sandbox_approval`, `rules` + // and `mcp_elicitations` have no upstream default, so a partial + // table makes codex refuse to load the config. + let mut table = toml_edit::Table::new(); + table.insert( + "sandbox_approval", + toml_edit::value(granular.sandbox_approval), + ); + table.insert("rules", toml_edit::value(granular.rules)); + table.insert("skill_approval", toml_edit::value(granular.skill_approval)); + table.insert( + "request_permissions", + toml_edit::value(granular.request_permissions), + ); + table.insert( + "mcp_elicitations", + toml_edit::value(granular.mcp_elicitations), + ); + let mut parent = toml_edit::Table::new(); + parent.insert("granular", toml_edit::Item::Table(table)); + doc["approval_policy"] = toml_edit::Item::Table(parent); + } + (None, None) => { + doc.remove("approval_policy"); + } + } + } + + if let Some(mode) = settings.sandbox_mode.as_ref() { + match mode.as_deref().map(str::trim).filter(|m| !m.is_empty()) { + Some(mode) => { + if !CODEX_SANDBOX_MODES.contains(&mode) { + return Err(AcpError::protocol(format!( + "unsupported codex sandbox_mode: {mode}" + ))); + } + doc["sandbox_mode"] = toml_edit::value(mode); + } + None => { + doc.remove("sandbox_mode"); + } + } + } + + let roots = match settings.writable_roots.as_ref() { + Some(roots) => { + let roots: Vec = roots + .iter() + .map(|root| root.trim().to_string()) + .filter(|root| !root.is_empty()) + .collect(); + if let Some(bad) = roots.iter().find(|root| !is_absolute_config_path(root)) { + return Err(AcpError::protocol(format!( + "writable_roots entries must be absolute paths (codex resolves relative entries against CODEX_HOME): {bad}" + ))); + } + Some(roots) + } + None => None, + }; + let ws_flags = [ + ("network_access", settings.network_access), + ("exclude_tmpdir_env_var", settings.exclude_tmpdir_env_var), + ("exclude_slash_tmp", settings.exclude_slash_tmp), + ]; + if roots.is_some() || ws_flags.iter().any(|(_, flag)| flag.is_some()) { + let item = &mut doc["sandbox_workspace_write"]; + if item.is_none() { + *item = toml_edit::Item::Table(toml_edit::Table::new()); + } else if item.as_table_like_mut().is_none() { + return Err(AcpError::protocol( + "cannot set [sandbox_workspace_write]: it exists but is not a table", + )); + } + if let Some(table) = item.as_table_like_mut() { + if let Some(roots) = roots { + if roots.is_empty() { + table.remove("writable_roots"); + } else { + let mut array = toml_edit::Array::new(); + for root in &roots { + array.push(root.as_str()); + } + table.insert("writable_roots", toml_edit::value(array)); + } + } + for (key, flag) in ws_flags { + match flag { + Some(true) => { + table.insert(key, toml_edit::value(true)); + } + // `false` is codex's own default for every flag here, so + // removing the key and writing `= false` are equivalent — + // removing keeps the file minimal. + Some(false) => { + table.remove(key); + } + None => {} + } + } + } + // Prune a section the patch just emptied — including one that was + // already empty in the base — so no bare `[sandbox_workspace_write]` + // header is left behind. + if doc + .get("sandbox_workspace_write") + .and_then(|item| item.as_table_like()) + .is_some_and(|table| table.is_empty()) + { + doc.remove("sandbox_workspace_write"); + } + } + + Ok(doc.to_string()) +} + /// Read the raw `~/.grok/config.toml` for the Grok settings panel's config-file /// editor. `GROK_HOME` is honored via `resolve_grok_home_dir`. Returns `None` /// when the file is absent (Grok writes it lazily on first `grok login`/run). @@ -8062,6 +8393,17 @@ pub(crate) async fn acp_list_agents_core(db: &AppDatabase) -> Result Result, codex_config_toml: Option, codex_model_catalog: Option, + codex_sandbox: Option, grok_config_toml: Option, grok_structured: Option, cursor_cli_config_json: Option, @@ -8608,11 +8952,24 @@ pub(crate) async fn acp_update_agent_config_core( } if agent_type == AgentType::Codex { - if codex_auth_json.is_some() || codex_config_toml.is_some() { - persist_codex_native_config_files( - codex_auth_json.as_deref(), - codex_config_toml.as_deref(), - )?; + // Mirrors the Grok/Cursor flow. The advanced raw editor sends the whole + // file (`codex_config_toml = Some(text)`), so that text is the verbatim + // base; the sandbox/approval controls send only a patch, merged onto the + // CURRENT on-disk config (read fresh, failing loudly on a real read + // error) so a stale in-memory snapshot can't drop keys written by codex + // itself or another window since the panel opened. + if codex_auth_json.is_some() || codex_config_toml.is_some() || codex_sandbox.is_some() { + let merged_toml = match &codex_sandbox { + Some(sandbox) => { + let base = match codex_config_toml { + Some(text) => text, + None => read_codex_config_or_empty()?, + }; + Some(apply_codex_sandbox_config(&base, sandbox)?) + } + None => codex_config_toml, + }; + persist_codex_native_config_files(codex_auth_json.as_deref(), merged_toml.as_deref())?; } // The frontend has already patched config.toml's `model_catalog_json` + // root `model` into `codex_config_toml` (comment-preserving text patch); @@ -8719,6 +9076,7 @@ pub(crate) async fn acp_update_agent_config_and_refresh( codex_auth_json: Option, codex_config_toml: Option, codex_model_catalog: Option, + codex_sandbox: Option, grok_config_toml: Option, grok_structured: Option, cursor_cli_config_json: Option, @@ -8735,6 +9093,7 @@ pub(crate) async fn acp_update_agent_config_and_refresh( codex_auth_json, codex_config_toml, codex_model_catalog, + codex_sandbox, grok_config_toml, grok_structured, cursor_cli_config_json, @@ -8755,6 +9114,7 @@ pub async fn acp_update_agent_config( codex_auth_json: Option, codex_config_toml: Option, codex_model_catalog: Option, + codex_sandbox: Option, grok_config_toml: Option, grok_structured: Option, cursor_cli_config_json: Option, @@ -8776,6 +9136,7 @@ pub async fn acp_update_agent_config( codex_auth_json, codex_config_toml, codex_model_catalog, + codex_sandbox, grok_config_toml, grok_structured, cursor_cli_config_json, @@ -10293,6 +10654,400 @@ mod tests { ); } + // ---- Codex sandbox / approval structured controls ------------------- + // Every expectation below was verified against a real codex-cli 0.145.0 by + // writing the shape into an isolated `CODEX_HOME` and reading the resulting + // `thread/start` sandbox back. + + #[test] + fn parse_codex_sandbox_settings_reads_plain_keys() { + let s = parse_codex_sandbox_settings( + "approval_policy = \"never\"\nsandbox_mode = \"danger-full-access\"\n", + ); + assert_eq!(s.approval_policy.as_deref(), Some("never")); + assert_eq!(s.sandbox_mode.as_deref(), Some("danger-full-access")); + assert!(s.granular.is_none()); + assert!(!s.shadowed_by_default_permissions); + } + + #[test] + fn parse_codex_sandbox_settings_normalizes_legacy_on_failure() { + // `on-failure` is a serde ALIAS of `on-request` upstream, not a distinct + // policy, so the panel must show it as `on-request` rather than as an + // unknown value it would then clobber. + let s = parse_codex_sandbox_settings("approval_policy = \"on-failure\"\n"); + assert_eq!(s.approval_policy.as_deref(), Some("on-request")); + } + + #[test] + fn parse_codex_sandbox_settings_reads_granular_in_both_toml_forms() { + let inline = parse_codex_sandbox_settings( + "approval_policy = { granular = { sandbox_approval = true, rules = false, \ + skill_approval = true, request_permissions = false, mcp_elicitations = true } }\n", + ); + let section = parse_codex_sandbox_settings( + "[approval_policy.granular]\nsandbox_approval = true\nrules = false\n\ + skill_approval = true\nrequest_permissions = false\nmcp_elicitations = true\n", + ); + for s in [inline, section] { + assert!(s.approval_policy.is_none(), "granular is not a string form"); + let g = s.granular.expect("granular table"); + assert!(g.sandbox_approval && g.skill_approval && g.mcp_elicitations); + assert!(!g.rules && !g.request_permissions); + } + } + + #[test] + fn parse_codex_sandbox_settings_reads_workspace_write_group() { + let s = parse_codex_sandbox_settings( + "sandbox_mode = \"workspace-write\"\n\n[sandbox_workspace_write]\n\ + writable_roots = [\"/srv/one\", \"/srv/two\"]\nnetwork_access = true\n\ + exclude_slash_tmp = true\n", + ); + assert_eq!(s.workspace_write.writable_roots, ["/srv/one", "/srv/two"]); + assert!(s.workspace_write.network_access); + assert!(s.workspace_write.exclude_slash_tmp); + assert!(!s.workspace_write.exclude_tmpdir_env_var); + } + + #[test] + fn parse_codex_sandbox_settings_flags_profile_shadowing() { + // `default_permissions` makes codex resolve permissions through the + // profile pipeline and ignore `sandbox_mode` entirely — verified live: + // `:read-only` + `danger-full-access` yields a read-only sandbox. + let s = parse_codex_sandbox_settings( + "sandbox_mode = \"danger-full-access\"\ndefault_permissions = \":read-only\"\n\n\ + [permissions.tight]\nfile_system = \"restricted\"\n", + ); + assert!(s.shadowed_by_default_permissions); + assert!(s.has_permissions_table); + } + + #[test] + fn parse_codex_sandbox_settings_ignores_unknown_values_and_bad_toml() { + let unknown = parse_codex_sandbox_settings( + "approval_policy = \"yolo\"\nsandbox_mode = \"wide-open\"\n", + ); + assert!(unknown.approval_policy.is_none()); + assert!(unknown.sandbox_mode.is_none()); + let broken = parse_codex_sandbox_settings("== not toml =="); + assert!(broken.sandbox_mode.is_none()); + } + + /// Set a field: `Some(Some(v))`. Clear it: `Some(None)`. Leave it exactly as + /// the base has it: `None` (the struct default). + fn set(value: T) -> Option> { + Some(Some(value)) + } + + #[test] + fn apply_codex_sandbox_config_writes_and_preserves_unmanaged_keys() { + let base = "# keep me\nmodel = \"gpt-5\"\n\n[features]\nskills = true\n"; + let merged = apply_codex_sandbox_config( + base, + &CodexSandboxStructuredConfig { + approval_policy: set("never".into()), + sandbox_mode: set("workspace-write".into()), + writable_roots: Some(vec!["/srv/extra".into()]), + network_access: Some(true), + ..Default::default() + }, + ) + .unwrap(); + assert!(merged.contains("# keep me"), "comments survive"); + assert!(merged.contains("model = \"gpt-5\"")); + assert!(merged.contains("skills = true")); + let back = parse_codex_sandbox_settings(&merged); + assert_eq!(back.approval_policy.as_deref(), Some("never")); + assert_eq!(back.sandbox_mode.as_deref(), Some("workspace-write")); + assert_eq!(back.workspace_write.writable_roots, ["/srv/extra"]); + assert!(back.workspace_write.network_access); + } + + #[test] + fn apply_codex_sandbox_config_writes_all_five_granular_keys() { + // `sandbox_approval`, `rules` and `mcp_elicitations` have no upstream + // default: a partial table makes codex refuse to load config.toml + // ("missing field `sandbox_approval`"), so all five are always written. + let merged = apply_codex_sandbox_config( + "", + &CodexSandboxStructuredConfig { + granular: set(CodexGranularApproval { + sandbox_approval: true, + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + for key in [ + "sandbox_approval", + "rules", + "skill_approval", + "request_permissions", + "mcp_elicitations", + ] { + assert!(merged.contains(key), "granular table must carry {key}"); + } + assert!(parse_codex_sandbox_settings(&merged).granular.is_some()); + } + + #[test] + fn apply_codex_sandbox_config_switches_granular_back_to_a_preset() { + // The string form must REPLACE the table; an emptied `[approval_policy]` + // would fail to deserialize as the externally tagged enum. + let base = "[approval_policy.granular]\nsandbox_approval = true\nrules = true\n\ + skill_approval = false\nrequest_permissions = false\nmcp_elicitations = true\n"; + let merged = apply_codex_sandbox_config( + base, + &CodexSandboxStructuredConfig { + approval_policy: set("on-request".into()), + granular: Some(None), + ..Default::default() + }, + ) + .unwrap(); + let back = parse_codex_sandbox_settings(&merged); + assert_eq!(back.approval_policy.as_deref(), Some("on-request")); + assert!(back.granular.is_none(), "the granular table is gone"); + assert!(!merged.contains("sandbox_approval")); + } + + #[test] + fn apply_codex_sandbox_config_keeps_root_keys_out_of_existing_tables() { + // TOML positioning guard: a root scalar or the `[approval_policy]` + // granular table must not be emitted AFTER an existing section, which + // would silently reparent unrelated root keys into that section. + let base = "model = \"gpt-5\"\nmodel_provider = \"codeg\"\n\n\ + [features]\nskills = true\n\n\ + [mcp_servers.ctx]\ncommand = \"npx\"\n"; + let merged = apply_codex_sandbox_config( + base, + &CodexSandboxStructuredConfig { + granular: set(CodexGranularApproval { + sandbox_approval: true, + rules: true, + ..Default::default() + }), + sandbox_mode: set("workspace-write".into()), + network_access: Some(true), + ..Default::default() + }, + ) + .unwrap(); + let table = merged + .parse::() + .expect("merged config must stay valid TOML"); + assert_eq!( + table.get("model").and_then(toml::Value::as_str), + Some("gpt-5"), + "root keys must stay at the root" + ); + assert_eq!( + table.get("model_provider").and_then(toml::Value::as_str), + Some("codeg") + ); + assert_eq!( + table + .get("features") + .and_then(toml::Value::as_table) + .and_then(|t| t.get("skills")) + .and_then(toml::Value::as_bool), + Some(true), + "unmanaged sections keep their own keys" + ); + assert!(table.get("mcp_servers").is_some()); + let back = parse_codex_sandbox_settings(&merged); + assert!(back.granular.is_some()); + assert_eq!(back.sandbox_mode.as_deref(), Some("workspace-write")); + assert!(back.workspace_write.network_access); + } + + #[test] + fn apply_codex_sandbox_config_removes_keys_and_empty_section() { + let base = "approval_policy = \"never\"\nsandbox_mode = \"danger-full-access\"\n\n\ + [sandbox_workspace_write]\nnetwork_access = true\n\n\ + [features]\nskills = true\n"; + let merged = apply_codex_sandbox_config( + base, + &CodexSandboxStructuredConfig { + approval_policy: Some(None), + granular: Some(None), + sandbox_mode: Some(None), + writable_roots: Some(vec![]), + network_access: Some(false), + exclude_tmpdir_env_var: Some(false), + exclude_slash_tmp: Some(false), + }, + ) + .unwrap(); + let back = parse_codex_sandbox_settings(&merged); + assert!(back.approval_policy.is_none()); + assert!(back.sandbox_mode.is_none()); + assert!( + !merged.contains("sandbox_workspace_write"), + "empty section removed" + ); + assert!(merged.contains("skills = true"), "other sections untouched"); + } + + #[test] + fn apply_codex_sandbox_config_leaves_absent_fields_untouched() { + // The core of the patch contract. The settings panel sends the raw + // config.toml text alongside this patch and the patch wins, so a field + // the user did not move must not be written from the panel's (possibly + // stale) view — otherwise a hand-edit in the raw editor gets reverted. + let base = "approval_policy = \"never\"\nsandbox_mode = \"read-only\"\n\n\ + [sandbox_workspace_write]\nwritable_roots = [\"/srv/keep\"]\n\ + network_access = true\nexclude_slash_tmp = true\n"; + let merged = apply_codex_sandbox_config( + base, + // Only the sandbox mode moved. + &CodexSandboxStructuredConfig { + sandbox_mode: set("workspace-write".into()), + ..Default::default() + }, + ) + .unwrap(); + let back = parse_codex_sandbox_settings(&merged); + assert_eq!(back.sandbox_mode.as_deref(), Some("workspace-write")); + assert_eq!( + back.approval_policy.as_deref(), + Some("never"), + "an untouched approval must survive the patch" + ); + assert_eq!(back.workspace_write.writable_roots, ["/srv/keep"]); + assert!(back.workspace_write.network_access); + assert!(back.workspace_write.exclude_slash_tmp); + } + + #[test] + fn apply_codex_sandbox_config_empty_patch_is_a_no_op() { + let base = "model = \"gpt-5\"\nsandbox_mode = \"read-only\"\n\n\ + [approval_policy.granular]\nsandbox_approval = true\nrules = true\n\ + skill_approval = false\nrequest_permissions = false\n\ + mcp_elicitations = true\n\n\ + [sandbox_workspace_write]\nnetwork_access = true\n"; + // An all-absent patch must not touch a single key. + let merged = + apply_codex_sandbox_config(base, &CodexSandboxStructuredConfig::default()).unwrap(); + assert_eq!(merged.trim(), base.trim()); + } + + #[test] + fn apply_codex_sandbox_config_partial_workspace_write_patch() { + // Toggling one flag must not disturb its siblings or the roots list. + let base = "[sandbox_workspace_write]\nwritable_roots = [\"/srv/keep\"]\n\ + network_access = true\n"; + let merged = apply_codex_sandbox_config( + base, + &CodexSandboxStructuredConfig { + exclude_slash_tmp: Some(true), + ..Default::default() + }, + ) + .unwrap(); + let back = parse_codex_sandbox_settings(&merged); + assert_eq!(back.workspace_write.writable_roots, ["/srv/keep"]); + assert!(back.workspace_write.network_access); + assert!(back.workspace_write.exclude_slash_tmp); + + // Clearing the last remaining key drops the now-bare section header. + let emptied = apply_codex_sandbox_config( + "[sandbox_workspace_write]\nnetwork_access = true\n", + &CodexSandboxStructuredConfig { + network_access: Some(false), + ..Default::default() + }, + ) + .unwrap(); + assert!(!emptied.contains("sandbox_workspace_write")); + } + + #[test] + fn apply_codex_sandbox_config_rejects_bad_input() { + // Relative writable_roots are NOT rejected by codex — they resolve + // against CODEX_HOME ("rel/dir" → ~/.codex/rel/dir), silently granting + // write access somewhere the user never meant. So codeg rejects them. + assert!(apply_codex_sandbox_config( + "", + &CodexSandboxStructuredConfig { + writable_roots: Some(vec!["rel/dir".into()]), + ..Default::default() + } + ) + .is_err()); + assert!(apply_codex_sandbox_config( + "", + &CodexSandboxStructuredConfig { + sandbox_mode: set("wide-open".into()), + ..Default::default() + } + ) + .is_err()); + assert!( + apply_codex_sandbox_config( + "", + &CodexSandboxStructuredConfig { + approval_policy: set("never".into()), + granular: set(CodexGranularApproval::default()), + ..Default::default() + } + ) + .is_err(), + "a string and a granular table cannot coexist" + ); + assert!( + apply_codex_sandbox_config("== not toml ==", &CodexSandboxStructuredConfig::default()) + .is_err(), + "a malformed base must error, never silently overwrite the user's config" + ); + } + + #[test] + fn apply_codex_sandbox_config_accepts_windows_absolute_roots() { + let merged = apply_codex_sandbox_config( + "", + &CodexSandboxStructuredConfig { + writable_roots: Some(vec!["C:\\work\\repo".into(), "\\\\srv\\share".into()]), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + parse_codex_sandbox_settings(&merged) + .workspace_write + .writable_roots + .len(), + 2 + ); + } + + #[test] + fn codex_projection_folds_sandbox_keys_for_the_fingerprint() { + // These keys only take effect at `thread/start`, so a running session + // must be marked restart-required when they change; the projection is + // what `fingerprint_config` hashes. + let plain = codex_config_projection_from_toml("model = \"gpt-5\"\n"); + assert!(!plain.contains_key("sandboxMode")); + assert!(!plain.contains_key("sandboxWorkspaceWrite")); + let with_sandbox = codex_config_projection_from_toml( + "model = \"gpt-5\"\napproval_policy = \"never\"\n\ + sandbox_mode = \"workspace-write\"\n\n\ + [sandbox_workspace_write]\nnetwork_access = true\n", + ); + assert_eq!( + with_sandbox.get("approvalPolicy").and_then(|v| v.as_str()), + Some("never") + ); + assert_eq!( + with_sandbox.get("sandboxMode").and_then(|v| v.as_str()), + Some("workspace-write") + ); + assert!(with_sandbox.contains_key("sandboxWorkspaceWrite")); + assert_ne!(plain, with_sandbox, "the fingerprint input must change"); + } + #[test] fn parse_grok_settings_reads_custom_model_and_session() { let toml = "[model.foo]\nmodel = \"foo\"\nbase_url = \"https://api/v1\"\n\ diff --git a/src-tauri/src/web/handlers/acp.rs b/src-tauri/src/web/handlers/acp.rs index d9c3ab502..553b6d046 100644 --- a/src-tauri/src/web/handlers/acp.rs +++ b/src-tauri/src/web/handlers/acp.rs @@ -658,6 +658,7 @@ pub struct AcpUpdateAgentConfigParams { pub codex_auth_json: Option, pub codex_config_toml: Option, pub codex_model_catalog: Option, + pub codex_sandbox: Option, pub grok_config_toml: Option, pub grok_structured: Option, pub cursor_cli_config_json: Option, @@ -676,6 +677,7 @@ pub async fn acp_update_agent_config( params.codex_auth_json, params.codex_config_toml, params.codex_model_catalog, + params.codex_sandbox, params.grok_config_toml, params.grok_structured, params.cursor_cli_config_json, diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index e7a47ebc4..619f6ca97 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "codeg", - "version": "0.21.8", + "version": "0.21.9", "identifier": "app.codeg", "build": { "beforeDevCommand": "pnpm tauri:before-dev", diff --git a/src/app/globals.css b/src/app/globals.css index 93dae2428..cff66c51b 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -1123,10 +1123,10 @@ box-shadow: inset 0 0 0 1px var(--border); } -/* 折叠态用户消息底部渐隐：遮罩内容本身而非叠加层，跟 .qa-marquee-viewport / - .browser-tab-label 同技术，天然适配纯色 bg-secondary 或开工作区背景图后的 - 半透明 ws-msg-secondary。不属于下面的 ws- 门控家族，始终生效。 */ -.collapsed-user-message-fade { +/* 折叠态内容底部渐隐（会话用户消息、提交 tab 的提交信息…）：遮罩内容本身而非叠加层，跟 + .qa-marquee-viewport / .browser-tab-label 同技术，天然适配纯色 bg-secondary 或开工作区 + 背景图后的半透明 ws-msg-secondary。不属于下面的 ws- 门控家族，始终生效。 */ +.collapsed-content-fade { -webkit-mask-image: linear-gradient( to bottom, #000 calc(100% - 2.5rem), @@ -1577,6 +1577,18 @@ border-radius: calc(var(--radius) - 2px); } +/* 并排 diff 的中缝：Monaco 自带样式在 original / modified 两侧各画一条 6px 模糊的 + box-shadow（`--vscode-scrollbar-shadow`），而 `diffEditor.border` 默认 null——中缝于是 + 只剩一团柔和阴影、没有实线，看上去发虚（用户反馈「左右中间的竖线/边框很虚」），开背景图 + 时画布透明更糊。这里去掉这两条阴影，中缝改由主题里的 `diffEditor.border`（monaco-themes.ts） + 画 1px 实线，两者配套。选择器加 `div` 元素限定（0,4,1）压过 Monaco 自己的 (0,4,0) 规则—— + Monaco 的 CSS 是运行时按需插入的，晚于本表，靠特异性而非顺序取胜。仅并排 diff 生效，普通 + 编辑器滚动时的阴影不受影响。 */ +div.monaco-diff-editor.side-by-side .editor.original, +div.monaco-diff-editor.side-by-side .editor.modified { + box-shadow: none; +} + .monaco-editor .codeg-dirty-diff-glyph { margin-left: 5px; position: relative; @@ -2315,14 +2327,17 @@ pointer-events: none; } -/* 标签条左右两端的补缝：group 的 px-2 在首/末 tab 外各留 0.5rem 无线缝（首 tab 非激活时 - 左端断层、末 tab 与新建按钮间右端空隙）。补缝不能画在 group 上（连续线会盖到激活 tab - 下方），且必须跟随边缘 tab 的激活态——激活的首/末 tab 该处是拱门脚（反向圆角描边）而非 - 平线，此处绝不能有平线。故补缝画在「非激活」首/末 tab 自己的 ::after 上（激活 tab 的 - ::after 已改画拱门轮廓，且 :not([data-active]) 排除它）：往 tab 外侧的 px-2 gutter 伸出 - 0.5rem 平线，接上两侧 reserve / 尾部按钮 wrapper 的 ws-strip-line。gutter 落在 group 的 - padding box 内，overflow-hidden 不裁（同 seat 脚）。宽度 = px-2 = 0.5rem（改 group 内边距 - 需同步）。仅 bg-on 生效，关图无补缝（零回归）。 */ +/* 标签条两端的补缝：group 的 pl-2 在首 tab 外留 0.5rem 无线缝（首 tab 非激活时左端断层）。 + 补缝不能画在 group 上（连续线会盖到激活 tab 下方），且必须跟随边缘 tab 的激活态——激活的 + 边缘 tab 该处是拱门脚（反向圆角描边）而非平线，此处绝不能有平线。故补缝画在「非激活」边缘 + tab 自己的 ::after 上（激活 tab 的 ::after 已改画拱门轮廓，且 :not([data-active]) 排除它）： + 往 tab 外侧的 gutter 伸出 0.5rem 平线，接上左侧 reserve 的 ws-strip-line。gutter 落在 group + 的 padding box 内，overflow-hidden 不裁（同 seat 脚）。宽度 = pl-2 = 0.5rem（改 group 内边距 + 需同步）。仅 bg-on 生效，关图无补缝（零回归）。 + :last-child 一支目前不会命中：两条嵌入式标签条（会话 tab-bar.tsx / 文件 + file-workspace-tab-bar.tsx）都把尾部 wrapper 放进 group 当最后一个子元素，末 tab 因此不是 + :last-child，且 group 右侧无 padding——尾部 wrapper 自己的 ws-strip-line 直接从末 tab 右沿 + 起画，无缝可补。保留该分支是给「末 tab 后确实留了右 gutter」的标签条兜底。 */ [data-workspace-bg="on"] .browser-tab-item:first-child:not([data-active="true"])::after, [data-workspace-bg="on"] diff --git a/src/app/workspace/layout.tsx b/src/app/workspace/layout.tsx index 3eb449f89..6c668d003 100644 --- a/src/app/workspace/layout.tsx +++ b/src/app/workspace/layout.tsx @@ -366,14 +366,26 @@ function WorkspaceContent({ children }: { children: React.ReactNode }) { + {/* The divider only belongs to a real two-column split. In conversation + mode the column overlays the whole area, so the handle collapses to + zero width. While files are MAXIMIZED it must go too: that overlay + is translucent under a workspace background image, so the handle's + 1px `bg-border` line stayed visible straight through it — a stray + vertical divider running down the maximized file column and on + through the editor / diff canvas below. There it only turns + invisible (no `w-0`): dropping its width would resize the + conversation panel and reset its stick-to-bottom scroll, the very + thing the overlay approach avoids. */} { + const events: string[] = [] + // The model pair the fake widget currently holds — the state Monaco's + // DiffEditorWidget consults through its `onWillDispose` listeners. + const state: { attached: Record | null } = { attached: null } + return { events, state } +}) + +// Stand-in for `@monaco-editor/react`'s DiffEditor that reproduces the two +// behaviours the component's teardown exists for: the library disposes both +// text models BEFORE the widget, and Monaco's DiffEditorWidget reports a model +// disposed while still attached as a bug ("TextModel got disposed before +// DiffEditorWidget model got reset", rethrown by its default error handler). +vi.mock("@monaco-editor/react", async () => { + const { createElement, useEffect } = await import("react") + + const makeDiffEditor = () => { + let model: Record void }> | null = null + const detach = () => { + model = null + harness.state.attached = null + } + // Monaco's widget listens for `onWillDispose` on the pair it holds: it + // reports the bug and then resets itself, which is why a real teardown + // surfaces one error rather than two. + const makeModel = (side: "original" | "modified") => { + const m = { + dispose: () => { + if (harness.state.attached?.[side] === m) { + harness.events.push(`bug:${side}`) + detach() + } + harness.events.push(`${side}:dispose`) + }, + } + return m + } + model = { original: makeModel("original"), modified: makeModel("modified") } + harness.state.attached = model + const innerEditor = { + revealLineInCenter: () => {}, + setPosition: () => {}, + } + return { + getModel: () => model, + setModel: (next: typeof model) => { + model = next + harness.state.attached = next + }, + getModifiedEditor: () => innerEditor, + getLineChanges: () => [], + onDidUpdateDiff: () => ({ dispose: () => {} }), + dispose: () => harness.events.push("editor:dispose"), + } + } + + const DiffEditor = ({ + onMount, + }: { + onMount?: (editor: ReturnType) => void + }) => { + useEffect(() => { + const editor = makeDiffEditor() + onMount?.(editor) + return () => { + // @monaco-editor/react@4.7.0's unmount path, verbatim in spirit: + // models first, widget last. + const current = editor.getModel() + current?.original?.dispose() + current?.modified?.dispose() + editor.dispose() + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) + return createElement("div", { "data-testid": "mock-diff-editor" }) + } + + return { + DiffEditor, + useMonaco: () => null, + loader: { config: () => {} }, + } +}) + +vi.mock("@/lib/monaco-themes", () => ({ + defineMonacoThemes: () => {}, + MONACO_UNICODE_HIGHLIGHT_OPTIONS: {}, + useMonacoWorkspaceTheme: () => "vs-dark", +})) + +vi.mock("@/hooks/use-appearance", () => ({ + useZoomLevel: () => ({ zoomLevel: 100 }), + useEditorFont: () => ({ + editorFontStack: "monospace", + editorFontSize: 12, + editorLigatures: false, + editorWordWrap: false, + }), +})) + +const { DiffViewer } = await import("./diff-viewer") +const { DiffEditor: MockDiffEditor } = + (await import("@monaco-editor/react")) as unknown as { + DiffEditor: () => React.ReactElement + } + +describe("DiffViewer teardown", () => { + beforeEach(() => { + harness.events.length = 0 + harness.state.attached = null + }) + + it("disposes the diff models only after detaching them from the widget", async () => { + const { unmount } = render() + expect(await screen.findByTestId("mock-diff-editor")).toBeInTheDocument() + + unmount() + + // No "TextModel got disposed before DiffEditorWidget model got reset": + // the widget no longer holds either model by the time it is disposed. + expect(harness.events.filter((e) => e.startsWith("bug:"))).toEqual([]) + // Both models still get disposed exactly once — detaching must not leak + // them (they are anonymous, so nothing else could ever reclaim them). + expect(harness.events).toEqual([ + "original:dispose", + "modified:dispose", + "editor:dispose", + ]) + }) + + it("the fake reproduces the library bug when the component does not intervene", async () => { + // Guards the test above from passing vacuously: rendered bare, the same + // fake records exactly the disposal-while-attached that Monaco flags. + const { unmount } = render() + expect(await screen.findByTestId("mock-diff-editor")).toBeInTheDocument() + + unmount() + + expect(harness.events).toEqual([ + "bug:original", + "original:dispose", + "modified:dispose", + "editor:dispose", + ]) + }) +}) diff --git a/src/components/diff/diff-viewer.tsx b/src/components/diff/diff-viewer.tsx index b16edfd59..3faf19a18 100644 --- a/src/components/diff/diff-viewer.tsx +++ b/src/components/diff/diff-viewer.tsx @@ -1,6 +1,13 @@ "use client" -import { useCallback, useMemo, useRef, useState } from "react" +import { + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from "react" import dynamic from "next/dynamic" import { ChevronLeft, ChevronRight } from "lucide-react" import { useMonaco } from "@monaco-editor/react" @@ -24,6 +31,13 @@ const MonacoDiffEditor = dynamic( { ssr: false } ) +// Commit-synchronous on the client so the teardown below runs in the deletion +// commit — strictly before the library's passive unmount cleanup, whatever the +// parent/child effect order is; a no-op-safe passive effect during the +// static-export prerender where `useLayoutEffect` would warn. +const useIsomorphicLayoutEffect = + typeof window !== "undefined" ? useLayoutEffect : useEffect + export interface DiffViewerProps { original: string modified: string @@ -55,6 +69,9 @@ export function DiffViewer({ [] ) const [currentChangeIndex, setCurrentChangeIndex] = useState(-1) + // Initial diff read, deferred because the first computation lands after + // mount. Cancelled on unmount so it can never poke a disposed widget. + const initialDiffReadRef = useRef | null>(null) const handleEditorMount: DiffOnMount = useCallback((editor) => { diffEditorRef.current = editor @@ -79,9 +96,40 @@ export function DiffViewer({ } editor.onDidUpdateDiff(updateDiffs) - setTimeout(updateDiffs, 300) + initialDiffReadRef.current = setTimeout(updateDiffs, 300) }, []) + // @monaco-editor/react tears the diff editor down in the wrong order on + // unmount: it disposes both text models and only then the widget. Monaco + // flags that as a self-check bug ("TextModel got disposed before + // DiffEditorWidget model got reset") and its default handler rethrows it from + // a timeout, i.e. an uncaught error in the dev overlay on every diff→file tab + // switch or dialog close. Do the teardown first, correctly: detach the models + // (which drops the widget's onWillDispose listeners), then dispose them — + // nothing else in the app owns or tracks them, since no model path is passed + // and Monaco hands out an anonymous URI per model. The library then reads a + // null model and only disposes the widget. + // + // This assumes the subtree is being DESTROYED. Layout cleanup also fires when + // a subtree is merely hidden (``, a Suspense + // fallback); no host does that today, and if one ever wraps a diff that way + // the models must be re-created on re-show instead of disposed here. + useIsomorphicLayoutEffect( + () => () => { + if (initialDiffReadRef.current) { + clearTimeout(initialDiffReadRef.current) + initialDiffReadRef.current = null + } + const editor = diffEditorRef.current + diffEditorRef.current = null + const model = editor?.getModel() + editor?.setModel(null) + model?.original.dispose() + model?.modified.dispose() + }, + [] + ) + const navigateToChange = useCallback( (index: number) => { const editor = diffEditorRef.current @@ -113,6 +161,35 @@ export function DiffViewer({ } }, [currentChangeIndex, diffChanges.length, navigateToChange]) + // Stable identity: the library runs `updateOptions` on every change of this + // prop, so an inline literal would re-push the whole option set each render. + const diffOptions = useMemo( + () => ({ + readOnly: true, + renderSideBySide: true, + renderSideBySideInlineBreakpoint: 0, + automaticLayout: true, + fontSize: (editorFontSize * zoomLevel) / 100, + fontFamily: editorFontStack, + fontLigatures: editorLigatures, + wordWrap: editorWordWrap ? "on" : "off", + minimap: { enabled: false }, + unicodeHighlight: MONACO_UNICODE_HIGHLIGHT_OPTIONS, + scrollBeyondLastLine: false, + renderOverviewRuler: false, + ignoreTrimWhitespace: true, + renderIndicators: true, + originalEditable: false, + }), + [ + zoomLevel, + editorFontStack, + editorFontSize, + editorLigatures, + editorWordWrap, + ] + ) + const { additions, deletions } = useMemo(() => { let add = 0 let del = 0 @@ -192,23 +269,7 @@ export function DiffViewer({ Loading diff viewer... } - options={{ - readOnly: true, - renderSideBySide: true, - renderSideBySideInlineBreakpoint: 0, - automaticLayout: true, - fontSize: (editorFontSize * zoomLevel) / 100, - fontFamily: editorFontStack, - fontLigatures: editorLigatures, - wordWrap: editorWordWrap ? "on" : "off", - minimap: { enabled: false }, - unicodeHighlight: MONACO_UNICODE_HIGHLIGHT_OPTIONS, - scrollBeyondLastLine: false, - renderOverviewRuler: false, - ignoreTrimWhitespace: true, - renderIndicators: true, - originalEditable: false, - }} + options={diffOptions} /> diff --git a/src/components/files/file-workspace-tab-bar.tsx b/src/components/files/file-workspace-tab-bar.tsx index ef145ac44..b29a2c5ba 100644 --- a/src/components/files/file-workspace-tab-bar.tsx +++ b/src/components/files/file-workspace-tab-bar.tsx @@ -66,63 +66,87 @@ export function FileWorkspaceTabBar() { const activeFileIndex = fileTabs.findIndex( (tab) => tab.id === activeFileTabId ) + const lastTabActive = + activeFileIndex >= 0 && activeFileIndex === fileTabs.length - 1 if (fileTabs.length === 0) return null return ( -
- + {fileTabs.map((tab, index) => ( + + ))} + {/* Trailing area: a drag spacer fills the leftover row (window-drag region) + and, in fusion, a maximize/restore button sits flush right (it used to + live in the file detail header). They are the group's own trailing + children but NOT Reorder.Items, so dragging a tab only ever permutes the + tabs. Wrapped in one `flex-1` box so the workspace-bg bottom hairline + (ws-strip-line) runs unbroken under both. NO `min-w-0`: the wrapper's + min-content (the spacer's `min-w-10` + the shrink-0 button) is its floor, + so under many-tab overflow the tabs shrink to reserve them instead of the + wrapper collapsing to 0. `relative` + `data-adjacent-active` anchor the + inset baseline (globals.css `.ws-strip-line::after`) used when the LAST + tab is active: its transparent reverse-corner foot flares 0.5rem over + this wrapper, and a full-width border-bottom would show through under + that foot. */} +
- {fileTabs.map((tab, index) => ( - - ))} - - {/* Trailing area: a drag spacer fills the leftover panel width (window-drag - region) and, in fusion, a maximize/restore button sits flush right (it - used to live in the file detail header). Wrapped in one `flex-1` box so - the workspace-bg bottom hairline (ws-strip-line) runs unbroken under - both. NO `min-w-0`: the wrapper's min-content (the spacer's `min-w-10` + - the shrink-0 button) is its floor, so under many-tab overflow the group - shrinks to reserve them instead of the wrapper collapsing to 0. */} -
{/* Drag spacer, floored at `min-w-10` (40px): even when many tabs overflow and squeeze this region, a grabbable window-drag gap always remains between the last tab and the maximize button. */} @@ -150,7 +174,7 @@ export function FileWorkspaceTabBar() { )}
-
+ ) } diff --git a/src/components/layout/aux-panel-git-log-tab.tsx b/src/components/layout/aux-panel-git-log-tab.tsx index 2883fc941..d291bd7f1 100644 --- a/src/components/layout/aux-panel-git-log-tab.tsx +++ b/src/components/layout/aux-panel-git-log-tab.tsx @@ -84,6 +84,7 @@ import { } from "@/components/ui/command" import { Skeleton } from "@/components/ui/skeleton" import { AuxPanelNoFolderEmpty } from "@/components/layout/aux-panel-no-folder-empty" +import { GitLogCommitMessage } from "@/components/layout/git-log-commit-message" import { subscribe } from "@/lib/platform" import { useActiveFolder } from "@/contexts/active-folder-context" import { useWorkspaceActions } from "@/contexts/workspace-context" @@ -2296,16 +2297,12 @@ export function GitLogTab() {
-
-

- {entry.message} -

- -
+ {/* Long release messages are capped with a + Show more / Show less toggle so the file + tree below stays reachable. */} + {/* File changes load lazily on expand (the list query runs with withFiles=false). CommitFilesTree renders its own "Files" diff --git a/src/components/layout/git-log-commit-message.test.tsx b/src/components/layout/git-log-commit-message.test.tsx new file mode 100644 index 000000000..0db1b77bb --- /dev/null +++ b/src/components/layout/git-log-commit-message.test.tsx @@ -0,0 +1,111 @@ +import { type ReactElement } from "react" +import { fireEvent, render, screen } from "@testing-library/react" +import { NextIntlClientProvider } from "next-intl" +import { afterEach, describe, expect, it } from "vitest" + +import { GitLogCommitMessage } from "./git-log-commit-message" +import enMessages from "@/i18n/messages/en.json" + +function renderWithIntl(ui: ReactElement) { + return render( + + {ui} + + ) +} + +const MESSAGE = "feat: add a thing\n\nWith a body." + +// jsdom does no layout: scrollHeight/clientHeight both default to 0, which +// already reads as "not overflowing" for the short-message case. The overflow +// cases patch both onto Element.prototype *before* rendering so the +// synchronous mount-time measurement picks them up (the global ResizeObserver +// stub in test-setup.ts never invokes its callback). Mirrors +// collapsible-user-message.test.tsx. +function mockScrollMetrics(scrollHeight: number, clientHeight: number) { + const scrollHeightDescriptor = Object.getOwnPropertyDescriptor( + Element.prototype, + "scrollHeight" + ) + const clientHeightDescriptor = Object.getOwnPropertyDescriptor( + Element.prototype, + "clientHeight" + ) + Object.defineProperty(Element.prototype, "scrollHeight", { + configurable: true, + get: () => scrollHeight, + }) + Object.defineProperty(Element.prototype, "clientHeight", { + configurable: true, + get: () => clientHeight, + }) + return () => { + if (scrollHeightDescriptor) { + Object.defineProperty( + Element.prototype, + "scrollHeight", + scrollHeightDescriptor + ) + } + if (clientHeightDescriptor) { + Object.defineProperty( + Element.prototype, + "clientHeight", + clientHeightDescriptor + ) + } + } +} + +describe("GitLogCommitMessage", () => { + let restoreMetrics: (() => void) | null = null + + afterEach(() => { + restoreMetrics?.() + restoreMetrics = null + }) + + it("renders a short message clamped but with no toggle", () => { + renderWithIntl() + + const content = screen.getByTestId("git-log-commit-message-content") + expect(content).toHaveTextContent("feat: add a thing") + expect( + screen.queryByTestId("git-log-commit-message-toggle") + ).not.toBeInTheDocument() + expect(content).not.toHaveClass("collapsed-content-fade") + }) + + it("shows a Show more toggle when the message overflows the cap", () => { + restoreMetrics = mockScrollMetrics(900, 192) + + renderWithIntl() + + const content = screen.getByTestId("git-log-commit-message-content") + const toggle = screen.getByTestId("git-log-commit-message-toggle") + expect(toggle).toHaveAttribute("aria-expanded", "false") + expect(toggle).toHaveTextContent("Show more") + expect(toggle).toHaveAttribute("aria-controls", content.id) + expect(content).toHaveClass("max-h-48", "collapsed-content-fade") + }) + + it("expands to Show less on click and removes the clamp", () => { + restoreMetrics = mockScrollMetrics(900, 192) + + renderWithIntl() + + fireEvent.click(screen.getByTestId("git-log-commit-message-toggle")) + + const toggle = screen.getByTestId("git-log-commit-message-toggle") + expect(toggle).toHaveAttribute("aria-expanded", "true") + expect(toggle).toHaveTextContent("Show less") + const content = screen.getByTestId("git-log-commit-message-content") + expect(content).not.toHaveClass("max-h-48") + expect(content).not.toHaveClass("collapsed-content-fade") + + // Clicking again re-collapses. + fireEvent.click(toggle) + expect(toggle).toHaveAttribute("aria-expanded", "false") + expect(toggle).toHaveTextContent("Show more") + }) +}) diff --git a/src/components/layout/git-log-commit-message.tsx b/src/components/layout/git-log-commit-message.tsx new file mode 100644 index 000000000..198a7280f --- /dev/null +++ b/src/components/layout/git-log-commit-message.tsx @@ -0,0 +1,68 @@ +"use client" + +import { memo } from "react" +import { useTranslations } from "next-intl" +import { ChevronDown, ChevronUp } from "lucide-react" + +import { CommitCopyButton } from "@/components/ai-elements/commit" +import { useCollapsibleOverflow } from "@/hooks/use-collapsible-overflow" +import { cn } from "@/lib/utils" + +/** + * An expanded commit's full message body. Release commits can run to dozens of + * lines, which would push the file tree and branch list far off the bottom of + * the (narrow) aux panel, so the body is capped and gets a "Show more"/"Show + * less" toggle once it's actually clipped — same treatment as a chat user + * message (see `CollapsibleUserMessage`), shared via `useCollapsibleOverflow`. + */ +export const GitLogCommitMessage = memo(function GitLogCommitMessage({ + message, +}: { + message: string +}) { + const t = useTranslations("Folder.gitLogTab") + const { contentRef, contentId, isOverflowing, expanded, toggle } = + useCollapsibleOverflow(message) + + const clipped = !expanded + + return ( +
+

+ {message} +

+ {isOverflowing && ( + + )} + +
+ ) +}) diff --git a/src/components/message/collapsible-user-message.test.tsx b/src/components/message/collapsible-user-message.test.tsx index b7266e935..497e75871 100644 --- a/src/components/message/collapsible-user-message.test.tsx +++ b/src/components/message/collapsible-user-message.test.tsx @@ -76,7 +76,7 @@ describe("CollapsibleUserMessage", () => { ).not.toBeInTheDocument() expect( screen.getByTestId("collapsible-user-message-content") - ).not.toHaveClass("collapsed-user-message-fade") + ).not.toHaveClass("collapsed-content-fade") }) it("shows a Show more toggle when content overflows the collapsed height", () => { @@ -89,7 +89,7 @@ describe("CollapsibleUserMessage", () => { expect(toggle).toHaveAttribute("aria-expanded", "false") expect(toggle).toHaveTextContent("Show more") expect(toggle).toHaveAttribute("aria-controls", content.id) - expect(content).toHaveClass("max-h-60", "collapsed-user-message-fade") + expect(content).toHaveClass("max-h-60", "collapsed-content-fade") }) it("expands to Show less on click and removes the clamp", () => { @@ -104,7 +104,7 @@ describe("CollapsibleUserMessage", () => { expect(toggle).toHaveTextContent("Show less") const content = screen.getByTestId("collapsible-user-message-content") expect(content).not.toHaveClass("max-h-60") - expect(content).not.toHaveClass("collapsed-user-message-fade") + expect(content).not.toHaveClass("collapsed-content-fade") // Clicking again re-collapses. fireEvent.click(toggle) diff --git a/src/components/message/collapsible-user-message.tsx b/src/components/message/collapsible-user-message.tsx index 28a73a380..f39603852 100644 --- a/src/components/message/collapsible-user-message.tsx +++ b/src/components/message/collapsible-user-message.tsx @@ -1,11 +1,12 @@ "use client" -import { memo, useEffect, useId, useRef, useState } from "react" +import { memo } from "react" import { useTranslations } from "next-intl" import { ChevronDown, ChevronUp } from "lucide-react" import { cn } from "@/lib/utils" import type { AdaptedContentPart } from "@/lib/adapters/ai-elements-adapter" +import { useCollapsibleOverflow } from "@/hooks/use-collapsible-overflow" import { ContentPartsRenderer } from "./content-parts-renderer" @@ -22,31 +23,8 @@ export const CollapsibleUserMessage = memo(function CollapsibleUserMessage({ parts: AdaptedContentPart[] }) { const t = useTranslations("Folder.chat.messageList") - const contentRef = useRef(null) - const [isOverflowing, setIsOverflowing] = useState(false) - const [expanded, setExpanded] = useState(false) - const contentId = useId() - - useEffect(() => { - // Nothing useful to redetect once expanded: the clamp class below is - // removed, so clientHeight === scrollHeight trivially and this would - // misreport `false`, dropping the "Show less" toggle. Freeze the last - // known value instead. - if (expanded) return - const el = contentRef.current - if (!el) return - const measure = () => { - // Both reads are on this same, currently `max-h-60`-clamped node: no - // numeric threshold duplicated from CSS. clientHeight is capped by the - // class below; scrollHeight always reports the untruncated height. - setIsOverflowing(el.scrollHeight > el.clientHeight + 1) - } - measure() // Synchronous initial read — doesn't depend on the - // ResizeObserver callback firing (the jsdom test stub never invokes it). - const observer = new ResizeObserver(measure) - observer.observe(el) - return () => observer.disconnect() - }, [parts, expanded]) + const { contentRef, contentId, isOverflowing, expanded, toggle } = + useCollapsibleOverflow(parts) const clipped = !expanded @@ -59,7 +37,7 @@ export const CollapsibleUserMessage = memo(function CollapsibleUserMessage({ className={cn( "min-w-0", clipped && "max-h-60 overflow-hidden", - clipped && isOverflowing && "collapsed-user-message-fade" + clipped && isOverflowing && "collapsed-content-fade" )} > @@ -68,7 +46,7 @@ export const CollapsibleUserMessage = memo(function CollapsibleUserMessage({