Skip to content

Port the iterate-pr skill scripts from Python to Node, for the test harness we already have #221

Description

@thecodedrift

Why

.agents/skills/iterate-pr/scripts/ is ~1400 lines of load-bearing orchestration with zero tests. CI runs node --test .github/scripts/*.test.cjs, so the .cjs scripts beside it are covered and these are not — there is nowhere to put a test, so nobody writes one.

That is not hypothetical. #220 fixed a real bug in propagate_stack.py's core operation: it rebased with git rebase <parent>, whose merge-base upstream is wrong exactly when the parent was rewritten — which is the only reason anyone runs the script. The failure was silent, surfacing as a conflict in files the child never touched, where the obvious resolution discards the parent's fix and the child's own tests still pass. It was found by using the script in anger, not by a test, and #220 ships without one for the same reason.

Scope

Five scripts, all stdlib-only — no dependency risk, nothing to find replacements for:

script lines notes
fetch_pr_feedback.py 610 the real logic: LOGAF bucketing, 🎉 reaction scoping, thread/summary handling
propagate_stack.py 263 cascade restack, balloon guard
fetch_pr_checks.py 241 check status + log snippets
stack_status.py 168 lineage and divergence
resolve_pr_threads.py 118 bulk thread resolve

Imports across all five are argparse, json, re, subprocess, sys, typing. Every one maps directly:

  • argparsenode:util parseArgs
  • subprocess.runnode:child_process execFileSync / spawnSync
  • json / reJSON / RegExp

The scripts shell out to gh and git and parse JSON, so the port is mechanical rather than a rewrite. fetch_pr_feedback.py is both the largest and the one carrying logic worth testing.

What it buys

  • node --test, already wired into Validate. The CI glob needs widening past .github/scripts/ to reach the skill directory — worth deciding deliberately, since .claude/skills is a symlink to .agents/skills and the skill is distributable.
  • Drops uv as a prerequisite. SKILL.md currently says uv run … in 8 places, so anyone using the skill needs a second toolchain the repo does not otherwise require.
  • One runtime for repo tooling instead of two.

Not urgent

Nothing is broken today. This is about the next bug in these scripts being caught by a test rather than by someone losing a parent's commits during a restack. Worth doing when the generator-payload-alignment stack has landed and the server team is unblocked, not before.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions