ci: gate NEMOCLAW_* env vars against documentation drift (#3184)#3187
Merged
Conversation
Adds a CI gate that fails if any NEMOCLAW_* env var read in src/ or bin/ is missing from docs/reference/commands.md and not explicitly allowlisted in ci/env-var-doc-allowlist.json with a real reason. Surfaced from #3116, where NEMOCLAW_OLLAMA_PULL_TIMEOUT shipped without a doc entry and was caught externally by NV QA rather than by our own pipeline. Changes: - scripts/check-env-var-docs.ts: TypeScript-AST extractor + auditor. Skips assignments, deletes, and comments. Validates the allowlist for short reasons, invalid name pattern, and simultaneous doc + allowlist. - test/check-env-var-docs.test.ts: 26 unit tests (extractor, doc parsing, allowlist parsing, full audit including stale + invalid entry detection). - ci/env-var-doc-allowlist.json: two genuinely internal vars (NEMOCLAW_DISABLE_AUTO_DISPATCH test harness sentinel, NEMOCLAW_TEST_NO_SLEEP Vitest sleep bypass), each with a real reason. - docs/reference/commands.md: 24 newly documented vars across three subsections — Onboarding Configuration (provider, endpoint URL, proxy, build-time model overrides), Onboarding Behavior Flags (NEMOCLAW_YES, NEMOCLAW_EXPERIMENTAL, etc.), and Probe Timeouts (sandbox exec + status probe ms overrides). - .pre-commit-config.yaml: new env-var-docs hook (priority 10) that runs whenever src/, bin/, the docs file, the allowlist, or the gate script changes. Verified the gate fails on: - a new process.env.NEMOCLAW_X read with no doc and no allowlist entry, - stale allowlist entries (vars no longer read in src/), - allowlist entries with too-short reasons or invalid name patterns, - vars that are both documented and allowlisted (must pick one). Signed-off-by: Charan Jagwani <charjags100@gmail.com>
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a static doc-drift gate: a TypeScript auditor extracts NEMOCLAW_* env var reads from source, compares them to commands.md and a CI allowlist, reports undocumented/stale/invalid entries, wires the check into pre-commit, adds the allowlist, and includes tests. ChangesEnvironment Variable Documentation Enforcement Gate
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.md`:
- Line 1002: The sentence "These tune how long internal probes wait before
giving up. Defaults are sized for typical hardware; override only if you see
false-positive timeouts." contains two sentences on one line; split them into
two separate lines so each sentence appears on its own line in
docs/reference/commands.md (preserve both sentences and punctuation exactly,
e.g., first line "These tune how long internal probes wait before giving up."
and second line "Defaults are sized for typical hardware; override only if you
see false-positive timeouts.").
- Line 968: The two sentences in the docs string "These variables let you tune
onboarding without editing the Dockerfile or passing repeated flags. Set them
before running `nemoclaw onboard`." should be split into two separate lines so
each sentence occupies its own line for diff-readability; edit the
docs/reference/commands.md entry to place the first sentence on one line and the
second sentence on the next line, preserving punctuation and the inline code
token `nemoclaw onboard`.
- Around line 988-990: Add a one-line introductory sentence immediately after
the "### Onboarding Behavior Flags" heading and before the table to describe
what the flags control (e.g., "These flags control the onboarding flow and
related feature toggles."). Update the section around the "### Onboarding
Behavior Flags" heading so the table is preceded by that brief explanatory
sentence, matching the pattern used by the sibling sections "Onboarding
Configuration" and "Probe Timeouts".
In `@scripts/check-env-var-docs.ts`:
- Around line 144-166: The walk function uses statSync without error handling
which can throw on broken symlinks or race conditions; wrap the statSync(full)
call in a try/catch (similar to readdirSync) and on error simply continue/skip
that entry (optionally log/debug the error) so the script doesn't crash; update
references in the walk function around the statSync usage and keep the existing
behavior of recursing for directories and pushing files into out when stat
succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5886e8a7-820a-4634-9da4-549548e853a1
📒 Files selected for processing (5)
.pre-commit-config.yamlci/env-var-doc-allowlist.jsondocs/reference/commands.mdscripts/check-env-var-docs.tstest/check-env-var-docs.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a CI gate that fails if any
NEMOCLAW_*env var read insrc/orbin/is missing fromdocs/reference/commands.mdand not explicitly allowlisted with a real reason. Closes the doc-drift class of bug surfaced by #3116, whereNEMOCLAW_OLLAMA_PULL_TIMEOUTshipped without a doc entry and was caught externally by NV QA rather than by our own pipeline.Related Issue
Closes #3184
Changes
scripts/check-env-var-docs.ts— TypeScript-AST extractor + auditor (~250 lines). Skips assignments, deletes, and comments. Validates the allowlist for short reasons, invalid name pattern, and simultaneous doc + allowlist.test/check-env-var-docs.test.ts— 26 unit tests covering extractor (property/element access, assignment/delete exclusion, non-NEMOCLAWexclusion, comment exclusion), doc parsing, allowlist parsing, and full audit (undocumented, stale-allowlist, invalid-allowlist, simultaneous-doc-and-allowlist).ci/env-var-doc-allowlist.json— two genuinely internal vars (NEMOCLAW_DISABLE_AUTO_DISPATCHtest harness sentinel,NEMOCLAW_TEST_NO_SLEEPVitest sleep bypass), each with a real reason.docs/reference/commands.md— 24 newly documented vars across three new subsections:NEMOCLAW_PROVIDER,NEMOCLAW_ENDPOINT_URL,NEMOCLAW_AGENT_TIMEOUT,NEMOCLAW_CONTEXT_WINDOW,NEMOCLAW_MAX_TOKENS,NEMOCLAW_REASONING,NEMOCLAW_INFERENCE_INPUTS,NEMOCLAW_PROXY_HOST/PORT,NEMOCLAW_OLLAMA_REQUIRE_TOOLS,NEMOCLAW_OPENSHELL_BIN,NEMOCLAW_SANDBOX,NEMOCLAW_INSTALL_REF/TAG,NEMOCLAW_PREFERRED_API).NEMOCLAW_YES,NEMOCLAW_EXPERIMENTAL,NEMOCLAW_IGNORE_RUNTIME_RESOURCES,NEMOCLAW_DISABLE_OVERLAY_FIX,NEMOCLAW_OVERLAY_SNAPSHOTTER,NEMOCLAW_SKIP_TELEGRAM_REACHABILITY,NEMOCLAW_CONFIG_ACCEPT_NEW_PATH.NEMOCLAW_SANDBOX_EXEC_TIMEOUT_MS,NEMOCLAW_STATUS_PROBE_TIMEOUT_MS..pre-commit-config.yaml— newenv-var-docshook (priority 10) that runs wheneversrc/,bin/, the docs file, the allowlist, or the gate script changes.Triage outcome
Initial naive grep over
src/showed 63 "undocumented"NEMOCLAW_*vars. The AST audit narrowed that to 26 actually read viaprocess.envinsrc/+bin/— the other 37 were string literals, constants, or comment text that the regex matched but the extractor (correctly) ignores. Of those 26: 24 documented, 2 allowlisted.Type of Change
Verification
npx prek run --all-filespasses (commit went through pre-commit hooks including coverage ratchet, markdownlint, biome, gitleaks).npm testpasses — 26 new unit tests intest/check-env-var-docs.test.ts.Verified the gate fails on each of:
process.env.NEMOCLAW_Xread with no doc and no allowlist entry,src/),Summary by CodeRabbit
New Features
Documentation
Chores / Tests