645f81a443613a0686a811d91a088766521a4414
59 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
645f81a443 |
fix(setup): mirror nvm init into .zshrc so zsh sees node + pnpm (#255)
## Summary Fix the dev-VM bootstrap so `node` and `pnpm` are on `PATH` in zsh sessions. On a fresh VM provisioned with `docs/setup/scripts/`, a freshly-cloned checkout fails to run anything (`command not found: pnpm`, and `node` too) because the nvm init never reaches the interactive shell. ## Root cause Two setup scripts interact badly: - `20-zsh.sh` patches `~/.bashrc` with an `exec zsh -l` hand-off on interactive shells (chsh is blocked on the corp VM, so this is how zsh becomes the effective shell), and patches `~/.zshrc` with theme/plugins/fzf only. - `40-node.sh` runs the upstream nvm installer, which appends its init block to **`~/.bashrc`** (its default target). Because `20-zsh.sh` runs first, the `exec zsh -l` guard sits **above** the nvm block in `~/.bashrc`. On every interactive shell, bash execs into zsh before reaching the nvm block — so it never runs — and `~/.zshrc` has no nvm init at all. Net result: zsh sessions have neither `node` nor `pnpm` on `PATH`, even though nvm + Node + corepack installed correctly. This is a procedure bug, not a one-off: every VM built from these scripts hits it. ## Fix `40-node.sh` now mirrors the nvm init block into `~/.zshrc` after enabling corepack — idempotent via a managed marker, same pattern `20-zsh.sh` already uses for its `~/.bashrc` patch: ```sh # Managed by docs/setup/scripts/40-node.sh — nvm init for zsh. export NVM_DIR="$HOME/.nvm" [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" ``` `docs/setup/01-dev-debian-vm-setup.md` — the `40-node.sh` row in the bootstrap table now notes the `~/.zshrc` patch and why it is needed. ## What lands | File | Change | | --- | --- | | `docs/setup/scripts/40-node.sh` | Append an idempotent nvm-init block to `~/.zshrc` after corepack enable. | | `docs/setup/01-dev-debian-vm-setup.md` | Document the `~/.zshrc` nvm patch in the `40-node.sh` table row. | ## Manual remediation for already-provisioned VMs VMs built before this fix won't be retroactively patched (the scripts are idempotent and skip already-installed nvm). On those, run once: ```sh cat >> ~/.zshrc <<'EOF' # nvm export NVM_DIR="$HOME/.nvm" [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" EOF exec zsh -l ``` (Or re-run `40-node.sh` once this lands — the marker check makes it safe; it will add the block and skip the rest.) ## Test plan - [x] `bash -n docs/setup/scripts/40-node.sh` — syntax valid. - [x] Manual remediation block verified on the affected VM: after appending + `exec zsh -l`, `node --version` (v24) and `pnpm --version` (10.34.1) both resolve. - [ ] Next fresh VM bootstrap: `node` + `pnpm` resolve in the first zsh session with no manual step. - [ ] Re-running `40-node.sh` on an already-set-up VM is a no-op on the nvm install and adds the `~/.zshrc` block exactly once (marker idempotency). ## Related - `docs/setup/01-dev-debian-vm-setup.md` — dev-VM bootstrap procedure. - `20-zsh.sh` — owns the `.bashrc` → zsh hand-off this fix complements. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #255 |
||
|
|
670f6303fe |
docs(adr-0028): accept CI/CD + git hosting migration to GitLab (#227)
## Summary Promotes [ADR-0028](docs/decisions/0028-migrate-cicd-and-git-hosting-to-gitlab.md) from `proposed` to `accepted`. Shipped as `proposed` in [#226](#226); no open question left on either drivers, considered options, or the 4-phase migration sequence. Same shape as [#219](#219) (ADR-0026 + ADR-0027 acceptance). Once merged, **Phase 1** of the migration (`mirror-and-bootstrap` — repos pushed to GitLab, branch protection / MR templates / deploy keys / Renovate reconfig) is unblocked. ## What lands | File | Change | | --- | --- | | `docs/decisions/0028-migrate-cicd-and-git-hosting-to-gitlab.md` | Frontmatter `status: proposed → accepted`. | | `docs/decisions/0015-cicd-gitea-actions.md` | **Status-update note** added at the top, right under the title — calls out that the platform choice "Gitea Actions" is superseded by ADR-0028, while the rest of ADR-0015's architectural principles (trunk-based + squash, all-gates-blocking, thin YAML, on-prem runners, signed commits, Conventional Commits) carry over unchanged. ADR-0015 stays `accepted`. | | `docs/decisions/README.md` | ADR-0028 row: `proposed → accepted`. | | `CLAUDE.md` | Roll-up bumped to `ADRs 0001 → 0028 accepted` with a one-sentence explanation that ADR-0028 supersedes only ADR-0015's platform choice. **CI/CD architecture bullet rewritten** to reflect the accepted-but-not-yet-implemented state — "Gitea Actions today, migrating to GitLab CE on `vm-gitlab` per ADR-0028 (4-phase rollout in follow-up PRs)". The architectural detail (gates list, thin YAML, signed commits) was preserved; only the platform headline and the act_runner→GitLab Runner line are touched. Also corrected: `ci:scan` (which doesn't exist as a script) → the real script names (`ci:catalogue-drift`, `ci:audit`, `ci:perf`, `ci:gzip-budgets`). | ## Notes for the reviewer - **No new Architecture bullet for ADR-0028 itself.** The decision is a *platform shift* for the existing CI/CD architecture, not a new architectural concern — so it folds into the existing ADR-0015 bullet rather than adding a sibling. - **The annotation pattern on ADR-0015** (status-update blockquote at the top) is the canonical MADR way to handle partial supersession without changing the frontmatter status. The architectural principles are still accepted; only the platform implementation moves. A future reader hitting ADR-0015 first sees the redirect immediately. - **The CLAUDE.md script-list correction** is a side-fix — `ci:scan` is not a real script name; the actual gates are `ci:check`, `ci:catalogue-drift`, `ci:audit`, `ci:commits`, `ci:perf`, `ci:gzip-budgets`. Updated in the same touch since the bullet was being rewritten anyway. - **No code changes**, so no `pnpm ci:check` impact. `pnpm exec prettier --check` clean on the four touched files. ## Test plan - [x] `pnpm exec prettier --check` — clean on the four touched files. - [x] ADR-0015 → ADR-0028 cross-reference resolves (the new blockquote link). - [x] `ADRs 0001 → 0028 accepted` matches reality (`grep '^status: ' docs/decisions/*.md` shows everything below 0029 as `accepted`). - [ ] **Review focus** — the ADR-0015 status-update note phrasing, the CLAUDE.md CI/CD bullet rewrite (especially the "carry over" wording), the roll-up sentence about partial supersession. ## What's next (post-merge) 1. **Phase 1 — `mirror-and-bootstrap`** — `git push --mirror gitlab` for `apf_portal` and the proto vendoring in `apf-ai-service`. GitLab side: groups, projects, branch protection (mirror Gitea's), MR templates, deploy keys, Renovate reconfigured for GitLab. **No `.gitlab-ci.yml` yet** — Gitea pipelines continue to gate. Ops work primarily; the only PR-shaped output is a Renovate config update on the apf-portal repo if its host detection changes. 2. **Phase 2 — `gitlab-ci-pipeline`** — `.gitlab-ci.yml` lands alongside `.gitea/workflows/ci.yml`. Both pipelines run in parallel for ~1 calendar week. GitLab Runner registered on `vm-gitlab`. 3. **Phase 3 — `cutover`** — remotes flip in CLAUDE.md, READMEs, `docs/setup/01-dev-debian-vm-setup.md` §8.3. `.gitea/workflows/` + `infra/ci-runners.compose.yml` deleted, Gitea read-only / archive. 4. **Phase 4 — `cleanup`** — stale references sweep, required signed-commits on `main` enabled. In parallel — once you've finished walking through the dev VM bootstrap — the paused **ADR-0027 PR 1** (Region / Delegation / Structure Prisma schema + inline seed) and **ADR-0026 PR 1** (Person / User / UserScope schema + provisioner) can ship on Gitea; they're decoupled from the migration and don't need to wait for it. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #227 |
||
|
|
04ee535de9 |
docs(adr-0028): propose CI/CD + git hosting migration Gitea -> GitLab (#226)
## Summary
Drafts [ADR-0028](docs/decisions/0028-migrate-cicd-and-git-hosting-to-gitlab.md) as `proposed`: migrate CI/CD + git hosting from Gitea (`git.unespace.com`) to GitLab CE self-hosted on `vm-gitlab` (`10.100.201.10`). The migration was anticipated by [ADR-0015](docs/decisions/0015-cicd-gitea-actions.md) ("level-2 implementation; will be superseded by a GitLab migration ADR within 6-18 months") — that window opens now. **Decision-only PR** — the actual 4-phase migration ships across follow-up PRs after acceptance.
ADR-0028's number was previously a placeholder reference in ADR-0026 and ADR-0027 for the Pléiades + Acteurs+ sync ADR. **Renumbering**: that future sync ADR shifts to `ADR-0029`, and the placeholder links in ADR-0026, ADR-0027 and `CLAUDE.md` update to match — included in the same PR so the chain stays consistent.
## What lands
| File | Change |
| --- | --- |
| `docs/decisions/0028-migrate-cicd-and-git-hosting-to-gitlab.md` | **New.** MADR 4.0.0 ADR, `proposed`. Decision = Option B (GitLab CE on `vm-gitlab`). Considered options A (status quo Gitea), C (Forgejo), D (cloud SaaS). Documents what carries over from ADR-0015 (architectural principles unchanged — thin YAML, trunk-based, all-gates-blocking, on-prem runners), what changes (host, pipeline file, runner type, scan tooling), the 4-phase migration sequence, and the signed-commits revisit. |
| `docs/decisions/0026-person-user-portal-data-model.md` | All 11 `ADR-0028` references → `ADR-0029` (sync + facets shifts to 0029). |
| `docs/decisions/0027-portal-side-organisational-hierarchy.md` | All 14 `ADR-0028` references → `ADR-0029`. |
| `docs/decisions/README.md` | New row for ADR-0028 (`proposed`, tags `infrastructure`, `process`, 2026-05-26). |
| `CLAUDE.md` | Roll-up updated: `ADRs 0001 → 0027 accepted; ADR-0028 + ADR-0029 proposed`. ADR-0028's relationship to ADR-0015 spelled out inline ("supersedes ADR-0015's Gitea Actions platform choice — the rest of ADR-0015's architectural principles carry over unchanged"). ADR-0026 + ADR-0027 architecture bullets renumbered 0028 → 0029 to track. |
## Key choices in the ADR
- **What carries over from ADR-0015 vs what changes** — explicit table so future readers see immediately that the migration is **platform-only**, not a re-litigation of CI principles. Trunk-based + squash, all-gates-blocking, thin YAML over portable scripts (`pnpm ci:check` etc. — unchanged), on-prem runners, Conventional Commits in CI + hook (defense in depth) — all carried over. Host, pipeline-file grammar, runner type, and scan tooling are the only things that move.
- **Native security scanning replaces the manual Trivy + gitleaks setup.** GitLab CE's built-in `Dependency-Scanning.gitlab-ci.yml` + `Secret-Detection.gitlab-ci.yml` includes consolidate the ~30 lines of inline `curl + tar` install dance currently in `.gitea/workflows/ci.yml`. Same blocking thresholds (CRITICAL+HIGH dependency vulns, any secret).
- **`vm-gitlab` is already provisioned.** No infra wait — the only sequencing constraint is operator-driven, not infrastructure-driven.
- **4-phase migration, parallel pipelines for ~1 week before cutover.** Phase 1 mirrors repos and bootstraps GitLab side (no `.gitlab-ci.yml` yet — Gitea still gates). Phase 2 lands `.gitlab-ci.yml` alongside `.gitea/workflows/ci.yml` so both pipelines run per PR until parity is confirmed. Phase 3 flips the remote URLs and deletes `.gitea/workflows/` + `infra/ci-runners.compose.yml`. Phase 4 sweeps stale references.
- **Gitea moves to read-only / archive, not decommissioned** at cutover. Existing references to Gitea PRs (`#213`, `#217`, `#219`, …) in commit messages and ADR bodies stay resolvable as historical artefacts. One VM at idle is a low long-term cost.
- **Signed commits revisit.** ADR-0015 noted "signed commits recommended, revisited at GitLab migration". ADR-0028 makes the recommendation: enable required signed commits on `main` once GitLab is live, paired with GnuPG agent forwarding (already documented in `docs/setup/01-dev-debian-vm-setup.md` §8.5). `apf-portal-bot` (Renovate) gets a dedicated signing key at PR 1.
## Renumbering — what moved and why
Before this PR, ADR-0026 and ADR-0027 used `ADR-0028` as a placeholder link for the Pléiades + Acteurs+ sync ADR. That sync ADR hasn't been drafted yet — the number was reserved.
This PR claims `ADR-0028` for the GitLab migration (the immediately-actionable decision), and shifts the sync placeholder to **ADR-0029**. All 25 link references across ADR-0026 (11) and ADR-0027 (14) update in lockstep — replace_all is safe here because in those files `ADR-0028` consistently meant "the sync ADR".
Content of the sync ADR is unchanged — only the number. When that ADR is eventually drafted as `0029-…md`, it gets the existing content reserved for it in the placeholder text.
## Test plan
- [x] `pnpm exec prettier --check` clean on the touched files.
- [x] All `ADR-0028` references in `0026-…md` / `0027-…md` / `CLAUDE.md` now read `ADR-0029` (grep confirms zero remaining references to the old number in those files).
- [x] The new `0028-…md` self-references (status frontmatter, title, internal anchors) are consistent — no leftover `0029`.
- [ ] **Review focus** — drivers / consequences / migration sequence in the ADR; the "what carries over from ADR-0015" table; the renumbering rationale.
## What's next
Per ADR-0028 §"Migration sequence", post-acceptance:
1. **ADR-0028 acceptance PR** — small status-flip, same pattern as #219 (ADR-0026 + ADR-0027 acceptance).
2. **`mirror-and-bootstrap` PR** — `git push --mirror` Gitea → GitLab; GitLab side groups / projects / branch protection / MR templates / deploy keys / Renovate reconfig. No `.gitlab-ci.yml` yet, Gitea pipelines still gate.
3. **`gitlab-ci-pipeline` PR** — `.gitlab-ci.yml` alongside the existing `.gitea/workflows/ci.yml`. Parallel runs ~1 week for parity. GitLab Runner registered on `vm-gitlab`.
4. **`cutover` PR** — remotes flip across docs, `.gitea/workflows/` + `infra/ci-runners.compose.yml` deleted, Gitea read-only.
5. **`cleanup` PR** — stale references sweep, signed-commit policy finalised on `main`.
In parallel — once the dev VM (#220 / #221 / #222 / #223 / #224) is fully bootstrapped — the paused **ADR-0027 PR 1** (Region / Delegation / Structure Prisma schema + inline seed) and **ADR-0026 PR 1** (Person / User / UserScope schema + provisioner) can ship on Gitea; they have no dependency on the GitLab migration and don't need to wait.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #226
|
||
|
|
72e1182308 |
docs(setup): version aliases.zsh + gitconfig.txt under docs/setup/dotfiles (#224)
## Summary Follow-up on [#220](#220). [`80-dotfiles.sh`](docs/setup/scripts/80-dotfiles.sh) was reading `notes/aliases.zsh` and `notes/gitconfig.txt` — but `notes/` is the project lead's personal scratchpad and is gitignored, so a fresh clone on the dev VM has nothing to read: ``` ✗ /home/APF/dev-jugautier/Works/apf_portal/notes/aliases.zsh not found. ``` Move both templates to a versioned location and update the script + docs to match. ## What lands | File | Change | | --- | --- | | `docs/setup/dotfiles/aliases.zsh` | **New.** Project-wide zsh aliases (navigation / files / search / git / dev), previously living in `notes/aliases.zsh`. Header explains scope (project-wide, all devs pick it up on next zsh restart) and where per-dev customizations go (private dotfiles repo). | | `docs/setup/dotfiles/gitconfig.txt` | **New.** Base `~/.gitconfig` template — init / core / aliases / colour. `[user]` block carries placeholder identity (`name = your name` / `email = your.email@example.com`) that the script overwrites at install via `git config --global user.{name,email}`. | | `docs/setup/scripts/80-dotfiles.sh` | Source paths flipped from `$REPO_ROOT/notes/…` to `$REPO_ROOT/docs/setup/dotfiles/…`. Header doc-comment updated. | | `docs/setup/01-dev-debian-vm-setup.md` | Step-3 table row and §8.4 (dotfiles repo) reference the new path. The §8.4 wording is also tightened — the script no longer "falls back" anywhere, it has one source of truth. | | `docs/setup/README.md` | New `dotfiles/` section in the folder index. Step-7 row in the scripts table also reflects the new paths. | ## Why this lives in `docs/setup/dotfiles/` and not `infra/` or a top-level `dotfiles/` - The directory is **consumed exclusively by [`80-dotfiles.sh`](docs/setup/scripts/80-dotfiles.sh)** — co-locating it under `docs/setup/` keeps the setup family self-contained (one folder to read, one folder to clone). - The name `dotfiles/` reads correctly for a future per-dev dotfiles repo migration (§8.4) — the templates here become the seed of that repo, and the install script will then check `~/.dotfiles/` first and fall back to `docs/setup/dotfiles/` second. ## Unblock path (if you're stuck mid-bootstrap) Either pull this PR, or manually create the two files at the **new** location on the VM: ```bash mkdir -p ~/Works/apf_portal/docs/setup/dotfiles # paste the aliases.zsh + gitconfig.txt content there ./docs/setup/scripts/80-dotfiles.sh ``` ## Test plan - [ ] On a fresh Trixie VM, `./docs/setup/scripts/80-dotfiles.sh` succeeds: aliases.zsh symlink lands at `~/.oh-my-zsh/custom/aliases.zsh` pointing at `docs/setup/dotfiles/aliases.zsh`, `~/.gitconfig` written with the prompted identity. - [ ] Re-running the script reports `↪ skip aliases.zsh symlink already correct` (idempotency). - [ ] `git config --global user.name` returns the prompted value (i.e. placeholder `your name` is overwritten). - [x] `pnpm exec prettier --check` clean on the touched files. - [x] `bash -n docs/setup/scripts/80-dotfiles.sh` clean. ## Notes - `notes/aliases.zsh` and `notes/gitconfig.txt` on existing dev workstations are unaffected — those files live outside the repo (gitignored), nothing here touches them. They can be deleted or kept as personal scratch at the dev's discretion. - A future PR creates the **private dotfiles repo** (`apf/dotfiles`) and teaches `80-dotfiles.sh` to prefer `~/.dotfiles/` over `docs/setup/dotfiles/` when both exist. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #224 |
||
|
|
1d11db9f36 |
docs(setup): drop deprecated apt packages on Debian 13 trixie (#223)
## Summary Follow-up on [#220](#220). [`10-base-packages.sh`](docs/setup/scripts/10-base-packages.sh) fails on a fresh Debian 13 Trixie VM with `E: Impossible de trouver le paquet software-properties-common`. The package is no longer shipped on Trixie — and it was never used by any of our downstream scripts in the first place. Drop three deprecated / unused packages from the install list. Add an inline comment explaining each kept package's purpose so a drive-by addition doesn't reintroduce the dropped ones. ## What lands | File | Change | | --- | --- | | `docs/setup/scripts/10-base-packages.sh` | Drop `software-properties-common`, `apt-transport-https`, `lsb-release`. Remaining minimal set: `curl wget git ca-certificates gnupg build-essential pkg-config`. Inline comment lists which downstream script consumes each kept package + which three were deliberately removed and why. | | `docs/setup/01-dev-debian-vm-setup.md` | Step-3 table row for `10-base-packages.sh` updated to match the new package list. | ## Why each removed package wasn't needed | Package | Why removed | | --- | --- | | `software-properties-common` | Drops `add-apt-repository`. We don't use it — [`50-docker.sh`](docs/setup/scripts/50-docker.sh) writes `/etc/apt/sources.list.d/docker.list` manually with the keyring-pinned `signed-by=` form. Also no longer in Trixie. | | `apt-transport-https` | Transitional package since apt 1.5 (2018) — apt has native HTTPS. Intermittently absent on Trixie. | | `lsb-release` | Shell scripts read `/etc/os-release` directly (see `50-docker.sh`'s `. /etc/os-release && echo "${VERSION_CODENAME}"`). | ## Unblock path (manual, if you're stuck mid-bootstrap) The user can either pull this PR and re-run, or shortcut manually: ```bash sudo apt-get install -y curl wget git ca-certificates gnupg build-essential pkg-config ./docs/setup/scripts/20-zsh.sh # continue from the next step ``` Scripts are idempotent — re-running `bootstrap.sh` after pulling this PR is also safe. ## Test plan - [ ] On a fresh Trixie VM, `./docs/setup/scripts/10-base-packages.sh` exits successfully. - [ ] Verify each downstream script still has the binaries it expects (curl in 40-node.sh, gpg in 50-docker.sh, build-essential in pnpm install, …). - [x] `pnpm exec prettier --check` clean on the touched files. - [x] `bash -n docs/setup/scripts/10-base-packages.sh` clean. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #223 |
||
|
|
d99254a280 |
docs(setup): make fail2ban opt-in in 70-hardening.sh (#222)
## Summary Follow-up on [#220](#220) / [#221](#221). Makes fail2ban **opt-in** in [`70-hardening.sh`](docs/setup/scripts/70-hardening.sh) instead of installing it unconditionally. Reasoning: some corp environments already ship brute-force protection at the network layer (ACL / corp firewall / appliance) — fail2ban on the host then becomes redundant and can be the wrong layer to debug from when a rule misfires. The other three hardening steps (UFW enable, sshd lockdown) were already prompt-gated; fail2ban was the odd one out. ## What lands | File | Change | | --- | --- | | `docs/setup/scripts/70-hardening.sh` | fail2ban block restructured into three branches: (1) already running → skip; (2) installed but stopped → prompt to enable+start; (3) not installed → prompt to install+enable+start. Each "no" path logs `↪ skip (user choice)` so re-runs don't repeatedly nag if the dev has already declined. | | `docs/setup/01-dev-debian-vm-setup.md` | Table row for `70-hardening.sh` clarified — each sub-step's prompt posture is now visible: UFW prompts before enabling, fail2ban prompts before installing, sshd hardening prompts before applying. `unattended-upgrades` is the only one applied unconditionally. | | `docs/setup/README.md` | Same descriptor adjustment. | ## Test plan - [ ] On a fresh Debian VM with no fail2ban installed, run `70-hardening.sh`, decline the fail2ban prompt → script continues, fail2ban not installed, no service started. - [ ] On the same VM, re-run `70-hardening.sh` → the fail2ban branch prompts again (the dev may have changed their mind); declining again produces the same `↪ skip (user choice)` result. - [ ] On a VM where fail2ban is pre-installed but stopped (rare, but possible if infra rolled it back), the script offers to start it without re-installing. - [x] `pnpm exec prettier --check` clean on the touched files. - [x] `bash -n docs/setup/scripts/70-hardening.sh` (syntax check) clean. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #222 |
||
|
|
c77313c693 |
docs(setup): use ~/.bashrc hand-off instead of chsh for zsh switch (#221)
## Summary Follow-up fix on [#220](#220). The corp infra locks the default shell at user-provisioning time on the dev VM (`10.100.201.21`) — `chsh` is denied at the PAM level, so the `sudo chsh -s` block in [`20-zsh.sh`](docs/setup/scripts/20-zsh.sh) fails on every fresh VM. Switch to the `~/.bashrc` exec-zsh hand-off — the same proven pattern used in the legacy [`02-wsl-terminal-setup.md`](docs/setup/02-wsl-terminal-setup.md). UX-identical for the dev, zero infra escalation required. ## What lands | File | Change | | --- | --- | | `docs/setup/scripts/20-zsh.sh` | Drop the `sudo chsh -s` block. Append a guarded `exec zsh -l` block to `~/.bashrc` instead, marked with a managed-by comment for idempotency on re-runs. | | `docs/setup/01-dev-debian-vm-setup.md` | Table row for `20-zsh.sh` updated — "set as default shell" → "`~/.bashrc` (exec zsh on interactive shells — `chsh` is blocked on the corp VM)". | | `docs/setup/README.md` | Same descriptor adjustment. | ## The hand-off block ```bash # Managed by docs/setup/scripts/20-zsh.sh — apf-portal zsh hand-off. # Hand off to zsh on interactive shells. The `case $-` guard avoids # breaking non-interactive bash (scp, rsync, cron, …), and the # ZSH_VERSION check prevents an infinite re-exec loop. case $- in *i*) if command -v zsh >/dev/null 2>&1 && [ -z "${ZSH_VERSION-}" ]; then exec zsh -l fi ;; esac ``` Two safety nets in the block: - **`case $- in *i*)`** — only runs the hand-off when the shell flag set contains `i` (interactive). `scp` / `rsync` / non-interactive ssh executions go through bash and are not hijacked. - **`[ -z "${ZSH_VERSION-}" ]`** — `ZSH_VERSION` is set by zsh itself; if it's already set we're already in zsh and a re-exec would loop. ## Test plan - [ ] On the dev VM, run `20-zsh.sh` on a fresh state: bash, no existing `~/.bashrc` zsh hand-off → script exits without `chsh` error, `~/.bashrc` gets the block appended, `exit` + reconnect lands in zsh. - [ ] Re-run `20-zsh.sh` → reports `↪ skip zsh hand-off already in ~/.bashrc` (idempotency). - [ ] `scp some-file vm-dev:/tmp/` from the workstation still works (non-interactive bash not hijacked). - [x] `pnpm exec prettier --check` clean on the touched markdown files. - [x] `bash -n docs/setup/scripts/20-zsh.sh` (syntax check) clean. ## Why not amend #220 #220 has already merged. Squash-merge collapsed it to `8a04540` on main; a force-push to amend would rewrite that commit and break anyone who's pulled it. The fix lands as a separate commit on the same setup-doc family. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #221 |
||
|
|
8a04540410 |
docs(setup): add Debian 13 dev-VM setup procedure + scripts + devcontainer (#220)
## Summary Adds a full Debian 13 dev-VM setup procedure ([docs/setup/01-dev-debian-vm-setup.md](docs/setup/01-dev-debian-vm-setup.md)) + 10 modular idempotent setup scripts + a systemd template + a `.devcontainer/` spec, in preparation for the new dev VM (`10.100.201.21`) replacing the WSL-based workflow. Both IDE flows (VSCode Remote-SSH + Devcontainer) and both Node toolchains (nvm on host + devcontainer image) are available — devs pick per task. Adjacent context (does not ship here, planned follow-up): - **Preview infra on the GitLab VM (`10.100.201.10`)** — same `dev.compose.yml`, deployed by CI on `main`. Doc placeholder in §8.6. - **GitLab Runner migration** (act_runner Gitea → GitLab Runner Docker executor) — bundled with the Gitea → GitLab cutover. - **Private dotfiles repo** (`apf/dotfiles`) — `~/.zshrc`, `~/.p10k.zsh`, `~/.tmux.conf` versioned. `80-dotfiles.sh` is already structured to fall back to a `~/.dotfiles/` clone when present. No application-code changes. No CI gate impact (doc + scripts + devcontainer spec only). ## What lands | Path | Change | | --- | --- | | `docs/setup/01-dev-debian-vm-setup.md` | **New.** Step-by-step doc: workstation prep (SSH agent + VSCode Remote-SSH + fonts), bootstrap orchestrator, per-script effects, project clone, infra boot, IDE flow A / B / C, apf-ai-service .NET appendix, troubleshooting. | | `docs/setup/02-wsl-terminal-setup.md` | Renamed from `01-wsl-terminal-setup.md`. No content change. | | `docs/setup/03-dev-web-stack.md` | Renamed from `02-dev-web-stack.md`. No content change. | | `docs/setup/04-angular-nx-monorepo.md` | Renamed from `03-angular-nx-monorepo.md`. No content change. | | `docs/setup/README.md` | **New.** Index of the `docs/setup/` folder. | | `docs/setup/scripts/lib.sh` | **New.** Shared helpers — colour-coded log/ok/warn/err/skip, `apt_install` skipping already-installed, `ensure_line` idempotent append, `confirm` prompt. | | `docs/setup/scripts/bootstrap.sh` | **New.** Orchestrator running scripts 10..80 in order with confirmation prompts. | | `docs/setup/scripts/10-base-packages.sh` | **New.** apt update + base packages (curl, wget, git, build-essential, …). | | `docs/setup/scripts/20-zsh.sh` | **New.** zsh + Oh My Zsh (RUNZSH=no, no shell hijack) + Powerlevel10k + `zsh-autosuggestions` + `zsh-syntax-highlighting`. Patches `~/.zshrc` (theme, plugins, fzf hook). | | `docs/setup/scripts/30-cli-tools.sh` | **New.** `bat eza fd-find ripgrep fzf zoxide ncdu keychain` + `jq yq httpie make tree htop tmux direnv dnsutils unzip rsync`. Symlinks Debian-renamed binaries (`batcat`→`bat`, `fdfind`→`fd`) into `~/.local/bin` so notes/aliases.zsh works as-is. | | `docs/setup/scripts/40-node.sh` | **New.** nvm v0.40.1 + Node from `.nvmrc` (currently 24) + corepack enable + pnpm warmed from `package.json#packageManager`. | | `docs/setup/scripts/50-docker.sh` | **New.** Docker CE + compose plugin from docker.com apt repo, user added to `docker` group, `docker.service` enabled at boot. Pinned GPG key + repo line for Debian Trixie. | | `docs/setup/scripts/60-tuning.sh` | **New.** `fs.inotify.max_user_watches=524288` (Vite/Nx watch ceiling), optional 4 GB swapfile, optional hostname rename (only prompts on generic hostnames like `debian13`). | | `docs/setup/scripts/70-hardening.sh` | **New.** Best-effort UFW (`allow OpenSSH` only) + unattended-upgrades on security channel + fail2ban + sshd drop-in (`PermitRootLogin no`, `PasswordAuthentication no`, `AllowAgentForwarding yes`). Probes before applying, validates `sshd -t` before reloading, skips cleanly if infra already locked the box down. | | `docs/setup/scripts/80-dotfiles.sh` | **New.** Symlinks `notes/aliases.zsh` → `~/.oh-my-zsh/custom/aliases.zsh` (backs up an existing target). Copies `notes/gitconfig.txt` to `~/.gitconfig`, prompts for identity, applies via `git config --global user.name/email`. | | `docs/setup/systemd/apf-portal-infra@.service` | **New.** Template systemd unit auto-starting `./infra/local/dev.sh up` at boot. Install: `enable apf-portal-infra@$USER.service`. | | `.devcontainer/devcontainer.json` | **New.** VSCode Dev Container spec on `mcr.microsoft.com/devcontainers/typescript-node:1-24-bookworm`. `docker-outside-of-docker` feature + `--network=apf-portal-dev` so DNS to `postgres`/`redis`/`otel-collector` works. `initializeCommand` fails fast if `dev.sh up` hasn't been run yet. Forwarded ports labelled. Six dev extensions pre-installed. | | `.devcontainer/post-create.sh` | **New.** `corepack enable && pnpm install --frozen-lockfile`. | | `CLAUDE.md` | "Environment conventions" rewritten: documents the two envs (`local` / `development`) + hybrid sub-mode + the two IDE flows (Remote-SSH / Devcontainer), points at the new VM setup doc and the legacy WSL doc. | ## Key choices - **Idempotent scripts, individually runnable.** Each script probes before doing anything (apt package already installed? plugin already cloned? UFW already active? sshd drop-in already present?). Bootstrap is the orchestrator; each script also runs standalone. Re-running after a partial setup is safe and reports `↪ skip` for the no-op cases. - **No private key on the VM — SSH agent forwarding instead.** `~/.ssh/config` on the workstation carries `ForwardAgent yes`; the VM never holds long-lived secrets. Same future pattern for GPG signing (covered in §8.5 as appendix). `keychain` is installed by `30-cli-tools.sh` as a fallback for scenarios where agent forwarding is not available (CI runners, scripts). - **Hardening is "best-effort, probe-first".** Some infra teams ship pre-hardened VMs; this script doesn't fight that. UFW already active? Print rules and skip. unattended-upgrades already on? Skip. SSH already locked down? Skip. Each section validates before reloading so a misconfig can't take SSH offline. - **Devcontainer assumes infra-on-host.** The container runs on the VM but talks to postgres / redis / otel **on the same VM's host docker daemon** through the shared `apf-portal-dev` Compose network. `initializeCommand` fails fast with a clear message if `./infra/local/dev.sh up` hasn't been run yet — better than puzzling `ECONNREFUSED` errors at runtime. - **`60-tuning.sh` raises inotify to 524288.** Default Debian limit is 8K; Vite/Nx in this monorepo blow past that. The setting is persisted in `/etc/sysctl.d/99-apf-portal.conf` so it survives reboot. - **Hybrid mode (workstation IDE + VM infra) is a documented sub-mode.** SSH `LocalForward` directives on 5432/6379/4317/4318 expose the VM's infra services as `localhost:*` on the workstation. Latency cost: 5-15 ms per query, fine for daily work; for long-running flows, wrap in `tmux` or use `autossh`. ## Notes for the reviewer - **Renaming the existing setup docs (01→02, 02→03, 03→04) is the only "destructive" change.** `git log --follow` still works because of `git mv`. Diff shows up as renames, not delete-and-add. - **The brief asked for `bat eza fd-find ripgrep fzf zoxide docker keychain ncdu git`.** `git` is installed by `10-base-packages.sh` (every other script needs it). Everything else lives in `30-cli-tools.sh` + `50-docker.sh`. The "fullstack-dev extras" (`jq yq httpie make tree htop tmux direnv dnsutils unzip rsync`) are additions I proposed in the lock-in question and that you greenlit (full scope) — easy to trim if any of them turn out to be unwanted. - **Both Node toolchains in parallel** — nvm on the VM (via `40-node.sh`) **and** devcontainer in the repo. Devs can use either; both read the same `.nvmrc` + `packageManager` pin so the version stays consistent. - **The systemd unit is a TEMPLATE** (`apf-portal-infra@.service`) — install once, enable per-user (`enable apf-portal-infra@$USER.service`). This is the right shape for a shared VM with multiple devs eventually, even if today only one user uses it. - **No PR-body Co-Authored-By trailer, no Generated-with-Claude footer**, per the project rule. ## Test plan Manual (no automated test exists for this kind of setup work): - [ ] On a fresh Debian 13 VM: `git clone …`, `./docs/setup/scripts/bootstrap.sh`, answer prompts, end up with zsh + Powerlevel10k + all the requested CLI tools + Node 24 + pnpm 10.33.4 + Docker on PATH. - [ ] `./infra/local/dev.sh up` boots successfully against the VM's local docker daemon. - [ ] `pnpm install` + `pnpm exec nx run-many -t lint test --parallel=3` passes on the VM. - [ ] VSCode Remote-SSH from a workstation: connect, open `~/Works/apf_portal`, run `pnpm exec nx serve portal-bff`, confirm reaches `postgres:5432`. - [ ] VSCode Dev Containers from the same workstation: `Reopen in Container`, image builds, `postCreateCommand` runs `pnpm install`, dev server reaches postgres through the `apf-portal-dev` network. - [ ] Hybrid mode: SSH tunnel from workstation, `pnpm exec nx serve portal-bff` locally, confirm postgres reachable via `localhost:5432`. - [x] `pnpm exec prettier --check` clean on the touched markdown files. - [x] Scripts pass `bash -n` (syntax check) — verified during writing. ## What's next - Validate by walking through this doc on the actual VM `10.100.201.21`. Any friction surfaced becomes a follow-up PR (`docs(setup): ...`). - Once the dev VM is operational, return to **ADR-0027 Implementation PR 1** (Region / Delegation / Structure Prisma schema + inline reference-data migration) — paused since the start of this PR. - Set up the **private dotfiles repo** (`apf/dotfiles`) as a small follow-up, then teach `80-dotfiles.sh` to prefer the dotfiles repo over `notes/`. - When migrating to GitLab: PR pair — (a) `git remote set-url` doc updates here, (b) `infra/gitlab-runners/` replacing `infra/ci-runners.compose.yml`. - Future: deploy the **shared preview infra** on `vm-gitlab` (10.100.201.10) — CI-driven, separate PR. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #220 |
||
|
|
0d8b3712fb |
docs(adr): accept ADR-0026 + ADR-0027 (#219)
## Summary Promotes [ADR-0026](docs/decisions/0026-person-user-portal-data-model.md) and [ADR-0027](docs/decisions/0027-portal-side-organisational-hierarchy.md) from `proposed` to `accepted`. Both shipped as `proposed` in #217 after the cascade / acteurs_plus source-of-truth audit reshaped the org-hierarchy model. No open questions left on either. No code changes — same shape as #205 (ADR-0025 acceptance). ## What lands | File | Change | | --- | --- | | `docs/decisions/0026-person-user-portal-data-model.md` | Frontmatter `status: proposed → accepted`. | | `docs/decisions/0027-portal-side-organisational-hierarchy.md` | Frontmatter `status: proposed → accepted`. | | `docs/decisions/README.md` | Rows for 0026 and 0027 flip to `accepted`. | | `CLAUDE.md` | Roll-up bumped to `0001 → 0027 accepted`; two new Architecture bullets ("Portal-side identity model" and "Portal-side organisational hierarchy") added after the ADR-0025 bullet; ADR-0025's bullet adjusted to clarify the scope literal `etablissement:<structure-code>` (with a pointer to ADR-0027 for the `Structure.code` semantics); the `@RequireScope` Prisma-resolver roadmap entry now references both ADRs as accepted with the two-schema-then-resolver phasing. | ## Notes for the reviewer - **ADR-0025's bullet got a small touch-up, not a rewrite.** The original copy listed scope kinds as `etablissement:<finess>`, which was accurate when ADR-0025 shipped but is now superseded by ADR-0027's `Structure.code` semantics (FINESS for medico-social rows, internal slugs otherwise). The change is `<finess>` → `<structure-code>` + a `see ADR-0027` parenthetical. No actual decision in ADR-0025 changes. - **Two new Architecture bullets, mirrored on the ADR-0024 / ADR-0025 pair of bullets that precede them in tone + length.** The "Portal-side identity model" bullet calls out the `entraOid`-only v1 dedup and the non-unique `Person.email` (both decisions made during the split rework); the "Portal-side organisational hierarchy" bullet calls out the `Structure.code` round-trip semantics and the deferred-to-ADR-0028 items (`Pole`, `Service`, arbitrary nesting, per-source enrichment, full cascade sync). - **No code changes**, so no `pnpm ci:check` impact. `pnpm exec prettier --check` clean on the four touched files. ## Test plan - [x] `pnpm exec prettier --check` — clean on the four touched files. - [x] Internal links resolve (ADR-0026 ↔ ADR-0027 mutual references, plus the `ADR-0028` dangling marker left intentional for the future sync ADR). - [ ] **Review focus** — the two new Architecture bullets phrasing; the ADR-0025 bullet's small scope-literal touch-up; CLAUDE.md roll-up wording. ## What's next Now unblocked — per ADR-0026 §"Phasing" and ADR-0027 §"Phasing": 1. **ADR-0027 Implementation PR 1** — `Region` / `Delegation` / `Structure` Prisma schema + inline reference-data migration (Région Nouvelle-Aquitaine + Délégation 33 + a handful of test-tenant structures) + `Structure.kind` catalogue + drift-gate extension. Independent of (2) at the schema level. 2. **ADR-0026 Implementation PR 1** — `Person` / `User` / `UserScope` Prisma schema + `PersonAndUserProvisioner` called from `SessionEstablisher` + `Person.source` catalogue + drift-gate extension + updated `PrincipalBuilder` populating `Principal.user.{id, personId}` from the real rows. Can ship in parallel with (1). 3. **ADR-0026 Implementation PR 2** — `PrismaScopeResolver` replacing `StubScopeResolver` + `/admin/users/:id/scopes` admin-app screen + `prisma/seed.ts` populating the 19 test personas' `user_scopes` per `notes/test-tenant-role-assignments.md`. Depends on both (1) and (2). 4. **ADR-0028 (proposed)** — Pléiades + Acteurs+ + cascade syncs + facet schemas (Salarié / Élu / Adhérent / Bénévole / Bénéficiaire / PartenaireExterne) + operator-confirmed Person-reconciliation flow + the deferred org-hierarchy extensions (`Pole`, `Service`, per-source enrichment). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #219 |
||
|
|
30cefc4488 |
docs(adr): split ADR-0026 + propose ADR-0027 (Structure hierarchy) (#217)
## Summary
Splits [ADR-0026](docs/decisions/0026-person-user-portal-data-model.md) into two sibling ADRs after a cascade / acteurs_plus source-of-truth audit caught a design break: the first draft pinned `Etablissement.finess` as primary key, but ≥ 30 % of APF's real structure inventory has no FINESS (antennes, dispositifs, entreprises adaptées, mouvement, administratif, siège).
| ADR | Status | Scope |
| --- | --- | --- |
| [ADR-0026](docs/decisions/0026-person-user-portal-data-model.md) — narrowed | `proposed` | `Person` + `User` + `UserScope` only (identity model) |
| [ADR-0027](docs/decisions/0027-portal-side-organisational-hierarchy.md) — new | `proposed` | `Region` + `Delegation` + `Structure` (cascade-aligned: `kind` discriminator + nullable FINESS / SIRET / `codePaie`) |
| `ADR-0028` (future) | — | Pléiades + Acteurs+ + cascade syncs + facet schemas (renumbered from the old `ADR-0027` placeholder) |
No code changes — all three artefacts moving in this PR are markdown.
## What lands
| File | Change |
| --- | --- |
| `docs/decisions/0026-person-user-portal-data-model.md` | Title trimmed (drop `+ organisational hierarchy`); `Region` / `Delegation` / `Etablissement` schema removed; `Person.email` unique constraint dropped (two distinct humans can share an email — see Lifecycle); Lifecycle rewritten without email-based dedup; `UserScope.value` documented as opaque string referencing ADR-0027 codes; Confirmation drops `Etablissement.kind` bullet; ADR-0027 sync references renumbered to ADR-0028; "What ADR-0026 ships vs adjacent ADRs" rewritten to three columns. |
| `docs/decisions/0027-portal-side-organisational-hierarchy.md` | **New.** Decision = Option B (`Structure` with `kind` discriminator + nullable FINESS / SIRET / `codePaie`, internal `code` PK that doubles as FINESS for medico-social structures). Considered options A (FINESS-only — original ADR-0026 draft), B (chosen), C (full cascade replication), D (remote read against cascade). Inline-migration seed for the test tenant (Region 75 + Delegation 33 + a handful of medico-social structures + `siege`); full inventory deferred to ADR-0028's cascade sync. `Structure.kind` enum-as-string drift-gated. |
| `docs/decisions/README.md` | ADR-0026 row: title updated, status stays `proposed`. New ADR-0027 row: `proposed`, tags `data, backend`. |
| `CLAUDE.md` | Roll-up clarifies: `0001 → 0025 accepted`; `ADR-0026 + ADR-0027 proposed`. `@RequireScope` Prisma-resolver roadmap entry references both ADRs + the ADR-0028 follow-up. No new Architecture bullet (entries land when their ADRs ship). |
## Why split
The cascade audit was the trigger. Cascade — APF's medico-social structure registry + Pléiades/Talentia HR integration — models `Structure` with a **seven-value type discriminator**: `medico_social`, `antenne`, `dispositif`, `entreprise_adaptee`, `mouvement`, `administratif`, `sanitaire`. Three of those (`antenne`, `dispositif`, part of `entreprise_adaptee`) **do not have a FINESS** by construction. Cascade carries FINESS / SIRET / SIREN / Pléiades `codePaie` / Talentia `codeCompta` on **separate per-source enrichment rows** (`StructureSourceFiness`, `StructureSourceSirene`, `StructureSourcePleiades`, `StructureSourceTalentia`), nullable and many-to-one against `Structure`.
The acteurs_plus audit confirmed: acteurs_plus does not store FINESS / SIRET / SIREN on its hierarchy entities at all — it uses a portal-internal `code` (unique string) + an `externalId` pointer.
The first ADR-0026 draft's `Etablissement.finess` PK excluded all non-medico-social structures by construction. The fix is **not** to make FINESS nullable on `Etablissement` (that smuggles the discriminator into absence-of-value semantics) — it is to adopt cascade's `Structure` + `kind` discriminator directly. Doing that inside ADR-0026 would have ballooned its scope; splitting is the cleaner shape:
- **ADR-0026** keeps a tight focus on identity (`Person` + `User` + `UserScope`). The Person model is unchanged from the first draft except for the email-dedup rewrite (already discussed before the audit landed).
- **ADR-0027** owns the org hierarchy with the cascade-aligned schema, the seeding posture, and the deferred parts (`Pole`, `Service`, arbitrary nesting, per-source enrichment) called out explicitly as ADR-0028's territory.
## ADR-0027 schema highlights
```prisma
model Structure {
// Portal-internal stable code. For medico-social structures we set
// code = FINESS (round-trips through scope literals + URLs cleanly).
// For non-medico-social structures: APF-internal slug ('siege',
// 'apf-bdx-merignac', 'ea-toulouse', 'mvt-national', …).
code String @id
name String
// Aligned with cascade's Structure.type discriminator. Drift-gated.
kind String // 'medico_social' | 'antenne' | 'dispositif'
// | 'entreprise_adaptee' | 'mouvement'
// | 'administratif' | 'siege'
finess String? @unique // 9 digits, NULL for non-medico-social
siret String? @unique // 14 chars, NULL when not SIRENE-registered
codePaie String? @unique // Pléiades 6-char, NULL in v1
delegationCode String? // NULL for siège, mouvement national
delegation Delegation? @relation(fields: [delegationCode], references: [code])
@@index([kind])
@@index([delegationCode])
}
```
The vocabulary mismatch — ADR-0025's scope kind name is `etablissement` but the value is now a `Structure.code` of any kind — is documented as a known wart, with a possible ADR-0025 amendment as the rename path if a maintainer trips over it.
## Notes for the reviewer
- **`Person.email` unique constraint dropped.** Two distinct humans genuinely can share an email (shared family alias, generic `info@` mailbox, error in an upstream feed). The first draft had `email String? @unique` carried over from a "let's use it as a v1 dedup key" line of thinking that the audit reshaped. The lifecycle now treats `entraOid` as the only natural key the v1 provisioner trusts; email is an attribute, indexed for operator-driven lookup (admin UI search, ADR-0028 reconciliation flow), not a constraint.
- **`UserScope.value` has no FK to ADR-0027 tables.** Deliberate: a scope can outlive its target (a structure decommissioned mid-quarter still has historical UserScope rows pointing at its code, which the audit log needs to read). Admin UI write path validates; runtime guard tolerates stale codes (they fail the resource match, not the sign-in).
- **The old open PR `docs/adr-0026-accept-and-tighten-lifecycle` is superseded by this one.** That branch promoted ADR-0026 (full first draft) to `accepted`. The Q1 / Q2 resolutions from that PR are preserved here — Q1 (no email-dedup) is the new ADR-0026 Lifecycle section; Q2 (inline-migration seed) moves to ADR-0027's "Seeding posture" section since it is org-hierarchy-specific. The old PR can be closed without merging.
- **No code changes**, so no `pnpm ci:check` impact. `pnpm exec prettier --check` clean on the four touched files.
## Test plan
- [x] `pnpm exec prettier --check` — clean on the four touched files.
- [x] ADR cross-references resolve (every `ADR-NNNN` link in the two ADRs round-trips; the new ADR-0028 reference is a known dangling marker for the future sync ADR).
- [ ] **Review focus** — cascade / acteurs_plus audit findings as cited in ADR-0027 §"Context"; the schema choices in ADR-0027 (kind enum, nullable FINESS/SIRET, internal `code` PK); the `Person.email` non-unique change in ADR-0026; the scope-kind vocabulary mismatch documented in ADR-0027.
## What's next (post-merge)
Per ADR-0026 §"Phasing" and ADR-0027 §"Phasing" — the two ADR PRs ship in parallel once accepted:
1. **ADR-0027 Implementation PR 1** — `Region` / `Delegation` / `Structure` Prisma schema + inline reference-data migration + `Structure.kind` catalogue + drift-gate extension.
2. **ADR-0026 Implementation PR 1** — `Person` / `User` / `UserScope` Prisma schema + `PersonAndUserProvisioner` called from `SessionEstablisher` + `Person.source` catalogue + drift-gate extension + updated `PrincipalBuilder`. Independent of (1) at the schema level — can ship in parallel.
3. **ADR-0026 Implementation PR 2** — `PrismaScopeResolver` replacing `StubScopeResolver` + `/admin/users/:id/scopes` admin screen + `prisma/seed.ts` populating the 19 test personas' `user_scopes` per `notes/test-tenant-role-assignments.md`. **Depends on both (1) and (2)** — the seed references `Structure.code` values from (1) and writes `UserScope` rows from (2).
4. **ADR-0028 (proposed)** — Pléiades + Acteurs+ + cascade syncs + facet schemas (Salarié / Élu / Adhérent / Bénévole / Bénéficiaire / PartenaireExterne) + operator-confirmed Person-reconciliation flow + schema extensions (`Pole`, `Service`, per-source enrichment) the sync needs.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #217
|
||
|
|
8266e5b172 |
docs(adr-0026): person + user portal-side data model (proposed) (#213)
## Summary
[ADR-0025](docs/decisions/0025-authorization-model-privileges-roles-scopes.md) shipped the authorization model with three stubs explicitly deferred to a follow-up ADR:
- `Principal.user.id` and `Principal.user.personId` carry the Entra `oid` as a placeholder — `apps/portal-bff/src/auth/principal-builder.ts` documents the seam.
- `StubScopeResolver` returns `[{ kind: 'unrestricted' }]` for every signed-in user; the per-persona scope values documented in `notes/test-tenant-role-assignments.md` have nowhere to live.
- `@RequireScope`'s `ScopableResource` shape references an `Etablissement` / `Delegation` / `Region` chain that has no Prisma table.
ADR-0026 specifies the portal-side data model that closes those stubs. **Decision-only, status `proposed`** — implementation lands in two PRs once accepted.
## What lands
| File | Change |
| --- | --- |
| `docs/decisions/0026-person-user-portal-data-model.md` | New ADR. MADR 4.0.0 format. Tags: `data`, `backend`, `security`. ~310 lines. |
| `docs/decisions/README.md` | One new index row for 0026 (status `proposed`, 2026-05-24). |
## ADR scope
The ADR commits the **schema** for:
- **`Person`** golden record — stable identity, can exist without a portal account (Pléiades pre-provisioning, dossier bénéficiaires, alumni). `source` field tracks provenance (`self-signin` in v1; `pleiades` / `acteurs-plus` join the catalogue with ADR-0027).
- **`User`** portal-account overlay — one-to-zero-or-one with Person, lazy-created on first OIDC callback. Portal-only state (`lastSignInAt`, future a11y preferences) rides here, not on the shared Person row.
- **`UserScope`** — confirms the migration whose shape ADR-0025 §"Sources of truth — apf_portal-side `user_scopes` table" already specified.
- **`Region` / `Delegation` / `Etablissement`** — organisational hierarchy with externally-meaningful codes (INSEE / French dept / FINESS) as primary keys, matching the on-the-wire shape of the scope literals (`etablissement:0330800013`, `delegation:33`).
The ADR **explicitly defers**:
- Facet schemas (`Salarie`, `Adherent`, `Benevole`, `Elu`, `Beneficiaire`, `PartenaireExterne`) — they track upstream-system shape and ride alongside their producing sync.
- Pléiades / Acteurs+ sync logic, reconciliation policy between sync-owned and admin-UI-owned fields.
Both deferrals point at **ADR-0027** (Pléiades + Acteurs+ syncs + facet schemas) as the next-up ADR.
## Notes for the reviewer
- **Why `Person` + `User` split and not a single table.** The "Considered Options" section walks through this. Option A (User-only) breaks down the moment a dossier holder who never signs in needs a stable identifier. Option D (Person with embedded facet columns) forces every Pléiades or Acteurs+ schema change through a table that both shapes share, which is exactly the kind of coupling that ADR-0027 needs the freedom to design out.
- **Why externally-meaningful primary keys for the hierarchy.** `Region.code` / `Delegation.code` / `Etablissement.finess` are INSEE / FINESS codes — stable across reorgs and already on every URL the portal will mint. Adding a separate UUID would force every consumer to indirect through it for no gain. The trade-off (no in-place rename — if a délégation merges, delete + re-insert with the new code) is bounded by how rarely INSEE rebases.
- **Lazy User creation at first sign-in.** v1 has no Pléiades data; the OIDC callback creates the Person + User pair the first time it sees a new `oid`. When Pléiades sync ships (ADR-0027), the same provisioner extends with a `Person.externalId` lookup before falling back to `email`. The schema does not change between the two regimes — only the provisioner's lookup order does.
- **`Person.source` and `Etablissement.kind` as enum-as-string + drift-gate extension.** The catalogue-drift gate ([ADR-0025 §"Confirmation"](docs/decisions/0025-authorization-model-privileges-roles-scopes.md) and `scripts/check-catalogue-drift.mjs`) was built precisely to constrain hand-edited string columns. Extending it to assert every `Person.source` write is in the closed catalogue closes one of the soft spots in the v1 schema without standing up a Postgres ENUM.
- **PII posture.** `Person.firstName` / `lastName` / `email` are flagged in the ADR's Decision Drivers. The Pino redact list ([ADR-0012](docs/decisions/0012-observability-pino-opentelemetry.md)) and the audit-log salt ([ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md)) already cover Entra `oid`; the new PII fields ride the same posture (caller-redacted at the audit module, redacted at the Pino logger before the line ships).
- **What "two PRs" means in the More Information section.** PR 1: Prisma schema migration + `PersonAndUserProvisioner` called from `SessionEstablisher` + drift-gate extension. PR 2: `PrismaScopeResolver` replacing `StubScopeResolver` + `/admin/users/:id/scopes` screen + `prisma/seed.ts` populating the 19 test-tenant personas' `user_scopes` rows.
- **Backward compatibility for in-flight Redis sessions.** Sessions minted before the schema migration carry a Principal whose `user.id` is the Entra `oid` placeholder. The legacy-session bridge in [`apps/portal-bff/src/auth/principal-extractor.ts`](apps/portal-bff/src/auth/principal-extractor.ts) (added in #208) already handles this — when the migration deploys, those sessions continue to work for the 12 h absolute-TTL window, after which every session in Redis has the real `personId`.
## Open questions to resolve before acceptance
These were flagged in the ADR text and are worth surfacing here for the review pass:
- **`Person.email` as the v1 dedup key.** The ADR's "Bad, because" item — two Pléiades records sharing an email would crash the unique constraint. ADR-0027 will add the `externalId` precedence, but in the interim the v1 lazy-creator either trusts emails-are-unique (acceptable for the test tenant where every persona has a distinct one) or skips the dedup attempt and creates a fresh Person per `oid`. The ADR currently picks the dedup-by-email path; flag if the safer choice is "always fresh, reconcile later".
- **Should `Region` / `Delegation` / `Etablissement` migrations be seeded as part of the schema PR, or kept as a separate dataset import?** The ADR is silent on this; my default is "seed the geographic codes APF actually operates in" (a one-off SQL fixture in the migration directory). Worth confirming.
## Test plan
- [x] `pnpm exec prettier --check docs/decisions/0026-person-user-portal-data-model.md docs/decisions/README.md` — clean.
- [ ] **Review focus** — the chosen Option B vs the rejected A/C/D, the deferred facets list, the `Person.source` catalogue, the `Etablissement.kind` catalogue, and the two open questions above.
- [ ] Once accepted, the implementation phasing in the ADR's `§More Information` opens (2 PRs).
## What's next
- **This PR** — ADR-0026 ships as `proposed`.
- **Acceptance PR** — review pass, address open questions, promote `proposed → accepted` (same cadence as [#201 → #205](docs/decisions/0025-authorization-model-privileges-roles-scopes.md) for ADR-0025).
- **Implementation PR 1** — Prisma schema + lazy provisioner.
- **Implementation PR 2** — `PrismaScopeResolver` + admin-UI scope-seeding + test-tenant seed.
- **ADR-0027** — Pléiades + Acteurs+ syncs + facet schemas (the deferred surface from this ADR).
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #213
|
||
|
|
12136f7a8a |
feat(auth): authorization catalogues + Principal builder skeleton (ADR-0025) (#206)
## Summary First implementation PR after [ADR-0025](docs/decisions/0025-authorization-model-privileges-roles-scopes.md) was accepted in #205. Lands the closed-set catalogues + the OIDC-callback hook that composes the session-resident `Principal`. No new guards yet — those come in the next PR per the ADR's `§More Information` phasing. ## What lands **New shared library: `libs/shared/auth/`** (`scope:shared`, `type:shared`, framework-agnostic, vitest) | File | Role | | --- | --- | | `src/lib/authorization.types.ts` | Closed catalogues: `PRIVILEGES` (4), `FUNCTIONAL_ROLES` (24), `SCOPE_KINDS` (6). `Principal`, `Scope` types. `isPrivilege` / `isFunctionalRole` / `isScopeKind` type-guards for the drift gate. | | `src/lib/entra-group-to-role.ts` | `parseEntraGroupMap` (validates GUID + slug closedness + dedup) + `EntraGroupToRoleResolver` (translates the Entra `groups` claim into ordered `apf-role-*` slugs, reports unknown GUIDs via callback). | | `src/lib/*.spec.ts` | 17 unit tests against the catalogue counts + resolver contracts. | **BFF wiring** (`apps/portal-bff/src/auth/` + `src/config/`) | File | Role | | --- | --- | | `auth.service.ts` | Extracts the Entra `groups` claim. `AuthenticatedUser` grows `groups: readonly string[]`. | | `principal-builder.ts` | Composes the three axes at sign-in. Filters `roles` claim through `isPrivilege` (drops + WARNs on unknown), resolves `groups` claim through the `EntraGroupToRoleResolver` (drops + WARNs on unknown). | | `scope-resolver.ts` | `ScopeResolver` abstract class + `StubScopeResolver` returning `[{ kind: 'unrestricted' }]` (per ADR-0025 §331 — replaced by a Prisma implementation when ADR-0026's `user_scopes` table lands). | | `session-establisher.service.ts` | Calls `PrincipalBuilder.build()` once, persists the result as `req.session.principal`. The legacy `req.session.user` shape is untouched (audit + AI bridge keep working). | | `session.types.ts` | Adds optional `principal?: Principal` field on the express-session augmentation. | | `entra-group-to-role.token.ts` | DI token for the lazily-loaded resolver. | | `config/load-entra-group-map.ts` | Reads the gitignored `infra/<env>-tenant.entra.json` at boot. Missing path → empty resolver + WARN. Malformed file → hard fail (alternative would be silently dropping a role). | **Tests against the 19 test-tenant personas** (`principal-builder.spec.ts`) Every persona provisioned in `apfrd.onmicrosoft.com` on 2026-05-20 is exercised end-to-end: `admin`, `directeur-bordeaux`, `directeur-complexe`, `rh-aquitaine`, `rh-siege`, `collaborateur-simple`, `tresorier-bordeaux`, `dpo`, `it`, `benevole-aquitaine`, `chef-equipe-bordeaux`, `chef-service-bordeaux`, `directeur-territorial-aquitaine`, `juriste-siege`, `rssi`, `communication-siege`, `elu-ca-national`, `president-cd-aquitaine`, `secretaire-cd-aquitaine`. A coverage assertion confirms the personas cover all 4 privileges + 23 of 24 functional roles (the `partenaire` gap is intentional per ADR-0025). **Infra + docs** | File | Change | | --- | --- | | `infra/test-tenant.entra.example.json` | Template GUID → slug map (24 entries, full v1 catalogue). Real file (`*-tenant.entra.json`) gitignored. | | `infra/README.md` | New row + "Entra group map" section documenting the load semantics + provisioning workflow. | | `apps/portal-bff/.env.example` | New `ENTRA_GROUP_MAP_PATH` block. | | `.gitignore` | `infra/*-tenant.entra.json` (except `.example.json`). | | `tsconfig.base.json` | `shared-auth` path alias. | | `notes/entra-groups-claim-activation.md` | Operator runbook for the Entra admin-centre step (Token configuration → Add groups claim → Security groups → Group ID). | **ADR amendment** | File | Change | | --- | --- | | `docs/decisions/0025-authorization-model-privileges-roles-scopes.md` | Path references updated `libs/feature/auth/...` → `libs/shared/auth/...` (4 occurrences), and the `ENTRA_GROUP_TO_ROLE` static-record example replaced by the actual `EntraGroupToRoleResolver` + JSON shape. | ## Notes for the reviewer - **Why `libs/shared/auth/` and not `libs/feature/auth/` (as the ADR initially said).** The existing `libs/feature/auth/` carries `tags: ['scope:portal-shell', 'type:feature']`, which the Nx `enforce-module-boundaries` rule in `eslint.config.mjs:36-46` blocks the BFF from importing. The catalogue is needed by both sides (BFF builds the `Principal`, SPA will gate UI conditionally later), so a `scope:shared` lib is the correct home. The ADR is patched in the same PR so the document and the code agree. - **Why drop unknown privileges with a WARN instead of preserving them.** `Portal.GhostRole` in the `roles` claim is either a leftover from a different app registration or a v1+1 experiment that hasn't been added to the catalogue yet. Both paths should not silently grant access — drop with a WARN so an operator notices, and the drift CI gate (next PR) catches the source-side equivalent before merge. - **Why the principal builder uses a `ScopeResolver` seam now.** Per ADR-0025 §331 the v1 implementation returns `unrestricted` for everyone; the real version queries the `user_scopes` Prisma table that lands with ADR-0026. The seam is here so the next PR replaces only the implementation, not the call-site in `PrincipalBuilder` or its 24 tests. Per-persona stub scope data was deliberately not embedded in this PR — no guard consumes scopes yet, so it would be write-only documentation that drifts from the eventual Prisma seed. - **Why `user.id` and `user.personId` placeholder to `entraOid`.** Same logic: the real UUIDs land with the `Person` + `User` Prisma schema (proposed ADR-0026). Until then, populating both fields with `entraOid` lets consumers key on `principal.user.id` today and get a real value later without branching on availability. The seam is one place (in the builder), not 24 guards. - **Why session.types.ts carries both `user` and `principal`.** Additive instead of replacement: the audit module + the AI-bridge controller key on `user.oid` / `user.tid` directly, and migrating them in the same PR would balloon the diff for zero behavioural benefit. New consumers (`@RequirePrivilege` / `@RequireRole` / `@RequireScope` decorators, next PR) reach for `principal`. - **Map file fails soft on absence, hard on malformation.** Path unset OR file missing → empty resolver + WARN. Sign-in still succeeds, every user gets `roles: []`. This is deliberately fail-soft so a fresh dev environment isn't blocked by an Entra-side dependency. **Bad JSON / unknown slug / duplicate GUID → throws at boot.** The alternative would silently mis-resolve a role. - **Why `boot-time` validation only (no per-request)** for the map file. Catalogue drift is detected once at boot; per-request validation would re-read the file from disk on every sign-in. The trade-off is that a hot-edit to the file requires a BFF restart — acceptable because the file changes once per tenant lifecycle, not per session. - **Entra `groups` claim must be activated** on the BFF's app registration before this code does anything useful. The runbook in `notes/entra-groups-claim-activation.md` walks through the steps; if the claim isn't activated, every user signs in with `principal.roles = []` and a future warn for every group GUID that would have arrived (none, in that case). - **No drift gate yet.** ADR-0025's §"Confirmation" §346 calls for an ESLint custom rule (or `pnpm run` script) that greps every `@RequireRole('...')` literal and asserts membership in the catalogue. That's the third PR in the phasing — there's no `@RequireRole` consumer in the tree yet, so the gate would have nothing to assert against today. ## Test plan - [x] `pnpm nx affected -t lint test build --base=main` — 13 projects green - 497 BFF tests (was 465 — added 4 new `groups`-claim tests in `auth.service.spec.ts`, 1 new test in `session-establisher.service.spec.ts`, 6 new tests in `load-entra-group-map.spec.ts`, 24 new tests in `principal-builder.spec.ts`) - 17 `shared-auth` tests (catalogue counts + resolver contracts) - 1 `shared-util` test (unchanged) - full lint + full build - [x] `pnpm nx format:check` — clean after `pnpm nx format:write` - [x] Manual: `tsc --build libs/shared/auth/tsconfig.lib.json` emits `.d.ts` files at the path `portal-bff`'s webpack expects. - [ ] **Review focus** — the closed catalogues (`PRIVILEGES`, `FUNCTIONAL_ROLES`, `SCOPE_KINDS`) verbatim match ADR-0025; the 19 personas verbatim match `notes/test-tenant-role-assignments.md`; the `Principal` shape matches the ADR §"Principal shape" (modulo the `user.id` / `personId` placeholder); fail-soft on missing map file, hard on malformed. - [ ] **After merge — operator step** : activate the Entra `groups` claim per `notes/entra-groups-claim-activation.md` and drop the real GUIDs into `infra/test-tenant.entra.json` (gitignored). Then a sign-in with `admin@apfrd.onmicrosoft.com` should land `principal.privileges = ['Portal.Admin']` and `principal.roles = ['collaborateur', 'rh']` in Redis. ## What's next Per ADR-0025 §"More Information" phasing: 1. ✅ **This PR** — types + Principal builder + group-to-role mapping skeleton. 2. **Next PR** — `@RequirePrivilege` + `@RequireRole` + `@RequireScope` decorators + guard integration tests against the 19 personas. 3. **PR after that** — drift CI gate (ESLint custom rule asserting every `@RequireX('...')` literal is in the catalogue). 4. **Depends on ADR-0026** — `user_scopes` Prisma table + seed + `PrismaScopeResolver` replacing `StubScopeResolver`. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #206 |
||
|
|
c9a1e195fe |
docs(adr-0025): promote to accepted + sync persona matrix with test tenant (#205)
## Summary ADR-0025 was merged as `proposed` in #201. The test tenant (`apfrd.onmicrosoft.com`) has since been provisioned with the full role / user matrix — 4 privileges + 24 security groups + 19 users with all assignments per the persona table. The ADR is now the implementation reference, so this PR: 1. Promotes the ADR from `proposed` to `accepted`. 2. Syncs the document with what is actually in place — the privilege catalogue grows from 1 to 4 entries (the previously "anticipated future" privileges that the test tenant already has), and the persona matrix grows from 10 to 19 entries (so every one of the 24 functional-role groups has at least one member, closing the gap that prompted `notes/test-tenant-role-assignments.md` and `notes/entra-group-members.md`). 3. Records the Entra app-role GUIDs in a new "Provisioned in the test tenant" subsection for traceability — the GUIDs are stable IDs the implementation will need. 4. Updates the index + the `CLAUDE.md` roll-up. No code changes. No implementation skeleton — that lands in the next PR (proposed: `feat(libs/feature/auth): authorization types + Principal builder skeleton`). ## What lands | File | Change | |---|---| | `docs/decisions/0025-authorization-model-privileges-roles-scopes.md` | Frontmatter `status: proposed → accepted`; privilege catalogue extended from 1 to 4 entries; persona matrix rewritten from 10 to 19 entries; new `Provisioned in the test tenant (2026-05-20)` subsection capturing the four app-role GUIDs. | | `docs/decisions/README.md` | 0025 row: `proposed → accepted`. | | `CLAUDE.md` | Roll-up `0001 → 0024 accepted` → `0001 → 0025 accepted`; Architecture list grows an "Authorization model" bullet. | The two operator-facing notes files (`notes/test-tenant-role-assignments.md`, `notes/entra-group-members.md`) are gitignored and unchanged — they served their purpose during tenant provisioning and remain as runbooks. ## Notes for the reviewer - **Why promote now rather than couple with the first implementation PR (the pattern from #194 → #195 → #196).** The Entra-side provisioning *is* the implementation for this ADR — the next PR is a portal-side reflection of decisions that are already concrete in the tenant. Promoting now keeps the document honest about what the test tenant runs against. - **Why all 4 privileges enter the v1 catalogue.** The ADR originally shipped `Portal.Admin` as the sole v1 entry and listed the other three under "anticipated near-future entries". The test tenant has all four; the catalogue should match. The three new entries are explicitly marked "provisioned; consumer surface deferred" so a reader does not look for non-existent surfaces. - **Why 19 personas, not the cleaner 24 (one per role).** Several APF jobs genuinely combine multiple roles (RH siège often handles paie + compta; DPO often wears the quality officer hat; local delegates often grow out of volunteer roles). Densifying these existing personas is more faithful to real APF org structure than inventing 14 single-role test users. Distinct personas were created where the *scenario* is distinct (scope variations, governance positions) — see the matrix in the ADR for the breakdown. - **Why `apf-role-partenaire` stays empty in v1.** Placeholder per the original ADR; no consuming surface to test against. The group exists in Entra so the schema is locked, but a user assignment without a guard to exercise would be theatre. The first partner-facing feature adds the user. - **GUIDs in the ADR.** The four app-role GUIDs are repo-stable identifiers; recording them in the ADR keeps the document self-sufficient when a future contributor opens it without access to the Entra portal. The 24 functional-role group GUIDs are tenant-specific and stay in a gitignored `infra/test-tenant.entra.json` once the implementation PR creates it — referenced by name only in `libs/feature/auth/src/lib/entra-group-to-role.ts`. - **No `prettier --write` damage.** The persona matrix is a wide table; Prettier sometimes reflows wide markdown tables. The diff is clean — Prettier left the table intact on this run. ## Test plan - [x] `prettier --check docs/decisions/0025-authorization-model-privileges-roles-scopes.md` — passes (hook ran on commit). - [x] Markdown links inside the ADR still resolve (`0020`, references to other ADRs unchanged). - [x] Status row in `docs/decisions/README.md` reflects `accepted`. - [x] `CLAUDE.md` roll-up line + Architecture list updated; no other instances of "0024" needed bumping. - [ ] **Review focus** — the expanded privilege catalogue (4 entries, one of them already had a guard, three new ones documented), the 19-entry persona matrix, the "Provisioned in the test tenant" subsection (especially the GUIDs — make sure none was mistyped from `notes/role-user.txt`). ## What's next With ADR-0025 accepted, the implementation phasing recorded in its `§More Information` opens: 1. **PR — `libs/feature/auth` extension** : `authorization.types.ts` (catalogue constants for the 4 privileges + 24 functional roles + 6 scope kinds), `entra-group-to-role.ts` (slug map skeleton with placeholder GUIDs ; the operator drops real GUIDs into `infra/test-tenant.entra.json` separately), `Principal` builder hook on the OIDC callback, no new guards yet. 2. **PR — `@RequireRole` + `@RequireScope` decorators + guard tests** : real composition tests against the 19 personas. 3. **PR — drift CI gate** : ESLint custom rule asserting every `@RequireRole('...')` literal in code is in the catalogue. 4. **PR — `prisma/seed.ts` for `user_scopes`** : depends on the `Person` + `User` schema (proposed ADR-0026). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #205 |
||
|
|
ef5073de8a |
docs(adr-0025): authorization model — privileges × roles × scopes (#201)
## Summary
Proposes [ADR-0025](docs/decisions/0025-authorization-model-privileges-roles-scopes.md) — the portal's general-purpose authorization model. **Status: `proposed`.** No code in this PR; the goal is to lock the model and the v1 catalogues before the implementation chantier opens.
The model rejects stargate's linear hierarchy (`Admin ⊃ Directeur ⊃ RH ⊃ Collaborateur`) and adopts **three orthogonal axes**:
| Axis | What it carries | Source of truth |
|---|---|---|
| **Privileges** | Portal-level capabilities (`Portal.Admin`, future `Portal.Auditor`, …) | Entra app roles, `roles` claim |
| **Functional roles** | What someone does in APF (`rh`, `directeur-etablissement`, `elu-cd-tresorier`, …) | Entra security groups, `groups` claim → curated slug catalogue |
| **Scopes** | Where a role applies (`etablissement:0330800013`, `delegation:33`, `unrestricted`, …) | apf_portal-side `user_scopes` table (v1) ; future Pléiades feed |
The three axes compose at sign-in into a session-resident `Principal`. The portal's guards consume the structured shape; a deterministic projector flattens it to the `roles[]` list that `apf-ai-service` expects per [ADR-0024](docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md).
## What lands
- `docs/decisions/0025-authorization-model-privileges-roles-scopes.md` — full MADR with context, decision drivers, four considered options, exhaustive v1 catalogues, Entra-side configuration, `Principal` shape, AI-service projection, guard surface, ten test-tenant personas, consequences, confirmation criteria, pros/cons per option, ABAC migration path, related ADRs, and proposed follow-up ADRs.
- `docs/decisions/README.md` — index row for ADR-0025 (`proposed`, tags `security, backend, data`, 2026-05-20).
No `CLAUDE.md` update — ADR stays in `proposed` until reviewed; the accepted-ADRs roll-up at the top of `CLAUDE.md` stays at 0001 → 0024. Promotion to `accepted` lands in the same PR that ships the implementation skeleton (`libs/feature/auth` extension with `Principal` builder + catalogues).
## Highlights worth review focus
- **Privilege catalogue is intentionally minimal**: only `Portal.Admin` in v1. Anticipated future entries (`Portal.Auditor`, `Portal.SecurityOfficer`, `Portal.DPO`) are mentioned in the ADR but not formalised — each one rides an amendment ADR.
- **Functional-role catalogue is closed-set, 22 entries** grouped into workforce (15), governance (6), volunteer (2), external (1, placeholder). Adding a new slug requires an ADR amendment. The CI drift gate (proposed in §"Confirmation") asserts no orphan `@RequireRole('x')` literal in code.
- **Scope kinds are also closed-set**: `self`, `etablissement:<finess>`, `delegation:<dept>`, `region:<insee>`, `siege`, `unrestricted`. The `value` carriers are documented (FINESS code rather than internal `etablissement.id` because FINESS is stable across reorgs).
- **`Principal` shape** is the contract for everything downstream. Documented field-by-field. Built once at sign-in, persisted in the Redis session, refreshed on every authenticated request.
- **`PrincipalProjector` for the AI service** is mechanical: union of privileges + roles + scope-strings, no inclusive expansion. The projector is the only seam that knows about the flat shape; the rest of the portal never touches it.
- **Closed-vs-open catalogue trade-off** spelled out: the friction of "every new role rides an ADR amendment" is the price of "every slug in code is one a human approved". The drift CI gate enforces the discipline.
- **ABAC migration path** documented so a future contributor does not feel they must rewrite authorization to introduce a single Cedar/OPA-shaped rule.
## Test-tenant personas
The ADR proposes ten test users covering every interesting combination of the three axes. Table in §"Test-tenant personas" of the ADR; here's the summary the user can act on:
| Login | Privileges | Functional roles | Scopes |
|---|---|---|---|
| `admin@<tenant>` | `Portal.Admin` | `collaborateur`, `rh` | `unrestricted` |
| `directeur-bordeaux@<tenant>` | — | `directeur-etablissement`, `collaborateur` | `etablissement:0330800013` |
| `directeur-complexe@<tenant>` | — | `directeur-etablissement`, `collaborateur` | two `etablissement:*` scopes |
| `rh-aquitaine@<tenant>` | — | `rh`, `collaborateur` | `delegation:33` |
| `rh-siege@<tenant>` | — | `rh`, `responsable-paie`, `collaborateur` | `unrestricted` |
| `collab-simple@<tenant>` | — | `collaborateur` | `self` |
| `tresorier-bordeaux@<tenant>` | — | `elu-cd-tresorier`, `elu-cd` | `delegation:33` |
| `dpo@<tenant>` | — | `dpo`, `collaborateur` | `unrestricted` |
| `it@<tenant>` | — | `it`, `collaborateur` | `unrestricted` |
| `benevole-aquitaine@<tenant>` | — | `benevole`, `benevole-responsable` | `delegation:33` |
**Entra test-tenant setup the user needs to provision after acceptance**:
1. One app role: `Portal.Admin` (likely already there from the existing dev tenant; document the GUID in `apps/portal-bff/.env.test`).
2. **22 security groups**, one per functional-role slug in the catalogue: `apf-role-collaborateur`, `apf-role-chef-equipe`, `apf-role-chef-service`, `apf-role-directeur-etablissement`, `apf-role-directeur-territorial`, `apf-role-rh`, `apf-role-responsable-paie`, `apf-role-comptable`, `apf-role-juriste`, `apf-role-dpo`, `apf-role-rssi`, `apf-role-it`, `apf-role-formation`, `apf-role-qualite`, `apf-role-communication`, `apf-role-elu-ca`, `apf-role-elu-cd`, `apf-role-elu-cd-president`, `apf-role-elu-cd-tresorier`, `apf-role-elu-cd-secretaire`, `apf-role-delegue`, `apf-role-benevole`, `apf-role-benevole-responsable`, `apf-role-partenaire`.
3. The ten test users with the membership matrix above.
4. App registration manifest tweak: `groupMembershipClaims: 'SecurityGroup'` + `optionalClaims.idToken: [{ name: 'groups' }]` so the BFF sees the memberships in the ID token.
GUIDs and credentials stay in the operator's hands (out of git). When the user has provisioned the tenant, drop the GUIDs into `infra/test-tenant.entra.json` (gitignored) and the implementation PR wires `libs/feature/auth/src/lib/entra-group-to-role.ts` against them.
## Notes for the reviewer
- **Why not just extend stargate's `RoleMapper`.** The mapper's inclusive expansion (`Admin → [admin, directeur, rh, collaborateur]`) bakes in the wrong assumption — that roles form a chain. Reusing it would force every new role into the chain too. The three-axis model has no such forcing function.
- **Why `Person` is conspicuously absent here.** Authorization is keyed on the portal-side `User` account (Entra OID, session). The proposed `Person` + `User` split lands in a sibling ADR (proposed: ADR-0026) because the two decisions have different audiences — auth model is a backend/security concern; golden record is a data/domain concern. They will land in coordinated PRs.
- **Why FINESS rather than internal UUID for `etablissement:*` scopes.** FINESS codes are the canonical APF identifier for an établissement, stable across the internal-database churn (etablissement merges, reorgs, system migrations). Using the FINESS as the scope value means scope strings stay readable, debuggable, and stable when an établissement gets a new internal `id` after a Prisma migration.
- **Why no time-bound roles in v1.** APF does have interim assignments (acting Directeur for two months while the permanent one is on leave). The `user_scopes` table already has `expiresAt` to lay the groundwork; extending the *role* axis with time bounds is a future ADR amendment when a concrete use case lands.
- **Coordination with apf-ai-service.** The PrincipalProjector spec here matches exactly what `apf-ai-service`'s RBAC matrix tests expect (each chunk's ACL is a string-match against `Principal.roles[]`). The ADR explicitly notes that the projector is the only place that knows about the flat shape — keeping the AI-side contract honoured without polluting the portal-side guards.
## Test plan
- [x] `prettier --check docs/decisions/0025-authorization-model-privileges-roles-scopes.md` — passes (hook ran on commit).
- [x] Markdown links inside the ADR resolve (`0008`, `0009`, `0010`, `0011`, `0013`, `0020`, `0021`, `0024`, plus the proposed ADR-0026 / ADR-0027 placeholders).
- [x] Index row in `docs/decisions/README.md` follows the table's existing format.
- [x] No tag-vocabulary additions required — `security`, `backend`, `data` are all in the existing vocab.
- [ ] **Review focus** — the v1 catalogues (privileges + 22 functional roles + 6 scope kinds), the `Principal` shape, the projection contract for the AI service, and the ten test personas. Catalogue closures are deliberate; raising the lid requires an amendment so the v1 list deserves a careful pass.
## What's next (once accepted)
The implementation phasing recorded in the ADR's §"More Information":
1. **PR — types + Principal builder + Entra mapping skeleton**. Lands `libs/feature/auth/src/lib/authorization.types.ts` (catalogue constants), `entra-group-to-role.ts` (slug map), and the OIDC callback hook that extends `req.session.user` with `privileges` / `roles` / `scopes`. No new guards yet.
2. **PR — `@RequireRole` + `@RequireScope` decorators + guard tests**. Stub principal in unit tests; real session in e2e.
3. **PR — drift CI gate**. ESLint custom rule or `pnpm run` script: every `@RequireRole('...')` / `@RequirePrivilege('...')` / scope literal must exist in the catalogue constants.
4. **PR — test-tenant seed**. `prisma/seed.ts` populating the ten personas' `user_scopes` rows. Depends on the `Person` + `User` schema PR landing first.
In parallel, the user provisions the test tenant per the §"Test-tenant personas" instructions above.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #201
|
||
|
|
883c5151de |
feat(portal-bff): ai-bridge controller — SSE chat + JSON rag/models (#196)
## Summary Step 3 of the AI-relay chantier (after #194 ADR and #195 client skeleton). Wires the BFF-side **live surface** that the SPA's future chatbot widget will consume. [ADR-0024](docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md) is promoted from `proposed` to `accepted` in the same change. Three end-user routes under `/api/ai/*`, gated by the active portal session (no `@RequireAdmin` — AI is a regular-user surface): | Route | Verb | Wire | Maps to | |---|---|---|---| | `/api/ai/chat` | `POST` | `text/event-stream` | `apf.ai.v1.ChatService.Chat` (server-stream) | | `/api/ai/rag/search` | `GET` | `application/json` | `apf.ai.v1.RagService.Search` (unary) | | `/api/ai/models` | `GET` | `application/json` | `apf.ai.v1.ModelsService.ListModels` (unary) | CSRF and session validation are delegated to the global middleware mounted in `main.ts` (per [ADR-0009](docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md) and [ADR-0021](docs/decisions/0021-phase-2-security-baseline.md)); the controller asserts `req.session.user` and emits 401 if absent. ## What lands ### `apps/portal-bff/src/grpc/ai-bridge/` ``` ai-bridge/ ├── ai-bridge.module.ts imports AiClientModule, exports the controller ├── ai-bridge.controller.ts 3 routes — POST chat (SSE), GET rag/search, GET models ├── sse.writer.ts ChatEvent oneof → SSE frame translator ├── sse.writer.spec.ts unit tests for the codec ├── ai-bridge.controller.spec.ts end-to-end against an in-process fake gRPC server └── dto/ ├── chat-request.dto.ts class-validator body shape (POST /chat) └── rag-search-query.dto.ts class-validator query shape (GET /rag/search) ``` ### SSE codec (`sse.writer.ts`) Each `ChatEvent` oneof case becomes one SSE frame with a kebab-case `event:` name and a JSON-encoded `data:` payload: ``` event: token data: {"token":"…","value":"…"} event: agent-step data: {"agent":"…","step":"…","stepId":"…"} event: tool-call data: {"callId":"…","name":"…","args":{…}} event: done data: {"stats":{"tokensIn":…,"tokensOut":…,"chunksRetrieved":…}} ``` A helper `relayErrorFrame(code, message, retriable)` synthesises a relay-side `event: error` frame that matches the AI service's own `ErrorEvent` shape — the SPA's renderer needs no second code path for relay-level failures vs upstream model errors. gRPC status codes map into the `urn:apf-ai:*` namespace (`UNAVAILABLE` → `urn:apf-ai:unavailable`, `DEADLINE_EXCEEDED` → `urn:apf-ai:timeout`, `PERMISSION_DENIED` → `urn:apf-ai:permission_denied`, `RESOURCE_EXHAUSTED` → `urn:apf-ai:rate_limited`, `INVALID_ARGUMENT` → `urn:apf-ai:invalid_argument`, anything else → `urn:apf-ai:relay_error`). The terminal `done` frame closes the stream — no `[DONE]` sentinel, per ADR-0024. ### Controller (`ai-bridge.controller.ts`) - `POST /api/ai/chat` — builds an `apf.ai.v1.ChatRequest` from the validated DTO + session-derived Principal, calls `ChatClient.chat()`, drains the `ClientReadableStream<ChatEvent>` into SSE frames written on the raw Express `Response`. `req.on('close', …)` propagates browser disconnect through an `AbortController` into `call.cancel()` so the upstream LLM stops (per `apf-ai-service/docs/streaming.md`). - `GET /api/ai/rag/search` — unary RAG call. `topK` defaults to 0 (server picks the default). `source` and `documentId` query params surface the same filter fields the upstream RPC accepts. - `GET /api/ai/models` — unary lookup of the provider catalogue. The SSE writes happen on the raw Express response (manual `setHeader` + `flushHeaders` + `write` + `end`) rather than through NestJS's `@Sse()` decorator, because `@Sse()` is GET-only and the chat endpoint is POST (the SPA carries the conversation history in the body). ### Lifecycle hooks `AiClientModule` now implements `OnApplicationShutdown` and closes the four gRPC stubs (Chat / Rag / Ingestion / Models). The four stubs share the same HTTP/2 channel (gRPC-js dedups on `endpoint + credentials`), so the `close()` calls are cheap, but kept explicit so adding a fifth stub later is an obvious one-line addition. `main.ts` now calls `app.enableShutdownHooks()` so `SIGTERM` / `SIGINT` / `SIGHUP` actually route through the lifecycle interface. ### DTOs `ChatRequestDto` constrains: - `messages` — 1 to 64 entries; each has `role ∈ {user, assistant, system}` (no `tool` — tool messages are constructed BFF-side per ADR-0024 §"Tool-dispatch contract") and `content` ≤ 16 KB. - `conversationId`, `model`, `provider` — optional, ≤ 64 / 128 chars. `RagSearchQueryDto`: - `query` — required, non-empty. - `topK` — optional, integer in `[1, 50]` (the AI service has its own cap; the BFF rejects out-of-range values early). - `source` / `documentId` — optional pass-through filters. ### Documentation - ADR-0024 frontmatter: `status: proposed` → `accepted`. - `docs/decisions/README.md` index reflects the new status. - `CLAUDE.md` Architecture section grows an "AI service relay" bullet; the roll-up line moves from "ADRs 0001 → 0023" to "0001 → 0024"; the shipped-on-main list grows an "AI relay surface" entry. - `apps/portal-bff/.env.example` documents `AI_SERVICE_GRPC_ENDPOINT` / `AI_SERVICE_CLIENT_ID` / `AI_SERVICE_GRPC_TLS` and points operators at `apf-ai-service`'s own docker-compose for the runtime dependency. ## Notes for the reviewer - **No live AI service in this PR's local-dev stack.** `apf-ai-service` runs from its own repo (`/home/jgautier/Works/apf-ai-service`) with its own `infra/docker-compose.yml`. The BFF dials `localhost:8080` by default — the host-published port of the AI service's container. This is option (a) from ADR-0024 §"Open question — Compose orchestration": two independent stacks, dial across via host networking. Merging the compose files into one would couple two release cadences without operational payoff. - **Tests run against an in-process fake `grpc.Server`.** All five spec cases on the controller wire it up against a fake `ChatService` + `RagService` + `ModelsService` server bound to `127.0.0.1:0` (random port). No mocks — the controller's gRPC client makes a real connection, real serialisation, real cancellation propagation. Cost: ~0.5 s overhead from the gRPC server setup. - **CSRF + session middleware are unchanged.** The new POST endpoint is protected by the existing double-submit CSRF middleware mounted in `main.ts` (per [ADR-0021](docs/decisions/0021-phase-2-security-baseline.md)). The SPA's fetch call needs to send the `X-CSRF-Token` header matching the `__Host-portal_csrf` cookie — same protocol as every other POST in the BFF. No per-controller wiring required. - **Manual session check rather than a guard.** Three reasons: (1) matches the existing pattern in `me.controller.ts`; (2) the session check is the only authorization gate (no roles to evaluate) — a guard would add ceremony without payoff; (3) the SSE controller already takes control of the response object (`@Res()`), which `UseGuards` interacts with awkwardly. Throwing `UnauthorizedException` lets `StructuredErrorFilter` produce the 401 envelope before any header is flushed. - **Why the controller does NOT use `@Sse()`.** NestJS's `@Sse()` decorator is GET-only and emits frames from `Observable<MessageEvent>`. The chat endpoint is POST (the SPA sends conversation history in the body) and the source is a Node `Readable` stream from `@grpc/grpc-js`. Manual response handling is simpler than adapting to / from `Observable` for a single consumer. - **Cancellation contract.** When the SPA aborts the fetch, the browser closes the TCP connection, Express emits `'close'` on the request, the controller's `AbortController.abort()` triggers, `ChatClient` calls `.cancel()` on the gRPC stream, the AI service's `ServerCallContext.CancellationToken` cancels the upstream LLM. The spec covers the `'close'` → server-side `cancelled` event end-to-end. - **No ingestion route in the BFF.** Per ADR-0024 §"Out of scope", v1 admin ingestion uses the `apf-ai-service/tools/Apf.Ai.Ingest/` CLI. A future PR adds the BFF endpoint when the admin "manage AI corpus" surface ships. `IngestionClient` remains in `AiClientModule` so that future PR is one new file, not a new module plus a new client. - **No bundle-size or perf surprise.** The BFF is a Node process, not a SPA chunk — bundle budgets don't apply. The gRPC channel is opened lazily on first call; idle BFFs incur no upstream TCP cost. ## Test plan - [x] `pnpm nx test portal-bff` — **461 specs pass** (was 443; +13 new: 8 SSE writer cases + 5 controller end-to-end cases against the in-process fake server). Worker-exit-leak warning persists from the gRPC server's slow shutdown — pre-existing pattern from PR #195; harmless. - [x] `pnpm nx lint portal-bff` — 6 pre-existing warnings, no new ones from the diff. - [x] `pnpm nx build portal-bff` — clean webpack compile. - [x] Module wiring: `AppModule` imports `AiBridgeModule`, which imports `AiClientModule`. Resolves cleanly through DI; the audit-side `HashUserIdService` is satisfied by `AiClientModule`'s local provider (per the rationale recorded in PR #195's `AiClientModule` docstring). - [ ] **Manual smoke** — bring up `apf-ai-service` from its own repo (`cd ../apf-ai-service && docker compose -f infra/docker-compose.yml up`), set `AI_SERVICE_GRPC_ENDPOINT=localhost:8080` in `apps/portal-bff/.env`, run `pnpm nx serve portal-bff`. Sign in to `portal-shell`, then in a terminal: ```bash curl --cookie-jar /tmp/portal-session http://localhost:3000/api/auth/login # follow Entra… curl -N \ -H 'Content-Type: application/json' \ -H 'X-CSRF-Token: <copied from cookie>' \ --cookie /tmp/portal-session \ -d '{"messages":[{"role":"user","content":"hello"}]}' \ http://localhost:3000/api/ai/chat ``` Expect a streamed SSE response terminated by an `event: done` frame. Verify `GET /api/ai/rag/search?query=test` returns a JSON response. Verify `GET /api/ai/models` lists the configured providers. ## What's next 1. **PR (frontend chantier)** — chatbot widget on `portal-shell` consuming the SSE endpoint. Will use `fetch` + `ReadableStream` parsing (not native `EventSource`, since POST is needed). Drag / fullscreen / suggestion UX carries forward from the stargate POC's `ChatbotWidget.tsx`. 2. **PR (post-v1)** — proto-drift CI gate that diffs `proto/apf-ai/` against an upstream tag of `apf-ai-service`. 3. **Coordinated amendment** — when the first production deployment is in scope, both repos record the same prod-hardening choice (signed `Principal` envelope vs mTLS) on the same date. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #196 |
||
|
|
3f3f47317b |
docs(adr-0024): ai service relay — gRPC dial + SSE bridge + POC principal (#194)
## Summary
Proposes [ADR-0024](docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md) — the integration contract between `apf_portal`'s BFF and the sibling `apf-ai-service` repository. The ADR bundles four tightly-coupled sub-choices: the wire transport between BFF and AI service, the wire transport between BFF and SPA for chat streaming, how the protos reach the BFF, and how user identity travels across the boundary in v1. **Status: `proposed`.** No code lands in this PR — the goal is to lock the contract before the implementation chantier starts.
The chosen design:
| Boundary | Choice |
|---|---|
| BFF ↔ AI service | Native **gRPC HTTP/2** via `@grpc/grpc-js`, h2c in dev / h2 + TLS in prod |
| BFF ↔ SPA (chat) | **`text/event-stream`** — one SSE frame per `ChatEvent` oneof case |
| BFF ↔ SPA (unary) | Plain JSON endpoints for `RagService.Search` + `ModelsService.ListModels` |
| Proto distribution | **Vendored** into `apps/portal-bff/src/grpc/proto/apf-ai/`, `ts-proto` codegen on demand, both `.proto` + generated `.ts` committed |
| Identity (POC) | **Unsigned `Principal { subject, roles[], attributes{} }`** in the proto body — mirrors `apf-ai-service`'s ADR-0010 |
| Production hardening | Choice between signed envelope and mTLS — **explicitly deferred** until first production deployment is in scope |
## What lands
- `docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md` — new MADR-formatted ADR with the four sub-choices, decision drivers, considered options, consequences, confirmation criteria, open production-hardening question, and the related-ADRs map.
- `docs/decisions/README.md` — one new index row for ADR-0024 (`proposed`, tags `backend, security, observability`, 2026-05-19).
No source-code changes. No `CLAUDE.md` update — the ADR stays in `proposed` until reviewed, so the accepted-ADRs roll-up at the top of `CLAUDE.md` stays at 0001 → 0023. Promotion to `accepted` lands in the same PR that ships the first implementation chantier (proto vendor + `AiClientModule`), at which point `CLAUDE.md` gets the "0024 accepted" line.
## Notes for the reviewer
- **Why bundle four sub-choices in one ADR rather than four.** They couple tightly: the SPA-facing transport choice depends on the BFF-facing transport choice (gRPC-Web from the browser would dissolve the bridge layer entirely); the auth posture depends on having identity travel in the proto body (vendoring a different contract would change that); the proto-distribution choice depends on the contract being stable enough to vendor (a churning OpenAPI spec would push toward an SDK package). Splitting would force cross-ADR coordination on every revision. The ADR keeps a separate "Sub-choice" section per topic so each one stays reviewable on its own.
- **Out of scope deliberately.** The chatbot UI lives in a future frontend chantier; the role mapper (Entra groups → inclusive-expanded `roles[]`) is a separate proposed ADR; the ingestion-through-BFF path waits for the admin app's "manage AI corpus" surface; tool dispatch is wired but exercised against an empty registry in v1.
- **Hash-salt coordination is the one operational gotcha.** The same `HashUserIdService` salt has to land in both repos' deployment config so `apf-ai-service.audit_log.actor_id_hash` and `apf_portal.audit.events.actor_id_hash` produce identical values. Recorded as an open item in the ADR's "More Information" section; the deployment doc that distributes the secret is a v1-launch deliverable.
- **`apf-ai-service` cross-reference**. The ADR references `apf-ai-service/docs/adr/ADR-0010` (POC unsigned principal) and `apf-ai-service/docs/adr/ADR-0011` (mono-transport gRPC) as upstream anchors. Both are already accepted on the AI side. The "production hardening" decision will be a coordinated amendment in both repos on the same date.
- **No `DownstreamApiClient` (ADR-0014) reuse.** The OBO pattern in ADR-0014 targets *Entra-protected* downstreams that validate the user's access token. `apf-ai-service` is not Entra-protected — it accepts an unsigned Principal proto. The ADR explicitly calls this out so the reader does not expect symmetry with the Entra-protected downstream path.
- **Phasing recorded in the ADR's "More Information" section.** This PR is step (1) "ADR accepted". Steps 2–5 are separate PRs in order: client skeleton → bridge controller → frontend chatbot → proto-drift CI gate.
## Test plan
- [x] `pnpm run --silent prettier --check docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md` — passes (hook ran on commit).
- [x] Markdown links inside the ADR resolve to existing files (`0005`, `0009`, `0010`, `0012`, `0013`, `0014`, `0017`, plus `CLAUDE.md`).
- [x] Index row in `docs/decisions/README.md` follows the table's existing format (column count, tag vocabulary, date format).
- [x] No tag-vocabulary additions required — `backend`, `security`, `observability` are all in the existing vocab.
- [ ] **Review focus** — the four sub-choices and the production-hardening deferral. Code chantier is gated on this PR's acceptance.
## What's next (once accepted)
1. **PR — proto vendor + codegen + `AiClientModule` skeleton** — vendors the protos, wires `ts-proto` codegen, sets up the NestJS module with the metadata interceptor and the Principal mapper, all tested against an in-process fake gRPC server. No live endpoint yet.
2. **PR — `ai-bridge` controller** — `POST /api/ai/chat` (SSE), `GET /api/ai/rag/search`, `GET /api/ai/models`, live against `apf-ai-service` in the dev Compose stack.
3. **PR (frontend chantier)** — the chatbot widget on `portal-shell` consuming the SSE endpoint.
4. **PR (post-v1)** — proto-drift CI gate that diffs the vendored copy against the upstream tag.
5. **Coordinated amendment** — when the first production deployment is in scope, both repos record the same prod-hardening choice (signed envelope or mTLS) on the same date.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #194
|
||
|
|
2cdeb74341 |
feat(portal-bff): audit-stats endpoint — server-side aggregations with redis cache (#173)
## Summary
PR 1 of the tabs + full-result-charts chantier. New BFF endpoint `GET /api/admin/audit/stats` that computes the three chart aggregations server-side over the **full filtered set** (not the paginated slice the SPA currently feeds the charts with).
```
PR 1 (this one) — BFF endpoint + Redis cache + audit event + ADR-0013 amendment.
PR 2 — SPA: Tabs UX (Table / Charts) + replace the per-page computeds
with calls to this endpoint.
```
## What lands
### New route — `GET /api/admin/audit/stats`
```ts
GET /api/admin/audit/stats?eventType=...&audience=...&outcome=...
&subjectPrefix=...&createdAtFrom=...&createdAtTo=...
&actorIdHash=...
→ {
dailyVolume: [{ day: 'YYYY-MM-DD', count }],
outcomeBreakdown: [{ outcome, count }],
eventTypeByDay: [{ day, eventType, count }],
total // sum of dailyVolume.count, drives the donut centre
}
```
Same filter shape as the existing `GET /api/admin/audit` minus pagination — the stats endpoint always aggregates the whole filtered set. `@RequireAdmin` gated (per [ADR-0020](docs/decisions/0020-portal-admin-app.md)). Time bound respects the filters strictly per the chantier brief: no filter → aggregates across the full audit retention (365 days per [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md)). The Redis cache below absorbs repeated heavy queries.
### New service — [`AuditStatsReader`](apps/portal-bff/src/admin/audit-stats.service.ts)
Mirrors `AuditReader`'s posture:
- Every query inside a transaction that opens with `SET LOCAL ROLE audit_reader`. SELECT-only on `audit.events` even if the BFF's connection is otherwise privileged.
- Parameterised SQL only. Filter values flow through positional parameters, never concatenated.
- Three `GROUP BY` queries scoped by the same `WHERE` clause:
- `date_trunc('day', created_at)::date AS day, COUNT(*) GROUP BY day`
- `outcome::text, COUNT(*) GROUP BY outcome`
- `date_trunc('day', created_at)::date AS day, event_type, COUNT(*) GROUP BY day, event_type`
### Redis cache — 5-minute TTL per filter-hash
- Cache key: `audit:stats:<sha256(canonical-JSON of filters), 16 hex chars>`. Sorted-keys canonicalisation so the same filters in different argument orders map to the same key.
- TTL: 300 s. Audit rows are append-only so past aggregations are stable; new events are continuously inserted, so admins see at most 5-minute-stale aggregations — acceptable for "approximate dashboard" usage, not for "did the last event just land" debugging (use the list endpoint for that).
- Cache writes are best-effort — a Redis-write failure does not fail the response. The DB read already happened; the next call rebuilds the cache.
- The cache *write* path is covered by spec; the cache-hit shortcut path is covered too (skips the DB transaction entirely).
### New audit event — `admin.audit.stats.query`
Mirrors `admin.audit.query` in posture (every admin read is auditable per ADR-0020 §"Read actions ... to deter fishing expeditions") with two differences:
- Distinct `event_type` so an auditor can spot "scanned aggregations" vs "paged through rows" — different observation signals (the stats endpoint can sweep millions of rows in one call; the list endpoint is bounded by `MAX_LIMIT=200`).
- Payload carries `total` (size of the aggregated set) instead of `resultCount` — stats responses don't paginate, the value carries more "size of scan" signal.
### Light amendment — [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md)
Two additions:
- New **"Reader endpoints"** subsection that enumerates the two-endpoint reader surface (list + stats), documents the Redis-cache caveat, and points at the new `admin.audit.stats.query` event family.
- The "events emitted in v1" table grows four rows it was previously missing on `main`: `admin.access_denied`, `admin.audit.query`, `admin.audit.stats.query`, `admin.users.query`.
No supersession, no new ADR. The decision shape (server-side aggregation + Redis cache + new audit event family) was settled in chat via `AskUserQuestion` before the implementation started; recording it here keeps the ADR honest without spawning a full ADR-0024 for what's essentially an extension of ADR-0013's reader surface.
## Notes for the reviewer
- **Why not factor `buildWhere` into a shared helper between `AuditReader` and `AuditStatsReader`?** Considered. The two readers' shapes diverge in non-trivial ways: `AuditReader` adds `LIMIT/OFFSET` parameters appended to the same parameter array, `AuditStatsReader` runs three queries that all share the same `WHERE` (no further params). A shared helper would have to either expose both shapes or hand back the raw clauses + params for callers to assemble — at which point the abstraction earns its weight back. Two ~50 LOC copies today, extraction when a third reader lands or when the shape diverges further.
- **Why not cap the time window when no filter is provided?** Honest disclosure beats clever defaults. The list endpoint also returns "everything matching the filters" with no protective cap; the stats endpoint follows the same posture. The Redis cache absorbs the cost when the same heavy query lands repeatedly; an admin running unfiltered queries at high rate will see flat latency after the first call. If we later observe a real perf issue, a `windowDays` parameter is a smaller change than retrofitting one across the API.
- **Why a `text` cast on `outcome` in the SQL?** Prisma's Postgres enum types come back as JS strings already, but the `outcome` column carries a Postgres enum (`audit.AuditOutcome`). The explicit `::text` is defensive — `$queryRawUnsafe`'s typing isn't enum-aware, and the cast keeps the projection unambiguous regardless of the driver's row-shape inference.
- **Why does the date round-trip through `Date.toISOString().slice(0, 10)`?** `date_trunc('day', ...)::date` returns a Postgres `date` that node-postgres surfaces as a JS `Date` at UTC midnight. The default `toJSON` serialises the full ISO timestamp with the timezone offset — which is not what the chart x-axis wants. Slicing to `YYYY-MM-DD` matches the SPA's chart bucket convention exactly.
- **No mention of the `actorIdHash` audit row for the stats endpoint?** It's the same hash flow as `adminAuditQuery` — the `actor.oid` from the session goes through `HashUserIdService` per ADR-0012's salt-based pseudonymisation. The same flow is exercised by the existing `adminAuditQuery` tests; the new `adminAuditStatsQuery` method just routes to `recordEvent` with a different `eventType`.
## Test plan
- [x] `pnpm nx test portal-bff` — **414 specs pass** (was 401; +13 new: 8 `AuditStatsReader` service + 5 controller `stats` endpoint).
- [x] `pnpm nx run portal-bff:lint` — clean.
- [x] `pnpm nx build portal-bff` — clean (webpack).
- [ ] **Manual smoke** — `pnpm nx serve portal-bff`, sign in to portal-admin with `Portal.Admin`:
- `curl http://localhost:3000/api/admin/audit/stats --cookie-jar /tmp/admin` returns the three projections.
- Verify the `admin.audit.stats.query` row in `audit.events` after the call (`SELECT * FROM audit.events WHERE event_type = 'admin.audit.stats.query' ORDER BY occurred_at DESC LIMIT 1`).
- Hit the endpoint twice in quick succession with the same filters → second call shows < 5 ms latency (cache hit, no DB transaction).
- Hit it with different filters → first call hits DB, second cache, third with same filters → cache hit.
- Stop Redis (`./infra/local/dev.sh stop redis`), hit the endpoint → still succeeds (cache miss + write swallowed), comes back live from DB.
## What's next
PR 2 — SPA Tabs UX (Table / Charts) + replace `dailyVolume() / outcomeBreakdown() / dailyByEventType()` (currently computed from `page().items`) with calls to this endpoint. The three computeds become signals filled by the HTTP call; the chart components on the Charts tab consume them unchanged.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #173
|
||
|
|
7ee7b2dadf |
docs(adr-0023): charts and dashboards — d3 + observable plot (#170)
## Summary
Records the decision to use **D3 + Observable Plot**, wrapped in a new `libs/shared/charts/`, as the chart toolkit shared by `portal-shell` and `portal-admin`. ADR-only — implementation lands as the next chantier(s).
This is staged as a 3-PR chantier per the agreed plan:
| PR | Périmètre |
| --- | --- |
| **PR 1 (this one)** | ADR-0023 — decision + a11y contract + bundle plan. |
| PR 2 | `libs/shared/charts/` foundations + 3 starter components (`<lib-bar-chart>`, `<lib-donut-chart>`, `<lib-stacked-bar-chart>`). |
| PR 3 | Integration on the `/audit` page — daily-volume bar + outcome-breakdown donut + event-type-over-time stacked bar. |
## What lands
### [`docs/decisions/0023-charts-d3-observable-plot.md`](docs/decisions/0023-charts-d3-observable-plot.md)
Full MADR 4.0.0 record. Highlights:
- **Choice**: D3 + Observable Plot, both from Mike Bostock / Observable Inc., both MIT, both past 1.0. Plot covers ~80 % of standard charts in declarative one-liners; D3 stays the escape hatch for bespoke viz (heatmap, sankey, …) inside the same lib.
- **Why not D3 alone**: ~250 LOC per chart × 4-5 types × a11y discipline = sustained code investment before the first dashboard ships.
- **Why not ECharts / Chart.js**: 600 KB minified + canvas-rendered + an `aria` plugin afterthought (ECharts), or narrower vocabulary + brittle dark-mode (Chart.js). Both furthest from the Angular-Signals-zoneless idiom the rest of the workspace runs on.
- **A11y contract** is baked into `_internal/` (palette, tabular fallback, SVG `<title>` / `<desc>` builders) so every chart inherits WCAG 2.2 AA + AAA-targeted compliance from the lib, not from contributor discipline. Six commitments, each unit-tested per chart component.
- **Bundle plan**: ~65 KB gzip added to a chart-bearing lazy chunk (d3 modules tree-shaken + Plot + thin wrapper) — well under [ADR-0017](docs/decisions/0017-performance-budgets-lighthouse-ci.md)'s 100 KB cap.
- **Component contract**: every `<lib-*-chart>` exposes the same Signal-based input shape (`[data]`, `[caption]`, `[description]`, `[ariaLabel]`, `[colorScheme]`) regardless of whether Plot or raw D3 powers the rendering.
### [`docs/decisions/README.md`](docs/decisions/README.md)
ADR-0023 added to the index table.
### [`CLAUDE.md`](CLAUDE.md)
- "Architecture (recorded in ADRs)" gains a "Charts + dashboards" bullet describing the lib + a11y baseline + bundle posture.
- "Repository status" bumps the ADR range to `0001 → 0023`.
- "Still on the roadmap" gains the charts implementation entry pointing at this ADR.
## Notes for the reviewer
- **Why honour the user's D3 preference rather than recommend pure ECharts?** D3 (and by extension Plot) is the closest match to the project's tech bar ("stable, recognized, battle-tested") for data-viz on the web; it's also the user's stated preference, and Plot's higher-level layer eliminates the "250 LOC per chart" cost that would otherwise push us toward an alternative. The ADR explicitly walks through ECharts + Chart.js as runners-up so future challengers see the trade-offs we chose against.
- **Why a single shared lib rather than per-app charts?** Both SPAs (portal-shell + portal-admin) will host dashboards. The chart vocabulary, a11y contract, palette, and theme integration are identical between the two — duplicating into app-local code would invite drift. The lib stays at `libs/shared/charts/` next to `libs/shared/ui/`.
- **Why the `_internal/` folder for cross-cutting code?** Single source of truth for the colour palette and the a11y plumbing. A lint rule (added in PR 2) will ban consumers from importing `d3-scale-chromatic` directly so the colour-blind-safe palette stays the only path.
- **Why no ADR amendment to ADR-0016 / ADR-0017?** Both are binding constraints, not superseded. The new ADR operationalises both for the chart surface; cross-references in the "Related ADRs" section make that explicit.
## Test plan
- [x] ADR validates as MADR 4.0.0 (frontmatter, section order, tag vocabulary).
- [x] No code touched — lint / test / build matrix unaffected.
- [x] `docs/decisions/README.md` index updated in the same change per the [ADR conventions](docs/decisions/README.md#conventions).
- [ ] Review for trade-off accuracy: are the bundle estimates fair? Is the "Plot covers ~80 % of standard charts" framing defensible against the user's mental model of D3?
- [ ] Implementation chantier (PR 2) lands directly behind this if accepted: `pnpm add -w d3 @observablehq/plot @types/d3`, `libs/shared/charts/` scaffold via `pnpm nx g @nx/angular:library --name=shared-charts --directory=libs/shared/charts --standalone=true --unitTestRunner=vitest-analog --tags="scope:shared,type:shared" --no-interactive`, then the 3 starter components.
## What's next
If accepted as-is, PR 2 (lib foundations + 3 starter components) follows. If a reviewer wants to push back on D3-vs-ECharts or on the a11y contract's strictness, this is the right PR to surface that — no implementation has started.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #170
|
||
|
|
2dbf2d8ce4 |
fix(docs): cap vite below 7 (rolldown-vite) and pin mermaid transitives (#161)
## Summary `pnpm docs:dev` failed after #159's transitive-vuln fix landed. Two distinct symptoms in the same log : 1. **VitePress refused to boot** — `VitePress v1 is not compatible with rolldown-vite. Use VitePress v2 instead.` The override added in #159 (`vite@<6.4.2 → >=6.4.2`) had no upper bound; pnpm resolved vitepress's `vite ^5.0.0` constraint up to **vite 7.3.2**, which uses the new rolldown bundler. VitePress 1.x explicitly rejects rolldown-vite. 2. **Resolution-warning flood** — Even on a fallback port, the console showed `Failed to resolve dependency: dayjs / debug / @braintree/sanitize-url / cytoscape / cytoscape-cose-bilkent` from `optimizeDeps.include`. The vitepress-plugin-mermaid wrapper injects those into the optimizer's include list, but under pnpm's strict isolation they aren't reachable from the workspace root (transitives of mermaid, never hoisted). ## What lands ### 1. Vite override is now an **unconditional** `>=6.4.2 <7` range [`package.json`](package.json): ```diff - "vite@<6.4.2": ">=6.4.2", + "vite": ">=6.4.2 <7", ``` The selector form (`vite@<6.4.2`) was a no-op once vite had resolved into 7.x — the override only activates when the resolved version is _in_ the vulnerable range. Vite 7 is ≥ 6.4.2, so the override stayed dormant and rolldown-vite slipped through. The unconditional form forces a downgrade across every consumer (vitepress, `@nx/vite`, `@analogjs/vite-plugin-angular`, Vitest, etc.). All six top-level projects still lint, test, and build under vite 6.4.2. **Trade-off acknowledged**: we're now pinning the whole workspace to vite 6.x to keep VitePress 1.x happy. The day VitePress 2 ships a 1.0 (currently in beta), we revisit and let vite advance again. Tracked as a soft follow-up. ### 2. `optimizeDeps.include` simplified to `['mermaid']` [`docs/.vitepress/config.mts`](docs/.vitepress/config.mts): ```diff - include: ['mermaid', 'dayjs', 'debug', '@braintree/sanitize-url'], + include: ['mermaid'], ``` Vite 6's dep optimizer walks Mermaid's transitives automatically once Mermaid itself is in `include`. The explicit child list from #157 was carried forward in the rolldown attempt and tripped on vite's stricter resolver — collapsing it now both removes the noise and matches what the plugin's docs recommend for vite 6. ### 3. Mermaid transitives pinned as top-level devDeps [`package.json`](package.json): ```diff + "@braintree/sanitize-url": "^7.1.2", + "cytoscape": "^3.33.3", + "cytoscape-cose-bilkent": "^4.1.0", + "dayjs": "^1.11.20", + "debug": "^4.4.3", ``` These are already in `node_modules` (pulled in by mermaid). Declaring them at the workspace root makes them reachable from `optimizeDeps.include` under pnpm's strict isolation, which silences the five "Failed to resolve dependency" warnings the plugin's wrapper produced. Cost: five extra devDep lines in `package.json` whose only purpose is to make the optimizer happy. Acceptable — they don't influence the resolved tree, just the resolver's reachability rules. ## Notes for the reviewer - **Why not bump VitePress 1 → 2?** VitePress 2 is still beta. Per [CLAUDE.md](CLAUDE.md) §"Project rules": pre-1.0 dependencies and one-maintainer projects are rejected unless an ADR justifies the exception. ADR-0022 already records VitePress 1.6.4 as the chosen baseline; switching to a beta on the very first follow-up PR would burn the rationale. - **Why an unconditional vite range, not a tighter selector?** The selector form (`vite@vulnerable-range → patched-range`) is the standard pattern when the parent dep's own range *includes* a patched version — pnpm picks it naturally and the override never fires. Here vite's 5.x branch was never patched (5.4.21 stayed vulnerable; vite team moved on to 6.x), so we need to force the downgrade from 7.x to 6.x regardless of the previous resolution. An unconditional override is the cleanest expression of that intent. - **Why not extract the mermaid-transitive pins into the ADR-0022 trail?** They're plumbing for the plugin wrapper, not an architectural decision worth recording. If the plugin ships a fix that removes the include list, these can be removed without consequence. Pinning them is reversible. ## Test plan - [x] `pnpm install` clean; lockfile changes reflect vite 6.4.2 across all consumers. - [x] `pnpm audit --audit-level=moderate` — **No known vulnerabilities found**. - [x] `pnpm docs:dev` — server boots cleanly on `:5173`, no warnings, home + ADR-0009 page return 200. - [x] `pnpm docs:build` — clean build in ~9 s (back to Rollup-based timings; the rolldown 3.87 s we saw briefly was the incompatible path). - [x] `pnpm exec nx run-many -t lint test --projects=portal-shell,portal-admin,portal-bff,shared-ui,shared-state,feature-auth` — 12/12 tasks green under the vite 6 downgrade. - [x] `pnpm exec nx build portal-shell` — clean Angular production build; no Vite 6 incompatibility surfaced. - [ ] **Manual smoke (visual)** — `pnpm docs:dev`, open `http://localhost:5173`, navigate to `/decisions/0009-…` and `/architecture`, confirm Mermaid diagrams render inline (this is the actual UX the user opened the issue on). Dark mode toggle still flips diagrams. ## Follow-ups (optional) - When VitePress 2 reaches 1.0 (`vue/vitepress > releases`), revisit this override and let vite resume its mainline cadence. - If the next Renovate cycle proposes a `cytoscape` / `dayjs` / `debug` major bump that VitePress 1.x can't keep up with, the pins above act as the safety net — Renovate will open a PR rather than silently break the dev server. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #161 |
||
|
|
e3a495ab46 |
docs(development): refresh after phase-3a + add topology / trace diagrams (#160)
## Summary `docs/development.md` drifted as `portal-admin` shipped, the docs site landed, Prisma stayed pinned at 6.x, and a fair chunk of the phase-2 "to come" roadmap quietly turned into "shipped". Surgical refresh of the affected sections + two Mermaid diagrams where prose alone wasn't carrying the cognitive load. ## What changed ### Section 1 — Repo layout - `apps/portal-admin/` + `apps/portal-admin-e2e/` added (both exist on `main` since #134, were never reflected in the tree). - `prisma.config.ts` removed (phantom — Prisma 6.x doesn't ship one). The "Prisma 7" tag corrected to "Prisma 6.x" with the [ADR-0006](docs/decisions/0006-persistence-postgresql-prisma.md) pin reference. - `docs/index.md`, `docs/architecture.md`, `docs/.vitepress/` added. - New workflows listed: `docs-site.yml`, `renovate.yml`. ### Section 3 — Initial setup, new diagram Mermaid `flowchart` of the local-dev topology. Host-side dev servers (`portal-shell:4200`, `portal-admin:4300`, `portal-bff:3000`, `docs:5173`) ↔ Compose containers (Postgres, Redis, OTel) ↔ viewer profiles (Jaeger, pgweb). Replaces a ports/services context that was scattered across §3 and §5. ### Section 4 — Daily commands - `portal-admin` added to serve / test / generate examples. - Single-test-file recipe corrected: Nx's vitest executor rejects `--testFile` (we hit this empirically while debugging the sidebar spec for #151). The new wording recommends positional path for Vitest, `--testPathPattern=…` for Jest. - **New "Documentation site" subsection** with the three commands (`docs:dev`, `docs:build`, `docs:preview`), plus the recipe for adding an ADR. ### Section 5 — Observability, new diagram Mermaid `sequenceDiagram` showing how a click becomes a trace: browser `user_interaction` span → `traceparent` header → BFF span → Pino log line with matching `trace_id` → OTLP batch → Jaeger UI. Anchors the prose's "trace_id is the correlation point" rule with a visual. ### Section 6 — Renovate New subsection **"Transitive vulnerabilities — `pnpm.overrides`"**. Documents the pattern from #159 so the next contributor hitting a transitive vuln (Renovate silent, dashboard empty) has the playbook on hand without re-discovering it. ### Section 9 — Sections to come Restructured into two groups: - **Code shipped — doc to write** (Auth dev-loop, session inspection, MFA step-up debugging, admin surface walkthroughs, audit-log workflow, downstream API recipe, OpenAPI/Scalar workflow, capabilities-driven SPA UX). The code is on `main`; only the prose is missing. Each entry now cites the PR(s) that landed the implementation. - **Not yet** (component patterns library, a11y testing workflow, perf debugging, release workflow, GitLab migration runbook). Both code and doc still pending. ## Notes for the reviewer - **Why diagrams now, not at the next chantier?** Two pieces of context were genuinely easier to grasp visually than as prose lists: the local-dev topology (which port goes where), and the trace-correlation flow. The other sections (Renovate, conventional commits, CI gates) already read well as tables — adding diagrams there would be ornament, not signal. - **Why two diagrams rather than ten?** Per the user instruction "use diagrams when useful" — restraint applies. The two added are the ones that *replace* explanatory prose rather than add to it. - **Why didn't I rewrite the "to come" roadmap from scratch?** The phase mapping was useful intent — kept the same structure, just moved entries between the two columns as code-shipping unlocked them. Future re-shuffles stay cheap. - **The doc still has phase-1 framing in places** (e.g. the "this doc starts as a phase-1 reference" paragraph in §9). I left it — promoting the doc to "phase-3a reference" is a larger editorial pass than this refresh deserves; the new diagrams + section-9 split already do the practical work. - **Open questions deferred to a future pass**: the §2 "Prerequisites" table doesn't mention the docs site (`pnpm docs:dev` adds nothing to the prereqs list — just Node + pnpm, both already required). Leaving the table as is. ## Test plan - [x] `rm -rf docs/.vitepress/{cache,dist} && pnpm docs:build` — clean, 6.3 s. - [x] Mermaid renders inside `docs/.vitepress/dist/development.html` — both new diagrams produce `class="mermaid"` markers. `grep -c 'class="mermaid"\|<svg' development.html` → **2**. - [ ] Visual smoke: `pnpm docs:dev`, navigate to `/development`, confirm both diagrams render (topology with port labels readable, sequence diagram with the trace flow). Toggle dark mode, confirm diagrams flip theme. - [ ] Spot-check the "Code shipped — doc to write" table — for any contributor reading this PR, do the PR-number citations match their memory of when each chantier landed? (`auth ≈ ADR-0009 series`, `admin ≈ #127, #128, #134, #136, #140–142`, `downstream ≈ #137–139`, `OpenAPI ≈ #143`, `capabilities ≈ #151`). ## What's next The two largest "doc to write" entries (Auth dev-loop, Audit-log inspection workflow) are good candidates for the next docs chantier — both have shipped code, RSSI-relevant content, and would benefit from a guided walkthrough. Not blocking anything; pick when the team has bandwidth. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #160 |
||
|
|
d94df427a1 |
fix(docs): pre-bundle mermaid deps so the dev server resolves dayjs/cytoscape (#157)
## Summary `pnpm docs:dev` rendered a blank page with this thrown by the browser on the first navigation: ``` Uncaught SyntaxError: The requested module '/@fs/.../node_modules/.pnpm/dayjs@1.11.20/node_modules/dayjs/dayjs.min.js?v=…' does not provide an export named 'default' (at chunk-AGHRB4JF.mjs?v=…:9:8) ``` Root cause: Mermaid 11 (transitive dep of `vitepress-plugin-mermaid` shipped in #154) pulls in `dayjs`, `cytoscape`, `debug`, `@braintree/sanitize-url` — each ships a CommonJS `main` field with no `default` ESM export. Vite's dev server resolves these modules eagerly as ESM and the browser blows up before the home page renders. The **production build was unaffected** — Rollup's plugin pipeline already wraps CJS deps for ESM consumers. `docs:build` shipped clean HTML in #154 and the CI Mermaid-fence (`grep class="mermaid"` in ADR-0009's HTML) passed precisely because it inspects the prod bundle, not the dev-server output. ## What lands [`docs/.vitepress/config.mts`](docs/.vitepress/config.mts) — adds a single `vite.optimizeDeps.include` block: ```ts vite: { optimizeDeps: { include: ['mermaid', 'dayjs', 'debug', '@braintree/sanitize-url'], }, }, ``` `optimizeDeps.include` tells Vite to pre-bundle those modules through esbuild at dev-server boot, applying the same CJS→ESM interop wrapper Rollup uses in prod. The dev server now serves a working `localhost:5173` and Mermaid diagrams render. ## Notes for the reviewer - **Why these four names specifically?** They are the Mermaid deps that ship CJS-only entrypoints. Adding `'mermaid'` alone is sometimes enough because Vite walks the dep tree, but the plugin's own README + a handful of upstream issue threads recommend listing the leaf CJS modules explicitly so the optimizer doesn't miss them on a cold cache. Cheap, defensive. - **Why didn't the CI gate catch this?** The `Assert Mermaid renders` step in `.gitea/workflows/docs-site.yml` greps `docs/.vitepress/dist/` — the production-build output. The bug only manifests on the dev server's runtime-pre-bundling path. Catching it in CI would require booting `docs:dev` headlessly and curling the page; not worth the workflow weight for a once-per-major-upgrade class of issue. Manual `pnpm docs:dev` smoke is the right gate for now. - **Why not bump dayjs / mermaid?** Dayjs's package shape is a long-standing upstream quirk (the `main` points at the UMD/CJS bundle); fixing it upstream would be a breaking change for non-bundler consumers. Mermaid upstream is aware; their fix has historically been "tell your bundler to pre-bundle us", which is exactly what `optimizeDeps.include` does. ## Test plan - [x] `rm -rf docs/.vitepress/cache && pnpm docs:dev` — server boots, `curl http://localhost:5173/` returns 200. - [x] `rm -rf docs/.vitepress/{cache,dist} && pnpm docs:build` — clean prod build in ~10 s, Mermaid SVG still present in ADR-0009's HTML (regression fence passes). - [ ] Manual smoke: with the fix applied, navigate `localhost:5173` → home → `/decisions/0009-…` → confirm the OIDC sequence diagram renders inline, no console errors. Toggle dark mode, confirm diagrams flip theme. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #157 |
||
|
|
7579b25dfe |
feat(docs): vitepress site for docs/, mermaid rendering, ci build workflow (#154)
## Summary
Implementation of [ADR-0022](docs/decisions/0022-docs-site-vitepress.md). Stands up the static documentation site that renders `docs/**/*.md` (architecture diagrams, daily-dev guide, ADRs, onboarding) via **VitePress + `vitepress-plugin-mermaid`**, behind a Gitea Actions build gate.
Local dev: `pnpm docs:dev`. Full build: `pnpm docs:build` (~9 s, output in `docs/.vitepress/dist/`).
## What lands
### Dependencies
`vitepress 1.6.4`, `vitepress-plugin-mermaid 2.0.17`, `mermaid 11.15.0` — workspace devDependencies. No runtime impact on `portal-shell` / `portal-admin` / `portal-bff`.
### [`docs/.vitepress/config.mts`](docs/.vitepress/config.mts)
The single source of truth for the site. Highlights:
- **`srcExclude`** drops `docs/README.md` (git/IDE-only index per ADR-0022's option A) and `docs/decisions/template.md` (authoring scaffold).
- **`rewrites`** maps `decisions/README.md` → `decisions/index.md` so `/decisions/` resolves to the curated tag-grouped landing while the source filename stays git-conventional.
- **`ignoreDeadLinks`** skips:
- `localhost:*` URLs (Jaeger, OTLP — only resolve in a live dev session),
- cross-repo references (`../CLAUDE`, `../../apps/**`, `../../infra/**`, `../../notes/**`) — intentional from git/IDE consumers; not the site's job to render them,
- excluded targets (`./template`, `./README`) — file exists in the repo, just not in the site.
- **Auto-sidebar for `/decisions/`** — `adrSidebarItems()` walks `docs/decisions/00*-*.md` and emits sorted `ADR-NNNN — title` entries. Adding an ADR is a single-file change, no `config.mts` edit.
- **Hand-curated top-level nav** (Development, Architecture, Decisions, Onboarding).
- **Mermaid via `withMermaid()`** with `securityLevel: 'strict'` so diagrams can't inject arbitrary HTML.
### [`docs/index.md`](docs/index.md)
VitePress Hero landing with four feature cards (Architecture, Decisions, Development, Onboarding).
### [`docs/development.md`](docs/development.md) — two surgical fixes
- Line ~5: `[setup/](setup/)` → `[setup/01-wsl-terminal-setup.md](setup/01-wsl-terminal-setup.md)`. Folder-style links don't resolve cleanly under `cleanUrls: true`; pointing at the first onboarding page is both correct and useful.
- Line 330: wrap `${{ github.* }}` in `<code v-pre>…</code>`. VitePress runs every Markdown file through the Vue template compiler, which sees the inline `{{ … }}` as an interpolation. `v-pre` keeps the literal text intact. The rest of the source is unaffected.
### [`package.json`](package.json)
Three new scripts:
```
docs:dev → vitepress dev docs
docs:build → vitepress build docs
docs:preview → vitepress preview docs
```
Pure pnpm scripts, no Nx project — the site has no cross-project dependency graph to track.
### [`.gitea/workflows/docs-site.yml`](.gitea/workflows/docs-site.yml)
Triggers on push to `main` and on PR, scoped by `paths:` to `docs/**`, `package.json`, `pnpm-lock.yaml`, and the workflow itself. Three steps:
1. `pnpm install --frozen-lockfile`
2. `pnpm docs:build`
3. Regression fence: `grep` ADR-0009's rendered HTML for `class="mermaid"` or `<svg>` so a silent Mermaid-plugin breakage on a major upgrade fails the workflow rather than ship a site with raw code blocks where diagrams should be.
4. On push only: upload `docs/.vitepress/dist/` as a `docs-site` artifact (30-day retention). The actual rsync to the static host lands when the future infrastructure ADR locks the deployment target.
### [`.gitignore`](.gitignore)
Excludes `docs/.vitepress/{cache,dist}/` so local builds don't leak into commits.
## Notes for the reviewer
- **Why `config.mts` and not `config.ts`?** VitePress is ESM-only, and `vitepress-plugin-mermaid` follows. Vite loads `.ts` config files via its CJS bundler in this workspace's setup and chokes on the ESM imports. `.mts` flips the loader to ESM and the build succeeds. Same pattern is used elsewhere in the workspace (`jest.config.cts`, app `vite.config.mts`).
- **Why no Nx project (`docs/project.json`)?** The doc site has no Nx-trackable dependencies (it consumes `.md` files, not TypeScript projects). Putting it in the Nx graph adds ceremony with no caching benefit — VitePress's incremental rebuilds are sub-second already, and the site never has cross-project `affected` semantics. Pure pnpm scripts keep the surface small.
- **Why the regression fence on Mermaid?** ADR-0022 §"Confirmation" promises it. The plugin is a community dep (sub-1.0 wrapper around the official Mermaid renderer); a major upgrade or a Mermaid runtime change could leave fenced ` ```mermaid ` blocks rendered as raw code without anyone noticing — until an RSSI clicks ADR-0009 and sees no diagram. Cheap grep gate, real signal.
- **Why upload as artifact, not deploy?** Per [ADR-0022](docs/decisions/0022-docs-site-vitepress.md) §"Deployment & CI": the host (`docs.portal.apf.fr` or a sub-path) is provisional. Locking an rsync target now would couple this PR to a not-yet-made infra decision. Artifact upload is the staging mechanism — manual drop on the host until the infrastructure ADR formalises the target.
- **Why `ignoreDeadLinks` rather than fixing every cross-repo reference?** The cross-repo links are genuinely useful from a git/IDE perspective (where the docs/ markdown is browsed alongside the rest of the codebase). Rewriting them to `https://git.unespace.com/julien/apf_portal/src/branch/main/…` would make them work on the site but lose the IDE quick-jump. Skipping at site-build time is the right trade-off — the site reader gets a graceful "link doesn't exist here" if they click, the IDE reader gets a working jump.
## Test plan
- [x] `pnpm docs:build` succeeds in ~9 s. Output at `docs/.vitepress/dist/` contains an `index.html`, every ADR, the development guide, the architecture diagrams, and the three setup pages.
- [x] Mermaid renders: `grep 'class="mermaid"' docs/.vitepress/dist/decisions/0009-…html` returns a match.
- [x] `pnpm exec nx run-many -t format:check lint test build` for the 6 main projects — 18/18 tasks green, no Nx regression from the new top-level config.
- [ ] **Manual smoke**: `pnpm docs:dev`, open `http://localhost:5173`, walk through:
- Landing renders Hero + 4 feature cards.
- Search box returns hits for "audit", "MFA", "OBO".
- `/decisions/0009-…` renders the OIDC sequence diagram (Mermaid SVG, not raw text).
- `/decisions/0010-…` ERD or `/architecture` C4 diagrams likewise.
- Dark-mode toggle flips diagrams to dark theme without page reload.
- Sidebar shows the 22 ADRs auto-listed under `/decisions/`.
- The "Decisions" curated index at `/decisions/` lists ADRs by tag (no regression on the source markdown).
## What's next
Once the deployment target is fixed (future infra ADR), wire the rsync step into the workflow — that lands as a small follow-up PR. Until then the artifact carries the bundle.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #154
|
||
|
|
aa8ad97feb |
fix(portal-admin): adr refs point at gitea, not the madr template repo (#153)
## Summary Records the decision to render `docs/**/*.md` as a separately-deployed static site using **VitePress + `vitepress-plugin-mermaid`**. This is **ADR-only** — the implementation (install + `.vitepress/config.ts` + `docs/index.md` + Gitea Actions workflow) lands as the next chantier. Splitting them keeps the decision review focused on the *why* before the *how*. ## What lands ### [`docs/decisions/0022-docs-site-vitepress.md`](docs/decisions/0022-docs-site-vitepress.md) Full MADR 4.0.0 ADR. Decision drivers, 4 considered options (VitePress, MkDocs Material, Docusaurus 3, Astro Starlight), site-structure mapping, Mermaid integration, deployment + CI plan, consequences, revisit triggers. Tags: `process`, `infrastructure`. **Key choices captured:** - VitePress wins on toolchain alignment (Vite already in the workspace via `@nx/vite`, `vite-plugin-angular`, Vitest). MkDocs Material was the strong runner-up; the Python runtime tax in Gitea Actions tipped the balance. - `vitepress-plugin-mermaid` for ```` ```mermaid ```` blocks ([ADR-0009](docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md) sequence + [architecture.md](docs/architecture.md) C4 are hard requirements). - Site sources entirely from `docs/`. Mapping: - `docs/index.md` (new, Hero layout) → `/` - `docs/development.md` → `/development` - `docs/architecture.md` → `/architecture` - `docs/decisions/README.md` → `/decisions/` (curated index, kept as section landing) - `docs/decisions/00NN-….md` → auto-listed in the sidebar by numeric prefix - `docs/setup/0N-….md` → `/setup/0N-…` - **Excluded** via `srcExclude`: `docs/README.md` (stays as the git-side / IDE-preview index — option A from the prior discussion) and `docs/decisions/template.md` (authoring scaffold). - Empty placeholder sections in `docs/README.md` (Operations runbooks, Security/perf/a11y rationales) are NOT pre-created as empty pages — they appear in the sidebar when real content lands. - Deployment: dedicated hostname (provisional `docs.portal.apf.fr`) behind the same Caddy reverse-proxy as the apps, fed by a new `.gitea/workflows/docs-site.yml` triggered on `docs/**` push. Exact hostname follows the future infrastructure ADR; not locked here. ### [`docs/decisions/README.md`](docs/decisions/README.md) ADR-0022 added to the index table. ### [`CLAUDE.md`](CLAUDE.md) Bumps the ADR range from `0001 → 0021` to `0001 → 0022`. New bullet in the "Architecture (recorded in ADRs)" section describing the docs-site choice in one paragraph. Implementation tracked in "Still on the roadmap" until the next PR lands it. ## Notes for the reviewer - **Why ADR before implementation?** The choice between VitePress / MkDocs Material / Docusaurus / Astro Starlight is exactly the "stable + recognized + innovative" trade-off [CLAUDE.md](CLAUDE.md) asks to document. Reviewing the rationale on its own (without dragging through the install diff) keeps the discussion focused. - **Why not surface ADRs inside `portal-admin`?** Audience mismatch — [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"Audience is disjoint" frames `portal-admin` around APF internal operators (CMS, audit, user directory), not architects. The full reasoning is in ADR-0022 §"Context and Problem Statement". - **Why two index artefacts (`docs/README.md` + future `docs/index.md`)?** Option A from the structure discussion. Each serves a distinct audience: `README.md` is the flat link list that renders well in IDEs / Gitea source view; `index.md` will be the VitePress Hero landing for the web audience. Light duplication, no maintenance pressure (the IDE one only needs updating when sections appear/disappear). - **Why `vitepress-plugin-mermaid` rather than Docusaurus's built-in Mermaid?** The community plugin is a sub-1.0 dependency on the wrapper (Mermaid itself is mature), but Mermaid is so mainstream that switching it out is a half-day rewrite if the plugin stalls. Trading that risk against Docusaurus's MDX-by-default footprint + React runtime is a net win. - **Why `process` + `infrastructure` tags?** Mirrors [ADR-0015](docs/decisions/0015-cicd-gitea-actions.md) (also a CI / deploy decision with content authoring implications) and is consistent with the [tag vocabulary](docs/decisions/README.md#tag-vocabulary). No new tag invented. ## Test plan - [x] `docs/decisions/0022-docs-site-vitepress.md` validates as MADR 4.0.0 (frontmatter, section order). Index in [`docs/decisions/README.md`](docs/decisions/README.md) updated in the same PR per [ADR conventions](docs/decisions/README.md#conventions). - [x] No code touched — `lint / test / build` matrix unaffected. - [ ] Review for trade-off accuracy: did I get MkDocs Material's strengths right? Is Astro Starlight's maturity argument fair? - [ ] Implementation chantier (next PR): `pnpm add -D vitepress vitepress-plugin-mermaid mermaid`, `docs/.vitepress/config.ts`, `docs/index.md`, `.gitea/workflows/docs-site.yml`, `package.json` scripts. Will land within the same week assuming this ADR holds. ## What's next If accepted as-is, the immediate follow-up is: 1. Install VitePress + the Mermaid plugin. 2. Author `docs/.vitepress/config.ts` with the sidebar shape spelled out in this ADR (auto-generated sub-sidebar for `/decisions`, hand-curated top-level). 3. Author `docs/index.md` (Hero layout). 4. Add the `docs-site` Gitea Actions workflow. 5. Wire the dev script (`pnpm docs:dev`) into `package.json` so contributors can preview locally. If reviewers want to push back on the toolchain choice (MkDocs Material in particular has a strong case for the theme polish), this is the right PR to surface that — implementation hasn't started. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #153 |
||
|
|
ee51efb688 |
feat(portal): /api/me/capabilities + cross-app menu links + real role label (#151)
## Summary PR 3 of 3 — final piece of the user-menu / profile / cross-app chantier. Closes the loop with the BFF capabilities endpoint, symmetric cross-app entries in both user menus, and a real role label in place of the hardcoded "Anonymous" widget on the portal-shell sidebar. | PR | Périmètre | | --- | --- | | PR 1 ✅ | Shared `UserMenu` dropdown + integration. | | PR 2 ✅ | `/profile` pages on both apps. | | **PR 3 (this one)** | `GET /api/me/capabilities` + real sidebar role label + cross-app menu entries. | ## What lands ### BFF — `GET /api/me/capabilities` New [`MeModule`](apps/portal-bff/src/me/me.module.ts) wiring a single endpoint: ```ts GET /api/me/capabilities → { canAccessAdmin: boolean } ``` Resolved against the user-portal session ([`portal_session`](apps/portal-bff/src/me/me.controller.ts) — the path-routed session middleware in `main.ts` already maps `/api/me/*` to that session). Returns 401 if no session is present, consistent with `/api/auth/me`. 5 specs cover the four state combinations + a regression-fence asserting the curated view never leaks the raw `roles` array. ### ADR-0009 amendment [`docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md`](docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md) — new **"Curated public view"** section codifies the design stance the user picked when we agreed the staging: > The `/auth/me` payload exposes a deliberately narrow projection of the session: `oid`, `tid`, `username`, `displayName`. **The raw `roles` claim is _not_ part of `/auth/me`** — it stays server-side […]. The SPA derives binary UX hints from a dedicated companion endpoint […]. The shape is intentional: the SPA can never reconstruct the raw role names from the curated view, so introducing additional internal-only roles […] does not widen the SPA-side surface. The routes table grows a row for `/me/capabilities`. ### Portal-shell — capabilities-driven UI - **New service** [`CapabilitiesService`](apps/portal-shell/src/app/services/capabilities.service.ts) (app-local, not in `feature-auth` — the admin app has its own roles channel via `/api/admin/auth/me` and would never use this). Signals: `capabilities`, `canAccessAdmin`. Fires `GET /me/capabilities` reactively via an `effect` that watches `auth.currentUser()`. Anonymous sessions short-circuit to the all-false default without a fetch — the BFF would 401 anyway. - **Header** ([`header.ts`](apps/portal-shell/src/app/components/header/header.ts)) — `userMenuItems` is now a `computed`. Profile + Settings stay unconditional; **"Open Portal Admin"** appears only when `canAccessAdmin()` flips true, with `href = environment.adminAppUrl`. Per ADR-0020 the two SPAs live on distinct origins, so this is a raw cross-origin anchor, not a routerLink. - **Sidebar** ([`sidebar.ts`](apps/portal-shell/src/app/components/sidebar/sidebar.ts) + [`sidebar.html`](apps/portal-shell/src/app/components/sidebar/sidebar.html)) — the hardcoded `Role: Anonymous` widget is replaced by a derived computed: - `Anonymous` when no session. - `Administrator` when `canAccessAdmin()` is true. - `User` otherwise (signed-in, no admin). The aria-label gains a `role` placeholder so screen readers hear the live value. ### Portal-admin — symmetric cross-app entry - **Header** ([`header.ts`](apps/portal-admin/src/app/components/header/header.ts)) — adds an unconditional `Open Portal Shell` row pointing at `environment.shellAppUrl`. Anyone able to reach portal-admin can reach portal-shell, so no capabilities check needed; admins always benefit from a one-click jump back to the end-user surface. ### Environments `adminAppUrl` and `shellAppUrl` added to the respective [`environment.ts`](apps/portal-shell/src/environments/environment.ts) files (dev defaults: `:4300` for admin, `:4200` for shell). Per-env siblings (staging / prod) will override the host once they exist, per ADR-0018. ### i18n | Key | EN source | FR target | | --- | --- | --- | | `header.userMenu.openAdmin` | Open Portal Admin | Ouvrir Administration APF Portal | | `sidebar.role.administrator` | Administrator | Administrateur | | `sidebar.role.user` | User | Utilisateur | | `sidebar.role.aria` | reshaped with `{role}` placeholder | reshaped likewise | Admin-side strings stay in English source per ADR-0020. ## Notes for the reviewer - **Why `CapabilitiesService` in the app, not in `feature-auth`?** Only `portal-shell` will ever call `/api/me/capabilities` — the admin SPA hits `/api/admin/auth/me` which already returns `roles`. Putting the service in `feature-auth` would publish a tree-shakable `providedIn: 'root'` injectable that ships in both bundles. Keeping it app-local makes the boundary explicit. - **Why a `computed` for `userMenuItems` rather than mutating an array in an `effect`?** Signals + computed = single source of truth. The shared `UserMenu` re-renders automatically when the items list changes (whenever capabilities flips). Less ceremony than maintaining a `WritableSignal<UserMenuItem[]>`. - **Why the `flushPendingEffects` test helper?** Zoneless apps rely on the signals scheduler to dispatch `effect()` callbacks via micro-task scheduling. `fixture.detectChanges() + whenStable()` once is not enough: the chain is `meReq.flush()` → `_state.set()` → effect scheduled → effect fires → `http.get()` queued. The helper loops 4× to give the scheduler enough rounds to settle before `expectOne(CAPABILITIES_URL)` looks up the request. - **Why no test for the dev URL values?** `environment.ts` is config that gets swapped at build time per ADR-0018; the values themselves are environmental. Asserting the dev value in a test would lock in a port (4200/4300) that's separately configured in `project.json`. ## Test plan - [x] `pnpm nx test portal-bff` — **401 specs pass** (was 396, +5 for `MeController`). - [x] `pnpm nx test portal-shell` — **40 specs pass** (was 35, +5: 3 sidebar role-label + 2 header admin-link). - [x] `pnpm nx run-many -t lint test build --projects=portal-shell,portal-admin,portal-bff,shared-ui,shared-state,feature-auth` — 18/18 tasks green, including the i18n-strict `portal-shell:build:production`. - [ ] Manual smoke (with a `Portal.Admin`-assigned account): - Sign in on portal-shell → sidebar reads `Role: Administrator`, user menu lists `Open Portal Admin` at the bottom (right above the Sign-out separator), clicking the link opens `localhost:4300`. - Sign in on portal-admin → user menu lists `Open Portal Shell`, clicking opens `localhost:4200`. - Sign in on portal-shell with a non-admin account → sidebar reads `Role: User`, `Open Portal Admin` is absent. - Sign out → sidebar reads `Role: Anonymous`, the menu collapses to its anonymous-state Sign-in button. ## What's next Chantier closed. The user-menu shape is now stable; further entries (notifications inbox, theme override, locale switcher inside the menu rather than the footer) plug into the existing `items` API without re-shaping the component. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #151 |
||
|
|
2480d0dd6d |
fix(portal-bff): align admin entra role name with Portal.Admin (#145)
## Summary The [`AdminRoleGuard`](apps/portal-bff/src/admin/admin-role.guard.ts) was matching on the literal `'admin'`, but the Entra app registration declares the admin app role with `value: "Portal.Admin"`. End result: an authenticated user with the role assigned in Entra still landed with `roles: []` in their session (claim simply not present in the id token), and every request to `/api/admin/audit` and `/api/admin/users` returned a **403**. Caught manually in the portal-admin SPA: login succeeded, sidebar links to "Audit log" / "User list" returned 403. The [`/api/admin/auth/me`](apps/portal-bff/src/admin/admin-auth.controller.ts) self-test confirmed the missing claim was the cause. ## What lands ### Constant value — single source of truth [`apps/portal-bff/src/admin/admin-role.guard.ts:18`](apps/portal-bff/src/admin/admin-role.guard.ts#L18): ```diff -export const ADMIN_ROLE = 'admin'; +export const ADMIN_ROLE = 'Portal.Admin'; ``` [`admin-role.guard.spec.ts`](apps/portal-bff/src/admin/admin-role.guard.spec.ts) already imports `ADMIN_ROLE` from the source rather than hardcoding the literal, so the guard contract spec rolls through unchanged. The fixtures elsewhere ([`auth.service.spec.ts`](apps/portal-bff/src/auth/auth.service.spec.ts), [`admin.controller.spec.ts`](apps/portal-bff/src/admin/admin.controller.spec.ts), [`admin-auth.controller.spec.ts`](apps/portal-bff/src/admin/admin-auth.controller.spec.ts), [`require-mfa.guard.spec.ts`](apps/portal-bff/src/auth/require-mfa.guard.spec.ts)) keep `roles: ['admin']` as fixture data — those tests exercise the extraction / serialization pipeline, which is role-value-agnostic; touching them would be incidental cleanup with no behaviour signal. ### Doc-comment refresh Inline references to the role name updated so future readers don't grep `'admin'` and find a phantom value: - [`admin-role.guard.ts`](apps/portal-bff/src/admin/admin-role.guard.ts) — class doc-block (3 mentions). - [`admin.controller.ts`](apps/portal-bff/src/admin/admin.controller.ts) — class doc-block + inline guard-contract comment. - [`audit.service.ts`](apps/portal-bff/src/audit/audit.service.ts) — `adminAccessDenied` doc-block (2 mentions). ### Documentation - [`docs/decisions/0020-portal-admin-app.md`](docs/decisions/0020-portal-admin-app.md) — 5 references to the role across §"How is admin access enforced", §"Auth — same Entra ID …", and the Consequences §. - [`docs/architecture.md`](docs/architecture.md) — note next to the C4 container diagram describing the admin entry gate. - [`CLAUDE.md`](CLAUDE.md) — "Admin application" project rule. ## Notes for the reviewer - **Why `Portal.Admin` rather than `admin`?** Operator's call on the Entra side. The `<Application>.<Role>` namespace is the conventional Entra App Role pattern when the directory may host roles for multiple applications, and `admin` alone is ambiguous in a directory shared across products. - **Why no migration / backfill?** The role value lives only in two places: Entra app-registration manifest (operator-managed) and the BFF constant (this PR). Existing Redis sessions captured `roles: []` (claim absent) — they'll naturally pick up the correct value on next sign-in. No persisted data references the old value. - **No ADR.** ADR-0020 §"How is admin access enforced" already commits to "Entra ID role claim + BFF guard"; the literal role string is an implementation detail the ADR happened to spell. Updated the ADR's prose to the new value to keep the doc honest, but the decision is unchanged. ## Test plan - [x] `pnpm nx test portal-bff` — **396 specs pass**, unchanged from `main`. The `AdminRoleGuard` contract spec (covers 401-on-no-session, 403-on-missing-role + audit emission, pass-through-on-role-present) imports `ADMIN_ROLE` and re-exercises with the new value. - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [ ] Manual verification — pending Entra-side App Role declaration with `value: "Portal.Admin"` + assignment to the test user. Once both exist: sign out + sign in on portal-admin, hit `/api/admin/auth/me` and confirm `roles: ["Portal.Admin"]`, then click "Audit log" + "User list" and confirm both render. An `admin.access_denied` row in `audit.events` is the negative-test signal (still emitted for any user without the role). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #145 |
||
|
|
aea395ae65 |
feat(ci): assert gzip transfer sizes against ADR-0017 budgets (#133)
## Summary Closes the ADR-0017 follow-up that was sitting on an unpushed local branch (`feat/ci/gzip-transfer-size-budgets`) for ~5 days, while ADRs 0018–0021 and the auth/admin/security tracks landed. Cherry-picked onto current `main`, with a small follow-up fix to make the script work against the locale-split build layout introduced by ADR-0019 between the two commits. ## What lands ### Commit 1 — `feat(ci): assert gzip transfer sizes against ADR-0017 budgets` [`scripts/check-gzip-budgets.mjs`](scripts/check-gzip-budgets.mjs) — plain-Node, no deps, ~120 LOC at landing. Closes the ADR-0017 §Confirmation gap (Angular CLI's `budgets` only compare RAW sizes): - Parses Angular's emitted `index.html` to separate **initial** assets (anything referenced via `src=` / `href=`) from **lazy** chunks (the rest). - Gzips every JS / CSS file at level 9 — what most HTTP servers serve for static assets — and reports a per-file + per-bucket table. - Asserts the [ADR-0017](docs/decisions/0017-performance-budgets-lighthouse-ci.md) thresholds (`initial JS total ≤ 300 KB`, `any lazy chunk ≤ 100 KB`, `total CSS ≤ 150 KB`) and exits non-zero on any breach. - Wired through: - `pnpm ci:gzip-budgets` invokes the script with the default dist path. - `ci:perf` chains build → gzip check → Lighthouse, so a budget breach short-circuits before Lighthouse even runs. ADR-0017 §Confirmation also updated to point at the script instead of describing it as a "future follow-up". ### Commit 2 — `fix(ci): adapt gzip-budget check for the @angular/localize multi-bundle layout` The original commit predated PR #91 / [ADR-0019](docs/decisions/0019-internationalisation-angular-localize.md), which made `@angular/localize` emit one self-contained bundle per locale under `dist/apps/portal-shell/browser/<locale>/`. The script looked for `index.html` directly under the dist root and failed with ENOENT on every build since. Fix: - Auto-detects layout. If `dist/index.html` exists → flat mode (unchanged behaviour, kept for single-locale apps like `portal-admin`). Otherwise enumerates immediate subdirectories with their own `index.html` and runs the check **per locale**. - Budget thresholds apply per locale — each bundle is what the user's browser actually downloads. Aggregating across locales would understate the worst case. - Violations across locales are collected + reported together with the locale tag prefixed so a single CI run surfaces every breach. - ADR-0017 §Confirmation amendment expanded to spell out the per-locale check + cite ADR-0019. ## Test plan - [x] `node --check scripts/check-gzip-budgets.mjs` — syntax OK. - [x] `pnpm ci:gzip-budgets` against the current production build: - 2 locale bundles detected (`en`, `fr`). - Initial JS total: ~141 KB / 300 KB budget ✓ (each locale). - CSS total: 4.62 KB / 150 KB budget ✓ (each locale). - Largest lazy chunk: 1.55 KB / 100 KB per-chunk budget ✓. - [x] No conflict on `package.json` despite 11 PRs touching it since the branch base — cherry-pick auto-merged cleanly. - [ ] CI: `pnpm ci:perf` end-to-end (build → gzip-budgets → Lighthouse). Validates on the runner. ## Notes for the reviewer - The script lives at the repo root under `scripts/` rather than as an Nx target — it's a pure CI helper, not project-scoped. Matches the existing `ci:audit` / `ci:commits` pattern in `package.json`. - No dependencies added. Uses `node:fs/promises` and `node:zlib`. - The original branch (`feat/ci/gzip-transfer-size-budgets`) is preserved in local-only state for paranoia; once this PR merges, it can be `git branch -D`'d. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #133 |
||
|
|
da2bd6d481 |
docs(adr-0021): phase-2 security baseline (helmet, CORS, CSRF, rate-limit, error envelope) (#124)
## Summary Documents the security middleware stack shipped across the five most recent BFF PRs (#115, #117, #120, #122, #123) as a single MADR ADR. Today the rationale for each choice lives in code comments and PR descriptions; the next contributor reaching for `csurf`, a cookie-only CSRF, or a hardcoded localhost CORS fallback won't have a single place to read why those are wrong here. ADR-0021 covers: - **Response envelope** — `{ error: { code, message, traceId } }`, single contract shared between Nest's `StructuredErrorFilter` and raw Express middlewares (CSRF, rate-limit) via the exported `errorResponse()` helper. Status code → code mapping documented. 500s never leak the underlying exception. - **CSRF — session-bound double-submit**, not pure cookie-vs-header. Rationale: a subdomain-takeover cookie injection can't bypass the comparison because the source of truth is the server-side session token, not the cookie. Cookie is the SPA's read-only mirror. - **CORS allowlist** — `CORS_ALLOWED_ORIGINS` mandatory at boot, no fallback. "Works in dev, breaks in prod" is the exact trap the validator catches. - **Rate limiting** — buckets keyed by session id (auth) or IP (anonymous), 10/min on `/auth/login` + `/auth/callback`, 120/min general, `/api/health` skipped. In-memory v1 store; Redis-backed migration is one constructor arg. - **Helmet config** — defaults plus three overrides (HSTS prod-only, `crossOriginResourcePolicy: cross-origin`, CSP prod-only). Each override has a code-anchored justification. ## Scope This is the **implementation-level** security ADR. The **strategic** security baseline ADR — OWASP ASVS reference level, HDS / GDPR / NIS 2 framing, RSSI sign-off — remains paused per the note in [CLAUDE.md](CLAUDE.md). When that ADR lands it'll either confirm 0021 or supersede pieces of it; 0021 is explicit about which choices are tactical and revisitable. ## Notable structural choices in the ADR - **"Considered Options" mirrors the actual debate.** For CSRF: session-bound vs pure double-submit vs `csurf` vs synchronizer. For rate-limit bucket key: sessionID-then-IP vs IP-only vs Entra `oid`. Each rejected option has its "Bad, because …" so the reader sees why we didn't go that way. - **Each "Decision Outcome" line points at the file / function that enforces it.** Cross-references are absolute paths so they survive folder reorgs. - **The "Consequences" section is brutally honest about trade-offs.** In-memory rate-limit doesn't scale horizontally. The CSRF cookie is XSS-readable. No `details` field in the envelope for field-level validation errors yet. These aren't hidden in the prose. ## Other doc touches - [docs/decisions/README.md](docs/decisions/README.md) index entry added. - [notes/handoff.md](notes/handoff.md) refreshed (gitignored; not part of the commit but useful for the next session). ## Noted for a separate PR (out of scope here) [CLAUDE.md](CLAUDE.md) §"Repository status" still says "The Nx workspace is **not yet bootstrapped**" and refs ADRs up to 0020. The whole paragraph is stale (the project is fully scaffolded, 22 ADRs in place). Worth a small dedicated docs PR. ## Test plan - [x] `pnpm exec prettier --check docs/decisions/` → clean. - [x] ADR follows the MADR 4.0.0 template (frontmatter with `status`, `date`, `decision-makers`, `tags`; sections in the canonical order). - [x] Tags drawn from the vocabulary in [docs/decisions/README.md](docs/decisions/README.md#tag-vocabulary): `security`, `backend`. - [x] Index in `docs/decisions/README.md` updated in the same change. - [x] Cross-references to ADRs 0009, 0010, 0012, 0015 verified. - [ ] Renders in Gitea / IDE markdown preview without parser warnings. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #124 |
||
|
|
a97be121e6 |
fix(portal-bff): audit writes use raw INSERT (audit_writer has no SELECT for RETURNING) (#121)
## Summary #120 shipped the audit pipeline but the end-to-end path was never smoke-tested against a running Postgres. First click on `/auth/logout` returned 500 with the Pino log: ``` PostgresError code 42501 — permission denied for table events ``` Despite: - ACL on `audit.events` showing `audit_writer=a/audit_owner` (INSERT granted). - `has_table_privilege('audit_writer', 'audit.events', 'INSERT')` returning `t`. - `has_schema_privilege` / `has_type_privilege` all `t`. - A direct psql `INSERT INTO audit.events ...` after `SET LOCAL ROLE audit_writer` **succeeding**. - A psql `INSERT ... RETURNING id` after the same `SET LOCAL ROLE` **failing** with the exact same error. Root cause: Prisma's ORM `tx.auditEvent.create(...)` issues `INSERT ... RETURNING *` to hydrate the returned entity. Postgres requires **SELECT** on every column listed in `RETURNING`. `audit_writer` has INSERT only by ADR-0013 design — RETURNING fails with `code 42501` and the error message reads "permission denied for table events" (no mention of SELECT or RETURNING, which is what made it deeply non-obvious to diagnose). ## Fix `AuditWriter.recordEvent` now issues a parameterised raw INSERT via `tx.$executeRawUnsafe` instead of the ORM `create()`: ```ts await tx.$executeRawUnsafe( `INSERT INTO "audit"."events" (id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload) VALUES (gen_random_uuid(), $1, $2::"audit"."AuditAudience", $3::"audit"."AuditOutcome", $4, $5, $6, $7::jsonb)`, input.eventType, input.audience, input.outcome, input.subject ?? null, actorIdHash, traceId, payloadJson, ); ``` The role contract per ADR-0013 stays strict: `audit_writer` keeps INSERT only, no SELECT/UPDATE/DELETE/TRUNCATE. The other natural fix (`GRANT SELECT` to `audit_writer`) would have weakened the writer/reader role separation, so we deliberately went the other way. ## Notable choices **`gen_random_uuid()` server-side instead of Prisma's `@default(uuid())` client-side.** The model still declares `@default(uuid())` for any future ORM read or `audit_reader`-side query, but the write path uses the built-in Postgres function. No extension required (Postgres 13+). **Explicit enum and jsonb casts.** Parameters travel as TEXT over the wire; the SQL casts (`$2::"audit"."AuditAudience"`, `$7::jsonb`) ensure Postgres parses them as the right type. Without the casts, the type system rejects the INSERT before privilege check even fires. **Parameterised, not interpolated.** `$executeRawUnsafe` accepts a SQL template with `$1, $2, …` placeholders and a vararg of values — same wire-level parameter binding as a prepared statement, so SQL injection isn't possible even on caller-controlled inputs like `eventType`. The spec pins this with a malicious-input test. **Also fixes an env-sensitivity bug in `auth.controller.spec.ts`.** The test that asserts `session.absoluteExpiresAt == createdAt + 43200000` was reading the default via `readSessionTimeouts()` but didn't override `SESSION_ABSOLUTE_TIMEOUT_SECONDS`. If `apps/portal-bff/.env` has a custom value (as it did during the manual audit debugging), the test failed non-deterministically. Now the test deletes the env var before running and restores it after — same pattern as the other env-touching tests in this file. ## ADR amendment [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md) §"Writer" now carries an **Implementation trap** callout explaining why Prisma's ORM `create()` cannot be used for audit writes (RETURNING requires SELECT, audit_writer has INSERT only). The corresponding Confirmation entry cross-references the callout. Two-commit shape on this PR (code + docs) — the squash-merge will fold them. ## Test plan - [x] `pnpm nx test portal-bff` (clean env) → **144/144 pass**. - [x] `pnpm nx lint portal-bff` → clean. - [x] `pnpm nx build portal-bff` → clean. - [x] Prettier-clean. - [ ] Manual smoke against running BFF + Postgres: - [ ] Sign in → row in `audit.events` with `event_type = 'auth.sign_in'`. - [ ] Sign out → row with `event_type = 'auth.sign_out'`. **The 500 from before is gone.** - [ ] Verify the role contract is still strict : ```sql SET ROLE audit_writer; SELECT * FROM audit.events LIMIT 1; -- should fail "permission denied" UPDATE audit.events SET event_type = 'x'; -- should fail DELETE FROM audit.events; -- should fail ``` --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #121 |
||
|
|
6aee56abe9 |
docs(architecture): refresh containers + nx boundaries diagrams, add local-infra diagram (#119)
## Summary
Dépoussiérage of `docs/architecture.md` after the auth/session track and a new local-infra diagram.
### Fixed
- **§2 Containers** — `portal-admin` was missing despite shipping in ADR-0020 and being scaffolded in `apps/portal-admin`. Added as a sibling SPA with its own session cookie (`__Host-portal_admin_session`), and the BFF box now distinguishes `/api/*` (end-user) from `/api/admin/*` (RBAC + `@RequireMfa({ freshness: 600 })`).
- **§3 Nx module boundaries** — three small lies:
- `shared-ui` was drawn under `scope:portal-shell`. The actual tag in [libs/shared/ui/project.json:7](libs/shared/ui/project.json#L7) is `scope:shared`. Same for `shared-state` (which wasn't on the diagram at all). Both now live in the `scope:shared` bubble.
- `portal-admin` was absent. Added with its planned (dashed) edges to the shared libs and a note that no `scope:portal-admin` row exists in `eslint.config.mjs` yet — that lands when admin modules grow real lib deps (follow-up).
- The "forbidden" examples included `shared-tokens ⟶ shared-ui` framed as "narrower scope", which doesn't apply (both are `scope:shared` / `type:shared`). Replaced with examples actually enforced by `depConstraints`.
### Added
- **§5 Local dev infrastructure** — visualises what `docker compose up` actually starts: postgres / redis / otel-collector always up, plus opt-in profiles `dbtools` (pgweb), `observability` (Jaeger), `serve-static` (Caddy). Shows ports, named volumes, the bring-up cheat sheet, and the separate CI-runners stack (`apf-portal-ci-runners`, distinct network). Sources point straight at the compose files so a contributor can chase any detail in one click.
### Misc
- Updated the "Trace context propagation" row in the "To be added" table — its trigger ("first observability instrumentation lands") was already met. Now flagged as overdue / pending a follow-up PR (deferred from this scope).
- Carried the pre-existing one-line fix on the §4 `squash[…]` node (mermaid was rejecting unquoted parens inside the label).
## Test plan
- [x] `pnpm exec prettier --check docs/architecture.md` → clean.
- [x] Re-read all five mermaid blocks for syntax (no unquoted parens / `:` / `(` inside `[]` labels). Compose-side `--profile X` only appears inside `"…"` so it's safe.
- [ ] Render in Gitea / IDE markdown preview — the five mermaid blocks should display without errors.
- [ ] Eyeball §5 against [infra/local/dev.compose.yml](../infra/local/dev.compose.yml): every service + profile present, ports match.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #119
|
||
|
|
758d723744 |
feat(portal-bff): session middleware with AES-256-GCM at rest per ADR-0010 (#110)
## Summary
Mounts `express-session` + `connect-redis` at bootstrap on top of the shared `ioredis` client, with **AES-256-GCM applied to the full JSON payload before it lands in Redis** (per ADR-0010). The configured middleware is exposed as a NestJS provider (`SESSION_MIDDLEWARE`) and `main.ts` mounts it through `app.get(...)` so it sits on the same Redis connection the rest of the BFF uses — no second client at the bootstrap layer.
Envelope is versioned (`v1.<iv>.<tag>.<ciphertext>`, all base64url) so the algorithm / key derivation can rotate without a flag-day re-encryption. Tamper / wrong-key / unknown-version all raise `SessionDecryptError`; for now the failure is logged via Pino with `event: session.decrypt_failed` — the first-class audit event lands with ADR-0013.
Scope is intentionally **infrastructure only**:
- middleware mounted on every request, `req.session` available downstream
- session id = `crypto.randomBytes(32).toString('base64url')` (256 bits per ADR-0010)
- cookie name: `__Host-portal_session` in production, `portal_session` in dev (the `__Host-` prefix mandates `Secure`, which dev HTTP can't satisfy)
- `httpOnly + sameSite=lax + path=/`; `resave:false`, `saveUninitialized:false`, `rolling:true`
- cookie `maxAge` follows `SESSION_IDLE_TIMEOUT_SECONDS` (default 1800)
- encryption-at-rest active end-to-end
Out of scope, landing in follow-ups: `/auth/callback` populating `req.session.user`, `/me`, `/auth/logout`, the absolute-timeout interceptor, and the `user_sessions:{userId}` secondary index.
## Notable shape choices (ADR-0010 amended in the same commit)
**Full-payload encryption vs. just the `tokens` field.** The first draft of ADR-0010 scoped at-rest encryption to a `tokens` sub-field. The session also carries claims (`oid`, `tid`, `preferred_username`, …) that qualify as PII under GDPR — for an APF-Handicap portal handling health-adjacent data this matters. Encrypting the envelope is strictly stronger and removes the need to classify fields one by one. The ADR text is updated to match.
**`ioredis` + adapter vs. switching the BFF to `node-redis`.** `connect-redis` v9 was rewritten for `node-redis` v4 and no longer accepts `ioredis` directly. Two reasonable paths:
1. **Adapter (chosen)** — keep the shared `ioredis` client; shim the six commands `connect-redis` actually calls (`get`, `set` with `{expiration:{type:'EX',value}}`, `expire`, `del`, `mGet`, `scanIterator`) to the node-redis shape. Smallest blast radius — RedisModule, OBO cache (ADR-0014), future pub/sub all stay on a single Redis library.
2. **Switch RedisModule to `node-redis`** — clean alignment with `connect-redis`'s expectations, but touches every Redis consumer and would itself require an ADR amendment.
The adapter is reversible: if we ever decide to standardise on `node-redis`, deleting one file removes it. Happy to switch if you'd rather take that path.
## Env vars
- `SESSION_ENCRYPTION_KEY` — **mandatory**, AES-256-GCM key (32 bytes after base64url decode). New `assertSessionEncryptionKey()` validator wired in `main.ts` alongside the other pre-flight checks.
- `SESSION_IDLE_TIMEOUT_SECONDS` — optional, default `1800`.
- `SESSION_ABSOLUTE_TIMEOUT_SECONDS` — optional, default `43200` (consumed by the absolute-timeout interceptor in a follow-up).
`.env.example` updated; the three variables are promoted from the "future vars" block to the active section.
## Test plan
- [x] `pnpm nx test portal-bff` — **99/99 pass** (was 62 before this PR; +37 new specs across the 5 new files).
- [x] `pnpm nx build portal-bff` — clean webpack build.
- [x] `pnpm nx lint portal-bff` — clean.
- [x] Prettier-clean for all PR source files.
- [ ] Local smoke test once the next PR wires `/auth/callback` → `req.session.user`; this PR has no user-visible behaviour to exercise on its own.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #110
|
||
|
|
c5e91f240b |
docs(decisions): add ADR-0019 i18n + ADR-0020 portal-admin (#89)
## Summary
Pure documentation PR. Two ADRs that answer the two strategic questions raised after the footer chantier:
- **[ADR-0019](docs/decisions/0019-internationalisation-angular-localize.md)** — how the portal handles multiple languages.
- **[ADR-0020](docs/decisions/0020-portal-admin-app.md)** — where portal administration lives.
Implementation will land across follow-up feature PRs, each consumable on its own.
## ADR-0019 — Internationalisation
**Decision:** `@angular/localize` in **build-time** mode, two locales (`fr` default served at `/`, `en` source). Path-based URLs always prefixed (`/fr/...`, `/en/...`); `/` smart-redirects via cookie → `Accept-Language` → `fr`. The locale switcher in the footer writes a `__Host-portal_locale` cookie and hard-refreshes to the matching bundle.
**Considered and rejected:**
- `@angular/localize` runtime mode (single bundle, higher LCP / payload cost).
- `@ngx-translate` / `transloco` (community libraries; tech-bar prefers Angular first-party for foundational primitives).
- Query-param URL strategy (fragile, weaker SEO, `<html lang>` becomes harder).
- Subdomain URL strategy (breaks `__Host-` cookie scoping from ADR-0010).
**Scope boundary:** UI strings owned by developers (templates + `$localize` in code). Editorial content (CMS-managed pages, news, etc.) is BFF-served already localised — that's the admin-app pipeline (ADR-0020), not `@angular/localize`.
**First sweep consequence:** the duplicate `/accessibility` + `/accessibilite` routes collapse to one Angular route with locale-translated paths.
## ADR-0020 — `portal-admin`
**Decision:** new Angular SPA `portal-admin` alongside `portal-shell`, sharing the existing `portal-bff` via `/api/admin/*` routes guarded by an Entra `admin` role plus `@RequireMfa({ freshness: 600 })` at the entry route. Distinct origin + cookie + session (`__Host-portal_admin_session`).
**v1 modules** (all four selected):
1. Editorial pages CMS (multilingual content, fed back to `portal-shell` via the BFF).
2. Sidebar menu management (activates the `requiredPermissions` field already on `MenuItem`).
3. User list (read-only).
4. Audit log viewer (consumes the `audit.events` table per ADR-0013, via the `audit_reader` role).
**Out of v1:** B2B invitations (stay in Entra Admin Center), feature flags (no substrate yet), CMS workflow / approval flows, theme customisation, live preview.
**Considered and rejected:**
- `/admin/*` lazy-loaded inside `portal-shell` (admin code in the same origin → weaker defense in depth, admin URL not IP-restrictable independently).
- Two SPAs **and** two BFFs (doubles infra at our scale — bricolage).
- Off-the-shelf admin tooling (Retool, etc. — escapes our security baseline).
**Performance budget for admin:** ≤ 500 KB gzip initial (vs 300 KB for `portal-shell`, per ADR-0017). Lighthouse Performance ≥ 85 on critical admin routes (vs ≥ 90 on `portal-shell`). Same a11y baseline (ADR-0016), same dark-mode support.
**Shared-libs graduation:** `Icon`, `LayoutStateService`, brand tokens, dark-mode SCSS helpers move from `portal-shell` to `libs/shared/{ui,state}` when both apps need them. Mechanical refactor; tracked as the first implementation PR.
## Implementation roadmap (out of scope of this PR)
ADR-0019:
1. Install `@angular/localize`, wire build target.
2. Mark every existing UI string in `portal-shell` with `i18n` + `@@id`; produce `messages.fr.xlf`.
3. Locale switcher in footer + `/api/preferences/locale` BFF route + smart redirect at `/`.
4. Collapse the duplicate accessibility routes into a localised single route, with 301s.
5. CI gate: `nx build portal-shell --localize` is added to `ci:check` and fails on missing translation.
ADR-0020:
1. `nx g @nx/angular:app portal-admin` skeleton.
2. Shared-libs extraction (`libs/shared/ui`, `libs/shared/state`).
3. BFF `AdminModule` + `AdminRoleGuard` + smoke `GET /api/admin/me`.
4. Admin shell (header / sidebar / footer with an "Admin" badge).
5. One PR per v1 module — suggested order: CMS pages → menu management → audit viewer → user list.
## Test plan
- [x] Both ADRs follow MADR 4.0.0 (frontmatter, sections, tags from the canonical vocabulary).
- [x] `docs/decisions/README.md` index updated in the same commit.
- [x] `CLAUDE.md` architecture summary picks up entries for both decisions and bumps the ADR coverage line to 0020.
- [ ] Read-through review — invite the project lead to push back on any decision before locking implementation.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #89
|
||
|
|
4476cbb518 |
docs(decisions): add ADR-0018 — environment configuration strategy (#80)
## Summary
Records the per-environment config strategy for both apps. ADR-only — no code changes; the implementation is anchored in §Confirmation against the next feature work that touches per-environment values.
## What lands
[**ADR-0018 — Environment configuration**](docs/decisions/0018-environment-configuration-strategy.md):
- **SPA**: Angular `environment.ts` + project.json `fileReplacements`. Build-time substitution; no runtime config fetch (rejected for the LCP/TTFB cost it would add and the deploy-time HTML rewrite that the alternatives need). Concretely cleans up the hard-coded URLs we left in `observability/tracing.ts` and `home-status.service.ts` ("hard-coded for v1 — env-config when it lands" comments).
- **BFF**: keep `process.env` + small per-key boot-time validators (the shape `apps/portal-bff/src/config/check-database-url.ts` already follows). `@nestjs/config` rejected as too heavy for the current key count; `zod` not justified yet.
- **Audit log connection**: formalises the `AUDIT_DATABASE_URL` split that ADR-0013 §"Wired as features land" already pointed at. When set, the `AuditModule` instantiates a second Prisma client with `audit_writer`-only credentials (defense in depth); when unset, the dev fallback (shared pool + `SET LOCAL ROLE audit_writer` per transaction) keeps working. A boot-time `UPDATE`-rejection self-test runs against the dedicated pool when configured — refuses to start if the pool can mutate the audit table.
The §Confirmation block cross-references the env-var-as-boot-gate items already pre-figured in ADR-0009 / ADR-0010 / ADR-0012 / ADR-0013 / ADR-0014, so the validator pattern is the single landing place when those features ship.
## What does NOT change in this PR
- No `apps/portal-shell/src/environments/*.ts` files yet — landed alongside the next feature that actually needs per-environment values.
- No `AUDIT_DATABASE_URL` validator in BFF — same; lands when the second pool is wired.
- No CLAUDE.md restructuring; just one extra bullet under the Architecture summary referencing ADR-0018.
## Doc updates
- `docs/decisions/README.md` index — new row for ADR-0018.
- `CLAUDE.md` Architecture summary — one-line reference to ADR-0018 between the perf-budget and local-quality-gates entries.
## Test plan
- [ ] CI green on this PR (`format:check`).
- [ ] ADR-0018 renders in the doc index with the right tags (`frontend`, `backend`, `infrastructure`, `process`) and 2026-05-10 date.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #80
|
||
|
|
922a44bb50 |
chore(ci): auto-merge low-risk renovate updates (#77)
## Summary After ~10 days of clean Renovate track record (~30 PRs merged without regression beyond the TS/ESLint/webpack-cli majors that the dashboard-approval rule now catches), enable auto-merge on the lowest-risk update types. CI is the gate — `check` / `scan` / `a11y` going red leaves the PR open for manual triage. | Update type | Pre-PR | Post-PR | | - | - | - | | **patch** | manual review | **auto-merge if CI green** | | **pin** (rolling-tag → fixed-version pin) | manual | **auto-merge if CI green** | | **digest** (image digest pin refresh) | manual | **auto-merge if CI green** | | **lockFileMaintenance** (weekly transitive refresh) | manual | **auto-merge if CI green** | | **minor** | manual review (unchanged) | manual review | | **major** | dashboard approval (unchanged) | dashboard approval | `automergeStrategy: "squash"` matches our trunk-based squash-merge convention. `automergeType: "pr"` keeps the PR + CI run as the audit trail (vs branch-direct push), and Gitea auto-merges the PR once green via the bot's existing `repo:write` permission. ## Doc updates - `renovate.json` `dependencyDashboardHeader` — the "Open" row now reflects the new reality: mostly minors, with red patches surfacing briefly when CI fails. - `docs/development.md` §"Reviewing Renovate PRs" gains a bullet describing the auto-merge for contributors landing on the project later. ## Test plan - [ ] CI green on this PR. - [ ] After merge, the next patch-level Renovate run produces a PR that auto-squashes into `main` once CI clears (visible in the merged log; no human action required). - [ ] A patch with red CI stays open in "Open" with the `dependencies` label. - [ ] Minor / major Renovate PRs continue to require human merge. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #77 |
||
|
|
02ac44e498 |
feat(portal-bff): audit log foundation per ADR-0013 (#76)
## Summary Lays down the append-only audit log per ADR-0013: schema declaration, first migration with role grants, NestJS `AuditWriter` service. Typed event-family methods, the separate `AUDIT_DATABASE_URL` pool, the retention job, and the live-DB integration tests are explicitly listed as "wired as features land" in the ADR's confirmation block — they ship when the matching feature ADRs do. ## What lands **Prisma schema** ([`apps/portal-bff/prisma/schema.prisma`](apps/portal-bff/prisma/schema.prisma)): - `multiSchema` preview enabled; datasource declares `public` + `audit` schemas. - `AuditEvent` model: `id` (uuid), `createdAt`, `eventType` (free-form in v1), `audience` enum (`workforce | customer`), `actorIdHash`, `traceId`, `subject`, `outcome` enum (`success | failure | denied`), `payload` (jsonb). - Indexes on `createdAt`, `eventType`, `traceId` — covering the three obvious query shapes. **Migration** ([`prisma/migrations/*_init_audit_schema/migration.sql`](apps/portal-bff/prisma/migrations/20260510011453_init_audit_schema/migration.sql)): - Standard Prisma `CREATE TABLE` / enums output, then the **append-only contract** re-applied explicitly: - `ALTER TABLE/TYPE OWNER TO audit_owner`. - `GRANT INSERT` to `audit_writer`, `SELECT` to `audit_reader`, **`SELECT, DELETE`** to `audit_archiver` (SELECT is needed to evaluate the `created_at` predicate of "delete older than retention" — Postgres requires SELECT on every column referenced in DELETE's WHERE). - `GRANT USAGE` on the enum types to all three roles (without it `audit_writer.INSERT` fails with "permission denied for type"). - **No** GRANT for `UPDATE` / `TRUNCATE` to anyone — including `audit_owner` at runtime; only fresh schema migrations amend the table. **Service** ([`apps/portal-bff/src/audit/`](apps/portal-bff/src/audit/)): - `AuditWriter.recordEvent(input)` — single entry point. Wraps every INSERT in a transaction whose first statement is `SET LOCAL ROLE audit_writer`, so the role contract holds at runtime even from the otherwise-privileged BFF connection. - `traceId` auto-resolved from the active OTel span (so audit row joins with traces and Pino logs on the same `trace_id`). - `actorIdHash` auto-resolved from CLS (key `actorIdHash`) with explicit input-side override; `null` when neither is set (placeholder until ADR-0009 / ADR-0010 guards populate CLS). - Errors propagate (no catch-and-swallow), per ADR-0013's "blocking writes: no audit ⇒ no action". **Tests** — 8 unit tests on `AuditWriter` (mocked Prisma + CLS): role-locking ordering, input pass-through, `Prisma.JsonNull` for missing payload, CLS-vs-input precedence on `actorIdHash`, OTel trace capture, error propagation. ## End-to-end verification (manual, against local-dev Postgres) ``` INSERT under audit_writer: ok UPDATE under audit_writer: permission denied for table events DELETE under audit_writer: permission denied for table events DELETE under audit_archiver: ok, row removed (after the SELECT-grant fix) ``` ## ADR-0013 §Confirmation rewritten Two-block split: "wired in foundation PR" lists what landed here; "wired as features land" lists the typed event-family methods, AUDIT_DATABASE_URL connection split, startup self-test probe, retention purge job, salt-shared cross-correlation test, and live-DB role-contract integration tests — each anchored to the feature ADR that triggers it. ## Recovery for anyone with a pre-existing local-dev DB If your local-dev Postgres already had the audit migration applied **before** the SELECT-grant fix, the archiver's DELETE will fail. Two options: 1. Apply the missing grant directly: ```bash psql "$DATABASE_URL" -c "GRANT SELECT ON audit.events TO audit_archiver;" ``` 2. Or wipe the volume and re-migrate cleanly: ```bash ./infra/local/dev.sh down -v ./infra/local/dev.sh up pnpm --filter @apf-portal/source exec prisma migrate deploy # or `cd apps/portal-bff && pnpm exec prisma migrate deploy` ``` Fresh DBs land with the corrected migration directly. ## Out of scope (separate PRs) - Typed event-family methods (`signIn`, `signInFailed`, …) — added per matching feature ADR. - `AUDIT_DATABASE_URL` separate connection pool — defense-in-depth, when production needs it. - Startup self-test probe (deliberate failing UPDATE asserting rejection) — lands with the connection split. - Retention purge job (`audit_archiver` daily cron) — phase-3b infra. - Live-DB integration tests asserting the role contract — Testcontainers-style harness, separate PR. ## Test plan - [ ] CI green on this PR. - [ ] `prisma migrate deploy` succeeds on a fresh DB (the recovery instructions cover the SELECT-grant gap for already-migrated dev DBs). - [ ] `psql -c "\dp audit.events"` shows the expected privilege matrix: `audit_owner=arwdDxtm/audit_owner`, `audit_writer=a/audit_owner`, `audit_reader=r/audit_owner`, `audit_archiver=rd/audit_owner`. - [ ] BFF boots; calling `AuditWriter.recordEvent` from a controller (manual smoke once a real flow lands) writes to `audit.events` with the expected `trace_id` matching the request's Jaeger span. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #76 |
||
|
|
e2dd2e4dd8 |
fix(portal-shell): use .postcssrc.json so tailwind utilities are emitted (#75)
## Summary The portal-shell scaffold shipped a `postcss.config.js`, but `@angular/build` (Angular 17+ esbuild pipeline) does **not** load that file format — it only reads `.postcssrc.json`. Result: the `@tailwindcss/postcss` plugin was never registered, Tailwind ran with no source files visible, generated only its `@theme` block, and dropped every utility class on the floor. The page therefore rendered unstyled — black text on white — even though `nx build` reported success and shipped a 23 KB `styles*.css`. Rename / re-encode the config to `.postcssrc.json`. Same plugin, same options, just the file format Angular's esbuild adapter actually reads. ## Verification | | Before | After | | - | - | - | | Class selectors in `dist/apps/portal-shell/browser/styles*.css` | **0** | **75** (production) | | `.text-3xl`, `.bg-amber-50`, `.font-semibold` etc. emitted | ❌ | ✓ | | Pages render with intended Tailwind layout | ❌ | ✓ | ```bash pnpm exec nx build portal-shell --configuration=production grep -oE '\.[a-zA-Z][a-zA-Z0-9_-]*' dist/apps/portal-shell/browser/styles*.css | sort -u | wc -l # 75 ``` ## Doc update `docs/development.md` repo-layout walkthrough now reads `.postcssrc.json` and explicitly calls out that `postcss.config.js` is ignored by `@angular/build` — so a future contributor reaching for the legacy filename gets a hint instead of a silent failure. ## Test plan - [ ] CI green on this PR. - [ ] Local: bring up the SPA dev server, `localhost:4200` shows the styled layout — header bar with brand link, system-status widget with rounded card, footer with the two language links. - [ ] Production build: `nx build portal-shell --configuration=production` succeeds; `dist/.../styles*.css` contains tens of utility-class selectors (not just `@theme` tokens). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #75 |
||
|
|
0d31937aeb |
feat(portal-shell): replace nx-welcome with real shell and home + budgets per ADR-0017 (#74)
## Summary Cuts the Nx scaffold's `NxWelcome` page (938 LOC of placeholder markup) and lays down the actual portal layout — header, main, footer landmarks plus a skip link — with a real home page and the two RGAA-mandated accessibility statement routes. Bundle budgets in `project.json` are tightened to match ADR-0017. ## What lands **Layout shell** (`apps/portal-shell/src/app/`): - `components/header/` — brand link + primary nav landmark stub - `components/footer/` — accessibility statement links (FR + EN) + version badge - `app.html` / `app.scss` — flex column with sticky footer + skip link to `#main-content` (WCAG 2.4.1) **Pages**: - `pages/home/` — welcome heading + "system status" widget that fetches `/api/health` from the BFF. Doubles as a smoke test of the full SPA → BFF stack: CORS, OTel trace propagation, Pino log correlation. After page load, http://localhost:16686 should show one trace whose root span is the SPA `document_load`, with a child `fetch` span that has its own child `HTTP GET /api/health` BFF span. - `pages/accessibility/` — single component, content selected by route data (`lang: 'fr' | 'en'`). Mounted at `/accessibility` (en) and `/accessibilite` (fr). v1 carries placeholder copy explicitly framed as "awaiting APF user panel review" — RGAA + ADR-0016 require the routes to exist; the substantive statement (audit grid, gaps list, remediation plan) lands separately. **Routing & wiring**: - `app.routes.ts` adds the three routes, all lazy-loaded. - `app.config.ts` adds `provideHttpClient(withFetch())` so HttpClient delegates to the patched browser `fetch` and the W3C `traceparent` header propagates automatically (per ADR-0012 phase 2). - `nx-welcome.ts` removed; `app.ts` no longer imports it. **Bundle budgets** (`apps/portal-shell/project.json`): | Type | Limit (raw) | ADR-0017 (gzip) | | ------------------- | ------------ | --------------- | | `initial` | 1 MB | ≤ 300 KB | | `anyScript` | 300 KB | ≤ 100 KB / chunk | | `anyComponentStyle` | 6 KB | ≤ 6 KB | | `bundle "styles"` | 150 KB | ≤ 150 KB | `maximumWarning == maximumError` so an overshoot fails the build (matching `type: "error"` in spirit). Angular CLI compares **raw** sizes; ADR-0017 §Confirmation is updated to record the gzip→raw translation and the deferred follow-up that will add a CI check on the actual gzipped transfer size (Angular has no native gzip-mode budget). ## Verified locally - `pnpm exec nx run-many -t lint test build` → 8 projects green; **12 tests pass** for `portal-shell` across 5 specs (App, Header, Footer, Home, AccessibilityStatement). - `pnpm nx build portal-shell --configuration=production` → initial bundle **326.99 KB raw / 89.82 KB transfer** (gzip) — well under the ADR-0017 300 KB gzip target. Lazy chunks: home 2.66 KB, accessibility 2.90 KB. - `pnpm audit` clean. After `./infra/local/dev.sh up observability` + `pnpm nx serve portal-bff` + `pnpm nx serve portal-shell`, opening http://localhost:4200 shows the new layout, the system-status widget connects to the BFF, and the corresponding trace appears in Jaeger. ## Out of scope (separate PRs) - **Real RGAA audit content** — APF user-panel review. - **Design tokens system** in `libs/shared/tokens`. - **Reusable UI primitives** in `libs/shared/ui`. - **User-preferences panel** (ADR-0016 §"User-preferences panel"). - **i18n machinery** (`@angular/localize`) — for v1 the FR/EN copy is inlined and route-driven; real i18n is a separate ADR. - **Env-config** (Angular `environment.ts`) — the hard-coded `http://localhost:3000/api/health` will move there alongside the OTLP endpoint when the env-config PR lands. - **CI gzip-transfer-size assertion** to complement the raw-size Angular budgets. ## Test plan - [ ] CI green on this PR. - [ ] Local: home page renders, system-status widget transitions from "Checking the backend…" to the success state when the BFF is up; falls back to "Backend unreachable" with the BFF stopped. - [ ] `/accessibility` and `/accessibilite` render the matching language with `<article lang="en|fr">`. - [ ] Skip link reachable via Tab from address bar; clicking it focuses `#main-content`. - [ ] Production build size summary roughly matches the figures above (initial total ≈ 90 KB transfer). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #74 |
||
|
|
288610b9ba |
docs(development): add observability dev-loop section (#73)
## Summary ADR-0012 phase 1 + phase 2 are wired (BFF + SPA), so the "Observability dev-loop" placeholder in the roadmap table now has real content to point at. Promote it from §9 future-work list to a full §5 walkthrough sitting between "Daily commands" and "Dependency updates". ## What §5 covers - **Bringing up the observability stack** — `./infra/local/dev.sh up observability`, with the endpoint table (Jaeger UI, OTLP receiver, BFF stdout, browser DevTools). - **Reading a trace in Jaeger** — service-dropdown filter, span attribute keys to look at (`http.method`, `db.statement`, `service.name`, `trace_id`), the trace-id-as-pivot pattern. - **Correlating a trace with the BFF Pino logs** via `trace_id` and `request_id` (the per-request UUID `nestjs-cls` provides). Concrete `grep` example. - **Reading SPA spans from the browser** — DevTools Network filter on `localhost:4318/v1/traces` + Jaeger UI cross-check. - **Common gotchas** — observability profile not active, CORS pre-flight on `/v1/traces`, `propagateTraceHeaderCorsUrls` mismatches, NODE_ENV mis-set, EADDRINUSE on serve restart. - **What's not in v1** — pointer at the "wired as features land" deltas (CLS keys for session/user/audience, redact list, custom domain spans, prod backend) so a contributor knows what's intentionally not yet there. ## Renumbering The new §5 pushes existing sections down. Final structure: 1. Repo layout / 2. Prerequisites / 3. Initial setup / 4. Daily commands / 5. **Observability dev-loop** / 6. Dependency updates (Renovate) / 7. Conventional commit cycle / 8. Where to look / 9. Roadmap. The "Observability dev-loop" line in §9's roadmap table is removed (now implemented). `docs/README.md` cross-link updated to mention the new section. ## Test plan - [ ] CI green on this PR (`format:check`). - [ ] In a fresh checkout, follow §5's "Bring up the observability stack" verbatim, reach the Jaeger UI, then walk a trace through the `grep` correlation example with a synthetic request — flag any step that's wrong / missing on this real-world rehearsal. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #73 |
||
|
|
8f2cd4e068 |
feat(portal-shell): wire spa-side opentelemetry tracing (#72)
## Summary Phase 2 of ADR-0012 — closes the loop SPA → BFF → DB. After this PR, a single user action (initial page load, click, form submit) produces one trace whose root span is owned by the SPA and whose child spans cover the BFF request, Postgres queries through Prisma, and (eventually) Redis / downstream-API hops. ## What lands **Browser-side OTel libs** (production deps): - `@opentelemetry/sdk-trace-web` — browser tracer + provider - `@opentelemetry/exporter-trace-otlp-http` — OTLP/HTTP+JSON exporter - `@opentelemetry/instrumentation` — auto-instrumentation runtime - `@opentelemetry/instrumentation-fetch` — `fetch` + W3C `traceparent` propagation - `@opentelemetry/instrumentation-document-load` — initial-paint timings - `@opentelemetry/instrumentation-user-interaction` — click / keypress / submit No `@opentelemetry/context-zone`: the workspace is zoneless per ADR-0004; the default `StackContextManager` covers the auto-instrumented paths. Custom spans across `await` will need explicit `context.with(...)` plumbing — fine, encountered as code lands. **Code**: - [`apps/portal-shell/src/observability/tracing.ts`](apps/portal-shell/src/observability/tracing.ts) — `WebTracerProvider` bootstrap. Documents the load-order constraint inline (same pattern as the BFF: must be the very first import of `main.ts`, otherwise auto-instrumentations miss everything imported above). - `apps/portal-shell/src/main.ts` now imports the tracing module as line 1. **CORS plumbing** for end-to-end trace propagation: - BFF (`apps/portal-bff/src/main.ts`) calls `enableCors` with a minimal dev allowlist (`http://localhost:4200`) and explicit permission for the W3C `traceparent` / `tracestate` headers. The full security-grade CORS (per-environment allowlists, helmet, cookie-session, CSRF) belongs to the future phase-2 security ADR — this PR adds the strict minimum for the SPA→BFF trace context to survive cross-origin pre-flight. - OTel Collector (`infra/local/otel-collector.yaml`) gains a `cors` block on its OTLP/HTTP receiver so the browser's own OTLP POST clears its pre-flight. **ADR-0012 §Confirmation** rewritten: a new "Wired in the SPA foundation PR (phase 2)" block enumerates what landed here; the carry-over "Wired as features land" list drops the SPA-side SDK item and adds a follow-up note about the security-grade CORS. ## Verification ```bash pnpm exec nx run-many -t lint test build # 8 projects green pnpm audit # 0 vulns ./infra/local/dev.sh up observability # bring up Collector + Jaeger ./infra/local/dev.sh # (separately, BFF stack — your choice) pnpm nx serve portal-bff # localhost:3000 pnpm nx serve portal-shell # localhost:4200 ``` Open http://localhost:4200 → a `document_load` trace appears in http://localhost:16686 with `service.name=portal-shell`. From DevTools, run `fetch('http://localhost:3000/api/health').then(r => r.json())` → a fetch span appears with a child BFF span on the same trace. ## Test plan - [ ] CI green on this PR. - [ ] After local up, `document_load` span visible in Jaeger UI for the SPA. - [ ] Cross-origin fetch from SPA carries `traceparent` (visible in Network tab) and produces a single end-to-end trace SPA → BFF in Jaeger. - [ ] DevTools console shows no CORS warnings about `traceparent`, `tracestate`, or the `localhost:4318/v1/traces` POST. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #72 |
||
|
|
b74d3f1b9b |
feat(portal-bff): observability foundations (Pino + CLS + OTel) (#70)
## Summary
Implements ADR-0012 phase 1, BFF side. The SPA wiring is a separate phase-2 PR.
The BFF now emits structured JSON logs to stdout, tagged with `trace_id` / `span_id` from the active OTel context, and exports OTLP traces over HTTP/Protobuf to the Collector that already runs in the local-dev compose. Anything Nest, Express, HTTP-out, Prisma (Postgres) or `ioredis` does is auto-spanned. A `GET /api/health` liveness endpoint is added to round things out.
## What lands
**Runtime libs added** (production deps):
- `nestjs-pino`, `pino`, `pino-http` — structured logging
- `nestjs-cls` — request-scoped context
- `@opentelemetry/api` / `sdk-node` / `resources` / `semantic-conventions`
- `@opentelemetry/exporter-trace-otlp-proto` (HTTP/Protobuf, port 4318)
- `@opentelemetry/instrumentation-{http,express,nestjs-core,pg,ioredis,pino}` — curated, **no** `auto-instrumentations-node` mass-import (anti-bricolage)
Dev: `pino-pretty` (gated by `NODE_ENV`).
**Code:**
- `apps/portal-bff/src/observability/tracing.ts` — OTel `NodeSDK` bootstrap. Documents the load-order constraint inline (must be the very first import of `main.ts`). Pure side-effect module.
- `apps/portal-bff/src/observability/observability.module.ts` — composes `ClsModule` (UUID per request stored as `request_id`) and `LoggerModule` (`pino-pretty` in dev, raw JSON in prod, `LOG_LEVEL` env-driven, `/health` excluded from auto-logging, `X-Request-Id` honoured if inbound).
- `apps/portal-bff/src/health/{health.controller,health.module,health.controller.spec}.ts` — `GET /api/health` returning `{status, uptimeSeconds, service, version}`. Cheap liveness only — `/readiness` lands when dependencies have a readiness story.
- `apps/portal-bff/src/config/check-database-url.{ts,spec.ts}` — fail-fast validator called from `main.ts` before NestFactory boots. Catches the same family of bug that bit pgweb in #63: a literal special character in `POSTGRES_PASSWORD` that needs URL-encoding in `DATABASE_URL`. Prisma requires a URL string (no discrete-flag escape hatch), so early validation + a clear error message is the v1 mitigation. Six unit tests cover happy path, missing URL, wrong scheme, encoded special chars, literal `@` in password, malformed URL.
**Wiring:**
- `main.ts` imports `./observability/tracing` as line 1, then uses `app.get(Logger)` from `nestjs-pino` with `bufferLogs: true` so early-bootstrap lines are not lost.
- `app.module.ts` imports `ObservabilityModule` first, then `PrismaModule`, then `HealthModule`.
- `apps/portal-bff/.env.example` promotes `LOG_LEVEL`, `OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`, `OTEL_EXPORTER_OTLP_ENDPOINT`, `OTEL_EXPORTER_OTLP_PROTOCOL`, `OTEL_TRACES_SAMPLER` from the "future" comment to active settings — defaults target the local-dev Collector.
- Both `apps/portal-bff/.env.example` and `infra/local/.env.example` now spell out the URL-encoding constraint on `POSTGRES_PASSWORD` with the char-by-char encoding table (`@` → `%40`, etc.).
**ADR-0012 §Confirmation** rewritten to distinguish what landed in this PR from what is wired as the corresponding feature ADRs ship (CLS keys for `session_id` / `user_id_hash` / `audience`, `LOG_USER_ID_SALT` enforcement, redact list, custom spans, SPA-side SDK, full integration tests, prod Collector config).
## Trace ↔ log correlation
Automatic via `@opentelemetry/instrumentation-pino` — every Pino record gets `trace_id` and `span_id` injected from the active OTel context. No CLS gymnastics needed for that concern.
## Verification
```bash
pnpm exec nx run-many -t lint test build # 8 projects green
pnpm audit --audit-level=moderate # 0 vulnerabilities
./infra/local/dev.sh up observability # start Collector + Jaeger
cp apps/portal-bff/.env.example apps/portal-bff/.env
pnpm nx serve portal-bff
curl http://localhost:3000/api/health
# → {"status":"ok","uptimeSeconds":N,"service":"portal-bff","version":"dev"}
```
Then hit `GET http://localhost:3000/api` once or twice and open http://localhost:16686 — the corresponding spans appear in Jaeger, and Pino logs on stdout carry the matching `trace_id`.
## Test plan
- [ ] `nx run-many -t lint test build` green on this PR's CI run.
- [ ] `pnpm audit` clean.
- [ ] BFF boots, `/api/health` returns the expected JSON.
- [ ] Pino logs in dev are colourised one-liners; in prod they would be raw JSON (toggled by `NODE_ENV=production`).
- [ ] With the local-dev stack's `--profile observability` active, traces are visible in Jaeger UI.
- [ ] Each Pino log line for a request carries the same `trace_id` as the trace span in Jaeger.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #70
|
||
|
|
fc9b63f24a |
feat(infra): add dev.sh wrapper for the local-dev compose stack (#68)
## Summary Two recurring frictions on the local-dev stack: 1. **Compose-profile asymmetry** — `docker compose down` only operates on services whose profile is currently active. Anything brought up with `--profile X` keeps running unless the same flag is passed on `down`. pgweb and Jaeger silently survived several `down -v` invocations before we noticed (#67 documented the gotcha; this PR makes it impossible to hit if you use the wrapper). 2. **Verbose invocations** — typing `docker compose -f infra/local/dev.compose.yml --profile … <verb>` for routine ops gets old fast. Add [`infra/local/dev.sh`](infra/local/dev.sh) as a thin wrapper. Always passes every profile in scope on teardown / status / log commands, exposes ergonomic verbs: | Command | Effect | | --------------------------------------------- | --------------------------------------------------------------- | | `./infra/local/dev.sh up` | Core only (postgres + redis + otel-collector) | | `./infra/local/dev.sh up all` | Core + every profile | | `./infra/local/dev.sh up dbtools` | Core + pgweb | | `./infra/local/dev.sh up observability` | Core + Jaeger | | `./infra/local/dev.sh down [-v]` | Tear down (every profile in scope, no orphaned services) | | `./infra/local/dev.sh stop <service>` | Stop one service (containers stay around) | | `./infra/local/dev.sh restart <service>` | Restart one service | | `./infra/local/dev.sh status` | `ps` with every profile visible | | `./infra/local/dev.sh logs [service]` | Follow logs | | `./infra/local/dev.sh exec <service> <cmd>` | Run a command inside a container | Anything not matching one of the named verbs is passed through to `docker compose -f dev.compose.yml ...` (with every profile flagged in), so the full Compose surface remains available. ## Doc updates - **`infra/README.md`** — new "Convenience script" subsection with the cheat-sheet table; "First-time setup" rewritten to use the script; the standalone "Profile symmetry" tip from #67 is collapsed into a one-liner since the script now handles it (the note remains as a fallback for direct `docker compose` users). - **`docs/development.md` §3** — points at the script for the typical setup flow. The compose file itself is unchanged. ## Test plan - [ ] `./infra/local/dev.sh help` prints the usage block. - [ ] `./infra/local/dev.sh up` brings up the 3 core services (no pgweb / no jaeger). - [ ] `./infra/local/dev.sh up all` adds pgweb and Jaeger. - [ ] `./infra/local/dev.sh down -v` stops and removes all 5 containers (incl. pgweb and Jaeger), wipes the postgres-data and redis-data volumes. - [ ] `./infra/local/dev.sh stop pgweb` stops just pgweb. - [ ] `./infra/local/dev.sh logs otel-collector` follows that service's logs. - [ ] `./infra/local/dev.sh exec postgres psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "\du"` lists the audit roles. - [ ] `./infra/local/dev.sh stop` (no arg) errors with a clear message. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #68 |
||
|
|
a93b1067e6 |
docs(setup): add psql and redis-cli to prerequisites (#61)
## Summary Verifying the local-dev stack from the host (`docker compose up -d` + `psql ... -c "\du"` / `redis-cli PING`) requires the postgres and redis client binaries on the developer's machine. They were missing from the prereqs table, so `apt install postgresql-client` / `apt install redis-tools` was an implicit step nobody knew to run. Add both to §2's table, with one-line rationale for each. The Docker row is also tightened to point at the actual local-dev stack location ([`infra/local/`](infra/local/)) instead of the placeholder "Postgres + Redis containers" wording from before that recipe existed. `docker compose exec` remains a viable zero-install alternative for developers who prefer not to touch their host. Mentioned only informally — the host-install path is the documented one. ## Test plan - [ ] Fresh-clone a checkout, follow §2 + §3 verbatim, end with a working stack and successful `psql ... -c "\du"` against it. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #61 |
||
|
|
0f00d6d93f |
feat(infra): add local-dev Docker Compose stack (#57)
## Summary Bring up Postgres + Redis + OTel Collector in one command so contributors can run the BFF end-to-end without manually wiring each service. Replaces the throwaway `docker run postgres:17-alpine` one-liner that was in `docs/development.md` §3. ### What lands - **`infra/local/dev.compose.yml`** — three core services (`postgres:17.2-alpine`, `redis:7.4-alpine`, `otel/opentelemetry-collector-contrib:0.115.0`) plus two viewers gated behind Compose profiles: - `--profile dbtools` → `sosedoff/pgweb:0.16.2` (Postgres GUI on port 8081) - `--profile observability` → `jaegertracing/all-in-one:1.62` (Jaeger UI on 16686) - All ports overridable via `.env`. State in named volumes. Healthchecks on data services. - **`infra/local/.env.example`** — credentials + ports template. `POSTGRES_PASSWORD` and `REDIS_PASSWORD` are mandatory (compose refuses to boot without them); other keys default sensibly. - **`infra/local/init/postgres/01-init.sql`** — bootstrap SQL per **ADR-0013**: `audit_owner` / `audit_writer` / `audit_reader` / `audit_archiver` roles + `audit` schema. Default privileges encode the append-only contract (INSERT to writer, SELECT to reader, DELETE to archiver, no UPDATE/TRUNCATE to anyone). Applied on first Postgres boot only; documented re-run procedure. - **`infra/local/otel-collector.yaml`** — pipeline: OTLP gRPC/HTTP → batch → debug exporter (always) + forward to `jaeger:4317`. When the observability profile is off, the Jaeger export logs warn-level retries but doesn't block the debug pipeline. ### Surrounding doc updates - **`infra/README.md`** — new "Local-dev stack" section: service inventory, port table, first-time setup walkthrough, persistence/bootstrap-replay tips. The previous `local/` placeholder line is removed. - **`docs/development.md`** §3 — rewritten to walk through the compose-based setup; cross-links to `infra/README.md` for the full reference. Roadmap entry for "Local infra recipe" removed from §8 (now implemented); "Observability dev-loop" line adjusted to point at the new Jaeger profile. ### Out of scope - **Production parity** — HA Postgres, Redis Sentinel, real OTel backend (Tempo / Loki / etc.) — defer to the on-prem infrastructure ADR (phase 3b). The dev-only nature of this stack is called out explicitly in `infra/README.md`. - **Wiring the BFF** to actually use these endpoints (NestJS config, Prisma datasource URL, OTel SDK init) — that's the **B — Observability foundations** chantier, next up. ## Test plan - [ ] `cd infra/local && cp .env.example .env && docker compose -f dev.compose.yml up -d` → all three core services come up healthy; verify with `docker compose ps`. - [ ] `psql postgres://portal:<pwd>@localhost:5432/portal_dev -c "\dn"` shows the `audit` schema; `\dg` shows the four audit roles. - [ ] `redis-cli -a <pwd> PING` → `PONG`. - [ ] Send a fake OTLP trace via grpcurl → see it printed by `docker compose logs otel-collector`. - [ ] `--profile dbtools up -d` → http://localhost:8081 shows pgweb UI, can navigate to the audit schema. - [ ] `--profile observability up -d` → http://localhost:16686 shows Jaeger UI; collector logs no longer report Jaeger export retries. - [ ] `docker compose down -v` cleanly removes everything; next `up -d` re-runs the bootstrap SQL. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #57 |
||
|
|
f5d3697466 |
fix(deps): revert TS6/ESLint10/webpack-cli7 majors and gate future majors (#43)
## Summary Three Renovate major bumps merged silently because `nx affected` doesn't see deps-only PRs as affecting any project — CI passed trivially, the breakage only surfaced when `nx run-many` was run locally: - **TypeScript 5→6** (#33) — `tsconfig.lib.json` fails with `TS5101: Option 'baseUrl' is deprecated`. Revert to 5.9.x. - **ESLint 9→10** (#36) — `@nx/eslint@22.7.1` not compatible: project graph fails with "Unable to find eslint". Revert eslint, `@eslint/js`, `jsonc-eslint-parser`, `eslint-plugin-playwright` to ESLint-9-compatible versions. - **webpack-cli 5→7** (#34) — webpack-cli 7 removed the `--node-env=production` flag Nx generates. Revert to 5.x. Bonus side-fix: the `ajv@<8.18.0` override added in #42 was over-broad and was forcing ESLint's bundled ajv to v8 (incompatible with ESLint 9's option contract). Narrow the override to `@angular-devkit/core>ajv@<8.18.0` so only the targeted nestjs-prisma chain is bumped. ## Prevention — gate majors behind the dependency dashboard Add a Renovate `packageRule` with `dependencyDashboardApproval: true` for `matchUpdateTypes: ["major"]`. Renovate stops auto-creating PRs for majors; they appear as checkboxes in the dashboard issue, and only get a PR after a human ticks the box (presumably after reading the changelog and confirming Nx-plugin / Angular / NestJS readiness). This is the surgical fix for the gap. The deeper fix (making `nx affected` correctly mark all projects as affected on package.json changes) is a separate investigation worth doing later — but the dashboard gate prevents the same trap regardless. ## Verification Locally on this branch: - `pnpm exec nx run-many -t lint test build --parallel=2` → ✓ 8 projects pass. - `pnpm audit --audit-level=moderate` → 0 vulnerabilities. ## Test plan - [ ] `check` job goes green on this PR (would have caught the regressions if `nx affected` were broader). - [ ] After merge, the next Renovate run does not create new PRs for any major (TS, eslint, webpack-cli, etc.). - [ ] Any pending major in the dashboard issue still appears, but only as a checkbox awaiting approval. ## Out of scope (follow-up) Investigate why `nx affected` misses package.json-only changes. Likely a missing entry in `nx.json` `namedInputs` (`default`) or `targetDefaults`. Worth its own focused PR; the dashboard gate is the conservative fix in the meantime. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #43 |
||
|
|
8bb2d7b43f |
chore(deps): defer Prisma major updates pending coordinated upgrade ADR (#39)
## Summary Renovate kept proposing Prisma 7 even though we deliberately downgraded to Prisma 6 in #3 — `nestjs-prisma@0.27.0` is incompatible with Prisma 7's driver-adapter contract, and the upgrade is non-trivial enough to warrant its own ADR rather than a silent Renovate merge. - **`renovate.json`** — add `enabled: false` packageRule for `matchUpdateTypes: ["major"]` on `prisma`, `@prisma/*`, `nestjs-prisma`. Patch and minor bumps of the 6.x line keep flowing. - **ADR-0006** — new "Prisma version pin: 6.x in v1" subsection records the narrowing of "latest stable major" to 6.x and the two triggers for revisiting: 1. `nestjs-prisma` ships a release supporting Prisma 7, or 2. We decide to drop `nestjs-prisma` for a hand-rolled `PrismaModule`. Either path needs its own ADR (schema, client instantiation, request-scoped lifecycle all to re-validate). ## Test plan - [ ] Once merged, the open Prisma 7 PR can be closed (see closure comment below) and won't be recreated. - [ ] Next Renovate run confirms no Prisma-major PR is created (check the dependency dashboard issue). - [ ] Next patch/minor of Prisma 6.x still produces a normal grouped "Prisma" PR. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #39 |
||
|
|
f9ed3cf82a |
chore(ci): skip perf and commits gates on Renovate-authored PRs (#23)
## Summary Renovate's dep-bump PRs run the full pipeline today (`check`, `scan`, `commits`, `perf`, `a11y`). Two of those gates have near-zero signal on a typical bump and dominate the wall-clock cost: - **`perf`** — Lighthouse build + 3-iteration median across the critical-routes list. 3-5 min per PR for a metric that is essentially zero on a patch/minor dep bump (the SPA today serves the static placeholder; even with real routes a typical bump stays inside the median noise floor). - **`commits`** — re-validates commit messages that Renovate generates from a Conventional-Commits-conformant template. Tautological. Skip both when the PR author is the `apf-portal-bot` Gitea user. The `push` event on `main` still runs the full pipeline post-merge, so any regression caught by `perf` is detected seconds after merge — fast enough to revert. Net result: Renovate PRs run `check + scan + a11y` only, ≈ 4-5 min faster per PR. ## ADR amendment ADR-0017 is amended in the same change: - "Where Lighthouse CI runs" table now distinguishes human PR / bot PR / push to main / scheduled / local. - New "Pre-merge gating policy: human PRs vs bot PRs" subsection records the rationale and the human-takeover edge case. - §Confirmation entry for `perf` is reworded to reflect the conditional gate. ## Test plan - [ ] After merge, the next Renovate-triggered PR (auto-rebase or new bump) shows only `check`, `scan`, `a11y` queued — no `perf`, no `commits`. - [ ] A human-opened PR (e.g. this one) still queues all 5 gates. - [ ] On `push` to `main` post-merge, the full pipeline runs including `perf`. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #23 |
||
|
|
f58cf13e33 |
fix(ci): give Renovate a git identity and a github.com token (#13)
## Summary First successful Renovate run (after the docker-image fix in #12) extracted 116 deps cleanly but every branch update failed with `fatal: empty ident name not allowed`, then the whole repo aborted on `Lock file error - aborting`. Two root causes: - **No git identity** — the bot user had an email but no **Full Name** on its Gitea profile, so Renovate produced incomplete git authorship. - **No github.com auth** — Renovate could not look up versions of GitHub-hosted Actions (`actions/checkout`, etc.) or `containerbase/node-prebuild` (the Node binary it dynamically fetches for lockfile maintenance) — anonymous rate limit (60 req/h) hit. Fixes: 1. **`renovate.json`** — pin `gitAuthor` explicitly. The Full Name was also set on the bot's Gitea profile for UI consistency, but the override in config means we don't depend on out-of-band UI state. 2. **`.gitea/workflows/renovate.yml`** — pass `RENOVATE_GITHUB_COM_TOKEN` from the new `GITHUBCOM_TOKEN` repo secret (no underscore between GITHUB and COM — Gitea reserves the `GITHUB_*` namespace). 3. **`docs/development.md`** — onboarding procedure now covers both tokens + the Full Name step. ## Manual setup required after merge Already done in this iteration: - Bot's Full Name set in Gitea ("APF Portal Bot"). - `GITHUBCOM_TOKEN` repo secret created with a zero-scope github.com PAT. (If the PAT was leaked during setup, regenerate before merging — the workflow only references the secret name, not the value.) ## Test plan - [ ] After merge, trigger Renovate manually (Actions → Renovate → Run workflow). - [ ] No `empty ident name` warning in the logs. - [ ] No `Lock file error - aborting`; repo finishes with `INFO: Repository finished … "cloned": true`. - [ ] Renovate creates the dependency dashboard issue + the first batch of grouped PRs (Angular, Nx, NestJS, Prisma, …) signed by `apf-portal-bot`. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #13 |
||
|
|
82911f9319 |
chore(ci): set up Renovate dependency automation (#11)
## Summary
Wire up Renovate to keep dependencies fresh without manual triage.
- **Workflow** — `.gitea/workflows/renovate.yml`: cron daily at 03:00 UTC + `workflow_dispatch`, runs on the existing self-hosted runners. Picked 03:00 specifically so Monday's tick sits inside Renovate's default `lockFileMaintenance.schedule` ("before 4am Monday") and triggers the weekly lockfile refresh in passing.
- **Config** — `renovate.json`: `config:recommended` baseline + groupings (Angular, Nx, NestJS, Prisma, Vitest, TypeScript tooling, ESLint, SWC, Tailwind), Conventional-Commits commit messages, OSV.dev as vulnerability source, dependency dashboard issue.
- **Docs** — new §5 in `docs/development.md` covering the bot onboarding (manual, one-shot), how to trigger Renovate manually, how to review its PRs. The placeholder roadmap entry is dropped from §8.
No ADR — Renovate is operational tooling, not an architectural decision (and on Gitea it's the only viable bot anyway, no real alternative to capture).
## Manual setup required after merge
The workflow is wired but inert until the bot is onboarded once on Gitea:
1. Create a non-admin Gitea user `apf-portal-bot`.
2. Add the bot as a **Write** collaborator on this repo.
3. Sign in as the bot, generate a PAT (scopes: read/write `repository`, read/write `issue`, read `user`).
4. Add the PAT as the repo secret `RENOVATE_TOKEN` (Settings → Actions → Secrets).
Detailed steps in [`docs/development.md`](docs/development.md) → "Dependency updates (Renovate)".
## Test plan
- [ ] After bot onboarding, trigger the workflow manually (Repo → Actions → Renovate → Run workflow).
- [ ] First run creates the "Renovate Dependency Dashboard" issue and a batch of grouped PRs (Angular, Nx, …).
- [ ] Each generated PR has CI green (`check`, `scan`, `commits`, `perf`, `a11y`).
- [ ] Commit subject matches `chore(deps): …` / `fix(deps): …` so the `commits` gate doesn't reject.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #11
|
||
|
|
02bdd43fa1 |
fix(ci): pin act_runner job image to catthehacker/ubuntu:act-22.04 (#4)
## Summary - Pin `act_runner`'s job container image to `catthehacker/ubuntu:act-22.04` via the `<label>:docker://<image>` format on the runner registration labels. - Update `infra/ci-runners.compose.yml`'s `GITEA_RUNNER_LABELS` for all three runner services, with an inline comment explaining the format requirement. - Amend ADR-0015 §"Runners" to specify the chosen image and explain the docker-suffix syntax trap. ## Motivation The first real PR test of the CI pipeline failed at the very first step: ``` Run actions/checkout@v4 0s Cannot find: node in PATH ❌ Failure - Main actions/checkout@v4 ``` Root cause: `act_runner` registers labels (`self-hosted`, `on-prem`) without a `docker://` image suffix. Without that suffix, act spawns jobs in a minimal default container that has no Node. Every JavaScript action (`actions/checkout`, `actions/setup-node`, `nrwl/nx-set-shas`, the Trivy/gitleaks actions) crashes during the action's launch step. None of the five CI gates can run. `catthehacker/ubuntu:act-22.04` is the de facto image used by `act` upstream and the standard recommendation for self-hosted Gitea Actions runners. It bundles Node, Python, git, common build tools, and the Docker CLI — exactly the assumed environment for GitHub Actions-compatible workflows. ## Implementation notes - The fix lives in `GITEA_RUNNER_LABELS` because that's what `act_runner` reads at registration time. The label-to-image mapping is then persisted in the runner's `data/runner-N/.runner` credential. - For runners **already registered** (i.e. the three currently-running `apf-portal-runner-N`), the persisted credential ignores the new env var. Their labels must be updated through Gitea's UI (Site Administration → Actions → Runners → each runner → edit `Labels`). This is documented in the commit message and is an operational follow-up to merging this PR. - The compose-file change applies the next time a runner is re-registered (e.g. when `data/runner-N/` is wiped or a new fourth runner is added). - ADR-0015 amendment is in-place, status remains `accepted`. The runner-image choice is an implementation detail under the existing decision; no new ADR. ## Verification - [ ] `pnpm ci:check` — n/a, this PR only changes infra and ADR docs, no code paths. - [ ] **Manual:** after merge + Gitea label updates, the next PR's `actions/checkout@v4` step runs without `Cannot find: node in PATH`. ## Related - [ADR-0015 — CI/CD pipeline](docs/decisions/0015-cicd-gitea-actions.md). Amended in §"Runners". - [`infra/README.md`](infra/README.md) — operational doc for the runners; mentions the registration workflow but predates this format requirement. A subsequent docs touch could mirror the new label format there too; deferred to keep this PR scoped to the actual fix. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #4 |
||
|
|
f0b437bdc9 |
Merge pull request 'docs: add docs/architecture.md with C4 contexts, Nx boundaries, and CI/CD pipeline' (#1) from docs/architecture-diagrams into main
Reviewed-on: #1 |
||
|
|
156e7ca2df |
chore: add PR template and document title/body convention
Formalise the PR-flow conventions while we install the PR-flow itself. .gitea/pull_request_template.md auto-populates the PR body in Gitea with five sections: Summary / Motivation / Implementation notes / Verification (with CI-gate checkboxes + ADR/diagram update flags) / Related. Sections can be left blank when irrelevant; the template guides without adding ceremony. Header HTML comment reminds the contributor of the PR title format and links to the full convention. docs/development.md §5 (Conventional commit cycle) gains a 'PR conventions' subsection that: - explains why the PR title format matters (squash-merge subject on main, validated by commitlint in the CI 'commits' job) - separates feature-branch commit hygiene (exploratory OK) from PR title hygiene (must conform) - documents the type vocabulary (feat/fix/docs/style/refactor/perf/ test/build/ci/chore/revert) - proposes an optional scope vocabulary (apps, libs, cross-cutting domains like decisions/docs/ci/deps) - describes the body template No new ADR. The PR title format is derived from ADR-0007 (Conventional Commits at the commit-msg layer) plus ADR-0015 (squash-merge means PR title becomes the commit subject on main). The body template is tactical guidance, not architectural. |