fix(ui/design-system): address PR-Agent feedback on PR #1109

Three substantive issues raised by upstream review, encoded in code +
spec so future drift can't reintroduce them:

1. Width floor was unenforceable. The previous spec wording said
   "form ≥ 360px / json ≥ 320px" but `react-resizable-panels` v3 only
   accepts percentage `minSize`. On a 1280px viewport this could let a
   user drag a pane down to ~250px — well below the documented floor.
   - `Panel minSize` bumped 25 → 30 (≥ 30% of body width after rail)
   - Spec rewritten percent-based with the actual 300–360px range
     across realistic viewports plus a note on the v3 API constraint
     and the `onResize`-clamp escape hatch if hard pixel floors become
     necessary later.

2. `storageKey` default caused cross-page state bleed. The previous
   default `storageKey="ccs.config-layout"` meant any `<ConfigLayout>`
   without an explicit key would share localStorage state with every
   other Config page — split ratios contaminating across unrelated
   pages.
   - `storageKey: string` is now REQUIRED (no default). TypeScript
     compile-fails any caller that omits it.
   - Spec restated to make the per-page-key contract explicit.

3. Sensitive-field heuristic was too narrow. The previous regex
   `AUTH_TOKEN|API_KEY|SECRET|PASSWORD|PRIVATE_KEY` missed common
   secret names (ACCESS_TOKEN, REFRESH_TOKEN, BEARER_TOKEN,
   CLIENT_SECRET, CLIENT_ID, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY,
   GCP/Azure/GitHub/OpenAI/Anthropic variants, JWT, OAUTH, CREDENTIAL,
   PAT, WEBHOOK_SECRET, HMAC_KEY, SIGNING_KEY, SSH_KEY).
   - New `src/lib/sensitive-label.ts` Single Source of Truth
     (`isSensitiveLabel(label)`) with broadened regex; case-insensitive
     and tolerant of `_`/`-` separators.
   - `Field` imports the shared helper; future consumers do too.
   - Spec §5g enumerates the new patterns and points at the SSoT.

Decisions log: v1.7 entry records the rationale and the connections
between spec wording and library API constraints, so the next reviewer
sees the trail rather than re-discovering it.

Validation: typecheck + lint + format clean; build clean; tests
519/521 pass (2 pre-existing account-visual-groups failures on dev,
unrelated). Styleguide demos already pass storageKey explicitly.
This commit is contained in:
Tam Nhu Tran committed 2026-04-26 13:50:47 -04:00
1 parent 53030891c6
commit 839997d08e
6 files changed
+68 -20

No files matched your search

+14
View File
@@ -52,6 +52,20 @@ After the v1.1 restructure, the `health` page received a separate, focused redes
---
## v1.7 revision (2026-04-26) — PR-Agent feedback: enforce mins, require storageKey, broaden sensitive heuristic
Three substantive issues surfaced by upstream PR review on PR #1109. Each is encoded in code + spec:
**Width floor — pixel claim was unenforceable.** The previous spec wording said "form ≥ 360px / json ≥ 320px" but `react-resizable-panels` v3 only accepts percentage `minSize`. On a 1280px viewport this could let a user drag a pane down to ~250px — well below the documented floor. Resolution: bump `minSize` from 25 to **30**, restate the floor as percentage (≥ 30% of body width after the rail), document the actual 300–360px range across realistic viewports, note the v3 API constraint, and leave the door open for a future `onResize` clamp if hard pixel floors become necessary.
**`storageKey` was no longer optional.** The previous default `storageKey="ccs.config-layout"` meant any `<ConfigLayout>` without an explicit key would share localStorage state with every other Config page — split ratios bleeding across unrelated pages. Resolution: make `storageKey` REQUIRED in the `ConfigLayout` props (no default). TypeScript now enforces explicit per-page keys. Pages MUST pass e.g. `storageKey="config-layout.cliproxy"` — this is checked at compile time, not at runtime.
**Sensitive-field heuristic was too narrow.** The previous regex `AUTH_TOKEN|API_KEY|SECRET|PASSWORD|PRIVATE_KEY` missed common secret names: `ACCESS_TOKEN`, `REFRESH_TOKEN`, `BEARER_TOKEN`, `CLIENT_SECRET`, `CLIENT_ID`, `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, GCP/Azure/GitHub/OpenAI/Anthropic variants, `JWT`, `OAUTH`, `CREDENTIAL`, `PAT`, `WEBHOOK_SECRET`, `HMAC_KEY`, `SIGNING_KEY`, `SSH_KEY`. Resolution: extract the heuristic to `src/lib/sensitive-label.ts` (`isSensitiveLabel(label)`) and broaden the pattern to cover all of those; case-insensitive; tolerates `_`/`-` separators. The Field component imports the shared helper. Adding a new pattern means editing one regex; every consumer inherits.
The shared helper is the Single Source of Truth so future drift can't reintroduce per-component heuristics that disagree with each other.
---
## v1.6 revision (2026-04-26) — content-fit rail (unified envelope, not fixed width)
Live review of the rail-anchored pattern on the API Profiles page surfaced a regression: at the previously-mandated fixed `260px` rail width, the rail header "API Profiles" wrapped onto two lines, the description band wrapped, and the action buttons crowded each other — i.e. the rail was overflowing its own content even though the system mandated that exact width.
+6 -4
View File
@@ -42,8 +42,8 @@ The **middle (form) and right (json) panes of `ConfigLayout` MUST be horizontall
- The left rail is **unified across pages but content-fit** — same envelope, same primitives, but the actual width adapts to its header content. Constraints: `min-w-[240px]` (floor — header controls never squeeze into a wrap) and `max-w-[360px]` (cap — rail can't dominate the body even when an entity label is unusually long; per-item `truncate` inside `ListPane` handles labels beyond that). The rail is NOT user-resizable — only the form↔json split is.
- The form ↔ json divider is the **only adjustable split**. Users frequently want a wide form for entering env values OR a wide json pane for reading effective config — never both at the design-time defaults.
- **Default ratio: form ~45% / json ~55%** of the body width remaining after the rail. The json pane is **slightly larger by default** because the canonical cliproxy reference shows users spend more time reading effective configuration than editing one field at a time. Persist the user's chosen ratio per-page via `react-resizable-panels` `autoSaveId` (resolves to a `localStorage` key).
- Each pane has an enforced **minimum width** (form ≥ 360px, json ≥ 320px) so neither collapses to unreadable.
- **Default ratio: form ~45% / json ~55%** of the body width remaining after the rail. The json pane is **slightly larger by default** because the canonical cliproxy reference shows users spend more time reading effective configuration than editing one field at a time. Persist the user's chosen ratio **per page** via `react-resizable-panels` `autoSaveId` — `ConfigLayout`'s `storageKey` prop is **required**, no default, so each page must pass a stable identifier (e.g. `storageKey="config-layout.cliproxy"`). A shared default would bleed split-ratio state across unrelated pages, so the type system enforces explicit keys.
- Each pane has an enforced **minimum size of 30%** of the resizable container (the body width after the rail), via `react-resizable-panels`' percentage-based `minSize`. On a 1280px viewport with a 280px rail this lands around 300px per pane — readable but not generous; on standard 1500px+ viewports it lands at 360px+, the practical comfort floor. The library's `minSize` API does NOT support pixel values in v3; if hard pixel floors become necessary later, layer a `onResize` clamp on top.
- When the json pane is omitted (`json={undefined}`), the form expands to fill the remaining width — no divider rendered.
- Below the `<1024px` breakpoint the layout collapses to tabs (Browse / Configure / JSON) — resizing is irrelevant in tab mode.
@@ -361,7 +361,9 @@ Sticky headers (`FormPane.header`) include a soft inset bottom shadow so when th
### 5g. Sensitive fields
Fields whose label matches `AUTH_TOKEN|API_KEY|SECRET|PASSWORD|PRIVATE_KEY` are **automatically** rendered as sensitive. The treatment:
Fields whose label matches the heuristic in `src/lib/sensitive-label.ts` (`isSensitiveLabel(label)`) are **automatically** rendered as sensitive. The pattern covers the common conventions: `AUTH_TOKEN | ACCESS_TOKEN | REFRESH_TOKEN | BEARER_TOKEN | API_KEY | API_TOKEN | API_SECRET | CLIENT_ID | CLIENT_SECRET | AWS_ACCESS_KEY_ID | AWS_SECRET_ACCESS_KEY | GCP/AZURE/GITHUB/GITLAB/OPENAI/ANTHROPIC/GEMINI variants of the above | PRIVATE_KEY | SSH_KEY | JWT | OAUTH | CREDENTIAL | PAT | PASSWORD | PASSPHRASE | WEBHOOK_SECRET | HMAC_KEY | SIGNING_KEY`. The match is case-insensitive and tolerates `_` / `-` separators. If the heuristic doesn't match a label that IS a credential, pass `sensitive` explicitly. Adding a new pattern means editing one regex in `sensitive-label.ts` — every consumer (Field, MaskedInput callers that delegate, future helpers) picks it up automatically.
The treatment:
- Lock glyph (`lucide-react/Lock`) prefixing the label, tinted `accent/70`
- "sensitive" status pill on the right of the label row (accent tones per §5c)
@@ -387,4 +389,4 @@ These remain bespoke and are out of scope:
## 7. Decisions
See [`design-decisions.md`](./design-decisions.md) for the resolved open questions and the v1.1 / v1.2 / v1.3 / v1.4 / v1.5 / v1.6 revision rationale.
See [`design-decisions.md`](./design-decisions.md) for the resolved open questions and the v1.1 / v1.2 / v1.3 / v1.4 / v1.5 / v1.6 / v1.7 revision rationale.