Add ytptube MCP server - #1
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds a self-hosted ytptube MCP server with environment configuration, authenticated HTTP access, secret redaction, read-only and mutation tools, stdio startup, documentation, and automated tests. Changesytptube MCP server
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Keep request abort protection active through response body consumption, redact serialized authentication fields, and align numeric ID schemas with task and preset handlers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 2-5: Add the package manifest’s private flag alongside the
existing metadata so npm treats this project as non-publishable and rejects
accidental npm publish attempts. Preserve the current name, version,
description, and module type settings.
In `@README.md`:
- Around line 50-58: Add two README examples under “Mutation safety”: one
invoking a documented read-only tool that continues to work with mutations
disabled, and one invoking a documented state-changing tool that the mutation
gate rejects before any network request. Keep both examples consistent with the
public tool contract and existing `YTPTUBE_ALLOW_MUTATIONS` behavior.
- Around line 9-10: Update the Markdown table separator immediately after the
header row so it contains three separator cells matching the Tool group, Tools,
and What it is for columns; leave the header and table content unchanged.
In `@src/redact.ts`:
- Around line 13-27: The redact function in src/redact.ts#L13-L27 must remove
each object from seen after its subtree finishes processing: wrap the object
traversal in try/finally and call seen.delete(value) in finally, preserving
active-ancestor cycle detection. In tests/redact.test.ts#L5-L59, add separate
regression tests for duplicate nested references under different keys (both
preserved) and a genuine self-reference (mapped to "[CIRCULAR]").
In `@src/server.ts`:
- Line 158: Update the input schemas for ytptube_generate_task_metadata,
ytptube_patch_task, ytptube_update_task, ytptube_patch_preset, and
ytptube_update_preset to declare id: numericId directly. Remove the redundant
numericId.parse(id) calls in each handler and pass the validated numeric id
through unchanged; leave the already-correct get-task and get-preset definitions
unchanged.
- Around line 168-172: Update the descriptions for ytptube_create_tasks and
ytptube_update_task to document the actual payload contract enforced by
taskCreatePayload.parse and taskCreate.parse: identify required fields such as
name and url, and state that unsupported extra fields including cookies and
config are rejected. Keep the intentionally loose arbitraryObject schemas and
existing mutation-gate behavior unchanged.
In `@tests/client.test.ts`:
- Around line 88-99: The client test suite needs coverage for the
AbortError-to-TIMEOUT mapping. Add a test alongside the existing transport
failure test that uses a fetch mock which remains pending until its AbortSignal
is triggered, advances fake timers to invoke the request timeout, and asserts
the rejected YtptubeApiError has code "TIMEOUT".
In `@tests/redact.test.ts`:
- Around line 5-59: Add a regression test in the redact suite covering both
reference cases: pass one shared nested object under two different keys and
assert both results retain the fully redacted object without "[CIRCULAR]", then
pass a self-referencing object and assert the recursive reference becomes
"[CIRCULAR]". Use the existing redact function and assertion style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a84fd75-927d-4f20-91b0-a1d033ae85ea
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.env.example.gitignoreAGENTS.mdREADME.mdpackage.jsonsrc/client.tssrc/config.tssrc/index.tssrc/redact.tssrc/server.tstests/client.test.tstests/config.test.tstests/redact.test.tstests/server.test.tstsconfig.json
Cover escaped serialized headers, authorization schemes, and equals-delimited cookie diagnostics before MCP output serialization.
Limit ytptube URL inputs to HTTP(S), load checkout-local .env configuration, and redact common credential assignments in returned text.
Redact URL userinfo and refresh tokens in returned text, and mark cancellation and overwrite-capable tools as potentially destructive.
Reject raw yt-dlp CLI strings for write operations and strengthen escape-aware credential redaction for returned logs and errors.
Keep read-only URL inspection within its documented API boundary by rejecting forwarded raw yt-dlp argument strings.
Expose strict nested write schemas to MCP clients and remove every plaintext authorization header value from returned output.
Remove Authorization and Proxy-Authorization values even when they appear within a returned diagnostic line.
Protect cookie payload values when backend logs or errors serialize the public cookies field.
Align extras with the documented API contract, normalize numeric bulk history IDs, and redact common credential aliases in returned output.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/redact.ts`:
- Around line 41-44: Update redactSerializedHeaders to redact unterminated
values through the end of output instead of continuing when end exceeds
output.length, while preserving delimiter handling for terminated values. In the
replacement construction, simplify the two tail slices after the redacted marker
to a single output.slice(end) call, and adjust startPattern.lastIndex
consistently with the resulting output.
- Around line 5-7: The quoted-value portions of SERIALIZED_SECRET and
ESCAPED_SERIALIZED_SECRET can backtrack excessively on unterminated,
backslash-rich input. Replace their value-matching expressions with mutually
exclusive quote-safe alternatives that explicitly handle non-quote/non-backslash
characters and escapes, avoiding [\s\S], while preserving matching of properly
terminated serialized secrets used by redactString().
In `@src/server.ts`:
- Around line 174-176: Update the registration of ytptube_set_history_archive so
its destructive flag is enabled, matching ytptube_queue_control and
ytptube_generate_history_nfo. Preserve the existing archived-dependent
POST/DELETE behavior and all other registration arguments.
- Around line 121-123: Update the ytptube_inspect_url registration to pass the
z.object schema directly to register, removing the unsafe as unknown as
ZodRawShape assertion while preserving the existing schema and handler.
In `@tests/index.test.ts`:
- Around line 11-19: Update the test setup around mkdtemp in the
environment-loading test to wrap file creation and assertions in a try/finally
block, and remove the temporary directory in finally using rm(directory, {
recursive: true, force: true }).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 429bf193-9c9b-4a56-b92b-b8a7af177ab9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/client.tssrc/index.tssrc/redact.tssrc/server.tstests/client.test.tstests/index.test.tstests/redact.test.tstests/server.test.ts
Comment on lines
+121
to
+123
| register("ytptube_inspect_url", "Inspect URL metadata without adding it to the download queue.", z.object({ | ||
| url: httpUrl, preset: z.string().optional(), force: z.boolean().optional(), entries: z.boolean().optional(), | ||
| }).strict() as unknown as ZodRawShape, (input) => call("/api/yt-dlp/url/info", { query: input as RequestOptions["query"] })); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🌐 Web query:
Does @modelcontextprotocol/sdk 1.17.3 registerTool accept a ZodObject for inputSchema or only a ZodRawShape?
💡 Result:
The registerTool method in @modelcontextprotocol/sdk accepts both ZodObject and ZodRawShape for the inputSchema parameter [1][2][3]. In the current SDK, ZodObject is the standard, preferred way to define schemas [4][5]. While the SDK also supports ZodRawShape (a plain object record like { field: z.string }) as a backward-compatible shorthand for v1 compatibility, it does so by internally auto-wrapping the raw shape with z.object [1][2][6]. You are encouraged to use z.object({...}) directly, as the raw-shape shorthand is deprecated and intended to be removed in future major versions [2][4][3].
Citations:
- 1: feat(compat): registerTool/registerPrompt accept raw Zod shape, auto-wrap with z.object() modelcontextprotocol/typescript-sdk#1901
- 2: modelcontextprotocol/typescript-sdk@182ec53
- 3: https://github.com/modelcontextprotocol/typescript-sdk/blob/5e0249f57fdc1d7b7d69cd6140952a8e4e7ae695/packages/server/src/server/mcp.ts
- 4: https://ts.sdk.modelcontextprotocol.io/v2/advanced/schema-libraries
- 5: https://ts.sdk.modelcontextprotocol.io/v2/documents/Documents.Server_Guide.html
- 6: https://ts.sdk.modelcontextprotocol.io/v2/types/_modelcontextprotocol_server.server_mcp.ZodRawShape.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate src/server.ts and package versions =="
git ls-files | rg '(^|/)src/server\.ts$|package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock' || true
if [ -f src/server.ts ]; then
echo
echo "== sr/src/server.ts outline =="
ast-grep outline src/server.ts --view expanded || true
echo
echo "== relevant src/server.ts lines =="
nl -ba src/server.ts | sed -n '1,180p'
fi
echo
echo "== package manager / version declarations =="
for f in package.json package-lock.json pnpm-lock.yaml yarn.lock; do
[ -f "$f" ] || continue
echo "--- $f ---"
if [ "$f" = "package.json" ]; then
jq '.dependencies["`@modelcontextprotocol/sdk`"], .devDependencies["`@modelcontextprotocol/sdk`"]' "$f" || true
else
rg -n '"`@modelcontextprotocol/sdk`"|`@modelcontextprotocol/sdk`|version' "$f" | head -80
fi
done
echo
echo "== tool registrations in repository =="
rg -n 'register\(".*", ".*"|z\.object\(\{[\s\S]{0,250}\}\)\.strict|registerTool|inputSchema' src package.json 2>/dev/null || true
echo
echo "== inspect installed typings if present =="
if [ -d node_modules/@modelcontextprotocol/sdk ]; then
rg -n 'registerTool|inputSchema|ZodRawShape|ZodObject' node_modules/@modelcontextprotocol/sdk -g '*.ts' -g '*.d.ts' | head -120
fd -e dts -e ts . node_modules/@modelcontextprotocol/sdk -d 5 -i | head -80
else
echo "node_modules/@modelcontextprotocol/sdk not present"
fiRepository: Snuffy2/ytptube-mcp
Length of output: 1318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/server.ts relevant lines =="
awk 'NR<=180 { printf "%6d\t%s\n", NR, $0 }' src/server.ts
echo
echo "== package version declarations =="
jq -r '.dependencies["`@modelcontextprotocol/sdk`"], .devDependencies["`@modelcontextprotocol/sdk`"]' package.json 2>/dev/null || true
echo
echo "== tool registrations =="
rg -n 'register\(".*",".*"|httpUrl|strict\(\)|registerTool|inputSchema' src/server.ts src package.json || true
echo
echo "== installed MCP SDK typings/source snippets =="
if [ -d node_modules/@modelcontextprotocol/sdk ]; then
rg -n 'registerTool|inputSchema|ZodRawShape|ZodObject' node_modules/@modelcontextprotocol/sdk -g '*.ts' -g '*.d.ts' | head -120
else
echo "node_modules/@modelcontextprotocol/sdk not present"
fi
echo
echo "== package-lock `@modelcontextprotocol/sdk` snippet =="
python3 - <<'PY'
import json
with open('package-lock.json') as f:
data=json.load(f)
for path in ['node_modules/@modelcontextprotocol/sdk']:
if 'packages' in data and path in data['packages']:
print(data['packages'][path].get('version'))
PYRepository: Snuffy2/ytptube-mcp
Length of output: 12855
Align the MCP input schema with the SDK contract.
registerTool accepts z.object(...), so ytptube_inspect_url does not need as unknown as ZodRawShape. Keep the wrapper; remove the unsafe type assertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server.ts` around lines 121 - 123, Update the ytptube_inspect_url
registration to pass the z.object schema directly to register, removing the
unsafe as unknown as ZodRawShape assertion while preserving the existing schema
and handler.
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.
Summary
Adds a local stdio MCP server for self-hosted ytptube instances, with an explicit read-only default so clients can inspect and manage downloads safely.
What Changed
apikeyfallback; all returned credentials and header/query secrets are redacted.YTPTUBE_ALLOW_MUTATIONS=falseas the default guard for every state-changing operation; metadata generation and history file deletion require explicit opt-in.Why
ytptube exposes a capable self-hosted HTTP API, but MCP clients need a narrow, credential-safe interface with predictable controls over destructive or state-changing actions.
Summary by CodeRabbit
New Features
Documentation
Tests
Greptile Summary
Adds a local stdio MCP server that exposes ytptube operations through validated tools, with mutations disabled by default.
Confidence Score: 5/5
The PR appears safe to merge, with no blocking failure remaining.
No blocking failure remains; the previously reported plaintext credential transport behavior is retained as an optional warning or documentation consideration for self-hosted deployments.
Important Files Changed
Reviews (4): Last reviewed commit: "fix review feedback safeguards" | Re-trigger Greptile