feat: enforce lifecycle.call_timeout for MCP tool calls

[jedp-docker]

Summary

• Enforces the existing lifecycle.call_timeout toolset field, which was previously parsed but never acted on ("informational today" per its own doc comment). A Harbor Watch investigation stalled 178s on a single hung MCP tool call because docker-agent had no per-call deadline — a stuck call was bounded only by the session context (~600s).
• Toolset.callTool now wraps the call and its one reconnect-retry in a single context.WithTimeout budget derived from the toolset's configured call_timeout. A fired timeout is classified distinctly from cancellation/generic errors, logs a WARN + emits an mcp.tool_call.timeout span event (the original hang logged nothing), and surfaces a plain tool call timed out after Ns error to the model — the session continues normally (Role=tool, IsError=true, same mechanics as any other tool error).
• Also wires lifecycle.Policy through NewGatewayToolset, which previously dropped it entirely for ref:-based (MCP-catalog) toolsets — a pre-existing gap this change also closes since gateway toolsets share the same constructor plumbing. Verified behaviorally identical to the historical zero-policy default when no lifecycle: block is configured (TestPolicyFromConfig_NilEquivalentToZeroPolicy). Let me know if this was intentional, please.
• Default (unset/zero, the vast majority of existing configs) is unchanged — if ts.callTimeout > 0 skips context.WithTimeout entirely, so callCtx is literally the caller's ctx and every code path (including the retry guard) behaves byte-for-byte as before. This is a strictly opt-in, per-toolset change, same precedent as startup_timeout enforcement in v1.94.0.
• Schema (agent-schema.json) and docs (docs/configuration/tools/index.md) updated to describe the enforced call_timeout semantics instead of "informational."
• Not touching CHANGELOG.md in this PR — every existing entry there is versioned/dated and appears to be added in batch at release-cut time (docs: update CHANGELOG.md for vX PRs), not accumulated per-feature-PR, so I'm leaving that to whoever cuts the next release.

Ref: AIDT-619

Test plan

• go vet ./..., go build ./..., go mod tidy --diff all clean
• golangci-lint run (full repo) — 0 issues; go run ./lint . (custom repo lint) — no offenses
• go test ./... — all packages pass; two failing tests are pre-existing/environment-specific (SSRF-probe tests behave differently under sandbox network policy), unrelated to this change
• New tests: timeout fires and is classified correctly (TestCallToolTimeoutFires), no-op when unset (TestCallToolNoTimeoutWhenCallTimeoutUnset), parent cancel wins over timeout (TestCallToolParentCancelWinsOverTimeout), timeout covers the reconnect-retry as one budget rather than stacking on top of the 35s retry window (TestCallToolTimeoutCoversReconnectRetry), constructors (incl. gateway) wire callTimeout from policy, translateError model-facing message, and a real streamable-transport test (TestRemoteClientCallToolAbortsOnContextDeadline) confirming a canceled client context actually aborts the POST and the server observes the cancellation
• Companion docker/internal-agents change (harbor-watch.yaml lifecycle.call_timeout: 60s + config version bump) validated end-to-end against this branch's config parser

🤖 Generated with Claude Code

[docker-agent]

Assessment: 🟢 APPROVE

The timeout enforcement is well-structured: the budget correctly covers both the initial call and the reconnect-retry as a single budget, cleanly distinguishes caller-cancel / call-timeout / generic errors, and the prevents context leaks. The nil-receiver guards on make the pre-nil-check call in safe (matching the pre-existing pattern). Gateway policy threading and schema/doc updates look correct. Test coverage for timeout firing, no-op when unset, parent-cancel priority, retry budgeting, and transport-level abort is comprehensive.

[docker-agent]

Assessment: 🟢 APPROVE

The implementation is clean and correct. Key design points reviewed:

• Timeout scoping: callCtx is correctly a child of ctx (never replaces it), so the session/connection context is unaffected by a fired call_timeout.
• Error classification in classifyCallToolError: properly distinguishes parent cancellation (propagates as-is), own timeout (ctx.Err()==nil && callCtx.Err()==DeadlineExceeded), and generic errors. Parent-deadline-exceeded correctly falls to the generic path.
• Reconnect guard: callCtx.Err() == nil prevents retrying when the timeout budget is already exhausted — correct.
• Zero-value default: if ts.callTimeout > 0 guard means unset toolsets are byte-for-byte identical to the prior behavior.
• PolicyFromConfig nil-ordering: calling cfg.EffectiveCallTimeout() before the nil guard matches the pre-existing EffectiveStartupTimeout() pattern and is safe (the method handles nil receivers).
• Test coverage: timeout fires, no-op when unset, parent cancel wins, reconnect covered by single budget, transport-level abort verified — comprehensive.

[docker-agent]

Assessment: 🟢 APPROVE

The timeout enforcement design is sound: context.WithTimeout is correctly scoped to a child of the caller's context (so callCtx never outlives ctx), defer cancel() is always called preventing timer leaks, the zero-value default is a genuine no-op, and error classification distinguishes cancellation from timeout correctly in the common case. Tests are thorough and the gateway policy wiring is properly threaded.

Lower-confidence findings (not posted inline)

• [low] pkg/tools/mcp/mcp.go:803 — classifyCallToolError could misclassify a parent-context deadline as a call timeout (confidence: 🟠 weak 52/100). When the parent ctx has its own deadline that fires nearly simultaneously with callCtx's timeout, ctx.Err() == nil may not yet reflect the parent's cancellation at the moment the check runs. The misclassification would report ErrCallTimeout instead of the session/run deadline to the caller. The impact is limited — it's a label difference, not silent swallowing, and doesn't affect retry behaviour — and the test TestCallToolParentCancelWinsOverTimeout covers the cancellation case. Because the practical race window is extremely narrow and the consequence is a label swap rather than data loss or a hang, this sits below the inline threshold.

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR correctly enforces lifecycle.call_timeout as a single budget covering both the initial tool call and the reconnect-retry. The timeout guard (if ts.callTimeout > 0), context lifecycle (defer cancel), error sentinel (ErrCallTimeout), span events, and dispatcher translation are all well-implemented. One logic gap was found in classifyCallToolError.

[docker-agent]

[medium] Parent context DeadlineExceeded is misclassified as a generic tool error

classifyCallToolError uses ctx.Err() == nil to prove that a deadline on callCtx came from the call-timeout timer rather than from the parent context. That works when the parent is canceled (context.Canceled), but it breaks when the parent itself has a deadline that fires first: ctx.Err() then returns context.DeadlineExceeded, making ctx.Err() == nil false. Neither errors.Is(err, context.Canceled) nor ctx.Err() == nil is true, so the function falls through to the generic path and logs at slog.ErrorContext with "Failed to call MCP tool" — misleading for what is really a context-deadline expiry.

Concretely: if the session context (~600 s) has an explicit deadline and that deadline fires while a tool call is in progress, the log will say "Failed to call MCP tool" at ERROR level instead of the expected cancellation/deadline handling.

A minimal fix is to add an explicit branch for the parent-deadline case before the generic fallthrough:

	if errors.Is(err, context.Canceled) || errors.Is(ctx.Err(), context.Canceled) {
		slog.DebugContext(ctx, "CallTool canceled by context", "tool", toolCall.Function.Name)
		return err
	}
	// Parent context expired with a deadline (not a cancellation): propagate cleanly.
	if ctx.Err() != nil {
		slog.DebugContext(ctx, "CallTool aborted by parent deadline", "tool", toolCall.Function.Name)
		return ctx.Err()
	}
	if ts.callTimeout > 0 && errors.Is(callCtx.Err(), context.DeadlineExceeded) {
		// ... timeout path unchanged ...
	}

Confidence	Score
🟢 strong	100/100