Add support for pre-agent-steps before agent execution#26666
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f31e2705-9e77-4c04-aee1-170074af0afd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f31e2705-9e77-4c04-aee1-170074af0afd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Draft ADR capturing the design decision to introduce `pre-agent-steps` as a distinct workflow extension point injected immediately before engine execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic ( AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft covers the key design decision to introduce 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: §24519930482
|
🧪 Test Quality Sentinel ReportTest Quality Score: 81/100✅ Excellent
Test Classification DetailsView all 7 test scenarios
Flagged Tests — Requires Review
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 81/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 7 new test scenarios verify behavioral contracts. Minor nit: add descriptive messages to 5 bare assert.* calls in compiler_orchestrator_workflow_test.go.
There was a problem hiding this comment.
Pull request overview
Adds a new pre-agent-steps frontmatter field that can be imported/merged and is injected immediately before the agent engine execution step, with the same strict-mode secret-expression restrictions as other custom step sections.
Changes:
- Add
pre-agent-stepsto workflow frontmatter types/serialization, schema validation, and user docs. - Support importing/merging
pre-agent-stepsfrom imported workflows and apply action pinning during merge. - Emit merged
pre-agent-stepsin the generated main job YAML right before engine execution; add unit tests covering generation, import merge order, and strict-mode secret validation.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/workflow_builder.go | Adds merge logic for imported + main pre-agent-steps, with action pinning and YAML serialization into WorkflowData. |
| pkg/workflow/strict_mode_steps_validation_test.go | Extends strict-mode secret validation tests to cover pre-agent-steps. |
| pkg/workflow/strict_mode_steps_validation.go | Includes pre-agent-steps in strict-mode step secret scanning. |
| pkg/workflow/frontmatter_types.go | Adds PreAgentSteps to the parsed frontmatter config struct. |
| pkg/workflow/frontmatter_serialization.go | Serializes pre-agent-steps when converting FrontmatterConfig to a map. |
| pkg/workflow/compiler_yaml_main_job.go | Injects pre-agent-steps right before engine execution in the main job. |
| pkg/workflow/compiler_yaml.go | Adds generatePreAgentSteps to emit the section via the shared steps writer. |
| pkg/workflow/compiler_types.go | Adds PreAgentSteps to WorkflowData. |
| pkg/workflow/compiler_pre_agent_steps_test.go | New tests asserting correct placement and import merge ordering in generated lock workflow YAML. |
| pkg/workflow/compiler_orchestrator_workflow_test.go | Adds unit tests for processAndMergePreAgentSteps behavior. |
| pkg/workflow/compiler_orchestrator_workflow.go | Wires processAndMergePreAgentSteps into workflow parsing. |
| pkg/parser/schemas/main_workflow_schema.json | Adds schema definition for pre-agent-steps. |
| pkg/parser/import_processor.go | Extends ImportsResult with MergedPreAgentSteps. |
| pkg/parser/import_field_extractor.go | Extracts pre-agent-steps from imports and accumulates them for merging. |
| docs/src/content/docs/reference/frontmatter.md | Documents the new pre-agent-steps field. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 16/16 changed files
- Comments generated: 3
| // It validates that secrets expressions are not used in custom steps (steps, | ||
| // pre-agent-steps, and post-steps injected in the agent job). In strict mode, secrets in step-level |
There was a problem hiding this comment.
File header comment says strict mode validation applies to steps/pre-agent-steps/post-steps, but validateStepsSecrets also validates the pre-steps section. Update the header comment to include pre-steps so it matches actual behavior.
| // It validates that secrets expressions are not used in custom steps (steps, | |
| // pre-agent-steps, and post-steps injected in the agent job). In strict mode, secrets in step-level | |
| // It validates that secrets expressions are not used in custom steps (pre-steps, | |
| // steps, pre-agent-steps, and post-steps injected in the agent job). In strict mode, secrets in step-level |
| // writeStepsSection writes a steps section (pre-steps or post-steps) to the YAML builder, | ||
| // stripping the header line and normalising indentation to match the agent job step format: | ||
| // top-level items get 6-space indent ( - name:) and nested properties get 8-space indent ( run:). |
There was a problem hiding this comment.
writeStepsSection is now used for pre-agent-steps as well (via generatePreAgentSteps), but the comment still says it only writes pre-steps/post-steps. Please update the comment to include pre-agent-steps so future readers don’t miss this usage.
See below for a potential fix:
// writeStepsSection writes a steps section (pre-steps, post-steps, or pre-agent-steps)
// to the YAML builder, stripping the header line and normalising indentation to match
// the agent job step format: top-level items get 6-space indent ( - name:) and
// nested properties get 8-space indent ( run:).
func writeStepsSection(yaml *strings.Builder, stepsYAML string) {
if stepsYAML == "" {
return
}
lines := strings.Split(stepsYAML, "\n")
for _, line := range lines[1:] { // skip the "pre-steps:" / "post-steps:" / "pre-agent-steps:" header line
| ``` | ||
|
|
||
| Use pre-agent steps when work must happen right before the engine runs (for example, final context preparation or last-moment validations). | ||
|
|
There was a problem hiding this comment.
The new pre-agent-steps section doesn’t mention that these steps execute outside the firewall sandbox (like steps: and post-steps: do). Consider adding a sentence clarifying the security/runtime context for pre-agent-steps for consistency and to avoid confusion.
| Pre-agent steps run outside the firewall sandbox. These steps execute with standard GitHub Actions security. |
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/57825618-a1ee-430e-a1c3-7ee10040d226 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all actionable review comments in commit |
Summary
pre-agent-stepspre-agent-stepsand merge them with main workflow valuespre-agent-stepsimmediately before the agent engine execution steppre-agent-stepsValidation
go test -v -run "TestPreAgentStepsGeneration|TestPreAgentStepsImportsMergeOrder|TestProcessAndMergePreAgentSteps|TestValidateStepsSecrets" ./pkg/workflow/go test -v ./pkg/parser/...make buildmake agent-finish(fails due to pre-existing unrelated testifylint issues inpkg/stats/spec_test.goandpkg/testutil/spec_test.go)Notes
parallel_validationcode review returned comments for files not changed by this PR; no actionable changes were required in the current diff.parallel_validation.