Phase 1 hardening + UI hygiene by borisstoyanov · Pull Request #4 · shapeblue/QA-Portal

borisstoyanov · 2026-06-05T08:27:42Z

Summary

Phase 1 of the architecture-review roadmap plus a UI-hygiene pass. Defensive + structural; no change to the data shown.

Security (server)

helmet, express-rate-limit (120/min global on /api/, 10/min on the artifact proxy), CORS scoped to CORS_ORIGIN
Artifact proxy validates numeric artifactId before spending the token; trust proxy for real client IPs

Observability (server)

All console.* → structured pino; schema-drift .catch blocks log WARN naming the source (ADR-0001)
/api/health pings the DB (SELECT 1), returns 503 when down; [DEBUG] noise removed

UI hygiene (client)

Fix version contradiction (v0.1.1 header vs v1.0.4 footer) → single APP_VERSION constant
Remove dead ReadyToMerge component family + orphaned CSS (8 files, ~350 lines); drop dead getReadyToMergePRs
Unify refresh icon (↻)
Fix pre-existing lint warnings so npm run build passes under CI=true (unused imports/vars; mount-only useEffect)

Docs / decisions

CONTEXT.md glossary; docs/adr/0001-scraper-is-system-of-record.md; docs/adr/0002-public-dashboard-no-auth.md
.env.example documents LOG_LEVEL / CORS_ORIGIN

Verification

Server: tsc clean, 19 tests pass
Client: tsc clean, 3 tests pass, production build passes under CI=true

Separate npm audit flags 8 pre-existing vulnerabilities unrelated to these deps.

🤖 Generated with Claude Code

- server: /api/prs/:prNumber/test-failures replaced the per-failure COUNT query loop with a single grouped query; added isNaN(prNumber) guard returning 400. - client: replaced the dead CRA boilerplate test (asserted "learn react") with real smoke tests for the header, the four nav tabs, and the mount API call. Resolved values set in beforeEach because CRA's Jest config sets resetMocks:true. npm test now passes, so deploys no longer need --skip-tests. - repo: removed check_staleness.js (one-off debug) and scrape-github-prs.js.backup; moved point-in-time work-logs into docs/history/; gitignore now excludes *.backup / *_temp.js / *.temp.js. - add CLAUDE.md for future Claude Code sessions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The smoketest and code-coverage parsing logic was copy-pasted across three handlers (getHealthPRs, getPR, getAllOpenPRs) and had drifted into three subtly different implementations. Consolidate into pure, unit-tested modules and call them from all three sites. - server/src/parsers/trillian.ts, codecov.ts: single source of truth, no DB/network access. Canonical behaviour follows the most complete variant (the /api/pr/:number handler): skipped ("did not run") tests count toward the total, and a run is FAIL only when there is >=1 error. - Fixes a latent bug in the all-open-PRs path, which used `passed === total` and so marked otherwise-passing runs as FAIL whenever any tests were skipped. - Unify code-coverage parsing (all-open-PRs previously used a different change regex and a GitHub URL instead of the codecov.io link). - 26 unit tests via node:test + ts-node; test files excluded from the tsc build. `npm test` now runs server then client. - Net: ~360 lines of duplicated parsing removed from index.ts. Behaviour changes to validate against production data: - all-open-PRs smoketest status now reflects errors, not skipped tests. - all-open-PRs coverage now links to codecov.io and parses +/- change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Add helmet, per-IP rate limiting, and origin-scoped CORS - Harden artifact proxy: numeric ID validation + strict rate limit - Replace console.* with structured pino logging; log schema drift loudly - Make /api/health ping the DB (503 when down) - Document scraper-as-system-of-record and public-no-auth decisions (ADRs) - Add CONTEXT.md glossary Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Phase 1 hardening of the QA Portal backend: introduces baseline HTTP security protections and better operational behavior (logging, health checks), while also reducing drift by centralizing bot-comment parsing into dedicated pure parser modules.

Changes:

Added security middleware and controls on the Express API (helmet, scoped CORS, rate limiting, proxy-aware client IPs, artifact proxy input validation).
Replaced ad-hoc console.* logging with structured pino logging and improved schema-drift warning logs.
Refactored Trillian smoketest + Codecov parsing into tested, reusable parser modules; adjusted build/test configuration to run server tests.

Reviewed changes

Copilot reviewed 15 out of 25 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
server/tsconfig.json	Excludes server `*.test.ts` from `tsc` compilation output.
server/src/parsers/trillian.ts	New single-source Trillian/BlueOrangutan smoketest parser utilities.
server/src/parsers/trillian.test.ts	Unit tests for the Trillian smoketest parser behavior.
server/src/parsers/codecov.ts	New single-source Codecov comment parser.
server/src/parsers/codecov.test.ts	Unit tests for Codecov parsing cases.
server/src/index.ts	Adds helmet/CORS/rate-limits/pino logging/health DB ping; uses new parser modules; optimizes N+1 query.
scripts/scrape-github-prs.js.backup	Removes an old backup copy from the repo.
package.json	Adds server+client test scripts; adds new backend dependencies (helmet/rate-limit/pino).
package-lock.json	Locks new dependencies and transitive updates (notably Node engine constraints).
docs/history/QA_TESTING_REPORT_PACKAGE_STALENESS.md	Adds historical QA report documentation.
docs/history/MILESTONE_ROLLBACK_PLAN.md	Adds historical rollback plan documentation.
docs/history/MILESTONE_IMPLEMENTATION_SUMMARY.md	Adds historical milestone implementation summary.
docs/history/MILESTONE_DEPLOYMENT_GUIDE.md	Adds historical deployment guide documentation.
docs/history/FIX_STALENESS_ISSUE.md	Adds historical write-up about a staleness issue.
docs/history/FEATURE_AVAILABLE_PACKAGES_COMPLETE.md	Adds historical available-packages feature documentation.
docs/history/CONTEXT_PROMPT.md	Adds historical context restoration prompt documentation.
docs/history/.github-copilot-setup.md	Adds historical Copilot setup notes.
docs/adr/0002-public-dashboard-no-auth.md	Adds ADR documenting intentional lack of auth + mitigation strategy.
docs/adr/0001-scraper-is-system-of-record.md	Adds ADR defining scraper as system-of-record and API drift behavior.
CONTEXT.md	Adds shared glossary/definitions for domain terms used in the portal.
client/src/App.test.tsx	Updates App tests to stub API calls and assert key UI navigation elements.
CLAUDE.md	Adds repository guidance for Claude Code (commands/architecture/constraints).
check_staleness.js	Removes a scratch staleness-check script.
.gitignore	Ignores scratch/temp/backup files.
.env.example	Documents new `LOG_LEVEL` and `CORS_ORIGIN` environment variables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+// Behind nginx in production — trust the proxy so rate-limit sees the real client IP.
+app.set('trust proxy', 1);


+// CORS scoped to the portal's own origin. CORS_ORIGIN may be a comma-separated list;
+// if unset we fall back to reflecting any origin (dev convenience only).
+const corsOrigins = process.env.CORS_ORIGIN
+  ? process.env.CORS_ORIGIN.split(',').map(o => o.trim())
+  : undefined;
+app.use(cors(corsOrigins ? { origin: corsOrigins } : {}));


+// Structured logger. Set LOG_LEVEL=debug locally for verbose output.
+const logger = pino({ level: process.env.LOG_LEVEL || 'info' });


+    "express-rate-limit": "^8.5.2",
+    "helmet": "^8.2.0",
+    "mysql2": "^3.15.3",
+    "pino": "^10.3.1",
+    "pino-http": "^11.0.0"


    ).catch(err => {
-      console.warn('Could not fetch approvals:', err.message);
+      logger.warn({ err: err.message, source: 'pr_approvals' }, 'Schema drift: could not fetch approvals');
      return [];


    ).catch(err => {
-      console.warn('Could not fetch approvals:', err.message);
+      logger.warn({ err: err.message, source: 'pr_approvals' }, 'Schema drift: could not fetch approvals');
      return [];


    ).catch(err => {
-      console.warn('Could not fetch labels:', err.message);
+      logger.warn({ err: err.message, source: 'pr_health_labels' }, 'Schema drift: could not fetch labels');
      return [];


    ).catch(err => {
-      console.warn('Could not fetch package builds:', err.message);
+      logger.warn({ err: err.message, source: 'pr_package_builds' }, 'Schema drift: could not fetch package builds');
      return [];


  } catch (error: any) {
-    console.error('Error downloading artifact:', error.message);
+    logger.error({ err: error.message }, 'Error downloading artifact');
    if (error.response?.status === 410) {


+ * Counts are read from the summary line, e.g. "141 look OK, 0 have errors,
+ * 3 did not run". Skipped ("did not run") tests are included in the total so
+ * that the pass ratio reflects the full suite. A run is FAIL only when there
+ * is at least one error — skipped tests alone do not fail a run.
+ */


- Single APP_VERSION constant (was v0.1.1 in header vs v1.0.4 in footer) - Delete unused ReadyToMerge component family + orphaned CSS (8 files); drop dead getReadyToMergePRs API method and its test mock - AllPRsView refresh icon ↻ to match the health tab Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Remove unused imports/vars: Routes/Route (App), isFresh (AllPRsView), stats state + UpgradeTestStats import + dead formatDate/getRelativeTime (UpgradeTests); drop the now-unused upgrade-stats fetch - Mark the mount-only useEffect with an exhaustive-deps disable Enables gating deploys on lint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

CRA treats warnings as errors under CI=true, so a lint regression now fails the deploy step (set -e) instead of shipping. Client build is already CI-clean as of the previous commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Global :focus-visible outline so keyboard focus is visible everywhere - Shared clickable() helper (role=button, tabIndex, Enter/Space) applied to the previously mouse-only divs: AllPRsView stat-box filters, PRCard smoketest expanders, TestResults file accordions, UpgradeTests accordion headers + test rows + heatmap cells - aria-expanded on expandable controls; aria-pressed on filter toggles - aria-label on emoji-only status badges and the package-status icon so meaning isn't conveyed by glyph/color alone Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Add :root token palette (border/text/bg/primary/accent/success/danger + shadow/radius scales) in index.css. Token values equal the hexes they replace, so the light theme is visually unchanged. - Migrate the 15 most-repeated colors (~200 occurrences) to var() across App/AllPRsView/PRCard/SearchBar/UpgradeTests CSS via exact-match replace. - Focus outline now uses var(--color-primary). Foundation for a prefers-color-scheme dark mode (override tokens in one place) and centralized contrast fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…lanking loads - Tab bar: drop inconsistent emoji, add count badges (Health, All Open, Flaky) - Shared RefreshControls: "Updated Xm ago" + auto-refresh toggle + refresh, used on Health and All-PRs tabs (timeAgo/useNow util) - All-PRs: free-text filter by #, title, or assignee - Loading no longer blanks the screen on refresh — stale content stays with a slim top progress bar; full spinner only on first load - App fetches lightweight counts (all-open + flaky) for the badges Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The old `auto 1fr` grid centered the title inside the right column, pushing it off-center while the logo floated mid-left. Use a symmetric `1fr auto 1fr` grid so the logo is left-aligned and the title is centered across the header. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Add --surface token; migrate white card/table backgrounds, hardcoded dark grey text, and near-white fills to tokens so they theme correctly - Add @media (prefers-color-scheme: dark) overriding the token values; the structural chrome flips automatically since components reference tokens - A few vivid status/heatmap accent colors remain untokenized (flagged for QA) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Records the token-based theming approach, the dark-mode override mechanism (prefers-color-scheme + data-theme), the data-viz (heatmap) exemption, and the WCAG AA contrast target — decisions reached in a design-grilling session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Retune design tokens to one blue system (primary #1f6feb) so links, tabs, focus rings, and stat numbers stop clashing (was purple header + orange numbers + a third blue for links) - Redesign header: brand-blue gradient via --header-bg, slimmer, softer shadow - Recolor TestResults stat numbers orange -> primary blue; map flaky success/failure/error counts to semantic tokens - Refine neutrals (AA-tuned greys, cooler borders/bg), softer shadow tokens, add --color-warn / --header-bg / --color-primary-strong (light + dark) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Drop the 1200px max-width on .header-content so the logo sits at the left gutter and the symmetric grid centers the title to the whole screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

borisstoyanov and others added 3 commits June 5, 2026 10:26

Copilot AI review requested due to automatic review settings June 5, 2026 08:27

Copilot started reviewing on behalf of borisstoyanov June 5, 2026 08:27 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

borisstoyanov and others added 2 commits June 5, 2026 11:41

borisstoyanov changed the title ~~Phase 1: security & ops hardening~~ Phase 1 hardening + UI hygiene Jun 5, 2026

borisstoyanov and others added 9 commits June 5, 2026 11:46

Header: pin logo to far left, center title across full width

cca15b2

Drop the 1200px max-width on .header-content so the logo sits at the left gutter and the symmetric grid centers the title to the whole screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Phase 1 hardening + UI hygiene#4

Phase 1 hardening + UI hygiene#4
borisstoyanov wants to merge 14 commits into
mainfrom
chore/now-tier-fixes

borisstoyanov commented Jun 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Behind nginx in production — trust the proxy so rate-limit sees the real client IP.
		app.set('trust proxy', 1);

		// Structured logger. Set LOG_LEVEL=debug locally for verbose output.
		const logger = pino({ level: process.env.LOG_LEVEL \|\| 'info' });

Uh oh!

Conversation

borisstoyanov commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Security (server)

Observability (server)

UI hygiene (client)

Docs / decisions

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

borisstoyanov commented Jun 5, 2026 •

edited

Loading