fix(config): three settings an operator can set that did nothing

[0xmanhnv]

Three settings an operator could set where nothing happened. Each parsed an environment variable at boot and then had no reader on any path the operator could observe.

All three premises held; details and evidence below.

1. Agent load-balancing weights were inert

config.LoadBalancingConfig parsed seven AGENT_LB_* variables that nothing outside config.go read. AgentSelector.selectLeastLoaded ranked agents on CurrentJobs/MaxConcurrentJobs alone.

The heartbeat side was worse than reported. AgentService.UpdateHeartbeat called Agent.UpdateMetrics, which sets CPU/memory/ActiveJobs and never touches LoadScore or MetricsUpdatedAt. UpdateExtendedMetrics — the only method that computes a score — had zero call sites. So the load_score column never reflected anything, and docs/architecture/agent-heartbeat-optimization.md ("Load score // Computed every heartbeat") described something that did not happen. The disk/network weights had no inputs at all: HeartbeatRequest carried no disk or network fields, so AGENT_LB_DISK_IO_WEIGHT and AGENT_LB_NETWORK_WEIGHT could only ever multiply zero.

Fixed:

• LoadBalancingWeights gains the two normalization ceilings, which were file-scope consts inside the scoring function. An operator on NVMe or 10 GbE had no way to say what "100% busy" meant on their hardware.
• config.LoadBalancingConfig.Weights() is the single seam; both AgentService (persisted score) and AgentSelector (job placement) take it at boot.
• Heartbeat recomputes load_score with the configured weights and stamps metrics_updated_at.
• HeartbeatRequest accepts optional disk_read_mbps / disk_write_mbps / network_rx_mbps / network_tx_mbps. Backwards compatible — agents that omit them leave those terms at zero, exactly as today.
• The selector scores with ComputeLoadScoreWithWeights rather than reading the stored column, so a weight change takes effect immediately instead of waiting for every agent to heartbeat.

Two safety valves: resource metrics older than 5 minutes are ignored and the agent is ranked on queue depth alone (a wedged agent keeps its last CPU reading forever, and one bad sample would otherwise bias scheduling permanently), and an all-zero weight set is rejected in favour of the defaults (it would score every agent at 0 and make selection arbitrary).

Existing selector behaviour is preserved: unlimited-capacity agents still short-circuit, fully-loaded agents are still never selected, ties still keep the first candidate.

2. Admin audit retention was permanently a dry run

cmd/server/workers.go hardcoded DryRun: true with no config plumbing, so admin_audit_logs grew forever and the 365-day policy could not be enforced by any deployment. (PriorityAuditRetentionController, right above it, deletes for real at 180 days — the inconsistency is what made this look accidental.)

Now configurable: ADMIN_AUDIT_RETENTION_ENABLED / _DRY_RUN / _DAYS / _INTERVAL / _BATCH_SIZE.

Default chosen: DRY_RUN=true, retention 365 days, controller enabled.

The defensible reading is that the operator decision here is whether to delete, not whether to have a policy. Flipping the default to real deletion would mean an upgrade silently destroys up to a year of platform-admin audit history — irreversible, and audit retention is exactly the thing SOC 2 / PCI DSS / HIPAA auditors ask about. Nobody asked for that on a version bump. Keeping the controller enabled in dry-run is what makes the growth visible rather than silent: it already counts what it would delete, and that report is now WARN-level and names the variable to flip. So the shipped behaviour is byte-identical to today, and the difference is that an operator now has a switch.

Guardrail on the irreversible path: config validation refuses to boot when deletion is enabled with a window under 30 days. RetentionDays: 0 or a negative value puts the cutoff at now-or-later and empties the table; NewAuditRetentionController mapped only == 0 to the default, so a negative slipped through. Both are now caught, and the controller's own fallback widened to <= 0 for defence in depth. Dry-run configs skip the check — nothing is deleted, so a bad value cannot hurt.

3. AI_RATE_LIMIT_RPM enforced nothing

No limiter existed in internal/app/aitriage/ or internal/infra/llm/. A setting that reads as a spend cap was decoration.

The limiter lives at the LLM call boundary — internal/infra/llm/ratelimit.go, a decorator over Provider applied by Factory to every provider it hands out. That is the one seam every triage call passes through (AITriageService.runTriage -> CreateProvider -> provider.Complete). Uses golang.org/x/time/rate, already a direct dependency in go.mod (internal/infra/http/middleware/ratelimit.go); no new dependency.

• One limiter per budget scope, shared process-wide via a registry on the Factory (a single Factory is built in services.go). Per-instance limiters would be trivially bypassed since providers are created per triage job.

• Scope is derived from AI mode plus tenant ID, never from the credential. Platform-mode tenants share the platform key's budget because they share the bill; each BYOK tenant gets its own because it pays its own. CreateProvider keeps its signature and delegates to a new CreateProviderForTenant.

  The first revision keyed the scope on a SHA-256 prefix of the API key. CodeQL's go/weak-sensitive-data-hashing flagged it, and while the alert is a heuristic — the digest was a map key, never stored or compared for authentication — hashing a credential to partition rate limits was the wrong shape regardless. Billing identity is the more accurate unit, the mapping is readable in a log, and no credential is involved. Fixed in the second commit.

• Over-budget calls wait for a token and fail with ErrRateLimited if none arrives within 30s or before the caller's deadline, whichever is sooner. They never reach the provider, which is the point of a cost control. categorizeError already maps ErrRateLimited to a user-facing "AI service is temporarily busy" message, so no new error surface.

• Burst is the full minute's allowance, so a batch of triage jobs starting together is not serialised to one call every 60/rpm seconds; sustained throughput is still capped.

• AI_RATE_LIMIT_RPM=0 disables the cap.

Read internal/app/aitriage/budget.go first as instructed. It is a per-tenant monthly token budget on a different axis (spend volume, persisted, RFC-008), gated off in Phase 1, and its warn/block setters are the documented deferral. A per-process request-rate cap does not belong inside it, so the limiter is separate and the two compose: budget pre-check, then rate limit, then call.

Also in scope, from the same item: AI_AUTO_TRIAGE_DEFAULT_ENABLED, AI_AUTO_TRIAGE_DEFAULT_SEVERITIES and AI_AUTO_TRIAGE_DELAY were equally inert. extractAISettings started from a zero AISettings, and ShouldAutoTriage / EnqueueAutoTriage read TypedSettings() (a JSON round-trip, so absent keys land on Go zero values, not on tenant.DefaultSettings()). Either way an operator who enabled auto-triage platform-wide saw nothing. Those three now seed tenants that have never set the corresponding key; both call sites go through the raw settings map so "absent" stays distinguishable from "explicitly false", and a tenant's explicit choice — including an explicit off — always wins. A tenant's severity list replaces the platform default rather than being unioned with it, which would have silently widened its scope.

Behaviour with no configuration change

Unchanged. Shipped weights equal the previous constants, retention stays a dry run, AI_RATE_LIMIT_RPM=60 is far above real triage volume, AI_AUTO_TRIAGE_DEFAULT_ENABLED=false.

Verification

GOWORK=off go build ./..., go vet ./... and go test ./... all exit 0; 92 packages pass. make lint-new BASE_REF=origin/develop is clean.

Every fix has a test proven red against the unfixed code by reverting only the production change:

Reverted	Test	Failure
selectLeastLoaded scoring	TestAgentSelLoadBalancing_CPUBreaksJobLoadTie	selected "hot" (95% CPU) over "cool" (5% CPU) at identical job load
	TestAgentSelLoadBalancing_WeightsChangeSelection	CPU weighted 0.9, CPU-heavy agent still selected
	TestAgentSelLoadBalancing_ZeroWeightsRejected	picked the saturated agent
throughput ceilings in ComputeLoadScoreWithWeights	TestComputeLoadScore_HonorsThroughputCeilings	disk score = 100 with a 2000 MB/s ceiling at 500 MB/s, want 25
adminAuditRetentionConfig -> hardcoded literal	TestAdminAuditRetention_DryRunIsConfigurable	DRY_RUN=false did not reach the controller; days/interval/batch all ignored
limiter in Complete + factory wiring	TestRateLimitedProvider_BlocksCallOverBudget	call 4 exceeded the 3 rpm cap but was allowed through
	TestRateLimitedProvider_SharedBudgetAcrossProviders	third call across two providers sharing one credential was allowed
	TestRateLimitedProvider_HonorsCallerContext	over-budget call did not fail when the caller's context expired
	TestFactory_AppliesRateLimit	provider is *llm.ClaudeProvider, not rate limited
platform defaults in extractAISettings	TestAITriage_AutoTriageDefaults_ApplyToUnconfiguredTenant	AI_AUTO_TRIAGE_DEFAULT_ENABLED=true did not reach an unconfigured tenant

Plus non-regression tests for the parts that must not change: tenant opt-out beats the platform default, tenant severity list replaces rather than merges, stale metrics fall back to queue depth, AI_RATE_LIMIT_RPM=0 means no cap, budgetScope partitions platform vs BYOK correctly, dry-run configs still validate, and the shipped default is dry-run at 365 days.

Found, not fixed

• AgentHeartbeatData.CurrentJobs is written to Agent.ActiveJobs, while scheduling reads Agent.CurrentJobs (maintained by ClaimJob/ReleaseJob). The names invite a mix-up. Pre-existing; preserved exactly rather than fixed here.
• selectTenantAgent returns SelectAgentResult{Agent: nil, Message: "Tenant agent assigned"} when every candidate is at capacity. FindAvailableWithCapacity should make that unreachable, but the message is a lie if it ever is. Pre-existing and pinned by an existing test, so left alone.
• Agents do not yet send disk/network throughput. The API accepts the fields, so AGENT_LB_DISK_IO_WEIGHT / AGENT_LB_NETWORK_WEIGHT become live the moment the agent reports them; that change belongs in the agent repo.