adk-go
adk-go copied to clipboard
fix: prevent panic when loadartifactstool used without artifact service
Fixes #283
Root Cause
internalArtifacts wrapper created with nil embedded Artifacts field. Calling List(), Load(), or Save() on nil receiver caused segfault.
Changes
internal/toolinternal/context.go: Added nil checks toList/Load/Savemethods, only create wrapper if service configuredtool/loadartifactstool/load_artifacts_tool_test.go: Test for error when service missinginternal/toolinternal/context_test.go: Tests for nil-safe methods
Result
Before: panic: runtime error: invalid memory address or nil pointer dereference
After: ERROR: failed to list artifacts: artifact service not configured
Summary of Changes
Hello @ShammiAnand, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves a critical issue where the loadartifactstool could lead to a runtime panic if it attempted to operate without a configured artifact service. The changes introduce robust checks and conditional logic to ensure that any operations on a missing service now result in a clear error message, significantly improving the stability and predictability of the tool's behavior.
Highlights
- Panic Prevention: Implemented nil checks within the
internalArtifactswrapper'sList,Load, andSavemethods to prevent runtime panics when the underlying artifact service is not configured. - Conditional Initialization: Modified the
NewToolContextfunction to conditionally initialize theinternalArtifactswrapper only if an artifact service is present, avoiding the creation of a wrapper with a nil embeddedArtifactsfield. - Enhanced Test Coverage: Added new unit tests (
TestInternalArtifacts_NilSafeandTestLoadArtifactsTool_ProcessRequest_NoArtifactService) to specifically verify the graceful error handling and nil-safe behavior when the artifact service is unavailable.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.
@baptmont can you please review this?
I would think it should be possible to catch this sooner, like when the agent is configured or starts up... run a few sanity checks
Right now this feels more like a bandaid, catching at the point of failure artifact.Load/Save/List. Better than it is today, but not really solving the issue of misconfiguration until I actually try to use the artifact service
@verdverm
Added runner-level validation in runner.New() that:
- Walks entire agent tree checking for load_artifacts tool
- Returns error immediately if tool present but ArtifactService not configured
- Validates during runner creation (proactive) vs at tool invocation (reactive)
Before: Error at runtime when tool called panic: runtime error: invalid memory address or nil pointer dereference
After: Error at runner creation Failed to create runner: agent "artifact_tester" uses load_artifacts tool but ArtifactService not configured in runner
@verdverm can we merge it?
@ShammiAnand I have no control over that, just an ADK user
@ShammiAnand well, this is less likely to be merged with ~1.5M LOC being contributed
what is going on with dd35910
@ShammiAnand, why do you need vendor/... directory? It adds huge amount of code, and I think your intention was just to modify 5 files...
+1, please drop vendor directory.
@kdroste-google @mazas-google done!
I defined the Validator interface and ValidationConfig struct in the agent package, which serves as the common definition:
// agent/agent.go
type ValidationConfig struct {
HasArtifactService bool
HasMemoryService bool
}
type Validator interface {
Validate(ValidationConfig) error
}
- Agents: The agent struct implements this interface by delegating to the
Validatefunction provided in itsConfig. - Tools: Tools (like
load_artifacts_tool) import the agent package and implement theValidate(agent.ValidationConfig) errormethod directly.
The runner then checks for this single interface on both entities during initialization.
@kdroste-google