Skip to content

fix(usage): reject persisted rows without provider labels - #2413

Merged
Ingwannu merged 1 commit into
lidge-jun:devfrom
luvs01:fix/usage-log-provider-guard
Aug 23, 2026
Merged

fix(usage): reject persisted rows without provider labels#2413
Ingwannu merged 1 commit into
lidge-jun:devfrom
luvs01:fix/usage-log-provider-guard

Conversation

@luvs01

@luvs01 luvs01 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject malformed persisted usage rows unless both requestId and provider are strings, at both the synchronous and cooperative reader boundaries.
  • Preserve the existing permissive normalization of optional legacy fields while skipping only rows whose identity cannot be used safely.
  • Prevent one provider-less JSONL row from throwing during provider-label grouping and collapsing /api/usage into a read_failed empty summary.

Verification

  • Bun 1.4 focused regression: bun test --isolate tests/usage-log.test.ts tests/usage-summary.test.ts tests/usage-provider-label.test.ts — 83 passed, 0 failed, 353 assertions.
  • Regression coverage includes null, scalar, and array roots; missing or non-string request identities; malformed provider values; both reader paths; and the 500,001-row retention-cap fixture.
  • bun run typecheck — passed.
  • bun run privacy:scan — passed.
  • git diff --check — passed.
  • Independent exact-diff review — no actionable findings.
  • Exact head 35f0b88cd9486df788318e1f54a6ff72e7a2be6c, based directly on dev at 4f41a8e936141af7ee828e335da314b9dc1ef761.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. This hardens an internal persisted-log read boundary and adds no public API or configuration surface.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This changes no credential, authorization, or external-input trust boundary.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved usage log processing to ignore malformed entries and rows missing required request or provider information.
    • Ensured invalid or incomplete log lines no longer interrupt processing of valid entries.
  • Tests
    • Expanded coverage for invalid usage records across standard and management log readers.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added bug Something isn't working review-ready labels Aug 22, 2026
@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e7d15528-65b8-4bce-80de-0c4afa994c91

📥 Commits

Reviewing files that changed from the base of the PR and between 4f41a8e and 35f0b88.

📒 Files selected for processing (2)
  • src/usage/log.ts
  • tests/usage-log.test.ts

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


📝 Walkthrough

Walkthrough

Usage-log parsing now validates persisted rows before normalization. Rows must contain string requestId and provider values. Malformed, non-object, and partial rows are skipped by both standard and management readers.

Changes

Usage row validation

Layer / File(s) Summary
Validated usage parsing
src/usage/log.ts:1145-1176, tests/usage-log.test.ts:229, tests/usage-log.test.ts:742-761
A shared validator requires object rows with string requestId and provider values. Both full-log and incremental parsing use the validator. Tests verify that malformed and invalid-identity rows are excluded while valid rows remain. The truncation fixture now includes provider.

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

Merge Risk: ⚪ Minimal · up to 35f0b

This focused change rejects malformed persisted usage rows while preserving valid legacy normalization, with no actionable merge-blocking risk remaining after normal checks and review.

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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: rejecting persisted usage rows without provider labels.
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 unit tests (beta)
  • Create PR with unit tests

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

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

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

설명: 이 PR 은 저장된 사용량 줄에서 요청 아이디와 제공자가 둘 다 글자가 아니면 그 줄만 건너뛴다. 지금 CURRENT dev HEAD 는 4f41a8e93 이다. 이번 시간에 origin/dev 는 그대로다. 새 머지는 없다. 착지는 여전히 2396 사용량 CLI 오늘 비용이다. package.json 은 2.27.0 이다. src/config.ts 는 3975줄이다. src/runtime 폴더는 지금 HEAD 에 없다. 지금 HEAD 의 src/usage/log.ts 1153줄 readUsageEntries 와 1169줄 parseUsageLines 는 요청 아이디가 글자면 통과한다. 제공자는 안 본다. 배열은 요청 아이디가 없어서 이미 건너뛴다. null 은 거짓이라 이미 건너뛴다. 구멍은 요청 아이디만 있고 제공자가 없거나 글자가 아닌 줄이다. 429줄 normalizeUsageEntry 는 제공자를 그대로 복사한다. src/usage/summary.ts 563줄과 669줄은 baseProviderLabel 에 그 값을 넣는다. src/providers/label.ts 7줄 baseProviderLabel 은 글자의 lastIndexOf 를 부른다. 글자가 아니면 터진다. src/server/management/logs-usage-routes.ts 304줄 잡으면 /api/usage 가 read_failed 빈 요약이 된다. 이번 시간 착지 2396 사용량 CLI 도 같은 요약을 쓴다. 이 PR 은 두 읽기 길에 같은 가드를 넣고, 선택 옛 필드는 그대로 느슨하다. 테스트는 null, 숫자, 배열, 빈 객체, 제공자 없음/null/숫자/객체/배열을 두 읽기 길에서 잠근다. 50만 줄 한도 픽스처에도 제공자를 넣었다. 작성자 로컬은 usage-log 와 usage-summary 와 usage-provider-label 83개, typecheck, privacy:scan 통과라고 적었다. 드래프트가 아니다. bug 와 review-ready 라벨이 있다. 체크리스트 네 칸이 채워져 있다. 작성자는 luvs01 이다. 베이스는 지금 HEAD 와 같다. Closes 가 없다. GitHub 위생/타깃/CodeRabbit 만 초록이다. Cross-platform CI 는 아직 안 보인다. 사용자 길이로는 제공자 없는 한 줄이 사용량 화면을 비우는 구멍이라서 52. 카탈로그 팁은 Ox Alpha x-preview-f-free + deepseek-v4-flash-vision-exp. Cursor 정적 카탈로그는 opus-4-8-fast / opus-5-fast. 2334 CursorCredentialRouter 는 여전히 src/providers/cursor-pool.ts 모듈+테스트만 있고 어댑터에 연결되지 않았다. 2332 H2 는 discovery 전용. 2320 overflow + 2342 는 이미 dev. 2188 사이드카는 이미 dev. 2382 데스크톱 앱 재시작은 이미 dev. 2292 는 아직 연다.

src/usage/log.ts 라인 1153 - 지금 HEAD 동기 읽기는 요청 아이디만 본다. 제공자가 없어도 normalizeUsageEntry 로 간다
src/usage/log.ts 라인 1169 - 관리 읽기 parseUsageLines 도 같다. PR 은 두 길에 같은 가드를 넣는다
src/providers/label.ts 라인 7 - baseProviderLabel 은 글자의 lastIndexOf 를 부른다. 글자가 아니면 터진다
src/server/management/logs-usage-routes.ts 라인 304 - 그 오류를 잡으면 /api/usage 가 read_failed 빈 요약이다
tests/usage-log.test.ts - null, 숫자, 배열, 제공자 없는 줄이 두 읽기 길에서 a 와 b 만 남는지 잠근다
GitHub CI - Cross-platform CI 가 아직 안 보인다. 작성자 로컬은 83개 통과

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

  • GitHub 시험이 아직 안 뜬 채로 머지할지. 작성자 로컬은 이미 통과라고 적었다
  • 세 번째 읽기 길 readRecentUsageEntries 도 같은 가드가 필요한지. 이 PR 은 두 길만 고친다
  • 이 구멍을 이슈로 남길지. 본문에 Closes 가 없다. 1820 캐시 지표와는 다른 읽기 가드다

너의 추천
GitHub 시험이 초록이면 머지한다. 가드를 더 넓히지 말 것. 선택 옛 필드는 그대로 둔다. 1820 은 닫지 않는다. types.ts/config.ts 스플릿과 겹치지 않는다. 라벨은 그대로 둔다. 프리뷰 배포가 아니다.

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

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved on exact head 35f0b88cd9486df788318e1f54a6ff72e7a2be6c.

The shared persisted-row parser now requires string requestId and provider identities before normalization, while keeping optional legacy fields permissive. All three readers, including bounded tail hydration, flow through the same guard. Focused usage coverage passes 83/83, typecheck and diff check pass, and exact-head cross-platform CI is green.

@Ingwannu
Ingwannu merged commit d8b620e into lidge-jun:dev Aug 23, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants