GeezerVison

Copy link

Copy Markdown

Collaborator

ibetitsmike added this pull request to stack #30614 October 11, 2026 00:29

chatgpt-codex-connector Bot reviewed Oct 11, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ibetitsmike marked this pull request as ready for review October 11, 2026 10:18

ibetitsmike requested review from a team and nickvigilante as code owners October 11, 2026 10:18

pr-queue-manager Bot requested a review from f0ssel October 11, 2026 10:19

f0ssel approved these changes Oct 12, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 13, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Base automatically changed from mike/skills/data-model to main October 13, 2026 15:51

## Stack

Part of
[CODAGT-1341](https://linear.app/codercom/issue/CODAGT-1341/support-organization-level-skills-in-coder-agents).
This stack adds organization skills to Coder Agents: admins publish a
skill once per organization, share it with groups or members, and chats
in that organization load it next to personal and workspace skills. It
replaces the 21-PR draft stack #30584 to #30604 with five PRs, bottom to
top:

1. **#30609** data model: one migration, queries, RBAC and dbauthz (this
PR)
2. #30610 organization skill API routes and the scope-neutral SDK
3. #30611 sharing ACL routes
4. #30612 settings UI: shared skills table, admin page, member section
5. #30613 chats: chatd loading, `/` menu, docs

## What changes

This PR adds the data model and authorization for organization skills.
Personal skills keep working unchanged: `git diff origin/main --
codersdk site` shows only generated RBAC and API key scope additions
(`apikey_scopes_gen.go`, `rbacresources_gen.go`,
`rbacresourcesGenerated.ts`, the RBAC and scope unions in
`typesGenerated.ts`) and `codersdk.ResourceTypeOrganizationSkill` with
its friendly string. No personal route, SDK type or site component
changes, and `coderd/userskills_test.go` changes only by mechanical
constant renames.

**One migration, `000621_skills`** (please review it closely):

- renames `user_skills` to `skills` with its constraints, index and
triggers, and recreates the cap function and
`delete_deleted_user_resources`, whose PL/pgSQL bodies name the table;
- adds `organization_id` and `project_id` (`ON DELETE CASCADE`),
`enabled` (default true), and `group_acl` / `user_acl` (`jsonb`, default
`{}`), with CHECKs for exactly one owner, ACLs that are JSON objects,
and ACLs that stay empty unless an organization owns the row, plus
partial unique name indexes per owner kind;
- replaces the cap trigger with `enforce_skills_per_owner_limit()`: 100
rows per owner, disabled rows included, locking the owner row `FOR NO
KEY UPDATE` so unrelated foreign key checks are not blocked;
- adds a `BEFORE DELETE` trigger on `organization_members` that removes
a departing member from `user_acl` on that organization's skills, so
rejoining doesn't restore a direct grant;
- adds the `organization_skill` `resource_type` and `api_key_scope` enum
values last; nothing in the migration uses them.

The down migration undoes the three old migrations in reverse order: it
drops organization and project rows and restores `user_skills`, its
names, the cap function and `user_id NOT NULL`.

Authorization: a new RBAC resource `organization_skill` (create, read,
update, delete, share). Owner and org admin get it; site and org
auditors get read; members read only through the skill's ACL, which
applies only to members of its organization. `Skill.RBACObject()`
returns the organization object with both ACLs; project rows stay denied
to everyone. New sqlc queries cover organization insert, get, list
(SQL-filtered through `GetAuthorizedOrganizationSkillMetadata` and a
`regosql` converter), update and delete, with dbauthz wrappers and the
audit table entries. No routes yet.

## Commits

The first commit reproduces the reviewed code from the old stack; each
later commit is a labeled trim or fix.

- `baa3150e3b7` feat: add the organization skills data model.
Reproduction: every non-site file this commit changes equals the
reviewed old `authz` head `1e16b3004c4`, except that the old stack's
migrations 000615, 000616 and 000617 are merged into one migration (then
`000615_skills`, now `000621_skills`) with one fixture
(`testdata/fixtures/000621_skills.up.sql`) and one migration test (now
`testMigration000621Skills`). At that commit, `dump.sql` was
byte-identical to the old head's.
- `1b84e83463a` test(coderd/database): keep only the SQL paths of the
org skill matrix. `TestOrganizationSkillAuthorization` becomes
`TestOrganizationSkillQueries` instead of being deleted, because
coverage showed it is this PR's only run of the
`GetAuthorizedOrganizationSkillMetadata` row scan (80% to 65% without
it) and of the organization update and delete queries (100% to 0%).
Swapping `group_acl` and `user_acl` in the converter makes
`GrantedMember` fail. The removed cases are decided by
`TestRolePermissions`, `TestSkillRBACObjectOrganizationACL` and the
dbauthz method suite.
- `e2931b412e9` test(coderd/database): fold the skill cap cases into
existing coverage. The user case is covered by `TestUserSkillLimit` and
`TestUserSkillLimitConcurrentCreates`; the organization case folds into
`ConcurrentOrganizationInsertsHonorCap`, which now also checks that
disabled rows count and another organization is unaffected (mutation
checked); the project case stays.
- `be62aea9021` test(coderd/rbac): drop
`TestOrganizationSkillReadScope`, covered by `TestExternalScopeNames`
and the generated scope file.
- `0c275ba37de` merges main at `454a2fcc697`, which took 000615 to
000618: it renumbers the migration to 000619 and resolves `dump.sql`,
`check_constraint.go` and the generated API docs. `8f2b4b87ee2` merges
main at `ad85646164e`, which took 000619: it renumbers the migration to
000620.
- `5248d56b64f` merges main at `e7b76872098`, which took 000620
(`000620_audit_chat_project_instructions_resource_type`): it renumbers
the migration, fixture and migration test to `000621_skills` (step
`Migration000621Skills`, version 621), and resolves
`enterprise/audit/table.go` (keeping main's
`AuditableChatProjectInstructions` entries and the `Skill` entry),
`dump.sql` and `models.go` (the `resource_type` enum gets
`chat_project_instructions`, then `organization_skill`) and the
regenerated `schemas.md`. The up and down SQL are unchanged by every
renumbering.

## Size

Production +549/-67, tests +728/-33, generated +881/-314; total
+2158/-414. This is above the 500-line reviewer-friendly limit on
purpose: Mike asked for 4 to 5 PRs instead of the 21 small ones this
stack replaces.

## Replaces

#30584 (rename `user_skills` to `skills`), #30585 (skill owners), #30586
(organization skill authorization).

## Validation

- Migration test: up keeps the existing personal row and its ID; down
restores the `user_skills` names, cap function and `NOT NULL`, and drops
organization rows; soft-deleting a user removes their skills after up,
after down, and after re-up.
- `go test` passed on `be62aea9021`, before the main merges, for
`coderd` (skill tests), `coderd/database`, `dbauthz`, `migrations`,
`gentest`, `coderd/rbac/...`, `coderd/x/chatd`, `coderd/x/skills`,
`enterprise/audit` and `enterprise/coderd`; `make lint/check-scopes`
passed; the four personal skills Vitest files pass unchanged.
- On `5248d56b64f`: build, vet, `TestMigrate`, `TestMigrationChain`, the
`migrations`, `database`, `dbauthz`, `gentest`, `rbac` and
`enterprise/audit` packages pass; `make gen` run twice is clean, and
`make pre-commit` passes.
- Remote UAT round 7 through Coder Agents passed on the whole stack; see
#30613's Validation for scope and findings.

> Xum opened this PR on behalf of @ibetitsmike.

52 of 54 checks passed

ibetitsmike deleted the mike/skills/org-api branch October 13, 2026 16:24