Skip to content

Fix BigInt gcd and lcm edge cases - #6870

Merged
gcanti merged 2 commits into
mainfrom
audit/repro-core-big-int-gcd-lcm
Aug 2, 2026
Merged

Fix BigInt gcd and lcm edge cases#6870
gcanti merged 2 commits into
mainfrom
audit/repro-core-big-int-gcd-lcm

Conversation

@fubhy

@fubhy fubhy commented Aug 1, 2026

Copy link
Copy Markdown
Member

Reproduction only

This PR adds reproduction tests only. No implementation fix is included. CI is expected to fail until the underlying issue is fixed.

Covered audit issues

1. core-a-f-bigint-gcd-negative: gcd can return a negative divisor

Module: BigInt

Expected contract: gcd determines the greatest common divisor of two bigint integer values; a greatest common divisor is non-negative.

Observed result: The accepted sign assertions failed: gcd(-6n, 4n) returned -2n and lcm(6n, -4n) returned -12n.

Reproduction command:

pnpm test --run packages/effect/test/BigIntGcdLcm.test.ts

2. core-a-f-bigint-lcm-negative: lcm can return a negative multiple

Module: BigInt

Expected contract: lcm determines the least common multiple of two bigint integer values; a least common multiple is non-negative.

Observed result: The accepted sign assertions failed: gcd(-6n, 4n) returned -2n and lcm(6n, -4n) returned -12n.

Reproduction command:

pnpm test --run packages/effect/test/BigIntGcdLcm.test.ts

@github-project-automation github-project-automation Bot moved this to Discussion Ongoing in PR Backlog Aug 1, 2026
@changeset-bot

changeset-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 392b148

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
effect Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-deno Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/vitest Patch
@effect/ai-anthropic Patch
@effect/ai-openai-compat Patch
@effect/ai-openai Patch
@effect/ai-openrouter Patch
@effect/atom-react Patch
@effect/atom-solid Patch
@effect/atom-vue Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-pglite Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/docgen Patch
@effect/doctest Patch
@effect/openapi-generator Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@effect-slopcop effect-slopcop Bot added the 4.0 label Aug 1, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — 1 new test file with 3 reproduction cases for BigInt gcd and lcm sign bugs.

  • BigIntGcdLcm.test.ts: tests that gcd(-6n, 4n) → 2n, lcm(6n, -4n) → 12n, and lcm(0n, 0n) → 0n

All three assertions use exact strictEqual on values the current implementation cannot satisfy (gcd returns -2n for negative inputs, lcm inherits the sign issue, and lcm(0, 0) divides by zero). Tests match the existing BigInt.test.ts conventions.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏

@gcanti
gcanti force-pushed the audit/repro-core-big-int-gcd-lcm branch from 8fd4fda to 392b148 Compare August 2, 2026 08:08
@gcanti gcanti changed the title Add reproduction for BigInt issue Fix BigInt gcd and lcm edge cases Aug 2, 2026
@effect-slopcop effect-slopcop Bot added the bug Something isn't working label Aug 2, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the implementation fix and test integration since the previous review (commit 8fd4fda).

  • BigInt.gcd fix: wraps the Euclidean algorithm result in abs() so negative inputs produce a non-negative GCD
  • BigInt.lcm fix: returns 0n when either operand is zero (avoiding division-by-zero), and wraps the result in abs() so negative operands produce a non-negative LCM
  • Test migration: moved reproduction tests from a separate file into BigInt.test.ts and migrated all assertions from strictEqual (test utils) to assert.strictEqual (@effect/vitest)

All 13 tests pass.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name Current Size Previous Size Difference
basic.ts 6.63 KB 6.63 KB 0.00 KB (0.00%)
batching.ts 9.42 KB 9.42 KB 0.00 KB (0.00%)
brand.ts 6.31 KB 6.31 KB 0.00 KB (0.00%)
cache.ts 10.18 KB 10.18 KB 0.00 KB (0.00%)
config.ts 20.32 KB 20.32 KB 0.00 KB (0.00%)
differ.ts 19.93 KB 19.93 KB 0.00 KB (0.00%)
http-client.ts 21.02 KB 21.02 KB 0.00 KB (0.00%)
logger.ts 10.32 KB 10.32 KB 0.00 KB (0.00%)
metric.ts 8.55 KB 8.55 KB 0.00 KB (0.00%)
optic.ts 7.33 KB 7.33 KB 0.00 KB (0.00%)
pubsub.ts 14.47 KB 14.47 KB 0.00 KB (0.00%)
queue.ts 11.13 KB 11.13 KB 0.00 KB (0.00%)
schedule.ts 10.31 KB 10.31 KB 0.00 KB (0.00%)
schema-class.ts 18.86 KB 18.86 KB 0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts 28.67 KB 28.67 KB 0.00 KB (0.00%)
schema-representation-roundtrip.ts 24.99 KB 24.99 KB 0.00 KB (0.00%)
schema-string-transformation.ts 12.99 KB 12.99 KB 0.00 KB (0.00%)
schema-string.ts 10.65 KB 10.65 KB 0.00 KB (0.00%)
schema-template-literal.ts 14.85 KB 14.85 KB 0.00 KB (0.00%)
schema-toArbitraryLazy.ts 21.66 KB 21.66 KB 0.00 KB (0.00%)
schema-toCodeDocument.ts 24.08 KB 24.08 KB 0.00 KB (0.00%)
schema-toCodecJson.ts 18.91 KB 18.91 KB 0.00 KB (0.00%)
schema-toEquivalence.ts 18.73 KB 18.73 KB 0.00 KB (0.00%)
schema-toFormatter.ts 18.59 KB 18.59 KB 0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts 22.33 KB 22.33 KB 0.00 KB (0.00%)
schema-toRepresentation.ts 19.25 KB 19.25 KB 0.00 KB (0.00%)
schema.ts 18.12 KB 18.12 KB 0.00 KB (0.00%)
stm.ts 12.11 KB 12.11 KB 0.00 KB (0.00%)
stream.ts 9.37 KB 9.37 KB 0.00 KB (0.00%)

@gcanti
gcanti merged commit cd122b9 into main Aug 2, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this from Discussion Ongoing to Done in PR Backlog Aug 2, 2026
@gcanti
gcanti deleted the audit/repro-core-big-int-gcd-lcm branch August 2, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.0 bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants