Skip to content

fix: move session dirs to ~/.browseros/sessions and update skill paths#494

Merged
Nikhil (shadowfax92) merged 5 commits into
mainfrom
fix/path-in-skills
Mar 12, 2026
Merged

fix: move session dirs to ~/.browseros/sessions and update skill paths#494
Nikhil (shadowfax92) merged 5 commits into
mainfrom
fix/path-in-skills

Conversation

@shadowfax92
Copy link
Copy Markdown
Contributor

Summary

  • Move default session directories from executionDir/sessions/ to ~/.browseros/sessions/ — co-locates session data with other user data (soul, memory, skills)
  • Add 30-day cleanup for stale session directories at server startup (matching memory retention)
  • Update 6 default skills to reference the agent's working directory instead of hardcoding ~/Downloads/

Design

Session directories conceptually belong with user data, not server infrastructure. In production, executionDir is ~/Library/Application Support/BrowserOS/.browseros (app-managed), while ~/.browseros/ is the user-level BrowserOS home. When no workspace is selected, sessions now default to ~/.browseros/sessions/{conversationId}/. User-selected workspaces still override this. executionDir and resourcesDir are unchanged.

Test plan

  • Verify server starts and creates ~/.browseros/sessions/ directory
  • Start a conversation without workspace selected — verify session dir is created under ~/.browseros/sessions/
  • Start a conversation with workspace selected — verify user workspace is used
  • Verify stale session cleanup works (directories older than 30 days removed at startup)
  • Verify bun run typecheck passes
  • Verify bun run lint passes
  • Test skills (deep-research, compare-prices, etc.) output to working directory instead of ~/Downloads/

🤖 Generated with Claude Code

Nikhil (shadowfax92) and others added 2 commits March 12, 2026 14:15
Session directories now live under ~/.browseros/sessions/{conversationId}/
instead of executionDir/sessions/. Adds 30-day cleanup for stale sessions
at server startup. Updates 6 default skills to reference the working
directory instead of hardcoding ~/Downloads/.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shadowfax92
Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

@github-actions github-actions Bot added the fix label Mar 12, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 12, 2026

Greptile Summary

This PR moves session directories from executionDir/sessions/ to ~/.browseros/sessions/ to co-locate them with other user data (memory, skills, soul), adds 30-day mtime-based session cleanup at startup, and renames the internal sessionExecutionDir/executionDir terminology to workingDir throughout the agent and tool layers. Six default skill prompts are also updated to reference the agent's working directory instead of hardcoded ~/Downloads/ paths.

Key changes:

  • getSessionsDir() added to browseros-dir.ts; ensureBrowserosDir now creates ~/.browseros/sessions/
  • cleanOldSessions() removes directories with mtime older than 30 days at server startup
  • utimes touch on every chat message keeps active session directory mtimes fresh (addressing the prior review concern about subdirectory writes not propagating mtime)
  • ResolvedAgentConfig.sessionExecutionDir renamed to workingDir; ToolDirectories.executionDir renamed to workingDir; resolveExecutionPath renamed to resolveWorkingPath
  • MCP server intentionally maps workingDir: deps.executionDir, preserving pre-existing behaviour for external MCP clients
  • The executionDir field in ChatServiceDeps is now dead code — resolveSessionDir calls getSessionsDir() directly and the field is never read

Confidence Score: 4/5

  • This PR is safe to merge — changes are well-scoped renames and a directory relocation with no logic regressions.
  • The rename from sessionExecutionDir/executionDir to workingDir is mechanically consistent and covered by the existing type-checker. The session-dir migration is non-destructive (old dirs under executionDir/sessions/ are simply orphaned, not deleted). The utimes touch correctly addresses the prior mtime concern. Minor issues found: stale LLM-facing tool descriptions, a dead executionDir field in ChatServiceDeps, and a notification-format/folder-name emoji inconsistency in the read-later skill — none of these are blockers.
  • apps/server/src/tools/page-actions.ts (stale cwd descriptions visible to the LLM) and apps/server/src/api/services/chat-service.ts (dead executionDir field).

Important Files Changed

Filename Overview
apps/server/src/lib/browseros-dir.ts Adds getSessionsDir(), ensureBrowserosDir now creates the sessions dir, and introduces cleanOldSessions() for 30-day mtime-based cleanup at startup. The utimes touch in chat-service.ts addresses the mtime-not-propagating concern raised in a prior review thread.
apps/server/src/api/services/chat-service.ts Session dir now resolves to ~/.browseros/sessions/{conversationId} via getSessionsDir(). Adds utimes touch on every message to keep active session mtimes fresh. The executionDir field in ChatServiceDeps is now dead code — it is declared but never read by the service.
apps/server/src/tools/page-actions.ts Renames resolveExecutionPathresolveWorkingPath and executionDirworkingDir throughout. The cwd parameter descriptions for all three tools still say "defaults to the execution directory" — stale LLM-facing text that should be updated.
apps/server/src/tools/framework.ts Clean rename of ToolDirectories.executionDirworkingDir and resolveExecutionPathresolveWorkingPath. No logic changes.
apps/server/src/api/services/mcp/mcp-server.ts Maps workingDir: deps.executionDir — intentionally preserves the app-managed execution directory as the MCP working directory, separate from the new user-level session dirs used by chat.
apps/server/src/skills/defaults/read-later/SKILL.md PDF now saves to working directory and the "📚 Read Later" emoji is restored in the description, but the Notification Format template still omits the emoji ("Saved to Read Later"), creating an inconsistency with the folder name.
packages/shared/src/constants/paths.ts Adds SESSIONS_DIR_NAME: 'sessions' and SESSION_RETENTION_DAYS: 30 constants. Clean change, consistent with existing MEMORY_RETENTION_DAYS.
apps/server/src/agent/types.ts Renames sessionExecutionDirworkingDir in ResolvedAgentConfig. Clean rename with no logic changes.
apps/server/src/main.ts Adds cleanOldSessions() call during initCoreServices() at server startup. Straightforward addition after ensureBrowserosDir().

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Server Startup] --> B[ensureBrowserosDir\ncreates ~/.browseros/sessions/]
    B --> C[cleanOldSessions\nremove dirs with mtime < 30 days]

    D[Chat Request] --> E{userWorkingDir set?}
    E -- Yes --> F[Use user workspace path]
    E -- No --> G[~/.browseros/sessions/conversationId/]
    F --> H[mkdir recursive]
    G --> H
    H --> I{userWorkingDir?}
    I -- No --> J[utimes touch dir\nupdates mtime to now]
    I -- Yes --> K[Skip touch]
    J --> L[workingDir passed to Agent]
    K --> L

    M[MCP Server] --> N[workingDir = executionDir\napp-managed directory]

    L --> O[Tools: save_pdf\nsave_screenshot\ndownload_file\nresolve paths against workingDir]
    N --> O
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/server/src/tools/page-actions.ts
Line: 21

Comment:
**Stale LLM-facing `cwd` description still says "execution directory"**

The `cwd` parameter descriptions for `save_pdf`, `save_screenshot`, and `download_file` still say "defaults to the execution directory" after the rename to `workingDir`. The LLM reads these descriptions to reason about tool behaviour, so using the old term is misleading. All three occurrences should be updated:

- `save_pdf` cwd description (line 21)
- `save_screenshot` cwd description (line 54)  
- `download_file` cwd description (line 111)

```suggestion
        'Working directory to resolve relative paths against; defaults to the working directory',
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/server/src/api/services/chat-service.ts
Line: 25

Comment:
**`executionDir` in `ChatServiceDeps` is now dead code**

After this PR, `resolveSessionDir` calls `getSessionsDir()` directly and never reads `this.deps.executionDir`. The field is still declared in the interface and populated in `chat.ts` (line 34), but it is never consumed. Consider removing it to avoid confusion about what directory the service actually uses.

```suggestion
export interface ChatServiceDeps {
  sessionStore: SessionStore
  klavisClient: KlavisClient
  browser: Browser
  registry: ToolRegistry
  browserosId?: string
}
```

The corresponding `executionDir` assignment in `apps/server/src/api/routes/chat.ts:34` and the `executionDir?: string` field in `ChatRouteDeps` (line 20) would also need to be removed.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/server/src/skills/defaults/read-later/SKILL.md
Line: 32

Comment:
**Notification format missing emoji — inconsistent with the folder name**

The description (line 3), the workflow table (lines 23–24), and the Tips section (line 48) all use `"📚 Read Later"` as the folder name. The Notification Format template at line 32 still shows `"Saved to Read Later"` (without the emoji), which will cause the agent-reported confirmation message to differ from the actual bookmark folder name.

```suggestion
Saved to 📚 Read Later
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: aa87c4e

Comment on lines +23 to +26
| Check for folder | `get_bookmarks` | Look for an existing folder named "Read Later" in the bookmark bar |
| Create folder (if needed) | `create_bookmark` | If the folder doesn't exist, create "Read Later" in the bookmark bar |
| Add bookmark | `create_bookmark` | Save the current page URL and title into the "Read Later" folder |
| Save PDF | `save_pdf` | Download the full page as a PDF to the working directory |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Emoji removal may create duplicate bookmark folders for existing users

The bookmark folder name is being changed from "📚 Read Later" to "Read Later". Existing users who already have a "📚 Read Later" folder from the old skill would see the agent fail to find it (the check at line 23 looks for "Read Later"), and subsequently create a new, duplicate folder named "Read Later" alongside the existing one.

Since get_bookmarks + create_bookmark is a purely name-based lookup, there is no automatic migration. Users with the old emoji folder will end up with two separate Read Later folders until they manually reconcile them. Consider whether this rename is intentional and, if so, whether the workflow should also check for the legacy "📚 Read Later" folder name and rename or merge it.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/skills/defaults/read-later/SKILL.md
Line: 23-26

Comment:
**Emoji removal may create duplicate bookmark folders for existing users**

The bookmark folder name is being changed from `"📚 Read Later"` to `"Read Later"`. Existing users who already have a `"📚 Read Later"` folder from the old skill would see the agent fail to find it (the check at line 23 looks for `"Read Later"`), and subsequently create a **new, duplicate** folder named `"Read Later"` alongside the existing one.

Since `get_bookmarks` + `create_bookmark` is a purely name-based lookup, there is no automatic migration. Users with the old emoji folder will end up with two separate Read Later folders until they manually reconcile them. Consider whether this rename is intentional and, if so, whether the workflow should also check for the legacy `"📚 Read Later"` folder name and rename or merge it.

How can I resolve this? If you propose a fix, please make it concise.

const entryPath = join(sessionsDir, entry)
try {
const info = await stat(entryPath)
if (info.isDirectory() && info.mtimeMs < cutoff) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

mtime-based staleness may not reflect recent file-write activity

The cleanup uses info.mtimeMs (directory modification time) to decide whether to remove a session directory. On Linux and macOS, a directory's mtime is updated only when entries are directly added to or removed from that directory — it is not updated when files inside subdirectories are modified.

Consider this sequence:

  1. Day 0 — session dir ~/.browseros/sessions/<id>/ is created; agent writes research-topic/sources/source.md.
    The session dir's mtime is updated when research-topic/ is created inside it.
  2. Day 1-30 — the agent only modifies content inside research-topic/ (no new entries in the session dir itself).
    The session dir's mtime stays at Day 0.
  3. Day 31 — server restarts; the session dir is removed because mtime < cutoff.

Additionally, mkdir(dir, { recursive: true }) in resolveSessionDir is a no-op when the directory already exists, so repeated messages in the same conversation do not refresh the mtime.

If the intent is truly "30 days since last write activity", consider using ctimeMs (last metadata change, including deep writes on some systems) or updating a sentinel file inside the session dir on each message to keep mtime fresh. If the intent is "30 days since the session dir was last structurally changed", the current behaviour is fine — just worth documenting.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/lib/browseros-dir.ts
Line: 53

Comment:
**`mtime`-based staleness may not reflect recent file-write activity**

The cleanup uses `info.mtimeMs` (directory modification time) to decide whether to remove a session directory. On Linux and macOS, a directory's `mtime` is updated only when entries are directly **added to or removed from** that directory — it is **not** updated when files inside subdirectories are modified.

Consider this sequence:
1. Day 0 — session dir `~/.browseros/sessions/<id>/` is created; agent writes `research-topic/sources/source.md`.  
   The session dir's `mtime` is updated when `research-topic/` is created inside it.
2. Day 1-30 — the agent only modifies content inside `research-topic/` (no new entries in the session dir itself).  
   The session dir's `mtime` stays at Day 0.
3. Day 31 — server restarts; the session dir is removed because `mtime < cutoff`.

Additionally, `mkdir(dir, { recursive: true })` in `resolveSessionDir` is a no-op when the directory already exists, so repeated messages in the same conversation do not refresh the `mtime`.

If the intent is truly "30 days since last write activity", consider using `ctimeMs` (last metadata change, including deep writes on some systems) or updating a sentinel file inside the session dir on each message to keep `mtime` fresh. If the intent is "30 days since the session dir was last structurally changed", the current behaviour is fine — just worth documenting.

How can I resolve this? If you propose a fix, please make it concise.

Nikhil (shadowfax92) and others added 2 commits March 12, 2026 15:29
Consistent naming for the per-conversation working directory:
- ResolvedAgentConfig.sessionExecutionDir → workingDir
- ToolDirectories.executionDir → workingDir
- resolveExecutionPath() → resolveWorkingPath()
- buildBrowserToolSet param: executionDir → workingDir

Server-level executionDir (DB, logs) unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…time

- Revert "Read Later" back to "📚 Read Later" to avoid creating
  duplicate bookmark folders for existing users
- Touch session dir mtime on each message via utimes() so cleanup
  correctly reflects last activity, not just directory creation time

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shadowfax92
Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

- Remove executionDir from ChatServiceDeps and ChatRouteDeps since
  resolveSessionDir now uses getSessionsDir() directly
- Fix missed emoji in notification format template

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shadowfax92 Nikhil (shadowfax92) merged commit 8a38e90 into main Mar 12, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant