feat: add personal skill storage, API, and SDK by ibetitsmike · Pull Request #25363 · coder/coder

ibetitsmike · 2026-05-14T20:30:06Z

Stack Context

This PR is the storage, permissions, API, and SDK layer for experimental personal skills. #25362 has landed on main, so this branch is restacked directly on main.

Stack order:

feat: add personal skill storage, API, and SDK #25363 storage, permissions, API, and SDK
test: cover personal skill API #25365 API test coverage
feat: support personal skills in chats #25366 chattool and chatd integration
feat: add personal skills settings UI and docs #25066 settings UI and docs
feat(site/src/pages/AgentsPage): add personal skills slash menu #25386 personal skills slash menu

What?

Adds the user_skills database table, generated queries, RBAC resources and scopes, audit resource handling, experimental user-scoped CRUD endpoints, SDK types, and generated API/site types.

Follow-up review and restack fixes:

Enforce a bounded personal skill description in parser and database constraints.
Return 403 Forbidden for unauthorized create and update attempts.
Return explicit conflict responses when soft-deleted users are targeted.
Keep user admins out of personal skills, while site owners can read and delete but not create or update.
Document trigger-raised constraint names and keep schema constants covered by tests.
Reuse UserSkillMetadata in the full UserSkill SDK response type.
Generate user skill IDs in Go instead of relying on a database default.
Rebase on latest main and renumber the user skills migration to 000502_user_skills.

Why?

Personal skills need durable user-owned storage with owner authorization, limited site-owner moderation, and a hidden API surface before chatd can consume them.

Validation

make gen
go test ./coderd/database -run '^TestUserSkillSchemaConstants$' -count=1
go test ./coderd/database/dbauthz -run '^TestMethodTestSuite/TestUserSkills$' -count=1
go test ./coderd -run '^TestPatchUserSkill$' -count=1
go test ./codersdk ./coderd/database/db2sdk
make lint
pre-commit hook on 97fd58108d

github-actions · 2026-05-14T20:30:16Z

Docs preview

📖 View docs preview for docs/admin/security/audit-logs.md

ibetitsmike · 2026-05-14T23:22:23Z

/coder-agents-review

ibetitsmike · 2026-05-15T06:31:02Z

/coder-agents-review

coder-agents-review

Solid infrastructure PR. The resource follows the user_secrets pattern faithfully across all layers (migration, RBAC, dbauthz, audit, SDK) and deliberately diverges where it should (owner-role access). The trigger-based limit enforcement with FOR UPDATE serialization, the three-layer soft-delete protection, the split list/detail types omitting content from metadata, and the audit content masking are all well-reasoned. The migration comments earn their lines by explaining WHY, not WHAT.

Severity count: 1 P2, 5 P3, 1 P4, 4 Nit.

The P2 is a missing Is404Error check causing 500s on authorization failures. Two of the P3s interact: the deleted-user trigger's FOR UPDATE creates a deadlock path with soft-delete, and its untyped ERRCODE makes the race error uncatchable. The fix for both is the same: drop FOR UPDATE to match the user_secrets pattern and add a typed ERRCODE.

Test coverage is in stacked PR #25365 and not flagged here.

coder-agents-review

DEREM-3 (P2, missing Is404Error check) was addressed in 37f4627. Thanks for the quick fix.

13 findings from round 1 remain open with no author response or code change. Further review is blocked until these are addressed, acknowledged, or contested. The open items:

DEREM-4 (P3): FOR UPDATE deadlock path in deleted-user trigger
DEREM-5 (P3): Trigger ERRCODE uncatchable, producing 500
DEREM-6 (P3): 403 for quota exhaustion instead of 409
DEREM-7 (P3): No CHECK constraints for content size / name format
DEREM-8 (P3): Dual-sourced skill limit with no sync test
DEREM-1 (P3): Zero handler test coverage (pending stacked PR #25365)
DEREM-9 (P4): Unbounded skill name length
DEREM-10 (Nit): Message capitalization convention
DEREM-11 (Nit): Magic numbers in body-size formula
DEREM-12 (Nit): xerrors.Is vs errors.Is
DEREM-13 (Nit): Converters inline vs db2sdk
DEREM-2 (Nit): Raw constraint string vs typed constant
DEREM-16 (Note): Dead branch in trigger

For each, either push a fix, reply explaining why the current code is correct, or note that it's deferred with a ticket reference.

ibetitsmike · 2026-05-15T07:08:01Z

/coder-agents-review

coder-agents-review

11 findings addressed in 943fba4. The deadlock fix (DEREM-4), typed ERRCODE (DEREM-5), CHECK constraints (DEREM-7), status code change (DEREM-6), name length cap (DEREM-9), and all nits look good. Substantial progress.

2 findings remain open with no author response:

DEREM-1 (P3): Zero handler test coverage. Thread was resolved in the GitHub UI but no code change and no comment. PR #25365 in the stack is titled "API test coverage." If that PR covers handler tests, a brief reply confirming that would unblock this.

DEREM-8 (P3): Dual-sourced skill limit (trigger hardcodes 100, Go constant says 100) with no sync test. Thread was resolved in the GitHub UI but no code change and no comment. Either add a test asserting the values match, or reply explaining why the comment-only coupling is acceptable.

Further review is blocked until these 2 items get a response (fix, acknowledgment, or ticket).

ibetitsmike · 2026-05-15T07:26:55Z

/coder-agents-review

coder-agents-review

This is the third consecutive blocked round. 12 of 14 findings are addressed; the fixes look substantial and the panel needs to verify them. Two items are holding up that verification:

DEREM-1 (test coverage): A one-line reply on that thread confirming "Tests are in #25365" would unblock this.

DEREM-8 (dual-sourced limit): A one-line reply like "Accepted; the comment coupling is sufficient for this experimental feature" would unblock this.

No code changes needed. Just a reply on each thread so the panel can proceed to verify the 12 fixes.

ibetitsmike · 2026-05-15T07:47:41Z

/coder-agents-review

coder-agents-review

All 12 fixes from round 1 verified by the panel: deadlock eliminated (DEREM-4), trigger ERRCODE typed and catchable (DEREM-5), 409 for quota (DEREM-6), CHECK constraints added (DEREM-7), name length capped (DEREM-9), message casing (DEREM-10), named constants (DEREM-11), stdlib errors.Is (DEREM-12), converters in db2sdk (DEREM-13), dead branch removed (DEREM-16). Clean work.

DEREM-1 (test coverage): Panel accepts the stacked PR defense (3/3). Tests in approved #25365. Closed.

2 new findings this round: 1 P3 from Netero confirmed by 7/8 panel reviewers, 1 P3 from Bisky.

ibetitsmike · 2026-05-15T09:02:55Z

/coder-agents-review

coder-agents-review

DEREM-18 (name-length test) addressed in dedac96. Thanks.

1 finding remains: DEREM-17 (P3, patchUserSkill missing userSkillUserDeletedConstraint handling on UPDATE). Confirmed by 7/8 panel reviewers in round 5. The fix is ~3 lines mirroring postUserSkill at line 84. Either push the fix or reply on that thread explaining why the current behavior is acceptable.

ibetitsmike · 2026-05-15T09:21:46Z

/coder-agents-review

coder-agents-review

DEREM-17 fixed in c4bd7c6. patchUserSkill now handles the deleted-user constraint on UPDATE, mirroring postUserSkill. Panel verified (4/4, no findings).

All 16 findings from this review are resolved: 15 fixed across 5 commits, 1 panel-closed (test coverage in stacked PR #25365), 1 acknowledged (dual-sourced limit acceptable for experimental scope). The fixes were systematic, root-cause-addressing, and introduced no regressions.

ibetitsmike · 2026-05-16T12:41:31Z

/coder-agents-review

coder-agents-review

All 20 DEREM findings remain resolved. No new panel findings to raise.

CI is failing with 18 failures (gen, lint, fmt, test-go-pg, build, sqlc-vet, and more). These need to pass before merge.

8 @Emyrk threads are still Unresolved. Three are new since R9:

userskills.go:31: Why aren't the check constraint constants auto-generated into check_constraint.go?
userskills.go:209: No rename path (delete and re-add). @Emyrk notes "fine for now."
migrations/000498:13: Description column has no size constraint, unlike name and content.

Plus 5 from earlier rounds (auth-failure 404, deleted-user 404, owner role, userAdmin access, sync test). The author should respond to @Emyrk on each.

> Mux updated this PR on behalf of Mike. ## Stack Context This stack splits experimental personal skills into smaller reviewable PRs. Personal skills are user-owned `SKILL.md` files stored by Coder and injected into chatd alongside workspace skills. Stack order: 1. #25362 personal skill resolver 2. #25363 storage, permissions, API, and SDK 3. #25365 API test coverage 4. #25366 chattool and chatd integration 5. #25066 settings UI and docs 6. #25386 personal skills slash menu ## What? Adds the shared personal skill parser and resolver package, plus reusable skill-name validation exported from `workspacesdk`. The parser enforces the full personal skill contract: max raw size, kebab-case name, max name length, and non-empty body. ## Why? The rest of the stack needs one source-aware resolver for personal and workspace skills, including collision handling and qualified aliases. Keeping personal skill constraints in the parser prevents callers from accidentally parsing invalid personal skills. ## Validation - `go test ./coderd/x/skills ./codersdk/workspacesdk` - pre-commit hooks on this branch

ibetitsmike · 2026-05-16T17:15:11Z

Addressed the open #25363 review feedback in the latest head:

Trigger-raised constraint names are documented next to their constants because dbgen only emits table CHECK constraints.
Personal skill descriptions are now capped in both parser validation and the database constraint.
Unauthorized create now returns 403 Forbidden, and soft-deleted user writes return explicit 409 Conflict responses.
User admins have no personal skill access. Site owners are limited to read and delete for support and abuse handling, with no create or update permission.
The SQL trigger skill limit is covered by TestUserSkillSchemaConstants.
Rename remains delete plus create for v1, which matches the PRD non-goal.

Validation is listed in the PR body. I am continuing to watch CI and review automation.

ibetitsmike · 2026-05-16T17:15:59Z

/coder-agents-review

coder-agents-review

Post-@Emyrk changes verified by the panel (3/3). The RBAC model is now correctly scoped: owner gets read+delete only (no create/update), postUserSkill returns 403 for unauthorized creates, deleted-user errors are descriptive, description has a DB size constraint, and schema constants are guarded by a sync test.

All 20 DEREM findings resolved across 11 rounds. All 8 @Emyrk threads resolved. CI passing.

Emyrk

Last nit, then I can stamp

ibetitsmike requested a review from Emyrk as a code owner May 14, 2026 20:30

github-actions Bot assigned ibetitsmike May 14, 2026

ibetitsmike changed the title ~~feat: add personal skill storage and permissions~~ feat: add personal skill storage, API, and SDK May 14, 2026

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from 217af13 to 37f4627 Compare May 15, 2026 06:30

coder-agents-review Bot reviewed May 15, 2026

View reviewed changes

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from 37f4627 to 943fba4 Compare May 15, 2026 07:07

coder-agents-review Bot reviewed May 15, 2026

View reviewed changes

Comment thread coderd/userskills.go

Comment thread coderd/x/skills/skills.go Outdated

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from 943fba4 to dedac96 Compare May 15, 2026 09:02

coder-agents-review Bot reviewed May 15, 2026

View reviewed changes

coder-agents-review Bot approved these changes May 15, 2026

View reviewed changes

This was referenced May 15, 2026

feat: add personal skills settings UI and docs #25066

Merged

feat(site/src/pages/AgentsPage): add personal skills slash menu #25386

Merged

Emyrk reviewed May 15, 2026

View reviewed changes

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from c4bd7c6 to 20c907f Compare May 15, 2026 22:43

ibetitsmike force-pushed the mike/personal-skills-resolver branch from 01003e4 to 10b94ad Compare May 16, 2026 12:39

coder-agents-review Bot reviewed May 16, 2026

View reviewed changes

Base automatically changed from mike/personal-skills-resolver to main May 16, 2026 15:33

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from 70e607f to 6c83f7d Compare May 16, 2026 16:15

ibetitsmike requested a review from Emyrk May 16, 2026 17:15

coder-agents-review Bot approved these changes May 16, 2026

View reviewed changes

Emyrk reviewed May 18, 2026

View reviewed changes

Comment thread codersdk/userskills.go

Emyrk reviewed May 18, 2026

View reviewed changes

Comment thread coderd/userskills.go

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from 6c83f7d to 6098a23 Compare May 19, 2026 16:10

ibetitsmike requested a review from Emyrk May 19, 2026 18:01

Emyrk reviewed May 19, 2026

View reviewed changes

Comment thread coderd/database/migrations/000498_user_skills.up.sql Outdated

Emyrk reviewed May 19, 2026

View reviewed changes

ibetitsmike requested a review from Emyrk May 19, 2026 19:00

ibetitsmike added 8 commits May 19, 2026 19:42

feat: add personal skill storage and permissions

2f2f087

feat: add personal skill API and SDK

a449235

fix(coderd): handle deleted users on skill update

d4492c3

fix(coderd): treat user skills as audit-visible instructions

e528f9c

fix(coderd): address personal skill review feedback

49c01ec

fix: address user skill review feedback

9844273

fix(coderd): generate user skill IDs in Go

3f8d1eb

fix: renumber user skills migration

97fd581

ibetitsmike force-pushed the mike/personal-skills-storage-permissions branch from f123e0a to 97fd581 Compare May 19, 2026 19:51

Emyrk approved these changes May 19, 2026

View reviewed changes

ibetitsmike merged commit 5a8d001 into main May 19, 2026
36 checks passed

ibetitsmike deleted the mike/personal-skills-storage-permissions branch May 19, 2026 22:09

github-actions Bot locked and limited conversation to collaborators May 19, 2026

Conversation

ibetitsmike commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack Context

What?

Why?

Validation

Uh oh!

github-actions Bot commented May 14, 2026

Docs preview

Uh oh!

ibetitsmike commented May 14, 2026

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibetitsmike commented May 15, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ibetitsmike commented May 16, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibetitsmike commented May 16, 2026

Uh oh!

ibetitsmike commented May 16, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ibetitsmike commented May 14, 2026 •

edited

Loading