diff --git a/.github/workflows/smoke-gemini.lock.yml b/.github/workflows/smoke-gemini.lock.yml index a015d7719a..2c43e464da 100644 --- a/.github/workflows/smoke-gemini.lock.yml +++ b/.github/workflows/smoke-gemini.lock.yml @@ -1018,6 +1018,9 @@ jobs: SERVER_URL_STRIPPED="${SERVER_URL#https://}" git remote set-url origin "https://x-access-token:${{ github.token }}@${SERVER_URL_STRIPPED}/${REPO_NAME}.git" echo "Git configured with standard GitHub Actions identity" + - name: Move Gemini error files to artifact directory + if: always() + run: mv /tmp/gemini-client-error-*.json /tmp/gh-aw/ 2>/dev/null || true - name: Stop MCP Gateway if: always() continue-on-error: true @@ -1065,9 +1068,6 @@ jobs: setupGlobals(core, github, context, exec, io); const { main } = require('/opt/gh-aw/actions/collect_ndjson_output.cjs'); await main(); - - name: Clean up engine output files - run: | - rm -fr /tmp/gemini-client-error-*.json - name: Parse agent logs for step summary if: always() uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 @@ -1111,7 +1111,7 @@ jobs: name: agent path: | /tmp/gh-aw/aw-prompts/prompt.txt - /tmp/gemini-client-error-*.json + /tmp/gh-aw/gemini-client-error-*.json /tmp/gh-aw/redacted-urls.log /tmp/gh-aw/mcp-logs/ /tmp/gh-aw/mcp-scripts/logs/ diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 7c2db6a27c..fa7b67ef8a 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -153,6 +153,13 @@ type WorkflowExecutor interface { // microsoft/apm-action. Supported values are "copilot", "claude", and "all". // The default implementation returns "all" (packs all primitive types). GetAPMTarget() string + + // GetPreBundleSteps returns GitHub Actions steps that must run before the unified artifact + // upload. These steps are injected prior to secret redaction so any engine-produced files + // are moved into /tmp/gh-aw/ where they will be scanned and picked up by the artifact + // upload under the correct common-ancestor path. + // The default implementation returns an empty slice. + GetPreBundleSteps(workflowData *WorkflowData) []GitHubActionStep } // MCPConfigProvider handles MCP (Model Context Protocol) configuration @@ -348,6 +355,13 @@ func (e *BaseEngine) GetAPMTarget() string { return "all" } +// GetPreBundleSteps returns an empty slice by default. +// Engines that need to relocate output files into /tmp/gh-aw/ before the unified artifact +// upload should override this method. +func (e *BaseEngine) GetPreBundleSteps(workflowData *WorkflowData) []GitHubActionStep { + return []GitHubActionStep{} +} + // ParseLogMetrics provides a default no-op implementation for log parsing // Engines can override this to provide detailed log parsing and metrics extraction func (e *BaseEngine) ParseLogMetrics(logContent string, verbose bool) LogMetrics { diff --git a/pkg/workflow/agentic_output_test.go b/pkg/workflow/agentic_output_test.go index c68b3c1ffe..fcf25bbd5c 100644 --- a/pkg/workflow/agentic_output_test.go +++ b/pkg/workflow/agentic_output_test.go @@ -268,8 +268,8 @@ func TestEngineOutputFileDeclarations(t *testing.T) { t.Error("Gemini engine should declare output files for error log collection") } - if len(geminiOutputFiles) > 0 && geminiOutputFiles[0] != "/tmp/gemini-client-error-*.json" { - t.Errorf("Gemini engine should declare /tmp/gemini-client-error-*.json, got: %v", geminiOutputFiles[0]) + if len(geminiOutputFiles) > 0 && geminiOutputFiles[0] != "/tmp/gh-aw/gemini-client-error-*.json" { + t.Errorf("Gemini engine should declare /tmp/gh-aw/gemini-client-error-*.json, got: %v", geminiOutputFiles[0]) } t.Logf("Claude engine declares: %v", claudeOutputFiles) @@ -703,8 +703,12 @@ This workflow tests that the Gemini engine error log wildcard is uploaded as an } // Verify that the Gemini error log wildcard path is included in the unified artifact upload - if !strings.Contains(lockStr, "/tmp/gemini-client-error-*.json") { - t.Error("Gemini workflow should include '/tmp/gemini-client-error-*.json' in unified agent artifact upload") + // under /tmp/gh-aw/ (not /tmp/) so the artifact upload LCA stays at /tmp/gh-aw/ + if !strings.Contains(lockStr, "/tmp/gh-aw/gemini-client-error-*.json") { + t.Error("Gemini workflow should include '/tmp/gh-aw/gemini-client-error-*.json' in unified agent artifact upload") + } + if strings.Contains(lockStr, "path: /tmp/gemini-client-error") { + t.Error("Gemini workflow must NOT have '/tmp/gemini-client-error' artifact path (causes wrong LCA)") } // Verify that the unified agent artifact name is used (not separate agent_outputs) diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index b11f9eb5bf..48fdbe037a 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -344,6 +344,14 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat } } + // Run engine pre-bundle steps to relocate files before secret redaction. + // This ensures all artifact paths share a common ancestor under /tmp/gh-aw/. + for _, step := range engine.GetPreBundleSteps(data) { + for _, line := range step { + yaml.WriteString(line + "\n") + } + } + // Stop MCP gateway after agent execution and before secret redaction // This ensures the gateway process is properly cleaned up // The MCP gateway is always enabled, even when agent sandbox is disabled diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 0a32835469..8987697fda 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -166,9 +166,25 @@ func (e *GeminiEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub // Gemini CLI writes structured error reports to /tmp/gemini-client-error-*.json // with a timestamp in the filename (e.g. gemini-client-error-Turn.run-sendMessageStream-2026-02-21T20-45-59-824Z.json). // These files provide detailed diagnostics when the Gemini API call fails. +// GetPreBundleSteps moves these files into /tmp/gh-aw/ so all artifact paths share a common +// ancestor under /tmp/gh-aw/ and the actions/upload-artifact LCA calculation stays correct. func (e *GeminiEngine) GetDeclaredOutputFiles() []string { return []string{ - "/tmp/gemini-client-error-*.json", + "/tmp/gh-aw/gemini-client-error-*.json", + } +} + +// GetPreBundleSteps returns a step that moves Gemini CLI error reports from /tmp/ into +// /tmp/gh-aw/ before the unified artifact upload. This keeps all artifact paths under +// /tmp/gh-aw/ so that actions/upload-artifact computes the correct least-common-ancestor +// path and downstream jobs find files at the expected locations. +func (e *GeminiEngine) GetPreBundleSteps(workflowData *WorkflowData) []GitHubActionStep { + return []GitHubActionStep{ + { + " - name: Move Gemini error files to artifact directory", + " if: always()", + " run: mv /tmp/gemini-client-error-*.json /tmp/gh-aw/ 2>/dev/null || true", + }, } } diff --git a/pkg/workflow/gemini_engine_test.go b/pkg/workflow/gemini_engine_test.go index 2d69c6d4b1..cac7364cc3 100644 --- a/pkg/workflow/gemini_engine_test.go +++ b/pkg/workflow/gemini_engine_test.go @@ -57,7 +57,18 @@ func TestGeminiEngine(t *testing.T) { t.Run("declared output files", func(t *testing.T) { outputFiles := engine.GetDeclaredOutputFiles() require.Len(t, outputFiles, 1, "Should declare one output file path") - assert.Equal(t, "/tmp/gemini-client-error-*.json", outputFiles[0], "Should declare Gemini error log wildcard path") + assert.Equal(t, "/tmp/gh-aw/gemini-client-error-*.json", outputFiles[0], "Should declare Gemini error log wildcard path under /tmp/gh-aw/") + }) + + t.Run("pre-bundle steps move files to /tmp/gh-aw/", func(t *testing.T) { + workflowData := &WorkflowData{Name: "test-workflow"} + steps := engine.GetPreBundleSteps(workflowData) + require.Len(t, steps, 1, "Should return exactly one pre-bundle step") + + stepContent := strings.Join(steps[0], "\n") + assert.Contains(t, stepContent, "Move Gemini error files", "Step name should describe move operation") + assert.Contains(t, stepContent, "mv /tmp/gemini-client-error-*.json /tmp/gh-aw/", "Step should move files to /tmp/gh-aw/") + assert.Contains(t, stepContent, "if: always()", "Step should run always so files are captured on failure") }) } diff --git a/pkg/workflow/step_order_validation.go b/pkg/workflow/step_order_validation.go index 61e4d5c5e8..24c07a3b91 100644 --- a/pkg/workflow/step_order_validation.go +++ b/pkg/workflow/step_order_validation.go @@ -190,11 +190,11 @@ func isPathScannedBySecretRedaction(path string) bool { } // Allow wildcard paths under /tmp/ with a known-safe extension. - // These are engine-declared diagnostic output files (e.g. Gemini CLI error reports at - // /tmp/gemini-client-error-*.json). They are produced by the CLI tool itself, not by - // agent-generated content, and they live outside /tmp/gh-aw/ so they are not scanned by - // the redact_secrets step. However, these files (JSON error reports, log files) are - // structurally unlikely to contain raw secret values, so we allow them through validation. + // These are engine-declared diagnostic output files that have not yet been relocated. + // NOTE: The Gemini engine now moves its error reports into /tmp/gh-aw/ via + // GetPreBundleSteps before the upload, so they are covered by the /tmp/gh-aw/ branch + // above. This exception is kept for any future engine that may produce wildcard + // diagnostic files directly under /tmp/. if strings.HasPrefix(path, "/tmp/") && strings.Contains(path, "*") { ext := filepath.Ext(path) safeExtensions := []string{".txt", ".json", ".log", ".jsonl"}