From c41b99bea147f9f03842254ede783044f695bc71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 01:07:55 +0000 Subject: [PATCH 1/3] Initial plan From a47ad7dcef2ae04dd700e25ae2cff7ad029a36ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 01:25:28 +0000 Subject: [PATCH 2/3] refactor: split audit.go and import_field_extractor.go by semantic clusters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pkg/cli/audit.go (1257 → 933 lines): - Extract job audit cluster → audit_job.go (223 lines) - Extract workflow run metadata cluster → workflow_run_metadata.go (127 lines) pkg/parser/import_field_extractor.go (1369 → 899 lines): - Extract observability/OTLP cluster → import_observability.go (202 lines) - Extract import schema validation cluster → import_schema_validation.go (228 lines) - Extract import input substitution cluster → import_input_substitution.go (57 lines) Pure function moves within each package — no signature or behavior changes. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/audit.go | 324 ---------------- pkg/cli/audit_job.go | 223 +++++++++++ pkg/cli/workflow_run_metadata.go | 127 +++++++ pkg/parser/import_field_extractor.go | 470 ------------------------ pkg/parser/import_input_substitution.go | 57 +++ pkg/parser/import_observability.go | 202 ++++++++++ pkg/parser/import_schema_validation.go | 228 ++++++++++++ 7 files changed, 837 insertions(+), 794 deletions(-) create mode 100644 pkg/cli/audit_job.go create mode 100644 pkg/cli/workflow_run_metadata.go create mode 100644 pkg/parser/import_input_substitution.go create mode 100644 pkg/parser/import_observability.go create mode 100644 pkg/parser/import_schema_validation.go diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index 73887e88600..2b43519cdd7 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -2,25 +2,19 @@ package cli import ( "context" - "encoding/json" "errors" "fmt" "os" "path/filepath" - "strconv" "strings" "sync" "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" - "github.com/github/gh-aw/pkg/errorutil" "github.com/github/gh-aw/pkg/fileutil" - "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" - "github.com/github/gh-aw/pkg/workflow" - "github.com/goccy/go-yaml" "github.com/spf13/cobra" ) @@ -937,321 +931,3 @@ func renderAuditCompletion(runOutputDir string, jsonOutput bool) { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Audit complete. Logs saved to "+absOutputDir)) fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Tip: use --artifacts to select specific artifact sets (agent, firewall, mcp, activation, detection, etc.)")) } - -// auditJobRunOptions holds parameters for auditJobRun. -type auditJobRunOptions struct { - runID int64 - jobID int64 - stepNumber int - owner string - repo string - hostname string - outputDir string - verbose bool - jsonOutput bool -} - -// auditJobRun performs a targeted audit of a specific job within a workflow run -// If stepNumber > 0, focuses on extracting output for that specific step -func auditJobRun(opts auditJobRunOptions) error { - opts.hostname = resolveAuditHostname(opts.hostname) - auditLog.Printf("Starting job-specific audit: runID=%d, jobID=%d, stepNumber=%d, hostname=%s", opts.runID, opts.jobID, opts.stepNumber, opts.hostname) - if err := os.MkdirAll(opts.outputDir, constants.DirPermSensitive); err != nil { - return fmt.Errorf("failed to create output directory: %w", err) - } - jobLogContent, jobLogPath, err := fetchAuditJobLog(opts) - if err != nil { - return err - } - if err := extractAuditJobDetails(opts, jobLogContent); err != nil { - return err - } - renderAuditJobSummary(opts, jobLogPath) - return nil -} - -func fetchAuditJobLog(opts auditJobRunOptions) (string, string, error) { - args := []string{"run", "view"} - if opts.owner != "" && opts.repo != "" { - args = append(args, "-R", fmt.Sprintf("%s/%s", opts.owner, opts.repo)) - } - args = append(args, "--job", strconv.FormatInt(opts.jobID, 10), "--log") - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Fetching logs for job %d...", opts.jobID))) - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Executing: gh "+strings.Join(args, " "))) - } - cmd := workflow.ExecGH(args...) - workflow.SetGHHostEnv(cmd, opts.hostname) - output, err := cmd.CombinedOutput() - if err != nil { - return "", "", fmt.Errorf("failed to fetch job logs: %w\nOutput: %s", err, string(output)) - } - jobLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d.log", opts.jobID)) - if err := os.WriteFile(jobLogPath, output, constants.FilePermSensitive); err != nil { - return "", "", fmt.Errorf("failed to write job log: %w", err) - } - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Job log saved to "+jobLogPath)) - } - return string(output), jobLogPath, nil -} - -func extractAuditJobDetails(opts auditJobRunOptions, jobLogContent string) error { - if opts.stepNumber > 0 { - return extractRequestedStepOutput(opts, jobLogContent) - } - return extractFirstFailingStepOutput(opts, jobLogContent) -} - -func extractRequestedStepOutput(opts auditJobRunOptions, jobLogContent string) error { - stepOutput, err := extractStepOutput(jobLogContent, opts.stepNumber) - if err != nil { - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not extract step %d output: %v", opts.stepNumber, err))) - } - return nil - } - stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d.log", opts.jobID, opts.stepNumber)) - if err := os.WriteFile(stepLogPath, []byte(stepOutput), constants.FilePermSensitive); err != nil { - return fmt.Errorf("failed to write step log: %w", err) - } - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Step %d output saved to %s", opts.stepNumber, stepLogPath))) - } - return nil -} - -func extractFirstFailingStepOutput(opts auditJobRunOptions, jobLogContent string) error { - failingStepNum, failingStepOutput := findFirstFailingStep(jobLogContent) - if failingStepNum == 0 { - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No failing steps found in job")) - } - return nil - } - stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d-failed.log", opts.jobID, failingStepNum)) - if err := os.WriteFile(stepLogPath, []byte(failingStepOutput), constants.FilePermSensitive); err != nil { - return fmt.Errorf("failed to write failing step log: %w", err) - } - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("First failing step %d output saved to %s", failingStepNum, stepLogPath))) - } - return nil -} - -func renderAuditJobSummary(opts auditJobRunOptions, jobLogPath string) { - if opts.jsonOutput { - return - } - absOutputDir, _ := filepath.Abs(opts.outputDir) - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Job audit complete. Logs saved to "+absOutputDir)) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("\nDownloaded files:")) - fmt.Fprintf(os.Stderr, " - %s (full job log)\n", jobLogPath) - if opts.stepNumber > 0 { - renderRequestedStepSummary(opts) - return - } - renderFailingStepSummary(opts) -} - -func renderRequestedStepSummary(opts auditJobRunOptions) { - stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d.log", opts.jobID, opts.stepNumber)) - if fileutil.FileExists(stepLogPath) { - fmt.Fprintf(os.Stderr, " - %s (step %d output)\n", stepLogPath, opts.stepNumber) - } -} - -func renderFailingStepSummary(opts auditJobRunOptions) { - failingStepPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-*-failed.log", opts.jobID)) - matches, _ := filepath.Glob(failingStepPath) - for _, match := range matches { - fmt.Fprintf(os.Stderr, " - %s (first failing step)\n", match) - } -} - -// extractStepOutput extracts the output of a specific step from job logs -func extractStepOutput(jobLog string, stepNumber int) (string, error) { - auditLog.Printf("Extracting output for step %d from job logs (%d bytes)", stepNumber, len(jobLog)) - lines := strings.Split(jobLog, "\n") - var stepOutput []string - inStep := false - stepPattern := "##[group]Run " // GitHub Actions step marker - stepEndPattern := "##[endgroup]" - currentStep := 0 - - for _, line := range lines { - // Detect step boundaries - if strings.Contains(line, stepPattern) || strings.HasPrefix(line, fmt.Sprintf("##[group]Step %d:", stepNumber)) { - currentStep++ - if currentStep == stepNumber { - inStep = true - } - } else if strings.Contains(line, stepEndPattern) { - if inStep { - break // End of target step - } - } - - if inStep { - stepOutput = append(stepOutput, line) - } - } - - if len(stepOutput) == 0 { - auditLog.Printf("Step %d not found in job logs (scanned %d lines)", stepNumber, len(lines)) - return "", fmt.Errorf("step %d not found in job logs", stepNumber) - } - - auditLog.Printf("Extracted %d lines for step %d", len(stepOutput), stepNumber) - return strings.Join(stepOutput, "\n"), nil -} - -// findFirstFailingStep finds the first step that failed in the job logs -func findFirstFailingStep(jobLog string) (int, string) { - auditLog.Printf("Searching for first failing step in job logs (%d bytes)", len(jobLog)) - lines := strings.Split(jobLog, "\n") - var stepOutput []string - inStep := false - currentStep := 0 - foundFailure := false - - for _, line := range lines { - // Detect step start - if strings.Contains(line, "##[group]") { - if inStep && foundFailure { - break // We found a complete failing step - } - inStep = true - currentStep++ - stepOutput = []string{line} - foundFailure = false - } else if inStep { - stepOutput = append(stepOutput, line) - - // Detect failure indicators - if strings.Contains(line, "##[error]") || - strings.Contains(line, "Error:") || - strings.Contains(line, "FAILED") || - strings.Contains(line, "exit code") && !strings.Contains(line, "exit code 0") { - foundFailure = true - } - } - } - - if foundFailure && len(stepOutput) > 0 { - auditLog.Printf("Found failing step %d with %d lines of output", currentStep, len(stepOutput)) - return currentStep, strings.Join(stepOutput, "\n") - } - - auditLog.Print("No failing step found in job logs") - return 0, "" -} - -// fetchWorkflowRunMetadata fetches metadata for a single workflow run -func fetchWorkflowRunMetadata(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) { - args := buildWorkflowRunMetadataArgs(runID, owner, repo, hostname) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(args, " "))) - } - output, err := workflow.RunGHCombinedContext(ctx, "Fetching run metadata...", args...) - if err != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(string(output))) - } - return WorkflowRun{}, classifyWorkflowRunMetadataError(runID, err, output) - } - var run WorkflowRun - if err := json.Unmarshal(output, &run); err != nil { - return WorkflowRun{}, fmt.Errorf("failed to parse run metadata: %w", err) - } - resolveWorkflowRunDisplayName(ctx, &run, owner, repo, hostname) - return run, nil -} - -func buildWorkflowRunMetadataArgs(runID int64, owner, repo, hostname string) []string { - endpoint := fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d", runID) - if owner != "" && repo != "" { - endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d", owner, repo, runID) - } - args := []string{"api"} - if hostname != "" && hostname != "github.com" { - args = append(args, "--hostname", hostname) - } - return append(args, endpoint, "--jq", "{databaseId: .id, number: .run_number, url: .html_url, status: .status, conclusion: .conclusion, workflowName: .name, workflowPath: .path, createdAt: .created_at, startedAt: .run_started_at, updatedAt: .updated_at, event: .event, headBranch: .head_branch, headSha: .head_sha, displayTitle: .display_title}") -} - -func classifyWorkflowRunMetadataError(runID int64, err error, output []byte) error { - outputStr := string(output) - if errorutil.IsNotFoundError(err) || - errorutil.IsNotFoundError(errors.New(outputStr)) || - strings.Contains(outputStr, "Could not resolve") { - return fmt.Errorf("workflow run %d not found. Please verify the run ID is correct and that you have access to the repository", runID) - } - return fmt.Errorf("failed to fetch run metadata: %w", err) -} - -func resolveWorkflowRunDisplayName(ctx context.Context, run *WorkflowRun, owner, repo, hostname string) { - if !strings.HasPrefix(run.WorkflowName, constants.GithubDir) { - return - } - if displayName := resolveWorkflowDisplayName(ctx, run.WorkflowPath, owner, repo, hostname); displayName != "" { - auditLog.Printf("Resolved workflow display name: %q -> %q", run.WorkflowName, displayName) - run.WorkflowName = displayName - } -} - -// resolveWorkflowDisplayName returns the human-readable display name for a workflow file. -// It first attempts to read the YAML file from the local filesystem (resolving the path -// relative to the git repository root so that it works from any working directory inside -// the repo); if that fails it falls back to a GitHub API call. An empty string is -// returned on any error so that callers can gracefully keep the original value. -func resolveWorkflowDisplayName(ctx context.Context, workflowPath, owner, repo, hostname string) string { - // Try local file first. workflowPath is a repo-relative path like - // ".github/workflows/foo.lock.yml", so we resolve it against the git root to - // produce a correct absolute path regardless of the current working directory. - if gitRoot, err := gitutil.FindGitRoot(); err == nil { - absPath := filepath.Join(gitRoot, workflowPath) - if content, err := os.ReadFile(absPath); err == nil { - if name := extractWorkflowNameFromYAML(content); name != "" { - return name - } - } - } - - // Fall back to the GitHub Actions workflows API. - filename := filepath.Base(workflowPath) - var endpoint string - if owner != "" && repo != "" { - endpoint = fmt.Sprintf("repos/%s/%s/actions/workflows/%s", owner, repo, filename) - } else { - endpoint = "repos/{owner}/{repo}/actions/workflows/" + filename - } - - args := []string{"api"} - if hostname != "" && hostname != "github.com" { - args = append(args, "--hostname", hostname) - } - args = append(args, endpoint, "--jq", ".name") - - out, err := workflow.RunGHCombinedContext(ctx, "Fetching workflow name...", args...) - if err != nil { - auditLog.Printf("Failed to fetch workflow display name for %q: %v", workflowPath, err) - return "" - } - - return strings.TrimSpace(string(out)) -} - -// extractWorkflowNameFromYAML parses a GitHub Actions workflow YAML document and -// returns the value of its top-level "name:" field. An empty string is returned -// when the field is absent or the document cannot be parsed. -func extractWorkflowNameFromYAML(content []byte) string { - var wf struct { - Name string `yaml:"name"` - } - if err := yaml.Unmarshal(content, &wf); err != nil { - auditLog.Printf("Failed to parse workflow YAML for name extraction (file may be malformed): %v", err) - return "" - } - return wf.Name -} diff --git a/pkg/cli/audit_job.go b/pkg/cli/audit_job.go new file mode 100644 index 00000000000..55158c5eedb --- /dev/null +++ b/pkg/cli/audit_job.go @@ -0,0 +1,223 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/fileutil" + "github.com/github/gh-aw/pkg/workflow" +) + +// auditJobRunOptions holds parameters for auditJobRun. +type auditJobRunOptions struct { + runID int64 + jobID int64 + stepNumber int + owner string + repo string + hostname string + outputDir string + verbose bool + jsonOutput bool +} + +// auditJobRun performs a targeted audit of a specific job within a workflow run +// If stepNumber > 0, focuses on extracting output for that specific step +func auditJobRun(opts auditJobRunOptions) error { + opts.hostname = resolveAuditHostname(opts.hostname) + auditLog.Printf("Starting job-specific audit: runID=%d, jobID=%d, stepNumber=%d, hostname=%s", opts.runID, opts.jobID, opts.stepNumber, opts.hostname) + if err := os.MkdirAll(opts.outputDir, constants.DirPermSensitive); err != nil { + return fmt.Errorf("failed to create output directory: %w", err) + } + jobLogContent, jobLogPath, err := fetchAuditJobLog(opts) + if err != nil { + return err + } + if err := extractAuditJobDetails(opts, jobLogContent); err != nil { + return err + } + renderAuditJobSummary(opts, jobLogPath) + return nil +} + +func fetchAuditJobLog(opts auditJobRunOptions) (string, string, error) { + args := []string{"run", "view"} + if opts.owner != "" && opts.repo != "" { + args = append(args, "-R", fmt.Sprintf("%s/%s", opts.owner, opts.repo)) + } + args = append(args, "--job", strconv.FormatInt(opts.jobID, 10), "--log") + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Fetching logs for job %d...", opts.jobID))) + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Executing: gh "+strings.Join(args, " "))) + } + cmd := workflow.ExecGH(args...) + workflow.SetGHHostEnv(cmd, opts.hostname) + output, err := cmd.CombinedOutput() + if err != nil { + return "", "", fmt.Errorf("failed to fetch job logs: %w\nOutput: %s", err, string(output)) + } + jobLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d.log", opts.jobID)) + if err := os.WriteFile(jobLogPath, output, constants.FilePermSensitive); err != nil { + return "", "", fmt.Errorf("failed to write job log: %w", err) + } + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Job log saved to "+jobLogPath)) + } + return string(output), jobLogPath, nil +} + +func extractAuditJobDetails(opts auditJobRunOptions, jobLogContent string) error { + if opts.stepNumber > 0 { + return extractRequestedStepOutput(opts, jobLogContent) + } + return extractFirstFailingStepOutput(opts, jobLogContent) +} + +func extractRequestedStepOutput(opts auditJobRunOptions, jobLogContent string) error { + stepOutput, err := extractStepOutput(jobLogContent, opts.stepNumber) + if err != nil { + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not extract step %d output: %v", opts.stepNumber, err))) + } + return nil + } + stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d.log", opts.jobID, opts.stepNumber)) + if err := os.WriteFile(stepLogPath, []byte(stepOutput), constants.FilePermSensitive); err != nil { + return fmt.Errorf("failed to write step log: %w", err) + } + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Step %d output saved to %s", opts.stepNumber, stepLogPath))) + } + return nil +} + +func extractFirstFailingStepOutput(opts auditJobRunOptions, jobLogContent string) error { + failingStepNum, failingStepOutput := findFirstFailingStep(jobLogContent) + if failingStepNum == 0 { + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No failing steps found in job")) + } + return nil + } + stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d-failed.log", opts.jobID, failingStepNum)) + if err := os.WriteFile(stepLogPath, []byte(failingStepOutput), constants.FilePermSensitive); err != nil { + return fmt.Errorf("failed to write failing step log: %w", err) + } + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("First failing step %d output saved to %s", failingStepNum, stepLogPath))) + } + return nil +} + +func renderAuditJobSummary(opts auditJobRunOptions, jobLogPath string) { + if opts.jsonOutput { + return + } + absOutputDir, _ := filepath.Abs(opts.outputDir) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Job audit complete. Logs saved to "+absOutputDir)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("\nDownloaded files:")) + fmt.Fprintf(os.Stderr, " - %s (full job log)\n", jobLogPath) + if opts.stepNumber > 0 { + renderRequestedStepSummary(opts) + return + } + renderFailingStepSummary(opts) +} + +func renderRequestedStepSummary(opts auditJobRunOptions) { + stepLogPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-%d.log", opts.jobID, opts.stepNumber)) + if fileutil.FileExists(stepLogPath) { + fmt.Fprintf(os.Stderr, " - %s (step %d output)\n", stepLogPath, opts.stepNumber) + } +} + +func renderFailingStepSummary(opts auditJobRunOptions) { + failingStepPath := filepath.Join(opts.outputDir, fmt.Sprintf("job-%d-step-*-failed.log", opts.jobID)) + matches, _ := filepath.Glob(failingStepPath) + for _, match := range matches { + fmt.Fprintf(os.Stderr, " - %s (first failing step)\n", match) + } +} + +// extractStepOutput extracts the output of a specific step from job logs +func extractStepOutput(jobLog string, stepNumber int) (string, error) { + auditLog.Printf("Extracting output for step %d from job logs (%d bytes)", stepNumber, len(jobLog)) + lines := strings.Split(jobLog, "\n") + var stepOutput []string + inStep := false + stepPattern := "##[group]Run " // GitHub Actions step marker + stepEndPattern := "##[endgroup]" + currentStep := 0 + + for _, line := range lines { + // Detect step boundaries + if strings.Contains(line, stepPattern) || strings.HasPrefix(line, fmt.Sprintf("##[group]Step %d:", stepNumber)) { + currentStep++ + if currentStep == stepNumber { + inStep = true + } + } else if strings.Contains(line, stepEndPattern) { + if inStep { + break // End of target step + } + } + + if inStep { + stepOutput = append(stepOutput, line) + } + } + + if len(stepOutput) == 0 { + auditLog.Printf("Step %d not found in job logs (scanned %d lines)", stepNumber, len(lines)) + return "", fmt.Errorf("step %d not found in job logs", stepNumber) + } + + auditLog.Printf("Extracted %d lines for step %d", len(stepOutput), stepNumber) + return strings.Join(stepOutput, "\n"), nil +} + +// findFirstFailingStep finds the first step that failed in the job logs +func findFirstFailingStep(jobLog string) (int, string) { + auditLog.Printf("Searching for first failing step in job logs (%d bytes)", len(jobLog)) + lines := strings.Split(jobLog, "\n") + var stepOutput []string + inStep := false + currentStep := 0 + foundFailure := false + + for _, line := range lines { + // Detect step start + if strings.Contains(line, "##[group]") { + if inStep && foundFailure { + break // We found a complete failing step + } + inStep = true + currentStep++ + stepOutput = []string{line} + foundFailure = false + } else if inStep { + stepOutput = append(stepOutput, line) + + // Detect failure indicators + if strings.Contains(line, "##[error]") || + strings.Contains(line, "Error:") || + strings.Contains(line, "FAILED") || + strings.Contains(line, "exit code") && !strings.Contains(line, "exit code 0") { + foundFailure = true + } + } + } + + if foundFailure && len(stepOutput) > 0 { + auditLog.Printf("Found failing step %d with %d lines of output", currentStep, len(stepOutput)) + return currentStep, strings.Join(stepOutput, "\n") + } + + auditLog.Print("No failing step found in job logs") + return 0, "" +} diff --git a/pkg/cli/workflow_run_metadata.go b/pkg/cli/workflow_run_metadata.go new file mode 100644 index 00000000000..530164040b6 --- /dev/null +++ b/pkg/cli/workflow_run_metadata.go @@ -0,0 +1,127 @@ +package cli + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/errorutil" + "github.com/github/gh-aw/pkg/gitutil" + "github.com/github/gh-aw/pkg/workflow" + "github.com/goccy/go-yaml" +) + +// fetchWorkflowRunMetadata fetches metadata for a single workflow run +func fetchWorkflowRunMetadata(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) { + args := buildWorkflowRunMetadataArgs(runID, owner, repo, hostname) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(args, " "))) + } + output, err := workflow.RunGHCombinedContext(ctx, "Fetching run metadata...", args...) + if err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(string(output))) + } + return WorkflowRun{}, classifyWorkflowRunMetadataError(runID, err, output) + } + var run WorkflowRun + if err := json.Unmarshal(output, &run); err != nil { + return WorkflowRun{}, fmt.Errorf("failed to parse run metadata: %w", err) + } + resolveWorkflowRunDisplayName(ctx, &run, owner, repo, hostname) + return run, nil +} + +func buildWorkflowRunMetadataArgs(runID int64, owner, repo, hostname string) []string { + endpoint := fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d", runID) + if owner != "" && repo != "" { + endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d", owner, repo, runID) + } + args := []string{"api"} + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) + } + return append(args, endpoint, "--jq", "{databaseId: .id, number: .run_number, url: .html_url, status: .status, conclusion: .conclusion, workflowName: .name, workflowPath: .path, createdAt: .created_at, startedAt: .run_started_at, updatedAt: .updated_at, event: .event, headBranch: .head_branch, headSha: .head_sha, displayTitle: .display_title}") +} + +func classifyWorkflowRunMetadataError(runID int64, err error, output []byte) error { + outputStr := string(output) + if errorutil.IsNotFoundError(err) || + errorutil.IsNotFoundError(errors.New(outputStr)) || + strings.Contains(outputStr, "Could not resolve") { + return fmt.Errorf("workflow run %d not found. Please verify the run ID is correct and that you have access to the repository", runID) + } + return fmt.Errorf("failed to fetch run metadata: %w", err) +} + +func resolveWorkflowRunDisplayName(ctx context.Context, run *WorkflowRun, owner, repo, hostname string) { + if !strings.HasPrefix(run.WorkflowName, constants.GithubDir) { + return + } + if displayName := resolveWorkflowDisplayName(ctx, run.WorkflowPath, owner, repo, hostname); displayName != "" { + auditLog.Printf("Resolved workflow display name: %q -> %q", run.WorkflowName, displayName) + run.WorkflowName = displayName + } +} + +// resolveWorkflowDisplayName returns the human-readable display name for a workflow file. +// It first attempts to read the YAML file from the local filesystem (resolving the path +// relative to the git repository root so that it works from any working directory inside +// the repo); if that fails it falls back to a GitHub API call. An empty string is +// returned on any error so that callers can gracefully keep the original value. +func resolveWorkflowDisplayName(ctx context.Context, workflowPath, owner, repo, hostname string) string { + // Try local file first. workflowPath is a repo-relative path like + // ".github/workflows/foo.lock.yml", so we resolve it against the git root to + // produce a correct absolute path regardless of the current working directory. + if gitRoot, err := gitutil.FindGitRoot(); err == nil { + absPath := filepath.Join(gitRoot, workflowPath) + if content, err := os.ReadFile(absPath); err == nil { + if name := extractWorkflowNameFromYAML(content); name != "" { + return name + } + } + } + + // Fall back to the GitHub Actions workflows API. + filename := filepath.Base(workflowPath) + var endpoint string + if owner != "" && repo != "" { + endpoint = fmt.Sprintf("repos/%s/%s/actions/workflows/%s", owner, repo, filename) + } else { + endpoint = "repos/{owner}/{repo}/actions/workflows/" + filename + } + + args := []string{"api"} + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) + } + args = append(args, endpoint, "--jq", ".name") + + out, err := workflow.RunGHCombinedContext(ctx, "Fetching workflow name...", args...) + if err != nil { + auditLog.Printf("Failed to fetch workflow display name for %q: %v", workflowPath, err) + return "" + } + + return strings.TrimSpace(string(out)) +} + +// extractWorkflowNameFromYAML parses a GitHub Actions workflow YAML document and +// returns the value of its top-level "name:" field. An empty string is returned +// when the field is absent or the document cannot be parsed. +func extractWorkflowNameFromYAML(content []byte) string { + var wf struct { + Name string `yaml:"name"` + } + if err := yaml.Unmarshal(content, &wf); err != nil { + auditLog.Printf("Failed to parse workflow YAML for name extraction (file may be malformed): %v", err) + return "" + } + return wf.Name +} diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index 4aec7cf66b7..b2a22910923 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -9,12 +9,9 @@ import ( "fmt" "maps" "path/filepath" - "regexp" "strings" "github.com/github/gh-aw/pkg/constants" - "github.com/github/gh-aw/pkg/importinpututil" - "github.com/github/gh-aw/pkg/setutil" ) // importAccumulator centralizes the builder/slice/set variables used during @@ -869,207 +866,6 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import } } -// observabilityImportEndpoint is an endpoint entry used during import merging. -// Headers are kept as any (original format: string or map) so that the workflow -// package can later normalise both supported forms correctly. -type observabilityImportEndpoint struct { - URL string `json:"url"` - Headers any `json:"headers,omitempty"` -} - -// extractOTLPEndpointsFromObsMap reads the `otlp.endpoint` field from a raw -// observability map and returns all endpoint entries as observabilityImportEndpoints. -// Supports string, object, and array forms of the endpoint field. -// Top-level `headers` is only applied to the backward-compat string endpoint form. -func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEndpoint { - otlpAny, ok := obs["otlp"] - if !ok { - return nil - } - otlpMap, ok := otlpAny.(map[string]any) - if !ok { - return nil - } - - endpointRaw := otlpMap["endpoint"] - headersRaw := otlpMap["headers"] // only applies to the backward-compat string form - - var result []observabilityImportEndpoint - switch ep := endpointRaw.(type) { - case string: - if ep != "" { - entry := observabilityImportEndpoint{URL: ep} - if headersRaw != nil { - entry.Headers = headersRaw - } - result = append(result, entry) - } - case map[string]any: - if url, _ := ep["url"].(string); url != "" { - entry := observabilityImportEndpoint{URL: url} - if h, hasH := ep["headers"]; hasH { - entry.Headers = h - } - result = append(result, entry) - } - case []any: - for _, item := range ep { - itemMap, ok := item.(map[string]any) - if !ok { - continue - } - url, _ := itemMap["url"].(string) - if url == "" { - continue - } - entry := observabilityImportEndpoint{URL: url} - if h, hasH := itemMap["headers"]; hasH { - entry.Headers = h - } - result = append(result, entry) - } - } - return result -} - -// mergeObservabilityConfigs takes a slice of observability config JSON strings (one per -// import), extracts all OTLP endpoint entries from each (supporting string, object, and -// array forms), deduplicates by URL (first occurrence wins), and returns a single merged -// observability JSON string with all endpoints expressed as an array. Custom OTLP -// attributes are also merged across imports (first occurrence wins per key). -// Returns "" when no valid endpoints or attributes are found. -func mergeObservabilityConfigs(configs []string) string { - seen := make(map[string]struct { - }) - var allEndpoints []observabilityImportEndpoint - mergedAttrs := make(map[string]string) - var mergedGitHubApp map[string]any - - for i, cfgJSON := range configs { - if cfgJSON == "" { - continue - } - var obs map[string]any - if err := json.Unmarshal([]byte(cfgJSON), &obs); err != nil { - parserLog.Printf("Failed to unmarshal observability config from import %d during merge: %v", i, err) - continue - } - for _, e := range extractOTLPEndpointsFromObsMap(obs) { - if !setutil.Contains(seen, e.URL) { - seen[e.URL] = struct { - }{} - allEndpoints = append(allEndpoints, e) - } - } - for k, v := range extractOTLPAttributesFromObsMap(obs) { - if _, exists := mergedAttrs[k]; !exists { - mergedAttrs[k] = v - } - } - if mergedGitHubApp == nil { - mergedGitHubApp = extractOTLPGitHubAppFromObsMap(obs) - } - } - - if len(allEndpoints) == 0 && len(mergedAttrs) == 0 && mergedGitHubApp == nil { - return "" - } - - // Produce a merged config with the endpoint field as an array so that the - // workflow package's collectAllOTLPEndpoints handles it uniformly. Include - // any merged custom attributes so the orchestrator can propagate them. - otlpMap := map[string]any{} - if len(allEndpoints) > 0 { - otlpMap["endpoint"] = allEndpoints - } - if len(mergedAttrs) > 0 { - otlpMap["attributes"] = mergedAttrs - } - if mergedGitHubApp != nil { - otlpMap["github-app"] = mergedGitHubApp - } - merged := map[string]any{"otlp": otlpMap} - b, err := json.Marshal(merged) - if err != nil { - parserLog.Printf("Failed to marshal %d merged OTLP endpoints: %v", len(allEndpoints), err) - return "" - } - return string(b) -} - -func extractOTLPGitHubAppFromObsMap(obs map[string]any) map[string]any { - if obs == nil { - return nil - } - otlpAny, ok := obs["otlp"] - if !ok { - return nil - } - otlpMap, ok := otlpAny.(map[string]any) - if !ok { - return nil - } - githubAppAny, ok := otlpMap["github-app"] - if !ok { - return nil - } - githubAppMap, ok := githubAppAny.(map[string]any) - if !ok { - return nil - } - copyMap := make(map[string]any, len(githubAppMap)) - maps.Copy(copyMap, githubAppMap) - return copyMap -} - -// extractOTLPAttributesFromObsMap reads the custom OTLP attributes map from a -// raw observability section (as parsed from an import's frontmatter). Only -// string values are accepted; non-string values are silently ignored. -// Returns nil when the field is absent or empty. -// -// Note: this intentionally duplicates the logic of -// workflow.extractOTLPCustomAttributesFromObsMap. The parser package must not -// import the workflow package (circular-dependency risk), so the helper lives -// here as a local copy. Both implementations must stay in sync. -func extractOTLPAttributesFromObsMap(obs map[string]any) map[string]string { - if obs == nil { - return nil - } - otlpAny, ok := obs["otlp"] - if !ok { - return nil - } - otlpMap, ok := otlpAny.(map[string]any) - if !ok { - return nil - } - attrsAny, ok := otlpMap["attributes"] - if !ok { - return nil - } - attrsMap, ok := attrsAny.(map[string]any) - if !ok { - return nil - } - result := make(map[string]string, len(attrsMap)) - for k, v := range attrsMap { - if s, ok := v.(string); ok && k != "" { - result[k] = s - } - } - return result -} - -// suitable for use in a {{#runtime-import ...}} macro. -// -// The rules are: -// 1. If fullPath contains "/.github/" (as a path component), trim everything before -// and including the leading slash so the result starts with ".github/". -// LastIndex is used so that repos named ".github" (e.g. path -// "/root/.github/.github/workflows/file.md") resolve to the correct -// ".github/workflows/…" segment rather than the first occurrence. -// 2. If fullPath already starts with ".github/" (a relative path) use it as-is. -// 3. Otherwise fall back to importPath (the original import spec). func computeImportRelPath(fullPath, importPath string) string { normalizedFullPath := filepath.ToSlash(fullPath) if idx := strings.LastIndex(normalizedFullPath, "/.github/"); idx >= 0 { @@ -1101,269 +897,3 @@ func validateGitHubAppJSON(appJSON string) string { } return appJSON } - -// validateWithImportSchema validates the provided 'with'/'inputs' values against -// the 'import-schema' declared in the imported workflow's frontmatter. -// It checks that: -// - all required parameters declared in import-schema are present in 'with' -// - no unknown parameters are provided (i.e., not declared in import-schema) -// - provided values match the declared type (string, number, boolean, choice) -// - choice values are within the allowed options list -// -// If the imported workflow has no 'import-schema', all provided 'with' values are -// accepted without validation (backward compatibility with 'inputs' form). -func validateWithImportSchema(inputs map[string]any, fm map[string]any, importPath string) error { - rawSchema, hasSchema := fm["import-schema"] - if !hasSchema { - return nil - } - schemaMap, ok := rawSchema.(map[string]any) - if !ok { - return nil - } - if len(schemaMap) == 0 { - return nil - } - - // Check for unknown keys not declared in import-schema - for key := range inputs { - if _, declared := schemaMap[key]; !declared { - return fmt.Errorf("import '%s': unknown 'with' input %q is not declared in the import-schema", importPath, key) - } - } - - // Check each declared schema field - for paramName, paramDefRaw := range schemaMap { - paramDef, _ := paramDefRaw.(map[string]any) - - // Check required parameters - if req, _ := paramDef["required"].(bool); req { - if _, provided := inputs[paramName]; !provided { - return fmt.Errorf("import '%s': required 'with' input %q is missing (declared in import-schema)", importPath, paramName) - } - } - - value, provided := inputs[paramName] - if !provided { - continue - } - - // Skip type validation when type is not specified - declaredType, _ := paramDef["type"].(string) - if declaredType == "" { - continue - } - - // Validate type - if err := validateImportInputType(paramName, value, declaredType, paramDef, importPath); err != nil { - return err - } - } - return nil -} - -// validateObjectInput validates a 'with' value of type object against the -// one-level deep 'properties' declared in the import-schema. -func validateObjectInput(name string, value any, paramDef map[string]any, importPath string) error { - objMap, ok := value.(map[string]any) - if !ok { - return fmt.Errorf("import '%s': 'with' input %q must be an object (got %T)", importPath, name, value) - } - propsAny, hasProps := paramDef["properties"] - if !hasProps { - return nil // no schema for properties - accept any object - } - propsMap, ok := propsAny.(map[string]any) - if !ok { - return nil - } - // Check for unknown sub-keys - for subKey := range objMap { - if _, declared := propsMap[subKey]; !declared { - return fmt.Errorf("import '%s': 'with' input %q has unknown property %q (not in import-schema)", importPath, name, subKey) - } - } - // Validate each declared property - for propName, propDefRaw := range propsMap { - propDef, _ := propDefRaw.(map[string]any) - // Check required sub-fields - if req, _ := propDef["required"].(bool); req { - if _, provided := objMap[propName]; !provided { - return fmt.Errorf("import '%s': required property %q of 'with' input %q is missing", importPath, propName, name) - } - } - subValue, provided := objMap[propName] - if !provided { - continue - } - propType, _ := propDef["type"].(string) - if propType == "" { - continue - } - qualifiedName := name + "." + propName - if err := validateImportInputType(qualifiedName, subValue, propType, propDef, importPath); err != nil { - return err - } - } - return nil -} - -// validateImportInputType checks that a single 'with' value matches the declared type. -func validateImportInputType(name string, value any, declaredType string, paramDef map[string]any, importPath string) error { - switch declaredType { - case "string": - if _, ok := value.(string); !ok { - return fmt.Errorf("import '%s': 'with' input %q must be a string (got %T)", importPath, name, value) - } - case "number": - // Accept all numeric types that YAML parsers may produce - switch value.(type) { - case int, int8, int16, int32, int64, - uint, uint8, uint16, uint32, uint64, - float32, float64: - // OK - default: - return fmt.Errorf("import '%s': 'with' input %q must be a number (got %T)", importPath, name, value) - } - case "boolean": - if _, ok := value.(bool); !ok { - return fmt.Errorf("import '%s': 'with' input %q must be a boolean (got %T)", importPath, name, value) - } - case "choice": - strVal, ok := value.(string) - if !ok { - return fmt.Errorf("import '%s': 'with' input %q must be a string for choice type (got %T)", importPath, name, value) - } - if opts, hasOpts := paramDef["options"]; hasOpts { - if optsList, ok := opts.([]any); ok { - for _, opt := range optsList { - if optStr, ok := opt.(string); ok && optStr == strVal { - return nil - } - } - return fmt.Errorf("import '%s': 'with' input %q value %q is not in the allowed options", importPath, name, strVal) - } - } - case "array": - arr, ok := value.([]any) - if !ok { - return fmt.Errorf("import '%s': 'with' input %q must be an array (got %T)", importPath, name, value) - } - // Validate item types if an 'items' schema is declared - itemsDefRaw, hasItems := paramDef["items"] - if !hasItems { - return nil - } - itemsDef, _ := itemsDefRaw.(map[string]any) - itemType, _ := itemsDef["type"].(string) - if itemType == "" { - return nil - } - for i, item := range arr { - itemName := fmt.Sprintf("%s[%d]", name, i) - if err := validateImportInputType(itemName, item, itemType, itemsDef, importPath); err != nil { - return err - } - } - case "object": - return validateObjectInput(name, value, paramDef, importPath) - } - return nil -} - -// applyImportSchemaDefaultsFromFrontmatter applies import-schema defaults from an -// already-parsed frontmatter map, avoiding a redundant YAML parse when the caller -// has already extracted the frontmatter. Returns a copy of inputs augmented with -// default values for any schema parameters declared with a "default" field but not -// present in the provided inputs map. Parameters already in inputs are left unchanged. -func applyImportSchemaDefaultsFromFrontmatter(frontmatter map[string]any, inputs map[string]any) map[string]any { - rawSchema, ok := frontmatter["import-schema"] - if !ok { - return inputs - } - schemaMap, ok := rawSchema.(map[string]any) - if !ok || len(schemaMap) == 0 { - return inputs - } - - // Check if there are any defaults to apply - avoid copying if not needed. - hasDefaults := false - for paramName, paramDefRaw := range schemaMap { - if _, provided := inputs[paramName]; provided { - continue - } - if paramDef, ok := paramDefRaw.(map[string]any); ok { - if _, hasDefault := paramDef["default"]; hasDefault { - hasDefaults = true - break - } - } - } - if !hasDefaults { - return inputs - } - - // Copy the inputs map and add defaults for unprovided parameters. - augmented := make(map[string]any, len(inputs)) - maps.Copy(augmented, inputs) - for paramName, paramDefRaw := range schemaMap { - if _, provided := augmented[paramName]; provided { - continue - } - paramDef, ok := paramDefRaw.(map[string]any) - if !ok { - continue - } - if defaultVal, hasDefault := paramDef["default"]; hasDefault { - augmented[paramName] = defaultVal - } - } - return augmented -} - -// importInputsExprRegex matches ${{ github.aw.import-inputs. }} and -// ${{ github.aw.import-inputs.. }} expressions in raw content. -var importInputsExprRegex = regexp.MustCompile(`\$\{\{\s*github\.aw\.import-inputs\.([a-zA-Z0-9_-]+(?:\.[a-zA-Z0-9_-]+)?)\s*\}\}`) - -// legacyInputsExprRegex matches ${{ github.aw.inputs. }} (legacy form) in raw content. -var legacyInputsExprRegex = regexp.MustCompile(`\$\{\{\s*github\.aw\.inputs\.([a-zA-Z0-9_-]+)\s*\}\}`) - -// substituteImportInputsInContent performs text-level substitution of -// ${{ github.aw.import-inputs.* }} and ${{ github.aw.inputs.* }} expressions -// in raw file content (including YAML frontmatter). This is called before YAML -// parsing so that array/object values serialised as JSON produce valid YAML. -func substituteImportInputsInContent(content string, inputs map[string]any) string { - if len(inputs) == 0 { - return content - } - - result := legacyInputsExprRegex.ReplaceAllStringFunc(content, buildImportInputReplaceFunc(legacyInputsExprRegex, inputs)) - result = importInputsExprRegex.ReplaceAllStringFunc(result, buildImportInputReplaceFunc(importInputsExprRegex, inputs)) - return result -} - -func buildImportInputReplaceFunc(regex *regexp.Regexp, inputs map[string]any) func(string) string { - return func(match string) string { - m := regex.FindStringSubmatch(match) - if len(m) < 2 { - return match - } - strVal, found := resolveImportInputPath(inputs, m[1]) - if found { - return strVal - } - return match - } -} - -func resolveImportInputPath(inputs map[string]any, inputPath string) (string, bool) { - value, ok := resolveImportInputValue(inputs, inputPath) - if !ok { - return "", false - } - return importinpututil.FormatResolvedValue(value) -} - -func resolveImportInputValue(inputs map[string]any, inputPath string) (any, bool) { - return importinpututil.ResolvePathValue(inputs, inputPath) -} diff --git a/pkg/parser/import_input_substitution.go b/pkg/parser/import_input_substitution.go new file mode 100644 index 00000000000..cee357486c5 --- /dev/null +++ b/pkg/parser/import_input_substitution.go @@ -0,0 +1,57 @@ +// Package parser provides functions for parsing and processing workflow markdown files. +// import_input_substitution.go implements text-level substitution of import-inputs +// expressions (${{ github.aw.import-inputs.* }}) in raw workflow file content. +package parser + +import ( + "regexp" + + "github.com/github/gh-aw/pkg/importinpututil" +) + +// importInputsExprRegex matches ${{ github.aw.import-inputs. }} and +// ${{ github.aw.import-inputs.. }} expressions in raw content. +var importInputsExprRegex = regexp.MustCompile(`\$\{\{\s*github\.aw\.import-inputs\.([a-zA-Z0-9_-]+(?:\.[a-zA-Z0-9_-]+)?)\s*\}\}`) + +// legacyInputsExprRegex matches ${{ github.aw.inputs. }} (legacy form) in raw content. +var legacyInputsExprRegex = regexp.MustCompile(`\$\{\{\s*github\.aw\.inputs\.([a-zA-Z0-9_-]+)\s*\}\}`) + +// substituteImportInputsInContent performs text-level substitution of +// ${{ github.aw.import-inputs.* }} and ${{ github.aw.inputs.* }} expressions +// in raw file content (including YAML frontmatter). This is called before YAML +// parsing so that array/object values serialised as JSON produce valid YAML. +func substituteImportInputsInContent(content string, inputs map[string]any) string { + if len(inputs) == 0 { + return content + } + + result := legacyInputsExprRegex.ReplaceAllStringFunc(content, buildImportInputReplaceFunc(legacyInputsExprRegex, inputs)) + result = importInputsExprRegex.ReplaceAllStringFunc(result, buildImportInputReplaceFunc(importInputsExprRegex, inputs)) + return result +} + +func buildImportInputReplaceFunc(regex *regexp.Regexp, inputs map[string]any) func(string) string { + return func(match string) string { + m := regex.FindStringSubmatch(match) + if len(m) < 2 { + return match + } + strVal, found := resolveImportInputPath(inputs, m[1]) + if found { + return strVal + } + return match + } +} + +func resolveImportInputPath(inputs map[string]any, inputPath string) (string, bool) { + value, ok := resolveImportInputValue(inputs, inputPath) + if !ok { + return "", false + } + return importinpututil.FormatResolvedValue(value) +} + +func resolveImportInputValue(inputs map[string]any, inputPath string) (any, bool) { + return importinpututil.ResolvePathValue(inputs, inputPath) +} diff --git a/pkg/parser/import_observability.go b/pkg/parser/import_observability.go new file mode 100644 index 00000000000..15090dc719d --- /dev/null +++ b/pkg/parser/import_observability.go @@ -0,0 +1,202 @@ +// Package parser provides functions for parsing and processing workflow markdown files. +// import_observability.go implements OTLP endpoint extraction and merging for imported +// workflow observability configurations. +package parser + +import ( + "encoding/json" + "maps" + + "github.com/github/gh-aw/pkg/setutil" +) + +// observabilityImportEndpoint is an endpoint entry used during import merging. +// Headers are kept as any (original format: string or map) so that the workflow +// package can later normalise both supported forms correctly. +type observabilityImportEndpoint struct { + URL string `json:"url"` + Headers any `json:"headers,omitempty"` +} + +// extractOTLPEndpointsFromObsMap reads the `otlp.endpoint` field from a raw +// observability map and returns all endpoint entries as observabilityImportEndpoints. +// Supports string, object, and array forms of the endpoint field. +// Top-level `headers` is only applied to the backward-compat string endpoint form. +func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEndpoint { + otlpAny, ok := obs["otlp"] + if !ok { + return nil + } + otlpMap, ok := otlpAny.(map[string]any) + if !ok { + return nil + } + + endpointRaw := otlpMap["endpoint"] + headersRaw := otlpMap["headers"] // only applies to the backward-compat string form + + var result []observabilityImportEndpoint + switch ep := endpointRaw.(type) { + case string: + if ep != "" { + entry := observabilityImportEndpoint{URL: ep} + if headersRaw != nil { + entry.Headers = headersRaw + } + result = append(result, entry) + } + case map[string]any: + if url, _ := ep["url"].(string); url != "" { + entry := observabilityImportEndpoint{URL: url} + if h, hasH := ep["headers"]; hasH { + entry.Headers = h + } + result = append(result, entry) + } + case []any: + for _, item := range ep { + itemMap, ok := item.(map[string]any) + if !ok { + continue + } + url, _ := itemMap["url"].(string) + if url == "" { + continue + } + entry := observabilityImportEndpoint{URL: url} + if h, hasH := itemMap["headers"]; hasH { + entry.Headers = h + } + result = append(result, entry) + } + } + return result +} + +// mergeObservabilityConfigs takes a slice of observability config JSON strings (one per +// import), extracts all OTLP endpoint entries from each (supporting string, object, and +// array forms), deduplicates by URL (first occurrence wins), and returns a single merged +// observability JSON string with all endpoints expressed as an array. Custom OTLP +// attributes are also merged across imports (first occurrence wins per key). +// Returns "" when no valid endpoints or attributes are found. +func mergeObservabilityConfigs(configs []string) string { + seen := make(map[string]struct { + }) + var allEndpoints []observabilityImportEndpoint + mergedAttrs := make(map[string]string) + var mergedGitHubApp map[string]any + + for i, cfgJSON := range configs { + if cfgJSON == "" { + continue + } + var obs map[string]any + if err := json.Unmarshal([]byte(cfgJSON), &obs); err != nil { + parserLog.Printf("Failed to unmarshal observability config from import %d during merge: %v", i, err) + continue + } + for _, e := range extractOTLPEndpointsFromObsMap(obs) { + if !setutil.Contains(seen, e.URL) { + seen[e.URL] = struct { + }{} + allEndpoints = append(allEndpoints, e) + } + } + for k, v := range extractOTLPAttributesFromObsMap(obs) { + if _, exists := mergedAttrs[k]; !exists { + mergedAttrs[k] = v + } + } + if mergedGitHubApp == nil { + mergedGitHubApp = extractOTLPGitHubAppFromObsMap(obs) + } + } + + if len(allEndpoints) == 0 && len(mergedAttrs) == 0 && mergedGitHubApp == nil { + return "" + } + + // Produce a merged config with the endpoint field as an array so that the + // workflow package's collectAllOTLPEndpoints handles it uniformly. Include + // any merged custom attributes so the orchestrator can propagate them. + otlpMap := map[string]any{} + if len(allEndpoints) > 0 { + otlpMap["endpoint"] = allEndpoints + } + if len(mergedAttrs) > 0 { + otlpMap["attributes"] = mergedAttrs + } + if mergedGitHubApp != nil { + otlpMap["github-app"] = mergedGitHubApp + } + merged := map[string]any{"otlp": otlpMap} + b, err := json.Marshal(merged) + if err != nil { + parserLog.Printf("Failed to marshal %d merged OTLP endpoints: %v", len(allEndpoints), err) + return "" + } + return string(b) +} + +func extractOTLPGitHubAppFromObsMap(obs map[string]any) map[string]any { + if obs == nil { + return nil + } + otlpAny, ok := obs["otlp"] + if !ok { + return nil + } + otlpMap, ok := otlpAny.(map[string]any) + if !ok { + return nil + } + githubAppAny, ok := otlpMap["github-app"] + if !ok { + return nil + } + githubAppMap, ok := githubAppAny.(map[string]any) + if !ok { + return nil + } + copyMap := make(map[string]any, len(githubAppMap)) + maps.Copy(copyMap, githubAppMap) + return copyMap +} + +// extractOTLPAttributesFromObsMap reads the custom OTLP attributes map from a +// raw observability section (as parsed from an import's frontmatter). Only +// string values are accepted; non-string values are silently ignored. +// Returns nil when the field is absent or empty. +// +// Note: this intentionally duplicates the logic of +// workflow.extractOTLPCustomAttributesFromObsMap. The parser package must not +// import the workflow package (circular-dependency risk), so the helper lives +// here as a local copy. Both implementations must stay in sync. +func extractOTLPAttributesFromObsMap(obs map[string]any) map[string]string { + if obs == nil { + return nil + } + otlpAny, ok := obs["otlp"] + if !ok { + return nil + } + otlpMap, ok := otlpAny.(map[string]any) + if !ok { + return nil + } + attrsAny, ok := otlpMap["attributes"] + if !ok { + return nil + } + attrsMap, ok := attrsAny.(map[string]any) + if !ok { + return nil + } + result := make(map[string]string, len(attrsMap)) + for k, v := range attrsMap { + if s, ok := v.(string); ok && k != "" { + result[k] = s + } + } + return result +} diff --git a/pkg/parser/import_schema_validation.go b/pkg/parser/import_schema_validation.go new file mode 100644 index 00000000000..aabacaecd21 --- /dev/null +++ b/pkg/parser/import_schema_validation.go @@ -0,0 +1,228 @@ +// Package parser provides functions for parsing and processing workflow markdown files. +// import_schema_validation.go validates 'with' inputs against import-schema declarations +// and applies default values from import-schema to missing inputs. +package parser + +import ( + "fmt" + "maps" +) + +// validateWithImportSchema validates the provided 'with'/'inputs' values against +// the 'import-schema' declared in the imported workflow's frontmatter. +// It checks that: +// - all required parameters declared in import-schema are present in 'with' +// - no unknown parameters are provided (i.e., not declared in import-schema) +// - provided values match the declared type (string, number, boolean, choice) +// - choice values are within the allowed options list +// +// If the imported workflow has no 'import-schema', all provided 'with' values are +// accepted without validation (backward compatibility with 'inputs' form). +func validateWithImportSchema(inputs map[string]any, fm map[string]any, importPath string) error { + rawSchema, hasSchema := fm["import-schema"] + if !hasSchema { + return nil + } + schemaMap, ok := rawSchema.(map[string]any) + if !ok { + return nil + } + if len(schemaMap) == 0 { + return nil + } + + // Check for unknown keys not declared in import-schema + for key := range inputs { + if _, declared := schemaMap[key]; !declared { + return fmt.Errorf("import '%s': unknown 'with' input %q is not declared in the import-schema", importPath, key) + } + } + + // Check each declared schema field + for paramName, paramDefRaw := range schemaMap { + paramDef, _ := paramDefRaw.(map[string]any) + + // Check required parameters + if req, _ := paramDef["required"].(bool); req { + if _, provided := inputs[paramName]; !provided { + return fmt.Errorf("import '%s': required 'with' input %q is missing (declared in import-schema)", importPath, paramName) + } + } + + value, provided := inputs[paramName] + if !provided { + continue + } + + // Skip type validation when type is not specified + declaredType, _ := paramDef["type"].(string) + if declaredType == "" { + continue + } + + // Validate type + if err := validateImportInputType(paramName, value, declaredType, paramDef, importPath); err != nil { + return err + } + } + return nil +} + +// validateObjectInput validates a 'with' value of type object against the +// one-level deep 'properties' declared in the import-schema. +func validateObjectInput(name string, value any, paramDef map[string]any, importPath string) error { + objMap, ok := value.(map[string]any) + if !ok { + return fmt.Errorf("import '%s': 'with' input %q must be an object (got %T)", importPath, name, value) + } + propsAny, hasProps := paramDef["properties"] + if !hasProps { + return nil // no schema for properties - accept any object + } + propsMap, ok := propsAny.(map[string]any) + if !ok { + return nil + } + // Check for unknown sub-keys + for subKey := range objMap { + if _, declared := propsMap[subKey]; !declared { + return fmt.Errorf("import '%s': 'with' input %q has unknown property %q (not in import-schema)", importPath, name, subKey) + } + } + // Validate each declared property + for propName, propDefRaw := range propsMap { + propDef, _ := propDefRaw.(map[string]any) + // Check required sub-fields + if req, _ := propDef["required"].(bool); req { + if _, provided := objMap[propName]; !provided { + return fmt.Errorf("import '%s': required property %q of 'with' input %q is missing", importPath, propName, name) + } + } + subValue, provided := objMap[propName] + if !provided { + continue + } + propType, _ := propDef["type"].(string) + if propType == "" { + continue + } + qualifiedName := name + "." + propName + if err := validateImportInputType(qualifiedName, subValue, propType, propDef, importPath); err != nil { + return err + } + } + return nil +} + +// validateImportInputType checks that a single 'with' value matches the declared type. +func validateImportInputType(name string, value any, declaredType string, paramDef map[string]any, importPath string) error { + switch declaredType { + case "string": + if _, ok := value.(string); !ok { + return fmt.Errorf("import '%s': 'with' input %q must be a string (got %T)", importPath, name, value) + } + case "number": + // Accept all numeric types that YAML parsers may produce + switch value.(type) { + case int, int8, int16, int32, int64, + uint, uint8, uint16, uint32, uint64, + float32, float64: + // OK + default: + return fmt.Errorf("import '%s': 'with' input %q must be a number (got %T)", importPath, name, value) + } + case "boolean": + if _, ok := value.(bool); !ok { + return fmt.Errorf("import '%s': 'with' input %q must be a boolean (got %T)", importPath, name, value) + } + case "choice": + strVal, ok := value.(string) + if !ok { + return fmt.Errorf("import '%s': 'with' input %q must be a string for choice type (got %T)", importPath, name, value) + } + if opts, hasOpts := paramDef["options"]; hasOpts { + if optsList, ok := opts.([]any); ok { + for _, opt := range optsList { + if optStr, ok := opt.(string); ok && optStr == strVal { + return nil + } + } + return fmt.Errorf("import '%s': 'with' input %q value %q is not in the allowed options", importPath, name, strVal) + } + } + case "array": + arr, ok := value.([]any) + if !ok { + return fmt.Errorf("import '%s': 'with' input %q must be an array (got %T)", importPath, name, value) + } + // Validate item types if an 'items' schema is declared + itemsDefRaw, hasItems := paramDef["items"] + if !hasItems { + return nil + } + itemsDef, _ := itemsDefRaw.(map[string]any) + itemType, _ := itemsDef["type"].(string) + if itemType == "" { + return nil + } + for i, item := range arr { + itemName := fmt.Sprintf("%s[%d]", name, i) + if err := validateImportInputType(itemName, item, itemType, itemsDef, importPath); err != nil { + return err + } + } + case "object": + return validateObjectInput(name, value, paramDef, importPath) + } + return nil +} + +// applyImportSchemaDefaultsFromFrontmatter applies import-schema defaults from an +// already-parsed frontmatter map, avoiding a redundant YAML parse when the caller +// has already extracted the frontmatter. Returns a copy of inputs augmented with +// default values for any schema parameters declared with a "default" field but not +// present in the provided inputs map. Parameters already in inputs are left unchanged. +func applyImportSchemaDefaultsFromFrontmatter(frontmatter map[string]any, inputs map[string]any) map[string]any { + rawSchema, ok := frontmatter["import-schema"] + if !ok { + return inputs + } + schemaMap, ok := rawSchema.(map[string]any) + if !ok || len(schemaMap) == 0 { + return inputs + } + + // Check if there are any defaults to apply - avoid copying if not needed. + hasDefaults := false + for paramName, paramDefRaw := range schemaMap { + if _, provided := inputs[paramName]; provided { + continue + } + if paramDef, ok := paramDefRaw.(map[string]any); ok { + if _, hasDefault := paramDef["default"]; hasDefault { + hasDefaults = true + break + } + } + } + if !hasDefaults { + return inputs + } + + // Copy the inputs map and add defaults for unprovided parameters. + augmented := make(map[string]any, len(inputs)) + maps.Copy(augmented, inputs) + for paramName, paramDefRaw := range schemaMap { + if _, provided := augmented[paramName]; provided { + continue + } + paramDef, ok := paramDefRaw.(map[string]any) + if !ok { + continue + } + if defaultVal, hasDefault := paramDef["default"]; hasDefault { + augmented[paramName] = defaultVal + } + } + return augmented +} From c62914e0b9813522cdadd2b51dba40ec9529d89a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 03:14:22 +0000 Subject: [PATCH 3/3] fix: address parser review feedback on formatting and docs Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/parser/import_observability.go | 6 ++---- pkg/parser/import_schema_validation.go | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/parser/import_observability.go b/pkg/parser/import_observability.go index 15090dc719d..6bdc048eb79 100644 --- a/pkg/parser/import_observability.go +++ b/pkg/parser/import_observability.go @@ -80,8 +80,7 @@ func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEnd // attributes are also merged across imports (first occurrence wins per key). // Returns "" when no valid endpoints or attributes are found. func mergeObservabilityConfigs(configs []string) string { - seen := make(map[string]struct { - }) + seen := make(map[string]struct{}) var allEndpoints []observabilityImportEndpoint mergedAttrs := make(map[string]string) var mergedGitHubApp map[string]any @@ -97,8 +96,7 @@ func mergeObservabilityConfigs(configs []string) string { } for _, e := range extractOTLPEndpointsFromObsMap(obs) { if !setutil.Contains(seen, e.URL) { - seen[e.URL] = struct { - }{} + seen[e.URL] = struct{}{} allEndpoints = append(allEndpoints, e) } } diff --git a/pkg/parser/import_schema_validation.go b/pkg/parser/import_schema_validation.go index aabacaecd21..9977aa3db4d 100644 --- a/pkg/parser/import_schema_validation.go +++ b/pkg/parser/import_schema_validation.go @@ -179,8 +179,9 @@ func validateImportInputType(name string, value any, declaredType string, paramD // applyImportSchemaDefaultsFromFrontmatter applies import-schema defaults from an // already-parsed frontmatter map, avoiding a redundant YAML parse when the caller -// has already extracted the frontmatter. Returns a copy of inputs augmented with -// default values for any schema parameters declared with a "default" field but not +// has already extracted the frontmatter. Returns the original inputs map unchanged +// when no import-schema defaults apply; otherwise returns a copy augmented with +// default values for schema parameters declared with a "default" field but not // present in the provided inputs map. Parameters already in inputs are left unchanged. func applyImportSchemaDefaultsFromFrontmatter(frontmatter map[string]any, inputs map[string]any) map[string]any { rawSchema, ok := frontmatter["import-schema"]