Skip to content

Fix Claude Desktop config to use uvx#4

Merged
cmeans merged 4 commits into
mainfrom
fix/claude-desktop-config
Apr 7, 2026
Merged

Fix Claude Desktop config to use uvx#4
cmeans merged 4 commits into
mainfrom
fix/claude-desktop-config

Conversation

@cmeans-claude-dev
Copy link
Copy Markdown
Contributor

@cmeans-claude-dev cmeans-claude-dev Bot commented Apr 7, 2026

Summary

  • Setup snippet: changed from uv --directory <cwd> run mcp-synology to uvx mcp-synology — resolves reliably without needing ~/.local/bin in PATH
  • README snippet: updated to match (uvx mcp-synology instead of bare mcp-synology)
  • Migration script: now auto-detects and updates claude_desktop_config.json (Linux/macOS/Windows paths) instead of just printing a manual reminder

Context

After publishing v0.4.0 to PyPI and installing via uv tool install, Claude Desktop failed with ENOENT because the bare mcp-synology command wasn't in its PATH. The uvx approach resolves the package from PyPI without PATH dependencies.

Test plan

  • Run python scripts/migrate-from-synology-mcp.py (dry run) — verify Claude Desktop section appears
  • Run mcp-synology setup — verify generated snippet uses uvx
  • Verify uvx mcp-synology serve --config ... launches correctly

🤖 Generated with Claude Code

The bare `mcp-synology` command fails with ENOENT in Claude Desktop
because ~/.local/bin isn't in its PATH. Switch to `uvx mcp-synology`
which resolves reliably across platforms.

- Setup snippet: use `uvx` instead of `uv --directory ... run`
- README snippet: match the setup output
- Migration script: auto-detect and update claude_desktop_config.json

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA label Apr 7, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added Ready for QA Dev work complete — QA can begin review and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 7, 2026
@cmeans cmeans added QA Active QA is actively reviewing; Dev should not push changes and removed Ready for QA Dev work complete — QA can begin review labels Apr 7, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Review — Round 1

Scope: 3 files changed (+101/−12), single commit. Code review + CI check.

CI Status

All checks pass: lint ✅, typecheck ✅, test (3.11/3.12/3.13) ✅

Code Review

setup.py — Clean change. uv --directory <cwd> runuvx is correct. Removes path dependency. The <path-to-uvx> fallback placeholder is a reasonable UX choice.

README.md — Snippet updated to match. Consistent with setup.py output.

migrate-from-synology-mcp.py — New migrate_claude_desktop_config() function. Detection logic for old-style configs is sound. Two findings:


Findings

1. (substantive) No backup before writing claude_desktop_config.json

migrate_claude_desktop_config modifies the user's Claude Desktop config in place with config_path.write_text(json.dumps(...)). If the write fails mid-stream or the reformatted JSON causes unexpected behavior, the user loses their working config. The rest of this script already handles state carefully (e.g., skip-if-exists logic for directory moves). A .bak copy before writing would be consistent:

if not dry_run:
    backup = config_path.with_suffix(".json.bak")
    shutil.copy2(config_path, backup)
    config_path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8")
    print(f"  SAVED {config_path} (backup: {backup})")

2. (observation) Extra args silently dropped during migration

The migration rebuilds args from scratch as ["mcp-synology", "serve"] + optional --config. If a user had additional flags in their old config (e.g., --verbose, --host, --port), those would be silently dropped. Unlikely in practice for the old synology-mcp package, but worth noting. Could log a warning if len(args) suggests extra flags were present.


Summary

One substantive finding (no backup), one observation. The core logic is correct and the setup.py/README changes are clean.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Adding QA Failed — one substantive finding: migration script should back up claude_desktop_config.json before modifying it in place. See review for details.

@cmeans cmeans added QA Failed QA found issues — needs dev attention and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 7, 2026
- Back up claude_desktop_config.json before modifying (.json.bak)
- Preserve extra args (--verbose, --host, etc.) during migration
  instead of silently dropping them

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed QA Failed QA found issues — needs dev attention Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 7, 2026
@cmeans cmeans added QA Active QA is actively reviewing; Dev should not push changes and removed Ready for QA Dev work complete — QA can begin review labels Apr 7, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Review — Round 2

Commit: e930637 — Address QA findings: backup config and preserve extra args

Verification

# Finding Status
1 No backup before writing claude_desktop_config.json Fixedshutil.copy2 creates .json.bak before write
2 Extra args silently dropped Fixed — new loop preserves non-structural args, logs them

CI remains green. Zero new findings. All Round 1 issues resolved.

Result: Pass — Ready for QA Signoff.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Adding Ready for QA Signoff — Round 2 passed, all findings resolved, CI green, checkboxes updated.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge and removed QA Active QA is actively reviewing; Dev should not push changes Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 7, 2026
- Add prominent migration section near top of README for synology-mcp users
- Standardize Quick Start on uvx (no separate install step needed)
- Move global install to an alternative section
- Update all Claude Desktop snippets and CLI examples to use uvx

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 7, 2026
@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 7, 2026
@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Re-opening review — adding QA Active, removing Ready for QA Signoff. Owner flagged missed findings.

@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 7, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Review — Round 3 (deep re-review)

Re-reviewing from scratch per owner request. Previous rounds caught the backup and extra-args issues, but missed the following:


New Findings

3. (substantive) --config=value equals syntax not handled

The config path extraction (line 187) checks arg == "--config" — this only matches the space-separated form (--config /path). If a user's Claude Desktop config uses the equals form (--config=/path/to/synology-mcp/config.yaml), two things break:

  • config_arg stays None — the synology-mcp to mcp-synology path rename is silently skipped
  • In the extra-args loop (line 198), --config=/path/... is NOT in known_old (the set contains bare --config), so it gets treated as an extra arg
  • Result: ["mcp-synology", "serve", "--config=/path/to/synology-mcp/config.yaml"] — stale path, no rename

Same issue applies to --directory=value form in the extra-args filter. Click supports both forms, so manual configs could plausibly use either.

4. (nit) Stale comment on line 207

            # Skip the --directory value (path)
            extra_args.append(arg)

Comment says "Skip" but the code appends. Leftover from editing — should be removed or rewritten (e.g., # Not a known structural arg — preserve it).

5. (observation) endswith("synology-mcp") is a loose match (line 178)

is_old_direct uses endswith which would match any command ending in synology-mcp (e.g., a hypothetical my-synology-mcp). Path(cmd).name == "synology-mcp" would be more precise. Low risk in practice.


Prior Findings (Round 1-2)

# Severity Finding Status
1 Substantive No backup before config write Fixed in e930637
2 Observation Extra args silently dropped Fixed in e930637

Summary

One new substantive finding (#3 — equals syntax), one nit (#4 — stale comment), one observation (#5 — loose endswith). The backup and extra-args fixes from Round 2 are solid.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Keeping QA Active — Round 3 posted with one new substantive finding (equals syntax in --config/--directory not handled), one nit, one observation.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Adding QA Failed — Round 3 found one new substantive issue (equals syntax in --config/--directory not handled). See review for details.

@cmeans cmeans added QA Failed QA found issues — needs dev attention and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 7, 2026
- Handle --config=/path equals syntax in addition to --config /path
- Handle --directory=/path equals syntax in extra-args filter
- Fix stale "Skip" comment that contradicted the append logic
- Use Path().name for exact command name matching instead of endswith

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed QA Failed QA found issues — needs dev attention Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 7, 2026
@cmeans cmeans added QA Active QA is actively reviewing; Dev should not push changes and removed Ready for QA Dev work complete — QA can begin review labels Apr 7, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Review — Round 4

Commits reviewed: a0bb0d3 (README overhaul), 41f543e (QA round 3 fixes)

Round 3 Finding Verification

# Finding Status
3 --config=value equals syntax not handled Fixed — extraction now handles both --config /path and --config=/path; extra-args filter catches startswith(("--config=", "--directory="))
4 Stale comment on line 207 Fixed — replaced with functional startswith check
5 Loose endswith match Fixed — now Path(cmd).name == "synology-mcp"

README Changes (a0bb0d3)

Good overhaul — uvx is now the primary path, global install demoted to alternative. All CLI examples and JSON snippets consistently use uvx mcp-synology. New migration section at the top with curl one-liner pointing to main branch (will resolve correctly after merge).

CI Status

All checks pass: lint, typecheck, test (3.11/3.12/3.13)

Summary

All findings from rounds 1-3 resolved. Zero new issues. README improvements are clean and consistent.

Result: Pass

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 7, 2026

Adding Ready for QA Signoff — Round 4 passed. All findings from rounds 1-3 resolved, zero new issues, CI green.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge QA Approved Manual QA testing completed and passed and removed QA Active QA is actively reviewing; Dev should not push changes Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 7, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

LGTM

@cmeans cmeans merged commit 3d56e26 into main Apr 7, 2026
34 checks passed
@cmeans-claude-dev cmeans-claude-dev Bot mentioned this pull request Apr 8, 2026
3 tasks
cmeans pushed a commit that referenced this pull request Apr 10, 2026
Findings from QA Round 1 (all substantive):

1. PR body error-code table was missing api_not_found and
   filestation_error. Table update will be applied via gh pr edit in
   the same push. filestation_error now has a dedicated section in
   docs/error-codes.md covering the unmapped-code fall-through path
   (common codes like 402 "System too busy", 900 "Unexpected server
   error") with a link to the Synology API guide for lookup.

2. unavailable had inconsistent retryable semantics: info.py used
   retryable=True at both call sites but utilization.py used
   retryable=False. PR body table says yes. Fixed utilization.py
   (modules/system/utilization.py:162) to retryable=True with a
   comment explaining the rationale (transient, service warming up
   after reboot, monitor not ready).

3. download_file reported local-disk-full with two different codes:
   the pre-flight branch used disk_full/retryable=True but the OSError
   fallback used filesystem_error/retryable=False despite emitting a
   "Free space on the local disk" suggestion. Fixed
   modules/filestation/transfer.py:200 to emit disk_full/retryable=True
   when e.errno == errno.ENOSPC, matching the pre-flight branch.

Also replaced the fragile "if 'space' in error_str.lower()" heuristic
with errno.ENOSPC detection. String matching on error messages is
locale-dependent and brittle across OS versions.

Observation #4 (centralize code names): added class ErrorCode(StrEnum)
in core/errors.py with every code the server can emit. Every
SynologyError subclass now sets error_code to an enum member, and every
bare-string error_response(code="...") call site has been updated to
pass ErrorCode.X. HELP_URLS is now built via a comprehension over
ErrorCode members, so adding a new code automatically registers a help
URL and the test enforces coverage.

StrEnum members are strs, so json serialization, dict lookups, and
equality against string literals all keep working without conversion.

Observation #5 (json.dumps safety): added default=str to the json.dumps
call in error_response() so a future caller passing a non-serializable
value type gets a best-effort string conversion instead of crashing
mid-error-handler.

Additional coverage tests in tests/core/test_help_urls.py:

- TestErrorCodeEnumCoverage.test_every_enum_member_is_registered_or_exempt
  ensures every new ErrorCode member either has a HELP_URLS entry or
  is explicitly exempted.
- test_help_urls_keys_are_all_valid_error_codes catches accidental
  string drift in HELP_URLS keys (typos, renamed codes).

EXEMPT_CODES now holds only SESSION_EXPIRED — filestation_error is
covered by the new docs section, so the exemption is gone.

Not addressed in this round (noted for follow-up):

- F18: operations.py:255 and :361 emit bare dsm_error when a background
  task (copy/move/delete) returns an error dict. The dict often contains
  a known FileStation code (1100 = insufficient filesystem permissions)
  that could be routed through error_from_code() for a more specific
  code. Changes error-response semantics QA should review separately.
- F17: several error_response() call sites omit param/value fields.
  Not a regression, just missed opportunity for structured dispatch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 10, 2026
* Add structured error responses with JSON envelopes

Replace all 35 format_error() return sites with structured JSON error
envelopes raised via ToolError, following the mcp-awareness pattern.

Error responses now include:
- isError=true flag (MCP protocol level)
- code: semantic error type (not_found, permission_denied, etc.)
- message: human-readable with operation context
- retryable: whether the AI should retry
- suggestion: actionable fix guidance
- help_url: Synology KB or Wikipedia reference

Changes:
- core/errors.py: add help_url, error_code, retryable to hierarchy
- core/formatting.py: add error_response() and synology_error_response()
- All 7 tool handler files: replace return format_error() with raises
- All test files: update assertions to pytest.raises(ToolError)
- 8 new tests for error helper functions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Point help_url at targeted error-code reference, add drift test

The previous help_url fields pointed at Synology KB pages that mostly
did not match the error semantics (e.g. auto-block page for bad
credentials, FileStation/connect page for "file not found"). Replace
them with a central HELP_URLS registry in core.errors keyed on error
code, pointing at per-section anchors in a new docs/error-codes.md.

The doc is owned by this repo, so every section can give mcp-synology-
specific guidance ("pass overwrite=true", "run mcp-synology check -v")
that vendor docs can never provide. Sections use the literal error code
as their H2 heading so URL anchors match 1:1 with the code field in the
error envelope.

Also:
- error_response() now looks up HELP_URLS[code] when no explicit
  help_url is passed, so callers do not need to know URLs.
- Explicit help_url argument still wins for per-call-site overrides.
- SessionExpiredError is intentionally omitted from HELP_URLS (auto-
  retried at the core client layer, should never be surfaced).
- New tests/core/test_help_urls.py validates every registered code has
  a matching ## anchor in error-codes.md, every URL points at the
  troubleshooting doc base, anchors equal error codes, no orphan
  sections exist, and every SynologyError subclass is either covered or
  explicitly exempted.
- test_formatting.py updated for the new auto-population behavior:
  previously-implicit "no help_url" case now uses an unregistered code,
  plus new tests for auto-population and explicit override.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address QA Round 1 findings + add ErrorCode StrEnum

Findings from QA Round 1 (all substantive):

1. PR body error-code table was missing api_not_found and
   filestation_error. Table update will be applied via gh pr edit in
   the same push. filestation_error now has a dedicated section in
   docs/error-codes.md covering the unmapped-code fall-through path
   (common codes like 402 "System too busy", 900 "Unexpected server
   error") with a link to the Synology API guide for lookup.

2. unavailable had inconsistent retryable semantics: info.py used
   retryable=True at both call sites but utilization.py used
   retryable=False. PR body table says yes. Fixed utilization.py
   (modules/system/utilization.py:162) to retryable=True with a
   comment explaining the rationale (transient, service warming up
   after reboot, monitor not ready).

3. download_file reported local-disk-full with two different codes:
   the pre-flight branch used disk_full/retryable=True but the OSError
   fallback used filesystem_error/retryable=False despite emitting a
   "Free space on the local disk" suggestion. Fixed
   modules/filestation/transfer.py:200 to emit disk_full/retryable=True
   when e.errno == errno.ENOSPC, matching the pre-flight branch.

Also replaced the fragile "if 'space' in error_str.lower()" heuristic
with errno.ENOSPC detection. String matching on error messages is
locale-dependent and brittle across OS versions.

Observation #4 (centralize code names): added class ErrorCode(StrEnum)
in core/errors.py with every code the server can emit. Every
SynologyError subclass now sets error_code to an enum member, and every
bare-string error_response(code="...") call site has been updated to
pass ErrorCode.X. HELP_URLS is now built via a comprehension over
ErrorCode members, so adding a new code automatically registers a help
URL and the test enforces coverage.

StrEnum members are strs, so json serialization, dict lookups, and
equality against string literals all keep working without conversion.

Observation #5 (json.dumps safety): added default=str to the json.dumps
call in error_response() so a future caller passing a non-serializable
value type gets a best-effort string conversion instead of crashing
mid-error-handler.

Additional coverage tests in tests/core/test_help_urls.py:

- TestErrorCodeEnumCoverage.test_every_enum_member_is_registered_or_exempt
  ensures every new ErrorCode member either has a HELP_URLS entry or
  is explicitly exempted.
- test_help_urls_keys_are_all_valid_error_codes catches accidental
  string drift in HELP_URLS keys (typos, renamed codes).

EXEMPT_CODES now holds only SESSION_EXPIRED — filestation_error is
covered by the new docs section, so the exemption is gone.

Not addressed in this round (noted for follow-up):

- F18: operations.py:255 and :361 emit bare dsm_error when a background
  task (copy/move/delete) returns an error dict. The dict often contains
  a known FileStation code (1100 = insufficient filesystem permissions)
  that could be routed through error_from_code() for a more specific
  code. Changes error-response semantics QA should review separately.
- F17: several error_response() call sites omit param/value fields.
  Not a regression, just missed opportunity for structured dispatch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add missing test coverage for error paths

The structured error refactor introduced new error branches that were
not exercised by the existing test suite, and the system modules were
essentially untested (13% coverage each). Codecov flagged 23 missing
patch lines. This commit closes that gap.

New test file: tests/modules/system/test_info.py (7 tests)
- Happy path with both SYNO.DSM.Info and SYNO.Core.System populated
- Temperature warning flag rendering
- DSM.Info only (Core.System absent from API cache short-circuits)
- Both sources fail → unavailable, retryable=True
- Both sources return empty data → unavailable, retryable=True
- Uptime formatting (days/hours/minutes)
- Uptime under 60 seconds

Module coverage: 13% → 99%

New test file: tests/modules/system/test_utilization.py (6 tests)
- Happy path with full CPU/memory/network/disk payload
- Disk as bare list (alternate DSM response shape)
- api_not_found when SYNO.Core.System.Utilization missing from cache
- DSM code 105 maps to permission_denied (admin required)
- Other DSM errors propagate via synology_error_response
- Empty data payload → unavailable, retryable=True (finding #2 regression test)

Module coverage: 13% → 93%

test_metadata.py additions (4 tests)
- get_file_info with empty files list returns not_found
- get_dir_size start error (DSM code 408 → not_found)
- get_dir_size poll error mid-operation (DSM code 402 → filestation_error,
  also exercises the try/finally cleanup path)
- get_dir_size timeout with tight 1s budget

Module coverage: 86% → 96%

test_operations.py additions (5 tests, new TestBackgroundTaskErrors class)
- Copy timeout (status never returns finished)
- Copy task completes with error dict → dsm_error with embedded DSM code
- Copy poll error mid-operation (DSM code 402)
- Delete timeout
- Delete task completes with error dict

Module coverage: 79% → 91%

test_transfer.py additions (2 tests)
- download_file OSError with errno=ENOSPC → disk_full, retryable=True
  (direct test for the Round 2 errno-based detection introduced by
  finding #3 / F19)
- download_file OSError with non-ENOSPC errno (EACCES) → filesystem_error,
  retryable=False (covers the fall-through branch)

Module coverage: 84% → 85%

Overall patch coverage recovered to >90% on the modified files. Total
project coverage: 73% → 80%. Tests: 281 → 305, all passing. No source
changes — this commit is pure test additions.

The ENOSPC test uses monkeypatch.setattr to replace client.download
with an async function that raises OSError(errno.ENOSPC, ...). Using
monkeypatch rather than a real disk-full condition because we cannot
reliably create ENOSPC in a sandboxed test environment, and the
behavior we care about is the error-dispatch logic in the except
clause, not the underlying OS behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Cover remaining 6 lines in utilization.py (93% → 100%)

Three targeted tests for the branches that weren't exercised:

1. CPU other_load fallback format (lines 49-51): DSM payload where
   CPU reports only ``other_load`` without 15min_load or
   system/user split. Triggers the ``elif "other_load" in cpu:``
   branch in _format_cpu.

2. Memory cached + swap-in detail (lines 73, 76): payload with
   ``cached`` and ``si_disk`` populated. Covers both conditional
   extensions in _format_memory — the "cached" detail append and
   the "Swap in" pair.

3. Disk as unexpected type (line 156): payload with disk=<string>,
   which is neither dict nor list. Exercises the ``else:
   disk_list = []`` fallback, confirming the tool renders without
   disk entries instead of raising.

utilization.py: 92 stmts, 0 missing, 100% coverage.
Project total: 89% → 90%. Tests: 305 → 308.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Cover remaining 6 patch lines flagged by codecov

Codecov reported 6 missing patch lines distributed across 4 files
(not the utilization.py lines I covered in the previous commit —
those were total-module misses, codecov measures patch coverage
against the diff). This commit targets the actual flagged lines:

- tests/modules/filestation/test_listing.py
  + test_list_shares_error: exercises the synology_error_response
    branch in list_shares. Was the only list-shares error case not
    already under test.

- tests/modules/filestation/test_search.py
  + test_search_poll_error_mid_operation: covers the poll_error = e
    branch and the post-cleanup synology_error_response in
    search_files. The task is started successfully but the first
    list (status) call fails, exercising both the break and the
    try/finally task cleanup.

- tests/modules/filestation/test_operations.py
  + test_delete_poll_error_mid_operation: mirrors the copy version
    that already existed. Previously only copy_files' poll-error
    branch was covered; delete_files' identical branch was not.

- tests/modules/system/test_info.py
  + test_unrecognized_fields_produce_no_pairs: distinct from the
    empty-data test. Sources return non-empty dicts containing only
    fields that the extraction code doesn't recognize, so the early
    ``not dsm and not core`` check passes but the late
    ``if not pairs`` branch fires. This is the second
    ``error_response(UNAVAILABLE, ...)`` call site in get_system_info,
    which had no coverage.

Coverage after this commit:
- filestation/listing.py:    95% → 98%
- filestation/operations.py: 91% → 93%
- filestation/search.py:     88% → 91%
- system/info.py:            99% → 100%
- system/utilization.py:     100% (unchanged)

Tests: 308 → 312, all passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Tighten error_response signature to ErrorCode (Round 2 nit)

Addresses QA Round 2 observation #2: ``error_response(code: str)``
accepted arbitrary strings, so a typo at a call site ("not_foundd")
would silently produce an envelope with a missing help_url rather
than a mypy error. The HELP_URLS/ErrorCode/docs drift test covers
all three directions of the registry → code → doc mapping, but it
cannot catch a bare-string typo at the call site.

Changes:

- core/formatting.py: ``code: str`` → ``code: ErrorCode``. A typo
  at a call site is now a mypy error rather than a silent envelope
  with a missing help_url. ``code.value`` is used explicitly in the
  dict and lookup — both would work with bare ``code`` (StrEnum
  members are strings), but the explicit form is unambiguous.

- core/formatting.py: docstring updated to mention the tightened
  type and its rationale.

- tests/core/test_formatting.py: all ``error_response(...)`` calls
  updated to pass ``ErrorCode`` members. ``test_omits_optional_fields_when_none``
  previously used a bogus string ("zzz_unregistered_test_code") to
  exercise the "no help_url" path; it now uses
  ``ErrorCode.SESSION_EXPIRED``, the one ErrorCode member
  intentionally absent from HELP_URLS (auto-retried, never surfaced
  to users). Comment added explaining why that specific member is
  the right choice under the tightened signature.

All 18 ``error_response()`` call sites in src/ were already using
``ErrorCode.X`` from Round 2's centralization work, so no source
call sites needed to change.

Verified:
- uv run mypy src/: Success, no issues found in 27 source files
- uv run mypy --strict tests/core/test_formatting.py: Success
- uv run pytest: 312 passed, 94 deselected
- uv run ruff check: clean
- uv run ruff format --check: clean

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: cmeans-claude-dev[bot] <3223881+cmeans-claude-dev[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cmeans cmeans deleted the fix/claude-desktop-config branch April 11, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants