Make unsupported workflow settings actionable - #383
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-01a031f0-ff2d-71b8-ba0e-5e961fb18f3b Co-authored-by: Chris Campbell <chris.c@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a031f0-ff2d-71b8-ba0e-5e961fb18f3b Co-authored-by: Chris Campbell <chris.c@buildkite.com>
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
The targeted environment and shorthand diagnostics retain their source and job attribution. One malformed scalar edge case is noted inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 20248, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
Model: gpt-5.6-sol with xhigh thinking.
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. Unless repository policy already allows L2 approval, grant it by mentioning @buildsworth-bk (see approval ceiling and L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only when repository policy or a verified grant allows it; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.
| } | ||
| return nil, locatedError(path, in.All.Pos, "permissions", "permission aliases are unsupported; declare each required permission explicitly") | ||
| access := strings.TrimSuffix(in.All.Value, "-all") | ||
| return nil, fmt.Errorf("%s:%d:%d: permissions: %s is unsupported as job-level shorthand. In job %q, declare each needed permission explicitly, such as contents: %s and pull-requests: %s. Move permissions: %s to the workflow top level only when that broader authority is intended for every job. If you need job-level permissions shorthand, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support", path, in.All.Pos.Line, in.All.Pos.Col, in.All.Value, jobID, access, access, in.All.Value) |
There was a problem hiding this comment.
Non-blocking: actionlint sets in.All for every scalar, not only read-all and write-all. For example, top-level permissions: write now reaches this branch with an empty jobID, so the diagnostic calls it job-level shorthand for job "" and tells the user to move it to the top level. Limit this wording to the two recognized shorthands and retain a generic invalid-scalar diagnostic for other values.
2 participants
Why
A job using
environment: productionreports a double-wrapped job name and does not say what to change. A job usingpermissions: write-allcalls the value an alias and attributes it to a job namedpermissions, even though both shorthands remain supported at workflow level.Closes PB-3022
Closes PB-3019
What
environment, explain that GitHub approvals, deployment records, and protection rules are unavailable, and direct them to Buildkite secrets.read-allandwrite-allshorthands, show explicit permission examples, and reserve top-level shorthand guidance for workflows that intend broader authority for every job.Preview
GitHub environment
Job-level
permissions: write-all