From 0eb98328d5f872493d3a6377352cf5006f45a9bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:13:16 +0000 Subject: [PATCH 1/4] Initial plan From 92da32c920fc3b0e4a047104b561a778e13af8f3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:23:13 +0000 Subject: [PATCH 2/4] Deduplicate constraint builders in tool_description_enhancer.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/tool_description_enhancer.go | 795 +++++++++------------- 1 file changed, 320 insertions(+), 475 deletions(-) diff --git a/pkg/workflow/tool_description_enhancer.go b/pkg/workflow/tool_description_enhancer.go index 734f075acec..03d00bda559 100644 --- a/pkg/workflow/tool_description_enhancer.go +++ b/pkg/workflow/tool_description_enhancer.go @@ -128,6 +128,41 @@ func appendMaxConstraint(constraints *[]string, max *string, format string) { } } +// buildConstraints centralizes the nil-guard + slice setup boilerplate shared by +// every per-tool constraint builder: it returns nil when config is nil, otherwise +// it hands a fresh constraints slice to build for population. +func buildConstraints[T any](config *T, build func(config *T, constraints *[]string)) []string { + if config == nil { + return nil + } + var constraints []string + build(config, &constraints) + return constraints +} + +// appendTargetConstraint appends the common "Target: ." constraint when target is set. +func appendTargetConstraint(constraints *[]string, target string) { + if target != "" { + *constraints = append(*constraints, fmt.Sprintf("Target: %s.", target)) + } +} + +// appendTargetRepoSlugConstraint appends a formatted constraint describing the target +// repository slug when set. format must contain a single %q/%s verb for the slug. +func appendTargetRepoSlugConstraint(constraints *[]string, targetRepoSlug, format string) { + if targetRepoSlug != "" { + *constraints = append(*constraints, fmt.Sprintf(format, targetRepoSlug)) + } +} + +// appendRequiredTitlePrefixConstraint appends a formatted constraint describing a +// required title prefix when set. format must contain a single %q/%s verb for the prefix. +func appendRequiredTitlePrefixConstraint(constraints *[]string, prefix, format string) { + if prefix != "" { + *constraints = append(*constraints, fmt.Sprintf(format, prefix)) + } +} + // enhanceToolDescription adds configuration-specific constraints to tool descriptions // This provides agents with context about limits and restrictions configured in the workflow func enhanceToolDescription(toolName, baseDescription string, safeOutputs *SafeOutputsConfig) string { @@ -158,583 +193,393 @@ func buildToolDescriptionConstraints(toolName string, safeOutputs *SafeOutputsCo } func createIssueConstraints(config *CreateIssuesConfig) []string { - if config == nil { - return nil - } - - toolDescriptionEnhancerLog.Printf("Found create_issue config: max=%v, titlePrefix=%s", config.Max, config.TitlePrefix) + return buildConstraints(config, func(config *CreateIssuesConfig, constraints *[]string) { + toolDescriptionEnhancerLog.Printf("Found create_issue config: max=%v, titlePrefix=%s", config.Max, config.TitlePrefix) - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d issue(s) can be created.") - if config.TitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } - if len(config.Labels) > 0 { - constraints = append(constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) - } - if len(config.AllowedLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) - } - appendAllowedIssueFieldsConstraint(&constraints, config.AllowedFields) - if len(config.Assignees) > 0 { - constraints = append(constraints, fmt.Sprintf("Assignees %s will be automatically assigned.", formatStringList(config.Assignees))) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Issues will be created in repository %q.", config.TargetRepoSlug)) - } - if config.RequireTemporaryID { - constraints = append(constraints, "temporary_id is required.") - } - if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { - constraints = append(constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") - } - return constraints + appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be created.") + if config.TitlePrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) + } + if len(config.Labels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) + } + if len(config.AllowedLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) + } + appendAllowedIssueFieldsConstraint(constraints, config.AllowedFields) + if len(config.Assignees) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Assignees %s will be automatically assigned.", formatStringList(config.Assignees))) + } + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issues will be created in repository %q.") + if config.RequireTemporaryID { + *constraints = append(*constraints, "temporary_id is required.") + } + if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { + *constraints = append(*constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") + } + }) } func setIssueFieldConstraints(config *SetIssueFieldConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d issue field update(s) can be made.") - appendAllowedIssueFieldsConstraint(&constraints, config.AllowedFields) - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Issue fields will be updated in repository %q.", config.TargetRepoSlug)) - } - return constraints + return buildConstraints(config, func(config *SetIssueFieldConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d issue field update(s) can be made.") + appendAllowedIssueFieldsConstraint(constraints, config.AllowedFields) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issue fields will be updated in repository %q.") + }) } func createAgentSessionConstraints(config *CreateAgentSessionConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d agent task(s) can be created.") - if config.Base != "" { - constraints = append(constraints, fmt.Sprintf("Base branch for tasks: %q.", config.Base)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Tasks will be created in repository %q.", config.TargetRepoSlug)) - } - if len(config.AllowedRepos) > 0 { - constraints = append(constraints, fmt.Sprintf("Sessions can target these repositories: %v.", config.AllowedRepos)) - } - return constraints + return buildConstraints(config, func(config *CreateAgentSessionConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d agent task(s) can be created.") + if config.Base != "" { + *constraints = append(*constraints, fmt.Sprintf("Base branch for tasks: %q.", config.Base)) + } + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Tasks will be created in repository %q.") + if len(config.AllowedRepos) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Sessions can target these repositories: %v.", config.AllowedRepos)) + } + }) } func createDiscussionConstraints(config *CreateDiscussionsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d discussion(s) can be created.") - if config.TitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } - if config.Category != "" { - constraints = append(constraints, fmt.Sprintf("Discussions will be created in category %q.", config.Category)) - } - if len(config.AllowedLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Discussions will be created in repository %q.", config.TargetRepoSlug)) - } - return constraints + return buildConstraints(config, func(config *CreateDiscussionsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d discussion(s) can be created.") + if config.TitlePrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) + } + if config.Category != "" { + *constraints = append(*constraints, fmt.Sprintf("Discussions will be created in category %q.", config.Category)) + } + if len(config.AllowedLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) + } + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Discussions will be created in repository %q.") + }) } func closeDiscussionConstraints(config *CloseDiscussionsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d discussion(s) can be closed.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Discussions will be closed in repository %q.", config.TargetRepoSlug)) - } - if config.RequiredTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Only discussions with title prefix %q can be closed.", config.RequiredTitlePrefix)) - } - if config.AllowBody != nil && !*config.AllowBody { - constraints = append(constraints, "Closing comments are disabled: do not include a body field.") - } - return constraints + return buildConstraints(config, func(config *CloseDiscussionsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d discussion(s) can be closed.") + appendTargetConstraint(constraints, config.Target) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Discussions will be closed in repository %q.") + appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only discussions with title prefix %q can be closed.") + if config.AllowBody != nil && !*config.AllowBody { + *constraints = append(*constraints, "Closing comments are disabled: do not include a body field.") + } + }) } func updateDiscussionConstraints(config *UpdateDiscussionsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d discussion(s) can be updated.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.Title != nil && *config.Title { - constraints = append(constraints, "Title updates are allowed.") - } - if config.Body != nil && *config.Body { - constraints = append(constraints, "Body updates are allowed.") - } - if config.Labels != nil { - if len(config.AllowedLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) - } else { - constraints = append(constraints, "Label updates are allowed.") + return buildConstraints(config, func(config *UpdateDiscussionsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d discussion(s) can be updated.") + appendTargetConstraint(constraints, config.Target) + if config.Title != nil && *config.Title { + *constraints = append(*constraints, "Title updates are allowed.") } - } - return constraints + if config.Body != nil && *config.Body { + *constraints = append(*constraints, "Body updates are allowed.") + } + if config.Labels != nil { + if len(config.AllowedLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) + } else { + *constraints = append(*constraints, "Label updates are allowed.") + } + } + }) } func closeIssueConstraints(config *CloseIssuesConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d issue(s) can be closed.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.RequiredTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Only issues with title prefix %q can be closed.", config.RequiredTitlePrefix)) - } - if config.AllowBody != nil && !*config.AllowBody { - constraints = append(constraints, "Closing comments are disabled: do not include a body field.") - } - return constraints + return buildConstraints(config, func(config *CloseIssuesConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be closed.") + appendTargetConstraint(constraints, config.Target) + appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only issues with title prefix %q can be closed.") + if config.AllowBody != nil && !*config.AllowBody { + *constraints = append(*constraints, "Closing comments are disabled: do not include a body field.") + } + }) } func closePullRequestConstraints(config *ClosePullRequestsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d pull request(s) can be closed.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Pull requests will be closed in repository %q.", config.TargetRepoSlug)) - } - if len(config.RequiredLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only PRs with labels %v can be closed.", config.RequiredLabels)) - } - if config.RequiredTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Only PRs with title prefix %q can be closed.", config.RequiredTitlePrefix)) - } - return constraints + return buildConstraints(config, func(config *ClosePullRequestsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be closed.") + appendTargetConstraint(constraints, config.Target) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Pull requests will be closed in repository %q.") + if len(config.RequiredLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only PRs with labels %v can be closed.", config.RequiredLabels)) + } + appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be closed.") + }) } func markPullRequestAsReadyForReviewConstraints(config *MarkPullRequestAsReadyForReviewConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d pull request(s) can be marked as ready for review.") - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Pull requests will be marked as ready in repository %q.", config.TargetRepoSlug)) - } - return constraints + return buildConstraints(config, func(config *MarkPullRequestAsReadyForReviewConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be marked as ready for review.") + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Pull requests will be marked as ready in repository %q.") + }) } func addCommentConstraints(config *AddCommentsConfig) []string { - var constraints []string - if config != nil { - appendMaxConstraint(&constraints, config.Max, "Maximum %d comment(s) can be added.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Comments will be added in repository %q.", config.TargetRepoSlug)) - } + constraints := buildConstraints(config, func(config *AddCommentsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d comment(s) can be added.") + appendTargetConstraint(constraints, config.Target) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Comments will be added in repository %q.") if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { - constraints = append(constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") + *constraints = append(*constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") } - } + }) return append(constraints, "Supports reply_to_id for discussion threading.") } func createPullRequestConstraints(config *CreatePullRequestsConfig) []string { - if config == nil { - return nil - } - - toolDescriptionEnhancerLog.Printf("Found create_pull_request config: max=%v, titlePrefix=%s, draft=%v", config.Max, config.TitlePrefix, config.Draft) + return buildConstraints(config, func(config *CreatePullRequestsConfig, constraints *[]string) { + toolDescriptionEnhancerLog.Printf("Found create_pull_request config: max=%v, titlePrefix=%s, draft=%v", config.Max, config.TitlePrefix, config.Draft) - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d pull request(s) can be created.") - if config.BranchPrefix != "" { - constraints = append(constraints, fmt.Sprintf("Branch name will be prefixed with %q.", config.BranchPrefix)) - } - if config.TitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } - if len(config.Labels) > 0 { - constraints = append(constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) - } - if len(config.AllowedLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) - } - if config.Draft != nil && *config.Draft == "true" { - constraints = append(constraints, "PRs will be created as drafts.") - } - if len(config.Reviewers) > 0 { - constraints = append(constraints, fmt.Sprintf("Reviewers %s will be assigned.", formatStringList(config.Reviewers))) - } - if len(config.Assignees) > 0 { - constraints = append(constraints, fmt.Sprintf("Assignees %s will be assigned to the created pull request and any fallback issue.", formatStringList(config.Assignees))) - } - if config.RequireTemporaryID { - constraints = append(constraints, "temporary_id is required.") - } - if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { - constraints = append(constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") - } - return constraints + appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be created.") + if config.BranchPrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("Branch name will be prefixed with %q.", config.BranchPrefix)) + } + if config.TitlePrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) + } + if len(config.Labels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) + } + if len(config.AllowedLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) + } + if config.Draft != nil && *config.Draft == "true" { + *constraints = append(*constraints, "PRs will be created as drafts.") + } + if len(config.Reviewers) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Reviewers %s will be assigned.", formatStringList(config.Reviewers))) + } + if len(config.Assignees) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Assignees %s will be assigned to the created pull request and any fallback issue.", formatStringList(config.Assignees))) + } + if config.RequireTemporaryID { + *constraints = append(*constraints, "temporary_id is required.") + } + if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { + *constraints = append(*constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") + } + }) } func createPullRequestReviewCommentConstraints(config *CreatePullRequestReviewCommentsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d review comment(s) can be created.") - if config.Side != "" { - constraints = append(constraints, fmt.Sprintf("Comments will be on the %s side of the diff.", config.Side)) - } - return constraints + return buildConstraints(config, func(config *CreatePullRequestReviewCommentsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d review comment(s) can be created.") + if config.Side != "" { + *constraints = append(*constraints, fmt.Sprintf("Comments will be on the %s side of the diff.", config.Side)) + } + }) } func submitPullRequestReviewConstraints(config *SubmitPullRequestReviewConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d review(s) can be submitted.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Reviews will be submitted in repository %q.", config.TargetRepoSlug)) - } - return constraints + return buildConstraints(config, func(config *SubmitPullRequestReviewConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d review(s) can be submitted.") + appendTargetConstraint(constraints, config.Target) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Reviews will be submitted in repository %q.") + }) } func replyToPullRequestReviewCommentConstraints(config *ReplyToPullRequestReviewCommentConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d reply/replies can be created.") - return constraints + return buildConstraints(config, func(config *ReplyToPullRequestReviewCommentConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d reply/replies can be created.") + }) } func dismissPullRequestReviewConstraints(config *DismissPullRequestReviewConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d review dismissal(s) can be performed.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Review dismissals will be performed in repository %q.", config.TargetRepoSlug)) - } - return append(constraints, "justification must contain at least 20 characters.") + return buildConstraints(config, func(config *DismissPullRequestReviewConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d review dismissal(s) can be performed.") + appendTargetConstraint(constraints, config.Target) + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Review dismissals will be performed in repository %q.") + *constraints = append(*constraints, "justification must contain at least 20 characters.") + }) } func resolvePullRequestReviewThreadConstraints(config *ResolvePullRequestReviewThreadConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d review thread(s) can be resolved.") - return constraints + return buildConstraints(config, func(config *ResolvePullRequestReviewThreadConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d review thread(s) can be resolved.") + }) } func createCodeScanningAlertConstraints(config *CreateCodeScanningAlertsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d alert(s) can be created.") - return constraints + return buildConstraints(config, func(config *CreateCodeScanningAlertsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d alert(s) can be created.") + }) } func createCheckRunConstraints(config *CreateCheckRunConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d check run(s) can be created.") - if config.Name != "" { - constraints = append(constraints, fmt.Sprintf("Check run name: %q.", config.Name)) - } - return constraints + return buildConstraints(config, func(config *CreateCheckRunConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d check run(s) can be created.") + if config.Name != "" { + *constraints = append(*constraints, fmt.Sprintf("Check run name: %q.", config.Name)) + } + }) } func addLabelsConstraints(config *AddLabelsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d label(s) can be added.") - if len(config.Allowed) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.Allowed))) - } - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - return constraints + return buildConstraints(config, func(config *AddLabelsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d label(s) can be added.") + if len(config.Allowed) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.Allowed))) + } + appendTargetConstraint(constraints, config.Target) + }) } func removeLabelsConstraints(config *RemoveLabelsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d label(s) can be removed.") - if len(config.Allowed) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels can be removed: %v.", config.Allowed)) - } - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - return constraints + return buildConstraints(config, func(config *RemoveLabelsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d label(s) can be removed.") + if len(config.Allowed) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels can be removed: %v.", config.Allowed)) + } + appendTargetConstraint(constraints, config.Target) + }) } func replaceLabelConstraints(config *ReplaceLabelConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d label replacement(s) allowed.") - if len(config.AllowedTransitions) > 0 { - pairs := make([]string, len(config.AllowedTransitions)) - for i, transition := range config.AllowedTransitions { - pairs[i] = fmt.Sprintf("%q → %q", transition.From, transition.To) + return buildConstraints(config, func(config *ReplaceLabelConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d label replacement(s) allowed.") + if len(config.AllowedTransitions) > 0 { + pairs := make([]string, len(config.AllowedTransitions)) + for i, transition := range config.AllowedTransitions { + pairs[i] = fmt.Sprintf("%q → %q", transition.From, transition.To) + } + *constraints = append(*constraints, fmt.Sprintf("Only these label transitions are allowed: %s.", formatStringList(pairs))) } - constraints = append(constraints, fmt.Sprintf("Only these label transitions are allowed: %s.", formatStringList(pairs))) - } - if len(config.AllowedAdd) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels can be added: %s.", formatStringList(config.AllowedAdd))) - } - if len(config.AllowedRemove) > 0 { - constraints = append(constraints, fmt.Sprintf("Only these labels can be removed: %s.", formatStringList(config.AllowedRemove))) - } - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - return constraints + if len(config.AllowedAdd) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels can be added: %s.", formatStringList(config.AllowedAdd))) + } + if len(config.AllowedRemove) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only these labels can be removed: %s.", formatStringList(config.AllowedRemove))) + } + appendTargetConstraint(constraints, config.Target) + }) } func addReviewerConstraints(config *AddReviewerConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d reviewer(s) can be added.") - return constraints + return buildConstraints(config, func(config *AddReviewerConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d reviewer(s) can be added.") + }) } func updateIssueConstraints(config *UpdateIssuesConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d issue(s) can be updated.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - titlePrefix := config.TitlePrefix - if config.RequiredTitlePrefix != "" { - titlePrefix = config.RequiredTitlePrefix - } - if titlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("The target issue title must start with %q.", titlePrefix)) - } - if config.Title != nil && *config.Title { - constraints = append(constraints, "Title updates are allowed.") - } - if config.Body != nil && *config.Body { - constraints = append(constraints, "Body updates are allowed.") - } - if config.Status != nil && *config.Status { - constraints = append(constraints, "Status updates (open/closed) are allowed.") - } - return constraints + return buildConstraints(config, func(config *UpdateIssuesConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be updated.") + appendTargetConstraint(constraints, config.Target) + titlePrefix := config.TitlePrefix + if config.RequiredTitlePrefix != "" { + titlePrefix = config.RequiredTitlePrefix + } + appendRequiredTitlePrefixConstraint(constraints, titlePrefix, "The target issue title must start with %q.") + if config.Title != nil && *config.Title { + *constraints = append(*constraints, "Title updates are allowed.") + } + if config.Body != nil && *config.Body { + *constraints = append(*constraints, "Body updates are allowed.") + } + if config.Status != nil && *config.Status { + *constraints = append(*constraints, "Status updates (open/closed) are allowed.") + } + }) } func updatePullRequestConstraints(config *UpdatePullRequestsConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d pull request(s) can be updated.") - if config.Target != "" { - constraints = append(constraints, fmt.Sprintf("Target: %s.", config.Target)) - } - if len(config.RequiredLabels) > 0 { - constraints = append(constraints, fmt.Sprintf("Only PRs with labels %v can be updated.", config.RequiredLabels)) - } - if config.RequiredTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("Only PRs with title prefix %q can be updated.", config.RequiredTitlePrefix)) - } - return constraints + return buildConstraints(config, func(config *UpdatePullRequestsConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be updated.") + appendTargetConstraint(constraints, config.Target) + if len(config.RequiredLabels) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Only PRs with labels %v can be updated.", config.RequiredLabels)) + } + appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be updated.") + }) } func pushToPullRequestBranchConstraints(config *PushToPullRequestBranchConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d push(es) can be made.") - if config.TitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("The target pull request title must start with %q.", config.TitlePrefix)) - } - return constraints + return buildConstraints(config, func(config *PushToPullRequestBranchConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d push(es) can be made.") + appendRequiredTitlePrefixConstraint(constraints, config.TitlePrefix, "The target pull request title must start with %q.") + }) } func uploadAssetConstraints(config *UploadAssetsConfig) []string { - if config == nil { - return nil - } - - toolDescriptionEnhancerLog.Printf("Found upload_asset config: max=%v, maxSizeKB=%d, allowedExts=%v", config.Max, config.MaxSizeKB, config.AllowedExts) + return buildConstraints(config, func(config *UploadAssetsConfig, constraints *[]string) { + toolDescriptionEnhancerLog.Printf("Found upload_asset config: max=%v, maxSizeKB=%d, allowedExts=%v", config.Max, config.MaxSizeKB, config.AllowedExts) - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d asset(s) can be uploaded.") - if config.MaxSizeKB > 0 { - constraints = append(constraints, fmt.Sprintf("Maximum file size: %dKB.", config.MaxSizeKB)) - } - if len(config.AllowedExts) > 0 { - constraints = append(constraints, fmt.Sprintf("Allowed file extensions: %v.", config.AllowedExts)) - } - return constraints + appendMaxConstraint(constraints, config.Max, "Maximum %d asset(s) can be uploaded.") + if config.MaxSizeKB > 0 { + *constraints = append(*constraints, fmt.Sprintf("Maximum file size: %dKB.", config.MaxSizeKB)) + } + if len(config.AllowedExts) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Allowed file extensions: %v.", config.AllowedExts)) + } + }) } func updateReleaseConstraints(config *UpdateReleaseConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d release(s) can be updated.") - return constraints + return buildConstraints(config, func(config *UpdateReleaseConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d release(s) can be updated.") + }) } func missingToolConstraints(config *MissingToolConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d missing tool report(s) can be created.") - return constraints + return buildConstraints(config, func(config *MissingToolConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d missing tool report(s) can be created.") + }) } func linkSubIssueConstraints(config *LinkSubIssueConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d sub-issue link(s) can be created.") - if config.ParentTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("The parent issue title must start with %q.", config.ParentTitlePrefix)) - } - if config.SubTitlePrefix != "" { - constraints = append(constraints, fmt.Sprintf("The sub-issue title must start with %q.", config.SubTitlePrefix)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Sub-issues will be linked in repository %q.", config.TargetRepoSlug)) - } - if len(config.AllowedRepos) > 0 { - constraints = append(constraints, fmt.Sprintf("Sub-issue linking can target these repositories: %v.", config.AllowedRepos)) - } - return constraints + return buildConstraints(config, func(config *LinkSubIssueConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d sub-issue link(s) can be created.") + if config.ParentTitlePrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("The parent issue title must start with %q.", config.ParentTitlePrefix)) + } + if config.SubTitlePrefix != "" { + *constraints = append(*constraints, fmt.Sprintf("The sub-issue title must start with %q.", config.SubTitlePrefix)) + } + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Sub-issues will be linked in repository %q.") + if len(config.AllowedRepos) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Sub-issue linking can target these repositories: %v.", config.AllowedRepos)) + } + }) } func assignMilestoneConstraints(config *AssignMilestoneConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d milestone assignment(s) can be made.") - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Milestones will be assigned in repository %q.", config.TargetRepoSlug)) - } - return constraints + return buildConstraints(config, func(config *AssignMilestoneConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d milestone assignment(s) can be made.") + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Milestones will be assigned in repository %q.") + }) } func assignToAgentConstraints(config *AssignToAgentConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d issue(s) can be assigned to agent.") - if config.BaseBranch != "" { - constraints = append(constraints, fmt.Sprintf("Pull requests will target the %q branch.", config.BaseBranch)) - } - if config.TargetRepoSlug != "" { - constraints = append(constraints, fmt.Sprintf("Issues will be assigned to agent in repository %q.", config.TargetRepoSlug)) - } - if len(config.AllowedRepos) > 0 { - constraints = append(constraints, fmt.Sprintf("Agent assignment can target these repositories: %v.", config.AllowedRepos)) - } - return constraints + return buildConstraints(config, func(config *AssignToAgentConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be assigned to agent.") + if config.BaseBranch != "" { + *constraints = append(*constraints, fmt.Sprintf("Pull requests will target the %q branch.", config.BaseBranch)) + } + appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issues will be assigned to agent in repository %q.") + if len(config.AllowedRepos) > 0 { + *constraints = append(*constraints, fmt.Sprintf("Agent assignment can target these repositories: %v.", config.AllowedRepos)) + } + }) } func updateProjectConstraints(config *UpdateProjectConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d project operation(s) can be performed.") - if config.Project != "" { - constraints = append(constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) - } - return constraints + return buildConstraints(config, func(config *UpdateProjectConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d project operation(s) can be performed.") + if config.Project != "" { + *constraints = append(*constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) + } + }) } func createProjectStatusUpdateConstraints(config *CreateProjectStatusUpdateConfig) []string { - if config == nil { - return nil - } - - var constraints []string - appendMaxConstraint(&constraints, config.Max, "Maximum %d status update(s) can be created.") - if config.Project != "" { - constraints = append(constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) - } - return constraints + return buildConstraints(config, func(config *CreateProjectStatusUpdateConfig, constraints *[]string) { + appendMaxConstraint(constraints, config.Max, "Maximum %d status update(s) can be created.") + if config.Project != "" { + *constraints = append(*constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) + } + }) } From 7ae77ed8b3ac6053f19bfe31d47f466297078d19 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:50:02 +0000 Subject: [PATCH 3/4] Add draft ADR-52157 for generic buildConstraints helper in tool_description_enhancer --- ...ts-helper-for-tool-description-enhancer.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md diff --git a/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md b/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md new file mode 100644 index 00000000000..0ac0d9db8a6 --- /dev/null +++ b/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md @@ -0,0 +1,44 @@ +# ADR-52157: Generic `buildConstraints` Helper for Tool Description Enhancer + +**Date**: 2026-08-11 +**Status**: Draft +**Deciders**: pelikhan (copilot-swe-agent) + +--- + +### Context + +`pkg/workflow/tool_description_enhancer.go` contained approximately 33 per-tool constraint builder functions (e.g., `createIssueConstraints`, `closeDiscussionConstraints`, `updatePullRequestConstraints`). Every builder repeated the same 4-line skeleton: a nil guard (`if config == nil { return nil }`), a local slice declaration (`var constraints []string`), a series of field-conditional appends, and a return statement. The nil guard alone appeared in every function; `appendTargetConstraint`, `appendTargetRepoSlugConstraint`, and `appendRequiredTitlePrefixConstraint` patterns appeared 12, 13, and 5 times respectively. The duplication inflated the file to 740 lines and created a risk of inconsistency if the shared pattern needed to change across all builders. + +### Decision + +We will introduce a generic `buildConstraints[T any](config *T, build func(*T, *[]string)) []string` helper that centralizes the nil-guard and slice-setup boilerplate. We will also extract `appendTargetConstraint`, `appendTargetRepoSlugConstraint`, and `appendRequiredTitlePrefixConstraint` as standalone helpers for the three most common field-gate patterns. All existing `*Constraints` functions will be rewritten to delegate to `buildConstraints`, keeping only their tool-specific field logic. No observable behavior change is made; all message strings, field selectors, and special cases (e.g., `addCommentConstraints` always appending a trailing constraint) are preserved exactly. + +### Alternatives Considered + +#### Alternative 1: Keep the duplication (status quo) + +Each builder function independently handles its nil check and slice initialization, as it did before. The pattern is simple to read in isolation and requires no shared abstraction. This was rejected because the repeated boilerplate created a real maintenance risk: a future change to the nil-check behavior or the slice initialization strategy would require touching 33 functions, and an inconsistent edit would not be caught by the compiler. + +#### Alternative 2: Code generation + +A code generator (e.g., `go generate` with a template) could produce all builder functions from a declarative spec. This would fully eliminate duplication and could also auto-generate tests. It was rejected because it introduces toolchain complexity (template authoring, build step, generated-file tracking) that is disproportionate to the problem: the field-specific logic inside each builder is already small and easily readable without generation. A simple helper function achieves the same nil-guard deduplication without the build tooling overhead. + +### Consequences + +#### Positive +- The file shrinks from 740 to 585 lines (a ~21% reduction), making it easier to scan and navigate. +- The nil-check and slice-initialization behavior is now consistent by construction across all builders — a future change to the shared pattern requires editing one function instead of 33. +- New tool builders only need to implement field-specific logic inside the callback, reducing the error surface for contributors adding new tools. + +#### Negative +- `buildConstraints` uses Go generics (type parameter `[T any]`), which requires Go 1.18+. Readers unfamiliar with generic callback patterns in Go may need a moment to parse the abstraction. +- `addCommentConstraints` is a special case: it must return a non-nil slice even when `config` is nil (to append the trailing `"Supports reply_to_id for discussion threading."` constraint). It uses `buildConstraints` in an asymmetric way (`constraints := buildConstraints(...)` followed by `append(constraints, ...)`), making it slightly less uniform than the other builders. + +#### Neutral +- All declarative differences between builders (message strings, field selectors, ordering) remain visible inside each function's callback, so the per-tool logic stays fully readable and easy to extend. +- The three new `append*Constraint` helpers are package-private and co-located with the existing `appendMaxConstraint` helper, following the established pattern in the file. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 1e5bd53f6f96616b02c450eaacf3511862d5ecd1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:25:29 +0000 Subject: [PATCH 4/4] Consolidate string-gate constraint helpers into appendStringConstraint Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...ts-helper-for-tool-description-enhancer.md | 10 +- pkg/workflow/tool_description_enhancer.go | 117 ++++++------------ .../tool_description_enhancer_test.go | 14 +++ 3 files changed, 60 insertions(+), 81 deletions(-) diff --git a/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md b/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md index 0ac0d9db8a6..6ac6d2925f6 100644 --- a/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md +++ b/docs/adr/52157-generic-buildconstraints-helper-for-tool-description-enhancer.md @@ -8,11 +8,11 @@ ### Context -`pkg/workflow/tool_description_enhancer.go` contained approximately 33 per-tool constraint builder functions (e.g., `createIssueConstraints`, `closeDiscussionConstraints`, `updatePullRequestConstraints`). Every builder repeated the same 4-line skeleton: a nil guard (`if config == nil { return nil }`), a local slice declaration (`var constraints []string`), a series of field-conditional appends, and a return statement. The nil guard alone appeared in every function; `appendTargetConstraint`, `appendTargetRepoSlugConstraint`, and `appendRequiredTitlePrefixConstraint` patterns appeared 12, 13, and 5 times respectively. The duplication inflated the file to 740 lines and created a risk of inconsistency if the shared pattern needed to change across all builders. +`pkg/workflow/tool_description_enhancer.go` contained approximately 33 per-tool constraint builder functions (e.g., `createIssueConstraints`, `closeDiscussionConstraints`, `updatePullRequestConstraints`). Every builder repeated the same 4-line skeleton: a nil guard (`if config == nil { return nil }`), a local slice declaration (`var constraints []string`), a series of field-conditional appends, and a return statement. The nil guard alone appeared in every function; `appendTargetConstraint`, `appendTargetRepoSlugConstraint`, and `appendRequiredTitlePrefixConstraint` patterns appeared 12, 13, and 5 times respectively. The duplication inflated the file to 740 lines and created a risk of inconsistency if the shared pattern needed to change across all builders. Beyond the nil guard, a second pattern repeated ~18 times: gate on a non-empty string field, then `fmt.Sprintf` a tool-specific message with that value. ### Decision -We will introduce a generic `buildConstraints[T any](config *T, build func(*T, *[]string)) []string` helper that centralizes the nil-guard and slice-setup boilerplate. We will also extract `appendTargetConstraint`, `appendTargetRepoSlugConstraint`, and `appendRequiredTitlePrefixConstraint` as standalone helpers for the three most common field-gate patterns. All existing `*Constraints` functions will be rewritten to delegate to `buildConstraints`, keeping only their tool-specific field logic. No observable behavior change is made; all message strings, field selectors, and special cases (e.g., `addCommentConstraints` always appending a trailing constraint) are preserved exactly. +We will introduce a generic `buildConstraints[T any](config *T, build func(*T, *[]string)) []string` helper that centralizes the nil-guard and slice-setup boilerplate. We will also extract a single `appendStringConstraint(constraints *[]string, value, format string)` helper covering the non-empty-string field gate, with `appendTargetConstraint` retained as a thin wrapper for the very common `"Target: %s."` message. Every non-empty-string gate in the file delegates to `appendStringConstraint`, so the same pattern is never hand-rolled. All existing `*Constraints` functions will be rewritten to delegate to `buildConstraints`, keeping only their tool-specific field logic. No observable behavior change is made; all message strings, field selectors, and special cases (e.g., `addCommentConstraints` always appending a trailing constraint) are preserved exactly. ### Alternatives Considered @@ -27,17 +27,17 @@ A code generator (e.g., `go generate` with a template) could produce all builder ### Consequences #### Positive -- The file shrinks from 740 to 585 lines (a ~21% reduction), making it easier to scan and navigate. +- The file shrinks from 740 to 550 lines (a ~26% reduction), making it easier to scan and navigate. - The nil-check and slice-initialization behavior is now consistent by construction across all builders — a future change to the shared pattern requires editing one function instead of 33. - New tool builders only need to implement field-specific logic inside the callback, reducing the error surface for contributors adding new tools. #### Negative - `buildConstraints` uses Go generics (type parameter `[T any]`), which requires Go 1.18+. Readers unfamiliar with generic callback patterns in Go may need a moment to parse the abstraction. -- `addCommentConstraints` is a special case: it must return a non-nil slice even when `config` is nil (to append the trailing `"Supports reply_to_id for discussion threading."` constraint). It uses `buildConstraints` in an asymmetric way (`constraints := buildConstraints(...)` followed by `append(constraints, ...)`), making it slightly less uniform than the other builders. +- `addCommentConstraints` is a special case: it must return a non-nil slice even when `config` is nil (to append the trailing `"Supports reply_to_id for discussion threading."` constraint). It uses `buildConstraints` in an asymmetric way (`constraints := buildConstraints(...)` followed by `append(constraints, ...)`), making it slightly less uniform than the other builders. `TestAddCommentConstraintsNilConfig` pins this contract so a future cleanup cannot silently drop the trailing constraint. #### Neutral - All declarative differences between builders (message strings, field selectors, ordering) remain visible inside each function's callback, so the per-tool logic stays fully readable and easy to extend. -- The three new `append*Constraint` helpers are package-private and co-located with the existing `appendMaxConstraint` helper, following the established pattern in the file. +- The new `appendStringConstraint` helper is package-private and co-located with the existing `appendMaxConstraint` helper, following the established pattern in the file. Call sites stay self-descriptive through their `format` argument rather than through distinct helper names. --- diff --git a/pkg/workflow/tool_description_enhancer.go b/pkg/workflow/tool_description_enhancer.go index 03d00bda559..09740d49167 100644 --- a/pkg/workflow/tool_description_enhancer.go +++ b/pkg/workflow/tool_description_enhancer.go @@ -140,27 +140,18 @@ func buildConstraints[T any](config *T, build func(config *T, constraints *[]str return constraints } -// appendTargetConstraint appends the common "Target: ." constraint when target is set. -func appendTargetConstraint(constraints *[]string, target string) { - if target != "" { - *constraints = append(*constraints, fmt.Sprintf("Target: %s.", target)) - } -} - -// appendTargetRepoSlugConstraint appends a formatted constraint describing the target -// repository slug when set. format must contain a single %q/%s verb for the slug. -func appendTargetRepoSlugConstraint(constraints *[]string, targetRepoSlug, format string) { - if targetRepoSlug != "" { - *constraints = append(*constraints, fmt.Sprintf(format, targetRepoSlug)) +// appendStringConstraint appends a formatted constraint when value is non-empty. +// format must contain a single %q/%s verb for the value; each call site stays +// self-descriptive through its format string. +func appendStringConstraint(constraints *[]string, value, format string) { + if value != "" { + *constraints = append(*constraints, fmt.Sprintf(format, value)) } } -// appendRequiredTitlePrefixConstraint appends a formatted constraint describing a -// required title prefix when set. format must contain a single %q/%s verb for the prefix. -func appendRequiredTitlePrefixConstraint(constraints *[]string, prefix, format string) { - if prefix != "" { - *constraints = append(*constraints, fmt.Sprintf(format, prefix)) - } +// appendTargetConstraint appends the common "Target: ." constraint when target is set. +func appendTargetConstraint(constraints *[]string, target string) { + appendStringConstraint(constraints, target, "Target: %s.") } // enhanceToolDescription adds configuration-specific constraints to tool descriptions @@ -197,9 +188,7 @@ func createIssueConstraints(config *CreateIssuesConfig) []string { toolDescriptionEnhancerLog.Printf("Found create_issue config: max=%v, titlePrefix=%s", config.Max, config.TitlePrefix) appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be created.") - if config.TitlePrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } + appendStringConstraint(constraints, config.TitlePrefix, "Title will be prefixed with %q.") if len(config.Labels) > 0 { *constraints = append(*constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) } @@ -210,7 +199,7 @@ func createIssueConstraints(config *CreateIssuesConfig) []string { if len(config.Assignees) > 0 { *constraints = append(*constraints, fmt.Sprintf("Assignees %s will be automatically assigned.", formatStringList(config.Assignees))) } - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issues will be created in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Issues will be created in repository %q.") if config.RequireTemporaryID { *constraints = append(*constraints, "temporary_id is required.") } @@ -224,17 +213,15 @@ func setIssueFieldConstraints(config *SetIssueFieldConfig) []string { return buildConstraints(config, func(config *SetIssueFieldConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d issue field update(s) can be made.") appendAllowedIssueFieldsConstraint(constraints, config.AllowedFields) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issue fields will be updated in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Issue fields will be updated in repository %q.") }) } func createAgentSessionConstraints(config *CreateAgentSessionConfig) []string { return buildConstraints(config, func(config *CreateAgentSessionConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d agent task(s) can be created.") - if config.Base != "" { - *constraints = append(*constraints, fmt.Sprintf("Base branch for tasks: %q.", config.Base)) - } - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Tasks will be created in repository %q.") + appendStringConstraint(constraints, config.Base, "Base branch for tasks: %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Tasks will be created in repository %q.") if len(config.AllowedRepos) > 0 { *constraints = append(*constraints, fmt.Sprintf("Sessions can target these repositories: %v.", config.AllowedRepos)) } @@ -244,16 +231,12 @@ func createAgentSessionConstraints(config *CreateAgentSessionConfig) []string { func createDiscussionConstraints(config *CreateDiscussionsConfig) []string { return buildConstraints(config, func(config *CreateDiscussionsConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d discussion(s) can be created.") - if config.TitlePrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } - if config.Category != "" { - *constraints = append(*constraints, fmt.Sprintf("Discussions will be created in category %q.", config.Category)) - } + appendStringConstraint(constraints, config.TitlePrefix, "Title will be prefixed with %q.") + appendStringConstraint(constraints, config.Category, "Discussions will be created in category %q.") if len(config.AllowedLabels) > 0 { *constraints = append(*constraints, fmt.Sprintf("Only these labels are allowed: %s.", formatStringList(config.AllowedLabels))) } - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Discussions will be created in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Discussions will be created in repository %q.") }) } @@ -261,8 +244,8 @@ func closeDiscussionConstraints(config *CloseDiscussionsConfig) []string { return buildConstraints(config, func(config *CloseDiscussionsConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d discussion(s) can be closed.") appendTargetConstraint(constraints, config.Target) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Discussions will be closed in repository %q.") - appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only discussions with title prefix %q can be closed.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Discussions will be closed in repository %q.") + appendStringConstraint(constraints, config.RequiredTitlePrefix, "Only discussions with title prefix %q can be closed.") if config.AllowBody != nil && !*config.AllowBody { *constraints = append(*constraints, "Closing comments are disabled: do not include a body field.") } @@ -293,7 +276,7 @@ func closeIssueConstraints(config *CloseIssuesConfig) []string { return buildConstraints(config, func(config *CloseIssuesConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be closed.") appendTargetConstraint(constraints, config.Target) - appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only issues with title prefix %q can be closed.") + appendStringConstraint(constraints, config.RequiredTitlePrefix, "Only issues with title prefix %q can be closed.") if config.AllowBody != nil && !*config.AllowBody { *constraints = append(*constraints, "Closing comments are disabled: do not include a body field.") } @@ -304,18 +287,18 @@ func closePullRequestConstraints(config *ClosePullRequestsConfig) []string { return buildConstraints(config, func(config *ClosePullRequestsConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be closed.") appendTargetConstraint(constraints, config.Target) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Pull requests will be closed in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Pull requests will be closed in repository %q.") if len(config.RequiredLabels) > 0 { *constraints = append(*constraints, fmt.Sprintf("Only PRs with labels %v can be closed.", config.RequiredLabels)) } - appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be closed.") + appendStringConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be closed.") }) } func markPullRequestAsReadyForReviewConstraints(config *MarkPullRequestAsReadyForReviewConfig) []string { return buildConstraints(config, func(config *MarkPullRequestAsReadyForReviewConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be marked as ready for review.") - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Pull requests will be marked as ready in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Pull requests will be marked as ready in repository %q.") }) } @@ -323,7 +306,7 @@ func addCommentConstraints(config *AddCommentsConfig) []string { constraints := buildConstraints(config, func(config *AddCommentsConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d comment(s) can be added.") appendTargetConstraint(constraints, config.Target) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Comments will be added in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Comments will be added in repository %q.") if config.NormalizeClosingKeywords != nil && *config.NormalizeClosingKeywords { *constraints = append(*constraints, "Backtick-wrapped issue-closing keyword references (e.g. `Closes #1`) in the body field will be automatically normalized to plain text.") } @@ -336,12 +319,8 @@ func createPullRequestConstraints(config *CreatePullRequestsConfig) []string { toolDescriptionEnhancerLog.Printf("Found create_pull_request config: max=%v, titlePrefix=%s, draft=%v", config.Max, config.TitlePrefix, config.Draft) appendMaxConstraint(constraints, config.Max, "Maximum %d pull request(s) can be created.") - if config.BranchPrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("Branch name will be prefixed with %q.", config.BranchPrefix)) - } - if config.TitlePrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("Title will be prefixed with %q.", config.TitlePrefix)) - } + appendStringConstraint(constraints, config.BranchPrefix, "Branch name will be prefixed with %q.") + appendStringConstraint(constraints, config.TitlePrefix, "Title will be prefixed with %q.") if len(config.Labels) > 0 { *constraints = append(*constraints, fmt.Sprintf("Labels %s will be automatically added.", formatStringList(config.Labels))) } @@ -369,9 +348,7 @@ func createPullRequestConstraints(config *CreatePullRequestsConfig) []string { func createPullRequestReviewCommentConstraints(config *CreatePullRequestReviewCommentsConfig) []string { return buildConstraints(config, func(config *CreatePullRequestReviewCommentsConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d review comment(s) can be created.") - if config.Side != "" { - *constraints = append(*constraints, fmt.Sprintf("Comments will be on the %s side of the diff.", config.Side)) - } + appendStringConstraint(constraints, config.Side, "Comments will be on the %s side of the diff.") }) } @@ -379,7 +356,7 @@ func submitPullRequestReviewConstraints(config *SubmitPullRequestReviewConfig) [ return buildConstraints(config, func(config *SubmitPullRequestReviewConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d review(s) can be submitted.") appendTargetConstraint(constraints, config.Target) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Reviews will be submitted in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Reviews will be submitted in repository %q.") }) } @@ -393,7 +370,7 @@ func dismissPullRequestReviewConstraints(config *DismissPullRequestReviewConfig) return buildConstraints(config, func(config *DismissPullRequestReviewConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d review dismissal(s) can be performed.") appendTargetConstraint(constraints, config.Target) - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Review dismissals will be performed in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Review dismissals will be performed in repository %q.") *constraints = append(*constraints, "justification must contain at least 20 characters.") }) } @@ -413,9 +390,7 @@ func createCodeScanningAlertConstraints(config *CreateCodeScanningAlertsConfig) func createCheckRunConstraints(config *CreateCheckRunConfig) []string { return buildConstraints(config, func(config *CreateCheckRunConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d check run(s) can be created.") - if config.Name != "" { - *constraints = append(*constraints, fmt.Sprintf("Check run name: %q.", config.Name)) - } + appendStringConstraint(constraints, config.Name, "Check run name: %q.") }) } @@ -473,7 +448,7 @@ func updateIssueConstraints(config *UpdateIssuesConfig) []string { if config.RequiredTitlePrefix != "" { titlePrefix = config.RequiredTitlePrefix } - appendRequiredTitlePrefixConstraint(constraints, titlePrefix, "The target issue title must start with %q.") + appendStringConstraint(constraints, titlePrefix, "The target issue title must start with %q.") if config.Title != nil && *config.Title { *constraints = append(*constraints, "Title updates are allowed.") } @@ -493,14 +468,14 @@ func updatePullRequestConstraints(config *UpdatePullRequestsConfig) []string { if len(config.RequiredLabels) > 0 { *constraints = append(*constraints, fmt.Sprintf("Only PRs with labels %v can be updated.", config.RequiredLabels)) } - appendRequiredTitlePrefixConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be updated.") + appendStringConstraint(constraints, config.RequiredTitlePrefix, "Only PRs with title prefix %q can be updated.") }) } func pushToPullRequestBranchConstraints(config *PushToPullRequestBranchConfig) []string { return buildConstraints(config, func(config *PushToPullRequestBranchConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d push(es) can be made.") - appendRequiredTitlePrefixConstraint(constraints, config.TitlePrefix, "The target pull request title must start with %q.") + appendStringConstraint(constraints, config.TitlePrefix, "The target pull request title must start with %q.") }) } @@ -533,13 +508,9 @@ func missingToolConstraints(config *MissingToolConfig) []string { func linkSubIssueConstraints(config *LinkSubIssueConfig) []string { return buildConstraints(config, func(config *LinkSubIssueConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d sub-issue link(s) can be created.") - if config.ParentTitlePrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("The parent issue title must start with %q.", config.ParentTitlePrefix)) - } - if config.SubTitlePrefix != "" { - *constraints = append(*constraints, fmt.Sprintf("The sub-issue title must start with %q.", config.SubTitlePrefix)) - } - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Sub-issues will be linked in repository %q.") + appendStringConstraint(constraints, config.ParentTitlePrefix, "The parent issue title must start with %q.") + appendStringConstraint(constraints, config.SubTitlePrefix, "The sub-issue title must start with %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Sub-issues will be linked in repository %q.") if len(config.AllowedRepos) > 0 { *constraints = append(*constraints, fmt.Sprintf("Sub-issue linking can target these repositories: %v.", config.AllowedRepos)) } @@ -549,17 +520,15 @@ func linkSubIssueConstraints(config *LinkSubIssueConfig) []string { func assignMilestoneConstraints(config *AssignMilestoneConfig) []string { return buildConstraints(config, func(config *AssignMilestoneConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d milestone assignment(s) can be made.") - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Milestones will be assigned in repository %q.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Milestones will be assigned in repository %q.") }) } func assignToAgentConstraints(config *AssignToAgentConfig) []string { return buildConstraints(config, func(config *AssignToAgentConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d issue(s) can be assigned to agent.") - if config.BaseBranch != "" { - *constraints = append(*constraints, fmt.Sprintf("Pull requests will target the %q branch.", config.BaseBranch)) - } - appendTargetRepoSlugConstraint(constraints, config.TargetRepoSlug, "Issues will be assigned to agent in repository %q.") + appendStringConstraint(constraints, config.BaseBranch, "Pull requests will target the %q branch.") + appendStringConstraint(constraints, config.TargetRepoSlug, "Issues will be assigned to agent in repository %q.") if len(config.AllowedRepos) > 0 { *constraints = append(*constraints, fmt.Sprintf("Agent assignment can target these repositories: %v.", config.AllowedRepos)) } @@ -569,17 +538,13 @@ func assignToAgentConstraints(config *AssignToAgentConfig) []string { func updateProjectConstraints(config *UpdateProjectConfig) []string { return buildConstraints(config, func(config *UpdateProjectConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d project operation(s) can be performed.") - if config.Project != "" { - *constraints = append(*constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) - } + appendStringConstraint(constraints, config.Project, "Default project URL: %q.") }) } func createProjectStatusUpdateConstraints(config *CreateProjectStatusUpdateConfig) []string { return buildConstraints(config, func(config *CreateProjectStatusUpdateConfig, constraints *[]string) { appendMaxConstraint(constraints, config.Max, "Maximum %d status update(s) can be created.") - if config.Project != "" { - *constraints = append(*constraints, fmt.Sprintf("Default project URL: %q.", config.Project)) - } + appendStringConstraint(constraints, config.Project, "Default project URL: %q.") }) } diff --git a/pkg/workflow/tool_description_enhancer_test.go b/pkg/workflow/tool_description_enhancer_test.go index db8dd20ecb9..dae8ac92b07 100644 --- a/pkg/workflow/tool_description_enhancer_test.go +++ b/pkg/workflow/tool_description_enhancer_test.go @@ -281,3 +281,17 @@ func TestEnhanceToolDescriptionNormalizeClosingKeywordsFalseCreatePullRequest(t t.Fatalf("did not expect normalize-closing-keywords note when disabled, got: %s", description) } } + +func TestAddCommentConstraintsNilConfig(t *testing.T) { + constraints := addCommentConstraints(nil) + + want := []string{"Supports reply_to_id for discussion threading."} + if len(constraints) != len(want) { + t.Fatalf("expected %d constraint(s), got %d: %v", len(want), len(constraints), constraints) + } + for i, expected := range want { + if constraints[i] != expected { + t.Fatalf("constraint %d: expected %q, got %q", i, expected, constraints[i]) + } + } +}