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
26 changes: 26 additions & 0 deletions src/panopticon/container/cli/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,32 @@
from pathlib import Path
from typing import Any, ClassVar, Protocol, TextIO

from panopticon.core.models import MODEL_TIERS


def resolve_tier(tier: str, tiers: Mapping[str, str]) -> str:
"""Resolve an abstract model **tier** to a concrete model id, failing loud on an unresolved tier.

``tiers`` is the CLI adapter's tier→model map. Resolution has three cases (ADR 0014 §3a):

- ``tier`` is in ``tiers`` → return its concrete model id (the normal path).
- ``tier`` is a **reserved** tier name (:data:`~panopticon.core.models.MODEL_TIERS`) but absent
from ``tiers`` → raise. An unresolved tier historically leaked straight to ``--model`` (a stale
container running pre-resolution code did exactly this — the bug this guards); refuse it
loudly instead of launching the wrong model silently.
- anything else → a concrete model id set directly (or a tier already persisted as its resolved
name); pass it through unchanged so back-compat holds.
"""
if tier in tiers:
return tiers[tier]
if tier in MODEL_TIERS:
raise ValueError(
f"model tier {tier!r} is not mapped by this CLI adapter (known tiers: {sorted(tiers)}); "
"refusing to pass an unresolved tier through to --model. This usually means a stale "
"container image running pre-resolution code — rebuild with `make clean && make build`."
)
return tier


class _Client(Protocol):
"""The slice of the task-service client the bootstrap needs (kept structural so tests fake it)."""
Expand Down
7 changes: 4 additions & 3 deletions src/panopticon/container/cli/claude.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from pathlib import Path
from typing import Any, ClassVar, TextIO

from panopticon.container.cli.base import AgentCLI, _Client
from panopticon.container.cli.base import AgentCLI, _Client, resolve_tier
from panopticon.container.config import update_json_config
from panopticon.container.hooks import write_settings
from panopticon.container.skills import write_commands, write_operation_commands
Expand Down Expand Up @@ -132,9 +132,10 @@ def resolve_model(self, tier: str) -> str:
that tier becomes a provider's model name (``"primary"`` → ``"opus"``), keeping model
vocabulary out of ``core``/``workflows``. Unknown values pass through unchanged so a raw
model id set directly (or a tier already persisted as its resolved name) still reaches
``--model`` verbatim.
``--model`` verbatim, while an unmapped **reserved** tier fails loud instead of leaking
through unresolved (see :func:`~panopticon.container.cli.base.resolve_tier`).
"""
return _MODEL_TIERS.get(tier, tier)
return resolve_tier(tier, _MODEL_TIERS)

def read_hook_payload(self, stdin: TextIO) -> dict[str, Any]:
"""Tolerantly parse the hook's stdin JSON; empty/invalid input yields an empty payload."""
Expand Down
8 changes: 5 additions & 3 deletions src/panopticon/container/cli/codex.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from pathlib import Path
from typing import Any, ClassVar, TextIO

from panopticon.container.cli.base import AgentCLI, _Client
from panopticon.container.cli.base import AgentCLI, _Client, resolve_tier
from panopticon.container.config import update_toml_config
from panopticon.container.hooks import HOOK_COMMAND
from panopticon.container.skills import write_commands, write_operation_commands
Expand Down Expand Up @@ -183,9 +183,11 @@ def resolve_model(self, tier: str) -> str:

The only place the tier (e.g. ``"primary"``) becomes a provider model name, keeping model
vocabulary out of ``core``/``workflows``. Unknown values pass through unchanged so a raw model
id set directly still reaches ``--model`` verbatim.
id set directly still reaches ``--model`` verbatim, while an unmapped **reserved** tier fails
loud instead of leaking through unresolved (see
:func:`~panopticon.container.cli.base.resolve_tier`).
"""
return _MODEL_TIERS.get(tier, tier)
return resolve_tier(tier, _MODEL_TIERS)

def read_hook_payload(self, stdin: TextIO) -> dict[str, Any]:
"""Tolerantly parse the hook's stdin JSON; empty/invalid input yields an empty payload."""
Expand Down
9 changes: 9 additions & 0 deletions src/panopticon/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@
from enum import Enum
from typing import Any

#: The control plane's abstract model **tiers** (ADR 0014 §3a) — CLI-agnostic labels a workflow's
#: ``default_model`` / :attr:`Task.starting_model` may hold (currently just ``"primary"``, the
#: built-in default). These names are **reserved**: every ``AgentCLI`` adapter must map each tier to
#: a concrete model id, so a reserved tier that reaches ``--model`` unresolved is a bug (a stale
#: image running pre-resolution code did exactly this) and the adapters fail loud on it rather than
#: passing it through (see ``panopticon.container.cli.base.resolve_tier``). A value *not* in this set
#: is treated as a concrete model id and passes through unchanged.
MODEL_TIERS: frozenset[str] = frozenset({"primary"})


class Actor(str, Enum):
"""A party that can act on a task: the user or the agent.
Expand Down
54 changes: 42 additions & 12 deletions src/panopticon/sessionservice/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@

_log = logging.getLogger(__name__)

#: The Docker image label stamping the ``panopticon`` package version a base image was built from.
#: :meth:`ImageBuilder.build_base_if_missing` reads it back to detect a **stale** base (the baked
#: package lagging the installed one) and rebuild — not only an absent one. The label rides on the
#: ``docker build`` argv, so the image **name** stays ``panopticon-base-<cli>`` (the rest of the
#: system keys on that name); only the stamp changes.
VERSION_LABEL = "org.panopticon.version"
#: Go template that reads :data:`VERSION_LABEL` off an existing image via ``docker image inspect
#: --format``; yields ``""`` when the image or the label is absent.
_VERSION_INSPECT_FORMAT = '{{ index .Config.Labels "' + VERSION_LABEL + '" }}'


def image_tag(workflow: str, repo_id: str, agent_cli: str = DEFAULT_AGENT_CLI) -> str:
"""The composed image's tag for a (workflow, repo, CLI) triple (ADR 0005 naming + ADR 0014 §4).
Expand Down Expand Up @@ -72,7 +82,9 @@ def _build_base(self, base: str, cli: str, *, verbose: bool) -> None:
"""`docker build` the bundled base Dockerfile as ``base`` for ``cli`` (ADR 0014 §4).

The ``AGENT_CLI`` build arg selects which agent CLI the bundled Dockerfile installs, so the
same file yields a genuinely different image per CLI (``panopticon-base-<cli>``)."""
same file yields a genuinely different image per CLI (``panopticon-base-<cli>``). The build is
stamped with the package version both as a ``--build-arg`` and the :data:`VERSION_LABEL` label
so :meth:`build_base_if_missing` can later detect a stale (older-package) image."""
import panopticon
import panopticon.docker as _docker_pkg

Expand All @@ -84,6 +96,8 @@ def _build_base(self, base: str, cli: str, *, verbose: bool) -> None:
"build",
"--tag",
base,
"--label",
f"{VERSION_LABEL}={panopticon.__version__}",
"--build-arg",
f"PANOPTICON_VERSION={panopticon.__version__}",
"--build-arg",
Expand All @@ -104,17 +118,33 @@ def build_base(self, *, agent_cli: str | None = None, verbose: bool = False) ->
self._build_base(base, agent_cli or DEFAULT_AGENT_CLI, verbose=verbose)

def build_base_if_missing(self, *, agent_cli: str | None = None, verbose: bool = False) -> bool:
"""Probe for the base image; build it from the bundled Dockerfile if absent.
"""Build the base image if it is **absent or stale**; a no-op when it is current.

``agent_cli`` selects the per-CLI base variant to probe + build (:func:`base_image`, ADR
0014 §4); ``None`` uses this builder's configured base (the claude default). Uses
``docker image inspect`` (fast, ~100 ms) to check presence. If the image is missing builds
it using the Dockerfile bundled with the installed package (``panopticon.docker``). Returns
``True`` if a build was triggered, ``False`` if the image was already present."""
0014 §4); ``None`` uses this builder's configured base (the claude default).

The baked ``panopticon`` package runs *inside* the container (installed from a wheel at build
time, not mounted), so a base image left in place silently reuses whatever package it was
built from. One ``docker image inspect --format`` (fast, ~100 ms) reads the
:data:`VERSION_LABEL` stamp: if it equals the installed ``panopticon.__version__`` the image
is current and we skip the build; otherwise (absent image → ``""``, unstamped image → ``""``,
or an older stamp) we rebuild from the Dockerfile bundled with the installed package
(``panopticon.docker``). Returns ``True`` if a build was triggered, ``False`` if the image was
already current."""
import panopticon

base = base_image(agent_cli) if agent_cli else self._base
result = self._run(["docker", "image", "inspect", base], check=False)
if result.strip() in ("", "[]"):
_log.warning("base image %r not found — building automatically", base)
self._build_base(base, agent_cli or DEFAULT_AGENT_CLI, verbose=verbose)
return True
return False
stamped = self._run(
["docker", "image", "inspect", "--format", _VERSION_INSPECT_FORMAT, base],
check=False,
).strip()
if stamped == panopticon.__version__:
return False
_log.warning(
"base image %r missing or stale (stamped %r, want %r) — building automatically",
base,
stamped or None,
panopticon.__version__,
)
self._build_base(base, agent_cli or DEFAULT_AGENT_CLI, verbose=verbose)
return True
9 changes: 9 additions & 0 deletions tests/container/test_claude.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,15 @@ def test_resolve_model_passes_unknown_values_through() -> None:
assert ClaudeAgentCLI().resolve_model("opus") == "opus"


def test_resolve_model_rejects_an_unmapped_reserved_tier(monkeypatch: pytest.MonkeyPatch) -> None:
# A reserved tier the adapter doesn't map must fail loud, not leak through to --model — the
# backstop for the stale-image bug (a container running pre-resolution code forwarded the raw
# tier). We simulate it by stripping the tier map.
monkeypatch.setattr("panopticon.container.cli.claude._MODEL_TIERS", {})
with pytest.raises(ValueError, match="primary"):
ClaudeAgentCLI().resolve_model("primary")


def test_built_in_workflow_tier_resolves_to_a_concrete_claude_model() -> None:
# End to end across the two halves: the tier the control plane declares (never a model name)
# resolves through the claude adapter to today's concrete model.
Expand Down
10 changes: 10 additions & 0 deletions tests/container/test_codex.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import tomllib
from pathlib import Path

import pytest

from panopticon.container.cli.codex import CodexAgentCLI


Expand Down Expand Up @@ -139,6 +141,14 @@ def test_resolve_model_passes_unknown_values_through() -> None:
assert CodexAgentCLI().resolve_model("gpt-5.6") == "gpt-5.6"


def test_resolve_model_rejects_an_unmapped_reserved_tier(monkeypatch: pytest.MonkeyPatch) -> None:
# A reserved tier the adapter doesn't map fails loud rather than leaking to --model (the
# stale-image backstop). Simulated by stripping the tier map.
monkeypatch.setattr("panopticon.container.cli.codex._MODEL_TIERS", {})
with pytest.raises(ValueError, match="primary"):
CodexAgentCLI().resolve_model("primary")


def test_built_in_workflow_tier_resolves_to_a_concrete_codex_model() -> None:
from panopticon.workflows.github_self_reviewed import GithubSelfReviewed

Expand Down
65 changes: 43 additions & 22 deletions tests/sessionservice/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,25 @@
from collections.abc import Sequence
from pathlib import Path

import panopticon
import panopticon.docker as _docker_pkg
from panopticon.sessionservice.images import ImageBuilder, compose_dockerfile, image_tag
from panopticon.sessionservice.images import (
VERSION_LABEL,
ImageBuilder,
compose_dockerfile,
image_tag,
)


def _assert_version_stamp(build_cmd: list[str]) -> None:
"""The base build stamps the package version both as a build-arg and the staleness label."""
assert "--build-arg" in build_cmd
assert (
build_cmd[build_cmd.index("--build-arg") + 1]
== f"PANOPTICON_VERSION={panopticon.__version__}"
)
assert "--label" in build_cmd
assert build_cmd[build_cmd.index("--label") + 1] == f"{VERSION_LABEL}={panopticon.__version__}"


def _bundled_dockerfile() -> str:
Expand Down Expand Up @@ -98,52 +115,59 @@ def __call__(self, args: Sequence[str], *, check: bool = True, verbose: bool = F
return self._responses.pop(0) if self._responses else ""


def test_build_base_if_missing_skips_build_when_image_present() -> None:
rec = _MultiRecorder('[{"Id": "sha256:abc"}]') # inspect returns JSON → image present
def test_build_base_if_missing_skips_build_when_version_label_matches() -> None:
# The inspect --format reads back the version stamp; matching the installed package → current,
# so the hot path is a single inspect with no rebuild.
rec = _MultiRecorder(f"{panopticon.__version__}\n")
result = ImageBuilder(base="panopticon-base", run=rec).build_base_if_missing()
assert result is False
assert len(rec.calls) == 1 # only the inspect probe, no build
assert rec.calls[0][0] == ["docker", "image", "inspect", "panopticon-base"]
inspect_cmd = rec.calls[0][0]
assert inspect_cmd[:3] == ["docker", "image", "inspect"]
assert "--format" in inspect_cmd
assert VERSION_LABEL in inspect_cmd[inspect_cmd.index("--format") + 1]
assert inspect_cmd[-1] == "panopticon-base"
assert rec.calls[0][1] is False # check=False so a missing image doesn't raise


def test_build_base_if_missing_builds_when_inspect_returns_empty_string() -> None:
rec = _MultiRecorder("") # inspect returns "" → image absent
def test_build_base_if_missing_rebuilds_when_image_absent() -> None:
rec = _MultiRecorder("") # inspect on a missing image → empty stamp
result = ImageBuilder(base="panopticon-base", run=rec).build_base_if_missing()
assert result is True
assert len(rec.calls) == 2
assert len(rec.calls) == 2 # inspect + build
build_cmd = rec.calls[1][0]
# command structure: docker build --tag <img> --build-arg PANOPTICON_VERSION=<v>
# command structure: docker build --tag <img> --label … --build-arg PANOPTICON_VERSION=<v>
# --build-arg AGENT_CLI=<cli> --file <path> <dir>
assert build_cmd[:4] == ["docker", "build", "--tag", "panopticon-base"]
assert "--build-arg" in build_cmd
version_arg = build_cmd[build_cmd.index("--build-arg") + 1]
assert version_arg.startswith("PANOPTICON_VERSION=")
_assert_version_stamp(build_cmd)
# No agent_cli given → the claude default selects the claude base variant's install (ADR 0014 §4).
assert "AGENT_CLI=claude" in build_cmd
assert "--file" in build_cmd
file_arg = build_cmd[build_cmd.index("--file") + 1]
assert file_arg.endswith("Dockerfile")
assert build_cmd[build_cmd.index("--file") + 1].endswith("Dockerfile")
assert Path(build_cmd[-1]).name == "docker" # context = parent dir of Dockerfile
assert rec.calls[1][1] is True # check=True so a build failure propagates


def test_build_base_if_missing_selects_the_codex_cli_install() -> None:
rec = _MultiRecorder("") # inspect returns "" → image absent
rec = _MultiRecorder("") # inspect returns "" (absent) → build
result = ImageBuilder(run=rec).build_base_if_missing(agent_cli="codex")
assert result is True
inspect_cmd, build_cmd = rec.calls[0][0], rec.calls[1][0]
assert inspect_cmd == ["docker", "image", "inspect", "panopticon-base-codex"]
assert inspect_cmd[:3] == ["docker", "image", "inspect"]
assert inspect_cmd[-1] == "panopticon-base-codex"
assert build_cmd[:4] == ["docker", "build", "--tag", "panopticon-base-codex"]
# The AGENT_CLI build arg drives which CLI the bundled Dockerfile installs (ADR 0014 §4).
assert "AGENT_CLI=codex" in build_cmd


def test_build_base_if_missing_builds_when_inspect_returns_empty_array() -> None:
rec = _MultiRecorder("[]") # docker inspect outputs "[]" on a missing image
def test_build_base_if_missing_rebuilds_when_version_label_stale() -> None:
# An older stamp (the reported bug: the baked package lagging the installed one) → rebuild.
rec = _MultiRecorder("0.0.1\n")
result = ImageBuilder(base="panopticon-base", run=rec).build_base_if_missing()
assert result is True
assert len(rec.calls) == 2 # inspect + build
assert rec.calls[1][0][:4] == ["docker", "build", "--tag", "panopticon-base"]
_assert_version_stamp(rec.calls[1][0])


def test_build_base_unconditional() -> None:
Expand All @@ -152,13 +176,10 @@ def test_build_base_unconditional() -> None:
assert len(rec.calls) == 1 # no inspect probe — just the build
build_cmd = rec.calls[0][0]
assert build_cmd[:4] == ["docker", "build", "--tag", "panopticon-base"]
assert "--build-arg" in build_cmd
version_arg = build_cmd[build_cmd.index("--build-arg") + 1]
assert version_arg.startswith("PANOPTICON_VERSION=")
_assert_version_stamp(build_cmd)
assert "AGENT_CLI=claude" in build_cmd # default CLI when none is given
assert "--file" in build_cmd
file_arg = build_cmd[build_cmd.index("--file") + 1]
assert file_arg.endswith("Dockerfile")
assert build_cmd[build_cmd.index("--file") + 1].endswith("Dockerfile")
assert Path(build_cmd[-1]).name == "docker" # context = parent dir of Dockerfile


Expand Down
Loading