feat(inline-comment): add confirmed param + probe-pattern safety net

[km-anthropic]

Problem

Subagents that inherit mcp__github_inline_comment__create_inline_comment post test/probe comments on customer PRs — e.g. "Test comment to see if I can create inline comments" — when they probe the tool after hitting unrelated GraphQL errors. Recurring issue since Dec 2025.

CLI-level root cause: MCP tools are unconditionally inherited by subagents (filterToolsForAgent in claude-cli-internal short-circuits for mcp__* tools).

Fix

MCP tool: confirmed param + buffering

• confirmed: true → post immediately
• omitted / false → buffer to /tmp/inline-comments-buffer.jsonl, return {success: true, buffered: true}

Post-step: Haiku classification → selective posting

src/entrypoints/post-buffered-inline-comments.ts runs after Claude exits:

1. Reads buffer
2. confirmed: false → never posted
3. confirmed: undefined → batch-classified by one Haiku call: real review comments post, probes dropped with ::warning::
4. Fail-open: if ANTHROPIC_API_KEY unavailable (Bedrock/Vertex) or classification fails, posts everything

Opt-out: classify_inline_comments input

Set to 'false' to restore pre-buffering behavior (all calls post immediately):

- uses: anthropics/claude-code-action@v1
  with:
    classify_inline_comments: 'false'

Behavior matrix

confirmed	classify_inline_comments	Result
true	any	Posts immediately
omitted	'true' (default)	Buffer → Haiku classifies → real posts, probe dropped
omitted	'false'	Posts immediately (legacy)
false	'true' (default)	Buffered, never posted
false	'false'	Posts immediately (legacy)

Why this works for existing customers

Both claude-code-action (via @v1 tag) and the code-review skill (loaded at runtime) auto-update. Existing customers get the fix without re-installing.

Companion skill PR: anthropics/claude-code#33472 (one line: confirmed: true in step 9)

[claude]

The buffered response coaches subagents on how to bypass the safety net.

The message "Set confirmed=true to post" explicitly teaches the calling subagent how to circumvent the guard on its very next call. Since the whole point of this PR is preventing subagent probe comments, this is counterproductive.

Consider removing the confirmed=true coaching and instead saying something like:

"Comment was not posted. If you believe this is a legitimate review comment, return it as text in your response and let the orchestrating agent handle posting."

Alternatively (and more robustly): invert the default so that confirmed: true is required to post. This makes the safety mechanism deterministic rather than relying on the fragile probe regex as the primary gate. Existing callers that never pass confirmed would need updating, but the companion skill PR is already being updated — so this may be the right time.

[claude]

can i\b will match legitimate review comments.

This pattern matches any comment starting with "Can I suggest..." or "Can improvements be made...", which are plausible review comment openings. Consider tightening to something like can i (post|comment|write|test|create)\b to reduce false positives.

Also: the regex is inherently brittle — minor rewording ("trying this out", "hello world", "checking tool access") bypasses it entirely. If you keep the deny-by-default approach from the companion suggestion, this regex becomes a nice-to-have diagnostic rather than the critical safety gate.

[claude]

appendFileSync failure will masquerade as a comment-creation error.

If /tmp is full or the write fails, the exception is caught by the outer catch block which returns "Error creating inline comment: ...". This is misleading since the actual failure was in the buffering path, not in posting.

Consider wrapping the appendFileSync in its own try-catch so a write failure degrades gracefully (still return the "buffered" response, perhaps with a note that logging failed).

[claude]

Comment is misleading — overstates the buffering behavior.

The comment says "Calls without confirmed=true are buffered here instead of posted", but that's not accurate. Calls with confirmed omitted are only buffered if the body matches the probe regex; otherwise they post normally. A future reader would incorrectly assume confirmed=true is mandatory to post.

Suggested change

	// Calls without confirmed=true are buffered here instead of posted. This
	// prevents subagents from posting test/probe comments when they inherit this
	// tool and probe it after hitting unrelated errors. The action's post-step
	// reports the buffer count for diagnostics.
	const BUFFER_PATH = "/tmp/inline-comments-buffer.jsonl";
	// Calls with confirmed=false, or probe-like calls without confirmed=true,
	// are buffered here instead of posted. This prevents subagents from posting
	// test/probe comments when they inherit this tool and probe it after hitting
	// unrelated errors. The action's post-step reports the buffer count for
	// diagnostics.
	const BUFFER_PATH = "/tmp/inline-comments-buffer.jsonl";

[claude]

Buffer contents are cat'd without neutralizing workflow commands.

The buffer body is user-influenced. Strings starting with :: (e.g. ::warning::, ::error::) are interpreted as workflow commands by the Actions runner. A crafted comment body could inject spurious warnings/errors into the log.

Consider wrapping with stop-commands:

Suggested change

	echo "::group::Buffered calls (not posted)"
	cat "$BUF"
	echo "::endgroup::"
	TOKEN=$(uuidgen)
	echo "::stop-commands::${TOKEN}"
	cat "$BUF"
	echo "::${TOKEN}::"

[claude]

Code Review Summary

Good problem identification — subagents posting probe comments on customer PRs is a real issue worth fixing. The layered defense approach (parameter + pattern detection) is reasonable, and the deferred createOctokit is a nice performance improvement.

Key Concern: Default-Allow Posture

The most impactful design question is whether the default should be post unless probe detected (current) vs buffer unless confirmed: true (deny-by-default). The current approach puts the fragile probe regex on the critical path as the primary safety gate. Since the companion skill PR is already updating callers to pass confirmed: true, this may be the right time to invert the default and make the regex a diagnostic rather than the gate.

Additionally, the buffered response message explicitly tells subagents "Set confirmed=true to post" — coaching them on how to bypass the guard on the next call.

Other Findings

• can i\b false positive risk — matches legitimate comments like "Can I suggest refactoring this?"
• appendFileSync error handling — failures in the buffer path surface as misleading "Error creating inline comment" messages
• Workflow command injection — cat "$BUF" in the action.yml post-step doesn't neutralize :: workflow commands in user-influenced content
• Misleading code comment — the BUFFER_PATH comment overstates the buffering behavior
• No test coverage — the probe regex and buffering logic have zero tests. Consider extracting the decision logic into a pure function (e.g., shouldBuffer(confirmed, body)) for easy unit testing
• Hardcoded /tmp path — duplicated between TypeScript and action.yml; consider using RUNNER_TEMP and passing via env var

What Looks Good

• Deferred createOctokit avoids unnecessary API client creation for buffered calls
• JSONL format is pragmatic for append-only structured logging
• Body sanitization before writing to buffer is correct
• action.yml diagnostic step with if: always() is well-placed

See inline comments for specific suggestions.