6 Commits

Author SHA1 Message Date
Colin McDonnell ddbc610569 review prompt: friendly green callouts + per-section severity emojis (#756)
* review prompt: friendly green callouts + per-section severity emojis

- Replace `[!NOTE]` informational tier and the no-callout minor-suggestions
  tier with friendly green blockquotes (`> ` / `> 💡`). The two loud
  tiers (`[!CAUTION]` / `[!IMPORTANT]`) keep their GitHub admonitions.
- Add a per-`##`-section severity-emoji rule (🚨/⚠️/💡/ℹ️) for
  cross-cutting review concerns that don't anchor to a line and would
  otherwise be buried in summary content.
- Drop the `<br/>` between summary sections — heading + blank line
  carries enough visual spacing.
- Skip the post-run learnings-reflection turn for `IncrementalReview`.
  It's the lowest-novelty mode (delta review against existing PR with
  prior summary already loaded) and almost never produces durable
  learnings — reflection there costs ~$0.50-0.80/run for nothing.
- Surface real error info on `agent-browser` skill install failures
  (exit code + stdout + stderr + spawn error). The skills CLI uses a
  TUI that prints errors to stdout, so the prior stderr-only logging
  silently swallowed every failure.

* review prompt: per-bullet severity emoji + bullets-only sections

Section headings are plain again (no leading severity emoji). Severity
moves to individual bullets so a section that mixes a 🚨 and a 💡 isn't
mislabeled by either. Section bodies are now bullets only — paragraph
prose under a heading is harder to scan and tends to bury the
actionable point.

Bullets can carry indented continuation content (sub-bullets, code
fences, blockquotes) by indenting two spaces under the parent.

* review prompt: cap section length + identifier discipline

Bound each summary section to at most 4 bullets at most 2 lines each,
and explicitly call out identifier-heavy prose as an anti-pattern. The
reader is often a manager or non-author; identifier-dense paragraphs
('foo calls bar.fetch which dispatches to baz via qux...') are
unreadable for them. Default to plain-language behavior descriptions,
name an identifier only when it's the subject of an actionable concern
or a public surface a reader would recognize, target 2-3 backtick
tokens per bullet.

Move the deep-explanation pattern from open blockquote to a default-
collapsed details/summary so depth doesn't dominate the visible body.

* review prompt: hard cap on bullet identifier density + worked rewrite example

Soft 'aim for 2-3 tokens' guidance was ignored — first big-PR e2e
showed 12 of 19 actionable bullets exceeded the target (avg 4.8 tokens,
several over 8). Promote to a hard cap of 3 backticked tokens per
bullet and pair with a concrete bad/good rewrite the agent can pattern-
match against. Also tighten the per-bullet length cap from ~240 to
~200 chars and explicitly call it 'hard cap, not target'.

* review prompt: tighten bullet length cap to 160 chars, dramatize the worked example

V2 e2e test: token discipline improved (4.8 -> 3.3 avg, 12/19 -> 6/14
violations) but length got worse (235 -> 286 chars, 13/14 over the 200
cap). The agent compensated for fewer identifiers with more prose.

Two changes: (1) tighten the cap from ~200 chars to 160 chars / 1
visual line and call out wrap-to-multiple-lines as the failure mode;
(2) rewrite the worked example so the good version is genuinely half
the length of the bad one, not just lower token count. The example was
the thing the agent pattern-matches against; making the good version
~130 chars vs the bad version's ~290 chars sets the right shape.

* review prompt: drop fixed bullet-count cap, keep length + identifier caps

Per user feedback — section length should be governed by content, not
an arbitrary count. Soft guidance ('past ~6, ask whether to split') is
fine; the hard '≤ 4 bullets per section' rule was the wrong shape.
Length cap (160c) and identifier cap (3 backtick tokens) stay; those
target the actual scanability problem.

* review prompt: drop ## subsystem sections, flat 'Issues found' list

Per-section structure forced every concern into a subsystem frame and
made the body read like a series of mini-essays. Replace with two
parts: (1) TL;DR + Key changes as the dispassionate overview, (2) flat
'### Issues found' list ordered by severity, intermixed across files
and subsystems. Per-bullet rules (≤160c, ≤3 backtick tokens, severity
emoji prefix, optional indented continuation) carry over unchanged.

* review prompt: full v6 structure — preamble + cross-cutting H3s + nitpicks

Replaces the flat 'Issues found' bullet list with the iterated v6 shape:

- Preamble is a bolded inline 'Reviewed changes' lead-in plus bullets
  plus a collapsed 'Review metadata' block (mode/files/commits/refs/
  reviewed commits list/prior pullfrog review/staleness note).
- Each cross-cutting concern gets a '### emoji Title' section. The
  visible problem write-up is human-friendly and DESCRIBES THE PROBLEM
  ONLY — no asks, no suggested fixes, no 'the right thing to do is'.
- Each section carries a collapsed 'Technical details' block wrapped
  in a 4-backtick markdown fence (so it can hold its own 3-tick code
  fences cleanly, agent-readable, one-click copyable). Standard four
  inner sections: Affected sites, Required outcome, optional Suggested
  approach, optional Open questions for the human.
- '### ℹ️ Nitpicks' at the bottom for body-only nits that don't
  inline; simple bullets, no technical-details collapse.
- Anti-paragraph-wall rule: never two successive plain paragraphs in
  visible '### ' sections; alternate prose with structure.
- Inline-vs-body discipline: anything that anchors to a single line
  goes inline, body is for cross-cutting only.
- Drops legacy '### Key changes', '### Issues found', '<b>TL;DR</b>',
  and the '<sub>Summary</sub>' line.

* model effort: bump Gemini + GPT to high effort; drop Gemini Pro→Flash subagent

E2E review eval against a substantive billing-module diff surfaced two
related quality gaps:

1. Gemini Pro at thinkingLevel=medium (#663's CI-timeout fix) reviewed
   the diff only, took the 0-lens path, and missed a catastrophic
   camelCase/snake_case service-vs-schema mismatch. Bumping back to
   high — review work is exactly the wrong shape for the medium/high
   tradeoff #663 was optimizing for; the per-turn TTFT cost is worth
   paying when reasoning IS the value.

2. GPT had no reasoningEffort override, defaulting to upstream medium.
   Same diff, similar shallow result vs Claude. Adding reasoningEffort:
   high for the curated direct-OpenAI slugs, mirroring the Gemini
   pattern (Anthropic separately uses --effort high via the Claude
   Code CLI flag in claude.ts).

3. Gemini Pro's subagentModel was 'gemini-flash' — but Google has no
   in-between tier between Pro and Flash, and Flash is a meaningful
   capability cliff for review work. Dropping the override so subagents
   inherit Pro. Cost stays reasonable since Gemini Pro is already the
   cheapest of the flagship trio.

Other providers unchanged: Anthropic opus→sonnet and OpenAI gpt→gpt-5.4
remain (each is a one-tier drop to a still-capable sibling).

* model effort: revert orchestrator override, set explicit high on reviewfrog subagent

Reshape the effort design after eval:

- Drop the explicit Gemini and GPT model-level overrides — orchestrators
  now run at upstream defaults (Gemini high, GPT-5.x medium). Gemini's
  upstream IS high, so this is a no-op there; GPT goes back to upstream
  medium for orchestrator-level routing work.
- Add explicit 'high' on the reviewfrog subagent via agent.options.
  OpenCode merge order is base ← model.options ← agent.options ← variant
  per session/llm.ts:141, so the subagent always runs at high regardless
  of which orchestrator dispatched it. Both thinkingConfig.thinkingLevel
  (Gemini) and reasoningEffort (GPT) keys included; irrelevant keys are
  ignored per provider.
- Bump providers-live timeouts (12min job / 10min step, from 8/6) to
  budget for Gemini's TTFT variance at high effort. #663's 4min timeout
  was sized for the medium-effort override that's now removed.

* model effort: restore Gemini explicit high override (no-override path breaks)

Bare 'rely on upstream default' for Gemini failed in e2e — removing the
model-level provider config produced 'Function call is missing a
thought_signature' API errors on every gemini-pro run. Even though
upstream opencode's options() returns the same thinkingLevel: high we
were explicitly setting, opencode's resolution path differs subtly
between the two cases. v2's explicit override worked; v3's removal
broke. Reproducible across two consecutive runs.

Restoring the explicit Gemini override (back to v2 design). GPT
orchestrator stays UN-overridden — at upstream default (medium) — since
removing that override didn't trigger the same failure pattern and the
reviewfrog subagent agent.options high override compensates for the
extra depth GPT loses at medium.

* diag: remove reviewfrog agent.options to isolate Gemini thought_signature failure

v3 (no Gemini orch override) failed with thought_signature error. v4
(restored Gemini orch override at v2-equivalent) ALSO failed, even
though the orchestrator config matches v2. The variable between v2
(working) and v4 (failing) is the new reviewfrog agent.options block.
Removing it to confirm — if Gemini works again, the agent.options
addition is the culprit and we need a different shape for it.

* opencode-ai: bump 1.1.56 → 1.15.0 + clean up gemini effort config

opencode-ai@1.1.56 was published 2026-02-10 (3 months old). The Google
API tightened thought_signature validation 24-48h ago (per
https://discuss.ai.google.dev/t/gemini-thought-signature-patch/122555),
and the bug class hits opencode's session→prompt serializer for MCP
tool-call parts (anomalyco/opencode#4832, #8321). Latest stable bumps
us through ~3 months of fixes; needed for Gemini-direct to stop dying
with 'thought_signature is missing' on every multi-turn run.

Companion cleanup: the gemini provider override in opencode.ts had
30-line block of comments, four unused constants, and a 6-line
Object.fromEntries map for two entries. Replaced with one source-of-
truth helper that loops modelAliases, filters provider==='google',
strips the 'google/' prefix, and returns the override map. Adding any
future Google alias to the registry now flows through automatically.

Test added: action/agents/opencode.test.ts asserts the helper covers
every direct-Google alias, strips the prefix correctly, and pins every
entry to thinkingLevel high — catches drift in helper logic without
hardcoding the API ids the test would have to update in lockstep
with the registry.

* fix(workflow): tolerate listJobsForWorkflowRun 404 in resolveRun

PR #750 (docker testing rewrite) replaced the per-call env allowlist
with full process.env passthrough into the test container. That now
leaks GITHUB_RUN_ID + GITHUB_JOB into runs whose MCP token is scoped
to a DIFFERENT repo (e.g. providers-live smoke runs the action against
pullfrog/test-repo with pullfrog/app's run ID). The unconditional
listJobsForWorkflowRun call 404s and crashes the entire run, breaking
every providers-live job on main since #750 landed.

jobId is purely cosmetic (deep-links 'View workflow run' footer to a
specific job vs the run-level URL). Wrapping the API call in try/catch
so a 404 logs a debug message and falls through to undefined jobId is
the right fix — the failure mode is exactly what graceful degradation
is for, and the alternative (filter the env vars at the docker boundary)
re-introduces the kind of allowlist #750 was getting rid of.

* opencode-ai: pin 1.14.51 instead of 1.15.0 (effect refactor breaks JSON output)

opencode 1.15.0 (May 15) ships a major architectural refactor onto
@effect — the run command boots an in-process server via
@opencode-ai/sdk/v2 and the JSON event emission path through that SDK
client doesn't surface on stdout the way our parser expects (CI run
on 1.15.0 produced 0 stdout events but the agent still completed).
Local invocation also hangs at the in-process server boot.

The Gemini thought_signature fixes (the original reason for bumping)
landed earlier in the 1.14.x line, so 1.14.51 (May 14) gets us the
upstream fix without the Effect rewrite. Defer the 1.15.x bump until
we're ready to rewire our parser/spawn around the new SDK.

* opencode-ai: revert to 1.1.56; gha: filter outer-CI workflow-run vars at the docker boundary

Two related changes for the docker testing harness's ergonomics:

1. Revert opencode-ai 1.14.51 → 1.1.56. The 1.14+ line ships an Effect
   refactor (the SDK-v2 client + in-process server architecture) that
   our --format json parser doesn't speak — even the 1.14.51 release,
   pre-dating the 1.15.0 Effect rename, produced 0 stdout events on
   our skill-invoke smoke. There's no clean pre-Effect version that
   ships the Gemini thought_signature fix; that fix needs a separate
   workstream once we're ready to rewire the parser onto SDK v2.

2. Filter outer-CI workflow-run identifiers (GITHUB_RUN_ID, GITHUB_JOB,
   GITHUB_WORKFLOW, GITHUB_ACTION, GITHUB_REF, GITHUB_SHA, etc.) from
   gha.ts's --env-file passthrough. PR #750's full-process.env design
   leaks pullfrog/app's CI run identifiers into runs that act against
   a different repo (e.g. pullfrog/test-repo); any code path inside
   the action that uses them as keys (most notably resolveRun's
   listJobsForWorkflowRun lookup) 404s. Filtering them here means
   the action sees undefined and skips the lookup, complementing the
   defensive try/catch in resolveRun (commit addc76d4). GITHUB_REPOSITORY
   and GITHUB_TOKEN are NOT filtered — those are genuinely needed.

Companion to addc76d4 (resolveRun 404 tolerance). The two together
make this class of bug 'either fix would have caught it' rather than
'silently breaks the entire test matrix'.

* fix(deps): sync pnpm-lock.yaml with opencode-ai 1.1.56 manifest revert

Forgot to refresh the lockfile after reverting the manifest in 02c6d8c1.
CI's frozen-lockfile install was failing with 'lockfile: 1.14.51,
manifest: 1.1.56' mismatch.
2026-05-16 04:58:31 +00:00
Colin McDonnell 8f36eca62a action: use log.success for skill install confirmations 2026-05-08 23:32:20 +00:00
Colin McDonnell 57f54e37c5 add bundled git-archaeology skill, auto-installed for opencode and claude (#565)
* add bundled git-archaeology skill, auto-installed for opencode and claude

ships a SKILL.md teaching agents the underused git history primitives
(pickaxe -S/-G, -L for function/line ranges, --reverse blame, deleted-file
recovery) so they stop scrolling git log -p when blame comes up empty.

introduces a lightweight bundled-skill path alongside the existing
addSkill (npx skills add) flow used for external skills like agent-browser.
SKILL.md is inlined into dist/cli.mjs via esbuild's text loader and written
to <home>/.agents/skills/<name>/SKILL.md at runtime — no network, no version
drift, no per-run install cost.

* fix: register vitest plugin to load .md as text for bundled-skill tests

* fix: drop vite type import from vitest plugin (vite isn't a direct dep)

* fix: load bundled skills via readFileSync so source mode works

esbuild's text loader only applies to the npm-bundled dist/cli.mjs path. the
preview / oss path runs cli.ts directly with node (PULLFROG_FORCE_LOCAL_CLI=1
in runCli.ts#runLocalCli), where node has no idea how to import .md files —
ERR_UNKNOWN_FILE_EXTENSION crashes the action before any agent starts.

switch to runtime readFileSync that checks both candidate locations:
- source mode: <actionRoot>/skills/<name>/SKILL.md (relative to utils/skills.ts)
- bundled mode: <distDir>/skills/<name>/SKILL.md (esbuild copies the tree)

drops the no-longer-needed esbuild text loader, vitest .md plugin, and
ambient *.md type declaration. wiki/skills.md updated with the why.

* fix: write bundled skills to per-agent dirs so claude actually registers them
2026-05-04 18:49:50 +00:00
Colin McDonnell b748355cbe homepage copy refresh + fix skills CLI installation (#539)
* add wiki/betterstack.md documenting log querying, request-ID grouping, and MCP usage

Made-with: Cursor

* fix webhook race conditions: separate runId assignment from data updates

the workflow_run webhook handler had a race where concurrent handlers assigned
the same runId to different pending records. the loser's P2002 silently dropped
data updates (jobId, status, completedAt). fix by splitting into two steps:
assignRunId() handles the race-safe unique assignment, then data updates always
target where: { runId } so they hit the correct record regardless of who won.

also downgrade R2 ObjectLockedByBucketPolicy errors from error to warn level
since duplicate webhook deliveries writing the same key is expected under load.

Made-with: Cursor

* homepage copy refresh + fix skills CLI installation

- update hero to "Agent x GitHub" with new subtagline
- rewrite intro paragraphs: workflow, harness capabilities, billing
- add feature sections: bash isolation, headless browser, MCP tools
- update FAQ answers, footer attribution, free-for-oss copy
- update APP_DESCRIPTION for SEO
- fix skills install: use npx from tmpdir instead of local binary
  (the bundled action has no node_modules; running npx from tmpdir
  avoids project .npmrc with pnpm settings breaking binary resolution)
- instruct agents to use markdown image syntax in upload_file tool
- start dependency installation eagerly from main.ts
- include event title in task instructions

Made-with: Cursor
2026-04-14 20:37:20 +00:00
Colin McDonnell 421607cf97 fix push-to-action: use CLI direct invocation for token acquisition
the inline `node -e` + `TOKEN=$(...)` approach broke because
`core.getIDToken()` in @actions/core writes `::debug::` and
`::add-mask::` to stdout, polluting the captured value.

`node cli.ts gha token` uses `core.setOutput()` which writes to
the $GITHUB_OUTPUT file instead of stdout.

Made-with: Cursor
2026-04-12 00:47:41 +00:00
Colin McDonnell 0055aef618 feat: add Claude Code agent for Anthropic model users (#502)
* feat: add Claude Code agent for Anthropic model users

Re-adds Claude Code support (removed in #478) so users with Anthropic API
keys or Claude Code OAuth tokens can use their Claude subscriptions directly.

When an Anthropic model is selected and Claude Code credentials are available,
the system auto-selects the Claude agent instead of OpenCode. The harness
mirrors opentoad's security model: native Bash blocked via --disallowedTools,
MCP ShellTool for restricted shell, ASKPASS for git auth. Includes NDJSON
streaming, provider error detection, cache/cost tracking, browser skill,
and todo progress tracking.

Key changes:
- action/agents/claude.ts: full Claude Code harness
- action/utils/agent.ts: auto-select Claude for anthropic/* models
- action/utils/providerErrors.ts: extracted shared provider error detection
- action/utils/skills.ts: extracted shared skill installation (agent-aware)
- action/models.ts: add CLAUDE_CODE_OAUTH_TOKEN to anthropic envVars
- action/utils/docker.ts: add CLAUDE_CODE_OAUTH_TOKEN to test env allowlist
- CI: add claude to test matrix, pass CLAUDE_CODE_OAUTH_TOKEN secret

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove unused toolId variable, fix apiKeys test env cleanup

The apiKeys test cleanup stripped *_API_KEY vars but missed
CLAUDE_CODE_OAUTH_TOKEN which doesn't match that pattern.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: strip provider prefix from PULLFROG_MODEL in Claude agent

the env override path was returning the raw value (e.g.
"anthropic/claude-sonnet-4-5") without stripping the provider prefix,
causing the Claude CLI to receive an invalid model ID.

Made-with: Cursor

* fix: remove dead cliPath field, add CLAUDE_CODE_OAUTH_TOKEN to workflows

remove unused cliPath from Claude agent RunParams, and pass
CLAUDE_CODE_OAUTH_TOKEN through all pullfrog.yml workflow templates
so users with Claude Pro/Team subscriptions can use their membership.

Made-with: Cursor

* fix: block Bash subagent in Claude Code disallowedTools

Made-with: Cursor

* chore: update model snapshot (opencode/openrouter latest → qwen3.6-plus-free)

Made-with: Cursor

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 03:29:54 +00:00