Skip to content

fix: escape ${expression} in formatString description to prevent ADK KeyError#1409

Open
brucearctor wants to merge 1 commit into
google:mainfrom
azra-ai-oss:fix/escape-expression-template-variable
Open

fix: escape ${expression} in formatString description to prevent ADK KeyError#1409
brucearctor wants to merge 1 commit into
google:mainfrom
azra-ai-oss:fix/escape-expression-template-variable

Conversation

@brucearctor
Copy link
Copy Markdown

Summary

Fixes #1388

The formatString function description in basic_catalog.json contained the literal text ${expression}, which the ADK's inject_session_state() matched as a template variable. Since expression passes Python's str.isidentifier() check, the ADK raised a KeyError when it couldn't find it in the session state.

Changes

Changed ${expression}${<expression>} in the formatString description for both v0_9 and v0_10 catalog schemas.

The angle brackets cause <expression> to fail the isidentifier() check, so the ADK's _is_valid_state_name() returns False and the match is returned as-is (line 108 of instructions_utils.py).

Why only ${expression} was affected

The other interpolation examples in the same description were already safe:

  • ${/absolute/path}/absolute/path is not a valid identifier (contains /)
  • ${now()}now() is not a valid identifier (contains ())
  • ${relative/path}relative/path is not a valid identifier (contains /)

Only expression was a bare, valid Python identifier.

Files changed

  • specification/v0_9/json/basic_catalog.json
  • specification/v0_10/json/basic_catalog.json

…KeyError

The formatString function description in basic_catalog.json contained
the literal text ${expression}, which the ADK's inject_session_state()
regex (r'{+[^{}]*}+') matched as a template variable. Since 'expression'
passes Python's str.isidentifier() check, the ADK attempted to look it
up in the session state dict and raised a KeyError when not found.

Changed ${expression} to ${<expression>} so that '<expression>' fails
the isidentifier() check, causing the ADK to skip it (returning the
match as-is). The other interpolation examples in the description
(${/absolute/path}, ${now()}, etc.) were already safe because they
contain characters (/, parentheses) that make them invalid identifiers.

Fixes google#1388
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the description of the formatString object in the JSON specifications for versions 0.9 and 0.10, changing the placeholder format from ${expression} to ${} to improve clarity. I have no feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

adk session: KeyError: 'Context variable not found: expression' when using adk run with A2UI agents

1 participant