Add structured payload channel to safe-output types#26338
Open
Add structured payload channel to safe-output types#26338
Conversation
- Add `IsMetadata` field to `FieldValidation` struct in Go validation config - Add `metadata` field to all body-having safe output types (add_comment, create_issue, update_issue, close_issue, close_pull_request, close_discussion, create_discussion, create_pull_request, submit_pull_request_review, update_pull_request, noop, create_agent_session, report_incomplete) - Add `validateMetadata()` function in JS validator for flat key-value objects (keys: safe identifiers, values: string/number/boolean/null) with no HTML sanitization - Create `metadata_helpers.cjs` with `renderMetadataBlock()` and `parseMetadataFromBody()` - Update `add_comment.cjs` to append metadata as fenced code block after body sanitization - Add `metadata` property to relevant tool schemas in `safe_outputs_tools.json` - Add tests for metadata validation and rendering Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e8654dac-016d-4c84-b925-4616e01daf43 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add structured metadata channel for safe-output sanitizer
Add structured metadata channel to safe-output types
Apr 15, 2026
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a structured metadata channel to safe-output types so agents can attach machine-readable markers without relying on HTML comments that are stripped during sanitization.
Changes:
- Extend Go validation config to flag
metadatafields viaisMetadataand addmetadatato multiple safe-output types. - Add JS-side
validateMetadata()and export related constants for flat key/value metadata validation (not passed throughsanitizeContent()). - Introduce
metadata_helpers.cjsto render/parse a fencedaw-metadatablock, and append the block inadd_commentoutput.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_validation_config.go | Adds IsMetadata to field validation and wires metadata into type configs. |
| actions/setup/js/safe_output_type_validator.cjs | Implements validateMetadata() and integrates it into field validation flow. |
| actions/setup/js/safe_output_type_validator.test.cjs | Adds unit tests covering metadata validation rules and non-sanitization behavior. |
| actions/setup/js/metadata_helpers.cjs | New helpers to render metadata into a fenced block and parse it back out of bodies. |
| actions/setup/js/metadata_helpers.test.cjs | Tests for rendering/parsing metadata blocks and round-tripping. |
| actions/setup/js/add_comment.cjs | Appends rendered metadata block after body sanitization and before footer/tracker markers. |
| .github/workflows/*.lock.yml (many files) | Regenerates embedded safe-output schema/config snapshots to include metadata fields. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 192/192 changed files
- Comments generated: 2
Comment on lines
+199
to
+215
| // Validate value type (primitives only — no nested objects or arrays) | ||
| if (val !== null && typeof val !== "string" && typeof val !== "number" && typeof val !== "boolean") { | ||
| return { | ||
| isValid: false, | ||
| error: `Line ${lineNum}: '${fieldName}' value for key '${key}' must be a string, number, boolean, or null (got ${Array.isArray(val) ? "array" : typeof val})`, | ||
| }; | ||
| } | ||
|
|
||
| // Validate string value length | ||
| if (typeof val === "string" && val.length > MAX_METADATA_VALUE_LENGTH) { | ||
| return { | ||
| isValid: false, | ||
| error: `Line ${lineNum}: '${fieldName}' string value for key '${key}' must not exceed ${MAX_METADATA_VALUE_LENGTH} characters`, | ||
| }; | ||
| } | ||
|
|
||
| normalized[key] = val; |
There was a problem hiding this comment.
validateMetadata() accepts numbers but does not reject non-finite values (NaN/Infinity). When later rendered via JSON.stringify, these values serialize to null, silently changing the metadata. Consider validating typeof val === "number" with Number.isFinite(val) and returning a validation error otherwise (or normalizing them to strings explicitly).
Comment on lines
+63
to
+65
| // Match the fenced code block: ```aw-metadata\n...\n``` | ||
| const pattern = new RegExp("```" + METADATA_FENCE_LANG + "\\n([\\s\\S]*?)\\n```", "m"); | ||
| const match = body.match(pattern); |
There was a problem hiding this comment.
parseMetadataFromBody() matches fences using \n only. If the body uses CRLF (\r\n) line endings, the metadata block won't be detected/parsed. Consider updating the regex to accept \r?\n (or normalizing newlines before matching) so parsing works regardless of line ending style.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification DetailsView all 23 tests
Flagged Tests — Requires ReviewNo tests flagged. All 23 tests are behavioral contract tests with solid observable-output assertions. HighlightsThe round-trip test ( The Language SupportTests analyzed:
Inflation Check
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 23 new/modified tests verify observable behavioral contracts with direct assertions on function outputs. No mocks, no guideline violations, no test inflation detected.
Collaborator
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f918b90a-dc4c-4568-ad85-4f16ea8e894c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot stopped work on behalf of
pelikhan due to an error
April 15, 2026 01:36
Collaborator
|
@copilot review comments |
…Infinity and CRLF issues Agent-Logs-Url: https://github.com/github/gh-aw/sessions/04e2e0c1-a631-4229-995c-613622062138 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in b1aff6a. Both review comments are addressed:
The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Add structured metadata channel to safe-output types
Add structured payload channel to safe-output types
Apr 15, 2026
Collaborator
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/221a2b82-14e4-4d29-b6ef-75a6ee59d402 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Collaborator
|
@copilot allowed-payload default value is false |
Contributor
Author
Done in f98696d. String values in the payload are now passed through |
…xplicit allowed-payload default false Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2909c083-e4f6-4cb9-ad56-e87f10eb8327 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…tuation Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2909c083-e4f6-4cb9-ad56-e87f10eb8327 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in 1d391e8.
Done in 1d391e8. Removed |
Collaborator
|
@copilot do not emit allowed-payload if false |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bdcac35d-2f86-4862-9491-32cba8229538 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Collaborator
|
@copilot Do not emit allowed-payload: false It is implied |
Contributor
Author
…8-4fca-b75c-c1e306d7a1b3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Collaborator
|
@copilot make sure to recompile |
Contributor
Author
Recompile ran cleanly — all 191 workflows compiled with no changes. The JSON config is already correct: Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot recompile and commit the changes |
Contributor
Author
Recompile ran cleanly — all 191 workflows compiled successfully with no lock file changes, so there's nothing new to commit. The compiled output is already in sync with the current code. The "action_required" CI status is a GitHub workflow approval gate, not a build failure. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
|
Hey The PR looks ready for maintainer review. ✅
|
Copilot AI
added a commit
that referenced
this pull request
Apr 15, 2026
- Add IsPayload bool to FieldValidation and AllowedPayload bool to TypeValidationConfig in safe_outputs_validation_config.go - Wire payload field into 13 body-having types (add_comment, create_issue, update_issue, close_issue, close_pull_request, close_discussion, create_discussion, create_pull_request, submit_pull_request_review, update_pull_request, noop, create_agent_session, report_incomplete) - Add validatePayload() to safe_output_type_validator.cjs with flat key-value validation - Add allowed-payload guard in validateItem() - Create payload_helpers.cjs with renderPayloadBlock() and parsePayloadFromBody() - Create payload_helpers.test.cjs with 20 tests - Add 15 payload validation tests to safe_output_type_validator.test.cjs - Add metadata property to 13 tool schemas in safe_outputs_tools.json (both copies) - Prepend payload block in add_comment, close_entity_helpers, close_pull_request, create_discussion, create_issue, create_pull_request handlers - Run make recompile to update all 191 lock files Agent-Logs-Url: https://github.com/github/gh-aw/sessions/67b8061a-6d6e-4e78-bf0f-3004e24e3326 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
removeXmlComments()silently strips all<!-- ... -->content from safe-output bodies, leaving agents with no sanitization-safe way to embed machine-readable markers. This PR adds apayloadfield as a structured alternative.What changed
Go validation config (
safe_outputs_validation_config.go): AddedIsPayload booltoFieldValidationandAllowedPayload bool(JSON:"allowed-payload") toTypeValidationConfig; wiredpayload: {type: "object", isPayload: true}andAllowedPayload: trueinto 13 body-having types (add_comment,create_issue,update_issue,close_issue,close_pull_request,close_discussion,create_discussion,create_pull_request,submit_pull_request_review,update_pull_request,noop,create_agent_session,report_incomplete). Only types that explicitly opt-in viaallowed-payload: truecan carry payload data. The"allowed-payload"key is omitted entirely from the JSON config for types that do not support payload (i.e. whenfalse).JS validator (
safe_output_type_validator.cjs): AddedvalidatePayload()— validates flat key-value objects (keys:^[a-zA-Z][a-zA-Z0-9_]*$max 64 chars; values: string/finite number/boolean/null, max 1024 chars; max 50 entries). Non-finite numbers (NaN/Infinity) are rejected with an explicit error to prevent silentnullcoercion viaJSON.stringify. String values are passed throughsanitizeContent()to strip HTML, XML comments, and injection patterns. Types without"allowed-payload": truein their config are rejected with a clear error.payload_helpers.cjs(new):renderPayloadBlock(payload)normalizes the object by round-tripping throughJSON.parse(JSON.stringify(...)), then renders it as a collapsible<details><summary>payload</summary>section containing a pretty-printed (2-space indented)```json gh-aw-payloadfenced block, usingjsonfor syntax highlighting andgh-aw-payloadas the machine-readable discriminator.parsePayloadFromBody(body)extracts the payload back for downstream consumers, handling both LF and CRLF line endings and both compact and pretty-printed JSON.add_comment.cjs,create_issue.cjs,create_discussion.cjs,create_pull_request.cjs,close_entity_helpers.cjs,close_pull_request.cjs: Payload block is prepended before the tracker ID and footer in every handler that generates a footer attribution.safe_outputs_tools.json: Addedpayloadproperty to all 13 tool schemas so agents know the field exists.Usage
{ "type": "add_comment", "body": "Review complete. All criteria pass.", "payload": { "verdict": "APPROVE", "criteria_passed": 5 } }Produces in the GitHub comment:
Downstream workflows can call
parsePayloadFromBody(body)to extract the object without fragile plaintext parsing.