Repository navigation
fix: emit compiler-generated constants as quoted strings - #71803
Conversation
Preserve deferred prompt markers as literal environment values across prompt creation, interpolation, and substitution. Emit unconditional pre-activation output as a quoted string rather than a literal-only Actions expression, without changing authored expressions or lint rules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Commit pushed:
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
No actionable blocking issues found in the changed lines. The deferred-marker quoting and unconditional activation output changes are covered by focused regression tests, and the generated YAML remains correctly typed.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
🏗️ Design Decision Gate — ADR RequiredThis PR adds a lot of code to core business logic (165 new lines in 📄 Draft ADR committed: Inferred decision: add an explicit
📋 What to do next
After the PR body links an ADR, this gate runs again and checks that the implementation matches the decision. ❓ Why ADRs Matter
ADRs keep a searchable, permanent record of why the codebase looks the way it does, so later contributors can see the reasoning. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — approving, no actionable issues found.
📋 Analysis
The fix addresses the root cause: compiler-generated literal content (deferred prompt placeholders, the unconditional activated output) was previously forced through single-quoted string syntax at the call site, which the obfuscation linter correctly flagged as ambiguous between a literal and an authored expression. The PR introduces an explicit ExpressionMapping.IsLiteral flag and a shared environmentValue() helper that routes literal content through the existing writeYAMLEnv (%q) helper, producing proper double-quoted YAML scalars, while authored expressions keep their ${{ ... }} wrapping unchanged.
Strengths:
- Root cause fixed in the type/flag, not papered over with linter suppressions.
TestExpressionMappingEnvironmentValueround-trips values (including empty string, the literal wordtrue, and a multi-line/quote-containing value) through YAML unmarshal to confirm the emitted scalar decodes back to the exact original content — a solid regression guard against the historical quoting bug.compiler_pre_activation_job_test.goandon_steps_test.goadditions assert theactivatedoutput decodes as the string"true"(not a YAML boolean), guarding the second part of the fix.- New tests invoke
workflowlint.ObfuscationRule().Check(...)directly against compiled fixtures to assert zero diagnostics, closing the loop back to the original 15 obfuscation-notice motivation. IsLiteraldefaults tofalseand is only set by the two call sites producing compiler-generated literals, so authored-expression handling (quoted-string heuristics, compound||expressions) is left untouched — verified inTestRewriteDeferredPromptExpressionswith explicitassert.Falsechecks ondeferred[0].IsLiteralandoriginal.IsLiteral.
I traced all three call sites that write expression-mapping values to YAML (generatePlaceholderEnvironment, template.go's interpolation step, unified_prompt_step.go's prompt-creation step) and confirmed they consistently route through environmentValue()/IsLiteral, so there's no remaining quoting divergence between the prompt-creation, interpolation, and substitution steps.
No blocking issues. This is a clean, well-tested, properly scoped bug fix.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~63 AIC · ⌖ ~15 AIC · ⊞ 10.2K
Comment /matt to run again
There was a problem hiding this comment.
🔵 Needs a closer look
The full validation gate remains incomplete, so human confirmation of validation results is needed before merging the generated-workflow changes.
No open issues 🎉
What changed in this PR
This PR updates gh-aw’s compiler to emit generated constants as quoted YAML strings, avoiding obfuscation notices while preserving authored expressions and deferred runtime bindings.
Changes:
- Marks deferred prompt placeholders as literals and handles them consistently across prompt-generation steps.
- Emits unconditional pre-activation output as
"true"rather than a literal-only expression. - Adds regression coverage for string typing, deferred bindings, and obfuscation diagnostics.
| File | Description |
|---|---|
pkg/workflow/unified_prompt_step.go |
Uses literal-aware serialization for prompt environment values. |
pkg/workflow/template.go |
Uses literal-aware serialization during interpolation. |
pkg/workflow/on_steps_test.go |
Checks quoted activation output, string typing, and lint diagnostics. |
pkg/workflow/known_needs_expressions_test.go |
Updates the deferred-marker quoting assertion. |
pkg/workflow/expression_extraction.go |
Adds literal tracking and environment-value serialization. |
pkg/workflow/compiler_pre_activation_job.go |
Quotes constant activation output values. |
pkg/workflow/compiler_pre_activation_job_test.go |
Covers constant outputs, conditional expressions, and missing checks. |
pkg/workflow/compiler_github_actions_steps.go |
Emits literal placeholder bindings directly as YAML strings. |
pkg/workflow/compiler_deferred_prompt.go |
Marks generated deferred placeholders as literals. |
pkg/workflow/compiler_deferred_prompt_test.go |
Expands prompt-mode, binding, string-typing, and lint regressions. |
Review effort: Max
Regenerate the four affected workflow locks with literal deferred prompt markers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Record verified literal mapping semantics, alternatives, and consequences while leaving approval pending PR review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
In response to this review comment: Accepted the skills review assessment; it reports no actionable issues, so no implementation change is needed. |
|
In response to this review comment: Accepted as an informational completion notice, not proof of readiness. Focused compiler and linter regressions pass independently; the existing custom-lint gate failures remain documented in the PR. |
|
In response to this review comment: Accepted the code quality review assessment that no actionable blocking issues were found. No implementation change is needed for this completion notice. |
|
In response to this review comment: Accepted as an informational no-action report rather than independent evidence of coverage. The focused compiler and linter regression tests were rerun successfully during this follow-up. |
|
In response to this review comment: Accepted and reviewed the generated ADR against the implementation, refining the rationale and preserving Proposed status without asserting reviewer approval. The finalized record is committed in 9d88b2c and linked in the PR description. |
|
In response to this review comment: Accepted: finalized the ADR in 9d88b2c with verified context, literal-versus-expression handling, alternatives, and consequences, and removed the unverified reviewer placeholder. The PR description now links the ADR; its status remains Proposed pending approval. |
|
🎉 This pull request is included in a new release. Release: |
Why
The public workflow corpus produced 15 obfuscation notices for compiler-generated string literals: 14 deferred prompt markers and one unconditional pre-activation output. Authors cannot safely repair generated bindings, so fix code generation rather than suppress the linter.
Changes
pre_activation.outputs.activatedas the string"true", not a YAML boolean or a redundant literal-only expression.main.ADR: ADR-71803: Quoted compiler-generated constants
Validation
make merge-maincompleted without conflicts, including build, formatting, and successful recompilation of all 348 workflows.--no-emitand no obfuscation notices. No workflows were executed.make agent-report-progress-no-testgates passed Go lint (zero issues), build, and schema freshness, but remain blocked by pre-existing custom-lint findings in touched files.The earlier full
make agent-report-progressgate also passed impacted tests. A broader test selection encountered unrelated checkout-shell syntax failures and a macOS/varversus/private/varpath assertion.