test(semver): measure the ReDoS guard as best-of-N, not one sample

[lidge-jun]

Summary

• The ReDoS guard timed a single parse against a 50ms budget. On a loaded macOS runner that sample came back at 53.77ms and failed the v2.41.0 promotion ([WRONG BRANCH] release: v2.41.0 #3342) — for a parse whose subject regression is 522ms, three orders of magnitude away.
• A first call also carries regex compilation and JIT warm-up that the parse itself does not, so the single-sample form was measuring the runner as much as the pattern.
• A gate that fires on runner weather teaches everyone to re-run it, which is exactly how a real ReDoS regression would eventually get waved through. The guard now takes the best of five runs.

Best-of-N is the right statistic here, and that is measured rather than asserted. Running the semver.org prerelease pattern this module replaced against the same inputs, three runs each:

reps=20  len=68    17.6ms  17.4ms  17.4ms
reps=30  len=98   545.4ms 521.2ms 500.0ms
reps=39  len=125  492.3ms 493.5ms 491.3ms
reps=45  len=128  495.3ms 507.9ms 527.8ms

The blowup lands on every run with under 10% spread, because superlinear backtracking is a property of the pattern. The ~4ms of scheduler jitter that broke the single-sample form is not. If the exponential path returns, no run is fast, so the guard still fails loudly. Those numbers are in the test file so the next person to touch the budget can see what it separates.

Verification

• bun test tests/strict-semver.test.ts — 7 pass, 0 fail.
• bun run typecheck — exit 0.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• Tests
  • Improved performance testing for strict semantic-version parsing by measuring the fastest result across multiple runs.
  • Updated ReDoS-resistance checks to reduce the impact of timing variability and produce more reliable performance measurements.
  • Added clearer test documentation explaining the multi-run timing approach and test input generation.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: c8a413a1-a137-4ac1-8346-551e5368e92b

📥 Commits

Reviewing files that changed from the base of the PR and between 81a1fc1 and d860fe6.

📒 Files selected for processing (1)

• tests/strict-semver.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The strict semver ReDoS tests now measure the fastest result across five parses. The tests retain input-length and null-result checks while replacing single-run timing helpers with fastestParseMs and inputFor.

Changes

Strict semver timing tests

Layer / File(s)	Summary
Repeated timing measurement and ReDoS assertions tests/strict-semver.test.ts	The tests add fastestParseMs(input, runs = 5), document best-of-N timing, and use the helper for both ReDoS timing assertions. The second test builds inputs with inputFor(reps). Existing length and null-result assertions remain unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d860f

Semver ReDoS timing tests now use the fastest of five parse measurements to reduce one-time runtime noise while retaining the existing performance guard. The updated tests and typecheck pass, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: updating the semver ReDoS test to use best-of-N timing instead of a single sample.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/semver-redos-flake

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 62 / 80

이 PR은 제품 코드를 바꾸지 않습니다. 바꾸는 파일은 tests/strict-semver.test.ts 하나뿐이고, src/lib/strict-semver.ts의 parseStrictSemver는 그대로입니다. 지금 dev HEAD(81a1fc1cc, package 2.41.0)에 이미 들어가 있는 ReDoS 방어(프리릴리스를 한 번에 받고 identifier를 개별 검증)는 유지한 채, 그 방어가 아직 살아 있는지 재는 시계만 고칩니다.

지금 dev의 테스트는 공격 모양 입력을 한 번 파싱하고 그 한 번의 시간이 50ms 미만인지 봅니다. PR 본문이 말하는 대로, 로드된 macOS 러너에서 그 한 샘플이 53.77ms가 나와 v2.41.0 승격(#3342)이 깨졌습니다. 그런데 이 가드가 지키려는 회귀는 예전에 ~522ms였던 초선형 경로입니다. 4ms짜리 스케줄러 잡음과 522ms짜리 결함을 같은 문으로 막으면, 팀이 “그냥 다시 돌리면 된다”고 배우게 되고, 그게 진짜 ReDoS가 돌아왔을 때 넘어가는 길이 됩니다.

그래서 이 PR은 fastestParseMs를 두고 5번 중 가장 빠른 값을 50ms와 비교합니다. 설명과 측정표(구 semver.org 프리릴리스 패턴, reps 20→45에서 매번 수백 ms)가 테스트 파일에 같이 들어 있어서, “왜 best-of-N인가”가 다음 사람에게도 남습니다. 초선형 백트래킹은 패턴 속성이라 매 실행마다 느리고, 러너 날씨는 그렇지 않다는 주장과 맞습니다. muse_release_train / v2.41.0 손으로 밀어 올리는 경로에서 CI 날씨로 승격이 막히는 상황을 푸는 작은 chore이고, types.ts/config.ts 분할과도 무관합니다. 범위가 테스트 한 파일이라 머지해도 다른 열린 PR을 무효화하지 않습니다.

라인 40-48 - fastestParseMs 기본 runs = 5는 합리적이지만, 상수 이름이나 주석에 “왜 5인지”(측정표는 3회)가 없어서 나중에 누군가 2로 줄이거나 20으로 늘릴 때 기준이 약하다.
라인 52-61 - 첫 테스트는 expect(parseStrictSemver(attack)).toBeNull()로 한 번 돌린 뒤 fastestParseMs로 또 5번 잰다. null 검증과 타이밍 분리는 좋고, 앞의 한 번이 JIT 워밍업 역할도 하지만, 그게 의도인지 파일에 한 줄만 있으면 더 분명하다.
라인 5-12 / 13-39 - 블록 주석이 연달아 두 개다. 내용은 모두 맞지만, 위쪽 “예전 패턴이 왜 위험했는지”와 아래쪽 “왜 best-of-N인지”를 한 주석으로 이어 쓰면 읽는 비용이 줄어든다.
라인 68-69 - 두 번째 테스트도 이제 best-of-N이라, 예전에 있던 measure(20) 워밍업 한 번은 없어도 된다. 동작은 맞다. 다만 두 입력을 각각 5회씩이라 CI 벽시계는 조금 늘어난다. ReDoS 가드 범위에서는 괜찮은 거래다.
src/lib/strict-semver.ts - 제품 파서 변경 없음. 가드가 가리키는 회귀면은 여전히 프리릴리스 초선형 경로이고, 길이 상한(128)만으로는 못 막는다는 기존 주석과 이 PR의 측정표가 같은 이야기를 한다.

메인테이너의 판단이 필요한 지점

• 이 PR을 먼저 dev에 올린 뒤 [WRONG BRANCH] release: v2.41.0 #3342(WRONG BRANCH release: v2.41.0) 승격을 다시 돌릴지, 아니면 릴리스 PR 쪽을 먼저 정리할지
• best-of-N으로 잡음이 줄었으니 50ms 예산을 더 타이트하게(예: 20~30ms) 낮출지, 아니면 여유를 남길지
• N=5를 테스트 상수로 고정할지, 측정표(3회)와 맞출지

너의 추천
CI(특히 macos / test shards)가 초록이면 머지한다. 제품 표면이 아니라 릴리스 가드 신뢰도 수정이고, 통계 선택(최소값)도 ReDoS에 맞다. 머지 후 #3342 쪽 v2.41.0 승격을 다시 시도하되, 이번엔 같은 53.77ms 단발 실패에 흔들리지 않게 이 가드가 받쳐 주는지 확인하면 된다. 예산 하향이나 N 상수 정리는 후속 nit로 남겨도 충분하다.

이 댓글은 grok-bot이 작성했습니다