Fix: remove duplicate dev-read rate-limit wrappers to unbreak main CI

[ZachL111]

What

Removes the duplicate Developer API read rate-limit wrappers in developer.py and deletes the superseded test_dev_read_rate_limits.py, so the backend unit suite is green on main again.

Why (a merge race broke main)

The 2026-07-03 Developer API auth refactor moved read rate-limiting into the dependency chain (dependencies.py get_auth_with_*_read calls check_api_key_rate_limit) and added a guard test, TestRouterWiring.test_developer_read_routes_do_not_add_duplicate_uid_wrappers, that forbids the old with_rate_limit(get_uid_with_X_read, "dev:X_read") route wrappers.

#8713's PR (#8923) predated that refactor. It added exactly those wrappers plus its own test requiring them, and merged because its selective CI never ran the new guard test. Main now has two contradictory tests, so developer.py cannot satisfy both and any PR touching a full-suite file goes red.

Details

• Reverts the 5 wrappers on the developer read routes (action-items, conversations/folders, and three goals routes) back to the bare Depends(get_uid_with_X_read) deps.
• Rate-limiting is preserved: those deps already call check_api_key_rate_limit via get_auth_with_X_read, so the wrappers were true duplicates. The intent of Harden Developer API read endpoints against abuse #8713 is still delivered by the dependency chain.
• Deletes test_dev_read_rate_limits.py, which is fully superseded (it required the wrappers and forbade the bare deps, both now inverted).
• Keeps the with_rate_limit import (still used for conversations:from-segments).

Test

TestRouterWiring (all 22 tests) passes locally, including the previously-failing test_developer_read_routes_do_not_add_duplicate_uid_wrappers and the *_has_rate_limits assertions confirming the read routes are still rate-limited.

[cubic-dev-ai]

No issues found across 1 file

Confidence score: 5/5

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

_{Re-trigger cubic}

[ZachL111]

Flagging since this affects main CI: the full backend unit suite is currently red on main because #8923's dev-read rate-limit wrappers and the 2026-07-03 auth-refactor's duplicate-wrapper guard test contradict each other (developer.py cannot satisfy both). This PR resolves it by dropping the now-duplicate wrappers (the dependency chain already rate-limits via check_api_key_rate_limit) and removing the superseded test, and it is green. Any PR touching a full-suite-trigger file will keep failing until this lands. Happy to adjust if a different resolution is preferred.

[Git-on-my-level]

Omi PR #9065 — Review

PR: Fix: remove duplicate dev-read rate-limit wrappers to unbreak main CI
Author: @ZachL111 (CONTRIBUTOR) · Tier: suspicious · Head: 15e7c6b
Reviewer: glm-5.2 (automated maintainer monitor) · Verdict: superseded / do-not-approve (comment-only + human review)

---

Summary

This PR removes 5 with_rate_limit(get_uid_with_*_read, "dev:*_read") wrappers from
backend/routers/developer.py and deletes backend/tests/unit/test_dev_read_rate_limits.py,
on the premise that the dev read dependency chain already enforces per-key rate limits, so the
route-level wrappers are true duplicates — and that main CI is currently red because of two
contradictory tests.

The rate-limiting claim is technically sound: each get_uid_with_{conversations,action_items,goals}_read
dependency in backend/dependencies.py calls get_auth_with_*_read, which calls
check_api_key_rate_limit(..., policy_name="dev:*_read") with fail_closed=True on the
dev:{uid}:{app_id}:{key_id} key. Removing the route wrapper does not weaken the API-key
read rate limit. (Minor nuance: the wrapper enforced a second limit on a different key — the
bare UID, fail-open — so it was a dual-enforcement rather than a byte-for-byte duplicate; dropping
it only relaxes that secondary UID-based check, not the security-critical per-key one.)

However, the PR is built on a premise that is now factually wrong, and the PR is effectively superseded.

Why this should not be approved/merged as-is

1. Main CI is already green. I verified the Backend Unit Tests workflow on main:
   the run at 81d28b0188 (2026-07-05 20:54 UTC) is success. The 4 red runs that actually
   existed were earlier (~18:24–18:27 UTC). The "unbreak main CI" motivation no longer applies.

2. Main already resolved the contradictory-test conflict — in the opposite direction. At the
   broken parent (80e9d76202), developer.py carried the wrappers AND
   test_rate_limiting.py::TestRouterWiring::test_developer_read_routes_do_not_add_duplicate_uid_wrappers
   forbade them AND test_dev_read_rate_limits.py required them. The fix commit 81d28b0188
   removed the anti-wrapper guard test from test_rate_limiting.py and kept both the wrappers
   and test_dev_read_rate_limits.py. I confirmed current main (HEAD 904658765a) contains
   the 5 wrappers in developer.py, still has test_dev_read_rate_limits.py, and has no
   test_developer_read_routes_do_not_add_duplicate_uid_wrappers. So this PR's central factual
   assertion — that main "now has two contradictory tests" — is no longer true.

3. This PR and the main-side fix are conflicting resolutions. Main chose: keep wrappers,
   keep the pro-wrapper test, drop the anti-wrapper guard. This PR chooses: drop wrappers,
   drop the pro-wrapper test. Merging this PR would simply re-open the design question and would
   require touching test_rate_limiting.py again (where the removed guard used to live). That is a
   maintainer-direction decision, not an unambiguous fix.

Verification performed

• Confirmed backend/dependencies.py on main enforces dev:conversations_read,
  dev:action_items_read, dev:goals_read via check_api_key_rate_limit (fail-closed, per-key).
• Confirmed main HEAD developer.py retains the 5 wrappers; PR head removes exactly those 5 and
  leaves other with_rate_limit uses intact.
• Confirmed TestRouterWiring on main HEAD has no ...do_not_add_duplicate_uid_wrappers test
  (it exists only at the broken parent).
• Checked main CI history: Backend Unit Tests success at 81d28b0188 (20:54 UTC); Lint success.

Risk assessment

• Security: No weakening of API-key rate limits. Not an attack/supply-chain attempt.
• Correctness of code change in isolation: The 5 wrapper removals are safe if the dependency
  chain is the intended single enforcement point. That is a reasonable design, but it conflicts with
  the direction main has already taken.
• Process: The PR description's narrative is outdated/inaccurate relative to current main, which
  makes review harder and risks a maintainer acting on stale information.

Recommendation

Do not approve or merge as-is. This needs a human maintainer to decide which resolution is
canonical (route-level wrappers + dual enforcement, as main now has; or dependency-chain-only, as
this PR proposes) and then reconcile the tests consistently. I've labeled it for maintainer review
and left a comment explaining the supersession. Credit to @ZachL111 for correctly identifying the
real contradiction and the safe removal direction — the diagnosis was right at the time the PR was
opened; main just moved first.

No urgent alert. No agent-behavior documentation changes.

[kodjima33]

Correct: read rate-limiting now lives in the get_auth_with_*_read dependency chain (verified check_api_key_rate_limit on main), so these wrappers were double-limiting; removal + obsolete-test deletion unbreaks main CI. conf 5/5.