Skip to content

Prevent GH_AW_TOOLS_META_JSON expression-size overflows in compiled workflows - #53965

Merged
pelikhan merged 8 commits into
mainfrom
copilot/aw-compat-daily-audit
Aug 20, 2026
Merged

Prevent GH_AW_TOOLS_META_JSON expression-size overflows in compiled workflows#53965
pelikhan merged 8 commits into
mainfrom
copilot/aw-compat-daily-audit

Conversation

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

The daily compatibility audit surfaced a compiler-side regression: large, compiler-generated GH_AW_TOOLS_META_JSON blocks could fail compilation when they contained ${{ }} expressions and crossed GitHub Actions’ 21KB expression-bearing limit. This failure mode affected otherwise valid workflows with large tool metadata.

  • Compiler path: sanitize tools-meta expression payload

    • In pkg/workflow/mcp_setup_safe_outputs.go, added tools-meta runtime data preparation that:
      • detects ${{ ... }} in generated tools meta JSON,
      • rewrites them to ${GH_AW_*} placeholders in the emitted YAML block scalar,
      • emits corresponding step env bindings so values can be restored at runtime.
    • This removes raw template expressions from the large YAML value while preserving semantics.
  • Runtime path: resolve placeholders before materializing tools_meta.json

    • In actions/setup/js/generate_safe_outputs_tools.cjs, added placeholder resolution for GH_AW_TOOLS_META_JSON prior to writing tools_meta.json.
    • Runtime behavior stays consistent; only transport/encoding of expression-bearing content changes.
  • Regression coverage

    • Added targeted tests for:
      • Go-side tools-meta expression extraction/sanitization,
      • JS-side placeholder resolution when GH_AW_TOOLS_META_JSON is provided via env.
sanitized, envKeys, envValues:=buildToolsMetaRuntimeData(toolsMetaJSON)
// sanitized: ... "${GH_AW_*}" ...// envValues: GH_AW_* -> "${{ ... }}"

Generated by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/32249206937> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 23.7 AIC · ⌖ 8.26 AIC · ⊞ 9.3K ·

Comment /souschef to run again

CopilotAIand others added 2 commits August 19, 2026 10:45
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Audit daily compatibility for AW with repositoriesPrevent GH_AW_TOOLS_META_JSON expression-size overflows in compiled workflowsAug 19, 2026
CopilotAI requested a review from pelikhanAugust 19, 2026 10:51
@pelikhan
pelikhan marked this pull request as ready for review August 19, 2026 10:54
CopilotAI balanced review requested due to automatic review settings August 19, 2026 10:54
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot add tests

CopilotAI left a comment

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.

Pull request overview

Prevents oversized expression-bearing tools metadata from exceeding GitHub Actions limits.

Changes:

  • Replaces embedded expressions with runtime environment placeholders.
  • Resolves placeholders before generating tools metadata.
  • Adds Go and JavaScript regression tests.
Show a summary per file
FileDescription
pkg/workflow/mcp_setup_safe_outputs.goSanitizes tools metadata expressions.
pkg/workflow/mcp_setup_safe_outputs_test.goTests expression extraction.
actions/setup/js/generate_safe_outputs_tools.cjsRestores runtime placeholders.
actions/setup/js/generate_safe_outputs_tools.test.cjsTests placeholder resolution.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

* @returns {string}
*/
function resolveEnvStringPlaceholders(value) {
return value.replace(/\$\{([A-Z_][A-Z0-9_]*)\}/g, (match, envName) => process.env[envName] ?? match);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in e403f67: resolveEnvStringPlaceholders now JSON-escapes the resolved env value (JSON.stringify(value).slice(1, -1)) before splicing it into the JSON text, so quotes/backslashes/newlines in the resolved value no longer corrupt tools_meta.json. Added regression tests covering these characters.

sanitizedToolsMeta := toolsMetaJSON
for _, expr := range sliceutil.SortedKeys(expressionEnvVars) {
envName := expressionEnvVars[expr]
envValues[envName] = expr

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in e403f67: buildToolsMetaRuntimeData now decodes the matched JSON string fragment via decodeJSONStringFragment (reversing encoding/json's HTML-escaping of <, >, &) before storing it as the step env value, while still replacing the encoded form in sanitizedToolsMeta. Added TestBuildToolsMetaRuntimeDataDecodesHTMLEscapedExpression to cover this.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot add tests

Added tests in 3e2b5f6 (approx, see latest commit): Go tests for buildToolsMetaRuntimeData covering multiple distinct expressions and deduped repeated expressions, plus JS tests for GH_AW_TOOLS_META_JSON covering multiple placeholders and unresolved placeholders.

@copilot add tests

Added tests in 8b6b0ba: Go tests for buildToolsMetaRuntimeData covering multiple distinct expressions and deduped repeated expressions, plus JS tests for GH_AW_TOOLS_META_JSON covering multiple placeholders and unresolved placeholders.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please address the remaining maintainer-visible blockers on this PR in one pass.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 23.7 AIC · ⌖ 8.26 AIC · ⊞ 9.3K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please address the remaining maintainer-visible blockers on this PR in one pass.

  • There are unresolved review comments about JSON-safe placeholder substitution in generate_safe_outputs_tools.cjs and encoded expression restoration in buildToolsMetaRuntimeData; please fix both and reply on-thread.
  • Please refresh the branch if needed after the fixes land.
  • Then run the pr-finisher skill before handing back for maintainer review.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14 AIC · ⌖ 8.19 AIC · ⊞ 9.3K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address the remaining maintainer-visible blockers on this PR in one pass.

  • There are unresolved review comments about JSON-safe placeholder substitution in `generate_safe_outputs_too...

Fixed both in e403f67:

  • generate_safe_outputs_tools.cjs: resolveEnvStringPlaceholders now JSON-escapes the resolved env value (via JSON.stringify(...).slice(1,-1)) before splicing it into the JSON text, so quotes/backslashes/newlines in the value no longer corrupt tools_meta.json.
  • mcp_setup_safe_outputs.go: buildToolsMetaRuntimeData now decodes the matched JSON string fragment (reversing encoding/json's HTML-escaping of <, >, &, etc.) before storing it as the step env value, while still replacing the encoded form in sanitizedToolsMeta.

Added regression tests for both.

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: bug
  • Risk: high
  • Priority score: 74/100 (impact 40 + urgency 22 + quality 12)
  • Recommended action: fast_track

Compiler fix: prevents GH_AW_TOOLS_META_JSON from exceeding GitHub Actions 21KB expression limit, a real compile-blocking regression.

Generated by 🔧 PR Triage Agent · auto · 44.3 AIC · ⌖ 2.78 AIC · ⊞ 8.3K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR looks close, but it still needs a maintainer-facing finish pass.

  • Please refresh the branch if needed.
  • Then run the pr-finisher skill and leave a concise maintainer-visible handoff comment with validation and any remaining risks.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.53 AIC · ⌖ 8.14 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot This PR looks close, but it still needs a maintainer-facing finish pass.

  • Please refresh the branch if needed....

Verified the branch is up to date and both prior review threads (JSON-safe placeholder escaping in generate_safe_outputs_tools.cjs, HTML-escape decoding in buildToolsMetaRuntimeData) are already fixed in e403f67/900b17f, with regression tests passing locally.

Local validation on HEAD (900b17f):

  • make fmt — clean, no diff.
  • make lint — 0 errors; only pre-existing warnings and one pre-existing testifylint failure in pkg/intent/governance_formal_test.go (untouched by this PR, last modified in [docs] Update glossary - daily scan #53961).
  • Go tests: TestBuildToolsMetaRuntimeData* (5 cases) pass.
  • JS tests: generate_safe_outputs_tools.test.cjs (32 cases) pass, including prettier formatting check.
  • make recompile — no diff, all 286 workflows compile clean.
  • go build ./... — clean.

I can't resolve the two GitHub review-thread conversations from this environment (no thread-resolve capability available to me), but their underlying issues are fixed in code as noted above. CI on this HEAD hasn't re-run since my push and needs a maintainer re-trigger (close/reopen, workflow_dispatch, or a new push) to go green.

Remaining: awaiting maintainer review/approval (mergeable_state is blocked pending requested reviews from @pelikhan and @gh-aw-bot).

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Category: bug · Risk: high · Score: 70/100 (impact 35 + urgency 25 + quality 10)
Recommended action:fast_track

Critical compiler correctness fix (expression-size overflow) with CI green; review history has churn (dismissed review) — highest score, needs prompt human attention.

Automated triage — see run report for full details.

Generated by 🔧 PR Triage Agent · auto · 90.2 AIC · ⌖ 2.82 AIC · ⊞ 8.3K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR looks close, but it still still needs a maintainer-facing finish pass.

Please do one final forward-progress sweep:

  • Resolve or answer any remaining unresolved review thread(s) directly on-thread.
  • Refresh the branch if GitHub offers one.
  • Then run the pr-finisher skill and leave a concise maintainer-facing handoff comment with validation and remaining risks.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 31.4 AIC · ⌖ 8.18 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot This PR looks close, but it still still needs a maintainer-facing finish pass.

Please do one final forward-progress sweep:...

Replied on-thread to both previously unresolved review comments (r3812371390, r3812371435) confirming the fixes landed in e403f67, with commit references and test names. I don't have a thread-resolve capability in this environment, so the threads remain open pending a maintainer/reviewer marking them resolved.

Branch base is unchanged (still on main@4bb738f) — no refresh needed, not behind.

Local validation on HEAD (900b17f), re-verified:

  • make fmt — clean, no diff.
  • make lint — 0 errors; only pre-existing warnings plus one pre-existing testifylint failure in pkg/intent/governance_formal_test.go (unrelated, last touched by [docs] Update glossary - daily scan #53961).
  • Go tests TestBuildToolsMetaRuntimeData* (5 cases) — pass.
  • JS tests generate_safe_outputs_tools.test.cjs (32 cases) — pass, prettier clean.
  • make recompile — no diff, 286/286 workflows compile clean.
  • go build ./... — clean.

Remaining risk/blocker: mergeable_state is blocked pending requested reviews from @pelikhan and @gh-aw-bot — not actionable from this environment. CI on this HEAD is stale since my last push and needs a maintainer re-trigger (close/reopen, workflow_dispatch, or a new push).

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR looks close, but it still needs a maintainer-facing finish pass.

Please do one final forward-progress sweep:

  • Resolve or answer the remaining unresolved review threads directly on-thread.
  • Refresh the branch if GitHub offers one.
  • Then run the pr-finisher skill and leave a concise maintainer-facing handoff comment with validation and remaining risks.

Open review threads (newest first):

If both are already fixed on the current head, reply on-thread with the exact commit/test evidence so reviewers can resolve them quickly.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.9 AIC · ⌖ 8.28 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot This PR looks close, but it still needs a maintainer-facing finish pass.

Please do one final forward-progress sweep:...

Both threads already have on-thread replies with commit/test evidence from the prior pass: r3816286526 and r3816287113. No code changed since then; HEAD is still 900b17f, branch is not behind main, and local validation (fmt/lint/tests/recompile/build) remains green as previously reported. Resolving the threads requires maintainer/reviewer action, which I can't do from this environment.

@pelikhan
pelikhan merged commit bece4a9 into mainAug 20, 2026
40 checks passed
@pelikhan
pelikhan deleted the copilot/aw-compat-daily-audit branch August 20, 2026 00:03
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.87.2

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[aw-compat] Daily AW compat audit (2026-08-19): 8/20 repos fail after fix, gh search code unavailable

4 participants

@pelikhan@gh-aw-bot