Skip to content

feat(runtime): add process-local per-model RPM admission limits - #1116

Merged
kevincodex1 merged 4 commits into
Twigpine:mainfrom
LortuArte:lortuarte/rpm-390
Oct 13, 2026
Merged

kevincodex1 merged 4 commits into
Twigpine:mainfrom
LortuArte:lortuarte/rpm-390

Conversation

@LortuArte

@LortuArte LortuArte commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Refs #390 — approved RPM-only slice. This does not implement the broader cost/TPM request.

Summary

Zero currently has no local per-model RPM admission gate at the turn-session boundary.

This change adds a process-local sliding 60-second request window keyed by resolved model ID and rejects request N+1 before the wrapped TurnSession.Stream method is entered.

The limiter is wired into:

  • zero exec
  • interactive sessions
  • model escalation

while preserving the existing provider/transport selection.

FileConfig.modelRPM configures the cap. Unset or zero means unlimited.

Project configuration may tighten an existing user limit, but may not relax it.

Admitted requests consume their slot even when the provider subsequently fails.

Scope / non-goals

This PR is intentionally limited to the scope discussed in #390:

  • RPM only
  • per process
  • sliding 60-second window
  • pre-send admission
  • per resolved model ID
  • native Go
  • clean zero exec error

It does not add:

  • TPM limits
  • monetary/cost budgets
  • persistence across restarts
  • cross-process coordination
  • account-wide limits
  • hidden sleeps or retries
  • Python or AEGIS integration
  • any new runtime dependency

Separate processes have independent windows and restarting Zero clears the limiter state.

The limiter counts TurnSession.Stream admissions. It does not claim to count setup/prewarm traffic, compaction traffic, provider-internal retries, discovery calls, or requests made outside this boundary.

See docs/MODEL_RPM.md.

Regression proof

The regression tests verify that request N+1 is rejected before entering the wrapped transport.

As a negative control, I temporarily bypassed the admission wrapper.

With the gate removed:

  • the provider regression test failed with N+1 was not denied: <nil>
  • the exec regression test observed two provider calls instead of one

The wrapper was then restored before final validation.

This verifies that the new tests fail when the behavior they are intended to protect is removed.

Concurrency result

A 100-request concurrent test with a configured RPM limit of 7 produced:

  • 100 total attempts
  • 7 admissions
  • 7 wrapped transport calls
  • 93 local rejections

No additional admitted request reached the wrapped transport.

Validation environment

Original validation run: 2026-09-27.

Revalidated for delivery on 2026-10-03.

Before opening this draft, I rechecked the upstream repository and confirmed that main still points to the exact validated base:

99721c762f37cd43ac511007a5f51d1846df959e

The submitted branch was compared against the preserved validation files before publication.

Tested on:

  • Linux amd64
  • Go 1.26.6
  • base commit 99721c762f37cd43ac511007a5f51d1846df959e

Passing checks

  • focused RPM/config/provider/CLI/errhint tests: PASS
  • affected RPM/escalation tests under -race: PASS
  • make fmt-check: PASS
  • go vet ./...: PASS
  • go run ./cmd/zero-release build: PASS
  • go run ./cmd/zero-release smoke: PASS
  • make vulncheck: PASS (No vulnerabilities found)
  • git diff HEAD --check: PASS
  • patch application against the clean base with git apply --check: PASS

Full-suite limitation

go test ./... does not pass completely in this validation environment.

It reports 14 failing tests.

I repeated the same suite on an unmodified clean checkout of main at:

99721c762f37cd43ac511007a5f51d1846df959e

under the same environment and network restrictions.

The same 14 test names fail on clean main.

No additional failed-test names were introduced by this patch.

The reproduced failures are:

  1. TestAltScreenTranscriptScrollKeepsFooterFixed
  2. TestPlatformTransportRoundTrip
  3. TestServeCleansStatusThroughBoundDirectoryAfterSwap
  4. TestServePreservesPreviousStatusWhenReplacementFails
  5. TestServeRemovesSocketBoundAfterFinalRuntimeSwap
  6. TestServeSupportsReadOnlyCustomRuntimeDirectory
  7. TestServerEndToEnd
  8. TestServerPublishesDefaultStatusAfterCrashReportCreatesRuntimeDirectory
  9. TestServerRejectsUnknownCommand
  10. TestServerSecondInstanceFails
  11. TestTerminateAndReapDaemonProcess
  12. TestTerminateCommandKillsChildAfterLeaderExits
  13. TestTerminateCommandStopsUnconfiguredCommand
  14. TestUnixTransportFallbackAndSocketMode

The failures include Unix-socket restrictions (operation not permitted), daemon/process lifecycle tests and one transcript-title assertion.

This comparison is evidence that the same failed-test names reproduce on the clean base in this runner.

It is not being presented as a full-suite PASS and it does not establish macOS or Windows validation.

No unrelated test was disabled or modified to hide these failures.

Static lint

make lint-static is also not completely clean in this environment.

It reports four findings:

  • internal/installtest/workflow_permissions_test.go:20:19 — QF1001
  • internal/proxydial/proxydial.go:67:15 — QF1008
  • internal/proxydial/proxydial.go:72:33 — QF1008
  • internal/tools/web_fetch.go:315:21 — QF1008

The same four findings reproduce on clean main, and these files are not modified by this PR.

I left the unrelated code unchanged.

Network isolation note

The new CLI fixtures explicitly inject a simulated/no-op MCP runtime.

For the broader validation run, external HTTP was rejected using a local deny-only HTTP proxy so tests did not make live MCP requests.

That proxy is not an OS-level network sandbox.

No credentials, live provider accounts, or real funds were used by the new fixtures.

The lint/vulnerability tooling was allowed to obtain its normal public tooling/data.

Windows revalidation — 2026-10-04

After the initial draft review, I fixed a recovery-path issue where ModelRPM could be lost when Resolve returned ErrNoActiveProvider.

Regression evidence:

  • before the fix, the new regression test failed with ModelRPM[gpt-test] = 0
  • after the fix, ./internal/config passes
  • the interactive provider-fallback test confirms the RPM limiter is preserved downstream
  • go test ./internal/config ./internal/cli -count=1: PASS
  • go test -race ./internal/config ./internal/cli -count=1: PASS
  • go vet ./...: PASS
  • formatting check: PASS
  • go run ./cmd/zero-release build: PASS
  • go run ./cmd/zero-release smoke: PASS
  • govulncheck: PASS (No vulnerabilities found)
  • git diff HEAD --check: PASS

Windows environment:

  • Windows amd64
  • Go 1.26.6
  • CGO enabled for race validation
  • MSYS2 UCRT64 GCC 16.2.0

The repository-wide go test ./... is still not all-green in this Windows environment.

On the PR branch, one full-suite run failed in internal/tools at TestExecCommandForegroundServerReturnsSessionAndServesHTTP; that same test then passed in isolation.

I then created a detached clean worktree at the exact base commit:

99721c762f37cd43ac511007a5f51d1846df959e

On that clean base:

  • TestExecCommandForegroundServerReturnsSessionAndServesHTTP passed 5/5 in isolation
  • a full go test ./... run still failed, but in a different unrelated baseline test: TestACPEndToEndPrompt with context deadline exceeded

I have not modified unrelated tests or runtime code to hide these environment/baseline failures.

Latest fix commit:

9de88d736ed1d1202e1093d958bcb44d7c1f2836

Maintainer review status

The RPM implementation, recovery-path regression fix, focused tests, race validation, vet, build, smoke, formatting, vulnerability scan and diff hygiene all pass locally.

The repository-wide Windows suite is not all-green, but the clean base also produces unrelated full-suite failures under the same environment.

The PR is ready for project CI and maintainer review. I have intentionally left unrelated baseline/environment failures unchanged.

Disclosure

AI-assisted implementation and local verification.

This PR does not claim maintainer endorsement, Zero adoption of AEGIS, or production certification.

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features
    • Configure per-model completion limits with modelRPM. Requests over a limit are rejected locally, with guidance on when to retry.
    • Project settings can tighten limits set in user configuration, but cannot relax them.
    • Limits are shared across sessions and model switches within a process; separate processes have independent limits.
  • Documentation
    • Added a guide to per-model limits, covering configuration rules, timing, and which requests count. The user documentation now links to it.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Twigpine/zero/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bc15c3fe-5d09-4b41-8688-78adcb230405
📥 Commits

Reviewing files that changed from the base of the PR and between 5f29a84 and 06e9116.

📒 Files selected for processing (1)
  • internal/config/resolver_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/config/resolver_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Adds configurable per-model completion limits with a process-local sliding-window limiter. The limiter applies to exec and interactive turn sessions, including provider escalation. Configuration validation, error hints, tests, and user documentation are also added.

Changes

Per-model RPM limits

Layer / File(s) Summary
Configure and resolve model limits
internal/config/types.go, internal/config/model_rpm.go, internal/config/resolver.go, internal/config/model_rpm_test.go, internal/config/resolver_test.go
Configuration accepts modelRPM values, normalizes model names, rejects invalid or conflicting limits, and applies project and provider-command limits without relaxing the user cap. Resolved configuration carries the merged limits.
Enforce the sliding-window limit
internal/zeroruntime/rpm.go, internal/zeroruntime/rpm_test.go
A process-local limiter tracks per-model stream admissions over 60 seconds. It rejects over-limit calls before the underlying stream, reports retry duration, and counts admitted calls even when the underlying stream fails.
Apply limits to provider sessions
internal/providers/factory.go, internal/providers/turn_session.go, internal/providers/escalation.go, internal/providers/model_rpm_test.go
Configured sessions wrap streams with the limiter. Escalation passes the limiter to switched-provider sessions. Tests cover optimized and unoptimized session paths.
Wire limits into CLI and interactive turns
internal/cli/app.go, internal/cli/exec.go, internal/cli/model_rpm_test.go, internal/cli/app_test.go, internal/tui/options.go, internal/tui/model.go, internal/errhint/errhint.go, internal/errhint/errhint_test.go, docs/MODEL_RPM.md, docs/README.md
Exec and TUI setup pass the limiter into provider sessions. Local RPM errors receive CLI and TUI guidance. Tests cover both entry points, and the documentation describes configuration and limiter behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant ConfiguredTurnSessions
  participant ModelRPMLimiter
  participant TurnSessionProvider
  CLI->>ConfiguredTurnSessions: provide profile and limiter options
  ConfiguredTurnSessions->>ModelRPMLimiter: wrap the selected model session
  CLI->>ModelRPMLimiter: call Stream
  ModelRPMLimiter->>TurnSessionProvider: forward admitted Stream call
Loading

Suggested reviewers: vasanthdev2004

Merge Risk: ⚪ Minimal · up to 06e91

No actionable issue is established for this incremental change; it is mergeable after normal checks.

Security architecture risk: 🔵 Low · up to 5f29a

No introduced security weakness was established in the reviewed paths. Existing limits cannot be relaxed by project configuration, and concurrent admissions are checked atomically. Protection is intentionally limited to selected requests within one running process; broader security coverage remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The new control affects configured-model admissions through the shared command-line or interactive limiter. It does not isolate independent callers using the same model within that limiter, coordinate other processes, or enforce a monetary budget.

Trust Boundaries and Controls

  • observed — Project configuration and provider-command output can add or tighten an existing limit but cannot increase it or replace a positive cap with zero. Provider-command limits are cleared before the unrestricted configuration merge, preventing a second merge from weakening the result.
  • observed — Escalation replaces the model field while retaining the provider profile's existing credential, endpoint, and header selection. Replacement sessions are wrapped with the shared limiter, preserving admission history when returning to a previously used model.

Resilience and Maintainability Implications

  • observed — Rejected admissions do not enter the wrapped Stream or append a timestamp. Successful admissions remain counted even if transport execution fails, preventing failures from repeatedly refunding capacity. Expired timestamps are removed during subsequent admission checks.
Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 21.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 19 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: process-local per-model RPM admission limits in the runtime.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 3, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 4, 2026
@LortuArte
LortuArte marked this pull request as ready for review October 4, 2026 07:36
@kevincodex1

Copy link
Copy Markdown
Member

hello @LortuArte thanks for your contribution please rebase to main and fix conflicts

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for keeping to the slice. I checked it against the list from #390, and it's all there: modelRPM on FileConfig keyed through ResolveID, admission inside Stream before the wrapped session is entered, the exec hint in errhint beside the rate-limit case, native Go and no new dependency. The wrapper forwards Prewarm, Compact and Close untouched, escalation shares the one limiter so a switch can't reset a cap, and the TUI builds the wrapper per run, so /model keys the new model.

I mutated it to see what the tests pin. Skipping admission, letting project config relax a cap, leaving escalated sessions unlimited, keying by the raw model string, keeping a request exactly 60s old and dropping the errhint case each fail a test. Two things before it can go in:

  • Merge main, as @kevincodex1 asked. It conflicts in internal/cli/app.go and internal/config/resolver.go after today's merges. The red Security & Code Health check is GO-2026-6617 in Go 1.26.6's net/http, and main moved to Go 1.26.9 today in #1124, so merging main clears that as well.
  • Pin the provider-command rule. mergeModelRPM(&cfg.ModelRPM, commandConfig.ModelRPM, true) keeps a provider command from relaxing the user's cap, but nothing tests it: passing false there leaves every test green. It sits next to the sandbox and crossSessionInbound handling for the same reason, since a provider command is an arbitrary executable, so it deserves the same kind of test.

Locally on Windows, the new RPM tests and internal/zeroruntime, internal/providers and internal/errhint pass. The one internal/config failure is TestResolveReportsExplicitMaxTurns, which reads the developer's environment; it fails the same way on this branch's base, and #1072 fixed it on main today.

Requesting changes for the two items above.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 13, 2026
@LortuArte

Copy link
Copy Markdown
Contributor Author

@Vasanthdev2004 @kevincodex1 — addressed both requested items on the latest head (5f29a841).

  • Rebased onto current main and resolved the internal/cli/app.go / internal/config/resolver.go conflicts while preserving the RPM slice.
  • Added TestResolveProviderCommandCannotRelaxUserModelRPM to pin the provider-command precedence rule.

I mutation-checked the new regression test:

  • restrictOnly=true → PASS
  • restrictOnly=false → FAIL (10, expected user cap 5)
  • restored true → PASS

Local validation is green for internal/config, internal/zeroruntime, internal/providers, and internal/errhint.

The two PR workflows are currently awaiting maintainer approval. Ready for re-review once they run.

kevincodex1
kevincodex1 previously approved these changes Oct 13, 2026

@kevincodex1 kevincodex1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks!

@kevincodex1

Copy link
Copy Markdown
Member

please address the failing test in code quality seems a formatting issue @LortuArte

@LortuArte

Copy link
Copy Markdown
Contributor Author

@kevincodex1 — fixed on the latest head 06e9116.

The failing Code Quality check was the gofmt issue in internal/config/resolver_test.go. I ran gofmt, git diff --check, and go test ./internal/config -count=1 locally; the config package passes.

The latest CI workflows are currently awaiting maintainer approval.

@kevincodex1 kevincodex1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevincodex1
kevincodex1 merged commit eebb384 into Twigpine:main Oct 13, 2026
10 checks passed
@LortuArte

Copy link
Copy Markdown
Contributor Author

Thanks for the review and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants