Loading...
Loading...
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.
npx skill4agent add groeimetai/snow-flow code-review// 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// ❌ 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);// ❌ DANGEROUS - Unescaped output
gs.addInfoMessage(userInput);
// ✅ SAFE - Escape HTML
gs.addInfoMessage(GlideStringUtil.escapeHTML(userInput));// ❌ 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
}// ❌ DANGEROUS - Logging sensitive data
gs.info('Password: ' + password);
gs.info('API Key: ' + apiKey);
// ✅ SAFE - Never log credentials
gs.info('Authentication attempt for user: ' + username);// ❌ 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()) { }// ❌ 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();// ❌ 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)) { }// ❌ 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;// ❌ 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();
}// ❌ 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) { }// ❌ 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;
}// ❌ BAD - No context in logs
gs.info('Error occurred');
// ✅ GOOD - Contextual logging
gs.info('[MyScriptInclude.process] Processing incident: ' +
current.number + ', user: ' + gs.getUserName());// ❌ DANGEROUS - Can cause infinite loop
current.update(); // In a Before rule
// ✅ SAFE - Use workflow control
current.setWorkflow(false);
current.update();
current.setWorkflow(true);// ❌ 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## 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...| Level | Action | Examples |
|---|---|---|
| CRITICAL | Must fix before deployment | Security vulnerabilities, ES6 syntax |
| HIGH | Should fix | Performance issues, missing error handling |
| MEDIUM | Recommend fixing | Code quality, hard-coded values |
| LOW | Consider fixing | Style, minor improvements |