Add bulk live-comment apply for agent reviews

[benvinegar]

Summary

• add hunk session comment apply so agents can send many live comments in one stdin JSON payload
• validate the whole batch before mutating the live session and keep focus stable by default, with optional --reveal-last
• update the repo skill and docs to show the new batch workflow for agent-driven reviews

Testing

• bun run typecheck
• bun test

Depends on #161 and addresses the remaining bulk-apply part of #158.

This PR description was generated by Pi using OpenAI o3

[greptile-apps]

Greptile Summary

This PR adds hunk session comment apply — a bulk batch-apply command that lets agents send many live inline review notes in a single stdin JSON payload, validate the whole batch before any state mutation, and optionally reveal the last note with --reveal-last.

Key changes:

• src/core/cli.ts: Adds parseSessionCommentApplyPayload (thorough input validation: type checks, hunk/hunkNumber alias reconciliation, exactly-one-selector enforcement) and the comment apply subcommand parser.
• src/mcp/types.ts: Adds CommentBatchToolInput, AppliedCommentBatchResult, and the comment_batch server message variant. CommentBatchToolInput.comments is typed as CommentToolInput[], which carries unused per-item session-targeting fields inherited from SessionTargetInput.
• src/ui/hooks/useHunkSessionBridge.ts: applyIncomingCommentBatch builds all prepared entries (including target resolution) before calling setLiveCommentsByFileId, giving true all-or-nothing state update semantics within a single React state transition.
• test/mcp-e2e.test.ts: Replaces three hardcoded listen ports with a reserveLoopbackPort() helper — a positive improvement, though the helper has a minor TOCTOU race between releasing the port and the server claiming it.
• Docs and skill guide updated with the new batch workflow and --reveal-last guidance.

Confidence Score: 5/5

Safe to merge — all remaining findings are P2 style suggestions with no impact on runtime correctness.

The implementation is well-structured: batch validation happens before any state mutation (all-or-nothing semantics), the 1-based → 0-based hunk offset translation is correctly applied in server.ts, and the revealMode default-none behaviour is properly enforced. Tests cover the daemon routing, HTTP server forwarding, CLI integration, and the full PTY session path. The only open items are a type-accuracy suggestion (CommentBatchToolInput.comments carrying unused per-item session fields) and a minor TOCTOU note on reserveLoopbackPort — neither affects correctness.

No files require special attention; the P2 type note is in src/mcp/types.ts lines 77–80.

Important Files Changed

Filename	Overview
src/core/cli.ts	Adds parseSessionCommentApplyPayload validator and the comment apply subcommand handler; validation is thorough (type checks, alias reconciliation, exactly-one-selector enforcement) and stdin reading is correctly guarded behind --stdin.
src/mcp/types.ts	Adds CommentBatchToolInput, AppliedCommentBatchResult, and the comment_batch server message variant; CommentBatchToolInput.comments is typed as CommentToolInput[] which carries unused per-item session-targeting fields.
src/ui/hooks/useHunkSessionBridge.ts	Adds applyIncomingCommentBatch — builds all prepared entries before any state mutation (true all-or-nothing semantics), then applies in one setLiveCommentsByFileId update; --reveal-last path correctly jumps to the final entry only.
src/mcp/server.ts	Routes comment-apply to sendCommentBatch, correctly translating 1-based hunkNumber from the CLI into 0-based hunkIndex for the internal API.
test/mcp-e2e.test.ts	Replaces three hardcoded ports with reserveLoopbackPort(); the helper has a minor TOCTOU race but is a clear improvement over fixed ports.
test/session-cli.test.ts	New integration test for comment apply using hardcoded port 48964, consistent with the rest of the file's pattern; verifies focus is unchanged by default and both comments are listed afterward.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant CLI as hunk session comment apply
    participant Daemon as HunkDaemonState
    participant Session as Live Hunk Session
    participant Bridge as useHunkSessionBridge

    Agent->>CLI: stdin JSON { comments: [...] }
    CLI->>CLI: parseSessionCommentApplyPayload()<br/>(validate all items before forwarding)
    CLI->>Daemon: POST /session-api action=comment-apply
    Daemon->>Session: WebSocket comment_batch command
    Session->>Bridge: applyIncomingCommentBatch()
    Bridge->>Bridge: map() — resolve all targets<br/>(throw on first bad filePath/hunk)
    Bridge->>Bridge: setLiveCommentsByFileId()<br/>(single atomic state update)
    alt revealMode === "last"
        Bridge->>Session: jumpToFile(last entry)
        Bridge->>Session: openAgentNotes()
    end
    Bridge-->>Session: AppliedCommentBatchResult
    Session-->>Daemon: { ok: true, result }
    Daemon-->>CLI: AppliedCommentBatchResult
    CLI-->>Agent: JSON or text output

Loading

Comments Outside Diff (1)

1. test/mcp-e2e.test.ts, line 764-775 (link)

   reserveLoopbackPort has a TOCTOU window

   The helper closes the listener before returning the port, so there is a brief window between listener.close() and the server's listen() call where another process could claim the same port. In a busy CI environment this can cause rare, hard-to-diagnose test failures.

   A more robust approach is to let the OS assign the port and keep the socket open until the server takes it over (hand off), or simply accept the existing rare-race trade-off and document it. As a lightweight alternative you could guard the listening attempt in the test with a retry:

   // alternative: pass the already-open server socket to Bun.serve / the test server
   // so the port is never released

   This is better than the previous hardcoded ports, so it's an improvement — just worth noting the residual race.

_{Reviews (1): Last reviewed commit: "Add bulk live-comment apply command" | Re-trigger Greptile}

[greptile-apps]

Per-item type carries unused session-targeting fields

CommentBatchToolInput.comments is typed as CommentToolInput[], but CommentToolInput extends SessionTargetInput, which adds sessionId?, sessionPath?, and repoRoot? to every item in the array. These fields have no effect per-item — the session is resolved once at the batch level via CommentBatchToolInput extends SessionTargetInput. The per-item session fields are silently discarded in server.ts when mapping over input.comments.

This makes the type slightly misleading: a reader looking at the interface would expect per-comment session routing, but that capability doesn't exist. Consider introducing a narrower type (e.g. CommentItemInput) that carries only the comment-specific fields (filePath, hunkIndex, side, line, summary, rationale, author) and using that for the comments array:

export interface CommentItemInput {
  filePath: string;
  hunkIndex?: number;
  side?: DiffSide;
  line?: number;
  summary: string;
  rationale?: string;
  author?: string;
}

export interface CommentBatchToolInput extends SessionTargetInput {
  comments: CommentItemInput[];
  revealMode?: "none" | "last";
}

[benvinegar]

Agreed. I split the shared comment payload shape away from session targeting, so batch items now use a comment-only type and no longer imply per-item session routing. The single-comment path still carries session selectors at the top level, while both paths reuse the same comment target fields.

This comment was generated by Pi using OpenAI o3