Skip to content
Request access

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 pull request #352 from flagon-io/347-close-any-commit

Closing a work item with a commit accepts any commit on the default branch from someone with Write, and a pull request's merge commit that names it

syntaqxcommitted Parents2bc76d73b6cfedBrowse files
14 files+268−590/14 viewed
+1−1
1313 const INSTRUCTIONS: &str = "g1t is where your code, its history and the work around it live: repositories, work items, pull requests, workflows, deployments and packages. You bring your own agent; these tools are how it reads and changes what is on g1t, as the person or token it acts for. Repositories are named \"owner/name\" and belong to a workspace. A work item is any tracked work (a feature, a bug, a chore) of a workspace, linked to the repositories it concerns: where it was filed (origin), where the work is meant to land (targets) and where it touched. A workspace numbers its work items and new pull requests in one sequence; pull requests live in their repository.
1414 Tools are resources, each with an `action`: search, repository, work_item, pull_request, session, workflow, observability, package, secret, webhook, access, team, workspace, ai, billing, security, notifications, account, artifact. The `action` field lists each action and the fields it needs. You see only what your token's scopes allow; a refusal names the scope it needs.
1515 Find a repository: account whoami lists your workspaces; repository list or search finds one. Find work: work_item next gives the work item to take next in a workspace (the first in the plan's Now nobody is on, else the top of Next), and work_item list the rest (with repo, the work items linked to it; with horizon, one horizon of the plan).
16−Work on a work item: work_item work (everything about it: what done means, each repository with its pull requests and whether the work landed there, the targets still missing and suggested ones, who is on it, what it waits on), work_item claim (advisory: it tells you who else is on it), then pull_request create in a target repository with the work item's number as `item`: you get a draft with its own fork to clone and push to. Push your commits, then pull_request ready with a summary. Watch `overlaps` and `behind` on pull_request get, and its checks there: `statuses` from the repository's workflows and `required_checks`, which must pass before it merges. If one fails, read why with workflow get_run and job_logs, push a fix, and the checks run again. Merging lands the work item in that repository; it closes once it has landed in every target (or, with none, once anything that closes it lands). A commit pushed straight to a default branch lands it only when its message says `Closes #N` (or Fixes, Resolves), never with a bare `(#N)`; `Part of #N` ties work to it without closing it; and work_item close with commit (and repo) lands one after the fact. When the work must land in another repository too, work_item link it as a target.
16+Work on a work item: work_item work (everything about it: what done means, each repository with its pull requests and whether the work landed there, the targets still missing and suggested ones, who is on it, what it waits on), work_item claim (advisory: it tells you who else is on it), then pull_request create in a target repository with the work item's number as `item`: you get a draft with its own fork to clone and push to. Push your commits, then pull_request ready with a summary. Watch `overlaps` and `behind` on pull_request get, and its checks there: `statuses` from the repository's workflows and `required_checks`, which must pass before it merges. If one fails, read why with workflow get_run and job_logs, push a fix, and the checks run again. Merging lands the work item in that repository; it closes once it has landed in every target (or, with none, once anything that closes it lands). A commit pushed straight to a default branch lands it only when its message says `Closes #N` (or Fixes, Resolves), never with a bare `(#N)`; `Part of #N` ties work to it without closing it; and work_item close with commit (and repo) lands one after the fact: with the Write role, any commit on the default branch, even one that does not name it. When the work must land in another repository too, work_item link it as a target.
1717 Coordinate: session start once per run of your harness (harness_session_id makes it idempotent), session update as a heartbeat, session get between work steps to read pending updates. Reading does not acknowledge: consider relevant changes, record a public decision summary, then session record with acknowledge containing only the exact update IDs considered. Keep your own session ID explicit, especially when sharing a checkout. Separate scopes may proceed concurrently; agree ownership before overlapping edits. session end when done; work_item release or handoff (with a note) when you stop.
1818 Record public summaries only, never private chain-of-thought, credentials or secrets. As you work, pull_request record_session appends user-facing decisions, evidence and tool activity to the pull request's session, so reviewers see why the change is what it is; pull_request read_session reads it back. pull_request record_decision keeps a decision (why the change is made this way, or what fixed a failure) and list_decisions reads them; repository why says why a line of code is the way it is.
1919 Merge queue: when your pull request fails combined with the ones ahead of it, g1t says which of them it breaks with and the check that failed (on the pull request, its work item and its session, and as integration.failed). Fix it and push: once its checks pass, it rejoins the queue by itself.
+2−2
18611861 "Change a work item's title, body, labels, milestone, project, the people it is assigned to, what it is blocked by, or its parent. Only the fields given are changed; labels, assignees and blocked_by each replace the whole set, milestone null or 0 takes it out of its milestone, project (a slug) null or an empty string out of its project, and parent (as set_work_item_parent) null or 0 out of its parent. blocked_by lists the numbers of the work items of the workspace that have to land first: one that is not a work item, the work item itself, or a set that would make a cycle is refused with `422`, and a change is a dependency.changed event. Its author may change their own work item, as may the person g1t filed one for; anyone else needs the Triage role, and so do the milestone and the project. Each label added or removed is a work_item.labeled or work_item.unlabeled event. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace."
18621862 }
18631863 Op::CloseWorkItem => {
1864− "Close a work item without a pull request. A work item closes by itself once what closes it has landed in every target: a pull request whose title or description names it after a closing keyword (Fixes #12) merging into a repository's default branch, or such a commit pushed there, marks that repository landed. When a commit already on a default branch did the work, give it as commit, with repo the repository it is in: it lands there as its push would have, and the work item closes if nothing else is left to land. A commit that is not on the default branch, or whose message does not name the work item after a closing keyword, is refused with `422`; a work item already closed, with `409`. A work item with open sub-items is never closed with them silently: without sub_items it is refused with `409`, naming them; sub_items leave leaves them open, close closes them too (with the same reason, and their own open sub-items), and move makes them sub-items of move_to. A commit closing it leaves them. Closing the last open sub-item leaves its parent open, and says so on the parent. Its author may close their own work item, as may the person g1t filed one for; anyone else needs the Triage role. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace."
1864+ "Close a work item without a pull request. A work item closes by itself once what closes it has landed in every target: a pull request whose title or description names it after a closing keyword (Fixes #12) merging into a repository's default branch, or such a commit pushed there, marks that repository landed. When a commit already on a default branch did the work, give it as commit, with repo the repository it is in: it lands there as its push would have, and the work item closes if nothing else is left to land. With the Write role on repo, any commit on its default branch will do, and the timeline says it was tied by hand; without it, the commit's message must name the work item after a closing keyword, or it must be the commit a pull request merged as whose own work item, closes or parts names it, else it is refused with `403`. A commit that is not on the default branch is refused with `422`; a work item already closed, with `409`. A work item with open sub-items is never closed with them silently: without sub_items it is refused with `409`, naming them; sub_items leave leaves them open, close closes them too (with the same reason, and their own open sub-items), and move makes them sub-items of move_to. A commit closing it leaves them. Closing the last open sub-item leaves its parent open, and says so on the parent. Its author may close their own work item, as may the person g1t filed one for; anyone else needs the Triage role. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace."
18651865 }
18661866 Op::ReopenWorkItem => {
18671867 "Reopen a closed work item. Its author may reopen their own work item, as may the person g1t filed one for; anyone else needs the Triage role. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace."
32093209 },
32103210 "commit": {
32113211 "type": "string",
3212− "description": "The commit that did it: one on repo's default branch whose message names this work item after a closing keyword (Fixes #12). It lands there as pushing it would have.",
3212+ "description": "The commit that did it, on repo's default branch: any commit there with the Write role, else one whose message names this work item after a closing keyword (Fixes #12), or a pull request's merge commit that names it. It lands there as pushing it would have.",
32133213 },
32143214 "sub_items": {
32153215 "type": "string",
+1−1
14871487 }
14881488 ]
14891489 },
1490− "notes": "With `commit` and `repo`, a commit already on that repository's default branch whose message names the work item after a closing keyword (`Fixes #12`), it lands there as pushing it would have: a resolution is recorded, the event is `work_item.landed` (and `work_item.closed` with `resolutions` once nothing else is left to land), and the timeline says \"landed this in\" the commit, linked. Give `commit` as its full hash. A commit not on the default branch, or one whose message does not name the work item, is refused (`422`); a work item already closed is `409`, saying what closed it. See [Closing work items](/guides/pull-requests/#closing-work-items)."
1490+ "notes": "With `commit` and `repo`, a commit already on that repository's default branch (any commit there with the Write role; without it, one whose message names the work item after a closing keyword, `Fixes #12`, or the commit a pull request naming it merged as), it lands there as pushing it would have: a resolution is recorded, the event is `work_item.landed` (and `work_item.closed` with `resolutions` once nothing else is left to land), and the timeline says \"landed this in\" the commit, linked. Give `commit` as its full hash. A commit not on the default branch is refused (`422`), and one nothing ties to the work item, from someone without Write, `403`; a work item already closed is `409`, saying what closed it. See [Closing work items](/guides/pull-requests/#closing-work-items)."
14911491 },
14921492 "reopen_work_item": {
14931493 "response": {
+1−1
13231323 | `<NUMBER>` | |
13241324 | `-r, --reason <REASON>` | One of `completed`, `not_planned`. |
13251325 | `-c, --comment <COMMENT>` | Leave a comment as it closes. |
1326−| `--commit <COMMIT>` | The commit on the repository's default branch that did it: its message names the work item after a closing keyword (Fixes #12). Its full hash. It lands there; the work item closes when nothing else is left to land. |
1326+| `--commit <COMMIT>` | The commit on the repository's default branch that did it, by its full hash: any commit there with the Write role, else one whose message names the work item (Fixes #12) or a pull request's merge commit that names it. It lands there; the work item closes when nothing else is left to land. |
13271327 | `--sub-items <SUB_ITEMS>` | What to do with its sub-items that are still open. One of `leave`, `close`, `move`. |
13281328 | `--move-to <NUMBER>` | With --sub-items move: the number of the work item they become sub-items of. |
13291329
+1−1
136136 | `g1t work comment N` | Comments on it. `--attach FILE` uploads an image to the work item and adds it to the comment. |
137137 | `g1t work check N BOX` | Ticks a box of its description's [task list](/work/task-lists/), by its number or words of its text; `--uncheck` unticks it. Only that box changes. |
138138 | `g1t work edit N` | Changes its title, description, labels, assignees or milestone. |
139−| `g1t work close N` | Closes it, as completed, or `--reason not_planned`; `--comment` says why. `--commit SHA` names a commit on the default branch that did it, which lands it there. See [closing work items](/work/closing/). |
139+| `g1t work close N` | Closes it, as completed, or `--reason not_planned`; `--comment` says why. `--commit SHA` names a commit on the default branch that did it, which lands it there: any commit with the Write role, even one that does not name it. See [closing work items](/work/closing/). |
140140 | `g1t work reopen N` | Opens it again. |
141141 | `g1t work link N` | Links a repository in a role: `--role target` (the default) or `touched`, the current repository or `-R owner/name`. |
142142 | `g1t work unlink N` | Unlinks a repository, never its origin. |
+24−9
127127
128128 ## Close one with a commit
129129
130−A commit already on the default branch can land a work item later: one
131−pushed before the work item was filed, or while `auto_close_items` was
132−off.
130+A commit already on the default branch can land a work item after the
131+fact: one pushed before the work item was filed, one whose message never
132+named it, or one pushed while `auto_close_items` was off.
133133
134134 ```sh
135135 g1t work close 12 --commit 3f9a2c1e…
136136 ```
137137
138138 In the repository, give the commit's full hash. Over MCP and the API,
139−`close` takes `repo` and `commit`. The commit's message must name the work
140−item after a closing keyword. Its repository then lands in that commit,
141−the way a push of it would have, by whoever did it, and the work item
142−closes if no target is left. A commit that is not on the default branch,
143−or whose message does not name the work item, is refused, and so is a work
144−item that is already closed.
139+`close` takes `repo` and `commit`. Which commits it accepts:
140+
141+- **With the Write role** on the repository, any commit on its default
142+ branch. Asking for it is what ties the commit to the work item, so its
143+ message need not name it.
144+- **Without it** (the work item's owner, or the Triage role), a commit
145+ whose message names the work item after a closing keyword
146+ (`Fixes #12`), or the commit a pull request merged as, when that pull
147+ request was made for the work item, or its title or description names
148+ it after `Closes` or `Part of`.
149+
150+Its repository then lands in that commit, the way a push of it would have,
151+by whoever did it, and the work item closes if no target is left. The
152+timeline says who tied it and how: **closed this in acme/web@3f9a2c1,
153+which names it**, **…, which merged acme/web#14**, or **…, tied by hand:
154+the commit does not name it**. A commit that is not on the default branch
155+is refused, and so is a work item that is already closed.
156+
157+Closing from pushes and merges is unchanged: a commit or pull request
158+closes a work item by itself only when it names it after a closing
159+keyword.
145160
146161 ## Reopen one
147162
+2−2
6229262292 "Work items"
6229362293 ],
6229462294 "summary": "Close a work item",
62295− "description": "Close a work item without a pull request. A work item closes by itself once what closes it has landed in every target: a pull request whose title or description names it after a closing keyword (Fixes #12) merging into a repository's default branch, or such a commit pushed there, marks that repository landed. When a commit already on a default branch did the work, give it as commit, with repo the repository it is in: it lands there as its push would have, and the work item closes if nothing else is left to land. A commit that is not on the default branch, or whose message does not name the work item after a closing keyword, is refused with `422`; a work item already closed, with `409`. A work item with open sub-items is never closed with them silently: without sub_items it is refused with `409`, naming them; sub_items leave leaves them open, close closes them too (with the same reason, and their own open sub-items), and move makes them sub-items of move_to. A commit closing it leaves them. Closing the last open sub-item leaves its parent open, and says so on the parent. Its author may close their own work item, as may the person g1t filed one for; anyone else needs the Triage role. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace.\n\nWith `commit` and `repo`, a commit already on that repository's default branch whose message names the work item after a closing keyword (`Fixes #12`), it lands there as pushing it would have: a resolution is recorded, the event is `work_item.landed` (and `work_item.closed` with `resolutions` once nothing else is left to land), and the timeline says \"landed this in\" the commit, linked. Give `commit` as its full hash. A commit not on the default branch, or one whose message does not name the work item, is refused (`422`); a work item already closed is `409`, saying what closed it. See [Closing work items](/guides/pull-requests/#closing-work-items).",
62295+ "description": "Close a work item without a pull request. A work item closes by itself once what closes it has landed in every target: a pull request whose title or description names it after a closing keyword (Fixes #12) merging into a repository's default branch, or such a commit pushed there, marks that repository landed. When a commit already on a default branch did the work, give it as commit, with repo the repository it is in: it lands there as its push would have, and the work item closes if nothing else is left to land. With the Write role on repo, any commit on its default branch will do, and the timeline says it was tied by hand; without it, the commit's message must name the work item after a closing keyword, or it must be the commit a pull request merged as whose own work item, closes or parts names it, else it is refused with `403`. A commit that is not on the default branch is refused with `422`; a work item already closed, with `409`. A work item with open sub-items is never closed with them silently: without sub_items it is refused with `409`, naming them; sub_items leave leaves them open, close closes them too (with the same reason, and their own open sub-items), and move makes them sub-items of move_to. A commit closing it leaves them. Closing the last open sub-item leaves its parent open, and says so on the parent. Its author may close their own work item, as may the person g1t filed one for; anyone else needs the Triage role. The workspace is `workspace`, its slug; with only `repo`, the repository's workspace.\n\nWith `commit` and `repo`, a commit already on that repository's default branch (any commit there with the Write role; without it, one whose message names the work item after a closing keyword, `Fixes #12`, or the commit a pull request naming it merged as), it lands there as pushing it would have: a resolution is recorded, the event is `work_item.landed` (and `work_item.closed` with `resolutions` once nothing else is left to land), and the timeline says \"landed this in\" the commit, linked. Give `commit` as its full hash. A commit not on the default branch is refused (`422`), and one nothing ties to the work item, from someone without Write, `403`; a work item already closed is `409`, saying what closed it. See [Closing work items](/guides/pull-requests/#closing-work-items).",
6229662296 "x-operation": "close_work_item",
6229762297 "x-mcp-tool": "work_item",
6229862298 "x-mcp-action": "close",
6250262502 },
6250362503 "commit": {
6250462504 "type": "string",
62505− "description": "The commit that did it: one on repo's default branch whose message names this work item after a closing keyword (Fixes #12). It lands there as pushing it would have."
62505+ "description": "The commit that did it, on repo's default branch: any commit there with the Write role, else one whose message names this work item after a closing keyword (Fixes #12), or a pull request's merge commit that names it. It lands there as pushing it would have."
6250662506 },
6250762507 "sub_items": {
6250862508 "type": "string",
+6−5
405405
406406 /// `close_work_item`: a [`WorkItemActionArgs`], and the commit that did
407407 /// it, if one did. With `commit` (and `repo`, the repository it is in), a
408−/// commit on that repository's default branch whose message names the
409−/// work item after a closing keyword (`Fixes #12`): it lands there as the
410−/// push of that commit would have, and closes when nothing else is left to
411−/// land; a commit not on the default branch, or one that does not name it,
412−/// is refused. Returns `Outcome<WorkItem>`.
408+/// commit on that repository's default branch: any commit there for
409+/// someone with the Write role, else one whose message names the work item
410+/// after a closing keyword (`Fixes #12`) or the commit a pull request
411+/// naming it merged as. It lands there as the push of that commit would
412+/// have, and closes when nothing else is left to land; a commit not on the
413+/// default branch is refused. Returns `Outcome<WorkItem>`.
413414 #[derive(Debug, Serialize, Deserialize)]
414415 pub struct CloseWorkItemArgs {
415416 #[serde(flatten)]
+4−3
15961596 /// Leave a comment as it closes.
15971597 #[arg(short, long)]
15981598 pub comment: Option<String>,
1599− /// The commit on the repository's default branch that did it: its message names the work item
1600− /// after a closing keyword (Fixes #12). Its full hash. It lands there; the work item closes when
1601− /// nothing else is left to land.
1599+ /// The commit on the repository's default branch that did it, by its full hash: any commit
1600+ /// there with the Write role, else one whose message names the work item (Fixes #12) or a pull
1601+ /// request's merge commit that names it. It lands there; the work item closes when nothing else
1602+ /// is left to land.
16021603 #[arg(long)]
16031604 pub commit: Option<String>,
16041605 /// What to do with its sub-items that are still open.
+2−2
172172
173173 #[test]
174174 fn a_landing_that_leaves_a_blocker_open_never_stands_in_for_its_close() {
175− let commit = items::Landing { pull: None, commit: Some("abc".into()), pushed_by: None };
176− let merged = items::Landing { pull: Some(5), commit: None, pushed_by: None };
175+ let commit = items::Landing { pull: None, commit: Some("abc".into()), pushed_by: None, tie: None };
176+ let merged = items::Landing { pull: Some(5), commit: None, pushed_by: None, tie: None };
177177 let landed = resolution_key("wki_2", "wki_1", false, Some(&commit));
178178 let closed = resolution_key("wki_2", "wki_1", true, None);
179179 assert_ne!(landed, closed);
+2−2
359359 return Ok(Outcome::fail(FailureCode::Invalid, "Say which repository the commit is in (repo)."));
360360 };
361361 let found = check!(self.repo(path, &Some(a.actor.clone())).await?);
362− let (commit, named) = check!(self.closing_commit(&found, &held, commit, &a.actor).await?);
362+ let (commit, named, tie) = check!(self.closing_commit(&found, &held, commit, &a.actor).await?);
363363 let session = match named {
364364 Some(session) => Some(session),
365365 None => self.caller_session(&held, &a.actor).await?,
366366 };
367− let landing = Landing { pull: None, commit: Some(commit), pushed_by: Some(a.actor.username.clone()) };
367+ let landing = Landing { pull: None, commit: Some(commit), pushed_by: Some(a.actor.username.clone()), tie: Some(tie) };
368368 self.land(&found, &landing, &a.actor, std::slice::from_ref(&held.item.id), &[], session.as_deref(), false).await?;
369369 return Ok(match self.item_by_id(&held.item.id).await? {
370370 Some(after) if after.item.state == State::Open && !after.item.missing().is_empty() => {
+220−28
77 //! ([`LAND_SQL`]); the work item closes once it has landed in every target
88 //! it has, or at once when it has none. What it is part of is linked
99 //! (`touched`), and closes nothing.
10+//!
11+//! Closing a work item by hand with a commit (`close_work_item` with
12+//! `commit`) lands it too, for a commit on the default branch that is tied
13+//! to it ([`tie`]): one whose message names it after a closing keyword, the
14+//! commit a pull request merged as whose own work item, `closes` or
15+//! `parts` names it, or, for someone with the Write role, any commit, the
16+//! explicit request standing for its message. Automatic closing, from
17+//! pushes and merges, still needs the closing keyword.
1018
1119 use g1t_contracts::events::WorkItemEvent;
1220 use g1t_contracts::repos::{BranchDriftArgs, BranchDrifts, Commit, LogArgs, Repo, RepoPath};
2937 pub pull: Option<u32>,
3038 pub commit: Option<String>,
3139 pub pushed_by: Option<String>,
40+ /// For a commit tied to a work item by hand: how it was.
41+ pub tie: Option<Tie>,
42+}
43+
44+/// How a commit closing a work item by hand (`close_work_item` with
45+/// `commit`) is tied to it.
46+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
47+pub(crate) enum Tie {
48+ /// Its message names the work item after a closing keyword.
49+ Named,
50+ /// A pull request (this number) merged as it, and names the work item:
51+ /// as its own, in its `closes` or `parts`, or in its title or
52+ /// description.
53+ Merged(u32),
54+ /// Nothing names the work item: someone with the Write role tied it.
55+ ByHand,
56+}
57+
58+/// How a commit is tied to a work item for an explicit close: whether its
59+/// message `names` it, the pull request it is the merge of that names it
60+/// (`merged`), and whether the actor `may_push` (has the Write role). None
61+/// when it may not be: nothing names it and the actor lacks Write.
62+pub(crate) fn tie(names: bool, merged: Option<u32>, may_push: bool) -> Option<Tie> {
63+ match (names, merged) {
64+ (true, _) => Some(Tie::Named),
65+ (false, Some(pull)) => Some(Tie::Merged(pull)),
66+ (false, None) => may_push.then_some(Tie::ByHand),
67+ }
68+}
69+
70+/// Where a commit stands against the default branch.
71+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
72+pub(crate) enum OnBranch {
73+ On,
74+ Off,
75+ /// Too far back to tell.
76+ Unknown,
77+}
78+
79+/// Where a commit stands against the default branch: on it when it is its
80+/// head (`at_tip`), else by how far `ahead` of the head it is, when that
81+/// could be told. Ahead of it by any commit, it is not on it.
82+pub(crate) fn on_branch(at_tip: bool, ahead: Option<u32>) -> OnBranch {
83+ match (at_tip, ahead) {
84+ (true, _) => OnBranch::On,
85+ (false, Some(0)) => OnBranch::On,
86+ (false, Some(_)) => OnBranch::Off,
87+ (false, None) => OnBranch::Unknown,
88+ }
89+}
90+
91+/// What a note of a landing by hand adds after `what` (landing_note):
92+/// what tied the commit; the note's author is who did.
93+pub(crate) fn tie_note(path: &str, tie: Option<Tie>) -> String {
94+ match tie {
95+ None => String::new(),
96+ Some(Tie::Named) => ", which names it".to_owned(),
97+ Some(Tie::Merged(pull)) => format!(", which merged {path}#{pull}"),
98+ Some(Tie::ByHand) => ", tied by hand: the commit does not name it".to_owned(),
99+ }
32100 }
33101
102+/// The pull requests of repository `?1` that merged as commit `?2`, and
103+/// whether each names work item `?3` as its own or in what it closed or
104+/// was part of when it merged.
105+const MERGED_AS: &str = "SELECT number, title, body,
106+ (item_id IS ?3 OR EXISTS (SELECT 1 FROM json_each(pulls.closes_items) WHERE value = ?3)
107+ OR EXISTS (SELECT 1 FROM json_each(pulls.parts) WHERE value = ?3)) AS names
108+ FROM pulls WHERE repo_id = ?1 AND status = 'merged' AND merge_commit = ?2
109+ ORDER BY number LIMIT 5";
110+
111+#[derive(Deserialize)]
112+struct MergedAs {
113+ number: u32,
114+ title: String,
115+ body: Option<String>,
116+ names: u8,
117+}
118+
34119 /// Landing a work item (`?1`) in repository `?2` at `?3`, by `?4` (an
35120 /// account id), resolved by `?5` (a stored resolution, JSON), whose pull
36121 /// request and commit are `?6` and `?7`, in one batch: linked (as touched,
139224 commit: landing.commit.clone(),
140225 pushed_by: landing.pushed_by.clone(),
141226 };
142− let what = landing_note(&path, landing.pull, landing.commit.as_deref());
227+ let what = landing_note(&path, landing.pull, landing.commit.as_deref()) + &tie_note(&path, landing.tie);
143228 let mut closed = Vec::new();
144229 for id in closes {
145230 let Some(before) = self.item_by_id(id).await? else { continue };
235320 }
236321
237322 /// `close_work_item` with a commit: the commit, on the default branch
238− /// of `repo`, whose message names work item `held` after a closing
239− /// keyword, with the session it names, if it names one of this
240− /// repository's. Anything else is refused, saying why.
323+ /// of `repo`, tied to work item `held` (see [`tie`]), with the session
324+ /// it names, if it names one of this repository's, and how it is tied.
325+ /// Anything else is refused, saying why.
241326 pub(super) async fn closing_commit(
242327 &self,
243328 repo: &Repo,
244329 held: &Held,
245330 commit: &str,
246331 actor: &User,
247− ) -> Result<Outcome<(String, Option<String>)>> {
332+ ) -> Result<Outcome<(String, Option<String>, Tie)>> {
248333 let number = held.item.number;
249334 let commit = commit.trim().to_lowercase();
250335 if !(4..=64).contains(&commit.len()) || !commit.bytes().all(|b| b.is_ascii_hexdigit()) {
276361 break;
277362 }
278363 }
279− if !names {
364+ let merged = if names { None } else { self.merged_naming(repo, held, &found.hash).await? };
365+ let may_push = matches!(crate::allowed(Some(actor), repo, g1t_contracts::access::Capability::Push), Outcome::Ok(()));
366+ let Some(tie) = tie(names, merged, may_push) else {
280367 return Ok(Outcome::fail(
281− FailureCode::Invalid,
282− format!("Commit {short} does not close #{number}: its message names it after no closing keyword (Fixes #{number})."),
368+ FailureCode::Forbidden,
369+ format!(
370+ "Commit {short} does not name #{number} (Fixes #{number}), nor is it the merge of a pull request that does: \
371+ tying it to #{number} by hand takes the Write role on {}/{}.",
372+ repo.namespace, repo.name
373+ ),
283374 ));
284− }
375+ };
285376 let head = match head? {
286377 Outcome::Ok(commits) => commits.into_iter().next(),
287378 Outcome::Fail(_) => None,
289380 let Some(head) = head else {
290381 return Ok(Outcome::fail(FailureCode::Invalid, format!("Commit {short} is not on {}.", repo.default_branch)));
291382 };
292− if head.hash != found.hash {
383+ let at_tip = head.hash == found.hash;
384+ let ahead = if at_tip {
385+ None
386+ } else {
293387 let drifts: Outcome<BranchDrifts> = g1t_kit::call(
294388 &self.repos,
295389 "branch_drift",
296390 &BranchDriftArgs { path: path.clone(), viewer: Some(actor.clone()), base: head.hash, heads: vec![found.hash.clone()] },
297391 )
298392 .await?;
299− let drift = match drifts {
300− Outcome::Ok(drifts) => drifts.branches.into_iter().next().and_then(|branch| branch.drift),
393+ match drifts {
394+ Outcome::Ok(drifts) => drifts.branches.into_iter().next().and_then(|branch| branch.drift).map(|drift| drift.ahead),
301395 Outcome::Fail(_) => None,
302− };
303− match drift {
304− Some(drift) if drift.ahead == 0 => {}
305− Some(_) => {
306− return Ok(Outcome::fail(
307− FailureCode::Invalid,
308− format!("Commit {short} is not on {}, the default branch: only a commit there closes a work item.", repo.default_branch),
309− ));
310− }
311− None => {
312− return Ok(Outcome::fail(
313− FailureCode::Invalid,
314− format!("Could not tell whether commit {short} is on {}: it is too far back.", repo.default_branch),
315− ));
396+ }
397+ };
398+ match on_branch(at_tip, ahead) {
399+ OnBranch::On => {}
400+ OnBranch::Off => {
401+ return Ok(Outcome::fail(
402+ FailureCode::Invalid,
403+ format!("Commit {short} is not on {}, the default branch: only a commit there closes a work item.", repo.default_branch),
404+ ));
405+ }
406+ OnBranch::Unknown => {
407+ return Ok(Outcome::fail(
408+ FailureCode::Invalid,
409+ format!("Could not tell whether commit {short} is on {}: it is too far back.", repo.default_branch),
410+ ));
411+ }
412+ }
413+ let session = self.trailer_session(&repo.id, &found.message).await?;
414+ Ok(Outcome::Ok((found.hash, session, tie)))
415+ }
416+
417+ /// The pull request of `repo` that merged as commit `hash` and names
418+ /// work item `held`: as its own, in what it closed or was part of when
419+ /// it merged, or after `Closes` or `Part of` in its title or
420+ /// description as they read now.
421+ async fn merged_naming(&self, repo: &Repo, held: &Held, hash: &str) -> Result<Option<u32>> {
422+ let pulls = self
423+ .db
424+ .prepare(MERGED_AS)
425+ .bind(&[repo.id.as_str().into(), hash.into(), held.item.id.as_str().into()])?
426+ .all()
427+ .await?
428+ .results::<MergedAs>()?;
429+ for pull in pulls {
430+ if pull.names != 0 {
431+ return Ok(Some(pull.number));
432+ }
433+ let named = refs::named(&format!("{}\n{}", pull.title, pull.body.as_deref().unwrap_or_default()));
434+ for reference in named.closes.iter().chain(&named.parts) {
435+ if self.ref_item(repo, reference).await?.as_deref() == Some(held.item.id.as_str()) {
436+ return Ok(Some(pull.number));
316437 }
317438 }
318439 }
319− let session = self.trailer_session(&repo.id, &found.message).await?;
320− Ok(Outcome::Ok((found.hash, session)))
440+ Ok(None)
321441 }
322442
323443 /// `closed_by_commit`: the work items a commit pushed to a repository
378498 }
379499
380500 #[test]
501+ fn an_explicit_close_ties_any_commit_for_someone_with_write() {
502+ // A commit that names the work item, or the merge of a pull request
503+ // that does, needs nothing more.
504+ assert_eq!(tie(true, None, false), Some(Tie::Named));
505+ assert_eq!(tie(true, Some(14), true), Some(Tie::Named));
506+ assert_eq!(tie(false, Some(14), false), Some(Tie::Merged(14)));
507+ // One that does not is tied by hand, only with the Write role.
508+ assert_eq!(tie(false, None, true), Some(Tie::ByHand));
509+ assert_eq!(tie(false, None, false), None);
510+ }
511+
512+ #[test]
513+ fn only_a_commit_on_the_default_branch_closes_a_work_item() {
514+ assert_eq!(on_branch(true, None), OnBranch::On);
515+ assert_eq!(on_branch(false, Some(0)), OnBranch::On);
516+ assert_eq!(on_branch(false, Some(2)), OnBranch::Off);
517+ assert_eq!(on_branch(false, None), OnBranch::Unknown);
518+ }
519+
520+ #[test]
521+ fn the_note_says_what_tied_the_commit() {
522+ let what = |tie| landing_note("acme/web", None, Some("abcdef1234")) + &tie_note("acme/web", tie);
523+ assert_eq!(what(None), "acme/web@abcdef1");
524+ assert_eq!(what(Some(Tie::Named)), "acme/web@abcdef1, which names it");
525+ assert_eq!(what(Some(Tie::Merged(14))), "acme/web@abcdef1, which merged acme/web#14");
526+ assert_eq!(what(Some(Tie::ByHand)), "acme/web@abcdef1, tied by hand: the commit does not name it");
527+ }
528+
529+ #[test]
530+ fn a_merge_commit_names_what_its_pull_request_landed() {
531+ let db = crate::migrated::database();
532+ db.execute_batch(
533+ "INSERT INTO work_items (id, workspace, number, title, body, state, resolutions, author_id, author_name, created_at, updated_at)
534+ VALUES ('wki_1', 'acme', 1, 'One', '', 'open', '[]', 'user', 'user', 'now', 'now'),
535+ ('wki_2', 'acme', 2, 'Two', '', 'open', '[]', 'user', 'user', 'now', 'now');",
536+ )
537+ .unwrap();
538+ let pull = |id: &str, repo: &str, number: u32, status: &str, merge: &str, item: Option<&str>, closes: &str, parts: &str| {
539+ db.execute(
540+ "INSERT INTO pulls (id, repo_id, number, title, body, agent, runtime, status, merge_commit, item_id, closes_items, parts,
541+ author_id, author_name, created_at, updated_at)
542+ VALUES (?1, ?2, ?3, 'Fix', NULL, 'a', 'r', ?4, ?5, ?6, ?7, ?8, 'user', 'user', 'now', 'now')",
543+ params![id, repo, number, status, merge, item, closes, parts],
544+ )
545+ .unwrap();
546+ };
547+ pull("pr_own", "repo", 14, "merged", "m14", Some("wki_1"), "[]", "[]");
548+ pull("pr_closes", "repo", 15, "merged", "m15", None, r#"["wki_1"]"#, "[]");
549+ pull("pr_part", "repo", 16, "merged", "m16", None, "[]", r#"["wki_2","wki_1"]"#);
550+ pull("pr_other", "repo", 17, "merged", "m17", Some("wki_2"), r#"["wki_2"]"#, "[]");
551+ pull("pr_open", "repo", 18, "open", "m18", Some("wki_1"), "[]", "[]");
552+ pull("pr_away", "away", 14, "merged", "m14b", Some("wki_1"), "[]", "[]");
553+ let merged = |repo: &str, commit: &str| -> Vec<(u32, u8)> {
554+ db.prepare(MERGED_AS)
555+ .unwrap()
556+ .query_map(params![repo, commit, "wki_1"], |row| Ok((row.get(0)?, row.get(3)?)))
557+ .unwrap()
558+ .map(Result::unwrap)
559+ .collect()
560+ };
561+ assert_eq!(merged("repo", "m14"), vec![(14, 1)]);
562+ assert_eq!(merged("repo", "m15"), vec![(15, 1)]);
563+ assert_eq!(merged("repo", "m16"), vec![(16, 1)]);
564+ // Another work item's merge is found, naming nothing of #1's.
565+ assert_eq!(merged("repo", "m17"), vec![(17, 0)]);
566+ // Not merged, or merged elsewhere: nothing.
567+ assert_eq!(merged("repo", "m18"), vec![]);
568+ assert_eq!(merged("repo", "m14b"), vec![]);
569+ assert_eq!(g1t_kit::rename::parameters(MERGED_AS), 3);
570+ }
571+
572+ #[test]
381573 fn landing_statements_take_the_item_repository_and_what_landed() {
382574 let counts: Vec<usize> = LAND_SQL.iter().map(|sql| g1t_kit::rename::parameters(sql)).collect();
383575 assert_eq!(counts, vec![4, 3, 7, 3]);
+1−1
22312231 None => merging::merged_note(landed.method).to_owned(),
22322232 };
22332233 self.note(&repo.id, pull.number, who, &note).await?;
2234− let landing = items::Landing { pull: Some(pull.number), commit: None, pushed_by: None };
2234+ let landing = items::Landing { pull: Some(pull.number), commit: None, pushed_by: None, tie: None };
22352235 items::land(self, repo, &landing, actor, &named.close_ids(), &named.part_ids(), None, false).await?;
22362236 let through_queue = self.landed_through_queue(&pull.id).await?;
22372237 self.keep_decision_quietly(
+1−1
191191 let message = pushed.commits.iter().find(|pushed| pushed.hash == *commit).map(|pushed| pushed.message.as_str());
192192 let session = self.trailer_session(&repo.id, message.unwrap_or_default()).await?;
193193 let landing =
194− items::Landing { pull: None, commit: Some(commit.clone()), pushed_by: Some(actor.username.clone()) };
194+ items::Landing { pull: None, commit: Some(commit.clone()), pushed_by: Some(actor.username.clone()), tie: None };
195195 let (closes, parts) = if closing { (vec![id], Vec::new()) } else { (Vec::new(), vec![id]) };
196196 items::land(self, &repo, &landing, &actor, &closes, &parts, session.as_deref(), agent).await?;
197197 }

Comments

Sign in to comment.