Repository navigation
Conversation
|
@salty-hambot review |
![salty-hambot[bot]](https://avatars.githubusercontent.com/in/2952321?s=60&v=4){kind=small}
|
@salty-hambot review |
| cancel it through that cursor as usual. Attaching sends no SQL, never creates | ||
| an execution, and does not extend server result retention. It raises the same | ||
| missing/cancelled execution errors described above. A second cursor cannot | ||
| attach an execution already tracked by that connection. |
There was a problem hiding this comment.
nit: cursor.attach() transfers ownership of a running execution, even if the previous connection is still open: sql-session's _take_ownership() replaces the owner, and later events go to the new connection. the duplicate check only covers cursors on the same connection, so another live connection can attach again and move event ownership back. similar to the above, are we assuming this is the caller's problem to solve?
| raise ProgrammingError("This cursor already tracks the execution") | ||
| # Preserve the current execution until the new attachment is accepted. | ||
| results: queue.Queue[ExecutionResult] = queue.Queue() | ||
| attached_id = self.__attach_fn(execution_id, results.put, store) |
There was a problem hiding this comment.
nit: sql-session's cancel() looks up the execution by ID without checking whether the requester still owns it. after a recovery worker attaches here, an old worker that still considers the execution pending can reach cursor.close() or reuse its cursor, send cancel, and kill the recovered execution. is the intention that this "ownership" logic will be part of the mcp server's usage of this?
Summary
A new client cannot recover a known SQL execution through the public driver API. The driver only reattaches queries already tracked in that process, which blocks MCP's Postgres-backed restart recovery.
Add
cursor.attach(execution_id, store=original_store)on a connection to the original SQL session. Fetching and cancellation reuse the existing recovery machinery. Attaching sendsretrieve_resultswithattach, neverexecute_sql. Missing or cancelled executions preserve their existing errors, and a rejected duplicate attach preserves the current execution and both cursors' result handlers.Related Issues
WBC-1934, a prerequisite for WBC-996 in Modern MCP architecture.
Verification
uv run pytest tests/test_attach.py tests/test_cursor.py tests/test_reconnect.py -q: 140 passed after the duplicate-attach fix, including preservation of pending/completed executions and successful replacement.uv run pytest -q: 349 passed before the final duplicate-attach fix on Python 3.11, with two existing WebSocket deprecation warnings. Covers public attachment to running/completed executions, stored results, cancellation, missing executions, duplicate tracking, invalid IDs, old-session polling, and another transport loss.uv run pre-commit run --all-files: all hooks passed.uv build: source distribution and wheel built successfully.execute_sqland oneretrieve_resultsrequest with the same ID, with no replay. Both clients, server, and temporary TLS files were cleaned up.Independent adversarial review of the implementation and duplicate-attach fix found no substantive issues. The fix review independently passed 10 focused tests and an immediate-result check.
On
b9763530db0209523fe2dec422392028ec51d765, CI passed on Python 3.10 through 3.14, hosted pre-commit passed, and Salty reviewed with no findings.No live SQL session, Postgres database, cloud credential, or deployed service was used. Authentication and real server orphan-grace/result-retention behavior need integration qualification. This API neither extends result retention nor provides a durable job store. The remaining MCP work must persist execution identity safely and reconcile uncertain submissions without replaying writes.
Requester Checklist
Visual Proof
Local restart smoke output:
Size Justification (if L/XL)
The change includes connection and cursor integration plus lifecycle tests. The cursor resets execution state through one shared method so execute and attach preserve the same result isolation.
Reviewer Checklist
Implemented by GPT-6.1 Sol through T3 Code's Codex harness, with an independent agent review.