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

Merging a pull request that has fallen behind waits for the required checks on the commit that brings it up to date, and a merge the repository does not answer says so instead of hanging. Pressing Merge on a pull request behind its target brought it up to date and then landed it at once, asking to skip its checks; a ruleset that requires checks and does not allow skipping them (flagon-io/g1t's "Main stays deployable", CI required) refused that, so every catch-up-then-merge there stopped at "Needs you" with "The required check CI from actions has not reported on its latest commit". Now the merge waits: once the catch-up push arrives, it lands as soon as every required check passes on that new commit (decided as each of the commit's checks finishes, statuses.rs store_status), at once when none are required, and when a required check fails there it stops with "It was brought up to date, but the required check CI failed on that commit, so it was not merged. Push a fix, then merge again." Two checks finishing together land it once (the waiting merge is taken in one statement). A merge asked to go past failing checks, where the repository allows that, still lands without waiting. The wait stands for three hours (the catch-up push still for 30 minutes); a pull request g1t is seeing through shows it as Checking. The merge box says "Merging once checks pass: Brought up to date with main. It lands once the required check CI passes on that commit.", then "Landing now."; while it is still being brought up to date it says it lands once the required checks pass on that commit, when there are any.

The merge's two writes to the repository service, the catch-up push (update_pull_branch) and the landing (land), now wait at most 25 seconds each (g1t_kit::call_within, new: a call raced against a deadline, logging which method did not answer), so a stalled repos call fails back with "g1t couldn't reach the repository to merge this. Try again in a moment." (unavailable) instead of holding the click until the site's 60-second deadline threw and the page showed a 500. Trying again is safe: landing the same checked commit again is recognised as done, and a catch-up whose push arrives late no longer has a merge waiting on it. The site's merge action turns a merge call that throws or misses its deadline into the same message in the merge box rather than an error page. The pull requests guide says what happens when a behind pull request is merged with required checks, and when the repository does not answer.

Checked:: cargo test -p g1t-work (146, new: a caught-up merge waits for, lands on and stops on its required checks; the unreachable message; an older waiting merge without ignore_checks still reads; the Checking stage while it waits) and -p g1t-kit (23, new: an answer in time passes through, a late one is not waited for); cargo check -p g1t-work -p g1t-kit --target wasm32-unknown-unknown; the site's tsc -b --force and node tests show nothing in pull.tsx (their only errors, and 5 failing tests, are in AI provider and integrations files, from the main checkout's uncommitted contracts the worktree's borrowed node_modules resolved to).

syntaqxcommitted 2d agoParent22641e9Browse files

36	36		type RepoPath,
37	37		type SessionEntry,
38	38		type Viewer,
	39	+	fail,
39	40		pullComparison,
40	41		workOwner,
41	42		} from "@g1t/contracts";
⋯
430	431		}
431	432		const result =
432	433		action === "merge"
433		−	? await work.mergePull(user, path, number, {
434		−	keepIssueOpen: form.get("keepIssueOpen") === "on",
435		−	ignoreChecks: form.get("ignoreChecks") === "on",
436		−	bypassRules: form.get("bypassRules") === "on",
437		−	})
	434	+	? // A merge that did not answer in time (lib/deadline.ts) or broke
	435	+	// off says so in the merge box, not as an error page: trying again
	436	+	// is safe, as a merge that went through meanwhile is recognised.
	437	+	await work
	438	+	.mergePull(user, path, number, {
	439	+	keepIssueOpen: form.get("keepIssueOpen") === "on",
	440	+	ignoreChecks: form.get("ignoreChecks") === "on",
	441	+	bypassRules: form.get("bypassRules") === "on",
	442	+	})
	443	+	.catch(() => fail("unavailable", "g1t couldn't reach the repository to merge this. Try again in a moment."))
438	444		: action === "unqueue"
439	445		? await work.removeFromQueue(user, path, number)
440	446		: action === "close"
⋯
606	612		);
607	613		}
608	614
	615	+	/** The required checks a caught-up merge still waits for: "CI passes", "CI and Deploy pass". */
	616	+	function awaitedChecks(checks: { name: string; state: string }[]): string {
	617	+	const names = checks.filter((check) => check.state !== "success").map((check) => check.name);
	618	+	if (names.length === 0) return "the required checks pass";
	619	+	if (names.length === 1) return `the required check ${names[0]} passes`;
	620	+	return `the required checks ${names.slice(0, -1).join(", ")} and ${names[names.length - 1]} pass`;
	621	+	}
	622	+
609	623		export default function PullPage({ loaderData, actionData, params }: Route.ComponentProps) {
610	624		const {
611	625		pull,
⋯
1153	1167		</StatusRow>
1154	1168		)}
1155	1169		{landing ? (
	1170	+	// Brought up to date first, then landed once the
	1171	+	// required checks pass on that new commit.
1156	1172		<StatusRow
1157	1173		icon={<Loader size={16} className="animate-spin text-accent" />}
1158		−	title="Merging"
	1174	+	title={!behind && unchecked ? "Merging once checks pass" : "Merging"}
1159	1175		>
1160		−	{defaultBranch} has moved, so it is being brought up to date first. It
1161		−	lands as soon as that is done.
	1176	+	{behind
	1177	+	? `${defaultBranch} has moved, so it is being brought up to date first. ${
	1178	+	requiredChecks.length > 0
	1179	+	? "It lands once the required checks pass on that commit."
	1180	+	: "It lands as soon as that is done."
	1181	+	}`
	1182	+	: unchecked
	1183	+	? `Brought up to date with ${defaultBranch}. It lands once ${awaitedChecks(requiredChecks)} on that commit.`
	1184	+	: `Brought up to date with ${defaultBranch}. Landing now.`}
1162	1185		</StatusRow>
1163	1186		) : conflicting ? (
1164	1187		<ConflictsSection

64	64		read_answer(method, &text)
65	65		}
66	66
	67	+	/// Calls `method` like [`call`], waiting at most `within` for the answer:
	68	+	/// `None` when none came in time. Nothing on Workers ends a subrequest that
	69	+	/// stops answering, so a caller that must answer someone (a click on Merge)
	70	+	/// bounds the calls it makes this way and says what happened instead of
	71	+	/// hanging until its own caller gives up. The call is not cancelled: what
	72	+	/// it was doing may still finish, so only a call that is safe to make
	73	+	/// again (or whose late result is harmless) should be bounded.
	74	+	pub async fn call_within<A: Serialize, R: DeserializeOwned>(
	75	+	service: &Fetcher,
	76	+	method: &str,
	77	+	arguments: &A,
	78	+	within: std::time::Duration,
	79	+	) -> Result<Option<R>> {
	80	+	match first_of(call(service, method, arguments), worker::Delay::from(within)).await {
	81	+	Some(answer) => answer.map(Some),
	82	+	None => {
	83	+	worker::console_error!("{method} did not answer within {} s", within.as_secs());
	84	+	Ok(None)
	85	+	}
	86	+	}
	87	+	}
	88	+
	89	+	/// `work`'s output if it finishes before `deadline` does, `None` otherwise.
	90	+	pub async fn first_of<W: std::future::Future, D: std::future::Future>(work: W, deadline: D) -> Option<W::Output> {
	91	+	use std::task::Poll;
	92	+	let mut work = Box::pin(work);
	93	+	let mut deadline = Box::pin(deadline);
	94	+	std::future::poll_fn(\|cx\| {
	95	+	if let Poll::Ready(output) = work.as_mut().poll(cx) {
	96	+	return Poll::Ready(Some(output));
	97	+	}
	98	+	if deadline.as_mut().poll(cx).is_ready() {
	99	+	return Poll::Ready(None);
	100	+	}
	101	+	Poll::Pending
	102	+	})
	103	+	.await
	104	+	}
	105	+
	106	+	#[cfg(test)]
	107	+	mod deadline_tests {
	108	+	use super::first_of;
	109	+	use std::future::{pending, ready};
	110	+	use std::task::{Context, Poll, Waker};
	111	+
	112	+	fn now<F: std::future::Future>(future: F) -> Poll<F::Output> {
	113	+	let mut future = Box::pin(future);
	114	+	future.as_mut().poll(&mut Context::from_waker(Waker::noop()))
	115	+	}
	116	+
	117	+	#[test]
	118	+	fn an_answer_in_time_is_passed_through_and_a_late_one_is_not_waited_for() {
	119	+	assert_eq!(now(first_of(ready(7), pending::<()>())), Poll::Ready(Some(7)));
	120	+	assert_eq!(now(first_of(pending::<u8>(), ready(()))), Poll::Ready(None));
	121	+	// Both ready: the answer wins.
	122	+	assert_eq!(now(first_of(ready(1), ready(()))), Poll::Ready(Some(1)));
	123	+	assert_eq!(now(first_of(pending::<u8>(), pending::<()>())), Poll::Pending);
	124	+	}
	125	+	}
	126	+
67	127		/// A method's answer, read from its body.
68	128		pub fn read_answer<R: DeserializeOwned>(method: &str, text: &str) -> Result<R> {
69	129		serde_json::from_str(text).map_err(\|error\| {

2211	2211		check!(allowed(Some(&a.actor), &repo, Capability::Merge));
2212	2212		self.record_merge_evaluations(&repo, &pull, &gate).await;
2213	2213		// Brought up to date here, without a sandbox: the merge is
2214		−	// recorded first, and lands when the push that catches it up
2215		−	// arrives (lifecycle.rs `land_if_requested`).
2216		−	self.request_landing(&pull, &a.actor, a.keep_issue_open)
	2214	+	// recorded first, and lands once the push that catches it up
	2215	+	// arrives and the required checks pass on that commit
	2216	+	// (lifecycle.rs `land_if_requested`, `land_when_checked`).
	2217	+	// Merging past failing checks, where that is allowed, carries
	2218	+	// over: it then lands without waiting for them.
	2219	+	self.request_landing(&pull, &a.actor, a.keep_issue_open, a.ignore_checks && settings.allow_ignoring_checks)
2217	2220		.await?;
2218		−	let updated: Outcome<PullBranchUpdate> = g1t_kit::call(
	2221	+	// Bounded: a repository that does not answer gets a clear
	2222	+	// failure back to whoever clicked, not a request that hangs
	2223	+	// until the site gives up on it.
	2224	+	let updated: Option<Outcome<PullBranchUpdate>> = g1t_kit::call_within(
2219	2225		&self.repos,
2220	2226		"update_pull_branch",
2221	2227		&UpdatePullBranchArgs {
⋯
2225	2231		actor: a.actor.clone(),
2226	2232		target_branch: pull.base.clone(),
2227	2233		},
	2234	+	lifecycle::REPOS_WRITE_WITHIN,
2228	2235		)
2229	2236		.await?;
	2237	+	// A push that still arrives later only brings it up to date.
	2238	+	let updated = updated.unwrap_or_else(lifecycle::repository_unreachable);
2230	2239		match updated {
2231	2240		Outcome::Ok(PullBranchUpdate::Updated { .. }) => return Ok(Outcome::Ok(pull)),
2232	2241		// Moved again in between, or already caught up: land it now.
⋯
2249	2258		}
2250	2259
2251	2260		// Whether the actor may write to the repository is decided by repos.
2252		−	let landed: Outcome<Landed> = g1t_kit::call(
	2261	+	// Bounded like the catch-up above. Landing the same checked commit
	2262	+	// again is recognised as done (repos `land`), so trying again after
	2263	+	// a land that finished late records it.
	2264	+	let landed: Option<Outcome<Landed>> = g1t_kit::call_within(
2253	2265		&self.repos,
2254	2266		"land",
2255	2267		&LandArgs {
⋯
2260	2272		target_branch: Some(base.clone()),
2261	2273		expected_commit: pull.head_commit.clone(),
2262	2274		},
	2275	+	lifecycle::REPOS_WRITE_WITHIN,
2263	2276		)
2264	2277		.await?;
2265		−	let landed = check!(landed);
	2278	+	let landed = check!(landed.unwrap_or_else(lifecycle::repository_unreachable));
2266	2279		self.record_merge_evaluations(&repo, &pull, &gate).await;
2267	2280		Ok(Outcome::Ok(
2268	2281		self.record_merge(&repo, pull, &a.actor, a.keep_issue_open, landed)

8	8		//! branch's rules ask for is met, attributed to `g1t` (`POLICY_ACTOR_ID`).
9	9		//!
10	10		//! A merge asked for while a pull request is behind is recorded here
11		−	//! (`request_landing`) and lands once the push that brings it up to date
12		−	//! arrives (`land_if_requested`).
	11	+	//! (`request_landing`). Once the push that brings it up to date arrives
	12	+	//! (`land_if_requested`), it lands when the checks the branch requires
	13	+	//! pass on that new commit, as they must for any merge (`land_when_checked`,
	14	+	//! also called as each of the commit's checks finishes).
13	15
14	16		use g1t_contracts::repos::{GetByIdArgs, Repo, RepoPath};
15	17		use g1t_contracts::time::rfc3339;
⋯
31	33		/// How long a claimed step is waited for before it may be taken again.
32	34		const CATCH_UP_MINUTES: u64 = 30;
33	35		const MERGE_MINUTES: u64 = 2;
	36	+	/// How long a merge that was brought up to date waits for the required
	37	+	/// checks on the new commit; after that it is no longer pending.
	38	+	const CHECKS_MINUTES: u64 = 180;
	39	+	/// The step under way while it does (`working_on`).
	40	+	const WAITING_FOR_CHECKS: &str = "checks";
	41	+
	42	+	/// How long a merge waits on the repository service for one write (the
	43	+	/// catch-up push, the landing): long enough for a large merge, short
	44	+	/// enough that the person who clicked hears back well before the site
	45	+	/// stops waiting on the merge (apps/web/app/lib/deadline.ts, 60 s).
	46	+	pub(crate) const REPOS_WRITE_WITHIN: std::time::Duration = std::time::Duration::from_secs(25);
	47	+
	48	+	/// What a merge says when the repository service did not answer in time.
	49	+	pub(crate) fn repository_unreachable<T>() -> Outcome<T> {
	50	+	Outcome::fail(
	51	+	FailureCode::Unavailable,
	52	+	"g1t couldn't reach the repository to merge this. Try again in a moment.",
	53	+	)
	54	+	}
	55	+
	56	+	/// What becomes of a merge that was brought up to date, by the checks the
	57	+	/// branch requires on the commit that did it.
	58	+	#[derive(Debug, PartialEq)]
	59	+	pub(crate) enum Landing {
	60	+	/// None are required, or all of them passed.
	61	+	Now,
	62	+	/// Some are still running or have not reported yet.
	63	+	Wait,
	64	+	/// One failed: a person has to push a fix.
	65	+	Stall(String),
	66	+	}
	67	+
	68	+	/// Whether a merge that was brought up to date lands now, by its new
	69	+	/// commit's checks. The same checks hold it as hold any merge
	70	+	/// (`merge_gate`): landing it before they report would only be refused.
	71	+	pub(crate) fn landing_step(facts: &WorkflowFacts) -> Landing {
	72	+	let failed = facts.required_failed();
	73	+	if !failed.is_empty() {
	74	+	let one = failed.len() == 1;
	75	+	return Landing::Stall(format!(
	76	+	"It was brought up to date, but the required {} {} failed on that commit, so it was not merged. Push a fix, then merge again.",
	77	+	if one { "check" } else { "checks" },
	78	+	statuses::list(&failed),
	79	+	));
	80	+	}
	81	+	if facts.required.iter().any(\|check\| check.state != RequiredState::Success) {
	82	+	return Landing::Wait;
	83	+	}
	84	+	Landing::Now
	85	+	}
	86	+
34	87		/// Who a merge made by a repository's settings is attributed to. Not an
35	88		/// account: `g1t` cannot be registered.
36	89		pub(crate) const POLICY_ACTOR_ID: &str = "g1t_policy";
⋯
54	107		struct LandRequest {
55	108		actor: User,
56	109		keep_issue_open: bool,
	110	+	/// The merger asked to merge past the checks, and the repository
	111	+	/// allows that: it lands without waiting for them.
	112	+	#[serde(default)]
	113	+	ignore_checks: bool,
57	114		}
58	115
59	116		#[derive(Deserialize)]
⋯
61	118		land_requested: Option<String>,
62	119		land_requested_at: Option<String>,
63	120		stalled: Option<String>,
	121	+	#[serde(default)]
	122	+	working_on: Option<String>,
64	123		}
65	124
66	125		/// What should happen next, if it is g1t's turn.
⋯
158	217		return wait(Stage::CatchingUp, "Bringing it up to date with the branch it will land on.");
159	218		}
160	219		Some("merge") => return wait(Stage::Ready, "Merging."),
	220	+	Some(WAITING_FOR_CHECKS) => {
	221	+	return wait(
	222	+	Stage::Checking,
	223	+	"Brought up to date. It lands once the required checks pass on that commit.",
	224	+	);
	225	+	}
161	226		_ => {}
162	227		}
163	228		if let Some(name) = &facts.person_request {
⋯
507	572		pull: &Pull,
508	573		actor: &User,
509	574		keep_issue_open: bool,
	575	+	ignore_checks: bool,
510	576		) -> Result<()> {
511	577		let now = now_ms();
512	578		let request = serde_json::to_string(&LandRequest {
513	579		actor: actor.clone(),
514	580		keep_issue_open,
	581	+	ignore_checks,
515	582		})?;
516	583		self.db
517	584		.prepare(
⋯
541	608		/// enough to still stand.
542	609		async fn land_request(&self, pull_id: &str) -> Result<Option<LandRequest>> {
543	610		let row = self.land_row(pull_id).await?;
544		−	let oldest = rfc3339(now_ms().saturating_sub(CATCH_UP_MINUTES * 60 * 1000));
	611	+	let now = now_ms();
545	612		Ok(row
546	613		.filter(\|row\| {
	614	+	// Waiting for checks on the caught-up commit stands longer
	615	+	// than waiting for the push.
	616	+	let minutes = if row.working_on.as_deref() == Some(WAITING_FOR_CHECKS) {
	617	+	CHECKS_MINUTES
	618	+	} else {
	619	+	CATCH_UP_MINUTES
	620	+	};
	621	+	let oldest = rfc3339(now.saturating_sub(minutes * 60 * 1000));
547	622		row.land_requested_at
548	623		.as_deref()
549	624		.is_some_and(\|at\| at >= oldest.as_str())
⋯
566	641		return found.first::<LandRow>(Slot::Pull);
567	642		}
568	643		self.db
569		−	.prepare("SELECT land_requested, land_requested_at, stalled FROM pulls WHERE id = ?")
	644	+	.prepare("SELECT land_requested, land_requested_at, stalled, working_on FROM pulls WHERE id = ?")
570	645		.bind(&[pull_id.into()])?
571	646		.first::<LandRow>(None)
572	647		.await
573	648		}
574	649
575	650		/// Forgets a merge that was waiting on a pull request, and the step
576		−	/// that was bringing it up to date.
	651	+	/// that was bringing it up to date or waiting for its checks.
577	652		pub(crate) async fn forget_landing(&self, pull_id: &str) -> Result<()> {
578		−	self.db
	653	+	self.take_landing(pull_id).await?;
	654	+	Ok(())
	655	+	}
	656	+
	657	+	/// Forgets a merge that was waiting on a pull request and returns it,
	658	+	/// in one statement, so that of two callers deciding to land it at
	659	+	/// once (two checks finishing together) only one does.
	660	+	async fn take_landing(&self, pull_id: &str) -> Result<Option<LandRequest>> {
	661	+	// SET sees the row as it was: the CASEs read the old `working_on`.
	662	+	let taken = self
	663	+	.db
579	664		.prepare(
580	665		"UPDATE pulls SET land_requested = NULL, land_requested_at = NULL,
581		−	working_on = CASE WHEN working_on = 'catch_up' THEN NULL ELSE working_on END,
582		−	working_until = CASE WHEN working_on = 'catch_up' THEN NULL ELSE working_until END
583		−	WHERE id = ?",
	666	+	working_on = CASE WHEN working_on IN ('catch_up', 'checks') THEN NULL ELSE working_on END,
	667	+	working_until = CASE WHEN working_on IN ('catch_up', 'checks') THEN NULL ELSE working_until END
	668	+	WHERE id = ? AND land_requested IS NOT NULL
	669	+	RETURNING land_requested AS value",
584	670		)
585	671		.bind(&[pull_id.into()])?
	672	+	.first::<ValueRow>(None)
	673	+	.await?;
	674	+	Ok(taken.and_then(\|row\| serde_json::from_str(&row.value).ok()))
	675	+	}
	676	+
	677	+	/// Notes that a merge which was brought up to date waits for the
	678	+	/// required checks on its new commit, from now: on the first look at
	679	+	/// this commit (a later push clears the step and starts it again).
	680	+	async fn wait_for_checks(&self, pull_id: &str) -> Result<()> {
	681	+	let now = now_ms();
	682	+	self.db
	683	+	.prepare(
	684	+	"UPDATE pulls SET working_on = 'checks', working_until = ?1, land_requested_at = ?2
	685	+	WHERE id = ?3 AND land_requested IS NOT NULL AND status = 'open'
	686	+	AND (working_on IS NULL OR working_on <> 'checks')",
	687	+	)
	688	+	.bind(&[
	689	+	rfc3339(now + CHECKS_MINUTES * 60 * 1000).into(),
	690	+	rfc3339(now).into(),
	691	+	pull_id.into(),
	692	+	])?
586	693		.run()
587	694		.await?;
588	695		Ok(())
589	696		}
590	697
591		−	/// Lands a pull request whose head has just moved, if a merge of it was
592		−	/// waiting for exactly that. What it was caught up to was already
593		−	/// checked and reviewed apart from the merge, so the checks are not
594		−	/// waited for again; a repository that wants them rerun turns on
595		−	/// "require up to date", and then nothing is landed this way.
	698	+	/// Moves on a merge that was waiting for a pull request's head to move
	699	+	/// (the push that brought it up to date): it lands once the checks the
	700	+	/// branch requires pass on that commit ([`Work::land_when_checked`]).
	701	+	/// The commit is new, so they are waited for, as for any merge: a
	702	+	/// ruleset that requires them refuses a merge that skips them.
596	703		pub(crate) async fn land_if_requested(&self, pull_id: &str) -> Result<()> {
	704	+	if self.land_request(pull_id).await?.is_none() {
	705	+	return Ok(());
	706	+	}
	707	+	self.land_when_checked(pull_id).await
	708	+	}
	709	+
	710	+	/// Lands a pull request that was brought up to date for a merge, once
	711	+	/// the checks the branch requires pass on its head: at once when none
	712	+	/// are required, never when one failed (it stops and says which), and
	713	+	/// otherwise when the last of them reports (statuses.rs `store_status`
	714	+	/// calls this as the commit's checks finish).
	715	+	pub(crate) async fn land_when_checked(&self, pull_id: &str) -> Result<()> {
597	716		let Some(request) = self.land_request(pull_id).await? else {
598	717		return Ok(());
599	718		};
600		−	self.forget_landing(pull_id).await?;
601	719		let Some(pull) = self.pull_by_id(pull_id).await? else {
602	720		return Ok(());
603	721		};
	722	+	if pull.status != PullStatus::Open {
	723	+	return self.forget_landing(pull_id).await;
	724	+	}
604	725		let repo: Outcome<Repo> = g1t_kit::call(
605	726		&self.repos,
606	727		"get_by_id",
⋯
611	732		)
612	733		.await?;
613	734		let Outcome::Ok(repo) = crate::retired::unless_archived(repo) else {
	735	+	return self.forget_landing(pull_id).await;
	736	+	};
	737	+	let step = if request.ignore_checks {
	738	+	Landing::Now
	739	+	} else {
	740	+	let settings = self.settings_on(&repo, &pull).await?;
	741	+	let statuses = self.statuses(&pull.repo_id, pull.head_commit.as_deref()).await?;
	742	+	let facts = self
	743	+	.workflow_facts(&pull.repo_id, pull.head_commit.as_deref(), &statuses, &settings.required_checks)
	744	+	.await?;
	745	+	landing_step(&facts)
	746	+	};
	747	+	match step {
	748	+	Landing::Wait => return self.wait_for_checks(pull_id).await,
	749	+	Landing::Stall(reason) => {
	750	+	self.stall(StallArgs {
	751	+	pull_id: pull.id,
	752	+	reason,
	753	+	by: None,
	754	+	})
	755	+	.await?;
	756	+	return Ok(());
	757	+	}
	758	+	Landing::Now => {}
	759	+	}
	760	+	// Taken by one caller: two checks finishing together land it once.
	761	+	let Some(request) = self.take_landing(pull_id).await? else {
614	762		return Ok(());
615	763		};
616	764		let merged = self
⋯
623	771		number: pull.number,
624	772		summary: String::new(),
625	773		keep_issue_open: request.keep_issue_open,
626		−	ignore_checks: true,
	774	+	ignore_checks: request.ignore_checks,
627	775		bypass_rules: false,
628	776		})
629	777		.await?;
⋯
843	991		}
844	992
845	993		#[test]
	994	+	fn a_merge_brought_up_to_date_lands_once_its_required_checks_pass_on_the_new_commit() {
	995	+	// The catch-up's merge commit, just pushed: nothing has reported yet.
	996	+	assert_eq!(landing_step(&checks(&[], &["CI"])), Landing::Wait);
	997	+	assert_eq!(landing_step(&checks(&[("CI / pull_request", "pending")], &["CI"])), Landing::Wait);
	998	+	assert_eq!(landing_step(&checks(&[("CI / pull_request", "success")], &["CI"])), Landing::Now);
	999	+	// Every required one, not just the first to report.
	1000	+	assert_eq!(
	1001	+	landing_step(&checks(&[("CI / pull_request", "success")], &["CI", "Deploy"])),
	1002	+	Landing::Wait
	1003	+	);
	1004	+	// Nothing required: it lands at once, whatever else runs or failed.
	1005	+	assert_eq!(landing_step(&checks(&[], &[])), Landing::Now);
	1006	+	assert_eq!(landing_step(&checks(&[("Lint / pull_request", "failure")], &[])), Landing::Now);
	1007	+	assert_eq!(
	1008	+	landing_step(&checks(&[("CI / pull_request", "success"), ("Lint / pull_request", "failure")], &["CI"])),
	1009	+	Landing::Now
	1010	+	);
	1011	+	// A required one that failed stops it, saying which.
	1012	+	let Landing::Stall(reason) = landing_step(&checks(&[("CI / pull_request", "failure")], &["CI"])) else {
	1013	+	panic!("a failing required check must stop it");
	1014	+	};
	1015	+	assert_eq!(
	1016	+	reason,
	1017	+	"It was brought up to date, but the required check CI failed on that commit, so it was not merged. Push a fix, then merge again."
	1018	+	);
	1019	+	let Landing::Stall(reason) =
	1020	+	landing_step(&checks(&[("CI / push", "error"), ("Deploy", "failure")], &["CI", "Deploy"]))
	1021	+	else {
	1022	+	panic!("failing required checks must stop it");
	1023	+	};
	1024	+	assert!(reason.contains("required checks CI and Deploy failed"), "{reason}");
	1025	+	}
	1026	+
	1027	+	#[test]
	1028	+	fn a_merge_that_cannot_reach_the_repository_says_so() {
	1029	+	let Outcome::Fail(failure) = repository_unreachable::<()>() else {
	1030	+	panic!("it must fail");
	1031	+	};
	1032	+	assert_eq!(failure.code, FailureCode::Unavailable);
	1033	+	assert!(failure.message.contains("couldn't reach the repository"));
	1034	+	assert!(failure.message.contains("Try again"));
	1035	+	// Well inside the site's own 60 s wait on a merge, catch-up and landing both.
	1036	+	assert!(REPOS_WRITE_WITHIN.as_secs() * 2 < 60);
	1037	+	}
	1038	+
	1039	+	#[test]
	1040	+	fn a_landing_asked_for_before_it_carried_ignore_checks_still_reads() {
	1041	+	let old: LandRequest = serde_json::from_value(serde_json::json!({
	1042	+	"actor": serde_json::to_value(User::default()).unwrap(),
	1043	+	"keepIssueOpen": true,
	1044	+	}))
	1045	+	.unwrap();
	1046	+	assert!(old.keep_issue_open && !old.ignore_checks);
	1047	+	}
	1048	+
	1049	+	#[test]
846	1050		fn drafts_steps_under_way_and_stops_wait() {
	1051	+	assert_eq!(
	1052	+	outcome(Facts { working_on: Some(WAITING_FOR_CHECKS.into()), auto_merge: true, ..facts() }),
	1053	+	(Stage::Checking, "wait")
	1054	+	);
847	1055		assert_eq!(outcome(Facts { draft: true, ..facts() }), (Stage::Working, "wait"));
848	1056		assert_eq!(outcome(Facts { working_on: Some("catch_up".into()), auto_merge: true, ..facts() }), (Stage::CatchingUp, "wait"));
849	1057		assert_eq!(outcome(Facts { working_on: Some("merge".into()), auto_merge: true, ..facts() }), (Stage::Ready, "wait"));

340	340
341	341		A pull request that only has fallen behind its target, without conflicts,
342	342		still merges: merging brings it up to date first, unless the repository
343		−	requires pull requests to be up to date.
	343	+	requires pull requests to be up to date. Bringing it up to date makes a new
	344	+	commit, so when the target branch requires checks, the merge box says
	345	+	Merging once checks pass and the pull request lands when the required
	346	+	checks pass on that commit, with nothing more to press. If one of them fails
	347	+	there, it is not merged: the pull request says which check failed, and you
	348	+	push a fix and merge again. With no required checks, it lands as soon as it
	349	+	is up to date.
	350	+
	351	+	If the repository does not answer in time while merging, the merge box says
	352	+	g1t couldn't reach the repository. Merge again: if the first merge went
	353	+	through meanwhile, merging again records it rather than merging twice.
344	354
345	355		## Catching up
346	356

439	439		&a.repo_id,
440	440		None,
441	441		ChecksEvent {
442		−	pull_id: head.id,
	442	+	pull_id: head.id.clone(),
443	443		repo_id: a.repo_id.clone(),
444	444		number: head.number,
445	445		status: if facts.failed.is_empty() { "passed" } else { "failed" },
⋯
447	447		},
448	448		)
449	449		.await?;
	450	+	// A merge that brought it up to date and waits for its required
	451	+	// checks on this commit lands, or stops on one that failed. The
	452	+	// status is kept either way.
	453	+	if let Err(error) = self.land_when_checked(&head.id).await {
	454	+	worker::console_error!("landing {} after its checks: {error}", head.id);
	455	+	}
450	456		}
451	457		Ok(Outcome::Ok(true))
452	458		}

flagon-io/g1t

Comments