You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Make service_templates find Dockerfiles in subdirectories of the template directory, so a
nested service template cannot ship outside every guard the declared service set feeds — while
keeping the two nested base-image Dockerfiles that exist today correctly outside that set.
Context you need
Where this came from. Codex review of PR #380 (fix/issue-361-declare-service-templates),
a P2 finding on src/cli/managers/base_image_preflight.py:96, raised against head 6a50effc
and reproduced there. Comment ID 3879446510. Verified and deferred by the 4AM review pass: it
is a gap in the guarantee, not a current fault, and closing it safely needs a change to how
exclusions are expressed (below), which is more than a review-response edit.
The gap.service_templates (src/cli/managers/base_image_preflight.py:86) is:
Path.glob("Dockerfile*") is not recursive, so a Dockerfile in a subdirectory is never a
member. Everything downstream inherits the blind spot: templates_missing_base_reference (:109), _refuse_uncoverable_templates (:521), and the
count assertions in tests/unit/test_base_image_preflight.py:1249-1268 and tests/unit/test_python_version_declaration.py. A nested service template on an unpinned
third-party base would therefore pass the whole suite — the exact "silently outside every
guard" failure #361 existed to end, one directory level down.
Meanwhile the packaging and deployment paths are recursive: package data ships templates/**/*, and TemplatesManager copies the tree into deployments. So a nested template
would really be deployed while being invisible to the preflight.
Measured at 6a50effc, fixture of a digest-pinned top-level Dockerfile-chat plus nested/Dockerfile-svc on FROM docker.io/library/python:3.11:
They define the a2rchi-python-base and a2rchi-pytorch-base images themselves — the same
role as the already-excluded top-level Dockerfile-base and Dockerfile-base-gpu. A plain
switch to rglob("Dockerfile*") pulls both in, and NON_SERVICE_TEMPLATES (:34) cannot
exclude them: it is keyed by filename, and both files are named exactly Dockerfile.
Adding "Dockerfile" as a key would be wrong twice — it excludes by a name that says nothing
about which file it means, and it would also exclude any future top-level file named Dockerfile.
So the exclusion keys must become relative paths (base-python-image/Dockerfile), which
also touches:
stale_template_exclusions (:98) — currently (directory / name).exists(). Works
unchanged with a relative path, but confirm it.
test_non_service_templates_matches_the_excluded_set (around tests/unit/test_base_image_preflight.py:1265) — it compares excluded names against NON_SERVICE_TEMPLATES.keys(). Read it before changing the keys.
The 19 / 15 / 4 counts asserted at tests/unit/test_base_image_preflight.py:1249-1268.
Recursion makes the total 21 and the exclusions 6; the service count must stay 15. Re-measure
rather than assuming; if the service count moves, stop and find out why.
TDD is mandatory. Failing test first, watched fail.
bash scripts/gate.sh must exit 0 before every commit. Never --no-verify.
Patch coverage vs the branch base at least 80%.
No Co-Authored-By trailers. closes #<this issue> in the PR body, not the title.
Do not edit any Dockerfile template, and do not move the nested base-image directories.
The layout is correct; only the traversal and the exclusion keys are wrong.
Do not merge. A human merges.
Plan
RED, the gap itself. In tests/unit/test_base_image_preflight.py, near test_templates_missing_base_reference_reports_replaced_line (:1280), add a test whose tmp_path fixture holds a digest-pinned top-level Dockerfile-chat (reuse _PINNED_FROM, :1273) and a nested nested/Dockerfile-svc carrying _THIRD_PARTY_FROM (:1277). Assert templates_missing_base_reference reports the nested template. Watch it fail — it returns [] today.
RED at the deploy entry point. Add the enforce_base_images counterpart asserting BaseImagePreflightError names the nested template. As in feat(#361): declare which Dockerfile templates are service templates #380, the entry-point assertion
is the one that protects the operator; the helper-level one alone is how the original defect
shipped.
RED, the exclusions must keep working by path. Add a test that the two real nested
base-image Dockerfiles are not in service_templates() — assert against the real
template directory, so it fails if a future traversal change pulls them in.
GREEN. Switch service_templates to directory.rglob("Dockerfile*") and compare each
path against the exclusion set by path relative to directory
(p.relative_to(directory).as_posix()), not by p.name. Re-key NON_SERVICE_TEMPLATES to
relative paths and add the two nested base-image Dockerfiles with their reasons, in the same
style as the existing four. Update stale_template_exclusions if the relative-path key
needs it, and keep its guarantee: a key naming a file that does not exist must fail the
suite.
Re-measure the counts and update the assertions to match reality.
The service count must remain 15. If it does not, stop — that is a finding, not an
assertion to adjust. Point the count prose at the declaration rather than restating a new
literal wherever the existing code already does so.
Check the whole downstream set, not just the tests you touched: grep -rn "service_templates\|NON_SERVICE_TEMPLATES\|stale_template_exclusions" src/ tests/ scripts/.
Every caller and every fixture. Report in the PR body what you found even if nothing needed
changing.
Commands
Re-derive the state before you start:
git fetch origin
gh pr view 380 --repo fasrc/archi --json state,mergedAt
sed -n '30,50p' src/cli/managers/base_image_preflight.py # NON_SERVICE_TEMPLATES
sed -n '86,120p' src/cli/managers/base_image_preflight.py # the three helpers
find src/cli/templates/dockerfiles -mindepth 2 -name 'Dockerfile*'
find src/cli/templates/dockerfiles -name 'Dockerfile*' -type f | wc -l
ls src/cli/templates/dockerfiles/Dockerfile*| wc -l
Reproduce the gap (writes only under a temp directory; never edit src/cli/templates/dockerfiles/). Pipe via stdin so the import resolves to this checkout
rather than an installed copy:
python - <<'EOF'import pathlib, sys, tempfilesys.path.insert(0, ".")from src.cli.managers import base_image_preflight as pfprint("module:", pf.__file__)d = pathlib.Path(tempfile.mkdtemp())(d / "Dockerfile-chat").write_text( "FROM ghcr.io/fasrc/a2rchi-python-base@sha256:" + "a"*64 + "\n")(d / "nested").mkdir()(d / "nested" / "Dockerfile-svc").write_text("FROM docker.io/library/python:3.11\n")print("service_templates:", [p.name for p in pf.service_templates(d)])print("missing:", [str(p) for p in pf.templates_missing_base_reference(d)])EOF
Before the fix this prints ['Dockerfile-chat'] and []. After it, service_templates must
include the nested template and missing must name it.
The reproduction script prints the nested template in both service_templates and missing.
A test asserts enforce_base_images refuses and names a nested service template on a
third-party base.
A test asserts src/cli/templates/dockerfiles/base-python-image/Dockerfile and base-pytorch-image/Dockerfile are not in service_templates() against the real
template directory.
Each new test failed before the implementation change; state the observed failure in the
commit message.
NON_SERVICE_TEMPLATES is keyed by relative path, and each of the 6 keys carries a reason
string, matching the existing style.
stale_template_exclusions() returns empty against the real template directory, and a
test still proves a bogus key fails.
test_templates_missing_base_reference_on_real_directory_is_empty still passes, and the
service-template count is still 15.
No file under src/cli/templates/dockerfiles/ is added, removed, moved, or edited: git diff --stat origin/dev -- src/cli/templates/dockerfiles/ prints nothing.
bash scripts/gate.sh exits 0, no --no-verify anywhere.
Patch coverage vs the branch base is at least 80%.
A PR is open against fasrc/archi:dev with closes #<this issue> in the body. Not
merged.
Start here
Run the re-derivation commands. Confirm the two nested base-image Dockerfiles exist and that
both are named exactly Dockerfile, which is why the filename-keyed exclusion list cannot
express them — that constraint, not the rglob call, is the actual work here. Then run the
reproduction script and watch the nested template go unseen.
Objective
Make
service_templatesfind Dockerfiles in subdirectories of the template directory, so anested service template cannot ship outside every guard the declared service set feeds — while
keeping the two nested base-image Dockerfiles that exist today correctly outside that set.
Context you need
Where this came from. Codex review of PR #380 (
fix/issue-361-declare-service-templates),a P2 finding on
src/cli/managers/base_image_preflight.py:96, raised against head6a50effcand reproduced there. Comment ID
3879446510. Verified and deferred by the 4AM review pass: itis a gap in the guarantee, not a current fault, and closing it safely needs a change to how
exclusions are expressed (below), which is more than a review-response edit.
The gap.
service_templates(src/cli/managers/base_image_preflight.py:86) is:Path.glob("Dockerfile*")is not recursive, so a Dockerfile in a subdirectory is never amember. Everything downstream inherits the blind spot:
templates_missing_base_reference(:109),_refuse_uncoverable_templates(:521), and thecount assertions in
tests/unit/test_base_image_preflight.py:1249-1268andtests/unit/test_python_version_declaration.py. A nested service template on an unpinnedthird-party base would therefore pass the whole suite — the exact "silently outside every
guard" failure #361 existed to end, one directory level down.
Meanwhile the packaging and deployment paths are recursive: package data ships
templates/**/*, andTemplatesManagercopies the tree into deployments. So a nested templatewould really be deployed while being invisible to the preflight.
Measured at
6a50effc, fixture of a digest-pinned top-levelDockerfile-chatplusnested/Dockerfile-svconFROM docker.io/library/python:3.11:The complication that makes this more than a one-word change. Two nested Dockerfiles
already exist, and both must stay out of the service set:
They define the
a2rchi-python-baseanda2rchi-pytorch-baseimages themselves — the samerole as the already-excluded top-level
Dockerfile-baseandDockerfile-base-gpu. A plainswitch to
rglob("Dockerfile*")pulls both in, andNON_SERVICE_TEMPLATES(:34) cannotexclude them: it is keyed by filename, and both files are named exactly
Dockerfile.Adding
"Dockerfile"as a key would be wrong twice — it excludes by a name that says nothingabout which file it means, and it would also exclude any future top-level file named
Dockerfile.So the exclusion keys must become relative paths (
base-python-image/Dockerfile), whichalso touches:
stale_template_exclusions(:98) — currently(directory / name).exists(). Worksunchanged with a relative path, but confirm it.
test_non_service_templates_matches_the_excluded_set(aroundtests/unit/test_base_image_preflight.py:1265) — it compares excluded names againstNON_SERVICE_TEMPLATES.keys(). Read it before changing the keys.tests/unit/test_base_image_preflight.py:1249-1268.Recursion makes the total 21 and the exclusions 6; the service count must stay 15. Re-measure
rather than assuming; if the service count moves, stop and find out why.
Constraints
gh pr view 380 --repo fasrc/archi --json state,mergedAt.git fetch origin && git checkout -b fix/issue-<this>-nested-service-templates origin/dev.origin/fix/issue-361-declare-service-templates, and say so in the PR body.service_templatesandNON_SERVICE_TEMPLATESarrive with feat(#361): declare which Dockerfile templates are service templates #380 and do not exist ondevuntil it merges; branching fromdevwhile feat(#361): declare which Dockerfile templates are service templates #380 is open gives you a tree where thisissue cannot be implemented. Call the uncoverable-service-template refusal from enforce_base_images, the path archi create actually takes #381 hit exactly that deadlock.
templates_missing_base_referencewhile this changesservice_templates. They are separatefunctions, so either order works, but the second one merged will need to re-measure the
counts. Do not fold the two together.
fasrc/archi:dev—gh pr create --repo fasrc/archi --base dev. NOTupstream/dev(archi-physics/archi, 100+ commits diverged).bash scripts/gate.shmust exit 0 before every commit. Never--no-verify.Co-Authored-Bytrailers.closes #<this issue>in the PR body, not the title.The layout is correct; only the traversal and the exclusion keys are wrong.
Plan
RED, the gap itself. In
tests/unit/test_base_image_preflight.py, neartest_templates_missing_base_reference_reports_replaced_line(:1280), add a test whosetmp_pathfixture holds a digest-pinned top-levelDockerfile-chat(reuse_PINNED_FROM,:1273) and a nestednested/Dockerfile-svccarrying_THIRD_PARTY_FROM(:1277). Asserttemplates_missing_base_referencereports the nested template. Watch it fail — it returns[]today.RED at the deploy entry point. Add the
enforce_base_imagescounterpart assertingBaseImagePreflightErrornames the nested template. As in feat(#361): declare which Dockerfile templates are service templates #380, the entry-point assertionis the one that protects the operator; the helper-level one alone is how the original defect
shipped.
RED, the exclusions must keep working by path. Add a test that the two real nested
base-image Dockerfiles are not in
service_templates()— assert against the realtemplate directory, so it fails if a future traversal change pulls them in.
GREEN. Switch
service_templatestodirectory.rglob("Dockerfile*")and compare eachpath against the exclusion set by path relative to
directory(
p.relative_to(directory).as_posix()), not byp.name. Re-keyNON_SERVICE_TEMPLATEStorelative paths and add the two nested base-image Dockerfiles with their reasons, in the same
style as the existing four. Update
stale_template_exclusionsif the relative-path keyneeds it, and keep its guarantee: a key naming a file that does not exist must fail the
suite.
Re-measure the counts and update the assertions to match reality.
The service count must remain 15. If it does not, stop — that is a finding, not an
assertion to adjust. Point the count prose at the declaration rather than restating a new
literal wherever the existing code already does so.
Check the whole downstream set, not just the tests you touched:
grep -rn "service_templates\|NON_SERVICE_TEMPLATES\|stale_template_exclusions" src/ tests/ scripts/.Every caller and every fixture. Report in the PR body what you found even if nothing needed
changing.
Commands
Re-derive the state before you start:
Reproduce the gap (writes only under a temp directory; never edit
src/cli/templates/dockerfiles/). Pipe via stdin so the import resolves to this checkoutrather than an installed copy:
Before the fix this prints
['Dockerfile-chat']and[]. After it,service_templatesmustinclude the nested template and
missingmust name it.Test and gate:
Acceptance criteria
service_templatesandmissing.enforce_base_imagesrefuses and names a nested service template on athird-party base.
src/cli/templates/dockerfiles/base-python-image/Dockerfileandbase-pytorch-image/Dockerfileare not inservice_templates()against the realtemplate directory.
commit message.
NON_SERVICE_TEMPLATESis keyed by relative path, and each of the 6 keys carries a reasonstring, matching the existing style.
stale_template_exclusions()returns empty against the real template directory, and atest still proves a bogus key fails.
test_templates_missing_base_reference_on_real_directory_is_emptystill passes, and theservice-template count is still 15.
src/cli/templates/dockerfiles/is added, removed, moved, or edited:git diff --stat origin/dev -- src/cli/templates/dockerfiles/prints nothing.bash scripts/gate.shexits 0, no--no-verifyanywhere.fasrc/archi:devwithcloses #<this issue>in the body. Notmerged.
Start here
Run the re-derivation commands. Confirm the two nested base-image Dockerfiles exist and that
both are named exactly
Dockerfile, which is why the filename-keyed exclusion list cannotexpress them — that constraint, not the
rglobcall, is the actual work here. Then run thereproduction script and watch the nested template go unseen.