fix(backend): keep Developer API conversation reads under one shared ceiling

[RatnamOjha]

Follow-up to the Developer API read hardening from #8713 (landed in af3ab1e).

What this fixes

Splitting conversation reads into per-route policies gave each route its own Redis bucket:

	Before the split	On main today
GET /v1/dev/user/conversations	dev:conversations_read 60/hr	dev:conversations_read 60/hr
GET /v1/dev/user/conversations/{id}	same 60/hr bucket	dev:conversation_detail_read separate 60/hr
Aggregate per key	60/hr	120/hr

Each policy is an independent counter (rl:{policy}:{prefix}:{uid}:{app_id}:{key_id}), so adding a policy adds budget rather than subdividing it.

Honest scoping — this is small

I don't want to oversell it. The practical exposure delta is minor:

• Detail reads return one conversation. The list endpoint already allows up to 100 per request (backend/routers/developer.py:1241), so 60 list req/hr ≈ 6,000 records/hr vs. the detail path's 60 records/hr — roughly 1% on top.
• The vector from the original incident (GET /v1/dev/user/conversations?limit=20 at ~1,300 req/hr) is the list endpoint, and it is unchanged at 60/hr. That cap still does its job.

So this is not a live vulnerability, and "120 vs 60" counts requests, which overstates it.

The reason to fix it is structural: nothing bounds the policy set. Every future dev:conversation_*_read policy silently raises the aggregate again, and no test or comment flags that. This caps it once, in a way that keeps working as policies are added.

Approach

Add dev:conversation_reads_total (60/hr). Every conversation read charges it before its per-route budget:

• Aggregate returns to the pre-split 60/hr — no client that worked before is affected.
• List / detail / transcript stay independently tunable underneath the ceiling. This also answers the review question on fix: harden Developer API conversation read limits #8743 about why detail was given the same value as list: the per-route numbers are headroom, the ceiling is the real limit.
• Transcript reads still charge their stricter 25/hr bucket on top.

Tradeoff: one extra Redis round trip on the two conversation read routes. The transcript sub-budget already charges two buckets on a single request, so the pattern isn't new — but it is a real cost on a read path, and worth a maintainer's call.

Tests

test_conversation_reads_share_an_aggregate_ceiling drives both routes in alternation, so neither per-route bucket can be what stops the caller, then asserts the aggregate is. It fails on the current wiring with:

AssertionError: 120 != 60

Also added dev:conversation_reads_total to the existing policy-wiring assertions, and updated the ordered policy list in test_dependency_async_boundaries.py (the shared ceiling is charged first, and still routes through critical_executor).

Verified per-file the way backend/test.sh runs in CI: test_rate_limiting.py, test_dependency_async_boundaries.py, test_dev_api_conversations_poison.py, test_dev_api_folder_filters.py, test_dev_api_lock_bypass.py all pass. black --line-length 120 --skip-string-normalization clean.

---

No existing class in .github/failure-classes/ describes this mode — subdividing a shared quota into per-route policies adds budget rather than partitioning it. Declaring none rather than minting a class, since reviewers asked for a declaration and not a registry change; happy to switch to new and add the definition if maintainers would rather have the class tracked.

Failure-Class: none

[cubic-dev-ai]

No issues found across 4 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

_{Re-trigger cubic}

[kodjima33]

Reasoning is sound — the per-route split gave each Developer API read its own Redis bucket, doubling the aggregate ceiling. Holding the merge until CI is green: PR Metadata Preflight needs a 'Failure-Class:' trailer on the fix: commit, and the Backend unit suite is failing on tests/unit/test_agent_vm_startup.py.

[Git-on-my-level]

Thanks @RatnamOjha — verified this end to end on the current head (791d066), alongside the approval already on record.

Code verification, file by file:

• backend/dependencies.py — the new _check_conversation_read_budgets_async charges dev:conversation_reads_total before the per-route policy, and both get_auth_with_conversations_read and get_auth_with_conversation_detail_read now route through it. Ordering is right: the shared bucket is checked first, so neither route can bypass the aggregate cap. Transcript reads keep stacking their stricter dev:conversation_transcript_read bucket on top (developer.py:1236/1410), which matches the config comment.
• backend/utils/rate_limit_config.py — dev:conversation_reads_total: (60, 3600) restores the pre-split aggregate while list/detail stay independently tunable underneath. The two-tier comment is accurate against the code.
• backend/tests/unit/test_rate_limiting.py — test_conversation_reads_share_an_aggregate_ceiling is the important piece: it alternates list/detail calls through a counting limiter and asserts the run stops at exactly the 60/hr ceiling (not 120), and that neither per-route counter is what rejected the caller. If the ceiling is ever dropped, this fails. It passed in CI along with the updated policy-allowlist tests.
• backend/tests/unit/test_dependency_async_boundaries.py — the expected executor policy sequence correctly inserts the reads_total charge before each per-route charge.

One observation, not a blocker: each list/detail read now makes two sequential limiter round-trips (ceiling + route) instead of one. That is consistent with how stacked policies already work here (transcript adds a third), so the trade looks right for the invariant it buys.

Remaining CI items, matching what the approval already noted:

1. PR Metadata Preflight fails on the failure-class-protocol rule — the fix: commit subject needs a Failure-Class: FC-<slug> | new | none declaration. That is a commit-message amend, not a code change.
2. Backend unit suite failure is tests/unit/test_agent_vm_startup.py::test_startup_parses_device_wait_timeout_as_decimal (exit 141 / SIGPIPE in the shell harness) — unrelated to this PR's four files, and this PR's own rate-limit and dependency-boundary tests all pass. main's backend unit suite is currently red on a different test (test_process_conversation_usage_context.py), so a rerun/rebase may clear this one.

The aggregate-ceiling invariant is exactly the structural fix described, and it is now pinned by a test. Once the commit trailer lands and the unrelated flake clears, this should be ready to merge under the approval already on this head.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[RatnamOjha]

Thanks both — addressed the two CI items. No code changes to the fix itself.

1. PR Metadata Preflight / failure-class-protocol

Added a Failure-Class declaration to the PR body. One correction to the diagnosis: the validator reads the declaration from the PR body, not the commit message — scripts/failure-class parses it via declarations_in_body(read_pr_body(args.pr_body_file)), and .github/checks-manifest.yaml invokes it with --pr-body-file. So this was a body edit rather than a commit amend.

I went through the registry before defaulting. No existing class covers this mode — subdividing a shared quota into per-route policies adds budget rather than partitioning it. The nearest candidates don't fit:

• FC-bounded-read-exceeds-request-budget — per-request document work against a middleware latency budget, not quota accounting
• FC-shared-backoff-conflated-independent-budgets — client-side cooldown keying, scoped to desktop/macos/**

Declared none rather than new, since new requires adding a definition JSON and the ask was for a declaration rather than a registry change. If you'd rather have the class minted I'll add the definition — the guard artifact would be the test this PR already ships.

Verified locally against the same commands the manifest runs:

failure-class validate  -> ok: true, errors: []
check_product_invariants.py -> OK: no locked invariants require naming
check_failure_class_guard_ratchet.py -> OK: every class at/above threshold names a guard artifact

2. Backend unit suite

Confirmed unrelated. tests/unit/test_agent_vm_startup.py fails identically on clean upstream/main at my previous base — 21 failed, 4 passed, byte-identical to the run on my branch — and this PR touches four files, none of which test_agent_vm_startup.py imports.

Rebased onto current main (was 92 commits behind) for a fresh run. Re-verified post-rebase, per-file the way backend/test.sh executes in CI:

test_rate_limiting.py                 73 passed, 1 skipped
test_dependency_async_boundaries.py    8 passed
test_dev_api_conversations_poison.py   3 passed
test_dev_api_folder_filters.py        19 passed
black --line-length 120 --skip-string-normalization  4 files unchanged

On the two-round-trip observation — agreed it's the right trade for the invariant, and it stays proportional: the ceiling is charged once per read, so a transcript read is 3 buckets rather than 2, not N.