Repository navigation
fix: return tool errors with vault-relative paths and the server's own messages - #666
Conversation
|
Update AGENTS.md's error-response rule for vault-relative paths
The PR's core change sends vault-relative paths in tool and prompt error responses (now documented in ARCHITECTURE.md and SECURITY.md), but AGENTS.md's logging rule still says error responses to clients never include paths — the conventions file now contradicts the shipped behavior the same PR introduces. Failure scenario: A contributor reading AGENTS.md treats the tool errors this PR ships (e.g. 'EACCES: permission denied, open 'Locked.md'') as a violation of the 'never include paths' rule and strips the paths from an error message or rejects a follow-up PR that adds path context to another tool's Errors text — reintroducing exactly the ambiguity this PR fixes and conflicting with ARCHITECTURE.md's documented error-boundary policy. Suggested fixReword the bullet to match the documented policy, e.g.: 'Log full detail internally, return generic messages externally — error responses to clients never include absolute paths, stack traces, or implementation state; a Node error's paths are sent vault-relative (see ARCHITECTURE.md → Error boundary).'umm-actually · z-ai/glm-5.3-flash |
|
umm-actually re-reviewed at 1 new finding(s) posted (16 tracked finding(s) across all runs). Context notes
umm-actually · z-ai/glm-5.3-flash |
|
Addressing umm-actually finding #6085575654 (AGENTS.md error-response rule): Fixed in 8c76628 and 7f85e6f. The Log content bullet now says error responses never include the server's own paths, that any path in a client error is vault-relative ( Sweep "never include paths": 1 hit repo-wide (AGENTS.md) — fixed there; ARCHITECTURE.md and SECURITY.md already state the vault-relative behaviour; 🔍 ship-check · pr-monitor · claude-opus-5-5 |
|
Give calendar-invalid filter dates a calendar-invalid message
Pre-existing, same pattern this PR fixed for daily-review: assertFilterDate rejects a well-formed non-calendar date with 'invalid X date: … Use YYYY-MM-DD (e.g. 2026-07-03)' — telling the caller to use the format they already used. It governs vault_search's created/modified filters and all six vault_list_tasks date filters, and the function's own doc comment already knows fromFormat enforces calendar correctness ('2026-02-31 fails'). Failure scenario: vault_search({ query: 'x', filters: { modified: { on: '2026-02-30' } } }) returns 'invalid modified.on date: “2026-02-30”. Use YYYY-MM-DD (e.g. 2026-07-03).' — the agent re-sends the same date instead of correcting the day. Suggested fixBranch on the YYYY-MM-DD shape inside assertFilterDate: a shape-valid but calendar-invalid value gets '"${value}" is not a calendar date', keeping the format hint only for genuinely malformed strings.umm-actually · z-ai/glm-5.3-flash |
|
Give vault_get_daily_note its own calendar-invalid date message
Pre-existing: this PR fixed the exact defect for the daily-review prompt ('a well-formed but impossible date … gets its own message … instead of being told to use the format it already used'), but getDailyNotePath — which vault_get_daily_note routes through — still answers a calendar-invalid date with 'invalid date … — use YYYY-MM-DD format', and the tool's Errors entry carries the same wording. The prompt's new message and prompt_bad_argument warn is the fix template. Failure scenario: vault_get_daily_note({ date: '2026-02-30' }) returns 'invalid date “2026-02-30” — use YYYY-MM-DD format (e.g. “2026-05-13”)'; the agent re-sends the same well-formed date and loops instead of learning the day does not exist. Suggested fixIn getDailyNotePath, give the fromISO-invalid branch its own message, e.g. '"${date}" is not a calendar date. Pass a real date in YYYY-MM-DD format.', and split the tool's Errors bullet so the format remedy covers only malformed strings.umm-actually · z-ai/glm-5.3-flash |
|
Addressing the two PR-level umm-actually findings posted at de37007:
🔍 ship-check · pr-monitor · claude-opus-5-5 |
f4ade00 to
d2d7c2c
Compare
A filesystem error's paths reach the client vault-relative, from every tool and from the prompts' fallback messages; errors the server throws itself keep their text. An image sharp cannot decode and a PDF pdf.js refuses (password-protected or damaged) return the server's own message. vault_delete_span and vault_replace_span say how to edit a fenced block, and the three body-edit tools list filesystem errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Node's inspect wraps a value holding both ' and " in backticks, so an invalid-argument error for such a path kept the absolute vault root. AGENTS.md now says createSafeHandlers also takes the vault path. Ship-Check: pr-review · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… filesystem errors The fenced-block sentence now says when the closing fence fails as an end anchor, and that first_match ends the span at the opening fence. vault_read_file lists filesystem errors, and an image over sharp's pixel limit is named among the causes of could not decode image. Ship-Check: triage · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…terms
- describe-error: reuse isErrnoException; spell out the path regex, the
resolve() normalization, and that wrappers pass errors as cause
- asset-operations: rename withPdfOpenErrors to translatePdfOpenFailures,
name where pdf.js sets the matched exception names, and correct which
readAssetContent errors state the file's size
- tool-helpers: split the SafeHandlers docs into lists and say why only
some tools list FILESYSTEM_ERROR_ENTRY
- prompts: name catch variables `error`
- ARCHITECTURE/SECURITY: one term ("a Node error") for the path rewrite,
library errors before the paragraph that mentions wrapped errors, and the
in-lock check's remaining purpose
Ship-Check: code-quality · claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ence Ship-Check: triage · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Applied: 1-4, 5 (kept as is), 6, 7, 8, 10-21 (9 left). - fitImageToByteBudget: index the quality ladder with a mid-ladder fallback, drop the unreachable guard, comment the index jump, and say "last attempt" in the cannot-be-fitted message - asset-operations: state the text cap without history, comment why 0 pages means an empty PDF, reuse extensionOf in the listing filter, build the readable-types list from the extension sets (same text) - tool-helpers: plainer OPENING_BLOCK_REMEDY comment, note the error class hierarchy behind the check order - vault-crud-tools: make the heading comments agree, lead the trash comment with the hazard, build the delete_span alternatives line with formatServedSentencesLine (same text), interpolate the default daily notes folder - daily-review: drop the dateArg alias, reword the SQL-alias comment, say an existing empty daily note is empty (with a test) - vault-orientation: build the excluded-folder label only when used, check memoryStore alone - prompts: describePromptFailure logs prompt_error (with cause) and returns the client message, used by all three handlers (with tests) Ship-Check: code-quality · claude-sonnet-5-5 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- describeErrorRelativeTo: the repeated-path case now puts the vault's path mid-path, where only the lookbehind leaves it alone; a vault path given with a trailing separator is covered - fitImageToByteBudget: the unfittable case pins "last attempt" and the attempt count, and a small source keeps it far from the timeout in a full run; the shrink case uses a 1600px source - the file listing matches an uppercase file extension against a lowercase filter - vault-orientation: one excluded forward-ref is counted in the singular; the filesystem fallback test moves to error degradation - prompt fallbacks and the orientation error log are asserted in full; a failing index is a real in-memory index with one method stubbed - the read-only folder integration case asserts the whole result Ship-Check: test-audit · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed comment A rendered page sharp cannot decode now arrives wrapped, so the page failure log records the wrapped error's cause, as the tool and prompt error logs already do. Ship-Check: bug-check · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iew dates The fenced-block sentence now covers any span that includes the opening fence. daily-review answers a date like a February 30 with its own message and logs it as bad input, instead of telling the caller to pass the format it already used. Ship-Check: triage · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace all 37 loose matchers in vault-orientation-prompt.test.ts (36 toContain / not.toContain and 1 toMatch) across 17 tests with 17 whole-value toBe assertions: - 3 on the full prompt: empty vault, MEMORY_ENABLED=false, custom MEMORY_DIR - 9 on one section: stats, folders, orphans, property keys, empty memory - 5 on the go-deeper footer: orphan gating, FILE_TOOLS_ENABLED, DISABLED_TOOLS Expected text is built in the test file from the fixtures. A shared helper extracts one section or the footer and throws when it is missing. The no-orphans test now uses the same helper. In asset-operations.test.ts, the page-cap test asserts the whole pages result with toEqual, including originalBytes, title and path, and buildPageItems gets a block body. Ship-Check: test-audit · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace all 86 toContain / not.toContain matchers in daily-review-prompt.test.ts across 33 tests with 37 whole-value toBe assertions: - 6 on the full prompt: default date, the no-raw-closing-tag check, the missing-note layout, and all three READONLY_MODE cases, so no write tool can appear anywhere in them - 31 on one section or the lead line: daily note blocks (truncation, emoji, escaping), link sections, task sections in order with their metadata, the overflow hint, and the numbered review steps under each MEMORY_ENABLED and DISABLED_TOOLS setting Expected text is built in the test file from the fixtures. The section helper stops only at headings the prompt writes, so a heading inside the embedded daily note cannot end a section early. The whitespace-only note test now uses the same helper. The date-argument, error and full-output tests already asserted exact values and are unchanged. Ship-Check: test-audit · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The truncation and data-marker tests, and the two vault_patch_note DISABLED_TOOLS tests, now pin identical output, so each pair is one test whose name covers both behaviours. The injection test registers through the shared helper. Ship-Check: triage · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… own Ship-Check: pr-monitor · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ship-Check: pr-monitor · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it broken The daily review left such links out of its broken-link count but still labelled each one broken in the list. It now labels them as daily notes not created yet, and a test covers the plural summary alongside one. Ship-Check: bug-check · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failure to load pdf.js or the canvas library quoted the server's install paths to the client. It now reads 'PDF reader could not start', with the original error kept as the cause. describeErrorForLog puts the cause in the tool, prompt, page-render, rebuild and file-watcher logs alike. Ship-Check: pr-monitor · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Node caches a module whose first load failed, so only a failure after the modules load is recovered by the next call; the comment now says so. Ship-Check: pr-monitor · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ship-Check: pr-monitor · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…be helper describePromptFailure wrote the log as a side effect of building the client text, so a reader of the catch could not see the error being logged. Each prompt's catch now logs prompt_error itself and builds the client text with describeErrorRelativeTo. A test pins the cause in the prompt_error log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… failed page renders The PDF and image decode failures share one remedy, so they share one bullet. A PDF that opens but renders no page now points at a text read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…medies - A path too long for the filesystem gets its own Errors entry on the three edit tools and vault_read_file, because the caller fixes it. A save writes a temp file named after the note plus a UUID, so the edit tools' entry also covers a note that reads but cannot be saved. - vault_get_daily_note answers a well-formed date the calendar lacks with the daily-review prompt's message instead of the format hint, and lists the filesystem error it can return. - "PDF page rendering failed" no longer points back and forth with "PDF has no extractable text". - The daily-review prompt says when more than 10 notes changed that day. - describeErrorRelativeTo's doc says it rewrites any error with a string code, and the boundary comment names the one name it can misread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ow much shorter a rename must be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the shared prompt helper gone, each prompt's catch writes its own prompt_error log, but only vault-orientation's log was tested. A test now pins the error level and the wrapped cause for daily-review and memory-review too. Each catch also says again why it logs at error while tool errors log at warn: bad arguments return before the catch. Ship-Check: bug-check · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pages Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hy in two comments - daily-review: the modified-notes section is built by formatModifiedNotesSection with early returns, like the outgoing-links, backlinks, and task sections beside it. - getDailyNotePath: the calendar-date comment says the caller would resend the same date, and the catch binding is `error`, not `err`. - FILESYSTEM_ERROR_ENTRIES: the doc comment says why the rename remedy asks for a name 41 bytes shorter. Ship-Check: code-quality · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e plainly - getDailyNotePath and getDailyNote tests assert each deterministic message in full, so the shape and calendar-date refusals cannot pass for each other. - The too-long-to-save integration case gets its own variable name, so it no longer shadows the describe-level long name, and escapes it with RegExp.escape. Its title and comments say the temp file's name goes over the limit while the note itself opens unchanged. - The no-cut-notice daily-review case says why nine notes make ten. Ship-Check: code-quality · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-failure remedy The daily-note tool and the daily-review prompt now build the refusal from one function, so the two cannot drift apart. The PDF render remedy drops an "either" that could be misread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With this branch's Errors entries on top of main, the default combo holds 139,145 characters against a cap of 139,029 (4,213 per tool, 33 tools). 4,217 is the smallest allowance that passes (139,161). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r handling A promise-taking wrapper named for its failure rewording made a call site read as if the PDF result were a translation of failures. Two helpers, openPdfDocument and readPdfText, each run their pdf.js call in a plain try/catch with the same rewording, so a synchronous throw is covered too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… input Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… rebuild Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he date-message change The default combo is 139,437 characters over 33 tools once main's calendar-date Errors entries join this PR's; 4,226 is the smallest allowance that passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ
a995b42 to
958ae93
Compare
Summary
Tool errors no longer show the server's own paths or a library's internal text.
EACCES: permission denied, open '/vault/Locked.md'). The same failures now readEACCES: permission denied, open 'Locked.md'. The prompts' fallback messages get the same treatment.could not decode image (the file is empty, damaged, not an image, or over about 268 million pixels)instead of the decoder's text, which could repeat over several lines. A PDF that needs a password returnsPDF is password-protected: …, and one pdf.js cannot parse returnsPDF is damaged or not a PDF: …. When pdf.js or the canvas library fails to load, whose error quoted the server's install paths, the read returnsPDF reader could not start. The library's error is kept as the cause and goes to the server log.tool_error,prompt_error,pdf_page_render_failed, and the search index's and file watcher's unreadable-PDF logs record the full message, plus the cause of a wrapped error.Error text changed for unexpected filesystem failures. A client that matched on the absolute path, or on sharp's or pdf.js's wording, sees the new text.
Tool descriptions
vault_read_file: its Errors list quotes the new image and PDF messages in one bullet (they share a remedy), gains entries for filesystem errors, and tells a caller whose PDF opened but rendered no page to read it as text instead.vault_delete_spanandvault_replace_span: the end_anchor bullet now says a plain closing code fence cannot end a span that includes the opening fence. Every fragment of the closing fence also appears in the opening fence, andfirst_match: truewould end the span at the opening fence. It points tovault_replace_in_notefor editing a fenced block.vault_delete_span,vault_replace_spanandvault_replace_in_note: new Errors entries for filesystem errors.ENAMETOOLONG) is the caller's to fix. A save first writes a temp file named after the note plus a UUID, so a note whose name is near the filesystem's limit reads fine but fails to save with a.tmpname in the message, and the entry says to rename that note at least 41 bytes shorter. Any other code (EACCES, a full or read-only disk) is the vault owner's.vault_read_filelistsENAMETOOLONGwith "file not found", since this tool only reads and both mean nothing can be read at that path.vault_get_daily_note: new Errors entries for a date in YYYY-MM-DD form that names a day the calendar lacks, such as2026-02-30, and for a daily note whose file can't be read.The other note tools can return the same filesystem errors. Their Errors entries come with a later pass over the tool descriptions.
Size cap. The PR raises
CHARS_PER_TOOL_ALLOWANCEintool-surface-snapshot.test.tsfrom 4,213 to 4,226:Main already carries the calendar-date Errors entries on
vault_searchandvault_list_tasks. This PR's entries add 2,557 characters on top of them, which puts the default combo 408 characters over the old cap. 4,226 is the smallest allowance that passes (4,225 caps the default combo at 139,425). Nothing is trimmed in this PR; a later pass over the tool descriptions brings the total back down.Tool-definition review: Claude's ship-check tool-definition reviewer compared the changed tools against main's tool list before each push that changed their text. Each review traced every failure the tools can return, and none found a dropped fact or a change to a tool outside this PR.
ENAMETOOLONGentry missed the save-time case;vault_get_daily_notelacked its filesystem entry; and twovault_read_filebullets that shared a remedy are now one;vault_read_file's entry names a whole path that is too long, not only a name in it, and the edit tools' entry says a rename must be at least 41 bytes shorter, since a rename writes the same temp file beside its new name.Errors entries for failures every path-taking tool shares, and other description rewording on these tools, are left for a later pass over the tool descriptions.
Also in this PR
2026-02-30gets its own message and aprompt_bad_argumentlog, instead of being told to use the format it already used. A link to a daily note that doesn't exist yet reads "daily note not created yet" instead of being labelled broken, matching the broken-link count, which already left those links out. When more than 10 notes changed that day, the "Notes modified on" list now says it was cut instead of stopping at 10 with no notice.vault_get_daily_note: the same impossible date now gets the same message as the prompt, instead of "invalid date … use YYYY-MM-DD format".describe-error.ts: a space counts as the start of a path, because some Node messages leave a path unquoted. A name holding a space followed by the vault folder's own path (Docker /vault volume.mdin a vault mounted at/vault) therefore comes back rewritten in an error message. It never exposes an absolute path.pdf_page_render_failedlog.image cannot be fitted: the message now says "last attempt" instead of "smallest attempt", which is what the number always was.prompt_erroritself, then builds the client text.Testing
vault-crud-tools.tsthat both sides had reworded: the rebase keeps main's comment, which describes main's current trash behaviour. Outside test files, the diff is otherwise unchanged by the rebase.npx vitest runat 958ae93, run as an unprivileged user on Node 24.21: 117 files, 5,417 passed, 1 skipped, 2 failed. Both failures are inrender-social-preview.test.ts, which this PR does not touch: that environment's Node printed a warning about an unreadable extra CA bundle on stderr, which the test's exact stderr assertion rejects. CI'schecksjob passes on this head.server-error-contracts.test.tsdrives each tool through the failing states over HTTP and asserts the exact text the client receives. That covers:vault_read_file, and a locked daily note;vault_read_file, and a 233-character name that reads but fails to save on the three edit tools;rawon and off, and a PDF with no pages read withraw;vault_get_daily_note.npm run build,eslint,knip: clean.Review of the commits after the last live test
The commits after 33ebd80 had a bug check and a code-quality pass.
prompt_errorlog in daily-review and memory-review, and the comment on why prompts log aterroris back (f1d897d);9999-12-31fails, because the next day,+010000-01-01, is not a date the task filter accepts. The prompt degrades to its fallback message instead of throwing, which is the contract for prompts. The input is the last calendar day a four-digit year can write, so no fix was made.ENAMETOOLONGbullet quotes one message and branches its remedy on the quoted name, which the caller can see;getDailyNote, which matches on another module's message, is tracked separately.Live Test Results
Head: 958ae93
ghcr.io/aliasunder/vault-cortex:test, built from 958ae93 by Test Deploy run 38152924226 (succeeded 15:54 UTC) and serving the instance. Every "Test image" message below comes from code that exists only on this head or on main after the last release, which confirms the container was re-created from the test build.vault_searchandvault_list_tasksanswers still name only the format. The last three rows are controls: behaviour this PR must leave as it was.vault_get_daily_notewith the date2026-02-30[Error]: invalid date "2026-02-30" — use YYYY-MM-DD format (e.g. "2026-05-13")[Error]: "2026-02-30" is not a calendar date. Pass a real date in YYYY-MM-DD format.vault_read_noteon a 300-character note name[Error]: ENAMETOOLONG: name too long, open '/vault/aaa….md'[Error]: ENAMETOOLONG: name too long, open 'aaa….md'vault_searchwithfilters.modified.on: "2026-02-30"(main's date message through this PR's error boundary)[Error]: invalid modified.on date: "2026-02-30". Use YYYY-MM-DD (e.g. 2026-07-03).[Error]: modified.on date "2026-02-30" is not a calendar date. Pass a real date in YYYY-MM-DD format.vault_list_taskswithdue.on: "2026-02-30"[Error]: invalid due.on date: "2026-02-30". Use YYYY-MM-DD (e.g. 2026-07-03).[Error]: due.on date "2026-02-30" is not a calendar date. Pass a real date in YYYY-MM-DD format.vault_write_notetoProbe Lane E/target.md/child.md, wheretarget.mdis a note[Error]: EEXIST: file already exists, mkdir '/vault/Probe Lane E/target.md'[Error]: EEXIST: file already exists, mkdir 'Probe Lane E/target.md'vault_read_noteon the absolute path/vault/Probe Lane E/absolute.md(control)[Error]: absolute path blocked: "/vault/Probe Lane E/absolute.md" must be vault-relativevault_list_taskswithdone.on: "2026-10-11"on a task board (control)vault_search_by_tagforsession-log, limit 2 (control)tagsonly, and neither repeats them inadditional_propertiesThe throwaway note
Probe Lane E/target.mdwas created for each through-a-note probe and deleted after it, with its empty folder; nothing was written under it. A note whose name reads but is too long to save could not be probed live, because no tool can create one; the integration tests cover it. Image and PDF failures were not probed live, because no tool can write a binary file into the vault; the integration tests cover them over HTTP.An earlier live test, at a995b42 before the rebase (Test Deploy run 38021425382), ran the same filesystem probes with the same results.
🤖 Generated with Claude Code