fix(scripts): detect env reads forwarded through same-file wrapper functions (#8651) - #8709
Conversation
…nctions gen-selfhost-env-reference.ts recognized env-name reads only via a literal, envString(env, "X"), or a registered name-helper called with a literal at the call site. It had no case for a same-file wrapper function that forwards its own string PARAMETER into a recognized sink, so operator-tunable knobs read only through such wrappers never reached the self-host reference table: - src/selfhost/queue-common.ts's envDurationMs(name) -> parsePositiveIntEnv(name, ...): QUEUE_STARTUP_JITTER_MS, QUEUE_RECOVERY_JITTER_MS, QUEUE_PROCESSING_TIMEOUT_MS, QUEUE_DEAD_LETTER_REVIVE_INTERVAL_MS, SCHEDULED_ENQUEUE_JITTER_MS (and QUEUE_RATE_LIMIT_JITTER_MS) - src/selfhost/maintenance-admission.ts's parsePositiveFloatEnv(name) -> process.env[name]: MAINTENANCE_ADMISSION_MAX_HOST_LOAD Add a per-file discovery pass that finds any function forwarding a string parameter, unmodified, into an already-recognized env sink (direct env[param] read, envString/parse*Env helper, literal-arg helper, or another discovered wrapper -- resolved to a fixpoint for wrapper-of-wrapper chains), and treats it like a literal-arg helper at its call sites. Fully general -- no wrapper name is special-cased. Because it generalizes, it also correctly surfaces vars read through the sibling wrappers repoJsonMap() (PAGERDUTY_REPO_ROUTING_KEYS, PAGERDUTY_REPO_COOLDOWN_MINUTES, SLACK_REPO_WEBHOOKS), processEnvString() (LOOPOVER_CENTRAL_POSTHOG_KEY), and configured() -- the last of which is why ANTHROPIC_API_KEY/OPENAI_API_KEY now attribute to their earlier-sorting real read site src/selfhost/ai-config.ts. All are genuine reads. Test: a fixture module exercises every wrapper form (parsePositiveIntEnv/process.env/envString/literal-arg/ chained wrapper, function-declaration/arrow/function-expression) plus non-wrapper and non-literal-call negatives.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8709 +/- ##
=======================================
Coverage 93.78% 93.78%
=======================================
Files 797 797
Lines 79466 79466
Branches 24071 24071
=======================================
Hits 74531 74531
Misses 3563 3563
Partials 1372 1372
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-26 00:27:11 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Problem
Closes #8651.
scripts/gen-selfhost-env-reference.tsrecognized an env-name read only via a literal,envString(env, "X"), or a registered helper called with a literal at the call site. It had no case for a same-file wrapper function that forwards its own string parameter (not a literal) into a recognized sink. Six operator-tunable knobs read only through such wrappers were missing from the reference:queue-common.ts'senvDurationMs(name)→parsePositiveIntEnv(name, …):QUEUE_STARTUP_JITTER_MS,QUEUE_RECOVERY_JITTER_MS,QUEUE_PROCESSING_TIMEOUT_MS,QUEUE_DEAD_LETTER_REVIVE_INTERVAL_MS,SCHEDULED_ENQUEUE_JITTER_MSmaintenance-admission.ts'sparsePositiveFloatEnv(name)→process.env[name]:MAINTENANCE_ADMISSION_MAX_HOST_LOADFix
A per-file discovery pass finds any function forwarding a string parameter, unmodified, into an already-recognized env sink — a direct
env[param]/process.env[param]read, anenvString/parse*Envhelper, a literal-arg helper, or another discovered wrapper (resolved to a fixpoint, so wrapper-of-wrapper chains are caught) — and treats it like a literal-arg helper at its call sites. Fully general: no wrapper name is special-cased.Additional genuine reads surfaced (same mechanism)
Because the fix generalizes, it also correctly surfaces vars read through the sibling wrappers the generator likewise never saw — all verified genuine
env[...]reads:repoJsonMap(env, "…")(notify-pagerduty/notify-discord):PAGERDUTY_REPO_ROUTING_KEYS,PAGERDUTY_REPO_COOLDOWN_MINUTES,SLACK_REPO_WEBHOOKSprocessEnvString(env, "…")(posthog):LOOPOVER_CENTRAL_POSTHOG_KEYconfigured(env, "…")(ai-config) readsANTHROPIC_API_KEY/OPENAI_API_KEY, so theirfirstReferencenow correctly attributes to the earlier-sortingsrc/selfhost/ai-config.ts(they are not dropped).Plus a 6th
envDurationMscall surfacesQUEUE_RATE_LIMIT_JITTER_MS. This is the intended "close the category, not just the 6 instances" outcome.Tests
A fixture module in
test/unit/selfhost-env-reference-script.test.tsexercises every wrapper form (viaparsePositiveIntEnv/process.env/envString/ a literal-arg helper / a chained wrapper; and function-declaration, arrow, and function-expression syntaxes) plus non-wrapper and non-literal-call negatives — covering every new branch.npm run selfhost:env-reference -- --checkpasses;git diff --checkclean.