Plan-issue seed states the literal arguments of its two Forgejo read calls #273

New issue

Closed

opened 2026-11-13 13:09:33 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-11-13 13:09:33 +01:00

Collaborator

Problem

Agents in plan-issue sessions consistently call forgejo_read_issue and forgejo_list_issue_comments with a parameter named issue_number. The schema declares number (plus the alias index), so the call fails validation and costs a round trip. This is the second time the same failure has appeared with a different wrong name: the index alias in numberedParams exists only because agents previously guessed index just as consistently.

The cause is not naming taste, it is missing information:

Deck registers its tools as customTools (apps/server/src/bridge/in-process.ts:323-329). A custom tool without loadMode defaults to "discoverable" (@oh-my-pi/pi-coding-agent/src/extensibility/custom-tools/types.ts:206-207), and deck sets loadMode nowhere.
Discoverable tools are mounted under xd:// and removed from the top-level tool list (tools/xdev.ts:85-88, sdk.ts:4546-4554, session/session-tools.ts:1328-1333), so their JSON Schema never reaches the provider request.
tools.xdevDocs defaults to "catalog" (tools/settings.ts:1025-1029), so no device docs are inlined either. Each Forgejo tool's entire prompt-side presence is one catalog line: its name plus a description truncated to 200 bytes (tools/xdev.ts:364-367, XDEV_EXTERNAL_DESCRIPTION_CAP = 200 at :253), followed by "Read xd://<tool> for full docs + JSON schema before first use".

So the agent must produce an args object for a tool whose parameter names it has never been shown, and fills the gap from training priors. Priors are stable across sessions, which is exactly why the wrong name is consistent: issue_number is GitHub's REST/MCP name for this parameter, and index — the previous consistent guess — is the Forgejo REST API's name for it. Adding index as an alias made one prior correct without closing the gap, so the next-strongest prior surfaced. Adding issue_number would repeat that.

Two aggravating facts, both confirmed by reading the code this session: the .refine() that makes a number mandatory is an ArkType narrow dropped from the wire schema (@oh-my-pi/pi-ai/src/utils/validation.ts:1783-1785), so even the on-demand docs render number?: number; index?: number with nothing marked required; and unknown keys are stripped before foldIndexAlias runs (omptype/src/zod.ts:405, extras: "delete"), so no deck-side tolerance can ever see a key the schema does not declare.

The failure is concentrated in the plan-issue workflow, where these two calls are the first thing a session does. The fix is to stop making the agent infer the call: the plan-issue prompt already knows the issue number, so it states the argument objects verbatim. The tool schema is deliberately left untouched — no new alias, no change to numberedParams.

Approach

Both edits are independent; either order builds and leaves tests passing.

1. Deck plan-issue system context states both calls verbatim

In apps/server/src/forgejo/plan-issue.ts, planIssueSystemContext (line 65) builds seed; its issue branch (line 68) currently reads:

Seed: issue #${b.issueNumber}. Read it with \`forgejo_read_issue\` and every page of \`forgejo_list_issue_comments\`. The plan replaces its description, so keep the substance of the existing description in \`## Problem\`. Its title is kept.

Replace the first two sentences so the call arguments appear literally, with the known issue number substituted. The rest of the sentence (description/title handling) is unchanged:

Seed: issue #${b.issueNumber}. Read it with \`forgejo_read_issue\` (\`{"number": ${b.issueNumber}}\`), then every page of \`forgejo_list_issue_comments\` (\`{"number": ${b.issueNumber}, "page": 1}\`, then page 2, and so on until a page reports no more pages). The plan replaces its description, so keep the substance of the existing description in \`## Problem\`. Its title is kept.

Only the b.issueNumber !== null branch changes. The inbox and free-text branches (lines 69-71) call neither tool and stay as they are. No other part of the returned template changes.

2. `/plan-issue` command states the same shape

In .omp/commands/plan-issue.md, step 1's issue bullet (line 12) currently reads:

   - `#N` or bare digits `N` → Forgejo issue N. Call `forgejo_read_issue`, then `forgejo_list_issue_comments` for all pages; comments may refine the brief.

Replace it with the argument shape spelled out (the number is only known at runtime here, so N stays symbolic):

   - `#N` or bare digits `N` → Forgejo issue N. Call `forgejo_read_issue` with `{"number": N}`, then `forgejo_list_issue_comments` with `{"number": N, "page": 1}` and so on for all pages; comments may refine the brief.

Line 21 of the same file also names forgejo_read_issue in the preflight; it instructs no call arguments and is left alone.

3. Pin the rendered text

In apps/server/src/forgejo/plan-issue.test.ts, inside the existing describe("planIssueSystemContext", …) block (line 37; base is defined at line 38 with issueNumber: null), add a test next to the existing seed tests:

test("an issue seed states the literal arguments of both read calls", () => {
	const out = planIssueSystemContext({ ...base, issueNumber: 12 });
	expect(out).toContain('`{"number": 12}`');
	expect(out).toContain('`{"number": 12, "page": 1}`');
});

This is the behaviour the change exists for — the number is interpolated into both argument objects — and it fails against the current text.

Nothing else is touched: numberedParams and foldIndexAlias in apps/server/src/forgejo/tools/shared.ts keep the number field and the index alias exactly as they are, so workflows outside plan-issue (implementation sessions, comment-triggered sessions) keep the fallback they rely on.

Critical files & anchors

apps/server/src/forgejo/plan-issue.ts:65-89 — planIssueSystemContext; the issue branch of seed is the only line to change. It is a TypeScript template literal, so the JSON braces need no escaping but the surrounding backticks do.
.omp/commands/plan-issue.md:11-17 — step 1 "Resolve the seed"; only the #N bullet changes.
apps/server/src/forgejo/plan-issue.test.ts:37-56 — the existing planIssueSystemContext tests and the base binding to spread from.

Verification

Prerequisite: workspace dependencies installed (bun install at the repo root) — a bare bun run in this container failed on a missing transitive module, so install first if the suite cannot start.

Unit proof of the new behaviour, from apps/server:
bun test src/forgejo/plan-issue.test.ts
The new test must pass, and the three pre-existing planIssueSystemContext tests must still pass. Confirm it is load-bearing by checking it fails before step 1 is applied.
Typecheck, from apps/server: bun run typecheck.
End-to-end: in the deck UI, start a plan-issue session seeded from any open Forgejo issue (its number is N). Expected observable result: the session's first Forgejo tool call is forgejo_read_issue with {"number": N} and succeeds — no Validation failed for tool "forgejo_read_issue" result and no retry — followed by forgejo_list_issue_comments with {"number": N, "page": 1}.

Assumptions & contingencies

The fix assumes the failing calls originate in sessions that receive planIssueSystemContext or run /plan-issue. If the wrong parameter name is later observed in implementation or comment-triggered sessions, apply the identical treatment to apps/server/src/bridge/session-policy.ts:643 and apps/server/src/forgejo/conversation-detect.ts:318-321, which instruct the same two calls; do not add a schema alias.
tools.xdevDocs is assumed to stay at its default "catalog". If it is ever set to "inline" or "builtins" with a forgejo_* glob in tools.xdevInlineDevices (apps/server/src/model-config.ts:530-536 is where deck would set it), full parameter schemas reach the prompt and this prompt text becomes redundant rather than wrong; leave it in place.

## Problem Agents in plan-issue sessions consistently call `forgejo_read_issue` and `forgejo_list_issue_comments` with a parameter named `issue_number`. The schema declares `number` (plus the alias `index`), so the call fails validation and costs a round trip. This is the second time the same failure has appeared with a different wrong name: the `index` alias in `numberedParams` exists only because agents previously guessed `index` just as consistently. The cause is not naming taste, it is missing information: - Deck registers its tools as `customTools` (`apps/server/src/bridge/in-process.ts:323-329`). A custom tool without `loadMode` defaults to `"discoverable"` (`@oh-my-pi/pi-coding-agent/src/extensibility/custom-tools/types.ts:206-207`), and deck sets `loadMode` nowhere. - Discoverable tools are mounted under `xd://` and removed from the top-level tool list (`tools/xdev.ts:85-88`, `sdk.ts:4546-4554`, `session/session-tools.ts:1328-1333`), so their JSON Schema never reaches the provider request. - `tools.xdevDocs` defaults to `"catalog"` (`tools/settings.ts:1025-1029`), so no device docs are inlined either. Each Forgejo tool's entire prompt-side presence is one catalog line: its name plus a description truncated to 200 bytes (`tools/xdev.ts:364-367`, `XDEV_EXTERNAL_DESCRIPTION_CAP = 200` at `:253`), followed by "Read `xd://<tool>` for full docs + JSON schema before first use". So the agent must produce an args object for a tool whose parameter names it has never been shown, and fills the gap from training priors. Priors are stable across sessions, which is exactly why the wrong name is consistent: `issue_number` is GitHub's REST/MCP name for this parameter, and `index` — the previous consistent guess — is the Forgejo REST API's name for it. Adding `index` as an alias made one prior correct without closing the gap, so the next-strongest prior surfaced. Adding `issue_number` would repeat that. Two aggravating facts, both confirmed by reading the code this session: the `.refine()` that makes a number mandatory is an ArkType narrow dropped from the wire schema (`@oh-my-pi/pi-ai/src/utils/validation.ts:1783-1785`), so even the on-demand docs render `number?: number; index?: number` with nothing marked required; and unknown keys are stripped before `foldIndexAlias` runs (`omptype/src/zod.ts:405`, `extras: "delete"`), so no deck-side tolerance can ever see a key the schema does not declare. The failure is concentrated in the plan-issue workflow, where these two calls are the first thing a session does. The fix is to stop making the agent infer the call: the plan-issue prompt already knows the issue number, so it states the argument objects verbatim. The tool schema is deliberately left untouched — no new alias, no change to `numberedParams`. ## Approach Both edits are independent; either order builds and leaves tests passing. ### 1. Deck plan-issue system context states both calls verbatim In `apps/server/src/forgejo/plan-issue.ts`, `planIssueSystemContext` (line 65) builds `seed`; its issue branch (line 68) currently reads: ``` Seed: issue #${b.issueNumber}. Read it with \`forgejo_read_issue\` and every page of \`forgejo_list_issue_comments\`. The plan replaces its description, so keep the substance of the existing description in \`## Problem\`. Its title is kept. ``` Replace the first two sentences so the call arguments appear literally, with the known issue number substituted. The rest of the sentence (description/title handling) is unchanged: ``` Seed: issue #${b.issueNumber}. Read it with \`forgejo_read_issue\` (\`{"number": ${b.issueNumber}}\`), then every page of \`forgejo_list_issue_comments\` (\`{"number": ${b.issueNumber}, "page": 1}\`, then page 2, and so on until a page reports no more pages). The plan replaces its description, so keep the substance of the existing description in \`## Problem\`. Its title is kept. ``` Only the `b.issueNumber !== null` branch changes. The inbox and free-text branches (lines 69-71) call neither tool and stay as they are. No other part of the returned template changes. ### 2. `/plan-issue` command states the same shape In `.omp/commands/plan-issue.md`, step 1's issue bullet (line 12) currently reads: ``` - `#N` or bare digits `N` → Forgejo issue N. Call `forgejo_read_issue`, then `forgejo_list_issue_comments` for all pages; comments may refine the brief. ``` Replace it with the argument shape spelled out (the number is only known at runtime here, so `N` stays symbolic): ``` - `#N` or bare digits `N` → Forgejo issue N. Call `forgejo_read_issue` with `{"number": N}`, then `forgejo_list_issue_comments` with `{"number": N, "page": 1}` and so on for all pages; comments may refine the brief. ``` Line 21 of the same file also names `forgejo_read_issue` in the preflight; it instructs no call arguments and is left alone. ### 3. Pin the rendered text In `apps/server/src/forgejo/plan-issue.test.ts`, inside the existing `describe("planIssueSystemContext", …)` block (line 37; `base` is defined at line 38 with `issueNumber: null`), add a test next to the existing seed tests: ```ts test("an issue seed states the literal arguments of both read calls", () => { const out = planIssueSystemContext({ ...base, issueNumber: 12 }); expect(out).toContain('`{"number": 12}`'); expect(out).toContain('`{"number": 12, "page": 1}`'); }); ``` This is the behaviour the change exists for — the number is interpolated into both argument objects — and it fails against the current text. Nothing else is touched: `numberedParams` and `foldIndexAlias` in `apps/server/src/forgejo/tools/shared.ts` keep the `number` field and the `index` alias exactly as they are, so workflows outside plan-issue (implementation sessions, comment-triggered sessions) keep the fallback they rely on. ## Critical files & anchors - `apps/server/src/forgejo/plan-issue.ts:65-89` — `planIssueSystemContext`; the issue branch of `seed` is the only line to change. It is a TypeScript template literal, so the JSON braces need no escaping but the surrounding backticks do. - `.omp/commands/plan-issue.md:11-17` — step 1 "Resolve the seed"; only the `#N` bullet changes. - `apps/server/src/forgejo/plan-issue.test.ts:37-56` — the existing `planIssueSystemContext` tests and the `base` binding to spread from. ## Verification Prerequisite: workspace dependencies installed (`bun install` at the repo root) — a bare `bun` run in this container failed on a missing transitive module, so install first if the suite cannot start. 1. Unit proof of the new behaviour, from `apps/server`: `bun test src/forgejo/plan-issue.test.ts` The new test must pass, and the three pre-existing `planIssueSystemContext` tests must still pass. Confirm it is load-bearing by checking it fails before step 1 is applied. 2. Typecheck, from `apps/server`: `bun run typecheck`. 3. End-to-end: in the deck UI, start a plan-issue session seeded from any open Forgejo issue (its number is `N`). Expected observable result: the session's first Forgejo tool call is `forgejo_read_issue` with `{"number": N}` and succeeds — no `Validation failed for tool "forgejo_read_issue"` result and no retry — followed by `forgejo_list_issue_comments` with `{"number": N, "page": 1}`. ## Assumptions & contingencies - The fix assumes the failing calls originate in sessions that receive `planIssueSystemContext` or run `/plan-issue`. If the wrong parameter name is later observed in implementation or comment-triggered sessions, apply the identical treatment to `apps/server/src/bridge/session-policy.ts:643` and `apps/server/src/forgejo/conversation-detect.ts:318-321`, which instruct the same two calls; do not add a schema alias. - `tools.xdevDocs` is assumed to stay at its default `"catalog"`. If it is ever set to `"inline"` or `"builtins"` with a `forgejo_*` glob in `tools.xdevInlineDevices` (`apps/server/src/model-config.ts:530-536` is where deck would set it), full parameter schemas reach the prompt and this prompt text becomes redundant rather than wrong; leave it in place.