Repository navigation
Propagate workflow environments to activation jobs - #71617
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review other builtin jobs |
|
✅ 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.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based 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. Design Decision Gate 🏗️Outcome: 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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: no blocking issues
The activation-job environment fallback is consistent with the stated goal of propagating top-level workflow environment protection to the job that consumes scoped secrets. Manual-approval precedence remains intact, and the added unit coverage covers empty, scalar, multiline, and precedence cases. No actionable correctness issue was identified in the changed lines.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd (per pr-triage). This is a clean, surgical bug fix: the activation job now inherits the workflow-level environment: unless manual-approval overrides it, matching the existing precedent set by resolveSafeOutputsEnvironment for safe-output jobs.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Root cause addressed, not just the symptom (
/diagnosing-bugs): the fix directly targets the gap described in the linked issue — activation previously dropped the top-levelenvironment:entirely, so environment-scoped secrets and protection rules never applied to that job. - ✅ Strong test coverage (
/tdd):TestBuildActivationJobEnvironmentis table-driven and covers all four meaningful branches — no environment, scalar inheritance, multi-line object inheritance with correct indentation, and manual-approval precedence. Descriptive names make each case read as a spec. - ✅ Consistent with existing conventions (
/codebase-design): reusingc.indentYAMLLines(ctx.data.Environment, " ")mirrors the exact pattern already used incompiler_main_job.gofor the main job's environment, and the gateway-provenance guard (job.Environment = ""forgateway_provenance/admission) already strips environment unconditionally, so this change doesn't reopen that safety gate. - ✅ Verified independently: ran
TestBuildActivationJobEnvironment,TestBuildActivationJob*,TestGatewayProvenance*, andTestEnvironmentOverride*— all pass.
Minor Observations (non-blocking)
- The PR changes generated output for any existing workflow that already sets a top-level
environment:(previously silently ignored by activation, now applied) — worth a one-line CHANGELOG.md entry under Unreleased so downstream consumers notice the behavior change ongh aw compile. - docs/reference/frontmatter.md's "Environment Protection" section and docs/reference/triggers.md's "Manual Approval Gates" section don't yet mention that the activation job now inherits the top-level
environment:whenmanual-approvalis absent — a short clarifying sentence in either doc would help authors understand the new inheritance/precedence relationship.
Neither point blocks merge; both are documentation-only follow-ups.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~111 AIC · ⌖ ~15 AIC · ⊞ 10.2K
Comment /matt to run again
There was a problem hiding this comment.
🔵 Needs a closer look
Changing approval gates and environment-scoped secret and variable availability across affected workflows warrants maintainer confirmation.
1 open issue
What changed in this PR
Propagates configured workflow environments to activation jobs, extending environment protections and scoped secrets to that stage of execution.
Changes:
- Inherits the workflow environment while preserving
manual-approvalprecedence. - Adds regression coverage for absent, scalar, and object environments and approval overrides.
- Refreshes three generated workflows to apply
aoai-modelduring activation.
| File | Description |
|---|---|
pkg/workflow/compiler_activation_outputs.go |
Adds workflow-environment fallback for activation. |
pkg/workflow/compiler_activation_jobs_test.go |
Tests environment inheritance and override behavior. |
.github/workflows/smoke-copilot-aoai-entra.lock.yml |
Applies aoai-model to activation. |
.github/workflows/smoke-azure-pi.lock.yml |
Applies aoai-model to activation. |
.github/workflows/smoke-azure-codex.lock.yml |
Applies aoai-model to activation. |
Review effort: Max
| assert.NotContains(t, job.Outputs, "secret_verification_result", "Activation job should not expose secret_verification_result output when validate-secret is skipped") | ||
| } | ||
|
|
||
| func TestBuildActivationJobEnvironment(t *testing.T) { |
There was a problem hiding this comment.
Renamed the test to TestBuildActivationJob_Environment and added the environment precedence comment. Commit: c48227e.
Reviewed the other built-in jobs. The agent and evals jobs already use the top-level environment; detection honors its configured override and otherwise falls back to the top-level environment. Pre-activation, safe-output jobs (including upload-assets), conclusion, and the upload-code-scanning/coverage jobs use Admission and gateway trust-verification jobs intentionally do not inherit it: gateway guards explicitly clear environment assignments so central trust credentials cannot resolve through an author-selected environment, and admission uses central variables/secrets. The custom graders job is also deliberately environment-free and isolated. I made no further changes. |
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 81a5de5
|
…is-report-2026-10-24 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
The activation job can consume secrets but did not inherit a workflow’s configured environment, so its environment protections and scoped secrets were not applied.
environment; preservemanual-approvalas the activation job’s override. Workflows without either remain unchanged.aoai-model.The activation job now uses
productionunlesson.manual-approvalspecifies a different environment. This does not add a default environment to workflows that have none.