Repository navigation
Merge v4.0.0 into main - #416
Merged
Merged
Conversation
Implements Issue KDM-cli#271: Phase 0 of the Multi-Agent SRE Architecture. - Add frozen baseline fixtures for 4 canonical K8s failure modes (OOMKilled, CrashLoopBackOff, ImagePullBackOff, probe failure) - Define frozen boundary interfaces (AnalysisRequest, LegacyAnalysisResult, AnalysisEvent) in src/analysis/types.ts - Implement 6 offline baseline regression tests that run without live cluster or active AI server - Add KDM_MULTI_AGENT feature flag gate in runAnalysis() for incremental multi-agent development All 420 existing tests pass with zero regressions. Closes KDM-cli#271
…eline [KDM-CLI-FEAT-339] feat(core): Phase 0 - Baseline pipeline mapping and contract freeze
Implements a decorator-based ToolRegistry with Ollama-compatible JSON schema generation and strict least-privilege enforcement for Kubernetes and Docker inspection tools. ## Changes ### agents/tools/__init__.py - New package exposing ToolRegistry and the tool_registry singleton. ### agents/tools/registry.py - ToolRegistry class with @register decorator. - Auto-generates Ollama function schemas via inspect.signature. - Whitelist enforcement: raises ValueError for unregistered tool calls. - 10-second asyncio timeout per execution; returns error dict on expiry. - Handles both Python type objects and string annotations (Python 3.13+). ### agents/tools/kubernetes.py - get_pod_status(namespace, pod) -> phase + containerStatuses. - get_container_logs(namespace, pod, container, tail_lines, previous) -> bounded log lines (max 500). - get_pod_events(namespace, pod) -> Warning events sorted by lastTimestamp. - get_deployment_spec(namespace, deployment) -> replicas, selector, template. - RBAC/Forbidden errors surface as structured dicts. - All responses scrubbed by core.sanitizer.redact_sensitive_data. ### agents/tools/docker.py - get_docker_container_inspect(container_id) -> State, ExitCode, OOMKilled, RestartCount. - get_docker_logs(container_id, tail) -> bounded stdout/stderr lines (max 500). - All responses scrubbed by core.sanitizer.redact_sensitive_data. ### agents/tests/test_tools.py - 20 unit tests, all passing (pytest + pytest-asyncio). - Covers: whitelist enforcement, schema generation, type mapping, tail clamping, --previous flag, Warning event filtering/sorting, Docker state extraction, timeout behaviour, and a final check that no mutating tool names are registered. Closes KDM-cli#273
…safe-typed-tool-layer feat(agents): Phase 2 - Safe Typed Tool Layer & Least-Privilege Registry
Implements the normalized, immutable evidence layer as the single source of truth for all specialist agents and deterministic rules. Core Architectural Rule: KDM collects facts; agents reason over facts. Changes: - agents/core/__init__.py: Export core models - agents/core/evidence.py: CollectionStatus enum, Target, EvidenceItem, and EvidenceBundle dataclasses with to_dict/from_dict round-trips - agents/core/context.py: AnalysisMetadata and AnalysisContext wrappers with UUID-based run identification - agents/core/sanitizer.py: Recursive credential redaction utility using case-insensitive regex for password/secret/key/token/auth/credential/cert - agents/tests/test_evidence.py: 42 unit tests covering bundle creation, status mapping, JSON serialization round-trips, deeply nested secret masking, PERMISSION_DENIED and TIMEOUT status handling Closes KDM-cli#272
- evidence.py: remove EvidenceItem.create (6-arg biomarker) and use field default_factory for UTC timestamp generation; require collected_at in from_dict - context.py: add __post_init__ invariant rejecting target mismatch with evidence.target across all construction pathways; remove unused field import - sanitizer.py: use explicit stack traversal instead of mutual recursion to prevent RecursionError; redact values of Kubernetes env entries whose sibling name matches sensitive patterns - __init__.py: sort exports in __all__ alphabetically to satisfy RUF022 - conftest.py: add pytest configuration ensuring agents/ is in sys.path - test_evidence.py: update callers to direct EvidenceItem instantiation, add target mismatch invariant tests, missing collected_at KeyError test, deep recursion sanitizer test, sibling env var secret test, and full docstring coverage across all test methods
…canonical-evidence-model feat(agents): Phase 1 - Canonical Evidence Model and Collectors
…into cohesive modules - sanitizer.py: extract _process_dict, _process_list, and _prepare_child helpers to eliminate nested complexity and satisfy CodeScene code health rules - split monolithic test suite into highly cohesive test_evidence.py, test_context.py, and test_sanitizer.py matching agents/core modules
…odescene-and-cohesion refactor(agents): resolve CodeScene code health rules and modularize tests for Phase 1
Implements an in-memory auditable Evidence Store and Tool Audit Trail that
records every tool call with millisecond timings and links diagnostic findings
to supporting evidence IDs and tool call records.
Changes:
- agents/core/store.py:
- ToolCallRecord: captures call_id (call-<hex8>), agent_role, tool_name,
arguments (with sensitive key scrubbing), result, error, started_at,
duration_ms, and status (pending/success/error/timed_out).
- FindingRecord: captures finding_id, agent_role, claim, confidence,
and evidence_refs with to_dict/from_dict round-tripping.
- EvidenceStore: thread-safe in-memory store supporting record_tool_call,
record_finding (validates non-empty evidence_refs), get_provenance
(resolves finding to supporting tool calls), export_audit_log, and JSON
serialisation.
- agents/core/__init__.py: Export EvidenceStore, FindingRecord, and ToolCallRecord
in alphabetical order in __all__.
- agents/tools/registry.py: Instrument ToolRegistry.execute with optional agent_role,
automatic duration measurement, error/timeout handling, argument redaction,
and automatic recording to EvidenceStore.
- agents/tests/test_store.py: 24 unit tests covering ToolCallRecord defaults/scrubbing,
FindingRecord validation, EvidenceStore recording/provenance/audit export,
thread safety, and ToolRegistry execution integration.
Closes KDM-cli#274
…tool-audit-logging-and-evidence-store feat(agents): Phase 3 - Tool Audit Logging and Evidence Store
Implements a deterministic diagnostic rule engine that diagnoses canonical
Kubernetes failure signatures in < 5ms before invoking any LLM turns.
Part of Milestone v4.0.0 — Phase 4 of the Multi-Agent SRE Architecture.
Changes:
- agents/rules/base.py:
- BaseRule abstract class with evaluate(EvidenceBundle) -> Optional[RuleMatch]
- RuleMatch dataclass with rule_id, title, root_cause, confidence, evidence_ids,
confidence range validation, and to_dict/from_dict round-trips
- agents/rules/oom.py:
- OOMKilledRule detecting exit code 137 or reason OOMKilled from container status
- agents/rules/image_pull.py:
- ImagePullRule detecting ErrImagePull and ImagePullBackOff waiting states
- agents/rules/crashloop.py:
- CrashLoopRule detecting CrashLoopBackOff waiting states with exit code > 0
- agents/rules/probes.py:
- ProbeFailureRule detecting liveness/readiness/startup probe failures from events
- agents/rules/scheduling.py:
- SchedulingRule detecting FailedScheduling events with Pending pod phase
- agents/rules/engine.py:
- RuleEngine dispatcher with evaluate_all, get_primary_match, and should_bypass_llm,
sorting matches by confidence descending with sub-millisecond execution
- agents/rules/__init__.py:
- Alphabetically sorted package exports
- agents/tests/test_rules.py:
- 40 unit tests covering all positive matches, negative cases, edge cases,
and execution benchmarking (< 10ms threshold)
Closes KDM-cli#275
…de health rules - engine.py: add logger.exception with rule_id when rule execution fails - oom.py: assign 0.8 confidence when exit code 137 is present without OOMKilled reason; prevent sidecar misattribution by restricting fallback scans when target container is present - image_pull.py: prevent sidecar misattribution by restricting fallback scans when target container is present - crashloop.py: prevent sidecar misattribution by restricting fallback scans when target container is present - scheduling.py: restrict pod phase lookup strictly to ev.pod.status and ev.pod.phase - reduce method cyclomatic complexity, nested depth, and bumpy road conditions across all rules - test_rules.py: parameterize tests to eliminate code duplication, improve cohesion, and reduce file length from 700 to 489 lines (< 600 line threshold)
…ive exit code for crashloop - crashloop.py: require parsed exitCode > 0 (rejecting missing/unparseable/negative exit codes) per issue KDM-cli#275 check; flatten methods to reduce mean complexity to <= 4.0 - oom.py: decompose _detect_oom into _detect_for_target and _scan_containers to eliminate Bumpy Road and reduce mean complexity to <= 4.0 - image_pull.py: decompose _detect_image_pull_failure into _detect_for_target and _scan_containers to eliminate Bumpy Road and reduce mean complexity to <= 4.0 - scheduling.py: split _is_scheduling_event and _inspect_pod_phase into modular helper functions with 0 complex conditionals and complexity <= 4.0 - test_rules.py: add test scenarios for missing, unparseable, and negative exit codes in CrashLoopRule
…deterministic-rule-engine feat(agents): Phase 4 - Deterministic Diagnostic Rule Engine
…, Resource)
Implements structured specialist agents (Runtime, Config, Resource) using
Ollama with strict JSON schemas to eliminate hallucinations and adhere to
a canonical SpecialistReport contract.
Part of Milestone v4.0.0 — Phase 5 of the Multi-Agent SRE Architecture.
Changes:
- agents/specialists/base.py:
- BaseSpecialistAgent abstract base class taking client: ollama.Client and model: str
- run_investigation(bundle: EvidenceBundle) -> Dict[str, Any] passing format="json"
and options={"temperature": 0.1}
- Safe JSON extraction, markdown code fence stripping, and fallback report recovery
on JSONDecodeError, KeyError, or execution timeouts
- Backward-compatible analyze() method supporting legacy council workflow
- agents/specialists/runtime.py:
- RuntimeLogAgent: Senior Linux & Container Runtime Diagnostics Specialist
- Extracts container exit codes, termination signals, and previous/current logs
- Explicitly notes empty previous logs rather than hallucinating application errors
- agents/specialists/config.py:
- ConfigDependencyAgent: Kubernetes Declarative Configuration Specialist
- Inspects missing volumes, missing ConfigMap/Secret keys, waiting states, and probe timeouts
- agents/specialists/resource.py:
- ClusterResourceAgent: Cluster Capacity & Linux Cgroups Specialist
- Inspects node pressure conditions (MemoryPressure, DiskPressure), QoS classes
(Guaranteed, Burstable, BestEffort), and cgroup OOM terminations
- agents/specialists/__init__.py:
- Package exports and legacy SynthesizerAgent compatibility
- agents/tests/test_specialists.py:
- 20 unit tests with mocked Ollama chat responses covering valid JSON, markdown fences,
malformed text recovery, Ollama timeouts/exceptions, domain prompt extractions,
and schema validation across all 3 specialist agents
Closes KDM-cli#276
… comments - agents/specialists/base.py: pass parsed evidence through _normalize_report in analyze(); eliminate compound conditionals in markdown fence stripping - agents/specialists/runtime.py: eliminate all complex conditionals by using single-clause guards and helper predicates; flatten status matching - agents/specialists/config.py: extract envFrom and env.valueFrom ConfigMap/Secret references across regular and init containers; eliminate complex conditionals - agents/specialists/resource.py: ensure QoS Guaranteed match requires both CPU and memory equality; eliminate complex conditionals in QoS calculation and node condition formatting - agents/specialists/__init__.py: log exception on legacy loader failure; split compound check - agents/tests/test_specialists.py: add tests for analyze() evidence normalization, envFrom/valueFrom manifest extraction, and CPU-only QoS Burstable classification
…cialists - base.py: eliminate Bumpy Road Ahead in _extract_content, _strip_markdown_fences, and _normalize_report by extracting flat helpers - runtime.py: resolve Deep Nested Complexity in _match_dict_status and _match_list_status via _find_named_container - config.py: eliminate Bumpy Road Ahead in _format_container_probes and _format_container_env_refs using flat comprehensions - test_specialists.py: resolve Low Cohesion by consolidating into cohesive test suites sharing SpecialistTestBase
…all methods under 60 lines
…o eliminate code duplication
…structured-specialist-agents feat(agents): Phase 5 - Structured Specialist Agents (Runtime, Config, Resource)
…omated code quality checks
…lity-automation feat(ci): configure optimized husky and lint-staged git hooks for automated code quality checks
…ation - Implement AgentOrchestrator in agents/orchestration/orchestrator.py - Enforce concurrency limiting using asyncio.Semaphore (default: 2) - Execute synchronous agent investigations asynchronously via loop.run_in_executor - Guard each specialist agent with asyncio.wait_for timeout enforcement (default: 25.0s) - Guarantee complete failure isolation across timeouts and unhandled agent exceptions - Add comprehensive unit and integration tests in agents/tests/test_orchestrator.py
…ly with CodeScene parameter limits
…parallel-agent-orchestrator feat(agents): Phase 6 - Parallel Agent Orchestrator with Failure Isolation
- Implement ProgressiveInvestigationLoop in agents/orchestration/tool_loop.py - Support multi-turn progressive reasoning (ReAct pattern) up to max_turns = 3 - Handle both Ollama native function calling and structured JSON tool actions - Automatically record all dynamic tool executions into the EvidenceStore - Enforce hard stop at max_turns and invoke forced synthesis with max depth instructions - Add unit tests in agents/tests/test_tool_loop.py validating multi-turn tool calling, error feedback, and loop termination - Add evaluate_next_step and force_synthesis methods to BaseSpecialistAgent
…tion for CodeScene
…phase19-secret-redaction-audit-logging [KDM-TEST-FEAT-377] feat(observability): Phase 19 - System Observability, Secret Redaction & Audit Logging
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…-multi-agent-test-suite-e2e-fixtures test(agents): Phase 20 - Comprehensive Multi-Agent Test Suite & E2E Fixtures
- Implement head/tail log clipping in agents/tools/kubernetes.py (clip to 20 head, 80 tail if > 100 lines) - Configure Ollama chat options with num_ctx=4096, temperature=0.1, and num_predict=512 across BaseSpecialistAgent and specialists - Minify specialist system prompts by > 50% without loss of diagnostic accuracy or domain boundaries - Add comprehensive verification and latency benchmark test suite in agents/tests/test_perf_budget.py - Fixes KDM-cli#292
…ig prompt, and benchmarks
…-concurrency-latency-resource-tuning perf(agents): Phase 21 - Concurrency, Latency & Resource Tuning
Implements Phase 22 dynamic per-role model strategy and configuration for the Multi-Agent SRE Architecture: - Created config/agents.yaml supporting dynamic model mapping per role (runtime, config, resource, synthesizer, default) - Implemented resolve_model(role, config, cli_override) in agents/council.py adhering to priority hierarchy: CLI --model > agents.yaml role mapping > agents.yaml default > fallback 'llama3.1' - Supported global CLI override via --model in kdm analyze and bridged parameters through python-bridge.ts - Retained backward compatibility with legacy resolve_model(client, requested_model) invocations - Created comprehensive test suite in agents/tests/test_model_config.py verifying YAML loading, priority hierarchy, edge cases, and AgentCouncil integration - Added TypeScript CLI option tests in src/__tests__/analyze-command.test.ts
…-dynamic-multi-model-strategy-config [KDM-TEST-FEAT-380] feat(agents): Phase 22 - Dynamic Multi-Model Strategy & Configuration
- Register --multi-agent CLI flag in src/commands/analyze.ts - Add multi_agent boolean configuration option to schema and store in src/config - Support 'kdm config set multi_agent true/false' via boolean value parser in src/commands/config.ts - Create consolidated src/config/config.ts re-exporting schema, store, and migration - Route execution in src/analysis/analysis.ts to runAgentCouncilProcess when multi-agent is enabled via flag, config, or env var - Emit deprecation notice advising that legacy single-prompt mode will be deprecated in v4.1.0 - Add comprehensive unit test suite in src/__tests__/cli-flags.test.ts and CLI option test in src/__tests__/analyze-command.test.ts - Fixes KDM-cli#294
…ll test suite concurrency
…nctions for CodeScene code health
…ature-flags-rollout-legacy-fallback [KDM-TEST-FEAT-381] feat(cli): Phase 23 - Feature Flags, Rollout Modes & Legacy Fallback
… CodeScene code health
…4-final-system-architecture-integration-e2e [KDM-TEST-FEAT-382] feat(release): Phase 24 - Final System Architecture Integration & E2E Validation
Contributor
|
Important Review skippedToo many files! This PR contains 113 files, which is 13 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
Contributor
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Failed
(5 new files with code health below 10.00)
Our agent can fix these. Install it.
Gates Passed
Reason for failure
| New code is healthy | Violations | Code Health Impact | |
|---|---|---|---|
| registry.py | 4 rules | 8.69 | Suppress |
| kubernetes.py | 2 rules | 9.24 | Suppress |
| analysis-baseline.test.ts | 1 rule | 9.39 | Suppress |
| test_tools.py | 1 rule | 9.39 | Suppress |
| sanitizer.py | 1 rule | 9.69 | Suppress |
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Comment on lines
+162
to
+185
| it('handles CrashLoopBackOff with non-zero exit code', async () => { | ||
| registry.clear(); | ||
| const mockPodAnalyzer = createMockAnalyzer('Pod', [CRASH_LOOP_FIXTURE]); | ||
| registry.register(mockPodAnalyzer); | ||
|
|
||
| mockGetCompletion.mockResolvedValueOnce(CRASH_LOOP_LLM_RESPONSE); | ||
|
|
||
| const options: AnalysisOptions = { | ||
| filters: ['Pod'], | ||
| namespace: 'payments', | ||
| explain: true, | ||
| backend: 'openai', | ||
| noCache: true, | ||
| }; | ||
|
|
||
| const result = await runAnalysis(options); | ||
|
|
||
| expect(result.status).toBe('ProblemDetected'); | ||
| expect(result.problems).toBe(1); | ||
| expect(result.results[0].name).toBe('payment-gateway-6c9d4f5c8-4kp2m'); | ||
| expect(result.results[0].errors[0].text).toContain('CrashLoopBackOff'); | ||
| expect(result.results[0].errors[0].text).toContain('exit code 1'); | ||
| expect(result.results[0].details).toBe(CRASH_LOOP_LLM_RESPONSE); | ||
| }); |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 3 functions with similar structure: 'gracefully handles probe threshold timeouts','handles CrashLoopBackOff with non-zero exit code','handles ImagePullBackOff with authorization failure'
Comment on lines
+196
to
+208
| async def test_tail_lines_clamped_to_max(self) -> None: | ||
| """Requesting more than 500 lines is silently clamped to 500.""" | ||
| proc = _make_process(b"log") | ||
| captured_args: list = [] | ||
|
|
||
| async def fake_exec(*args: Any, **kwargs: Any) -> MagicMock: | ||
| captured_args.extend(args) | ||
| return proc | ||
|
|
||
| with patch("asyncio.create_subprocess_exec", side_effect=fake_exec): | ||
| await k8s_tools.get_container_logs("prod", "pod", "c", tail_lines=9999) | ||
|
|
||
| assert "--tail=500" in captured_args |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 3 functions with similar structure: TestGetContainerLogs.test_previous_flag_included,TestGetContainerLogs.test_tail_lines_clamped_to_max,TestGetDockerLogs.test_tail_clamped_to_max
| Raises: | ||
| ValueError: If head or tail are negative, or if head + tail > max_lines. | ||
| """ | ||
| if head < 0 or tail < 0 or max_lines < 0 or (head + tail) > max_lines: |
There was a problem hiding this comment.
❌ New issue: Complex Conditional
clip_log_lines has 1 complex conditionals with 3 branches, threshold = 2
Comment on lines
+202
to
+233
| async def get_container_logs( | ||
| namespace: str, | ||
| pod: str, | ||
| container: str, | ||
| tail_lines: int = 100, | ||
| previous: bool = False, | ||
| ) -> Dict[str, Any]: | ||
| """Retrieve bounded log output for a container. | ||
|
|
||
| Args: | ||
| namespace: Kubernetes namespace containing the pod. | ||
| pod: Pod name. | ||
| container: Container name within the pod. | ||
| tail_lines: Maximum number of lines to return (clamped to 500). | ||
| previous: If ``True``, fetch logs from the *previous* container run. | ||
|
|
||
| Returns: | ||
| A dict with a ``"lines"`` key containing a list of log strings, or an | ||
| ``{"error": ...}`` dict on failure. | ||
| """ | ||
| clamped = _clamp_tail(tail_lines) | ||
| cmd = _build_logs_cmd(namespace, pod, container, clamped, previous) | ||
| proc = await asyncio.create_subprocess_exec( | ||
| "kubectl", *cmd, | ||
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.PIPE, | ||
| ) | ||
| stdout, stderr = await proc.communicate() | ||
| if proc.returncode != 0: | ||
| return _kubectl_error(stderr.decode()) | ||
| raw_lines = stdout.decode().splitlines() | ||
| return {"lines": clip_log_lines(raw_lines)} |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
get_container_logs has 5 arguments, max arguments = 4
Comment on lines
+236
to
+262
| def _build_logs_cmd( | ||
| namespace: str, | ||
| pod: str, | ||
| container: str, | ||
| tail_lines: int, | ||
| previous: bool, | ||
| ) -> List[str]: | ||
| """Construct the kubectl logs argument list. | ||
|
|
||
| Args: | ||
| namespace: Kubernetes namespace. | ||
| pod: Pod name. | ||
| container: Container name. | ||
| tail_lines: Number of lines (already clamped). | ||
| previous: Whether to fetch previous-run logs. | ||
|
|
||
| Returns: | ||
| A list of string arguments for kubectl (excluding ``"kubectl"`` itself). | ||
| """ | ||
| cmd = [ | ||
| "logs", pod, "-n", namespace, | ||
| "-c", container, | ||
| f"--tail={tail_lines}", | ||
| ] | ||
| if previous: | ||
| cmd.append("--previous") | ||
| return cmd |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
_build_logs_cmd has 5 arguments, max arguments = 4
Comment on lines
+182
to
+272
| async def execute( | ||
| self, | ||
| name: str, | ||
| agent_role: str = "", | ||
| store: Optional[EvidenceStore] = None, | ||
| re_raise: bool = False, | ||
| **kwargs: Any, | ||
| ) -> Any: | ||
| """Invoke a registered tool by name with the supplied keyword arguments. | ||
|
|
||
| Automatically measures wall-clock duration in milliseconds, handles | ||
| exceptions and timeouts, scrubs sensitive argument keys, and records | ||
| the invocation into the active :class:`~core.store.EvidenceStore`. | ||
|
|
||
| Args: | ||
| name: The registered tool name to call. | ||
| agent_role: Optional specialist agent role initiating the call (e.g. ``"runtime"``). | ||
| store: Optional override :class:`~core.store.EvidenceStore` for this call. | ||
| re_raise: When ``True``, re-raises unhandled tool exceptions after recording. | ||
| **kwargs: Arguments forwarded to the tool implementation. | ||
|
|
||
| Raises: | ||
| ValueError: If *name* is not in the whitelist. | ||
|
|
||
| Returns: | ||
| The structured JSON-serialisable result produced by the tool, or | ||
| an ``{"error": ...}`` dict on timeout, exception, or RBAC failure. | ||
| """ | ||
| if name not in self._tools: | ||
| raise ValueError(f"Unauthorized or unknown tool: {name}") | ||
|
|
||
| func = self._tools[name]["func"] | ||
| sig = inspect.signature(func) | ||
| call_kwargs = dict(kwargs) | ||
| if ( | ||
| "agent_role" in sig.parameters | ||
| and "agent_role" not in call_kwargs | ||
| and agent_role | ||
| ): | ||
| call_kwargs["agent_role"] = agent_role | ||
| if "store" in sig.parameters and "store" not in call_kwargs and store: | ||
| call_kwargs["store"] = store | ||
|
|
||
| scrubbed_args = redact_sensitive_data(dict(kwargs)) | ||
| if not isinstance(scrubbed_args, dict): | ||
| scrubbed_args = {} | ||
|
|
||
| start_time = time.time() | ||
| status = "pending" | ||
| result: Any = None | ||
| error: Optional[str] = None | ||
| unhandled_exc: Optional[Exception] = None | ||
|
|
||
| try: | ||
| result = await asyncio.wait_for( | ||
| func(**call_kwargs), timeout=_TOOL_TIMEOUT_SECONDS | ||
| ) | ||
| if isinstance(result, dict) and "error" in result: | ||
| status = "error" | ||
| error = str(result["error"]) | ||
| else: | ||
| status = "success" | ||
| except asyncio.TimeoutError: | ||
| status = "timed_out" | ||
| error = f"Tool execution timed out after {int(_TOOL_TIMEOUT_SECONDS)}s" | ||
| result = {"error": error} | ||
| except Exception as exc: | ||
| status = "error" | ||
| error = str(exc) | ||
| result = {"error": error} | ||
| unhandled_exc = exc | ||
| finally: | ||
| duration_ms = int(round((time.time() - start_time) * 1000)) | ||
| record = ToolCallRecord( | ||
| agent_role=agent_role, | ||
| tool_name=name, | ||
| arguments=scrubbed_args, | ||
| result=result, | ||
| error=error, | ||
| started_at=start_time, | ||
| duration_ms=duration_ms, | ||
| status=status, | ||
| ) | ||
| target_store = store if store is not None else self._store | ||
| if target_store is not None: | ||
| target_store.record_tool_call(record) | ||
|
|
||
| if re_raise and unhandled_exc is not None: | ||
| raise unhandled_exc | ||
|
|
||
| return result |
There was a problem hiding this comment.
❌ New issue: Complex Method
ToolRegistry.execute has a cyclomatic complexity of 19, threshold = 9
Comment on lines
+216
to
+222
| if ( | ||
| "agent_role" in sig.parameters | ||
| and "agent_role" not in call_kwargs | ||
| and agent_role | ||
| ): | ||
| call_kwargs["agent_role"] = agent_role | ||
| if "store" in sig.parameters and "store" not in call_kwargs and store: |
There was a problem hiding this comment.
❌ New issue: Complex Conditional
ToolRegistry.execute has 2 complex conditionals with 4 branches, threshold = 2
Comment on lines
+182
to
+272
| async def execute( | ||
| self, | ||
| name: str, | ||
| agent_role: str = "", | ||
| store: Optional[EvidenceStore] = None, | ||
| re_raise: bool = False, | ||
| **kwargs: Any, | ||
| ) -> Any: | ||
| """Invoke a registered tool by name with the supplied keyword arguments. | ||
|
|
||
| Automatically measures wall-clock duration in milliseconds, handles | ||
| exceptions and timeouts, scrubs sensitive argument keys, and records | ||
| the invocation into the active :class:`~core.store.EvidenceStore`. | ||
|
|
||
| Args: | ||
| name: The registered tool name to call. | ||
| agent_role: Optional specialist agent role initiating the call (e.g. ``"runtime"``). | ||
| store: Optional override :class:`~core.store.EvidenceStore` for this call. | ||
| re_raise: When ``True``, re-raises unhandled tool exceptions after recording. | ||
| **kwargs: Arguments forwarded to the tool implementation. | ||
|
|
||
| Raises: | ||
| ValueError: If *name* is not in the whitelist. | ||
|
|
||
| Returns: | ||
| The structured JSON-serialisable result produced by the tool, or | ||
| an ``{"error": ...}`` dict on timeout, exception, or RBAC failure. | ||
| """ | ||
| if name not in self._tools: | ||
| raise ValueError(f"Unauthorized or unknown tool: {name}") | ||
|
|
||
| func = self._tools[name]["func"] | ||
| sig = inspect.signature(func) | ||
| call_kwargs = dict(kwargs) | ||
| if ( | ||
| "agent_role" in sig.parameters | ||
| and "agent_role" not in call_kwargs | ||
| and agent_role | ||
| ): | ||
| call_kwargs["agent_role"] = agent_role | ||
| if "store" in sig.parameters and "store" not in call_kwargs and store: | ||
| call_kwargs["store"] = store | ||
|
|
||
| scrubbed_args = redact_sensitive_data(dict(kwargs)) | ||
| if not isinstance(scrubbed_args, dict): | ||
| scrubbed_args = {} | ||
|
|
||
| start_time = time.time() | ||
| status = "pending" | ||
| result: Any = None | ||
| error: Optional[str] = None | ||
| unhandled_exc: Optional[Exception] = None | ||
|
|
||
| try: | ||
| result = await asyncio.wait_for( | ||
| func(**call_kwargs), timeout=_TOOL_TIMEOUT_SECONDS | ||
| ) | ||
| if isinstance(result, dict) and "error" in result: | ||
| status = "error" | ||
| error = str(result["error"]) | ||
| else: | ||
| status = "success" | ||
| except asyncio.TimeoutError: | ||
| status = "timed_out" | ||
| error = f"Tool execution timed out after {int(_TOOL_TIMEOUT_SECONDS)}s" | ||
| result = {"error": error} | ||
| except Exception as exc: | ||
| status = "error" | ||
| error = str(exc) | ||
| result = {"error": error} | ||
| unhandled_exc = exc | ||
| finally: | ||
| duration_ms = int(round((time.time() - start_time) * 1000)) | ||
| record = ToolCallRecord( | ||
| agent_role=agent_role, | ||
| tool_name=name, | ||
| arguments=scrubbed_args, | ||
| result=result, | ||
| error=error, | ||
| started_at=start_time, | ||
| duration_ms=duration_ms, | ||
| status=status, | ||
| ) | ||
| target_store = store if store is not None else self._store | ||
| if target_store is not None: | ||
| target_store.record_tool_call(record) | ||
|
|
||
| if re_raise and unhandled_exc is not None: | ||
| raise unhandled_exc | ||
|
|
||
| return result |
There was a problem hiding this comment.
❌ New issue: Large Method
ToolRegistry.execute has 81 lines, threshold = 70
Comment on lines
+182
to
+272
| async def execute( | ||
| self, | ||
| name: str, | ||
| agent_role: str = "", | ||
| store: Optional[EvidenceStore] = None, | ||
| re_raise: bool = False, | ||
| **kwargs: Any, | ||
| ) -> Any: | ||
| """Invoke a registered tool by name with the supplied keyword arguments. | ||
|
|
||
| Automatically measures wall-clock duration in milliseconds, handles | ||
| exceptions and timeouts, scrubs sensitive argument keys, and records | ||
| the invocation into the active :class:`~core.store.EvidenceStore`. | ||
|
|
||
| Args: | ||
| name: The registered tool name to call. | ||
| agent_role: Optional specialist agent role initiating the call (e.g. ``"runtime"``). | ||
| store: Optional override :class:`~core.store.EvidenceStore` for this call. | ||
| re_raise: When ``True``, re-raises unhandled tool exceptions after recording. | ||
| **kwargs: Arguments forwarded to the tool implementation. | ||
|
|
||
| Raises: | ||
| ValueError: If *name* is not in the whitelist. | ||
|
|
||
| Returns: | ||
| The structured JSON-serialisable result produced by the tool, or | ||
| an ``{"error": ...}`` dict on timeout, exception, or RBAC failure. | ||
| """ | ||
| if name not in self._tools: | ||
| raise ValueError(f"Unauthorized or unknown tool: {name}") | ||
|
|
||
| func = self._tools[name]["func"] | ||
| sig = inspect.signature(func) | ||
| call_kwargs = dict(kwargs) | ||
| if ( | ||
| "agent_role" in sig.parameters | ||
| and "agent_role" not in call_kwargs | ||
| and agent_role | ||
| ): | ||
| call_kwargs["agent_role"] = agent_role | ||
| if "store" in sig.parameters and "store" not in call_kwargs and store: | ||
| call_kwargs["store"] = store | ||
|
|
||
| scrubbed_args = redact_sensitive_data(dict(kwargs)) | ||
| if not isinstance(scrubbed_args, dict): | ||
| scrubbed_args = {} | ||
|
|
||
| start_time = time.time() | ||
| status = "pending" | ||
| result: Any = None | ||
| error: Optional[str] = None | ||
| unhandled_exc: Optional[Exception] = None | ||
|
|
||
| try: | ||
| result = await asyncio.wait_for( | ||
| func(**call_kwargs), timeout=_TOOL_TIMEOUT_SECONDS | ||
| ) | ||
| if isinstance(result, dict) and "error" in result: | ||
| status = "error" | ||
| error = str(result["error"]) | ||
| else: | ||
| status = "success" | ||
| except asyncio.TimeoutError: | ||
| status = "timed_out" | ||
| error = f"Tool execution timed out after {int(_TOOL_TIMEOUT_SECONDS)}s" | ||
| result = {"error": error} | ||
| except Exception as exc: | ||
| status = "error" | ||
| error = str(exc) | ||
| result = {"error": error} | ||
| unhandled_exc = exc | ||
| finally: | ||
| duration_ms = int(round((time.time() - start_time) * 1000)) | ||
| record = ToolCallRecord( | ||
| agent_role=agent_role, | ||
| tool_name=name, | ||
| arguments=scrubbed_args, | ||
| result=result, | ||
| error=error, | ||
| started_at=start_time, | ||
| duration_ms=duration_ms, | ||
| status=status, | ||
| ) | ||
| target_store = store if store is not None else self._store | ||
| if target_store is not None: | ||
| target_store.record_tool_call(record) | ||
|
|
||
| if re_raise and unhandled_exc is not None: | ||
| raise unhandled_exc | ||
|
|
||
| return result |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
ToolRegistry.execute has 5 arguments, max arguments = 4
| has_sensitive_env = _has_sensitive_sibling(src) | ||
| for key, val in src.items(): | ||
| str_key = str(key) | ||
| if _is_sensitive_key(str_key) or (has_sensitive_env and str_key.lower() == "value"): |
There was a problem hiding this comment.
❌ New issue: Complex Conditional
_process_dict has 1 complex conditionals with 2 branches, threshold = 2
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
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
Please review the release branch changes before merging.