Skip to content

chore: upgrade to upstream google/adk-go v1.5.0 (true merge)#50

Merged
marconn merged 87 commits into
mainfrom
upgrade/v1.5.0-live
Jul 13, 2026
Merged

chore: upgrade to upstream google/adk-go v1.5.0 (true merge)#50
marconn merged 87 commits into
mainfrom
upgrade/v1.5.0-live

Conversation

@dmora

@dmora dmora commented Jul 13, 2026

Copy link
Copy Markdown

Upgrades the fork to upstream google/adk-go v1.5.0 via a true merge (merge commit 5aefe315), followed by 5 fork commits (2 documentation commits + 3 review-fix commits).

Why

Decisions

  • True merge, not squash. This restores shared ancestry with upstream after the v1.2.0 squash import. Future upstream syncs become ordinary git merge operations instead of manual re-imports.
  • Fork RunLive renamed to RunLiveQueue; upstream keeps the RunLive name. Upstream v1.5.0 defines Runner.RunLive with a different signature, so the fork method moved aside. This is a one-time consumer rename (huli companion PR handles it) that makes every future sync cheaper — we never fight upstream for the name again.
  • Both live engines coexist. The fork liveflow engine (RunLiveQueue) is the supported path; the upstream engine (RunLive) is fenced via docs (godoc on the rebound method, liveflow package docs, and LiveDiagnostics docs all state which engine populates what).
  • Upstream telemetry semantics adopted as-is: gen_ai.usage.output_tokens on adk-emitted generate_content spans now includes thinking tokens (candidates+thoughts), and the reasoning span attribute was renamed (gen_ai.usage.experimental.reasoning_tokensgen_ai.usage.reasoning.output_tokens). The huli usageledger companion PR compensates on the consumer side (the ledger reads raw UsageMetadata, never span attributes, so billing is unaffected; see the cross-linked PR).

Validation

  • go build ./..., go vet ./..., and the full test suite green; race detector green on the live packages.
  • 4-lens adversarial review: 5 findings confirmed (findings 1 and 2 were duplicates of one defect), all fixed on this branch; 1 finding deferred with rationale.

Confirmed and fixed

  1. runner/runner.go:383 (findings 1+2, duplicates of one defect) — extended fork fix fix-subrunner-plugin-inheritance: let sub-runners inherit parent plugins #46's plugin-manager inheritance guard (if r.pluginManager != nil && r.pluginManager.HasPlugins()) to upstream's Runner.RunLive, so a plugin-less sub-runner no longer clobbers an inherited parent plugin manager. Added regression test TestUpstreamRunLiveInheritsContextPluginManager in runner/runner_subagent_plugin_test.go (captures the context plugin manager via a fake live agent, since the upstream engine cannot run offline; verified the test fails with the guard reverted). Commit 7cb8006.
  2. runner/runner.go RunLive doc (finding 3) — added a godoc comment on the rebound Runner.RunLive stating it drives the upstream v1.5.0 engine, that its events never carry session.LiveDiagnostics, and that pre-v1.5.0 fork RunLive callers should migrate to RunLiveQueue — removing the silent-misdirection risk the finding flagged. (The rename itself was deliberate and could not be reverted: the name is occupied by upstream's different-signature method.) Commit 9617b71.
  3. session/live_diagnostics.go:19-23 (finding 4)LiveDiagnostics struct doc now says the field is populated only by RunLiveQueue and is nil for Run() and upstream-API RunLive events (whose engine does not compute diagnostics). Commit 4cfd2e8.
  4. session/session.go:121-123 (finding 5)session.Event.LiveDiagnostics field doc now references RunLiveQueue and explicitly lists upstream-API RunLive events as nil. Commit 4cfd2e8.

Deferred

  • go.mod:3 (finding 6, govulncheck) — 17 reachable Go stdlib vulnerabilities are an artifact of building with go1.26.0 (the toolchain installed here); all are fixed in go1.26.1. The finding's own evidence states go.mod/go.sum are byte-identical to upstream v1.5.0 and "remediation is building/scanning with go >= 1.26.1, not a branch change". Adding a toolchain go1.26.1 directive would diverge go.mod from upstream (sync friction) and impose a toolchain floor on all consumers — an ask-first maintainer policy call per AGENTS.md ("Adding or upgrading a dependency (go.mod)"). Recommended action outside this branch: upgrade CI/build/scan environments to Go >= 1.26.1.

Next steps

  • Merge this PR
  • Tag v1.5.0-live on the merge commit
  • Flip the huli draft PR to ready and run go mod tidy in backend/ there
  • Pre-production live soak before rolling huli branches

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR

baptmont and others added 30 commits March 23, 2026 14:46
…le#672)

Co-authored-by: Mikalai Senkevich <hanorik@google.com>
* feat: Implement and test recursive agent lookup by name.

* refactor: convert `TestFindAgent` to a table-driven test and enable parallel execution.

* feat: Introduce a `FindAgent` method on `llmAgent` to locate sub-agents by name, replacing the `runner` package's internal `findAgent` helper.
* feat: add GetArtifactVersion method to artifact service

* chore: remove adk-go main executable.

* Update artifact/service.go

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Extract resolveVersion helper in gcsartifact.

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* custom runner provider for adka2a executor

* remove the mandatory dependency on runnerConfig
* Add session service test suite

* lint fix

* lint fix

---------

Co-authored-by: Dmitry Pasiukevich <20398573+dpasiukevich@users.noreply.github.com>
* fix data part conversions
* update a2a-go and fix double cleanup

* tidy
…s. (google#693)

Define frontmatter per https://agentskills.io/specification#frontmatter.

Implement helper functions for:
- parsing frontmatter and instructions out from SKILL.md file content,
- validating frontmatter struct,
- building SKILL.md file content from frontmatter and instructions.
Create a new subrouter that will handle the pubsub trigger endpoint and add the implementation for this trigger.
update cloudrun deployment script to include pubsub trigger
google#714)

* feat: add telemetry attributes for cache read input tokens and reasoning tokens

* fix: align indentation in telemetry test usage metadata struct fields

* refactor: align attribute key variable definitions for better readability

---------

Co-authored-by: Dmitry Pasiukevich <20398573+dpasiukevich@users.noreply.github.com>
…ource. (google#711)

Source interface defines a contract via which skills framework is going to access skills.
…id memory leaks (google#687)

* feat(telemetry): add configurable LRU cache in debug telemetry to avoid memory leaks

* improve error handling and mention cache size default in the docs

* refactor the logic touching the trace ids for event spans

---------

Co-authored-by: Dmitry Pasiukevich <20398573+dpasiukevich@users.noreply.github.com>
Implementation of Cloud Events trigger processing
Co-authored-by: Karol Droste <kdroste@google.com>
…google#717)

* Update content processor exclude

* remove mds

---------

Co-authored-by: Dmitry Pasiukevich <20398573+dpasiukevich@users.noreply.github.com>
…google#716)

Adds the eventarc trigger configuration to the cloudrun deployment docker container
…#730)

While for tools it's a requirement, for toolsets it's optional.
The ability of toolsets to implement RequestProcessor is required
to implement Agent Skills feature as a toolset.
SkillToolset encapsulates the Agent Skills feature.
This toolset ensures agent is equipped with all the tools needed to use
skills and is provided with proper instructions.

What skill toolset enable agent to do:
- List available skills.
- Read instructions of the skills (by skill name).
- Read specific resources of the skills (by resource path).

Current implementation is very basic, e.g.:
- There is no script execution support yet.
- Skill activation is history-based: skill instructions and resources
  are provided to the llm as function call results in conversation history.
- Every resource should be specifically mentioned in the SKILL.md
  file: the agent is unable to list resources available for the skills.
fix adk-web sse error format so it corresponds with SSE specifications
WithCompletePreloadSource proxy allows to load the skills to memory on
initialization.
It offers the fastest possible data access speed upon initialization,
at the expense of higher memory usage and longer initialization.
Given instructions and resource files can be large, it might turn out to
be expensive to load them all to RAM.

Fortunately, at this stage of skill toolset implementation, instructions
and resources are most likely going to be read just once - and then
they'll stay in the conversation history.

Frontmatters, on the other hand, are going to be listed frequently, and
frontmatters are relatively lightweight, thus a proxy caching only the
frontmatters (as opposed to the full preload) can help reach maximum
benefit.
This proxy allows combining multiple skill sources into a single source.
* Agent Engine support

* Test fix

* Fixes

* Fixes

* Error handling fix

* Fixes

* Fixes in encode

* Fixed encode

* sseTimeout fix

* reflection fix

* sseWriteTime fix

* fix

* encode fixes

* linter fixes

* various fixes

* encode tests

* linter fixes

* fix

* linter fixes

* Fixes in encode
…runner (google#754)

* refactor: remove manual session ID input and enable auto-creation in runner

* refactor: remove unused output field from DeleteSessionResponse serialization
…e#755)

* feat: add update functionality for Agent Engine instances via gcloud command flags

* feat: add --update and --instance_name flags to support updating existing Agent Engine instances

* feat: include and upload class methods in reasoning engine update request

* refactor: replace update and instance_name flags with agent_engine_id for streamlined resource identification

* refactor: replace update and instance_name flags with agent_engine_id for streamlined resource identification
…query (google#773)

* Add support for simple text instead of full genai.Content for stream_query

* Small non-functional fixes

* Small non-functional fixes (~/go/bin/golangci-lint run --fix)
kdroste-google and others added 21 commits May 29, 2026 15:31
…reamingResponseAggregator (google#918)

* fix: skip metadata-only SSE chunks instead of aborting stream in StreamingResponseAggregator

## Link to Issue or Description of Change

Signed-off-by: anish k <ak8686@princeton.edu>

* Fix for SSE chunks with no candidates

* Linter fix

---------

Signed-off-by: anish k <ak8686@princeton.edu>
Co-authored-by: anish k <ak8686@princeton.edu>
* Replaced tool.Context with agent.ToolContext

* fixed runnableTool
…es (google#994)

Resolves the nightly govulncheck failures (exit 3) by updating the modules
flagged in the call graph to their fixed versions:

- golang.org/x/net v0.54.0 -> v0.55.0 (GO-2026-5026)
- go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.16.0 -> v0.19.0 (GO-2026-4985)
- go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0 -> v1.43.0 (GO-2026-4985)

Core go.opentelemetry.io/otel stays at v1.43.0; companion indirect deps
(otel/log, otel/sdk/log, otlptrace, proto/otlp, grpc-gateway, x/sys) move
forward via go mod tidy.

Verified locally: go build ./... and go test -race -mod=readonly -count=1
-shuffle=on ./... both pass, and govulncheck no longer reports GO-2026-5026
or GO-2026-4985.
Bump the remaining actions still on the deprecated Node 20 runtime:
- actions/setup-go v5.5.0 -> v6.4.0 (shared setup composite)
- actions/cache v4.2.3 -> v5.0.5 (shared setup composite)
- golang/govulncheck-action v1.0.4 -> master HEAD

setup-go only moved to Node 24 in v6.2.0, so v5.5.0 was still Node 20. The
govulncheck-action latest release (v1.0.4) internally pins
actions/checkout@v4.1.1 and actions/setup-go@v5.0.0 (Node 20); its master
branch already uses Node 24 actions but has not been tagged, so it is pinned to
master HEAD with a TODO to re-pin once a release past v1.0.4 is cut.

A previous pass only bumped actions/checkout, leaving these behind.
…ent creation (google#964)

Introduce a platform package whose current time and UUID generation can be
overridden per-context, mirroring the ContextVar-based seams in ADK-Python:

  - platform.Now(ctx) / platform.WithTimeProvider(ctx, fn)
  - platform.NewUUID(ctx) / platform.WithUUIDProvider(ctx, fn)

Both fall back to time.Now and uuid.NewString when no provider is installed,
so default behavior is unchanged.

To honor the API stability policy (no breaking changes in a minor release),
the existing session.NewEvent(invocationID) signature is preserved and marked
deprecated; it delegates to the new session.NewEventWithContext(ctx,
invocationID), which sources the event ID and timestamp from the platform
seams. All in-repo event/session creation call sites are threaded with context
and use NewEventWithContext, so a host runtime can make event creation
deterministic and replay-safe (e.g. a workflow engine that must reproduce an
execution exactly on replay).
StrictContextMock implements the full ToolContext / CallbackContext /
ReadonlyContext surface so it can be embedded in a test fake; embedders keep
compiling as the interfaces grow, instead of breaking on every added method.

Un-overridden methods panic with "not implemented" (a loud failure on
unexpected calls), while the context.Context methods (Deadline/Done/Err/Value)
delegate to the wrapped Ctx.
* fix/allow manual setting of session IDs

* fixing missing return + session case issues for inevitable test replay

* Changes in tests

* Updated replays

* check rpc status code instead of string match

---------

Co-authored-by: Karol Droste <kdroste@google.com>
Add an AGENTS.md with project context, exact build/test/lint commands,
repository layout, idioms, a minimal example, framework-extension and
testing guidance, and contribution boundaries for AI coding agents.

GEMINI.md and CLAUDE.md are thin pointers to AGENTS.md so Gemini CLI and
Claude Code pick up the same single source of truth.
…e#777)

* fix(agentengine): support Gemini Enterprise AgentSpace streams

* refactor(agentengine): split streaming_agent_run_with_events handler

* fix(agentengine): add MemoryService to streamingAgentRunWithEventsHandler runner configuration.

* fix(adkrest): make debug telemetry span tests order-insensitive

- The Go test check was failing in TestDebugTelemetryGetSpansBySessionID because the test asserted a fixed ordering for returned spans. In practice, the debug telemetry store can return the same set of spans in a different  order depending on span/export timing, especially when spans are created and ended very close together.

- This change keeps the production behavior unchanged and updates the tests to compare spans as a stable sorted set before diffing. The comparison still checks the span names, relevant attributes, and logs, but no longer treats  incidental retrieval order as part of the contract.

* chore(agentengine): clarify streaming agent run metadata

Document the request_json payload for streaming_agent_run_with_events, mark the method as async_stream consistently, and replace realistic test fixture IDs with clearly fake values.
* Got rid of context.Background

* Linter fixes
…SessionService (google#793)

* fix: update load_memory tool with json marshaling of memory search response into map[string]any to avoid downstream incompatibility with Go struct types

* revert: undo initial changes to load_memory tool

* fix: safely normalize all part.FunctionResponse.Response types via JSON round-trip before conversion to structpb.

This prevents errors or panics in `structpb.NewStruct()` caused by raw Go types that are incompatible with Protocol Buffers.

* feat: add utility to safely normalize Go types via JSON round-trip before conversion to structpb.

This prevents errors or panics in structpb.NewStruct() caused by raw Go types that are incompatible with Protocol Buffers.

* chore: rename and simplify toStructPB util function

* chore: add broader vertexai sessions test coverage

---------

Co-authored-by: nicholas@alisx.com <nicholas@alisx.com>
Co-authored-by: Karol Droste <kdroste@google.com>
* session test suite fix

* Linter fixes
…nse mapping (google#739)

Add TestAiplatformToGenaiContent_FunctionCallMapping, a table-driven
test that verifies aiplatformToGenaiContent correctly preserves:
- ID, Name, and Args for FunctionCall parts
- ID, Name, and Response for FunctionResponse parts
- empty-string IDs are passed through unchanged

The table-driven format matches the style used elsewhere in this file
and makes it easy to add further cases.
True merge (both parents preserved) of upstream tag v1.5.0
(caf798a) into the fork, restoring real ancestry for future syncs
after the squashed v1.2.0 sync poisoned the merge-base.

Policy: both live stacks coexist. The fork liveflow engine
(internal/llminternal/liveflow + LiveRequestQueue) remains the
supported path; upstream's new live stack (agent/live.go,
googlellm/live_connection.go, Flow.RunLive, Runner.RunLive,
adkrest /run_live) is adopted as-is alongside it.

Taken from upstream verbatim:
- session/vertexai/testdata/*.replay (all, incl. missing_author /
  missing_invocation_id), server/agentengine/**, tool/skilltoolset/**,
  examples/skills/**, examples/agentengine/**,
  cmd/adkgo/internal/deploy/agentengine/*
- server/adka2a/executor.go (upstream moved the executor to v2/ and
  left a shim) and server/adka2a/v2/**
- server/adkrest conflicted files (/run_live route + error-handler
  refactor)
- internal/configurable/conformance/** (plus mock fix below)
- internal/version/version.go (upstream value kept verbatim)
- test files: agent/llmagent/state_agent_test.go,
  internal/llminternal/base_flow_test.go,
  internal/llminternal/stream_aggregator_test.go, session/*_test.go,
  agent/remoteagent/v2/a2a_e2e_test.go (upstream supersets; the fork
  UNEXPECTED_TOOL_CALL test was upstreamed)
- go.mod/go.sum verbatim (hashicorp/golang-lru dep converged);
  go mod tidy produced no further changes

Manual resolutions:
- internal/telemetry/telemetry.go: adopted upstream attribute
  semantics (gen_ai.usage.output_tokens now includes
  ThoughtsTokenCount; gen_ai.usage.reasoning.output_tokens replaces
  the fork's gen_ai.usage.experimental.reasoning_tokens;
  gen_ai.usage.cache_read.input_tokens kept) while preserving the
  fork live span keys GCPVertexAgentOperationKey,
  GCPVertexAgentToolCallCancelledKey, GCPVertexAgentReconnectAttemptKey.
  telemetry_test.go follows upstream expectations (output=35).
- internal/context/invocation_context.go: union of both sides; both
  sides had implemented identical LiveSessionResumptionHandle
  plumbing, so the auto-merge declared it twice - kept the fork copy
  (alongside LiveRequestQueue()) and dropped upstream's duplicate.
- model/llm.go: kept the entire fork Live block (LiveConnection,
  LiveCapableLLM, LiveRequest, LLMRequest.LiveConfig, LLMResponse
  SessionResumptionUpdate/GoAway/ReceivedAt/ReceiveLatency) plus
  upstream's new LLMResponse.SessionResumptionHandle; deduped the
  InputTranscription/OutputTranscription fields both sides added.
- internal/llminternal/base_flow.go: adopted upstream structure
  (toolsetPreprocess uses agent.NewToolContext;
  newResponseWithEventID now takes ctx and uses platform.NewUUID);
  kept the fork tool-callback type aliases into liveflow (signatures
  identical - tool.Context == agent.ToolContext via alias).
- agent/llmagent/llmagent.go: disjoint union - fork unexported
  runLive dispatch (StreamingModeBidi) and upstream exported RunLive
  both kept.
- runner/runner.go: kept fork isToolEvent and upstream hasInlineData;
  renamed the fork queue-based Runner.RunLive to RunLiveQueue so
  upstream keeps the RunLive name (minimizes future sync conflicts).
  Callers updated: examples/live/main.go, runner/runner_live_test.go,
  runner/runner_subagent_plugin_test.go.
- internal/llminternal/liveflow/tools.go: toolinternal.NewToolContext
  was removed upstream; switched to agent.NewToolContext.
- conformance replayplugin/recordplugin MockInvocationContext: added
  the 3 stub methods for the fork's agent.InvocationContext
  extensions (LiveRequestQueue, LiveSessionResumptionHandle,
  SetLiveSessionResumptionHandle).
- agent/agent.go + agent/context.go: auto-merge verified - fork live
  extensions intact within upstream restructuring
  (callback_context.go / context_mock.go are upstream-new).

Deliberate deviations from the pre-validated reference resolution:
- runner.go: RunLiveQueue carries a doc comment explaining it is the
  fork-supported live entry point driving liveflow, coexisting with
  upstream RunLive (reference left the old comment unchanged).
- invocation_context.go: kept the fork method copy next to
  LiveRequestQueue() as planned (reference kept upstream's copy
  further down; both are semantically identical).
- liveflow/tools.go: updated the comment that still referenced the
  removed toolinternal.NewToolContext (reference left it stale).
- Mock stub whitespace differs trivially (gofmt-clean in both).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
Post-v1.5.0-merge, LLMResponse carries session resumption twice by
design: upstream's SessionResumptionHandle string (populated by the
googlellm connection, read by llminternal Flow.RunLive) and the fork's
structured SessionResumptionUpdate (populated by model/gemini, consumed
by the liveflow engine, which also honors Resumable=false handle
invalidation). Document that on the fork field so neither carrier is
removed as an apparent duplicate during future syncs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
The v1.5.0 merge brought in a second live engine: Runner.RunLive /
agent.LiveSession / adkrest /run_live drive upstream Flow.RunLive plus
the googlellm connection. Fence it off in the liveflow package doc by
listing its verified v1.5.0 gaps (string-matched GoAway, no reconnect
backoff or attempt budget, history re-sent on resume, dropped
ToolCallCancellation, input transcriptions authored as the agent) and
naming Runner.RunLiveQueue + liveflow as the hulilabs-supported path.
Strengthen the RunLiveQueue doc comment to point at that comparison.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
…unLive

Fork fix #46 (2208905) made Run and the queue-based live entry point
(now RunLiveQueue) skip installing the runner's own plugin manager into
context when it has no plugins, so a plugin-less sub-runner inherits the
parent's manager and model/tool/agent callbacks keep propagating.

The upstream Runner.RunLive that arrived with v1.5.0 kept the
unconditional upstream line, so a plugin-less sub-runner driven through
it would clobber an inherited parent manager with its own empty one.
Apply the same HasPlugins guard there and add a regression test that
captures the context plugin manager via a fake live agent (the upstream
live engine itself cannot run offline).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
The v1.5.0 merge renamed the fork's queue-based Runner.RunLive to
RunLiveQueue and rebound the RunLive name to upstream's new
agent.LiveSession entry point, whose engine never sets LiveDiagnostics.
The LiveDiagnostics struct doc and the session.Event field doc still
said the field is populated "for events from RunLive", steering
upstream-RunLive callers into expecting diagnostics they will never
get. Reference RunLiveQueue and state explicitly that upstream-API
RunLive events carry nil.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
The v1.5.0 merge renamed the fork's queue-based Runner.RunLive to
RunLiveQueue while upstream introduced a different RunLive
(agent.LiveSession API) under the surviving name. The compile break for
out-of-repo fork consumers is loud, but the surviving name silently
points at the weaker upstream engine, so a consumer adapting to the new
signature could migrate onto it unknowingly. Document on RunLive itself
that it is the upstream engine, that its events never carry
LiveDiagnostics, and that pre-v1.5.0 callers belong on RunLiveQueue.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
@dmora

dmora commented Jul 13, 2026

Copy link
Copy Markdown
Author

Companion consumer PR (draft until this merges and v1.5.0-live is tagged): https://github.com/hulilabs/huli/pull/3228 — RunLive → RunLiveQueue rename (3 call sites) + usage-ledger reasoning-token regression canary.

dmora and others added 2 commits July 13, 2026 12:34
The three existing tests all prove the inheritance branch (plugin-less
runner leaves the context-seeded parent manager). A guard regressed to
never installing would still pass the suite. Add the positive
counterpart: a runner that owns plugins must replace the seeded parent
manager with its own. Verified the test fails with the guard mutated to
never-install.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
… docs

The v1.5.0 merge union kept the fork's method ordering and dropped
upstream's doc comments on the resumption-handle accessors, inflating
the permanent diff against upstream in a file both sides edit. Restore
upstream's declaration order and comments so the fork delta is exactly
the LiveRequestQueue param and accessor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR

@marconn marconn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR has 10 blocking issues across testing, documentation, concurrency, observability, code quality, and integration boundaries. Five are major severity (concurrency goroutine leaks, missing integration tests for /run_live WebSocket handler, untested streaming tool confirmation gates, missing memory/vertexai unit tests, and documentation errors in the tool.Context deprecation notice), and five are minor. Posted as a non-blocking comment for the author to analyze.

server/adkrest/controllers/runtime.go:807 — ws.WriteJSON error on line 807 breaks with no log statement. Every other failure path in this handler logs; add log.Printf with appName to correlate dropped live sessions with logged errors.

  • Flow.RunLive has a goroutine leak: both recv and send goroutines send to an unbuffered errChan with no connCtx.Done() guard. On reconnect/teardown, the old errChan is abandoned while peer goroutines still block on it, leaking one goroutine per cycle. Wrap each errChan send in a select with case <-connCtx.Done(), matching the eventsChan pattern.
  • New package memory/vertexai (~234 added lines) ships with no test file. AddSessionToMemory has untested branching: state-key dispatch (stateKeySessionLastUpdateTime=='' vs filtered path) and type assertion (t.(time.Time)). Add unit tests covering both paths and the error case.
  • New /run_live WebSocket endpoint (RunLiveHandler) has no integration/E2E test. Critical untested path: websocket.Upgrader handshake, binary/text frame protocol contract, two-goroutine lifecycle, and close-frame drain. Add integration tests using mockLiveAgent and dummyLiveSession patterns already present in the repo.
  • Transcription-aggregation loop duplicates ~15 lines of logic between input and output branches (lines 685-716). Both accumulate text, check continuation, clone event, null field, and build genai.Content differing only in role and field name. Extract a shared helper to prevent logic drift in future bug fixes.
  • Deprecated notice on tool.Context alias says 'use agent.Context directly', but no such exported type exists in agent package. The alias resolves to agent.ToolContext, and all call sites migrate to agent.ToolContext. Update deprecation text to say 'use agent.ToolContext' to avoid misleading consumers.
  • Godoc block for Sequential.New constructor is now mis-associated with seqAgent type due to code insertion. The comment 'New creates a SequentialAgent...' now attaches to the type, leaving New with only the orphaned fragment. Restore blank '//' continuation line to maintain correct comment association.

Comment thread tool/functiontool/streaming_function.go
Comment thread tool/functiontool/streaming_function.go
Review feedback on PR #50 identified a per-reconnect goroutine leak in
the upstream Flow.RunLive engine (unbuffered errChan abandoned on
resumable errors). The engine is fenced as unsupported here; record the
gap alongside the others and link the upstream report
(google#1152).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQUNntrYi9Yn1ExuN9f5VR
@dmora

dmora commented Jul 13, 2026

Copy link
Copy Markdown
Author

Thanks @marconn — verified every item against the merged tree and both merge parents. Summary of dispositions:

Confirmed, but upstream-verbatim code (8 of 9). These are real findings about code google/adk-go shipped in v1.3–v1.5 that this PR takes verbatim under our merge policy (no fork patches in upstream-owned subsystems — each one is permanent sync tax, and none of these paths are driven by huli, which uses RunLiveQueue/liveflow). Dispositions:

  • Flow.RunLive goroutine leak — confirmed, including on upstream main@0c88126. Reported upstream as google/adk-go#1152 with a suggested fix, and added to the fenced-engine known-gaps list in liveflow/doc.go (700d279) so the fence stays honest.
  • sequentialagent godoc mis-association — confirmed, plus a bonus you were one line away from: sequentialagent.New's guard error says "LoopAgent doesn't allow custom Run implementations". Both fixed upstream in google/adk-go#1151.
  • Test gaps (memory/vertexai, streaming-tool confirmation gates ×2, /run_live handler) and the unlogged ws.WriteJSON error — all confirmed still present on upstream main; filed as google/adk-go#1153 (logging note included there).
  • tool.Context deprecation text — confirmed wrong at v1.5.0 (agent.Context doesn't exist here), but upstream main has since introduced agent.Context and deleted the tool.Context alias entirely, so the note was premature rather than misdirected and self-resolves at the next sync. No fork edit (it would diverge from a file that's already moved on). ⚠️ Forward-looking consequence for consumers: the next upstream sync removes tool.Context — huli's ~319 call sites need migrating to the unified context type before/with that sync.

Refuted (1 of 9).

  • Transcription-aggregation duplication (runner.go ~685-716) — the cited range contains the extraction you're asking for: persistTranscript and bufferTranscript already centralize the create-event → set-fields → append → reset pattern; the remaining input/output symmetry is argument passing ("user","user" vs agentName,"model"), not duplicated logic.

Net: no changes to PR scope beyond the one fork-owned doc commit (700d279). The upstream engine's defects stay documented-and-fenced rather than fork-patched, per the merge policy in the PR description.

@marconn
marconn merged commit da223f2 into main Jul 13, 2026
5 checks passed
@marconn
marconn deleted the upgrade/v1.5.0-live branch July 13, 2026 20:36
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.