Add verify and min_severity inputs to action.yml #108

Manually merged
marvin8 merged 6 commits from fix/issue-107-actionyml-wave1-inputs into main 2026-10-17 00:56:41 +00:00
Collaborator

Wave 1 added WUMING_VERIFY (#102) and WUMING_MIN_SEVERITY (#104) but action.yml was never updated, so repos consuming WuMing as a Forgejo/GitHub Action silently ran both features at their defaults — the settings were unreachable from the action interface.

  • New verify input (default "true", matching the env default) mapped to WUMING_VERIFY
  • New min_severity input (default "" = no floor) mapped to WUMING_MIN_SEVERITY
  • Regression test tests/test_action_inputs.py: asserts every WUMING_* env var config.py reads has an action.yml input, every mapping resolves to a declared input, no orphan mappings, no unused inputs (fails against pre-fix main, passes here)
  • AGENTS.md Documentation section: action.yml added to the must-update file list with the same-PR rule
  • Release-Notes.md: intro bullet + follow-on post; version line corrected 0.11.0 → 0.12.0 to match pyproject

Closes #107

Part of wave 2 (convergence): #79, #85, #84 follow.

Wave 1 added `WUMING_VERIFY` (#102) and `WUMING_MIN_SEVERITY` (#104) but `action.yml` was never updated, so repos consuming WuMing as a Forgejo/GitHub Action silently ran both features at their defaults — the settings were unreachable from the action interface. - New `verify` input (default `"true"`, matching the env default) mapped to `WUMING_VERIFY` - New `min_severity` input (default `""` = no floor) mapped to `WUMING_MIN_SEVERITY` - Regression test `tests/test_action_inputs.py`: asserts every `WUMING_*` env var config.py reads has an action.yml input, every mapping resolves to a declared input, no orphan mappings, no unused inputs (fails against pre-fix `main`, passes here) - AGENTS.md Documentation section: `action.yml` added to the must-update file list with the same-PR rule - Release-Notes.md: intro bullet + follow-on post; version line corrected 0.11.0 → 0.12.0 to match pyproject Closes #107 Part of wave 2 (convergence): #79, #85, #84 follow.
🐛 Add verify and min_severity inputs to action.yml
All checks were successful
/ gitleaks (pull_request) Successful in 13s
/ pr-review (pull_request) Successful in 1m17s
/ checks (pull_request) Successful in 1m47s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
d44eb33645
agent-pi changed title from t to Add verify and min_severity inputs to action.yml 2026-10-16 23:40:56 +00:00
✅ Add action.yml parity test and AGENTS.md input rule
Some checks failed
/ gitleaks (pull_request) Successful in 24s
/ pr-review (pull_request) Successful in 1m48s
/ checks (pull_request) Failing after 2m13s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
9e34070fd2
forgejo-actions left a comment

WuMing

Found 4 issue(s). See inline comments below.

## WuMing Found **4** issue(s). See inline comments below.
Release-Notes.md Outdated
@ -6,3 +6,4 @@
- Findings are now fact-checked against the changed file's full content before they are posted — claims the file contradicts are dropped automatically (`WUMING_VERIFY=false` to disable).
- Repos can now set a minimum severity for posted findings (`WUMING_MIN_SEVERITY`): anything below the floor — and unrecognised severity values — are dropped before posting; agent prompts now also treat a clean report as the preferred outcome.
- Wave-1 settings are reachable from the Forgejo/GitHub Action interface again: `action.yml` now has `verify` and `min_severity` inputs (previously they silently fell back to defaults).

docs [LOW]

The word "again" is inaccurate and will mislead readers: per this PR's own description the verify/min_severity settings were never reachable from the action interface (action.yml was never updated, so there was nothing to restore). Reword to drop "again" — e.g. "Wave-1 settings are now reachable from the Forgejo/GitHub Action interface:". While here, consider replacing the internal project jargon "Wave-1 settings" with user-facing wording such as "the two review-quality settings from the previous release", matching the phrasing used in the follow-on post at line 37.

**docs** [LOW] The word "again" is inaccurate and will mislead readers: per this PR's own description the `verify`/`min_severity` settings were *never* reachable from the action interface (action.yml was never updated, so there was nothing to restore). Reword to drop "again" — e.g. "Wave-1 settings are now reachable from the Forgejo/GitHub Action interface:". While here, consider replacing the internal project jargon "Wave-1 settings" with user-facing wording such as "the two review-quality settings from the previous release", matching the phrasing used in the follow-on post at line 37. <!-- wuming:sha256:8cf7671ac48694ae91465d036f37adc7ac7678b6f73cc0099c2caa1e0ecc89bb -->
marvin8 marked this conversation as resolved
@ -100,0 +110,4 @@
Unset by default — everything posts. Invalid values fail closed with a
startup error.
required: false
default: ""

config [MEDIUM]

The default "" is exported as WUMING_MIN_SEVERITY: ${{ inputs.min_severity }} on line 141, so for a default run the env var is present but empty rather than truly unset (contrary to the description on lines 110-111 and the documented 'unset = everything posts' contract). If config.py treats the presence of WUMING_MIN_SEVERITY as 'a value was supplied' and validates it against low|medium|high|critical, the empty string will fail closed with a startup error, breaking every consumer that does not set the input. Fix by either omitting the env mapping when the input is empty, defaulting the input to a sentinel that config.py maps to "no floor", or ensuring config.py treats empty/whitespace as unset.

**config** [MEDIUM] The default `""` is exported as `WUMING_MIN_SEVERITY: ${{ inputs.min_severity }}` on line 141, so for a default run the env var is present but empty rather than truly unset (contrary to the description on lines 110-111 and the documented 'unset = everything posts' contract). If config.py treats the presence of WUMING_MIN_SEVERITY as 'a value was supplied' and validates it against low|medium|high|critical, the empty string will fail closed with a startup error, breaking every consumer that does not set the input. Fix by either omitting the env mapping when the input is empty, defaulting the input to a sentinel that config.py maps to "no floor", or ensuring config.py treats empty/whitespace as unset. <!-- wuming:sha256:c617d41f2058f266b1d338f10c492e9353f11f8bfcd12c346a10153f0d99a21c -->
@ -0,0 +16,4 @@
_REPO_ROOT = Path(__file__).resolve().parent.parent
_ENV_READ_PATTERN = r'os\.environ\.get\(\s*"(WUMING_[A-Z0-9_]+)"'
_INPUT_REFERENCE_PATTERN = r"\$\{\{\s*inputs\.([A-Za-z0-9_]+)\s*\}\}"

code [MEDIUM]

The input-name regex [A-Za-z0-9_]+ does not allow hyphens, but GitHub Actions input identifiers can contain '-' (along with letters, numbers, and ''). If action.yml ever maps a hyphenated input such as '${{ inputs.my-input }}', re.findall will extract only the prefix before the hyphen, causing test_env_mappings_reference_declared_inputs and test_all_inputs_are_used to fail or pass incorrectly. Widen the capture group to [A-Za-z0-9-]+ so valid hyphenated input names are parsed correctly.

**code** [MEDIUM] The input-name regex [A-Za-z0-9_]+ does not allow hyphens, but GitHub Actions input identifiers can contain '-' (along with letters, numbers, and '_'). If action.yml ever maps a hyphenated input such as '${{ inputs.my-input }}', re.findall will extract only the prefix before the hyphen, causing test_env_mappings_reference_declared_inputs and test_all_inputs_are_used to fail or pass incorrectly. Widen the capture group to [A-Za-z0-9_-]+ so valid hyphenated input names are parsed correctly. <!-- wuming:sha256:13c2b3bae7a6191b7238368ea8fff813ff0b25af878aae4f7bdbb143cd7388b3 -->
@ -0,0 +64,4 @@
env_mappings = _action_env_mappings()
config_env_vars = _config_env_vars()
for env_var in sorted(config_env_vars - _INTENTIONALLY_UNMAPPED):
expect(env_mappings).to_contain(env_var)

tests [MEDIUM]

This assertion (like the one in test_env_mappings_are_read_by_config at line 83) only runs inside a loop over a scraped set. If _config_env_vars() returns an empty set — e.g. config.py is later refactored to read env vars via a helper so _ENV_READ_PATTERN no longer matches — the loop body never executes and the test passes vacuously, silently defeating the exact drift regression it was written to catch. Add a non-emptiness guard before the loop (e.g. expect(config_env_vars).to_contain("WUMING_VERIFY") or assert the set has the expected length) so a broken scrape fails loudly instead of passing.

**tests** [MEDIUM] This assertion (like the one in test_env_mappings_are_read_by_config at line 83) only runs inside a loop over a scraped set. If `_config_env_vars()` returns an empty set — e.g. config.py is later refactored to read env vars via a helper so `_ENV_READ_PATTERN` no longer matches — the loop body never executes and the test passes vacuously, silently defeating the exact drift regression it was written to catch. Add a non-emptiness guard before the loop (e.g. `expect(config_env_vars).to_contain("WUMING_VERIFY")` or assert the set has the expected length) so a broken scrape fails loudly instead of passing. <!-- wuming:sha256:da018ee6643c29653e50c87ba9803c291a5b1c79a03e5800f0effc1f7c0f89a9 -->
marvin8 approved these changes 2026-10-17 00:16:39 +00:00
Dismissed
🐛 Declare pyyaml for tests, guard parity test scrapes, widen input regex
Some checks failed
/ gitleaks (pull_request) Successful in 36s
/ checks (pull_request) Failing after 1m45s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 3m19s
63257d1016
agent-pi dismissed marvin8's review 2026-10-17 00:18:20 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Author
Collaborator

Wave-1 review responses (commit 63257d1)

✅ Fixed — input-name regex rejects hyphenated identifiers

tests/test_action_inputs.py line 19

The input-name regex [A-Za-z0-9_]+ does not allow hyphens, but GitHub Actions input identifiers can contain '-'…

Widened to [A-Za-z0-9_-]+ — valid for both Forgejo and GitHub Actions input names.


✅ Fixed — vacuous-pass risk in the scrape loops

tests/test_action_inputs.py line 67

This assertion (like the one in test_env_mappings_are_read_by_config at line 83) only runs inside a loop over a scraped set. If _config_env_vars() returns an empty set…

Good catch, and exactly the failure mode this test file exists to prevent. All four tests now open with sentinel expectations (WUMING_VERIFY/WUMING_BACKEND on the config scrape, WUMING_FORGEJO_TOKEN on the env mapping, verify on the declared inputs), so a broken scrape fails loudly instead of passing vacuously.


🔴 Not actioned — empty WUMING_MIN_SEVERITY failing closed

action.yml line 113

The default "" is exported as WUMING_MIN_SEVERITY: ${{ inputs.min_severity }}… If config.py treats the presence of WUMING_MIN_SEVERITY as 'a value was supplied' and validates it…

The conditional premise doesn't hold: _parse_min_severity() returns None for unset and empty values — the empty string never reaches validation (src/wuming/config.py lines 102–117, docstring: "or None when unset/empty (no floor)"). tests/test_config.py::test_min_severity_unset pins exactly this contract. Present-but-empty is also the standard action.yml pattern every other optional input in this file uses (dry_run, verbose, summary_comment — all default ""/"false" and rely on the parser's unset handling), and an env: block cannot conditionally omit a mapping. So the default run behaves as documented: unset = everything posts.


✅ Fixed — "again" in the release-note bullet

Release-Notes.md line 9

The word "again" is inaccurate and will mislead readers: per this PR's own description the verify/min_severity settings were never reachable from the action interface…

Reworded to drop "again" and the internal "Wave-1" jargon: "The review-quality settings are now settable from the Forgejo/GitHub Action interface…"

### Wave-1 review responses (commit `63257d1`) ### ✅ Fixed — input-name regex rejects hyphenated identifiers [`tests/test_action_inputs.py` line 19](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5740) > The input-name regex [A-Za-z0-9_]+ does not allow hyphens, but GitHub Actions input identifiers can contain '-'… Widened to `[A-Za-z0-9_-]+` — valid for both Forgejo and GitHub Actions input names. --- ### ✅ Fixed — vacuous-pass risk in the scrape loops [`tests/test_action_inputs.py` line 67](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5743) > This assertion (like the one in test_env_mappings_are_read_by_config at line 83) only runs inside a loop over a scraped set. If `_config_env_vars()` returns an empty set… Good catch, and exactly the failure mode this test file exists to prevent. All four tests now open with sentinel expectations (`WUMING_VERIFY`/`WUMING_BACKEND` on the config scrape, `WUMING_FORGEJO_TOKEN` on the env mapping, `verify` on the declared inputs), so a broken scrape fails loudly instead of passing vacuously. --- ### 🔴 Not actioned — empty `WUMING_MIN_SEVERITY` failing closed [`action.yml` line 113](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5741) > The default `""` is exported as `WUMING_MIN_SEVERITY: ${{ inputs.min_severity }}`… If config.py treats the presence of WUMING_MIN_SEVERITY as 'a value was supplied' and validates it… The conditional premise doesn't hold: `_parse_min_severity()` returns `None` for unset **and** empty values — the empty string never reaches validation (`src/wuming/config.py` lines 102–117, docstring: "or ``None`` when unset/empty (no floor)"). `tests/test_config.py::test_min_severity_unset` pins exactly this contract. Present-but-empty is also the standard action.yml pattern every other optional input in this file uses (`dry_run`, `verbose`, `summary_comment` — all default `""`/`"false"` and rely on the parser's unset handling), and an `env:` block cannot conditionally omit a mapping. So the default run behaves as documented: unset = everything posts. --- ### ✅ Fixed — "again" in the release-note bullet [`Release-Notes.md` line 9](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5742) > The word "again" is inaccurate and will mislead readers: per this PR's own description the `verify`/`min_severity` settings were *never* reachable from the action interface… Reworded to drop "again" and the internal "Wave-1" jargon: "The review-quality settings are now settable from the Forgejo/GitHub Action interface…"
forgejo-actions left a comment

WuMing

Found 2 issue(s). See inline comments below.

## WuMing Found **2** issue(s). See inline comments below.
Release-Notes.md Outdated
@ -33,6 +34,10 @@ The pass is cautious in both directions: it only judges whether a finding is fac
---
A small repair for anyone running WuMing as a Forgejo or GitHub Action: the two review-quality settings from the last release — the fact-checking pass and the severity floor — existed only as environment variables, so the action wrapper quietly ignored them and ran with defaults. Both now have proper inputs (`verify` and `min_severity`), so you can set them directly in your workflow file. Anyone already setting the environment variables by hand is unaffected.

docs [MEDIUM]

Inconsistency with the rest of the release notes. The version header at line 5 was changed to wuming 0.12.0, and the two settings' own bullets (lines 7–8, WUMING_VERIFY and WUMING_MIN_SEVERITY) sit under that same 0.12.0 header — so the notes present the fact-checking pass and severity floor as part of this release. Calling them 'the two review-quality settings from the last release' in the new follow-on post contradicts that and will mislead readers about which release introduced them. Either reword to 'from this release' / 'introduced alongside this release', or, if they really shipped in a prior release, keep their bullets under the prior version's header and revert/qualify the version bump. Make the version header and the post agree.

**docs** [MEDIUM] Inconsistency with the rest of the release notes. The version header at line 5 was changed to `wuming 0.12.0`, and the two settings' own bullets (lines 7–8, `WUMING_VERIFY` and `WUMING_MIN_SEVERITY`) sit under that same 0.12.0 header — so the notes present the fact-checking pass and severity floor as part of this release. Calling them 'the two review-quality settings from the last release' in the new follow-on post contradicts that and will mislead readers about which release introduced them. Either reword to 'from this release' / 'introduced alongside this release', or, if they really shipped in a prior release, keep their bullets under the prior version's header and revert/qualify the version bump. Make the version header and the post agree. <!-- wuming:sha256:94fd3b7adee108cfc8c60d219913c790aa61e486ef67aea9b4a1e67060716307 -->
@ -0,0 +55,4 @@
Returns:
Every WUMING_* variable name passed to ``os.environ.get`` in config.py.
"""
config_source = (_REPO_ROOT / "src" / "wuming" / "config.py").read_text()

code [MEDIUM]

config_env_vars scrapes env vars from config.py with a regex. This is fragile and undermines the test's purpose of preventing drift: it will miss semantically equivalent reads such as os.getenv('WUMING...') or os.environ['WUMING_...'], and it can produce false positives from comments or string literals. Prefer parsing config.py with the ast module to collect the actual arguments of os.environ.get / os.getenv calls, which also handles single vs double quotes automatically.

**code** [MEDIUM] _config_env_vars scrapes env vars from config.py with a regex. This is fragile and undermines the test's purpose of preventing drift: it will miss semantically equivalent reads such as os.getenv('WUMING_...') or os.environ['WUMING_...'], and it can produce false positives from comments or string literals. Prefer parsing config.py with the ast module to collect the actual arguments of os.environ.get / os.getenv calls, which also handles single vs double quotes automatically. <!-- wuming:sha256:6c0008eb5e80e30e0face494a8d4c10c3dcc39d0338bd57160d3524a52d43eac -->
♻️ Parse config.py env reads with ast, fix release-notes phrasing
All checks were successful
/ gitleaks (pull_request) Successful in 13s
/ checks (pull_request) Successful in 1m48s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 2m39s
5565c037fd
Author
Collaborator

Wave-2 review responses (commit 5565c03)

✅ Fixed — regex scrape of config.py replaced with ast parsing

tests/test_action_inputs.py line 67

_config_env_vars scrapes env vars from config.py with a regex. This is fragile and undermines the test's purpose of preventing drift…

Agreed — the sentinel guards catch a broken scrape loudly, but an extractor that understands the code beats one that needs a sentinel to notice it's broken. _config_env_vars() now walks config.py's AST and collects WUMING_* literals from os.environ.get(...), os.getenv(...), and os.environ[...] reads. Verified it returns the identical 24-variable set the regex found before the rewrite.

Separately, the CI failure that motivated this push: the nox tryke sessions install only runtime deps + tryke (uv sync --no-default-groups), so pyyaml — needed to parse action.yml — was missing there. The tryke/tryke_betas sessions now install pyyaml alongside tryke.


✅ Fixed — "last release" contradicts the 0.12.0 header

Release-Notes.md line 9

The version header at line 5 was changed to wuming 0.12.0, and the two settings' own bullets (lines 7–8…) sit under that same 0.12.0 header — so the notes present the fact-checking pass and severity floor as part of this release…

Correct — the bullets live under the 0.12.0 header, so "from the last release" was wrong. The follow-on post now reads "the fact-checking pass and the severity floor introduced in this release originally exposed only their environment variables…"

### Wave-2 review responses (commit `5565c03`) ### ✅ Fixed — regex scrape of config.py replaced with ast parsing [`tests/test_action_inputs.py` line 67](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5781) > _config_env_vars scrapes env vars from config.py with a regex. This is fragile and undermines the test's purpose of preventing drift… Agreed — the sentinel guards catch a broken scrape loudly, but an extractor that understands the code beats one that needs a sentinel to notice it's broken. `_config_env_vars()` now walks config.py's AST and collects `WUMING_*` literals from `os.environ.get(...)`, `os.getenv(...)`, and `os.environ[...]` reads. Verified it returns the identical 24-variable set the regex found before the rewrite. Separately, the CI failure that motivated this push: the nox `tryke` sessions install only runtime deps + `tryke` (`uv sync --no-default-groups`), so `pyyaml` — needed to parse `action.yml` — was missing there. The `tryke`/`tryke_betas` sessions now install `pyyaml` alongside `tryke`. --- ### ✅ Fixed — "last release" contradicts the 0.12.0 header [`Release-Notes.md` line 9](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5782) > The version header at line 5 was changed to `wuming 0.12.0`, and the two settings' own bullets (lines 7–8…) sit under that same 0.12.0 header — so the notes present the fact-checking pass and severity floor as part of this release… Correct — the bullets live under the 0.12.0 header, so "from the last release" was wrong. The follow-on post now reads "the fact-checking pass and the severity floor introduced in this release originally exposed only their environment variables…"
forgejo-actions left a comment

WuMing

Found 2 issue(s). See inline comments below.

## WuMing Found **2** issue(s). See inline comments below.
@ -0,0 +135,4 @@
return env_vars
@test("every WUMING_ env var config.py reads has an action.yml input")

code [MEDIUM]

The parity tests verify that every env var has an input and vice versa, but they do not check that action.yml default values match the defaults used in config.py (e.g. verify defaulting to 'true' and min_severity defaulting to ''). A mismatch would silently change behavior for Action users, which is the same class of drift this regression test is meant to prevent. Add a test that compares the default in each action.yml input with the literal default passed to the corresponding os.environ.get/os.getenv call in config.py.

**code** [MEDIUM] The parity tests verify that every env var has an input and vice versa, but they do not check that action.yml default values match the defaults used in config.py (e.g. verify defaulting to 'true' and min_severity defaulting to ''). A mismatch would silently change behavior for Action users, which is the same class of drift this regression test is meant to prevent. Add a test that compares the default in each action.yml input with the literal default passed to the corresponding os.environ.get/os.getenv call in config.py. <!-- wuming:sha256:46c9e7590f33dd0786f4434f656db044ebb5a0f33be5c7ad9f423615d670020d -->
@ -0,0 +139,4 @@
def test_env_vars_have_inputs() -> None:
env_mappings = _action_env_mappings()
config_env_vars = _config_env_vars()
# Guard against vacuous passes: if the scrape ever breaks (e.g. config.py

code [LOW]

The sentinel comment refers to '_ENV_READ_PATTERN', but the actual scraper uses AST helper functions rather than a regex with that name. Update the comment to match the implementation so it does not confuse future maintainers.

**code** [LOW] The sentinel comment refers to '_ENV_READ_PATTERN', but the actual scraper uses AST helper functions rather than a regex with that name. Update the comment to match the implementation so it does not confuse future maintainers. <!-- wuming:sha256:89527792e4095554cc8ca5ef862bd0f5430a1f85d47847245749abc6e924169b -->
✅ Add defaults parity test, fix stale scraper comment
All checks were successful
/ gitleaks (pull_request) Successful in 11s
/ checks (pull_request) Successful in 1m57s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 5m30s
3eaf1b7486
Author
Collaborator

Wave-3 review responses (commit 3eaf1b7)

✅ Fixed — input defaults now parity-checked against config.py

tests/test_action_inputs.py line 193

The parity tests verify that every env var has an input and vice versa, but they do not check that action.yml default values match the defaults used in config.py…

Agreed — defaults are part of the contract. New test_input_defaults_match_config extracts each WUMING_* literal default from config.py's os.environ.get calls (same ast approach as the scraper) and compares it against the mapped input's declared default:. Parser-mediated defaults (the four booleans, plus WUMING_FORGEJO_URL, whose default is deliberately CI-context-dependent: Woodpecker pins the zone URL, Actions auto-detects from GITHUB_SERVER_URL) are documented in an explicit _DEFAULT_EQUIVALENCES table; everything else must match literally. The test already earned its keep: it caught that the Forgejo-URL default needed its context dependence written down.


✅ Fixed — stale _ENV_READ_PATTERN reference in the sentinel comment

tests/test_action_inputs.py line 67

The sentinel comment refers to '_ENV_READ_PATTERN', but the actual scraper uses AST helper functions rather than a regex with that name.

The comment now describes the real failure mode: "config.py stops reading env vars through the call patterns the scraper understands".

### Wave-3 review responses (commit `3eaf1b7`) ### ✅ Fixed — input defaults now parity-checked against config.py [`tests/test_action_inputs.py` line 193](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5790) > The parity tests verify that every env var has an input and vice versa, but they do not check that action.yml default values match the defaults used in config.py… Agreed — defaults are part of the contract. New `test_input_defaults_match_config` extracts each `WUMING_*` literal default from config.py's `os.environ.get` calls (same ast approach as the scraper) and compares it against the mapped input's declared `default:`. Parser-mediated defaults (the four booleans, plus `WUMING_FORGEJO_URL`, whose default is deliberately CI-context-dependent: Woodpecker pins the zone URL, Actions auto-detects from `GITHUB_SERVER_URL`) are documented in an explicit `_DEFAULT_EQUIVALENCES` table; everything else must match literally. The test already earned its keep: it caught that the Forgejo-URL default needed its context dependence written down. --- ### ✅ Fixed — stale `_ENV_READ_PATTERN` reference in the sentinel comment [`tests/test_action_inputs.py` line 67](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5791) > The sentinel comment refers to '_ENV_READ_PATTERN', but the actual scraper uses AST helper functions rather than a regex with that name. The comment now describes the real failure mode: "config.py stops reading env vars through the call patterns the scraper understands".
forgejo-actions left a comment

WuMing

Found 3 issue(s). See inline comments below.

## WuMing Found **3** issue(s). See inline comments below.
@ -0,0 +181,4 @@
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
if _dotted_name(node.func) != "os.environ.get" or not node.args:

code [MEDIUM]

The defaults scraper only handles os.environ.get, but _wuming_name_from_call and _config_env_vars also accept os.getenv. If config.py reads a WUMING_* variable via os.getenv(..., default=...), this test will record an empty default instead of the real one and can miss a true mismatch. Change the check to _dotted_name(node.func) in {"os.environ.get", "os.getenv"} and extract the second positional argument for both calls.

**code** [MEDIUM] The defaults scraper only handles `os.environ.get`, but `_wuming_name_from_call` and `_config_env_vars` also accept `os.getenv`. If config.py reads a `WUMING_*` variable via `os.getenv(..., default=...)`, this test will record an empty default instead of the real one and can miss a true mismatch. Change the check to `_dotted_name(node.func) in {"os.environ.get", "os.getenv"}` and extract the second positional argument for both calls. <!-- wuming:sha256:bf5f1a983a772fa82db404bff7d50b74c70ef354248cf0ac3a5db2fa2f47e924 -->
@ -0,0 +195,4 @@
and isinstance(node.args[1], ast.Constant)
and isinstance(node.args[1].value, str)
):
defaults[first_argument.value] = node.args[1].value

code [MEDIUM]

The scraper only looks at node.args[1] for the default value. A call like os.environ.get("WUMING_X", default="...") will be recorded as having an empty default, which can cause false mismatches in test_input_defaults_match_config. Inspect node.keywords for a keyword argument named default as well.

**code** [MEDIUM] The scraper only looks at `node.args[1]` for the default value. A call like `os.environ.get("WUMING_X", default="...")` will be recorded as having an empty default, which can cause false mismatches in `test_input_defaults_match_config`. Inspect `node.keywords` for a keyword argument named `default` as well. <!-- wuming:sha256:e4f610bdec4f094ee05fc560052f2a8abc4e48baddcaee0e21cc2d7ace4bc506 -->
@ -0,0 +240,4 @@
mismatches: list[str] = []
for env_var, target in _action_env_mappings().items():
referenced_inputs = re.findall(_INPUT_REFERENCE_PATTERN, str(target))
input_default = str(

code [LOW]

If action.yml declares default: with no value (YAML null) or omits default, yaml.safe_load returns None and str(None) becomes "None". In GitHub Actions an unset default is effectively the empty string, so the comparison should treat None as "". Coerce with input_default = str(input_default or "").

**code** [LOW] If action.yml declares `default:` with no value (YAML null) or omits `default`, `yaml.safe_load` returns `None` and `str(None)` becomes `"None"`. In GitHub Actions an unset default is effectively the empty string, so the comparison should treat `None` as `""`. Coerce with `input_default = str(input_default or "")`. <!-- wuming:sha256:f70bf55da7d3380066c0bd85154aed42db7602d4f4be27ddc02a3ba0d38d7085 -->
✅ Handle getenv, keyword defaults, and null YAML defaults in parity test
All checks were successful
/ gitleaks (pull_request) Successful in 14s
/ pr-review (pull_request) Successful in 1m8s
/ checks (pull_request) Successful in 1m45s
/ deploy-docs (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
1ce18f6db1
Author
Collaborator

Wave-4 review responses (commit 1ce18f6)

✅ Fixed — defaults collector now covers os.getenv

tests/test_action_inputs.py

The defaults scraper only handles os.environ.get, but _wuming_name_from_call and _config_env_vars also accept os.getenv…

Right — the reader and the defaults extractor now accept the same call set.


✅ Fixed — keyword default= arguments are extracted

tests/test_action_inputs.py

A call like os.environ.get("WUMING_X", default="...") will be recorded as having an empty default…

A _default_argument helper now returns the second positional argument or the default keyword value. Verified against a synthetic module covering all four shapes: positional default, keyword default, no default, and a non-string default — each lands on the right value.


✅ Fixed — YAML null defaults coerced to empty string

tests/test_action_inputs.py

If action.yml declares default: with no value (YAML null) … str(None) becomes "None".

None now coerces to "" before the comparison, matching Actions' treatment of an unset default.

### Wave-4 review responses (commit `1ce18f6`) ### ✅ Fixed — defaults collector now covers `os.getenv` [`tests/test_action_inputs.py`](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5795) > The defaults scraper only handles `os.environ.get`, but `_wuming_name_from_call` and `_config_env_vars` also accept `os.getenv`… Right — the reader and the defaults extractor now accept the same call set. --- ### ✅ Fixed — keyword `default=` arguments are extracted [`tests/test_action_inputs.py`](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5796) > A call like `os.environ.get("WUMING_X", default="...")` will be recorded as having an empty default… A `_default_argument` helper now returns the second positional argument or the `default` keyword value. Verified against a synthetic module covering all four shapes: positional default, keyword default, no default, and a non-string default — each lands on the right value. --- ### ✅ Fixed — YAML null defaults coerced to empty string [`tests/test_action_inputs.py`](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5797) > If action.yml declares `default:` with no value (YAML null) … `str(None)` becomes `"None"`. `None` now coerces to `""` before the comparison, matching Actions' treatment of an unset default.
forgejo-actions left a comment

WuMing

Found 1 issue(s). See inline comments below.

## WuMing Found **1** issue(s). See inline comments below.
@ -124,2 +138,4 @@
WUMING_VERBOSE: ${{ inputs.verbose }}
WUMING_SUMMARY_COMMENT: ${{ inputs.summary_comment }}
WUMING_VERIFY: ${{ inputs.verify }}
WUMING_MIN_SEVERITY: ${{ inputs.min_severity }}

config [MEDIUM]

WUMING_MIN_SEVERITY is always exported, but with an empty string rather than being unset. The description for this input (lines 109-111) promises "Unset by default — everything posts. Invalid values fail closed with a startup error," yet an empty value is present, not absent. If config.py distinguishes os.environ.get("WUMING_MIN_SEVERITY") / None from "" when validating against low|medium|high|critical, the shipped default will trip the fail-closed startup error for every consumer of the action. Either make config.py treat an empty (or whitespace-only) value as "no floor" explicitly, or drop the default: "" from this input and guard the mapping (e.g. only export the variable when a value was supplied) so the variable is genuinely unset by default. Add a test asserting the default action invocation exits cleanly.

**config** [MEDIUM] `WUMING_MIN_SEVERITY` is always exported, but with an empty string rather than being unset. The description for this input (lines 109-111) promises "Unset by default — everything posts. Invalid values fail closed with a startup error," yet an empty value is *present*, not absent. If config.py distinguishes `os.environ.get("WUMING_MIN_SEVERITY")` / `None` from `""` when validating against `low|medium|high|critical`, the shipped default will trip the fail-closed startup error for every consumer of the action. Either make config.py treat an empty (or whitespace-only) value as "no floor" explicitly, or drop the `default: ""` from this input and guard the mapping (e.g. only export the variable when a value was supplied) so the variable is genuinely unset by default. Add a test asserting the default action invocation exits cleanly. <!-- wuming:sha256:3ddf1a6e3945b73779da264961aa01836ef24183fc54d4e99ef79953044b2088 -->
Author
Collaborator

Duplicate of a wave-1 finding (already declined)

action.yml line 113

WUMING_MIN_SEVERITY is always exported, but with an empty string rather than being unset…

Same substance as the wave-1 finding declined in the consolidated wave-1 reply: the premise that config.py might distinguish None from "" is false — _parse_min_severity() returns None for both unset and empty (src/wuming/config.py lines 102–117), pinned by tests/test_config.py::test_min_severity_unset. Present-but-empty remains the standard action.yml contract for optional inputs. No further action.

### Duplicate of a wave-1 finding (already declined) [`action.yml` line 113](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5801) > `WUMING_MIN_SEVERITY` is always exported, but with an empty string rather than being unset… Same substance as the wave-1 finding declined in [the consolidated wave-1 reply](https://forge.marvin8.zone/marvin8/wuming/pulls/108#issuecomment-5778): the premise that config.py might distinguish `None` from `""` is false — `_parse_min_severity()` returns `None` for both unset and empty (`src/wuming/config.py` lines 102–117), pinned by `tests/test_config.py::test_min_severity_unset`. Present-but-empty remains the standard action.yml contract for optional inputs. No further action.
marvin8 approved these changes 2026-10-17 00:54:42 +00:00
marvin8 manually merged commit 8ac997525f into main 2026-10-17 00:56:41 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
marvin8/wuming!108
No description provided.