From e75d78b00c856eb6e4c8b598e693e052cb17e686 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 5 Mar 2026 14:58:20 +0000 Subject: [PATCH] some fixes --- codex-rs/core/src/tools/handlers/artifacts.rs | 83 ++++++++++++++++++- codex-rs/core/src/tools/runtimes/artifacts.rs | 56 +++++++++++++ .../skills/src/assets/samples/slides/SKILL.md | 3 + .../samples/slides/references/presentation.md | 3 +- .../src/assets/samples/spreadsheets/SKILL.md | 4 + .../spreadsheets/references/workbook.md | 2 + 6 files changed, 149 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/artifacts.rs b/codex-rs/core/src/tools/handlers/artifacts.rs index 5c6684f53a..f046021ee6 100644 --- a/codex-rs/core/src/tools/handlers/artifacts.rs +++ b/codex-rs/core/src/tools/handlers/artifacts.rs @@ -290,6 +290,7 @@ async fn prepare_artifact_build( let approval_key = ArtifactApprovalKey { command_prefix: artifact_prefix_rule(&command), cwd: turn.cwd.clone(), + staged_script: source_path.clone(), }; let escalation_approval_requirement = session .services @@ -299,9 +300,26 @@ async fn prepare_artifact_build( approval_policy: turn.approval_policy.value(), sandbox_policy: turn.sandbox_policy.get(), sandbox_permissions: SandboxPermissions::RequireEscalated, - prefix_rule: Some(approval_key.command_prefix.clone()), + prefix_rule: None, }) .await; + let escalation_approval_requirement = match escalation_approval_requirement { + crate::tools::sandboxing::ExecApprovalRequirement::Skip { bypass_sandbox, .. } => { + crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox, + proposed_execpolicy_amendment: None, + } + } + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { reason, .. } => { + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason, + proposed_execpolicy_amendment: None, + } + } + crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } => { + crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } + } + }; let env = build_artifact_env( &installed_runtime, @@ -506,9 +524,11 @@ fn format_artifact_stderr(output: &ExecToolCallOutput) -> String { #[cfg(test)] mod tests { use super::*; + use crate::codex::make_session_and_context; use crate::exec::StreamOutput; use codex_artifacts::RuntimeEntrypoints; use codex_artifacts::RuntimePathEntry; + use pretty_assertions::assert_eq; use tempfile::TempDir; #[test] @@ -764,4 +784,65 @@ mod tests { assert!(launcher_source.contains("globalThis.artifacts = artifactTool;")); assert!(launcher_source.contains("await import(pathToFileURL(sourcePath).href);")); } + + #[tokio::test] + async fn prepare_artifact_build_uses_script_specific_approval_key_without_execpolicy_rule() { + let (session, turn) = make_session_and_context().await; + let runtime = codex_artifacts::InstalledArtifactRuntime::new( + PathBuf::from("/runtime"), + PINNED_ARTIFACT_RUNTIME_VERSION.to_string(), + codex_artifacts::ArtifactRuntimePlatform::detect_current().expect("detect platform"), + codex_artifacts::ExtractedRuntimeManifest { + schema_version: 1, + runtime_version: PINNED_ARTIFACT_RUNTIME_VERSION.to_string(), + node: RuntimePathEntry { + relative_path: "node/bin/node".to_string(), + }, + entrypoints: RuntimeEntrypoints { + build_js: RuntimePathEntry { + relative_path: "artifact-tool/dist/artifact_tool.mjs".to_string(), + }, + render_cli: RuntimePathEntry { + relative_path: "granola-render/dist/render_cli.mjs".to_string(), + }, + }, + }, + PathBuf::from("/runtime/node/bin/node"), + PathBuf::from("/runtime/artifact-tool/dist/artifact_tool.mjs"), + PathBuf::from("/runtime/granola-render/dist/render_cli.mjs"), + ); + + let prepared = prepare_artifact_build( + &session, + &turn, + runtime, + "console.log('ok');".to_string(), + 5_000, + ) + .await + .expect("prepare artifact build"); + + assert_eq!( + prepared.request.approval_key.command_prefix, + vec![ + prepared.request.command[0].clone(), + turn.config + .codex_home + .join(ARTIFACT_BUILD_LAUNCHER_RELATIVE) + .display() + .to_string(), + ] + ); + assert_eq!( + prepared.request.approval_key.staged_script, + PathBuf::from(&prepared.request.command[2]) + ); + assert!( + prepared + .request + .escalation_approval_requirement + .proposed_execpolicy_amendment() + .is_none() + ); + } } diff --git a/codex-rs/core/src/tools/runtimes/artifacts.rs b/codex-rs/core/src/tools/runtimes/artifacts.rs index 0406ae91a7..c9306c1829 100644 --- a/codex-rs/core/src/tools/runtimes/artifacts.rs +++ b/codex-rs/core/src/tools/runtimes/artifacts.rs @@ -24,6 +24,7 @@ use std::path::PathBuf; pub(crate) struct ArtifactApprovalKey { pub(crate) command_prefix: Vec, pub(crate) cwd: PathBuf, + pub(crate) staged_script: PathBuf, } #[derive(Clone, Debug)] @@ -190,6 +191,7 @@ mod tests { "/path/to/launcher.mjs".to_string(), ], cwd: PathBuf::from("/tmp"), + staged_script: PathBuf::from("/tmp/source.mjs"), }, escalation_approval_requirement: ExecApprovalRequirement::Skip { bypass_sandbox: false, @@ -212,4 +214,58 @@ mod tests { assert_eq!(decision, ReviewDecision::Approved); } + + #[test] + fn approval_keys_differ_for_different_staged_scripts() { + let runtime = ArtifactRuntime; + let req_one = ArtifactExecRequest { + command: vec![ + "/path/to/node".to_string(), + "/path/to/launcher.mjs".to_string(), + "/tmp/source-one.mjs".to_string(), + ], + cwd: PathBuf::from("/tmp"), + timeout_ms: Some(5_000), + env: HashMap::new(), + approval_key: ArtifactApprovalKey { + command_prefix: vec![ + "/path/to/node".to_string(), + "/path/to/launcher.mjs".to_string(), + ], + cwd: PathBuf::from("/tmp"), + staged_script: PathBuf::from("/tmp/source-one.mjs"), + }, + escalation_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + }; + let req_two = ArtifactExecRequest { + command: vec![ + "/path/to/node".to_string(), + "/path/to/launcher.mjs".to_string(), + "/tmp/source-two.mjs".to_string(), + ], + cwd: PathBuf::from("/tmp"), + timeout_ms: Some(5_000), + env: HashMap::new(), + approval_key: ArtifactApprovalKey { + command_prefix: vec![ + "/path/to/node".to_string(), + "/path/to/launcher.mjs".to_string(), + ], + cwd: PathBuf::from("/tmp"), + staged_script: PathBuf::from("/tmp/source-two.mjs"), + }, + escalation_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + }; + + assert_ne!( + runtime.approval_keys(&req_one), + runtime.approval_keys(&req_two) + ); + } } diff --git a/codex-rs/skills/src/assets/samples/slides/SKILL.md b/codex-rs/skills/src/assets/samples/slides/SKILL.md index fb5614660b..e2dcace250 100644 --- a/codex-rs/skills/src/assets/samples/slides/SKILL.md +++ b/codex-rs/skills/src/assets/samples/slides/SKILL.md @@ -128,6 +128,9 @@ for (const slide of presentation.slides.items) { - Start with the smallest script that creates or imports the deck. - Do not begin by checking whether the local artifacts runtime package or cache exists. Assume the `artifacts` tool is ready and start authoring immediately; only investigate runtime installation or packaging if the tool fails before your slide code runs. +- If the first `artifacts` run fails before your slide code executes, treat it as runtime setup failure. +- Ask for approval to install Node or the required artifact runtime, explaining that a JavaScript runtime is required to build presentation artifacts. +- After approval, install it and retry once. Do not loop on the same failed script. - If the API surface is unclear, do a tiny probe first: create one slide, add one shape, set `text` or `textStyle`, export one PNG, and inspect the result before scaling up to the full deck. - Save the `.pptx` after meaningful milestones so the user can inspect output. - After saving a `.pptx`, verify the on-disk file type before assuming export succeeded. If it is actually an image blob, keep the PNG previews and rebuild a valid deck from them. diff --git a/codex-rs/skills/src/assets/samples/slides/references/presentation.md b/codex-rs/skills/src/assets/samples/slides/references/presentation.md index da07bcf3cc..3087e74eee 100644 --- a/codex-rs/skills/src/assets/samples/slides/references/presentation.md +++ b/codex-rs/skills/src/assets/samples/slides/references/presentation.md @@ -14,7 +14,8 @@ const presentation = Presentation.create({ - `await PresentationFile.importPptx(await FileBlob.load("deck.pptx"))` imports an existing deck. - `await PresentationFile.exportPptx(presentation)` exports the deck as a saveable blob. - Do not assume that saving the blob always yields a real PowerPoint container. On March 5, 2026 a fresh export path returned PNG bytes while keeping the `.pptx` extension. -- When using this skill operationally, start by authoring with these APIs rather than checking local runtime package directories first. Runtime or package-cache inspection is a fallback for cases where the `artifacts` tool itself fails before deck code executes. +- When using this skill operationally, start by authoring with these APIs rather than checking local runtime package directories first. +- If the first `artifacts` run fails before deck code executes, ask for approval to install Node or the required artifact runtime, then retry once. ## Slides diff --git a/codex-rs/skills/src/assets/samples/spreadsheets/SKILL.md b/codex-rs/skills/src/assets/samples/spreadsheets/SKILL.md index 23ac22e223..f0f9588e5a 100644 --- a/codex-rs/skills/src/assets/samples/spreadsheets/SKILL.md +++ b/codex-rs/skills/src/assets/samples/spreadsheets/SKILL.md @@ -62,6 +62,10 @@ console.log([ ## Workflow - Model the workbook structure first: sheets, headers, and key formulas. +- Do not begin by checking whether the local artifacts runtime package or cache exists. Assume the `artifacts` tool is ready and start authoring immediately; only investigate runtime installation or packaging if the tool fails before your workbook code runs. +- If the first `artifacts` run fails before your workbook code executes, treat it as runtime setup failure. +- Ask for approval to install Node or the required artifact runtime, explaining that a JavaScript runtime is required to build spreadsheet artifacts. +- After approval, install it and retry once. Do not loop on the same failed script. - Use formulas instead of copying computed values when the sheet should remain editable. - Recalculate before exporting or reading formula results. - End the script with a final `console.log(...)` summary that names every file the run touched, using a compact user-facing format with one path per line. diff --git a/codex-rs/skills/src/assets/samples/spreadsheets/references/workbook.md b/codex-rs/skills/src/assets/samples/spreadsheets/references/workbook.md index 527a29d879..c8ea36df07 100644 --- a/codex-rs/skills/src/assets/samples/spreadsheets/references/workbook.md +++ b/codex-rs/skills/src/assets/samples/spreadsheets/references/workbook.md @@ -13,6 +13,8 @@ const sheet = workbook.worksheets.add("Sheet1"); - `await SpreadsheetFile.importXlsx(await FileBlob.load("book.xlsx"))` imports an existing workbook. - `workbook.recalculate()` evaluates formulas. - `await SpreadsheetFile.exportXlsx(workbook)` exports a saveable `.xlsx` blob. +- When using this skill operationally, start by authoring with these APIs rather than checking local runtime package directories first. +- If the first `artifacts` run fails before workbook code executes, ask for approval to install Node or the required artifact runtime, then retry once. ## Worksheets