Fix searchfolder-create's inert default --source_folders (issue #21) - #23
Conversation
…er-create
Graph's mailSearchFolder creation API does not accept a well-known folder name
(e.g. "inbox") verbatim in sourceFolderIds the way destinationId fields elsewhere
in this codebase do — it silently produces a search folder with zero source
folders, whose filter can never match any mail regardless of how much matching
mail exists. Well-known names (including the documented `inbox` default) are now
resolved to real Graph folder ids via GET /me/mailFolders/{name} before creation,
and the verb verifies the created folder's sourceFolderIds actually applied,
refusing loudly instead of reporting a false success if it comes back short.
Fixes #21.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NWPnfcJ9JXDvRThCCUcCH
neilgfoster
left a comment
neilgfoster
left a comment
There was a problem hiding this comment.
Adversarial review (cold pass)
1. Possible false-positive refusal on duplicate source folder names — plugin/src/msgraph/verbs.py (the new applied_ids check after the creation POST): the refusal condition is len(applied_ids) < len(source_ids), a count-only comparison. If a caller passes --source_folders inbox inbox (or a well-known name plus a display name that happens to resolve to the same folder id), source_ids will have 2 entries but Graph will legitimately dedupe and return a sourceFolderIds array of length 1 on the created folder. The verb would then raise SteerError claiming only 1 of 2 requested source folders applied — even though the search folder was created correctly and its filter can match mail fine. This leaves an orphaned-but-functional search folder while telling the operator to go remove it. Comparing distinct/deduped ids (or deduping source_ids before building the request body) would avoid this false positive. Low likelihood in practice (requires a caller to pass overlapping --source_folders values) but worth a quick fix or an explicit acknowledgment that duplicate inputs aren't supported.
2. Minor: the new guard checks cardinality, not identity — if Graph ever returned the same count of ids but a different set (e.g. silently substituting one folder for another), the guard wouldn't catch it. This is consistent with what's actually implemented/described ("refuses loudly ... if it comes back short"), just narrower than the CHANGELOG's phrase "verifies the created folder's sourceFolderIds actually applied" might imply. No action required unless the CHANGELOG wording should be tightened.
No other correctness issues found. The forward-reference from _resolve_source_folder_id to _WELL_KNOWN_FOLDERS (defined later in the same module) is safe — Python resolves module-level names at call time. Test coverage for both the well-known-name and non-well-known display-name resolution paths, plus the short-count refusal case, looks solid.
cmd_searchfolder_create's post-creation short-list check compared len(returned_ids) against len(source_ids) — the raw requested list. A duplicate/aliased --source_folders input (two names resolving to the same folder id) would trip a false-positive SteerError even though the search folder was created correctly, since Graph legitimately dedupes sourceFolderIds. Compare against the count of distinct resolved ids instead. Add a regression test covering duplicate source_folders input resolving to the same id twice.
2 participants
What and why
Fix searchfolder-create's inert default --source_folders (issue #21)
Conventions
urllib/json; ruff/pytest aredev tooling only).
0600).docs/AGENT-FRIENDLY.md(description + CLI I/O are the contract).Mail.Read-only read path, scope ratchet for writes,verify-then-install (read-only catch-set), file-to-folder (never delete).
Verification
ruff check .andruff format --check .pass.python3 -m pytest -qpasses.