Trigger the agent when a checkbox is toggled in a Forgejo comment or description #229

Closed
opened 2026-11-06 21:27:37 +01:00 by omp-deck · 0 comments
Collaborator

Problem

When the agent posts a markdown checklist on a Forgejo issue or pull request, a user with
write access ticking one of those boxes must wake the agent. The same must work when the
edited checklist sits in someone else's comment, in an inline review comment, or in the
issue/PR description.

Today this does not work, blocked at three independent layers:

  1. Receiver (apps/server/src/forgejo/webhook.ts:160-168): issue_comment /
    pull_request_comment with action: "edited" is classified, but the author is read from
    comment.user.login and anything authored by the agent is ignored as "self-authored".
    An edit to the agent's own comment therefore never becomes a trigger row.
  2. Dispatcher (apps/server/src/forgejo/dispatcher.ts:613-622): Forgejo puts the editor
    in sender, and a row whose sender differs from row.commentAuthor is dropped with
    "edited by someone other than its author". That is exactly "user ticks a box in the
    agent's (or a third party's) comment".
  3. Detection (apps/server/src/forgejo/conversation-detect.ts:175-182): for
    action: "edited", detectAddressed returns only edited-mention — an edit that adds an
    @mention which was absent before. A checkbox toggle adds no mention, so the row is
    dropped as "not addressed to the agent".

Issue/PR description edits are rejected outright: webhook.ts:137-146 has no edited
branch and webhook.test.ts:137 pins "unsupported action edited".

Forgejo-side facts, grounded in services/webhook/notifier.go and
web_src/js/markup/tasklist.js on the forgejo branch:

  • UpdateComment has no comment-type guard, so editing an inline review comment fires
    the same issue_comment / pull_request_comment event with action: "edited",
    changes.body.from, and issue = the PR. Inline comments need no new event handling.
  • IssueChangeContent fires issues (non-PR) or pull_request (PR) with action: "edited"
    and changes.body.from.
  • IssueChangeTitle also fires action: "edited", with changes.title and no
    changes.body — so a missing changes.body.from must be rejected.
  • The rendered checkbox widget POSTs the whole rewritten body to the normal content-update
    URL, one request per click, flipping exactly one character. Clean positional pairing holds
    in practice; messy diffs only arise from hand-edited markdown.
  • The widget is interactive only where data-can-edit=true (poster or write access), so
    Forgejo already enforces most of the permission gate; the deck's check is defence in depth.

End state: a checkbox state change in a comment, inline review comment, or description, made
by the repo owner or a user with write/admin access, on an artifact the agent has taken part
in, wakes the agent with a neutral seed naming which boxes flipped. Bursts of toggles
collapse into one turn.

Approach

1. Export the prose-line splitter

In apps/server/src/forgejo/conversation-detect.ts, change forEachProseLine (line 80) from
module-private to export. Signature is unchanged:

export function forEachProseLine(markdown: string, onLine: (line: string) => void): void

It emits every line outside fenced code blocks, blank lines and > quote lines included,
which is what block extraction below needs. No other change to that function.

2. New pure module: checkbox parsing and diffing

Create apps/server/src/forgejo/checkbox-diff.ts. Nothing in the repo parses markdown task
lists (searched for \[[ xX]\], checkbox, task.?list across apps/server/src and
packages), so this is new code. Pure: no DB, no network, mirroring conversation-detect.ts.

export interface TaskItem {
	/** Item text after the checkbox marker, trimmed. */
	label: string;
	checked: boolean;
}

export interface CheckboxFlip {
	label: string;
	/** The item's state *after* the edit. */
	checked: boolean;
}

export interface CheckboxChange {
	/** The flipped items, or null when the two lists could not be paired reliably. */
	flips: CheckboxFlip[] | null;
	/** Checklist blocks of the body before the edit; "" when there were none. */
	beforeBlocks: string;
	/** Checklist blocks of the body after the edit. */
	afterBlocks: string;
}

parseTaskItems(markdown: string): TaskItem[] — run forEachProseLine; skip lines
matching /^\s*>/ (a quoted checklist is not the author's); for each remaining line match
/^\s*[-*+] \[([ xX])\][ \t]?(.*)$/, pushing { label: m[2].trim(), checked: m[1] !== " " }.
Order is document order. Indentation is ignored, so nested sub-items are ordinary items.

checklistBlocks(markdown: string): string — run forEachProseLine collecting lines into
an array. Walk it: a block starts at the first line matching the task-item regex and runs up
to (excluding) the first line matching /^\s*$/ or the end of the array; scanning then
resumes after that blank line. Join the blocks' lines with \n, and separate blocks from one
another with a single blank line. Returns "" when there is no task line. This keeps a long
comment from bloating the turn: only the checklist travels, not the whole body.

firstWord(label: string): string (module-private) — lowercase, take the first
whitespace-delimited token, strip characters not matching /[\p{L}\p{N}]/u from both ends.
"" for an empty label.

diffCheckboxes(before: string, after: string): CheckboxChange | null —

  1. a = parseTaskItems(before), b = parseTaskItems(after).
  2. Change predicate: build for each side the sorted array of
    `${firstWord(item.label)}\u0000${item.checked ? "1" : "0"}` and compare the two with
    JSON.stringify. Equal → return null (no checkbox state change). This makes a pure
    reorder, and any prose edit that leaves every item's first word and state alone, a non-event,
    while a tick combined with an unrelated reorder, and a simultaneous tick + untick, both
    still register.
  3. Pairing: clean when a.length === b.length and firstWord(a[i].label) === firstWord(b[i].label)
    for every i. Then flips = b[i] (as { label, checked }) for every i where
    a[i].checked !== b[i].checked. Otherwise flips = null — the dispatcher does not try
    harder, it hands the agent both checklists and lets it work out the difference.
  4. Return { flips, beforeBlocks: checklistBlocks(before), afterBlocks: checklistBlocks(after) }.

3. Accept edits that the receiver currently discards

All in apps/server/src/forgejo/webhook.ts.

Comment edits — in the issue_comment / pull_request_comment branch
(classifyForgejoDelivery, lines 150-187), the author/self check becomes action-dependent:

  • action === "created": unchanged. commentAuthor is comment.user.login; a comment
    authored by the agent is ignored as "self-authored".
  • action === "edited": the acting principal is the editor. Read
    sender.login; malformed "missing sender.login" when absent or not a string. Ignore with
    reason "self-edited" when it equals agentUsername case-insensitively — this is the loop
    guard, since the agent rewrites its own checklists. comment.user.login is still required
    (malformed otherwise) but no longer gates acceptance. The accepted trigger carries
    commentAuthor: sender.login.

Description edits — add, before the issues branch's final ignore, and in the
pull_request family's action switch, an action === "edited" case calling a new
classifyDescriptionEdit(eventType, payload, agentUsername, isPull):

  • Reject with { kind: "ignore", reason: "no body change" } unless
    payload.changes.body.from is a string. This is what separates a content edit from a
    title-only edit, which Forgejo also sends as edited.
  • sender.login required (malformed "missing sender.login"); ignored as "self-edited"
    when it is the agent.
  • repository.full_name and the number validated exactly as the comment branch does
    (number from issue.number for issues, from pull_request.number or number for
    pull_request).
  • Trigger: { event: eventType, action: "edited", repo, number, isPull, commentAuthor: sender.login }.

Dedupe — generalise dedupeKey (line 478) to take the edited subject instead of only a
comment:

function dedupeKey(repo: string, commentId: number, action: string, subject: unknown): string

Body unchanged (it already reads updated_at and body off the value). Comment branches pass
payload.comment with the real comment id; description edits pass payload.issue (or
payload.pull_request) with commentId 0. Successive toggles differ in body, so each
toggle gets its own key even inside one second.

Protocol: no change. packages/protocol/src/index.ts:2168-2210 already has issues,
pull_request, issue_comment, pull_request_comment in the event union and edited in
the action union. Extend the commentAuthor doc comment (line 2211-2217) to say that for
edited it is the editor, not the author.

Existing test to change: apps/server/src/forgejo/webhook.test.ts:137 asserts
issues + edited yields { kind: "ignore", reason: "unsupported action edited" }. That
behaviour is being replaced; rewrite the case to assert the new verdicts (accepted with
changes.body.from present; "no body change" without it).

4. New trigger reason and detection rule

In apps/server/src/forgejo/conversation-detect.ts:

  • Add "checkbox-change" to the TriggerReason union (line 53) and to the
    CONVERSATION_REASONS set (line 62) — membership there is what lets the turn run in a
    workspace-less session (dispatcher.ts:667).
  • Add to REASON_TEXT (line 258): "checkbox-change": "a checkbox was ticked or unticked".
  • Extend the action === "edited" branch of detectAddressed (lines 175-182). Two new
    optional input fields:
	/** True when `diffCheckboxes` found a checkbox state change in this edit. */
	checkboxChanged?: boolean;
	/** Login of the author of the edited comment or description; null when unknown. */
	editedAuthor?: string | null;

New branch body, replacing the current one:

	if (action === "edited") {
		if (input.checkboxChanged === true && conversation !== null) {
			const agent = agentUsername.toLowerCase();
			const participated =
				(input.editedAuthor ?? "").toLowerCase() === agent ||
				conversation.some((e) => e.author.toLowerCase() === agent);
			if (participated) return "checkbox-change";
		}
		const body = bodies[0] ?? "";
		return previousBody !== null && mentions(body, agentUsername) && !mentions(previousBody, agentUsername)
			? "edited-mention"
			: null;
	}

The participation gate: the agent wrote the edited text, or it appears anywhere in the
conversation. Because the dispatcher prepends openingEntry (dispatcher.ts:1953), an
artifact the agent opened satisfies it too. Checkbox detection outranks edited-mention
when an edit does both; a failed participation check still falls through to the mention rule,
so no existing behaviour is lost.

5. Seed builder for a checkbox turn

New export in conversation-detect.ts, next to buildConversationSeed. A dedicated builder
rather than overloading buildConversationSeed, because the turn has a different shape
(editor, flips, two checklists) and may target a description, which has no payload.comment.

export function buildCheckboxEditSeed(t: {
	repo: string;
	number: number;
	isPull: boolean;
	/** "comment" for an issue/inline comment, "description" for the issue/PR body. */
	target: "comment" | "description";
	/** Login of the user who made the edit. */
	editor: string;
	/** Login of the author of the edited text; null when unknown. */
	author: string | null;
	change: CheckboxChange;
	payload: IssueCommentPayloadLike;
	orientation: TurnOrientation;
}): string

Lines, in order:

  1. Forgejo checkbox change on <repo> <PR|issue> #<number>: "<issue.title ?? "?">" — and for
    target: "description", … in the description.
  2. Labels: … · Comments: …, copied verbatim from buildConversationSeed (lines 331-332).
  3. Trigger: a checkbox was ticked or unticked (from REASON_TEXT).
  4. Edited by @<editor> — plus in a comment by @<author> when author is non-null and
    differs from editor.
  5. ...orientationLines(t.orientation, { isPull: t.isPull, number: t.number }).
  6. When change.flips !== null and non-empty: Changed items: followed by one
    - [x] <label> / - [ ] <label> line per flip, reflecting the state after the edit.
    When change.flips === null (or empty): The checklist could not be matched up reliably. Before and after follow. then a fenced change.beforeBlocks labelled Before: and a
    fenced change.afterBlocks labelled After:, using fenceFor (line 416) on each so the
    content cannot close its own fence.
  7. Always, after the flips list, a fenced change.afterBlocks labelled Checklist now: (the
    flips !== null path only; the null path already showed it).
  8. Closing paragraph, neutral by design — the agent decides what a tick means:
    A checkbox changed; nothing else is implied. Read the conversation and decide from its content what this asks of you — it may mean the item was selected for you to act on, or that someone reports it done, or neither. If it is not asking anything of you, end the turn without posting.

6. Dispatcher: detection wiring

In apps/server/src/forgejo/dispatcher.ts.

Add to Job (line 341):

	/** The checkbox change of an `edited` trigger, once computed; null when there is none. */
	checkbox: CheckboxChange | null;
	/** Body before the edit, coalesced across a settled burst. */
	checkboxBefore: string | null;
	/** Body after the edit, coalesced across a settled burst. */
	checkboxAfter: string | null;

Initialise all three to null wherever a Job is constructed.

Rewrite detect (lines 1928-1955). The body of an edit is no longer only
payload.comment?.body, because a description edit has none:

		const editedBody = payload.comment?.body ?? payload.issue?.body ?? payload.pull_request?.body ?? "";
		const editedAuthor = payload.comment?.user?.login ?? payload.issue?.user?.login ?? payload.pull_request?.user?.login ?? null;
  • bodies becomes [editedBody]; previousBody stays payload.changes?.body?.from ?? null,
    but for a settled burst use job.checkboxBefore ?? from and job.checkboxAfter ?? editedBody
    (set in step 7).
  • For action === "edited": compute job.checkbox = diffCheckboxes(before, after) when
    before !== null. If job.checkbox !== null, fetch the conversation
    (this.fetchConversation(job), propagating its "failed" / "dropped" results) and call
    detectAddressed with conversation: [...openingEntry(...), ...conversation],
    checkboxChanged: true, editedAuthor. If job.checkbox === null, keep today's fast path:
    detectAddressed({ ...input, conversation: null }) and return its result without fetching.
  • created behaviour is untouched.

7. Dispatcher: 15-second settle window

Bursts: each click is a separate HTTP request and a separate trigger row, so a user ticking
several boxes would otherwise get one agent turn per tick. The dispatcher already has the
mechanism — deferred (line 388) keeps a row pending and blocks its artifact in pump
(line 452), and drain returns on "deferred" (lines 474-478) so later rows stay behind it.
Only a timed release is missing; releaseIntakeSlot (line 2690) is the model for the release.

Add near the other constants (around line 188):

/** How long a checkbox edit waits for further toggles of the same text before running. */
const CHECKBOX_SETTLE_MS = 15_000;

Add a field alongside deferred:

	/** Pending settle-window releases, by trigger row id. */
	private readonly settleTimers = new Map<string, ReturnType<typeof setTimeout>>();

Clear every timer in the dispatcher's dispose/teardown path (where disposed is set), so a
shutdown leaves no pending handle.

In gate(row) (line 501), before any API work, when row.action === "edited" and the
payload's changes.body.from is a string:

  1. Compute the row's own diff with diffCheckboxes(from, body). Null → skip the settle
    window entirely
    and continue gating as a normal (possibly edited-mention) edit, so a
    plain text edit is never delayed.
  2. Otherwise build the sibling set from listPendingForgejoTriggers(): rows with the same
    repo (case-insensitive) and number, action === "edited", the same comment key — the
    payload's comment.id, or 0 for a description edit — and a non-null diffCheckboxes of
    their own. Keep queue order (the query already returns oldest first) and include row.
  3. Let newest be the last sibling. If Date.now() - Date.parse(newest.receivedAt) < CHECKBOX_SETTLE_MS,
    schedule a setTimeout for the remaining milliseconds that deletes row.id from
    settleTimers, deferred and gated, then calls this.pump(); store it in settleTimers
    (replacing and clearing any existing timer for that id) and return "deferred".
  4. Otherwise the burst has settled. drain always picks the oldest ungated pending row, so
    row is the first sibling. Set job.checkboxBefore to the first sibling's
    changes.body.from and job.checkboxAfter to the newest sibling's body (comment.body,
    else issue.body, else pull_request.body). For every sibling other than row:
    finishForgejoTrigger(id, "dropped", \coalesced into ${row.id}`)andthis.gated.delete(id). Then continue gating row` normally.

Everything in step 4 is read from payload_json in the database at gate time, so a crash
inside the window loses nothing: the rows are still pending and the next boot's drain
recomputes the same result.

8. Dispatcher: authorisation for a checkbox trigger

Two edits in gate, both in the block at lines 613-651.

Replace the sender/author guard (lines 613-622). It currently drops every edit whose
sender differs from row.commentAuthor; since commentAuthor is now the editor for
edited rows, that comparison would never fire again and would silently open the
edited-mention path to third-party editors. The rule becomes explicit:

		// Forgejo puts the editor of a comment in `sender`. A checkbox change is
		// authorised in the editor's own name below, but any other edit would run
		// the author's trigger with someone else's text, so it must be self-edited.
		if (row.action === "edited" && reason !== "checkbox-change") {
			const editor = typeof payload.sender?.login === "string" ? payload.sender.login : "";
			const written = payload.comment?.user?.login ?? payload.issue?.user?.login ?? "";
			if (editor.toLowerCase() !== written.toLowerCase()) {
				finishForgejoTrigger(row.id, "dropped", "edited by someone other than its author");
				return null;
			}
		}

Non-edited rows keep passing freely, exactly as the old sender === commentAuthor test let
them (for those rows commentAuthor is the sender).

Tighten the permission gate (lines 636-651). The editor must be the repo owner or hold
write/admin; the issue/PR-opener shortcut and the read fallback must not apply, so that an
outside contributor who opened an issue cannot drive the agent by ticking boxes in it:

		const checkbox = reason === "checkbox-change";
		const trusted =
			triggerAuthor === repoOwner || (!checkbox && typeof author === "string" && triggerAuthor === author.toLowerCase());

and inside the perm.data !== "write" && perm.data !== "admin" branch, require
!checkbox && perm.data === "read" && (await this.readOnlyAllowed(job, row, reason)) before
tolerating a read principal. row.commentAuthor is already the editor, so
checkPermission(row.repo, row.commentAuthor) asks about the right user with no further change.

9. Dispatcher: seed selection

At the seed if/else chain (lines 1150-1172), add a branch before the final else:

				} else if (reason === "checkbox-change" && job.checkbox !== null) {
					seed = buildCheckboxEditSeed({
						repo: row.repo,
						number: row.number,
						isPull: row.isPull,
						target: payload.comment ? "comment" : "description",
						editor: row.commentAuthor,
						author: payload.comment?.user?.login ?? payload.issue?.user?.login ?? null,
						change: job.checkbox,
						payload,
						orientation,
					});

ReadyReason already includes TriggerReason (line 277), so "checkbox-change" needs no
union change there.

Critical files & anchors

  • apps/server/src/forgejo/webhook.ts — classifyForgejoDelivery lines 120-187 (comment
    branch and the issues / pull_request action switches) and dedupeKey line 478. The
    header doc comment lists the Forgejo facts the module relies on; add the edited /
    changes.body.from / title-edit facts there.
  • apps/server/src/forgejo/dispatcher.ts — gate line 501 (settle window, sender guard,
    permission gate), detect line 1928, seed chain line 1150, Job line 341, deferred
    line 388, releaseIntakeSlot line 2690 as the model for the timed release.
  • apps/server/src/forgejo/conversation-detect.ts — forEachProseLine line 80 (export),
    TriggerReason line 53, CONVERSATION_REASONS line 62, REASON_TEXT line 258,
    detectAddressed line 167, buildConversationSeed line 311 as the shape to mirror,
    fenceFor line 416.
  • apps/server/src/forgejo/webhook.test.ts — line 137 pins the old "unsupported action
    edited" verdict and must be rewritten.
  • apps/server/src/forgejo/dispatcher.test.ts — trigger(...) helper around line 500 builds
    rows for the harness; reuse it for the end-to-end checks.

Verification

Working directory apps/server for every command.

  1. Diff unit behaviour — new apps/server/src/forgejo/checkbox-diff.test.ts, run with
    bun test src/forgejo/checkbox-diff.test.ts. Cases, each an explicit input → expected output:
    • diffCheckboxes("- [ ] alpha\n- [ ] beta", "- [x] alpha\n- [ ] beta") →
      flips equal to [{ label: "alpha", checked: true }].
    • Untick: same pair reversed → [{ label: "alpha", checked: false }].
    • Pure reorder "- [x] a\n- [ ] b" → "- [ ] b\n- [x] a" → null.
    • Prose-only edit of an item's tail, states unchanged → null.
    • Simultaneous tick and untick under a reorder → non-null, flips === null.
    • Inserted item above a ticked one → non-null, flips === null, with beforeBlocks and
      afterBlocks carrying only the checklist lines.
    • A checklist inside a fenced code block, and one inside a > quote → null.
    • checklistBlocks on a body with prose, a checklist, a blank line and more prose returns
      only the checklist lines.
  2. Receiver — extend src/forgejo/webhook.test.ts, run bun test src/forgejo/webhook.test.ts:
    an issue_comment edited payload whose comment.user.login is the agent and whose
    sender.login is alice is accepted with commentAuthor: "alice"; the same with
    sender.login equal to the agent is ignored with reason "self-edited"; an issues
    edited payload with changes.body.from is accepted, and without it is ignored with
    "no body change".
  3. Detection — extend src/forgejo/conversation-detect.test.ts: detectAddressed with
    action: "edited", checkboxChanged: true and a conversation containing an agent entry
    returns "checkbox-change"; with a conversation holding no agent entry and an
    editedAuthor that is not the agent it returns null; with checkboxChanged: false and a
    newly added mention it still returns "edited-mention".
  4. End-to-end through the dispatcher — extend src/forgejo/dispatcher.test.ts using its
    existing harness, run bun test src/forgejo/dispatcher.test.ts:
    • An issue_comment edited row on an issue where the agent has a comment, whose payload
      ticks one box and whose sender is the repo owner, starts a session whose seed contains
      Trigger: a checkbox was ticked or unticked and the line - [x] alpha. This is the
      headline new behaviour and the check that proves the feature.
    • The same row with a sender whose checkPermission returns "read" is dropped with
      "author lacks write access (read)", even though that user opened the issue.
    • An edited row that adds a mention but changes no checkbox still yields
      "edited-mention", and one edited by a non-author with no checkbox change is still
      dropped with "edited by someone other than its author" — the two regression checks for
      the rewritten guard.
    • Two pending edited rows for the same comment, the newer received less than
      CHECKBOX_SETTLE_MS ago, leave the artifact deferred; advancing past the window runs one
      turn and drops the other row with a reason starting "coalesced into ". Use the
      harness's existing clock/timer control if it has one; otherwise inject
      CHECKBOX_SETTLE_MS through the dispatcher's deps so the test can set it to a few
      milliseconds.
  5. bun test (full server suite) and bun run typecheck from the repository root.

Assumptions & contingencies

  • The deck's Forgejo webhook is already subscribed to the issue and pull request event
    families.
    The receiver classifies issues and pull_request deliveries today, so the
    subscription covers them; but whether this deployment's hook has every needed box ticked
    is unverified — confirm first, since the hook configuration lives in Forgejo, not in this
    repo. If issues edited deliveries never arrive after step 3 ships, the hook's event
    selection is the cause, not the code; check it in Forgejo's repository webhook settings.
  • Forgejo sends changes.body.from with the full previous body, not a truncated one. The
    dispatcher already reads this field for edited-mention (dispatcher.ts:1930) and the
    notifier passes oldContent whole. If a deployment is found to truncate it, fall back to
    flips: null whenever before is shorter than the common prefix of after suggests, and
    let the agent compare the two checklists.
  • 15 seconds is the settle window. If this proves too short in practice (bursts still
    splitting into several turns), raise CHECKBOX_SETTLE_MS; the constant is the only thing to
    change. Do not make it configurable per repository without a new decision.
  • A tick means nothing in particular. The seed deliberately refuses to interpret it, so an
    agent checklist can be used both for "pick the items you want me to do" and for "I did
    these". If turns prove inconsistent, the fix is a convention in the agent's own checklist
    wording, not in the seed.
## Problem When the agent posts a markdown checklist on a Forgejo issue or pull request, a user with write access ticking one of those boxes must wake the agent. The same must work when the edited checklist sits in someone else's comment, in an inline review comment, or in the issue/PR description. Today this does not work, blocked at three independent layers: 1. **Receiver** (`apps/server/src/forgejo/webhook.ts:160-168`): `issue_comment` / `pull_request_comment` with `action: "edited"` is classified, but the author is read from `comment.user.login` and anything authored by the agent is ignored as `"self-authored"`. An edit to the agent's own comment therefore never becomes a trigger row. 2. **Dispatcher** (`apps/server/src/forgejo/dispatcher.ts:613-622`): Forgejo puts the editor in `sender`, and a row whose `sender` differs from `row.commentAuthor` is dropped with `"edited by someone other than its author"`. That is exactly "user ticks a box in the agent's (or a third party's) comment". 3. **Detection** (`apps/server/src/forgejo/conversation-detect.ts:175-182`): for `action: "edited"`, `detectAddressed` returns only `edited-mention` — an edit that adds an `@mention` which was absent before. A checkbox toggle adds no mention, so the row is dropped as `"not addressed to the agent"`. Issue/PR **description** edits are rejected outright: `webhook.ts:137-146` has no `edited` branch and `webhook.test.ts:137` pins `"unsupported action edited"`. Forgejo-side facts, grounded in `services/webhook/notifier.go` and `web_src/js/markup/tasklist.js` on the `forgejo` branch: - `UpdateComment` has **no comment-type guard**, so editing an inline review comment fires the same `issue_comment` / `pull_request_comment` event with `action: "edited"`, `changes.body.from`, and `issue` = the PR. Inline comments need no new event handling. - `IssueChangeContent` fires `issues` (non-PR) or `pull_request` (PR) with `action: "edited"` and `changes.body.from`. - `IssueChangeTitle` **also** fires `action: "edited"`, with `changes.title` and no `changes.body` — so a missing `changes.body.from` must be rejected. - The rendered checkbox widget POSTs the whole rewritten body to the normal content-update URL, one request per click, flipping exactly one character. Clean positional pairing holds in practice; messy diffs only arise from hand-edited markdown. - The widget is interactive only where `data-can-edit=true` (poster or write access), so Forgejo already enforces most of the permission gate; the deck's check is defence in depth. End state: a checkbox state change in a comment, inline review comment, or description, made by the repo owner or a user with write/admin access, on an artifact the agent has taken part in, wakes the agent with a neutral seed naming which boxes flipped. Bursts of toggles collapse into one turn. ## Approach ### 1. Export the prose-line splitter In `apps/server/src/forgejo/conversation-detect.ts`, change `forEachProseLine` (line 80) from module-private to `export`. Signature is unchanged: ```ts export function forEachProseLine(markdown: string, onLine: (line: string) => void): void ``` It emits every line outside fenced code blocks, blank lines and `>` quote lines included, which is what block extraction below needs. No other change to that function. ### 2. New pure module: checkbox parsing and diffing Create `apps/server/src/forgejo/checkbox-diff.ts`. Nothing in the repo parses markdown task lists (searched for `\[[ xX]\]`, `checkbox`, `task.?list` across `apps/server/src` and `packages`), so this is new code. Pure: no DB, no network, mirroring `conversation-detect.ts`. ```ts export interface TaskItem { /** Item text after the checkbox marker, trimmed. */ label: string; checked: boolean; } export interface CheckboxFlip { label: string; /** The item's state *after* the edit. */ checked: boolean; } export interface CheckboxChange { /** The flipped items, or null when the two lists could not be paired reliably. */ flips: CheckboxFlip[] | null; /** Checklist blocks of the body before the edit; "" when there were none. */ beforeBlocks: string; /** Checklist blocks of the body after the edit. */ afterBlocks: string; } ``` **`parseTaskItems(markdown: string): TaskItem[]`** — run `forEachProseLine`; skip lines matching `/^\s*>/` (a quoted checklist is not the author's); for each remaining line match `/^\s*[-*+] \[([ xX])\][ \t]?(.*)$/`, pushing `{ label: m[2].trim(), checked: m[1] !== " " }`. Order is document order. Indentation is ignored, so nested sub-items are ordinary items. **`checklistBlocks(markdown: string): string`** — run `forEachProseLine` collecting lines into an array. Walk it: a block starts at the first line matching the task-item regex and runs up to (excluding) the first line matching `/^\s*$/` or the end of the array; scanning then resumes after that blank line. Join the blocks' lines with `\n`, and separate blocks from one another with a single blank line. Returns `""` when there is no task line. This keeps a long comment from bloating the turn: only the checklist travels, not the whole body. **`firstWord(label: string): string`** (module-private) — lowercase, take the first whitespace-delimited token, strip characters not matching `/[\p{L}\p{N}]/u` from both ends. `""` for an empty label. **`diffCheckboxes(before: string, after: string): CheckboxChange | null`** — 1. `a = parseTaskItems(before)`, `b = parseTaskItems(after)`. 2. Change predicate: build for each side the sorted array of `` `${firstWord(item.label)}\u0000${item.checked ? "1" : "0"}` `` and compare the two with `JSON.stringify`. **Equal → return `null`** (no checkbox state change). This makes a pure reorder, and any prose edit that leaves every item's first word and state alone, a non-event, while a tick combined with an unrelated reorder, and a simultaneous tick + untick, both still register. 3. Pairing: clean when `a.length === b.length` and `firstWord(a[i].label) === firstWord(b[i].label)` for every `i`. Then `flips` = `b[i]` (as `{ label, checked }`) for every `i` where `a[i].checked !== b[i].checked`. Otherwise `flips = null` — the dispatcher does not try harder, it hands the agent both checklists and lets it work out the difference. 4. Return `{ flips, beforeBlocks: checklistBlocks(before), afterBlocks: checklistBlocks(after) }`. ### 3. Accept edits that the receiver currently discards All in `apps/server/src/forgejo/webhook.ts`. **Comment edits** — in the `issue_comment` / `pull_request_comment` branch (`classifyForgejoDelivery`, lines 150-187), the author/self check becomes action-dependent: - `action === "created"`: unchanged. `commentAuthor` is `comment.user.login`; a comment authored by the agent is ignored as `"self-authored"`. - `action === "edited"`: the acting principal is the **editor**. Read `sender.login`; malformed `"missing sender.login"` when absent or not a string. Ignore with reason `"self-edited"` when it equals `agentUsername` case-insensitively — this is the loop guard, since the agent rewrites its own checklists. `comment.user.login` is still required (malformed otherwise) but no longer gates acceptance. The accepted trigger carries `commentAuthor: sender.login`. **Description edits** — add, before the `issues` branch's final `ignore`, and in the `pull_request` family's action switch, an `action === "edited"` case calling a new `classifyDescriptionEdit(eventType, payload, agentUsername, isPull)`: - Reject with `{ kind: "ignore", reason: "no body change" }` unless `payload.changes.body.from` is a string. This is what separates a content edit from a title-only edit, which Forgejo also sends as `edited`. - `sender.login` required (malformed `"missing sender.login"`); ignored as `"self-edited"` when it is the agent. - `repository.full_name` and the number validated exactly as the comment branch does (`number` from `issue.number` for `issues`, from `pull_request.number` or `number` for `pull_request`). - Trigger: `{ event: eventType, action: "edited", repo, number, isPull, commentAuthor: sender.login }`. **Dedupe** — generalise `dedupeKey` (line 478) to take the edited subject instead of only a comment: ```ts function dedupeKey(repo: string, commentId: number, action: string, subject: unknown): string ``` Body unchanged (it already reads `updated_at` and `body` off the value). Comment branches pass `payload.comment` with the real comment id; description edits pass `payload.issue` (or `payload.pull_request`) with `commentId` `0`. Successive toggles differ in `body`, so each toggle gets its own key even inside one second. **Protocol**: no change. `packages/protocol/src/index.ts:2168-2210` already has `issues`, `pull_request`, `issue_comment`, `pull_request_comment` in the `event` union and `edited` in the `action` union. Extend the `commentAuthor` doc comment (line 2211-2217) to say that for `edited` it is the editor, not the author. **Existing test to change**: `apps/server/src/forgejo/webhook.test.ts:137` asserts `issues` + `edited` yields `{ kind: "ignore", reason: "unsupported action edited" }`. That behaviour is being replaced; rewrite the case to assert the new verdicts (accepted with `changes.body.from` present; `"no body change"` without it). ### 4. New trigger reason and detection rule In `apps/server/src/forgejo/conversation-detect.ts`: - Add `"checkbox-change"` to the `TriggerReason` union (line 53) and to the `CONVERSATION_REASONS` set (line 62) — membership there is what lets the turn run in a workspace-less session (`dispatcher.ts:667`). - Add to `REASON_TEXT` (line 258): `"checkbox-change": "a checkbox was ticked or unticked"`. - Extend the `action === "edited"` branch of `detectAddressed` (lines 175-182). Two new optional input fields: ```ts /** True when `diffCheckboxes` found a checkbox state change in this edit. */ checkboxChanged?: boolean; /** Login of the author of the edited comment or description; null when unknown. */ editedAuthor?: string | null; ``` New branch body, replacing the current one: ```ts if (action === "edited") { if (input.checkboxChanged === true && conversation !== null) { const agent = agentUsername.toLowerCase(); const participated = (input.editedAuthor ?? "").toLowerCase() === agent || conversation.some((e) => e.author.toLowerCase() === agent); if (participated) return "checkbox-change"; } const body = bodies[0] ?? ""; return previousBody !== null && mentions(body, agentUsername) && !mentions(previousBody, agentUsername) ? "edited-mention" : null; } ``` The participation gate: the agent wrote the edited text, or it appears anywhere in the conversation. Because the dispatcher prepends `openingEntry` (`dispatcher.ts:1953`), an artifact the agent opened satisfies it too. Checkbox detection outranks `edited-mention` when an edit does both; a failed participation check still falls through to the mention rule, so no existing behaviour is lost. ### 5. Seed builder for a checkbox turn New export in `conversation-detect.ts`, next to `buildConversationSeed`. A dedicated builder rather than overloading `buildConversationSeed`, because the turn has a different shape (editor, flips, two checklists) and may target a description, which has no `payload.comment`. ```ts export function buildCheckboxEditSeed(t: { repo: string; number: number; isPull: boolean; /** "comment" for an issue/inline comment, "description" for the issue/PR body. */ target: "comment" | "description"; /** Login of the user who made the edit. */ editor: string; /** Login of the author of the edited text; null when unknown. */ author: string | null; change: CheckboxChange; payload: IssueCommentPayloadLike; orientation: TurnOrientation; }): string ``` Lines, in order: 1. `Forgejo checkbox change on <repo> <PR|issue> #<number>: "<issue.title ?? "?">"` — and for `target: "description"`, `… in the description`. 2. `Labels: … · Comments: …`, copied verbatim from `buildConversationSeed` (lines 331-332). 3. `Trigger: a checkbox was ticked or unticked` (from `REASON_TEXT`). 4. `Edited by @<editor>` — plus ` in a comment by @<author>` when `author` is non-null and differs from `editor`. 5. `...orientationLines(t.orientation, { isPull: t.isPull, number: t.number })`. 6. When `change.flips !== null` and non-empty: `Changed items:` followed by one `- [x] <label>` / `- [ ] <label>` line per flip, reflecting the state *after* the edit. When `change.flips === null` (or empty): `The checklist could not be matched up reliably. Before and after follow.` then a fenced `change.beforeBlocks` labelled `Before:` and a fenced `change.afterBlocks` labelled `After:`, using `fenceFor` (line 416) on each so the content cannot close its own fence. 7. Always, after the flips list, a fenced `change.afterBlocks` labelled `Checklist now:` (the `flips !== null` path only; the `null` path already showed it). 8. Closing paragraph, neutral by design — the agent decides what a tick means: `A checkbox changed; nothing else is implied. Read the conversation and decide from its content what this asks of you — it may mean the item was selected for you to act on, or that someone reports it done, or neither. If it is not asking anything of you, end the turn without posting.` ### 6. Dispatcher: detection wiring In `apps/server/src/forgejo/dispatcher.ts`. Add to `Job` (line 341): ```ts /** The checkbox change of an `edited` trigger, once computed; null when there is none. */ checkbox: CheckboxChange | null; /** Body before the edit, coalesced across a settled burst. */ checkboxBefore: string | null; /** Body after the edit, coalesced across a settled burst. */ checkboxAfter: string | null; ``` Initialise all three to `null` wherever a `Job` is constructed. Rewrite `detect` (lines 1928-1955). The body of an edit is no longer only `payload.comment?.body`, because a description edit has none: ```ts const editedBody = payload.comment?.body ?? payload.issue?.body ?? payload.pull_request?.body ?? ""; const editedAuthor = payload.comment?.user?.login ?? payload.issue?.user?.login ?? payload.pull_request?.user?.login ?? null; ``` - `bodies` becomes `[editedBody]`; `previousBody` stays `payload.changes?.body?.from ?? null`, but for a settled burst use `job.checkboxBefore ?? from` and `job.checkboxAfter ?? editedBody` (set in step 7). - For `action === "edited"`: compute `job.checkbox = diffCheckboxes(before, after)` when `before !== null`. If `job.checkbox !== null`, fetch the conversation (`this.fetchConversation(job)`, propagating its `"failed"` / `"dropped"` results) and call `detectAddressed` with `conversation: [...openingEntry(...), ...conversation]`, `checkboxChanged: true`, `editedAuthor`. If `job.checkbox === null`, keep today's fast path: `detectAddressed({ ...input, conversation: null })` and return its result without fetching. - `created` behaviour is untouched. ### 7. Dispatcher: 15-second settle window Bursts: each click is a separate HTTP request and a separate trigger row, so a user ticking several boxes would otherwise get one agent turn per tick. The dispatcher already has the mechanism — `deferred` (line 388) keeps a row `pending` and blocks its artifact in `pump` (line 452), and `drain` returns on `"deferred"` (lines 474-478) so later rows stay behind it. Only a timed release is missing; `releaseIntakeSlot` (line 2690) is the model for the release. Add near the other constants (around line 188): ```ts /** How long a checkbox edit waits for further toggles of the same text before running. */ const CHECKBOX_SETTLE_MS = 15_000; ``` Add a field alongside `deferred`: ```ts /** Pending settle-window releases, by trigger row id. */ private readonly settleTimers = new Map<string, ReturnType<typeof setTimeout>>(); ``` Clear every timer in the dispatcher's `dispose`/teardown path (where `disposed` is set), so a shutdown leaves no pending handle. In `gate(row)` (line 501), before any API work, when `row.action === "edited"` and the payload's `changes.body.from` is a string: 1. Compute the row's own diff with `diffCheckboxes(from, body)`. **Null → skip the settle window entirely** and continue gating as a normal (possibly `edited-mention`) edit, so a plain text edit is never delayed. 2. Otherwise build the sibling set from `listPendingForgejoTriggers()`: rows with the same `repo` (case-insensitive) and `number`, `action === "edited"`, the same comment key — the payload's `comment.id`, or `0` for a description edit — and a non-null `diffCheckboxes` of their own. Keep queue order (the query already returns oldest first) and include `row`. 3. Let `newest` be the last sibling. If `Date.now() - Date.parse(newest.receivedAt) < CHECKBOX_SETTLE_MS`, schedule a `setTimeout` for the remaining milliseconds that deletes `row.id` from `settleTimers`, `deferred` and `gated`, then calls `this.pump()`; store it in `settleTimers` (replacing and clearing any existing timer for that id) and return `"deferred"`. 4. Otherwise the burst has settled. `drain` always picks the oldest ungated pending row, so `row` is the first sibling. Set `job.checkboxBefore` to the first sibling's `changes.body.from` and `job.checkboxAfter` to the newest sibling's body (`comment.body`, else `issue.body`, else `pull_request.body`). For every sibling other than `row`: `finishForgejoTrigger(id, "dropped", \`coalesced into ${row.id}\`)` and `this.gated.delete(id)`. Then continue gating `row` normally. Everything in step 4 is read from `payload_json` in the database at gate time, so a crash inside the window loses nothing: the rows are still `pending` and the next boot's drain recomputes the same result. ### 8. Dispatcher: authorisation for a checkbox trigger Two edits in `gate`, both in the block at lines 613-651. **Replace the sender/author guard** (lines 613-622). It currently drops every edit whose `sender` differs from `row.commentAuthor`; since `commentAuthor` is now the editor for `edited` rows, that comparison would never fire again and would silently open the `edited-mention` path to third-party editors. The rule becomes explicit: ```ts // Forgejo puts the editor of a comment in `sender`. A checkbox change is // authorised in the editor's own name below, but any other edit would run // the author's trigger with someone else's text, so it must be self-edited. if (row.action === "edited" && reason !== "checkbox-change") { const editor = typeof payload.sender?.login === "string" ? payload.sender.login : ""; const written = payload.comment?.user?.login ?? payload.issue?.user?.login ?? ""; if (editor.toLowerCase() !== written.toLowerCase()) { finishForgejoTrigger(row.id, "dropped", "edited by someone other than its author"); return null; } } ``` Non-`edited` rows keep passing freely, exactly as the old `sender === commentAuthor` test let them (for those rows `commentAuthor` *is* the sender). **Tighten the permission gate** (lines 636-651). The editor must be the repo owner or hold write/admin; the issue/PR-opener shortcut and the `read` fallback must not apply, so that an outside contributor who opened an issue cannot drive the agent by ticking boxes in it: ```ts const checkbox = reason === "checkbox-change"; const trusted = triggerAuthor === repoOwner || (!checkbox && typeof author === "string" && triggerAuthor === author.toLowerCase()); ``` and inside the `perm.data !== "write" && perm.data !== "admin"` branch, require `!checkbox && perm.data === "read" && (await this.readOnlyAllowed(job, row, reason))` before tolerating a `read` principal. `row.commentAuthor` is already the editor, so `checkPermission(row.repo, row.commentAuthor)` asks about the right user with no further change. ### 9. Dispatcher: seed selection At the seed `if`/`else` chain (lines 1150-1172), add a branch before the final `else`: ```ts } else if (reason === "checkbox-change" && job.checkbox !== null) { seed = buildCheckboxEditSeed({ repo: row.repo, number: row.number, isPull: row.isPull, target: payload.comment ? "comment" : "description", editor: row.commentAuthor, author: payload.comment?.user?.login ?? payload.issue?.user?.login ?? null, change: job.checkbox, payload, orientation, }); ``` `ReadyReason` already includes `TriggerReason` (line 277), so `"checkbox-change"` needs no union change there. ## Critical files & anchors - `apps/server/src/forgejo/webhook.ts` — `classifyForgejoDelivery` lines 120-187 (comment branch and the `issues` / `pull_request` action switches) and `dedupeKey` line 478. The header doc comment lists the Forgejo facts the module relies on; add the `edited` / `changes.body.from` / title-edit facts there. - `apps/server/src/forgejo/dispatcher.ts` — `gate` line 501 (settle window, sender guard, permission gate), `detect` line 1928, seed chain line 1150, `Job` line 341, `deferred` line 388, `releaseIntakeSlot` line 2690 as the model for the timed release. - `apps/server/src/forgejo/conversation-detect.ts` — `forEachProseLine` line 80 (export), `TriggerReason` line 53, `CONVERSATION_REASONS` line 62, `REASON_TEXT` line 258, `detectAddressed` line 167, `buildConversationSeed` line 311 as the shape to mirror, `fenceFor` line 416. - `apps/server/src/forgejo/webhook.test.ts` — line 137 pins the old "unsupported action edited" verdict and must be rewritten. - `apps/server/src/forgejo/dispatcher.test.ts` — `trigger(...)` helper around line 500 builds rows for the harness; reuse it for the end-to-end checks. ## Verification Working directory `apps/server` for every command. 1. **Diff unit behaviour** — new `apps/server/src/forgejo/checkbox-diff.test.ts`, run with `bun test src/forgejo/checkbox-diff.test.ts`. Cases, each an explicit input → expected output: - `diffCheckboxes("- [ ] alpha\n- [ ] beta", "- [x] alpha\n- [ ] beta")` → `flips` equal to `[{ label: "alpha", checked: true }]`. - Untick: same pair reversed → `[{ label: "alpha", checked: false }]`. - Pure reorder `"- [x] a\n- [ ] b"` → `"- [ ] b\n- [x] a"` → `null`. - Prose-only edit of an item's tail, states unchanged → `null`. - Simultaneous tick and untick under a reorder → non-null, `flips === null`. - Inserted item above a ticked one → non-null, `flips === null`, with `beforeBlocks` and `afterBlocks` carrying only the checklist lines. - A checklist inside a fenced code block, and one inside a `>` quote → `null`. - `checklistBlocks` on a body with prose, a checklist, a blank line and more prose returns only the checklist lines. 2. **Receiver** — extend `src/forgejo/webhook.test.ts`, run `bun test src/forgejo/webhook.test.ts`: an `issue_comment` `edited` payload whose `comment.user.login` is the agent and whose `sender.login` is `alice` is accepted with `commentAuthor: "alice"`; the same with `sender.login` equal to the agent is ignored with reason `"self-edited"`; an `issues` `edited` payload with `changes.body.from` is accepted, and without it is ignored with `"no body change"`. 3. **Detection** — extend `src/forgejo/conversation-detect.test.ts`: `detectAddressed` with `action: "edited"`, `checkboxChanged: true` and a conversation containing an agent entry returns `"checkbox-change"`; with a conversation holding no agent entry and an `editedAuthor` that is not the agent it returns `null`; with `checkboxChanged: false` and a newly added mention it still returns `"edited-mention"`. 4. **End-to-end through the dispatcher** — extend `src/forgejo/dispatcher.test.ts` using its existing harness, run `bun test src/forgejo/dispatcher.test.ts`: - An `issue_comment` `edited` row on an issue where the agent has a comment, whose payload ticks one box and whose `sender` is the repo owner, starts a session whose seed contains `Trigger: a checkbox was ticked or unticked` and the line `- [x] alpha`. This is the headline new behaviour and the check that proves the feature. - The same row with a `sender` whose `checkPermission` returns `"read"` is dropped with `"author lacks write access (read)"`, even though that user opened the issue. - An `edited` row that adds a mention but changes no checkbox still yields `"edited-mention"`, and one edited by a non-author with no checkbox change is still dropped with `"edited by someone other than its author"` — the two regression checks for the rewritten guard. - Two pending `edited` rows for the same comment, the newer received less than `CHECKBOX_SETTLE_MS` ago, leave the artifact deferred; advancing past the window runs one turn and drops the other row with a reason starting `"coalesced into "`. Use the harness's existing clock/timer control if it has one; otherwise inject `CHECKBOX_SETTLE_MS` through the dispatcher's deps so the test can set it to a few milliseconds. 5. `bun test` (full server suite) and `bun run typecheck` from the repository root. ## Assumptions & contingencies - **The deck's Forgejo webhook is already subscribed to the issue and pull request event families.** The receiver classifies `issues` and `pull_request` deliveries today, so the subscription covers them; but whether *this* deployment's hook has every needed box ticked is `unverified — confirm first`, since the hook configuration lives in Forgejo, not in this repo. If `issues` `edited` deliveries never arrive after step 3 ships, the hook's event selection is the cause, not the code; check it in Forgejo's repository webhook settings. - **Forgejo sends `changes.body.from` with the full previous body, not a truncated one.** The dispatcher already reads this field for `edited-mention` (`dispatcher.ts:1930`) and the notifier passes `oldContent` whole. If a deployment is found to truncate it, fall back to `flips: null` whenever `before` is shorter than the common prefix of `after` suggests, and let the agent compare the two checklists. - **15 seconds is the settle window.** If this proves too short in practice (bursts still splitting into several turns), raise `CHECKBOX_SETTLE_MS`; the constant is the only thing to change. Do not make it configurable per repository without a new decision. - **A tick means nothing in particular.** The seed deliberately refuses to interpret it, so an agent checklist can be used both for "pick the items you want me to do" and for "I did these". If turns prove inconsistent, the fix is a convention in the agent's own checklist wording, not in the seed.
lucas closed this issue 2026-11-07 09:45:22 +01:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
lucas/omp-deck#229
No description provided.