anthropics / anthropics/claude-code-action

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

Ouverte
#1,667 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
bug p2
Langage dominant
TypeScript
Étoiles
8.9k
Forks
2.1k
Métriques de merge des PR
Métriques de PR en attente

Description

## 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`)

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.