docs(rewrite): add 04-authorization-model — FGA vs Postgres split

[mlehotskylf]

Summary

Adds docs/rewrite/04-authorization-model.md — a follow-up to the Architecture-call feedback that Mentorship (project-subordinated, unlike Crowdfunding's free-floating initiatives) should sit behind the v2 API Gateway with Heimdall edge authorization and OpenFGA, rather than following Crowdfunding's interim standalone-API model.

Proposes:

• Principle: PostgreSQL is the system of record for all data (including memberships/ownership); OpenFGA holds a derived authorization index of only the relations Heimdall checks at the edge, synced via lfx-v2-fga-sync. Delivery is a transactional outbox: the state change records a dirty-object marker in the same transaction, and the relay re-derives each payload from current Postgres state at send time — never replaying a stored one, since the GenericFGAMessage envelope carries no object version, so a stale full-state payload retried after a newer revocation would restore exactly the tuple that was revoked. Plus a periodic reconciliation job that re-derives expected relations from Postgres
• Relationship graph (Mermaid) marking each edge as FGA-mirrored vs Postgres-only
• FGA type sketch: mentorship_program (parent project, writer, mentor, manager, auditor, viewer), mentorship_application (parent, applicant, manager, writer, reviewer), mentorship_task (parent, assignee, manager) — written in the platform's model.fga conventions, with vote_response/survey_response as the closest analogs
• Modeling decisions: no mentee→program relation ("accepted" is application state, not an access relation); tasks parent to the program in both phases (prerequisite + non_prerequisite); workflow gates (prereqs before consideration, remaining tasks before graduation) stay business logic in Postgres; and no attribute-level access control — Heimdall authorizes a route, not a field, so per-field permissions become separate sub-resource endpoints (evaluation checks reviewer, status checks manager, withdrawal checks writer)
• Lifecycle → FGA emission table, covering publish/unpublish (the wildcard viewer tuple is per-object), mentor-application acceptance (member_put — mentors apply as well as being invited, so acceptance is a real grant rather than a no-op), program and child deletion (delete_access, else tuples orphan in FGA), role-named member_remove (an unnamed removal deletes every direct relation the user holds), and the backfill seed, which must emit only currently effective membership rows so history cannot restore revoked access
• Identity: /me scoping uses the Heimdall principal claim (populated from the subject's username); the upstream Auth0 sub is not forwarded to services
• Open questions AQ-1–AQ-7 for the Architecture team: /me list-endpoint scoping, per-object tuples, the Postgres-vs-NATS-KV deviation, the two new relations on the shared project type (including who owns/emits them, since project-service emits full-state project updates), dropping HMAC email-approval links, program-creation policy vs super-admin approval, and how the invite-accept route is authorized when no tuple exists yet

Also updates 03-migration-plan.md: removes the "split applications/enrollments" direction — the legacy application is the lifecycle object, one row carrying pending → accepted → active → graduated, so there is no enrollment entity to create. The ETL contract now requires a target mapping for all three project-members member types (so pending/declined mentor applications are not silently dropped) and preserves the legacy hold status as a distinct state.

Also revises 02-target-architecture.md (this was originally deferred, but the file's ERD and authorization section directly contradicted the model once it landed, so leaving it stale would have made the series self-inconsistent): the authorization section moves from service-layer checks and the super-admin allowlist to the gateway/Heimdall/FGA design, enrollments / enrollment_mentors come out of the ERD, and programs gains a project_uid column — inherited project permissions need a persisted LF project UID for the outbox to derive the mentorship_program#project tuple.

Note: the target task category vocabulary (prerequisite | non_prerequisite) and the application lifecycle (pending → accepted → active → graduated) in these docs follow the constraints already merged in backend/db/migrations/001_initial.up.sql.

Review notes

• Grounded in lfx-v2-helm/docs/service-chart-patterns.md, lfx-v2-fga-sync/docs/fga-sync-contract.md, and lfx-v2-member-service as the reference implementation
• Emission semantics verified against the fga-sync implementation, not just its docs: delete_access exists, member_remove with an empty relations array deletes every direct relation on the object, and the message envelope has no object version/revision field to rely on for supersession
• Task semantics verified against the legacy code (jobspring task.Category: prerequisite vs nonPrerequisite, single owner + project/term)
• Authorization behavior verified in legacy: applications are created by the applicant; tasks are created by admins/mentors and owned by the mentee; mentors can review and change task status but cannot change application status; mentors reach a program by applying or by invitation (ProjectMemberStatus.Pending covers both)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Proposes the Mentorship rewrite’s gateway/OpenFGA authorization model, with PostgreSQL remaining authoritative.

Changes:

• Defines FGA relationships and permissions.
• Documents lifecycle tuple synchronization and backfill.
• Records architecture questions requiring resolution.

Suppressed comments (2)

docs/rewrite/04-authorization-model.md:74

• parent.mentor authorizes every program mentor to view/review every task. This contradicts the approved model, where enrollment_mentors routes task-submission review to the assigned mentor(s), not “any mentor on the program” (02-target-architecture.md:151), and broadens access to submissions/files. Model an assignment-specific reviewer relation (or inheritance through the enrollment), while retaining program writers as the admin override; also define one viewer union plus distinct submit/review permissions.

  view/review    = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:97

• The fga-sync contract reserves delete_access for genuine resource deletion, but withdrawal is only a status change and retains the application row. Purging all tuples on withdrawal removes both applicant and parent access, so the retained application cannot be opened through the flat edge-checked route. Split withdrawal from actual deletion and retain/reconcile the applicant and parent tuples according to the intended historical-access policy.

| Withdrawal / member removal | status change / row removal | `member_remove` / `delete_access` |

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

docs/rewrite/04-authorization-model.md:15

• Publishing only at the state transition leaves no recovery boundary between PostgreSQL and the authorization index. If the database commit succeeds but the NATS publish is not confirmed, a grant can remain missing—or a revoked grant can remain live—indefinitely; fga-sync's durability starts only after broker receipt, and exhausted deliveries require republishing current state. Specify a transactional outbox plus reconciliation/reindex path, including revocation delivery and convergence semantics, before treating PostgreSQL as authoritative.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:17

• The shared Heimdall create_jwt finalizer emits the LFID username in principal, not sub; sub is not included in the service JWT. Filtering /me queries with sub would therefore use a missing or incorrect identifier instead of the LFID stored in PostgreSQL.

- Backend services make **no authorization decisions**. The single residue is `/me/*` **list** endpoints ("my applications", "my tasks"), where the service filters rows by the JWT `sub` from Heimdall — data scoping on the caller's own records, not a grant/deny decision on an identified resource. Every route that carries a resource ID gets a Heimdall rule instead.

docs/rewrite/04-authorization-model.md:60

• The nested applications list must admit both mentors and writers, but this type has only separate base relations. Heimdall configures one OpenFGA relation per rule, so checking either mentor or writer would exclude the other intended audience. Add a derived union permission and use it for GET /programs/{uid}/applications.

  relations: parent (project), writer (user), mentor (user)

docs/rewrite/04-authorization-model.md:82

• This contradicts both the lifecycle above (pending is the post-submission application status) and the emission table below, which creates tuples when an application is submitted. As written, reviewers cannot tell whether pending applications are edge-authorized. Distinguish pending invitations from submitted applications explicitly.

4. **Pending invitations/applications have no FGA presence.** A pending mentor invitation or mentee application is a Postgres row. Tuples appear when the relationship becomes effective (invite accepted → `mentor`; application submitted → `applicant` + `parent` so mentors can evaluate it).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

docs/rewrite/04-authorization-model.md:15

• The sync design has no durable recovery path after the initial seed. fga-sync retries are bounded and stream messages expire, so a publish lost between the PostgreSQL commit and NATS—or an exhausted member_remove—can leave FGA permanently divergent and stale access active. Specify a PostgreSQL-backed outbox/confirmed-delivery strategy plus a repeatable reconciliation or reindex path, particularly for revocations.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:17

• The v2 gateway's forwarded JWT identifies the LFID in the custom principal claim, not sub (the platform finalizer and member-service JWT parser both use principal). Implementing /me filtering as written would key queries with the wrong identifier and fail to return the caller's records.

- Backend services make **no authorization decisions**. The single residue is `/me/*` **list** endpoints ("my applications", "my tasks"), where the service filters rows by the JWT `sub` from Heimdall — data scoping on the caller's own records, not a grant/deny decision on an identified resource. Every route that carries a resource ID gets a Heimdall rule instead.

docs/rewrite/04-authorization-model.md:67

• The sketch defines view twice through the view/withdraw and view/evaluate shorthand. Heimdall must check one concrete relation for the GET route, and an OpenFGA relation cannot be declared twice, so the model needs an explicit viewer union plus separate action relations.

type mentorship_application
  relations: parent (mentorship_program), applicant (user)
  view/withdraw  = applicant
  view/evaluate  = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:72

• As with applications, this gives view two different definitions instead of one relation that Heimdall can check. Define a viewer union and distinct submit/review relations so both assignees and reviewers can access the same task endpoint.

type mentorship_task
  relations: parent (mentorship_program), assignee (user)
  view/submit    = assignee
  view/review    = parent.mentor or parent.writer

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

docs/rewrite/04-authorization-model.md:15

• A state-transition-only publish can leave the sole edge-authorization index permanently stale when the PostgreSQL commit succeeds but the NATS publish or flush fails; for revocations, that preserves access after removal. The fga-sync contract is asynchronous and requires publishers to republish authoritative current state after delivery failures, so this design needs a durable handoff and reconciliation path.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:17

• sub is the Auth0 subject, but this architecture stores users by LFID and the v2 gateway derives the FGA principal from the username attribute. Filtering /me by sub would therefore use a different identifier from both PostgreSQL and the user:{lfid} tuples described below; use the Heimdall-normalized LFID instead.

- Backend services make **no authorization decisions**. The single residue is `/me/*` **list** endpoints ("my applications", "my tasks"), where the service filters rows by the JWT `sub` from Heimdall — data scoping on the caller's own records, not a grant/deny decision on an identified resource. Every route that carries a resource ID gets a Heimdall rule instead.

docs/rewrite/04-authorization-model.md:82

• This leaves two edge-authorization gaps: a submitted application is still pending but line 91 says it receives tuples on submission, while a pending mentor invitation has no tuple with which Heimdall can verify the invitee before accepting it. Distinguish unsubmitted drafts from submitted-pending applications, and model invitation-recipient access (the v2 reference pattern protects invite routes with an invitation viewer relation) or document a backend authorization exception.

4. **Pending invitations/applications have no FGA presence.** A pending mentor invitation or mentee application is a Postgres row. Tuples appear when the relationship becomes effective (invite accepted → `mentor`; application submitted → `applicant` + `parent` so mentors can evaluate it).

docs/rewrite/04-authorization-model.md:103

• This repeats the sub identity mismatch from the principle section. The proposed default must scope by the LFID username normalized by Heimdall so it matches the PostgreSQL user key and FGA principal.

| AQ-1 | Is the `/me/*` list-endpoint residue (service filters by JWT `sub`; no resource ID in path) acceptable, or should "my stuff" go through the query/indexer service with FGA access checks? | `/me` + sub-filtering for v1 |

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

docs/rewrite/04-authorization-model.md:56

• This decision also invalidates 03-migration-plan.md:26, which still instructs the ETL to split applications and enrollments. Calling out only the ERD correction leaves the migration plan directing implementers to create an entity this proposal rejects; include that backfill step among the required follow-up corrections.

**No enrollment entity.** In the legacy system the application *is* the lifecycle object: one `project-members` row (memberType `apprentice`, keyed by user + program term) whose status runs the full journey `pending → accepted → graduated`. Acceptance and graduation are status changes on that row, and mentors relate to the **program**, not to individual mentees (a mentee's "mentors" list is a cron-denormalized copy of the program's approved mentors). The rewrite keeps that shape — no `enrollments` table, no mentor-mentee assignment — and the ERD in [02](./02-target-architecture.md) will be corrected accordingly.

docs/rewrite/04-authorization-model.md:17

• Use Heimdall's principal claim rather than sub for row scoping. The v2 platform explicitly avoids downstream use of the upstream sub, and the member-service reference implementation extracts and validates the custom principal claim; using sub here would make /me filtering inconsistent with the LFID used in FGA tuples.

- Backend services make **no authorization decisions**. The single residue is `/me/*` **list** endpoints ("my applications", "my tasks"), where the service filters rows by the JWT `sub` from Heimdall — data scoping on the caller's own records, not a grant/deny decision on an identified resource. Every route that carries a resource ID gets a Heimdall rule instead.

docs/rewrite/04-authorization-model.md:15

• Publishing only at state transitions leaves a Postgres/NATS dual-write gap: if the database commit succeeds but publication is lost, a grant is absent or—more seriously—a revoked user can retain edge access indefinitely. The one-time migration seed cannot repair drift created after cutover. Define a durable publication/reconciliation mechanism (for example, a transactional outbox or CDC plus retries and periodic full-state repair) and its failure handling.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:69

• view is assigned two different audiences here, but Heimdall performs one relation check per route. Define a single union permission for reads and separate permissions for withdraw/evaluate so the model and RuleSet have unambiguous relation names.

  view/withdraw  = applicant
  view/evaluate  = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:74

• As with applications, view is defined twice even though each Heimdall endpoint checks one relation. Model reads as the union of assignee and reviewers, then give submit and review distinct permission names.

  view/submit    = assignee
  view/review    = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:84

• This contradicts the lifecycle table: a submitted application is in pending status, and line 93 emits its applicant and parent tuples immediately. Distinguish pending invitations from submitted/pending applications so implementers do not omit access tuples for applications awaiting evaluation.

4. **Pending invitations/applications have no FGA presence.** A pending mentor invitation or mentee application is a Postgres row. Tuples appear when the relationship becomes effective (invite accepted → `mentor`; application submitted → `applicant` + `parent` so mentors can evaluate it).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

docs/rewrite/04-authorization-model.md:17

• sub is the opaque Auth0 identity (for example, auth0|...), while this model and the existing ERD key users and FGA tuples by LFID. Filtering /me queries by sub will therefore not match the caller's LFID-backed records. Use the same canonical LFID username identity that Heimdall uses for FGA checks.

- Backend services make **no authorization decisions**. The single residue is `/me/*` **list** endpoints ("my applications", "my tasks"), where the service filters rows by the JWT `sub` from Heimdall — data scoping on the caller's own records, not a grant/deny decision on an identified resource. Every route that carries a resource ID gets a Heimdall rule instead.

docs/rewrite/04-authorization-model.md:15

• Publishing only at state transitions plus a one-time seed leaves an unhandled Postgres/NATS dual-write gap. A crash after the database commit but before broker receipt can permanently omit a grant or revocation; the cited member-service contract explicitly notes that local publish acceptance is not broker receipt and provides flush/reindex/repair paths. Define durable delivery and repair here (for example, a transactional outbox/CDC publisher plus reconciliation), especially for revocations.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:69

• The slash shorthand gives view two separate definitions, but Heimdall can check only one relation per rule. As written, it is unclear whether both applicants and reviewers satisfy the same application-view check. Define view once as their union and keep the action-specific permissions separate.

type mentorship_application
  relations: parent (mentorship_program), applicant (user)
  view/withdraw  = applicant
  view/evaluate  = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:74

• The task sketch likewise defines view twice via slash shorthand, leaving no single relation for Heimdall to check when both assignees and reviewers may read a task. Make the read permission an explicit union.

type mentorship_task
  relations: parent (mentorship_program), assignee (user)
  view/submit    = assignee
  view/review    = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:93

• Submitted applications remain pending until accepted (lines 54 and 56), so this emission conflicts with decision 4's statement that pending applications have no FGA presence. Clarify that only pre-submission applications lack tuples; submitted pending applications need them so mentors can evaluate them.

| Application submitted | `applications` row | `update_access`: applicant + parent program |

docs/rewrite/04-authorization-model.md:84

• With no tuple for a pending invitation, Heimdall cannot authorize the invite-acceptance request. The migration plan preserves signed invitation tokens, so token validity and recipient binding must still be checked in the service (and documented as another exception to line 17), or pending invitations need an edge-authorizable representation; otherwise the proposal leaves this grant-creating endpoint without an authorization mechanism.

4. **Pending invitations/applications have no FGA presence.** A pending mentor invitation or mentee application is a Postgres row. Tuples appear when the relationship becomes effective (invite accepted → `mentor`; application submitted → `applicant` + `parent` so mentors can evaluate it).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

docs/rewrite/04-authorization-model.md:15

• Publishing only at state transitions creates an unprotected PostgreSQL/NATS dual write: a crash after the database commit but before Publish permanently leaves the derived authorization index stale. The fga-sync contract guarantees delivery only after a message reaches its stream and explicitly requires the owning service to republish current state for recovery. Define a transactional outbox/CDC mechanism and a reconciliation path (or another durable equivalent), rather than relying on transition-time publication plus a one-time seed.

- The service publishes tuples to [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) (`GenericFGAMessage` over NATS) **at state transitions**, and once as a bulk seed after the DynamoDB → Postgres backfill (re-runnable: `update_access` is a full-state sync per object).

docs/rewrite/04-authorization-model.md:71

• view is assigned twice here, once to the applicant and once to program roles. An OpenFGA permission/relation has a single definition, so this sketch does not specify whether both audiences can view an application. Define one combined viewer permission and separate action permissions.

  view/withdraw  = applicant
  view/evaluate  = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:76

• As with applications, view is defined by two different expressions here. This is ambiguous when translated into the single relation Heimdall checks; combine both viewing audiences and keep submit/review as separate permissions.

  view/submit    = assignee
  view/review    = parent.mentor or parent.writer

docs/rewrite/04-authorization-model.md:86

• This says pending applications have no FGA presence, but the same paragraph and the lifecycle table say tuples are emitted when an application is submitted, whose resulting status is pending according to the status model above. Clarify that submitted/pending applications do receive tuples; otherwise mentors cannot evaluate them at the edge.

4. **Pending invitations/applications have no FGA presence.** A pending mentor invitation or mentee application is a Postgres row. Tuples appear when the relationship becomes effective (invite accepted → `mentor`; application submitted → `applicant` + `parent` so mentors can evaluate it).

docs/rewrite/04-authorization-model.md:94

• The lifecycle table defines how an admin is removed but has no corresponding event for adding an admin after program creation. Because admin rights depend on the writer tuple, a subsequently added admin would never gain edge authorization. Add the missing emission explicitly.

| Mentor invite accepted | `program_members` row | `member_put` mentor→program |

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

docs/rewrite/04-authorization-model.md:20

• The reconciliation described here cannot recover a delete_access message that is stored by NATS but later exhausts delivery: after the PostgreSQL row is gone, there is no current object from which this job can re-derive the deletion. The fga-sync contract also provides no application-level acknowledgement, so marking the outbox row sent on publish does not close this gap. Retain deletion tombstones/object inventory for periodic re-emission, or define a comparison against FGA, so deleted objects cannot leave permanent orphan tuples.

- **The relay builds each payload at send time, and never replays a stored one.** `update_access` is a full-state sync per object, which makes it idempotent only while it is still the newest state: the `GenericFGAMessage` envelope carries no object version or revision (verified against [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) — only the NATS stream sequence, which is not a per-object version), so fga-sync applies whatever arrives last. A stale payload retried after a newer `member_remove` or unpublish would restore exactly the tuple that was revoked. Two requirements follow: the relay **re-derives** the payload from current Postgres state when it sends (the outbox row is a dirty-object marker, not a frozen message), and pending rows for the same object are **coalesced** so only one in-flight sync per object exists. On the same basis, the periodic reconciliation job re-derives expected relations from Postgres and re-emits where FGA has drifted. Revocation lag is the metric to alert on.
- FGA is never queried by the Mentorship service and never holds business state (statuses, dates, categories, history).

docs/rewrite/04-authorization-model.md:45

• This graph labels the new authorization role with the banned legacy term maintainer. Repository terminology requires Program Admin for the rewrite and permits maintainer only when explicitly describing legacy data, so remove the parenthetical from this new-model node.

    ADMIN(["Program Admin<br/>(maintainer)"])

docs/rewrite/03-migration-plan.md:26

• This mapping cannot classify mentor rows solely as written. The PR notes that the same legacy pending status represents both invited mentors and self-applicants, while the target model stores invitations in program_members and applications in applications; treating all pending/declined mentor rows as applications will mis-migrate invitation records. Name the source discriminator (or a deterministic fallback) and provide mappings for both origins.

2. **Backfill.** One-shot, idempotent ETL: DynamoDB export → transform (un-nest program terms, map LFIDs, map every `project-members` row by member type) → Postgres. There is **no enrollment entity** — the application is the lifecycle object, so no application/enrollment split is performed (see [04](./04-authorization-model.md)). `project-members` holds three member types and **all three need a target**: `apprentice` rows become application rows carrying the full `pending → accepted → graduated` lifecycle; `mentor` rows become program-membership rows, and because mentors can *apply* as well as be invited ([01](./01-current-system.md)), their `pending` / `declined` rows are mentor applications that must survive the migration rather than be dropped as non-mentee; `maintainer` rows become program admins. Mapping only mentee rows would silently drop mentor applications — a parity feature per [02](./02-target-architecture.md). Field-level mapping dictionary as in Crowdfunding's `data-design_and_migration.md`. Re-runnable; validated with per-entity row-count reports, field-level source-to-target reconciliation (every source field maps to the expected target column), and referential-integrity checks (every un-nested child row resolves to the correct parent) — counts and spot checks alone are not sufficient for a document-to-relational transformation.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

docs/rewrite/03-migration-plan.md:26

• The migration still cannot distinguish the two legacy meanings of a pending mentor row. This document maps pending/declined mentor rows to applications, while 04-authorization-model.md:154 says a pending mentor row may instead be an invitation and gives invitations a different target lifecycle/FGA treatment. Specify the source discriminator (and fallback when absent) so pending invitations are not converted into applications or lost.

2. **Backfill.** One-shot, idempotent ETL: DynamoDB export → transform (un-nest program terms, map LFIDs, map every `project-members` row by member type) → Postgres. There is **no enrollment entity** — the application is the lifecycle object, so no application/enrollment split is performed (see [04](./04-authorization-model.md)). `project-members` holds three member types and **all three need a target**: `apprentice` rows become mentee application rows carrying the full `pending → accepted → graduated` lifecycle; `mentor` rows become program-membership rows, and because mentors can *apply* as well as be invited ([01](./01-current-system.md)), their `pending` / `declined` rows are mentor applications that must survive the migration rather than be dropped as non-mentee; `maintainer` rows become **program admin** membership rows. Mapping only mentee rows would silently drop mentor applications — a parity feature per [02](./02-target-architecture.md). Field-level mapping dictionary as in Crowdfunding's `data-design_and_migration.md`. Re-runnable; validated with per-entity row-count reports, field-level source-to-target reconciliation (every source field maps to the expected target column), and referential-integrity checks (every un-nested child row resolves to the correct parent) — counts and spot checks alone are not sufficient for a document-to-relational transformation.

docs/rewrite/04-authorization-model.md:224

• The proposed group-administered approver grant cannot be represented by the sketch: program_approver: [user] accepts only direct users, and the project type defines no approver relation to inherit. Either allow the intended team/userset or add a project relation and inheritance; otherwise the documented default requires user-by-user tuples despite saying to avoid them. This row also still says the sketch models no relation even though program_approver is now present.

| AQ-5 | Drop the HMAC email-approval links in favor of LF staff approving via a logged-in Self Serve page? The approval route needs a relation for Heimdall to check, and the sketch models none. | Drop them — one authorization model. Resolve the missing relation with **`program_approver` on `mentorship_program`** (in the sketch above) rather than by depending on an unmodeled platform-level super-admin: it is a relation this service owns and emits, so it is implementable without a cross-team prerequisite. Confirm the LF-staff grant is administered somewhere sane (a project-level or platform group tuple) rather than user-by-user. |

docs/rewrite/04-authorization-model.md:94

• This comment excludes the mentor-application path documented below: accepting a mentor application also writes a direct mentor tuple (line 181). Update it so implementers do not model invitations as the only source of direct mentors.

    # mentors are directly assigned only (via accepted invitation)

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Suppressed comments (1)

docs/rewrite/04-authorization-model.md:20

• This contradicts the preceding reconciliation requirement: re-emitting only "where FGA has drifted" requires reading actual FGA tuples, while this line prohibits every FGA query. Either make reconciliation an unconditional authoritative full-state re-emission, or explicitly permit the reconciler to read tuples; the current design cannot detect drift.

- FGA is never queried by the Mentorship service and never holds business state (statuses, dates, categories, history).

[mlehotskylf]

Leaving this open deliberately — it is the substance of AQ-5 rather than a defect. You are right that program_approver: [user] supports only direct per-program grants while the proposed default argues for a project-level or group tuple; that inconsistency is the open question the architecture team needs to settle, and I would rather it stay visible in the sketch than be resolved unilaterally here. Once they pick, the sketch and the lifecycle emission get the chosen term.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

docs/rewrite/01-current-system.md:59

• This notation reads as though both mentor and maintainer map to program admin. Make the unchanged mentor mapping explicit so this current-system summary cannot be mistaken for the ETL mapping.

| `project-members` | Many-to-many user↔program: memberType (`mentor` / `maintainer` → program admin / `apprentice` → mentee), status lifecycle                    |

docs/rewrite/04-authorization-model.md:175

• update_access is a full-state sync: omitted relation keys are deleted. Describing this transition as sending only the new public value would therefore remove the current writers, mentors, and approvers whenever a program is published or archived. Re-emit the complete current program snapshot here (or explicitly exclude separately managed relations).

| Program approved / unpublished / archived | `programs.status` change | `update_access` with the new `public` value — approval is what first emits `viewer@user:*`. The tuple is per-object, so without re-emitting on the way back down an archived program stays publicly authorized |

docs/rewrite/04-authorization-model.md:184

• The relay cannot re-derive these child deletions after the transaction has hard-deleted/cascaded the rows: their IDs are no longer present in PostgreSQL. Enqueue a durable delete marker for the program and every child object before removing the rows (or retain tombstones), and document this as the deletion exception to the “re-derive every payload” rule; otherwise child tuples remain orphaned.

| Program deleted | `programs` row + children deleted | `delete_access` for the program **and** for every application and task under it — otherwise their tuples are orphaned in FGA |

docs/rewrite/03-migration-plan.md:26

• This mapping conflates two legacy records: the authorization document states that the same pending mentor row can represent either an application or an invitation, but this sentence maps pending mentor rows unconditionally to applications. It also maps accepted mentor rows only to memberships, losing the accepted application record for mentors who applied. Define how the ETL distinguishes invitation-originated and application-originated rows (for example via invitation/token data), or document a fallback when that provenance is unavailable.

2. **Backfill.** One-shot, idempotent ETL: DynamoDB export → transform (un-nest program terms, map LFIDs, map every `project-members` row by member type) → Postgres. There is **no enrollment entity** — the application is the lifecycle object, so no application/enrollment split is performed (see [04](./04-authorization-model.md)). `project-members` holds three member types and **all three need a target**: `apprentice` rows become mentee application rows carrying the full `pending → accepted → graduated` lifecycle; `mentor` rows become program-membership rows, and because mentors can *apply* as well as be invited ([01](./01-current-system.md)), their `pending` / `declined` rows are mentor applications that must survive the migration rather than be dropped as non-mentee; `maintainer` rows become **program admin** membership rows. Mapping only mentee rows would silently drop mentor applications — a parity feature per [02](./02-target-architecture.md). Field-level mapping dictionary as in Crowdfunding's `data-design_and_migration.md`. Re-runnable; validated with per-entity row-count reports, field-level source-to-target reconciliation (every source field maps to the expected target column), and referential-integrity checks (every un-nested child row resolves to the correct parent) — counts and spot checks alone are not sufficient for a document-to-relational transformation.

docs/rewrite/04-authorization-model.md:213

• A mis-scoped relation does not necessarily fail open; an incorrect object or overly narrow subject can fail closed, while an overly broad relation can grant unintended access. Avoid the absolute security claim here.

**Validate locally before pushing.** The model and its tests run against a local OpenFGA in Docker, so relation changes are checked in seconds without a cluster deploy — worth doing per-change during PR 1, since a mis-scoped relation fails *open*.

docs/rewrite/04-authorization-model.md:204

• The deployment order also matters between PRs 2 and 3. Enabling RuleSets before the service has emitted/backfilled tuples makes every protected program, application, and task fail the new checks, denying legitimate traffic. The work can be developed in parallel, but tuple emission and backfill must run before enforcement is activated (or enforcement needs a coordinated feature flag).

PRs 2 and 3 are independent of each other and can land in parallel. The ordering matters in one direction only: a RuleSet referencing a relation that does not exist fails closed, so the model must be in place first.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (5)

docs/rewrite/03-migration-plan.md:26

• The source model does not distinguish these cases as written: 04-authorization-model.md:69 says one pending mentor row can represent either an application or an invitation, while the target gives those flows different records and FGA behavior. Mapping pending/declined rows wholesale to applications can turn outstanding invitations into evaluable applications, and mapping accepted rows only to memberships can discard accepted-application history. Define how the ETL detects provenance (or an explicit lossless fallback) for every mentor status.

2. **Backfill.** One-shot, idempotent ETL: DynamoDB export → transform (un-nest program terms, map LFIDs, map every `project-members` row by member type) → Postgres. There is **no enrollment entity** — the application is the lifecycle object, so no application/enrollment split is performed (see [04](./04-authorization-model.md)). `project-members` holds three member types and **all three need a target**: `apprentice` rows become mentee application rows carrying the full `pending → accepted → graduated` lifecycle; `mentor` rows become program-membership rows, and because mentors can *apply* as well as be invited ([01](./01-current-system.md)), their `pending` / `declined` rows are mentor applications that must survive the migration rather than be dropped as non-mentee; `maintainer` rows become **program admin** membership rows. Mapping only mentee rows would silently drop mentor applications — a parity feature per [02](./02-target-architecture.md). Field-level mapping dictionary as in Crowdfunding's `data-design_and_migration.md`. Re-runnable; validated with per-entity row-count reports, field-level source-to-target reconciliation (every source field maps to the expected target column), and referential-integrity checks (every un-nested child row resolves to the correct parent) — counts and spot checks alone are not sufficient for a document-to-relational transformation.

docs/rewrite/04-authorization-model.md:99

• This direct-user relation conflicts with AQ-5's proposed group/project-level administration and with the lifecycle table, which only seeds approvers when a program is created. A later LF-staff roster change would leave former staff authorized on existing programs and new staff unable to approve them. Define an inherited/team-backed relation and its tuple owner, or add explicit approver grant/revoke transitions for every program.

    define program_approver: [user]

docs/rewrite/04-authorization-model.md:19

• A coalesced dirty-object marker no longer identifies which username/relation changed, but the lifecycle contract later requires member_put/member_remove, whose payloads require that transition-specific data. Re-deriving only current state cannot safely reconstruct a removal when a user is removed and re-added before the relay runs. Either emit a full-state update_access for program membership changes, or retain mutation identity plus supersession semantics instead of coalescing it away.

- **The relay builds each payload at send time, and never replays a stored one.** `update_access` is a full-state sync per object, which makes it idempotent only while it is still the newest state: the `GenericFGAMessage` envelope carries no object version or revision (verified against [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) — only the NATS stream sequence, which is not a per-object version), so fga-sync applies whatever arrives last. A stale payload retried after a newer `member_remove` or unpublish would restore exactly the tuple that was revoked. Two requirements follow: the relay **re-derives** the payload from current Postgres state when it sends (the outbox row is a dirty-object marker, not a frozen message), and pending rows for the same object are **coalesced** so only one in-flight sync per object exists. On the same basis, the periodic reconciliation job re-derives expected relations from Postgres and re-emits where FGA has drifted. Revocation lag is the metric to alert on.

docs/rewrite/04-authorization-model.md:204

• RuleSets and tuple emission are not deployment-independent. If PR 2 activates a relation check before PR 3 has deployed the relay and seeded existing objects, the relation exists but every missing tuple evaluates as denied, making protected routes unavailable. The RuleSet rollout must wait for emission deployment and successful initial seeding, even if the code changes are developed in parallel.

PRs 2 and 3 are independent of each other and can land in parallel. The ordering matters in one direction only: a RuleSet referencing a relation that does not exist fails closed, so the model must be in place first.

docs/rewrite/02-target-architecture.md:16

• The PR description says the authorization and enrollment revisions to 02-target-architecture.md are deliberately out of scope, but this diff now makes both revisions. Update the description and remove its stale note so reviewers are not told that a changed architecture file remains intentionally untouched.

- **Behind the v2 API Gateway**, with Heimdall + OpenFGA authorizing at the edge — the idiomatic v2 pattern, since Mentorship programs are always subordinated under LF projects. See [04](./04-authorization-model.md).

[mlehotskylf]

Review Feedback Addressed

Commit: 8de33ab

The most useful thing this round surfaced: backend/db/migrations/001_initial.up.sql and specs/001-mentorship-core-workflow/ landed on main while this branch was open, and the proposal had drifted out of agreement with them. Three of the fixes below are that drift.

Changes Made

• 02, 04 — task category is prerequisite | non_prerequisite, per tasks_category_check. The docs used program, a value Postgres would reject.
• 02, 03, 04 — application lifecycle is pending → accepted → active → graduated, per applications_status_check. 03 also records that active has no distinct legacy value and must be derived at backfill (legacy leaves an admitted mentee on accepted for the whole term).
• 02 — added programs.project_uid to the ERD. The inheritance model depends on a mentorship_program#project@project:{uid} tuple and there was no column for the outbox to derive it from. Legacy carries it as lfProjectId and CreateProject requires it, but older programs predate that rule — so the backfill must report unmapped programs rather than import them silently.
• 04 — program_approver is now part of auditor. Pending programs have no public wildcard, so an approver holding only that relation could act on a program but not read it.
• 04 — the outbox durable-handoff criterion is the JetStream publish ack, not a reply; fga-sync mutation subjects are fire-and-forget.
• 04 — clarified that the no-FGA-reads rule constrains the request path, and noted that a Postgres-driven reconcile cannot discover orphan tuples left by hard deletes.
• PR description — updated: revising 02 is now in scope, since leaving it stale would have contradicted 04.

Declined

• Coalesced outbox marker vs. transition-specific member_put — already resolved by the re-derive-at-send-time rule stated in the same section.
• PR 2/3 deployment ordering — implementation-phase sequencing; the table scopes PRs, not rollouts.
• "Not every incorrect relation fails open" — the fail-closed case is stated a few lines later.
• Invitation-vs-application provenance in the ETL (two threads) — real, and exactly the field-level mapping-dictionary work 03 defers to the Build phase.
• Mentor-grant path missing from a DSL comment — the lifecycle table is normative; duplicating it in comments invites drift.

Still Open

• 04:99 — program_approver grant mechanism. Left unresolved on purpose: [user] supports only direct per-program grants while the AQ-5 default argues for a project-level or group tuple. That inconsistency is the question for the Architecture team, not something to settle unilaterally in this PR.

15 of 16 threads resolved.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Suppressed comments (2)

docs/rewrite/03-migration-plan.md:26

• This mapping treats pending/declined mentor rows as applications, but the authorization proposal says the legacy pending mentor state represents both self-applications and invitations (04-authorization-model.md:69,156). Converting every pending mentor row into an application would give outstanding invitations applicant/parent tuples and place them in the review workflow. Define the source discriminator and map invitations and applications separately; also clarify which states produce an application row, a program-membership row, or both.

2. **Backfill.** One-shot, idempotent ETL: DynamoDB export → transform (un-nest program terms, map LFIDs, map every `project-members` row by member type) → Postgres. There is **no enrollment entity** — the application is the lifecycle object, so no application/enrollment split is performed (see [04](./04-authorization-model.md)). `project-members` holds three member types and **all three need a target**: `apprentice` rows become mentee application rows carrying the full `pending → accepted → active → graduated` lifecycle; `mentor` rows become program-membership rows, and because mentors can *apply* as well as be invited ([01](./01-current-system.md)), their `pending` / `declined` rows are mentor applications that must survive the migration rather than be dropped as non-mentee; `maintainer` rows become **program admin** membership rows. Mapping only mentee rows would silently drop mentor applications — a parity feature per [02](./02-target-architecture.md). Field-level mapping dictionary as in Crowdfunding's `data-design_and_migration.md`. Re-runnable; validated with per-entity row-count reports, field-level source-to-target reconciliation (every source field maps to the expected target column), and referential-integrity checks (every un-nested child row resolves to the correct parent) — counts and spot checks alone are not sufficient for a document-to-relational transformation.

docs/rewrite/04-authorization-model.md:19

• The reconciliation publisher is not explicitly covered by the relay's one-in-flight guarantee. It can read pre-revocation state and publish that full snapshot after the relay publishes a newer removal; with no object version and stream publication order determining the winner, this restores revoked access until another reconciliation. Route reconciliation through the same per-object serialization/outbox (or otherwise detect a state-version change before publishing).

- **The relay builds each payload at send time, and never replays a stored one.** `update_access` is a full-state sync per object, which makes it idempotent only while it is still the newest state: the `GenericFGAMessage` envelope carries no object version or revision (verified against [fga-sync](https://github.com/linuxfoundation/lfx-v2-fga-sync) — only the NATS stream sequence, which is not a per-object version), so fga-sync applies whatever arrives last. A stale payload retried after a newer `member_remove` or unpublish would restore exactly the tuple that was revoked. Two requirements follow: the relay **re-derives** the payload from current Postgres state when it sends (the outbox row is a dirty-object marker, not a frozen message), and pending rows for the same object are **coalesced** so only one in-flight sync per object exists. On the same basis, the periodic reconciliation job re-derives expected relations from Postgres and re-emits them. It re-emits unconditionally rather than diffing against FGA — the "no FGA reads" rule constrains the **request path**, where Heimdall is the only component that queries FGA; an offline job may read FGA, but a blind full re-emit is simpler and `update_access` is idempotent. Note this repairs drift only for objects Postgres still knows about; a lost `delete_access` for a hard-deleted row leaves an orphan tuple that no Postgres-driven scan can discover, which is a further reason deletion needs the explicit transition described below. Revocation lag is the metric to alert on.

[mlehotskylf]

Review Feedback Addressed (iteration 2)

Commit: 1bd0523

Same theme as the last round — the proposal disagreeing with the schema already merged in backend/db/migrations/001_initial.up.sql. This time the program status enum, plus an error I introduced in the previous commit.

Changes Made

• 02, 04 — program status now uses the real enum from programs_status_check: draft | submitted | published | rejected | archived | hidden. The docs used a pending status that does not exist, and collapsed draft → submitted — which is a genuine authorization transition (a writer submits; a program_approver then decides), so the emissions table now lists each transition on its own row.
• 04 — stated explicitly that published alone emits viewer@user:*, making public derivable as status == 'published'. It previously was not derivable from the table at all.
• 02 — added active to the applications ERD; the prose was fixed last round but the ERD in the same file still omitted it.
• 03, 04 — removed the claim that the status list matches applications_status_check. It does not: the constraint permits hold while these docs recommend moving pause state to its own field.

Open Decision Surfaced

Is hold an application status? The merged schema says yes; this proposal argues no — a paused mentorship is a state of an accepted mentee, not an application outcome. Both documents now name the divergence rather than assert an agreement that does not exist. Resolving it means either a follow-up migration dropping hold from the constraint, or dropping the recommendation and mapping hold straight through. Either is defensible; shipping the ETL contract while the two disagree is not. Worth an explicit call during review.

Still Open

• 04:99 — program_approver grant mechanism. Unchanged from the last iteration: [user] supports only direct per-program grants while the AQ-5 default argues for a project-level or group tuple. Left for the Architecture team.

4 of 5 threads resolved this iteration.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/rewrite/01-current-system.md:59

• The arrow grouping currently reads as though both mentor and maintainer map to program admin. Mentor remains a distinct target role; make each legacy-to-target mapping explicit so this inventory does not contradict the migration plan.

| `project-members` | Many-to-many user↔program: memberType (`mentor` / `maintainer` → program admin / `apprentice` → mentee), status lifecycle                    |