Skip to content

Fix phantom asset failures: align safe-outputs staging path with RUNNER_TEMP#39900

Merged
pelikhan merged 6 commits into
mainfrom
copilot/aw-failures-fix-upload-assets
Jun 17, 2026
Merged

Fix phantom asset failures: align safe-outputs staging path with RUNNER_TEMP#39900
pelikhan merged 6 commits into
mainfrom
copilot/aw-failures-fix-upload-assets

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Asset-producing workflows were completing the agent job successfully but failing upload_assets with ERR_SYSTEM: Asset file not found. The uploadAssetHandler wrote files to /tmp/gh-aw/safeoutputs/assets — the stdio container's private /tmp, not the host filesystem — so files were lost on container exit and the safe-outputs-assets artifact was never produced.

Changes

Path fix (root cause)

  • safe_outputs_handlers.cjs: assetsDir now uses process.env.RUNNER_TEMP || "/tmp" base, matching the working uploadArtifactHandler pattern. Assets land in ${RUNNER_TEMP}/gh-aw/safeoutputs/assets which is bind-mounted to the host.
  • upload_assets.cjs: Read path is now aligned to the same ${RUNNER_TEMP}/gh-aw/safeoutputs/assets location.
// Before
const assetsDir = "/tmp/gh-aw/safeoutputs/assets";

// After
const assetsDir = path.join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "safeoutputs", "assets");

Gateway mount

  • mcp_gateway_config.go: Added ${RUNNER_TEMP}/gh-aw/safeoutputs:${RUNNER_TEMP}/gh-aw/safeoutputs:rw to default MCP gateway mounts.
  • mcp_gateway_config.go: When upload-asset is configured, this safeoutputs mount is now ensured even if custom MCP mounts are already set.

Setup & artifact upload alignment

  • mcp_setup_generator.go: Creates ${RUNNER_TEMP}/gh-aw/safeoutputs/assets at setup time when UploadAssets is configured.
  • publish_assets.go: Fixed artifact upload path from /tmp/gh-aw/safeoutputs/assets/${{ runner.temp }}/gh-aw/safeoutputs/assets/.

Missing asset behavior

  • upload_assets.cjs: Missing files are still warning+skip for partial cases, but now fail explicitly when all declared assets are missing (no assets published) to avoid silent zero-asset success.

Test coverage

  • upload_assets.test.cjs: Added focused coverage for mixed present/missing assets under RUNNER_TEMP to verify warn-and-skip behavior and successful partial publication.
  • mcp_gateway_config_test.go / mcp_gateway_mounts_test.go: Added coverage for safeoutputs mount behavior with custom mounts + upload-asset.

pr-sous-chef: please refresh the branch and keep the fix moving forward. Run: https://github.com/github/gh-aw/actions/runs/27726614160

Generated by 👨‍🍳 PR Sous Chef ·



✨ PR Review Safe Output Test - Run 27726899549

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude ·


Changeset\n\n- Type: patch\n- Description: Fixed safe-outputs asset staging so uploaded assets use RUNNER_TEMP and can be published reliably.

Generated by 📋 Changeset Generator for issue #39900 ·

@github-actions

Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — thanks for picking up the upload_assets phantom-asset bug! The problem statement in the body is clear and the three-pronged remediation plan (staging path alignment, emit-time file validation, fail-soft upload) is well-scoped.

This is a WIP draft with no code yet — a couple of things to keep in mind as the implementation lands:

  • Add tests — the two new behaviours (emit-time file-existence validation and fail-soft upload_assets) will each need coverage. Key cases: a unit test that rejects an upload_asset safe-output pointing at a non-existent file, and an integration/e2e test confirming upload_assets warns-and-continues when some declared assets are absent rather than failing the whole job.
  • Verify path canonicalisation — the root cause identifies a mismatch between .../.gh-aw-assets/<file> (agent output) and /tmp/gh-aw/safeoutputs/assets/ (upload pipeline read path). Make sure the fix picks one canonical path and no other code still references the old location after the change.

If you'd like a head start on the test scaffolding, here's a ready-to-use prompt:

Implement and test the upload_assets phantom-asset fix described in github/gh-aw#39900.

Required changes:
1. Emit-time validation: when an `upload_asset` safe-output item is emitted, check that the referenced file exists on disk. If it does not, return a clear agent-facing error (not ERR_SYSTEM) and skip/reject the item.
2. Fail-soft upload_assets: in the upload_assets job, warn and skip missing asset files rather than aborting the whole job; exit 0 if at least one asset was uploaded successfully.
3. Path alignment: ensure staged asset files are written to `/tmp/gh-aw/safeoutputs/assets/` (or update the upload pipeline to read the agent's actual output path) so the `safe-outputs-assets` artifact is consistently produced by the agent job.

Add tests covering:
- Emitting an `upload_asset` for a file that does not exist → validation error returned at emit time, no ERR_SYSTEM in a later job.
- `upload_assets` with a mix of present and missing declared files → missing files produce a warning, present files are uploaded, job exits 0.
- Happy path: all declared assets are staged correctly → all uploaded, no warnings.

Generated by ✅ Contribution Check ·

Copilot AI and others added 2 commits June 17, 2026 21:45
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…th with RUNNER_TEMP

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix upload_assets job failure due to missing staged PNGs Fix phantom asset failures: align safe-outputs staging path with RUNNER_TEMP Jun 17, 2026
Copilot AI requested a review from pelikhan June 17, 2026 22:02
@pelikhan pelikhan marked this pull request as ready for review June 17, 2026 22:11
Copilot AI review requested due to automatic review settings June 17, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes “phantom” safe-outputs asset upload failures by moving asset staging under RUNNER_TEMP (host-visible), ensuring the MCP gateway container can see that directory via default mounts, and aligning the generated artifact upload path with ${{ runner.temp }}. Also updates compiled workflow outputs (goldens + .lock.yml) to reflect the new mount, and makes the asset upload step fail-soft when a declared asset file is missing.

Changes:

  • Stage safe-outputs assets under ${RUNNER_TEMP}/gh-aw/safeoutputs/assets and create the directory during setup when UploadAssets is enabled.
  • Add ${RUNNER_TEMP}/gh-aw/safeoutputs to default MCP gateway mounts and update tests/goldens accordingly.
  • Make upload_assets skip missing files with a warning instead of failing the entire job.
Show a summary per file
File Description
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden Updates expected MCP gateway docker command to include safeoutputs mount.
pkg/workflow/publish_assets.go Aligns safe-outputs assets artifact upload path with ${{ runner.temp }}.
pkg/workflow/mcp_setup_generator.go Creates RUNNER_TEMP safeoutputs assets directory when UploadAssets is enabled.
pkg/workflow/mcp_gateway_mounts_test.go Updates expected default mounts to include safeoutputs mount.
pkg/workflow/mcp_gateway_config.go Adds RUNNER_TEMP safeoutputs mount to default MCP gateway mounts.
pkg/workflow/mcp_gateway_config_test.go Updates default-mount count/contents assertions to include safeoutputs mount.
actions/setup/js/upload_assets.cjs Changes missing asset handling from hard-fail to warning + skip.
actions/setup/js/safe_outputs_handlers.test.cjs Adds coverage asserting asset staging uses RUNNER_TEMP base path.
actions/setup/js/safe_outputs_handlers.cjs Stages assets under `process.env.RUNNER_TEMP
actions/setup-cli/install.sh Updates header comment (currently incorrect/self-referential).
.github/workflows/workflow-normalizer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/workflow-health-manager.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/workflow-generator.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/video-analyzer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/update-astro.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/ubuntu-image-analyzer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/tidy.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/test-workflow.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/test-quality-sentinel.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/test-project-url-default.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/test-dispatcher.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/terminal-stylist.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/technical-doc-writer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/super-linter.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/stale-pr-cleanup.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/spec-librarian.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/spec-extractor.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/smoke-test-tools.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/smoke-project.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/smoke-pi.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/smoke-copilot-sdk.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/smoke-ci.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/security-review.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/security-compliance.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/ruflo-backed-task.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/research.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/repo-tree-map.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/release.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/refiner.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/refactoring-cadence.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/q.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/pr-description-caveman.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/plan.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/pdf-summary.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/outcome-collector.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/objective-impact-report.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/metrics-collector.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/mergefest.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/linter-miner.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/lint-monster.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/layout-spec-maintainer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/jsweep.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/gpclean.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/firewall.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/firewall-escape.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/example-permissions-warning.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/example-failure-category-filter.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/dev-hawk.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/designer-drift-audit.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/dependabot-worker.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/delight.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-windows-terminal-integration-builder.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-team-status.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-safeoutputs-git-simulator.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-max-ai-credits-test.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-credit-limit-test.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/daily-byok-ollama-test.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/craft.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/copilot-opt.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/copilot-centralization-optimizer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/copilot-centralization-drilldown.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/contribution-check.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/code-simplifier.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/ci-coach.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/brave.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/bot-detection.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/artifacts-summary.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/architecture-guardian.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/archie.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/agentic-token-optimizer.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.
.github/workflows/ace-editor.lock.yml Regenerates workflow to include safeoutputs mount in gateway docker command.

Copilot's findings

Tip

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

  • Files reviewed: 267/267 changed files
  • Comments generated: 2

Comment thread actions/setup-cli/install.sh Outdated
set +o histexpand

# Kept in sync with ../../install-gh-aw.sh — edit that file, then copy here.
# Kept in sync with actions/setup-cli/install.sh — edit this file, then copy to that path.
Comment on lines 125 to +129
// Check if file exists in artifacts
const assetSourcePath = path.join("/tmp/gh-aw/safeoutputs/assets", fileName);
if (!fs.existsSync(assetSourcePath)) {
core.setFailed(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath}`);
return;
core.warning(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath} — skipping`);
continue;
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #39900 does not have the 'implementation' label and has 17 new lines of code in business logic directories (≤100 threshold).

@github-actions github-actions Bot mentioned this pull request Jun 17, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /diagnose and /tdd — requesting changes on one blocking correctness issue.

📋 Key Themes & Findings

Blocking issue

  • Incomplete path fix in upload_assets.cjs: The write path was correctly updated to ${RUNNER_TEMP}/gh-aw/safeoutputs/assets, but the read path on line 126 remains hardcoded to /tmp/gh-aw/safeoutputs/assets. When RUNNER_TEMP ≠ /tmp (the normal case this PR was meant to fix), every asset will hit the new warning+continue branch and be silently skipped — the failure mode changes from a loud job failure to a silent data loss.

Secondary issues

  • Soft-fail masking: The warning+continue change is a useful resilience addition in general, but here it makes the residual bug invisible in CI output. Worth keeping once the read path is fixed.
  • Mount/mkdir mismatch: The new safeoutputs bind-mount is applied to all workflows as a default, but the mkdir -p guard only runs for UploadAssets workflows. On Docker hosts that don't auto-create missing bind-mount sources this could cause container-start failures in unrelated workflows.
  • Read-path test gap: The new RUNNER_TEMP regression test covers the handler's write path nicely, but upload_assets.test.cjs still writes directly to /tmp/gh-aw/safeoutputs/assets, so the read-path mismatch has no failing test to catch it.

Positive highlights

  • ✅ Three-layer coordinated fix (write path + gateway bind-mount + artifact upload path) shows good systems thinking
  • ✅ The /diagnose discipline of identifying the container isolation root cause is solid
  • mcp_setup_generator.go creating the directory proactively is the right pattern
  • ✅ New handler regression test is well-structured with proper beforeEach/afterEach cleanup

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

Comment thread actions/setup/js/upload_assets.cjs Outdated
Comment on lines +126 to +128
const assetSourcePath = path.join("/tmp/gh-aw/safeoutputs/assets", fileName);
if (!fs.existsSync(assetSourcePath)) {
core.setFailed(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath}`);
return;
core.warning(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath} — skipping`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnose] The read path on line 126 is still hardcoded to /tmp/gh-aw/safeoutputs/assets — the same private-container path that was the original root cause. The handler now writes to ${RUNNER_TEMP}/gh-aw/safeoutputs/assets, but this reader looks in the wrong place, so when RUNNER_TEMP ≠ /tmp every asset silently hits this warning+continue branch and is skipped.

💡 Suggested fix

Apply the same RUNNER_TEMP pattern used in safe_outputs_handlers.cjs:

const assetsDir = path.join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "safeoutputs", "assets");
const assetSourcePath = path.join(assetsDir, fileName);

Also update upload_assets.test.cjs to set process.env.RUNNER_TEMP in beforeEach so the mismatch can't silently regress.

Comment on lines +287 to +298
it("should stage asset file under RUNNER_TEMP not /tmp", () => {
process.env.GH_AW_ASSETS_BRANCH = "test-branch";

const testFile = path.join(testWorkspaceDir, "chart.png");
fs.writeFileSync(testFile, "chart content");

handlers.uploadAssetHandler({ path: testFile });

// File must be staged under RUNNER_TEMP, not hardcoded /tmp
const expectedDir = path.join(testRunnerTemp, "gh-aw", "safeoutputs", "assets");
expect(fs.existsSync(path.join(expectedDir, "chart.png"))).toBe(true);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] ✅ Solid regression test for the write path — this is exactly the kind of test that would have caught the original bug. However, the complementary read-path assertion is still missing: nothing verifies that upload_assets.cjs reads from ${RUNNER_TEMP}/gh-aw/safeoutputs/assets rather than the hardcoded /tmp path.

💡 What to add

Add a RUNNER_TEMP-aware test in upload_assets.test.cjs (similar to the beforeEach/afterEach pattern added here) that:

  1. Sets RUNNER_TEMP to a temp directory
  2. Places a fixture asset under ${RUNNER_TEMP}/gh-aw/safeoutputs/assets/
  3. Asserts that upload_assets finds and processes it — confirming both paths are aligned end-to-end

Comment thread pkg/workflow/mcp_gateway_config.go Outdated
"/opt:/opt:ro",
"/tmp:/tmp:rw",
"${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw",
"${RUNNER_TEMP}/gh-aw/safeoutputs:${RUNNER_TEMP}/gh-aw/safeoutputs:rw",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnose] The safeoutputs bind-mount is added to the default mounts list (applied to every workflow), but mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs/assets" in mcp_setup_generator.go only runs when UploadAssets != nil. On Docker versions that do not auto-create missing source paths, any workflow that does not use upload_assets will get a bind mount pointing at a non-existent directory — which may cause the container start to fail.

💡 Two safe options

Option A — always pre-create the directory at setup time (simplest):

mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs"

in the unconditional setup block, regardless of UploadAssets.

Option B — make the mount conditional on UploadAssets being configured, matching the pattern used for other feature-specific resources.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

⚠️ Test Quality Score: 70/100 — Acceptable

Analyzed 3 test(s): 3 design, 0 implementation, 0 guideline violation(s). Test inflation detected in 1 file (32 lines added in test vs 3 in production).

📊 Metrics & Test Classification (3 tests analyzed)
Metric Value
New/modified tests analyzed 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (33%)
Duplicate test clusters 0
Test inflation detected YES — safe_outputs_handlers.test.cjs: 32 lines added vs 3 in production (10.67:1)
🚨 Coding-guideline violations 0
Test File Classification Issues Detected
should stage asset file under RUNNER_TEMP not /tmp safe_outputs_handlers.test.cjs ✅ Design No negative assertion; happy-path only
TestEnsureDefaultMCPGatewayConfig (modified) mcp_gateway_config_test.go:33 ✅ Design
TestMCPGatewayDefaultMounts (modified) mcp_gateway_mounts_test.go:28 ✅ Design

Go: 2 (*_test.go); JavaScript: 1 (*.test.cjs). Other languages detected but not scored.

Score breakdown:

  • Behavioral coverage: 40/40 (3/3 design tests)
  • Error/edge case coverage: 10/30 (1/3 tests — TestEnsureDefaultMCPGatewayConfig as table-driven)
  • Duplication penalty: 0/20 deducted
  • Proportional growth: 0/10 (inflation in safe_outputs_handlers.test.cjs)
⚠️ Flagged Tests (2 issues)

should stage asset file under RUNNER_TEMP not /tmp (actions/setup/js/safe_outputs_handlers.test.cjs) — Two gaps worth addressing:

  1. Missing negative assertion. The test name promises "not /tmp" but only asserts the file exists under RUNNER_TEMP. A bug that writes to both paths would silently pass. Consider adding:

    expect(fs.existsSync("/tmp/gh-aw/safeoutputs/assets/chart.png")).toBe(false);
  2. Happy-path only. No coverage for RUNNER_TEMP being unset or the handler failing gracefully. An error-path case (e.g., delete process.env.RUNNER_TEMP before calling the handler) would protect the fallback behavior.

Test inflation — safe_outputs_handlers.test.cjs — 32 lines added vs 3 lines in production (safe_outputs_handlers.cjs). The bulk of new lines are beforeEach/afterEach setup providing solid per-test isolation (unique RUNNER_TEMP per test, proper cleanup) which is legitimate engineering. However, the 10.67:1 ratio exceeds the 2:1 threshold and triggers the proportional growth deduction.

Verdict

Check passed. 0% implementation tests (threshold: 30%). All 3 modified/added tests verify observable behavior — filesystem routing to RUNNER_TEMP and MCP gateway mount configuration. No coding-guideline violations detected (build tags ✅, no mock libraries ✅, assertion messages present ✅).

References:

🧪 Test quality analysis by Test Quality Sentinel ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 70/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two issues need fixing before merge

The core path fix is correct: staging assets under ${RUNNER_TEMP}/gh-aw/safeoutputs/assets/ (writer) and uploading the artifact from ${{ runner.temp }}/gh-aw/safeoutputs/assets/ (upload-artifact step) is the right approach. The upload_assets job pipeline (download-artifact → upload_assets.cjs) is internally consistent with its own /tmp path and is not broken.

### Blocking concerns

1. Conditional mount gap (mcp_gateway_config.go): The ${RUNNER_TEMP}/gh-aw/safeoutputs bind-mount is only injected into the defaults block, which is skipped entirely when a workflow defines any custom mounts. Those workflows are still exposed to the original staging bug — files written to a container path that is not bind-mounted to the host are lost on container exit. See inline comment.

2. Zero-signal total failure (upload_assets.cjs): The setFailedwarning+continue downgrade, combined with continue-on-error: true on the download step and if-no-files-found: ignore on the artifact upload step, means a completely empty asset delivery (e.g. triggered by the conditional-mount gap above) produces no CI failure and no summary annotation. See inline comment.

These two interact: the conditional-mount gap creates the failure scenario; the fail-soft change silences it. Both should be addressed together.

🔎 Code quality review by PR Code Quality Reviewer

Comment thread pkg/workflow/mcp_gateway_config.go Outdated
"/opt:/opt:ro",
"/tmp:/tmp:rw",
"${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw",
"${RUNNER_TEMP}/gh-aw/safeoutputs:${RUNNER_TEMP}/gh-aw/safeoutputs:rw",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New safeoutputs mount only added when Mounts is empty — custom-mount workflows are not covered by this fix.

The if len(workflowData.SandboxConfig.MCP.Mounts) == 0 guard (line 90) means the entire default-mounts block is skipped when a workflow declares any explicit mount. Those workflows will not receive the ${RUNNER_TEMP}/gh-aw/safeoutputs bind-mount, so the MCP container still writes assets into its private overlay filesystem and loses them on exit — the original staging bug is unfixed for them.

💡 Suggested fix

Ensure the safeoutputs mount is always present when UploadAssets is configured, regardless of whether other mounts were customised:

const safeOutputsMount = "${RUNNER_TEMP}/gh-aw/safeoutputs:${RUNNER_TEMP}/gh-aw/safeoutputs:rw"
if workflowData.SafeOutputs != nil && workflowData.SafeOutputs.UploadAssets != nil {
    hasSafeOutputsMount := slices.Contains(workflowData.SandboxConfig.MCP.Mounts, safeOutputsMount)
    if !hasSafeOutputsMount {
        workflowData.SandboxConfig.MCP.Mounts = append(workflowData.SandboxConfig.MCP.Mounts, safeOutputsMount)
    }
}

Without this, a workflow that opts in to any custom mounts will silently drop all assets after this PR — worse than the pre-fix behavior, which at least produced an explicit setFailed.

if (!fs.existsSync(assetSourcePath)) {
core.setFailed(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath}`);
return;
core.warning(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath} — skipping`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning+continue creates a zero-signal failure path when staging is broken.

The error is downgraded from a job-failing setFailed to a soft warning. Combined with continue-on-error: true on the download-artifact pre-step and if-no-files-found: ignore on the upload-artifact step in the agent job, a completely empty asset pipeline now produces: no red checks, no annotations summary, and no explicit count of skipped assets. The job simply succeeds with a few warning lines buried in the log.

This is particularly risky alongside the conditional-mount gap in mcp_gateway_config.go: workflows with custom mounts won't have the ${RUNNER_TEMP}/gh-aw/safeoutputs bind-mount, so the agent's staged files are lost before the artifact is uploaded. That produces an empty artifact, an empty download, and then this warn-and-skip path — end result is all declared assets silently dropped with job exit 0.

💡 Suggested improvement

Track how many assets were skipped and surface a summary, or restore the failure for the all-skipped case:

let skippedCount = 0;
// ... inside the loop:
core.warning(`${ERR_SYSTEM}: Asset file not found: ${assetSourcePath} — skipping`);
skippedCount++;
continue;
// ... after loop:
if (skippedCount > 0 && skippedCount === totalAssets) {
  core.setFailed(`All ${skippedCount} declared assets were missing; no assets published.`);
}

This preserves the graceful-skip behavior for phantom declarations while restoring an explicit failure when the entire upload is empty.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

Copilot AI and others added 3 commits June 17, 2026 23:39
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot run pr-finisher skill

Completed a pr-finisher pass and pushed fixes in 15667f9 and 84bfdb0:

  • aligned upload_assets.cjs read path to ${RUNNER_TEMP}/gh-aw/safeoutputs/assets
  • added focused vitest coverage for warn-and-skip with mixed present/missing assets under RUNNER_TEMP
  • ensured safeoutputs mount is added when upload-asset is enabled even with custom MCP mounts
  • corrected setup-cli header source comment sync issue
  • added explicit all-missing asset failure signal, while preserving partial-upload success

Local validation passed (make fmt, make lint, make test-unit, make test, make recompile, and make agent-report-progress).

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot - AOAI (Entra) is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot - AOAI (apikey) is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results\n- GitHub MCP Testing: ✅\n- Web Fetch Testing: ✅\n- File Writing Testing: ✅\n- Bash Tool Testing: ✅\n- Build gh-aw: ❌\n\nOverall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

Smoke Gemini — Powered by Gemini ·

@github-actions

Copy link
Copy Markdown
Contributor

Agent Container Tool Check

Tool Status Version
bash 5.2.21
sh available
git 2.54.0
jq 1.7
yq v4.53.3
curl 8.5.0
gh 2.94.0
node v22.22.3
python3 3.11.15 (PyPy)
go 1.24.13
java 21.0.11
dotnet 10.0.301

Result: 12/12 tools available ✅ — PASS

🔧 Tool validation by Agent Container Smoke Test ·

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Fog on the window
Build lights hum through the long night
Checks settle like dust

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex ·

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test results: 1. ✅; 2. ❌; 3. ❌; 4. ❌; 5. ❌; 6. ❌; 7. ❌; 8. ❌; 9. skipped; 10. ❌; 11. ❌; 12. ❌; 13. ❌; 14. ❌; 15. ❌; Overall: FAIL; @github-actions[bot]

📰 BREAKING: Report filed by Smoke Copilot - AOAI (apikey) ·

@github-actions

Copy link
Copy Markdown
Contributor

💨 Smoke Test: Claude — Run 27726899549

Core #1-12: ✅ all passed
PR Review #13-18: ✅ all passed | #19: ⚠️ skipped

Overall: PARTIAL (all executed tests passed; #19 no safe PR to close)
Run

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💥 Automated smoke test review - all systems nominal!

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude

Comments that could not be inline-anchored

.github/workflows/ab-testing-advisor.lock.yml:723

Smoke test review: confirmed the RUNNER_TEMP safeoutputs mount was added here. Consistent with the staging path alignment.

.github/workflows/ace-editor.lock.yml:723

Smoke test review: same RUNNER_TEMP mount change applied here — good for consistency across compiled workflows.

@pelikhan pelikhan merged commit b79a2d5 into main Jun 17, 2026
223 of 225 checks passed
@pelikhan pelikhan deleted the copilot/aw-failures-fix-upload-assets branch June 17, 2026 23:56
@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test 27726926051:
✅ 1,2,5,6,8,10
❌ 3,4,7,9,11-15
Overall: FAIL
@app/copilot-swe-agent @pelikhan @Copilot

📰 BREAKING: Report filed by Smoke Copilot - AOAI (Entra) ·

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Browsers greet the dawn
Green builds hum through careful wires
Checkmarks bloom at dusk

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 5 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Smoke run 27726924811 completed; see the PR summary comment for the concise result matrix.

Warning

Firewall blocked 5 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants