1026 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Colin McDonnell	b6df2860c3	action: bump to 0.1.7 v0.1.7	2026-05-14 02:48:31 +00:00
Colin McDonnell	d495f0b984	surface BYOK failures + chronic-failures card + WorkflowRunStatus mirrors GitHub conclusions (#722) ... - Migrates `WorkflowRunStatus` from `running | completed | cancelled` to a 9-state mirror of `workflow_run.conclusion`. Backfill: old `completed → success`, `cancelled → failure`. New rows write `hook.workflow_run.conclusion` verbatim via `statusFromConclusion`. - Adds Discord links to `formatApiKeyErrorSummary` (both missing-key and 401 invalid-key shapes). - Repo console: `<ChronicFailuresCard>` fires when the last 3 terminal-state runs are all `failure`. Pure DB read; latest-run button hidden for pre-dispatch failures (`runId: null`). - `StatusIcon` distinguishes `cancelled` (gray X, intentional stop) from `failure` (red X) so the visual matches the chronic-card threshold. - Pre-dispatch failures (workflow lookup miss, dispatch API error) write `failure` instead of `cancelled` so they feed the card. - Cascade: every `status: "completed"` filter in billing routes / cron / cohort queries / analyzer becomes `status: "success"`. Verified end-to-end on `pullfrog/preview-722-failure-surfaces` — Better Stack logs confirm webhooks reached the preview deploy and all three e2e runs got `marked as failure (conclusion=failure)` via the new mapper. Closes #679, #702.	2026-05-14 02:39:41 +00:00
Colin McDonnell	206c11fe7c	review: drop misleading 'with the same arguments' from diff-coverage nudge ... agent is free to refine review body/comments on retry — there's no enforcement that the second call matches the first, and if reading the nudged region surfaces a new finding the agent should add it.	2026-05-14 02:36:06 +00:00
Colin McDonnell	7414c1e9ca	review: clarify diff-coverage nudge gives explicit license to skip generated artifacts ... the one-time pre-flight nudge said "optionally read" but never told the agent it's free to retry without reading when every unread region is generated (lockfiles, codegen, snapshots, migration metadata). audit #677 surfaced ~21 runs/24h burning an extra model turn re-reading drizzle snapshots, pnpm-lock, and *.gen.ts files purely to satisfy the gate. mode prompts only mention generated content in the "skip self-review entirely" path, not the "in-progress substantive review" path, so the in-the-moment error message was the gap. behavior unchanged for legitimately-unread source regions.	2026-05-14 02:30:37 +00:00
Colin McDonnell	8f9208bd3f	feat: Amazon Bedrock support via routing slug (#720) ... * add Amazon Bedrock as a routing slug introduces a single `bedrock/byok` catalog entry that the harness translates to the appropriate Bedrock model ID at run time via `BEDROCK_MODEL_ID`. routes Anthropic IDs through claude-code (with `CLAUDE_CODE_USE_BEDROCK=1`) and everything else through opencode's `amazon-bedrock` provider — keeps the catalog flat for an audience that needs version pinning rather than aliasing. accepts either `AWS_BEARER_TOKEN_BEDROCK` or `AWS_ACCESS_KEY_ID` + `AWS_SECRET_ACCESS_KEY` for auth; both validated alongside `AWS_REGION` and `BEDROCK_MODEL_ID` in `validateAgentApiKey`. catalog drift tests, the bumps cron, and per-alias smoke scripts all skip routing slugs since there's no fixed `resolve` to validate. docs/bedrock.mdx walks through setup; wiki/model-resolution.md has a section explaining why bedrock breaks the usual alias pattern. closes pullfrog/pullfrog#40 * ci: add bedrock env vars to test workflows mirrors the new bedrock provider's required env vars (AWS_BEARER_TOKEN_BEDROCK inherited from org secret + AWS_REGION + BEDROCK_MODEL_ID hardcoded) into both .github/workflows/test.yml files so the ci.test "env vars cover all provider API keys" assertion passes. * docs(bedrock): clearer setup flow + screenshot of model selector restructures the setup section into three concrete steps in execution order: select Bedrock from the dropdown, store the bearer token as a secret (Pullfrog or GitHub — links to keys.mdx for the trade-off), then add region + model id directly in pullfrog.yml since neither is sensitive. enable-model-access in the Bedrock console moved to step 4 (only required once per model and only when AWS rejects the call, not blocking on first run). adds a screenshot of the console model selector with Amazon Bedrock selected so readers can recognize the UI state they're aiming for. * fix(bedrock): tolerate raw Bedrock model IDs in validateAgentApiKey main.ts passes the resolved model into validateAgentApiKey (`payload.proxyModel ?? resolvedModel ?? payload.model`). For Bedrock, `resolveModel` translates `bedrock/byok` into the raw AWS model ID (e.g. `us.anthropic.claude-opus-4-6-v1`), which has no `/` and so trips parseModel inside getModelEnvVars. Detect the no-slash case and re-run the bedrock setup check (auth + region; BEDROCK_MODEL_ID is already enforced upstream by resolveModel). Caught by PR #720 e2e dispatch on pullfrog/preview-720-bedrock — "invalid model slug 'us.anthropic.claude-opus-4-6-v1' — expected 'provider/model'". Two regression tests cover the raw-ID path. * fix(bedrock): always prepend amazon-bedrock/ prefix when bedrock-routed opencode.ts was gating the prefix-injection on `!isBedrockAnthropicId(rawModel)`, on the theory that Anthropic Bedrock IDs always go through claude-code. But `PULLFROG_AGENT=opencode` is a documented escape hatch — when it forces opencode for an Anthropic Bedrock model, the prefix still has to be added or opencode fails with 'Model not found: <modelId>/.'. The Anthropic-vs-other discriminant only belongs in resolveAgent. Once an agent is selected, it should consistently honor the bedrock route. Caught by the PULLFROG_AGENT=opencode + Opus 4.6 e2e on pullfrog/preview-720-bedrock — run 25823437606. * ui+docs(bedrock): bespoke setup callout + clearer docs UI: - BedrockSetupCallout in components/AgentSettings.tsx covers both the Model costs section and the onboarding card. Detects bedrock via resolveDisplayAlias().routing === "bedrock", shows a dedicated message ("store AWS_BEARER_TOKEN_BEDROCK as a secret, then put AWS_REGION + BEDROCK_MODEL_ID directly in pullfrog.yml") + link to the setup guide. Replaces the generic "X, Y, or Z is required" prompt that misrepresented the three values as three separate secrets to add (and used the wrong "or" connector for what's actually an AND). - OnboardingCard re-uses the same callout with the gradient-card variant. Docs: - Drop the obsolete "Enable model access" step. AWS retired the manual enrollment page; foundation models auto-enable on first invocation. Anthropic models still need a one-time use-case form for first-time users — surfaced under the AccessDenied troubleshooting entry. - Drop the "Testing a different model in one run" PULLFROG_MODEL note. It introduced the secrets-vs-vars distinction we want to keep out of the bedrock setup story. - Step 3 already recommends hardcoding region + model id in pullfrog.yml. Workflow template: - The default pullfrog.yml customers receive (utils/github/pullfrog.yml.ts) now references AWS_BEARER_TOKEN_BEDROCK from secrets but inlines AWS_REGION and BEDROCK_MODEL_ID as plain values. Matches the docs. * fix(bedrock): three review-caught edges in routing + UI copy Addresses three real issues from PR #720 review: 1. agent.ts: PULLFROG_MODEL=bedrock/byok no longer leaks the literal sentinel "bedrock" downstream. resolveCliModel returns the alias's resolve field verbatim, which for routing entries IS the sentinel. Refactored both the env-override and slug-lookup paths through a shared resolveSlug() that recognizes routing aliases and defers to their backing env var (BEDROCK_MODEL_ID). 2. models.ts: isBedrockAnthropicId() now anchors on a discrete dot/slash/colon-segment match (case-insensitive) instead of a substring contains. The substring check was fragile in both directions for inference-profile ARNs (BEDROCK_MODEL_ID accepts ARNs per AWS docs) — a non-Anthropic profile whose user-chosen name contained "anthropic" would mis-route to claude-code, and an Anthropic profile whose name omitted it would miss CLAUDE_CODE_USE_BEDROCK=1. 3. AgentSettings.tsx: BedrockSetupCallout's configured-state copy showed "AWS_BEARER_TOKEN_BEDROCK configured" even when the user satisfied the gate via AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY, gaslighting access-key users about a secret they never set. Detect which auth method is actually present and name the right secret(s) in the success message. Regression tests in models.test.ts (5 new isBedrockAnthropicId cases including positive and negative ARN forms) and agent.test.ts (2 new PULLFROG_MODEL=bedrock/byok cases). 171/171 action tests pass. * yml template: add commented AWS access-key alternative for Bedrock auth Mirrors the IAM access-key path verified end-to-end on PR #720 e2e run 25830764987. Bearer token stays as the primary nudge; the access-key pair is the fallback for users who can't mint Bedrock API keys. * yml template: drop redundant 'or, alternatively' annotation * ui+docs(bedrock): rewrite callout copy + refresh screenshot Reframes the BedrockSetupCallout away from generic BYOK language to a Bedrock-specific message: leads with "Amazon Bedrock is configured entirely via environment variables", lists all four (auth, region, model id), and ends with the requested CTA sentence ("click below to learn more about Bedrock support in Pullfrog"). Promotes the "Bedrock setup guide" docs link from an inline anchor to a prominent button (always visible, regardless of auth state). The "Add AWS_BEARER_TOKEN_BEDROCK" affordance is now a secondary chip shown only when no auth secret is configured. Refreshes docs/images/model-selector-bedrock.png to capture the new callout — the prior screenshot still showed the old generic "BYOK / X, Y, or Z required" wording.	2026-05-14 02:12:38 +00:00
Colin McDonnell	1a9d3c1f82	fix bootstrap ETARGET when customer has npm min-release-age policy (#725) ... * fix bootstrap ETARGET when customer has npm min-release-age policy set npm_config_min_release_age=0 in the action runtime env so `npx --yes pullfrog@<spec>` doesn't get rejected by a customer-side release-age gate (npm 11.5+'s min-release-age / pnpm's minimumReleaseAge). env vars beat .npmrc in npm config precedence, so this neutralises the policy regardless of where it's defined. pullfrog's npm version is server-stamped from a SHA-pinned action ref customers already vet at the action layer — it isn't a customer-vetted dep, so the release-age policy is the wrong affordance for our bootstrap and would otherwise hard-fail every run while the latest publish ages into the customer's window. closes #713 * also cover pnpm's minimumReleaseAge key for corepack fallback path * correct pnpm env var (pnpm v11+ uses pnpm_config_*, not npm_config_*) the prior commit set `npm_config_minimum_release_age=0` to cover the pnpm corepack-dlx fallback path, but pnpm v11+ only reads env vars prefixed `pnpm_config_*` / `PNPM_CONFIG_*` (the v10→v11 migration explicitly renamed the prefix). swap to the correct env var so the fallback path actually neutralises pnpm's `minimumReleaseAge`. also tighten the comment block, and add an AGENTS.md rule reminding us to fetch top-level reviews AND inline review comments together — they live on different endpoints and the inline set is easy to miss with `gh pr view --json reviews,comments` alone. * add scripts/pr-reviews.ts for one-shot review evaluation dumps top-level reviews + inline review threads (with resolved/outdated state) + PR-level conversation in a single GraphQL round trip, so agents don't miss inline-comment feedback. fixes the trap where `gh pr view --json reviews,comments` silently omits the inline `pulls/{n}/comments` set. borrows `gh auth token` so no env vars are required. registered in `wiki/scripts.md`; AGENTS.md rule updated to point at the script instead of the two-step gh-CLI workaround. * pr-reviews: dump raw JSON for jq piping	2026-05-14 01:48:14 +00:00
Colin McDonnell	951745ec89	disable stop hook (runtime + dashboard) (#727)	2026-05-14 01:44:32 +00:00
Colin McDonnell	56793d4a81	claude: prefer non-JSON stdout over NDJSON tail in exit-1 fallback (#643) (#726) ... Claude CLI under CLAUDE_CODE_OAUTH_TOKEN exits 1 without setting `is_error` when the OAuth subscription's quota is exhausted. The existing fallback chain (`lastResultError || stderr || tailLines(stdout)`) had nothing structured to grab and dumped ~2KB of `system/init` NDJSON into the progress comment, hiding the actionable quota notice the CLI had already printed as plain text. Capture non-JSON stdout lines into a 20-line ring buffer (mirroring the existing `recentStderr` pattern) and prefer it over the raw NDJSON tail. Generic — no regex on bubble text — so any human-readable line the CLI emits surfaces instead of the event stream. Also adds a `failure:claude-oauth-quota` bucket to `analyze-logs.ts`, ordered before the SIGTERM check so the NDJSON tail's `cancelled` / `cancel_url` substrings (from learnings content) stop shadowing it.	2026-05-14 01:26:10 +00:00
Colin McDonnell	d857e06731	postrun: tighten unsubmitted-review gate to require create_pull_request_review for Review mode (#724) ... The gate at `getUnsubmittedReview` accepted `toolState.finalSummaryWritten` as a valid Review exit, contradicting the post-failure error message which already says Review's only valid exit is `create_pull_request_review`. This let any caller that flipped `finalSummaryWritten` — including a `task`-dispatched `reviewfrog` subagent calling `pullfrog_report_progress` in violation of its prose-only read-only contract — silence the gate even when the orchestrator never submitted a review. Split per-mode: Review requires `toolState.review`, IncrementalReview keeps the existing `||` (its post-failure message explicitly accepts `report_progress` as a "no review warranted" exit). Test split mirrors the new semantics. closes #648	2026-05-14 00:01:15 +00:00
Colin McDonnell	b9f0938405	mcp: restore operational guidance dropped in #723 ... #723's revision pass cut four substantive strings along with the negative anchors. those strings address real, audit-observed failure modes and the positive examples don't carry them. restored: - push_branch: "if the response reports a timeout, the underlying push may have actually succeeded — verify with git log origin/<branch> before retrying" (was on the tool description) - create_pull_request_review commit_id .describe(): "must be the FULL 40-character SHA — abbreviated SHAs are rejected by GitHub with 422" - create_pull_request_review comments[].line .describe(): "must sit inside a `@@` hunk... dropped entries are reported under droppedComments in the response" - create_pull_request_review comments[].start_line .describe(): "both start_line and line must sit inside the same @@ hunk" also: get_commit_info example used a 31-character SHA (non-standard truncation). swapped to a 7-char short form, which is what git log --oneline emits and what agents see in practice. note that this tool accepts either full or abbreviated, unlike create_pull_request_review which requires full.	2026-05-13 22:49:06 +00:00
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.	2026-05-13 22:45:08 +00:00
Colin McDonnell	868576a474	audit: format byok auth errors actionably + tighten audit prompt ... - `action/utils/apiKeys.ts`: rewrite the missing-key body as Markdown with linked CTAs (repo secrets / model settings / docs). add `isApiKeyAuthError` + `formatApiKeyErrorSummary` covering both shapes: missing key (#679) and revoked/invalid 401 key (#702). - `action/main.ts`: reclassify in the result-failure branch and the catch block so the PR progress comment surfaces the actionable CTA instead of the raw `Invalid API key · Fix external API key` / numbered-list dump. - `scripts/analyze-logs.ts`: split `failure:user-misconfig` into `:no-key` and `:invalid-key` so both buckets are visible separately and the audit can ignore them as user-correctable. - `.github/workflows/run-audit.yml`: add three explicit prompt rules — cross-customer signal required (≥3 distinct accounts; single-customer concentration is not enough), recovered failures are not actionable, user misconfig is out of scope. closes the loop on #679 / #702 being filed in the first place.	2026-05-13 21:59:47 +00:00
Colin McDonnell	b2b1e588e7	biome: exclude .scripts/ — gitignored operator scratchpad ... Mirrors the gitignore. Same shape as the existing !**/logs / !**/.logs / !.worktrees exclusions in files.includes. Matches the upstream .gitignore policy for the .scripts/ directory. Without this, .scripts/ scripts (`.scripts/kyle-*.ts`, `.scripts/check-comment.ts`, etc.) get scanned by `pnpm lint` and `pnpm format` from the repo root and routinely fail husky pre-push even though they're explicitly intended to be local-only / personal. The companion to .gitignore — both are operator-owned scratchpads; neither participates in repo-wide hygiene.	2026-05-13 21:25:43 +00:00
Colin McDonnell	5caeb75344	review: 0-or-2+ lens rule, parallel-or-bust, downshifted subagent models (#710) ... * review: 0-or-2+ lens rule, parallel-or-bust, downshifted subagent models PR review wall-time was dominated by two failure modes: orchestrator serial-dispatching subagents (despite prompt asking for parallel) and running every lens on the same Opus tier as the orchestrator. Sample of recent runs showed 25-60min reviews on small PRs, with 8-10min idle gaps between subagent dispatches. Three changes: 1. `action/modes.ts` — replace the soft "1 trivial / 2-3 typical / 4-5 high-stakes" lens calibration with a binary 0-or-2+ rule. Default is 0 lenses (orchestrator handles review solo with optional cheap tracerfrog dispatches). 2+ parallel lenses only fire for substantive PRs (>5 files AND >200 lines) or high-stakes-subsystem touches. Never exactly one. Both Review and IncrementalReview prompts get loud ALL-CAPS framing on parallel dispatch — emit ALL Task tool_use blocks in a single assistant turn before reading any result. Drop the "do NOT lens-review the diff yourself" advice; orchestrator pulls context aggressively, in parallel with the lens fan-out. 2. New `tracerfrog` subagent for mechanical code tracing ("where is X used / who calls Y / what depends on Z"). Pure read+grep+report with no judgment — orchestrator can dispatch many tracers cheaply in parallel. Defined in `action/agents/reviewer.ts`. Wired into both claude.ts (`--agents` JSON) and opencode.ts (`agent` config block). 3. Per-subagent model downshifts via `deriveSubagentModels`: - Anthropic: reviewfrog → Sonnet, tracerfrog → Haiku - OpenAI: both → gpt-5.4-mini - other providers (xai, deepseek, gemini, etc.): inherit (no standard tier triplet to downshift to) Claude Code path always runs Anthropic so the downshift is hardcoded inline in claude.ts. OpenCode uses the helper since orchestrator provider varies. Both runtimes' subagent-definition formats verified directly against their source: `--agents` JSON `model` field (claude-code's `AgentJsonSchema` accepts model+effort+maxTurns+more) and OpenCode's `agent.{name}.model` config field (parsed via Provider.parseModel, applied per-task in tool/task.ts line 92). Parallel dispatch is infra-supported in both — only the orchestrator model's tool_use emission pattern was the bottleneck. Tests: subagentModels.test.ts (14 tests covering provider matrix), subagentRegistration.test.ts (6 source-asserts catching shape regressions in buildAgentsJson / buildReviewerAgentConfig). * subagentModels: add openrouter routes (proxy/router mode) Initial helper missed the openrouter prefix used by Pullfrog's router proxy. preview-710 e2e showed the OpenCode + openrouter path receiving no downshift — orchestrator and lenses both ran on opus-4.7 because 'openrouter/anthropic/claude-opus-4.7' didn't match any of the anthropic/openai prefixes the helper checked. Add explicit branches for 'openrouter/anthropic/...' (uses dot notation: claude-sonnet-4.6 / claude-haiku-4.5) and 'openrouter/openai/...' (gpt-5.4-mini for both reviewer and tracer). Same opus->sonnet, sonnet->keep-but-haiku-tracer, haiku->no-op semantics as the direct anthropic path. * opencode: log resolved subagent models at startup So we can verify per-subagent model overrides actually take effect at runtime. Prints once per run alongside the existing model/effort log lines. * drop tracerfrog: keep reviewfrog only, LSP-powered tracer planned later Removes the cheap-haiku-tracer subagent (TRACER_AGENT_NAME + TRACER_SYSTEM_PROMPT, registrations in claude.ts/opencode.ts, dispatch guidance in modes.ts). The mechanical-tracing use case will be served better by an LSP-powered tool than by a separately-prompted subagent. deriveSubagentModels collapses to a single { reviewer } shape; the reviewfrog-on-Sonnet downshift stays. Same source-assert + provider- matrix tests, minus the tracer-specific cases. modes.ts wording: drop the 'subagent type cheat sheet' bullet, drop the parenthetical 'often better served by tracerfrog than reviewfrog' on the impact lens, drop tracerfrog from the same-turn-context-pulling hint. The 0-or-2+ rule and ALL-CAPS parallel emphasis are unchanged. * subagentModels: broader downshift coverage (gpt-pro, gemini-pro, grok); drop gpt-mini target Scanned every resolved orchestrator slug in action/models.ts against models.dev pricing data. Identified five clear cases where the orchestrator is meaningfully expensive AND has a cheaper sibling that remains capable enough for review-style judgment work. Changes: - Anthropic: opus → sonnet (kept; -40%) - OpenAI: gpt → gpt-5.4 (was: gpt-mini; -54% instead of -85% but preserves review-quality judgment — gpt-mini was too dumb) - OpenAI: gpt-pro → gpt (NEW; -93%, biggest single unlock — gpt-5.5-pro is $30/Mtok in vs gpt-5.5 at $5) - Google: gemini-pro → gemini-flash (NEW; -75%) - xAI: grok-4.3 → grok-4-1-fast (NEW; -80%) Every branch handles the three routes in use: direct provider slug, opencode-vendored, and openrouter-proxied. Variants below the downshift target (mini/nano/flash/fast/sonnet/haiku) inherit (no further drop). Skipped: - DeepSeek: v4-flash ($0.14/Mtok) is too far below review judgment threshold; v4-pro orchestrator already cheap ($0.55 blended). - Moonshot: kimi-k2-thinking would only save 32% and slug stability on OpenRouter is uncertain; revisit if cost matters. - o3: already mid-tier in OpenAI's reasoning family; no clean target. * models: hoist subagent downshift into the registry, add hidden flag The downshift relationship now lives next to each alias's resolve / openRouterResolve as a sibling field. Two new ModelDef fields: - subagentModel?: string — alias key (within same provider) of the cheaper sibling reviewfrog should use as a lens-fanout subagent. e.g. claude-opus → 'claude-sonnet'. - hidden?: boolean — exclude from selectable lists (UI dropdown, CLI init picker). Does NOT affect resolution; for that use fallback. Used so internal-only subagent targets like openai/gpt-5.4 exist in the registry but never appear as a user-facing pick. Wiring: - anthropic.claude-opus → claude-sonnet (-40%) - openai.gpt-pro → gpt (-93%, biggest unlock) - openai.gpt → gpt-5.4 (-54%); gpt-5.4 added with hidden:true - google.gemini-pro → gemini-flash (-75%) - mirrored across opencode + openrouter providers (each provider declares its own three-route data so the downshift declaration is colocated with the rest of the alias definition). deriveSubagentModels collapses from ~85 lines of prefix-matching to a ~15-line registry reverse-lookup: find the alias whose resolve OR openRouterResolve matches the orchestrator's spec, follow its subagentModel pointer, return the matching field of the target alias. Filter sites updated: - components/ModelSelector.tsx: !a.fallback && !a.hidden - action/commands/init.ts: same Tests rewritten to exercise the registry through the public surface; the matrix collapses to one assertion per (provider × route) pair. * TEMP: log per-step cost+tokens for subagent model verification (PR #710) * TEMP: also log SUBAGENT step_finish from bus envelope handler * remove temporary per-step diagnostic logs (verification done) Verified subagent model downshift takes effect end-to-end on the OpenCode + openrouter path. PR #8 in pullfrog/preview-710-review-perf dispatched 3 lenses (billing-subsystem / security / correctness) on the orchestrator's opus-4.7 session, and per-subagent step_finish events showed actual cost exactly matching Sonnet pricing rates (60% of what Opus would have cost): session n actual if-Opus if-Sonnet match T3VrUuF... 5 $0.2425 $0.4042 $0.2425 Sonnet ✓ 93ZZR7E... 4 $0.2253 $0.3754 $0.2253 Sonnet ✓ Fb1Kr7b... 4 $0.2495 $0.4158 $0.2495 Sonnet ✓ The startup '» subagent models: reviewfrog=...' line stays — useful permanent diagnostic showing the resolved subagent model per-run. * TEMP: log per-event model from claude.ts assistant handler * remove temporary per-event model log (claude.ts verification done) Verified subagent model downshift takes effect end-to-end on the Claude Code path. PR #9 in pullfrog/preview-710-review-perf dispatched 2 lenses on an opus-4-7 orchestrator. Per-assistant-event model field from the SDK's stream-json output, partitioned by parent_tool_use_id: ORCH (parent_tool_use_id=null): 17 events all model=claude-opus-4-7 SUBAGENT lens:billing-subsystem: 17 events all model=claude-sonnet-4-6 SUBAGENT lens:security: 21 events all model=claude-sonnet-4-6 Zero leakage to opus from either subagent session. The per-subagent 'model' field in --agents JSON is honored by claude-code at the SDK level, identical to the OpenCode path verified earlier. * opencode: bump per-call output cap 5K → 16K to unblock large reviews The 5K cap (added in #616 to lower OpenRouter upfront budget reservation for low-wallet runs) was capping the entire response of a single LLM call, not just the budget reservation. A single tool_use response — like a `create_pull_request_review` with many inline comments — would truncate mid-stream past 5K output tokens, leave the JSON unparseable, and the tool would never actually invoke. We hit this on PR #710's verify-downshift PR: review aggregated from 3 lenses had 11 inline comments + a long body, truncated at out=5000 on every retry attempt, action exited with 'Review mode finished without calling create_pull_request_review after 3 retry attempts'. Investigated whether OpenCode (or OpenAI/Anthropic/OpenRouter directly) exposes a separate budget-reservation parameter that could stay small while letting the response exceed it. They don't — `max_tokens` / `max_completion_tokens` is the single value all four use for both the upfront reservation and the hard output ceiling. No way to decouple them at the API surface. Bumped to 16K as a middle ground: 8× the prior cap (handles every review shape we've observed plus headroom), still half of OpenCode's 32K default so the wallet-burn benefit for low-balance accounts is preserved, just smaller. For Opus 4.7 a typical ~50K-input call now reserves roughly $0.65 instead of the prior $0.38. Updated the constant comment to spell out the trade-off clearly so this doesn't happen again. * opencode: drop OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX override entirely Verified the original rationale for the override is obsolete. From #616 the cap shrunk OpenRouter's per-call upfront budget reservation so a single call's reservation wouldn't exceed the per-run key cap (`ROUTER_PER_RUN_LIMIT_USD = 25`) and lock low-balance accounts out of starting a run. That per-run gate is gone. `app/api/proxy-token/route.ts` ~line 422 explicitly says: 'No upper cap (the old ROUTER_PER_RUN_LIMIT_USD = 25 is gone). The natural ceiling is whatever the user has + their buffer.' Router now mints keys with `keyLimitCents = balance + buffer` ($50 for autoreload+card, $5 for card-only, $0 for no-card). A single call's upfront reservation fits comfortably within that — no separate per-call gate to fail past. The cap had a real downside as a hard per-call output truncation. A single `create_pull_request_review` tool_use with many inline comments would truncate mid-stream past 5K output tokens, the JSON would be unparseable, and the tool never invoked. Hit on PR #710's verify-downshift PR. Removing the override entirely; OpenCode falls back to its 32K default. Left an explanatory note above the env-var assignment site so the next person doesn't unknowingly re-add it.	2026-05-13 21:05:52 +00:00
David Blass	5518890b18	learnings: TOC + section taxonomy + 100k cap, hygiene rules, tool-quirk descriptions (#717) ... * audit learnings: reshape reflection prompt + bake tool quirks into descriptions (#619) Cross-repo audit of the 48 repos with non-null learnings turned up two recurring failure modes: 1. ~25-30% of bullets across the most-active repos are pullfrog-tool quirks ("shell timeout is in milliseconds", "git args must be a JSON array", "create_pull_request_review drops out-of-hunk comments", "push_branch may report timeout when push succeeded", "checkout_pr shallow.lock retries", "commit_id needs full 40-char SHA"). These are universal across repos and should live in tool descriptions, not be rediscovered and stored 48 times. Tool descriptions now surface them. 2. Bullets are routinely 200-1000 chars (paragraph-length), and 12 of 48 repos are at the 10k cap. The reflection prompt now: caps bullets at ~240 chars (one specific fact), bans PR/review/commit/date-anchored facts that decay within weeks, bans tool-quirk learnings, and tells the agent that cap pressure means compress+prune existing bullets, not skip new findings. Co-authored-by: Cursor <cursoragent@cursor.com> * learnings: add server-generated TOC, fixed section taxonomy, raise cap to 100k (#707) Cap goes 10k → 100k. Reads stay bounded because the seeded file now opens with a server-generated table of contents listing every `## ` section's line range — agents read the TOC, then `read_file offset/limit` just the sections relevant to the current task instead of slurping the whole file. ## Section taxonomy (fixed) `## Build & test`, `## CI`, `## Conventions`, `## Architecture`, `## Gotchas`. Free-form `### ` sub-headings inside a section are fine. Pre-taxonomy free-text rows get wrapped in a `## Legacy` carve-out on first seed so they remain visible while the agent gradually re-curates them during reflection turns. ## Storage shape unchanged `Repo.learnings` still holds raw markdown (no schema migration). The TOC is a pure read-side affordance: prepended at seed time, stripped from the agent-edited file before persist. Markers `<!-- pullfrog-learnings-toc:* -->` delimit the strip region. Agent edits inside the markers are discarded. ## Round-trip semantics `seedLearningsFile` now returns `{ path, canonicalSeed }` where `canonicalSeed` is the post-TOC body — same shape `readLearningsFile` returns at end-of-run, so `persistLearnings` byte-compares them directly to skip the no-op PATCH. Empty-repo first runs end up with the section scaffold both as seed and as read-back, so untouched runs still short-circuit cleanly. ## Reflection prompt Adds explicit section-placement guidance (place each new bullet under the most relevant `## `; do NOT add new top-level headings; do NOT edit anything between the TOC markers). Carries forward the bullet hygiene from the previous commit: ≤240 chars per bullet, no pullfrog-tool quirks (those belong in tool descriptions), no PR/review/commit/date references. The "near cap" framing is replaced with "compress and prune within a section when it grows noisy" since the cap pressure that drove cramming is gone. Co-authored-by: Cursor <cursoragent@cursor.com> * anneal round 1: line-anchored taxonomy detect, partial-merge, line-boundary truncation, scaffold-empty UI Multi-lens review of the TOC + taxonomy diff surfaced a cluster of correctness and operational bugs. Fixes: - `hasAnyTaxonomyHeading` used `String.includes("## X")` which false-positives on `### X` (the `## ` substring sits inside `### `), prose containing `## CI`, fenced code documenting markdown, etc. Replaced with a line-anchored predicate that reuses `parseHeadings` so detection and TOC construction stay consistent. - The "any heading present → pass through verbatim" rule meant a body with one taxonomy heading would seed without the other four. Worse, requiring all five would flip a body back into Legacy when the agent legitimately pruned a section to empty. New `partial` kind: keep existing content in place, append missing sections in canonical order so the agent always has the full scaffold without losing pruning intent. - `stripLearningsToc` collapsed `\n{3,}` globally; `canonicalSeed` doesn't, so an untouched body with intentional triple-newline spacing would compare unequal and burn a spurious LearningsRevision row each run. Drop the global collapse — only the leading newlines that the strip itself introduces are normalized. - 100k truncation via `slice(0, 100_000)` could cut mid-line, breaking `parseHeadings` (whole-line `^## `) on the next seed and flipping a cut body back into Legacy. New `truncateAtLineBoundary` cuts at the last newline before the cap. - `LearningsSection.tsx` rendered a scaffold-only body as "has learnings" instead of the empty placeholder. Added a `hasOnlyEmptyScaffold` guard so the console behaves the same as pre-PR for the empty case. - Seed log line distinguishes `kind=structured/partial/legacy-wrapped/ empty` instead of `existing=yes/no`, so operators can spot legacy migration activity in logs. - New tests cover: substring false-positive (`### Build & test`, in-prose mentions), partial-taxonomy merge (no Legacy wrap), full-taxonomy structured pass-through, last-newline truncation, triple-newline preservation. Deferred (documented in PR body): deploy-ordering footgun (action before API), rollback for rows >10k, Gemini sanitizer dropping `description` on `anyOf` branches, reflection-on-failed-runs. Co-authored-by: Cursor <cursoragent@cursor.com> * anneal r2: hard-truncate fallback when line boundary discards >4k Round-2 review caught a regression in `truncateAtLineBoundary`: when the only newline within the first 100k chars sits near the start (e.g. one heading + 100k+ char single line — pathological pasted log dumps), the line-boundary cut discards almost all of the body. losing one partial line is preferable to losing kilobytes; threshold the fallback at 4k. Co-authored-by: Cursor <cursoragent@cursor.com> * move TOC out of file: prompt-side rendering, server-parsed headings drops the in-file TOC + fixed taxonomy in favor of: - file on disk = verbatim Repo.learnings (no markers, no scaffold) - server parses headings (mdast-util-from-markdown) at run-context time and returns them as RepoSettings.learningsHeadings - action renders heading TOC into the LEARNINGS prompt section as parenthesized line ranges like `Build & test (L1-L42)` with hierarchy via 2-space indent off the shallowest depth - reflection prompt teaches agent-curated structure with a soft 300-line per-section cap and explicit guidance to restructure flat legacy lists cuts 8 helpers (ensureSections, stripLearningsToc, assembleFile, buildTocBlock, parseHeadings, buildSectionScaffold, hasAnyTaxonomyHeading, LEARNINGS_SECTIONS) and the canonicalSeed round-trip dance. action seedLearningsFile is now { path } only; main.ts byte-compares the trimmed read-back against (current ?? "").trim() to gate the persist PATCH. truncateAtLineBoundary kept for safety. new tests: - test/learningsToc.test.ts (11 parser cases incl. fenced-code, blockquote, arbitrary h1-h6 nesting, startLine-points-at-heading invariant) - action/utils/learningsTocRender.test.ts (7 renderer cases) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>	2026-05-13 20:14:26 +00:00
Colin McDonnell	d04c1ca3da	action: bump to 0.1.6 v0.1.6	2026-05-13 18:23:45 +00:00
Colin McDonnell	ae976e7159	parallel tool execution: enable opencode batch + nudge agents to parallelize (#719) ... opencode: opt into `experimental.batch_tool` (anomalyco/opencode#2983) so the `batch` tool registers and the model can bundle 1-25 independent calls into one round trip. edit calls are excluded upstream. instructions.ts: add a "Parallel tool execution" section to the SYSTEM Workflow block, agent-specialized via ctx.agentId. uses Anthropic's canonical wording ("invoke all relevant tools simultaneously...") so Claude reliably emits multiple tool_use blocks per message; tells OpenCode about the new `batch` affordance. verified end-to-end against haiku-class models (sonnet for claude, default for opencode) with a "read 3 files and report first lines" fixture. results: - opencode used `batch` with 3 nested reads AND emitted 3 native parallel read calls in the same assistant turn - claude went from 3 serial turns (1 read each) to 1 message with 3 parallel Read tool_use blocks	2026-05-13 18:05:39 +00:00
Colin McDonnell	5aabd1e4a9	fix(action): cap subprocess stdout/stderr retention to prevent RangeError crashes (#680) (#715) ... * fix(action): cap subprocess stdout/stderr retention to prevent RangeError crashes (#680) unbounded `stdoutBuffer += chunk` / `stderrBuffer += chunk` in `action/utils/subprocess.ts` previously crashed the wrapper with `RangeError: Invalid string length` once V8's ~1 GiB kMaxLength was breached on long-lived agent runs. multi-lens opencode Reviews on large monorepos (e.g. tambo-ai/buildy) hit this consistently — 23 runs in the last 24h, 100% of Review-mode hard failures on that repo. - add `retain: "tail" | "none"` to SpawnOptions, defaulting to "tail" with an 8 MiB cap. tail-mode prepends a `... [N MiB truncated] ...` sentinel so downstream consumers can detect truncation. - export `TailBuffer` helper for callers that need the same bounded accumulator semantics at their own layer. - wrap stream `data` listeners in try/catch as defense in depth — any synchronous throw inside a stream handler is otherwise fatal. - opencode + claude pass `retain: "none"` (they drain via onStdout / onStderr) and switch their own `output` accumulators to TailBuffer. their error paths read the agent-layer bounded mirrors instead of the now-empty `result.stdout` / `result.stderr`. - add `failure:string-length-overflow` heuristic to scripts/analyze-logs.ts so post-fix recurrences are visible at a glance instead of bucketing into `failure:unknown`. - regression tests cover >1 MiB stderr without crash, retain:"none" contract, and TailBuffer truncation semantics. * fix: avoid TS parameter property syntax in TailBuffer for strip-only node loader * address review: clarify try/catch scope + lock retain default to "tail" - the original comment claimed the try/catch caught "any synchronous throw" in the data listener, but `options.onStdout?.(chunk)` returns a Promise in the agent callers (claude.ts:569, opencode.ts:933) — a throw inside an async user callback surfaces as an unhandled Promise rejection, not a synchronous exception. reword to describe the actual protection: defense-in-depth for synchronous throws in the listener body, which is exactly the shape of the original RangeError on `+= chunk`. - add a test that locks `retain` default to "tail" by spawning without the option and asserting `result.stderr` is non-empty. a future refactor that flipped the default to "none" would silently break gitAuth, package installs, and lifecycle hooks that read result.stderr for failure messages, and the rest of the suite wouldn't catch it.	2026-05-13 17:54:28 +00:00
Colin McDonnell	60cc8772a6	fix(log-audit): kill 404 noise from /api/github/installation-token at source (#693) (#708) ... * fix(log-audit): kill 404 noise from `/api/github/installation-token` at source (#693) Closes #693. Issue diagnosed a surface symptom (`log.error` on expected 404s) but missed the actual root causes. Investigation revealed two distinct populations producing identical 3-call 404 bursts: 1. **Fork-CI on `pullfrog/pullfrog`**: `test-token.yml` and `trigger-sync.yml` ship with `on: push: main`, so every fork inherits them and 404s our token endpoint on first push. Self-inflicted noise that scales with fork count. 2. **Real users hitting the full action without installing the App**: `/api/repo/.../run-context` uses the caller's `GITHUB_TOKEN` to read the repo from GitHub and then unconditionally lazy-provisions Account+Repo rows via `fetchOrCreateRepo`, even when the App isn't installed. Generates phantom DB rows and false `new account created` team@ alerts. (Confirmed via Prisma: `ezcorp-org` has an Account row with `installerLogin: null`, never installed our App.) Both populations then trip the client retry loop in `acquireTokenViaOIDC`, which matched `"Token exchange failed"` and retried 3× on terminal 4xx — tripling log volume and wasting CI time. ## Changes - `action/.github/workflows/{test-token,trigger-sync}.yml`: gate jobs with `if: github.repository == 'pullfrog/pullfrog'`. Forks inherit the files but the jobs no-op. - `app/api/repo/[owner]/[repo]/run-context/route.ts`: call `getRepoInstallation` first; return 404 with install URL if the App isn't installed, before any DB writes or GitHub repo fetch. - `action/utils/github.ts`: introduce `TokenExchangeError` for non-2xx server responses; `acquireNewToken` no longer retries it. Retry now fires only on genuine network/timeout failures. 404 surfaces a user-actionable error pointing at the install URL. - `app/api/github/installation-token/route.ts`: move `log.error` inside the 500 branch only. 404 branch is silent (expected user-state) and returns the same install URL message for consistency. ## Effect - Better Stack `level=error` lines from this path: 6/day → 0. - Failed user-trial CI time: 3 wasted token requests → 1. - User-facing error: opaque `Token exchange failed: 404` → actionable install URL. - No more phantom Account rows from never-installed callers. Skipped per design discussion: phantom-account cleanup (conservative — stop the bleed, leave history), `AGENTS.md` rule (overgeneralized). * review: address oracle leak + per-env install URL + retryable 5xx Addresses pullfrog[bot] (IMPORTANT) and Copilot review findings on #708: - **Install-status oracle in `run-context`** [pullfrog, Copilot]: `getRepoInstallation` runs with our App's JWT, *before* the caller's bearer token is validated against the repo. Pre-PR the route was uniformly bad-token-shaped; the new install-specific 404 turned it into an unauthenticated oracle distinguishing "Pullfrog installed here" from "not installed". Collapsed the 404 message to match the outer catch's ambiguous "repository not found or token lacks access". Legit runners still get the actionable install URL from `/api/github/installation-token`, which IS gated by OIDC. - **Hardcoded `github.com/apps/pullfrog`** [Copilot]: server-side `installation-token` now uses `GITHUB_APP_INSTALL_URL` from `app/globals.ts`, so dev/staging deployments with a different `GITHUB_APP_SLUG` direct users to the correct app. Action-side echoes the server's `error` body when present (single source of truth) and falls back to a generic message only if the body isn't JSON. - **Transient 5xx/429 made terminal** [Copilot]: `shouldRetry` now returns `true` for `TokenExchangeError` with `status >= 500` or `status === 429`. 4xx remains terminal (the actual #693 fix). Real outages no longer fail the workflow immediately. - **Stale comment** [pullfrog, Copilot]: reworded the comment at `installation-token/route.ts:141` to reflect the new retry policy ("the action surfaces this once (no retry)" instead of "the action retries on this"). * review: restore caller-token-first auth in run-context Pre-PR, `getEnrichedRepo({owner, repo, token})` used the caller's token as the auth boundary — `getRepo({token})` succeeding was the proof-of-access check. My initial install-gate inverted the order and ran the App-credentialed `getRepoInstallation` first, which is how it became: - an install-status oracle (pullfrog bot, addressed previously by matching the outer-catch wording), and - an outbound amplifier against our App JWT for arbitrary `owner/repo` (pullfrog bot, this commit). Reordered so `getRepo({token})` runs first. Garbage / unauthorized bearers get rejected by github (mapped to 403 by the outer catch) before any App-credentialed call fires. `getRepo` is cached 5min, so `getEnrichedRepo` below remains a free re-hit.	2026-05-13 17:47:13 +00:00
Colin McDonnell	4260984257	attribute claude subagent log lines + per-session thinking timer; tighten lens calibration (#700) ... * attribute claude subagent log lines + per-session thinking timer; tighten lens calibration three orthogonal fixes diagnosed from the 10m PR-699 review run: 1. wire SessionLabeler into the Claude Code harness. claude-agent-sdk stamps every Assistant/User/System message with session_id and a non-null parent_tool_use_id when emitted from a subagent context, so the same FIFO labeler the OpenCode harness uses works here too. parallel reviewfrog dispatches now log with [lens:correctness] / [lens:operational-readiness] / etc. prefixes instead of being indistinguishable from the orchestrator. matches both "Task" and "Agent" tool names per the v2.1.63 rename. 2. one ThinkingTimer per session. the global timer treated cross-session interleaving (parent thinks → child tool_call, child returns → parent dispatches next) as parent thinking time, so individual "thought for Xs" numbers were untrustworthy. each session now owns its own timer and prefixes its own log line. 3. tighten the Review/IncrementalReview lens-add discipline. PR-699 triggered 4 lenses on a typical refactor (no auth/billing/schema) when the prompt's own calibration says 2-3 is typical; the research-validated lens went deep on Resend idempotency window + prisma updateMany lost-updates without either being load-bearing. adds an explicit "name the failure mode this lens would catch that the diff plausibly introduces" bar, and tightens research-validated specifically: only when correctness depends on the third-party contract, not when the API is merely used. side benefits from #1: subagents' TodoWrite events no longer clobber the orchestrator's progress comment; subagent text no longer overwrites finalOutput; system-event handler safely routes through eventLabel even though SDK only emits system:init for the top-level query today. * fix node strip-only mode: declare formatLine as field, not parameter property * key claude subagent labels by parent_tool_use_id, not session_id claude-agent-sdk runs subagents inside the orchestrator's session — they share session_id — and stamps subagent messages with parent_tool_use_id pointing at the Agent tool_use that spawned them. e2e on PR-700 with preview-700-claude-labeling#1 confirmed the original session_id-keyed wiring never differentiated subagent activity (only the dispatch line got [lens:correctness] in the log; the subagent's reads, writes, and todos all rendered as orchestrator). extend SessionLabeler so labelFor accepts an optional parent_tool_use_id and short-circuits to a direct map keyed by Agent tool_use id when set. recordTaskDispatch optionally takes the Agent tool_use id (block.id at dispatch time) and binds it. orchestrator events keep flowing through the sessionID/FIFO path unchanged so opencode wiring is untouched. * drop weak timer test that asserted only field isolation per pullfrog review on PR-700: the 'two timers do not bleed timestamps' test only verified that two ThinkingTimer instances have separate private fields, which has always been true. doesn't earn its keep — the per-session behavior is exercised by integration through claude.ts + opencode.ts.	2026-05-13 15:28:08 +00:00
Colin McDonnell	d5f881e9fc	action: trim sensitive env values before GitHub Actions log masking (#698) ... * action: trim sensitive env values before GitHub Actions log masking GitHub Actions' log masking is line-based: a secret value containing a newline only registers the first line as a mask, leaving the remainder exposed verbatim in logs. A trailing newline copied from a terminal into a GitHub Actions secret (e.g. ANTHROPIC_API_KEY) was enough to leak "a large part of the key" in run logs (pullfrog/pullfrog#41). normalizeEnv now trims leading/trailing whitespace from any value whose key matches the sensitive name pattern, masks the cleaned value, and warns when whitespace was stripped so the user notices the source. sanitizeSecret is reused for dbSecrets injection in main.ts. The three secret-store PUT/POST routes also trim values defensively, matching the existing name.trim() pattern. Real multi-line secrets are not used in practice — even GITHUB_PRIVATE_KEY PEMs are stored single-line with escaped \n and unescaped at the point of use — so a straight trim() is safe. * action: address review — use core.setSecret for masking, don't zero whitespace-only Pullfrog's review of #698 caught two real issues in the original fix: 1. `console.log(\`::add-mask::\${trimmed}\`)` doesn't escape \r/\n. If a value survives trim with an embedded newline (PEMs, kubeconfigs, JSON), the runner only registers the first line as a mask and the rest leaks. `core.setSecret(trimmed)` routes through @actions/core which percent-encodes \r/\n so the runner V2 parser decodes back to the full value and registers every non-empty line as a separate mask. Removes the load-bearing "no embedded newlines" invariant from the fix. 2. Whitespace-only sensitive values silently became "". Downstream truthy checks would flip from "set" to "missing" with no log. Now sanitizeSecret returns null in that case and callers skip the process.env write, surfacing a clear missing-key error instead. Tests rewritten to assert process.env state directly — no stdout spies. Masking correctness is delegated to @actions/core (trusted dependency).	2026-05-13 15:27:13 +00:00
Colin McDonnell	1dc53043a6	chore: bump action to 0.1.5 v0.1.5	2026-05-13 04:56:01 +00:00
Colin McDonnell	076e5a17b5	default Claude Code effort to high ... max effort burns roughly 2x the wall time per turn for marginal quality gain. high is the model's tuned default ('equivalent to not setting the parameter' per Anthropic docs). full-send can be reintroduced as an opt-in per-run override later if needed.	2026-05-13 04:49:07 +00:00
Colin McDonnell	d5d8a0d7ac	fix(#691): drop opencode/gpt-5-nano + opencode/mimo-v2-pro-free (not actually keyless on Zen) (#695) ... * remove opencode/gpt-5-nano and opencode/mimo-v2-pro-free from catalog #7 delete aliases. both were listed as `isFree: true, envVars: []` but neither is keyless on opencode zen, producing a hard-fail `UnknownError: Model not found: opencode/<id>` on every run without an opencode_api_key. fixes pullfrog/app#691 (5 runs across 3 repos, 100% failure rate in the last 24h). root cause: opencode's provider gate (`packages/opencode/src/provider/provider.ts` `opencode:` loader) keeps a zen model only when models.dev reports `cost.input === 0` for it, then signs requests with `apiKey: "public"`. paid zen models get deleted from the autoloaded set and opencode surfaces the deletion as "model not found". - `opencode/gpt-5-nano`: models.dev reports `cost: {input: 0.05, output: 0.4, cache_read: 0.005}`. paid → requires `OPENCODE_API_KEY`. - `opencode/mimo-v2-pro-free`: free on models.dev but not in `https://opencode.ai/zen/v1/models` — zen never served it, so even the public-key path fails. remaining free aliases (`opencode/big-pickle`, `opencode/minimax-m2.5-free`) both pass both checks (cost.input === 0 in models.dev AND present in zen's served list) and continue to work without a key — verified against the opencode source. callers swept: `action/utils/apiKeys.test.ts`, `action/models.test.ts`, `action/test/list-aliases.ts`, `action/test/model-smoke.ts`, `components/ModelSelector.tsx` (`modelIdToUpstream`), `wiki/model-resolution.md`, `wiki/models-catalog.md`. wrote up the free-zen verification rule in models-catalog so the next maintainer can sanity-check both conditions before adding any `isFree` alias. users with a stored `opencode/gpt-5-nano` or `opencode/mimo-v2-pro-free` will now fall through `resolveCliModel → undefined` into the auto-select path — a strict improvement over today's hard fail. no DB migration needed; the slugs are simply unknown and treated like any other unrecognized stored value. * rework: keep mimo deprecated, demote gpt-5-nano to paid, add free-zen invariants revised approach after the first commit over-corrected. mimo was never broken at runtime — `fallback: "opencode/big-pickle"` already routes stored values through to a real free model before any zen call. the literal `opencode/mimo-v2-pro-free` being absent from zen's served list is irrelevant because `resolveCliModel` walks the chain first. restoring it as-is. the actual bug was `opencode/gpt-5-nano`: marked `isFree: true, envVars: []` but `models.dev` reports `cost: {input: 0.05, output: 0.4}` on the opencode provider, so opencode's keyless gate (`packages/opencode/src/provider/provider.ts` `opencode:`) deletes it when `OPENCODE_API_KEY` is missing and the run hard-fails with `UnknownError: Model not found: opencode/gpt-5-nano`. demoting it to a regular paid zen alias (drop `isFree`/`envVars: []`, add `openRouterResolve: "openrouter/openai/gpt-5-nano"` — verified to exist on openrouter at the same price). users without `OPENCODE_API_KEY` now get our explicit "no API key found" error pointing at the secrets page instead of opencode's cryptic upstream error. confirmed via `https://opencode.ai/zen/v1/models` that zen serves no free GPT variants, so there's no cheaper-than-`gpt-mini` free option to suggest in its place. CI gap analysis (why this slipped through): - `models-catalog.main.test.ts` only checked existence + `status !== "deprecated"` on models.dev. paid-model-marked-free regressions and zen-served-list drift both passed. - `models-live` (`model-smoke.ts`) runs with `OPENCODE_API_KEY` in env, so the keyless deletion gate never fires. `gpt-5-nano` returned "OK" in CI even though end users hit a hard fail. - `model-smoke.ts` walks the fallback chain, so mimo would have been smoked as big-pickle anyway — the dead resolve target was never exercised directly. (this is the right design; the gap is at the catalog layer, not the smoke layer.) new tests: - PR-blocking, static (`action/test/models.test.ts`, `isFree invariants`): every `isFree` alias must live under `opencode`, have `envVars: []`, omit `openRouterResolve`, AND have a fallback chain whose terminal alias is also `isFree` (catches "deprecate a free alias to a paid target" — the worst silent-charge regression). - main-only, network (`action/test/models-catalog.main.test.ts`, `opencode Zen served list`): every alias whose terminal-fallback resolve is `opencode/*` must appear in `https://opencode.ai/zen/v1/models`. catches zen dropping a model from its served list. - main-only, network (same file, `isFree models.dev cost`): every `isFree` alias's terminal-fallback resolve must have `cost.input === 0` in the `opencode` provider block on `models.dev`. would have caught `gpt-5-nano` at the next models-bump run. both network tests dedupe on terminal resolve, so deprecated aliases sharing a target aren't double-counted. `pnpm vitest run`: 113 static tests pass. `pnpm test:catalog`: 142 network tests pass against the live `models.dev`, `openrouter.ai`, and `opencode.ai/zen/v1/models` endpoints. wiki/models-catalog.md: rewrote the new "Free-Zen aliases need Zen-side verification" section to (a) describe the two conditions, (b) note that a fallback to an isFree alias is the legitimate escape hatch (mimo's pattern), and (c) point at the three tests by name so the next maintainer can find the enforcement surface. wiki/model-resolution.md points at the new section. * make gpt-5-nano a deprecated free alias falling back to big-pickle revising the previous "demote to paid" approach. the user-facing ergonomics are cleaner: anyone who picked gpt-5-nano under the "Free" badge gets transparent-upgraded to a real free model (big-pickle) instead of suddenly being asked to set OPENCODE_API_KEY. matches the existing mimo pattern exactly. the dropdown already filters `!a.fallback`, so the slug disappears from the picker on its own and the trigger renders it as "Big Pickle" via `resolveDisplayAlias`. no other catalog or test surface changes — the isFree invariants and the main-only zen/cost checks still pass (gpt-5-nano's terminal is now big-pickle, which is both isFree and zero-cost on models.dev, deduping with big-pickle's own row in both network tests). * revise: keep gpt-5-nano as paid alias, backfill affected DB rows instead dropping the deprecated-alias approach. `opencode/gpt-5-nano` is a legitimate cheap paid model people may want with BYOK (`OPENCODE_API_KEY`) — giving it `fallback: "opencode/big-pickle"` would foreclose that for everyone going forward. correct fix is two parts: (a) reclassify in the catalog as a regular paid OpenCode alias: - drop `isFree: true` and `envVars: []` so the local validator demands `OPENCODE_API_KEY` - add `openRouterResolve: "openrouter/openai/gpt-5-nano"` to satisfy the completeness test and route BYOK-via-OpenRouter users - no `fallback` — slug stays visible in the picker as a paid option (b) one-shot DB backfill of provably-affected repos (`scripts/backfill-gpt5-nano-affected.ts`). scope: - `Repo.model = "opencode/gpt-5-nano"` - AND at least one `WorkflowRun` with `inputTokens IS NULL` (evidence of an attempted run that didn't get past the model-init gate) skipped intentionally: - repos whose runs have `inputTokens > 0` — they have a key, gpt-5- nano works for them - repos with zero WorkflowRun rows — never dispatched; touching them would be presumptuous - `LearningsRevision.model` — audit trail of which model authored a revision, rewriting it would falsify history ran against .env.prod: 2 repos stored the slug; 1 was provably affected (sodown4thecause/seobot, 5/5 zero-token runs — matches #691's 3 failed runs from this repo plus 2 outside the 24h audit window). 1 was an internal test account that never dispatched (left as-is). applied: 1 row updated. confirmed idempotent on re-run. the other two repos in #691 (Nantiee/ALTA-breast-pump-tool, keksiqc/ansible-setup-linux) don't store the slug in `Repo.model`; their failed dispatches passed the model inline in the `workflow_dispatch` `prompt` payload, so the catalog fix alone (no longer offering it as free) is what helps them. tests: - models.test.ts: `getModelEnvVars("opencode/gpt-5-nano")` now returns `["OPENCODE_API_KEY"]`, moved into the keyed-model group - apiKeys.test.ts: added "throws without OPENCODE_API_KEY" case - isFree invariants from the previous commit still pass — gpt-5-nano no longer triggers them since it's no longer isFree - main-only catalog tests still pass (gpt-5-nano served by Zen, just paid; no isFree cost check applies) * docs: drop stale GPT Nano + MiMo V2 Pro from free-tier lists addressing pullfrog auto-review feedback on #695. three mintlify pages still advertised both as keyless after the catalog pivot, which now makes the docs affirmatively wrong rather than merely stale: - gpt nano is paid in the catalog (no `isFree`, inherits `OPENCODE_API_KEY`); a user following the docs would hit the same "missing API key" failure that's described 4 lines below in `docs/keys.mdx`. - mimo v2 pro is hidden from the picker (`fallback` triggers `ModelSelector`'s `!a.fallback` filter); the alias only exists for legacy stored-value resolution. a user reading the docs cannot actually pick it. surviving picker-visible free set: Big Pickle and MiniMax M2.5. - `docs/keys.mdx`: drop both bullets from the "Free models" list - `docs/billing.mdx`: drop both bullets from the "Free models" list - `docs/getting-started.mdx`: collapse the inline mention from a 4-model list to "Big Pickle and MiniMax M2.5" * address third review: picker grouping + backfill classifier honesty i had not pulled the third pullfrog review (`02:17:28Z`) when i declared reviews triaged after the docs sweep — the fourth review flagged that three findings remained pending. addressing them now. 1. picker grouping for now-selectable paid gpt-5-nano. when i removed `"gpt-5-nano": "OpenAI"` from `modelIdToUpstream` in the previous pivot-to-paid commit, i mistook it for dead code. it's not — the map IS consulted for paid opencode aliases via `groupByUpstream → getUpstreamLabel` inside the OpenCode submenu's `renderSubContent`. without the entry, `gpt-5-nano` falls back to `getProviderDisplayName("opencode")` = "OpenCode" and gets dropped into its own sub-header instead of joining opencode/gpt, opencode/gpt-pro, opencode/gpt-mini under the "OpenAI" upstream group. re-added with an explanatory comment so the next refactor doesn't make the same mistake. 2. JSDoc / code mismatch in `scripts/backfill-gpt5-nano-affected.ts`. the JSDoc said "at least one `WorkflowRun` with `inputTokens IS NULL`" but the code is `no WorkflowRun has inputTokens > 0` — a strictly broader filter (catches `null` AND `0`). rewrote the scope block to describe what the code actually does, with the operative classifier spelled out: "a billable run with `inputTokens > 0` is proof the agent successfully reached and called the model". 3. classifier breadth (raised in the same review). honest answer: the "no positive-token run" filter IS a heuristic — a repo whose only dispatches happened to fail or cancel for unrelated reasons would get false-positive-classified A. for THIS one-shot population (2 repos, 1 with 5/5 zero-token runs — strong systematic-failure signal) the heuristic was good enough and the dry-run inspection confirmed before APPLY. for any larger reuse of this pattern, you need to cross-reference the runtime error string (`UnknownError: Model not found: opencode/gpt-5-nano`) from GitHub Actions logs or Better Stack — that error doesn't live on `WorkflowRun` rows. added a "Classifier limitations" section to the JSDoc making this explicit. nothing about the actual applied backfill changes — the prod write (1 repo: sodown4thecause/seobot → opencode/big-pickle) is unchanged and re-running the script remains idempotent.	2026-05-13 02:43:08 +00:00
Colin McDonnell	159389fad2	fix(mcp): sanitize for gemini when model is unresolved (#697) ... * fix(mcp): sanitize for gemini when model is unresolved isGeminiRouted() previously required the effective model string to contain "gemini" — but when payload.model="auto" (or any unresolved slug) reaches addTools(), `effective` is the literal "auto", which doesn't match. opencode then auto-selects gemini *after* the MCP server has registered raw arktype schemas, and every tool turn dies on `function_declarations[*].properties[*].any_of[*].enum: only allowed for STRING type`. widen the gate: any unresolved specifier (undefined / "auto" / a slug without a `provider/` prefix) is treated as gemini-routed and sanitized. the transforms are universally compatible normalizations so the false-positive cost is negligible. tighten case 3 to preserve `description` so the only lossy path no longer drops operator-facing context. fixes #676. * revert case-3 description preservation per pullfrog review on #697: keeping `description` as a peer of `anyOf`/`oneOf` directly contradicts the file's own header (lines 19-21) and the upstream opencode #14659 rationale that gates this sanitizer — gemini requires anyOf to be the ONLY field on a schema node, sibling keywords trigger `anyOf must be the only field in a schema node`. the change was speculative scope creep with no evidence, and would silently re-introduce a different gemini failure for any future schema using `.describe().or(...)`. the bug fix for #676 doesn't need it (arktype doesn't emit non-collapsible anyOf for current tool schemas).	2026-05-13 02:31:59 +00:00
Colin McDonnell	43bb14bf87	action: strip Content-Type on body-less apiFetch requests (#692) (#694) ... * action: strip Content-Type on body-less apiFetch requests (#692) Vercel's Next.js lambda adapter (Next 16.1.x) attempts to decode a request body when Content-Type is set and throws `SyntaxError: Unexpected end of data` before delegating to the route handler, returning a 500. Hit /run-context exclusively because it was the only body-less GET that sent `Content-Type: application/json`. - Drop `Content-Type: application/json` from the GET in `action/utils/runContext.ts` (meaningless on a body-less request). - Defensively strip any `content-type` header in `action/utils/apiFetch.ts` when no body is present so future callers can't reintroduce this. * apiFetch: soften comment — empirical observation, RFC 9110 §8.3 framing	2026-05-13 02:03:24 +00:00
Colin McDonnell	d8f825034f	billing: $10 signup credit + lazy claim modal; disable welcome credit promo (#674) ... * billing: $10 signup credit + lazy claim modal; disable welcome credit promo Adds a per-Account $10 Router signup credit granted on first Router-tab mount via a new admin-gated POST /api/account/[owner]/signup-credit/claim. The endpoint is idempotent — the inserted CreditGrant row IS the dedup state, so subsequent calls return granted:false. Client SignupCreditModal fires the POST on mount (only when modelAccessMode === "router") and opens a celebratory dialog when granted:true. Disables the legacy welcome credit ($10 on first card add) via a new WELCOME_CREDIT_PROMO_ACTIVE = false flag in utils/stripe.ts. Code path stays intact — flip the flag to revive. Strips the now-untruthful "$10 on enabling billing" copy from BillingCard, EnableRouterPrompt, triggerWorkflow paywall comment, action router_requires_card summary, email snippet, billing/pricing docs and wiki. Cuts WELCOME_CREDIT_CENTS from 2000 to 1000 to reflect the lower amount that would land if the flag is ever re-enabled. Adds "signup" reason mapping to BillingCard wallet history. Verified end-to-end against dev: admin+Router fires modal, admin+BYOK gate-blocks mount, BYOK→Router transition fires modal on click, member and collaborator paths skip the mount entirely, reload after grant is idempotent. Wallet history shows "Router signup credit +$10.00". * billing: address PR review (race fix, copy sweep, modal retry) Correctness: - Add @@unique([accountId, reason]) on CreditGrant + migration. The prior check-then-insert pattern in /signup-credit/claim and finalizeCheckoutSession raced at READ COMMITTED — two concurrent admin tabs could land two grants of the same reason on a fresh account ($10 each). Both write sites now rely on the unique index for dedup (P2002 = "already granted") and route updated to catch P2002 cleanly. Verified zero existing duplicates in prod before migration. - Add log.info on signup grant insert so a successful grant has any chance of being caught by ops monitoring. - Add retry: 2 with backoff to the claim mutation. Endpoint is idempotent so a server-side success that lost its response cleanly returns granted:false on retry. Public copy that still advertised the (now-deleted) $20 welcome credit: - app/page.tsx landing pricing card - emails/announceBilling.ts broadcast template - docs/keys.mdx BYOK note - components/AgentSettings.tsx Router-without-billing warning - utils/stripe.ts finalizeCheckoutSession JSDoc - utils/email/snippets.ts ROUTER_CREDIT_PS_HTML JSDoc Wiki staleness sweep: - wiki/billing.md TOC, mermaid diagram (signup edge added; welcome marked dormant), test coverage list, key modules section, no-card wallet narrative - wiki/pricing.md welcome-credit drawdown reference - Rewrote my own internally-inconsistent dormancy paragraph to be honest about the $20-historical / $10-on-revival framing. Trivia: - ModelAccessCard JSX comment had a literal \\u2192 instead of →. * billing: address PR review round 2 - Replace try/catch P2002 inside finalizeCheckoutSession's prisma.$transaction with createMany skipDuplicates. The previous form is broken on Postgres: a unique-violation poisons the surrounding TX, so the catch block returns cleanly but the outer commit fails and the account.update (stripeCustomerId) silently rolls back too. Currently armed only behind the dormant welcome- credit flag, but would have broken billing enablement the moment the flag flipped. createMany skipDuplicates yields a single ON CONFLICT DO NOTHING statement that returns count: 0 cleanly without aborting the TX. - Apply the same createMany skipDuplicates pattern to the signup-credit route too — drops the exception-as-control-flow Prisma namespace import and is more uniform with the welcome path. - Drop the now-orphaned credit_grants_accountId_idx in the same migration. The schema removed @@index([accountId]) when @@unique([accountId, reason]) was added (covered by the leftmost prefix), but the migration only added the unique index, leaving prod drifted. * billing: fix stale finalizeCheckoutSession JSDoc The function-level JSDoc still described the abandoned try/catch P2002 mechanism after switching to createMany skipDuplicates. The inline comment + code now agree on the new ON CONFLICT DO NOTHING shape. * billing: decouple first-card alert, drop vestigial billing field, fix modal cents; sync copy * docs+homepage: align Router credit copy with signup claim (no card-on-add carrot) * homepage: add pricing screenshot and pay-as-you-go promo line * billing: fix once-per-lifetime misframe on first-card alert * billing: suppress signup credit for prior welcome-credit recipients * billing: drop bogus '1000 users' cap; invalidate billing on signup-credit settle	2026-05-12 23:47:52 +00:00
Colin McDonnell	f0805b78f5	learnings: surface persist failures as warnings, not debug ... `persistLearnings` only emitted `log.info("» learnings updated")` on success; every failure path (non-2xx, fetch throw, 10s timeout) was `log.debug`, which is hidden unless `ACTIONS_RUNNER_DEBUG=true`. Survey of recent runs caught at least one case where the agent definitively edited the tmpfile but no DB row was written and no warning surfaced. Promote both failure paths to `log.warning` so dropped agent work is visible in CI logs. The unchanged-from-seed short-circuit stays at debug — that's a genuine no-op.	2026-05-11 23:51:46 +00:00
Colin McDonnell	e20b4d5515	action: bump to 0.1.4 v0.1.4	2026-05-11 23:22:47 +00:00
David Blass	8c6cd2bda2	cancel + restart workflow run when @pullfrog mention is edited (#612) ... * cancel + restart workflow run when @pullfrog mention is edited - add `WorkflowRun.triggeringCommentId` (BigInt?, indexed) so the webhook handler can find the run that was fired by a given comment - thread `triggeringCommentId` through `reserveRun` / `triggerWorkflow` - factor `dispatchMentionRun` out of `issue_comment_created` so the same shape is reused on edit - replace the `issue_comment_edited` stub: re-evaluates the trigger gate, cancels prior runs (`octokit.rest.actions.cancelWorkflowRun` + DB status='cancelled'), then re-dispatches with a `previousRunsNote` appended to `eventInstructions` so the agent acknowledges the prior run/PR/artifacts in its summary - if the edit removes `@pullfrog`, cancel only (no restart) Co-authored-by: Cursor <cursoragent@cursor.com> * thread previousRunsNote via dedicated payload field user prompt has precedence over eventInstructions, so stuffing the prior-runs note into eventInstructions made it vanish whenever the trigger comment contained an @pullfrog mention (which is always for the edit path). pass it as its own payload field and render it alongside the user's task so the agent actually sees it. * delete cancelled run's progress comment on edit-restart so the issue thread doesn't accumulate "This run was cancelled" stubs on every edit. only deletes for runs we actively cancel; runs that were already terminal (e.g. completed before the edit) keep their summary comment in the thread, and `previousRunsNote` links to it so the new agent can reference prior work. post-cleanup is race-safe: the action's `validateStuckProgressComment` swallows the 404 from the deleted comment and exits cleanly, so the old run's post step cannot clobber the new run's leaping comment. Co-authored-by: Cursor <cursoragent@cursor.com> * also cancel + delete progress comment when triggering comment is deleted mirrors the edit-removes-@pullfrog path: when an @pullfrog comment that fired a run is hard-deleted, look up any prior runs by triggeringCommentId, GH-cancel running ones, and delete their leaping progress comments. skips trigger-gate re-eval (we're tearing down a run, not firing one) and performs no restart. reuses the existing cancelRunsForTriggeringComment helper; the returned previousRunsNote is discarded since no dispatch follows. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: move cancellation before trigger gate in issue_comment_edited cancelRunsForTriggeringComment now runs before the triggerEnabled check, so edits that remove @pullfrog still cancel in-flight runs even when the repo mention trigger is currently disabled (e.g. for non-collaborators). * anneal: scope cancel updates per-row + simplify edit gate - replace blanket updateMany on (triggeringCommentId, repoId) with per-row, status-guarded updates so a parallel handler's freshly-reserved run cannot be clobbered into cancelled by a racing edit delivery. - drop wasMention/isMention early-break in issue_comment_edited; always run cancelRunsForTriggeringComment (DB is the canonical "did this comment ever trigger a run" source). closes the missing-changes.body.from edge and lets us tear down a still-running prior run even if the admin disabled the mention trigger mid-flight. - buildPreviousRunsNote returns undefined (not "") when no link lines materialize. - doc cleanups + wiki/modes.md addendum noting issue_comment_edited / _deleted now drive cancel + restart. Co-authored-by: Cursor <cursoragent@cursor.com> * address review feedback on cancel/restart semantics - guard workflow_run.completed update against status='cancelled' so a successful-but-uncancellable GH Actions job can't resurrect a cancelled row (and re-bill it) via the completed webhook. - bucket only status='completed' runs into `preserved` in cancelRunsForTriggeringComment; cancelled/failed prior runs have stubs as their progress comment, not summaries worth referencing. - emit previousRunsNote for the runId-null cancel case so the restarted agent always knows when it's superseding a prior dispatch. - drop the agent-forbidden `gh pr list` hint and soften 'was cancelled' to 'was signalled to cancel' in the note body. - post a fallback comment when the edit-path dispatch fails (prior run already torn down and progress comment already deleted). - symmetrize the delete-handler's pullfrog guard with the edit handler (key off hook.comment.user, not hook.sender). - trim misleading comments on the per-row DB update guard. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>	2026-05-11 23:20:44 +00:00
Colin McDonnell	e4d0fc7e3d	biome: ignore .logs/ (was matching only logs/) v0.1.3	2026-05-11 23:06:33 +00:00
Colin McDonnell	a4a5010441	gemini-3: default thinkingLevel to medium + restrict eager prep to frozen install (#663) ... * gemini-3: default thinkingLevel to medium + don't `npm ci` without a lockfile upstream opencode hardcodes `thinkingLevel: "high"` for every gemini-3 model on the direct google SDK (see `packages/opencode/src/provider/transform.ts` `options()`). that added 30-60s of pre-tool-call TTFT and 5-46s of post-tool jabber per turn, which is overkill for the tool-routing decisions that dominate agentic loops — and the variance caused the `providers-live (google/gemini-pro)` smoke job to time out at 4 minutes (see job 75405504847 on run 25684766415). three changes: - inject `provider.google.models.<api-id>.options.thinkingConfig.thinkingLevel = "medium"` for the two curated gemini-3 slugs in `buildSecurityConfig`. deep-merges over the upstream default; explicit `--variant high` / user opencode config still wins. flash stays at medium too — low-effort flash is visibly worse and the latency win isn't meaningful (flash is already fast). - bump the `providers-live` harness step from 4 → 6 minutes. the job-level 8-minute cap stays as the upper bound, but gemini's intrinsic TTFT variance was eating most of the 4-minute slack on its own. - in `installNodeDependencies`, pick `frozen` only when a lockfile was actually detected. previously a package.json-only repo (like the smoke fixture's `pullfrog/test-repo`) always triggered `npm ci` and emitted a noisy `EUSAGE` error before falling through. * prep: skip eager install when neither lockfile nor `packageManager` field present the previous commit changed the no-lockfile path from `npm ci` (always errored `EUSAGE`, never wrote any artifact) to a successful `npm install`, which had an unintended side effect: it generated `package-lock.json` in the working tree, tripping the post-run dirty-tree gate. the agent then committed the lockfile and opened a real PR — and in the openai/gpt smoke run on PR #663, the agent overwrote the `SMOKE TEST PASSED` output with the PR URL, failing the smoke validator. a repo with `package.json` but no lockfile and no `packageManager` field has not committed dependency state. eagerly installing produces state the repo doesn't track, which is the dirty-tree problem above. skip the eager install entirely in that case; the agent can opt in via `await_dependency_installation` when it actually needs deps. repos with a lockfile or a `packageManager` field keep the existing frozen-install behavior unchanged. * post-run: suppress dirty-tree gate in non-committing modes (Review / IncrementalReview / Plan) the dirty-tree post-run gate currently fires for every mode and tells the agent to commit and push whatever is in the working tree. that's wrong for modes that complete by submitting a review (`Review` / `IncrementalReview`) or posting a Plan comment (`Plan`) — those modes never touch files as part of their contract, so any tree dirt at end-of-run is incidental tool noise on an ephemeral worktree. nudging the agent to commit it can produce a spurious PR, as seen in the openai/gpt smoke run on PR #663 where a stray `package-lock.json` from `npm install` led the agent to open pullfrog/test-repo#32 and overwrite the smoke output. introduce `NON_COMMITTING_MODES` in `action/modes.ts` and consult it in `collectPostRunIssues`. when the selected mode is read-only, log the suppression for visibility but skip populating `issues.dirtyTree`. modes that legitimately commit (`Build`, `AddressReviews`, `Fix`, `ResolveConflicts`, `Task`) keep the existing nudge. * prep: restore eager frozen-install, drop non-frozen fallback eager dependency prep is non-mutating by contract — it runs before the agent starts and any artifact it leaves in the tree (e.g. a generated `package-lock.json`) trips the dirty-tree post-run gate and can lead the agent to open a spurious PR (seen on the openai/gpt smoke run earlier in this PR). revert the previous skip-when-no-lockfile branch: that was the wrong layer to enforce the invariant. instead, run `frozen` (`npm ci` / `pnpm install --frozen-lockfile` / etc.) unconditionally and drop the `|| install` fallback that could silently mutate the tree when `frozen` is missing. frozen commands fail cleanly without writing artifacts when there's no lockfile, which is exactly the safety contract we want. repos that need a real install must opt in explicitly via a `setup` lifecycle hook. * review nits: single getGitStatus call, tighten gemini-3 override scope comment addresses two inline nits from the PR review: - `collectPostRunIssues` was calling `getGitStatus()` (spawns `git status --porcelain`) in both branches of the mode check. lift the call above the conditional and branch on the result; same behavior, one git invocation. - the JSDoc on `GEMINI_3_DIRECT_API_IDS` said the override applies "across the board," but the constant only covers the two curated slugs in `action/models.ts`. tighten the wording to call out that other gemini-3 ids in models.dev keep the upstream "high" default. skipped the bot's yarn-1 concern after reading yarn 1's `install.js`: `bailout()` (lines 461-465) throws `frozenLockfileError` when `frozenLockfile && (!lockfileClean || missingPatterns.length > 0)`, which fires before `linker.init()` writes node_modules or runs lifecycle scripts. the existing comment's claim that frozen commands fail without artifacts holds for yarn 1 too.	2026-05-11 22:04:19 +00:00
Colin McDonnell	cf94773bf0	modes: make task-list authoring the explicit first step in every mode checklist (#665) ... * modes: make task-list authoring the explicit first step in every mode checklist The system prompt already instructs the agent to author an internal task list at the start of every run (action/utils/instructions.ts:291), but the rule lives several hundred tokens above the agent's first decision point and references the mode's checklist before the agent has it. Compliance is roughly coin-flip across opus runs — PR #610 dead-air for 9m20s was the extreme case; my own #664 e2e runs split 1-for-1 on `todowrite` compliance. Putting the directive *inside* the checklist that `select_mode` returns co-locates instruction with referent at the moment the agent decides what to do next. Same vocabulary as the existing rule (`task list`, agent-agnostic; the harness already maps to `todowrite`/`TodoWrite` per-agent in agents/opencode.ts and agents/claude.ts). The directive is deliberately non-prescriptive about list contents — the agent authors items based on the work it's about to do, not from a hand-shaped template. Touches all 8 built-in modes and the PlanEdit override: - Build / AddressReviews / Review / IncrementalReview / Plan / Fix / ResolveConflicts / Task: inserts `1. **task list**: create your task list for this run as your first action.` and renumbers existing steps. - action/mcp/selectMode.ts: same insertion in the PlanEdit override checklist. - All internal step cross-references shifted +1 (`step 5` → `step 6`, `skip steps 3–4` → `skip steps 4–5`, etc.) across Review, IncrementalReview, and ResolveConflicts modes. One code-comment reference in IncrementalReview's preamble updated to match. Complements #664 (live progress streaming): streaming guarantees the user sees *something* regardless of compliance; this PR raises the ceiling on what they see when the agent does comply (clean numbered checklist tracking through the run instead of just the latest assistant message). 488 action tests pass; typecheck, lint, format all clean. * postRun: fix stale 'step 7' reference missed during +1 renumbering	2026-05-11 21:57:11 +00:00
Colin McDonnell	8e36f76cfa	postrun: thread AgentRunContext through the retry loop instead of repackaging (#652) ... * postrun: thread AgentRunContext through the retry loop instead of repackaging drop the per-gate plumbing in `runPostRunRetryLoop`: the loop now receives `ctx: AgentRunContext` whole and reads `ctx.stopScript` + `ctx.toolState.*` directly. `getUnsubmittedReview` becomes a pure utility in postRun.ts instead of a closure shipped over `AgentRunContext`. `AgentRunContext` loses 4 fields that duplicated `toolState` (`summaryFilePath`, `summarySeed`, `learningsFilePath`, `getUnsubmittedReview`) and gains `toolState: ToolState`. both harness call sites collapse from 11 lines to 7; main.ts deletes the inline closure. `ToolState` and friends move from `action/mcp/server.ts` to `action/toolState.ts` so non-MCP code (agents, post-run loop) stops importing run-state types from the MCP server module. no behavior change. 503/503 tests green. * toolState: relocate `CommentableLines` to break dep cycle with mcp/review `action/toolState.ts` was importing `CommentableLines` from `mcp/review.ts`, which pulled the entire MCP server compile graph (24 files) into any consumer of `ToolState` — including `cf-worker-indexing` via the `pullfrog/internal` re-export chain through `utils/log.ts` → `agents/shared.ts` → `toolState.ts`. that exposed a pre-existing TS error in `mcp/issueEvents.ts` (octokit types resolve differently under cf-worker's `moduleResolution: bundler`). move `CommentableLines` (a small `{ RIGHT: Set<number>; LEFT: Set<number> }` state-shape type) to `toolState.ts` where it's used; re-export from `mcp/review.ts` for back-compat with test and call-site imports. cuts cf-worker's mcp/ compile inclusion from 24 files back to 0. * postRun: drop mock-heavy retry-loop tests; keep pure gate predicate `runPostRunRetryLoop` and `executeStopHook` were covered by ~560 lines of mock-heavy regression-gate tests that stubbed `spawn` / `getGitStatus` and fabricated `AgentRunContext` to drive orchestration paths. per AGENTS.md ("prefer no test over a mock-heavy test that only catches the most obvious form of regression") and the empirical track record — the one real production failure of this code path (#646) was a missing npm release, not a logic bug a unit test could catch — the value-to-ceremony ratio is poor. delete them. keep only the pure predicate: `getUnsubmittedReview(toolState)` is a decision function whose four input conditions have user-visible consequences when wrong. 5 assertions, no mocks, no ctx fabrication. 488 tests still pass. * toolState: import PrepResult from prep/types.ts, not the barrel same dep-cycle class as the previous CommentableLines fix. importing PrepResult from prep/index.ts pulled prep/installNodeDependencies.ts into the Next.js production build's typecheck graph (via pullfrog/internal → utils/log.ts → agents/shared.ts → toolState.ts → prep/index.ts → installNodeDependencies.ts), and Next.js's stricter NODE_ENV-required ProcessEnv shape rejected an existing `env: { PATH: ... }` literal. prep/types.ts is a leaf module with zero imports — re-routing the type import severs the chain. Vercel preview deploy goes from Error → Ready; preview-sync stops racing the deploy.	2026-05-11 18:47:08 +00:00
Colin McDonnell	dee13b160f	console: case-insensitive owner/repo slug resolution (#649) ... * console: case-insensitive owner/repo slug resolution URL slugs may be any case but GitHub treats logins and repo names as case-insensitive (and 301-redirects to canonical case). Internal find/filter sites compared with `===`, so mixed-case slugs (e.g. `/console/Pullfrog`) hard-403'd in resolveOwnerAccess and silently redirected from the per-repo console when currentRepo lookup missed. Lowercase both sides at every slug comparison: resolveOwnerAccess installation lookup, currentRepo lookup in repo + history pages, ConsoleHeader installation/repo lookups, getInstallations personal split, getOrgMembership user/org checks, getInstallationRepos node filter, getUserRole owner-as-collaborator check, and the action runtime's installation-repo access check. Caches keyed by raw input remain case-split across casings; that's fine since both entries resolve to the same canonical GitHub data and TTLs are short. * api: resolve targetAccountId by gh node id getAuthenticatedAccountContext was looking up Account by `name` using the raw URL slug, but `Account.name` is plain String populated from canonical GitHub login. Mixed-case URLs would render the page (since resolveOwnerAccess is now case-insensitive) but every billing/secrets API call would 403 on the find-by-name miss. Resolve by gh_${access.installation.account.node_id} instead — invariant to case-folding and login renames. Same pattern as the sibling owner page route already uses.	2026-05-11 18:45:20 +00:00
Colin McDonnell	ef394277c1	review: synthesize [!NOTE] informational tier with #644 alert judiciousness — 4-callout visual ladder + approved Fix-gate (#653) ... * review: NOTE-tier callout + `actionable` flag to suppress Fix buttons Adds an `actionable` parameter to the `create_pull_request_review` tool (defaults true) so the agent can opt out of the Fix-it/Fix-all/Fix-👍s footer affordance on informational reviews. Threaded through `createAndSubmitWithFooter` so the buttons are omitted when `actionable: false`. Updates `Review` and `IncrementalReview` mode prompts with a 4th tier: `> [!NOTE]` + `actionable: false` for mergeable, FYI-style observations (prior feedback addressed cleanly, minor stale doc reference, etc.). Calibration note: `[!IMPORTANT]`/`[!CAUTION]` are reserved for findings that warrant code changes, because that's what trains users to click Fix. `[!NOTE]` reviews must not carry inline comments — if a point is concrete enough to anchor to a line, upgrade the whole review tier. * review: drop redundant `actionable` flag, key Fix buttons off `approved` `approved` already encodes "this PR is mergeable, nothing for the Fix button to act on" — `actionable` was a second flag carrying the same signal. Drop it from the tool schema and `FooterOpts`; the footer gate stays `if (!opts.approved)` (unchanged from pre-PR behavior, with a new comment documenting the UX rationale). NOTE-tier reviews now use `approved: true` + `> [!NOTE]` body instead of `approved: false` + `actionable: false`. For repos with `prApproveEnabled: false`, the runtime already downgrades APPROVE to COMMENT, so the GitHub-side shape is identical to the prior design. * review: address Pullfrog feedback — drop ambiguous parenthetical + update postRun nudge - Review-mode calibration: drop the "(or no callout at all)" parenthetical that didn't map cleanly to a bullet; replace with explicit "both the `[!NOTE]` tier and the 'no actionable issues' tier below use approved: true" so the bullet-list anchor is obvious. - `buildUnsubmittedReviewPrompt` (Review mode): the fallback nudge for unsubmitted reviews now defers to the mode prompt's tier matrix and acknowledges that `> [!NOTE]` informational reviews submit with `approved: true` alongside the canonical "No new issues found." path. Previously the nudge only described the pre-NOTE binary world.	2026-05-11 17:14:03 +00:00
David Blass	ee479474ce	action: tighten review alert judiciousness in prompts (#644) ... The Review and IncrementalReview prompts unconditionally wrapped any non-critical review body in `> [!IMPORTANT]`, even for trivial nits or "rough edge" observations. The result is alert fatigue — full-width colored callouts dominate the page when the actual finding is a single JSDoc tweak. Adds an explicit judiciousness preamble to both Review step 5 and IncrementalReview step 7, and splits the prior single non-critical tier into two: - must-address non-critical (`[!IMPORTANT]`) — gated on real consequences if shipped (incorrect behavior, missing validation, regressions the author should fix before merge) - minor suggestions only (no alert) — single-line nits, doc/comment polish, defer-able observations, "rough edges" Critical tier wording also tightened to spell out the bar (`bugs, security, data loss, broken core flows`). Co-authored-by: Cursor <cursoragent@cursor.com>	2026-05-11 17:07:00 +00:00
Colin McDonnell	96910f0f50	fix(run-audit): drop summary comment, fall back to agent final message in job summary ... the audit agent's final 'post a short summary' instruction was ambiguous and, with no PR/issue context on schedule runs, caused the agent to invent a target — landing the summary as a comment on the most recent open PR (see #650). drop the comment instruction outright. writeJobSummary now falls back to the agent's final assistant message (result.output) when lastProgressBody is empty, so non-PR runs surface a real summary in the GitHub Actions job summary tab instead of just the usage table. lastProgressBody still wins when present to avoid duplicating the progress comment body.	2026-05-11 16:56:26 +00:00
Colin McDonnell	4cc6d95a91	ci: split per-alias resolution smoke from per-provider harness smoke (#650) ... * ci: split per-alias resolution smoke from per-provider harness smoke `models-live` previously ran the full Pullfrog harness (Docker + MCP + agent + structured-output validation) once per alias on every PR that touched `models.ts` or `agents/**`. That cost minutes and dollars per alias and re-validated tool-calling for every routing wrapper. The per-alias signal we actually need from `models.ts` changes is just "does this alias resolve and authenticate." Tool-calling correctness is a property of the underlying model, not the alias, and it doesn't change when someone adds a row to the catalog. Splitting the two concerns: - `models-live` now runs `action/test/model-smoke.ts` per alias — a top-level CLI invocation (`opencode run -m <resolve> "reply OK"` or `claude -p "reply OK" --model <bare>`) with no Docker, MCP, or Pullfrog harness. Validates resolution + auth in seconds at fractions of a cent. Lets us drop the `EXPENSIVE_RESOLVE_SUBSTRINGS` carve-out for `gpt-pro` since the cheap smoke covers it for free. - `providers-live` (new) runs the full harness smoke once per provider against a hand-curated standard-tier model (`anthropic/claude-sonnet`, `openai/gpt`, `google/gemini-pro`, `xai/grok`, `deepseek/deepseek-pro`, `moonshotai/kimi-k2`, `opencode/big-pickle`, `openrouter/claude-sonnet`). Catches provider-class regressions like the Gemini schema sanitizer or OpenAI tool-call format drift. ~8 jobs, ~$0.40/push, ~4min critical path in parallel. Net change per push that touches `models.ts`: ~$20 → ~$0.40. `list-aliases.ts` now branches on `MODE` to emit either matrix; the flagship list asserts each slug exists in `modelAliases` so renames break CI loudly. Wiki updated to reflect the new two-tier coverage and the operational rule for new Gemini aliases (cheap smoke covers auth, manual harness run still needed for sanitizer compatibility on non-flagship Gemini additions). * fix(model-smoke): walk fallback chain; address pr review comments - model-smoke now uses `resolveCliModel(slug)` instead of `alias.resolve` so deprecated aliases (those with `fallback` set, e.g. `opencode/mimo-v2-pro-free` → `opencode/big-pickle`) hit the replacement model the way production does. mimo-v2-pro-free was failing CI because the underlying opencode model is dead — the fallback chain is the whole point of marking it deprecated. - tighten stale `agentForSlug()` reference in model-smoke.ts comment (function was deleted in this same PR; classification is now inline in `list-aliases.ts toMatrixEntry`). - tighten `FLAGSHIPS` drift comment to call out that the assertion is one-way (catches slug-rename, but silently omits new providers). Update wiki step 4 of "To add a provider" to require adding the standard-tier slug to `FLAGSHIPS` for harness coverage. * docs: scrub stale env-knob refs in models-catalog parity section `wiki/models-catalog.md` cross-provider parity paragraph still pointed at `INCLUDE_ALL_PASSTHROUGHS` / `INCLUDE_EXPENSIVE` and the implicit filter→expensive-gate coupling — all removed in this PR. Aligned the copy with Step 9 (which was already updated): `INCLUDE_PASSTHROUGHS`, no expensive gate, `MATRIX_FILTER` applies to both aliases and flagships modes.	2026-05-11 16:36:14 +00:00
David Blass	10590993f4	checkout_pr: retry missing pull/N/head ref with PR-state guard (#627) ... * checkout_pr: retry missing pull/N/head ref with PR-state guard Co-authored-by: Cursor <cursoragent@cursor.com> * checkout_pr tests: satisfy ToolState required fields Co-authored-by: Cursor <cursoragent@cursor.com> * checkout_pr: tighten retry-helper semantics (anneal round 1) Co-authored-by: Cursor <cursoragent@cursor.com> * checkout_pr: use retry util, drop retry tests * Update action/mcp/checkout.ts Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>	2026-05-09 00:58:55 +00:00
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>	2026-05-09 00:30:51 +00:00
Colin McDonnell	85d25a6fe6	post-run gate: fail review-mode runs that don't submit a review or progress (#638) ... * post-run gate: fail the run when review mode finishes without a review or progress review-mode runs that ended in a text-only assistant turn ("now I have enough to draft the review...") were silently swallowed: the progress comment was deleted by stranded-comment cleanup and no review appeared on the PR. user- visible result was identical to "the agent never ran." caught in https://github.com/pullfrog/app/actions/runs/25583698781. new post-run gate alongside stopHook / dirtyTree / summaryStale: derived inline from toolState (selectedMode in {Review, IncrementalReview} && !review && !finalSummaryWritten && hadProgressComment) — no parallel toolState flag. when it fires, the resume prompt nudges the agent to call either create_pull_request_review or report_progress; persistent failure after MAX_POST_RUN_RETRIES surfaces as AgentResult.error. also: when the post-run loop returns success=false, write the error to the progress comment before the stranded-comment cleanup runs, and skip the delete in that case. previously a !success run from the loop would lose the error message into the void. IncrementalReview's trivial-skip branch now calls report_progress with a brief "no review warranted" note instead of exiting silently — keeps the contract symmetric with the gate and gives the user a visible signal even on no-op review runs. documents the literal-record design rule on the ToolState interface so future fields don't drift back into derived/absence-encoding state. * review feedback: mode-aware nudge, gate-error preservation, prompt order addresses three findings from the auto-review on this PR: 1. Review mode nudge no longer offers `report_progress` as an exit. Review mode's contract (modes.ts step 5) forbids it; the gate previously sent contradictory copy. IncrementalReview's nudge still offers both since its trivial-skip path legitimately allows `report_progress`. 2. `writeJobSummary` is now wrapped in try/catch on the success-path cleanup. without this, a throw there jumped to the outer catch and overwrote the gate's failure message in the progress comment with the (less actionable) writeJobSummary error — restoring exactly the invisible-failure UX this PR fixes. step-summary writes are informational; let them fail silently. 3. `buildPostRunPrompt` reorders gates to match the terminal hard-fail order: `stopHook` → `unsubmittedReview` → `dirtyTree` → `summaryStale`. when both hard-fail gates co-fire (rare in review modes), the prompt's emphasis now matches the user-visible failure message. new test asserts the IncrementalReview nudge offers both exits while the Review nudge offers only `create_pull_request_review`. e2e validation already passed against pullfrog/preview-638-review-stop-hook PR #1 (gate fired once; agent recovered on second turn). * mode-aware terminal error copy second auto-review caught a residual contradiction: the terminal hard-fail error string reported "create_pull_request_review or report_progress" for both modes, even though the new mode-aware nudge tells Review-mode agents "Review mode does not have a no-submit exit". the error message now mirrors the nudge — Review names only `create_pull_request_review`, IncrementalReview lists both. additional Review-mode hard-fail test asserts the absence of `report_progress` in the error.	2026-05-09 00:14:31 +00:00
David Blass	653fae47a5	claude: surface structured error from is_error result events instead of dumping NDJSON (#626) ... * claude: surface structured error from is_error result events instead of dumping NDJSON Co-authored-by: Cursor <cursoragent@cursor.com> * claude: tighten error-surface fixes (anneal round 1) Co-authored-by: Cursor <cursoragent@cursor.com> * claude: remove tests per request * claude: gate is_error short-circuit on subtype=success, restore error_* branches * claude: preserve fallback token table for error_* subtypes the `lastResultError === null` guard was too broad — `error_max_turns` / `error_during_execution` / `error_*` subtypes set `lastResultError` from `event.errors[]` and represent runs that genuinely consumed tokens, so suppressing the fallback table silently dropped billing visibility for those cases. gate on a dedicated `syntheticStopFailure` flag that's set only for the `subtype: "success"` + `is_error: true` case where `accumulatedTokens` is stale. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>	2026-05-09 00:07:50 +00:00
Colin McDonnell	363e4cbed8	ci: gate gpt-5.5-pro by resolve, refresh stale matrix docs (#639) ... * address review: gate by resolve, refresh stale doc claims - list-aliases.ts: gate EXPENSIVE on alias.resolve substring (catches opencode/gpt-pro and openrouter/gpt-pro, which both resolve to a gpt-5.5-pro variant — would have re-entered the matrix under INCLUDE_ALL_PASSTHROUGHS=1 and tripled the cost). - test.yml + models-catalog.md: stop describing the matrix as exhaustive. Mention pruning + INCLUDE_EXPENSIVE/MATRIX_FILTER opt-ins. * address review: clarify env vars are local-only, filter input is the CI knob Pullfrog review caught that wiki/models-catalog.md was advertising INCLUDE_EXPENSIVE / INCLUDE_ALL_PASSTHROUGHS as workflow_dispatch knobs — they're not, only `filter` (→ MATRIX_FILTER) is wired through. The filter coupling already implicitly opens the expensive gate, so dispatch + filter is the canonical CI path.	2026-05-09 00:01:30 +00:00
Colin McDonnell	c8888cecde	bump action version to 0.1.2 v0.1.2	2026-05-08 23:37:52 +00:00
Colin McDonnell	c0de70431e	ci: prune openai/gpt-pro from default models-live matrix (#637) ... * ci: prune openai/gpt-pro from default models-live matrix gpt-5.5-pro burns ~$2.40/run ($30/M input, $180/M output) — flagship reasoning tier with hidden reasoning tokens dominating cost. Multiplied by every push that touches a resolution-affecting file, the bill is untenable for a smoke that just verifies set_output works. Pruned by default; re-enable with INCLUDE_EXPENSIVE=1 or MATRIX_FILTER when validating the alias on demand. Also adds a comment-frugality rule to AGENTS.md. * ci: include list-aliases.ts in models paths-filter The matrix builder is resolution-affecting from a validation standpoint — a regression to it (e.g. accidentally pruning all aliases) wouldn't trigger models-live on its own commit.	2026-05-08 23:36:26 +00:00
Colin McDonnell	b0274e3265	local proxy-key testing via x-dev-repo bypass (#629) ... * local proxy-key testing via x-dev-repo bypass `pnpm play` previously couldn't exercise the proxy/router/oss code path — `resolveProxyModel` early-exits without OIDC credentials, and `mintProxyKey` always sends an OIDC bearer to `/api/proxy-token`. since GitHub Actions OIDC only exists in real workflow runs, billing flows (auto-reload, balance gates, key rotation, OSS subsidy) had no local feedback loop. a server-side dev bypass already exists at `app/api/proxy-token/route.ts` that accepts an `x-dev-repo: owner/repo` header instead of an OIDC bearer when `NODE_ENV === "development"`. wire the action side so it sends that header when there are no OIDC credentials AND `API_URL` resolves to localhost (i.e. the developer is talking to their own `pnpm dev` server). production is unreachable through this path because vercel never sets `NODE_ENV=development`. document the affordance in `wiki/action-tests.md` so the next person doesn't have to re-discover it (the server bypass had been sitting there undocumented since the WIP billing rewrite). verified end-to-end: `PLAY_LOCAL=1 GITHUB_REPOSITORY=pullfrog/app API_URL=http://localhost:3100 pnpm play …` now logs `» proxy: dev bypass (x-dev-repo) for pullfrog/app` → `» proxy: router → openrouter/ anthropic/claude-opus-4.7` → `» model: …(proxy)`, mints a real OpenRouter key against the dev DB, and the agent runs through the proxy. * wiki: cross-reference dev proxy-key affordance from main/e2e/stripe action-tests.md already documents the localhost+x-dev-repo path; mention it from the natural discovery points so the next person finds it without spelunking through git history again: - main.md: resolveProxyModel row in the dependencies table notes the two auth paths (OIDC bearer in prod, x-dev-repo in dev). - e2e-testing.md: "When to use this" calls out the lighter-weight alternative for proxy-only changes. - stripe.md: new "Loop including the action" subsection in the Dev workflow section, alongside the existing dev-script and cron-endpoint loops.	2026-05-08 23:35:58 +00:00
Colin McDonnell	8f36eca62a	action: use log.success for skill install confirmations	2026-05-08 23:32:20 +00:00
Colin McDonnell	3c9799adda	add models-bump cron + drop snapshot test ... every 12h, scripts/find-newer-models.ts scans models.dev for newer GA versions of every alias in action/models.ts and writes a focused per-alias diff. .github/workflows/models-bump.yml short-circuits when no candidates exist; otherwise hands the diff to pullfrog/pullfrog@main to evaluate against the policy in wiki/model-resolution.md and open a single living PR on the pullfrog/models-bump branch. drops the brittle "latest model per provider" snapshot block in action/test/models-catalog.main.test.ts (and its .snap file) — the cron keeps the registry in sync with upstreams, and the remaining validity tests act as the integrity gate on the bump PR.	2026-05-08 23:27:42 +00:00
Colin McDonnell	5f3e46c42d	fix: don't reuse disabled proxy key on workflow re-runs; non-fatal title-gen errors (#636) ... * fix: don't reuse disabled proxy key on workflow re-runs; non-fatal title-gen errors Three small surgical fixes addressing run https://github.com/pullfrog/app/actions/runs/25580969379: 1. **`/api/proxy-token` idempotency now checks `finalizedAt`.** GitHub re-runs share the same `run_id` (only `run_attempt` increments), so attempt N+1's action calls /api/proxy-token and inherits attempt N's `proxyKeyId`. The `workflow_run.completed` webhook between attempts retires that key on OpenRouter (`disableKey`), so attempt N+1 was getting back a disabled key and OpenRouter responded with `401 User not found` on every call. Falling through when finalized routes through the same billing gate (`handleRouterBilling` balance check), so no new attack surface. 2. **OpenCode title-gen / small-model errors no longer fatal.** OpenCode auto-spawns a small `agent=title small=true` background call at session start to name the thread, defaulting to `anthropic/claude-haiku-4.5` (anomalyco/opencode#1243). Pre-fix, the wrapper's `error` event handler treated any `type=error` as fatal, so a cosmetic title failure killed the run before primary inference even started. Now: stderr matching `small=true` sets a one-shot suppression flag for the next stdout `error` event, which is logged as a warning instead. 3. **Provider-error classifier puts auth patterns above rate-limit.** OpenRouter 401 payloads bundle `x-ratelimit-*` response headers, and the loose `\brate[_ ]limit/i` pattern was winning. Added 401/403 status, `User not found`, `Invalid authentication`, `No auth credentials found` patterns ahead of rate-limit. Updated the existing 401-headers regression test to assert correct auth classification rather than `null`. * opencode: correlate small-model error suppression by message, not by next-event Pullfrog self-review on #636 flagged a real concurrency hole. OpenCode forks the title-gen call (`session/prompt.ts:1452-1457` via `Effect.forkIn(scope)`) so it races primary inference. The previous one-shot `suppressNextErrorEvent` boolean had no per-call correlation: it was consumed by whichever stdout `type=error` event landed next, regardless of which subagent produced it. Under concurrent failures, a primary-agent error landing first could be silently downgraded to a warning while the small-model error then propagated fatally — the inverse of the bug the suppression was meant to prevent. Replaced the boolean with a `Set<string>` of pending small-model error messages. stderr extracts the inner `"message":"..."` from any classified provider error tagged `small=true`; the stdout `error` handler suppresses only when `event.error.data.message` matches a pending entry. Set is capped at 32 entries so a long stream of small-model failures can't wedge memory. Also corrected the comment that referenced "session summarizer" — verified in opencode source that summarize() does NOT use `small: true`; only the title generator does today (only `small: true` match in the codebase). * revert: drop opencode title-gen suppression We have no evidence — and can't construct a realistic scenario — where title-gen fails on an otherwise-successful run. Title-gen and primary share the same OPENROUTER_API_KEY and hit the same proxy/upstream; whatever breaks one breaks the other. The original repro on run 25580969379 is fully explained by the stale proxy key (fix #1) — title-gen happened to be the first call that surfaced the auth error, but every subsequent primary call would have died the same way. Suppression code adds complexity (cross-stream correlation logic, message matching, set capping) and a real failure mode of its own (a small-model error with a unique message could mask an unrelated primary error landing shortly after). Net negative. Removing.	2026-05-08 23:00:41 +00:00