fix: escape curly braces in agent instructions to prevent context variable errors

[fl-sean03]

Summary

Fixes #1382

When an agent's instructions contain curly braces (e.g., {repo}, {branch}), ADK's inject_session_state tries to resolve them as context variable references. Since kagent doesn't use ADK context variables, this causes a KeyError.

Root cause: ADK's LlmAgent.canonical_instruction() returns bypass_state_injection=False when instruction is a string, triggering inject_session_state which uses regex {+[^{}]*}+ to find and resolve {text} patterns as state variables.

Fix: Wrap the instruction string in a callable (InstructionProvider) before passing to the ADK Agent constructor. When instruction is callable, ADK sets bypass_state_injection=True, completely skipping the template processing. The _configure_memory method is also updated to properly handle callable instructions when appending memory usage instructions.

Changes

• python/packages/kagent-adk/src/kagent/adk/types.py: Wrap self.instruction in an InstructionProvider callable in to_agent(). Update _configure_memory() to compose callables when appending memory instructions.
• python/packages/kagent-adk/tests/unittests/test_instruction_escaping.py: 7 unit tests covering curly braces, JSON-like content, nested braces, empty instructions, and verifying the callable bypass mechanism.

Test plan

• All 7 new unit tests pass
• Existing test suite unaffected (pre-existing env import issues for other tests are unrelated)
• CI pipeline validation

Signed-off-by: opspawn agent@opspawn.com

[Copilot]

Pull request overview

Disables ADK session-state template injection for kagent agent instructions to prevent {...} braces in prompts from being interpreted as missing context variables (KeyError), and adds unit tests around instruction handling.

Changes:

• Wrap AgentConfig.instruction in a callable when constructing the ADK Agent to bypass ADK state injection.
• Update memory-instruction appending logic to compose instruction callables instead of string-concatenating.
• Add unit tests intended to cover curly-brace and JSON-like instruction content.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
python/packages/kagent-adk/src/kagent/adk/types.py	Wraps instructions in a provider callable and updates memory instruction composition to support callable instructions.
python/packages/kagent-adk/tests/unittests/test_instruction_escaping.py	Adds tests for curly-brace-containing instructions and verifies instructions are callable and preserved.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

These tests don’t currently exercise the ADK code path that was failing (inject_session_state / canonical_instruction template processing). Creating the agent and calling agent.instruction(mock_ctx) will likely pass even with a raw-string instruction, so the suite may not catch regressions. Consider invoking the ADK method that canonicalizes/injects state (or otherwise reproducing the original KeyError) and asserting it returns the unmodified instruction without raising.

[Copilot]

_configure_memory defines a nested function immediately after the if callable(prev_instruction): line. With the repo's Ruff/pycodestyle rules (E306), nested def blocks should be preceded by a blank line; otherwise linting can fail. Add the expected blank line (or refactor to avoid the nested def) before defining _with_memory.

Suggested change

	if callable(prev_instruction):
	if callable(prev_instruction):

[Copilot]

The comment says this imports types “directly (not via kagent.adk) to avoid init side effects”, but import kagent.adk.types still executes kagent/adk/__init__.py first (which imports _a2a, etc.). Either adjust the comment to reflect that the test relies on stubbing to make the package importable, or switch to a true direct module load (e.g., via importlib.util.spec_from_file_location) if avoiding __init__ execution is important.

Suggested change

	# Import types directly (not via kagent.adk) to avoid __init__ side effects
	# Import types via kagent.adk; __init__ side effects are neutralized by
	# the _isolate_types_import fixture stubbing heavy dependencies.

[supreme-gg-gg]

Perhaps using this new field in ADK will solve the issue, I believe it would not cause any regression either:

static_instruction: Optional[types.ContentUnion] = Field(
      default=None,
      description=(
          'Optional. LlmAgent.static_instruction. Static content sent literally'
          ' at position 0 without placeholder processing. When set, changes'
          ' instruction behavior to go to user content instead of'
          ' system_instruction. Supports context caching. Accepts'
          ' types.ContentUnion (str, types.Content, types.Part,'
          ' PIL.Image.Image, types.File, or list[PartUnion]).'
      ),
  )

[fl-sean03]

@supreme-gg-gg Thanks for the suggestion! I looked into static_instruction — it's an interesting field but it wouldn't fully solve this issue on its own.

The key problem: static_instruction changes where the instruction content goes (from system_instruction to user content), but ADK still runs _process_agent_instruction() on the instruction field, which calls canonical_instruction() → inject_session_state(). Since a raw string instruction returns bypass_state_injection=False, the {curly_brace} KeyError would still occur.

The callable approach directly sets bypass_state_injection=True via ADK's own mechanism — when instruction is callable, canonical_instruction() returns (text, True), completely skipping state injection. This is simpler (one wrapping layer vs two-step setup) and semantically correct for kagent's use case since we don't use ADK context variables at all.

Rebased against main to resolve the merge conflicts. The Copilot review feedback on tests and linting has been addressed in the follow-up commits.

[supreme-gg-gg]

So I looked deeper and that doesn't seem to be the case, I think static instructions are not preprocessed:

Static instruction content sent literally as system instruction at the beginning....This field is for content that never changes and doesn't contain placeholders. It's sent directly to the model without any processing or variable substitution.

in llm_agent.py

and also in the code in instructions.py. I think this pattern aligns better with how ADK is designed, but yea indeed either works

[fl-sean03]

@supreme-gg-gg You're absolutely right — I dug deeper into the ADK source and static_instruction is the correct approach here. Looking at instructions.py, static_instruction goes through _transformers.t_content() directly without any call to _process_agent_instruction() or inject_session_state(), so it completely bypasses placeholder processing.

I've updated the PR to use static_instruction instead of the callable wrapper:

• Agent instructions are now passed as static_instruction (sent literally, no template substitution)
• The memory suffix (which has no curly braces) goes into the instruction field
• This is simpler and aligns with how ADK is designed

Thanks for pushing back on this — much cleaner solution.

[fl-sean03]

Thanks for digging into the ADK source — you're right that static_instruction bypasses the template preprocessing entirely (instructions.py doesn't touch it). I've updated the PR to use this approach: memory instructions now append to static_instruction instead of setting agent.instruction separately.

All 21 CI checks pass including both e2e suites (sqlite + postgres). Would you be open to approving this? 🙏