Add Ollama Support and Conversation History#10
Add Ollama Support and Conversation History#10iffystrayer wants to merge 3 commits intodfinke:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ef6a8f9a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function Execute-Tool { | ||
| param([string]$Name, $ToolInput) | ||
|
|
||
| switch ($Name) { | ||
| "Read-File" { |
There was a problem hiding this comment.
Because Delegate-Task is advertised in the tools array above, Claude can legitimately return a tool_use with that name. Execute-Tool has no case for it (and no default), so it returns $null; the next block then calls $result.Substring(...), which throws on null and terminates the loop. This means any model tool call to Delegate-Task will crash the session instead of returning a tool result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds additional PowerShell agent entrypoints to support Ollama and to persist conversation history to disk.
Changes:
- Added
Invoke-PSOllamafor OpenAI-compatible Ollama chat/tool-calling with conversation logging. - Added
Invoke-PSClaudeCodeWithLogvariant that logs tool calls/results and final responses toConversations/. - Updated
.gitignoreto exclude conversation logs and other local artifacts.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| Public/Invoke-PSOllama.ps1 | New Ollama-backed agent cmdlet with tool execution and markdown logging. |
| Public/Invoke-PSClaudeCodeWithLog.ps1 | New Claude-backed agent cmdlet variant that logs conversations and tool activity. |
| .gitignore | Ignores conversation logs and other generated/local files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $toolResults = @() | ||
| foreach ($toolCall in $message.tool_calls) { | ||
| $toolName = $toolCall.function.name | ||
| $toolInput = $toolCall.function.arguments | ConvertFrom-Json |
There was a problem hiding this comment.
$toolCall.function.arguments | ConvertFrom-Json is not wrapped in a try/catch in the sub-agent loop. If the provider returns an already-parsed object or invalid JSON, this will throw and terminate the sub-agent. Handle string-vs-object like the main loop does and return a tool error result instead of throwing.
| $toolInput = $toolCall.function.arguments | ConvertFrom-Json | |
| $rawArgs = $toolCall.function.arguments | |
| # Handle string vs. already-parsed object and JSON errors gracefully | |
| if ($rawArgs -is [string]) { | |
| try { | |
| $toolInput = $rawArgs | ConvertFrom-Json -ErrorAction Stop | |
| } | |
| catch { | |
| $errorMessage = "Error parsing JSON arguments for tool '$toolName': $($_.Exception.Message)" | |
| $toolResults += @{ | |
| tool_call_id = $toolCall.id | |
| role = "tool" | |
| name = $toolName | |
| content = $errorMessage | |
| } | |
| continue | |
| } | |
| } | |
| else { | |
| # Assume non-string arguments are already an object | |
| $toolInput = $rawArgs | |
| } |
| function Log-Message { | ||
| param([string]$Content) | ||
| $timestamp = (Get-Date).ToString("yyyy-MM-dd HH:mm:ss") | ||
| Add-Content -Path $logFile -Value $Content |
There was a problem hiding this comment.
Log-Message computes a $timestamp but never uses it, so log entries have no time context and the variable is dead code. Either include the timestamp in the logged content (e.g., prefix each entry) or remove the unused variable.
| Add-Content -Path $logFile -Value $Content | |
| Add-Content -Path $logFile -Value "[$timestamp] $Content" |
| [Parameter(ValueFromPipeline = $true)] | ||
| [object]$InputObject, | ||
| [string]$Model = "llama3.2:latest", | ||
| [string]$Endpoint = "http://192.168.12.176:11434/v1/chat/completions", |
There was a problem hiding this comment.
The default Endpoint is hard-coded to a private LAN IP. This will fail for most users out of the box and can unintentionally send prompts to an unexpected host. Consider defaulting to localhost (or requiring the parameter) and optionally reading from an env var/config instead of embedding an internal address in the module.
| [string]$Endpoint = "http://192.168.12.176:11434/v1/chat/completions", | |
| [string]$Endpoint = $(if ($env:PSOLLAMA_ENDPOINT) { $env:PSOLLAMA_ENDPOINT } else { "http://localhost:11434/v1/chat/completions" }), |
| function Invoke-PSOllama { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Position = 0)] | ||
| [string]$Task, |
There was a problem hiding this comment.
This function is in Public/ and will be dot-sourced by the module, but it will not be discoverable after Import-Module because the manifest exports only Invoke-PSClaudeCode (see PSClaudeCode.psd1:10). Either export this function in the manifest (and document it) or move it to Examples//Private/ if it’s meant to be a demo-only script.
| @{ | ||
| name = "Delegate-Task" | ||
| description = "Delegate a focused task to a sub-agent with limited context" | ||
| input_schema = @{ | ||
| type = "object" | ||
| properties = @{ | ||
| task = @{ type = "string"; description = "The task to delegate" } | ||
| maxTurns = @{ type = "integer"; description = "Maximum turns for the sub-agent (default 10)" } | ||
| } | ||
| required = @("task") | ||
| } | ||
| } |
There was a problem hiding this comment.
Delegate-Task is included in the tools list, but this script doesn’t implement it (there’s no corresponding Execute-Tool case / Run-SubAgent helper). If the model calls it, tool execution will fail/return nothing. Either implement delegation here (as Invoke-PSClaudeCode does) or remove Delegate-Task from the tool definitions.
| @{ | |
| name = "Delegate-Task" | |
| description = "Delegate a focused task to a sub-agent with limited context" | |
| input_schema = @{ | |
| type = "object" | |
| properties = @{ | |
| task = @{ type = "string"; description = "The task to delegate" } | |
| maxTurns = @{ type = "integer"; description = "Maximum turns for the sub-agent (default 10)" } | |
| } | |
| required = @("task") | |
| } | |
| } |
| function Log-Message { | ||
| param([string]$Content) | ||
| $timestamp = (Get-Date).ToString("yyyy-MM-dd HH:mm:ss") | ||
| Add-Content -Path $logFile -Value $Content |
There was a problem hiding this comment.
Log-Message assigns a $timestamp but never uses it, so log entries don’t include timing information and the variable is dead code. Either incorporate the timestamp into each log entry or remove the unused variable.
| Add-Content -Path $logFile -Value $Content | |
| Add-Content -Path $logFile -Value "[$timestamp] $Content" |
| function Invoke-PSOllama { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Position = 0)] | ||
| [string]$Task, |
There was a problem hiding this comment.
The repo has a Pester test suite (Tests/Invoke-PSClaudeCode.Tests.ps1) and CI runs it. Adding new cmdlets without tests makes regressions likely (e.g., parameter binding, logging behavior, tool-call parsing). Consider adding Pester tests for this cmdlet (and the new WithLog cmdlet if it’s meant to be supported).
| function Invoke-PSOllama { | ||
| [CmdletBinding()] | ||
| param( |
There was a problem hiding this comment.
This new public cmdlet doesn’t include comment-based help. The module’s existing exported cmdlet does (see Public/Invoke-PSClaudeCode.ps1:1-40), and users will rely on Get-Help for usage. Add a help block with at least SYNOPSIS/DESCRIPTION/PARAMETER/EXAMPLE sections.
| "Run-Command" { | ||
| try { | ||
| $output = Invoke-Expression $ToolInput.command 2>&1 | Out-String | ||
| return "`$ $($ToolInput.command)`n$output" | ||
| } | ||
| catch { | ||
| return "Error: $_" | ||
| } | ||
| } |
There was a problem hiding this comment.
The Run-Command tool executes untrusted command strings from the remote Claude API using Invoke-Expression $ToolInput.command, effectively granting https://api.anthropic.com the ability to run arbitrary PowerShell on the local machine. Because Check-Permission only prompts on a narrow set of regex-matched patterns, an attacker controlling or compromising the Claude response can return commands that bypass this heuristic and perform full code execution or sensitive data exfiltration. Avoid using Invoke-Expression on model-provided strings and instead enforce strict command whitelisting and/or explicit, per-command user approval before any tool-invoked command is executed.
| "Run-Command" { | ||
| try { | ||
| $output = Invoke-Expression $ToolInput.command 2>&1 | Out-String | ||
| return "`$ $($ToolInput.command)`n$output" | ||
| } | ||
| catch { | ||
| return "Error: $_" | ||
| } | ||
| } |
There was a problem hiding this comment.
The Run-Command tool executes command strings returned from the LLM using Invoke-Expression $ToolInput.command, which means the Ollama HTTP endpoint can cause arbitrary PowerShell commands to run on the local machine. The only safeguard is a regex-based check in Check-Permission that flags a few patterns, so a malicious or compromised endpoint (or MITM on $Endpoint) can return commands that bypass this check and achieve full code execution or data exfiltration. Replace Invoke-Expression on LLM-provided strings with a safer, parameterized invocation mechanism and require strict whitelisting or explicit user approval for any command the model is allowed to run.
I addd two .ps1 files to demonstrate conversation history and llama support.