Skip to content

fix(spend): enforce ceilings on native chat sends - #5358

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/native-chat-spend-ceiling
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/native-chat-spend-ceiling

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • The durable token ceilings introduced for spend accounting were only wired into the Responses execution budget, so native Chat Completions could reach upstream providers without creating or consulting a durable reservation, bypassing configured root/identity/pool ceilings.
  • Attach the durable spend tracker to the native Chat Completions path and charge it before each physical upstream send, refusing the dispatch when the ledger denies a reservation (src/server/chat-native.ts); populate native request spend metadata (input token estimate and output ceiling) when selecting the native route (src/server/chat-completions.ts).
  • Surface a local refusal that names the exhausted scope when a native dispatch is denied, refresh API-key attribution when credentials rotate, and note in structure/transports/responses.md that native Chat participates in the same ledger enforcement.

Verification

  • bun test tests/responses/chat-completions-endpoint.test.ts — 123 tests pass, including a new regression proving a configured pool.maxTokens ceiling yields a local 429 with x-opencodex-local-refusal: workflow_spend_exhausted and no upstream contact.
  • bun run typecheck — clean.

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.

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.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

@github-actions github-actions Bot added the bug Something isn't working label Sep 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Sep 20, 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.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 18 / 80

이 PR은 네이티브 Chat Completions 빠른 경로가 내구성 spend 한도를 건너뛰던 구멍을 막습니다. 예전에는 Responses 실행 예산에만 spend tracker가 붙어서, native chat로 바로 나가는 요청은 root/identity/pool 토큰 천장에 예약·조회 없이 upstream으로 갈 수 있었습니다. 이제는 native 경로를 고를 때 입력 토큰 추정과 출력 ceiling을 logCtx에 넣고, handleNativeChatCompletions에서 tracker를 붙인 뒤 실제 upstream send 직전마다 charge()로 예약합니다. 거절되면 upstream에 닿지 않고 로컬 429(workflow_spend_exhausted)를 돌립니다. 키 회전 때 account label도 다시 찍어서 identity 범위가 맞게 잡히게 했고, 문서와 회귀 테스트도 같이 왔습니다. base는 dev이고, draft 상태입니다.

라인 - structure/transports/responses.md (Durable spend reservations): 바로 위에서는 native가 dispatch마다 charge한다고 했는데, 다음 문단은 여전히 “budget send counter만 보고 book하고 dispatch 사이트에서는 부르지 않는다”고 적혀 있어 서로 어긋납니다. native 예외를 한 문장으로 밝혀 두는 편이 낫습니다.
라인 - src/server/chat-completions.ts (native 분기의 usageLogInputTokens): messages만 추정합니다. 같은 파일의 cursor/kiro 경로는 tools도 넣습니다. tools가 큰 native 요청이면 예약 입력이 실제보다 작게 잡혀 ceiling이 느슨해질 수 있습니다.
라인 - tests/responses/chat-completions-endpoint.test.ts: pool.maxTokens: 1 → 로컬 429·upstream 0회는 잘 잡습니다. root/identity 천장·출력 ceiling metadata·키 회전 후 label 갱신은 이 테스트만으로는 안 보입니다(필수까지는 아님).

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

native 경로의 입력 추정을 messages-only로 둘지, tools까지 넣을지. 문서의 “observer vs 직접 charge” 설명을 native 예외로 고칠지, 아니면 Responses만의 이야기라고 범위를 좁힐지.

너의 추천

구멍 자체는 맞고 회귀도 있으니, 문서 한 문단만 맞춘 뒤 readiness 체크리스트 채우고 draft를 풀면 됩니다. tools 추정은 이번 PR에서 같이 맞추거나 follow-up으로 명시해도 됩니다. types.ts/config.ts 분할이나 preview deploy 이야기는 이 PR과 무관합니다.

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

@luvs01
luvs01 force-pushed the fix/native-chat-spend-ceiling branch from 8ebbb86 to 506e9d4 Compare September 20, 2026 17:12
@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5556 as a single related-function aggregate.

Source head: 347204ff33e42bba7781e7b057443ea9808e9d06. Replacement head: d3589638a877530f89c111d3dba69d7a76908939.

The complete source matches carried 823a7d2 by stable patch ID, with author/date/provenance preserved. The integration also fixes the reviewed omission of tool definitions from the input estimate and proves refusal before any upstream send. Both native spend cases passed after explicit fixture shutdown and ACL-child reaping; source assertions and limits are retained. The final latest-dev merge leaves all 18 tested contribution files unchanged, with separate successful release-file/structure checks. Full final suite, hosted CI and security review remain pending on the draft replacement.

Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained.

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.

2 participants