Skip to content

Enforce minimum create_issue body length in safe outputs schema and validator #38114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 6 commits into from
Jun 9, 2026
Merged

Enforce minimum create_issue body length in safe outputs schema and validator #38114

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0776993
Initial plan
Copilot Jun 9, 2026
c9274ae
Plan: enforce create_issue body minimum length
Copilot Jun 9, 2026
786a7e7
Require minimum 20-char body for create_issue safe output
Copilot Jun 9, 2026
9bc3e93
Add create_issue body minLength and boundary tests
Copilot Jun 9, 2026
b8637db
chore: start PR finisher pass
Copilot Jun 9, 2026
7a7a915
fix safe-output sample placeholders and add missing minLength tests
Copilot Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
325 changes: 238 additions & 87 deletions .github/workflows/daily-safeoutputs-git-simulator.lock.yml
View file
Open in desktop

Large diffs are not rendered by default.

45 changes: 37 additions & 8 deletions actions/setup/js/safe_output_type_validator.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const SAMPLE_VALIDATION_CONFIG = {
defaultMax: 1,
fields: {
title: { required: true, type: "string", sanitize: true, maxLength: 128 },
body: { required: true, type: "string", sanitize: true, maxLength: 65000 },
body: { required: true, type: "string", sanitize: true, maxLength: 65000, minLength: 20 },
labels: { type: "array", itemType: "string", itemSanitize: true, itemMaxLength: 128 },
parent: { issueOrPRNumber: true },
temporary_id: { type: "string" },
Expand Down Expand Up @@ -196,7 +196,7 @@ describe("safe_output_type_validator", () => {
it("should validate create_issue with all required fields", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test Issue", body: "Test body" }, "create_issue", 1);
const result = validateItem({ type: "create_issue", title: "Test Issue", body: "Detailed issue body text." }, "create_issue", 1);

expect(result.isValid).toBe(true);
expect(result.normalizedItem).toBeDefined();
Expand All @@ -205,7 +205,7 @@ describe("safe_output_type_validator", () => {
it("should fail validation when required title is missing", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", body: "Test body" }, "create_issue", 1);
const result = validateItem({ type: "create_issue", body: "Detailed issue body text." }, "create_issue", 1);

expect(result.isValid).toBe(false);
expect(result.error).toContain("title");
Expand All @@ -223,7 +223,7 @@ describe("safe_output_type_validator", () => {
it("should sanitize string fields", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test @mention Issue", body: "Test body" }, "create_issue", 1);
const result = validateItem({ type: "create_issue", title: "Test @mention Issue", body: "Detailed issue body text." }, "create_issue", 1);

expect(result.isValid).toBe(true);
// The sanitizeContent function converts @mentions to backticked format
Expand Down Expand Up @@ -263,7 +263,7 @@ describe("safe_output_type_validator", () => {
it("should not normalize backticked closing references when disabled", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test", body: "Closes `#123`" }, "create_issue", 1, { normalizeIssueClosingKeywords: false });
const result = validateItem({ type: "create_issue", title: "Test", body: "Detailed context. Closes `#123`" }, "create_issue", 1, { normalizeIssueClosingKeywords: false });

expect(result.isValid).toBe(true);
expect(result.normalizedItem.body).toContain("`#123`");
Expand Down Expand Up @@ -776,6 +776,35 @@ describe("safe_output_type_validator", () => {
});

describe("minLength validation", () => {
it("should reject create_issue body shorter than minLength", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test Issue", body: "Short" }, "create_issue", 1);

expect(result.isValid).toBe(false);
expect(result.error).toContain("too short");
expect(result.error).toContain("20");
});

it("should accept create_issue body that meets minLength", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const body = "Detailed issue body with clear context.";
const result = validateItem({ type: "create_issue", title: "Test Issue", body }, "create_issue", 1);

expect(result.isValid).toBe(true);
});

it("should accept create_issue body at exact minLength", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const body = "Exactly twenty chars";
expect(body.length).toBe(20);
const result = validateItem({ type: "create_issue", title: "Test Issue", body }, "create_issue", 1);

expect(result.isValid).toBe(true);
});

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The boundary test confirms == 20 is accepted, but has no companion test for == 19 (minLength − 1). Without it we cannot tell whether the implementation uses < 20 or <= 20 — a classic off-by-one gap.

💡 Suggested test at minLength − 1 
it("should reject create_issue body at exactly minLength - 1", async () => {
  const { validateItem } = await import("./safe_output_type_validator.cjs");

  const body = "Exactly nineteen ch"; // 19 chars
  expect(body.length).toBe(19);
  const result = validateItem(
    { type: "create_issue", title: "Test Issue", body },
    "create_issue",
    1,
  );

  expect(result.isValid).toBe(false);
  expect(result.error).toContain("too short");
});

Pairing == 19 (reject) with == 20 (accept) fully pins the boundary semantics.

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The create_discussion test suite covers the whitespace-trim edge case. The new create_issue block skips it. The validator trims before comparing (finalValue.trim().length < validation.minLength), so this behaviour is worth pinning explicitly for parity.

💡 Suggested whitespace test 
it('should reject create_issue body that is only whitespace', async () => {
  const { validateItem } = await import('./safe_output_type_validator.cjs');

  // 25 spaces: raw length passes 20 but trimmed length is 0
  const result = validateItem(
    { type: 'create_issue', title: 'Test Issue', body: '                         ' },
    'create_issue',
    1,
  );

  expect(result.isValid).toBe(false);
  expect(result.error).toContain('too short');
});

Mirrors the existing create_discussion whitespace test and guards the trim-before-check contract.


it("should reject body shorter than minLength", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

Expand Down Expand Up @@ -818,7 +847,7 @@ describe("safe_output_type_validator", () => {
it("should validate array of strings", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test", body: "Body", labels: ["bug", "enhancement"] }, "create_issue", 1);
const result = validateItem({ type: "create_issue", title: "Test", body: "Detailed issue body text.", labels: ["bug", "enhancement"] }, "create_issue", 1);

expect(result.isValid).toBe(true);
expect(Array.isArray(result.normalizedItem.labels)).toBe(true);
Expand All @@ -827,7 +856,7 @@ describe("safe_output_type_validator", () => {
it("should reject array with non-string items", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "create_issue", title: "Test", body: "Body", labels: ["bug", 123] }, "create_issue", 1);
const result = validateItem({ type: "create_issue", title: "Test", body: "Detailed issue body text.", labels: ["bug", 123] }, "create_issue", 1);

expect(result.isValid).toBe(false);
expect(result.error).toContain("must contain only strings");
Expand Down Expand Up @@ -873,7 +902,7 @@ describe("safe_output_type_validator", () => {
const item = {
type: "create_issue",
title: "Test",
body: "Body text",
body: "Detailed issue body text.",
metadata: { project: "test" },
};

Expand Down
1 change: 1 addition & 0 deletions actions/setup/js/safe_outputs_tools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"body": {
"type": "string",
"minLength": 20,

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] This PR correctly aligns the create_issue JSON schema with its Go validation config. However, create_discussion has MinDiscussionBodyLength = 64 in safe_outputs_validation_config.go (line 48) but no corresponding "minLength": 64 in either safe_outputs_tools.json. The gap means:

  • Go sample validation (which compiles the JSON schema) will not catch short discussion bodies at compile time
  • The LLM tool description gives no hint about the length floor

A follow-up to add "minLength": 64 on create_discussion.body would close the same class of inconsistency this PR is fixing for create_issue.

"description": "Detailed issue description in Markdown. Must be the final intended body \u2014 not a placeholder or test value. Do NOT repeat the title as a heading since it already appears as the issue's h1. Include context, reproduction steps, or acceptance criteria as appropriate."
},
"labels": {
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/js/safe_outputs_tools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"body": {
"type": "string",
"minLength": 20,

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regression: sampleRuntimeExpressionPlaceholder ("aw_sample", 9 chars) is shorter than this new minLength: 20, breaking compile-time validation for create_issue samples whose body is a ${{ ... }} runtime expression.

💡 Details and fix options

validateSafeOutputsSamples in samples_validation.go substitutes runtime expressions with a placeholder before schema validation. For plain string fields with no pattern/format, placeholderForType unconditionally returns "aw_sample" (9 chars). Now that the JSON Schema requires body.length >= 20, "aw_sample" fails that check, and any workflow like:

samples:
  - title: "Bug from ${{ github.event.inputs.component }}"
    body:  "${{ github.event.inputs.description }}"

will be rejected at gh aw compile time with a spurious too

"description": "Detailed issue description in Markdown. Must be the final intended body \u2014 not a placeholder or test value. Do NOT repeat the title as a heading since it already appears as the issue's h1. Include context, reproduction steps, or acceptance criteria as appropriate."
},
Comment on lines 12 to 17
"labels": {
Expand Down
3 changes: 2 additions & 1 deletion pkg/workflow/safe_outputs_validation_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (
MaxBodyLength = 65000
MaxGitHubUsernameLength = 39
MaxGitHubTeamSlugLength = 100
MinIssueBodyLength = 20 // Minimum body length for create_issue to prevent placeholder-only submissions

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing parity test: no TestCreateIssueBodyMinLength analogous to the existing TestCreateDiscussionBodyMinLength.

💡 Details

safe_output_validation_config_test.go has TestCreateDiscussionBodyMinLength which asserts:

if bodyField.MinLength != MinDiscussionBodyLength { ... }

No equivalent test was added for create_issue. A future refactor that accidentally removes MinLength from the create_issue body field would be silently undetected. Add:

func TestCreateIssueBodyMinLength(t *testing.T) {
    config, ok := ValidationConfig["create_issue"]
    require.True(t, ok)
    body, ok := config.Fields["body"]
    require.True(t, ok)
    if body.MinLength != MinIssueBodyLength {
        t.Errorf("create_issue body MinLength = %d, want %d", body.MinLength, MinIssueBodyLength)
    }
}

MinDiscussionBodyLength = 64 // Minimum body length for create_discussion to prevent placeholder-only submissions
)

Expand All @@ -54,7 +55,7 @@ var ValidationConfig = map[string]TypeValidationConfig{
DefaultMax: 1,
Fields: map[string]FieldValidation{
"title": {Required: true, Type: "string", Sanitize: true, MaxLength: 128},
"body": {Required: true, Type: "string", Sanitize: true, MaxLength: MaxBodyLength},
"body": {Required: true, Type: "string", Sanitize: true, MaxLength: MaxBodyLength, MinLength: MinIssueBodyLength},
Comment on lines 47 to +58
"labels": {Type: "array", ItemType: "string", ItemSanitize: true, ItemMaxLength: 128},
"fields": {Type: "array"},
"parent": {IssueOrPRNumber: true},
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/samples_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestValidateSafeOutputsSamples_Valid(t *testing.T) {
Samples: []map[string]any{
{
"title": "Sample issue",
"body": "Sample body",
"body": "Sample issue body with enough detail.",

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two other tests in this file were not updated and now silently test the wrong failure path.

💡 Details

1. TestValidateSafeOutputsSamples_MissingRequired (line 56)

"body": "Body without title",  // 18 chars — below the new minLength: 20

The test's intent is to verify that a missing title surfaces an error. After this PR, the JSON Schema also requires body.length >= 20, so the 18-char body now triggers a minLength failure before the missing-title check. The test still passes (there is still an error), but it no longer verifies what its comment claims. Fix: update the body to ≥ 20 chars, e.g., "Body text without title.".

2. TestValidateSafeOutputsSamples_NonExpressionErrorsStillReported (line 253)

"body": "${{ github.event.inputs.body }}",  // substituted → "aw_sample" (9 chars < 20)

This test verifies that a runtime expression on body does not mask a missing-title error. After this PR, the substituted placeholder "aw_sample" (9 chars) fails the new minLength: 20 JSON Schema constraint, so the error that surfaces is now body-too-short, not missing-title. The test still passes (error is still non-nil with the right key/index prefix), but the stated invariant is no longer being tested. This is also a symptom of the regression flagged on safe_outputs_tools.json line 15.

},
},
},
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestValidateSafeOutputsSamples_RuntimeExpressionsInNestedValues(t *testing.
Samples: []map[string]any{
{
"title": "Issue ${{ github.event.inputs.title_suffix }}",
"body": "Body",
"body": "Body with enough detail.",
"labels": []any{
"static-label",
"${{ github.event.inputs.dynamic_label }}",
Expand Down
Loading