Skip to content

docs: add ai context#395

Open
lirantal wants to merge 1 commit intomainfrom
chore/ai-context
Open

docs: add ai context#395
lirantal wants to merge 1 commit intomainfrom
chore/ai-context

Conversation

@lirantal
Copy link
Owner

@lirantal lirantal commented Dec 6, 2025

User description

Description

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

  • I have updated the documentation (if required).
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Documentation


Description

  • Add comprehensive AI context instructions for Node.js development

  • Include error handling, file system APIs, and modern conventions

  • Add testing guidelines using Node.js built-in test runner

  • Configure MCP servers for development environment


Diagram Walkthrough

flowchart LR
  A["AI Context Instructions"] --> B["Error Handling"]
  A --> C["File System APIs"]
  A --> D["Modern Node.js Conventions"]
  A --> E["Module Resolution"]
  A --> F["Built-in Preferences"]
  A --> G["Testing Guidelines"]
  H["MCP Configuration"] --> I["Snyk Security"]
  H --> J["Node.js API Docs"]
  H --> K["Sequential Thinking"]
  L["GitHub Issue Prompt"] --> M["Implementation Guide"]
Loading

File Walkthrough

Relevant files
Documentation
8 files
error-handling.instructions.md
Structured error handling and diagnostics patterns             
+234/-0 
file-system-api.instructions.md
File system operations decision tree and patterns               
+147/-0 
general.instructions.md
General agent rules and coding principles                               
+15/-0   
modern-nodejs-conventions.instructions.md
Modern Node.js programming conventions and patterns           
+214/-0 
module-resolution.instructions.md
Module resolution strategy with import maps                           
+112/-0 
prefer-builtins-over-deps.instructions.md
Guidance on using Node.js built-in APIs over dependencies
+91/-0   
testing.instructions.md
Comprehensive testing guidelines for Node.js applications
+688/-0 
github-issue-impl.prompt.md
GitHub issue implementation workflow and guidelines           
+28/-0   
Configuration changes
1 files
mcp.json
Model Context Protocol server configuration                           
+28/-0   

@lirantal lirantal self-assigned this Dec 6, 2025
@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.36%. Comparing base (251b4e1) to head (42b4092).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #395   +/-   ##
=======================================
  Coverage   93.36%   93.36%           
=======================================
  Files          27       27           
  Lines        1220     1220           
  Branches      262      262           
=======================================
  Hits         1139     1139           
  Misses         72       72           
  Partials        9        9           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Logging Guidance: The new documentation adds guidance and examples but does not implement or mandate audit
logging of critical actions; verify that actual code changes elsewhere add audit trails if
required by this PR.

Referred Code
---
applyTo: '**'
description: Error Handling and Diagnostics in Node.js
---

# Error Handling & Diagnostics (Node.js/TS)

Goal: produce **structured, contextual errors** that are easy to debug, log, monitor, and chain—without losing the original failure cause.

---

## Decision Rules

- **App-level errors** → use a **custom error class** that captures `code`, `statusCode`, `context`, `timestamp`, and supports JSON serialization.
- **Wrapping lower-level failures** → rethrow with `new Error(msg, { cause })` (or your custom class with a `cause` field) to preserve the original error and stack.
- **TypeScript** → remember `catch (e)` is `unknown`; **narrow** or cast before using (`e as Error`) when passing as `cause`.
- **Diagnostics** → always include minimal, non-sensitive context (what, where, ids)—not secrets or PII.

---

## Structured Errors (baseline)


 ... (clipped 213 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Stack Exposure Risk: The guidance includes toJSON() returning stack and optional cause details which is fine
for internal logs but could leak internals if used in user-facing responses; confirm
downstream usage limits this to secure logs.

Referred Code
toJSON() {
  return {
    name: this.name,
    message: this.message,
    code: this.code,
    statusCode: this.statusCode,
    context: this.context,
    timestamp: this.timestamp,
    stack: this.stack,
    // Optional: include cause message safely if it is an Error
    cause: this.cause instanceof Error ? this.cause.message : this.cause,
  };
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive Context: Examples encourage adding contextual fields to errors and diagnostics; while they caution
against secrets/PII, verify that implementations do not log sensitive data especially via
context or diagnostics channels.

Referred Code
* **Always preserve the root cause** when layering errors: `throw new Error("high-level", { cause: err })`.
* **Prefer one custom base class** (`AppError`) with `code`, `statusCode`, `context`, `timestamp`. Keep it small and serializable.
* **Use context, not blobs:** add *pertinent* fields (ids, file paths, operation) to `context`; avoid secrets/large payloads.
* **TS-safe catches:** treat caught errors as `unknown`; refine (e.g., `instanceof Error`) or cast before accessing `.message`/passing as `cause`.
* **Log clearly:** when logging/serializing, include `error.toJSON?.()` if present, plus `error.cause` (recursively if you need deep chains).
* **User vs operator messages:** top-level `message` should be actionable (for logs/ops). Map to user-facing messages elsewhere.

---

## Error Handling Antipatterns (avoid)

* **Swallowing errors** or rethrowing without the original cause → loses vital context. Use `{ cause }`.
* **Throwing raw strings** (e.g., `throw "oops"`): breaks tooling/typing. Always throw `Error` or `AppError`.
* **Overstuffing context** with secrets/PII or huge objects (Responses/buffers) → log bloat, risk. Put *summaries* or safe fields in `context`; if you need a large object for local inspection, put it under `cause` carefully.
* **Multi-style errors** scattered across codebase (inconsistent shape) → standardize on `AppError`.

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Potential sensitive data logging: The error handling examples include logging context fields without explicit guidance on
sanitizing sensitive data from the context object

Referred Code
* **Use context, not blobs:** add *pertinent* fields (ids, file paths, operation) to `context`; avoid secrets/large payloads.
* **TS-safe catches:** treat caught errors as `unknown`; refine (e.g., `instanceof Error`) or cast before accessing `.message`/passing as `cause`.

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consider a more iterative, tool-based approach

Instead of adding a large, monolithic set of documentation-based rules, consider
using machine-readable tools like ESLint to enforce conventions. This approach
is more maintainable, less ambiguous for an AI, and aligns with industry best
practices.

Examples:

.github/instructions/modern-nodejs-conventions.instructions.md [13-16]
# Prefer ES Modules and Use the `node:` Prefix

**Why:**
ES Modules (`import`/`export`) are now the standard for modern JavaScript. Combined with the `node:` prefix, they make it unmistakably clear which dependencies come from the Node.js standard library versus installed packages. Agents should always use ES Modules unless they are explicitly modifying legacy CommonJS code.
.github/instructions/prefer-builtins-over-deps.instructions.md [40]
- **UUID v4:** Use `crypto.randomUUID()` instead of `uuid`.

Solution Walkthrough:

Before:

// The PR adds a large set of natural language instruction files.
.github/
  instructions/
    error-handling.instructions.md
    file-system-api.instructions.md
    modern-nodejs-conventions.instructions.md
    prefer-builtins-over-deps.instructions.md
    testing.instructions.md
    // ... and several other rule documents

After:

// The suggestion proposes replacing many documents with machine-readable rules.
.github/
  instructions/
    // Only keep high-level, non-lintable instructions.
    ai-agent-philosophy.md
    testing-strategy.md

// Add a linter configuration to enforce code style and conventions automatically.
.eslintrc.json
package.json // Add eslint and plugins as dev dependencies
Suggestion importance[1-10]: 9

__

Why: This is an excellent high-level suggestion that critiques the fundamental approach of the PR, proposing a more maintainable and less ambiguous tool-based alternative (like ESLint) instead of relying solely on extensive natural language documentation.

High
Possible issue
Correct file handle write parameters

Correct the parameters for fh.write to write the buffer at a specific file
position instead of an invalid buffer offset, preventing a runtime error.

.github/instructions/file-system-api.instructions.md [89]

-const { bytesWritten } = await fh.write(Buffer.from('ABC'), 10);
+const { bytesWritten } = await fh.write(Buffer.from('ABC'), 0, 3, 10);
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the fh.write method is used with incorrect parameters, which would cause a runtime error, and provides the correct usage to achieve the intended behavior.

Medium
Fix hanging promise in worker thread

Refactor the worker exit event handler to only reject the promise on a non-zero
exit code, preventing the promise from hanging on a successful exit.

.github/instructions/modern-nodejs-conventions.instructions.md [144-146]

-worker.once('exit', code =>
-  code === 0 || reject(new Error(`Worker exit: ${code}`))
-);
+worker.once('exit', (code) => {
+  if (code !== 0) {
+    reject(new Error(`Worker exit: ${code}`));
+  }
+});
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logic flaw in the worker exit handler where a successful exit (code === 0) could cause the promise to hang, and the proposed fix correctly addresses this issue.

Medium
General
Ensure test fails on unexpected success

Add assert.fail() to the try block of a test to ensure it fails if the tested
command succeeds unexpectedly instead of throwing an error.

.github/instructions/testing.instructions.md [472-478]

 test('should output an error message for invalid input', async () => {
   try {
     await execPromise(`node ${cliPath} --invalid-command`);
+    assert.fail('Expected command to fail but it succeeded');
   } catch (error) {
     assert.match(error.stderr, /Unknown command: --invalid-command/);
   }
 });
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion improves the robustness of a test case by ensuring it fails if the command unexpectedly succeeds, which is a good testing practice.

Low
  • More

@qodo-free-for-open-source-projects
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Restructure large instruction files for AI

The testing.instructions.md file is nearly 700 lines long, which may be too
large for an AI's context window. It should be broken into smaller, more focused
files to improve the AI's effectiveness.

Examples:

.github/instructions/testing.instructions.md [1-688]
---
applyTo: "**/*.test.js,**/*.test.ts,**/*.spec.js,**/*.spec.ts,__tests__/**/*.js,__tests__/**/*.ts,test/**/*.js,test/**/*.ts"
description: Testing guidelines for Node.js applications
source: https://github.com/lirantal/agent-rules
---

# Testing Guidelines for Node.js applications 

This document outlines the rules and guidelines for AI agents generating test code for modern Node.js applications.


 ... (clipped 678 lines)

Solution Walkthrough:

Before:

// File structure before
.github/
└── instructions/
    ├── error-handling.instructions.md
    ├── file-system-api.instructions.md
    ├── ...
    └── testing.instructions.md (single, 688-line file)

After:

// File structure after
.github/
└── instructions/
    ├── error-handling.instructions.md
    ├── file-system-api.instructions.md
    ├── ...
    ├── testing-basics.md
    ├── testing-mocking.md
    ├── testing-web-servers.md
    └── testing-cli.md
    // (Original testing.instructions.md is split into smaller, focused files)
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a critical design flaw where the very large testing.instructions.md file could be ineffective for an AI agent, proposing a modular split that directly improves the core goal of the PR.

Medium
Possible issue
Fix unhandled worker exit promise

Fix a potential promise hang in the worker thread example by handling the case
where a worker exits with code 0 before sending a message.

.github/instructions/modern-nodejs-conventions.instructions.md [144-146]

-worker.once('exit', code =>
-  code === 0 || reject(new Error(`Worker exit: ${code}`))
-);
+worker.once('exit', code => {
+  if (code === 0) {
+    resolve(undefined); // Or some other default value if appropriate
+  } else {
+    reject(new Error(`Worker exit: ${code}`));
+  }
+});
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential unhandled promise scenario where a worker exiting with code 0 without sending a message would cause the promise to hang, and the fix correctly resolves this issue.

Medium
Improve error serialization to prevent crashes

Improve the toJSON method in the AppError example to handle non-Error causes
more robustly by serializing them to prevent potential crashes or unhelpful
output.

.github/instructions/error-handling.instructions.md [46-58]

 toJSON() {
+  const getSerializedCause = () => {
+    if (this.cause instanceof Error) {
+      // Serialize error with its own toJSON if available, or key properties
+      if (typeof (this.cause as any).toJSON === 'function') {
+        return (this.cause as any).toJSON();
+      }
+      return {
+        message: this.cause.message,
+        stack: this.cause.stack,
+        name: this.cause.name,
+      };
+    }
+    if (this.cause !== undefined && this.cause !== null) {
+      return String(this.cause);
+    }
+    return this.cause;
+  };
+
   return {
     name: this.name,
     message: this.message,
     code: this.code,
     statusCode: this.statusCode,
     context: this.context,
     timestamp: this.timestamp,
     stack: this.stack,
-    // Optional: include cause message safely if it is an Error
-    cause: this.cause instanceof Error ? this.cause.message : this.cause,
+    cause: getSerializedCause(),
   };
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential serialization issue with non-Error cause objects and provides a more robust implementation for the toJSON method in the example AppError class.

Low
General
Assert on stdout for success cases

Correct the CLI testing example to assert on stdout for success messages instead
of stderr, aligning with standard command-line application behavior.

.github/instructions/testing.instructions.md [492-500]

 test('should exit with status 0 for valid input', () => {
   // Assuming 'my-cli' is an executable command.
   // The arguments are passed as an array.
-  const { status, stderr } = spawnSync('my-cli', ['--frail', 'input.txt']);
+  const { status, stdout, stderr } = spawnSync('my-cli', ['--frail', 'input.txt']);
 
-  assert.equal(status, 0);
-  assert.equal(stripVTControlCharacters(String(stderr)), 'input.txt: no issues found
+  assert.equal(status, 0, `Process exited with non-zero status. Stderr: ${stderr}`);
+  assert.equal(stripVTControlCharacters(String(stdout)), 'input.txt: no issues found
 ');
 });
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that success output should be on stdout, not stderr, and improves the testing example to follow this standard convention, making it a better guide.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant