Skip to content

fix(mwpw-184989): send x-session-id to the LLM proxy and stop Feature QA failing silently - #599

Closed
raissanjay wants to merge 1 commit into
mainfrom
fix/qa-llm-session-id
Closed

fix(mwpw-184989): send x-session-id to the LLM proxy and stop Feature QA failing silently#599
raissanjay wants to merge 1 commit into
mainfrom
fix/qa-llm-session-id

Conversation

@raissanjay

Copy link
Copy Markdown
Collaborator

What broke

On #591 the deterministic layer worked perfectly — build-output-diff published NO_CHANGE and review-score-gate auto-passed the PR with auto-pass: deps-dev + build-output-diff NO_CHANGE (bundle byte-identical).

No agent ever said so. Feature QA Review — the one wired to report exactly that — died at its first LLM call (run 32946357057, job 98107789143):

[llm] transport attempt 1 failed: HTTP 403: {"error":{"type":"missing_required_header","message":"Missing required header: x-session-id"}}
[llm] transport attempt 2 failed: HTTP 403: ... (same)
[llm] transport attempt 3 failed: HTTP 403: ... (same)
feature-review error: Error: llm failed after retries
    at llm (feature-review.mjs:156:9)
    at async feature-review.mjs:306:18

The proxy started requiring x-session-id. Only qa-runner-v2.mjs ever sent it — which is why Agent QA Review kept working (it shells out to that runner) while every direct caller began 403ing.

Then .catch(e => { console.error(...); process.exit(0) }) at the bottom of feature-review.mjs turned the crash into a green job with no comment and no notification. Nothing anywhere said the run had failed.

What this changes

1. Send the header. x-session-id (one randomUUID() per process, same shape as qa-runner-v2.mjs) added to all eight direct proxy callers:

feature-review.mjs · feature-backtest-worker.mjs · feature-backtest-batch.mjs · action-judge.mjs · ai-judge-test.mjs · mobile-probe.mjs · plan-probe.mjs · aggregate-report.mjs

2. Stop gating a deterministic fact behind a model. bundleDiffVerdict() is a commit-status read over the REST API — it needs no LLM at all. But it only runs after the triage LLM call, so a dead proxy took the bundle-diff report down with it. The triage call is now wrapped: on transport failure the agent falls through to postNonInjectable() and still reports SAFE (no bundle change) or NEEDS REVIEW (bundle changed).

That alone would have given #591 the comment it should have had.

3. Make silent outages visible. feature-review.mjs now writes a FEATURE_REVIEW_FAILED marker on an unhandled error, and qa-feature-review.yml logs it to the shared CI: AI / Agent review tool failures issue — the same monitor qa-agent-review.yml has had all along. The PR check still stays green; the failure just stops being invisible.

Notes

  • No product code touched — .github/qa/ and one workflow only.
  • .github/qa/ is outside the npm run lint path (react/src/js/components/Consonant), so no lint impact.
  • No new module, so qa-feature-review.yml's explicit reviewer copy-list is unchanged.
  • Verified with node --check across .github/qa/*.mjs and a YAML parse of the workflow.

Testing

The real check is the next run of this workflow on any PR: Feature QA Review should post a comment again instead of finishing green and silent. Worth a workflow_dispatch against #591 once this lands.

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

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

Last updated Aug 26, 2026, 2:00 AM PDT · new commit pushed · commit 69f4658 · 9 files changed.

0 open · 0 resolved · visual diff 0.00% · verdict PASS

Open findings

  • Smoke test passed ✅
What the agent checked

Tested PR #599 ("send x-session-id to LLM proxy / stop Feature QA failing silently") against the live business.adobe.com/resources/main.html CaaS collection page.

Context: This PR only touches CI/QA tooling scripts (.github/qa/*.mjs) and a GitHub Actions workflow YAML — it adds an x-session-id header to internal LLM-proxy calls used by automated review tooling. It does not touch any front-end CaaS rendering code, consonant components, or card/filter logic.

Diff check: Loaded the provided pixel diff (pr vs stable). No magenta/colored regions were visible anywhere in the image, consistent with the reported 0.00% pixel difference. This confirms the PR produces a byte-identical front-end bundle, as expected for a CI-only change.

Live page functional check (business.adobe.com/resources/main.html with PR build injected):

  • Page loads cleanly, header nav, breadcrumb (Home/Resource Center), and "Explore all Adobe resources" heading render correctly.
  • Card grid renders correctly with images, titles, descriptions, and CTAs (Watch now / Read now) properly aligned, no overlap or truncation.
  • Left filter panel ("Refine Your Results") expands/collapses correctly (tested "Products" filter group — expanded to show full checkbox list: Acrobat, Acrobat Sign, Advertising, Analytics, etc.).
  • Applying a filter (checked "Acrobat") worked as expected: result count updated from 3843 → 186, a removable filter chip "Acrobat ×" appeared, filter badge count "1" displayed on the Products group, and the card grid re-rendered with filtered results (e.g. "Empower your legal teams with AI workspaces" card appeared with green Acrobat-themed imagery). No layout shift issues or rendering glitches observed during filter application.
  • Search box, Clear button, Sort by dropdown, and other filter groups (Content type, Industry) were visible and structurally intact.
  • Floating chat widget ("What do you need help solving today?") rendered at bottom without obstructing card content.

Did not get to complete run_axe and get_console_errors calls before turn limit, but no visual console-error indicators, broken images, or layout breakage were observed during manual interaction.

Conclusion: Since this PR is CI-tooling-only (adds a required auth header for internal QA automation scripts) and does not modify any shipped front-end code, the 0% pixel diff is expected and correct. Live interactive testing of the resources collection (filters, search, results count, cards) shows the page functioning normally with no regressions attributable to this PR.

Verdict: PASS

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

Review history (2 runs)
  • 69f4658 · Aug 26, 2026, 2:00 AM PDT · new commit pushed · passing, no regressions — fix(mwpw-184989): send x-session-id to the LLM proxy and sto
  • 6cbd0e3 · Aug 26, 2026, 1:42 AM PDT · PR opened · passing, no regressions — fix(mwpw-184989): send x-session-id to the LLM proxy and sto

@github-actions

Copy link
Copy Markdown

AI Code Review

Last updated Aug 26, 2026 1:42 AM PDT · PR opened · commit 6cbd0e3 · 9 files changed in PR.

0 open · 0 resolved

Open findings

No open findings.

Review history (1 run)
  • 6cbd0e3 · Aug 26, 2026 1:42 AM PDT · PR opened · no changes (0 open) — fix(mwpw-184989): send x-session-id to the LLM proxy and stop Feature…

@github-actions

Copy link
Copy Markdown

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

Last updated Aug 26, 2026, 1:44 AM PT · commit 07acf31 · pull_request.

Overall: SKIPPED

Not an injectable feature -- skipped.

Triage model unavailable (llm failed after retries) -- reporting the deterministic bundle diff only.

This PR's change isn't driven by config/collection data the harness can force, and the build-output-diff verdict wasn't available to consult. (The visual/smoke review still applies.)

Screenshot in the workflow run.

Review history (1 run)
  • Aug 26, 2026, 1:44 AM PT · SKIPPED · 07acf31 · pull_request

… QA failing silently

The proxy now rejects requests without x-session-id (HTTP 403 missing_required_header). Only qa-runner-v2.mjs sent it, so Agent QA Review kept working while every direct caller started 403ing.

On PR #591 that killed feature-review.mjs at its first LLM call: three 403s, llm failed after retries, and the top-level catch exited 0. Green job, no comment - even though build-output-diff had already published NO_CHANGE and review-score-gate had auto-passed the PR.

- add x-session-id to all eight direct proxy callers

- feature-review.mjs: fall back to postNonInjectable when the triage model is unavailable, so the deterministic bundle-diff verdict is still reported (it is a REST status read, it never needed a model)

- feature-review.mjs: write FEATURE_REVIEW_FAILED on an unhandled error

- qa-feature-review.yml: log that marker to the shared monitor issue, mirroring qa-agent-review.yml, so a silent outage is visible

Claude-Session: https://claude.ai/code/session_01QimdQaYPSvXAaSDAssrfK1
@sanrai
sanrai force-pushed the fix/qa-llm-session-id branch from 6cbd0e3 to 69f4658 Compare August 26, 2026 08:48
@sanrai

sanrai commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Closing and reopening under @sanrai so the PR and commit are authored by the right account. No content change.

@sonarqube-acom-pr-deco-public

Copy link
Copy Markdown

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.

2 participants