Skip to content

fix(image-ref): preserve digest and tag refs in PYTHON_EXECUTOR_DOCKER_IMAGE#36

Merged
Danelegend merged 1 commit into
onyx-dot-app:mainfrom
civichero:fix/normalize-image-ref-digest-tag
May 18, 2026
Merged

fix(image-ref): preserve digest and tag refs in PYTHON_EXECUTOR_DOCKER_IMAGE#36
Danelegend merged 1 commit into
onyx-dot-app:mainfrom
civichero:fix/normalize-image-ref-digest-tag

Conversation

@civichero
Copy link
Copy Markdown
Contributor

Fixes #35.

What

_ensure_docker_image_available (code-interpreter/app/main.py:44) and DockerExecutor.check_health (code-interpreter/app/services/executor_docker.py:88) both unconditionally appended :latest to whatever value PYTHON_EXECUTOR_DOCKER_IMAGE held:

image_with_tag = f"{self.image}:latest"

This is correct when the env var holds a bare repository like onyxdotapp/python-executor-sci. It breaks when the operator wants to pin the executor image to a specific digest for reproducibility:

PYTHON_EXECUTOR_DOCKER_IMAGE=onyxdotapp/python-executor-sci@sha256:462c2fb0…

The resulting repo@sha256:462c2fb0…:latest is malformed; image lookup fails, the FastAPI lifespan aborts, and the container goes into a restart loop.

Fix

Extracts a normalize_image_ref(ref) helper in a new module code-interpreter/app/image_ref.py. The helper:

  • Returns the reference unchanged if it contains @ (digest reference).
  • Returns the reference unchanged if it has a : after the rightmost / (tag reference).
  • Otherwise appends :latest (bare repository — preserves the previous default).

Care is taken with registry ports: registry.example.com:5000/owner/repo has a : before the last /, which is a port separator and must not be confused with a tag.

Both call sites in main.py and executor_docker.py are updated to use the helper.

Tests

New code-interpreter/tests/integration_tests/test_image_ref.py covers:

  • bare repo / namespaced repo / registry/repo → :latest appended
  • tagged references unchanged at every nesting level
  • digest references unchanged at every nesting level
  • registry-with-port variants: port + bare repo (gets tag), port + tag (unchanged), port + digest (unchanged)
  • idempotence: running the function twice on its own output is a no-op

All 10 cases verified locally.

Verification

ruff check and ruff format --check pass on the touched files. mypy is configured strictly upstream — all helpers carry full type annotations (str -> str); imports are organized so isort/ruff are happy.

No behavioral change for operators using the default bare-repo configuration: normalize_image_ref("onyxdotapp/python-executor-sci") still produces onyxdotapp/python-executor-sci:latest, byte-identical to the prior f"{self.image}:latest" output.

…R_IMAGE

`_ensure_docker_image_available` (app/main.py) and
`DockerExecutor.check_health` (app/services/executor_docker.py) both
unconditionally appended `:latest` to whatever value
`PYTHON_EXECUTOR_DOCKER_IMAGE` held. When an operator sets that env var
to a digest reference (e.g. `repo@sha256:abc...`) for reproducible
deploys, the resulting `repo@sha256:abc...:latest` is invalid Docker
syntax: image lookup fails, the FastAPI lifespan aborts, and the
container crashloops.

Refactor both call sites to use a new `normalize_image_ref` helper.
The helper returns the reference unchanged when it already carries a
tag (`:` after the last `/`) or a digest (`@`), and only appends
`:latest` for bare repositories. Registry ports are handled correctly
because a `:` before the last `/` is a port separator, not a tag.

Coverage added in tests/integration_tests/test_image_ref.py:

  * bare repo / namespaced repo / registry/repo all gain :latest
  * tagged refs unchanged at every nesting level
  * digest refs unchanged at every nesting level
  * registry-with-port variants (port + bare, port + tag, port + digest)
  * idempotence

Refs onyx-dot-app#35
Copy link
Copy Markdown
Contributor

@Danelegend Danelegend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@Danelegend Danelegend merged commit f4deba2 into onyx-dot-app:main May 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PYTHON_EXECUTOR_DOCKER_IMAGE: :latest suffix is appended even when value is already a digest reference, breaks SHA pinning

2 participants