Conversation
Co-authored-by: dshvvvshr <216969053+dshvvvshr@users.noreply.github.com>
Co-authored-by: dshvvvshr <216969053+dshvvvshr@users.noreply.github.com>
…dules Co-authored-by: dshvvvshr <216969053+dshvvvshr@users.noreply.github.com>
Co-authored-by: dshvvvshr <216969053+dshvvvshr@users.noreply.github.com>
|
npx -y @smithery/cli install brave @copilot |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Check dependencies | ||
| const missing = this.checkDependencies(entry.module); | ||
| if (missing.length > 0) { | ||
| throw new Error(`Module ${name} has missing dependencies: ${missing.join(', ')}`); |
There was a problem hiding this comment.
In ModuleRegistry.initialize, dependencies are only checked for registration, not for having reached INITIALIZED. If a caller initializes a dependent module directly (or a dependency cycle exists), the module’s init can run before its dependencies are ready, which can lead to runtime failures or inconsistent state. Consider either initializing dependencies first or throwing when any dependency is not initialized.
Useful? React with 👍 / 👎.
| const fs = await import('fs/promises'); | ||
| const path = await import('path'); | ||
| const directivePath = path.join(process.cwd(), 'CORE_DIRECTIVE.md'); | ||
| await fs.access(directivePath); |
There was a problem hiding this comment.
The core-directive-exists check resolves CORE_DIRECTIVE.md via process.cwd(). When PrimeSecurity is used as a dependency or launched from a different working directory, the file can exist in the package but not in the current cwd, causing compliance to fail and initialization to abort. Use a path relative to the module/package (e.g., __dirname) or allow a configurable location.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR initializes the Prime Security (Under Pressure Looming) repository with a foundational self-organizing, multi-agent security framework based on autonomic computing principles. The implementation provides core infrastructure for building resilient, modular security systems governed by an immutable Core Directive.
Key Changes:
- Implemented cryptographic primitives (AES-256-GCM, SHA-256/512, PBKDF2) with input validation for SQL injection and XSS detection
- Created module registry system with dependency resolution and lifecycle management (init/start/stop/destroy phases)
- Developed Digital DNA system enabling autonomous system reconstruction from serializable blueprints
- Established governance framework with audit logging and Core Directive compliance validation
- Added comprehensive documentation (Core Directive, Architecture, Contributing Guide, Research Foundations)
- Configured CI/CD pipeline with lint, test, build, and compliance checks
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/security/crypto.ts | Implements cryptographic operations and input validation primitives |
| src/registry/index.ts | Provides plugin system with dependency resolution and module lifecycle management |
| src/governance/compliance.ts | Implements audit logging and Core Directive compliance checking |
| src/autonomic/dna.ts | Defines system blueprint (Digital DNA) for self-reconstruction capabilities |
| src/index.ts | Main entry point orchestrating system initialization and module coordination |
| tests/security/crypto.test.ts | Unit tests for cryptographic functions and validators (11 tests) |
| tests/registry/registry.test.ts | Unit tests for module registry and lifecycle (10 tests) |
| examples/basic-usage.ts | Demonstrates basic usage patterns for the framework |
| package.json | Defines project metadata, dependencies, and build scripts |
| tsconfig.json | TypeScript compiler configuration with strict mode enabled |
| jest.config.js | Test framework configuration with 70% coverage threshold |
| .eslintrc.js | Linting rules enforcing TypeScript best practices |
| .github/workflows/ci.yml | CI/CD pipeline with lint, test, build, and compliance jobs |
| README.md | Comprehensive project documentation with quickstart and API examples |
| CORE_DIRECTIVE.md | Immutable foundational principles governing system behavior |
| CONTRIBUTING.md | Contribution guidelines and development workflow |
| UNDER_PRESSURE_LOOMING.md | Research foundations and theoretical background |
| notes/ARCHITECTURE_DRAFT.md | Detailed system architecture and component specifications |
| LICENSE | MIT License for the project |
| .prettierrc | Code formatting configuration |
| .gitignore | Git ignore patterns for dependencies and build artifacts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/registry/index.ts
Outdated
| throw new Error(`Module ${name} not found`); | ||
| } | ||
|
|
||
| if (entry.state !== ModuleState.RUNNING) { |
There was a problem hiding this comment.
The stop function returns early if the module state is not RUNNING, but this doesn't allow stopping modules that are in transitional states like STARTING or INITIALIZING. If a module fails to start or gets stuck in STARTING state, there's no way to cleanly stop it. Consider allowing stop operations for modules in STARTING, INITIALIZING, or ERROR states to enable recovery from failures.
| if (entry.state !== ModuleState.RUNNING) { | |
| // Only ignore stop requests for modules that are clearly inactive or already stopping. | |
| if ( | |
| entry.state === ModuleState.UNINITIALIZED || | |
| entry.state === ModuleState.STOPPED || | |
| entry.state === ModuleState.STOPPING | |
| ) { |
src/autonomic/dna.ts
Outdated
| if (!blueprint.version) { | ||
| throw new Error('Blueprint must have a version'); | ||
| } | ||
|
|
||
| if (!blueprint.coreDirective) { | ||
| throw new Error('Blueprint must reference Core Directive'); | ||
| } | ||
|
|
||
| if (!Array.isArray(blueprint.modules)) { | ||
| throw new Error('Blueprint must have modules array'); | ||
| } | ||
|
|
||
| if (!blueprint.configuration) { | ||
| throw new Error('Blueprint must have configuration'); | ||
| } |
There was a problem hiding this comment.
The validateBlueprint method only performs shallow validation of the blueprint structure. It doesn't verify the actual content or format of the coreDirective object (version, checksum, url), the modules array structure, or the configuration object. This could allow invalid blueprints to be loaded. Consider adding deeper validation to check that required fields exist with proper types and formats.
| if (!blueprint.version) { | |
| throw new Error('Blueprint must have a version'); | |
| } | |
| if (!blueprint.coreDirective) { | |
| throw new Error('Blueprint must reference Core Directive'); | |
| } | |
| if (!Array.isArray(blueprint.modules)) { | |
| throw new Error('Blueprint must have modules array'); | |
| } | |
| if (!blueprint.configuration) { | |
| throw new Error('Blueprint must have configuration'); | |
| } | |
| // Top-level version | |
| if (typeof blueprint.version !== 'string' || blueprint.version.trim() === '') { | |
| throw new Error('Blueprint must have a non-empty string version'); | |
| } | |
| // Core directive | |
| const coreDirective = blueprint.coreDirective as unknown; | |
| if ( | |
| !coreDirective || | |
| typeof coreDirective !== 'object' | |
| ) { | |
| throw new Error('Blueprint must reference a valid Core Directive object'); | |
| } | |
| const cd = coreDirective as CoreDirectiveReference; | |
| if (typeof cd.version !== 'string' || cd.version.trim() === '') { | |
| throw new Error('Core Directive must have a non-empty string version'); | |
| } | |
| if (typeof cd.checksum !== 'string' || cd.checksum.trim() === '') { | |
| throw new Error('Core Directive must have a non-empty string checksum'); | |
| } | |
| if (typeof cd.url !== 'string' || cd.url.trim() === '') { | |
| throw new Error('Core Directive must have a non-empty string url'); | |
| } | |
| // Best-effort URL format check (ignore environments without URL constructor) | |
| try { | |
| // eslint-disable-next-line no-new | |
| new URL(cd.url); | |
| } catch { | |
| // Do not fail hard on URL parsing; only ensure it looks like a URL scheme | |
| if (!/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(cd.url)) { | |
| throw new Error('Core Directive url must be a valid URL or URL-like string'); | |
| } | |
| } | |
| // Modules | |
| if (!Array.isArray(blueprint.modules)) { | |
| throw new Error('Blueprint must have modules array'); | |
| } | |
| blueprint.modules.forEach((mod, index) => { | |
| if (!mod || typeof mod !== 'object') { | |
| throw new Error(`Module at index ${index} must be an object`); | |
| } | |
| if (typeof mod.name !== 'string' || mod.name.trim() === '') { | |
| throw new Error(`Module at index ${index} must have a non-empty string name`); | |
| } | |
| if (typeof mod.version !== 'string' || mod.version.trim() === '') { | |
| throw new Error(`Module "${mod.name}" must have a non-empty string version`); | |
| } | |
| if (mod.type !== 'core' && mod.type !== 'extension' && mod.type !== 'plugin') { | |
| throw new Error( | |
| `Module "${mod.name}" has invalid type "${(mod as ModuleDefinition).type}", expected "core" | "extension" | "plugin"`, | |
| ); | |
| } | |
| if (mod.repository !== undefined && typeof mod.repository !== 'string') { | |
| throw new Error(`Module "${mod.name}" repository must be a string if provided`); | |
| } | |
| if (mod.configuration !== undefined) { | |
| const cfg = mod.configuration; | |
| if (cfg === null || typeof cfg !== 'object' || Array.isArray(cfg)) { | |
| throw new Error(`Module "${mod.name}" configuration must be a non-null object if provided`); | |
| } | |
| } | |
| if (mod.dependencies !== undefined) { | |
| if (!Array.isArray(mod.dependencies)) { | |
| throw new Error(`Module "${mod.name}" dependencies must be an array of strings if provided`); | |
| } | |
| mod.dependencies.forEach((dep, depIndex) => { | |
| if (typeof dep !== 'string' || dep.trim() === '') { | |
| throw new Error( | |
| `Module "${mod.name}" dependency at index ${depIndex} must be a non-empty string`, | |
| ); | |
| } | |
| }); | |
| } | |
| }); | |
| // Configuration | |
| const configuration = blueprint.configuration as unknown; | |
| if (!configuration || typeof configuration !== 'object') { | |
| throw new Error('Blueprint must have configuration object'); | |
| } | |
| const config = configuration as SystemConfiguration; | |
| if ( | |
| config.environment !== 'development' && | |
| config.environment !== 'staging' && | |
| config.environment !== 'production' | |
| ) { | |
| throw new Error( | |
| `Configuration environment must be one of "development", "staging", or "production"`, | |
| ); | |
| } | |
| if (config.features === null || typeof config.features !== 'object' || Array.isArray(config.features)) { | |
| throw new Error('Configuration features must be an object mapping feature names to booleans'); | |
| } | |
| Object.keys(config.features).forEach((key) => { | |
| if (typeof config.features[key] !== 'boolean') { | |
| throw new Error(`Configuration feature "${key}" must be a boolean`); | |
| } | |
| }); | |
| const limits = config.limits as unknown; | |
| if (!limits || typeof limits !== 'object') { | |
| throw new Error('Configuration limits must be an object'); | |
| } | |
| const resLimits = limits as ResourceLimits; | |
| const numericLimitFields: (keyof ResourceLimits)[] = [ | |
| 'maxMemoryMB', | |
| 'maxCPUPercent', | |
| 'maxConnections', | |
| 'requestRateLimit', | |
| ]; | |
| numericLimitFields.forEach((field) => { | |
| const value = resLimits[field]; | |
| if (value !== undefined && (typeof value !== 'number' || Number.isNaN(value))) { | |
| throw new Error(`Configuration limits field "${String(field)}" must be a valid number if provided`); | |
| } | |
| }); | |
| // Policies (optional) | |
| if (blueprint.policies !== undefined) { | |
| if (!Array.isArray(blueprint.policies)) { | |
| throw new Error('Policies must be an array if provided'); | |
| } | |
| blueprint.policies.forEach((policy, index) => { | |
| if (!policy || typeof policy !== 'object') { | |
| throw new Error(`Policy at index ${index} must be an object`); | |
| } | |
| if (typeof policy.name !== 'string' || policy.name.trim() === '') { | |
| throw new Error(`Policy at index ${index} must have a non-empty string name`); | |
| } | |
| if ( | |
| policy.type !== 'security' && | |
| policy.type !== 'performance' && | |
| policy.type !== 'governance' | |
| ) { | |
| throw new Error( | |
| `Policy "${policy.name}" has invalid type "${policy.type}", expected "security" | "performance" | "governance"`, | |
| ); | |
| } | |
| if (typeof policy.enabled !== 'boolean') { | |
| throw new Error(`Policy "${policy.name}" enabled flag must be a boolean`); | |
| } | |
| if (!Array.isArray(policy.rules)) { | |
| throw new Error(`Policy "${policy.name}" rules must be an array`); | |
| } | |
| policy.rules.forEach((rule, ruleIndex) => { | |
| if (!rule || typeof rule !== 'object') { | |
| throw new Error(`Rule at index ${ruleIndex} in policy "${policy.name}" must be an object`); | |
| } | |
| if (typeof rule.condition !== 'string' || rule.condition.trim() === '') { | |
| throw new Error( | |
| `Rule at index ${ruleIndex} in policy "${policy.name}" must have a non-empty string condition`, | |
| ); | |
| } | |
| if (typeof rule.action !== 'string' || rule.action.trim() === '') { | |
| throw new Error( | |
| `Rule at index ${ruleIndex} in policy "${policy.name}" must have a non-empty string action`, | |
| ); | |
| } | |
| if (rule.parameters !== undefined) { | |
| const params = rule.parameters; | |
| if (params === null || typeof params !== 'object' || Array.isArray(params)) { | |
| throw new Error( | |
| `Rule parameters in policy "${policy.name}" must be a non-null object if provided`, | |
| ); | |
| } | |
| } | |
| }); | |
| }); | |
| } |
src/security/crypto.ts
Outdated
| * Check for potential SQL injection patterns | ||
| */ | ||
| static hasSQLInjection(input: string): boolean { | ||
| const sqlPattern = /(\b(SELECT|INSERT|UPDATE|DELETE|DROP|CREATE|ALTER|EXEC|EXECUTE)\b)|(--)|(;)|(')|(\*)/i; |
There was a problem hiding this comment.
The SQL injection detection pattern will produce false positives. The pattern flags any string containing asterisks or single quotes, which are common in legitimate text. For example, "3 * 4 = 12" or "It's working" would be incorrectly flagged. Consider refining the pattern to detect actual SQL injection contexts, such as quotes followed by SQL keywords, or remove the overly broad character matching.
| const sqlPattern = /(\b(SELECT|INSERT|UPDATE|DELETE|DROP|CREATE|ALTER|EXEC|EXECUTE)\b)|(--)|(;)|(')|(\*)/i; | |
| const sqlPattern = /(\b(SELECT|INSERT|UPDATE|DELETE|DROP|CREATE|ALTER|EXEC|EXECUTE)\b)|(--)|(;)/i; |
| private resolveDependencyOrder(): string[] { | ||
| const visited = new Set<string>(); | ||
| const order: string[] = []; | ||
|
|
||
| const visit = (name: string): void => { | ||
| if (visited.has(name)) return; | ||
|
|
||
| const entry = this.modules.get(name); | ||
| if (!entry) return; | ||
|
|
||
| visited.add(name); | ||
|
|
||
| // Visit dependencies first | ||
| if (entry.module.dependencies) { | ||
| for (const dep of entry.module.dependencies) { | ||
| visit(dep); | ||
| } | ||
| } | ||
|
|
||
| order.push(name); | ||
| }; | ||
|
|
||
| for (const name of this.modules.keys()) { | ||
| visit(name); | ||
| } | ||
|
|
||
| return order; | ||
| } |
There was a problem hiding this comment.
The dependency resolution algorithm doesn't detect circular dependencies. If Module A depends on Module B and Module B depends on Module A, the visit function will not detect this cycle and the system could fail during initialization. Add circular dependency detection by maintaining a "visiting" set in addition to the "visited" set, and throw an error if a cycle is detected.
| - name: Verify no secrets in code | ||
| run: | | ||
| if grep -r "api[_-]key\|password\|secret" --include="*.ts" --include="*.js" src/ 2>/dev/null; then | ||
| echo "Warning: Potential secrets found in code" | ||
| else | ||
| echo "✓ No obvious secrets detected" | ||
| fi |
There was a problem hiding this comment.
The hardcoded secret detection grep pattern is too simplistic and will produce false positives. It will flag legitimate variable names like 'api_key_length' or comments mentioning 'password validation'. Consider using a dedicated secrets scanning tool like gitleaks or truffleHog, or at minimum, improve the pattern to match actual secret values (e.g., strings with high entropy, API key formats, etc.) rather than just variable names.
| static isValidEmail(email: string): boolean { | ||
| const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| return emailPattern.test(email); |
There was a problem hiding this comment.
The email validation regex is overly permissive and doesn't properly validate email addresses. It would accept invalid formats like "user@domain" (missing TLD), "user@@domain.com" (double @), or "user@.com" (missing domain). Consider using a more robust email validation pattern that checks for proper domain structure, or leverage a well-tested email validation library.
src/governance/compliance.ts
Outdated
| // Rotate if needed | ||
| if (this.events.length > this.maxEvents) { | ||
| this.events = this.events.slice(-this.maxEvents); |
There was a problem hiding this comment.
The audit event rotation slices the events array when it exceeds maxEvents, but this creates a new array and discards the old one immediately. This pattern is inefficient for high-volume logging scenarios as it triggers garbage collection frequently. Consider implementing a circular buffer or using a more efficient data structure for event storage, especially since the comment mentions this is intended for production use with up to 10,000 events.
| // Rotate if needed | |
| if (this.events.length > this.maxEvents) { | |
| this.events = this.events.slice(-this.maxEvents); | |
| // Rotate if needed (remove oldest events in-place to avoid reallocating the array) | |
| while (this.events.length > this.maxEvents) { | |
| this.events.shift(); |
src/governance/compliance.ts
Outdated
| // Import registry to check loaded modules | ||
| const { registry } = await import('../registry'); | ||
|
|
||
| const requiredModules = ['core-security', 'governance']; | ||
| const loadedModules = registry.list(); | ||
| const loadedNames = new Set(loadedModules.map(m => m.name)); | ||
|
|
||
| const missing = requiredModules.filter(name => !loadedNames.has(name)); | ||
|
|
||
| if (missing.length > 0) { | ||
| return { | ||
| passed: false, | ||
| message: 'Required security modules not loaded', | ||
| violations: missing, | ||
| }; | ||
| } | ||
|
|
||
| // Check that modules are in running or initialized state | ||
| const notReady = loadedModules.filter(m => | ||
| requiredModules.includes(m.name) && | ||
| m.state !== 'running' && | ||
| m.state !== 'initialized' | ||
| ); | ||
|
|
||
| if (notReady.length > 0) { | ||
| return { | ||
| passed: false, | ||
| message: 'Security modules exist but are not operational', | ||
| violations: notReady.map(m => `${m.name} is ${m.state}`), | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| passed: true, | ||
| message: `All ${requiredModules.length} required security modules loaded and operational`, | ||
| }; |
There was a problem hiding this comment.
The compliance check dynamically imports the registry module to avoid circular dependencies, but this creates a timing issue. The check runs during system initialization (in src/index.ts) before modules are registered, so the required modules won't exist yet. This will cause the compliance check to fail during normal system startup. Consider restructuring this check to run after module registration, or make it optional during initialization.
| // Import registry to check loaded modules | |
| const { registry } = await import('../registry'); | |
| const requiredModules = ['core-security', 'governance']; | |
| const loadedModules = registry.list(); | |
| const loadedNames = new Set(loadedModules.map(m => m.name)); | |
| const missing = requiredModules.filter(name => !loadedNames.has(name)); | |
| if (missing.length > 0) { | |
| return { | |
| passed: false, | |
| message: 'Required security modules not loaded', | |
| violations: missing, | |
| }; | |
| } | |
| // Check that modules are in running or initialized state | |
| const notReady = loadedModules.filter(m => | |
| requiredModules.includes(m.name) && | |
| m.state !== 'running' && | |
| m.state !== 'initialized' | |
| ); | |
| if (notReady.length > 0) { | |
| return { | |
| passed: false, | |
| message: 'Security modules exist but are not operational', | |
| violations: notReady.map(m => `${m.name} is ${m.state}`), | |
| }; | |
| } | |
| return { | |
| passed: true, | |
| message: `All ${requiredModules.length} required security modules loaded and operational`, | |
| }; | |
| try { | |
| // Import registry to check loaded modules | |
| const { registry } = await import('../registry'); | |
| // If registry is not available or does not expose a list function yet, | |
| // treat this as an initialization phase and skip strict checking. | |
| if (!registry || typeof registry.list !== 'function') { | |
| return { | |
| passed: true, | |
| message: 'Security module check skipped: registry not initialized', | |
| }; | |
| } | |
| const requiredModules = ['core-security', 'governance']; | |
| const loadedModules = registry.list(); | |
| // During early startup, the registry may exist but be empty; in that | |
| // case, consider the check not yet applicable instead of failing. | |
| if (!Array.isArray(loadedModules) || loadedModules.length === 0) { | |
| return { | |
| passed: true, | |
| message: 'Security module check deferred: no modules registered yet', | |
| }; | |
| } | |
| const loadedNames = new Set(loadedModules.map(m => m.name)); | |
| const missing = requiredModules.filter(name => !loadedNames.has(name)); | |
| if (missing.length > 0) { | |
| return { | |
| passed: false, | |
| message: 'Required security modules not loaded', | |
| violations: missing, | |
| }; | |
| } | |
| // Check that modules are in running or initialized state | |
| const notReady = loadedModules.filter(m => | |
| requiredModules.includes(m.name) && | |
| m.state !== 'running' && | |
| m.state !== 'initialized' | |
| ); | |
| if (notReady.length > 0) { | |
| return { | |
| passed: false, | |
| message: 'Security modules exist but are not operational', | |
| violations: notReady.map(m => `${m.name} is ${m.state}`), | |
| }; | |
| } | |
| return { | |
| passed: true, | |
| message: `All ${requiredModules.length} required security modules loaded and operational`, | |
| }; | |
| } catch (error) { | |
| // If the registry cannot be imported at all (e.g., during very early | |
| // initialization), avoid failing compliance and mark the check as skipped. | |
| return { | |
| passed: true, | |
| message: `Security module check skipped: registry unavailable (${(error as Error).message})`, | |
| }; | |
| } |
src/governance/compliance.ts
Outdated
| let results = [...this.events]; | ||
|
|
||
| if (filter.component) { | ||
| results = results.filter((e) => e.component === filter.component); | ||
| } | ||
|
|
||
| if (filter.level) { | ||
| results = results.filter((e) => e.level === filter.level); | ||
| } | ||
|
|
||
| if (filter.since) { | ||
| const sinceDate = filter.since; | ||
| results = results.filter((e) => e.timestamp >= sinceDate); | ||
| } | ||
|
|
||
| if (filter.limit) { | ||
| results = results.slice(-filter.limit); |
There was a problem hiding this comment.
The query method creates a shallow copy of the entire events array before filtering, which is inefficient for large audit logs. With maxEvents set to 10,000, this could create significant memory overhead, especially if multiple queries run concurrently. Consider applying filters progressively without creating intermediate copies, or implementing the query as a generator/iterator pattern.
| let results = [...this.events]; | |
| if (filter.component) { | |
| results = results.filter((e) => e.component === filter.component); | |
| } | |
| if (filter.level) { | |
| results = results.filter((e) => e.level === filter.level); | |
| } | |
| if (filter.since) { | |
| const sinceDate = filter.since; | |
| results = results.filter((e) => e.timestamp >= sinceDate); | |
| } | |
| if (filter.limit) { | |
| results = results.slice(-filter.limit); | |
| const { component, level, since, limit } = filter; | |
| const hasLimit = typeof limit === 'number' && limit > 0; | |
| // When a limit is specified, collect matching events starting from the most recent | |
| // to avoid building large intermediate arrays, then reverse to maintain | |
| // chronological order in the returned results. | |
| if (hasLimit) { | |
| const limitedResults: AuditEvent[] = []; | |
| for (let i = this.events.length - 1; i >= 0; i--) { | |
| const e = this.events[i]; | |
| if (component && e.component !== component) { | |
| continue; | |
| } | |
| if (level && e.level !== level) { | |
| continue; | |
| } | |
| if (since && e.timestamp < since) { | |
| continue; | |
| } | |
| limitedResults.push(e); | |
| if (limitedResults.length === limit) { | |
| break; | |
| } | |
| } | |
| return limitedResults.reverse(); | |
| } | |
| // No limit: build the result array in a single forward pass with all filters applied. | |
| const results: AuditEvent[] = []; | |
| for (const e of this.events) { | |
| if (component && e.component !== component) { | |
| continue; | |
| } | |
| if (level && e.level !== level) { | |
| continue; | |
| } | |
| if (since && e.timestamp < since) { | |
| continue; | |
| } | |
| results.push(e); |
.eslintrc.js
Outdated
| 'no-console': ['warn', { allow: ['warn', 'error'] }] | ||
| } |
There was a problem hiding this comment.
The no-console ESLint rule allows 'warn' and 'error' but the codebase uses console.log extensively in examples/basic-usage.ts and the persist method uses console.error. While this is appropriate for the example file and audit logging, consider documenting the rationale for console usage or using a proper logging framework that can be configured for different environments.
| 'no-console': ['warn', { allow: ['warn', 'error'] }] | |
| } | |
| // Disallow general console usage in application code. | |
| // We allow console.warn and console.error because: | |
| // - console.error is used for audit/error logging where a logging framework | |
| // may not be available (e.g. early bootstrapping, process-level failures). | |
| // - console.warn is allowed for non-fatal operational warnings. | |
| // For application-level logging, prefer a proper logging framework that can be | |
| // configured per environment (e.g. debug levels, transports, formatting). | |
| 'no-console': ['warn', { allow: ['warn', 'error'] }] | |
| }, | |
| // Examples and documentation snippets may use console.log freely for clarity. | |
| // We disable the no-console rule for those files to avoid noisy warnings. | |
| overrides: [ | |
| { | |
| files: ['examples/basic-usage.ts'], | |
| rules: { | |
| 'no-console': 'off' | |
| } | |
| } | |
| ] |
|
@copilot apply changes based on the comments in this thread |
… improve error handling Co-authored-by: dshvvvshr <216969053+dshvvvshr@users.noreply.github.com>
Applied all review feedback in commit eb0b9d9. Changes include: Security:
Performance:
Robustness:
Validation:
All tests passing (21/21), build successful, lint clean. |
dshvvvshr
left a comment
dshvvvshr
left a comment
There was a problem hiding this comment.
I didn’t understand it but I trust you
Applied Review Feedback
Security Improvements
*and'from pattern)Performance Optimizations
Robustness Enhancements
startedflag to track system state accuratelyValidation Improvements
Developer Experience
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.