Allow conversation sessions to relabel their own issue or pull request #241

New issue

Closed

opened 2026-11-09 14:40:00 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-11-09 14:40:00 +01:00

Collaborator

Problem

Unattended Forgejo conversation sessions — both @mention turns and triage
(issue-handler) turns, which are forgejo_conversation sessions carrying a mark
in forgejo_issue_handler_sessions — cannot change labels. forgejo_update_issue_labels
is absent from CONVERSATION_TOOL_NAMES (apps/server/src/bridge/session-policy.ts:158-186),
so the tool is never even constructed for them, and their system prompt states
You cannot change labels, state, descriptions, branches or code.

This blocks the documented triage use case: issue_handler.prompt in
.omp/forgejo.yaml is "the steering half of the triage task — what to analyse,
label or assign", and the commented example in .omp/forgejo.yaml:7-8 reads
"Apply a severity label (sev/1..sev/4) and assign the owner …". A repository
can ask for labeling, and the session then has no tool to do it.

Give every forgejo_conversation session the label tool, restricted to the
issue or PR the session is about, with the repository's automation-steering
labels off limits.

Plan

1. Put the tool on the conversation surface, keep it off the watchdog

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

Append "forgejo_update_issue_labels" to CONVERSATION_TOOL_NAMES (after
"forgejo_create_issue", line 185). CONVERSATION_NO_CHECKOUT_TOOL_NAMES
(line 188) filters out only git_-prefixed names, so the no-checkout variant
picks it up automatically — intended, since labeling needs no checkout.
Extend the WATCHDOG_TOOL_NAMES filter (lines 292-294) with
&& name !== "forgejo_update_issue_labels". The CI watchdog's system prompt
(line 328) says "You cannot change code, branches, labels or issue state"
and stays true.
IMPLEMENTER_TOOL_NAMES and forgejoPullRequestPolicy keep their own lists
and are not touched.

Why an explicit "allow" is required in step 3: forgejo_update_issue_labels
is in FORGEJO_MUTATING_TOOLS, so defaultAsk (permission-filter.ts:55-58)
turns it into an approval prompt, and under a trigger an ask becomes a denial.
Returning "default" from the policy would make the tool unusable. Returning
"allow" skips user allow/ask rules but not user deny rules, which
permission-filter.ts checks before consulting the policy.

2. Make `Status/Blocked` shareable

BLOCKED_LABEL = "Status/Blocked" is currently a module-local constant in
apps/server/src/forgejo/dispatcher.ts:212. dispatcher.ts imports
session-policy.ts, so importing the constant from there would create a cycle.

Move it instead: add export const BLOCKED_LABEL = "Status/Blocked"; to
apps/server/src/forgejo/repo-config.ts (next to REPO_CONFIG_PATH, line 37),
delete the local constant in dispatcher.ts:212, and import it there from
./repo-config.ts — a module dispatcher.ts already imports (parseRepoConfig,
sameLabel). repo-config.ts imports only the yaml package, so
session-policy.ts importing it introduces no cycle.

grep -n "BLOCKED_LABEL" apps/server/src must return only repo-config.ts,
dispatcher.ts and the new session-policy.ts use.

3. Protected-label check in the conversation policy

In session-policy.ts, add the import:

import { BLOCKED_LABEL, labelKey, parseRepoConfig, REPO_CONFIG_PATH, sameLabel } from "../forgejo/repo-config.ts";

Add the fallback constant and the helper above forgejoConversationPolicy:

// Namespace of the deck's own workflow labels, protected when the repository
// config — the authoritative list — cannot be read.
const AGENT_LABEL_PREFIX = "agent/";

/**
 * Denial when `params` adds or removes a label that steers this repository's
 * automation, null when every named label is free to change. The authoritative
 * list is `.omp/forgejo.yaml`, read live because it changes without a restart;
 * when it cannot be read or parsed, `Status/Blocked` and the `agent/`
 * namespace stay protected and everything else is allowed.
 */
async function protectedLabelDenial(
	scope: { owner: string; repo: string },
	params: unknown,
	api: () => DispatchApi | null,
	signal?: AbortSignal,
): Promise<{ deny: string } | null>

Behavior, in order:

Collect the touched names: add and remove from params, keeping only
non-empty strings. Empty set → return null (the tool itself rejects a call
with neither, forgejo/tools/issues.ts:208-212).
Read the config: const client = api(); when non-null,
await client.readRepoFile(scope.owner, scope.repo, REPO_CONFIG_PATH, signal),
then parseRepoConfig(file.data) when file.ok. A 404 is not a failure —
readRepoFile maps it to { ok: true, data: null } and parseRepoConfig(null)
returns DEFAULT_REPO_CONFIG, whose label fields are all null, so only
BLOCKED_LABEL is protected.
Build the protected list.
- Config available (parsed.ok): BLOCKED_LABEL, issueHandler?.label,
  implementation.intakeLabel, implementation.progressLabel,
  implementation.planningLabel, review?.label,
  ciWatchdogOthersPrs?.label, and every entry of review?.excludeLabels
  and ciWatchdogOthersPrs?.excludeLabels. Drop nulls.
- Config unavailable (api() null, !file.ok, or !parsed.ok): just
  BLOCKED_LABEL, plus the rule that any name with
  labelKey(name).startsWith(AGENT_LABEL_PREFIX) is protected. This
  agent/ rule applies only on this fallback path, never when the
  config was read.
Match with sameLabel(name, protectedName) (repo-config.ts:413), which
folds case and -/_/space; / is preserved, so Agent/Ready,
agent ready and AGENT-Ready all collide, while agentic-ux does not
match the agent/ prefix.
On the first match return
{ deny: \the label "${name}" steers this repository's automation; propose the change in your comment instead` }. The tool applies addandremove` in one atomic update, so one protected
name denies the whole call.

Then rewrite forgejoConversationPolicy.decide (lines 255-272) as async decide
(the SessionPolicy.decide signature already permits a promise, line 122, and
forgejoImplementerPolicy is async, line 462). Keep every existing branch in
place and insert the new one after the forgejo_create_issue branch (line 268),
before the noCheckout git check:

if (toolName === "forgejo_update_issue_labels") {
	if (paramNumber(params) !== scope.number) {
		return { deny: `this session may only change labels on #${scope.number}` };
	}
	return (await protectedLabelDenial(scope, params, api, signal)) ?? "allow";
}

The signature of decide gains the api and signal parameters it currently
drops; sessionId and cwd keep their _-prefixed names (still unused).
Note the deliberate asymmetry with the comment tools directly above, which allow
any number in the repository: labels are state, comments are not.

4. System-prompt orientation

In forgejoConversationPolicy.systemContext, replace the single bullet at line
285 with two bullets (keep every other line of the array as is):

`- You can add and remove labels on #${number} with \`forgejo_update_issue_labels\`, but not on any other issue or pull request, and not labels that steer this repository's automation (its issue-handler, implementation, review and CI-watchdog labels, and \`Status/Blocked\`); those are refused with the label named.`,
"- You cannot change state, descriptions, branches or code. Propose such actions in your reply. You can file unrelated problems or follow-up work as new issues with `forgejo_create_issue`.",

The per-turn triage task text in conversation-detect.ts:765-767 ("Do not
change labels or anything else; do not implement") is left unchanged on
purpose: the built-in triage steering stays passive, and a repository opts
into labeling by setting issue_handler.prompt, whose branch (lines 768-772)
replaces that text entirely. Consequently conversation-detect.ts, its tests
(conversation-detect.test.ts:527-547) and the repo-config.ts:54-57 doc
comment are out of scope for this change.

5. Tests

In apps/server/src/bridge/session-policy.test.ts, inside
describe("decidePolicy: forgejo_conversation") (line 42). The existing
TRIGGER (line 22) has scope: { owner: "o", repo: "r", number: 7, isPull: true }
and NO_API (line 27) returns null — reuse both. Add a stub next to pullsApi
(line 30):

/** A Forgejo whose `readRepoFile` answers with `text`, or fails with `fail`. */
function configApi(text: string | null, fail?: { status: number; message: string }): () => DispatchApi

Cases:

surface — policyToolNames(TRIGGER) contains forgejo_update_issue_labels;
so does policyToolNames for a noCheckout: true conversation scope (see the
existing no-checkout trigger at line 80); the forgejo_ci_watchdog trigger's
names do not contain it.
own number only — decidePolicy(TRIGGER, "forgejo_update_issue_labels", { number: 7, add: ["kind/bug"] }, "s", process.cwd(), configApi(null))
is "allow"; the same call with number: 8 denies with
may only change labels on #7.
configured automation label — configApi("issue_handler:\n label: needs-triage\nimplementation:\n intake_label: Agent/Ready\n progress_label: Agent/In Progress\n"):
{ number: 7, add: ["Agent/Ready"] } and { number: 7, remove: ["needs triage"] }
(normalization) both deny with steers this repository's automation, while
{ number: 7, add: ["kind/bug"] } is "allow".
Status/Blocked with a config present — denied even though the file names
no labels (configApi(null)).
config unreadable — configApi(null, { status: 500, message: "boom" })
and NO_API: add: ["kind/bug"] is "allow", while add: ["Agent/Ready"],
add: ["agent/anything"] and remove: ["Status/Blocked"] deny. Also assert
add: ["agentic-ux"] is "allow", pinning that the fallback matches the
agent/ namespace and not a bare agent prefix.
agent/ is fallback-only — with configApi("issue_handler:\n label: needs-triage\n"),
add: ["Agent/Ready"] is "allow" (the config does not list it).

The cross-kind tests at lines 359, 416, 430 and 442 iterate every trigger kind
including TRIGGER; none asserts an exhaustive tool list, so they need no
change. Re-read them before editing to confirm.

6. Documentation of the fixed tool set

In docs/configuration.md:250, extend the trigger tool-set sentence after the
forgejo_create_issue clause with:

…, and forgejo_update_issue_labels on the triggering issue or PR only,
excluding the labels that steer the repository's automation (the
issue_handler, implementation, review and CI-watchdog labels from
.omp/forgejo.yaml, and Status/Blocked).

Leave line 221 alone: it describes the attended default-ask behavior, which is
unchanged — trigger sessions never reach defaultAsk for this tool now.

Verification

Run from the repository root.

bun test apps/server/src/bridge/session-policy.test.ts — all tests pass,
including the new cases from step 5. This is the end-to-end proof of the new
behavior: decidePolicy with { number: 7, add: ["kind/bug"] } returns
"allow" (previously { deny: "not available in unattended Forgejo conversation sessions" }), and { number: 7, add: ["Agent/Ready"] } against
a config naming that intake label returns a deny mentioning the label.
bun test apps/server/src/forgejo/dispatcher.test.ts — guards the
BLOCKED_LABEL move in step 2.
bun run typecheck — catches the async decide signature change and any
missed BLOCKED_LABEL import.

Assumptions & contingencies

The repository config is fetched on each label call rather than cached. Label
calls are rare (at most a few per session) and .omp/forgejo.yaml is read
live everywhere else in the dispatcher so edits apply without a restart. If a
turn is observed issuing many label calls, cache per (owner, repo) for the
turn — do not introduce a process-wide cache, which would defeat the live-edit
property.
DispatchApi.readRepoFile is reachable from the policy through the existing
api: () => DispatchApi | null parameter, exactly as openPrDenial
(session-policy.ts:439-457) reaches readPull. If api() is null in
practice for conversation sessions, the fallback path in step 3 covers it and
no further work is needed.

## Problem Unattended Forgejo **conversation** sessions — both @mention turns and triage (issue-handler) turns, which are `forgejo_conversation` sessions carrying a mark in `forgejo_issue_handler_sessions` — cannot change labels. `forgejo_update_issue_labels` is absent from `CONVERSATION_TOOL_NAMES` (`apps/server/src/bridge/session-policy.ts:158-186`), so the tool is never even constructed for them, and their system prompt states `You cannot change labels, state, descriptions, branches or code`. This blocks the documented triage use case: `issue_handler.prompt` in `.omp/forgejo.yaml` is "the steering half of the triage task — what to analyse, label or assign", and the commented example in `.omp/forgejo.yaml:7-8` reads *"Apply a severity label (sev/1..sev/4) and assign the owner …"*. A repository can ask for labeling, and the session then has no tool to do it. Give every `forgejo_conversation` session the label tool, restricted to the issue or PR the session is about, with the repository's automation-steering labels off limits. ## Plan ### 1. Put the tool on the conversation surface, keep it off the watchdog In `apps/server/src/bridge/session-policy.ts`: - Append `"forgejo_update_issue_labels"` to `CONVERSATION_TOOL_NAMES` (after `"forgejo_create_issue"`, line 185). `CONVERSATION_NO_CHECKOUT_TOOL_NAMES` (line 188) filters out only `git_`-prefixed names, so the no-checkout variant picks it up automatically — intended, since labeling needs no checkout. - Extend the `WATCHDOG_TOOL_NAMES` filter (lines 292-294) with `&& name !== "forgejo_update_issue_labels"`. The CI watchdog's system prompt (line 328) says *"You cannot change code, branches, labels or issue state"* and stays true. - `IMPLEMENTER_TOOL_NAMES` and `forgejoPullRequestPolicy` keep their own lists and are not touched. Why an explicit `"allow"` is required in step 3: `forgejo_update_issue_labels` is in `FORGEJO_MUTATING_TOOLS`, so `defaultAsk` (`permission-filter.ts:55-58`) turns it into an approval prompt, and under a trigger an `ask` becomes a denial. Returning `"default"` from the policy would make the tool unusable. Returning `"allow"` skips user *allow/ask* rules but **not** user *deny* rules, which `permission-filter.ts` checks before consulting the policy. ### 2. Make `Status/Blocked` shareable `BLOCKED_LABEL = "Status/Blocked"` is currently a module-local constant in `apps/server/src/forgejo/dispatcher.ts:212`. `dispatcher.ts` imports `session-policy.ts`, so importing the constant from there would create a cycle. Move it instead: add `export const BLOCKED_LABEL = "Status/Blocked";` to `apps/server/src/forgejo/repo-config.ts` (next to `REPO_CONFIG_PATH`, line 37), delete the local constant in `dispatcher.ts:212`, and import it there from `./repo-config.ts` — a module `dispatcher.ts` already imports (`parseRepoConfig`, `sameLabel`). `repo-config.ts` imports only the `yaml` package, so `session-policy.ts` importing it introduces no cycle. `grep -n "BLOCKED_LABEL" apps/server/src` must return only `repo-config.ts`, `dispatcher.ts` and the new `session-policy.ts` use. ### 3. Protected-label check in the conversation policy In `session-policy.ts`, add the import: ```ts import { BLOCKED_LABEL, labelKey, parseRepoConfig, REPO_CONFIG_PATH, sameLabel } from "../forgejo/repo-config.ts"; ``` Add the fallback constant and the helper above `forgejoConversationPolicy`: ```ts // Namespace of the deck's own workflow labels, protected when the repository // config — the authoritative list — cannot be read. const AGENT_LABEL_PREFIX = "agent/"; ``` ```ts /** * Denial when `params` adds or removes a label that steers this repository's * automation, null when every named label is free to change. The authoritative * list is `.omp/forgejo.yaml`, read live because it changes without a restart; * when it cannot be read or parsed, `Status/Blocked` and the `agent/` * namespace stay protected and everything else is allowed. */ async function protectedLabelDenial( scope: { owner: string; repo: string }, params: unknown, api: () => DispatchApi | null, signal?: AbortSignal, ): Promise<{ deny: string } | null> ``` Behavior, in order: 1. Collect the touched names: `add` and `remove` from `params`, keeping only non-empty strings. Empty set → return null (the tool itself rejects a call with neither, `forgejo/tools/issues.ts:208-212`). 2. Read the config: `const client = api()`; when non-null, `await client.readRepoFile(scope.owner, scope.repo, REPO_CONFIG_PATH, signal)`, then `parseRepoConfig(file.data)` when `file.ok`. A 404 is **not** a failure — `readRepoFile` maps it to `{ ok: true, data: null }` and `parseRepoConfig(null)` returns `DEFAULT_REPO_CONFIG`, whose label fields are all null, so only `BLOCKED_LABEL` is protected. 3. Build the protected list. - Config available (`parsed.ok`): `BLOCKED_LABEL`, `issueHandler?.label`, `implementation.intakeLabel`, `implementation.progressLabel`, `implementation.planningLabel`, `review?.label`, `ciWatchdogOthersPrs?.label`, and every entry of `review?.excludeLabels` and `ciWatchdogOthersPrs?.excludeLabels`. Drop nulls. - Config unavailable (`api()` null, `!file.ok`, or `!parsed.ok`): just `BLOCKED_LABEL`, plus the rule that any name with `labelKey(name).startsWith(AGENT_LABEL_PREFIX)` is protected. This `agent/` rule applies **only** on this fallback path, never when the config was read. 4. Match with `sameLabel(name, protectedName)` (`repo-config.ts:413`), which folds case and `-`/`_`/space; `/` is preserved, so `Agent/Ready`, `agent ready` and `AGENT-Ready` all collide, while `agentic-ux` does not match the `agent/` prefix. 5. On the first match return `{ deny: \`the label "${name}" steers this repository's automation; propose the change in your comment instead\` }`. The tool applies `add` and `remove` in one atomic update, so one protected name denies the whole call. Then rewrite `forgejoConversationPolicy.decide` (lines 255-272) as `async decide` (the `SessionPolicy.decide` signature already permits a promise, line 122, and `forgejoImplementerPolicy` is async, line 462). Keep every existing branch in place and insert the new one after the `forgejo_create_issue` branch (line 268), before the `noCheckout` git check: ```ts if (toolName === "forgejo_update_issue_labels") { if (paramNumber(params) !== scope.number) { return { deny: `this session may only change labels on #${scope.number}` }; } return (await protectedLabelDenial(scope, params, api, signal)) ?? "allow"; } ``` The signature of `decide` gains the `api` and `signal` parameters it currently drops; `sessionId` and `cwd` keep their `_`-prefixed names (still unused). Note the deliberate asymmetry with the comment tools directly above, which allow any number in the repository: labels are state, comments are not. ### 4. System-prompt orientation In `forgejoConversationPolicy.systemContext`, replace the single bullet at line 285 with two bullets (keep every other line of the array as is): ```ts `- You can add and remove labels on #${number} with \`forgejo_update_issue_labels\`, but not on any other issue or pull request, and not labels that steer this repository's automation (its issue-handler, implementation, review and CI-watchdog labels, and \`Status/Blocked\`); those are refused with the label named.`, "- You cannot change state, descriptions, branches or code. Propose such actions in your reply. You can file unrelated problems or follow-up work as new issues with `forgejo_create_issue`.", ``` The per-turn triage task text in `conversation-detect.ts:765-767` ("Do not change labels or anything else; do not implement") is **left unchanged on purpose**: the built-in triage steering stays passive, and a repository opts into labeling by setting `issue_handler.prompt`, whose branch (lines 768-772) replaces that text entirely. Consequently `conversation-detect.ts`, its tests (`conversation-detect.test.ts:527-547`) and the `repo-config.ts:54-57` doc comment are out of scope for this change. ### 5. Tests In `apps/server/src/bridge/session-policy.test.ts`, inside `describe("decidePolicy: forgejo_conversation")` (line 42). The existing `TRIGGER` (line 22) has `scope: { owner: "o", repo: "r", number: 7, isPull: true }` and `NO_API` (line 27) returns null — reuse both. Add a stub next to `pullsApi` (line 30): ```ts /** A Forgejo whose `readRepoFile` answers with `text`, or fails with `fail`. */ function configApi(text: string | null, fail?: { status: number; message: string }): () => DispatchApi ``` Cases: - **surface** — `policyToolNames(TRIGGER)` contains `forgejo_update_issue_labels`; so does `policyToolNames` for a `noCheckout: true` conversation scope (see the existing no-checkout trigger at line 80); the `forgejo_ci_watchdog` trigger's names do **not** contain it. - **own number only** — `decidePolicy(TRIGGER, "forgejo_update_issue_labels", { number: 7, add: ["kind/bug"] }, "s", process.cwd(), configApi(null))` is `"allow"`; the same call with `number: 8` denies with `may only change labels on #7`. - **configured automation label** — `configApi("issue_handler:\n label: needs-triage\nimplementation:\n intake_label: Agent/Ready\n progress_label: Agent/In Progress\n")`: `{ number: 7, add: ["Agent/Ready"] }` and `{ number: 7, remove: ["needs triage"] }` (normalization) both deny with `steers this repository's automation`, while `{ number: 7, add: ["kind/bug"] }` is `"allow"`. - **`Status/Blocked` with a config present** — denied even though the file names no labels (`configApi(null)`). - **config unreadable** — `configApi(null, { status: 500, message: "boom" })` and `NO_API`: `add: ["kind/bug"]` is `"allow"`, while `add: ["Agent/Ready"]`, `add: ["agent/anything"]` and `remove: ["Status/Blocked"]` deny. Also assert `add: ["agentic-ux"]` is `"allow"`, pinning that the fallback matches the `agent/` namespace and not a bare `agent` prefix. - **`agent/` is fallback-only** — with `configApi("issue_handler:\n label: needs-triage\n")`, `add: ["Agent/Ready"]` is `"allow"` (the config does not list it). The cross-kind tests at lines 359, 416, 430 and 442 iterate every trigger kind including `TRIGGER`; none asserts an exhaustive tool list, so they need no change. Re-read them before editing to confirm. ### 6. Documentation of the fixed tool set In `docs/configuration.md:250`, extend the trigger tool-set sentence after the `forgejo_create_issue` clause with: > …, and `forgejo_update_issue_labels` on the triggering issue or PR only, > excluding the labels that steer the repository's automation (the > `issue_handler`, `implementation`, `review` and CI-watchdog labels from > `.omp/forgejo.yaml`, and `Status/Blocked`). Leave line 221 alone: it describes the *attended* default-ask behavior, which is unchanged — trigger sessions never reach `defaultAsk` for this tool now. ## Verification Run from the repository root. 1. `bun test apps/server/src/bridge/session-policy.test.ts` — all tests pass, including the new cases from step 5. This is the end-to-end proof of the new behavior: `decidePolicy` with `{ number: 7, add: ["kind/bug"] }` returns `"allow"` (previously `{ deny: "not available in unattended Forgejo conversation sessions" }`), and `{ number: 7, add: ["Agent/Ready"] }` against a config naming that intake label returns a deny mentioning the label. 2. `bun test apps/server/src/forgejo/dispatcher.test.ts` — guards the `BLOCKED_LABEL` move in step 2. 3. `bun run typecheck` — catches the `async decide` signature change and any missed `BLOCKED_LABEL` import. ## Assumptions & contingencies - The repository config is fetched on each label call rather than cached. Label calls are rare (at most a few per session) and `.omp/forgejo.yaml` is read live everywhere else in the dispatcher so edits apply without a restart. If a turn is observed issuing many label calls, cache per `(owner, repo)` for the turn — do not introduce a process-wide cache, which would defeat the live-edit property. - `DispatchApi.readRepoFile` is reachable from the policy through the existing `api: () => DispatchApi | null` parameter, exactly as `openPrDenial` (`session-policy.ts:439-457`) reaches `readPull`. If `api()` is null in practice for conversation sessions, the fallback path in step 3 covers it and no further work is needed.