Skip to content

Comments

fix(doctor): preserve precision of large Discord snowflake IDs in --fix#22524

Open
jmasson wants to merge 4 commits intoopenclaw:mainfrom
jmasson:fix/doctor-discord-id-precision
Open

fix(doctor): preserve precision of large Discord snowflake IDs in --fix#22524
jmasson wants to merge 4 commits intoopenclaw:mainfrom
jmasson:fix/doctor-discord-id-precision

Conversation

@jmasson
Copy link

@jmasson jmasson commented Feb 21, 2026

Summary

  • Problem: doctor --fix silently corrupts Discord snowflake IDs that exceed Number.MAX_SAFE_INTEGER. JSON5.parse() rounds the bare integer, then the repair stringifies the lossy number — producing a valid-looking but wrong ID.
  • Why it matters: The original Discord ID digits are lost permanently, breaking authorization.
  • What changed: maybeRepairDiscordNumericIds now accepts the raw config file text and extracts original digit sequences via regex before conversion. Handles collisions where multiple distinct IDs round to the same Number by consuming originals in file order.
  • What did NOT change (scope boundary): No changes to schema validation, config parsing, or other repair functions.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • doctor --fix now preserves the exact original digits of large Discord IDs instead of silently truncating them.
  • Change output includes (recovered original precision from file) when raw text recovery is used.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Any
  • Runtime/container: Node.js
  • Integration/channel (if any): Discord

Steps

  1. Write openclaw.json with a bare numeric Discord ID exceeding MAX_SAFE_INTEGER, e.g. { "channels": { "discord": { "allowFrom": [100000000000000001] } } }
  2. Run openclaw doctor --fix
  3. Check the written config

Expected

  • allowFrom contains "100000000000000001" (original digits preserved)

Actual

  • Before fix: allowFrom contains "100000000000000000" (truncated)
  • After fix: allowFrom contains "100000000000000001" (correct)

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Two new e2e tests: single large ID precision preservation, and multiple IDs that round to the same Number.

Human Verification (required)

  • Verified scenarios: All 14 e2e tests pass including 2 new precision tests
  • Edge cases checked: Multiple IDs colliding to same lossy Number value
  • What you did not verify: Manual end-to-end run with a real Discord bot token

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: Revert commit; the repair falls back to String(entry) when raw text is unavailable
  • Files/config to restore: None
  • Known bad symptoms reviewers should watch for: Discord IDs in config not matching expected values after doctor --fix

Risks and Mitigations

  • Risk: Regex extracts a bare integer from raw text that isn't actually a Discord ID (e.g. in a comment or unrelated field).
    • Mitigation: The lookup is keyed by the lossy Number value, so it only matches when the parsed config already contains that exact number in a Discord ID list. False positives would require an unrelated bare integer that happens to round to the same Number as a Discord ID in the same config file.

🤖 Generated with Claude Code

Greptile Summary

This PR fixes a critical data corruption bug in doctor --fix where large Discord snowflake IDs exceeding Number.MAX_SAFE_INTEGER were silently truncated during repair.

Key changes:

  • maybeRepairDiscordNumericIds now accepts raw config file text and extracts original digit sequences via regex before JSON parsing
  • Handles collision cases where multiple distinct IDs round to the same Number by storing them in an array and consuming in file order (matching JSON parse order)
  • Scopes regex extraction to only the Discord config section using brace-depth tracking with proper string literal handling
  • Adds string-skipping logic to correctly handle braces and quotes inside JSON strings
  • Includes comprehensive test coverage for both single large ID preservation and collision scenarios

All previously identified issues from review threads have been addressed.

Confidence Score: 4/5

  • Safe to merge with minor risk around edge cases in Discord config section extraction
  • All previously identified critical bugs have been fixed (map collisions, string handling, test coverage). The implementation correctly handles the complex case of multiple IDs rounding to the same Number. String-skipping logic properly handles escape sequences. However, deducting one point because the regex-based extraction from raw text could theoretically match unrelated 16+ digit numbers in comments or other Discord fields that happen to round to the same value as actual IDs, though this is acknowledged as a known risk with reasonable mitigation.
  • No files require special attention - all critical issues from previous review have been addressed

Last reviewed commit: 9791c9c

When `openclaw.json` contains a bare numeric Discord ID exceeding
`Number.MAX_SAFE_INTEGER`, `JSON5.parse()` silently rounds it. The
repair function then called `String()` on the lossy number, producing
a valid-looking but wrong ID.

Fix by extracting original digit sequences from the raw config text
before conversion, so `maybeRepairDiscordNumericIds` can recover the
exact original value instead of stringifying the rounded number.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openclaw-barnacle openclaw-barnacle bot added commands Command implementations size: S labels Feb 21, 2026
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Store an array of original digit strings per lossy Number key and
shift() during repair so each occurrence gets the correct original
value. Adds a test for the collision case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jmasson
Copy link
Author

jmasson commented Feb 21, 2026

@greptile-apps review

1 similar comment
@jmasson
Copy link
Author

jmasson commented Feb 21, 2026

@greptile-apps review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Extract only the "discord" object from raw text using brace-depth
tracking before scanning for large integers. Handles both quoted and
unquoted JSON5 keys. Prevents numbers from unrelated config fields
from polluting the precision recovery lookup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jmasson
Copy link
Author

jmasson commented Feb 21, 2026

@greptile_apps review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Braces inside JSON string values (e.g. "note": "Use } here") would
throw off the depth counter when extracting the discord config section.
Now skips over single- and double-quoted strings, handling escapes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jmasson
Copy link
Author

jmasson commented Feb 21, 2026

@greptile-apps review

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

Labels

commands Command implementations size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant