mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Thomas Kosiewski
9aa7130e01
Slice 3 of 4 for runtime experiment rules: evaluate stored rules per user in discovery, the frontend experiments query, and the pilot `mcp-tool-search` gate in chatd. There is still no way to edit rules through the product; that comes in slice 4. With no rules stored and a healthy database, backend decisions match today. This PR deliberately changes three things: 1. **Fail closed on rule reads.** If the rules read fails, `mcp-tool-search` is off, even when it is enabled at startup. 2. **Frontend freshness.** The experiments query now refetches. 3. **Caching.** `/api/v2/experiments` and rendered HTML are sent with `Cache-Control: no-store`. Static assets keep their long-lived caching. ## What's in it - **coderd** builds `api.ExperimentEvaluator` from `experiments.NewDBStore` plus the startup `--experiments` list. - **Discovery:** `GET /api/v2/experiments` and the `<meta property="experiments">` tag both use `EnabledExperiments(ctx, caller)`. - **chatd:** `mcp-tool-search` is now user-scoped. `chatd.Config.ExperimentEvaluator` is required, so `chatd.New` returns an error without it. `prepareGeneration` decides once per turn for the chat owner (`chat.OwnerID`); child chats inherit the owner. - **Ruleguard** (`userScopedExperimentStaticCheck` in `scripts/rules.go`): rejects static `Experiments.Enabled(...)` and `httpmw.RequireExperiment*(...)` checks of user-scoped experiments in server packages. A codersdk test keeps its list equal to `codersdk.ExperimentsUserScoped`. - **Frontend:** the experiments query is keyed by user ID and seeded from the meta tag only when the embedded user matches. It is stale 60 s after page load and refetches on mount, window focus and reconnect. Logout also clears the experiments metadata. The dashboard keeps the last loaded list when a background refetch fails, instead of replacing the page with an error. - **Once per turn:** chatd decides `mcp-tool-search` on a turn's first step and reuses that decision for the turn's later steps, so a rule change applies from the next turn and never withdraws a `find_tools` call the model already made. The decision lives on the chat runner, keyed by the turn's prompt row, like the stop-nudge tracker. A runner handoff in the middle of a turn evaluates again. - **Reference audit:** the only non-test server check of `mcp-tool-search` is in `generation_preparer.go`, and it now goes through the evaluator. ## Testing - **Go:** per-user results and `no-store` on `/api/v2/experiments`; the per-user meta tag and the HTML cache headers in `site`; group and org conditions end to end in enterprise; chatd owner eligibility, on → off → on across turns including a child chat, and a failed rules read. - **Vitest:** user-keyed seeding, staleness from page load after a remount, refetch on focus, logout clearing, and the dashboard surviving a failed refetch. Remote dogfood UAT through Coder Agents passed on a licensed test deployment: - per-user discovery in both the API and the meta tag; - widening a rule, and turning it off, with no restart; - `off` beating a startup default, and `inherit` restoring it; - fail closed on malformed JSON and on an unknown mode, with no condition text in the logs; - the `no-store` headers; - real-model chatd enforcement: `find_tools` offered on turn 1, gone after `off`, back on turn 3. That round tested `b57207d4ee`. The later commits change only tests, and a follow-up round will cover the final head. **For the author:** `coderd/x/chatd/ARCHITECTURE.md`'s list of the runner's local state doesn't mention the per-turn trackers (debug turn, session start, stop nudges, and now this experiment decision). I did not edit that document. **Known behavior:** a broken rule logs one error line per evaluation (per request or turn), with only the experiment, revision, category and position. ## Stack 1. #29869: evaluator library 2. #29898: storage and audit types 3. **This PR:** enforcement and discovery 4. Control plane (experimental API, `coder exp experiment-rules` CLI, docs) > [!NOTE] > This PR was written by an AI agent (Xum). --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --------- Signed-off-by: Thomas Kosiewski <tk@coder.com>
78 lines
2.4 KiB
Go
78 lines
2.4 KiB
Go
package codersdk_test
|
|
|
|
import (
|
|
"go/ast"
|
|
"go/parser"
|
|
"go/token"
|
|
"os"
|
|
"regexp"
|
|
"slices"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/coder/coder/v2/codersdk"
|
|
)
|
|
|
|
// TestExperimentsUserScopedRuleguard keeps the user-scoped experiment
|
|
// names in the userScopedExperimentStaticCheck ruleguard rule equal to
|
|
// codersdk.ExperimentsUserScoped, so the rule cannot silently skip a newly
|
|
// user-scoped experiment.
|
|
func TestExperimentsUserScopedRuleguard(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Map experiment values to their constant names from the source, so a
|
|
// user-scoped entry without a named constant fails.
|
|
file, err := parser.ParseFile(token.NewFileSet(), "deployment.go", nil, 0)
|
|
require.NoError(t, err)
|
|
constNames := map[codersdk.Experiment]string{}
|
|
ast.Inspect(file, func(n ast.Node) bool {
|
|
spec, ok := n.(*ast.ValueSpec)
|
|
if !ok {
|
|
return true
|
|
}
|
|
typ, ok := spec.Type.(*ast.Ident)
|
|
if !ok || typ.Name != "Experiment" {
|
|
return true
|
|
}
|
|
for i, name := range spec.Names {
|
|
lit, ok := spec.Values[i].(*ast.BasicLit)
|
|
if !ok || lit.Kind != token.STRING {
|
|
continue
|
|
}
|
|
value, err := strconv.Unquote(lit.Value)
|
|
require.NoError(t, err)
|
|
constNames[codersdk.Experiment(value)] = name.Name
|
|
}
|
|
return true
|
|
})
|
|
want := make([]string, 0, len(codersdk.ExperimentsUserScoped))
|
|
for _, ex := range codersdk.ExperimentsUserScoped {
|
|
name, ok := constNames[ex]
|
|
require.Truef(t, ok, "user-scoped experiment %q has no Experiment constant in deployment.go", ex)
|
|
want = append(want, name)
|
|
}
|
|
slices.Sort(want)
|
|
|
|
rules, err := os.ReadFile("../scripts/rules.go")
|
|
require.NoError(t, err)
|
|
src := string(rules)
|
|
start := strings.Index(src, "func userScopedExperimentStaticCheck(")
|
|
require.NotEqual(t, -1, start, "userScopedExperimentStaticCheck rule not found in scripts/rules.go")
|
|
end := strings.Index(src[start:], "\n}\n")
|
|
require.NotEqual(t, -1, end)
|
|
body := src[start : start+end]
|
|
|
|
// Every experiment alternation in the rule must list exactly the
|
|
// user-scoped experiments.
|
|
matches := regexp.MustCompile(`codersdk\\\.\(([A-Za-z0-9|]+)\)`).FindAllStringSubmatch(body, -1)
|
|
require.Len(t, matches, 2, "expected one experiment list per rule pattern")
|
|
for _, match := range matches {
|
|
got := strings.Split(match[1], "|")
|
|
slices.Sort(got)
|
|
require.Equal(t, want, got, "update userScopedExperimentStaticCheck in scripts/rules.go to match codersdk.ExperimentsUserScoped")
|
|
}
|
|
}
|