مراجع Pull Request بالذكاء الاصطناعي: احصل على نظر مهندس أول على كودك قبل كل Merge
Why this prompt matters
Most code bugs that reach production are not found in reviews because reviews are inconsistent — the reviewer is tired, distracted, or focuses on style while missing logic errors. A structured prompt with defined categories and required severity levels forces coverage of the issues that actually matter: correctness, security, and error handling. It also produces reviews that explain the why behind every finding, which builds understanding rather than just producing a checklist of changes to make.
What we use it for
You have a PR open, it's been waiting for review for two days, and the person who normally reviews your security-sensitive code is OOO. Or you are a solo developer shipping a feature and want a second pair of eyes before deploying. Or you are a senior engineer who wants to pre-check your own work before asking colleagues to spend time on it. This prompt gives you a structured review that covers the categories most likely to become production incidents.
Prompt
Role: Act as a senior software engineer with 10+ years of experience doing code reviews. You have a strong bias toward correctness, maintainability, and security. You write reviews that teach — not just list problems — because you want the author to understand the why, not just fix the what. Context: I am submitting a pull request for review. The code is written in [LANGUAGE: e.g., Python / TypeScript / Go / Rust / Java]. The codebase is [TYPE: e.g., a REST API / a frontend React app / a CLI tool / a data pipeline]. The PR is doing the following: [DESCRIBE WHAT THE PR DOES IN 1-3 SENTENCES]. The most important things for this codebase are [PRIORITIES: e.g., performance / security / readability / test coverage / backward compatibility]. Task: Review this code as a senior engineer doing a thorough pre-merge review. Go beyond surface-level issues. Look for: 1. Correctness bugs — logic errors, edge cases, off-by-one errors, null/undefined handling, incorrect assumptions about input 2. Security vulnerabilities — injection risks, authentication flaws, insecure data handling, exposed secrets, unsafe deserialization 3. Performance issues — unnecessary allocations, N+1 query patterns, blocking operations, missing indexes, inefficient algorithms 4. Maintainability problems — functions doing too much, poor naming, missing abstractions, code duplication, hard-coded values that should be configurable 5. Error handling gaps — unhandled exceptions, swallowed errors, missing logging for failure cases, no retry logic where needed 6. Test coverage — are the happy path, error cases, and edge cases all tested? Are the tests testing the right things? 7. API and interface design — is this the right abstraction? Will this be easy to change later? Are there breaking changes? 8. Concurrency and race conditions — if applicable to the language and context Constraints: - Every finding must include: the specific location or pattern (line, function, or concept), the severity (Critical / High / Medium / Low / Nitpick), and the explanation of why it is a problem — not just that it is. - Do not invent problems. Only flag what you can see in the code. - If something is genuinely well done, say so briefly — a good review acknowledges what works. - Do not rewrite the entire function in your review unless the existing implementation is fundamentally broken. - Separate blocking issues (must fix before merge) from non-blocking suggestions (good to fix, but optional for this PR). - If you lack enough context to assess something, say so explicitly rather than guessing. Output Format: 1. Summary (2-3 sentences: overall assessment, biggest concerns, merge readiness) 2. Blocking Issues — must fix before merge (severity, location, explanation, suggested fix) 3. Non-Blocking Issues — recommended improvements (severity, location, explanation) 4. Security Check — pass / flag / needs-closer-look, with brief reasoning 5. Test Coverage Assessment — adequate / thin / missing, with specific gaps 6. What is well done (1-3 specific positives) 7. Suggested questions for the author (things you need clarification on before approving) Code to review: [PASTE YOUR DIFF OR CODE HERE]
Result
SUMMARY This PR adds a user authentication endpoint with JWT token issuance. The core logic is sound and the structure is clean, but there are two blocking security issues — a timing attack vulnerability in password comparison and missing token expiry validation — plus one high-severity missing error case that could expose internal stack traces to API consumers. Test coverage is thin on the error paths. Not merge-ready as-is, but close.
BLOCKING ISSUES
1. Timing attack in password comparison Severity: Critical Location: comparePasswords() function, line 34 Problem: Using a simple string equality check (userPassword === hashedPassword) for password comparison is vulnerable to timing attacks. An attacker who sends many requests can measure response time differences to enumerate valid usernames or reconstruct password hashes character by character. Fix: Replace with a constant-time comparison function. In Node.js use crypto.timingSafeEqual(). In Python use hmac.compare_digest(). Never use == for secret comparisons.
2. JWT token expiry not validated on refresh Severity: Critical Location: refreshToken() handler, line 89 Problem: The refresh endpoint checks token signature validity but does not verify the exp claim against the current timestamp. An expired access token can be used to generate a fresh token indefinitely, effectively making tokens immortal and defeating the purpose of expiry. Fix: Add explicit exp check: if (decoded.exp < Math.floor(Date.now() / 1000)) throw new TokenExpiredError(). Most JWT libraries have a built-in option for this — set ignoreExpiration: false explicitly rather than relying on default behavior.
3. Unhandled database error exposes stack trace Severity: High Location: loginHandler() catch block, line 67 Problem: The catch block re-throws the raw database error object directly to the Express error handler, which in the current middleware configuration will serialize the full stack trace into the 500 response body. This leaks internal file paths, dependency versions, and query structure to API consumers. Fix: Log the raw error internally with a correlation ID, then return a generic error response: res.status(500).json({ error: 'Authentication service error', correlationId: uuid }).
NON-BLOCKING ISSUES
4. Magic numbers for token expiry Severity: Medium Location: Lines 45 and 102 Problem: Token expiry durations are hard-coded as 900 and 86400 (seconds). These should be named constants or environment-configurable values so they can be changed without a code deployment, and so the next developer understands what these numbers represent. Suggestion: const ACCESS_TOKEN_TTL_SECONDS = parseInt(process.env.ACCESS_TOKEN_TTL ?? '900', 10)
5. Missing rate limiting on login endpoint Severity: Medium Location: loginHandler route registration Problem: There is no rate limiting on POST /auth/login. This allows unlimited password brute-force attempts. This might be handled at the infrastructure level (API gateway, load balancer), but if not, it should be at the application level. Suggestion: Add express-rate-limit or equivalent middleware, or at minimum document that infrastructure-level rate limiting covers this endpoint.
6. Function does two things Severity: Low (Nitpick) Location: validateAndDecodeToken(), line 78 Problem: This function both validates the token signature AND extracts the user ID from the payload, returning both as a tuple. These are two distinct concerns and should probably be separate functions, or the return value should be a typed object with clearly named fields rather than a positional tuple.
SECURITY CHECK: FLAG Blocking issues (1 and 2) make this endpoint insecure in its current state. After fixes, recommend a second security-focused pass.
TEST COVERAGE ASSESSMENT: THIN Happy path (valid credentials → token returned) is covered. Missing: - Expired token refresh attempt (the blocking issue above) - Invalid password attempt — verify 401, not 500 - Malformed JWT in Authorization header - Database connection failure during login - Rate limiting behavior (if applicable)
WHAT IS WELL DONE 1. Token rotation on refresh is implemented correctly — the old refresh token is invalidated when a new one is issued. This is easy to get wrong and it is done right here. 2. Password hashing uses bcrypt with a cost factor of 12, which is appropriate. 3. The middleware composition is clean and easy to follow.
QUESTIONS FOR THE AUTHOR 1. Is there infrastructure-level rate limiting on this endpoint? If yes, please document it in the code or README so it is not duplicated. 2. Is token revocation (logout/blocklist) in scope for this PR or a follow-up? The current architecture does not support it, which should be a conscious decision, not an oversight.
معظم المطورين اختبروا طرفي طيف مراجعة الكود. في أحد الطرفين: المُوافق الذي ينقر خلال ثلاثين ثانية ويترك رمزًا تعبيريًا واحدًا. وفي الطرف الآخر: المهندس الأول الذي يعود بصفحتين من الملاحظات سطرًا بسطر تغطي الصحة، الأمان، القابلية للصيانة، والاختبار — ويشرح لماذا كل مشكلة مهمة بدلاً من مجرد وضع علامة خطأ. النوع الثاني من المراجعة يجعلك مطورًا أفضل. وهو أيضًا النوع الذي يستغرق وقتًا للحصول عليه، خاصة من الأشخاص المثقلين بالفعل.
هذا البرومبت (Prompt) هو محاولة لجلب بعض من تلك التجربة الثانية إلى كل Pull Request. إنه ينظم المراجعة في الفئات المهمة — أخطاء الصحة، الثغرات الأمنية، الأداء، معالجة الأخطاء، تغطية الاختبار — ويطلب من النموذج شرح كل نتيجة مع الموقع، الخطورة، والسبب قبل اقتراح إصلاح.
مشكلة برومبتات "راجع كودي" العامة
اطلب من نموذج لغوي أن "يراجع هذا الكود" دون هيكل وستحصل عادةً على قائمة مسطحة من الملاحظات دون أولوية ودون شرح للخطورة. قد يضع النموذج عدم تناسق في تسمية المتغيرات في نفس مستوى إلحاحية ثغرة هجوم التوقيت. قد يهنئك على هيكلك النظيف ثم يفشل في ملاحظة المؤشر الفارغ (null pointer) الذي يتسبب في تعطل الإنتاج عند أول إدخال فارغ.
البنية في هذا البرومبت تفرض الأولوية. يتم فصل المشكلات العرقلة — الأشياء التي يجب إصلاحها قبل الدمج — عن الاقتراحات غير العرقلة. كل نتيجة تتطلب مستوى خطورة (حرج، عال، متوسط، منخفض، تدقيق دقيق)، موقع محدد، وشرح لسبب وجود المشكلة بدلاً من مجرد ما يجب فعله حيالها. فحص الأمان وتقييم تغطية الاختبار هما قسمان صريحان وليسا أفكارًا لاحقة.
كيفية الاستخدام
املأ أربعة حقول سياق في الأعلى: لغة البرمجة، نوع قاعدة الكود، وصف من جملة إلى ثلاث جمل لما يفعله PR، والأولويات الأكثر أهمية لقاعدة الكود هذه. ثم الصق الفرق (diff) أو الكود ذي الصلة في الأسفل.
حقول السياق تحدث فرقًا ذا معنى في جودة المخرجات. النموذج الذي يعرف أن هذا واجهة برمجة تطبيقات REST بلغة Python ذات أولوية أمان سيلاحظ أشياء مختلفة عن ذلك الذي يراجع كود CLI بلغة Go حيث الأداء هو الأولوية. وصف ما يحاول PR تحقيقه يساعد النموذج على تقييم ما إذا كان التنفيذ يحقق الهدف بالفعل، بدلاً من مجرد فحص الأسلوب.
بالنسبة لـ PRs الكبيرة جدًا، الصق الملفات الأكثر أهمية أولاً — المصادقة، معالجة البيانات، واجهات API العامة — واطلب تمريرة منفصلة لتغييرات الأدوات أو التكوين. معظم النماذج التي تحتوي على نوافذ سياق 100k+ يمكنها التعامل مع PRs المتوسطة في تمريرة واحدة.
ما ستحصل عليه
تنتج صيغة المخرجات سبعة أقسام: ملخص بلغة بسيطة مع تقييم جاهزية الدمج، مشكلات عرقلة مع إصلاحات، اقتراحات غير عرقلة، حكم أمني، تقييم تغطية الاختبار مع فجوات محددة، تقدير لما هو جيد، وأسئلة توضيحية للمؤلف. يظهر مثال المخرجات في هذه التدوينة مراجعة لنقطة نهاية مصادقة JWT — لاحظ كيف أن نتيجة هجوم التوقيت تشرح آلية الهجوم ووظيفة المكتبة المحددة لاستخدامها في الإصلاح.
قسم «ما تم بشكل جيد» ليس مجرد لطف اختياري. المراجعات التي تعدد المشكلات فقط تفقد الإشارة التي يرسلها الكود الجيد — أن المؤلف يفهم نية التصميم ويجب أن يستمر في فعل ما يعمل. كما أنه يقدم نموذجًا لما تبدو عليه مراجعة الكود المدروسة فعليًا للمطورين الذين لم يحصلوا بعد على مهندس أول كمرشد.
قيود جديرة بالمعرفة
ينتج هذا البرومبت نتائج أفضل على الكود الذي تعرض له النموذج أثناء التدريب. سيكتشف مشكلات أكثر في Python و TypeScript و Java و Go مقارنة باللغات المتخصصة في مجالات ضيقة. لا يمكنه تحليل سلوك وقت التشغيل، أو تتبعات التنميط، أو سجلات الإنتاج — فقط ما هو مرئي في الفرق. بالنسبة للأنظمة الحساسة أمنيًا، يجب أن تكمل مراجعة الكود بالذكاء الاصطناعي مراجعة الأمن البشري بدلاً من استبدالها، خاصة بالنسبة للتنفيذات التشفيرية وتدفقات المصادقة.
سيضع النموذج أحيانًا نتائج إيجابية كاذبة — مشكلات ليست في الواقع أخطاء بالنظر إلى السياق الذي لا يمتلكه. القيد الذي يطلب منه وضع علامة صريحة عندما يفتقر إلى السياق يساعد في تقليل ذلك، لكن معاملة كل نتيجة كنقطة بداية للتحقيق بدلاً من حكم نهائي تنتج نتائج أفضل.