fix(reset): stop refusing eligible credits on wham applicable_available_count - #133
Conversation
…le_count Remove applicable_available_count from the exhaustion precondition while retaining it for display and confirmation disclosure. Use available credit counts in the confirmation and update reset command, core, and dialog coverage for stale windows and server applicability disagreement.
The markdown confirmation already spends "1 of ${availableCount}"; the
TUI DialogConfirm still interpolated applicableAvailableCount, so the
newly eligible shape (available 1, applicable 0) rendered "Spend 1 of 1"
and "SPENDS 1 of 0" in the same dialog. Both surfaces now read
availableCount. The confirmation fixture carries that exact shape so the
existing "SPENDS 1 of 2" assertion pins it.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/commands.ts">
<violation number="1" location="packages/opencode/src/commands.ts:1315">
P2: When the credit list contains an eligible credit but both available-count fields are omitted, this confirmation still says `Spend 1 of 0`. Preserve the unknown count or render an explicit unknown/at-least-one value instead of defaulting it to zero.</violation>
<violation number="2" location="packages/opencode/src/commands.ts:1320">
P2: When WHAM omits `applicable_available_count`, this condition treats the missing metric as an explicit zero and tells users the server excludes the credit. Keep `undefined` distinct from `0`, and disclose only for an explicitly reported zero.</violation>
</file>
<file name="packages/opencode/src/tests/reset-credits.test.ts">
<violation number="1" location="packages/opencode/src/tests/reset-credits.test.ts:461">
P3: The renamed test 'accepts exhausted quota when the server reports no applicable credits' is now identical to the test above it ('accepts an exhausted live window with an applicable credit'): both call evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now) and expect { ok: true }. Because applicableAvailableCount was removed from the function signature, this test no longer tests anything about applicable credits and only duplicates the prior case. Rename it to reflect what it actually asserts (e.g. 'accepts exhausted quota regardless of applicable credits') or drop it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| `Quota resets: **${row.resetTime ?? 'unavailable'}**`, | ||
| '', | ||
| ] | ||
| if ((row.availableCount ?? 0) > 0 && row.applicableAvailableCount === 0) { |
There was a problem hiding this comment.
P2: When WHAM omits applicable_available_count, this condition treats the missing metric as an explicit zero and tells users the server excludes the credit. Keep undefined distinct from 0, and disclose only for an explicitly reported zero.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/commands.ts, line 1320:
<comment>When WHAM omits `applicable_available_count`, this condition treats the missing metric as an explicit zero and tells users the server excludes the credit. Keep `undefined` distinct from `0`, and disclose only for an explicitly reported zero.</comment>
<file context>
@@ -1313,11 +1312,17 @@ function renderResetConfirm(row: ResetPreviewRow): string {
`Quota resets: **${row.resetTime ?? 'unavailable'}**`,
'',
]
+ if ((row.availableCount ?? 0) > 0 && row.applicableAvailableCount === 0) {
+ lines.push(
+ 'The server does not currently count this credit as applicable; redemption may return a no-op, and a no-op does not spend the credit.',
</file context>
There was a problem hiding this comment.
Valid. The === 0 was right but commands.ts:1240 coerced the omitted field with ?? 0 before it reached the row. 225c437 keeps undefined through to the row, discloses only on an explicit 0, and renders ?/N in the preview line when the field is absent. Test: omitted resetCreditsApplicable → no disclosure, ? marker; re-adding the ?? 0 makes it fail.
| `Account: **${row.label}** (\`${row.accountKey}\`)`, | ||
| `Current quota: **${row.usedPercent ?? 'unknown'}% used**`, | ||
| `Credit: **Spend 1 of ${row.applicableAvailableCount ?? 0}**`, | ||
| `Credit: **Spend 1 of ${row.availableCount ?? 0}**`, |
There was a problem hiding this comment.
P2: When the credit list contains an eligible credit but both available-count fields are omitted, this confirmation still says Spend 1 of 0. Preserve the unknown count or render an explicit unknown/at-least-one value instead of defaulting it to zero.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/commands.ts, line 1315:
<comment>When the credit list contains an eligible credit but both available-count fields are omitted, this confirmation still says `Spend 1 of 0`. Preserve the unknown count or render an explicit unknown/at-least-one value instead of defaulting it to zero.</comment>
<file context>
@@ -1313,11 +1312,17 @@ function renderResetConfirm(row: ResetPreviewRow): string {
`Account: **${row.label}** (\`${row.accountKey}\`)`,
`Current quota: **${row.usedPercent ?? 'unknown'}% used**`,
- `Credit: **Spend 1 of ${row.applicableAvailableCount ?? 0}**`,
+ `Credit: **Spend 1 of ${row.availableCount ?? 0}**`,
`Credit expires: **${row.selectedCreditExpiresAt ?? 'unavailable'}**`,
`Quota resets: **${row.resetTime ?? 'unavailable'}**`,
</file context>
There was a problem hiding this comment.
Valid. 225c437: when both available_count fields are absent, availableCount falls back to the number of credits the eligibility filter accepts (factored into isResetCreditEligible / countEligibleResetCredits so selectCreditToSpend and the count share one definition); if that is also 0 the confirmation renders unknown, never 0. The TUI DialogConfirm reads the same row value, so both surfaces agree. Test: both counts omitted, one eligible credit → Spend 1 of 1; removing the list fallback makes it fail.
| }) | ||
|
|
||
| it('refuses exhausted quota without applicable credits', () => { | ||
| it('accepts exhausted quota when the server reports no applicable credits', () => { |
There was a problem hiding this comment.
P3: The renamed test 'accepts exhausted quota when the server reports no applicable credits' is now identical to the test above it ('accepts an exhausted live window with an applicable credit'): both call evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now) and expect { ok: true }. Because applicableAvailableCount was removed from the function signature, this test no longer tests anything about applicable credits and only duplicates the prior case. Rename it to reflect what it actually asserts (e.g. 'accepts exhausted quota regardless of applicable credits') or drop it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/reset-credits.test.ts, line 461:
<comment>The renamed test 'accepts exhausted quota when the server reports no applicable credits' is now identical to the test above it ('accepts an exhausted live window with an applicable credit'): both call evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now) and expect { ok: true }. Because applicableAvailableCount was removed from the function signature, this test no longer tests anything about applicable credits and only duplicates the prior case. Rename it to reflect what it actually asserts (e.g. 'accepts exhausted quota regardless of applicable credits') or drop it.</comment>
<file context>
@@ -448,20 +448,20 @@ describe('reset redemption precondition', () => {
})
- it('refuses exhausted quota without applicable credits', () => {
+ it('accepts exhausted quota when the server reports no applicable credits', () => {
expect(
- evaluateResetPrecondition({ primary: quotaWindow(100) }, false, 0, now),
</file context>
There was a problem hiding this comment.
Correct, it was a duplicate after the parameter went away. Dropped in 225c437; the property is pinned at the commands level by exhausted preview remains eligible when the server reports no applicable credits, which is the test that goes red if the refusal comes back.
|
Merged as
The TUI catch is yours and I missed it. My ruling on the issue named the confirmation as the place to disclose the disagreement, and I did not trace that One thing I checked because it is the failure mode I have been rejecting all week: Also worth recording: my first attempt at the disclosure mutation did not match the source, so the suite stayed green and briefly looked like a vacuous test. The mutation has to be confirmed to have applied before its result means anything — a substitution that silently matches nothing proves exactly as much as not running it. Gate on Declining the probe was right. The confirmation says "may no-op" and the operator decides with everything we know. |
2 participants
Fixes #131. Implements the ruling in the issue thread: eligibility no longer refuses on wham's
applicable_available_count; the disagreement is disclosed in the confirmation instead.Changes
core/reset-credits.tsevaluateResetPreconditiondrops theapplicableAvailableCount <= 0refusal and its parameter. Exhaustion (resetWindowIsExhausted:usedPercent >= 100andresetsAtin the future, or an active rate-limit mark) is the only precondition.selectCreditToSpendstays the credit-side gate.no_applicable_creditsis gone fromResetRedemptionErrorKindand the reason union; nothing produced it anymore.commands.tsapplicableAvailableCountfor theapplicable/availabledisplay line.Spend 1 of ${availableCount}, plus, whenavailableCount > 0 && applicableAvailableCount === 0, one line: the server does not currently count this credit as applicable; redemption may return a no-op, and a no-op does not spend the credit. "May", not a promise.tui/command-dialogs.tsxDialogConfirminterpolatedapplicableAvailableCountin itsSPENDS 1 of Nline, so the newly eligible shape would have shown "Spend 1 of 1" and "SPENDS 1 of 0" in the same dialog. NowavailableCounton both surfaces.Tests
The two regressions from the thread plus one for the disclosure:
resetsAt,resetCreditsAvailable: 1, resetCreditsApplicable: 0, oneavailableplan-supportedcodex_rate_limitscredit → preview roweligible: true. Reddened by restoring theapplicable <= 0refusal.usedPercent: 100withresetsAtin the past →eligible: false, reasonnot exhausted. Reddened by makingresetWindowIsExhaustedignoreresetsAt.availableCount > 0 && applicable === 0. Reddened by removing the condition.available 2, applicable 0, so the existingSPENDS 1 of 2assertion reddens if the tsx reverts to the applicable count.Existing tests that asserted the old refusal were rewritten to the new property, not deleted.
bun run test: 1122 pass / 0 fail / 45 files (baseline at512e451: 1120). Typecheck and Biome clean. Order-dependence scan clean oncommands.test.ts,reset-credits.test.ts,command-dialogs.test.ts. An independent review re-applied all three mutations and confirmed each goes red.Not done, on purpose
No probe of what the server returns for a consume against a window it considers healthy, per the thread. The confirmation says "may no-op" and stops there.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes #131. Reset eligibility no longer refuses on
wham'sapplicable_available_count; the precondition now checks exhaustion only, and the confirmation discloses when the server reports zero applicable credits.no_applicable_creditserror path and updates tests for the new eligibility rule.Written for commit cc694fb. Summary will update on new commits.