Summary
.github/instructions/github-actions-ci-cd-best-practices.instructions.md §4
says:
Explicitly declare type, required, and default for every input in
workflow_call.
Two shipped workflows in this repository contradict that rule, for two
different and independently correct reasons. It is a mandatory instruction
surface, so an agent following it would "fix" both of them back into defects.
Evidence
Audit of all 10 workflows here that declare workflow_call:
| Shape |
Workflows |
Satisfies "default for every input" |
Pure workflow_call, has inputs |
trunk.yml, release-prepare.yml, zsh-ci.yml |
Yes |
Pure workflow_call, has inputs |
zsh-lint.yml |
No: 2 required inputs, 0 defaults |
| Dual-trigger, no inputs |
labels-sync-test.yml, labeler-config-audit-test.yml, repo-settings-audit-test.yml, lychee.yml |
Vacuously |
| Dual-trigger, has inputs |
commit-lint.yml |
Cannot, meaningfully |
A required input cannot carry a default
zsh-lint.yml declares files and zsh-lint-sha as required: true. A
default on a required input is contradictory: the caller must supply the value,
so the default is unreachable. This is the org's flagship reusable workflow and
the subject of the #543 versioning pilot, with two live callers.
A dual-trigger workflow's input defaults are dead on the direct path
commit-lint.yml declares workflow_call alongside pull_request. GitHub's
contexts documentation states the inputs context contains "the inputs of a
reusable or manually triggered workflow", and a pull_request run is neither.
The context is empty there and the workflow_call defaults are never applied.
#586 removed the defaults for exactly this reason and moved the operative value
into the job step as : "${VAR:=...}". Leaving them in place created two
sources of truth where the documented-looking one was dead on the live path,
which is how the empty-pattern trap in #586 arose: an empty grep -E pattern
matches every line, so every commit would have been flagged and every branch
silently passed.
#597 now asserts, as a test, that no such default exists.
Proposed change
Replace the single bullet with three clauses:
- Declare
type and required for every workflow_call input.
- Declare
default only for optional inputs. A required: true input must not
carry one.
- When a workflow is also triggered directly (
push, pull_request,
schedule), put the operative fallback in the job step. workflow_call
input defaults are not applied on those runs, so a default declared on the
input is dead text on the path the workflow actually takes.
Scope
Instruction surface only. No workflow changes: the two workflows are already
correct and it is the rule that is over-stated.
runbooks/instruction-update.md requires the seven-question impact review in
the pull-request body, which the implementing PR will carry.
Found while adding #597.
Summary
.github/instructions/github-actions-ci-cd-best-practices.instructions.md§4says:
Two shipped workflows in this repository contradict that rule, for two
different and independently correct reasons. It is a mandatory instruction
surface, so an agent following it would "fix" both of them back into defects.
Evidence
Audit of all 10 workflows here that declare
workflow_call:workflow_call, has inputstrunk.yml,release-prepare.yml,zsh-ci.ymlworkflow_call, has inputszsh-lint.ymllabels-sync-test.yml,labeler-config-audit-test.yml,repo-settings-audit-test.yml,lychee.ymlcommit-lint.ymlA required input cannot carry a default
zsh-lint.ymldeclaresfilesandzsh-lint-shaasrequired: true. Adefault on a required input is contradictory: the caller must supply the value,
so the default is unreachable. This is the org's flagship reusable workflow and
the subject of the #543 versioning pilot, with two live callers.
A dual-trigger workflow's input defaults are dead on the direct path
commit-lint.ymldeclaresworkflow_callalongsidepull_request. GitHub'scontexts documentation states the
inputscontext contains "the inputs of areusable or manually triggered workflow", and a
pull_requestrun is neither.The context is empty there and the
workflow_calldefaults are never applied.#586 removed the defaults for exactly this reason and moved the operative value
into the job step as
: "${VAR:=...}". Leaving them in place created twosources of truth where the documented-looking one was dead on the live path,
which is how the empty-pattern trap in #586 arose: an empty
grep -Epatternmatches every line, so every commit would have been flagged and every branch
silently passed.
#597 now asserts, as a test, that no such default exists.
Proposed change
Replace the single bullet with three clauses:
typeandrequiredfor everyworkflow_callinput.defaultonly for optional inputs. Arequired: trueinput must notcarry one.
push,pull_request,schedule), put the operative fallback in the job step.workflow_callinput defaults are not applied on those runs, so a default declared on the
input is dead text on the path the workflow actually takes.
Scope
Instruction surface only. No workflow changes: the two workflows are already
correct and it is the rule that is over-stated.
runbooks/instruction-update.mdrequires the seven-question impact review inthe pull-request body, which the implementing PR will carry.
Found while adding #597.