Build Entry Point Conventions
Context
build.py is the single orchestration entry point. It handles env validation, host probing with fallback, playbook resolution, and subprocess execution. Shell wrappers (run.sh, cleanup.sh) exist for convenience but MUST delegate to build.py. Bypassing it loses host auto-detection, env validation, and quote-safe parsing — all of which caused real production issues.
Rules
- ALL shell scripts (
run.sh, cleanup.sh) MUST delegate to build.py. NEVER call ansible-playbook directly from a shell script. Previous bug: run.sh bypassed build.py, so host probing and state file fallback didn't run. After cable swaps the script silently connected to the wrong host or failed.
- Functions NEVER call
sys.exit(). Return None, an error code, or raise an exception. Let main() handle all process exits. Previous bug: find_ansible_playbook() called sys.exit(1) — impossible to test and breaks function composition.
.env parsing MUST strip surrounding quotes from values. Users write FOO="bar" and FOO='bar' interchangeably. Previous bug: quoted values passed literal "192.168.1.100" to SSH, which silently failed.
- Every public function in
build.py MUST have a corresponding test class in tests/test_build.py. Every error path (missing file, unreachable host, missing binary) MUST have a test.
- Optional env variables (e.g.,
WAN_MAC) are handled in Ansible role defaults via lookup('env', ...) | default('', true). NEVER add optional variables to REQUIRED_ENV in build.py.
- When adding a new CLI argument to
build.py, ALWAYS add a corresponding test in TestBuildCommand verifying the flag appears in the command list.
build.py MUST probe the Proxmox host before running Ansible. If unreachable, it MUST try cached IPs from .state/addresses.json. If all fail, exit with a clear error — NEVER pass an unreachable host to ansible-playbook.
Patterns
Shell scripts delegate to build.py
# BAD — bypasses host probing, env validation, quote handling
source .venv/bin/activate
set -a; source .env; set +a
ansible-playbook playbooks/site.yml "$@"
# GOOD — all logic lives in build.py
source .venv/bin/activate
set -a; source .env; set +a
python3 build.py "$@"
Functions return errors, never exit
# BAD — untestable, kills the process
def find_ansible_playbook() -> str:
...
print("ERROR: not found", file=sys.stderr)
sys.exit(1)
# GOOD — caller decides what to do
def find_ansible_playbook() -> str | None:
...
return None
Env parsing handles quotes
# BAD — FOO="bar" stores literal quotes
env[key.strip()] = value.strip()
# GOOD — strips matched surrounding quotes
value = value.strip()
if len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"):
value = value[1:-1]
env[key.strip()] = value
Test coverage requirements
Every test class maps to a function:
| Function |
Test Class |
Minimum coverage |
load_env |
TestLoadEnv |
Basic, comments, equals-in-value, whitespace, empty, quoted values |
validate_env |
TestValidateEnv |
All present, missing one, all missing, empty value |
resolve_playbook |
TestResolvePlaybook |
Absolute path, name, name without ext, nonexistent |
build_command |
TestBuildCommand |
Each flag individually, combined flags |
find_ansible_playbook |
TestFindAnsiblePlaybook |
Venv found, system fallback, returns None |
resolve_proxmox_host |
TestResolveProxmoxHost |
Primary reachable, fallback, all unreachable, corrupt state |
main |
TestMain |
Each error path + happy path with subprocess mock |
1---2name: build-conventions3description: Python code conventions for build.py and supporting scripts. Use when modifying build.py, run.sh, cleanup.sh, test_build.py, adding CLI arguments, changing env variable handling, or adding new entry points.4---56# Build Entry Point Conventions78## Context910`build.py` is the single orchestration entry point. It handles env validation, host probing with fallback, playbook resolution, and subprocess execution. Shell wrappers (`run.sh`, `cleanup.sh`) exist for convenience but MUST delegate to `build.py`. Bypassing it loses host auto-detection, env validation, and quote-safe parsing — all of which caused real production issues.1112## Rules13141. ALL shell scripts (`run.sh`, `cleanup.sh`) MUST delegate to `build.py`. NEVER call `ansible-playbook` directly from a shell script. Previous bug: `run.sh` bypassed `build.py`, so host probing and state file fallback didn't run. After cable swaps the script silently connected to the wrong host or failed.152. Functions NEVER call `sys.exit()`. Return `None`, an error code, or raise an exception. Let `main()` handle all process exits. Previous bug: `find_ansible_playbook()` called `sys.exit(1)` — impossible to test and breaks function composition.163. `.env` parsing MUST strip surrounding quotes from values. Users write `FOO="bar"` and `FOO='bar'` interchangeably. Previous bug: quoted values passed literal `"192.168.1.100"` to SSH, which silently failed.174. Every public function in `build.py` MUST have a corresponding test class in `tests/test_build.py`. Every error path (missing file, unreachable host, missing binary) MUST have a test.185. Optional env variables (e.g., `WAN_MAC`) are handled in Ansible role defaults via `lookup('env', ...) | default('', true)`. NEVER add optional variables to `REQUIRED_ENV` in `build.py`.196. When adding a new CLI argument to `build.py`, ALWAYS add a corresponding test in `TestBuildCommand` verifying the flag appears in the command list.207. `build.py` MUST probe the Proxmox host before running Ansible. If unreachable, it MUST try cached IPs from `.state/addresses.json`. If all fail, exit with a clear error — NEVER pass an unreachable host to `ansible-playbook`.2122## Patterns2324### Shell scripts delegate to build.py2526```bash27# BAD — bypasses host probing, env validation, quote handling28source .venv/bin/activate29set -a; source .env; set +a30ansible-playbook playbooks/site.yml "$@"3132# GOOD — all logic lives in build.py33source .venv/bin/activate34set -a; source .env; set +a35python3 build.py "$@"36```3738### Functions return errors, never exit3940```python41# BAD — untestable, kills the process42def find_ansible_playbook() -> str:43 ...44 print("ERROR: not found", file=sys.stderr)45 sys.exit(1)4647# GOOD — caller decides what to do48def find_ansible_playbook() -> str | None:49 ...50 return None51```5253### Env parsing handles quotes5455```python56# BAD — FOO="bar" stores literal quotes57env[key.strip()] = value.strip()5859# GOOD — strips matched surrounding quotes60value = value.strip()61if len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"):62 value = value[1:-1]63env[key.strip()] = value64```6566## Test coverage requirements6768Every test class maps to a function:6970| Function | Test Class | Minimum coverage |71|---|---|---|72| `load_env` | `TestLoadEnv` | Basic, comments, equals-in-value, whitespace, empty, quoted values |73| `validate_env` | `TestValidateEnv` | All present, missing one, all missing, empty value |74| `resolve_playbook` | `TestResolvePlaybook` | Absolute path, name, name without ext, nonexistent |75| `build_command` | `TestBuildCommand` | Each flag individually, combined flags |76| `find_ansible_playbook` | `TestFindAnsiblePlaybook` | Venv found, system fallback, returns None |77| `resolve_proxmox_host` | `TestResolveProxmoxHost` | Primary reachable, fallback, all unreachable, corrupt state |78| `main` | `TestMain` | Each error path + happy path with subprocess mock |