mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(memory): reject absolute filesystem paths with corrective routing (#934)
* ci(staging): use default branch instead of hardcoded main * fix(memory): route absolute paths to filesystem tools
This commit is contained in:
@@ -44,6 +44,7 @@ jobs:
|
||||
id: check
|
||||
env:
|
||||
FORCE_RUN: ${{ inputs.force }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
run: |
|
||||
CURRENT_HEAD=$(git rev-parse HEAD)
|
||||
echo "current_head=${CURRENT_HEAD}" >> "$GITHUB_OUTPUT"
|
||||
@@ -65,8 +66,8 @@ jobs:
|
||||
echo "Found ${COMMIT_COUNT} new commit(s) since last tested"
|
||||
DIFF_RANGE="${LAST_TESTED}..${CURRENT_HEAD}"
|
||||
else
|
||||
git fetch origin main
|
||||
MERGE_BASE=$(git merge-base origin/main HEAD)
|
||||
git fetch origin "${DEFAULT_BRANCH}"
|
||||
MERGE_BASE=$(git merge-base "origin/${DEFAULT_BRANCH}" HEAD)
|
||||
echo "First run -- reviewing from merge-base ${MERGE_BASE}"
|
||||
DIFF_RANGE="${MERGE_BASE}..${CURRENT_HEAD}"
|
||||
fi
|
||||
@@ -129,18 +130,19 @@ jobs:
|
||||
echo "token=${{ github.token }}" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
- name: Check if staging is ahead of main
|
||||
- name: Check if staging is ahead of target branch
|
||||
id: ahead-check
|
||||
env:
|
||||
GH_TOKEN: ${{ steps.token.outputs.token }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
run: |
|
||||
git fetch origin main
|
||||
AHEAD=$(git rev-list --count origin/main..origin/staging)
|
||||
git fetch origin "${DEFAULT_BRANCH}"
|
||||
AHEAD=$(git rev-list --count "origin/${DEFAULT_BRANCH}..origin/staging")
|
||||
echo "commits_ahead=${AHEAD}" >> "$GITHUB_OUTPUT"
|
||||
if [ "$AHEAD" -eq 0 ]; then
|
||||
echo "Staging is not ahead of main. Nothing to promote."
|
||||
echo "Staging is not ahead of ${DEFAULT_BRANCH}. Nothing to promote."
|
||||
else
|
||||
echo "Staging is ${AHEAD} commits ahead of main."
|
||||
echo "Staging is ${AHEAD} commits ahead of ${DEFAULT_BRANCH}."
|
||||
fi
|
||||
|
||||
- name: Create promotion branch
|
||||
@@ -159,6 +161,7 @@ jobs:
|
||||
if: steps.ahead-check.outputs.commits_ahead != '0'
|
||||
env:
|
||||
GH_TOKEN: ${{ steps.token.outputs.token }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
run: |
|
||||
# Find the newest open promotion PR with a staging-promote/* head branch
|
||||
LATEST=$(gh pr list --label staging-promotion --state open \
|
||||
@@ -168,8 +171,8 @@ jobs:
|
||||
echo "base=${LATEST}" >> "$GITHUB_OUTPUT"
|
||||
echo "Chaining onto existing promotion branch: ${LATEST}"
|
||||
else
|
||||
echo "base=main" >> "$GITHUB_OUTPUT"
|
||||
echo "No existing promotion PR — targeting main"
|
||||
echo "base=${DEFAULT_BRANCH}" >> "$GITHUB_OUTPUT"
|
||||
echo "No existing promotion PR — targeting ${DEFAULT_BRANCH}"
|
||||
fi
|
||||
|
||||
- name: Create promotion PR
|
||||
@@ -186,7 +189,7 @@ jobs:
|
||||
PR_URL=$(gh pr create \
|
||||
--base "$BASE" \
|
||||
--head "$BRANCH" \
|
||||
--title "chore: promote staging to main (${TIMESTAMP})" \
|
||||
--title "chore: promote staging to ${BASE} (${TIMESTAMP})" \
|
||||
--body "## Auto-promotion from staging CI
|
||||
|
||||
**Batch range:** \`${RANGE}\`
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
//! Use `memory_write` to persist important facts that should be remembered
|
||||
//! across sessions.
|
||||
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
@@ -26,6 +27,28 @@ use crate::workspace::{Workspace, paths};
|
||||
const PROTECTED_IDENTITY_FILES: &[&str] =
|
||||
&[paths::IDENTITY, paths::SOUL, paths::AGENTS, paths::USER];
|
||||
|
||||
/// Detect paths that are clearly local filesystem references, not workspace-memory docs.
|
||||
///
|
||||
/// Examples:
|
||||
/// - `/Users/.../file.md` (Unix absolute)
|
||||
/// - `C:\Users\...` or `D:/work/...` (Windows absolute)
|
||||
/// - `~/notes.md` (home expansion shorthand)
|
||||
fn looks_like_filesystem_path(path: &str) -> bool {
|
||||
if path.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
if Path::new(path).is_absolute() || path.starts_with("~/") {
|
||||
return true;
|
||||
}
|
||||
|
||||
let bytes = path.as_bytes();
|
||||
bytes.len() >= 3
|
||||
&& bytes[0].is_ascii_alphabetic()
|
||||
&& bytes[1] == b':'
|
||||
&& (bytes[2] == b'\\' || bytes[2] == b'/')
|
||||
}
|
||||
|
||||
/// Tool for searching workspace memory.
|
||||
///
|
||||
/// Performs hybrid search (FTS + semantic) across all memory documents.
|
||||
@@ -143,7 +166,8 @@ impl Tool for MemoryWriteTool {
|
||||
be remembered across sessions. Targets: 'memory' for curated long-term facts, \
|
||||
'daily_log' for timestamped session notes, 'heartbeat' for the periodic \
|
||||
checklist (HEARTBEAT.md), 'bootstrap' to clear the first-run ritual file, \
|
||||
or provide a custom path for arbitrary file creation."
|
||||
or provide a custom workspace path for arbitrary file creation. \
|
||||
Never pass absolute filesystem paths like '/Users/...' or 'C:\\...'."
|
||||
}
|
||||
|
||||
fn parameters_schema(&self) -> serde_json::Value {
|
||||
@@ -183,6 +207,14 @@ impl Tool for MemoryWriteTool {
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("daily_log");
|
||||
|
||||
if looks_like_filesystem_path(target) {
|
||||
return Err(ToolError::InvalidParameters(format!(
|
||||
"'{}' looks like a local filesystem path. memory_write only works with workspace-memory paths. \
|
||||
Use write_file for filesystem writes. For opening files in an editor, use shell with: open \"<absolute_path>\".",
|
||||
target
|
||||
)));
|
||||
}
|
||||
|
||||
// Bootstrap target: clear BOOTSTRAP.md to mark first-run ritual complete.
|
||||
// Handled early because it accepts empty content (unlike other targets).
|
||||
if target == "bootstrap" {
|
||||
@@ -332,7 +364,8 @@ impl Tool for MemoryReadTool {
|
||||
fn description(&self) -> &str {
|
||||
"Read a file from the workspace memory (database-backed storage). \
|
||||
Use this to read files shown by memory_tree. NOT for local filesystem files \
|
||||
(use read_file for those). Works with identity files, heartbeat checklist, \
|
||||
(use read_file for those). Do not pass absolute paths like '/Users/...' or 'C:\\...'. \
|
||||
Works with identity files, heartbeat checklist, \
|
||||
memory, daily logs, or any custom workspace path."
|
||||
}
|
||||
|
||||
@@ -358,6 +391,14 @@ impl Tool for MemoryReadTool {
|
||||
|
||||
let path = require_str(¶ms, "path")?;
|
||||
|
||||
if looks_like_filesystem_path(path) {
|
||||
return Err(ToolError::InvalidParameters(format!(
|
||||
"'{}' looks like a local filesystem path. memory_read only works with workspace-memory paths. \
|
||||
Use read_file for filesystem reads. For opening files in an editor, use shell with: open \"<absolute_path>\".",
|
||||
path
|
||||
)));
|
||||
}
|
||||
|
||||
let doc = self
|
||||
.workspace
|
||||
.read(path)
|
||||
@@ -379,6 +420,26 @@ impl Tool for MemoryReadTool {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod path_routing_tests {
|
||||
use super::looks_like_filesystem_path;
|
||||
|
||||
#[test]
|
||||
fn detects_filesystem_paths() {
|
||||
assert!(looks_like_filesystem_path("/Users/nige/file.md"));
|
||||
assert!(looks_like_filesystem_path("C:\\Users\\nige\\file.md"));
|
||||
assert!(looks_like_filesystem_path("D:/work/file.md"));
|
||||
assert!(looks_like_filesystem_path("~/notes.md"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allows_workspace_memory_paths() {
|
||||
assert!(!looks_like_filesystem_path("MEMORY.md"));
|
||||
assert!(!looks_like_filesystem_path("daily/2026-03-11.md"));
|
||||
assert!(!looks_like_filesystem_path("projects/alpha/notes.md"));
|
||||
}
|
||||
}
|
||||
|
||||
/// Tool for viewing workspace structure as a tree.
|
||||
///
|
||||
/// Returns a hierarchical view of files and directories with configurable depth.
|
||||
|
||||
Reference in New Issue
Block a user