feat(infra): Phase 01 packets 4-6 + ADRs 0029-0031 backend swap

[cemililik]

Summary

• Phase 01 packets 4-6 ship the rest of the dev compose stack on top of
  the data-plane scaffolding already on main: Keycloak two-realm identity
  (packet 4), LiveKit OSS + Coturn live-media (packet 5), and Kafka + Vault
  • Dapr sidecar + APISIX gateway (packet 6).
• Three follow-up ADRs record backend choices that were a MinIO /
  Redis / Postgres row in ADR-0002 originally and now have their own
  decision records: ADR-0029 SeaweedFS (MinIO repo archived; license
  trajectory removed the no-phone-home Self-Hosted Air-Gapped path),
  ADR-0030 Valkey (Redis 7.4 was the last BSD-3-Clause release; Valkey
  is the Linux-Foundation-governed RESP-protocol drop-in), ADR-0031
  PostgreSQL 18 (longest LTS runway, native gen_uuid_v7() that the
  ADR-0023 draft can adopt without an extension, async I/O for audit_log
  scans). ADR-0002 gets dated Amendment 1 + 2 without its Decision section
  being rewritten.
• Stack-wide image refresh: every pinned tag bumped to the latest
  stable (Postgres 18.4, Valkey 8.1, SeaweedFS 3.94, Mailpit 1.29.7,
  Meilisearch 1.44, Keycloak 26.6.2, LiveKit 1.12, Coturn 4.11, Kafka CP
  8.2.1, Vault 1.21.4, Dapr 1.17.7, APISIX 3.16); kafka-ui swapped from
  the abandoned provectuslabs fork to the active kafbat/kafka-ui fork.
• Three review-fix rounds addressed during the packet work: port-9000
  collision (resolved by removing the dashboard companion when APISIX moved
  to file-driven data_plane standalone), Keycloak healthcheck (curl not
  in image → bash /dev/tcp probe), Dapr sidecar -app-channel-address host.docker.internal (subscription deliveries to workstation API),
  APISIX mtls route-plugin removed (mTLS in APISIX is SSL-object config,
  not a route plugin), statestore-redis.yamlactorStateStore: "false"
  (ADR-0014 actors out of scope), Keycloak-hub demo operator
  requiredActions: ["CONFIGURE_TOTP"] so the MFA flow surfaces.

What's on the branch

3fd9054 feat(infra): ADR-0030 Valkey + ADR-0031 Postgres 18 + dev-stack swap
0d7aad2 feat(infra): replace MinIO with SeaweedFS across dev compose + docs
9c4a0c6 docs(adr): ADR-0029 SeaweedFS supersedes MinIO storage row
28ea750 chore(infra): bump dev-stack images to latest stable + swap kafka-ui fork
07175ba fix(infra): address Phase 01 packets 4-6 review
4243f3a feat(infra): Phase 01 packet 6 — Kafka + Vault + Dapr + APISIX
be8a10e feat(infra): Phase 01 packet 5 — LiveKit OSS + Coturn live-media stack
2680da9 feat(infra): Phase 01 packet 4 — Keycloak two-realm identity stack

73 files, +2,313 / −218.

What does NOT change (worth flagging up front for reviewers)

• IStorageProvider / ICacheService / IEventBus / ISecretProvider
  contracts — none of these exist as code yet (Phase 02a). The ADRs are
  forward-looking; the adapter implementations will sit behind the same
  interfaces ADR-0014 defined.
• Library / protocol / Dapr-provider-type identifiers preserved:
  StackExchange.Redis, IConnectionMultiplexer, state.redis Dapr
  component, Microsoft.Extensions.Caching.Redis, RedisCacheService
  example names. The Redis→Valkey doc sweep treats these as RESP / .NET
  type identifiers, not vendor brands.
• RLS policy syntax + app.tenant_id / app.organization_id session
  vars — unchanged from Postgres 16 → 18, so the ADR-0003 defense-in-
  depth pattern transfers verbatim.
• APISIX data_plane mode explicitly excludes the Admin API and the
  companion dashboard per ADR-0015's "standalone, no etcd" commitment.
  apisix.yaml is now the only source of truth for routes; diff-review
  replaces the dashboard for dev.

Deliberately deferred to later packets / phases

• make dev orchestrator, .env.example, pre-commit hook, e2e.yml
  companion stack → Phase 01 packet 7 (DX).
• GitHub Actions CI workflow + make seed with two demo tenants →
  Phase 01 packet 8 (CI + seed).
• The IEventBus / ICacheService / ISecretProvider / IStorageProvider
  adapter implementations + the outbox + the IInboxGuard → Phase 02b.
• Production Vault HA / Valkey HA / Postgres replication topology →
  Phase 11 (production hardening).

Test plan

• docker compose -f infra/compose/dev.yml config -q exits 0 (already
  green locally — please confirm on a fresh clone).
• docker compose -f infra/compose/dev.yml up -d brings every service
  to a healthy (or running for services without a probe) state within
  ~90s. Keycloak's first boot is the slowest at ~60s. (Live up smoke
  was not run locally — Docker daemon unreachable on the workstation that
  produced these commits.)
• On boot:
  • Postgres 18 reachable on localhost:5432
    (psql -h localhost -U learnstack -d learnstack -c '\\dt').
  • Valkey reachable on localhost:6379
    (valkey-cli ping returns PONG).
  • SeaweedFS S3 reachable on localhost:9000 with credentials
    learnstack / learnstack-dev-secret; filer UI on localhost:9001.
  • Mailpit web UI on localhost:8025; Meilisearch on localhost:7700.
  • Keycloak admin console on localhost:8080 with admin /
    admin-dev-secret; both learnstack + learnstack-hub realms
    imported.
  • LiveKit signaling reachable on ws://localhost:7880; Coturn on
    localhost:3478.
  • Kafka reachable from kafka-ui on localhost:8081; Vault status
    OK on localhost:8200 with token learnstack-dev-root-token.
  • Dapr sidecar HTTP API on localhost:3500/v1.0/healthz; metadata
    endpoint lists pubsub + statestore + secretstore components
    loaded.
  • APISIX HTTP gateway on localhost:9080; Prometheus metrics on
    localhost:9091.
• Architecture-test project + xUnit smoke tests still compile (no
  backend code changed, but a sanity check on a fresh clone is cheap).
• git grep -nE '\\bMinIO\\b|minio' returns only the 5 intentional
  historical-context sites (ADR-0002, ADR-0029, infra/seaweedfs/README,
  infra/compose/dev.yml inline comment, infra/compose/README.md).
• git grep -nE '\\bRedis\\b' returns only the 6 intentional sites
  (ADR-0002, ADR-0030, ADR-0014 library refs, ADR-0029, dev.yml inline
  comment, statestore-redis.yaml inline comment + library refs in
  architecture/29-dapr-integration.md).
• gh pr view confirms no ADR-0002 Decision section edits — only
  Amendment-block additions.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation

  • Updated architecture and decision records to reflect infrastructure technology changes
  • Expanded development setup documentation with comprehensive local environment guidance
  • Refreshed standards and roadmap documentation for current deployment configurations

• Chores

  • Modernized local development infrastructure stack
  • Updated environment configuration files and initialization scripts
  • Enhanced developer experience with improved local setup automation

[sourcery-ai]

Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR modernizes LearnStack's infrastructure stack by adopting Valkey as the Redis-compatible cache/state backend, SeaweedFS as the object storage backend, and PostgreSQL 18 as the primary RDBMS, formalized through three new architecture decision records (ADRs 0029–0031) and accompanied by comprehensive updates to architecture documentation, development standards, roadmap, and a substantial expansion of the local Docker Compose development environment with Keycloak identity, LiveKit media, Kafka events, Vault secrets, Dapr sidecar, and APISIX API gateway.

Changes

Infrastructure Modernization: Valkey + SeaweedFS + PostgreSQL 18

Layer / File(s)	Summary
Architecture Decision Records (ADR-0029, ADR-0030, ADR-0031) docs/decisions/0029-object-storage-seaweedfs.md, docs/decisions/0030-redis-compatible-store-valkey.md, docs/decisions/0031-postgresql-major-version.md	Three new ADRs establish SeaweedFS as object storage backend across all deployment modes (ADR-0029), Valkey as Redis-compatible state/cache backend with RESP drop-in compatibility (ADR-0030), and PostgreSQL 18.x as primary RDBMS for LTS runway and feature parity (ADR-0031). Each document decision drivers, alternatives considered, context, implementation notes, and consequences.
ADR Amendments and Index Updates docs/decisions/0002-initial-architecture.md, docs/decisions/0003-tenant-isolation-defense-in-depth.md, docs/decisions/0010-cross-module-communication.md, docs/decisions/0014-adopt-dapr.md, docs/decisions/0017-tenant-organization-hierarchy.md, docs/decisions/0018-tenant-driven-customization-model.md, docs/decisions/0022-custom-domain-tls.md, docs/decisions/README.md	Existing ADRs 0002, 0003, 0010, 0014, 0017, 0018, 0022 amended with blocks superseding backend rows (storage to SeaweedFS, cache to Valkey, PostgreSQL to 18.x); ADR index updated to record new decisions and supersession relationships.
Architecture Documentation (02–32) docs/architecture/*	All 32 architecture documents consistently updated to reflect Valkey/SeaweedFS/PostgreSQL 18: domain model, module boundaries, technical architecture (with Mermaid diagram rewires), MVP scope, extension model, live classroom, cost model, tenant isolation, media pipeline, WebRTC, feature flags, data protection, deployment models (SaaS/Dedicated/Self-Hosted), Dapr integration, and tenant customization.
Standards, Principles, and Glossary docs/standards/00-principles.md, docs/standards/05-database.md, docs/standards/06-testing.md, docs/standards/09-error-handling.md, docs/standards/10-observability.md, docs/standards/11-security.md, docs/standards/12-infrastructure.md, docs/standards/15-performance.md, docs/standards/20-infrastructure-stack.md, docs/glossary.md	Development standards updated to specify Valkey (state store), SeaweedFS (object storage), and PostgreSQL 18+ in all environments; Testcontainers integration testing updated to Postgres/Valkey/SeaweedFS; observability, security, and infrastructure networking updated with new backend references; glossary refreshed with infrastructure and service definitions.
Roadmap and Skills Documentation docs/roadmap/phase-00-product-architecture.md, docs/roadmap/phase-01-repository-tooling.md, docs/roadmap/phase-02a-kernel-tenancy.md, docs/roadmap/phase-04-cms-media-pages.md, docs/roadmap/phase-08c-classroom.md, .claude/skills/README.md, .claude/skills/add-feature-key/SKILL.md, .claude/skills/add-integration-test/SKILL.md, .claude/skills/local-dev-setup/SKILL.md, .claude/skills/run-tests-locally/SKILL.md, .claude/skills/standards-check/SKILL.md	Roadmap phases updated to reflect Valkey/SeaweedFS/PostgreSQL 18 and Phase 01 completion status; developer skills guides updated to reference new infrastructure, testing patterns, and deployment mode options.
Root Configuration and Project Metadata .gitignore, CLAUDE.md, README.md, backend/Directory.Packages.props, backend/README.md	Root docs updated to reflect Valkey/SeaweedFS volumes, PostgreSQL 18 in local dev targets, and direction roadmap; .gitignore adds valkey-data/ and seaweedfs-data/, removes redis-data/ and minio-data/; Testcontainers documentation clarifies Valkey compatibility via Testcontainers.Redis with RESP-compatible image selection.
Docker Compose Scaffold and Core Services infra/compose/dev.yml, infra/compose/postgres-init/01-create-keycloak-db.sql	Header rewritten, x-host-gateway anchor added for Linux support; PostgreSQL 18.4 with postgres-init mount and pg_isready healthcheck; Valkey replacing Redis with AOF and healthcheck; SeaweedFS replacing MinIO with S3 gateway, identity config mount, and master health probe; all named volumes updated.
Dapr Components and Configuration infra/dapr/README.md, infra/dapr/components/pubsub-kafka.yaml, infra/dapr/components/statestore-redis.yaml, infra/dapr/components/secretstore-vault.yaml, infra/dapr/config/dapr-config.yaml	Dapr sidecar setup documented: building blocks adopted (Kafka pub/sub, Valkey state store, Vault secrets), host networking for dev, abstraction pattern enforcement; Kafka pub/sub component configured for learnstack-api consumer group; Valkey state store (RESP protocol) configured; Vault secret store configured with dev root token; tracing configuration with full sampling.
Keycloak Identity: Two-Realm Configuration infra/keycloak/README.md, infra/keycloak/realms/learnstack.json, infra/keycloak/realms/learnstack-hub.json	Two hard-isolated Keycloak realms: learnstack for tenant users with roles (tenant-admin, tenant-instructor, tenant-learner) and demo users; learnstack-hub for operator/admin with roles (hub-platform-admin, hub-operator, hub-billing-viewer) and demo operator user. Complete realm JSONs with clients, MFA, SMTP via Mailpit, and zero cross-trust invariants documented; reseed procedures explained.
LiveKit and Coturn Media Services infra/livekit/README.md, infra/livekit/livekit.yaml, infra/coturn/turnserver.conf	LiveKit OSS dev config with service port 7880, RTC TCP 7881, dev API key/secret, room auto-creation, and logging. Coturn config with STUN/TURN ports, relay port range, long-term credentials (devuser:devsecret), and hardening options. README documents endpoints, dev token signing, recording flow via ILiveClassProvider, and excluded components.
SeaweedFS Object Storage Configuration infra/seaweedfs/README.md, infra/seaweedfs/s3-identities.json	SeaweedFS S3-compatible gateway with dev identity (learnstack-dev credential with full S3 actions). README documents local endpoints (S3 gateway, filer UI, master API, volume API), tenant isolation via {tenant_id}/... key-prefix, reseed/wipe procedures, and excluded components.
APISIX API Gateway: File-Driven Configuration infra/apisix/README.md, infra/apisix/config.yaml, infra/apisix/apisix.yaml	APISIX standalone data-plane deployment: config.yaml sets deployment role, YAML provider, NGINX tuning, plugin universe (OIDC, JWT, rate limiting, IP restriction, request-id, Prometheus); apisix.yaml defines /healthz (public, CORS), /api/v*/** OPTIONS (preflight), /api/v*/** authenticated (GET/POST/PUT/PATCH/DELETE), and commented /api/internal/* mTLS stub. README explains hot-reload, upstream addressing, plugin chain, and excluded etcd/Admin API/dashboard.
Compose Infrastructure Documentation infra/compose/README.md	Phase-by-phase walkthrough of dev.yml: data plane (PostgreSQL 18, Valkey, SeaweedFS, Mailpit, Meilisearch), identity (Keycloak two realms with import/reseed), media (LiveKit, Coturn), and eventing (Kafka KRaft, Vault dev, Dapr, APISIX). Service descriptions, versions, endpoints, credentials, and tenant isolation patterns documented.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐰 From Redis past to Valkey's light,
SeaweedFS shores a storage flight,
PostgreSQL's eighteen-strong foundation,
Keycloak realms in separation,
Docker sings with all compose delight! 🌟

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/phase-01-infra-identity-eventing

[gemini-code-assist]

Code Review

This pull request executes a significant infrastructure migration by replacing MinIO with SeaweedFS for object storage and Redis with Valkey for caching and state management, while also pinning PostgreSQL to major version 18. It completes the local development stack by integrating Keycloak, LiveKit, Kafka, Vault, Dapr, and APISIX into the Docker Compose environment. The changes include extensive updates to architectural documentation, ADRs, and test configurations to align with these new backend selections. I have no feedback to provide as there were no review comments.

[coderabbitai]

Actionable comments posted: 8

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)

docs/standards/05-database.md (1)

4-10: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add ADR-0031 to the standards provenance.

Line 16 introduces a non-trivial baseline change (PostgreSQL 18+), but the Derives from list does not cite the new decision record. Please add ADR-0031 in the header so the rule is traceable.

Suggested patch

 **Derives from:** [ADR-0002 Initial Architecture](../decisions/0002-initial-architecture.md),
[ADR-0003 Tenant Isolation Defense in Depth](../decisions/0003-tenant-isolation-defense-in-depth.md)
(Amendment 1: Organization Scope),
[ADR-0006 Events and Outbox](../decisions/0006-events-and-outbox.md)
(Amendment 1: Dapr pub/sub dispatch transport),
[ADR-0014 Adopt Dapr](../decisions/0014-adopt-dapr.md),
-[ADR-0017 Tenant + Organization Hierarchy](../decisions/0017-tenant-organization-hierarchy.md).+[ADR-0017 Tenant + Organization Hierarchy](../decisions/0017-tenant-organization-hierarchy.md),+[ADR-0031 PostgreSQL Major Version](../decisions/0031-postgresql-major-version.md).

As per coding guidelines, "Standards changes must cite an ADR for non-trivial rule changes or new rules".

Also applies to: 16-16

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/standards/05-database.md` around lines 4 - 10, The standards header
"Derives from:" is missing a reference to ADR-0031 despite a non-trivial
baseline change noting "PostgreSQL 18+"; update the "Derives from:" list to
include ADR-0031 so the provenance is complete—locate the header block that
begins with "**Derives from:**" and add "ADR-0031" (with its title if you follow
the existing pattern) to the comma-separated entries.

docs/standards/06-testing.md (1)

10-19: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add diagram title and text fallback for the test pyramid.

The Mermaid block is missing the required title + bullet fallback, so it is not fully readable in plain-text contexts.

As per coding guidelines, "Use Mermaid for diagrams in fenced mermaid blocks; diagrams must remain readable in text form with titles and bullet fallbacks".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/standards/06-testing.md` around lines 10 - 19, The Mermaid diagram lacks
a human-readable title and plain-text fallback; update the fenced ```mermaid```
block that defines nodes e2e, contract, integration, arch, and unit by adding a
visible diagram title (e.g., "Test Pyramid" via a title statement) and include a
short bullet-list fallback immediately before or after the mermaid block that
lists the same five items (End-to-end / Playwright, Contract & API tests,
Integration tests, Architecture tests, Unit tests) so the diagram remains
understandable in plain-text contexts.

docs/standards/12-infrastructure.md (1)

52-70: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Local compose stack list is stale (redis, apisix-dashboard).

In this block, Line 54 still lists redis and Line 69 lists apisix-dashboard, but this PR’s infra direction is Valkey + APISIX data_plane (dashboard removed). Please update this list to avoid operator/developer confusion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/standards/12-infrastructure.md` around lines 52 - 70, Update the local
compose stack list by removing the stale entries "redis" and "apisix-dashboard"
and replacing them with the new infra pieces: add "valkey" and
"apisix-data_plane" (or "apisix-data-plane" to match naming convention used
elsewhere); edit the block that currently contains "redis" and
"apisix-dashboard" so the list reflects Valkey + APISIX data_plane instead of
the old Redis and dashboard entries.

.claude/skills/local-dev-setup/SKILL.md (2)

48-49: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the current deployment mode names (SelfHostedOnline / SelfHostedAirGapped).

This skill still documents SelfHosted, which conflicts with the current split-mode model used elsewhere. Please update both the inputs and mode table to the two explicit values.

Also applies to: 171-174

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/local-dev-setup/SKILL.md around lines 48 - 49, Update the
deployment mode documentation to use the current split-mode names: replace any
occurrence of "SelfHosted" in the Deployment mode table and input descriptions
with the two explicit values "SelfHostedOnline" and "SelfHostedAirGapped" (e.g.,
update the table row that currently lists `Development / SaaS / Dedicated /
SelfHosted` to `Development / SaaS / Dedicated / SelfHostedOnline /
SelfHostedAirGapped`), and make the same replacements in the inputs section
referenced around the later occurrence (the block near the original 171-174).
Ensure both the table header/values and any example input keys or explanatory
text (the lines mentioning deployment mode) consistently use the new names.

---

115-116: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove APISIX dashboard guidance if data-plane stack no longer ships it.

The line still advertises an optional dashboard, but this PR’s infra fixes explicitly removed dashboard usage for APISIX data_plane mode. Keeping this here will send devs to a non-existent surface.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/local-dev-setup/SKILL.md around lines 115 - 116, Remove the
obsolete "APISIX dashboard (optional) | 9000 | Route inspection." table row from
the SKILL.md table: locate the row that advertises the APISIX dashboard and
delete it (or replace it with a note that the APISIX data-plane no longer
includes a dashboard) so the ports/hosts list no longer points developers to a
non-existent dashboard surface.

docs/roadmap/phase-01-repository-tooling.md (1)

175-175: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve APISIX dashboard contradiction in this phase doc.

Line 175 says apisix-dashboard (dev only), but Lines 46-47 state standalone data_plane with no companion dashboard. Keep one authoritative statement here to prevent insecure or invalid setup assumptions.

Proposed doc fix

-- **APISIX** (standalone YAML-reload mode) + apisix-dashboard (dev only).+- **APISIX** (standalone YAML-reload mode, `data_plane`; no Admin API/dashboard).

Based on learnings: Single source of truth: each piece of knowledge lives in exactly one place (glossary for terms, ADRs for decisions, standards for ongoing rules, architecture docs for conceptual descriptions, roadmap for phases).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/roadmap/phase-01-repository-tooling.md` at line 175, The roadmap
currently contradicts itself about APISIX: change the phrasing so only one
authoritative statement exists by either removing "apisix-dashboard (dev only)"
from the APISIX line or updating the earlier "data_plane" mention to explicitly
include the dashboard; reference the unique terms APISIX, apisix-dashboard, and
data_plane to locate the lines and make them consistent, and add a short note
pointing readers to the canonical source (glossary/ADR/architecture doc) for the
final decision to enforce single-source-of-truth.

CLAUDE.md (1)

29-33: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update Phase-01 status text to match current scope.

The status paragraph still says Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX will “land incrementally,” but this PR already includes those packets. This is now stale and onboarding-misleading.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 29 - 33, The Phase-01 status paragraph in CLAUDE.md
still claims Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX “will land incrementally”;
update that paragraph to reflect that those packets are already included in this
PR by replacing the future-tense wording with present-tense confirmation (e.g.,
state that Keycloak, LiveKit, Kafka, Vault, Dapr, and APISIX are included),
remove the misleading “will land incrementally” phrase, and adjust any remaining
scope/next-steps text to accurately list only items truly outstanding; look for
the "Phase-01" status paragraph and the mention of
Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX to make this change.

🟡 Minor comments (13)

docs/decisions/0014-adopt-dapr.md-15-19 (1)

15-19: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align component file names with current infra paths.

Line 17–19 still reference generic component filenames, but this PR’s infra layer uses infra/dapr/components/pubsub-kafka.yaml and infra/dapr/components/statestore-redis.yaml. Please update the table paths to avoid stale guidance.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/decisions/0014-adopt-dapr.md` around lines 15 - 19, Update the component
file paths in the DAPR decision table to match the infra layer names: replace
`dapr/components/pubsub.yaml` with `infra/dapr/components/pubsub-kafka.yaml` and
replace `dapr/components/statestore.yaml` with
`infra/dapr/components/statestore-redis.yaml` (leave the secret store
`secretstore-vault.yaml` entry as-is if it already matches
`infra/dapr/components/secretstore-vault.yaml`); locate the table in
docs/decisions/0014-adopt-dapr.md and update the three Component file cell
values accordingly.

docs/decisions/0002-initial-architecture.md-76-77 (1)

76-77: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Correct ADR-0023 link target.

Line 76 links [ADR-0023 draft] to README.md instead of the ADR file, which makes navigation ambiguous.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/decisions/0002-initial-architecture.md` around lines 76 - 77, The link
target for the text "[ADR-0023 draft]" incorrectly points at README.md; update
the link so the anchor points to the actual ADR-0023 markdown file (replace
README.md with the ADR-0023 filename) while keeping the link text unchanged,
ensuring the reference resolves to the ADR document rather than README.

docs/decisions/0031-postgresql-major-version.md-22-23 (1)

22-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix broken ADR-0023 references.

Line 22 and Line 181 link ADR-0023 to README.md, which resolves to the decisions index, not the ADR itself. Please point both references to the actual ADR-0023 file path.

Also applies to: 181-182

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/decisions/0031-postgresql-major-version.md` around lines 22 - 23, The
markdown links in this document that reference "ADR-0023 (Strongly-typed ID
source generator)" currently point to README.md; update both occurrences of that
link (the one near the top and the one later in the file) to point to the actual
ADR-0023 markdown file (replace README.md with the ADR-0023 file name/slug for
the Strongly-typed ID ADR) so the link resolves directly to the ADR-0023
document.

docs/architecture/09-tenant-isolation.md-198-201 (1)

198-201: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language tag to the fenced block.

The storage example block is missing a fenced code language specifier, which triggers markdown lint (MD040).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/09-tenant-isolation.md` around lines 198 - 201, The fenced
code block showing the storage example (the block containing
"tenants/{tenant_id}/organizations/{org_id}/courses/{course_id}/... ←
org-scoped" and "tenants/{tenant_id}/brand/... ← tenant-wide") is missing a language tag which triggers MD040; add an
appropriate language identifier (e.g., "text" or "yaml") after the opening
triple backticks of that fenced block so the markdown linter recognizes the
language and the lint error is resolved.

docs/architecture/04-technical-architecture.md-10-10 (1)

10-10: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align DB version wording with the pinned major policy.

Line 10 says PostgreSQL 18+, but this PR’s architecture decision is a pin to 18.x. Please tighten this to avoid drift in upgrade expectations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/04-technical-architecture.md` at line 10, The DB version
wording should be tightened from a floating "PostgreSQL 18+" to the pinned major
as described in ADR-0003; update the table entry that currently reads
"PostgreSQL 18+ (shared schema + RLS isolation; ADR-0003)" to the explicit
pinned-major wording like "PostgreSQL 18.x (shared schema + RLS isolation;
ADR-0003)" so the architecture doc and ADR align.

docs/architecture/04-technical-architecture.md-208-212 (1)

208-212: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix stale redis naming in Local Infrastructure list.

This block still lists redis while the document now standardizes on Valkey; update the service label to keep setup docs consistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/04-technical-architecture.md` around lines 208 - 212,
Replace the stale "redis" entry in the Local Infrastructure list with the
standardized service name "Valkey" (match casing used elsewhere in the docs),
i.e., update the list item in the block containing "postgres / redis / seaweedfs
/ meilisearch" so it reads "postgres / Valkey / seaweedfs / meilisearch"; ensure
any other nearby occurrences in that same Local Infrastructure section are also
renamed to "Valkey" for consistency.

docs/architecture/29-dapr-integration.md-83-83 (1)

83-83: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify Valkey-vs-Redis naming in the state store snippet.

This section declares a Valkey state store, but the sample still uses Redis-shaped host/secret identifiers. Add a one-line note here that state.redis/redis* keys are Dapr component naming, while runtime points to Valkey, to avoid operator misconfiguration.

Suggested doc tweak

 ### `statestore.yaml` — Valkey state store
+Note: Dapr uses the `state.redis` component and `redis*` metadata names for Redis-compatible backends; in LearnStack this backend is Valkey.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/29-dapr-integration.md` at line 83, Add a one-line
clarifying note under the "statestore.yaml — Valkey state store" heading that
explains the apparent Redis-shaped keys are Dapr component naming only (e.g.,
state.redis / redis*), and that although the keys look like Redis identifiers
the runtime component is configured to use Valkey; reference "statestore.yaml",
"Valkey", and the "state.redis"/"redis*" keys in the note so operators
understand the naming vs actual provider.

.claude/skills/add-feature-key/SKILL.md-268-270 (1)

268-270: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align cache TTL guidance with the infrastructure standard.

This currently states the Valkey layer is 60s TTL, but the standard cache policy uses 60s for L1 and 15-min upper bound for L2/Valkey. Please sync wording to prevent conflicting implementation guidance.

Based on learnings, “Single source of truth: each piece of knowledge lives in exactly one place.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/add-feature-key/SKILL.md around lines 268 - 270, Update the
TTL guidance in the SKILL.md paragraph referencing the Valkey layer and
IsEnabledAsync: change the sentence that currently says "Valkey layer (60s
TTL...)" to state the standard cache policy—L1 TTL 60s and L2/Valkey TTL up to
15 minutes (with Valkey still eager-invalidated by the Dapr event), and add a
short note reinforcing "Single source of truth: each piece of knowledge lives in
exactly one place." Ensure you update the line mentioning the Valkey layer and
IsEnabledAsync so readers get the correct L1/L2 TTL values and invalidation
behavior.

README.md-23-25 (1)

23-25: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix stale Phase-01 packet status in the top-level README.

This still says Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX “land in subsequent Phase-01 packets,” but those packets are already present in this PR.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 23 - 25, Update the README.md sentence that says
"land in subsequent Phase-01 packets" to reflect that Keycloak, LiveKit, Kafka,
Vault, Dapr, APISIX (and other items listed) are now included in this PR;
specifically edit the phrase containing "land in subsequent Phase-01 packets"
and, if needed, adjust or remove the reference to
docs/roadmap/phase-01-repository-tooling.md so the top-level README accurately
states these components are present in the current Phase-01 packet rather than
pending.

README.md-33-34 (1)

33-34: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align Valkey version text with compose pin.

Line 33 documents “Valkey 7,” but infra/compose/dev.yml pins valkey/valkey:8.1-alpine. Keep the README version in sync to avoid setup confusion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 33 - 34, The README's dependency list currently says
"Valkey 7" but the compose pin in infra/compose/dev.yml uses
valkey/valkey:8.1-alpine; update the README text to match the pinned version
(e.g., change "Valkey 7" to "Valkey 8.1" or "Valkey 8.1-alpine") so the
documentation and the infra compose pin are consistent; ensure the same phrasing
appears where the README references Valkey in the Cache / Pub-Sub / Secrets
line.

infra/dapr/README.md-23-23 (1)

23-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language to the fenced code block (markdownlint MD040).

The fence starting at Line 23 should include a language (e.g., text) to satisfy linting.

Suggested fix

-```+```text
┌──────────────────────────┐ ┌──────────────────────────────────────────┐
...
-```+```

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/dapr/README.md` at line 23, Update the fenced code block shown in the
diff by adding a language identifier to the opening triple backticks (e.g.,
change ``` to ```text) so the markdownlint MD040 rule is satisfied; locate the
code fence that contains the ASCII box art and add "text" (or another
appropriate language) after the opening backticks.

infra/compose/dev.yml-277-277 (1)

277-277: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Follow Kafka KRaft's official CLUSTER_ID generation method.

The value learnstack-dev-cluster-id-1 does not follow the standard Kafka format. KRaft expects CLUSTER_ID to be a URL-safe Base64-encoded UUID generated via kafka-storage.sh random-uuid, not a custom string. While this custom value matches the allowed character set and won't cause initialization failure on an already-established cluster, it diverges from Kafka's design and best practices.

Generate and use the proper format:

- CLUSTER_ID: learnstack-dev-cluster-id-1+ CLUSTER_ID: <output from: kafka-storage.sh random-uuid>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/compose/dev.yml` at line 277, The CLUSTER_ID value currently set as
learnstack-dev-cluster-id-1 does not follow Kafka KRaft's required format;
replace the hard-coded CLUSTER_ID in the dev.yml service environment with a
KRaft-generated UUID (run kafka-storage.sh random-uuid on your Kafka binaries or
CI runner) and paste that URL-safe Base64-encoded UUID into the CLUSTER_ID
environment variable so the KRaft broker uses the official cluster identifier
format.

infra/apisix/README.md-34-38 (1)

34-38: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced plugin-chain block.

Line 34 starts a fenced block without a language (MD040), which can trip markdownlint in CI.

♻️ Proposed fix

-```+```text
real-ip → cors → openid-connect → limit-req → request-id → proxy-rewrite → upstream
↓
prometheus (response)

</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @infra/apisix/README.md around lines 34 - 38, The fenced code block showing
the plugin chain (the triple-backtick block containing "real-ip → cors →
openid-connect → limit-req → request-id → proxy-rewrite → upstream") is missing
a language identifier and triggers MD040; update that fenced block to include a
language tag such as "text" (e.g., change totext) so markdownlint stops
flagging it, leaving the block content unchanged.

</details>
</blockquote></details>
</blockquote></details>
<details>
<summary>🧹 Nitpick comments (2)</summary><blockquote>
<details>
<summary>infra/compose/dev.yml (1)</summary><blockquote>
`293-293`: _⚡ Quick win_
**Pin `kafka-ui` to a fixed image tag.**
Using `:latest` makes the dev stack non-reproducible and can introduce surprise breakages between runs.
<details>
<summary>Suggested fix</summary>
```diff
- image: ghcr.io/kafbat/kafka-ui:latest
+ image: ghcr.io/kafbat/kafka-ui:<pinned-version>
```
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/compose/dev.yml` at line 293, Replace the non-reproducible image
reference "ghcr.io/kafbat/kafka-ui:latest" for the kafka-ui service with a
fixed, explicit tag (e.g. ghcr.io/kafbat/kafka-ui:<VERSION>) or an
environment-variable-backed tag (e.g. ghcr.io/kafbat/kafka-ui:${KAFKA_UI_TAG})
to pin the image; update the image line in the kafka-ui service block (replace
":latest") and document/set the chosen <VERSION> or KAFKA_UI_TAG in your dev
environment or .env so runs are reproducible.
```
</details>
</blockquote></details>
<details>
<summary>infra/dapr/config/dapr-config.yaml (1)</summary><blockquote>
`11-14`: _⚡ Quick win_
**Disable tracing sampling when no Zipkin endpoint is configured.**
With `samplingRate: "1"` and an empty `zipkin.endpointAddress`, the sidecar generates spans that fail to export, creating unnecessary telemetry overhead in development. Set sampling to `"0"` until an exporter endpoint is available.
<details>
<summary>Suggested adjustment</summary>
```diff
spec:
tracing:
- samplingRate: "1"
- zipkin:
- endpointAddress: ""
+ samplingRate: "0"
features: []
```
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/dapr/config/dapr-config.yaml` around lines 11 - 14, The dapr config
currently sets samplingRate: "1" while zipkin.endpointAddress is empty, causing
generated spans to fail exporting; change the samplingRate value to "0" when
zipkin.endpointAddress is not configured (i.e., set samplingRate: "0") so
tracing is disabled until a valid Zipkin endpoint is provided—update the
samplingRate entry in the config (referencing the samplingRate and
zipkin.endpointAddress keys) accordingly.
```
</details>
</blockquote></details>
</blockquote></details>
<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @backend/Directory.Packages.props:

• Line 40: The PackageVersion entry for Testcontainers.Valkey (PackageVersion
  Include="Testcontainers.Valkey" Version="3.10.0") references a package that does
  not exist on NuGet and causes restore failures; remove this PackageVersion or
  replace it with the correct package name/version (for example remove
  Testcontainers.Valkey if unused, or change to the proper Testcontainers.*
  package such as Testcontainers.PostgreSql if that was intended) and ensure any
  project references relying on Testcontainers.Valkey are updated to the correct
  package symbol.

In @docs/architecture/08-livekit-cost-model.md:

• Line 225: The table row string "LiveKit OSS Hetzner | 3× SFU + 2× Egress +
  Valkey + TURN + 1 TB recording storage ≈ $650–800" understates recording
  storage: replace the "1 TB recording storage" with the correct estimate based on
  the document's ~60 MB/min assumption (300,000 minutes → ~18 TB) and update the
  cost line accordingly (adjust the ≈ $650–800 to reflect the higher storage
  cost or annotate that storage is additional ~18 TB), ensuring the same table
  cell/row format and the exact phrase "LiveKit OSS Hetzner" is preserved.

In @docs/decisions/0002-initial-architecture.md:

• Around line 21-22: The ADR-0002 decision section has been edited (lines
  replacing the original "decision/consequence" content) which must remain
  immutable; revert any changes made to ADR-0002's decision body and restore the
  original text, then create a new amendment or superseding ADR that records the
  backend stack changes (PostgreSQL 18.x, Valkey, SeaweedFS, .NET 10, etc.) and
  reference ADR-0002 from that new ADR; ensure the file named
  0002-initial-architecture.md contains only the original accepted decision text
  and move all suggested backend updates into a separate ADR or amendment document
  that supersedes or amends ADR-0002.

In @docs/standards/20-infrastructure-stack.md:

• Around line 194-195: The sentence that currently groups "in-process / Valkey
  layer" as having a 60s TTL must be split: update the wording so that the
  in-process (L1) cache TTL is stated as 60s, and the Valkey (L2) cache TTL is
  described as having a 15-minute upper bound; keep the note that eager
  invalidation flows from the Dapr event and that the TTL is a safety net. Locate
  and edit the sentence mentioning "in-process / Valkey layer" and replace it with
  two concise clauses referencing L1 (in-process) = 60s and L2/Valkey = 15 min
  upper bound while preserving the Dapr invalidation and safety-net wording.
• Around line 52-53: Update the standards doc to cite the relevant ADRs for the
  infrastructure rule changes: add links/references to ADR-0029 and ADR-0030 next
  to the table entries and header where Cache (InMemoryCacheService →
  DaprCacheService → Valkey) and Secrets (EnvironmentSecretProvider →
  DaprSecretProvider → Vault) are described, and include ADR-0031 as well if
  Postgres policy (any Postgres mentions) is affected; ensure the ADR IDs are
  clearly referenced in the standards header/reference list and adjacent to the
  specific symbols InMemoryCacheService, DaprCacheService, Valkey,
  EnvironmentSecretProvider, DaprSecretProvider, Vault, and Postgres mentioned in
  the document.

In @infra/compose/dev.yml:

• Around line 93-97: The compose file maps two services to the same host port
  "8080:8080" (SeaweedFS volume HTTP API and Keycloak), causing a bind collision;
  update one of the mappings (e.g., change Keycloak's host binding from
  "8080:8080" to an unused host port like "8081:8080" or change SeaweedFS instead)
  so the host ports are unique, and then update any references (docs, env vars,
  tests) that expect the old host port; locate the mappings by the port strings
  "8080:8080" and the service names SeaweedFS/Keycloak in the compose YAML and
  change the left-hand host port accordingly.

In @infra/keycloak/realms/learnstack-hub.json:

• Line 79: Change the public client object where "fullScopeAllowed" is set to
  true: set "fullScopeAllowed" to false and add explicit client scopes or
  scopeMappings that grant only the necessary roles to the client token.
  Specifically, update the client’s "defaultClientScopes" or add
  "scopeMappings"/"roleMappings" entries that include the operator portal roles
  (e.g., hub-operator and any other required roles such as hub-platform-admin or
  hub-billing-viewer as appropriate) so tokens retain needed role claims; ensure
  these mappings reference the realm roles used by your operator portal.

In @infra/keycloak/realms/learnstack.json:

• Around line 68-75: The learnstack-api client config is over-permissive:
  disable direct access grants by setting "directAccessGrantsEnabled" to false
  (remove deprecated direct grant use for a confidential service-account), replace
  the wildcard webOrigins entry ("webOrigins": ["+"]) with an empty array or
  explicit origins (e.g., []) so backend-only clients have no permissive CORS
  origins, and ensure "fullScopeAllowed" is explicitly set to false for the client
  (add "fullScopeAllowed": false) to prevent automatic granting of all
  roles/scopes to the service account; keep "serviceAccountsEnabled": true and
  only whitelist necessary "redirectUris" if required.

---

Outside diff comments:
In @.claude/skills/local-dev-setup/SKILL.md:

• Around line 48-49: Update the deployment mode documentation to use the current
  split-mode names: replace any occurrence of "SelfHosted" in the Deployment mode
  table and input descriptions with the two explicit values "SelfHostedOnline" and
  "SelfHostedAirGapped" (e.g., update the table row that currently lists
  Development / SaaS / Dedicated / SelfHosted to Development / SaaS / Dedicated / SelfHostedOnline / SelfHostedAirGapped), and make the same replacements in
  the inputs section referenced around the later occurrence (the block near the
  original 171-174). Ensure both the table header/values and any example input
  keys or explanatory text (the lines mentioning deployment mode) consistently use
  the new names.
• Around line 115-116: Remove the obsolete "APISIX dashboard (optional) | 9000 |
  Route inspection." table row from the SKILL.md table: locate the row that
  advertises the APISIX dashboard and delete it (or replace it with a note that
  the APISIX data-plane no longer includes a dashboard) so the ports/hosts list no
  longer points developers to a non-existent dashboard surface.

In @CLAUDE.md:

• Around line 29-33: The Phase-01 status paragraph in CLAUDE.md still claims
  Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX “will land incrementally”; update that
  paragraph to reflect that those packets are already included in this PR by
  replacing the future-tense wording with present-tense confirmation (e.g., state
  that Keycloak, LiveKit, Kafka, Vault, Dapr, and APISIX are included), remove the
  misleading “will land incrementally” phrase, and adjust any remaining
  scope/next-steps text to accurately list only items truly outstanding; look for
  the "Phase-01" status paragraph and the mention of
  Keycloak/LiveKit/Kafka/Vault/Dapr/APISIX to make this change.

In @docs/roadmap/phase-01-repository-tooling.md:

• Line 175: The roadmap currently contradicts itself about APISIX: change the
  phrasing so only one authoritative statement exists by either removing
  "apisix-dashboard (dev only)" from the APISIX line or updating the earlier
  "data_plane" mention to explicitly include the dashboard; reference the unique
  terms APISIX, apisix-dashboard, and data_plane to locate the lines and make them
  consistent, and add a short note pointing readers to the canonical source
  (glossary/ADR/architecture doc) for the final decision to enforce
  single-source-of-truth.

In @docs/standards/05-database.md:

• Around line 4-10: The standards header "Derives from:" is missing a reference
  to ADR-0031 despite a non-trivial baseline change noting "PostgreSQL 18+";
  update the "Derives from:" list to include ADR-0031 so the provenance is
  complete—locate the header block that begins with "Derives from:" and add
  "ADR-0031" (with its title if you follow the existing pattern) to the
  comma-separated entries.

In @docs/standards/06-testing.md:

• Around line 10-19: The Mermaid diagram lacks a human-readable title and
  plain-text fallback; update the fenced mermaid block that defines nodes
  e2e, contract, integration, arch, and unit by adding a visible diagram title
  (e.g., "Test Pyramid" via a title statement) and include a short bullet-list
  fallback immediately before or after the mermaid block that lists the same five
  items (End-to-end / Playwright, Contract & API tests, Integration tests,
  Architecture tests, Unit tests) so the diagram remains understandable in
  plain-text contexts.

In @docs/standards/12-infrastructure.md:

• Around line 52-70: Update the local compose stack list by removing the stale
  entries "redis" and "apisix-dashboard" and replacing them with the new infra
  pieces: add "valkey" and "apisix-data_plane" (or "apisix-data-plane" to match
  naming convention used elsewhere); edit the block that currently contains
  "redis" and "apisix-dashboard" so the list reflects Valkey + APISIX data_plane
  instead of the old Redis and dashboard entries.

---

Minor comments:
In @.claude/skills/add-feature-key/SKILL.md:

• Around line 268-270: Update the TTL guidance in the SKILL.md paragraph
  referencing the Valkey layer and IsEnabledAsync: change the sentence that
  currently says "Valkey layer (60s TTL...)" to state the standard cache policy—L1
  TTL 60s and L2/Valkey TTL up to 15 minutes (with Valkey still eager-invalidated
  by the Dapr event), and add a short note reinforcing "Single source of truth:
  each piece of knowledge lives in exactly one place." Ensure you update the line
  mentioning the Valkey layer and IsEnabledAsync so readers get the correct L1/L2
  TTL values and invalidation behavior.

In @docs/architecture/04-technical-architecture.md:

• Line 10: The DB version wording should be tightened from a floating
  "PostgreSQL 18+" to the pinned major as described in ADR-0003; update the table
  entry that currently reads "PostgreSQL 18+ (shared schema + RLS isolation;
  ADR-0003)" to the explicit pinned-major wording like "PostgreSQL 18.x (shared
  schema + RLS isolation; ADR-0003)" so the architecture doc and ADR align.
• Around line 208-212: Replace the stale "redis" entry in the Local
  Infrastructure list with the standardized service name "Valkey" (match casing
  used elsewhere in the docs), i.e., update the list item in the block containing
  "postgres / redis / seaweedfs / meilisearch" so it reads "postgres / Valkey /
  seaweedfs / meilisearch"; ensure any other nearby occurrences in that same Local
  Infrastructure section are also renamed to "Valkey" for consistency.

In @docs/architecture/09-tenant-isolation.md:

• Around line 198-201: The fenced code block showing the storage example (the
  block containing
  "tenants/{tenant_id}/organizations/{org_id}/courses/{course_id}/... ←
  org-scoped" and "tenants/{tenant_id}/brand/...
  ← tenant-wide") is missing a language tag which triggers MD040; add an
  appropriate language identifier (e.g., "text" or "yaml") after the opening
  triple backticks of that fenced block so the markdown linter recognizes the
  language and the lint error is resolved.

In @docs/architecture/29-dapr-integration.md:

• Line 83: Add a one-line clarifying note under the "statestore.yaml — Valkey
  state store" heading that explains the apparent Redis-shaped keys are Dapr
  component naming only (e.g., state.redis / redis*), and that although the keys
  look like Redis identifiers the runtime component is configured to use Valkey;
  reference "statestore.yaml", "Valkey", and the "state.redis"/"redis*" keys in
  the note so operators understand the naming vs actual provider.

In @docs/decisions/0002-initial-architecture.md:

• Around line 76-77: The link target for the text "[ADR-0023 draft]" incorrectly
  points at README.md; update the link so the anchor points to the actual ADR-0023
  markdown file (replace README.md with the ADR-0023 filename) while keeping the
  link text unchanged, ensuring the reference resolves to the ADR document rather
  than README.

In @docs/decisions/0014-adopt-dapr.md:

• Around line 15-19: Update the component file paths in the DAPR decision table
  to match the infra layer names: replace dapr/components/pubsub.yaml with
  infra/dapr/components/pubsub-kafka.yaml and replace
  dapr/components/statestore.yaml with
  infra/dapr/components/statestore-redis.yaml (leave the secret store
  secretstore-vault.yaml entry as-is if it already matches
  infra/dapr/components/secretstore-vault.yaml); locate the table in
  docs/decisions/0014-adopt-dapr.md and update the three Component file cell
  values accordingly.

In @docs/decisions/0031-postgresql-major-version.md:

• Around line 22-23: The markdown links in this document that reference
  "ADR-0023 (Strongly-typed ID source generator)" currently point to README.md;
  update both occurrences of that link (the one near the top and the one later in
  the file) to point to the actual ADR-0023 markdown file (replace README.md with
  the ADR-0023 file name/slug for the Strongly-typed ID ADR) so the link resolves
  directly to the ADR-0023 document.

In @infra/apisix/README.md:

• Around line 34-38: The fenced code block showing the plugin chain (the
  triple-backtick block containing "real-ip → cors → openid-connect → limit-req →
  request-id → proxy-rewrite → upstream") is missing a language identifier and
  triggers MD040; update that fenced block to include a language tag such as
  "text" (e.g., change totext) so markdownlint stops flagging it, leaving
  the block content unchanged.

In @infra/compose/dev.yml:

• Line 277: The CLUSTER_ID value currently set as learnstack-dev-cluster-id-1
  does not follow Kafka KRaft's required format; replace the hard-coded CLUSTER_ID
  in the dev.yml service environment with a KRaft-generated UUID (run
  kafka-storage.sh random-uuid on your Kafka binaries or CI runner) and paste that
  URL-safe Base64-encoded UUID into the CLUSTER_ID environment variable so the
  KRaft broker uses the official cluster identifier format.

In @infra/dapr/README.md:

• Line 23: Update the fenced code block shown in the diff by adding a language
  identifier to the opening triple backticks (e.g., change totext) so the
  markdownlint MD040 rule is satisfied; locate the code fence that contains the
  ASCII box art and add "text" (or another appropriate language) after the opening
  backticks.

In @README.md:

• Around line 23-25: Update the README.md sentence that says "land in subsequent
  Phase-01 packets" to reflect that Keycloak, LiveKit, Kafka, Vault, Dapr, APISIX
  (and other items listed) are now included in this PR; specifically edit the
  phrase containing "land in subsequent Phase-01 packets" and, if needed, adjust
  or remove the reference to docs/roadmap/phase-01-repository-tooling.md so the
  top-level README accurately states these components are present in the current
  Phase-01 packet rather than pending.
• Around line 33-34: The README's dependency list currently says "Valkey 7" but
  the compose pin in infra/compose/dev.yml uses valkey/valkey:8.1-alpine; update
  the README text to match the pinned version (e.g., change "Valkey 7" to "Valkey
  8.1" or "Valkey 8.1-alpine") so the documentation and the infra compose pin are
  consistent; ensure the same phrasing appears where the README references Valkey
  in the Cache / Pub-Sub / Secrets line.

---

Nitpick comments:
In @infra/compose/dev.yml:

• Line 293: Replace the non-reproducible image reference
  "ghcr.io/kafbat/kafka-ui:latest" for the kafka-ui service with a fixed, explicit
  tag (e.g. ghcr.io/kafbat/kafka-ui:) or an environment-variable-backed
  tag (e.g. ghcr.io/kafbat/kafka-ui:${KAFKA_UI_TAG}) to pin the image; update the
  image line in the kafka-ui service block (replace ":latest") and document/set
  the chosen or KAFKA_UI_TAG in your dev environment or .env so runs are
  reproducible.

In @infra/dapr/config/dapr-config.yaml:

• Around line 11-14: The dapr config currently sets samplingRate: "1" while
  zipkin.endpointAddress is empty, causing generated spans to fail exporting;
  change the samplingRate value to "0" when zipkin.endpointAddress is not
  configured (i.e., set samplingRate: "0") so tracing is disabled until a valid
  Zipkin endpoint is provided—update the samplingRate entry in the config
  (referencing the samplingRate and zipkin.endpointAddress keys) accordingly.

</details>
<details>
<summary>🪄 Autofix (Beta)</summary>
Fix all unresolved CodeRabbit comments on this PR:
- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes
</details>
---
<details>
<summary>ℹ️ Review info</summary>
<details>
<summary>⚙️ Run configuration</summary>
**Configuration used**: defaults
**Review profile**: CHILL
**Plan**: Pro
**Run ID**: `15b5e51a-3551-4fd2-9d46-b1f9ef741786`
</details>
<details>
<summary>📥 Commits</summary>
Reviewing files that changed from the base of the PR and between da08550bbe241589a66c93a1510dc3589b013715 and 3fd9054bea79f9e3f276092400f62d0dfee21fec.
</details>
<details>
<summary>📒 Files selected for processing (73)</summary>
* `.claude/skills/README.md`
* `.claude/skills/add-feature-key/SKILL.md`
* `.claude/skills/add-integration-test/SKILL.md`
* `.claude/skills/local-dev-setup/SKILL.md`
* `.claude/skills/run-tests-locally/SKILL.md`
* `.claude/skills/standards-check/SKILL.md`
* `.gitignore`
* `CLAUDE.md`
* `README.md`
* `backend/Directory.Packages.props`
* `backend/README.md`
* `backend/tests/LearnStack.Tests.Integration/LearnStack.Tests.Integration.csproj`
* `docs/architecture/02-domain-model.md`
* `docs/architecture/03-module-boundaries.md`
* `docs/architecture/04-technical-architecture.md`
* `docs/architecture/05-mvp-scope.md`
* `docs/architecture/06-extension-model.md`
* `docs/architecture/07-in-app-live-classroom.md`
* `docs/architecture/08-livekit-cost-model.md`
* `docs/architecture/09-tenant-isolation.md`
* `docs/architecture/16-media-pipeline.md`
* `docs/architecture/18-webrtc-build-vs-adopt.md`
* `docs/architecture/21-feature-flags.md`
* `docs/architecture/23-data-protection.md`
* `docs/architecture/24-learnstack-hub.md`
* `docs/architecture/25-deployment-models.md`
* `docs/architecture/29-dapr-integration.md`
* `docs/architecture/32-tenant-customization-model.md`
* `docs/decisions/0002-initial-architecture.md`
* `docs/decisions/0003-tenant-isolation-defense-in-depth.md`
* `docs/decisions/0010-cross-module-communication.md`
* `docs/decisions/0014-adopt-dapr.md`
* `docs/decisions/0017-tenant-organization-hierarchy.md`
* `docs/decisions/0018-tenant-driven-customization-model.md`
* `docs/decisions/0022-custom-domain-tls.md`
* `docs/decisions/0029-object-storage-seaweedfs.md`
* `docs/decisions/0030-redis-compatible-store-valkey.md`
* `docs/decisions/0031-postgresql-major-version.md`
* `docs/decisions/README.md`
* `docs/glossary.md`
* `docs/roadmap/phase-00-product-architecture.md`
* `docs/roadmap/phase-01-repository-tooling.md`
* `docs/roadmap/phase-02a-kernel-tenancy.md`
* `docs/roadmap/phase-04-cms-media-pages.md`
* `docs/roadmap/phase-08c-classroom.md`
* `docs/standards/00-principles.md`
* `docs/standards/05-database.md`
* `docs/standards/06-testing.md`
* `docs/standards/09-error-handling.md`
* `docs/standards/10-observability.md`
* `docs/standards/11-security.md`
* `docs/standards/12-infrastructure.md`
* `docs/standards/15-performance.md`
* `docs/standards/20-infrastructure-stack.md`
* `infra/apisix/README.md`
* `infra/apisix/apisix.yaml`
* `infra/apisix/config.yaml`
* `infra/compose/README.md`
* `infra/compose/dev.yml`
* `infra/compose/postgres-init/01-create-keycloak-db.sql`
* `infra/coturn/turnserver.conf`
* `infra/dapr/README.md`
* `infra/dapr/components/pubsub-kafka.yaml`
* `infra/dapr/components/secretstore-vault.yaml`
* `infra/dapr/components/statestore-redis.yaml`
* `infra/dapr/config/dapr-config.yaml`
* `infra/keycloak/README.md`
* `infra/keycloak/realms/learnstack-hub.json`
* `infra/keycloak/realms/learnstack.json`
* `infra/livekit/README.md`
* `infra/livekit/livekit.yaml`
* `infra/seaweedfs/README.md`
* `infra/seaweedfs/s3-identities.json`
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Keycloak 26.x, what is the security impact of setting fullScopeAllowed=true on a public OIDC client, and what is the recommended default for least-privilege setups?

💡 Result:

Setting fullScopeAllowed=true on a public OIDC client in Keycloak makes the client issue access tokens containing the user’s full set of roles (including roles beyond what’s explicitly mapped for that client), rather than restricting the token to the roles/scope mappings intended for least-privilege use.[1] This can effectively broaden the authorization surface: if the token contains additional roles, downstream resource servers that rely on token role claims may grant more access than intended. For least-privilege setups, the recommended default is to disable it: set fullScopeAllowed=false (i.e., keep token role delegation restricted to explicitly configured scopes/role mappings). This aligns with Keycloak community/security guidance pushing toward setting the flag to false by default for new clients to avoid over-privileged tokens.[2][3]

Citations:

• 1: https://linuxdatahub.com/keycloak-full-scope-allowed/
• 2: Change the default value of fullScopeAllowed to false keycloak/keycloak#37927
• 3: Proposal to Set Full Scope Allowed to False by Default for New Clients keycloak/keycloak#34334

---

🏁 Script executed:

# First, locate and verify the file exists
find . -name "learnstack-hub.json" -type f

Repository: cemililik/LearnStack

Length of output: 108

---

🏁 Script executed:

# Read the specific file to see the context around line 79if [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
sed -n '75,85p' infra/keycloak/realms/learnstack-hub.json
elseecho"File not found at expected path"fi

Repository: cemililik/LearnStack

Length of output: 364

---

🏁 Script executed:

# Get the full client configuration to understand scope mappingsif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then# Find the client name and its configuration structure
jq '.clients[] | select(.clientId == "hub-public" or .publicClient == true) | {clientId, publicClient, fullScopeAllowed, redirectUris, scope, protocolMappers}' infra/keycloak/realms/learnstack-hub.json 2>/dev/null | head -50
fi

Repository: cemililik/LearnStack

Length of output: 254

---

🏁 Script executed:

# Check if there are any scope or role mappings defined for clientsif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then# Look for scope and role mappings structure
jq '.scopeMappings // .clientScopeMappings' infra/keycloak/realms/learnstack-hub.json 2>/dev/null | head -100
fi

Repository: cemililik/LearnStack

Length of output: 69

---

🏁 Script executed:

# Get complete client configuration including scope detailsif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
jq '.clients[] | select(.clientId == "learnstack-hub-web")' infra/keycloak/realms/learnstack-hub.json 2>/dev/null
fi

Repository: cemililik/LearnStack

Length of output: 705

---

🏁 Script executed:

# Check for client scope bindings and defaultsif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
jq '.clientScopes // empty' infra/keycloak/realms/learnstack-hub.json 2>/dev/null | head -80
fi

Repository: cemililik/LearnStack

Length of output: 46

---

🏁 Script executed:

# Check if there are any role mappings or scope mappings defined at realm level for this clientif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
jq '.roles, .roleMappings' infra/keycloak/realms/learnstack-hub.json 2>/dev/null | head -50
fi

Repository: cemililik/LearnStack

Length of output: 709

---

🏁 Script executed:

# Check for any nested clientScopeMappings structureif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
jq '.clientScopeMappings // .scopeMappings[] | select(.client == "learnstack-hub-web") // empty' infra/keycloak/realms/learnstack-hub.json 2>/dev/null
fi

Repository: cemililik/LearnStack

Length of output: 46

---

🏁 Script executed:

# Check full structure to see if defaultClientScopes exists in client configif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
jq '.clients[] | select(.clientId == "learnstack-hub-web") | keys' infra/keycloak/realms/learnstack-hub.json 2>/dev/null
fi

Repository: cemililik/LearnStack

Length of output: 334

---

🏁 Script executed:

# Verify actual size and structure of the config file to make sure we're seeing the full client configif [ -f"infra/keycloak/realms/learnstack-hub.json" ];then
wc -l infra/keycloak/realms/learnstack-hub.json
# Also check if there are defaultClientScopes or similar in client arrays
jq '.clients[] | select(.clientId == "learnstack-hub-web") | {defaultClientScopes, optionalClientScopes, scopeMappings, clientScopeMappings}' infra/keycloak/realms/learnstack-hub.json 2>/dev/null
fi

Repository: cemililik/LearnStack

Length of output: 232

---

Restrict fullScopeAllowed and configure explicit scope mappings for the Hub public client.

fullScopeAllowed: true on the public client currently grants access to all user roles (hub-platform-admin, hub-operator, hub-billing-viewer) regardless of least-privilege principle. Changing this setting alone to false will break the operator portal unless explicit scope or role mappings are simultaneously configured to include the required roles in the token.

To properly harden this:

1. Set fullScopeAllowed to false
2. Configure defaultClientScopes or add scope mappings that explicitly include the roles the operator portal requires

Without step 2, the portal will lose access to role claims and authentication will fail.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/keycloak/realms/learnstack-hub.json` at line 79, Change the public
client object where "fullScopeAllowed" is set to true: set "fullScopeAllowed" to
false and add explicit client scopes or scopeMappings that grant only the
necessary roles to the client token. Specifically, update the client’s
"defaultClientScopes" or add "scopeMappings"/"roleMappings" entries that include
the operator portal roles (e.g., hub-operator and any other required roles such
as hub-platform-admin or hub-billing-viewer as appropriate) so tokens retain
needed role claims; ensure these mappings reference the realm roles used by your
operator portal.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For Keycloak 26.x confidential API clients, are directAccessGrantsEnabled=true, webOrigins=["+"], and fullScopeAllowed=true recommended? What secure defaults are advised for service-account-oriented backends?

💡 Result:

No—those values are generally not recommended for Keycloak 26.x confidential (service-account) API clients; secure defaults are to keep password-style direct access disabled, avoid broad CORS exposure, and avoid “full scope” so tokens remain least-privilege. 1) directAccessGrantsEnabled=true Not recommended. Keycloak’s “direct access grant” corresponds to the OAuth Resource Owner Password Credentials Grant (ROPCG), which is widely treated as insecure/deprecated (OAuth 2.1 removes it) and is recommended to be disabled by default for new clients. Keycloak maintainers explicitly propose setting directAccessGrantsEnabled/“Direct access grant” to false by default for security reasons. [1][2] 2) webOrigins=["+"] Not a security recommendation for service-account backends. Web Origins controls CORS behavior for browser-based requests; Keycloak docs describe it as a per-client CORS allowlist policy. [3] Using + is convenience for mapping from Valid Redirect URIs, but if you are building a backend using client credentials/service accounts, you typically do not need browser CORS at all—so safest practice is to restrict/omit Web Origins rather than rely on +. (Also, other Keycloak discussions clarify that + has limitations vs wildcard redirect URIs and that * may be required to truly permit all, indicating it can be easy to mis-assume the scope of “allow all”.) [4] 3) fullScopeAllowed=true Not recommended. For service-account-oriented backends, Keycloak’s own service-account documentation warns that the “Full Scope Allowed” switch is useful only for development, and recommends disabling it in production and properly configuring role scope mappings instead. [5] Keycloak maintainers also discuss the security need to change/disable the default because it can lead to over-privileged tokens, with proposals to set fullScopeAllowed to false by default. [6][7][8] Recommended secure defaults for confidential service-account backends (client credentials) - Keep “Client authentication” / confidential access type on (service account usage depends on enabling service account roles and client credentials). [5] - Disable “Direct access grant” (i.e., keep directAccessGrantsEnabled=false). [1][2] - Do not configure permissive Web Origins for token endpoints/backend APIs; leave it empty unless you have a browser flow that truly needs CORS. Web Origins is a client-level CORS allowlist. [3] - Set fullScopeAllowed=false and explicitly map/assign only the required roles via scope/role mappings; Keycloak notes this switch is dev-only and should be disabled in production. [5] If you tell me whether your backend is strictly token-exchange-only (client_credentials to resource server) or also serves browser traffic (Swagger/CORS), I can suggest a concrete minimal client configuration set for Keycloak 26.x.

Citations:

• 1: Admin-UI: disable Direct Access Grant by default when creating a new client keycloak/keycloak#30226
• 2: Set directAccessGrantsEnabled to false keycloak/keycloak#36871
• 3: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/clients/oidc/con-basic-settings.adoc
• 4: Web Origins don't follow Redirect URI wildcard keycloak/keycloak#44069
• 5: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/clients/oidc/proc-using-a-service-account.adoc
• 6: Change the default value of fullScopeAllowed to false keycloak/keycloak#37927
• 7: Proposal to Set Full Scope Allowed to False by Default for New Clients keycloak/keycloak#34334
• 8: https://www.keycloak.org/docs-api/26.2.5/javadocs/org/keycloak/services/clientpolicy/executor/FullScopeDisabledExecutor.html

---

🏁 Script executed:

#!/bin/bash# Check if file exists and verify the content at specified lines
file_path="infra/keycloak/realms/learnstack.json"if [ -f"$file_path" ];thenecho"=== File exists ==="echo"Total lines: $(wc -l <"$file_path")"echo""echo"=== Content around lines 68-75 and line 80 ==="
sed -n '60,85p'"$file_path"| cat -n | sed "s/^[[:space:]]*//"| awk '{print NR+59": "$0}'elseecho"File not found at $file_path"fi

Repository: cemililik/LearnStack

Length of output: 1279

---

Restrict over-permissive settings on learnstack-api client.

For a confidential backend client, enabling direct grants, wildcard web origins, and full scope allowance materially expands attack surface without clear need. Keycloak 26.x documentation and maintainers recommend disabling these for production-grade service-account backends: directAccessGrantsEnabled (deprecated in OAuth 2.1), webOrigins (unnecessary for backend-only clients), and fullScopeAllowed (dev-only per official service-account docs).

🔒 Suggested hardening baseline

- "directAccessGrantsEnabled": true,+ "directAccessGrantsEnabled": false,
@@
- "webOrigins": ["+"],+ "webOrigins": [],
@@
- "fullScopeAllowed": true+ "fullScopeAllowed": false

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/keycloak/realms/learnstack.json` around lines 68 - 75, The
learnstack-api client config is over-permissive: disable direct access grants by
setting "directAccessGrantsEnabled" to false (remove deprecated direct grant use
for a confidential service-account), replace the wildcard webOrigins entry
("webOrigins": ["+"]) with an empty array or explicit origins (e.g., []) so
backend-only clients have no permissive CORS origins, and ensure
"fullScopeAllowed" is explicitly set to false for the client (add
"fullScopeAllowed": false) to prevent automatic granting of all roles/scopes to
the service account; keep "serviceAccountsEnabled": true and only whitelist
necessary "redirectUris" if required.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

docs/standards/11-security.md (1)

57-72: ⚡ Quick win

Add language specifiers to fenced code blocks.

The code blocks at lines 57 and 76 should include a language identifier for better rendering and tooling support. For service lists like these, use ```text or ```yaml if the content will become YAML, or ```sh if they're meant to be command-like.

📝 Proposed fix

For line 57:

-```+```text
postgres # PostgreSQL 18.x per ADR-0031

For line 76:

-```+```text
livekit-egress # Phase 08c (recording / consent / cost model)

Based on learnings: static analysis tool flagged this as MD040 (fenced-code-language).

Also applies to: 74-79

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/standards/11-security.md` around lines 57 - 72, Add a language specifier
to the fenced code blocks that list services (e.g., the blocks containing the
lines starting with "postgres # PostgreSQL 18.x per ADR-0031" and
"livekit-egress # Phase 08c (recording / consent / cost model)") by
changing their opening fences to include a language like "text" (or "yaml" if
the block will be parsed as YAML, or "sh" if command-like); apply the same
change to the other nearby service list block(s) flagged (the block covering the
"livekit-egress" entry and the surrounding 74-79 range) so all fenced code
blocks include a language identifier to satisfy MD040 and improve
rendering/tooling.

infra/dapr/README.md (1)

23-37: 💤 Low value

Consider Mermaid diagram for better maintainability.

The ASCII art effectively shows the sidecar topology, but the coding guideline requires Mermaid for diagrams. A Mermaid flowchart or graph would be more maintainable and render consistently across viewers.

♻️ Example Mermaid conversion

-```text-┌──────────────────────────┐ ┌──────────────────────────────────────────┐-│ dotnet run │ │ daprd │-│ → host:5080 │ │ ./daprd -app-id learnstack-api \ │-│ │ │ -app-port 5080 \ │-│ │◄──┤ -app-channel-address \ │-│ │ │ host.docker.internal \ │-│ │ │ -dapr-http-port 3500 \ │-│ │ │ -dapr-grpc-port 50001 \ │-│ │ │ -placement-host-address \ │-│ │ │ dapr-placement:50005 \ │-│ │ │ -resources-path /components \ │-│ │ │ -config /config/dapr-config.yaml-└──────────────────────────┘ └──────────────────────────────────────────┘-```+```mermaid+graph LR+ A["dotnet run<br/>→ host:5080"]+ B["daprd<br/>-app-id learnstack-api<br/>-app-port 5080<br/>-app-channel-address host.docker.internal<br/>-dapr-http-port 3500<br/>-dapr-grpc-port 50001<br/>-placement-host-address dapr-placement:50005<br/>-resources-path /components<br/>-config /config/dapr-config.yaml"]+ B -->|subscribes| A+```

Based on learnings: Use Mermaid for diagrams in fenced code blocks (per coding guidelines for **/*.md).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/dapr/README.md` around lines 23 - 37, Replace the ASCII art diagram
block with a Mermaid diagram fenced code block that models the same sidecar
topology: create a left node for the application ("dotnet run → host:5080") and
a right node for the dapr sidecar including the CLI flags shown (e.g., -app-id
learnstack-api, -app-port 5080, -app-channel-address host.docker.internal,
-dapr-http-port 3500, -dapr-grpc-port 50001, -placement-host-address
dapr-placement:50005, -resources-path /components, -config
/config/dapr-config.yaml) and draw a directed link from the dapr node to the app
node; ensure the Mermaid block uses graph LR and preserves the same text/flags
from the original ASCII art so the diagram remains accurate and maintainable.

infra/coturn/turnserver.conf (1)

7-16: ⚡ Quick win

Coturn TURN server integration deferred to Phase 08c — external-ip and credential-passing not yet implemented.

The concern about unroutable relay addresses is technically valid for Coturn's general behavior, but the current dev setup is placeholder infrastructure. The livekit.yaml comment explicitly states that rtc.turn_servers (which passes Coturn credentials to clients) lands in Phase 08c; until then, Coturn is not integrated at all, so the lack of external-ip is not a blocker.

Additionally, a static external-ip in the config is insufficient for a multi-developer localhost setup — developers testing symmetric-NAT relay must switch --node-ip from 127.0.0.1 to their LAN IP anyway, which means external-ip also needs to be dynamic or environment-injected. The proper fix is part of the Phase 08c ILiveClassProvider wiring that includes per-session credential issuance and ICE server configuration, not a static value in turnserver.conf.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/coturn/turnserver.conf` around lines 7 - 16, Update the turnserver.conf
comment to explicitly state that external-ip and credential injection are
deferred to Phase 08c (so no static external-ip is set here), and reference the
planned work items: rtc.turn_servers in livekit.yaml and ILiveClassProvider for
per-session credential issuance; also mention developer-local testing requires
setting --node-ip to the machine LAN IP (so external-ip must be
dynamic/env-injected later). This makes it clear the lack of external-ip is
intentional and will be implemented as part of Phase 08c rather than in this
static config.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infra/dapr/README.md`:
- Around line 10-12: Update the README to remove ambiguity around phase
assignments by explicitly stating whether ICacheService and ISecretProvider
interfaces are defined in Phase 02a but their implementations (Valkey and Vault)
ship in Phase 02b; if that is the case add a short note near the building blocks
table and at the sentences around lines referencing "all three implementations
ship in Phase 02b" clarifying that interfaces (ICacheService, ISecretProvider)
are available as stubs/mocks or as interface-only contracts in 02a and full
implementations arrive in 02b, and if instead all three should read Phase 02b,
change the table entries for ICacheService and ISecretProvider to Phase 02b so
table and paragraph match.
---
Nitpick comments:
In `@docs/standards/11-security.md`:
- Around line 57-72: Add a language specifier to the fenced code blocks that
list services (e.g., the blocks containing the lines starting with "postgres # PostgreSQL 18.x per ADR-0031" and "livekit-egress # Phase 08c
(recording / consent / cost model)") by changing their opening fences to include
a language like "text" (or "yaml" if the block will be parsed as YAML, or "sh"
if command-like); apply the same change to the other nearby service list
block(s) flagged (the block covering the "livekit-egress" entry and the
surrounding 74-79 range) so all fenced code blocks include a language identifier
to satisfy MD040 and improve rendering/tooling.
In `@infra/coturn/turnserver.conf`:
- Around line 7-16: Update the turnserver.conf comment to explicitly state that
external-ip and credential injection are deferred to Phase 08c (so no static
external-ip is set here), and reference the planned work items: rtc.turn_servers
in livekit.yaml and ILiveClassProvider for per-session credential issuance; also
mention developer-local testing requires setting --node-ip to the machine LAN IP
(so external-ip must be dynamic/env-injected later). This makes it clear the
lack of external-ip is intentional and will be implemented as part of Phase 08c
rather than in this static config.
In `@infra/dapr/README.md`:
- Around line 23-37: Replace the ASCII art diagram block with a Mermaid diagram
fenced code block that models the same sidecar topology: create a left node for
the application ("dotnet run → host:5080") and a right node for the dapr sidecar
including the CLI flags shown (e.g., -app-id learnstack-api, -app-port 5080,
-app-channel-address host.docker.internal, -dapr-http-port 3500, -dapr-grpc-port
50001, -placement-host-address dapr-placement:50005, -resources-path
/components, -config /config/dapr-config.yaml) and draw a directed link from the
dapr node to the app node; ensure the Mermaid block uses graph LR and preserves
the same text/flags from the original ASCII art so the diagram remains accurate
and maintainable.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b1dfbc63-0a72-4eb6-b636-8c61eb18641e

📥 Commits

Reviewing files that changed from the base of the PR and between 3fd9054 and ec88235.

📒 Files selected for processing (32)

• .claude/skills/add-feature-key/SKILL.md
• .claude/skills/local-dev-setup/SKILL.md
• CLAUDE.md
• README.md
• backend/Directory.Packages.props
• docs/architecture/04-technical-architecture.md
• docs/architecture/08-livekit-cost-model.md
• docs/architecture/09-tenant-isolation.md
• docs/architecture/29-dapr-integration.md
• docs/decisions/0002-initial-architecture.md
• docs/decisions/0014-adopt-dapr.md
• docs/decisions/0029-object-storage-seaweedfs.md
• docs/decisions/0031-postgresql-major-version.md
• docs/decisions/README.md
• docs/roadmap/phase-01-repository-tooling.md
• docs/roadmap/phase-02a-kernel-tenancy.md
• docs/standards/05-database.md
• docs/standards/06-testing.md
• docs/standards/11-security.md
• docs/standards/12-infrastructure.md
• docs/standards/20-infrastructure-stack.md
• infra/apisix/README.md
• infra/compose/README.md
• infra/compose/dev.yml
• infra/coturn/turnserver.conf
• infra/dapr/README.md
• infra/dapr/components/secretstore-vault.yaml
• infra/keycloak/README.md
• infra/keycloak/realms/learnstack-hub.json
• infra/keycloak/realms/learnstack.json
• infra/livekit/livekit.yaml
• infra/seaweedfs/README.md

✅ Files skipped from review due to trivial changes (17)

• docs/roadmap/phase-02a-kernel-tenancy.md
• docs/standards/06-testing.md
• CLAUDE.md
• docs/architecture/09-tenant-isolation.md
• backend/Directory.Packages.props
• docs/decisions/README.md
• docs/decisions/0002-initial-architecture.md
• docs/architecture/08-livekit-cost-model.md
• infra/keycloak/README.md
• README.md
• docs/decisions/0029-object-storage-seaweedfs.md
• infra/apisix/README.md
• docs/decisions/0031-postgresql-major-version.md
• docs/roadmap/phase-01-repository-tooling.md
• infra/compose/README.md
• docs/architecture/29-dapr-integration.md
• docs/architecture/04-technical-architecture.md

🚧 Files skipped from review as they are similar to previous changes (6)

• docs/standards/05-database.md
• infra/keycloak/realms/learnstack-hub.json
• infra/keycloak/realms/learnstack.json
• docs/decisions/0014-adopt-dapr.md
• infra/dapr/components/secretstore-vault.yaml
• docs/standards/20-infrastructure-stack.md

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify phase assignments for cache and secret abstractions.

The building blocks table indicates ICacheService and ISecretProvider are available in Phase 02a, but lines 64 and 96-97 state all three implementations ship in Phase 02b. This creates ambiguity: are the interfaces defined in 02a but unimplemented until 02b, or should the table show 02b for all three?

If interfaces are genuinely split from implementations across phases, please add a note explaining the gap and what (if any) stub/mock is used in 02a.

Also applies to: 96-97

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/dapr/README.md` around lines 10 - 12, Update the README to remove
ambiguity around phase assignments by explicitly stating whether ICacheService
and ISecretProvider interfaces are defined in Phase 02a but their
implementations (Valkey and Vault) ship in Phase 02b; if that is the case add a
short note near the building blocks table and at the sentences around lines
referencing "all three implementations ship in Phase 02b" clarifying that
interfaces (ICacheService, ISecretProvider) are available as stubs/mocks or as
interface-only contracts in 02a and full implementations arrive in 02b, and if
instead all three should read Phase 02b, change the table entries for
ICacheService and ISecretProvider to Phase 02b so table and paragraph match.