|
| 1 | +# 010 — Phase 1: main-account DTO reads the merged quota store |
| 2 | + |
| 3 | +Work phase: `wp1`. Depends on: nothing. Consumed by: `020`. |
| 4 | + |
| 5 | +## Goal |
| 6 | + |
| 7 | +The main account's DTO quota must be the same merged store object a pool |
| 8 | +account's DTO quota is, so every field the store carries forward (today |
| 9 | +`resetCredits`, tomorrow anything else) reaches the dashboard. |
| 10 | + |
| 11 | +## Scope boundary |
| 12 | + |
| 13 | +IN: `src/codex/auth-api.ts` main DTO construction; a focused test in |
| 14 | +`tests/codex-auth-api.test.ts`. |
| 15 | +OUT: `src/codex/quota.ts` merge semantics (already correct), the pool path, |
| 16 | +any WHAM fetch/refresh policy, credential handling. |
| 17 | + |
| 18 | +## File change map |
| 19 | + |
| 20 | +### `src/codex/auth-api.ts` — `listCodexAuthAccountsSnapshot`, main DTO (~line 1642) |
| 21 | + |
| 22 | +Before: |
| 23 | + |
| 24 | +```ts |
| 25 | +quota: mainInfo.quota ? { |
| 26 | + ...quotaForPlan({ |
| 27 | + ...mainInfo.quota, |
| 28 | + updatedAt: getAccountQuota(MAIN_CODEX_ACCOUNT_ID)?.updatedAt ?? Date.now(), |
| 29 | + }, mainInfo.plan), |
| 30 | +} : null, |
| 31 | +``` |
| 32 | + |
| 33 | +After: |
| 34 | + |
| 35 | +```ts |
| 36 | +quota: mainInfo.quota ? { |
| 37 | + ...quotaForPlan(mergeMainQuotaWithStore(mainInfo.quota), mainInfo.plan), |
| 38 | +} : null, |
| 39 | +``` |
| 40 | + |
| 41 | +with a small local helper next to the DTO builders: |
| 42 | + |
| 43 | +```ts |
| 44 | +/** |
| 45 | + * The main account is the only account whose DTO quota came from the raw parse |
| 46 | + * result rather than the merged store, so a resetCredits the store had carried |
| 47 | + * forward vanished from the response whenever the current /wham/usage payload |
| 48 | + * omitted `rate_limit_reset_credits`. Pool DTOs never had that hole because |
| 49 | + * commitPoolQuotaResponse re-reads getAccountQuota() after committing. |
| 50 | + * |
| 51 | + * Only resetCredits is filled from the store, deliberately. The window fields |
| 52 | + * have *clearing* semantics -- a monthly-only snapshot must drop a stale weekly |
| 53 | + * value (#382) -- so a blanket spread of the stored object would resurrect a |
| 54 | + * window the parse intended to clear whenever the store write was refused by |
| 55 | + * generation gating. resetCredits is the one field setAccountQuotaFromParsed |
| 56 | + * itself carries forward (quota.ts:339-340), so mirroring exactly that rule |
| 57 | + * here keeps the DTO consistent with the store instead of inventing a second, |
| 58 | + * looser merge policy. |
| 59 | + */ |
| 60 | +function mainQuotaWithStoredResetCredits( |
| 61 | + parsed: Omit<StoredAccountQuota, "updatedAt">, |
| 62 | +): StoredAccountQuota { |
| 63 | + const stored = getAccountQuota(MAIN_CODEX_ACCOUNT_ID); |
| 64 | + return { |
| 65 | + ...parsed, |
| 66 | + ...(parsed.resetCredits === undefined && stored?.resetCredits !== undefined |
| 67 | + ? { resetCredits: stored.resetCredits } |
| 68 | + : {}), |
| 69 | + updatedAt: stored?.updatedAt ?? Date.now(), |
| 70 | + }; |
| 71 | +} |
| 72 | +``` |
| 73 | + |
| 74 | +Call site becomes `quotaForPlan(mainQuotaWithStoredResetCredits(mainInfo.quota), mainInfo.plan)`. |
| 75 | + |
| 76 | +Precedence rationale: a freshly parsed `resetCredits` always wins, including a |
| 77 | +deliberate `0` (0 is defined, so it is present in `parsed` and the fill branch |
| 78 | +does not run). The store supplies the value only when the parse omitted the key |
| 79 | +entirely. Every other field is untouched, so no window-clearing behaviour |
| 80 | +changes. |
| 81 | + |
| 82 | +### Audit finding folded in (blocker 1) |
| 83 | + |
| 84 | +The first draft of this document proposed `{ ...stored, ...parsed }`. That is |
| 85 | +unsafe: `setAccountQuotaFromParsed` refuses to commit when |
| 86 | +`mayCommitAccountQuota` fails generation gating (`quota.ts:280`), so the store |
| 87 | +can legitimately hold a PRE-clear snapshot while `parsed` is monthly-only. The |
| 88 | +blanket spread would then re-introduce the stale `weeklyPercent` that #382 |
| 89 | +exists to clear, and it would show up as a phantom weekly bar on the main card. |
| 90 | +Narrowing the merge to `resetCredits` removes that failure mode entirely. |
| 91 | + |
| 92 | +### Identity-change safety (corrected — audit blocker 3) |
| 93 | + |
| 94 | +The first two drafts claimed a swapped identity "cannot leak" a previous |
| 95 | +account's credits. **That claim was wrong**, and the second reviewer |
| 96 | +(muse-spark-1.3-contributor) refuted it with the exact path: |
| 97 | + |
| 98 | +- In-process swaps ARE safe: `reconcileMainCodexAccountRuntimeState` |
| 99 | + (`account-lifecycle.ts:60-70`) purges alias-keyed `__main__` quota when it |
| 100 | + observes the account id change, and `mainSnapshotLive === false` forces |
| 101 | + `EMPTY_MAIN_ACCOUNT_INFO`, whose null quota short-circuits the DTO guard. |
| 102 | +- Across a RESTART it is not. `observedMainChatgptAccountId` |
| 103 | + (`account-lifecycle.ts:21`) is memory-only, and the first observation after a |
| 104 | + restart hits the `previousAccountId === undefined` early return with no purge |
| 105 | + (`:67`). If `~/.codex/auth.json` was swapped while the proxy was down, the |
| 106 | + disk-hydrated `__main__` quota entry still belongs to the PREVIOUS login, and |
| 107 | + a store-based fill would print its ticket count on the new account's card. |
| 108 | + Pool accounts never have this hole because their store key is the account id |
| 109 | + itself; `__main__` is an alias. |
| 110 | + |
| 111 | +Fix: do not read the fill value from the store at all. Keep an in-process, |
| 112 | +identity-tagged observation of the last parsed count |
| 113 | +(`mainResetCreditsProvenance = { accountId, credits }`), recorded in |
| 114 | +`fetchMainAccountInfoWhileOwned` next to the existing `freshResetCredits`, and |
| 115 | +return it only when `getMainChatgptAccountId()` still matches. A restart simply |
| 116 | +starts with no observation, so the badge waits for the first response that |
| 117 | +carries the summary rather than showing a stale or foreign number. |
| 118 | + |
| 119 | +```ts |
| 120 | +let mainResetCreditsProvenance: { accountId: string; credits: number } | null = null; |
| 121 | + |
| 122 | +function mainResetCreditsForCurrentIdentity(): number | undefined { |
| 123 | + if (!mainResetCreditsProvenance) return undefined; |
| 124 | + const currentAccountId = getMainChatgptAccountId(); |
| 125 | + if (currentAccountId === null) return undefined; |
| 126 | + if (currentAccountId !== mainResetCreditsProvenance.accountId) { |
| 127 | + mainResetCreditsProvenance = null; |
| 128 | + return undefined; |
| 129 | + } |
| 130 | + return mainResetCreditsProvenance.credits; |
| 131 | +} |
| 132 | +``` |
| 133 | + |
| 134 | +`updatedAt` still comes from the store, unchanged from today's behaviour. |
| 135 | + |
| 136 | +### Consume-route interaction (audit question Q2c) |
| 137 | + |
| 138 | +`auth-api.ts:2135` deliberately refuses to report a preserved cached |
| 139 | +`resetCredits` as the consume response's `remaining`. That governs a |
| 140 | +*transactional* claim about a just-executed redeem and is a different guarantee |
| 141 | +from best-effort display state, so the DTO carry does not violate it. The real |
| 142 | +overlap, recorded rather than fixed: if the forced post-consume refresh omits |
| 143 | +the summary, the main card keeps showing the pre-consume count until the next |
| 144 | +response that carries it — exactly the staleness every pool card already has. |
| 145 | + |
| 146 | +### Upstream omission semantics (residual, non-blocking) |
| 147 | + |
| 148 | +If upstream ever omits `rate_limit_reset_credits` to MEAN zero, a carried |
| 149 | +non-zero would persist until the next explicit reading. Nothing in this |
| 150 | +repository settles that question, the risk is pre-existing in the store merge, |
| 151 | +and it is shared with every pool card. Named here rather than guessed at. |
| 152 | + |
| 153 | +### quotaForPlan interaction |
| 154 | + |
| 155 | +`quotaForPlan` already forwards `resetCredits` for 30-day plans |
| 156 | +(`auth-api.ts:272`) and `withSparkVisibility` only filters `customWindows`, |
| 157 | +which this helper does not touch. No change needed in either. |
| 158 | + |
| 159 | +Note on `quotaForPlan`: it already passes `resetCredits` through for 30-day |
| 160 | +plans (`auth-api.ts:272`), so no change is needed there. |
| 161 | + |
| 162 | +## Accept criteria |
| 163 | + |
| 164 | +1. Given a main WHAM parse result without `resetCredits` and a store entry for |
| 165 | + `__main__` holding `resetCredits: 1`, the main DTO carries `resetCredits: 1`. |
| 166 | +2. Given a parse result WITH `resetCredits: 0` and a store entry holding |
| 167 | + `resetCredits: 3`, the DTO carries `0` — fresh wins, including zero. |
| 168 | +3. Given no store entry, the DTO is byte-identical to today's output. |
| 169 | +4. `updatedAt` behaviour is unchanged (store value, else now). |
| 170 | +5. Window fields are never taken from the store: a monthly-only parse with a |
| 171 | + stored weekly value still produces a DTO without `weeklyPercent`. |
| 172 | +6. A carried count is dropped when the physical main account id changes. |
| 173 | + |
| 174 | +### Activation scenario for the conditional path |
| 175 | + |
| 176 | +The new helper's store branch only runs when `getAccountQuota("__main__")` |
| 177 | +returns an entry. The test triggers it by calling |
| 178 | +`setAccountQuotaFromParsed(MAIN_CODEX_ACCOUNT_ID, { resetCredits: 1 })` before |
| 179 | +listing accounts, and proves it ran by asserting `resetCredits` is present in |
| 180 | +the returned DTO where it is absent today. |
| 181 | + |
| 182 | +## Verifier |
| 183 | + |
| 184 | +`bun test tests/codex-auth-api.test.ts` — this file already exercises the main |
| 185 | +DTO and the `__main__` quota store (it references |
| 186 | +`getAccountQuota(MAIN_CODEX_ACCOUNT_ID)?.resetCredits` at lines 2631, 2668, |
| 187 | +2706), so it observes the change target directly. |
| 188 | + |
| 189 | +## Field chain (PLAN-FIELD-CHAIN-01) |
| 190 | + |
| 191 | +`resetCredits: number | undefined` is not a new field; this phase changes which |
| 192 | +object the DTO reads. Chain for completeness: |
| 193 | + |
| 194 | +- creation: `parseUsageQuota` (`quota.ts:562`) from |
| 195 | + `rate_limit_reset_credits.available_count`; also |
| 196 | + `updateAccountQuota(..., resetCredits)` (`quota.ts:473`). |
| 197 | +- store merge: `setAccountQuotaFromParsed` (`quota.ts:294, 339-340`). |
| 198 | +- serialization: `poolAccountDto` (store-read) and the main DTO (this fix). |
| 199 | +- deserialization: `hydrateAccountQuotasFromDisk` reads |
| 200 | + `codex-quota-cache.json`; N/A for the DTO, which is response-only. |
| 201 | +- consumers: `CodexTicketBadge` (`gui/.../codex-account-pool-helpers.tsx:29`), |
| 202 | + `src/cli/account-auth.ts`, the reset-credit consume route |
| 203 | + (`auth-api.ts:2135`). |
| 204 | + |
| 205 | +## Bypass record (PLAN-BYPASS-NAMED-01) |
| 206 | + |
| 207 | +This phase adds no enforcement. Tier: n/a. Executing surface: n/a. Known bypass: |
| 208 | +n/a. Residual risk: a future main DTO rewrite could reintroduce the raw-parse |
| 209 | +read; the regression test in criterion 1 is the early warning. Final enforcement |
| 210 | +layer: none. |
0 commit comments