fix: harden agent hooks, MCP, sessions and token accounting

CHANGELOG.md

@@ -7,6 +7,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- HTTP MCP transports validate response IDs and object results; streamable HTTP
+  ignores unrelated SSE responses and adopts session headers only after a valid
+  response. Invalid JSON becomes an explicit MCP error.
+- MCP pagination validates page shapes and opaque cursors, with a configurable
+  `max_discovery_pages` limit (100 by default) to bound endless discovery.
+- Custom before-tool hooks validate successive argument rewrites, preserve
+  approval requirements, and pass rewritten arguments to permission checks.
+- Post-tool output replacement clears stale model-facing excerpts; synchronous
+  hook dispatch rejects awaitables instead of silently ignoring async policies.
+- MCP structured-only results are rendered as compact JSON instead of being
+  described as empty; existing text responses are not duplicated. Execution
+  status overrides static metadata, and only boolean `true` is a read-only hint.
+- Malformed schema-cache roots/descriptors trigger live discovery instead of
+  crashing warm startup.
+- Repeated-result suppression compares both raw evidence and model-facing
+  views, so a new relevant excerpt of unchanged evidence is not hidden.
+- Shared core-tool workspaces reject prefix-sibling and symlink escapes using
+  path containment rather than string prefixes.
+- Exact file edits reject empty `old_text`, preventing accidental insertion
+  between every character when `replace_all` is enabled.
+- Session-manager chats use the manager's configured store without mutating
+  the original agent. Historical forks discard derived summaries/facts and
+  deep-copy retained messages so later evidence and edits cannot leak across.
+- Closing agent/chat streams explicitly closes nested runtime iterators;
+  async cleanup is awaited instead of depending on generator finalization.
+- Async streams propagate worker exceptions after draining emitted events;
+  provider failures no longer look like a successful end of stream.
+
+- OpenAI-compatible, LiteLLM (including streaming), and Anthropic usage
+  normalization no longer invents zero counters when providers omit input or
+  output usage. Budget enforcement can now recognize incomplete accounting.
+- MCP resilience retries discovery/read requests, but no longer automatically
+  replays `tools/call` after ambiguous transport failures. A lost response may
+  follow a successful write; silently retrying could duplicate side effects.
+- File session storage uses bounded, collision-resistant filenames, including
+  on case-insensitive filesystems. Legacy files remain readable only when their
+  embedded session identity matches; listing deduplicates migrated records.
+- A text tool call such as `{"path": …, "content": …}` fit several file tools
+  (each declares `path`) and was dropped as ambiguous, so the file was never
+  written. When several tools accept a nameless object, the one tool that
+  declares every key it carries is chosen; a genuine tie is still left alone.
+
 ### Security
 
 - `bash` (including background jobs read via `bash_job`) and `run_code`
@@ -28,12 +72,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `DOCKER_*` to the `docker` CLI. `bash` still runs a login shell (`bash
   -lc`), so profile files can re-export variables; override `HOME` via
   `extra_env` if that matters.
-### Fixed
+### Added
 
-- A text tool call such as `{"path": …, "content": …}` fit several file tools
-  (each declares `path`) and was dropped as ambiguous, so the file was never
-  written. When several tools accept a nameless object, the one tool that
-  declares every key it carries is chosen; a genuine tie is still left alone.
+- `recall_tool_result` accepts an optional case-insensitive literal `query`,
+  returning an exact bounded excerpt around a match instead of requiring
+  sequential paging through large historical results.
+- `AgentChatSession.asend()`, `astream()`, and `astream_packets()` use the
+  native async runtime while retaining per-chat history, budgets, and callbacks.
+- Regression coverage for custom hooks across all four agent entrypoints,
+  concurrent 20-turn scripted sessions, real local file-edit/test workflows,
+  provider usage normalization, and MCP protocol/result/cache boundaries.
+- Hook integration guidance and a focused runtime reliability review documenting
+  implemented safeguards and remaining production-validation gaps.
+
+### Validation and limitations
+
+- Offline suite: **4,706 passed, 32 skipped**, with one pytest collection
+  warning (`TestCase` in `shipit_agent/deep/benchmark.py`). Live tests excluded.
+- No live Bedrock quality or billed-token savings claim is made by this update.
+  Cross-worker durable budgets and production OAuth/MCP lifecycle validation
+  remain outstanding. Hook callbacks are synchronous, including in async runs;
+  post-tool hooks cannot retract output already streamed to clients.
+- No Google Workspace extension is included in this update.
 
 ## [2.2.0] - 2026-10-02
 

docs/agent-review-2026-10-06.md

@@ -0,0 +1,55 @@
+# Agent runtime review — 2026-10-06
+
+## Scope and evidence
+
+This is a focused review of the main Agent runtime, sync/async streaming,
+session management, output budgeting, MCP result handling and schema caching,
+and core file tools. It is not a completed line-by-line audit of every module,
+integration or optional dependency. Local regression suites and scripted-model
+workflows do not establish production quality across all providers.
+
+## Research applied
+
+- Anthropic's [tool engineering guidance](https://www.anthropic.com/engineering/writing-tools-for-agents)
+  emphasizes relevant responses, discoverable tools, and evaluations measuring
+  correctness alongside calls, errors, latency and token consumption. Shipit
+  already has deferred discovery and bounded model-facing tool output; preserve
+  canonical evidence while improving those boundaries rather than indiscriminately
+  adding more overlapping tools.
+- The MCP project's [annotation guidance](https://blog.modelcontextprotocol.io/posts/2026-03-16-tool-annotations/)
+  distinguishes behavioral hints from security guarantees. A read-only annotation
+  is not authorization or proof of safe retry. The host must enforce permissions.
+
+## Concrete findings addressed
+
+1. Structured-only MCP results were kept in metadata but described as empty in
+   model-visible text. They now render as compact JSON if no other text exists.
+2. Static tool metadata could override current execution success/failure. Current
+   execution status now wins; non-boolean read-only hints are not accepted.
+3. Result deduplication compared raw output but ignored changing semantic views.
+   Both must now match before a repeated result is suppressed.
+4. Syntactically valid but malformed schema-cache data could crash warm discovery.
+   Invalid descriptors now produce a cache miss and live discovery.
+
+See [reliability hardening](reliability-hardening.md) for the preceding session,
+stream lifecycle, token accounting, workspace, and targeted-recall changes.
+
+## Highest-value remaining work
+
+- **Durable budgets:** current session usage is in-memory. Company-wide or
+  cross-worker spend limits need durable accounting, atomic reservations and
+  reconciliation, not a prompt instruction or a per-agent counter.
+- **Real-provider evaluations:** run held-out tasks on actual configured models,
+  compare success rates, uncached/cache input, output, retries, latency and cost.
+  An offline payload reduction is not a billed-token reduction.
+- **Provider/cache behavior:** test prefix stability versus dynamic schemas and
+  compaction. Caching must be reported by the provider, not inferred from reuse.
+- **Production MCP lifecycle:** validate authentication renewal, server-side
+  tool changes, tenant scoping, and reconnect behavior with authorized test
+  connectors. Annotation hints are not substitutes for these checks.
+- **App-building evaluations:** expand beyond the tested small WSGI repair loop
+  to multi-file projects and real-model tasks with independent verification.
+
+More tools, more retries, or more subagents can increase cost. Optimization must
+hold correctness and evidence availability constant, and measure total cost per
+successful task rather than only the size of one request.

docs/guides/context-management.md

@@ -55,7 +55,9 @@ override these defaults.
 
 Optimized project agents also default to durable project-local stores:
 
-- `.shipit/sessions/<session-id>.json` keeps canonical multi-turn history.
+- `.shipit/sessions/~<session-id-sha256>.json` keeps canonical multi-turn history
+  without filename collisions. Use the session store API to resolve IDs; legacy
+  files remain readable after checking their embedded identity.
 - `.shipit/memory.json` keeps facts explicitly persisted by tools.
 
 Recreate the agent and reuse the same session ID to resume after a process

docs/guides/hooks.md

@@ -28,7 +28,7 @@ def log_tool_start(name, arguments):
 def log_tool_end(name, result):
     print(f"{name} returned {len(result.output)} chars")
 
-agent = Agent.with_builtins(
+agent = Agent(
     llm=OpenAIChatLLM(model="gpt-4o-mini"),
     hooks=hooks,
 )
@@ -44,6 +44,7 @@ result = agent.run("What is the weather in Tokyo?")
 | `after_llm` | `fn(response: LLMResponse)` | After each LLM completion returns |
 | `before_tool` | `fn(name: str, arguments: dict)` | Before a tool is executed |
 | `after_tool` | `fn(name: str, result: ToolResult)` | After a tool returns (success or error) |
+| `user_prompt` | `fn(prompt: str)` | Rewrite or deny the incoming prompt before the model receives it |
 | `stop` | `fn(answer: str)` | When the agent is about to finish; return a reason to keep it working |
 
 ## Registration
@@ -166,6 +167,59 @@ profile = (
 
 `AgentHooks` works identically with `AsyncAgentRuntime`. The hook callbacks themselves are synchronous — the async runtime calls them inline between awaits.
 
+This includes ordinary `Agent.run`, `stream`, `arun`, and `astream`, not only
+the built-in-tools factory. Keep callbacks fast: slow network calls or sleeps
+inside a callback block the async event loop. Returning an awaitable raises
+`TypeError`; an `async def` policy must not be silently treated as approval.
+
+## Custom policies with ordinary Agent
+
+```python
+from shipit_agent import Agent, AgentHooks
+
+hooks = AgentHooks()
+
+@hooks.on_user_prompt
+def check_request(prompt):
+    if len(prompt) > 100_000:
+        return {"decision": "deny", "reason": "Request exceeds this application's limit."}
+
+@hooks.on_before_tool_matching("delete_*|send_*")
+def require_review(name, arguments):
+    return {"decision": "ask", "reason": "This action needs review."}
+
+@hooks.on_after_llm
+def record_usage(response):
+    usage_logger(response.usage)  # your application-owned logging function
+
+agent = Agent(llm=llm, tools=tools, hooks=hooks)
+```
+
+Configure your application's approval handler when using `ask`; a hook does
+not grant approval by itself. No provider-specific hook implementation is needed.
+
+Before-tool hooks run in registration order. Return a decision with
+`updated_arguments` to rewrite the call. Later hooks and the permission engine
+see those rewritten arguments. Deny wins over ask, and ask wins over allow;
+later rewrites do not erase an earlier approval requirement.
+
+### Output transformations and boundaries
+
+Replacing output through a string, output/text dictionary, or `ToolOutput`
+clears a stale `model_text` excerpt. A dictionary or `ToolOutput` can explicitly
+supply a new `model_text` if a separate compact model view is desired.
+
+Post-tool hooks run **after execution**. They cannot undo side effects or retract
+raw output deltas already streamed to subscribers. Do not rely on them alone to
+keep secrets out of logs/UI: redact at the tool source or buffer/filter events
+before forwarding them. Hook code is trusted application Python, not sandboxed.
+Create separately scoped callback state for different users; cloned agents can
+share callback objects. Model hooks are not a durable billing ledger.
+
+Ordinary callback exceptions propagate. Stop-hook exceptions are the documented
+exception: they emit `stop_hook_error` and allow finishing. Stop hooks are quality
+checks, not a security boundary, and their continuations can add token usage.
+
 ## Match and transform tool output
 
 Matcher hooks accept glob patterns separated by `|`. Post-tool hooks may

docs/reliability-hardening.md

@@ -0,0 +1,135 @@
+# Runtime reliability hardening
+
+## Provider usage and budgets
+
+OpenAI-compatible, LiteLLM, and Anthropic adapters preserve missing input/output
+token counters as unknown. An explicit zero is valid; a missing, negative,
+boolean, or non-integer value is not treated as free work. A reported total alone
+cannot determine the input/output split or cache semantics.
+
+Configured task/session budgets use this distinction to stop subsequent model
+requests when accounting is unavailable. These remain soft, between-step limits,
+not a guarantee that an in-flight request cannot exceed the limit. Cache savings
+must be measured from provider-reported counters; these changes do not establish
+a live cost reduction.
+
+## MCP replay safety
+
+The resilience wrapper retries allowlisted protocol discovery/read requests.
+It does not replay `tools/call` or unknown methods automatically. A timeout or
+disconnected socket does not prove the server failed to perform an action.
+Applications needing retries for writes should use server-supported idempotency
+or reconcile execution status first. This transport policy does not itself stop
+a model from requesting the action again in a later step.
+
+## File session identity
+
+New session files use SHA-256-derived filenames, avoiding separator, filename
+length, and case-insensitive filesystem collisions. Existing files can still be
+read after verifying their embedded session ID. Subsequent saves use the new
+filename; listings deduplicate legacy/current copies. Old files are not deleted.
+
+This prevents filename collisions, not unauthorized access: the host application
+must still authenticate users, authorize session access, and provide process-safe
+coordination for multiple workers. In-process session locks are not distributed
+locks. Do not treat the file store as a multi-tenant security boundary.
+
+## Async chat lifecycle and forks
+
+`AgentChatSession.asend`, `astream`, and `astream_packets` share the same chat
+state as synchronous sends. Event/packet callbacks remain synchronous and should
+be short and non-blocking. When abandoning a stream, explicitly close it (for
+example with `contextlib.aclosing`); merely breaking an async loop is not a
+guarantee of immediate cleanup. Closing these wrappers now awaits nested runtime
+cleanup and honors the configured runtime cancellation policy.
+Worker exceptions propagate after buffered events are drained; they no longer
+silently end an async stream as though it succeeded.
+
+```python
+from contextlib import aclosing
+
+chat = agent.chat_session(session_id=authorized_session_id)
+result = await chat.asend("Review the evidence")
+async with aclosing(chat.astream_packets("Compare with the earlier result", transport="sse")) as packets:
+    async for packet in packets:
+        await send_to_client(packet)
+```
+
+The host must authenticate and authorize `authorized_session_id` before creating
+the chat. A disconnected client should close the iterator. Do not convert an
+exception from the stream into a completed assistant message.
+
+`SessionManager` binds returned chats to its own store without changing the
+supplied agent. Forks deep-copy messages and discard derived facts, compaction
+summaries, and discovery checkpoints; these can otherwise reference messages
+after a historical fork point. Custom metadata is copied independently.
+
+Session token budgets still live in chat/agent runtime state and are not a
+durable company billing ledger. Reconstructing the agent can reset that budget;
+production cross-worker spending limits need durable accounting and coordination.
+
+## Validation boundaries
+
+### Targeted evidence recall and app-building tools
+
+Historical `recall_tool_result` accepts `query` alongside `call_id`, `offset`,
+and `limit`. It searches literal text case-insensitively (not model-generated
+regex), returning exact source text around the first match at or after offset.
+Use a small `limit` when only a fact or error is needed. No-match responses are
+explicit; normal paging remains available. The source result is unchanged.
+
+The regression fixture retrieves a relevant excerpt under 750 characters from
+over 400,000 characters of historical output. This is returned-text reduction,
+not a claim about real provider token billing or task success.
+
+Core workspace path resolution rejects similarly named sibling directories and
+symlink escapes. This is a file-tool boundary, not an OS sandbox for subprocesses.
+Exact edits reject empty search text, including `replace_all` requests.
+
+An app-building regression uses real file tools and Python subprocesses to write
+a WSGI greeting application, observe a failing assertion, read/edit the source,
+and verify the corrected response. It runs through both `Agent.run` and
+`Agent.stream`; model choices are scripted. It does not install dependencies,
+start a public server, or establish autonomous real-model coding quality.
+
+Regression coverage includes missing usage, cache-only usage, sync/stream budget
+enforcement, ambiguous MCP writes, and colliding session IDs. Existing suites
+cover large-catalog discovery, HTTP failure isolation, stream events, routing,
+and session behavior. Fixture tests do not establish live Gemma/Bedrock quality,
+production connector reliability, or comparative savings against another agent.
+
+The concurrent chat regression runs two isolated twenty-turn chats, alternating
+async sends and streams, with forty unique local tool executions. It checks prior
+user turns on every model call, exact terminal output, retained tool results, and
+separate usage totals. Its model and usage counts are scripted fixtures, not a
+live quality or cost benchmark.
+
+### Offline discovery measurement
+
+`PYTHONPATH=. .venv/bin/python scripts/benchmark_tool_discovery.py` exercised
+twenty successful fixture lookups per configuration. Automatic discovery versus
+eager loading reduced serialized request characters by 10.7% with 30 tools,
+56.6% with 100 tools, and 89.1% with 500 tools. With 10 tools it retained eager
+loading (no reduction). Discovery added one model request in each deferred case.
+These measure existing discovery behavior, not savings introduced by this patch.
+They are payload measurements, not tokenizer counts, cache hits, billed cost, or
+real-model task-success measurements.
+## MCP response and pagination boundaries
+
+HTTP MCP responses must match the outgoing request ID and contain an object
+result. Streamable HTTP skips unrelated SSE responses and accepts session
+headers only after response validation. This prevents unrelated content from
+being treated as tool evidence; it does not change SSE into incremental reading.
+
+`RemoteMCPServer(..., max_discovery_pages=100)` bounds each paginated listing.
+Increase the positive integer limit for servers with unusually small pages.
+Cursors remain opaque and are forwarded unchanged, following the
+[MCP pagination specification](https://modelcontextprotocol.io/specification/2025-11-25/server/utilities/pagination).
+Malformed pages, repeated cursors, and exhausted page budgets raise `MCPError`
+inside discovery. Failed tool discovery does not publish a partial catalog.
+The existing optional resources/prompts listing APIs still translate MCP errors
+to empty lists; they do not yet distinguish unsupported methods from failures.
+
+Regression coverage includes HTTP/SSE response identity, malformed payloads,
+session-header validation, cursor preservation, and bounded discovery. These
+are local transport fixtures, not live-provider or production-server validation.