Wrap per-line JSON.parse in try-catch so a single truncated/malformed
entry (e.g. from an OOM mid-write) skips with a warning instead of
aborting the entire batch and dropping all comments.
Co-authored-by: vtmocanu <vtmocanu@users.noreply.github.com>
Two defenses for tag mode where an attacker with repo write access could
craft a prompt injection payload in an issue/PR to gain RCE on the
Actions runner:
1. git-push wrapper (H1 #3556799)
The Bash(git\ push:*) rule permitted arbitrary flags and remotes,
including combinations that execute shell commands locally. Replaced
with scripts/git-push.sh which allowlists exactly 'origin <ref>' with
no flags, validates the ref via check-ref-format. Same pattern as
scripts/gh.sh.
2. acceptEdits instead of blanket Write/Edit (Asana 1213310082312048)
Edit/MultiEdit/Write in allowedTools granted write access to the
whole runner filesystem (~/.bashrc etc). Removed from allowedTools
and set --permission-mode acceptEdits, which auto-accepts edits
inside cwd ($GITHUB_WORKSPACE) and denies outside. Headless SDK has
no prompt handler so 'ask' becomes deny.
Also:
- Noted that create-prompt/index.ts exports ALLOWED_TOOLS env var that
nothing reads. The live path is modes/tag/index.ts. Mirrored the fix
in both so the file the H1 report likely points to stays in sync.
- Updated prompt text (3 callsites) to reference the wrapper.
- Updated tests (4 prompt-content asserts, 7 tool-list asserts).
* feat(inline-comment): add confirmed param + probe-pattern safety net
Subagents that inherit this tool sometimes probe it with test comments
('Test comment to see if I can create inline comments') after hitting
unrelated errors elsewhere. Recurring issue across customer PRs.
Adds two defenses:
1. confirmed param: set true to post (final review comments should pass
this). When false, buffers to a JSONL file instead of posting.
2. Probe-pattern safety net: when confirmed is omitted (backward compat
for existing prompts), the body is checked against obvious probe
patterns ('test comment', 'can i', 'does this work', etc.). Matching
calls are buffered instead of posted.
A post-run step in action.yml reports the buffered call count and bodies
as a workflow warning for diagnostics.
Backward compatibility:
- Existing single-agent prompts (no confirmed param) post normally unless
the body happens to start with a probe phrase (unlikely for real
review comments)
- The code-review skill is being updated to pass confirmed: true in its
final posting step
- Subagent probes that would previously post now harmlessly buffer
* refactor: replace probe-regex with Haiku classification in post-step
The regex approach was narrow and could miss creative probe phrasings.
Replaced with a batch Haiku classification that runs after the session
completes.
Flow:
- MCP server: confirmed !== true -> buffer to JSONL (no classification
in-band, no latency in the tool path)
- Post-step (src/entrypoints/post-buffered-inline-comments.ts): reads
buffer, sends all bodies to a single Haiku call, posts only those
classified as real review comments
- confirmed=false entries are never posted regardless of classification
Fail-open: if ANTHROPIC_API_KEY is unavailable (Bedrock/Vertex users)
or the classification call fails, posts all unconfirmed comments. This
matches pre-PR behavior where all calls posted immediately.
The post-step emits ::warning:: for each filtered comment so users can
see what was dropped and why.
* feat: add classify_inline_comments opt-out input
New action input classify_inline_comments (default 'true'). Setting to
'false' restores pre-buffering behavior: all inline comment calls post
immediately regardless of the confirmed param.
Threads through: action input -> CLASSIFY_INLINE_COMMENTS env ->
context.inputs.classifyInlineComments -> MCP server env ->
CLASSIFY_ENABLED module const.
Post-step is also gated on the input so it skips entirely when
classification is disabled.
* docs: document classify_inline_comments input and confirmed param
- usage.md: add classify_inline_comments to inputs table
- solutions.md: mention confirmed=true in the prompt example and explain
buffering/classification in the tool permissions section
The native binary (install.sh) requires posix_getdents which is only
available in Alpine edge (musl 1.2.5-r21+), not in any stable Alpine
release. Detect musl upfront via ldd and skip straight to npm install,
avoiding 15+ seconds of wasted retries on deterministic failures.
On glibc systems the native install path is unchanged with npm as a
fallback if it fails.
The self-trigger guard blocked the claude bot on all events. Loop risk
only exists for comment/review events (tag mode). PR, workflow_dispatch,
and schedule events are safe — refined the guard to only block on
issue_comment, issues, pull_request_review, and
pull_request_review_comment events.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Agent mode never passed ALLOWED_TOOLS_INPUT to the Claude SDK —
only tag mode did (via --allowedTools in claudeArgs). This meant
the allowed_tools action input was silently ignored in agent mode,
causing all Bash commands to require interactive approval in CI.
Pass ALLOWED_TOOLS_INPUT directly as options.allowedTools, which
the SDK merges with any tools from claudeArgs.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw JSON dumps with concise, scannable log lines when
show_full_output is enabled. Adds an onMessage callback to the SDK
runner that formats each message type into 1-2 line summaries with
emoji prefixes, tool-specific parameter display, result correlation
for parallel tool calls, and subagent progress tracking.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The disallowed tools env var read in run.ts was DISALLOWED_TOOLS_INPUT
(raw user input) instead of DISALLOWED_TOOLS (computed value including
WebSearch/WebFetch defaults). This meant default tool disallowance was
never enforced. Also removes orphaned ALLOWED_TOOLS env var export from
createPrompt() (tag mode passes allowed tools via claudeArgs instead),
extracts shared parseToolsInput utility to eliminate duplication, and
adds tests.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo REST API returns state as lowercase ("closed"/"open") but
branch.ts compares against GraphQL-style uppercase ("CLOSED"/"MERGED").
This caused @claude on closed/merged PRs to take the open-PR checkout
path, failing with "fatal: invalid reference: refs/pull/N/head".
Normalize state to uppercase in the Forgejo data fetcher, matching the
existing pattern used for review state (line 153).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo's pull_request_comment event uses an issue_comment-like payload
with issue.number instead of pull_request.number. Add fallback to
extract entity number from issue field when pull_request is absent.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Removed unused re-exports (CommentUpdateInput, ExecutionDetails,
ensureProperlyEncodedUrl) from forgejo/comment-logic.ts
- Verified no `any` type escapes in Forgejo-specific code
- Confirmed naming conventions consistent with upstream
- No dead code, unused imports, or unreachable branches
- All complex decisions documented with comments
- Typecheck, format, and 834 tests pass clean
Progress: 83% complete — M6 (documentation accuracy) remaining
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- F1: Replace process.exit(1) with throw in branch.ts (cleanup was bypassed)
- F2: Replace process.exit(1) with throw in install-mcp-server.ts (same issue)
- F2b: Replace process.exit(1) with throw in create-prompt/index.ts (same issue)
- F3: Use getServerUrl() instead of GITHUB_SERVER_URL constant in branch-cleanup.ts
- F4: Use updated_at for review TOCTOU filtering in Forgejo data fetcher
- Accepted notes: no pagination >100, no retry, unconditional trigger, image no-op
Progress: 33% complete — M2 done, M3 (security review) next
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The prompt template hardcoded quick_pull=1 (GitHub-specific) in the
"Create a PR" URL. On Forgejo, this parameter is ignored so the
compare page doesn't auto-expand the PR creation form. Forgejo uses
expand=1 for this purpose.
Since Claude writes the PR URL into the comment body, and
updateForgejoCommentBody() extracts it (prLinkFromContent) with
priority over the URL generated by update-comment-link.ts, the
prompt template is the correct place to fix this.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo's issues/assigned webhook payload doesn't include a top-level
assignee object. The assignee is only available under issue.assignee.
Fall back to that field when payload.assignee is missing.
Also removes M5 debug logging.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, all pull_request_comment events on Forgejo triggered the
full Claude pipeline unconditionally — both in the workflow if-condition
and in trigger.ts. This wasted CI resources and API credits on reviews
without @claude mentions (e.g. plain "LGTM" reviews).
Workflow: add @claude check via contains() on comment.body and review.body.
Trigger validation: replace blind auto-trigger with payload-aware check
that inspects available body fields, falling back only when no body exists.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On Forgejo, the tracking comment for pull_request_review_comment events
is always created as a regular issue comment (because Forgejo's webhook
payload lacks the top-level comment object). The MCP server was
incorrectly setting isPullRequestReviewComment=true based on the event
name, causing every update to try pulls.updateReviewComment first —
which 404'd and silently swallowed Claude's responses.
Three fixes:
- MCP server: detect Forgejo and always use issue comment path
- update-comment-link: skip pulls.getReviewComment on Forgejo (saves
one wasted 404 API call)
- update-claude-comment: expand fallback to also catch 422 (defense
in depth for Forgejo edge cases)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mock.module("fs/promises") on Linux Bun poisons the entire mock
tracking system — not just fs mocks, but ALL mock/spy assertions
(octokit mocks, spyOn(global, "fetch"), etc.) stop recording calls.
Replace with dependency injection: downloadCommentImages() now accepts
an optional fsDeps parameter. Tests pass mock mkdir/writeFile directly.
Production callers omit it and get real fs. No mock.module anywhere.
Fixes 18 test failures blocking CI on Forgejo Linux runners.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo fires both issues/opened AND issues/label_updated when an
issue is created with a label. Having the label check on opened
caused duplicate Claude runs. Remove it — the label_updated event
handles label triggers correctly now that the workflow gate and
action normalization are fixed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When an issue is created with the trigger label already applied,
Forgejo only fires issues/opened (not a separate labeled event).
Check payload.issue.labels for the trigger label during opened
events so creating an issue with the claude label works in one step.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Debug logging confirmed Forgejo sends 'label_updated' as eventAction
for issues/labeled events. Normalization fix in context.ts handles it.
Remove temporary debug lines.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo sends eventAction 'label_updated' instead of 'labeled' for
issue label events. Add FORGEJO_EVENT_ACTION_MAP in context.ts to
normalize at parse time (same pattern as event name normalization),
so all downstream code (trigger.ts, create-prompt) works unchanged.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo sends eventAction 'label_updated' instead of 'labeled'
for issue label events (discovered via debug logging, run 52786).
Accept both values in trigger.ts label check.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Temporary debug logging to inspect eventAction and payload.label
on Forgejo for issues/labeled events.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the _fsForTest/DI patterns with Bun's native mock.module() API.
This mocks fs/promises at the module loader level, which works reliably
on Linux where jest.fn() call tracking fails cross-module. The source
code is now clean with no testing artifacts.
Also switches from jest.fn() to mock() (Bun's native API) for Octokit
mocks in these tests.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The _fsForTest mutable object pattern fails on Linux Bun because
test and source files get different module instances. Switch to
passing fs operations as an optional parameter with defaults.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On Forgejo, pull_request_review_comment events create the tracking
comment as a regular issue comment (because the payload lacks
comment.id for reply-to-review). The update-comment-link step then
tried pulls.getReviewComment() first, which 404'd and threw before
reaching the issues.getComment() fallback.
Wrap the PR review comment fetch in its own try/catch so it falls
through to the issue comment fetch on failure.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo's pull_request_review_comment payload doesn't include the
comment body. The Forgejo data fetcher retrieves it via REST instead.
Skip the COMMENT_BODY required check on Forgejo to avoid crashing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forgejo's pull_request_review_comment event does NOT include a top-level
`comment` object in the webhook payload. This caused crashes at:
- create-initial.ts: context.payload.comment.id (for reply-to-review)
- fetcher.ts: context.payload.comment.created_at (for trigger timestamp)
Fix: use optional chaining and fall back gracefully — reply-to-review
becomes a regular issue comment, trigger timestamp returns undefined.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
