Skip to content

feat: add memory editor to web ui sidebar#334

Open
Yusufkotavom wants to merge 1 commit into
obot-platform:mainfrom
Yusufkotavom:feature/memory-editor
Open

feat: add memory editor to web ui sidebar#334
Yusufkotavom wants to merge 1 commit into
obot-platform:mainfrom
Yusufkotavom:feature/memory-editor

Conversation

@Yusufkotavom

Copy link
Copy Markdown

This PR adds a memory editor to the Web UI sidebar, allowing users to view, edit, and save memory files (SOUL.md, USER.md, MEMORY.md).

Copilot AI review requested due to automatic review settings June 22, 2026 21:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a “Memory Editor” panel to the Web UI sidebar and introduces backend API endpoints to read/write memory files so users can view, edit, and save SOUL.md, USER.md, and MEMORY.md.

Changes:

  • Added /api/memory/{file} GET/PUT routes.
  • Implemented a new API handler for reading/writing memory files on disk.
  • Added a new MemoryEditor Svelte component and embedded it into the sidebar layout.

Reviewed changes

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

File Description
pkg/api/routes.go Registers new memory GET/PUT API routes.
pkg/api/memory.go Adds handlers to load and persist memory file content.
packages/ui/src/routes/+layout.svelte Adds MemoryEditor to the sidebar and adjusts threads panel sizing.
packages/ui/src/lib/components/MemoryEditor.svelte New UI component for selecting, editing, and saving memory files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/api/memory.go
Comment on lines +20 to +36
func getMemoryFile(w http.ResponseWriter, r *http.Request) {
fileName := r.PathValue("file")
homeDir, err := os.UserHomeDir()
if err != nil {
http.Error(w, "Failed to get user home directory", http.StatusInternalServerError)
return
}

filePath := filepath.Join(homeDir, memoryDir, fileName)
content, err := ioutil.ReadFile(filePath)
if err != nil {
http.Error(w, "Failed to read file", http.StatusInternalServerError)
return
}

json.NewEncoder(w).Encode(FileContent{Content: string(content)})
}
Comment thread pkg/api/memory.go
Comment on lines +3 to +9
import (
"encoding/json"
"io/ioutil"
"net/http"
"os"
"path/filepath"
)
Comment thread pkg/api/memory.go
Comment on lines +38 to +59
func updateMemoryFile(w http.ResponseWriter, r *http.Request) {
fileName := r.PathValue("file")
homeDir, err := os.UserHomeDir()
if err != nil {
http.Error(w, "Failed to get user home directory", http.StatusInternalServerError)
return
}

var fileContent FileContent
if err := json.NewDecoder(r.Body).Decode(&fileContent); err != nil {
http.Error(w, "Invalid request body", http.StatusBadRequest)
return
}

filePath := filepath.Join(homeDir, memoryDir, fileName)
if err := ioutil.WriteFile(filePath, []byte(fileContent.Content), 0644); err != nil {
http.Error(w, "Failed to write file", http.StatusInternalServerError)
return
}

w.WriteHeader(http.StatusOK)
}
Comment thread pkg/api/memory.go
Comment on lines +11 to +14
const (
memoryDir = ".nanobot/workspace/memory"
skillsDir = ".nanobot/workspace/skills"
)
Comment on lines +2 to +7
import { onMount } from 'svelte';
import { writable } from 'svelte/store';

let activeFile = writable('SOUL.md');
let fileContent = writable('');
let files = ['SOUL.md', 'USER.md', 'MEMORY.md'];
Comment on lines +25 to +44
async function saveFileContent() {
try {
const response = await fetch(`/api/memory/${$activeFile}`, {
method: 'PUT',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify({ content: $fileContent })
});
if (!response.ok) {
console.error('Failed to save file');
alert('Failed to save file.');
} else {
alert('File saved successfully.');
}
} catch (error) {
console.error('Error saving file:', error);
alert('Error saving file.');
}
}

async function fetchFileContent(fileName: string) {
try {
const response = await fetch(`/api/memory/${fileName}`);
Comment on lines +5 to +7
let activeFile = writable('SOUL.md');
let fileContent = writable('');
let files = ['SOUL.md', 'USER.md', 'MEMORY.md'];
@entelligence-ai-pr-reviews

Copy link
Copy Markdown

EntelligenceAI PR Summary

Adds end-to-end memory file read/write functionality spanning a new Go API backend and a new Svelte frontend editor component.

  • pkg/api/memory.go: Implements getMemoryFile (reads file) and updateMemoryFile (writes file) handlers targeting ~/.nanobot/workspace/memory; uses deprecated ioutil functions and lacks path traversal sanitization
  • pkg/api/routes.go: Registers GET /api/memory/{file} and PUT /api/memory/{file} routes with API middleware
  • packages/ui/src/lib/components/MemoryEditor.svelte: New tabbed Svelte component with async fetch/save logic for SOUL.md, USER.md, MEMORY.md
  • packages/ui/src/routes/+layout.svelte: Embeds MemoryEditor in sidebar; limits Threads section height to 40%
  • packages/ui/package-lock.json: Locks all transitive dependencies for reproducible installs

Confidence Score: 1/5 - Blocking Issues

Not safe to merge — the {file} path parameter in pkg/api/memory.go is passed directly to ioutil.ReadFile/ioutil.WriteFile without any sanitization, creating a critical path traversal vulnerability that allows any authenticated caller to read or overwrite arbitrary files on the host filesystem (e.g., ../../.ssh/authorized_keys), completely bypassing the intended ~/.nanobot/workspace/memory boundary. While the PR successfully delivers an end-to-end memory editor feature with both a Go API backend and a Svelte frontend component, the security flaw alone is a hard blocker before merge. Additionally, the frontend handleFileSelection in MemoryEditor.svelte silently discards unsaved edits on tab switch, and error handling in getMemoryFile returns HTTP 500 for non-existent files rather than the correct 404, indicating the implementation needs both a security fix and correctness work.

Key Findings:

  • CRITICAL: In pkg/api/memory.go, the {file} route parameter is joined with the memory directory path and passed directly to ioutil.ReadFile/ioutil.WriteFile with no filepath.Clean, strings.Contains('..'), or allowlist check — an attacker can send GET /api/memory/../../.ssh/authorized_keys to read or overwrite sensitive files outside the intended sandbox.
  • BUG: In MemoryEditor.svelte, handleFileSelection immediately fires fetchFileContent when a new tab is clicked, silently overwriting any in-progress textarea edits with no dirty-state check or confirmation prompt — users can unknowingly lose work.
  • BUG: In getMemoryFile, when the target file does not yet exist ioutil.ReadFile returns fs.ErrNotExist, but the handler unconditionally returns HTTP 500 instead of checking the error type and returning a 404; this will break frontend logic that might legitimately initialize an empty file.
  • GUIDELINE VIOLATION: MemoryEditor.svelte uses legacy Svelte writable stores rather than Svelte 5 runes ($state, $derived, $effect) as required by the project's CLAUDE.md, meaning the component is inconsistent with the codebase standard and may need a full rewrite.
Files requiring special attention
  • pkg/api/memory.go
  • packages/ui/src/lib/components/MemoryEditor.svelte

Comment thread pkg/api/memory.go
return
}

filePath := filepath.Join(homeDir, memoryDir, fileName)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL SECURITY Path traversal: unsanitized {file} param allows arbitrary file read/write

The frontend (MemoryEditor.svelte) only sends 3 hardcoded filenames, but the backend uses r.PathValue("file") directly in filepath.Join without sanitization. Any caller can send GET /api/memory/../../.ssh/id_rsa or PUT /api/memory/../../.bashrc to read or overwrite arbitrary files under the user's home directory.

Suggested change
filePath := filepath.Join(homeDir, memoryDir, fileName)
fileName := r.PathValue("file")
if strings.Contains(fileName, "/") || strings.Contains(fileName, "..") {
http.Error(w, "Invalid file name", http.StatusBadRequest)
return
}
allowed := map[string]bool{"SOUL.md": true, "USER.md": true, "MEMORY.md": true}
if !allowed[fileName] {
http.Error(w, "File not allowed", http.StatusForbidden)
return
}
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In pkg/api/memory.go, both getMemoryFile (line 21) and updateMemoryFile (line 39) must validate the `fileName` from PathValue before using it in filepath.Join. Add: (1) a check that fileName contains no `/` or `..` components (filepath.Base trick or explicit Contains check), and (2) an allowlist check against the three permitted filenames ["SOUL.md", "USER.md", "MEMORY.md"]. Return 400/403 on violation. Apply to both handlers.

Comment on lines +3 to +6
import { writable } from 'svelte/store';

let activeFile = writable('SOUL.md');
let fileContent = writable('');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR CORRECTNESS Replace legacy writable stores with Svelte 5 runes

Guideline Violation (CLAUDE.md): "Use Svelte 5 runes ($state, $derived, $effect) rather than legacy store patterns". activeFile and fileContent use writable() from svelte/store; the rest of the codebase (e.g. +layout.svelte) uses $state.

Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In `packages/ui/src/lib/components/MemoryEditor.svelte`, replace the legacy Svelte store pattern with Svelte 5 runes as required by CLAUDE.md. Remove `import { writable } from 'svelte/store'` (line 3). Change line 5 from `let activeFile = writable('SOUL.md')` to `let activeFile = $state('SOUL.md')`. Change line 6 from `let fileContent = writable('')` to `let fileContent = $state('')`. Update all `activeFile.set(x)` calls to `activeFile = x`, all `fileContent.set(x)` calls to `fileContent = x`, and all template references `$activeFile` / `$fileContent` to just `activeFile` / `fileContent`.

Comment on lines +46 to +49
function handleFileSelection(fileName: string) {
activeFile.set(fileName);
fetchFileContent(fileName);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR BUG Unsaved edits silently discarded on tab switch

When a user edits the textarea and clicks a different file tab, handleFileSelection immediately calls fetchFileContent, overwriting fileContent with the newly loaded file's content — the in-progress edits are lost without any warning or save prompt.

Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In `packages/ui/src/lib/components/MemoryEditor.svelte`, fix `handleFileSelection` (lines 46–49) to prevent silent data loss when switching tabs. Add a dirty-state flag (e.g. `let isDirty = $state(false)`) that is set `true` whenever the textarea content changes (bind a change handler or use `$effect` comparing `fileContent` to the last-saved value). In `handleFileSelection`, if `isDirty` is true, prompt the user (e.g. `if (!confirm('You have unsaved changes. Discard them?')) return;`) before proceeding to `fetchFileContent`. Reset `isDirty` to `false` after a successful save in `saveFileContent`.

Comment thread pkg/api/memory.go
Comment on lines +29 to +32
content, err := ioutil.ReadFile(filePath)
if err != nil {
http.Error(w, "Failed to read file", http.StatusInternalServerError)
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR BUG Return 404 for missing memory file instead of 500

ioutil.ReadFile fails with fs.ErrNotExist when the file doesn't exist yet, but the handler returns HTTP 500 unconditionally. The UI receives a server-error status for a normal "file not created yet" state, making it impossible to distinguish a missing file from a real failure.

Suggested change
content, err := ioutil.ReadFile(filePath)
if err != nil {
http.Error(w, "Failed to read file", http.StatusInternalServerError)
return
content, err := ioutil.ReadFile(filePath)
if err != nil {
if os.IsNotExist(err) {
http.Error(w, "File not found", http.StatusNotFound)
} else {
http.Error(w, "Failed to read file", http.StatusInternalServerError)
}
return
}
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In `pkg/api/memory.go`, function `getMemoryFile`, lines 29-32: the error from `ioutil.ReadFile(filePath)` is returned as HTTP 500 for all errors. Add an `os.IsNotExist(err)` check: if true, return `http.Error(w, "File not found", http.StatusNotFound)` instead of 500. This lets the UI correctly handle the case where a memory file has not been created yet.

Comment thread pkg/api/memory.go
return
}

json.NewEncoder(w).Encode(FileContent{Content: string(content)})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT CORRECTNESS GET response missing Content-Type: application/json header

json.NewEncoder(w).Encode(...) in getMemoryFile writes JSON to the body but never sets Content-Type: application/json. The frontend (MemoryEditor.svelte:13) calls response.json() which relies on the browser inferring JSON; some HTTP middleware or proxies may reject or mishandle the untyped response.

Suggested change
json.NewEncoder(w).Encode(FileContent{Content: string(content)})
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(FileContent{Content: string(content)})
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In pkg/api/memory.go getMemoryFile, add `w.Header().Set("Content-Type", "application/json")` before calling json.NewEncoder(w).Encode() on line 35.

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