diff --git a/.github/aw/actions-lock.json b/.github/aw/actions-lock.json index 743652c946..dd862fd68f 100644 --- a/.github/aw/actions-lock.json +++ b/.github/aw/actions-lock.json @@ -3,7 +3,25 @@ "actions-ecosystem/action-add-labels@v1": { "repo": "actions-ecosystem/action-add-labels", "version": "v1", - "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" + "sha": "18f1af5e3544586314bbe15c0273249c770b2daf", + "inputs": { + "github-token": { + "description": "The GitHub token for authentication.", + "default": "${{ github.token }}" + }, + "labels": { + "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", + "required": true + }, + "number": { + "description": "The number of the issue or pull request." + }, + "repo": { + "description": "The owner and name of the repository. Defaults to the current repository.", + "default": "${{ github.repository }}" + } + }, + "action_description": "A GitHub Action to add GitHub labels to pull requests and issues" }, "actions/ai-inference@v2.0.7": { "repo": "actions/ai-inference", diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 7201204915..2b9da5373f 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -455,13 +455,20 @@ jobs: { "description": "Add the 'smoked' label to the current pull request (can only be called once)", "inputSchema": { - "additionalProperties": true, + "additionalProperties": false, "properties": { - "payload": { - "description": "JSON-encoded payload to pass to the action", + "labels": { + "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", + "type": "string" + }, + "number": { + "description": "The number of the issue or pull request.", "type": "string" } }, + "required": [ + "labels" + ], "type": "object" }, "name": "add_smoked_label" @@ -1566,7 +1573,8 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} with: - payload: ${{ steps.process_safe_outputs.outputs.action_add_smoked_label_payload }} + labels: ${{ fromJSON(steps.process_safe_outputs.outputs.action_add_smoked_label_payload).labels }} + number: ${{ fromJSON(steps.process_safe_outputs.outputs.action_add_smoked_label_payload).number }} - name: Upload safe output items if: always() uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 07dc039d62..30e749183f 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -7871,6 +7871,32 @@ "type": "string" }, "examples": [{ "GITHUB_TOKEN": "${{ github.token }}" }] + }, + "inputs": { + "type": "object", + "description": "Explicit input definitions for the action, overriding the auto-fetched action.yml schema. Use this to ensure deterministic compilation without requiring network access at compile time.", + "patternProperties": { + "^[a-zA-Z_][a-zA-Z0-9_-]*$": { + "type": "object", + "description": "Definition for a single action input.", + "properties": { + "description": { + "type": "string", + "description": "Description of the input." + }, + "required": { + "type": "boolean", + "description": "Whether the input is required." + }, + "default": { + "type": "string", + "description": "Default value for the input." + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false } }, "required": ["uses"], diff --git a/pkg/workflow/action_cache.go b/pkg/workflow/action_cache.go index ec43318b27..989747eee9 100644 --- a/pkg/workflow/action_cache.go +++ b/pkg/workflow/action_cache.go @@ -20,9 +20,11 @@ const ( // ActionCacheEntry represents a cached action pin resolution. type ActionCacheEntry struct { - Repo string `json:"repo"` - Version string `json:"version"` - SHA string `json:"sha"` + Repo string `json:"repo"` + Version string `json:"version"` + SHA string `json:"sha"` + Inputs map[string]*ActionYAMLInput `json:"inputs,omitempty"` // cached inputs from action.yml + ActionDescription string `json:"action_description,omitempty"` // cached description from action.yml } // ActionCache manages cached action pin resolutions. @@ -189,7 +191,9 @@ func (c *ActionCache) FindEntryBySHA(repo, sha string) (ActionCacheEntry, bool) return ActionCacheEntry{}, false } -// Set stores a new cache entry +// Set stores a new cache entry, preserving any already-cached inputs when the SHA +// is unchanged. If the SHA changes (e.g. a moving tag points to a new commit), +// cached inputs are cleared to stay consistent with the newly-pinned commit. func (c *ActionCache) Set(repo, version, sha string) { key := formatActionCacheKey(repo, version) @@ -208,14 +212,101 @@ func (c *ActionCache) Set(repo, version, sha string) { } actionCacheLog.Printf("Setting cache entry: key=%s, sha=%s", key, sha) + + // Preserve previously-cached inputs only when the SHA is unchanged. If the SHA + // changes (e.g. for a moving tag that now points to a new commit), drop any + // existing inputs so they stay consistent with the pinned commit. + existing := c.Entries[key] + var inputs map[string]*ActionYAMLInput + var description string + if existing.SHA == sha { + inputs = existing.Inputs + description = existing.ActionDescription + } else if existing.SHA != "" { + // Log when an existing entry's SHA is being changed (covers both the case + // where cached inputs exist and where they don't, for consistent observability). + actionCacheLog.Printf("Clearing cached inputs for key=%s due to SHA change (%s -> %s)", key, existing.SHA, sha) + } c.Entries[key] = ActionCacheEntry{ - Repo: repo, - Version: version, - SHA: sha, + Repo: repo, + Version: version, + SHA: sha, + Inputs: inputs, + ActionDescription: description, } c.dirty = true // Mark cache as modified } +// GetInputs retrieves the cached action inputs for the given repo and version. +// Returns the inputs map and true if cached inputs exist, otherwise nil and false. +func (c *ActionCache) GetInputs(repo, version string) (map[string]*ActionYAMLInput, bool) { + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists || entry.Inputs == nil { + actionCacheLog.Printf("No cached inputs for key=%s", key) + return nil, false + } + actionCacheLog.Printf("Cache hit for inputs: key=%s, inputs=%d", key, len(entry.Inputs)) + return entry.Inputs, true +} + +// SetInputs stores the action inputs in the cache entry for the given repo and version. +// If no cache entry exists for the key, a new entry is created with an empty SHA so that +// inputs fetched from the network are persisted even before the SHA is resolved. +func (c *ActionCache) SetInputs(repo, version string, inputs map[string]*ActionYAMLInput) { + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists { + actionCacheLog.Printf("No cache entry for key=%s, creating new entry to store inputs", key) + entry = ActionCacheEntry{ + Repo: repo, + Version: version, + } + } + entry.Inputs = inputs + c.Entries[key] = entry + c.dirty = true + actionCacheLog.Printf("Cached inputs for key=%s, inputs=%d", key, len(inputs)) +} + +// GetActionDescription retrieves the cached action description for the given repo and version. +// Returns the description and true if a non-empty description is cached, otherwise "" and false. +func (c *ActionCache) GetActionDescription(repo, version string) (string, bool) { + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists || entry.ActionDescription == "" { + return "", false + } + return entry.ActionDescription, true +} + +// SetActionDescription stores the action description in the cache entry for the given repo and version. +// If no cache entry exists for the key, a new entry is created. +// Empty descriptions are not stored; actions without a description string are treated the same as +// actions whose description has not yet been fetched, so we avoid caching an empty string that +// would prevent a later fetch from populating the field. +func (c *ActionCache) SetActionDescription(repo, version, description string) { + if description == "" { + // Skip persisting empty descriptions; callers that want to distinguish + // "no description fetched" from "action has no description" should use + // a sentinel value. For our use case (action.yml display text), omitting + // empty values is intentional to keep the cache file tidy. + return + } + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists { + entry = ActionCacheEntry{ + Repo: repo, + Version: version, + } + } + entry.ActionDescription = description + c.Entries[key] = entry + c.dirty = true + actionCacheLog.Printf("Cached description for key=%s", key) +} + // GetCachePath returns the path to the cache file func (c *ActionCache) GetCachePath() string { return c.path diff --git a/pkg/workflow/action_cache_test.go b/pkg/workflow/action_cache_test.go index 9475af5397..f5bf5acb58 100644 --- a/pkg/workflow/action_cache_test.go +++ b/pkg/workflow/action_cache_test.go @@ -545,3 +545,70 @@ func TestActionCacheFindEntryBySHA(t *testing.T) { t.Error("Expected not to find entry for different repo") } } + +// TestActionCacheInputs tests caching and retrieving action inputs +func TestActionCacheInputs(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + cache := NewActionCache(tmpDir) + cache.Set("owner/repo", "v1", "abc123sha456789012345678901234567890123") + + // Initially no inputs cached + inputs, ok := cache.GetInputs("owner/repo", "v1") + if ok { + t.Error("Expected no cached inputs, got some") + } + if inputs != nil { + t.Error("Expected nil inputs, got non-nil") + } + + // Store inputs + toCache := map[string]*ActionYAMLInput{ + "labels": {Description: "Labels to add.", Required: true}, + "number": {Description: "PR number."}, + } + cache.SetInputs("owner/repo", "v1", toCache) + + // Retrieve inputs + inputs, ok = cache.GetInputs("owner/repo", "v1") + if !ok { + t.Fatal("Expected cached inputs to exist") + } + if len(inputs) != 2 { + t.Errorf("Expected 2 inputs, got %d", len(inputs)) + } + if inputs["labels"] == nil || !inputs["labels"].Required { + t.Error("Expected 'labels' input to be required") + } + + // Set with the SAME SHA - inputs must be preserved + cache.Set("owner/repo", "v1", "abc123sha456789012345678901234567890123") + inputs, ok = cache.GetInputs("owner/repo", "v1") + if !ok { + t.Error("Expected inputs to survive Set() with same SHA") + } + if len(inputs) != 2 { + t.Errorf("Expected inputs to be preserved after Set() with same SHA, got %d", len(inputs)) + } + + // Set with a NEW SHA - inputs must be cleared (stale inputs no longer match pinned commit) + cache.Set("owner/repo", "v1", "newsha456789012345678901234567890123456") + inputs, ok = cache.GetInputs("owner/repo", "v1") + if ok { + t.Error("Expected inputs to be cleared after Set() with new SHA") + } + if inputs != nil { + t.Error("Expected nil inputs after SHA change, got non-nil") + } + + // SetInputs on a missing key now creates a new entry + cache.SetInputs("owner/repo", "v99", map[string]*ActionYAMLInput{ + "x": {Description: "x"}, + }) + inputs, ok = cache.GetInputs("owner/repo", "v99") + if !ok { + t.Error("Expected SetInputs on missing key to create entry") + } + if len(inputs) != 1 || inputs["x"] == nil { + t.Error("Expected created entry to have the given inputs") + } +} diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index 7a93a8329c..043efc10d5 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -33,9 +33,10 @@ func formatActionCacheKey(repo, version string) string { // ActionPin represents a pinned GitHub Action with its commit SHA type ActionPin struct { - Repo string `json:"repo"` // e.g., "actions/checkout" - Version string `json:"version"` // e.g., "v5" - the golden/default version - SHA string `json:"sha"` // Full commit SHA for the pinned version + Repo string `json:"repo"` // e.g., "actions/checkout" + Version string `json:"version"` // e.g., "v5" - the golden/default version + SHA string `json:"sha"` // Full commit SHA for the pinned version + Inputs map[string]*ActionYAMLInput `json:"inputs,omitempty"` // optional cached inputs (not used for SHA pinning) } // ActionPinsData represents the structure of the embedded JSON file diff --git a/pkg/workflow/data/action_pins.json b/pkg/workflow/data/action_pins.json index 743652c946..dd862fd68f 100644 --- a/pkg/workflow/data/action_pins.json +++ b/pkg/workflow/data/action_pins.json @@ -3,7 +3,25 @@ "actions-ecosystem/action-add-labels@v1": { "repo": "actions-ecosystem/action-add-labels", "version": "v1", - "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" + "sha": "18f1af5e3544586314bbe15c0273249c770b2daf", + "inputs": { + "github-token": { + "description": "The GitHub token for authentication.", + "default": "${{ github.token }}" + }, + "labels": { + "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", + "required": true + }, + "number": { + "description": "The number of the issue or pull request." + }, + "repo": { + "description": "The owner and name of the repository. Defaults to the current repository.", + "default": "${{ github.repository }}" + } + }, + "action_description": "A GitHub Action to add GitHub labels to pull requests and issues" }, "actions/ai-inference@v2.0.7": { "repo": "actions/ai-inference", diff --git a/pkg/workflow/safe_outputs_actions.go b/pkg/workflow/safe_outputs_actions.go index 9f76c3881c..ab9cf21a89 100644 --- a/pkg/workflow/safe_outputs_actions.go +++ b/pkg/workflow/safe_outputs_actions.go @@ -34,9 +34,9 @@ type SafeOutputActionConfig struct { // ActionYAMLInput holds an input definition parsed from a GitHub Action's action.yml. type ActionYAMLInput struct { - Description string `yaml:"description,omitempty"` - Required bool `yaml:"required,omitempty"` - Default string `yaml:"default,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Required bool `yaml:"required,omitempty" json:"required,omitempty"` + Default string `yaml:"default,omitempty" json:"default,omitempty"` } // actionYAMLFile is the parsed structure of a GitHub Action's action.yml. @@ -92,6 +92,24 @@ func parseActionsConfig(actionsMap map[string]any) map[string]*SafeOutputActionC } } } + if inputsMap, ok := actionConfigMap["inputs"].(map[string]any); ok { + actionConfig.Inputs = make(map[string]*ActionYAMLInput, len(inputsMap)) + for inputName, inputValue := range inputsMap { + inputDef := &ActionYAMLInput{} + if inputDefMap, ok := inputValue.(map[string]any); ok { + if desc, ok := inputDefMap["description"].(string); ok { + inputDef.Description = desc + } + if req, ok := inputDefMap["required"].(bool); ok { + inputDef.Required = req + } + if def, ok := inputDefMap["default"].(string); ok { + inputDef.Default = def + } + } + actionConfig.Inputs[inputName] = inputDef + } + } if actionConfig.Uses == "" { safeOutputActionsLog.Printf("Warning: action %q is missing required 'uses' field, skipping", actionName) @@ -145,6 +163,14 @@ func parseActionUsesField(uses string) (*actionRef, error) { // fetchAndParseActionYAML resolves the inputs and description from the action.yml // for each configured action. Results are stored in the action config's computed fields. // This function should be called before tool generation and step generation. +// +// Resolution priority (highest wins): +// 1. Inputs already specified in the frontmatter (config.Inputs != nil) +// 2. Inputs cached in the ActionCache (actions-lock.json) +// 3. Inputs fetched from the remote action.yml (result cached for future runs) +// +// When available, the action reference is pinned to a commit SHA for security; +// if no pin is available, later step generation falls back to the original config.Uses. func (c *Compiler) fetchAndParseActionYAML(actionName string, config *SafeOutputActionConfig, markdownPath string, data *WorkflowData) { if config.Uses == "" { return @@ -156,17 +182,23 @@ func (c *Compiler) fetchAndParseActionYAML(actionName string, config *SafeOutput return } + // Remember whether inputs were provided via frontmatter so we can skip lower- + // priority resolution paths. + inputsFromFrontmatter := config.Inputs != nil + var actionYAML *actionYAMLFile var resolvedRef string if ref.IsLocal { - actionYAML, err = readLocalActionYAML(ref.LocalPath, markdownPath) - if err != nil { - safeOutputActionsLog.Printf("Warning: failed to read local action.yml for %q at %s: %v", actionName, ref.LocalPath, err) + if !inputsFromFrontmatter { + actionYAML, err = readLocalActionYAML(ref.LocalPath, markdownPath) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to read local action.yml for %q at %s: %v", actionName, ref.LocalPath, err) + } } resolvedRef = config.Uses // local paths stay as-is } else { - // Pin the action ref and fetch the action.yml + // Pin the action ref for security. pinned, pinErr := GetActionPinWithData(ref.Repo, ref.Ref, data) var fetchRef string if pinErr != nil { @@ -185,15 +217,44 @@ func (c *Compiler) fetchAndParseActionYAML(actionName string, config *SafeOutput } } - actionYAML, err = fetchRemoteActionYAML(ref.Repo, ref.Subdir, fetchRef) - if err != nil { - safeOutputActionsLog.Printf("Warning: failed to fetch action.yml for %q (%s): %v", actionName, config.Uses, err) + if !inputsFromFrontmatter { + // Check the ActionCache for previously-fetched inputs before going to the network. + // The cache key uses the original version tag from the `uses:` field (ref.Ref, e.g. + // "v1") which matches the key stored in actions-lock.json. + if data.ActionCache != nil { + if cachedInputs, ok := data.ActionCache.GetInputs(ref.Repo, ref.Ref); ok { + safeOutputActionsLog.Printf("Using cached inputs for %q (%s@%s)", actionName, ref.Repo, ref.Ref) + config.Inputs = cachedInputs + } + if cachedDesc, ok := data.ActionCache.GetActionDescription(ref.Repo, ref.Ref); ok { + config.ActionDescription = cachedDesc + } + } + + // If inputs are still not resolved, fetch action.yml from the network and + // store the result in the cache to make future compilations deterministic. + if config.Inputs == nil { + actionYAML, err = fetchRemoteActionYAML(ref.Repo, ref.Subdir, fetchRef) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to fetch action.yml for %q (%s): %v", actionName, config.Uses, err) + } + // Cache the fetched inputs and description so subsequent compilations are + // deterministic even when the network is unavailable. + if actionYAML != nil && data.ActionCache != nil { + if actionYAML.Inputs != nil { + data.ActionCache.SetInputs(ref.Repo, ref.Ref, actionYAML.Inputs) + } + data.ActionCache.SetActionDescription(ref.Repo, ref.Ref, actionYAML.Description) + } + } } } config.ResolvedRef = resolvedRef - if actionYAML != nil { + // Only overwrite Inputs/ActionDescription from action.yml when the inputs were + // not already provided via frontmatter or cache. + if !inputsFromFrontmatter && config.Inputs == nil && actionYAML != nil { config.Inputs = actionYAML.Inputs config.ActionDescription = actionYAML.Description } diff --git a/pkg/workflow/safe_outputs_actions_test.go b/pkg/workflow/safe_outputs_actions_test.go index e9687edcb1..d309daf19c 100644 --- a/pkg/workflow/safe_outputs_actions_test.go +++ b/pkg/workflow/safe_outputs_actions_test.go @@ -442,7 +442,42 @@ func TestParseActionsConfigWithEnv(t *testing.T) { assert.Equal(t, "other-value", myTool.Env["OTHER_VAR"], "OTHER_VAR should match") } -// TestBuildActionStepsWithEnv verifies that env vars are emitted in generated steps +// TestParseActionsConfigWithInputs verifies parsing of inputs field in actions config +func TestParseActionsConfigWithInputs(t *testing.T) { + actionsMap := map[string]any{ + "add-label": map[string]any{ + "uses": "actions-ecosystem/action-add-labels@v1", + "inputs": map[string]any{ + "labels": map[string]any{ + "description": "The labels' name to be added.", + "required": true, + }, + "number": map[string]any{ + "description": "The number of the issue or pull request.", + }, + }, + }, + } + + result := parseActionsConfig(actionsMap) + require.Len(t, result, 1, "Should have one action") + + tool := result["add-label"] + require.NotNil(t, tool, "Should have add-label") + require.NotNil(t, tool.Inputs, "Should have inputs populated from frontmatter") + require.Len(t, tool.Inputs, 2, "Should have 2 inputs") + + labelsInput := tool.Inputs["labels"] + require.NotNil(t, labelsInput, "Should have labels input") + assert.Equal(t, "The labels' name to be added.", labelsInput.Description, "Labels description should match") + assert.True(t, labelsInput.Required, "Labels should be required") + + numberInput := tool.Inputs["number"] + require.NotNil(t, numberInput, "Should have number input") + assert.Equal(t, "The number of the issue or pull request.", numberInput.Description, "Number description should match") + assert.False(t, numberInput.Required, "Number should not be required") +} + func TestBuildActionStepsWithEnv(t *testing.T) { compiler := NewCompiler() data := &WorkflowData{