fix: the audit reads the refusals it is given

internal/audit/run/adjust.go

@@ -103,6 +103,23 @@ var (
 	reNotValid  = regexp.MustCompile(`field (\S+) is not valid(?: when (\w+)=(\S+))?`)
 	reReference = regexp.MustCompile(`field (\S+) must reference an existing (\w+)`)
 	reRequired  = regexp.MustCompile(`field (\S+) is required(?: when (\w+)=(\S+))?`)
+
+	// reFieldNamed matches a refusal that names its field mid-sentence and
+	// states the complaint after a separator, which is how a framework that
+	// wraps its validation errors in prose reports them.
+	reFieldNamed = regexp.MustCompile(`(?i)\bfield\s+([\w.\[\]-]+)\s*[:\-]\s*(.+)`)
+	// reTheRequired matches the bare English an API writes when it names the
+	// field it wanted and nothing else about it.
+	reTheRequired = regexp.MustCompile(`(?i)\bthe\s+([\w.]+)\s+(?:is|are)\s+required\b`)
+	// reFieldSaid matches the field-prefixed refusal a validation framework
+	// emits: the property it rejected, a colon, then its complaint. The field
+	// is a dotted path when the API validates a nested request object, and the
+	// last segment is the name the request body spells.
+	reFieldSaid = regexp.MustCompile(`^\s*([\w.\[\]-]+)\s*:\s*(.+)$`)
+	// reAbsent matches the complaints that mean "you sent nothing for this",
+	// as distinct from "what you sent is wrong": only absence is healed by
+	// adding a value.
+	reAbsent = regexp.MustCompile(`(?i)\b(?:is required|is mandatory|must not be (?:null|empty|blank)|may not be (?:null|empty|blank)|cannot be (?:null|empty|blank)|must be (?:provided|specified|present)|missing)\b`)
 )
 
 // classifyRefusal reads a 4xx response body and decides what to change. The
@@ -126,9 +143,32 @@ func classifyRefusal(res *httpResult) refusalAction {
 	if m := reRequired.FindStringSubmatch(msg); m != nil {
 		return refusalAction{kind: actAdd, field: cleanField(m[1]), condGate: m[2], condVal: cleanField(m[3])}
 	}
+	if m := reFieldNamed.FindStringSubmatch(msg); m != nil && reAbsent.MatchString(m[2]) {
+		return refusalAction{kind: actAdd, field: leafField(m[1])}
+	}
+	if m := reTheRequired.FindStringSubmatch(msg); m != nil {
+		return refusalAction{kind: actAdd, field: leafField(m[1])}
+	}
+	// Checked last, because it is the loosest: any sentence at all can be read
+	// as "<field>: <complaint>", so it must not pre-empt a grammar that names
+	// two fields or a gate.
+	if m := reFieldSaid.FindStringSubmatch(msg); m != nil && reAbsent.MatchString(m[2]) {
+		return refusalAction{kind: actAdd, field: leafField(m[1])}
+	}
 	return refusalAction{kind: actStop}
 }
 
+// leafField is the last segment of a dotted refusal path, which is the name
+// the request body spells: an API that validates its own nested request object
+// reports "endpoint.url" for a property the document declares as "url".
+func leafField(s string) string {
+	s = cleanField(s)
+	if i := strings.LastIndex(s, "."); i >= 0 {
+		s = s[i+1:]
+	}
+	return s
+}
+
 // refusalMessage pulls the human-legible sentence out of whichever error
 // envelope the API used — problem+json's detail, an oauth error_description, a
 // legacy errorMessage — falling back to the raw body when it is not JSON. It
@@ -137,6 +177,12 @@ func classifyRefusal(res *httpResult) refusalAction {
 func refusalMessage(raw []byte) string {
 	var m map[string]any
 	if err := json.Unmarshal(raw, &m); err == nil {
+		// The listed complaints come first: an envelope that carries both
+		// names the field it rejected in the list and only summarises it in
+		// the sentence, and a summary heals nothing.
+		if listed := firstListed(m); listed != "" {
+			return listed
+		}
 		for _, k := range []string{"detail", "message", "error_description", "errorMessage", "error", "title"} {
 			if s, ok := m[k].(string); ok && s != "" {
 				return s
@@ -147,6 +193,54 @@ func refusalMessage(raw []byte) string {
 	return string(raw)
 }
 
+// firstListed pulls the first complaint out of an envelope that carries them
+// as a list rather than a sentence, which is how an API that validates every
+// property before answering reports what it rejected.
+//
+// Only the first is read: one refusal heals one field, and the next attempt
+// re-reads whatever the API then complains about, so taking them one at a time
+// converges without assuming the list is ordered or complete.
+func firstListed(m map[string]any) string {
+	for _, k := range []string{"errors", "messages", "details", "errorMessages", "validationErrors"} {
+		listed, ok := m[k].([]any)
+		if !ok {
+			continue
+		}
+		for _, entry := range listed {
+			switch e := entry.(type) {
+			case string:
+				if e != "" {
+					return e
+				}
+			case map[string]any:
+				// An entry that names the field separately is spelled back
+				// into the "<field>: <complaint>" shape the grammar reads.
+				field, _ := firstString(e, "field", "name", "property", "path", "pointer", "code")
+				complaint, found := firstString(e, "message", "defaultMessage", "detail", "description", "error", "reason")
+				if !found {
+					continue
+				}
+				if field != "" {
+					return field + ": " + complaint
+				}
+				return complaint
+			}
+		}
+	}
+	return ""
+}
+
+// firstString returns the first of the named keys the map carries as a
+// non-empty string, and whether it found one.
+func firstString(m map[string]any, keys ...string) (string, bool) {
+	for _, k := range keys {
+		if s, ok := m[k].(string); ok && s != "" {
+			return s, true
+		}
+	}
+	return "", false
+}
+
 // cleanField strips the trailing punctuation a refusal sentence might carry
 // after a field name.
 func cleanField(s string) string {
@@ -162,6 +256,17 @@ func cleanField(s string) string {
 // for a per-enum-value create — so cycling searches the other enum fields for a
 // body the API accepts without abandoning what the step is exercising.
 func (r *runner) adjustCreate(ctx context.Context, ent *entityState, rec *entityRecipe, body map[string]any, held string) (adjustResult, error) {
+	return r.adjustCreateRecording(ctx, ent, rec, body, held, true)
+}
+
+// adjustCreateRecording is adjustCreate, and also says whether the healing it
+// does is a fact about the entity.
+//
+// Re-creating a parent so a child has something to address is a means to an
+// end: the fields that create needs are facts about the parent, and the
+// parent's own steps record them against the parent. Recorded here they would
+// be attributed to whichever child happened to need the parent first.
+func (r *runner) adjustCreateRecording(ctx context.Context, ent *entityState, rec *entityRecipe, body map[string]any, held string, record bool) (adjustResult, error) {
 	applied := map[string]bool{}
 	var last *httpResult
 	adjusted := false
@@ -177,7 +282,7 @@ func (r *runner) adjustCreate(ctx context.Context, ent *entityState, rec *entity
 		if res == nil || !res.refused() {
 			return adjustResult{res: res, body: body, adjusted: adjusted, gaveUp: true}, nil
 		}
-		if r.applyAdjustment(ctx, ent, body, res, applied) {
+		if r.applyAdjustment(ctx, ent, body, res, applied, record) {
 			adjusted = true
 			continue
 		}
@@ -204,7 +309,7 @@ func (r *runner) adjustCreate(ctx context.Context, ent *entityState, rec *entity
 // already present, a remove of a field already absent, a nested "a.b" target
 // it cannot synthesise, or a borrow that returns the same value all stop the
 // loop rather than spin it.
-func (r *runner) applyAdjustment(ctx context.Context, ent *entityState, body map[string]any, res *httpResult, applied map[string]bool) bool {
+func (r *runner) applyAdjustment(ctx context.Context, ent *entityState, body map[string]any, res *httpResult, applied map[string]bool, record bool) bool {
 	act := classifyRefusal(res)
 	switch act.kind {
 	case actAdd:
@@ -213,23 +318,29 @@ func (r *runner) applyAdjustment(ctx context.Context, ent *entityState, body map
 		}
 		body[act.field] = r.synthField(ent, act.field)
 		applied["a:"+act.field] = true
-		r.recordAdjustAdd(ent, act.field, act.condGate, act.condVal, res.excerpt)
+		if record {
+			r.recordAdjustAdd(ent, act.field, act.condGate, act.condVal, res.excerpt)
+		}
 		return true
 	case actRequires:
 		if strings.Contains(act.field, ".") || applied["a:"+act.field] || present(body, act.field) {
 			return false
 		}
 		body[act.field] = r.synthField(ent, act.field)
 		applied["a:"+act.field] = true
-		r.recordAdjustment(ent, infer.AdjustRequires, act.field, act.trigger, "")
+		if record {
+			r.recordAdjustment(ent, infer.AdjustRequires, act.field, act.trigger, "")
+		}
 		return true
 	case actRemove:
 		if !present(body, act.field) || applied["r:"+act.field] {
 			return false
 		}
 		delete(body, act.field)
 		applied["r:"+act.field] = true
-		r.recordAdjustment(ent, infer.AdjustRemove, act.field, act.condGate, act.condVal)
+		if record {
+			r.recordAdjustment(ent, infer.AdjustRemove, act.field, act.condGate, act.condVal)
+		}
 		return true
 	case actBorrow:
 		id, ok := r.borrow(ctx, ent, act.collection)
@@ -238,7 +349,9 @@ func (r *runner) applyAdjustment(ctx context.Context, ent *entityState, body map
 		}
 		body[act.field] = id
 		applied["b:"+act.field] = true
-		r.recordAdjustment(ent, infer.AdjustBorrow, act.field, act.collection, "")
+		if record {
+			r.recordAdjustment(ent, infer.AdjustBorrow, act.field, act.collection, "")
+		}
 		return true
 	default:
 		return false

internal/audit/run/adjust_unit_test.go

@@ -63,6 +63,56 @@ func TestUnit_Adjust_ClassifyRefusalGrammar(t *testing.T) {
 		{"plain-text", `field interval is required`, actAdd, "interval", ""},
 		{"empty", ``, actStop, "", ""},
 		{"unparseable", `{"weird":true}`, actStop, "", ""},
+
+		// An envelope that lists its complaints rather than stating one, and
+		// spells the rejected property as a path into its own request object.
+		{"listed-strings",
+			`{"errors":["endpoint.streamEndpointUrl: Endpoint URL cannot be blank"],"httpStatus":"400 BAD_REQUEST"}`,
+			actAdd, "streamEndpointUrl", ""},
+		{"listed-objects",
+			`{"errors":[{"field":"interval","message":"must not be null"}]}`,
+			actAdd, "interval", ""},
+		{"listed-under-messages",
+			`{"messages":["interval: is required"]}`,
+			actAdd, "interval", ""},
+		{"listed-empty", `{"errors":[]}`, actStop, "", ""},
+		// A field-prefixed complaint about the value that was sent, rather
+		// than about its absence: adding a value cannot heal it.
+		{"field-said-not-absence",
+			`{"errors":["interval: must be one of 60, 120, 300"]}`,
+			actStop, "", ""},
+		// A sentence that merely contains a colon is not a field complaint.
+		{"prose-with-colon",
+			`{"detail":"Validation failed: the request was rejected"}`,
+			actStop, "", ""},
+		// Both shapes at once: the sentence only summarises, the list names
+		// the field, so the list is what the loop must act on.
+		{"summary-beside-listed",
+			`{"detail":"There are invalid or missing fields","errors":[{"field":"testName","message":"must not be null"}],"title":"Request validation failed"}`,
+			actAdd, "testName", ""},
+		// A validation framework's own error object, which spells the field
+		// as a code and the complaint as a default message.
+		{"listed-code-and-default-message",
+			`{"errors":[{"code":"name","defaultMessage":"must not be blank"}]}`,
+			actAdd, "name", ""},
+		// A refusal that names its field mid-sentence, wrapped in prose.
+		{"field-named-in-prose",
+			`{"title":"There were some errors in your request, please correct them before trying again. Error in field roleName : must not be null."}`,
+			actAdd, "roleName", ""},
+		// The same shape, but complaining about the value rather than its
+		// absence: adding one cannot heal it.
+		{"field-named-in-prose-not-absence",
+			`{"title":"Error in field roleName : must be one of a, b"}`,
+			actStop, "", ""},
+		// Bare English naming only the field it wanted.
+		{"the-field-is-required",
+			`{"title":"The loginAccountGroup is required"}`,
+			actAdd, "loginAccountGroup", ""},
+		// "field X is required" still wins over the bare-English reading, so
+		// the field is X and not the word "field".
+		{"field-keyword-beats-bare-english",
+			`{"detail":"the field interval is required"}`,
+			actAdd, "interval", ""},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -213,3 +263,29 @@ func TestUnit_Borrow_CollectionPaths(t *testing.T) {
 		t.Errorf("collectionPaths(agents) = %v", got)
 	}
 }
+
+// TestUnit_Adjust_ParentRecreationHealsWithoutRecording pins the split
+// between healing and recording. A parent re-created so a child has something
+// to address must still be healed — otherwise every child of an understated
+// parent blocks — but the fields it needed are facts about the parent, and
+// recording them here would attribute them to the child that asked.
+func TestUnit_Adjust_ParentRecreationHealsWithoutRecording(t *testing.T) {
+	t.Parallel()
+
+	refusal := &httpResult{body: []byte(
+		`{"detail":"There are invalid or missing fields","errors":[{"field":"testName","message":"must not be null"}]}`)}
+
+	r := &runner{opts: Options{NamePrefix: "tfpfgen"}}
+	ent := &entityState{plan: &plan.EntityPlan{Entity: "scheduled_test"}}
+	body := map[string]any{}
+
+	if !r.applyAdjustment(context.Background(), ent, body, refusal, map[string]bool{}, false) {
+		t.Fatal("a listed field complaint did not heal the body")
+	}
+	if _, added := body["testName"]; !added {
+		t.Errorf("the named field was not added: %#v", body)
+	}
+	if len(r.adjustments) != 0 {
+		t.Errorf("a silent heal recorded %d adjustment(s)", len(r.adjustments))
+	}
+}

internal/audit/run/entity.go

@@ -247,10 +247,14 @@ func (r *runner) resolveCreated(ctx context.Context, ent *entityState, entity st
 	if !ok || rec.minimalBody == nil {
 		return "", blockedError{reason: fmt.Sprintf("no created %s object exists and its entity has no create recipe", entity)}
 	}
-	obj, _, err := r.createObject(ctx, ent, rec, rec.minimalBody)
+	// Healed like any other create: a parent the document understates is
+	// refused the same way its own create was, and without the loop every
+	// child of it blocks on a refusal the loop can read.
+	rr, err := r.adjustCreateRecording(ctx, ent, rec, cloneAnyMap(rec.minimalBody), "", false)
 	if err != nil {
 		return "", err
 	}
+	obj := rr.obj
 	if obj == nil {
 		return "", blockedError{reason: fmt.Sprintf("re-creating the %s parent object was refused", entity)}
 	}

internal/audit/run/steps_update.go

@@ -33,7 +33,7 @@ func (r *runner) adjustUpdate(ctx context.Context, ent *entityState, step *plan.
 		if res.ok() || !res.refused() {
 			return res, sent, nil
 		}
-		if !r.applyAdjustment(ctx, ent, body, res, applied) {
+		if !r.applyAdjustment(ctx, ent, body, res, applied, true) {
 			return res, sent, nil
 		}
 	}