Files
shockbot/play.ts
T
Colin McDonnell f87e0f878c action: minimize pullfrog.yml permissions and drop actions:read (#594)
* action: minimize pullfrog.yml permissions and drop actions:read

The recommended pullfrog.yml workflow asked for a permissions block that's
broader than what the action actually uses with the workflow GITHUB_TOKEN —
all real work (git push, PR comments, reviews) goes through installation
tokens that the action mints via OIDC. Customer security scanners flagged
the workflow-level block as too permissive.

- Move permissions to the job level and reduce to id-token: write,
  pull-requests: write, issues: write. contents:read is the implicit default
  and covers actions/checkout; contents:write, checks:read are unused by
  any GITHUB_TOKEN consumer; actions:read was only used by post-cleanup's
  listJobsForWorkflowRun call.
- Replace listJobsForWorkflowRun with a SIGTERM/SIGINT handler in main.ts
  that calls core.saveState("cancelled", "true"); post-cleanup reads it
  back via core.getState. Same cancel-vs-failure UX, no extra scope needed.
- Sync the docs (headless-action, getting-started, action/README) and the
  two dogfood pullfrog.yml workflows to the new minimal block. Update the
  post-cleanup wiki to describe the saveState approach.

* action: drop pull-requests/issues from required workflow scopes

Switch postCleanup.ts to mint its own short-lived installation token via OIDC
(acquireNewToken with issues:write + pull_requests:write) instead of using the
workflow GITHUB_TOKEN. Same comment-update behavior, but the workflow no longer
needs those scopes — the only permissions Pullfrog ever asks for are id-token:write
(OIDC exchange) and contents:read (actions/checkout).

Also fixes a bug from the previous commit: setting an explicit permissions block
drops every unlisted scope to none (with metadata as the only exception), so
omitting contents would have broken actions/checkout. Restored at both workflow
and job level.

* action: scope id-token:write to pullfrog job, not workflow level

id-token:write is the powerful one — it lets a job mint OIDC tokens that can
be exchanged for cloud credentials or our installation tokens. Keeping it at
workflow level means any future job added to this file silently inherits it.
Move it to the job level where it's actually used; leave only contents:read
at workflow level as a safe baseline for any future jobs.

* action: move stuck-comment cleanup server-side, drop write perms entirely

The action's post-cleanup step lived inside the runner and used the workflow
GITHUB_TOKEN to update the "Leaping into action…" progress comment when a run
failed/cancelled, requiring pull-requests:write + issues:write at the workflow
level. Move that responsibility to the workflow_run.completed webhook handler:
it already has installation-token access via the GitHub App, runs server-side
(no Pullfrog API dependency loop on failure), and lets us drop both write perms.

Recommended workflow permissions block is now truly minimal:

  permissions:
    contents: read
  jobs:
    pullfrog:
      permissions:
        id-token: write
        contents: read

Server side
- handleWorkflowRunCompleted: when conclusion != "success" and the WorkflowRun
  has progressCommentId, mint installation octokit and update the stuck comment
  in place. Try issues.getComment first, fall back to pulls.getReviewComment on
  404 (we don't store comment type — one wasted GET on the rarer review case).
- Reuses buildPullfrogFooter and updateProgressComment from pullfrog/internal,
  matching the wording the action used to write client-side.

Client side
- Delete action/utils/postCleanup.ts and action/post.ts.
- Remove post: + post-if: from action/action.yml.
- Drop runPostCleanup wiring from action/commands/gha.ts and action/play.ts.
- Remove the SIGTERM/saveState handler I added in main.ts in the previous commit
  (no longer needed; cancel/fail signal comes from the webhook hook payload).

Plumbing
- Extract isLeapingIntoActionCommentBody into action/utils/leapingComment.ts so
  the predicate can be re-exported via pullfrog/internal without dragging the
  MCP server's transitive type graph into the Next.js app's typecheck.
- mcp/comment.ts re-exports from the new location for backward compat.

Wiki
- Delete wiki/post-cleanup.md (obsolete; cleanup is now a one-liner branch in
  the workflow_run webhook handler).

* chore: ignore .worktrees in biome config

Recently-added pnpm worktree feature creates nested git worktrees under
.worktrees/, each with their own biome.jsonc declaring root. Biome's
recursive scan trips on the nested config and fails pnpm lint. Excluding
the directory matches the existing .gitignore entry.

* fix: address PR #594 review findings

Two real bugs caught by code review:

1. handleWorkflowRunWebhook.ts:323 — drop the /m flag on the stuck-comment
   detection regex. With /m, ^ matches any line start, so any finalized
   progress comment that embeds a task list (report_progress writes
   `- [x]`/`- [ ]` lines via todoTracking.ts) would be flagged as "stuck"
   and silently overwritten with the "This run croaked" boilerplate
   whenever the workflow concluded non-success after the agent's final
   summary already landed. Restores the body-start anchoring the original
   in-process postCleanup.ts:90 had.

2. action/scripts/check-entrypoint-imports.ts — drop ../post.ts from the
   esbuild entry-point list (the file was deleted in aa43b9af). The
   `pnpm check:entrypoints` step in test.yml would have failed on every
   run with an unresolvable-entry-point error.

Plus three small follow-ups:
- main.ts:580 — comment said "post-cleanup has its own verify-retry loop"
  but post-cleanup is gone. Updated to describe the new server-side path.
- mcp/comment.ts:443 — comment said "so post script doesn't think the run
  failed". Updated to describe the actual current consumers of wasUpdated.
- commands/gha.ts:84 — `--post` help text said "run post-cleanup flow" but
  with the post-cleanup path removed, --post is only valid alongside the
  `token` subcommand for installation-token revocation. Updated wording.

* fix(action): scope --post help text to gha token subcommand

Root gha help text was documenting --post, but --post only makes sense
paired with the token subcommand (it's how the post step revokes the
installation token previously acquired in the main step). Move it to a
dedicated gha token help section and add a parser layer that rejects
--post on the bare gha command.

  $ pullfrog gha --help
  usage: pullfrog gha [subcommand]
  ...
  options:
    -h, --help   show help

  $ pullfrog gha token --help
  usage: pullfrog gha token [--post]
  ...
  options:
    -h, --help   show help
    --post       revoke the previously-acquired token (post-step usage only)

* webhook: artifact-aware cleanup of stranded leaping comments on success

Previously the workflow_run.completed cleanup only handled non-success
conclusions. Extend it to also catch the rare case where a successful
run leaves a "Leaping into action…" comment stuck (in-process cleanup at
action/main.ts:723 normally handles this, but can be skipped on SIGKILL,
runner host crash, or any exit path that bypasses main()'s finally block).

New behavior in cleanupStuckProgressComment:

  - cancelled       → update with "cancelled 🛑" body  (unchanged)
  - failure (other) → update with "croaked 😵" body    (unchanged)
  - success + artifact recorded → delete the comment (the artifact is the
                                  user-facing surface; the leaping comment
                                  is just stale UI noise at this point)
  - success + no artifact recorded → delete the comment AND alert
                                     team@pullfrog.com via emailAlert

The "success + no artifact" path is "should never happen" territory: the
run claims success but produced no review, PR, issue, plan, or summary
comment. The team alert helps us catch in-process cleanup regressions or
artifact-tracking gaps. hasRecordedArtifact reads {review,pr,issue,
planComment,summaryComment}NodeId off the WorkflowRun row to make the call.

* webhook: narrow stuck-comment detection to leaping prefix only

Drop the stranded-todo-pattern branch from cleanupStuckProgressComment.
The leaping prefix is highly specific and impossible to confuse with a
legitimate summary; a leading todo line is not — the agent's
error-reporting paths can produce useful explanatory comments whose
body leads with a checklist (e.g. "here's what I was working on" + the
incomplete todo list), and we don't want to silently overwrite those
with the generic "croaked" boilerplate.

In-process cleanup at action/main.ts:723 still handles the stranded-todo
case in the common path (gated on !finalSummaryWritten with full access
to the in-memory tool state). Missing the rare runner-died-mid-todo case
server-side is a worthwhile trade vs. the false-positive risk on real
explanatory comments.
2026-05-07 18:59:52 +00:00

183 lines
6.2 KiB
TypeScript

import { execSync } from "node:child_process";
import { mkdtemp } from "node:fs/promises";
import { devNull, tmpdir } from "node:os";
import { dirname, join, resolve } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import arg from "arg";
import { config } from "dotenv";
import type { AgentResult } from "./agents/shared.ts";
import { type Inputs, main } from "./main.ts";
import { defineFixture } from "./test/utils.ts";
import { log } from "./utils/cli.ts";
import { runInDocker } from "./utils/docker.ts";
import { ensureGitHubToken } from "./utils/github.ts";
import { isInsideDocker } from "./utils/globals.ts";
import { setupTestRepo } from "./utils/setup.ts";
/**
* default play fixture for ad-hoc testing.
* change this freely without affecting any tests.
*/
export const playFixture = defineFixture(
{
prompt: `List every MCP tool you have access to. Call set_output with a JSON array of all tool names you can see.`,
},
{ localOnly: true }
);
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
// load action's .env file in case it exists for local dev
config();
// also load .env from repo root (for monorepo structure)
config({ path: join(__dirname, "..", ".env") });
export async function run(inputsOrPrompt: Inputs | string): Promise<AgentResult> {
await ensureGitHubToken();
// play.ts is a CI-emulator — isolate it from the developer's user- and
// system-scope gitconfig so checks like `validatePushDestination` see the
// raw stored remote URL instead of values mutated by `url.*.insteadOf`
// rewrites (a common SSH-auth convenience on dev boxes). CI runners have
// empty gitconfigs so this is a no-op there; locally it makes `pnpm play`
// and real runs produce identical git state. `os.devNull` canonicalizes
// the null device across Unix (`/dev/null`) and Windows (`\\.\nul`).
process.env.GIT_CONFIG_GLOBAL = devNull;
process.env.GIT_CONFIG_SYSTEM = devNull;
// create unique temp directory path in OS temp location for parallel execution
// use a parent dir from mkdtemp, then clone into a 'repo' subdirectory
const tempParent = await mkdtemp(join(tmpdir(), "pullfrog-play-"));
const tempDir = join(tempParent, "repo");
const originalCwd = process.cwd();
try {
setupTestRepo({ tempDir });
process.chdir(tempDir);
// run repo setup commands if provided (for pre-planting test state like symlinks).
// this runs AFTER clone but BEFORE the agent, simulating pre-existing repo content.
if (process.env.PULLFROG_TEST_REPO_SETUP) {
log.info("» running repo setup commands...");
execSync(process.env.PULLFROG_TEST_REPO_SETUP, { cwd: tempDir, stdio: "pipe" });
}
// set GITHUB_WORKSPACE to tempDir so main() doesn't try to chdir to the CI checkout path
process.env.GITHUB_WORKSPACE = tempDir;
// allow passing full Inputs object or just a prompt string
const inputs: Inputs =
typeof inputsOrPrompt === "string" ? { prompt: inputsOrPrompt } : inputsOrPrompt;
// set INPUT_* env vars for @actions/core.getInput()
for (const [key, value] of Object.entries(inputs)) {
if (value !== undefined && value !== null) {
process.env[`INPUT_${key.toUpperCase()}`] = String(value);
}
}
const result: AgentResult = await main();
process.chdir(originalCwd);
if (result.success) {
log.success("Action completed successfully");
return { success: true, output: result.output || undefined, error: undefined };
} else {
log.error(`Action failed: ${result.error || "Unknown error"}`);
return { success: false, error: result.error || undefined, output: undefined };
}
} catch (err) {
const errorMessage = (err as Error).message;
log.error(`Error: ${errorMessage}`);
return { success: false, error: errorMessage, output: undefined };
} finally {
// cleanup temp directory - use sudo rm because sandbox isolation may create
// files with different ownership that rmSync can't delete
process.chdir(originalCwd);
try {
execSync(`sudo rm -rf "${tempParent}"`, { stdio: "ignore" });
} catch {
// ignore - cleanup failure is not critical
}
}
}
const isDirectExecution = process.argv[1]
? import.meta.url === pathToFileURL(resolve(process.argv[1])).href
: false;
if (isDirectExecution) {
const args = arg({
"--help": Boolean,
"--raw": String,
"--local": Boolean,
"-h": "--help",
"-l": "--local",
});
if (args["--help"]) {
log.info(`
Usage: node play.ts [options]
Test the Pullfrog action with the inline playFixture.
Options:
--raw [input] Use raw string as prompt, or JSON object as full fixture
--local, -l Run locally (default: runs in Docker)
-h, --help Show this help message
Environment:
PLAY_LOCAL=1 Same as --local
Examples:
node play.ts # Run inline playFixture
node play.ts --raw "Hello world" # Use raw string as prompt
node play.ts --raw '{"prompt":"Hello","timeout":"5s"}' # Use JSON fixture
`);
process.exit(0);
}
// default: run in Docker (unless --local, PLAY_LOCAL=1, or already inside Docker)
const useLocal = args["--local"] || process.env.PLAY_LOCAL === "1" || isInsideDocker;
if (!useLocal) {
const passArgs = process.argv
.slice(2)
.map((a) => `'${a.replace(/'/g, "'\\''")}'`)
.join(" ");
const nodeCmd = `node play.ts ${passArgs}`;
const volumeName = "pullfrog-action-node-modules";
const result = runInDocker({
actionDir: __dirname,
args: process.argv.slice(2),
nodeCmd,
volumeName,
envFilterMode: "passthrough",
onStart: () => log.info("» running in Docker container..."),
});
process.exit(result.status ?? 1);
}
if (args["--raw"]) {
const raw = args["--raw"];
// try to parse as JSON, otherwise treat as prompt string
let input: Inputs | string = raw;
try {
input = JSON.parse(raw) as Inputs;
} catch {
// not valid JSON, use as prompt string
}
const result = await run(input);
process.exit(result.success ? 0 : 1);
}
// no args - use inline playFixture
const result = await run(playFixture);
process.exit(result.success ? 0 : 1);
}