fix(server): detect bare uvicorn --workers N for per-worker rate-limit warning (#680)

[frankbria]

Summary

Closes #680.

The per-worker in-memory rate-limit WARNING (#678) only fired when the worker count came from WEB_CONCURRENCY / UVICORN_WORKERS. A bare uvicorn ... --workers N on the command line (no env var) went undetected, so a genuinely multi-worker, in-memory deployment stayed silent even though auth brute-force protection was weakened.

What changed

codeframe/ui/server.py:

• New best-effort, dependency-free signal: walk this process's ancestors via /proc and read a --workers N / --workers=N / -w N / -w=N flag off a recognized uvicorn/gunicorn command line.
• _detect_worker_count() now returns max(env-based, process-tree).
• Detection is gated to ASGI-server cmdlines (_is_asgi_server_cmdline) so an unrelated wrapper/test-runner's --workers flag can't false-positive (raised by cross-family codex review).
• Degrades silently to the env-var result on non-Linux or any error — never raises during startup.

Worker subprocesses are spawned, so their own sys.argv doesn't carry --workers; the flag lives on the supervisor (parent) process, which is why the ancestor walk is the reliable signal. psutil was deliberately not added — it isn't a current dependency and this is an advisory-only warning.

Acceptance criteria — all demonstrated

• Bare uvicorn ... --workers 2 (no env vars) triggers the WARNING — verified with a live server run (logged "in-memory storage with 2 workers").
• Single-worker and Redis-backed deployments stay silent — live single-worker run silent; existing tests/ui/test_rate_limit_worker_warning.py matrix still passes.
• Detection failure degrades gracefully to the env-var result with no startup error — unit-tested + live boot.

Tests

tests/ui/test_rate_limit_worker_warning.py — 42 passing (11 pre-existing + 31 new): argv parser matrix, ASGI-server recognition, /proc ancestor walk via a faked proc tree, env+tree integration, and graceful-degradation paths. uv run ruff check . clean; full tests/ui/ suite green.

Known limitations

• The /proc ancestor walk is Linux-only (the deployment target). On macOS/Windows, detection falls back to the env-var result — bare-CLI --workers won't be detected there, but the conventional WEB_CONCURRENCY path (used by production process managers) is unaffected.

[coderabbitai]

Warning

Review limit reached

@frankbria, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 39 minutes. Learn how PR review limits work.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb460e5a-efcf-4dd0-90aa-70c5945cb72c

📥 Commits

Reviewing files that changed from the base of the PR and between b0fb421 and a6520a1.

📒 Files selected for processing (2)

• codeframe/ui/server.py
• tests/ui/test_rate_limit_worker_warning.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/680-detect-bare-uvicorn-workers

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review — fix/680-detect-bare-uvicorn-workers

This is a clean, well-targeted fix. The implementation is appropriately conservative given that this is an advisory-only startup warning.

---

What's good

Refactoring is clean. Renaming the old _detect_worker_count() to _worker_count_from_env() and composing both signals in the new _detect_worker_count() is the right split. The single-responsibility breakdown across _parse_worker_count_from_argv, _is_asgi_server_cmdline, _read_proc_cmdline, _read_proc_ppid, _iter_ancestor_cmdlines, and _detect_workers_from_process_tree makes each unit independently testable.

Never-raise contract is honored at every level. Each function either handles its own errors or wraps the call in a bare except Exception catch-all. The startup path is fully protected.

Test coverage is thorough. The argv parser matrix covers all four flag forms (--workers N, --workers=N, -w N, -w=N), edge cases (trailing flag with no value, non-positive counts, empty argv), and the integration tests correctly assert the max-wins merge logic. The fake-/proc tree walk in test_iter_ancestor_cmdlines_walks_proc is a good pattern for this kind of OS-interface code.

Binary vs. text mode is used correctly. /proc/<pid>/cmdline is read in rb mode (since argv tokens can be arbitrary bytes) while /proc/<pid>/status is read in text mode (PPid: is always ASCII). The decode("utf-8", "replace") fallback is appropriate.

---

Minor observations

_is_asgi_server_cmdline does substring matching on any argv token. A path like /home/ci/uvicorn-test-harness/runner.py would match. For a best-effort advisory this is acceptable — false positives here only mean an unnecessary warning, not a broken startup — but worth documenting the known loose bound in the docstring (e.g., "substring match on any token; may false-positive on paths containing 'uvicorn'"). The current guard is already sufficient to avoid the codex --workers case that motivated it.

_read_proc_ppid does not specify a text encoding. /proc/<pid>/status is always ASCII, but passing encoding="ascii" to open() makes the intent explicit and avoids any locale-dependent behavior on non-standard systems. Minor nit.

test_iter_ancestor_cmdlines_walks_proc implicitly tests two units. After asserting on _iter_ancestor_cmdlines output it also asserts on _detect_workers_from_process_tree(). This is fine since the monkeypatches are still active, but the test name only advertises the walk — the second assertion could be silently broken by a future refactor without the test name signaling it. Not a blocker.

---

Verdict

No correctness issues, no security concerns, no missing error paths. The implementation matches the documented behavior exactly. The observations above are cosmetic and don't block merge.

Approved — good to merge once CI is green.

[claude]

Follow-up review

The second commit (a6520a1c, "docs(server): note loose substring bound + pin ascii encoding") addresses both nits from my initial review:

• Substring bound documented — _is_asgi_server_cmdline's docstring now explicitly calls out: "This is a loose substring match on any token, so a path that merely contains uvicorn/gunicorn would also match. That's acceptable for an advisory-only warning." Exactly what was asked for.

• ASCII encoding pinned — _read_proc_ppid now opens /proc/<pid>/status with encoding="ascii", making the intent explicit and removing any locale-dependent ambiguity.

The third observation (test name not advertising the second assertion in test_iter_ancestor_cmdlines_walks_proc) was noted as cosmetic / not a blocker, and is still fine as-is.

All three minor observations are resolved or accepted. No outstanding concerns.

Still approved — ready to merge.