@@ -0,0 +1,119 @@ |
| | 1 | +# Teams (S31) |
| | 2 | + |
| | 3 | +Teams are sub-groups within an organization. They're the canonical |
| | 4 | +way to grant repo access to a set of users without per-user collab |
| | 5 | +rows. The S15 policy aggregator unions team-granted permissions with |
| | 6 | +direct collaborator permissions and the org-owner implicit-admin rule. |
| | 7 | + |
| | 8 | +## Schema (migration 0035) |
| | 9 | + |
| | 10 | +``` |
| | 11 | +teams — (org_id, slug, parent_team_id, privacy, …) |
| | 12 | +team_members — (team_id, user_id, role enum('member','maintainer')) |
| | 13 | +team_repo_access — (team_id, repo_id, role enum('read'..'admin')) |
| | 14 | +``` |
| | 15 | + |
| | 16 | +**One-level nesting** is structurally enforced by a `BEFORE INSERT |
| | 17 | +OR UPDATE` trigger that checks the parent team's `parent_team_id IS |
| | 18 | +NULL`. Two-level deep nesting raises SQLSTATE 23514, which the |
| | 19 | +orchestrator translates to `ErrTeamNestingTooDeep`. |
| | 20 | + |
| | 21 | +## Permission resolution |
| | 22 | + |
| | 23 | +The S15 policy contract gets one new source. For an org-owned repo, |
| | 24 | +`policy.effectiveRole` returns: |
| | 25 | + |
| | 26 | +``` |
| | 27 | +max( |
| | 28 | + org_role == owner → admin, (S30) |
| | 29 | + direct collab role, (S15: repo_collaborators) |
| | 30 | + team grant role over team-set, (S31: team_repo_access) |
| | 31 | +) |
| | 32 | +``` |
| | 33 | + |
| | 34 | +Where the **team-set** for a user in an org is: |
| | 35 | +- every team they're a direct member of, **plus** |
| | 36 | +- the parent team of each (one hop, per nesting cap). |
| | 37 | + |
| | 38 | +The aggregator resolves team-set + grants in two indexed queries |
| | 39 | +(`team_members JOIN teams` + `team_repo_access WHERE team_id = ANY(...)`) |
| | 40 | +and stores the winning role in the per-request `policy.WithCache` |
| | 41 | +memo so subsequent `Can()` calls in the same request reuse it. |
| | 42 | + |
| | 43 | +`team_role = maintainer` is about managing the team (adding members); |
| | 44 | +it does **not** elevate repo perms beyond the team's |
| | 45 | +`team_repo_access.role`. Confusion with the repo `maintain` role is |
| | 46 | +flagged in the team-management UI. |
| | 47 | + |
| | 48 | +## Visibility |
| | 49 | + |
| | 50 | +* `privacy='visible'`: team metadata visible to all org members; basic |
| | 51 | + info (name + description) visible to anyone who can see the org. |
| | 52 | +* `privacy='secret'`: team metadata, members, and repo grants visible |
| | 53 | + only to (team members ∪ org owners). |
| | 54 | + |
| | 55 | +The list page filters secret teams the viewer can't see; the team |
| | 56 | +view page 404s for the same reason. The markdown `@org/team` resolver |
| | 57 | +must return `ok=false` for secret teams the viewer can't see — it |
| | 58 | +falls back to plain text rather than leaking existence via a 404. |
| | 59 | + |
| | 60 | +## Cross-repo refs + mentions |
| | 61 | + |
| | 62 | +* **`owner/repo#N`**: now dispatches via `principals.Resolve` so |
| | 63 | + org-owned repos resolve. `internal/issues/references.go` was the |
| | 64 | + S21 deferral; cleared this sprint. |
| | 65 | +* **`@org/team`**: a new alternation in |
| | 66 | + `internal/markdown/extensions/extensions.go::reCombined` matches |
| | 67 | + the slash form before the bare `@user` branch, so the trailing |
| | 68 | + `/team` doesn't get split off. Routes to `/{org}/teams/{team}` |
| | 69 | + via the Team resolver. S25 deferral cleared. |
| | 70 | + |
| | 71 | +## Routes |
| | 72 | + |
| | 73 | +``` |
| | 74 | +GET /{org}/teams list (secret teams filtered) |
| | 75 | +POST /{org}/teams create (owner-only) |
| | 76 | +GET /{org}/teams/{slug} view: members + repo grants |
| | 77 | +POST /{org}/teams/{slug}/members add (form + role) / remove (action=remove) |
| | 78 | +POST /{org}/teams/{slug}/repos grant (repo_id + role) / revoke (action=remove) |
| | 79 | +``` |
| | 80 | + |
| | 81 | +## What we deferred from the spec |
| | 82 | + |
| | 83 | +* **Set-parent UI**: orchestrator (`SetTeamParent`) + the trigger |
| | 84 | + validation are in place; the form to retarget a parent in the UI |
| | 85 | + lands in a follow-up. |
| | 86 | +* **Repo-settings team-grant UI**: the team-side grant form on |
| | 87 | + `/{org}/teams/{slug}` works today; mirroring it on |
| | 88 | + `/{owner}/{repo}/settings/access` is S32 territory. |
| | 89 | +* **Team rename + redirects**: `team_redirects` table from the spec |
| | 90 | + isn't shipped; defer to the same follow-up that does the |
| | 91 | + `username_redirects → principal_redirects` rename. |
| | 92 | +* **Notifications for "added to team" / "team granted access"**: |
| | 93 | + routing matrix in S29 is wired for arbitrary kinds; emit-side hook |
| | 94 | + lands when these surfaces next get touched. |
| | 95 | +* **API endpoints** `GET /api/v1/orgs/{org}/teams` etc.: same |
| | 96 | + posture as S30 — defer to S33/S34 API consolidation. |
| | 97 | +* **CODEOWNERS / SCIM team provisioning** are post-MVP per spec. |
| | 98 | + |
| | 99 | +## Pitfalls noted in code |
| | 100 | + |
| | 101 | +* **Permission caching staleness**: the per-request memo is |
| | 102 | + invalidated by `policy.InvalidateRepo`; cross-request changes are |
| | 103 | + picked up on the next request. |
| | 104 | +* **Cycle prevention**: `parent_team_id != id` row CHECK + the |
| | 105 | + one-level trigger together prevent every cycle shape. |
| | 106 | +* **Deleting the parent team** flips children to top-level via |
| | 107 | + `ON DELETE SET NULL` on `parent_team_id` (the spec's lean). |
| | 108 | +* **Outside collaborator on a member's account**: removing the user |
| | 109 | + from the org clears team memberships but NOT direct collab rows |
| | 110 | + — this matches GitHub. Implementation note: today the org member |
| | 111 | + removal in `orgs.RemoveMember` doesn't delete team rows |
| | 112 | + explicitly; the `org_members` row removal cascades through |
| | 113 | + `team_members` only via shared `users` FK. **Follow-up**: when |
| | 114 | + `orgs.RemoveMember` lands its policy-driven cleanup pass, it |
| | 115 | + should also `DELETE FROM team_members WHERE user_id = $1 AND |
| | 116 | + team_id IN (SELECT id FROM teams WHERE org_id = $2)`. Tracked. |
| | 117 | +* **Secret-team metadata leak via repo-settings page**: the |
| | 118 | + `/{owner}/{repo}/settings/access` extension ships in S32; the |
| | 119 | + visibility filter must match the team-list page's logic. |
