diff --git a/actions/setup/js/create_issue.test.cjs b/actions/setup/js/create_issue.test.cjs index 2736a4cf3b3..aeeb6854ccc 100644 --- a/actions/setup/js/create_issue.test.cjs +++ b/actions/setup/js/create_issue.test.cjs @@ -491,6 +491,32 @@ describe("create_issue", () => { expect(mockGithub.rest.issues.create).toHaveBeenCalledTimes(1); }); + it("should accept stringified boolean and integer deduplication values from workflow expressions", async () => { + const exactMatchHandler = await main({ + deduplicate_by_title: "true", + }); + + const firstExact = await exactMatchHandler({ title: "Duplicate title" }); + const secondExact = await exactMatchHandler({ title: "Duplicate title" }); + + expect(firstExact.success).toBe(true); + expect(secondExact.success).toBe(true); + expect(secondExact.dropped_duplicate).toBe(true); + expect(secondExact.dedup_source).toBe("within-run"); + + const fuzzyHandler = await main({ + deduplicate_by_title: "1", + }); + + const firstFuzzy = await fuzzyHandler({ title: "Fix login bug" }); + const secondFuzzy = await fuzzyHandler({ title: "Fix login bag" }); + + expect(firstFuzzy.success).toBe(true); + expect(secondFuzzy.success).toBe(true); + expect(secondFuzzy.dropped_duplicate).toBe(true); + expect(secondFuzzy.duplicate_distance).toBe(1); + }); + it("should drop duplicates that already exist in the repository", async () => { mockGithub.rest.search.issuesAndPullRequests .mockResolvedValueOnce({ diff --git a/actions/setup/js/issue_title_dedup.cjs b/actions/setup/js/issue_title_dedup.cjs index b6227e62fac..216c2147993 100644 --- a/actions/setup/js/issue_title_dedup.cjs +++ b/actions/setup/js/issue_title_dedup.cjs @@ -19,10 +19,19 @@ function parseDeduplicateByTitle(value) { if (value === true) { return { enabled: true, maxDistance: 0 }; } + if (value === "false") { + return { enabled: false, maxDistance: 0 }; + } + if (value === "true") { + return { enabled: true, maxDistance: 0 }; + } + if (typeof value === "string" && /^\d+$/.test(value)) { + value = Number.parseInt(value, 10); + } if (typeof value === "number" && Number.isFinite(value) && Number.isInteger(value) && value >= 0 && value <= MAX_DEDUPLICATE_BY_TITLE_DISTANCE) { return { enabled: true, maxDistance: value }; } - throw new Error(`deduplicate-by-title must be a boolean or a non-negative integer (0-${MAX_DEDUPLICATE_BY_TITLE_DISTANCE})`); + throw new Error(`deduplicate-by-title must be a boolean, a boolean-like string, or a non-negative integer (0-${MAX_DEDUPLICATE_BY_TITLE_DISTANCE})`); } /** diff --git a/docs/adr/43345-templatable-bool-or-int-type-for-config-fields.md b/docs/adr/43345-templatable-bool-or-int-type-for-config-fields.md new file mode 100644 index 00000000000..0c1dd60d518 --- /dev/null +++ b/docs/adr/43345-templatable-bool-or-int-type-for-config-fields.md @@ -0,0 +1,44 @@ +# ADR-43345: Introduce TemplatableBoolOrInt for Config Fields Accepting Boolean, Integer, or GitHub Actions Expressions + +**Date**: 2026-07-04 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The `deduplicate-by-title` field in `CreateIssuesConfig` was typed as `any`, which caused the field's value to be silently dropped when building the emitted handler config for the `create_issue` safe output. Additionally, users need to pass GitHub Actions expression strings (e.g., `${{ inputs.dedup }}`) to this field so that deduplication mode can be configured dynamically at runtime; the previous `any` type and schema definition only accepted booleans and integers, rejecting expression strings at validation time. A type-safe, serialization-aware representation was needed to correctly parse all three input forms (boolean, integer 0–100, or expression string) and emit them as their appropriate JSON types (JSON boolean, JSON number, or JSON string respectively). + +### Decision + +We decided to introduce a new string-backed type `TemplatableBoolOrInt` in `pkg/workflow/templatables.go` with custom YAML and JSON marshaling. The type stores all three forms internally as a Go string (`"true"`, `"false"`, a decimal integer, or a `${{ … }}` expression), then emits the correct native JSON value via `MarshalJSON`/`ToValue`. `CreateIssuesConfig.DeduplicateByTitle` was changed from `any` to `*TemplatableBoolOrInt`, and the handler registry was updated to call a new `AddTemplatableBoolOrInt` builder method, completing the emission path that was previously silently broken. + +### Alternatives Considered + +#### Alternative 1: Keep `any` type and add post-parse type-switch conversion + +The `AddBoolOrInt` builder method already performs a type-switch on `any` values to emit booleans or integers. The emission bug could have been fixed by simply wiring `DeduplicateByTitle` into `AddBoolOrInt` without changing the field type. However, `any` carries no type constraints at parse time, so schema validation alone blocks expression strings from entering the field. Expression strings would have required additional ad-hoc handling that bypasses the type system and is invisible in struct definitions. + +#### Alternative 2: Explicit union struct (`struct { Bool *bool; Int *int; Expr *string }`) + +A struct with three optional fields makes the valid value space explicit in the type system with no custom marshalers beyond implementing `yaml.Unmarshaler`. This is more verbose and requires callers to inspect which field is set. It also complicates the JSON schema `$defs` entry and adds branching in every emission site. The string-backed approach centralises all conversion logic inside the type and allows the schema to reference a single `$defs/templatable_bool_or_int` entry. + +### Consequences + +#### Positive +- `deduplicate-by-title` is now correctly emitted in the handler config for all three input forms, fixing the silent-drop bug that caused deduplication to never activate when configured. +- Users can pass GitHub Actions expression strings (e.g., `${{ inputs.dedup }}`) to `deduplicate-by-title`; the value is stored and emitted as a JSON string for runtime evaluation, enabling dynamic configuration without a schema change. +- The JSON schema `$defs/templatable_bool_or_int` definition is reusable for future fields that need the same three-way acceptance. + +#### Negative +- `TemplatableBoolOrInt` introduces a custom string type with non-trivial YAML and JSON unmarshal/marshal logic that must be maintained and kept in sync with the schema definition. +- Integer range validation (0–100) is baked into the type's unmarshal methods, coupling the generic type to a specific field's constraints; reusing the type for a field with a different integer range would require a new type or a parameterised constructor. + +#### Neutral +- All existing callsites that previously used `typeutil.ParseIntValue` to read `DeduplicateByTitle` have been updated to access the `*TemplatableBoolOrInt` pointer directly. +- Four new unit tests cover the boolean, integer, expression, and nil cases end-to-end through the config-generation pipeline. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index bde72c0b599..197d2997a29 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5146,17 +5146,12 @@ ] }, "deduplicate-by-title": { - "description": "Title-based deduplication for create-issue. Set to true for exact title matching, or provide a non-negative integer to deduplicate by Levenshtein edit distance (e.g., 1 allows one-character differences). Applies within-run and against open/recently-closed repository issues.", - "oneOf": [ - { - "type": "boolean" - }, + "allOf": [ { - "type": "integer", - "minimum": 0, - "maximum": 100 + "$ref": "#/$defs/templatable_bool_or_int" } - ] + ], + "description": "Title-based deduplication for create-issue. Set to true for exact title matching, or provide a non-negative integer (0–100) to deduplicate by Levenshtein edit distance (e.g., 1 allows one-character differences). Accepts a GitHub Actions expression that resolves to a boolean or integer at runtime. Applies within-run and against open/recently-closed repository issues." }, "target-repo": { "type": "string", @@ -11801,6 +11796,24 @@ } ] }, + "templatable_bool_or_int": { + "description": "A boolean or non-negative integer (0–100) value that may also be specified as a GitHub Actions expression string that resolves to a boolean or integer at runtime (e.g. '${{ inputs.dedup }}').", + "oneOf": [ + { + "type": "boolean" + }, + { + "type": "integer", + "minimum": 0, + "maximum": 100 + }, + { + "type": "string", + "pattern": "^\\$\\{\\{.*\\}\\}$", + "description": "GitHub Actions expression that resolves to a boolean or integer at runtime" + } + ] + }, "positive_integer_with_km_suffix_or_expression": { "type": "string", "oneOf": [ diff --git a/pkg/workflow/compile_outputs_issue_test.go b/pkg/workflow/compile_outputs_issue_test.go index 4db35a338f5..bcbefcc9700 100644 --- a/pkg/workflow/compile_outputs_issue_test.go +++ b/pkg/workflow/compile_outputs_issue_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/github/gh-aw/pkg/testutil" - "github.com/github/gh-aw/pkg/typeutil" ) // assertTokenInProcessSafeOutputsEnv verifies that a given environment variable name @@ -200,9 +199,9 @@ This workflow tests the output configuration parsing. } } - deduplicateByTitle, ok := typeutil.ParseIntValue(workflowData.SafeOutputs.CreateIssues.DeduplicateByTitle) - if !ok || deduplicateByTitle != 1 { - t.Errorf("Expected deduplicate-by-title to parse as 1, got %#v", workflowData.SafeOutputs.CreateIssues.DeduplicateByTitle) + deduplicateByTitle := workflowData.SafeOutputs.CreateIssues.DeduplicateByTitle + if deduplicateByTitle == nil || string(*deduplicateByTitle) != "1" { + t.Errorf("Expected deduplicate-by-title to parse as '1', got %#v", deduplicateByTitle) } } @@ -495,3 +494,163 @@ This workflow tests that copilot assignment is wired in consolidated safe output t.Error("Did not expect legacy assign_copilot_to_created_issues output wiring in safe_outputs job") } } + +// TestDeduplicateByTitleBoolean verifies that deduplicate-by-title: true is emitted correctly. +func TestDeduplicateByTitleBoolean(t *testing.T) { + tmpDir := testutil.TempDir(t, "dedup-bool-test") + + testContent := `--- +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +safe-outputs: + create-issue: + max: 1 + deduplicate-by-title: true +--- + +Create test issues. +` + testFile := filepath.Join(tmpDir, "test-dedup-bool.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile: %v", err) + } + + lockContent, err := os.ReadFile(filepath.Join(tmpDir, "test-dedup-bool.lock.yml")) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(string(lockContent), `\"deduplicate_by_title\":true`) { + t.Errorf("Expected deduplicate_by_title:true in handler config, got:\n%s", lockContent) + } +} + +// TestDeduplicateByTitleFalse verifies that deduplicate-by-title: false is emitted correctly. +func TestDeduplicateByTitleFalse(t *testing.T) { + tmpDir := testutil.TempDir(t, "dedup-false-test") + + testContent := `--- +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +safe-outputs: + create-issue: + max: 1 + deduplicate-by-title: false +--- + +Create test issues. +` + testFile := filepath.Join(tmpDir, "test-dedup-false.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile: %v", err) + } + + lockContent, err := os.ReadFile(filepath.Join(tmpDir, "test-dedup-false.lock.yml")) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(string(lockContent), `\"deduplicate_by_title\":false`) { + t.Errorf("Expected deduplicate_by_title:false in handler config, got:\n%s", lockContent) + } +} + +// TestDeduplicateByTitleInteger verifies that deduplicate-by-title: is emitted correctly. +func TestDeduplicateByTitleInteger(t *testing.T) { + tmpDir := testutil.TempDir(t, "dedup-int-test") + + testContent := `--- +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +safe-outputs: + create-issue: + max: 1 + deduplicate-by-title: 2 +--- + +Create test issues. +` + testFile := filepath.Join(tmpDir, "test-dedup-int.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile: %v", err) + } + + lockContent, err := os.ReadFile(filepath.Join(tmpDir, "test-dedup-int.lock.yml")) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(string(lockContent), `\"deduplicate_by_title\":2`) { + t.Errorf("Expected deduplicate_by_title:2 in handler config, got:\n%s", lockContent) + } +} + +// TestDeduplicateByTitleExpression verifies that deduplicate-by-title accepts +// GitHub Actions expressions and emits them as a JSON string so the runtime +// can evaluate them. +func TestDeduplicateByTitleExpression(t *testing.T) { + tmpDir := testutil.TempDir(t, "dedup-expr-test") + + testContent := `--- +on: + workflow_dispatch: + inputs: + dedup: + type: boolean + default: true +permissions: + contents: read +engine: copilot +strict: false +safe-outputs: + create-issue: + max: 1 + deduplicate-by-title: ${{ inputs.dedup }} +--- + +Create test issues. +` + testFile := filepath.Join(tmpDir, "test-dedup-expr.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile: %v", err) + } + + lockContent, err := os.ReadFile(filepath.Join(tmpDir, "test-dedup-expr.lock.yml")) + if err != nil { + t.Fatal(err) + } + + // GH Actions expressions are emitted as JSON strings so they can be evaluated at runtime. + if !strings.Contains(string(lockContent), `\"deduplicate_by_title\":\"${{ inputs.dedup }}\"`) { + t.Errorf("Expected deduplicate_by_title expression in handler config, got:\n%s", lockContent) + } +} diff --git a/pkg/workflow/compiler_safe_outputs_builder.go b/pkg/workflow/compiler_safe_outputs_builder.go index 7c3dcb5b451..04eb5d03ed0 100644 --- a/pkg/workflow/compiler_safe_outputs_builder.go +++ b/pkg/workflow/compiler_safe_outputs_builder.go @@ -85,6 +85,22 @@ func (b *handlerConfigBuilder) AddBoolPtr(key string, value *bool) *handlerConfi return b } +// AddTemplatableBoolOrInt adds a TemplatableBoolOrInt field to the handler config. +// +// The stored JSON value depends on the content of *value: +// - "true" → JSON boolean true +// - "false" → JSON boolean false +// - a numeric string (e.g. "1") → JSON number +// - any other string (GitHub Actions expression) → JSON string evaluated at runtime +// - nil → field is omitted +func (b *handlerConfigBuilder) AddTemplatableBoolOrInt(key string, value *TemplatableBoolOrInt) *handlerConfigBuilder { + if value == nil { + return b + } + b.config[key] = value.ToValue() + return b +} + // AddBoolOrInt adds a boolean-or-integer field when the value is set. // This preserves explicit false/0 values, which differ from an omitted field. func (b *handlerConfigBuilder) AddBoolOrInt(key string, value any) *handlerConfigBuilder { diff --git a/pkg/workflow/create_issue.go b/pkg/workflow/create_issue.go index f3a90188d3b..658668bbae1 100644 --- a/pkg/workflow/create_issue.go +++ b/pkg/workflow/create_issue.go @@ -11,21 +11,21 @@ var createIssueLog = logger.New("workflow:create_issue") // CreateIssuesConfig holds configuration for creating GitHub issues from agent output type CreateIssuesConfig struct { BaseSafeOutputConfig `yaml:",inline"` - TitlePrefix string `yaml:"title-prefix,omitempty"` - RequireTemporaryID bool `yaml:"require-temporary-id,omitempty"` // When true, create_issue tool calls must include temporary_id. - Labels []string `yaml:"labels,omitempty"` - AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). - AllowedFields []string `yaml:"allowed-fields,omitempty"` // Optional list of allowed issue field names. If omitted or empty, any issue fields are allowed. Use ["*"] to explicitly allow all. - Assignees []string `yaml:"assignees,omitempty"` // List of users/bots to assign the issue to - DeduplicateByTitle any `yaml:"deduplicate-by-title,omitempty"` // When true or 0, deduplicate by exact title match. When set to a positive integer N, also allow fuzzy matches up to edit distance N. When false or omitted, disable title-based deduplication. - TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository issues - AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that issues can be created in - CloseOlderIssues *string `yaml:"close-older-issues,omitempty"` // When true, close older issues with same title prefix or labels as "not planned" - CloseOlderKey string `yaml:"close-older-key,omitempty"` // Optional explicit deduplication key for close-older matching. When set, uses gh-aw-close-key marker instead of workflow-id markers. - GroupByDay *string `yaml:"group-by-day,omitempty"` // When true, if an open issue was already created today (UTC), post new content as a comment on it instead of creating a duplicate. Works best with close-older-issues: true. - Expires int `yaml:"expires,omitempty"` // Hours until the issue expires and should be automatically closed - Group *string `yaml:"group,omitempty"` // If true, group issues as sub-issues under a parent issue (workflow ID is used as group identifier) - Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + TitlePrefix string `yaml:"title-prefix,omitempty"` + RequireTemporaryID bool `yaml:"require-temporary-id,omitempty"` // When true, create_issue tool calls must include temporary_id. + Labels []string `yaml:"labels,omitempty"` + AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). + AllowedFields []string `yaml:"allowed-fields,omitempty"` // Optional list of allowed issue field names. If omitted or empty, any issue fields are allowed. Use ["*"] to explicitly allow all. + Assignees []string `yaml:"assignees,omitempty"` // List of users/bots to assign the issue to + DeduplicateByTitle *TemplatableBoolOrInt `yaml:"deduplicate-by-title,omitempty"` // When true or 0, deduplicate by exact title match. When set to a positive integer N, also allow fuzzy matches up to edit distance N. When false or omitted, disable title-based deduplication. Accepts GitHub Actions expressions. + TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository issues + AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that issues can be created in + CloseOlderIssues *string `yaml:"close-older-issues,omitempty"` // When true, close older issues with same title prefix or labels as "not planned" + CloseOlderKey string `yaml:"close-older-key,omitempty"` // Optional explicit deduplication key for close-older matching. When set, uses gh-aw-close-key marker instead of workflow-id markers. + GroupByDay *string `yaml:"group-by-day,omitempty"` // When true, if an open issue was already created today (UTC), post new content as a comment on it instead of creating a duplicate. Works best with close-older-issues: true. + Expires int `yaml:"expires,omitempty"` // Hours until the issue expires and should be automatically closed + Group *string `yaml:"group,omitempty"` // If true, group issues as sub-issues under a parent issue (workflow ID is used as group identifier) + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. } // parseCreateIssuesConfig handles create-issue configuration diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index a02011c33ba..d82c04309b1 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -1048,3 +1048,134 @@ func TestGenerateSafeOutputsConfigClosePullRequestStaged(t *testing.T) { assert.True(t, closePRConfig["staged"].(bool), "staged should be true") assert.Nil(t, closePRConfig["github-token"], "github-token should not be set when empty") } + +// TestGenerateSafeOutputsConfigDeduplicateByTitleBool tests that deduplicate_by_title +// with a boolean value is correctly serialized into config.json for create_issue. +func TestGenerateSafeOutputsConfigDeduplicateByTitleBool(t *testing.T) { + trueVal := TemplatableBoolOrInt("true") + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + DeduplicateByTitle: &trueVal, + }, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") + require.NotEmpty(t, result, "Expected non-empty config") + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON") + + ciConfig, ok := parsed["create_issue"].(map[string]any) + require.True(t, ok, "Expected create_issue key in config") + + assert.Equal(t, true, ciConfig["deduplicate_by_title"], "deduplicate_by_title should be true (JSON boolean)") +} + +// TestGenerateSafeOutputsConfigDeduplicateByTitleFalse tests that deduplicate_by_title +// with an explicit false value is correctly serialized into config.json for create_issue. +func TestGenerateSafeOutputsConfigDeduplicateByTitleFalse(t *testing.T) { + falseVal := TemplatableBoolOrInt("false") + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + DeduplicateByTitle: &falseVal, + }, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") + require.NotEmpty(t, result, "Expected non-empty config") + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON") + + ciConfig, ok := parsed["create_issue"].(map[string]any) + require.True(t, ok, "Expected create_issue key in config") + + assert.Equal(t, false, ciConfig["deduplicate_by_title"], "deduplicate_by_title should be false (JSON boolean)") +} + +// TestGenerateSafeOutputsConfigDeduplicateByTitleInt tests that deduplicate_by_title +// with an integer value is correctly serialized into config.json for create_issue. +func TestGenerateSafeOutputsConfigDeduplicateByTitleInt(t *testing.T) { + intVal := TemplatableBoolOrInt("2") + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + DeduplicateByTitle: &intVal, + }, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") + require.NotEmpty(t, result, "Expected non-empty config") + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON") + + ciConfig, ok := parsed["create_issue"].(map[string]any) + require.True(t, ok, "Expected create_issue key in config") + + // JSON numbers unmarshal as float64 in map[string]any + assert.InDelta(t, float64(2), ciConfig["deduplicate_by_title"], 0.0001, "deduplicate_by_title should be 2 (JSON number)") +} + +// TestGenerateSafeOutputsConfigDeduplicateByTitleExpression tests that deduplicate_by_title +// with a GitHub Actions expression is correctly serialized as a JSON string into config.json. +func TestGenerateSafeOutputsConfigDeduplicateByTitleExpression(t *testing.T) { + expr := TemplatableBoolOrInt("${{ inputs.dedup }}") + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + DeduplicateByTitle: &expr, + }, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") + require.NotEmpty(t, result, "Expected non-empty config") + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON") + + ciConfig, ok := parsed["create_issue"].(map[string]any) + require.True(t, ok, "Expected create_issue key in config") + + assert.Equal(t, "${{ inputs.dedup }}", ciConfig["deduplicate_by_title"], "deduplicate_by_title should be the expression string") +} + +// TestGenerateSafeOutputsConfigDeduplicateByTitleNil tests that deduplicate_by_title +// is omitted from config.json when not set. +func TestGenerateSafeOutputsConfigDeduplicateByTitleNil(t *testing.T) { + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + DeduplicateByTitle: nil, + }, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") + require.NotEmpty(t, result, "Expected non-empty config") + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON") + + ciConfig, ok := parsed["create_issue"].(map[string]any) + require.True(t, ok, "Expected create_issue key in config") + + _, hasDedup := ciConfig["deduplicate_by_title"] + assert.False(t, hasDedup, "deduplicate_by_title should not be present when nil") +} diff --git a/pkg/workflow/safe_outputs_handler_registry.go b/pkg/workflow/safe_outputs_handler_registry.go index 23d639fbaf9..4818397cbb9 100644 --- a/pkg/workflow/safe_outputs_handler_registry.go +++ b/pkg/workflow/safe_outputs_handler_registry.go @@ -28,7 +28,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("github-token", c.GitHubToken). AddBoolPtr("normalize_closing_keywords", c.NormalizeClosingKeywords). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). - AddBoolOrInt("deduplicate_by_title", c.DeduplicateByTitle) + AddTemplatableBoolOrInt("deduplicate_by_title", c.DeduplicateByTitle) return builder.Build() }, "add_comment": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/templatables.go b/pkg/workflow/templatables.go index 566bbfaa4e6..de982ddc419 100644 --- a/pkg/workflow/templatables.go +++ b/pkg/workflow/templatables.go @@ -329,6 +329,121 @@ func defaultIntStr(n int) *string { return &s } +const templatableBoolOrIntErrorExample = "value must be a boolean, a non-negative integer (0–100), or a GitHub Actions expression (e.g. '${{ inputs.dedup }}')" + +// TemplatableBoolOrInt represents a field that accepts a boolean, a non-negative integer +// (0–100), or a GitHub Actions expression string (e.g. "${{ inputs.dedup }}"). +// The underlying value is stored as a string: +// - booleans as "true" or "false" +// - integers as their decimal representation (e.g. "0", "1") +// - expressions verbatim (e.g. "${{ inputs.dedup }}") +// +// Use *TemplatableBoolOrInt in struct fields with yaml:"field,omitempty" so that +// unset fields are omitted during YAML marshaling. +// +// Example struct usage: +// +// DeduplicateByTitle *TemplatableBoolOrInt `yaml:"deduplicate-by-title,omitempty"` +// +// Example frontmatter values all accepted: +// +// deduplicate-by-title: true +// deduplicate-by-title: 1 +// deduplicate-by-title: ${{ inputs.dedup }} +type TemplatableBoolOrInt string + +// UnmarshalYAML allows TemplatableBoolOrInt to accept YAML booleans, YAML integers, +// and GitHub Actions expression strings. +func (t *TemplatableBoolOrInt) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.ScalarNode { + return fmt.Errorf("%s", templatableBoolOrIntErrorExample) + } + switch node.Tag { + case "!!bool": + *t = TemplatableBoolOrInt(node.Value) // "true" or "false" + return nil + case "!!int": + n, err := strconv.Atoi(node.Value) + if err != nil || n < 0 || n > 100 { + return fmt.Errorf("integer must be between 0 and 100, got %q", node.Value) + } + *t = TemplatableBoolOrInt(node.Value) + return nil + case "!!str": + if !isExpression(node.Value) { + return fmt.Errorf("%s, got string %q", templatableBoolOrIntErrorExample, node.Value) + } + *t = TemplatableBoolOrInt(node.Value) + return nil + } + return fmt.Errorf("%s", templatableBoolOrIntErrorExample) +} + +// UnmarshalJSON allows TemplatableBoolOrInt to accept JSON booleans, JSON numbers, +// and JSON strings that are GitHub Actions expressions. +func (t *TemplatableBoolOrInt) UnmarshalJSON(data []byte) error { + var b bool + if err := json.Unmarshal(data, &b); err == nil { + if b { + *t = TemplatableBoolOrInt("true") + } else { + *t = TemplatableBoolOrInt("false") + } + return nil + } + var n int + if err := json.Unmarshal(data, &n); err == nil { + if n < 0 || n > 100 { + return fmt.Errorf("integer must be between 0 and 100, got %d", n) + } + *t = TemplatableBoolOrInt(strconv.Itoa(n)) + return nil + } + var s string + if err := json.Unmarshal(data, &s); err != nil { + return fmt.Errorf("%s, got %s", templatableBoolOrIntErrorExample, data) + } + if !isExpression(s) { + return fmt.Errorf("%s, got string %q", templatableBoolOrIntErrorExample, s) + } + *t = TemplatableBoolOrInt(s) + return nil +} + +// MarshalJSON emits a JSON boolean for "true"/"false", a JSON integer for numeric +// strings, and a JSON string for GitHub Actions expressions. +func (t *TemplatableBoolOrInt) MarshalJSON() ([]byte, error) { + return json.Marshal(t.ToValue()) +} + +// String returns the underlying string representation of the value. +func (t *TemplatableBoolOrInt) String() string { + return string(*t) +} + +// IsExpression returns true if the value is a GitHub Actions expression. +func (t *TemplatableBoolOrInt) IsExpression() bool { + return isExpression(string(*t)) +} + +// ToValue returns the native Go value for use in map literals and JSON output: +// - true/false for boolean literals +// - an int for numeric literals +// - a string for GitHub Actions expressions +func (t *TemplatableBoolOrInt) ToValue() any { + s := string(*t) + switch s { + case "true": + return true + case "false": + return false + } + if n, err := strconv.Atoi(s); err == nil { + return n + } + return s // expression string – evaluated at runtime +} + // templatableIntValue parses a *string templatable integer value to int. // Returns 0 if value is nil or is a GitHub Actions expression (not evaluable at compile time). // Returns the parsed integer for literal numeric strings. diff --git a/pkg/workflow/templatables_bool_or_int_test.go b/pkg/workflow/templatables_bool_or_int_test.go new file mode 100644 index 00000000000..b69c089db45 --- /dev/null +++ b/pkg/workflow/templatables_bool_or_int_test.go @@ -0,0 +1,14 @@ +package workflow + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTemplatableBoolOrIntUnmarshalJSONRejectsFloat(t *testing.T) { + var value TemplatableBoolOrInt + err := json.Unmarshal([]byte("1.5"), &value) + require.Error(t, err, "float input should be rejected") +}