**One-line summary:** Phase 1 is structurally complete, behaviorally verified (all offline-testable paths green), and secure for a single-learner tech-validation harness. The two unverifiable exit criteria (live audio session + live latency measurement) are blocked on voice-service key provisioning, not on code defects — auto-generated tests in `tests/test_pending_keys.py` will exercise them when keys are present. Two risk-free cosmetic P0 fixes were applied (a misspelled constant `_DEBRIFF_` → `_DEBRIEF_` and a dead-code line in `debrief.py`); neither changed runtime behavior (verified by re-running the full suite).
No referenced file is missing. `server/asr/__init__.py` exists but is empty (an organizational placeholder — ASR uses Pipecat's Deepgram service directly in `pipeline.py`; no adapter needed for v0.1 since Deepgram is the only ASR). Acceptable.
### 1.2 Imports resolve (no dangling references)
Ran `python3 -c "import ..."` for every server/db module + the public API:
### 1.3 No stub implementations or TODO placeholders left behind
Grep for `TODO|FIXME|XXX|HACK|NotImplemented|NotImplementedError` → **0 matches** in `.py` files (no `NotImplementedError` stubs; no TODO/FIXME markers).
`pass` statements found: 9 — all legitimate (bare `except: pass` / `except ImportError: pass` in probe graceful-degradation paths and one no-op branch in `session_recorder.py:70` which is an intentional placeholder for future real audio-minute metering, documented in a comment). No empty-function-body stubs.
### 1.4 Declared exports exist
Verified each `__all__` entry resolves to a real symbol in its module. No dangling exports.
### 1.5 Client typecheck + build
```
npm run typecheck → tsc -b --noEmit → clean (exit 0, no output)
npm run build → vite build → ✓ built in 636ms (152 modules, dist/ produced)
```
**PASS.** (One vite chunk-size warning >500kB — a cosmetic bundling advisory, not an error; acceptable for a v0.1 single-page client.)
### 1.6 Python syntax check
`python3 -m py_compile` on all 20 key server/db/script modules → **PY_COMPILE OK** (no syntax errors).
> **Note on Pipecat LSP static-type noise:** `pipeline.py` / `__main__.py` / `e2e_smoke.py` show Pyright/LSP errors (dataclass-settings API: `No parameter named "api_key"`/`"allow_interruptions"`; `LLMContextAggregator` "abstract"; `_FakeLLM` not assignable to `LLMProvider`). These are **static-type-only** — they stem from Pipecat's dataclass-`Settings` pattern (fields valid at runtime, not visible to the static analyzer) and test fakes that structurally satisfy the ABC but aren't registered as subclasses. **Runtime imports, the e2e smoke test, and all 73 tests pass despite the static warnings.** This matches the documented EXECUTE state. Flagged as P2 (maintainability) — see Quality findings.
| No secrets in committed files | ✅ PASS | `grep` for hardcoded API keys/tokens in new files → none found; all use `${VAR}` expansion or empty defaults |
| .dockerignore excludes `.ciagent/.env*` | ✅ PASS | `.ciagent/` directory excluded; secrets never in build context |
| stage-snippet.sh bakes GITEA_TOKEN at runtime (G-101) | ✅ PASS | `sed -i "s\|\${GITEA_TOKEN}\|${GITEA_TOKEN}\|g"` substitutes the placeholder; token is NOT committed to repo, only baked into the snippet at staging time (stored in Proxmox snippet storage, not git) |
| docker-compose.yml uses env_file (not hardcoded secrets) | ✅ PASS | `env_file: /etc/praxis/server.env` (written by install-service.sh from lxc.environment); no secret values in compose file |
The 1 warning is a benign `DeprecationWarning: 'audioop' is deprecated` from Pipecat's `audio/utils.py` (third-party, Python 3.13 advisory — not actionable in v0.1).
**Coverage: 15/15 P1 REQ-IDs covered by code.** All have at least one offline test except where the requirement is inherently live-key-dependent (REQ-VOICE-03 live number, REQ-LLM-01/02 live call) — those are covered by auto-generated pending-key tests that activate when keys are provisioned.
### 2.5 Auto-generated tests for unverifiable items
`tests/test_pending_keys.py` (NEW — 9 tests, all skip cleanly without keys):
| Test | Verifies | Activates when |
|---|---|---|
| `test_r1_deepgram_first_partial_latency` | R1 probe runs live | DEEPGRAM_API_KEY |
| `test_r2_cartesia_first_audio_latency` | R2 probe runs live | CARTESIA_API_KEY |
Threat model context: v0.1 is a **single-learner tech-validation harness** (G-008), local SQLite, no auth (D-007), no PII beyond a hardcoded display name, no network exposure beyond the pilot host. STRIDE findings are dispositioned per the auto-policy (low=accept, medium=mitigate, high=escalate).
| No remaining "coreci" references in praxis scripts (except origin comments) | ✅ PASS (after P0 fix) | timing.sh was using `coreci_deploy_timing_*` metric names — **fixed to `praxis_deploy_timing_*`**; remaining "coreci" refs are: origin comments ("Adapted from coreci"), Gitea org name (`GITEA_ORG="coreci"` — the repo owner), D-026 secret path (`~/coreci/.ciagent/.env.secrets`) — all correct |
| Bats tests cover all scripts (10 files, not 9 — G-106) | ⚠️ NOTE | 10 .bats files exist (121 tests), but **3 PLAN-specified test files are missing**: `timing.bats` (TASK-09-07), `idempotency.bats` (TASK-09-08), `docker-build.bats` (TASK-09-10). Idempotency IS covered in lxc-deploy.bats (16 tests), timing is exercised via lxc-deploy.bats, and docker-build is verified manually here. Coverage is adequate but doesn't match the PLAN's file list. |
| Health-check timeout is 600s (G-104, not 300s or 180s) | ✅ PASS | `health-check.sh:29` — `timeout_s="${PRAXIS_HEALTH_TIMEOUT:-600}"`; praxis.service `TimeoutStartSec=600`; .env.example documents `PRAXIS_HEALTH_TIMEOUT=600` |
**Quality result: PASS with notes** (1 P0 fixed: timing.sh metric names; 1 note: missing 3 bats files but coverage is adequate via other files)
|---|---|---|---|---|
| **Spoofing** | No auth in v0.1 (D-007 — single hardcoded learner "Alex"). Anyone who can reach the Pipecat server's `/pipecat/webrtc` endpoint could start a session. | Low (pilot) | **Accept** | D-007 explicitly defers auth. Single-learner harness; the server binds `0.0.0.0:8789` but is intended for a single pilot host. CORS is `allow_origins=["*"]` (dev) — acceptable for v0.1, **flag for tightening before any multi-learner milestone** (P1). |
| **Tampering** | SQLite local file (`praxis.db`) — no integrity protection. A local user can `sqlite3 praxis.db` and edit session/outcome/cost rows. | Low (pilot) | **Accept** | D-007: local pilot, single-learner. Trust model assumes the pilot host is trusted. No tamper-evidence needed for tech-validation. Documented in `db/schema.sql` header. |
| **Repudiation** | Sessions are logged with auto-generated ids (`sess-<uuid>`) and timestamps; no signed audit trail. A learner could dispute "I never did that session." | N/A (pilot) | **Accept** | Single hardcoded learner, no auth → no multi-party repudiation surface. Sessions are for learner self-review, not compliance. |
| **Info Disclosure** | (a) `.ciagent/.env.secrets` is `0600` perms + gitignored — ✅ verified. (b) `.env`, `.env.secrets`, `.env.*` all in `.gitignore` — ✅ verified. (c) `git ls-files` confirms **no secret/key/db files tracked**. (d) Grep for hardcoded API keys (`sk-...`, `*_API_KEY="..."` assignments) → **0 matches** in non-example files. (e) `db/*.db` gitignored — no learner data leaked. | Low | **Accept** | Secrets handling is correct. The local `.ciagent/.env.secrets` contains a `DEEPGRAM_API_KEY` value (40 chars) but it is **not committed** (gitignored, 0600) — this is the intended dev-secret pattern. No info-disclosure vulnerability found. |
| **Denial of Service** | No rate limiting on the FastAPI/Pipecat server; no connection cap; a client can open many WebRTC sessions. `asyncio.create_task(runner.run(task))` fires-and-forgets per request. | Low-Medium (pilot) | **Accept (v0.1) / Flag (P1)** | D-007/D-012: single-learner pilot, no adversarial threat model. Acceptable for v0.1. **Flag for P1 post-hoc review**: before any multi-learner exposure, add connection limits + task lifecycle management (the current `create_task` without tracking could leak tasks on disconnect). |
| **Elevation of Privilege** | No auth → no privilege ladder → no escalation surface. | N/A | **Accept** | N/A for v0.1. |
### Injection-vector review (security persona)
| Vector | Status | Evidence |
|---|---|---|
| **YAML scenario loading** | ✅ Safe | `server/scenarios/loader.py` uses `yaml.safe_load` (not `yaml.load`) — no arbitrary Python object construction. Scenario files are repo-authored (D-007: no user-uploaded scenarios in v0.1). |
| **LLM prompt construction** | ✅ Contained | `classifier.py::_build_user_prompt` and `debrief.py::_render` interpolate learner text into the prompt via string replacement. A malicious learner ASR transcript could inject prompt text, but: (a) the LLM is role-playing a customer (no tool calls / no DB writes from LLM output), (b) the guardrail output filter runs on the response, (c) the branch classifier output is JSON-parsed leniently with fallback. Prompt injection impact is bounded to a misclassified branch or a weird debrief — not a security boundary for v0.1. **Accept.** |
| **Path traversal (scenario id)** | Low | `loader.load(scenario_id)` builds `base / f"{scenario_id}.yaml"` — a `scenario_id` containing `../` could escape `scenarios/`. In v0.1 the id comes from the env var `PRAXIS_SCENARIO` (operator-controlled), not user input. **Accept for v0.1; flag for P1** if scenario ids ever become user-selectable. |
**Layer 3 verdict: ACCEPT.** No high-severity STRIDE findings. 3 P1 flags for future hardening (CORS tightening, DoS/connection limits, path-traversal guard) — all appropriate for a post-pilot milestone, not v0.1 blockers.
| MH-02 | `docker compose config` parses without error | ✅ PASS (fixed) | Was failing due to invalid `restart_policy` key; fixed → exits 0 |
| MH-03 | `docker run --rm praxis:test python -c "import server, pipecat"` | ✅ PASS (fixed) | Prints `ok` after fastapi added to pyproject.toml |
| MH-04 | Image contains `client/dist/index.html` | ✅ PASS | Verified via `docker run --rm praxis:verify ls /app/client/dist/index.html` |
| MH-05 | `.dockerignore` excludes node_modules, .git, client/dist, .ciagent/.env* | ✅ PASS | All patterns present in .dockerignore |
| MH-06 | SQLite persists across `docker compose restart` via named volume | ✅ PASS (design) | `praxis-data` volume mounted at `/app/data`; `PRAXIS_DB_PATH=/app/data/praxis.db` set in compose + env; `db/store.py` + `db/migrate.py` read PRAXIS_DB_PATH (G-102 fix). Live restart test not run (no Docker daemon persistence in verify env), but the wiring is correct. |
| MH-18 | lxc-deploy.sh has NO proxy/PROXY_VMID/BACKEND_DOMAIN steps | ✅ PASS | 0 matches for PROXY_VMID/BACKEND_DOMAIN/backend-add/smoke-test |
| MH-19 | praxis.service: ExecStart=docker compose up + ExecStartPre=docker compose build + Restart=on-failure + TimeoutStartSec | ✅ PASS (fixed) | ExecStartPre=/usr/bin/docker compose build; ExecStart=/usr/bin/docker compose up; Restart=on-failure; TimeoutStartSec=600 (G-104). **Fixed**: User=root → User=praxis (MH-21 alignment). Unit is written inline via heredoc in install-service.sh (not a separate file, but functionally equivalent). |
| MH-20 | praxis.service has NO Docker-incompatible hardening | ✅ PASS | No ProtectSystem/PrivateDevices/RestrictNamespaces/NoNewPrivileges/MemoryDenyWriteExecute; comment documents the decision |
| MH-21 | install-service.sh creates praxis user in docker group + writes env file + installs unit | ✅ PASS (fixed) | useradd + usermod -aG docker; writes /etc/praxis/server.env (0640, root:praxis); installs systemd unit; **Fixed**: User=praxis in unit (was User=root) |
| MH-22 | config.json secrets.scopes has release/proxmox/voice with correct env vars | ✅ PASS (fixed) | All 3 scopes present; **Fixed**: removed PROXMOX_LXC_VMID from proxmox scope (D-037 — it's `auto`, not a secret) |
| MH-23 | lxc-deploy.sh sources ~/coreci/.ciagent/.env.secrets + praxis .ciagent/.env.secrets | ✅ PASS (fixed) | **Fixed**: added secret-sourcing block to lxc-deploy.sh (was only in e2e-deploy.sh wrapper). Sources both files with graceful warnings if absent; pve_env validates after. |
| MH-24 | .env.example documents all PROXMOX_* + deploy vars (no actual secrets) | ✅ PASS | Deployment section documents PROXMOX_API_URL/TOKEN/NODE/STORAGE/TEMPLATE_VOLID/LXC_VMID/TLS_SKIP_VERIFY/MEMORY_MB + PRAXIS_HEALTH_URL/PORT/TIMEOUT + PRAXIS_CLIENT_DIST; all commented out or empty; D-026 source-from-coreci documented |
| MH-25 | git check-ignore: .ciagent/.env.secrets matches; .env.example does not | ✅ PASS | Verified both |
| MH-26 | `make test-proxmox-scripts` passes — 10 bats files | ⚠️ PARTIAL | 121 bats tests pass via `bats scripts/proxmox/test/`, but **no Makefile exists** (TASK-09-11 not implemented). `make test-proxmox-scripts` target unavailable. Tests pass when run directly via bats. |
| MH-27 | e2e-deploy.bats passes against live Proxmox (or skips) | ✅ PASS | e2e-deploy.bats has `PRAXIS_E2E_LIVE=1` skip guard — skips by default (no live cluster in CI); 7 e2e tests present |
| MH-28 | E2E deploy completes in < 5 min | ⏭️ DEFERRED | Requires live Proxmox cluster + secrets; not runnable in verify env. Wiring (timing wrappers, 600s timeout) is correct. |
Both are risk-free cosmetic cleanups with no logic/behavior change. Verified by re-running the full suite (73 passed, 9 skipped, 0 failed) + e2e smoke after each fix.
**Must-have result: 25/28 PASS, 2 PARTIAL (MH-12 comment-only diffs, MH-26 no Makefile), 1 DEFERRED (MH-28 live E2E)**
| # | File:line | Issue | Fix | Verification |
|---|---|---|---|---|
| P0-1 | `server/guardrails/customer_service.py:119,123` | Misspelled constant `_DEBRIFF_LEGAL_REDIRECT` (two F's; should be `_DEBRIEF_`). Worked at runtime only because the method references the constant by the same misspelled name and Python resolves globals at call time — but the typo is a latent trap: any future refactor that renames one occurrence would silently break the debrief filter, causing legal-action recommendations to pass unfiltered (a safety regression). | Renamed both occurrences to `_DEBRIEF_LEGAL_REDIRECT`. | `test_debrief_guardrail_blocks_legal_action` passes; manual end-to-end check confirms legal-action text still replaced by the redirect. |
| P0-2 | `server/debrief.py:31` | Dead code: `rel = template_id.replace("/", ".") ...` computed but never used (the actual path resolution uses `template_id.split('/')[-1]`). Confusing for maintainers and flagged by linters. | Removed the dead line. | `test_debrief_*` (5 tests) pass; template loading verified. |
| Q-1 | P1 | Maintainability | `server/pipeline.py`, `server/__main__.py`, `scripts/e2e_smoke.py` | Pipecat LSP static-type noise (~12 Pyright errors: dataclass-`Settings` fields, `LLMContextAggregator` abstractness, `_FakeLLM` not subclassing `LLMProvider`). Runtime is fine; static analysis is noisy. | Add `# type: ignore[...]` annotations with reasons, or wrap Pipecat service construction in typed helper functions. Register test fakes via `LLMProvider.register` or duck-type with `Protocol`. Non-blocking. |
| Q-2 | P1 | Correctness | `server/latency.py:106-112` | `TextFrame` is treated as an LLM-first-token proxy, but `TextFrame` is generic — it can carry non-LLM text (e.g. the opening-line TTS input), which could misattribute the first-token timestamp. The `LLMFullResponseEndFrame` branch (L99) is a better proxy but also imperfect. | For v0.1 accept (latency is logged, not enforced); for Phase 2 use Pipecat's `LLMTokenUsageFrame` / metrics service for accurate TTFT. |
| Q-3 | P1 | Adversarial/Security | `server/scenarios/loader.py:34` | `load(scenario_id)` builds `base / f"{scenario_id}.yaml"` without sanitizing `../` — path traversal possible if `scenario_id` is ever user-controlled. Currently env-var-controlled (operator), so low risk. | Add a guard: reject `scenario_id` containing path separators or `..`, or resolve + verify the result stays within `base`. |
| Q-4 | P1 | Security/DoS | `server/__main__.py:96-98` | `asyncio.create_task(runner.run(task))` is fire-and-forget — no tracking of running tasks, no cap on concurrent sessions, no cancellation on client disconnect. Acceptable for single-learner pilot but would leak resources at scale. | Track tasks in a set; cancel on disconnect; cap concurrency. Defer to multi-learner milestone. |
| Q-5 | P1 | Security | `server/__main__.py:55` | CORS `allow_origins=["*"]` — dev setting. Acceptable for v0.1 single-origin pilot but must be tightened before any non-local exposure. | Make CORS origin env-configurable (`PRAXIS_CORS_ORIGINS`); default to the client dev origin. |
| Q-6 | P2 | Testing | `tests/test_e2e.py:16-37` | The 3 e2e test functions each call `asyncio.run(run_e2e(...))` independently — the full loop runs 3× per test session (wasteful, ~3× the DB writes). Also `test_e2e_debrief_non_empty` re-runs the whole loop just to assert `debrief_chars > 50`. | Refactor to a session-scoped fixture that runs `run_e2e` once and shares the result dict across the 3 assertions. Non-blocking. |
### Per-persona summary
**Correctness:** Logic is sound across the hot path. `classify_branch_sync_heuristic` correctly scores branches by signal-keyword overlap and tie-breaks to the first branch (deterministic). `derive_cost` arithmetic verified (`test_derive_cost_piper_zero_tts` confirms Piper $0 path). `LatencyRecord.e2e_asr_to_tts_ms` math correct (550ms in test). Branch classifier parser is lenient (handles code fences, malformed JSON, empty input) with safe fallbacks. **No correctness P0s.**
**Testing:** 73 tests are meaningful — they cover schema validation, adapter graceful degradation, guardrail block categories, cost math, store CRUD, recorder lifecycle, debrief generation/filter, latency math, and the full e2e loop with DB assertions. Coverage is broad; gaps are the live-key paths (now covered by `test_pending_keys.py` skips) and client-side (no React component tests — v0.1 relies on e2e smoke per `package.json` "test" script). The `_FakeLLM`/`_StubDebriefLLM` fakes structurally satisfy the `LLMProvider` contract. **No testing P0s.** One P2 (test redundancy, Q-6).
**Performance:** No O(n²) in the voice-loop hot path. `LatencyObserver.process_frame` is O(1) per frame (passes through + records a timestamp). `lru_cache` on registry getters avoids repeated adapter construction. `SessionRecorder.log_turn` is O(1) per turn. The classifier runs once at session end (D-P1-05 — offline from the latency path). **No performance P0s.** One observation: `LLMContextAggregator` + Pipecat's context object grow with conversation length (unbounded turn history) — acceptable for v0.1 short sessions; flag for Phase 2 if sessions exceed ~50 turns.
**Maintainability:** Interfaces (`TTSProvider`/`LLMProvider`/`Guardrail`) are clean ABCs with typed dataclasses (`TTSResult`, `LLMStreamChunk`, `GuardrailVerdict`, `GuardrailContext`). The registry centralizes env-based selection. Adapters are thin and consistently degrade gracefully on missing keys/models. Naming is clear. The one maintainability defect was the `_DEBRIFF` typo (fixed as P0-1). Pipecat static-type noise (Q-1) is the remaining friction. **No maintainability P0s after fixes.**
**Adversarial:** What if the LLM returns malicious content? → Guardrail output filter (`_DEBRIEF_LEGAL_ACTION_RE` + 4 category regexes) blocks legal/financial/medical/impersonation; the debrief path replaces blocked content with a coaching redirect. What if the YAML scenario is malformed? → Pydantic `ValidationError` raised at load (typed, tested). What if the classifier returns garbage? → `_parse_branch` falls back to scanning for a known branch id, then to the first branch — never crashes. What if a probe key is missing? → `KEY_MISSING` banner, exit 0. **No adversarial P0s.** The guardrail regexes are heuristic (not LLM-based) and could be evaded by paraphrase — acceptable for v0.1 Customer Service (low-risk domain per D-019); the pluggable interface allows a stronger ruleset for high-risk domains later.
| REQ-DEPLOY-01 | Multi-stage Dockerfile | ✅ COVERED | Dockerfile: node:22-slim → python:3.12-slim; client/dist built in Stage 1, served via StaticFiles in Stage 2 |
| G-002 | Branch is post-hoc classification, not runtime fork | ✅ | `server/scenarios/runtime.py:as_flow_spec` → `transitions: []` with comment "v0.1: no in-flight transitions (G-002)"; classifier runs at session end. |
| REQ-DEPLOY-13 | FastAPI StaticFiles mount | ✅ COVERED | server/__main__.py mounts client/dist at "/" after API routes; PRAXIS_CLIENT_DIST env override; graceful degradation if dist absent |
| REQ-DEPLOY-14 | .env.example with deployment vars | ✅ COVERED | Proxmox LXC deployment section with all PROXMOX_* + PRAXIS_HEALTH_* + PRAXIS_CLIENT_DIST; D-026 documented; no actual secrets |
- **Root cause:** `restart_policy` is only valid for `docker stack deploy` (Swarm), not `docker compose`. A duplicate `restart: on-failure` was already present on line 9.
- **Fix:** Removed the `restart_policy` block; changed `restart: on-failure` → `restart: unless-stopped` (per PLAN spec); changed `env_file` to `required: false` syntax so `docker compose config` validates without the file present (install-service.sh always creates it before `up` in production).
**Overall: PASSED (with documented gaps).** The two key-pending exit criteria are environment gaps (no voice-service keys provisioned), not code defects — `tests/test_pending_keys.py` will verify them automatically when keys are present. The codebase is ready for SHIP subject to the orchestrator's decision on the key-pending items.
- **Symptom:** `docker run praxis:verify` failed with `ModuleNotFoundError: No module named 'fastapi'`; server couldn't start.
- **Root cause:** `server/__main__.py` imports `fastapi` and `uvicorn`, but neither was declared in `pyproject.toml``[project.dependencies]`. They were installed in the dev environment (v0.1) but not declared — the Dockerfile exposed the gap because the image only installs `pip install .` deps.
- **Fix:** Added `"fastapi>=0.110"` and `"uvicorn>=0.30"` to `pyproject.toml` dependencies. Rebuilt image → server starts, `/health` and `/` both work.
- **Status:** ✅ FIXED
### P0-03: timing.sh still used `coreci_deploy_timing_*` metric names (REQ-DEPLOY-08, TASK-03-07)
- **Symptom:** timing.sh emitted `{"event":"deploy_timing",...}` and Prometheus metric `coreci_deploy_timing_seconds` — not the praxis-prefixed names required by TASK-03-07.
- **Root cause:** timing.sh was copied verbatim from coreci with a note saying "rename in a follow-up if desired" — but TASK-03-07 requires the rename as part of the deliverable.
- **Symptom:** The idempotency check `[ -x /usr/local/bin/praxis-deploy ] && systemctl is-active --quiet praxis` would NEVER short-circuit in production because praxis never creates `/usr/local/bin/praxis-deploy` (that's a coreci Go binary path). Every CT restart that triggers the post-start hook would re-run the full install (apt install docker, git clone, install-service).
- **Root cause:** The check was copied from coreci's firstboot-hook (which installs a binary to `/usr/local/bin/`) without adapting for praxis's docker-compose-based deployment.
- **Fix:** Changed check to `[ -d /opt/praxis/.git ] && systemctl is-active --quiet praxis` — verifies the repo is cloned AND the service is active.
- **Note:** The bats test for this passed before the fix because the mock `pct` returns exit 0 regardless of the actual command body — the test validates the hook's behavior given a successful idempotency probe, not the probe's actual logic. This is a test-design limitation (mocking `pct exec` at the process level can't validate the `sh -c` body).
- **Status:** ✅ FIXED
---
---
*End of Phase 1 verification report. VERIFY only — SHIP is the orchestrator's next step.*
## 8. P1+ Issues (Non-critical — flagged for post-hoc review)
- The PLAN specifies `scripts/proxmox/praxis.service` as a file, but the unit is written inline via heredoc in `install-service.sh` (line 74). Functionally equivalent (the unit content is identical), but doesn't match the PLAN's file structure. No fix applied — the inline approach works and avoids a path-resolution issue (install-service.sh would need to locate the service file relative to itself).
- **Recommendation:** Accept the inline approach; update PLAN if needed.
### P1-02: Missing 3 bats test files (MH-26, TASK-09-07/08/10)
-`timing.bats`, `idempotency.bats`, `docker-build.bats` are not present. However:
- Idempotency IS tested in `lxc-deploy.bats` (16 tests cover --recreate/--reconfigure/healthy-skip/no-flag-error)
- Timing is exercised via `lxc-deploy.bats` (timing_start/timing_end wrappers called)
- Docker-build is verified manually in this verification (MH-01/03/04 pass)
- **Recommendation:** Add the 3 missing bats files for explicit coverage in a follow-up; current coverage is adequate for ship.
### P1-03: Missing `Makefile` (MH-26, TASK-09-11)
- No `Makefile` with `test-proxmox-scripts` target. Tests run via `bats scripts/proxmox/test/` directly.
- **Recommendation:** Add a minimal Makefile in a follow-up.
### P1-04: Missing `e2e-smoke.sh` (TASK-10-02)
- The standalone smoke script isn't present, but `e2e-deploy.sh` covers the same checks (/health JSON, / HTML, keys field).
- **Recommendation:** Accept e2e-deploy.sh as the smoke verification; add e2e-smoke.sh if a manual post-deploy smoke tool is wanted.
### P1-05: lxc-config.sh defaults were inconsistent with .env.example + docker-compose.yml (FIXED)
- **Status:** ✅ FIXED — aligned all defaults with .env.example + docker-compose.yml + install-service.sh.
### P1-06: e2e-deploy.sh always passes `--insecure` to curl (line 80)
-`curl -sS --insecure ${PROXMOX_TLS_SKIP_VERIFY:+--insecure}` — the first `--insecure` is unconditional, so TLS verification is always skipped regardless of `PROXMOX_TLS_SKIP_VERIFY`.
- **Recommendation:** Remove the unconditional `--insecure`, keep only the conditional one.
### P1-07: MH-12 — lxc-start.sh and ct-exists.sh have comment-only diffs from coreci
- lxc-start.sh differs in header comment line 2 ("CoreCI"→"Praxis"); ct-exists.sh differs in comments + path reference (proxy/ → top-level). Functionally identical. The PLAN said "verbatim" but header-comment adaptation is reasonable.
- **Recommendation:** Accept as verbatim-equivalent.
3. timing.sh `coreci_*` metric names → renamed to `praxis_*`
4. firstboot-hook.sh idempotency check referencing non-existent binary → fixed to check `/opt/praxis/.git` + service active
### P1+ issues: 8 (1 fixed, 7 noted)
- P1-05 (lxc-config defaults) fixed; P1-01/02/03/04/06/07/08 noted for follow-up.
### Verdict: **APPROVE_WITH_NOTES**
Phase 1 is structurally complete and behaviorally sound after the 4 P0 fixes. All 121 bats tests pass, all 77 non-live pytest tests pass, the Docker image builds and serves both the API and client, secrets are properly excluded from git/image, and the G-101/G-102/G-103/G-104/G-105/G-106 grill fixes are all applied. The remaining P1 items are non-blocking (missing Makefile, missing 3 bats files with adequate alternative coverage, comment-only coreci diffs). The 2 deferred REQ-NFR-DEPLOY-03 (live first-boot timing) and MH-28 require a live Proxmox cluster and cannot be verified in this environment — the wiring is correct and ready for live E2E.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Praxis CI