Repository navigation
feat(shape): normalize compose shapes; refuse unsupported constructs with 422 - #141
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
omarsy
force-pushed
the
feat/compose-shape
branch
2 times, most recently
from
October 5, 2026 16:04
503388f to
e12c34f
Compare
omarsy
force-pushed
the
feat/compose-shape
branch
from
October 9, 2026 10:35
e12c34f to
1365325
Compare
…with 422
compose-containment Feature 2. The legal-syntax constructs that raised
uncaught exceptions out of create_greffon_info (each a bare HTTP 500,
per the epic's failure table) either normalize now or refuse with a
structured error naming the service and key:
- top-level networks used by a service (dict .append) -> supported;
containers as a LIST, the shape the rebuild already iterates; compose's
implicit 'default' network is a tolerated NO-OP (the pipeline maps
every service onto the internal network), not a registration
- service networks in mapping form (unhashable 'dict') -> supported;
networks: [] (opt-out) is a no-op, not a refusal
- 3-part mounts 'vol:/path:ro' (unpack ValueError) -> supported AND the
mode preserved through the rebuild, so a read-only declaration stays
read-only (dropping it would deploy :ro mounts read-write); unknown
third tokens refused
- long-syntax volumes -> type volume supported (long-syntax mode ro/rw
honoured); bind/tmpfs refused with the structural reason; anonymous
volumes refused
- bare-int and long-syntax ports -> supported as {target,protocol};
port ranges refused with a 'not supported, declare each port' message
- missing services: key -> named refusal
- mapping-form service ports/volumes (legacy dead else branches with
unbound locals) -> refused as invalid compose; dead branches and the
now-unused _split_proto deleted
Mechanism: one normalization pass (compose_shape.py) at the top of
create_greffon_info — the same pass the Feature 6 preflight will run, so
a green preflight provably implies a start that parses. ComposeShapeError
renders as services.<name>.<key>: <message>; the start router maps it to
422 beside the L4 catches (ConfigRenderError convention), bound at import
time (route tests patch the whole repository module; an except on a Mock
attribute raises TypeError mid-handler).
Extraction equivalence (the catalog bar) verified adversarially across
all 30 catalog entries at origin/main: 30/30 greffon_info deep-equal and
30/30 rendered composes byte-identical, legacy vs new.
Independent adversarial review (DO NOT SHIP) found the default-network
KeyError the vacuous test hid (the normalizer blessed what the consumer
crashed on), the networks:[] regression, and the dropped :ro mode — all
fixed here with end-to-end tests; plus message/decorator/docstring
polish. Pre-existing ruff findings in touched files left for their own
fix (TRY002 in the fetch helper; F601 duplicated 'volumes' key).
Codex CLI review (bundled 0.162): P1 — long-syntax volumes express
access with read_only, not the short syntax's mode token; reading mode
alone rendered a declared read-only mount WRITABLE. read_only: true now
preserves :ro through the rebuild (asserted on the rendered mapping).
P2 — nested volume options (subpath/nocopy) cannot be expressed in the
rebuilt short syntax and were silently dropped, deploying a DIFFERENT
mount (a subpath declaration mounted the whole volume); now refused with
a named error instead.
omarsy
force-pushed
the
feat/compose-shape
branch
from
October 10, 2026 12:04
1365325 to
2418b37
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
compose-containment Feature 2 per the HLD: the seven legal-syntax constructs that raise uncaught exceptions out of
create_greffon_info(each a bare HTTP 500, per the epic's failure table) either normalize or refuse with a 422 naming the service and key.Mechanism — one normalization pass (
apps/utils/greffon/compose_shape.py) at the top ofcreate_greffon_info, not scattered try/except; the same pass the Feature 6 preflight will run, so a green preflight provably implies a start that parses.networksused by a serviceAttributeError: 'dict' has no attribute 'append'containersas a list (the shape the rebuild already iterates)networksmapping formTypeError: unhashable 'dict'vol:/path:roValueError: too many values to unpack{type,source,target}AttributeError: 'dict' has no attribute 'split'type: volumesupported; bind/tmpfs refused with the structural reasonAttributeError: 'int' has no attribute 'split'{'target','protocol'}services:KeyErrorelsebranches (unbound locals)_split_protodeletedComposeShapeErrorrenders asservices.<name>.<key>: <message>; the start router maps it to 422 beside the L4 catches (theConfigRenderErrorconvention), bound at import time — route tests patch the whole repository module and an except on a Mock attribute would raise mid-handler.Extraction equivalence (the catalog regression bar): identical
port_name/protocol/volume derivation for every short form the catalog uses, asserted parametrically; the full existing suite passes unchanged against the normalized path. Undeclared volume sources still pass verbatim — namespacing-everything is Feature 3.Tests: 35 new;
SplitProtoTestsmoved to the behavior's new home. Suite: 958 passed, 0 skipped. Pre-existing ruff findings in touched files noted in the commit (TRY002 bareExceptionin the fetch helper; F601 duplicated'volumes'key — the second silently wins today), left for their own fix.Stacks independently of #136 (compose.py) and #140 (limits).