Agent #19

garland3 · 2025-10-29T01:21:27Z

No description provided.

- Implement handle_attach_file method in ChatService to attach user files to sessions via S3 key - Update WebSocket connection adapter to authenticate user from query params or config fallback - Handle "attach_file" message type in WebSocket endpoint, calling handle_attach_file with authenticated user - Ensures file ownership verification and adds file to session context securely

Update websocket endpoint to authenticate users primarily through X-Authenticated-User header set by authenticated reverse proxy in production, falling back to insecure query parameter for local development. Add comprehensive security documentation explaining the authentication flow, production requirements, and risks of direct access. This ensures secure connections in production while maintaining development flexibility.

Relocated the /files/healthz endpoint to appear before the /files/{file_key:path} route in files_routes.py to avoid path capture conflicts in the router. The endpoint now includes an explanatory note in its docstring for clarity.

Prevent leading '-' in tags by conditionally enabling SHA types: use branch prefix for branches and 'pr-' for pull requests.

- Replace template variables with ${{ github.ref_name }}, ${{ github.event_name }}, and ${{ github.ref }} for safer and more accurate branch/PR tagging - Simplify enabling conditions for SHA tags on branches and PRs - Set 'latest' tag only on main branch push - Remove trailing newline_clip económicallyselectedocument at the end of the file.

- In chat service, reduced verbosity in log messages by removing server/prompt_name and filename details to streamline output. - Removed unused response variable assignment in websocket endpoint. - Eliminated unused Filter icon import from frontend component.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

…error Previously, the error log for failed file attachments included raw s3_key and exception details, potentially introducing multi-line entries or injection risks. This update strips '\n' and '\r' characters from both to ensure clean, single-line log output for better readability and security.

- Introduced new `output_filename` parameter with default "presentation" - Added `_sanitize_filename` helper to clean filenames (remove non-alphanumeric/underscore/dash, truncate to 50 chars) - Updated output paths for PPTX/HTML to use sanitized custom names instead of hardcoded ones - Reflected changes in artifacts, metadata, and display primary file references - Improves file identification for generated presentations and exports when used across multiple instances

Update json_to_pptx and markdown_to_pptx function signatures to accept Annotated[Optional[str], ...] instead of Annotated[str, ...] for output_filename, image_filename, and image_data_base64 parameters. Add None-handling logic in function bodies to use default values ("presentation" for output_filename, empty strings otherwise), preventing potential errors from None inputs and enhancing API robustness and usability.

- Add ActAgentLoop for fast, minimal-overhead tasks without reasoning steps - Update AgentLoopFactory to register "act" strategy - Document new strategy in CLAUDE.md alongside existing ReAct and Think-Act loops - Add file naming guidelines and emoji policy to CLAUDE.md - Mark agent start events with strategy type for better traceability

backend/application/chat/agent/factory.py

backend/application/chat/service.py

backend/mcp/pptx_generator/main.py

Copilot

Pull Request Overview

This PR adds support for configurable agent loop strategies, introducing a new "act" strategy and refactoring the agent loop instantiation to use a factory pattern. Key changes include:

New AgentLoopFactory for creating agent loop instances based on strategy selection
Added new "act" agent loop implementation (pure action loop without explicit reasoning)
Frontend UI controls for selecting agent loop strategy (react, think-act, act)
Changed default strategy from "react" to "think-act" in configuration
Improved agent event notifications to include strategy information
Updated tool_choice behavior in agent loops to use "required" instead of "auto"
Added customizable output filenames to PowerPoint generation tools
Minor documentation updates and new service refactor planning document

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`frontend/src/hooks/useSettings.js`	Added `agentLoopStrategy` to default settings with "think-act" default
`frontend/src/handlers/chat/websocketHandlers.js`	Updated agent start message to display strategy name
`frontend/src/contexts/ChatContext.jsx`	Pass agent loop strategy from settings to backend
`frontend/src/components/SettingsPanel.jsx`	Added UI controls for selecting agent loop strategy with descriptions
`backend/modules/config/manager.py`	Changed default agent loop strategy from "react" to "think-act"
`backend/main.py`	Extract and pass agent_loop_strategy parameter to chat service
`backend/application/chat/service.py`	Refactored to use AgentLoopFactory instead of direct instantiation
`backend/application/chat/agent/factory.py`	New factory class for creating agent loop instances with caching
`backend/application/chat/agent/act_loop.py`	New pure action agent loop implementation
`backend/application/chat/agent/think_act_loop.py`	Updated to emit strategy in events and use "required" tool_choice
`backend/application/chat/agent/react_loop.py`	Updated to emit strategy in events and use "required" tool_choice
`backend/application/chat/agent/__init__.py`	Export AgentLoopFactory
`backend/mcp/pptx_generator/main.py`	Added customizable output filenames and sanitization
`backend/modules/llm/litellm_caller.py`	Simplified comment about tool_choice handling
`backend/modules/file_storage/s3_client.py`	Added log sanitization helper function
`config/overrides/llmconfig.yml`	Updated model configuration (appears to be test data)
`config/overrides/messages.txt`	Added test message file
`docs/service-refactor-plan.md`	New architecture planning document
`CLAUDE.md`	Updated documentation for agent strategies and file naming guidelines
`.env.example`	Added AGENT_LOOP_STRATEGY configuration

Copilot · 2025-10-29T01:24:24Z

backend/application/chat/agent/factory.py

+            AgentLoopProtocol instance
+
+        Raises:
+            ValueError: If strategy is not recognized


The docstring claims this method raises ValueError if the strategy is not recognized, but the implementation actually falls back to 'react' with a warning (lines 83-88) instead of raising an exception. Either update the docstring to reflect the actual behavior or implement the documented exception.

Copilot · 2025-10-29T01:24:24Z

backend/application/chat/agent/think_act_loop.py

+                        model, messages, data_sources, tools_schema, context.user_email, "required", temperature=temperature
+                    )
+                else:
+                    llm_response = await self.llm.call_with_tools(
+                        model, messages, tools_schema, "required", temperature=temperature
+                    )


The tool_choice parameter was changed from 'auto' to 'required' in the action loop. This is a potentially breaking behavior change that could cause failures with LLM providers that don't support 'required'. While the LiteLLM caller has fallback logic, this change should be accompanied by tests to verify the fallback behavior works correctly in agent mode, or the tool_choice should remain 'auto' to maintain existing behavior.

Copilot · 2025-10-29T01:24:25Z

backend/application/chat/agent/react_loop.py

+                        model, messages, data_sources, tools_schema, context.user_email, "required", temperature=temperature
                    )
                else:
                    llm_response = await self.llm.call_with_tools(
-                        model, messages, tools_schema, "auto", temperature=temperature
+                        model, messages, tools_schema, "required", temperature=temperature
                    )


Same as think_act_loop.py: changing tool_choice from 'auto' to 'required' could cause failures with LLM providers that don't support 'required'. This behavior change needs test coverage or should be reverted to maintain compatibility.

Copilot · 2025-10-29T01:24:25Z

backend/mcp/pptx_generator/main.py

+    # Remove bad characters (anything not alphanumeric, underscore, or dash)
+    sanitized = re.sub(r'[^\w\-]', '', filename)


The regex pattern [^\w\-] removes all non-alphanumeric characters except underscore and dash, but it will also remove dots (periods). This means if a user passes 'my.file.name', it becomes 'myfilename', losing the dot separators. Consider preserving dots: r'[^\w\-.]' or documenting this behavior clearly.

Suggested change

# Remove bad characters (anything not alphanumeric, underscore, or dash)

sanitized = re.sub(r'[^\w\-]', '', filename)

# Remove bad characters (anything not alphanumeric, underscore, dash, or dot)

sanitized = re.sub(r'[^\w\-.]', '', filename)

Copilot · 2025-10-29T01:24:25Z

backend/application/chat/agent/factory.py

+"""Factory for creating agent loop instances based on strategy."""
+
+import logging
+from typing import Any, Optional


Import of 'Any' is not used.

Suggested change

from typing import Any, Optional

from typing import Optional

Copilot · 2025-10-29T01:24:26Z

backend/application/chat/service.py

 # Import utilities
 from .utilities import tool_utils, file_utils, notification_utils, error_utils
-from .agent import AgentLoopProtocol, ReActAgentLoop, ThinkActAgentLoop
+from .agent import AgentLoopProtocol, AgentLoopFactory


Import of 'AgentLoopProtocol' is not used.

Suggested change

from .agent import AgentLoopProtocol, AgentLoopFactory

from .agent import AgentLoopFactory

Copilot · 2025-10-29T01:24:26Z

backend/mcp/pptx_generator/main.py


+def _sanitize_filename(filename: str, max_length: int = 50) -> str:
+    """Sanitize filename by removing bad characters and truncating."""
+    import re


This import of module re is redundant, as it was previously imported on line 21.

Suggested change

import re

Copilot · 2025-10-29T01:24:26Z

backend/application/chat/service.py

+        except Exception:
+            pass


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

pass

except Exception as e:

logger.warning("Failed to read agent loop strategy from config, using default '%s': %s", self.default_agent_strategy, e)

- Remove unused imports (Any, AgentLoopProtocol) - Add explanatory comment to empty except clause - Remove redundant 're' import in pptx_generator - Fix factory.py docstring (falls back instead of raising ValueError) - Add comments explaining tool_choice='required' with fallback logic - Rename 'sanitized' variable to 'cleaned_filename' for clarity Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>

garland3 and others added 18 commits October 25, 2025 20:05

Remove MinIO volumes from version control and add to .gitignore

3c38310

Update .gitignore with MinIO volume exclusions

ebff30c

addressed bot identified issues

fee54a4

reverted changes that were breaking the websocket setup

829377c

ci: fix SHA tagging for branches and PRs to avoid invalid prefix

332073f

Prevent leading '-' in tags by conditionally enabling SHA types: use branch prefix for branches and 'pr-' for pull requests.

Potential fix for code scanning alert no. 275: Log Injection

edddee1

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Merge remote-tracking branch 'origin/minio' into minio

8a41d00

use can select agent

7153a44

Merge remote-tracking branch 'origin/main' into agent

1a44a6a

garland3 requested a review from Copilot October 29, 2025 01:21

github-advanced-security bot found potential problems Oct 29, 2025

View reviewed changes

backend/application/chat/agent/factory.py Fixed Show fixed Hide fixed

backend/application/chat/service.py Fixed Show fixed Hide fixed

backend/application/chat/service.py Fixed Show fixed Hide fixed

backend/mcp/pptx_generator/main.py Fixed Show fixed Hide fixed

Copilot AI reviewed Oct 29, 2025

View reviewed changes

garland3 merged commit d172154 into main Oct 29, 2025
7 of 8 checks passed

garland3 deleted the agent branch October 29, 2025 02:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Agent #19

Agent #19

Uh oh!

garland3 commented Oct 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Copilot AI Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		# Remove bad characters (anything not alphanumeric, underscore, or dash)
		sanitized = re.sub(r'[^\w\-]', '', filename)

	from .agent import AgentLoopProtocol, AgentLoopFactory
	from .agent import AgentLoopFactory

-        except Exception:
-            pass
+        except Exception as e:
+            logger.warning("Failed to read agent loop strategy from config, using default '%s': %s", self.default_agent_strategy, e)

Agent #19

Agent #19

Uh oh!

Conversation

garland3 commented Oct 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants