refactor(billing): make SpendLog direct-only and fail closed - #92
Draft
kitbuilder587 wants to merge 16 commits into
Draft
refactor(billing): make SpendLog direct-only and fail closed#92kitbuilder587 wants to merge 16 commits into
kitbuilder587 wants to merge 16 commits into
Conversation
A successful, token-consuming request for a model that has no resolvable
price was persisted with spend=0 (only a Warn log and price_status in
metadata). In the `spend` column that is indistinguishable from a
legitimately free or cache-hit row, so a paid model missing a price
silently under-billed to zero and vanished into the aggregates.
Add auto_ai_router_shadow_spend_price_missing_total{price_status}, incremented
for every successful row with usage>0 whose price could not be resolved
(missing_registry / missing_model). Priced and cache-hit rows keep
price_status="found" and never touch the counter, so the stand can assert the
paid-model-without-price condition directly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…check An ordinary key naming an internal backend deployment ID (e.g. the bare "claude-sonnet-4.5" behind the public "anthropic/claude-sonnet-4.5") got a 403 "Model not allowed" whenever its model ACL was non-empty, because the token ACL check ran before the product-surface check. That leaks the existence of internal IDs and diverges from the stack MiXaiLL76#83..MiXaiLL76#85 fix (a) intent ("404 for internal backend IDs for ordinary keys"), which previously only held for empty-ACL keys. Resolve configured-but-not-client-routable backend IDs to 404 "not found" before the ACL check for every non-master key. A truly unknown (non-configured) model still falls through to the ACL and returns 403; the trusted LiteLLM->AIR master-key hop still routes internal IDs exactly. This is what the stand's client-model-surface runtime gate asserts (backend-only IDs -> 404). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
initializeLiteLLMDB called os.Exit(1) inline, so the is_required=true fail-closed contract (ru01 runs is_required=true) could not be tested, and the optional-degrade path fell back to a NoopManager silently — virtual-key auth stays fail-closed there, but budget checks and spend logging become silent no-ops with no signal. Extract the policy into resolveLiteLLMDBManager (no os.Exit): required+unreachable returns an error so startup aborts; optional+unreachable degrades to NoopManager and raises the new auto_ai_router_litellm_db_degraded gauge to 1. main() keeps the os.Exit shell. Tests cover required-fails-closed, optional-degrades-loudly, and disabled-uses-noop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n-audit-candidate # Conflicts: # internal/config/config.go # internal/config/config_budget_test.go # internal/litellmdb/budget/reservation.go # internal/litellmdb/budget/reservation_test.go # internal/litellmdb/models/models.go # internal/litellmdb/models/models_test.go # internal/litellmdb/spendlog/logger.go # internal/litellmdb/spendlog/spend_updater.go
…n-audit-candidate # Conflicts: # internal/litellmdb/auth/auth.go # internal/models/manager.go
…n-audit-candidate # Conflicts: # cmd/server/main.go # internal/proxy/budget_enforcement.go # internal/proxy/proxy.go # internal/proxy/proxy_log.go # internal/proxy/proxy_websocket.go
…n-audit-candidate
Two LiteLLM auth behaviors AIR did not reproduce, both blocking cutover: - LiteLLM_DeprecatedVerificationToken: a rotated key kept working on the LiteLLM proxy until revoke_at but got 401 from AIR. ValidateToken now mirrors _lookup_deprecated_key: on a main-table miss it resolves active_token_id (revoke_at > now, single non-chaining hop) and loads the active token's hierarchy, so spend attribution follows the active key. - LiteLLM_AccessGroupTable: keys/teams granted models through unified access groups were denied by AIR's native allowlist intersection. The hierarchy query now carries key/team access_group_ids; the resolved groups extend the key scope unconditionally (can_key_call_model fallback) and the team scope via the team's own groups plus owner-authorized key groups (_key_access_group_grants_model: assigned_team_ids/assigned_key_ids). Empty native lists stay unrestricted; group-resolution failures degrade to no expansion. Red-green: unit tests for scope expansion and group resolution, plus integration tests against PostgreSQL 16 (added to the CI integration job) that fail on the previous code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kitbuilder587
changed the title
…ecycle-fixes Rebases the lifecycle fixes onto the renamed spend layer: this branch's own additions follow the new naming — SpendPriceMissingTotal / auto_ai_router_spend_price_missing_total, TestBuildSpendEntry* — and LiteLLMDBDegraded is kept alongside the renamed sink metrics. No shadowspend naming remains; the shadowcontext family stays untouched. Validated: go build/vet, full unit suite, spendsink+proxy integration (race) against PostgreSQL 16. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ration-blockers Actualizes the assembled candidate to the renamed stack (MiXaiLL76#83 -> MiXaiLL76#84 -> MiXaiLL76#85 -> MiXaiLL76#90): spendsink/spendcompare naming end-to-end, config-first master key (FetchMasterKey never lets the LiteLLM_Config copy override server.master_key), and the remaining local leftovers cleaned up: AIR_SPEND_*_DSN, SPEND_DATABASE_URL/SPEND_PUBLIC_KEY test env names, docs unified under docs/spend-log, last shadow-named test and help strings. The shadowcontext (signed tenant context) family and the spend_logs_metadata JSON contract remain unchanged. Validated: go build/vet, full unit suite, and the integration set (spendsink, proxy, litellmdb/auth, litellmdb/budget) with -race against PostgreSQL 16. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
|
@kitbuilder587 как будто уже не актуально? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spend_log.modeis now rejected instead of selecting disabled, shadow, or direct behaviorWhy
Shadow spend mode is not part of the desired product. Keeping separate shadow and direct paths made startup, readiness, streaming finalization, and migration behavior depend on mode-specific branches and allowed the migration target chain to reject valid signed deployment identities.
Root cause
The writer contract mixed comparison-only behavior with authoritative billing. In addition, the pre-provider deployment check always queried the local AIR model snapshot even though SpendLog construction already treated a verified signed LiteLLM identity as authoritative. Backend-only target-chain models therefore returned
503 Deployment identity unavailablebefore reaching the provider.Impact
There is one writer policy:
Verified LiteLLM target-chain calls keep their signed deployment attribution without requiring the same public model to exist in the inner AIR snapshot. No production infrastructure or production database changes are included.
Main commits
7a36db93109a49408aacbb8702b9e32cb1fd6730— remove shadow spend mode and rename the writer surfacee53c677489f1c3d64d339059db89c7fef89c8640— trust verified signed deployment identity while preserving fail-closed boundariesNo services-repository changes or services PR are included.
AIR validation
go test ./... -count=1go build ./...go vet ./...go test -race ./... -count=1SPEND_TEST_DATABASE_URL=... go test -race -tags=integration ./internal/spendsink ./internal/proxy -count=1rg -i shadow[ _-]?spend|shadow mode|SpendLogMode|spendLogMode .returns no matchesMigration validation
The exact candidate was packaged from
e53c677489f1c3d64d339059db89c7fef89c8640:62bf26bdfece3db247bc918c9942cf20916a62e13aff955b1663a6a726a516215c8da63d3d9fe730ee17d710efedbe07fcd79df971e99efabf9a06f13fb4cdREADY/GO, zero confirmed AIR blockersThe production switch remains operationally
NO-GOonly because the signed exact-three ingress allowlist attestation is absent. That external production attestation was intentionally not fabricated or changed by this PR.