Address PR review feedback: security, docs, error handling, tests

[Copilot]

Addresses 15 review comments from PR #292 covering security vulnerabilities, documentation inconsistencies, error handling improvements, and missing test coverage.

Security

• Cache key collision vulnerability: Replaced token suffix (last 8 chars) with SHA-256 hash to prevent cross-user request deduplication
• Added HTTP method to cache key to prevent GET/POST collisions

// Before: token suffix could collide
const cacheKey = `${token.slice(-8)}:${endpoint}:${JSON.stringify(body || {})}`;

// After: secure hash with method
const tokenHash = createHash('sha256').update(token).digest('hex').slice(0, 16);
const cacheKey = `${method}:${tokenHash}:${endpoint}:${JSON.stringify(body || {})}`;

Circuit Breaker

• HALF_OPEN state thundering herd: Added halfOpenInFlight counter to limit concurrent test requests (max 2)
• Error handling: Introduced CircuitBreakerOpenError class, replaced string matching with instanceof checks

// HALF_OPEN now rejects excess concurrent requests
if (this.state === 'HALF_OPEN' && this.halfOpenInFlight >= this.halfOpenMaxRequests) {
  throw new CircuitBreakerOpenError('Circuit breaker is testing recovery');
}

Error Handling

• Oversized response regression: Created UpstreamResponseTooLargeError class, restored specific 502 error message
• Circuit breaker detection: Replaced brittle message substring matching with instanceof CircuitBreakerOpenError

Documentation Consistency

• Rate limiter: Fixed MAX_CONCURRENT references (5 → 3) in code comments and docs
• Circuit breaker: Updated config docs (3 failures, 60s timeout, 2 test requests)
• Loading component: Corrected timeout (15s)
• OpenAPI spec: Version bump (1.4.9 → 1.5.0)
• EZYGO_VERIFICATION.md: Converted absolute paths to relative

UI/CSS

• Course card: Fixed invalid wrap-break-word → break-words
• Dropdown menu: Fixed Tailwind arbitrary variants (data-disabled → data-[disabled], data-inset → data-[inset])

Health Check

• Capacity logic: Changed < to <= comparison and incorporated queueLength to avoid false degraded status at full utilization

const atOrBelowCapacity = rateLimiterStats.activeRequests <= rateLimiterStats.maxConcurrent;
const hasBacklog = rateLimiterStats.queueLength > 0;
const isHealthy = !circuitBreakerStatus.isOpen && atOrBelowCapacity && !hasBacklog;

Tests

Added 28 unit tests (13 circuit breaker + 15 rate limiter):

• All circuit breaker state transitions and HALF_OPEN request limiting
• Request deduplication, cache key separation (method/token/body), rate limiting enforcement
• Cache key security verification (tokens with identical suffixes use different hashes)

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• fonts.googleapis.com
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/GhostClass/GhostClass/node_modules/.bin/next build (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

This PR addresses prior review feedback for the EzyGo batch fetcher/proxy stack by tightening request-dedup cache keys, improving circuit breaker behavior and error typing, aligning documentation/comments with current limits, and adding unit tests for the new behaviors.

Changes:

• Harden request deduplication cache keys (token hashing + method separation) and adjust health-check capacity logic.
• Improve circuit breaker HALF_OPEN behavior (concurrency limiting) and switch to typed errors (CircuitBreakerOpenError, oversized response error).
• Add Vitest unit tests for rate limiting, deduplication, and circuit breaker state transitions; update docs/UI Tailwind variants.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/lib/ezygo-batch-fetcher.ts	Secure cache keying (token hash + method) for deduplication and updated concurrency docs/comments.
src/lib/circuit-breaker.ts	Adds typed open error and limits HALF_OPEN concurrent probes to reduce thundering herd.
src/lib/tests/ezygo-batch-fetcher.test.ts	New unit tests for deduplication, rate limiting, URL construction, and cache-key security.
src/lib/tests/circuit-breaker.test.ts	New unit tests for circuit breaker transitions and HALF_OPEN request limiting.
src/components/ui/dropdown-menu.tsx	Fixes Tailwind data-attribute variant syntax for disabled/inset states.
src/components/loading.tsx	Corrects timeout value in documentation comment.
src/components/attendance/course-card.tsx	Fixes invalid Tailwind class (wrap-break-word → break-words).
src/app/api/health/ezygo/route.ts	Health logic now considers full utilization and queue backlog.
src/app/api/backend/[...path]/route.ts	Improves error typing/handling for circuit-open and oversized upstream responses.
src/app/(protected)/dashboard/page.tsx	Updates comment to reflect max concurrency = 3.
public/api-docs/openapi.yaml	Bumps OpenAPI version to 1.5.0.
docs/EZYGO_VERIFICATION.md	Converts absolute paths to repo-relative references.
docs/EZYGO_RATE_LIMITING.md	Aligns docs with MAX_CONCURRENT=3 and updated circuit breaker config.

---

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

[Copilot]

The cache key uses the raw endpoint string, but the actual request URL is built from a normalized cleanEndpoint (leading slashes removed). This means calls like fetchEzygoData('/myprofile', ...) and fetchEzygoData('myprofile', ...) hit the same upstream URL but will not deduplicate / share rate-limit slots because their cache keys differ. Consider normalizing the endpoint (and any other normalization applied to the URL) before constructing cacheKey so semantically identical requests map to the same key.

Suggested change

	const cacheKey = `${method}:${tokenHash}:${endpoint}:${JSON.stringify(body || {})}`;
	// Normalize endpoint (e.g., remove leading slashes) so semantically identical endpoints share a cache key
	const normalizedEndpoint = endpoint.replace(/^\/+/, '');
	const cacheKey = `${method}:${tokenHash}:${normalizedEndpoint}:${JSON.stringify(body || {})}`;

[Copilot]

These tests share module-level state from ezygo-batch-fetcher (LRU cache + the 100ms delayed requestCache.delete). Because some tests reuse the same token/endpoint combination (e.g. /myprofile + test-token here and again in later tests), a previously cached promise can be returned and global.fetch won't be called, making the suite timing-dependent/flaky. Use unique token/endpoint per test, or advance/run timers to flush the 100ms cache eviction, or add a test-only reset hook to clear the cache/queue between tests.