feat: Reimplement APM artifact pack/unpack support (#20385)#20564
feat: Reimplement APM artifact pack/unpack support (#20385)#20564
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
Smoke Test (Codex)PRs: #20569 [docs] Self-healing documentation fixes from issue analysis - 2026-03-11; #20566 [ca] fix: update test expectations for actions/download-artifact v8.0.1
Warning
|
|
Commit pushed:
|
|
Smoke Test Results - Run §22970861289
Overall: PARTIAL (15 passed, 2 skipped, 0 failed)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
|
|
||
| // GetAPMTarget returns the APM target value to use when packing dependencies with | ||
| // microsoft/apm-action. Supported values are "copilot", "claude", and "all". | ||
| // The default implementation returns "all" (packs all primitive types). |
There was a problem hiding this comment.
The GetAPMTarget() interface method is well-placed. Consider adding a note in the godoc that "all" packs every primitive type, which may result in larger bundles compared to engine-specific targets.
| // GenerateAPMRestoreStep generates the GitHub Actions step that restores APM packages | ||
| // from a pre-packed bundle in the agent job. | ||
| // | ||
| // Parameters: |
There was a problem hiding this comment.
The GenerateAPMRestoreStep cleanly separates restore logic from pack logic. The wildcard *.tar.gz in the bundle path is flexible — worth verifying behavior when multiple .tar.gz files exist in the directory.
|
Smoke Test: Copilot - 22970861231
Overall:
|
There was a problem hiding this comment.
APM pack/unpack support looks well-structured. Two inline comments left on smoke-claude.md and the frontmatter docs. Overall the change is clean — documentation updates match the implementation changes nicely.
📰 BREAKING: Report filed by Smoke Copilot
| serena: | ||
| languages: | ||
| go: {} | ||
| dependencies: |
There was a problem hiding this comment.
The dependencies field looks good here — using the simple array format to add the APM sample package. This will trigger the pack/unpack lifecycle in the activation and agent jobs respectively.
| packages: | ||
| - microsoft/apm-sample-package | ||
| - github/awesome-copilot/skills/review-and-refactor | ||
| isolated: true # clear repo primitives before unpack (default: false) |
There was a problem hiding this comment.
Nice addition of the isolated: true option with an inline explanation. Might be worth mentioning what "repo primitives" refers to, or linking to the APM docs for readers unfamiliar with the concept.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
Pull request overview
Reimplements APM dependency handling by packing dependencies during the activation job and restoring them in the agent job via a dedicated apm artifact, improving determinism and reducing agent-job startup cost.
Changes:
- Adds
Isolatedsupport to APM dependency frontmatter (new object format withpackages+isolated). - Splits APM workflow generation into activation-time pack + upload and agent-time download + restore.
- Infers
microsoft/apm-actiontargetfrom the configured engine via a newGetAPMTarget()method.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/frontmatter_types.go | Extends APM dependency config with Isolated flag and updates commentary. |
| pkg/workflow/frontmatter_extraction_metadata.go | Adds object-format parsing for dependencies and extracts isolated. |
| pkg/workflow/copilot_engine.go | Implements GetAPMTarget() returning copilot. |
| pkg/workflow/claude_engine.go | Implements GetAPMTarget() returning claude. |
| pkg/workflow/agentic_engine.go | Extends WorkflowExecutor with GetAPMTarget() and provides BaseEngine default (all). |
| pkg/workflow/apm_dependencies.go | Replaces single install step with GenerateAPMPackStep + GenerateAPMRestoreStep. |
| pkg/workflow/compiler_activation_job.go | Emits pack step and uploads the apm artifact in the activation job. |
| pkg/workflow/compiler_yaml_main_job.go | Downloads the apm artifact and restores dependencies in the agent job. |
| pkg/workflow/apm_dependencies_test.go | Updates/extends unit tests for extraction, targets, and pack/restore step generation. |
| pkg/workflow/apm_dependencies_compilation_test.go | Updates/extends integration compilation assertions for pack/upload and download/restore behavior. |
| docs/src/content/docs/reference/frontmatter.md | Documents the new object format and pack/restore behavior. |
| .github/workflows/smoke-claude.md | Adds an APM dependency to exercise the pack/restore path. |
| .github/workflows/smoke-claude.lock.yml | Updates compiled output to include activation pack/upload + agent download/restore steps. |
💡 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.
| // Restore step should include isolated: true because frontmatter says so | ||
| assert.Contains(t, lockContent, "isolated: 'true'", | ||
| "Lock file restore step should include isolated flag") |
There was a problem hiding this comment.
In TestAPMDependenciesCompilationObjectFormatIsolated, the assertion assert.Contains(lockContent, "isolated: 'true'") can pass even if the restore step fails to include isolated, because the activation pack step always includes isolated: 'true'. To actually validate propagation of the frontmatter isolated: true flag into the agent-job restore step, assert that isolated: 'true' appears within the restore step block (e.g., by locating the "Restore APM dependencies" section and checking the subsequent lines), or assert the expected occurrence count (pack + restore) is 2 for isolated=true.
| // Restore step should include isolated: true because frontmatter says so | |
| assert.Contains(t, lockContent, "isolated: 'true'", | |
| "Lock file restore step should include isolated flag") | |
| // Both pack and restore steps should include isolated: true because frontmatter says so | |
| isolatedCount := strings.Count(lockContent, "isolated: 'true'") | |
| assert.Equal(t, 2, isolatedCount, | |
| "Lock file should include isolated flag in both pack and restore steps") |
Reimplements the APM dependency resolution improvements from PR #20385.
APM dependencies were previously resolved at agent-job runtime — slow, network-dependent, and non-deterministic across retries. This moves resolution to the activation job (pack) and unpacking to the agent job (restore) via a separate
apmartifact.New compilation output
Activation job — pack step + artifact upload:
Agent job — download + restore:
Changes
APMDependenciesInfo— addsIsolated boolfieldextractAPMDependenciesFromFrontmatter— adds object format support alongside existing array format:apm_dependencies.go— replacesGenerateAPMDependenciesStepwithGenerateAPMPackStepandGenerateAPMRestoreStepWorkflowExecutorinterface — addsGetAPMTarget() stringmethod;BaseEnginedefaults to"all",CopilotEnginereturns"copilot",ClaudeEnginereturns"claude"; the APM target is inferred from the engine at compile timecompiler_activation_job.go— emits pack step (usingengine.GetAPMTarget()) and separateapmartifact upload after prompt generationcompiler_yaml_main_job.go— replaces old install step with artifact download + restorefrontmatter.md— documents new object format and pack/unpack behaviorsmoke-claude.md— addsmicrosoft/apm-sample-packageas an APM dependency to exercise the pack/restore path in the smoke test workflowChangeset
apmartifact so agent jobs use a deterministic dependency bundle.✨ PR Review Safe Output Test - Run 22970861289