Skip to content

Commit a9be23a

Browse files
committed
fix(discovery): integrate session lifecycle cleanup from main
Preserve the admission, drain, and retryable cleanup semantics from agentrust-io#634 while invalidating shared discovery and comparison caches together. Add public-call pagination/drain regressions across HTTP JSON, SSE, and stdio, plus failed-cleanup cache tests. Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
2 parents 8b8ba3b + c9d0133 commit a9be23a

14 files changed

Lines changed: 2393 additions & 113 deletions

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3535
unset those routes still fall back to the bearer token, so an existing
3636
single-token deployment keeps working until it sets the new variable.
3737

38+
- **Session close retained stdio children and per-session upstream caches.**
39+
(`#625`) `rebind_session()` rotated the audit chain and call logs but never
40+
called `aclose()`, so the stdio child, pooled HTTP clients, and the
41+
provenance/drift-checked caches all survived a close and were reused by the
42+
next session - the exact cross-session contamination `docs/spec/stdio-transport.md`
43+
names as the reason children are scoped to one session rather than pooled.
44+
Close now serializes session transitions, drains admitted calls before
45+
signing, cleans up resources before rebinding, and preserves claim and
46+
resource ownership across retryable failures. Partial claim failures keep
47+
admission sealed and are reported for operator investigation. A failed
48+
cancellation drain also keeps admission sealed until a close retry can
49+
drain the remaining work. Graceful shutdown rejects new work and resource
50+
acquisition, drains active calls, and coordinates spawning with cleanup;
51+
an incomplete drain is reported as failure. Concurrent first-use stdio
52+
spawning is serialized to avoid creating an untracked second child.
53+
Cancellation during session hydration is finalized before signing; a failed
54+
terminal audit write prevents signing or rotating an incomplete claim.
55+
`POST /sessions/{id}/reset` retires a session id and opens a successor, so it
56+
leaked the same resources for the same reason; it now drains admitted calls
57+
and releases them before recording the boundary, which also leaves a failed
58+
reset retryable with the session untouched. A reset naming an already-rotated
59+
session is rejected before draining, so it cannot cancel the successor's
60+
in-flight calls. A child that fails to close is retained for retry; a pooled
61+
HTTP client that fails to close is dropped and logged instead, because an
62+
`AsyncClient` marks itself closed and HTTPcore empties its pool before the
63+
streams are released, leaving nothing a retry could reach. A call arriving
64+
during a transition still waits for the successor, but the wait is bounded:
65+
a close that cannot resolve, such as one whose successor creation keeps
66+
failing, now answers callers with the reason instead of blocking them
67+
indefinitely.
68+
3869
### Added
3970

4071
- **The accumulated session-sensitivity value can now live in a shared,

LIMITATIONS.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Separately, the approved description rather than the live one is what the gatewa
2727

2828
HTTP and stdio discovery exhaust `tools/list` pagination before comparing either drift or provenance. A later-page failure, malformed discovery shape, duplicate tool name, repeated/cyclic cursor, or continuation beyond 1,000 pages makes the entire acquisition unchecked; no partial list is compared. Cursors are passed back unchanged, including an empty string. This bound limits page count, not total elapsed time or response bytes, and pagination does not establish an atomic snapshot of a changing server. This is acquisition validation, not full MCP schema validation or a new approval/hash policy. The unchecked-call behavior above is unchanged.
2929

30-
Drift and provenance share the completed first-contact acquisition for the same server and publisher authority within a session, rather than independently walking all pages. An unchecked acquisition is also cached until the next session. Cancellation leaves no cached acquisition; existing stdio child-close behavior is unchanged, not an automatic child restart. Session rebinding resets both comparisons and the shared acquisition. This avoids duplicate discovery work; it does not add continuous monitoring or make the listing an atomic snapshot.
30+
Drift and provenance share the completed first-contact acquisition for the same server and publisher authority within a session, rather than independently walking all pages. An unchecked acquisition is also cached until the next session. Cancellation leaves no cached acquisition; existing stdio child-close behavior is unchanged, not an automatic child restart. Close/reset drains admitted calls before cleanup invalidates both comparisons and the shared acquisition, even if resource cleanup subsequently fails. This avoids duplicate discovery work; it does not add continuous monitoring or make the listing an atomic snapshot.
3131

3232
**Phase 2 completeness: server-side attestation**
3333
Phase 1 attests the gateway boundary. It does not attest what happens on the other side of that boundary. The `tool_transcript.hash` field in the TRACE Claim records a hash of the audit chain tip, but the tool transcript binding that ties a specific tool execution to a specific response is Phase 2 work. Phase 1 partially addresses P1.4 (transitive trust into upstream dependencies) and P4.1 (typosquatted packages added to catalog) -- both are fully closed by Phase 2. Any compliance claim that relies on server-side proof must wait for Phase 2.
@@ -71,6 +71,15 @@ The TEE prevents plaintext from leaving the enclave to any destination not cover
7171
**Tool name collision via malicious catalog entries**
7272
The catalog binds each tool name to a specific upstream server identity, which prevents routing ambiguity for approved servers. It does not prevent a typosquatted or look-alike package from being added to the catalog in the first place. Catalog approval is human-gated. The gateway trusts the catalog; it cannot detect that a catalog entry was added via a compromised reviewer or a social engineering attack.
7373

74+
**Session cleanup is bounded by cooperation and by time**
75+
Session-scoped resources, meaning the stdio child, the pooled HTTP clients, and the provenance and drift caches, are released on every path that ends a session: `POST /sessions/{id}/close`, `POST /sessions/{id}/reset`, and graceful shutdown. None of those paths released anything before this was implemented, so the first three limits below are what remains rather than what was added. The last two are deliberate trades the behaviour introduces.
76+
77+
- **A pooled HTTP client that fails to close leaks its connections.** `AsyncClient` marks itself closed, and HTTPcore empties its pool, before the underlying streams are released, so nothing a retry could reach survives a failed close. The client is dropped so the successor cannot reuse it, and the failure is logged. A child process that fails to close is retained instead, and a close retry can still reap it.
78+
- **Graceful shutdown can outlast a deployment's termination grace period.** It waits for any in-flight close, then drains again on the same budget, so with the defaults cleanup can begin as late as seventy seconds in. A shorter grace period ends in SIGKILL and none of this runs. Size the grace period above twice `CMCP_SESSION_CLOSE_DRAIN_SECONDS`, or lower that deadline.
79+
- **Cancellation is cooperative, so a failed drain does not prove a call stopped.** Close requests cancellation at the deadline and allows a further five seconds to unwind. A call that does not honour it leaves the drain incomplete, which seals admission rather than signing a claim that omits an outcome.
80+
- **A failed terminal audit write leaves the session unavailable, with no repair.** A deliberate trade: it blocks signing, rotation, reset, and further admission for that session, because the alternative is a signed claim missing a call the gateway made. Restoring the writer does not reconstruct the missing outcome, and none is provided. Recovery is a new session.
81+
- **A close that trips the kill switch leaves the gateway with no live session.** Also deliberate. The claim for the closed session is signed and retrievable, but no successor can be created until an operator unblocks that agent identity. This is the kill switch working as specified, at the cost of availability.
82+
7483
## What Level 0 (CMCP_DEV_MODE) does not provide
7584

7685
`CMCP_DEV_MODE=1` uses a software-only TEE provider. It is suitable for development, testing, and demo scenarios. It does not satisfy production governance requirements because:

docs/spec/stdio-transport.md

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,46 @@ the launch measurement and must be reported as a distinct evidence class, not fo
111111

112112
1. **Lifecycle.** Implemented as children scoped to a session, reused by execution identity within it, and closed with that session. The original alternative was a pool across sessions. A pool is
113113
faster and leaks state between sessions, which is exactly the kind of cross-session
114-
contamination the audit chain cannot see.
114+
contamination the audit chain cannot see. "Closed with that session" covers every way a
115+
session ends: an explicit `POST /sessions/{id}/close`, the gateway process exiting
116+
with a session still live during graceful shutdown, and `POST /sessions/{id}/reset`,
117+
which also retires a session id and opens a successor. Reset drains admitted calls
118+
and releases the same session-scoped resources before it records the boundary, so
119+
the successor never inherits a child, a pooled client, or a provenance entry from
120+
the session it replaced. If a child fails to close, the current session ID and
121+
audit boundary remain unchanged, admission stays sealed, and a retry closes the
122+
retained child before recording the reset.
123+
A call that arrives during a transition waits for it and is admitted to the
124+
successor, but that wait is bounded: a transition that has already failed is
125+
lifted only by a close retry or operator action, so a call waiting past the
126+
bound is answered with the reason rather than held on an open socket.
127+
Close blocks new calls,
128+
waits up to `CMCP_SESSION_CLOSE_DRAIN_SECONDS` (default 30 seconds), then
129+
requests cancellation and allows a further five seconds for calls to unwind.
130+
If calls remain, close fails with `SessionDrainIncomplete` and admission stays
131+
sealed; a retry must drain them before signing and rebinding. Partial claim
132+
failure also seals admission and requires operator investigation. Successful
133+
task completion alone does not prove audit completeness: a failed terminal
134+
audit write prevents signing, rotation, reset, and further call admission.
135+
Hydration failures and cancellations are included in terminal finalization.
136+
Shutdown can still release resources without signing an incomplete claim.
137+
Successful cleanup precedes rebinding; a child that fails to close is retained
138+
for retry. Pooled HTTP clients are closed on a best-effort basis instead: an
139+
`AsyncClient` marks itself closed and HTTPcore empties its pool before the
140+
underlying streams are released, so a failed close leaves connections no retry
141+
reaches through any public API. Such a client is dropped and the failure logged
142+
rather than sealing the session, because it is reuse by the successor, not the
143+
socket, that this lifecycle rule exists to prevent.
144+
Graceful shutdown permanently rejects new work and resource acquisition,
145+
drains active calls, and serializes spawning with cleanup. An incomplete
146+
drain, or a child that could not be closed, is reported as shutdown failure
147+
rather than success; a pooled client that could not be closed is logged and
148+
does not fail the shutdown, for the reason given above. Hard
149+
process termination cannot run this cleanup, and a shutdown that waits out an
150+
in-flight close can need twice the drain budget before cleanup begins, so a
151+
deployment's termination grace period has to exceed it or the cleanup is cut
152+
short by the kill. These drain deadlines do not
153+
bound arbitrary signing or transport cleanup time.
115154
2. **stderr.** The implementation logs diagnostics through the gateway logger and records a byte count in evidence. MCP servers write diagnostics there. Capturing it into the audit chain risks
116155
payload leakage into an artifact meant to be shareable; discarding it loses the only
117156
signal when a child misbehaves.

src/cmcp_runtime/errors.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,24 @@ class TeeFault(CMCPError):
130130
http_status = 500
131131

132132

133+
class SessionCloseIncomplete(CMCPError):
134+
"""Terminal audit or close bookkeeping failed without a safe recovery.
135+
136+
Repeating accounting/signing is unsafe; operator investigation is required."""
137+
138+
code = "SESSION_CLOSE_INCOMPLETE"
139+
http_status = 500
140+
141+
142+
class SessionDrainIncomplete(CMCPError):
143+
"""Calls remain active after the drain deadline and cancellation grace.
144+
145+
Admission stays sealed; a transition retry must finish draining first."""
146+
147+
code = "SESSION_DRAIN_INCOMPLETE"
148+
http_status = 503
149+
150+
133151
class UpstreamUnavailable(CMCPError):
134152
code = "UPSTREAM_UNAVAILABLE"
135153
http_status = 502

0 commit comments

Comments
 (0)