TimZander / TimZander/claude

review-pr: reuse resolve-pr.sh for ADO remote parsing instead of re-deriving org/project/repo

Open
#225 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Follow-up from merging #208.

plugins/review-pr/commands/review-pr.md Step 1.3 re-derives org, project, and repo name from the origin remote by walking four ADO remote dialects inline in markdown. plugins/deep-review/scripts/resolve-pr.sh already does the host-component and org-URL derivation (remote_host, ado_org_url, origin_repo_name), with the subtle ordering fix that vs-ssh.visualstudio.com must be matched before the generic visualstudio.com arm or the org resolves as vs-ssh.

Two copies of that parsing will drift, and the failure mode is quiet: a wrong project resolves the wrong repo GUID, and review-pr writes.

This also sits against the repo standard in CLAUDE.mdPlugin Logic: Prefer Scripts Over Inline Markdown. Most of review-pr.md is genuinely MCP orchestration and user gates, which can't live in a shell script and shouldn't be forced into one. The remote parsing is the part that can.

Proposal

Extract the shared derivation into a script review-pr owns (plugins are installed independently and must stand alone, so copy, don't source across plugins — the same reason resolve-pr.sh copied its host detection from craft-pr). Have it emit KEY=value lines the command reads:

ORG_URL=https://dev.azure.com/bgvone
PROJECT=BGV Development
REPO=BgvCore

Ship a smoke test alongside it covering the four dialects plus percent-decoding (BGV%20Development), per the standard.

Alternative worth considering

Add ORG_URL/PROJECT/REPO as new output keys on resolve-pr.sh itself and have review-pr invoke it. That keeps one implementation, but couples the two plugins at runtime — review-pr already requires deep-review to be installed, so it may be acceptable. Decide which before implementing.

Notes

  • Whichever way it goes, the vs-ssh ordering constraint needs a test in the new suite; it is currently only covered by deep-review's.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read plugins/review-pr/commands/review-pr.md Step 1.3 and plugins/deep-review/scripts/resolve-pr.sh, then inspect the repository’s smoke-test standard. Decide whether review-pr owns a copied parser or invokes resolve-pr.sh. Done means the chosen approach emits ORG_URL, PROJECT, and REPO, covers all four ADO remote dialects and percent-decoding, and tests vs-ssh.visualstudio.com ordering.

Written by the indexing model from the issue text.

Assessment

Tech stack
shell
Domain
tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.