fix: address 6 bugbot review comments on PR #4
- GraphQL reviewThreads(first: 250) → first: 100 (GitHub max)
- noConflicts false when mergeable is UNKNOWN (not just CONFLICTING)
- updateIssue now handles labels and assignee (not just state/comment)
- Add res.on("error") handler in linearQuery to prevent crashes
- createIssue returns only actually-applied labels, not all requested
- Tests added/updated for all fixes (144 total across 3 plugins)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
ac44f62c0b
commit
4fee8a9627
|
|
@ -319,7 +319,7 @@ function createGitHubSCM(): SCM {
|
|||
`query=query($owner: String!, $name: String!, $number: Int!) {
|
||||
repository(owner: $owner, name: $name) {
|
||||
pullRequest(number: $number) {
|
||||
reviewThreads(first: 250) {
|
||||
reviewThreads(first: 100) {
|
||||
nodes {
|
||||
isResolved
|
||||
comments(first: 1) {
|
||||
|
|
@ -492,10 +492,10 @@ function createGitHubSCM(): SCM {
|
|||
// Conflicts / merge state
|
||||
const mergeable = (data.mergeable ?? "").toUpperCase();
|
||||
const mergeState = (data.mergeStateStatus ?? "").toUpperCase();
|
||||
const noConflicts = mergeable !== "CONFLICTING";
|
||||
if (!noConflicts) {
|
||||
const noConflicts = mergeable === "MERGEABLE";
|
||||
if (mergeable === "CONFLICTING") {
|
||||
blockers.push("Merge conflicts");
|
||||
} else if (mergeable === "UNKNOWN") {
|
||||
} else if (mergeable === "UNKNOWN" || mergeable === "") {
|
||||
blockers.push("Merge status unknown (GitHub is computing)");
|
||||
}
|
||||
if (mergeState === "BEHIND") {
|
||||
|
|
|
|||
|
|
@ -584,6 +584,16 @@ describe("scm-github plugin", () => {
|
|||
expect(result.blockers).toContain("Merge conflicts");
|
||||
});
|
||||
|
||||
it("reports UNKNOWN mergeable as noConflicts false", async () => {
|
||||
mockGh({ mergeable: "UNKNOWN", reviewDecision: "APPROVED", mergeStateStatus: "CLEAN", isDraft: false });
|
||||
mockGh([{ name: "build", state: "COMPLETED", conclusion: "SUCCESS" }]);
|
||||
|
||||
const result = await scm.getMergeability(pr);
|
||||
expect(result.noConflicts).toBe(false);
|
||||
expect(result.blockers).toContain("Merge status unknown (GitHub is computing)");
|
||||
expect(result.mergeable).toBe(false);
|
||||
});
|
||||
|
||||
it("reports draft status as blocker", async () => {
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "DRAFT", isDraft: true });
|
||||
mockGh([{ name: "build", state: "COMPLETED", conclusion: "SUCCESS" }]);
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ async function linearQuery<T>(
|
|||
},
|
||||
(res) => {
|
||||
const chunks: Buffer[] = [];
|
||||
res.on("error", (err: Error) => settle(() => reject(err)));
|
||||
res.on("data", (chunk: Buffer) => chunks.push(chunk));
|
||||
res.on("end", () => {
|
||||
settle(() => {
|
||||
|
|
@ -392,6 +393,62 @@ function createLinearTracker(): Tracker {
|
|||
);
|
||||
}
|
||||
|
||||
// Handle assignee
|
||||
if (update.assignee) {
|
||||
const usersData = await linearQuery<{
|
||||
users: { nodes: Array<{ id: string; displayName: string; name: string }> };
|
||||
}>(
|
||||
`query($filter: UserFilter) {
|
||||
users(filter: $filter) {
|
||||
nodes { id displayName name }
|
||||
}
|
||||
}`,
|
||||
{ filter: { displayName: { eq: update.assignee } } },
|
||||
);
|
||||
|
||||
const user = usersData.users.nodes[0];
|
||||
if (user) {
|
||||
await linearQuery(
|
||||
`mutation($id: String!, $assigneeId: String!) {
|
||||
issueUpdate(id: $id, input: { assigneeId: $assigneeId }) {
|
||||
success
|
||||
}
|
||||
}`,
|
||||
{ id: issueUuid, assigneeId: user.id },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Handle labels
|
||||
if (update.labels && update.labels.length > 0) {
|
||||
const labelsData = await linearQuery<{
|
||||
issueLabels: { nodes: Array<{ id: string; name: string }> };
|
||||
}>(
|
||||
`query($teamId: ID) {
|
||||
issueLabels(filter: { team: { id: { eq: $teamId } } }) {
|
||||
nodes { id name }
|
||||
}
|
||||
}`,
|
||||
{ teamId },
|
||||
);
|
||||
|
||||
const labelMap = new Map(labelsData.issueLabels.nodes.map((l) => [l.name, l.id]));
|
||||
const labelIds = update.labels
|
||||
.map((name) => labelMap.get(name))
|
||||
.filter((id): id is string => id !== undefined);
|
||||
|
||||
if (labelIds.length > 0) {
|
||||
await linearQuery(
|
||||
`mutation($id: String!, $labelIds: [String!]!) {
|
||||
issueUpdate(id: $id, input: { labelIds: $labelIds }) {
|
||||
success
|
||||
}
|
||||
}`,
|
||||
{ id: issueUuid, labelIds },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Handle comment
|
||||
if (update.comment) {
|
||||
await linearQuery(
|
||||
|
|
@ -507,9 +564,15 @@ function createLinearTracker(): Tracker {
|
|||
);
|
||||
|
||||
const labelMap = new Map(labelsData.issueLabels.nodes.map((l) => [l.name, l.id]));
|
||||
const labelIds = input.labels
|
||||
.map((name) => labelMap.get(name))
|
||||
.filter((id): id is string => id !== undefined);
|
||||
const appliedLabels: string[] = [];
|
||||
const labelIds: string[] = [];
|
||||
for (const name of input.labels) {
|
||||
const id = labelMap.get(name);
|
||||
if (id) {
|
||||
labelIds.push(id);
|
||||
appliedLabels.push(name);
|
||||
}
|
||||
}
|
||||
|
||||
if (labelIds.length > 0) {
|
||||
await linearQuery(
|
||||
|
|
@ -520,8 +583,8 @@ function createLinearTracker(): Tracker {
|
|||
}`,
|
||||
{ id: node.id, labelIds },
|
||||
);
|
||||
// Reflect the labels we added
|
||||
issue.labels = input.labels;
|
||||
// Reflect only the labels that actually exist in Linear
|
||||
issue.labels = appliedLabels;
|
||||
}
|
||||
} catch {
|
||||
// Labels are best-effort; don't fail the whole creation
|
||||
|
|
|
|||
|
|
@ -571,6 +571,47 @@ describe("tracker-linear plugin", () => {
|
|||
);
|
||||
expect(requestMock).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it("updates assignee by resolving display name to ID", async () => {
|
||||
// 1: resolve identifier
|
||||
mockLinearAPI({ issue: { id: "uuid-123", team: { id: "team-1" } } });
|
||||
// 2: user lookup
|
||||
mockLinearAPI({
|
||||
users: { nodes: [{ id: "user-1", displayName: "Alice", name: "Alice Smith" }] },
|
||||
});
|
||||
// 3: issueUpdate (assignee)
|
||||
mockLinearAPI({ issueUpdate: { success: true } });
|
||||
|
||||
await tracker.updateIssue!("INT-123", { assignee: "Alice" }, project);
|
||||
expect(requestMock).toHaveBeenCalledTimes(3);
|
||||
|
||||
const writeCall = requestMock.mock.results[2].value.write.mock.calls[0][0];
|
||||
const body = JSON.parse(writeCall);
|
||||
expect(body.variables.assigneeId).toBe("user-1");
|
||||
});
|
||||
|
||||
it("updates labels by resolving names to IDs", async () => {
|
||||
// 1: resolve identifier
|
||||
mockLinearAPI({ issue: { id: "uuid-123", team: { id: "team-1" } } });
|
||||
// 2: label lookup
|
||||
mockLinearAPI({
|
||||
issueLabels: {
|
||||
nodes: [
|
||||
{ id: "label-1", name: "bug" },
|
||||
{ id: "label-2", name: "urgent" },
|
||||
],
|
||||
},
|
||||
});
|
||||
// 3: issueUpdate (labels)
|
||||
mockLinearAPI({ issueUpdate: { success: true } });
|
||||
|
||||
await tracker.updateIssue!("INT-123", { labels: ["bug", "urgent"] }, project);
|
||||
expect(requestMock).toHaveBeenCalledTimes(3);
|
||||
|
||||
const writeCall = requestMock.mock.results[2].value.write.mock.calls[0][0];
|
||||
const body = JSON.parse(writeCall);
|
||||
expect(body.variables.labelIds).toEqual(["label-1", "label-2"]);
|
||||
});
|
||||
});
|
||||
|
||||
// ---- createIssue -------------------------------------------------------
|
||||
|
|
@ -681,6 +722,31 @@ describe("tracker-linear plugin", () => {
|
|||
expect(body.variables.labelIds).toEqual(["label-1", "label-2"]);
|
||||
});
|
||||
|
||||
it("only reflects actually-applied labels when some don't exist", async () => {
|
||||
mockLinearAPI({
|
||||
issueCreate: {
|
||||
success: true,
|
||||
issue: { ...sampleIssueNode, labels: { nodes: [] } },
|
||||
},
|
||||
});
|
||||
// Only "bug" exists in Linear; "nonexistent" does not
|
||||
mockLinearAPI({
|
||||
issueLabels: {
|
||||
nodes: [
|
||||
{ id: "label-1", name: "bug" },
|
||||
],
|
||||
},
|
||||
});
|
||||
mockLinearAPI({ issueUpdate: { success: true } });
|
||||
|
||||
const issue = await tracker.createIssue!(
|
||||
{ title: "Bug", description: "", labels: ["bug", "nonexistent"] },
|
||||
project,
|
||||
);
|
||||
// Should only include the label that was actually found and applied
|
||||
expect(issue.labels).toEqual(["bug"]);
|
||||
});
|
||||
|
||||
it("throws when teamId is missing from config", async () => {
|
||||
const noTeam: ProjectConfig = {
|
||||
...project,
|
||||
|
|
|
|||
Loading…
Reference in New Issue