Handoff: finish checkpoints v1.1 topology coverage

Background

strategy_options.checkpoints_version = "1.1" opts a clone into reading committed metadata from the local-only custom ref refs/entire/checkpoints/v1.1 instead of the entire/checkpoints/v1 branch. The v1 branch stays the durable source of truth: all writes, fetches, pushes target v1, and the mirror is advanced best-effort after every v1 update.

Topology resolution lives in checkpoint.ResolveCommittedRefs(ctx), returning CommittedRefs{Primary, Read, Mirror}. The v1.1-aware reader is checkpoint.NewCommittedReadStore(ctx, repo). The plain checkpoint.NewGitStore(repo) always pins reads to v1 — correct for write paths and remote/network paths, but wrong for committed reads.

Most user-facing commands already resolve via topology (resume, rewind execution, checkpoint list/explain, status review flags, review, dispatch, push mirror advance). Two real callers are still pinned to v1 plus one block of dead code.

Changes to make

1. Rewind picker prompt text — strategy/manual_commit_rewind.go:164

GetRewindPoints reads per-session prompt text for the picker via strategy.GetMetadataBranchTree(repo) — hardcoded to v1.

The rewind execution itself (line ~632) already resolves topology via NewCommittedReadStore. Only the picker's displayed prompts come from v1. With v1.1 enabled and a stale or missing mirror, the prompts shown in the picker can diverge from what entire explain would show for the same checkpoint.

Fix: resolve the metadata tree against the topology read ref. Options:

• Pass ctx to GetMetadataBranchTree and resolve via checkpoint.ResolveCommittedRefs(ctx).Read.
• Add a GetCommittedReadTree(ctx, repo) helper next to it and have the rewind picker call that, leaving GetMetadataBranchTree for the v1-bound callers in resume.go (fetch chain) and explain.go (blob prefetch) that must stay on v1.

Prefer the second option — keeps the existing v1-specific callers honest and the rename surface small.

2. Orphan detection in entire clean — strategy/cleanup.go:172

ListOrphanedItems constructs checkpoint.NewGitStore(repo) and calls ListCommitted to discover which session states still have associated checkpoints. Reads should go through the topology.

Fix:

ctx is already in scope on ListOrphanedItems.

3. Optional cleanup — drop dead strategy.ListSessions chain

strategy.ListSessions, strategy.GetSession, and getDescriptionForCheckpoint (strategy/session.go:67/178/187) are not called from any production code. entire session list reads .git/entire-sessions/ via strategy.ListSessionStates. The only consumers of ListSessions/GetSession are their own tests.

getDescriptionForCheckpoint is the v1-hardcoded read inside that chain. If the functions are confirmed dead, delete all three plus their tests. If a caller is being added back soon, migrate the read to NewCommittedReadStore instead.

Recommend a quick grep across the org's downstream usage before deleting, since these are exported from the strategy package.

What NOT to touch

These are intentionally pinned to v1:

• All writers in checkpoint/committed.go (WriteCommitted, UpdateCommitted, UpdateSummary, UpdateTranscript, ensureSessionsBranch, getSessionsBranchRef). Writes target v1; the mirror follows.
• strategy/manual_commit.go:54 getCheckpointStore — explicit write-path store.
• Post-commit / finalize / carry-forward hooks in manual_commit_hooks.go:1064/2773/2889.
• attach.go:258 (writes to v1) and attach.go:423/430 checkpointPresentLocally (precondition check on local v1 before writing).
• Remote / network paths: strategy/common.go:973 GetRemoteMetadataBranchTree, manual_commit_push.go:35 push, metadata_reconcile.go reconcile, doctor.go:339 disconnection check. v1 is the only pushable / fetchable ref.
• resume.go:344/368/390/408 getMetadataTree — fetch chain. Fetches always update v1; mirror is advanced afterwards by separate hooks.
• explain.go:825 loadV1MetadataRootTree — blob prefetch needs the fetchable ref, function is named "v1" deliberately.
• manual_commit_logs.go:74/87/95 trailer / source-ref strings. Trailers must reference v1 because that is the durable identifier other clones resolve.
• checkpoint/temporary.go:239 — shadow-branch iterator that filters out the v1 branch by exact name match.

Verification

After each change:

Focused tests likely to move:

• strategy/manual_commit_rewind_test.go
• strategy/cleanup_test.go
• strategy/v1_custom_ref_mirror_test.go
• cmd/entire/cli/checkpoint/committed_refs_test.go

For the rewind picker change, add a test that sets checkpoints_version = "1.1", mutates only the v1.1 mirror ref to a divergent tree, and asserts the picker reads from the mirror — analogous to the existing v1.1 read tests under checkpoint/committed_read_store_test.go.

For entire clean, the orphan listing test in cleanup_test.go should be extended with a v1.1 fixture verifying the read resolves against the mirror.

Risk

Both real changes are cosmetic in steady state — the mirror tracks v1 after every v1 advancement, so reads against either ref return the same tree. The fix matters when:

• The mirror's best-effort update has failed and the refs have drifted.
• The eventual topology flip (v1.1 as Primary) lands; at that point any remaining v1-pinned read is a bug.

No data migration, no settings changes, no user-visible surface change in the default v1 mode.

Out of scope

• Flipping committed_refs.go so v1.1 is Primary. That's a separate rollout step gated on these reads being topology-aware first.
• Pushing the v1.1 ref to remotes. It stays local-only.
• Touching the design of fetches / pushes — v1 remains the only network-visible ref.

Handoff: finish checkpoints v1.1 topology coverage

Background

strategy_options.checkpoints_version = "1.1" opts a clone into reading committed metadata from the local-only custom ref refs/entire/checkpoints/v1.1 instead of the entire/checkpoints/v1 branch. The v1 branch stays the durable source of truth: all writes, fetches, pushes target v1, and the mirror is advanced best-effort after every v1 update.

Topology resolution lives in checkpoint.ResolveCommittedRefs(ctx), returning CommittedRefs{Primary, Read, Mirror}. The v1.1-aware reader is checkpoint.NewCommittedReadStore(ctx, repo). The plain checkpoint.NewGitStore(repo) always pins reads to v1 — correct for write paths and remote/network paths, but wrong for committed reads.

Most user-facing commands already resolve via topology (resume, rewind execution, checkpoint list/explain, status review flags, review, dispatch, push mirror advance). Two real callers are still pinned to v1 plus one block of dead code.

Changes to make

1. Rewind picker prompt text — strategy/manual_commit_rewind.go:164

GetRewindPoints reads per-session prompt text for the picker via strategy.GetMetadataBranchTree(repo) — hardcoded to v1.

The rewind execution itself (line ~632) already resolves topology via NewCommittedReadStore. Only the picker's displayed prompts come from v1. With v1.1 enabled and a stale or missing mirror, the prompts shown in the picker can diverge from what entire explain would show for the same checkpoint.

Fix: resolve the metadata tree against the topology read ref. Options:

• Pass ctx to GetMetadataBranchTree and resolve via checkpoint.ResolveCommittedRefs(ctx).Read.
• Add a GetCommittedReadTree(ctx, repo) helper next to it and have the rewind picker call that, leaving GetMetadataBranchTree for the v1-bound callers in resume.go (fetch chain) and explain.go (blob prefetch) that must stay on v1.

Prefer the second option — keeps the existing v1-specific callers honest and the rename surface small.

2. Orphan detection in entire clean — strategy/cleanup.go:172

ListOrphanedItems constructs checkpoint.NewGitStore(repo) and calls ListCommitted to discover which session states still have associated checkpoints. Reads should go through the topology.

Fix:

ctx is already in scope on ListOrphanedItems.

3. Optional cleanup — drop dead strategy.ListSessions chain

strategy.ListSessions, strategy.GetSession, and getDescriptionForCheckpoint (strategy/session.go:67/178/187) are not called from any production code. entire session list reads .git/entire-sessions/ via strategy.ListSessionStates. The only consumers of ListSessions/GetSession are their own tests.

getDescriptionForCheckpoint is the v1-hardcoded read inside that chain. If the functions are confirmed dead, delete all three plus their tests. If a caller is being added back soon, migrate the read to NewCommittedReadStore instead.

Recommend a quick grep across the org's downstream usage before deleting, since these are exported from the strategy package.

What NOT to touch

These are intentionally pinned to v1:

• All writers in checkpoint/committed.go (WriteCommitted, UpdateCommitted, UpdateSummary, UpdateTranscript, ensureSessionsBranch, getSessionsBranchRef). Writes target v1; the mirror follows.
• strategy/manual_commit.go:54 getCheckpointStore — explicit write-path store.
• Post-commit / finalize / carry-forward hooks in manual_commit_hooks.go:1064/2773/2889.
• attach.go:258 (writes to v1) and attach.go:423/430 checkpointPresentLocally (precondition check on local v1 before writing).
• Remote / network paths: strategy/common.go:973 GetRemoteMetadataBranchTree, manual_commit_push.go:35 push, metadata_reconcile.go reconcile, doctor.go:339 disconnection check. v1 is the only pushable / fetchable ref.
• resume.go:344/368/390/408 getMetadataTree — fetch chain. Fetches always update v1; mirror is advanced afterwards by separate hooks.
• explain.go:825 loadV1MetadataRootTree — blob prefetch needs the fetchable ref, function is named "v1" deliberately.
• manual_commit_logs.go:74/87/95 trailer / source-ref strings. Trailers must reference v1 because that is the durable identifier other clones resolve.
• checkpoint/temporary.go:239 — shadow-branch iterator that filters out the v1 branch by exact name match.

Verification

After each change:

Focused tests likely to move:

• strategy/manual_commit_rewind_test.go
• strategy/cleanup_test.go
• strategy/v1_custom_ref_mirror_test.go
• cmd/entire/cli/checkpoint/committed_refs_test.go

For the rewind picker change, add a test that sets checkpoints_version = "1.1", mutates only the v1.1 mirror ref to a divergent tree, and asserts the picker reads from the mirror — analogous to the existing v1.1 read tests under checkpoint/committed_read_store_test.go.

For entire clean, the orphan listing test in cleanup_test.go should be extended with a v1.1 fixture verifying the read resolves against the mirror.

Risk

Both real changes are cosmetic in steady state — the mirror tracks v1 after every v1 advancement, so reads against either ref return the same tree. The fix matters when:

• The mirror's best-effort update has failed and the refs have drifted.
• The eventual topology flip (v1.1 as Primary) lands; at that point any remaining v1-pinned read is a bug.

No data migration, no settings changes, no user-visible surface change in the default v1 mode.

Use the brainstorming skill to tighten the implementation of this change.

Base directory for this skill: /Users/pfleidi/.claude/plugins/cache/claude-plugins-official/superpowers/5.0.7/skills/brainstorming

Brainstorming Ideas Into Designs

Help turn ideas into fully formed designs and specs through natural collaborative dialogue.

Start by understanding the current project context, then ask questions one at a time to refine the idea. Once you understand what you're building, present the design and get user approval.

Anti-Pattern: "This Is Too Simple To Need A Design"

Every project goes through this process. A todo list, a single-function utility, a config change — all of them. "Simple" projects are where unexamined assumptions cause the most wasted work. The design can be short (a few sentences for truly simple projects), but you MUST present it and get approval.

Checklist

You MUST create a task for each of these items and complete them in order:

1. Explore project context — check files, docs, recent commits
2. Offer visual companion (if topic will involve visual questions) — this is its own message, not combined with a clarifying question. See the Visual Companion section below.
3. Ask clarifying questions — one at a time, understand purpose/constraints/success criteria
4. Propose 2-3 approaches — with trade-offs and your recommendation
5. Present design — in sections scaled to their complexity, get user approval after each section
6. Write design doc — save to docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md and commit
7. Spec self-review — quick inline check for placeholders, contradictions, ambiguity, scope (see below)
8. User reviews written spec — ask user to review the spec file before proceeding
9. Transition to implementation — invoke writing-plans skill to create implementation plan

Process Flow

The terminal state is invoking writing-plans. Do NOT invoke frontend-design, mcp-builder, or any other implementation skill. The ONLY skill you invoke after brainstorming is writing-plans.

The Process

Understanding the idea:

• Check out the current project state first (files, docs, recent commits)
• Before asking detailed questions, assess scope: if the request describes multiple independent subsystems (e.g., "build a platform with chat, file storage, billing, and analytics"), flag this immediately. Don't spend questions refining details of a project that needs to be decomposed first.
• If the project is too large for a single spec, help the user decompose into sub-projects: what are the independent pieces, how do they relate, what order should they be built? Then brainstorm the first sub-project through the normal design flow. Each sub-project gets its own spec → plan → implementation cycle.
• For appropriately-scoped projects, ask questions one at a time to refine the idea
• Prefer multiple choice questions when possible, but open-ended is fine too
• Only one question per message - if a topic needs more exploration, break it into multiple questions
• Focus on understanding: purpose, constraints, success criteria

Exploring approaches:

• Propose 2-3 different approaches with trade-offs
• Present options conversationally with your recommendation and reasoning
• Lead with your recommended option and explain why

Presenting the design:

• Once you believe you understand what you're building, present the design
• Scale each section to its complexity: a few sentences if straightforward, up to 200-300 words if nuanced
• Ask after each section whether it looks right so far
• Cover: architecture, components, data flow, error handling, testing
• Be ready to go back and clarify if something doesn't make sense

Design for isolation and clarity:

• Break the system into smaller units that each have one clear purpose, communicate through well-defined interfaces, and can be understood and tested independently
• For each unit, you should be able to answer: what does it do, how do you use it, and what does it depend on?
• Can someone understand what a unit does without reading its internals? Can you change the internals without breaking consumers? If not, the boundaries need work.
• Smaller, well-bounded units are also easier for you to work with - you reason better about code you can hold in context at once, and your edits are more reliable when files are focused. When a file grows large, that's often a signal that it's doing too much.

Working in existing codebases:

• Explore the current structure before proposing changes. Follow existing patterns.
• Where existing code has problems that affect the work (e.g., a file that's grown too large, unclear boundaries, tangled responsibilities), include targeted improvements as part of the design - the way a good developer improves code they're working in.
• Don't propose unrelated refactoring. Stay focused on what serves the current goal.

After the Design

Documentation:

• Write the validated design (spec) to docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md
  • (User preferences for spec location override this default)
• Use elements-of-style:writing-clearly-and-concisely skill if available
• Commit the design document to git

Spec Self-Review: After writing the spec document, look at it with fresh eyes:

1. Placeholder scan: Any "TBD", "TODO", incomplete sections, or vague requirements? Fix them.
2. Internal consistency: Do any sections contradict each other? Does the architecture match the feature descriptions?
3. Scope check: Is this focused enough for a single implementation plan, or does it need decomposition?
4. Ambiguity check: Could any requirement be interpreted two different ways? If so, pick one and make it explicit.

Fix any issues inline. No need to re-review — just fix and move on.

User Review Gate: After the spec review loop passes, ask the user to review the written spec before proceeding:

"Spec written and committed to <path>. Please review it and let me know if you want to make any changes before we start writing out the implementation plan."

Wait for the user's response. If they request changes, make them and re-run the spec review loop. Only proceed once the user approves.

Implementation:

• Invoke the writing-plans skill to create a detailed implementation plan
• Do NOT invoke any other skill. writing-plans is the next step.

Key Principles

• One question at a time - Don't overwhelm with multiple questions
• Multiple choice preferred - Easier to answer than open-ended when possible
• YAGNI ruthlessly - Remove unnecessary features from all designs
• Explore alternatives - Always propose 2-3 approaches before settling
• Incremental validation - Present design, get approval before moving on
• Be flexible - Go back and clarify when something doesn't make sense

Visual Companion

A browser-based companion for showing mockups, diagrams, and visual options during brainstorming. Available as a tool — not a mode. Accepting the companion means it's available for questions that benefit from visual treatment; it does NOT mean every question goes through the browser.

Offering the companion: When you anticipate that upcoming questions will involve visual content (mockups, layouts, diagrams), offer it once for consent:

"Some of what we're working on might be easier to explain if I can show it to you in a web browser. I can put together mockups, diagrams, comparisons, and other visuals as we go. This feature is still new and can be token-intensive. Want to try it? (Requires opening a local URL)"

This offer MUST be its own message. Do not combine it with clarifying questions, context summaries, or any other content. The message should contain ONLY the offer above and nothing else. Wait for the user's response before continuing. If they decline, proceed with text-only brainstorming.

Per-question decision: Even after the user accepts, decide FOR EACH QUESTION whether to use the browser or the terminal. The test: would the user understand this better by seeing it than reading it?

• Use the browser for content that IS visual — mockups, wireframes, layout comparisons, architecture diagrams, side-by-side visual designs
• Use the terminal for content that is text — requirements questions, conceptual choices, tradeoff lists, A/B/C/D text options, scope decisions

A question about a UI topic is not automatically a visual question. "What does personality mean in this context?" is a conceptual question — use the terminal. "Which wizard layout works better?" is a visual question — use the browser.

If they agree to the companion, read the detailed guide before proceeding: skills/brainstorming/visual-companion.md

I'm wondering if we should update the existing helpers and aim to rename them rather than creating new ones and replacing the old ones. The resulting git history might be much cleaner if we did that. GetMetadataBranchTree could become GetMetadataRefTree and we could support git reference objecs rather than a hardcoded branch name. What would be the tradeoffs of doing so?

Yes please revise the design accordingly

Base directory for this skill: /Users/pfleidi/.claude/skills/grill-me

Interview me relentlessly about every aspect of this plan until we reach a shared understanding. Walk down each branch of the design tree, resolving dependencies between decisions one-by-one. For each question, provide your recommended answer.

Ask the questions one at a time.

If a question can be answered by exploring the codebase, explore the codebase instead.

ARGUMENTS: about these changes

Yes the trade-off is intentional: When the mirror operation is behind, it is usually a sign that something else went wrong. In subsequent changes, we'll flip the mirroring around such that we'll be writing to v1.1 first and then mirroring to v1 so this would become a non-issue. In the meantime, checkpoints v1.1 will only be used by internal engineers who know what to expect.

You're saying about 40 lines per helper. What would they contain? Would that mean inlining 40 lines of setup code? Why so much?

I think in this case adding a helper for two tests isn't worth it. I even have something about preferring to not add extra test helpers in my CLAUDE.md file, I think.

Okay

I approve. Are we done grilling?

Okay go ahead

Base directory for this skill: /Users/pfleidi/.claude/plugins/cache/claude-plugins-official/superpowers/5.0.7/skills/writing-plans

Writing Plans

Overview

Write comprehensive implementation plans assuming the engineer has zero context for our codebase and questionable taste. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. DRY. YAGNI. TDD. Frequent commits.

Assume they are a skilled developer, but know almost nothing about our toolset or problem domain. Assume they don't know good test design very well.

Announce at start: "I'm using the writing-plans skill to create the implementation plan."

Context: This should be run in a dedicated worktree (created by brainstorming skill).

Save plans to: docs/superpowers/plans/YYYY-MM-DD-<feature-name>.md

• (User preferences for plan location override this default)

Scope Check

If the spec covers multiple independent subsystems, it should have been broken into sub-project specs during brainstorming. If it wasn't, suggest breaking this into separate plans — one per subsystem. Each plan should produce working, testable software on its own.

File Structure

Before defining tasks, map out which files will be created or modified and what each one is responsible for. This is where decomposition decisions get locked in.

• Design units with clear boundaries and well-defined interfaces. Each file should have one clear responsibility.
• You reason best about code you can hold in context at once, and your edits are more reliable when files are focused. Prefer smaller, focused files over large ones that do too much.
• Files that change together should live together. Split by responsibility, not by technical layer.
• In existing codebases, follow established patterns. If the codebase uses large files, don't unilaterally restructure - but if a file you're modifying has grown unwieldy, including a split in the plan is reasonable.

This structure informs the task decomposition. Each task should produce self-contained changes that make sense independently.

Bite-Sized Task Granularity

Each step is one action (2-5 minutes):

• "Write the failing test" - step
• "Run it to make sure it fails" - step
• "Implement the minimal code to make the test pass" - step
• "Run the tests and make sure they pass" - step
• "Commit" - step

Plan Document Header

Every plan MUST start with this header:

Task Structure

No Placeholders

Every step must contain the actual content an engineer needs. These are plan failures — never write them:

• "TBD", "TODO", "implement later", "fill in details"
• "Add appropriate error handling" / "add validation" / "handle edge cases"
• "Write tests for the above" (without actual test code)
• "Similar to Task N" (repeat the code — the engineer may be reading tasks out of order)
• Steps that describe what to do without showing how (code blocks required for code steps)
• References to types, functions, or methods not defined in any task

Remember

• Exact file paths always
• Complete code in every step — if a step changes code, show the code
• Exact commands with expected output
• DRY, YAGNI, TDD, frequent commits

Self-Review

After writing the complete plan, look at the spec with fresh eyes and check the plan against it. This is a checklist you run yourself — not a subagent dispatch.

1. Spec coverage: Skim each section/requirement in the spec. Can you point to a task that implements it? List any gaps.

2. Placeholder scan: Search your plan for red flags — any of the patterns from the "No Placeholders" section above. Fix them.

3. Type consistency: Do the types, method signatures, and property names you used in later tasks match what you defined in earlier tasks? A function called clearLayers() in Task 3 but clearFullLayers() in Task 7 is a bug.

If you find issues, fix them inline. No need to re-review — just fix and move on. If you find a spec requirement with no task, add the task.

Execution Handoff

After saving the plan, offer execution choice:

"Plan complete and saved to docs/superpowers/plans/<filename>.md. Two execution options:

1. Subagent-Driven (recommended) - I dispatch a fresh subagent per task, review between tasks, fast iteration

2. Inline Execution - Execute tasks in this session using executing-plans, batch execution with checkpoints

Which approach?"

If Subagent-Driven chosen:

• REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development
• Fresh subagent per task + two-stage review

If Inline Execution chosen:

• REQUIRED SUB-SKILL: Use superpowers:executing-plans
• Batch execution with checkpoints for review

Before going ahead, can you throughly review the docs with fresh eyes and check for inconsistencies and potential issues. Simplicity and maintainability are important goals.

Can you review the spec with fresh eyes and check for inconsistencies and potential issues. Simplicity, clarity, and maintainability are an explicit goal.

Make it so

Base directory for this skill: /Users/pfleidi/.claude/plugins/cache/claude-plugins-official/superpowers/5.0.7/skills/executing-plans

Executing Plans

Overview

Load plan, review critically, execute all tasks, report when complete.

Announce at start: "I'm using the executing-plans skill to implement this plan."

Note: Tell your human partner that Superpowers works much better with access to subagents. The quality of its work will be significantly higher if run on a platform with subagent support (such as Claude Code or Codex). If subagents are available, use superpowers:subagent-driven-development instead of this skill.

The Process

Step 1: Load and Review Plan

1. Read plan file
2. Review critically - identify any questions or concerns about the plan
3. If concerns: Raise them with your human partner before starting
4. If no concerns: Create TodoWrite and proceed

Step 2: Execute Tasks

For each task:

1. Mark as in_progress
2. Follow each step exactly (plan has bite-sized steps)
3. Run verifications as specified
4. Mark as completed

Step 3: Complete Development

After all tasks complete and verified:

• Announce: "I'm using the finishing-a-development-branch skill to complete this work."
• REQUIRED SUB-SKILL: Use superpowers:finishing-a-development-branch
• Follow that skill to verify tests, present options, execute choice

When to Stop and Ask for Help

STOP executing immediately when:

• Hit a blocker (missing dependency, test fails, instruction unclear)
• Plan has critical gaps preventing starting
• You don't understand an instruction
• Verification fails repeatedly

Ask for clarification rather than guessing.

When to Revisit Earlier Steps

Return to Review (Step 1) when:

• Partner updates the plan based on your feedback
• Fundamental approach needs rethinking

Don't force through blockers - stop and ask.

Remember

• Review plan critically first
• Follow plan steps exactly
• Don't skip verifications
• Reference skills when plan says to
• Stop when blocked, don't guess
• Never start implementation on main/master branch without explicit user consent

Integration

Required workflow skills:

• superpowers:using-git-worktrees - REQUIRED: Set up isolated workspace before starting
• superpowers:writing-plans - Creates the plan this skill executes
• superpowers:finishing-a-development-branch - Complete development after all tasks

Just checking: How many lines of code changes are we expecting? It might be simpler to implement things in one go as long as the changes are small enough that I can review them in one go.

Let's do everything in one go subagent driven

Base directory for this skill: /Users/pfleidi/.claude/plugins/cache/claude-plugins-official/superpowers/5.0.7/skills/subagent-driven-development

Subagent-Driven Development

Execute plan by dispatching fresh subagent per task, with two-stage review after each: spec compliance review first, then code quality review.

Why subagents: You delegate tasks to specialized agents with isolated context. By precisely crafting their instructions and context, you ensure they stay focused and succeed at their task. They should never inherit your session's context or history — you construct exactly what they need. This also preserves your own context for coordination work.

Core principle: Fresh subagent per task + two-stage review (spec then quality) = high quality, fast iteration

When to Use

vs. Executing Plans (parallel session):

• Same session (no context switch)
• Fresh subagent per task (no context pollution)
• Two-stage review after each task: spec compliance first, then code quality
• Faster iteration (no human-in-loop between tasks)

The Process

Model Selection

Use the least powerful model that can handle each role to conserve cost and increase speed.

Mechanical implementation tasks (isolated functions, clear specs, 1-2 files): use a fast, cheap model. Most implementation tasks are mechanical when the plan is well-specified.

Integration and judgment tasks (multi-file coordination, pattern matching, debugging): use a standard model.

Architecture, design, and review tasks: use the most capable available model.

Task complexity signals:

• Touches 1-2 files with a complete spec → cheap model
• Touches multiple files with integration concerns → standard model
• Requires design judgment or broad codebase understanding → most capable model

Handling Implementer Status

Implementer subagents report one of four statuses. Handle each appropriately:

DONE: Proceed to spec compliance review.

DONE_WITH_CONCERNS: The implementer completed the work but flagged doubts. Read the concerns before proceeding. If the concerns are about correctness or scope, address them before review. If they're observations (e.g., "this file is getting large"), note them and proceed to review.

NEEDS_CONTEXT: The implementer needs information that wasn't provided. Provide the missing context and re-dispatch.

BLOCKED: The implementer cannot complete the task. Assess the blocker:

1. If it's a context problem, provide more context and re-dispatch with the same model
2. If the task requires more reasoning, re-dispatch with a more capable model
3. If the task is too large, break it into smaller pieces
4. If the plan itself is wrong, escalate to the human

Never ignore an escalation or force the same model to retry without changes. If the implementer said it's stuck, something needs to change.

Prompt Templates

• ./implementer-prompt.md - Dispatch implementer subagent
• ./spec-reviewer-prompt.md - Dispatch spec compliance reviewer subagent
• ./code-quality-reviewer-prompt.md - Dispatch code quality reviewer subagent

Example Workflow

Advantages

vs. Manual execution:

• Subagents follow TDD naturally
• Fresh context per task (no confusion)
• Parallel-safe (subagents don't interfere)
• Subagent can ask questions (before AND during work)

vs. Executing Plans:

• Same session (no handoff)
• Continuous progress (no waiting)
• Review checkpoints automatic

Efficiency gains:

• No file reading overhead (controller provides full text)
• Controller curates exactly what context is needed
• Subagent gets complete information upfront
• Questions surfaced before work begins (not after)

Quality gates:

• Self-review catches issues before handoff
• Two-stage review: spec compliance, then code quality
• Review loops ensure fixes actually work
• Spec compliance prevents over/under-building
• Code quality ensures implementation is well-built

Cost:

• More subagent invocations (implementer + 2 reviewers per task)
• Controller does more prep work (extracting all tasks upfront)
• Review loops add iterations
• But catches issues early (cheaper than debugging later)

Red Flags

Never:

• Start implementation on main/master branch without explicit user consent
• Skip reviews (spec compliance OR code quality)
• Proceed with unfixed issues
• Dispatch multiple implementation subagents in parallel (conflicts)
• Make subagent read plan file (provide full text instead)
• Skip scene-setting context (subagent needs to understand where task fits)
• Ignore subagent questions (answer before letting them proceed)
• Accept "close enough" on spec compliance (spec reviewer found issues = not done)
• Skip review loops (reviewer found issues = implementer fixes = review again)
• Let implementer self-review replace actual review (both are needed)
• Start code quality review before spec compliance is ✅ (wrong order)
• Move to next task while either review has open issues

If subagent asks questions:

• Answer clearly and completely
• Provide additional context if needed
• Don't rush them into implementation

If reviewer finds issues:

• Implementer (same subagent) fixes them
• Reviewer reviews again
• Repeat until approved
• Don't skip the re-review

If subagent fails task:

• Dispatch fix subagent with specific instructions
• Don't try to fix manually (context pollution)

Integration

Required workflow skills:

• superpowers:using-git-worktrees - REQUIRED: Set up isolated workspace before starting
• superpowers:writing-plans - Creates the plan this skill executes
• superpowers:requesting-code-review - Code review template for reviewer subagents
• superpowers:finishing-a-development-branch - Complete development after all tasks

Subagents should use:

• superpowers:test-driven-development - Subagents follow TDD for each task

Alternative workflow:

• superpowers:executing-plans - Use for parallel session instead of same-session execution

Just to validate: Did you take my approval to implement all changes in one go as an approval to commit or why did you try to commit?

1: Try to implement all changes in one go and using git add -p to compose commits as logical steps at the end after I've reviewed them 2: I don't see any changes to common.go. What do you mean?

Are you still there? Are you stuck?

Are you stuck?

Task 4 has been going on for over 20 minutes with no obvious edits

1. I just wanted to ensure one thing is clear: Task for had been running for a very long time when I interrupted

One thing I immediately dislike is the excessive use of code comments. If a competent engineer can understand the code without the comments, remove them or condense them down and make them very concise. It's okay to describe what a section of a test is doing but only if the comment is short and to the point.

Otherwise, the diff looks good. Feel free to start comitting now

Base directory for this skill: /Users/pfleidi/.agents/skills/pfleidi/skills/pr

Create Pull Request

1. Understand the Branch

Use origin/main as the comparison base for understanding branch scope. Do not use local main, a local PR target branch, or direct tree diffs such as git diff main or git diff origin/main for scope checks; those can include upstream-only changes and make this branch look like it reverted unrelated work.

Read the commit history to understand the full scope of changes on this branch.

Review the changed file list from the merge base to the current working tree and confirm every changed file belongs to the PR's stated goal:

If unrelated files or commits are present, STOP and report them. Do not create a PR that bundles unrelated work.

2. Discover Project Verification Commands

Inspect the project to determine how to build, lint, and test. Collect candidate commands from these sources, then deduplicate them before running anything:

1. Makefile — look for build, lint, check, test, ci, verify targets. Read the target recipes to understand what they run.
2. mise — check for .mise.toml or .mise/*.toml. Look for [tasks] definitions covering build, lint, test. If found, use mise run <task>.
3. CI workflows — read .github/workflows/*.yml (or .gitlab-ci.yml, etc.) to understand required coverage. CI is the ground truth for what must pass, but CI matrix shards and CI-only wrappers are not automatically local verification commands.
4. README.md — look for "Development", "Contributing", "Building", or "Testing" sections that document how to run checks.
5. Package manager conventions — detect from project files:
   • go.mod → go build ./..., go vet ./..., go test ./...; do NOT infer a lint command from Go alone
   • package.json → check scripts for build, lint, test
   • Cargo.toml → cargo build, cargo clippy, cargo test
   • pyproject.toml / setup.py → check for configured linters, pytest

If no lint command exists after checking all sources, state that explicitly instead of assuming an unavailable linter binary.

Reuse Cached Verification Discovery

Before rediscovering commands from scratch, choose an artifact directory using the AGENTS.md temporary artifact rule with agent name pfleidi-pr:

• Use ./tmp/pfleidi-pr/ only when ./tmp/ already exists and is already ignored.
• If no project-local artifact directory is available, do not use a verification cache by default. Ask before using /tmp/pfleidi-pr/ or modifying ignore files.

When an artifact directory is available, check for a verification cache at <artifact-dir>/verification-<repo-name>.md. The cache is only an input-token optimization; never commit it and never trust it blindly. If no artifact directory is available, perform normal discovery and skip writing the cache.

Reuse the cache only when all of these are true:

• It names the same worktree root and remote.
• It lists the verification source files it was based on, such as Makefile, .mise.toml, .mise/*.toml, CI workflow files, README files, and package manifests.
• Those source files still exist or are still intentionally absent.
• git diff --name-only origin/main -- <source files> shows no branch changes to those source files.

If the cache is missing, stale, or incomplete, perform normal discovery. After discovery, update the cache with:

• Repository root and remote.
• Verification source files inspected.
• Selected command plan grouped by coverage area.
• Commands intentionally skipped as duplicates, aggregate/subtask overlaps, CI-only jobs, or too-slow shard matrices.
• Any assumptions, such as "no documented lint task found."

Deduplicate Verification Commands

Build a command plan by coverage area, not by source. Do not run every command discovered.

• Run at most one command for each coverage area: build/compile, lint/static analysis, unit/core tests, integration tests, e2e/smoke tests.
• Prefer documented local developer tasks over CI-specific commands when they cover the same area.
• Do not run both an aggregate task and its constituent tasks. For example, if mise run check runs lint and tests, either run mise run check alone or run the narrower lint/test tasks, not both.
• Treat CI matrix shards as duplicated slices of one suite. Do not run every *:shard:* command locally when an unsharded local task covers the suite.
• If CI has only sharded commands and no local equivalent, ask before running all shards. Otherwise, run the smallest representative or changed-scope test command and note that the full shard matrix remains for CI.
• Do not run CI-only canary/e2e jobs locally by default. Run them only when the PR changes that surface, when the user asks, or when the project documents them as required local PR verification.

Log which sources you used, which duplicate/CI-only commands you skipped, and what commands you will run. If the deduplication rules require asking before slow CI-only coverage, STOP for confirmation; otherwise immediately proceed to step 3.

3. Run Verification and Auto-Fix

Run the deduplicated command plan in the fewest safe batches. Prefer background processing for independent validation tasks instead of running everything sequentially.

The commands should cover, at minimum:

• Build — the project compiles without errors
• Lint / static analysis — no lint warnings or static analysis failures
• Tests — the selected local test coverage passes without duplicating CI shards or aggregate/subtask combinations

Use the exact commands, flags, and build tags found in step 2 for the commands you selected. Do not invent your own flags.

Parallel Verification Rules

Partition the selected commands into dependency-safe batches before running them:

• Run mutating commands alone and before validators that depend on their output. This includes formatters, generators, codegen, migrations, package installation, or commands known to update snapshots, lockfiles, generated files, caches in the repo, or test fixtures.
• Run dependent commands after their prerequisite batch passes. For example, do not start tests that require generated code until generation succeeds.
• Run independent read-only validation commands concurrently in the same background batch. Build, lint/static analysis, typecheck/vet, and unit tests can usually share a batch when they do not mutate the working tree and do not require the same exclusive service, port, database, or fixture directory.
• Keep integration, e2e, or service-backed commands separate unless the project documents that they are parallel-safe.
• If unsure whether two commands are independent, run them sequentially. Correctness of validation beats speed.

For each background batch:

1. Start every command from the same working-tree state.

2. Run each selected validator directly, for example mise run lint, go test ..., or npm test -- .... Do not wrap validators in sh -c, shell redirection, tee, command separators, or pipelines solely to capture logs; that defeats command-prefix approvals and causes extra permission prompts.

3. Capture each command's stdout, stderr, exit status, and command line from the tool output separately.

4. While the batch is running, do not edit files, start auto-fixes, or treat partial output as a result.

5. Wait for every command in the batch to finish, then show verification as a compact table:

   Command	Exit	Relevant output
   go test ./pkg/foo -run TestBar -count=1	0	Short success excerpt.

6. For failures or short outputs, show complete output in the relevant-output column or immediately below the table. For long successful outputs, show the relevant excerpt and state that the rest was truncated.

7. If any command in the batch fails, treat the whole batch as failed for the fix loop. Results from other commands in that stale batch may help diagnose, but they do not count as passing verification after files change.

On Failure: Fix and Re-verify

If any command fails, do NOT stop. Instead:

1. Read the error output and identify every failure
2. Fix all issues — apply the minimal changes needed to make the failing command pass
3. Re-run the deduplicated verification plan from the top, using the same safe batching rules (not just the previously failing command — fixes can introduce new issues)
4. Show the updated verification table again, including complete failure output for any command that still fails

Repeat this cycle until all commands pass. Cap at 3 fix attempts. If verification still fails after 3 rounds, STOP and present the remaining failures to the user with full failure output — do not keep looping.

4. Prompt for Commit

After all verification passes, check for uncommitted changes:

If there are uncommitted changes (from auto-fixes in step 3):

1. Show the diff of all uncommitted changes
2. Propose a semantically correct commit message using the subject-plus-context style from AGENTS.md. The message must describe the net fix (e.g., "fix lint warnings in config parser" not "fix issues found during PR prep").
3. STOP and wait for user approval. The user may edit the message, split the changes, or commit themselves.

If the user approves the commit, do not rerun the full verification suite before committing unless files changed after step 3. If another sanity check is needed, use the commit-time verification scope from AGENTS.md: lint tasks, a fast compile/build check, and tests directly related to the changed code only.

If there are no uncommitted changes, proceed directly to step 5.

5. Push the Branch

If the branch has no upstream yet, use git push -u origin HEAD.

6. Create the PR

Determine a concise PR title (under 70 characters) from the commit history and diff.

Use the same branch-only comparison from step 1 ($MERGE_BASE to the current working tree) when deriving the title, PR body, changed-file list, and mostly-Markdown detection. Do not use local main or direct git diff origin/main output for PR description decisions.

Write the PR body with:

• What this PR does and why
• How it was implemented (brief, not exhaustive)
• Any migration, deployment, or reviewer notes if applicable
• For mostly Markdown PRs, a Rendered Markdown section linking to the changed Markdown files rendered on GitHub

Create the PR:

Create a ready-for-review PR only when the user explicitly asks for it with wording such as "non-draft", "ready for review", or "publish it as ready". In that case, omit --draft.

Show the PR URL and STOP.

Rules

• Do NOT add "Generated with Claude Code" or any AI attribution anywhere
• Do NOT summarize what you just did after showing the URL
• Treat a PR as "mostly Markdown" when more than half of changed files are Markdown (.md, .mdx, .markdown) or the main review target is Markdown documentation. In that case, include GitHub rendered links for the changed Markdown files using the pushed branch, e.g. https://github.com/<owner>/<repo>/blob/<branch>/<path>.md. Link all changed Markdown files when the list is short; for large docs PRs, link the main entry points or most important changed files.
• If the user provides a title or description, use it verbatim instead of generating one, but append rendered Markdown links for mostly Markdown PRs unless the user explicitly says not to modify the description.
• If the PR description contains checklist items (- [ ]), check them (- [x]) if and only if the corresponding step was actually verified during this workflow (e.g., build passed, tests passed, lint passed). Do NOT check items that were not verified. Do NOT leave items unchecked if they were verified.

xhigh effort → 5 angles × 8 candidates → 1-vote verify → sweep → ≤15 findings

You are reviewing for recall at extra-high effort: catch every real bug. At this level, catching real bugs matters more than avoiding false positives — a missed bug ships. Err on the side of surfacing.

Phase 0 — Gather the diff

Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Find candidates (5 angles, up to 8 each)

Run 5 independent finder angles via the Agent tool. Each surfaces up to 8 candidate findings. Do NOT let one angle's conclusions suppress another's — if two angles flag the same line for different reasons, record both.

Angle A — line-by-line diff scan

Read every hunk in the diff, line by line. Then Read the enclosing function for each hunk — bugs in unchanged lines of a touched function are in scope (the PR re-exposes or fails to fix them). For every line ask: what input, state, timing, or platform makes this line wrong? Look for inverted/wrong conditions, off-by-one, null/undefined deref, missing await, falsy-zero checks, wrong-variable copy-paste, error swallowed in catch, unescaped regex metachars.

Angle B — removed-behavior auditor

For every line the diff DELETES or replaces, name the invariant or behavior it enforced, then search the new code for where that invariant is re-established. If you can't find it, that's a candidate: a removed guard, a dropped error path, a narrowed validation, a deleted test that was covering a real case.

Angle C — cross-file tracer

For each function the diff changes, find its callers (Grep for the symbol) and check whether the change breaks any call site: a new precondition, a changed return shape, a new exception, a timing/ordering dependency. Also check callees: does a parallel change in the same PR make a call unsafe?

Angle D — language-pitfall specialist

Scan for the classic pitfalls of the diff's language/framework — for example: JS falsy-zero, == coercion, closure-captured loop var; Python mutable default args, late-binding closures; Go nil-map write, range-var capture; SQL injection; timezone/DST drift; float equality. Flag any instance the diff introduces.

Angle E — wrapper/proxy correctness

When the PR adds or modifies a type that wraps another (cache, proxy, decorator, adapter): check that every method routes to the wrapped instance and not back through a registry/session/global — e.g. a caching provider holding a delegate field that resolves IDs via session.get(...) instead of delegate.get(...) will re-enter the cache or recurse. Also check that the wrapper forwards all the methods the callers actually use.

Phase 2 — Verify (1-vote, 3-state)

Dedup candidates that point at the same line/mechanism, keeping the one with the most concrete failure scenario. For each remaining candidate, run one verifier via the Agent tool: give it the diff, the relevant file(s), and the candidate, and have it return exactly one of:

• CONFIRMED — can name the inputs/state that trigger it and the wrong output or crash. Quote the line.
• PLAUSIBLE — mechanism is real, trigger is uncertain (timing, env, config). State what would confirm it.
• REFUTED — factually wrong (code doesn't say that) or guarded elsewhere. Quote the line that proves it.

Keep candidates where the vote is CONFIRMED or PLAUSIBLE.

This is recall mode — a single non-REFUTED vote carries the finding. Do NOT drop on uncertainty.

Phase 3 — Sweep for gaps

Run one more finder as a fresh reviewer who has the verified list. Re-read the diff and enclosing functions looking ONLY for defects not already listed. Do not re-derive or re-confirm anything already there — the job is gaps. Focus on what the first pass tends to miss: moved/extracted code that dropped a guard or anchor; second-tier footguns (dataclass default evaluated once, hash() non-determinism, lock-scope shrink, predicate methods with side effects); setup/teardown asymmetry in tests; config defaults flipped.

Surface up to 8 additional candidates, each naming a defect not already on the list. If nothing new, return an empty sweep — do not pad.

Output

Return findings as a JSON array of at most 15 objects:

Ranked most-severe first. If more than 15 survive, keep the 15 most severe. If nothing survives verification, return [].

Double check which of these apply and then rank them

Having no remote tracking fallback in v1.1 is intended as this functionality is simply not implemented, yet. It will only be added once we've validated the current changes locally without pushing for a while. Feel free to address the other findings and I'll review the fixes

Commit the changes and push. Then run /pfleidi:pr-feedback

Base directory for this skill: /Users/pfleidi/.agents/skills/pfleidi/skills/pr-feedback

PR Feedback

1. Identify the PR

If the user provided a PR number or URL, use that. Otherwise, detect from the current branch:

If no PR exists for the current branch, stop and tell the user.

2. Fetch Review Comments

Fetch PR metadata first so comment decisions have intent context:

Also collect branch commits and changed files for context:

Show the PR context as a table before classifying comments:

Fetch unresolved review threads with GraphQL as the primary source of truth. Group work by thread, not by individual REST comment:

Filter to unresolved threads only. If there are no unresolved threads, report that to the user and stop — there is nothing to fix.

If GraphQL pagination indicates more review threads or thread comments are available, paginate before classifying. Do not classify a partial thread set as complete.

Use REST pull-review comments only as a fallback when GraphQL data is incomplete or a thread cannot be mapped to a review comment ID:

When REST fallback is used, deduplicate by GraphQL thread ID first, then by file/line/body/author. Do not present or fix the same review request twice.

3. Parse, Classify, and Group

Use permission-friendly reads while investigating comments. Avoid shell pipelines, command separators, subshells, and output filters for read-only source inspection because they create extra permission prompts and can block background work. Do not run commands like git show HEAD:path | sed -n '10,40p'. Use workspace file range reads, rg with path limits, path-scoped diffs, or one standalone git show <rev>:<path> only when the output is acceptably small.

For each comment, extract:

• Author — who left it
• Author type — bot, automated reviewer, human reviewer, or maintainer
• File and line — where it points
• Body — the actual feedback (verbatim, not paraphrased)
• Thread context — any replies in the same thread (to understand if it was already discussed or resolved conversationally)
• Thread ID and comment ID — the GraphQL review thread ID and original comment ID needed to reply and resolve

Group each unresolved review thread into a single finding. If multiple comments in one thread refine or supersede each other, use the latest unresolved reviewer request as the finding and retain the earlier messages as context.

Classify each finding source:

• Bot — GitHub bot, CI system, or linter/static-analysis account such as github-actions[bot] or codecov[bot]
• Automated reviewer — review-assistant accounts that produce natural-language suggestions, such as Copilot or CodeRabbit
• Human reviewer — non-bot reviewer
• Maintainer — repository owner/member/maintainer when that can be inferred from GitHub metadata

4. Present Findings

Present two separate sections:

Human Comments

Table ordered by:

1. Bugs / correctness issues — reviewer identified broken logic or missing error handling
2. Design / architecture feedback — structural changes, API shape, naming of public interfaces
3. Style / nits — formatting, naming of local variables, minor readability

Use this table format:

For automated reviewers, use the same table and set Reviewer to the tool account, with Priority based on the substance of the request.

Bot Comments (batched)

Table continuing the numbering from above, grouped by tool/bot:

Keep table cells short and scannable. Use the smallest useful verbatim quote, not the full comment body. Escape | characters inside code or text so the table remains valid Markdown.

End with a summary: total human comments, total bot comments, overall assessment of effort.

Do not stop for mode selection. Proceed by default with bot comments and human comments marked Autofix eligible. Mark a human comment Autofix eligible only when the requested change is source-backed, high confidence, minimal, unambiguous, does not require a product/design decision, does not add a dependency, does not change a shared/public interface, and has a clear verification path.

Leave all other human comments unresolved as Needs decision, with the exact decision needed. Do not reject a reviewer comment by default; rejection requires a user-provided public rationale.

Before applying any fixes, record the starting commit:

Choose an artifact directory using the AGENTS.md temporary artifact rule with agent name pfleidi-pr-feedback:

• Use ./tmp/pfleidi-pr-feedback/ only when ./tmp/ already exists and is already ignored.
• If no project-local artifact directory is available, do not create file artifacts by default; keep ledger/log/cache information in the response and mark file paths n/a. Ask before using /tmp/pfleidi-pr-feedback/ or modifying ignore files.

When an artifact directory is available, create a temporary thread ledger at <artifact-dir>/pr-feedback-<pr-number>.md. If no artifact directory is available, keep the same ledger fields in the final summary table instead. Update the ledger after each thread with:

• Thread ID, source category, reviewer, location, and status.
• Files touched.
• What changed and why.
• Related tests or verification commands.
• Planned public reply, if any.
• Resolve decision: yes/no and why.

5. Fix Bot Comments (batched)

Fix all bot comments first — these are mechanical and clearing them reduces noise before the human-comment phase.

1. For each bot finding:
   • Read the relevant code
   • Implement the fix — ONLY the changes needed for that single finding
   • Track the files changed for this finding so the final PR reply can identify the commit that contains the fix
   • If a fix is ambiguous or would conflict with a human-comment fix already applied, mark it Needs decision and continue
2. After all bot fixes are applied, present a summary table. Do NOT show a diff — the Edit tool already showed each change inline.

1. Proceed directly to Step 6.

6. Fix Human Comments (batched)

After bot fixes, work through Autofix eligible human comments in report order:

1. State which finding you are addressing (number and one-line description)
2. Read the relevant code and the full comment thread to understand intent
3. Re-check eligibility before editing; if the fix is no longer clearly eligible, mark it Needs decision and continue
4. Implement the fix — ONLY the changes needed for that single finding
5. Track the files changed for this finding so the final PR reply can identify the commit that contains the fix
6. If a comment needs a product/design decision, shared/public interface change, dependency, broad refactor, or has multiple reasonable fixes, mark it Needs decision and continue
7. If the user rejects the comment instead of fixing it, record the specific rationale to use in the final PR reply

Scope Rules

• Make the MINIMAL change that addresses the reviewer's feedback
• Keep the diff limited to files and lines directly required by the feedback
• First decide whether the feedback points to a local or systemic issue. Fix at the narrowest correct level; do not add a local workaround that hides a shared/root-cause bug.
• If the feedback requires a behavior-changing code fix, add or update the directly related test in the same fix. Prefer TDD, but complete the focused red-to-green cycle before stopping: write/update the failing test, confirm it fails, implement the fix, confirm the focused test passes. Do not stop after only adding the failing test unless the user explicitly asks.
• Do NOT rename variables, reformat code, or touch lines outside the feedback scope
• Do NOT refactor adjacent code, even if it looks related
• If the reviewer's comment is ambiguous, mark it Needs decision and continue with unrelated unambiguous comments
• Do NOT create any git commits during the fix cycle. Commits are handled only in the publish step, and only with explicit user approval when needed.

7. Verify Fixes

After all fixes are applied, run the project's lint and test commands scoped to only the changed files and their directly related tests. If no code changed, skip verification and proceed to Step 8. Use safe background batches for independent validators instead of running every command sequentially.

When selecting verification commands, reuse <artifact-dir>/verification-<repo-name>.md if an artifact directory is available and the cache is fresh under the cache rules from pfleidi:pr; otherwise discover the smallest relevant lint/test/build commands. Update the cache only when an artifact directory is available.

• Lint / static analysis — run the project's documented lint task, scoped to the files that were modified when the task supports scoping. Prefer lint-specific task wrappers such as make lint or mise run lint over invoking linter binaries directly. Do not use aggregate check, ci, or verify tasks unless you have confirmed they only run lint/static analysis. If the documented lint task cannot be scoped, run the smallest relevant project lint task.
• Tests — run only the test files that cover the modified code (same package, same module, co-located test files). Do NOT run the full test suite.

If no project lint task exists, state that explicitly instead of assuming an unavailable linter binary.

Run formatters, generators, snapshot updates, or other mutating commands alone before validators that depend on their output. Run independent read-only validators concurrently when they do not require the same exclusive service, port, database, fixture directory, or generated output. Keep integration/e2e/service-backed commands separate unless the project documents that they are parallel-safe.

For each background batch, start every command from the same working-tree state, capture stdout/stderr/exit status from the tool, do not edit files while the batch is running, and wait for every command to finish. Run each selected validator directly, for example mise run lint, go test ..., or npm test -- .... Do not wrap validators in sh -c, shell redirection, tee, command separators, or pipelines solely to write logs; that defeats command-prefix approvals and causes extra permission prompts. If an artifact directory is available and file logs can be written after the command completes without rerunning through a shell wrapper, save them under <artifact-dir>/logs-<pr-number>-<timestamp>/; otherwise mark the full-log path as n/a. If files change after a failed batch, none of that batch's successful results count as current verification.

Show verification as a compact table:

For failures or short outputs, show complete output in the relevant-output column or immediately below the table. For long successful outputs, show the relevant excerpt and log path.

If lint or tests fail due to issues introduced by the fixes:

1. Read the error output and identify every failure
2. Fix all issues — apply the minimal changes needed
3. Re-run the failing commands using the same safe batching rules
4. Show the complete output again

Cap at 2 fix attempts. If still failing after 2 rounds, present the remaining failures to the user with full output.

Once verification passes, show a summary: how many comments were addressed, rejected, intentionally left unresolved, or still blocked. Do NOT show a diff — the Edit tool already showed each change inline.

Proceed to Step 8 for threads that were addressed or intentionally rejected. Leave Needs decision threads unresolved and do not reply to them unless the user provided a public rejection rationale. Do not block publishing addressed threads just because unrelated threads still need a decision.

8. Publish PR Updates

After addressed/rejected threads are ready to publish:

1. Check branch state:

2. If there are uncommitted fix changes, STOP and ask the user whether to commit them now or let the user commit manually. Do not push until the fixes are committed. If the user approves committing, stage only files changed for the PR feedback fixes and write the commit message from the actual diff using the subject-plus-context style from AGENTS.md.

3. Push the committed changes for the current branch:

   If the branch has no upstream and the push fails for that reason, use:

   Never force-push.

4. Map each addressed finding to the commit or commits that contain its fix. Use the recorded starting commit, changed-file tracking, ledger, and git log / git show to identify the relevant short SHA(s). If one commit fixes multiple comments, reference the same commit in each reply.

5. Build and show a reply plan table before calling the API:

   Thread	Status	Reply body	Resolve
   PRRT_...	Addressed	Addressed in abc1234 by adding the nil check before dereferencing.	Yes
   PRRT_...	Needs decision	n/a	No

   Proceed without asking when every actionable reply row is either addressed or a user-approved rejection. Needs decision rows with Reply = n/a and Resolve = No do not block publishing addressed threads. Stop before API calls if a rejection lacks a public rationale or if any reply body is uncertain.

6. Reply to each original PR review thread:

   • For addressed comments, state what changed and the commit SHA(s), e.g. Addressed in abc1234 by adding the nil check before dereferencing.

   • For rejected comments, state why it was not changed, e.g. Not changed: this path already returns before allocation failure can be observed.

   • Keep replies concise but specific. Do not over-explain or repeat the diff.

   • Do not reply to comments the user explicitly chose to leave unresolved.

   • Prefer replying with the review thread ID:

     If the thread cannot be replied to by GraphQL, use the original review comment ID with the REST reply endpoint.

7. Resolve each thread after replying:

   Do not resolve a thread that was not replied to, is still ambiguous, or still needs user approval.

8. Present a final summary table:

   Thread	Reviewer	Location	Status	Reply	Resolved
   PRRT_...	reviewer	file.go:42	Addressed	Yes	Yes
   PRRT_...	reviewer	file.go:99	Needs decision	No	No

   Include push result, branch, ledger path, rejected comments and rationale, and any comments intentionally left unresolved or not resolvable via API.

9. Post-Publish

• If the user asks for adjustments, apply them
• If the user asks to revert specific fixes, revert ONLY those changes
• Do NOT commit or push additional changes unless explicitly asked

Base directory for this skill: /Users/pfleidi/.agents/skills/pfleidi/skills/pr-feedback

PR Feedback

1. Identify the PR

If the user provided a PR number or URL, use that. Otherwise, detect from the current branch:

If no PR exists for the current branch, stop and tell the user.

2. Fetch Review Comments

Fetch PR metadata first so comment decisions have intent context:

Also collect branch commits and changed files for context:

Show the PR context as a table before classifying comments:

Fetch unresolved review threads with GraphQL as the primary source of truth. Group work by thread, not by individual REST comment:

Filter to unresolved threads only. If there are no unresolved threads, report that to the user and stop — there is nothing to fix.

If GraphQL pagination indicates more review threads or thread comments are available, paginate before classifying. Do not classify a partial thread set as complete.

Use REST pull-review comments only as a fallback when GraphQL data is incomplete or a thread cannot be mapped to a review comment ID:

When REST fallback is used, deduplicate by GraphQL thread ID first, then by file/line/body/author. Do not present or fix the same review request twice.

3. Parse, Classify, and Group

Use permission-friendly reads while investigating comments. Avoid shell pipelines, command separators, subshells, and output filters for read-only source inspection because they create extra permission prompts and can block background work. Do not run commands like git show HEAD:path | sed -n '10,40p'. Use workspace file range reads, rg with path limits, path-scoped diffs, or one standalone git show <rev>:<path> only when the output is acceptably small.

For each comment, extract:

• Author — who left it
• Author type — bot, automated reviewer, human reviewer, or maintainer
• File and line — where it points
• Body — the actual feedback (verbatim, not paraphrased)
• Thread context — any replies in the same thread (to understand if it was already discussed or resolved conversationally)
• Thread ID and comment ID — the GraphQL review thread ID and original comment ID needed to reply and resolve

Group each unresolved review thread into a single finding. If multiple comments in one thread refine or supersede each other, use the latest unresolved reviewer request as the finding and retain the earlier messages as context.

Classify each finding source:

• Bot — GitHub bot, CI system, or linter/static-analysis account such as github-actions[bot] or codecov[bot]
• Automated reviewer — review-assistant accounts that produce natural-language suggestions, such as Copilot or CodeRabbit
• Human reviewer — non-bot reviewer
• Maintainer — repository owner/member/maintainer when that can be inferred from GitHub metadata

4. Present Findings

Present two separate sections:

Human Comments

Table ordered by:

1. Bugs / correctness issues — reviewer identified broken logic or missing error handling
2. Design / architecture feedback — structural changes, API shape, naming of public interfaces
3. Style / nits — formatting, naming of local variables, minor readability

Use this table format:

For automated reviewers, use the same table and set Reviewer to the tool account, with Priority based on the substance of the request.

Bot Comments (batched)

Table continuing the numbering from above, grouped by tool/bot:

Keep table cells short and scannable. Use the smallest useful verbatim quote, not the full comment body. Escape | characters inside code or text so the table remains valid Markdown.

End with a summary: total human comments, total bot comments, overall assessment of effort.

Do not stop for mode selection. Proceed by default with bot comments and human comments marked Autofix eligible. Mark a human comment Autofix eligible only when the requested change is source-backed, high confidence, minimal, unambiguous, does not require a product/design decision, does not add a dependency, does not change a shared/public interface, and has a clear verification path.

Leave all other human comments unresolved as Needs decision, with the exact decision needed. Do not reject a reviewer comment by default; rejection requires a user-provided public rationale.

Before applying any fixes, record the starting commit:

Choose an artifact directory using the AGENTS.md temporary artifact rule with agent name pfleidi-pr-feedback:

• Use ./tmp/pfleidi-pr-feedback/ only when ./tmp/ already exists and is already ignored.
• If no project-local artifact directory is available, do not create file artifacts by default; keep ledger/log/cache information in the response and mark file paths n/a. Ask before using /tmp/pfleidi-pr-feedback/ or modifying ignore files.

When an artifact directory is available, create a temporary thread ledger at <artifact-dir>/pr-feedback-<pr-number>.md. If no artifact directory is available, keep the same ledger fields in the final summary table instead. Update the ledger after each thread with:

• Thread ID, source category, reviewer, location, and status.
• Files touched.
• What changed and why.
• Related tests or verification commands.
• Planned public reply, if any.
• Resolve decision: yes/no and why.

5. Fix Bot Comments (batched)

Fix all bot comments first — these are mechanical and clearing them reduces noise before the human-comment phase.

1. For each bot finding:
   • Read the relevant code
   • Implement the fix — ONLY the changes needed for that single finding
   • Track the files changed for this finding so the final PR reply can identify the commit that contains the fix
   • If a fix is ambiguous or would conflict with a human-comment fix already applied, mark it Needs decision and continue
2. After all bot fixes are applied, present a summary table. Do NOT show a diff — the Edit tool already showed each change inline.

1. Proceed directly to Step 6.

6. Fix Human Comments (batched)

After bot fixes, work through Autofix eligible human comments in report order:

1. State which finding you are addressing (number and one-line description)
2. Read the relevant code and the full comment thread to understand intent
3. Re-check eligibility before editing; if the fix is no longer clearly eligible, mark it Needs decision and continue
4. Implement the fix — ONLY the changes needed for that single finding
5. Track the files changed for this finding so the final PR reply can identify the commit that contains the fix
6. If a comment needs a product/design decision, shared/public interface change, dependency, broad refactor, or has multiple reasonable fixes, mark it Needs decision and continue
7. If the user rejects the comment instead of fixing it, record the specific rationale to use in the final PR reply

Scope Rules

• Make the MINIMAL change that addresses the reviewer's feedback
• Keep the diff limited to files and lines directly required by the feedback
• First decide whether the feedback points to a local or systemic issue. Fix at the narrowest correct level; do not add a local workaround that hides a shared/root-cause bug.
• If the feedback requires a behavior-changing code fix, add or update the directly related test in the same fix. Prefer TDD, but complete the focused red-to-green cycle before stopping: write/update the failing test, confirm it fails, implement the fix, confirm the focused test passes. Do not stop after only adding the failing test unless the user explicitly asks.
• Do NOT rename variables, reformat code, or touch lines outside the feedback scope
• Do NOT refactor adjacent code, even if it looks related
• If the reviewer's comment is ambiguous, mark it Needs decision and continue with unrelated unambiguous comments
• Do NOT create any git commits during the fix cycle. Commits are handled only in the publish step, and only with explicit user approval when needed.

7. Verify Fixes

After all fixes are applied, run the project's lint and test commands scoped to only the changed files and their directly related tests. If no code changed, skip verification and proceed to Step 8. Use safe background batches for independent validators instead of running every command sequentially.

When selecting verification commands, reuse <artifact-dir>/verification-<repo-name>.md if an artifact directory is available and the cache is fresh under the cache rules from pfleidi:pr; otherwise discover the smallest relevant lint/test/build commands. Update the cache only when an artifact directory is available.

• Lint / static analysis — run the project's documented lint task, scoped to the files that were modified when the task supports scoping. Prefer lint-specific task wrappers such as make lint or mise run lint over invoking linter binaries directly. Do not use aggregate check, ci, or verify tasks unless you have confirmed they only run lint/static analysis. If the documented lint task cannot be scoped, run the smallest relevant project lint task.
• Tests — run only the test files that cover the modified code (same package, same module, co-located test files). Do NOT run the full test suite.

If no project lint task exists, state that explicitly instead of assuming an unavailable linter binary.

Run formatters, generators, snapshot updates, or other mutating commands alone before validators that depend on their output. Run independent read-only validators concurrently when they do not require the same exclusive service, port, database, fixture directory, or generated output. Keep integration/e2e/service-backed commands separate unless the project documents that they are parallel-safe.

For each background batch, start every command from the same working-tree state, capture stdout/stderr/exit status from the tool, do not edit files while the batch is running, and wait for every command to finish. Run each selected validator directly, for example mise run lint, go test ..., or npm test -- .... Do not wrap validators in sh -c, shell redirection, tee, command separators, or pipelines solely to write logs; that defeats command-prefix approvals and causes extra permission prompts. If an artifact directory is available and file logs can be written after the command completes without rerunning through a shell wrapper, save them under <artifact-dir>/logs-<pr-number>-<timestamp>/; otherwise mark the full-log path as n/a. If files change after a failed batch, none of that batch's successful results count as current verification.

Show verification as a compact table:

For failures or short outputs, show complete output in the relevant-output column or immediately below the table. For long successful outputs, show the relevant excerpt and log path.

If lint or tests fail due to issues introduced by the fixes:

1. Read the error output and identify every failure
2. Fix all issues — apply the minimal changes needed
3. Re-run the failing commands using the same safe batching rules
4. Show the complete output again

Cap at 2 fix attempts. If still failing after 2 rounds, present the remaining failures to the user with full output.

Once verification passes, show a summary: how many comments were addressed, rejected, intentionally left unresolved, or still blocked. Do NOT show a diff — the Edit tool already showed each change inline.

Proceed to Step 8 for threads that were addressed or intentionally rejected. Leave Needs decision threads unresolved and do not reply to them unless the user provided a public rejection rationale. Do not block publishing addressed threads just because unrelated threads still need a decision.

8. Publish PR Updates

After addressed/rejected threads are ready to publish:

1. Check branch state:

2. If there are uncommitted fix changes, STOP and ask the user whether to commit them now or let the user commit manually. Do not push until the fixes are committed. If the user approves committing, stage only files changed for the PR feedback fixes and write the commit message from the actual diff using the subject-plus-context style from AGENTS.md.

3. Push the committed changes for the current branch:

   If the branch has no upstream and the push fails for that reason, use:

   Never force-push.

4. Map each addressed finding to the commit or commits that contain its fix. Use the recorded starting commit, changed-file tracking, ledger, and git log / git show to identify the relevant short SHA(s). If one commit fixes multiple comments, reference the same commit in each reply.

5. Build and show a reply plan table before calling the API:

   Thread	Status	Reply body	Resolve
   PRRT_...	Addressed	Addressed in abc1234 by adding the nil check before dereferencing.	Yes
   PRRT_...	Needs decision	n/a	No

   Proceed without asking when every actionable reply row is either addressed or a user-approved rejection. Needs decision rows with Reply = n/a and Resolve = No do not block publishing addressed threads. Stop before API calls if a rejection lacks a public rationale or if any reply body is uncertain.

6. Reply to each original PR review thread:

   • For addressed comments, state what changed and the commit SHA(s), e.g. Addressed in abc1234 by adding the nil check before dereferencing.

   • For rejected comments, state why it was not changed, e.g. Not changed: this path already returns before allocation failure can be observed.

   • Keep replies concise but specific. Do not over-explain or repeat the diff.

   • Do not reply to comments the user explicitly chose to leave unresolved.

   • Prefer replying with the review thread ID:

     If the thread cannot be replied to by GraphQL, use the original review comment ID with the REST reply endpoint.

7. Resolve each thread after replying:

   Do not resolve a thread that was not replied to, is still ambiguous, or still needs user approval.

8. Present a final summary table:

   Thread	Reviewer	Location	Status	Reply	Resolved
   PRRT_...	reviewer	file.go:42	Addressed	Yes	Yes
   PRRT_...	reviewer	file.go:99	Needs decision	No	No

   Include push result, branch, ledger path, rejected comments and rationale, and any comments intentionally left unresolved or not resolvable via API.

9. Post-Publish

• If the user asks for adjustments, apply them
• If the user asks to revert specific fixes, revert ONLY those changes
• Do NOT commit or push additional changes unless explicitly asked

commit and push and then resolve comment