Skip to content

fix(agent): report load_skill truncation instead of silently dropping content - #4167

Open
jkeskikangas wants to merge 1 commit into
block:mainfrom
jkeskikangas:fix/load-skill-truncation-marker
Open

fix(agent): report load_skill truncation instead of silently dropping content#4167
jkeskikangas wants to merge 1 commit into
block:mainfrom
jkeskikangas:fix/load-skill-truncation-marker

Conversation

@jkeskikangas

Copy link
Copy Markdown

Summary

load_skill capped its output at MAX_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.rs now go through one helper that appends [truncated: N of M bytes shown; the remainder was dropped by load_skill] and emits a tracing::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) touches builtin.rs but not this path.

Decisions

Three calls worth flagging, since the issue left them open:

  1. 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_middle in mcp.rs already reserves ELISION_MARKER_ALLOWANCE inside its own budget, so reserving is the house pattern; and the existing tests call_load_skill_truncates_large_body / call_load_skill_truncates_large_supporting_file both assert text.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.

  2. 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.

  3. 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_marker and both sites call it. The tracing::warn! logs the name as requested (my-skill or my-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 --check and cargo clippy --all-targets --all-features -- -D warnings clean on the crate. I scoped to buzz-agent rather than running just 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 ## SENTINEL tail is dropped and the marker reports it. Fails on main.
  • call_load_skill_marks_truncated_supporting_file — same for the supporting-file path. Fails on main.
  • 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_skill round-trip against a provider.

Scope

load_skill only. MAX_HINTS_BYTES and the AGENTS.md chain in hints.rs have 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.

@jkeskikangas
jkeskikangas requested a review from a team as a code owner August 1, 2026 19:52
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

load_skill silently truncates skill content over 32 KiB with no warning, log, or marker

1 participant