AgentSkillsCN

code-review

当用户要求“评审代码”、“代码评审”、“检查我的代码”、“审计代码”、“查找 Bug”、“安全评审”、“性能评审”或任何 ServiceNow 代码质量评估时,应使用此技能。

SKILL.md
--- frontmatter
name: code-review
description: This skill should be used when the user asks to "review code", "code review", "check my code", "audit code", "find bugs", "security review", "performance review", or any ServiceNow code quality assessment.
license: Apache-2.0
compatibility: Designed for Snow-Code and ServiceNow development
metadata:
  author: groeimetai
  version: "1.0.0"
  category: servicenow
tools:
  - snow_find_artifact
  - snow_execute_script_with_output
  - snow_analyze_artifact

ServiceNow Code Review Checklist

Use this checklist when reviewing ServiceNow server-side code (Business Rules, Script Includes, Scheduled Jobs, etc.).

1. ES5 Compliance (CRITICAL)

javascript
// CHECK FOR THESE ES6+ VIOLATIONS:
const x = 5;           // ❌ Use var
let items = [];        // ❌ Use var
() => {}               // ❌ Use function()
`template ${var}`      // ❌ Use 'string ' + var
for (x of arr)         // ❌ Use traditional for loop
{a, b} = obj           // ❌ Use obj.a, obj.b

Action: Flag ALL ES6+ syntax as CRITICAL errors.

2. Security Issues

2.1 SQL/GlideRecord Injection

javascript
// ❌ DANGEROUS - User input directly in query
gr.addEncodedQuery(userInput)
gr.addQuery("field", userInput) // OK if validated

// ✅ SAFE - Validate and sanitize
var safeInput = new GlideSysAttachment().cleanFileName(userInput)
gr.addQuery("field", safeInput)

2.2 Cross-Site Scripting (XSS)

javascript
// ❌ DANGEROUS - Unescaped output
gs.addInfoMessage(userInput)

// ✅ SAFE - Escape HTML
gs.addInfoMessage(GlideStringUtil.escapeHTML(userInput))

2.3 Access Control

javascript
// ❌ MISSING - No ACL check
var gr = new GlideRecord("sys_user")
gr.get(userProvidedSysId)

// ✅ SAFE - Check permissions
var gr = new GlideRecord("sys_user")
if (gr.get(userProvidedSysId) && gr.canRead()) {
  // Process record
}

2.4 Sensitive Data Exposure

javascript
// ❌ DANGEROUS - Logging sensitive data
gs.info("Password: " + password)
gs.info("API Key: " + apiKey)

// ✅ SAFE - Never log credentials
gs.info("Authentication attempt for user: " + username)

3. Performance Issues

3.1 Queries in Loops

javascript
// ❌ SLOW - N+1 query problem
for (var i = 0; i < userIds.length; i++) {
  var gr = new GlideRecord("sys_user")
  gr.get(userIds[i]) // Query for each user!
}

// ✅ FAST - Single query
var gr = new GlideRecord("sys_user")
gr.addQuery("sys_id", "IN", userIds.join(","))
gr.query()
while (gr.next()) {}

3.2 Missing setLimit()

javascript
// ❌ SLOW - Could return millions of records
var gr = new GlideRecord("incident")
gr.query()

// ✅ FAST - Limit results
var gr = new GlideRecord("incident")
gr.setLimit(1000)
gr.query()

3.3 Unnecessary Queries

javascript
// ❌ WASTEFUL - Query just to check existence
var gr = new GlideRecord("incident")
gr.addQuery("number", incNumber)
gr.query()
if (gr.getRowCount() > 0) {
}

// ✅ EFFICIENT - Use get() for single record
var gr = new GlideRecord("incident")
if (gr.get("number", incNumber)) {
}

3.4 GlideRecord vs GlideAggregate

javascript
// ❌ SLOW - Counting with loop
var count = 0
var gr = new GlideRecord("incident")
gr.addQuery("active", true)
gr.query()
while (gr.next()) count++

// ✅ FAST - Use GlideAggregate
var ga = new GlideAggregate("incident")
ga.addQuery("active", true)
ga.addAggregate("COUNT")
ga.query()
var count = ga.next() ? ga.getAggregate("COUNT") : 0

4. Code Quality Issues

4.1 Hard-coded sys_ids

javascript
// ❌ BAD - Hard-coded sys_id (breaks across instances)
var assignmentGroup = "681ccaf9c0a8016400b98a06818d57c7"

// ✅ GOOD - Use property or lookup
var assignmentGroup = gs.getProperty("my.default.assignment.group")
// OR
var gr = new GlideRecord("sys_user_group")
if (gr.get("name", "Service Desk")) {
  var assignmentGroup = gr.getUniqueValue()
}

4.2 Magic Numbers/Strings

javascript
// ❌ BAD - Magic numbers
if (current.state == 6) {
}
if (current.priority == 1) {
}

// ✅ GOOD - Named constants or comments
var STATE_RESOLVED = 6
var PRIORITY_CRITICAL = 1
if (current.state == STATE_RESOLVED) {
}

4.3 Missing Error Handling

javascript
// ❌ BAD - No error handling
var response = request.execute()
var data = JSON.parse(response.getBody())

// ✅ GOOD - Proper error handling
try {
  var response = request.execute()
  var status = response.getStatusCode()
  if (status != 200) {
    gs.error("API call failed: " + status)
    return null
  }
  var data = JSON.parse(response.getBody())
} catch (e) {
  gs.error("Exception: " + e.message)
  return null
}

4.4 Proper Logging

javascript
// ❌ BAD - No context in logs
gs.info("Error occurred")

// ✅ GOOD - Contextual logging
gs.info("[MyScriptInclude.process] Processing incident: " + current.number + ", user: " + gs.getUserName())

5. Business Rule Specific

5.1 Recursion Prevention

javascript
// ❌ DANGEROUS - Can cause infinite loop
current.update() // In a Before rule

// ✅ SAFE - Use workflow control
current.setWorkflow(false)
current.update()
current.setWorkflow(true)

5.2 Appropriate Rule Type

javascript
// ❌ WRONG - Heavy processing in Before rule
// Before rules should be fast!

// ✅ RIGHT - Use Async for heavy operations
// Move integrations and heavy processing to Async rules

6. Review Output Format

When reviewing code, structure your feedback as:

markdown
## Code Review Summary

### Critical Issues (Must Fix)

1. [SECURITY] Description...
2. [ES5] Description...

### Performance Issues (Should Fix)

1. [PERF] Description...

### Code Quality (Nice to Have)

1. [QUALITY] Description...

### Positive Observations

- Good use of...
- Well-structured...

7. Severity Levels

LevelActionExamples
CRITICALMust fix before deploymentSecurity vulnerabilities, ES6 syntax
HIGHShould fixPerformance issues, missing error handling
MEDIUMRecommend fixingCode quality, hard-coded values
LOWConsider fixingStyle, minor improvements