fix: preserve CI test failures - #3354
Merged
Priyanshu-byte-coder merged 2 commits intoAug 6, 2026
Merged
Conversation
github-actions
Bot
added
gssoc26
GSSoC Label Checklist 🏷️@Priyanshu-byte-coder — please apply the appropriate labels before merging: Difficulty (pick one):
Quality (optional):
Validation (required to score):
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the CI workflow so the automated test step preserves Vitest’s real exit status, ensuring failing test suites correctly fail the GitHub Actions job (aligning with issue #3349).
Changes:
- Removed the
|| echo ...fallback from the Vitest run step so failures aren’t masked. - Keeps the workflow behavior aligned with the repository’s CI gate expectations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Author
|
@Priyanshu-byte-coder Please review, repo is already stared by me |
Priyanshu-byte-coder
added
gssoc:approved
Priyanshu-byte-coder
merged commit Aug 6, 2026
2513b35
into
Priyanshu-byte-coder:main
15 of 17 checks passed
Priyanshu-byte-coder
added a commit
that referenced
this pull request
Aug 17, 2026
…#3499) * fix(deps): resolve 41 advisories and repair the ignored override config pnpm >=10 reads dependency overrides from pnpm-workspace.yaml. This repo has that file, so the `overrides` and `pnpm` blocks in package.json were never applied - the lockfile header recorded only the four entries from the workspace file. Pins added there for undici, esbuild, glob, form-data, swagger-client and three @opentelemetry packages had no effect at all. Worse, the workspace file pinned js-yaml to exactly 4.1.0, the version carrying the prototype-pollution and quadratic-DoS advisories, so the override was causing two of the reported CVEs rather than preventing them. Consolidate every override into pnpm-workspace.yaml and delete the dead blocks from package.json: - bump the four direct deps (next, next-auth, dompurify, postcss) and eslint-config-next to match - add caret-bounded overrides for the transitive advisories in postcss, js-yaml, fast-uri, immutable, nanoid, sharp, undici and brace-expansion - drop the stale swagger-client 3.36.2 pin, which now resolves higher anyway, and the glob >=10.5.0 pin, which would force a major on workbox for no advisory Overrides are caret-bounded on purpose: an open ">=" drags dependents onto the newest major (js-yaml 5, nanoid 6, undici 8) instead of the patched release. brace-expansion ships three parallel majors, so each is bounded separately - a single range would let the vulnerable 2.x satisfy "^1.1.18". next is pinned to ~16.2.11 to stay on the patch line that carries the fix; ^16.2.11 resolves to 16.3.1, a minor bump this change should not smuggle in. pnpm audit: 1 critical / 24 high / 10 moderate / 1 low -> 0. Unit tests unchanged at 2274 passed / 83 pre-existing failures. Build green. * fix(security): keep the rebuild token out of URLs, harden links and CI tokens Four findings that survived verification against the code, out of the 212 the static-analysis report raised. Leaderboard rebuild endpoint The token was read from `?token=` when the header was absent, which writes a shared secret into access logs, proxy logs and Referer headers (CWE-598). The documented caller already sends the header, so drop the fallback. Also corrects docs/self-hosting.md, which described a `Authorization: Bearer` header the route never read. Distributed lock ownership token upstashTryAcquireLock derived its value from Date.now() plus Math.random(). That value is what proves ownership of the lock, so it must not be guessable from the clock. Uses crypto.randomUUID() via Web Crypto, so the helper stays usable from the edge runtime. External link scheme allowlist RepoCard, SponsorAnalytics and PRMetrics render URLs straight from the GitHub API into an href. Nothing there is attacker-controlled today, but a link is one API shape away from being so. Adds safeExternalHref(), which allows http/https/mailto plus relative URLs and drops the attribute otherwise, so a rejected URL renders as text rather than a dead link. Covered by tests. Workflow token permissions automated-tests, e2e, stale, typecheck and visual-regression declared no permissions block and inherited the repo default. Adds an explicit one to every workflow: contents: read, except stale, which needs issues: write and pull-requests: write. ci, codeql and labeler already scoped permissions per job and now also default to read-only at the top - labeler matters most there, since pull_request_target runs with a privileged token. Also adds a Dockerfile HEALTHCHECK. It probes the landing page with node rather than curl, which node:20-alpine does not ship, because /api/debug/health is auth-gated behind DEBUG_SECRET. typecheck.yml is a disabled placeholder that asks in its own header to be deleted; it gets the permissions block rather than special-casing it, and removal is left as a separate call. * fix(ui): replace native alert() with toast notifications Thirteen error paths in the repo comparison page, the room view and the members panel reported failures through window.alert. Everywhere else in DevTrack uses sonner toasts, so these were the only places where a validation message froze the page behind an OS-styled modal that has to be dismissed before anything else works. Swaps all thirteen for toast.error and rewrites the copy to say what went wrong and what to do about it - "Use the owner/repo format, for example facebook/react" instead of "Invalid format!". The confirm() calls in these files are left alone. Replacing those needs a real confirmation dialog component, which the ui/ directory does not have yet. * test: repair stale assertions in 6 suites (83 -> 56 failures) * test: share a chainable Supabase query stub; count skippedUnconfigured in digest * fix(settings): restore cache invalidation and validate preferred_locale * fix: repair the red test suite and the six product bugs it was hiding The unit suite has been failing on main since #3354 removed the `pnpm test || echo "tests pass"` that swallowed the exit code: 83 failures across 22 files. Working through them turned up five real defects, not just stale assertions. Product bugs fixed ------------------ - Settings: PATCH never called clearLeaderboardCache, so a user who opted out of the leaderboard stayed listed for up to an hour, and one who opted in stayed missing for the same window. The import was present and unused; the #1779 fix had been dropped in a later refactor. - Settings: GET caches its payload for 5 minutes and PATCH never evicted it, so saved settings did not show up until the entry expired. cacheDelete was likewise imported and never called. - Settings: preferred_locale was written straight to the database with no validation, then used to select a message bundle on every later render. isValidLocale was imported but only applied to the response cookie. - Settings: the result of the update was discarded, so a failed write still returned 200 with a body implying success. - ProfileQrModal: the download handler looked for an `svg` inside the QR container, but the component renders QRCodeCanvas — a `canvas`. The lookup always returned null, so the "Download QR Code" button did nothing. It now reads the canvas directly, which also removes the SVG-serialise/Image/Blob round-trip. - ShortcutsModal: the component returns null until a `mounted` flag flips, so the focus effect ran while the close button did not exist yet and never re-ran. Opening the dialog left focus outside it. `mounted` is now a dependency of that effect. Test repairs ------------ Most of the rest were assertions describing behaviour that had deliberately changed, so the tests were updated to the current contract rather than the code reverted: - getClientIp and getBadgeClientIp were hardened to ignore spoofable forwarding headers unless a trusted proxy is declared. Rewritten to pin behaviour per TRUSTED_PROXY mode, including that x-forwarded-for is ignored. - 429 from GitHub is a rate limit even when x-ratelimit-remaining > 0, because secondary limits do not spend the primary quota. - github-accounts GET now degrades to an empty list instead of 401/500. - fetchUserSettings degrades through three column sets for older schemas. - weekly-digest reports emailsSent / emailsFailed, not sentCount / failedCount. Infrastructure -------------- - Adds test/supabase-query-stub.ts, a chainable thenable query-builder stub. Mocks written as literal nestings encode the exact call chain a route used on the day they were written, which is why several suites broke with ".range is not a function" when the digest and sync routes gained pagination. - The Anthropic SDK mock wrapped an arrow function, which has no [[Construct]] slot, so `new Anthropic()` threw. Now a function declaration. - user-settings-api mocks the settings cache; left real, the first test to run populated it and every later test was served that payload. - weekly-summary-combined pins the clock: its fixtures are anchored to the week of 2026-06-15, so it could only pass during that week. - One route-import test gets a 30s timeout; a cold module transform can exceed the 5s default when the whole suite runs in parallel. Also counts skipped_unconfigured in the weekly-digest response. A deployment with no RESEND_API_KEY previously incremented no counter at all, so the outcome vanished from the response entirely. Full suite: 161 files, 2376 tests, all passing. Type-check and build green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make the automated test step return Vitest's actual exit status instead of replacing a failure with a successful echo command.
Closes #3349
Type of Change
What Changed
How to Test
Expected result: a failing test suite fails the CI job.
Checklist
Additional Context
The current full suite has unrelated existing failures (86 failed, 2271 passed). This change intentionally ensures CI reports them rather than masking them.