Document the MCP surface (S62/S63/S243/S249), soften the catalog trust claim, and correct S39/S40/S44-b drift - #98
Merged
Conversation
…ust claim Part 1 — MCP was entirely undocumented (grep -ril 'mcp' docs/ returned nothing) while a full surface shipped in phlix-hub across S62/S63/S243/S249. Adds docs/integrations/mcp.md covering POST/GET /mcp, PAT auth and the phlix-mcp- prefix, token create/revoke (UI at /app/mcp-tokens and the three /api/v1/me/mcp-tokens endpoints), the scope vocabulary, the tool catalogue, SSE vs POST, and protocol versioning. Registered in the sidebar. Three things are stated rather than glossed: - playback_control is default-OFF and best effort; the Chromecast/Roku/AirPlay backends are not production-functional, so a call can be accepted and do nothing. It is not documented as a working feature. - mcp:playback:control is returned by McpScopes::all() unconditionally, so it appears in available_scopes and the token UI ticks it by default. The operator flag gates tool REGISTRATION, not the scope; granting it does not imply the tools work. - MCP-Protocol-Version is VERIFIED (400 on an unsupported value); only initialize.protocolVersion is NEGOTIATED. Documented as two mechanisms. Part 2 (S217(b), docs half) — the plugin-catalog docs claimed the dev channel "widens discovery, never trust". ref and artifactSha256 are self-asserted by the catalog document, so on a moving dev branch anyone who can push master can pin their own commit AND its digest and it verifies. The digest check survives; the trust ANCHOR moves. Softened in plugin-catalog.md (both assertions) and in the admin-plugins.yaml restatement so the two do not disagree. Also records the two documented ways an un-pinned install is still allowed (file:// sources and PHLIX_PLUGINS_ALLOW_UNVERIFIED), which the old absolute wording denied. Part 3 — drift from the same batch: - S39/S40: documents the stale flags on /health/relay (relay.stale, hub.stale, both always present) and /health/network (stale, emitted only on the stale branch), plus heartbeatStale on the relay ping. Removes the now-false claim that there is no updatedAt-staleness guard. - S44-b: qualifies two absolute "nothing stalls" claims in coroutine-runtime.md and points at the closed BLOCKING_IO_EXCEPTIONS.md register. - @phlix/ui repin: drops two "needs a newer SPA pin" caveats whose condition is now met. - S90 (Alexa): no change — the middleware has no route, and docs mention it nowhere, so there is nothing user-facing to document. Verified: npm test 84/84 exit 0. npm run docs:build exits 1 on 8 dead anchors in docs/dev/client-windows.md — PRE-EXISTING and proven so: a control worktree at the untouched origin/master (376fbad) exits 1 with the identical 8, and this branch does not touch that file. The new page's own anchors resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 medium |
| CodeStyle | 3 minor |
🟢 Metrics 0 complexity · 0 duplication
Metric Results Complexity 0 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two unrelated pieces of documentation debt, both found while auditing what the MCP steps left behind.
1. The MCP surface was entirely undocumented
grep -ril 'mcp' docs/returned nothing. Four steps shipped user-facing MCP surface — S62 (core + PAT auth), S63 (SSE transport, protocol negotiation, validation), S243 (the admin token UI), S249 (the scope vocabulary) — and none of it had a page.New
docs/integrations/mcp.md, registered in the/integrations/sidebar group. Written from the source, not from the plan steps. Three things it deliberately does not overstate:playback_controlis off by default and best-effort.config/server.php:70-73defaults itfalse,mcpPlaybackControlEnabled()compares=== true, and the tool is only appended to the registry when set. Documented in adangerblock with the casting-backend caveat, not written up as a working feature.mcp:playback:controlis granted whether or not you ask for it.McpScopes::all()returns all four unconditionally,available_scopesis built from it, andMcpTokensPage.vue:185-187seedsselectedScopes = [...availableScopes]— so the mint form arrives pre-ticked.McpTokenController::create():110also grantsall()whenscopesis omitted, so an API caller gets it by default too. Both documented rather than glossed.MCP-Protocol-Versionis verified, not negotiated.protocolVersionRefusal()400s the header;negotiate()echoes-or-downgrades onlyinitialize.protocolVersion. Written as two distinct mechanisms with an explicit do-not-conflate warning.2. The plugin-catalog dev-channel trust claim was too strong
Softened in three places, not the two the prose sweep found:
plugins/plugin-catalog.md:83-88,:98-104, and the restatement inreference/api/admin-plugins.yaml:155-157. The YAML surfaced only on a non-.mdpass — a markdown-only search would have left the spec asserting exactly what the prose had just retracted.While rewriting the sentence, an adjacent absolute in it turned out to be false and was corrected: "un-pinned entry cannot be installed" —
PluginLoader.php:1008-1032has two exemptions (file://sources, andPHLIX_PLUGINS_ALLOW_UNVERIFIED).docs/old/andarchive/were searched separately and are clean. The### Release channelheading was left alone —env-vars.md:26links its anchor.3. Drift from recently-shipped work
Corrected against the current source rather than against the step descriptions:
/health/relay's two flags are always present but/health/network'sstaleis emitted only on the stale branch. A sentence claiming "noupdatedAt-staleness guard" was not merely stale, it was affirmatively false.@phlix/uirepin — two caveats whose condition is now met were still worded as unmet gates.No Alexa page. S90 shipped the signature middleware but no route exists yet, and the docs mention Alexa nowhere. Documenting an unreachable endpoint would be worse than the current silence.
Verification
npm test84/84, exit 0.npm run docs:buildexit 0 — 158 pages (157 + the new page), 3474 unique fragment links, 0 dead anchors. Rebased onto #97, which repaired the pre-existing anchor breakage; this branch was verified green on the rebased tree before pushing.🤖 Generated with Claude Code