Skip to content

Commit 5289cc4

Browse files
CopilotCopilot
authored
fix: include workflow ID in experiment artifact cache keys and artifact name (#29747)
1 parent 395cfcc commit 5289cc4

8 files changed

Lines changed: 121 additions & 70 deletions

File tree

‎.github/workflows/daily-community-attribution.lock.yml‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/smoke-copilot.lock.yml‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/workflow/compiler_experiments.go‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func extractIntSlice(raw any) []int {
242242
// 2. Pick variants – pick_experiment.cjs (reads/writes state.json, sets step outputs,
243243
// writes a Markdown step summary); outputs: one per experiment (e.g. "caveman=yes") + "experiments" JSON blob
244244
// 3. Save experiment cache – actions/cache/save keyed by workflow ID
245-
// 4. Upload experiment artifact – actions/upload-artifact named "experiment"
245+
// 4. Upload experiment artifact – actions/upload-artifact named "{workflowID}-experiment"
246246
func (c *Compiler) generateExperimentSteps(data *WorkflowData) []string {
247247
if len(data.Experiments) == 0 {
248248
return nil
@@ -251,8 +251,11 @@ func (c *Compiler) generateExperimentSteps(data *WorkflowData) []string {
251251
experimentNames := sortedExperimentNames(data.Experiments)
252252
experimentsLog.Printf("Generating experiment steps for %d experiment(s): %v", len(experimentNames), experimentNames)
253253

254-
cacheKey := "experiments-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}"
255-
restoreKey := "experiments-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-"
254+
// Use the literal sanitized workflow ID in the cache key so it is correct in the
255+
// activation job, which does not have GH_AW_WORKFLOW_ID_SANITIZED in its environment.
256+
sanitizedID := SanitizeWorkflowIDForCacheKey(data.WorkflowID)
257+
cacheKey := fmt.Sprintf("experiments-%s-${{ github.run_id }}", sanitizedID)
258+
restoreKey := fmt.Sprintf("experiments-%s-", sanitizedID)
256259

257260
var steps []string
258261

@@ -298,7 +301,10 @@ func (c *Compiler) generateExperimentSteps(data *WorkflowData) []string {
298301
)
299302

300303
// ── Step 4: Upload experiment artifact ────────────────────────────────────
301-
experimentArtifactName := artifactPrefixExprForActivationJob(data) + constants.ExperimentArtifactName
304+
// For workflow_call the artifact prefix expression is prepended at runtime.
305+
// For regular workflows the sanitized workflow ID is used as a prefix so the
306+
// artifact name uniquely identifies which workflow produced it.
307+
experimentArtifactName := experimentArtifactUploadName(data, sanitizedID)
302308
steps = append(steps,
303309
" - name: Upload experiment artifact\n",
304310
" if: always()\n",
@@ -388,17 +394,50 @@ func sortedExperimentNames(experiments map[string][]string) []string {
388394
return names
389395
}
390396

397+
// experimentArtifactUploadName returns the artifact name used when uploading the experiment
398+
// artifact from the activation job.
399+
// For workflow_call workflows the runtime prefix expression is prepended.
400+
// For regular workflows the sanitized workflow ID is used as a prefix so the artifact name
401+
// uniquely identifies the producing workflow (e.g. "smokecopilot-experiment").
402+
// An empty sanitizedID falls back to the base name for defensive compatibility; in practice
403+
// the compiler always sets a non-empty WorkflowID before this function is called.
404+
func experimentArtifactUploadName(data *WorkflowData, sanitizedID string) string {
405+
if hasWorkflowCallTrigger(data.On) {
406+
return artifactPrefixExprForActivationJob(data) + constants.ExperimentArtifactName
407+
}
408+
if sanitizedID == "" {
409+
return constants.ExperimentArtifactName
410+
}
411+
return sanitizedID + "-" + constants.ExperimentArtifactName
412+
}
413+
414+
// experimentArtifactDownloadName returns the artifact name used when downloading the experiment
415+
// artifact from a downstream job.
416+
// For workflow_call workflows the runtime prefix expression is prepended.
417+
// For regular workflows the sanitized workflow ID is used as a prefix, matching the name
418+
// produced by experimentArtifactUploadName.
419+
// An empty sanitizedID falls back to the base name for defensive compatibility; in practice
420+
// the compiler always sets a non-empty WorkflowID before this function is called.
421+
func experimentArtifactDownloadName(data *WorkflowData) string {
422+
if hasWorkflowCallTrigger(data.On) {
423+
return artifactPrefixExprForDownstreamJob(data) + constants.ExperimentArtifactName
424+
}
425+
sanitizedID := SanitizeWorkflowIDForCacheKey(data.WorkflowID)
426+
if sanitizedID == "" {
427+
return constants.ExperimentArtifactName
428+
}
429+
return sanitizedID + "-" + constants.ExperimentArtifactName
430+
}
431+
391432
// buildExperimentArtifactDownloadSteps creates a download step for the experiment artifact.
392433
// The artifact is downloaded to experimentsCacheDir so the detection agent can read the
393434
// current variant assignments from state.json.
394-
// prefix must be the artifact prefix expression for a job that directly depends on the
395-
// activation job (i.e. artifactPrefixExprForDownstreamJob).
396435
// The step is a no-op when no experiments are declared.
397-
func buildExperimentArtifactDownloadSteps(prefix string, experiments map[string][]string) []string {
398-
if len(experiments) == 0 {
436+
func buildExperimentArtifactDownloadSteps(data *WorkflowData) []string {
437+
if len(data.Experiments) == 0 {
399438
return nil
400439
}
401-
artifactName := prefix + constants.ExperimentArtifactName
440+
artifactName := experimentArtifactDownloadName(data)
402441
return buildArtifactDownloadSteps(ArtifactDownloadConfig{
403442
ArtifactName: artifactName,
404443
DownloadPath: experimentsCacheDir + "/",

‎pkg/workflow/compiler_experiments_test.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ func TestGenerateExperimentSteps_Empty(t *testing.T) {
226226
func TestGenerateExperimentSteps_Generated(t *testing.T) {
227227
c := &Compiler{}
228228
data := &WorkflowData{
229+
WorkflowID: "my-workflow",
229230
Experiments: map[string][]string{
230231
"feature1": {"A", "B"},
231232
},
@@ -239,7 +240,10 @@ func TestGenerateExperimentSteps_Generated(t *testing.T) {
239240
assert.Contains(t, joined, "pick_experiment.cjs", "should reference pick_experiment.cjs")
240241
assert.Contains(t, joined, "Save experiment state", "should include cache save step")
241242
assert.Contains(t, joined, "Upload experiment artifact", "should include artifact upload step")
242-
assert.Contains(t, joined, "experiment", "artifact name should include 'experiment'")
243+
assert.Contains(t, joined, "myworkflow-experiment", "artifact name should include sanitized workflow ID and 'experiment'")
244+
// Cache key must embed the literal sanitized workflow ID, not the env var.
245+
assert.Contains(t, joined, "experiments-myworkflow-", "cache key should include the sanitized workflow ID")
246+
assert.NotContains(t, joined, "GH_AW_WORKFLOW_ID_SANITIZED", "cache key must not reference unset env var")
243247
}
244248

245249
func TestGenerateExperimentSteps_SpecJSON(t *testing.T) {
@@ -295,32 +299,41 @@ func TestExperimentExpressionMappings(t *testing.T) {
295299
// ── buildExperimentArtifactDownloadSteps ──────────────────────────────────
296300

297301
func TestBuildExperimentArtifactDownloadStep_Empty(t *testing.T) {
298-
steps := buildExperimentArtifactDownloadSteps("prefix-", nil)
302+
steps := buildExperimentArtifactDownloadSteps(&WorkflowData{WorkflowID: "test-wf"})
299303
assert.Empty(t, steps, "no steps when experiments is nil")
300304

301-
steps = buildExperimentArtifactDownloadSteps("prefix-", map[string][]string{})
305+
steps = buildExperimentArtifactDownloadSteps(&WorkflowData{WorkflowID: "test-wf", Experiments: map[string][]string{}})
302306
assert.Empty(t, steps, "no steps when experiments is empty")
303307
}
304308

305309
func TestBuildExperimentArtifactDownloadStep_Generated(t *testing.T) {
306-
experiments := map[string][]string{"caveman": {"yes", "no"}}
307-
steps := buildExperimentArtifactDownloadSteps("${{ needs.activation.outputs.artifact_prefix }}", experiments)
310+
// workflow_call trigger: artifact name uses the runtime prefix expression.
311+
data := &WorkflowData{
312+
WorkflowID: "my-wf",
313+
Experiments: map[string][]string{"caveman": {"yes", "no"}},
314+
On: "workflow_call:",
315+
}
316+
steps := buildExperimentArtifactDownloadSteps(data)
308317
require.NotEmpty(t, steps, "steps should be generated when experiments are declared")
309318
joined := strings.Join(steps, "")
310319
assert.Contains(t, joined, "Download experiment artifact", "should include download step name")
311320
assert.Contains(t, joined, "experiment", "should reference experiment artifact")
312321
assert.Contains(t, joined, experimentsCacheDir, "should download to experiments cache dir")
313322
assert.Contains(t, joined, "actions/download-artifact", "should use download-artifact action")
323+
assert.Contains(t, joined, "${{ needs.activation.outputs.artifact_prefix }}", "workflow_call should use runtime prefix")
314324
}
315325

316326
func TestBuildExperimentArtifactDownloadStep_NoPrefix(t *testing.T) {
317-
// Non-workflow_call workflows use empty prefix
318-
experiments := map[string][]string{"style": {"A", "B"}}
319-
steps := buildExperimentArtifactDownloadSteps("", experiments)
327+
// Non-workflow_call workflows use the sanitized workflow ID as prefix.
328+
data := &WorkflowData{
329+
WorkflowID: "smoke-copilot",
330+
Experiments: map[string][]string{"style": {"A", "B"}},
331+
}
332+
steps := buildExperimentArtifactDownloadSteps(data)
320333
require.NotEmpty(t, steps, "steps should be generated")
321334
joined := strings.Join(steps, "")
322-
// Artifact name should be just the base name (no prefix)
323-
assert.Contains(t, joined, " name: experiment\n", "artifact name should be unqualified for non-workflow_call")
335+
// Artifact name should include the sanitized workflow ID as prefix.
336+
assert.Contains(t, joined, " name: smokecopilot-experiment\n", "artifact name should include sanitized workflow ID")
324337
}
325338

326339
// ── extractExperimentConfigsFromFrontmatter ───────────────────────────────

‎pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ jobs:
6565
GH_AW_INFO_AWMG_VERSION: ""
6666
GH_AW_INFO_FIREWALL_TYPE: "squid"
6767
GH_AW_COMPILED_STRICT: "true"
68-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
68+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
6969
with:
7070
script: |
7171
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
@@ -100,7 +100,7 @@ jobs:
100100
run: bash "${RUNNER_TEMP}/gh-aw/actions/save_base_github_folders.sh"
101101
- name: Check workflow lock file
102102
id: check-lock-file
103-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
103+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
104104
env:
105105
GH_AW_WORKFLOW_FILE: "basic-copilot.lock.yml"
106106
GH_AW_CONTEXT_WORKFLOW_REF: "${{ github.workflow_ref }}"
@@ -171,7 +171,7 @@ jobs:
171171
GH_AW_PROMPT_NORM_EOF
172172
} > "$GH_AW_PROMPT"
173173
- name: Interpolate variables and render templates
174-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
174+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
175175
env:
176176
GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt
177177
with:
@@ -181,7 +181,7 @@ jobs:
181181
const { main } = require('${{ runner.temp }}/gh-aw/actions/interpolate_prompt.cjs');
182182
await main();
183183
- name: Substitute placeholders
184-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
184+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
185185
env:
186186
GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt
187187
GH_AW_GITHUB_ACTOR: ${{ github.actor }}
@@ -297,7 +297,7 @@ jobs:
297297
id: checkout-pr
298298
if: |
299299
github.event.pull_request || github.event.issue.pull_request
300-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
300+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
301301
env:
302302
GH_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}
303303
with:
@@ -315,7 +315,7 @@ jobs:
315315
run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.29
316316
- name: Determine automatic lockdown mode for GitHub MCP Server
317317
id: determine-automatic-lockdown
318-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
318+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9
319319
env:
320320
GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }}
321321
GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }}
@@ -466,7 +466,7 @@ jobs:
466466
bash "${RUNNER_TEMP}/gh-aw/actions/stop_mcp_gateway.sh" "$GATEWAY_PID"
467467
- name: Redact secrets in logs
468468
if: always()
469-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
469+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
470470
with:
471471
script: |
472472
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
@@ -484,7 +484,7 @@ jobs:
484484
run: bash "${RUNNER_TEMP}/gh-aw/actions/append_agent_step_summary.sh"
485485
- name: Parse agent logs for step summary
486486
if: always()
487-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
487+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
488488
env:
489489
GH_AW_AGENT_OUTPUT: /tmp/gh-aw/sandbox/agent/logs/
490490
with:
@@ -496,7 +496,7 @@ jobs:
496496
- name: Parse MCP Gateway logs for step summary
497497
if: always()
498498
id: parse-mcp-gateway
499-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
499+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
500500
with:
501501
script: |
502502
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
@@ -521,7 +521,7 @@ jobs:
521521
- name: Parse token usage for step summary
522522
if: always()
523523
continue-on-error: true
524-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
524+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
525525
with:
526526
script: |
527527
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
@@ -531,7 +531,7 @@ jobs:
531531
- name: Print AWF reflect summary
532532
if: always()
533533
continue-on-error: true
534-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
534+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
535535
with:
536536
script: |
537537
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
@@ -583,7 +583,7 @@ jobs:
583583
job-name: ${{ github.job }}
584584
- name: Check team membership for workflow
585585
id: check_membership
586-
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
586+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
587587
env:
588588
GH_AW_REQUIRED_ROLES: ""
589589
with:

0 commit comments

Comments
 (0)