fix(usage): make /limit a real stop boundary and stop reporting unknown spend as zero [SC-A5.1] - #78
Merged
Merged
Conversation
…wn spend as zero
Lane SC-A5, slice 1 of truthful usage and continuity.
/limit was a control that enforced nothing.
`uvtSpent` was only ever written as `= 0` — at construction, in purge(), and
on snapshot restore. No usage frame ever incremented it. `checkUvtCap()` had
zero callers anywhere in the tree. So:
* `/limit 50000` printed "agent will pause and ask permission if ceiling
hit". Nothing paused. Nothing checked. The cap was never consulted.
* `/limit` reported "spent: 0" and a 0% bar for every session, whatever it
had actually cost, because the number was a constant.
* The field comment claimed it was "read from custody log". Nothing read it.
Three changes.
1. Usage is measured, or it is unknown. Never zero by default.
`uvtObserved: number | null` replaces the always-zero counter. null means no
authoritative frame has arrived, which is not the same as a measured zero
and is no longer rendered as one. Nothing estimates UVT from token counts —
only the server's own number is recorded.
`uvtSpent` survives as a getter for the HUD, which needs a number, and is
documented as reporting 0 when the answer is unknown so that anything which
must tell those apart reads `uvtObserved`.
2. Turns settle once, by id.
The terminal frame carries the turn total and a reconnect can replay it, so
`settleTurn(turnId, uvt)` is keyed rather than accumulated blindly. A
replayed done frame is ignored; distinct turns accumulate.
3. The cap is checked before a billable turn starts, and says what it is.
`runCloudTurn` consults `checkUvtCap()` before doing anything and refuses to
start when the observed spend has reached the cap. Two states deliberately
do not trip it: a local brain (Aether meters nothing, so it is marked
unmetered rather than zero-spend) and a session with no observed usage
(there is no evidence the cap was reached, and guessing either blocks free
work or waves through expensive work).
The wording is now accurate about what it can and cannot do:
no further turn will START once the server-reported spend reaches it.
a turn already in flight may still complete and be billed.
this is a local stop only — your plan and balance are unchanged.
A design change came out of the mutation pass rather than the plan. `remaining`
was a number, and an unmeasured session reported the full cap as headroom —
which is the same false zero in a different costume: it tells the user their
whole budget is intact when in truth none of it has been counted. The first
mutation run did not fail any test, which is what exposed it. `remaining` is now
`number | null`, null when unmeasured, and a test pins it.
Tests: 10 added. Unknown is not zero; a duplicate done frame does not
double-count; distinct turns accumulate; an unknown session is never reported
as capped; the cap trips on reaching it; no cap never caps; local sessions are
labelled unmetered; purge returns to unknown rather than to zero; and
unmeasured headroom is null rather than the full cap.
Mutation-checked, both guards: restoring the false-zero headroom and removing
the replay dedupe fails two tests (8 pass / 2 fail); restoring gives 10 / 10.
Gates at this commit:
npm run typecheck exit 0
npm test 932 pass / 0 fail (922 on clean 41a7e26)
Known limits. The HUD still renders `uvtUsed: reg.uvtSpent`, so it shows 0 for
an unknown session — hud.ts is SC-INT's surface and is deliberately not touched
here. Session-cap persistence across resume, and the cross-workspace rejection
that goes with it, are a later slice. The cap is enforced on the REPL cloud
path; `aether agent` has its own loop and is not yet gated.
… in runTurn
Found by SC-INT when the six lanes were composed, not by this lane's own tests
or CI — both of which were green.
The patch that introduced the gate was anchored on a signature shared by
runTurn and runCloudTurn, so the cap check and the turn-id allocation were
written into both. Every turn ran checkUvtCap() twice and consumed two ids from
the counter.
Not a correctness bug: settleTurn keys off the id runCloudTurn actually uses,
and a doubled gate returns the same verdict both times. But it is duplicated
control flow on a spend boundary, and this PR's own description says the gate
lives in runCloudTurn.
Removed the runTurn copy. The gate now exists once, in runCloudTurn, which is
also the only billable path — local turns are unmetered and must not be gated
at all.
Gates at this commit:
npm run typecheck exit 0
npm test 932 pass / 0 fail — unchanged, which is the point:
the duplication was invisible to the test suite
Unrelated to this lane's subject. Landing here because it turned windows-latest
red on this PR and would do the same to any other lane that ran at the wrong
moment.
"a hanging backend cannot stall the fast report" injected clients that never
resolve, set timeoutMs to 50, and then asserted:
assert.ok(Date.now() - started < 500);
That measures the machine, not the behaviour. The 50ms timeout can fire exactly
as designed and the assertion still fails because the runner was busy. Observed
at 689ms on windows-latest here, and locally at 1818ms under full-suite load
while passing in isolation at 82ms — flagged as a known fragility in SC-A2's PR
description before it became a failure.
Replaced with the property the test exists to defend: a probe fed by a hanging
client must never come back claiming it verified anything. agent.transport,
auth.credential, agent.catalog and mcp.broker are named explicitly rather than
filtered on an axis, because local checks like workspace.git legitimately do
verify in this fixture — nothing about them touches the backend that is hanging.
The first attempt filtered on `reachable !== "na"` and failed on workspace.git
for exactly that reason.
The wall clock is still bounded, but as a hang detector rather than a stopwatch:
30s distinguishes "returned" from "awaited forever", which is the failure the
test was written to catch. A slow runner no longer registers as a bug.
Stronger than what it replaces, not weaker. Mutation-checked: making notChecked
return a verified axis fails this test along with two others (5 pass / 3 fail);
restoring gives 8 / 8. The old assertion would have passed that mutation
untouched — it never looked at a single axis.
Gates at this commit:
npm run typecheck exit 0
npm test 932 pass / 0 fail
This fix is independent of the usage work and would be better as its own PR
against main. It is here because it is what is currently red.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
/limitwas a control that enforced nothing.uvtSpentwas only ever written as= 0— at construction, inpurge(), and on snapshot restore. No usage frame ever incremented it. AndcheckUvtCap()had zero callers anywhere in the tree.So:
/limit 50000printed "agent will pause and ask permission if ceiling hit". Nothing paused. Nothing checked. The cap was never consulted by anything./limitreportedspent: 0and a 0% bar for every session, whatever it had actually cost — because the number was a constant.This is a display-only control presented as enforcement, on the one surface where a user is trying to bound their spend.
Contract
1. Usage is measured, or it is unknown — never zero by default
uvtObserved: number | nullreplaces the always-zero counter.nullmeans no authoritative frame has arrived, which is not a measured zero and is no longer rendered as one. Nothing estimates UVT from token counts; only the server's own number is recorded.uvtSpentsurvives as a getter for the HUD (which needs a number) and is documented as reporting0when the answer is unknown — so anything that must tell those apart readsuvtObserved.2. Turns settle once, by id
The terminal frame carries the turn total and a reconnect can replay it, so
settleTurn(turnId, uvt)is keyed rather than accumulated blindly. A replayeddoneframe is ignored; distinct turns accumulate.3. The cap is checked before a billable turn starts — and says what it is
runCloudTurnconsultscheckUvtCap()before doing anything, and refuses to start once observed spend reaches the cap.Two states deliberately do not trip it:
The wording is now accurate about what it can and cannot do:
The readout stops conflating two different answers:
A design change the mutation pass forced
remainingwas anumber, and an unmeasured session reported the full cap as headroom.That is the same false zero in a different costume: it tells the user their entire budget is intact when in truth none of it has been counted.
I only found it because the first mutation run did not fail any test — reintroducing the false-zero assumption changed nothing observable. Rather than write a test around the weak design, I changed the design:
remainingis nownumber | null, null when unmeasured, and a test pins it.Tests
10 added.
Mutation-checked, both guards. Restoring the false-zero headroom and removing the replay dedupe fails two tests — 8 pass / 2 fail. Restoring gives 10 / 10.
Gates at
79e0bbc:npm run typechecknpm testBaseline on clean
41a7e261, measured in the same session: 922 / 0.Security / correctness notes
Known limits
uvtUsed: reg.uvtSpent, so an unknown session renders as 0 there.hud.tsis SC-INT's surface and is deliberately not touched — this PR fixes the model and/limit; the HUD read is a one-line follow-up in the integration lane.aether agentis not yet gated. The cap is enforced on the REPL cloud path; the one-shot command runs its own loop.type: "usage") are not yet used for live display; only the authoritative terminal frame settles a turn.Dependency and merge order
Touches
src/commands/chat.ts, which SC-A1 (#77) also touches — different functions (runCloudTurnhere,runLocalTurnthere) but the same file, so one will need a trivial rebase. Per the integration order SC-A1 lands first, so this is the one that rebases.Independent of #72, #73, #74, #75.