fix(agent): report load_skill truncation instead of silently dropping content - #4167
Open
jkeskikangas wants to merge 1 commit into
Open
fix(agent): report load_skill truncation instead of silently dropping content#4167jkeskikangas wants to merge 1 commit into
jkeskikangas wants to merge 1 commit into
Conversation
… content `load_skill` capped output at MAX_SKILL_BODY_BYTES and returned the head as if it were complete — no log, no marker, `is_error: false`. A skill whose tail held its output format or refusal rules loaded "successfully" and then behaved subtly wrong, with nothing anywhere saying why. Both call sites (SKILL.md and supporting files) now go through one helper that appends `[truncated: N of M bytes shown; ...]` and emits a `tracing::warn!` naming the requested skill. The marker is charged against the cap rather than appended past it, so the existing "output stays within MAX_SKILL_BODY_BYTES" tests still hold. Fixes block#4163 Signed-off-by: Jarno Keskikangas <jarno.keskikangas@gmail.com>
jkeskikangas
force-pushed
the
fix/load-skill-truncation-marker
branch
from
August 1, 2026 20:02
2260bdc to
3847c61
Compare
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.
Summary
load_skillcapped its output atMAX_SKILL_BODY_BYTES(32 KiB) and returned the head as if it were the whole thing — no log, no marker,is_error: false. Since the tail of a SKILL.md is usually where the output format, refusal rules, and verification checklist live, an oversized skill loads "successfully" and then behaves subtly wrong, and nothing anywhere says why.Both truncating call sites in
crates/buzz-agent/src/builtin.rsnow go through one helper that appends[truncated: N of M bytes shown; the remainder was dropped by load_skill]and emits atracing::warn!naming the requested skill. Behaviour is otherwise unchanged: content under the cap is returned byte-identical.Related issue
Fixes #4163.
Duplicate search: none found. No open or closed PR mentions
load_skill,MAX_SKILL_BODY_BYTES, or skill truncation. The nearest neighbours are all unrelated: #3532 (fix/pack-validate-skill-metadata, validation of skill metadata at pack time), #4027 (Windows skill symlinks), #2525 (skill directory rename). #3262 (draft, swapping the agent loop onto goose) touchesbuiltin.rsbut not this path.Decisions
Three calls worth flagging, since the issue left them open:
The marker is charged against the cap, not appended past it. The patch sketched in the issue appends after truncating, so the result exceeds 32 KiB. Two things argue against that here:
truncate_middleinmcp.rsalready reservesELISION_MARKER_ALLOWANCEinside its own budget, so reserving is the house pattern; and the existing testscall_load_skill_truncates_large_body/call_load_skill_truncates_large_supporting_fileboth asserttext.len() <= MAX_SKILL_BODY_BYTES, which the issue's version would break. The cap is a context budget, so it should stay one. Edge case: if the limit is ever smaller than the reserved allowance, the marker still wins and the result overruns — reporting the loss beats honouring a budget too small to report it in. That is unreachable at 32 KiB but the helper is total, and there's a test pinning it.Kept tail-truncation rather than switching to
truncate_middle. Middle-elision would preserve the tail, which for a skill body is often the part that matters most — a real argument, and it's a one-line change if you'd rather have it. I left it because it's a behaviour change beyond what the issue reports, and because the failure modes differ: a head cut produces a document that visibly stops, whereas a middle elision produces one that reads as intact at both ends while the procedure the tail's rules refer to is gone.truncate_middle's own doc comment justifies itself for tool output ("test summaries, error trailers"), which is a different shape of text from an authored document. The bug filed is "silent", not "wrong half" — fixing the reporting first leaves the choice of which half survives as a clean, separate decision.Both call sites, one helper. The supporting-file path had the identical defect and is easy to miss, so the marker construction lives in
truncate_with_markerand both sites call it. Thetracing::warn!logs the name as requested (my-skillormy-skill/references/foo.md), which is the string the caller actually passed.Testing
cargo test -p buzz-agent— 389 unit + 85 integration tests pass, including the two pre-existing cap assertions.cargo fmt --checkandcargo clippy --all-targets --all-features -- -D warningsclean on the crate. I scoped tobuzz-agentrather than runningjust ci; the change is confined to one file in that crate.New tests, matching the style of
truncate_middle_respects_max_and_boundaries:truncate_with_marker_reports_accurate_byte_counts— marker fits inside the cap, and its two counts match the kept length and the pre-cut length.truncate_with_marker_leaves_content_under_the_limit_byte_identical— no marker, byte-identical passthrough.truncate_with_marker_keeps_valid_utf8_on_a_multibyte_cut—"é".repeat(60_000)at odd limits, so the cut lands mid-character.truncate_with_marker_keeps_the_marker_when_the_limit_cannot_hold_it— pins the edge case in decision 1.call_load_skill_marks_truncated_body— end-to-end on the SKILL.md path; a## SENTINELtail is dropped and the marker reports it. Fails onmain.call_load_skill_marks_truncated_supporting_file— same for the supporting-file path. Fails onmain.call_load_skill_does_not_mark_body_under_the_limit— no marker on a normal skill.What I could not test: I have no model credential wired to this checkout, so I have not observed an agent reading the marker in a live session — same caveat as the issue. The change is covered by the unit tests above and by reading the call path; I have not run a real
load_skillround-trip against a provider.Scope
load_skillonly.MAX_HINTS_BYTESand theAGENTS.mdchain inhints.rshave the same shape and are deliberately left alone — the issue names that as a separate, milder follow-up (the chain is additive rather than a single authored document), and a focused diff reviews faster. Happy to do it in a second PR if you want it. No UI change, so no screenshots.