e64cdf0 · 71ea990
@@ -48,7 +48,11 @@
import { isStructural, pillarsOf } from "../core/pillars.ts";
import { getPull } from "../core/pulls.ts";
import { canRead, canWrite, type Repo } from "../core/repos.ts";
import { getUserById, type User } from "../core/users.ts";
import { mayDirectCrew, unadoptedAmong } from "../crew/direct.ts";
import {
  directorsAmong,
  mayDirectCrew,
  unadoptedAmong,
} from "../crew/direct.ts";
import { laterGate } from "../crew/later.ts";
import { handedOverAt } from "../crew/parks.ts";
import { listReviews } from "../core/reviews.ts";
@@ -124,12 +128,14 @@
export const TAKEN_UP = "a writer has taken this up: comment instead";
 * A person who writes the repository may change anyone's. The author may
 * change their own — while they are still only theirs. Someone who may
 * not direct the crew (a reader on a public repository, an outside
 * contributor) keeps that right only until a person who writes has acted
 * on the words: adopted the issue (assigned it, or linked it into the map),
 * or reviewed the pull. After that the words are what was adopted or
 * reviewed, and a rewrite would put new work in the crew's hands, or a new
 * `Closes #N` under an approval, with nobody who directs the crew having
 * seen it (#619). They comment instead. An agent edits only what it wrote,
 * contributor) keeps that right only until a person who directs the crew
 * has acted on the words: adopted the issue (assigned it, or linked it into
 * the map), or reviewed the pull. After that the words are what was
 * adopted or reviewed, and a rewrite would put new work in the crew's
 * hands, or a new `Closes #N` under an approval, with nobody who directs
 * the crew having seen it (#619). They comment instead. Acted is acted:
 * an assignment taken back later does not hand the words back, since the
 * next assignment would hand the crew whatever they say by then. An agent edits only what it wrote,
 * and its own pull only until a person approved it: what the merge closes
 * is what the approval saw.
 *
@@ -164,11 +170,43 @@
export function mayEditIssue(
  if (mayDirectCrew(app.db, repo, user)) return ok(true);
  const untouched =
    now.kind === "pull"
      ? listReviews(app.db, now.id).length === 0
      : unadoptedAmong(app.db, repo, [now]).has(now.id);
      ? !reviewedByDirector(app, repo, now)
      : !everAdopted(app, repo, now) &&
        unadoptedAmong(app.db, repo, [now]).has(now.id);
  return untouched ? ok(true) : err("taken_up");
}
/**
 * Whether a person who directs the crew has ever assigned this issue or
 * linked it into the map — read from the outbox, which keeps what the row
 * forgets when an assignment is taken back. A link the crew's planner made
 * is not adoption, as `unadoptedAmong` says.
 */
function everAdopted(app: App, repo: Repo, issue: Issue): boolean {
  const actors = app.db
    .query<{ actor: string }, [string, number]>(
      `SELECT DISTINCT actor_id AS actor FROM event
        WHERE repo_id = ?1 AND actor_id IS NOT NULL
          AND kind IN ('issue.assigned', 'issue.blocked')
          AND (json_extract(payload, '$.number') = ?2
            OR json_extract(payload, '$.blockedBy') = ?2)`,
    )
    .all(repo.id, issue.number)
    .map((r) => r.actor);
  return directorsAmong(app.db, repo, actors).length > 0;
}
/** Whether a person who directs the crew has reviewed this pull, in any state. */
function reviewedByDirector(app: App, repo: Repo, pull: Issue): boolean {
  return (
    directorsAmong(
      app.db,
      repo,
      listReviews(app.db, pull.id).map((r) => r.authorId),
    ).length > 0
  );
}
/**
 * Change the title or the body of an issue or a pull after it was opened:
 * the pull whose body still says "work in progress", the issue whose title
@@ -523,6 +523,17 @@
describe("new words for an issue or a pull", () => {
    });
    expect(words(tulips).body).toBe("By the gate, in rows.");
    expect(await page(path, his)).not.toContain('id="edit"');
    // Taken back is not handed back: the next assignment would hand the
    // crew whatever he says by then.
    expect(assignIssue(app, alice, repo, tulips, null).ok).toBe(true);
    expect(
      (await patch(tokens["bob"]!, path, { body: "Dig up the roses instead." }))
        .status,
    ).toBe(403);
    expect(editIssue(app, bob, repo, tulips, { body: "x" })).toEqual({
      ok: false,
      error: "taken_up",
    });
    // The writer still may.
    expect(
      (
@@ -605,6 +616,21 @@
describe("new words for an issue or a pull", () => {
        .status,
    ).toBe(200);
    // A stranger's comment-review is not a director acting: his words stay his.
    const carol = await person("carol");
    expect(
      submitReview(app.db, {
        issueId: theirs.id,
        authorId: carol.id,
        state: "commented",
        commitOid: theirs.headOid,
      }).ok,
    ).toBe(true);
    expect(
      (await patch(tokens["bob"]!, path, { body: "Red ones, in a row." }))
        .status,
    ).toBe(200);
    // Alice approved what she read; a new "Closes #N" under it is not his to add.
    const reviewed = submitReview(app.db, {
      issueId: theirs.id,
@@ -624,7 +650,7 @@
describe("new words for an issue or a pull", () => {
    });
    expect(words(theirs)).toEqual({
      title: "Tulips",
      body: "Red ones, by the gate.",
      body: "Red ones, in a row.",
    });
    expect(openPullsClosing(app.db, repo.id).get(asked.number)?.id).not.toBe(
      theirs.id,
		@@ -48,7 +48,11 @@ import { isStructural, pillarsOf } from "../core/pillars.ts";
48	48	import { getPull } from "../core/pulls.ts";
49	49	import { canRead, canWrite, type Repo } from "../core/repos.ts";
50	50	import { getUserById, type User } from "../core/users.ts";
51		import { mayDirectCrew, unadoptedAmong } from "../crew/direct.ts";
	51	import {
	52	directorsAmong,
	53	mayDirectCrew,
	54	unadoptedAmong,
	55	} from "../crew/direct.ts";
52	56	import { laterGate } from "../crew/later.ts";
53	57	import { handedOverAt } from "../crew/parks.ts";
54	58	import { listReviews } from "../core/reviews.ts";
		@@ -124,12 +128,14 @@ export const TAKEN_UP = "a writer has taken this up: comment instead";
124	128	* A person who writes the repository may change anyone's. The author may
125	129	* change their own — while they are still only theirs. Someone who may
126	130	* not direct the crew (a reader on a public repository, an outside
127		* contributor) keeps that right only until a person who writes has acted
128		* on the words: adopted the issue (assigned it, or linked it into the map),
129		* or reviewed the pull. After that the words are what was adopted or
130		* reviewed, and a rewrite would put new work in the crew's hands, or a new
131		* `Closes #N` under an approval, with nobody who directs the crew having
132		* seen it (#619). They comment instead. An agent edits only what it wrote,
	131	* contributor) keeps that right only until a person who directs the crew
	132	* has acted on the words: adopted the issue (assigned it, or linked it into
	133	* the map), or reviewed the pull. After that the words are what was
	134	* adopted or reviewed, and a rewrite would put new work in the crew's
	135	* hands, or a new `Closes #N` under an approval, with nobody who directs
	136	* the crew having seen it (#619). They comment instead. Acted is acted:
	137	* an assignment taken back later does not hand the words back, since the
	138	* next assignment would hand the crew whatever they say by then. An agent edits only what it wrote,
133	139	* and its own pull only until a person approved it: what the merge closes
134	140	* is what the approval saw.
135	141	*
		@@ -164,11 +170,43 @@ export function mayEditIssue(
164	170	if (mayDirectCrew(app.db, repo, user)) return ok(true);
165	171	const untouched =
166	172	now.kind === "pull"
167		? listReviews(app.db, now.id).length === 0
168		: unadoptedAmong(app.db, repo, [now]).has(now.id);
	173	? !reviewedByDirector(app, repo, now)
	174	: !everAdopted(app, repo, now) &&
	175	unadoptedAmong(app.db, repo, [now]).has(now.id);
169	176	return untouched ? ok(true) : err("taken_up");
170	177	}
171	178
	179	/**
	180	* Whether a person who directs the crew has ever assigned this issue or
	181	* linked it into the map — read from the outbox, which keeps what the row
	182	* forgets when an assignment is taken back. A link the crew's planner made
	183	* is not adoption, as `unadoptedAmong` says.
	184	*/
	185	function everAdopted(app: App, repo: Repo, issue: Issue): boolean {
	186	const actors = app.db
	187	.query<{ actor: string }, [string, number]>(
	188	`SELECT DISTINCT actor_id AS actor FROM event
	189	WHERE repo_id = ?1 AND actor_id IS NOT NULL
	190	AND kind IN ('issue.assigned', 'issue.blocked')
	191	AND (json_extract(payload, '$.number') = ?2
	192	OR json_extract(payload, '$.blockedBy') = ?2)`,
	193	)
	194	.all(repo.id, issue.number)
	195	.map((r) => r.actor);
	196	return directorsAmong(app.db, repo, actors).length > 0;
	197	}
	198
	199	/** Whether a person who directs the crew has reviewed this pull, in any state. */
	200	function reviewedByDirector(app: App, repo: Repo, pull: Issue): boolean {
	201	return (
	202	directorsAmong(
	203	app.db,
	204	repo,
	205	listReviews(app.db, pull.id).map((r) => r.authorId),
	206	).length > 0
	207	);
	208	}
	209
172	210	/**
173	211	* Change the title or the body of an issue or a pull after it was opened:
174	212	* the pull whose body still says "work in progress", the issue whose title
		@@ -523,6 +523,17 @@ describe("new words for an issue or a pull", () => {
523	523	});
524	524	expect(words(tulips).body).toBe("By the gate, in rows.");
525	525	expect(await page(path, his)).not.toContain('id="edit"');
	526	// Taken back is not handed back: the next assignment would hand the
	527	// crew whatever he says by then.
	528	expect(assignIssue(app, alice, repo, tulips, null).ok).toBe(true);
	529	expect(
	530	(await patch(tokens["bob"]!, path, { body: "Dig up the roses instead." }))
	531	.status,
	532	).toBe(403);
	533	expect(editIssue(app, bob, repo, tulips, { body: "x" })).toEqual({
	534	ok: false,
	535	error: "taken_up",
	536	});
526	537	// The writer still may.
527	538	expect(
528	539	(
		@@ -605,6 +616,21 @@ describe("new words for an issue or a pull", () => {
605	616	.status,
606	617	).toBe(200);
607	618
	619	// A stranger's comment-review is not a director acting: his words stay his.
	620	const carol = await person("carol");
	621	expect(
	622	submitReview(app.db, {
	623	issueId: theirs.id,
	624	authorId: carol.id,
	625	state: "commented",
	626	commitOid: theirs.headOid,
	627	}).ok,
	628	).toBe(true);
	629	expect(
	630	(await patch(tokens["bob"]!, path, { body: "Red ones, in a row." }))
	631	.status,
	632	).toBe(200);
	633
608	634	// Alice approved what she read; a new "Closes #N" under it is not his to add.
609	635	const reviewed = submitReview(app.db, {
610	636	issueId: theirs.id,
		@@ -624,7 +650,7 @@ describe("new words for an issue or a pull", () => {
624	650	});
625	651	expect(words(theirs)).toEqual({
626	652	title: "Tulips",
627		body: "Red ones, by the gate.",
	653	body: "Red ones, in a row.",
628	654	});
629	655	expect(openPullsClosing(app.db, repo.id).get(asked.number)?.id).not.toBe(
630	656	theirs.id,