Forgejo tools to edit and delete the agent's own comments #254

Closed
opened 2026-11-11 10:23:28 +01:00 by omp-deck · 0 comments
Collaborator

Problem

The deck exposes 24 forgejo_* tools, four of which post comments
(forgejo_create_issue_comment, forgejo_create_pr_comment,
forgejo_create_pr_thread_comment, forgejo_post_review). None of them can
change or remove a comment afterwards: a typo, a wrong claim or a comment
posted on the wrong thread stays on a public repository forever, and in
unattended sessions there is no human who could clean it up either.

Add two tools so the agent can correct itself:

  • forgejo_update_comment — rewrite the body of a comment the agent's own
    Forgejo account posted, on an issue or a pull request, plain or inline.
  • forgejo_delete_comment — delete a plain issue/pull-request comment the
    agent's own account posted.

Both hard-refuse comments authored by anyone else: Forgejo authorises
PATCH/DELETE for the poster or any user with write access, and the deck's
token account has write access, so without an own-author check the agent could
rewrite or erase a human's comment. forgejo_delete_comment is restricted to
normal sessions (where it asks for approval); forgejo_update_comment is also
available in the unattended Forgejo session types and in idea mode.

Plan

Forgejo API facts this plan relies on

Read from Forgejo v15.0.9 sources (routers/api/v1/repo/issue_comment.go,
routers/api/v1/repo/pull_review.go, routers/api/v1/api.go,
models/issues/comment.go, modules/structs/issue_comment.go):

  • GET /repos/{o}/{r}/issues/comments/{id} → 200 with the comment
    (id, body, user.login, html_url, issue_url, pull_request_url)
    when it is a plain comment; 204 with an empty body when the id is an
    inline code comment or a review body (GetIssueComment returns
    StatusNoContent for any type other than CommentTypeComment).
    ForgejoClient.json turns an empty body into { ok: true, data: undefined },
    so res.ok && res.data?.id == null is the "not a plain comment" signal.
  • PATCH /repos/{o}/{r}/issues/comments/{id} with { "body": "…" } → 200
    with the updated comment. Accepts every content-bearing type
    (HasContentSupport(): comment, code, review, dismiss-review), so it edits
    inline review comments too. Authorisation: poster or write access. Archived
    repository → 423.
  • DELETE /repos/{o}/{r}/issues/comments/{id} → 204. It filters on
    CommentTypeComment: given an inline comment id it answers 204 without
    deleting anything
    . This is why delete must pre-check the type.
  • Deleting an inline review comment would need
    DELETE /repos/{o}/{r}/pulls/{index}/reviews/{review_id}/comments/{comment}
    (review id required). Out of scope: forgejo_delete_comment refuses inline
    comments and points at forgejo_update_comment.
  • A review body's comment id is not surfaced by any existing tool
    (forgejo_list_pr_comments_and_reviews prints the review id, not a comment
    id), so in practice the reachable edit targets are plain comments and inline
    review comments. Anything else fails resolution with an error.

No webhook loop guard is needed: classifyForgejoDelivery in
apps/server/src/forgejo/webhook.ts already drops edited deliveries whose
sender.login is the agent (self-edited).

Step 1 — Result payload types

In packages/protocol/src/forgejo-tools.ts, next to ForgejoCommentResult,
add (the file is re-exported wholesale by packages/protocol/src/index.ts):

/** `forgejo_update_comment`. `bytes` is the UTF-8 size of the new body. */
export interface ForgejoUpdateCommentResult {
	number: number;
	commentId: number;
	/** Inline review comment (true) or plain issue/pull-request comment (false). */
	inline: boolean;
	url: ForgejoUrl;
	targetUrl: ForgejoUrl;
	bytes: number;
}

/** `forgejo_delete_comment`. `url` is the issue/pull request the comment was on. */
export interface ForgejoDeleteCommentResult {
	number: number;
	commentId: number;
	url: ForgejoUrl;
}

Step 2 — Agent identity in the tool context

apps/server/src/forgejo/tools/shared.ts: add to ForgejoToolContext

	/** Login the configured token authenticates as (`forgejo.agent_username`); the only author whose comments the comment tools may change. */
	agentUsername: string;

apps/server/src/forgejo/tools/index.ts (buildForgejoToolsForRepo): set
agentUsername: cfg.agentUsername in the ctx literal. readForgejoConfig()
already returns it and returns null unless all three fields are stored, so no
extra request and no nullable field.

apps/server/src/forgejo/tools/index.test.ts builds a bare context literal in
describe("self-identifying tools"):
const ctx = { client: null as never, owner: "o", repo: "r", cwd: "/" }; — add
agentUsername: "bot".

Step 3 — Export the inline-comment lookup from prs.ts

apps/server/src/forgejo/tools/prs.ts: change async function fetchInlineComments(…) to export async function fetchInlineComments(…). It
already fetches every review of a PR and flattens their comments with
pull_request_review_id filled in; the new module reuses it instead of
duplicating the walk. No call-site changes.

Step 4 — New module apps/server/src/forgejo/tools/comments.ts

File doc comment: the two comment-maintenance tools, why ownership is enforced
here (Forgejo authorises write-access users, the agent must only touch its own
comments) and why delete refuses inline comments (the DELETE route no-ops
with 204 on them).

Imports mirror issues.ts: z, the two protocol result types, repoApiPath,
ApiComment/ApiReviewComment from ./api-types.ts, fetchInlineComments
from ./prs.ts, and apiError, defineForgejoTool, errorResult,
numberedParams, REPO_SCOPE, textResult, type ForgejoToolContext from
./shared.ts.

Pure helpers (unit-tested, no I/O):

/** Issue/pull-request number from a comment's `issue_url` / `pull_request_url` (`…/issues/12`, `…/pulls/12`); null when neither is parseable. */
export function commentTargetNumber(c: { issue_url?: string | null; pull_request_url?: string | null }): number | null;

/** Verdict on whether `comment` may be changed by `agentUsername` on `#number`. */
export function checkCommentTarget(input: {
	comment: { id: number; user?: { login: string } | null; issue_url?: string | null; pull_request_url?: string | null };
	number: number;
	agentUsername: string;
}): { ok: true } | { ok: false; error: string };

checkCommentTarget rules, in this order:

  1. commentTargetNumber returns a number that is not number →
    `comment ${id} is on #${found}, not #${number}`.
    A null result (neither URL present) is not an error: the membership
    check is skipped, because the author check below is the load-bearing one.
  2. Author missing (user?.login empty/absent — e.g. a migrated comment with
    only original_author) →
    `could not determine who wrote comment ${id}; only ${agentUsername}'s own comments can be changed`.
  3. Author ≠ agentUsername compared with toLowerCase() →
    `comment ${id} was written by ${login}; only ${agentUsername}'s own comments can be changed`.
  4. Otherwise { ok: true }.

Shared resolution (one place, used by both tools):

type Resolved =
	| { kind: "plain"; comment: ApiComment }
	| { kind: "inline"; comment: ApiReviewComment };

/** Fetch the target comment and verify it is on `#number` and written by the agent. */
async function resolveOwnComment(
	ctx: ForgejoToolContext,
	number: number,
	commentId: number,
	signal?: AbortSignal,
): Promise<{ ok: true; target: Resolved } | { ok: false; result: ToolResult }>;

Behaviour:

  1. GET repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(commentId)).
    Not ok → apiError(res) (a wrong id is Forgejo's 404, an outage keeps its
    status, so the agent can tell them apart).
  2. res.data?.id != null → plain comment. Run checkCommentTarget; on failure
    return errorResult(error); else { kind: "plain", comment: res.data }.
  3. Otherwise (the 204 case) the id is an inline/review comment: call
    fetchInlineComments(ctx, number, reviews, signal) after
    GET repoApiPath(ctx.owner, ctx.repo, "pulls", String(number), "reviews")
    (both failures → apiError). Find the entry whose id === commentId.
    • not found → errorResult(comment ${commentId} is not a comment on #${number}; check the number and the id shown by forgejo_list_issue_comments or forgejo_list_pr_comments_and_reviews)
    • found → checkCommentTarget with that comment (its issue_url/
      pull_request_url may be absent, which step 1 above tolerates), then
      { kind: "inline", comment }.

forgejo_update_comment:

const updateComment = defineForgejoTool({
	name: "forgejo_update_comment",
	label: "Forgejo Update Comment",
	description:
		`Forgejo: replace the body of a comment the agent itself posted on an issue or pull request in ${REPO_SCOPE}, plain or inline. The previous body is replaced entirely; read the comment first if you need to keep part of it. Comments written by anyone else are refused.`,
	mutating: true,
	parameters: numberedParams("Issue or pull request", {
		comment_id: z.number().int().describe("Id of the comment to edit, as shown by forgejo_list_issue_comments or forgejo_list_pr_comments_and_reviews"),
		body: z.string().min(1).describe("New body (markdown); replaces the existing one"),
	}),
	…
});

run: resolveOwnComment → on failure return its result; else
PATCH repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))
with { body: { body: p.body } }. Not ok → apiError. On success:

const bytes = Buffer.byteLength(p.body, "utf8");
return textResult(`updated comment ${p.comment_id} on #${p.number} (${bytes} bytes)`, {
	number: p.number,
	commentId: p.comment_id,
	inline: target.kind === "inline",
	url: res.data?.html_url ?? null,
	targetUrl: stripFragment(res.data?.html_url),
	bytes,
} satisfies ForgejoUpdateCommentResult);

(stripFragment from ./shared.ts, same as createIssueComment.)

forgejo_delete_comment:

const deleteComment = defineForgejoTool({
	name: "forgejo_delete_comment",
	label: "Forgejo Delete Comment",
	description:
		`Forgejo: delete a comment the agent itself posted on an issue or pull request in ${REPO_SCOPE}. Irreversible. Comments written by anyone else are refused, and inline review comments cannot be deleted — rewrite them with forgejo_update_comment.`,
	mutating: true,
	parameters: numberedParams("Issue or pull request", {
		comment_id: z.number().int().describe("Id of the comment to delete, as shown by forgejo_list_issue_comments"),
	}),
	…
});

run: resolveOwnComment → on failure return its result. If
target.kind === "inline" →

return errorResult(
	`comment ${p.comment_id} is an inline review comment; Forgejo cannot delete it through this route (it would report success without deleting). Replace its text with forgejo_update_comment instead.`,
);

Else DELETE repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))
(client.json<undefined>; the 204 body is empty, which json reports as
ok with data: undefined). Not ok → apiError. On success:

return textResult(`deleted comment ${p.comment_id} on #${p.number}`, {
	number: p.number,
	commentId: p.comment_id,
	url: stripFragment(target.comment.html_url),
} satisfies ForgejoDeleteCommentResult);

ApiComment in ./api-types.ts must carry the fields used here — check it and
add whichever of html_url, issue_url, pull_request_url, user are
missing as optional fields (the file's convention: everything optional).

End of file: export const COMMENT_TOOLS = [updateComment, deleteComment] as const;

Step 5 — Register the tools

apps/server/src/forgejo/tools/index.ts: import COMMENT_TOOLS from
./comments.ts and extend
FORGEJO_TOOL_DEFS = [...ISSUE_TOOLS, ...PR_TOOLS, ...CI_TOOLS, ...COMMENT_TOOLS].
FORGEJO_TOOL_NAMES and FORGEJO_MUTATING_TOOLS derive from it, so the
approve-by-default behaviour and idea-mode gating pick both tools up
automatically.

Step 6 — Session availability

apps/server/src/bridge/session-policy.ts:

  • Add "forgejo_update_comment" to CONVERSATION_TOOL_NAMES and to
    IMPLEMENTER_TOOL_NAMES. WATCHDOG_TOOL_NAMES is derived from
    CONVERSATION_TOOL_NAMES by filtering, so the watchdog inherits it; leave
    the filter alone.
  • Add forgejo_update_comment: true to FORGEJO_COMMENT_TOOLS, so the
    conversation policy returns "allow" for it (its default ask would become
    an unattended denial). Extend that map's comment: the comment tools post on
    any number in the repository, and forgejo_update_comment additionally only
    touches comments the agent itself wrote.
  • In forgejoCiWatchdogPolicy.decide, extend the reported-issue guard to the
    new tool by changing the existing condition
    if (toolName === "forgejo_create_issue_comment") to
    if (toolName === "forgejo_create_issue_comment" || toolName === "forgejo_update_comment")
    (same reported === null || paramNumber(params) === reported rule and the
    same this CI watchdog reports on #${reported}; comment only there denial).
  • forgejoConversationPolicy.systemContext: after the existing
    `- Reply on #${number}: …` bullet add
    "- You can correct a comment you posted yourself with \forgejo_update_comment` (the issue or pull request number plus the comment id); you cannot change or delete anyone else's comment, and you cannot delete your own."`
  • forgejoCiWatchdogPolicy.systemContext: after the "Report on exactly one
    issue" bullet add
    "- You can correct a comment you posted yourself with \forgejo_update_comment`."`
  • No change for forgejo_delete_comment: it is absent from every unattended
    toolNames list, and each policy's decide already ends with a denial for
    anything not in its list.

Step 7 — Idea mode

apps/server/src/bridge/idea-mode-policy.ts:

  • Add "forgejo_update_comment" to IDEA_ALLOWED_FORGEJO_TOOLS.
  • Change line if (toolName === "forgejo_create_issue_comment") return "default";
    to also return "default" for "forgejo_update_comment", i.e. after the
    res.data.isPull check (idea mode stays issues-only) and before the
    issue-author check (which is about who opened the issue and must not apply
    to comment edits; the comment's own authorship is enforced inside the tool).
  • Update the module doc comment: idea mode keeps issue creation, issue
    comments, comment edits and description updates.

Step 8 — Approval dialog preview

apps/web/src/components/chat/ToolPermissionDialog.tsx, in
formatForgejoParams:

		case "forgejo_update_comment":
			lines.push(ref, `comment ${String(p.comment_id)}`);
			addBody("new body", body);
			break;
		case "forgejo_delete_comment":
			lines.push(ref, `delete comment ${String(p.comment_id)}`);
			break;

Step 9 — Tool cards

New apps/web/src/components/tools/forgejo/UpdateCommentView.tsx, modelled on
CommentView.tsx (same ForgejoShell / ArgRow / MarkdownDetails /
ForgejoLink structure and the same isForgejoUrl-based type guard over
ForgejoUpdateCommentResult): rows show the issue ref (IssueRef), the
comment id, and the new body; the children show
comment <id> linked to url, the word on, and #<number> linked to
targetUrl, with inline rendered as a trailing inline review comment tag
when true.

New apps/web/src/components/tools/forgejo/DeleteCommentView.tsx, same shape
over ForgejoDeleteCommentResult: rows show the issue ref and the comment id;
the children show deleted comment <id> (no link, the comment is gone) and a
link to url.

apps/web/src/components/tools/ForgejoTool.tsx: import both and add

		case "forgejo_update_comment":
			return <UpdateCommentView {...view} />;
		case "forgejo_delete_comment":
			return <DeleteCommentView {...view} />;

to the switch in ForgejoTool, and in summarizeForgejoArgs add both names to
the case group that returns ref(num) (alongside
forgejo_create_issue_comment).

Step 10 — Session system-context block

apps/server/src/forgejo/tools/context.ts, inside the returned template, after
the ### Before posting section:

### Fixing your own comments
- A comment you posted yourself can be rewritten with \`forgejo_update_comment\` (issue/pull-request number + comment id, body replaced entirely) and a plain one deleted with \`forgejo_delete_comment\`. Both refuse comments written by anyone else.
- Prefer editing over deleting; an inline review comment cannot be deleted, only rewritten.

Step 11 — Tests

  • apps/server/src/forgejo/tools/comments.test.ts (new, bun:test, pure
    helpers only, in the style of prs.test.ts): commentTargetNumber parses
    issue_url and pull_request_url and returns null for neither/garbage;
    checkCommentTarget accepts the agent's own comment including a different
    login case, rejects a foreign author naming it, rejects a missing author,
    rejects a comment whose URL number differs from number, and accepts when
    neither URL is present.
  • apps/server/src/forgejo/tools/index.test.ts: bump the inventory test to
    26 tools: 13 read + 13 mutating (FORGEJO_TOOL_DEFS.length,
    FORGEJO_TOOL_NAMES.size), add both names to MUTATING_TOOLS, add
    update_comment and delete_comment to the bare list in
    describe("self-identifying tools"), add agentUsername: "bot" to its ctx
    literal, and change the two expect(tools.length).toBe(24) assertions in
    describe("buildForgejoTools") / describe("tools fail soft") to 26. Add
    ["forgejo_update_comment", { number: 1, comment_id: 1, body: "x" }] to the
    fail-soft case list so the unreachable-instance path is covered for the new
    code.
  • apps/server/src/bridge/permission-filter.test.ts: add both names to the
    MUTATING list (it asserts the default-ask rule set).
  • apps/server/src/bridge/idea-mode-policy.test.ts: add a case that
    forgejo_update_comment on an issue number returns "default" and on a pull
    request number is denied with is a pull request, and that
    forgejo_delete_comment is denied with not available in idea mode.
  • apps/server/src/bridge/session-policy.test.ts: assert
    forgejo_update_comment is "allow" in a conversation session, that the
    watchdog denies it for a number other than its reported issue with
    this CI watchdog reports on #7; comment only there, and that
    forgejo_delete_comment is denied in a conversation session with
    not available in unattended Forgejo conversation sessions.

Verification

Run from the repository root.

  1. bun test apps/server/src/forgejo/tools apps/server/src/bridge — the unit
    suites above, including the new comments.test.ts.
  2. bun run typecheck (or the repo's configured typecheck script, see
    package.json) — catches the ForgejoToolContext.agentUsername addition at
    every construction site and the protocol re-export.
  3. End-to-end against the real instance, in a session whose cwd is this
    checkout (Forgejo must be configured in Settings → Forgejo; the tools only
    register when readForgejoConfig() returns a config and the cwd's origin
    points at that instance):
    • forgejo_create_issue_comment on a scratch issue with body v1; note the
      returned comment id.
    • forgejo_update_comment with that number and id and body v2 → result
      text updated comment <id> on #<n> (2 bytes); the comment on the web page
      reads v2 and carries Forgejo's "edited" marker.
    • forgejo_update_comment with the same id but a different issue number →
      error comment <id> is on #<n>, not #<other>, and the comment is
      unchanged.
    • forgejo_update_comment against a comment written by a human → error
      … was written by <login>; only <agent>'s own comments can be changed,
      body unchanged on the web page.
    • forgejo_update_comment against an inline review comment the agent posted
      (id from forgejo_list_pr_comments_and_reviews) → succeeds, and the
      inline comment's text changes in the PR's diff view.
    • forgejo_delete_comment on that same inline id → error naming
      forgejo_update_comment, comment still present.
    • forgejo_delete_comment on the agent's plain comment → result
      deleted comment <id> on #<n>, comment gone from the page.
    • Confirm no new deck session was started by any of these edits (Settings →
      Forgejo → Recent deliveries shows the edited deliveries as ignored,
      reason self-edited).

Assumptions & contingencies

  • The stored forgejo.agent_username is the login the token authenticates as
    (saveForgejoConfig requires callers to verify it). If a deployment's stored
    value has drifted from the token's real account, every ownership check fails
    closed with "was written by …" — the fix is re-saving the connection in
    Settings → Forgejo, not a code change, and the tools must not call
    GET /user per invocation to paper over it.
  • fetchInlineComments expands at most MAX_REVIEWS_EXPANDED (100) reviews of
    a pull request. If an inline comment on a PR with more reviews than that
    cannot be found, resolveOwnComment returns the "not a comment on #N" error.
    That is the intended outcome: refuse rather than edit unverified.
  • (unverified — confirm first) The exact Forgejo version behind
    git.sclu1034.dev was not checked; all route and status-code claims come
    from Forgejo v15.0.9 sources. If the instance is older and
    GET /issues/comments/{id} is unavailable (404 for a known-good plain
    comment id during step 3 of Verification), fall back to resolving plain
    comments through GET /repos/{o}/{r}/issues/{number}/comments and matching
    the id there; keep every ownership rule unchanged.
## Problem The deck exposes 24 `forgejo_*` tools, four of which post comments (`forgejo_create_issue_comment`, `forgejo_create_pr_comment`, `forgejo_create_pr_thread_comment`, `forgejo_post_review`). None of them can change or remove a comment afterwards: a typo, a wrong claim or a comment posted on the wrong thread stays on a public repository forever, and in unattended sessions there is no human who could clean it up either. Add two tools so the agent can correct itself: - `forgejo_update_comment` — rewrite the body of a comment the agent's own Forgejo account posted, on an issue or a pull request, plain or inline. - `forgejo_delete_comment` — delete a plain issue/pull-request comment the agent's own account posted. Both hard-refuse comments authored by anyone else: Forgejo authorises `PATCH`/`DELETE` for the poster *or* any user with write access, and the deck's token account has write access, so without an own-author check the agent could rewrite or erase a human's comment. `forgejo_delete_comment` is restricted to normal sessions (where it asks for approval); `forgejo_update_comment` is also available in the unattended Forgejo session types and in idea mode. ## Plan ### Forgejo API facts this plan relies on Read from Forgejo v15.0.9 sources (`routers/api/v1/repo/issue_comment.go`, `routers/api/v1/repo/pull_review.go`, `routers/api/v1/api.go`, `models/issues/comment.go`, `modules/structs/issue_comment.go`): - `GET /repos/{o}/{r}/issues/comments/{id}` → `200` with the comment (`id`, `body`, `user.login`, `html_url`, `issue_url`, `pull_request_url`) when it is a plain comment; **`204` with an empty body** when the id is an inline code comment or a review body (`GetIssueComment` returns `StatusNoContent` for any type other than `CommentTypeComment`). `ForgejoClient.json` turns an empty body into `{ ok: true, data: undefined }`, so `res.ok && res.data?.id == null` is the "not a plain comment" signal. - `PATCH /repos/{o}/{r}/issues/comments/{id}` with `{ "body": "…" }` → `200` with the updated comment. Accepts every content-bearing type (`HasContentSupport()`: comment, code, review, dismiss-review), so it edits inline review comments too. Authorisation: poster or write access. Archived repository → `423`. - `DELETE /repos/{o}/{r}/issues/comments/{id}` → `204`. It filters on `CommentTypeComment`: given an inline comment id it answers `204` **without deleting anything**. This is why delete must pre-check the type. - Deleting an inline review comment would need `DELETE /repos/{o}/{r}/pulls/{index}/reviews/{review_id}/comments/{comment}` (review id required). Out of scope: `forgejo_delete_comment` refuses inline comments and points at `forgejo_update_comment`. - A review *body*'s comment id is not surfaced by any existing tool (`forgejo_list_pr_comments_and_reviews` prints the review id, not a comment id), so in practice the reachable edit targets are plain comments and inline review comments. Anything else fails resolution with an error. No webhook loop guard is needed: `classifyForgejoDelivery` in `apps/server/src/forgejo/webhook.ts` already drops `edited` deliveries whose `sender.login` is the agent (`self-edited`). ### Step 1 — Result payload types In `packages/protocol/src/forgejo-tools.ts`, next to `ForgejoCommentResult`, add (the file is re-exported wholesale by `packages/protocol/src/index.ts`): ```ts /** `forgejo_update_comment`. `bytes` is the UTF-8 size of the new body. */ export interface ForgejoUpdateCommentResult { number: number; commentId: number; /** Inline review comment (true) or plain issue/pull-request comment (false). */ inline: boolean; url: ForgejoUrl; targetUrl: ForgejoUrl; bytes: number; } /** `forgejo_delete_comment`. `url` is the issue/pull request the comment was on. */ export interface ForgejoDeleteCommentResult { number: number; commentId: number; url: ForgejoUrl; } ``` ### Step 2 — Agent identity in the tool context `apps/server/src/forgejo/tools/shared.ts`: add to `ForgejoToolContext` ```ts /** Login the configured token authenticates as (`forgejo.agent_username`); the only author whose comments the comment tools may change. */ agentUsername: string; ``` `apps/server/src/forgejo/tools/index.ts` (`buildForgejoToolsForRepo`): set `agentUsername: cfg.agentUsername` in the `ctx` literal. `readForgejoConfig()` already returns it and returns `null` unless all three fields are stored, so no extra request and no nullable field. `apps/server/src/forgejo/tools/index.test.ts` builds a bare context literal in `describe("self-identifying tools")`: `const ctx = { client: null as never, owner: "o", repo: "r", cwd: "/" };` — add `agentUsername: "bot"`. ### Step 3 — Export the inline-comment lookup from `prs.ts` `apps/server/src/forgejo/tools/prs.ts`: change `async function fetchInlineComments(…)` to `export async function fetchInlineComments(…)`. It already fetches every review of a PR and flattens their comments with `pull_request_review_id` filled in; the new module reuses it instead of duplicating the walk. No call-site changes. ### Step 4 — New module `apps/server/src/forgejo/tools/comments.ts` File doc comment: the two comment-maintenance tools, why ownership is enforced here (Forgejo authorises write-access users, the agent must only touch its own comments) and why delete refuses inline comments (the `DELETE` route no-ops with 204 on them). Imports mirror `issues.ts`: `z`, the two protocol result types, `repoApiPath`, `ApiComment`/`ApiReviewComment` from `./api-types.ts`, `fetchInlineComments` from `./prs.ts`, and `apiError`, `defineForgejoTool`, `errorResult`, `numberedParams`, `REPO_SCOPE`, `textResult`, `type ForgejoToolContext` from `./shared.ts`. **Pure helpers (unit-tested, no I/O):** ```ts /** Issue/pull-request number from a comment's `issue_url` / `pull_request_url` (`…/issues/12`, `…/pulls/12`); null when neither is parseable. */ export function commentTargetNumber(c: { issue_url?: string | null; pull_request_url?: string | null }): number | null; /** Verdict on whether `comment` may be changed by `agentUsername` on `#number`. */ export function checkCommentTarget(input: { comment: { id: number; user?: { login: string } | null; issue_url?: string | null; pull_request_url?: string | null }; number: number; agentUsername: string; }): { ok: true } | { ok: false; error: string }; ``` `checkCommentTarget` rules, in this order: 1. `commentTargetNumber` returns a number that is not `number` → `` `comment ${id} is on #${found}, not #${number}` ``. A `null` result (neither URL present) is **not** an error: the membership check is skipped, because the author check below is the load-bearing one. 2. Author missing (`user?.login` empty/absent — e.g. a migrated comment with only `original_author`) → `` `could not determine who wrote comment ${id}; only ${agentUsername}'s own comments can be changed` ``. 3. Author ≠ `agentUsername` compared with `toLowerCase()` → `` `comment ${id} was written by ${login}; only ${agentUsername}'s own comments can be changed` ``. 4. Otherwise `{ ok: true }`. **Shared resolution (one place, used by both tools):** ```ts type Resolved = | { kind: "plain"; comment: ApiComment } | { kind: "inline"; comment: ApiReviewComment }; /** Fetch the target comment and verify it is on `#number` and written by the agent. */ async function resolveOwnComment( ctx: ForgejoToolContext, number: number, commentId: number, signal?: AbortSignal, ): Promise<{ ok: true; target: Resolved } | { ok: false; result: ToolResult }>; ``` Behaviour: 1. `GET repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(commentId))`. Not ok → `apiError(res)` (a wrong id is Forgejo's `404`, an outage keeps its status, so the agent can tell them apart). 2. `res.data?.id != null` → plain comment. Run `checkCommentTarget`; on failure return `errorResult(error)`; else `{ kind: "plain", comment: res.data }`. 3. Otherwise (the `204` case) the id is an inline/review comment: call `fetchInlineComments(ctx, number, reviews, signal)` after `GET repoApiPath(ctx.owner, ctx.repo, "pulls", String(number), "reviews")` (both failures → `apiError`). Find the entry whose `id === commentId`. - not found → `errorResult(`comment ${commentId} is not a comment on #${number}; check the number and the id shown by forgejo_list_issue_comments or forgejo_list_pr_comments_and_reviews`)` - found → `checkCommentTarget` with that comment (its `issue_url`/ `pull_request_url` may be absent, which step 1 above tolerates), then `{ kind: "inline", comment }`. **`forgejo_update_comment`:** ```ts const updateComment = defineForgejoTool({ name: "forgejo_update_comment", label: "Forgejo Update Comment", description: `Forgejo: replace the body of a comment the agent itself posted on an issue or pull request in ${REPO_SCOPE}, plain or inline. The previous body is replaced entirely; read the comment first if you need to keep part of it. Comments written by anyone else are refused.`, mutating: true, parameters: numberedParams("Issue or pull request", { comment_id: z.number().int().describe("Id of the comment to edit, as shown by forgejo_list_issue_comments or forgejo_list_pr_comments_and_reviews"), body: z.string().min(1).describe("New body (markdown); replaces the existing one"), }), … }); ``` `run`: `resolveOwnComment` → on failure return its `result`; else `PATCH repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))` with `{ body: { body: p.body } }`. Not ok → `apiError`. On success: ```ts const bytes = Buffer.byteLength(p.body, "utf8"); return textResult(`updated comment ${p.comment_id} on #${p.number} (${bytes} bytes)`, { number: p.number, commentId: p.comment_id, inline: target.kind === "inline", url: res.data?.html_url ?? null, targetUrl: stripFragment(res.data?.html_url), bytes, } satisfies ForgejoUpdateCommentResult); ``` (`stripFragment` from `./shared.ts`, same as `createIssueComment`.) **`forgejo_delete_comment`:** ```ts const deleteComment = defineForgejoTool({ name: "forgejo_delete_comment", label: "Forgejo Delete Comment", description: `Forgejo: delete a comment the agent itself posted on an issue or pull request in ${REPO_SCOPE}. Irreversible. Comments written by anyone else are refused, and inline review comments cannot be deleted — rewrite them with forgejo_update_comment.`, mutating: true, parameters: numberedParams("Issue or pull request", { comment_id: z.number().int().describe("Id of the comment to delete, as shown by forgejo_list_issue_comments"), }), … }); ``` `run`: `resolveOwnComment` → on failure return its `result`. If `target.kind === "inline"` → ```ts return errorResult( `comment ${p.comment_id} is an inline review comment; Forgejo cannot delete it through this route (it would report success without deleting). Replace its text with forgejo_update_comment instead.`, ); ``` Else `DELETE repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))` (`client.json<undefined>`; the `204` body is empty, which `json` reports as `ok` with `data: undefined`). Not ok → `apiError`. On success: ```ts return textResult(`deleted comment ${p.comment_id} on #${p.number}`, { number: p.number, commentId: p.comment_id, url: stripFragment(target.comment.html_url), } satisfies ForgejoDeleteCommentResult); ``` `ApiComment` in `./api-types.ts` must carry the fields used here — check it and add whichever of `html_url`, `issue_url`, `pull_request_url`, `user` are missing as optional fields (the file's convention: everything optional). End of file: `export const COMMENT_TOOLS = [updateComment, deleteComment] as const;` ### Step 5 — Register the tools `apps/server/src/forgejo/tools/index.ts`: import `COMMENT_TOOLS` from `./comments.ts` and extend `FORGEJO_TOOL_DEFS = [...ISSUE_TOOLS, ...PR_TOOLS, ...CI_TOOLS, ...COMMENT_TOOLS]`. `FORGEJO_TOOL_NAMES` and `FORGEJO_MUTATING_TOOLS` derive from it, so the approve-by-default behaviour and idea-mode gating pick both tools up automatically. ### Step 6 — Session availability `apps/server/src/bridge/session-policy.ts`: - Add `"forgejo_update_comment"` to `CONVERSATION_TOOL_NAMES` and to `IMPLEMENTER_TOOL_NAMES`. `WATCHDOG_TOOL_NAMES` is derived from `CONVERSATION_TOOL_NAMES` by filtering, so the watchdog inherits it; leave the filter alone. - Add `forgejo_update_comment: true` to `FORGEJO_COMMENT_TOOLS`, so the conversation policy returns `"allow"` for it (its default *ask* would become an unattended denial). Extend that map's comment: the comment tools post on any number in the repository, and `forgejo_update_comment` additionally only touches comments the agent itself wrote. - In `forgejoCiWatchdogPolicy.decide`, extend the reported-issue guard to the new tool by changing the existing condition `if (toolName === "forgejo_create_issue_comment")` to `if (toolName === "forgejo_create_issue_comment" || toolName === "forgejo_update_comment")` (same `reported === null || paramNumber(params) === reported` rule and the same `this CI watchdog reports on #${reported}; comment only there` denial). - `forgejoConversationPolicy.systemContext`: after the existing `` `- Reply on #${number}: …` `` bullet add `"- You can correct a comment you posted yourself with \`forgejo_update_comment\` (the issue or pull request number plus the comment id); you cannot change or delete anyone else's comment, and you cannot delete your own."` - `forgejoCiWatchdogPolicy.systemContext`: after the "Report on exactly one issue" bullet add `"- You can correct a comment you posted yourself with \`forgejo_update_comment\`."` - No change for `forgejo_delete_comment`: it is absent from every unattended `toolNames` list, and each policy's `decide` already ends with a denial for anything not in its list. ### Step 7 — Idea mode `apps/server/src/bridge/idea-mode-policy.ts`: - Add `"forgejo_update_comment"` to `IDEA_ALLOWED_FORGEJO_TOOLS`. - Change line `if (toolName === "forgejo_create_issue_comment") return "default";` to also return `"default"` for `"forgejo_update_comment"`, i.e. after the `res.data.isPull` check (idea mode stays issues-only) and before the issue-author check (which is about who *opened the issue* and must not apply to comment edits; the comment's own authorship is enforced inside the tool). - Update the module doc comment: idea mode keeps issue creation, issue comments, comment edits and description updates. ### Step 8 — Approval dialog preview `apps/web/src/components/chat/ToolPermissionDialog.tsx`, in `formatForgejoParams`: ```ts case "forgejo_update_comment": lines.push(ref, `comment ${String(p.comment_id)}`); addBody("new body", body); break; case "forgejo_delete_comment": lines.push(ref, `delete comment ${String(p.comment_id)}`); break; ``` ### Step 9 — Tool cards New `apps/web/src/components/tools/forgejo/UpdateCommentView.tsx`, modelled on `CommentView.tsx` (same `ForgejoShell` / `ArgRow` / `MarkdownDetails` / `ForgejoLink` structure and the same `isForgejoUrl`-based type guard over `ForgejoUpdateCommentResult`): rows show the issue ref (`IssueRef`), the comment id, and the new body; the children show `comment <id>` linked to `url`, the word `on`, and `#<number>` linked to `targetUrl`, with `inline` rendered as a trailing `inline review comment` tag when true. New `apps/web/src/components/tools/forgejo/DeleteCommentView.tsx`, same shape over `ForgejoDeleteCommentResult`: rows show the issue ref and the comment id; the children show `deleted comment <id>` (no link, the comment is gone) and a link to `url`. `apps/web/src/components/tools/ForgejoTool.tsx`: import both and add ```tsx case "forgejo_update_comment": return <UpdateCommentView {...view} />; case "forgejo_delete_comment": return <DeleteCommentView {...view} />; ``` to the switch in `ForgejoTool`, and in `summarizeForgejoArgs` add both names to the `case` group that returns `ref(num)` (alongside `forgejo_create_issue_comment`). ### Step 10 — Session system-context block `apps/server/src/forgejo/tools/context.ts`, inside the returned template, after the `### Before posting` section: ``` ### Fixing your own comments - A comment you posted yourself can be rewritten with \`forgejo_update_comment\` (issue/pull-request number + comment id, body replaced entirely) and a plain one deleted with \`forgejo_delete_comment\`. Both refuse comments written by anyone else. - Prefer editing over deleting; an inline review comment cannot be deleted, only rewritten. ``` ### Step 11 — Tests - `apps/server/src/forgejo/tools/comments.test.ts` (new, `bun:test`, pure helpers only, in the style of `prs.test.ts`): `commentTargetNumber` parses `issue_url` and `pull_request_url` and returns `null` for neither/garbage; `checkCommentTarget` accepts the agent's own comment including a different login case, rejects a foreign author naming it, rejects a missing author, rejects a comment whose URL number differs from `number`, and accepts when neither URL is present. - `apps/server/src/forgejo/tools/index.test.ts`: bump the inventory test to `26 tools: 13 read + 13 mutating` (`FORGEJO_TOOL_DEFS.length`, `FORGEJO_TOOL_NAMES.size`), add both names to `MUTATING_TOOLS`, add `update_comment` and `delete_comment` to the `bare` list in `describe("self-identifying tools")`, add `agentUsername: "bot"` to its `ctx` literal, and change the two `expect(tools.length).toBe(24)` assertions in `describe("buildForgejoTools")` / `describe("tools fail soft")` to `26`. Add `["forgejo_update_comment", { number: 1, comment_id: 1, body: "x" }]` to the fail-soft case list so the unreachable-instance path is covered for the new code. - `apps/server/src/bridge/permission-filter.test.ts`: add both names to the `MUTATING` list (it asserts the default-ask rule set). - `apps/server/src/bridge/idea-mode-policy.test.ts`: add a case that `forgejo_update_comment` on an issue number returns `"default"` and on a pull request number is denied with `is a pull request`, and that `forgejo_delete_comment` is denied with `not available in idea mode`. - `apps/server/src/bridge/session-policy.test.ts`: assert `forgejo_update_comment` is `"allow"` in a conversation session, that the watchdog denies it for a number other than its reported issue with `this CI watchdog reports on #7; comment only there`, and that `forgejo_delete_comment` is denied in a conversation session with `not available in unattended Forgejo conversation sessions`. ## Verification Run from the repository root. 1. `bun test apps/server/src/forgejo/tools apps/server/src/bridge` — the unit suites above, including the new `comments.test.ts`. 2. `bun run typecheck` (or the repo's configured typecheck script, see `package.json`) — catches the `ForgejoToolContext.agentUsername` addition at every construction site and the protocol re-export. 3. End-to-end against the real instance, in a session whose cwd is this checkout (Forgejo must be configured in Settings → Forgejo; the tools only register when `readForgejoConfig()` returns a config and the cwd's origin points at that instance): - `forgejo_create_issue_comment` on a scratch issue with body `v1`; note the returned comment id. - `forgejo_update_comment` with that number and id and body `v2` → result text `updated comment <id> on #<n> (2 bytes)`; the comment on the web page reads `v2` and carries Forgejo's "edited" marker. - `forgejo_update_comment` with the same id but a *different* issue number → error `comment <id> is on #<n>, not #<other>`, and the comment is unchanged. - `forgejo_update_comment` against a comment written by a human → error `… was written by <login>; only <agent>'s own comments can be changed`, body unchanged on the web page. - `forgejo_update_comment` against an inline review comment the agent posted (id from `forgejo_list_pr_comments_and_reviews`) → succeeds, and the inline comment's text changes in the PR's diff view. - `forgejo_delete_comment` on that same inline id → error naming `forgejo_update_comment`, comment still present. - `forgejo_delete_comment` on the agent's plain comment → result `deleted comment <id> on #<n>`, comment gone from the page. - Confirm no new deck session was started by any of these edits (Settings → Forgejo → Recent deliveries shows the `edited` deliveries as ignored, reason `self-edited`). ## Assumptions & contingencies - The stored `forgejo.agent_username` is the login the token authenticates as (`saveForgejoConfig` requires callers to verify it). If a deployment's stored value has drifted from the token's real account, every ownership check fails closed with "was written by …" — the fix is re-saving the connection in Settings → Forgejo, not a code change, and the tools must not call `GET /user` per invocation to paper over it. - `fetchInlineComments` expands at most `MAX_REVIEWS_EXPANDED` (100) reviews of a pull request. If an inline comment on a PR with more reviews than that cannot be found, `resolveOwnComment` returns the "not a comment on #N" error. That is the intended outcome: refuse rather than edit unverified. - *(unverified — confirm first)* The exact Forgejo version behind `git.sclu1034.dev` was not checked; all route and status-code claims come from Forgejo v15.0.9 sources. If the instance is older and `GET /issues/comments/{id}` is unavailable (`404` for a known-good plain comment id during step 3 of Verification), fall back to resolving plain comments through `GET /repos/{o}/{r}/issues/{number}/comments` and matching the id there; keep every ownership rule unchanged.
lucas closed this issue 2026-11-11 11:20:08 +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#254
No description provided.