fix: add maxLength: 65536 to create_issue and create_discussion body fields#43911
Conversation
…fields The SM-IS-01 global string-length cap (10 KB) is bypassed for any field with an explicit maxLength in the schema. add_comment.body correctly has maxLength: 65536 and is exempt. This commit adds the same maxLength to create_discussion.body and create_issue.body so they are also exempt from the 10 KB cap and deferred to handler-level (GitHub API) enforcement. Also removes the "Maximum 10 KB" guidance from create_discussion.body description, which described the accidental cap, not an intended limit. Fixes #43867 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the safe-outputs MCP tool schemas so create_issue.body and create_discussion.body declare an explicit maxLength: 65536, aligning them with add_comment.body and preventing the SM-IS-01 “10KB per string parameter” guardrail from applying to these body fields.
Changes:
- Add
maxLength: 65536tocreate_issue.body. - Add
maxLength: 65536tocreate_discussion.body. - Remove the “Maximum 10 KB” guidance text from the
create_discussion.bodydescription (since it reflected an accidental cap, not an intended constraint).
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/js/safe_outputs_tools.json | Updates the source tool schema definitions for issue/discussion body fields to declare a 65KB max length and adjusts discussion guidance text accordingly. |
| actions/setup/js/safe_outputs_tools.json | Mirrors the same schema updates in the runtime copy used by the safe-outputs MCP server. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Low
| "body": { | ||
| "type": "string", | ||
| "minLength": 20, | ||
| "maxLength": 65536, | ||
| "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." |
| "body": { | ||
| "type": "string", | ||
| "minLength": 20, | ||
| "maxLength": 65536, | ||
| "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." |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #43911 does not have the 'implementation' label and has only 3 new lines of code in business logic directories (threshold: 100). |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. This change adds maxLength validation to safe_outputs_tools.json configuration files only. |
There was a problem hiding this comment.
The fix is clean and correct. Both create_issue.body and create_discussion.body now carry maxLength: 65536, consistent with add_comment.body. The two JSON files are kept in sync. The removal of the stale 10 KB truncation guidance from create_discussion.body is appropriate since that limit is no longer in effect. No issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 10.5 AIC · ⌖ 5.81 AIC · ⊞ 4.9K
There was a problem hiding this comment.
Non-blocking observation on schema consistency
The core change is correct: create_issue.body and create_discussion.body now carry maxLength: 65536, matching add_comment.body and aligning all three content-body fields with the same effective limit.
One item to consider
The create_discussion.body description had actionable overflow guidance (trim / link to gist / use upload_asset) that was removed as part of cleaning up the stale 10 KB reference. That guidance is still needed — just updated to reflect the new ceiling. See inline comment.
All other aspects of the change are correct and the synchronisation of both JSON files is complete.
🔎 Code quality review by PR Code Quality Reviewer · 58.1 AIC · ⌖ 6.4 AIC · ⊞ 5.5K
Comment /review to run again
| "type": "string", | ||
| "description": "Discussion content in Markdown. Do NOT repeat the title as a heading since it already appears as the discussion's h1. Include all relevant context, findings, or questions. Maximum 10 KB (10240 bytes). If your content is larger, trim it, link to an external artifact (for example a gist), or upload content via upload_asset instead of inlining." | ||
| "maxLength": 65536, | ||
| "description": "Discussion content in Markdown. Do NOT repeat the title as a heading since it already appears as the discussion's h1. Include all relevant context, findings, or questions." |
There was a problem hiding this comment.
Lost overflow guidance may degrade agent behaviour: the removed description told agents what to do when content exceeds the limit; no equivalent guidance has been added for the new 65536-byte ceiling.
💡 Explanation
The deleted text said:
If your content is larger, trim it, link to an external artifact (for example a gist), or upload content via upload_asset instead of inlining.
That guidance was tied to the old accidental 10 KB cap, but the need for fallback advice doesn't go away — it just applies to a different threshold now. Compare add_comment.body, which still contains:
If these limits are exceeded, the tool call will fail with a detailed error message indicating which constraint was violated.
Without equivalent wording in create_discussion.body, an AI agent that generates a 70 KB discussion body has no cue that it should trim or link out before calling the tool. Consider adding at minimum:
"description": "Discussion content in Markdown ... If your content exceeds 65536 characters, trim it or link to an external artifact instead of inlining."Non-blocking — the schema constraint itself is correct — but this omission will silently degrade agent behaviour on large payloads.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — the fix is correct; leaving a comment on test coverage.
📋 Key Themes & Highlights
What's fixed
The root cause (missing maxLength in the JSON schema for create_issue.body and create_discussion.body) is correctly identified and addressed. Both files are updated in sync, which TestSafeOutputsToolsJSONInSync enforces. Removing the stale "Maximum 10 KB" guidance text from create_discussion.body is the right call.
One observation
- No regression test — the bug was silent truncation caused by the missing
maxLength. There is currently nothing in the test suite that would catch the 10 KB cap re-appearing. A test that validates a >10 KB body against the embedded schema would close this gap. (See inline comment.)
Positive highlights
- ✅ Minimal, focused diff — only the two required changes per file
- ✅ Both copies updated together; sync test already enforces this invariant
- ✅ PR description is clear and includes a before/after table
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 85.9 AIC · ⌖ 6.55 AIC · ⊞ 6.7K
Comment /matt to run again
| "body": { | ||
| "type": "string", | ||
| "minLength": 20, | ||
| "maxLength": 65536, |
There was a problem hiding this comment.
[/tdd] No regression test for the fixed bug — a body > 10 KB should now be accepted for create_issue, but there is no test guarding against this regression.
💡 Suggested test shape
In safe_outputs_tools_test.go or samples_validation_test.go, add a case that validates a create_issue.body value larger than 10 KB against the embedded JSON schema and asserts it passes. Without this, the 10 KB cap could silently creep back in future schema edits without any failing test.
func TestSafeOutputsToolsBodyAcceptsLargeContent(t *testing.T) {
bigBody := strings.Repeat("x", 11000) // > 10 KB, within new 65536 limit
// validate bigBody against the embedded safe_outputs_tools.json schema for create_issue
// expect: no maxLength violation
}@copilot please address this.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey One thing worth addressing before merge:
If you'd like a hand adding coverage, here's a ready-to-use prompt: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "patchdiff.githubusercontent.com"See Network Configuration for more information.
|
|
🎉 This pull request is included in a new release. Release: |
create_discussion.bodyandcreate_issue.bodylacked an explicitmaxLength, so the SM-IS-01 validator applied its 10 KB global cap to both — silently truncating large report bodies.add_comment.bodyalready carriesmaxLength: 65536and is correctly exempt from this cap.Changes
pkg/workflow/js/safe_outputs_tools.jsonandactions/setup/js/safe_outputs_tools.json(runtime copy — kept in sync):create_issue.body: add"maxLength": 65536create_discussion.body: add"maxLength": 65536; remove the "Maximum 10 KB" guidance text, which described the accidental cap rather than an intended constraintAfter this change, all three content-body fields are treated identically by the SM-IS-01 validator:
body.maxLengthadd_comment65536create_issue65536create_discussion65536