flagon-io/g1t

public

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

git agents cloudflare rust forge mcp

Commit

A pull request's merge options are read from what work keeps, not from the git store. GET /repos/flagon-io/g1t/pulls/41/merge-options took 8.0 to 8.6 s every time: merge_choices made the default merge messages from the branch's commits, and asked repos inspect_commits for them on every call, uncached. That call read the source's and the base's histories (1,000 commits each), worked out the merge base, fetched a pack from the git store and parsed it, asked identity who owns each signing key and address, and, for each of up to 250 commits, read its parent's tree and every changed level of both trees to list its files: all of it for messages that use only each commit's message and author. The site's stream gives up after 5 s, so the merge box never got its real choices and stayed on the settings-only box.

Where they are kept, and why there. Work keeps them in a new table, pull_message_commits (migration 0045), written after a push rather than on the request path. A push already makes work assess each moved pull request's mergeability (mergeability.rs after_push, which asks repos divergence for its head, its base's head and its merge base), so the commits are read there, once, with the merge base divergence already found. The request path then only reads stored data. A cache in repos keyed by commits would also never be stale, but it would still be filled on the first page view, which is the 8 s this removes, and repos' in-isolate caches do not outlive the isolate. Why they are never stale. A row holds the head and merge base its commits were read for, and the pair of commits (mergeable_key, "head..base") they were last confirmed for. merge_choices and the merge itself use a row only while its head is the pull request's head and its pair is the pull request's mergeable_key (kept_is_current); any push to either side moves mergeable_key first, so a list read for another head or base is never used, and a row that could not be confirmed is passed over. At assessment (keeping): the same head and the same merge base means the base moved without taking any of the branch's commits, so the list holds and only its pair is moved on (one UPDATE, no git read; this is what a push to main does to every open pull request into it); a new head or merge base reads them again; no history in common keeps nothing. Nothing is read when no method the repository allows writes commits into a message (needs_commits), and a list over 1 MB of JSON is not kept. Rows go with their repository (repo.purged). On the request path, merge_choices reads the kept commits beside the settings and whether it is behind (one more D1 read in the same round), and only when none are current asks repos, as before. A merged or closed pull request never reads commits. inspect_commits itself. It takes two optional fields: merge_base, which skips reading both 1,000-commit histories and the walk for the merge base when the caller knows it, and messages_only, which returns each commit's message, addresses and parents from the pack alone (rule_facts::messages): no tree is read and identity is not asked about signatures. Default messages always ask for messages only. Rules (rulesets.rs pull_commits) still get full facts. The pull request page asks for the merge options beside getPull instead of after it, and sets the answer aside for a pull request that turns out merged or closed; the merge box still opens on the settings and takes the service's choices when they arrive (useArrived), keeping the method picked and an open dialog. Expected: with commits kept, merge-options costs what the settings-only box did: the pull request's lookup and one round of D1 reads, a few hundred milliseconds, and the merge box's choices arrive well within the stream. A pull request with nothing kept yet (no push since this deploys, a base just changed by hand, or one over the limit) still asks repos, but for messages only: the histories, the merge base and one pack, without the 250 commits' tree reads or identity's lookups; its first push after this deploys keeps them. This is reasoned from the calls removed; it is not measured against the live store.

Checked:: cargo test for g1t-work (193), g1t-repos (342) and g1t-contracts (312); the site's typecheck and unit tests (934 passed, 1 skipped). New tests: kept commits are used only for their head and pair; they are read again only when the head or merge base moves; they read back as written; merge commits are left out, oldest first; messages are read from the pack alone.
G1t-Session:: ses_01m4y12gdde2r9q832m7xk87kd

syntaqxcommitted 2d agoParent6e24edfBrowse files

88	88		Some(out)
89	89		}
90	90
	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	+
91	115		/// The files that differ between two trees, deletions included, with the
92	116		/// size of each new blob the pack holds. Whether the list is complete.
93	117		/// Each level of the trees is read at once.
⋯
305	329		assert_eq!(added(&pack, &"0".repeat(40), 10), Some(Vec::new()), "a tip the pack does not hold adds nothing");
306	330		}
307	331
	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	+
308	364		/// Histories by where they start; a start missing from it fails.
309	365		struct Histories(HashMap<String, Vec<g1t_contracts::repos::Commit>>);
310	366

319	319		}
320	320
321	321		/// 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.
323	324		pub(crate) async fn inspect_commits(&self, a: InspectCommitsArgs) -> Result<Outcome<InspectedCommits>> {
324	325		let (Some(source), Some(target)) = (self.registry.by_id(&a.source_id).await?, self.registry.by_id(&a.target_id).await?) else {
325	326		return Ok(Outcome::fail(FailureCode::NotFound, "Repository not found."));
326	327		};
327	328		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	+	}
335	345		};
336	346		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?;
338	348		if fetched.len() > crate::secret_scan::MAX_SCANNED_PUSH {
339	349		return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: false }));
340	350		}
⋯
346	356		}
347	357		};
348	358		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 {
350	360		return Ok(Outcome::Ok(InspectedCommits { commits: Vec::new(), complete: false }));
351	361		};
	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	+	}
352	367		let (fingerprints, emails) = rule_facts::signing_facts(&pack, &ids);
353	368		let owners = self.signing_owners(&fingerprints, &emails).await;
354	369		let commits = rule_facts::read_all(&pack, &source_git, &ids, Some(&owners)).await?;

10	10		//! probe in a sandbox, which merges the two without an agent and reports the
11	11		//! files that conflict. Only so many probes run at once per repository; the
12	12		//! 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).
13	16
14	17		use std::collections::HashSet;
15	18
⋯
113	116		/// being. Settles it at once where it can, and asks for a probe where
114	117		/// it cannot.
115	118		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>> {
116	125		if !pull.status.is_active() {
117		−	return Ok(());
	126	+	return Ok(None);
118	127		}
119	128		let divergence: Option<Divergence> = g1t_kit::call(
120	129		&self.repos,
⋯
129	138		.await?;
130	139		// Nothing pushed yet.
131	140		let Some(divergence) = divergence else {
132		−	return Ok(());
	141	+	return Ok(None);
133	142		};
134	143		let key = key(&divergence);
135	144		let now = now_ms();
⋯
155	164		_ => false,
156	165		};
157	166		if settled {
158		−	return Ok(());
	167	+	return Ok(Some(divergence));
159	168		}
160	169		}
161	170		if !needs_probe(&divergence) {
⋯
173	182		if row.is_some_and(\|row\| row.mergeable.as_deref() == Some("conflicting")) {
174	183		self.announce_mergeability(pull).await?;
175	184		}
176		−	return Ok(());
	185	+	return Ok(Some(divergence));
177	186		}
178	187		self.db
179	188		.prepare(
⋯
190	199		])?
191	200		.run()
192	201		.await?;
193		−	self.ask_for_probe(pull, &divergence.head).await
	202	+	self.ask_for_probe(pull, &divergence.head).await?;
	203	+	Ok(Some(divergence))
194	204		}
195	205
196	206		async fn ask_for_probe(&self, pull: &Pull, head: &str) -> Result<()> {
⋯
244	254		if moved.contains(&id) {
245	255		self.keep_pull_refs(&pull).await;
246	256		}
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	+	}
248	265		}
249	266		Ok::<_, worker::Error>(())
250	267		};

8	8
9	9		use g1t_contracts::access::{self, Capability};
10	10		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};
12	12		use g1t_contracts::rules::{InspectCommitsArgs, InspectedCommits, MergeMethod};
13	13		use g1t_contracts::work::*;
14	14		use g1t_contracts::{FailureCode, Outcome, PrincipalKind, User};
	15	+	use g1t_kit::now_ms;
15	16		use g1t_rules::merge::{Requirements, method_label};
	17	+	use serde::Deserialize;
16	18		use worker::Result;
17	19
18	20		use crate::merge_message::{self, CommitText, MessageFacts};
⋯
31	33
32	34		/// How many of a pull request's commits its default messages read.
33	35		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	+	}
34	53
	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	+
35	147		/// Methods named for people, joined: "squash or rebase".
36	148		fn labels(methods: &[MergeMethod]) -> String {
37	149		let named: Vec<&str> = methods.iter().map(\|method\| method_label(*method)).collect();
⋯
178	290		}
179	291
180	292		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>> {
184	303		let Some(head) = pull.head_commit.clone() else {
185	304		return Ok(Vec::new());
186	305		};
⋯
190	309		&InspectCommitsArgs {
191	310		source_id: pull.source_id().to_owned(),
192	311		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(),
195	314		limit: Some(MESSAGE_COMMITS),
	315	+	merge_base,
	316	+	messages_only: true,
196	317		},
197	318		)
198	319		.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
210	361		}
211	362
	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	+
212	437		/// The default title and body of the commit each of `methods` makes
213	438		/// for `pull`, as the repository's settings write them: `None` for one
214	439		/// that makes no commit of its own.
⋯
223	448		self.commit_texts(repo, pull).await?
224	449		} else {
225	450		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)),
233	451		};
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))
253	453		}
254	454
255	455		/// The whole message of the commit a merge by `method` makes: what the
⋯
281	481		let (repo, mut pull) = check!(self.pull_at(&a.repo, a.number, &a.viewer).await?);
282	482		crate::fill_base(&mut pull, &repo);
283	483		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?;
286	492		let methods = offered(&settings, &requirements);
287	493		let default_method = choose_method(None, &settings, &requirements, &base).ok();
288	494		let viewer = a.viewer.as_ref();
⋯
295	501		(None, true) => true,
296	502		(None, false) => access::can(Some(viewer), &repo, Capability::Push),
297	503		});
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);
299	515		let options = messages
300	516		.into_iter()
301	517		.map(\|(method, message)\| {
⋯
627	843		assert!(issues_to_close(&fixes, &repo, &settings, false).is_empty());
628	844		}
629	845
	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	+
630	907		#[test]
631	908		fn the_timeline_says_how_it_merged() {
632	909		assert_eq!(merged_note(MergeMethod::Squash), "squashed and merged this");

144	144		// Its labels and milestone, with the repository's to choose from.
145	145		const labelsFound = work.listLabels(path, viewer);
146	146		// 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.
149	150		const choicesFound =
150	151		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,
156	155		)
157	156		.catch(() => null)
158	157		: Promise.resolve(null);

1296	1296		pub base_branch: String,
1297	1297		#[serde(default)]
1298	1298		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,
1299	1308		}
1300	1309
1301	1310		#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]

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	+	);

979	979		target_id: repo.id.clone(),
980	980		base_branch: base.to_owned(),
981	981		limit: None,
	982	+	merge_base: None,
	983	+	messages_only: false,
982	984		},
983	985		)
984	986		.await?;

11	11		/// The longest commit message body a merger may write.
12	12		pub(crate) const MAX_MESSAGE_CHARS: usize = 64_000;
13	13
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)]
16	17		pub(crate) struct CommitText {
17	18		pub message: String,
18	19		pub author_email: Option<String>,

47	47		"DELETE FROM overlap_pairs WHERE repo_id = ?1",
48	48		"DELETE FROM check_runs WHERE pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
49	49		"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)",
50	51		"DELETE FROM agent_messages WHERE repo_id = ?1 OR pull_id IN (SELECT id FROM pulls WHERE repo_id = ?1)",
51	52		"DELETE FROM queue_entries WHERE repo_id = ?1",
52	53		"DELETE FROM queue_probes WHERE repo_id = ?1",
⋯
264	265		}
265	266		// Rows kept by pull request go before the pull requests do.
266	267		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"] {
268	269		let at = PURGED.iter().position(\|sql\| sql.contains(&format!("FROM {table} "))).unwrap();
269	270		assert!(at < pulls, "{table}");
270	271		}

flagon-io/g1t

Comments