Fix input contract: user-provided context/dockerfile override service_name resolution - #5
Merged
Merged
Conversation
…_name resolution The resolve step always emitted resolved_context=code (even with no service_name or as a safety-net on early exit), which made the build step's `steps.skyhook_config.outputs.resolved_context || inputs.context` chain ignore inputs.context entirely. Changes: - Drop default values from `context` and `dockerfile` inputs so empty means "not provided" (defaults still effective via build-step fallback). - Gate the resolve step on service_name being set AND at least one of context/dockerfile being unset. - Only emit resolved_context/resolved_dockerfile when actually resolved from skyhook.yaml (no `code` safety-net on early exit). - Flip build step precedence to `inputs.X || resolved_X || default` so user-provided inputs win over service_name-derived values. - Update step summary to reflect the same precedence and tag the source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.5.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
context/dockerfileactually take effect whenservice_nameis also set.resolved_context=code, which was makinginputs.contextdead code.inputs.X || resolved_X || default.Reviewer guide
Where to start:
action.ymlis the only changed file.action.yml:45-50- droppeddefault: '.'anddefault: 'Dockerfile'fromcontext/dockerfileinputs so empty == "not provided". Defaults still apply via the build-step fallback below.action.yml:195-253- resolve step now gated onservice_name != '' && (context == '' || dockerfile == ''). Eachresolved_*output is only emitted when the corresponding user input is empty.action.yml:347-348- build step precedence flipped:inputs.X || resolved_X || 'default'.action.yml:399-422- step summary mirrors the same precedence; source tag is now_(from input)_vs_(from config)_.What's risky: the
resolved_context=codesafety-net previously emitted on early exit (commit 56d501c) is gone. If a caller passesservice_namebut the config file or service is missing, the build now falls back toinputs.context || '.'instead of the implicitcode/. The existing warning ("Falling back to input parameters") now matches behavior; previously it lied. Callers in that state should either fix their config or pass explicitcontext: code.What's mechanical: the description text on the two inputs.
Test plan
service_nameonly and valid config -> resolves from.skyhook/skyhook.yaml(existing behavior).service_name+ explicitcontext/dockerfile-> user inputs win (the bug being fixed).service_nameand no inputs -> defaults to./Dockerfile.service_namebut missing config -> warning + falls back to inputs (or defaults).🤖 Generated with Claude Code
Note
Medium Risk
Changes how build
context/fileare selected, which can alter what gets built for existing workflows (especially whenservice_nameis set but config is missing/mismatched). Scope is limited toaction.ymlbut impacts all callers of the composite action.Overview
Fixes the composite action’s input contract so user-provided
context/dockerfiletake precedence over values derived fromservice_name(and defaults apply only when neither input nor config provides a value).The
.skyhook/skyhook.yamlresolve step now runs only when needed and only emitsresolved_context/resolved_dockerfilewhen the corresponding input is empty, and the job summary now reports whether each value came from input vs config.Reviewed by Cursor Bugbot for commit d9632bf. Bugbot is set up for automated code reviews on this repo. Configure here.