fix(schemas): follow-ups to the JSON Schemas (#522)

docs/content/docs/manual/json-schemas.md

@@ -25,11 +25,13 @@ the published file differs from the source), next to
 | `customizations.workharbor` in `devcontainer.json` | `https://wstein.github.io/workharbor/schemas/devcontainer-workharbor.v0-provisional.schema.json` |
 
 **Versions.** `v0-provisional` is the name of the configuration these files
-map: the known structure as of 2026-10. A published file never changes. When the
-configuration changes, the schema gets a new name (`v1-provisional`, later a
-stable `v1`) and the old file and its URL stay, so a file that names an older
-version keeps working in an editor. That the published file is never edited is
-a rule of review; no test checks it against history.
+map: the known structure as of 2026-10. A published file never changes once a
+stable name is chosen; the provisional files may still be corrected until then
+(no one relies on them yet). When the configuration changes, the schema gets a
+new name (`v1-provisional`, later a stable `v1`) and the old file and its URL
+stay, so a file that names an older version keeps working in an editor. That a
+published file is never edited is a rule of review; no test checks it against
+history.
 
 **How they are kept honest.** A Go test compares each schema recursively with
 the Go struct the product reads (every key at every depth on both sides, types

internal/config/schema_test.go

@@ -91,7 +91,7 @@ func TestSchemaAcceptsGoodExamplesAndRejectsWrongOnes(t *testing.T) {
 		"missing":         {mutate(func(m map[string]any) { delete(nested(m, "roots"), "tool_store") }), `missing required key "tool_store"`},
 		"enum":            {mutate(func(m map[string]any) { m["account"] = "private" }), "$.account: "},
 		"enum in a list":  {mutate(func(m map[string]any) { m["repositories"].([]any)[0].(map[string]any)["workflow"] = "yolo" }), "$.repositories[0].workflow: "},
-		"range":           {mutate(func(m map[string]any) { m["preview"] = map[string]any{"first_port": 80, "last_port": 90} }), "$.preview.first_port: 80 violates minimum"},
+		"range":           {mutate(func(m map[string]any) { m["preview"] = map[string]any{"first_port": 80, "last_port": 90} }), "$.preview.first_port: matches none of the alternatives"},
 		"percent":         {mutate(func(m map[string]any) { m["budgets"] = map[string]any{"soft_percent": 100} }), "$.budgets.soft_percent: 100 violates maximum"},
 		"list item type":  {mutate(func(m map[string]any) { m["agent_allowed_tools"] = []any{"Read", 7} }), "$.agent_allowed_tools[1]: want string"},
 		"empty workspace": {mutate(func(m map[string]any) { nested(m, "roots")["workspaces"] = []any{} }), "fewer than 1 items"},
@@ -105,6 +105,63 @@ func TestSchemaAcceptsGoodExamplesAndRejectsWrongOnes(t *testing.T) {
 	}
 }
 
+// TestSchemaAcceptsWhatTheLoaderAccepts pins the values the loader takes that
+// a stricter schema would flag: 0 means "default" or "off" for the percents and
+// the preview range, and url.Parse lowercases the scheme. Each document is also
+// parsed by the loader, so the schema cannot drift from it. Out-of-range values
+// must keep failing in the schema.
+func TestSchemaAcceptsWhatTheLoaderAccepts(t *testing.T) {
+	s := schematest.Load(t, configSchemaPath)
+	base, _ := json.Marshal(newRig(t).cfg)
+	with := func(edit func(m map[string]any)) []byte {
+		var m map[string]any
+		if err := json.Unmarshal(base, &m); err != nil {
+			t.Fatal(err)
+		}
+		edit(m)
+		raw, _ := json.Marshal(m)
+		return raw
+	}
+	ok := map[string]func(m map[string]any){
+		"soft_percent 0":    func(m map[string]any) { m["budgets"] = map[string]any{"soft_percent": 0} },
+		"warn_percent 0":    func(m map[string]any) { m["limits"] = map[string]any{"warn_percent": 0} },
+		"preview off":       func(m map[string]any) { m["preview"] = map[string]any{"first_port": 0, "last_port": 0} },
+		"public_url scheme": func(m map[string]any) { m["public_url"] = "HTTPS://whr.example.ts.net" },
+		"board scheme": func(m map[string]any) {
+			m["board"] = map[string]any{"owner": "octo", "number": 1, "public_url": "Https://whr.example.ts.net"}
+		},
+	}
+	for name, edit := range ok {
+		t.Run("accepts "+name, func(t *testing.T) {
+			doc := with(edit)
+			if p := schematest.Validate(s, doc); len(p) != 0 {
+				t.Errorf("schema rejects a document the loader accepts: %v", p)
+			}
+			if _, err := Parse(doc); err != nil {
+				t.Errorf("loader rejects the document: %v", err)
+			}
+		})
+	}
+	bad := map[string]struct {
+		edit func(m map[string]any)
+		want string
+	}{
+		"soft_percent -1":  {func(m map[string]any) { m["budgets"] = map[string]any{"soft_percent": -1} }, "violates minimum"},
+		"warn_percent 100": {func(m map[string]any) { m["limits"] = map[string]any{"warn_percent": 100} }, "violates maximum"},
+		"preview 1023":     {func(m map[string]any) { m["preview"] = map[string]any{"first_port": 1023, "last_port": 2000} }, "matches none"},
+		"preview 65536":    {func(m map[string]any) { m["preview"] = map[string]any{"first_port": 2000, "last_port": 65536} }, "matches none"},
+		"public_url http":  {func(m map[string]any) { m["public_url"] = "http://whr.example.ts.net" }, "does not match"},
+	}
+	for name, tc := range bad {
+		t.Run("rejects "+name, func(t *testing.T) {
+			got := strings.Join(schematest.Validate(s, with(tc.edit)), "\n")
+			if !strings.Contains(got, tc.want) {
+				t.Errorf("problems = %q, want one containing %q", got, tc.want)
+			}
+		})
+	}
+}
+
 // TestSchemaKeyIsDataOnly pins the rule of issue #268. Parse decodes the file
 // with the Go struct and nothing else: the only place that ever sees the value
 // is Config.Schema, and no code in this package imports net/http or net, opens a

internal/schematest/schematest.go

@@ -46,6 +46,7 @@ func Validate(root Schema, raw []byte) []string {
 	if err := json.Unmarshal(raw, &doc); err != nil {
 		return []string{"not JSON: " + err.Error()}
 	}
+	checkKeywords(root, "$")
 	var problems []string
 	validate(root, root, doc, "$", &problems)
 	return problems
@@ -65,6 +66,38 @@ func resolve(root, s object) object {
 	return target
 }
 
+// supported lists the keywords validate evaluates; annotations are the others
+// that are allowed.
+var supported = []string{"type", "enum", "minimum", "maximum", "maxLength", "pattern", "minItems", "items", "anyOf", "required", "additionalProperties", "properties", "$ref"}
+
+// checkKeywords walks the whole schema once, whether or not a document reaches
+// a subschema, and panics on any keyword that is neither supported nor an
+// annotation. It follows properties, items, anyOf and $defs; a schema-valued
+// additionalProperties is unsupported (only false and true are evaluated).
+func checkKeywords(s object, path string) {
+	for k, v := range s {
+		if !slices.Contains(supported, k) && !slices.Contains(annotations, k) {
+			panic("schematest: unsupported keyword " + k + " at " + path)
+		}
+		switch k {
+		case "properties", "$defs":
+			for name, sub := range v.(object) {
+				checkKeywords(sub.(object), path+"."+k+"."+name)
+			}
+		case "items":
+			checkKeywords(v.(object), path+".items")
+		case "anyOf":
+			for i, sub := range v.([]any) {
+				checkKeywords(sub.(object), fmt.Sprintf("%s.anyOf[%d]", path, i))
+			}
+		case "additionalProperties":
+			if _, ok := v.(bool); !ok {
+				panic("schematest: additionalProperties must be a boolean at " + path)
+			}
+		}
+	}
+}
+
 func validate(root, s object, v any, path string, out *[]string) {
 	s = resolve(root, s)
 	bad := func(format string, args ...any) { *out = append(*out, path+": "+fmt.Sprintf(format, args...)) }
@@ -184,6 +217,7 @@ func typeMatches(want string, v any) bool {
 // nested objects, arrays and pointers.
 func CompareStruct(t testing.TB, root Schema, typ reflect.Type, extraOptional map[string]string, rawAny ...string) {
 	t.Helper()
+	checkKeywords(root, "$")
 	compare(t, root, root, typ, "$", extraOptional, rawAny)
 }
 

internal/schematest/schematest_test.go

@@ -70,3 +70,23 @@ func (r *recorder) Helper() {}
 func (r *recorder) Errorf(format string, args ...any) {
 	r.errs = append(r.errs, fmt.Sprintf(format, args...))
 }
+
+func TestUnsupportedKeywordInUnreachedSubschemaPanics(t *testing.T) {
+	for name, doc := range map[string]string{
+		"property":   `{"type":"object","properties":{"a":{"type":"array","uniqueItems":true,"items":{"type":"string"}}}}`,
+		"def":        `{"type":"object","$defs":{"d":{"type":"string","minLength":4}},"properties":{"a":{"$ref":"#/$defs/d"}}}`,
+		"anyOf":      `{"type":"object","properties":{"a":{"anyOf":[{"type":"string"},{"type":"integer","multipleOf":2}]}}}`,
+		"items":      `{"type":"object","properties":{"a":{"type":"array","items":{"type":"string","format":"uri"}}}}`,
+		"additional": `{"type":"object","additionalProperties":{"type":"string","minLength":1}}`,
+	} {
+		t.Run(name, func(t *testing.T) {
+			defer func() {
+				if recover() == nil {
+					t.Fatal("an unsupported keyword in a subschema no document reaches must panic")
+				}
+			}()
+			// The document reaches none of the subschemas.
+			Validate(schema(t, doc), []byte(`{}`))
+		})
+	}
+}

schemas/config.v0-provisional.schema.json

@@ -22,7 +22,7 @@
       }
     },
     "listen": { "type": "string", "description": "Loopback address of the web UI, for example 127.0.0.1:8787." },
-    "public_url": { "type": "string", "pattern": "^https://" },
+    "public_url": { "type": "string", "pattern": "^[hH][tT][tT][pP][sS]://" },
     "repositories": {
       "type": "array",
       "items": {
@@ -105,23 +105,23 @@
       "properties": {
         "per_run": { "$ref": "#/$defs/budgetLimit" },
         "per_task": { "$ref": "#/$defs/budgetLimit" },
-        "soft_percent": { "type": "integer", "minimum": 1, "maximum": 99 }
+        "soft_percent": { "type": "integer", "minimum": 0, "maximum": 99 }
       }
     },
     "limits": {
       "type": "object",
       "additionalProperties": false,
       "properties": {
-        "warn_percent": { "type": "integer", "minimum": 1, "maximum": 99 },
+        "warn_percent": { "type": "integer", "minimum": 0, "maximum": 99 },
         "low_balance_usd": { "type": "number", "minimum": 0 }
       }
     },
     "preview": {
       "type": "object",
       "additionalProperties": false,
       "properties": {
-        "first_port": { "type": "integer", "minimum": 1024, "maximum": 65535 },
-        "last_port": { "type": "integer", "minimum": 1024, "maximum": 65535 }
+        "first_port": { "type": "integer", "anyOf": [{ "enum": [0] }, { "minimum": 1024, "maximum": 65535 }], "description": "0 (both ports) turns previews off; otherwise a port from 1024 to 65535." },
+        "last_port": { "type": "integer", "anyOf": [{ "enum": [0] }, { "minimum": 1024, "maximum": 65535 }], "description": "0 (both ports) turns previews off; otherwise a port from 1024 to 65535." }
       }
     },
     "tool_profile": { "type": "string" },
@@ -137,7 +137,7 @@
         "session_field": { "type": "string" },
         "link_field": { "type": "string" },
         "queue_status": { "type": "string" },
-        "public_url": { "type": "string", "pattern": "^https://" }
+        "public_url": { "type": "string", "pattern": "^[hH][tT][tT][pP][sS]://" }
       }
     },
     "ntfy": {

scripts/docs_schema_test.go

@@ -24,6 +24,16 @@ var (
 	siteErr  error
 )
 
+// TestMain removes the site this package built itself; a site given with
+// -docs-site belongs to the caller and stays.
+func TestMain(m *testing.M) {
+	code := m.Run()
+	if siteDir != "" {
+		_ = os.RemoveAll(siteDir)
+	}
+	os.Exit(code)
+}
+
 // builtSite returns the already built site of -docs-site, else builds it once
 // for all tests of this package.
 func builtSite(t *testing.T) string {