Replace raw fmt.Errorf with RequiredStringField and NonEmptyString validators

[lpcox]

Summary

Fixes #8321

Adds a RequiredStringField helper to validation_rules.go that returns *ValidationError with structured context (Field, JSONPath, Suggestion) instead of plain fmt.Errorf error strings. Replaces raw if field == "" { return fmt.Errorf(...) } patterns with calls to RequiredStringField and the existing NonEmptyString validator across the config package.

Changes

New function: RequiredStringField

• Added to internal/config/validation_rules.go
• Signature: RequiredStringField(value, fieldName, jsonPath, suggestion string) *ValidationError
• Returns nil if value is non-empty, structured *ValidationError if empty
• Includes comprehensive unit tests

Call sites converted

guard_policy.go — min-integrity check in AllowOnlyPolicy.UnmarshalJSON
guard_policy_parse.go — min-integrity check in BuildAllowOnlyPolicy
guard_policy_validation.go — 4 empty-entry checks:

• write-sink.accept entries
• allow-only.repos scope entries
• normalizeStringSlice entries (blocked-users, refusal-labels, approval-labels, trusted-users, endorsement-reactions, disapproval-reactions)
• tool-call-limits keys

validation.go — tool response filter empty name/value checks

Tests updated

All affected test assertions updated to match the new structured error message format from ValidationError.Error(). All existing tests pass.

Verification

make agent-finished passes (format, build, lint, all tests including Rust guard).

[Copilot]

Pull request overview

This PR improves config validation consistency by introducing a RequiredStringField helper that returns structured *ValidationError instances (with Field, JSONPath, Suggestion) and by replacing several ad-hoc fmt.Errorf empty-string checks with RequiredStringField / NonEmptyString across the config and guard-policy validation paths.

Changes:

• Added RequiredStringField(...) *ValidationError to centralize “required string” checks with structured context.
• Migrated several guard policy and tool-response-filter validations from raw fmt.Errorf to RequiredStringField / NonEmptyString.
• Updated unit/coverage tests to assert against the new structured error formatting and messages.

Show a summary per file

File	Description
internal/config/validation.go	Converts tool response filter empty-name/value checks to NonEmptyString-based structured validation.
internal/config/validation_test.go	Updates expected substrings to match new ValidationError.Error() output for tool response filter validation.
internal/config/validation_rules.go	Adds the new RequiredStringField validator helper returning *ValidationError.
internal/config/validation_rules_test.go	Adds unit tests for RequiredStringField.
internal/config/validate_helpers_coverage_test.go	Updates coverage test expectations for the new structured validation messages.
internal/config/guard_policy.go	Uses RequiredStringField for AllowOnlyPolicy min-integrity required validation.
internal/config/guard_policy_validation.go	Replaces several empty-string checks with NonEmptyString for structured errors.
internal/config/guard_policy_unmarshal_coverage_test.go	Updates expected error substrings for missing min-integrity and empty entries.
internal/config/guard_policy_test.go	Updates expected error substrings for empty-entry cases to match NonEmptyString.
internal/config/guard_policy_parse.go	Uses RequiredStringField for min-integrity required validation in policy builder.
internal/config/guard_policy_coverage_test.go	Updates expected substrings in guard policy validation coverage tests.
internal/config/config_core_test.go	Updates expected substrings for tool response filter validation errors in TOML load tests.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Low