mirror of
https://github.com/coder/coder.git
synced 2026-11-01 04:47:17 +08:00
d44aae37de6e431f2992d0500c3f94d3aaf77eb8
4435
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jaayden Halko
|
d44aae37de |
refactor(coderd/x/chatd): return PersistedStep directly from ExecuteLocalTools (#28359)
**Stack**: **#28359 (this PR)** → #28360 → #28361 → #28362 `chatloop.ToolExecutionOutcome` only wrapped a `PersistedStep`, so callers had to reach through `outcome.Step` for every field. `ExecuteLocalTools` now returns the `PersistedStep` directly and the wrapper type is deleted. No behavior change. First of a four-PR stack that makes local tool execution count toward Coder Agent runtime (splitting #28211). This refactor keeps the rename noise out of the feature PRs. Refs CODAGT-928 (https://linear.app/codercom/issue/CODAGT-928/track-local-tool-execution-for-agent-runtime). |
||
|
Jake Howell
|
95fb22c82a |
test(coderd): fix flaky TestExpireAPIKey timing assertions (#28455)
> 🤖 This PR was written by Coder Agents on behalf of Jake Howell. Closes [DEVEX-730](https://linear.app/codercom/issue/DEVEX-730/flake-testexpireapikey-multiple-variants). ## Problem `TestExpireAPIKey` flakes on Windows CI (`test-go-pg (windows-2022)`), failing `AdminCanExpireOtherUsersToken`, `DeletingExpiredTokenSucceeds`, and the parent test. The subtests asserted: ```go require.True(t, key.ExpiresAt.Before(dbtime.Now())) ``` The `expireAPIKey` handler sets `ExpiresAt: dbtime.Now()`, and `dbtime.Now()` rounds to microsecond precision. On platforms with coarse clock resolution (Windows, ~1–15ms), the handler's timestamp and the test's later `dbtime.Now()` can round to the *same* value, so `Before()` is `false` (equal is not before) and the assertion fails. On Linux the nanosecond-resolution clock makes the two reads differ, which is why it never reproduces there. ## Fix Use the timing-safe assertion already used by the sibling `ExpiringAlreadyExpiredTokenSucceeds` subtest: ```go require.LessOrEqual(t, key.ExpiresAt, dbtime.Now(), "key should be expired") ``` Applied to all three occurrences of the flaky pattern (`OwnerCanExpireOwnToken`, `AdminCanExpireOtherUsersToken`, `DeletingExpiredTokenSucceeds`). ## Prior art This is the established fix for this class of flake in this repo; `require.LessOrEqual(x, now)` is equivalent to `require.False(x.After(now))` (both allow equality, which is the point): - **[#22685](https://github.com/coder/coder/pull/22685)** — *use dbtime.Now() instead of time.Now() in test assertions against DB timestamps*. States the principle: `time.Now()` is nanosecond precision while Postgres timestamps are microsecond, so `Before`/`After` comparisons carry non-zero flake risk. That sweep **edited this same file** (`coderd/apikey_test.go`, 11 `ExpiresAt` comparisons) plus 10 others; these three assertions pre-dated it. - **[#22684](https://github.com/coder/coder/pull/22684)** — *fix TestTokens harder*. The direct predecessor ("Follows from #22684"), fixing the identical token-expiry flake with the equality-tolerant `require.False(t, token.ExpiresAt.After(dbtime.Now()))`. - **[#21783](https://github.com/coder/coder/pull/21783)** — introduced the sibling `ExpiringAlreadyExpiredTokenSucceeds` subtest in this file, which already uses `require.LessOrEqual(t, key.ExpiresAt, dbtime.Now(), "key should be expired")`. This PR just makes the other three subtests consistent with it. ## Verification - Ran the original `TestExpireAPIKey` 100× against Postgres on Linux: 100/100 passed (as expected, the flake is Windows-specific). - Confirmed the root cause by reading the handler and `dbtime.Now()` (microsecond rounding). - Re-ran the test after the fix; passes. <details> <summary>Investigation notes</summary> - CI reference: https://github.com/coder/coder/actions/runs/29892773275 (job `test-go-pg (windows-2022)`). - Error snippet showed `key.ExpiresAt.Before(dbtime.Now())` expected true but observed false (token appeared unexpired). - Handler: `coderd/apikey.go` `expireAPIKey` sets `ExpiresAt: dbtime.Now()`. - `dbtime.Now()` returns `time.Now().UTC()` rounded to `time.Microsecond`. - The equal-timestamp collision only manifests where the OS clock resolution is coarse (Windows), which matches the CI-only reproduction. </details> |
||
|
Jon Ayers
|
26de6140fb | feat: add flag to disable workspace agent context sync (#28522) | ||
|
Kyle Carberry
|
cf97b56479 |
fix(coderd/x/chatd): refresh context after workspace restart (#28441)
Workspace lifecycle tools clear a chat's agent binding while retaining its pinned workspace context. When the chat binds to the replacement agent, the missing previous agent ID caused chatd to skip re-pinning and continue using stale instructions, skills, and MCP definitions. Re-pin when an unbound chat still carries pinned context, while preserving the existing first-time hydration path for new chats without a pin. Adds regression coverage for the stop/start binding sequence. ### CI note All required checks pass. The external `Pixel / Review` status reports the same 3 pre-existing failures on this PR's merge base (`610f321d52c16d43e73c489aa672ef60a92c20ea`) and recent `main` commits. This backend-only change does not touch `site/` or Storybook snapshots. --- Generated by Coder Agents on behalf of @kylecarbs. |
||
|
Hank Hwang
|
2f50b2d740 |
fix(coderd): reject agent requests from suspended owners (#28513)
## What `ExtractWorkspaceAgentAndLatestBuild` computed the workspace owner's RBAC subject via `UserRBACSubject`, but discarded the returned user status. As a result, workspace agent requests were still authorized even when the owner's account was suspended. This change captures `userStatus` and rejects the request with `401 Unauthorized` when the owner is not `Active`, returning a clear message directing the user to contact an admin to reactivate the account. ## Changes - `coderd/httpmw/workspaceagent.go`: stop discarding the owner status from `UserRBACSubject`; return `401` when the owner is not active. - `coderd/httpmw/workspaceagent_test.go`: add an `InactiveUser` test that suspends the owner and asserts a `401` with the expected message. ## Testing - New `TestWorkspaceAgent/InactiveUser` unit test covers the suspended-owner path. --- > This PR was created by Coder Agents on behalf of @hwang251. |
||
|
Bobby HoandMcKayla はな
|
c029869a04 |
fix(coderd): deflake TestHealthSettings dismissal subtests (#28501)
Fixes
https://linear.app/codercom/issue/PLAT-514/flake-testhealthsettingsdismisssection
Both TestHealthSettings dismissal subtests built their server with
coderdtest.New(t, nil), so GET /api/v2/debug/health ran the real
healthcheck, DERP probe included, only to assert that dismissed sections
come back as dismissed.
- Raising the timeout would not fix it. The healthcheck runs on a
context detached from the request (30s HealthcheckTimeout), and
derphealth's 10s per-node cap is skipped whenever a deadline already
exists, which is always true from that call site. Both subtests share
one 10s WaitShort across setup and four round trips, so the server may
spend 3x the test's entire lifetime on one request.
- Fix: inject a stub HealthcheckFunc in both subtests, as the six
TestDebugHealth subtests already do.
- The stub reports nothing dismissed, so the assertions still prove the
flags are applied per request in formatHealthcheck rather than read from
the cached report, which is what
|
||
|
Michael Suchacz
|
b6d7653231 |
fix: apply MCP server selection when editing a chat message (#28471)
## Problem
Editing a user message in an Agent chat silently dropped the MCP server
selection. The composer renders the MCP picker in edit mode and toggles
update local client state, so the picker displayed the new selection,
but the edit request omitted `mcp_server_ids` at every layer (frontend
request builder, `codersdk.EditChatMessageRequest`, the `PATCH
/chats/{chat}/messages/{message}` handler, and `chatd.EditMessage`). The
chat's persisted selection never changed and the regenerated turn ran
without the newly enabled MCP tools. Two dogfood users hit this within
hours; there is no error anywhere and the UI shows the opposite of the
server state.
## Changes
- `codersdk`: add `MCPServerIDs *[]uuid.UUID` to
`EditChatMessageRequest`, mirroring `CreateChatMessageRequest` (nil
preserves the current selection).
- `chatd`: extract the send path's MCP update block into one shared
`applyRequestedMCPServerIDs` helper (explore-subagent snapshot
immutability guard plus Force On enforcement, Cure53 CDM-02-010) and
call it from both `SendMessage` and `EditMessage`, so enforcement cannot
drift between the two paths.
- `coderd`: extract the send handler's request validation (dedupe,
enabled-in-organization check, persisted-ID exemption) into
`normalizeRequestedChatMCPServerIDs` and wire it into the edit handler,
which now threads the selection into `EditMessageOptions`.
- Frontend: the edit request now includes `mcp_server_ids:
[...effectiveMCPServerIds]`, exactly like the send path, making the
picker's displayed state real.
- `make gen` artifacts (swagger, API docs, `typesGenerated.ts`).
## Tests
Each layer is covered and was proven with independent red toggles
(removing one layer's wiring fails only that layer's tests):
- chatd (`TestEditMessage_MCPServerIDs`): edit applies a provided
selection, nil preserves it, an emptied list cannot remove a `force_on`
server, and explore subagent chats keep the spawn-time snapshot.
- API (`TestPatchChatMessage/MCPServerIDsApplied`,
`MCPServerIDsInvalidRejected`): persistence via the endpoint, omission
preserves, unknown IDs get the same 400 as the send path.
- Storybook (`EditAppliesMCPServerSelection`): toggling a server on
during an edit puts it in the edit request payload.
Note: the `AgentChatPage.stories.tsx` story "Queued For Capacity After
Polling" fails locally on current main as well (verified against the
main baseline with this branch's changes reverted); it is unrelated to
this diff.
Remote dogfood UAT ran on the exact head and passed, including proof
that the regenerated turn actually gains the newly enabled MCP server's
tools.
> 🤖 Xum acted on Mike's (@ibetitsmike) behalf. • Model:
`anthropic:claude-fable-5`
<!-- xum-attribution: model=anthropic:claude-fable-5 -->
|
||
|
Michael Suchacz
|
a48aedd924 |
fix(coderd/x): surface Gemini malformed-function-call stream deaths as retryable errors (#28470)
## Problem
When Gemini's OpenAI-compatible endpoint rejects a model-generated
function call server-side, it ends the SSE stream cleanly with the
nonstandard finish reason `function_call_filter:
MALFORMED_FUNCTION_CALL` after streaming only thought summaries; the
rejected call never reaches the wire. chatd treated this as a normal
completion: fantasy maps the unrecognized finish reason to `unknown`,
the reasoning block never closes (the only non-thought delta is the
`</thought>` marker, which the transport seam strips to an empty string,
and the openaicompat hook only ends reasoning on a non-empty content
delta), so the step accumulates no content and the generation loop
finishes the turn as complete. The user sees the model think for ~40
seconds and then nothing: no assistant message, no `last_error`, chat
status `waiting`. Observed twice in a row in production on
gemini-3.7-flash, with "Resume" reproducing it identically.
## Fix
Two independent layers:
- `coderd/x/googleopenai`: the stream rewrite now converts any chunk
whose `finish_reason` starts with `function_call_filter` into an
OpenAI-style SSE `{"error": ...}` event embedding the raw reason.
openai-go turns error-bearing events into stream errors, so the failure
rides the existing stream-error path instead of ending the stream
cleanly.
- `coderd/x/chatd/chaterror`: classifies that injected error as
retryable (kind `generic`, provider `google`) with a clear user-facing
message, so the existing generation retry machinery re-runs the step and
persists a `last_error` if retries exhaust.
- `coderd/x/chatd/chatloop`: provider-agnostic guard: a step that
produced no user-visible content and no tool calls under a finish reason
of `unknown`, `error`, `other`, or `tool-calls` (a tool-calls finish
that delivered zero calls) now returns a retryable error instead of
silently completing the turn. `stop` and `length` finishes keep their
existing semantics.
Tests cover the seam rewrite (live-capture SSE shape plus standard
finish reason passthrough), the new classification, and the chatloop
guard (error cases plus preserved stop, length, text, and tool-call
behavior). Each layer was red-green verified independently.
Remote dogfood UAT ran against this exact commit: normal reasoning and
tool-call chats on a real model complete cleanly with no spurious guard
errors and no retry loops. The Google-side failure itself is not
deterministically triggerable against live Gemini and is owned by the
unit tests.
> Xum acted on Mike's (@ibetitsmike) behalf.
<!-- xum-attribution: model=claude-opus, thinking=enabled -->
|
||
|
Spike Curtis
|
6a58d50f1d |
feat(coderd): add queue-position-free provisioner job query (#28367)
Follow-up to #28302 ([GRU-82](https://linear.app/codercom/issue/GRU-82), [RFC](https://app.notion.com/p/coderhq/Lite-Workspace-API-Requests-3aad579be5928060b5e7cc65c539717f)). That PR added related-data selection but noted that selecting `latest_build.job` still ran the expensive `GetProvisionerJobsByIDsWithQueuePosition` query, which computes queue position and size with window functions over pending jobs and provisioner daemons and is consistently the top resource consumer in scale tests. This PR adds a cheaper `GetProvisionerJobsByIDs` query that fetches jobs by ID without the queue-position computation, and selects between the two based on whether `latest_build.job.queue_position` is requested. When the queue position is not selected, `QueuePosition` and `QueueSize` are left zero and the rest of the response is unchanged. Both paths go through a single `provisionerJobsByIDs` API method that takes the `jobRelated` selection, switches on it, and reshapes the cheaper query's rows into `GetProvisionerJobsByIDsWithQueuePositionRow` so downstream conversion is uniform. The new query has the same dbauthz properties as the queue-position variant (system-level pass-through pending the proper provisioner-job RBAC check tracked in #16160), with a matching authorization test. No behavior change for existing callers: they select `queue_position` (via `allLatestBuildRelated`), so they continue to use the queue-position query. --- *Generated by Coder Agents on behalf of @spikecurtis.* |
||
|
Cian Johnston
|
042421694f |
fix: surface duplicate AI provider hostname as warning, not error (#28208)
Two live `ai_providers` rows can share the same `base_url` hostname.
This is an intended configuration (AIGOV-97): the direct routing path
(`/api/v2/ai-gateway/{name}/...`) routes by provider name and fully
supports it. Only the MITM proxy path (`aibridgeproxyd`) cannot
disambiguate, because its `nameByHost` map is 1:1.
`classifyProviderRow` marked the second provider as
`ProviderStatusError`, which surfaced as a misleading log warning and a
`status=error` Prometheus label. The provider is not actually broken; it
is fully routable via the direct path. The error was only visible in
server logs and Prometheus, not in the API or UI.
This change:
- Extracts `BaseURLHostname` into `coderd/aibridged` and eliminates
three duplicate implementations (proxy classifier, chatprovider, and the
API layer now uses it for status checks).
- Downgrades the proxy classifier from `ProviderStatusError` to a new
`ProviderStatusProxyExcluded` status. The provider is not broken; it is
just not reachable via the MITM proxy path.
- Adds `Status *AIProviderStatus` to `codersdk.AIProvider` with
`Warnings []string`, matching the `WorkspaceProxy` precedent.
- Populates warnings on create, update, GET, and list when a provider's
hostname collides with another enabled provider.
- Adds an integration test proving that a duplicate provider is routable
via the direct path but not via the proxy path.
Refs AIGOV-596
Generated by Coder Agents
https://github.com/user-attachments/assets/bf323837-5121-4d1b-ab19-593f1d32374e
|
||
|
Ethan
|
c275327fb7 |
feat: audit operational agent settings (#28369)
Depends on #28394. Audit changes to seven operational agent settings through a typed `chat_operational_settings` resource. Each handler serializes baseline capture and mutation with a per-setting advisory lock, keeps both operations in one transaction, and records field-specific diffs. Effective no-op requests now skip both the database upsert and the success audit, including absent rows whose effective value is already the default. The change also retains the allowlisted raw `site_configs` query, updates generated audit and API surfaces, adds failure and concurrency coverage, and reuses the existing AuditPage resource-filter story pattern. Relates to CODAGT-720. |
||
|
Ethan
|
2aba2685ad |
fix: require audit capture for chat settings (#28394)
Existing chat instruction setting handlers could complete a mutation without an audit event when advisory locking or old-state capture failed. Make system prompt and plan mode instruction updates fail closed: baseline capture and mutation stay in one transaction, audit infrastructure failures reject the request without changing the setting, and focused tests cover lock, read, transaction, and write failures. |
||
|
Michael SuchaczandEthan Dickson
|
0ef331fcd0 |
feat!: scope chat model override settings to organizations (#28442)
Moves chat model override settings from deployment scope to organization scope, following the org-scoping of chat model configs in #28440. Depends on #28440 (based on `ethan/chat-models-availability-consolidation`). Remote dogfood UAT passed on this change, including multi-organization scenarios on a licensed deployment with real models. ## Problem Chat model configs are now owned by organizations, but the override settings that reference them stayed deployment-wide: admin subagent/title/compaction overrides in `site_configs`, personal overrides in `user_configs`, and the advisor model embedded in the advisor runtime JSON. A single deployment-level override can only point at one organization's model config, so overrides effectively worked for the default organization only, and title generation had to soften its hard-failure contract to tolerate unusable overrides. ## Fix Admin overrides (general, explore, title generation, compaction, advisor) and personal overrides (root, general, explore) become organization-scoped rows in two new typed tables, `chat_organization_model_overrides` and `chat_user_model_overrides`. Composite foreign keys on `(organization_id, model_config_id)` make cross-organization model references unrepresentable. The migration deletes the legacy serialized `site_configs`/`user_configs` override keys instead of migrating them; admins and users re-select models in the new organization-scoped settings. New endpoints live under `/api/experimental/organizations/{organization}/chats/model-overrides` (admin defaults) and `/api/experimental/organizations/{organization}/members/{user}/chats/model-overrides` (personal overrides); the legacy deployment-level routes are removed. chatd resolves overrides via the chat's organization, restoring the hard-failure contract for configured-but-unusable title generation overrides. The deployment-level `AllowUsers` switch and advisor runtime limits stay deployment-scoped. The Coder Agents settings UI gains a per-organization "Defaults & overrides" page under AI Settings > Models, and the personal overrides page gains an organization selector for users in multiple organizations. Advisor telemetry now reports per-organization override rows. **BREAKING CHANGE**: Existing deployment-level overrides are dropped, not migrated. Every organization starts with no overrides after upgrade, and model selections must be re-applied in the new settings. The legacy deployment-level override endpoints are removed. Closes https://linear.app/coder/issue/CODAGT-872 > [!NOTE] > Xum (AI agent) authored this PR on behalf of @ibetitsmike. <!-- xum-attribution: model=claude-opus-4-6 thinking=high --> --------- Co-authored-by: Ethan Dickson <ethan@coder.com> |
||
|
Ethan
|
3809314b31 |
refactor!: consolidate chat model availability (#28440)
## Summary Consolidate chat model management and runtime availability into a single organization model collection contract. The organization model response now includes provider availability, unavailable reasons, and unsupported-provider details, so callers no longer need a separate availability request. Depends on #28439. ## Problem The organization-scoped API exposes two overlapping representations of the same chat models: ```text GET /api/experimental/organizations/{organization}/chats/models GET /api/experimental/organizations/{organization}/chats/models/available ``` The collection route returns organization-owned model configurations and redacted provider descriptors. The availability route returns a second catalog-oriented response with provider availability and synthetic catalog entries. Callers must fetch, reconcile, and cache both responses even though they describe the same effective set of models. This creates duplicated server logic and separate SDK, OpenAPI, and frontend types. It can also produce inconsistent client state when one request succeeds while the other fails or when the two responses are refreshed at different times. ## Fix Return all model-management and runtime-availability information from the organization model collection route: ```text GET /api/experimental/organizations/{organization}/chats/models ``` `OrganizationChatModelsResponse` continues to return the caller-readable organization model configurations and redacted provider descriptors. Provider descriptors now also include: - `available` - `unavailable_reason` The response additionally includes `unsupported_providers`, allowing clients to explain configurations that the Agents harness cannot use. Frontend model settings, agent creation, existing chat recovery, and model override surfaces now consume this single collection query. This keeps model ownership, caller visibility, provider availability, and unsupported-provider guidance in one cache entry and one authorization path. The default-organization compatibility route remains available: ```text GET /api/experimental/chats/models ``` It now returns the same consolidated `OrganizationChatModelsResponse` contract instead of the former availability response. ## Breaking change This removes the separate organization model availability endpoint: ```text GET /api/experimental/organizations/{organization}/chats/models/available ``` Clients using that endpoint must migrate to: ```text GET /api/experimental/organizations/{organization}/chats/models ``` This PR also removes the corresponding public experimental client and schema surface: - `ExperimentalClient.ChatModelAvailability` - `ChatModelAvailabilityResponse` - `ChatModelProvider` - `ChatModelCatalogEntry` - The generated OpenAPI schema and frontend API/query types for the availability endpoint Callers must use `ExperimentalClient.ChatModels` and `OrganizationChatModelsResponse` instead. Availability is now reported on each `ChatModelProviderDescriptor`, and unsupported providers are returned in `OrganizationChatModelsResponse.UnsupportedProviders`. The default-organization compatibility route retains its URL, but its response schema changes from `ChatModelAvailabilityResponse` to `OrganizationChatModelsResponse`. Direct HTTP clients and generated clients that decode the previous response type must be updated even if they do not use the removed organization-scoped `/available` route. |
||
|
Ethan
|
a53c6e6366 |
feat: audit chat model config changes (#28439)
Adds audit logging for chat model config creates, updates, deletes, ACL changes, and automatic default-model transitions. Audit records use the config display name or ID as the target and are emitted only after successful transactions. Depends on #27960. |
||
|
Ethan
|
aefc761fcf |
feat!: enforce organization scope for chat models (#27959)
## Summary Move chat model management and runtime availability to organization-scoped API routes. The API now identifies the organization in every model-management operation, applies chat model RBAC and ACLs within that organization, and returns only models the caller can use there. Depends on #27958. ## Problem Chat model configurations now belong to organizations, but the existing experimental management and discovery APIs are deployment-shaped. Their routes do not identify an organization, SDK methods do not accept an organization ID, and item operations address a model configuration by ID alone. That contract cannot safely expose organization-owned model configurations. It also does not provide the organization-specific provider information, authorization behavior, ACL management, or recovery guidance needed by the settings and chat clients later in the stack. ## Fix Add organization-scoped chat model endpoints under: ```text /api/experimental/organizations/{organization}/chats/models ``` The new API provides: - Organization-scoped model listing and creation. - Model get, update, and delete operations scoped by both organization and model ID. - Model ACL read and update operations. - Organization-scoped runtime availability and provider status. - Redacted provider descriptors for model-management clients. - Chat model RBAC and row ACL enforcement for writes, runtime discovery, and Chatd model selection. - Recovery guidance when a chat's organization has no usable local model. Historical chats can retain a model reference owned by another organization, but runtime selection uses an authorized local default when one is available. Foreign, inaccessible, disabled, and otherwise unusable models are not exposed as usable selections for the organization. The existing collection and availability routes remain as default-organization compatibility endpoints in this layer: ```text GET /api/experimental/chats/model-configs POST /api/experimental/chats/model-configs GET /api/experimental/chats/models ``` These compatibility routes do not provide access to models in non-default organizations. The previous unscoped item update and delete routes are removed because item operations must include the owning organization. ## Breaking change This changes the experimental HTTP API and Go SDK contracts for chat model management. Clients must migrate model-management and availability requests to the organization-scoped routes and provide an organization identifier. In the Go SDK, the following methods now require an organization ID: - `ChatModelAvailability` - `ChatModels` - `CreateChatModel` - `UpdateChatModel` - `DeleteChatModel` `ChatModels` also changes its return value from `[]ChatModel` to `OrganizationChatModelsResponse`, which includes the organization models and the redacted provider descriptors required by management clients. New item and ACL methods likewise require both the organization ID and model ID. The default-organization compatibility routes reduce the immediate impact for collection and availability callers, but they do not preserve the previous item API or SDK method signatures. Generated clients and direct consumers of the experimental API must be updated before adopting this change. _This pull request description was generated by Coder Agents._ |
||
|
Ethan
|
9bd8899359 |
refactor: clarify chat model resource names (#27958)
Rename the admin-managed SDK resource from `ChatModelConfig` to `ChatModel`. Rename catalog entries from `ChatModel` to `ChatModelCatalogEntry` and replace `ChatModelsResponse` with `ChatModelAvailabilityResponse`. API routes and JSON fields do not change. Experimental SDK and OpenAPI schema names do change. Depends on #27957 _This pull request description was generated by Coder Agents._ |
||
|
Ethan
|
cbe24a3da5 |
feat: add chat model config RBAC resource (#27957)
Add the `chat_model_config` RBAC resource, actions, and API key scopes. This layer includes the scope migration and generated API types. Model list queries apply token scopes, SQL authorization filters, and row ACLs. Write and default-management handlers keep deployment configuration authorization until the next API layer replaces it. Depends on #27956 _This pull request description was generated by Coder Agents._ |
||
|
Ethan
|
ac41b5b4f9 |
feat(coderd): include chat model organization IDs in telemetry (#27956)
Include the owning organization ID for each chat model configuration in telemetry snapshots. Depends on #27955 _This pull request description was generated by Coder Agents._ |
||
|
Ethan
|
f32592ec6b |
feat!: scope chat model configs to organizations (#27955)
## Summary Scope chat model configurations and runtime model resolution to organizations. Each model configuration now belongs to exactly one organization, each organization can have its own default model, and Chatd only uses models that belong to the chat's organization. Depends on #27968. ## Problem Chat model configurations are currently deployment-wide even though chats belong to organizations. A default model, explicitly selected model, or configured model override can therefore reference the same model configuration from every organization. This prevents model catalogs and defaults from being isolated by organization and allows a chat to retain or select a model that its organization does not own. Existing model configurations also have no organization ownership or organization-scoped ACL data, and the database enforces a single live default across the entire deployment. ## Fix Add required organization ownership and ACL storage to chat model configurations. The migration assigns every existing configuration to the default organization, seeds organization-wide read access, and replaces the deployment-wide default constraint with a per-organization constraint. Scope Chatd lookup, admission, fallback, and queue-promotion behavior to the chat's organization: - New chats use the usable default model from their organization. - Explicit model selections from another organization are rejected. - Deployment or personal overrides that point to a foreign model are not used for the chat. - Historical cross-organization model references remain unchanged in the database, but runtime generation falls back to a usable local default. - Queue promotion resolves unavailable, disabled, missing, or foreign stored models to the usable local default and fails when no usable local default exists. The existing model configuration management and availability routes remain compatibility routes for the default organization in this layer. A later layer in the stack adds organization-scoped management routes and updates clients to use them. ## Breaking change This changes chat model behavior from deployment-wide to organization-scoped. Existing configurations are migrated only to the default organization; they are not copied into every organization. As a result: - A non-default organization no longer inherits the deployment's existing default model. - Creating a chat in an organization without a usable local default fails with `400: No default chat model config is configured.` - Explicitly selecting a configuration owned by another organization fails with `400: Model config does not belong to the specified organization.` - Existing chats whose stored model belongs to another organization can generate only if a usable local default is available; otherwise generation fails. Deployments using Agents in non-default organizations must configure a usable default chat model separately for each organization after the organization-scoped management layers in this stack are available. _This pull request description was generated by Coder Agents._ |
||
|
Ethan
|
cd1362abfd |
fix(coderd): prevent stale chat model config writes (#27968)
Chat model updates could merge PATCH fields from a row read before the serialized transaction. Deletes could also succeed after another request deleted the row. Merge updates from the row read after the write lock. Make soft deletes report missing rows, and validate provider availability and model compatibility in the same transaction. This PR also removes `DeleteChatModelConfigsByAIProviderID`. The query has no production callers, including the AI provider deletion path, and only remained in generated database wrappers and mocks. Its removal is intentional dead-code cleanup and does not change provider deletion behavior. _This pull request description was generated by Coder Agents._ |
||
|
McKayla はな
|
5ca0143992 | refactor: upgrade golangci-lint to v2.0.2 (#28416) | ||
|
david-fraley
|
cb52f5339d | feat: show a premium paywall on the external auth settings page (#28435) | ||
|
david-fraley
|
a2afb458c5 |
feat: add premium paywall conversion telemetry (#28425)
**TLDR:** When someone using free Coder taps a "Start trial for free" button, we now write down which page they tapped it on, and whether they went on to actually start the trial. That tells us which locked features make people want Premium. ## Brief summary - Records two things: the button being clicked, and a trial actually being started. - Each click gets a random ID that travels with the person to the trial form, so we can tell who finished from who walked away. - Records a short label for the page (like `appearance` or `audit_log`), never the web address, so no customer names are collected. #28226 added the trial form and has since merged, so this now targets `main`. ## Description Today the "Start trial for free" button just moves you to the Premium page and nothing is recorded, so there is no way to know which feature convinced someone. This adds two events: | Event | Happens when | |---|---| | `cta_click` | Someone clicks "Start trial for free" | | `trial_signup` | A trial licence is actually issued | The click generates a random ID. It rides along to the trial form and gets saved on the signup, so a click and a signup can be matched up later. Clicks with no matching signup are the people who gave up. Trials started without a paywall (for example straight from the sidebar) are labelled `direct`, so "no label" never gets confused with "broken". A few small choices worth knowing: - The ID is kept in browser session storage, not in the web address, so the URL stays clean. It expires after 30 minutes. - The trial signup event is recorded by the server, not the browser, so nobody can fake a signup. - The browser cannot say which kind of event it is sending. It can only report a click; the server labels it. - Only people allowed to start a trial are counted. The server rejects the rest, which matters because a few paywalls show their button to everyone. - Nothing is recorded at all when telemetry is turned off. There is deliberately no "paywall was shown" event. Every admin page visit would report one, which is a lot of noise for little insight. Every paywall in the [Premium Copy Updates](https://www.notion.so/coderhq/Premium-Copy-Updates-3c1d579be59280a59557cabd54fd8a1f) table has a label. The rows that do not are badges and alerts with no button to click. ## Proof Ran a local Coder with no licence and clicked the paywall button on the Appearance page. What the browser sent, and the reply: ```json POST /api/v2/deployment/premium-funnel-events -> 204 No Content {"id":"92d96afa-f975-4789-b8a7-b1fd19d4df85","source":"appearance","variant":"premium"} ``` What the browser saved for the trial form, same ID: ```json {"id":"92d96afa-f975-4789-b8a7-b1fd19d4df85","source":"appearance","createdAt":1787268520857} ``` The click then landed on the Premium page with the trial form. Screenshots of the network panel, the saved value, and the Premium page are attached below. The form itself was not submitted, since that would contact the real licence server. ## Testing - Backend tests: a click is recorded with the server's own label, an unknown page label is refused, and a normal member is refused. - Storybook tests: button clicked and ID saved on all three paywall styles, plus the case where the viewer only sees guidance text and has nothing to click. - Unit tests for the saved ID: expiry, bad data, unknown page label, and the `direct` fallback. - `make gen` is clean, and lint, types, and formatting pass. Two stories fail on `main` today and fail the same way here: `ExternalAuthSettingsPageView > Page` and `PremiumPageView > No License`. Neither file is touched by this branch; both were verified failing on `main` with this branch checked out elsewhere. ## Follow-up coder/coder-telemetry-server#45 saves these events for reporting. It merges after this one. <details> <summary>Plan and decisions</summary> **Goal:** find out which Premium paywall drives trial signups, and tell abandons apart from completions. **Options considered and dropped:** 1. Put `?from=appearance` in the URL. Rejected: messy for users. 2. Use router state. Rejected: lost if the page is refreshed. 3. Let the browser report the signup. Rejected: it could be faked or lost during the redirect. 4. Work out the page from the URL. Rejected: URLs contain organization and template names. 5. Report every paywall impression. Rejected: one event per admin page visit is noise, and the click is the signal. 6. Separate event types and tables. Rejected: makes the matching query harder for no gain. **Event shape:** ```go type PremiumFunnelEvent struct { ID uuid.UUID // event ID; for a click this is also the matching ID EventType string // cta_click | trial_signup, set by the server Source string // appearance, audit_log, ..., direct Variant string // which paywall design was shown AttributionID uuid.UUID // the click this signup came from UserID uuid.UUID CreatedAt time.Time } ``` **Where the code lives:** nothing under `components/` imports from `modules/`, so the paywall components stay presentational and only gain an `onCTAClick` prop. New wrappers in `modules/paywall/` own the reporting, one per paywall style, each taking a fixed `source`. **Self-review:** the `frontend-review` skill caught that reading permissions inside the wrapper forced an auth context into presentational page views, which broke their stories. Every call site already passes `canViewPremium={permissions.viewAllLicenses}` and the button only renders when that is true, so the wrapper no longer reads permissions and the server stays the gate. It also flagged one unattributed paywall on the Security page, now wired up as `browser_only`. </details> > Generated with [Coder Agents](https://coder.com/agents) on behalf of @david-fraley |
||
|
Samuel VolinandMatt Vollmer
|
f44a3b59d7 |
feat: Premium Page CTA updates and form handling (#28226)
* Premium page has form for signing up for a trial * Input validation and normalization for trial form fields * api endpoint definition to make this processes seamless * license page updates to access `?success=true` queryparam and confetti <img width="1512" height="833" alt="Screenshot 2026-08-17 at 10 08 41 PM" src="https://github.com/user-attachments/assets/1766e724-c63a-4da9-b2bd-a98e6b66cc9b" /> <img width="1510" height="874" alt="Aug-17-2026 22-12-38" src="https://github.com/user-attachments/assets/3a2a0f35-1176-4290-aa0d-b3f7d66c15d6" /> ```mermaid sequenceDiagram autonumber actor Owner participant UI as UI<br/>TrialRequestForm + PremiumPage participant API as API<br/>api.ts + coderd middleware participant Coderd as Coderd<br/>postTrialLicense + trialer participant Licensor as Licensor<br/>v2-licensor + Postgres + pubsub + cache Note over UI: mutation.status = "idle" Owner->>UI: fills 10 fields, checks acknowledgement UI->>UI: Yup validate, strip "acknowledged", submit UI->>API: POST /api/v2/licenses/trial<br/>cookie + X-CSRF-TOKEN Note over UI: mutation.status = "pending" API->>API: verify CSRF, apiRateLimiter,<br/>apiKeyMiddleware sets actor API->>Coderd: dispatch Coderd->>Coderd: audit.InitRequest, authorize license:create,<br/>httpapi.Read validates, HasLicense is false Coderd->>Licensor: POST trial request<br/>deployment_id unspoofable, 10s deadline Licensor-->>Coderd: 200, raw signed JWT Coderd->>Coderd: ParseClaimsIgnoreNbf, uuid.Parse Coderd->>Licensor: InsertLicense, updateEntitlements,<br/>publish PubsubEventLicense, commit audit log Licensor-->>Coderd: database.License Coderd-->>API: 201 codersdk.License API-->>UI: 201 codersdk.License Note over UI: mutation.status = "success" UI->>Licensor: invalidate entitlements + licenses keys UI->>Owner: navigate /deployment/premium?success=true Note over Owner: confetti fires on the success param ``` --------- Co-authored-by: Matt Vollmer <matthewjvollmer@outlook.com> |
||
|
Ehab Younes
|
903d7b756d |
feat: add workspace agent session count protocol (#28125)
Phase 2 of the Scalable Approach for Adding New IDE Session Types RFC, tracked in #27410. Adds a session_counts map to Stats on the Agent API, so an agent can report a count per app name instead of the four fixed fields, which are now marked deprecated. Bumps the API to v2.11. - coderd normalizes names on ingest, caps distinct entries, and sums the overflow under unknown, so an arbitrary client name cannot blow up cardinality. - Agents predating v2.11 set the fixed fields instead, never both, and the ingest path falls back to them. Nothing sends session_counts yet. The agent starts populating it in #28338. |
||
|
Michael Suchacz
|
9168dbf4d3 |
refactor(coderd/x/chatd): unify LLM model-call construction behind a resolver (#28145)
Centralizes every LLM client build in chatd behind one resolver and makes all model calls behave the same, instead of preserving per-flow quirks. ## Why `chatd` built LLM clients in five places (standard turn, compaction override, quickgen, title override, advisor), each repeating config parsing, route resolution, client construction, and debug wrapping with slightly different behavior. An earlier iteration of this PR centralized construction but preserved every historical quirk behind purpose-specific spec constructors and policy flags. Review concluded the quirks themselves were the problem. ## What changed - `Server.resolveModelCall` (`modelcall.go`) is the single pipeline from a `modelCallSpec` to a ready client plus call metadata. Callers build spec literals inline; the only spec fields are data: model source (chat config, explicit config row, or fixed provider/model pair for computer use), requested reasoning effort, chatd-scoped routing, and build options. - Uniform behavior for every call: - Provider options are always derived. Whole-chat summaries, turn status labels, and chat-model compaction summaries previously sent none; they now follow the config like everything else. - `MaxOutputTokens` always defaults to 32k when the config leaves it unset. The compaction summary call nulls the cap after copying the template because it is non-streaming and the Anthropic SDK rejects non-streaming requests whose cap implies a completion over ten minutes. - Debug recording always wraps the client when chat debug is enabled, labeled with the resolved provider/model. The debug-rebuild paths (`manualTitleDebugSpec`, `quickgenDebugSpec`) are deleted; debug helpers now only create and finalize run records, since run association flows through the generate context, not the model. - Clients are always constructed with the configured model string and unmutated route, so AI Gateway validation sees the name exactly as configured. The resolved identity feeds metadata, logs, and debug labels only. - `resolvedModelCall.newCall` / `newObjectCall` are the only production constructors of `fantasy.Call` / `fantasy.ObjectCall`; object calls now carry the config's sampling parameters. - Net: -240 lines. ## Validation - Full `./coderd/x/chatd/...` suite green (PostgreSQL-backed). - Red-green toggles verified for the three key unifications (option derivation, token default, summary cap removal). - ARCHITECTURE.md: prose the refactor made stale is deleted; no new resolver documentation is added. > Mux acted on Mike's behalf for this PR. |
||
|
Bobby Ho
|
6348c83142 |
feat: register public clients without a secret (#28046)
Third in the stack, split out of #27873. This is where dynamic client registration first produces a public client. - A registration requesting token_endpoint_auth_method `none` issues no secret and persists the client_type derived in #28043. The response omits client_secret entirely, per RFC 7591 §3.2.1. - Discovery still advertises only the two secret-based methods. `none` is withheld until the token endpoint honors it, so a conforming client is not told to attempt an exchange that would be rejected. - The app and its secret are written in one transaction. They were two independent inserts, so a failed second one left a committed app that could never authenticate while still holding a registration access token. Pre-existing, but a public client's missing secret row makes the orphaned case unspottable by inspection. - registration_client_uri uses url.JoinPath, fixing a trailing-slash access URL producing //oauth2/clients/{id}. - Docs state both limits above, so the page does not describe a flow that returns 400 until #28047 lands. A client registered here cannot obtain a token yet; #28047 adds that. Dynamic client registration is off by default, so none of this is user-visible until then. Refs https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client |
||
|
Yevhenii Shcherbina
|
211f490910 |
refactor: share the AI Gateway price book contract (#28310)
Follow-up to the review discussion on the price book refresh automation: https://github.com/coder/coder/pull/28146#discussion_r3806439966 ## Problem The price seed row struct was duplicated three times, so a JSON tag change had to be repeated in each copy: - `scripts/aibridgepricesgen.priceRow` - `scripts/aibridgepricesdiff.priceRow` - `coderd/aibridge/prices.seedRow` The `(provider, model)` key and its parsing were duplicated too. ## Change Add `coderd/aibridge/prices/pricebook` holding the shared contract: `Row`, `Key`, `Row.Key()`, and `Parse`. The package depends only on the standard library, so the scripts can import it without pulling in the database layer that `coderd/aibridge/prices` brings. Consumer-specific behaviour stays where it is: upstream conversion and validation in the generator, comparison and Markdown rendering in the diff tool, DB seeding and runtime lookup in the seeder. The batch SQL upsert still extracts JSON fields by name, so it remains a separate contract boundary. The note about keeping it in sync now lives on the shared `Row`. No generated artifacts change; the serialized schema is identical. --- <sub>Opened by Coder Agents on behalf of @evgeniy-scherbina.</sub> |
||
|
Bobby HoandClaude Opus 5
|
dea3a2944a |
feat(coderd/rbac): compare scopes by permission coverage (#28167)
Split out of #28045 ([PLAT-479](https://linear.app/codercom/issue/PLAT-479)) so the coverage rules can be reviewed on their own. First of three. No behavior change: both functions are added without production callers, and the OAuth2 authorize endpoint consumes them in the next PR. **ScopesCover(allowed, requested)** reports whether every permission the request grants is also granted by the allowlist. It expands both sides and compares permissions rather than names, so coder:workspaces.access covers workspace:read without ever naming it. The fail-closed rules are the part worth reviewing: - Coverage models site-level grants only. A scope carrying org or user permissions, a negative permission, or a narrowed allow list is refused on either side rather than compared on the part that is modeled. Answering "covered" from the fraction that was read would report authority that was never examined. - An unknown name on either side is an error, not a false, since a caller cannot tell "not covered" apart from "could not decide". - A wildcard request is covered only by a wildcard grant. Enumerating every workspace action that exists today does not cover workspace:*, because the wildcard also authorizes the actions added tomorrow. **CanonicalScopeName** maps the aliases IsExternalScope accepts, all and application_connect, onto the api_key_scope enum spellings. IsExternalScope answers whether a name may be requested, not how it is spelled once persisted, so anything storing a validated name has to canonicalize in between. Both sides of ScopesCover must already be canonical, which its parameter names restate at every call site. Covered by 17 coverage cases, guard cases that drive each unmodeled shape through both sides of the comparison, and an exhaustive sweep over the external catalog: coder:all covers all of it, and every catalog name covers itself. Stack: this PR, then negotiate and persist the scope, then the consent page and invalid_scope redirect. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Michael Suchacz
|
8eb1e43f0b |
feat: apply reasoning effort and surface thinking blocks for Google chat models (#28273)
## Problem
Selecting a reasoning effort on a Google (Gemini) chat model either got
dropped or broke the request outright, and even when the request
succeeded, Gemini chats never showed thinking blocks:
- The native `fantasygoogle` path had no case in `applyReasoningEffort`
at all, so effort was silently ignored. With only `include_thoughts`
configured, Gemini 3+ models default to no thinking, so chats produced
zero reasoning tokens.
- The path chat generation actually uses today routes Google providers
through aibridge as openai-compat clients and forwarded the raw effort
string as `reasoning_effort`. Google's OpenAI-compatible endpoint
validates that value against the model's supported thinking levels
instead of clamping, so out-of-range efforts fail the whole request with
HTTP 400. Verified against the live Gemini API: `gemini-3.1-pro-preview`
accepts low/medium/high and rejects none/minimal/xhigh/max. Because the
config's default effort is sent whenever the user selects nothing, a
config whose default falls outside the model's supported set failed
every no-selection message.
- Google's OpenAI-compatible endpoint never emits thought text for
`reasoning_effort` requests. Thoughts require
`extra_body.google.thinking_config` with `include_thoughts`, which is
mutually exclusive with `reasoning_effort`, and even then the thought
deltas come back inline in `content` (marked
`extra_content.google.thought`, wrapped in `<thought>` markers) rather
than in the `reasoning_content` field OpenAI-compatible clients parse.
So no reasoning parts were ever produced, persisted, or rendered for
Gemini chats (verified live in UAT).
## Fix
Reasoning effort:
- Map the global reasoning effort scale onto Google's `thinking_level`
in `applyReasoningEffort`, gated to Gemini 3+ models and clamped to each
model's supported subset (3.0 Pro launched with LOW/HIGH only, 3.1 Pro
added MEDIUM, Flash accepts all four through 3.6 while 3.7 and the
latest flash aliases drop MINIMAL, image models accept HIGH plus MINIMAL
for flash). Pre-Gemini-3 models reject `thinking_level`, so effort
degrades to a no-op for them and a config-pinned level is dropped at
call time instead of failing every generation.
- Add `thinking_level` to `ChatModelGoogleThinkingConfig` so admins can
pin a level for configs without user-selectable effort. A resolved
per-turn effort overrides the pinned level; an explicit
`thinking_budget` wins over both since the Google API rejects requests
that set `thinking_budget` and `thinking_level` together, and config
validation now rejects that combination up front. The admin panel picks
up the new field automatically through the generated model-options
schema.
Thinking blocks on the openai-compat path (the path chat generation
uses), translated in both directions at the existing Gemini transport
seam:
- chatd's OpenAI-compat request patch swaps `reasoning_effort` for an
equivalent `extra_body.google.thinking_config` with `include_thoughts`
enabled: `thinking_level` on Gemini 3+ (clamped to the model's supported
subset), `thinking_budget` on earlier models (effort `none` maps to
budget 0, which disables thinking). Non-Gemini models and non-Google
routes are untouched.
- The same transport rewrites Google's responses so thought output
surfaces as `reasoning_content`, which fantasy already converts into
reasoning parts: streaming bodies are rewritten per SSE line (preserving
streaming latency), JSON bodies in place, with the
`<thought>`/`</thought>` markers stripped at the transitions.
- aibridge's chat-completions interception preserves the `extra_body`
passthrough that openai-go's typed params silently drop, forwarding it
to Google upstreams only, so the chatd -> aibridge -> Google route
carries the thinking config end to end.
## Validation
- `go test ./coderd/x/chatd/... ./aibridge/... ./internal/googleopenai
./codersdk` pass; golangci-lint clean on touched packages.
- New unit tests cover the effort-to-level mapping for every effort
value, per-model clamping on both paths, the effort-to-thinking_config
request swap (level, budget, pinned-config precedence, non-Gemini
passthrough), SSE and JSON thought-response rewriting (fixtures mirror
live captures, including tool-call and no-thought streams), an
end-to-end fake-SSE test through the fantasy pipeline proving reasoning
deltas emerge with markers stripped, and aibridge `extra_body`
preservation (including proof the typed round trip drops it).
- Verified live against `generativelanguage.googleapis.com` on
`gemini-3-flash-preview` and `gemini-2.5-flash` through the real
chatprovider stack: reasoning deltas stream through with markers
stripped and answer text intact.
- Live UAT on a dogfood deployment (remote Coder Agents chat, 4 rounds):
effort clamping verified end to end (rounds 1-2), the missing-thoughts
limitation isolated (round 3), and the fix validated (round 4):
expandable, readable thinking blocks render for
`gemini-3.1-pro-preview`, `gemini-3-flash-preview`, and
`gemini-2.5-flash` at effort=high; no `<thought>` marker leaks;
effort=none produces no block and no 400; multi-turn history replay
works with a thinking block on each turn.
- `make gen` regenerated `typesGenerated.ts` and
`chatModelOptionsGenerated.json`; `pnpm -C site lint:types` passes.
> 🤖 Mux acted on Mike's behalf to create this pull request.
<!-- mux-attribution: model=claude-opus-4-6 thinking=high -->
|
||
|
Danielle Maywood
|
5480299cef |
fix: serialize empty MCP tool properties as {} not null for OpenAI (#28380)
## Summary
Fixed a bug where MCP tools with an empty `properties` schema had their
tool definitions sent to OpenAI with `"properties": null`, causing
OpenAI to reject the request.
## Problem
When an MCP server reports a tool with an empty input schema, e.g.:
```json
{
"type": "object",
"properties": {},
"required": []
}
```
Coder was forwarding the tool definition to OpenAI as:
```json
{
"type": "object",
"properties": null
}
```
OpenAI's function-calling API rejects `properties: null`, making these
tools unusable with OpenAI models.
## Root cause
The empty `properties` object collapsed to Go `nil` at two layers:
1. `agent/x/agentmcp/manager.go` `toolInputSchemaMap` stripped the
present-but-empty `"properties": {}` (guarded by `len(properties) > 0`),
so the wire copy lost it entirely.
2. `coderd/x/chatd/chatloop/chatloop.go` `buildToolDefinitions` embedded
`info.Parameters` directly into the input schema. When nil,
`encoding/json` marshals it as `null`.
## Fix
- `buildToolDefinitions` now substitutes an empty object when
`Parameters` is nil, mirroring the existing `required` guard (the sink,
protects all tool sources).
- `toolInputSchemaMap` now preserves an empty `properties` object
instead of dropping it (the upstream producer).
## Tests
Added regression tests (failing before, passing after) covering both
layers and asserting the schema serializes to
`{"type":"object","properties":{}}`:
- `TestBuildToolDefinitionsNilPropertiesBecomesEmptyObject`
- `TestToolInputSchemaMapPreservesEmptyProperties`
-
`TestWorkspaceMCPToolInfosFromResources/EmptyPropertiesYieldsEmptySchema`
---
*This PR was generated by Coder Agents.*
|
||
|
Susana Ferreira
|
363d7fc4d9 |
feat: add CTAs to AI budget admin notifications (#28138)
## Problem
AI budget admin notifications tell an admin that a user crossed a budget
threshold, but give them nothing to act on: no link to the user's
budget, and no pointer to the docs.
## Changes
- Add a **Manage user AI budget** action to both admin templates,
linking to the effective group's members page filtered to the affected
user, where the per-user override is set.
- Link to the AI budget documentation from both notification bodies.
- Say when the block lifts in the limit-reached notification ("blocked
until `{{period_end}}`").
- Add an `organization_name` label, needed to build the members page
URL.
- Grant the AI Gateway daemon subject read access to organizations,
which the label lookup requires.
- Add an authorization test for the notification path, which also covers
the existing group read grant.
<details>
<summary>Screenshots</summary>
**Admin Notifications in Coder UI**
<img width="456" height="695" alt="admin_notifications"
src="https://github.com/user-attachments/assets/a579f91c-55cc-497c-87f4-235f15493725"
/>
**Admin Notification in Email**
<img width="660" height="399" alt="admin_notifications_email"
src="https://github.com/user-attachments/assets/39b876fb-f512-41e5-997f-a5269cd26ddc"
/>
</details>
Related to internal Slack threads:
* https://codercom.slack.com/archives/C096PFVBZKN/p1786051630688569
* https://codercom.slack.com/archives/C04M9CNGHGF/p1786453662306149
> [!NOTE]
> Initially generated by Claude Opus 5, modified and reviewed by
@ssncferreira
|
||
|
Susana Ferreira
|
50ecd6959f |
feat: add a source filter to the AI model price list (#28329)
## Description With prices from the price book and prices set through the API both in the table, `list` showed only the price in effect and gave no way to tell where it came from. There was also no way to see the price book's entry for a model that had been overridden, since the listing resolves to the custom price. This adds a `source` column to the output and a `--source` filter for narrowing to one source, or to every row with `all`. Note: `--source` selects rows rather than models. A model carrying both a price book price and a custom one appears under either value, so the two filtered listings do not sum to the unfiltered one. `--source all` reports both rows at once. ## Changes - Add `Source` to `codersdk.AIModelPrice` and `AIModelPricesFilter`, and map it in `db2sdk`, which previously dropped it. - Filter `GetAIModelPrices` by source before resolution, so the price book's row for an overridden model stays reachable. - Accept `all` as a source, which reports every price a model holds instead of resolving to one. - Accept a `source` query parameter on the list endpoint, rejected with a 400 when outside the enum. - Add `--source` to the list command and a `source` column to its output. - Document the column and the filter, and lead the price section with `coder exp ai-model-prices list`. - Distinguish a deployment's default prices from the price book that ships with each release. Closes https://linear.app/codercom/issue/AIGOV-593 > [!NOTE] > Initially generated by Claude Opus 5, modified and reviewed by @ssncferreira |
||
|
Danielle Maywood
|
4a7a0a3348 | feat: label process_output rows with the process command (#28300) | ||
|
Susana Ferreira
|
19d1ee9fc8 |
feat: support custom prices for models in the price book (#28328)
## Description The experimental CLI could only price models the price book does not cover, so an operator with a negotiated rate for a model Coder already prices had no way to record it. The restriction existed because a single row served both the price book and prices set through the API, and the startup seeder would overwrite an override on the next restart. Each source now keeps its own row (https://github.com/coder/coder/pull/28204), so that no longer applies. This removes the restriction: a price set through the CLI takes effect over the price book for any model, and the book stays underneath it. ## Changes - Remove the validation that rejected models the price book covers. - Delete `prices.IsDefaultPriced` and the index behind it, which existed only for that check. - Generalize the `coder exp ai-model-prices update` help. - Document that a custom price overrides the price book and does not follow its updates. - Rework the spend approximation callout around what a custom price can and cannot correct. Closes https://linear.app/codercom/issue/AIGOV-594 > [!NOTE] > Initially generated by Claude Opus 5, modified and reviewed by @ssncferreira |
||
|
Susana Ferreira
|
9390e51422 |
feat: distinguish default and custom AI model prices (#28204)
## Description Prices in `ai_model_prices` come from two places: the price book embedded in each Coder release, and prices set through the experimental CLI (https://github.com/coder/coder/pull/27926). Until now, they were indistinguishable, so the startup seeder re-applied the book over every row, and a price someone set would silently revert on the next restart. This adds a `source` column with values `default` and `custom`, and keeps a row for each. Readers resolve a model to its custom price when one exists and fall back to the book, including the cost path that prices an interception. Keeping both rows means the price book stays current underneath an override, so a future delete can restore it. Note: the endpoint still only accepts models the price book does not cover. Overriding a price the book already carries is handled by the follow-up issue https://linear.app/codercom/issue/AIGOV-594. ## Changes - Add a migration for the `ai_model_price_source` enum and a `source` column, backfilled to `default` and then `NOT NULL`. `source` joins the primary key, so a model can hold one row per source. - Take a `source` parameter on `UpsertAIModelPrices`. Each source keeps its own row, so the price book and a custom price never overwrite each other. - Resolve to the custom price in `GetAIModelPriceByProviderModel` and `GetAIModelPrices`, so both the interception cost snapshot and the list endpoint use it. - Seed the price book as `default`, and write prices set through the endpoint as `custom`. Closes https://linear.app/codercom/issue/AIGOV-592 > [!NOTE] > Initially generated by Claude Opus 5, modified and reviewed by @ssncferreira |
||
|
cdrciandgithub-actions[bot]
|
5e0710de92 |
chore: refresh AI model price book (#28365)
## Price book changes 61 models added, 14 models removed, 51 models changed. <details> <summary>Added</summary> - azure/claude-mythos-5 - azure/claude-opus-4-7 - azure/kimi-k2.7-code - bedrock/global.openai.gpt-5.6-luna - bedrock/global.openai.gpt-5.6-sol - bedrock/global.openai.gpt-5.6-terra - bedrock/xai.grok-4.6 - copilot/gemini-3.7-flash - copilot/grok-4.6 - copilot/kimi-k3 - copilot/mai-code-1.1-flash - google/gemini-3.7-flash - openrouter/bytedance-seed/seed-2-1-turbo - openrouter/bytedance-seed/seed-2.0-code - openrouter/deepseek/deepseek-v4-pro-0813 - openrouter/dots-studio/dots-3-note-preview:free - openrouter/google/gemini-3.7-flash - openrouter/inclusionai/ling-3.0-flash - openrouter/liquid/lfm-2.5-2.6b:free - openrouter/meta/muse-glimmer-30b - openrouter/meta/muse-spark-1.2 - openrouter/nvidia/nemotron-3.5-lightning - openrouter/nvidia/nemotron-3.5-lightning:free - openrouter/qwen/qwen3.8-2.4t-a95b - openrouter/qwen/qwen3.8-27b - openrouter/sakana/sakana-namazu - openrouter/upstage/solar-pro4 - openrouter/x-ai/grok-4.6 - openrouter/z-ai/glm-5.2:free - openrouter/z-ai/glm-5.3 - openrouter/~z-ai/glm-latest - vercel/alibaba/qwen3.8-2.4t-a95b - vercel/alibaba/qwen3.8-27b - vercel/deepseek/deepseek-v4-pro-0813 - vercel/google/gemini-3.7-flash - vercel/inclusionai/ling-3.0-flash - vercel/meta/muse-glimmer-30b - vercel/meta/muse-spark-1.2 - vercel/meta/muse-spark-1.2-contributor - vercel/nvidia/nemotron-3.5-lightning - vercel/openai/gpt-4.1-fast - vercel/openai/gpt-4.1-mini-fast - vercel/openai/gpt-4.1-nano-fast - vercel/openai/gpt-4o-fast - vercel/openai/gpt-4o-mini-fast - vercel/openai/gpt-5-fast - vercel/openai/gpt-5-mini-fast - vercel/openai/gpt-5.1-thinking-fast - vercel/openai/gpt-5.2-fast - vercel/openai/gpt-5.3-codex-fast - vercel/openai/gpt-5.4-fast - vercel/openai/gpt-5.4-mini-fast - vercel/openai/gpt-5.5-fast - vercel/openai/gpt-5.6-luna-fast - vercel/openai/gpt-5.6-sol-fast - vercel/openai/gpt-5.6-terra-fast - vercel/openai/o3-fast - vercel/openai/o4-mini-fast - vercel/sakana/namazu - vercel/xai/grok-4.6 - vercel/zai/glm-5.3 </details> <details> <summary>Removed</summary> - anthropic/claude-opus-4-1 - anthropic/claude-opus-4-1-20250805 - google/gemini-2.0-flash - google/gemini-2.0-flash-lite - google/gemini-3-pro-preview - openrouter/ai21/jamba-large-1.7 - openrouter/inclusionai/ling-3.0-flash:free - openrouter/mancer/weaver - openrouter/openai/gpt-5.3-chat - vercel/anthropic/claude-opus-4.1 - vercel/inclusionai/ling-3.0-flash-free - vercel/openai/gpt-5.1-instant - vercel/openai/gpt-5.3-chat - vercel/zai/glm-4.6v </details> <details> <summary>Changed</summary> - azure/gpt-5.6-luna - azure/gpt-5.6-sol - azure/gpt-5.6-terra - bedrock/openai.gpt-5.6-sol - openrouter/deepseek/deepseek-chat-v3-0324 - openrouter/deepseek/deepseek-v3.1-terminus - openrouter/deepseek/deepseek-v4-flash - openrouter/deepseek/deepseek-v4-flash-0731 - openrouter/deepseek/deepseek-v4-pro - openrouter/google/gemini-3.6-flash - openrouter/google/gemma-4-31b-it - openrouter/gryphe/mythomax-l2-13b - openrouter/minimax/minimax-m2.5 - openrouter/minimax/minimax-m2.7 - openrouter/mistralai/mistral-small-3.2-24b-instruct - openrouter/moonshotai/kimi-k2.5 - openrouter/moonshotai/kimi-k2.6 - openrouter/moonshotai/kimi-k2.7-code - openrouter/openai/gpt-5.6-luna - openrouter/openai/gpt-5.6-luna-pro - openrouter/openai/gpt-5.6-sol - openrouter/openai/gpt-5.6-sol-pro - openrouter/openai/gpt-5.6-terra - openrouter/openai/gpt-5.6-terra-pro - openrouter/openai/gpt-oss-120b - openrouter/qwen/qwen-plus-2025-07-28:thinking - openrouter/qwen/qwen2.5-vl-72b-instruct - openrouter/qwen/qwen3-14b - openrouter/qwen/qwen3-235b-a22b-2507 - openrouter/qwen/qwen3-30b-a3b - openrouter/qwen/qwen3-coder-30b-a3b-instruct - openrouter/qwen/qwen3-vl-235b-a22b-thinking - openrouter/qwen/qwen3-vl-30b-a3b-instruct - openrouter/qwen/qwen3.5-35b-a3b - openrouter/qwen/qwen3.6-27b - openrouter/qwen/qwen3.6-35b-a3b - openrouter/tencent/hy3-preview - openrouter/thinkingmachines/inkling - openrouter/thinkingmachines/inkling-small - openrouter/z-ai/glm-5 - openrouter/z-ai/glm-5.2 - openrouter/~deepseek/deepseek-v4-flash-latest - openrouter/~google/gemini-flash-latest - openrouter/~moonshotai/kimi-latest - openrouter/~openai/gpt-latest - openrouter/~x-ai/grok-latest - vercel/deepseek/deepseek-v4-flash - vercel/deepseek/deepseek-v4-pro - vercel/google/gemini-3.6-flash - vercel/openai/gpt-5.6-sol - vercel/zai/glm-5.2 </details> ## Review notes Regenerated by `make gen/aibridge-prices` from the live [models.dev](https://models.dev) catalog. Both artifacts come from one snapshot, so they ship together: - `coderd/aibridge/prices/data/prices.json` - `site/src/pages/AgentsPage/components/ChatModelAdminPanel/knownModels/knownModelsGenerated.json` These are customer-visible cost numbers taken from upstream data, so this PR is never merged automatically. The summary above lists what moved; check the diff for exact figures before approving. Opened automatically by the [aigateway-prices-refresh workflow](https://github.com/coder/coder/actions/runs/32352560765). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Spike Curtis
|
f5d42a868e |
refactor(coderd): add related-data selection to workspace queries (#28302)
This is the first of three PRs implementing lite `codersdk.Workspace` responses ([GRU-82](https://linear.app/codercom/issue/GRU-82), [RFC](https://app.notion.com/p/coderhq/Lite-Workspace-API-Requests-3aad579be5928060b5e7cc65c539717f)). A fully populated workspace fans out into a large tree of DB queries (template, latest build, provisioner job + queue position, resources, metadata, agents, apps, statuses, scripts, log sources, template version). Most callers do not need most of that data, and one of these queries, `GetProvisionerJobsByIDsWithQueuePosition`, is consistently the top resource consumer in scale tests. This PR adds the internal plumbing to load only a selected subset of that tree. It introduces a `workspaceRelated` selection type and threads it through `workspaceData` and `workspaceBuildsData` down to the individual query call sites, skipping any query whose data was not selected. The selection is a pointer tree that mirrors the parent/child relationships between the objects: branch nodes are pointers (non-nil when selected) and leaf nodes are bools. Modeling it this way makes selecting a child without its parent unrepresentable, which matches how loading works (a parent must be queried to learn its children's identifiers), so there is no normalization step to forget. No behavior change: every existing caller passes a fully populated selection (`allWorkspaceRelated` / `allLatestBuildRelated`), so responses are identical. Wiring the HTTP `include_related` query parameter (Phase 2) and narrowing individual callers (Phase 3) come in follow-up PRs. Two notes on the current mapping: - `queue_position` is modeled as its own node, but only one job query exists and it always computes the ranking, so selecting `job` runs the full query today. Splitting off a cheaper queue-position-free job query is deferred (it needs a new SQL query + `make gen`). - The workspace-level `LatestAppStatus` is gated on the `latest_build.resources.agents.apps.statuses` node. It is queried directly by workspace ID, but a caller selecting it also pulls in the build chain as ancestors. Happy to decouple this if preferred. <details> <summary>Implementation plan</summary> # Lite Workspace API — Phase 1 Plan Add an internal configuration structure to the Coderd method that fetches a workspace with its related data (`workspaceData` /`workspaceBuildsData`), thread it down to the SQL query call sites, and skip ("squash") queries that are not requested. Server-internal only. No HTTP query-string parsing (Phase 2) and no `wsrelated` struct tags or field nil-ing (Phase 3). ## Scope - In: config struct mapping the RFC related-data hierarchy; threading through `workspaceData` and `workspaceBuildsData`; gating every related-data query; comprehensive unit tests with a mocked `database.Store` (gomock/dbmock). - Out: HTTP `include_related` parsing, codersdk changes, `wsrelated` tags, docs autogen, and changing `convertWorkspaces` skip-on-missing-data behavior. All existing callers pass an "include all" config, so Phase 1 is a behavior- preserving refactor. ## Design New file `coderd/wsrelateddata.go` with a pointer tree that mirrors the RFC hierarchy. Branch nodes are pointers (present when non-nil); leaf nodes are bools. Because a child cannot be expressed without its parent, the RFC "includes all ancestors" rule is a structural invariant of the type: an illegal selection is unrepresentable, so no normalization pass exists or is needed. ```go type workspaceRelated struct { Template bool LatestBuild *latestBuildRelated } type latestBuildRelated struct { Job *jobRelated Resources *resourcesRelated TemplateVersion bool } type jobRelated struct{ QueuePosition bool } type resourcesRelated struct { Metadata bool Agents *agentsRelated } type agentsRelated struct { Apps *appsRelated Scripts bool LogSources bool } type appsRelated struct{ Statuses bool } ``` Helpers: - `allWorkspaceRelated()` / `allLatestBuildRelated()` construct a fully populated tree. Passed by all current callers so behavior is unchanged. - Zero value (nil subtree) includes nothing but the workspace itself. - `(*latestBuildRelated).appStatuses()` is a nil-safe accessor for the deep `apps.statuses` leaf so call sites descend the tree without a guard chain. Rationale for the pointer tree over a flat struct plus `withAncestors()`: the invariant holds by construction (illegal state is unrepresentable) instead of relying on a normalization step a caller must remember. `workspaceBuildsData` takes the `*latestBuildRelated` subtree directly. ### Query gating `workspaceData(ctx, workspaces, cfg)`: - `GetTemplatesWithFilter` gated on `Template`. - `GetLatestWorkspaceBuildsByWorkspaceIDs` gated on `LatestBuild`; when off, skip `workspaceBuildsData` entirely (empty builds). - `GetLatestWorkspaceAppStatusesByWorkspaceIDs` (workspace-level LatestAppStatus) gated on `AppStatuses`. `workspaceBuildsData(ctx, builds, cfg)` gates each stage: - jobs (`GetProvisionerJobsByIDsWithQueuePosition`) + eligible provisioner daemons gated on `Job`. Only one job query exists and it always computes the queue position, so `QueuePosition` does not gate a separate query yet; it is modeled for hierarchy/Phase 2 completeness. A cheaper job query without the window-function ranking is a worthwhile follow-up but is out of scope for a simple Phase 1 (needs new SQL + sqlc gen). - resources gated on `Resources`; metadata on `Metadata`; agents on `Agents`; apps on `Apps`; app statuses on `AppStatuses`; scripts on `Scripts`; log sources on `LogSources`; template versions on `TemplateVersion`. Downstream `convertWorkspaceBuilds` already tolerates empty slices, so squashing queries yields zero/nil fields without extra work. ## Testing (comprehensive, mocked database.Store) Internal tests (`coderd/wsrelateddata_internal_test.go`) build `&API{Options: &Options{Database: dbmock.NewMockStore(ctrl)}}` and call `workspaceData` directly. gomock is strict, so setting EXPECT only on permitted queries proves the rest are squashed. Cases: include-all (every query runs); zero value (no related queries run); each single node (template only, latest_build only, job, resources, metadata, agents, apps, statuses, scripts, log_sources, template_version, workspace-level app statuses); the empty-resources short-circuit; and the constructors plus the nil-safe `appStatuses()` accessor at every depth. ## Steps 1. Add `coderd/wsrelateddata.go` (selection tree, constructors, `appStatuses`). 2. Thread the selection through `workspaceData` and `workspaceBuildsData`; gate queries by descending the tree. 3. Update the callers to pass `allWorkspaceRelated()` / `allLatestBuildRelated()`. 4. Add unit tests; run `make fmt`, `make lint`, targeted `go test`. ## Resolved decisions 1. Pointer tree over a flat struct + normalizer, so the ancestor rule is a structural invariant. 2. Cheaper queue-position-free job query deferred to a follow-up. </details> --- *Generated by Coder Agents on behalf of @spikecurtis.* |
||
|
McKayla はな
|
676b9bb06b | fix: only create workspace agents for start builds (#28276) | ||
|
Asher
|
7ba2e6e22a |
fix: add distributed protection for external auth refresh (#27479)
Before refreshing, the caller has to put a lease on the row to prevent any other replicas from trying to refresh using the same token. Callers unable to get a lease enter a holding pattern waiting for the results. Also use this same lease query to re-read the link. This more or less functions the same as the re-read we used to have after a refresh failure, except it also allows us to avoid making the failed request in the first place. The downside is that we have to refetch the link every time even if the caller just fetched it, but since we have to make a database call to set the lease anyway... |
||
|
Michael Suchacz
|
24b83dc727 |
feat: add organization picker to MCP server settings pages (#28150)
Adds an organization picker to the MCP server settings pages so
multi-org deployments can manage every organization's MCP servers
through the UI, not just the default organization's. Closes CODAGT-714
(scoped to MCP servers only; model configs stay deployment-wide until
CODAGT-709 lands).
## Stack Context
Top of the CODAGT-711/717/712/806 stack: MCP server configs are
org-scoped with RBAC, audit logging, per-config ACLs, and owner-scoped
user tokens. This layer makes the admin UI org-aware, resolving the
"default org only" limitation documented in the bottom PR.
## Summary
- The MCP servers list and add pages resolve the working organization
from an `org` search param (falling back to the default org), and render
an organization picker built on the existing `OrganizationAutocomplete`.
The picker only appears when more than one organization is visible, so
single-org deployments see no change.
- Selecting an organization rewrites the search param, refetches the
list for that org, and threads through to the add page so creation posts
to the selected organization.
- The update page now loads the server by ID (new organization-scoped
`GET /api/experimental/organizations/{organization}/mcp-servers/{id}`
frontend binding) instead of listing the default org and searching
client-side, so servers from any organization can be edited. Post-save
navigation returns to the owning organization's list via the server's
`organization_id`.
- Removes the now-unused `getDefaultOrganizationId` helper.
- Review hardening: organization display labels are disambiguated with
the unique org name when effective display names collide (picker
options, trigger text, and accessible name). Access is no longer
restricted to deployment admins: the pages, sidebar item, and nav entry
accept org-scoped `mcp_server_config` permissions, the picker lists only
organizations the user can view, content and affordances (add, edit,
delete) are gated on the selected organization's permissions, and
deployment admins keep full access everywhere.
## Testing
Storybook interaction stories cover: picker hidden with one org,
switching orgs refetches and swaps list contents, add posting to the
URL-selected org, add posting after an in-form org switch (asserting
typed form state survives the switch), and update loading by ID.
Red-green verified by breaking the search-param resolution and observing
the new stories fail.
> Mux acted on Mike's behalf for this PR.
|
||
|
david-fraley
|
8203b2ebaf | feat!: hide Coder Tasks behind the enable-ai-tasks flag (#28008) | ||
|
Michael Suchacz
|
a3eb1ee930 |
fix: gate MCP user tokens on owner-scoped personal RBAC (#27945)
Re-gates every `mcp_server_user_tokens` dbauthz method from `rbac.ResourceDeploymentConfig` onto owner-scoped personal actions, mirroring the `external_auth_links` prior art. These are per-user credential rows, not deployment config, and the old gate forced every call site to elevate with `dbauthz.AsSystemRestricted`. ## Summary - Reads gate on `ActionReadPersonal` and writes on `ActionUpdatePersonal` against the token owner (`MCPServerUserToken.RBACObject()` derives from `user_id`). The id-keyed refresh and refresh-failure mutations resolve the owner via a `GetMCPServerUserTokenByID` prefetch inside dbauthz; the mutation queries cannot retarget a row's owner, so there is no fetch-authorize race. - All seven token-path `AsSystemRestricted` elevations in `coderd/mcp.go` and all four in chatd are removed along with their `//nolint:gocritic` markers; handlers use the request actor. - The daemon-wide chatd subject keeps only `ActionReadPersonal` on `ResourceUser`. Background token refresh and failure persistence use a new per-user `AsChatdTokenOwner` subject holding only user-level `ActionUpdatePersonal` scoped to the token owner (both mutations resolve the row via the underlying store, so the subject needs no read grant), and chatd never holds site-wide personal-write access. `TestAsChatd` pins the complete `ResourceUser` action set and `TestMCPServerUserTokensAuth` exercises both subjects against the real authorizer. - New strict-authorizer test (`rbac.NewAuthorizer` against real rows): owner passes get/upsert/delete, a stranger fails, and an org admin of another org fails. No user-visible behavior change. Stacked on #27944. Completes the MCP org-separation stack. Closes https://linear.app/codercom/issue/CODAGT-806 UAT: regression pass on a dogfood instance: OAuth2 connect redirect, member list/status with redaction, self-scoped disconnect (cannot touch another user's token), end-to-end chat MCP tool invocation, and lower-stack audit/ACL smoke checks. > Mux (AI agent) authored this PR on Mike's behalf. <!-- mux-attribution: model=claude-fable-5 thinking=high --> |
||
|
Michael Suchacz
|
7ca7c30f40 |
feat: add group and user ACLs to MCP server configs (#27944)
Adds group and user ACLs to org-scoped MCP server configs so
organizations can restrict specific MCP servers to subsets of members,
mirroring the template ACL pattern.
## Summary
- Migration adds `group_acl`/`user_acl` JSONB columns (nested `{"<id>":
{"permissions": [...]}}` shape) and seeds every existing config with its
organization's Everyone group read entry (the Everyone group ID equals
the org ID), so member access is unchanged by default. Creation seeds
the same entry.
- The blanket org-member read grant from the base PR is replaced by ACL
evaluation: Rego requires org membership for every ACL grant, and
`GetAuthorizedMCPServerConfigs` compiles ACL-aware SQL filters.
- New `ActionShare` (org admins) gates `GET|PATCH
/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl`
(nested under the organization like the rest of the config surface).
PATCH validates principals against the config's organization, merges
sparse updates under a row lock, stamps `updated_by`/`updated_at`, and
is audited as a Write with `Old` captured before authorization. A config
deleted concurrently between the middleware fetch and the locked
re-fetch is concealed as 404, matching the update and delete handlers.
ACL columns are tracked in the audit table.
- ACL management is available in all editions (no enterprise
entitlement), documented in the MCP servers page. Revoking an ACL does
not retro-strip already-selected configs from existing chats; new
selection is blocked at chat create.
- Force On respects the ACL: the forced set is loaded as the chat owner,
so a `force_on` server whose ACL denies the owner never attaches at
create, send, or generation time.
- No rolling-upgrade machinery: upgrades run in scheduled maintenance
downtime, so the migration only backfills existing rows; the API sets
the Everyone read grant explicitly on every insert.
Stacked on #27943. Part of the MCP org-separation stack.
Closes https://linear.app/codercom/issue/CODAGT-712
UAT: verified on a dogfood instance with two members and a custom group:
Everyone-seed default visibility, group grant with Everyone removal
(non-member loses list/fetch/selection), foreign-principal rejection,
non-admin share denial (audited 403 / concealed 404), user_acl restore,
and audited ACL diffs.
> Mux (AI agent) authored this PR on Mike's behalf.
<!-- mux-attribution: model=claude-fable-5 thinking=high -->
|
||
|
Michael Suchacz
|
299e72ad30 |
feat: audit MCP server config changes (#27943)
Adds enterprise audit logging for MCP server config create, update, and
delete, with strict secret redaction. MCP configs hold credentials
(OAuth2 client secrets, API keys, custom headers), so admin changes to
them need an audit trail.
## Summary
- `enterprise/audit/table.go` gains an `MCPServerConfig` entry
enumerating every column: `oauth2_client_secret`, `api_key_value`, and
`custom_headers` are `ActionSecret` (never appear in diffs); dbcrypt
`*_key_id` bookkeeping, IDs, and timestamps are ignored; the remaining
config fields, including the endpoint URL fields, are tracked so
auditors can see which endpoints a config points at.
- Type registration in `coderd/audit` (diff, request, resource target
with org attribution), `codersdk/audit.go`, and a `resource_type` enum
migration.
- Handlers wire `audit.InitRequest`: create records `New`; update and
delete record `Old` from the param middleware before the
write-authorization check, so a readable-but-not-writable caller
produces an audited 403 while read-denied callers stay concealed as
unaudited 404s.
- Tests: create/update/delete audit entries, write-denied and
delete-denied 403 auditing, cross-org concealment producing zero
entries, and a serializer-level regression test proving none of the
three secret classes can reach a serialized diff.
- Review round: MCP config audit entries link to
`/ai/settings/mcp-servers/{id}`, audit table comments are trimmed per
review, and a fault-injection test pins that a config row surviving a
failed post-discovery credential update still gets its creation audit
entry.
Stacked on #27942 (org-scoped MCP configs). Part of the MCP
org-separation stack.
Closes https://linear.app/codercom/issue/CODAGT-717
UAT: verified on a trial-licensed dogfood instance: audit entries for
the full CRUD lifecycle with correct actor/org/target, redacted secrets
in the update and OAuth2 create diffs, and a full plaintext scan of the
audit dump finding zero secret leaks.
> Mux (AI agent) authored this PR on Mike's behalf.
<!-- mux-attribution: model=claude-fable-5 thinking=high -->
|
||
|
Michael Suchacz
|
f7a0de7a11 |
fix: harden org-scoped MCP config chat gating, updates, and visibility (#28065)
Hardens the org-scoped MCP server config surface from #27942 with fixes and regression pins that are independent of the core cutover: - Keep chats sendable after a selected MCP server is disabled (persisted selections are exempt from message-time rejection). - Keep cached MCP selections usable after a background refetch error (gate the composer on missing data, not `isSuccess`). - Merge PATCH updates onto the current row so unset fields are not clobbered. - Give auditors the full management view of MCP configs their audit logs reference (site and org auditor roles). - Regression pins: frozen OAuth2 callback path, cross-org concealment for item routes, OAuth callback token binding, and disconnect responses indistinguishable from nonexistent config IDs. - Storybook: semantic textbox queries in MCP loading stories; docs note that the MCP settings page needs deployment access. ## Stack context Part of the MCP org-separation stack (CODAGT-711 org scope -> apidocs -> hardening -> CODAGT-717 audit -> CODAGT-712 ACLs -> CODAGT-806 token RBAC). Split out of #27942 to keep the core cutover reviewable; each change here builds on the org-scoped routes and chat gating introduced below it. > Mux (AI agent) authored this PR on Mike's behalf. <!-- mux-attribution: model=claude-fable-5 thinking=high --> |
||
|
Michael Suchacz
|
f2bc9ab1f5 |
docs: complete swagger annotations for organization-scoped MCP routes (#28064)
Adds the missing swagger annotations for the eight organization-scoped MCP server config routes introduced in #27942 and checks in the regenerated API artifacts (`coderd/apidoc`, `docs/reference/api`). No behavior changes: 58 hand-written annotation lines, the rest is generated output. ## Stack context Part of the MCP org-separation stack (CODAGT-711 org scope -> apidocs -> hardening -> CODAGT-717 audit -> CODAGT-712 ACLs -> CODAGT-806 token RBAC). Split out of #27942 to keep the core cutover reviewable; these routes live under `/api/experimental`, where main already ships several MCP handlers without annotations, so the base PR is consistent with existing precedent until this lands. Closes nothing on its own; documentation completion for CODAGT-711. > Mux (AI agent) authored this PR on Mike's behalf. <!-- mux-attribution: model=claude-fable-5 thinking=high --> |
||
|
Michael SuchaczandMathias Fredriksson
|
443e3b9b80 |
feat!: org-scope MCP server configs with RBAC (#27942)
Moves `mcp_server_configs` from deployment scope to organization scope
so each organization fully controls the MCP servers its members can use
with Coder Agents.
## Summary
- Migration: adds `organization_id` (NOT NULL, FK) and keeps existing
rows as the default organization's originals with credentials intact.
Other organizations start with no MCP servers and configure their own;
nothing is copied across organizations. Chats outside the default
organization keep any now-cross-organization `mcp_server_ids` entries;
the runtime already ignores IDs that do not resolve in the chat's
organization, so no data rewrite is needed. Slug uniqueness becomes
`(organization_id, slug)`.
- RBAC: new org-scoped `ResourceMCPServerConfig` with regosql converter
and `GetAuthorizedMCPServerConfigs`; org admins get in-org CRUD, org
members get read (replaced by ACL evaluation in the follow-up ACL PR in
this stack).
- API: all config routes nest under the organization, matching
templates: `POST|GET
/api/experimental/organizations/{organization}/mcp-servers` and
`GET|PATCH|DELETE .../mcp-servers/{mcpserverconfig}` (plus
`oauth2/connect`), resolved by a read-only param middleware that
conceals read-denied and cross-organization access as 404. Two routes
stay on the frozen `/api/experimental/mcp/servers/{mcpServer}` block:
the OAuth2 callback (the redirect URI baked into existing AS-side client
registrations) and `oauth2/disconnect`, which must remain reachable by
users removed from the organization so they can still revoke their token
grant.
- Chat runtime: selection validation and generation resolve configs
strictly by IDs, enabled state, and the chat's organization in SQL;
requested duplicates are normalized; invalid or cross-org IDs are
rejected with the precise ID list. IDs already persisted on a chat are
exempt from message-time rejection so disabling a selected server never
blocks sends; generation skips servers that are no longer usable.
- Frontend: API layer and admin settings pages target the new endpoints.
The admin page manages the default organization's servers; the org
picker is tracked separately (CODAGT-714).
- Security hardening from review: OAuth user grants are additionally
bound to `oauth2_revocation_url` (changing it invalidates grants, and a
racing OAuth callback gets 409 instead of recreating a grant).
Stack-wide SSRF protection for MCP config-directed traffic was split
into its own PR at the top of this stack (#28242) to keep this diff
reviewable; this PR keeps main's existing discovery IP-range guard.
OAuth2 auto-discovery now completes before the config row is inserted: a
failed discovery persists nothing, and there is no provisional row that
concurrent updates could race against.
Two follow-up PRs in this stack were split out to keep this diff
reviewable: #28064 completes the swagger annotations for the moved
routes (main already ships these experimental MCP handlers unannotated),
and #28065 carries hardening fixes and regression pins on top of the
cutover.
- Force On enforcement (landed on main mid-review) is org-scoped: the
forced set is read per chat organization
(`GetForcedMCPServerConfigsByOrganization`), so another organization's
`force_on` server never attaches to a chat.
## Breaking changes (experimental API)
The MCP server config endpoints move from the deployment-scoped
`/api/experimental/mcp/servers` block to organization-nested paths:
`POST|GET /api/experimental/organizations/{organization}/mcp-servers`
and `GET|PATCH|DELETE
/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}`
(plus `oauth2/connect`). The old paths are removed, so API consumers
must supply an organization. Two routes intentionally stay on the frozen
`/api/experimental/mcp/servers/{mcpServer}` block: the OAuth2 callback
(its redirect URI is baked into existing AS-side client registrations)
and `oauth2/disconnect` (must remain reachable by users removed from the
organization). These endpoints are under `/api/experimental`, so no
deprecation window is provided.
## Rolling upgrades
During a rolling deploy, an old replica creating an MCP config can fail
the new `NOT NULL organization_id` constraint until it is upgraded
(reads are unaffected: old binaries' generated queries select their own
column lists). This matches the repo's existing precedent for additive
NOT NULL migrations (000562) and affects only the admin config-create
path in the upgrade window.
Upgrades are expected to run in scheduled maintenance downtime with the
database locked during migration, so the migration ships no
rolling-upgrade compatibility machinery. The down migration deletes
organization-created configs (their chat references are cleaned by the
000510 delete trigger) and restores deployment-wide slug uniqueness.
Part of the MCP org-separation stack (CODAGT-711 -> CODAGT-717 audit ->
CODAGT-712 ACLs -> CODAGT-806 token RBAC).
Closes https://linear.app/codercom/issue/CODAGT-711
UAT: validated end to end on a two-org dogfood deployment, including a
real pre-migration to post-migration upgrade, cross-org isolation
(404s), same-slug-two-orgs, chat selection gating, and a live MCP tool
call through the org-scoped generation path. The migration was later
revised to keep existing rows in the default organization only (no
per-organization copies); that revision is covered by the migration test
suite.
> Mux (AI agent) authored this PR on Mike's behalf.
<!-- mux-attribution: model=claude-fable-5 thinking=high -->
---------
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
|
||
|
Hugo Dutka
|
c2bb62446d |
fix(coderd): remove per-chat system prompt limit (#28294)
Per-chat system prompts longer than 10,000 characters are rejected during chat creation. Remove that validation so chat creation accepts longer system prompts within the existing request-body limit. |