Skip to content

[THROWAWAY] feature-review self-test: #545 secondary link as button - #551

Open
sanrai wants to merge 1 commit into
mainfrom
fr-test-545
Open

[THROWAWAY] feature-review self-test: #545 secondary link as button#551
sanrai wants to merge 1 commit into
mainfrom
fr-test-545

Conversation

@sanrai

@sanrai sanrai commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Throwaway. Verifies the agent can lift #545's Group.spec.js test and assert it on prod (infobit link with style primary/call-to-action renders as a button).

@github-actions

Copy link
Copy Markdown

🧪 Feature QA review — injected feature test (advisory, non-blocking)

Overall: FAIL

Injected the PR build with a forced config + a crafted collection to exercise the feature.

Source test: Consonant/Infobit/Group.spec.js > renders a link infobit with style "primary"/"call-to-action" as a button (not a plain text link) — reproduced per PR #545 feature-review target
Config (truncated): {"collection":{"mode":"lightest","layout":{"type":"3up","gutter":"4x","container":"1200MaxWidth"},"button":{"style":"primary"},"resultsPerPage":12,"endpoint":"https://www.adobe.com/chimera-api/collection?originSelection=bacom&contentTypeTags=&secondSource=&secondaryTags=&collectionTags=&excludeContentWithTags=caas:events/max,caas:content-type/product&language=en&country=xx&complexQuery=((%22caas%3Acontent-type%2Fcustomer-story%22))&excludeIds=&currentEntityId=&featuredCards=&environment=&draft=f ...
Fixture cards injected: 3
Expected: On card 'Alpha Story: Button Link Test' the footer link infobit with style 'primary' (text 'Explore the story', url .../customer-story-1) renders as a button-styled anchor (class containing 'consonant-Button', e.g. 'consonant-Button--primary'), NOT as a plain 'consonant-Link'/text infobit. Likewise card 'Bravo Story: Call To Action Link' with style 'call-to-action' (text 'Read the case study') also renders as a button-styled anchor. By contrast, card 'Charlie Story: Plain Link Control' whose footer link has no 'style' field (text 'Read more') renders as a plain text link, not a button, confirming the style-driven button behaviour is what causes the difference.
Rendered (first collection):

    1. Alpha Story: Button Link Test
    1. Bravo Story: Call To Action Link
    1. Charlie Story: Plain Link Control

Verdict: Observed rendered cards show empty 'links' arrays for all three cards, meaning no anchor elements (button-styled or plain) were rendered at all, so neither the expected button-styled CTAs for cards 1 and 2 nor the plain link for card 3 are present.

Screenshot in the workflow run.

@github-actions

Copy link
Copy Markdown

🤖 Agent QA review — interactive + visual diff (advisory, non-blocking)

Drove the PR build on the live business.adobe.com collection (filtered, searched, paginated, inspected cards), guided by a PR-vs-stable visual diff (0.00% of pixels changed) and the PR code diff. Verdict: PASS.

Tested PR #551 (throwaway self-test PR; changed files were mostly QA-harness infra plus test-only Group.spec.js/general.js additions) on the live https://business.adobe.com/resources/main.html page with the PR's CaaS build injected.

Visual diff review: the provided diff.png showed 0.00% pixel change / no magenta regions, confirming the PR introduces no visible rendering change on this page — consistent with it being a test-infra-only throwaway PR.

Manual verification performed:

  1. Page load: Resource Center page rendered correctly, header/nav, hero, industry icon row, and "Explore all Adobe resources" card grid all displayed normally. No console errors on load.
  2. Infobit CTA buttons (the feature the lifted Group.spec.js test targets — "secondary link as button"): inspected multiple cards' "Read article" / "Read now" links. All render as pill-shaped outlined buttons with class consonant-BtnInfobit, proper href, aria-label, daa-ll analytics attribute, and tabindex="0". Visually consistent, no unstyled/broken links found. All sampled links used the same class regardless of individual card style, i.e. no regression introduced — behavior matches current production styling used on this page's live data set.
  3. Filters panel ("Refine Your Results"): expanded "Content Type" filter group successfully — checkbox list (Blogs, Customer Blogs, Customer Stories, Demos and Videos, Event Sessions, Guides, Infographics, Interactive Experiences, Reports, Webinars) rendered correctly with proper expand/collapse chevron toggle. Search box and Clear button present and functional-looking.
  4. Accessibility: ran axe-core scoped to the cards grid — 20 passes, 1 moderate violation: heading-order ("Heading levels should only increase by one" on a card title using aria-level="6"). This is a pre-existing structural issue unrelated to this PR (confirmed by 0% pixel diff / PR touches no card-title heading logic).
  5. Console: no JavaScript errors observed at any point (initial load, after filter expand).

No layout breakage, overlap, truncation, or misalignment was observed anywhere in the tested regions (hero, card grid, filter panel, CTA buttons, floating chat widget).

Conclusion: The PR is a throwaway/self-test change (QA harness + test files only) and produces no functional or visual regression on the live business.adobe.com resources collection. The one existing axe violation (heading-order) is pre-existing and not introduced by this PR. Everything else checked out clean.

PR / stable / diff screenshots + console + axe artifacts in the workflow run.

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. New CI workflow executes untrusted PR code on a persistent, credentialed self-hosted runner

qa-feature-review.yml runs gh pr checkout + npm ci && npm run build for the PR's own code, then runs feature-review.mjs, on self-hosted, qa-audit — described as "the only runner with the persistent Chrome/CDP". This means:

  • Any PR from the same repo (not just forks) can execute arbitrary code (via npm ci/build scripts, e.g. postinstall) on a long-lived runner that has access to IMS_ACCESS_TOKEN, LLM_PROXY_ENDPOINT, and GITHUB_TOKEN (with pull-requests: write), and to a persistent authenticated browser session over CDP.
  • A malicious/compromised branch could exfiltrate these secrets or hijack the shared CDP browser session (which may hold real cookies/auth for business.adobe.com).

This is a high-risk pattern (untrusted code + self-hosted runner + secrets + shared stateful browser) and should be gated behind required review/approval (e.g. environment protection) before merge, not run automatically on opened/synchronize.

2. Attacker-influenced content is injected into a live, shared browser session without sanitization

feature-review.mjs builds LLM prompts from the PR's own diff/spec text (fully attacker-controlled), parses the LLM's JSON reply as config/cards, then:

  • writes it to localStorage and mocks chimera-api/collection responses,
  • navigates the shared, persistent CDP browser context to a real production URL (https://business.adobe.com/...),
  • injects the resulting cards (titles, hrefs, footer text, etc.) into the DOM of that live page.

Since the LLM output is derived from PR-controlled input (classic prompt-injection vector) and is never validated/sanitized before being used as href/text/config values rendered into a real page in a session that may carry real auth cookies, this is a plausible XSS / session-hijack vector, not just a test artifact. At minimum this should run in an isolated, ephemeral, unauthenticated browser context — never the shared "persistent Chrome" session.

3. browser.close() may kill the shared persistent CDP browser

The script calls browser.close() on a browser obtained via chromium.connectOverCDP(CDP). Depending on Playwright version/behavior, closing a CDP-connected Browser can terminate the actual browser process rather than just disconnecting. Given the comment that this runner has "the persistent Chrome/CDP" (implying it's shared across runs/jobs), this could break subsequent QA runs. Use ctx.close() / close only the pages/contexts you created, not browser.close().

Minor (non-blocking, mentioned for completeness)

  • Group.spec.js has a stray // feature-test target: 545 comment with no clear purpose — looks like leftover tooling metadata, should be removed.

Not a concern

The general.js changes (window.__caasQaConfigs capture, _caasQaReplace) are correctly gated behind the ?caasqa param, wrapped in try/catch, and match the added unit tests — no correctness bug found there.

@github-actions

Copy link
Copy Markdown

🧪 Feature QA review — injected feature test (advisory, non-blocking)

Overall: SKIPPED

Scenario mapping was not proven -- skipped instead of guessing.

The actual diff only changes applyQaConfigOverride (general.js) and adds a no-op comment to Group.spec.js; it does not modify Button.jsx/isCtaButton or any Infobit rendering logic. The provided research never located the call site where applyQaConfigOverride (or its window.__caasQaConfigs / _caasQaReplace behavior) is invoked within the CardsGrid/Collection initialization pipeline that consumes the card-grid config JSON supplied for this page. Without that traced injection path, mapping the ?caasqa/localStorage override test onto the given collection configs or onto card fixtures would be a guess, so per instructions I cannot produce a fixture.

Code searches performed: 6.

Screenshot in the workflow run.

@github-actions

Copy link
Copy Markdown

🧪 Feature QA review — injected feature test (advisory, non-blocking)

Overall: SKIPPED

Scenario mapping was not proven -- skipped instead of guessing.

The changed test exercises applyQaConfigOverride's localStorage/URL-param driven config capture and _caasQaReplace override path in general.js. The provided research only traced Group/CardFooter/Infobit rendering (an unrelated component chain from a different feature) and never searched for or located the call site(s) where applyQaConfigOverride is invoked with a collection's live config during page/collection initialization. Without that call-site evidence, there is no proven injection path from the test's inputs (URL param ?caasqa, localStorage 'caasQaConfig', and window.__caasQaConfigs) into the page's card-grid config or DOM output, so a fixture would be guessed rather than sourced.

Code searches performed: 8.

Screenshot in the workflow run.

@github-actions

Copy link
Copy Markdown

🧪 Feature QA review — injected feature test (advisory, non-blocking)

Overall: PASS

Injected the PR build with a searched, source-grounded config + collection fixture.

Source test: react/src/js/components/Consonant/Infobit/__test__/Group.spec.js > Should render Link infobit with style "primary" as a Button
Code searches: 8
Mapping evidence: react/src/js/components/Consonant/Cards/CardFooter/CardFooter.jsx:168, react/src/js/components/Consonant/Cards/Card.jsx:262, react/src/js/components/Consonant/Cards/OneHalf.jsx:78, react/src/js/components/Consonant/Infobit/__test__/Group.spec.js:100
Fixture cards: 1
Expected: A rendered link infobit with style 'primary' inside the card footer is rendered as a button element (data-testid='consonant-BtnInfobit'), with href='https://example.com' and text content 'Primary Link'
Rendered (first collection):

    1. Primary Link Card [consonant-BtnInfobit https://example.com]

Verdict: The rendered footer contains an element with data-testid='consonant-BtnInfobit' (an tag) with href='https://example.com' and text content 'Primary Link', matching the expected values from the test.

Screenshot in the workflow run.

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