Skip to content

Commit 1d3a0f7

Browse files
authored
Merge pull request #22 from HodeTech/development
feat(education): implement P02d-1 schema and isolation
2 parents 31a8cd8 + bc18170 commit 1d3a0f7

81 files changed

Lines changed: 8728 additions & 794 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ index plus the relevant [Architecture](../../docs/architecture/) doc.
5656
| [implement-task](implement-task/SKILL.md) | The default entry point for substantive work — scope, implement, self-check, run linter + tests, update docs, commit, produce a review-agent prompt. |
5757
| [start-task](start-task/SKILL.md) | Lightweight scoping-only entry point. Reading order + alignment check. Use when you don't need the full end-to-end workflow. |
5858
| [write-adr](write-adr/SKILL.md) | Capturing a one-time architectural decision. Uses Decision Drivers + Considered Options template. |
59-
| [commit-and-pr](commit-and-pr/SKILL.md) | Conventional Commit + AI trailer + PR body conventions. |
59+
| [commit-and-pr](commit-and-pr/SKILL.md) | Conventional Commit + applicable trailers + PR body conventions. |
6060
| [update-glossary](update-glossary/SKILL.md) | Introducing a new project-specific term anywhere in the corpus. |
6161

6262
### Review

‎.claude/skills/add-tenant-owned-entity/SKILL.md‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ authority.
8686
| Owning module | Yes | Determines DbContext, namespace, migration project. |
8787
| Org-scoped? | Yes | `false` = tenant-wide; `true` = needs `OrganizationId` + org RLS. |
8888
| Soft-deletable? | Yes | Decides the **query filter**, not the columns: `AuditableEntity<TId>` implements `ISoftDelete` unconditionally, so `deleted_at` / `deleted_by` are on every derived table either way ([Database Standards § Audit Columns](../../../docs/standards/05-database.md)). |
89-
| Strongly-typed id | Yes | Even simple entities use `<Name>Id : strongly-typed Guid` per [02-backend-coding.md](../../../docs/standards/02-backend-coding.md). |
89+
| Strongly-typed id | For roots and surrogate-key entities | Use `<Name>Id : strongly-typed Guid` per [02-backend-coding.md](../../../docs/standards/02-backend-coding.md). Contained translation satellites use their accepted natural composite key, with no surrogate id ([Database Standards § Translation satellite tables](../../../docs/standards/05-database.md#translation-satellite-tables)). |
9090

9191
## Workflow
9292

@@ -139,12 +139,15 @@ Rules:
139139
- `OrganizationId` is nullable on every `[OrganizationScoped]` entity — null means
140140
tenant-wide — and `Every_OrgScoped_Entity_HasOrgIdAndFilter` fails a mapped
141141
`organization_id` that is not nullable (Step 4).
142-
> **Open in Phase 02d.** Whether a child or satellite row carries its parent's
143-
> organization, and what forces it to at insert, is G7 in
144-
> [Phase 02d's decision register](../../../docs/roadmap/phase-02d-walking-skeleton.md#the-decision-register).
145-
> The pass that closes it edits this rule with its answer.
142+
A child whose scope mirrors its parent derives both tenant and organization in its
143+
factory. The independent INSERT/UPDATE parent-scope trigger, immutable organization
144+
guard and composite tenant foreign key enforce that relation in the database; use
145+
[Database Standards § Parent organization mirrors](../../../docs/standards/05-database.md#parent-organization-mirrors).
146146
- Use `AuditableEntity<<Name>Id>` for mutable aggregates. **Never** `Entity<TId>`
147147
unless the aggregate is append-only (e.g. `AuditEntry`).
148+
A contained translation satellite uses its natural key as a plain entity, with no
149+
independent audit timestamps, row version or soft-delete lifecycle; changes belong
150+
to its owning root ([Translation satellite tables](../../../docs/standards/05-database.md#translation-satellite-tables)).
148151
- Domain events for state changes; don't write to other aggregates from this one.
149152
- Mark PII properties `[PiiSensitive]`. From Packet 9 the audit capture replaces a
150153
marked — or name-token-matched — value with `SensitiveTokenCatalog.RedactedValue` in
@@ -264,7 +267,8 @@ dotnet ef migrations add add_<name> \
264267

265268
`--output-dir` is not optional, and this is the skill that needs it most: EF
266269
defaults the output to `Migrations/` when the project has no sibling migration to
267-
reuse, which is six of the seven module assemblies today. `make migrate`,
270+
reuse. Content, Identity and Media are the three module assemblies without a
271+
migration chain after P02d-1 Step 2. `make migrate`,
268272
`backend/.editorconfig` and `Migrate_Target_Covers_Every_Migration_Chain` all key
269273
on `Persistence/Migrations` — a chain landing one directory up is skipped by the
270274
Makefile loop in silence and is invisible to the architecture test written for
@@ -366,24 +370,28 @@ what you pasted — a reviewer will:
366370
nothing may *write* outside its own. `WITH CHECK` is not sufficient on its own for
367371
that guarantee — PostgreSQL has no `WITH CHECK` for `DELETE`, and `USING` is also
368372
what selects the rows an `UPDATE` may target — which is why the two `AS RESTRICTIVE`
369-
guards above are part of the template and not an optional extra. One write path stays
370-
open — the organization arm of `WITH CHECK` admits `organization_id IS NULL` from any
371-
session, so an organization-scoped session can `INSERT` a tenant-wide row; its status
372-
is in
373+
guards above are part of the template and not an optional extra. The tenant-wide
374+
arm of `WITH CHECK` also requires an unset or empty `app.organization_id`, so an
375+
organization-scoped session cannot `INSERT` a tenant-wide row. Copy the accepted
376+
[ADR-0003 Amendment 6](../../../docs/decisions/0003-tenant-isolation-defense-in-depth.md#amendment-6--insert-scope-and-parent-mirrors-2026-09-14)
377+
predicate from
373378
[05-database.md § Tenant-Owned and Organization-Scoped Tables](../../../docs/standards/05-database.md#tenant-owned-and-organization-scoped-tables).
374379
- **The composite `UNIQUE (tenant_id, id)` and the composite foreign keys** — referential
375380
integrity is checked on behalf of the table owner and bypasses RLS entirely, so a
376381
single-column FK is a cross-tenant reference waiting to happen, invisible to every
377382
policy. See
378383
[05-database.md § Foreign keys between tenant-owned tables](../../../docs/standards/05-database.md).
379384

380-
Two obligations the policy block does not carry, both in
385+
Three obligations the policy block does not carry, all in
381386
[05-database.md](../../../docs/standards/05-database.md): the `BEFORE UPDATE`
382387
organization immutability trigger on every org-scoped table (the rule after the
383-
template), and an index supporting every foreign key
384-
([§ Indexes](../../../docs/standards/05-database.md#indexes)). Neither makes a child's
385-
or satellite's mirrored `organization_id` equal its parent's at insert — see
386-
[§ Translation satellite tables](../../../docs/standards/05-database.md#translation-satellite-tables).
388+
template), an index supporting every foreign key
389+
([§ Indexes](../../../docs/standards/05-database.md#indexes)), and the independent
390+
INSERT/UPDATE [parent-scope trigger](../../../docs/standards/05-database.md#parent-organization-mirrors)
391+
for a child whose scope mirrors its parent. Derive that child's tenant and organization
392+
from the parent in its factory; retain the composite tenant foreign key as a separate
393+
database control. The shared immutability function assumes no surrogate `id`, because
394+
contained translation satellites have none.
387395

388396
Always call `current_setting` with the second argument `true`. Without it an unset
389397
context raises inside a pooled connection instead of simply filtering the row out.
@@ -420,14 +428,20 @@ Two gaps remain, and both are yours to close by hand:
420428
- **A marker-gated rule cannot catch a missing marker.** It iterates what it
421429
finds. An entity you forget to mark is invisible to both rules, and the
422430
isolation test in Step 5 is the net for it.
423-
- **The sweep covers every module in `Modules.Scoped`** — Tenancy, Customization and
424-
Audit today. A module with a schema that is missing from that list fails
431+
- **The sweep covers every module in `Modules.Scoped`** — Tenancy, Customization,
432+
Audit and Education after P02d-1 Step 2. A module with a schema that is missing
433+
from that list fails
425434
`Every_Module_With_A_Schema_Is_Swept`, so add yours there.
426435

427436
Also live against your migration: `Every_Foreign_Key_Has_A_Supporting_Index` and
428437
the schema sweeps in `TenancySchemaTests` — row security enabled *and* forced,
429438
no second permissive policy for one command, snake_case identifiers, and the exact
430-
grant matrix. Those run against the applied schema.
439+
grant matrix. Those run against the applied schema. P02d-1 Step 2's
440+
[EducationStructureTests](../../../backend/tests/LearnStack.Tests.Integration/Database/EducationStructureTests.cs)
441+
also exercises the parent-mirror and Pattern A detectors with planted violations;
442+
[EducationPersistenceTests](../../../backend/tests/LearnStack.Tests.Integration/Database/EducationPersistenceTests.cs)
443+
proves natural-key satellite persistence, owning-root concurrency and contained audit
444+
capture through the actual composition roots.
431445

432446
If you're adding a *new* marker attribute or a *new* isolation pattern, write a
433447
new architecture test (see
@@ -471,8 +485,8 @@ public sealed class <Name>IsolationTests(SchemaFixture schema)
471485

472486
There is no `TestFixture`, no `CreateTenantAsync` and no `AsTenant(...)` helper —
473487
an earlier version of this file used all three. The shipped fixtures are
474-
`PostgresFixture` (container + the four roles) and `SchemaFixture` (both migration
475-
chains, every table seeded for two tenants), shared with
488+
`PostgresFixture` (container + the four roles) and `SchemaFixture` (all five migration
489+
chains, tenant-owned tables seeded for two tenants), shared with
476490
`[Collection(SharedSchema.Name)]`. The fixture must seed **both** tenants: a count
477491
of zero against a table nothing populated passes whatever the policy says.
478492

‎.claude/skills/commit-and-pr/SKILL.md‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
name: commit-and-pr
33
description: >
44
Format LearnStack commits and pull requests to the project's conventions —
5-
Conventional Commits + AI co-author trailer + scope-correct PR body. USE FOR:
5+
Conventional Commits + scope-correct PR body. USE FOR:
66
preparing a commit, opening a PR, picking the right scope for a doc-only change,
7-
adding the AI co-author trailer correctly. DO NOT USE FOR: deciding whether a
7+
adding applicable trailers correctly. DO NOT USE FOR: deciding whether a
88
change is ready to commit (that's a code-review concern, not a commit-format
99
concern), force-pushing, or changing what an Accepted ADR decides (write a new
1010
ADR that supersedes it — amendments and the bounded corrections in
@@ -25,7 +25,7 @@ move fast and `git log --grep` stays useful.
2525

2626
- Producing one or more commits at the end of a unit of work.
2727
- Opening a pull request against `main`.
28-
- Adding trailers (ADR refs, module list, AI co-author).
28+
- Adding trailers (ADR refs, module list, i18n keys).
2929

3030
## When not to use
3131

@@ -81,20 +81,8 @@ Trailers go at the **end** of the body (after a blank line). The supported set:
8181
| `ADR: NNNN[, NNNN]` | The commit implements or derives from one or more ADRs. **Bare numbers** — `ADR: 0040, 0003` — never `ADR: ADR-0040`: `git log --grep='ADR: 0017'` is what the trailer exists for, and the prefixed form does not match it. A `feat` commit that creates a schema an ADR decides carries this too; the trailer is about the *derivation*, not the commit type. |
8282
| `Module: <list>` | Multi-module change; list every module touched. |
8383
| `I18n: <keys>` | Added / renamed / removed user-facing i18n keys. |
84-
| `Co-Authored-By: …` | Required for AI-assisted commits. |
8584

86-
### Step 4: AI co-author trailer
87-
88-
Pick the trailer that matches the agent that did material work:
89-
90-
- Claude Code session:
91-
`Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>`
92-
- OpenAI Codex session:
93-
`Co-Authored-By: Codex Opus 4.7 (1M context) <noreply@anthropic.com>`
94-
95-
Multiple assistants: include one trailer per assistant.
96-
97-
### Step 5: HEREDOC the message
85+
### Step 4: HEREDOC the message
9886

9987
Always pass the message via a HEREDOC so newlines and quotes survive:
10088

@@ -107,35 +95,47 @@ access shares the per-learner entitlement read path with free access.
10795
10896
ADR: 0010
10997
Module: Enrollment, Billing
110-
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11198
EOF
11299
)"
113100
```
114101

115-
### Step 6: Pull request
102+
### Step 5: Pull request
116103

117104
The PR body uses the structure in
118105
[14-git-workflow.md § Pull Requests](../../../docs/standards/14-git-workflow.md).
119-
Minimum sections:
106+
Use that standard's canonical sections; `Approach` is needed only when non-obvious
107+
and `Migration / Rollback` for schema or configuration changes. The template below
108+
also includes this skill's existing supplements: the `Risk` section, used as described
109+
below, and the assistant attribution footer. Those supplements are not canonical
110+
sections defined by Standard 14.
120111

121112
```markdown
122113
## Summary
123114
- <1-3 bullet points: what changed and why>
124115

125-
## Test plan
126-
- [ ] <Concrete checks the reviewer can run>
116+
## Approach
117+
<Only when non-obvious>
118+
119+
## Tests
120+
<Completed checks and concrete checks the reviewer can run>
127121

128122
## Risk
129123
- <Anything that could ripple: schema, contract, perf>
130124

131-
🤖 Generated with [Claude Code](https://claude.com/claude-code)
125+
## Migration / Rollback
126+
<For schema or configuration changes>
127+
128+
## Related
129+
<Links to the relevant issues, ADRs and standards>
130+
131+
Prepared with <assistant used>.
132132
```
133133

134134
If the change touches the Hub HTTPS surface, the entitlement projection, or any
135135
tenant-isolation boundary, the PR description **must** call it out under "Risk" and
136136
reference the relevant ADR.
137137

138-
### Step 7: Open the PR
138+
### Step 6: Open the PR
139139

140140
```bash
141141
gh pr create --title "<imperative subject>" --body "$(cat <<'EOF'
@@ -151,9 +151,9 @@ The PR title follows the **same** rules as the commit subject (imperative mood,
151151

152152
- `git log --pretty=format:'%s' main..HEAD` shows commits in
153153
`type(scope): subject` form.
154-
- Every AI-assisted commit carries the `Co-Authored-By` trailer.
155154
- `git log --grep='ADR: 0017'` finds commits that implemented ADR-0017.
156-
- PR title matches the convention; PR body has Summary + Test plan + Risk.
155+
- PR title matches the convention; PR body follows § Description's canonical
156+
sections and includes this skill's applicable Risk disclosure.
157157
- No commit on the branch force-pushes after review starts.
158158

159159
## Common pitfalls

‎.claude/skills/implement-task/SKILL.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,7 @@ Run [commit-and-pr](../commit-and-pr/SKILL.md). Concretely:
223223
- Subject imperative, ≤ 72 chars.
224224
- Body: one short paragraph saying *why* (not *what* — the diff is what).
225225
- Trailers in this order: `ADR:` (if applicable), `Module:` (if multi-module),
226-
`I18n:` (if applicable), `Co-Authored-By:` (the AI assistant doing the
227-
work — Claude / Codex / both).
226+
`I18n:` (if applicable).
228227
- HEREDOC every multi-line message.
229228

230229
Do **not** push or open a PR unless the user has asked. Default = local

‎.claude/skills/run-tests-locally/SKILL.md‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,22 @@ pnpm test -t "shows danger tone" # name filter
194194
### Step 8: Coverage (optional)
195195

196196
```bash
197-
dotnet test --collect:"XPlat Code Coverage"
198-
reportgenerator -reports:"**/coverage.cobertura.xml" \
199-
-targetdir:"coverage-html" \
197+
# From the repository root; collect behavioral suites separately.
198+
dotnet test backend/tests/LearnStack.Tests.Unit \
199+
--collect:"XPlat Code Coverage" --results-directory coverage/unit
200+
dotnet test backend/tests/LearnStack.Tests.Integration \
201+
--collect:"XPlat Code Coverage" --results-directory coverage/integration
202+
dotnet test backend/tests/LearnStack.Tests.Architecture
203+
reportgenerator -reports:"coverage/**/coverage.cobertura.xml" \
204+
-targetdir:"coverage/html" \
200205
-reporttypes:Html
201206
```
202207

208+
Keep the architecture suite uninstrumented. In P02d-1, a solution-wide Coverlet
209+
run injected a tracker type that NetArchTest's IL/reflection sweep could not resolve,
210+
raising `TypeLoadException`; unit and integration coverage plus a normal architecture
211+
run passed. Do not skip or weaken the structural test to make coverage succeed.
212+
203213
Targets per
204214
[06-testing.md § Coverage Targets](../../../docs/standards/06-testing.md): Domain
205215
≥ 90% line and ≥ 80% branch, Application ≥ 80% line, Infrastructure adapters

‎.claude/skills/standards-check/SKILL.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ domain the diff doesn't touch.
309309

310310
#### `14-git-workflow.md`
311311
- [ ] `type(scope): subject` commit format; subject imperative ≤ 72 chars.
312-
- [ ] AI-assisted commit carries `Co-Authored-By` trailer (Claude or Codex).
313312
- [ ] `ADR:` / `Module:` / `I18n:` trailers where applicable.
314313
- [ ] No `--force` on `main`; no `--amend` on a published commit.
315314

‎.github/CONTRIBUTING.md‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ Configure these in **GitHub → Settings → Branches → Branch protection rule
2525
> force pushes, no direct pushes — is live today. Only the two named settings
2626
> are deferred.
2727
>
28-
> **Two required-check edits are outstanding**, both flagged in the list below:
29-
> the `meta` check is required under a name nothing reports any more, and
30-
> `backend integration (Testcontainers)` runs on every pull request and is not
31-
> required at all. The first blocks every merge; the second gates nothing.
28+
> **Required checks repaired — 2026-09-14 (P02d-1).** The live rule now requires
29+
> the current `meta` name and `backend integration (Testcontainers)`. A fresh API
30+
> read verified all five check names and their GitHub Actions attribution, with
31+
> strict up-to-date checking and every other protection setting preserved.
32+
> [P02d-1's delivery record](../docs/roadmap/phase-02d-walking-skeleton.md#step-3--packet-completion-2026-09-14)
33+
> carries the dated verification.
3234
3335
- **Require a pull request before merging**
3436
- Require approvals: **1** (raise to 2 once the team grows past two
@@ -41,21 +43,13 @@ Configure these in **GitHub → Settings → Branches → Branch protection rule
4143
- `backend (build + unit + arch + contract)`
4244
- `frontend (typecheck + lint + build + test)`
4345
- `secret scan (leakwatch)`
44-
- `meta (compose + commit hygiene + link audit)` — ⚠️ **the live rule still
45-
requires the pre-rename name** `meta (commit hygiene + link audit)`, which
46-
nothing reports. GitHub matches by name, so that required check never
47-
arrives and **every** pull request sits at "Expected — waiting for status to
48-
be reported". Re-require it under the current name; the job itself is green.
49-
This is the failure the warning below describes, in the direction that
50-
blocks rather than the one that waves through.
46+
- `meta (compose + commit hygiene + link audit)`
5147
- `backend integration (Testcontainers)` — **activated in Phase 02a Packet 6**
5248
with the four-role provisioning suite, one packet earlier than planned:
5349
Packet 6 ships the first Docker-bound test and is therefore the packet that
5450
has to split them. The job carries no `vars.ENABLE_*` gate and no placeholder
55-
step, so it already runs on every pull request. **Adding it to the live
56-
branch-protection rule is the one remaining edit**, and it is a repository
57-
setting rather than a file in this repo — until it is made, the job runs and
58-
gates nothing. Once it is made the check cannot pass empty: both backend jobs
51+
step, so it runs on every pull request. **Required since P02d-1,
52+
2026-09-14.** The check cannot pass empty: both backend jobs
5953
end with `scripts/assert-tests-ran.py`, which fails a run whose filter matched
6054
no test — `dotnet test --filter` exits 0 when nothing matches, and a renamed
6155
`Requires=Docker` trait would otherwise leave the whole Docker suite unrun
@@ -101,21 +95,15 @@ Per CLAUDE.md § Commit conventions:
10195

10296
- **Conventional Commits**: `type(scope): subject` with subject in
10397
imperative mood, ≤ 72 chars.
104-
- AI-assisted commits carry the trailer
105-
`Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>`
106-
(replace the model name when authoring with a different assistant).
10798
- `docs(scope)` for doc-only commits; scope ∈ `architecture | decisions |
10899
standards | roadmap` or omitted for cross-cutting changes.
109100

110101
## Pull requests
111102

112103
- Title mirrors the primary commit's subject.
113-
- Description has three sections:
114-
1. **What** — bullet list of changes grouped by area.
115-
2. **Why** — one paragraph; link to the ADR / phase / issue.
116-
3. **Verification** — what suites you ran locally, what manual checks
117-
you walked.
118-
- Link the related ADR / phase doc with relative paths (`../docs/...`).
104+
- Follow [Git Workflow § Description](../docs/standards/14-git-workflow.md#description)
105+
for the canonical structure, including migration/rollback details for schema or
106+
configuration changes and links to the related ADR / phase documents.
119107

120108
## Local checks before pushing
121109

0 commit comments

Comments
 (0)