fix(preview): only update preview comments written by the token's own identity

src/forge/gitea/api_tests.rs

@@ -111,22 +111,44 @@ fn publish_release_patches_draft_false() {
 }
 
 #[test]
-fn find_comment_matches_the_marker_on_the_issue_thread() {
-    let server = FakeServer::start(vec![Reply::json(
-        200,
-        json!([
-            { "id": 1, "body": "hello" },
-            { "id": 2, "body": "<!-- ferrflow --> preview" },
-        ]),
-    )]);
+fn find_comment_only_matches_the_tokens_own_comments() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([
+                { "id": 1, "body": "hello", "user": { "login": "me" } },
+                { "id": 2, "body": "<!-- ferrflow --> preview", "user": { "login": "me" } },
+                { "id": 3, "body": "<!-- ferrflow --> quoted", "user": { "login": "someone" } },
+            ]),
+        ),
+        Reply::json(200, json!({ "login": "me" })),
+    ]);
 
     let found = forge(&server).find_comment(8, "<!-- ferrflow -->").unwrap();
 
     assert_eq!(found, Some(2));
+    let requests = server.requests();
     assert_eq!(
-        server.only_request().path,
+        requests[0].path,
         "/repos/owner/repo/issues/8/comments?limit=50&page=1"
     );
+    assert_eq!(requests[1].path, "/user");
+}
+
+#[test]
+fn find_comment_matches_nothing_when_the_token_cannot_read_its_user() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([{ "id": 2, "body": "<!-- ferrflow -->", "user": { "login": "me", "type": "Bot" } }]),
+        ),
+        Reply::json(403, json!({})),
+    ]);
+
+    assert_eq!(
+        forge(&server).find_comment(8, "<!-- ferrflow -->").unwrap(),
+        None
+    );
 }
 
 #[test]

src/forge/github/api_tests.rs

@@ -284,24 +284,86 @@ fn a_transport_failure_on_auto_merge_uses_the_request_code() {
     assert_eq!(code(&err).as_deref(), Some("E3008"));
 }
 
+fn marked_comment(id: u64, login: &str, kind: &str) -> serde_json::Value {
+    json!({
+        "id": id,
+        "body": "<!-- ferrflow -->\npreview",
+        "user": { "login": login, "type": kind },
+    })
+}
+
 #[test]
-fn find_comment_returns_the_first_comment_carrying_the_marker() {
-    let server = FakeServer::start(vec![Reply::json(
-        200,
-        json!([
-            { "id": 1, "body": "looks good" },
-            { "id": 2, "body": "<!-- ferrflow -->\npreview" },
-            { "id": 3, "body": "<!-- ferrflow --> again" },
-        ]),
-    )]);
+fn find_comment_skips_a_foreign_comment_and_prefers_the_most_recent_own_one() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([
+                { "id": 1, "body": "looks good", "user": { "login": "me", "type": "User" } },
+                marked_comment(2, "me", "User"),
+                marked_comment(3, "me", "User"),
+                marked_comment(4, "someone", "User"),
+            ]),
+        ),
+        Reply::json(200, json!({ "login": "me" })),
+    ]);
 
     let found = forge(&server).find_comment(5, "<!-- ferrflow -->").unwrap();
 
-    assert_eq!(found, Some(2));
+    assert_eq!(found, Some(3));
+    let requests = server.requests();
     assert_eq!(
-        server.only_request().path,
+        requests[0].path,
         "/repos/owner/repo/issues/5/comments?per_page=100&page=1"
     );
+    assert_eq!(requests[1].path, "/user");
+}
+
+#[test]
+fn find_comment_is_none_when_only_someone_else_carries_the_marker() {
+    let server = FakeServer::start(vec![
+        Reply::json(200, json!([marked_comment(4, "someone", "User")])),
+        Reply::json(200, json!({ "login": "me" })),
+    ]);
+
+    assert_eq!(
+        forge(&server).find_comment(5, "<!-- ferrflow -->").unwrap(),
+        None
+    );
+}
+
+#[test]
+fn find_comment_with_an_integration_token_only_matches_bots() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([
+                marked_comment(2, "github-actions[bot]", "Bot"),
+                marked_comment(4, "someone", "User"),
+            ]),
+        ),
+        Reply::json(
+            403,
+            json!({ "message": "Resource not accessible by integration" }),
+        ),
+    ]);
+
+    let found = forge(&server).find_comment(5, "<!-- ferrflow -->").unwrap();
+
+    assert_eq!(found, Some(2));
+}
+
+#[test]
+fn find_comment_propagates_a_server_error_from_the_identity_lookup() {
+    let server = FakeServer::start(vec![
+        Reply::json(200, json!([marked_comment(2, "me", "User")])),
+        Reply::json(500, json!({})),
+    ]);
+
+    let err = forge(&server)
+        .find_comment(5, "<!-- ferrflow -->")
+        .unwrap_err();
+
+    assert!(format!("{err:#}").contains("token's user"), "{err:#}");
 }
 
 #[test]

src/forge/gitlab.rs

@@ -64,6 +64,29 @@ impl GitLabForge {
             what,
         )
     }
+
+    fn token_username(&self) -> Result<Option<String>> {
+        let response = self
+            .agent
+            .get(format!("{}/user", self.api_base))
+            .header(self.token_kind.header(), &self.token)
+            .header("User-Agent", "ferrflow")
+            .call();
+        match response {
+            Ok(mut response) => {
+                let user: serde_json::Value = response
+                    .body_mut()
+                    .read_json()
+                    .context("Failed to parse the token's user")?;
+                let username = user["username"]
+                    .as_str()
+                    .context("The token's user has no username")?;
+                Ok(Some(username.to_string()))
+            }
+            Err(ureq::Error::StatusCode(401 | 403)) => Ok(None),
+            Err(e) => Err(e).context("Failed to read the token's user"),
+        }
+    }
 }
 
 impl Forge for GitLabForge {
@@ -227,15 +250,22 @@ impl Forge for GitLabForge {
             mr_id
         );
         let notes = self.paginated_json_array(&base_url, "MR notes")?;
-        for note in notes {
-            if let Some(body) = note["body"].as_str()
-                && body.contains(marker)
-                && let Some(id) = note["id"].as_u64()
-            {
-                return Ok(Some(id));
-            }
+        let mut marked = notes
+            .iter()
+            .filter(|note| note["body"].as_str().is_some_and(|b| b.contains(marker)))
+            .peekable();
+        if marked.peek().is_none() {
+            return Ok(None);
         }
-        Ok(None)
+        let Some(username) = self.token_username()? else {
+            tracing::warn!(
+                "Warning: the token cannot read its own user, posting a new note instead of updating one"
+            );
+            return Ok(None);
+        };
+        Ok(marked
+            .find(|note| note["author"]["username"].as_str() == Some(username.as_str()))
+            .and_then(|note| note["id"].as_u64()))
     }
 
     fn create_comment(&self, mr_id: u64, body: &str) -> Result<()> {

src/forge/gitlab/api_tests.rs

@@ -197,7 +197,11 @@ fn find_comment_pages_through_notes_until_the_marker() {
         .collect();
     let server = FakeServer::start(vec![
         Reply::json(200, json!(first_page)),
-        Reply::json(200, json!([{ "id": 500, "body": "x <!-- ferrflow --> y" }])),
+        Reply::json(
+            200,
+            json!([{ "id": 500, "body": "x <!-- ferrflow --> y", "author": { "username": "me" } }]),
+        ),
+        Reply::json(200, json!({ "username": "me" })),
     ]);
 
     let found = forge(&server)
@@ -212,6 +216,45 @@ fn find_comment_pages_through_notes_until_the_marker() {
     );
 }
 
+#[test]
+fn find_comment_skips_notes_from_someone_else() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([
+                { "id": 9, "body": "<!-- ferrflow -->", "author": { "username": "someone" } },
+                { "id": 8, "body": "<!-- ferrflow -->", "author": { "username": "me" } },
+                { "id": 7, "body": "<!-- ferrflow -->", "author": { "username": "me" } },
+            ]),
+        ),
+        Reply::json(200, json!({ "username": "me" })),
+    ]);
+
+    let found = forge(&server)
+        .find_comment(15, "<!-- ferrflow -->")
+        .unwrap();
+
+    assert_eq!(found, Some(8));
+}
+
+#[test]
+fn find_comment_matches_nothing_when_the_token_cannot_read_its_user() {
+    let server = FakeServer::start(vec![
+        Reply::json(
+            200,
+            json!([{ "id": 8, "body": "<!-- ferrflow -->", "author": { "username": "me" } }]),
+        ),
+        Reply::json(401, json!({})),
+    ]);
+
+    assert_eq!(
+        forge(&server)
+            .find_comment(15, "<!-- ferrflow -->")
+            .unwrap(),
+        None
+    );
+}
+
 #[test]
 fn a_malformed_notes_page_is_an_error() {
     let server = FakeServer::start(vec![Reply::raw(200, "<html>")]);

src/forge/rest.rs

@@ -6,9 +6,33 @@ use super::ReleaseResult;
 
 const MAX_PAGES: u32 = 100;
 
+#[derive(Clone, Copy)]
+enum Unidentified {
+    AnyBot,
+    Nobody,
+}
+
+enum CommentOwner {
+    Login(String),
+    Unidentified(Unidentified),
+}
+
+impl CommentOwner {
+    fn wrote(&self, comment: &serde_json::Value) -> bool {
+        let author = &comment["user"];
+        match self {
+            Self::Login(login) => author["login"].as_str() == Some(login),
+            Self::Unidentified(Unidentified::AnyBot) => author["type"].as_str() == Some("Bot"),
+            Self::Unidentified(Unidentified::Nobody) => false,
+        }
+    }
+}
+
 pub(super) struct RestClient<'a> {
     agent: &'a ureq::Agent,
     repo_url: String,
+    user_url: String,
+    unidentified: Unidentified,
     auth: (&'static str, String),
     extra_headers: &'static [(&'static str, &'static str)],
     page_size_param: &'static str,
@@ -20,6 +44,8 @@ impl<'a> RestClient<'a> {
         Self {
             agent,
             repo_url: format!("{api_base}/repos/{slug}"),
+            user_url: format!("{api_base}/user"),
+            unidentified: Unidentified::AnyBot,
             auth: ("Authorization", format!("Bearer {token}")),
             extra_headers: &[
                 ("Accept", "application/vnd.github+json"),
@@ -34,6 +60,8 @@ impl<'a> RestClient<'a> {
         Self {
             agent,
             repo_url: format!("{api_base}/repos/{slug}"),
+            user_url: format!("{api_base}/user"),
+            unidentified: Unidentified::Nobody,
             auth: ("Authorization", format!("token {token}")),
             extra_headers: &[],
             page_size_param: "limit",
@@ -119,13 +147,46 @@ impl<'a> RestClient<'a> {
     pub fn find_comment(&self, pr_id: u64, marker: &str, what: &str) -> Result<Option<u64>> {
         let comments =
             self.paginated_json_array(&format!("{}/issues/{pr_id}/comments", self.repo_url), what)?;
-        Ok(comments.iter().find_map(|comment| {
-            comment["body"]
-                .as_str()
-                .is_some_and(|body| body.contains(marker))
-                .then(|| comment["id"].as_u64())
-                .flatten()
-        }))
+        let mut marked = comments
+            .iter()
+            .rev()
+            .filter(|comment| {
+                comment["body"]
+                    .as_str()
+                    .is_some_and(|body| body.contains(marker))
+            })
+            .peekable();
+        if marked.peek().is_none() {
+            return Ok(None);
+        }
+        let owner = self.comment_owner()?;
+        Ok(marked
+            .find(|comment| owner.wrote(comment))
+            .and_then(|comment| comment["id"].as_u64()))
+    }
+
+    fn comment_owner(&self) -> Result<CommentOwner> {
+        match self.get(&self.user_url).call() {
+            Ok(mut response) => {
+                let user: serde_json::Value = response
+                    .body_mut()
+                    .read_json()
+                    .context("Failed to parse the token's user")?;
+                let login = user["login"]
+                    .as_str()
+                    .context("The token's user has no login")?;
+                Ok(CommentOwner::Login(login.to_string()))
+            }
+            Err(ureq::Error::StatusCode(401 | 403)) => {
+                if matches!(self.unidentified, Unidentified::Nobody) {
+                    tracing::warn!(
+                        "Warning: the token cannot read its own user, posting a new comment instead of updating one"
+                    );
+                }
+                Ok(CommentOwner::Unidentified(self.unidentified))
+            }
+            Err(e) => Err(e).context("Failed to read the token's user"),
+        }
     }
 
     pub fn create_comment(&self, pr_id: u64, body: &str) -> Result<()> {