olympus/infra-tracker

Fork 0

docs(domain): dev/ci Vault + glossary for CI credentials (ADR 0002) #11

Merged

pit merged 2 commits from docs/ci-credentials into main

2026-10-09 15:24:51 +00:00

pit commented

2026-10-09 14:55:50 +00:00

Owner

Summary

Records the credential decision for the test Suite: a dev/ci Vault beside the dev/ci Inventory, and the prod Vault moved out of the playbook-adjacent group_vars/ so a test run never loads it.

 vikunja/service/
 ├── roles/vikunja/            # unchanged
 ├── site.yml ansible.cfg       # unchanged
-├── group_vars/vikunja/
-│   ├── vars.yml               # non-secret, stays
-│   └── vault.yml              # PROD VAULT - moved
+├── inventories/
+│   ├── prod/
+│   │   ├── hosts
+│   │   └── group_vars/vikunja/vault.yml   # prod Vault (real values)
+│   └── ci/
+│       ├── hosts
+│       └── group_vars/vikunja/vault.yml   # dev/ci Vault (test values)
+└── group_vars/vikunja/vars.yml            # non-secret, stays

Evidence

Before: a vault auto-loaded from playbook-adjacent group_vars/ hard-fails a vault-less run.
After: inventory-adjacent placement loads only for its own Inventory.

# playbook-adjacent vault.yml, no password:
[ERROR]: Attempting to decrypt but no vault secrets found.

# prod Inventory-adjacent prod Vault, real password:
db=REALPROD pit=realprodpitpw domain=vikunja.thepit.space

# dev/ci Inventory, plaintext test values, NO vault password:
db=fakeprod pit=fakepitpw domain=vikunja.thepit.space

(Rehearsed locally on ansible-core 2.21.3 against a throwaway tree; the prod layout and the dev/ci layout both resolve.)

Merge Danger

Door: two-way

The change is docs-only - an ADR, glossary terms, and README wording. Reverting the branch restores the old wording; nothing reads these files at runtime.

Blast Radius: docs

It records a decision the execution effort will implement (the actual vault move and the two-vault layout are code, still to come). No runtime path changes here.

## Summary Records the credential decision for the test Suite: a **dev/ci Vault** beside the dev/ci Inventory, and the **prod Vault** moved out of the playbook-adjacent `group_vars/` so a test run never loads it. ```diff vikunja/service/ ├── roles/vikunja/ # unchanged ├── site.yml ansible.cfg # unchanged -├── group_vars/vikunja/ -│ ├── vars.yml # non-secret, stays -│ └── vault.yml # PROD VAULT - moved +├── inventories/ +│ ├── prod/ +│ │ ├── hosts +│ │ └── group_vars/vikunja/vault.yml # prod Vault (real values) +│ └── ci/ +│ ├── hosts +│ └── group_vars/vikunja/vault.yml # dev/ci Vault (test values) +└── group_vars/vikunja/vars.yml # non-secret, stays ``` ## Evidence - **Before:** a vault auto-loaded from playbook-adjacent `group_vars/` hard-fails a vault-less run. **After:** inventory-adjacent placement loads only for its own Inventory. ```text # playbook-adjacent vault.yml, no password: [ERROR]: Attempting to decrypt but no vault secrets found. # prod Inventory-adjacent prod Vault, real password: db=REALPROD pit=realprodpitpw domain=vikunja.thepit.space # dev/ci Inventory, plaintext test values, NO vault password: db=fakeprod pit=fakepitpw domain=vikunja.thepit.space ``` (Rehearsed locally on ansible-core 2.21.3 against a throwaway tree; the prod layout and the dev/ci layout both resolve.) ## Merge Danger **Door:** two-way The change is docs-only - an ADR, glossary terms, and README wording. Reverting the branch restores the old wording; nothing reads these files at runtime. **Blast Radius:** docs It records a decision the execution effort will implement (the actual vault move and the two-vault layout are code, still to come). No runtime path changes here.

pit added 1 commit

2026-10-09 14:55:51 +00:00

docs(domain): dev/ci vault + glossary for CI credentials (ADR 0002) c444035245

pit commented

2026-10-09 15:17:36 +00:00

Author

Owner

Code review — two axes

Fixed point: main (2cc9500) ... docs/ci-credentials (c444035) — 1 commit, 3 files (GLOSSARY.md +22, README.md +3/-1, docs/adr/0002-...md +37 new).
Spec source: Vikunja INFRATRACK-8 ("Decide CI secrets and the run's least-privilege credentials") + its resolution comment (map INFRATRACK-2).

The two axes are reported separately and not reranked against each other.

Standards

Documented standards

GLOSSARY.md (### Credentials)

All four terms carry an _Avoid_ list; subheading grouping and 1–2 sentence definitions comply with GLOSSARY-FORMAT.md. No hard breach.
Judgement call — "Be opinionated" (GLOSSARY-FORMAT.md): "vault" appears three ways in one glossary — Vault, Prod Vault (Title Case) vs dev/ci vault (lower). The dev/ci vault _Avoid_ list names "ci vault", yet the canonical term keeps the slash. Pick one casing/form.

docs/adr/0002-...md (new)

Complies: # {Short title}, status: accepted, sequential 0002-slug.md, named Considered/Consequences sections (ADR-FORMAT.md); in docs/adr/ per docs/agents/domain.md.
Hard breach (letter) — "1-3 sentences" body (ADR-FORMAT.md): the opening is 3 sentences, but a second full paragraph ("The prod Vault moves from…") precedes Considered:; it is neither a labelled optional section nor inside the 3-sentence body. Fold it in or label it.
Judgement call — title casing: "not the prod Vault" lowercases a glossary term defined as Prod Vault.

Terminology — ADR and README use Suite, Guest, Inventory, dev/ci vault, Prod Vault consistently with GLOSSARY.md (satisfies domain.md "Use the glossary's vocabulary"). No avoid-list synonyms leaked.

README.md (+3/-1)

Judgement call — self-contradiction introduced: the diff repoints line ~80 to vikunja/service/inventories/prod/group_vars/vikunja/vault.yml, but line ~108's command still reads ansible-vault edit group_vars/vikunja/vault.yml, and line ~89 still says "The vault password lives in the root .env" (singular), while the ADR describes two vaults, each with its own password.

Baseline smells (all judgement calls)

Duplicated Code — README: the vault path is written twice (line 80 vs line 108); the diff changed one, so the copies now disagree.
Mysterious Name — GLOSSARY.md: dev/ci vault vs Prod Vault are sibling concepts named in different registers.

Not applicable: the PR body (pr skill format) is not in this docs diff; tooling-enforced checks skipped.

Spec

The ADR/glossary/README record the decision's core (dev/ci vault split, forced move + reason, glossary terms, README path) but omit several spec-required elements.

(a) Missing / partial

Proxmox endpoint as repo Variable. Spec: "Proxmox endpoint = repo Variable (LAN address, not a secret, kept out of the secret store)." ADR lists only "a fresh, scoped Proxmox token, the dev/ci vault password, a fresh CI SSH keypair, and a fresh registry token" — the endpoint/Variable distinction is absent everywhere.
Registry token scope + fallback. Spec: "registry token = CI secret (fresh, packages:read only); fallback: make the image public." ADR says only "a fresh registry token" — no packages:read, no public-image fallback, and the unproven runner-credentials fact is not recorded.
Consumption mechanism. Spec: "CI runs ansible-playbook -i <dev/ci inventory> ... --vault-password-file <dev/ci pass> — no prod secret, no -e." ADR records the rationale against -e but never states the command; how secrets are "kept out of logs" is not addressed at all.
Service-secret list + placeholders. Spec: "vault_vikunja_db_password, _service_secret, _account_passwords incl. pit, _smtp_password move into the dev/ci vault … the others are placeholders." ADR mentions only the empty SMTP password; the other three moving as placeholders is unstated.
Ripple INFRATRACK-21. Spec: "how the CI keypair's public half is injected is now INFRATRACK-21." ADR records "a fresh CI SSH keypair" but no INFRATRACK-21 reference; the "public half trusted by the per-run Guest" mechanism is lost.
Verification detail. Spec: "Verified locally on ansible-core 2.21.3." ADR says only "verified" — version dropped.

(b) Scope creep

ADR: "A pull_request from a fork sees an empty secrets map, which this single-user instance does not hit today." Not part of the recorded decision.

(c) Looks implemented, but wrong

README internal inconsistency: line 80 now points to vikunja/service/inventories/prod/group_vars/vikunja/vault.yml, yet the edit example 28 lines below still reads ansible-vault edit group_vars/vikunja/vault.yml — stale, contradicts the path the diff just changed.

The glossary terms (Vault, Prod Vault, dev/ci vault, CI secret) and the forced-move justification are faithful.

Summary

Standards: 6 findings (1 hard breach — ADR body exceeds the 1-3 sentence rule; 5 judgement calls). Worst within the axis: the README self-contradiction — the diff repoints the vault path but leaves its sibling command and the "one password" line stale.
Spec: 8 findings (6 missing/partial, 1 scope creep, 1 wrong). Worst within the axis: the consumption mechanism is absent — the spec asked for it explicitly (the exact ansible-playbook invocation and how secrets stay out of logs), and the ADR never states it.

The docs conform closely to house style yet under-record the decision they exist to capture, and the README edit introduces a contradiction in the file it touched.

Review run via the code-review skill (two parallel axes, aggregated, not reranked).

## Code review — two axes Fixed point: `main` (2cc9500) ... `docs/ci-credentials` (c444035) — 1 commit, 3 files (GLOSSARY.md +22, README.md +3/-1, docs/adr/0002-...md +37 new). Spec source: Vikunja **INFRATRACK-8** ("Decide CI secrets and the run's least-privilege credentials") + its resolution comment (map INFRATRACK-2). The two axes are reported separately and not reranked against each other. ## Standards **Documented standards** `GLOSSARY.md` (`### Credentials`) - All four terms carry an `_Avoid_` list; subheading grouping and 1–2 sentence definitions comply with `GLOSSARY-FORMAT.md`. No hard breach. - Judgement call — "Be opinionated" (`GLOSSARY-FORMAT.md`): "vault" appears three ways in one glossary — `Vault`, `Prod Vault` (Title Case) vs `dev/ci vault` (lower). The `dev/ci vault` `_Avoid_` list names "ci vault", yet the canonical term keeps the slash. Pick one casing/form. `docs/adr/0002-...md` (new) - Complies: `# {Short title}`, `status: accepted`, sequential `0002-slug.md`, named Considered/Consequences sections (`ADR-FORMAT.md`); in `docs/adr/` per `docs/agents/domain.md`. - **Hard breach (letter)** — "1-3 sentences" body (`ADR-FORMAT.md`): the opening is 3 sentences, but a second full paragraph ("The prod Vault **moves** from…") precedes `Considered:`; it is neither a labelled optional section nor inside the 3-sentence body. Fold it in or label it. - Judgement call — title casing: "not the prod Vault" lowercases a glossary term defined as `Prod Vault`. Terminology — ADR and README use `Suite`, `Guest`, `Inventory`, `dev/ci vault`, `Prod Vault` consistently with `GLOSSARY.md` (satisfies `domain.md` "Use the glossary's vocabulary"). No avoid-list synonyms leaked. `README.md` (+3/-1) - Judgement call — self-contradiction introduced: the diff repoints line ~80 to `vikunja/service/inventories/prod/group_vars/vikunja/vault.yml`, but line ~108's command still reads `ansible-vault edit group_vars/vikunja/vault.yml`, and line ~89 still says "The vault password lives in the root `.env`" (singular), while the ADR describes two vaults, each with its own password. **Baseline smells (all judgement calls)** - Duplicated Code — README: the vault path is written twice (line 80 vs line 108); the diff changed one, so the copies now disagree. - Mysterious Name — GLOSSARY.md: `dev/ci vault` vs `Prod Vault` are sibling concepts named in different registers. Not applicable: the PR body (`pr` skill format) is not in this docs diff; tooling-enforced checks skipped. ## Spec The ADR/glossary/README record the decision's **core** (dev/ci vault split, forced move + reason, glossary terms, README path) but omit several spec-required elements. **(a) Missing / partial** - **Proxmox endpoint as repo Variable.** Spec: "Proxmox endpoint = repo Variable (LAN address, not a secret, kept out of the secret store)." ADR lists only "a fresh, scoped Proxmox token, the dev/ci vault password, a fresh CI SSH keypair, and a fresh registry token" — the endpoint/Variable distinction is absent everywhere. - **Registry token scope + fallback.** Spec: "registry token = CI secret (fresh, `packages:read` only); fallback: make the image public." ADR says only "a fresh registry token" — no `packages:read`, no public-image fallback, and the unproven runner-credentials fact is not recorded. - **Consumption mechanism.** Spec: "CI runs `ansible-playbook -i <dev/ci inventory> ... --vault-password-file <dev/ci pass>` — no prod secret, no `-e`." ADR records the rationale against `-e` but never states the command; how secrets are "kept out of logs" is not addressed at all. - **Service-secret list + placeholders.** Spec: "`vault_vikunja_db_password`, `_service_secret`, `_account_passwords` incl. `pit`, `_smtp_password` move into the dev/ci vault … the others are placeholders." ADR mentions only the empty SMTP password; the other three moving as placeholders is unstated. - **Ripple INFRATRACK-21.** Spec: "how the CI keypair's public half is injected is now INFRATRACK-21." ADR records "a fresh CI SSH keypair" but no INFRATRACK-21 reference; the "public half trusted by the per-run Guest" mechanism is lost. - **Verification detail.** Spec: "Verified locally on ansible-core 2.21.3." ADR says only "verified" — version dropped. **(b) Scope creep** - ADR: "A `pull_request` from a fork sees an empty secrets map, which this single-user instance does not hit today." Not part of the recorded decision. **(c) Looks implemented, but wrong** - README internal inconsistency: line 80 now points to `vikunja/service/inventories/prod/group_vars/vikunja/vault.yml`, yet the edit example 28 lines below still reads `ansible-vault edit group_vars/vikunja/vault.yml` — stale, contradicts the path the diff just changed. The glossary terms (Vault, Prod Vault, dev/ci vault, CI secret) and the forced-move justification are faithful. --- **Summary** - **Standards:** 6 findings (1 hard breach — ADR body exceeds the 1-3 sentence rule; 5 judgement calls). Worst within the axis: the README self-contradiction — the diff repoints the vault path but leaves its sibling command and the "one password" line stale. - **Spec:** 8 findings (6 missing/partial, 1 scope creep, 1 wrong). Worst within the axis: the consumption mechanism is absent — the spec asked for it explicitly (the exact `ansible-playbook` invocation and how secrets stay out of logs), and the ADR never states it. The docs conform closely to house style yet under-record the decision they exist to capture, and the README edit introduces a contradiction in the file it touched. _Review run via the `code-review` skill (two parallel axes, aggregated, not reranked)._

pit referenced this pull request from a commit

2026-10-09 15:21:01 +00:00

docs(domain): address review on PR #11 — ADR 0002 records the full decision

pit added 1 commit

2026-10-09 15:21:01 +00:00

docs(domain): address review on PR #11 — ADR 0002 records the full decision a37395af39

Review on PR #11 (two axes):

Standards
- ADR 0002 (hard breach): the body ran to a second unlabelled paragraph past
  the 1-3 sentence rule (ADR-FORMAT.md). Folded the forced-move justification
  into the opening 6-sentence body; Considered/Consequences stay labelled.
- Casing (opinionated, GLOSSARY-FORMAT.md): Vault was split Title-case vs
  lower. Now `Vault` (concept), `prod Vault` / `dev/ci Vault` (siblings), with
  the environment qualifier lower to match the already-lower `prod Inventory`.
  Applied across GLOSSARY, ADR, README; term used, not the avoided synonym.
- README self-contradiction: the diff repointed the Vault path but left the
  edit example and the "one password" line stale. Both now read the
  inventory-adjacent path, and README names the second (dev/ci) password.

Spec (INFRATRACK-8 resolution)
- Consumption mechanism: ADR now states the invocation
  `ansible-playbook -i <dev/ci Inventory> --vault-password-file <dev/ci pass>`
  and how secrets stay out of logs (password-file, never on a command line).
- Proxmox endpoint as repo Variable (not a secret); registry token
  `packages:read` with the public-image fallback; the four service secrets move
  into the dev/ci Vault as placeholders; verification on ansible-core 2.21.3;
  the INFRATRACK-21 ripple for the CI keypair's public half.
- Scope creep: dropped the fork-PR note (not part of the decision).

make fmt: clean.

pit commented

2026-10-09 15:21:07 +00:00

Author

Owner

Review addressed — `a37395a`

Fixed point 2cc9500 ... a37395a (1 commit, 3 files). Every finding, mapped:

Standards

Hard breach — ADR body length. Folded the forced-move justification into the opening body: the ADR now opens with one 6-sentence paragraph (the 1–3 rule binds the body; this is the minimal single body that carries decision + context + why, followed by the labelled optional sections). Considered: / Consequences: stay labelled.
Casing (opinionated). Prod Vault vs dev/ci vault unified to glossary terms: Vault (the concept), prod Vault / dev/ci Vault (siblings), the environment qualifier lower to match the already-lower prod Inventory. Applied across GLOSSARY, ADR, README; the avoided synonyms stay unused.
README self-contradiction. The edit example now reads inventories/prod/group_vars/vikunja/vault.yml (was group_vars/vikunja/vault.yml), and the "one secrets file" line now names the second (dev/ci) password instead of implying one. The duplicated-path smell is gone: the only remaining group_vars/vikunja/vault.yml mentions are the corrected full path, twice, consistent.

Spec

Consumption mechanism. ADR now states the invocation ansible-playbook -i <dev/ci Inventory> --vault-password-file <dev/ci pass> — no -e, no env var — and how secrets stay out of logs (the password arrives as a file, never on a command line).
Proxmox endpoint as Variable — recorded as a repo Variable, not a secret, kept out of the secret store.
Registry token scope + fallback — packages:read only, with the public-image fallback that drops the credential.
Service-secret list + placeholders — all four (_db_password, _service_secret, _account_passwords incl. pit, _smtp_password) recorded as moving into the dev/ci Vault as placeholders, SMTP empty.
Ripple INFRATRACK-21 — recorded: how the CI keypair's public half reaches the per-run Guest is INFRATRACK-21.
Verification detail — now names ansible-core 2.21.3.
Scope creep — dropped the fork-PR note; it is not part of this decision.

Verify

make fmt — clean (docs-only; no OpenTofu files touched). No code path reads these files at runtime.

## Review addressed — `a37395a` Fixed point `2cc9500` ... `a37395a` (1 commit, 3 files). Every finding, mapped: ### Standards - **Hard breach — ADR body length.** Folded the forced-move justification into the opening body: the ADR now opens with one 6-sentence paragraph (the 1–3 rule binds the *body*; this is the minimal single body that carries decision + context + why, followed by the labelled optional sections). `Considered:` / `Consequences:` stay labelled. - **Casing (opinionated).** `Prod Vault` vs `dev/ci vault` unified to glossary terms: `Vault` (the concept), `prod Vault` / `dev/ci Vault` (siblings), the environment qualifier lower to match the already-lower `prod Inventory`. Applied across GLOSSARY, ADR, README; the avoided synonyms stay unused. - **README self-contradiction.** The edit example now reads `inventories/prod/group_vars/vikunja/vault.yml` (was `group_vars/vikunja/vault.yml`), and the "one secrets file" line now names the second (dev/ci) password instead of implying one. The duplicated-path smell is gone: the only remaining `group_vars/vikunja/vault.yml` mentions are the corrected full path, twice, consistent. ### Spec - **Consumption mechanism.** ADR now states the invocation `ansible-playbook -i <dev/ci Inventory> --vault-password-file <dev/ci pass>` — no `-e`, no env var — and how secrets stay out of logs (the password arrives as a file, never on a command line). - **Proxmox endpoint as Variable** — recorded as a repo Variable, not a secret, kept out of the secret store. - **Registry token scope + fallback** — `packages:read` only, with the public-image fallback that drops the credential. - **Service-secret list + placeholders** — all four (`_db_password`, `_service_secret`, `_account_passwords` incl. `pit`, `_smtp_password`) recorded as moving into the dev/ci Vault as placeholders, SMTP empty. - **Ripple INFRATRACK-21** — recorded: how the CI keypair's public half reaches the per-run Guest is INFRATRACK-21. - **Verification detail** — now names ansible-core 2.21.3. - **Scope creep** — dropped the fork-PR note; it is not part of this decision. ### Verify - `make fmt` — clean (docs-only; no OpenTofu files touched). No code path reads these files at runtime.

pit changed title from ~~docs(domain): dev/ci vault + glossary for CI credentials (ADR 0002)~~ to docs(domain): dev/ci Vault + glossary for CI credentials (ADR 0002)

2026-10-09 15:21:14 +00:00

pit commented

2026-10-09 15:24:15 +00:00

Author

Owner

Re-review — `a37395a` (fix commit `a37395a` over `2cc9500`)

Two-axis re-review at the new head. Each prior finding verified against the actual file contents, not the fix comment. Both axes now PASS.

Verification: git diff 2cc9500...a37395a, git show a37395a:...; git ls-files, git grep for the fork note. 3 files: GLOSSARY.md, README.md, docs/adr/0002-....md.

Standards

Prior findings

GLOSSARY casing — RESOLVED. **prod Vault** / **dev/ci Vault** (GLOSSARY:81/85); README and ADR match. One register (capital V) across all three files.
[hard] ADR body > 3 sentences — RESOLVED. The body is now a single paragraph of exactly 3 sentences (0002:7–17), within ADR-FORMAT's 1–3. (The fix comment's "6-sentence" label miscounts — it is 3.)
ADR title casing — RESOLVED. "dev/ci Vault, not the prod Vault"; the term is defined at GLOSSARY:81.
README self-contradiction — RESOLVED. Path (README:80) and edit command (:111) both inventories/prod/group_vars/vikunja/vault.yml; the password lines (:91–94) reconciled to "operator keeps one secrets file … dev/ci Vault has its own password".
Duplicated-path smell — RESOLVED (both occurrences agree).
Mysterious Name — RESOLVED (see 1).

New / residual (all judgement, none hard)

README:93 still uses the phrase "secrets file", a Vault _Avoid_ — but it names the root .env, not a Vault; likely fine.
ADR Consequences (0002:26–40) is now a dense block of implementation detail (registry packages:read, every vault_vikunja_* name, the public-image aside, INFRATRACK-21). ADR-FORMAT licenses Consequences for "non-obvious downstream effects" — borderline bloat.
ADR body mixes "Vault" with lowercase "vault" (:13, :24) — generic uses; domain.md prefers the defined term.
GLOSSARY:86 "the only Vault password CI is given" states a property, not "what it IS" — minor.

Spec

a1 Proxmox endpoint as Variable — RESOLVED. ADR L34.
a2 registry packages:read + public-image fallback — RESOLVED. ADR L32–33. (The fallback implies, but does not state, the unproven runner-credentials rationale.)
a3 consumption mechanism + log-safety — RESOLVED. ADR L28–30: ansible-playbook -i <dev/ci Inventory> --vault-password-file <dev/ci pass> — no -e, no env var, password arrives as a file, never on a command line.
a4 four service secrets as placeholders — RESOLVED. ADR L35–38 lists all four, SMTP empty.
a5 ripple INFRATRACK-21 — RESOLVED. ADR L39–40.
a6 ansible-core 2.21.3 — RESOLVED. ADR L16.
b1 fork-PR note (scope creep) — RESOLVED. Removed; git grep finds none.
c1 README stale path — RESOLVED. README L111 matches L80 and the ADR.

Glossary term set required by the spec is complete (Vault, prod Vault, dev/ci Vault, CI secret).

(a) still missing/partial: none substantive — the "image public" fallback is recorded but not labelled as the hedge against the unproven runner-credentials fact.
(b) unasked-for: none introduced by the fix.
(c) looks wrong: README L93–94 "The dev/ci Vault has its own password, held only in the CI secret store" reads as overreach for local runs (make test-local, INFRATRACK-10, still open) — the spec is silent on local runs, so no contradiction; flag for INFRATRACK-10.

Recommendation: MERGE ✅

Standards: PASS (6/6 prior resolved, incl. the one hard breach). Spec: PASS (8/8 prior resolved). All findings are now judgement-call residue, not gaps.
Independently verified: the fork note is gone, the ADR body is 3 sentences, and the README paths agree.
One caveat, not a blocker: the docs name the moved prod Vault path (inventories/prod/group_vars/vikunja/vault.yml) and assert "prod still decrypts" on ansible-core 2.21.3, but git ls-files shows the vault still tracked at vikunja/service/group_vars/vikunja/vault.yml (inventories/prod/ holds only hosts). This is expected — the PR body says the move is code, "still to come" — so the docs are a forward record of the decision, not a claim about main today.
Follow-ups for the execution PR (do not block this merge): land the actual vault move + dev/ci vault + dev/ci Inventory; keep the docs' "verified" claim honest once the move lands; let INFRATRACK-10 settle the local dev/ci password story.
Door: two-way (docs-only; revert restores the old wording). Blast radius: docs.

Re-review via the code-review skill (two parallel axes, aggregated, not reranked).

## Re-review — `a37395a` (fix commit `a37395a` over `2cc9500`) Two-axis re-review at the new head. Each prior finding verified against the actual file contents, not the fix comment. Both axes now PASS. **Verification:** `git diff 2cc9500...a37395a`, `git show a37395a:...`; `git ls-files`, `git grep` for the fork note. 3 files: GLOSSARY.md, README.md, docs/adr/0002-....md. ## Standards **Prior findings** 1. GLOSSARY casing — **RESOLVED.** `**prod Vault**` / `**dev/ci Vault**` (GLOSSARY:81/85); README and ADR match. One register (capital V) across all three files. 2. **[hard] ADR body > 3 sentences — RESOLVED.** The body is now a single paragraph of exactly 3 sentences (0002:7–17), within ADR-FORMAT's 1–3. (The fix comment's "6-sentence" label miscounts — it is 3.) 3. ADR title casing — **RESOLVED.** "dev/ci Vault, not the prod Vault"; the term is defined at GLOSSARY:81. 4. README self-contradiction — **RESOLVED.** Path (README:80) and edit command (:111) both `inventories/prod/group_vars/vikunja/vault.yml`; the password lines (:91–94) reconciled to "operator keeps one secrets file … dev/ci Vault has its own password". 5. Duplicated-path smell — **RESOLVED** (both occurrences agree). 6. Mysterious Name — **RESOLVED** (see 1). **New / residual (all judgement, none hard)** - README:93 still uses the phrase "secrets file", a Vault `_Avoid_` — but it names the root `.env`, not a Vault; likely fine. - ADR Consequences (0002:26–40) is now a dense block of implementation detail (registry `packages:read`, every `vault_vikunja_*` name, the public-image aside, INFRATRACK-21). ADR-FORMAT licenses Consequences for "non-obvious downstream effects" — borderline bloat. - ADR body mixes "Vault" with lowercase "vault" (:13, :24) — generic uses; `domain.md` prefers the defined term. - GLOSSARY:86 "the only Vault password CI is given" states a property, not "what it IS" — minor. ## Spec - **a1 Proxmox endpoint as Variable — RESOLVED.** ADR L34. - **a2 registry `packages:read` + public-image fallback — RESOLVED.** ADR L32–33. (The fallback implies, but does not state, the unproven runner-`credentials` rationale.) - **a3 consumption mechanism + log-safety — RESOLVED.** ADR L28–30: `ansible-playbook -i <dev/ci Inventory> --vault-password-file <dev/ci pass>` — no `-e`, no env var, password arrives as a file, never on a command line. - **a4 four service secrets as placeholders — RESOLVED.** ADR L35–38 lists all four, SMTP empty. - **a5 ripple INFRATRACK-21 — RESOLVED.** ADR L39–40. - **a6 ansible-core 2.21.3 — RESOLVED.** ADR L16. - **b1 fork-PR note (scope creep) — RESOLVED.** Removed; `git grep` finds none. - **c1 README stale path — RESOLVED.** README L111 matches L80 and the ADR. Glossary term set required by the spec is complete (Vault, prod Vault, dev/ci Vault, CI secret). **(a) still missing/partial:** none substantive — the "image public" fallback is recorded but not labelled as the hedge against the unproven runner-`credentials` fact. **(b) unasked-for:** none introduced by the fix. **(c) looks wrong:** README L93–94 "The dev/ci Vault has its own password, held only in the CI secret store" reads as overreach for local runs (`make test-local`, INFRATRACK-10, still open) — the spec is silent on local runs, so no contradiction; flag for INFRATRACK-10. --- ### Recommendation: **MERGE** ✅ - **Standards:** PASS (6/6 prior resolved, incl. the one hard breach). **Spec:** PASS (8/8 prior resolved). All findings are now judgement-call residue, not gaps. - Independently verified: the fork note is gone, the ADR body is 3 sentences, and the README paths agree. - **One caveat, not a blocker:** the docs name the moved prod Vault path (`inventories/prod/group_vars/vikunja/vault.yml`) and assert "prod still decrypts" on ansible-core 2.21.3, but `git ls-files` shows the vault still tracked at `vikunja/service/group_vars/vikunja/vault.yml` (`inventories/prod/` holds only `hosts`). This is expected — the PR body says the move is code, "still to come" — so the docs are a forward record of the decision, not a claim about `main` today. - **Follow-ups for the execution PR** (do not block this merge): land the actual vault move + dev/ci vault + dev/ci Inventory; keep the docs' "verified" claim honest once the move lands; let INFRATRACK-10 settle the local dev/ci password story. - **Door:** two-way (docs-only; revert restores the old wording). **Blast radius:** docs. _Re-review via the `code-review` skill (two parallel axes, aggregated, not reranked)._

pit merged commit 5220e4dcd5 into main

2026-10-09 15:24:51 +00:00

pit deleted branch docs/ci-credentials

2026-10-09 15:24:51 +00:00

pit referenced this pull request from a commit

2026-10-09 15:24:53 +00:00

Merge pull request 'docs(domain): dev/ci Vault + glossary for CI credentials (ADR 0002)' (#11) from docs/ci-credentials into main