Skip to content

revert: undo squashed backport #3586 on v1.0.x #3595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
EItanya wants to merge 1 commit into from
Closed

revert: undo squashed backport #3586 on v1.0.x #3595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 0 additions & 27 deletions docs/architecture/mcp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,3 @@ not protected: registering a name or URI core already uses replaces it.
Authorization inside an added tool stays with the program that adds it.

The implementation is in [`go/core/internal/mcp`](../../go/core/internal/mcp).

## Go ADK runtime discovery

The Go ADK connects to MCP servers and discovers tools during invocation
preparation. Startup and Actor resume do not wait for HTTP or SSE connections
or start stdio MCP processes. This also applies to subagents.

Discovery filters app-only tools and applies configured allowlists before tools
reach the model. The MCP SDK caches filtered responses and owns TTL expiry and
catalog-notification invalidation. Cached tools remain filtered. Credentials,
approvals, and tool-call authorization continue to apply.

If discovery fails, that server's toolset stays hidden for the current A2A
execution, including subsequent model steps and subagent calls. Healthy toolsets
remain available. The next execution retries, including an approval continuation.
There is no fallback to unclassified tools. Discovery failures are logged as
warnings. Cancellation and tool-call errors retain their existing handling.

MCP App results are compacted for the model while stored results remain intact
for UI rendering. Removed app names remain recognized for historical compaction
during the process lifetime. Tools reclassified as ordinary keep their full
output, including historical results. A new process rediscovers app metadata
during invocation preparation.

Standalone runners can use `mcp.WithDiscoveryScope(ctx)` for each `runner.Run`
to get the same omission behavior. Without it, discovery errors propagate.
Controller-side catalog discovery used by the UI is independent.
61 changes: 0 additions & 61 deletions docs/architecture/runtime-and-lifecycle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,67 +223,6 @@ claims still block new work, but completed results remain readable. The recorded
actor UID is checked before lifecycle calls; a same-name replacement cannot be
adopted implicitly.

`SessionService.RevertSession` lets an authorized caller restore a confirmed
crashed runtime. It accepts active and idle Sessions, but verifies Substrate's
CRASHED state for the recorded Actor UID and pinned template before admitting
recovery. Quiet turns, transport failures, missing Actors and same-name
replacements cannot authorize a revert. A runtime that is not crashed, including
a Session at rest outside READY, has nothing to recover, so RevertSession
returns the Session unchanged. That is also what a retry sees after a revert
whose response was lost. A missing or replaced Actor returns
`FAILED_PRECONDITION`, because no retry restores it. A transport failure returns `UNAVAILABLE`. Only an active
lifecycle operation returns `ABORTED`.

Admission records FAILED/REVERT with a RuntimeCrashed failure under the Session
lock. REVERT is its own `RuntimeOperation`: only a confirmed crash starts one, and
`BeginSessionOperation` can join a REVERT but never start one. The same transaction
publishes a durable terminal result saved before the crash, or fails nonterminal
work absent from the retained snapshot. Waiting tasks captured by that snapshot
remain available. Checkpoint creation excludes competing recovery.

Admission settles waits against the Actor's latest completed durable snapshot,
which is the one Substrate's revert restores. Every suspend kagent issues uses
synchronous durability, so its upload completes before suspension returns, and
a crashed Actor cannot be suspended. No upload can complete between admission
and the revert, so settling at admission settles against what is restored, and
the interrupted turn fails as soon as the crash is confirmed. An asynchronous
suspend would break this and require settling when the revert finishes.

The caller reads the Session's generation before it observes Substrate: its
lifecycle generation, recorded Actor UID, and Actor template, in one statement.
The observed Actor must match that template and UID. Admission takes the read
back, rejects the crash when another lifecycle operation began after it, and
re-checks the Actor UID under the Session lock. A concurrent revert keeps the
Actor UID and returns the Session to READY, so only the generation exposes a
stale observation. The recorded UID changes only through a lifecycle operation,
which also replaces the generation.

An issued idle suspension does not exclude recovery. It cannot finish against the
crashed Actor, and its worker keeps the claim when suspension fails. Admission
retires the claim in its transaction, so a late finish records nothing.

Execution reverts the same Actor to its last durable snapshot without replaying
input. A runtime without a durable snapshot returns to its initial state.
An interrupted revert continues from REVERTING; a lost response can be reconciled
from SUSPENDED. Nothing retries a recovery in the background. An admitted
recovery survives API restarts as a durable REVERT operation, and the client
continues it by retrying RevertSession, as with Create and Delete. While it is
pending, `GetSession` reports FAILED/REVERT with failure reason `RuntimeCrashed`,
and interactions are refused because a lifecycle operation is in progress.

Delete supersedes an admitted recovery even after an attempt issued the revert.
Other issued lifecycle work blocks Delete because it may still create something
Delete would miss; a revert restores the recorded Actor in place and creates
nothing. Substrate serializes the two calls, and a late revert fails against the
deleted Actor. A recovery whose Actor is missing or replaced stays pending, and
each retry returns
`FAILED_PRECONDITION` so the client deletes the Session.

A successful revert returns the Session to READY with the same Actor suspended.
The next authorized interaction resumes it through Substrate ingress. Runtime
changes after the retained snapshot are lost; the discarded turn receives no
checkpoint at that older snapshot. Another revert returns the Session.

```mermaid
sequenceDiagram
participant Client
Expand Down
27 changes: 2 additions & 25 deletions docs/lifecycle-retries.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

Session and Sandbox lifecycle mutations run within the API request. The server
records the intended operation before calling the runtime and records completion
before returning success. Sessions expose Create, Fork, Revert, and Delete; explicit
Suspend and Resume are available only for Sandboxes. Session runtime suspension and resumption
before returning success. Sessions expose Create, Fork, and Delete; explicit Suspend
and Resume are available only for Sandboxes. Session runtime suspension and resumption
are platform-managed. A failed request can have changed the runtime: a timeout
does not mean that nothing happened, and canceling a request does not undo it.

Expand Down Expand Up @@ -39,29 +39,6 @@ for subsequent invocations. Omitting it generates a new ID on each invocation,
so rerunning that command is a new creation rather than a retry. MCP callers
should likewise retain the `request_id` passed to `create_sandbox`.

## Recovering a crashed Session

Call `SessionService.RevertSession` with the Session ID to restore its recorded runtime to the last durable snapshot.
The caller needs Session update access. The operation verifies Substrate's CRASHED state and the recorded Actor identity.
Recovery fails interrupted turns and waiting tasks absent from the retained snapshot.
It preserves durable terminal results and returns the Session to READY with its Actor suspended.
It discards runtime changes since that snapshot and does not replay input.
A runtime without a snapshot restarts from its initial state.

Retry an interrupted revert against the same Session ID; nothing retries it in the background.
Pending recovery retains its operation across API restarts, and a retry joins it.
While recovery is pending, `GetSession` reports state `FAILED`, operation `REVERT`, and failure reason `RuntimeCrashed`.
Interactions fail because a lifecycle operation is in progress. Call `RevertSession` again to finish the recovery.
A retry after a revert whose response was lost returns the recovered Session.
Checkpoint creation returns `ABORTED` until it finishes.
A revert that observed the crash before another lifecycle operation began also returns `ABORTED`.
So does a revert while another lifecycle operation is active.

A revert of a runtime that is not crashed returns the Session unchanged.
A revert that can never succeed returns `FAILED_PRECONDITION`: the recorded Actor is missing, or a same-name Actor replaced it.
Stop retrying, delete the Session, then start another or fork one from a checkpoint. Delete succeeds even while the recovery is pending.
Read the reason rather than the message text, which is not stable.

## Errors

| gRPC status | Client action |
Expand Down
3 changes: 1 addition & 2 deletions go/adk/examples/oneshot/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/kagent-dev/kagent/go/adk/pkg/agent"
"github.com/kagent-dev/kagent/go/adk/pkg/config"
"github.com/kagent-dev/kagent/go/adk/pkg/mcp"
"github.com/kagent-dev/kagent/go/pkg/logging"
)

Expand Down Expand Up @@ -107,7 +106,7 @@ func main() {
streamMode = adkagent.StreamingModeSSE
}

for ev, err := range r.Run(mcp.WithDiscoveryScope(ctx), "user", sess.Session.ID(), content, adkagent.RunConfig{
for ev, err := range r.Run(ctx, "user", sess.Session.ID(), content, adkagent.RunConfig{
StreamingMode: streamMode,
}) {
if err != nil {
Expand Down
2 changes: 0 additions & 2 deletions go/adk/pkg/a2a/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"google.golang.org/genai"

"github.com/kagent-dev/kagent/go/adk/pkg/auth"
"github.com/kagent-dev/kagent/go/adk/pkg/mcp"
"github.com/kagent-dev/kagent/go/adk/pkg/models"
"github.com/kagent-dev/kagent/go/adk/pkg/telemetry"
apia2a "github.com/kagent-dev/kagent/go/api/a2a"
Expand Down Expand Up @@ -265,7 +264,6 @@ func (e *KAgentExecutor) Execute(ctx context.Context, reqCtx *a2asrv.ExecutorCon

ctx = context.WithValue(ctx, publicContextIDKey{}, reqCtx.ContextID)
ctx = withBearerToken(ctx)
ctx = mcp.WithDiscoveryScope(ctx)
// The synthetic ADK user ID is only a native session lookup key. Memory
// and outgoing credentials must receive only the passed-through caller.
ctx = auth.WithUserID(ctx, trustedUserID)
Expand Down
118 changes: 0 additions & 118 deletions go/adk/pkg/a2a/executor_mcp_test.go
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions go/adk/pkg/a2a/executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type recordingExecutor struct {
userID string
cleanupCalled bool
events []a2atype.Event
onExecute func(context.Context)
// span, when set, is a span the turn emits, the way ADK does.
span string
}
Expand All @@ -49,9 +48,6 @@ func (e *recordingExecutor) Execute(ctx context.Context, reqCtx *a2asrv.Executor
return func(yield func(a2atype.Event, error) bool) {
e.message = reqCtx.Message
e.userID = auth.UserIDFromContext(ctx)
if e.onExecute != nil {
e.onExecute(ctx)
}
if e.span != "" {
_, span := otel.Tracer("adk-test").Start(ctx, e.span)
span.End()
Expand Down
76 changes: 0 additions & 76 deletions go/adk/pkg/a2a/server/lazy_mcp_startup_test.go
View file
Open in desktop

This file was deleted.

Loading
Loading