Add PR label automation and QA gate workflows by cmeans · Pull Request #2 · cmeans/mcp-synology

cmeans · 2026-04-06T02:28:51Z

Summary

pr-labels.yml — label state machine for PR events (open, push, label/unlabel). Resets to Awaiting CI on new commits, cleans up conflicting labels, handles Dev Active coexistence.
pr-labels-ci.yml — promotes Awaiting CI → Ready for QA on CI pass, sets CI Failed on CI failure. Skips if Dev Active is present.
qa-gate.yml — sets a commit status check ("QA Gate") that blocks merge until QA Approved label is applied.

State flow: Awaiting CI → Ready for QA → QA Active → QA Approved → Merge

Ported from mcp-awareness. Infrastructure change — merge before PR #1.

Test plan

Verify label automation triggers on PR Rename package to mcp-synology, add transfer module and icons #1 after this merges
Confirm QA Gate status check appears on PRs
Verify pr-labels-ci.yml fires on CI completion (requires merge to main first)

🤖 Generated with Claude Code

Port dev/QA pipeline from mcp-awareness: - pr-labels.yml: label state machine for PR events (push, label, unlabel) - pr-labels-ci.yml: CI completion triggers (Awaiting CI → Ready for QA) - qa-gate.yml: QA Approved label as merge-blocking status check State flow: Awaiting CI → Ready for QA → QA Active → QA Approved → Merge Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

codecov-commenter · 2026-04-06T02:30:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

cmeans

QA Review — PR #2: Add PR label automation and QA gate workflows

Code Review

Verified:

pr-labels.yml — logic-identical to mcp-awareness original. Label state machine handles open/push/label/unlabel correctly. Dev Active coexistence preserved. QA invalidation comment on push-during-review works.
pr-labels-ci.yml — logic-identical to mcp-awareness original. workflows: [CI] matches the CI workflow name in ci.yml. PR lookup with dependabot fallback. Dev Active skip logic correct.
qa-gate.yml — logic-identical to mcp-awareness original. Sets pending/success commit status based on QA Approved label presence.
Labels — all referenced labels (Awaiting CI, CI Failed, Dev Active, QA Active, QA Approved, QA Failed, Ready for QA, Ready for QA Signoff) exist in the repo.
License — AGPL headers correctly omitted (mcp-synology is MIT-licensed).
CI — lint ✅, typecheck ✅, test (3.11) ✅, test (3.12) ✅, test (3.13) ✅

Findings

None. Clean port from mcp-awareness.

Test Plan Status

Verify label automation triggers on PR #1 after this merges — post-merge, cannot verify now
Confirm QA Gate status check appears on PRs — verified (QA Gate pending on this PR)
Verify pr-labels-ci.yml fires on CI completion — post-merge (workflow_run runs from default branch)

Result

Pass — zero findings. Ready for signoff.

cmeans · 2026-04-06T21:20:43Z

Adding Ready for QA Signoff — code review complete, zero findings, CI green, QA Gate status confirmed working. Two test plan items are post-merge by design (workflow_run triggers only fire from default branch).

cmeans

LGTM

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>

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>

* 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 added the merge-order: 0 Merge before all others — infrastructure/CI change label Apr 6, 2026

github-actions Bot added the Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA label Apr 6, 2026

cmeans added Ready for QA Dev work complete — QA can begin review QA Active QA is actively reviewing; Dev should not push changes and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review labels Apr 6, 2026

cmeans commented Apr 6, 2026

View reviewed changes

cmeans merged commit 765b5f6 into main Apr 6, 2026
46 checks passed

cmeans mentioned this pull request Apr 6, 2026

Rename package to mcp-synology, add transfer module and icons #1

Merged

7 tasks

cmeans-claude-dev Bot mentioned this pull request Apr 10, 2026

Add structured error responses with JSON envelopes #9

Merged

7 tasks

cmeans mentioned this pull request Apr 11, 2026

Centralize version: sync server.json from pyproject.toml #15

Merged

4 tasks

cmeans deleted the add-pr-workflow-automation branch April 11, 2026 14:26

cmeans mentioned this pull request Apr 11, 2026

Phase 1 of #14: cover CLI and module registration (81% → 85%) #16

Merged

3 tasks

This was referenced Apr 26, 2026

chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow) #58

Merged

fix(filestation): per-path serial for multi-path getinfo and delete #77

Merged

cmeans-claude-dev Bot mentioned this pull request May 2, 2026

Read-modify-write race on ~/.local/state/mcp-synology/global.yaml #93

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add PR label automation and QA gate workflows#2

Add PR label automation and QA gate workflows#2
cmeans merged 1 commit into
mainfrom
add-pr-workflow-automation

cmeans commented Apr 6, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 6, 2026

Uh oh!

cmeans left a comment

Uh oh!

cmeans commented Apr 6, 2026

Uh oh!

cmeans left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cmeans commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

codecov-commenter commented Apr 6, 2026

Codecov Report

Uh oh!

cmeans left a comment

Choose a reason for hiding this comment

QA Review — PR #2: Add PR label automation and QA gate workflows

Code Review

Findings

Test Plan Status

Result

Uh oh!

cmeans commented Apr 6, 2026

Uh oh!

cmeans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cmeans commented Apr 6, 2026 •

edited

Loading