mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Michael Suchacz
7993c73c87
Adds organization-scoped Coder Agents instructions. Organization admins
set them in **Admin settings** > **AI** > **Coder Agents** >
**Organization settings**, or with `GET`/`PUT
/api/v2/organizations/{organization}/chats/config/system-prompt`.
Implements [CODAGT-984](https://linear.app/codercom/issue/CODAGT-984).
The instructions add to the deployment system prompt and never replace
it. When a chat is created, chatd stores the organization prompt as its
own system row right after the deployment row, only for chats in that
organization. Subagents resolve their parent organization's current
prompt when they spawn, and existing chats keep their stored rows.
Access reuses the organization `chat_model_config` permission: owners
and organization admins edit, auditors read, members get 404 on read and
403 on write. Writes reuse the deployment prompt's sanitization, size
limit (128 KiB by default), and audit behavior, with a new
`chat_organization_system_prompt` audit resource type. Migration
`000613` (the next number after `000612` on `main`) adds the
`chat_organization_system_prompts` table.
## Testing
- Go: enterprise authorization (including an update-only custom role),
validation, and audit tests; chatd row order and subagent tests.
- Frontend: Vitest interaction tests and stories for the card (loading,
load error, refetch error over the cached value, save error, read-only).
- Remote dogfood UAT, [evidence
chat](https://dogfood.cdr.dev/agents/b595a709-0709-44e6-9535-96a922974d61):
round 1 **PASS** on `b027cb5ca67` (permission matrix, validation, UI,
row order, cross-organization isolation, audit, subagents, regression).
Round 2 **PASS** on `618bff67c75` (loading and load-error states,
refetch error over the cached value with unsaved edits kept, update-only
custom role saves over the API, row order, the documented size limit,
rapid organization switching, double-click save).
- CI: rebased onto `main` (`98332c739b0`) as `3850fbaa922`; other commit
SHAs in this description are from before the rebase. Every check passed
on `3850fbaa922`, with `test-go-pg` green on the first attempt. After
the review fixes in item 7, `main` was merged in as `006c4951749` to
resolve new conflicts in generated files and renumber the migration to
`000613`. Every check passes on `006c4951749`. Before the rebase, every
check passed on `b4e76543df2`. `test-go-pg (ubuntu-latest)` passed on
its fourth attempt; the first three failed only the `cli` package
goroutine leak check, with every test passing. Two of those leaked a
coderd instance, which also happens intermittently on `main`:
`TestServer/Logging` cancels `coder server` during startup, and an early
return in `cli/server.go` skips closing coderd. That fix is in #30280.
The first attempt leaked an idle HTTP connection instead (not
investigated).
## Screenshots
From remote UAT on a test deployment (test data only).
| Organization admin saves instructions (`b027cb5ca67`) | Auditor sees
read-only instructions, no save actions (`b027cb5ca67`) |
|---|---|
| 
| 
|
| **Loading state while the first fetch is pending (`618bff67c75`)** |
**Refetch error shown above the field; the unsaved edit is kept and Save
stays enabled (`618bff67c75`)** |
| 
| 
|
## Review record
1. Codex,
[review](https://github.com/coder/coder/pull/30272#pullrequestreview-5387245297)
on `c42b5f9e05c`: 6 findings. Fixed the missing loading state, refetch
failures hiding cached instructions, and two docs style issues
(`58eab5603a4`, `cc73b714630`). Two were first declined in error, then
corrected: the read-only test now checks that typing leaves the value
unchanged (`cc73b714630`), and update-only custom roles no longer get a
500 on save (`618bff67c75`).
2. Independent security review by a clean-context agent,
[comment](https://github.com/coder/coder/pull/30272#issuecomment-5943505041)
on `c42b5f9e05c`: no P0 to P2 findings. A stale P3 comment was fixed in
`34c17aadb7f`; returning `err.Error()` in 500 details was kept to match
the deployment prompt handlers.
3. Codex,
[review](https://github.com/coder/coder/pull/30272#pullrequestreview-5387323380)
on `58eab5603a4`: 2 findings (refetch errors not surfaced, docs line
breaks), both fixed in `cc73b714630`.
4. Codex, blockers only,
[review](https://github.com/coder/coder/pull/30272#pullrequestreview-5387462070)
on `618bff67c75`: 1 P2 (a failed refresh right after a successful save
shows the previous value). Not a blocker, deferred.
5. Final fresh advisory by a clean-context agent,
[comment](https://github.com/coder/coder/pull/30272#issuecomment-5944285083)
on `618bff67c75`: **READY**, no blockers and nothing to cut. It
independently checked authorization, audit, the migration, chatd row
insertion and subagent inheritance, the tests, and soft-deleted
organization handling, and agreed with every deferral. No code changes
followed: the snapshot behavior it suggested documenting is already
documented, and its docs wording note joins the deferred docs item.
6. Codex,
[comment](https://github.com/coder/coder/pull/30272#issuecomment-5952100918)
on `b4e76543df2` (the docs-only follow-up below): no major issues. Mike
asked for a passing Codex review loop after item 5, so this review goes
past the planned stop at the final advisory. No code changes followed.
7. ethanndickson,
[review](https://github.com/coder/coder/pull/30272#pullrequestreview-5409332502)
on `3850fbaa922`: approved with 4 comments. Fixed 3: the audit target is
now empty (`91e77d3fa8c`), the docs no longer re-explain audit logs
(`a0fb6891af5`), and the organization instructions stories went from 9
to 5 (`e836097f93f`). Not fixed: the organization prompt read fails open
on a database error, like the deployment prompt, and ethanndickson is
changing both after merge
([thread](https://github.com/coder/coder/pull/30272#discussion_r4180191267)).
UAT did not rerun for these changes.
Not counted as reviews: the pre-PR frontend self-review, simplify pass,
and test audit (`de8377e94e1`), and a second test audit of
`de8377e94e1..cc73b714630` (pass with nits; the update-only test case
was added after it).
Docs-only follow-up after the advisory, requested by Mike: `b4e76543df2`
adds audit log guidance for prompt changes to the platform controls page
and an update-only custom role note to the organizations page. Codex
reviewed it (item 6). No UAT covers it, and the code is unchanged since
`618bff67c75`. The deferred claim that the Instructions page path is
outdated was wrong, so it's dropped. Item 7 removed the audit log
guidance again; the custom role note stays.
## Soft-deleted organizations
UAT found that a soft-deleted organization's prompt stays readable and
writable by UUID. This is inherited behavior, not new exposure:
`ExtractOrganizationParam` resolves UUIDs without a deleted filter, so
the sibling organization chat routes (MCP servers, model overrides,
models) behave the same. The same `chat_model_config` RBAC still
applies, nobody gains access to another organization, and writes are
audited.
## Deferred (owner: Mike)
- A failed refresh right after a successful save shows the previous
value with an error alert, so a further edit from that view starts from
the old text (Codex round 4, UAT round 2). Trigger: reports of stale
values after saving on unreliable networks.
- Update-only custom roles can save over the API, but the card stays
hidden without read access. Trigger: a customer uses such a role.
- Soft-deleted organization access by UUID, above. Trigger: the platform
starts rejecting deleted organizations in `ExtractOrganizationParam` or
purging their configuration.
- A contrived "code word" instruction was refused as possible prompt
injection; neutral instructions work. Trigger: reports of ignored
organization instructions.
- UX polish: there is no retry action after a failed first load, the
save toast can cover Cancel, there is no unsaved-changes guard, the
oversize error omits the limit, and whitespace-only text clears the
prompt without notice. Trigger: the next settings UX pass.
- Docs: the 413 response and organization name paths are undocumented.
Trigger: the next Coder Agents docs pass.
- Test audit nits: a redundant `waitFor` and an optional read-only plus
refetch story. Trigger: the next change to these tests.
> Xum, an AI coding agent, implemented, tested, and opened this PR on
behalf of @ibetitsmike.
13 lines
425 B
SQL
13 lines
425 B
SQL
-- name: GetChatOrganizationSystemPrompt :one
|
|
SELECT *
|
|
FROM chat_organization_system_prompts
|
|
WHERE organization_id = @organization_id;
|
|
|
|
-- name: UpsertChatOrganizationSystemPrompt :one
|
|
INSERT INTO chat_organization_system_prompts (organization_id, system_prompt)
|
|
VALUES (@organization_id, @system_prompt)
|
|
ON CONFLICT (organization_id) DO UPDATE
|
|
SET system_prompt = EXCLUDED.system_prompt,
|
|
updated_at = now()
|
|
RETURNING *;
|