Skip to content

tool/loadartifactstool: return error instead of panic when artifact service is not configured - #1390

Open
GerardGao wants to merge 1 commit into
google:mainfrom
GerardGao:fix/loadartifacts-nil-panic
Open

tool/loadartifactstool: return error instead of panic when artifact service is not configured#1390
GerardGao wants to merge 1 commit into
google:mainfrom
GerardGao:fix/loadartifacts-nil-panic

Conversation

@GerardGao

Copy link
Copy Markdown

Fixes #283

What changed

  • loadartifactstool now returns a descriptive error ("load_artifacts tool requires an artifact service to be configured") from ProcessRequest when no artifact service is configured, instead of panicking with a nil pointer dereference.
  • agent.NewToolContext and agent.NewCallbackContextWithArtifactTracking no longer wrap a nil Artifacts in trackedArtifacts, so "no artifact service configured" remains observable as a nil Artifacts instead of panicking on the first promoted method call.

Why

When the load_artifacts tool is registered but no artifact service is configured, the agent panics during request preprocessing (nil pointer dereference in trackedArtifacts.List, previously internalArtifacts.List — see #283). A configuration error should be reported as an error instead of crashing the process.

Testing plan

  • Added TestLoadArtifactsTool_ProcessRequest_NilArtifacts: constructs a context with no artifact service and asserts ProcessRequest returns an error mentioning the artifact service (this test panicked before the fix).
  • go test ./agent/... ./tool/... ./runner/... — all pass.
  • gofmt -l clean.

@google-cla

google-cla Bot commented Aug 23, 2026

Copy link
Copy Markdown

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.

@GerardGao

Copy link
Copy Markdown
Author

I signed it!

@GerardGao
GerardGao force-pushed the fix/loadartifacts-nil-panic branch from ef62a47 to e538345 Compare August 25, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panic when Artifact tool is not properly setup

1 participant