feat: folder operations (change 0012)#6
Open
fx wants to merge 3 commits into
Open
Conversation
Add list_folders / create_folder / delete_folder on REST and MCP so empty
folders (invisible to list_files today) are reachable. Motivated by ~20/34
empty leaves under social-graphs/people/ on prod that ob sync preserves
but the API hides. Introduces a new folder_not_empty error code (409).
- Adds docs/changes/0012-folder-operations.md
- Extends docs/specs/rest-api/index.md with the Folder CRUD section and
the new error code in the closed set
- Extends docs/specs/mcp-server/index.md tool surface with the three
new tools
- Updates docs/index.{yml,md}
Implementation follows on this same branch.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughFolder Operations Specification (RFC/Change Doc)Adds spec and change docs for folder CRUD across REST and MCP; implementation to follow on the same branch. Key Additions
Errors
Decisions & Constraints
Files Changed (docs/spec-only)
Tests & Implementation Notes
WalkthroughAdds a draft "Folder Operations" change document and updates REST and MCP specs and documentation indexes to define list/create/delete folder semantics, error codes (including ChangesFolder Operations Specification and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
Adds the design/spec documentation for Change 0012 (“folder operations”), defining a new folder CRUD surface on both REST and MCP so empty folders (which are currently invisible via file listing) are discoverable and manageable.
Changes:
- Added a REST API “Folder CRUD” section including semantics for listing/creating/deleting folders and a new
folder_not_empty(409) error. - Extended the MCP tool surface with
list_folders,create_folder, anddelete_folderentries mirroring the REST folder routes. - Registered Change 0012 in the docs indexes and introduced the new change doc with requirements, scenarios, and an implementation task list.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/specs/rest-api/index.md | Adds Folder CRUD spec + new error code + changelog entry. |
| docs/specs/mcp-server/index.md | Adds the three new folder tools + description requirements + changelog entry. |
| docs/index.yml | Adds change 0012 metadata entry (draft). |
| docs/index.md | Adds change 0012 row to the changes table (draft). |
| docs/changes/0012-folder-operations.md | New change doc describing motivation, requirements, scenarios, and task checklist for implementation. |
- Replace `list_files` references in the REST spec with the REST endpoint
name (`GET /v1/vaults/:slug/files`) since `list_files` is the MCP tool.
- State symlink omission explicitly in `GET /v1/vaults/:slug/files` so the
spec is internally consistent with the folder-listing rule that points to it.
- Spell out both folder routes in the rest-api changelog row instead of
the ambiguous `[*path]` bracket notation.
- Align `delete_folder` output with `delete_file`: `{ deleted: boolean }`
in both mcp-server spec and 0012.
- Tick the "Spec changelog rows" and "Docs index updates" task boxes
since those landed in the same PR.
Owner
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/changes/0012-folder-operations.md`:
- Around line 17-23: The fenced code block containing the two kubectl shell
commands (the block that begins with "$ kubectl -n ob exec deploy/ob -- sh -c
'find /data/vaults/...") is missing a language tag; update the opening fence to
include "bash" so the block reads ```bash to satisfy MD040 and markdown linting
(no other content changes).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 476193fe-10d4-491b-8b7e-657dd6f55f80
📒 Files selected for processing (5)
docs/changes/0012-folder-operations.mddocs/index.mddocs/index.ymldocs/specs/mcp-server/index.mddocs/specs/rest-api/index.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Plans the folder-operations surface that the file API is missing today. Spec + change doc only in this commit — implementation will land on the same branch after docs review.
list_folders,create_folder,delete_folderon both REST (/v1/vaults/:slug/folders[*path]) and MCP, sharing a newsrc/vault/folders.tsservice-core module.social-graphs/people/(e.g.peter-thiel/) are empty leaves preserved by Obsidian Sync but invisible tolist_files(which only emitsDirent.isFile()). No upstream sync setting purges them — confirmed against Obsidian's sync settings docs.folder_not_empty(409), surfaced whenDELETE /folders/*pathis called without?recursive=trueagainst a non-empty folder.What's in this commit
docs/changes/0012-folder-operations.md— full change doc (requirements, scenarios, tasks, decisions).docs/specs/rest-api/index.md— new### Folder CRUDsection + closed-set error code extension + changelog row.docs/specs/mcp-server/index.md— three new tool rows + description guidance + changelog row.docs/index.{yml,md}— index entries for change 0012.Decisions worth flagging
includeFolders: trueflag onlist_files. Keeps types clean and tool descriptions actionable.create_folderis idempotent (mkdir -psemantics); creating a folder where a file already lives returnsinvalid_path.delete_folderrefuses non-empty folders by default;recursive: trueis required to opt in. Recursive delete walks the subtree, callsindexer.drop(best-effort, chokidar reconciles) for every Markdown descendant, thenfs.rm.Test plan
folder_not_empty(409) is the right code/status for the non-recursive refusal