Skip to content

Commit dc91207

Browse files
authored
[Repo Assist] refactor(config): extract shared non-empty string slice validator (#12158)
🤖 *This PR was created by Repo Assist, an automated AI assistant.* ## Summary Partial fix for duplicate-code finding #12143 (recommendation 1): `validateAgentIDs` and `validateTrustedBots` in `internal/config/validation_gateway.go` independently re-implemented the same "non-empty array of non-empty strings" validation logic with slightly different signatures. ## Change - Added `validateNonEmptyStringSlice(values []string, defined bool, fieldName, specSuffix string) error` to `internal/config/validation_rules.go` as the single source of truth for this rule. - Rewired `validateAgentIDs` and `validateTrustedBots` to delegate to the new helper, preserving their exact existing error-message wording (verified against `validation_gateway_coverage_test.go` and `config_stdin_test.go`). ## Scope note I intentionally left `ValidateStringArrayField` in `internal/config/guard_policy_validation.go` (recommendation 2 in #12143) unchanged. It operates on `[]interface{}` rather than `[]string` and its error messages ("invalid %s value: ...") are pinned by an extensive existing test suite (`guard_policy_string_validation_test.go`). Bridging it onto the same helper would require changing those user-facing error strings, which felt out of scope for a "clearly beneficial, low-risk" improvement — happy to revisit if maintainers want that follow-up too. ## Test Status - `go build ./...` — pass - `go vet ./...` — pass - `gofmt -l .` — no diffs - `go test ./...` (all packages, unit) — pass - `go test ./test/integration/...` (binary built via `make build`) — pass (46.8s) - golangci-lint not available in this sandbox; relied on `gofmt`/`go vet` as a fallback. Closes #12143 > [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `api.github.com` > > [!TIP] > `api.github.com` is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding `api.github.com` to `network.allowed`, use `tools.github.mode: gh-proxy` for direct pre-authenticated GitHub CLI access without requiring network access to `api.github.com`: > > ```yaml > tools: > github: > mode: gh-proxy > ``` > > See [GitHub Tools](https://github.github.com/gh-aw/reference/github-tools/) for more information on `gh-proxy` mode. > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "api.github.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/33312260725) · copilot · auto · 182.5 AIC · ⊞ 18.9K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests) > <sub>Comment <em>/repo-assist</em> to run again</sub> > <details> <summary><sub>Add this agentic workflow to your repo</sub></summary> To install this agentic workflow, run ``` gh aw add githubnext/agentics@851905c ``` </details> <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto, id: 33312260725, workflow_id: repo-assist, run: https://github.com/github/gh-aw-mcpg/actions/runs/33312260725 --> <!-- gh-aw-workflow-id: repo-assist --> <!-- gh-aw-workflow-call-id: github/gh-aw-mcpg/repo-assist -->
2 parents bc4e02f + 5e9d7f7 commit dc91207

2 files changed

Lines changed: 22 additions & 24 deletions

File tree

internal/config/validation_gateway.go

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,7 @@ func validateGatewayConfig(gateway *StdinGatewayConfig) error {
104104
}
105105

106106
func validateAgentIDs(agentIDs []string, defined bool, fieldName string) error {
107-
if !defined {
108-
return nil
109-
}
110-
if len(agentIDs) == 0 {
111-
return fmt.Errorf("%s must be a non-empty array when present", fieldName)
112-
}
113-
for i, agentID := range agentIDs {
114-
if strings.TrimSpace(agentID) == "" {
115-
return fmt.Errorf("%s[%d] must be a non-empty string", fieldName, i)
116-
}
117-
}
118-
return nil
107+
return validateNonEmptyStringSlice(agentIDs, defined, fieldName, "")
119108
}
120109

121110
func validateGatewayPayloadSizeThreshold(value int, fieldName, jsonPath string) error {
@@ -136,18 +125,7 @@ func validateContainerRuntimeCommandNotBlank(command, fieldName, jsonPath string
136125
// validateTrustedBots checks that the trusted_bots/trustedBots list conforms to spec §4.1.3.4:
137126
// when present, it must be a non-empty array of non-empty strings.
138127
func validateTrustedBots(bots []string) error {
139-
if bots == nil {
140-
return nil
141-
}
142-
if len(bots) == 0 {
143-
return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)")
144-
}
145-
for i, bot := range bots {
146-
if strings.TrimSpace(bot) == "" {
147-
return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i)
148-
}
149-
}
150-
return nil
128+
return validateNonEmptyStringSlice(bots, bots != nil, "trusted_bots", " (spec §4.1.3.4)")
151129
}
152130

153131
// validateTOMLStdioContainerization validates that TOML stdio servers use the selected container runtime command.

internal/config/validation_rules.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@ func PortRange(port int, jsonPath string) *ValidationError {
3232
return nil
3333
}
3434

35+
// validateNonEmptyStringSlice validates that, when defined is true, values is a non-empty
36+
// slice containing only non-empty (non-whitespace) strings. When defined is false, the field
37+
// is treated as absent and validation passes trivially. fieldName is used for both the
38+
// top-level error and per-element errors; specSuffix (e.g. " (spec §4.1.3.4)") is appended
39+
// verbatim after "when present" in the top-level error, or may be empty.
40+
func validateNonEmptyStringSlice(values []string, defined bool, fieldName, specSuffix string) error {
41+
if !defined {
42+
return nil
43+
}
44+
if len(values) == 0 {
45+
return fmt.Errorf("%s must be a non-empty array when present%s", fieldName, specSuffix)
46+
}
47+
for i, v := range values {
48+
if strings.TrimSpace(v) == "" {
49+
return fmt.Errorf("%s[%d] must be a non-empty string", fieldName, i)
50+
}
51+
}
52+
return nil
53+
}
54+
3555
func validatePositiveIntegerRule(value int, fieldName, jsonPath, logLabel, failureLabel, suggestion string) *ValidationError {
3656
logValidation.Printf("Validating %s: field=%s, value=%d, jsonPath=%s", logLabel, fieldName, value, jsonPath)
3757
if value < 1 {

0 commit comments

Comments
 (0)