Phase 14: Core UX — stores, navigation, routing, real API#352
Merged
JustAGhosT merged 8 commits intodevfrom Mar 11, 2026
Merged
Phase 14: Core UX — stores, navigation, routing, real API#352JustAGhosT merged 8 commits intodevfrom
JustAGhosT merged 8 commits intodevfrom
Conversation
Upgrade Azure.AI.OpenAI to v2.1.0, add RoadmapCrew & AgentTeamFramework
…eletons FE-005: 5 Zustand stores - useAuthStore: mirrors AuthContext for non-React consumers - useAgentStore: agent registry with real agenticApi integration - useDashboardStore: dashboard data (fetch-based, pending backend endpoints) - useNotificationStore: in-app notifications with unread tracking - usePreferencesStore: persisted user preferences (theme, accessibility) FE-022: Navigation components - Sidebar with collapsible sections, active route highlighting - TopBar with breadcrumbs, notification bell, connection indicator - MobileMenu responsive drawer (<768px) FE-021: Multi-page routing - (app) route group with shared layout (sidebar + topbar + ProtectedRoute) - 6 routes: /dashboard, /agents, /analytics, /compliance, /marketplace, /settings - Per-route loading.tsx and error.tsx boundaries - Dashboard page wired to useDashboardStore - Agents page wired to useAgentStore with table view - Settings page wired to usePreferencesStore with toggle controls FE-003: SignalR real-time client - useSignalR hook with auto-reconnect (exponential backoff) - subscribe/unsubscribe/invoke/joinGroup/leaveGroup methods - ConnectionIndicator component shows live status FE-007: Skeleton loading components - Skeleton, SkeletonCard, SkeletonTable, SkeletonMetric, SkeletonDashboard Dependencies: zustand@5.0.11, @microsoft/signalr@10.0.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Root `/` now redirects to `/dashboard` (server-side via next/navigation) - Deleted `services/api.ts` (DashboardAPI singleton with hardcoded mock data) - Deleted `hooks/useDashboardData.ts` (hook wrapper around mock) - Dashboard page uses `useDashboardStore` fetching from real backend - Updated AGENT_BACKLOG.md: Phase 14 marked complete, Phase 14b added (CognitiveMeshUI component library integration) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive platform update adding a new ASP.NET Core API host with dependency injection and real-time SignalR support, a complete React frontend application with authentication, multiple feature pages, state management via Zustand, and enhanced backend services with cancellation token propagation and improved error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser Client
participant Auth as AuthContext
participant API as Backend API
participant Storage as LocalStorage
Client->>Auth: Mount
Auth->>Storage: Load stored refreshToken
Storage-->>Auth: refreshToken (if exists)
alt Token is valid and not expired
Auth->>Storage: Retrieve accessToken
Storage-->>Auth: accessToken
Auth->>Client: Set isAuthenticated=true
else Token is expired or missing
Auth->>API: POST /api/v1/auth/refresh
API-->>Auth: New accessToken + refreshToken
Auth->>Storage: Save tokens
Auth->>Client: Set isAuthenticated=true
else Refresh fails
Auth->>Client: Set isAuthenticated=false, redirect to login
end
sequenceDiagram
participant Client as Frontend App
participant SignalR as SignalR Hub
participant Server as Backend Services
participant DB as Database
Client->>Client: Mount useSignalR hook
Client->>SignalR: Connect (with accessToken)
SignalR-->>Client: Connection established
Client->>Client: Update status=connected
Client->>SignalR: Subscribe to "AgentStatusChanged"
SignalR->>Server: Register listener
Server->>DB: Agent status updates
DB-->>Server: Change detected
Server->>SignalR: Broadcast "AgentStatusChanged"
SignalR-->>Client: Receive event
Client->>Client: Update store
sequenceDiagram
participant User as User/Browser
participant Controller as AgentController
participant Service as AgentRegistryService
participant Port as IAgentRegistryPort
participant DB as Database
User->>Controller: POST /agents (RegisterAgent)
Controller->>Service: RegisterAgentAsync(request, cancellationToken)
Service->>Port: RegisterAgentAsync(request, cancellationToken)
Port->>DB: Save agent definition
DB-->>Port: Agent created
Port-->>Service: Return Agent
Service-->>Controller: Return Agent
Controller-->>User: 201 Created with Agent
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SummarySummary
CoverageCognitiveMesh.Shared - 14.2%
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs
Fixed
Show fixed
Hide fixed
src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs
Fixed
Show fixed
Hide fixed
src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs
Fixed
Show fixed
Hide fixed
src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs
Fixed
Show fixed
Hide fixed
…tness Backend: - CORS empty-origins guard in Program.cs - Replace ThrowIfNullOrWhiteSpace with BadRequest in AdaptiveBalanceController - Add ProducesResponseType attributes, CancellationToken forwarding, error handling in AgentController and CustomerServiceController - Simplify AgentRegistryService circuit breaker delegation - Fix AuthorityService RevokeAuthorityOverrideAsync return and null-forgiving Frontend: - Prevent open redirect in login returnTo validation - Move ApiBootstrap inside ErrorBoundary in layout - Dev-only error messages in ErrorBoundary and error page - Guard e.message in ExtensionErrorSuppressor - Keyboard a11y on agent table rows, settings focus styles, label htmlFor - MobileMenu active state fix, Escape key, backdrop aria-hidden - navItems fallback group in groupBySections - Add pathname to ProtectedRoute useEffect deps - Toast aria-live on container - Fix agent store name mapping and dashboard store error handling - Auth context: logout in proactive-refresh deps, Secure cookie flag Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (19)
src/BusinessApplications/CustomerIntelligence/Models/InteractionRecord.cs (1)
1-2:⚠️ Potential issue | 🟡 MinorRemove these using directives as they are redundant.
The project has
<ImplicitUsings>enable</ImplicitUsings>in CustomerIntelligence.csproj, which automatically includesSystemandSystem.Collections.Genericnamespaces globally. Per coding guidelines, do not add using directives for namespaces already covered by ImplicitUsings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/CustomerIntelligence/Models/InteractionRecord.cs` around lines 1 - 2, Remove the redundant using directives "using System;" and "using System.Collections.Generic;" from InteractionRecord.cs (they are covered by the project's ImplicitUsings setting); simply delete those two lines so the file relies on implicit global usings instead of explicitly importing System and System.Collections.Generic.src/BusinessApplications/ValueGeneration/ValueGeneration.csproj (1)
3-9:⚠️ Potential issue | 🟡 MinorMissing
TreatWarningsAsErrorsconfiguration.The project configuration does not include
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>which is required per coding guidelines. This ensures that warnings (including CS1591 for missing XML documentation on public types) are treated as errors and block the build.Proposed fix
<PropertyGroup> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> <AssemblyName>CognitiveMesh.BusinessApplications.ValueGeneration</AssemblyName> </PropertyGroup>As per coding guidelines: "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/ValueGeneration/ValueGeneration.csproj` around lines 3 - 9, The project file is missing TreatWarningsAsErrors which enforces warnings-as-errors (per guidelines); update the existing PropertyGroup in ValueGeneration.csproj (the section containing TargetFramework, ImplicitUsings, Nullable, GenerateDocumentationFile, AssemblyName) to add a <TreatWarningsAsErrors>true</TreatWarningsAsErrors> entry so the build treats warnings (including CS1591) as errors; ensure the new element is placed alongside the other project-level properties within that PropertyGroup.src/BusinessApplications/ValueGeneration/Controllers/ValueGenerationController.cs (1)
75-75:⚠️ Potential issue | 🟠 MajorAdd
CancellationTokenparameter to all public async methods.This method and all other public async action methods (
DetectOrganizationalBlindnessAsync,CheckEmployabilityAsync,GetEmployabilityReviewStatusAsync) are missing the requiredCancellationTokenparameter. Without it, request cancellations cannot propagate to downstream port calls, potentially wasting resources on abandoned requests.As per coding guidelines: "All public methods must be async with
CancellationTokenparameter".Proposed fix for this method (apply similar pattern to others)
-public async Task<ActionResult<ValueDiagnosticResponse>> RunValueDiagnosticAsync([FromBody] ValueDiagnosticApiRequest request) +public async Task<ActionResult<ValueDiagnosticResponse>> RunValueDiagnosticAsync( + [FromBody] ValueDiagnosticApiRequest request, + CancellationToken cancellationToken = default)Then propagate to downstream calls:
-var consentResponse = await _consentPort.ValidateConsentAsync(consentRequest); +var consentResponse = await _consentPort.ValidateConsentAsync(consentRequest, cancellationToken); -var result = await _valueDiagnosticPort.RunValueDiagnosticAsync(domainRequest); +var result = await _valueDiagnosticPort.RunValueDiagnosticAsync(domainRequest, cancellationToken); -await _auditLogger.LogEventAsync(new AuditEvent { ... }); +await _auditLogger.LogEventAsync(new AuditEvent { ... }, cancellationToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/ValueGeneration/Controllers/ValueGenerationController.cs` at line 75, Update the public async controller action signatures to accept a CancellationToken parameter (e.g., change RunValueDiagnosticAsync, DetectOrganizationalBlindnessAsync, CheckEmployabilityAsync, GetEmployabilityReviewStatusAsync to include CancellationToken cancellationToken) and pass that token through to all downstream async calls they make (e.g., service/port methods invoked inside these actions such as the methods called from ValueGenerationController) so request cancellations propagate; update any calls to these controller methods in tests or routing if needed and ensure the token is forwarded to repository/service methods that support CancellationToken.src/BusinessApplications/DecisionSupport/DecisionSupport.csproj (1)
3-8: 🛠️ Refactor suggestion | 🟠 MajorEnable XML docs and warnings-as-errors in this project.
This
csprojstill doesn't turn onGenerateDocumentationFileorTreatWarningsAsErrors, so CS1591 and other warnings won't be enforced consistently.Suggested change
<PropertyGroup> <OutputType>Library</OutputType> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> + <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/DecisionSupport/DecisionSupport.csproj` around lines 3 - 8, Update the DecisionSupport.csproj PropertyGroup to enable XML docs and treat warnings as errors: set GenerateDocumentationFile to true and TreatWarningsAsErrors to true (so CS1591 and other warnings fail the build). Locate the PropertyGroup that contains OutputType/TargetFramework/ImplicitUsings/Nullable and add these two properties (GenerateDocumentationFile and TreatWarningsAsErrors) so the project enforces XML documentation and fails on warnings.src/BusinessApplications/AdaptiveBalance/AdaptiveBalance.csproj (1)
3-8: 🛠️ Refactor suggestion | 🟠 MajorBring this project file in line with the repo build rules.
This library still omits both
GenerateDocumentationFileandTreatWarningsAsErrors, so doc-comment and warning enforcement can drift.Suggested change
<PropertyGroup> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AdaptiveBalance/AdaptiveBalance.csproj` around lines 3 - 8, The project file's PropertyGroup is missing the repository-required build flags: enable XML docs and treat warnings as errors; update the AdaptiveBalance.csproj PropertyGroup to include <GenerateDocumentationFile>true</GenerateDocumentationFile> (if not already) and add <TreatWarningsAsErrors>true</TreatWarningsAsErrors> so CS1591 is enforced and warnings fail the build, ensuring these properties are present alongside <TargetFramework>, <ImplicitUsings>, and <Nullable>.src/BusinessApplications/NISTCompliance/NISTCompliance.csproj (1)
3-8: 🛠️ Refactor suggestion | 🟠 MajorAdd
TreatWarningsAsErrorsto keep the build strict.The project already emits XML docs, but warnings still won't fail the build. That undercuts the repo-level CS1591 expectation.
Suggested change
<PropertyGroup> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/NISTCompliance/NISTCompliance.csproj` around lines 3 - 8, Add a build-time failure on warnings by adding TreatWarningsAsErrors set to true inside the existing PropertyGroup (next to TargetFramework/ImplicitUsings/Nullable/GenerateDocumentationFile); specifically add a <TreatWarningsAsErrors>true</TreatWarningsAsErrors> entry and optionally augment with a WarningsAsErrors entry (e.g., include CS1591) so the project treats missing XML docs as build errors.src/BusinessApplications/CustomerIntelligence/CustomerIntelligence.csproj (1)
3-8: 🛠️ Refactor suggestion | 🟠 MajorAdd the missing build-enforcement properties while touching this project.
Without
GenerateDocumentationFileandTreatWarningsAsErrors, this library won't reliably enforce CS1591 or fail on new warnings.Suggested change
<PropertyGroup> <OutputType>Library</OutputType> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> + <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/CustomerIntelligence/CustomerIntelligence.csproj` around lines 3 - 8, The project lacks build-enforcement properties causing CS1591 and new warnings to be missed; add the MSBuild properties GenerateDocumentationFile and TreatWarningsAsErrors inside the existing PropertyGroup so the compiler emits XML docs and treats warnings as errors. Specifically, update the CustomerIntelligence.csproj PropertyGroup (where TargetFramework/ImplicitUsings/Nullable are declared) to include GenerateDocumentationFile set to true and TreatWarningsAsErrors set to true so public API documentation is generated and any warnings fail the build.src/BusinessApplications/ImpactMetrics/ImpactMetrics.csproj (1)
3-8: 🛠️ Refactor suggestion | 🟠 MajorEnforce warnings as errors here too.
GenerateDocumentationFileis already enabled, but the project still allows warnings to pass. That weakens the CS1591 requirement and other build-time guardrails.Suggested change
<PropertyGroup> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/ImpactMetrics/ImpactMetrics.csproj` around lines 3 - 8, The project enables XML docs via GenerateDocumentationFile but doesn't treat warnings as errors; update the csproj PropertyGroup to set TreatWarningsAsErrors to true (and optionally add Nullable, ImplicitUsings remain as-is) so build-time warnings (including CS1591) fail the build; locate the Project's PropertyGroup containing TargetFramework/GenerateDocumentationFile and add the TreatWarningsAsErrors element set to true (and consider adding WarningLevel/WarningsAsErrors entries if fine-grained control is needed).src/BusinessApplications/ResearchAnalysis/ResearchAnalysis.csproj (1)
3-12: 🛠️ Refactor suggestion | 🟠 MajorEnable XML-doc and warning enforcement in this project.
This project still won't surface CS1591 or fail on warnings because
GenerateDocumentationFileandTreatWarningsAsErrorsare not enabled.As per coding guidelines, "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true) and XML doc comments are required on public types (CS1591)".Suggested patch
<PropertyGroup> <OutputType>Library</OutputType> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> + <GenerateDocumentationFile>true</GenerateDocumentationFile> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/ResearchAnalysis/ResearchAnalysis.csproj` around lines 3 - 12, Enable XML documentation generation and treat warnings as errors by adding the MSBuild properties GenerateDocumentationFile and TreatWarningsAsErrors to the project file: set <GenerateDocumentationFile>true</GenerateDocumentationFile> to produce XML docs (so CS1591 is reported) and set <TreatWarningsAsErrors>true</TreatWarningsAsErrors> so builds fail on warnings; add these properties inside the existing PropertyGroup that contains TargetFramework/Nullable/ImplicitUsings to ensure they apply to the project.src/BusinessApplications/AgencyRouter/Controllers/AgencyRouterController.cs (1)
50-61:⚠️ Potential issue | 🟠 MajorPropagate
CancellationTokenthrough these controller actions.These endpoints start async work but never accept the request token, so abandoned HTTP requests can keep running in the port layer. All three methods—
RouteTask,ApplyOverride, andGetIntrospectionData—along with their corresponding port interface methods (RouteTaskAsync,ApplyOverrideAsync,GetIntrospectionDataAsync) must accept and propagateCancellationTokenper coding guidelines.Example fix for RouteTask
- public async Task<IActionResult> RouteTask([FromBody] TaskContext context) + public async Task<IActionResult> RouteTask([FromBody] TaskContext context, CancellationToken cancellationToken) { var correlationId = Guid.NewGuid().ToString(); try { var (tenantId, actorId) = GetAuthContextFromClaims(); if (tenantId == null) return Unauthorized(new { error_code = "UNAUTHORIZED", message = "Tenant ID is missing or invalid.", correlationID = correlationId }); context.Provenance = new ProvenanceContext { TenantId = tenantId!, ActorId = actorId ?? string.Empty, CorrelationId = correlationId }; _logger.LogInformation("Initiating agency routing for Task '{TaskId}' with CorrelationId '{CorrelationId}'.", context.TaskId, correlationId); - var response = await _agencyRouterPort.RouteTaskAsync(context); + var response = await _agencyRouterPort.RouteTaskAsync(context, cancellationToken); return Ok(response); }Also applies to
ApplyOverride(lines 92–103) andGetIntrospectionData(lines 143–149).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgencyRouter/Controllers/AgencyRouterController.cs` around lines 50 - 61, The controller methods RouteTask, ApplyOverride, and GetIntrospectionData do not accept or propagate a CancellationToken, so async work can continue after the request is aborted; modify each controller action signature to accept a CancellationToken (e.g., CancellationToken cancellationToken = default) and pass HttpContext.RequestAborted (or the incoming token) into the corresponding port calls RouteTaskAsync, ApplyOverrideAsync, and GetIntrospectionDataAsync; then update the IAgencyRouterPort (and all implementations) to include CancellationToken parameters on RouteTaskAsync, ApplyOverrideAsync, and GetIntrospectionDataAsync and ensure the cancellationToken is threaded through to any downstream async operations.src/BusinessApplications/AgentRegistry/Services/AgentConsentService.cs (2)
32-41:⚠️ Potential issue | 🟠 MajorPublic async methods are missing
CancellationTokenparameters.Per coding guidelines, all public async methods must include a
CancellationTokenparameter. Methods likeRecordConsentAsync,ValidateConsentAsync,RevokeConsentAsync, etc. should accept aCancellationTokenand pass it through to database operations.Example fix for one method signature
-public async Task<ConsentRecord> RecordConsentAsync(ConsentRequest request) +public async Task<ConsentRecord> RecordConsentAsync(ConsentRequest request, CancellationToken cancellationToken = default)Then pass
cancellationTokentoSaveChangesAsync(cancellationToken)and other async operations.As per coding guidelines: "All public methods must be async with
CancellationTokenparameter"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AgentConsentService.cs` around lines 32 - 41, Update all public async methods on AgentConsentService (e.g., RecordConsentAsync, ValidateConsentAsync, RevokeConsentAsync) to accept a CancellationToken parameter (cancellationToken) in their signatures and defaulting only if necessary at call sites; propagate that token into all async database calls such as _dbContext.SaveChangesAsync(cancellationToken) and any EF Core queries (e.g., ToListAsync/FirstOrDefaultAsync) and into the AgentCircuitBreakerPolicy execution paths so the token is honored during retries. Ensure method overrides/implementations and any callers are updated accordingly to pass the CancellationToken through, and keep the constructor and _circuitBreaker initialization unchanged.
890-914:⚠️ Potential issue | 🟡 MinorJSON deserialization with null-forgiving operator may cause runtime exceptions.
The
!operator onJsonSerializer.Deserialize<T>(...)!suppresses null warnings but doesn't prevent null at runtime. If the stored JSON isnull, empty, or malformed, this will throw aNullReferenceException.Consider defensive deserialization
entity.Property(e => e.OperationContext) .HasConversion( v => System.Text.Json.JsonSerializer.Serialize(v, new System.Text.Json.JsonSerializerOptions()), - v => System.Text.Json.JsonSerializer.Deserialize<Dictionary<string, object>>(v, new System.Text.Json.JsonSerializerOptions())!); + v => System.Text.Json.JsonSerializer.Deserialize<Dictionary<string, object>>(v, new System.Text.Json.JsonSerializerOptions()) ?? new Dictionary<string, object>());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AgentConsentService.cs` around lines 890 - 914, The JSON conversions for OperationContext, AgentPreferences, and ConsentTypePreferences use JsonSerializer.Deserialize<T>(...)! which can throw if the DB value is null/empty/malformed; update the HasConversion lambdas in modelBuilder.Entity<AgentConsentPreferences> (and the earlier OperationContext property) to defensively handle null/empty input and deserialization failure by returning a safe default (e.g., new Dictionary<string, object>() or new Dictionary<Guid, AgentSpecificPreferences>() or new Dictionary<string,bool>() as appropriate) instead of using the null-forgiving operator; implement this by checking string.IsNullOrWhiteSpace(v) and/or using JsonSerializer.Deserialize<T>(v, options) ?? defaultValue and optionally catching JsonException to return the default so runtime nulls/malformed JSON do not throw.src/BusinessApplications/AgentRegistry/Services/AuthorityService.cs (2)
590-595:⚠️ Potential issue | 🟡 MinorUnreachable code after
GetAuthorityPolicyTemplateAsync.Line 592-594 checks if
policy == nulland throwsPolicyNotFoundException, butGetAuthorityPolicyTemplateAsyncalready throwsPolicyNotFoundExceptionwhen the policy is not found (line 524). This null check is dead code.Remove dead code
return await _circuitBreaker.ExecuteAsync(async () => { // Get the policy template var policy = await GetAuthorityPolicyTemplateAsync(policyId, tenantId); - if (policy == null) - { - throw new PolicyNotFoundException($"Authority policy template {policyId} not found"); - } // Start with the base scope var resultScope = policy.BaseScope ?? new AuthorityScope();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AuthorityService.cs` around lines 590 - 595, The null-check after calling GetAuthorityPolicyTemplateAsync is dead because GetAuthorityPolicyTemplateAsync already throws PolicyNotFoundException when not found; remove the redundant if (policy == null) { throw new PolicyNotFoundException(...) } block following the await call (the policy local returned by GetAuthorityPolicyTemplateAsync should be used directly), leaving only var policy = await GetAuthorityPolicyTemplateAsync(policyId, tenantId); and subsequent logic that uses policy.
880-941:⚠️ Potential issue | 🟡 MinorJSON deserialization with null-forgiving operator may cause runtime exceptions.
Similar to AgentConsentService, using
!onJsonSerializer.Deserialize<T>(...)!can causeNullReferenceExceptionif stored JSON is null or malformed. Consider defensive deserialization with null-coalescing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AuthorityService.cs` around lines 880 - 941, The conversions for JSON columns (AuthorityScope, OverrideScope, AuthorityPolicyTemplate.BaseScope, and AuthorityPolicyTemplate.Rules) use JsonSerializer.Deserialize<T>(...)! which can throw if the stored JSON is null or invalid; update those HasConversion delegates to defensively handle null/empty/malformed input (remove the null-forgiving operator), e.g. check for null or empty string before deserializing and return a safe default (null, new AuthorityScope(), or new List<AuthorityPolicyRule>() as appropriate) and catch/desanitize deserialization exceptions so the model methods (the conversion lambdas for AuthorityScope, OverrideScope, BaseScope, and Rules) never return a null-unexpected value or throw at runtime.src/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cs (1)
371-486:⚠️ Potential issue | 🟠 MajorInterface methods are missing
CancellationTokenparameters.Per coding guidelines, all public async methods must include a
CancellationTokenparameter. TheIAgentRegistryPortinterface methods should acceptCancellationToken cancellationToken = defaultto allow callers to cancel long-running operations.Example fix for interface signatures
-Task<Agent> RegisterAgentAsync(AgentRegistrationRequest request); +Task<Agent> RegisterAgentAsync(AgentRegistrationRequest request, CancellationToken cancellationToken = default); -Task<Agent> GetAgentByIdAsync(Guid agentId, string tenantId); +Task<Agent> GetAgentByIdAsync(Guid agentId, string tenantId, CancellationToken cancellationToken = default);Apply this pattern to all interface methods.
As per coding guidelines: "All public methods must be async with
CancellationTokenparameter"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cs` around lines 371 - 486, The interface IAgentRegistryPort is missing CancellationToken parameters on all async methods; update every method signature (RegisterAgentAsync, GetAgentByIdAsync, UpdateAgentAsync, DeactivateAgentAsync, GetAgentsByTypeAsync, GetAgentsByComplianceStatusAsync, SearchAgentsAsync, VerifyAgentComplianceAsync, GetAgentComplianceHistoryAsync, RequestComplianceCertificationAsync) to accept CancellationToken cancellationToken = default, preserving existing parameters and return types; after changing the interface, update all implementing classes and any callers/tests to pass or forward the cancellationToken accordingly to compile.src/BusinessApplications/AgentRegistry/Ports/IAuthorityPort.cs (1)
485-646:⚠️ Potential issue | 🟠 MajorInterface methods are missing
CancellationTokenparameters.Per coding guidelines, all public async methods must include a
CancellationTokenparameter. TheIAuthorityPortinterface methods should acceptCancellationToken cancellationToken = default.As per coding guidelines: "All public methods must be async with
CancellationTokenparameter"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Ports/IAuthorityPort.cs` around lines 485 - 646, The interface IAuthorityPort methods are missing CancellationToken parameters; update every async method signature (e.g., ConfigureAgentAuthorityAsync, GetAgentAuthorityScopeAsync, CreateAuthorityScopeTemplateAsync, GetAuthorityScopeTemplateAsync, ListAuthorityScopeTemplatesAsync, ApplyAuthorityScopeTemplateAsync, ValidateAuthorityAsync, ValidateActionWithinScopeAsync, OverrideAuthorityAsync, RevokeAuthorityOverrideAsync, GrantOverridePermissionAsync, RevokeOverridePermissionAsync, HasOverridePermissionAsync, ListUserOverridePermissionsAsync, GetAuthorityAuditRecordsAsync, GetAuthorityAuditRecordByIdAsync, ValidateAuthorityScopeComplianceAsync) to accept a final parameter CancellationToken cancellationToken = default, and add the necessary using for System.Threading if missing; ensure XML docs/signatures stay consistent and default value is applied so callers remain compatible.src/BusinessApplications/DecisionSupport/DecisionSupportController.cs (1)
51-123:⚠️ Potential issue | 🟠 MajorThread
CancellationTokenthrough to downstream async calls.The
cancellationTokenparameter is declared but never passed to_coordinator.ProcessQueryAsync(...)or_causalComponentmethod calls. This prevents request cancellation from propagating, potentially leaving long-running operations executing after clients disconnect.🔧 Proposed fix for AnalyzeSituation
- var analysisResponse = await _coordinator.ProcessQueryAsync(analysisQuery, options); + var analysisResponse = await _coordinator.ProcessQueryAsync(analysisQuery, options, cancellationToken); - var factorsResponse = await _coordinator.ProcessQueryAsync(factorsQuery); + var factorsResponse = await _coordinator.ProcessQueryAsync(factorsQuery, cancellationToken); - var causalAnalysis = await _causalComponent.ExtractCausalRelationsAsync( - request.Situation + "\n" + factorsResponse.Response, - "decision-support"); + var causalAnalysis = await _causalComponent.ExtractCausalRelationsAsync( + request.Situation + "\n" + factorsResponse.Response, + "decision-support", + cancellationToken); - var causalReasoning = await _causalComponent.PerformCausalReasoningAsync( - $"What are the likely outcomes and their causes in this situation: {request.Situation}?", - "decision-support"); + var causalReasoning = await _causalComponent.PerformCausalReasoningAsync( + $"What are the likely outcomes and their causes in this situation: {request.Situation}?", + "decision-support", + cancellationToken);Note: This requires the coordinator and engine methods to also accept
CancellationToken(see related comments on those files). Apply the same pattern toGenerateOptionsandExploreScenariosendpoints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/DecisionSupport/DecisionSupportController.cs` around lines 51 - 123, The AnalyzeSituation endpoint accepts a CancellationToken but never forwards it; update all async calls in AnalyzeSituation—specifically calls to _coordinator.ProcessQueryAsync (both for analysisQuery and factorsQuery) and to _causalComponent.ExtractCausalRelationsAsync and PerformCausalReasoningAsync—to accept and pass the cancellationToken parameter; also update the signatures of the corresponding coordinator and causal component methods to accept CancellationToken and propagate it through their downstream async operations, and apply the same change pattern to the GenerateOptions and ExploreScenarios endpoints so request cancellation consistently flows through the stack.src/BusinessApplications/AgentRegistry/AgentRegistry.csproj (1)
3-9: 🛠️ Refactor suggestion | 🟠 MajorRemove CS1591 suppression and add
TreatWarningsAsErrors.Line 8 suppresses CS1591 globally, which violates coding guidelines requiring XML documentation on public types. Additionally,
TreatWarningsAsErrorsis not enabled.♻️ Proposed fix
<PropertyGroup> <TargetFramework>net10.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <GenerateDocumentationFile>true</GenerateDocumentationFile> - <NoWarn>$(NoWarn);1591</NoWarn> + <TreatWarningsAsErrors>true</TreatWarningsAsErrors> </PropertyGroup>As per coding guidelines: "Do not suppress CS1591 warnings globally — add XML docs to public types instead" and "Build configuration must treat warnings as errors (TreatWarningsAsErrors=true)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/AgentRegistry.csproj` around lines 3 - 9, Remove the global CS1591 suppression from the NoWarn property and enable TreatWarningsAsErrors in the project properties: update the PropertyGroup that includes TargetFramework/ImplicitUsings/Nullable/GenerateDocumentationFile to remove "1591" from the NoWarn value (so public XML-doc warnings are not suppressed) and add a new element TreatWarningsAsErrors set to true; ensure the Project's GenerateDocumentationFile remains true so XML docs are produced and public types will surface CS1591 for authoring instead of being ignored.src/BusinessApplications/AdaptiveBalance/Controllers/AdaptiveBalanceController.cs (1)
65-84:⚠️ Potential issue | 🟡 MinorReturn
BadRequestinstead of throwingArgumentExceptionfor validation failures.
ApplyOverrideAsyncthrowsArgumentExceptionfor missingDimensionandOverriddenBy(lines 71, 76), which will result in a 500 error unless global exception handling converts it. In contrast,GetSpectrumHistoryAsynccorrectly returnsBadRequest(...). Use consistent validation patterns across endpoints.🔧 Proposed fix
public async Task<ActionResult<OverrideResponse>> ApplyOverrideAsync([FromBody] OverrideRequest request, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(request); if (string.IsNullOrWhiteSpace(request.Dimension)) { - throw new ArgumentException("Dimension is required.", nameof(request)); + return BadRequest("Dimension is required."); } if (string.IsNullOrWhiteSpace(request.OverriddenBy)) { - throw new ArgumentException("OverriddenBy is required.", nameof(request)); + return BadRequest("OverriddenBy is required."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AdaptiveBalance/Controllers/AdaptiveBalanceController.cs` around lines 65 - 84, In ApplyOverrideAsync, stop throwing ArgumentException for missing fields and return BadRequest results instead: validate request.Dimension and request.OverriddenBy (after ArgumentNullException.ThrowIfNull(request)) and if either is null/whitespace return BadRequest(...) with a clear message (e.g., "Dimension is required." / "OverriddenBy is required."); keep the existing log and only call _servicePort.ApplyOverrideAsync(request, cancellationToken) when validation passes so the method consistently returns ActionResult<OverrideResponse> like GetSpectrumHistoryAsync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80dab483-4780-4adc-8893-35b9decb7573
⛔ Files ignored due to path filters (4)
src/UILayer/web/package-lock.jsonis excluded by!**/package-lock.jsonsrc/UILayer/web/src/lib/api/generated/agentic.d.tsis excluded by!**/generated/**src/UILayer/web/src/lib/api/generated/index.tsis excluded by!**/generated/**src/UILayer/web/src/lib/api/generated/services.d.tsis excluded by!**/generated/**
📒 Files selected for processing (96)
.claude/settings.json.dockerignore.github/workflows/build.yml.gitignoreAGENT_BACKLOG.mdCognitiveMesh.slnDirectory.Packages.propsrenovate.jsonsrc/ApiHost/ApiHost.csprojsrc/ApiHost/Program.cssrc/BusinessApplications/AdaptiveBalance/AdaptiveBalance.csprojsrc/BusinessApplications/AdaptiveBalance/Controllers/AdaptiveBalanceController.cssrc/BusinessApplications/AgencyRouter/AgencyRouter.csprojsrc/BusinessApplications/AgencyRouter/Controllers/AgencyRouterController.cssrc/BusinessApplications/AgentRegistry/AgentRegistry.csprojsrc/BusinessApplications/AgentRegistry/Controllers/AgentController.cssrc/BusinessApplications/AgentRegistry/Data/AgentDataModels.cssrc/BusinessApplications/AgentRegistry/Infrastructure/ServiceCollectionExtensions.cssrc/BusinessApplications/AgentRegistry/Models/AgentConsentTypes.cssrc/BusinessApplications/AgentRegistry/Ports/IAgentConsentPort.cssrc/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cssrc/BusinessApplications/AgentRegistry/Ports/IAuthorityPort.cssrc/BusinessApplications/AgentRegistry/Services/AgentConsentService.cssrc/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cssrc/BusinessApplications/AgentRegistry/Services/AuthorityService.cssrc/BusinessApplications/CustomerIntelligence/CustomerIntelligence.csprojsrc/BusinessApplications/CustomerIntelligence/CustomerServiceController.cssrc/BusinessApplications/CustomerIntelligence/Models/InteractionRecord.cssrc/BusinessApplications/CustomerIntelligence/Models/ProductInfo.cssrc/BusinessApplications/CustomerIntelligence/Ports/ILatentDemandPort.cssrc/BusinessApplications/DecisionSupport/CausalUnderstandingEngine.cssrc/BusinessApplications/DecisionSupport/DecisionSupport.csprojsrc/BusinessApplications/DecisionSupport/DecisionSupportController.cssrc/BusinessApplications/DecisionSupport/DecisionSupportCoordinator.cssrc/BusinessApplications/DecisionSupport/Models/OptionsGenerationRequest.cssrc/BusinessApplications/DecisionSupport/Models/OptionsGenerationResponse.cssrc/BusinessApplications/DecisionSupport/Models/ScenarioExplorationRequest.cssrc/BusinessApplications/DecisionSupport/Models/ScenarioExplorationResponse.cssrc/BusinessApplications/DecisionSupport/Models/SituationAnalysisRequest.cssrc/BusinessApplications/DecisionSupport/Models/SituationAnalysisResponse.cssrc/BusinessApplications/DecisionSupport/RelationshipInfo.cssrc/BusinessApplications/ImpactMetrics/ImpactMetrics.csprojsrc/BusinessApplications/NISTCompliance/Controllers/NISTComplianceController.cssrc/BusinessApplications/NISTCompliance/NISTCompliance.csprojsrc/BusinessApplications/ResearchAnalysis/KnowledgeWorkController.cssrc/BusinessApplications/ResearchAnalysis/ResearchAnalysis.csprojsrc/BusinessApplications/ResearchAnalysis/ResearchAnalysisCoordinator.cssrc/BusinessApplications/ResearchAnalysis/ResearchRequest.cssrc/BusinessApplications/ValueGeneration/Adapters/InMemoryValueDiagnosticDataAdapter.cssrc/BusinessApplications/ValueGeneration/Controllers/ValueGenerationController.cssrc/BusinessApplications/ValueGeneration/Infrastructure/ServiceCollectionExtensions.cssrc/BusinessApplications/ValueGeneration/ValueGeneration.csprojsrc/ReasoningLayer/AgencyRouter/Engines/ContextualAdaptiveAgencyEngine.cssrc/UILayer/web/package.jsonsrc/UILayer/web/src/app/(app)/agents/page.tsxsrc/UILayer/web/src/app/(app)/analytics/page.tsxsrc/UILayer/web/src/app/(app)/compliance/page.tsxsrc/UILayer/web/src/app/(app)/dashboard/page.tsxsrc/UILayer/web/src/app/(app)/error.tsxsrc/UILayer/web/src/app/(app)/layout.tsxsrc/UILayer/web/src/app/(app)/loading.tsxsrc/UILayer/web/src/app/(app)/marketplace/page.tsxsrc/UILayer/web/src/app/(app)/settings/page.tsxsrc/UILayer/web/src/app/forbidden/page.tsxsrc/UILayer/web/src/app/layout.tsxsrc/UILayer/web/src/app/login/page.tsxsrc/UILayer/web/src/app/page.tsxsrc/UILayer/web/src/components/ApiBootstrap.tsxsrc/UILayer/web/src/components/ErrorBoundary/ErrorBoundary.tsxsrc/UILayer/web/src/components/ErrorBoundary/index.tssrc/UILayer/web/src/components/ExtensionErrorSuppressor.tsxsrc/UILayer/web/src/components/Navigation/Breadcrumbs.tsxsrc/UILayer/web/src/components/Navigation/ConnectionIndicator.tsxsrc/UILayer/web/src/components/Navigation/MobileMenu.tsxsrc/UILayer/web/src/components/Navigation/Sidebar.tsxsrc/UILayer/web/src/components/Navigation/TopBar.tsxsrc/UILayer/web/src/components/Navigation/index.tssrc/UILayer/web/src/components/Navigation/navItems.tssrc/UILayer/web/src/components/ProtectedRoute.tsxsrc/UILayer/web/src/components/Skeleton/Skeleton.tsxsrc/UILayer/web/src/components/Skeleton/index.tssrc/UILayer/web/src/components/Toast/Toast.tsxsrc/UILayer/web/src/components/Toast/index.tssrc/UILayer/web/src/contexts/AuthContext.tsxsrc/UILayer/web/src/hooks/useDashboardData.tssrc/UILayer/web/src/hooks/useSignalR.tssrc/UILayer/web/src/lib/api/client.tssrc/UILayer/web/src/lib/api/interceptors.tssrc/UILayer/web/src/middleware.tssrc/UILayer/web/src/services/api.tssrc/UILayer/web/src/stores/index.tssrc/UILayer/web/src/stores/useAgentStore.tssrc/UILayer/web/src/stores/useAuthStore.tssrc/UILayer/web/src/stores/useDashboardStore.tssrc/UILayer/web/src/stores/useNotificationStore.tssrc/UILayer/web/src/stores/usePreferencesStore.ts
💤 Files with no reviewable changes (2)
- src/UILayer/web/src/hooks/useDashboardData.ts
- src/UILayer/web/src/services/api.ts
Comment on lines
+11
to
+23
| "Bash(git *)", | ||
| "Bash(gh api *)", | ||
| "Bash(gh repo *)", | ||
| "Bash(gh issue *)", | ||
| "Bash(gh pr *)", | ||
| "Bash(gh label *)", | ||
| "Bash(gh run *)", | ||
| "Bash(gh run list *)", | ||
| "Bash(gh run view *)", | ||
| "Bash(gh run watch *)", | ||
| "Bash(gh workflow *)", | ||
| "Bash(gh variable *)", | ||
| "Bash(az *)", |
Contributor
There was a problem hiding this comment.
Replace the git/gh/az wildcards with explicit safe subcommands.
Line 11 and Lines 12-23 now authorize whole admin CLIs, so ordinary git push, arbitrary gh api mutations, and Azure create/update operations are allowed by default. The deny list only blocks a few cases, so this no longer enforces least privilege for repo or cloud access.
Suggested direction
- "Bash(git *)",
- "Bash(gh api *)",
- "Bash(gh repo *)",
- "Bash(gh issue *)",
- "Bash(gh pr *)",
- "Bash(gh label *)",
- "Bash(gh run *)",
- "Bash(gh run list *)",
- "Bash(gh run view *)",
- "Bash(gh run watch *)",
- "Bash(gh workflow *)",
- "Bash(gh variable *)",
- "Bash(az *)",
+ "Bash(git status *)",
+ "Bash(git diff *)",
+ "Bash(git log *)",
+ "Bash(gh pr view *)",
+ "Bash(gh pr checks *)",
+ "Bash(gh issue view *)",
+ "Bash(gh run list *)",
+ "Bash(gh run view *)",
+ "Bash(gh run watch *)",
+ "Bash(az account show *)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "Bash(git *)", | |
| "Bash(gh api *)", | |
| "Bash(gh repo *)", | |
| "Bash(gh issue *)", | |
| "Bash(gh pr *)", | |
| "Bash(gh label *)", | |
| "Bash(gh run *)", | |
| "Bash(gh run list *)", | |
| "Bash(gh run view *)", | |
| "Bash(gh run watch *)", | |
| "Bash(gh workflow *)", | |
| "Bash(gh variable *)", | |
| "Bash(az *)", | |
| "Bash(git status *)", | |
| "Bash(git diff *)", | |
| "Bash(git log *)", | |
| "Bash(gh pr view *)", | |
| "Bash(gh pr checks *)", | |
| "Bash(gh issue view *)", | |
| "Bash(gh run list *)", | |
| "Bash(gh run view *)", | |
| "Bash(gh run watch *)", | |
| "Bash(az account show *)" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json around lines 11 - 23, The settings.json currently
grants broad CLI permissions via wildcard entries like "Bash(git *)", "Bash(gh
api *)", "Bash(gh repo *)", "Bash(gh issue *)", "Bash(gh pr *)", "Bash(gh label
*)", "Bash(gh run *)", "Bash(gh run list *)", "Bash(gh run view *)", "Bash(gh
run watch *)", "Bash(gh workflow *)", "Bash(gh variable *)", "Bash(az *)";
replace each wildcard with an explicit allow-list of only the safe subcommands
your automation needs (e.g., specific git/gh/az commands used in CI or read-only
ops) and remove the star entries, updating the deny list accordingly so
dangerous mutations (like gh api mutations, repo admin, or az create/update)
remain blocked; locate the wildcard strings in .claude/settings.json and edit
them to explicit entries (e.g., permit only the exact "git" or "gh" subcommands
required) rather than using *.
Comment on lines
+24
to
+31
| "Bash(node *)", | ||
| "Bash(pnpm *)", | ||
| "Bash(npm *)", | ||
| "Bash(npx *)", | ||
| "Bash(turbo *)", | ||
| "Bash(python *)", | ||
| "Bash(python3 *)", | ||
| "Bash(pip *)", |
Contributor
There was a problem hiding this comment.
Remove the general-purpose runtimes and package managers from the allow list.
These entries effectively grant arbitrary code execution. A one-liner like python -c ..., node -e ..., npx ..., or PowerShell can bypass every command-specific deny rule here and still delete files, exfiltrate data, or mutate repo/cloud state.
Suggested direction
- "Bash(node *)",
- "Bash(pnpm *)",
- "Bash(npm *)",
- "Bash(npx *)",
- "Bash(turbo *)",
- "Bash(python *)",
- "Bash(python3 *)",
- "Bash(pip *)",
...
- "Bash(powershell *)",
- "Bash(powershell.exe *)",Also applies to: 52-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json around lines 24 - 31, The allow list currently
includes broad runtime/package patterns such as "Bash(node *)", "Bash(pnpm *)",
"Bash(npm *)", "Bash(npx *)", "Bash(python *)", "Bash(python3 *)", "Bash(pip *)"
(and the similar entries later in the file), which effectively permit arbitrary
code execution; remove these general-purpose runtime and package-manager
patterns from the allow list so only explicit, minimal command patterns remain
(e.g., keep only tightly-scoped, purpose-specific entries) and ensure the
overall policy no longer contains wildcarded runtime/package manager entries
like the strings above.
Comment on lines
+55
to
+65
| var app = builder.Build(); | ||
|
|
||
| if (app.Environment.IsDevelopment()) | ||
| { | ||
| app.MapOpenApi(); | ||
| } | ||
|
|
||
| app.UseCors("Frontend"); | ||
|
|
||
| app.MapControllers(); | ||
| app.MapCognitiveMeshHubs(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect auth metadata in referenced assemblies versus host middleware.
rg -n -C2 '\[Authorize|RequireAuthorization\(' src/AgencyLayer src/BusinessApplications src/ApiHost
rg -n -C2 'AddAuthentication|AddAuthorization|UseAuthentication|UseAuthorization' src/ApiHostRepository: phoenixvc/cognitive-mesh
Length of output: 18545
🏁 Script executed:
cat -n src/ApiHost/Program.csRepository: phoenixvc/cognitive-mesh
Length of output: 3221
🏁 Script executed:
rg -n 'AuthorizeAttribute|Authorize|RequireAuthorization' --type cs | grep -i 'hub\|signalr' | head -20Repository: phoenixvc/cognitive-mesh
Length of output: 50
🏁 Script executed:
rg -n 'MapCognitiveMeshHubs|class.*Hub' --type cs -A5 | head -50Repository: phoenixvc/cognitive-mesh
Length of output: 1951
🏁 Script executed:
cat -n src/AgencyLayer/RealTime/Hubs/CognitiveMeshHub.cs | head -30Repository: phoenixvc/cognitive-mesh
Length of output: 1612
🏁 Script executed:
cat -n src/AgencyLayer/RealTime/Hubs/CognitiveMeshHub.cs | tail -n +30Repository: phoenixvc/cognitive-mesh
Length of output: 6854
Add authentication and authorization middleware before mapping controllers and hubs.
The host has no AddAuthentication(), AddAuthorization(), UseAuthentication(), or UseAuthorization() registered. Multiple mapped controllers require authentication via [Authorize] attributes (SecurityController, ValueGenerationController, KnowledgeWorkController, DecisionSupportController, ConvenerController, ComplianceController, AgencyRouterController, AgentController). Without the middleware, these endpoints will remain unprotected or fail at runtime with policy evaluation errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ApiHost/Program.cs` around lines 55 - 65, The app is missing
authentication/authorization middleware; add service registrations and
middleware before mapping controllers and hubs: call
builder.Services.AddAuthentication(/* configure scheme or call
AddJwtBearer/whatever scheme used by the project */) and
builder.Services.AddAuthorization(/* policies if any */) during startup service
configuration, then call app.UseAuthentication() and app.UseAuthorization()
immediately after app.UseCors("Frontend") and before app.MapControllers() and
app.MapCognitiveMeshHubs(); ensure you use the same auth scheme/policy names
that controllers/hubs expect (e.g., the scheme configured for [Authorize]
policies) so SecurityController, ValueGenerationController,
KnowledgeWorkController, DecisionSupportController, ConvenerController,
ComplianceController, AgencyRouterController, and AgentController are protected.
Comment on lines
+21
to
+22
| await Task.CompletedTask; | ||
| return new CoordinatorResponse(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n# References to ResearchAnalysisCoordinator (look for DI registration or direct construction)\n'
rg -nC2 --type cs -g '!**/bin/**' -g '!**/obj/**' '\bResearchAnalysisCoordinator\b'
printf '\n# Derived implementations / overrides\n'
rg -nC3 --type cs -g '!**/bin/**' -g '!**/obj/**' ':\s*ResearchAnalysisCoordinator\b|override\s+(async\s+)?Task<CoordinatorResponse>\s+ProcessQueryAsync|override\s+(async\s+)?Task\s+IndexDocumentAsync'Repository: phoenixvc/cognitive-mesh
Length of output: 1892
🏁 Script executed:
# Check if ResearchAnalysisCoordinator is abstract or concrete, and view the method signatures
head -40 src/BusinessApplications/ResearchAnalysis/ResearchAnalysisCoordinator.cs
# Search for DI registration patterns
rg -n --type cs 'AddScoped|AddSingleton|AddTransient|services\.Add' | grep -i 'research\|coordinator' | head -20
# Broader search for DI configuration files
fd -e cs 'Program|Startup|ServiceCollection|DependencyInjection' src/Repository: phoenixvc/cognitive-mesh
Length of output: 2629
🏁 Script executed:
# Look for ResearchAnalysis-specific DI configuration
fd -path '*/ResearchAnalysis/*' -name '*ServiceCollection*' -o -name '*DependencyInjection*' src/
# Check the ResearchAnalysis folder structure
find src/BusinessApplications/ResearchAnalysis -type f -name '*.cs' | head -20
# Search for any registration or bootstrap code in ResearchAnalysis
rg -n --type cs 'ResearchAnalysisCoordinator|AddResearch|AddKnowledgeWork' src/BusinessApplications/ResearchAnalysis/Repository: phoenixvc/cognitive-mesh
Length of output: 952
🏁 Script executed:
# Check Program.cs for ResearchAnalysisCoordinator registration
cat src/ApiHost/Program.cs | grep -A5 -B5 -i 'researchanalysis\|knowledgework'
# Also check if there's a global DI extension
rg -n 'ResearchAnalysisCoordinator|AddResearch' src/ApiHost/Program.csRepository: phoenixvc/cognitive-mesh
Length of output: 50
🏁 Script executed:
# View Program.cs to understand DI setup
head -100 src/ApiHost/Program.cs
# Check KnowledgeWorkController for any registration attributes or DI configuration
cat src/BusinessApplications/ResearchAnalysis/KnowledgeWorkController.csRepository: phoenixvc/cognitive-mesh
Length of output: 16256
🏁 Script executed:
# Search for any implicit or auto-registration of ResearchAnalysisCoordinator
rg -n 'ResearchAnalysisCoordinator' src/ --type cs
# Check if there's any scanning or reflection-based registration
rg -n 'Scan|Assembly|Keyed|Factory|Activator' src/ApiHost/Program.csRepository: phoenixvc/cognitive-mesh
Length of output: 1191
Add CancellationToken parameters to both public async methods and register a derived implementation or make the class abstract.
Two issues confirmed:
-
No-op coordinator will execute at runtime: The concrete
ResearchAnalysisCoordinatorclass is injected intoKnowledgeWorkControllerwith no DI registration found and no derived implementations in the codebase. If instantiated, bothProcessQueryAsyncandIndexDocumentAsyncreturn empty responses, making all three API endpoints (research, synthesis, content creation) appear successful while returning no data. -
CancellationTokenparameter is missing: BothProcessQueryAsync(line 17) andIndexDocumentAsync(line 30) are public async methods that must accept aCancellationTokenparameter per guidelines. The controller receives cancellation tokens from callers but cannot propagate them to the coordinator.
Fix by: (a) making ResearchAnalysisCoordinator abstract or non-instantiable, (b) adding a derived implementation with real logic, (c) registering that implementation in DI, and (d) adding CancellationToken ct parameter to both public async methods and propagating it through all async calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BusinessApplications/ResearchAnalysis/ResearchAnalysisCoordinator.cs`
around lines 21 - 22, The ResearchAnalysisCoordinator class currently provides
no-op implementations for ProcessQueryAsync and IndexDocumentAsync and lacks
CancellationToken parameters; make the class abstract (or otherwise
non-instantiable), add a concrete derived implementation (e.g.,
ResearchAnalysisCoordinatorImpl) with real logic, register that implementation
in DI so KnowledgeWorkController receives a functional coordinator, and update
both public methods ProcessQueryAsync and IndexDocumentAsync to accept a
CancellationToken ct and propagate ct through all awaited async calls inside the
coordinator and its implementation.
Comment on lines
+5
to
+13
| function isJwtExpired(token: string): boolean { | ||
| try { | ||
| const parts = token.split(".") | ||
| if (parts.length !== 3) return true | ||
| const payload = JSON.parse(atob(parts[1].replace(/-/g, "+").replace(/_/g, "/"))) | ||
| if (typeof payload.exp !== "number") return true | ||
| return Date.now() >= payload.exp * 1000 | ||
| } catch { | ||
| return true |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "middleware.ts" -path "*/UILayer/web/*" | head -20Repository: phoenixvc/cognitive-mesh
Length of output: 103
🏁 Script executed:
cat -n src/UILayer/web/src/middleware.ts 2>/dev/null || find . -type f -name "middleware.ts" | xargs wc -l 2>/dev/nullRepository: phoenixvc/cognitive-mesh
Length of output: 1629
Don't use unverified JWT payloads for authentication decisions.
At line 31, isJwtExpired (lines 5–15) only base64-decodes and checks the exp claim without verifying the JWT signature. Any client can forge a JWT with a valid three-part structure and a far-future expiration, then set it as a cookie to bypass this middleware guard entirely. Before allowing access to protected routes, verify the token signature cryptographically (using a shared secret or public key), and validate all required claims—don't rely on client-provided payload fields alone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UILayer/web/src/middleware.ts` around lines 5 - 13, The isJwtExpired
function currently parses the JWT payload without verifying the signature;
replace this manual base64 decode with a proper JWT verification step (e.g.,
using a library like jsonwebtoken or jose) inside isJwtExpired (or rename to
isJwtValidAndNotExpired) to cryptographically verify the token using the
configured HMAC secret or public key, then check the verified token's exp claim
and any other required claims (iss/aud) and return false only if verification
passes and exp is in the future; ensure all verification errors cause the
function to return true (expired/invalid) and avoid trusting client-provided
payload fields.
Brings in ESLint migration (next lint → eslint .), CodeQL LogSanitizer fix, CI type-check continue-on-error, and all Phase 13 fixes. Resolves CustomerServiceController conflict by keeping try/catch + Sanitize(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sync with latest dev to resolve PR merge conflicts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
+199
to
+204
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error executing orchestrated task."); | ||
| return StatusCode(StatusCodes.Status500InternalServerError, | ||
| ErrorEnvelope.Create("SERVICE_UNAVAILABLE", "Orchestration service is unavailable.")); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 24 days ago
In general, the issue is fixed by not catching System.Exception blindly. Instead, catch and handle only the exception types you expect (for example, domain-specific exceptions or cancellation-related ones), and either let unexpected exceptions bubble up to global error handling middleware or handle them in a final catch after the specific ones. This preserves diagnostic information and allows more accurate HTTP status codes.
For this specific method, the minimal change without altering intended functionality is to:
- Add a specific
catchforOperationCanceledException(and possiblyTaskCanceledException, depending on how cancellation is surfaced) so that request cancellations don’t get logged as orchestration failures and don’t return a misleading 500. - Keep a final
catch (Exception ex)as a last-resort safety net, which maintains the current behavior for all other exceptions.
Concretely:
- In
src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs, withinExecuteTask, split the existingcatch (Exception ex)into two catch blocks:catch (OperationCanceledException)that returns an appropriate status code (commonly 499 or 400/408; since 499 is non-standard and 400/408 are standard, we can choose 400 Bad Request to indicate client-initiated cancel, or 499 if your infra uses it. Given standard ASP.NET Core constants, 400 is safe) and avoids logging it as an error.- Followed by the existing
catch (Exception ex)block unchanged, preserving the current logging and 500"SERVICE_UNAVAILABLE"response for all other exceptions.
No new imports are needed, as OperationCanceledException is in System, which is already imported. The change is localized to the ExecuteTask method.
Suggested changeset
1
src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs
| @@ -196,6 +196,12 @@ | ||
| var response = await _orchestrationPort.ExecuteTaskAsync(request, cancellationToken); | ||
| return Ok(response); | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
| // The request was canceled (e.g., client disconnected or timeout); do not treat as an internal error. | ||
| return StatusCode(StatusCodes.Status400BadRequest, | ||
| ErrorEnvelope.Create("REQUEST_CANCELLED", "The orchestration task was cancelled.")); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error executing orchestrated task."); |
Copilot is powered by AI and may make mistakes. Always verify output.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs (2)
106-130:⚠️ Potential issue | 🟠 Major404 is not actually reachable for missing customers here.
GenerateCustomerInsightsAsynccurrently returns an empty sequence when no interaction history exists; it does not throwKeyNotFoundException. As written, this endpoint still returns200 OKin that path while the new contract advertises404. Either add a real existence check before generating insights, or remove the 404 response/catch until the manager signals not-found.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs` around lines 106 - 130, The controller's GenerateInsightsAsync advertises a 404 but catches KeyNotFoundException which never occurs because GenerateCustomerInsightsAsync returns an empty sequence; either remove the NotFound handling and the [ProducesResponseType(StatusCodes.Status404NotFound)] attribute, or add an explicit existence check before generating insights (e.g., call a new or existing method on _intelligenceManager or a customer repository such as CustomerExistsAsync/GetCustomerByIdAsync) and return NotFound($"Customer not found: {customerId}") when that check fails; update the catch block accordingly (remove KeyNotFoundException or adjust to actual exceptions thrown) so the response contract matches the implementation.
145-169:⚠️ Potential issue | 🟠 Major404 is unreachable for predictions too.
PredictCustomerBehaviorAsyncdoes not throwKeyNotFoundExceptionwhen the customer has no behavioral data; it continues with empty features and returns a prediction. That means this endpoint still answers200 OKwhile the new 404 metadata suggests otherwise. Please align the contract with the actual manager behavior before shipping this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs` around lines 145 - 169, The 404 handling in PredictBehaviorAsync is misleading because _intelligenceManager.PredictCustomerBehaviorAsync never throws KeyNotFoundException; either remove the NotFound response metadata and the catch, or change PredictBehaviorAsync to detect the "no behavioral data" result from PredictCustomerBehaviorAsync and return NotFound explicitly; specifically update PredictBehaviorAsync (the controller method) to call PredictCustomerBehaviorAsync, inspect the returned prediction or its features/IsEmpty flag (or adjust PredictCustomerBehaviorAsync to throw a KeyNotFoundException when no data) and then return NotFound($"Customer not found: {customerId}") when appropriate, otherwise return Ok(prediction). Ensure _logger and Sanitize(customerId) usage remains unchanged.src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs (4)
148-155:⚠️ Potential issue | 🟡 MinorCancellationToken not propagated to DeactivateAgentAsync.
Consistent with the pattern noted in other methods - the token should be passed through.
🔧 Proposed fix
-var success = await _registryPort.DeactivateAgentAsync(agentId, tenantId, userId ?? "system", reason ?? "Manual deactivation"); +var success = await _registryPort.DeactivateAgentAsync(agentId, tenantId, userId ?? "system", reason ?? "Manual deactivation", cancellationToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 148 - 155, The DeactivateAgent controller action calls _registryPort.DeactivateAgentAsync without forwarding the method's CancellationToken, so update the call inside DeactivateAgent (after GetAuthContextFromClaims and before handling the result) to pass the cancellationToken into _registryPort.DeactivateAgentAsync (the call that currently supplies agentId, tenantId, userId ?? "system", reason ?? "Manual deactivation"); ensure the signature matches the DeactivateAgentAsync overload and propagate cancellationToken through to the registry port.
117-124:⚠️ Potential issue | 🔴 CriticalPort method signature is missing CancellationToken parameter.
The
GetAgentByIdAsyncmethod inIAgentRegistryPortdoes not accept aCancellationTokenparameter. According to coding guidelines, all public async methods must include aCancellationTokenparameter. Update the port interface method signature to acceptCancellationToken, then pass it from the controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 117 - 124, Update the IAgentRegistryPort.GetAgentByIdAsync signature to accept a CancellationToken and propagate that change through its implementations, then pass the controller's cancellationToken when calling _registryPort.GetAgentByIdAsync in GetAgentDetails; specifically, modify the IAgentRegistryPort interface method GetAgentByIdAsync to include a CancellationToken parameter and update all concrete classes implementing GetAgentByIdAsync, and change the call in AgentController.GetAgentDetails from _registryPort.GetAgentByIdAsync(agentId, tenantId) to _registryPort.GetAgentByIdAsync(agentId, tenantId, cancellationToken).
70-79:⚠️ Potential issue | 🟠 MajorThe
IAgentRegistryPort.RegisterAgentAsyncmethod does not currently accept aCancellationTokenparameter.Per the coding guidelines, all public methods should be async with a
CancellationTokenparameter. Update the port interface to include the parameter, then pass it from the controller:🔧 Proposed fix
Update
IAgentRegistryPort:-Task<Agent> RegisterAgentAsync(AgentRegistrationRequest request); +Task<Agent> RegisterAgentAsync(AgentRegistrationRequest request, CancellationToken cancellationToken = default);Then update the controller call:
-var agent = await _registryPort.RegisterAgentAsync(request); +var agent = await _registryPort.RegisterAgentAsync(request, cancellationToken);Also update the implementation in
AgentRegistryServiceto accept and use the parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 70 - 79, Update the async registration flow to accept and propagate a CancellationToken: add a CancellationToken parameter to IAgentRegistryPort.RegisterAgentAsync, update AgentRegistryService.RegisterAgentAsync (and any other implementations) to accept and honor that token, and change the call in AgentController.RegisterAgent to call _registryPort.RegisterAgentAsync(request, cancellationToken) so the controller's cancellationToken is passed through to the port and service layers.
215-229:⚠️ Potential issue | 🟡 MinorAdd try-catch wrapper for consistent error handling.
Unlike other endpoints (
RegisterAgent,GetAgentDetails,DeactivateAgent,ExecuteTask), this method lacks a try-catch wrapper. Unhandled exceptions will result in a generic 500 response without the structuredErrorEnvelope.🔧 Proposed fix
public async Task<IActionResult> ConfigureAuthority(string agentType, [FromBody] RegistryAuthorityScope scope, CancellationToken cancellationToken = default) { + try + { var (tenantId, userId) = GetAuthContextFromClaims(); if (tenantId == null) return Unauthorized(ErrorEnvelope.Create("UNAUTHORIZED", "Tenant ID is missing.")); // Look up agent by type to resolve the agentId var agents = await _registryPort.GetAgentsByTypeAsync(agentType, tenantId); var targetAgent = agents.FirstOrDefault(); if (targetAgent == null) { return NotFound(ErrorEnvelope.Create("AGENT_NOT_FOUND", $"No agent of type '{agentType}' was found.")); } await _authorityPort.ConfigureAgentAuthorityAsync(targetAgent.AgentId, scope, userId ?? "system", tenantId); return NoContent(); + } + catch (Exception ex) + { + _logger.LogError(ex, "An error occurred while configuring authority for agent type '{AgentType}'.", Sanitize(agentType)); + return StatusCode(StatusCodes.Status500InternalServerError, + ErrorEnvelope.Create("SERVICE_UNAVAILABLE", "Authority configuration service is unavailable.")); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 215 - 229, The ConfigureAuthority endpoint lacks the try-catch pattern used by other actions, so unhandled exceptions won't return a structured ErrorEnvelope; wrap the body of ConfigureAuthority (after calling GetAuthContextFromClaims and before returning) in a try-catch, catch Exception ex and return StatusCode(500, ErrorEnvelope.Create("UNEXPECTED_ERROR", ex.Message)) (or include ex.ToString() if stack is desired) to match the error handling used in RegisterAgent/GetAgentDetails/DeactivateAgent/ExecuteTask; ensure the try block still calls _registryPort.GetAgentsByTypeAsync(agentType, tenantId), selects targetAgent, and awaits _authorityPort.ConfigureAgentAuthorityAsync(...) as before.
🧹 Nitpick comments (3)
src/UILayer/web/package.json (1)
17-17: Consider using consistent version pinning strategy.The new dependencies (
@microsoft/signalr,zustand) use caret (^) versioning, while most other runtime dependencies (e.g.,next,react,d3,framer-motion) use exact versions. The existingopenapi-fetch@^0.17.0also uses caret versioning, so there's already some inconsistency.Using exact versions provides deterministic builds and prevents unexpected breaking changes from minor/patch updates. Consider pinning these to exact versions for consistency:
- "@microsoft/signalr": "^10.0.0", + "@microsoft/signalr": "10.0.0",- "zustand": "^5.0.11" + "zustand": "5.0.11"Alternatively, if flexible versioning is intentional for these packages, document the rationale.
Also applies to: 26-26, 33-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UILayer/web/package.json` at line 17, The package.json entries for "@microsoft/signalr" and "zustand" currently use caret versioning (e.g., "@microsoft/signalr": "^10.0.0") which is inconsistent with most other runtime deps pinned to exact versions; either change these entries to exact versions (remove the caret, e.g., "@microsoft/signalr": "10.0.0" and "zustand": "X.Y.Z") to match the repo’s deterministic pinning strategy, or add a short comment in README/CONTRIBUTING explaining why caret ranges are allowed for these packages; update the entries referenced by the diff (the "@microsoft/signalr" and "zustand" keys) accordingly to maintain consistency.src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs (2)
162-167: Same fire-and-forget exception handling concern as RegisterAgent.Consider broadening the catch to
Exceptionfor consistency with the other fire-and-forget blocks, once that refactor is applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 162 - 167, The fire-and-forget Task.Run wrapper that calls _audit.LogAgentRetiredAsync currently only catches InvalidOperationException and HttpRequestException; change the catch strategy to mirror other fire-and-forget blocks (e.g., RegisterAgent) by catching Exception instead, logging the exception via _logger.LogWarning or appropriate level with the same message ("Failed to log agent retirement audit for {AgentId}.") and including agentId and the exception; update the anonymous async Task.Run block around _audit.LogAgentRetiredAsync to catch Exception to ensure all unexpected errors are handled consistently.
87-98: Fire-and-forget pattern has limited exception handling and potential service lifetime issues.Two concerns with the
Task.Runapproach:
Limited exception types: Only
InvalidOperationExceptionandHttpRequestExceptionare caught. Per theAuditLoggingAdapterimplementation,ArgumentNullExceptioncan also be thrown, and other exceptions from circuit breakers or repositories may surface.Service lifetime risk:
_auditand_notifyare captured by the closure. If these are scoped or transient services, they could be disposed when the HTTP request completes, before the background task finishes.Consider catching a broader exception type or using a background task queue as noted in the comment.
♻️ Proposed fix to catch broader exceptions
_ = Task.Run(async () => { try { await _audit.LogAgentRegisteredAsync(agent.AgentId, agent.AgentType, actorName); } - catch (InvalidOperationException ex) { _logger.LogWarning(ex, "Failed to log agent registration audit for {AgentId}.", agent.AgentId); } - catch (HttpRequestException ex) { _logger.LogWarning(ex, "Failed to log agent registration audit for {AgentId}.", agent.AgentId); } + catch (Exception ex) { _logger.LogWarning(ex, "Failed to log agent registration audit for {AgentId}.", agent.AgentId); } }); _ = Task.Run(async () => { try { await _notify.SendAgentRegistrationNotificationAsync(agent.AgentId, agent.AgentType, actorName, new[] { actorName }); } - catch (InvalidOperationException ex) { _logger.LogWarning(ex, "Failed to send registration notification for {AgentId}.", agent.AgentId); } - catch (HttpRequestException ex) { _logger.LogWarning(ex, "Failed to send registration notification for {AgentId}.", agent.AgentId); } + catch (Exception ex) { _logger.LogWarning(ex, "Failed to send registration notification for {AgentId}.", agent.AgentId); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 87 - 98, The current fire-and-forget Task.Run blocks that call _audit.LogAgentRegisteredAsync and _notify.SendAgentRegistrationNotificationAsync risk unhandled exceptions and capturing scoped/transient services; replace these Task.Run usages with enqueuing work into a background task queue (e.g., an IBackgroundTaskQueue consumed by an IHostedService) or, at minimum, create an IServiceScope inside the background execution and resolve scoped services there and broaden exception handling to catch Exception (not just InvalidOperationException/HttpRequestException) around the calls to LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync so all errors are logged; update the code paths referencing Task.Run, _audit, _notify, LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/UILayer/web/coverage/clover.xml`:
- Around line 1-15: Remove generated coverage artifacts and stop tracking them:
delete the committed coverage files (e.g., src/UILayer/web/coverage/clover.xml
and coverage-final.json), add an entry to .gitignore to exclude the coverage/
directory (or specifically coverage/ and **/coverage/), and run git rm --cached
on any tracked coverage files so they are untracked going forward; ensure the
clover.xml path/PII (CognitiveMeshButton.tsx absolute path) is no longer present
in the repo history for new commits.
In `@src/UILayer/web/coverage/lcov-report/base.css`:
- Around line 1-224: The LCOV-generated stylesheet (base.css) and its coverage
assets are checked into source; remove the generated coverage folder from the
source tree and treat it as a build artifact by deleting the committed coverage
files (including base.css), adding the coverage output pattern (e.g.
coverage/lcov-report/**) to .gitignore, and updating CI/build configs to
generate coverage into a non-src output directory (or explicitly exclude the
coverage path from lint/build inputs) so selectors like .coverage-summary,
.cover-fill and .wrapper remain only in generated artifacts and not in the
source repository.
In `@src/UILayer/web/coverage/lcov-report/block-navigation.js`:
- Around line 24-38: The functions toggleClass and makeCurrent assume
missingCoverageElements contains nodes and directly access .item(...).classList
and .item(...).scrollIntoView, which throws when the NodeList is empty; update
both toggleClass and makeCurrent to first check that missingCoverageElements &&
missingCoverageElements.length > 0 and that missingCoverageElements.item(index)
is not null (and that currentIndex is within range) and simply return/no-op if
there are no elements or the requested index is out of bounds; keep the rest of
the behavior (updating currentIndex, adding/removing 'highlighted', and calling
scrollIntoView) only after those guards to prevent dereferencing null on fully
covered reports.
In `@src/UILayer/web/coverage/lcov-report/prettify.js`:
- Around line 1-2: Summary: Generated LCOV/HTML coverage artifacts (e.g., files
like CognitiveMeshButton.tsx.html and the bundled prettify.js) were committed
under the app tree. Fix: remove the generated coverage output from source
control by deleting the coverage directory and any vendored files (e.g., the
prettify.js and HTML files referenced such as CognitiveMeshButton.tsx.html) from
the commit/branch, and add the coverage output path to .gitignore (so
CI-generated reports are not committed). Ensure no leftover references to these
files remain in the app code or build, and regenerate coverage only in CI or an
artifacts bucket.
---
Outside diff comments:
In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs`:
- Around line 148-155: The DeactivateAgent controller action calls
_registryPort.DeactivateAgentAsync without forwarding the method's
CancellationToken, so update the call inside DeactivateAgent (after
GetAuthContextFromClaims and before handling the result) to pass the
cancellationToken into _registryPort.DeactivateAgentAsync (the call that
currently supplies agentId, tenantId, userId ?? "system", reason ?? "Manual
deactivation"); ensure the signature matches the DeactivateAgentAsync overload
and propagate cancellationToken through to the registry port.
- Around line 117-124: Update the IAgentRegistryPort.GetAgentByIdAsync signature
to accept a CancellationToken and propagate that change through its
implementations, then pass the controller's cancellationToken when calling
_registryPort.GetAgentByIdAsync in GetAgentDetails; specifically, modify the
IAgentRegistryPort interface method GetAgentByIdAsync to include a
CancellationToken parameter and update all concrete classes implementing
GetAgentByIdAsync, and change the call in AgentController.GetAgentDetails from
_registryPort.GetAgentByIdAsync(agentId, tenantId) to
_registryPort.GetAgentByIdAsync(agentId, tenantId, cancellationToken).
- Around line 70-79: Update the async registration flow to accept and propagate
a CancellationToken: add a CancellationToken parameter to
IAgentRegistryPort.RegisterAgentAsync, update
AgentRegistryService.RegisterAgentAsync (and any other implementations) to
accept and honor that token, and change the call in
AgentController.RegisterAgent to call _registryPort.RegisterAgentAsync(request,
cancellationToken) so the controller's cancellationToken is passed through to
the port and service layers.
- Around line 215-229: The ConfigureAuthority endpoint lacks the try-catch
pattern used by other actions, so unhandled exceptions won't return a structured
ErrorEnvelope; wrap the body of ConfigureAuthority (after calling
GetAuthContextFromClaims and before returning) in a try-catch, catch Exception
ex and return StatusCode(500, ErrorEnvelope.Create("UNEXPECTED_ERROR",
ex.Message)) (or include ex.ToString() if stack is desired) to match the error
handling used in RegisterAgent/GetAgentDetails/DeactivateAgent/ExecuteTask;
ensure the try block still calls _registryPort.GetAgentsByTypeAsync(agentType,
tenantId), selects targetAgent, and awaits
_authorityPort.ConfigureAgentAuthorityAsync(...) as before.
In `@src/BusinessApplications/CustomerIntelligence/CustomerServiceController.cs`:
- Around line 106-130: The controller's GenerateInsightsAsync advertises a 404
but catches KeyNotFoundException which never occurs because
GenerateCustomerInsightsAsync returns an empty sequence; either remove the
NotFound handling and the [ProducesResponseType(StatusCodes.Status404NotFound)]
attribute, or add an explicit existence check before generating insights (e.g.,
call a new or existing method on _intelligenceManager or a customer repository
such as CustomerExistsAsync/GetCustomerByIdAsync) and return NotFound($"Customer
not found: {customerId}") when that check fails; update the catch block
accordingly (remove KeyNotFoundException or adjust to actual exceptions thrown)
so the response contract matches the implementation.
- Around line 145-169: The 404 handling in PredictBehaviorAsync is misleading
because _intelligenceManager.PredictCustomerBehaviorAsync never throws
KeyNotFoundException; either remove the NotFound response metadata and the
catch, or change PredictBehaviorAsync to detect the "no behavioral data" result
from PredictCustomerBehaviorAsync and return NotFound explicitly; specifically
update PredictBehaviorAsync (the controller method) to call
PredictCustomerBehaviorAsync, inspect the returned prediction or its
features/IsEmpty flag (or adjust PredictCustomerBehaviorAsync to throw a
KeyNotFoundException when no data) and then return NotFound($"Customer not
found: {customerId}") when appropriate, otherwise return Ok(prediction). Ensure
_logger and Sanitize(customerId) usage remains unchanged.
---
Nitpick comments:
In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs`:
- Around line 162-167: The fire-and-forget Task.Run wrapper that calls
_audit.LogAgentRetiredAsync currently only catches InvalidOperationException and
HttpRequestException; change the catch strategy to mirror other fire-and-forget
blocks (e.g., RegisterAgent) by catching Exception instead, logging the
exception via _logger.LogWarning or appropriate level with the same message
("Failed to log agent retirement audit for {AgentId}.") and including agentId
and the exception; update the anonymous async Task.Run block around
_audit.LogAgentRetiredAsync to catch Exception to ensure all unexpected errors
are handled consistently.
- Around line 87-98: The current fire-and-forget Task.Run blocks that call
_audit.LogAgentRegisteredAsync and
_notify.SendAgentRegistrationNotificationAsync risk unhandled exceptions and
capturing scoped/transient services; replace these Task.Run usages with
enqueuing work into a background task queue (e.g., an IBackgroundTaskQueue
consumed by an IHostedService) or, at minimum, create an IServiceScope inside
the background execution and resolve scoped services there and broaden exception
handling to catch Exception (not just
InvalidOperationException/HttpRequestException) around the calls to
LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync so all errors
are logged; update the code paths referencing Task.Run, _audit, _notify,
LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync accordingly.
In `@src/UILayer/web/package.json`:
- Line 17: The package.json entries for "@microsoft/signalr" and "zustand"
currently use caret versioning (e.g., "@microsoft/signalr": "^10.0.0") which is
inconsistent with most other runtime deps pinned to exact versions; either
change these entries to exact versions (remove the caret, e.g.,
"@microsoft/signalr": "10.0.0" and "zustand": "X.Y.Z") to match the repo’s
deterministic pinning strategy, or add a short comment in README/CONTRIBUTING
explaining why caret ranges are allowed for these packages; update the entries
referenced by the diff (the "@microsoft/signalr" and "zustand" keys) accordingly
to maintain consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98f916e5-c55a-4465-b450-74517cf4de14
⛔ Files ignored due to path filters (3)
src/UILayer/web/coverage/lcov-report/favicon.pngis excluded by!**/*.pngsrc/UILayer/web/coverage/lcov-report/sort-arrow-sprite.pngis excluded by!**/*.pngsrc/UILayer/web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
src/BusinessApplications/AdaptiveBalance/Controllers/AdaptiveBalanceController.cssrc/BusinessApplications/AgentRegistry/Controllers/AgentController.cssrc/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cssrc/BusinessApplications/CustomerIntelligence/CustomerServiceController.cssrc/UILayer/web/coverage/clover.xmlsrc/UILayer/web/coverage/coverage-final.jsonsrc/UILayer/web/coverage/lcov-report/CognitiveMeshButton.tsx.htmlsrc/UILayer/web/coverage/lcov-report/base.csssrc/UILayer/web/coverage/lcov-report/block-navigation.jssrc/UILayer/web/coverage/lcov-report/index.htmlsrc/UILayer/web/coverage/lcov-report/prettify.csssrc/UILayer/web/coverage/lcov-report/prettify.jssrc/UILayer/web/coverage/lcov-report/sorter.jssrc/UILayer/web/coverage/lcov.infosrc/UILayer/web/package.json
✅ Files skipped from review due to trivial changes (4)
- src/UILayer/web/coverage/lcov-report/CognitiveMeshButton.tsx.html
- src/UILayer/web/coverage/lcov-report/index.html
- src/UILayer/web/coverage/lcov.info
- src/UILayer/web/coverage/coverage-final.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/BusinessApplications/AdaptiveBalance/Controllers/AdaptiveBalanceController.cs
- src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs
src/UILayer/web/coverage/clover.xml
Outdated
Comment on lines
+1
to
+15
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <coverage generated="1773186088062" clover="3.2.0"> | ||
| <project timestamp="1773186088062" name="All files"> | ||
| <metrics statements="6" coveredstatements="6" conditionals="6" coveredconditionals="5" methods="1" coveredmethods="1" elements="13" coveredelements="12" complexity="0" loc="6" ncloc="6" packages="1" files="1" classes="1"/> | ||
| <file name="CognitiveMeshButton.tsx" path="C:\Users\smitj\repos\cognitive-mesh\src\UILayer\web\src\components\CognitiveMeshButton\CognitiveMeshButton.tsx"> | ||
| <metrics statements="6" coveredstatements="6" conditionals="6" coveredconditionals="5" methods="1" coveredmethods="1"/> | ||
| <line num="13" count="1" type="stmt"/> | ||
| <line num="21" count="1" type="stmt"/> | ||
| <line num="22" count="1" type="stmt"/> | ||
| <line num="23" count="1" type="stmt"/> | ||
| <line num="24" count="1" type="cond" truecount="1" falsecount="1"/> | ||
| <line num="26" count="1" type="stmt"/> | ||
| </file> | ||
| </project> | ||
| </coverage> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if coverage is already in .gitignore
echo "=== Checking .gitignore for coverage patterns ==="
rg -n 'coverage' .gitignore 2>/dev/null || echo ".gitignore not found or no coverage patterns"
# List all coverage files in the repository
echo -e "\n=== Coverage files in repository ==="
fd -t f . -p 'coverage/' 2>/dev/null | head -20Repository: phoenixvc/cognitive-mesh
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Check if the specific file exists
echo "=== Checking if clover.xml exists at src/UILayer/web/coverage/clover.xml ==="
if [ -f "src/UILayer/web/coverage/clover.xml" ]; then
echo "File EXISTS"
head -10 src/UILayer/web/coverage/clover.xml
else
echo "File NOT FOUND at that location"
fi
# Check .gitignore existence and content
echo -e "\n=== .gitignore content (if exists) ==="
if [ -f ".gitignore" ]; then
cat -n .gitignore
else
echo ".gitignore does not exist"
fi
# Search for any coverage files more broadly
echo -e "\n=== Searching for any coverage-related files ==="
find . -type f -name "*coverage*" -o -type d -name "coverage" 2>/dev/null | grep -v node_modules | head -20Repository: phoenixvc/cognitive-mesh
Length of output: 3078
Remove coverage reports from version control.
This coverage/ directory (including clover.xml and coverage-final.json) contains generated test artifacts that should not be committed to the repository:
- PII leakage: The
clover.xmlfile contains an absolute path with a developer's Windows username (smitj):C:\Users\smitj\repos\cognitive-mesh\.... This violates privacy best practices. - Repository hygiene: Coverage reports are generated on each test run and will cause unnecessary diffs, bloat history, and merge conflicts.
The .gitignore file does not currently exclude the coverage/ directory. Add this exclusion and remove the coverage directory from the commit.
Suggested .gitignore addition
+ # Test coverage
+ coverage/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UILayer/web/coverage/clover.xml` around lines 1 - 15, Remove generated
coverage artifacts and stop tracking them: delete the committed coverage files
(e.g., src/UILayer/web/coverage/clover.xml and coverage-final.json), add an
entry to .gitignore to exclude the coverage/ directory (or specifically
coverage/ and **/coverage/), and run git rm --cached on any tracked coverage
files so they are untracked going forward; ensure the clover.xml path/PII
(CognitiveMeshButton.tsx absolute path) is no longer present in the repo history
for new commits.
Comment on lines
+24
to
+38
| function toggleClass(index) { | ||
| missingCoverageElements | ||
| .item(currentIndex) | ||
| .classList.remove('highlighted'); | ||
| missingCoverageElements.item(index).classList.add('highlighted'); | ||
| } | ||
|
|
||
| function makeCurrent(index) { | ||
| toggleClass(index); | ||
| currentIndex = index; | ||
| missingCoverageElements.item(index).scrollIntoView({ | ||
| behavior: 'smooth', | ||
| block: 'center', | ||
| inline: 'center' | ||
| }); |
Contributor
There was a problem hiding this comment.
Handle pages with no navigation targets.
Line 20 can legitimately produce an empty NodeList on fully covered reports. In that case, the first n/j/b/k/p keypress dereferences null here via classList/scrollIntoView instead of becoming a no-op.
Minimal guard
function toggleClass(index) {
- missingCoverageElements
- .item(currentIndex)
- .classList.remove('highlighted');
+ if (typeof currentIndex === 'number') {
+ missingCoverageElements
+ .item(currentIndex)
+ .classList.remove('highlighted');
+ }
missingCoverageElements.item(index).classList.add('highlighted');
}
function makeCurrent(index) {
+ if (missingCoverageElements.length === 0) {
+ return;
+ }
toggleClass(index);
currentIndex = index;
missingCoverageElements.item(index).scrollIntoView({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UILayer/web/coverage/lcov-report/block-navigation.js` around lines 24 -
38, The functions toggleClass and makeCurrent assume missingCoverageElements
contains nodes and directly access .item(...).classList and
.item(...).scrollIntoView, which throws when the NodeList is empty; update both
toggleClass and makeCurrent to first check that missingCoverageElements &&
missingCoverageElements.length > 0 and that missingCoverageElements.item(index)
is not null (and that currentIndex is within range) and simply return/no-op if
there are no elements or the requested index is out of bounds; keep the rest of
the behavior (updating currentIndex, adding/removing 'highlighted', and calling
scrollIntoView) only after those guards to prevent dereferencing null on fully
covered reports.
- AGENT_BACKLOG.md: fix item count (29→27), fix circular gate deferral - AgentController: propagate CancellationToken to all registry port calls - IAgentRegistryPort: add CancellationToken to Register/GetById/Deactivate - AgentRegistryService: sanitize framework in compliance status log - CustomerIntelligenceManager: sanitize customerId in exception message, escape single quotes in Cypher query to prevent injection - NISTComplianceService: sanitize audit entry Details fields, lock EvidenceRecord mutation for thread safety, capture TotalCount inside lock for consistent snapshot - AdaptiveBalanceService: snapshot ConcurrentBag for confidence calc, lock DimensionState reads/writes for atomic updates - Agents page: remove role="grid" (no 2D nav), add focus-visible ring - Remove coverage/ artifacts from git, add to .gitignore Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs (2)
533-547:⚠️ Potential issue | 🟠 MajorPropagate the new
CancellationTokeninstead of dropping it here.These bridge methods now accept a token, but none of them use it:
RegisterAgentAsync(definition)andGetAgentByIdAsync(agentId)discard it, andFindAsync/SaveChangesAsyncare called without it. Callers can cancel the HTTP request, but the DB work still runs to completion.Also applies to: 551-555, 573-582
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs` around lines 533 - 547, The bridge method IAgentRegistryPort.RegisterAgentAsync is ignoring the provided CancellationToken—change the call to RegisterAgentAsync to pass the token (RegisterAgentAsync(definition, cancellationToken)) and then update the underlying methods it calls (e.g., RegisterAgentAsync, GetAgentByIdAsync, repository.FindAsync, and repository.SaveChangesAsync) to accept and forward the CancellationToken so DB operations and FindAsync/SaveChangesAsync use the token; ensure signatures and all internal calls (e.g., in RegisterAgentAsync, GetAgentByIdAsync, FindAsync, SaveChangesAsync) are updated to accept CancellationToken and that cancellationToken is threaded through each call.
602-611:⚠️ Potential issue | 🟠 MajorDon't return unfiltered data from a compliance-filtered API.
Logging that
frameworkandisCompliantare unsupported does not make the fallback safe: returning all active agents will populate compliant and non-compliant views with incorrect data. Until the backing store exists, fail fast or return an empty result instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs` around lines 602 - 611, The method IAgentRegistryPort.GetAgentsByComplianceStatusAsync currently logs a warning and returns all active agents via the AgentDefinitions query and MapToPortAgent, which is unsafe; change it to fail fast by throwing a NotSupportedException (or NotImplementedException) with a clear message that compliance filtering is not available (instead of calling _logger.LogWarning and returning definitions.Select(...)), or alternatively return an empty IEnumerable<Ports.Models.Agent> (Enumerable.Empty<Ports.Models.Agent>()) if you prefer a silent no-data response; update the implementation in IAgentRegistryPort.GetAgentsByComplianceStatusAsync (remove the fallback to the AgentDefinitions query and MapToPortAgent) so callers do not receive unfiltered data.src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs (2)
124-136:⚠️ Potential issue | 🟠 MajorThe 404 path is unreachable; missing agents will return 500.
_registryPort.GetAgentByIdAsync(...)is currently implemented byAgentRegistryServiceto throwAgentNotFoundExceptionon a miss, soif (agent == null)never runs. The generic catch then converts a normal not-found intoSERVICE_UNAVAILABLE. CatchAgentNotFoundExceptionhere or make the port consistently returnnull.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 124 - 136, The 404 branch in AgentController is never hit because _registryPort.GetAgentByIdAsync throws AgentNotFoundException instead of returning null; update AgentController to explicitly catch AgentNotFoundException from _registryPort.GetAgentByIdAsync (inside the try block) and in that catch return NotFound(ErrorEnvelope.Create("AGENT_NOT_FOUND", $"Agent with ID '{agentId}' was not found.")); keep the existing generic catch for Exception to return SERVICE_UNAVAILABLE so normal errors still yield 500; alternatively, if you prefer changing the port contract, make AgentRegistryService return null from GetAgentByIdAsync and adjust AgentController accordingly, but prefer adding the specific catch for AgentNotFoundException in AgentController for minimal change.
81-98:⚠️ Potential issue | 🟠 MajorDon't fire-and-forget scoped adapters with
Task.Runfrom the request path.
_auditand_notifyare injected request-time dependencies, and these delegates can outlive the HTTP scope. Once the request completes, they can start failing on disposed services/DbContexts and silently drop audit/notification work. Please hand this off to a background queue/hosted service that creates its own scope.Also applies to: 161-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around lines 81 - 98, Replace the fire-and-forget Task.Run blocks that call _audit.LogAgentRegisteredAsync and _notify.SendAgentRegistrationNotificationAsync with enqueuing a work item to a background queue/hosted service (e.g., IBackgroundTaskQueue or a Channel) so the background worker can create its own IServiceScope and resolve scoped dependencies; specifically, stop calling Task.Run with injected request-scoped _audit/_notify, instead create a lightweight DTO containing agent.AgentId, agent.AgentType, actorName and enqueue that to the background processor which will call LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync within a created scope and handle/log exceptions; apply the same change for the similar Task.Run usages around lines 161-167.src/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cs (1)
384-416:⚠️ Potential issue | 🟠 MajorFinish the cancellation-token rollout on
IAgentRegistryPort.This patch updates only three async members, but
UpdateAgentAsync,GetAgentsByTypeAsync,GetAgentsByComplianceStatusAsync,SearchAgentsAsync,VerifyAgentComplianceAsync,GetAgentComplianceHistoryAsync, andRequestComplianceCertificationAsyncare still public async methods without aCancellationToken. That leaves request cancellation inconsistent across the port surface. As per coding guidelines, "All public methods must be async withCancellationTokenparameter".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cs` around lines 384 - 416, The interface is missing CancellationToken parameters on several public async methods; update the signatures of UpdateAgentAsync, GetAgentsByTypeAsync, GetAgentsByComplianceStatusAsync, SearchAgentsAsync, VerifyAgentComplianceAsync, GetAgentComplianceHistoryAsync, and RequestComplianceCertificationAsync to accept an optional CancellationToken cancellationToken = default, adjust their XML docs accordingly, and then propagate the new parameter to all implementing classes and callers (ensure defaulting to CancellationToken.None or = default where appropriate) so cancellation is consistent across IAgentRegistryPort.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENT_BACKLOG.md`:
- Around line 288-300: The final summary row label "**Total remaining**" is
misleading; update that table row in AGENT_BACKLOG.md (the row with "**Total
remaining | **49** | **12** | **37**") to a correct label such as "**Total**" or
"**Overall**" so it reflects the overall rollup values (49 / 12 / 37); ensure
the bold formatting and column values remain unchanged.
- Line 3: Update the top header sentence that currently says "Phase 14 in
progress" to mark Phase 14 as complete so it matches the detailed section;
specifically edit the header text that mentions "Phase 14 in progress (6/6 core
items done)" and change it to indicate Phase 14 is complete (e.g., "Phase 14
complete (6/6 core items done)"), ensuring consistency with the dedicated Phase
14 section below.
In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs`:
- Around line 188-204: The current try/catch around
_orchestrationPort.ExecuteTaskAsync in the AgentController action swallows
OperationCanceledException and maps cancellations to a 500; add a specific catch
for OperationCanceledException (or TaskCanceledException) that rethrows (or lets
the exception propagate) before the generic catch so client
cancellations/timeouts are not translated into SERVICE_UNAVAILABLE; locate the
block containing ExecuteTaskAsync and replace the single generic catch with a
two-tiered catch: catch (OperationCanceledException) { throw; } followed by
catch (Exception ex) { ... } so cancellation escapes.
In `@src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs`:
- Around line 551-555: The port methods accept tenantId but ignore it, causing
cross-tenant access; update IAgentRegistryPort.GetAgentByIdAsync and
DeactivateAgentAsync (and the other affected block around 573-583) to enforce
tenant scoping by loading agents using both agentId and tenantId (or by checking
definition.TenantId == tenantId after load) and refusing or returning null if
the tenant does not match, and ensure any mutation (e.g., marking retired in
DeactivateAgentAsync) only proceeds when the loaded definition belongs to the
caller's tenant; adjust calls to GetAgentByIdAsync/MapToPortAgent or add a
GetAgentByIdForTenantAsync helper to centralize this check.
In
`@src/BusinessApplications/CustomerIntelligence/CustomerIntelligenceManager.cs`:
- Around line 71-74: The code in CustomerIntelligenceManager builds a Cypher
string with interpolated safeCustomerId but only escapes single quotes, allowing
backslash-based injection; change the call to use a parameterized query instead
of string interpolation by updating IKnowledgeGraphManager.QueryAsync to accept
query parameters (e.g., a parameter dictionary) and then call it from
CustomerIntelligenceManager with a query like "MATCH (c:Customer {id:
$customerId})- [r]->(s:Segment) RETURN s" passing customerId as a parameter; if
you must keep string escaping, first replace backslashes then single quotes
(escape '\' before '\''), but preferred fix is to add parameter support to
QueryAsync and replace the interpolated call to use that parameter.
In `@src/BusinessApplications/NISTCompliance/Services/NISTComplianceService.cs`:
- Around line 223-229: Normalize and validate request.Decision before
persisting: inside the lock(record) block in NISTComplianceService, map
request.Decision to a canonical set of allowed statuses (e.g., treat any
case-insensitive "approved" as exactly "Approved", similarly normalize other
known statuses) and reject or default unknown values rather than storing
arbitrary strings; set record.ReviewStatus to the canonical value (not the raw
request.Decision), then assign record.ReviewedBy, record.ReviewedAt, and
record.ReviewNotes as before; if an invalid decision is encountered, either
throw a validation exception or set a safe default status to avoid breaking
checklist/scoring logic.
---
Outside diff comments:
In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs`:
- Around line 124-136: The 404 branch in AgentController is never hit because
_registryPort.GetAgentByIdAsync throws AgentNotFoundException instead of
returning null; update AgentController to explicitly catch
AgentNotFoundException from _registryPort.GetAgentByIdAsync (inside the try
block) and in that catch return NotFound(ErrorEnvelope.Create("AGENT_NOT_FOUND",
$"Agent with ID '{agentId}' was not found.")); keep the existing generic catch
for Exception to return SERVICE_UNAVAILABLE so normal errors still yield 500;
alternatively, if you prefer changing the port contract, make
AgentRegistryService return null from GetAgentByIdAsync and adjust
AgentController accordingly, but prefer adding the specific catch for
AgentNotFoundException in AgentController for minimal change.
- Around line 81-98: Replace the fire-and-forget Task.Run blocks that call
_audit.LogAgentRegisteredAsync and
_notify.SendAgentRegistrationNotificationAsync with enqueuing a work item to a
background queue/hosted service (e.g., IBackgroundTaskQueue or a Channel) so the
background worker can create its own IServiceScope and resolve scoped
dependencies; specifically, stop calling Task.Run with injected request-scoped
_audit/_notify, instead create a lightweight DTO containing agent.AgentId,
agent.AgentType, actorName and enqueue that to the background processor which
will call LogAgentRegisteredAsync and SendAgentRegistrationNotificationAsync
within a created scope and handle/log exceptions; apply the same change for the
similar Task.Run usages around lines 161-167.
In `@src/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cs`:
- Around line 384-416: The interface is missing CancellationToken parameters on
several public async methods; update the signatures of UpdateAgentAsync,
GetAgentsByTypeAsync, GetAgentsByComplianceStatusAsync, SearchAgentsAsync,
VerifyAgentComplianceAsync, GetAgentComplianceHistoryAsync, and
RequestComplianceCertificationAsync to accept an optional CancellationToken
cancellationToken = default, adjust their XML docs accordingly, and then
propagate the new parameter to all implementing classes and callers (ensure
defaulting to CancellationToken.None or = default where appropriate) so
cancellation is consistent across IAgentRegistryPort.
In `@src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs`:
- Around line 533-547: The bridge method IAgentRegistryPort.RegisterAgentAsync
is ignoring the provided CancellationToken—change the call to RegisterAgentAsync
to pass the token (RegisterAgentAsync(definition, cancellationToken)) and then
update the underlying methods it calls (e.g., RegisterAgentAsync,
GetAgentByIdAsync, repository.FindAsync, and repository.SaveChangesAsync) to
accept and forward the CancellationToken so DB operations and
FindAsync/SaveChangesAsync use the token; ensure signatures and all internal
calls (e.g., in RegisterAgentAsync, GetAgentByIdAsync, FindAsync,
SaveChangesAsync) are updated to accept CancellationToken and that
cancellationToken is threaded through each call.
- Around line 602-611: The method
IAgentRegistryPort.GetAgentsByComplianceStatusAsync currently logs a warning and
returns all active agents via the AgentDefinitions query and MapToPortAgent,
which is unsafe; change it to fail fast by throwing a NotSupportedException (or
NotImplementedException) with a clear message that compliance filtering is not
available (instead of calling _logger.LogWarning and returning
definitions.Select(...)), or alternatively return an empty
IEnumerable<Ports.Models.Agent> (Enumerable.Empty<Ports.Models.Agent>()) if you
prefer a silent no-data response; update the implementation in
IAgentRegistryPort.GetAgentsByComplianceStatusAsync (remove the fallback to the
AgentDefinitions query and MapToPortAgent) so callers do not receive unfiltered
data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87a31b3a-54bc-4c78-b89a-0348df3b5c16
📒 Files selected for processing (9)
AGENT_BACKLOG.mdsrc/BusinessApplications/AdaptiveBalance/Services/AdaptiveBalanceService.cssrc/BusinessApplications/AgentRegistry/Controllers/AgentController.cssrc/BusinessApplications/AgentRegistry/Ports/IAgentRegistryPort.cssrc/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cssrc/BusinessApplications/CustomerIntelligence/CustomerIntelligenceManager.cssrc/BusinessApplications/NISTCompliance/Services/NISTComplianceService.cssrc/UILayer/web/.gitignoresrc/UILayer/web/src/app/(app)/agents/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/UILayer/web/src/app/(app)/agents/page.tsx
| # Cognitive Mesh — Agent Backlog | ||
|
|
||
| > Prioritized, actionable work items. Backend is 100% complete (70/70 items). Frontend Phase 13 complete (4/4). Remaining 35 frontend items + 6 DevOps evaluation tickets across Phases 14–18. | ||
| > Prioritized, actionable work items. Backend is 100% complete (70/70 items). Frontend Phase 13 complete (4/4), Phase 14 in progress (6/6 core items done). Remaining 27 frontend items + 4 Phase 14b items + 6 DevOps evaluation tickets across Phases 14b–18. |
Contributor
There was a problem hiding this comment.
Mark Phase 14 as complete in the header.
Line 3 still says Phase 14 is "in progress" even though the same sentence says all 6/6 core items are done and the dedicated Phase 14 section below is marked complete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENT_BACKLOG.md` at line 3, Update the top header sentence that currently
says "Phase 14 in progress" to mark Phase 14 as complete so it matches the
detailed section; specifically edit the header text that mentions "Phase 14 in
progress (6/6 core items done)" and change it to indicate Phase 14 is complete
(e.g., "Phase 14 complete (6/6 core items done)"), ensuring consistency with the
dedicated Phase 14 section below.
Comment on lines
288
to
+300
| | Priority | Total | Done | Remaining | | ||
| |----------|-------|------|-----------| | ||
| | P0-CRITICAL (frontend) | 4 | 4 | **0** | | ||
| | P1-HIGH (frontend infra) | 6 | 0 | **6** | | ||
| | P1-HIGH (frontend infra) | 6 | 6 | **0** | | ||
| | P1-HIGH (UI library — 14b) | 4 | 0 | **4** | | ||
| | P1-HIGH (widget PRDs) | 5 | 0 | **5** | | ||
| | P2-MEDIUM (widgets + nav) | 8 | 0 | **8** | | ||
| | P2-MEDIUM (widgets + nav) | 8 | 2 | **6** | | ||
| | P2-MEDIUM (security) | 1 | 0 | **1** | | ||
| | P2-MEDIUM (CI/CD) | 5 | 0 | **5** | | ||
| | P2-MEDIUM (testing) | 5 | 0 | **5** | | ||
| | P3-LOW (features) | 5 | 0 | **5** | | ||
| | DEVOPS (evaluation) | 6 | 0 | **6** | | ||
| | **Total remaining** | **45** | **4** | **41** | | ||
| | **Total remaining** | **49** | **12** | **37** | |
Contributor
There was a problem hiding this comment.
Rename the final summary row.
49 / 12 / 37 is the overall rollup, not the "remaining" rollup. A label like Total or Overall would match the numbers shown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENT_BACKLOG.md` around lines 288 - 300, The final summary row label
"**Total remaining**" is misleading; update that table row in AGENT_BACKLOG.md
(the row with "**Total remaining | **49** | **12** | **37**") to a correct label
such as "**Total**" or "**Overall**" so it reflects the overall rollup values
(49 / 12 / 37); ensure the bold formatting and column values remain unchanged.
Comment on lines
+188
to
+204
| try | ||
| { | ||
| var (tenantId, userId) = GetAuthContextFromClaims(); | ||
| if (tenantId == null) return Unauthorized(ErrorEnvelope.Create("UNAUTHORIZED", "Tenant ID is missing.")); | ||
|
|
||
| request.TenantId = tenantId; | ||
| request.RequestingUserId = userId ?? string.Empty; | ||
| request.TenantId = tenantId; | ||
| request.RequestingUserId = userId ?? string.Empty; | ||
|
|
||
| var response = await _orchestrationPort.ExecuteTaskAsync(request); | ||
| return Ok(response); | ||
| var response = await _orchestrationPort.ExecuteTaskAsync(request, cancellationToken); | ||
| return Ok(response); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error executing orchestrated task."); | ||
| return StatusCode(StatusCodes.Status500InternalServerError, | ||
| ErrorEnvelope.Create("SERVICE_UNAVAILABLE", "Orchestration service is unavailable.")); | ||
| } |
Contributor
There was a problem hiding this comment.
Let request cancellation escape instead of mapping it to 500.
This action now passes a CancellationToken, but catch (Exception) will also catch OperationCanceledException and return SERVICE_UNAVAILABLE. Client disconnects/timeouts will look like server faults.
Suggested fix
try
{
var (tenantId, userId) = GetAuthContextFromClaims();
if (tenantId == null) return Unauthorized(ErrorEnvelope.Create("UNAUTHORIZED", "Tenant ID is missing."));
request.TenantId = tenantId;
request.RequestingUserId = userId ?? string.Empty;
var response = await _orchestrationPort.ExecuteTaskAsync(request, cancellationToken);
return Ok(response);
}
+ catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+ {
+ throw;
+ }
catch (Exception ex)
{
_logger.LogError(ex, "Error executing orchestrated task.");
return StatusCode(StatusCodes.Status500InternalServerError,
ErrorEnvelope.Create("SERVICE_UNAVAILABLE", "Orchestration service is unavailable."));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BusinessApplications/AgentRegistry/Controllers/AgentController.cs` around
lines 188 - 204, The current try/catch around
_orchestrationPort.ExecuteTaskAsync in the AgentController action swallows
OperationCanceledException and maps cancellations to a 500; add a specific catch
for OperationCanceledException (or TaskCanceledException) that rethrows (or lets
the exception propagate) before the generic catch so client
cancellations/timeouts are not translated into SERVICE_UNAVAILABLE; locate the
block containing ExecuteTaskAsync and replace the single generic catch with a
two-tiered catch: catch (OperationCanceledException) { throw; } followed by
catch (Exception ex) { ... } so cancellation escapes.
Comment on lines
+551
to
555
| async Task<Ports.Models.Agent> IAgentRegistryPort.GetAgentByIdAsync(Guid agentId, string tenantId, CancellationToken cancellationToken) | ||
| { | ||
| var definition = await _dbContext.AgentDefinitions.FindAsync(agentId); | ||
| if (definition == null) | ||
| { | ||
| throw new KeyNotFoundException($"Agent with ID '{agentId}' was not found."); | ||
| } | ||
|
|
||
| var definition = await GetAgentByIdAsync(agentId); | ||
| return MapToPortAgent(definition, tenantId); | ||
| } |
Contributor
There was a problem hiding this comment.
Enforce tenant scoping before reading or mutating agents.
Both port methods accept tenantId and then ignore it. GetAgentByIdAsync loads by agentId only and re-labels the result with the caller's tenant, while DeactivateAgentAsync can retire any matching record regardless of owner. That's a cross-tenant isolation hole.
Also applies to: 573-583
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BusinessApplications/AgentRegistry/Services/AgentRegistryService.cs`
around lines 551 - 555, The port methods accept tenantId but ignore it, causing
cross-tenant access; update IAgentRegistryPort.GetAgentByIdAsync and
DeactivateAgentAsync (and the other affected block around 573-583) to enforce
tenant scoping by loading agents using both agentId and tenantId (or by checking
definition.TenantId == tenantId after load) and refusing or returning null if
the tenant does not match, and ensure any mutation (e.g., marking retired in
DeactivateAgentAsync) only proceeds when the loaded definition belongs to the
caller's tenant; adjust calls to GetAgentByIdAsync/MapToPortAgent or add a
GetAgentByIdForTenantAsync helper to centralize this check.
Comment on lines
+71
to
+74
| // Escape single quotes to prevent Cypher injection | ||
| var safeCustomerId = customerId.Replace("'", "\\'", StringComparison.Ordinal); | ||
| var relationships = await _knowledgeGraphManager.QueryAsync( | ||
| $"MATCH (c:Customer {{id: '{customerId}'}})-[r]->(s:Segment) RETURN s", | ||
| $"MATCH (c:Customer {{id: '{safeCustomerId}'}})-[r]->(s:Segment) RETURN s", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/BusinessApplications/CustomerIntelligence/CustomerIntelligenceManager.cs | head -100Repository: phoenixvc/cognitive-mesh
Length of output: 5632
🏁 Script executed:
find . -name "CustomerIntelligenceManager.cs" -type fRepository: phoenixvc/cognitive-mesh
Length of output: 146
🏁 Script executed:
find . -name "*KnowledgeGraphManager*" -type fRepository: phoenixvc/cognitive-mesh
Length of output: 179
🏁 Script executed:
cat -n ./src/Shared/Interfaces/IKnowledgeGraphManager.csRepository: phoenixvc/cognitive-mesh
Length of output: 2553
🏁 Script executed:
cat -n ./src/FoundationLayer/KnowledgeGraph/KnowledgeGraphManager.csRepository: phoenixvc/cognitive-mesh
Length of output: 17933
🌐 Web query:
Neo4j Cypher query parameters best practices injection prevention
💡 Result:
Best practices for Neo4j Cypher parameters (and preventing Cypher injection)
-
Always parameterize user-supplied values (strings, numbers, lists, maps, IDs) instead of building Cypher with string concatenation. This prevents user input from changing the query structure. [1]
MATCH (p:Person) WHERE p.email = $email RETURN p
-
Use the official driver parameter APIs (e.g.,
session.run(query, params)), not ad-hoc string interpolation. This is explicitly recommended to avoid malicious code injection. [5] -
Know what cannot be parameterized (query structure) and avoid stitching those parts from user input:
- Traditionally: property keys, labels, relationship types are part of the compiled plan and not parameterizable in the basic form (
MATCH (n:$param)is invalid). [2] - Neo4j 5.26 introduced supported ways to reference dynamic labels and relationship types via parameters (use the documented “dynamic labels/types” features rather than concatenation). [2]
- Traditionally: property keys, labels, relationship types are part of the compiled plan and not parameterizable in the basic form (
-
If you must build dynamic Cypher (e.g., dynamic label/type/key):
- Prefer data-model changes that move variability into properties (so it becomes parameterizable). [1]
- Otherwise, enforce strict allow-lists (e.g., only permit labels from a fixed set) and reject everything else. [1]
-
Be extra careful with APOC procedures that execute Cypher (e.g., procedures that take a Cypher string and run it). They may build/execute derived queries and can reduce safety guarantees if you pass untrusted strings—treat them as higher risk and keep input constrained/allow-listed. [3], [4]
-
Reduce blast radius via configuration/permissions:
- Lock down file system–related capabilities (APOC import/export/load and
LOAD CSV) unless required; keep Neo4j/APOC file access restricted to necessary directories. [3]
- Lock down file system–related capabilities (APOC import/export/load and
Sources
[1] Neo4j KB: Protecting against Cypher Injection — https://neo4j.com/developer/kb/protecting-against-cypher-injection/
[2] Neo4j Cypher Manual: Parameters — https://neo4j.com/docs/cypher-manual/5/syntax/parameters/
[3] APOC Core Docs: Security Guidelines — https://neo4j.com/docs/apoc/current/security-guidelines/
[4] APOC (Labs) Docs: Security Guidelines — https://neo4j.com/labs/apoc/5/security-guidelines/
[5] Neo4j GraphAcademy: Executing Cypher statements (drivers) — https://graphacademy.neo4j.com/courses/drivers-java/1-driver/3-execute-query/
Cypher injection vulnerability: insufficient escaping allows bypass via backslash injection.
The current escaping only replaces single quotes (') but not backslashes (\). Since Cypher treats \ as an escape character, an input containing backslashes can still break the string literal after parsing. For example, an ID like test\' would become test\\' after escaping, but the backslash escapes the following quote during Cypher interpretation.
The immediate mitigation is to escape backslashes before single quotes:
Minimal hardening
- var safeCustomerId = customerId.Replace("'", "\\'", StringComparison.Ordinal);
+ var safeCustomerId = customerId
+ .Replace("\\", "\\\\", StringComparison.Ordinal)
+ .Replace("'", "\\'", StringComparison.Ordinal);However, Neo4j strongly recommends parameterized queries over string escaping. Refactor IKnowledgeGraphManager.QueryAsync to accept query parameters so callers do not need to hand-roll escaping.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Escape single quotes to prevent Cypher injection | |
| var safeCustomerId = customerId.Replace("'", "\\'", StringComparison.Ordinal); | |
| var relationships = await _knowledgeGraphManager.QueryAsync( | |
| $"MATCH (c:Customer {{id: '{customerId}'}})-[r]->(s:Segment) RETURN s", | |
| $"MATCH (c:Customer {{id: '{safeCustomerId}'}})-[r]->(s:Segment) RETURN s", | |
| // Escape single quotes to prevent Cypher injection | |
| var safeCustomerId = customerId | |
| .Replace("\\", "\\\\", StringComparison.Ordinal) | |
| .Replace("'", "\\'", StringComparison.Ordinal); | |
| var relationships = await _knowledgeGraphManager.QueryAsync( | |
| $"MATCH (c:Customer {{id: '{safeCustomerId}'}})-[r]->(s:Segment) RETURN s", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BusinessApplications/CustomerIntelligence/CustomerIntelligenceManager.cs`
around lines 71 - 74, The code in CustomerIntelligenceManager builds a Cypher
string with interpolated safeCustomerId but only escapes single quotes, allowing
backslash-based injection; change the call to use a parameterized query instead
of string interpolation by updating IKnowledgeGraphManager.QueryAsync to accept
query parameters (e.g., a parameter dictionary) and then call it from
CustomerIntelligenceManager with a query like "MATCH (c:Customer {id:
$customerId})- [r]->(s:Segment) RETURN s" passing customerId as a parameter; if
you must keep string escaping, first replace backslashes then single quotes
(escape '\' before '\''), but preferred fix is to add parameter support to
QueryAsync and replace the interpolated call to use that parameter.
Comment on lines
+223
to
+229
| lock (record) | ||
| { | ||
| record.ReviewStatus = request.Decision; | ||
| record.ReviewedBy = request.ReviewerId; | ||
| record.ReviewedAt = now; | ||
| record.ReviewNotes = request.Notes; | ||
| } |
Contributor
There was a problem hiding this comment.
Validate and normalize request.Decision before persisting it.
This method stores any string into ReviewStatus, but the rest of this service only treats the exact value "Approved" as approved. A casing difference or unexpected value here will silently drop the evidence out of checklist completion, scoring, and roadmap calculations.
💡 Proposed fix
var now = DateTimeOffset.UtcNow;
+ var decision = request.Decision?.Trim();
+ if (decision is not ("Pending" or "Approved" or "Rejected"))
+ {
+ throw new ArgumentOutOfRangeException(
+ nameof(request.Decision),
+ "Decision must be one of: Pending, Approved, Rejected.");
+ }
+
lock (record)
{
- record.ReviewStatus = request.Decision;
+ record.ReviewStatus = decision;
record.ReviewedBy = request.ReviewerId;
record.ReviewedAt = now;
record.ReviewNotes = request.Notes;
}
@@
- NewStatus = request.Decision,
+ NewStatus = decision,
ReviewedAt = now,
- Message = $"Evidence review completed with decision '{request.Decision}'."
+ Message = $"Evidence review completed with decision '{decision}'."
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BusinessApplications/NISTCompliance/Services/NISTComplianceService.cs`
around lines 223 - 229, Normalize and validate request.Decision before
persisting: inside the lock(record) block in NISTComplianceService, map
request.Decision to a canonical set of allowed statuses (e.g., treat any
case-insensitive "approved" as exactly "Approved", similarly normalize other
known statuses) and reject or default unknown values rather than storing
arbitrary strings; set record.ReviewStatus to the canonical value (not the raw
request.Decision), then assign record.ReviewedBy, record.ReviewedAt, and
record.ReviewNotes as before; if an invalid decision is encountered, either
throw a validation exception or set a safe default status to avoid breaking
checklist/scoring logic.
This was referenced Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
(app)route group — 6 routes (dashboard, agents, settings, analytics, compliance, marketplace) with loading/error boundariesuseSignalRhook) with exponential backoff reconnection/redirects to/dashboard,DashboardAPImock +useDashboardDatahook deleted, stores fetch from real backendDependencies added
zustand@5.0.11— state management@microsoft/signalr@10.0.0— real-time updatesBacklog updates
Test plan
npx tsc --noEmitpasses for modified files (pre-existing radix errors incomponents/ui/are known)/redirects to/dashboard🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements