Skip to content

Run the CI fixer with Pi and publish fixes from a separate job - #310

Merged
rgarcia merged 1 commit into
mainfrom
hypeship/fix-ci-pi
Nov 5, 2026
Merged

rgarcia merged 1 commit into
mainfrom
hypeship/fix-ci-pi

Conversation

@rgarcia

@rgarcia rgarcia commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Moves the "Fix CI Failures" workflow from Cursor CLI to Pi. The agent no longer gets write credentials.

Before this change, the agent ran with an admin app token that could write contents and pull requests, and it read untrusted PR content and CI logs. Now the workflow is split into two jobs.

generate (no write credentials; GITHUB_TOKEN is contents/actions/pull-requests: read):

  • Runs only for failed pull_request test runs whose head repository is this repository. It never runs for forks.
  • A runner step finds the open PR for the run's head branch and SHA. It skips the run if the PR is from another repository or has moved past that SHA, then saves gh run view --log-failed to a file.
  • Pi runs on a checkout of the PR head SHA, with persist-credentials: false and no GH_TOKEN.
    • The prompt and allowlist script come from a separate checkout of the default branch, so PR content cannot change them.
    • Pi runs with --no-approve, so it does not load project-local .pi resources from PR content.
  • A runner step snapshots every change relative to the PR head, committed or not. It keeps only allowlisted paths: go.mod, go.sum, and files under cmd/ and pkg/. Paths with a dot-prefixed component are excluded, so .github/ and other dotfiles never pass. It then cleans the checkout (git clean -fdx) and validates the patch:
    • regular files only
    • no local-path replace
    • make test
    • git diff --check
  • setup-go caching is disabled in this job. It runs PR and agent code, and caches saved from a workflow_run job land in the default branch's scope, where other workflows restore them.

publish (never runs repository code, go, make, or Pi):

  • Rebuilds the fix on the PR head SHA from the uploaded patch. It re-checks the allowlist and regular-file rule with the default branch's script, and commits.
  • Only then does it mint an app token scoped to kernel/cli with contents: write and pull-requests: write.
  • Pushes to ci-fix/<pr-head-branch> with --force-with-lease against the SHA recorded before Pi ran.
  • Posts the 🔧 CI Fix Available comment with the agent's description and a compare link. If a previous comment from the bot exists, it updates that comment instead.

Behavior kept from the current workflow: the fix goes to ci-fix/<pr-head-branch>, never to the PR branch. The workflow comments with a compare link and never opens a PR. If there is no actionable fix, there is no push and no comment.

Behavior that changed:

  • The agent works on the PR head commit instead of a default-branch checkout.
  • The fix branch is regenerated from the current PR head on each run.
  • The runner, not the agent, writes the comment.

The prompt now lives in .github/prompts/fix-ci.md. It is ported from the inline prompt, with the gh and push steps replaced by instructions to leave changes in the working tree and write a summary file. PI_PREFERRED_MODEL replaces CURSOR_PREFERRED_MODEL. The workflow had no model input, so none is added. Actions in this workflow are pinned to commit SHAs at the major versions the repo already uses.

.github/tests/fix-ci-security.test.mjs adds credential-boundary regression tests. They run in a new Fix CI security tests workflow on changes under .github/. That workflow is separate from Run tests for the CLI so its failures don't trigger this fixer.

Validation

Ran:

  • actionlint 1.7.12 with shellcheck 0.10.0 on every workflow in the repo: clean.
  • node --test .github/tests/fix-ci-security.test.mjs: 7/7 pass. Mutation checks confirmed the tests fail when any of these is removed:
    • the same-repo condition
    • --no-approve
    • cache: false
    • the snapshot base reset
    • make test
    • persist-credentials: false
    • read-only generate permissions
    • the publish HEAD check
    • the ci-fix/ prefix
    • the allowlist's dot-path rule
  • Local fixture run of the extracted step scripts against local git repos, with stubbed pi and gh. It covered:
    • allowlist filtering of a misbehaving agent that commits, edits a workflow, and leaves stray and ignored files
    • cleanup
    • symlink rejection in both jobs
    • make test
    • publish rebuild on the PR head
    • tampered-patch rejection
    • lease rejection on a stale fix branch
    • branch creation with an empty lease
    • comment update and the comment body
  • The Find pull request step against the live GitHub API: it resolved an open PR and fetched --log-failed output. It skipped when the SHA wasn't the PR head and rejected an invalid branch name.
  • A local dry run of the generate job on a past failed PR run, using real Pi (openai/gpt-5.5) with no GitHub credentials in the environment:
    • Pi fixed the failing test.
    • The snapshot kept only cmd/ changes.
    • Verify fix (make test) passed.
    • Nothing was pushed.

Not run:

  • A live GitHub Actions run. workflow_run workflows always use the default branch's definition, so this can only run after merge, on the next failed PR test run.
  • The real publish token, push, and comment against GitHub.
  • An Anthropic model. My local run used OpenAI.
  • Whether ANTHROPIC_API_KEY, OPENAI_API_KEY, and the PI_PREFERRED_MODEL variable are visible to this repository. I don't have permission to list them. The run fails fast with a clear error if PI_PREFERRED_MODEL is unset.

@rgarcia
rgarcia requested a review from masnwilliams November 5, 2026 21:25
@rgarcia

rgarcia commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Picked masnwilliams to review: touched the changed code in workflows; 8 pending

@masnwilliams masnwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving. splitting into a read-only generate job and a publish job that only mints the token after re-checking the patch is a big improvement over handing the agent an admin token. none of the below are blockers.

1. credential test misses env inherited from the job or workflow (the one i'd most like to see)
the generate job has no write credentials only looks at the text of the Fix CI failure step. adding GH_TOKEN: ${{ github.token }} to generate's job-level env: would hand Pi a token and still pass 7/7. the same applies to cache: true # was cache: false and persist-credentials: true # was persist-credentials: false, since the regexes match the old value in the comment. parsing the YAML closes all three:

const wf = JSON.parse(execFileSync('yq', ['-o=json', workflowPath], { encoding: 'utf8' }));
const step = (job, name) => wf.jobs[job].steps.find((s) => s.name === name) ?? assert.fail(`missing ${job} step: ${name}`);
const env = { ...wf.env, ...wf.jobs.generate.env, ...step('generate', 'Fix CI failure').env };
for (const key of ['GH_TOKEN', 'GITHUB_TOKEN']) assert.equal(env[key], undefined);
assert.equal(step('generate', 'Setup Go').with.cache, false);

2. one definition of "publishable"
the path rule is in fix-ci-disallowed-paths.sh, but the regular-file awk check is pasted in both jobs. one script reading git diff --cached --raw -z could check path and mode together, so both jobs call the same thing. a one-line comment saying publish is the trust boundary (and generate's checks are cleanup) would also help, since Pi and make test can both touch generate's files before upload.

3. longer term: scripts + fixture tests
the fixture runs in the PR description (allowlist filtering, symlink rejection, tampered patch, stale lease, comment update) are worth more than most of the text-shape assertions. moving the step bodies into .github/scripts/ and committing those tests would make this much easier to change safely.

optional / nits

  • consider pinning @earendil-works/pi-coding-agent to an exact version. it's the one floating dependency that runs with the model API keys. --ignore-scripts already covers install hooks, and Pi releases often, so this is a judgment call.
  • test "$(git rev-parse HEAD)" = "$HEAD_SHA" in publish always passes, since the checkout uses ref: HEAD_SHA.
  • the comment lookup relies on the 🔧 CI Fix Available string being identical in the printf and the jq filter. an HTML marker like <!-- fix-ci --> would be sturdier.
  • the seven if: steps.pr.outputs.number != '' lines could become a separate lookup job, but that adds its own overhead, so fine as is.
  • the temp-file paths are a bit scattered (ci-fix/ vs fix-ci/, excluded-paths vs disallowed-paths).

@rgarcia
rgarcia merged commit 09d8206 into main Nov 5, 2026
9 checks passed
@rgarcia
rgarcia deleted the hypeship/fix-ci-pi branch November 5, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants