main
77 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 wolfy
|
0438688e32 | fix: issues with pagination not resolving correct url templates | ||
|
wolfy
|
2aca1a3aa3 | feat: adapt pullfrog for gitea + ollama | ||
 Colin McDonnell
|
7e90e5cae6 |
Align Plan-mode prompts on report_progress as the canonical plan tool (#786)
* fix: align Plan-mode prompts on report_progress as the canonical plan tool Fixes #673. Three sites disagreed on where Plan output should be posted, letting a model synthesize a broken third interpretation (initial post via `report_progress({ target_plan_comment: true })`, which then misses the `existingPlanCommentId` precondition). This PR aligns all three on `report_progress` as canonical, with `target_plan_comment` reserved for revisions only: - `action/modes.ts` Plan step 4 — spell out that the initial plan post uses `report_progress` WITHOUT `target_plan_comment`, and that revisions go through `select_mode`'s PlanEdit override. - `action/mcp/comment.ts` `target_plan_comment` flag description — make the "revisions only" precondition explicit and call out the initial-post path by name. - `action/utils/instructions.ts` Progress reporting paragraph — drop the misleading "(e.g., Plan comments)" parenthetical that read as "use create_issue_comment for plans". `PlanEdit` (in `action/mcp/selectMode.ts`) was already correct and is unchanged. Intentionally out of scope (to keep the fix minimal): a `publish_plan` tool, removing the vestigial `create_issue_comment({ type: "Plan" })` branch, hardening the run-end cleanup guard for the `target_plan_comment but no existingPlanCommentId` fallthrough, and renaming `target_plan_comment`. * align create_issue_comment description with report_progress as canonical plan tool |
||
|
Colin McDonnell
|
f3d18401ac |
eager signup credit + free-OpenCode fallback when BYOK has no key (#789)
* eager signup credit + free-OpenCode fallback when BYOK has no key
addresses the silent-churn pattern that took out 15 first-run-failure
accounts post-launch: GH Actions secret references resolved to empty
strings (because the secrets didn't exist on the repo), the action
launched Claude Code with no key, the LLM provider 401'd, and the run
died in seconds with a synthetic "Invalid API key" message. those
accounts had no Router credits to fall back to because the lazy claim
required a dashboard visit they never made.
three changes, one PR:
1. Eager $10 signup credit at account creation. Both account-creation
sites (`upsertAccountByClerkId` for dashboard signin, `fetchOrCreateRepo`
for CLI / GH-App-only) now insert the `CreditGrant { reason: "signup" }`
in the same transaction as the `accounts` row. CLI installers who
never sign in get the credit. The dashboard `/signup-credit/claim`
POST stays as an idempotent backstop for accounts created before
this shipped.
2. Free-OpenCode fallback in the action. When the configured BYOK slug
needs a provider key the runner doesn't have, swap to
`opencode/minimax-m2.5-free` before agent selection so the run still
succeeds. Surfaced via a `» fell back from <slug> to <free>` warning
in the action log. Skipped on Router runs (Pullfrog mints the key)
and when no model is configured (auto-select-with-throw still fires
for the genuinely-misconfigured case).
3. New action-test fixture `byok-no-keys-fallback` that empty-strings
every known provider key (matching how GH Actions handles missing
secrets) and asserts the run succeeds with the fallback log line
present. plus a unit test for the helper covering each skip case.
skipping the schema flip from `byok` to `router` — that's coming via
the onboarding-stepper PR (#762).
* fallback: skip Bedrock + surface in PR-comment footer
addresses copilot review on #789 (real bug — parseModel throws on
Bedrock raw IDs that have no slash, would crash before
validateBedrockSetup could surface its own error) and the user-side
ask to make the fallback visible in PR comments.
- selectFallbackModelIfNeeded skips when resolvedModel has no '/' so
Bedrock routing IDs (e.g. us.anthropic.claude-opus-4-7) don't crash
inside hasProviderKey -> parseModel. unit test covers it.
- toolState.modelFallback records the configured slug we fell back
from. set in main.ts when fallback engages.
- buildPullfrogFooter accepts fallbackFrom and renders
"Using `MiniMax M2.5` (free) (credentials for Claude Opus not
configured)" so the substitution is visible in PR comments,
reviews, PR bodies, and error reports.
- threaded through all four action-side footer call sites
(mcp/comment, mcp/pr, mcp/review, utils/errorReport). server-side
call sites in triggerWorkflow.ts / handleWorkflowRunWebhook.ts
fire pre-action and don't have toolState — left as-is.
* fallback footer: use provider display name + document email asymmetry
addresses pullfrog reviewer findings on #789:
- footer now shows 'credentials for Anthropic not configured' (provider
display name from `providers.anthropic.displayName`) instead of the
per-model name. credentials are provider-scoped (ANTHROPIC_API_KEY
covers all Anthropic models), so this matches what the user actually
needs to fix.
- document the intentional asymmetry between eager and lazy signup
credit paths: eager skips both the signupCreditClaimedEmail and the
per-grant team@ alert. comment explains why (the 'new account
created' alert already covers it on the eager path; the user-facing
email assumes a user-initiated action that hasn't happened yet for
CLI/GH-App-only signups).
- skipping the backfill for the 15 historical accounts per user's
earlier decision — they all uninstalled, so the cohort self-selected
out of being reachable.
* fallback: gate on resolvedModel + skip resolveModel re-resolve post-swap
local agnostic fixture run surfaced two real bugs the unit tests didn't
catch:
1. fallback gate was on configuredSlug (=payload.model) but the test
uses PULLFROG_MODEL to set the model, which is read by resolveModel
AFTER its slug arg. configuredSlug stayed undefined → fallback never
fired. drop configuredSlug from the helper signature; gate purely on
resolvedModel since that's the same value regardless of how the
model was specified (DB config vs PULLFROG_MODEL env).
2. when fallback engaged, the post-swap resolveModel({slug: fallback.to})
call was ALSO honoring PULLFROG_MODEL, re-overriding the fallback
target back to the unkeyed model. validateAgentApiKey then threw
"no API key found" against the original model. fix: skip the
re-resolve. fallback.to is already a CLI-ready specifier.
unit tests updated for the new helper signature (8 tests, all pass).
fallback log line confirmed emitted in the local run pre-second-fix;
the second fix unblocks the validation that previously threw.
|
||
|
Colin McDonnell
|
b8ac42e875 |
mcp: embed example calls in top-level tool descriptions (#723)
* mcp: embed example calls in top-level tool descriptions
agents (esp. claude sonnet) hallucinate param names from training-data
priors — `pr_number` instead of `pull_number`, `summary` instead of
`body`, full subcommand strings jammed into `git({command})` like it
were `shell({command})`. each error burns a tool round-trip plus a
follow-up ToolSearch, ~40+ events / 24h, no observable recovery cost
to us but visible to users in agent logs.
cheapest fix: add a sample formatted function call to every affected
tool's top-level description. example anchors are more reliable than
schema descriptions alone because the model treats descriptions as
narrative but call examples as canonical structure. for `git` and
`shell` (whose `command` fields collide), include explicit
counter-examples disambiguating which tool owns which shape.
no schema aliases / coercion yet — try the cheap thing first; if the
next audit window still shows the same hallucination rate, layer
aliases on top per #585's recommendation.
closes #585, closes #701
* mcp: drop negative anchors from tool descriptions
negation is a footgun in tool descriptions — telling the model "NOT
pr_number" makes pr_number more salient, not less. let the positive
example carry the schema and trust the model to read it.
removes:
- "the parameter is pull_number (a number), NOT pr_number" and
similar across checkout_pr, get_pull_request, list_pull_request_reviews,
get_review_comments, create_pull_request_review
- "NOT summary, message, or content" on report_progress
- "WRONG: git({ command: 'log --oneline' })" counter-example on git
- redundant param-type restatements after the example (e.g. "depth is a
number, not a string" on git_fetch, "description is required" on shell)
keeps a single positive example per tool. for tools with multiple call
shapes (git, git_fetch, push_branch), two positive examples instead of
one + a counter-example.
|
||
|
David Blass
|
10aeaf8c11 |
action: dedupe identical reply_to_review_comment calls within a session (#623)
* action: dedupe identical reply_to_review_comment calls within a session PR #610 reproduced a Kimi K2 stutter where the agent's tool_use surface showed one `pullfrog_reply_to_review_comment` call but GitHub recorded two byte-identical POSTs 3s apart, leaving a duplicate response on `action/mcp/review.ts:14`. Add `duplicateReplyDecision` (mirrors `duplicateReviewDecision`) and track per-session replies on `ToolState.reviewReplies`, keyed by parent `comment_id` + `bodyWithFooter`. Identical re-emissions short circuit with `{ skipped: true, reason }` instead of POSTing again. Body-keyed (not just id-keyed) so legitimate follow-up replies with different content still go through. Tighten `AddressReviews` step 5 to say *exactly once per comment* and note that the runtime dedupes identical bodies, so the agent has both prompt-level guidance and a server-side guarantee. Co-authored-by: Cursor <cursoragent@cursor.com> * address review: drop stale file ref in dedupe comment; soften tool description * remove comment.test.ts --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Colin McDonnell
|
2e6c01670e |
mcp: log artifact id after every github write (#633)
makes debugging easier by emitting a single `» <verb> <kind> <id>` line after every successful GitHub write (and upload) the agent performs via the Pullfrog MCP, mirroring the chevron convention used elsewhere. |
||
|
Colin McDonnell
|
e4e93ea6d3 |
PR summary as agent-edited tmpfile snapshot (#568)
* PR summary as agent-edited tmpfile snapshot Replaces the comment-based PR summary path (and the in-progress update_pr_summary tool from #534) with a snapshot file the agent edits in place during Review / IncrementalReview / pr-summary Task runs. The server seeds the tmpfile with the previous snapshot (incremental) or a stable scaffold (first run), exposes the path via select_mode, and reads it back at end-of-run to persist to WorkflowRun.summarySnapshot and (when the prSummaryComment toggle is on) splice into the PR description body. Why a tmpfile rather than a tool call: incremental snapshot edits are output-token-cheap when the agent uses native file-editing tools, and range-diff cleanly across runs because section headings are stable. The agent never has to regurgitate the full snapshot to update it. Gating: snapshot generation is opt-in via either prSummaryComment="enabled" (splice into PR body) or prReReview="enabled" (snapshot feeds future incremental review runs as context). Users who disable both pay nothing end-to-end — no seeding, DB write, or body splice. Behavior changes: - Drop the Summarize mode and the Summary comment type entirely; the rolling summary is no longer a separate run shape. - pull_request_synchronize with re-review off and summary on still dispatches a silent pr-summary Task, but it edits the snapshot file instead of posting a fresh comment. - /api/repo/.../pr/.../summary-comment now returns { snapshot: string | null } from the DB instead of fetching a comment via GraphQL. URL kept stable so deployed older actions degrade gracefully. - summaryCommentNodeId is retained on WorkflowRun for legacy data and a future backfill of pre-snapshot comment-based summaries. Supersedes #534. The commit-tool/sub-agent direction in that PR is abandoned in favor of this file-based shape. * address review pass #1: synchronize fallback, splice idempotency, docs * address review pass #2: in-flight skip should not race summary fallback * address review pass #3: signal-handler flush, doc clarifications * address review pass #4: in-flight persist promise + bounded body-splice timeout * address review pass #5: defensive catch on persist worker, doc nit * add summary-stale post-run gate When generateSummary is set, we capture the bytes of the seeded snapshot file and pass them to the agent's post-run loop alongside the file path. After each agent attempt, the loop diffs the current file against the seed; if they're byte-identical the agent never touched it, and we nudge once via a resume turn (similar to the dirty-tree gate, but soft and fire-once so smaller models that legitimately decide no edit is warranted don't burn the retry budget). Mostly defends against forgetful smaller models on the Review path — their mode prompt asks them to edit the snapshot file, but the multi-step instruction can fall through when the diff is large. * trigger: retry vercel preview build * fix(action): drop unused re-export that pulled node:fs/promises into next bundle action/internal/index.ts was re-exporting DEFAULT_PR_SUMMARY_INSTRUCTIONS from action/utils/prSummary.ts, but nothing in the next.js app imports it. prSummary.ts uses node:fs/promises, and pullfrog/internal is aliased into the next bundle by next.config.ts, which made turbopack try to resolve node:fs/promises in client chunks and fail with: the chunking context (unknown) does not support external modules (request: node:fs/promises) drop the re-export — selectMode.ts (the only real consumer) already imports it directly from action/utils/prSummary.ts. * firewall PR summary snapshot from user instructions; resurrect rich format for Review The agent-internal snapshot (the markdown file the agent edits in place across runs) is exclusively durable context for future agent runs — user-supplied summarization instructions warp it and degrade that context. Drop the prSummaryCommentInstructions read path end-to-end: - handleWebhook: stop reading prSummaryCommentInstructions, stop passing prSummaryInstructions through dispatch options - action payload + ToolState + selectMode addendum: drop the instructions appendix; the snapshot prompt is fixed, not user-shaped - TriggersSettings: drop the InstructionsEditor for prSummaryCommentInstructions - prSummary.ts: reframe DEFAULT_PR_SUMMARY_INSTRUCTIONS as agent-targeted (durable context, not human-facing prose) Prisma columns (prSummaryComment, prSummaryCommentInstructions) and the matching zod schema entry stay for graceful retreat. Separately, resurrect PR_SUMMARY_FORMAT (deleted along with the Summarize mode in the original PR) and wire it into Review mode only. Initial PR reviews now include a structured summary section in the review body using the rich format (TL;DR, key changes, ## sections with before/after, file-link trails). IncrementalReview keeps its existing terser bullet-list shape since re-review bodies are deltas, not introductions. The user-facing review summary and the agent-internal snapshot are deliberately separate artifacts with separate prompts and zero shared content. * address review comments: prompt self-consistency + stale-doc cleanup PR 568 self-review (4232488109) flagged a self-contradiction the firewall commit introduced and three stale doc references that survived. - action/modes.ts: Review-mode step 2's trivial-PR shortcut said `submit "Reviewed — no issues found." per step 5`, but step 5's rewrite removed exactly that preamble. Aligned both: trivial PRs and no-actionable-issues PRs now produce a body that opens with "No new issues found." followed by the PR summary, so the user gets the headline up front and still sees what was reviewed. - docs/pr-reviews.mdx: dropped the "customize the summary style with Summary instructions in the console" sentence (the editor was removed in the firewall commit). Replaced with a note that the snapshot uses Pullfrog's built-in format and is not user-customizable. - wiki/prompt.md, wiki/modes.md: rewrote the snapshot-prompt entries to reflect the firewall — DEFAULT_PR_SUMMARY_INSTRUCTIONS is the entire prompt, prSummaryCommentInstructions is no longer wired in. * drop orphaned prSummaryCommentInstructions column Prod audit (455 repos): 5 non-null rows on a single account, all containing the literal placeholder text from the InstructionsEditor we removed in the firewall commit. No account has an intentional preference set, so silent-ignore (the keep-for-retreat option) costs us nothing meaningful while leaving an orphan column in the schema. Drop it. - prisma/schema.prisma: remove the column - prisma/migrations/20260506000000_drop_pr_summary_comment_instructions: ALTER TABLE ... DROP COLUMN - utils/schemas/triggers.ts: drop the matching zod entry * drop body splicing; snapshot is internal-only User-visible PR summarization continues to ship in Review and IncrementalReview review bodies (which already render PR_SUMMARY_FORMAT and "Reviewed changes" respectively). The snapshot tmpfile is now purely durable cross-run agent context — seed, edit-in-place, save to DB, feed the next run. Massive simplification: the body splice mechanics, the two-toggle gating matrix, the summaryHandlingCovered race tracking, and the synchronize summary-only Task fallback all go away. Code: - prSummary.ts: drop splice/strip/marker code (`splicePrSummary`, `stripExistingSummaryBlock`, `buildSummaryBlock`, `extractPrSummary`, PULLFROG_SUMMARY_START/END). keep scaffold, instructions, seed/read. - main.ts: rename persistAndPostSummary -> persistSummary; collapse to a single DB PATCH. drop pulls.get/pulls.update, drop AbortSignal timeout, drop in-flight promise machinery, drop prSummaryToBody plumbing. - ToolState: add summarySeed (replaces local var in main.ts so persist can compare). drop prSummaryToBody and summaryPersistInFlight. - persistSummary now compares against the seed and skips the DB write with a warning when unchanged — saving the seed verbatim is either a no-op or persists the placeholder scaffold, neither useful. - postRun.ts: when summary-stale is the only failing gate and the resume turn itself fails, restore the pre-resume successful result and break. symmetric with the existing reflection-failure preservation. summary-stale can no longer flip a successful run to failed. Webhook: - pull_request_opened: generateSummary follows prReReview only (the snapshot has no consumer when re-review is off). - pull_request_synchronize: collapses to "if prReReview enabled, dispatch IncrementalReview". the summaryHandlingCovered flag, the same-SHA/in-flight coordination it was protecting, and the summary-only Task fallback all delete cleanly. UI / config: - drop SummarizePRsTrigger (the toggle gated body splice; with that gone it has no behavior). drop sidebar entry, console import, Text icon import. - drop prSummaryComment from triggers zod schema, prisma schema, preview settings script. Migration: squash the two existing migrations into one timestamped 20260507000000_pr_summary_snapshot covering all three column changes (add summarySnapshot on workflow_runs, drop prSummaryCommentInstructions and prSummaryComment on repos). repo convention is one migration per PR. Action: bump 0.0.203 -> 0.0.205 (payload contract changed: prSummaryToBody removed; main is at 0.0.204). Out-of-diff cleanup: - review.ts:190 + review.test.ts:651 — "Reviewed — no issues found." -> "No new issues found." to match the canonical body in modes.ts. Verified: pnpm typecheck clean, pnpm lint clean, postRun + review tests pass, dev DB reset against production and the squashed migration applied cleanly (summarySnapshot present, prSummaryComment / prSummaryCommentInstructions both gone). * re-orient snapshot toward functional summary; drop prior-review-feedback section Empirical audit on preview-568 PR #5 showed the snapshot IS load-bearing for the orchestrator: lens-dispatch prompts on incremental runs carried forward context from the snapshot's risk register (e.g. "the JSDoc explicitly scopes to code points — do not flag grapheme-cluster issues" on the surrogate-pair fix run, "consistency with native padStart" on the padStart-added run). The orchestrator was reading the snapshot, reasoning about it, and using it to anti-prime / focus subagents — exactly the high-leverage path. My earlier "snapshot is write-only" claim was wrong. The shape, however, was steering it toward review-history-log instead of functional summary. This commit re-orients: - prSummary.ts: replace the four-section scaffold (~580 chars of placeholder italics under "What this PR does / Key changes / Risk / Reviewed in prior runs") with a minimal seed (~150 chars: just a header + a one-line comment about what the file is for). different PRs warrant different organization; forcing a refactor and a feature into the same template is procrustean. minimal seed also makes the unchanged-from-seed gate in persistSummary more sensitive. - selectMode.ts addendum: rewrite around three principles. (1) the snapshot is a FUNCTIONAL summary of what the PR does and the risks it carries, not a chronological review log — commit history can already be reconstructed from list_pull_request_reviews. (2) the orchestrator should USE the snapshot during triage and dispatch — concrete example given of carrying snapshot context into subagent lens prompts. (3) structure is the agent's call; stable headings make snapshots range-diff cleanly when they fit, but riff when they don't. - modes.ts IncrementalReview: drop the "Prior review feedback" checklist from the user-facing review body (step 6b gone, step 7 ELSE IFs cleaned up). It duplicated content that's already covered by the Reviewed-changes bullets and tracked durably in the snapshot for the next agent run; in the user-facing body it was noise. step 3 still fetches prior reviews but its role is now just filtering aggregation in step 5, not rendering. - AGENTS.md: codify "no follow-ups" rule. when an issue is identified during code review, fix it in this PR — PR scope does not constrain quality. follow-up TODOs are forbidden as a substitute for doing the work now. Empirical evidence supporting the re-orientation: - Run 25568912293 (PR#5 incr1, surrogate-pair fix): orchestrator's correctness lens dispatch said "Do NOT flag grapheme-cluster issues — the JSDoc scopes to code points." The grapheme-cluster framing was not in the diff; it was downstream of the snapshot's prior risk-section framing of truncate's contract. Snapshot influencing dispatch. - Run 25569054779 (PR#5 incr2, padStart added): orchestrator's correctness lens dispatch enumerated edge cases including "consistency with native String.prototype.padStart contract" and "fill = multi-code-point string (e.g. emoji)". Both threads carried over from the snapshot's prior truncate code-point-vs-code-unit discussion. Snapshot informing the shape of what was looked for. The cost of maintaining the snapshot (~800 tokens, ~$0.005/run) is trivially affordable when it materially improves orchestrator triage on the 1-5 lenses dispatched per review. |
||
|
Colin McDonnell
|
f87e0f878c |
action: minimize pullfrog.yml permissions and drop actions:read (#594)
* action: minimize pullfrog.yml permissions and drop actions:read
The recommended pullfrog.yml workflow asked for a permissions block that's
broader than what the action actually uses with the workflow GITHUB_TOKEN —
all real work (git push, PR comments, reviews) goes through installation
tokens that the action mints via OIDC. Customer security scanners flagged
the workflow-level block as too permissive.
- Move permissions to the job level and reduce to id-token: write,
pull-requests: write, issues: write. contents:read is the implicit default
and covers actions/checkout; contents:write, checks:read are unused by
any GITHUB_TOKEN consumer; actions:read was only used by post-cleanup's
listJobsForWorkflowRun call.
- Replace listJobsForWorkflowRun with a SIGTERM/SIGINT handler in main.ts
that calls core.saveState("cancelled", "true"); post-cleanup reads it
back via core.getState. Same cancel-vs-failure UX, no extra scope needed.
- Sync the docs (headless-action, getting-started, action/README) and the
two dogfood pullfrog.yml workflows to the new minimal block. Update the
post-cleanup wiki to describe the saveState approach.
* action: drop pull-requests/issues from required workflow scopes
Switch postCleanup.ts to mint its own short-lived installation token via OIDC
(acquireNewToken with issues:write + pull_requests:write) instead of using the
workflow GITHUB_TOKEN. Same comment-update behavior, but the workflow no longer
needs those scopes — the only permissions Pullfrog ever asks for are id-token:write
(OIDC exchange) and contents:read (actions/checkout).
Also fixes a bug from the previous commit: setting an explicit permissions block
drops every unlisted scope to none (with metadata as the only exception), so
omitting contents would have broken actions/checkout. Restored at both workflow
and job level.
* action: scope id-token:write to pullfrog job, not workflow level
id-token:write is the powerful one — it lets a job mint OIDC tokens that can
be exchanged for cloud credentials or our installation tokens. Keeping it at
workflow level means any future job added to this file silently inherits it.
Move it to the job level where it's actually used; leave only contents:read
at workflow level as a safe baseline for any future jobs.
* action: move stuck-comment cleanup server-side, drop write perms entirely
The action's post-cleanup step lived inside the runner and used the workflow
GITHUB_TOKEN to update the "Leaping into action…" progress comment when a run
failed/cancelled, requiring pull-requests:write + issues:write at the workflow
level. Move that responsibility to the workflow_run.completed webhook handler:
it already has installation-token access via the GitHub App, runs server-side
(no Pullfrog API dependency loop on failure), and lets us drop both write perms.
Recommended workflow permissions block is now truly minimal:
permissions:
contents: read
jobs:
pullfrog:
permissions:
id-token: write
contents: read
Server side
- handleWorkflowRunCompleted: when conclusion != "success" and the WorkflowRun
has progressCommentId, mint installation octokit and update the stuck comment
in place. Try issues.getComment first, fall back to pulls.getReviewComment on
404 (we don't store comment type — one wasted GET on the rarer review case).
- Reuses buildPullfrogFooter and updateProgressComment from pullfrog/internal,
matching the wording the action used to write client-side.
Client side
- Delete action/utils/postCleanup.ts and action/post.ts.
- Remove post: + post-if: from action/action.yml.
- Drop runPostCleanup wiring from action/commands/gha.ts and action/play.ts.
- Remove the SIGTERM/saveState handler I added in main.ts in the previous commit
(no longer needed; cancel/fail signal comes from the webhook hook payload).
Plumbing
- Extract isLeapingIntoActionCommentBody into action/utils/leapingComment.ts so
the predicate can be re-exported via pullfrog/internal without dragging the
MCP server's transitive type graph into the Next.js app's typecheck.
- mcp/comment.ts re-exports from the new location for backward compat.
Wiki
- Delete wiki/post-cleanup.md (obsolete; cleanup is now a one-liner branch in
the workflow_run webhook handler).
* chore: ignore .worktrees in biome config
Recently-added pnpm worktree feature creates nested git worktrees under
.worktrees/, each with their own biome.jsonc declaring root. Biome's
recursive scan trips on the nested config and fails pnpm lint. Excluding
the directory matches the existing .gitignore entry.
* fix: address PR #594 review findings
Two real bugs caught by code review:
1. handleWorkflowRunWebhook.ts:323 — drop the /m flag on the stuck-comment
detection regex. With /m, ^ matches any line start, so any finalized
progress comment that embeds a task list (report_progress writes
`- [x]`/`- [ ]` lines via todoTracking.ts) would be flagged as "stuck"
and silently overwritten with the "This run croaked" boilerplate
whenever the workflow concluded non-success after the agent's final
summary already landed. Restores the body-start anchoring the original
in-process postCleanup.ts:90 had.
2. action/scripts/check-entrypoint-imports.ts — drop ../post.ts from the
esbuild entry-point list (the file was deleted in aa43b9af). The
`pnpm check:entrypoints` step in test.yml would have failed on every
run with an unresolvable-entry-point error.
Plus three small follow-ups:
- main.ts:580 — comment said "post-cleanup has its own verify-retry loop"
but post-cleanup is gone. Updated to describe the new server-side path.
- mcp/comment.ts:443 — comment said "so post script doesn't think the run
failed". Updated to describe the actual current consumers of wasUpdated.
- commands/gha.ts:84 — `--post` help text said "run post-cleanup flow" but
with the post-cleanup path removed, --post is only valid alongside the
`token` subcommand for installation-token revocation. Updated wording.
* fix(action): scope --post help text to gha token subcommand
Root gha help text was documenting --post, but --post only makes sense
paired with the token subcommand (it's how the post step revokes the
installation token previously acquired in the main step). Move it to a
dedicated gha token help section and add a parser layer that rejects
--post on the bare gha command.
$ pullfrog gha --help
usage: pullfrog gha [subcommand]
...
options:
-h, --help show help
$ pullfrog gha token --help
usage: pullfrog gha token [--post]
...
options:
-h, --help show help
--post revoke the previously-acquired token (post-step usage only)
* webhook: artifact-aware cleanup of stranded leaping comments on success
Previously the workflow_run.completed cleanup only handled non-success
conclusions. Extend it to also catch the rare case where a successful
run leaves a "Leaping into action…" comment stuck (in-process cleanup at
action/main.ts:723 normally handles this, but can be skipped on SIGKILL,
runner host crash, or any exit path that bypasses main()'s finally block).
New behavior in cleanupStuckProgressComment:
- cancelled → update with "cancelled 🛑" body (unchanged)
- failure (other) → update with "croaked 😵" body (unchanged)
- success + artifact recorded → delete the comment (the artifact is the
user-facing surface; the leaping comment
is just stale UI noise at this point)
- success + no artifact recorded → delete the comment AND alert
team@pullfrog.com via emailAlert
The "success + no artifact" path is "should never happen" territory: the
run claims success but produced no review, PR, issue, plan, or summary
comment. The team alert helps us catch in-process cleanup regressions or
artifact-tracking gaps. hasRecordedArtifact reads {review,pr,issue,
planComment,summaryComment}NodeId off the WorkflowRun row to make the call.
* webhook: narrow stuck-comment detection to leaping prefix only
Drop the stranded-todo-pattern branch from cleanupStuckProgressComment.
The leaping prefix is highly specific and impossible to confuse with a
legitimate summary; a leading todo line is not — the agent's
error-reporting paths can produce useful explanatory comments whose
body leads with a checklist (e.g. "here's what I was working on" + the
incomplete todo list), and we don't want to silently overwrite those
with the generic "croaked" boilerplate.
In-process cleanup at action/main.ts:723 still handles the stranded-todo
case in the common path (gated on !finalSummaryWritten with full access
to the in-memory tool state). Missing the rare runner-died-mid-todo case
server-side is a worthwhile trade vs. the false-positive risk on real
explanatory comments.
|
||
|
pullfrog[bot]
|
4c1413d925 |
fix(action): flip wasUpdated on substantive MCP write tools (#563) (#577)
* fix(action): flip wasUpdated on substantive MCP write tools (#563) Review/Respond/etc. agents that submit a `create_pull_request_review`, `create_issue_comment`, or `update_pull_request_body` and exit without calling `report_progress` were being marked as workflow failures by the strict completion check in handleAgentResult. Extend the set of tools that flip toolState.wasUpdated so a substantive user-visible artifact satisfies the check. The isReviewMode bypass is retained for IncrementalReview's non-substantive path. Flag is set BEFORE patchWorkflowRunFields / deleteProgressComment in each tool so a best-effort cleanup failure does not undo the signal. * fix(action): use finalSummaryWritten for stranded progress cleanup The stranded-progress-comment cleanup at the end of main() previously fired only when toolState.wasUpdated was false (or the tracker was the last writer). With wasUpdated now set by additional MCP write tools (create_issue_comment, update_pull_request_body), an agent that produced a substantive artifact via one of those tools and skipped report_progress would leave the placeholder "Leaping into action" comment intact — the post-script then converted it into an error message on a successful run. Key the cleanup off finalSummaryWritten instead. That flag is only set when report_progress actually wrote the progress comment, so it cleanly distinguishes "comment is finalized" from "agent did other work but never touched the progress comment". * refactor(mcp): extract markSubstantiveArtifact() helper replaces 4 inline `ctx.toolState.wasUpdated = true` flips in CreateCommentTool, UpdatePullRequestBodyTool, and CreatePullRequestReviewTool with a single helper in mcp/server.ts. JSDoc on the helper documents the contract (call BEFORE downstream patch/cleanup; gates the strict completion check and stranded-comment cleanup) so future MCP write tool authors only need to grep for one symbol. no behavioral change. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): only flip finalSummaryWritten after non-skipped write Previously the flag was set unconditionally on any non-plan call, including paths where reportProgress skipped (silent events, deleted comment, no issue/PR target). The cleanup check in main.ts is safeguarded by toolState.progressComment so the bug doesn't manifest today, but aligning the flag with actual writes matches the wasUpdated pattern and the design intent in the cleanup plan. * refactor(mcp): inline markSubstantiveArtifact helper --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> Co-authored-by: David Blass <david@arktype.io> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Colin McDonnell
|
560e27bda5 |
refactor progress comments into a bundled type + helper module (#567)
* refactor progress comments into a single bundled type + helper module
introduce ProgressComment ({ id, type: "issue" | "review" }) as the canonical handle for
the GitHub comment a run uses to report progress, and route every read/update/delete/create
through a single helper module (action/utils/progressComment.ts). previously every site that
touched the progress comment hardcoded octokit.rest.issues.*Comment, which made adding a
second comment type (review-thread replies) require duplicating the same branch in 6+ places
— the same shape that bit pullfrog/app#445.
new capability: when the address-reviews trigger fires for a one-off review comment, the
"Leaping into action" comment is now posted as a reply in that review thread instead of as
a top-level PR timeline comment. the helper handles failure (e.g. parent comment deleted)
by silently falling back to a top-level issue comment, so the run never loses its progress
surface.
changes:
- action/utils/progressComment.ts (new) — ProgressComment type + getProgressComment,
updateProgressComment, deleteProgressCommentApi, createLeapingProgressComment. uses a
structural Octokit interface to bridge the @octokit/rest version mismatch between the
action package (v22) and the root project (v21).
- action/internal/index.ts — re-export the new types and helpers for cross-boundary use.
- action/external.ts, action/utils/payload.ts — replace progressCommentId: string with
progressComment: { id: string, type: "issue" | "review" } in WriteablePayload + JsonPayload.
wire-format breaking, no legacy fallback (in-flight runs across the deploy lose their
progress comment, fine).
- action/mcp/server.ts — ToolState.progressCommentId becomes
progressComment: ProgressComment | null | undefined (same tristate semantics).
- action/main.ts, action/mcp/comment.ts, action/utils/errorReport.ts,
action/utils/postCleanup.ts — every issues.*Comment call against the progress comment
routes through the helper module. zero hardcoded API branching outside the helper.
- utils/github/triggerWorkflow.ts — drop createLeapingComment + updateCommentToLeaping;
dispatchAndTrackWorkflow gains a resolution chain (existingComment → replyToReviewComment
→ triggeringIssue → none) and an existingComment: ProgressComment param plus
replyToReviewComment: { pullNumber, commentId }.
- utils/webhooks/handleWebhook.ts — dispatch closure threads replyToReviewComment through;
the one-off review comment branch passes it and skips the now-redundant eyes reaction
on the comment we're about to reply to.
- app/trigger/[owner]/[repo]/[number]/page.tsx, utils/github/runActionLocal.ts,
app/api/cli/dispatch/route.ts, app/api/dispatch-workflow/route.ts — call sites updated to
new shape.
no schema or DB column changes. the existing WorkflowRun.progressCommentId column is still
written by id only; type lives only on the in-flight payload, which is sufficient for
runtime since it's the only thing that needs to know which API to call.
* anneal pass 1: fallback visibility + stale doc/comment updates
- progressComment.ts: when reviewReply→issue fallback fires, prepend a [!NOTE] callout
with a permalink back to the original review comment. without this, the parent comment
showed no eyes reaction (deliberately skipped) and no reply, leaving the user with no
signal that anything happened.
- wiki/post-cleanup.md: update progressCommentId references to progressComment, document
the new helper-based dispatch by type.
- wiki/main.md: update initToolState({ progressCommentId }) → ({ progressComment })
in the resolver-chain diagram.
- action/main.ts, action/mcp/review.ts: update two stale comments that referenced the
old field name.
* anneal pass 2: post-cleanup detection through fallback notice + log cleanup
- isLeapingIntoActionCommentBody: strip a leading GFM blockquote/alert before
testing the leaping prefix. without this, the [!NOTE] callout that the
reviewReply→issue fallback prepends would prevent post-cleanup from
recognizing the stuck "Leaping into action..." comment, leaving it permanently
on the PR timeline if the workflow died before any progress update.
- progressComment helper: switch from log.warning (action-flavored, emits a
::warning:: GitHub Actions annotation) to console.warn so the helper doesn't
pollute Vercel logs when invoked from the webhook context.
- triggerWorkflow.ts: drop the duplicate caller-side log on review-reply
failure — the helper already speaks loudly. Reword the catch-branch log to
reflect that it now only fires when both the reply AND the helper's internal
fallback failed.
- progressComment.ts: document that the [!NOTE] fallback notice is overwritten
on the first report_progress call, and explain the trade-off vs persisting
it through the action payload + ToolState.
* debloat: drop the [!NOTE] fallback callout
Reverting two pieces from the prior anneal pass:
- progressComment.ts: drop the [!NOTE] callout that the reviewReply→issue fallback
prepended to the leaping body. It disappeared on the agent's first report_progress
call, which made it half-committed to visibility — worse than either properly
persisting it (real engineering) or leaving the fallback silent (current choice).
The console.warn diagnostic and the workflow-run footer link in the leaping
comment itself give us enough signal for the rare case where both API endpoints
fail at once.
- isLeapingIntoActionCommentBody: revert the leading-blockquote stripping; only
needed to compensate for the [!NOTE] callout.
Keeping: the console.warn-vs-log.warning fix (real cross-runtime concern), the
duplicate-log drop in triggerWorkflow.ts, the wiki updates, and the two stale
source-comment fixes.
* fix: prevent stranded task list overwriting post-cleanup message
When a run is cancelled, the action's todoTracker may have an HTTP write in
flight to GitHub when SIGTERM lands. The action process dies, but the request
data has already left the socket — GitHub processes it and updates the comment
body to the (stale) task list. Meanwhile post-cleanup, running in a separate
process, writes the "This run was cancelled 🛑" message. If the tracker's
in-flight write happens to land *after* post-cleanup's write, the user never
sees the cancellation message.
Two-layer fix:
- Action side: cancel the tracker in the SIGTERM signal handler so no new
debounced writes get scheduled. This shrinks the race window but can't
un-send a request already on the wire.
- Post-cleanup side: after writing, verify the body landed and re-issue if
another write clobbered ours. Loops up to 3× with a 3s settle delay so
delayed in-flight writes from the dying action have time to arrive before
our read-back check decides whether to retry.
* lint: import createLeapingProgressComment from pullfrog/internal in test script
* address bot review findings: reply-target root, version bump, GET error handling
Three real findings from the bot reviews on #567 plus a small DRY pass:
1. handleWebhook reply-target: `newComments[0]` may be a reply, not a
top-level review comment. `getReviewCommentsWithReplies` returns root +
replies for any thread the review touched, and `pull_request_review_id`
filtering only narrows by *which review submitted*, not *root vs reply*.
When a user submits a single reply as their entire review (e.g. replying
to someone else's comment to ping @pullfrog), the reply ID flowed through
to `createReplyForReviewComment`, which 422s on replies-to-replies and
degraded to a top-level issue comment — exactly the polluted-PR-timeline
behavior this PR was built to remove. Walk up `in_reply_to` from the
already-fetched thread data to find the root and reply there instead.
2. action/package.json: bumped 0.0.202 → 0.0.204. main is at 0.0.203 and
our wire format changed; without a bump validateCompatibility can't
surface the mismatch on the deploy boundary, and the merge would have
gone backwards.
3. postCleanup writeAndVerify: distinguish a thrown verify-GET from a
"body got overwritten" mismatch. Treating a transient 5xx/rate-limit GET
the same as a clobber wasted PUT attempts and printed a misleading
"in-flight writes kept clobbering us" warning. We trust our PUT (which
returned 200) and exit instead of amplifying writes against a flaky API.
4. Small DRY: extracted parseProgressComment for the
`{ id: string; type } -> ProgressComment` parse that had drifted across
server.ts and postCleanup.ts.
|
||
|
Colin McDonnell
|
55c95e6f50 |
Fix Node 24 action bootstrap fallback (#556)
* Fix Node 24 action bootstrap fallback Resolve the published CLI launcher through PATH so runners missing a sibling Node 24 npx can still start, and make post cleanup recognize prefixed leaping comments. * Bump Pullfrog action package version Ensure the Node 24 bootstrap and post-cleanup fixes publish to npm and move the v0 action tag. * Walk PATH for corepack and npx in action bootstrap ensureActionDependencies and runPackageCli now resolve corepack/npx through PATH the same way as the npx-via-PATH fix, so Node 24 runner pools missing either sibling can still bootstrap. Also adds a Zod-mirror settings helper for the preview-556 repo and documents the per-PR settings workflow. * log when corepack PATH fallback is used |
||
|
Colin McDonnell
|
18c8d34da6 |
remove task list from review bodies; keep in progress comments only (#542)
review bodies were embedding a task-list snapshot that could capture stale in-progress state due to timing between the agent's final TodoWrite and the review submission API call. progress comments are the authoritative checklist surface — remove the review-body embedding entirely so there is a single source of truth. also adds a `completeInProgress` option to `renderCollapsible` so the progress-comment path can finalize any in-progress items at render time without mutating tracker state. Made-with: Cursor |
||
|
David Blass
|
a4c7c0fc15 |
feat: workflow run artifact chips + GraphQL url resolution (#447) (#527)
* plan: issue 447 run artifact tracking and UI (supersedes stale pill notes) Made-with: Cursor * feat: workflow run artifact urls, chips, and safe PATCH validation Made-with: Cursor * chore(action): refresh latest-by-provider model snapshot Made-with: Cursor * refactor: resolve artifact urls via GraphQL nodes(ids), drop stored url columns Made-with: Cursor * docs: finalize issue 447 run-artifacts plan; remove demo backfill script Made-with: Cursor * refactor: DRY node-id constraint, replace margin with padding wrapper Made-with: Cursor * refactor: DRY audit — shared row info, derived types, unified Prisma select - extract WorkflowRunRowInfo component (description + issue link + time + pills) shared by ActiveWorkflowRunsSection and WorkflowRunHistory - derive API payload types via Omit + & instead of manual field lists; serialize with spread + override for bigint/date fields - extract workflowRunListSelect shared Prisma select base; history extends with completedAt - inline updateCommentNodeId → direct patchWorkflowRunFields calls - derive WorkflowRunArtifactSlice from canonical exported types - delete cancelling-out URL column migrations (no schema change vs main) Made-with: Cursor * refactor: artifact chips as inline CTAs with proper vertical alignment - chips now render as action links: "Open PR #N", "View summary", etc. - only render chips with resolved URLs; remove inert span fallback - inline chips in the row (right-justified) instead of a separate line - fix vertical alignment: remove ul/li wrappers that caused line-height mismatch, render chips as direct row siblings via flat flex layout - change row to items-center, remove compensating self-start/pt nudges - cancelled run X icon uses red-600 Made-with: Cursor * chore(action): refresh latest-by-provider model snapshot Made-with: Cursor --------- Co-authored-by: Colin McDonnell <colinmcd94@gmail.com> |
||
|
Colin McDonnell
|
b282e8b599 |
Improve incremental review output and fix todo tracker race (#529)
* improve incremental review output and fix todo tracker race - reviewed changes section: summarize at logical-change level with past-tense verbs, not per-file enumerations - add TodoTracker.completeAll() to mark all non-cancelled items as completed before snapshotting the collapsible in review/progress posts Made-with: Cursor * completeAll -> completeInProgress: only mark in-progress items as completed Pending items that were genuinely skipped stay as-is in the collapsible, so the task list honestly reflects what the agent actually did. Made-with: Cursor |
||
|
Colin McDonnell
|
b3112e4a15 |
Fix typos in AGENTS.md (#525)
* fix WorkflowRun mis-assignment when multiple dispatches are in flight workflow_run_requested fires before GitHub applies the custom run-name, so display_title has no [suffix]. the old desc ordering picked the newest pending record, cross-linking enrichment ↔ auto-label records. switch to FIFO (asc) ordering so records are claimed in dispatch order, and add a 15s createdAt window to avoid claiming stale records. fixes #523 Made-with: Cursor * WIP * plan: update issue indexing resolution to R2-backed lazy filesystem replace the direct GitHub tarball + in-memory extraction approach with a two-phase architecture: streaming tarball sync to R2 (per-file, via tar-stream) and on-demand lazy loading via just-bash InMemoryFs backed by R2 GETs. scales to 200K+ file monorepos at <50MB memory overhead. Made-with: Cursor * plan: switch to tarball + R2 range requests, add design alternatives rule update issue indexing plan to use a single uncompressed tar in R2 with byte-offset index instead of per-file uploads. 2 PUTs per sync vs 10K, 5000x cheaper, trivial lifecycle. add AGENTS.md rule: generate 3 alternatives before committing to a design. Made-with: Cursor * fix typos in AGENTS.md --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Colin McDonnell
|
2ea447a780 |
refactor: replace narrow parameter types with context objects (#519)
* refactor: replace narrow parameter types with context objects across action/ pass broader context objects (ToolContext, PromptContext, PostCleanupContext) to utility functions instead of cherry-picking fields into single-use interfaces. deletes 8 narrow types, simplifies call sites, and makes buildCommentFooter synchronous by reading ctx.runId/ctx.jobId directly instead of re-deriving from env vars and making an extra API call. Made-with: Cursor * fix: replace non-null assertion with local guard in validatePushDestination addresses review feedback — the function now validates pushUrl itself instead of relying on the caller's check, eliminating the ! assertion. Made-with: Cursor * revert: remove GH_TOKEN injection from restricted shell the original change exposed the git token in restricted-mode shell so `gh` CLI would work. this is a security regression for public repos: MCP tools are deliberately constrained (no merge, no release, no arbitrary API calls), but `gh api` with the token gives full GitHub API access to any prompt-injected agent. Made-with: Cursor |
||
|
Colin McDonnell
|
6b93e6b368 |
review/incremental-review: always submit review, never call report_progress (#516)
* WIP * WIP * review/incremental-review: always submit review, never call report_progress The progress comment is auto-deleted by the stranded-comment cleanup in main.ts when the agent skips report_progress. This makes reviews the sole PR artifact for both modes, reducing noise. - soften report_progress tool description to allow mode opt-out - Review mode: always submit exactly one review (approve or request changes) - IncrementalReview mode: submit review for substantive outcomes, silently exit for non-substantive changes (formatting-only pushes produce zero artifacts) Made-with: Cursor * incremental-review: clarify approval condition for substantive no-issues case Made-with: Cursor * report_progress: s/completed/current task list Made-with: Cursor * system instructions: align report_progress guidance with mode opt-out Made-with: Cursor |
||
|
David Blass
|
6b18b6730b |
live todo tracking, collapsible task list in final progress, hide set_output outside standalone (#492)
* fix false "without reporting progress" error + live todo tracking clean up orphaned progress comments when review is skipped or only set_output is used, preventing the false positive in handleAgentResult. parse todowrite events from OpenCode's NDJSON stream and render a live markdown checklist in the PR progress comment (2s debounce). agent's explicit report_progress always takes priority. Made-with: Cursor * fix contradictory review/progress prompting align Review and IncrementalReview mode prompts with their guidance — mode prompts said "always submit" while guidance said "skip if clean." remove the empty-approval submission that was silently dropped by the tool. make progress comment lifecycle explicit: created on first call, updated in place, removed after review submission. Made-with: Cursor * centralize todo tracking into shared TodoTracker module extract inline todo tracking logic (~95 lines) from opentoad.ts into action/utils/todoTracking.ts. the tracker is created once in main.ts and passed to agents via AgentRunContext.todoTracker, making it agent-agnostic and reusable for future agent implementations. Made-with: Cursor * fix todoTracker optional type to match file convention add | undefined to todoTracker in AgentRunContext, matching every other optional property in the same interface. Made-with: Cursor * instruct agents to always maintain a task list for live progress system prompt now tells agents to create an internal task list at the start of every run. the tracker renders it to the progress comment automatically. report_progress is reserved for final results only — no more intermediate "Checking..." messages that cancel the tracker and leave stale text on the comment. Made-with: Cursor * require report_progress summary at end of every run agents must always call report_progress with a final summary — the completed task list should never be the end state of the progress comment. updated all review mode prompts to call report_progress after submitting (or not submitting) a review. Made-with: Cursor * keep progress comment after review with final summary stop deleting the progress comment after review submission — the agent now always calls report_progress with a summary at the end, and that summary should persist as a record of what was done. Made-with: Cursor * harden stranded progress comment cleanup - main.ts: detect when tracker was last writer (agent never called report_progress) and delete the stranded checklist instead of leaving it as the final comment state - postCleanup.ts: expand stuck-comment detection to also catch stranded todo checklists (regex match for checklist patterns) when the process is killed before normal cleanup runs - modes.ts + selectMode.ts: add report_progress step to Summarize and SummaryUpdate modes (only modes that were missing it) Made-with: Cursor * fix stale comments, typo, and build mode redundancy - comment.ts: update deleteProgressComment docstring and inline comment to reflect current usage (stranded-comment cleanup, not post-review) - modes.ts: merge duplicate report_progress steps (8 + 10) into single step 9, fix "optimizatfixons" typo - wiki/post-cleanup.md: document checklist detection regex Made-with: Cursor * collapsible completed todos in final progress, hide set_output outside standalone mode - add renderCollapsible() to TodoTracker, append completed task list as <details> section when agent calls report_progress - cancel tracker after agent's final report_progress so it doesn't overwrite with raw checklist - conditionally register SetOutputTool only in standalone mode or when output_schema is provided - remove unconditional set_output instruction from orchestrator task section - update Summarize/SummaryUpdate mode guidance to not reference set_output Made-with: Cursor * show completion count in collapsible task list summary Made-with: Cursor * only count completed (not cancelled) in collapsible task list summary Made-with: Cursor * reinforce concise summary prompting across system prompt, modes, and tool description Made-with: Cursor * address review feedback: wasUpdated bypass, tracker false-positive, race condition - remove wasUpdated=true from cleanup paths so handleAgentResult correctly detects genuinely silent runs - add hadProgressComment to ToolState as immutable snapshot for the safety check - use todoTracker.hasPublished instead of enabled for stranded-comment cleanup - serialize onUpdate calls via inflightPromise chain with post-cancel guard - add settled() to wait for in-flight updates before writing final summary Made-with: Cursor * address round-2 review: hasPublished after success, finalSummaryWritten flag - set hasPublished only after onUpdate resolves (not before) so failed writes are not counted as published - add finalSummaryWritten flag to ToolState, set after successful non-plan reportProgress; decouple cleanup detection from todoTracker.enabled so it survives API failures where cancel() ran but the write didn't succeed Made-with: Cursor |
||
|
Colin McDonnell
|
c6a3ee0e9a |
show model name in footer, drop pullfrog.com link (#484)
* show model name in footer, drop pullfrog.com link add model slug to buildPullfrogFooter so every Pullfrog comment displays the active model (e.g. "Using `Big Pickle` (free)" or "Using `Claude Opus`"). remove the pullfrog.com link from all footers. Made-with: Cursor * reject <br/> tags in comment bodies, add prompt guidance add runtime validation in addFooter that throws if <br/> is followed by a non-blank line (breaks GitHub heading rendering). the agent sees the error and retries with clean markdown. also update Summarize mode prompt to explicitly forbid <br/> tags. Made-with: Cursor * fix <br/> guidance: move to event instructions, clarify blank line rule the formatting rule belongs in DEFAULT_PR_SUMMARY_INSTRUCTIONS (event instructions), not the Summarize mode prompt. clarify that <br/> must always be followed by a blank line before headings. Made-with: Cursor * generalize block-level HTML rule in summary instructions add a prominent top-level rule about requiring blank lines between ALL block-level HTML elements and markdown syntax, not just <br/>. Made-with: Cursor * move model to toolState instead of threading through params model is set once at startup and read everywhere — it belongs on toolState, not threaded as a separate param through 8 call sites. postCleanup runs without toolState so it just omits the model label. Made-with: Cursor * update models.dev snapshot (openai latest changed) Made-with: Cursor * add comment to models snapshot test explaining its purpose Made-with: Cursor |
||
|
Colin McDonnell
|
8a734c32f4 |
fix workflow detection, duplicate summaries, review resilience (#482)
* fix workflow detection when repos have many workflows Switch workflow lookup to GitHub's direct workflow-by-filename API so pullfrog.yml is found even when list endpoints paginate, and paginate installation scans in maintenance scripts to avoid partial coverage. Made-with: Cursor * fix review comment line resolution: pre-validate against diff hunks + auto-bisect fallback when submitting a review with inline comments, the tool now: 1. fetches the PR diff and validates each comment's line range against the actual hunk boundaries 2. moves invalid comments to the review body with a clear explanation 3. on 422 from GitHub (rare API quirks where valid-looking lines are rejected), bisects comments using disposable pending reviews to isolate failures 4. retries with only the comments GitHub accepts also fixes getHttpStatus (previously isStatusError) which wasn't recognizing Octokit errors, and warns in the tool description that each call creates a permanent visible review. Made-with: Cursor * remove bisect fallback, anchor review to checkout sha, make start_line optional - drop the auto-bisect-on-422 logic entirely; pre-validation catches the real issues and the 422 catch now just throws a clear actionable error - anchor review submission to checkoutSha so line numbers match the diff the agent actually analyzed (avoids stale-line 422s from new pushes) - make start_line optional and only set start_line/start_side when it differs from line (single-line comments don't need the range fields) - improve headMovedDuringReview detection to use latestHeadSha directly Made-with: Cursor * drop review comment pre-validation in favor of pinned commit_id The pre-validation (listFiles + hunk parsing) was checking comments against the current PR diff, but the review is now anchored to checkoutSha. When HEAD moves, pre-validation checks the wrong diff and can false-reject valid comments. GitHub's own commit_id-anchored validation is the correct source of truth. Made-with: Cursor * add logging to fetchExistingSummaryComment for duplicate summary debug Made-with: Cursor * fix duplicate summary comments: guard create_issue_comment for existing summaries When select_mode finds an existing summary comment (existingSummaryCommentId), create_issue_comment with type: "Summary" now auto-redirects to update instead of creating a new comment. Belt-and-suspenders for the token fix in selectMode.ts. Made-with: Cursor * document api auth patterns to prevent token misuse add wiki/api-auth.md explaining the two auth patterns (GitHub token vs Pullfrog JWT) and when to use each. add auth comments to all action-facing routes and their callers so the correct token is obvious. Made-with: Cursor * fix models.dev snapshot: filter beta models, add tiebreaker Skip models with any status (beta, deprecated) so nightly/experimental releases don't cause snapshot churn. Add lexicographic tiebreaker for stable ordering when release dates match. Made-with: Cursor * add tests to pre-push hook Made-with: Cursor * fix: decouple summary dispatch from re-review gate on pull_request_synchronize The summary workflow was never dispatched on new commits because the pull_request_synchronize handler broke early when prReReview was disabled, before reaching the prSummaryComment check. Now re-review and summary are dispatched independently. Made-with: Cursor * resolve merge conflicts in rebase.md and checkout.ts Made-with: Cursor * fix: restore checkout.ts and rebase.md from remote Made-with: Cursor |
||
|
Colin McDonnell
|
4a8c432a48 |
add Summarize mode for updatable PR summary comments (#470)
* add Summarize mode for updatable PR summary comments Introduces a Summarize mode that manages a single summary comment per PR, updated in place on subsequent pushes. Mirrors the Plan/PlanEdit pattern: API endpoint for existing-comment lookup at select_mode time, node ID tracking on WorkflowRun, and SummaryUpdate guidance for edits. Also fixes summary format instructions: Before/After uses inline <br/> to avoid double line breaks, metadata line placed after key changes, SHA-256 anchor instructions strengthened against fabrication. Made-with: Cursor * fix pre-existing lint error in checkout.ts Made-with: Cursor * fix dead restricted param in deepenForBeforeSha GitAuthOptions dropped the restricted field in the ASKPASS refactor (#478) but deepenForBeforeSha (#471) still passed it. Remove the field and the now-unused shell param from DeepenForBeforeShaParams. Made-with: Cursor |
||
|
Colin McDonnell
|
6d25adfd1a |
Agent & model refactor (#478)
* agent & model refactor with ASKPASS git auth, UI restructure, clerk v7 Made-with: Cursor * fix stale agent/effort refs, add tests for askpass + model resolution - reviewCleanup.ts: payload.agent -> payload.model, remove effort - selectMode.ts PlanEdit: remove delegation/subagent/effort references - pullfrog.yml.ts: update env vars (drop GOOGLE_API_KEY/CURSOR_API_KEY, add GOOGLE_GENERATIVE_AI_API_KEY/XAI_API_KEY/MOONSHOT_API_KEY/OPENCODE_API_KEY) - FlagsSettings/RepoInstructionsSection: remove stale effort/timeout copy - new: gitAuthServer.test.ts (10 tests — lifecycle, token delivery, tamper detection, script gen) - new: agent.test.ts (4 tests — default opentoad, AGENT_OVERRIDE, invalid override) - new: models.test.ts (19 tests — parseModel, resolution, registry invariants) - update models.dev snapshot Made-with: Cursor * fix changed-agents.sh to filter legacy agent files from CI matrix legacy agent files (claude.ts, codex.ts, etc.) are @ts-nocheck and not exported from index.ts. changed-agents.sh now reads index.ts imports to build the active agent set and treats changes to inactive files as non-agent changes (opentoad canary only). Made-with: Cursor * remove MCP file tools, old agent harnesses, and obsolete security tests ASKPASS-based git auth makes the old MCP file tool security layer unnecessary: - token never in subprocess env, so symlink/gitattributes/hook attacks can't exfiltrate it - agents now use native file tools (OpenCode builtin read/edit) deleted: - action/mcp/file.ts (file_read, file_write, file_edit, file_delete, list_directory) - action/mcp/index.ts (dead re-export) - agent harnesses: claude.ts, codex.ts, cursor.ts, gemini.ts, opencode.ts - opencode-runner.ts (inlined into opentoad.ts) - security tests that validated MCP file tool restrictions - commented-out three-step review flow (~300 lines) - sanitizeSchema/wrapSchema dead code from mcp/shared.ts - OPENCODE_MODEL_MINI/MAX env vars (effort-level model overrides removed) updated test prompts to use generic file ops instead of MCP tool names. restored pkg-json-scripts + requirements-txt-attack (test --ignore-scripts defense). Made-with: Cursor * bump actions/checkout v4 → v6 (node 24) node 20 actions deprecated june 2, 2026. Made-with: Cursor * temporarily disable fail-fast on agnostic tests to debug checkout@v6 Made-with: Cursor * re-enable fail-fast on agnostic tests Made-with: Cursor * fix test token mismatch: mint OIDC tokens scoped to target repo CI tests override GITHUB_REPOSITORY to pullfrog/test-repo but inherit the runner's GITHUB_TOKEN (scoped to pullfrog/app), causing 401s on every run-context fetch. Clear GITHUB_TOKEN in the test subprocess so ensureGitHubToken() mints a properly scoped token via OIDC. Also centralizes the default GITHUB_REPOSITORY in runAgentStreaming instead of repeating it in every test file, and fixes preview-cleanup to remove workers from all queues (not just name-matching ones). Made-with: Cursor * fix ensureGitHubToken to try OIDC when app credentials are absent ensureGitHubToken only attempted token minting when GITHUB_APP_ID and GITHUB_PRIVATE_KEY were set. In CI, OIDC is available but app creds aren't exposed — so the guard prevented minting entirely. Made-with: Cursor * dead code cleanup: remove remnants of deleted agents, file tools, effort system remove unused @anthropic-ai/claude-agent-sdk and @openai/codex-sdk deps, orphaned file-tool security tests, dead GEMINI_MODEL passthrough, stale opencode-runner wiki refs, deleted test file references, and MCP file tool docs. rename docs/effort → docs/models. fix vitest setup: move dotenv to globalSetup (runs once before forks instead of per-file, 19s → 200ms). Made-with: Cursor * address review feedback: remove dead code, update stale references - remove AGENT_OVERRIDE (only opentoad exists) - remove shellToolName plumbing (always restricted shell) - bump action version to 0.0.179 - remove CURSOR_API_KEY from all workflows/configs - remove OPENCODE_MODEL_MINI/MAX from workflows/docs - delete wiki/effort.md, rewrite docs/effort.mdx as "Models" - rewrite wiki/modes.md: orchestrator/subagent → single agent - simplify flag system: drop builtin flag extraction (debug, effort, timeout, agent), keep custom flag replacement only - reserve all legacy flag names to prevent custom flag conflicts Made-with: Cursor * regenerate lockfile after removing claude-agent-sdk and codex-sdk Made-with: Cursor * fix import ordering, add lockfile check to pre-push hook Made-with: Cursor * remove dead debug payload field, stale packageExtensions Made-with: Cursor * merge proc-sandbox and token-exfil into a single test proc-sandbox and token-exfil were duplicative — both tested that SANDBOX_TEST_TOKEN couldn't be exfiltrated. consolidated into token-exfil with shell:restricted (which actually exercises filterEnv) and the /proc attack vector hints from proc-sandbox. Made-with: Cursor * fix wiki adversarial.md to match actual tokenExfil validator Made-with: Cursor |
||
|
Anna Bocharova
|
9c99bcbbac |
feat: Improving the plan revisions (#465)
* feat(plans): Suggested plan for plan revisions. * fix: add planCommentId to reduce GitHub API calls. * Revert "fix: add planCommentId to reduce GitHub API calls." This reverts commit ef9c24811fa291b12ac3601cc4cd3edb7c9a0fca. * Improving plan revision: the implementation draft. * fix schema composition order. * fix: reusing existing retry helper (action) for reportPlanCommentToRun. * mv: updatePlanCommentId. * fix: higher severity for logging error. * fix: add error handling when calling findExistingPlanCommentIdForIssue. * feat: improving the revisit plan request detection by adding PLAN_REVISION_VERBS. * Updating the plan with alternative non-determenistic solution. * add more verbs to PLAN_REVISION_VERBS. * fix: supply the previous plan in the event context as previousPlanBody, updating Plan mode instructions. * fix: adjusting the way PLAN_REVISION_VERBS are used in sentences. * fix: using GraphQL approach with NodeId to find commentId in findExistingPlanCommentIdForIssue. * fix: use double word boundaries (both sides). * fix condition in findExistingPlanCommentIdForIssue. * fix: rm unused args from findExistingPlanCommentIdForIssue. * bump the action version. * fix(plan): rm everything related to approach A. * fix(plan): No limit for progress comments. * feat(plan): the new plan. * Revert: changed to webhook (no longer involved). * feat: mv plan comment lookup into a new API endpoint. * Revert: changes to select_mode tool. * FEAT: The new implementation. * fix arktype issues. * fix plan diagram. * fix(selectMode): e2e type constraints for fetchExistingPlanComment. * Revert "fix(selectMode): e2e type constraints for fetchExistingPlanComment." This reverts commit 53f3b6650a9928e3080700faa9eead0052e94333. * fix(selectMode): type constraints (copy) for fetchExistingPlanComment. * feat: improving isHttpError helper and reusing it consistently instead of casting. * address review: remove unconditional retry, add plan comment warning, dedupe type, remove dead guard, fix GraphQL types * Fix: tightening the PlanEdit guidance. * fix(select_mode): Providing the agent with existingPlanCommentId as well. * fix(instructions): Adjusting the primary guidance to prefer Plan mode for issue-related ambiguous requests. * fix(wiki): updating the delegation docs according to the current instructions. * fix(instructions): rm implication to call for issue details. * fix(select_mode): tweak for PlanEdit. * fix(select_mode): more tweaks to PlanEdit. * fix(select_mode): tweaks for the order of instructions and context. * revert: to the state of 5c400efce1f1fec0a0855eeacad2bc3b721fd1bf. * fix(select_mode): Correcting the guideline. * rm the plan from the branch (impletemented). --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Mateusz Burzyński
|
a0af59b52a | Fixed a wrong issue number being used for "Implement Plan" links at times (#404) | ||
|
pullfrog[bot]
|
20b08b5321 |
Add "Rerun failed job ➔" link to error comment footer (#355)
* add "Rerun failed job" link to error comment footer * Remove issueNumber guard from rerun link The rerun action only needs run_id — the issue number in the trigger URL path is just a route segment requirement. Use 0 as a fallback so the link is always shown when a run ID is available. * Overload `[number]` path segment as `runId` for the rerun action For the rerun trigger, the `[number]` path param now carries the workflow run ID instead of an issue number. The rerun link changes from `/trigger/o/r/ISSUE?action=rerun&run_id=RID` to `/trigger/o/r/RID?action=rerun`. - Remove `run_id` query param from page.tsx and searchParams type - Add error handling around `reRunWorkflow` for invalid run IDs - Drop `issueNumber` from `BuildErrorCommentBodyParams` and all rerun link builders (errorReport, exitHandler, postCleanup) * Reorder validation: action first, then rerun, then issueNumber * address review: remove early toolState assignment, restructure rerun into dedicated block * revert self-contained rerun block, share auth logic via issueOrRunId * improve error handling * normalize runid early --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> |
||
|
David Blass
|
cfd38d82fc |
refactor delegation system, add PR summary comments, and improve code quality (#334)
* refactor delegation system and add PR summary comments Delegation system: - replace mode-based delegation with select_mode → delegate two-step flow - orchestrator crafts self-contained subagent prompts (clean context — no system/repo/event instructions leak) - add role-based tool filtering via FastMCP authenticate hook (?role=subagent hides orchestrator-only tools) - add select_mode tool for orchestrator guidance per mode - add ask_question tool for lightweight research subagents - extract shared subagent lifecycle into subagent.ts (create, complete, stdout, instructions) - route set_output to per-subagent state when activeSubagentId is set - track per-subagent state (SubagentState Map) replacing boolean delegationActive flag - capture and aggregate AgentUsage across all agents (claude, codex, gemini, opencode) - write usage summary table to GitHub job summary - block built-in subagent spawning (Task for Claude, Task(*) for Cursor) - increase activity timeout from 60s to 300s (subagent thinking phases) - fix gh CLI misguidance in system prompt — explicitly forbid usage PR summary comments: - add prSummaryComment trigger (DB schema + migrations + Zod + UI toggle) - dispatch mini-effort summary job alongside PR review on pr.created - add update_pull_request_body MCP tool - add defaultEffort option to webhook dispatch Hardening: - rewrite delegate/selectMode tests with simulated state management - add toolFiltering.test.ts for role extraction, canAccess, set_output routing - remove non-null assertions for PULLFROG_TEMP_DIR (proper error throws) - use fetchWithRetry for direct tarball downloads - DRY fix for rate limit check in test runner Co-authored-by: Cursor <cursoragent@cursor.com> * fix: add type keyword to Effort import in handleWebhook.ts Co-authored-by: Cursor <cursoragent@cursor.com> * clean up delegation system, improve code quality across the codebase - simplify delegate tool to instructions + effort params with subagent lifecycle in subagent.ts - add select_mode and ask_question orchestrator-only tools with canAccess filtering - replace delegate.test.ts/selectMode.test.ts with toolFiltering.test.ts (live MCP integration) - add set_output routing for subagent context and AgentUsage tracking across all agents - add PR summary comment trigger (schema, UI, webhook dispatch with silent flag) - add update_pull_request_body MCP tool - fix changed-agents.sh to always include claude canary for non-agent action changes - fix cursor pagination bug in getSelectedInstallationReposPage - remove destructuring patterns, inline type definitions, and unsafe type casts - replace non-null assertions with explicit checks in install.ts - convert multi-param functions to single param objects (postCleanup, runActionLocal, etc.) - use isHttpError helper in API routes instead of catch-any patterns - add adhoc test fixtures for delegation scenarios (context isolation, error handling, synthesis, etc.) Co-authored-by: Cursor <cursoragent@cursor.com> * no subagent mutation, one mcp per subagent * address review feedback: parallel-safe usage tracking, subagent isolation, minor improvements * fix subagent state isolation: replace Object.freeze with shallow copy Object.freeze throws TypeErrors when subagent tools (checkout_pr, report_progress) write scalar properties to toolState. A shallow copy achieves the same isolation for scalar fields while allowing tools to work normally. Shared references (subagents Map, usageEntries array) remain shared for coordination. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Colin McDonnell
|
97937f46f7 |
console UI improvements and cleanup (#311)
* console UI improvements and cleanup - add verify workflow button and API endpoint for manual installation check - move env var check into PromptBox as blocking overlay (hoisted to RepoConsole) - extract FlagsCheatSheet modal, replace verbose flag hints everywhere - add info popovers for repo setup / post-checkout script descriptions - remove unused prAutoFixCiFailures schema fields and migration - default mentionAllowNonCollaborator to disabled for safety on public repos - update docs for triggers and getting started Co-authored-by: Cursor <cursoragent@cursor.com> * add diagnostic logging for push_branch bug investigation temporary [push-debug] logs to trace why getPushDestination falls back to origin/<localBranch> instead of using the correct remote branch name for same-repo PRs. Co-authored-by: Cursor <cursoragent@cursor.com> * add git config diagnostic to verify original bug cause Co-authored-by: Cursor <cursoragent@cursor.com> * temporarily disable StoredPushDest to test git config path Co-authored-by: Cursor <cursoragent@cursor.com> * remove diagnostic logging for push_branch investigation verified that StoredPushDest fix works correctly on preview repo. both the stored dest path and the git config fallback resolve to the correct remote branch in the GitHub Actions environment. Co-authored-by: Cursor <cursoragent@cursor.com> * fix formatting in AgentSettings and TriggersSettings Co-authored-by: Cursor <cursoragent@cursor.com> * pass derived env var state to PromptBox instead of raw secrets data eliminates duplicated derivation logic between RepoConsole and PromptBox by passing envVarMissing, envVarChecking, and agentKeyNames as props. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: prevent duplicate comment after PR review deletes progress comment progressCommentId now uses three states: undefined (no comment yet), number (active), null (deliberately deleted). After create_pull_request_review deletes the progress comment, subsequent report_progress calls skip instead of creating a new comment. Co-authored-by: Cursor <cursoragent@cursor.com> * effort descriptions, test ordering, husky, docs images, typo fix - rewrote delegation effort level descriptions to per-level breakdown - action-agents now waits for action-agnostic; action-agnostic waits for root - added husky + lint-staged (biome check --write on staged files) - updated triggers docs images and triggers.mdx content - fixed "figured" → "figures" typo on landing page - updated pnpm-lock.yaml Co-authored-by: Cursor <cursoragent@cursor.com> * Commit --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
pullfrog[bot]
|
d7759734f2 |
Clarify issue comment semantics and strengthen report_progress guidance (#292)
- Add `comment_type: "issue"` to `IssueCommentCreatedEvent` interface and dispatch sites so agents can distinguish issue comments from PR review comments - Add dedicated "Progress reporting" section to system prompt making `report_progress` the mandatory tool for sharing results - Update `reply_to_review_comment` description to clarify it only works for inline review comments on PR diffs, not issue comments Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Colin McDonnell
|
78cf05f111 | Clean up url resolution | ||
|
Colin McDonnell
|
3a7145db1a |
Scope installation token permissions in restricted mode (#226)
* Scope installation token permissions in restricted mode
In restricted/disabled bash mode, the installation token is now scoped
to match the workflow's permissions block. This preserves fork push
capability while limiting what the agent can do with the token.
- Read workflow permissions from pullfrog.yml at runtime
- Pass permissions to API when acquiring installation token
- Clear OIDC env vars in restricted mode to prevent token minting
- Simplify setupGit by moving token resolution to main.ts
* Address review feedback: fail closed with default permissions
- Add restrictive default permissions (contents:read, pull_requests:read,
issues:read) as fallback when workflow permissions can't be read
- Add support for job-level permissions via GITHUB_JOB env var
- Fix misleading comment about token resolution in restricted mode
- Add documentation about fork PR checkout behavior
* Simplify to separate git/MCP tokens without workflow permission scoping
- gitToken: minimal contents:write only (assumed exfiltratable)
- mcpToken: full installation token (not exfiltratable via MCP tools)
- Remove workflowPermissions.ts - security-conscious users can pass
their own token via GH_TOKEN or inputs.token
- Add type-safe InstallationTokenPermissions to github.ts and API route
* Rename `write` permission to `push` and remove vestigial tool blocking
The `write` permission was previously used to block local file write tools
in agents. This was security theater since bash can write files anyway.
Now `push` only controls the git token scope:
- push: enabled → contents:write (can push commits)
- push: disabled → contents:read (read-only, can't push)
Changes:
- Rename `write` to `push` in action.yml, Prisma schema, and all TS types
- Remove vestigial write tool blocking from all agents (claude, cursor,
gemini, opencode, codex)
- Add data-preserving Prisma migration using RENAME COLUMN
- Update UI: "Write files" → "Git push" with updated description
* add PID namespace isolation for bash sandbox
when running in CI, attempts to use unshare --pid to create a new PID
namespace for bash subprocesses. this prevents the /proc/$PPID/environ
attack where a malicious command could read secrets from the parent
process's environment.
the protection works by:
1. creating a new PID namespace (subprocess becomes PID 1)
2. mounting fresh /proc showing only sandbox PIDs
3. parent PIDs become invisible (PPID = 0, /proc/0 doesn't exist)
combined with filterEnv(), this provides complete protection against
/proc-based secret theft. falls back gracefully if namespaces aren't
available.
includes test script to verify the protection works.
* add PID namespace test to CI workflow
tests whether unshare --pid works on GHA runners out of the box,
and if not, whether enabling via sysctl helps. also runs the
pidNamespace.ts test to verify the full protection.
* fix pnpm setup and add procIsolation agent test
- fix pnpm/action-setup by specifying package_json_file path
- add procIsolation crossagent test that has agent attempt to
read secrets via /proc/$PPID/environ
- add procIsolation to CI test matrix
* add pid-namespace test job to main workflow
this job tests unshare --pid capabilities on GHA runners and runs
the pidNamespace.ts adhoc test to verify /proc isolation works
* test bubblewrap's sysctl approach for enabling namespaces
- write to /etc/sysctl.d/99-userns.conf and run sysctl --system
- try aa-complain on unshare binary
- more detailed diagnostics
* fix pidNamespace test and add sudo-unshare fallback for GHA
- fix reference error in pidNamespace.ts (renamed function but didn't update calls)
- add sudo-unshare as fallback method for GHA runners where unprivileged
namespaces are blocked but sudo is available
- update bash.ts to detect and use sudo unshare when unprivileged fails
* consolidate security docs and document PID namespace isolation
- update security.md with current implementation details
- document sudo unshare fallback for GHA runners
- add testing instructions for local Docker and CI
- add "Further Exploration" section with Landlock and path validation ideas
- delete bash-sandbox.md and landlock.md (consolidated into security.md)
* move procIsolation test to adhoc folder
the procIsolation test requires PID namespace capabilities that aren't
available in the Docker test environment. moved to adhoc/ so it's excluded
from default test runs and can be run explicitly when needed (e.g. via
the pid-namespace CI job or locally with --privileged docker).
* fix Docker test environment for PID namespace isolation
- add CI and GITHUB_ACTIONS to testEnvAllowList so sandbox detection runs
- add --privileged to Docker run for PID namespace support (unshare)
this fixes the test environment to properly test the sandbox. in production,
the action runs directly on GHA runner where sudo unshare works.
* fix getJobToken() to work in test environment
add fallback to GH_TOKEN and GITHUB_TOKEN when INPUT_TOKEN is not set.
this allows tests to run without requiring workflow-level token input.
the token resolution order is:
1. INPUT_TOKEN (from workflow `with: token:`)
2. GH_TOKEN (external token override)
3. GITHUB_TOKEN (pre-acquired in tests or from GHA env)
* security: filter secrets from all subprocess environments
- extract filterEnv() to shared utils/secrets.ts
- make $() utility filter secrets by default (git, npm, etc. don't need them)
- disable git hooks via core.hooksPath to prevent hook-based exfiltration
- git auth uses token embedded in URL, not env vars
this prevents malicious git hooks, npm postinstall scripts, and other
code execution vectors from exfiltrating GITHUB_TOKEN and API keys.
* docs: clarify defense-in-depth security model
update security.md to explain why BOTH layers are required:
- filterEnv(): cleans child's own /proc/self/environ
- PID namespace: hides parent's /proc entries
PID namespace alone isn't sufficient - with --mount-proc, the child
becomes PID 1, so /proc/1/environ is the child's OWN environment.
without filterEnv(), secrets would still be accessible there.
* add procSandbox crossagent test for PID namespace security
- add crossagent/procSandbox.ts: security test that instructs agent to try
various /proc attack vectors and validates sandbox blocks them
- update wiki/security.md: document PID namespace isolation details, add
verification commands, explain why sudo inside sandbox doesn't break security
- update docker.ts: use node:24 with sudo for GHA-like test environment
- update instructions.ts: allow disabling security messaging for pentests
- clean up adhoc test files (procIsolation.ts, securityAudit.ts)
the procSandbox test sets SANDBOX_TEST_TOKEN (matches *_TOKEN filter), gives
the agent explicit hints about attack vectors (/proc, sudo, nsenter, etc.),
disables prompt-based mitigations, and validates the secret is never leaked.
* move procSandbox test to agnostic/ (runs with one agent)
* WIP
* docs: add agent testing guide (pnpm play, Docker, pentesting)
* docs: add CI details to agent testing guide
* docs: add interesting findings and gotchas from pentesting
* improve test fidelity: auto-set CI=true, verify sandbox active
- docker.ts: always set CI=true in container so sandbox activates
- docker.ts: skip sudo/user setup if already done (faster reruns)
- bash.ts: export getSandboxMethod() for diagnostics
- bash.ts: add debug log when sandbox disabled
- procSandbox.ts: add sandbox_was_active check to catch vacuous passes
the CI=true change is critical: without it, PID namespace isolation
is skipped and security tests pass without actually testing anything.
* docs: update agent-testing.md with CI=true auto-set note
* docs: clarify log format is agent-specific
* fix git auth, simplify MCP tools, add adversarial tests
- fix gitWithAuth to use Basic auth format (Bearer doesn't work with git's http.extraheader)
- fix token.ts: push:restricted now correctly gets contents:write
- fix github.ts: pass permissions when acquiring installation tokens locally
- remove commit_files and create_branch MCP tools (redundant, don't require credentials)
- remove containsSecrets() - trivially bypassable, not sound security
- add agnostic adversarial tests: pushDisabled, pushRestricted, tokenExfil
- update instructions.ts to clearly list available git MCP tools
- add wiki/git.md documenting credential isolation and push permission tiers
- update wiki/docker.md with custom image considerations
- update wiki/agent-testing.md with adversarial testing patterns
* fix type errors after rebase
- change ResolveTokensParams.push from ToolPermission to PushPermission
- use tags: ["agnostic"] instead of agnostic: true in test files
* fix cleanup permission error in sandbox tests
when sandbox isolation is enabled (CI=true), files created by the unshare
subprocess may have different ownership, causing rmSync to fail with EACCES.
this error in the finally block was overriding the test's success result.
fix: wrap cleanup in try-catch and fall back to sudo rm if rmSync fails.
* Add adhoc
* Handle git config/remote bypasses
* add git hooks protection and simplify ToolState
- disable git hooks in restricted mode via -c core.hooksPath=/dev/null
- add gitHooks adversarial test to verify hook protection works
- unify prNumber/issueNumber into single issueNumber field
- add pushUrl to ToolState for push validation
- add generateTestMarker() for simpler single-agent test markers
- export SENSITIVE_PATTERNS and isSensitiveEnvName from secrets.ts
- remove redundant pidNamespace.ts (duplicated by procSandbox.ts)
- update documentation
* harden $git() auth: subcommand whitelist, binary tamper detection
- rename gitWithAuth() to $git() with explicit subcommand first arg
- restrict to "fetch" | "push" at type level (filters don't run for these)
- resolve git binary path at startup via resolveGit(), sha256 fingerprint
- verify hash before each $git() call to detect binary replacement
- rename disableHooks to restricted for cleaner semantics
- document filter exfiltration attack and empirical verification in wiki
* remove redundant pid-namespace CI job
the PID namespace isolation testing is now handled by
action/test/agnostic/procSandbox.ts via pnpm runtest agnostic
* fix push_branch for new branches and improve token leak detection
- getPushDestination now falls back to origin/<branch> when @{push}
is not configured (happens for new branches created locally)
- gitPerms validator now checks for actual token patterns instead
of matching "x-access-token" string in test instructions
* use kebab-case for test names
* simplify shell env API: "restricted" | "inherit" | object
replace passFullEnv boolean with cleaner env option that accepts:
- "restricted" (default): filterEnv() to prevent secret leakage
- "inherit": full process.env
- object: custom env merged with restricted base
* share EnvMode and resolveEnv between shell.ts and bash.ts
move shared env resolution logic to secrets.ts
* add env option to bash tool (default: restricted)
* delete agent-testing.md (renamed to adversarial.md)
* Add checkout tests
* reframe githooks test prompt to avoid claude safety refusal
claude was refusing to execute the test because the prompt used words
like "malicious" and "security testing". reframed as a debugging task
with innocuous env var name (TESTING_DEBUG_TAG) per adversarial.md guidance.
Co-authored-by: Cursor <cursoragent@cursor.com>
* clean up verbose token acquisition logs
move logging responsibility to call sites which have better context
(git token vs MCP token). remove redundant intermediate OIDC logs
and unused "(permission-scoped)" suffix.
Co-authored-by: Cursor <cursoragent@cursor.com>
* isolate agnostic tests with matrix strategy, fix .pullfrog-env secret leak
- split action-agnostic into per-test matrix jobs for isolated logs and filesystems
- only write explicitly opted-in env vars to .pullfrog-env via fileAgentEnv
(fixes token-exfil test where claude found SANDBOX_TEST_TOKEN on disk)
- mcpmerge test opts in via fileAgentEnv for cursor's repo-level MCP fallback
Co-authored-by: Cursor <cursoragent@cursor.com>
* remove env parameter from bash tool to prevent agents bypassing filterEnv
the bash tool exposed an `env` parameter accepting "restricted" | "inherit"
which allowed agents to pass env: "inherit" and see all secrets including
SANDBOX_TEST_TOKEN, bypassing the restricted environment filtering entirely.
env mode is now determined internally (always restricted).
Co-authored-by: Cursor <cursoragent@cursor.com>
* use pullfrog/test-repo for push tests to stop polluting main repo
push tests were creating branches and tags on pullfrog/app directly.
now all push tests (push-restricted, push-disabled, push-enabled,
git-permissions) target pullfrog/test-repo instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
* use pullfrog/test-repo for all tests, not just push tests
no test should clone or operate on pullfrog/app directly.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix token scoping for test-repo and bash timeout defaults
- acquireTokenViaOIDC now includes GITHUB_REPOSITORY repo in token
scope so push tests work against pullfrog/test-repo
- bash tool default timeout: 120s -> 30s, cap: 600s -> 120s
- activity timeout: 30s -> 60s
- prevents hung bash commands (e.g. find /) from killing the agent
via activity timeout
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Mateusz Burzyński
|
adc165d95f |
Tweak comment footer (#208)
* Tweak comment footer * take 2 * tweak * tweak * add dev path * defensively guard against missing job in the array |
||
|
Colin McDonnell
|
0a4fcc556a |
Improve Review mode instructions (#194)
* Review hard * Clean up suggestion instrcuctions * Permalink tip * Update action/modes.ts Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> |
||
|
Colin McDonnell
|
102417f442 |
Add post hooks for cleanup (#193)
* Add post hooks for cleanup * Switch to signal-based cleanup * Better exit handling |
||
|
Colin McDonnell
|
7144f3de88 | Tweaks | ||
|
Colin McDonnell
|
cb925556e8 | refactor instructions to return object with full/system/user/event/runtime properties, fix duplicate modes and json prompt extraction (#110) | ||
|
Colin McDonnell
|
69b9b96ddd | Refactor (#109) | ||
|
Colin McDonnell
|
9e019d89d2 |
Clean up actions and payloads (#98)
* Clean up actions and payloads * Clean up action * Cleanup |
||
|
pullfrog[bot]
|
71feba0a76 |
fix: prevent log.writeSummary from overwriting reportProgress content (#87)
* fix: prevent log.writeSummary from overwriting reportProgress content The run summary was showing logs instead of the final reportProgress content because log.writeSummary() was called after reportProgress. Now log.writeSummary() checks if the summary was already overwritten by reportProgress and skips if so. Fixes #86 * refactor: replace dynamic import with static import in cli.ts Replace unnecessary dynamic import of wasSummaryOverwritten with static import. No circular dependency exists since comment.ts doesn't import from cli.ts. * Fix run summary writing --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> Co-authored-by: Colin McDonnell <colinmcd94@gmail.com> |
||
|
pullfrog[bot]
|
3b880eb478 |
Implement GitHub suggestion format instructions (#79)
* implement github suggestion format instructions add instructions for agents to use github's suggestion format (```suggestion blocks) when providing code suggestions in comments. this enables one-click apply for suggested changes. updated: - action/mcp/review.ts: added suggestion format guidance to create_pull_request_review tool description and comment body parameter - action/mcp/comment.ts: added suggestion format guidance to all comment tools with clarification that suggestions only work on pr line-level review comments - action/modes.ts: added detailed example in review mode and reminder in address reviews mode fixes #70 * Address PR review feedback - Remove suggestion format guidance from report_progress (not applicable) - De-duplicate description across Comment, EditComment, ReplyToReviewComment - Drop outer fence in suggestion format example - Clarify that suggestions only work for self-contained changes - Remove useless example comment from review tool description --------- Co-authored-by: pullfrog <team@pullfrog.com> |
||
|
Colin McDonnell
|
45f837cedb | Fixes | ||
|
Robin Tail
|
7fe0233c24 | fix(DNRY): Moving isGitHubActions to the module context (expensive operation), and the condition to updateSummary(). | ||
|
Robin Tail
|
e10f756560 | fix(DNRY): Extracting the summary writing into updateSummary() helper. | ||
|
Robin Tail
|
48108b137a | fix(DNRY): Extracting isGitHubActions flag. | ||
|
pullfrog
|
074a860a95 |
Update job summary with progress comment content
Modified reportProgress() to write the same content to core.summary with overwrite: true. This replaces the verbose log accumulation with the concise progress updates that stakeholders see in comments. The job summary now stays in sync with the progress comment, providing a clean overview of the agent's work rather than accumulated logs throughout execution. Fixes #17 |
||
|
Mateusz Burzyński
|
a483711fee | Auto-retry ratelimited octokit requests | ||
|
Colin McDonnell
|
0504fc42ff | Do not print 'This run croaked' if the agent only replies in a PR review comment | ||
|
Colin McDonnell
|
ad1f51d704 | Implement Plan button | ||
|
Colin McDonnell
|
c200c7aff9 | Tweak message | ||
|
Colin McDonnell
|
04c695038f | 151 |