Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Versions follow [SemVer](https://semver.org).

## [Unreleased]

- Startup validation of `OUTERLOOP_AUTHOR_OVERRIDES` (the tick and `outerloop start`) uses the
image sessions actually run with, the default image when `OUTERLOOP_IMAGE` is unset. Before, a
codex override on a deployment without `OUTERLOOP_IMAGE` failed validation and stopped the tick.

- `OUTERLOOP_AUTHOR_OVERRIDES` accepts a list of entries per target, so different
slots of one target can use different authors (each listed entry names its slots;
a slot may appear only once). The single-object form is unchanged.
Expand Down
30 changes: 22 additions & 8 deletions src/outerloop/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,21 @@ def _setting_of(key: str, values: Mapping[str, str], environ: Mapping[str, str])
return (environ[key] if key in environ else values.get(key, "")).strip()


def _tick_image(values: Mapping[str, str], environ: Mapping[str, str], mode: str) -> str:
"""The image the launched tick runs with, resolved the way its launch mode passes
settings on: the resident chain's deploy step lets a settings-file line win (even
empty, meaning no image); a local or login loop keeps the shell's value and fills
only what is missing from the file. Absent everywhere: the default image."""
from outerloop.tick import _default_image

key = "OUTERLOOP_IMAGE"
first, second = (values, environ) if mode == "slurm" else (environ, values)
for source in (first, second):
if key in source:
return source[key].strip()
return _default_image()


def missing_harness_binary(values: Mapping[str, str], environ: Mapping[str, str]) -> str:
"""Check all configured authors' host CLIs using the harness's lookup."""
from outerloop.author_overrides import override_entries
Expand Down Expand Up @@ -609,14 +624,6 @@ def start(args: argparse.Namespace) -> int:
values = env_file_values(
operator_env_file(ENV_FILE), START_KEYS + TICK_ENV_KEYS
) # one read for everything
from outerloop.author_overrides import validate_overrides

try:
validate_overrides(
{**values, **os.environ}, _setting_of("OUTERLOOP_IMAGE", values, os.environ)
)
except ValueError as exc:
raise StartError(str(exc)) from exc
problem = "" if args.dry_run else missing_harness_binary(values, os.environ)
try:
author_model_setting(
Expand All @@ -643,6 +650,13 @@ def start(args: argparse.Namespace) -> int:
sbatch_on_path=shutil.which("sbatch") is not None,
cwd=Path.cwd(),
)
from outerloop.author_overrides import validate_overrides

try:
# validate with the image the tick this start launches will actually run with
validate_overrides({**values, **os.environ}, _tick_image(values, os.environ, plan.mode))
except ValueError as exc:
raise StartError(str(exc)) from exc
except (StartError, ValueError, OSError) as e:
print(f"outerloop start: {e}", file=sys.stderr)
return 2
Expand Down
10 changes: 8 additions & 2 deletions src/outerloop/tick.py
Original file line number Diff line number Diff line change
Expand Up @@ -3440,6 +3440,12 @@ def _default_image() -> str:
return os.path.expanduser("~/outerloop-images/agent-py312.sif")


def startup_image() -> str:
"""The image sessions run with: OUTERLOOP_IMAGE, else the default. Startup
validation uses the same value, so it can never refuse what the tick would run."""
return os.environ.get("OUTERLOOP_IMAGE", _default_image())


def _service_spec_from_env(
root: Path, *, gpu_lanes: dict[str, GpuLane] | None = None
) -> tuple[Any, ServiceSpec | None]:
Expand All @@ -3452,7 +3458,7 @@ def _service_spec_from_env(
account = os.environ.get("OUTERLOOP_ACCOUNT", "")
partition = os.environ.get("OUTERLOOP_PARTITION", "")
qos = os.environ.get("OUTERLOOP_QOS", "")
image = os.environ.get("OUTERLOOP_IMAGE", _default_image())
image = startup_image()
home = os.environ.get("OUTERLOOP_HOME", "")
# Account and partition are optional on Slurm: empty ones leave the billing
# association and the partition to Slurm's defaults, as `start` already
Expand Down Expand Up @@ -3588,7 +3594,7 @@ def main() -> int:

try:
gpu_lanes = gpu_lanes_from_env()
validate_overrides(os.environ, os.environ.get("OUTERLOOP_IMAGE", ""))
validate_overrides(os.environ, startup_image())
except ValueError as exc:
parser.error(str(exc))
logging.basicConfig(level=logging.INFO, format="%(asctime)s %(message)s")
Expand Down
88 changes: 88 additions & 0 deletions tests/test_author_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,91 @@ def test_a_target_may_list_overrides_for_different_slots(monkeypatch):
def test_listed_overrides_are_validated(entries):
with pytest.raises(ValueError, match=r"^OUTERLOOP_AUTHOR_OVERRIDES:"):
parse_overrides(json.dumps({"owner/repo": entries}))


def test_startup_validation_uses_the_image_the_tick_runs(monkeypatch):
# A deployment that leaves OUTERLOOP_IMAGE unset runs codex sessions on the default
# image; startup validation of a codex override must use that same image, or the
# tick refuses to start at all.
import os

from outerloop import tick
from outerloop.author_overrides import validate_overrides

monkeypatch.delenv("OUTERLOOP_IMAGE", raising=False)
monkeypatch.setenv(
"OUTERLOOP_AUTHOR_OVERRIDES",
json.dumps(
{"owner/repo": {"backend": "codex", "model": "some-codex-model", "slots": ["agent-03"]}}
),
)
assert tick.startup_image() == tick._default_image()
with pytest.raises(ValueError, match="requires --image"):
validate_overrides(os.environ, "") # the old startup behaviour
try:
validate_overrides(os.environ, tick.startup_image())
except ValueError as exc:
assert "requires --image" not in str(exc)


def test_tick_startup_validates_with_startup_image():
# The tick's entry point must validate overrides with the image sessions run with.
import inspect

from outerloop import tick

src = inspect.getsource(tick.main)
assert "validate_overrides(os.environ, startup_image())" in src


@pytest.mark.parametrize(
("mode", "env_value", "file_value", "expected"),
[
("local", None, None, "DEFAULT"),
("local", "", None, ""),
("local", "/img.sif", None, "/img.sif"),
("local", "/inherited.sif", "", "/inherited.sif"), # a local loop keeps the shell's value
("local", None, "/file.sif", "/file.sif"),
("slurm", None, None, "DEFAULT"),
("slurm", "/inherited.sif", "", ""), # the deploy step lets the settings file win
("slurm", "/inherited.sif", "/file.sif", "/file.sif"),
("slurm", "/inherited.sif", None, "/inherited.sif"),
],
)
def test_start_validates_with_the_launched_tick_image(
monkeypatch, tmp_path, mode, env_value, file_value, expected
):
# outerloop start must validate overrides with exactly the image the tick it launches uses.
from outerloop import cli, tick

seen = []
monkeypatch.setattr(
"outerloop.author_overrides.validate_overrides", lambda env, image: seen.append(image)
)
monkeypatch.setattr(tick, "_default_image", lambda: "DEFAULT")
if env_value is None:
monkeypatch.delenv("OUTERLOOP_IMAGE", raising=False)
else:
monkeypatch.setenv("OUTERLOOP_IMAGE", env_value)
env_file = tmp_path / "settings.env"
env_file.write_text("" if file_value is None else f"OUTERLOOP_IMAGE={file_value}\n")
env_file.chmod(0o600)
monkeypatch.setattr(cli, "ENV_FILE", env_file)
monkeypatch.setenv("OUTERLOOP_ENV_FILE", str(env_file))
monkeypatch.delenv("OUTERLOOP_COMPUTE", raising=False)
monkeypatch.delenv("OUTERLOOP_TICK_HOST", raising=False)
real_which = cli.shutil.which
monkeypatch.setattr(
cli.shutil,
"which",
lambda name, *a, **k: (
("/usr/bin/sbatch" if mode == "slurm" else None)
if name == "sbatch"
else real_which(name, *a, **k)
),
)
import contextlib

with contextlib.suppress(SystemExit):
cli.main(["start", "--dry-run", "--root", str(tmp_path / "state")])
assert seen and seen[0] == expected
Loading