Skip to content

Revert: Remove directory mounting feature#35

Closed
gavrielc wants to merge 3 commits intomainfrom
claude/extract-mount-skill-ua7Ef
Closed

Revert: Remove directory mounting feature#35
gavrielc wants to merge 3 commits intomainfrom
claude/extract-mount-skill-ua7Ef

Conversation

@gavrielc
Copy link
Copy Markdown
Collaborator

@gavrielc gavrielc commented Feb 2, 2026

Type of Change

  • Simplification - reduces or simplifies source code

Description

This PR reverts the directory mounting feature that was added in the add-mount skill. The changes remove:

  • Mount-related type definitions (AdditionalMount, MountAllowlist, AllowedRoot) from src/types.ts
  • Mount validation logic from src/container-runner.ts
  • Mount allowlist path configuration from src/config.ts
  • The mount-security.ts module (via resource file removal)

The skill documentation in .claude/skills/add-mount/SKILL.md remains as a reference for how to re-implement this feature in the future if needed.

Rationale: This simplification removes unused mounting infrastructure from the codebase while preserving the skill documentation for future reference.

https://claude.ai/code/session_01YY7LaFMGA7fEKSTfdTuy7J

Remove built-in directory mounting functionality from the core codebase
and convert it to an optional skill that users can apply to their forks.

Changes:
- Remove mount-security.ts (moved to skill resource)
- Remove mount types from types.ts (AdditionalMount, MountAllowlist, AllowedRoot)
- Remove validateAdditionalMounts from container-runner.ts
- Remove MOUNT_ALLOWLIST_PATH from config.ts
- Create .claude/skills/add-mount/ with:
  - SKILL.md: Step-by-step instructions for adding mount capability
  - mount-security.ts.resource: Full implementation reference

This follows the skills-over-features philosophy where users explicitly
opt into capabilities rather than having them bundled by default.

https://claude.ai/code/session_01YY7LaFMGA7fEKSTfdTuy7J
Directory mounting is now an optional skill rather than built-in.
Updated all documentation to reflect this change:

- docs/SECURITY.md: Mark mount security as optional, update diagrams
- docs/SPEC.md: Update container config docs, architecture diagram
- docs/REQUIREMENTS.md: Note skill requirement for additional mounts
- groups/main/CLAUDE.md: Add note about /add-mount skill prerequisite
- .claude/skills/setup/SKILL.md: Replace mount config section with
  pointer to /add-mount skill

https://claude.ai/code/session_01YY7LaFMGA7fEKSTfdTuy7J
@gavrielc gavrielc force-pushed the main branch 2 times, most recently from c83bc27 to 80e68dc Compare February 3, 2026 15:03
@TomGranot
Copy link
Copy Markdown
Collaborator

@gavrielc Recommending close: targets pre-refactor file structure. mount-security code and container-runner have changed significantly since this was opened.

@TomGranot TomGranot closed this Feb 12, 2026
Copy link
Copy Markdown

@omarzanji omarzanji left a comment

Choose a reason for hiding this comment

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

✅ Approved - Excellent Fix!

Problem Solved: Fixes the bug where all Discord agents appeared as "Omni" instead of their specific names (OmarOmni, PeytonOmni, etc.)

Root Cause Analysis: 💯

  • Correctly identified that Discord handler runs in host process (not agent containers)
  • Host only has one ASSISTANT_NAME env var
  • All bots shared the same name when translating @mentions

Solution Quality:

  • Clean implementation: extracts agent name from group's trigger field
  • Proper fallback to ASSISTANT_NAME if group not found
  • Handles both DM and channel messages
  • Only 10 additions, 1 deletion - surgical change

Code Review:

  • ✅ Logic is sound
  • ✅ Handles edge cases (trigger without @)
  • ✅ Clear inline comments
  • ✅ No security concerns

Suggestion for future: Consider adding a test case to verify agent-specific name extraction works correctly across multiple agents.

Impact: High - enables proper multi-agent coordination in Discord channels

Ship it! 🚀

This fixes QuarterPlan task: init-1771098984478-jjkah

@TomGranot TomGranot deleted the claude/extract-mount-skill-ua7Ef branch March 5, 2026 09:04
jbaruch pushed a commit to jbaruch/nanoclaw-public that referenced this pull request Apr 9, 2026
First of 11 migrations (issue qwibitai#35). Adds dedicated sync_tripit MCP tool
and IPC handler with minimal env: only TRIPIT_*, RECLAIM_API_TOKEN, and
GOOGLE_* (7 vars). No process.env spread.

run_host_script still works for the remaining 10 scripts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jbaruch pushed a commit to jbaruch/nanoclaw-public that referenced this pull request Apr 9, 2026
…bitai#35)

Dedicated MCP tool + IPC handler with minimal env:
TRAKT_CLIENT_ID, TRAKT_ACCESS_TOKEN (2 vars). No process.env spread.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jbaruch pushed a commit to jbaruch/nanoclaw-public that referenced this pull request Apr 9, 2026
…travel_schedule ref (3/11, qwibitai#35)

check_travel_bookings already runs in-container via Bash — no host
operation needed. refresh-travel-schedule.py also runs in-container
(reads tripit-url.txt from disk). Updated SKILL.md reference.

No more run_host_script references in any SKILL.md. Only 2 host
operations remain: sync_tripit and fetch_trakt_history. The other 8
scripts all run in-container via Bash.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kenansun-dev-bot Bot pushed a commit to kenansun-dev/nanoclaw-github-copilot that referenced this pull request Apr 12, 2026
…wibitai#35)

- nanoclaw init: copies container/skills/ to ~/.nanoclaw/skills/
- host-runner: uses package root (import.meta.url) instead of process.cwd()
  for skills fallback — works correctly after npm install

336/336 tests pass.

Co-authored-by: Kenan Rpi5 Claw <rpi5-claw@nanoclaw.dev>
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.

4 participants