cc-ci-orchestrator/cc-ci-plan/plan-phase1b-review-lint.md

# cc-ci Phase 1b — Review & lint pass (Autonomous Build Plan)

**Status:** QUEUED — a **bounded** pass that runs after Phase 1 **and Phase 1c**
(`plan-phase1c-full-reproducibility.md`), and **before** Phase 2 (`plan-phase2-recipe-tests.md`). It
runs *after* 1c on purpose: the review/lint + full D1–D10 re-verification then covers the final,
refactored state (the `cc-ci-secrets` split, cert-in-sops, the genuine D8 live rebuild).
**Transition:** **manual** (operator kicks it off).
**Builds on:** the complete post-1c codebase (flake/modules, `runner/` + harness, the comment-bridge,
dashboard, scripts, the first recipes' tests, the `cc-ci-secrets` split, docs).
**Owner agents:** same Builder + Adversary loops (`plan.md` §6/§7). Here the Adversary also acts as
**white-box reviewer** (reads the code, not just black-box behavior).
**This file's path:** `/srv/cc-ci/cc-ci-plan/plan-phase1b-review-lint.md`

---

## 0. Why this phase (and why it's bounded)

Before scaling to ~18 recipes (Phase 2), clean and harden the foundation built in Phase 1 — it's far
cheaper to fix the harness/modules now than after every recipe copies their patterns. This is a
**one-time, bounded pass**, not an open-ended loop. Two distinct workstreams that go to two places,
to avoid bikeshedding:

1. **Style & hygiene → deterministic tooling** (formatters/linters in CI). No judgment calls, no
   churn, enforceable going forward.
2. **Code health a linter can't judge → a white-box review checklist with teeth** (§3) — anchored to
   plan-relevant invariants, *not* taste. Blocking items get fixed; the rest are advisory.

Discipline: **good-enough + enforceable**, not gold-plating. Don't reopen settled design; don't
weaken any test in the name of "cleanliness." When in doubt, lint it deterministically or leave it.

---

## 1. Definition of Done (Phase 1b exit condition)

Terminates when every item holds **and the Adversary/reviewer has independently confirmed** (logged
in `REVIEW.md`):

- [ ] **RL1 — Lint/format tooling in place + green.** Formatters + linters are added to the repo (a
      `lint` entrypoint + a Nix devshell) and wired as a **stage in cc-ci's own `.drone.yml`** so
      future commits stay clean. The whole Phase-1 codebase **passes** them. Tooling (defaults; settle
      in `DECISIONS.md`):
      - **Nix:** `alejandra` (or `nixpkgs-fmt`) format · `statix` (lints) · `deadnix` (dead code).
      - **Python** (`runner/`, harness, `bridge/`, `tests/`): `ruff` (lint + format).
      - **Shell:** `shellcheck` + `shfmt`.
      - **(optional) YAML:** `yamllint` for `.drone.yml`/compose.
- [ ] **RL2 — Review checklist run, blocking items fixed.** The white-box checklist (§3) has been run
      over the codebase; every **blocking** finding is fixed; every **advisory** finding is either
      fixed or moved to `BACKLOG.md`/`IDEAS.md` as `[idea]` with a one-line rationale. Findings +
      resolutions recorded in `REVIEW.md`.
- [ ] **RL3 — Full Phase-1 re-verification (the final gate).** This is the **last step before 1b is
      marked DONE**, performed *after* all lint/review findings (RL1+RL2) have been responded to and
      resolved. The Adversary **independently re-verifies every Phase-1 Definition-of-Done item
      D1–D10 from a cold start** — re-running each acceptance check itself, to the **same bar as
      Phase 1's own DONE** (fresh PASS + evidence + timestamps logged in `REVIEW.md` within 24h). 1b
      cannot be DONE until **all of D1–D10 are re-confirmed green post-cleanup** and the Adversary
      confirms no test/assertion was softened, skipped, or otherwise regressed by the cleanup.
- [ ] **RL4 — Documented.** `docs/` notes how to run lint/format locally and that CI enforces it; any
      accepted deviations from the review checklist are in `DECISIONS.md`.

When RL1–RL4 hold and are confirmed, write `## DONE` to Phase-1b `STATUS.md`.

---

## 2. Method

1. **Tooling first.** Add the formatters/linters + a `lint` entrypoint + devshell; run format, then
   fix lint findings. Wire the `.drone.yml` lint stage so it's enforced from here on. (Auto-fixable
   style should just be auto-fixed — don't deliberate over it.)
2. **Then the review checklist (§3).** Read the code against the checklist; classify each finding
   **blocking** vs **advisory**; fix blocking, triage advisory.
3. **Re-verify everything last.** Once all fixes have landed, the Adversary re-runs the **entire**
   Phase-1 D1–D10 acceptance suite from a cold start (RL3) — this is the final gate; cleanup must
   regress nothing. Order matters: tooling → review responses/fixes → *then* full re-verification.
4. **Bound it.** Cap iterations; this is a pass, not a rewrite. Record dead-ends/deviations and stop.

---

## 3. The white-box review checklist (teeth, not taste)

Blocking unless noted; these are *plan-relevant invariants visible only by reading code*:

- **Tests are real (blocking).** Every recipe/harness test asserts actual app state — not
  health-only, not `skip`/`xfail`, not assertions that can't fail. (Pre-empts Phase-2 §7.1 at the
  foundation.)
- **Harness is DRY (blocking-ish).** Recipe-specific quirks live in the **shared harness**, not
  copy-pasted per recipe (the M6.5 "no harness surgery / no one-off hacks" rule) — so 18 recipes
  don't multiply the mess.
- **Server state is Nix-declared & idempotent (blocking).** Bring-up is declarative idempotent
  reconciliation (the `swarm-init`/`deploy-proxy` oneshot pattern, §9) — no imperative drift, no
  run-once sentinels, no manual post-rebuild steps in `install.md`.
- **No footguns (blocking).** No bare `sleep` for readiness (use polling); correct `--chaos` usage;
  no hardcoded versions/domains that break across upstream versions; teardown in `try/finally`;
  per-run secrets reused across stages (not regenerated).
- **No secrets in code or committed files (blocking).** Grep + read for tokens/passwords/keys in
  source, configs, `.drone.yml`, fixtures (complements the Adversary's behavioral leak test).
- **Architecture matches the plan (advisory→blocking on real drift).** Layout/》 modules follow
  `plan.md` §3; the trigger is poll-primary + optional manual webhook (§4.1); Traefik is the
  coop-cloud recipe (§4.2); deviations are recorded in `DECISIONS.md` (not silent).
- **Log redaction is real (blocking).** The log/dashboard pipeline scrubs secrets before publish.
- **Readability & error handling (advisory).** Clear names, sensible structure, errors surfaced not
  swallowed, dead code removed — advisory; auto-fixable parts belong to the linters, not debate.
- **Docs accurate (advisory).** `install.md` reproduces from scratch; `enroll-recipe.md` matches the
  real enroll flow.

---

## 4. Milestones (bounded)

- **W0 — Tooling + format.** Linters/formatters + `lint` entrypoint + devshell added; codebase
  auto-formatted; `.drone.yml` lint stage wired and green. *Accept:* `lint` passes in CI from a clean
  checkout.
- **W1 — Review + fixes.** Checklist (§3) run; blocking findings fixed; advisory triaged. *Accept:*
  `REVIEW.md` lists findings + resolutions; no blocking item open.
- **W2 — Full re-verification + document.** *After* W0+W1 fixes land, the Adversary re-verifies **all
  of D1–D10** cold (RL3) and docs/deviations are recorded. *Accept:* every Phase-1 D1–D10 item has a
  fresh post-cleanup PASS + evidence in `REVIEW.md` (nothing weakened); then flip Phase-1b
  `STATUS.md` to `## DONE`.

---

## 5. Guardrails

- **Never weaken a test to satisfy a lint/review nit** (the cardinal rule still wins).
- **Style → tooling, judgment → checklist.** Don't spend agent effort (or loop iterations) on what a
  formatter/linter settles deterministically; don't let the review drift into subjective taste.
- **Bounded.** This is a pass, not a refactor project — fix invariants + enforce linting, then stop;
  bigger improvements go to `IDEAS.md`, not this phase.
- **Don't reopen settled design.** Architecture decisions from Phase 1 stand unless there's real
  drift from the plan; record, don't relitigate.

---

## 6. Open decisions (log in DECISIONS.md)
- Exact formatter choices (alejandra vs nixpkgs-fmt; ruff-format vs black) and lint strictness.
- Whether to add Python **type-checking** (mypy/pyright) now or defer to `IDEAS.md`.
- The precise **blocking vs advisory** split for the checklist.
- Whether the `.drone.yml` lint stage should **fail** the build or just warn initially.

---

## 7. Operator review items (added 2026-05-27) — repo layout (do in this 1b pass)

Two structural-review items from the operator. Both are **blocking** for 1b. Apply them as part of
this pass, then re-verify (RL3 covers the re-verification). **Mind the coordination caveats — these
touch the live flake build and the running multi-agent machinery.**

### RL5 — Consolidate all Nix-code folders under a root `nix/`
- Move the folders that contain `.nix` code — **`modules/` and `hosts/`** — to **`nix/modules/` and
  `nix/hosts/`**. (Add future Nix dirs under `nix/` too.)
- **Keep `flake.nix` / `flake.lock` at the repo root** (entry point) so the build ref is unchanged
  (`docs/install.md`'s `nixos-rebuild switch --flake 'git+file://…?submodules=1#cc-ci'` stays valid).
  Just update the flake's internal paths (`./modules` → `./nix/modules`, `./hosts` → `./nix/hosts`)
  and any `imports`/`scripts`/`.drone.yml` references.
- **Re-verify after the move:** the byte-identical clean-room result is the bar. The toplevel store
  hash *will* change (paths differ) — that's fine; what must hold is that a fresh recursive clone
  still rebuilds **byte-identical to the running system** and the Adversary re-confirms it cold
  (folds into RL3). Update `docs/architecture.md` to describe the `nix/` layout.

### RL6 — Move uppercase multi-agent-protocol files into `machine-docs/`
- Move the uppercase protocol files — **`STATUS*.md`, `REVIEW*.md`, `JOURNAL*.md`, `BACKLOG*.md`,
  `DECISIONS.md`** — into a root **`machine-docs/`** folder. **`README.md` stays in the repo root**
  (operator decision, 2026-05-27) — it is the human-facing repo readme, not a protocol file; do
  **not** move it into `machine-docs/`.
- **Update every reference** to the new paths: the `cc-ci-plan/` plans (this file, `plan.md`,
  `plan-phase1c-*`, `README.md`, `kickoff.md`, `test-e2e-testme-acceptance.md`), `AGENTS.md`,
  `.drone.yml`, `scripts/`, and any in-repo doc that points at `STATUS.md`/`REVIEW.md`/etc.
- **⚠ COORDINATION CAVEAT (do not move these unilaterally mid-run):** the live **watchdog**
  (`cc-ci-plan/launch.sh`, the orchestrator's file) reads `STATUS-<id>.md` and `REVIEW-<id>.md` at
  the **repo root** to drive handoff pings + the 1c→1b auto-transition. Moving them breaks the
  running watchdog until `launch.sh` is updated to the `machine-docs/` paths and the watchdog is
  restarted. **So sequence it with the orchestrator:** the orchestrator updates `launch.sh`'s
  `PHASES_SPEC`/path logic and restarts the watchdog **in lockstep** with the loops' `git mv`.
  Safest to do this **near the end of 1b** (or as its final step), not while a phase transition is
  pending. Flag the orchestrator when ready and it will handle `launch.sh` + the watchdog restart.