feat(exceptions): add typed exception hierarchy by parthloglogn · Pull Request #1 · parthloglogn/Agents

parthloglogn · 2026-03-12T07:05:37Z

AgentException (base RuntimeException for full pipeline)
ScanException (moved from inner class in SecuritySupervisorAgentService)
McpToolNullResponseException (MCP returned null/empty — stops LLM loops)
McpToolExecutionException (MCP tool threw during execution)
MaxLlmCallsExceededException (loop guard circuit-breaker)
AgentRoutingException (supervisor cannot determine valid route)

Fixes: ScanException was an inner class — non-standard, untestable. All exceptions extend AgentException for single-catch supervisor handling.

- AgentException (base RuntimeException for full pipeline) - ScanException (moved from inner class in SecuritySupervisorAgentService) - McpToolNullResponseException (MCP returned null/empty — stops LLM loops) - McpToolExecutionException (MCP tool threw during execution) - MaxLlmCallsExceededException (loop guard circuit-breaker) - AgentRoutingException (supervisor cannot determine valid route) Fixes: ScanException was an inner class — non-standard, untestable. All exceptions extend AgentException for single-catch supervisor handling.

Copilot

Pull request overview

This PR introduces a typed exception hierarchy and supporting utilities to make the agent pipeline more robust (loop guards, MCP null-response circuit breakers), while also extracting a few service/tool contracts to enable easier unit testing and starting to scaffold multi-turn conversation context.

Changes:

Added AgentException hierarchy (e.g., ScanException, McpToolNullResponseException, MaxLlmCallsExceededException) and integrated loop guards/null-response validation into agents.
Added shared utilities (AgentUtils, McpResponseValidator) and a compact supervisor routing prompt to reduce token usage on subsequent routing calls.
Extracted interfaces for services/tool registries and added ConversationContext plumbing (history loaded from Redis and passed through state).

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
02-Cybersecurity/Vulnerability-Detection-Agent-Java/vulnerability-intel-mcp-server/src/main/java/com/agent/service/VulnerabilityService.java	Implements new service contract interface.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/vulnerability-intel-mcp-server/src/main/java/com/agent/service/IVulnerabilityService.java	New interface to support mocking/testing of vulnerability analysis service.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/util/McpResponseValidator.java	New validator to fail fast on null/empty MCP tool responses.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/util/AgentUtils.java	New shared utilities for MCP response extraction, markdown stripping, and empty checks.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/tool/ToolRegistry.java	New tool registry interface intended to enable mocking.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/tool/McpToolRegistry.java	Registry now implements `ToolRegistry` interface.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/service/SecuritySupervisorAgentService.java	Wires `ConversationContext` into initial state and externalizes `ScanException`.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/prompt/PromptLibrary.java	Adds compact supervisor routing prompt; refactors prompt docs.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/model/ConversationContext.java	New model to carry session history and build multi-message LLM inputs.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/ScanException.java	New top-level exception replacing former inner class.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/McpToolNullResponseException.java	New typed exception for MCP null/empty tool outputs.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/McpToolExecutionException.java	New typed exception for MCP tool execution failures.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/MaxLlmCallsExceededException.java	New typed exception for LLM loop guard circuit-breaker.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/AgentRoutingException.java	New typed exception for invalid supervisor routing outcomes.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/exception/AgentException.java	New base exception for agent pipeline failures.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/controller/ScanController.java	Updates import to new `ScanException` location.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/sub_agents/VulnerabilityAgent.java	Adds loop guard, early exits, shared utils, and null-response validation.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/sub_agents/GithubRepoAgent.java	Adds loop guard, shared utils, and MCP null-response validation.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/sub_agents/DirectAnswerAgent.java	Uses `ConversationContext` history to build multi-message prompts when available.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/sub_agents/DependencyParserAgent.java	Adds loop guard and shared utils; treats empty parser output as valid.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/SupervisorAgent.java	Adds supervisor iteration guard, compact routing prompt, and typed exception handling.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/github-repo-mcp-server/src/main/java/com/agent/service/impl/GithubRepoServiceImpl.java	Adds an additional repo service implementation (currently duplicates existing logic).
02-Cybersecurity/Vulnerability-Detection-Agent-Java/github-repo-mcp-server/src/main/java/com/agent/service/IGithubRepoService.java	Adds repo service interface (currently has naming/compilation issues).
02-Cybersecurity/Vulnerability-Detection-Agent-Java/github-repo-mcp-server/src/main/java/com/agent/service/GithubRepoService.java	Updates existing repo service to implement the new interface.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/dependency-parser-mcp-server/src/main/java/com/agent/service/IDependencyParserService.java	Adds dependency parser service interface.
02-Cybersecurity/Vulnerability-Detection-Agent-Java/dependency-parser-mcp-server/src/main/java/com/agent/service/DependencyParserService.java	Updates implementation to implement new interface.

Comments suppressed due to low confidence (1)

02-Cybersecurity/Vulnerability-Detection-Agent-Java/security-supervisor-agent/src/main/java/com/agent/agent/sub_agents/VulnerabilityAgent.java:103

tool() throws McpToolExecutionException, but invoke() catches it in the generic catch (Exception e) and converts it into a finalAnswer, so the typed exception won’t reach SupervisorAgent (despite being part of the new exception hierarchy). Either rethrow McpToolExecutionException here or don’t throw it from tool() and keep the existing string-based error propagation.


        } catch (McpToolNullResponseException | MaxLlmCallsExceededException e) {
            // Re-throw typed exceptions to SupervisorAgent for proper handling
            throw e;

        } catch (Exception e) {
            log.error("[VulnerabilityAgent] Unexpected error", e);
            Map<String, Object> err = new HashMap<>(incomingState);
            err.put("finalAnswer", "VulnerabilityAgent error: " + e.getMessage());
            return err;
        }
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T07:35:53Z

+    private ConversationContext buildConversationContext(String sessionId, String currentQuery) {
+        try {
+            java.util.List<String> rawHistory = sessionStore.getHistory(sessionId);


currentQuery is not used in buildConversationContext(...), which is misleading and will be flagged by IDEs/static analysis. Either remove the parameter or use it (e.g., to exclude the current turn from history, or to append it explicitly).

Copilot · 2026-03-12T07:35:53Z

+    /**
+     * A single chat turn: user query + agent response summary.
+     * Stored in Redis as JSON via SessionStore.saveChatEntry().
+     */
+    @Data
+    @AllArgsConstructor
+    public static class ChatMessage {
+
+        /** Original user query for this turn */
+        private String userQuery;
+
+        /**
+         * Compact summary of the agent's response for this turn.
+         * Used as the assistant message in history — not the full vulnerability report,
+         * just enough context for the LLM to understand what was done.
+         */
+        private String agentSummary;
+


This model (and its Javadoc) assumes Redis history entries contain an agentSummary field, but SessionStore.saveChatEntry() currently writes agentOutput. Unless those keys are aligned, agentSummary will always be null and assistant turns won’t be included when building message history. Consider renaming this field to agentOutput (or updating SessionStore/buildConversationContext to write/read agentSummary).

Copilot · 2026-03-12T07:35:53Z

+        } catch (McpToolNullResponseException | MaxLlmCallsExceededException e) {
+            throw e;
+
        } catch (Exception e) {
-            log.error("[GithubRepoAgent] Error", e);
+            log.error("[GithubRepoAgent] Unexpected error", e);
            Map<String, Object> err = new HashMap<>(incomingState);
            err.put("finalAnswer", "GithubRepoAgent error: " + e.getMessage());
            return err;


tool() throws McpToolExecutionException, but invoke() catches it in the generic catch (Exception e) and converts it into a finalAnswer, so the typed exception will never propagate to SupervisorAgent (and the toolName context is effectively lost). Either rethrow McpToolExecutionException here (alongside the other typed exceptions) or avoid wrapping/throwing it in tool() and keep the current finalAnswer path.

Copilot · 2026-03-12T07:35:54Z

+@Slf4j
+@Service
+public class GithubRepoServiceImpl implements IGithubRepoService {
+


This introduces a second @Service implementation of the repo service with effectively identical logic to com.agent.service.GithubRepoService. Once the IGithubRepoService interface is corrected/used for injection, Spring autowiring by that interface will become ambiguous (two beans). Consider keeping a single implementation (either move the existing class into impl and delete/convert the other, or remove this duplicate).

Copilot · 2026-03-12T07:35:54Z

+    /**
+     * Build ConversationContext from Redis session history.
+     * Each history entry is a JSON string of {userQuery, agentSummary, timestamp}.
+     * If history is empty or an error occurs, returns an empty context.
+     */
+    private ConversationContext buildConversationContext(String sessionId, String currentQuery) {
+        try {
+            java.util.List<String> rawHistory = sessionStore.getHistory(sessionId);
+            java.util.List<ConversationContext.ChatMessage> messages = new java.util.ArrayList<>();
+
+            for (String entry : rawHistory) {
+                try {
+                    com.fasterxml.jackson.databind.JsonNode node = objectMapper.readTree(entry);
+                    String query   = node.path("userQuery").asText(null);
+                    String summary = node.path("agentSummary").asText(null);
+                    String ts      = node.path("timestamp").asText(Instant.now().toString());
+                    if (query != null) {
+                        messages.add(new ConversationContext.ChatMessage(query, summary, ts));
+                    }


buildConversationContext() parses history entries using agentSummary, but SessionStore.saveChatEntry() persists the field as agentOutput. As a result, assistant history will always be null and ConversationContext.buildMessages() will only replay user turns. Align the JSON field names (e.g., read agentOutput here or write agentSummary in SessionStore) and update the method/Javadoc accordingly.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-03-12T07:35:54Z

+        ConversationContext ctx = (ConversationContext) state.data().get("conversationContext");

-        String prompt = PromptLibrary.DIRECT_ANSWER_AGENT_PROMPT.formatted(userQuery);
+        List<Message> messages = (ctx != null && ctx.hasHistory())
+                ? ctx.buildMessages(prompt)
+                : List.of(new UserMessage(prompt));


This unchecked cast can throw ClassCastException if the state contains a different type (or a serialized Map) under conversationContext. Prefer an instanceof ConversationContext guard and treat non-matching values as “no history”.

Copilot · 2026-03-12T07:35:54Z

+        } catch (MaxLlmCallsExceededException e) {
+            log.error("[Supervisor] LLM loop guard triggered — agent={} limit={}", e.getAgentName(), e.getLimit());
+            Map<String, Object> err = new HashMap<>(initialState);
+            err.put("finalAnswer", "Analysis stopped: agent '" + e.getAgentName() + "' exceeded " + e.getLimit() + " LLM calls.");
+            return err;
+
+        } catch (McpToolNullResponseException e) {
+            log.warn("[Supervisor] MCP null response — tool={}", e.getToolName());
+            Map<String, Object> err = new HashMap<>(initialState);
+            err.put("finalAnswer", "No data returned from tool '" + e.getToolName() + "'. The MCP server may be unavailable.");
+            return err;
+
+        } catch (AgentRoutingException e) {
+            log.error("[Supervisor] Routing failed — received='{}'", e.getReceivedRoute());
+            Map<String, Object> err = new HashMap<>(initialState);
+            err.put("finalAnswer", "Pipeline routing error: " + e.getMessage());
+            return err;
+


The new AgentException hierarchy includes McpToolExecutionException, but SupervisorAgent.invoke() doesn’t handle it (or AgentException) explicitly, so execution failures won’t get the intended typed handling/message. Consider adding a catch (AgentException e) (or at least McpToolExecutionException) to produce a consistent user-facing error and keep the “single-catch” goal described in the PR.

Copilot · 2026-03-12T07:35:55Z

+ * in unit tests. Each agent injects ToolRegistry rather than McpToolRegistry
+ * directly, so tests can substitute a stub without bringing up MCP servers.


This Javadoc claims “Each agent injects ToolRegistry rather than McpToolRegistry”, but the current agents still inject McpToolRegistry (see GithubRepoAgent/DependencyParserAgent/VulnerabilityAgent constructors). Update the comment or change the injection types so the documentation matches reality.

Suggested change

* in unit tests. Each agent injects ToolRegistry rather than McpToolRegistry

* directly, so tests can substitute a stub without bringing up MCP servers.

* in unit tests. Production agents may still inject McpToolRegistry

* directly, but tests can inject ToolRegistry implementations or stubs

* without bringing up MCP servers.

Copilot · 2026-03-12T07:35:55Z

+ * IDependencyParserService — contract for dependency file parsers.
+ *
+ * Extracted from the concrete class to enable mock injection in unit tests.
+ * Implementation: {@link impl.DependencyParserServiceImpl}


The Javadoc references impl.DependencyParserServiceImpl, but there is no such class in this module (the implementation is com.agent.service.DependencyParserService). Update the link to the correct implementation so IDE navigation and generated docs aren’t broken.

Suggested change

* Implementation: {@link impl.DependencyParserServiceImpl}

* Implementation: {@link DependencyParserService}

…lization; implement Serializable interface and improve dependency parsing logic

Copilot · 2026-03-12T09:11:08Z

@parthloglogn I've opened a new pull request, #2, to work on those changes. Once the pull request is ready, I'll request review from you.

parthloglogn requested a review from Copilot March 12, 2026 07:19

Copilot started reviewing on behalf of parthloglogn March 12, 2026 07:29 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

feat: Enhance ConversationContext and DependencyParserAgent for seria…

d646f5c

…lization; implement Serializable interface and improve dependency parsing logic

Copilot AI mentioned this pull request Mar 12, 2026

Fix: assistant history always null due to mismatched JSON field key in buildConversationContext #2

Open

parthloglogn merged commit 04f0e0a into main Mar 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(exceptions): add typed exception hierarchy#1

feat(exceptions): add typed exception hierarchy#1
parthloglogn merged 2 commits into
mainfrom
feature/agent-migration-improvements

parthloglogn commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

parthloglogn Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		* in unit tests. Each agent injects ToolRegistry rather than McpToolRegistry
		* directly, so tests can substitute a stub without bringing up MCP servers.

- * in unit tests. Each agent injects ToolRegistry rather than McpToolRegistry
- * directly, so tests can substitute a stub without bringing up MCP servers.
+ * in unit tests. Production agents may still inject McpToolRegistry
+ * directly, but tests can inject ToolRegistry implementations or stubs
+ * without bringing up MCP servers.

	* Implementation: {@link impl.DependencyParserServiceImpl}
	* Implementation: {@link DependencyParserService}

Conversation

parthloglogn commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

parthloglogn Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants