Skip to content

fix(consent): keep a connector's declared brand background on the consent mark - #626

Open
tnunamak wants to merge 5 commits into
feat/console-connector-identity-iconsfrom
fix/consent-icon-background
Open

tnunamak wants to merge 5 commits into
feat/console-connector-identity-iconsfrom
fix/consent-icon-background

Conversation

@tnunamak

@tnunamak tnunamak commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #620: the base is feat/console-connector-identity-icons, because the backgroundColor field on the shared mark renderer (ConnectorIcon) comes from that PR. Merge #620 first and GitHub retargets this PR to main.

What

The consent screen showed a connector's manifest glyph without its declared brand background. buildHostedMcpConsentChallengeModel (reference-implementation/server/routes/as-consent-ui-helpers.ts) copied only color, kind and svg from the manifest icon and dropped brand.background_color. A white glyph then rendered on the default tile and was invisible.

How

  • The server model's icon now carries backgroundColor from manifest.brand.background_color. The server checks it with hexColorOrNull (#rgb or #rrggbb), which lives in @pdpp/brand-react (hex-color.ts) and is the same check the console uses in feat(console): show one identity-based connector mark everywhere #620; anything else, or no declaration, gives null and the default tile. validateManifestIcon covers icon only, so brand needed its own check.
  • brand is an implementation extension: spec-collection-profile.md in this repo does not define it. It is operator-trusted manifest data, not connector-asserted identity, and the doc comment on ConsentPickerManifest says so.
  • ConsentPickerManifest declares the optional brand and icon fields, so the compiler checks the capability boundary; the projection reads them without a cast.
  • ConsentSourceIcon in consent-screen-model.ts mirrors the field. The shared mark validates it as a hex color and paints it as the tile background.

Relation to #620

#620 resolves each manifest's display mark, backdrop included, once in the console (resolve-manifest-display-mark.ts). The consent challenge is built by the authorization server from its own manifest reader, which the console cannot call, so this projection is that boundary for the consent screen and is still needed. It follows the same rule: the declared brand.background_color travels with the supplied mark, and a source with no mark falls back to the bundled one.

Tested

  • New consent-screen-runtime.test.mjs case: a manifest with a white glyph and brand.background_color: "#1DB954" goes through the real projection and the real screen, and the mark tile's computed background is rgb(29, 185, 84). Before the change it is rgba(0, 0, 0, 0); the test fails there.
  • Red then green: six cases (transparent, white, rgb(0 0 0 / 0), a declaration break-out string, a number, an empty string) failed before hexColorOrNull (the model carried the raw value) and pass after. Also pinned: a short hex is accepted, and a brand background with no manifest glyph is not applied to the resolved identity mark.
  • From apps/console, npm run test:files on the consent-screen directory: 56 pass, 0 fail.
  • From reference-implementation, node --import tsx --test on hosted-mcp-picker-scope-render, hosted-mcp-picker-render, hosted-mcp-selection and consent-challenge-csrf: 69 pass, 0 fail (re-run after the validation change).
  • node --import tsx --test test/connector-manifest-validation.test.ts: 51 pass, 0 fail (the manifest icon.color check now uses the shared function).
  • tsc --noEmit in reference-implementation and npm run types:check in apps/console: clean.

Assisted-by: AI

@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from 63923e6 to 9d35610 Compare November 5, 2026 21:34
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch from 9add160 to 681488b Compare November 5, 2026 21:34
@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from 9d35610 to d698bcc Compare November 5, 2026 22:46
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch 2 times, most recently from 50dd3ea to 86fd5e1 Compare November 5, 2026 23:10
@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from e3633fe to 5f47de1 Compare November 6, 2026 06:13
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch 2 times, most recently from b0c6fb1 to 33eea12 Compare November 6, 2026 06:20
@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from af740a0 to f8ec588 Compare November 6, 2026 13:14
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch from 4e9aea1 to 3c75171 Compare November 6, 2026 13:15
@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from f8ec588 to 0de0510 Compare November 6, 2026 13:25
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch from 3c75171 to bfa3dfe Compare November 6, 2026 13:25
@tnunamak
tnunamak force-pushed the feat/console-connector-identity-icons branch from 0de0510 to c5f7d87 Compare November 6, 2026 14:12
…sent mark

The consent projection copied only color, kind and svg from the manifest
icon and dropped brand.background_color, so a white glyph rendered on the
default tile and was invisible. Carry the declared background through the
server model and the console model; the shared mark renderer already
paints it.

Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The white-glyph test only checked that some SVG existed, so a fallback
mark would have passed the glyph check. It now reads the rendered path
fill and requires the manifest's white. It also no longer passes
connectorIndex, which ConsentScreen does not accept.

Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
buildHostedMcpConsentChallengeModel read `brand.background_color` and `icon`
through a cast to a second manifest shape, while getConnectorManifest
promised a ConsentPickerManifest that declared neither field. A producer
with an incompatible brand value therefore still compiled.

Declare the optional readonly `brand` and `icon` fields on
ConsentPickerManifest and read them directly, so the compiler checks both at
the capability boundary. Runtime behavior is unchanged. A manifest whose
`brand.background_color` is a number now fails to compile; with the cast it
compiled.

Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
brand.background_color reached the browser unchecked: validateManifestIcon
covers icon, not brand. A value such as "transparent" painted no backdrop
behind a white glyph, which hides the mark again. The consent model now
passes the value through hexColorOrNull and falls back to the default tile
for anything that is not #rgb or #rrggbb.

Document that brand is an implementation extension and operator-trusted
manifest data, and pin the fallback, short hex and no-glyph cases.

Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
hexColorOrNull and its regex lived in the manifest validator while the
console kept its own 6-digit regex for the same job. The check now lives
in @pdpp/brand-react (hex-color.ts) and both the server and the console
import it, so a color accepted by one is accepted by the other. The
manifest icon.color check uses it too; behavior is unchanged.

Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
@tnunamak
tnunamak force-pushed the fix/consent-icon-background branch from bfa3dfe to 89240c1 Compare November 6, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant