Skip to content

Prepare Screenly MCP for Claude Desktop marketplace listing - #304

Open
salmanfarisvp wants to merge 12 commits into
Screenly:masterfrom
salmanfarisvp:feat/mcp-annotations-and-mcpb-bundle
Open

Prepare Screenly MCP for Claude Desktop marketplace listing#304
salmanfarisvp wants to merge 12 commits into
Screenly:masterfrom
salmanfarisvp:feat/mcp-annotations-and-mcpb-bundle

Conversation

@salmanfarisvp

@salmanfarisvpsalmanfarisvp commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

Unblocks Anthropic Claude Desktop marketplace submission by finishing the remaining MCP listing prerequisites, and fixes a stdio bug that would have failed review on first launch.

  • Tool annotations on all 33 MCP tools (title, readOnlyHint, destructiveHint, idempotentHint, openWorldHint) so clients can distinguish read-only vs destructive actions
  • MCPB Desktop Extension packaging (mcpb/manifest.json + mcpb/README.md with privacy policy, 512×512 icon) and a release workflow step that builds .mcpb for macOS and Windows (rustc-target names plus friendlier aliases like screenly-macos-arm64.mcpb), injecting the version from the git tag and pinning @anthropic-ai/mcpb@2.1.2
  • Fix MCP stdout contamination by enabling simple_logger's stderr feature — log lines were previously interleaved with JSON-RPC on stdout and caused Claude Desktop to fail with Unexpected non-whitespace character after JSON at position 4
  • Docs updated for Desktop Extensions install, Gatekeeper on unsigned macOS bundles, privacy (API host + Sentry), and token URL alignment

Review follow-ups

Addressed @sergey-borovkov's review on this PR:

High

  • Corrected privacy policy: production API host api.screenlyapp.com, and disclosed unconditional Sentry init
  • Documented Desktop Extensions as the primary install path; Gatekeeper → Privacy & Security → Open Anyway for unsigned macOS sideloads

Medium

  • Annotation semantics: asset_createopenWorldHint, updates marked destructive, link/share tools idempotent (13 destructive)
  • Drift test: MCPB manifest tools must match server.rs
  • Per-platform entry_point / command in the release pack step (incl. win32)
  • 512×512 mcpb/icon.png
  • Dropped unexplained compatibility.claude_desktop version floor
  • Aligned token docs with *.screenlyapp.com → Settings → Security → API tokens

Minor

  • Pinned @anthropic-ai/mcpb@2.1.2 (latest under @2, as noted in review)
  • Friendlier release aliases: screenly-macos-arm64, screenly-macos-x64, screenly-windows-x64

Changes

AreaFiles
Annotationssrc/mcp/server.rs — all 33 tools
MCPB bundlemcpb/manifest.json, mcpb/README.md, mcpb/icon.png
Release packaging.github/workflows/release.yml
CI.github/workflows/actions.yml — skip List screens on fork PRs
Drift guardsrc/mcp/tests.rs — manifest tools match server
Stdout fixCargo.toml (simple_logger + stderr)
DocsREADME.md, .gitignore (*.mcpb)

Test plan

  • cargo check --all-targets
  • cargo fmt --check
  • cargo test (including MCPB manifest drift test)
  • Local .mcpb pack with @anthropic-ai/mcpb — manifest schema validation passed; executable bit preserved
  • Confirmed stdout is pure JSON-RPC after the stderr logging fix; logs go to stderr only
  • Confirmed tools/list returns annotations for all 33 tools (12 read-only, 13 destructive)
  • Installed .mcpb in Claude Desktop locally — connects successfully with API token; tools work
  • Review release.yml MCP Bundle step for macOS/Windows matrix targets (aliases + pinned mcpb)
  • After merge + tagged release, verify .mcpb artifacts (target names + aliases) attach to the GitHub release

Add tool annotations, an MCPB bundle manifest, release packaging for
desktop platforms, and route logger output to stderr so the MCP stdio
JSON-RPC stream stays clean.
@salmanfarisvp

Copy link
Copy Markdown
MemberAuthor

Tested with the local build and working fine as expected

image

and different commands
image

image

Fork pull_request runs do not receive repository secrets, so
SCREENLY_API_TOKEN is empty and `screen list` fails auth. The job also
uses screenly/cli@master, so it never exercises the PR's own code.
@vpetersson

Copy link
Copy Markdown
Contributor

Love it!

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@salmanfarisvp

Copy link
Copy Markdown
MemberAuthor

@renatgalimov, could you please request a review from Copilot?

@sergey-borovkov

Copy link
Copy Markdown
Contributor

Overview

This prepares the Screenly MCP server for Claude Desktop marketplace listing: MCP tool annotations on all 33 tools, an .mcpb bundle (manifest + README) built per-target in the release workflow, and a fix routing simple_logger output to stderr so stdio JSON-RPC isn't corrupted. Plus a CI guard so the integration job skips on fork PRs.

The core direction is right and the stdout fix is a genuine bug fix. Several things I verified directly:

  • simple_logger 5.1.0 does expose a stderr feature; default-features stays on, so timestamps is preserved. And there are no println!/print! calls on the screenly mcp path — stdout really is clean now.
  • rmcp-macros 1.4.0 supports annotations(...) with exactly this syntax.
  • manifest_version: "0.3" validates against @anthropic-ai/mcpb@2 (latest 2.1.2). I unpacked the schema — 0.4 only adds the uv runtime, so 0.3 is correct for a binary server, and every field used is in the strict schema.
  • pack [directory] [output] is the right signature, and files: "screenly*" in the Publish step will pick up screenly-cli-<target>.mcpb at workspace root — the open checklist item should pass.
  • Counts are consistent: 33 tools, 12 read-only, 8 destructive across manifest, both READMEs, and server.rs.

High — should block marketplace submission

1. The privacy policy in mcpb/README.md is factually wrong on two counts. This is precisely what listing review scrutinizes.

  • It says the extension "connects only to the Screenly API at api.screenly.io". The production host is api.screenlyapp.com (build.rs:7), with login.screenly.local / api.screenlyappstage.com for other builds, and overridable via API_BASE_URL (src/authentication.rs:45).
  • It says the extension "collects no analytics, sends no telemetry, and transmits data to no third party other than Screenly itself." But src/main.rs:32-40 unconditionally initializes Sentry with a hardcoded DSN pointing at o4505481987489792.ingest.sentry.io. sentry 0.46's default features include the panic integration plus contexts/debug-images, so panics and device/OS context are uploaded to a third party on every subcommand — including mcp. Either disclose Sentry in the privacy section, or skip sentry::init when the subcommand is mcp.

2. No macOS code signing or notarization anywhere in release.yml. The .mcpb ships an unsigned, un-notarized Mach-O. A bundle downloaded from a GitHub release carries com.apple.quarantine, and Gatekeeper will refuse to exec it. The test-plan entry "Installed screenly-cli-aarch64-apple-darwin.mcpb in Claude Desktop locally" used a locally built bundle, which has no quarantine attribute — so that test cannot catch this. Before submitting, cut a pre-release tag, download the artifact through a browser, and install it. Longer term this needs codesign + notarytool steps for the darwin targets.

Medium — annotation semantics

3. open_world_hint = false on asset_create is wrong. The tool takes an arbitrary source_url that Screenly then fetches and renders. That is the textbook open-world interaction (and an SSRF-adjacent surface). Blanket-false is defensible for the other 32 tools — the Screenly API is a closed, well-defined domain — but not this one.

4. destructive_hint = false on every *_update tool. Per the MCP spec, destructiveHint: false means the tool performs only additive updates. Overwriting a title, a playlist's predicate, or a label name is not additive. asset_update is the sharpest case: it overwrites js_injection, i.e. arbitrary JavaScript that executes on the customer's screens. That should be destructive_hint = true at minimum, so clients prompt before it.

5. Link/unlink idempotency is asymmetric.label_link_screen, label_link_playlist, and shared_playlist_create are idempotent_hint = false, while their unlink/delete counterparts are true. All six are set-membership operations — re-linking an already-linked label has no additional effect. Either mark the link side idempotent too, or document why the API doesn't behave that way.

Medium — maintainability & packaging

6. The 33-tool list now exists in four places (src/mcp/server.rs, mcpb/manifest.json with tools_generated: false, README.md, mcpb/README.md), each carrying descriptions that must stay byte-identical. There's no test guarding drift, and no test covers annotations at all — src/mcp/tests.rs is 35 per-tool behavior tests, none of which touch the new metadata. A single test asserting that manifest.json's tool names and descriptions match the server's tool router would cover both gaps cheaply, and would have caught any typo in this 600-line diff.

7. entry_point: "server/screenly" is wrong for the win32 bundle — that bundle contains server/screenly.exe, and there's no platform override for entry_point (only for mcp_config.command). I checked mcpb's validator: it only verifies icon paths, not entry_point, so this packs silently, and Claude Desktop launches via mcp_config.command, which is correct. So it likely works — but the manifest is internally inconsistent. Extend the jq filter to set entry_point per platform alongside .compatibility.platforms.

8. No icon in the manifest. Not schema-required, but mcpb emits a recommendation (512×512) and a marketplace listing almost certainly wants one.

9. compatibility.claude_desktop: ">=0.10.0" is an unexplained magic floor. If it's higher than the real minimum you silently block installs. Worth a comment or a source.

10. Conflicting token instructions in the same PR.mcpb/README.md says console.screenly.ioSettings → Tokens; manifest.json says https://[your-workspace].screenlyapp.comSettings > Security > API tokens. Given the API host is screenlyapp.com, the manifest version looks right. Pick one and use it in both.

Minor

  • Scope creep in .github/workflows/actions.yml — the fork-PR guard is a correct fix and well-commented, but it isn't mentioned in the PR body's Changes table. Also confirm that if "List screens" is a required status check, a skipped job satisfies it for fork PRs.
  • screenly-cli-x86_64-pc-windows-gnu.mcpb is cryptic against the instruction "download screenly-cli-<your-platform>.mcpb". Consider a friendlier alias for the bundle artifacts specifically.
  • npx --yes @anthropic-ai/mcpb@2 floats across the 2.x range. Pinning an exact version is more reproducible for a release pipeline.
  • The mcpb/README.md local-build snippet is correct and I traced it end to end — no issues.

Verdict

The stderr fix and the annotation pass are solid work. I'd hold merge on #1 (the privacy policy misstates both the API host and the Sentry telemetry) and #2 (unsigned macOS binary, untested through the actual download path) — both are submission blockers rather than code-quality nits. #3 and #4 are quick annotation corrections worth folding in at the same time.

The listing text claimed api.screenly.io and no third-party telemetry;
production uses api.screenlyapp.com and the CLI always initializes Sentry.
Prefer Settings → Extensions over GitHub sideloads, and guide macOS
Gatekeeper blocks via Privacy & Security → Open Anyway instead of xattr.
Mark asset_create as open-world, treat updates as destructive, and mark
link/share tools idempotent so hints match MCP spec meaning.
Keeps the duplicated 33-tool name/description catalog from drifting
between mcpb/manifest.json and the #[tool] handlers.
Windows bundles ship screenly.exe; bake matching entry_point/command into
each single-platform manifest instead of relying on a stale override.
The >=0.10.0 bound had no Screenly-specific source and could silently
block installs; keep platform compatibility only.
Use the workspace screenlyapp.com Settings → Security → API tokens path
in both places so install docs no longer conflict.
Gives Claude Desktop and the marketplace listing a proper brand mark;
the release pack step and local build docs now include icon.png.
Pin @anthropic-ai/mcpb to 2.1.2 for reproducible releases, and publish
screenly-macos-arm64/x64 and screenly-windows-x64 aliases alongside the
rustc-target filenames.
@salmanfarisvp

Copy link
Copy Markdown
MemberAuthor

Thanks for the detailed review, @sergey-borovkov , really helpful. I’ve addressed the High / Medium / Minor items in the latest pushes:

High

  • Privacy policy: corrected API host to api.screenlyapp.com and disclosed Sentry — 7bd86b9
  • Desktop Extensions as primary install path; Gatekeeper / Open Anyway for unsigned macOS sideloads — 789f0e0

Medium

  • Annotation semantics (asset_create open-world, updates destructive, link/share idempotent) — 5e6453c
  • Drift test so MCPB manifest tools stay in sync with server.rs4334ae8
  • Per-platform entry_point / command in the release pack step — 4a522e4
  • Dropped the unexplained claude_desktop version floor — 0fbd086
  • Token docs aligned with *.screenlyapp.com → Settings → Security → API tokens — 0ac561b
  • 512×512 mcpb/icon.pnga8616e5

Minor

  • Pinned @anthropic-ai/mcpb@2.1.2 and added friendlier release aliases (screenly-macos-arm64, etc.) — b73fbe9

Full Apple codesign / notarization for the binary is still an org-level follow-up (same as CLI releases today). Happy to adjust anything else you spot.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@salmanfarisvp@vpetersson@sergey-borovkov