feat(v2): align Settings with artboard by lilyshen0722 · Pull Request #1546 · Team-Commonly/commonly

lilyshen0722 · 2026-09-04T21:11:27Z

TASK-126

Aligns /v2/settings with the corrected 03 · Settings artboard:

adds the 200px anchored section nav with an active tint state at desktop and a stacked mobile layout
adds persisted human display names; Settings edits only Name, while Username remains read-only
renders the actual plan from entitlements.pro: paid accounts see a cobalt Pro · $12 a month badge and Manage billing; Free accounts see a bordered Free badge and Upgrade · $12 a month
removes Settings narration; the Account form is now Name + Username, then a full-width editable Email input with the staged Change control for TASK-128
rejects a display name that case-insensitively matches another account’s username or display label, while allowing the current account’s existing name
applies a coarse per-IP profile-write ingress limit (120 / 15 min) before auth plus the durable per-user limit (30 / 15 min) after auth
mirrors the display label to PostgreSQL chat authorship and returns a retryable error if that mirror fails
renders Connected apps as plain bordered Settings sections—no Settings-variant MUI Cards/icons—and folds Developer Tips under the form
scopes legacy V1 element styles outside the v2 canvas and guards against future unscoped element selectors

Verification

frontend: 90 suites / 586 tests, typecheck, production build
backend: 351 suites / 2,977 tests (5 expected skips) and typecheck
limiter mutation: changing the per-user maximum from 30 to 31 makes the 31st-write assertion fail; restored value passes
scoped lint: 0 errors (known JSX/legacy warnings only)

Visual evidence

UX must recapture signed-in 1440 and 390 screenshots against the corrected artboard at this head. This remains a draft until CI and both gates pass.

lilyshen0722

Gate at 1d1184b0dd43658f803a6d48ccb2c40b69556971 (draft). Both gate lines I stated in the pod are met; one finding I'd want ruled before this leaves draft.

Gate line 1 — card-free, not radius-only: PASS

I said a radius-only change to the three <Card>s would be a finding. It isn't one. AppSurface (AppsManagement.tsx:112-115) renders a plain <Box> in the settings variant and keeps <Card> elsewhere, so the .MuiCard-root grammar — v2.css:5252's !important ground/border/shadow and the :5261 translateY(-1px) hover — no longer applies at all. Developer Tips is an inline muted <Typography variant="body2" color="text.secondary"> (:441). That is Sam's delta 4 as worded.

Gate line 2 — the pg mirror is observable: PASS

syncUserToPostgreSQL now returns boolean instead of void, and updateProfile:126-132 turns a failed mirror into a 503 with a retry message rather than a silent 200. That closes the failure I flagged: a name saved in Mongo while chat keeps rendering the old one, with nothing surfaced. The message reaches the user — V2AccountSection.save reads response.data.error.

Verified by mutation rather than by reading. Reverting agentIdentityService.ts:595 from user.displayName || user.username to user.username gives 1 failed, 1 passed — agentIdentityService.displayName.test.js:41 fails on the $2 parameter, and the return-value test stays green. So the two assertions are independent and the sync assertion discriminates. Restored, tree clean.

Caveat on what that test can prove: the pg pool is mocked, so it pins the SQL and the parameters, not that the render path reads them. The column is right — users.username is what pg/Message.ts:199/:246 select and what V2MessageBubble.tsx:342 renders — but nothing here executes that join.

Finding — displayName is unvalidated against other users' handles, and chat shows nothing else

isValidAccountLabel (userController.ts:39) checks length 1–80 and nothing more. displayName has no unique index and no collision check, and syncUserToPostgreSQL:595 writes it into the pg users.username column.

Failure scenario: user B saves the display name lily-shen. Their next chat message renders as lily-shen, because V2MessageBubble.tsx:342-344 takes message.user?.username — now B's displayName — as the sole author label. There is no handle beside it and no other identity string in the bubble. Two accounts are then indistinguishable in chat.

This is the impersonation case the repo already solved on the agent side: scripts/dedupe-agent-display-names.ts appends (<HumanizedInstanceId>) to non-canonical siblings precisely so two agents can't render identically, and CLAUDE.md states the rule as "collisions live in DB, not in display logic". Humans get no equivalent here.

Smallest fix: reject a displayName that case-insensitively matches any other user's username — one indexed lookup next to the length check, 400 with a message. That is cheaper than the 409 rename machinery the ruling deferred, because it needs no uniqueness index on displayName itself; two people may share a display name, just not borrow a third party's handle.

Flagging rather than blocking: the ruling in the pod chose "username stays the handle, displayName is the label", and whether an unconstrained label is acceptable is that ruling's to extend.

Two notes, neither blocking

agentIdentityService.ts:583 treats the literal string 'undefined' as an unset PG_HOST and returns true. The comment says this is for test harnesses that clear env with undefined. It also means a genuinely misconfigured PG_HOST=undefined reports a successful mirror that never happened — the 503 can't fire. Narrow, but it is production behaviour shaped by a harness artifact.

The D.1 radius inventory is not closed by this PR and shouldn't be read as regressed by it. <Alert severity="info"> (:580) still renders in the settings variant and is a MuiPaper, so App.css:229's border-radius: 12px !important still rounds it; same for the conditional <Paper>s at :447 and :545. Those belong to the App.css scoping item, not to delta 4.

lilyshen0722

UX-GATE: CHANGES_REQUESTED @ 1d1184b — built the branch locally and walked it signed in at 1440 + 390 beside the 03 · Settings artboard (sheet in Sprint HQ). Passing: 200px anchored left nav with the active tint pill (Plan click scrolls the section to the top, pill follows, URL #v2-settings-plan); Bricolage Title-case section headings; Name editable, Username + Email read-only with a note, Save ink; cobalt 'Pro · free in beta' pill; connected apps card-free with Developer Tips as a muted line; nav hidden at 390, width 390. Three fixes: (1) the Account block renders as a DARK 12px card on both viewports — the

is caught by App.css's bare form { background: #111827; border-radius: 12px; padding: 20px } (App.css:126, bundle order 2087; the only matching rule for background/radius on that element). Same class as the bare .MuiPaper-root — scope it to V1 or reset it on .v2-settings form; the artboard's Account is a flat form on white with Name + Username side by side and Email full width, three-up inputs truncate the values. (2) Double headings: 'Plan' (section heading + .v2-billing__title) and 'Devices' (section heading + the panel's h2) — one heading per section, drop the inner titles in the settings variant. (3) Badge type: artboard is mono sentence case 'Pro · free in beta'; rendered Plex Sans uppercase — font-family mono, no text-transform. Non-blocking: one MuiPaper left in connected apps (the info Alert, 12px) plus MUI inputs/selects at 8/12px — legacy inner controls, later slice; Language's active option border is cobalt (selection should be ink).

lilyshen0722

Re-gate at 24993c08c6ae62dd8e54e5326cd8481654d7a38b.

The diff from 1d1184b0 touches seven frontend files and leaves AppsManagement.tsx, agentIdentityService.ts, userController.ts and User.ts untouched, so both gate lines from my previous review carry unchanged: connected apps is still card-free via AppSurface, and the pg mirror still reports failure through the 503. The displayName-collision finding is also unchanged — still open, still lily-shen's to rule.

Two problems with the form fix.

The neutralizer leaves two v2 forms rendering the dark shell

The fix is .v2-settings form.v2-settings__account in v2.css:7596. It neutralizes one form. App.css:126 is untouched, so form { background: #111827; padding: 20px; max-width: 400px; margin: auto; border-radius: 12px; box-shadow: ... } still reaches every other <form> in v2.

There are eight. Five carry .v2-login__card, which sets its own surface. .v2-pods__create (V2PodsSidebar.tsx:509) sets background: var(--v2-surface), its own border and radius. That leaves two that declare no background, padding, max-width or radius:

.v2-connect__gateway-form — V2ConnectPage.tsx:139; the rule sets only display: grid, grid-template-columns: repeat(3, minmax(0, 1fr)) auto, align-items, gap
.v2-team__redeem-form — V2YourTeamPage.tsx:471; sets only display: flex, align-items, gap, flex-wrap

Cascade here is per-property, not per-rule: for background, padding, max-width, margin and border-radius the only declaration in play is App.css's type selector, so both render as a dark 12px card. On the Connect form the max-width: 400px; margin: auto also collapses a three-column grid into a 400px centred box. Worth one browser look at /v2/connect and /v2/team before anyone calls this class closed — I can read the cascade but not the paint.

The repo has admitted this trap twice already without fixing it: V2Register.tsx:14-15 ("a bare <form> picks up a dark global background, so it must carry the card") and v2.css:5090-5092. This PR is the fourth workaround. Scoping the rule in App.css fixes all four sites and the two live ones at once.

The new guard asserts the bug must stay

v2-layout-invariants.test.ts:471 is expect(appStyles).toContain('form {').

That pins the defect in place. When someone scopes App.css — the fix this whole thread has been converging on — the bare form { selector stops existing and this assertion goes red, on a commit that made the codebase correct. A guard should fail when the bug returns, not when it is fixed.

If the intent is "the neutralizer is still load-bearing," assert the neutralizer: ruleBody(v2, '.v2-settings form.v2-settings__account') already carries padding: 0, background: transparent, box-shadow: none three lines below, and those three assertions do the job on their own. Drop the appStyles line.

The guard I suggested in the pod is the inverse of this one: App.css contains no unscoped type-selector rules. Seven exist today (form, input, input:focus, button, button:hover, button:active, img), so it has to land with the scoping rather than before it.

lilyshen0722

UX-GATE: APPROVED @ 24993c0 — rebuilt and re-walked signed in at 1440 + 390 beside the 03 · Settings artboard (sheet in Sprint HQ). The three fixes hold: the Account form is flat on white (transparent ground, radius 0, padding 0, no shadow, full width; inputs 259px with no truncation); one heading per section; the badge is IBM Plex Mono 12px sentence case on cobalt. Standing checks unchanged: 200px anchored nav with the active pill at 1440, hidden at 390; cobalt fills 1 (the ruled badge); scroll width 390. Non-blocking, carried: Account fields are three-up in one row where the artboard draws Name + Username side by side and Email full width (cosmetic while two are read-only); legacy MUI inner controls in connected apps at 8/12px radius; Language's active option border is cobalt where selection should be ink.

lilyshen0722

Re-gate at 387e523d22f7496fe6c012ce8ac916d0bb17e99f. The boundary fix is right and the guard discriminates. One hole in the guard, demonstrated.

The scoping is correct, and the ordering works out

All seven rules now sit under body.modern-ui:not(.v2-canvas). I checked the one thing that could have made this a paint regression rather than a fix — whether a v2 route can ever render while v2-canvas is absent but modern-ui is present.

It can't. index.tsx:20-22 stamps v2-canvas before createRoot for / and /v2*; App.tsx adds modern-ui in an effect. For the case the comment calls out as the exception — a legacy deep link that redirects into v2, where the boot-path check misses — React runs child effects before parent effects, so V2App.tsx:177's add lands before App's. The compound never matches on a v2 surface.

The inverse is a new, minor flash: on V1 routes modern-ui arrives after first paint, so bare forms, inputs and buttons render unstyled for one frame where they used to be styled immediately. V1 is legacy and this is one frame; noting it rather than asking for anything.

The guard is a denylist of seven, not a boundary

Ran it rather than reading it, with a positive control.

Mutation: unscope button back to button {. Result 1 failed, 81 passed — the new test is the one that fails, so it discriminates for the names it lists.

Probe, on the unmutated head: append to App.css

select {
  background: #111827;
  border-radius: 12px;
}

textarea, form {
  color: #f8fafc;
}

Result 82 passed. Both slip through, and both paint v2.

Two separate holes. The regex /^(?:form|input(?::focus)?|button(?::hover|:active)?|img)\s*\{/m enumerates the seven names that exist today, so an eighth bare type selector — select, textarea, a, table — is unguarded. And it is anchored to the start of a line, so a bare rule that reaches a listed element through a comma list (textarea, form {) is missed even for a name on the list.

Closing both is a shape change, not more entries: reject any rule whose selector list contains a component starting with a bare element name. Something like — for each selector before a {, split on ,, and fail if any part's first compound matches ^[a-zA-Z] and isn't html/body. That is the invariant the thread has actually been converging on ("App.css has no unscoped type selectors"), and it would have caught form in 2026-08-24 without anyone naming it in advance.

Not blocking on it — the seven that exist are correctly scoped and correctly pinned. But the current test's failure mode is silent: the next bare rule someone adds arrives green.

Prior gate lines

Unchanged and still passing. AppsManagement.tsx, agentIdentityService.ts, userController.ts and User.ts are untouched since 1d1184b0. The displayName-collision finding is still open and still lily-shen's to rule.

One note on my own instrument: my first run of this file reported a failure in E2E specs do not reference a retired legacy feature route. That was my working tree sitting 32 commits behind origin/main, not this PR. Against origin/main's e2e/ the file is 82/82 green at this head.

lilyshen0722

UX-GATE: APPROVED @ 387e523 — rebuilt and walked signed in at 1440 + 390 (sheet in Sprint HQ). Settings holds everything from 24993c0: flat Account form (transparent, radius 0, 800px), one heading per section, mono sentence-case badge on cobalt, 200px anchored nav at 1440 and hidden at 390, cobalt fills 1, width 390. Regression sweep for the App.css scoping across /v2/login, /v2/register, /v2/settings, /v2/agents (invitation form open), /v2/connect and a pod chat: 0 controls render with UA defaults (checked background, border style and font on every button/input/select), login and register cards unchanged (white, radius 6, 380px), and the Your team invitation form is now a flat inline row instead of the dark 400px card it renders as on live main today. Radius-over-6 count under Settings fell from 11 to 6 with the scoping; the remaining six are MUI inner controls in connected apps — carried, not this PR.

lilyshen0722

Code gate at 566b2aff0979044d0f505dcb967162904b16b299. Clear from my side.

The delta from 387e523d is one file, v2-layout-invariants.test.ts, +24/-20 — no CSS, no component, no route. So the visual gate carries as claimed, and my 387e523d verdict carries with it: both original gate lines still hold (AppsManagement.tsx, agentIdentityService.ts, userController.ts, User.ts untouched since 1d1184b0).

The guard hole is closed, verified against the same probes that opened it

bareElementSelectors strips comments, splits every selector group on ,, and rejects any part whose first compound is an element name other than html/body. That is the shape rather than a name list, and it carries its own positive controls inline (select { } → ['select'], textarea, form { } → ['textarea', 'form']), so the extractor can't silently stop extracting.

Re-ran the exact probes that passed green at 387e523d:

appended to App.css	at `387e523d`	at `566b2aff`
`select { background: #111827 }`	green	red
`textarea, form { color: #f8fafc }`	green	red
`@media (max-width: 768px) { button { color: red } }`	not tested	red

Unmutated: 82/82. Also checked the two shapes that could have made this a false-positive machine — App.css has no @keyframes, so no from/to blocks to misread as elements, and the three @media groups are filtered by the @ check while their nested rules are still scanned.

Remaining boundary, for the record and not for this PR

Two unscoped shapes still pass, both probed green just now:

[type="text"] { background: #111827; }
* { letter-spacing: 0.2px; }

Neither exists in App.css today and neither is what any of the four historical leaks looked like, so this is not a gap to close now — it is the edge of what the invariant claims, worth knowing before someone reads it as "App.css cannot reach v2".

Still open, and not mine

The displayName collision from my 1d1184b0 review is unchanged and unaddressed: isValidAccountLabel checks length only, and the value lands in the pg column the chat bubble renders as its sole author label. That is lily-shen's ruling, not a code defect I can gate on — flagging it once more only because this PR is otherwise ready to leave draft.

Instrument note: run from a working tree pinned to this head with e2e/ taken from origin/main, since my checkout sits behind main and the retired-route guard reads e2e/ from the repo root. Tree restored after each probe; git status --porcelain clean.

lilyshen0722

UX-GATE: APPROVED @ 566b2af — the delta from 387e523 is test-only (v2-layout-invariants.test.ts, the selector-shape invariant); no rendered change, so the 387e523 captures and sweep (sheet in Sprint HQ) stand for this head.

lilyshen0722 · 2026-09-04T22:41:20Z

Before/after beside the artboard, signed in, 1440 and 390: https://claude.ai/code/artifact/6e55ebfb-9e88-4844-a3ae-278d3027c89f (Settings section). Matches 03 · Settings; waiting only on the display-name collision reject, then Sam's look.

lilyshen0722

Gate at 2a4b2c7b93727b7c7c11a53609a02ceee4e1dd2c. The three tests I said I'd gate on all discriminate. Clear from my side.

The delta from 566b2aff is four backend files, +129/-2; nothing frontend, so ux-lead's visual gate carries untouched.

The lookup matches the ruling, including the widening

userController.ts:118-128 is an $or across username, displayName and botMetadata.displayName, self-excluded by _id: { $ne: req.user.id }, run under .collation({ locale: 'en', strength: 2 }), returning the ruled 400 message. User.ts:396-398 adds the three matching collation indexes, so the lookup is index-backed rather than a scan — the thing I flagged as the difference between one line and three.

strength: 2 is the right choice and the comment says why: folds case, keeps accents. So Lily/lIlY collide and Renee/Renée do not.

Mutation results

Ran four, each reverting one property of the change. Baseline 18/18.

mutation	result	which assertion caught it
drop the `botMetadata.displayName` clause	1 red	the exact-`$or` assertion — the widening is pinned
drop `_id: { $ne: req.user.id }`	2 red	both the collision and self-exclusion tests
drop `.collation(...)` entirely	4 red	partly structural, see below
`strength: 2` → `strength: 3`	1 red	`toHaveBeenCalledWith({ locale: 'en', strength: 2 })`

The third row is the weak one and worth naming rather than counting. Removing .collation() breaks the mocked chain shape (findOne → collation → select), so four tests fail for a reason that has nothing to do with case-folding. The strength: 3 mutation is the honest version of that check, and it does redden — on the mock's argument assertion, not on behaviour.

That leaves a seam, and I think it's an acceptable one: User.test.js exercises real case-folding and self-exclusion against mongodb-memory-server, but builds its own query rather than calling the controller; the controller test pins the call but cannot observe the fold. Both halves exist, neither alone is sufficient, and closing it would mean a service-tier test for one predicate. Not asking for it.

Two limits, neither blocking, both by design

Check-then-write, no unique index. Two concurrent saves of the same name can both pass the lookup — the User.ts comment states the non-uniqueness as deliberate. Needs simultaneous requests to hit; noting the shape, not asking for a transaction.

The reject guards the displayName path only. updateProfile still accepts username (:130, if (username) userFields.username = username) with no collision check and no E11000 handling. So via a direct API call — not the UI, which sends displayName alone — an account can still take another user's display name as its own username and render identically, which is the ruled invariant reached from the other side. This is pre-existing and explicitly deferred ("username stays the handle and is read-only in this pass; a rename with a duplicate check and a 409 is its own later task"), so it belongs to that task. Recording it because the rename task should now inherit this check rather than only the 409.

Prior gate lines

All carry. AppsManagement.tsx, agentIdentityService.ts and the frontend are untouched since 566b2aff; the App.css selector-shape invariant and the pg-mirror 503 are unchanged.

Instrument note: tree pinned to this head for each run and restored after; git status --porcelain clean.

lilyshen0722

Gate at 74f3582b9350ba1e544af313e0fdc32ec8ddb94e.

Delta from 2a4b2c7b is two files, +29/-1 — routes/users.ts and one route test. Nothing frontend, nothing in the controller, so ux-lead's visual gate and my 2a4b2c7b verdict both carry.

Both gate lines pass

Limiter mounted. routes/users.ts:148 is now router.put('/profile', auth, profileWriteLimit, updateProfile). It follows the house pattern (rateLimit({...}) declared in the route file, mounted on the route), 30 per 15 minutes, keyed by req.userId || req.user?.id || req.user?._id with an ipKeyGenerator fallback, standardHeaders: true, and a 429 handler. Placement is right: after auth, so the user identity exists when the key is computed.

Reject unchanged. userController.ts is untouched at this head, so the collision check still returns 400 with the ruled message; the mutation results from my 2a4b2c7b review stand.

The test pins mounting, not the ruled keying — demonstrated

The assertion is behavioural rather than a source grep: it reads the ratelimit-policy response header for 30;w=900, which only exists if the limiter actually ran. Good.

Baseline 4/4. Two mutations:

mutation	result
drop `profileWriteLimit` from the route	1 red — mounting is pinned
drop the `keyGenerator` block, leaving everything else	4 passed

The second is the gap. Without keyGenerator the limiter silently reverts to express-rate-limit's per-IP default, which is exactly the failure the code comment above it says it exists to prevent — "Limit by authenticated user rather than IP so shared office/NAT traffic cannot spend another account's budget." The ruling said "per user". The header the test reads is identical either way, so nothing in the suite can tell per-user from per-IP.

Closing it is one line in the same test — assert two different authenticated users each get a full budget, or assert ratelimit-remaining doesn't decrement across users. Not blocking: the code is correct as written and the ruling is satisfied at this head. But the property most likely to be lost in a later refactor is the one property not under test.

Not yet verifiable

CodeQL currently reports skipping because Analyze (javascript-typescript) is still pending; E2E Tests and Test & Coverage are pending too. So I can confirm the limiter is mounted and correct, and I cannot yet confirm the alert clears — that needs a re-read once the analysis job finishes. Two rows, and the gate one only resolves after the analysis one.

Worth remembering for the press: the alert this closes (users.ts:129, #1286) has been open on main since 2026-04-08, and js/missing-rate-limiting has ~85 open siblings there. A limiter on put('/profile') does not touch the alerts at lines 128 and 130–133 of the same file.

Instrument note: tree pinned to this head per run and restored after; git status --porcelain clean.

lilyshen0722

Gate at 4d3783531b656287d365132fb343c0551805737a. CodeQL is green — Analyze (javascript-typescript) pass and CodeQL pass, the two rows that were pending/skipping at the last head. E2E Tests and Test & Coverage still pending.

Delta from 74f3582b is two files, +27/-6, both backend. Frontend untouched, controller untouched, so ux-lead's visual gate and the collision-reject verdict both carry.

The credential keying is real and the test discriminates

keyGenerator now tries userId, then a sha256 of the authorization/x-auth-token header, then ipKeyGenerator. Mounted before auth at :157, so in practice the header branch is the live one.

The new test is behavioural and good: exhaust account A's 30, assert account B still gets 200, assert A's 31st gets 429. Removing the bearer-hash branch → 1 red, expected 200 "OK", got 429 on B's request, which is exactly the right failure — B falls into A's shared IP bucket.

Correction to my own instrument, worth recording. My first run of that mutation reported 5 green, and I nearly filed it as "the keying is unpinned". The edit had not landed — the write did not persist and the file still contained the branch. grep -c authHeader on the anchor before re-running caught it. A mutation that never applied is indistinguishable from a suite that ignores the code, and it fails in the reassuring direction.

What is not pinned is the ordering — the property this head exists to establish

Reverting :157 to router.put('/profile', auth, profileWriteLimit, updateProfile) leaves 5 passed.

The cause is the harness: users.social.test.js:13 mocks auth as (req, res, next) => next(). That no-op sets no req.user, so the userId branch is dead in the test regardless of position, and the bearer branch answers identically on both sides of auth. The test is named "keys profile writes by authenticated credentials before auth runs" and verifies the first half only.

So the mount order that CodeQL required, and that this whole head is about, is guarded by nothing. It regresses green.

One line closes it: have the auth mock set req.user = { id: 'u1' }. Then a limiter mounted after it keys both accounts to profile-write:u1, B collides with A, and the existing assertions redden — no new test needed, and the suite starts distinguishing the two orders.

Semantic note for the record

This is per-credential, not per-user as ruled. A person holding a browser JWT, a CLI device token and an API token has three independent 30-write budgets, and a fresh login mints a fresh key. That is looser than "per user" and tighter than "per IP", it is the right trade given the limiter must precede auth, and it is worth saying out loud because the ruling's words were "per user" and the code no longer means that.

Instrument note: tree pinned to this head per run, anchor grepped before each run after the false null above, restored after; git status --porcelain clean.

lilyshen0722

UX-GATE: CHANGES_REQUESTED @ 496fc3a — rebuilt and walked signed in at 1440 + 390 against the corrected 03 · Settings (sheet in Sprint HQ). Sam's three corrections hold: nav note gone, no beta or email helper copy anywhere, plan from entitlements renders the Free state as a bordered mono badge with an ink 'Upgrade · a month' action, cobalt fills 0, one heading per section, nav anchored at 1440 and hidden at 390, width 390. One fix: the Email field is no longer an — the account form has two inputs (Name, Username) and Email renders as a static box squeezed into the three-up row, so the value wraps across two lines at 1440 ('stranger-' / 'smoke@commonly.me') with Change jammed against it. The ruling is an editable field with a Change action, and the artboard draws Email as a full-width input on its own row with the outline Change beside it: make it an editable on its own row (Name + Username above it), Change to its right. That also closes the three-up carry from 24993c0.

lilyshen0722

Gate at 496fc3aa1658c710474b105b95bbf53a6a79d862. Every gate line I set is met, and each one discriminates. Clear from my side.

The coverage swap went the right way

I gated on "replaced, not deleted" because four invariants pinned copy that Sam's corrections remove. All four were rewritten to assert the new rule:

the email sentence became not.toContain(...) plus positive assertions on the Change affordance and .v2-settings__email-control
the beta badge/note assertions became currentUser?.entitlements, the endpoint branch, and both tier/action keys
the badge CSS assertion was split per class — base background: #ffffff, --pro background: var(--v2-accent) — which is the Free-bordered / Pro-cobalt rule stated as two facts instead of one wrong one
billing.betaBadge / betaNote are gone from en.json and zh-CN.json, so no dead keys

V2BillingPanel.test.tsx also gained real behavioural coverage of both branches and the checkout/portal routing, which is stronger than the source-string invariants alone.

Mutations

Baseline: backend 5/5, frontend 86/86 across the two suites.

mutation	result
`router.put('/profile', auth, profileWriteLimit, …)` — revert the mount order	1 red
`isPro = true` — drop the entitlements read	2 red (Free rendering, and endpoint routing)
`.v2-billing__badge--pro` background → `#ffffff`	1 red

The first is the one that mattered most: at 4d378353 this same mutation left 5 green because the auth mock was a no-op. users.social.test.js:13 now sets req.user/req.userId, and the mount order is observable. That was the open gap and it is closed.

Two things I checked that are fine

/api/billing/checkout and /api/billing/portal both exist — routes/billing.ts:64 and :126, mounted at server.ts:211, each behind checkoutLimit and auth. The Upgrade button is live, not dormant, and it has somewhere to go.

entitlements reaches the client for the account holder: PRIVATE_USER_FIELDS is kept when isSelf, and GET /api/auth/user returns the full doc minus password/deviceTokens.

One non-blocking observation

authController.login returns a hand-built six-field user (id, username, email, profilePicture, role, verified) — no entitlements, no displayName. AuthContext.login sets that thin object as currentUser, and the [token, scheduleRefresh] effect then refetches /api/auth/user and replaces it.

So the wrong state is transient, not persistent — but between those two renders a paying account sees the Free badge and an "Upgrade · $12 a month" button, and a user with a display name sees their username. Same class as the v2 dark-flash, with a worse thing showing: an upgrade prompt to someone who already pays. Cheapest fix is adding the two fields to the login payload rather than anything in this PR. Recording it here so it is a known flash rather than a bug report later.

Instrument note: tree pinned to this head per run, anchor asserted before each mutation, restored after; git status --porcelain clean.

lilyshen0722

Gate at b50067f2d96d0b5d5168d9ce5489c1f3772a568b.

The split is built as ruled and the discriminating assertion I asked for is real. One gate line that was closed at 496fc3aa has reopened.

What passes

routes/users.ts:164 is router.put('/profile', profileWriteIngressLimit, auth, profileWriteUserLimit, updateProfile) — IP-keyed 120/15m in front, user-keyed 30/15m behind, each with its own 429 body. That is the shape from the ruling, with the two changes that made the ruled assertion possible: distinct handler messages, and the coarse cap set above the fine one.

The test now runs 30 writes as account A and 30 as account B (60 from one IP, under the 120 ingress cap), then asserts the 31st from A returns { msg: 'rate limit exceeded: 30 profile writes per 15 minutes' } — the user limiter's message, not the ingress one. That distinguishes which limiter refused, which the earlier single-message version could not.

Positive control: raising profileWriteUserLimit.max from 30 to 1000 gives 2 red, including that assertion. The test discriminates.

Also done: the stale comment at V2YourTeamPage.tsx:124-125 is gone.

The reopened gap

Deleting profileWriteIngressLimit from the route entirely — leaving router.put('/profile', auth, profileWriteUserLimit, updateProfile) — leaves 5 passed.

So the pre-auth limiter, which is the whole reason CodeQL went green and the reason the split exists, is guarded by nothing again. This is the same class as the gap at 4d378353, which 496fc3aa closed by making the auth mock set req.user. The split reintroduced it from the other side: the new test exercises the user limiter by design, and 60 requests from one IP never reach the 120 ingress cap, so that limiter is never exercised at all.

It is cheap to close, and the existing harness already supports it. Either:

assert the ingress message directly — drive 120 writes across enough distinct credentials that no single user hits 30, and expect { msg: 'rate limit exceeded: too many profile writes from this address' }; or
assert the mount order structurally, the way the previous head's test did, so that moving the ingress limiter behind auth reddens.

The first is stronger and costs one test. Without one of them, a later refactor that drops the ingress limiter ships green and CodeQL goes red on main rather than on the PR — the exact sequence this thread has already spent four heads on.

Not calling it a blocker on my own authority: every ruled property is implemented correctly at this head, and this is a coverage gap rather than a defect. @lily-shen it is your call whether the press waits for it.

Carried forward, unchanged

The login-payload flash from my 496fc3aa review still stands: authController.login returns six fields with no entitlements and no displayName, so a paying account briefly sees the Free badge and an upgrade prompt before /api/auth/user replaces it. Still a separate fix, not this PR.

Instrument note: head re-resolved before running (b50067f2, still draft, still open); tree pinned to it per run, anchor asserted before each mutation, restored after; git status --porcelain clean.

lilyshen0722

Rebase check at eaac4f073a3bfc46ae574928b785fd822a0d36c8. My b50067f2 gate carries verbatim — the rebase changed nothing but the base.

Compared base-relative rather than by tree hash, since a rebase moves hunk context and makes byte comparison report false differences. For each head: merge-base against origin/main, diff, keep only +/- content lines, sha1 per file.

base b50067f2: a5f57679cf49    base eaac4f07: 1b200414f3d2
files only in b50067f2: []
files only in eaac4f07: []
files with CHANGED +/- lines: NONE
identical: 24 of 24

So every verdict from that review stands unchanged, including the open one below. No re-run of the mutations was needed and none would have told us anything new.

What the moved base did bring, and why it's clean

Three commits landed on main between the two bases — #1542, #1547, #1548, the Connectors Signal pass. Two of their files overlap with this PR: frontend/src/v2/v2.css and frontend/src/v2/__tests__/v2-layout-invariants.test.ts. Git merged both without conflict, but a textual merge says nothing about two Signal passes agreeing, and #1546 also rescopes all seven bare element rules in App.css — which the Connectors page's own form controls sit under.

Ran the three affected suites at this head: 91/91 across v2-layout-invariants, V2BillingPanel, V2SettingsPage. The two passes agree.

Branch authorship is clean for the eventual squash — both commits are Lily Shen <115414357+lilyshen0722@users.noreply.github.com>.

Still open, unchanged from `b50067f2`

Deleting profileWriteIngressLimit from routes/users.ts:164 leaves the profile-route suite 5 passed. The pre-auth limiter — the reason CodeQL is green and the reason the split exists — remains guarded by nothing, because the discriminating test drives 60 writes from one IP and never approaches the 120 cap.

One test closes it: drive 120 writes across enough distinct credentials that no single user reaches 30, and expect { msg: 'rate limit exceeded: too many profile writes from this address' }.

Not a blocker on my authority — every ruled property is correctly implemented at this head. It is lily-shen's call whether the press waits for it.

The login-payload flash from the 496fc3aa review also still stands, and is still a separate fix.

lilyshen0722

UX-GATE: APPROVED @ b50067f — rebuilt and walked signed in at 1440 + 390 against the corrected 03 · Settings (sheet in Sprint HQ). Account now matches the artboard: Name + Username side by side (394px each), Email a full-width editable input (721px) on its own row with the outline Change beside it, no wrapping; Free state renders the bordered mono badge + ink 'Upgrade · a month'; no narration anywhere; one heading per section; cobalt fills 0; nav anchored at 1440, hidden at 390; width 390. Non-blocking: at 390 the email input shrinks to 179px beside Change so the address scrolls inside the field — stack Change under the field at ≤760px when convenient. Legacy MUI inner controls in connected apps (6 elements over 6px radius) remain the carried AppsManagement slice.

lilyshen0722

UX-GATE: APPROVED @ 53a1836 — checked the patch: the rebase onto 1b20041 is content-identical to b50067f (range-diff: every prior commit equal) and the only new commit is backend test coverage for the ingress limiter (users.social.test.js); no rendered change, so the b50067f captures (sheet in Sprint HQ) stand for this head.

lilyshen0722

Code gate at 53a1836f0dd860f4edd1c0ba06a5503b6873fe16. CLEAR. The one item I left open is closed, and I have nothing further.

Delta from eaac4f07 is one file, users.social.test.js, +42/-4. No source change, so every prior verdict carries and ux-lead's b50067f2 sheet stands for this head.

The gap is closed

The new test drives 120 writes across 120 distinct credentials — so no single user approaches the 30 cap — then asserts the 121st returns { msg: 'rate limit exceeded: too many profile writes from this address' } with 120;w=900 in the policy header. That is the ingress limiter answering in its own voice, which is what was missing.

Mutation, the same one that left the suite green at b50067f2 and eaac4f07: delete profileWriteIngressLimit from routes/users.ts:164.

head	result
`b50067f2` / `eaac4f07`	5 passed — unguarded
`53a1836f`	1 failed, 5 passed

Baseline 6/6.

The harness fix that came with it is worth noting on its own: buildApp() now calls jest.resetModules() per test so each limiter gets a fresh store. Without it the 120-request test would have leaked into every test after it, and the suite would have been reporting quotas it did not set. That is a real defect avoided, not bookkeeping.

The one thing still unguarded, and why I am not asking for it

Moving the limiter rather than deleting it — router.put('/profile', auth, profileWriteIngressLimit, profileWriteUserLimit, …) — leaves 6 passed. The test's name says "before auth" and the suite still cannot see position, because the mocked auth is a pass-through that never rejects, so placement has no observable effect in-process.

I am not asking for a fourth round on this, because the property is already guarded by a working instrument: CodeQL. This PR demonstrated it directly — at 74f3582b the limiter sat behind auth and js/missing-rate-limiting was red at users.ts:148; moving it in front cleared it. So position is enforced by CodeQL, existence and cap by this suite, and between them the property is covered.

Worth one sentence in the PR body or beside the test, so the next reader does not conclude from the test's name that the suite proves ordering.

Carried, unchanged

The login-payload flash from the 496fc3aa review still stands and is still a separate fix: authController.login returns six fields with no entitlements and no displayName, so a paying account briefly renders the Free badge and an upgrade prompt before /api/auth/user replaces it.

Branch authorship is clean for the squash — all three commits are Lily.

Instrument note: head re-resolved before running; tree pinned per run, anchor asserted before each mutation, restored after; git status --porcelain clean.