Conversation
…istic compilation When `inputs` are specified in the workflow frontmatter for a custom safe-output action, the compiler now uses them directly and skips the network fetch of action.yml. This prevents the lock file from alternating between the specific-inputs form and the generic-payload fallback form depending on network/auth availability at compile time. - Add `inputs` parsing in parseActionsConfig - Skip action.yml network fetch when inputs are pre-specified - Update JSON schema to allow `inputs` field on actions - Add explicit inputs to smoke-codex.md for add-smoked-label - Add TestParseActionsConfigWithInputs test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35231559-de42-47fb-97b6-4899334142a5
…lation Instead of requiring users to specify action inputs in the workflow frontmatter, extend the ActionCache (actions-lock.json) to store inputs alongside the SHA. Compilation now uses cached inputs when available, falling back to a network fetch only when needed, and storing the result for future deterministic compilations. - Revert smoke-codex.md to original (no manually-added inputs) - Add Inputs field to ActionCacheEntry in action_cache.go - Update Set() to preserve existing cached inputs - Add GetInputs() and SetInputs() methods - Update fetchAndParseActionYAML to check cache before network - Add inputs for actions-ecosystem/action-add-labels@v1 to actions-lock.json - Add TestActionCacheInputs test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ca74d9bc-5381-4c9d-b8c6-8a89e7175593
Done in 0636976. Reverted the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot fix tests https://github.com/github/gh-aw/actions/runs/23368141084/job/67986288345?pr=22048 review comments |
There was a problem hiding this comment.
Pull request overview
This PR aims to make smoke-codex.lock.yml compilation deterministic by avoiding network-dependent alternation in derived action input schemas, using cached inputs and an explicit frontmatter override.
Changes:
- Adds an
inputsescape hatch to safe-output action configuration (frontmatter + schema). - Extends the action cache (
actions-lock.json) to persist action input definitions and uses them before network fetches. - Updates lock/pin data and adds tests covering input parsing and cache behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_actions.go | Adds frontmatter inputs parsing and implements input-resolution priority (frontmatter → cache → network). |
| pkg/workflow/safe_outputs_actions_test.go | Adds test ensuring inputs in actions config are parsed correctly. |
| pkg/workflow/action_cache.go | Extends cache entries with Inputs and adds Get/Set APIs for cached inputs. |
| pkg/workflow/action_cache_test.go | Adds test coverage for caching/retrieving inputs and preserving them across Set(). |
| pkg/parser/schemas/main_workflow_schema.json | Adds safe-outputs.actions.*.inputs schema for deterministic/offline compilation. |
| pkg/workflow/data/action_pins.json | Adds inputs data to embedded pins file. |
| .github/aw/actions-lock.json | Adds cached inputs for actions-ecosystem/action-add-labels@v1. |
| .github/workflows/smoke-codex.lock.yml | Updates compiled metadata hash after recompilation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflow/safe_outputs_actions.go
Outdated
| 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 | ||
| // Skip the remote fetch; inputs are already populated. |
There was a problem hiding this comment.
When cached inputs are found, the code skips fetching action.yml entirely and never sets config.ActionDescription. This changes tool/step display names and tool descriptions compared to the network path. Consider caching the action description alongside inputs (or still fetching/parsing description when Inputs come from cache) so offline deterministic compiles keep the same descriptions.
| // Skip the remote fetch; inputs are already populated. | |
| // Inputs are populated from cache; still fetch action.yml to populate description | |
| // so that cached and non-cached paths produce consistent tool/step descriptions. | |
| actionYAML, err = fetchRemoteActionYAML(ref.Repo, ref.Subdir, fetchRef) | |
| if err != nil { | |
| safeOutputActionsLog.Printf("Warning: failed to fetch action.yml for description for %q (%s): %v", actionName, config.Uses, err) | |
| } else if actionYAML != nil && actionYAML.Description != "" { | |
| config.ActionDescription = actionYAML.Description | |
| } |
pkg/workflow/safe_outputs_actions.go
Outdated
| // 2. Inputs cached in the ActionCache (actions-lock.json) | ||
| // 3. Inputs fetched from the remote action.yml (result cached for future runs) | ||
| // | ||
| // The action reference is always pinned to a commit SHA for security. |
There was a problem hiding this comment.
The comment says the action reference is always pinned to a commit SHA, but GetActionPinWithData can return an empty pinned ref (no pin available) and the code then falls back to using config.Uses at step-generation time. Consider rewording this comment to reflect the actual behavior (pinned when available, otherwise falls back).
| // The action reference is always pinned to a commit SHA for security. | |
| // 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. |
pkg/workflow/action_cache.go
Outdated
| // Preserve any previously-cached inputs so that SetInputs calls are not lost | ||
| // when Set is called later (e.g. when the SHA is refreshed from the network). | ||
| // When no entry exists, `existing` is the zero value and existing.Inputs is nil, | ||
| // so the new entry simply has no cached inputs — which is the correct behavior. | ||
| existing := c.Entries[key] | ||
| c.Entries[key] = ActionCacheEntry{ | ||
| Repo: repo, | ||
| Version: version, | ||
| SHA: sha, | ||
| Inputs: existing.Inputs, |
There was a problem hiding this comment.
ActionCache.Set() now always preserves existing cached Inputs even when the SHA for the same repo@version key changes. If a moving tag (e.g., v1/v4) is refreshed to a new SHA, this can leave Inputs describing the old commit while the workflow is pinned to the new SHA. Consider clearing Inputs when the stored SHA changes, or keying inputs by SHA so they stay consistent with the pinned commit.
| // Preserve any previously-cached inputs so that SetInputs calls are not lost | |
| // when Set is called later (e.g. when the SHA is refreshed from the network). | |
| // When no entry exists, `existing` is the zero value and existing.Inputs is nil, | |
| // so the new entry simply has no cached inputs — which is the correct behavior. | |
| existing := c.Entries[key] | |
| c.Entries[key] = ActionCacheEntry{ | |
| Repo: repo, | |
| Version: version, | |
| SHA: sha, | |
| Inputs: existing.Inputs, | |
| // 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 | |
| if existing.SHA == sha { | |
| inputs = existing.Inputs | |
| } else if existing.Inputs != nil { | |
| 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, | |
| Inputs: inputs, |
pkg/workflow/action_cache.go
Outdated
| // If no cache entry exists for the key, this call is a no-op. | ||
| 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, skipping SetInputs", key) | ||
| return |
There was a problem hiding this comment.
SetInputs is currently a no-op when the cache entry is missing. That means a successful action.yml fetch may not actually persist inputs unless some earlier step already created the cache entry. Consider having SetInputs create the entry (with repo/version and existing SHA if known), or ensuring callers always create/set the entry before calling SetInputs so the determinism guarantee holds.
| // If no cache entry exists for the key, this call is a no-op. | |
| 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, skipping SetInputs", key) | |
| return | |
| // If no cache entry exists for the key, a new entry is created (with an empty SHA). | |
| 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 for inputs", key) | |
| entry = ActionCacheEntry{ | |
| Repo: repo, | |
| Version: version, | |
| } |
| }, | ||
| "additionalProperties": false | ||
| } | ||
| } |
There was a problem hiding this comment.
The new safe-outputs.actions.*.inputs schema uses patternProperties but does not set additionalProperties:false on the inputs object itself. As a result, keys that don't match the input-name regex are still permitted by default. Consider adding additionalProperties:false at the inputs object level to enforce the intended key pattern.
| } | |
| }, | |
| "additionalProperties": false |
pkg/workflow/data/action_pins.json
Outdated
| "sha": "18f1af5e3544586314bbe15c0273249c770b2daf", | ||
| "inputs": { | ||
| "github-token": { | ||
| "description": "The GitHub token for authentication.", | ||
| "required": false, | ||
| "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.", | ||
| "required": false | ||
| }, | ||
| "repo": { | ||
| "description": "The owner and name of the repository. Defaults to the current repository.", | ||
| "required": false, | ||
| "default": "${{ github.repository }}" | ||
| } | ||
| } |
There was a problem hiding this comment.
This JSON now includes an "inputs" field, but the Go types used to unmarshal embedded action_pins.json (ActionPin / ActionPinsData in pkg/workflow/action_pins.go) do not include Inputs, so the data is ignored at runtime. Either remove "inputs" from this file, or update the pin data model and pinning logic to actually consume it.
| "sha": "18f1af5e3544586314bbe15c0273249c770b2daf", | |
| "inputs": { | |
| "github-token": { | |
| "description": "The GitHub token for authentication.", | |
| "required": false, | |
| "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.", | |
| "required": false | |
| }, | |
| "repo": { | |
| "description": "The owner and name of the repository. Defaults to the current repository.", | |
| "required": false, | |
| "default": "${{ github.repository }}" | |
| } | |
| } | |
| "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" |
| SHA string `json:"sha"` | ||
| Inputs map[string]*ActionYAMLInput `json:"inputs,omitempty"` // cached inputs from action.yml | ||
| } | ||
|
|
There was a problem hiding this comment.
ActionCacheEntry.Inputs is persisted to actions-lock.json using encoding/json, but ActionYAMLInput currently has no json struct tags. This will cause newly-written cache files to emit fields like "Description"/"Required"/"Default" (capitalized) rather than the existing lowercase keys ("description", "required", "default") used in the repo JSON. Consider adding json tags to ActionYAMLInput (or a dedicated cache DTO) to keep the on-disk format stable and deterministic.
| // actionInputDTO is a dedicated cache representation for ActionYAMLInput | |
| // that preserves the existing lowercase JSON keys used on disk. | |
| type actionInputDTO struct { | |
| Description string `json:"description,omitempty"` | |
| Required bool `json:"required,omitempty"` | |
| Default string `json:"default,omitempty"` | |
| } | |
| // MarshalJSON customizes JSON encoding for ActionCacheEntry to ensure that | |
| // cached inputs are written using lowercase keys ("description", | |
| // "required", "default") to keep the on-disk format stable. | |
| func (e ActionCacheEntry) MarshalJSON() ([]byte, error) { | |
| // Build a DTO for serialization that mirrors the existing cache layout. | |
| type cacheEntryDTO struct { | |
| Repo string `json:"repo"` | |
| Version string `json:"version"` | |
| SHA string `json:"sha"` | |
| Inputs map[string]*actionInputDTO `json:"inputs,omitempty"` | |
| } | |
| dto := cacheEntryDTO{ | |
| Repo: e.Repo, | |
| Version: e.Version, | |
| SHA: e.SHA, | |
| } | |
| if len(e.Inputs) > 0 { | |
| dto.Inputs = make(map[string]*actionInputDTO, len(e.Inputs)) | |
| for name, input := range e.Inputs { | |
| if input == nil { | |
| continue | |
| } | |
| dto.Inputs[name] = &actionInputDTO{ | |
| Description: input.Description, | |
| Required: input.Required, | |
| Default: input.Default, | |
| } | |
| } | |
| } | |
| return json.Marshal(dto) | |
| } |
|
@copilot review comment |
- Add JSON tags to ActionYAMLInput (description, required, default) so inputs serialized to actions-lock.json use lowercase keys - Fix Set() to clear cached inputs when SHA changes (stale inputs would describe a different commit) - Fix SetInputs() to create entry if none exists (was silently dropping network-fetched inputs when no cache entry existed) - Add ActionDescription field to ActionCacheEntry to cache action description alongside inputs for consistent offline compilation - Add GetActionDescription/SetActionDescription methods - Update fetchAndParseActionYAML to restore and cache description - Add Inputs field to ActionPin so action_pins.json data is parsed rather than silently ignored by the JSON decoder - Add additionalProperties: false to inputs schema object - Fix actions-lock.json to use lowercase JSON keys for inputs - Update TestActionCacheInputs to match new SHA-change/create-entry behavior Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/54823ddb-6e1b-4c40-ad02-b2d62756aea2
Fixed in 08384da. Addressed all 7 review comments:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Note
🔒 Integrity filtering filtered 2 items
Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
💥 [THE END] — Illustrated by Smoke Claude
Summary
Fixes the compiler so
smoke-codex.lock.ymlstops alternating between two forms on different branches.Root Cause
The compiler fetches
action.ymlfrom GitHub at compile time to derive the input schema for custom safe-output actions. When the network/auth is available it produces a specific-inputs form (additionalProperties: false, withlabelsandnumberfields). When the fetch fails (no auth, network timeout), it falls back to a genericpayloadfield (additionalProperties: true). This makes the lock file non-deterministic.Fix
Extended
ActionCacheEntry(inactions-lock.json) with aninputsfield. The compiler now resolves inputs using the following priority:actions-lock.jsonaction.yml(result stored in cache for future deterministic compilations)The action reference is always pinned to a commit SHA for security.
Changes
pkg/workflow/action_cache.go: AddedInputsfield toActionCacheEntry; updatedSet()to preserve existing cached inputs; addedGetInputs()andSetInputs()methodspkg/workflow/safe_outputs_actions.go:fetchAndParseActionYAMLchecks theActionCachefor cached inputs before making a network request; on a successful network fetch, inputs are stored in the cachepkg/parser/schemas/main_workflow_schema.json: Addedinputsfield to the action configuration schema (frontmatter escape hatch).github/aw/actions-lock.json+pkg/workflow/data/action_pins.json: Added inputs foractions-ecosystem/action-add-labels@v1pkg/workflow/safe_outputs_actions_test.go: AddedTestParseActionsConfigWithInputstestpkg/workflow/action_cache_test.go: AddedTestActionCacheInputstest.github/workflows/smoke-codex.lock.yml: Recompiled using cached inputsSecurity Summary
No security vulnerabilities introduced or found. CodeQL analysis passed with 0 alerts.
⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.
✨ PR Review Safe Output Test - Run 23368377758
Note
🔒 Integrity filtering filtered 2 items
Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)