Use aw_context fallbacks for injected GitHub prompt context in centralized slash/workflow-call paths#31820
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
aw_context fallbacks for injected GitHub prompt context in centralized slash/workflow-call paths
|
@copilot use built-in syntax sugar and support it in the compiler to reference parsed aw context values. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 44a6769. I added compiler support for |
|
@copilot merge main and recompile |
| @@ -9,17 +9,17 @@ The following GitHub context information is available for this workflow: | |||
| {{#if ${{ github.workspace }} }} | |||
| - **workspace**: ${{ github.workspace }} | |||
| {{/if}} | |||
| {{#if ${{ github.event.issue.number }} }} | |||
| - **issue-number**: #${{ github.event.issue.number }} | |||
| {{#if ${{ github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number) }} }} | |||
There was a problem hiding this comment.
@copilot the nested expression seems unecessary:
{{#if ${{ ... }} }} => {{#if ... }}
There was a problem hiding this comment.
Addressed in fee36f4 — I removed the nested ${{ ... }} wrappers in the {{#if ...}} tags and regenerated the affected wasm golden fixtures.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (151 new lines in default business directories, above the 100-line threshold) but does not have a linked Architecture Decision Record (ADR). AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in References:
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /tdd — this is a bug fix that centralises aw_context fallback logic for slash-command and workflow_call paths.
Key Themes
Critical: {{#if}} conditions are always truthy (regression introduced)
The old pattern was {{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__}} — after env-var substitution the condition became the actual value (e.g. 42 or "") which isTruthy evaluates correctly.
The new pattern is:
{{#if github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number)}}
This raw string is never substituted — isTruthy in is_truthy.cjs receives the full literal string, which is not in the falsy list ("", "false", "0", etc.), so it always returns true. The prompt will always render - **issue-number**: #false even when no issue exists.
Fix: The {{#if}} condition should use the env-var placeholder that IS substituted — i.e. the same ${{ ... }} wrapper approach as the value (which gets compiled to __GH_AW_EXPR_XXXX__). Either restore the {{#if ${{ expr }} }} pattern, or extend isTruthy to evaluate || expressions.
Missing end-to-end regression test
The new github_context_prompt_test.go only checks the source template text. There is no test that runs the full compile→render pipeline with a workflow_call trigger and asserts the prompt contains the right number — meaning the original bug could return undetected.
Positive Highlights
- ✅
github.aw.context.*syntax sugar is a clean abstraction that hides thefromJSON(...)verbosity from template authors - ✅ The
transformAwContextExpressionfunction is well-isolated and has a doc comment with a clear example - ✅ The lock files are consistently regenerated across all affected workflows
- ✅ The existing
expression_extraction_test.gotable-driven test was correctly extended
Verdict
Requesting changes on the {{#if}} condition bug before merge — it's a correctness regression that would cause all four conditional context fields to always appear in the prompt with false as their value.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 12.5M
| - **workspace**: ${{ github.workspace }} | ||
| {{/if}} | ||
| {{#if ${{ github.event.issue.number }} }} | ||
| - **issue-number**: #${{ github.event.issue.number }} | ||
| {{#if github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number)}} |
There was a problem hiding this comment.
[/diagnose] The {{#if ...}} condition here is always truthy at runtime.
interpolate_prompt.cjs only env-var-substitutes __GH_AW_EXPR_XXXX__ tokens inside the body of the block, not inside the {{#if ...}} tag itself. So isTruthy receives the raw literal string "github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number)" — which is never falsy per the rules in is_truthy.cjs. The section will always render, producing - **issue-number**: #false when no issue context exists.
The correct fix mirrors the old pattern: use the already-computed env-var placeholder as the condition too:
-{{#if github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number)}}
+{{#if ${{ github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number) }}}}This keeps condition and value in sync and the substituted value (false or a number) is correctly evaluated by isTruthy.
| } | ||
| } | ||
|
|
||
| assertContains("github.event.issue.number || (github.aw.context.item_type == 'issue' && github.aw.context.item_number)") |
There was a problem hiding this comment.
[/tdd] This test verifies the source template text contains the right strings, but does not test the rendered output at runtime. The actual bug being fixed — that workflow_call triggers without native event payloads produce empty context fields — has no regression test that would fail if the fix were reverted.
Consider adding an interpolate_prompt.test.cjs test (or a Go integration test) that simulates:
- No
github.event.*values present aw_contextinput set to{"item_type":"issue","item_number":42}- Asserts the rendered prompt contains
- **issue-number**: #42
This is the seam closest to the actual bug and would catch regressions in the full compile→render pipeline.
| @@ -250,6 +257,16 @@ func transformExperimentsExpression(expr string) string { | |||
| return expr | |||
| } | |||
|
|
|||
| // transformAwContextExpression rewrites github.aw.context.<field> references to | |||
| // parsed aw_context access expressions. | |||
| // | |||
There was a problem hiding this comment.
[/diagnose] transformAwContextExpression applies a regex replacement globally across the entire expression string, but there are no unit tests for edge cases of the regex [a-zA-Z0-9_-]+.
For example: what happens with github.aw.context. (empty field name) or github.aw.context.item-type (hyphen in field)? The regex allows - but fromJSON(...).item-type is not valid JavaScript property access syntax — it would be interpreted as fromJSON(...).item minus type.
Consider adding test cases:
{name: "hyphenated field treated correctly", expr: "github.aw.context.item-type"},
{name: "standalone transform", expr: "github.aw.context.item_number", want: "fromJSON(github.event.inputs.aw_context || '{}').item_number"},Also: the regex allows - in field names but the only known fields use _. Restricting to [a-zA-Z0-9_]+ would be safer.
|
@copilot review all comments and create an integration test that runders the github prompt template, simulate github action expression substitutaion and runs the javascript renderer |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot also applies to workflow_dispatch, repository_dispatch |
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in 4fbe7e1 (with core additions in dfe3b9f). I added an integration-style JS test in
Addressed in 4fbe7e1 (with core logic in dfe3b9f). |
…ntext-injected-prompt # Conflicts: # .github/workflows/daily-cache-strategy-analyzer.lock.yml # .github/workflows/daily-fact.lock.yml # .github/workflows/daily-observability-report.lock.yml # .github/workflows/duplicate-code-detector.lock.yml # .github/workflows/grumpy-reviewer.lock.yml # .github/workflows/issue-arborist.lock.yml # .github/workflows/necromancer.lock.yml # .github/workflows/schema-feature-coverage.lock.yml # .github/workflows/smoke-call-workflow.lock.yml # .github/workflows/smoke-codex.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Bug Fix
What was the bug?
Centralized slash-command and workflow_call executions could produce empty/unresolved GitHub context fields in the agent prompt (
issue-number,discussion-number,pull-request-number,comment-id) because those runs often lack nativegithub.event.*entity payloads.As a result, prompts lost key routing context even though the same metadata existed in
aw_context.How did you fix it?
github.event.*first, then fall back tofromJSON(github.event.inputs.aw_context || '{}').item_type) for number fields to avoid cross-entity leakage.aw_contextfallback expressions remain present in the embedded GitHub context prompt.Testing