anthropics / anthropics/claude-code-action

post-buffered-inline-comments: one malformed buffer line discards all buffered comments

Đang mở
#1,667 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
bug p2
Ngôn ngữ chính
TypeScript
Star
8.9k
Fork
2.1k
Chỉ số merge pull request
Chỉ số pull request đang chờ

Mô tả

## Summary

In [`src/entrypoints/post-buffered-inline-comments.ts#L155-L158`](https://github.com/anthropics/claude-code-action/blob/d721746d683d812e669ce117cebe55a85fbd9c3e/src/entrypoints/post-buffered-inline-comments.ts#L155-L158), the buffer file is parsed with an unguarded `JSON.parse` inside a `.map()`. One malformed line throws, the rejection propagates out of `main()`, and the process exits 1 — discarding every valid buffered comment alongside the bad one.

## Current code

```ts
const comments: BufferedComment[] = raw
.split("\n")
.filter(Boolean)
.map((line) => JSON.parse(line));
```

with the top-level handler at [L230-L233](https://github.com/anthropics/claude-code-action/blob/d721746d683d812e669ce117cebe55a85fbd9c3e/src/entrypoints/post-buffered-inline-comments.ts#L230-L233):

```ts
main().catch((e) => {
console.error("post-buffered-inline-comments failed:", e);
process.exit(1);
});
```

So a single unparseable line means: no comments posted, and the `Post buffered inline comments` step fails.

## The codebase already solves this correctly next door

[`src/mcp/inline-comment-buffer.ts#L34-L41`](https://github.com/anthropics/claude-code-action/blob/d721746d683d812e669ce117cebe55a85fbd9c3e/src/mcp/inline-comment-buffer.ts#L34-L41) handles exactly the same file format and guards the parse, with a comment explaining the intent:

```ts
.filter((line) => {
let entry: BufferedCommentMatch;
try {
entry = JSON.parse(line);
} catch {
// Keep anything we cannot parse rather than silently dropping it.
return true;
}
```

The two readers of the same file disagree on how to handle malformed input. The consumer that actually posts to GitHub is the unguarded one.

## How a malformed line can occur

The buffer is written with `appendFileSync` from the MCP server ([`github-inline-comment-server.ts#L111-L124`](https://github.com/anthropics/claude-code-action/blob/d721746d683d812e669ce117cebe55a85fbd9c3e/src/mcp/github-inline-comment-server.ts#L111-L124)). Realistic ways to end up with a partial line:

- The Claude process is killed mid-append (job timeout, cancellation, OOM) leaving a truncated final line.
- Concurrent appends from more than one MCP server process interleave. The server is spawned per session, but subagents inheriting the tool make multiple writers plausible.
- Any pre-existing content at the path from a previous run — see #1542, which reports that this fixed `/tmp` path is never cleared and leaks across runs on self-hosted runners. That issue and this one compound: stale content from another run is exactly the content most likely to be malformed or truncated here.

## Impact

This runs as an `always()` step (`action.yml:431-446`), so when it fails it fails loudly on every affected run. The failure mode is also the worst-shaped one: the user loses *all* review comments Claude produced, and the visible symptom is a red post-step rather than anything pointing at a truncated buffer line.

## No test coverage

The file has no test. `test/inline-comment-buffer.test.ts` covers `removeBufferedComment` in `src/mcp/`, not this entrypoint. Untested behaviour in this module includes:

- the `confirmed === false` partition that must never be posted (L180-L186),
- the classification fallback when `ANTHROPIC_API_KEY` is absent or the API errors, where `classifyComments` returns `null` and everything is posted (L45-L109),
- the response-shape validation that guards against a malformed model response (L92-L101),
- multi-line vs single-line comment parameter construction (L128-L134).

The classification path is also the only place in the action outside the Agent SDK that calls the Anthropic API directly, and it is currently unexercised by any test.

## Suggested fix

1. Guard the parse — skip unparseable lines with a warning naming the line number, and continue with the rest. Dropping a corrupt line is the right call here rather than keeping it as `inline-comment-buffer.ts` does, since this consumer must produce a well-formed API call from each entry.
2. Add a test suite for the entrypoint covering the parse guard, the `confirmed === false` partition, and the classification fallback paths.

Happy to open a PR with both. If maintainers would prefer the fix to land independently of #1542, it is self-contained and does not conflict with changing `BUFFER_PATH`.

## Environment

- Repository at `d721746d683d812e669ce117cebe55a85fbd9c3e` (`main`)

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.