Two findings from the v2.5.0 milestone-merge review (#2215), both in clients/web/src/components/groups/ReAuthBanner/ReAuthBannerBar.tsx, which shipped on v2/main during the milestone. Filed here rather than fixed in the merge PR, whose tree is byte-identical to origin/v2/main.
1. The fixed 420px width overflows a narrow viewport (bug)
export const ReAuthBannerBar = Paper.withProps({
pos: "fixed", top: "50%", left: "50%", w: 420,
styles: { root: { transform: "translate(-50%, -50%)", zIndex: 200 } },
});w: 420 is unconditional. Below a 420px viewport the banner is wider than the window, and because it is centred by left: 50% + translate(-50%, -50%) it overflows both edges equally — so it clips its close button on one side and the "Authorize again" action on the other. Those are the only two controls it has, and the component's own header comment explains that dismissing is not equivalent to ignoring it ("Authorize again" also clears the stale OAuth state, which a plain reconnect does not). Losing both controls is therefore a dead end, not a cosmetic clip.
It is reachable in a narrow desktop window, not only on a phone — the component is fixed-positioned against the viewport, so a user who has the browser docked to a third of a wide screen hits it.
Fix: cap rather than fix the width — a viewport-relative width with a 420px maximum, e.g. w="min(420px, calc(100vw - 2rem))" (or maw={420} with a relative w), keeping a gutter so the shadow and radius still read. Worth a story or a test at a narrow viewport, since nothing currently exercises this component below 420px.
2. transform / zIndex belong in a Paper theme variant (convention)
The same constant carries flat CSS in component-level styles. AGENTS.md is explicit that flat CSS properties belong in the theme layer, and clients/web/src/theme/Paper.ts already carries code, contained and panel root styles for exactly this.
The file's existing comment is correct that Mantine exposes neither transform nor zIndex as a style prop — but that argues for the theme variant, which is the next tier in the repo's preference order (props → theme variant → CSS class), not for inline styles.
Fix: add a reauth variant to theme/Paper.ts holding transform and zIndex, and select it here with variant: "reauth" in the .withProps() call.
The trade-off, recorded rather than hidden: this splits one component's positioning across two files, and these styles are genuinely local to this banner rather than a global Paper customization. That is an argument about where the convention's boundary should sit, not an exemption from it — the convention as written covers this, and the variant costs three lines.
Not actionable
The same review also flagged an unterminated inline-code span in clients/web/src/lib/authToken.ts:18. That is incorrect — the line reads // in the `x-mcp-remote-auth: Bearer …` header or the Hono backend returns 401. and carries exactly two backticks, balanced. Nothing to change.
Reported by Copilot on #2215.
Two findings from the v2.5.0 milestone-merge review (#2215), both in
clients/web/src/components/groups/ReAuthBanner/ReAuthBannerBar.tsx, which shipped onv2/mainduring the milestone. Filed here rather than fixed in the merge PR, whose tree is byte-identical toorigin/v2/main.1. The fixed 420px width overflows a narrow viewport (bug)
w: 420is unconditional. Below a 420px viewport the banner is wider than the window, and because it is centred byleft: 50%+translate(-50%, -50%)it overflows both edges equally — so it clips its close button on one side and the "Authorize again" action on the other. Those are the only two controls it has, and the component's own header comment explains that dismissing is not equivalent to ignoring it ("Authorize again" also clears the stale OAuth state, which a plain reconnect does not). Losing both controls is therefore a dead end, not a cosmetic clip.It is reachable in a narrow desktop window, not only on a phone — the component is
fixed-positioned against the viewport, so a user who has the browser docked to a third of a wide screen hits it.Fix: cap rather than fix the width — a viewport-relative width with a 420px maximum, e.g.
w="min(420px, calc(100vw - 2rem))"(ormaw={420}with a relativew), keeping a gutter so the shadow and radius still read. Worth a story or a test at a narrow viewport, since nothing currently exercises this component below 420px.2.
transform/zIndexbelong in aPapertheme variant (convention)The same constant carries flat CSS in component-level
styles.AGENTS.mdis explicit that flat CSS properties belong in the theme layer, andclients/web/src/theme/Paper.tsalready carriescode,containedandpanelroot styles for exactly this.The file's existing comment is correct that Mantine exposes neither
transformnorzIndexas a style prop — but that argues for the theme variant, which is the next tier in the repo's preference order (props → theme variant → CSS class), not for inlinestyles.Fix: add a
reauthvariant totheme/Paper.tsholdingtransformandzIndex, and select it here withvariant: "reauth"in the.withProps()call.The trade-off, recorded rather than hidden: this splits one component's positioning across two files, and these styles are genuinely local to this banner rather than a global
Papercustomization. That is an argument about where the convention's boundary should sit, not an exemption from it — the convention as written covers this, and the variant costs three lines.Not actionable
The same review also flagged an unterminated inline-code span in
clients/web/src/lib/authToken.ts:18. That is incorrect — the line reads// in the `x-mcp-remote-auth: Bearer …` header or the Hono backend returns 401.and carries exactly two backticks, balanced. Nothing to change.Reported by Copilot on #2215.