Skip to content

fix(ccrotate-tier-gate): pick Claude base account by utilization headroom - #6

Merged
kkroo merged 1 commit into
masterfrom
omar/tier-gate-utilization-ranking-blockcast
May 14, 2026
Merged

fix(ccrotate-tier-gate): pick Claude base account by utilization headroom#6
kkroo merged 1 commit into
masterfrom
omar/tier-gate-utilization-ranking-blockcast

Conversation

@kkroo

@kkroo kkroo commented May 14, 2026

Copy link
Copy Markdown

Summary

evaluateTierCacheSnapshot returned the first account with serviceTier ∈ {base, extra}. base/extra mean the org has capacity — they don't say the immediate 5h window has headroom. Result: the gate routinely picked an account at 5h:100% and the agent pod immediately hit [wrapper] terminal rate-limit: out_of_credits overage rejected.

Observed 2026-05-14: gate switched cluster active to ramadan@blockcast.net (base, 5h:100%) while omar.ramadan@berkeley.edu (base, 5h:27%) sat unused. Release Engineer / QA Engineer / UXDesigner failed back-to-back with out_of_credits in the same minute.

Fix

  • Rank candidates by max(utilization5h, utilization7d) ascending.
  • Skip any candidate ≥99% (practical exhaustion — a real run will burn the last 1% on input alone).
  • Accounts missing utilization data (older ccrotate / Usage API cooldown) get a neutral mid-rank (50).
  • Codex path unchanged — its tier-cache has no utilization fields.
  • Adds utilization5h? / utilization7d? to the CcrotateTierCacheAccount.rateLimits type.

Tests

3 new in the evaluateTierCacheSnapshot group:

  • picks lowest-util not first-in-cache (regression case)
  • defers when every base candidate ≥99%
  • neutral-rank for missing util data

All evaluateTierCacheSnapshot tests pass.

Test plan

  • bun test src/__tests__/ccrotate-tier-gate.test.ts -t "evaluateTierCacheSnapshot" → 14/14
  • After deploy: paperclip-0 log should show ccrotate active account switched to base-tier account picking the lowest-util email, not the first.

🤖 Generated with Claude Code

…room

evaluateTierCacheSnapshot iterated accounts in cache order and returned
the FIRST one with serviceTier in {base, extra}. Result: it routinely
picked an account whose 5h window was at 100% — `base` tier just means
"org has capacity," not "this account has 5h headroom." Agent pods then
hit `[wrapper] terminal rate-limit: out_of_credits overage rejected` on
the first API call and exited non-zero.

Observed 2026-05-14: gate switched cluster active to ramadan@blockcast.net
(base, 5h:100%) while omar.ramadan@berkeley.edu (base, 5h:27%) sat unused;
Release Engineer / QA Engineer / UXDesigner all failed back-to-back with
out_of_credits in the same minute.

Fix: rank candidates by max(utilization5h, utilization7d) ascending; skip
any candidate >=99% (practically exhausted). Accounts missing utilization
data (older ccrotate, Usage API cooldown) get a neutral mid-rank so they
tie-break behind known-good accounts but ahead of known-empty ones.

Codex path unchanged — its tier-cache has no utilization fields.
@kkroo
kkroo merged commit 8948378 into master May 14, 2026
1 check failed
@kkroo
kkroo deleted the omar/tier-gate-utilization-ranking-blockcast branch May 14, 2026 19:13
kkroo added a commit that referenced this pull request Jun 13, 2026
* feat(dashboard): agent scorecard aggregation + API (BLO-10275)

Adds per-agent cost/done-issue, failure rate, and review pass rate over a
configurable window for the monthly staffing routine (retro BLO-10264 #6).

- agent-scorecards.ts: pure computeAgentScorecards() with per-metric and
  agent-level low-sample gating (MIN_SAMPLE_DONE/RUNS/REVIEWS) so thin-sample
  agents are never ranked as real performers.
- dashboardService.agentScorecards(): four grouped-by-agent aggregates over
  cost_events / issues(done) / heartbeat_runs / last_evidence_verdict.
- GET /companies/:companyId/agent-scorecards?windowDays=.
- DB-free unit test covering normal, zero-done, low-sample, zero-activity,
  cancelled-excluded, and warn/block-not-pass cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(ui): agent scorecards panel on the board dashboard (BLO-10275)

Renders cost/done, failure rate, and review pass per agent with a window
selector (7/30/90d). Low-sample agents are split into a separate, un-ranked
'Insufficient sample' group (Von Restorff) and every rate shows its sample
size n inline (Recognition-over-Recall) so 1/1 can't read as a track record.
Color-independent + tooltipped metric headers (WCAG). Anchor #agent-scorecards
for the staffing routine to cite. Uses semantic theme tokens, no inline hex.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(ui): storybook story + fixtures for agent scorecards panel (BLO-10275)

Renders the real AgentScorecardsPanel via the storybook fetch fixture so the
panel's populated (ranked + paused poor performer + insufficient-sample group)
and empty states are verifiable without a backend. Includes the captured
1440x900 + 390x844 screenshots used for the BLO-10275 visual-truth gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(scorecards): attribute issue metrics to implementer + window heartbeat runs on started_at (BLO-10275)

Addresses Ally's two Important review findings on PR #397:

1. Issue-derived metrics (doneIssues, costPerDoneIssue, reviewPassRate)
   were grouped by the mutable issues.assignee_agent_id. Execution-policy
   review/approval stages reassign the issue to the reviewer while it is
   in review and can leave it assigned there at `done`, so the reviewer
   could be credited for the implementer's work. Now derive an immutable
   implementer via COALESCE(executionState.returnAssignee.agentId when it
   is an agent, assignee_agent_id) for both the done and review aggregates.

2. The heartbeat-run aggregate filtered on created_at, which sits behind
   liveness_state in its only composite index and would force a company-wide
   scan as run history grows. Now window on started_at — the time column in
   the (company_id, agent_id, started_at) index — and restrict to terminal
   statuses in SQL so non-terminal rows are never grouped just to be
   discarded in TS.

Adds DB-backed regression tests in dashboard-service.test.ts:
- credits the implementer (returnAssignee), not the reviewer it was
  reassigned to (worker A implements, reviewer B holds it at done -> A
  gets the done/review/cost-per-done credit, B gets none)
- falls back to the current assignee for issues with no execution policy
- counts only terminal heartbeat runs windowed on started_at, not
  created_at (out-of-window-by-started_at run is in-window by created_at,
  pinning the index fix)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(scorecards): index review verdict window

Materialize the evidence verdict evaluated timestamp so the dashboard scorecard review-pass aggregate can use a normal company/timestamp index instead of scanning JSONB for every company issue.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(scorecards): register agent-scorecards OpenAPI route + raise access-route test timeouts (BLO-10337)

- server/src/routes/openapi.ts: register GET /api/companies/{companyId}/agent-scorecards
  so openapi-routes "covers the mounted server routes exactly" passes (route existed in
  dashboard.ts but was undocumented).
- server/src/__tests__/access-routes-permissions-upgrade.test.ts: raise beforeAll timeout
  to 60_000 and the self-lockout test timeout to 20_000 to absorb embedded-Postgres
  startup/runtime variance (Verify serialized server suites 1/4 flake).

Reproduces CTO local fix 01b60a0ca which could not be pushed from its runtime.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: kkroo <kkroo@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: kkroo <60861014+kkroo@users.noreply.github.com>
Co-authored-by: allyblockcast[bot] <290875700+allyblockcast[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
allyblockcast Bot pushed a commit that referenced this pull request Aug 23, 2026
…EN-2370)

Ally's consolidated review on 8dab928 was clean, so this commit fixes no
defect. It closes out the one acceptance criterion that a clean review
cannot discharge on its own: PEN-2370 ask 3's "the method is the control"
-- every remediation must be followed by an attempt to reach the same
material *another way*, rather than by re-reading the patch. Doors #5 and
#6 were both found by that method; this is that method run against the
scrubber itself.

Four routes were probed. All four already passed on the first probe:

  1. Workload controllers -- resources_get(apps/v1, Deployment) is an
     advertised use of the same read-only grant, and its material sits at
     spec.template.spec.containers[], a level deeper than a pod's.
  2. CronJob -- spec.jobTemplate.spec.template.spec.containers[], the
     deepest nesting the core API ships.
  3. List responses -- resources_list/pods_list return items[]; a
     root-only scrub would disclose a whole namespace per call.
  4. metadata.annotations["kubectl.kubernetes.io/last-applied-config..."]
     -- kubectl stores the whole submitted object, values included, as
     JSON *text*. A structural walker sees one opaque string rather than
     a container list, so this route sidesteps the env rule entirely.

The scrubber earns all four by keying on `containers:`/`env:` by NAME at
any depth, never on a pod-shaped `spec.containers` path. That property
was load-bearing and unpinned: every one of the 105 pre-existing cases
reads a bare pod, so the obvious "tighten the match to the pod path"
refactor would have reopened all four routes with the suite still green.
Pinning it is the deliverable here -- coverage, not behaviour.

Each assertion was verified to be load-bearing rather than vacuous: with
scrubYamlText and scrubJsonValue stubbed to identity, all seven fail; the
stub was reverted and the module is byte-identical to 8dab928. A test
that cannot fail would manufacture exactly the false assurance the module
header warns about.

Test-only change. No fixture carries real credential material -- every
one uses the file's existing synthetic LEAKED_* marker, and the central
assertion is that no marker survives.

Refs: PEN-2370
Signed-off-by: Cto <cto@paperclip.blockcast.net>
kkroo pushed a commit that referenced this pull request Aug 23, 2026
…EN-2370)

Ally's consolidated review on 8dab928 was clean, so this commit fixes no
defect. It closes out the one acceptance criterion that a clean review
cannot discharge on its own: PEN-2370 ask 3's "the method is the control"
-- every remediation must be followed by an attempt to reach the same
material *another way*, rather than by re-reading the patch. Doors #5 and
#6 were both found by that method; this is that method run against the
scrubber itself.

Four routes were probed. All four already passed on the first probe:

  1. Workload controllers -- resources_get(apps/v1, Deployment) is an
     advertised use of the same read-only grant, and its material sits at
     spec.template.spec.containers[], a level deeper than a pod's.
  2. CronJob -- spec.jobTemplate.spec.template.spec.containers[], the
     deepest nesting the core API ships.
  3. List responses -- resources_list/pods_list return items[]; a
     root-only scrub would disclose a whole namespace per call.
  4. metadata.annotations["kubectl.kubernetes.io/last-applied-config..."]
     -- kubectl stores the whole submitted object, values included, as
     JSON *text*. A structural walker sees one opaque string rather than
     a container list, so this route sidesteps the env rule entirely.

The scrubber earns all four by keying on `containers:`/`env:` by NAME at
any depth, never on a pod-shaped `spec.containers` path. That property
was load-bearing and unpinned: every one of the 105 pre-existing cases
reads a bare pod, so the obvious "tighten the match to the pod path"
refactor would have reopened all four routes with the suite still green.
Pinning it is the deliverable here -- coverage, not behaviour.

Each assertion was verified to be load-bearing rather than vacuous: with
scrubYamlText and scrubJsonValue stubbed to identity, all seven fail; the
stub was reverted and the module is byte-identical to 8dab928. A test
that cannot fail would manufacture exactly the false assurance the module
header warns about.

Test-only change. No fixture carries real credential material -- every
one uses the file's existing synthetic LEAKED_* marker, and the central
assertion is that no marker survives.

Refs: PEN-2370
Signed-off-by: Cto <cto@paperclip.blockcast.net>
kkroo pushed a commit that referenced this pull request Aug 23, 2026
…EN-2370)

Ally's consolidated review on 8dab928 was clean, so this commit fixes no
defect. It closes out the one acceptance criterion that a clean review
cannot discharge on its own: PEN-2370 ask 3's "the method is the control"
-- every remediation must be followed by an attempt to reach the same
material *another way*, rather than by re-reading the patch. Doors #5 and
#6 were both found by that method; this is that method run against the
scrubber itself.

Four routes were probed. All four already passed on the first probe:

  1. Workload controllers -- resources_get(apps/v1, Deployment) is an
     advertised use of the same read-only grant, and its material sits at
     spec.template.spec.containers[], a level deeper than a pod's.
  2. CronJob -- spec.jobTemplate.spec.template.spec.containers[], the
     deepest nesting the core API ships.
  3. List responses -- resources_list/pods_list return items[]; a
     root-only scrub would disclose a whole namespace per call.
  4. metadata.annotations["kubectl.kubernetes.io/last-applied-config..."]
     -- kubectl stores the whole submitted object, values included, as
     JSON *text*. A structural walker sees one opaque string rather than
     a container list, so this route sidesteps the env rule entirely.

The scrubber earns all four by keying on `containers:`/`env:` by NAME at
any depth, never on a pod-shaped `spec.containers` path. That property
was load-bearing and unpinned: every one of the 105 pre-existing cases
reads a bare pod, so the obvious "tighten the match to the pod path"
refactor would have reopened all four routes with the suite still green.
Pinning it is the deliverable here -- coverage, not behaviour.

Each assertion was verified to be load-bearing rather than vacuous: with
scrubYamlText and scrubJsonValue stubbed to identity, all seven fail; the
stub was reverted and the module is byte-identical to 8dab928. A test
that cannot fail would manufacture exactly the false assurance the module
header warns about.

Test-only change. No fixture carries real credential material -- every
one uses the file's existing synthetic LEAKED_* marker, and the central
assertion is that no marker survives.

Refs: PEN-2370
Signed-off-by: Cto <cto@paperclip.blockcast.net>
kkroo pushed a commit that referenced this pull request Aug 23, 2026
…EN-2370)

Ally's consolidated review on 8dab928 was clean, so this commit fixes no
defect. It closes out the one acceptance criterion that a clean review
cannot discharge on its own: PEN-2370 ask 3's "the method is the control"
-- every remediation must be followed by an attempt to reach the same
material *another way*, rather than by re-reading the patch. Doors #5 and
#6 were both found by that method; this is that method run against the
scrubber itself.

Four routes were probed. All four already passed on the first probe:

  1. Workload controllers -- resources_get(apps/v1, Deployment) is an
     advertised use of the same read-only grant, and its material sits at
     spec.template.spec.containers[], a level deeper than a pod's.
  2. CronJob -- spec.jobTemplate.spec.template.spec.containers[], the
     deepest nesting the core API ships.
  3. List responses -- resources_list/pods_list return items[]; a
     root-only scrub would disclose a whole namespace per call.
  4. metadata.annotations["kubectl.kubernetes.io/last-applied-config..."]
     -- kubectl stores the whole submitted object, values included, as
     JSON *text*. A structural walker sees one opaque string rather than
     a container list, so this route sidesteps the env rule entirely.

The scrubber earns all four by keying on `containers:`/`env:` by NAME at
any depth, never on a pod-shaped `spec.containers` path. That property
was load-bearing and unpinned: every one of the 105 pre-existing cases
reads a bare pod, so the obvious "tighten the match to the pod path"
refactor would have reopened all four routes with the suite still green.
Pinning it is the deliverable here -- coverage, not behaviour.

Each assertion was verified to be load-bearing rather than vacuous: with
scrubYamlText and scrubJsonValue stubbed to identity, all seven fail; the
stub was reverted and the module is byte-identical to 8dab928. A test
that cannot fail would manufacture exactly the false assurance the module
header warns about.

Test-only change. No fixture carries real credential material -- every
one uses the file's existing synthetic LEAKED_* marker, and the central
assertion is that no marker survives.

Refs: PEN-2370
Signed-off-by: Cto <cto@paperclip.blockcast.net>
allyblockcast Bot pushed a commit that referenced this pull request Aug 31, 2026
…fier (PEN-2370)

The parent commit claimed one prefix predicate for "every body sniff". It
missed one, and the miss was reachable.

`scrubTextTracked` -- the classifier that recurses into a resource nested in
`content[].text`, which is how the k8s MCP servers actually deliver a resource
-- kept its own notion of where a document begins:

  trimStart()  counts U+FEFF as whitespace (ECMAScript WhiteSpace), so it
               ACCEPTS a BOM
  JSON.parse   rejects one

So a BOM-prefixed nested document classified as JSON, threw on parse, and fell
through to the `catch` into the YAML scanner -- which does not match a compact
single-line JSON document. The entry passed through with
`spec.containers[].env[].value` in the clear. Exactly the fail-open the parent
commit unified the byte-level sniffs to close, surviving one layer down because
this classifier was reached by recursion rather than by dispatch, and the
parent only audited dispatch.

Driving the real `scrubResponseBody` before the fix: a pod body whose nested
`content[].text` opens on a BOM leaked its value, while the same body without
the BOM scrubbed. That asymmetry is the whole bug.

The fix is the parent's own shape, applied where it was missed.
`stripLeadingBom` is now the single string-side definition, used by both
`scrubJsonRpcBody` and `scrubTextTracked`; `significantByteOffset` remains the
byte-side one. Two representations, one definition each -- the failure this
file keeps re-learning is a second private copy, not a second representation.

The YAML fallback deliberately keeps the ORIGINAL text: that scanner is
line-based and already tolerates a BOM, and handing it the stripped copy would
drop that byte from every YAML body we pass through unchanged, breaking
byte-exact pass-through for a case that was never broken. Verified: a
BOM-prefixed YAML pod scrubs, and a BOM-prefixed benign body round-trips
identical.

The matrix gains the nested entry point, with the prefix applied to the INNER
document -- that is the offset the recursive classifier actually sees. Wrapping
the prefix outside would have tested the parent's fix a second time and this
one not at all.

Reverting the source with the new tests fails exactly the 2 BOM rows of the
nested entry point; the other 4 nested rows and all 12 pre-existing rows pass
in both states, so this is a tightening, not a widening into redact-everything.
361/361 in the package, tsc clean.

Method note, since PEN-2370 ask 3 names it as the control: this was not found
by re-reading the patch. Ally's review returned zero findings on the parent and
CI was fully green -- including the mcp-gateway lane, which I confirmed really
executed its 245 suites rather than trusting the aggregate. It was found by
going looking for the same material through a different route, which is the
criterion the ticket asks every remediation to be followed by. Door #5 and #6
came from auditing a fix's own blind spots; so did this one.

Signed-off-by: Cto <cto@paperclip.blockcast.net>
kkroo pushed a commit that referenced this pull request Aug 31, 2026
…fier (PEN-2370)

The parent commit claimed one prefix predicate for "every body sniff". It
missed one, and the miss was reachable.

`scrubTextTracked` -- the classifier that recurses into a resource nested in
`content[].text`, which is how the k8s MCP servers actually deliver a resource
-- kept its own notion of where a document begins:

  trimStart()  counts U+FEFF as whitespace (ECMAScript WhiteSpace), so it
               ACCEPTS a BOM
  JSON.parse   rejects one

So a BOM-prefixed nested document classified as JSON, threw on parse, and fell
through to the `catch` into the YAML scanner -- which does not match a compact
single-line JSON document. The entry passed through with
`spec.containers[].env[].value` in the clear. Exactly the fail-open the parent
commit unified the byte-level sniffs to close, surviving one layer down because
this classifier was reached by recursion rather than by dispatch, and the
parent only audited dispatch.

Driving the real `scrubResponseBody` before the fix: a pod body whose nested
`content[].text` opens on a BOM leaked its value, while the same body without
the BOM scrubbed. That asymmetry is the whole bug.

The fix is the parent's own shape, applied where it was missed.
`stripLeadingBom` is now the single string-side definition, used by both
`scrubJsonRpcBody` and `scrubTextTracked`; `significantByteOffset` remains the
byte-side one. Two representations, one definition each -- the failure this
file keeps re-learning is a second private copy, not a second representation.

The YAML fallback deliberately keeps the ORIGINAL text: that scanner is
line-based and already tolerates a BOM, and handing it the stripped copy would
drop that byte from every YAML body we pass through unchanged, breaking
byte-exact pass-through for a case that was never broken. Verified: a
BOM-prefixed YAML pod scrubs, and a BOM-prefixed benign body round-trips
identical.

The matrix gains the nested entry point, with the prefix applied to the INNER
document -- that is the offset the recursive classifier actually sees. Wrapping
the prefix outside would have tested the parent's fix a second time and this
one not at all.

Reverting the source with the new tests fails exactly the 2 BOM rows of the
nested entry point; the other 4 nested rows and all 12 pre-existing rows pass
in both states, so this is a tightening, not a widening into redact-everything.
361/361 in the package, tsc clean.

Method note, since PEN-2370 ask 3 names it as the control: this was not found
by re-reading the patch. Ally's review returned zero findings on the parent and
CI was fully green -- including the mcp-gateway lane, which I confirmed really
executed its 245 suites rather than trusting the aggregate. It was found by
going looking for the same material through a different route, which is the
criterion the ticket asks every remediation to be followed by. Door #5 and #6
came from auditing a fix's own blind spots; so did this one.

Signed-off-by: Cto <cto@paperclip.blockcast.net>
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.

1 participant