Forgejo: give conversation and PR sessions read access to their checkout #49

New issue

Closed

opened 2026-10-20 13:53:16 +02:00 by omp-deck · 0 comments

omp-deck commented

2026-10-20 13:53:16 +02:00

Collaborator

Implementation: #52

Problem

forgejo_read_file_at_ref is in the tool set of unattended Forgejo conversation and pull request sessions, but it only snapshots a file to local://tree@<sha7>/<path> and tells the agent to open it with read — which those sessions don't have. So the tool is available but unusable. Beyond that, answers on issues and PR reviews often benefit from looking at the code in the session's checkout.

Give these sessions read, grep, find and glob.

These tools bypass PermissionFilter and can't be confined to the checkout. This is accepted: trigger sessions run in a per-issue/PR clone, and only comments from users with push access start them — the same trust bar as implementer sessions, which already have read and bash. The general problem of read tools bypassing permissions is tracked separately (deck inbox).

Plan

Approach

Extend the tool surface in apps/server/src/bridge/session-policy.ts: prepend "read", "grep", "find", "glob" (in that order, same as IMPLEMENTER_TOOL_NAMES) to CONVERSATION_TOOL_NAMES. This covers both forgejoConversationPolicy and forgejoPullRequestPolicy (the latter spreads CONVERSATION_TOOL_NAMES). No decide change: both policies already return "default" for any tool in toolNames, and these tools never reach decide anyway.
Rewrite the comment above CONVERSATION_TOOL_NAMES (currently "No read/grep/find/glob: they can't be confined…") to state the actual reasoning: read/grep/find/glob are included even though they bypass PermissionFilter and can't be confined to the checkout, because the session runs in its own per-issue/PR clone and is only started by comments from users with push access (same trust bar as implementer sessions); there is still no file write, bash or ask (ask would hang without a UI).
System context: in both forgejoConversationPolicy.systemContext and forgejoPullRequestPolicy.systemContext, add a bullet after the "Read the thread/conversation first" bullet: "- Your working directory is a checkout of the repository; use \read`, `grep`, `find` and `glob` to look at code when it helps your answer. `forgejo_read_file_at_ref` snapshots a file at another ref; open the snapshot with `read`."` Keep the existing "You cannot change … code" bullet.
Tests in apps/server/src/bridge/session-policy.test.ts: the "tools outside the surface are denied" test (≈line 87) uses "read"; change it to "bash" with params { command: "ls" }, same expected deny message. Add one test asserting policyToolNames of both a forgejo_conversation and a forgejo_pull_request trigger contain read, grep, find, glob and do not contain bash, edit, write.
Docs: in docs/configuration.md (≈line 237, trigger sessions bullet), add read, grep, find, glob (read-only, not confined to the checkout) to the listed tool set and change "There is no file access, bash, …" to "There is no file writing, bash, …".

Critical files & anchors

apps/server/src/bridge/session-policy.ts — CONVERSATION_TOOL_NAMES + its comment; both systemContext functions.
apps/server/src/bridge/session-policy.test.ts — deny test using read.
docs/configuration.md — trigger-session tool set bullet.

Verification

cd apps/server && bun test src/bridge/session-policy.test.ts src/bridge/permission-filter.test.ts src/forgejo — all pass, including the new tool-surface test.
Manual (needs configured Forgejo webhook): comment @<agent> what does forgejo_read_file_at_ref return for README.md on main? on an issue; the session calls forgejo_read_file_at_ref, then read on the local://tree@… path, and the reply quotes file content.

Implementation: #52 ## Problem `forgejo_read_file_at_ref` is in the tool set of unattended Forgejo conversation and pull request sessions, but it only snapshots a file to `local://tree@<sha7>/<path>` and tells the agent to open it with `read` — which those sessions don't have. So the tool is available but unusable. Beyond that, answers on issues and PR reviews often benefit from looking at the code in the session's checkout. Give these sessions `read`, `grep`, `find` and `glob`. These tools bypass `PermissionFilter` and can't be confined to the checkout. This is accepted: trigger sessions run in a per-issue/PR clone, and only comments from users with push access start them — the same trust bar as implementer sessions, which already have `read` and `bash`. The general problem of read tools bypassing permissions is tracked separately (deck inbox). ## Plan ### Approach 1. **Extend the tool surface** in `apps/server/src/bridge/session-policy.ts`: prepend `"read"`, `"grep"`, `"find"`, `"glob"` (in that order, same as `IMPLEMENTER_TOOL_NAMES`) to `CONVERSATION_TOOL_NAMES`. This covers both `forgejoConversationPolicy` and `forgejoPullRequestPolicy` (the latter spreads `CONVERSATION_TOOL_NAMES`). No `decide` change: both policies already return `"default"` for any tool in `toolNames`, and these tools never reach `decide` anyway. 2. **Rewrite the comment above `CONVERSATION_TOOL_NAMES`** (currently "No read/grep/find/glob: they can't be confined…") to state the actual reasoning: read/grep/find/glob are included even though they bypass `PermissionFilter` and can't be confined to the checkout, because the session runs in its own per-issue/PR clone and is only started by comments from users with push access (same trust bar as implementer sessions); there is still no file write, `bash` or `ask` (`ask` would hang without a UI). 3. **System context**: in both `forgejoConversationPolicy.systemContext` and `forgejoPullRequestPolicy.systemContext`, add a bullet after the "Read the thread/conversation first" bullet: `"- Your working directory is a checkout of the repository; use \`read\`, \`grep\`, \`find\` and \`glob\` to look at code when it helps your answer. \`forgejo_read_file_at_ref\` snapshots a file at another ref; open the snapshot with \`read\`."` Keep the existing "You cannot change … code" bullet. 4. **Tests** in `apps/server/src/bridge/session-policy.test.ts`: the "tools outside the surface are denied" test (≈line 87) uses `"read"`; change it to `"bash"` with params `{ command: "ls" }`, same expected deny message. Add one test asserting `policyToolNames` of both a `forgejo_conversation` and a `forgejo_pull_request` trigger contain `read`, `grep`, `find`, `glob` and do not contain `bash`, `edit`, `write`. 5. **Docs**: in `docs/configuration.md` (≈line 237, trigger sessions bullet), add `read`, `grep`, `find`, `glob` (read-only, not confined to the checkout) to the listed tool set and change "There is no file access, `bash`, …" to "There is no file writing, `bash`, …". ### Critical files & anchors - `apps/server/src/bridge/session-policy.ts` — `CONVERSATION_TOOL_NAMES` + its comment; both `systemContext` functions. - `apps/server/src/bridge/session-policy.test.ts` — deny test using `read`. - `docs/configuration.md` — trigger-session tool set bullet. ### Verification - `cd apps/server && bun test src/bridge/session-policy.test.ts src/bridge/permission-filter.test.ts src/forgejo` — all pass, including the new tool-surface test. - Manual (needs configured Forgejo webhook): comment `@<agent> what does forgejo_read_file_at_ref return for README.md on main?` on an issue; the session calls `forgejo_read_file_at_ref`, then `read` on the `local://tree@…` path, and the reply quotes file content.