Conversation
Adds support for configuring environment variables on the APM pack step:
dependencies:
packages:
- microsoft/apm-sample-package
env:
NAME: VALUE
The env values are set on the 'Install and pack APM dependencies' step
that runs microsoft/apm-action. Env vars are sorted alphabetically for
deterministic output and merged with GITHUB_TOKEN when github-app is
also configured.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Upgrade to microsoft/apm v0.8.2 |
|
@copilot upgrade microsoft/apm and its actions |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in f8d55c1:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
env support to dependencies configurationenv support to dependencies configuration and upgrade microsoft/apm to v0.8.2
|
💥 WHOOSH! Smoke Claude springs into action on this pull request! [Panel 1 begins...] |
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds support for specifying environment variables under the dependencies frontmatter configuration so they can be passed to the microsoft/apm-action pack step, and upgrades the default APM CLI/action versions.
Changes:
- Extend
APMDependenciesInfoand frontmatter extraction to parse adependencies.envmap (skipping non-string values). - Update APM pack-step generation to include the parsed env vars in a deterministic (sorted) order, alongside the GitHub App token env when configured.
- Bump pinned versions: APM CLI default to
v0.8.2andmicrosoft/apm-actiontov1.3.2(SHA-pinned), with related lockfile updates and new tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/frontmatter_types.go |
Adds Env map[string]string to APM dependency config. |
pkg/workflow/frontmatter_extraction_metadata.go |
Parses dependencies.env from object-format frontmatter and logs env count. |
pkg/workflow/apm_dependencies.go |
Emits env: block for APM pack step including GitHub App token + user env vars (sorted). |
pkg/workflow/apm_dependencies_test.go |
Adds unit tests for env extraction + pack-step env rendering and determinism. |
pkg/workflow/apm_dependencies_compilation_test.go |
Adds integration compilation coverage for env vars appearing in the lock file. |
pkg/constants/constants.go |
Bumps DefaultAPMVersion to v0.8.2. |
pkg/workflow/data/action_pins.json |
Updates pin for microsoft/apm-action@v1.3.2 SHA. |
.github/aw/actions-lock.json |
Updates internal actions lock for microsoft/apm-action@v1.3.2 SHA. |
.github/workflows/smoke-claude.lock.yml |
Updates smoke workflow lock to APM v0.8.2 and action SHA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| keys = append(keys, k) | ||
| } | ||
| sort.Strings(keys) | ||
| for _, k := range keys { |
|
|
||
| aPos := strings.Index(combined, "A_VAR:") | ||
| mPos := strings.Index(combined, "M_VAR:") | ||
| zPos := strings.Index(combined, "Z_VAR:") |
This comment has been minimized.
This comment has been minimized.
|
Smoke Test Results — Run §23273976334 ✅ GitHub MCP | ✅ GH CLI | ❌ Serena MCP | ✅ Playwright | ✅ Web Fetch | ✅ File Write | ✅ Bash | ✅ Discussion | ✅ Build | ✅ Discussion Create | ✅ Haiku Dispatch | ✅ PR Review Overall: FAIL — Serena
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Smoke test review of PR #21687. The env support for APM dependencies is well-implemented with sorted keys for determinism and comprehensive test coverage. One potential concern: env values are written to YAML without quoting, which could produce invalid YAML if values contain special characters.
📰 BREAKING: Report filed by Smoke Copilot
| } | ||
| sort.Strings(keys) | ||
| for _, k := range keys { | ||
| lines = append(lines, fmt.Sprintf(" %s: %s", k, apmDeps.Env[k])) |
There was a problem hiding this comment.
YAML values are written here without quoting. If a user passes a value containing YAML special characters (e.g., a colon followed by a space, a #, or curly braces), the generated workflow file could be invalid. Consider wrapping the value in quotes or using a YAML-safe formatter: fmt.Sprintf(" %s: %q", k, apmDeps.Env[k]) or at least single-quoting values that contain special chars.
| Isolated bool // If true, agent restore step clears primitive dirs before unpacking | ||
| GitHubApp *GitHubAppConfig // Optional GitHub App for cross-org private package access | ||
| Version string // Optional APM CLI version override (e.g., "v0.8.0"); defaults to DefaultAPMVersion | ||
| Env map[string]string // Optional environment variables to set on the APM pack step |
There was a problem hiding this comment.
Nice, clean addition. The sorted-key determinism in GenerateAPMPackStep ensures reproducible compiled output, which is great for lock file stability.
There was a problem hiding this comment.
Agreed! Deterministic ordering is essential for stable diffs in generated YAML. Great to see this covered by a test.
📰 BREAKING: Report filed by Smoke Copilot
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Commit pushed:
|
|
@copilot update Microsoft/apm again |
…test assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 950cfd1:
Also addressed the two code review issues in the same commit:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
Smoke TestPRs: #21768
Warning
|
|
Smoke test results for PR #21687 (author:
Overall:
|
There was a problem hiding this comment.
Clean implementation of env support for APM dependencies. Deduplication logic for GITHUB_TOKEN is correct and the sorted output ordering ensures deterministic YAML generation. Tests are thorough covering the new cases.
📰 BREAKING: Report filed by Smoke Copilot
| Env map[string]string // Optional environment variables to set on the APM pack step | ||
| } | ||
|
|
||
| // RateLimitConfig represents rate limiting configuration for workflow triggers |
There was a problem hiding this comment.
The new Env field is well-placed in the struct. Documenting it as "Optional environment variables to set on the APM pack step" makes intent clear. Consistent with the existing comment style in the struct.
| lines = append(lines, fmt.Sprintf(" %s: %s", k, apmDeps.Env[k])) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Good defensive check — skipping GITHUB_TOKEN when a GitHub App token is already being injected prevents duplicate env key conflicts. The sort.Strings(keys) call ensures deterministic output ordering across Go map iterations. 👍
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
| GitHubApp *GitHubAppConfig // Optional GitHub App for cross-org private package access | ||
| Version string // Optional APM CLI version override (e.g., "v0.8.0"); defaults to DefaultAPMVersion | ||
| Packages []string // APM package slugs to install (e.g., "org/package") | ||
| Isolated bool // If true, agent restore step clears primitive dirs before unpacking |
There was a problem hiding this comment.
Good addition of the Env field to APMDependenciesInfo. Using map[string]string is appropriate here since env vars are key-value pairs. The comment clearly explains its purpose.
| lines = append(lines, fmt.Sprintf(" %s: %s", k, apmDeps.Env[k])) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Using sort.Strings(keys) ensures deterministic output ordering for env vars — great practice for generated YAML to keep diffs clean and avoid spurious recompilations.
|
Commit pushed:
|
The
dependenciesobject format lacked a way to pass environment variables to themicrosoft/apm-actionpack step, making it impossible to configure private registries or inject tokens without workarounds.Changes
APMDependenciesInfo— AddedEnv map[string]stringfieldextractAPMDependenciesFromFrontmatter— Parsesenvmap from object-format dependencies; non-string values are skipped with a debug log; env count added to extraction logGenerateAPMPackStep— Injects user env vars (sorted for determinism) into the pack step'senv:block, merged alongsideGITHUB_TOKENwhengithub-appis also configured; user-suppliedGITHUB_TOKENis silently skipped whengithub-appis configured to prevent duplicate YAML keysDefaultAPMVersion— Upgraded fromv0.8.0tov0.8.2microsoft/apm-action— Upgraded fromv1.3.1tov1.3.4(SHA83d54a6c7941049210433b16c8dfac573665b12a); includes SARIF/markdown audit report support and fixes for GITHUB_TOKEN passthrough and preservationUsage
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.
✨ PR Review Safe Output Test - Run 23274525021
✨ PR Review Safe Output Test - Run 23294151374