|
| 1 | +# 000 — API tab improvement |
| 2 | + |
| 3 | +Unit: `devlog/_plan/260731_api_tab_improvement/` |
| 4 | +Opened: 2026-07-31 · Work class: C4 · Branch target: `dev` |
| 5 | +Base commit at open: `33caf3364` (`dev`, fast-forwarded from `origin/dev`) |
| 6 | + |
| 7 | +## Objective |
| 8 | + |
| 9 | +Make the GUI API tab tell the truth about the proxy's public surface, and give a |
| 10 | +`ocx_` key an identity beyond "a row that can be deleted". |
| 11 | + |
| 12 | +Today the tab answers one question — *what is my base URL and which models can I |
| 13 | +call* — and answers a second one wrongly. It documents an authentication rule the |
| 14 | +server does not implement, it advertises three protocols but tests one, and every |
| 15 | +key it lists shows the same eight characters because every generated key starts |
| 16 | +with the same nine. |
| 17 | + |
| 18 | +## The three defects that set the scope |
| 19 | + |
| 20 | +These are not a wish list. Each one is a statement the tab makes that the code |
| 21 | +contradicts. |
| 22 | + |
| 23 | +**1. The tab documents a header the server rejects.** `api.authChatCompletions` |
| 24 | +says Chat Completions accepts `Authorization: Bearer ocx_...` |
| 25 | +(`gui/src/i18n/en.ts:1266`). The route calls `requireResponsesApiAuth` |
| 26 | +(`src/server/index.ts:654`), which reads only `x-opencodex-api-key` |
| 27 | +(`src/server/auth-cors.ts:265-274`). A remote user following the on-screen |
| 28 | +instruction gets a 401 and no way to find out why. `/v1/models` and `/v1/messages` |
| 29 | +*do* take bearer (`src/server/index.ts:403`, `src/server/index.ts:617`), so the |
| 30 | +rule is per-endpoint and the single sentence cannot express it. |
| 31 | + |
| 32 | +**2. Every key looks identical.** POST builds `ocx_data_` + 40 hex |
| 33 | +(`src/server/management/oauth-account-routes.ts:457`) and GET masks |
| 34 | +`key.slice(0, 8)` (`.ts:444`), so the prefix column reads `ocx_data...` for every |
| 35 | +row (`ApiKeysWorkspace.tsx:150`). The one field meant to identify which secret is |
| 36 | +deployed where identifies nothing. |
| 37 | + |
| 38 | +**3. The key detail pane repeats the rail.** The rail row shows name, prefix and |
| 39 | +date (`ApiKeysWorkspace.tsx:142-153`); selecting it shows name, prefix and date |
| 40 | +(`ApiKeysWorkspace.tsx:204-219`). This is the exact duplication that got the |
| 41 | +Subagents rail deleted a week ago (`5aa51b9d7`: "the per-model detail pane carried |
| 42 | +no information the row did not already show"). The API tab kept its rail through |
| 43 | +that sweep and still has nothing to put in it. |
| 44 | + |
| 45 | +Defect 3 is the reason this unit adds per-key attribution rather than only fixing |
| 46 | +copy: a detail pane earns its place when it holds something the row cannot. |
| 47 | + |
| 48 | +## Constraints |
| 49 | + |
| 50 | +| Constraint | Source | |
| 51 | +|-----------|--------| |
| 52 | +| Never render or log key material beyond the one-time POST response | AGENTS.md privacy boundary; `privacy:scan` | |
| 53 | +| Admission behavior must not change while attribution is added | `001` §1 — auth is a security boundary | |
| 54 | +| No new dependency; Bun-native TypeScript | AGENTS.md runtime constraints | |
| 55 | +| Every new visible string lands in all six locales | `gui/AGENTS.md` §Text and i18n | |
| 56 | +| Capped regions hand the wheel back (`overscroll-behavior: auto`) | `87681e540`, `5aa51b9d7` | |
| 57 | +| `usage.jsonl` rows written by older builds must keep parsing | `src/usage/log.ts:266-337` | |
| 58 | +| Security findings never enter a tracked path | AGENTS.md §Security working notes | |
| 59 | + |
| 60 | +## Scope boundary |
| 61 | + |
| 62 | +IN |
| 63 | + |
| 64 | +- Per-endpoint auth truth: server-derived header matrix replacing the four prose |
| 65 | + lines, and the endpoint rows that carry it. |
| 66 | +- Key identity: a distinguishing suffix for new keys, and per-key request |
| 67 | + attribution end to end (admission → request log → usage → management → GUI). |
| 68 | +- Key lifecycle: rename, and a DELETE that reports whether it deleted anything. |
| 69 | +- The detail pane rebuilt around what attribution provides. |
| 70 | +- Layout: remove the desktop `100dvh` shell and the `overscroll-behavior: contain` |
| 71 | + scrollers, matching what Usage/Storage/Subagents already did. |
| 72 | +- Tests at every phase, including the first direct `/api/keys` route tests. |
| 73 | + |
| 74 | +OUT |
| 75 | + |
| 76 | +- Expiry, scopes/allowlists, and rotation. Each changes what a key is *allowed to |
| 77 | + do*, which is an authorization-semantics decision, not a dashboard improvement. |
| 78 | + `001` §3 records the design; this unit does not build it. |
| 79 | +- Reverse-proxy / forwarded-header endpoint derivation. Real defect, wrong unit — |
| 80 | + it is a trust-boundary change and `fix/760-management-origin-tls` is already |
| 81 | + open upstream. `001` §5 records it and phase 2 must not conflict with it. |
| 82 | +- Retroactive attribution. Rows already in `usage.jsonl` have no key id and will |
| 83 | + not gain one; the GUI states this rather than showing a misleading zero. |
| 84 | +- Changes to routing, adapters, the catalog, or the provider registry. |
| 85 | +- The client-config export panel — that is `260731_client_config_export` phase 4. |
| 86 | + See §Coordination. |
| 87 | + |
| 88 | +## Work-phase map |
| 89 | + |
| 90 | +Dependency-ordered (PHASE-SPLIT-01). Each phase consumes the verified output of |
| 91 | +the one before it and closes with something independently verifiable. |
| 92 | + |
| 93 | +| Phase | Doc | Delivers | Verified by | |
| 94 | +|-------|-----|----------|-------------| |
| 95 | +| 1 | `010_key_identity_and_crud.md` | Distinguishable key suffix; `PATCH /api/keys` rename; DELETE reports 404 on unknown id; explicit `apiKeys` config schema | first direct `/api/keys` route tests; config round-trip | |
| 96 | +| 2 | `020_admission_identity.md` | `resolveDataPlaneAdmissionSecret()` returning the matched key id; boolean wrappers unchanged | admission matrix test incl. loopback/env/unknown; no behavior delta | |
| 97 | +| 3 | `030_attribution_telemetry.md` | `apiKeyId` + `admissionKind` + `inboundProtocol` through request-log → `usage.jsonl`; per-key rollup and `authMatrix` on `GET /api/keys` | end-to-end: authed request appears attributed in the rollup | |
| 98 | +| 4 | `040_gui_detail_and_matrix.md` | Detail pane on real attribution; server-derived auth matrix; rename UI; model-test protocol fix | GUI tests + render grounding at 1280x720 | |
| 99 | +| 5 | `050_layout_and_hardening.md` | Kill the `100dvh` shell and `contain` scrollers; uncollapse usage examples; locale parity test | wheel-handoff proof; full GUI gates | |
| 100 | + |
| 101 | +**Where the small correctness fixes go.** W4 (clipboard), W9 (empty vs filtered), |
| 102 | +W13 (models retry), W14 (live region), W15 (name `maxLength`) are not a phase. |
| 103 | +They belong to the component whose markup owns them, so each lands in the phase |
| 104 | +that is already editing that file: |
| 105 | + |
| 106 | +| Defect | Phase | Why there | |
| 107 | +|--------|-------|-----------| |
| 108 | +| W14 live region on test results | 4 | Phase 4 rewrites the result badge into per-protocol chips; the announcement is part of that rewrite, not a later pass | |
| 109 | +| W15 `maxLength` on name inputs | 4 | Phase 4 builds the rename input; the create input gets the same constant in the same edit | |
| 110 | +| W4 clipboard failure | 4 | Same `ApiKeys.tsx` copy handlers phase 4 already touches | |
| 111 | +| W9 empty vs filtered | 5 | Copy-only change in the models panel phase 5 is restyling | |
| 112 | +| W13 models retry | 5 | Needs `refreshModels` plumbed through the resource, independent of phase 4's contract | |
| 113 | + |
| 114 | +Phase 5 is then one thing — the scroll model and what depends on it — plus the |
| 115 | +locale parity gate that must run after every string in the unit exists. That is a |
| 116 | +dependency, not a leftovers bucket: parity can only be asserted once phase 4's |
| 117 | +keys are in the tree. |
| 118 | + |
| 119 | +Phase 1 is first because every later phase writes to the key entry, and an |
| 120 | +unvalidated passthrough schema (`src/config.ts:669-704`) is the wrong foundation |
| 121 | +to add fields to. Phase 2 is the only phase that touches the admission path, and |
| 122 | +it changes no decision — only what the function returns. Phase 3 cannot start |
| 123 | +before 2 because there is no id to record. Phase 4 is the first phase a user sees. |
| 124 | +Phase 5 is polish and must come last: it moves the same markup phase 4 rewrites. |
| 125 | + |
| 126 | +## Coordination with `260731_client_config_export` |
| 127 | + |
| 128 | +That unit's phase 4 (`040_gui_panel.md`) mounts a new `ClientConfigPanel` into |
| 129 | +`ApiKeysWorkspace` "in the connect cluster" — the `awi-overview-left` column that |
| 130 | +today holds Manage, Endpoints and Usage panels |
| 131 | +(`ApiKeysWorkspace.tsx:224-247`). |
| 132 | + |
| 133 | +Both units therefore write `ApiKeysWorkspace.tsx` and |
| 134 | +`styles-apikeys-workspace.css`. They **do** overlap; the reconciliation below is |
| 135 | +what makes the overlap safe, and it binds this unit. |
| 136 | + |
| 137 | +- **Panel order.** This unit adds, removes and reorders nothing in |
| 138 | + `awi-overview-left` (`ApiKeysWorkspace.tsx:224-247`). Phase 4 changes the |
| 139 | + **detail** pane and the **endpoints** panel's internals; phase 5 changes scroll |
| 140 | + geometry. The other unit appends `ClientConfigPanel` to that column. Since one |
| 141 | + unit only appends and the other never touches the list, the JSX conflict is a |
| 142 | + trivial adjacent-insert either way it lands. |
| 143 | +- **Landing order: this unit's phase 5 should land before the export panel.** |
| 144 | + Phase 5 removes the desktop `100dvh` shell, after which a long JSON block needs |
| 145 | + no special handling. If the export panel lands first it still works, but its |
| 146 | + `040_gui_panel.md:63-65` wheel-scroll note becomes stale and should be dropped |
| 147 | + when phase 5 lands. |
| 148 | +- **The shared code-block rules stay unscoped.** `040_gui_panel.md:63-65` says the |
| 149 | + export panel's JSON block inherits `.api-example-pre`'s wheel behavior. Phase 5 |
| 150 | + therefore keeps `.api-example-pre` / `.api-example-copy-btn` as unscoped |
| 151 | + selectors rather than narrowing them to `.awi-usage-panel`, and asserts that in |
| 152 | + a source guard. Narrowing them would silently re-trap the export panel's |
| 153 | + scroll — the exact defect phase 5 exists to remove. |
| 154 | +- **i18n namespaces are disjoint:** `api.clientConfig.*` is theirs; |
| 155 | + `api.auth.*` / `api.key.*` / `api.attribution.*` are this unit's. Phase 5's |
| 156 | + full-parity locale test counts every `api.*` key, so whichever unit lands |
| 157 | + second must have its keys in all six locales — which both units already require. |
| 158 | + |
| 159 | +## SoT sync target (SOT-SYNC-01) |
| 160 | + |
| 161 | +`structure/` holds the maintainer invariants. Phase 2 and 3 change a security- |
| 162 | +adjacent contract (what admission returns) and a durable record shape |
| 163 | +(`usage.jsonl`), so their C phase patches the structure doc that owns the server |
| 164 | +and usage subsystems. Phase 4/5 are GUI-local and sync `docs-site/` only if the |
| 165 | +auth matrix changes documented user instructions — it does, so the Chat |
| 166 | +Completions bearer claim must be corrected there in the same phase it is |
| 167 | +corrected in the GUI. |
| 168 | + |
| 169 | +## Documents in this unit |
| 170 | + |
| 171 | +| Doc | Contents | |
| 172 | +|-----|----------| |
| 173 | +| `000_plan.md` | this file | |
| 174 | +| `001_surface_inventory.md` | current component/data map, key model, weakness list with evidence | |
| 175 | +| `002_backend_feasibility.md` | admission path, telemetry shape, lifecycle gaps, endpoint derivation | |
| 176 | +| `003_ux_direction.md` | what the detail pane is for, the auth matrix design, sibling-tab grammar | |
| 177 | +| `010`–`050` | one diff-level phase document each | |
| 178 | + |
| 179 | +## Cross-phase contracts settled at roadmap lock |
| 180 | + |
| 181 | +Two shapes are shared across phase boundaries and are fixed here so a later P |
| 182 | +cannot quietly reinterpret them. |
| 183 | + |
| 184 | +**`attributionSince` is top-level and singular.** `GET /api/keys` returns |
| 185 | +`usage` on every key row (zeroes are a real answer) plus one response-level |
| 186 | +`attributionSince`. The field describes the usage *data set* — the earliest row |
| 187 | +carrying a recognized `admissionKind` — not a property of a key. It is keyed on |
| 188 | +the kind rather than on `apiKeyId` because environment and loopback rows are |
| 189 | +attributed traffic too; they simply have no configured key to point at. The GUI branches on it to tell |
| 190 | +"attributed nothing" from "nothing is attributable yet"; branching on `usage` |
| 191 | +would merge those two states. Defined in `030` §Design, consumed in `040` |
| 192 | +§Phase-P contract gate. |
| 193 | + |
| 194 | +**Admission identity is two fields, and `usage` is a union.** `apiKeyId` holds a |
| 195 | +configured entry's id and nothing else; `admissionKind` |
| 196 | +(`configured|environment|loopback`) says which kind of admission ran. A single |
| 197 | +field with `"loopback"` as a sentinel would collide with a hand-edited entry whose |
| 198 | +id is that word, since ids are only validated as non-empty strings. And because |
| 199 | +two entries *can* share an id, `usage` is |
| 200 | +`{ ambiguous: true } | { requests7d; totalRequests; lastUsedAt? }` — a union, so no |
| 201 | +consumer can print a total that belongs to two keys. Defined in `030`, rendered in |
| 202 | +`040`. |
| 203 | + |
| 204 | +**The authenticated model test only runs on a freshly generated key.** The GUI |
| 205 | +holds key material exactly once, in the POST response; GET returns a prefix. The |
| 206 | +test controls are therefore disabled outside that window, with copy explaining |
| 207 | +why. Rationale and rejected alternatives in `003` §4. |
| 208 | + |
| 209 | +**The rail is conditional on phase 3.** If attribution does not land, phase 4 |
| 210 | +deletes the rail rather than shipping a third repeated detail pane. That is a |
| 211 | +build-time branch chosen at phase 4's P from the shipped contract, never a |
| 212 | +runtime inference from rows that happen to have no usage (`003` §2, `040` |
| 213 | +§Rail decision). |
0 commit comments