fix: expose request-changes on reusable review workflow#72
Conversation
#61 added the action input and repo-config key, but callers of review.yml@main could not pass request-changes because it was never declared under workflow_call. Wire request-changes, config-file, and use-json-response-mode through the reusable workflow, and add a parity test so new action inputs cannot silently skip that layer again. Co-authored-by: Cursor <cursoragent@cursor.com>
🏹 Robin✅ Review done. I flagged 1 thing worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
The change cleanly adds three new workflow inputs (request-changes, config-file, use-json-response-mode) with thoughtful tri-state string defaults that defer to repo config, and the docs are updated consistently. A new meta-test verifies that all non-secret action inputs are exposed and forwarded by the reusable workflow, which is a great guardrail. The main concern is that the test's regex for parsing action.yml inputs is not scoped to the inputs section and will also match outputs, making it fragile when outputs are added to action.yml.
Robin — the Robin Hood of code review. Free for every PR.
The #61 miss was a dual-entry-point gap: action.yml gained request-changes, review.yml did not. Add a consumer fixture that passes every workflow_call input, run actionlint in Self-Test (same schema check GitHub uses), harden Jest parity/docs checks, and put a dual-entry checklist in the PR template and CONTRIBUTING. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Follow-up hardening on this PR so we don't repeat the dual-entry miss:
|
Robin flagged that the parity-test regex matched any 2-space key with a description, including a future outputs: block. Slice to the inputs: section first and add a regression case with a synthetic outputs key. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/robin Fixed in 88408f2:
Verification:
|
🏹 Robin✅ Review done. I flagged 5 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
This is a well-structured change that adds new workflow_call inputs (request-changes, config-file, use-json-response-mode), introduces actionlint in CI, and adds comprehensive parity tests plus a consumer fixture to prevent undeclared-input regressions. The test coverage for action/workflow input parity is thoughtful and the documentation updates are thorough. A few minor concerns around supply-chain safety of the actionlint install, a potential docs/code default mismatch for max-comments, and an internal name reference in a public fixture.
Robin — the Robin Hood of code review. Free for every PR.
Download the pinned linux_amd64 tarball and sha256sum-check it instead of curl|bash, and genericize the consumer-fixture comment so it does not name a reporter. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/robin Fixed in 92667c1:
Skipped (with replies):
Verification:
|
🏹 Robin✅ Review done. I flagged 3 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
🔵 2 Low | 💡 1 Suggestions
Summary
Well-structured change that adds three new workflow_call inputs (request-changes, config-file, use-json-response-mode) to the reusable review workflow, forwards them to the action step, and adds comprehensive parity tests plus an actionlint-validated consumer fixture. The defer-to-config pattern using string-typed inputs with empty defaults is consistent and correctly documented. The actionlint installation uses a pinned SHA256 checksum, which is good supply-chain hygiene. No high or medium severity issues found.
Robin — the Robin Hood of code review. Free for every PR.
Cesar's consumer YAML uses `request-changes: false`. A string-typed workflow_call input rejects that as a bool/string mismatch under actionlint (and is awkward for real callers). Use type: boolean with no default so omit still defers to .github/robin.yml. Co-authored-by: Cursor <cursoragent@cursor.com>
🤖 I have created a release *beep* *boop* --- ## [2.4.3](v2.4.2...v2.4.3) (2026-07-09) ### Bug Fixes * expose request-changes on reusable review workflow ([#72](#72)) ([d648b4b](d648b4b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
request-changeson the action and.github/robin.yml, but not on.github/workflows/review.yml, so reusable-workflow callers gotInvalid input, request-changes is not defined.request-changes, plus the other documented-but-missing inputs (config-file,use-json-response-mode).Root cause
Reusable workflows only accept inputs listed under
on.workflow_call.inputs. Adding an action input alone is not enough for the commonuses: .../review.yml@mainpath.Test plan
npm test— 72 passedrequest-changesis stripped fromreview.yml(verified locally)Fixes the report on #59.