Skip to content

fix(azure-devops): edited comments wake PR watchers #31876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spoukyii wants to merge 2 commits into from
Closed

fix(azure-devops): edited comments wake PR watchers #31876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd80aa7
test(azure-devops): an edited pull request comment carries its edit time
spoukyii Nov 6, 2026
1f13b78
fix(azure-devops): edited comments wake PR watchers
spoukyii Nov 6, 2026
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
35 changes: 35 additions & 0 deletions packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,41 @@ describe("decodeThreadsJson", () => {
expect(comments.map((comment) => comment.id)).toEqual(["4:1", "4:2", "4:3"]);
});

it("marks a comment whose text was edited, not one only otherwise updated", () => {
const comments = expectSuccess(
decodeThreadsJson(
asJson({
value: [
{
id: 5,
comments: [
{
id: 1,
content: "Edited remark.",
publishedDate: "2026-07-02T00:00:00Z",
lastUpdatedDate: "2026-07-02T03:00:00Z",
lastContentUpdatedDate: "2026-07-02T03:00:00Z",
},
{
id: 2,
content: "Updated remark.",
publishedDate: "2026-07-02T01:00:00Z",
lastUpdatedDate: "2026-07-02T04:00:00Z",
lastContentUpdatedDate: "2026-07-02T01:00:00Z",
},
],
},
],
}),
),
);

expect(comments.map((comment) => comment.editedAt)).toEqual([
"2026-07-02T03:00:00Z",
undefined,
]);
});

it("drops deleted threads and threads with nothing to show", () => {
const comments = expectSuccess(
decodeThreadsJson(
Expand Down
6 changes: 6 additions & 0 deletions packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ const RawThreadSchema = Schema.Struct({
content: Schema.optional(Schema.NullOr(Schema.String)),
author: Schema.optional(Schema.NullOr(RawIdentitySchema)),
publishedDate: Schema.optional(Schema.NullOr(Schema.String)),
// lastUpdatedDate moves on any change to the comment; this one only on its text.
lastContentUpdatedDate: Schema.optional(Schema.NullOr(Schema.String)),
isDeleted: Schema.optional(Schema.NullOr(Schema.Boolean)),
commentType: Schema.optional(Schema.NullOr(Schema.String)),
}),
Expand Down Expand Up @@ -301,6 +303,7 @@ export function decodeThreadsJson(
const path = toRepositoryPath(thread.threadContext?.filePath);
for (const comment of thread.comments ?? []) {
const publishedDate = trimmed(comment.publishedDate);
const editedAt = trimmed(comment.lastContentUpdatedDate);
if (
comment.isDeleted === true ||
comment.commentType?.trim().toLowerCase() === "system" ||
Expand All @@ -315,6 +318,9 @@ export function decodeThreadsJson(
author: toActor(comment.author),
body: comment.content ?? "",
createdAt: publishedDate,
...(editedAt !== null && Date.parse(editedAt) > Date.parse(publishedDate)

@aravhawk aravhawk Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets editedAt whenever lastContentUpdatedDate parses later than publishedDate. On a never-edited comment, does Azure's create response keep those two equal, or have you seen create payloads where lastContentUpdatedDate is a few ms later?

If create routinely lands with a later content date, every new comment would carry editedAt, and a watch that started in that tiny window would treat a brand-new remark as an edit. The > check and the new test cover the intentional edit vs otherwise-updated case well; asking because the verification note says there was no live Azure org to confirm create shapes.

@spoukyii spoukyii Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I haven't seen a live create payload, so I can't say for sure they're equal. Microsoft's samples leave the field out, as the body notes. What I could check is what an offset would do if Azure's create path does stamp lastContentUpdatedDate slightly after publishedDate:

  • The watch compares editedAt ?? createdAt against its watermark (pullRequestWatch.ts:112-118).
  • A new comment is already after the watermark, so it wakes the watch once, as it would have anyway. The watermark then moves to its editedAt, so it doesn't wake a second time.
  • An older comment stays below the watermark, so nothing changes.
  • The one leftover case is a watch that was already running before this change, whose newest comment has such an offset. That comment would wake it once more after the update.
  • No web or mobile client reads editedAt, so nothing is shown as "edited".

The other way round, a real edit is minutes or hours after publishedDate in practice. If you'd rather not depend on Azure keeping the two equal, I can require lastContentUpdatedDate to be more than a second after publishedDate. That costs nothing for real edits, and it would rule out any create-time jitter. Happy to add it if you prefer.

? { editedAt }
: {}),
Comment thread
spoukyii marked this conversation as resolved.
Resolved
url: null,
path,
reviewState: null,
Expand Down
Loading