chore: automated code quality fixes () - #2
Conversation
📝 WalkthroughWalkthroughThe pull request reformats workflows, documentation, stylesheets, templates, and client scripts. It also substantially updates board gameplay, daily-puzzle validation, reconnect handling, premoves, end-game statistics, analysis, and related UI behavior. ChangesRepository update
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant BoardScript
participant StockfishWorker
participant AnalysisAPI
Player->>BoardScript: submit move or daily-puzzle attempt
BoardScript->>StockfishWorker: evaluate position and candidate move
StockfishWorker-->>BoardScript: return score and best move
BoardScript->>BoardScript: update game and end-game UI
BoardScript->>AnalysisAPI: request post-game analysis
AnalysisAPI-->>BoardScript: return game metrics
BoardScript-->>Player: render results and statistics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)game/static/game/js/stockfish.jsast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (16)
game/static/game/js/board.js (2)
2020-2051: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor cleanups: dead code and a stale comment.
- Lines 2020-2051:
onDragStartduplicates the inlined.ondragstarthandler assigned inbuildBoard(Lines 1190-1234) and appears unused — either wire the inline handler to it or delete it.- Lines 2887-2891:
activeClockis computed and never used.- Line 4450: comment says "Wait exactly 2 seconds" but the call is
setInterval(checkAssets, 5000).Also applies to: 2887-2893, 4449-4451
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/js/board.js` around lines 2020 - 2051, Remove the unused onDragStart function or wire buildBoard’s d.ondragstart handler to use it, avoiding duplicate drag-start logic. Remove the unused activeClock computation near the clock update flow. Update the comment adjacent to setInterval(checkAssets, 5000) so it accurately describes the five-second interval.
4023-4046: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo
initThemeSwitcherdeclarations — this one is dead and uses a different storage key.A second
function initThemeSwitcher()at Line 4372 hoists over this one, so the calls at Lines 4080/4085 always run thedata-board-theme/boardThemeversion. This definition (writingdata-themeand onlychessBoardTheme) never executes. Remove it to avoid confusion and key drift with the template's theme bootstrap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/js/board.js` around lines 4023 - 4046, Remove the earlier initThemeSwitcher declaration that uses data-theme and chessBoardTheme. Keep the later initThemeSwitcher implementation using data-board-theme and boardTheme as the sole definition, preserving the existing callers and template theme bootstrap behavior.game/static/game/css/lesson.css (2)
224-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: blank lines before declarations in
.lesson-board,#demo-board,.demo-square.
declaration-empty-line-beforeflags lines 228, 231 (.lesson-board), 259, 263 (#demo-board), and 272, 274 (.demo-square) — same pattern as auth.css and forum.css, see consolidated note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/lesson.css` around lines 224 - 275, Remove the blank lines between consecutive declarations in the .lesson-board, `#demo-board`, and .demo-square CSS rules so declaration-empty-line-before passes, while preserving all declaration values and rule structure.Source: Linters/SAST tools
200-248: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDuplicate
.lesson-squareselector with divergent properties.
.lesson-squareis declared twice: once at lines 200-208 (width/height 60px, font-size 38px, cursor: pointer) and again at lines 234-240 (font-size 42px, user-select: none, no width/height/cursor). Since both share the same specificity, the second block silently overridesdisplay/justify-content/align-items/font-sizefrom the first wherever.lesson-squareis used without.light/.dark, whilewidth/height/cursorsurvive only from the first. This is easy to misread during future edits — worth merging into a single rule (or renaming one of them if they're genuinely meant for different board views).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/lesson.css` around lines 200 - 248, Merge the duplicate .lesson-square declarations into one selector, preserving the intended combined properties: sizing, flex alignment, cursor, font size, and user-select behavior. Remove the redundant second .lesson-square block while keeping the separate .lesson-square.light and .lesson-square.dark color rules unchanged.game/static/game/css/404.css (2)
95-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: unquote
"Cinzel"infont-family.Stylelint's
font-family-name-quotesrule flags the quoted"Cinzel"in.code(line 96) andh1(line 104). This is likely a formatter/linter config mismatch introduced by the automated pass — see consolidated note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/404.css` around lines 95 - 109, Remove the quotes around the Cinzel font family name in both the .code and h1 declarations, preserving the existing fallback serif and all other styling.Source: Linters/SAST tools
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint (17.14.0) violations remain after the automated formatting pass — the formatter and linter config appear misaligned. The same rule types recur across five CSS files:
font-family-name-quotesis inconsistently applied (someCinzel/Inter/Rajdhanioccurrences are quoted, others in the very same files are unquoted — comparelanding.cssline 663 vs. 752, or404.cssvs.500.css),declaration-empty-line-beforefires wherever the reformatter left blank lines between grouped declarations, andkeyframes-name-pattern/comment-whitespace-insideare one-off misses. Since this PR is specifically meant to be a lint/format cleanup, the residual stylelint errors will likely still fail CI's stylelint step.
game/static/game/css/404.css#L95-109: unquote"Cinzel"in.code(line 96) andh1(line 104).game/static/game/css/auth.css#L39-60: remove the blank lines before declarations inbody(lines 48, 50, 55, 57, 59).game/static/game/css/auth.css#L94-94: rename@keyframes logoGlowto kebab-case (e.g.logo-glow) and update itsanimationreference on.logo-icon-img.game/static/game/css/auth.css#L106-112: unquote"Cinzel"in.logo-text(line 107).game/static/game/css/auth.css#L137-149: remove the blank lines before declarations in.auth-card(lines 137, 139, 141).game/static/game/css/landing.css#L65-74: unquote"Inter"inbody(line 66) — also fix the same issue for"Cinzel"(line 110) and"Rajdhani"(line 752) elsewhere in this file, and add whitespace inside the/*BACK TO TOP BUTTON*/comment at line 1694.game/static/game/css/forum.css#L96-127: remove the blank lines before declarations in.forum-home-btn(lines 99, 101, 104, 107) and.forum-navbar(lines 120, 123, 126).game/static/game/css/lesson.css#L224-275: remove the blank lines before declarations in.lesson-board(lines 228, 231),#demo-board(lines 259, 263), and.demo-square(lines 272, 274).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/404.css` at line 1, Resolve the remaining Stylelint violations across 404.css, auth.css, landing.css, forum.css, and lesson.css: unquote the specified font-family names, remove blank lines before declarations in the identified rule blocks, rename auth.css’s logoGlow keyframes to kebab-case and update .logo-icon-img’s animation reference, and add whitespace inside the BACK TO TOP BUTTON comment in landing.css.Source: Linters/SAST tools
game/static/game/css/auth.css (4)
137-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: blank lines before declarations inside
.auth-card.Lines 137, 139, 141 trigger
declaration-empty-line-beforein the same way as thebodyrule above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/auth.css` around lines 137 - 149, Remove the unnecessary blank lines between declarations in the .auth-card rule, matching the declaration spacing used by the body rule and satisfying declaration-empty-line-before.Source: Linters/SAST tools
106-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: unquote
"Cinzel"in.logo-text.Same
font-family-name-quotesviolation as in 404.css.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/auth.css` around lines 106 - 112, Update the .logo-text font-family declaration to use the unquoted Cinzel font-family name, matching the project’s font-family-name-quotes stylelint rule and the existing correction in 404.css.Source: Linters/SAST tools
94-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStylelint:
@keyframes logoGlowshould be kebab-case.♻️ Proposed rename
-@keyframes logoGlow { +@keyframes logo-glow {Update the corresponding
animation-name/animationreference at.logo-icon-img(line ~90) to match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/auth.css` at line 94, Rename the `@keyframes` identifier logoGlow to kebab-case and update the corresponding animation reference in .logo-icon-img to use the renamed identifier consistently.Source: Linters/SAST tools
39-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: blank lines before declarations inside
body.
declaration-empty-line-beforeflags the blank lines betweenfont-family,min-height,display/flex properties,gap, andpadding(lines 48, 50, 55, 57, 59). Same pattern recurs in this file and others — see consolidated note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/auth.css` around lines 39 - 60, Remove the blank lines between declarations in the body rule, including the gaps after font-family, min-height, flex properties, gap, and padding, and apply the same declaration spacing cleanup throughout this stylesheet as indicated by the consolidated stylelint note.Source: Linters/SAST tools
game/static/game/css/forum.css (1)
96-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: blank lines before declarations in
.forum-home-btnand.forum-navbar.
declaration-empty-line-beforeflags lines 99, 101, 104, 107 and 120, 123, 126 — same pattern as auth.css and lesson.css, see consolidated note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/forum.css` around lines 96 - 127, Remove the extra blank lines between declarations in the .forum-home-btn and .forum-navbar CSS rules, matching the declaration spacing style used in auth.css and lesson.css while preserving all existing declarations and values.Source: Linters/SAST tools
game/static/game/css/landing.css (2)
65-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint: inconsistent font-family quoting throughout this file.
font-family-name-quotesflags the quoted"Inter"here (line 66), and the same pattern recurs for"Cinzel"(line 110) and"Rajdhani"(line 752) — yet the same fonts appear unquoted elsewhere in this very file (Rajdhaniat line 663,Interat lines 2562/2605/2650), proving the automated formatter applied quoting inconsistently. There's also acomment-whitespace-insideviolation at line 1694 (/*BACK TO TOP BUTTON*/). See consolidated note for the broader cross-file pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/landing.css` around lines 65 - 74, Normalize font-family declarations throughout the stylesheet to use the file’s existing unquoted convention for Inter, Cinzel, and Rajdhani, including the declarations near body and the other cited usages. Also fix the comment-whitespace violation by adding spaces inside the BACK TO TOP BUTTON comment delimiters.Source: Linters/SAST tools
777-864: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate/dead CSS declarations across this file — worth cleaning up.
Several rules in this file define the same selector or property twice within the same cascade context, so the earlier declaration is silently dead:
.about-track-wrapperand.about-trackare defined twice (lines 777-789 and again 854-864 under the/* Track */comment) with slightly different property sets — the second block silently wins for shared properties (width,overflow,transition), whileheight: 100%from the first block is the only surviving unique property..navbarsetsbackground-color: var(--nav-bg)(line 81) which is fully overridden by the later shorthandbackground: rgba(15, 15, 25, 0.55)(line 88) — the first declaration is dead..btn-primary-golddeclarestransitiontwice (lines 695 and 699) — the first0.25stiming is dead, only0.3sapplies..about-card-tagdeclaresdisplaytwice (inline-blockat 914,blockat 925) —inline-blockis dead.♻️ Suggested cleanup
.about-track-wrapper { width: 100%; - height: 100%; overflow: hidden; } .about-track { display: flex; width: 400%; - height: 100%; will-change: transform; transition: transform 0.5s ease-in-out; }(Remove the second
/* Track */block at lines 854-864 entirely, keeping only the first, or vice versa if the missingheight: 100%was intentional.).navbar { ... - background-color: var(--nav-bg); ... background: rgba(15, 15, 25, 0.55);.btn-primary-gold { ... - transition: all 0.25s cubic-bezier(0.16, 1, 0.3, 1); border: none; cursor: pointer; position: relative; transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1);.about-card-tag { display: inline-block; ... margin-bottom: 1.25rem; - display: block; width: fit-content;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/landing.css` around lines 777 - 864, Remove the duplicate /* Track */ .about-track-wrapper and .about-track declarations, consolidating them into one authoritative definition that preserves all required properties, including height: 100% on .about-track-wrapper and the existing track sizing and transition behavior. Also remove the dead earlier .navbar background-color, duplicate .btn-primary-gold transition, and obsolete .about-card-tag display declaration while preserving the later effective values.Source: Linters/SAST tools
game/static/game/css/preloading.css (1)
6-6: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDuplicate font load:
@importhere plus<link>ingame/templates/game/preloading.html.The template already preconnects and links the same Cinzel/Inter stylesheet, so the CSS
@importadds a second, render-blocking request chain on a page whose whole purpose is fast first paint. Dropping the@importkeeps behavior identical.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/static/game/css/preloading.css` at line 6, Remove the Google Fonts `@import` from the preloading stylesheet, relying on the existing font stylesheet link in the preloading template to preserve font behavior without triggering a duplicate render-blocking request.game/templates/game/rules.html (1)
1749-1752: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo-op else branch.
backBtn.href = backBtn.getAttribute("href")reassigns the value it just read; the label is also already "← Back to Home" in the markup. The branch can be dropped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/templates/game/rules.html` around lines 1749 - 1752, Remove the no-op else branch associated with backBtn, including the redundant backBtn.href assignment and duplicate label update; preserve the surrounding navigation behavior and existing markup label.game/templates/game/forum_list.html (1)
86-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrettier split a Django variable tag across lines.
{{ discussion.title\n}}still resolves (Django strips surrounding whitespace in variable tags), but the wrap is fragile and injects awkward whitespace into the heading. Consider wrapping such tags in{% verbatim %}-safe formatting or adding aprettier-ignoreso the tag stays on one line.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@game/templates/game/forum_list.html` around lines 86 - 89, Keep the Django template variable tag in the forum-card title intact on a single line by applying an appropriate Prettier ignore or safe formatting directive around `{{ discussion.title }}`. Preserve the rendered heading text and existing `forum-card-title` markup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 285: Update the test-summary command in the CI workflow so both matching
grep output and the fallback “No summary line found” message are redirected to
$GITHUB_STEP_SUMMARY. Preserve the existing grep pattern and fallback behavior
while applying the redirection to the entire command expression.
- Around line 146-147: Update every read-only checkout step in the CI workflow,
including the checkout actions near the referenced occurrences, to set
persist-credentials to false. Leave any checkout that requires credentials for a
subsequent push or other write operation unchanged.
- Around line 27-31: Update the workflow-level permissions in ci.yml so
pull-requests is set to read instead of write. Keep the existing contents,
issues, and deployments permissions unchanged, leaving PR write access
exclusively to pr-guardian.yml.
- Around line 427-442: Update the “Run Bandit scan” step to capture Bandit’s
exit status without using `|| true`, distinguish scanner execution failures from
successful scans with high/medium findings, and fail CI for either condition.
Only report success when Bandit completes successfully and the severity check
finds no high or medium issues.
- Around line 92-99: Update the changed-file lookup in the pull request
permission check to use github.paginate with pulls.listFiles, collecting all
pages before evaluating the infra-only condition. Keep the existing filename
predicate and infra_only result, but apply files.every only to the complete
changed-file list.
- Line 282: Update the step containing the GITHUB_STEP_SUMMARY echo to pass
github.ref_name and github.sha through step-level env variables, then reference
those variables with shell quoting in the command instead of interpolating
workflow expressions directly.
In @.github/workflows/pr-guardian.yml:
- Around line 83-94: Update the changed-file retrieval in the infra-only bypass
logic to paginate through every page of pull-request files before evaluating the
allowlist. Ensure all returned files are combined and checked by the existing
filename predicate, so non-infrastructure files beyond the first 100 prevent the
`{ allowed: true, reason: 'infra_only' }` result.
In `@CONTRIBUTING.md`:
- Around line 14-33: Update all table-of-contents links in CONTRIBUTING.md to
remove backticks from their fragment destinations, including nested entries, so
each uses the standard ](`#heading`) format and navigates to the corresponding
heading.
In `@docs/API_WALKTHROUGH.md`:
- Around line 53-60: Update the new-game “Payload Structure” example in
API_WALKTHROUGH.md to use the documented API fields player_color, time_limit,
and increment instead of color and timer_mins, while preserving the existing
mode, difficulty, and valid example values.
In `@docs/API.md`:
- Around line 589-597: The daily-puzzle public response example must not expose
the solution. Update the API response contract and implementation associated
with the daily-puzzle endpoint to return only puzzle metadata and FEN, while
validating submitted moves server-side against the stored solution.
In `@game/static/game/css/preloading.css`:
- Around line 374-380: Move the “close to --text-secondary at low opacity”
comment above the color declaration in the relevant preloading CSS rule, leaving
the reflowed rgba() value unchanged and removing the trailing comment from its
closing line so Stylelint passes.
In `@game/static/game/js/auth.js`:
- Around line 246-249: The toast exit class does not match the stylesheet
selector. In game/static/game/js/auth.js lines 246-249, update the timeout in
the toast removal flow to add the existing hiding class instead of toast-exit;
no direct change is required in game/static/game/css/toast.css lines 40-42
because the JavaScript fix reuses its existing .toast.hiding animation selector.
- Around line 189-191: Guard the form lookup in the password validation handler
before calling querySelector: store the result of passwordInput.closest("form"),
and only query for the submit button and bind related validation behavior when a
form exists. Preserve execution of the subsequent toast auto-dismiss logic when
the password input is outside a form.
In `@game/static/game/js/board.js`:
- Around line 1693-1753: Ensure the Stockfish-validation branch refreshes the
board after validation completes, including both correct and failed/error
outcomes as appropriate. Extract or reuse the existing post-move UI refresh
logic involving syncPieces(), updateTurn(), and clock updates, and invoke it
from the validateMoveWithStockfish().then/.catch flow before returning or
showing the next state; keep all puzzle branches visually synchronized.
- Around line 4496-4525: Update the boardEl touchstart handler to assign
touchDragSrc and initialize touchStartPos from the captured touch’s client
coordinates after validating the playable piece. Remove the unused touch
variable only if no longer needed, while preserving the existing touchmove and
touchcancel guards and tap behavior.
- Around line 1924-1929: Fix requestAIMove by defining a local threefold-warning
flag near the start of its data.valid block, then use that flag in the existing
status-display condition instead of the out-of-scope hasThreefoldWarning from
executeMove. Preserve premove execution and avoid triggering the surrounding
reconnect handler for normal non-check AI moves.
In `@game/static/game/js/toast.js`:
- Around line 38-41: Update the toast rendering around the innerHTML assignment
to avoid interpolating the message into markup. Create the icon and message span
elements separately, assign their values with textContent, and append them to
toast while preserving the existing icon fallback and display order.
In `@game/templates/game/forum_detail.html`:
- Around line 13-20: Correct the Google Fonts CSS2 syntax in the Cinzel and
Inter links in game/templates/game/forum_detail.html lines 13-20 by replacing
wght= with wght@, and apply the same correction to both links in
game/templates/game/forum_list.html lines 13-20. Leave the existing font
families and weight lists unchanged.
In `@game/templates/game/password_reset_email.html`:
- Around line 1-5: Restore the original plaintext layout in the password reset
email template, preserving the shown blank lines and placing the reset URL on
its own line with a trailing newline before the following sentence. Exclude this
template from both Prettier and HTMLHint ignore configuration so formatters and
the doctype-first rule do not modify or flag it.
In `@game/templates/game/rules.html`:
- Around line 794-844: Replace the clickable div elements in the rule navigation
around showRule with keyboard-accessible button elements, including nav-basics,
nav-pieces, nav-pawn, nav-castling, nav-enpassant, nav-check, nav-stalemate, and
nav-promotion; preserve their IDs, labels, icons, active state, and click
behavior so every entry can be reached and activated via keyboard.
---
Nitpick comments:
In `@game/static/game/css/404.css`:
- Around line 95-109: Remove the quotes around the Cinzel font family name in
both the .code and h1 declarations, preserving the existing fallback serif and
all other styling.
- Line 1: Resolve the remaining Stylelint violations across 404.css, auth.css,
landing.css, forum.css, and lesson.css: unquote the specified font-family names,
remove blank lines before declarations in the identified rule blocks, rename
auth.css’s logoGlow keyframes to kebab-case and update .logo-icon-img’s
animation reference, and add whitespace inside the BACK TO TOP BUTTON comment in
landing.css.
In `@game/static/game/css/auth.css`:
- Around line 137-149: Remove the unnecessary blank lines between declarations
in the .auth-card rule, matching the declaration spacing used by the body rule
and satisfying declaration-empty-line-before.
- Around line 106-112: Update the .logo-text font-family declaration to use the
unquoted Cinzel font-family name, matching the project’s font-family-name-quotes
stylelint rule and the existing correction in 404.css.
- Line 94: Rename the `@keyframes` identifier logoGlow to kebab-case and update
the corresponding animation reference in .logo-icon-img to use the renamed
identifier consistently.
- Around line 39-60: Remove the blank lines between declarations in the body
rule, including the gaps after font-family, min-height, flex properties, gap,
and padding, and apply the same declaration spacing cleanup throughout this
stylesheet as indicated by the consolidated stylelint note.
In `@game/static/game/css/forum.css`:
- Around line 96-127: Remove the extra blank lines between declarations in the
.forum-home-btn and .forum-navbar CSS rules, matching the declaration spacing
style used in auth.css and lesson.css while preserving all existing declarations
and values.
In `@game/static/game/css/landing.css`:
- Around line 65-74: Normalize font-family declarations throughout the
stylesheet to use the file’s existing unquoted convention for Inter, Cinzel, and
Rajdhani, including the declarations near body and the other cited usages. Also
fix the comment-whitespace violation by adding spaces inside the BACK TO TOP
BUTTON comment delimiters.
- Around line 777-864: Remove the duplicate /* Track */ .about-track-wrapper and
.about-track declarations, consolidating them into one authoritative definition
that preserves all required properties, including height: 100% on
.about-track-wrapper and the existing track sizing and transition behavior. Also
remove the dead earlier .navbar background-color, duplicate .btn-primary-gold
transition, and obsolete .about-card-tag display declaration while preserving
the later effective values.
In `@game/static/game/css/lesson.css`:
- Around line 224-275: Remove the blank lines between consecutive declarations
in the .lesson-board, `#demo-board`, and .demo-square CSS rules so
declaration-empty-line-before passes, while preserving all declaration values
and rule structure.
- Around line 200-248: Merge the duplicate .lesson-square declarations into one
selector, preserving the intended combined properties: sizing, flex alignment,
cursor, font size, and user-select behavior. Remove the redundant second
.lesson-square block while keeping the separate .lesson-square.light and
.lesson-square.dark color rules unchanged.
In `@game/static/game/css/preloading.css`:
- Line 6: Remove the Google Fonts `@import` from the preloading stylesheet,
relying on the existing font stylesheet link in the preloading template to
preserve font behavior without triggering a duplicate render-blocking request.
In `@game/static/game/js/board.js`:
- Around line 2020-2051: Remove the unused onDragStart function or wire
buildBoard’s d.ondragstart handler to use it, avoiding duplicate drag-start
logic. Remove the unused activeClock computation near the clock update flow.
Update the comment adjacent to setInterval(checkAssets, 5000) so it accurately
describes the five-second interval.
- Around line 4023-4046: Remove the earlier initThemeSwitcher declaration that
uses data-theme and chessBoardTheme. Keep the later initThemeSwitcher
implementation using data-board-theme and boardTheme as the sole definition,
preserving the existing callers and template theme bootstrap behavior.
In `@game/templates/game/forum_list.html`:
- Around line 86-89: Keep the Django template variable tag in the forum-card
title intact on a single line by applying an appropriate Prettier ignore or safe
formatting directive around `{{ discussion.title }}`. Preserve the rendered
heading text and existing `forum-card-title` markup.
In `@game/templates/game/rules.html`:
- Around line 1749-1752: Remove the no-op else branch associated with backBtn,
including the redundant backBtn.href assignment and duplicate label update;
preserve the surrounding navigation behavior and existing markup label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 01eb593c-a25e-41bc-97bc-8a761a6ff4d5
📒 Files selected for processing (66)
.github/ISSUE_TEMPLATE/bug_report.md.github/ISSUE_TEMPLATE/feature_request.md.github/workflows/ci.yml.github/workflows/contributors.yml.github/workflows/label-gssoc.yml.github/workflows/pr-guardian.ymlCODE_OF_CONDUCT.mdCONTRIBUTING.mdREADME.mdboard.test.jsdocs/API.mddocs/API_WALKTHROUGH.mddocs/SECURITY_HEADERS_AUDIT.mddocs/development.mddocs/engine_architecture.mddocs/testing.mdgame/static/game/css/404.cssgame/static/game/css/500.cssgame/static/game/css/auth.cssgame/static/game/css/forum.cssgame/static/game/css/landing.cssgame/static/game/css/lesson.cssgame/static/game/css/opening_trainer.cssgame/static/game/css/preloading.cssgame/static/game/css/toast.cssgame/static/game/js/auth.jsgame/static/game/js/board.jsgame/static/game/js/dropdown.jsgame/static/game/js/lesson_board.jsgame/static/game/js/lesson_coordinates.jsgame/static/game/js/lesson_demo.jsgame/static/game/js/lesson_practice.jsgame/static/game/js/opening_trainer.jsgame/static/game/js/roadmap_connectors.jsgame/static/game/js/stockfish.jsgame/static/game/js/theme.jsgame/static/game/js/toast.jsgame/templates/404.htmlgame/templates/500.htmlgame/templates/game/achievements.htmlgame/templates/game/board.htmlgame/templates/game/delete_account.htmlgame/templates/game/delete_success.htmlgame/templates/game/forum_detail.htmlgame/templates/game/forum_list.htmlgame/templates/game/forum_new.htmlgame/templates/game/includes/messages.htmlgame/templates/game/leaderboard.htmlgame/templates/game/lesson_map.htmlgame/templates/game/lessons.htmlgame/templates/game/login.htmlgame/templates/game/opening_detail.htmlgame/templates/game/opening_trainer.htmlgame/templates/game/password_reset.htmlgame/templates/game/password_reset_complete.htmlgame/templates/game/password_reset_confirm.htmlgame/templates/game/password_reset_done.htmlgame/templates/game/password_reset_email.htmlgame/templates/game/preloading.htmlgame/templates/game/register.htmlgame/templates/game/rules.htmlgame/templates/game/stats.htmlgame/templates/game/verify_otp.htmlgame/templates/game/welcome_email.htmlstructure.mdvercel.json
| permissions: | ||
| contents: read | ||
| pull-requests: write # allows posting CI summary comments on PRs | ||
| issues: read # PR guardian reads linked issue assignees | ||
| deployments: read # required for Vercel preview polling | ||
| contents: read | ||
| pull-requests: write # allows posting CI summary comments on PRs | ||
| issues: read # PR guardian reads linked issue assignees | ||
| deployments: read # required for Vercel preview polling |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files '.github/workflows/*' || true
echo "== ci.yml =="
if [ -f .github/workflows/ci.yml ]; then
nl -ba .github/workflows/ci.yml | sed -n '1,220p'
fi
echo "== search for pull request comment/write APIs in workflows =="
rg -n "create.*comment|pull.*request|pullrequests|issues|pull-requests|pull_request" .github/workflows || trueRepository: saurabhhhcodes/Checkora
Length of output: 363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ci.yml permissions and relevant steps =="
awk '{printf "%5d %s\n", NR, $0}' .github/workflows/ci.yml | sed -n '1,220p'
echo "== pull request write/read/comment usages across workflows =="
rg -n "GITHUB_TOKEN|permissions:|pull-requests|issues|create.*comment|comments|pull_request|pullRequests|PR Guardian|guardian|Vercel|Vercel|summary" .github/workflows || trueRepository: saurabhhhcodes/Checkora
Length of output: 18925
Downgrade the workflow-level pull request permission.
.github/workflows/ci.yml only reads pull-request metadata/files, while pr-guardian.yml is the separate workflow that owns PR write operations. Set pull-requests: read here so test/scan steps can’t access the write token.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 29-29: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 27 - 31, Update the workflow-level
permissions in ci.yml so pull-requests is set to read instead of write. Keep the
existing contents, issues, and deployments permissions unchanged, leaving PR
write access exclusively to pr-guardian.yml.
Source: Linters/SAST tools
| const { data: files } = await github.rest.pulls.listFiles({ | ||
| owner, repo, pull_number: prNumber, per_page: 100, | ||
| }); | ||
| if ( | ||
| files.length > 0 && | ||
| files.every((f) => f.filename.startsWith('.github/') || f.filename === 'vercel.json') | ||
| ) { | ||
| return { allowed: true, reason: 'infra_only' }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file around referenced lines =="
if [ -f .github/workflows/ci.yml ]; then
nl -ba .github/workflows/ci.yml | sed -n '70,120p'
else
echo ".github/workflows/ci.yml not found"
fi
echo
echo "== GitHub GraphQL/REST references in workflow =="
rg -n "pulls\.listFiles|paginate|github\.rest|allowed: true|infra_only|pull_number" .github/workflows/ci.yml || true
echo
echo "== deterministic page-size/loop check from source =="
python3 - <<'PY'
from pathlib import Path
p=Path('.github/workflows/ci.yml')
text=p.read_text()
checks = [
'pulls.listFiles' in text,
'per_page: 100' in text,
'github.paginate' in text,
'files.every' in text,
]
print(dict(zip(['pulls_listFiles', 'per_page_100', 'github_paginate', 'files_every'], checks)))
PYRepository: saurabhhhcodes/Checkora
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file around referenced lines =="
if [ -f .github/workflows/ci.yml ]; then
sed -n '70,120p' .github/workflows/ci.yml | cat -n
else
echo ".github/workflows/ci.yml not found"
fi
echo
echo "== GitHub GraphQL/REST references in workflow =="
grep -En "pulls\.listFiles|paginate|github\.rest|allowed: true|infra_only|pull_number" .github/workflows/ci.yml || true
echo
echo "== source checks =="
grep -q "pulls\.listFiles" .github/workflows/ci.yml && echo "pulls_listFiles=true" || echo "pulls_listFiles=false"
grep -q "per_page: 100" .github/workflows/ci.yml && echo "per_page_100=true" || echo "per_page_100=false"
grep -qm1 "github\.paginate" .github/workflows/ci.yml && echo "github_paginate=true" || echo "github_paginate=false"
grep -q "files\.every" .github/workflows/ci.yml && echo "files_every=true" || echo "files_every=false"Repository: saurabhhhcodes/Checkora
Length of output: 3884
Paginate the changed-file lookup before applying the infra-only bypass.
pulls.listFiles({ per_page: 100 }) returns only the first page; if a PR has more than 100 changed files, a non-infrastructure file outside that page can be skipped while files.every(...) still returns true. Use github.paginate(...) before deciding infra_only.
Proposed fix
- const { data: files } = await github.rest.pulls.listFiles({
- owner, repo, pull_number: prNumber, per_page: 100,
- });
+ const files = await github.paginate(
+ github.rest.pulls.listFiles,
+ { owner, repo, pull_number: prNumber, per_page: 100 },
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: files } = await github.rest.pulls.listFiles({ | |
| owner, repo, pull_number: prNumber, per_page: 100, | |
| }); | |
| if ( | |
| files.length > 0 && | |
| files.every((f) => f.filename.startsWith('.github/') || f.filename === 'vercel.json') | |
| ) { | |
| return { allowed: true, reason: 'infra_only' }; | |
| const files = await github.paginate( | |
| github.rest.pulls.listFiles, | |
| { owner, repo, pull_number: prNumber, per_page: 100 }, | |
| ); | |
| if ( | |
| files.length > 0 && | |
| files.every((f) => f.filename.startsWith('.github/') || f.filename === 'vercel.json') | |
| ) { | |
| return { allowed: true, reason: 'infra_only' }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 92 - 99, Update the changed-file
lookup in the pull request permission check to use github.paginate with
pulls.listFiles, collecting all pages before evaluating the infra-only
condition. Keep the existing filename predicate and infra_only result, but apply
files.every only to the complete changed-file list.
| - name: 📥 Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
cat -n .github/workflows/ci.yml | sed -n '120,490p'
echo
echo "== checkout occurrences with context =="
rg -n -C 3 'uses:\s*actions/checkout@v4|persist-credentials|permissions:|GITHUB_TOKEN|github\.token' .github/workflows/ci.yml || true
echo
echo "== workflow jobs summary =="
python3 - <<'PY'
import re
from pathlib import Path
p=Path('.github/workflows/ci.yml')
txt=p.read_text()
for i,line in enumerate(txt.splitlines(),1):
if re.fullmatch(r'\s*\w[\w.-]*:\s*$', line) or re.search(r'^\s*name:\s*(?:.+)$', line):
print(f"{i}: {line}")
PYRepository: saurabhhhcodes/Checkora
Length of output: 21642
Disable persisted checkout credentials in read-only CI jobs.
The default checkout stores GITHUB_TOKEN in local Git config while subsequent project-controlled pip install / lint / test / scan steps run in these jobs. Add persist-credentials: false to each non-push checkout, unless credentials are required for a later write operation.
Proposed fix
uses: actions/checkout@v4
+ with:
+ persist-credentials: falseAlso applies to: 189-190, 239-240, 322-323, 364-365, 408-409, 468-469
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 146-147: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 146 - 147, Update every read-only
checkout step in the CI workflow, including the checkout actions near the
referenced occurrences, to set persist-credentials to false. Leave any checkout
that requires credentials for a subsequent push or other write operation
unchanged.
Source: Linters/SAST tools
| if: always() | ||
| run: | | ||
| echo "## 🧪 Test Results — $(date -u '+%Y-%m-%d %H:%M UTC')" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Branch:** \`${{ github.ref_name }}\` | **Commit:** \`${{ github.sha }}\`" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
sed -n '250,300p' .github/workflows/ci.yml || true
echo
echo "== nearby ref_name/sha interpolations =="
rg -n '\$\{\{ github\.ref_name \}\}|\$\{\{ github\.sha \}\}|GITHUB_STEP_SUMMARY|run:' .github/workflows/ci.yml || true
echo
echo "== shell injection probe for interpolated ref_name =="
python3 - <<'PY'
import subprocess
malicious_ref = "${IFS}$(printf '%s' "INJECT")*"
script_before = 'echo "**Branch:** `${malicious_ref}` | **Commit:** `${malicious_sha}`" >> $GITHUB_STEP_SUMMARY'
actual = script_before.replace("${malicious_sha}", "${malicious_ref}"[0]) # same injection shape for branch
cmd = ["bash", "-c", actual]
try:
p = subprocess.run(cmd, text=True, capture_output=True, env={"GITHUB_STEP_SUMMARY":"/tmp/coderabbit_probe_output"})
except subprocess.TimeoutExpired:
p = subprocess.TimeoutExpired(cmd, 0)
print("command:", " ".join(cmd))
print("stdout:", repr(p.stdout))
print("stderr:", repr(p.stderr))
print("returncode:", p.returncode)
PYRepository: saurabhhhcodes/Checkora
Length of output: 5571
🏁 Script executed:
#!/bin/bash
set -euho pipefail
python3 - <<'PY'
import subprocess
import shlex
malicious_ref = "${IFS}$(printf '%s' 'INJECT')*"
sha = "abc123"
script = 'echo "**Branch:** `${' + maliciou` shRepository: saurabhhhcodes/Checkora
Length of output: 402
🏁 Script executed:
#!/bin/bash
set -uho pipefail
python3 - <<'PY'
import subprocess
malicious_ref = "${IFS}$(printf '%s' 'INJECT')*"
sha = "abc123"
script = 'echo "**Branch:** `${malicious_ref}` | **Commit:** `${sha}`" >> $GITHUB_STEP_SUMMARY'
print("script after GitHub interpolation:")
print(script)
p = subprocess.run(["bash", "-c", script], text=True, capture_output=True, timeout=5)
print("exitcode:", p.returncode)
print("stdout:", repr(p.stdout))
print("stderr:", repr(p.stderr))
PYRepository: saurabhhhcodes/Checkora
Length of output: 373
🏁 Script executed:
#!/bin/bash
set -u
tmp="$(mktemp /tmp/coderabbit_probe.XXXXXX)"
malicious_ref='${IFS}$(printf "%s" "INJECT")*'
sha='abc123'
printf 'GITHUB_STEP_SUMMARY=%s\nREF_NAME=%s\nCOMMIT_SHA=%s\n' "$tmp" "$malicious_ref" "$sha" > "$tmp/env.txt"
export GITHUB_STEP_SUMMARY="$tmp/summary" REF="$malicious_ref" SHA="$sha"
echo "unquoted run injection shape:"
cat "$tmp/summary" || true
bash -c 'echo "**Branch:** `$REF` | **Commit:** `$SHA`" >> $GITHUB_STEP_SUMMARY'
cat "$tmp/summary" || true
echo "quoted run pass-through shape:"
rm -f "$tmp/summary"
bash -c 'echo "**Branch:** `$REF` | **Commit:** `$SHA`" >> "$GITHUB_STEP_SUMMARY"'
cat "$tmp/summary" || true
rm -f "$tmp/summary" "$tmp/env.txt" "$tmp"Repository: saurabhhhcodes/Checkora
Length of output: 779
🌐 Web query:
GitHub Actions security shell injection github.ref_name user controlled run block github_ref_name shell injection
💡 Result:
In GitHub Actions, using ${{ github.ref_name }} directly within a run: block is a significant security risk known as shell injection [1][2][3]. Because ${{ github.ref_name }} is evaluated and replaced with its raw value before the shell executes the command, a malicious branch or tag name containing shell metacharacters (e.g., $(...), |, ;, or backticks) can lead to arbitrary code execution on the runner [4][5][3]. To prevent this vulnerability, never interpolate context variables directly into run: scripts [1][2][6]. Instead, use the following defensive practices: 1. Use Intermediate Environment Variables: The recommended mitigation is to assign the context value to an environment variable within the env: block of the step [2][3][7]. The shell will then treat the variable as a literal string rather than as executable code [8][4][5]. Example of vulnerable code: - run:./publish.sh ${{ github.ref_name }} Example of secure code: - name: Publish env: REF_NAME: ${{ github.ref_name }} run:./publish.sh "$REF_NAME" 2. Treat All Context Data as Untrusted: Always treat github context data—including github.ref_name, github.head_ref, and event-based inputs like issue titles or pull request bodies—as untrusted user input [1][2][6]. 3. Use Actions Instead of Inline Scripts: Where possible, use dedicated GitHub Actions instead of run: blocks, as actions typically accept inputs as arguments rather than constructing shell scripts, which inherently reduces the risk of shell injection [5][7]. 4. Follow Least Privilege: Limit the permissions of the GITHUB_TOKEN in your workflow configuration to minimize the potential impact if a compromise occurs [9][6]. These practices are widely documented by GitHub and security researchers as the standard approach to hardening CI/CD pipelines against injection attacks [2][3][7][6].
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 3: https://orbisappsec.com/blog/github-actions-shell-injection-how-variables-can-hijack-your-cicd-pipeline
- 4: https://sisaku-security.github.io/lint/docs/advisory/ghsa-vxmw-7h4f-hqxh/
- 5: https://www.kenmuse.com/blog/the-hidden-danger-in-git-ref-names/
- 6: https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.md
- 7: https://docs.github.com/en/enterprise-server@2.22/actions/security-guides/security-hardening-for-github-actions
- 8: aws/jsii-rosetta@bcb9c37
- 9: https://github.blog/security/vulnerability-research/how-to-catch-github-actions-workflow-injections-before-attackers-do/
Avoid interpolating github.ref_name directly into the shell script.
GitHub workflow context values are substituted before the script parses, so shell metacharacters in a branch/ref name can be executed by the runner. Pass summary values through env and reference quoted environment variables instead.
Proposed fix
- echo "**Branch:** \`${{ github.ref_name }}\` | **Commit:** \`${{ github.sha }}\`" >> $GITHUB_STEP_SUMMARY
+ echo "**Branch:** \`$REF_NAME\` | **Commit:** \`$COMMIT_SHA\`" >> "$GITHUB_STEP_SUMMARY"env:
REF_NAME: ${{ github.ref_name }}
COMMIT_SHA: ${{ github.sha }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "**Branch:** \`${{ github.ref_name }}\` | **Commit:** \`${{ github.sha }}\`" >> $GITHUB_STEP_SUMMARY | |
| echo "**Branch:** \`$REF_NAME\` | **Commit:** \`$COMMIT_SHA\`" >> "$GITHUB_STEP_SUMMARY" |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 282-282: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 282, Update the step containing the
GITHUB_STEP_SUMMARY echo to pass github.ref_name and github.sha through
step-level env variables, then reference those variables with shell quoting in
the command instead of interpolating workflow expressions directly.
Source: Linters/SAST tools
| echo "**Branch:** \`${{ github.ref_name }}\` | **Commit:** \`${{ github.sha }}\`" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo '```' >> $GITHUB_STEP_SUMMARY | ||
| grep -E '(FAIL|ERROR|OK|Ran [0-9]+ test)' test_output.txt || echo 'No summary line found' >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Redirect the successful test summary output.
The current redirection applies only to the fallback echo; when grep finds matches, its output goes to the log instead of $GITHUB_STEP_SUMMARY.
Proposed fix
- grep -E '(FAIL|ERROR|OK|Ran [0-9]+ test)' test_output.txt || echo 'No summary line found' >> $GITHUB_STEP_SUMMARY
+ grep -E '(FAIL|ERROR|OK|Ran [0-9]+ test)' test_output.txt >> "$GITHUB_STEP_SUMMARY" ||
+ echo 'No summary line found' >> "$GITHUB_STEP_SUMMARY"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| grep -E '(FAIL|ERROR|OK|Ran [0-9]+ test)' test_output.txt || echo 'No summary line found' >> $GITHUB_STEP_SUMMARY | |
| grep -E '(FAIL|ERROR|OK|Ran [0-9]+ test)' test_output.txt >> "$GITHUB_STEP_SUMMARY" || | |
| echo 'No summary line found' >> "$GITHUB_STEP_SUMMARY" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 285, Update the test-summary command in the
CI workflow so both matching grep output and the fallback “No summary line
found” message are redirected to $GITHUB_STEP_SUMMARY. Preserve the existing
grep pattern and fallback behavior while applying the redirection to the entire
command expression.
| boardEl.addEventListener( | ||
| "touchstart", | ||
| (e) => { | ||
| const touch = e.touches[0]; | ||
| const squareEl = e.target.closest(".square"); | ||
| if (!squareEl) return; | ||
|
|
||
| const r = parseInt(squareEl.dataset.r); | ||
| const c = parseInt(squareEl.dataset.c); | ||
| const isPremoveMode = gameMode === "ai" && turn !== playerColor; | ||
| const vBoard = isPremoveMode ? getVirtualBoard() : board; | ||
| const piece = vBoard[r][c]; | ||
| if (!piece || paused || gameOver) return; | ||
|
|
||
| // Check if the piece is playable by the current player (including AI premoves) | ||
| const isPremoveDrag = | ||
| gameMode === "ai" && | ||
| turn !== playerColor && | ||
| pColor(piece) === playerColor; | ||
| const isNormalDrag = | ||
| gameMode === "ai" | ||
| ? turn === playerColor && pColor(piece) === playerColor | ||
| : pColor(piece) === turn; | ||
|
|
||
| if (!isPremoveDrag && !isNormalDrag) return; | ||
|
|
||
| touchDragSrc = { r, c }; | ||
| }, | ||
| { passive: true }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
touchStartPos is never assigned, so touch dragging can never start.
touchstart only sets touchDragSrc, but touchmove bails out on !touchStartPos (Line 4530) and touchcancel bails likewise. Result: mobile drag-and-drop never activates and only the tap path in touchend works. The captured touch here is also unused.
🐛 Proposed fix
- touchDragSrc = { r, c };
+ touchStartPos = { x: touch.clientX, y: touch.clientY };
+ touchDragSrc = { r, c };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| boardEl.addEventListener( | |
| "touchstart", | |
| (e) => { | |
| const touch = e.touches[0]; | |
| const squareEl = e.target.closest(".square"); | |
| if (!squareEl) return; | |
| const r = parseInt(squareEl.dataset.r); | |
| const c = parseInt(squareEl.dataset.c); | |
| const isPremoveMode = gameMode === "ai" && turn !== playerColor; | |
| const vBoard = isPremoveMode ? getVirtualBoard() : board; | |
| const piece = vBoard[r][c]; | |
| if (!piece || paused || gameOver) return; | |
| // Check if the piece is playable by the current player (including AI premoves) | |
| const isPremoveDrag = | |
| gameMode === "ai" && | |
| turn !== playerColor && | |
| pColor(piece) === playerColor; | |
| const isNormalDrag = | |
| gameMode === "ai" | |
| ? turn === playerColor && pColor(piece) === playerColor | |
| : pColor(piece) === turn; | |
| if (!isPremoveDrag && !isNormalDrag) return; | |
| touchDragSrc = { r, c }; | |
| }, | |
| { passive: true }, | |
| ); | |
| boardEl.addEventListener( | |
| "touchstart", | |
| (e) => { | |
| const touch = e.touches[0]; | |
| const squareEl = e.target.closest(".square"); | |
| if (!squareEl) return; | |
| const r = parseInt(squareEl.dataset.r); | |
| const c = parseInt(squareEl.dataset.c); | |
| const isPremoveMode = gameMode === "ai" && turn !== playerColor; | |
| const vBoard = isPremoveMode ? getVirtualBoard() : board; | |
| const piece = vBoard[r][c]; | |
| if (!piece || paused || gameOver) return; | |
| // Check if the piece is playable by the current player (including AI premoves) | |
| const isPremoveDrag = | |
| gameMode === "ai" && | |
| turn !== playerColor && | |
| pColor(piece) === playerColor; | |
| const isNormalDrag = | |
| gameMode === "ai" | |
| ? turn === playerColor && pColor(piece) === playerColor | |
| : pColor(piece) === turn; | |
| if (!isPremoveDrag && !isNormalDrag) return; | |
| touchStartPos = { x: touch.clientX, y: touch.clientY }; | |
| touchDragSrc = { r, c }; | |
| }, | |
| { passive: true }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@game/static/game/js/board.js` around lines 4496 - 4525, Update the boardEl
touchstart handler to assign touchDragSrc and initialize touchStartPos from the
captured touch’s client coordinates after validating the playable piece. Remove
the unused touch variable only if no longer needed, while preserving the
existing touchmove and touchcancel guards and tap behavior.
| toast.innerHTML = ` | ||
| <span class="toast-icon">${icons[type] || icons.info}</span> | ||
| <span class="toast-message">${message}</span> | ||
| `; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent HTML injection in toast messages.
message is interpolated into innerHTML. Django messages/form errors can contain user-controlled text; content that was safe as text can be reparsed as markup here. Build the spans with textContent instead.
Proposed fix
- toast.innerHTML = `
- <span class="toast-icon">${icons[type] || icons.info}</span>
- <span class="toast-message">${message}</span>
- `;
+ const icon = document.createElement("span");
+ icon.className = "toast-icon";
+ icon.textContent = icons[type] || icons.info;
+
+ const messageElement = document.createElement("span");
+ messageElement.className = "toast-message";
+ messageElement.textContent = message;
+
+ toast.append(icon, messageElement);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toast.innerHTML = ` | |
| <span class="toast-icon">${icons[type] || icons.info}</span> | |
| <span class="toast-message">${message}</span> | |
| `; | |
| const icon = document.createElement("span"); | |
| icon.className = "toast-icon"; | |
| icon.textContent = icons[type] || icons.info; | |
| const messageElement = document.createElement("span"); | |
| messageElement.className = "toast-message"; | |
| messageElement.textContent = message; | |
| toast.append(icon, messageElement); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@game/static/game/js/toast.js` around lines 38 - 41, Update the toast
rendering around the innerHTML assignment to avoid interpolating the message
into markup. Create the icon and message span elements separately, assign their
values with textContent, and append them to toast while preserving the existing
icon fallback and display order.
Source: Linters/SAST tools
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Cinzel:wght=400;700&display=swap" | ||
| rel="stylesheet" | ||
| /> | ||
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Inter:wght=400;500;600;700&display=swap" | ||
| rel="stylesheet" | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Invalid Google Fonts CSS2 syntax (wght= instead of wght@) in both forum templates. The Cinzel and Inter requests are malformed, so those weights never load and the pages fall back to default fonts; the JetBrains Mono link right below uses the correct wght@.
game/templates/game/forum_detail.html#L13-L20: changefamily=Cinzel:wght=400;700andfamily=Inter:wght=400;500;600;700to usewght@.game/templates/game/forum_list.html#L13-L20: apply the identicalwght@correction to both links.
📍 Affects 2 files
game/templates/game/forum_detail.html#L13-L20(this comment)game/templates/game/forum_list.html#L13-L20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@game/templates/game/forum_detail.html` around lines 13 - 20, Correct the
Google Fonts CSS2 syntax in the Cinzel and Inter links in
game/templates/game/forum_detail.html lines 13-20 by replacing wght= with wght@,
and apply the same correction to both links in
game/templates/game/forum_list.html lines 13-20. Leave the existing font
families and weight lists unchanged.
| Hello {{ user.username }}, You requested a password reset for your Checkora | ||
| account. Username: {{ user.username }} Click the link below to set a new | ||
| password: http://{{ domain }}/password-reset-confirm/{{ uid }}/{{ token }}/ If | ||
| you did not request this, please ignore this email. Your password will remain | ||
| unchanged. — The Checkora Team ♟️ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Formatter collapsed a plain-text email body; restore the line breaks and exclude this file.
This template is sent as plain text, so the removed blank lines/newlines turn the message into one wrapped paragraph, and the reset URL now runs directly into the following sentence — mail-client autolinkers can absorb the trailing text into the href. Add it to .prettierignore (and the HTMLHint ignore, which is also flagging doctype-first) and restore the original layout:
✉️ Suggested content
Hello {{ user.username }},
You requested a password reset for your Checkora account.
Username: {{ user.username }}
Click the link below to set a new password:
http://{{ domain }}/password-reset-confirm/{{ uid }}/{{ token }}/
If you did not request this, please ignore this email. Your password will remain unchanged.
— The Checkora Team ♟️
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@game/templates/game/password_reset_email.html` around lines 1 - 5, Restore
the original plaintext layout in the password reset email template, preserving
the shown blank lines and placing the reset URL on its own line with a trailing
newline before the following sentence. Exclude this template from both Prettier
and HTMLHint ignore configuration so formatters and the doctype-first rule do
not modify or flag it.
Source: Linters/SAST tools
| <div | ||
| class="rule-nav-item active" | ||
| onclick="showRule('basics')" | ||
| id="nav-basics" | ||
| > | ||
| <span class="icon">📖</span> The Basics | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('pieces')" id="nav-pieces"> | ||
| <span class="icon">♟</span> Piece Moves | ||
| <span class="icon">♟</span> Piece Moves | ||
| </div> | ||
| <button class="rule-nav-item" onclick="showRule('values')" id="nav-values"> | ||
| <span class="icon">🧮</span> Piece Values & Scoring | ||
| <button | ||
| class="rule-nav-item" | ||
| onclick="showRule('values')" | ||
| id="nav-values" | ||
| > | ||
| <span class="icon">🧮</span> Piece Values & Scoring | ||
| </button> | ||
| <div class="rule-nav-item" onclick="showRule('pawn')" id="nav-pawn"> | ||
| <span class="icon">🔼</span> Pawn Special Rules | ||
| <span class="icon">🔼</span> Pawn Special Rules | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('castling')" id="nav-castling"> | ||
| <span class="icon">🏰</span> Castling | ||
| <div | ||
| class="rule-nav-item" | ||
| onclick="showRule('castling')" | ||
| id="nav-castling" | ||
| > | ||
| <span class="icon">🏰</span> Castling | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('enpassant')" id="nav-enpassant"> | ||
| <span class="icon">💨</span> En Passant | ||
| <div | ||
| class="rule-nav-item" | ||
| onclick="showRule('enpassant')" | ||
| id="nav-enpassant" | ||
| > | ||
| <span class="icon">💨</span> En Passant | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('check')" id="nav-check"> | ||
| <span class="icon">⚠️</span> Check & Checkmate | ||
| <span class="icon">⚠️</span> Check & Checkmate | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('stalemate')" id="nav-stalemate"> | ||
| <span class="icon">🤝</span> Stalemate & Draw | ||
| <div | ||
| class="rule-nav-item" | ||
| onclick="showRule('stalemate')" | ||
| id="nav-stalemate" | ||
| > | ||
| <span class="icon">🤝</span> Stalemate & Draw | ||
| </div> | ||
| <div class="rule-nav-item" onclick="showRule('promotion')" id="nav-promotion"> | ||
| <span class="icon">👑</span> Pawn Promotion | ||
| <div | ||
| class="rule-nav-item" | ||
| onclick="showRule('promotion')" | ||
| id="nav-promotion" | ||
| > | ||
| <span class="icon">👑</span> Pawn Promotion | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Sidebar navigation is keyboard-inaccessible: div + onclick (only nav-values is a real button).
Every rule entry except "Piece Values & Scoring" is a <div onclick=...>, so it can't be reached with Tab or activated with Enter/Space — the whole rulebook is unusable without a mouse. The .rule-nav-item:focus-visible style at line 191 suggests buttons were intended. The same pattern applies to .piece-card (lines 898-945) and .step items.
♿ Proposed fix (apply to all nav items)
- <div
- class="rule-nav-item active"
- onclick="showRule('basics')"
- id="nav-basics"
- >
- <span class="icon">📖</span> The Basics
- </div>
+ <button
+ type="button"
+ class="rule-nav-item active"
+ onclick="showRule('basics')"
+ id="nav-basics"
+ >
+ <span class="icon">📖</span> The Basics
+ </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@game/templates/game/rules.html` around lines 794 - 844, Replace the clickable
div elements in the rule navigation around showRule with keyboard-accessible
button elements, including nav-basics, nav-pieces, nav-pawn, nav-castling,
nav-enpassant, nav-check, nav-stalemate, and nav-promotion; preserve their IDs,
labels, icons, active state, and click behavior so every entry can be reached
and activated via keyboard.
Automated code quality improvements using official language tools.
Tools applied:
Diff stats:
.github/ISSUE_TEMPLATE/bug_report.md | 5 +-
.github/ISSUE_TEMPLATE/feature_request.md | 5 +-
.github/workflows/ci.yml | 1016 +-
.github/workflows/contributors.yml | 142 +-
.github/workflows/label-gssoc.yml | 108 +-
.github/workflows/pr-guardian.yml | 490 +-
CODE_OF_CONDUCT.md | 20 +-
CONTRIBUTING.md | 71 +-
README.md | 328 +-
board.test.js | 103 +-
docs/API.md | 512 +-
docs/API_WALKTHROUGH.md | 166 +-
docs/SECURITY_HEADERS_AUDIT.md | 13 +-
docs/development.md | 10 +-
docs/engine_architecture.md | 37 +-
docs/testing.md | 50 +-
game/static/game/css/404.css | 182 +-
game/static/game/css/500.css | 226 +-
game/static/game/css/auth.css | 687 +-
game/static/game/css/forum.css | 534 +-
game/static/game/css/landing.css | 3608 +-
game/static/game/css/lesson.css | 469 +-
game/static/game/css/opening_trainer.css | 88 +-
game/static/game/css/preloading.css | 373 +-
game/static/game/css/toast.css | 132 +-
game/static/game/js/auth.js | 100 +-
game/static/game/js/board.js | 8312 +-
game/static/game/js/dropdown.js | 64 +-
game/static/game/js/lesson_board.js | 183 +-
game/static/game/js/lesson_coordinates.js | 60 +-
game/static/game/js/lesson_demo.js | 191 +-
game/static/game/js/lesson_practice.js | 734 +-
game/static/game/js/opening_trainer.js | 110 +-
game/static/game/js/roadmap_connectors.js | 112 +-
game/static/game/js/stockfish.js | 136673 +++++++++++++++++++-
game/static/game/js/theme.js | 114 +-
game/static/game/js/toast.js | 168 +-
game/templates/404.html | 48 +-
game/templates/500.html | 55 +-
game/templates/game/achievements.html | 527 +-
game/templates/game/board.html | 2991 +-
game/templates/game/delete_account.html | 403 +-
game/templates/game/delete_success.html | 260 +-
game/templates/game/forum_detail.html | 214 +-
game/templates/game/forum_list.html | 184 +-
game/templates/game/forum_new.html | 153 +-
game/templates/game/includes/messages.html | 13 +-
game/templates/game/leaderboard.html | 171 +-
game/templates/game/lesson_map.html | 894 +-
game/templates/game/lessons.html | 782 +-
game/templates/game/login.html | 117 +-
game/templates/game/opening_detail.html | 79 +-
game/templates/game/opening_trainer.html | 64 +-
game/templates/game/password_reset.html | 137 +-
game/templates/game/password_reset_complete.html | 58 +-
game/templates/game/password_reset_confirm.html | 105 +-
game/templates/game/password_reset_done.html | 60 +-
game/templates/game/password_reset_email.html | 19 +-
game/templates/game/preloading.html | 333 +-
game/templates/game/register.html | 167 +-
game/templates/game/rules.html | 2888 +-
game/templates/game/stats.html | 794 +-
game/templates/game/verify_otp.html | 229 +-
game/templates/game/welcome_email.html | 180 +-
structure.md | 57 +-
vercel.json | 4 +-
66 files changed, 153626 insertions(+), 14556 deletions(-)
This PR contains only automated fixes from linters/formatters (eslint, prettier, black, rustfmt, etc.).
No manual changes or AI-generated code.
Summary by CodeRabbit
New Features
Documentation
Style