Per-repo config: prompt fields for the issue-handler (triage) case #235

Closed
opened 2026-11-09 10:27:39 +01:00 by omp-deck · 0 comments
Collaborator

Problem

.omp/forgejo.yaml lets a repository opt into an issue handler (issue_handler.label,
issue_handler.title_pattern) that triages issues, but offers no way to steer what that
handler does or how it behaves. Two things are missing:

  1. No system_prompts key for it. system_prompts has global, conversation,
    pull_request, implementer and plan_issue (apps/server/src/forgejo/repo-config.ts:77-83).
    A triage session is started as a plain forgejo_conversation trigger
    (apps/server/src/forgejo/dispatcher.ts:1733) and promptSessionKind derives the prompt kind
    from trigger.kind alone (apps/server/src/forgejo/repo-system-prompt.ts:11-14), so a triage
    session is indistinguishable from a generic @mention conversation and silently receives
    system_prompts.conversation.

  2. No way to change what triage does. The triage task is hardcoded in
    buildIssueHandlerSeed (apps/server/src/forgejo/conversation-detect.ts:686): analyze, search
    duplicates, post one comment, "Do not change labels or anything else; do not implement." A
    repository that wants triage to apply a severity label, assign an owner, or correlate a
    monitoring alert against recent deploys cannot express it.

End state: two new config fields.

  • system_prompts.issue_handler — standing repository instructions for a triage session.
  • issue_handler.prompt — replaces the steering half of the triage task, keeping a minimal
    built-in core.

Decided behaviour

  • Stickiness. A session becomes a triage session the first time a triage turn runs on it,
    whether triage created it or it already existed (someone @mentioned the agent on the issue
    first). It stays one for the rest of its life, so later comment turns on that issue also run
    under the triage instructions.
  • Layering. A triage session receives global + issue_handler, and not conversation —
    the same one-block-per-kind rule every other kind follows. Text that should apply during triage
    too belongs in global.
  • Task split. When issue_handler.prompt is set it replaces the built-in steering entirely.
    The surviving core: always finish by posting one comment on the issue, and never implement.
    Label and assignee changes become available to the custom prompt.
  • One prompt for both routes. label and title_pattern triage share issue_handler.prompt.
  • Timing. The triage turn itself must carry the triage instructions, otherwise the field is
    pointless. Repository instructions are baked in when a session is created or resumed, so when a
    triage trigger lands on a live session that is not yet marked, the dispatcher lets the running
    turn finish, marks the session, recycles it (dispose + ensureSessionLive, which replays the
    persisted JSONL and loses no conversation history), and only then dispatches the triage turn.

Plan

Steps are ordered so the tree builds after each. Steps 1–3 are pure additions plus one signature
change; step 4 wires them into session build; steps 5–6 are the dispatcher and seed.

1. Config surface — apps/server/src/forgejo/repo-config.ts

Add prompt to the issue-handler section and issue_handler to system_prompts.

  • RepoConfig["issueHandler"] becomes
    { label: string | null; titlePattern: RegExp | null; prompt: string | null } | null.
    Doc the new member: the steering half of the triage task; null = the built-in steering.

  • RepoConfig["systemPrompts"] gains issueHandler: string | null.

  • DEFAULT_REPO_CONFIG.systemPrompts gains issueHandler: null (line ~99).

  • In parseIssueHandler (line 249), before the existing
    if (label === null && titlePattern === null) check, parse the new key with the same shape the
    other string keys use:

    let prompt: string | null = null;
    if (handler.prompt !== undefined && handler.prompt !== null) {
    	if (typeof handler.prompt !== "string" || !handler.prompt.trim()) {
    		return { ok: false, error: "`issue_handler.prompt` must be a non-empty string" };
    	}
    	prompt = handler.prompt.trim();
    }
    

    Return { ok: true, value: { label, titlePattern, prompt } }. Leave the
    "issue_handlerneedslabelortitle_pattern" check exactly as it is — it already rejects
    a section containing only prompt, which is the correct outcome.

  • In parseSystemPrompts (line 212) add issue_handler: "issueHandler" to the keys map. The
    existing loop then validates and trims it like the rest.

  • Extend PromptSessionKind (line 232) with "issue_handler" and add
    issue_handler: "issueHandler" to PROMPT_FIELD (line 234). repoSystemPrompt needs no change:
    it already emits global + the one kind block, which is the decided layering.

  • Update the module doc (lines 1-29): the system_prompts sentence must list issue_handler
    among the per-kind keys, and the issue_handler sentence must mention that prompt replaces the
    triage task's steering while the agent still always comments and never implements.

This changes the shape of two object literals used in repo-config.test.ts (lines 29, 220, 231
build { label, titlePattern }; line 287 asserts the full systemPrompts object). Update them to
include prompt: null / issueHandler: null; they are shape assertions, not behaviour pins.

2. Persisted marker — new migration + db module

The trigger row cannot carry this: insertSessionTrigger is insert-only
(apps/server/src/db/session-triggers.ts:21-25) and a triage trigger reaching an already-mapped
session never rebuilds it. Mirror the plan_issue_sessions binding instead.

  • New apps/server/src/db/migrations/024-forgejo-issue-handler-sessions.sql (the runner
    auto-discovers numbered .sql files, apps/server/src/db/index.ts:80-82):

    -- 024-forgejo-issue-handler-sessions.sql
    --
    -- Marks a Forgejo conversation session as an issue-handler (triage) session.
    -- The mark is set the first time a triage turn runs on the session and is
    -- never cleared: it selects `system_prompts.issue_handler` over
    -- `system_prompts.conversation` for every later turn of that session.
    
    CREATE TABLE IF NOT EXISTS forgejo_issue_handler_sessions (
    	session_id TEXT PRIMARY KEY,
    	created_at TEXT NOT NULL
    );
    
  • New apps/server/src/db/issue-handler-sessions.ts, modelled on
    apps/server/src/db/plan-issue-sessions.ts (same getDb/nowIso imports, same doc style):

    export function markIssueHandlerSession(sessionId: string): void
    export function isIssueHandlerSession(sessionId: string): boolean
    export function deleteIssueHandlerSession(sessionId: string): void
    

    mark uses INSERT OR IGNORE so re-marking an already-marked session is a no-op (unlike the
    plan-issue insert, this is called on every triage turn). is is a
    SELECT 1 FROM forgejo_issue_handler_sessions WHERE session_id = ? existence check.

  • Call deleteIssueHandlerSession(id) in the session-delete route next to
    deletePlanIssueBinding(id) (apps/server/src/routes.ts:216), so a deleted session leaves no
    orphan row.

3. Prompt-kind resolution — apps/server/src/forgejo/repo-system-prompt.ts

promptSessionKind gains a third input:

export function promptSessionKind(
	triggerKind: string | null,
	planIssue: boolean,
	issueHandler: boolean,
): PromptSessionKind | null

Body: return "issue_handler" first when issueHandler is true (it is only ever set on
conversation sessions, and triage-ness outranks the generic conversation kind); otherwise keep the
existing logic unchanged. Update the doc comment to name the new kind and say the mark wins over
the trigger kind.

Both call sites are in apps/server/src/bridge/in-process.ts (lines 303 and 451) and are updated
in step 4.

4. Session build — apps/server/src/bridge/in-process.ts

Create path (createSession, around lines 300-378):

  • Add issueHandler?: true to CreateSessionOpts in apps/server/src/bridge/types.ts, documented
    as: the session is started by the repository's issue handler; only valid together with a
    forgejo_conversation trigger.
  • Next to the existing guard at line 301, reject the impossible combination:
    if (opts.issueHandler && opts.trigger?.kind !== "forgejo_conversation") throw new Error("issueHandler requires a forgejo_conversation trigger");
  • Line 303 becomes
    promptSessionKind(opts.trigger?.kind ?? null, planIssue !== null, opts.issueHandler === true).
  • After insertSessionTrigger succeeds (inside the same if (trigger) block, lines 362-369), call
    markIssueHandlerSession(handle.sessionId) when opts.issueHandler is set, inside the same
    try so a failure takes the same deleteSession + rethrow path. The mark must exist before the
    first turn so a later resume keeps the kind.

Resume path (resumeSession, around lines 404-452):

  • Alongside const planIssue = existingId ? getPlanIssueBinding(existingId) : null; (line 405) add
    const issueHandler = existingId ? isIssueHandlerSession(existingId) : false;
  • Line 451 becomes promptSessionKind(trigger?.kind ?? null, planIssue !== null, issueHandler).

Nothing else in the prompt assembly changes: repoPrompt already flows into the systemPrompt
callback at lines 329 and 471.

5. Dispatcher — mark, recycle, and carry the prompt

apps/server/src/forgejo/dispatcher.ts.

Carry the custom prompt on Ready. Add to the Ready interface (line 359):

/** `issue_handler.prompt` for an "issue-labeled" or "issue-title" trigger; null otherwise. */
issueHandlerPrompt: string | null;

Populate it where the reason is decided (lines 590-602), reusing the config each branch already
reads — do not re-read .omp/forgejo.yaml, which is what implementation on Ready exists to
avoid:

  • Change resolveTitleTrigger (line 858) to return Promise<RepoConfig["issueHandler"] | null>:
    null on every path that already finishes the row, and the non-null handler on the success
    path. Its only caller is line 591; rewrite it as
    const handler = await this.resolveTitleTrigger(job, payload); if (handler === null) return null; issueHandlerPrompt = handler.prompt; reason = "issue-title";
    and update the doc comment.
  • In the label branch's else (line 600-602) set
    issueHandlerPrompt = resolved.config.issueHandler?.prompt ?? null; — resolveLabelTrigger
    already returns config.
  • Declare let issueHandlerPrompt: string | null = null; next to let label (line 552) and add
    issueHandlerPrompt to the returned Ready object (line 736).

Mark a newly created triage session. In resolveSession the createSession call (line 1753)
gains issueHandler: true when ready.reason === "issue-labeled" || ready.reason === "issue-title"
— pass it only for the conversation branches; the prHead !== null implementer branch can never be
reached by a triage reason, so a plain conditional spread is enough:

...(isTriage(ready.reason) ? { issueHandler: true as const } : {}),

Add a module-level function isTriage(reason: ReadyReason): boolean next to continueOnly
(line 2746) returning reason === "issue-labeled" || reason === "issue-title", and use it
everywhere this plan tests for triage.

Mark and recycle a reused session. Add a private method and call it in runReady immediately
after resolveSession (line 1067), before getPreviousSessionTurn — the recycled handle must be
the one the rest of the turn uses:

/**
 * Make `resolved` a triage session before its triage turn runs. A session
 * created for this trigger is already marked; an existing one is marked here
 * and then recycled — its current turn is awaited, its handle disposed and
 * resumed from the JSONL — so the rebuilt system prompt carries
 * `system_prompts.issue_handler` for this turn, not only the next one.
 */
private async ensureIssueHandlerSession(ready: Ready, resolved: ResolvedSession): Promise<ResolvedSession>

Body:

  1. Return resolved unchanged when !isTriage(ready.reason) or resolved.created (the create
    path already marked it) or isIssueHandlerSession(resolved.handle.sessionId) (already a triage
    session; its prompt is correct).
  2. markIssueHandlerSession(sessionId).
  3. await this.waitUntilIdle(sessionId, this.deps.turnTimeoutMs ?? SESSION_POLICIES[resolved.kind].turnTimeoutMs)
    — the existing helper at line 2037; it returns immediately when nothing is streaming.
  4. await resolved.handle.dispose(), then const live = await this.deps.bridge.ensureSessionLive(sessionId).
  5. On live.ok, return { ...resolved, handle: live.handle }. Keep broughtLive as it was: it
    drives the end-of-turn disposal at line 1180, and the session's liveness ownership has not
    changed.
  6. On !live.ok, log a warning (log.warn) naming the session and live.code, and return
    resolved unchanged. The mark is already persisted, so the instructions apply from the
    session's next wake; failing the whole trigger over a prompt refresh would be worse than running
    the turn with the old prompt.

Dispose errors follow the existing convention at line 1895: .catch + log.warn, never fatal — if
dispose fails, skip the resume and return resolved unchanged.

6. Triage task seed — apps/server/src/forgejo/conversation-detect.ts

buildIssueHandlerSeed (line 665) gains a prompt: string | null field in its parameter object.
Replace the single hardcoded task line (line 686) with:

  • prompt === null — byte-identical to today, so an unconfigured repository sees no change:

    Task: analyze this issue. Search for similar or duplicate issues with forgejo_search_issues, then post one comment on #N with your analysis: related issues (linked as #N), clarifying questions, and initial feedback. Do not change labels or anything else; do not implement.
    
  • prompt set — the custom steering, then the invariant core as its own line:

    Task: <prompt>
    Whatever else you do, finish by posting one comment on #N with forgejo_create_issue_comment. Do not implement: write no code, create no branch, open no pull request.
    

    The core comes last so it is not buried by a long custom prompt. Both lines join into the same
    [...].join("\n") array the function already builds; everything else in the seed (title, labels,
    trigger line, orientation lines, description fence) is unchanged.

Update the function's doc comment to say prompt replaces the built-in steering and that the
comment-and-never-implement core always applies.

Both call sites are in runReady (lines 1119-1128); pass prompt: ready.issueHandlerPrompt in
each.

7. Commented-out examples in this repository's own config — .omp/forgejo.yaml

Every optional section of the file is documented in place as a commented-out block showing a
realistic value and the default (system_prompts at lines 18-30, compaction at lines 32-36).
The two new keys must follow that convention, or they are undiscoverable: nothing else in the
repository documents this schema except the module doc in repo-config.ts.

  • Under the existing live issue_handler: block (lines 1-2), append the new key as a comment with
    a short explanation of the core that survives:

    issue_handler:
      label: "agent/triage"
      # Replaces the steering of the triage task. The agent still always finishes
      # by posting one comment on the issue and never implements. Unset = analyze
      # the issue, search for duplicates, comment with the analysis, change nothing.
      # prompt: |
      #   Apply a severity label (sev/1..sev/4) and assign the owner of the
      #   affected component. Name the component in your comment.
    
  • In the commented system_prompts block, add issue_handler between conversation and
    pull_request (matching the order the keys are declared in RepoConfig["systemPrompts"]), and
    extend the block's header comment to say that a session which has triaged an issue uses
    issue_handler instead of conversation for the rest of its life:

    #   issue_handler: |
    #     State the affected component and the blast radius before anything else.
    

These are comments only: the live config of this repository keeps its current behaviour, and
parseRepoConfig never sees the new keys. Verified by step 4 of Verification — the file is parsed
by the dispatcher tests' fixtures, not by the repository itself, so a typo here cannot break a
test; read the block back after editing to confirm the YAML comment markers are intact.

Critical files & anchors

File Where Why
apps/server/src/forgejo/repo-config.ts parseIssueHandler :249, parseSystemPrompts :212, PROMPT_FIELD :234, module doc :1-29 Three separate places must learn the new keys; the module doc is the only user-facing documentation of this file's schema.
apps/server/src/bridge/in-process.ts createSession :300-378, resumeSession :404-452 The prompt is assembled from a value captured before createAgentSession; both paths must resolve the kind identically or the prompt flips between wake-ups.
apps/server/src/forgejo/dispatcher.ts gate :551-602 and :736, runReady :1067, resolveSession :1753, waitUntilIdle :2037 The mark-and-recycle must happen between session resolution and the turn; waitUntilIdle and ensureSessionLive already exist and must be reused rather than re-implemented.
apps/server/src/forgejo/conversation-detect.ts buildIssueHandlerSeed :665-693 The only place the triage task text exists.
apps/server/src/db/plan-issue-sessions.ts + migrations/017-plan-issue-sessions.sql whole files The exact template for the new marker module and migration — match their doc style and query shape.
.omp/forgejo.yaml commented system_prompts block :18-30, compaction :32-36 The in-file documentation convention the new keys must match: commented block, realistic value, default named.

Verification

Run from the repository root.

  1. Config parsing. Extend apps/server/src/forgejo/repo-config.test.ts:

    • parseRepoConfig("issue_handler:\n label: needs-triage\n prompt: Apply a severity label.\n")
      → config.issueHandler equals
      { label: "needs-triage", titlePattern: null, prompt: "Apply a severity label." }.
    • parseRepoConfig("issue_handler:\n label: needs-triage\n prompt: ' '\n") → ok: false
      with error "`issue_handler.prompt` must be a non-empty string".
    • parseRepoConfig("issue_handler:\n prompt: x\n") → ok: false with the existing
      "`issue_handler` needs `label` or `title_pattern`".
    • parseRepoConfig("system_prompts:\n issue_handler: t\n") → config.systemPrompts.issueHandler === "t".
    • repoSystemPrompt(config, "issue_handler") with global: "g", conversation: "c",
      issueHandler: "t" → contains "g" and "t", and not "c".
    • promptSessionKind("forgejo_conversation", false, true) === "issue_handler" and
      promptSessionKind("forgejo_conversation", false, false) === "forgejo_conversation".

    bun test apps/server/src/forgejo/repo-config.test.ts

  2. Seed composition. New test in apps/server/src/forgejo/conversation-detect.test.ts:
    buildIssueHandlerSeed({ ..., prompt: "Apply a severity label and assign an owner." }) contains
    that sentence, contains forgejo_create_issue_comment, contains Do not implement, and does
    not contain Search for similar or duplicate issues. With prompt: null it contains
    Search for similar or duplicate issues and Do not change labels.

    bun test apps/server/src/forgejo/conversation-detect.test.ts

  3. End-to-end through the dispatcher — this is the check that proves the feature, using the
    existing harness in apps/server/src/forgejo/dispatcher.test.ts (see TRIAGE_CONFIG at :608 and
    the label tests from :611):

    • Custom task reaches the turn. h.api.repoFile returns
      "issue_handler:\n label: needs-triage\n prompt: Apply a severity label.\n"; enqueue a label
      trigger; assert h.sessions[0].handle.prompts[0] contains Apply a severity label. and
      forgejo_create_issue_comment, and does not contain Search for similar or duplicate issues.
    • New triage session is marked. Same setup; after whenIdle, assert
      isIssueHandlerSession(h.sessions[0].handle.sessionId) is true, and that
      h.sessions[0].opts.issueHandler is true.
    • Existing session gets marked and recycled. Pre-insert a forgejo_conversation trigger and
      map a live session to the issue (the pattern at :1733 and :2304), then enqueue the label
      trigger. Assert the session id is unchanged (no new session created), that
      isIssueHandlerSession(id) is now true, and that ensureSessionLive was called for it after
      the dispose — i.e. the turn ran on a handle obtained after the recycle.
    • Unconfigured repositories are untouched. The existing test at :611-626 (TRIAGE_CONFIG
      without prompt) must still pass with its 'added the label "needs-triage"' assertion and the
      original task text.

    bun test apps/server/src/forgejo/dispatcher.test.ts

  4. Full server suite and typecheck, which also catches the promptSessionKind signature change
    and the issueHandler object-shape updates across existing tests:

    bun test apps/server and the repository's typecheck script.

  5. Migration applies. Boot the server once against an existing database and confirm
    024-forgejo-issue-handler-sessions.sql is recorded in schema_migrations and the table exists.
    An IF NOT EXISTS table with no data dependency means a re-run is safe.

Assumptions & contingencies

  • ensureSessionLive after dispose returns a handle with the rebuilt prompt. This follows
    from resumeSession re-resolving repoPrompt at line 451, and ensureSessionLive resuming from
    the persisted JSONL (apps/server/src/bridge/types.ts:42-48). If the bridge instead hands back a
    cached handle for a session disposed moments earlier, drop the recycle: keep the mark, log that
    the instructions apply from the next wake, and leave the turn running on the existing handle —
    the custom task text still reaches it, which is the larger half of the feature.
  • A triage reason never reaches the implementer branch of resolveSession. prHead is only set
    for pull requests (dispatcher.ts:679-713) and both triage reasons are issue-only. If a triage
    reason is ever observed with prHead !== null, do not mark the session — an implementer session
    must keep system_prompts.implementer.
  • Orphan marker rows. Sessions deleted outside the DELETE /sessions/:id route leave a row
    behind, exactly as plan_issue_sessions does today. Harmless: the row is only ever read by
    session id. No cleanup job.
## Problem `.omp/forgejo.yaml` lets a repository opt into an issue handler (`issue_handler.label`, `issue_handler.title_pattern`) that triages issues, but offers no way to steer what that handler does or how it behaves. Two things are missing: 1. **No `system_prompts` key for it.** `system_prompts` has `global`, `conversation`, `pull_request`, `implementer` and `plan_issue` (`apps/server/src/forgejo/repo-config.ts:77-83`). A triage session is started as a plain `forgejo_conversation` trigger (`apps/server/src/forgejo/dispatcher.ts:1733`) and `promptSessionKind` derives the prompt kind from `trigger.kind` alone (`apps/server/src/forgejo/repo-system-prompt.ts:11-14`), so a triage session is indistinguishable from a generic @mention conversation and silently receives `system_prompts.conversation`. 2. **No way to change what triage does.** The triage task is hardcoded in `buildIssueHandlerSeed` (`apps/server/src/forgejo/conversation-detect.ts:686`): analyze, search duplicates, post one comment, *"Do not change labels or anything else; do not implement."* A repository that wants triage to apply a severity label, assign an owner, or correlate a monitoring alert against recent deploys cannot express it. End state: two new config fields. - `system_prompts.issue_handler` — standing repository instructions for a triage session. - `issue_handler.prompt` — replaces the *steering* half of the triage task, keeping a minimal built-in core. ### Decided behaviour - **Stickiness.** A session becomes a triage session the first time a triage turn runs on it, whether triage created it or it already existed (someone @mentioned the agent on the issue first). It stays one for the rest of its life, so later comment turns on that issue also run under the triage instructions. - **Layering.** A triage session receives `global` + `issue_handler`, and *not* `conversation` — the same one-block-per-kind rule every other kind follows. Text that should apply during triage too belongs in `global`. - **Task split.** When `issue_handler.prompt` is set it replaces the built-in steering entirely. The surviving core: always finish by posting one comment on the issue, and never implement. Label and assignee changes become available to the custom prompt. - **One prompt for both routes.** `label` and `title_pattern` triage share `issue_handler.prompt`. - **Timing.** The triage turn itself must carry the triage instructions, otherwise the field is pointless. Repository instructions are baked in when a session is created or resumed, so when a triage trigger lands on a live session that is not yet marked, the dispatcher lets the running turn finish, marks the session, recycles it (dispose + `ensureSessionLive`, which replays the persisted JSONL and loses no conversation history), and only then dispatches the triage turn. ## Plan Steps are ordered so the tree builds after each. Steps 1–3 are pure additions plus one signature change; step 4 wires them into session build; steps 5–6 are the dispatcher and seed. ### 1. Config surface — `apps/server/src/forgejo/repo-config.ts` Add `prompt` to the issue-handler section and `issue_handler` to `system_prompts`. - `RepoConfig["issueHandler"]` becomes `{ label: string | null; titlePattern: RegExp | null; prompt: string | null } | null`. Doc the new member: the steering half of the triage task; null = the built-in steering. - `RepoConfig["systemPrompts"]` gains `issueHandler: string | null`. - `DEFAULT_REPO_CONFIG.systemPrompts` gains `issueHandler: null` (line ~99). - In `parseIssueHandler` (line 249), before the existing `if (label === null && titlePattern === null)` check, parse the new key with the same shape the other string keys use: ```ts let prompt: string | null = null; if (handler.prompt !== undefined && handler.prompt !== null) { if (typeof handler.prompt !== "string" || !handler.prompt.trim()) { return { ok: false, error: "`issue_handler.prompt` must be a non-empty string" }; } prompt = handler.prompt.trim(); } ``` Return `{ ok: true, value: { label, titlePattern, prompt } }`. Leave the `"`issue_handler` needs `label` or `title_pattern`"` check exactly as it is — it already rejects a section containing only `prompt`, which is the correct outcome. - In `parseSystemPrompts` (line 212) add `issue_handler: "issueHandler"` to the `keys` map. The existing loop then validates and trims it like the rest. - Extend `PromptSessionKind` (line 232) with `"issue_handler"` and add `issue_handler: "issueHandler"` to `PROMPT_FIELD` (line 234). `repoSystemPrompt` needs no change: it already emits `global` + the one kind block, which is the decided layering. - Update the module doc (lines 1-29): the `system_prompts` sentence must list `issue_handler` among the per-kind keys, and the `issue_handler` sentence must mention that `prompt` replaces the triage task's steering while the agent still always comments and never implements. This changes the shape of two object literals used in `repo-config.test.ts` (lines 29, 220, 231 build `{ label, titlePattern }`; line 287 asserts the full `systemPrompts` object). Update them to include `prompt: null` / `issueHandler: null`; they are shape assertions, not behaviour pins. ### 2. Persisted marker — new migration + db module The trigger row cannot carry this: `insertSessionTrigger` is insert-only (`apps/server/src/db/session-triggers.ts:21-25`) and a triage trigger reaching an already-mapped session never rebuilds it. Mirror the `plan_issue_sessions` binding instead. - New `apps/server/src/db/migrations/024-forgejo-issue-handler-sessions.sql` (the runner auto-discovers numbered `.sql` files, `apps/server/src/db/index.ts:80-82`): ```sql -- 024-forgejo-issue-handler-sessions.sql -- -- Marks a Forgejo conversation session as an issue-handler (triage) session. -- The mark is set the first time a triage turn runs on the session and is -- never cleared: it selects `system_prompts.issue_handler` over -- `system_prompts.conversation` for every later turn of that session. CREATE TABLE IF NOT EXISTS forgejo_issue_handler_sessions ( session_id TEXT PRIMARY KEY, created_at TEXT NOT NULL ); ``` - New `apps/server/src/db/issue-handler-sessions.ts`, modelled on `apps/server/src/db/plan-issue-sessions.ts` (same `getDb`/`nowIso` imports, same doc style): ```ts export function markIssueHandlerSession(sessionId: string): void export function isIssueHandlerSession(sessionId: string): boolean export function deleteIssueHandlerSession(sessionId: string): void ``` `mark` uses `INSERT OR IGNORE` so re-marking an already-marked session is a no-op (unlike the plan-issue insert, this is called on every triage turn). `is` is a `SELECT 1 FROM forgejo_issue_handler_sessions WHERE session_id = ?` existence check. - Call `deleteIssueHandlerSession(id)` in the session-delete route next to `deletePlanIssueBinding(id)` (`apps/server/src/routes.ts:216`), so a deleted session leaves no orphan row. ### 3. Prompt-kind resolution — `apps/server/src/forgejo/repo-system-prompt.ts` `promptSessionKind` gains a third input: ```ts export function promptSessionKind( triggerKind: string | null, planIssue: boolean, issueHandler: boolean, ): PromptSessionKind | null ``` Body: return `"issue_handler"` first when `issueHandler` is true (it is only ever set on conversation sessions, and triage-ness outranks the generic conversation kind); otherwise keep the existing logic unchanged. Update the doc comment to name the new kind and say the mark wins over the trigger kind. Both call sites are in `apps/server/src/bridge/in-process.ts` (lines 303 and 451) and are updated in step 4. ### 4. Session build — `apps/server/src/bridge/in-process.ts` Create path (`createSession`, around lines 300-378): - Add `issueHandler?: true` to `CreateSessionOpts` in `apps/server/src/bridge/types.ts`, documented as: the session is started by the repository's issue handler; only valid together with a `forgejo_conversation` trigger. - Next to the existing guard at line 301, reject the impossible combination: `if (opts.issueHandler && opts.trigger?.kind !== "forgejo_conversation") throw new Error("issueHandler requires a forgejo_conversation trigger");` - Line 303 becomes `promptSessionKind(opts.trigger?.kind ?? null, planIssue !== null, opts.issueHandler === true)`. - After `insertSessionTrigger` succeeds (inside the same `if (trigger)` block, lines 362-369), call `markIssueHandlerSession(handle.sessionId)` when `opts.issueHandler` is set, inside the same `try` so a failure takes the same `deleteSession` + rethrow path. The mark must exist before the first turn so a later resume keeps the kind. Resume path (`resumeSession`, around lines 404-452): - Alongside `const planIssue = existingId ? getPlanIssueBinding(existingId) : null;` (line 405) add `const issueHandler = existingId ? isIssueHandlerSession(existingId) : false;` - Line 451 becomes `promptSessionKind(trigger?.kind ?? null, planIssue !== null, issueHandler)`. Nothing else in the prompt assembly changes: `repoPrompt` already flows into the `systemPrompt` callback at lines 329 and 471. ### 5. Dispatcher — mark, recycle, and carry the prompt `apps/server/src/forgejo/dispatcher.ts`. **Carry the custom prompt on `Ready`.** Add to the `Ready` interface (line 359): ```ts /** `issue_handler.prompt` for an "issue-labeled" or "issue-title" trigger; null otherwise. */ issueHandlerPrompt: string | null; ``` Populate it where the reason is decided (lines 590-602), reusing the config each branch already reads — do not re-read `.omp/forgejo.yaml`, which is what `implementation` on `Ready` exists to avoid: - Change `resolveTitleTrigger` (line 858) to return `Promise<RepoConfig["issueHandler"] | null>`: `null` on every path that already finishes the row, and the non-null `handler` on the success path. Its only caller is line 591; rewrite it as `const handler = await this.resolveTitleTrigger(job, payload); if (handler === null) return null; issueHandlerPrompt = handler.prompt; reason = "issue-title";` and update the doc comment. - In the label branch's `else` (line 600-602) set `issueHandlerPrompt = resolved.config.issueHandler?.prompt ?? null;` — `resolveLabelTrigger` already returns `config`. - Declare `let issueHandlerPrompt: string | null = null;` next to `let label` (line 552) and add `issueHandlerPrompt` to the returned `Ready` object (line 736). **Mark a newly created triage session.** In `resolveSession` the `createSession` call (line 1753) gains `issueHandler: true` when `ready.reason === "issue-labeled" || ready.reason === "issue-title"` — pass it only for the conversation branches; the `prHead !== null` implementer branch can never be reached by a triage reason, so a plain conditional spread is enough: ```ts ...(isTriage(ready.reason) ? { issueHandler: true as const } : {}), ``` Add a module-level `function isTriage(reason: ReadyReason): boolean` next to `continueOnly` (line 2746) returning `reason === "issue-labeled" || reason === "issue-title"`, and use it everywhere this plan tests for triage. **Mark and recycle a reused session.** Add a private method and call it in `runReady` immediately after `resolveSession` (line 1067), before `getPreviousSessionTurn` — the recycled handle must be the one the rest of the turn uses: ```ts /** * Make `resolved` a triage session before its triage turn runs. A session * created for this trigger is already marked; an existing one is marked here * and then recycled — its current turn is awaited, its handle disposed and * resumed from the JSONL — so the rebuilt system prompt carries * `system_prompts.issue_handler` for this turn, not only the next one. */ private async ensureIssueHandlerSession(ready: Ready, resolved: ResolvedSession): Promise<ResolvedSession> ``` Body: 1. Return `resolved` unchanged when `!isTriage(ready.reason)` or `resolved.created` (the create path already marked it) or `isIssueHandlerSession(resolved.handle.sessionId)` (already a triage session; its prompt is correct). 2. `markIssueHandlerSession(sessionId)`. 3. `await this.waitUntilIdle(sessionId, this.deps.turnTimeoutMs ?? SESSION_POLICIES[resolved.kind].turnTimeoutMs)` — the existing helper at line 2037; it returns immediately when nothing is streaming. 4. `await resolved.handle.dispose()`, then `const live = await this.deps.bridge.ensureSessionLive(sessionId)`. 5. On `live.ok`, return `{ ...resolved, handle: live.handle }`. Keep `broughtLive` as it was: it drives the end-of-turn disposal at line 1180, and the session's liveness ownership has not changed. 6. On `!live.ok`, log a warning (`log.warn`) naming the session and `live.code`, and return `resolved` unchanged. The mark is already persisted, so the instructions apply from the session's next wake; failing the whole trigger over a prompt refresh would be worse than running the turn with the old prompt. Dispose errors follow the existing convention at line 1895: `.catch` + `log.warn`, never fatal — if dispose fails, skip the resume and return `resolved` unchanged. ### 6. Triage task seed — `apps/server/src/forgejo/conversation-detect.ts` `buildIssueHandlerSeed` (line 665) gains a `prompt: string | null` field in its parameter object. Replace the single hardcoded task line (line 686) with: - `prompt === null` — byte-identical to today, so an unconfigured repository sees no change: ``` Task: analyze this issue. Search for similar or duplicate issues with forgejo_search_issues, then post one comment on #N with your analysis: related issues (linked as #N), clarifying questions, and initial feedback. Do not change labels or anything else; do not implement. ``` - `prompt` set — the custom steering, then the invariant core as its own line: ``` Task: <prompt> Whatever else you do, finish by posting one comment on #N with forgejo_create_issue_comment. Do not implement: write no code, create no branch, open no pull request. ``` The core comes last so it is not buried by a long custom prompt. Both lines join into the same `[...].join("\n")` array the function already builds; everything else in the seed (title, labels, trigger line, orientation lines, description fence) is unchanged. Update the function's doc comment to say `prompt` replaces the built-in steering and that the comment-and-never-implement core always applies. Both call sites are in `runReady` (lines 1119-1128); pass `prompt: ready.issueHandlerPrompt` in each. ### 7. Commented-out examples in this repository's own config — `.omp/forgejo.yaml` Every optional section of the file is documented in place as a commented-out block showing a realistic value and the default (`system_prompts` at lines 18-30, `compaction` at lines 32-36). The two new keys must follow that convention, or they are undiscoverable: nothing else in the repository documents this schema except the module doc in `repo-config.ts`. - Under the existing live `issue_handler:` block (lines 1-2), append the new key as a comment with a short explanation of the core that survives: ```yaml issue_handler: label: "agent/triage" # Replaces the steering of the triage task. The agent still always finishes # by posting one comment on the issue and never implements. Unset = analyze # the issue, search for duplicates, comment with the analysis, change nothing. # prompt: | # Apply a severity label (sev/1..sev/4) and assign the owner of the # affected component. Name the component in your comment. ``` - In the commented `system_prompts` block, add `issue_handler` between `conversation` and `pull_request` (matching the order the keys are declared in `RepoConfig["systemPrompts"]`), and extend the block's header comment to say that a session which has triaged an issue uses `issue_handler` instead of `conversation` for the rest of its life: ```yaml # issue_handler: | # State the affected component and the blast radius before anything else. ``` These are comments only: the live config of this repository keeps its current behaviour, and `parseRepoConfig` never sees the new keys. Verified by step 4 of Verification — the file is parsed by the dispatcher tests' fixtures, not by the repository itself, so a typo here cannot break a test; read the block back after editing to confirm the YAML comment markers are intact. ### Critical files & anchors | File | Where | Why | | --- | --- | --- | | `apps/server/src/forgejo/repo-config.ts` | `parseIssueHandler` :249, `parseSystemPrompts` :212, `PROMPT_FIELD` :234, module doc :1-29 | Three separate places must learn the new keys; the module doc is the only user-facing documentation of this file's schema. | | `apps/server/src/bridge/in-process.ts` | `createSession` :300-378, `resumeSession` :404-452 | The prompt is assembled from a value captured before `createAgentSession`; both paths must resolve the kind identically or the prompt flips between wake-ups. | | `apps/server/src/forgejo/dispatcher.ts` | `gate` :551-602 and :736, `runReady` :1067, `resolveSession` :1753, `waitUntilIdle` :2037 | The mark-and-recycle must happen between session resolution and the turn; `waitUntilIdle` and `ensureSessionLive` already exist and must be reused rather than re-implemented. | | `apps/server/src/forgejo/conversation-detect.ts` | `buildIssueHandlerSeed` :665-693 | The only place the triage task text exists. | | `apps/server/src/db/plan-issue-sessions.ts` + `migrations/017-plan-issue-sessions.sql` | whole files | The exact template for the new marker module and migration — match their doc style and query shape. | | `.omp/forgejo.yaml` | commented `system_prompts` block :18-30, `compaction` :32-36 | The in-file documentation convention the new keys must match: commented block, realistic value, default named. | ### Verification Run from the repository root. 1. **Config parsing.** Extend `apps/server/src/forgejo/repo-config.test.ts`: - `parseRepoConfig("issue_handler:\n label: needs-triage\n prompt: Apply a severity label.\n")` → `config.issueHandler` equals `{ label: "needs-triage", titlePattern: null, prompt: "Apply a severity label." }`. - `parseRepoConfig("issue_handler:\n label: needs-triage\n prompt: ' '\n")` → `ok: false` with error ``"`issue_handler.prompt` must be a non-empty string"``. - `parseRepoConfig("issue_handler:\n prompt: x\n")` → `ok: false` with the existing ``"`issue_handler` needs `label` or `title_pattern`"``. - `parseRepoConfig("system_prompts:\n issue_handler: t\n")` → `config.systemPrompts.issueHandler === "t"`. - `repoSystemPrompt(config, "issue_handler")` with `global: "g"`, `conversation: "c"`, `issueHandler: "t"` → contains `"g"` and `"t"`, and **not** `"c"`. - `promptSessionKind("forgejo_conversation", false, true) === "issue_handler"` and `promptSessionKind("forgejo_conversation", false, false) === "forgejo_conversation"`. `bun test apps/server/src/forgejo/repo-config.test.ts` 2. **Seed composition.** New test in `apps/server/src/forgejo/conversation-detect.test.ts`: `buildIssueHandlerSeed({ ..., prompt: "Apply a severity label and assign an owner." })` contains that sentence, contains `forgejo_create_issue_comment`, contains `Do not implement`, and does **not** contain `Search for similar or duplicate issues`. With `prompt: null` it contains `Search for similar or duplicate issues` and `Do not change labels`. `bun test apps/server/src/forgejo/conversation-detect.test.ts` 3. **End-to-end through the dispatcher** — this is the check that proves the feature, using the existing harness in `apps/server/src/forgejo/dispatcher.test.ts` (see `TRIAGE_CONFIG` at :608 and the label tests from :611): - *Custom task reaches the turn.* `h.api.repoFile` returns `"issue_handler:\n label: needs-triage\n prompt: Apply a severity label.\n"`; enqueue a label trigger; assert `h.sessions[0].handle.prompts[0]` contains `Apply a severity label.` and `forgejo_create_issue_comment`, and does not contain `Search for similar or duplicate issues`. - *New triage session is marked.* Same setup; after `whenIdle`, assert `isIssueHandlerSession(h.sessions[0].handle.sessionId)` is true, and that `h.sessions[0].opts.issueHandler` is `true`. - *Existing session gets marked and recycled.* Pre-insert a `forgejo_conversation` trigger and map a live session to the issue (the pattern at :1733 and :2304), then enqueue the label trigger. Assert the session id is unchanged (no new session created), that `isIssueHandlerSession(id)` is now true, and that `ensureSessionLive` was called for it after the dispose — i.e. the turn ran on a handle obtained after the recycle. - *Unconfigured repositories are untouched.* The existing test at :611-626 (`TRIAGE_CONFIG` without `prompt`) must still pass with its `'added the label "needs-triage"'` assertion and the original task text. `bun test apps/server/src/forgejo/dispatcher.test.ts` 4. **Full server suite and typecheck**, which also catches the `promptSessionKind` signature change and the `issueHandler` object-shape updates across existing tests: `bun test apps/server` and the repository's typecheck script. 5. **Migration applies.** Boot the server once against an existing database and confirm `024-forgejo-issue-handler-sessions.sql` is recorded in `schema_migrations` and the table exists. An `IF NOT EXISTS` table with no data dependency means a re-run is safe. ### Assumptions & contingencies - **`ensureSessionLive` after `dispose` returns a handle with the rebuilt prompt.** This follows from `resumeSession` re-resolving `repoPrompt` at line 451, and `ensureSessionLive` resuming from the persisted JSONL (`apps/server/src/bridge/types.ts:42-48`). If the bridge instead hands back a cached handle for a session disposed moments earlier, drop the recycle: keep the mark, log that the instructions apply from the next wake, and leave the turn running on the existing handle — the custom task text still reaches it, which is the larger half of the feature. - **A triage reason never reaches the implementer branch of `resolveSession`.** `prHead` is only set for pull requests (`dispatcher.ts:679-713`) and both triage reasons are issue-only. If a triage reason is ever observed with `prHead !== null`, do not mark the session — an implementer session must keep `system_prompts.implementer`. - **Orphan marker rows.** Sessions deleted outside the `DELETE /sessions/:id` route leave a row behind, exactly as `plan_issue_sessions` does today. Harmless: the row is only ever read by session id. No cleanup job.
lucas closed this issue 2026-11-09 11:17:02 +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#235
No description provided.