diff --git a/CHANGELOG.md b/CHANGELOG.md index d78e270..fe95d9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Breaking Changes -* Child processes now scrub inherited release boot variables and release PATH entries when `RELEASE_ROOT` is set. Set `release_env: :inherit` to opt out. Explicit `Config.env` values apply after the scrub, with release entries still removed from an explicit PATH. +* Child processes now scrub inherited release boot variables and release PATH entries when `RELEASE_ROOT` is set. Set `release_env: :inherit` to opt out. Explicit `Config.env` values apply after the scrub, with release entries still removed from an explicit PATH. Custom runners must honor `{name, false}` entries in `opts[:env]` by removing those variables from the inherited environment on every implemented execution callback. * Prompt commands now pass `-` in argv and deliver prompt bytes through stdin by default. Custom runners must deliver `opts[:stdin]` and EOF, or callers must select `prompt_input: :argv`. ### Features diff --git a/lib/codex_wrapper/iex.ex b/lib/codex_wrapper/iex.ex index 9b181b8..017d0eb 100644 --- a/lib/codex_wrapper/iex.ex +++ b/lib/codex_wrapper/iex.ex @@ -45,7 +45,8 @@ defmodule CodexWrapper.IEx do Start a new conversation. Prints the response. Accepts all options from `CodexWrapper.exec/2` -- config options - (`:working_dir`, `:binary`, `:env`, `:timeout`, and `:verbose`, where only `false` is supported) + (`:working_dir`, `:binary`, `:env`, `:release_env`, `:timeout`, and `:verbose`, + where only `false` is supported) and exec options (`:model`, `:sandbox`, `:approval_policy`, etc.). """ def chat(prompt, opts \\ []) do @@ -210,7 +211,7 @@ defmodule CodexWrapper.IEx do IO.puts("\e[33m(#{turns} turn#{plural(turns)})\e[0m") end - @config_keys [:binary, :working_dir, :env, :timeout, :verbose] + @config_keys [:binary, :working_dir, :env, :release_env, :timeout, :verbose] defp split_opts(opts) do Enum.split_with(opts, fn {k, _v} -> k in @config_keys end) diff --git a/lib/codex_wrapper/runner.ex b/lib/codex_wrapper/runner.ex index c64e3e9..8bb1fe8 100644 --- a/lib/codex_wrapper/runner.ex +++ b/lib/codex_wrapper/runner.ex @@ -78,7 +78,9 @@ defmodule CodexWrapper.Runner do Execution options, as produced by `CodexWrapper.Config.cmd_opts/1`: * `:cd` -- working directory (string) - * `:env` -- list of `{name, value}` string tuples + * `:env` -- list of `{String.t(), String.t() | false}` tuples. A `false` + value removes an inherited variable. Custom runners must honor these + removals on every implemented execution callback. * `:stderr_to_stdout` -- merge stderr into stdout * `:stdin` -- complete prompt bytes, delivered before EOF. This key is absent when the command has no input. Custom runners must support it diff --git a/test/codex_wrapper/iex_test.exs b/test/codex_wrapper/iex_test.exs index e17bc40..15ec90c 100644 --- a/test/codex_wrapper/iex_test.exs +++ b/test/codex_wrapper/iex_test.exs @@ -1,6 +1,8 @@ defmodule CodexWrapper.IExTest do use ExUnit.Case, async: false + import ExUnit.CaptureIO + alias CodexWrapper.IEx, as: CIEx setup do @@ -69,4 +71,99 @@ defmodule CodexWrapper.IExTest do assert {:last, 0} in CIEx.__info__(:functions) end end + + describe "release_env" do + @describetag :tmp_dir + + setup %{tmp_dir: directory} do + parent = %{ + "RELEASE_ROOT" => "/release/iex-test", + "RELEASE_COOKIE" => "iex-cookie", + "ROOTDIR" => "/release/iex-test", + "BINDIR" => "/release/iex-test/erts-17/bin", + "PATH" => "/release/iex-test/bin:/release/iex-test/erts-17/bin:/usr/bin:/bin" + } + + previous = Map.new(parent, fn {key, _value} -> {key, System.get_env(key)} end) + + on_exit(fn -> + Enum.each(previous, fn + {key, nil} -> System.delete_env(key) + {key, value} -> System.put_env(key, value) + end) + end) + + System.put_env(parent) + + binary = Path.join(directory, "fake_codex") + report = Path.join(directory, "child-env") + + report_commands = + Enum.map_join(parent, "\n", fn {key, _value} -> + ~s(printf '%s\\n' "#{key}=${#{key}-}") + end) + + File.write!(binary, """ + #!/bin/sh + { + #{report_commands} + } > "$CXW_ENV_REPORT" + printf '%s\\n' '{"type":"thread.started","thread_id":"iex-release-probe"}' + printf '%s\\n' '{"type":"turn.completed","usage":{}}' + """) + + File.chmod!(binary, 0o755) + + opts = [binary: binary, timeout: 10_000, env: [{"CXW_ENV_REPORT", report}]] + %{opts: opts, report: report, parent: parent} + end + + for policy <- [:inherit, :scrub] do + @policy policy + + test "chat/2 retains #{@policy} for later say/2 calls", context do + capture_io(fn -> + opts = Keyword.put(context.opts, :release_env, @policy) + assert CIEx.chat("hello", opts) == :ok + assert_release_env(context, @policy) + + assert CIEx.say("next") == :ok + assert_release_env(context, @policy) + end) + end + + test "resume/2 retains #{@policy} for later say/2 calls", context do + capture_io(fn -> + opts = Keyword.put(context.opts, :release_env, @policy) + assert CIEx.resume("iex-release-probe", opts) == :ok + + for prompt <- ["hello", "next"] do + assert CIEx.say(prompt) == :ok + assert_release_env(context, @policy) + end + end) + end + end + end + + defp assert_release_env(context, policy) do + actual = + context.report + |> File.read!() + |> String.split("\n", trim: true) + |> Map.new(fn line -> + [key, value] = String.split(line, "=", parts: 2) + {key, value} + end) + + expected = + case policy do + :inherit -> context.parent + :scrub -> Map.new(context.parent, fn {key, _value} -> {key, ""} end) + end + + expected = if policy == :scrub, do: Map.put(expected, "PATH", "/usr/bin:/bin"), else: expected + assert actual == expected + File.rm!(context.report) + end end