Skip to content

chore(lint): remove underscore-dangle allow list#83542

Merged
steipete merged 5 commits into
mainfrom
lint/reduce-underscore-exceptions
May 18, 2026
Merged

chore(lint): remove underscore-dangle allow list#83542
steipete merged 5 commits into
mainfrom
lint/reduce-underscore-exceptions

Conversation

@steipete

@steipete steipete commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • removes the eslint/no-underscore-dangle allow list entirely; the rule is now plain error
  • renames internal test helpers/state to non-underscore names across core, UI, scripts, and plugins
  • preserves public/deprecated compatibility aliases where they are part of the SDK/plugin surface, and keeps wire/runtime keys as bracket or literal access
  • keeps the bundled __OPENCLAW_VERSION__ define as a direct identifier with a narrow inline lint suppression and regression test

Real behavior proof

Behavior addressed: the underscore-dangle rule has no repository allow list anymore; compatibility aliases and wire keys remain represented explicitly in code instead of via a global exception list.

Real environment tested: local OpenClaw source checkout at PR head ca556c1bbf on macOS.

Exact steps or command run after this patch: node -e 'const fs=require("node:fs"); const cfg=JSON.parse(fs.readFileSync(".oxlintrc.json","utf8")); const rule=cfg.rules["eslint/no-underscore-dangle"]; console.log(JSON.stringify({rule,hasAllowList:Boolean(rule && typeof rule==="object" && "allow" in rule)}));'.

Evidence after fix: console output from the command above:

{"rule":"error","hasAllowList":false}

Observed result after fix: the configured rule value is the string error, and hasAllowList is false; there is no allow-list payload left in .oxlintrc.json.

What was not tested: no live provider/channel traffic was exercised; this is an internal rule/config and test-helper cleanup with compatibility aliases preserved.

Verification

  • git diff --check
  • node scripts/run-oxlint.mjs
  • node scripts/run-oxlint.mjs scripts/check-plugin-sdk-exports.mjs src/media-understanding/runner.auto-audio.test.ts
  • node scripts/run-vitest.mjs src/version.test.ts
  • node scripts/run-vitest.mjs src/media-understanding/runner.auto-audio.test.ts
  • node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts src/agents/subagent-announce-delivery.test.ts src/commands/doctor-auth-oauth-sidecar.test.ts src/infra/resolve-system-bin.test.ts src/media-understanding/runner.vision-skip.test.ts extensions/slack/src/monitor.threading.missing-thread-ts.test.ts extensions/slack/src/monitor/message-handler/prepare.test.ts src/media-understanding/runner.cli-audio.test.ts src/media-understanding/runtime.test.ts extensions/slack/src/monitor/monitor.thread-resolution.test.ts src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts src/version.test.ts
  • node scripts/build-all.mjs ciArtifacts
  • Testbox: provider blacksmith-testbox, id tbx_01krxm3z9vkbckwkyqv50ahapa, Actions https://github.com/openclaw/openclaw/actions/runs/26036295762, command pnpm check:changed, exit 0
  • AUTOREVIEW_OPENCLAW_MAINTAINER_VALIDATION=1 codex review --base origin/main

@steipete steipete requested a review from a team as a code owner May 18, 2026 09:39
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime channel: qqbot size: M maintainer Maintainer-authored PR labels May 18, 2026
@clawsweeper

clawsweeper Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The PR removes the underscore-dangle allow list from .oxlintrc.json and mechanically renames underscore-prefixed internal helpers/state across core, UI, scripts, tests, and plugins while preserving selected compatibility aliases.

Reproducibility: not applicable. this is a cleanup PR rather than a bug report with a failing user path. The relevant checks are diff review, lint proof, and merge-head CI/Testbox validation.

PR rating
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Summary: Strong terminal proof and no blocking findings, with overall confidence limited mainly by the very broad mechanical surface.

Rank-up moves:

  • none
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal proof from Blacksmith Testbox plus local lint/test commands for the internal cleanup.

Risk before merge
Why this matters: - The patch is a very broad mechanical cleanup, so final landing should still rely on required CI/Testbox checks for the exact merge head; this read-only review did not run tests locally.

Maintainer options:

  1. Decide the mitigation before merge
    Proceed through maintainer review and final CI/Testbox gating, then land the lint cleanup if the exact merge head remains green.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
Protected maintainer label and XL mechanical surface mean the next step is maintainer review plus final CI, not an automated repair or cleanup close.

Security
Cleared: The diff does not add dependencies, workflow execution, lockfile changes, secret handling, package resolution changes, or downloaded code paths.

Review details

Best possible solution:

Proceed through maintainer review and final CI/Testbox gating, then land the lint cleanup if the exact merge head remains green.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a cleanup PR rather than a bug report with a failing user path. The relevant checks are diff review, lint proof, and merge-head CI/Testbox validation.

Is this the best way to solve the issue?

Yes; removing the central allow list while preserving compatibility aliases for selected public/deprecated helper exports is the maintainable direction. The broad surface makes final automated validation important before landing.

Label justifications:

  • P3: This is a low-risk lint hygiene cleanup with no intended user-facing behavior change.

What I checked:

  • Protected label: The provided live GitHub context lists the protected maintainer label, so this workflow should not close the PR automatically.
  • Merge-head diff scope: The synthetic merge commit applies the PR to current main and changes 739 files with 3220 insertions and 3210 deletions, centered on the lint rule and mechanical identifier cleanup. (f6cb93ecc1e8)
  • Lint rule change: The merge result sets eslint/no-underscore-dangle to a plain error rule with no allow list. (.oxlintrc.json:11, f6cb93ecc1e8)
  • Compatibility alias sample: Sampled renamed test helpers keep old __testing export aliases where the PR preserves compatibility. (extensions/acpx/src/runtime.ts:1261, f6cb93ecc1e8)
  • Bundled version define handling: The PR keeps the required __OPENCLAW_VERSION__ direct identifier behind a narrow lint suppression and adds a source regression check for that shape. (src/version.ts:4, f6cb93ecc1e8)
  • Diff hygiene: The merge-result diff has no git diff --check whitespace errors. (f6cb93ecc1e8)

Likely related people:

  • Ayaan Zaidi: Current-main blame and log history in this checkout point to commit 53d14d0 as the recent owner of the central lint/config/version/UI surfaces sampled for this PR. (role: recent area contributor; confidence: medium; commits: 53d14d0561f6; files: .oxlintrc.json, src/version.ts, ui/vite.config.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6a5a1353c7f0.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels May 18, 2026
@openclaw-barnacle openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: L and removed size: M labels May 18, 2026
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: matrix Channel integration: matrix channel: mattermost Channel integration: mattermost channel: nextcloud-talk Channel integration: nextcloud-talk channel: nostr Channel integration: nostr channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: tlon Channel integration: tlon channel: voice-call Channel integration: voice-call channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo channel: zalouser Channel integration: zalouser labels May 18, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 18, 2026
@steipete

Copy link
Copy Markdown
Contributor Author

Verification before merge:

Behavior addressed: removed the eslint/no-underscore-dangle allow list and kept the rule at plain error.
Real environment tested: local checkout plus Blacksmith Testbox and GitHub Actions.
Exact steps or command run after this patch:

  • node scripts/run-oxlint.mjs
  • node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts src/agents/subagent-announce-delivery.test.ts src/commands/doctor-auth-oauth-sidecar.test.ts src/infra/resolve-system-bin.test.ts src/media-understanding/runner.vision-skip.test.ts extensions/slack/src/monitor.threading.missing-thread-ts.test.ts extensions/slack/src/monitor/message-handler/prepare.test.ts src/media-understanding/runner.cli-audio.test.ts src/media-understanding/runtime.test.ts extensions/slack/src/monitor/monitor.thread-resolution.test.ts src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts src/version.test.ts
  • node scripts/run-vitest.mjs src/media-understanding/runner.auto-audio.test.ts
  • node scripts/build-all.mjs ciArtifacts
  • node scripts/run-vitest.mjs test/scripts/lint-suppressions.test.ts
  • AUTOREVIEW_OPENCLAW_MAINTAINER_VALIDATION=1 codex review --base origin/main
  • Testbox pnpm check:changed, provider blacksmith-testbox, id tbx_01krxm3z9vkbckwkyqv50ahapa, run https://github.com/openclaw/openclaw/actions/runs/26036295762
  • CI run https://github.com/openclaw/openclaw/actions/runs/26037882030
    Evidence after fix: local checks passed; Testbox pnpm check:changed exited 0; CI completed success.
    Observed result after fix: underscore-dangle exceptions are gone from the rule config; the only remaining suppression is the explicit bundled version define identifier in src/version.ts.
    What was not tested: no live provider/channel scenario; this is lint/test/internal cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: web-ui App: web-ui channel: discord Channel integration: discord channel: feishu Channel integration: feishu channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: matrix Channel integration: matrix channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: nextcloud-talk Channel integration: nextcloud-talk channel: nostr Channel integration: nostr channel: qqbot channel: slack Channel integration: slack channel: synology-chat channel: telegram Channel integration: telegram channel: tlon Channel integration: tlon channel: twitch Channel integration: twitch channel: voice-call Channel integration: voice-call channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo channel: zalouser Channel integration: zalouser cli CLI command changes commands Command implementations docker Docker and sandbox tooling extensions: acpx extensions: amazon-bedrock extensions: anthropic extensions: brave extensions: cloudflare-ai-gateway extensions: codex extensions: device-pair extensions: diagnostics-prometheus extensions: duckduckgo extensions: elevenlabs extensions: fal extensions: firecrawl extensions: github-copilot extensions: google extensions: lmstudio extensions: lobster Extension: lobster extensions: memory-core Extension: memory-core extensions: memory-lancedb Extension: memory-lancedb extensions: minimax extensions: mistral extensions: moonshot extensions: ollama extensions: openai extensions: openrouter extensions: perplexity extensions: qa-lab extensions: tavily extensions: xai gateway Gateway runtime maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. plugin: google-meet proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant