-
-
Notifications
You must be signed in to change notification settings - Fork 29
Add a CLI that agents can also use instead of the MCP #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This CLI introduces commands like `logs` or `errors` which can also be combined with a `,` or `+` to listen to multiple streams at once. The first iteration will be a streaming log like `tail -f` but in the next iteration we will add a `no-follow` mode that accepts some filters like a `timeWindow` or `limit` as the MCP does. This requires connecting to an already running upstream sidecar instance, which we also wanna address with the streaming/follow mode.
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
250 new lines and 17 files modified but not a single comment from cursor 🥇. Having a look at this later today (I want to try it locally also) edit: oh cursor took like 10 mins to leave a comment and there were no 👀 in the PR description... |
Examples: | ||
spotlight-sidecar # Start on default port 8969 | ||
spotlight-sidecar --port 3000 # Start on port 3000 | ||
spotlight-sidecar -p 3000 -d # Start on port 3000 with debug logging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug: The code does not validate the result of envelope.getParsedEnvelope()
, leading to a crash if the envelope is malformed (null) or empty (empty items array).
-
Description: The function
envelope.getParsedEnvelope()
can returnnull
if the envelope header is malformed. The code destructures this result viaconst { event } = envelope.getParsedEnvelope()
without a null check, which will cause the process to crash. Additionally, even with a valid envelope, if it contains no items, theevent[1]
array will be empty. The subsequent accessevent[1][0][0].type
will then cause a crash due to an out-of-bounds access. Both scenarios, a malformed header or a valid but empty envelope, will lead to a server crash. -
Suggested fix: Before destructuring or accessing properties, check if the return value of
envelope.getParsedEnvelope()
is null. Also, add a check to ensure theevent[1]
array (the envelope items) is not empty before attempting to access its elements. A guard clause likeif (!envelope || !event || event[1].length === 0) { return; }
would prevent both crashes.
severity: 0.9, confidence: 0.95
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass. Added a couple of commits to fix some bugs. I'll do another review later.
I tried it locally, it's great and we need to improve the formatter 😆 .
209026c
to
29dc7af
Compare
This CLI introduces commands like
logs
orerrors
which can also becombined with a
,
or+
to listen to multiple streams at once.The first iteration will be a streaming log like
tail -f
but in the nextiteration we will add a
no-follow
mode that accepts some filters like atimeWindow
orlimit
as the MCP does.Closes #958.