[PR #317] [MERGED] docs(callers): document the 13 undocumented workflow_call inputs and drain KNOWN_UNDOCUMENTED #318

New issue

Closed

opened 2026-10-26 22:38:25 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:38:25 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/317
Author: @mattmillerai
Created: 9/18/2026
Status: ✅ Merged
Merged: 9/19/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-15261-document-undocumented-workflow-call-inputs

📝 Commits (2)

00f93a8 docs(callers): document the 13 undocumented workflow_call inputs and drain KNOWN_UNDOCUMENTED
f3f8896 docs(callers): correct eight input rows the review panel found overclaimed

📊 Changes

5 files changed (+39 additions, -27 deletions)

View changed files

📝 .github/workflow-pins/tests/test_inputs_docs_drift.py (+24 -25)
📝 docs/callers/agents-md-integrity.md (+1 -0)
📝 docs/callers/groom.md (+6 -2)
📝 docs/callers/pr-derisk.md (+2 -0)
📝 docs/callers/pr-risk.md (+6 -0)

📄 Description

ELI-5

Sixteen reusable workflows in this repo each accept a set of with: inputs, and each has a caller guide under docs/callers/ whose Inputs table is supposed to list them. Thirteen inputs were missing from those tables — real, shipped knobs that a consumer could only discover by opening the workflow YAML and reading it. This PR writes the thirteen missing rows and empties the allowlist that was holding the debt open.

What changed

One table row per undocumented input, in each page's existing column style, with the Default cell copied from the workflow's own default: and the Notes cell condensed from that input's description::

docs/callers/agents-md-integrity.md — exclude_paths.
docs/callers/pr-risk.md — enabled, pr_number, pr_numbers, sticky_comment, check_run, check_name. Three of these were already referenced by the guide's own prose: the permissions section explains why checks: write is required "whether or not you set check_run: true", and the enrollment section describes the backfill and manual re-grade that only pr_number / pr_numbers make possible. The prose named the behaviour; nothing named the input that reaches it.
docs/callers/pr-derisk.md — fleet_logins, bot_logins, each linked to pr-risk's row for the same input, since the workflow forwards both to the grader unchanged.
docs/callers/groom.md — sink, path, config, bail_sink.

groom's table now names every input the workflow declares (20 inputs across 19 rows — scope_label / scope_desc share one), so the "The ones that matter:" preamble is reworded to "All of them:". That is a claim the table has to keep earning, and the harness is what keeps it honest from here.

KNOWN_UNDOCUMENTED in .github/workflow-pins/tests/test_inputs_docs_drift.py drains to {}. This is not a loosening: test_known_undocumented_is_not_stale fails on an entry whose input is now documented, so every one of the thirteen became a hard failure the moment its row landed, and removing them is what the file was built to make necessary. The dict, its explanatory comment and both directions of the check stay in place for the day a deliberately internal input appears; the comment now says so, and points at where the debt came from.

No workflow input is added, removed or renamed. No workflows_ref row is touched. check_defaults stays off on every row — see Residual.

Verification

python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v — 458 tests, OK (32 skipped), with KNOWN_UNDOCUMENTED empty.
python3 .github/workflow-pins/check_workflow_pins.py — OK, 11 workflows declare workflows_ref, none with a default, every ref checkout guarded. Run because docs/callers/** edits trigger its guide cross-check.
Negative control, so the green is not vacuous: deleting the new sink row from groom's table fails InputsDocsDrift_groom.test_every_declared_input_is_documented_or_allowlisted, and restoring it returns the suite to green. The undocumented direction is genuinely live on these rows now, not passing by an empty-vs-empty comparison.
Drift probe across all 16 reusables, driving the harness's own scanners directly: declared-minus-documented and documented-minus-declared are both empty for every row.
Default cells checked by machine, not by eye, using the harness's workflow_input_defaults / documented_input_defaults: all 13 documented Default cells equal the declaration verbatim ('', false, issue, github, PR risk (advisory)).
Adjacent suites that own the two hand-rolled originals and the other guides, to confirm nothing regressed: .github/cursor-review/tests (600 OK), .github/refresh-reviewers/tests (56 OK), .github/public-repo-hygiene/tests (204 OK), .github/agents-md-integrity/tests (46 OK), scripts/linear-ticket/tests (116 OK).
python3 .github/agents-md-integrity/check_agents_md.py --root . — passed with 2 pre-existing warnings (AGENTS.md at 155 lines over the aspirational 150; no CODEOWNERS). Neither is touched by this diff.
Falsification of the two capability-denying sentences this diff writes. The new sink and bail_sink rows state that linear is reserved and not implemented. Rather than assert that from the input's own description:, I went and read the enforcing code: groom.yml's Validate sink step hard-fails any sink other than github before the audit runs, and .github/groom/config.py defines BAIL_SINKS = ("issue", "none") with a dedicated _coerce_bail_sink branch that refuses linear by name. Both denials are accurate descriptions of shipped behaviour; this diff adds no dead-end path and changes no code.
No bump fleet watches docs/callers/** or .github/workflow-pins/**, checked against every bump-*-callers.yml paths: filter, so no Skip-caller-bump: true trailer applies here — adding one would declare the PR bump-irrelevant for fleets that were never going to fire.

Judgment calls

Row placement. New rows sit next to the input they interact with rather than appended: enabled / pr_number / pr_numbers above the grading knobs (switch, then targeting), the two output knobs below them; groom's sink beside max_findings, path beside scope_label / scope_desc, config beside workflows_ref, bail_sink beside the other builder: true rows.
bail_sink's Notes cell is long because the quiet option is a footgun worth stating inline: none records nothing in the ledger, so a deterministic bail re-bails every run and permanently holds one of the max_prs slots — at the common pilot setting max_prs: 1, nothing else is ever built. The cell also states the one bail none does not suppress (the pre-publish secret-scan withhold).
config's Notes cell leads with what you should use instead. The workflow's own description opens "You almost certainly do NOT need to pass this"; the row says plainly that vars.GROOM_CONFIG is the normal route and names the one case the input exists for (a workflow_dispatch value that must outrank the repo variable, which a reusable cannot otherwise see).
pr-derisk gets two rows, not one combined row, even though that page already combines repo_map_path / repo_runbooks_path — the two forwarded logins lists have different meanings and different consequences, and the harness reads combined cells correctly either way.

Residual

The Default column is documented but not yet enforced. check_defaults is off on all 16 rows of the drift harness by design — the two hand-rolled originals still own that pin, and turning it on is the fold-in change's job, not this one's. I verified all 13 new Default cells against their declarations by machine (above), but nothing in CI will catch it if a workflow's default: later moves and one of these cells does not follow. The fold-in that flips check_defaults on would close this; it is deliberately out of scope here.
The dir_readme / readme_heading / readme_mode columns are likewise still off, for the same reason and with the same owner. A directory README knob table can still drift from its workflow without failing anything.
Unexercised artifacts. The upstream read-only investigation's findings comment — the evidence this work was specified from — and the parent epic live in the issue tracker, which is not reachable from this environment. I re-derived the same 13-input set independently by running the harness at merge base and probing its scanners across all 16 reusables, and the set matched the one specified, so the specification is corroborated rather than taken on trust; but I did not read the original findings comment and cannot confirm it carries no additional acceptance detail beyond what was relayed to me.
Corpus not fixed, sized. The sweep covered all 16 reusables in the harness's table, not only the 4 this PR edits. The other 12 guides show zero drift in both directions today, so there is nothing left undone there — but that is a measurement at this commit, not a guarantee, and it is exactly what the harness exists to re-measure.
Prose elsewhere in these guides was left alone. Several of the newly-documented inputs are also discussed in surrounding prose (pr-risk's permissions and enrollment sections, groom's footguns section). I did not rewrite that prose to cross-link the new rows; the tables are now complete and the prose is unchanged, so the two can still describe the same knob in different words.

Provenance

Authored by: agent-work loop
Verified: python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v: 458 passed, 0 failed, 32 skipped; python3 .github/workflow-pins/check_workflow_pins.py: OK, 0 violations across 11 workflows; negative control (row deleted): 1 expected failure, restored to green; cursor-review 600 / refresh-reviewers 56 / public-repo-hygiene 204 / agents-md-integrity 46 / linear-ticket 116 — all OK; check_agents_md.py --root .: passed, 2 pre-existing warnings.
Deviations: none — every acceptance criterion met. check_defaults was deliberately left off, as the change specified.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/317 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/18/2026 **Status:** ✅ Merged **Merged:** 9/19/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-15261-document-undocumented-workflow-call-inputs` --- ### 📝 Commits (2) - [`00f93a8`](https://github.com/Comfy-Org/github-workflows/commit/00f93a8ddd1a5a7f70d879203dfdec191af7e79b) docs(callers): document the 13 undocumented workflow_call inputs and drain KNOWN_UNDOCUMENTED - [`f3f8896`](https://github.com/Comfy-Org/github-workflows/commit/f3f889675cd14d97d20d868ebd50467cea66d6a9) docs(callers): correct eight input rows the review panel found overclaimed ### 📊 Changes **5 files changed** (+39 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflow-pins/tests/test_inputs_docs_drift.py` (+24 -25) 📝 `docs/callers/agents-md-integrity.md` (+1 -0) 📝 `docs/callers/groom.md` (+6 -2) 📝 `docs/callers/pr-derisk.md` (+2 -0) 📝 `docs/callers/pr-risk.md` (+6 -0) </details> ### 📄 Description ## ELI-5 Sixteen reusable workflows in this repo each accept a set of `with:` inputs, and each has a caller guide under `docs/callers/` whose Inputs table is supposed to list them. Thirteen inputs were missing from those tables — real, shipped knobs that a consumer could only discover by opening the workflow YAML and reading it. This PR writes the thirteen missing rows and empties the allowlist that was holding the debt open. ## What changed One table row per undocumented input, in each page's existing column style, with the Default cell copied from the workflow's own `default:` and the Notes cell condensed from that input's `description:`: - **`docs/callers/agents-md-integrity.md`** — `exclude_paths`. - **`docs/callers/pr-risk.md`** — `enabled`, `pr_number`, `pr_numbers`, `sticky_comment`, `check_run`, `check_name`. Three of these were already referenced by the guide's own prose: the permissions section explains why `checks: write` is required "whether or not you set `check_run: true`", and the enrollment section describes the backfill and manual re-grade that only `pr_number` / `pr_numbers` make possible. The prose named the behaviour; nothing named the input that reaches it. - **`docs/callers/pr-derisk.md`** — `fleet_logins`, `bot_logins`, each linked to pr-risk's row for the same input, since the workflow forwards both to the grader unchanged. - **`docs/callers/groom.md`** — `sink`, `path`, `config`, `bail_sink`. groom's table now names every input the workflow declares (20 inputs across 19 rows — `scope_label` / `scope_desc` share one), so the "The ones that matter:" preamble is reworded to "All of them:". That is a claim the table has to keep earning, and the harness is what keeps it honest from here. `KNOWN_UNDOCUMENTED` in `.github/workflow-pins/tests/test_inputs_docs_drift.py` drains to `{}`. This is not a loosening: `test_known_undocumented_is_not_stale` fails on an entry whose input is now documented, so every one of the thirteen became a hard failure the moment its row landed, and removing them is what the file was built to make necessary. The dict, its explanatory comment and both directions of the check stay in place for the day a deliberately internal input appears; the comment now says so, and points at where the debt came from. No workflow input is added, removed or renamed. No `workflows_ref` row is touched. `check_defaults` stays off on every row — see Residual. ## Verification - `python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v` — 458 tests, OK (32 skipped), with `KNOWN_UNDOCUMENTED` empty. - `python3 .github/workflow-pins/check_workflow_pins.py` — OK, 11 workflows declare `workflows_ref`, none with a default, every ref checkout guarded. Run because `docs/callers/**` edits trigger its guide cross-check. - **Negative control, so the green is not vacuous:** deleting the new `sink` row from groom's table fails `InputsDocsDrift_groom.test_every_declared_input_is_documented_or_allowlisted`, and restoring it returns the suite to green. The undocumented direction is genuinely live on these rows now, not passing by an empty-vs-empty comparison. - **Drift probe across all 16 reusables**, driving the harness's own scanners directly: declared-minus-documented and documented-minus-declared are both empty for every row. - **Default cells checked by machine, not by eye**, using the harness's `workflow_input_defaults` / `documented_input_defaults`: all 13 documented Default cells equal the declaration verbatim (`''`, `false`, `issue`, `github`, `PR risk (advisory)`). - Adjacent suites that own the two hand-rolled originals and the other guides, to confirm nothing regressed: `.github/cursor-review/tests` (600 OK), `.github/refresh-reviewers/tests` (56 OK), `.github/public-repo-hygiene/tests` (204 OK), `.github/agents-md-integrity/tests` (46 OK), `scripts/linear-ticket/tests` (116 OK). - `python3 .github/agents-md-integrity/check_agents_md.py --root .` — passed with 2 pre-existing warnings (AGENTS.md at 155 lines over the aspirational 150; no CODEOWNERS). Neither is touched by this diff. - **Falsification of the two capability-denying sentences this diff writes.** The new `sink` and `bail_sink` rows state that `linear` is reserved and not implemented. Rather than assert that from the input's own `description:`, I went and read the enforcing code: `groom.yml`'s `Validate sink` step hard-fails any `sink` other than `github` before the audit runs, and `.github/groom/config.py` defines `BAIL_SINKS = ("issue", "none")` with a dedicated `_coerce_bail_sink` branch that refuses `linear` by name. Both denials are accurate descriptions of shipped behaviour; this diff adds no dead-end path and changes no code. - No bump fleet watches `docs/callers/**` or `.github/workflow-pins/**`, checked against every `bump-*-callers.yml` `paths:` filter, so no `Skip-caller-bump: true` trailer applies here — adding one would declare the PR bump-irrelevant for fleets that were never going to fire. ## Judgment calls - **Row placement.** New rows sit next to the input they interact with rather than appended: `enabled` / `pr_number` / `pr_numbers` above the grading knobs (switch, then targeting), the two output knobs below them; groom's `sink` beside `max_findings`, `path` beside `scope_label` / `scope_desc`, `config` beside `workflows_ref`, `bail_sink` beside the other `builder: true` rows. - **`bail_sink`'s Notes cell is long** because the quiet option is a footgun worth stating inline: `none` records nothing in the ledger, so a deterministic bail re-bails every run and permanently holds one of the `max_prs` slots — at the common pilot setting `max_prs: 1`, nothing else is ever built. The cell also states the one bail `none` does not suppress (the pre-publish secret-scan withhold). - **`config`'s Notes cell leads with what you should use instead.** The workflow's own description opens "You almost certainly do NOT need to pass this"; the row says plainly that `vars.GROOM_CONFIG` is the normal route and names the one case the input exists for (a `workflow_dispatch` value that must outrank the repo variable, which a reusable cannot otherwise see). - **pr-derisk gets two rows, not one combined row**, even though that page already combines `repo_map_path` / `repo_runbooks_path` — the two forwarded logins lists have different meanings and different consequences, and the harness reads combined cells correctly either way. ## Residual - **The Default column is documented but not yet enforced.** `check_defaults` is off on all 16 rows of the drift harness by design — the two hand-rolled originals still own that pin, and turning it on is the fold-in change's job, not this one's. I verified all 13 new Default cells against their declarations by machine (above), but nothing in CI will catch it if a workflow's `default:` later moves and one of these cells does not follow. The fold-in that flips `check_defaults` on would close this; it is deliberately out of scope here. - **The `dir_readme` / `readme_heading` / `readme_mode` columns are likewise still off**, for the same reason and with the same owner. A directory README knob table can still drift from its workflow without failing anything. - **Unexercised artifacts.** The upstream read-only investigation's findings comment — the evidence this work was specified from — and the parent epic live in the issue tracker, which is not reachable from this environment. I re-derived the same 13-input set independently by running the harness at merge base and probing its scanners across all 16 reusables, and the set matched the one specified, so the specification is corroborated rather than taken on trust; but I did not read the original findings comment and cannot confirm it carries no additional acceptance detail beyond what was relayed to me. - **Corpus not fixed, sized.** The sweep covered all 16 reusables in the harness's table, not only the 4 this PR edits. The other 12 guides show zero drift in both directions today, so there is nothing left undone there — but that is a measurement at this commit, not a guarantee, and it is exactly what the harness exists to re-measure. - **Prose elsewhere in these guides was left alone.** Several of the newly-documented inputs are also discussed in surrounding prose (pr-risk's permissions and enrollment sections, groom's footguns section). I did not rewrite that prose to cross-link the new rows; the tables are now complete and the prose is unchanged, so the two can still describe the same knob in different words. ## Provenance - **Authored by:** agent-work loop - **Verified:** `python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v`: 458 passed, 0 failed, 32 skipped; `python3 .github/workflow-pins/check_workflow_pins.py`: OK, 0 violations across 11 workflows; negative control (row deleted): 1 expected failure, restored to green; cursor-review 600 / refresh-reviewers 56 / public-repo-hygiene 204 / agents-md-integrity 46 / linear-ticket 116 — all OK; `check_agents_md.py --root .`: passed, 2 pre-existing warnings. - **Deviations:** none — every acceptance criterion met. `check_defaults` was deliberately left off, as the change specified. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:38:25 +00:00