Conversation
…vation output transforms into dev.md v2.9 - Add Engine Interface Architecture subsection (ISP 7-interface design, BaseEngine, EngineRegistry) from adding-new-engines.md and engine-review-summary.md - Add JavaScript Content Sanitization Pipeline subsection with HTML entity bypass fix documentation and T24 template delimiter neutralization - Add Activation Output Transformations subsection documenting compiler expression rewriting for needs.activation.outputs.* → steps.sanitized.* - Add 4 new Related Documentation links to previously uncovered spec files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
automation
documentation
|
Pull request created: #17999 |
There was a problem hiding this comment.
Pull request overview
Consolidates previously separate scratchpad specifications into scratchpad/dev.md, updating the document to v2.9.
Changes:
- Bumped
scratchpad/dev.mdversion to v2.9 and updated last-updated date. - Added new consolidated subsections for engine interface architecture, JS content sanitization pipeline, and activation output transformations.
- Expanded “Related Documentation” links and appended a v2.9 entry to document history.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ├── Engine – core identity (GetID, GetDisplayName, IsExperimental) | ||
| ├── CapabilityProvider – feature flags (SupportsFirewall, SupportsMaxTurns, ...) | ||
| ├── WorkflowExecutor – GitHub Actions step generation | ||
| ├── MCPConfigProvider – MCP server configuration rendering | ||
| ├── LogParser – log metric extraction | ||
| └── SecurityProvider – secret names and detection model |
There was a problem hiding this comment.
The section says CodingAgentEngine is composed from “7 focused interfaces”, but the diagram only lists 6 and omits ModelEnvVarProvider (present in pkg/workflow/agentic_engine.go where CodingAgentEngine embeds ModelEnvVarProvider). Also, the Engine interface includes GetDescription() in code, but it’s missing from the “core identity” method list here. Please update the diagram/text to match the current interface set/signatures.
| ├── Engine – core identity (GetID, GetDisplayName, IsExperimental) | |
| ├── CapabilityProvider – feature flags (SupportsFirewall, SupportsMaxTurns, ...) | |
| ├── WorkflowExecutor – GitHub Actions step generation | |
| ├── MCPConfigProvider – MCP server configuration rendering | |
| ├── LogParser – log metric extraction | |
| └── SecurityProvider – secret names and detection model | |
| ├── Engine – core identity (GetID, GetDisplayName, GetDescription, IsExperimental) | |
| ├── CapabilityProvider – feature flags (SupportsFirewall, SupportsMaxTurns, ...) | |
| ├── WorkflowExecutor – GitHub Actions step generation | |
| ├── MCPConfigProvider – MCP server configuration rendering | |
| ├── LogParser – log metric extraction | |
| ├── SecurityProvider – secret names and detection model | |
| └── ModelEnvVarProvider – model environment variable configuration |
| The JavaScript sanitization module (`actions/setup/js/sanitize_content_core.cjs`) applies a multi-stage pipeline to all incoming content before it is written to GitHub resources: | ||
|
|
||
| ``` | ||
| Input Text | ||
| │ | ||
| ▼ hardenUnicodeText() | ||
| ├─ Unicode normalization (NFC) | ||
| ├─ HTML entity decoding ← prevents entity-encoded bypass attacks | ||
| ├─ Zero-width character removal | ||
| ├─ Bidirectional control removal | ||
| └─ Full-width ASCII conversion | ||
| │ | ||
| ▼ ANSI escape sequence removal | ||
| │ | ||
| ▼ neutralizeTemplateDelimiters() ← T24 defense-in-depth | ||
| ├─ Jinja2/Liquid: {{ }} → \{\{ | ||
| ├─ ERB: <%= %> → \<%= | ||
| ├─ JS template literals: ${ } → \$\{ | ||
| └─ Jekyll/Liquid directives: {% %} → \{% | ||
| │ | ||
| ▼ neutralizeMentions() | ||
| │ | ||
| ▼ Output (safe text) | ||
| ``` |
There was a problem hiding this comment.
The sanitization pipeline diagram/order here doesn’t match the actual sanitizeContentCore() implementation in actions/setup/js/sanitize_content_core.cjs: mentions are neutralized before neutralizeTemplateDelimiters(), and there are additional core stages (e.g., neutralizeCommands, XML comment removal/tag conversion, URL redaction, truncation, GitHub ref/bot-trigger neutralization, markdown code-region balancing). Also, the {% escaping is shown as \{%, but the code replaces {% with \{\% (rendered as \{\%). Please adjust the diagram and accompanying description so the documented order/output matches the code.
| **Why this matters for runtime-import**: When a workflow uses `{{#runtime-import}}` to include an external file at runtime (without recompiling), any new references to `needs.activation.outputs.{text|title|body}` introduced by that file will work correctly because the compiler pre-generates all known expressions and applies the transformation before execution. | ||
|
|
||
| **Implementation**: `pkg/workflow/expression_extraction.go::transformActivationOutputs()` |
There was a problem hiding this comment.
The “Why this matters for runtime-import” explanation appears incorrect: imports without inputs are emitted as {{#runtime-import ...}} and their file contents are not parsed for expression extraction/rewrite at compile time (see pkg/workflow/compiler_yaml.go runtime-import generation). Since transformActivationOutputs() only runs during expression extraction, new needs.activation.outputs.{text|title|body} references introduced solely in runtime-imported files won’t be rewritten/substituted. Please reword this to reflect that the rewrite applies to expressions the compiler extracts (or recommend using steps.sanitized.outputs.* directly in runtime-imported content / enable inlined-imports when relying on rewrites).
| **Why this matters for runtime-import**: When a workflow uses `{{#runtime-import}}` to include an external file at runtime (without recompiling), any new references to `needs.activation.outputs.{text|title|body}` introduced by that file will work correctly because the compiler pre-generates all known expressions and applies the transformation before execution. | |
| **Implementation**: `pkg/workflow/expression_extraction.go::transformActivationOutputs()` | |
| **Why this matters for runtime-import**: The rewrite only applies to expressions that the compiler extracts at compile time. Imports without inputs are emitted as `{{#runtime-import ...}}`, and their file contents are not parsed for expression extraction or rewrite. New references to `needs.activation.outputs.{text|title|body}` introduced *solely* inside runtime-imported files will therefore **not** be rewritten. When authoring runtime-imported content, either: | |
| - use `steps.sanitized.outputs.{text|title|body}` directly, or | |
| - enable inlined imports for that file so the compiler can see and transform the expressions during extraction. | |
| **Implementation**: `pkg/workflow/expression_extraction.go::transformActivationOutputs()` — this runs during expression extraction over the content known at compile time. |
Consolidates documentation from three previously uncovered spec files into
scratchpad/dev.md, bringing it to v2.9.Changes Made
scratchpad/dev.md(84 lines net, 1,949 → 2,033)New Content
Engine Interface Architecture (under Code Organization)
Documents the ISP-based 7-interface composite design from
adding-new-engines.mdandengine-review-summary.md:Engine,CapabilityProvider,WorkflowExecutor,MCPConfigProvider,LogParser,SecurityProvider→CodingAgentEngine. ExplainsBaseEngineembedding andEngineRegistryusage.JavaScript Content Sanitization Pipeline (under Security Best Practices)
Documents the multi-stage JS pipeline from
html-entity-mention-bypass-fix.mdandtemplate-syntax-sanitization.md— previously absent from dev.md:@mention bypass attacksActivation Output Transformations (under Workflow Patterns)
Documents compiler behavior from
activation-output-transformations.md: the compiler rewritesneeds.activation.outputs.{text,title,body}→steps.sanitized.outputs.*within the activation job. Explains the runtime-import compatibility rationale and word-boundary checking.Files Modified
scratchpad/dev.md— v2.9 with 3 new subsectionsValidation
Review Notes
pkg/workflow/agentic_engine.gosanitize_content_core.cjsexpression_extraction.go