Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DiffPalReviewDevV1/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"version": {
"Major": 1,
"Minor": 6,
"Patch": 8
"Patch": 9
},
"minimumAgentVersion": "3.224.0",
"instanceNameFormat": "DiffPal Review Dev",
Expand All @@ -28,7 +28,7 @@
"type": "string",
"label": "DiffPal version",
"required": false,
"defaultValue": "latest",
"defaultValue": "0.1.31",
"helpMarkDown": "npm version or dist-tag for @diffpal/diffpal."
},
{
Expand Down
4 changes: 2 additions & 2 deletions DiffPalReviewV1/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"version": {
"Major": 1,
"Minor": 6,
"Patch": 8
"Patch": 9
},
"minimumAgentVersion": "3.224.0",
"instanceNameFormat": "DiffPal Review",
Expand All @@ -28,7 +28,7 @@
"type": "string",
"label": "DiffPal version",
"required": false,
"defaultValue": "latest",
"defaultValue": "0.1.31",
"helpMarkDown": "npm version or dist-tag for @diffpal/diffpal."
},
{
Expand Down
14 changes: 9 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ VSIX packaging, and Marketplace release flow.

The task installs `@diffpal/diffpal` by default and runs `diffpal review ado`.
Bring the provider recipe you want to use; the Azure review flow stays the same.
By default it installs `@diffpal/diffpal@latest`, so pin `diffpalVersion` when
you need rollout-safe CLI behavior.
By default it installs `@diffpal/diffpal@0.1.31`, the tested CLI release paired
with this extension. Set `diffpalVersion` only when you need to override that
default rollout.

## Behavior

Expand All @@ -39,8 +40,8 @@ base/head, and redacted CLI arguments before the review starts.

With `feedback: balanced` or `feedback: inline`, DiffPal publishes Azure threads
for all findings. Blocking findings stay active; non-blocking findings are
published as closed immediately. Findings without canonical file/line mapping
fall back to non-file PR threads.
published as closed immediately. Findings without canonical file/line mapping to
current PR changes are skipped instead of publishing a broken file thread.

## Examples

Expand All @@ -66,14 +67,17 @@ steps:
- task: DiffPalReview@1
displayName: DiffPal review
inputs:
diffpalVersion: latest
diffpalVersion: 0.1.31
profile: ci
feedback: balanced
env:
OPENAI_API_KEY: $(OPENAI_API_KEY)
SYSTEM_ACCESSTOKEN: $(System.AccessToken)
```

When blocking findings are present, the task fails with a human-readable gate
message and preserves the non-zero DiffPal exit code for pipeline control.

### Blocking gate

```yaml
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@
"publish:prod": "npm run build \u0026\u0026 tfx extension publish --manifest-globs vss-extension.json",
"smoke": "npm run build \u0026\u0026 node scripts/smoke-test.js"
},
"version": "0.1.30"
"version": "0.1.31"
}
69 changes: 68 additions & 1 deletion scripts/smoke-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,16 @@ function read(file) {
return fs.readFileSync(file, "utf8");
}

function makeFakeDiffPal(file, argvFile) {
function makeFakeDiffPal(file, argvFile, options = {}) {
const exitCode = options.exitCode ?? 0;
const stdout = options.stdout ?? "";
const stderr = options.stderr ?? "";
writeExecutable(file, `#!/usr/bin/env bash
set -euo pipefail
printf '%s\\n' "$@" > "${argvFile}"
printf '%b' ${JSON.stringify(stdout)}
printf '%b' ${JSON.stringify(stderr)} >&2
exit ${exitCode}
`);
}

Expand Down Expand Up @@ -134,6 +140,25 @@ function testDefaultInstall() {
assert(read(diffpalArgv).includes("review\nado"), "default install did not run diffpal review ado");
}

function testDefaultPinnedVersionIsUsedWhenInputIsUnset() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "diffpal-ado-default-version-"));
const fakeBin = path.join(dir, "bin");
const npmArgv = path.join(dir, "npm-argv");
const diffpalArgv = path.join(dir, "diffpal-argv");
const agentTemp = path.join(dir, "agent-temp");
fs.mkdirSync(agentTemp, { recursive: true });
makeFakeNpm(path.join(fakeBin, "npm"), npmArgv, diffpalArgv);

runHandler("default pinned version", {
AGENT_TEMPDIRECTORY: agentTemp,
INPUT_INSTALL: "true",
PATH: `${fakeBin}${path.delimiter}${process.env.PATH || ""}`
});

assert(read(npmArgv).includes("@diffpal/diffpal@0.1.31"), "default install did not request the pinned DiffPal version");
assert(read(diffpalArgv).includes("review\nado"), "default pinned version did not run diffpal review ado");
}

function testCustomPathSkipsInstall() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "diffpal-ado-custom-"));
const fakeBin = path.join(dir, "bin");
Expand Down Expand Up @@ -475,7 +500,47 @@ function testExplainPrintsResolvedContext() {
assert(!result.stdout.includes("secret-token-value"), "explain output should redact secrets");
}

function testGateFailureUsesHumanMessageForReviewBlockedExitCode() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "diffpal-ado-gate-blocked-"));
const diffpalArgv = path.join(dir, "diffpal-argv");
const customDiffPal = path.join(dir, "diffpal");
makeFakeDiffPal(customDiffPal, diffpalArgv, {
exitCode: 10,
stderr: "review blocked: blocking findings detected: 2\n"
});

const result = runHandlerExpectFailure("gate blocked exit code", {
INPUT_INSTALL: "false",
INPUT_DIFFPALPATH: customDiffPal,
INPUT_GATE: "true"
});

assert(result.status === 10, `gate blocked exit code should be preserved, got ${result.status}`);
assert(result.stdout.includes("DiffPal code review found blocking issues at or above the high threshold."), "gate failure should be human-readable");
assert(!result.stdout.includes("diffpal exited with code 10"), "gate failure should not fall back to the generic exit code message");
}

function testNonGateFailureStaysGeneric() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "diffpal-ado-generic-failure-"));
const diffpalArgv = path.join(dir, "diffpal-argv");
const customDiffPal = path.join(dir, "diffpal");
makeFakeDiffPal(customDiffPal, diffpalArgv, {
exitCode: 4,
stderr: "platform publish failed\n"
});

const result = runHandlerExpectFailure("generic failure", {
INPUT_INSTALL: "false",
INPUT_DIFFPALPATH: customDiffPal,
INPUT_GATE: "true"
});

assert(result.status === 4, `generic failure exit code should be preserved, got ${result.status}`);
assert(result.stdout.includes("diffpal exited with code 4"), "non-gate failure should keep the generic task message");
}

testDefaultInstall();
testDefaultPinnedVersionIsUsedWhenInputIsUnset();
testCustomPathSkipsInstall();
testInstallDisabledUsesPath();
testDefaultInstructionsFileDirectoryIsIgnored();
Expand All @@ -492,4 +557,6 @@ testPrContextComputesMergeBase();
testExplicitRangeBypassesGitResolution();
testMissingTargetRefExplainsFetchFailure();
testExplainPrintsResolvedContext();
testGateFailureUsesHumanMessageForReviewBlockedExitCode();
testNonGateFailureStaysGeneric();
console.log("Azure DevOps task smoke tests passed");
51 changes: 45 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type ReviewRange = {
pullRequest: PullRequestContext;
};

const DEFAULT_DIFFPAL_VERSION = "0.1.31";
const REVIEW_BLOCKED_EXIT_CODE = 10;

function input(name: string): string {
return (tl.getInput(name, false) ?? "").trim();
}
Expand Down Expand Up @@ -233,6 +236,34 @@ function spawnCommand(command: string, args: string[]): Promise<number> {
});
}

function spawnCommandWithCapture(command: string, args: string[]): Promise<CommandResult> {
return new Promise((resolve, reject) => {
const child = spawn(command, args, {
env: process.env,
shell: false,
stdio: ["ignore", "pipe", "pipe"]
});
let stdout = "";
let stderr = "";
child.stdout.on("data", (chunk: Buffer) => {
const text = chunk.toString("utf8");
stdout += text;
process.stdout.write(text);
});
child.stderr.on("data", (chunk: Buffer) => {
const text = chunk.toString("utf8");
stderr += text;
process.stderr.write(text);
});
child.on("error", reject);
child.on("close", (code) => resolve({
code: code ?? 1,
stdout,
stderr
}));
});
}

function runCommand(command: string, args: string[]): Promise<CommandResult> {
return new Promise((resolve, reject) => {
const child = spawn(command, args, {
Expand All @@ -257,6 +288,10 @@ function runCommand(command: string, args: string[]): Promise<CommandResult> {
});
}

function isReviewBlockedFailure(code: number): boolean {
return code === REVIEW_BLOCKED_EXIT_CODE;
}

async function fetchTargetBranch(targetBranch: string): Promise<void> {
if (!targetBranch) {
return;
Expand Down Expand Up @@ -349,7 +384,7 @@ async function installDiffPal(version: string): Promise<string> {
const installRoot = path.join(tempDir, "diffpal-task");
fs.mkdirSync(installRoot, { recursive: true });

const packageSpec = `@diffpal/diffpal@${version || "latest"}`;
const packageSpec = `@diffpal/diffpal@${version || DEFAULT_DIFFPAL_VERSION}`;
tl.debug(`Installing ${packageSpec} into ${installRoot}`);
const code = await spawnCommand(npm, [
"install",
Expand Down Expand Up @@ -392,7 +427,7 @@ async function resolveDiffPalCommand(): Promise<string> {
if (!boolInput("install", true)) {
return tl.which(diffpalPath, true);
}
return installDiffPal(input("diffpalVersion") || "latest");
return installDiffPal(input("diffpalVersion") || DEFAULT_DIFFPAL_VERSION);
}

async function run(): Promise<void> {
Expand Down Expand Up @@ -431,10 +466,14 @@ async function run(): Promise<void> {
}

tl.debug(`Running ${command} ${args.join(" ")}`);
const code = await spawnCommand(command, args);
if (code !== 0) {
process.exitCode = code;
tl.setResult(tl.TaskResult.Failed, `diffpal exited with code ${code}`);
const result = await spawnCommandWithCapture(command, args);
if (result.code !== 0) {
process.exitCode = result.code;
if (gate && isReviewBlockedFailure(result.code)) {
tl.setResult(tl.TaskResult.Failed, `DiffPal code review found blocking issues at or above the ${blockOn} threshold.`);
return;
}
tl.setResult(tl.TaskResult.Failed, `diffpal exited with code ${result.code}`);
return;
}
tl.setResult(tl.TaskResult.Succeeded, "DiffPal review completed");
Expand Down
2 changes: 1 addition & 1 deletion vss-extension.dev.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@
"id": "Microsoft.VisualStudio.Services"
}
],
"version": "0.1.30"
"version": "0.1.31"
}
2 changes: 1 addition & 1 deletion vss-extension.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@
"id": "Microsoft.VisualStudio.Services"
}
],
"version": "0.1.30"
"version": "0.1.31"
}