Skip to content

feat: use upload-artifact with skip-archive:true for image artifact URLs in markdown#25923

Merged
pelikhan merged 3 commits intomainfrom
copilot/use-upload-artifact-skip-archive
Apr 12, 2026
Merged

feat: use upload-artifact with skip-archive:true for image artifact URLs in markdown#25923
pelikhan merged 3 commits intomainfrom
copilot/use-upload-artifact-skip-archive

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

Summary

Pass skipArchive option to @actions/artifact client and expose artifact URL outputs. This enables workflows to upload images without zip archiving and use the resulting artifact URLs to render images inline in generated markdown (issues, PR comments, discussions, step summaries).

Changes

actions/setup/js/upload_artifact.cjs

  • Pass skipArchive: true to client.uploadArtifact() options when skip-archive is enabled in workflow config (previously parsed but never used)
  • Construct artifact URL from the upload result's artifact ID and GitHub context env vars (GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID)
  • Output slot_N_artifact_id and slot_N_artifact_url step outputs for downstream consumption
  • Return artifactId and artifactUrl in the handler result object

actions/setup/js/upload_artifact.test.cjs

  • Add test that skipArchive option is passed to artifact client when enabled
  • Add test that skipArchive is not passed when disabled
  • Add tests for artifact URL construction with GitHub env vars
  • Add test for missing env vars (empty URL)
  • Add test for staged mode (no artifact ID/URL outputs)

.github/workflows/shared/python-dataviz.md

  • Add upload-artifact safe-output config with skip-archive: true, max-uploads: 5, and allowed image paths (*.png, *.jpg, *.svg)
  • Rewrite "Including Images in Reports" section with two approaches:
    1. Artifact URLs (recommended for inline images) - using upload_artifact tool with skip-archive: true
    2. Upload Asset (alternative for public URLs) - existing upload asset approach

.github/workflows/shared/safe-output-upload-artifact.md

  • Add documentation section on rendering images from artifacts using artifact URLs
  • Document the artifact URL format and authentication requirements

How artifact image rendering works

  1. Workflow config sets skip-archive: true in safe-outputs.upload-artifact
  2. Agent generates a chart/image and calls upload_artifact tool with the image path
  3. The handler uploads the single file via @actions/artifact with skipArchive: true (no zip)
  4. The artifact ID from the upload response is used to construct a GitHub artifact URL
  5. The URL is output as slot_N_artifact_url and can be used in markdown: ![Chart](URL)

Testing

  • All 35 JS tests pass (including 7 new tests)
  • All Go workflow tests pass
  • All 187 workflows compile successfully
  • Linting passes

Copilot AI and others added 2 commits April 12, 2026 17:29
- Pass skipArchive option to @actions/artifact client.uploadArtifact()
  when skip-archive is enabled in workflow config
- Construct artifact URL from upload result ID and GitHub context env vars
  (GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID)
- Output slot_N_artifact_id and slot_N_artifact_url step outputs
- Return artifactId and artifactUrl in handler result
- Add upload-artifact safe-output with skip-archive:true to python-dataviz
  shared workflow for chart image uploads
- Update safe-output-upload-artifact.md with artifact URL rendering docs
- Add tests for skipArchive passthrough, artifact URL construction, and
  staged mode behavior

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/59d512e7-d814-41cc-8eb5-90f3694512d2

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan April 12, 2026 17:55
@pelikhan pelikhan marked this pull request as ready for review April 12, 2026 17:55
Copilot AI review requested due to automatic review settings April 12, 2026 17:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables image-friendly artifact uploads by wiring the previously-parsed skip-archive configuration through to @actions/artifact, and exposing per-upload artifact IDs/URLs for use in generated markdown.

Changes:

  • Passes skipArchive: true to @actions/artifact when skip-archive is enabled, and surfaces artifactId/artifactUrl in handler results and step outputs.
  • Adds unit tests covering skipArchive option behavior, artifact URL construction, and staged mode output suppression.
  • Updates shared docs and multiple compiled workflow lockfiles to support upload_artifact staging + mounting.
Show a summary per file
File Description
actions/setup/js/upload_artifact.cjs Implements skipArchive upload option and emits artifact ID/URL outputs.
actions/setup/js/upload_artifact.test.cjs Adds tests for skipArchive option passing and artifact URL output behavior.
.github/workflows/shared/safe-output-upload-artifact.md Documents artifact URL usage for rendering images in markdown.
.github/workflows/shared/python-dataviz.md Adds upload-artifact safe-output config and updated guidance for embedding images.
.github/workflows/weekly-issue-summary.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/stale-repo-identifier.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/python-data-charts.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/org-health-report.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/github-mcp-structural-analysis.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/daily-repo-chronicle.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/daily-news.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/daily-issues-report.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/daily-integrity-analysis.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/daily-code-metrics.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/copilot-token-audit.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/copilot-session-insights.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/copilot-pr-nlp-analysis.lock.yml Adds upload_artifact safe-output config and staging artifact plumbing.
.github/workflows/agentics-maintenance.yml Changes validate_workflows runner from ubuntu-slim to ubuntu-latest.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 18/18 changed files
  • Comments generated: 3

Comment thread actions/setup/js/upload_artifact.cjs Outdated
const repository = process.env.GITHUB_REPOSITORY || "";
const runId = process.env.GITHUB_RUN_ID || "";
if (repository && runId) {
artifactUrl = `${serverUrl}/${repository}/actions/runs/${runId}/artifacts/${artifactId}`;
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact URL construction uses string concatenation with GITHUB_SERVER_URL and can produce a malformed URL with a double // if GITHUB_SERVER_URL ends with a trailing slash (this repo already normalizes it elsewhere, e.g. actions/setup/js/git_helpers.cjs:30). Consider normalizing serverUrl (trim trailing /) or using new URL() to build the URL safely.

Suggested change
artifactUrl = `${serverUrl}/${repository}/actions/runs/${runId}/artifacts/${artifactId}`;
artifactUrl = new URL(`/${repository}/actions/runs/${runId}/artifacts/${artifactId}`, serverUrl).toString();

Copilot uses AI. Check for mistakes.
Comment on lines 22 to +25
The tool returns a temporary opaque artifact ID (`aw_*`) that can be resolved to
a download URL by an authorised downstream step.
a download URL by an authorised downstream step. When `skip-archive` is enabled, the
tool also outputs `slot_N_artifact_url` containing a direct link to the uploaded artifact,
which can be used to render images inline in markdown.
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc says slot_N_artifact_url is only output when skip-archive is enabled, but the handler currently emits slot_N_artifact_url whenever it can construct one (independent of skip-archive). Please either update the documentation to match the implementation, or gate the output/URL construction behind skip-archive: true if that’s the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines 282 to 286
validate_workflows:
if: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.operation == 'validate' && !github.event.repository.fork }}
runs-on: ubuntu-slim
runs-on: ubuntu-latest
permissions:
contents: read
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_workflows switches from runs-on: ubuntu-slim to ubuntu-latest while the rest of this workflow (and most workflows in the repo) still use ubuntu-slim. If ubuntu-slim is a deliberate runner label (custom image) this change could alter tool availability/costs and looks unrelated to the PR’s stated goal. Please confirm this is intentional and, if so, document the rationale in the PR description (or keep the runner consistent).

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot mentioned this pull request Apr 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 78/100

⚠️ Acceptable, with suggestions

Metric Value
New/modified tests analyzed 5
✅ Design tests (behavioral contracts) 5 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 3 (60%)
Duplicate test clusters 0
Test inflation detected YES (2.13:1 — upload_artifact.test.cjs)
🚨 Coding-guideline violations None

Test Classification Details

All 5 tests (click to expand)
Test File Classification Issues Detected
passes skipArchive option to artifact client when skip-archive=true actions/setup/js/upload_artifact.test.cjs:229 ✅ Design Happy-path only; no error branch
does not pass skipArchive option when skip-archive is false actions/setup/js/upload_artifact.test.cjs:239 ✅ Design Negative/false-branch case ✔
outputs artifact_id and artifact_url when upload succeeds actions/setup/js/upload_artifact.test.cjs:262 ✅ Design Happy-path only; verifies URL format + core.setOutput
does not output artifact_url when env vars are missing actions/setup/js/upload_artifact.test.cjs:278 ✅ Design Edge case: missing env vars ✔
does not output artifact_id or artifact_url in staged mode actions/setup/js/upload_artifact.test.cjs:293 ✅ Design Edge case: staged mode security invariant ✔

Test Inflation Warning

actions/setup/js/upload_artifact.test.cjs added 66 lines while upload_artifact.cjs added 31 lines → ratio 2.13:1 (threshold: 2:1).

This is a minor breach and the tests are high-quality, so no action is required — just worth noting.


Suggestions (non-blocking)

passes skipArchive option to artifact client when skip-archive=true and does not pass skipArchive option when skip-archive is false

These two tests correctly verify that skipArchive is forwarded to the artifact SDK call. Since this is adapter code, checking the call arguments is the right approach. However, only the true/false happy paths are covered — consider adding a test for the documented restriction that skipArchive cannot coexist with multiple files (which is already tested in the existing validation suite, so this is just a pointer, not a gap).

outputs artifact_id and artifact_url when upload succeeds

Solid test. One minor enhancement: consider adding a case where uploadResult.id is null/undefined (SDK returns no ID) to assert the URL stays empty and no slot_X_artifact_id output is emitted.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 tests
  • 🟨 JavaScript (*.test.cjs, *.test.js): 5 tests (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All 5 new tests verify behavioral contracts: option forwarding to the SDK, URL construction from GitHub env vars, and staged-mode suppression of outputs.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 603.3K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 78/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 5 new tests verify behavioral contracts for the skipArchive option forwarding and artifact URL output feature. Minor test inflation noted (2.13:1) but non-blocking.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot Review all comments

…n, fix docs, revert unrelated runner change

- Use new URL() instead of string concatenation to avoid double-slash
  when GITHUB_SERVER_URL has trailing slash
- Update safe-output-upload-artifact.md docs to clarify artifact URL is
  output on all successful uploads (not only when skip-archive is enabled)
- Revert unrelated ubuntu-slim → ubuntu-latest change in agentics-maintenance.yml

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/753ddb30-9510-467d-b5ee-446cd3b5065b

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot Review all comments

Addressed all three review comments in f40dcf9:

  1. URL construction — Replaced string concatenation with new URL() to safely handle trailing slashes in GITHUB_SERVER_URL.
  2. Documentation mismatch — Updated safe-output-upload-artifact.md to clarify that slot_N_artifact_url is output on all successful uploads, not only when skip-archive is enabled.
  3. Unrelated runner change — Reverted the ubuntu-slimubuntu-latest change in agentics-maintenance.yml.

@pelikhan pelikhan merged commit 0e58049 into main Apr 12, 2026
@pelikhan pelikhan deleted the copilot/use-upload-artifact-skip-archive branch April 12, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants