From 2b3c84bfefbf3e73033477c53189bf7fe47c58a2 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:58:59 -0700 Subject: [PATCH 1/2] test(v2): pin the connector mark and the Tools glyph to the same size (TASK-177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing carried the agreement between the two glyphs: the mark is 14px through `size={14}` at its call site, and the Tools glyph is 14px through V2ConnectorTools' own local `G` (:157, hardcoded width/height) — it never imports MarkGlyph, and icons/glyphs.tsx's `G` defaults to 16. Two independent literals in two files, no shared source, no test: drop the prop and the mark renders 16 while Tools stays 14, silently desyncing the rows v2.css claimed cannot drift. The pin is the agreement, not a 14 in one file. The test renders the page (which mounts V2ConnectorTools at :1545) with one live grant and the Slack row, asserts both glyph nodes exist and carry digit attributes as a positive control, then compares width and height. Mutation-verified: dropping `size={14}` -> red (16 vs 14); Tools' literal 14 -> 16 -> red; unmutated and restored -> green. Full frontend suite 113 suites / 1009 tests green; tsc --noEmit clean. Three comments were false and are corrected: v2.css said the mark 'takes the Tools glyph's rule below (14px...)' and that the shared rule means 'the two rows cannot drift'; V2ConnectorsPage.tsx said the glyph 'rides .v2-tools__mode's 14px rule'; the layout-invariants comment said the size 'comes from the shared rule above plus the glyph's size prop'. The shared rule is layout and colour only — it carries no dimensions, so it could never have sized either glyph. --- .../v2/__tests__/V2ConnectorsPage.test.tsx | 53 +++++++++++++++++++ .../v2/__tests__/v2-layout-invariants.test.ts | 2 +- .../src/v2/components/V2ConnectorsPage.tsx | 7 ++- frontend/src/v2/v2.css | 15 ++++-- 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx index 94d90b450..44b926451 100644 --- a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx @@ -122,6 +122,59 @@ describe('V2ConnectorsPage', () => { expect(row?.querySelector('.v2-connector-row__when')).toBeNull(); }); + // TASK-177. The mark and the Tools glyph agree at 14px through TWO independent + // literals in two files: `size={14}` at the mark's call site, and + // V2ConnectorTools' own local `G` (`:157`, `width="14" height="14"`) — it does + // not import `MarkGlyph`, and `icons/glyphs.tsx`'s `G` defaults to 16. + // So nothing shared carries the agreement and nothing tested it: drop the prop + // and the mark renders 16 while Tools stays 14, silently desyncing the two rows + // v2.css claims cannot drift. And a `14` asserted in one file would have passed + // through every misreading of this chain, so what is pinned is the AGREEMENT: + // whichever number the design lands on, both glyphs must carry it. The absolute + // size stays the gate's decision, not this test's. + it('TASK-177: the connector mark and the Tools mode glyph render at the same size', async () => { + const toolsEntry = { + installableId: 'github', list: 'tools', label: 'GitHub', description: 'Issues and pull requests.', available: true, + broker: { id: 'commonly-grant-broker' }, + tools: [{ name: 'github.list_issues', requiredWriteMode: 'read', irreversible: false }], + connections: [], + }; + const grant = { + grantId: 'grant_live', installationId: 'inst-1', target: { kind: 'pod', id: 'p1' }, tools: ['github.list_issues'], + writeMode: 'read', budget: { calls: 50, windowMs: 3600000 }, effectiveAudience: [], + expiresAt: new Date(Date.now() + 6 * 86400000).toISOString(), revokedAt: null, revokedBy: null, + parentGrantId: null, rootGrantId: null, createdAt: new Date().toISOString(), grantedBy: 'u1', + }; + axios.get.mockImplementation((url) => { + if (url === '/api/integrations/user/all') return Promise.resolve({ data: [connectors[1]] }); + if (url === '/api/pods') return Promise.resolve({ data: [{ _id: 'p1', name: 'Launch pod', type: 'chat' }] }); + if (url === '/api/installables') return Promise.resolve({ data: { installables: [toolsEntry] } }); + if (url === '/api/pods/p1/grants') return Promise.resolve({ data: { podId: 'p1', grants: [grant] } }); + if (url === '/api/registry/pods/p1/agents') return Promise.resolve({ data: { agents: [] } }); + if (url.includes('/calls')) { + return Promise.resolve({ data: { grantId: 'grant_live', calls: [], counts: { total: 0, ok: 0, refused: 0, pending_approval: 0, failed: 0 } } }); + } + return Promise.resolve({ data: [] }); + }); + const { container } = renderPage(); + // Both glyphs have to actually BE there, or the comparison would pass on two + // absent nodes — the vacuity the same-size claim is easiest to fake. + const markSvg = await waitFor(() => { + expect(container.querySelector('.v2-connector-row__mark svg')).not.toBeNull(); + return container.querySelector('.v2-connector-row__mark svg') as SVGElement; + }); + const modeSvg = await waitFor(() => { + expect(container.querySelector('.v2-tools__mode svg')).not.toBeNull(); + return container.querySelector('.v2-tools__mode svg') as SVGElement; + }); + // Positive control: these attributes are the instrument, so prove they carry a + // value at all (an empty string would make the equality below meaningless). + expect(markSvg.getAttribute('width')).toMatch(/^\d+$/); + expect(modeSvg.getAttribute('width')).toMatch(/^\d+$/); + expect(markSvg.getAttribute('width')).toBe(modeSvg.getAttribute('width')); + expect(markSvg.getAttribute('height')).toBe(modeSvg.getAttribute('height')); + }); + it('TASK-162: line 3 never repeats the mode word the kicker already carries', async () => { // The prefixes lived in the component's defaultValue, which is exactly where // they could not be guarded: once the key exists in the catalog the catalog diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index f20f47ed4..c25dc5a94 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -1860,7 +1860,7 @@ describe('v2 layout invariants (CSS rule presence)', () => { expect(sharedMarkRule?.[2]).toContain('display: inline-flex'); expect(sharedMarkRule?.[2]).toContain('vertical-align: -2px'); expect(sharedMarkRule?.[2]).toContain('color: var(--v2-text-tertiary)'); - // Rule 3: the kicker is mono 11; rule 1 (as revised by the TASK-162 gate): the mode word is hidden until 760 and the mark's own 16px rule is GONE — its size now comes from the shared rule above plus the glyph's `size` prop; rule 2: the gear is 32 (44 on the phone). + // Rule 3: the kicker is mono 11; rule 1 (as revised by the TASK-162 gate): the mode word is hidden until 760 and the mark's own 16px rule is GONE — the shared rule carries no dimensions, so the size is the glyph's `size` prop alone, pinned same-size against the Tools glyph in V2ConnectorsPage.test.tsx (TASK-177); rule 2: the gear is 32 (44 on the phone). expect(ruleBody(v2, '.v2-connector-row__kicker')).toContain('var(--v2-font-mono)'); expect(ruleBody(v2, '.v2-connector-row__kicker')).toContain('font-size: 11px'); expect(ruleBody(v2, '.v2-connector-row__kicker-mode')).toContain('display: none'); diff --git a/frontend/src/v2/components/V2ConnectorsPage.tsx b/frontend/src/v2/components/V2ConnectorsPage.tsx index 57487b2bb..27002a00e 100644 --- a/frontend/src/v2/components/V2ConnectorsPage.tsx +++ b/frontend/src/v2/components/V2ConnectorsPage.tsx @@ -1067,8 +1067,11 @@ const V2ConnectorsPage: React.FC = () => { // TASK-162 (3): the consequence is the visible words; the mode word // (mirror / attention / relay off) is what the glyph itself means, // so it rides title + aria-label rather than being the only thing - // on the line. The glyph rides .v2-tools__mode's 14px rule (see - // v2.css): #1782 says the channel mark follows the Tools glyph. + // on the line. The glyph's 14px is THIS call site's `size` prop — no + // rule carries it (the shared rule with .v2-tools__mode is layout and + // colour only), so a same-size pin in V2ConnectorsPage.test.tsx holds + // it to the Tools glyph's own 14 (TASK-177). #1782 says the channel + // mark follows the Tools glyph. {row.mark.label} diff --git a/frontend/src/v2/v2.css b/frontend/src/v2/v2.css index ffeeefe59..3a4071f47 100644 --- a/frontend/src/v2/v2.css +++ b/frontend/src/v2/v2.css @@ -9951,9 +9951,13 @@ body.modern-ui.v2-canvas { reachable only through the glyph's tooltip — and at ≤760 the mark is hidden, so line 3 disappeared entirely on a phone. The consequence is now TEXT beside the mark; the mode word moved onto the mark itself (title + aria-label), and - the glyph itself follows #1782: it takes the Tools glyph's rule below (14px, - gap 6, tertiary, vertical-align -2px) rather than a 16px rule of its own, - because beside a 12/16 mono sentence a 16px mark grows the line box to 18px. */ + the glyph follows #1782 — it takes the Tools glyph's rule below for layout and + colour (gap 6, tertiary, vertical-align -2px) rather than a rule of its own. + That rule carries NO dimensions: the 14px is `size={14}` on the call site, + held equal to the Tools glyph's own hardcoded 14 (V2ConnectorTools' local `G`) + by a same-size pin in V2ConnectorsPage.test.tsx (TASK-177). 16px would grow the + line box to 18px beside a 12/16 mono sentence, which is why it is 14 and not + the glyph's 16 default. */ /* TASK-162 (2): the not-yet row lists two channels. Above 760 they stack in the 140px name track; at ≤760 they join with ' · ' (the separator is the only element that switches, so nothing here depends on generated content). */ @@ -9966,7 +9970,10 @@ body.modern-ui.v2-canvas { .v2-connector-row__kicker-mode { display: none; } /* Direction A rule 1 (as revised by TASK-162): a category on the row is a mark, the sentence in title + aria-label — and the mark shares the Tools glyph's rule - (search `.v2-connector-row__mark, .v2-tools__mode`) so the two rows cannot drift. */ + (search `.v2-connector-row__mark, .v2-tools__mode`) for layout and colour. That + sharing does NOT carry the size: the two agree at 14px through two independent + literals in two files, so a same-size pin in V2ConnectorsPage.test.tsx is what + keeps them from drifting (TASK-177). */ .v2-root button.v2-connector-row__action, .v2-root a.v2-connector-row__action { justify-self: end; From 84389ee464f76fee8de27d239f101eb84adb769a Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:12:18 -0700 Subject: [PATCH 2/2] test(v2): control the height attribute too, not just width (TASK-177 gate follow-up) sprint-review's gate removed `height` from BOTH `G` components (icons/glyphs.tsx and V2ConnectorTools' local one) and the suite stayed green: both heights become null, null === null passes, and only width was doing any work. Both attributes on both glyphs now carry the /^\d+$/ control. Verified: strip height from both G components -> red (was green); control and restored -> green; full frontend suite 113 suites / 1009 tests green twice. --- frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx index 44b926451..a53e88d21 100644 --- a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx @@ -169,8 +169,13 @@ describe('V2ConnectorsPage', () => { }); // Positive control: these attributes are the instrument, so prove they carry a // value at all (an empty string would make the equality below meaningless). + // Both attributes, both glyphs: controlling only `width` left the height + // comparison able to pass on null === null — strip `height` from *both* `G` + // components and a width-only control stays green (sprint-review's gate). expect(markSvg.getAttribute('width')).toMatch(/^\d+$/); + expect(markSvg.getAttribute('height')).toMatch(/^\d+$/); expect(modeSvg.getAttribute('width')).toMatch(/^\d+$/); + expect(modeSvg.getAttribute('height')).toMatch(/^\d+$/); expect(markSvg.getAttribute('width')).toBe(modeSvg.getAttribute('width')); expect(markSvg.getAttribute('height')).toBe(modeSvg.getAttribute('height')); });