Add missing safe-output configuration schema coverage

docs/adr/53659-add-safe-output-config-fields-to-json-schema.md

@@ -0,0 +1,43 @@
+# ADR-53659: Add Missing Safe-Output Config Fields to JSON Schema
+
+**Date**: 2026-08-18
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The workflow JSON schema at `pkg/parser/schemas/main_workflow_schema.json` defines `safe-outputs.*` sub-schemas that control what configuration fields workflow authors may set. The Go implementation in `pkg/workflow/` had already grown 13 YAML-tagged config fields across six safe-output types (`add-comment`, `assign-milestone`, `comment-memory`, `create-issue`, `create-pull-request`, `push-to-pull-request-branch`, `threat-detection`) that were absent from the schema. The daily conformance checker (IMP-004 / `check_safe_output_config_schema_coverage`) surfaces this as a spec violation: because the schema does not set `additionalProperties: false` for these sections, there is no hard validation failure at parse time, but editor autocomplete, type checking, and docs-generation tooling are blind to the missing fields. Three of the 13 flagged fields (`call-workflow.workflow_files`, `dispatch-workflow.workflow_files`, `dispatch-workflow.aw_context_workflows`) are compiler-populated internals and must be excluded from the user-facing schema rather than documented in it.
+
+### Decision
+
+We will add all 10 user-facing safe-output configuration fields to `main_workflow_schema.json` under their respective `safe-outputs.*` sub-schemas, matching the Go types and semantics. We will also add the `comment-memory` top-level key (previously absent entirely from `safe-outputs.properties`). For the 3 compiler-populated fields, we will add an explicit `compiler_populated_fields` allowlist to `scripts/check-safe-outputs-conformance.sh` so IMP-004 skips them rather than reporting them as missing. The primary driver is IMP-004 conformance and closing the DX gap for workflow authors.
+
+### Alternatives Considered
+
+#### Alternative 1: Auto-generate the JSON schema from Go struct `yaml` tags
+
+Generate `main_workflow_schema.json` entries automatically from Go struct reflection or `go generate` tooling, eliminating manual drift. This would make schema and Go implementation structurally impossible to diverge. It was not chosen because it requires building a code-generation pipeline and the schema also contains human-authored descriptions, constraints, and `anyOf`/`oneOf` wrappers that go beyond what struct tags can express automatically; the investment was not justified for this incremental fix.
+
+#### Alternative 2: Annotate compiler-populated fields with a skip marker on Go structs
+
+Add a `schema:"-"` tag (or equivalent) to the three compiler-populated Go struct fields and update the conformance checker to honour the annotation, rather than maintaining a hardcoded allowlist in the shell script. This would be more self-documenting at the field level. It was not chosen because it requires changing the Go struct definitions and establishing a new annotation convention, while the hardcoded allowlist in the script is simpler for the immediate fix; the three fields are stable and unlikely to change.
+
+### Consequences
+
+#### Positive
+- Workflow authors gain full editor autocomplete, type checking, and schema-based documentation for all 10 previously undocumented user-facing safe-output config fields.
+- IMP-004 conformance check now passes cleanly; the explicit `compiler_populated_fields` allowlist distinguishes internal-only fields from user-authored ones, preventing false positives on future runs.
+
+#### Negative
+- The JSON schema must continue to be manually maintained in sync with Go struct changes; any future new YAML-tagged field in a Go safe-output config struct requires a coordinated schema update to avoid regressing IMP-004.
+- The `compiler_populated_fields` set in `check-safe-outputs-conformance.sh` is a separate maintenance artifact: if compiler-populated fields are renamed or added in Go, the allowlist must be updated in lockstep.
+
+#### Neutral
+- No change to the Go parsing or validation logic; existing workflow files that already use these fields continue to compile and run identically.
+- The conformance checker script is now slightly more complex (a set lookup before the `missing` append), but the logic remains easy to follow.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/parser/schema_safe_outputs_target_test.go

@@ -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)
+	}
+}
+
 // 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

pkg/parser/schemas/main_workflow_schema.json

@@ -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'])",
@@ -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"
+                  }
+                },
+                "hide-older-comments-match": {
+                  "type": "array",
+                  "description": "Exact workflow IDs whose older comments are eligible to be hidden.",
+                  "items": {
+                    "type": "string"
+                  }
+                },
                 "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).",
@@ -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": {
+          "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."
+                },
+                "footer": {
+                  "type": "boolean",
+                  "description": "Controls whether the AI-generated footer is added to the managed comment."
+                }
+              },
+              "additionalProperties": false
+            }
+          ]
+        },
         "create-pull-request": {
           "oneOf": [
             {
@@ -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"
@@ -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": [
@@ -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"
@@ -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."
+                },
                 "steps": {
                   "type": "array",
                   "description": "Array of extra job steps to run before engine execution",
@@ -10967,6 +11042,10 @@
                     }
                   ]
                 },
+                "environment": {
+                  "type": "string",
+                  "description": "GitHub Actions environment override for the detection job."
+                },
                 "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."

scripts/check-safe-outputs-conformance.sh

@@ -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():
@@ -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