refactor: share the JSON-key rules between internal/cli and internal/app

docs/results/2026-10-05-share-the-json-key-rules-between-internal-cli-and-internal-a-results.md

@@ -0,0 +1,17 @@
+<!-- docket:backlink:start (generated — do not hand-edit) -->
+> ↩ **[Change 0522 — Share the JSON-key rules between internal/cli and internal/app](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0522-share-the-json-key-rules-between-internal-cli-and-internal-a.md)**
+<!-- docket:backlink:end -->
+# Share the JSON-key rules between internal/cli and internal/app — Results
+
+**Human action:** None needed. This is a behavior-preserving refactor, and the existing cross-check tests cover it.
+
+## Outcome
+
+The code that lists the JSON keys a request file accepts now lives in one place. `internal/app` exports `RequestJSONKeys`, and both it and the private `requiredJSONKeys` (the same keys filtered to `docket:"required"` fields) run on one shared walk. The CLI's separate copy (`internal/cli/requestkeys.go`) is gone. The unknown-key refusal message ("accepted keys: …") and the published-schema cross-check tests now call `app.RequestJSONKeys`. The two comments that pointed readers at the old CLI copy now name the shared function. Nothing changes for users: the refusal text and `docket schema` output are the same as before.
+
+## Verification performed
+
+- Each build task ran focused tests. The moved unit tests (`TestRequestJSONKeysReconcile`, `TestRequestJSONKeysSkipsAndPromotes`) and a new `TestRequiredJSONKeysFiltersTheSharedWalk` pass in `internal/app`. Two mutations of the shared walk (removing embedded-struct promotion, and removing the required filter) turned them red. `TestPublishedRequestIsTheDecodedJSONFile` and the unknown-key refusal test pass in `internal/cli`.
+- A whole-repo grep finds no remaining `requestJSONKeys` in Go source.
+- The full suite ran through the build gate at the final head.
+- Whole-branch review (standard tier): no findings.

internal/app/schema.go

@@ -60,11 +60,11 @@ func reflectDescriptor(prototype any) (TypeDescriptor, error) {
 }
 
 // reflectFields walks a struct type's fields into descriptors, in declaration
-// order. It mirrors requiredJSONKeys / the CLI's requestJSONKeys walk: an
-// embedded struct promotes its fields inline; a `json:"-"` field and an
-// untagged unexported field contribute nothing; an untagged exported field
-// falls back to its Go field name. The docket tag (via Task 4's helpers)
-// supplies Required, Presence, and Enum.
+// order. It walks the same shape as walkJSONKeys (behind RequestJSONKeys and
+// requiredJSONKeys): an embedded struct promotes its fields inline; a
+// `json:"-"` field and an untagged unexported field contribute nothing; an
+// untagged exported field falls back to its Go field name. The docket tag (via
+// Task 4's helpers) supplies Required, Presence, and Enum.
 func reflectFields(t reflect.Type) ([]FieldDescriptor, error) {
 	var out []FieldDescriptor
 	for i := 0; i < t.NumField(); i++ {

internal/app/schema_tags.go

@@ -56,10 +56,11 @@ func docketEnumRef(tag reflect.StructTag) string {
 }
 
 // jsonFieldKey derives the JSON key of one non-embedded struct field, the
-// single copy of the key rules reflectFields, requiredJSONKeys, and the tests'
-// field lookup share: a `json:"-"` field and an untagged unexported field
-// contribute nothing (ok is false), and an untagged exported field falls back to
-// its Go field name. Promoting an embedded struct's fields is the caller's walk.
+// single copy of the key rules reflectFields, walkJSONKeys (behind
+// RequestJSONKeys and requiredJSONKeys), and the tests' field lookup share:
+// a `json:"-"` field and an untagged unexported field contribute nothing (ok
+// is false), and an untagged exported field falls back to its Go field name.
+// Promoting an embedded struct's fields is the caller's walk.
 func jsonFieldKey(f reflect.StructField) (key string, ok bool) {
 	key = strings.Split(f.Tag.Get("json"), ",")[0]
 	if key == "-" || (key == "" && !f.IsExported()) {
@@ -71,12 +72,30 @@ func jsonFieldKey(f reflect.StructField) (key string, ok bool) {
 	return key, true
 }
 
+// RequestJSONKeys returns the sorted top-level JSON keys a closed request
+// struct accepts, which is the exact set DisallowUnknownFields enforces. The
+// CLI lists them in an unknown-key refusal. It is walkJSONKeys keeping every
+// field.
+func RequestJSONKeys(prototype any) []string {
+	return walkJSONKeys(prototype, func(reflect.StructField) bool { return true })
+}
+
 // requiredJSONKeys returns the sorted top-level JSON keys of prototype whose
-// field carries docket:"required". It walks the same shape as the CLI's
-// requestJSONKeys — embedded structs promote their fields; `json:"-"` and
-// untagged unexported fields contribute nothing — filtered to required-tagged
-// fields. It lives app-side because Task 5's schema generator consumes it.
+// field carries docket:"required": the RequestJSONKeys walk filtered to
+// required-tagged fields. The request-shape validator tests and the schema
+// surface consume it.
 func requiredJSONKeys(prototype any) []string {
+	return walkJSONKeys(prototype, func(f reflect.StructField) bool {
+		return hasDocketOption(f.Tag, "required")
+	})
+}
+
+// walkJSONKeys is the one walk of a struct's top-level JSON keys behind
+// RequestJSONKeys and requiredJSONKeys. It dereferences pointers, lets an
+// embedded struct promote its fields, takes every other field's key from
+// jsonFieldKey, and keeps a key only when keep reports true for its field.
+// The result is sorted and non-nil.
+func walkJSONKeys(prototype any, keep func(reflect.StructField) bool) []string {
 	t := reflect.TypeOf(prototype)
 	for t != nil && t.Kind() == reflect.Pointer {
 		t = t.Elem()
@@ -94,7 +113,7 @@ func requiredJSONKeys(prototype any) []string {
 			if !ok {
 				continue
 			}
-			if hasDocketOption(f.Tag, "required") {
+			if keep(f) {
 				seen[key] = true
 			}
 		}

internal/app/schema_tags_test.go

@@ -439,3 +439,54 @@ func flatRequiredKeys(t *testing.T, prototype any) []string {
 	}
 	return out
 }
+
+// TestRequestJSONKeysReconcile proves RequestJSONKeys returns exactly the
+// sorted top-level JSON keys DisallowUnknownFields enforces for a real request.
+func TestRequestJSONKeysReconcile(t *testing.T) {
+	got := RequestJSONKeys(&ChangeReconcileRequest{})
+	want := []string{"id", "reconcile_log_entry", "relations", "revision", "sections", "spec_sections"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("RequestJSONKeys = %v, want %v", got, want)
+	}
+}
+
+// TestRequestJSONKeysSkipsAndPromotes proves a `json:"-"` field contributes no
+// key and an embedded struct's fields are promoted into the key set.
+func TestRequestJSONKeysSkipsAndPromotes(t *testing.T) {
+	type embedded struct {
+		Promoted string `json:"promoted"`
+	}
+	type fixture struct {
+		embedded
+		Kept    string `json:"kept"`
+		Skipped string `json:"-"`
+		Named   string `json:"renamed"`
+	}
+	got := RequestJSONKeys(&fixture{})
+	want := []string{"kept", "promoted", "renamed"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("RequestJSONKeys = %v, want %v", got, want)
+	}
+}
+
+// TestRequiredJSONKeysFiltersTheSharedWalk proves requiredJSONKeys is the
+// RequestJSONKeys walk filtered to docket:"required": a required field promoted
+// from an embedded struct counts, an optional field does not, and a `json:"-"`
+// field contributes no key to either set even when it is tagged required.
+func TestRequiredJSONKeysFiltersTheSharedWalk(t *testing.T) {
+	type embedded struct {
+		Promoted string `json:"promoted" docket:"required"`
+	}
+	type fixture struct {
+		embedded
+		Kept     string `json:"kept" docket:"required"`
+		Optional string `json:"optional"`
+		Skipped  string `json:"-" docket:"required"`
+	}
+	if got, want := RequestJSONKeys(&fixture{}), []string{"kept", "optional", "promoted"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("RequestJSONKeys = %v, want %v", got, want)
+	}
+	if got, want := requiredJSONKeys(&fixture{}), []string{"kept", "promoted"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("requiredJSONKeys = %v, want %v", got, want)
+	}
+}

internal/cli/change.go

@@ -567,7 +567,7 @@ func decodeRequest(stdin io.Reader, flagName, source string, dst any) error {
 		// An unknown-field refusal names only the offending key; append the
 		// accepted key set so the caller learns every allowed value.
 		if strings.Contains(err.Error(), "unknown field") {
-			return fmt.Errorf("decoding %s JSON: %w (accepted keys: %s)", flagName, err, strings.Join(requestJSONKeys(dst), ", "))
+			return fmt.Errorf("decoding %s JSON: %w (accepted keys: %s)", flagName, err, strings.Join(app.RequestJSONKeys(dst), ", "))
 		}
 		return fmt.Errorf("decoding %s JSON: %w", flagName, err)
 	}

internal/cli/jsonfile_production_test.go

@@ -79,7 +79,7 @@ func TestPublishedRequestIsTheDecodedJSONFile(t *testing.T) {
 				published = append(published, f.Key)
 			}
 			sort.Strings(published)
-			if got := requestJSONKeys(b.Request); !reflect.DeepEqual(got, published) {
+			if got := app.RequestJSONKeys(b.Request); !reflect.DeepEqual(got, published) {
 				t.Errorf("operation %q: the decoder accepts keys %v but docket schema publishes %v", id, got, published)
 			}
 		}
@@ -153,7 +153,7 @@ func TestPublishedRequestKeysAreAccepted(t *testing.T) {
 		sample["schema_version"] = 1
 		body, _ = json.Marshal(sample)
 		err := decodeRequest(bytes.NewReader(body), "--input", "-", reflect.New(typ).Interface())
-		accepted := "accepted keys: " + strings.Join(requestJSONKeys(reflect.New(typ).Interface()), ", ")
+		accepted := "accepted keys: " + strings.Join(app.RequestJSONKeys(reflect.New(typ).Interface()), ", ")
 		if err == nil || !strings.Contains(err.Error(), `unknown field "schema_version"`) || !strings.Contains(err.Error(), accepted) {
 			t.Errorf("operation %q: schema_version refusal = %v, want an unknown-field refusal naming %q", op.ID, err, accepted)
 		}