summaryrefslogtreecommitdiff
path: root/internal/executor/classifier_test.go
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-07-11 09:37:31 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-07-11 09:37:31 +0000
commit027536520409757ba38cd4c78e07b2ca0797a2d7 (patch)
treedc0671430ede3e0adf80d6640ed62c7d7c979264 /internal/executor/classifier_test.go
parent7978760316319d22670cd6369c15b68c649761bc (diff)
fix(executor): validate the model classifier's output against a known model list
The Gemini-based Classifier trusted its LLM output verbatim. Twice in production it echoed part of its own prompt's JSON schema literally -- 'model-name' (the prompt's own placeholder, also fixed here to be less echo-prone) and separately 'choose-the-best-model' -- instead of substituting a real model identifier. Both were syntactically valid JSON strings that passed straight through to the claude CLI's --model flag, which rejected them and failed the task outright. validateClassification now rejects anything outside the known model list, which Classify's existing error handling already treats as 'classification failed' and falls back to no explicit model override -- the same documented fallback path, now also covering 'succeeded but returned garbage.'
Diffstat (limited to 'internal/executor/classifier_test.go')
-rw-r--r--internal/executor/classifier_test.go42
1 files changed, 42 insertions, 0 deletions
diff --git a/internal/executor/classifier_test.go b/internal/executor/classifier_test.go
index 84fffcf..f0bb32f 100644
--- a/internal/executor/classifier_test.go
+++ b/internal/executor/classifier_test.go
@@ -112,6 +112,48 @@ func TestClassifier_LLMTakesPrecedence_OverGemini(t *testing.T) {
}
}
+// TestClassifier_Classify_RejectsEchoedPlaceholder proves the 2026-07-11 fix:
+// a gemini CLI response whose "model" field is the prompt's own literal
+// placeholder text (or any other string outside the known model list) must
+// be rejected as an error, not trusted as a real model identifier -- this is
+// exactly what happened in production ("model-name" and separately
+// "choose-the-best-model" both slipped through unvalidated and were passed
+// straight to the claude CLI's --model flag, which rejected them outright).
+func TestClassifier_Classify_RejectsEchoedPlaceholder(t *testing.T) {
+ mockBinary := filepathJoin(t.TempDir(), "mock-gemini")
+ mockContent := `#!/bin/sh
+echo '{"response": "{\"agent_type\": \"claude\", \"model\": \"model-name\", \"reason\": \"echoed the placeholder\"}"}'
+`
+ if err := os.WriteFile(mockBinary, []byte(mockContent), 0755); err != nil {
+ t.Fatal(err)
+ }
+
+ c := &Classifier{GeminiBinaryPath: mockBinary}
+ _, err := c.Classify(context.Background(), "Test Task", "Test Instructions", SystemStatus{}, "claude")
+ if err == nil {
+ t.Fatal("expected an error for an unrecognized model, got nil")
+ }
+ if !strings.Contains(err.Error(), "model-name") {
+ t.Errorf("expected error to name the bad model value, got: %v", err)
+ }
+}
+
+// TestClassifier_ClassifyViaLLM_RejectsUnrecognizedModel mirrors the above
+// for the local-LLM path, which is equally capable of echoing garbage.
+func TestClassifier_ClassifyViaLLM_RejectsUnrecognizedModel(t *testing.T) {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ fmt.Fprintln(w, `{"model":"x","choices":[{"message":{"content":"{\"agent_type\":\"claude\",\"model\":\"choose-the-best-model\",\"reason\":\"r\"}"},"finish_reason":"stop"}],"usage":{}}`)
+ }))
+ defer srv.Close()
+
+ c := &Classifier{LLM: &llm.Client{Endpoint: srv.URL + "/v1", Model: "x"}}
+ _, err := c.Classify(context.Background(), "n", "i", SystemStatus{}, "claude")
+ if err == nil {
+ t.Fatal("expected an error for an unrecognized model, got nil")
+ }
+}
+
func filepathJoin(elems ...string) string {
var path string
for i, e := range elems {