create_inline_comment accepts line/startLine of 0, silently collapsing a multi-line comment to single-line

[rajarshidattapy]

Type: bug (input validation)
Severity: low-medium
Area: src/mcp/github-inline-comment-server.ts
Effort: trivial

Summary

line and startLine are validated with zod's .nonnegative(), which permits
0. Git diff line numbers are 1-based, so 0 is never valid - and the code
downstream uses plain falsiness checks, so a 0 is silently reinterpreted rather
than rejected.

Affected code

src/mcp/github-inline-comment-server.ts:51-66

line: z.number().nonnegative().optional()
  .describe("Line number for single-line comments (required if startLine is not provided)"),
startLine: z.number().nonnegative().optional()
  .describe("Start line for multi-line comments (use with line parameter for the end line)"),

The two consumers both treat 0 as "absent":

// line 97
if (!line && !startLine) {
  throw new Error("Either 'line' ... or both 'startLine' and 'line' ... must be provided");
}

// line 137
const isSingleLine = !startLine;

Failure scenarios

A. startLine: 0, line: 12 - schema accepts it. isSingleLine evaluates to
true, so params.start_line and params.start_side are never set. The request
the model asked for (a comment spanning lines 0-12) is posted as a single-line
comment on line 12. The tool reports success. The model has no way to detect that
its range was discarded.

B. line: 0 - schema accepts it, then line 97 rejects it with
"Either 'line' for single-line comments or both 'startLine' and 'line' ... must be provided",
even though line was provided. The message sends the model looking for a
missing argument rather than an out-of-range one.

C. The same !startLine falsiness bug is repeated in the replay path,
src/entrypoints/post-buffered-inline-comments.ts:126 (if (c.startLine)), so a
buffered comment with startLine: 0 collapses to single-line there too.

Neither line nor startLine is constrained to an integer either, so line: 3.5
is accepted by the schema and rejected by the GitHub API as a 422.

Suggested fix

Tighten the schema so invalid values are rejected at the boundary with a clear
zod message, rather than being silently reinterpreted:

line: z.number().int().positive().optional()
  .describe("Line number for single-line comments (required if startLine is not provided)"),
startLine: z.number().int().positive().optional()
  .describe("Start line for multi-line comments (use with line parameter for the end line)"),

With 0 and non-integers excluded at the schema, the existing !line /
!startLine checks become correct as written, so no downstream change is
required.