From d750b12a592483e38e9f959f3fd8bbe6963ed50b Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Tue, 18 Aug 2026 01:08:44 +0000 Subject: [PATCH 1/6] feat(coderd_chat_system_prompt): manage the deployment-wide chat system prompt Adds a singleton resource for the Coder Agents chat system prompt (Settings -> Instructions), backed by the experimental /api/experimental/chats/config/system-prompt endpoint via ExperimentalClient.GetChatSystemPrompt/UpdateChatSystemPrompt. Coder sanitizes the stored prompt (invisible-char stripping, CRLF normalization, blank-line collapsing, trimming), so system_prompt is a custom string type whose semantic equality compares sanitized forms. The everyday case this absorbs is the trailing newline from file("system-prompt.md"); real edits still diff. The sanitizer is a straight port of chatd.SanitizePromptText, and divergence fails loud (a visible diff) rather than silent. Follows the coderd_oauth2_provider_settings singleton pattern: shared PUT for create/update, destroy resets the never-configured defaults (empty prompt, include_default_system_prompt = true), import adopts the live value without writing, a plan-time warning fires when a first apply would overwrite a non-empty out-of-band prompt, and a 404 maps to an actionable version hint (the endpoint shipped in Coder v2.32.0). Prompt length is validated at plan time against coderd's 128 KiB cap. Closes #411 --- docs/resources/chat_system_prompt.md | 72 +++ .../coderd_chat_system_prompt/import.sh | 3 + .../coderd_chat_system_prompt/resource.tf | 9 + .../provider/chat_system_prompt_resource.go | 451 ++++++++++++++++++ .../chat_system_prompt_resource_test.go | 336 +++++++++++++ .../provider/chat_system_prompt_sanitize.go | 123 +++++ internal/provider/provider.go | 1 + 7 files changed, 995 insertions(+) create mode 100644 docs/resources/chat_system_prompt.md create mode 100644 examples/resources/coderd_chat_system_prompt/import.sh create mode 100644 examples/resources/coderd_chat_system_prompt/resource.tf create mode 100644 internal/provider/chat_system_prompt_resource.go create mode 100644 internal/provider/chat_system_prompt_resource_test.go create mode 100644 internal/provider/chat_system_prompt_sanitize.go diff --git a/docs/resources/chat_system_prompt.md b/docs/resources/chat_system_prompt.md new file mode 100644 index 0000000..b7de0c9 --- /dev/null +++ b/docs/resources/chat_system_prompt.md @@ -0,0 +1,72 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "coderd_chat_system_prompt Resource - terraform-provider-coderd" +subcategory: "" +description: |- + ~> This resource is experimental. Changes are to be expected, and we recommend using it with caution in production environments. + The deployment-wide chat system prompt for Coder Agents (Settings → Instructions in the dashboard). + This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. + Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from file(...) does not cause drift. + ~> Warning + If a system prompt was configured out of band, terraform import this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. + ~> Warning + terraform destroy resets the prompt to empty and include_default_system_prompt to true, the defaults of a never-configured deployment. The API has no delete operation for this setting. + ~> Warning + This resource requires Coder version 2.32.0 https://github.com/coder/coder/releases/tag/v2.32.0 or later, and a token with site-wide owner permissions. +--- + +# coderd_chat_system_prompt (Resource) + +~> This resource is experimental. Changes are to be expected, and we recommend using it with caution in production environments. + +The deployment-wide chat system prompt for Coder Agents (`Settings → Instructions` in the dashboard). + +This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. + +Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from `file(...)` does not cause drift. + +~> **Warning** +If a system prompt was configured out of band, `terraform import` this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. + +~> **Warning** +`terraform destroy` resets the prompt to empty and `include_default_system_prompt` to `true`, the defaults of a never-configured deployment. The API has no delete operation for this setting. + +~> **Warning** +This resource requires Coder version [2.32.0](https://github.com/coder/coder/releases/tag/v2.32.0) or later, and a token with site-wide `owner` permissions. + +## Example Usage + +```terraform +// Keep the prompt itself in a Markdown file next to the Terraform +// configuration so it can be reviewed like any other prose. +resource "coderd_chat_system_prompt" "this" { + system_prompt = file("${path.module}/system-prompt.md") + + // Append to Coder's built-in system prompt (the default) rather + // than replacing it. + include_default_system_prompt = true +} +``` + + +## Schema + +### Required + +- `system_prompt` (String) The custom system prompt text, typically `file("${path.module}/system-prompt.md")`. Limited to 128 KiB after sanitization. + +### Optional + +- `include_default_system_prompt` (Boolean) Whether the custom prompt is appended to Coder's built-in system prompt (`true`, the default) or replaces it entirely (`false`). + +## Import + +Import is supported using the following syntax: + +The [`terraform import` command](https://developer.hashicorp.com/terraform/cli/commands/import) can be used, for example: + +```shell +# The chat system prompt is a deployment-wide singleton, so the import ID is +# required by the CLI syntax but otherwise unused. +terraform import coderd_chat_system_prompt.this chat_system_prompt +``` diff --git a/examples/resources/coderd_chat_system_prompt/import.sh b/examples/resources/coderd_chat_system_prompt/import.sh new file mode 100644 index 0000000..0276997 --- /dev/null +++ b/examples/resources/coderd_chat_system_prompt/import.sh @@ -0,0 +1,3 @@ +# The chat system prompt is a deployment-wide singleton, so the import ID is +# required by the CLI syntax but otherwise unused. +terraform import coderd_chat_system_prompt.this chat_system_prompt diff --git a/examples/resources/coderd_chat_system_prompt/resource.tf b/examples/resources/coderd_chat_system_prompt/resource.tf new file mode 100644 index 0000000..a272f06 --- /dev/null +++ b/examples/resources/coderd_chat_system_prompt/resource.tf @@ -0,0 +1,9 @@ +// Keep the prompt itself in a Markdown file next to the Terraform +// configuration so it can be reviewed like any other prose. +resource "coderd_chat_system_prompt" "this" { + system_prompt = file("${path.module}/system-prompt.md") + + // Append to Coder's built-in system prompt (the default) rather + // than replacing it. + include_default_system_prompt = true +} diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go new file mode 100644 index 0000000..66b77b9 --- /dev/null +++ b/internal/provider/chat_system_prompt_resource.go @@ -0,0 +1,451 @@ +package provider + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/coder/coder/v2/codersdk" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-log/tflog" +) + +// Ensure provider defined types fully satisfy framework interfaces. +var _ resource.Resource = &ChatSystemPromptResource{} +var _ resource.ResourceWithImportState = &ChatSystemPromptResource{} +var _ resource.ResourceWithModifyPlan = &ChatSystemPromptResource{} + +// chatSystemPromptMinVersion is the first Coder release serving +// `/api/experimental/chats/config/system-prompt` (coder/coder#22857). It is +// named in the error surfaced when the endpoint 404s so an admin pointed at an +// older deployment gets an actionable message instead of a bare "not found". +const chatSystemPromptMinVersion = "2.32.0" + +// maxChatSystemPromptBytes mirrors coderd's maxSystemPromptLenBytes (128 KiB, +// coderd/exp_chats.go). The server rejects longer prompts with a 400 at apply +// time; validating here fails the same way at plan time instead. +const maxChatSystemPromptBytes = 131072 + +// pathSystemPrompt anchors plan-time diagnostics to the attribute they are +// about. +var pathSystemPrompt = path.Root("system_prompt") + +type ChatSystemPromptResource struct { + *CoderdProviderData +} + +// ChatSystemPromptResourceModel describes the resource data model. +type ChatSystemPromptResourceModel struct { + SystemPrompt chatSystemPromptTextValue `tfsdk:"system_prompt"` + IncludeDefaultSystemPrompt types.Bool `tfsdk:"include_default_system_prompt"` +} + +func NewChatSystemPromptResource() resource.Resource { + return &ChatSystemPromptResource{} +} + +func (r *ChatSystemPromptResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_chat_system_prompt" +} + +func (r *ChatSystemPromptResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + MarkdownDescription: `~> This resource is experimental. Changes are to be expected, and we recommend using it with caution in production environments. + +The deployment-wide chat system prompt for Coder Agents (` + "`Settings → Instructions`" + ` in the dashboard). + +This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. + +Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from ` + "`file(...)`" + ` does not cause drift. + +~> **Warning** +If a system prompt was configured out of band, ` + "`terraform import`" + ` this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. + +~> **Warning** +` + "`terraform destroy`" + ` resets the prompt to empty and ` + "`include_default_system_prompt`" + ` to ` + "`true`" + `, the defaults of a never-configured deployment. The API has no delete operation for this setting. + +~> **Warning** +This resource requires Coder version [` + chatSystemPromptMinVersion + `](https://github.com/coder/coder/releases/tag/v` + chatSystemPromptMinVersion + `) or later, and a token with site-wide ` + "`owner`" + ` permissions. +`, + Attributes: map[string]schema.Attribute{ + "system_prompt": schema.StringAttribute{ + CustomType: chatSystemPromptTextType{}, + Required: true, + MarkdownDescription: "The custom system prompt text, typically `file(\"${path.module}/system-prompt.md\")`. " + + "Limited to 128 KiB after sanitization.", + Validators: []validator.String{ + chatSystemPromptLengthValidator{}, + }, + }, + "include_default_system_prompt": schema.BoolAttribute{ + Optional: true, + Computed: true, + Default: booldefault.StaticBool(true), + MarkdownDescription: "Whether the custom prompt is appended to Coder's built-in system prompt (`true`, the default) " + + "or replaces it entirely (`false`).", + }, + }, + } +} + +func (r *ChatSystemPromptResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + // Prevent panic if the provider has not been configured. + if req.ProviderData == nil { + return + } + + data, ok := req.ProviderData.(*CoderdProviderData) + if !ok { + resp.Diagnostics.AddError( + "Unable to configure provider data", + fmt.Sprintf("Expected *CoderdProviderData, got: %T. Please report this issue to the provider developers.", req.ProviderData), + ) + return + } + + r.CoderdProviderData = data +} + +func (r *ChatSystemPromptResource) experimentalClient() *codersdk.ExperimentalClient { + return codersdk.NewExperimentalClient(r.Client) +} + +func (r *ChatSystemPromptResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var data ChatSystemPromptResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + prompt, err := r.experimentalClient().GetChatSystemPrompt(ctx) + if err != nil { + // Deliberately not treated as "resource deleted": this setting is a + // deployment singleton that always exists on a supported deployment, + // so a 404 means the endpoint is missing, not the resource. + resp.Diagnostics.Append(chatSystemPromptDiag("read", err)...) + return + } + + // The custom type's semantic equality keeps the prior (configured) value + // when the live value differs only by sanitization. + data.SystemPrompt = newChatSystemPromptTextValue(prompt.SystemPrompt) + data.IncludeDefaultSystemPrompt = types.BoolValue(prompt.IncludeDefaultSystemPrompt) + + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +func (r *ChatSystemPromptResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var data ChatSystemPromptResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "creating chat system prompt") + + // Create and Update use a shared implementation: the underlying API is a + // single idempotent PUT with no separate create semantics. + resp.Diagnostics.Append(r.put(ctx, "create", &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "successfully created chat system prompt") + + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +func (r *ChatSystemPromptResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var data ChatSystemPromptResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "updating chat system prompt") + + resp.Diagnostics.Append(r.put(ctx, "update", &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "successfully updated chat system prompt") + + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +// put writes the planned prompt and include-default flag. The flag pointer is +// always non-nil: the API treats an omitted field as "leave the current value +// alone", which is the right default for a partial update but wrong for this +// resource, which owns the value outright (the attribute has a schema default, +// so the plan always carries a known value). +func (r *ChatSystemPromptResource) put(ctx context.Context, action string, data *ChatSystemPromptResourceModel) diag.Diagnostics { + var diags diag.Diagnostics + + includeDefault := data.IncludeDefaultSystemPrompt.ValueBool() + err := r.experimentalClient().UpdateChatSystemPrompt(ctx, codersdk.UpdateChatSystemPromptRequest{ + SystemPrompt: data.SystemPrompt.ValueString(), + IncludeDefaultSystemPrompt: &includeDefault, + }) + if err != nil { + diags.Append(chatSystemPromptDiag(action, err)...) + return diags + } + + // The PUT returns 204 with no body, so nothing to reconcile: state keeps + // the configured value and the custom type's semantic equality absorbs + // the server-side sanitization on the next Read. + return diags +} + +func (r *ChatSystemPromptResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + tflog.Trace(ctx, "deleting chat system prompt") + + // There is no DELETE endpoint for this setting: it is a `site_configs` + // upsert. Reset to the defaults of a never-configured deployment (empty + // prompt, include-default true) so `terraform destroy` leaves the + // deployment in a well-defined state. + // + // If this fails, the appended error keeps the resource in state, so a + // subsequent `terraform destroy` retries rather than the admin wrongly + // believing the prompt was reset. + includeDefault := true + err := r.experimentalClient().UpdateChatSystemPrompt(ctx, codersdk.UpdateChatSystemPromptRequest{ + SystemPrompt: "", + IncludeDefaultSystemPrompt: &includeDefault, + }) + if err != nil { + resp.Diagnostics.Append(chatSystemPromptDiag("reset", err)...) + return + } + + tflog.Trace(ctx, "successfully deleted chat system prompt") +} + +// ModifyPlan emits the standard experimental-resource warning and, on a first +// apply, warns when a non-empty out-of-band prompt is about to be overwritten, +// giving the admin a chance to `terraform import` instead. +func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + resp.Diagnostics.AddWarning( + "Experimental Resource", + "coderd_chat_system_prompt is experimental. Changes are expected, and it is not recommended for production use.", + ) + + // A destroy plan has a null plan. Nothing to advise on. + if req.Plan.Raw.IsNull() { + return + } + // Only a genuine create reaches the no-prior-state case this warns about. + // `terraform import` populates state without ever running Create(), so + // this correctly stays quiet on the first plan after an import. + if !req.State.Raw.IsNull() { + return + } + // Configure() has not run during the validate walk. + if r.CoderdProviderData == nil { + return + } + + var data ChatSystemPromptResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + // A Required attribute can still be unknown when it comes from an input + // variable or a module output. Defer rather than guess. + if data.SystemPrompt.IsUnknown() || data.SystemPrompt.IsNull() { + return + } + + live, err := r.experimentalClient().GetChatSystemPrompt(ctx) + if err != nil { + // Best-effort advisory only. Create() makes the same call for real + // moments later and reports the error there, with the right wording + // for the operation that actually failed. + tflog.Debug(ctx, "skipping chat system prompt plan-time check", map[string]any{ + "error": err.Error(), + }) + return + } + if live.SystemPrompt == "" { + // Nothing configured out of band; nothing to lose. + return + } + if sanitizePromptText(live.SystemPrompt) == sanitizePromptText(data.SystemPrompt.ValueString()) { + // The planned prompt matches the live one. + return + } + + resp.Diagnostics.AddAttributeWarning( + pathSystemPrompt, + "Overwriting an out-of-band value", + "This deployment already has a chat system prompt configured, and applying will overwrite it. "+ + "Terraform has no prior state for this resource, so this change is not shown as a diff.\n\n"+ + "If you meant to adopt the deployment's existing value rather than overwrite it, run "+ + "`terraform import coderd_chat_system_prompt. chat_system_prompt` first.", + ) +} + +func (r *ChatSystemPromptResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + // No identifying attribute exists to extract from req.ID: this resource is + // a deployment-wide singleton and Read() takes no parameters. Terraform + // calls Read() immediately after this to populate both attributes from the + // live API; the import ID itself is required by Terraform's CLI syntax but + // otherwise unused. + // + // The framework requires at least one attribute be set for the import to + // produce a non-null state object for Read() to overwrite. + resp.Diagnostics.Append(resp.State.Set(ctx, ChatSystemPromptResourceModel{ + SystemPrompt: newChatSystemPromptTextValue(""), + IncludeDefaultSystemPrompt: types.BoolValue(true), + })...) +} + +// chatSystemPromptDiag converts a codersdk error from the chat system prompt +// endpoint into a diagnostic. Every CRUD path routes through here so a +// deployment that predates the endpoint produces the same actionable message +// whichever operation hit it first. +func chatSystemPromptDiag(action string, err error) diag.Diagnostics { + var diags diag.Diagnostics + + // Not isNotFound: that helper also maps a 400 "must be an existing uuid or + // username" to not-found, which is meaningless for a parameterless + // endpoint and would mislabel an unrelated bad request as a version + // problem. + var sdkErr *codersdk.Error + if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { + diags.AddError( + "Unsupported Coder Version", + fmt.Sprintf("Unable to %s the chat system prompt: the deployment returned 404 for %s. "+ + "This endpoint requires Coder version %s or later and a token with site-wide permissions; "+ + "upgrade the deployment, or remove `coderd_chat_system_prompt` from your configuration. "+ + "Original error: %s", + action, "/api/experimental/chats/config/system-prompt", chatSystemPromptMinVersion, err), + ) + return diags + } + + // Every other failure passes coderd's own message straight through. + diags.AddError("Client Error", fmt.Sprintf("unable to %s the chat system prompt, got error: %s", action, err)) + return diags +} + +// chatSystemPromptLengthValidator rejects prompts whose sanitized form exceeds +// coderd's 128 KiB cap, failing at plan time with the same limit the server +// would enforce with a 400 at apply time. +type chatSystemPromptLengthValidator struct{} + +func (chatSystemPromptLengthValidator) Description(context.Context) string { + return fmt.Sprintf("system prompt must be at most %d bytes after sanitization", maxChatSystemPromptBytes) +} + +func (v chatSystemPromptLengthValidator) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} + +func (v chatSystemPromptLengthValidator) ValidateString(ctx context.Context, req validator.StringRequest, resp *validator.StringResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { + return + } + if got := len(sanitizePromptText(req.ConfigValue.ValueString())); got > maxChatSystemPromptBytes { + resp.Diagnostics.AddAttributeError( + req.Path, + "System Prompt Too Long", + fmt.Sprintf("The system prompt is %d bytes after sanitization; the maximum Coder accepts is %d bytes (128 KiB).", got, maxChatSystemPromptBytes), + ) + } +} + +// chatSystemPromptTextType is a string type whose values compare equal when +// their sanitized forms match, absorbing the server-side prompt sanitization +// (trailing newlines from `file(...)`, CRLF line endings, invisible +// characters) instead of reporting it as drift. +type chatSystemPromptTextType struct { + basetypes.StringType +} + +var _ basetypes.StringTypable = chatSystemPromptTextType{} + +// String implements basetypes.StringTypable. +func (t chatSystemPromptTextType) String() string { + return "chatSystemPromptTextType" +} + +// Equal implements basetypes.StringTypable. +func (t chatSystemPromptTextType) Equal(o attr.Type) bool { + if o, ok := o.(chatSystemPromptTextType); ok { + return t.StringType.Equal(o.StringType) + } + return false +} + +// ValueType implements basetypes.StringTypable. +func (t chatSystemPromptTextType) ValueType(ctx context.Context) attr.Value { + return chatSystemPromptTextValue{} +} + +// ValueFromString implements basetypes.StringTypable. +func (t chatSystemPromptTextType) ValueFromString(ctx context.Context, in basetypes.StringValue) (basetypes.StringValuable, diag.Diagnostics) { + return chatSystemPromptTextValue{StringValue: in}, nil +} + +// ValueFromTerraform implements basetypes.StringTypable. +func (t chatSystemPromptTextType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { + attrValue, err := t.StringType.ValueFromTerraform(ctx, in) + if err != nil { + return nil, err + } + stringValue, ok := attrValue.(basetypes.StringValue) + if !ok { + return nil, fmt.Errorf("unexpected type %T, expected basetypes.StringValue", attrValue) + } + return chatSystemPromptTextValue{StringValue: stringValue}, nil +} + +type chatSystemPromptTextValue struct { + basetypes.StringValue +} + +var _ basetypes.StringValuableWithSemanticEquals = chatSystemPromptTextValue{} + +func newChatSystemPromptTextValue(value string) chatSystemPromptTextValue { + return chatSystemPromptTextValue{StringValue: basetypes.NewStringValue(value)} +} + +// Type implements basetypes.StringValuable. +func (v chatSystemPromptTextValue) Type(ctx context.Context) attr.Type { + return chatSystemPromptTextType{} +} + +// Equal implements basetypes.StringValuable. +func (v chatSystemPromptTextValue) Equal(o attr.Value) bool { + if o, ok := o.(chatSystemPromptTextValue); ok { + return v.StringValue.Equal(o.StringValue) + } + return false +} + +// StringSemanticEquals implements basetypes.StringValuableWithSemanticEquals: +// two prompts are the same setting iff they sanitize to the same string. +func (v chatSystemPromptTextValue) StringSemanticEquals(ctx context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { + var diags diag.Diagnostics + newValue, ok := newValuable.(chatSystemPromptTextValue) + if !ok { + diags.AddError( + "Semantic Equality Check Error", + fmt.Sprintf("Expected chatSystemPromptTextValue, got: %T. Please report this issue to the provider developers.", newValuable), + ) + return false, diags + } + return sanitizePromptText(v.ValueString()) == sanitizePromptText(newValue.ValueString()), diags +} diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go new file mode 100644 index 0000000..eb30447 --- /dev/null +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -0,0 +1,336 @@ +package provider + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "regexp" + "strings" + "sync" + "testing" + + "github.com/coder/coder/v2/codersdk" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" + "github.com/stretchr/testify/require" +) + +// chatSystemPromptPath is the experimental endpoint backing the resource. +const chatSystemPromptPath = "/api/experimental/chats/config/system-prompt" + +const chatSystemPromptResourceAddr = "coderd_chat_system_prompt.test" + +// fakeChatCoderd is a minimal stand-in for a Coder deployment, serving just +// the endpoints the provider touches: the two Configure() calls plus the chat +// system prompt singleton. +// +// A fake rather than `integration.StartCoder` for the same reason as the +// OAuth2 settings tests: most of the matrix is about what the *provider* does +// with a given API response, including that the server-side sanitization of +// the stored prompt does not surface as drift. The fake sanitizes on PUT +// exactly like coderd does, which is the behavior under test. +type fakeChatCoderd struct { + *httptest.Server + + mu sync.Mutex + prompt string + includeDflt bool + requests []fakeRequest + putPrompts []string + getStatus int + putStatus int + sanitizeOnPut bool +} + +func newFakeChatCoderd(t *testing.T) *fakeChatCoderd { + t.Helper() + + f := &fakeChatCoderd{includeDflt: true, sanitizeOnPut: true} + f.Server = httptest.NewServer(http.HandlerFunc(f.handle)) + t.Cleanup(f.Close) + return f +} + +func (f *fakeChatCoderd) handle(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + f.requests = append(f.requests, fakeRequest{Method: r.Method, Path: r.URL.Path}) + f.mu.Unlock() + + switch { + case r.URL.Path == "/api/v2/users/me": + writeJSON(w, http.StatusOK, map[string]any{ + "id": "00000000-0000-0000-0000-000000000001", + "username": "admin", + "organization_ids": []string{"00000000-0000-0000-0000-000000000002"}, + }) + case r.URL.Path == "/api/v2/entitlements": + writeJSON(w, http.StatusOK, codersdk.Entitlements{ + Features: map[codersdk.FeatureName]codersdk.Feature{}, + }) + case r.URL.Path == chatSystemPromptPath && r.Method == http.MethodGet: + f.mu.Lock() + status, prompt, include := f.getStatus, f.prompt, f.includeDflt + f.mu.Unlock() + if status != 0 { + writeJSON(w, status, codersdk.Response{Message: errorMessage(status, "")}) + return + } + writeJSON(w, http.StatusOK, codersdk.ChatSystemPromptResponse{ + SystemPrompt: prompt, + IncludeDefaultSystemPrompt: include, + DefaultSystemPrompt: "built-in prompt", + }) + case r.URL.Path == chatSystemPromptPath && r.Method == http.MethodPut: + f.mu.Lock() + status := f.putStatus + f.mu.Unlock() + if status != 0 { + writeJSON(w, status, codersdk.Response{Message: errorMessage(status, "")}) + return + } + var req codersdk.UpdateChatSystemPromptRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeJSON(w, http.StatusBadRequest, codersdk.Response{Message: "Bad Request."}) + return + } + f.mu.Lock() + f.putPrompts = append(f.putPrompts, req.SystemPrompt) + f.prompt = req.SystemPrompt + if f.sanitizeOnPut { + // Match coderd: the stored value is the sanitized value. + f.prompt = sanitizePromptText(req.SystemPrompt) + } + if req.IncludeDefaultSystemPrompt != nil { + f.includeDflt = *req.IncludeDefaultSystemPrompt + } + f.mu.Unlock() + w.WriteHeader(http.StatusNoContent) + default: + writeJSON(w, http.StatusNotFound, codersdk.Response{Message: "Not Found."}) + } +} + +func chatSystemPromptConfig(url, prompt string, includeDefault *bool) string { + include := "" + if includeDefault != nil { + include = fmt.Sprintf("\n\tinclude_default_system_prompt = %t", *includeDefault) + } + return oauth2SettingsProviderBlock(url) + fmt.Sprintf(` +resource "coderd_chat_system_prompt" "test" { + system_prompt = %q%s +} +`, prompt, include) +} + +// TestSanitizePromptText pins the local port of coderd's sanitizer to the +// upstream behavior it must mirror for semantic equality to be correct. +func TestSanitizePromptText(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + in string + want string + }{ + {"trailing newline", "prompt\n", "prompt"}, + {"crlf", "a\r\nb\rc", "a\nb\nc"}, + {"zero-width space", "a\u200bb", "ab"}, + {"zwj stripped", "a\u200db", "ab"}, + {"zwnj preserved", "a\u200cb", "a\u200cb"}, + {"bom", "\ufeffprompt", "prompt"}, + {"collapse blank lines", "a\n\n\n\nb", "a\n\nb"}, + {"trailing line whitespace", "a \nb", "a\nb"}, + {"leading indentation preserved", "a\n b", "a\n b"}, + {"idempotent", " a\u200b\n\n\n\nb \n", "a\n\nb"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := sanitizePromptText(tc.in) + require.Equal(t, tc.want, got) + // Sanitization must be idempotent: the server stores the + // sanitized form, and Read compares it against the config's + // sanitized form. + require.Equal(t, got, sanitizePromptText(got)) + }) + } +} + +// TestChatSystemPromptSemanticEquals covers the custom type directly: two +// prompts are the same setting iff they sanitize to the same string. +func TestChatSystemPromptSemanticEquals(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + for _, tc := range []struct { + name string + a, b string + want bool + }{ + {"identical", "prompt", "prompt", true}, + {"trailing newline", "prompt\n", "prompt", true}, + {"crlf vs lf", "a\r\nb", "a\nb", true}, + {"different text", "prompt a", "prompt b", false}, + {"whitespace-only difference inside a line", "a b", "a b", false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, diags := newChatSystemPromptTextValue(tc.a).StringSemanticEquals(ctx, newChatSystemPromptTextValue(tc.b)) + require.False(t, diags.HasError()) + require.Equal(t, tc.want, got) + }) + } +} + +func TestChatSystemPromptLengthValidator(t *testing.T) { + t.Parallel() + + f := newFakeChatCoderd(t) + + // The sanitized form is what the server measures, so padding that + // sanitizes away must not trip the validator. + okPrompt := strings.Repeat("a", maxChatSystemPromptBytes) + "\n\n\n" + tooLong := strings.Repeat("a", maxChatSystemPromptBytes+1) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: chatSystemPromptConfig(f.URL, tooLong, nil), + ExpectError: regexp.MustCompile("System Prompt Too Long"), + }, + { + Config: chatSystemPromptConfig(f.URL, okPrompt, nil), + }, + }, + }) +} + +// TestAccChatSystemPromptResource exercises the full lifecycle against the +// fake: create, refresh without drift despite server-side sanitization, +// update, and destroy resetting the deployment defaults. +func TestAccChatSystemPromptResource(t *testing.T) { + t.Parallel() + + f := newFakeChatCoderd(t) + includeFalse := false + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + // Create with a trailing newline, the `file()` everyday case. The + // fake stores the sanitized (trimmed) form, like coderd does. + { + Config: chatSystemPromptConfig(f.URL, "You are a helpful agent.\n", nil), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue( + chatSystemPromptResourceAddr, + tfjsonpath.New("include_default_system_prompt"), + knownvalue.Bool(true), + ), + }, + }, + // Re-planning the same config must be empty: the live value is + // the sanitized form, which is semantically equal. + { + Config: chatSystemPromptConfig(f.URL, "You are a helpful agent.\n", nil), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + // A real edit must show up and apply. + { + Config: chatSystemPromptConfig(f.URL, "You are a very helpful agent.\n", &includeFalse), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue( + chatSystemPromptResourceAddr, + tfjsonpath.New("include_default_system_prompt"), + knownvalue.Bool(false), + ), + }, + }, + }, + }) + + // Destroy (run by resource.Test after the last step) resets the + // deployment defaults rather than stranding the last-applied value. + f.mu.Lock() + defer f.mu.Unlock() + require.Empty(t, f.prompt) + require.True(t, f.includeDflt) +} + +// TestAccChatSystemPromptImport adopts a live out-of-band prompt without +// issuing any PUT. +func TestAccChatSystemPromptImport(t *testing.T) { + t.Parallel() + + f := newFakeChatCoderd(t) + f.mu.Lock() + f.prompt = "configured in the dashboard" + f.mu.Unlock() + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: chatSystemPromptConfig(f.URL, "configured in the dashboard", nil), + ResourceName: chatSystemPromptResourceAddr, + ImportState: true, + ImportStatePersist: true, + // The ID is required by the CLI syntax but unused. + ImportStateId: "chat_system_prompt", + }, + // After import, the matching config plans clean: nothing to + // overwrite, nothing to PUT. + { + Config: chatSystemPromptConfig(f.URL, "configured in the dashboard", nil), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) + + // Adopting an existing value must never write it back. The only PUT in + // the whole test is the framework's final `terraform destroy`, which + // resets the prompt to empty. + f.mu.Lock() + defer f.mu.Unlock() + require.Equal(t, []string{""}, f.putPrompts) +} + +// TestAccChatSystemPromptUnsupportedVersion pins the 404-to-version-hint +// mapping: an old deployment produces one actionable error. +func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { + t.Parallel() + + f := newFakeChatCoderd(t) + f.mu.Lock() + f.getStatus = http.StatusNotFound + f.putStatus = http.StatusNotFound + f.mu.Unlock() + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: chatSystemPromptConfig(f.URL, "prompt", nil), + // Terraform wraps error text, so match the summary line only. + ExpectError: regexp.MustCompile("Unsupported Coder Version"), + }, + }, + }) +} diff --git a/internal/provider/chat_system_prompt_sanitize.go b/internal/provider/chat_system_prompt_sanitize.go new file mode 100644 index 0000000..6ce428a --- /dev/null +++ b/internal/provider/chat_system_prompt_sanitize.go @@ -0,0 +1,123 @@ +package provider + +import ( + "strings" + "unicode" +) + +// sanitizePromptText mirrors coderd's chatd.SanitizePromptText +// (coderd/x/chatd/sanitize.go in coder/coder): it strips invisible +// Unicode characters, normalizes line endings, collapses excessive +// blank lines, and trims surrounding whitespace. +// +// The chat system prompt endpoint stores the sanitized form of +// whatever is PUT to it, so the value read back rarely matches the +// configured value byte-for-byte (a trailing newline from +// `file("system-prompt.md")` is the everyday case). This local copy +// exists so `system_prompt` can compare semantically: two values are +// the same setting iff they sanitize to the same string. The logic is +// deliberately a straight port; if the upstream sanitizer changes, the +// worst case is a visible (loud) diff on the next plan rather than +// silent drift. +func sanitizePromptText(s string) string { + // 1. Normalize line endings. + s = strings.ReplaceAll(s, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + + // 2. Strip invisible characters rune-by-rune. + var b strings.Builder + b.Grow(len(s)) + for _, r := range s { + if !isVisiblePromptRune(r) { + continue + } + _, _ = b.WriteRune(r) + } + s = b.String() + + // 3. Collapse 3+ consecutive newlines down to 2. + s = collapsePromptNewlines(s) + + // 4. Final trim. + return strings.TrimSpace(s) +} + +// isVisiblePromptRune reports whether r survives coderd's prompt +// sanitization. The codepoint list matches chatd.isVisible upstream: +// an explicit list rather than blanket unicode.Cf stripping, so +// legitimate format characters (e.g. subdivision flag emoji) survive. +func isVisiblePromptRune(r rune) bool { + switch { + // Soft hyphen. + case r == 0x00AD: + return false + // Combining grapheme joiner. + case r == 0x034F: + return false + // Arabic letter mark. + case r == 0x061C: + return false + // Mongolian vowel separator. + case r == 0x180E: + return false + // Zero-width space. + case r == 0x200B: + return false + // U+200C (ZWNJ) is deliberately NOT stripped, matching upstream: + // it is required for correct rendering of Persian, Urdu, and + // Kurdish scripts. + // Zero-width joiner. + case r == 0x200D: + return false + // Left-to-right and right-to-left marks. + case r == 0x200E, r == 0x200F: + return false + // Bidi embedding and override controls: LRE, RLE, PDF, LRO, RLO. + case r >= 0x202A && r <= 0x202E: + return false + // Word joiner and invisible operators. + case r >= 0x2060 && r <= 0x2064: + return false + // Bidi isolate controls: LRI, RLI, FSI, PDI. + case r >= 0x2066 && r <= 0x2069: + return false + // Deprecated format characters. + case r >= 0x206A && r <= 0x206F: + return false + // Byte order mark / zero-width no-break space. + case r == 0xFEFF: + return false + // Interlinear annotation anchor, separator, and terminator. + case r >= 0xFFF9 && r <= 0xFFFB: + return false + default: + return true + } +} + +// collapsePromptNewlines trims trailing whitespace from each line, +// then replaces runs of 3 or more consecutive newlines with exactly 2, +// matching chatd.collapseNewlines upstream. +func collapsePromptNewlines(s string) string { + lines := strings.Split(s, "\n") + for i, line := range lines { + lines[i] = strings.TrimRightFunc(line, unicode.IsSpace) + } + s = strings.Join(lines, "\n") + + var b strings.Builder + b.Grow(len(s)) + consecutiveNewlines := 0 + for _, r := range s { + if r == '\n' { + consecutiveNewlines++ + if consecutiveNewlines <= 2 { + _, _ = b.WriteRune(r) + } + continue + } + consecutiveNewlines = 0 + _, _ = b.WriteRune(r) + } + return b.String() +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 7177a46..3ff4440 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -236,6 +236,7 @@ func (p *CoderdProvider) Resources(ctx context.Context) []func() resource.Resour NewAIProviderResource, NewAgentsModelResource, NewDefaultAgentsModelResource, + NewChatSystemPromptResource, NewOAuth2ProviderSettingsResource, } } From 494fdd47f2d621b2cb1f33677457f390acc527c1 Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Tue, 18 Aug 2026 01:30:29 +0000 Subject: [PATCH 2/6] test(coderd_chat_system_prompt): follow repo test conventions - Gate the fake-server TestAcc tests on TF_ACC with testAccPreCheck, matching every other TestAcc in the repo. - Rewrite the length-validator test as a direct ValidateString unit test; ungated Test* functions here do not spin up the Terraform CLI. - Add real-Coder acceptance tests via integration.StartCoder, the dominant pattern for resources the stock coder image serves (the fake-only approach is justified for oauth2_provider_settings because its endpoint needs unreleased Coder; that does not apply here). The no-drift test is the live proof the sanitizer port matches the server: the prompt carries CRLF, a zero-width space, a blank-line run, and a trailing newline, and the re-plan must be empty. The import test pins both convergence behaviors: a byte-matching config plans clean immediately; a config differing only by sanitization applies one normalization update and then converges. - Document the one-time post-import normalization update and the trimspace(file(...)) escape hatch in the resource description. No license required: the endpoint has no entitlement check, so UseLicense would only cause needless skips on fork PRs. --- docs/resources/chat_system_prompt.md | 4 +- .../provider/chat_system_prompt_resource.go | 2 +- .../chat_system_prompt_resource_test.go | 188 ++++++++++++++++-- 3 files changed, 177 insertions(+), 17 deletions(-) diff --git a/docs/resources/chat_system_prompt.md b/docs/resources/chat_system_prompt.md index b7de0c9..ffa3464 100644 --- a/docs/resources/chat_system_prompt.md +++ b/docs/resources/chat_system_prompt.md @@ -6,7 +6,7 @@ description: |- ~> This resource is experimental. Changes are to be expected, and we recommend using it with caution in production environments. The deployment-wide chat system prompt for Coder Agents (Settings → Instructions in the dashboard). This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. - Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from file(...) does not cause drift. + Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from file(...) does not cause drift after apply. On the first plan after an import, a configured value that differs from the live one only by sanitization shows a single in-place normalization update and then converges; use trimspace(file(...)) to avoid even that. ~> Warning If a system prompt was configured out of band, terraform import this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. ~> Warning @@ -23,7 +23,7 @@ The deployment-wide chat system prompt for Coder Agents (`Settings → Instructi This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. -Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from `file(...)` does not cause drift. +Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from `file(...)` does not cause drift after apply. On the first plan after an import, a configured value that differs from the live one only by sanitization shows a single in-place normalization update and then converges; use `trimspace(file(...))` to avoid even that. ~> **Warning** If a system prompt was configured out of band, `terraform import` this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go index 66b77b9..761e8e9 100644 --- a/internal/provider/chat_system_prompt_resource.go +++ b/internal/provider/chat_system_prompt_resource.go @@ -66,7 +66,7 @@ The deployment-wide chat system prompt for Coder Agents (` + "`Settings → Inst This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. -Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from ` + "`file(...)`" + ` does not cause drift. +Coder sanitizes the stored prompt (strips invisible Unicode characters, normalizes line endings, collapses runs of blank lines, and trims surrounding whitespace), and this resource compares values the same way, so a trailing newline from ` + "`file(...)`" + ` does not cause drift after apply. On the first plan after an import, a configured value that differs from the live one only by sanitization shows a single in-place normalization update and then converges; use ` + "`trimspace(file(...))`" + ` to avoid even that. ~> **Warning** If a system prompt was configured out of band, ` + "`terraform import`" + ` this resource before the first apply. Otherwise Terraform overwrites the live value; a plan-time warning is emitted when this is about to happen. diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go index eb30447..414cd49 100644 --- a/internal/provider/chat_system_prompt_resource_test.go +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -5,12 +5,16 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" "regexp" "strings" "sync" "testing" "github.com/coder/coder/v2/codersdk" + "github.com/coder/terraform-provider-coderd/integration" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" @@ -189,26 +193,33 @@ func TestChatSystemPromptSemanticEquals(t *testing.T) { func TestChatSystemPromptLengthValidator(t *testing.T) { t.Parallel() - f := newFakeChatCoderd(t) + ctx := t.Context() // The sanitized form is what the server measures, so padding that // sanitizes away must not trip the validator. okPrompt := strings.Repeat("a", maxChatSystemPromptBytes) + "\n\n\n" tooLong := strings.Repeat("a", maxChatSystemPromptBytes+1) - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: chatSystemPromptConfig(f.URL, tooLong, nil), - ExpectError: regexp.MustCompile("System Prompt Too Long"), - }, - { - Config: chatSystemPromptConfig(f.URL, okPrompt, nil), - }, - }, - }) + for _, tc := range []struct { + name string + value basetypes.StringValue + wantErr bool + }{ + {"at limit after sanitization", basetypes.NewStringValue(okPrompt), false}, + {"over limit", basetypes.NewStringValue(tooLong), true}, + {"null", basetypes.NewStringNull(), false}, + {"unknown", basetypes.NewStringUnknown(), false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + resp := &validator.StringResponse{} + chatSystemPromptLengthValidator{}.ValidateString(ctx, validator.StringRequest{ + Path: pathSystemPrompt, + ConfigValue: tc.value, + }, resp) + require.Equal(t, tc.wantErr, resp.Diagnostics.HasError()) + }) + } } // TestAccChatSystemPromptResource exercises the full lifecycle against the @@ -216,12 +227,16 @@ func TestChatSystemPromptLengthValidator(t *testing.T) { // update, and destroy resetting the deployment defaults. func TestAccChatSystemPromptResource(t *testing.T) { t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } f := newFakeChatCoderd(t) includeFalse := false resource.Test(t, resource.TestCase{ IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ // Create with a trailing newline, the `file()` everyday case. The @@ -272,6 +287,9 @@ func TestAccChatSystemPromptResource(t *testing.T) { // issuing any PUT. func TestAccChatSystemPromptImport(t *testing.T) { t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } f := newFakeChatCoderd(t) f.mu.Lock() @@ -280,6 +298,7 @@ func TestAccChatSystemPromptImport(t *testing.T) { resource.Test(t, resource.TestCase{ IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { @@ -315,6 +334,9 @@ func TestAccChatSystemPromptImport(t *testing.T) { // mapping: an old deployment produces one actionable error. func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } f := newFakeChatCoderd(t) f.mu.Lock() @@ -324,6 +346,7 @@ func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { resource.Test(t, resource.TestCase{ IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { @@ -334,3 +357,140 @@ func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { }, }) } + +// TestAccChatSystemPromptRealCoderNoDrift runs the lifecycle against a real +// Coder instance. This is the live proof that the local sanitizer port +// matches the server's: the configured prompt deliberately carries a CRLF, a +// zero-width space, a run of blank lines, and a trailing newline, so if +// coderd's sanitization ever diverges from sanitizePromptText, the re-plan +// stops being empty. +func TestAccChatSystemPromptRealCoderNoDrift(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + ctx := t.Context() + client := integration.StartCoder(ctx, t, "chat_system_prompt_acc") + experimental := codersdk.NewExperimentalClient(client) + + messyPrompt := "You are a helpful agent.\r\nBe concise.\u200b\n\n\n\nAlways cite sources.\n" + + cfg := fmt.Sprintf(` +provider "coderd" { + url = %[1]q + token = %[2]q +} + +resource "coderd_chat_system_prompt" "test" { + system_prompt = %[3]q +} +`, client.URL.String(), client.SessionToken(), messyPrompt) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: cfg, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue( + chatSystemPromptResourceAddr, + tfjsonpath.New("include_default_system_prompt"), + knownvalue.Bool(true), + ), + }, + }, + // Re-planning the identical config must yield an empty plan: the + // live value is whatever the real server sanitized and stored. + { + Config: cfg, + PlanOnly: true, + }, + }, + }) + + // The server must have stored the sanitized form, and the test-framework + // destroy after the last step must have reset the deployment defaults. + live, err := experimental.GetChatSystemPrompt(ctx) + require.NoError(t, err) + require.Empty(t, live.SystemPrompt) + require.True(t, live.IncludeDefaultSystemPrompt) +} + +// TestAccChatSystemPromptRealCoderImportNoDrift proves that adopting a prompt +// configured out of band (via the API, as the dashboard would) and re-planning +// the matching config is a clean, empty plan. +// +// The empty plan requires the config to byte-match the stored (sanitized) +// value: semantic equality preserves prior state on Read and Apply, but a +// Required attribute's planned value must equal config, so a config that +// differs only by sanitization shows one in-place normalization update after +// import and then converges. Both behaviors are pinned here. +func TestAccChatSystemPromptRealCoderImportNoDrift(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + ctx := t.Context() + client := integration.StartCoder(ctx, t, "chat_system_prompt_import_acc") + experimental := codersdk.NewExperimentalClient(client) + + // Configured out of band, so import (not a prior apply) seeds state. + includeDefault := true + require.NoError(t, experimental.UpdateChatSystemPrompt(ctx, codersdk.UpdateChatSystemPromptRequest{ + SystemPrompt: "configured in the dashboard", + IncludeDefaultSystemPrompt: &includeDefault, + })) + + providerBlock := fmt.Sprintf(` +provider "coderd" { + url = %[1]q + token = %[2]q +} +`, client.URL.String(), client.SessionToken()) + + // Byte-matches the stored value, like trimspace(file(...)) would. + cfgExact := providerBlock + ` +resource "coderd_chat_system_prompt" "test" { + system_prompt = "configured in the dashboard" +} +` + // Differs only by a trailing newline, like a bare file(...) would. + cfgTrailingNewline := providerBlock + ` +resource "coderd_chat_system_prompt" "test" { + system_prompt = "configured in the dashboard\n" +} +` + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: cfgExact, + ResourceName: chatSystemPromptResourceAddr, + ImportState: true, + ImportStatePersist: true, + ImportStateId: "chat_system_prompt", + }, + // A config that byte-matches the stored value plans clean. + { + Config: cfgExact, + PlanOnly: true, + }, + // A config that differs only by sanitization applies one + // normalization update... + { + Config: cfgTrailingNewline, + }, + // ...and then converges: refreshes keep the configured value via + // semantic equality, so the re-plan is empty. + { + Config: cfgTrailingNewline, + PlanOnly: true, + }, + }, + }) +} From 02e26f242eafa0fc5923412bb728ece058482d80 Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Tue, 18 Aug 2026 22:59:08 +0000 Subject: [PATCH 3/6] fix(internal/provider): warn when create would flip include_default_system_prompt Per review, the create-time overwrite advisory now also fires when the first apply would change include_default_system_prompt away from the deployment's live value, not just when it would overwrite a non-empty prompt. Covered by a direct ModifyPlan table test in the oauth2_provider_settings style. Also leaves a TODO on the mirrored sanitizer pointing at coder/coder#28283, which exports it as codersdk.SanitizePromptText; once the pinned coder/coder includes that commit the local copy goes away. --- .../provider/chat_system_prompt_resource.go | 54 +++--- .../chat_system_prompt_resource_test.go | 163 ++++++++++++++++++ .../provider/chat_system_prompt_sanitize.go | 5 + 3 files changed, 202 insertions(+), 20 deletions(-) diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go index 761e8e9..9ced6a0 100644 --- a/internal/provider/chat_system_prompt_resource.go +++ b/internal/provider/chat_system_prompt_resource.go @@ -36,9 +36,12 @@ const chatSystemPromptMinVersion = "2.32.0" // time; validating here fails the same way at plan time instead. const maxChatSystemPromptBytes = 131072 -// pathSystemPrompt anchors plan-time diagnostics to the attribute they are -// about. -var pathSystemPrompt = path.Root("system_prompt") +// pathSystemPrompt and pathIncludeDefaultSystemPrompt anchor plan-time +// diagnostics to the attributes they are about. +var ( + pathSystemPrompt = path.Root("system_prompt") + pathIncludeDefaultSystemPrompt = path.Root("include_default_system_prompt") +) type ChatSystemPromptResource struct { *CoderdProviderData @@ -233,8 +236,9 @@ func (r *ChatSystemPromptResource) Delete(ctx context.Context, req resource.Dele } // ModifyPlan emits the standard experimental-resource warning and, on a first -// apply, warns when a non-empty out-of-band prompt is about to be overwritten, -// giving the admin a chance to `terraform import` instead. +// apply, warns when a non-empty out-of-band prompt or a differing +// include-default flag is about to be overwritten, giving the admin a chance +// to `terraform import` instead. func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { resp.Diagnostics.AddWarning( "Experimental Resource", @@ -277,23 +281,33 @@ func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource. }) return } - if live.SystemPrompt == "" { - // Nothing configured out of band; nothing to lose. - return - } - if sanitizePromptText(live.SystemPrompt) == sanitizePromptText(data.SystemPrompt.ValueString()) { - // The planned prompt matches the live one. - return + + if live.SystemPrompt != "" && + sanitizePromptText(live.SystemPrompt) != sanitizePromptText(data.SystemPrompt.ValueString()) { + resp.Diagnostics.AddAttributeWarning( + pathSystemPrompt, + "Overwriting an out-of-band value", + "This deployment already has a chat system prompt configured, and applying will overwrite it. "+ + "Terraform has no prior state for this resource, so this change is not shown as a diff.\n\n"+ + "If you meant to adopt the deployment's existing value rather than overwrite it, run "+ + "`terraform import coderd_chat_system_prompt. chat_system_prompt` first.", + ) } - resp.Diagnostics.AddAttributeWarning( - pathSystemPrompt, - "Overwriting an out-of-band value", - "This deployment already has a chat system prompt configured, and applying will overwrite it. "+ - "Terraform has no prior state for this resource, so this change is not shown as a diff.\n\n"+ - "If you meant to adopt the deployment's existing value rather than overwrite it, run "+ - "`terraform import coderd_chat_system_prompt. chat_system_prompt` first.", - ) + // The attribute has a schema default, so it is only unknown when it comes + // from an unresolved expression. + if !data.IncludeDefaultSystemPrompt.IsUnknown() && !data.IncludeDefaultSystemPrompt.IsNull() && + data.IncludeDefaultSystemPrompt.ValueBool() != live.IncludeDefaultSystemPrompt { + resp.Diagnostics.AddAttributeWarning( + pathIncludeDefaultSystemPrompt, + "Overwriting an out-of-band value", + fmt.Sprintf("`include_default_system_prompt` is currently `%t` on this deployment, and applying will set it "+ + "to `%t`. Terraform has no prior state for this resource, so this change is not shown as a diff.\n\n"+ + "If you meant to adopt the deployment's existing value rather than overwrite it, run "+ + "`terraform import coderd_chat_system_prompt. chat_system_prompt` first.", + live.IncludeDefaultSystemPrompt, data.IncludeDefaultSystemPrompt.ValueBool()), + ) + } } func (r *ChatSystemPromptResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go index 414cd49..c8e763d 100644 --- a/internal/provider/chat_system_prompt_resource_test.go +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "os" "regexp" "strings" @@ -13,13 +14,17 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/terraform-provider-coderd/integration" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/statecheck" "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -494,3 +499,161 @@ resource "coderd_chat_system_prompt" "test" { }, }) } + +// TestChatSystemPromptModifyPlan covers the create-time overwrite advisories +// for both attributes. Every case carries the always-on experimental-resource +// warning, so the baseline warning count is 1. +func TestChatSystemPromptModifyPlan(t *testing.T) { + t.Parallel() + + objType := tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "system_prompt": tftypes.String, + "include_default_system_prompt": tftypes.Bool, + }, + } + // promptObject builds a state/plan value. A nil prompt yields a null + // object, which is how the framework signals "no prior state" (a create) + // and "no plan" (a destroy). + promptObject := func(prompt any, include any) tftypes.Value { + if prompt == nil && include == nil { + return tftypes.NewValue(objType, nil) + } + return tftypes.NewValue(objType, map[string]tftypes.Value{ + "system_prompt": tftypes.NewValue(tftypes.String, prompt), + "include_default_system_prompt": tftypes.NewValue(tftypes.Bool, include), + }) + } + nullObject := promptObject(nil, nil) + + for _, tc := range []struct { + name string + // livePrompt and liveInclude are the deployment's current values. + livePrompt string + liveInclude bool + // lookupStatus, when non-zero, makes the GET fail. + lookupStatus int + plan tftypes.Value + state tftypes.Value + wantWarnings int + }{ + { + name: "WarnsWhenFirstApplyWouldOverwriteLivePrompt", + livePrompt: "configured in the dashboard", + liveInclude: true, + plan: promptObject("from terraform", true), + state: nullObject, + wantWarnings: 2, + }, + { + name: "WarnsWhenFirstApplyWouldFlipIncludeDefault", + livePrompt: "", + liveInclude: false, + plan: promptObject("from terraform", true), + state: nullObject, + wantWarnings: 2, + }, + { + name: "WarnsOnBothWhenBothDiffer", + livePrompt: "configured in the dashboard", + liveInclude: false, + plan: promptObject("from terraform", true), + state: nullObject, + wantWarnings: 3, + }, + { + // A live prompt differing from the plan only by sanitization is + // the same setting, not an overwrite. + name: "SilentWhenPromptMatchesModuloSanitization", + livePrompt: "from terraform", + liveInclude: true, + plan: promptObject("from terraform\n", true), + state: nullObject, + wantWarnings: 1, + }, + { + // A never-configured deployment has nothing to lose. + name: "SilentOnGreenfieldDeployment", + livePrompt: "", + liveInclude: true, + plan: promptObject("from terraform", true), + state: nullObject, + wantWarnings: 1, + }, + { + // Not a create: an update already renders a real diff, and this + // is also the first plan after `terraform import`. + name: "SilentWhenPriorStateExists", + livePrompt: "configured in the dashboard", + liveInclude: false, + plan: promptObject("from terraform", true), + state: promptObject("from terraform", true), + wantWarnings: 1, + }, + { + name: "SilentOnDestroyPlan", + livePrompt: "configured in the dashboard", + liveInclude: false, + plan: nullObject, + state: promptObject("from terraform", true), + wantWarnings: 1, + }, + { + // A Required attribute is still unknown when it comes from an + // input variable or module output. Defer rather than guess. + name: "SilentWhenPlannedPromptUnknown", + livePrompt: "configured in the dashboard", + liveInclude: false, + plan: promptObject(tftypes.UnknownValue, true), + state: nullObject, + wantWarnings: 1, + }, + { + // Best-effort: a failed lookup must not turn into a plan error. + // Create() makes the same call and reports it properly. + name: "SilentWhenLookupFails", + livePrompt: "configured in the dashboard", + liveInclude: false, + lookupStatus: http.StatusForbidden, + plan: promptObject("from terraform", true), + state: nullObject, + wantWarnings: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + f := newFakeChatCoderd(t) + f.mu.Lock() + f.prompt = tc.livePrompt + f.includeDflt = tc.liveInclude + f.getStatus = tc.lookupStatus + f.mu.Unlock() + + serverURL, err := url.Parse(f.URL) + require.NoError(t, err) + client := codersdk.New(serverURL) + client.SetSessionToken("test-token") + + r := &ChatSystemPromptResource{ + CoderdProviderData: &CoderdProviderData{Client: client}, + } + + schemaResp := &fwresource.SchemaResponse{} + r.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) + require.Empty(t, schemaResp.Diagnostics) + s := schemaResp.Schema + + resp := &fwresource.ModifyPlanResponse{Plan: tfsdk.Plan{Schema: s, Raw: tc.plan}} + r.ModifyPlan(ctx, fwresource.ModifyPlanRequest{ + Config: tfsdk.Config{Schema: s, Raw: tc.plan}, + Plan: tfsdk.Plan{Schema: s, Raw: tc.plan}, + State: tfsdk.State{Schema: s, Raw: tc.state}, + }, resp) + + assert.Empty(t, resp.Diagnostics.Errors(), "a plan-time advisory must never fail the plan") + assert.Len(t, resp.Diagnostics.Warnings(), tc.wantWarnings) + }) + } +} diff --git a/internal/provider/chat_system_prompt_sanitize.go b/internal/provider/chat_system_prompt_sanitize.go index 6ce428a..c1c15eb 100644 --- a/internal/provider/chat_system_prompt_sanitize.go +++ b/internal/provider/chat_system_prompt_sanitize.go @@ -10,6 +10,11 @@ import ( // Unicode characters, normalizes line endings, collapses excessive // blank lines, and trims surrounding whitespace. // +// TODO(https://github.com/coder/coder/pull/28283): the sanitizer is +// being exported as codersdk.SanitizePromptText. Once the pinned +// coder/coder dependency includes that commit, delete this file and +// use the codersdk function so the two cannot drift. +// // The chat system prompt endpoint stores the sanitized form of // whatever is PUT to it, so the value read back rarely matches the // configured value byte-for-byte (a trailing newline from From 7bcd352109efbefef83736db3f038ffb556fa825 Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Wed, 19 Aug 2026 14:00:24 +0000 Subject: [PATCH 4/6] refactor(internal/provider): use codersdk.SanitizePromptText Deletes the mirrored sanitizer now that coder/coder#28283 exports it from codersdk and the pinned dependency includes it. Semantic equality, the plan-time length validator, and the create-time overwrite advisory all call the codersdk function directly, so the provider and server can no longer drift. The local parity unit test goes with it (the function's tests moved to codersdk upstream); the real-Coder no-drift acceptance test remains the end-to-end guard that the pinned SDK's sanitizer matches the deployed server. --- .../provider/chat_system_prompt_resource.go | 6 +- .../chat_system_prompt_resource_test.go | 45 +----- .../provider/chat_system_prompt_sanitize.go | 128 ------------------ 3 files changed, 9 insertions(+), 170 deletions(-) delete mode 100644 internal/provider/chat_system_prompt_sanitize.go diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go index 9ced6a0..b65375f 100644 --- a/internal/provider/chat_system_prompt_resource.go +++ b/internal/provider/chat_system_prompt_resource.go @@ -283,7 +283,7 @@ func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource. } if live.SystemPrompt != "" && - sanitizePromptText(live.SystemPrompt) != sanitizePromptText(data.SystemPrompt.ValueString()) { + codersdk.SanitizePromptText(live.SystemPrompt) != codersdk.SanitizePromptText(data.SystemPrompt.ValueString()) { resp.Diagnostics.AddAttributeWarning( pathSystemPrompt, "Overwriting an out-of-band value", @@ -371,7 +371,7 @@ func (v chatSystemPromptLengthValidator) ValidateString(ctx context.Context, req if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { return } - if got := len(sanitizePromptText(req.ConfigValue.ValueString())); got > maxChatSystemPromptBytes { + if got := len(codersdk.SanitizePromptText(req.ConfigValue.ValueString())); got > maxChatSystemPromptBytes { resp.Diagnostics.AddAttributeError( req.Path, "System Prompt Too Long", @@ -461,5 +461,5 @@ func (v chatSystemPromptTextValue) StringSemanticEquals(ctx context.Context, new ) return false, diags } - return sanitizePromptText(v.ValueString()) == sanitizePromptText(newValue.ValueString()), diags + return codersdk.SanitizePromptText(v.ValueString()) == codersdk.SanitizePromptText(newValue.ValueString()), diags } diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go index c8e763d..0aa42bb 100644 --- a/internal/provider/chat_system_prompt_resource_test.go +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -111,7 +111,7 @@ func (f *fakeChatCoderd) handle(w http.ResponseWriter, r *http.Request) { f.prompt = req.SystemPrompt if f.sanitizeOnPut { // Match coderd: the stored value is the sanitized value. - f.prompt = sanitizePromptText(req.SystemPrompt) + f.prompt = codersdk.SanitizePromptText(req.SystemPrompt) } if req.IncludeDefaultSystemPrompt != nil { f.includeDflt = *req.IncludeDefaultSystemPrompt @@ -135,39 +135,6 @@ resource "coderd_chat_system_prompt" "test" { `, prompt, include) } -// TestSanitizePromptText pins the local port of coderd's sanitizer to the -// upstream behavior it must mirror for semantic equality to be correct. -func TestSanitizePromptText(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - name string - in string - want string - }{ - {"trailing newline", "prompt\n", "prompt"}, - {"crlf", "a\r\nb\rc", "a\nb\nc"}, - {"zero-width space", "a\u200bb", "ab"}, - {"zwj stripped", "a\u200db", "ab"}, - {"zwnj preserved", "a\u200cb", "a\u200cb"}, - {"bom", "\ufeffprompt", "prompt"}, - {"collapse blank lines", "a\n\n\n\nb", "a\n\nb"}, - {"trailing line whitespace", "a \nb", "a\nb"}, - {"leading indentation preserved", "a\n b", "a\n b"}, - {"idempotent", " a\u200b\n\n\n\nb \n", "a\n\nb"}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got := sanitizePromptText(tc.in) - require.Equal(t, tc.want, got) - // Sanitization must be idempotent: the server stores the - // sanitized form, and Read compares it against the config's - // sanitized form. - require.Equal(t, got, sanitizePromptText(got)) - }) - } -} - // TestChatSystemPromptSemanticEquals covers the custom type directly: two // prompts are the same setting iff they sanitize to the same string. func TestChatSystemPromptSemanticEquals(t *testing.T) { @@ -364,11 +331,11 @@ func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { } // TestAccChatSystemPromptRealCoderNoDrift runs the lifecycle against a real -// Coder instance. This is the live proof that the local sanitizer port -// matches the server's: the configured prompt deliberately carries a CRLF, a -// zero-width space, a run of blank lines, and a trailing newline, so if -// coderd's sanitization ever diverges from sanitizePromptText, the re-plan -// stops being empty. +// Coder instance. This is the live proof that the pinned SDK's sanitizer +// matches the deployed server's: the configured prompt deliberately carries a +// CRLF, a zero-width space, a run of blank lines, and a trailing newline, so +// if the deployment's sanitization ever diverges from the pinned +// codersdk.SanitizePromptText, the re-plan stops being empty. func TestAccChatSystemPromptRealCoderNoDrift(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { diff --git a/internal/provider/chat_system_prompt_sanitize.go b/internal/provider/chat_system_prompt_sanitize.go deleted file mode 100644 index c1c15eb..0000000 --- a/internal/provider/chat_system_prompt_sanitize.go +++ /dev/null @@ -1,128 +0,0 @@ -package provider - -import ( - "strings" - "unicode" -) - -// sanitizePromptText mirrors coderd's chatd.SanitizePromptText -// (coderd/x/chatd/sanitize.go in coder/coder): it strips invisible -// Unicode characters, normalizes line endings, collapses excessive -// blank lines, and trims surrounding whitespace. -// -// TODO(https://github.com/coder/coder/pull/28283): the sanitizer is -// being exported as codersdk.SanitizePromptText. Once the pinned -// coder/coder dependency includes that commit, delete this file and -// use the codersdk function so the two cannot drift. -// -// The chat system prompt endpoint stores the sanitized form of -// whatever is PUT to it, so the value read back rarely matches the -// configured value byte-for-byte (a trailing newline from -// `file("system-prompt.md")` is the everyday case). This local copy -// exists so `system_prompt` can compare semantically: two values are -// the same setting iff they sanitize to the same string. The logic is -// deliberately a straight port; if the upstream sanitizer changes, the -// worst case is a visible (loud) diff on the next plan rather than -// silent drift. -func sanitizePromptText(s string) string { - // 1. Normalize line endings. - s = strings.ReplaceAll(s, "\r\n", "\n") - s = strings.ReplaceAll(s, "\r", "\n") - - // 2. Strip invisible characters rune-by-rune. - var b strings.Builder - b.Grow(len(s)) - for _, r := range s { - if !isVisiblePromptRune(r) { - continue - } - _, _ = b.WriteRune(r) - } - s = b.String() - - // 3. Collapse 3+ consecutive newlines down to 2. - s = collapsePromptNewlines(s) - - // 4. Final trim. - return strings.TrimSpace(s) -} - -// isVisiblePromptRune reports whether r survives coderd's prompt -// sanitization. The codepoint list matches chatd.isVisible upstream: -// an explicit list rather than blanket unicode.Cf stripping, so -// legitimate format characters (e.g. subdivision flag emoji) survive. -func isVisiblePromptRune(r rune) bool { - switch { - // Soft hyphen. - case r == 0x00AD: - return false - // Combining grapheme joiner. - case r == 0x034F: - return false - // Arabic letter mark. - case r == 0x061C: - return false - // Mongolian vowel separator. - case r == 0x180E: - return false - // Zero-width space. - case r == 0x200B: - return false - // U+200C (ZWNJ) is deliberately NOT stripped, matching upstream: - // it is required for correct rendering of Persian, Urdu, and - // Kurdish scripts. - // Zero-width joiner. - case r == 0x200D: - return false - // Left-to-right and right-to-left marks. - case r == 0x200E, r == 0x200F: - return false - // Bidi embedding and override controls: LRE, RLE, PDF, LRO, RLO. - case r >= 0x202A && r <= 0x202E: - return false - // Word joiner and invisible operators. - case r >= 0x2060 && r <= 0x2064: - return false - // Bidi isolate controls: LRI, RLI, FSI, PDI. - case r >= 0x2066 && r <= 0x2069: - return false - // Deprecated format characters. - case r >= 0x206A && r <= 0x206F: - return false - // Byte order mark / zero-width no-break space. - case r == 0xFEFF: - return false - // Interlinear annotation anchor, separator, and terminator. - case r >= 0xFFF9 && r <= 0xFFFB: - return false - default: - return true - } -} - -// collapsePromptNewlines trims trailing whitespace from each line, -// then replaces runs of 3 or more consecutive newlines with exactly 2, -// matching chatd.collapseNewlines upstream. -func collapsePromptNewlines(s string) string { - lines := strings.Split(s, "\n") - for i, line := range lines { - lines[i] = strings.TrimRightFunc(line, unicode.IsSpace) - } - s = strings.Join(lines, "\n") - - var b strings.Builder - b.Grow(len(s)) - consecutiveNewlines := 0 - for _, r := range s { - if r == '\n' { - consecutiveNewlines++ - if consecutiveNewlines <= 2 { - _, _ = b.WriteRune(r) - } - continue - } - consecutiveNewlines = 0 - _, _ = b.WriteRune(r) - } - return b.String() -} From 13041d893a29699ad7504d4faf68ad3286f913a3 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 19 Aug 2026 16:51:49 +0100 Subject: [PATCH 5/6] fix(coderd_chat_system_prompt): clarify unavailable endpoint error --- .../provider/chat_system_prompt_resource.go | 17 +++++++++-------- .../chat_system_prompt_resource_test.go | 8 ++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go index b65375f..8661dbc 100644 --- a/internal/provider/chat_system_prompt_resource.go +++ b/internal/provider/chat_system_prompt_resource.go @@ -133,8 +133,9 @@ func (r *ChatSystemPromptResource) Read(ctx context.Context, req resource.ReadRe prompt, err := r.experimentalClient().GetChatSystemPrompt(ctx) if err != nil { // Deliberately not treated as "resource deleted": this setting is a - // deployment singleton that always exists on a supported deployment, - // so a 404 means the endpoint is missing, not the resource. + // deployment singleton that always exists. A 404 means either the + // endpoint is unavailable or the caller lacks permission, not that the + // resource was deleted. resp.Diagnostics.Append(chatSystemPromptDiag("read", err)...) return } @@ -326,9 +327,9 @@ func (r *ChatSystemPromptResource) ImportState(ctx context.Context, req resource } // chatSystemPromptDiag converts a codersdk error from the chat system prompt -// endpoint into a diagnostic. Every CRUD path routes through here so a -// deployment that predates the endpoint produces the same actionable message -// whichever operation hit it first. +// endpoint into a diagnostic. Every CRUD path routes through here so an +// unavailable endpoint produces the same actionable message whichever +// operation hit it first. func chatSystemPromptDiag(action string, err error) diag.Diagnostics { var diags diag.Diagnostics @@ -339,11 +340,11 @@ func chatSystemPromptDiag(action string, err error) diag.Diagnostics { var sdkErr *codersdk.Error if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { diags.AddError( - "Unsupported Coder Version", + "Chat System Prompt Endpoint Unavailable", fmt.Sprintf("Unable to %s the chat system prompt: the deployment returned 404 for %s. "+ "This endpoint requires Coder version %s or later and a token with site-wide permissions; "+ - "upgrade the deployment, or remove `coderd_chat_system_prompt` from your configuration. "+ - "Original error: %s", + "upgrade the deployment or use a token with the required permissions. If neither is possible, "+ + "remove `coderd_chat_system_prompt` from your configuration. Original error: %s", action, "/api/experimental/chats/config/system-prompt", chatSystemPromptMinVersion, err), ) return diags diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go index 0aa42bb..1094796 100644 --- a/internal/provider/chat_system_prompt_resource_test.go +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -302,9 +302,9 @@ func TestAccChatSystemPromptImport(t *testing.T) { require.Equal(t, []string{""}, f.putPrompts) } -// TestAccChatSystemPromptUnsupportedVersion pins the 404-to-version-hint -// mapping: an old deployment produces one actionable error. -func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { +// TestAccChatSystemPromptEndpointUnavailable pins the 404 diagnostic used for +// both old deployments and tokens without site-wide permissions. +func TestAccChatSystemPromptEndpointUnavailable(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { t.Skip("Acceptance tests are disabled.") @@ -324,7 +324,7 @@ func TestAccChatSystemPromptUnsupportedVersion(t *testing.T) { { Config: chatSystemPromptConfig(f.URL, "prompt", nil), // Terraform wraps error text, so match the summary line only. - ExpectError: regexp.MustCompile("Unsupported Coder Version"), + ExpectError: regexp.MustCompile("Chat System Prompt Endpoint Unavailable"), }, }, }) From ea9bca86bba664f01bc246ca42cd71d7d3360f96 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 19 Aug 2026 17:06:38 +0100 Subject: [PATCH 6/6] refactor(coderd_chat_system_prompt): trim redundant comments --- .../provider/chat_system_prompt_resource.go | 94 ++----------------- .../chat_system_prompt_resource_test.go | 90 +----------------- 2 files changed, 14 insertions(+), 170 deletions(-) diff --git a/internal/provider/chat_system_prompt_resource.go b/internal/provider/chat_system_prompt_resource.go index 8661dbc..b93be91 100644 --- a/internal/provider/chat_system_prompt_resource.go +++ b/internal/provider/chat_system_prompt_resource.go @@ -20,24 +20,16 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" ) -// Ensure provider defined types fully satisfy framework interfaces. var _ resource.Resource = &ChatSystemPromptResource{} var _ resource.ResourceWithImportState = &ChatSystemPromptResource{} var _ resource.ResourceWithModifyPlan = &ChatSystemPromptResource{} -// chatSystemPromptMinVersion is the first Coder release serving -// `/api/experimental/chats/config/system-prompt` (coder/coder#22857). It is -// named in the error surfaced when the endpoint 404s so an admin pointed at an -// older deployment gets an actionable message instead of a bare "not found". +// First release with the chat system prompt endpoint (coder/coder#22857). const chatSystemPromptMinVersion = "2.32.0" -// maxChatSystemPromptBytes mirrors coderd's maxSystemPromptLenBytes (128 KiB, -// coderd/exp_chats.go). The server rejects longer prompts with a 400 at apply -// time; validating here fails the same way at plan time instead. +// Mirrors coderd/exp_chats.go. const maxChatSystemPromptBytes = 131072 -// pathSystemPrompt and pathIncludeDefaultSystemPrompt anchor plan-time -// diagnostics to the attributes they are about. var ( pathSystemPrompt = path.Root("system_prompt") pathIncludeDefaultSystemPrompt = path.Root("include_default_system_prompt") @@ -47,7 +39,6 @@ type ChatSystemPromptResource struct { *CoderdProviderData } -// ChatSystemPromptResourceModel describes the resource data model. type ChatSystemPromptResourceModel struct { SystemPrompt chatSystemPromptTextValue `tfsdk:"system_prompt"` IncludeDefaultSystemPrompt types.Bool `tfsdk:"include_default_system_prompt"` @@ -102,7 +93,6 @@ This resource requires Coder version [` + chatSystemPromptMinVersion + `](https: } func (r *ChatSystemPromptResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { - // Prevent panic if the provider has not been configured. if req.ProviderData == nil { return } @@ -132,16 +122,10 @@ func (r *ChatSystemPromptResource) Read(ctx context.Context, req resource.ReadRe prompt, err := r.experimentalClient().GetChatSystemPrompt(ctx) if err != nil { - // Deliberately not treated as "resource deleted": this setting is a - // deployment singleton that always exists. A 404 means either the - // endpoint is unavailable or the caller lacks permission, not that the - // resource was deleted. resp.Diagnostics.Append(chatSystemPromptDiag("read", err)...) return } - // The custom type's semantic equality keeps the prior (configured) value - // when the live value differs only by sanitization. data.SystemPrompt = newChatSystemPromptTextValue(prompt.SystemPrompt) data.IncludeDefaultSystemPrompt = types.BoolValue(prompt.IncludeDefaultSystemPrompt) @@ -157,8 +141,6 @@ func (r *ChatSystemPromptResource) Create(ctx context.Context, req resource.Crea tflog.Trace(ctx, "creating chat system prompt") - // Create and Update use a shared implementation: the underlying API is a - // single idempotent PUT with no separate create semantics. resp.Diagnostics.Append(r.put(ctx, "create", &data)...) if resp.Diagnostics.HasError() { return @@ -188,11 +170,7 @@ func (r *ChatSystemPromptResource) Update(ctx context.Context, req resource.Upda resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } -// put writes the planned prompt and include-default flag. The flag pointer is -// always non-nil: the API treats an omitted field as "leave the current value -// alone", which is the right default for a partial update but wrong for this -// resource, which owns the value outright (the attribute has a schema default, -// so the plan always carries a known value). +// Always send include-default because an omitted field preserves the remote value. func (r *ChatSystemPromptResource) put(ctx context.Context, action string, data *ChatSystemPromptResourceModel) diag.Diagnostics { var diags diag.Diagnostics @@ -206,23 +184,13 @@ func (r *ChatSystemPromptResource) put(ctx context.Context, action string, data return diags } - // The PUT returns 204 with no body, so nothing to reconcile: state keeps - // the configured value and the custom type's semantic equality absorbs - // the server-side sanitization on the next Read. return diags } func (r *ChatSystemPromptResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { tflog.Trace(ctx, "deleting chat system prompt") - // There is no DELETE endpoint for this setting: it is a `site_configs` - // upsert. Reset to the defaults of a never-configured deployment (empty - // prompt, include-default true) so `terraform destroy` leaves the - // deployment in a well-defined state. - // - // If this fails, the appended error keeps the resource in state, so a - // subsequent `terraform destroy` retries rather than the admin wrongly - // believing the prompt was reset. + // The API has no DELETE, so restore the deployment defaults. includeDefault := true err := r.experimentalClient().UpdateChatSystemPrompt(ctx, codersdk.UpdateChatSystemPromptRequest{ SystemPrompt: "", @@ -236,27 +204,19 @@ func (r *ChatSystemPromptResource) Delete(ctx context.Context, req resource.Dele tflog.Trace(ctx, "successfully deleted chat system prompt") } -// ModifyPlan emits the standard experimental-resource warning and, on a first -// apply, warns when a non-empty out-of-band prompt or a differing -// include-default flag is about to be overwritten, giving the admin a chance -// to `terraform import` instead. func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { resp.Diagnostics.AddWarning( "Experimental Resource", "coderd_chat_system_prompt is experimental. Changes are expected, and it is not recommended for production use.", ) - // A destroy plan has a null plan. Nothing to advise on. if req.Plan.Raw.IsNull() { return } - // Only a genuine create reaches the no-prior-state case this warns about. - // `terraform import` populates state without ever running Create(), so - // this correctly stays quiet on the first plan after an import. + // Import populates state without running Create, so only warn on true creates. if !req.State.Raw.IsNull() { return } - // Configure() has not run during the validate walk. if r.CoderdProviderData == nil { return } @@ -266,17 +226,14 @@ func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource. if resp.Diagnostics.HasError() { return } - // A Required attribute can still be unknown when it comes from an input - // variable or a module output. Defer rather than guess. + // Required values can still be unknown during planning. if data.SystemPrompt.IsUnknown() || data.SystemPrompt.IsNull() { return } live, err := r.experimentalClient().GetChatSystemPrompt(ctx) if err != nil { - // Best-effort advisory only. Create() makes the same call for real - // moments later and reports the error there, with the right wording - // for the operation that actually failed. + // This lookup is advisory; CRUD reports endpoint failures. tflog.Debug(ctx, "skipping chat system prompt plan-time check", map[string]any{ "error": err.Error(), }) @@ -295,8 +252,6 @@ func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource. ) } - // The attribute has a schema default, so it is only unknown when it comes - // from an unresolved expression. if !data.IncludeDefaultSystemPrompt.IsUnknown() && !data.IncludeDefaultSystemPrompt.IsNull() && data.IncludeDefaultSystemPrompt.ValueBool() != live.IncludeDefaultSystemPrompt { resp.Diagnostics.AddAttributeWarning( @@ -312,31 +267,16 @@ func (r *ChatSystemPromptResource) ModifyPlan(ctx context.Context, req resource. } func (r *ChatSystemPromptResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { - // No identifying attribute exists to extract from req.ID: this resource is - // a deployment-wide singleton and Read() takes no parameters. Terraform - // calls Read() immediately after this to populate both attributes from the - // live API; the import ID itself is required by Terraform's CLI syntax but - // otherwise unused. - // - // The framework requires at least one attribute be set for the import to - // produce a non-null state object for Read() to overwrite. + // The singleton has no ID, but Read needs a non-null placeholder state. resp.Diagnostics.Append(resp.State.Set(ctx, ChatSystemPromptResourceModel{ SystemPrompt: newChatSystemPromptTextValue(""), IncludeDefaultSystemPrompt: types.BoolValue(true), })...) } -// chatSystemPromptDiag converts a codersdk error from the chat system prompt -// endpoint into a diagnostic. Every CRUD path routes through here so an -// unavailable endpoint produces the same actionable message whichever -// operation hit it first. func chatSystemPromptDiag(action string, err error) diag.Diagnostics { var diags diag.Diagnostics - // Not isNotFound: that helper also maps a 400 "must be an existing uuid or - // username" to not-found, which is meaningless for a parameterless - // endpoint and would mislabel an unrelated bad request as a version - // problem. var sdkErr *codersdk.Error if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { diags.AddError( @@ -350,14 +290,10 @@ func chatSystemPromptDiag(action string, err error) diag.Diagnostics { return diags } - // Every other failure passes coderd's own message straight through. diags.AddError("Client Error", fmt.Sprintf("unable to %s the chat system prompt, got error: %s", action, err)) return diags } -// chatSystemPromptLengthValidator rejects prompts whose sanitized form exceeds -// coderd's 128 KiB cap, failing at plan time with the same limit the server -// would enforce with a 400 at apply time. type chatSystemPromptLengthValidator struct{} func (chatSystemPromptLengthValidator) Description(context.Context) string { @@ -381,22 +317,17 @@ func (v chatSystemPromptLengthValidator) ValidateString(ctx context.Context, req } } -// chatSystemPromptTextType is a string type whose values compare equal when -// their sanitized forms match, absorbing the server-side prompt sanitization -// (trailing newlines from `file(...)`, CRLF line endings, invisible -// characters) instead of reporting it as drift. +// Compares sanitized values to absorb server-side prompt normalization. type chatSystemPromptTextType struct { basetypes.StringType } var _ basetypes.StringTypable = chatSystemPromptTextType{} -// String implements basetypes.StringTypable. func (t chatSystemPromptTextType) String() string { return "chatSystemPromptTextType" } -// Equal implements basetypes.StringTypable. func (t chatSystemPromptTextType) Equal(o attr.Type) bool { if o, ok := o.(chatSystemPromptTextType); ok { return t.StringType.Equal(o.StringType) @@ -404,17 +335,14 @@ func (t chatSystemPromptTextType) Equal(o attr.Type) bool { return false } -// ValueType implements basetypes.StringTypable. func (t chatSystemPromptTextType) ValueType(ctx context.Context) attr.Value { return chatSystemPromptTextValue{} } -// ValueFromString implements basetypes.StringTypable. func (t chatSystemPromptTextType) ValueFromString(ctx context.Context, in basetypes.StringValue) (basetypes.StringValuable, diag.Diagnostics) { return chatSystemPromptTextValue{StringValue: in}, nil } -// ValueFromTerraform implements basetypes.StringTypable. func (t chatSystemPromptTextType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { attrValue, err := t.StringType.ValueFromTerraform(ctx, in) if err != nil { @@ -437,12 +365,10 @@ func newChatSystemPromptTextValue(value string) chatSystemPromptTextValue { return chatSystemPromptTextValue{StringValue: basetypes.NewStringValue(value)} } -// Type implements basetypes.StringValuable. func (v chatSystemPromptTextValue) Type(ctx context.Context) attr.Type { return chatSystemPromptTextType{} } -// Equal implements basetypes.StringValuable. func (v chatSystemPromptTextValue) Equal(o attr.Value) bool { if o, ok := o.(chatSystemPromptTextValue); ok { return v.StringValue.Equal(o.StringValue) @@ -450,8 +376,6 @@ func (v chatSystemPromptTextValue) Equal(o attr.Value) bool { return false } -// StringSemanticEquals implements basetypes.StringValuableWithSemanticEquals: -// two prompts are the same setting iff they sanitize to the same string. func (v chatSystemPromptTextValue) StringSemanticEquals(ctx context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { var diags diag.Diagnostics newValue, ok := newValuable.(chatSystemPromptTextValue) diff --git a/internal/provider/chat_system_prompt_resource_test.go b/internal/provider/chat_system_prompt_resource_test.go index 1094796..a1af8a6 100644 --- a/internal/provider/chat_system_prompt_resource_test.go +++ b/internal/provider/chat_system_prompt_resource_test.go @@ -28,20 +28,10 @@ import ( "github.com/stretchr/testify/require" ) -// chatSystemPromptPath is the experimental endpoint backing the resource. const chatSystemPromptPath = "/api/experimental/chats/config/system-prompt" const chatSystemPromptResourceAddr = "coderd_chat_system_prompt.test" -// fakeChatCoderd is a minimal stand-in for a Coder deployment, serving just -// the endpoints the provider touches: the two Configure() calls plus the chat -// system prompt singleton. -// -// A fake rather than `integration.StartCoder` for the same reason as the -// OAuth2 settings tests: most of the matrix is about what the *provider* does -// with a given API response, including that the server-side sanitization of -// the stored prompt does not surface as drift. The fake sanitizes on PUT -// exactly like coderd does, which is the behavior under test. type fakeChatCoderd struct { *httptest.Server @@ -110,7 +100,6 @@ func (f *fakeChatCoderd) handle(w http.ResponseWriter, r *http.Request) { f.putPrompts = append(f.putPrompts, req.SystemPrompt) f.prompt = req.SystemPrompt if f.sanitizeOnPut { - // Match coderd: the stored value is the sanitized value. f.prompt = codersdk.SanitizePromptText(req.SystemPrompt) } if req.IncludeDefaultSystemPrompt != nil { @@ -135,8 +124,6 @@ resource "coderd_chat_system_prompt" "test" { `, prompt, include) } -// TestChatSystemPromptSemanticEquals covers the custom type directly: two -// prompts are the same setting iff they sanitize to the same string. func TestChatSystemPromptSemanticEquals(t *testing.T) { t.Parallel() @@ -167,8 +154,6 @@ func TestChatSystemPromptLengthValidator(t *testing.T) { ctx := t.Context() - // The sanitized form is what the server measures, so padding that - // sanitizes away must not trip the validator. okPrompt := strings.Repeat("a", maxChatSystemPromptBytes) + "\n\n\n" tooLong := strings.Repeat("a", maxChatSystemPromptBytes+1) @@ -194,9 +179,6 @@ func TestChatSystemPromptLengthValidator(t *testing.T) { } } -// TestAccChatSystemPromptResource exercises the full lifecycle against the -// fake: create, refresh without drift despite server-side sanitization, -// update, and destroy resetting the deployment defaults. func TestAccChatSystemPromptResource(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { @@ -211,8 +193,6 @@ func TestAccChatSystemPromptResource(t *testing.T) { PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ - // Create with a trailing newline, the `file()` everyday case. The - // fake stores the sanitized (trimmed) form, like coderd does. { Config: chatSystemPromptConfig(f.URL, "You are a helpful agent.\n", nil), ConfigStateChecks: []statecheck.StateCheck{ @@ -223,8 +203,6 @@ func TestAccChatSystemPromptResource(t *testing.T) { ), }, }, - // Re-planning the same config must be empty: the live value is - // the sanitized form, which is semantically equal. { Config: chatSystemPromptConfig(f.URL, "You are a helpful agent.\n", nil), ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -233,7 +211,6 @@ func TestAccChatSystemPromptResource(t *testing.T) { }, }, }, - // A real edit must show up and apply. { Config: chatSystemPromptConfig(f.URL, "You are a very helpful agent.\n", &includeFalse), ConfigStateChecks: []statecheck.StateCheck{ @@ -247,16 +224,12 @@ func TestAccChatSystemPromptResource(t *testing.T) { }, }) - // Destroy (run by resource.Test after the last step) resets the - // deployment defaults rather than stranding the last-applied value. f.mu.Lock() defer f.mu.Unlock() require.Empty(t, f.prompt) require.True(t, f.includeDflt) } -// TestAccChatSystemPromptImport adopts a live out-of-band prompt without -// issuing any PUT. func TestAccChatSystemPromptImport(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { @@ -278,11 +251,8 @@ func TestAccChatSystemPromptImport(t *testing.T) { ResourceName: chatSystemPromptResourceAddr, ImportState: true, ImportStatePersist: true, - // The ID is required by the CLI syntax but unused. - ImportStateId: "chat_system_prompt", + ImportStateId: "chat_system_prompt", }, - // After import, the matching config plans clean: nothing to - // overwrite, nothing to PUT. { Config: chatSystemPromptConfig(f.URL, "configured in the dashboard", nil), ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -294,16 +264,11 @@ func TestAccChatSystemPromptImport(t *testing.T) { }, }) - // Adopting an existing value must never write it back. The only PUT in - // the whole test is the framework's final `terraform destroy`, which - // resets the prompt to empty. f.mu.Lock() defer f.mu.Unlock() require.Equal(t, []string{""}, f.putPrompts) } -// TestAccChatSystemPromptEndpointUnavailable pins the 404 diagnostic used for -// both old deployments and tokens without site-wide permissions. func TestAccChatSystemPromptEndpointUnavailable(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { @@ -322,20 +287,13 @@ func TestAccChatSystemPromptEndpointUnavailable(t *testing.T) { ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { - Config: chatSystemPromptConfig(f.URL, "prompt", nil), - // Terraform wraps error text, so match the summary line only. + Config: chatSystemPromptConfig(f.URL, "prompt", nil), ExpectError: regexp.MustCompile("Chat System Prompt Endpoint Unavailable"), }, }, }) } -// TestAccChatSystemPromptRealCoderNoDrift runs the lifecycle against a real -// Coder instance. This is the live proof that the pinned SDK's sanitizer -// matches the deployed server's: the configured prompt deliberately carries a -// CRLF, a zero-width space, a run of blank lines, and a trailing newline, so -// if the deployment's sanitization ever diverges from the pinned -// codersdk.SanitizePromptText, the re-plan stops being empty. func TestAccChatSystemPromptRealCoderNoDrift(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { @@ -373,8 +331,6 @@ resource "coderd_chat_system_prompt" "test" { ), }, }, - // Re-planning the identical config must yield an empty plan: the - // live value is whatever the real server sanitized and stored. { Config: cfg, PlanOnly: true, @@ -382,23 +338,12 @@ resource "coderd_chat_system_prompt" "test" { }, }) - // The server must have stored the sanitized form, and the test-framework - // destroy after the last step must have reset the deployment defaults. live, err := experimental.GetChatSystemPrompt(ctx) require.NoError(t, err) require.Empty(t, live.SystemPrompt) require.True(t, live.IncludeDefaultSystemPrompt) } -// TestAccChatSystemPromptRealCoderImportNoDrift proves that adopting a prompt -// configured out of band (via the API, as the dashboard would) and re-planning -// the matching config is a clean, empty plan. -// -// The empty plan requires the config to byte-match the stored (sanitized) -// value: semantic equality preserves prior state on Read and Apply, but a -// Required attribute's planned value must equal config, so a config that -// differs only by sanitization shows one in-place normalization update after -// import and then converges. Both behaviors are pinned here. func TestAccChatSystemPromptRealCoderImportNoDrift(t *testing.T) { t.Parallel() if os.Getenv("TF_ACC") == "" { @@ -408,7 +353,6 @@ func TestAccChatSystemPromptRealCoderImportNoDrift(t *testing.T) { client := integration.StartCoder(ctx, t, "chat_system_prompt_import_acc") experimental := codersdk.NewExperimentalClient(client) - // Configured out of band, so import (not a prior apply) seeds state. includeDefault := true require.NoError(t, experimental.UpdateChatSystemPrompt(ctx, codersdk.UpdateChatSystemPromptRequest{ SystemPrompt: "configured in the dashboard", @@ -422,13 +366,11 @@ provider "coderd" { } `, client.URL.String(), client.SessionToken()) - // Byte-matches the stored value, like trimspace(file(...)) would. cfgExact := providerBlock + ` resource "coderd_chat_system_prompt" "test" { system_prompt = "configured in the dashboard" } ` - // Differs only by a trailing newline, like a bare file(...) would. cfgTrailingNewline := providerBlock + ` resource "coderd_chat_system_prompt" "test" { system_prompt = "configured in the dashboard\n" @@ -447,18 +389,13 @@ resource "coderd_chat_system_prompt" "test" { ImportStatePersist: true, ImportStateId: "chat_system_prompt", }, - // A config that byte-matches the stored value plans clean. { Config: cfgExact, PlanOnly: true, }, - // A config that differs only by sanitization applies one - // normalization update... { Config: cfgTrailingNewline, }, - // ...and then converges: refreshes keep the configured value via - // semantic equality, so the re-plan is empty. { Config: cfgTrailingNewline, PlanOnly: true, @@ -467,9 +404,6 @@ resource "coderd_chat_system_prompt" "test" { }) } -// TestChatSystemPromptModifyPlan covers the create-time overwrite advisories -// for both attributes. Every case carries the always-on experimental-resource -// warning, so the baseline warning count is 1. func TestChatSystemPromptModifyPlan(t *testing.T) { t.Parallel() @@ -479,9 +413,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { "include_default_system_prompt": tftypes.Bool, }, } - // promptObject builds a state/plan value. A nil prompt yields a null - // object, which is how the framework signals "no prior state" (a create) - // and "no plan" (a destroy). promptObject := func(prompt any, include any) tftypes.Value { if prompt == nil && include == nil { return tftypes.NewValue(objType, nil) @@ -494,11 +425,9 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { nullObject := promptObject(nil, nil) for _, tc := range []struct { - name string - // livePrompt and liveInclude are the deployment's current values. - livePrompt string - liveInclude bool - // lookupStatus, when non-zero, makes the GET fail. + name string + livePrompt string + liveInclude bool lookupStatus int plan tftypes.Value state tftypes.Value @@ -529,8 +458,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { wantWarnings: 3, }, { - // A live prompt differing from the plan only by sanitization is - // the same setting, not an overwrite. name: "SilentWhenPromptMatchesModuloSanitization", livePrompt: "from terraform", liveInclude: true, @@ -539,7 +466,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { wantWarnings: 1, }, { - // A never-configured deployment has nothing to lose. name: "SilentOnGreenfieldDeployment", livePrompt: "", liveInclude: true, @@ -548,8 +474,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { wantWarnings: 1, }, { - // Not a create: an update already renders a real diff, and this - // is also the first plan after `terraform import`. name: "SilentWhenPriorStateExists", livePrompt: "configured in the dashboard", liveInclude: false, @@ -566,8 +490,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { wantWarnings: 1, }, { - // A Required attribute is still unknown when it comes from an - // input variable or module output. Defer rather than guess. name: "SilentWhenPlannedPromptUnknown", livePrompt: "configured in the dashboard", liveInclude: false, @@ -576,8 +498,6 @@ func TestChatSystemPromptModifyPlan(t *testing.T) { wantWarnings: 1, }, { - // Best-effort: a failed lookup must not turn into a plan error. - // Create() makes the same call and reports it properly. name: "SilentWhenLookupFails", livePrompt: "configured in the dashboard", liveInclude: false,