docs(adr): defer per-client DSM sessions, restructure name helper (closes #47)

[cmeans-claude-dev]

Summary

Closes #47. Records the deferred-stub decision (Option 3 from the issue) for the per-MCP-client DSM session model. Spec drift closed; the helper is restructured so a future per-client session pool is a bounded change; runtime behavior unchanged.

What changed

• src/mcp_synology/core/auth.py — _build_session_name() now accepts an optional session_key: str | None = None. With None it preserves the current uuid-suffix shape (MCPSynology_{instance_id}_{uuid8}); with a key it returns the planned per-key shape (MCPSynology_{instance_id}_{session_key}). The constructor still calls _build_session_name() with no key, so every existing code path is byte-identical. get_session() signature unchanged — no public-API surface change today.

• docs/specs/architecture.md — the Auth Manager interface example was showing the future get_session(session_key=...) signature even though the live code is get_session() -> str. Reverted the example to match live code with a forward-pointer to the planned section + ADR. Renamed the dedicated section header from "Future: Per-Client Sessions (Streamable HTTP)" to "Planned: Per-Client Sessions (Streamable HTTP)" and added an explicit deferral-not-oversight statement plus a back-reference to the ADR.

• docs/adr/0001-per-client-dsm-sessions.md (new) — first ADR in the project. Captures: question, three options with concrete tradeoffs, decision (Option 3), consequences (zero behavior change, spec drift closed, bounded next step), and five concrete revisit triggers:

  1. Streamable HTTP gaining a concrete implementation plan.
  2. A multi-tenant deployment use case appearing.
  3. Subagent isolation explicitly requested.
  4. Tool surface roughly doubling (e.g., New module: Storage + health tools that feed awareness alerts #49 Storage + Health module).
  5. Single-shared-session model exhibiting real-world pain (concurrent stalls, rate limits).

  Also inventories what the next implementation step looks like when a trigger fires — define client-identity contract, spec session-pool lifecycle, decide credentials policy — so the deferred work has a clear shape rather than an open-ended re-design.

• tests/core/test_auth.py — three new tests in TestSessionNaming pin the helper contract:

  • test_build_session_name_without_key_uses_uuid — no-arg form keeps the uuid-suffix shape (defends against a future change accidentally altering the default).
  • test_build_session_name_with_key_uses_key_as_suffix — session_key="mcp-client-xyz" produces MCPSynology_{instance_id}_mcp-client-xyz.
  • test_build_session_name_with_empty_key_uses_empty_suffix — session_key="" is honored as-is, not treated as None. Pins the None-vs-empty distinction so a future pool cannot accidentally collide with the no-key default.

• CHANGELOG.md — entry under ### Added.

Why Option 3 (and not 1 or 2)

From the ADR's tradeoffs section, condensed:

• Option 1 (ship now) — real refactor for a feature with zero current users. Maintenance tax (every future module author has to thread session_key through; one missed call site silently regresses). YAGNI risk if the trigger never fires. The strongest case for it — "refactor cost grows linearly with the tool surface" — is real but conditional.
• Option 2 (retract) — honest but throws away the design thinking. Re-deriving when the trigger fires costs more than recording it now.
• Option 3 (defer) — chosen. Spec and code agree (within the deferral framing). Helper is structurally ready. No public-API change. No behavior change. Design intent preserved. Clear revisit triggers so the next person knows when to revisit and what to do.

Out of scope

• Adding session_key to get_session()'s signature. Doing so without the underlying session pool would either silently no-op (misleading) or raise NotImplementedError (a real behavior change for callers that don't exist yet). Both are net-worse than the current "no parameter at all". Punted to the trigger-fired follow-up.
• The session-pool lifecycle design (max-concurrent cap, idle GC, eviction policy, per-key re-auth, callback dispatch). The ADR records that the design is deferred, not solved. Recording the question is the work this PR ships; the answer is what fires when a revisit trigger lands.
• Per-client credentials, multi-user identity contracts, and the Streamable HTTP transport itself. All downstream of the same trigger.

Verification

• uv run ruff check src/ tests/ — clean.
• uv run ruff format --check src/ tests/ — 69 files already formatted.
• uv run mypy src/ — Success: no issues found in 28 source files.
• uv run pytest — 603 passed (up from 600 on main after fix(filestation): create_folder per-path serial calls (closes #95) #97), 17 warnings, 0 failed. Coverage 96.25% (auth.py at 100%).

Test plan

• Spec sections read coherently end-to-end — Auth Manager example matches auth.py:254; "Planned" section reads as a deferral pointer, not a roadmap promise.
• ADR is internally consistent — Option 3 description matches the implementation; revisit triggers are concrete enough to be actionable.
• _build_session_name() no-arg behavior is byte-identical to pre-PR (verified by the existing test_session_name_format).
• _build_session_name(session_key="x") produces the documented per-key shape (covered by new tests).
• No new public API surface — get_session() signature unchanged.
• CHANGELOG entry under ### Added (placeholder #PR_PLACEHOLDER to be substituted post-merge).

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cmeans]

LGTM

[cmeans]

[QA] Starting review at e59ad2b. Will run lint/mypy/pytest, audit ADR↔spec↔code consistency, and verify issue #47 scope coverage. Reporting back with findings or signoff.

[cmeans]

[QA] Round 1 — PASS

Verified at e59ad2b against current main (3790b62 = #97 + #98 in).

Static checks

• uv run ruff check src/ tests/ → clean
• uv run ruff format --check src/ tests/ → 69 files already formatted
• uv run mypy src/ → Success: no issues found in 28 source files

Tests

• uv run pytest → 603 passed, 112 deselected, 18 warnings, 0 failed in 23.4s
• Coverage 96.25% (matches PR body); auth.py at 100% (matches)
• The 112 deselected = addopts = "-m 'not integration and not vdsm'" from pyproject.toml:99. Those tests run separately; vdsm CI ran SUCCESS at this SHA.

Spec ↔ code ↔ ADR alignment

• docs/specs/architecture.md:64 Auth Manager example shows async def get_session(self) -> str: → matches live src/mcp_synology/core/auth.py:266. ✓
• Spec line 69 forward-pointer link #planned-per-client-sessions-streamable-http resolves to the renamed #### Planned: Per-Client Sessions (Streamable HTTP) heading at line 309 (GitHub-style anchor lowercases + drops parens). ✓
• Spec → ADR back-reference ../adr/0001-per-client-dsm-sessions.md resolves correctly relative to docs/specs/architecture.md. ✓
• ADR §Decision states _build_session_name(session_key: str | None = None) -> str → matches auth.py:72. ✓
• Five revisit triggers each name a concrete, observable condition (Streamable HTTP plan, multi-tenant deployment, subagent isolation request, tool-surface roughly doubling, real-world single-shared-session pain). Actionable. ✓

Helper contract

• _build_session_name() no-arg path: existing test_session_name_format still passes; new test_build_session_name_without_key_uses_uuid independently pins the uuid-suffix shape. Byte-identical to pre-PR.
• _build_session_name(session_key="mcp-client-xyz") → MCPSynology_{instance_id}_mcp-client-xyz (new test).
• _build_session_name(session_key="") → MCPSynology_{instance_id}_ (new test pins the None-vs-empty distinction).
• Re-verified TDD claim: rolled auth.py back to 79933a7 baseline; the three new tests fail with TypeError: _build_session_name() got an unexpected keyword argument 'session_key'; restored, all pass.

Public API surface

• get_session() signature unchanged (auth.py:266). No callers in src/ pass a session_key (grep confirms zero hits outside of docstrings/tests/docs). The deferred-stub framing holds.

Issue #47 coverage

• All three options the issue laid out (ship multi-session, retract spec, deferred stub) are enumerated in the ADR with concrete pros/cons. Option 3 chosen, decision recorded, revisit triggers documented. The "decide before expanding the DSM tool surface" framing from #47's context is met.

Stale-reference grep

• No remaining "Future: Per-Client Sessions" string in the spec.
• No get_session(session_key=...) callers in src/. All occurrences are doc/test text or the planned-section example.

Transparency notes (not findings)

• PR body says "17 warnings"; my run shows 18. Pytest warning counts are environment-dependent (Python sub-version, dep versions); test count and coverage match exactly so I'm not flagging this.
• PR body refers to auth.py:254 for the unchanged signature; current line is auth.py:266 (drift due to expanded _build_session_name docstring). Cosmetic; the claim itself holds.

CI rollup

• 13 SUCCESS / 5 SKIPPED required checks at e59ad2b incl. vdsm integration tests SUCCESS. The "QA Gate" PENDING context will resolve when this label transition lands.

Zero blockers, zero substantive, zero observations. Ticking all six test-plan checkboxes pre-signoff. Applying Ready for QA Signoff as the final act; awaiting maintainer's QA Approved.