Skip to content

Add missing safe-output configuration schema coverage #53659

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 8 commits into from
Aug 18, 2026
Merged

Add missing safe-output configuration schema coverage #53659

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6ad002f
Initial plan
Copilot Aug 18, 2026
5249f53
Add safe output schema coverage
Copilot Aug 18, 2026
b8e8be5
docs(adr): add draft ADR-53659 for safe-output config schema coverage
github-actions[bot] Aug 18, 2026
6bf802b
Merge branch 'main' into copilot/imp-004-add-missing-json-schema-prop…
github-actions[bot] Aug 18, 2026
3a9fe61
Merge remote-tracking branch 'origin/main' into copilot/imp-004-add-m…
Copilot Aug 18, 2026
0fc0f52
Correct comment-memory schema placement
Copilot Aug 18, 2026
1d171e8
Restrict comment-memory to tools configuration
Copilot Aug 18, 2026
f87dc22
Merge main into copilot/imp-004-add-missing-json-schema-properties
Copilot Aug 18, 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
43 changes: 43 additions & 0 deletions pkg/parser/schema_safe_outputs_target_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,49 @@ import (
"testing"
)

func TestMainWorkflowSchema_SafeOutputConfigCoverage(t *testing.T) {
t.Parallel()

frontmatter := map[string]any{
"on": "push",
"engine": "copilot",
"safe-outputs": map[string]any{
"add-comment": map[string]any{
"allows-comment-ids": []any{"IC_kwDOABCD123456"},
"hide-older-comments-match": []any{"workflow-id"},
},
"assign-milestone": map[string]any{
"auto_create": true,
},
"comment-memory": map[string]any{
"target": "triggering",
"target-repo": "github/gh-aw",
"allowed-repos": []any{"github/docs"},
"memory-id": "default",
"footer": true,
},
"create-issue": map[string]any{
"require-temporary-id": true,
},
"create-pull-request": map[string]any{
"require-temporary-id": true,
},
"push-to-pull-request-branch": map[string]any{
"base-branch": "main",
},
"threat-detection": map[string]any{
"engine-config": "copilot",
"environment": "production",
"model": "gpt-5",
},
},
}

if err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/safe-output-config-coverage-test.md"); err != nil {
t.Fatalf("expected safe-output configuration fields to pass schema validation, got: %v", err)

@github-actions github-actions Bot Aug 18, 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 new test only validates the happy path. Per the pattern in this file, adding at least one negative case would give this schema regression real value — e.g. an unknown field in assign-milestone or a wrong type for auto_create.

💡 Suggested addition 
func TestMainWorkflowSchema_SafeOutputConfigCoverage_InvalidField(t *testing.T) {
	t.Parallel()

	frontmatter := map[string]any{
		"on":     "push",
		"engine": "copilot",
		"safe-outputs": map[string]any{
			"assign-milestone": map[string]any{
				"not_a_real_field": true,
			},
		},
	}

	if err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/safe-output-config-negative-test.md"); err == nil {
		t.Fatal("expected schema validation to reject unknown field, but got no error")
	}
}

@copilot please address this.

}
}

// TestMainWorkflowSchema_SafeOutputsTargetProperties validates that safe output
// types which support target/target-repo/allowed-repos in the Go code also accept
// those properties in the JSON schema. This is a regression test for cases where
Expand Down
79 changes: 79 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5474,6 +5474,10 @@
"type": "string",
"description": "Optional prefix to add to the beginning of the issue title (e.g., '[ai] ' or '[analysis] ')"
},
"require-temporary-id": {
"type": "boolean",
"description": "Require create_issue tool calls to include a temporary_id."
},
"labels": {
"type": "array",
"description": "Optional list of labels to automatically attach to created issues (e.g., ['automation', 'ai-generated'])",
Expand Down Expand Up @@ -7074,6 +7078,20 @@
}
]
},
"allows-comment-ids": {
"type": "array",
"description": "Trusted allowlist of issue or pull request comment IDs the agent may update when target is '*'.",
"items": {
"type": "string"
}
Comment on lines +7081 to +7086
},
"hide-older-comments-match": {
"type": "array",
"description": "Exact workflow IDs whose older comments are eligible to be hidden.",
"items": {
"type": "string"
}
},
Comment on lines +7088 to +7094
"allowed-reasons": {
"type": "array",
"description": "List of allowed reasons for hiding older comments when hide-older-comments is enabled. Default: all reasons allowed (spam, abuse, off_topic, outdated, resolved, low_quality).",
Expand Down Expand Up @@ -7165,6 +7183,43 @@
],
"description": "Enable AI agents to add comments to GitHub issues, pull requests, or discussions. Supports templating, cross-repository commenting, and automatic mentions."
},
"comment-memory": {

@github-actions github-actions Bot Aug 18, 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.

[/codebase-design] The comment-memory safe-outputs entry's first anyOf arm is $ref: #/properties/tools/properties/comment-memory. The referenced tools definition is itself a oneOf that includes true and null shorthand variants, so those are silently allowed here too. Unless true/null are valid in the safe-outputs context, consider using a plain inline object with additionalProperties: false (matching the create-pull-request and create-issue patterns) rather than a $ref to the tools definition.

@copilot please address this.

"description": "Comment memory configuration for managed comment persistence.",
"anyOf": [
{
"$ref": "#/properties/tools/properties/comment-memory"
},
{
"type": "object",
"properties": {
"target": {
"type": "string",
"description": "Target for comment memory: 'triggering' (default), '*' or an explicit issue or pull request number."
},
"target-repo": {
"type": "string",
"description": "Target repository in owner/repo format."
},
"allowed-repos": {
"type": "array",
"description": "Additional repositories allowed for comment-memory operations.",
"items": {
"type": "string"
}
},
"memory-id": {
"type": "string",
"description": "Default memory identifier when an item does not provide memory_id."

@github-actions github-actions Bot Aug 18, 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.

[/codebase-design] The new comment-memory safe-outputs branch (the second anyOf arm) lacks "additionalProperties": false, so unknown fields won't be caught by schema validation.

💡 Suggested fix

Add "additionalProperties": false to the inline object branch — every other safe-outputs config object in this schema closes with it; this branch is the only exception introduced by this PR.

{
  "type": "object",
  "properties": { ... },
  "additionalProperties": false
}

@copilot please address this.

},
"footer": {
"type": "boolean",
"description": "Controls whether the AI-generated footer is added to the managed comment."
}
},
"additionalProperties": false
}
]
},
"create-pull-request": {
"oneOf": [
{
Expand All @@ -7191,6 +7246,10 @@
"type": "string",
"description": "Optional prefix to prepend to the pull request branch name (e.g. \"signed/\"). Applied before the agent-specified or auto-generated branch name."
},
"require-temporary-id": {
"type": "boolean",
"description": "Require create_pull_request tool calls to include a temporary_id."
},
"title-prefix": {
"type": "string",
"description": "Optional prefix for the pull request title"
Expand Down Expand Up @@ -8679,6 +8738,10 @@
"minItems": 1,
"maxItems": 50
},
"auto_create": {
"type": "boolean",
"description": "Automatically create missing milestones from the allowed list."
},
"max": {
"description": "Optional maximum number of milestone assignments (default: 1) Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').",
"oneOf": [
Expand Down Expand Up @@ -9546,6 +9609,10 @@
"type": "string",
"description": "The branch to push changes to (defaults to 'triggering')"
},
"base-branch": {
"type": "string",
"description": "Base branch of the target repository for incremental patch computation. Defaults to the local checkout branch or the repository default branch."
},
"target": {
"type": "string",
"description": "Target for push operations: 'triggering' (default), '*' (any pull request), or explicit pull request number"
Expand Down Expand Up @@ -10937,6 +11004,14 @@
}
]
},
"engine-config": {
"$ref": "#/$defs/engine_config",
"description": "Extended engine configuration for threat detection."
},
"model": {
"type": "string",
"description": "Model override for threat detection engine execution."
},
Comment on lines +10975 to +10982
"steps": {
"type": "array",
"description": "Array of extra job steps to run before engine execution",
Expand Down Expand Up @@ -10967,6 +11042,10 @@
}
]
},
"environment": {
"type": "string",
"description": "GitHub Actions environment override for the detection job."
},
Comment on lines +11013 to +11016
"continue-on-error": {
"$ref": "#/$defs/templatable_boolean",
"description": "When true (default), detection failures produce warnings and allow safe outputs to proceed with a caution notice and 'needs-review' label. When false, detection failures block safe outputs entirely. Accepts a boolean literal or a GitHub Actions expression."
Expand Down
11 changes: 9 additions & 2 deletions scripts/check-safe-outputs-conformance.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,12 @@ for line in handlers.splitlines():
if field_match and handler_key:
handler_fields[field_match.group(1)] = handler_key

compiler_populated_fields = {
"safe-outputs.call-workflow.workflow_files",
"safe-outputs.dispatch-workflow.workflow_files",
"safe-outputs.dispatch-workflow.aw_context_workflows",
}


def yaml_fields(struct_name):
for line in structs.get(struct_name, "").splitlines():
Expand Down Expand Up @@ -496,8 +502,9 @@ for line in structs["SafeOutputsConfig"].splitlines():

output_properties = properties(output_schema)
for tag, inline in yaml_fields(config_type):
if not inline and tag not in output_properties:
missing.append(f"safe-outputs.{output_name}.{tag}")
property_path = f"safe-outputs.{output_name}.{tag}"
if not inline and property_path not in compiler_populated_fields and tag not in output_properties:
missing.append(property_path)

print("\n".join(sorted(set(missing))))
PY
Expand Down
Loading