Mr Fix

mr-fix

Turns code review feedback into reviewed, formatted, committed changes. Language- and environment-agnostic: it detects the forge, the container setup, and the toolchain before it touches anything.

Step 0 — Preflight (detect everything; assume nothing)

Run this block first. It answers every environmental question the rest of the skill depends on. Nothing below is hardcoded — if a check comes back empty, the skill adapts rather than failing.

# --- Forge: which platform, which CLI, is it authed? ---
git remote get-url origin
git branch --show-current
git status --porcelain

command -v glab && glab auth status 2>&1 | tail -2
command -v gh   && gh   auth status 2>&1 | tail -2

# --- Runner: is this project containerized? ---
ls docker-compose.yml docker-compose.yaml compose.yml compose.yaml 2>/dev/null
docker compose ps --services --status running 2>/dev/null

# --- Toolchain: what actually exists in this repo? ---
ls composer.json package.json vendor/bin/pint vendor/bin/php-cs-fixer 2>/dev/null
ls pyproject.toml uv.lock poetry.lock Pipfile requirements.txt setup.py tox.ini 2>/dev/null
ls -d .venv venv 2>/dev/null
command -v uv poetry pipenv hatch pdm rye conda 2>/dev/null
command -v oco

If pyproject.toml exists, read it — [tool.ruff], [tool.black], [tool.mypy], [tool.pytest.ini_options] tell you what the project actually uses, which beats guessing from what happens to be installed globally.

Derive four facts, then reuse them everywhere:

1. Platform + CLI. Remote host contains gitlab → GitLab/glab; contains github → GitHub/gh. Self-hosted GitLab often has a custom domain — if the host matches neither, check glab api /version against the remote host before asking.

If the matching CLI is missing or unauthenticated, say exactly which (glab not found vs glab found but not logged in) and give the one command that fixes it (glab auth login). Do not attempt to scrape the MR from the web.

2. Command runner. Determine the prefix for every command this skill runs. Call it $RUN. It composes in two layers — container first, then language environment.

Layer 1 — container:

• A compose file exists and services are running → find the app service (the one with composer.json, artisan, or pyproject.toml mounted; commonly app, php, web, api, or laravel.test under Sail — confirm from docker compose ps --services, don't guess). $RUN = docker compose exec <service>
• A compose file exists but nothing is running → ask whether to docker compose up -d or run on host. Don't decide this for the user.
• No compose file → this layer is empty.

Layer 2 — Python environment (only if the repo is Python). Pick the first that applies; the lockfile is stronger evidence than what's on PATH:

Inside a container, the venv is usually already active, so Layer 2 is often unnecessary — check with $RUN which python before stacking uv run on top of docker compose exec. Stacking both when the container already activates the venv is harmless with uv, but breaks with poetry in some images.

Once $RUN is fixed, never bypass it. Every later command in this skill is written as $RUN <command> and must be expanded with the detected prefix.

3. Toolchain. Resolve per ecosystem; skip and say so when a tool is absent.

Committer: oco if on PATH, else write the commit message directly.

For Python, prefer the project's configured entrypoints over your memory of them: [tool.pytest.ini_options] addopts may already set flags, and a Makefile or tox.ini target (make lint, make test) is usually the intended interface. Use it if it exists.

4. Working tree. If git status --porcelain is non-empty, show it and ask before touching anything — uncommitted work plus automated edits is how changes get lost. If the current branch isn't the MR's source branch (known after Step 1), warn and offer to check it out.

Step 1 — Resolve the MR

Parse the MR/PR number from the invocation: /mr-fix 298, /mr-fix MR ID 298, !298, #298, a full URL, or a bare number.

If no number was given, list open MRs authored by the current user on the current branch and offer them:

glab mr list --source-branch "$(git branch --show-current)"   # or: gh pr list --head "$(git branch --show-current)"

A single match → use it. Multiple or zero → ask.

Step 2 — Fetch unresolved comments only

This is the step that matters most. Raw note lists include resolved threads, system notes ("added 1 commit"), and the user's own replies. Filter all of those out. Acting on a resolved thread is worse than missing one.

GitLab — use the API directly, since glab mr note list does not expose resolved:

glab api "projects/:id/merge_requests/<IID>/discussions?per_page=100" \
  --paginate \
  | jq '[.[] | select(.notes[0].resolvable == true and .notes[0].resolved == false)
        | {file: .notes[0].position.new_path,
           line: .notes[0].position.new_line,
           author: .notes[0].author.username,
           body: .notes[0].body,
           discussion_id: .id}]'

GitHub:

gh pr view <NUMBER> --json reviews,comments
gh api "repos/{owner}/{repo}/pulls/<NUMBER>/comments" --paginate \
  | jq '[.[] | select(.in_reply_to_id == null) | {file: .path, line: .line, author: .user.login, body: .body}]'

If zero unresolved comments: say so plainly and stop. Do not invent work.

Step 3 — Classify each finding

Before planning, sort every comment into one of:

Never silently resolve a conflict by picking one side.

Step 4 — Present the plan, then wait

Group by file. For each finding show: line, reviewer, what they asked, what you'll change, and why.

app/Services/BookingService.php
  L42  @jdoe  "This N+1s on large result sets"
       → Eager-load `customer` in the base query.
  L88  @jdoe  "Magic number"
       → Extract 30 into a MAX_RETRIES class constant.  [nitpick]

resources/js/stores/useBookingStore.ts
  L15  @amara "Mutating state outside an action"
       → Move the assignment into `setFilters()`.

Then ask: "Implement this plan?" Stop and wait. Do not proceed on ambiguity or silence.

Step 5 — Implement

• One finding at a time, smallest diff that satisfies the comment
• Do not opportunistically refactor adjacent code
• If a fix turns out to require a change the reviewer did not ask for, surface it before writing it

Step 6 — Verify

Expand $RUN and the tool names from Step 0. The same skill runs on the host, in a container, under uv run, or under both — nothing below is language-specific until you substitute.

$RUN <format>     # pint --dirty   |   ruff format .   |   black .
$RUN <lint>       #                    ruff check --fix .
$RUN <types>      #                    mypy <changed-paths>
$RUN <test>       # php artisan test --filter=X   |   pytest <path>::<test> -q

Concretely, a uv project inside Docker becomes:

docker compose exec api uv run ruff check --fix .
docker compose exec api uv run pytest tests/test_bookings.py -q

...and a bare uv project on the host is just uv run pytest -q.

Run the narrowest test selection that covers the changed code, not the whole suite — then the full suite once, at the end, before committing.

Skip any check whose tool Step 0 didn't find — and say which you skipped. A silently skipped test suite is worse than no test suite. If ruff format and ruff check --fix are both available, run format after lint: lint autofixes can leave formatting artifacts.

If tests fail, fix and re-run. Do not commit red.

Step 7 — Self-review the diff (loop until clean)

Green tests do not mean the change is right. Before committing, review your own diff as if it were someone else's MR.

git diff

Scope the review deliberately. You are checking three things, in this order:

1. Did each original finding actually get addressed? Walk the Step 4 plan item by item and point at the hunk that satisfies it. A finding with no corresponding hunk is unaddressed, regardless of what the tests say.
2. Did the fix introduce a new problem? N+1s, unhandled nulls, swallowed exceptions, changed public signatures, altered behavior outside the reviewed lines, debug artifacts (dd(), dump(), console.log, print(), breakpoint()), commented-out code, secrets.
3. Is the diff minimal? Anything in the diff that no finding asked for is scope creep. Flag it.

Do not re-review the whole file, the whole codebase, or design decisions that predate this MR. That's how a two-comment fix becomes a refactor.

The loop

Classify what the self-review turns up:

If there are any Blocker or Should-fix findings:

1. State them plainly, grouped by file, with line references.
2. Return to Step 5, fixing only those findings — do not re-plan, do not revisit the reviewer's comments.
3. Re-run Step 6 (verify).
4. Re-run this step.

Iterate until the self-review yields nothing above Consider.

Bound the loop

Maximum three passes. If pass 3 still surfaces Blockers, stop. Do not attempt a fourth.

Report to the user: what's still broken, what you tried on each pass, and why you think it isn't converging. Three failed passes usually means one of:

• The reviewer's comment is ambiguous and you're solving the wrong problem
• Two findings genuinely conflict and Step 3 missed it
• The fix requires a design change outside the MR's scope

All three need a human, not another pass. Say which one you suspect.

Also stop early — before three passes — if you notice a pass undoing something a previous pass did. That's oscillation, not convergence, and one more iteration will not help.

Announce the outcome

Before moving to commit, state the result explicitly:

Self-review: pass 2 — clean.
  Addressed:  3/3 findings
  Fixed on pass 2:  leftover dd() in BookingService.php:51
  Consider (not acted on):  setFilters() could take a typed DTO — out of scope for this MR

Only then proceed.

Step 8 — Commit

One commit per logical group, not one commit for the whole MR.

git add <files-for-this-group>
oco --yes

Use oco only if Step 0 found it. If it's absent, or it fails (EMPTY_MESSAGE, 405, provider unreachable), do not retry more than once. Write the conventional commit directly:

git commit -m "fix(booking): eager-load customer to avoid N+1"

Tell the user oco was bypassed and why.

Step 9 — Close the loop

Print a summary: what was fixed, what was skipped and why, any drafted replies to questions.

Then offer — do not perform automatically:

• Push the branch
• Reply to each discussion thread with what changed
• Resolve the threads (glab api --method PUT ".../discussions/<id>?resolved=true")

Error handling