fix(skills): preserve cache points in system prompt during skills inj…

[mattdai01]

AgentSkills._on_before_invocation was reading the system prompt as a string, appending skills XML, and writing it back as a string. This caused the setter to collapse structured SystemContentBlock lists (including cachePoint blocks) into a single flat text block.

The hook now operates on the content block list when available, appending skills XML as a separate block and leaving cache points untouched. Falls back to the original string path when _system_prompt_content is None.

Description

AgentSkills._on_before_invocation was reading the system prompt as a string, appending skills XML, and writing it back as a string. This caused the setter to collapse structured SystemContentBlock lists (including cachePoint blocks) into a single flat text block.

The hook now operates on the content block list when available, appending skills XML as a separate block and leaving cache points untouched. Falls back to the original string path when _system_prompt_content is None.

Related Issues

Fixes #2133

Type of Change

Bug fix

Testing

Created new unit test

• [X ] I ran hatch run prepare

Checklist

• [ X] I have read the CONTRIBUTING document
• [ X] I have added any necessary tests that prove my fix is effective or my feature works
• [ X] I have updated the documentation accordingly
• [ X] I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• [ X] My changes generate no new warnings
• [ X] Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[gfrid]

this is very critical bug can someone please merge it, strands + bedrock agents are getting stuck.

[mkmeral]

/strands review

[github-actions]

Assessment: Comment

The fix correctly addresses the cache point destruction bug by branching into content-block-level manipulation when _system_prompt_content is available, preserving structured blocks like cachePoint. The fallback to string manipulation for None prompts maintains backward compatibility. Tests are well-structured and cover the key scenarios (preservation, no-accumulation, warning path).

Review Details

• Private attribute access: The main concern (already flagged by @mkmeral) is that agent._system_prompt_content is accessed directly at line 158. This couples the plugin to Agent internals. A public system_prompt_content getter on Agent would be the proper fix — the setter already accepts list[SystemContentBlock], so having a matching getter maintains API symmetry. Consider adding this as a follow-up or as part of this PR.
• Testing: Good coverage — cache point preservation, idempotent re-invocation, warning path, and mock updated to handle list-based system prompts.

[mkmeral]

/strands review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

Assessment: Comment

The bug fix correctly preserves structured system prompt blocks (including cachePoint) during skills injection, and the addition of the public system_prompt_content getter addresses the previous review feedback about accessing private internals. The core logic is sound, and the test coverage for the skills plugin is solid.

Review Details

• API safety: The new system_prompt_content property returns a direct mutable reference to internal state. Consider returning a defensive copy to prevent accidental mutation by callers.
• Test coverage gap: The new public Agent.system_prompt_content property has no dedicated tests in test_agent.py — only tested indirectly through the skills plugin mock.
• Docstring consistency: Minor — the new property is missing a structured Returns: section that adjacent properties include.

The previous review feedback on private attribute access has been properly addressed. Nice work on the fix.

[github-actions]

Assessment: Comment

All prior review feedback has been addressed. The fix is solid — one minor test accuracy issue remains.

Review Details

• Prior feedback resolved: Defensive copy ✅, Returns: docstring ✅, dedicated agent tests ✅, public getter instead of private access ✅
• Test accuracy: test_system_prompt_content_structured uses cache_control instead of the correct cachePoint key from SystemContentBlock — the test passes incidentally but doesn't validate what it describes. See inline comment.
• Coverage: Codecov shows 80% patch coverage — the uncovered lines are in the string fallback path (lines 175-179), which is a legacy edge case for system_prompt=None. Acceptable for a bug fix PR.

The implementation is clean and the core bug is well-fixed. Nice iteration on the feedback.

[github-actions]

Assessment: Approve

All prior review feedback has been addressed across 4 iterations. The fix is clean, well-tested, and ready to merge.

Review Details

• Bug fix: Content-block path correctly preserves cachePoint and other structured blocks during skills XML injection; string fallback path maintained for the system_prompt=None edge case.
• New public API: Agent.system_prompt_content property provides read-only access to structured content blocks with a defensive copy — complements the existing setter symmetrically.
• Test coverage: Codecov at 100%. Comprehensive tests cover cache point preservation, idempotent re-invocation, both warning paths, string fallback replacement, defensive copy, and dedicated property tests.
• All 7 prior review items resolved: Public getter ✅, defensive copy ✅, docstring Returns: ✅, cachePoint key fix ✅, agent property tests ✅, string path coverage ✅.

[mkmeral]

@mattdai01 thank you for your contribution!