[groom] grade-targets.sh is a script and a library at once, and the seam leaks into collect-pr-inputs.sh

[cloud-code-bot]

Groom auto-builder — Comfy-Org/github-workflows: this CONFIRMED finding could not be auto-built (patch modifies a CI-privileged path (per .github/groom/patch_policy.py — workflow/action defs, lockfiles, or build/test config that executes in pre-review CI) — a human must author these changes), so it is filed for a human. · run

Verified facts

scripts/pr-derisk/collect-pr-inputs.sh:77 sources the 619-line scripts/pr-risk/grade-targets.sh for exactly three helpers - resolve_base_ref, fetch_override, retry_read - as its own comment at :72-76 states. Making that safe cost two workarounds, both properties of the file boundary rather than of the logic:

1. scripts/pr-risk/grade-targets.sh:114-132 - a GT_DIRECT flag plus lazy init_scratch, added (per its own comment) because at file scope the EXIT trap replaced the SOURCING shell's trap, so a suite that sourced these helpers silently lost its sandbox cleanup and leaked both the sandbox and the temp files.
2. scripts/pr-derisk/collect-pr-inputs.sh:50-63 and :78-86 - log/warn/die defined TWICE, before and after the source, because grade-targets.sh:111-112 defines its own log/die at file scope and captures them. The first copy carries ten lines of comment plus a shellcheck disable for SC2317 and SC2329 to keep two linter versions quiet.

Both files are already WATCHED_EXEC entries of their fleets (bump-pr-risk-callers.yml:181, bump-pr-derisk-callers.yml:219), and bump-pr-derisk-callers.yml:195-203 already documents the sourcing relationship, so the coupling is known and load-bearing rather than accidental.

Fix

Move the sourceable helpers - enc, enc_path, init_scratch/gherr, retryable_err, retry_read, resolve_base_ref, fetch_override (grade-targets.sh:125-289) - into a new scripts/pr-risk/lib.sh with NO top-level side effects and PREFIXED diagnostic names (e.g. gt_log / gt_die) so they cannot capture a caller's log/die. The precedent already exists here: scripts/area-label/lib.sh. Then:

• grade-targets.sh sources it and keeps main() and the per-target logic. With nothing else sourcing it, GT_DIRECT and its lazy-trap dance can go and the trap can be installed unconditionally.
• collect-pr-inputs.sh sources lib.sh instead; the duplicated log/warn/die block and both shellcheck suppressions disappear.

Behaviour must be identical. scripts/pr-risk/tests/test_grade_targets.sh and scripts/pr-derisk/tests/test_plan_derisk.sh must stay green, and the shellcheck -x list in .github/workflows/test-pr-risk.yml:51 must gain the new file.

Risk

Medium and concrete: scripts/pr-risk/lib.sh becomes a run-time dependency of BOTH fleets, so WATCHED_EXEC in bump-pr-risk-callers.yml AND bump-pr-derisk-callers.yml must gain it in the same commit - a missing entry could re-point pinned callers at a SHA where the file is absent, which is exactly what those lists exist to prevent. Scope the move to the helpers listed above; do not opportunistically move parse_targets, settle_grade, process_target or main.

Steelman

The current arrangement works, is documented in painful detail and is pinned by tests; a lib.sh adds a third file to a two-file relationship and one more WATCHED_EXEC entry a caller breaks without. If the sourcing were a one-off it would not be worth touching. The counter is that the file is now maintained under two contradictory contracts at once, and every future helper added to it inherits both hazards - the trap one already bit.