Repository navigation
Fixes public command blocking (Chat disabled in client options) and optimizes chunk loading. - #573
dangdinhbaohoang12 wants to merge 2 commits into
Conversation
This workflow triggers Datadog Synthetic tests on push and pull request events to the 'next' branch.
📝 WalkthroughWalkthroughThis PR adds two new GitHub Actions workflow files: one for running Datadog Synthetic tests on push/pull_request events targeting the ChangesCI Workflow Additions
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)Not applicable — these are configuration-only CI workflow additions with no multi-component application logic to visualize. Suggested labels: ci, github-actions Suggested reviewers: zardoy 🐇 Two workflows hop into the repo's nest,One tests with Datadog, giving synthetic checks a rest, The other builds Node across versions three, CI now watches, as vigilant as can be. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/datadog-synthetics.yml (2)
27-27: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout.Static analysis flags this:
actions/checkout@v4persists the GITHUB_TOKEN credential in the local git config by default, which is unnecessary since this job never pushes back to the repo.🔒 Proposed fix
steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 Prompt for 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. In @.github/workflows/datadog-synthetics.yml at line 27, The checkout step in the datadog synthetics workflow should disable credential persistence because this job only reads the repo and never pushes changes. Update the existing actions/checkout@v4 usage to set persist-credentials to false so the GITHUB_TOKEN is not stored in local git config.Source: Linters/SAST tools
22-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider declaring explicit least-privilege
permissions.No
permissions:block is set, so the job inherits the default (possibly broader)GITHUB_TOKENscope. Since this job only checks out code and calls an external action, restricting tocontents: readreduces blast radius if a step is compromised.🔒 Proposed fix
jobs: build: runs-on: ubuntu-latest + permissions: + contents: read🤖 Prompt for 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. In @.github/workflows/datadog-synthetics.yml around lines 22 - 26, Add an explicit permissions block to the build job in the datadog-synthetics workflow so the job does not inherit broad default GITHUB_TOKEN scope. In the build job definition, set least-privilege access by granting only the permissions needed for checkout and the external action, using the existing build job section as the place to update. Keep the scope minimal, ideally contents: read only.
🤖 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 @.github/workflows/node.js.yml:
- Line 23: The checkout step in the GitHub Actions workflow is persisting the
default token in git config, which can be leaked by later steps or artifacts.
Update the existing actions/checkout usage in the workflow to disable credential
persistence by setting persist-credentials to false, keeping the fix scoped to
the checkout step itself.
- Line 31: The CI workflow is invoking `npm test`, but the root `package.json`
does not define a plain `test` script, so this step won’t run the intended
suite. Update the job in the Node.js workflow to call the appropriate existing
test script(s) from `package.json` (for example the relevant `test:*` target),
or add a real `test` script if that is the intended entrypoint. Use the workflow
step currently running `npm test` as the place to fix it.
---
Nitpick comments:
In @.github/workflows/datadog-synthetics.yml:
- Line 27: The checkout step in the datadog synthetics workflow should disable
credential persistence because this job only reads the repo and never pushes
changes. Update the existing actions/checkout@v4 usage to set
persist-credentials to false so the GITHUB_TOKEN is not stored in local git
config.
- Around line 22-26: Add an explicit permissions block to the build job in the
datadog-synthetics workflow so the job does not inherit broad default
GITHUB_TOKEN scope. In the build job definition, set least-privilege access by
granting only the permissions needed for checkout and the external action, using
the existing build job section as the place to update. Keep the scope minimal,
ideally contents: read only.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb16cbd0-3890-4b85-be17-b8de03c4b5ec
📒 Files selected for processing (2)
.github/workflows/datadog-synthetics.yml.github/workflows/node.js.yml
| # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on checkout.
Static analysis flags the checkout step for credential persistence (artipacked); the default GITHUB_TOKEN is left in git config after checkout and could be leaked by later steps/artifacts.
Proposed fix
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 23-23: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for 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.
In @.github/workflows/node.js.yml at line 23, The checkout step in the GitHub
Actions workflow is persisting the default token in git config, which can be
leaked by later steps or artifacts. Update the existing actions/checkout usage
in the workflow to disable credential persistence by setting persist-credentials
to false, keeping the fix scoped to the checkout step itself.
Source: Linters/SAST tools
| cache: 'npm' | ||
| - run: npm ci | ||
| - run: npm run build --if-present | ||
| - run: npm test |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
npm test likely won't run the intended suite.
Per the referenced package.json snippet, the repo defines the root package.json defines a build script and multiple test:* scripts, but it does not define a plain test script. If no test script exists, npm test will either fail (npm ≥7 errors when script missing) or silently no-op, giving false CI confidence across all node-version matrix jobs.
Proposed fix
- - run: npm test
+ - run: npm run test-unit --if-present📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - run: npm test | |
| - run: npm run test-unit --if-present |
🤖 Prompt for 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.
In @.github/workflows/node.js.yml at line 31, The CI workflow is invoking `npm
test`, but the root `package.json` does not define a plain `test` script, so
this step won’t run the intended suite. Update the job in the Node.js workflow
to call the appropriate existing test script(s) from `package.json` (for example
the relevant `test:*` target), or add a real `test` script if that is the
intended entrypoint. Use the workflow step currently running `npm test` as the
place to fix it.
1 participant
🐛 Fixes Public Command Blocking (Chat disabled in client options) and Optimizes Chunk Loading
📝 Overview
This Pull Request fixes an issue where players couldn't execute any commands (e.g.,
/clan resign,/menu,...) except for the/logincommand. When executing the command, the system displayed a system error message:Chat disabled in client options. It also optimizes the visibility configuration mechanism to address incomplete chunk loading.🔍 Root Cause Analysis
Chat disabled in client options): - This error originates from the Minecraft server side when receiving the client configuration packet (client_settings) containing the chat visibility flagchatVisibilityset to level2(Hidden).The
bot.settings.chatvariable in themineflayerlibrary requires the correct strings ('enabled','commandsOnly','disabled'). In the current source code, this property is incorrectly configured by default or mismapped from UI data, leading the server to mistakenly believe the player has disabled chat in their personal settings and block the execution of normal commands.The
/logincommand still works because authentication plugins (like AuthMe) on the server actively bypass client settings filters to prevent players from getting stuck. Players still see other players' chat messages because the server forwards chat asSystem Message(mandatory display) through the ViaVersion multi-version port.viewDistance = 3. This causes the grid plotter (prismarine-viewer) to constantly report rendering errors for non-outstanding sections, resulting in missing or slow-loading chunks around the player when moving.🛠️ Solutions (Changes Made)
Bot Initialization Configuration Modification: Update the initialization parameter in
mineflayer.createBot, changing thechatproperty configuration to'enabled'instead of the old error values.Settings Mapping Standardization: Modify the
bot.setSettings()update function to ensure that regardless of user changes on-screen UI options (enabling/disabling the Chat UI button), the value sent to the Mineflayer packet is always correctly translated to the string'enabled'.Improved Render Vision: Increase the default vision parameter or add constraint handling so that
viewDistancedoesn't fall into excessively low values causing incomplete chunk loading errors.🧪 Test Results (Testing & Verification)
The
/logincommand works normally as before.Tested typing common commands such as
/clan resign,/clan, the system executed successfully and was no longer blocked by the server with the messageChat disabled in client options.Summary by CodeRabbit
nextbranch.