Skip to content
Sign up

flagon-io/g1t

public
00

Where people and agents ship software together. The open-source git platform for the whole job: issues, agents, checks and deploys to the edge.

gitagentscloudflarerustforgemcp

Commit

Merge branch 'worktree-agent-a57a81856918d70a7' into ship-batch

syntaqxcommitted Parents3497fcc3213b85Browse files
10 files+464−720/10 viewed
+6−7
144144 // Its labels and milestone, with the repository's to choose from.
145145 const labelsFound = work.listLabels(path, viewer);
146146 // How it may merge now, for the merge box: streamed, since its default
147− // messages are read from the branch's commits, which takes seconds. Only
148− // one still open or a draft can merge.
147+ // messages may have to be read from the branch's commits. Asked for
148+ // beside the pull request rather than after it; only one still open or
149+ // a draft can merge, so the answer for any other is set aside.
149150 const choicesFound =
150151 tab === "conversation"
151− ? pullFound
152− .then((found) =>
153− found.ok && (found.value.pull.status === "open" || found.value.pull.status === "draft")
154− ? work.mergeChoices(viewer, path, number)
155− : null,
152+ ? Promise.all([work.mergeChoices(viewer, path, number).catch(() => null), pullFound.catch(() => null)])
153+ .then(([choices, found]) =>
154+ found?.ok && (found.value.pull.status === "open" || found.value.pull.status === "draft") ? choices : null,
156155 )
157156 .catch(() => null)
158157 : Promise.resolve(null);
+9−0
12961296 pub base_branch: String,
12971297 #[serde(default)]
12981298 pub limit: Option<u32>,
1299+ /// Where the branch left `base_branch`, when the caller knows it (a
1300+ /// `divergence` worked it out): the two histories are not read again.
1301+ #[serde(default)]
1302+ pub merge_base: Option<String>,
1303+ /// Only each commit's message, addresses and parents, for default merge
1304+ /// messages: no files are listed and no signature is decided, so
1305+ /// `files` is empty and `signature` is left as unsigned.
1306+ #[serde(default)]
1307+ pub messages_only: bool,
12991308 }
13001309
13011310 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
+56−0
8888 Some(out)
8989 }
9090
91+/// The commits of `ids` as default merge messages read them: message,
92+/// addresses and parents, from the pack alone. No files are listed and no
93+/// signature is decided, so nothing is read from the repository.
94+pub fn messages(pack: &Pack, ids: &[String]) -> Vec<CommitFacts> {
95+ ids.iter()
96+ .filter_map(|id| {
97+ let Some((ObjectKind::Commit, data)) = pack.get(id) else {
98+ return None;
99+ };
100+ let commit = read_commit(data);
101+ Some(CommitFacts {
102+ sha: id.clone(),
103+ message: commit.message,
104+ author_email: commit.author_email,
105+ committer_email: commit.committer_email,
106+ parents: commit.parents.len() as u32,
107+ signature: Signature::default(),
108+ files: Vec::new(),
109+ files_complete: false,
110+ })
111+ })
112+ .collect()
113+}
114+
91115 /// The files that differ between two trees, deletions included, with the
92116 /// size of each new blob the pack holds. Whether the list is complete.
93117 /// Each level of the trees is read at once.
305329 assert_eq!(added(&pack, &"0".repeat(40), 10), Some(Vec::new()), "a tip the pack does not hold adds nothing");
306330 }
307331
332+ #[test]
333+ fn messages_are_read_from_the_pack_alone() {
334+ use g1t_scan::pack::write_pack;
335+ let first = b"tree t
336+author Ada <ada@x> 1 +0000
337+committer A <a@x> 1 +0000
338+
339+feat: one
340+".to_vec();
341+ let first_id = g1t_scan::pack::object_id(ObjectKind::Commit, &first);
342+ let merge = format!("tree t
343+parent {first_id}
344+parent {}
345+author B <b@x> 1 +0000
346+committer B <b@x> 1 +0000
347+
348+Merge main
349+", "f".repeat(40)).into_bytes();
350+ let merge_id = g1t_scan::pack::object_id(ObjectKind::Commit, &merge);
351+ let pack = Pack::parse(&write_pack(&[(ObjectKind::Commit, first), (ObjectKind::Commit, merge)])).unwrap();
352+ let ids = added(&pack, &merge_id, 10).unwrap();
353+ let read = messages(&pack, &ids);
354+ assert_eq!(read.iter().map(|commit| commit.message.as_str()).collect::<Vec<_>>(), ["Merge main
355+", "feat: one
356+"]);
357+ assert_eq!(read[0].parents, 2);
358+ assert_eq!(read[1].author_email.as_deref(), Some("ada@x"));
359+ assert!(read.iter().all(|commit| commit.files.is_empty() && commit.signature == Signature::Unsigned));
360+ // An id the pack does not hold is left out.
361+ assert_eq!(messages(&pack, &["0".repeat(40)]).len(), 0);
362+ }
363+
308364 /// Histories by where they start; a start missing from it fails.
309365 struct Histories(HashMap<String, Vec<g1t_contracts::repos::Commit>>);
310366
+25−10
319319 }
320320
321321 /// Services only: the commits a pull request would land, read as rules
322− /// look at them, fetched from its source as a pack.
322+ /// look at them (or only their messages), fetched from its source as a
323+ /// pack.
323324 pub(crate) async fn inspect_commits(&self, a: InspectCommitsArgs) -> Result<Outcome<InspectedCommits>> {
324325 let (Some(source), Some(target)) = (self.registry.by_id(&a.source_id).await?, self.registry.by_id(&a.target_id).await?) else {
325326 return Ok(Outcome::fail(FailureCode::NotFound, "Repository not found."));
326327 };
327328 self.live(&source).await?;
328− let (source_git, target_git) = (self.store.open(&store_key(&source)).await?, self.store.open(&store_key(&target)).await?);
329− let (history, base_history) =
330− futures_util::future::try_join(source_git.log(&a.head, MAX_ANCESTRY), target_git.log(&a.base_branch, MAX_ANCESTRY)).await?;
331− let shared: std::collections::HashSet<String> = base_history.into_iter().map(|commit| commit.hash).collect();
332− let merge_base = crate::nearest_ancestor_in(&source_git, &history, &shared).await?;
333− let Some(head) = history.first() else {
334− return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: true }));
329+ let source_git = self.store.open(&store_key(&source)).await?;
330+ // Where the branch left the base: as the caller found it (from a
331+ // `divergence`), else from both histories, read at once.
332+ let (head, merge_base) = match a.merge_base.clone() {
333+ Some(merge_base) => (a.head.clone(), Some(merge_base)),
334+ None => {
335+ let target_git = self.store.open(&store_key(&target)).await?;
336+ let (history, base_history) =
337+ futures_util::future::try_join(source_git.log(&a.head, MAX_ANCESTRY), target_git.log(&a.base_branch, MAX_ANCESTRY)).await?;
338+ let shared: std::collections::HashSet<String> = base_history.into_iter().map(|commit| commit.hash).collect();
339+ let merge_base = crate::nearest_ancestor_in(&source_git, &history, &shared).await?;
340+ let Some(head) = history.first() else {
341+ return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: true }));
342+ };
343+ (head.hash.clone(), merge_base)
344+ }
335345 };
336346 let access = source_git.access(crate::store::Scope::Read).await?;
337− let fetched = crate::land::fetch_pack(&access, &head.hash, merge_base.as_deref()).await?;
347+ let fetched = crate::land::fetch_pack(&access, &head, merge_base.as_deref()).await?;
338348 if fetched.len() > crate::secret_scan::MAX_SCANNED_PUSH {
339349 return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: false }));
340350 }
346356 }
347357 };
348358 let limit = a.limit.map_or(MAX_COMMITS, |limit| (limit as usize).min(MAX_COMMITS));
349− let Some(ids) = rule_facts::added(&pack, &head.hash, limit) else {
359+ let Some(ids) = rule_facts::added(&pack, &head, limit) else {
350360 return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: false }));
351361 };
362+ // For messages, what the pack holds is enough: no file is listed, so
363+ // no tree is read, and no signature's owner is asked for.
364+ if a.messages_only {
365+ return Ok(Outcome::Ok(InspectedCommits { commits: rule_facts::messages(&pack, &ids), complete: true }));
366+ }
352367 let (fingerprints, emails) = rule_facts::signing_facts(&pack, &ids);
353368 let owners = self.signing_owners(&fingerprints, &emails).await;
354369 let commits = rule_facts::read_all(&pack, &source_git, &ids, Some(&owners)).await?;
+15−0
1+-- The commits a pull request's default merge messages read (merging.rs),
2+-- kept when its mergeability is worked out after a push (mergeability.rs),
3+-- so the merge box reads them here rather than from the git store.
4+-- `head` and `merge_base` are what they were read for: the list changes
5+-- only when one of them moves. `pair` is the mergeable_key ("head..base")
6+-- they were last confirmed for; they are used only while it is the pull
7+-- request's mergeable_key and `head` its head.
8+CREATE TABLE pull_message_commits (
9+ pull_id TEXT PRIMARY KEY,
10+ head TEXT NOT NULL,
11+ merge_base TEXT,
12+ pair TEXT NOT NULL,
13+ commits TEXT NOT NULL,
14+ updated_at TEXT NOT NULL
15+);
+3−2
1111 /// The longest commit message body a merger may write.
1212 pub(crate) const MAX_MESSAGE_CHARS: usize = 64_000;
1313
14−/// One commit the pull request adds, as its message says.
15−#[derive(Clone, Debug, Default, PartialEq, Eq)]
14+/// One commit the pull request adds, as its message says. Kept as JSON
15+/// with its pull request (`pull_message_commits`, merging.rs).
16+#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
1617 pub(crate) struct CommitText {
1718 pub message: String,
1819 pub author_email: Option<String>,
+23−6
1010 //! probe in a sandbox, which merges the two without an agent and reports the
1111 //! files that conflict. Only so many probes run at once per repository; the
1212 //! rest wait their turn and start as earlier ones report.
13+//!
14+//! After a push, the commits a pull request's default merge messages read
15+//! are kept for the same pair of commits (`keep_commit_texts`, merging.rs).
1316
1417 use std::collections::HashSet;
1518
113116 /// being. Settles it at once where it can, and asks for a probe where
114117 /// it cannot.
115118 pub(crate) async fn assess_mergeability(&self, pull: &Pull) -> Result<()> {
119+ self.assess_for(pull).await.map(|_| ())
120+ }
121+
122+ /// [`Work::assess_mergeability`], saying what it was worked out for:
123+ /// the pull request's divergence from its target, if it has one.
124+ async fn assess_for(&self, pull: &Pull) -> Result<Option<Divergence>> {
116125 if !pull.status.is_active() {
117− return Ok(());
126+ return Ok(None);
118127 }
119128 let divergence: Option<Divergence> = g1t_kit::call(
120129 &self.repos,
129138 .await?;
130139 // Nothing pushed yet.
131140 let Some(divergence) = divergence else {
132− return Ok(());
141+ return Ok(None);
133142 };
134143 let key = key(&divergence);
135144 let now = now_ms();
155164 _ => false,
156165 };
157166 if settled {
158− return Ok(());
167+ return Ok(Some(divergence));
159168 }
160169 }
161170 if !needs_probe(&divergence) {
173182 if row.is_some_and(|row| row.mergeable.as_deref() == Some("conflicting")) {
174183 self.announce_mergeability(pull).await?;
175184 }
176− return Ok(());
185+ return Ok(Some(divergence));
177186 }
178187 self.db
179188 .prepare(
190199 ])?
191200 .run()
192201 .await?;
193− self.ask_for_probe(pull, &divergence.head).await
202+ self.ask_for_probe(pull, &divergence.head).await?;
203+ Ok(Some(divergence))
194204 }
195205
196206 async fn ask_for_probe(&self, pull: &Pull, head: &str) -> Result<()> {
244254 if moved.contains(&id) {
245255 self.keep_pull_refs(&pull).await;
246256 }
247− self.assess_mergeability(&pull).await?;
257+ // The commits its default messages read are kept for
258+ // the same pair of commits (merging.rs), so that the
259+ // merge box need not read them from its source.
260+ if let Some(divergence) = self.assess_for(&pull).await?
261+ && let Err(error) = self.keep_commit_texts(&pull, &divergence).await
262+ {
263+ worker::console_warn!("commits for the merge messages of {id} not kept: {error}");
264+ }
248265 }
249266 Ok::<_, worker::Error>(())
250267 };
+323−46
88
99 use g1t_contracts::access::{self, Capability};
1010 use g1t_contracts::events::PullEvent;
11−use g1t_contracts::repos::{CreateBranchArgs, HeadArgs, RemoveBranchArgs, Repo, RepoPath};
11+use g1t_contracts::repos::{CreateBranchArgs, Divergence, HeadArgs, RemoveBranchArgs, Repo, RepoPath};
1212 use g1t_contracts::rules::{InspectCommitsArgs, InspectedCommits, MergeMethod};
1313 use g1t_contracts::work::*;
1414 use g1t_contracts::{FailureCode, Outcome, PrincipalKind, User};
15+use g1t_kit::now_ms;
1516 use g1t_rules::merge::{Requirements, method_label};
17+use serde::Deserialize;
1618 use worker::Result;
1719
1820 use crate::merge_message::{self, CommitText, MessageFacts};
3133
3234 /// How many of a pull request's commits its default messages read.
3335 const MESSAGE_COMMITS: u32 = 250;
36+/// The most JSON of commits kept for one pull request (D1 holds 2 MB to a
37+/// row); a pull request with more has them read each time.
38+const MAX_KEPT_COMMITS: usize = 1_000_000;
39+
40+/// The commits kept for a pull request's default messages
41+/// (`pull_message_commits`), with its mergeability's pair beside them.
42+#[derive(Deserialize)]
43+struct KeptCommitsRow {
44+ #[serde(default)]
45+ head: Option<String>,
46+ #[serde(default)]
47+ pair: Option<String>,
48+ #[serde(default)]
49+ commits: Option<String>,
50+ #[serde(default)]
51+ mergeable_key: Option<String>,
52+}
3453
54+/// Whether commits kept for `kept_head`, last confirmed for the pair of
55+/// commits `kept_pair`, are a pull request's now: its head is `kept_head`
56+/// and its mergeability was last worked out for that same pair. A push to
57+/// either side moves `mergeable_key` before they are kept again, so a list
58+/// read for another head or another base is never used.
59+pub(crate) fn kept_is_current(kept_head: Option<&str>, kept_pair: Option<&str>, head: Option<&str>, mergeable_key: Option<&str>) -> bool {
60+ let (Some(kept_head), Some(kept_pair), Some(head), Some(mergeable_key)) = (kept_head, kept_pair, head, mergeable_key) else {
61+ return false;
62+ };
63+ kept_head == head && kept_pair == mergeable_key && mergeable_key.split_once("..").is_some_and(|(of, _)| of == head)
64+}
65+
66+/// What to do with the commits kept for a pull request when its
67+/// mergeability is worked out for a divergence.
68+#[derive(Debug, PartialEq, Eq)]
69+pub(crate) enum Keeping {
70+ /// Kept for this pair of commits already.
71+ Current,
72+ /// The list holds (same head, same merge base: its base moved without
73+ /// taking any of its commits); only the pair it holds for moves on.
74+ Confirm,
75+ /// Read them again: its head or where it left its base moved, or none
76+ /// are kept.
77+ Read,
78+ /// Keep nothing: it shares no history with its base, so its commits
79+ /// are read live each time.
80+ Nothing,
81+}
82+
83+/// See [`Keeping`]. `kept` is what is stored: head, merge base and pair.
84+pub(crate) fn keeping(kept: Option<(&str, Option<&str>, &str)>, divergence: &Divergence) -> Keeping {
85+ let Some(merge_base) = divergence.merge_base.as_deref() else {
86+ return Keeping::Nothing;
87+ };
88+ let pair = format!("{}..{}", divergence.head, divergence.base);
89+ match kept {
90+ Some((head, kept_base, kept_pair)) if head == divergence.head && kept_base == Some(merge_base) => {
91+ if kept_pair == pair { Keeping::Current } else { Keeping::Confirm }
92+ }
93+ _ => Keeping::Read,
94+ }
95+}
96+
97+/// A pull request's commits as its default messages read them, oldest
98+/// first, merge commits left out.
99+fn texts_of(inspected: InspectedCommits) -> Vec<CommitText> {
100+ inspected
101+ .commits
102+ .into_iter()
103+ .rev()
104+ .filter(|commit| commit.parents <= 1)
105+ .map(|commit| CommitText { message: commit.message, author_email: commit.author_email })
106+ .collect()
107+}
108+
109+/// The default title and body of the commit each of `methods` makes for
110+/// `pull`, from its `commits` (oldest first): `None` for one that makes no
111+/// commit of its own.
112+fn messages_for(
113+ repo: &Repo,
114+ pull: &Pull,
115+ settings: &RepoSettings,
116+ methods: &[MergeMethod],
117+ commits: &[CommitText],
118+) -> Vec<(MergeMethod, Option<(String, String)>)> {
119+ let owner = pull.owner().username.clone();
120+ let (head_owner, head_repo) = match (&pull.fork, &pull.head_repo) {
121+ // A working copy is whoever asked for the change's.
122+ (Some(copy), _) => (owner.clone(), format!("{}/{}", copy.namespace, copy.name)),
123+ (None, Some(fork)) => (fork.namespace.clone(), format!("{}/{}", fork.namespace, fork.name)),
124+ (None, None) => (repo.namespace.clone(), format!("{}/{}", repo.namespace, repo.name)),
125+ };
126+ let branch = pull
127+ .branch
128+ .clone()
129+ .or_else(|| pull.fork.as_ref().map(|copy| copy.name.clone()))
130+ .unwrap_or_default();
131+ let issues = issues_to_close(pull, repo, settings, false);
132+ let facts = MessageFacts {
133+ number: pull.number,
134+ title: &pull.title,
135+ body: pull.body.as_deref().unwrap_or_default(),
136+ branch: &branch,
137+ head_owner: &head_owner,
138+ head_repo: &head_repo,
139+ base: pull.base_branch(&repo.default_branch),
140+ author: &owner,
141+ commits,
142+ issues: &issues,
143+ };
144+ methods.iter().map(|method| (*method, merge_message::default_message(*method, settings, &facts))).collect()
145+}
146+
35147 /// Methods named for people, joined: "squash or rebase".
36148 fn labels(methods: &[MergeMethod]) -> String {
37149 let named: Vec<&str> = methods.iter().map(|method| method_label(*method)).collect();
178290 }
179291
180292 impl Work {
181− /// A pull request's commits as its default messages read them, oldest
182− /// first, merge commits left out. None when they cannot be read.
183− async fn commit_texts(&self, repo: &Repo, pull: &Pull) -> Result<Vec<CommitText>> {
293+ /// A pull request's commits as its default messages read them, read
294+ /// from its source now: only their messages, from a pack fetched from
295+ /// `merge_base` when that is known. Empty when they cannot be read.
296+ async fn read_commit_texts(
297+ &self,
298+ pull: &Pull,
299+ target_id: &str,
300+ base_branch: &str,
301+ merge_base: Option<String>,
302+ ) -> Result<Vec<CommitText>> {
184303 let Some(head) = pull.head_commit.clone() else {
185304 return Ok(Vec::new());
186305 };
190309 &InspectCommitsArgs {
191310 source_id: pull.source_id().to_owned(),
192311 head,
193− target_id: repo.id.clone(),
194− base_branch: pull.base_branch(&repo.default_branch).to_owned(),
312+ target_id: target_id.to_owned(),
313+ base_branch: base_branch.to_owned(),
195314 limit: Some(MESSAGE_COMMITS),
315+ merge_base,
316+ messages_only: true,
196317 },
197318 )
198319 .await?;
199− let Outcome::Ok(inspected) = found else {
200− return Ok(Vec::new());
201− };
202− // Read newest first.
203− Ok(inspected
204− .commits
205− .into_iter()
206− .rev()
207− .filter(|commit| commit.parents <= 1)
208− .map(|commit| CommitText { message: commit.message, author_email: commit.author_email })
209− .collect())
320+ Ok(match found {
321+ Outcome::Ok(inspected) => texts_of(inspected),
322+ Outcome::Fail(_) => Vec::new(),
323+ })
324+ }
325+
326+ /// The commits kept for a pull request's default messages, if they are
327+ /// its commits as it is now ([`kept_is_current`]). One database read.
328+ async fn kept_commit_texts(&self, pull: &Pull) -> Result<Option<Vec<CommitText>>> {
329+ if !pull.status.is_active() || pull.head_commit.is_none() {
330+ return Ok(None);
331+ }
332+ let row = self
333+ .timing
334+ .db(
335+ 1,
336+ self.db
337+ .prepare(
338+ "SELECT kept.head, kept.pair, kept.commits, pulls.mergeable_key
339+ FROM pulls LEFT JOIN pull_message_commits AS kept ON kept.pull_id = pulls.id
340+ WHERE pulls.id = ?",
341+ )
342+ .bind(&[pull.id.as_str().into()])?
343+ .first::<KeptCommitsRow>(None),
344+ )
345+ .await?;
346+ let Some(row) = row else { return Ok(None) };
347+ if !kept_is_current(row.head.as_deref(), row.pair.as_deref(), pull.head_commit.as_deref(), row.mergeable_key.as_deref()) {
348+ return Ok(None);
349+ }
350+ Ok(row.commits.as_deref().and_then(|commits| serde_json::from_str(commits).ok()))
351+ }
352+
353+ /// A pull request's commits as its default messages read them, oldest
354+ /// first, merge commits left out: those kept for it when they are
355+ /// current, else read now.
356+ async fn commit_texts(&self, repo: &Repo, pull: &Pull) -> Result<Vec<CommitText>> {
357+ if let Some(kept) = self.kept_commit_texts(pull).await? {
358+ return Ok(kept);
359+ }
360+ self.read_commit_texts(pull, &repo.id, pull.base_branch(&repo.default_branch), None).await
210361 }
211362
363+ /// Keeps a pull request's commits for its default messages, once its
364+ /// mergeability was worked out for `divergence` after a push, so that
365+ /// the merge box reads them from here. They are read again only when
366+ /// its head or where it left its base moved; a push to its base that
367+ /// took none of its commits only moves the pair they hold for. Nothing
368+ /// is read when no method the repository allows writes commits into a
369+ /// message.
370+ pub(crate) async fn keep_commit_texts(&self, pull: &Pull, divergence: &Divergence) -> Result<()> {
371+ if !pull.status.is_active() || pull.head_commit.as_deref() != Some(divergence.head.as_str()) {
372+ return Ok(());
373+ }
374+ #[derive(Deserialize)]
375+ struct Kept {
376+ head: String,
377+ merge_base: Option<String>,
378+ pair: String,
379+ }
380+ let (settings, kept) = futures_util::future::try_join(
381+ self.settings(&pull.repo_id),
382+ self.db
383+ .prepare("SELECT head, merge_base, pair FROM pull_message_commits WHERE pull_id = ?")
384+ .bind(&[pull.id.as_str().into()])?
385+ .first::<Kept>(None),
386+ )
387+ .await?;
388+ if !settings.allowed_methods().into_iter().any(|method| merge_message::needs_commits(method, &settings)) {
389+ return Ok(());
390+ }
391+ let pair = format!("{}..{}", divergence.head, divergence.base);
392+ let now = g1t_contracts::time::rfc3339(now_ms());
393+ match keeping(kept.as_ref().map(|kept| (kept.head.as_str(), kept.merge_base.as_deref(), kept.pair.as_str())), divergence) {
394+ Keeping::Current | Keeping::Nothing => Ok(()),
395+ Keeping::Confirm => {
396+ self.db
397+ .prepare("UPDATE pull_message_commits SET pair = ?, updated_at = ? WHERE pull_id = ? AND head = ?")
398+ .bind(&[pair.into(), now.into(), pull.id.as_str().into(), divergence.head.as_str().into()])?
399+ .run()
400+ .await?;
401+ Ok(())
402+ }
403+ Keeping::Read => {
404+ let texts = self
405+ .read_commit_texts(pull, &pull.repo_id, pull.base.as_deref().unwrap_or_default(), divergence.merge_base.clone())
406+ .await?;
407+ let commits = serde_json::to_string(&texts)?;
408+ if commits.len() > MAX_KEPT_COMMITS {
409+ self.db
410+ .prepare("DELETE FROM pull_message_commits WHERE pull_id = ?")
411+ .bind(&[pull.id.as_str().into()])?
412+ .run()
413+ .await?;
414+ return Ok(());
415+ }
416+ self.db
417+ .prepare(
418+ "INSERT INTO pull_message_commits (pull_id, head, merge_base, pair, commits, updated_at)
419+ VALUES (?1, ?2, ?3, ?4, ?5, ?6)
420+ ON CONFLICT (pull_id) DO UPDATE SET head = ?2, merge_base = ?3, pair = ?4, commits = ?5, updated_at = ?6",
421+ )
422+ .bind(&[
423+ pull.id.as_str().into(),
424+ divergence.head.as_str().into(),
425+ crate::optional(&divergence.merge_base),
426+ pair.into(),
427+ commits.into(),
428+ now.into(),
429+ ])?
430+ .run()
431+ .await?;
432+ Ok(())
433+ }
434+ }
435+ }
436+
212437 /// The default title and body of the commit each of `methods` makes
213438 /// for `pull`, as the repository's settings write them: `None` for one
214439 /// that makes no commit of its own.
223448 self.commit_texts(repo, pull).await?
224449 } else {
225450 Vec::new()
226− };
227− let owner = pull.owner().username.clone();
228− let (head_owner, head_repo) = match (&pull.fork, &pull.head_repo) {
229− // A working copy is whoever asked for the change's.
230− (Some(copy), _) => (owner.clone(), format!("{}/{}", copy.namespace, copy.name)),
231− (None, Some(fork)) => (fork.namespace.clone(), format!("{}/{}", fork.namespace, fork.name)),
232− (None, None) => (repo.namespace.clone(), format!("{}/{}", repo.namespace, repo.name)),
233451 };
234− let branch = pull
235− .branch
236− .clone()
237− .or_else(|| pull.fork.as_ref().map(|copy| copy.name.clone()))
238− .unwrap_or_default();
239− let issues = issues_to_close(pull, repo, settings, false);
240− let facts = MessageFacts {
241− number: pull.number,
242− title: &pull.title,
243− body: pull.body.as_deref().unwrap_or_default(),
244− branch: &branch,
245− head_owner: &head_owner,
246− head_repo: &head_repo,
247− base: pull.base_branch(&repo.default_branch),
248− author: &owner,
249− commits: &commits,
250− issues: &issues,
251− };
252− Ok(methods.iter().map(|method| (*method, merge_message::default_message(*method, settings, &facts))).collect())
452+ Ok(messages_for(repo, pull, settings, methods, &commits))
253453 }
254454
255455 /// The whole message of the commit a merge by `method` makes: what the
281481 let (repo, mut pull) = check!(self.pull_at(&a.repo, a.number, &a.viewer).await?);
282482 crate::fill_base(&mut pull, &repo);
283483 let base = pull.base_branch(&repo.default_branch).to_owned();
284− let ((settings, requirements), behind) =
285− futures_util::future::try_join(self.settings_and_requirements(&repo, &pull), self.behind_now(&pull)).await?;
484+ // The commits kept for its messages are read beside its settings;
485+ // only when none are current are they read from its source.
486+ let ((settings, requirements), behind, kept) = futures_util::future::try_join3(
487+ self.settings_and_requirements(&repo, &pull),
488+ self.behind_now(&pull),
489+ self.kept_commit_texts(&pull),
490+ )
491+ .await?;
286492 let methods = offered(&settings, &requirements);
287493 let default_method = choose_method(None, &settings, &requirements, &base).ok();
288494 let viewer = a.viewer.as_ref();
295501 (None, true) => true,
296502 (None, false) => access::can(Some(viewer), &repo, Capability::Push),
297503 });
298− let messages = self.default_messages(&repo, &pull, &settings, &methods).await?;
504+ let commits = if !methods.iter().any(|method| merge_message::needs_commits(*method, &settings)) {
505+ Vec::new()
506+ } else if let Some(kept) = kept {
507+ kept
508+ } else if pull.status.is_active() {
509+ self.read_commit_texts(&pull, &repo.id, &base, None).await?
510+ } else {
511+ // A merged or closed one merges no more.
512+ Vec::new()
513+ };
514+ let messages = messages_for(&repo, &pull, &settings, &methods, &commits);
299515 let options = messages
300516 .into_iter()
301517 .map(|(method, message)| {
627843 assert!(issues_to_close(&fixes, &repo, &settings, false).is_empty());
628844 }
629845
846+ fn diverged(head: &str, base: &str, merge_base: Option<&str>) -> Divergence {
847+ Divergence { head: head.into(), base: base.into(), merge_base: merge_base.map(Into::into), ..Divergence::default() }
848+ }
849+
850+ #[test]
851+ fn kept_commits_are_used_only_for_the_head_and_pair_they_were_kept_for() {
852+ let current = |kept_head, kept_pair, head, key| kept_is_current(kept_head, kept_pair, head, key);
853+ assert!(current(Some("h1"), Some("h1..b1"), Some("h1"), Some("h1..b1")));
854+ // Its head moved: pushed to since they were kept.
855+ assert!(!current(Some("h1"), Some("h1..b1"), Some("h2"), Some("h1..b1")));
856+ // Its base moved and mergeability was worked out again without them
857+ // being confirmed for the new pair.
858+ assert!(!current(Some("h1"), Some("h1..b1"), Some("h1"), Some("h1..b2")));
859+ // Mergeability worked out for another head than the one it has now.
860+ assert!(!current(Some("h1"), Some("h1..b1"), Some("h1"), Some("h0..b1")));
861+ assert!(!current(Some("h1"), Some("h0..b1"), Some("h1"), Some("h0..b1")));
862+ // Nothing kept, nothing worked out, or nothing pushed.
863+ assert!(!current(None, None, Some("h1"), Some("h1..b1")));
864+ assert!(!current(Some("h1"), Some("h1..b1"), Some("h1"), None));
865+ assert!(!current(Some("h1"), Some("h1..b1"), None, Some("h1..b1")));
866+ }
867+
868+ #[test]
869+ fn commits_are_read_again_only_when_the_head_or_merge_base_moves() {
870+ let now = diverged("h1", "b1", Some("m1"));
871+ assert_eq!(keeping(None, &now), Keeping::Read, "none kept");
872+ assert_eq!(keeping(Some(("h1", Some("m1"), "h1..b1")), &now), Keeping::Current);
873+ // Its base moved on without taking any of its commits.
874+ assert_eq!(keeping(Some(("h1", Some("m1"), "h1..b0")), &now), Keeping::Confirm);
875+ // Pushed to.
876+ assert_eq!(keeping(Some(("h0", Some("m1"), "h0..b1")), &now), Keeping::Read);
877+ // Its base took some of its commits, or it caught up with its base.
878+ assert_eq!(keeping(Some(("h1", Some("m0"), "h1..b0")), &now), Keeping::Read);
879+ assert_eq!(keeping(Some(("h1", None, "h1..b0")), &now), Keeping::Read);
880+ // No history in common: nothing is kept, and the box reads them live.
881+ assert_eq!(keeping(Some(("h1", Some("m1"), "h1..b1")), &diverged("h1", "b1", None)), Keeping::Nothing);
882+ }
883+
884+ #[test]
885+ fn kept_commits_read_back_as_they_were_written() {
886+ let texts = vec![
887+ CommitText { message: "feat: one\n\nWhy.".into(), author_email: Some("ada@acme.com".into()) },
888+ CommitText { message: "fix: two".into(), author_email: None },
889+ ];
890+ let json = serde_json::to_string(&texts).unwrap();
891+ assert_eq!(serde_json::from_str::<Vec<CommitText>>(&json).unwrap(), texts);
892+ }
893+
894+ #[test]
895+ fn merge_commits_are_left_out_of_the_messages_oldest_first() {
896+ let commit = |sha: &str, parents| g1t_contracts::rules::CommitFacts {
897+ sha: sha.into(),
898+ message: sha.into(),
899+ parents,
900+ ..g1t_contracts::rules::CommitFacts::default()
901+ };
902+ let inspected = InspectedCommits { commits: vec![commit("newest", 1), commit("merge", 2), commit("oldest", 0)], complete: true };
903+ let texts: Vec<String> = texts_of(inspected).into_iter().map(|text| text.message).collect();
904+ assert_eq!(texts, ["oldest", "newest"]);
905+ }
906+
630907 #[test]
631908 fn the_timeline_says_how_it_merged() {
632909 assert_eq!(merged_note(MergeMethod::Squash), "squashed and merged this");
+2−1
4747 "DELETE FROM overlap_pairs WHERE repo_id = ?1",
4848 "DELETE FROM check_runs WHERE pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
4949 "DELETE FROM review_runs WHERE pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
50+ "DELETE FROM pull_message_commits WHERE pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
5051 "DELETE FROM agent_messages WHERE repo_id = ?1 OR pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
5152 "DELETE FROM queue_entries WHERE repo_id = ?1",
5253 "DELETE FROM queue_probes WHERE repo_id = ?1",
264265 }
265266 // Rows kept by pull request go before the pull requests do.
266267 let pulls = PURGED.iter().position(|sql| sql.starts_with("DELETE FROM pulls")).unwrap();
267− for table in ["session_entries", "check_runs", "review_runs", "agent_messages"] {
268+ for table in ["session_entries", "check_runs", "review_runs", "pull_message_commits", "agent_messages"] {
268269 let at = PURGED.iter().position(|sql| sql.contains(&format!("FROM {table} "))).unwrap();
269270 assert!(at < pulls, "{table}");
270271 }
+2−0
979979 target_id: repo.id.clone(),
980980 base_branch: base.to_owned(),
981981 limit: None,
982+ merge_base: None,
983+ messages_only: false,
982984 },
983985 )
984986 .await?;

Comments

Sign in to comment.