mirror of
https://github.com/Wei-Shaw/sub2api.git
synced 2026-10-14 12:02:27 +08:00
fix(claude): tighten reset redeem outcome handling (review R1)
- R1-1: an explicit upstream "unavailable" answer now fences the organization for 15 minutes (CLAUDE_RESET_UPSTREAM_UNAVAILABLE) instead of 24h; ambiguous outcomes keep the 24h fence. Frontend tells the operator to retry later. - R1-2: rotate the idempotency key after definite pre-claim refusals (busy, not available, unresolved, upstream unavailable); map IDEMPOTENCY_IN_PROGRESS and IDEMPOTENCY_RETRY_BACKOFF to clear zh/en messages. - R1-3: redeem reason and cleared windows pass only through explicit allowlists, so upstream values can never echo a grant ID to clients. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
1872d44b58
commit
dc2efc6b9a
@@ -11,7 +11,6 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -30,6 +29,10 @@ const (
|
||||
// the upstream outcome has certainly settled; after that a fresh query is
|
||||
// authoritative again (a consumed credit shows up as a lower count or cooldown).
|
||||
claudeResetUnknownFenceTTL = 24 * time.Hour
|
||||
// An explicit, well-formed "unavailable" answer means nothing was claimed, so it
|
||||
// only fences briefly instead of locking the organization out for a day.
|
||||
claudeResetUnavailableFenceTTL = 15 * time.Minute
|
||||
claudeResetReasonUnavailable = "upstream_unavailable"
|
||||
|
||||
ClaudeResetOutcomeReset = "reset"
|
||||
ClaudeResetOutcomeAlreadyUsed = "already_used"
|
||||
@@ -39,7 +42,17 @@ const (
|
||||
ClaudeResetOutcomeUnknown = "unknown"
|
||||
)
|
||||
|
||||
var claudeResetReasonPattern = regexp.MustCompile(`^[a-z0-9_]{1,64}$`)
|
||||
// Only known reason codes and window names reach clients, so an upstream value can
|
||||
// never echo a grant ID or other identifier.
|
||||
var (
|
||||
claudeResetKnownReasons = map[string]bool{
|
||||
"no_grant": true, "unknown_grant": true, "not_next_grant": true, "grant_id_required": true,
|
||||
"tenure": true, "other_experiment": true, "stamp_indeterminate": true, "reset_unconfirmed": true,
|
||||
"authorization_rejected": true, "claim_unconfirmed": true, claudeResetReasonUnavailable: true,
|
||||
"result_persistence_failed": true,
|
||||
}
|
||||
claudeResetKnownWindows = map[string]bool{"five_hour": true, "seven_day": true, "seven_day_overage_included": true}
|
||||
)
|
||||
|
||||
// ClaudeResetOutcome is the sanitized redemption result. It never carries grant,
|
||||
// organization, or upstream request IDs.
|
||||
@@ -157,7 +170,11 @@ func (s *ClaudeResetCreditService) redeemOnce(ctx context.Context, id int64, ope
|
||||
// A crashed or interrupted attempt of this same confirmation: never resend.
|
||||
return &ClaudeResetOutcome{Outcome: prior.Outcome, Reason: prior.Reason, Replayed: true}, nil
|
||||
}
|
||||
if prior.Outcome == ClaudeResetOutcomeUnknown && s.now().Before(prior.At.Add(claudeResetUnknownFenceTTL)) {
|
||||
if prior.Outcome == ClaudeResetOutcomeUnknown && prior.Reason == claudeResetReasonUnavailable {
|
||||
if s.now().Before(prior.At.Add(claudeResetUnavailableFenceTTL)) {
|
||||
return nil, infraerrors.Conflict("CLAUDE_RESET_UPSTREAM_UNAVAILABLE", "reset service was unavailable; retry after a while")
|
||||
}
|
||||
} else if prior.Outcome == ClaudeResetOutcomeUnknown && s.now().Before(prior.At.Add(claudeResetUnknownFenceTTL)) {
|
||||
return nil, infraerrors.Conflict("CLAUDE_RESET_UNRESOLVED", "previous reset outcome is unconfirmed; redemption is blocked for now")
|
||||
}
|
||||
}
|
||||
@@ -295,7 +312,7 @@ func (s *ClaudeResetCreditService) claim(ctx context.Context, token, proxy, org,
|
||||
return unknown
|
||||
}
|
||||
reason := ""
|
||||
if claudeResetReasonPattern.MatchString(result.Reason) {
|
||||
if claudeResetKnownReasons[result.Reason] {
|
||||
reason = result.Reason
|
||||
}
|
||||
if reason == "stamp_indeterminate" || reason == "reset_unconfirmed" {
|
||||
@@ -305,16 +322,13 @@ func (s *ClaudeResetCreditService) claim(ctx context.Context, token, proxy, org,
|
||||
case ClaudeResetOutcomeReset, ClaudeResetOutcomeAlreadyUsed, ClaudeResetOutcomeNotLimited, ClaudeResetOutcomeCooldown, ClaudeResetOutcomeIneligible:
|
||||
out := &ClaudeResetOutcome{Outcome: result.Result, Reason: reason, CooldownUntil: result.CooldownUntil}
|
||||
for _, w := range result.Cleared {
|
||||
if claudeResetReasonPattern.MatchString(w) {
|
||||
if claudeResetKnownWindows[w] {
|
||||
out.Cleared = append(out.Cleared, w)
|
||||
}
|
||||
}
|
||||
return out
|
||||
case "unavailable":
|
||||
if reason == "" {
|
||||
reason = "upstream_unavailable"
|
||||
}
|
||||
return &ClaudeResetOutcome{Outcome: ClaudeResetOutcomeUnknown, Reason: reason}
|
||||
return &ClaudeResetOutcome{Outcome: ClaudeResetOutcomeUnknown, Reason: claudeResetReasonUnavailable}
|
||||
default:
|
||||
return unknown
|
||||
}
|
||||
|
||||
@@ -227,7 +227,7 @@ func TestClaudeResetRedeemDuplicateOrgAccountsConcurrentOnlyOnePost(t *testing.T
|
||||
}
|
||||
|
||||
func TestClaudeResetRedeemUnknownOutcomeFencesOrganization(t *testing.T) {
|
||||
for _, claim := range []string{"network-error", `{broken`, `{"result":"weird"}`, `{"result":"unavailable"}`, `{"result":"reset","reason":"reset_unconfirmed"}`, "http-500"} {
|
||||
for _, claim := range []string{"network-error", `{broken`, `{"result":"weird"}`, `{"result":"unavailable","reason":"stamp_indeterminate"}`, `{"result":"reset","reason":"reset_unconfirmed"}`, "http-500"} {
|
||||
t.Run(claim, func(t *testing.T) {
|
||||
f := &redeemFake{claim: claim}
|
||||
if claim == "http-500" {
|
||||
@@ -256,6 +256,11 @@ func TestClaudeResetRedeemUnknownOutcomeFencesOrganization(t *testing.T) {
|
||||
}
|
||||
require.Equal(t, 1, f.postCount())
|
||||
|
||||
// Still blocked past the short unavailable fence.
|
||||
s.now = func() time.Time { return time.Now().Add(claudeResetUnavailableFenceTTL + time.Minute) }
|
||||
_, err = s.Redeem(context.Background(), 1, "op-new")
|
||||
require.Equal(t, "CLAUDE_RESET_UNRESOLVED", infraerrors.Reason(err))
|
||||
|
||||
// Once the fence has settled, a fresh query is authoritative again.
|
||||
s.now = func() time.Time { return time.Now().Add(claudeResetUnknownFenceTTL + time.Minute) }
|
||||
f.claim, f.claimHTTP = `{"result":"reset"}`, 0
|
||||
@@ -292,7 +297,7 @@ func TestClaudeResetRedeemMapsUpstreamResults(t *testing.T) {
|
||||
{`{"result":"already_used"}`, ClaudeResetOutcomeAlreadyUsed, 0},
|
||||
{`{"result":"not_limited"}`, ClaudeResetOutcomeNotLimited, 0},
|
||||
{`{"result":"cooldown","cooldown_until":"2099-01-01T00:00:00Z"}`, ClaudeResetOutcomeCooldown, 0},
|
||||
{`{"result":"ineligible","reason":"plan_changed"}`, ClaudeResetOutcomeIneligible, 0},
|
||||
{`{"result":"ineligible","reason":"tenure"}`, ClaudeResetOutcomeIneligible, 0},
|
||||
{`{"result":"unavailable","reason":"stamp_indeterminate"}`, ClaudeResetOutcomeUnknown, 0},
|
||||
{`{}`, ClaudeResetOutcomeIneligible, http.StatusForbidden},
|
||||
}
|
||||
@@ -307,7 +312,7 @@ func TestClaudeResetRedeemMapsUpstreamResults(t *testing.T) {
|
||||
require.NotNil(t, out.CooldownUntil)
|
||||
}
|
||||
if tc.outcome == ClaudeResetOutcomeIneligible && tc.http == 0 {
|
||||
require.Equal(t, "plan_changed", out.Reason)
|
||||
require.Equal(t, "tenure", out.Reason)
|
||||
}
|
||||
// Definite outcomes never block a later confirmation.
|
||||
if tc.outcome != ClaudeResetOutcomeUnknown {
|
||||
@@ -327,3 +332,37 @@ func TestClaudeResetRedeemSanitizesUpstreamReason(t *testing.T) {
|
||||
require.Empty(t, out.Reason)
|
||||
require.Equal(t, []string{"five_hour"}, out.Cleared)
|
||||
}
|
||||
|
||||
func TestClaudeResetRedeemExplicitUnavailableFencesBriefly(t *testing.T) {
|
||||
f := &redeemFake{claim: `{"result":"unavailable","reason":"grant_next"}`}
|
||||
s, _, _ := newRedeemService(t, f)
|
||||
out, err := s.Redeem(context.Background(), 1, "op-1")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, ClaudeResetOutcomeUnknown, out.Outcome)
|
||||
require.Equal(t, claudeResetReasonUnavailable, out.Reason)
|
||||
|
||||
s.locks = &redeemLeaseStub{}
|
||||
_, err = s.Redeem(context.Background(), 2, "op-new")
|
||||
require.Equal(t, "CLAUDE_RESET_UPSTREAM_UNAVAILABLE", infraerrors.Reason(err))
|
||||
require.Equal(t, 1, f.postCount())
|
||||
|
||||
s.now = func() time.Time { return time.Now().Add(claudeResetUnavailableFenceTTL + time.Minute) }
|
||||
f.claim = `{"result":"reset"}`
|
||||
out, err = s.Redeem(context.Background(), 1, "op-later")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, ClaudeResetOutcomeReset, out.Outcome)
|
||||
require.Equal(t, 2, f.postCount())
|
||||
}
|
||||
|
||||
func TestClaudeResetRedeemNeverEchoesGrantIDs(t *testing.T) {
|
||||
f := &redeemFake{claim: `{"result":"reset","reason":"grant_next","cleared":["five_hour","grant_next","launch","seven_day_overage_included"]}`}
|
||||
s, _, _ := newRedeemService(t, f)
|
||||
out, err := s.Redeem(context.Background(), 1, "op-1")
|
||||
require.NoError(t, err)
|
||||
raw, err := json.Marshal(out)
|
||||
require.NoError(t, err)
|
||||
require.NotContains(t, string(raw), "grant_next")
|
||||
require.NotContains(t, string(raw), "launch")
|
||||
require.Empty(t, out.Reason)
|
||||
require.Equal(t, []string{"five_hour", "seven_day_overage_included"}, out.Cleared)
|
||||
}
|
||||
|
||||
@@ -257,16 +257,23 @@ function outcomeFeedback(result: ClaudeResetOutcome): { kind: 'success' | 'warni
|
||||
case 'ineligible':
|
||||
return { kind: 'error', text: t(`${key}.ineligible`) }
|
||||
default:
|
||||
if (result.reason === 'upstream_unavailable') return { kind: 'warning', text: t(`${key}.unavailable`) }
|
||||
return { kind: 'warning', text: t(`${key}.unknown`) }
|
||||
}
|
||||
}
|
||||
|
||||
// Refusals issued before any claim was sent: the next attempt is a new confirmation.
|
||||
const preClaimRefusals = new Set(['CLAUDE_RESET_BUSY', 'CLAUDE_RESET_NOT_AVAILABLE', 'CLAUDE_RESET_UNRESOLVED', 'CLAUDE_RESET_UPSTREAM_UNAVAILABLE'])
|
||||
|
||||
function errorText(e: unknown): string {
|
||||
const err = e as { reason?: string; message?: string }
|
||||
switch (err?.reason) {
|
||||
case 'CLAUDE_RESET_UNRESOLVED': return t('admin.accounts.claudeResetCredits.outcome.unknown')
|
||||
case 'CLAUDE_RESET_BUSY': return t('admin.accounts.claudeResetCredits.outcome.busy')
|
||||
case 'CLAUDE_RESET_NOT_AVAILABLE': return t('admin.accounts.claudeResetCredits.outcome.notAvailable')
|
||||
case 'CLAUDE_RESET_UPSTREAM_UNAVAILABLE': return t('admin.accounts.claudeResetCredits.outcome.unavailable')
|
||||
case 'IDEMPOTENCY_IN_PROGRESS': return t('admin.accounts.claudeResetCredits.outcome.inProgress')
|
||||
case 'IDEMPOTENCY_RETRY_BACKOFF': return t('admin.accounts.claudeResetCredits.outcome.retryBackoff')
|
||||
default: return err?.message || t('admin.accounts.claudeResetCredits.outcome.failed')
|
||||
}
|
||||
}
|
||||
@@ -290,6 +297,8 @@ async function confirmRedeem() {
|
||||
await refresh()
|
||||
} catch (e) {
|
||||
if (current !== generation) return
|
||||
// Transport or unknown errors keep the key so a retry replays server-side.
|
||||
if (preClaimRefusals.has((e as { reason?: string })?.reason ?? '')) pendingKey = null
|
||||
redeemFeedback.value = { kind: 'error', text: errorText(e) }
|
||||
} finally {
|
||||
if (current === generation) redeeming.value = false
|
||||
|
||||
@@ -161,6 +161,45 @@ describe('Claude reset credit status', () => {
|
||||
expect(wrapper.get('[data-testid="claude-reset-feedback"]').text()).toBe('admin.accounts.claudeResetCredits.outcome.busy')
|
||||
})
|
||||
|
||||
it.each(['CLAUDE_RESET_BUSY', 'CLAUDE_RESET_NOT_AVAILABLE', 'CLAUDE_RESET_UNRESOLVED', 'CLAUDE_RESET_UPSTREAM_UNAVAILABLE'])(
|
||||
'rotates the key after pre-claim refusal %s',
|
||||
async reason => {
|
||||
const wrapper = await queried()
|
||||
redeem.mockRejectedValueOnce({ status: 409, reason, message: 'x' })
|
||||
await confirmReset(wrapper)
|
||||
redeem.mockResolvedValueOnce({ outcome: 'not_limited', replayed: false })
|
||||
await confirmReset(wrapper)
|
||||
expect(redeem.mock.calls[1][1]).not.toBe(redeem.mock.calls[0][1])
|
||||
}
|
||||
)
|
||||
|
||||
it.each([
|
||||
['IDEMPOTENCY_IN_PROGRESS', 'inProgress'],
|
||||
['IDEMPOTENCY_RETRY_BACKOFF', 'retryBackoff'],
|
||||
['CLAUDE_RESET_UPSTREAM_UNAVAILABLE', 'unavailable']
|
||||
])('maps %s to a clear message', async (reason, key) => {
|
||||
const wrapper = await queried()
|
||||
redeem.mockRejectedValueOnce({ status: 409, reason, message: 'x' })
|
||||
await confirmReset(wrapper)
|
||||
expect(wrapper.get('[data-testid="claude-reset-feedback"]').text()).toBe(`admin.accounts.claudeResetCredits.outcome.${key}`)
|
||||
})
|
||||
|
||||
it('reuses the key after an idempotency conflict', async () => {
|
||||
const wrapper = await queried()
|
||||
redeem.mockRejectedValueOnce({ status: 409, reason: 'IDEMPOTENCY_IN_PROGRESS', message: 'x' })
|
||||
await confirmReset(wrapper)
|
||||
redeem.mockResolvedValueOnce({ outcome: 'not_limited', replayed: false })
|
||||
await confirmReset(wrapper)
|
||||
expect(redeem.mock.calls[1][1]).toBe(redeem.mock.calls[0][1])
|
||||
})
|
||||
|
||||
it('tells the operator to retry later after an explicit unavailable answer', async () => {
|
||||
const wrapper = await queried()
|
||||
redeem.mockResolvedValue({ outcome: 'unknown', reason: 'upstream_unavailable', replayed: false })
|
||||
await confirmReset(wrapper)
|
||||
expect(wrapper.get('[data-testid="claude-reset-feedback"]').text()).toBe('admin.accounts.claudeResetCredits.outcome.unavailable')
|
||||
})
|
||||
|
||||
it('keeps slot content for setup tokens and discards responses after account changes', async () => {
|
||||
let resolve!: (value: typeof snapshot) => void
|
||||
getCredits.mockReturnValue(new Promise(r => { resolve = r }))
|
||||
|
||||
@@ -953,6 +953,9 @@ export default {
|
||||
notLimited: 'Not at a limit, so nothing was reset and no credit was used',
|
||||
ineligible: 'This account cannot use resets right now',
|
||||
unknown: 'Result unconfirmed; further redemption is blocked for now. Check again later',
|
||||
unavailable: 'Reset service is temporarily unavailable; retry after a while',
|
||||
inProgress: 'This reset request is still processing; check again shortly',
|
||||
retryBackoff: 'This reset request just failed; retry after a moment',
|
||||
busy: 'Another reset is in progress; try again later',
|
||||
notAvailable: 'No reset can be used right now; no credit was used',
|
||||
failed: 'Reset request failed'
|
||||
|
||||
@@ -1054,6 +1054,9 @@ export default {
|
||||
notLimited: '当前未达到限额,无需重置,未消耗次数',
|
||||
ineligible: '此账号当前不可使用重置',
|
||||
unknown: '结果未确认,已阻止再次兑换,请稍后查询',
|
||||
unavailable: '重置服务暂时不可用,未确认消耗,请稍后再试',
|
||||
inProgress: '该重置请求仍在处理中,请稍后查询结果',
|
||||
retryBackoff: '该重置请求刚刚失败,请稍后再试',
|
||||
busy: '另一个重置正在进行中,请稍后再试',
|
||||
notAvailable: '当前没有可立即使用的重置,未消耗次数',
|
||||
failed: '重置请求失败'
|
||||
|
||||
Reference in New Issue
Block a user