Skip to content

Commit 6d97a9f

Browse files
committed
Merge remote-tracking branch 'upstream/dev' into pr-747-full-review
2 parents 75ac18d + 5ddb14c commit 6d97a9f

107 files changed

Lines changed: 10333 additions & 857 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/scripts/enforce-pr-target.test.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ describe("enforce-pr-target workflow", () => {
6565
assert.match(workflow, /stackedBase/);
6666
assert.match(workflow, /github\.rest\.pulls\.list/);
6767
assert.match(workflow, /treating as stacked/);
68+
assert.match(workflow, /other\.base\?\.repo\?\.owner/);
6869
const qualityCall = workflow.match(
6970
/collectPrQualityFailures\(\{([\s\S]*?)\}\);/,
7071
);

.github/workflows/enforce-pr-target.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,8 @@ jobs:
299299
other =>
300300
other.number !== pull_number &&
301301
other.head?.ref === pr.base.ref &&
302-
(other.head?.repo?.owner?.login ?? owner) === baseOwner &&
303-
(other.head?.repo?.name ?? repo) === baseName
302+
(other.base?.repo?.owner?.login ?? owner) === baseOwner &&
303+
(other.base?.repo?.name ?? repo) === baseName
304304
);
305305
if (stackedBase) {
306306
core.info(
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
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

Comments
 (0)