Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/smoke-gemini.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions pkg/workflow/agentic_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions pkg/workflow/agentic_output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions pkg/workflow/compiler_yaml_main_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion pkg/workflow/gemini_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}
}

Expand Down
13 changes: 12 additions & 1 deletion pkg/workflow/gemini_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/workflow/step_order_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
Loading