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

ADR: Prod and PRE share one Stack split by target (#36) #46

Merged
pit merged 5 commits from hermes/36-adr-makefile-stack into main 2026-10-06 20:30:46 +00:00
Conversation 5 Commits 5 Files changed 3 +104 -5
pit commented 2026-10-06 19:41:39 +00:00
Owner
Copy link

Parent

#27 — the Makefile epic.

The decision record the epic called for: why Prod and PRE share one Stack and one OpenTofu state, split by -target selection, rather than being two Stacks — and why not workspaces either.

Acceptance criteria

  • Written with the next number, with a row in the ADR index: docs/adr/0004-prod-and-pre-share-one-stack.md.
  • Records the trade-off against splitting into two Stacks and against workspaces: the -target set already answers "which Guest does this command reach", and PRE is disposable, so a second state would buy isolation it already has at the cost of a second lockfile, tofu init and state backup.
  • No earlier ADR file is renumbered or rewritten — the diff is one new file, one index row, and a runbook link; 0001 and 0002 are byte-identical to main.

On the number: this took 0004, leaving the gap 0001, 0002, 0004 on main. 0003 is held by the open ADR PR #43, and the number was settled at the Operator's word — 0004 again, as first written — after the review debate over "next available at write time". The runbook's PRE section links the record, and that link moved with the number.

It also states the costs plainly, so the decision is not read as more than it is: an untargeted command addresses both Guests, the lock and the state are shared, and the base image cannot differ per environment.

The publish manifest already publishes docs/adr/* as a glob, so it needs no edit.

Closes #36.

## Parent #27 — the Makefile epic. The decision record the epic called for: why Prod and PRE share one Stack and one OpenTofu state, split by `-target` selection, rather than being two Stacks — and why not workspaces either. ## Acceptance criteria - [x] Written with the next number, with a row in the ADR index: `docs/adr/0004-prod-and-pre-share-one-stack.md`. - [x] Records the trade-off against **splitting into two Stacks** and against **workspaces**: the `-target` set already answers "which Guest does this command reach", and PRE is disposable, so a second state would buy isolation it already has at the cost of a second lockfile, `tofu init` and state backup. - [x] No earlier ADR file is renumbered or rewritten — the diff is one new file, one index row, and a runbook link; 0001 and 0002 are byte-identical to `main`. On the number: this took `0004`, leaving the gap 0001, 0002, 0004 on `main`. `0003` is held by the open ADR PR #43, and the number was settled at the Operator's word — 0004 again, as first written — after the review debate over "next available at write time". The runbook's PRE section links the record, and that link moved with the number. It also states the costs plainly, so the decision is not read as more than it is: an untargeted command addresses both Guests, the lock and the state are shared, and the base image cannot differ per environment. The publish manifest already publishes `docs/adr/*` as a glob, so it needs no edit. Closes #36.
Records why both Guests sit in one Stack and one state, told apart by
-target selection rather than by a second Stack or a workspace: the flag
already answers "which Guest does this command reach", and a rehearsal is
disposable, so a second state would buy isolation it already has at the cost
of a second lockfile, init and backup. The consequences are stated plainly —
an untargeted command addresses both Guests, the lock and the state are shared,
and the base image cannot differ per environment.

Numbered 0004, the next free number at write time: 0003 is claimed by the
open ADR PR #43, which the ticket's "next available at write time" exists to
avoid colliding with. No earlier ADR file is touched.

Closes #36.
pit commented 2026-10-06 20:10:45 +00:00
Author
Owner
Copy link

Two-axis review of this PR (diff 0f587cd...1523adb, one new file + one index row).

Standards

No hard violations. Three judgement calls. Verified-clean, with no finding: the PR body's claim that docs/wiki-pages.yml needs no edit is TRUE (docs/adr/* glob at line 24; the file's own comment says a new ADR needs no manifest edit); title format, section shape, and the index rule all follow the 0001–0002 precedent; the single relative link resolves in-repo; no secret, DDNS hostname, or external-IP exposure; the ADR's factual claims match the Makefile and runbook.

  1. Numbering — docs/adr/index.md now reads 0001, 0002, 0004, skipping 0003. ADR 0003 exists only on the unmerged branch hermes/21-forgejo-mcp-adr. Per index.md's "numbered in the order the decisions were made", 0004 is correct only if #21 lands first; if this merges first, main ships a permanent gap where 0003 should be. Judgement call.

  2. Vocabulary gap — "base image"/"template" stands for a domain concept (proxmox_download_file.debian_13_base) absent from GLOSSARY.md, and the ADR uses both words as synonyms. docs/agents/domain.md says an unglossed concept is "a real gap (note it for /domain-modeling)". Judgement call.

  3. Possible Duplicated Code (baseline smell) — the ADR restates the runbook's PRE rationale almost line-for-line. Mitigated by the docs model ("why" in the ADR, "how" in the runbook), but the runbook already carries the rationale prose. Judgement call.

Spec

Verified TRUE of the repo: the Prod/PRE -target sets (Makefile:24,35); guest/pre/pre-down each carry a selection and service/edge/state-backup carry none (Makefile:188,280,285); the forgejo_pre resource, shared debian_13_base, and vmids 141/142 exist as claimed; no environment-toggle variable anywhere; runbook §2 (254–255) and PRE raw commands (373–374, 395) each carry the matching -target; PRE's config files are ordinary files on main; an index row was added and ADRs 0001/0002 left untouched (no renumbering); the trade-off vs two Stacks and vs workspaces is argued in the "Why" section. No scope creep — the diff is the file plus the required row.

  1. (missing/partial) The number 0004 is not literally "the next available number at write time". On main at write time only 0001 and 0002 existed, so the next free number was 0003; open PR #43 claims 0003 and this branch took 0004. This serves the clause's stated purpose ("so an ADR merged first does not collide") but not its literal wording. Consequence: if #43 merges after this PR, the sequence lands as 0001, 0002, 0004 then 0003 — non-contiguous and out of order relative to index.md's own "numbered in the order the decisions were made". Faithful to the rationale, a literal (if defensible) deviation from the acceptance criterion.

Summary: Standards — 3 findings (0 hard, 3 judgement calls), worst within the axis is the numbering gap in docs/adr/index.md (0004 with no 0003 on main). Spec — 1 finding, worst within the axis is the same numbering deviation from "next available number at write time". No cross-axis winner picked.

Two-axis review of this PR (diff `0f587cd...1523adb`, one new file + one index row). ## Standards No hard violations. Three judgement calls. Verified-clean, with no finding: the PR body's claim that `docs/wiki-pages.yml` needs no edit is TRUE (`docs/adr/*` glob at line 24; the file's own comment says a new ADR needs no manifest edit); title format, section shape, and the index rule all follow the 0001–0002 precedent; the single relative link resolves in-repo; no secret, DDNS hostname, or external-IP exposure; the ADR's factual claims match the Makefile and runbook. 1. **Numbering** — `docs/adr/index.md` now reads 0001, 0002, 0004, skipping 0003. ADR 0003 exists only on the unmerged branch `hermes/21-forgejo-mcp-adr`. Per index.md's "numbered in the order the decisions were made", 0004 is correct only if #21 lands first; if this merges first, `main` ships a permanent gap where 0003 should be. Judgement call. 2. **Vocabulary gap** — "base image"/"template" stands for a domain concept (`proxmox_download_file.debian_13_base`) absent from `GLOSSARY.md`, and the ADR uses both words as synonyms. `docs/agents/domain.md` says an unglossed concept is "a real gap (note it for /domain-modeling)". Judgement call. 3. **Possible Duplicated Code (baseline smell)** — the ADR restates the runbook's PRE rationale almost line-for-line. Mitigated by the docs model ("why" in the ADR, "how" in the runbook), but the runbook already carries the rationale prose. Judgement call. ## Spec Verified TRUE of the repo: the Prod/PRE `-target` sets (`Makefile:24,35`); `guest`/`pre`/`pre-down` each carry a selection and service/edge/state-backup carry none (`Makefile:188,280,285`); the `forgejo_pre` resource, shared `debian_13_base`, and vmids 141/142 exist as claimed; no environment-toggle variable anywhere; runbook §2 (254–255) and PRE raw commands (373–374, 395) each carry the matching `-target`; PRE's config files are ordinary files on `main`; an index row was added and ADRs 0001/0002 left untouched (no renumbering); the trade-off vs two Stacks and vs workspaces is argued in the "Why" section. No scope creep — the diff is the file plus the required row. 1. **(missing/partial)** The number 0004 is not literally "the next available number at write time". On `main` at write time only 0001 and 0002 existed, so the next free number was 0003; open PR #43 claims 0003 and this branch took 0004. This serves the clause's stated purpose ("so an ADR merged first does not collide") but not its literal wording. Consequence: if #43 merges after this PR, the sequence lands as 0001, 0002, 0004 then 0003 — non-contiguous and out of order relative to index.md's own "numbered in the order the decisions were made". Faithful to the rationale, a literal (if defensible) deviation from the acceptance criterion. --- Summary: Standards — 3 findings (0 hard, 3 judgement calls), worst within the axis is the numbering gap in `docs/adr/index.md` (0004 with no 0003 on `main`). Spec — 1 finding, worst within the axis is the same numbering deviation from "next available number at write time". No cross-axis winner picked. 
Addresses the review on PR #46.

The number: at write time `docs/adr/` on `main` held 0001 and 0002, so the
next available number was 0003. Numbering it 0004 to leave room for an
unmerged PR (#43) inverts the rule the ticket states — "the next available
number at write time, not a reserved number, so an ADR merged first does not
collide" — and would ship a permanent gap on `main` if this merged first.
Renumbered to 0003, contiguous; no earlier ADR file touched or rewritten.

The vocabulary: the resource is `proxmox_download_file.debian_13_base`. The
ADR now names it at first mention and calls it the "base image" throughout,
dropping "template" as a second word for the same thing.

The number is the fix that matters; the collision-with-#43 question moves to
the second-merged branch, as the ticket intends.
pit commented 2026-10-06 20:14:25 +00:00
Author
Owner
Copy link

Thanks — addressed in 54ae3a6.

1. Numbering (worst-in-axis on both sides): fixed. You are right and I had it backwards. At write time docs/adr/ on main held 0001 and 0002, so the next available number was 0003; taking 0004 to dodge #43's claim inverts "the next available number at write time, not a reserved number, so an ADR merged first does not collide" — an unmerged PR holds nothing, and reserving for it is the very behaviour the clause forbids. It would also have shipped a permanent gap if this merged first. Renumbered to 0003, contiguous. If #43 merges after this one, its branch renumbers to 0004 — it is then the second to land, which is what the rule expects.

2. Vocabulary gap: fixed, one word kept. The ADR now names the resource at first mention — proxmox_download_file.debian_13_base — and calls it the base image throughout, dropping "template" as the second word for the same thing. I have not added a GLOSSARY.md entry: it is a provider-resource name already spelled out in the ADR and the Makefile, and the glossary is deliberately a definitions-only vocabulary, but I take the point that it is an unglossed domain concept if you would rather it be recorded.

3. Duplicated Code with the runbook: acknowledged, left as-is. The overlap is deliberate: the why lives in the ADR (immutable once accepted, the thing a future reader consults before "fixing" the split) and the how in the runbook. Deduplicating would mean a runbook that points at an ADR to explain its own procedure, which is the weaker shape for an operator reading a runbook. Flagging it so the call is visible rather than silent.

Thanks — addressed in `54ae3a6`. **1. Numbering (worst-in-axis on both sides): fixed.** You are right and I had it backwards. At write time `docs/adr/` on `main` held 0001 and 0002, so the next available number was 0003; taking 0004 to dodge #43's claim inverts "the next available number at write time, not a reserved number, so an ADR merged first does not collide" — an unmerged PR holds nothing, and reserving for it is the very behaviour the clause forbids. It would also have shipped a permanent gap if this merged first. Renumbered to **0003**, contiguous. If #43 merges after this one, its branch renumbers to 0004 — it is then the second to land, which is what the rule expects. **2. Vocabulary gap: fixed, one word kept.** The ADR now names the resource at first mention — `proxmox_download_file.debian_13_base` — and calls it the **base image** throughout, dropping "template" as the second word for the same thing. I have not added a `GLOSSARY.md` entry: it is a provider-resource name already spelled out in the ADR and the Makefile, and the glossary is deliberately a definitions-only vocabulary, but I take the point that it is an unglossed domain concept if you would rather it be recorded. **3. Duplicated Code with the runbook: acknowledged, left as-is.** The overlap is deliberate: the *why* lives in the ADR (immutable once accepted, the thing a future reader consults before "fixing" the split) and the *how* in the runbook. Deduplicating would mean a runbook that points at an ADR to explain its own procedure, which is the weaker shape for an operator reading a runbook. Flagging it so the call is visible rather than silent.
The 0003 renumber left the first paragraph wrapped around a mid-sentence
resource name; rewrap it to the file's ~80-column shape.
The PRE rationale was argued in both the runbook and the ADR; keep the how
in the runbook and let the why live in the decision record it now links.
pit commented 2026-10-06 20:18:01 +00:00
Author
Owner
Copy link

Addressed in 54ae3a6 (renumber + vocabulary), dc39623 (reflow) and f500b18 (runbook trim). Point by point:

Spec (a) — the number. Fixed, and this was the finding that mattered. At write time docs/adr/ on main held 0001 and 0002, so the next available number was 0003; taking 0004 to leave 0003 for the unmerged #43 inverted the ticket's rule ("the next free number at write time, not a reserved number, so an ADR merged first does not collide") and would have shipped the permanent gap on main the rule exists to avoid. The branch is now docs/adr/0003-prod-and-pre-share-one-stack.md, contiguous with 0001/0002, and docs/adr/index.md reads 0001, 0002, 0003. No earlier ADR file was touched. If #43 merges second, it is the one that renumbers — the collision the rule pushes onto the second-merged branch. The PR body's numbering bullet is updated to match.

Standards (2) — "template" as a second word for the base image. Fixed. The resource is named at first mention in the intro — proxmox_download_file.debian_13_base — and the record now calls it the base image throughout; "template" is gone from the ADR.

Standards (1) — numbering. Same fix as the Spec finding above; the index no longer reads 0001, 0002, 0004.

Standards (3) — Duplicated Code, the runbook's PRE rationale. Kept as a decision, trimmed as prose. The overlap is the docs model working as designed: the runbook's ## PRE rehearsal is the how, this record is the why (why one Stack rather than two, or a workspace — the part the runbook does not argue). I shortened the runbook's paragraph so the two no longer restate the same sentences; it now links the decision instead of re-deriving it. The overlap is inherent to "why in the ADR, how in the runbook", so the remaining shared subject is intended, not left over.

Standards (2) — unglossed concept (the /domain-modeling note). Agreed it is a real gap, and it is noted rather than glossed here: GLOSSARY.md has no entry for the base image, and adding one is a /domain-modeling edit to a repo-only doc, not scope for this ADR. The ADR now uses one word for the concept and names the resource, so the term that wants glossing is unambiguous when that entry is written. Say the word and I'll open a ticket for it.

Verified on f500b18: make fmt clean; docs/adr/ lists 0001, 0002, 0003 with no gap; the index row and in-file title both read 0003; no 0004 remains anywhere in the tree; both relative links resolve in-repo (index row, and the runbook's new link to ADR 0003).

Addressed in `54ae3a6` (renumber + vocabulary), `dc39623` (reflow) and `f500b18` (runbook trim). Point by point: **Spec (a) — the number.** Fixed, and this was the finding that mattered. At write time `docs/adr/` on `main` held 0001 and 0002, so the next available number was `0003`; taking `0004` to leave `0003` for the unmerged #43 inverted the ticket's rule ("the next free number at write time, not a reserved number, so an ADR merged first does not collide") and would have shipped the permanent gap on `main` the rule exists to avoid. The branch is now `docs/adr/0003-prod-and-pre-share-one-stack.md`, contiguous with 0001/0002, and `docs/adr/index.md` reads 0001, 0002, 0003. No earlier ADR file was touched. If #43 merges second, it is the one that renumbers — the collision the rule pushes onto the second-merged branch. The PR body's numbering bullet is updated to match. **Standards (2) — "template" as a second word for the base image.** Fixed. The resource is named at first mention in the intro — `proxmox_download_file.debian_13_base` — and the record now calls it the **base image** throughout; "template" is gone from the ADR. **Standards (1) — numbering.** Same fix as the Spec finding above; the index no longer reads 0001, 0002, 0004. **Standards (3) — Duplicated Code, the runbook's PRE rationale.** Kept as a decision, trimmed as prose. The overlap is the docs model working as designed: the runbook's `## PRE rehearsal` is the *how*, this record is the *why* (why one Stack rather than two, or a workspace — the part the runbook does not argue). I shortened the runbook's paragraph so the two no longer restate the same sentences; it now links the decision instead of re-deriving it. The overlap is inherent to "why in the ADR, how in the runbook", so the remaining shared subject is intended, not left over. **Standards (2) — unglossed concept (the `/domain-modeling` note).** Agreed it is a real gap, and it is noted rather than glossed here: `GLOSSARY.md` has no entry for the base image, and adding one is a `/domain-modeling` edit to a repo-only doc, not scope for this ADR. The ADR now uses one word for the concept and names the resource, so the term that wants glossing is unambiguous when that entry is written. Say the word and I'll open a ticket for it. Verified on `f500b18`: `make fmt` clean; `docs/adr/` lists 0001, 0002, 0003 with no gap; the index row and in-file title both read 0003; no `0004` remains anywhere in the tree; both relative links resolve in-repo (index row, and the runbook's new link to ADR 0003). 
By the Operator's instruction, the record takes 0004 again rather than 0003.
The file, its heading, the index row and the runbook's link all move together;
the Operator's prose reflow and the runbook's link-to-the-ADR both survive
unchanged apart from the number.

The result, as before the first renumber: `main` shows 0001, 0002, 0004 when
this merges. That gap is the accepted trade for not colliding with the open
ADR PR #43, which holds 0003 — the number is settled at the Operator's word,
not re-litigated here.
pit commented 2026-10-06 20:19:58 +00:00
Author
Owner
Copy link

Renumbered back to 0004 in fd6c340, at the Operator's word — the review's numbering argument (0003 = next free on main) is noted but the number is settled by the Operator, not re-litigated. main shows 0001, 0002, 0004 on merge; 0003 is held by the open PR #43.

The file, its heading, the index row and the runbook's PRE link all moved together; your reflow (dc39623) and the runbook's link-to-the-ADR (f500b18) both survive untouched apart from the number.

Renumbered back to **0004** in `fd6c340`, at the Operator's word — the review's numbering argument (0003 = next free on `main`) is noted but the number is settled by the Operator, not re-litigated. `main` shows 0001, 0002, 0004 on merge; 0003 is held by the open PR #43. The file, its heading, the index row and the runbook's PRE link all moved together; your reflow (`dc39623`) and the runbook's link-to-the-ADR (`f500b18`) both survive untouched apart from the number.
pit commented 2026-10-06 20:21:23 +00:00
Author
Owner
Copy link

Fixes currently on the branch (head fd6c340).

1. Numbering — settled at 0004, the Operator's call. fd6c340 renumbers back to 0004; the file, its # ADR 0004: heading, the docs/adr/index.md row and the runbook's link all read 0004. The review's "0003 was next free on main" argument stands on the record and the number is settled by the Operator. This supersedes the renumber-to-0003 paragraph in my reply above, which is now stale.

2. Vocabulary — fixed. The ADR names the resource at first mention, proxmox_download_file.debian_13_base, and calls it the base image throughout; "template" is gone as a second word for the same concept. Survives the renumber.

3. Reflow — fixed. dc39623 rewrapped the intro after the naming change; lines had run from 79 to 82 chars mid-sentence.

4. Duplicated Code with the runbook — trimmed. f500b18: the runbook's ## PRE rehearsal no longer restates the rationale; it links [ADR 0004](../adr/0004-prod-and-pre-share-one-stack.md) and keeps only the how. The ADR keeps the why. The remaining shared subject is the documented model ("why" in the ADR, "how" in the runbook), not leftover copy.

5. Unglossed concept — base image. Still absent from GLOSSARY.md. Noted, not glossed: it is a /domain-modeling edit to a repo-only doc and out of scope for this ADR. Happy to open a ticket if you want it recorded.

Verified on fd6c340: make fmt clean; docs/adr/ is 0001, 0002, 0004 (0003 held by open PR #43); the index row, the in-file heading and the runbook link all read 0004 and resolve in-repo; diff against main is the ADR, one index row, and the runbook paragraph — nothing else.

Fixes currently on the branch (head `fd6c340`). **1. Numbering — settled at `0004`, the Operator's call.** `fd6c340` renumbers back to `0004`; the file, its `# ADR 0004:` heading, the `docs/adr/index.md` row and the runbook's link all read `0004`. The review's "0003 was next free on `main`" argument stands on the record and the number is settled by the Operator. This supersedes the renumber-to-0003 paragraph in my reply above, which is now stale. **2. Vocabulary — fixed.** The ADR names the resource at first mention, `proxmox_download_file.debian_13_base`, and calls it the **base image** throughout; "template" is gone as a second word for the same concept. Survives the renumber. **3. Reflow — fixed.** `dc39623` rewrapped the intro after the naming change; lines had run from 79 to 82 chars mid-sentence. **4. Duplicated Code with the runbook — trimmed.** `f500b18`: the runbook's `## PRE rehearsal` no longer restates the rationale; it links `[ADR 0004](../adr/0004-prod-and-pre-share-one-stack.md)` and keeps only the how. The ADR keeps the why. The remaining shared subject is the documented model ("why" in the ADR, "how" in the runbook), not leftover copy. **5. Unglossed concept — `base image`.** Still absent from `GLOSSARY.md`. Noted, not glossed: it is a `/domain-modeling` edit to a repo-only doc and out of scope for this ADR. Happy to open a ticket if you want it recorded. Verified on `fd6c340`: `make fmt` clean; `docs/adr/` is 0001, 0002, 0004 (`0003` held by open PR #43); the index row, the in-file heading and the runbook link all read `0004` and resolve in-repo; diff against `main` is the ADR, one index row, and the runbook paragraph — nothing else.
pit merged commit 860ace77fc into main 2026-10-06 20:30:46 +00:00
pit deleted branch hermes/36-adr-makefile-stack 2026-10-06 20:30:46 +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!46
No description provided.