coilyco-gaming/sirens-echo

Watch

Drop the issue field from the response envelope and let the model use the Forgejo tool it already has #102

New issue

Closed

opened 2026-08-11 08:07:48 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-11 08:07:48 +00:00

Member

Every model response is a JSON envelope:

{"reply": "...", "issue": null}

issue is occasionally populated with a full proposal, and the worker files it afterwards in forgejo.issue.ensure:

{"reply": "…I've also flagged the stockpile question as a knowledge gap…",
 "issue": {"kind": "knowledge-gap",
           "title": "Eco stockpile mechanic undefined in knowledge base",
           "body": "A member asked what a stockpile is in Eco. The current skillpack has no authoritative gameplay guide…"}}

The capability is good. The layer looks wrong.

The model already has the tool

Measured across captured requests on the sirens-echo/deepseek route over six hours:

14 of 14 requests contain forgejo__, the MCP tool-namespace prefix, the same convention as eco__get_climate
13 of 14 contain create_issue

So Forgejo tools are handed to the model on every turn, while the envelope carries a parallel path for the same action. Two mechanisms for one capability.

The envelope path is the broken one

forgejo.issue.ensure does not use the MCP. It calls http://sirens-deep-forgejo-mcp:8080/api/list_issue, a REST-shaped path against a server that serves MCP at /mcp, and gets 404 on every attempt. 17 such spans in one window, producing the 14 silent forgejo.issue.failed errors in #89.

So the worker hand-rolled an HTTP client to do something the model was already equipped to call. Every knowledge gap Deep has identified so far has been lost.

What removing the envelope buys

The reply stops needing to be JSON. Today every response must parse as an envelope, which is why the turn path carries a model.response.repair round. That retry exists to absorb malformed JSON. Plain prose removes the failure class rather than retrying it.

Issue filing becomes observable. It would appear as a tool call in the trace beside the Eco calls, instead of a side effect only visible through a failed span.

Downstream reads get simpler. The captured assistant message is currently the raw envelope, so anything displaying model output shows {"reply":"pear","issue":null} rather than pear. That is true on the ser8 console today.

The honest counterargument

The envelope is deterministic. The issue decision is made with the finished answer in hand, in the same completion, and the worker can rely on the field being present.

A tool-call model is probabilistic. It might file mid-turn and then write a reply that does not mention it, file nothing when it should, or file twice across a retry. If the envelope was chosen for that reason, that reason still holds and this issue should be closed rather than implemented.

Worth checking whether the guardfile's create_issue grant is even reachable in practice before committing to the swap, given the /mcp endpoint currently returns 400 once per turn per #90.

Relationship to the open issues

#89 says fix the 404. This supersedes it: if the envelope goes, forgejo.issue.ensure goes with it and there is no 404 to fix. #89 should stay open only as the fallback if this is rejected.
#90 is independent and blocks either path, since it is the /mcp endpoint itself failing once per turn.
#86 is unaffected, though dropping the JSON requirement removes one source of completion-budget pressure.

Suggested scope

Remove issue from the response contract and the prompt that requests it.
Delete forgejo.issue.ensure and its HTTP client.
Confirm the Forgejo MCP create_issue grant is reachable, blocked on #90.
Update the prompt to tell the model to call the tool when it identifies a knowledge gap.
Keep the reply as plain text.

Every model response is a JSON envelope: ```json {"reply": "...", "issue": null} ``` `issue` is occasionally populated with a full proposal, and the worker files it afterwards in `forgejo.issue.ensure`: ```json {"reply": "…I've also flagged the stockpile question as a knowledge gap…", "issue": {"kind": "knowledge-gap", "title": "Eco stockpile mechanic undefined in knowledge base", "body": "A member asked what a stockpile is in Eco. The current skillpack has no authoritative gameplay guide…"}} ``` The capability is good. The layer looks wrong. ## The model already has the tool Measured across captured requests on the `sirens-echo/deepseek` route over six hours: * **14 of 14** requests contain `forgejo__`, the MCP tool-namespace prefix, the same convention as `eco__get_climate` * **13 of 14** contain `create_issue` So Forgejo tools are handed to the model on every turn, while the envelope carries a parallel path for the same action. Two mechanisms for one capability. ## The envelope path is the broken one `forgejo.issue.ensure` does not use the MCP. It calls `http://sirens-deep-forgejo-mcp:8080/api/list_issue`, a REST-shaped path against a server that serves MCP at `/mcp`, and gets **404 on every attempt**. 17 such spans in one window, producing the 14 silent `forgejo.issue.failed` errors in #89. So the worker hand-rolled an HTTP client to do something the model was already equipped to call. Every knowledge gap Deep has identified so far has been lost. ## What removing the envelope buys **The reply stops needing to be JSON.** Today every response must parse as an envelope, which is why the turn path carries a `model.response.repair` round. That retry exists to absorb malformed JSON. Plain prose removes the failure class rather than retrying it. **Issue filing becomes observable.** It would appear as a tool call in the trace beside the Eco calls, instead of a side effect only visible through a failed span. **Downstream reads get simpler.** The captured assistant message is currently the raw envelope, so anything displaying model output shows `{"reply":"pear","issue":null}` rather than `pear`. That is true on the ser8 console today. ## The honest counterargument The envelope is **deterministic**. The issue decision is made with the finished answer in hand, in the same completion, and the worker can rely on the field being present. A tool-call model is probabilistic. It might file mid-turn and then write a reply that does not mention it, file nothing when it should, or file twice across a retry. If the envelope was chosen for that reason, that reason still holds and this issue should be closed rather than implemented. Worth checking whether the guardfile's `create_issue` grant is even reachable in practice before committing to the swap, given the `/mcp` endpoint currently returns 400 once per turn per #90. ## Relationship to the open issues * **#89** says fix the 404. This supersedes it: if the envelope goes, `forgejo.issue.ensure` goes with it and there is no 404 to fix. #89 should stay open only as the fallback if this is rejected. * **#90** is independent and blocks either path, since it is the `/mcp` endpoint itself failing once per turn. * **#86** is unaffected, though dropping the JSON requirement removes one source of completion-budget pressure. ## Suggested scope 1. Remove `issue` from the response contract and the prompt that requests it. 2. Delete `forgejo.issue.ensure` and its HTTP client. 3. Confirm the Forgejo MCP `create_issue` grant is reachable, blocked on #90. 4. Update the prompt to tell the model to call the tool when it identifies a knowledge gap. 5. Keep the reply as plain text.

coilyco-ops referenced this issue

2026-08-11 08:42:42 +00:00

The final user message is a composed blob rather than the user's message #104

coilyco-ops commented

2026-08-11 10:00:08 +00:00

Author

Member

Before implementing this I checked the premise, and the layering argument does not hold. The design case might still, but it should be argued on its own merits rather than on the envelope path being broken.

`/api/{tool}` is not a hand-rolled REST path

It is ward-mcp's own first-class surface. From internal/mcpserver/api.go:25:

// serveAPITool projects every registered MCP tool onto POST
// /api/{tool-name}. The HTTP and MCP surfaces share the exact handler.

Handler() mounts both: mux.Handle("/mcp", ...) and mux.HandleFunc(apiPrefix, s.serveAPITool) with apiPrefix = "/api/".

The worker targets it deliberately, not accidentally. internal/community/forgejo.go:153:

if !strings.HasSuffix(path, "/mcp") {
    return "", fmt.Errorf("Forgejo MCP URL must end in /mcp")
}
endpoint.Path = strings.TrimSuffix(path, "/mcp") + "/api/" + url.PathEscape(tool)

So it validates the configured MCP URL and converts it to the documented projection of the same handler. Both sides of this were built to fit each other. "The worker hand-rolled an HTTP client to do something the model was already equipped to call" is not what the code does.

The grant is present too

deploy/services/sirens-echo/forgejo-mcp.mcp.kdl:27 has:

can list issue {
    path "/repos/coilyco-gaming/sirens-echo/issues"

which is exactly the list_issue tool the worker calls at forgejo.go:96, and the sibling guardfile annotates that same shape as GET -> list_issue.

So what is the 404

Not the path shape and not a missing grant in the tracked guardfile. That leaves a deployment mismatch: the running sirens-deep-forgejo-mcp serving an older guardfile than the one on main, or a different spec than the one I read. That is checkable against the live pod and I cannot settle it from source.

Worth noting the failure order this explains cleanly: EnsureIssue calls findOpenIssue -> list_issue -> 404 -> returns early, so create_issue is never reached. That matches the observed spans exactly, 17 x 404 on list_issue and zero attempts on create_issue, and it is why nothing was ever created.

Recommendation

Split the two claims.

The bug is one deployment fixing a stale guardfile, not a refactor. Worth confirming first, because if that is all it is, the envelope path starts working with no code change.
The design case for dropping the envelope stands on its own: plain-text replies remove the model.response.repair round rather than retrying it, issue filing becomes visible as a tool call, and downstream consumers stop rendering raw JSON. Those are real and unaffected by the above.

The issue's own counterargument also still stands: the envelope is deterministic and a tool call is probabilistic. Trading a working deterministic path for a probabilistic one is a different decision than replacing a broken one, and that is the decision this now is.

Also relevant: #90 is listed here as blocking step 3. I could not reproduce it on current main (see my comment there), so it may not be a blocker at all.

Happy to implement whichever way you call it. I did not want to delete a working mechanism on a premise that turned out to be inaccurate.

Before implementing this I checked the premise, and **the layering argument does not hold**. The design case might still, but it should be argued on its own merits rather than on the envelope path being broken. ## `/api/{tool}` is not a hand-rolled REST path It is ward-mcp's own first-class surface. From `internal/mcpserver/api.go:25`: ```go // serveAPITool projects every registered MCP tool onto POST // /api/{tool-name}. The HTTP and MCP surfaces share the exact handler. ``` `Handler()` mounts both: `mux.Handle("/mcp", ...)` and `mux.HandleFunc(apiPrefix, s.serveAPITool)` with `apiPrefix = "/api/"`. The worker targets it deliberately, not accidentally. `internal/community/forgejo.go:153`: ```go if !strings.HasSuffix(path, "/mcp") { return "", fmt.Errorf("Forgejo MCP URL must end in /mcp") } endpoint.Path = strings.TrimSuffix(path, "/mcp") + "/api/" + url.PathEscape(tool) ``` So it validates the configured MCP URL and converts it to the documented projection of the same handler. Both sides of this were built to fit each other. "The worker hand-rolled an HTTP client to do something the model was already equipped to call" is not what the code does. ## The grant is present too `deploy/services/sirens-echo/forgejo-mcp.mcp.kdl:27` has: ```kdl can list issue { path "/repos/coilyco-gaming/sirens-echo/issues" ``` which is exactly the `list_issue` tool the worker calls at `forgejo.go:96`, and the sibling guardfile annotates that same shape as `GET -> list_issue`. ## So what is the 404 Not the path shape and not a missing grant in the tracked guardfile. That leaves a deployment mismatch: the running `sirens-deep-forgejo-mcp` serving an older guardfile than the one on `main`, or a different spec than the one I read. That is checkable against the live pod and I cannot settle it from source. Worth noting the failure order this explains cleanly: `EnsureIssue` calls `findOpenIssue` -> `list_issue` -> 404 -> returns early, so `create_issue` is never reached. That matches the observed spans exactly, 17 x 404 on `list_issue` and zero attempts on `create_issue`, and it is why nothing was ever created. ## Recommendation Split the two claims. * **The bug** is one deployment fixing a stale guardfile, not a refactor. Worth confirming first, because if that is all it is, the envelope path starts working with no code change. * **The design case** for dropping the envelope stands on its own: plain-text replies remove the `model.response.repair` round rather than retrying it, issue filing becomes visible as a tool call, and downstream consumers stop rendering raw JSON. Those are real and unaffected by the above. The issue's own counterargument also still stands: the envelope is deterministic and a tool call is probabilistic. Trading a working deterministic path for a probabilistic one is a different decision than replacing a broken one, and that is the decision this now is. Also relevant: https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/90 is listed here as blocking step 3. I could not reproduce it on current `main` (see my comment there), so it may not be a blocker at all. Happy to implement whichever way you call it. I did not want to delete a working mechanism on a premise that turned out to be inaccurate.

coilyco-ops referenced this issue from a commit

2026-08-11 10:28:36 +00:00

feat: drop the issue envelope and let the model file through its own tool

coilyco-ops referenced this issue

2026-08-11 10:28:59 +00:00

feat: drop the issue envelope and let the model file through its own tool #109

coilyco-ops referenced this issue from a commit

2026-08-12 10:01:51 +00:00

fix(proxy): stop asking the backend for a JSON object

coilyco-ops closed this issue

2026-08-12 10:01:52 +00:00

coilyco-ops referenced this issue from a commit