chore: community health files (CONTRIBUTING, CoC, SECURITY)

[cmeans-claude-dev]

Summary

Adds the three standard GitHub community-health files. mcp-synology already had .github/ISSUE_TEMPLATE/ and .github/FUNDING.yml; this fills in the remaining slots: CONTRIBUTING.md, CODE_OF_CONDUCT.md, SECURITY.md. Also rolls forward the now-merged .github/PULL_REQUEST_TEMPLATE.md (from #58) so the template, the CONTRIBUTING.md PR-body example, and CI all use the same commands and placeholders.

Pattern ported from cmeans/pypi-winnow-downloads#20.

Round 3 → Round 4 deltas (5c4cb2d → 43e5993)

Addressing QA round 2:

• F1-r2 (substantive) — SECURITY.md referenced an is_session_error helper that doesn't exist in src/. Rewrote the session-error retry bullet to name only real symbols: the 100-series lookup table and SessionExpiredError (produced by error_from_code()) in errors.py, and _SESSION_ERROR_CODES plus its call sites in client.py. Avoided pinning to specific line numbers since they drift under refactor. Same drift class as round-1 F2 — a doc symbol that doesn't appear in the codebase.
• O2-r2 (observation) — CONTRIBUTING.md PR-body example used test_<module>.py while the PR template uses test_<file>.py. Aligned CONTRIBUTING.md to match the template.

Round 2 → Round 3 deltas (reference, already addressed)

• Cross-PR alignment: fixed .github/PULL_REQUEST_TEMPLATE.md (merged via chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow) #58) to add scripts/ to ruff/mypy lines, use tests/modules/<area>/... path, and match the ## CHANGELOG wording with CONTRIBUTING.md.

Round 1 → Round 2 deltas (reference)

• F1: 6 scripts/ occurrences fixed in CONTRIBUTING.md.
• F2: SECURITY.md split — auth bullet vs session-error retry bullet (now correctly attributed to core/client.py + core/errors.py).
• F3: checkboxes ticked.
• O4: stale "PVR off" warning removed.
• O5: ## CHANGELOG section added to CONTRIBUTING.md PR-body example.

Test plan

• grep -n is_session_error src/ tests/ returns no matches — bullet now names only real symbols (SessionExpiredError in errors.py:96, error_from_code() at errors.py:264, _SESSION_ERROR_CODES in client.py:29 plus call sites at L308/L410/L512)
• CONTRIBUTING.md PR-body example placeholder matches .github/PULL_REQUEST_TEMPLATE.md — both use tests/modules/<area>/test_<file>.py::test_<name>
• uv run ruff check src/ tests/ scripts/, uv run ruff format --check src/ tests/ scripts/, uv run mypy src/ scripts/ — all match CI commands
• PVR is enabled: true (gh api .../private-vulnerability-reporting)
• CHANGELOG entry under ## Unreleased → ### Added references this PR (chore: community health files (CONTRIBUTING, CoC, SECURITY) #56) and notes the PR-template alignment

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cmeans]

QA Round 1 — PR #56

Solid scaffolding work, well-grounded in the actual codebase. Most of the doc claims check out against src/. Three substantive findings — two are doc drift against the live repo / live CI, one is the standard test-plan-ticking item. Two observations.

Verification on f40bc0e

Check	Result
uv run pytest	499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	69 files clean
uv run mypy src/ scripts/	clean (29 source files)
python3 scripts/sync-server-json.py --check	in sync (0.5.0)
Remote CI	all green (lint, typecheck, validate-server-json, version-sync, test 3.11/3.12/3.13, vdsm)
gh api repos/cmeans/mcp-synology/private-vulnerability-reporting	{"enabled":true} — PVR is already on

Codebase-claim audit (factual checks against src/)

Claim location	Claim	Verdict
CONTRIBUTING.md:64	tests/modules/filestation/test_listing.py exists	✅ exists
CONTRIBUTING.md:65	test_list_shares test exists	✅ test_list_shares_* in test_listing.py
CONTRIBUTING.md:74-77	tests/integration_config.yaml.example exists	✅ exists
CONTRIBUTING.md:96-97	mirror src/...listing.py ↔ tests/...test_listing.py	✅ matches
SECURITY.md:40-41	mcp-synology --version works	✅ cli/version.py
SECURITY.md:46-47	tests/vdsm/ exists	✅ exists
SECURITY.md:64-79	core/auth.py, core/client.py, modules/filestation/ exist	✅ all exist
SECURITY.md:83-86	DirSize, CopyMove, Delete, Search async tasks with try/finally cleanup	✅ confirmed in metadata.py:30,38,40, search.py:41,43, operations.py:34
SECURITY.md:94-96	publish-registry job in .github/workflows/publish.yml	✅ exists at line 51
SECURITY.md:101-102	pr-labels-ci.yml env: pattern	✅ landed in #53
CODE_OF_CONDUCT.md:28 + SECURITY.md:18-29	PVR is enabled on the repo	✅ confirmed via API

Findings

1. [substantive — doc drift] CONTRIBUTING.md lint/format/typecheck commands omit scripts/, but CI checks it.

CI (.github/workflows/ci.yml:21,22,50) runs:

• uv run ruff check src/ tests/ scripts/
• uv run ruff format --check src/ tests/ scripts/
• uv run mypy src/ scripts/

CONTRIBUTING.md tells contributors:

• L66: uv run ruff check src/ tests/
• L67: uv run ruff format --check src/ tests/
• L68: uv run mypy src/
• L105-106: uv run ruff check src/ tests/ and uv run mypy src/
• L126: uv run ruff check src/ tests/
• L127: uv run mypy src/

A contributor who follows the doc literally gets a clean local run and then a red CI on scripts/. Suggest replacing all 6 occurrences with the CI form (src/ tests/ scripts/ for ruff; src/ scripts/ for mypy).

2. [substantive — doc drift] SECURITY.md attributes the session-error retry logic to auth.py, but it lives in client.py and errors.py.

Line 64-68 lists, under core/auth.py:

... and the lazy keepalive / re-auth logic on errors 106/107/119.

But:

• src/mcp_synology/core/client.py:29 defines _SESSION_ERROR_CODES = frozenset({106, 107, 119})
• src/mcp_synology/core/client.py:246 documents "On session errors (106/107/119), triggers re-auth and retries once"
• src/mcp_synology/core/errors.py:165-170,296 defines the codes themselves

auth.py has the credential bits (keyring, env-var fallback, plaintext-config last resort, session-name format) — but no 106/107/119 matching, and no retry logic. A reporter clicking through the path to look at re-auth handling would find nothing in auth.py. Suggest splitting into two bullets, or amending the path to (src/mcp_synology/core/auth.py and src/mcp_synology/core/client.py).

3. [substantive] Test plan checkboxes all unticked. Five items in the PR body, four of which can be checked now (the dev-setup-skim, the CoC URL, the SECURITY.md scope check, the CHANGELOG entry confirmation). The fifth (maintainer enabling PVR) is moot — see observation 4.

Observations

4. [observation] PR body's "Action required from maintainer" warning is incorrect — PVR is already enabled. The body says:

⚠️ Action required from maintainer: the SECURITY.md Reporting section claims private vulnerability reporting is enabled. It is currently off on this repo — flip it on at Settings → Code security → "Private vulnerability reporting" → Enable before merging this PR.

Live state: gh api repos/cmeans/mcp-synology/private-vulnerability-reporting returns {"enabled":true}. So SECURITY.md:28-29 ("The private vulnerability reporting feature is enabled on this repository") is already accurate; no action needed. Recommend striking the warning from the PR body.

5. [observation] CONTRIBUTING.md's PR body example will diverge from .github/PULL_REQUEST_TEMPLATE.md (added in #58). CONTRIBUTING.md:117-130 shows a ## Summary / ## Test plan skeleton; PR #58 adds a ## CHANGELOG section to the actual template. Whichever PR lands second should align — either add ## CHANGELOG to the CONTRIBUTING example, or vice versa. Not blocking #56 in isolation.

Verdict

QA Failed pending findings 1, 2, 3 — observations 4 and 5 also need addressing per project convention before signoff.

[cmeans]

QA audit — round 1

Branch / SHA: `chore/community-health-files` @ `f40bc0e`

Commands run locally (reproducible):

```
git checkout chore/community-health-files
uv sync --extra dev
uv run pytest # 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 source files)
python3 scripts/sync-server-json.py --check # in sync (0.5.0)

Also exercised the CONTRIBUTING.md-recommended forms (which differ from CI):

uv run ruff check src/ tests/ # clean (66 files reported by ruff format)
uv run ruff format --check src/ tests/ # 66 files clean
uv run mypy src/ # clean (27 source files — no scripts/ coverage)
```

Live-state verification:

```
gh api repos/cmeans/mcp-synology/private-vulnerability-reporting

→ {"enabled":true}

```

PVR is already on, so the PR body's "Action required from maintainer" warning is moot, and the SECURITY.md / CODE_OF_CONDUCT.md statements about PVR are already accurate.

Codebase claim audit: every file path and feature claim in CONTRIBUTING.md, CODE_OF_CONDUCT.md, and SECURITY.md was checked against `src/`, `tests/`, and `.github/workflows/` — see the table in the review comment. One drift surfaced (F2): the session-retry logic is in `client.py` / `errors.py`, not `auth.py` as SECURITY.md L64-68 implies.

Outcome: QA Failed — see review comment for findings.

[cmeans]

QA Round 2 — PR #56

Strong round-2 cleanup. Round-1 findings all closed. One new substantive sub-finding (same drift class — symbol that doesn't exist) and one cross-doc nit. Bonus catch: round 2 also fixes the scripts/ drift that landed in .github/PULL_REQUEST_TEMPLATE.md via PR #58 — that's the same drift round-1 caught in CONTRIBUTING.md, and I should have flagged it on PR #58's review. Good catch by Dev.

Round-1 findings — verified closed on 5c4cb2d

Finding	Status	Evidence
F1 (CONTRIBUTING.md missing scripts/)	✅ closed	All 6 occurrences updated. CONTRIBUTING.md L66-68, L105-107, L126-128 now match CI. Ran uv run ruff check src/ tests/ scripts/ and uv run mypy src/ scripts/ literally — clean.
F2 (SECURITY.md mis-attribution)	✅ closed (with one new sub-finding, see below)	Now split into two bullets: L64-68 auth concerns under auth.py, L69-76 session-error retry logic correctly attributed to core/client.py (_SESSION_ERROR_CODES set + call sites at L308, L410, L512) and core/errors.py (codes 106/107/119 at L165-170, dispatch in error_from_code() at L264-296).
F3 (test plan checkboxes unticked)	✅ closed	All 7 round-2 boxes ticked.
O4 (PR body PVR warning)	✅ closed	Stale warning removed. SECURITY.md L28-29 still correctly states PVR is enabled.
O5 (CONTRIBUTING.md PR template alignment)	✅ closed	CONTRIBUTING.md L131-134 now has ## CHANGELOG section matching the PR template.

Verification on 5c4cb2d

Check	Result
uv run pytest	499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	69 files clean
uv run mypy src/ scripts/	clean (29 source files)
_SESSION_ERROR_CODES in client.py:29	found, used at L308/L410/L512 ✓
SessionExpiredError in errors.py:96	exists ✓
Lookup table for 106/107/119 in errors.py:163-171	exists ✓
error_from_code() dispatch at errors.py:264,296	exists ✓
is_session_error helper anywhere in src/	NOT FOUND — see F1-r2
Remote CI rollup (required checks)	all green

Round-2 findings

1. [substantive — same drift class as round 1] SECURITY.md:72-73 references an is_session_error helper that doesn't exist.

The new bullet says:

The codes and the is_session_error helper live in errors.py; the retry path lives in client.py (_SESSION_ERROR_CODES set plus the call sites that consult it).

grep -rn "is_session_error" src/ tests/ returns zero matches. The actual mechanism in errors.py is:

• SessionExpiredError class at L96
• Lookup table at L163-171 (codes 106/107/119)
• error_from_code() factory at L264, dispatching SessionExpiredError for 106/107/119 at L296

A reporter clicking through the path described in SECURITY.md to look at is_session_error would find nothing. Suggested rewrite:

The codes are defined in errors.py (lookup table at L163-171; SessionExpiredError class at L96; classification dispatch in error_from_code() at L264). The retry path lives in client.py (_SESSION_ERROR_CODES set at L29 plus the three call sites that consult it at L308, L410, L512).

Or shorter: drop the "and the is_session_error helper" clause; the rest of the bullet is accurate.

This is the same finding class as round 1's F2 — a symbol reference that doesn't match the codebase. Worth tightening before signoff.

Round-2 observations

2. [observation] Cross-doc placeholder mismatch. The PR-body example in CONTRIBUTING.md:125 uses tests/modules/<area>/test_<module>.py::test_<name>, while the merged-in .github/PULL_REQUEST_TEMPLATE.md uses tests/modules/<area>/test_<file>.py::test_<name>. Picking one (either <module> or <file>) and using it in both places keeps the two docs in sync. Either is fine — <file> is slightly more general, since not every test file maps 1:1 to a Python module name.

Verdict

QA Failed pending finding 1 (substantive) and observation 2 — per project convention, observations also block signoff.

[cmeans]

QA audit — round 2

Branch / SHA: `chore/community-health-files` @ `5c4cb2d` (round-2 commit `9b86d47` plus merge of main with #58)

Symbol-claim audit (against current `src/`):

```
grep -n "_SESSION_ERROR_CODES" src/mcp_synology/core/client.py

29:_SESSION_ERROR_CODES = frozenset({106, 107, 119})

308: ... if code in _SESSION_ERROR_CODES ...

410: ... if code in _SESSION_ERROR_CODES ...

512: ... if code in _SESSION_ERROR_CODES ...

grep -nE "^def |^class " src/mcp_synology/core/errors.py

17:class ErrorCode(StrEnum)

69:class SynologyError(Exception)

90:class AuthenticationError(SynologyError)

96:class SessionExpiredError(SynologyError)

...

264:def error_from_code(code: int, api_name: str = "") -> SynologyError

grep -rn "is_session_error" src/ tests/

(no matches — see F1-r2)

```

Local stack:
```
uv sync --extra dev
uv run pytest # 499 passed, 94 deselected, 96.04%
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 source files)
```

Cross-PR self-audit: Round 1 of this PR caught the missing-`scripts/` drift in CONTRIBUTING.md but I missed flagging the identical drift in `.github/PULL_REQUEST_TEMPLATE.md` introduced by PR #58 (which I QA'd to signoff). Round 2 of this PR fixes both. Lesson on the QA side: when flagging a doc-drift pattern in one file, grep the whole repo for the same pattern before signing off.

Outcome: QA Failed — see review comment for findings F1-r2 (substantive: `is_session_error` doesn't exist) and O2-r2 (observation: `` vs `` placeholder mismatch between CONTRIBUTING.md and the PR template).

[cmeans]

QA Round 3 — PR #56

Round-2 findings — verified closed on 43e5993

Finding	Status	Evidence
F1-r2 (is_session_error doesn't exist)	✅ closed	SECURITY.md:71-77 rewritten: now points at "the 100-series lookup table and the SessionExpiredError exception produced by the error_from_code() factory". grep -rn "is_session_error" src/ tests/ *.md returns zero matches. The new symbol references all exist: errors.py:163-171 (lookup table), errors.py:96 (SessionExpiredError), errors.py:264 (error_from_code()).
O2-r2 (<module> vs <file> placeholder mismatch)	✅ closed	Both CONTRIBUTING.md:125 and .github/PULL_REQUEST_TEMPLATE.md:17 now use tests/modules/<area>/test_<file>.py::test_<name>.

Verification on 43e5993

Check	Result
uv run pytest	499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	69 files clean
uv run mypy src/ scripts/	clean (29 source files)
Required CI rollup	all green (lint, typecheck, test 3.11/3.12/3.13, version-sync, validate-server-json, vdsm)

Verdict

Ready for QA Signoff. Round-1 + round-2 findings all closed; no new findings. QA Approved remains the maintainer's call per project label-ownership.

CHANGELOG ### Added duplication in ## Unreleased (round-1 O5 carryover) is still present but matches pre-existing project pattern — same note as PR #58: worth a single consolidate pass at next release-prep PR; not blocking now.

[cmeans]

QA audit — round 3

Branch / SHA: `chore/community-health-files` @ `43e5993`

Round-3 verifications:

```
grep -rn "is_session_error" src/ tests/ SECURITY.md CONTRIBUTING.md CODE_OF_CONDUCT.md

(no matches — F1-r2 closed)

grep -nE "test_<(module|file)>" CONTRIBUTING.md .github/PULL_REQUEST_TEMPLATE.md

CONTRIBUTING.md:125 → test_.py

.github/PULL_REQUEST_TEMPLATE.md:17 → test_.py

(consistent — O2-r2 closed)

uv run pytest # 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 source files)
```

Symbol-claim spot-check on the new SECURITY.md prose:

• `errors.py:163-171` — 100-series lookup table ✓
• `errors.py:96` — `SessionExpiredError` ✓
• `errors.py:264` — `error_from_code()` factory ✓

Outcome: Ready for QA Signoff. `QA Approved` remains the maintainer's call.

[cmeans]

LGTM