Repository navigation
feat(mcp): a tool response carries errors recorded since the last one - #288
Merged
Merged
Conversation
…ponse once errors.jsonl gets records no MCP tool returns: the companion's heal hook (unfaithful_heal), browser chart render failures, code edits in the UI. These tests pin a `new_errors` field on every tool response: each record reported once, from this process's start, per project, capped, without repeating a tool's own reported failure, and tolerant of a torn line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_tag_project now adds `new_errors` to a reply when the active project's errors.jsonl has records this MCP process has not reported: the companion's unfaithful_heal, a browser chart failure, a UI code edit. A per-log cursor (inode, byte offset) starts at this process's start, so an unchanged log costs one stat and old sessions' history is not replayed. A record the call already answers with is left out: one this process wrote for the same tool (tracked through a new errors.RECORD_HOOKS), or one whose id is in the reply. Items are compact (no traceback or recipe code), newest first, capped at 10 with `new_errors_omitted`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
paddymul
marked this pull request as ready for review
September 28, 2026 22:08
FastMCP runs tool calls on a thread pool. A project_list that returns while catalog_run is between recording its build error and replying reads that record from the log and reports it as new_errors, and catalog_run's own reply carries it again in `error`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…any reply A record is left out of new_errors when this process wrote it with `tool` set to the tool whose call was running at the time (a contextvar set by the tool wrapper). Such a record is in that call's own reply, as `error` or in `recalc`. The check compared the record's tool with the tool being answered, so a concurrent call of another tool reported it. A record written outside a tool call, or during one without that tool's name (a heal while a tool reads an entry), is still reported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Review pass over this PR. Findings and how each was addressed:
Noted, not changed: 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
errors.jsonlis written by more than the MCP tools. The heal path inresult_cache.pyrecordsunfaithful_heal, the companion records browser chart failures (chart_render) and failed code edits from the UI (api_code). No MCP tool returns those records, so a model driving tallyman over MCP does not see them.In one session the log collected six
unfaithful_healrecords, one for each version (v1 to v6) of the derived entryqb_epa. The model saw only the first, and only because it tailederrors.jsonlby hand. It guessed a wrong cause from that one record and never learned about the other five.Fix
_tag_project, which already addsprojectand the project-switchwarningto every tool reply, now addsnew_errorswhen the active project's log has records this MCP process has not reported yet. With none, the field is absent.{ "active": "demo", "available": ["demo"], "project": "demo", "new_errors": [ { "id": "57b476ad2161", "created_at": "2026-09-28T21:53:26.210465+00:00", "code": "unfaithful_heal", "hash": "abc123def456", "message": "self-heal produced different rows than were built [structural (#88)]: digest 1 != recorded 2" } ] }id,created_at,code,hash,aliasandversion(fromversion_of_hash, when the hash is in an alias history),tool, andmessagecut at 300 characters.codeis echoed only when it is a short kind likeunfaithful_heal: on a build failure the field holds the whole recipe, so it is left out. No traceback, no prompt, and keys whose value is null are dropped.new_errors_omittedgiving the count left out.stat. A changed one is read from the offset. A line with no newline yet (an append in progress) is left for the next call, a line that is not a JSON object is skipped, and a log that was cleared and written again (new inode, or smaller than the offset) is read from the start.toolequal to the tool being answered (its own build error, therecalcfailures and skips ofcatalog_revise's cascade), or one whose id is anywhere in the reply asidorerror_id(catalog_chart_errorsreturns the records themselves). To know which records this process wrote,record_errornow calls the hooks in a newerrors.RECORD_HOOKSlist, and the server registers one that keeps the ids. A heal recorded in this process while a tool reads an entry hastoolNone, so it is still reported.catalog_run's docstring has two lines onnew_errors, since the model never sees_tag_project's.#248 makes the SPA refetch on the
unfaithful_healSSE event. This PR does not touch the SPA or SSE, so the two do not overlap.Tests
tests/test_mcp_new_errors.py, 11 tests.record_errorcalled directly stands in for another process. They cover: a record reported once on the next reply and absent after; a record from before the process started skipped while one from before the first call is reported; the compact item with alias and version and a 2000-character message; recipe code not echoed;catalog_run's own build error not repeated;catalog_revise's cascade failure not repeated; a heal recorded in this process during a call still reported; the cap of 10 withnew_errors_omitted == 3; a torn line and a half-written line; an unchanged log not opened; separate cursors acrossproject_switch.new_errorsfield.uvx ruff checkclean.End to end, I ran
tallyman mcpover stdio with fastmcp'sClientagainst a temporaryTALLYMAN_HOME. A record written before the server started was not reported. I calledproject_list, then recorded anunfaithful_healfrom a separateuv run pythonprocess. The nextproject_listreply is the JSON above, and the call after it had nonew_errors.Not in this PR
qb_epahealed unfaithfully at every version. This PR only makes the records reach the model.clear_errorsis called only by the companion's clear-errors route (the error banner's dismiss).🤖 Generated with Claude Code