Skip to content

PR: 55N10E/picoclaw-1#1 Fix/tool config load image reaction#2888

Closed
ghost wants to merge 13 commits into
mainfrom
unknown repository
Closed

PR: 55N10E/picoclaw-1#1 Fix/tool config load image reaction#2888
ghost wants to merge 13 commits into
mainfrom
unknown repository

Conversation

@ghost

@ghost ghost commented May 17, 2026

Copy link
Copy Markdown

📝 Description

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

  • Reference URL:
  • Reasoning:

🧪 Test Environment

  • Hardware:
  • OS:
  • Model/Provider:
  • Channels:

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@CLAassistant

CLAassistant commented May 17, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ 55N10E
❌ Codex-Agent
You have signed the CLA already but the status is still pending? Let us recheck it.

@ghost ghost force-pushed the fix/tool-config-load-image-reaction branch from c82db88 to 7e3fe16 Compare May 17, 2026 12:25
55N10E and others added 9 commits May 17, 2026 14:37
When a gateway process exits without cleaning up the PID file (kill -9,
hard crash, power loss), and the PID is reused by an unrelated process
(e.g. systemd-resolved), the singleton check would incorrectly block
gateway startup, causing a crash loop under systemd.

The fix adds isPicoclawProcess() which reads /proc/<pid>/cmdline on Linux
to verify the process name contains 'picoclaw'. If the check cannot be
performed (non-Linux systems without /proc), it conservatively returns
true to avoid breaking existing behavior.

Changes:
- pkg/pid/pidfile_unix.go: Add isPicoclawProcess() verification after
  signal(0) succeeds and after EPERM. Process must contain 'picoclaw'
  in its cmdline to be considered a running gateway instance.
- pkg/pid/pidfile_windows.go: Add TODO comment for equivalent fix.
- pkg/pid/pidfile_test.go: Add 3 tests for isPicoclawProcess.

Fixes #2720
…safety guard

The absolutePathPattern regex matches any /... sequence, which incorrectly
captures path separators inside relative paths. For example, 'skills/whoami/SKILL.md'
would match '/whoami/SKILL.md' starting at the internal slash, causing the
workspace restriction to incorrectly block the command.

Add a context check: if the character before a / match is alphanumeric,
underscore, dash, dot, or slash, the / is part of a relative path and
should be skipped. This preserves correct blocking of true absolute paths
like /etc/passwd while allowing relative paths like skills/whoami/SKILL.md.

Fixes #2749
fix(pid): verify process identity to prevent stale PID false positives
fix(shell): prevent relative paths from being treated as absolute in safety guard
…tions API

OpenAI renamed the web_search_preview tool type to web_search in the
Chat Completions API. Using the legacy name causes a 400 error:

  Invalid value: 'web_search_preview'.
  Supported values are: 'function' and 'custom'.

This fix updates the openai_compat provider to use the current tool type
name, aligning with the OpenAI Responses API which already uses
web_search via the SDK.

Fixes #2887

Changes:
- provider.go: buildToolsList() emits "web_search" instead of
  "web_search_preview"
- provider_test.go: updated all test assertions and comments
- types.go: updated NativeSearchCapable doc comment
- config.go: updated PreferNative doc comment
fix(provider): rename web_search_preview to web_search in Chat Completions API
…bled

Both load_image and reaction are registered in agent_init.go via
IsToolEnabled(), but neither had a case in the switch statement
nor a field in ToolsConfig. This meant they always fell through to
default: return true, making them impossible to disable via config.

Changes:
- Add LoadImage ToolConfig field to ToolsConfig struct
- Add Reaction ToolConfig field to ToolsConfig struct
- Add case 'load_image' and 'reaction' to IsToolEnabled switch
- Set both to Enabled: true in DefaultConfig (preserving old behavior)
- Add config.example.json entries for both tools
- Add tools_test.go covering:
  - Default enabled state
  - Explicit disable/enable toggle
  - All known tools have explicit cases
  - Unknown tools default to enabled

Fixes #2878
@ghost ghost force-pushed the fix/tool-config-load-image-reaction branch from bac6a51 to 61c84f6 Compare May 18, 2026 00:47
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants