Repository navigation
Conversation
Validated local implementation: cb0b33977560b6f3d17c9ee521a0774b51f2b7ef
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74e8a6c476
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (ArchiveUnavailable) { | ||
| $this->addFlash('archive_error', 'Collection could not be submitted. Check your archive connection and try again.'); | ||
| } | ||
|
|
||
| return $this->redirectToRoute('app_player', ['provider' => $provider, 'username' => $username], Response::HTTP_SEE_OTHER); |
There was a problem hiding this comment.
When the archive accepts the POST but the response times out, is truncated, or is otherwise lost, this catch redirects to the player page, where player() generates a new submission_key. Following the displayed instruction to try again therefore sends a different idempotency key and can enqueue a duplicate collection; retain the submitted key across this ambiguous failure so the retry replays the original request.
Useful? React with 👍 / 👎.
| public function __construct(public readonly int $status = 503) | ||
| { | ||
| parent::__construct(404 === $status ? 'This player is not in your archive yet.' : 'Your archive is temporarily unavailable.'); |
There was a problem hiding this comment.
Because this exception is also used by game() and run(), any 404 from /v1/games/{id} or /v1/runs/{id} renders “This player is not in your archive yet.” A stale game link or missing collection therefore reports the wrong resource to the user; make the 404 message generic or pass context for player, game, and collection lookups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 74e8a6c476
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| #[Route('/players/{provider}/{username}/collect', name: 'app_player_collect', requirements: ['provider' => 'chess\.com|lichess', 'username' => '[A-Za-z0-9_-]{1,100}'], methods: ['POST'])] | ||
| public function collect(string $provider, string $username, Request $request, CrawlClient $crawl): Response |
There was a problem hiding this comment.
🛡️ Codex Security Review
When CHESS_CRAWL_TOKEN is configured and an untrusted peer can reach the web port, require authentication and authorization before this route. The main firewall has no authenticator or access rule, and the anonymous player page gives the same peer a CSRF token plus a fresh idempotency key, so CSRF does not stop a direct client from POSTing here. The handler then uses the server bearer credential to create imports of up to 1,000 games; sibling player, game, and run routes expose the same credential-backed access. Local-only documentation is not an enforcement boundary, while Compose publishes the web ports without a host-IP restriction.
Useful? React with 👍 / 👎.
Chess Dog currently has an application shell and a local preview. This adds its first connection to the stored player archive so a local installation can look up a player, collect a selected period, and inspect preserved moves and clocks.
Changes:
Validation: 15 PHPUnit tests / 87 assertions across unit and functional suites; Twig (21 files) and YAML (28 files) lint; production Tailwind build; Mate verified client service wiring with credentials redacted; git diff --check. Existing native browser lifecycle tests are left to CI. Live cross-repository API verification will be completed after the Crawl API foundation is integrated.
Dependencies and limits: requires the game-evidence API foundation in xormania/chess-crawl (PR #27 plus scoped API follow-up). Current UI collection caps at 1000 games / one year, and progress refreshes on demand. This is a local single-workspace client; SaaS application-user authentication, per-user workspace credential selection, full-history controls, working sets and live events are later increments. Unsupported variants preserve notation with an explicit interpretation status. No deployed SaaS or engine/Fair Play detector is claimed.
This draft targets restored dev from current master f6a996e, whose source tree matches the local reviewed release baseline. Leave draft and unmerged.
Closed after the maintainer clarified that this implementation task is restricted to chess-crawl. No merge is requested.