Skip to content

Refuse a deployment whose service templates pin the same base image at different references #389

Description

@swinney

Objective

Make the deploy preflight refuse — before archi create --force tears anything down — when
two service templates name the same a2rchi-*-base image at different references, instead of
probing whichever one sorts first and reporting the deployment ready.

Context you need

base_reference in src/cli/managers/base_image_preflight.py (on dev at 7c9915d0,
lines 123-135) returns the first reference whose text names the requested image and stops:

directory = template_dir or TEMPLATE_DIR
for dockerfile in sorted(directory.glob("Dockerfile-*")):
    for match in _FROM_BASE_RE.finditer(dockerfile.read_text()):
        reference = match.group("ref")
        if image in reference:
            return reference
return None

required_base_images calls it once per required image name, so exactly one reference per base
reaches enforce_base_images, no matter how many templates declare one. Nothing anywhere
checks that the templates agree.

Measured on db85e01b (PR #388 head), with two top-level templates pinning the python base at
different digests:

templates_missing_base_reference: []
required_base_images: ['ghcr.io/fasrc/a2rchi-python-base@sha256:aaaa…aaaa']

The bbbb…bbbb digest that Dockerfile-piazza builds from is never probed. The preflight
returns AVAILABLE, create --force runs remove_existing_deployment(), and the piazza build
then fails on an image nobody checked — the exact post-teardown failure this module exists to
prevent (see the module docstring, and #266 / #287).

Why the risk is currently latent, and why that is not enough. In-tree,
test_service_templates_pin_one_explicit_base_tag
(tests/unit/test_python_version_declaration.py:389) asserts len(builds) == 1 over every
# base-image-pin: annotation, so a split pin reddens CI. Two gaps remain:

  1. The guard keys on the annotation, not on the reference. Two templates carrying the same
    # base-image-pin: dev-abc1234 line above different digests satisfy it.
  2. It is a repo test. It does not run against the templates installed in an operator's
    environment, which is where enforce_base_images actually decides.

Why it was deferred. Found by an adversarial review pass on PR #388
(#388) during the 2026-08-29 nightly review round 2. The
defect predates that PR — it is in base_reference on dev, not in #388's recursive-traversal
change — so fixing it there would have expanded a scoped review diff. #388 closed only the part
it caused: base_reference now reads the same recursive file set service_templates declares.

Constraints

  • Branch from origin/dev. Open the PR with
    gh pr create --repo fasrc/archi --base dev. Never commit to dev directly.
  • origin is fasrc/archi. Do not target upstream (archi-physics/archi).
  • No Co-Authored-By and no session trailers on commits in this repo.
  • TDD: write the failing test first, watch it fail, then the minimum code, then the gate.
  • The gate must pass before every commit: bash scripts/gate.sh (black 24.10.0 + isort 6.0.1,
    then pytest tests/unit/ with diff-cover patch coverage --fail-under=80 vs origin/dev).
    Never bypass it with --no-verify.
  • Do not touch deploy/fasrc-dev/**.
  • Keep the refusal in the preflight module. src/interfaces/chat_app/app.py is not imported by
    unit tests, so logic added there cannot make patch coverage.

Plan

One PR — the change is behavior only, with no mechanical churn to separate.

  1. Add a failing test: a template directory with two templates naming the python base at
    different digests, asserting required_base_images raises BaseImagePreflightError and that
    the message names both templates and both references. Watch it fail.
  2. Add a failing test that enforce_base_images raises on the same fixture before any probe
    work — assert probe.pulled == [], the pattern used by
    test_enforce_base_images_refuses_an_uncoverable_service_template.
  3. Add a failing test that the agreeing case is unchanged: every template on one reference
    returns exactly the references it returns today, for both the CPU and the GPU/grader
    selections.
  4. Implement. Suggested shape: a base_references(image, template_dir) returning all
    distinct references for that image across service_templates(), and a
    _refuse_divergent_base_references(template_dir) called from the same two entry points that
    already call _refuse_uncoverable_templates (required_base_images and
    enforce_base_images), so the two entry points cannot disagree. Keep base_reference's
    signature — other callers and several tests depend on it.
  5. Decide and document one point explicitly in the docstring: whether a base not required by
    this deployment
    (pytorch on a CPU-only create) is still checked for agreement. Refusing on
    a base nothing will build is over-refusal; ignoring it hides a split pin until the next
    GPU deployment. Recommendation: check agreement only for the images
    required_base_image_names returns, and say so in the docstring.
  6. Update docs/docs/developer_guide.md, in the section "Which service templates the deploy
    preflight refuses", with the new refusal case. AGENTS.md:L53-L55 requires the docs change in
    the same PR as the user-facing behavior change.

Commands

git fetch origin && git checkout -b fix/issue-<N>-divergent-base-pins origin/dev

# reproduce the fail-open before you change anything
python - <<'PY'
import sys, pathlib, tempfile
sys.path.insert(0, ".")
from src.cli.managers import base_image_preflight as pf
d = pathlib.Path(tempfile.mkdtemp())
good = "ghcr.io/fasrc/a2rchi-python-base@sha256:" + "a"*64
stale = "ghcr.io/fasrc/a2rchi-python-base@sha256:" + "b"*64
(d/"Dockerfile-chat").write_text(f"FROM {good}\n")
(d/"Dockerfile-piazza").write_text(f"FROM {stale}\n")
print("missing:", pf.templates_missing_base_reference(d))
print("required:", pf.required_base_images(gpu_ids=None, grader_enabled=False, template_dir=d))
PY

python -m pytest tests/unit/test_base_image_preflight.py -q
python -m pytest tests/unit/test_python_version_declaration.py -q
bash scripts/gate.sh

Acceptance criteria

  • The reproduction above raises BaseImagePreflightError instead of printing a
    required: line, and the message names Dockerfile-chat, Dockerfile-piazza, and both
    digests.
  • enforce_base_images raises on the same fixture with probe.pulled == [] — the refusal
    lands before any image work, so no teardown can precede it.
  • A directory whose templates agree returns exactly the references it returns today, for
    both gpu_ids=None, grader_enabled=False and gpu_ids="all", grader_enabled=True.
  • python -m pytest tests/unit/test_base_image_preflight.py -q and
    tests/unit/test_python_version_declaration.py -q are green.
  • bash scripts/gate.sh exits 0 with patch coverage at or above 80% on the diff.
  • docs/docs/developer_guide.md names the new refusal case.
  • The PR targets fasrc/archi:dev and its body carries a Closes #<N> keyword — the
    keyword must be in the body, not the title, or the issue is not linked.

Start here

Run the reproduction block above on a fresh origin/dev checkout and confirm it prints the
aaaa… reference while the bbbb… one is never mentioned. That output is the defect. Then
write the step-1 test and watch it fail before writing any implementation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Priority: this cycleai-wipClaimed by an automated run; in flightauto-okOpt-in: the nightly run may pick this issue up and open a PRbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions