olympus/infra-forge
3
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

make pre / make pre-down: the PRE rehearsal (#35) #45

Merged
pit merged 5 commits from hermes/35-make-pre into main 2026-10-06 16:46:21 +00:00
Conversation 3 Commits 5 Files changed 2 +108 -28
pit commented 2026-10-06 16:22:02 +00:00
Owner
Copy link

Summary

One command replaces the PRE rehearsal's remembered incantations; prod and PRE
never share a -target.

 pre          = Guest (PRE) -> service (PRE inventory)
-               plan?  apply?  which inventory?  branch checkout?
+               no Edge, no snapshot, no state backup
 pre-down     = destroy PRE Guest -> clear its host key

Both compositions are built from the same macros as prod, so the gate is the
same gate:

guest_apply  (targets, word)      service_apply  (inventory)
  guest_stack_init                  cd repo root; require vault-pass
    cd repo root; require .env      cd ansible
    cd tofu; source .env            ansible-playbook -i <inv> --check --diff
    tofu init                       ansible-playbook -i <inv>
  tofu plan -out=plan.out <targets>
  tofu show plan.out              pre:
  confirm by <word>                 guest_apply PRE_TARGETS, pre
  tofu apply plan.out               service_apply inventories/pre/hosts

Prod and PRE are told apart by Target selection alone — -target names the
Guest, the inventory names the service — so neither Target can reach the
other's Guest. The runbook's PRE section is rewritten around the two Targets.

Evidence

Dry-run assertions over the Target surface, plus an execution test of the gate
against stub binaries (no Proxmox, no Node, no Guest):

$ bash tests/make-targets.sh
the Target surface ........................................................
make pre — the PRE rehearsal .............................................
  ok    pre selects the PRE Guest
  ok    pre selects the shared base image
  ok    pre applies the service against the PRE inventory
  ok    pre never selects the Prod Guest
  ok    pre takes no Edge step / no snapshot / no state backup
  ok    pre runs the Guest before the service
make pre-down — the teardown .............................................
the Prod Targets still hold their own ground .............................
the apply gate, executed against stubs ...................................
  ok    pre with the wrong word applies nothing
  ok    pre with the right word applies the Guest
  ok    pre then plays against the PRE inventory

make-targets: 71 passed, 0 failed

make fmt clean; make validate -> "Success! The configuration is valid."

Merge Danger

Door: two-way

The Targets are additive; make guest and make service compose the same
lines as before the refactor. Nothing is applied by merging this — the
Makefile and the runbook change, the estate does not.

Blast Radius: low

make pre acts only on the throwaway PRE Guest and make pre-down destroys
only that container, both pinned by -target. A wrong word at the gate applies
nothing.

Refs #35

## Summary One command replaces the PRE rehearsal's remembered incantations; prod and PRE never share a `-target`. ```diff pre = Guest (PRE) -> service (PRE inventory) - plan? apply? which inventory? branch checkout? + no Edge, no snapshot, no state backup pre-down = destroy PRE Guest -> clear its host key ``` Both compositions are built from the same macros as prod, so the gate is the same gate: ```text guest_apply (targets, word) service_apply (inventory) guest_stack_init cd repo root; require vault-pass cd repo root; require .env cd ansible cd tofu; source .env ansible-playbook -i <inv> --check --diff tofu init ansible-playbook -i <inv> tofu plan -out=plan.out <targets> tofu show plan.out pre: confirm by <word> guest_apply PRE_TARGETS, pre tofu apply plan.out service_apply inventories/pre/hosts ``` Prod and PRE are told apart by Target selection alone — `-target` names the Guest, the inventory names the service — so neither Target can reach the other's Guest. The runbook's PRE section is rewritten around the two Targets. ## Evidence Dry-run assertions over the Target surface, plus an execution test of the gate against stub binaries (no Proxmox, no Node, no Guest): ``` $ bash tests/make-targets.sh the Target surface ........................................................ make pre — the PRE rehearsal ............................................. ok pre selects the PRE Guest ok pre selects the shared base image ok pre applies the service against the PRE inventory ok pre never selects the Prod Guest ok pre takes no Edge step / no snapshot / no state backup ok pre runs the Guest before the service make pre-down — the teardown ............................................. the Prod Targets still hold their own ground ............................. the apply gate, executed against stubs ................................... ok pre with the wrong word applies nothing ok pre with the right word applies the Guest ok pre then plays against the PRE inventory make-targets: 71 passed, 0 failed ``` `make fmt` clean; `make validate` -> "Success! The configuration is valid." ## Merge Danger **Door:** two-way The Targets are additive; `make guest` and `make service` compose the same lines as before the refactor. Nothing is applied by merging this — the Makefile and the runbook change, the estate does not. **Blast Radius:** low `make pre` acts only on the throwaway PRE Guest and `make pre-down` destroys only that container, both pinned by `-target`. A wrong word at the gate applies nothing. Refs #35
`make pre` creates the PRE Guest and applies the service to it against the
PRE inventory — Guest then service, with no Edge, no snapshot and no state
backup, since PRE is LAN-only and disposable. `make pre-down` destroys the
PRE Guest and clears its SSH host key. Prod and PRE are separated by Target
selection alone: the -target set and the inventory both name the environment,
so neither Target can reach the other Guest.

The Guest and service halves are extracted into `guest_stack_init`,
`guest_apply` and `service_apply` macros, so Prod and PRE share one apply
gate rather than a copy of it; `guest` and `service` are refactored onto
them, unchanged in what they compose. Each macro anchors to `$(CURDIR)`
first, which is what lets `pre` chain a Guest macro into a service macro
without the first one's `cd` leaking into the second's secret-file guard.
`make ssh-pre` reaches the PRE Guest.

The runbook's PRE section is rewritten around the two Targets, with the raw
commands kept as the manual path, and the apply gate now names `pre` as the
PRE word. `tests/make-targets.sh` asserts the Target surface by dry run —
what each Target would compose — and, for `pre`, executes the gate against
stub binaries to prove a wrong word applies nothing.
pit commented 2026-10-06 16:28:27 +00:00
Author
Owner
Copy link

Code review — main...hermes/35-make-pre

Two-axis review against issue #35. Verified live: make -n pre / make -n pre-down composed commands, bash tests/make-targets.sh → 71 passed / 0 failed, make fmt clean, make validate → "Success! The configuration is valid."

Standards

(a) Documented-standard violations

1. GLOSSARY.md — "Target … Avoid: recipe, rule, task" (letter is hard; scope arguable).
The new test header uses the banned synonym:

+# prints the command without executing it. Recipe internals (helper variables,

The glossary binds "every document — and every issue, commit and test name." A comment is prose, not a name, so coverage is debatable — but the word is on the avoid-list.

2. GLOSSARY.md — exposure classes ("LAN-reachable / WAN-published") (judgement).
The Makefile PRE comment coins a non-term for the defined class:

+# either can be built on a fresh Node. PRE is LAN-only and disposable, so `pre`

"LAN-only" is neither the glossary term nor an avoided synonym. The runbook's "no WAN entry" is compliant; the comment should read "LAN-reachable".

No breaches of AGENTS.md (Secrets/Privacy) or docs/index.md conventions: no secret or our-address is added (10.12.0.142 follows the existing 10.12.0.141), all added links are same-page anchors that resolve (#the-pre-rehearsal), and the section file-shape is unchanged.

(b) Baseline smells (all judgement calls)

Duplicated Code — the runbook states the PRE lifecycle twice. The new section near the top:

+| `make pre-down` | Destroys the PRE Guest and clears its SSH host key, so a finished rehearsal leaves nothing behind. |

repeats the edited section at the bottom:

-When the rehearsal is over:
+`make pre-down` is the teardown; by hand it is:

Both say "PRE is the disposable rehearsal Guest (LXC 142)" and both describe the same teardown/host-key clear. State the lifecycle once; have the other link to it.

Duplicated Code — service-layer preamble. service_apply re-states the pair already in service-check:

+define service_apply
+cd $(CURDIR)
+$(call require_secret,$(VAULT_PASS))
+cd $(CURDIR)/$(ANSIBLE_DIR)

The diff already extracted the Guest half (guest_stack_init); do the same here. Also, service_apply anchors with cd $(CURDIR) while service-check doesn't — the guest_stack_init comment argues the anchor matters.

Mysterious Name (weak). The test header's "the deliverable's public interface" names no concrete thing; use "the Target surface".

No Speculative Generality, Feature Envy, Data Clumps, or Middle Man spotted: the new $(1)/$(2) macro parameters are used by both Prod and PRE sites, not speculative.

Spec

(a) MISSING or PARTIAL — none

All four ACs are implemented. make -n pre composes -target=…debian_13_base -target=…container.forgejo_pre, then tofu plan -out=plan.out → show → word gate 'pre' → apply plan.out, then ansible-playbook -i inventories/pre/hosts …. No Edge, snapshot or state-backup line appears. make -n pre-down composes tofu destroy -target=...container.forgejo_pre then ssh-keygen -R "10.12.0.142" — no playbook, no Edge. The Prod path never selects forgejo_pre; the PRE path never selects forgejo (the prod_guest_target regex correctly excludes the _pre prefix). The rewritten ## PRE rehearsal section leads with the two Targets.

(b) NOT ASKED FOR (scope creep)

  • ssh-pre Target — +ssh-pre: ## Open a shell on the PRE Guest. Spec's "What to build" names only make pre and make pre-down.
  • tests/make-targets.sh (new, 199 lines) plus its runbook paragraph. No acceptance criterion asks for a target-surface test suite.
  • Prod-target refactor — guest, service, service-check are rewritten onto new guest_apply/service_apply macros. Behaviour is equivalent (make -n guest/service match main apart from an absolute cd), but it changes Prod surface AC4 didn't request. Neutral, not harmful.

(c) Looks implemented but WRONG

No spec-level AC is wrongly implemented; the concrete defect is in the added test's help assertion:

if grep -qE "^  make ${t}\b" <<<"$help_out"; then ok "help lists ${t}"; ...

\b matches at the - in service-check, pre-down, guest-plan, edge-plan, ssh-pre, so help lists pre (and service, guest, edge, ssh) pass even if the shorter Target were absent from make help. The deliverable's own pre/pre-down pair is therefore never actually verified by the suite. Reproduced live: a help listing containing only pre-down passes the help lists pre check.

Summary — Standards: 2 documented-standard findings (both judgement-weighted; neither hard-severe) + 3 baseline smells (2 Duplicated Code, 1 weak Mysterious Name); worst: the runbook's duplicated PRE lifecycle. Spec: 0 missing/partial, 3 scope-creep items, 1 wrong implementation; worst: the \b false-positive that leaves pre/pre-down unverified by the suite that exists to verify them.

All four acceptance criteria verified by executing the branch.

## Code review — `main...hermes/35-make-pre` Two-axis review against issue #35. Verified live: `make -n pre` / `make -n pre-down` composed commands, `bash tests/make-targets.sh` → 71 passed / 0 failed, `make fmt` clean, `make validate` → "Success! The configuration is valid." ### Standards **(a) Documented-standard violations** **1. GLOSSARY.md — "Target … _Avoid_: recipe, rule, task"** (letter is hard; scope arguable). The new test header uses the banned synonym: ``` +# prints the command without executing it. Recipe internals (helper variables, ``` The glossary binds "every document — and every issue, commit and test name." A comment is prose, not a name, so coverage is debatable — but the word is on the avoid-list. **2. GLOSSARY.md — exposure classes ("LAN-reachable / WAN-published")** (judgement). The Makefile PRE comment coins a non-term for the defined class: ``` +# either can be built on a fresh Node. PRE is LAN-only and disposable, so `pre` ``` "LAN-only" is neither the glossary term nor an avoided synonym. The runbook's "no WAN entry" is compliant; the comment should read "LAN-reachable". No breaches of AGENTS.md (Secrets/Privacy) or docs/index.md conventions: no secret or our-address is added (`10.12.0.142` follows the existing `10.12.0.141`), all added links are same-page anchors that resolve (`#the-pre-rehearsal`), and the section file-shape is unchanged. **(b) Baseline smells** (all judgement calls) **Duplicated Code — the runbook states the PRE lifecycle twice.** The new section near the top: ``` +| `make pre-down` | Destroys the PRE Guest and clears its SSH host key, so a finished rehearsal leaves nothing behind. | ``` repeats the edited section at the bottom: ``` -When the rehearsal is over: +`make pre-down` is the teardown; by hand it is: ``` Both say "PRE is the disposable rehearsal Guest (LXC 142)" and both describe the same teardown/host-key clear. State the lifecycle once; have the other link to it. **Duplicated Code — service-layer preamble.** `service_apply` re-states the pair already in `service-check`: ``` +define service_apply +cd $(CURDIR) +$(call require_secret,$(VAULT_PASS)) +cd $(CURDIR)/$(ANSIBLE_DIR) ``` The diff already extracted the Guest half (`guest_stack_init`); do the same here. Also, `service_apply` anchors with `cd $(CURDIR)` while `service-check` doesn't — the `guest_stack_init` comment argues the anchor matters. **Mysterious Name (weak).** The test header's "the deliverable's public interface" names no concrete thing; use "the Target surface". No Speculative Generality, Feature Envy, Data Clumps, or Middle Man spotted: the new `$(1)`/`$(2)` macro parameters are used by both Prod and PRE sites, not speculative. ### Spec **(a) MISSING or PARTIAL — none** All four ACs are implemented. `make -n pre` composes `-target=…debian_13_base -target=…container.forgejo_pre`, then `tofu plan -out=plan.out` → `show` → word gate `'pre'` → `apply plan.out`, then `ansible-playbook -i inventories/pre/hosts …`. No Edge, snapshot or state-backup line appears. `make -n pre-down` composes `tofu destroy -target=...container.forgejo_pre` then `ssh-keygen -R "10.12.0.142"` — no playbook, no Edge. The Prod path never selects `forgejo_pre`; the PRE path never selects `forgejo` (the `prod_guest_target` regex correctly excludes the `_pre` prefix). The rewritten `## PRE rehearsal` section leads with the two Targets. **(b) NOT ASKED FOR (scope creep)** - **`ssh-pre` Target** — `+ssh-pre: ## Open a shell on the PRE Guest`. Spec's "What to build" names only `make pre` and `make pre-down`. - **`tests/make-targets.sh`** (new, 199 lines) plus its runbook paragraph. No acceptance criterion asks for a target-surface test suite. - **Prod-target refactor** — `guest`, `service`, `service-check` are rewritten onto new `guest_apply`/`service_apply` macros. Behaviour is equivalent (`make -n guest`/`service` match `main` apart from an absolute `cd`), but it changes Prod surface AC4 didn't request. Neutral, not harmful. **(c) Looks implemented but WRONG** No *spec-level* AC is wrongly implemented; the concrete defect is in the added test's help assertion: ```bash if grep -qE "^ make ${t}\b" <<<"$help_out"; then ok "help lists ${t}"; ... ``` `\b` matches at the `-` in `service-check`, `pre-down`, `guest-plan`, `edge-plan`, `ssh-pre`, so `help lists pre` (and `service`, `guest`, `edge`, `ssh`) pass even if the shorter Target were absent from `make help`. The deliverable's own `pre`/`pre-down` pair is therefore never actually verified by the suite. Reproduced live: a help listing containing only `pre-down` passes the `help lists pre` check. --- **Summary** — Standards: 2 documented-standard findings (both judgement-weighted; neither hard-severe) + 3 baseline smells (2 Duplicated Code, 1 weak Mysterious Name); worst: the runbook's duplicated PRE lifecycle. Spec: 0 missing/partial, 3 scope-creep items, 1 wrong implementation; worst: the `\b` false-positive that leaves `pre`/`pre-down` unverified by the suite that exists to verify them. All four acceptance criteria verified by executing the branch. 
- tests: `help_has` now requires the Target name to be followed by a space or
  end-of-line. The old `\b` matched at the `-` of `pre-down`/`service-check`/…,
  so `help lists pre` passed on a listing that showed only `pre-down` — the
  deliverable's own `pre`/`pre-down` pair was never verified. A regression
  assertion pins the matcher against a synthetic `pre-down`-only listing.
- Makefile: "LAN-only" -> "LAN-reachable", the glossary's term for the class.
- Makefile: extract `service_stack_init`, so `service-apply` and `service-check`
  share the preamble as the Guest Targets do, and both anchor at `$(CURDIR)`.
- tests: replace "deliverable's public interface" with "Target surface" and
  "Recipe internals" with "Makefile internals" (the glossary's avoid-list).
- runbook: state the PRE lifecycle once. The Targets section keeps the table
  and links down; the procedure section no longer restates the LXC 142 line.
pit commented 2026-10-06 16:29:51 +00:00
Author
Owner
Copy link

All six findings addressed in 3a52d8c. Test count 71 -> 73.

Standards (a)(1) — "Recipe internals" / GLOSSARY avoid-list. Fixed: the test header now says "Makefile internals". Also removed the second avoid-list slip you flagged in (b): "the deliverable's public interface" -> "the Target surface".

Standards (a)(2) — "LAN-only" -> "LAN-reachable". Fixed in the Makefile PRE comment.

Standards (b) Duplicated Code — runbook PRE lifecycle stated twice. Fixed. ### The PRE rehearsal keeps the Target table and links down to ## PRE rehearsal; the bottom section no longer restates "PRE is the disposable rehearsal Guest (LXC 142)" / "It is destroyed between rehearsals" a second time, and the make pre-down prose ("destroys only the PRE container … host key changes with every clone") is stated once, in the teardown that follows.

Standards (b) Duplicated Code — service-layer preamble. Fixed: extracted service_stack_init (the shape of guest_stack_init), used by both service_apply and service-check — so service-check also gains the cd $(CURDIR) anchor it lacked. make -n service-check and make -n service still compose the same commands, now with the anchor first.

Spec (c) — the \b false positive. This was the real defect. help_has now matches ^ make <name>( |$), so the name must be followed by the listing's separator or end-of-line. A regression assertion runs the matcher against a synthetic pre-down-only listing and confirms it does not read as carrying pre, and that it does read pre-down. Both pass; help lists pre / help lists pre-down now verify the pair for real.

Spec (b) — scope creep: ssh-pre, tests/make-targets.sh + its runbook paragraph, Prod-target refactor. Kept, deliberately. All three come from parent epic #27, not this ticket: ssh-pre is in the epic's Utility Target set (User Story 20), and the test suite is the epic's Testing Decisions verbatim ("invoke a Target read-only and inspect it", "there is no test framework" — so the file had to land somewhere). The Prod refactor is the mechanism that lets pre reuse the gate rather than copy it. Happy to split any of these out if you'd rather #35 stay minimal.

The manual "by hand" block is labelled as the manual path rather than "the same run" now, since it does not itself drive the word gate — the earlier wording overstated the equivalence.

Verification on 3a52d8c: bash tests/make-targets.sh -> 73 passed / 0 failed; make fmt clean; make -n pre / pre-down / service / service-check compose unchanged command lines.

All six findings addressed in `3a52d8c`. Test count 71 -> 73. **Standards (a)(1) — "Recipe internals" / GLOSSARY avoid-list.** Fixed: the test header now says "Makefile internals". Also removed the second avoid-list slip you flagged in (b): "the deliverable's public interface" -> "the Target surface". **Standards (a)(2) — "LAN-only" -> "LAN-reachable".** Fixed in the Makefile PRE comment. **Standards (b) Duplicated Code — runbook PRE lifecycle stated twice.** Fixed. `### The PRE rehearsal` keeps the Target table and links down to `## PRE rehearsal`; the bottom section no longer restates "PRE is the disposable rehearsal Guest (LXC 142)" / "It is destroyed between rehearsals" a second time, and the `make pre-down` prose ("destroys only the PRE container … host key changes with every clone") is stated once, in the teardown that follows. **Standards (b) Duplicated Code — service-layer preamble.** Fixed: extracted `service_stack_init` (the shape of `guest_stack_init`), used by both `service_apply` and `service-check` — so `service-check` also gains the `cd $(CURDIR)` anchor it lacked. `make -n service-check` and `make -n service` still compose the same commands, now with the anchor first. **Spec (c) — the `\b` false positive.** This was the real defect. `help_has` now matches `^ make <name>( |$)`, so the name must be followed by the listing's separator or end-of-line. A regression assertion runs the matcher against a synthetic `pre-down`-only listing and confirms it does not read as carrying `pre`, and that it does read `pre-down`. Both pass; `help lists pre` / `help lists pre-down` now verify the pair for real. **Spec (b) — scope creep: `ssh-pre`, `tests/make-targets.sh` + its runbook paragraph, Prod-target refactor.** Kept, deliberately. All three come from parent epic #27, not this ticket: `ssh-pre` is in the epic's Utility Target set (User Story 20), and the test suite is the epic's Testing Decisions verbatim ("invoke a Target read-only and inspect it", "there is no test framework" — so the file had to land somewhere). The Prod refactor is the mechanism that lets `pre` reuse the gate rather than copy it. Happy to split any of these out if you'd rather #35 stay minimal. The manual "by hand" block is labelled as the manual path rather than "the same run" now, since it does not itself drive the word gate — the earlier wording overstated the equivalence. Verification on `3a52d8c`: `bash tests/make-targets.sh` -> 73 passed / 0 failed; `make fmt` clean; `make -n pre` / `pre-down` / `service` / `service-check` compose unchanged command lines. 
`tests/make-targets.sh` and the runbook paragraph that advertised it leave
this branch: the suite belongs to the epic's testing work, not this ticket.
Nothing else referenced it.
pit commented 2026-10-06 16:43:30 +00:00
Author
Owner
Copy link

Dropped tests/make-targets.sh (and the runbook paragraph that pointed at it) in 5a508eb, per review. The branch now touches only Makefile and docs/runbooks/0001-deploy-and-rollback.md — the pre/pre-down Targets and their docs. The suite itself is good; it belongs to the epic testing ticket rather than #35, so it can land there.

Dropped `tests/make-targets.sh` (and the runbook paragraph that pointed at it) in `5a508eb`, per review. The branch now touches only `Makefile` and `docs/runbooks/0001-deploy-and-rollback.md` — the `pre`/`pre-down` Targets and their docs. The suite itself is good; it belongs to the epic testing ticket rather than #35, so it can land there.
5a508eb removed tests/make-targets.sh but left the paragraph in the runbook
pointing at it; the reference is gone now.
# Conflicts:
#	Makefile
pit merged commit 0f587cd5c0 into main 2026-10-06 16:46:21 +00:00
pit deleted branch hermes/35-make-pre 2026-10-06 16:46:21 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Ready for an agent to pick up; see docs/agents/

  
ready-for-human
Requires human implementation

  
wontfix
Will not be actioned

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

  
review/merge-ready
No blocking findings; human merge pending

  
review/needs-fix
Reviewer found blocking findings; fixer dispatched

  
review/needs-human
Circuit stopped: needs a human decision

  
review/needs-review
Awaiting review by the review agent

  
wayfinder:grilling
HITL ticket: conversation; grilling + domain-modeling

  
wayfinder:map
The map itself: destination, decisions so far, and the fog

  
wayfinder:prototype
HITL ticket: a cheap concrete artifact to react to

  
wayfinder:research
AFK ticket: read docs/APIs for a fact a decision waits on

  
wayfinder:task
HITL or AFK ticket: manual work that unblocks a decision

  
wontfix
Will not be actioned

No labels
needs-info
needs-triage
ready-for-agent
ready-for-human
wontfix
needs-info
needs-triage
ready-for-agent
ready-for-human
review/merge-ready
review/needs-fix
review/needs-human
review/needs-review
wayfinder:grilling
wayfinder:map
wayfinder:prototype
wayfinder:research
wayfinder:task
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
olympus/infra-forge!45
No description provided.