better codex prompt

Author: Zvonimir

.codex/review-prompt.md

@@ -48,40 +48,50 @@ If any check fails, skip the comment.
 ### Pass 1: Blockers
 
 #### Correctness
+
 - Logic errors, off-by-one, null/undefined handling, incorrect assumptions, race conditions.
 - Boundary conditions — empty arrays, null inputs, zero values, maximum values.
 - Error handling — swallowed errors, missing error propagation, unhelpful error messages. Do not flag missing error handling for internal code that cannot reasonably fail.
+- Streaming/multipart handlers — verify a request cannot send multiple responses (e.g., multi-file parts triggering repeated `res.json()` calls). If a route expects one file, ensure parser limits and single-response guards exist.
 
 #### Security
+
 - Injection risks (SQL, command, XSS) when handling user input.
 - Hardcoded secrets — API keys, passwords, tokens in code.
 - Missing input validation at system boundaries (user input, external APIs). Not for internal function calls.
 - Auth bypass, privilege escalation, or missing authorization checks.
+- Filesystem path confinement — when IDs/paths come from requests, verify storage layers enforce root containment via resolved-path checks; do not rely only on caller-side sanitization.
 
 #### API Compatibility
+
 - Breaking changes to API response schemas or status codes without migration path.
 - Removed or renamed API endpoints, query parameters, or response fields that existing consumers depend on.
 - Database schema changes that require migration or backfill.
 - MCP tool signature changes (renamed tools, changed input schemas) that break existing clients.
+- HTTP status semantics — ensure client/input errors are 4xx and unexpected internal failures are 5xx; blanket 400 handling in catch-all paths is a correctness/API contract issue.
 
 #### Tests for Changed Behavior
+
 - New behavior must have corresponding tests covering core functionality and error handling.
 - Bug fixes must include a regression test that would have caught the original bug.
 - Changed behavior must have updated tests reflecting the new expectations.
 - If tests are present but brittle (testing implementation details rather than behavior), flag it.
+- For single-file upload endpoints, look for regression coverage of multi-file/malformed multipart inputs and confirm no double-response behavior.
 
 Missing tests for changed behavior are blockers (`🔴 Bug`) only when the change affects user-facing behavior, API contracts, or data integrity. Missing tests for internal refactors or trivial changes are `🟡 Issue`.
 
 ### Pass 2: Maintainability
 
 #### Code Bloat and Unnecessary Complexity
+
 - **Excessive code** — More lines than necessary. Could this be done in fewer lines without sacrificing clarity?
 - **Over-engineering** — Abstractions, helpers, or utilities for one-time operations. Premature generalization. Feature flags or config for things that could just be code.
 - **Speculative generality** — Code handling hypothetical future requirements nobody asked for.
 - **Dead code** — Unused variables, unreachable branches, commented-out code.
 - **Duplicate code** — Same logic repeated instead of extracted. But do not suggest extraction for only 2-3 similar lines — that is premature abstraction.
 
 #### Readability and Naming
+
 - **Confusing variable/function names** — Names that don't describe what the thing is or does. Generic names like `data`, `result`, `item`, `temp`, `val` when a specific name would be clearer.
 - **Misleading names** — Names that suggest different behavior than what the code does.
 - **Inconsistent naming** — Not following conventions in the rest of the codebase.
@@ -91,20 +101,25 @@ Missing tests for changed behavior are blockers (`🔴 Bug`) only when the chang
 - **Unclear control flow** — Complex conditionals that could be simplified or decomposed.
 
 #### Architecture and Pattern Violations
+
 - **Inline validation instead of Zod schemas** — Validation logic written in code (if/else checks, manual type coercion) instead of using Zod schemas in `openAPIRoute()`. All request validation belongs in the schema, not handler code. This applies to both API routes and MCP tool `inputSchema`.
 - **Missing `openAPIRoute()` wrapper** — API endpoints defined without the OpenAPI wrapper.
 - **Wrong import paths in tests** — Tests importing from `src/` instead of `dist/`.
 - **Missing test categories** — Tests without "Core Functionality" and "Error Handling" describe blocks.
 - **Mixing concerns** — Route handlers doing business logic, database queries in API handlers, etc.
+- **Cross-provider behavior drift** — When multiple providers/implementations exist, verify shared options and output semantics behave consistently unless explicitly documented otherwise.
 
 #### Hardcoded Values and Magic Constants
+
 Flag only when the value is:
+
 - **Reused 3+ times** in touched files or the diff — should be a named constant.
 - **Domain-significant** — timeout values, retry counts, port numbers, API URLs, status messages. Even if used once, these belong in constants or environment variables.
 
 Do not flag one-off numeric literals that are self-explanatory in context (e.g., `array.slice(0, 2)`, `Math.round(x * 100) / 100`).
 
 #### Performance (Only Obvious Issues)
+
 - N+1 queries — database queries inside loops.
 - Blocking operations in async contexts — synchronous I/O in async code.
 - Unnecessary work in hot paths — redundant allocations, repeated computations.
@@ -120,6 +135,7 @@ Do not flag one-off numeric literals that are self-explanatory in context (e.g.,
 ## Comment Format
 
 Use severity prefixes:
+
 - `🔴 Bug:` — Correctness error, security issue, API break, data integrity risk. Will cause incorrect behavior.
 - `🟡 Issue:` — Code quality problem that should be fixed. Bloated code, bad naming, pattern violation, missing tests.
 - `🔵 Nit:` — Minor improvement, optional.