Skip to content

Commit 47cfebe

Browse files
lesnik512lesnik512
authored
docs: audit docs, comments, and the validate gate (#52)
Sweep of architecture/, README.md, and in-code comments against actual behavior. Every finding reproduced against the code. Headline: decisions/2026-07-10-reject-parse-dont-validate.md rests on the premise that validate() owns every shape emit reads, and that premise is false -- environment, env_file, and volumes are structural keys with no shape validator, so emit walks them raw and crashes (AttributeError, TypeError, and character-wise destructuring of a string volumes). Plus a raw ValueError on --artifact with no colon. Also: --add-host seeds hosts document-wide but layers extra_hosts closure-scoped and conflict-checks across the seam, so a service that never runs can veto a valid config. Doc drift: a dead _MAP_FLAGS citation, glossary/subset contradicting each other on KeySpec's arity, 19 supported keys missing from the supported list. Compaction: secrets/configs are 123 lines of near-verbatim twins; supported-subset.md can go 537 -> ~300 with nothing lost. Spawns four changes (two Full, two Lightweight).
1 parent 10bf175 commit 47cfebe

1 file changed

Lines changed: 171 additions & 0 deletions

File tree

‎planning/audits/2026-07-13-docs-and-gate-audit.md‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
# Docs and gate audit
2+
3+
A sweep of `architecture/`, `README.md`, and the in-code comments against what
4+
`compose2pod` actually does, hunting inconsistencies, redundancies, bugs, and
5+
compaction candidates. Every finding below was reproduced or verified against
6+
the code, not read off the prose. Findings spawn follow-up change files; they
7+
are not themselves changes.
8+
9+
Two accepted decisions govern the fixes and both survive intact:
10+
`decisions/2026-07-10-reject-parse-dont-validate.md` (the gate stays
11+
`validate(dict)`, no typed model) and
12+
`decisions/2026-07-12-reject-structural-key-registry.md` (no uniform structural
13+
registry; each structural key's behavior stays in its owning module).
14+
15+
## Guiding contract
16+
17+
The tool's stated promise is a **complete gate**: `validate()`
18+
(`compose2pod/parsing.py`) either accepts, warns, or raises
19+
`UnsupportedComposeError` — it never lets a malformed document reach `emit` and
20+
crash raw. `architecture/supported-subset.md` states it outright for
21+
`depends_on` ("raises `UnsupportedComposeError` at the gate instead of failing
22+
later with a raw `AttributeError`/`TypeError` when the shape is walked"), and
23+
`decisions/2026-07-10-reject-parse-dont-validate.md` rests its entire rationale
24+
on that promise already holding document-wide:
25+
26+
> **validate() owning every shape emit reads** (`changes/2026-07-10.01`) made
27+
> the shape-reading functions robust: a direct `emit_script(dict)` call on a
28+
> *malformed* document now fails with `UnsupportedComposeError`, not a raw
29+
> crash, and `validate()` exercises every shape.
30+
31+
**That premise is false today.** Findings A1-A3 are three structural keys where
32+
it does not hold. This is the audit's headline: an accepted decision is standing
33+
on an invariant the code does not have. Restoring the invariant (A1-A3) makes
34+
the decision true again — it does not reopen it.
35+
36+
## Bucket A — the gate is incomplete (bugs)
37+
38+
`environment`, `env_file`, and `volumes` are **structural keys**: they carry no
39+
`KeySpec`, so the `SERVICE_KEYS` validate loop never sees them, and unlike
40+
`tmpfs` / `entrypoint` / `healthcheck` they have no hand-written validator in
41+
`parsing.py` either. `emit` walks their raw shape and crashes. All three are
42+
reachable from the CLI with an ordinary compose file.
43+
44+
| # | Input | Today | Should be |
45+
|---|-------|-------|-----------|
46+
| A1 | `environment: "FOO=bar"` (string) | `AttributeError: 'str' object has no attribute 'items'` | `UnsupportedComposeError` |
47+
| A2 | `env_file: 5` | `TypeError: 'int' object is not iterable` | `UnsupportedComposeError` |
48+
| A3 | `volumes: "/data:/data"` (string) | iterates *characters*; reports `anonymous volume 'd' must be an absolute path` | `UnsupportedComposeError` |
49+
| A4 | `--artifact nocolon` | `ValueError: not enough values to unpack` out of `emit.py:209` | clean CLI error, exit 2 |
50+
51+
A3 has a silent-corruption face as well as a crash face: `volumes: "/"` is
52+
**accepted** and emits `-v "/"`, because the single character `/` happens to
53+
pass the anonymous-volume check. A string `volumes` is never rejected as the
54+
wrong shape; it is destructured one character at a time and whatever survives is
55+
emitted.
56+
57+
A1-A3 share one root cause (a structural key with no shape validator) and one
58+
fix shape: a `_validate_*` function in `parsing.py` mirroring the existing
59+
`_validate_tmpfs`. No registry needed — `decisions/2026-07-12` stands.
60+
61+
A4 is a different code path (CLI argument, not compose input) but the same
62+
user-visible contract: a raw traceback where a clean refusal was promised.
63+
`emit._emit_target` splits `--artifact` on `:` without checking the value has
64+
one, and `cli.main` only catches `UnsupportedComposeError`.
65+
66+
## Bucket B — inconsistent scoping (behavior)
67+
68+
**B1. `--add-host` mixes document-wide and closure-scoped sources, then
69+
conflict-checks across the seam.** `emit._plan` seeds hosts from
70+
`graph.hostnames(services)` — every service in the *document* — while
71+
`extra_hosts` is layered per service in `order`, the target's dependency
72+
closure. `pod._add_host_flags` then refuses any host landing on two addresses.
73+
So a service that **never runs** can veto a valid configuration:
74+
75+
```yaml
76+
services:
77+
app: {image: i, extra_hosts: ["db:1.2.3.4"]}
78+
other: {image: i, hostname: db} # not in app's closure; never started
79+
```
80+
→ `UnsupportedComposeError: service 'app': conflicting host 'db'
81+
('127.0.0.1' vs '1.2.3.4')`
82+
83+
Every other aggregate in the emit path — `dns`, `dns_search`, `dns_opt`,
84+
`sysctls`, secrets, configs — is closure-scoped. `--add-host` is the lone
85+
exception, and `supported-subset.md:200-202` documents the split honestly as
86+
"pre-existing, orthogonal behavior" rather than defending it.
87+
88+
**Resolution (ruled):** scope the emit-side host set to the closure. `hostnames`
89+
has exactly two callers — `parsing.py:135` (shape validation, stays
90+
document-wide, loses nothing) and `emit.py:233` (the add-host source). Only the
91+
latter changes. An `--add-host` entry for a service that never starts is a lie
92+
anyway: it points a name at `127.0.0.1`, where nothing is listening, turning an
93+
honest NXDOMAIN into a connection-refused.
94+
95+
## Bucket C — doc drift (all verified against code)
96+
97+
| # | Doc says | Code says |
98+
|---|----------|-----------|
99+
| C1 | `supported-subset.md:70`: `annotations` shares "the `_MAP_FLAGS` machinery" | `_MAP_FLAGS` does not exist anywhere in the repo. It is `_map()` (`keys.py`). |
100+
| C2 | `glossary.md:8`: a service-key spec is the `(validate, emit, merge)` **triple** | `supported-subset.md:35`: "each as a `(validate, emit)` **pair**". Two architecture files contradict each other. `KeySpec` has three fields. |
101+
| C3 | `supported-subset.md:31-39` lists the service-key registry as 15 keys | `SERVICE_KEYS` has **28**. All 13 resource keys (`mem_limit`, `cpus`, `pids_limit`, `oom_kill_disable`, …) are absent from the list, though the Resource limits section documents them correctly further down. |
102+
| C4 | `supported-subset.md:22-27` "Supported" service keys | Omits **19** supported keys: `configs`, `deploy`, `dns`, `dns_search`, `dns_opt`, `sysctls`, and all 13 resource keys. |
103+
| C5 | `supported-subset.md:13-14` top-level "Ignored (warns): `networks`" | `parsing.py:127` also warns for top-level `volumes`. |
104+
| C6 | `README.md:60-68` "supports an honest subset … `image`/`build`, `command`, `environment`/`env_file`, short-form bind `volumes`, `healthcheck`, `depends_on`, and network `aliases`" | Predates `extends`, secrets, configs, resource limits, and pod-level dns/sysctls — all shipped. Also says "bind `volumes`" when named and anonymous volumes are supported too. |
105+
106+
C1 and C3 are the drift signature of the doc's ~40 inline code citations: the
107+
prose names private identifiers that rename out from under it. Worth keeping the
108+
citations (they are good navigation) but at one-per-section, not one-per-bullet.
109+
110+
## Bucket D — redundancy and token economy
111+
112+
**D1. Secrets and Configs are near-verbatim twins.** 123 lines
113+
(`supported-subset.md:331-454`) for two things the doc itself says are the same:
114+
"mirroring secrets", "the same closure-scoped-creation rule secrets follow",
115+
"`uid`/`gid`/`mode` behave exactly as for secrets", "byte-for-byte the same
116+
teardown parity as secrets". They differ in exactly four ways — store name
117+
prefix, allowed sources, default target, absolute-target requirement — which is
118+
a four-row table. Collapsing to one **Stores** section costs no information.
119+
120+
**D2. Per-key prose restates the code.** ~30 bullets of the form "`X` is a list,
121+
emitted as repeated `--x`" (`cap_add`, `cap_drop`, `security_opt`, `devices`,
122+
`group_add`, `platform`, `user`, `working_dir`, …). This is a table.
123+
124+
**D3. Changelog voice in present-tense docs.** `architecture/README.md` defines
125+
these files as "the living truth about what the system does **now**". Five sites
126+
narrate history instead:
127+
128+
- `supported-subset.md:201` "pre-existing, orthogonal behavior"
129+
- `supported-subset.md:214` "pre-existing behavior, unchanged by this move"
130+
- `supported-subset.md:216` "same as before it was per-service"
131+
- `supported-subset.md:292` "previously a non-mapping healthcheck reached
132+
`.get()` calls downstream and crashed raw"
133+
- `pod.py:91-98` — an 8-line docstring that is mostly changelog ("as before this
134+
move", "relocating the flags changes nothing else observable about either
135+
source")
136+
137+
The *why* belongs in `changes/`; the diff already records the move.
138+
139+
Together D1-D3 take `supported-subset.md` from **537 to roughly 300 lines**
140+
with nothing lost.
141+
142+
**D4. `extends` duplicates the `keys` merge primitives.**
143+
`extends._as_list(key, name, value)` and `keys._as_list(name, key, value)` have
144+
identical bodies and identical error messages, with the **first two parameters
145+
swapped** — a live footgun, correct today only because each call site matches
146+
its own local signature. `extends._as_mapping` likewise re-implements
147+
`keys.pairs_to_mapping` plus a `depends_on` case. `extends.py:9-12` already
148+
flags the duplication and defers it to `decisions/2026-07-12`'s revisit trigger;
149+
the trigger is about a structural *registry*, which this is not — collapsing two
150+
copies of one helper onto the `keys.py` primitive needs no registry.
151+
152+
## Spawned changes
153+
154+
| Lane | Scope | Findings |
155+
|------|-------|----------|
156+
| Full | Close the structural-key gate: shape validators for `environment`, `env_file`, `volumes`; validate `--artifact SRC:DST` in the CLI. Failing test first for each. | A1, A2, A3, A4 |
157+
| Lightweight | Closure-scope the emit-side host set in `emit._plan`. | B1 |
158+
| Full | Rewrite `supported-subset.md` (537 → ~300): merge Stores, tabulate keys, fix drift, strip changelog voice. Fix `glossary.md`, `README.md`, `pod.py` docstring. | C1-C6, D1-D3 |
159+
| Lightweight | Collapse `extends._as_list` / `_as_mapping` onto the `keys.py` primitives. | D4 |
160+
161+
## Non-findings (checked, no action)
162+
163+
- `interval_seconds` correctly refuses `inf`/`nan` (via the guarded parse) and
164+
compound durations (`"1h30m"`); the `ms`-before-`m` suffix order is right.
165+
- `to_shell` / `variable_names` share one regex, so the emitted script and the
166+
CLI's variable note cannot disagree.
167+
- A healthcheck `test: []` is refused, but by `health_cmd` at emit rather than at
168+
the gate. User-visible behavior is still `UnsupportedComposeError`, so this is
169+
a purity nit, not a bug — left alone.
170+
- `deferred.md`'s "Unify the store render/vars seam" remains correctly deferred:
171+
no third reader has appeared and no drift bug has surfaced.

0 commit comments

Comments
 (0)