Forgejo tools to edit and delete the agent's own comments #254
Labels
No labels
Agent
Ignore
Agent
In Progress
Agent
Planned
Agent
Planning
Agent
Ready
Agent
Triage
Compat
Breaking
Kind
Bug
Kind
Documentation
Kind
Enhancement
Kind
Feature
Kind
Security
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
lucas/omp-deck#254
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 canchange 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 ownForgejo account posted, on an issue or a pull request, plain or inline.
forgejo_delete_comment— delete a plain issue/pull-request comment theagent's own account posted.
Both hard-refuse comments authored by anyone else: Forgejo authorises
PATCH/DELETEfor the poster or any user with write access, and the deck'stoken account has write access, so without an own-author check the agent could
rewrite or erase a human's comment.
forgejo_delete_commentis restricted tonormal sessions (where it asks for approval);
forgejo_update_commentis alsoavailable 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}→200with the comment(
id,body,user.login,html_url,issue_url,pull_request_url)when it is a plain comment;
204with an empty body when the id is aninline code comment or a review body (
GetIssueCommentreturnsStatusNoContentfor any type other thanCommentTypeComment).ForgejoClient.jsonturns an empty body into{ ok: true, data: undefined },so
res.ok && res.data?.id == nullis the "not a plain comment" signal.PATCH /repos/{o}/{r}/issues/comments/{id}with{ "body": "…" }→200with the updated comment. Accepts every content-bearing type
(
HasContentSupport(): comment, code, review, dismiss-review), so it editsinline review comments too. Authorisation: poster or write access. Archived
repository →
423.DELETE /repos/{o}/{r}/issues/comments/{id}→204. It filters onCommentTypeComment: given an inline comment id it answers204withoutdeleting anything. This is why delete must pre-check the type.
DELETE /repos/{o}/{r}/pulls/{index}/reviews/{review_id}/comments/{comment}(review id required). Out of scope:
forgejo_delete_commentrefuses inlinecomments and points at
forgejo_update_comment.(
forgejo_list_pr_comments_and_reviewsprints the review id, not a commentid), 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:
classifyForgejoDeliveryinapps/server/src/forgejo/webhook.tsalready dropsediteddeliveries whosesender.loginis the agent (self-edited).Step 1 — Result payload types
In
packages/protocol/src/forgejo-tools.ts, next toForgejoCommentResult,add (the file is re-exported wholesale by
packages/protocol/src/index.ts):Step 2 — Agent identity in the tool context
apps/server/src/forgejo/tools/shared.ts: add toForgejoToolContextapps/server/src/forgejo/tools/index.ts(buildForgejoToolsForRepo): setagentUsername: cfg.agentUsernamein thectxliteral.readForgejoConfig()already returns it and returns
nullunless all three fields are stored, so noextra request and no nullable field.
apps/server/src/forgejo/tools/index.test.tsbuilds a bare context literal indescribe("self-identifying tools"):const ctx = { client: null as never, owner: "o", repo: "r", cwd: "/" };— addagentUsername: "bot".Step 3 — Export the inline-comment lookup from
prs.tsapps/server/src/forgejo/tools/prs.ts: changeasync function fetchInlineComments(…)toexport async function fetchInlineComments(…). Italready fetches every review of a PR and flattens their comments with
pull_request_review_idfilled in; the new module reuses it instead ofduplicating the walk. No call-site changes.
Step 4 — New module
apps/server/src/forgejo/tools/comments.tsFile 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
DELETEroute no-opswith 204 on them).
Imports mirror
issues.ts:z, the two protocol result types,repoApiPath,ApiComment/ApiReviewCommentfrom./api-types.ts,fetchInlineCommentsfrom
./prs.ts, andapiError,defineForgejoTool,errorResult,numberedParams,REPO_SCOPE,textResult,type ForgejoToolContextfrom./shared.ts.Pure helpers (unit-tested, no I/O):
checkCommentTargetrules, in this order:commentTargetNumberreturns a number that is notnumber→`comment ${id} is on #${found}, not #${number}`.A
nullresult (neither URL present) is not an error: the membershipcheck is skipped, because the author check below is the load-bearing one.
user?.loginempty/absent — e.g. a migrated comment withonly
original_author) →`could not determine who wrote comment ${id}; only ${agentUsername}'s own comments can be changed`.agentUsernamecompared withtoLowerCase()→`comment ${id} was written by ${login}; only ${agentUsername}'s own comments can be changed`.{ ok: true }.Shared resolution (one place, used by both tools):
Behaviour:
GET repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(commentId)).Not ok →
apiError(res)(a wrong id is Forgejo's404, an outage keeps itsstatus, so the agent can tell them apart).
res.data?.id != null→ plain comment. RuncheckCommentTarget; on failurereturn
errorResult(error); else{ kind: "plain", comment: res.data }.204case) the id is an inline/review comment: callfetchInlineComments(ctx, number, reviews, signal)afterGET repoApiPath(ctx.owner, ctx.repo, "pulls", String(number), "reviews")(both failures →
apiError). Find the entry whoseid === commentId.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)checkCommentTargetwith that comment (itsissue_url/pull_request_urlmay be absent, which step 1 above tolerates), then{ kind: "inline", comment }.forgejo_update_comment:run:resolveOwnComment→ on failure return itsresult; elsePATCH repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))with
{ body: { body: p.body } }. Not ok →apiError. On success:(
stripFragmentfrom./shared.ts, same ascreateIssueComment.)forgejo_delete_comment:run:resolveOwnComment→ on failure return itsresult. Iftarget.kind === "inline"→Else
DELETE repoApiPath(ctx.owner, ctx.repo, "issues", "comments", String(p.comment_id))(
client.json<undefined>; the204body is empty, whichjsonreports asokwithdata: undefined). Not ok →apiError. On success:ApiCommentin./api-types.tsmust carry the fields used here — check it andadd whichever of
html_url,issue_url,pull_request_url,useraremissing 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: importCOMMENT_TOOLSfrom./comments.tsand extendFORGEJO_TOOL_DEFS = [...ISSUE_TOOLS, ...PR_TOOLS, ...CI_TOOLS, ...COMMENT_TOOLS].FORGEJO_TOOL_NAMESandFORGEJO_MUTATING_TOOLSderive from it, so theapprove-by-default behaviour and idea-mode gating pick both tools up
automatically.
Step 6 — Session availability
apps/server/src/bridge/session-policy.ts:"forgejo_update_comment"toCONVERSATION_TOOL_NAMESand toIMPLEMENTER_TOOL_NAMES.WATCHDOG_TOOL_NAMESis derived fromCONVERSATION_TOOL_NAMESby filtering, so the watchdog inherits it; leavethe filter alone.
forgejo_update_comment: truetoFORGEJO_COMMENT_TOOLS, so theconversation policy returns
"allow"for it (its default ask would becomean unattended denial). Extend that map's comment: the comment tools post on
any number in the repository, and
forgejo_update_commentadditionally onlytouches comments the agent itself wrote.
forgejoCiWatchdogPolicy.decide, extend the reported-issue guard to thenew tool by changing the existing condition
if (toolName === "forgejo_create_issue_comment")toif (toolName === "forgejo_create_issue_comment" || toolName === "forgejo_update_comment")(same
reported === null || paramNumber(params) === reportedrule and thesame
this CI watchdog reports on #${reported}; comment only theredenial).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 oneissue" bullet add
"- You can correct a comment you posted yourself with \forgejo_update_comment`."`forgejo_delete_comment: it is absent from every unattendedtoolNameslist, and each policy'sdecidealready ends with a denial foranything not in its list.
Step 7 — Idea mode
apps/server/src/bridge/idea-mode-policy.ts:"forgejo_update_comment"toIDEA_ALLOWED_FORGEJO_TOOLS.if (toolName === "forgejo_create_issue_comment") return "default";to also return
"default"for"forgejo_update_comment", i.e. after theres.data.isPullcheck (idea mode stays issues-only) and before theissue-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).
comments, comment edits and description updates.
Step 8 — Approval dialog preview
apps/web/src/components/chat/ToolPermissionDialog.tsx, informatForgejoParams:Step 9 — Tool cards
New
apps/web/src/components/tools/forgejo/UpdateCommentView.tsx, modelled onCommentView.tsx(sameForgejoShell/ArgRow/MarkdownDetails/ForgejoLinkstructure and the sameisForgejoUrl-based type guard overForgejoUpdateCommentResult): rows show the issue ref (IssueRef), thecomment id, and the new body; the children show
comment <id>linked tourl, the wordon, and#<number>linked totargetUrl, withinlinerendered as a trailinginline review commenttagwhen true.
New
apps/web/src/components/tools/forgejo/DeleteCommentView.tsx, same shapeover
ForgejoDeleteCommentResult: rows show the issue ref and the comment id;the children show
deleted comment <id>(no link, the comment is gone) and alink to
url.apps/web/src/components/tools/ForgejoTool.tsx: import both and addto the switch in
ForgejoTool, and insummarizeForgejoArgsadd both names tothe
casegroup that returnsref(num)(alongsideforgejo_create_issue_comment).Step 10 — Session system-context block
apps/server/src/forgejo/tools/context.ts, inside the returned template, afterthe
### Before postingsection:Step 11 — Tests
apps/server/src/forgejo/tools/comments.test.ts(new,bun:test, purehelpers only, in the style of
prs.test.ts):commentTargetNumberparsesissue_urlandpull_request_urland returnsnullfor neither/garbage;checkCommentTargetaccepts the agent's own comment including a differentlogin case, rejects a foreign author naming it, rejects a missing author,
rejects a comment whose URL number differs from
number, and accepts whenneither URL is present.
apps/server/src/forgejo/tools/index.test.ts: bump the inventory test to26 tools: 13 read + 13 mutating(FORGEJO_TOOL_DEFS.length,FORGEJO_TOOL_NAMES.size), add both names toMUTATING_TOOLS, addupdate_commentanddelete_commentto thebarelist indescribe("self-identifying tools"), addagentUsername: "bot"to itsctxliteral, and change the two
expect(tools.length).toBe(24)assertions indescribe("buildForgejoTools")/describe("tools fail soft")to26. Add["forgejo_update_comment", { number: 1, comment_id: 1, body: "x" }]to thefail-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 theMUTATINGlist (it asserts the default-ask rule set).apps/server/src/bridge/idea-mode-policy.test.ts: add a case thatforgejo_update_commenton an issue number returns"default"and on a pullrequest number is denied with
is a pull request, and thatforgejo_delete_commentis denied withnot available in idea mode.apps/server/src/bridge/session-policy.test.ts: assertforgejo_update_commentis"allow"in a conversation session, that thewatchdog denies it for a number other than its reported issue with
this CI watchdog reports on #7; comment only there, and thatforgejo_delete_commentis denied in a conversation session withnot available in unattended Forgejo conversation sessions.Verification
Run from the repository root.
bun test apps/server/src/forgejo/tools apps/server/src/bridge— the unitsuites above, including the new
comments.test.ts.bun run typecheck(or the repo's configured typecheck script, seepackage.json) — catches theForgejoToolContext.agentUsernameaddition atevery construction site and the protocol re-export.
checkout (Forgejo must be configured in Settings → Forgejo; the tools only
register when
readForgejoConfig()returns a config and the cwd's originpoints at that instance):
forgejo_create_issue_commenton a scratch issue with bodyv1; note thereturned comment id.
forgejo_update_commentwith that number and id and bodyv2→ resulttext
updated comment <id> on #<n> (2 bytes); the comment on the web pagereads
v2and carries Forgejo's "edited" marker.forgejo_update_commentwith the same id but a different issue number →error
comment <id> is on #<n>, not #<other>, and the comment isunchanged.
forgejo_update_commentagainst 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_commentagainst an inline review comment the agent posted(id from
forgejo_list_pr_comments_and_reviews) → succeeds, and theinline comment's text changes in the PR's diff view.
forgejo_delete_commenton that same inline id → error namingforgejo_update_comment, comment still present.forgejo_delete_commenton the agent's plain comment → resultdeleted comment <id> on #<n>, comment gone from the page.Forgejo → Recent deliveries shows the
editeddeliveries as ignored,reason
self-edited).Assumptions & contingencies
forgejo.agent_usernameis the login the token authenticates as(
saveForgejoConfigrequires callers to verify it). If a deployment's storedvalue 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 /userper invocation to paper over it.fetchInlineCommentsexpands at mostMAX_REVIEWS_EXPANDED(100) reviews ofa pull request. If an inline comment on a PR with more reviews than that
cannot be found,
resolveOwnCommentreturns the "not a comment on #N" error.That is the intended outcome: refuse rather than edit unverified.
git.sclu1034.devwas not checked; all route and status-code claims comefrom Forgejo v15.0.9 sources. If the instance is older and
GET /issues/comments/{id}is unavailable (404for a known-good plaincomment id during step 3 of Verification), fall back to resolving plain
comments through
GET /repos/{o}/{r}/issues/{number}/commentsand matchingthe id there; keep every ownership rule unchanged.
this.swapLabels(typecheck error + failing test) #258