Let unattended Forgejo sessions comment on any issue or PR in their repository #180

New issue

Closed

opened 2026-10-31 01:46:36 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-10-31 01:46:36 +01:00

Collaborator

Problem

Unattended, trigger-started sessions can only comment on the issue/PR that started them. If a conversation session tries to comment on a different issue, it gets denied, e.g. only #7 may be commented on in this unattended conversation session. The restriction lives in decide() of all three policies in apps/server/src/bridge/session-policy.ts:

forgejo_conversation: only scope.number
forgejo_pull_request: only scope.number
forgejo_implementer: scope.number plus PRs mapped to the session (getForgejoSession(...) === sessionId)

Wanted: sessions of all three kinds can post comments (forgejo_create_issue_comment, forgejo_create_pr_comment, forgejo_create_pr_thread_comment) on any issue or PR in the session's own repository, with no prompt. #N stays the place to answer the triggering conversation. Commenting on other issues/PRs is allowed when relevant.

What stays unchanged: the same-repo confinement (repoMismatch / assertTriggerRepo). The forgejo_post_review restriction in PR sessions stays (own PR only). forgejo_abandon_implementation is still limited to scope.number. The thread tool's own check that comment_id belongs to the given PR is in the tool itself and is not touched.

Plan

Approach

All edits go in apps/server/src/bridge/session-policy.ts and its tests.

Policy decisions. In each of the three decide() methods, replace the FORGEJO_COMMENT_TOOLS block with if (Object.hasOwn(FORGEJO_COMMENT_TOOLS, toolName)) return "allow";:
- Conversation policy (~line 213): also update the comment above it. It becomes: "Any issue or PR in the repository (repoMismatch confines the repo); the issue and PR comment tools work for both because PR comments use issue routes. "allow" so the default ask doesn't become an unattended denial; user deny rules still win."
- PR policy (~line 279): replace the block the same way.
- Implementer policy (~line 360): replace the block the same way. Remove the "PRs the session opened are mapped…" comment. Drop getForgejoSession from the import on line 24, since it has no other use in the file. Keep listForgejoSessionNumbers.
- Update the comment above FORGEJO_COMMENT_TOOLS (lines 97–98). It becomes: "All three post on an issue/PR given by number, which may be any number in the session's repository; the thread tool itself checks that its comment_id is an inline comment of that PR."
System prompts.
- Conversation systemContext line 234: change - Reply only on #${number}: ... to - Reply on #${number}: ... (keep the rest of the sentence). Add a new bullet right after it: "- You may also comment on other issues or pull requests in this repository when it is relevant (e.g. to cross-reference related work)."
- PR systemContext line 320: change "Reply only on" to "Reply on" the same way and add the same bullet after it.
- Implementer systemContext: append the same bullet to the shared rules array (after the forgejo_create_issue bullet, ~line 405).
Tests.
- apps/server/src/bridge/session-policy.test.ts:
  - Line ~26 test: rename it to "inline-thread replies are allowed on any number". Expect "allow" for number 8.
  - Line ~75 test: rename it to "comments are allowed on any number". Expect "allow" for number 8.
  - Line ~187 test: rename it to "comments are allowed on any number". Assert "allow" for number 7 and for number 99 with sessions s1 and s2, with no mapping needed. Remove the deny constant and the setForgejoSession call if nothing else in that test uses them.
- apps/server/src/bridge/permission-filter.test.ts line 131: rewrite the test as "commenting on another number runs without a prompt". Expect prompt null, isError false, and h.ran equal to ["forgejo_create_pr_comment"]. Drop the toContain("#7") assertion. The deny-rule test at ~line 170 stays: it proves that user deny rules still win.

Verification

cd apps/server && bun test src/bridge/session-policy.test.ts src/bridge/permission-filter.test.ts: all tests pass. With the conversation trigger on #7, decidePolicy(TRIGGER, "forgejo_create_pr_thread_comment", {number: 8, ...}, "s") returns "allow".
Typecheck the server package (bun run typecheck or the repo's equivalent) to catch the unused import.

## Problem Unattended, trigger-started sessions can only comment on the issue/PR that started them. If a conversation session tries to comment on a different issue, it gets denied, e.g. `only #7 may be commented on in this unattended conversation session`. The restriction lives in `decide()` of all three policies in `apps/server/src/bridge/session-policy.ts`: - `forgejo_conversation`: only `scope.number` - `forgejo_pull_request`: only `scope.number` - `forgejo_implementer`: `scope.number` plus PRs mapped to the session (`getForgejoSession(...) === sessionId`) Wanted: sessions of all three kinds can post comments (`forgejo_create_issue_comment`, `forgejo_create_pr_comment`, `forgejo_create_pr_thread_comment`) on any issue or PR in the session's own repository, with no prompt. `#N` stays the place to answer the triggering conversation. Commenting on other issues/PRs is allowed when relevant. What stays unchanged: the same-repo confinement (`repoMismatch` / `assertTriggerRepo`). The `forgejo_post_review` restriction in PR sessions stays (own PR only). `forgejo_abandon_implementation` is still limited to `scope.number`. The thread tool's own check that `comment_id` belongs to the given PR is in the tool itself and is not touched. ## Plan ### Approach All edits go in `apps/server/src/bridge/session-policy.ts` and its tests. 1. **Policy decisions.** In each of the three `decide()` methods, replace the `FORGEJO_COMMENT_TOOLS` block with `if (Object.hasOwn(FORGEJO_COMMENT_TOOLS, toolName)) return "allow";`: - Conversation policy (~line 213): also update the comment above it. It becomes: "Any issue or PR in the repository (repoMismatch confines the repo); the issue and PR comment tools work for both because PR comments use issue routes. "allow" so the default ask doesn't become an unattended denial; user deny rules still win." - PR policy (~line 279): replace the block the same way. - Implementer policy (~line 360): replace the block the same way. Remove the "PRs the session opened are mapped…" comment. Drop `getForgejoSession` from the import on line 24, since it has no other use in the file. Keep `listForgejoSessionNumbers`. - Update the comment above `FORGEJO_COMMENT_TOOLS` (lines 97–98). It becomes: "All three post on an issue/PR given by `number`, which may be any number in the session's repository; the thread tool itself checks that its `comment_id` is an inline comment of that PR." 2. **System prompts.** - Conversation `systemContext` line 234: change `- Reply only on #${number}: ...` to `- Reply on #${number}: ...` (keep the rest of the sentence). Add a new bullet right after it: `"- You may also comment on other issues or pull requests in this repository when it is relevant (e.g. to cross-reference related work)."` - PR `systemContext` line 320: change "Reply only on" to "Reply on" the same way and add the same bullet after it. - Implementer `systemContext`: append the same bullet to the shared `rules` array (after the `forgejo_create_issue` bullet, ~line 405). 3. **Tests.** - `apps/server/src/bridge/session-policy.test.ts`: - Line ~26 test: rename it to "inline-thread replies are allowed on any number". Expect `"allow"` for number 8. - Line ~75 test: rename it to "comments are allowed on any number". Expect `"allow"` for number 8. - Line ~187 test: rename it to "comments are allowed on any number". Assert `"allow"` for number 7 and for number 99 with sessions s1 and s2, with no mapping needed. Remove the `deny` constant and the `setForgejoSession` call if nothing else in that test uses them. - `apps/server/src/bridge/permission-filter.test.ts` line 131: rewrite the test as "commenting on another number runs without a prompt". Expect `prompt` null, `isError` false, and `h.ran` equal to `["forgejo_create_pr_comment"]`. Drop the `toContain("#7")` assertion. The deny-rule test at ~line 170 stays: it proves that user deny rules still win. ### Verification - `cd apps/server && bun test src/bridge/session-policy.test.ts src/bridge/permission-filter.test.ts`: all tests pass. With the conversation trigger on #7, `decidePolicy(TRIGGER, "forgejo_create_pr_thread_comment", {number: 8, ...}, "s")` returns `"allow"`. - Typecheck the server package (`bun run typecheck` or the repo's equivalent) to catch the unused import.