refactor
This commit is contained in:
parent
7cef56de15
commit
ecb5f68e32
GPG key ID:
B97F30FDC4636357
17 changed files with 760 additions and 1626 deletions
|
|
@ -1,163 +1,188 @@
|
|||
# Plan: make local account state authoritative
|
||||
# Plan: rewrite token selection around a simple disk-first state model
|
||||
|
||||
## Problem
|
||||
## Goal
|
||||
|
||||
Current `accounts.json` state is not acting like a real source of truth for `/token`:
|
||||
Throw away the current layered selection/cooldown/state model and replace it with a small implementation that:
|
||||
|
||||
- `active_account_id` is persisted but not actually used to drive selection
|
||||
- `last_known_usage` is mostly treated as a sort hint, then immediately replaced by a live refresh
|
||||
- `/token` live-refreshes every candidate it tries, so local limits are not truly trusted
|
||||
- the persisted snapshot contains duplicate derived fields (`used_percent`, `remaining_percent`, `exhausted`) in addition to canonical window and flag data
|
||||
- as a result, the file is hard to reason about: some fields look authoritative but are only decorative or transient
|
||||
- reads the main JSON file on every `/token` request
|
||||
- keeps only the minimum necessary account fields on disk
|
||||
- decides from file state first
|
||||
- refreshes usage only when missing or stale
|
||||
- validates the selected token before returning it
|
||||
- moves invalid accounts to `failed.json`
|
||||
- does not touch the helper scripts in this pass
|
||||
|
||||
## Desired behavior
|
||||
## Required file model
|
||||
|
||||
1. `accounts.json` is the source of truth for `/token` selection.
|
||||
2. If `active_account_id` exists and local state says the active account is usable, `/token` must try that account first.
|
||||
3. Only if local state says the active account is blocked or unusable should `/token` fall back to choosing another account.
|
||||
4. Fallback selection can keep the current ranking approach: most-used eligible account first.
|
||||
5. `/usage` should continue to refresh all accounts live.
|
||||
6. Persisted usage snapshots should store canonical data only, without duplicated derived fields.
|
||||
### Main state file
|
||||
|
||||
## Recommended approach
|
||||
`accounts.json`
|
||||
|
||||
### 1. Split `/token` selection into local selection and live validation
|
||||
```json
|
||||
{
|
||||
"active_account": "user@example.com",
|
||||
"accounts": [
|
||||
{
|
||||
"email": "user@example.com",
|
||||
"access_token": "...",
|
||||
"refresh_token": "...",
|
||||
"token_refresh_at": 1710000000,
|
||||
"usage": {
|
||||
"primary": {
|
||||
"used_percent": 72,
|
||||
"reset_at": 1710018000
|
||||
},
|
||||
"secondary": {
|
||||
"used_percent": 18,
|
||||
"reset_at": 1710600000
|
||||
}
|
||||
},
|
||||
"usage_checked_at": 1710000000,
|
||||
"disabled": false
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
In `src/gibby/manager.py`:
|
||||
Only these fields should exist for account state.
|
||||
|
||||
- Add `_find_active_account(state)` to resolve `state.active_account_id` to an `AccountRecord`
|
||||
- Add `_is_locally_blocked(account, current)` to decide from local file state only whether an account can be tried
|
||||
- Add `_build_selection_order(state, current)` that:
|
||||
- returns the active account first if it exists and is not locally blocked
|
||||
- otherwise falls back to the remaining eligible accounts sorted by current saved-usage ranking
|
||||
- never duplicates the active account in the fallback list
|
||||
### Failed state file
|
||||
|
||||
`_is_locally_blocked()` should use only persisted local state:
|
||||
`failed.json`
|
||||
|
||||
- blocked if `cooldown_until > now`
|
||||
- blocked if `last_known_usage` exists and local usage indicates exhaustion
|
||||
- otherwise not blocked
|
||||
```json
|
||||
{
|
||||
"accounts": [
|
||||
{
|
||||
"email": "bad@example.com",
|
||||
"access_token": "...",
|
||||
"refresh_token": "...",
|
||||
"token_refresh_at": 1710000000,
|
||||
"usage": {
|
||||
"primary": {
|
||||
"used_percent": 100,
|
||||
"reset_at": 1710018000
|
||||
},
|
||||
"secondary": {
|
||||
"used_percent": 100,
|
||||
"reset_at": 1710600000
|
||||
}
|
||||
},
|
||||
"usage_checked_at": 1710000000,
|
||||
"disabled": false
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
This gives the exact behavior the user requested:
|
||||
Top-level must contain only `accounts`.
|
||||
|
||||
- active account is mandatory first choice when nothing local blocks it
|
||||
- local file decides whether active is allowed before any network call
|
||||
- live refresh remains only a validation step for the chosen candidate
|
||||
## Selection rules
|
||||
|
||||
### 2. Keep live refresh only for accounts actually attempted
|
||||
### Active account first
|
||||
|
||||
In `src/gibby/manager.py`:
|
||||
For each `/token` request:
|
||||
|
||||
- keep the current live refresh path (`refresh_account_usage`) once an account has been selected for an attempt
|
||||
- if active account passes local checks but fails live validation, persist the updated state and continue to the next candidate
|
||||
- if active account is locally blocked, skip live refresh for it during that `/token` call
|
||||
1. Read `accounts.json` fresh from disk.
|
||||
2. Resolve `active_account` by email.
|
||||
3. Evaluate active first.
|
||||
|
||||
`/usage` stays as-is and continues refreshing all accounts live.
|
||||
### When an account is usable
|
||||
|
||||
### 3. Clean up the persisted usage snapshot schema
|
||||
An account is usable when:
|
||||
|
||||
In `src/gibby/models.py` and `src/gibby/store.py`:
|
||||
- `disabled == false`
|
||||
- `secondary.used_percent < 100`
|
||||
- `primary.used_percent < GIBBY_EXHAUSTED_USAGE_THRESHOLD`
|
||||
|
||||
- stop persisting derived snapshot fields:
|
||||
- `used_percent`
|
||||
- `remaining_percent`
|
||||
- `exhausted`
|
||||
- keep only canonical persisted snapshot data:
|
||||
- `checked_at`
|
||||
- `primary_window`
|
||||
- `secondary_window`
|
||||
- `limit_reached`
|
||||
- `allowed`
|
||||
Default threshold remains `95`.
|
||||
|
||||
Implementation direction:
|
||||
### Usage freshness
|
||||
|
||||
- keep `UsageSnapshot` as the in-memory model for now, but derive:
|
||||
- `used_percent`
|
||||
- `remaining_percent`
|
||||
- `exhausted`
|
||||
from canonical fields when loading/parsing
|
||||
- update `store._snapshot_to_dict()` to write only canonical fields
|
||||
- update `store._snapshot_from_dict()` to reconstruct the full in-memory `UsageSnapshot` from canonical persisted fields
|
||||
Usage must be refreshed only when missing or stale.
|
||||
|
||||
This keeps code churn smaller than a full model rewrite while making the file itself cleaner and more honest.
|
||||
Add env:
|
||||
|
||||
### 4. Keep cooldown as the persisted local block, but make local exhaustion matter too
|
||||
- `GIBBY_USAGE_STALE_SECONDS`, default `3600`
|
||||
|
||||
Local selection should not depend on a fresh API round-trip.
|
||||
Usage is stale when:
|
||||
|
||||
For `/token`:
|
||||
- `usage` is missing
|
||||
- `usage_checked_at` is missing
|
||||
- `now - usage_checked_at > GIBBY_USAGE_STALE_SECONDS`
|
||||
|
||||
- `cooldown_until` remains the strongest persisted block
|
||||
- if cooldown is clear but local `last_known_usage` still says exhausted, treat the account as locally blocked too
|
||||
- only accounts that pass local checks are eligible to be attempted live
|
||||
If active account usage is stale or missing, refresh usage for that account before deciding if it is usable.
|
||||
|
||||
This changes current behavior in an important way:
|
||||
### Fallback selection
|
||||
|
||||
- today, an account with expired or missing cooldown can still be live-refreshed even if local snapshot says exhausted
|
||||
- after the change, local state truly gates the initial decision
|
||||
If active account cannot be used, choose the next account by:
|
||||
|
||||
### 5. Preserve existing fallback ranking for non-active accounts
|
||||
- filtering to usable accounts
|
||||
- sorting by highest primary `used_percent`
|
||||
- using file order as the tie-breaker
|
||||
|
||||
After active account is rejected locally, keep the current fallback sort in `manager.py`:
|
||||
If a new account is chosen, write its email into `active_account` in `accounts.json`.
|
||||
|
||||
- primary window used percent descending
|
||||
- secondary window used percent descending
|
||||
## Token flow
|
||||
|
||||
That avoids a larger policy change in this pass and isolates the refactor to "trust local state first".
|
||||
For the chosen account:
|
||||
|
||||
## Files to modify
|
||||
1. Ensure token is fresh enough.
|
||||
2. If `token_refresh_at` says refresh is needed, refresh token and persist new values.
|
||||
3. After selection decisions are finished and the token is ready, validate it by calling:
|
||||
|
||||
- `/home/wzray/AI/gibby/src/gibby/manager.py`
|
||||
- respect `active_account_id`
|
||||
- add local-only eligibility predicate
|
||||
- change selection order to active-first-when-locally-usable
|
||||
- `/home/wzray/AI/gibby/src/gibby/models.py`
|
||||
- keep canonical usage derivation helpers centralized
|
||||
- support reconstructing derived values from canonical fields
|
||||
`https://chatgpt.com/backend-api/codex/models`
|
||||
|
||||
4. Only return the token if validation returns `200`.
|
||||
|
||||
## Invalid account handling
|
||||
|
||||
If refresh, usage auth, or final validation shows the token/account is invalid:
|
||||
|
||||
1. Read current main state.
|
||||
2. Remove that full account object from `accounts.json`.
|
||||
3. Append the same full account object to `failed.json.accounts`.
|
||||
4. If it was the active account, clear `active_account` before reselection.
|
||||
5. Persist both files atomically.
|
||||
|
||||
No `failed.txt` in the rewritten core flow.
|
||||
|
||||
## Files to rewrite
|
||||
|
||||
- `/home/wzray/AI/gibby/src/gibby/settings.py`
|
||||
- keep only env needed for the new flow
|
||||
- `/home/wzray/AI/gibby/src/gibby/store.py`
|
||||
- write canonical snapshot shape only
|
||||
- read canonical snapshot shape into full in-memory model
|
||||
- rewrite as simple JSON read/write helpers for `accounts.json` and `failed.json`
|
||||
- `/home/wzray/AI/gibby/src/gibby/client.py`
|
||||
- keep only token refresh, usage fetch, and token validation calls
|
||||
- `/home/wzray/AI/gibby/src/gibby/manager.py`
|
||||
- rewrite into one small service for `/token`
|
||||
- `/home/wzray/AI/gibby/src/gibby/app.py`
|
||||
- keep thin FastAPI wiring for `/health` and `/token`
|
||||
|
||||
## Files to remove or stop using
|
||||
|
||||
- `/home/wzray/AI/gibby/src/gibby/models.py`
|
||||
- `/home/wzray/AI/gibby/src/gibby/account_ops.py`
|
||||
- keep refresh path aligned with canonical snapshot handling
|
||||
- reuse a local exhaustion predicate if helpful instead of duplicating logic
|
||||
- `/home/wzray/AI/gibby/tests/test_core.py`
|
||||
- add and update selection behavior tests
|
||||
- `/home/wzray/AI/gibby/tests/test_account_ops.py`
|
||||
- update snapshot persistence assumptions if needed
|
||||
- `/home/wzray/AI/gibby/tests/test_app.py`
|
||||
- adjust fixture shapes only if response expectations change
|
||||
- `/home/wzray/AI/gibby/tests/test_refresh_limits.py`
|
||||
- ensure live refresh still rewrites canonical local state correctly
|
||||
- `/home/wzray/AI/gibby/tests/test_oauth_helper.py`
|
||||
- ensure oauth helper stores canonical snapshot shape correctly
|
||||
|
||||
## Test plan
|
||||
Their logic should be folded into the new minimal data model and service flow instead of preserved.
|
||||
|
||||
### Selection behavior
|
||||
## Out of scope for this pass
|
||||
|
||||
Add or update tests in `tests/test_core.py` for:
|
||||
|
||||
- active account is used first when locally allowed, even if another account has higher saved usage
|
||||
- active account is skipped without live refresh when `cooldown_until` is still active
|
||||
- active account is skipped without live refresh when local snapshot says exhausted
|
||||
- active account passes local checks but fails live refresh, then fallback account is tried
|
||||
- missing or stale `active_account_id` falls back cleanly to non-active selection logic
|
||||
- fallback ordering still prefers higher saved primary usage and uses secondary as tie-breaker
|
||||
|
||||
### Snapshot/file behavior
|
||||
|
||||
Add or update tests to verify:
|
||||
|
||||
- `accounts.json` no longer writes `used_percent`, `remaining_percent`, or `exhausted`
|
||||
- loading canonical persisted snapshots still reconstructs full in-memory `UsageSnapshot`
|
||||
- `/usage`, `refresh_limits.py`, and `oauth_helper.py` still persist refreshed canonical state correctly
|
||||
- do not touch `scripts/oauth_helper.py`
|
||||
- do not touch `scripts/refresh_limits.py`
|
||||
- do not preserve old cooldown, failed.txt, dual-state, or derived snapshot machinery unless absolutely required to keep app booting during rewrite
|
||||
|
||||
## Verification
|
||||
|
||||
- `uv run pytest -q tests/test_core.py tests/test_account_ops.py tests/test_app.py tests/test_refresh_limits.py tests/test_oauth_helper.py`
|
||||
- inspect a real `accounts.json` after `/usage` or `refresh_limits.py` and confirm snapshot entries contain only canonical fields
|
||||
- manually test `/token` with:
|
||||
- a locally usable active account
|
||||
- a locally blocked active account
|
||||
- a dangling `active_account_id`
|
||||
- verify that active account is not live-refreshed when local file state already blocks it
|
||||
- `uv run pytest -q`
|
||||
- API tests for:
|
||||
- `/health` returns `ok`
|
||||
- `/token` returns `503` when file has no usable accounts
|
||||
- `/token` prefers active account when usable
|
||||
- `/token` rereads the file between requests
|
||||
- stale usage triggers a refresh before decision
|
||||
- fresh usage skips refresh
|
||||
- invalid token moves full account object to `failed.json`
|
||||
- fallback chooses highest primary usage among usable non-disabled accounts
|
||||
- direct file tests for exact `accounts.json` and `failed.json` schema
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue