-
-
Notifications
You must be signed in to change notification settings - Fork 874
fix(cli): various mcp server fixes #2430
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
Changes from all commits
7c6cb14
ca882a7
20c547a
46248ff
0747699
b994978
f1e430a
f39dce7
42d668e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -123,7 +123,13 @@ export const CommonRunsInput = CommonProjectsInput.extend({ | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type CommonRunsInput = z.output<typeof CommonRunsInput>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const GetRunDetailsInput = CommonRunsInput.extend({}); | ||||||||||||||||||||||||||||||||||
| export const GetRunDetailsInput = CommonRunsInput.extend({ | ||||||||||||||||||||||||||||||||||
| maxTraceLines: z | ||||||||||||||||||||||||||||||||||
| .number() | ||||||||||||||||||||||||||||||||||
| .int() | ||||||||||||||||||||||||||||||||||
| .describe("The maximum number of lines to show in the trace. Defaults to 500") | ||||||||||||||||||||||||||||||||||
| .optional(), | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+126
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Constrain and coerce maxTraceLines to safe bounds Good addition. To prevent pathological inputs (negative, zero, NaN, huge), coerce and bound the value. Also clarify the described default range. Apply this diff: export const GetRunDetailsInput = CommonRunsInput.extend({
- maxTraceLines: z
- .number()
- .int()
- .describe("The maximum number of lines to show in the trace. Defaults to 500")
- .optional(),
+ maxTraceLines: z
+ .coerce.number()
+ .int()
+ .min(1)
+ .max(10000)
+ .describe("The maximum number of lines to show in the trace. Defaults to 500 (range: 1–10,000)")
+ .optional(),
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type GetRunDetailsInput = z.output<typeof GetRunDetailsInput>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,7 +74,6 @@ export const deployTool = { | |
| cwd: cwd.cwd, | ||
| env: { | ||
| TRIGGER_MCP_SERVER: "1", | ||
| CI: "true", | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,7 +35,7 @@ export const getRunDetailsTool = { | |||||||||||||
| ]); | ||||||||||||||
|
|
||||||||||||||
| const formattedRun = formatRun(runResult); | ||||||||||||||
| const formattedTrace = formatRunTrace(traceResult.trace); | ||||||||||||||
| const formattedTrace = formatRunTrace(traceResult.trace, input.maxTraceLines); | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+38
to
39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Clamp user-supplied maxTraceLines before passing to formatter Even with Zod validation, clamping at the call site makes the behavior robust and aligns with the schema bounds. Apply this diff: - const formattedTrace = formatRunTrace(traceResult.trace, input.maxTraceLines);
+ const maxTraceLines =
+ typeof input.maxTraceLines === "number" && Number.isFinite(input.maxTraceLines)
+ ? Math.min(Math.max(Math.trunc(input.maxTraceLines), 1), 10_000)
+ : undefined;
+ const formattedTrace = formatRunTrace(traceResult.trace, maxTraceLines);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| const runUrl = await ctx.getDashboardUrl(`/projects/v3/${projectRef}/runs/${runResult.id}`); | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,7 +12,7 @@ import { SemanticInternalAttributes } from "../semanticInternalAttributes.js"; | |||||||||||||||
| import { TriggerTracer } from "../tracer.js"; | ||||||||||||||||
| import { zodfetch } from "../zodfetch.js"; | ||||||||||||||||
| import { flattenAttributes } from "./flattenAttributes.js"; | ||||||||||||||||
| import get from "lodash.get"; | ||||||||||||||||
| import { JSONHeroPath } from "@jsonhero/path"; | ||||||||||||||||
|
|
||||||||||||||||
| export type IOPacket = { | ||||||||||||||||
| data?: string | undefined; | ||||||||||||||||
|
|
@@ -536,7 +536,7 @@ export async function replaceSuperJsonPayload(original: string, newPayload: stri | |||||||||||||||
| .map(([key]) => key); | ||||||||||||||||
|
|
||||||||||||||||
| const overridenUndefinedKeys = originalUndefinedKeys.filter( | ||||||||||||||||
| (key) => get(newPayloadObject, key) !== undefined | ||||||||||||||||
| (key) => getKeyFromObject(newPayloadObject, key) !== undefined | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
|
Comment on lines
538
to
541
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Path lookup returns a Node object, so
This would cause us to strip “undefined” metadata from SuperJSON even when the new payload still contains undefined at that path. Apply this minimal fix (and optional typo fix) to the filter logic: - const overridenUndefinedKeys = originalUndefinedKeys.filter(
- (key) => getKeyFromObject(newPayloadObject, key) !== undefined
- );
+ const overriddenUndefinedKeys = originalUndefinedKeys.filter(
+ (key) => getKeyFromObject(newPayloadObject, key) !== undefined
+ );Note: This relies on getKeyFromObject returning the raw value (undefined when the value is undefined). See suggested fix to getKeyFromObject in a separate comment. Optionally fix the spelling: “overridden” instead of “overriden” for clarity. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| overridenUndefinedKeys.forEach((key) => { | ||||||||||||||||
|
|
@@ -551,3 +551,9 @@ export async function replaceSuperJsonPayload(original: string, newPayload: stri | |||||||||||||||
|
|
||||||||||||||||
| return superjson.deserialize(newSuperJson); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| function getKeyFromObject(object: unknown, key: string) { | ||||||||||||||||
| const jsonHeroPath = new JSONHeroPath(key); | ||||||||||||||||
|
|
||||||||||||||||
| return jsonHeroPath.first(object); | ||||||||||||||||
| } | ||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Line-cap can be exceeded mid-span; enforce a hard cap and reserve room for the notice
Currently, a single large span can push many lines before the recursive call returns, resulting in outputs that exceed the intended cap. Enforce the cap after formatting and ensure the truncation notice is included deterministically.
Apply this diff:
export function formatRunTrace( trace: RetrieveRunTraceResponseBody["trace"], maxTraceLines: number = DEFAULT_MAX_TRACE_LINES ): string { const lines: string[] = []; lines.push(`Trace ID: ${trace.traceId}`); lines.push(""); // Format the root span and its children recursively - const reachedMaxLines = formatSpan(trace.rootSpan, lines, 0, maxTraceLines); - - if (reachedMaxLines) { - lines.push(`(truncated logs to ${maxTraceLines} lines)`); - } + const reachedMaxLines = formatSpan(trace.rootSpan, lines, 0, maxTraceLines); + + // Enforce a hard cap and include the truncation notice within the cap + const truncated = reachedMaxLines || lines.length > maxTraceLines; + if (truncated) { + if (lines.length >= maxTraceLines) { + // leave room for the truncation notice line + lines.length = Math.max(0, maxTraceLines - 1); + } + lines.push(`(truncated logs to ${maxTraceLines} lines)`); + } return lines.join("\n"); }As an optional follow-up, we can short-circuit earlier inside formatSpan after heavy sections (properties/output/events) to avoid extra work. Happy to provide that refinement if you want it.
📝 Committable suggestion
🤖 Prompt for AI Agents