Skip to content

fix devvit logs#15

Merged
mwood23 merged 3 commits into
mainfrom
fix-devvit-logs
Aug 15, 2025
Merged

fix devvit logs#15
mwood23 merged 3 commits into
mainfrom
fix-devvit-logs

Conversation

@mwood23
Copy link
Copy Markdown
Collaborator

@mwood23 mwood23 commented Aug 14, 2025

Closes #

💸 TL;DR

I gave llms too much power and it kept messing up stuff. Also, since is required because we only stream this from the logs and I need to stop it forcibly since we need a more rest styled request.

📜 Details

Design Doc

Jira

🧪 Testing Steps / Validation

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

@mwood23 mwood23 requested a review from a team as a code owner August 14, 2025 18:53
Comment thread package.json Outdated
"clean": "rm -rf ./dist ./fixtures ./db ./fixtures.tar.gz",
"dev": "concurrently -p \"[{name}]\" -n \"ESBUILD,INSPECTOR\" -c \"blue,green\" \"npm run build:watch\" \"npm run dev:inspector\"",
"dev:inspector": "npx @modelcontextprotocol/inspector node ./dist/index.js",
"dev:inspector": "DANGEROUSLY_OMIT_AUTH=true npx --yes @modelcontextprotocol/inspector@0.14.3 node ./dist/index.js",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

stdio is broken on the latest lol

Comment thread package.json
"dependencies": {
"@modelcontextprotocol/sdk": "1.11.0",
"@modelcontextprotocol/inspector": "0.14.2",
"@modelcontextprotocol/sdk": "1.17.3",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

go to latest sdk, no breaking changes, the 1.0 was cosmetic

Comment thread src/tools/logs.ts
{ subreddit: "r/myTestSubreddit", app: "my-app", since: "15m" }`,
inputSchema: z.object({
subreddit: z.string().describe('Provide the subreddit name. The "r/" prefix is optional'),
since: z.string().describe('Start time for logs (e.g. "15s", "2w1d", "30m").'),
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The actual fix

Comment thread src/tools/logs.ts

return await new Promise((resolve) => {
const child = spawn('devvit', args, {
const child = spawn('npx', args, {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

npx since people could have global 0.11.x installations floating around

Comment thread src/tools/logs.ts
});

const KILL_TIMEOUT_MS = 2_000; // Prevent indefinite hanging by killing process after 10s
const KILL_TIMEOUT_MS = 4_000; // Prevent indefinite hanging by killing process after 4s
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

bump it to make sure all the logs come in. It net slows down the tool call but these agents are super slow as is. This will be one of the fastest parts

Comment thread package.json Outdated
},
"dependencies": {
"@modelcontextprotocol/sdk": "1.11.0",
"@modelcontextprotocol/inspector": "0.14.2",
Copy link
Copy Markdown
Member

@ben-harris-5 ben-harris-5 Aug 15, 2025

Choose a reason for hiding this comment

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

❓ Should this be a dev dependency?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good find! I thought about pinning it here instead but the NPX pin above works so we can remove altogether

Copy link
Copy Markdown
Member

@ben-harris-5 ben-harris-5 left a comment

Choose a reason for hiding this comment

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

Got a Q about a dependency but assuming that's fine the rest seems fine

@mwood23 mwood23 merged commit 2b2958d into main Aug 15, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants