Conversation
) - Add `scripts/lib/remote-match.mjs`: pure helper exporting `remotesMatch(adoRemoteUrl, localRemoteUrls)` with ADO-specific HTTPS/SSH identity extraction (dev.azure.com, ssh.dev.azure.com, *.visualstudio.com), .git suffix stripping, credential removal, host-casing normalisation, and a CLI entry point for agent use - Add `tests/remote-match.test.mjs`: full normalisation matrix (ADO HTTPS↔SSH, .git, casing, credentials, multi-remote, empty list, generic hosts) - Update `agents/ado-fetcher.md`: rewrite Step 5 non-re-review branch to compute the merge-base diff via `git fetch origin` + `git diff <commonRefCommit> <sourceRefCommit> --unified=3`, gated by the repo-match guard; correct frontmatter description; update Step 6 diffUnavailable comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- remote-match.mjs: parse raw `git remote -v` stdout (eliminates awk/jq/sort deps) - remote-match.mjs: add null-guard on adoRemoteUrl + typeof string guard for array elements - remote-match.mjs: use `original` in normalise() catch to avoid returning expanded form - remote-match.mjs: fix catch-block comment and normalise() JSDoc return description - remote-match.mjs: wrap CLI in main().catch() per codebase pattern - ado-fetcher.md: drop Bash(awk *) and Bash(sort *) from allowed-tools - ado-fetcher.md: add explicit ADO_REMOTE_URL assignment in Step 5a - ado-fetcher.md: guard REMOTES_MATCH on exit code as well as value - ado-fetcher.md: add SOURCE_REF_COMMIT guard in Step 5b - ado-fetcher.md: switch git fetch origin → git fetch --all (fixes hardcoded remote name) - ado-fetcher.md: add explicit git fetch failure warning in Step 5c - tests: add ADO path-component casing, full ssh:// URI, catch-path, and null-guard tests - README.md: split first-review / Pre-PR flowchart nodes; update Quick Start description Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three robustness fixes to the first-review diff branch, all flagged by Copilot on PR #198: - Step 5a referenced $PR_METADATA_JSON, but Step 1 stores PR_METADATA; corrected so ADO_REMOTE_URL is actually populated. - REMOTES_MATCH command substitution swallowed a non-zero exit, letting a tool failure pass as a "false" no-match. Added `|| REMOTES_MATCH=error` and made the guard fail-closed: only the literal `true` is a match. - Reverted `git fetch --all` to `git fetch origin` to match ADR-0012 and the re-review path; --all contacts every remote and diverges from the documented checkout-free sequence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… behavior From the multi-agent review of PR #198: - Step 5a repo-match guard: add `set -o pipefail` so a `git remote -v` failure propagates and `|| REMOTES_MATCH=error` fires as documented. Without it the pipeline exit status was the helper's alone, so a git failure (with the helper printing `false`) was swallowed; the prose claimed a guarantee the snippet did not provide. Outcome was already fail-closed, but the rationale is now true. - Step 6 footer: "git failure" -> "git diff failure" — git fetch failure is non-fatal (warn-and-continue), only git diff failure forces diffUnavailable. - Add three regression tests to remote-match.test.mjs locking in current correct behavior of the security-gate helper: cross-form ADO different-repo negative, ADO/generic namespace no-collision, and generic trailing-slash normalisation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…port
Step 5a resolved the ADO remote URL with `jq -r`, and this PR had added
`Bash(jq *)` to allowed-tools. jq is not bundled with git-bash on
Windows, violating the cross-platform requirement in CLAUDE.md ("use
Node.js APIs instead of shell commands"). The original review flagged
awk/jq/sort; the earlier self-fix removed awk/sort but kept jq.
Replace the jq extraction with an inline `node -e` reading PR_METADATA
from stdin (consistent with the node-helper pattern at lines 80 and 162),
and drop `Bash(jq *)` from allowed-tools. A missing repository.remoteUrl
yields an empty string, which the repo-match guard treats as no-match and
falls back to diffUnavailable — fail-closed, unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hanged files git diff exits zero on success whether or not it finds differences, so the Step 5c "otherwise" branch could set RAW_DIFF="" with diffUnavailable=false when the diff was empty for the wrong reason (a silent git fetch failure leaving stale commits, both SHAs resolving to the same tree, or a shallow clone collapsing the refs). Review agents would then receive an empty diff plus a "diff available" flag and report a clean PR — the worst-case false negative for a review tool. Cross-check the empty diff against CHANGED_FILES: an empty diff with a non-empty changedFiles is treated as diffUnavailable=true (+ warning) so the structural changedFiles review (#176) runs instead of a false clean; an empty diff with empty changedFiles stays diffUnavailable=false (genuinely no changes). Footer diffUnavailable contract updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Step 5a invoked the helper as a bare relative path
`node scripts/lib/remote-match.mjs`. But the repo-match guard runs with
the user's clone as the working directory (ADR-0012: the agent runs from
inside a clone of the PR repo, so `git remote -v`/`git fetch`/`git diff`
inspect that clone). With cwd at the user's clone, a path relative to cwd
cannot find the plugin's script, so `node` errored, the guard failed
closed, and first-review fell back to diffUnavailable on every real run —
the feature never produced a diff in practice.
Reference the helper via `${CLAUDE_PLUGIN_ROOT}`, matching every other
script invocation in the plugin (intent-checker, ado-writer, review-pr,
the setup commands). The pre-existing bare-relative call at line 80
(parse-prior-signature.mjs) is the same latent bug but lives on develop;
left for a separate fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lise normalise() ran `.replace(/\.git$/, '').replace(/\/+$/, '')`, so a path ending in `.git/` kept its `.git` — the `\.git$` anchor can't match when a trailing slash follows. `https://github.com/org/repo.git/` therefore failed to match `https://github.com/org/repo`, defeating normalisation for that shape. Swap the order (strip trailing slashes first, then `.git`) in both the generic and the unparseable-URL fallback branches, and align the catch branch to strip multiple trailing slashes like the generic path. Adds a regression test for the combined `repo.git/` form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ggers The Step 5 churn left two docs trailing the implementation: - ado-fetcher.md Step 6 footer listed four diffUnavailable:true triggers but Step 5b also sets it on a missing sourceRefCommit; add that case. - CHANGELOG 2.1.2 listed three fallback cases, omitting the empty-diff guard (the headline safety behaviour) and the sourceRefCommit guard; list all of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…space Two low-severity precision fixes plus CLI coverage, from the high-effort review: - adoIdentity matched ADO paths with `parts.length >= N`, so a remote with extra trailing segments (e.g. .../_git/r/extra) normalised to the same token as the clean URL. Strip leading/trailing slashes then require an exact segment count, so trailing junk no longer collapses to a match. - The unparseable-URL fallback returned the raw string verbatim, which could in principle forge an `ado:` identity token and false-positive match. Tag it `raw:` so the ado:/generic/raw namespaces are disjoint. - Add CLI coverage (execFileSync, mirroring parse-prior-signature.test): true/false output, empty stdin, and the missing-arg usage exit. All fail-safe to diffUnavailable; none could produce a wrong diff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ROOT
ado-fetcher.md Step 3a invoked `node scripts/parse-prior-signature.mjs`
bare-relative — the same path bug as the first-review helper, but on the
re-review path and pre-existing on develop. With the user's clone as cwd
the relative path does not resolve, breaking prior-signature parsing.
Reference it via ${CLAUDE_PLUGIN_ROOT} like every other script call.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ADR-0012 now documents that the Step 5 diffUnavailable/rawDiff branching lives as agent prose rather than an extracted tested helper, states the accepted risk (model-interpreted, untested, false-clean failure mode), and names the three mitigating layers plus the extraction follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tsc --checkJs types a catch binding as `unknown`, so `err.status` raised
TS18046 (caught by CI typecheck, not the test run). Cast to
`{ status?: number }` before reading the exit status.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…196-checkout-free-first-rev feat(unic-pr-review): compute checkout-free first-review line diff
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.