feat(skills): expose additional tools after skill activation by wangjl1993 · Pull Request #94 · google/adk-kotlin

wangjl1993 · 2026-06-18T07:46:50Z

Aligns Kotlin SkillToolset with adk-python's "load_skill activates additional tools" behavior.

Skills can declare additional tool names in frontmatter via metadata.adk_additional_tools. These tools remain hidden from the LLM until the model calls load_skill for that skill.
Activation state is recorded per-agent under _adk_activated_skill_{agent} and re-read on each LLM step, so subsequent requests expose the expanded toolset.

This also:

Preserves metadata.adk_additional_tools in loaded/listed frontmatter responses.
Validates metadata.adk_additional_tools as a list of non-blank strings.
Supports resolving activated tools from both provided tools and provided toolsets.
Adds parser and toolset tests for the activation flow.

Validation:

./gradlew -Pkotlin.daemon.jvmargs="-Xmx4g" --no-daemon --stacktrace build

google-cla · 2026-06-18T07:47:14Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

damianmomotgoogle · 2026-06-22T14:38:38Z

Thx for contribution! We need a bit of time to verify how it will co-work with just introduced context compaction.

In the mean time can you sync it to HEAD revision? Thx!

wangjl1993 · 2026-06-24T06:22:01Z

Thx for contribution! We need a bit of time to verify how it will co-work with just introduced context compaction.

In the mean time can you sync it to HEAD revision? Thx!

Synced the PR to latest main HEAD and verified with full build:

GRADLE_OPTS="-Xmx4g -XX:MaxMetaspaceSize=1g -Dorg.gradle.daemon=false" JAVA_HOME=/usr/lib/jvm/java-21-openjdk-amd64 ./gradlew -Pkotlin.daemon.jvmargs="-Xmx4g"
--no-daemon --stacktrace build

BUILD SUCCESSFUL in 2m 18s

The skill activation state is stored in session state and re-read when toolsets build tools for each LLM step, so it should survive context compaction.

krwc · 2026-06-26T13:17:07Z

+        ?: throw SkillSourceException(
+          "Skill $skillName is malformed: metadata.${Frontmatter.ADK_ADDITIONAL_TOOLS_METADATA_KEY} must be a list of strings"
+        )
+    text.trim().takeIf { it.isNotEmpty() }


Why do we trim this? IMO the name with the blank characters is wrong to begin with, but let the models decide and keep the name exactly as provided.

krwc · 2026-06-26T14:28:07Z

  val allowedTools: String? = null,
-  val metadata: Map<String, String> = emptyMap(),
+  val metadata: Map<String, Any?> = emptyMap(),
+  val adkAdditionalTools: List<String>? = null,


This doesn't belong to the frontmatter. If you look at the spec, it specifies a fixed set of fields https://agentskills.io/specification#frontmatter

krwc · 2026-06-26T14:35:47Z

+  val metadata: Map<String, Any?> = emptyMap(),
+  val adkAdditionalTools: List<String>? = null,
 ) {
+  constructor(


This constructor is merely for convenience and should be deleted at some point. Since we agreed to break the API (it's a moderately small change and easy to fix at version bump), we probably shouldn't introduce this constructor at all.

krwc · 2026-06-26T14:37:59Z

      "compatibility must not exceed 500 characters"
    }
+    require(adkAdditionalTools?.all { it.isNotBlank() } ?: true) {
+      "adkAdditionalTools must not contain blank tool names"


Again, I am not sure why we're so fixed on the blank characters. ADK Python doesn't do any validation there, let's keep it consistent for now.

SG, thanks for the review. I’ll wait for your metadata change, then rebase and adjust this PR to keep adk_additional_tools under metadata only, without adding a
top-level Frontmatter field or extra constructor. I’ll also remove the trimming / blank-name validation to stay consistent with ADK Python.

Ack, please sync to HEAD now.

Ack, please sync to HEAD now.

Synced to latest HEAD and updated the implementation based on the review feedback.

Changes:

Kept adk_additional_tools under frontmatter metadata only.

Removed the top-level Frontmatter.adkAdditionalTools API addition.

Removed extra blank-name validation / trimming to stay aligned with ADK Python.

SkillToolset now reads additional tool names directly from metadata when resolving activated skills.

Validated locally with full build successfully.

krwc

I did a

krwc · 2026-07-02T11:53:19Z

+    val applied = (context.context.state[stateKey] as? List<*>).orEmpty()
+    val combined =
+      (applied + pending + skillName)
+        .mapNotNull { it?.toString()?.takeIf(String::isNotBlank) }


Here and in a few more places you're still filtering blanks. Why?

krwc · 2026-07-02T12:04:18Z


 /** Toolset that manages and provides access to a collection of [Skill]s. */
-class SkillToolset(internal val source: SkillSource) : Toolset {
+class SkillToolset(


At the moment Python ADK presents an approach to Skill different from Kotlin/Java/Go ADK. It's not clear how we want to resolve those discrepancies yet. And while I acknowledge that adk_additional_tools are useful, it may happen to be replaced with something else (script execution for example / environments API). As such I'd treat it as experimental feature with no guarantees of stability.

Because of that, if you want to pursue this, I'd like to propose a middle ground, that is: we create a dedicated constructor which is clearly marked as experimental and requires explicit opt-in:

@RequiresOptIn(message = "Skill-provided additional tools are experimental and may change or be removed.") @Retention(AnnotationRetention.BINARY) annotation class ExperimentalSkillTools class SkillToolset @ExperimentalSkillTools constructor( internal val source: SkillSource, additionalTools: List<BaseTool>, additionalToolsets: List<Toolset>, ) : Toolset { // no opt-in needed by callers of the plain form @OptIn(ExperimentalSkillTools::class) constructor(source: SkillSource) : this(source, emptyList(), emptyList()) @ExperimentalSkillTools constructor(source: SkillSource, additionalTools: List<BaseTool>) : this(source, additionalTools, emptyList()) ... }

On the other hand, if at some point we decide this is what we want, we can stabilize it, dropping the OptIns.

krwc · 2026-07-02T12:10:49Z

+    tools.forEach { it.close() }
+    providedToolsByName.values.forEach { it.close() }
+    providedToolsets.forEach { it.close() }


Are we sure we don't over-close something here? Let's have a comprehensive test to ensure that.

Also it should be made clear in the constructor KDoc that the provided tools are going to be "owned" by the SkillToolset and that it will call close() on them.

krwc · 2026-07-02T12:22:33Z

+    val additionalToolNames = LinkedHashSet<String>()
+    for (skillName in skillNames) {
+      val frontmatter =
+        source.loadFrontmatter(skillName).getOrElse { e ->


I'm honestly not sure if we should reload frontmatter on each turn. I mean - if the skill is already loaded into context, it means it won't/shouldn't change unless the model decided to re-load it for some reason.

In other words, when the skill gets loaded, we should persist the list of associated additional tools names in the state and use it here.

Thoughts?

krwc · 2026-07-02T12:27:54Z

+        .mapNotNull { it?.toString()?.takeIf(String::isNotBlank) }
+    if (skillNames.isEmpty()) return emptyList()
+
+    val existingNames = tools.mapTo(mutableSetOf()) { it.name }


nit: I'd rename to forbiddenToolNames to better indicate this is about SkillToolset defined tools which cannot be overridden. We should also promote it to a private getter for clarity.

krwc · 2026-07-02T12:47:19Z

+        candidateTools[tool.name] = tool
+      }
+    }
+    return candidateTools


Is this just:

private suspend fun getCandidateTools(readonlyContext: ReadonlyContext): Map<String, BaseTool> = providedToolsByName + providedToolsets.flatMap { it.getTools(readonlyContext) }.associateBy { it.name }

?

krwc · 2026-07-02T12:52:43Z

@@ -16,9 +16,14 @@

 package com.google.adk.kt.tools


I think we should have an e2e test which uses InMemoryRunner, InMemorySession and mocked model to ensure that:

In turn 1, skill is loaded with additional tools, and the names are stored in the session.

In turn 2: the tools actually injected correctly into the model request based on the names stored in session state.

krwc · 2026-07-02T12:55:43Z

+  internal val providedToolsByName: Map<String, BaseTool> =
+    run {
+      val byName = LinkedHashMap<String, BaseTool>()
+      for (tool in additionalTools) {
+        val previous = byName.put(tool.name, tool)
+        if (previous != null) {
+          logger.warn { "Duplicate additional tool name '${tool.name}'; last one wins." }
+        }
+      }
+      byName
+    }


More idiomatic would be

internal val providedToolsByName: Map<String, BaseTool> = additionalTools.associateBy { it.name }.also { additionalTools.groupingBy { it.name }.eachCount() .forEach { (name, count) -> if (count > 1) logger.warn { "Duplicate additional tool name '$name'; last one wins." } } }

krwc · 2026-07-02T13:00:10Z

+    for (toolName in additionalToolNames) {
+      if (toolName in existingNames) {
+        logger.warn {
+          "Tool name collision: additional tool '$toolName' shadows a core skill tool; skipping."


BTW, what is the strategy of dealing with a skill whose additional tool name overrides some other tool installed by the Agent? I think this shouldn't be allowed, but I don't immediately see a simple way to validate against and prevent that. Thoughts?

krwc · 2026-07-02T13:17:00Z

+   * for serial `load_skill` calls across steps.
+   */
+  private fun recordSkillActivation(context: ToolContext, skillName: String) {
+    val agentName = context.context.agentName


Nit: this variable is unnecessary. It's used in the very next line.

The PR has been updated to the latest revision.

The latest revision:

preserves the existing Toolset.processLlmRequest() contract;

snapshots the owned tool and toolset collections and stores activated tool
names per skill;

applies first-wins handling to direct tools and tools returned by
Toolset.getTools();

synchronizes tools injected by earlier Toolset.processLlmRequest() hooks
before registering later tools, preventing duplicate function declarations
when an activated skill exposes the same tool name;

adds E2E coverage for model-visible function declarations, toolsDict
contents, and the selected execution target.

I also reran the complete core JVM test suite with --rerun-tasks and the full
Gradle build. Both passed successfully. The full build reported 295 actionable
tasks.

Could you please take another look? Thanks!

Align SkillToolset with adk-python by activating tools declared in metadata.adk_additional_tools when load_skill succeeds. Store activated tool names per skill and agent in session state so subsequent LLM requests expose only activated tools without reloading frontmatter. Synchronize request-injected tools into a first-wins registry before registering later toolset tools. Mark the API experimental and cover activation replacement, name collisions, resource ownership, and model request declarations.

wangjl1993 force-pushed the feat/skill-activation-tools branch 3 times, most recently from 64a95df to f4dac0a Compare June 22, 2026 02:58

damianmomotgoogle requested a review from wikaaaaa June 22, 2026 14:34

wangjl1993 force-pushed the feat/skill-activation-tools branch from f4dac0a to e0695bd Compare June 24, 2026 06:01

krwc requested review from krwc and removed request for wikaaaaa June 26, 2026 07:38

krwc reviewed Jun 26, 2026

View reviewed changes

Comment thread core/src/commonMain/kotlin/com/google/adk/kt/skills/Frontmatter.kt

krwc reviewed Jun 26, 2026

View reviewed changes

wangjl1993 force-pushed the feat/skill-activation-tools branch from e0695bd to 308550d Compare July 2, 2026 09:25

krwc requested changes Jul 2, 2026

View reviewed changes

wangjl1993 force-pushed the feat/skill-activation-tools branch from 308550d to 1b933b2 Compare July 16, 2026 03:24

Uh oh!

Conversation

wangjl1993 commented Jun 18, 2026

Uh oh!

google-cla Bot commented Jun 18, 2026

Uh oh!

damianmomotgoogle commented Jun 22, 2026

Uh oh!

wangjl1993 commented Jun 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krwc Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krwc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krwc Jun 26, 2026 •

edited

Loading