Pick any line to see why it is the way it is: the commit, the pull request and issue it came from, and what the agent was thinking.
| Cards you act on in chat; agents comment and review as themselves; names shown cleanly; commits on the calendar | 1 | //! A workspace's own agents (Margo, @margo) commenting on issues and pull |
| 2 | //! requests, and reviewing pull requests, as themselves, on behalf of a | |
| 3 | //! person whose access caps them. The agents service calls these; nothing | |
| 4 | //! reaches them with a person's token. | |
| 5 | //! | |
| 6 | //! What an agent writes is stored with the agent as its author and the | |
| 7 | //! person it acted for beside it (migrations/0033_agent_comments.sql). Its | |
| 8 | //! review is advisory: the verdict is kept in `agent_verdict`, never in | |
| 9 | //! `verdict`, which every rule about approvals reads (required approvals | |
| 10 | //! in mergeability.rs and rulesets.rs through the `Verdicts` prefetch, | |
| 11 | //! a person's request for changes in lifecycle.rs `person_request`, | |
| 12 | //! confidence.rs, contributions.rs), and code owners skip it by name | |
| 13 | //! (codeowners.rs `latest_verdicts`). So it never satisfies a required | |
| 14 | //! approval or a code owner, and never blocks a merge. | |
| 15 | ||
| 16 | use g1t_contracts::credentials::Principal; | |
| 17 | use g1t_contracts::events::CommentCreated; | |
| 18 | use g1t_contracts::time::rfc3339; | |
| 19 | use g1t_contracts::work::*; | |
| 20 | use g1t_contracts::{FailureCode, Outcome, PrincipalKind, User, new_id}; | |
| 21 | use g1t_kit::now_ms; | |
| 22 | use serde::Deserialize; | |
| 23 | use worker::Result; | |
| 24 | use worker::wasm_bindgen::JsValue; | |
| 25 | ||
| 26 | use crate::retired::writable; | |
| 27 | use crate::{MAX_ENTRY_CHARS, UNVERIFIED, Work}; | |
| 28 | ||
| 29 | /// What an agent may write where, before anything is read: why not, or none. | |
| 30 | fn refusal(agent: &AgentRef, body: &str, verdict: Option<AgentVerdict>) -> Option<&'static str> { | |
| 31 | if agent.id.trim().is_empty() || agent.handle.trim().is_empty() { | |
| 32 | return Some("Name the agent: its id and handle."); | |
| 33 | } | |
| 34 | // An approval speaks for itself; anything else has to say something. | |
| 35 | if body.is_empty() && verdict != Some(AgentVerdict::Approve) { | |
| 36 | return Some("A comment cannot be empty."); | |
| 37 | } | |
| 38 | if body.chars().count() > MAX_ENTRY_CHARS { | |
| 39 | return Some("That comment is too long."); | |
| 40 | } | |
| 41 | None | |
| 42 | } | |
| 43 | ||
| 44 | /// Why an agent may not review a pull request as it stands, or none. | |
| 45 | fn review_refusal(status: PullStatus) -> Option<&'static str> { | |
| 46 | match status { | |
| 47 | PullStatus::Open => None, | |
| 48 | PullStatus::Draft => Some("A draft is still being worked on; an agent reviews it once it is ready for review."), | |
| 49 | PullStatus::Merged | PullStatus::Closed => Some("Only an open pull request can be reviewed."), | |
| 50 | } | |
| 51 | } | |
| 52 | ||
| 53 | /// Whether an agent that wrote `written` times on one issue or pull request | |
| 54 | /// in the last hour may write again. | |
| 55 | fn within_limit(written: u32) -> bool { | |
| 56 | written < AGENT_COMMENTS_PER_WINDOW | |
| 57 | } | |
| 58 | ||
| 59 | #[derive(Deserialize)] | |
| 60 | struct Count { | |
| 61 | n: u32, | |
| 62 | } | |
| 63 | ||
| 64 | impl Work { | |
| 65 | /// `workspace_agent_comment`. | |
| 66 | pub(crate) async fn workspace_agent_comment(&self, a: WorkspaceAgentCommentArgs) -> Result<Outcome<Comment>> { | |
| 67 | self.agent_writes(a.repo, a.number, a.agent, a.acting_for, a.body, None).await | |
| 68 | } | |
| 69 | ||
| 70 | /// `workspace_agent_review`. | |
| 71 | pub(crate) async fn workspace_agent_review(&self, a: WorkspaceAgentReviewArgs) -> Result<Outcome<Comment>> { | |
| 72 | self.agent_writes(a.repo, a.number, a.agent, a.acting_for, a.body, Some(a.verdict)).await | |
| 73 | } | |
| 74 | ||
| 75 | /// A comment, or with `verdict` a review, by `agent` as itself on | |
| 76 | /// behalf of `acting_for`, with the checks a person's comment has. | |
| 77 | async fn agent_writes( | |
| 78 | &self, | |
| 79 | path: g1t_contracts::repos::RepoPath, | |
| 80 | number: u32, | |
| 81 | agent: AgentRef, | |
| 82 | acting_for: User, | |
| 83 | body: String, | |
| 84 | verdict: Option<AgentVerdict>, | |
| 85 | ) -> Result<Outcome<Comment>> { | |
| 86 | let body = body.trim(); | |
| 87 | if let Some(refused) = refusal(&agent, body, verdict) { | |
| 88 | return Ok(Outcome::fail(FailureCode::Invalid, refused)); | |
| 89 | } | |
| 90 | // The person it acts for, as if they wrote it themselves. | |
| 91 | if !acting_for.verified { | |
| 92 | return Ok(Outcome::fail(FailureCode::Forbidden, UNVERIFIED)); | |
| 93 | } | |
| 94 | let repo = match self.repo(&path, &Some(acting_for.clone())).await? { | |
| 95 | Outcome::Ok(repo) => repo, | |
| 96 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 97 | }; | |
| 98 | if let Outcome::Fail(failure) = writable(&repo) { | |
| 99 | return Ok(Outcome::Fail(failure)); | |
| 100 | } | |
| 101 | let mut pull_id = None; | |
| 102 | let table = if self.issue(&repo.id, number).await?.is_some() { | |
| 103 | if verdict.is_some() { | |
| 104 | return Ok(Outcome::fail(FailureCode::Invalid, "Only a pull request can be reviewed.")); | |
| 105 | } | |
| 106 | "issues" | |
| 107 | } else if let Some(pull) = self.pull(&repo.id, number).await? { | |
| 108 | if verdict.is_some() | |
| 109 | && let Some(refused) = review_refusal(pull.status) | |
| 110 | { | |
| 111 | return Ok(Outcome::fail(FailureCode::Conflict, refused)); | |
| 112 | } | |
| 113 | // A command to g1t on a dependency update it opened is a | |
| 114 | // person's to give, never an agent's. | |
| 115 | if pull.author.is_system() && g1t_contracts::updates::update_command(body).is_some() { | |
| 116 | return Ok(Outcome::fail( | |
| 117 | FailureCode::Forbidden, | |
| 118 | "An agent cannot give commands on g1t's dependency updates; ask a person with the Write role.", | |
| 119 | )); | |
| 120 | } | |
| 121 | pull_id = Some(pull.id.clone()); | |
| 122 | "pulls" | |
| 123 | } else { | |
| 124 | return Ok(Outcome::fail(FailureCode::NotFound, "No issue or pull request has that number.")); | |
| 125 | }; | |
| 126 | ||
| 127 | let now = now_ms(); | |
| 128 | let written = self | |
| 129 | .db | |
| 130 | .prepare( | |
| 131 | "SELECT count(*) AS n FROM comments | |
| 132 | WHERE agent_id = ? AND repo_id = ? AND number = ? AND created_at >= ?", | |
| 133 | ) | |
| 134 | .bind(&[ | |
| 135 | agent.id.as_str().into(), | |
| 136 | repo.id.as_str().into(), | |
| 137 | number.into(), | |
| 138 | rfc3339(now.saturating_sub(AGENT_COMMENT_WINDOW_MS)).into(), | |
| 139 | ])? | |
| 140 | .first::<Count>(None) | |
| 141 | .await? | |
| 142 | .map_or(0, |count| count.n); | |
| 143 | if !within_limit(written) { | |
| 144 | return Ok(Outcome::fail( | |
| 145 | FailureCode::Conflict, | |
| 146 | format!( | |
| 147 | "@{} has written {AGENT_COMMENTS_PER_WINDOW} times on #{number} in the last hour; it can write again later.", | |
| 148 | agent.handle | |
| 149 | ), | |
| 150 | )); | |
| 151 | } | |
| 152 | ||
| 153 | let comment = Comment { | |
| 154 | id: new_id("cmt", now), | |
| 155 | kind: CommentKind::Comment, | |
| 156 | author: User { | |
| 157 | id: agent.id.clone(), | |
| 158 | username: agent.handle.clone(), | |
| 159 | kind: PrincipalKind::Agent, | |
| 160 | ..User::default() | |
| 161 | }, | |
| 162 | body: body.to_owned(), | |
| 163 | path: None, | |
| 164 | line: None, | |
| 165 | verdict: verdict.and_then(AgentVerdict::verdict), | |
| 166 | created_at: rfc3339(now), | |
| 167 | edited_at: None, | |
| 168 | acting_for: Some(User { | |
| 169 | id: acting_for.id.clone(), | |
| 170 | username: acting_for.username.clone(), | |
| 171 | ..User::default() | |
| 172 | }), | |
| 173 | advisory: verdict.is_some(), | |
| 174 | agent: Some(agent.clone()), | |
| 175 | }; | |
| 176 | self.db | |
| 177 | .batch(vec![ | |
| 178 | self.db | |
| 179 | .prepare( | |
| 180 | "INSERT INTO comments | |
| 181 | (id, repo_id, number, author_id, author_name, body, created_at, | |
| 182 | agent_id, agent_handle, agent_name, agent_avatar_seed, | |
| 183 | acting_for_id, acting_for_name, agent_verdict) | |
| 184 | VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", | |
| 185 | ) | |
| 186 | .bind(&[ | |
| 187 | comment.id.as_str().into(), | |
| 188 | repo.id.as_str().into(), | |
| 189 | number.into(), | |
| 190 | agent.id.as_str().into(), | |
| 191 | agent.handle.as_str().into(), | |
| 192 | body.into(), | |
| 193 | comment.created_at.as_str().into(), | |
| 194 | agent.id.as_str().into(), | |
| 195 | agent.handle.as_str().into(), | |
| 196 | agent.display_name.as_str().into(), | |
| 197 | agent.avatar_seed.as_str().into(), | |
| 198 | acting_for.id.as_str().into(), | |
| 199 | acting_for.username.as_str().into(), | |
| 200 | verdict.map_or(JsValue::NULL, |verdict| verdict.as_str().into()), | |
| 201 | ])?, | |
| 202 | self.db | |
| 203 | .prepare(format!("UPDATE {table} SET updated_at = ? WHERE repo_id = ? AND number = ?")) | |
| 204 | .bind(&[comment.created_at.as_str().into(), repo.id.as_str().into(), number.into()])?, | |
| 205 | ]) | |
| 206 | .await?; | |
| 207 | // What an agent says never summons g1t (mentions.rs `may_summon`), | |
| 208 | // so no mention is recorded. The event is the person's doing, as | |
| 209 | // their own comment's would be, with the agent named on it. | |
| 210 | self.publish( | |
| 211 | "comment.created", | |
| 212 | &repo.id, | |
| 213 | &acting_for, | |
| 214 | CommentCreated { | |
| 215 | comment_id: comment.id.clone(), | |
| 216 | repo_id: repo.id.clone(), | |
| 217 | number, | |
| 218 | pull_id, | |
| 219 | verdict: comment.verdict, | |
| 220 | agent: Some(agent), | |
| 221 | acting_for: Some(Principal::from(&acting_for)), | |
| 222 | advisory: comment.advisory, | |
| 223 | }, | |
| 224 | ) | |
| 225 | .await?; | |
| 226 | Ok(Outcome::Ok(comment)) | |
| 227 | } | |
| 228 | } | |
| 229 | ||
| 230 | #[cfg(test)] | |
| 231 | mod tests { | |
| 232 | use super::*; | |
| 233 | ||
| 234 | fn margo() -> AgentRef { | |
| 235 | AgentRef { | |
| 236 | id: "agt_1".into(), | |
| 237 | handle: "margo".into(), | |
| 238 | display_name: "Margo".into(), | |
| 239 | avatar_seed: "margo".into(), | |
| 240 | } | |
| 241 | } | |
| 242 | ||
| 243 | #[test] | |
| 244 | fn an_agent_says_something_unless_it_approves() { | |
| 245 | assert_eq!(refusal(&margo(), "", None), Some("A comment cannot be empty.")); | |
| 246 | assert_eq!(refusal(&margo(), "", Some(AgentVerdict::Comment)), Some("A comment cannot be empty.")); | |
| 247 | assert_eq!(refusal(&margo(), "", Some(AgentVerdict::RequestChanges)), Some("A comment cannot be empty.")); | |
| 248 | assert_eq!(refusal(&margo(), "", Some(AgentVerdict::Approve)), None); | |
| 249 | assert_eq!(refusal(&margo(), "Looks right.", None), None); | |
| 250 | let nobody = AgentRef { handle: String::new(), ..margo() }; | |
| 251 | assert!(refusal(&nobody, "hi", None).is_some()); | |
| 252 | } | |
| 253 | ||
| 254 | #[test] | |
| 255 | fn an_agent_reviews_only_what_is_ready() { | |
| 256 | assert_eq!(review_refusal(PullStatus::Open), None); | |
| 257 | assert!(review_refusal(PullStatus::Draft).is_some()); | |
| 258 | assert!(review_refusal(PullStatus::Merged).is_some()); | |
| 259 | assert!(review_refusal(PullStatus::Closed).is_some()); | |
| 260 | } | |
| 261 | ||
| 262 | #[test] | |
| 263 | fn five_an_hour_on_one_pull_request() { | |
| 264 | assert!(within_limit(0)); | |
| 265 | assert!(within_limit(4)); | |
| 266 | assert!(!within_limit(5)); | |
| 267 | } | |
| 268 | ||
| 269 | #[test] | |
| 270 | fn the_wire_reads_snake_case_and_comments_read_camel_case() { | |
| 271 | let asked: WorkspaceAgentReviewArgs = serde_json::from_value(serde_json::json!({ | |
| 272 | "repo": { "namespace": "acme", "name": "web" }, | |
| 273 | "number": 7, | |
| 274 | "agent": { "id": "agt_1", "handle": "margo", "display_name": "Margo", "avatar_seed": "margo" }, | |
| 275 | "acting_for": { "id": "usr_1", "username": "ana" }, | |
| 276 | "verdict": "request_changes", | |
| 277 | "body": "Trim the name." | |
| 278 | })) | |
| 279 | .unwrap(); | |
| 280 | assert_eq!(asked.verdict, AgentVerdict::RequestChanges); | |
| 281 | assert_eq!(asked.agent, margo()); | |
| 282 | let shown = serde_json::to_value(&asked.agent).unwrap(); | |
| 283 | assert_eq!(shown["displayName"], "Margo"); | |
| 284 | assert_eq!(shown["avatarSeed"], "margo"); | |
| 285 | } | |
| 286 | ||
| 287 | #[test] | |
| 288 | fn an_agent_review_shows_its_verdict_but_is_advisory() { | |
| 289 | let row: crate::rows::CommentRow = serde_json::from_value(serde_json::json!({ | |
| 290 | "id": "cmt_1", "kind": "comment", "author_id": "agt_1", "author_name": "margo", | |
| 291 | "body": "Ship it.", "path": null, "line": null, "verdict": null, "created_at": "2026-10-09T00:00:00Z", | |
| 292 | "agent_id": "agt_1", "agent_handle": "margo", "agent_name": "Margo", "agent_avatar_seed": "m", | |
| 293 | "acting_for_id": "usr_1", "acting_for_name": "ana", "agent_verdict": "approve" | |
| 294 | })) | |
| 295 | .unwrap(); | |
| 296 | assert_eq!(row.answerable_id(), "usr_1"); | |
| 297 | assert!(row.has_verdict()); | |
| 298 | let comment = Comment::from(row); | |
| 299 | assert_eq!(comment.verdict, Some(Verdict::Approve)); | |
| 300 | assert!(comment.advisory); | |
| 301 | assert_eq!(comment.author.kind, PrincipalKind::Agent); | |
| 302 | assert_eq!(comment.answerable().username, "ana"); | |
| 303 | assert_eq!(comment.agent.unwrap().display_name, "Margo"); | |
| 304 | } | |
| 305 | } |