From 2ffbcd62e1ce0a8b8f1d861cd21d9e28f6d21c5d Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sun, 19 Apr 2026 18:19:56 +0800 Subject: [PATCH 1/7] docs(adr): define pg phase 2 ownership model Record the Phase 2 boundary after Phase 1 PG persistence and add an implementation checklist for owner, lease, fencing, routing, and failover work in the HTTP session stack. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...-session-ownership-for-pg-http-sessions.md | 64 +++++++ .../pg-phase2-multi-instance-checklist.md | 168 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 coding-agent/docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md create mode 100644 coding-agent/docs/pg-phase2-multi-instance-checklist.md diff --git a/coding-agent/docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md b/coding-agent/docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md new file mode 100644 index 000000000..7da30b6d4 --- /dev/null +++ b/coding-agent/docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md @@ -0,0 +1,64 @@ +# ADR-0013: Define Phase 2 multi-instance session ownership for PG HTTP sessions + +**Status**: Proposed +**Date**: 2026-04-19 + +## Context + +ADR-0012 closes the Phase 1 PostgreSQL persistence gap for the HTTP/UI stack by making session metadata, tape history, and checkpoint snapshots durable in PostgreSQL. But it keeps active runtime state (`task`, approval state, event queues, pipeline/context/adapter ownership) single-instance and local-authoritative. + +That Phase 1 boundary is acceptable for restart-safe storage, but it is not sufficient for multi-pod HTTP deployment. Once multiple application instances can read the same persisted session metadata, the repository needs an explicit answer to three questions that Phase 1 intentionally left open: who owns execution for a session right now, how other instances know they are no longer allowed to act, and how approval/event flows reach the active owner. + +Phase 2 is the ownership and coordination layer that sits on top of Phase 1 persistence. It does not primarily add “more PostgreSQL persistence”; it defines how multiple instances safely share a PostgreSQL-backed session/tape/checkpoint substrate without split-brain behavior. + +## Decision + +Define Phase 2 as a multi-instance coordination layer for HTTP sessions with four core properties: + +- Each active HTTP session has one current runtime **owner** recorded in PostgreSQL. +- Ownership is time-bounded by a renewable **lease** rather than an infinite claim. +- Every ownership grant yields a monotonically increasing **fencing token** that must accompany owner-sensitive writes and commands. +- Runtime execution, approval handling, and event streaming are all **owner-routed** operations rather than implicit local-memory behavior. + +Specifically, Phase 2 will include: + +- A PostgreSQL-backed `session_owner` record keyed by `session_id`, storing `owner_id`, `lease_expires_at`, `fencing_token`, and timestamps. +- App-layer primitives to `acquire`, `renew`, and `release` session ownership atomically. +- Owner checks in `run_agent`, checkpoint restore, and close/shutdown flows before they perform runtime-sensitive work. +- Fencing-token validation so a stale owner cannot continue mutating session-associated state after ownership changes. +- An explicit routing rule for approval responses and SSE/event delivery so only the active owner drives runtime-visible side effects. +- A first failover contract of **at-most-once** execution for in-flight turns: after owner loss, a new owner may rebuild cold state from persisted data, but it does not resume partially executed runtime state. + +Phase 2 should initially prefer a minimal operational model: + +- sticky routing is acceptable as the first event/approval routing strategy, +- in-flight turn replay/resume is out of scope, +- only owner-sensitive actions need fencing in the first cut. + +## Alternatives Rejected + +- Keep Phase 2 as “just add more PG-backed state” — rejected because Phase 1 already made durable state explicit; the unresolved risk is multi-instance coordination, not missing tables. +- Let any instance read persisted session state and optimistically execute if local cache is empty — rejected because it allows concurrent turn execution, stale approval handling, and conflicting close/restore behavior. +- Solve multi-instance coordination with only sticky load balancing and no persisted owner record — rejected because it provides no correctness guarantee under pod restarts, manual retries, or load balancer drift. +- Attempt best-effort in-flight turn resume in the first Phase 2 slice — rejected because the repository still treats runtime objects as local-authoritative and not serializable across owners. + +## Acceptance Criteria + +- [ ] `test_session_owner_acquire_conflicts_with_live_lease` +- [ ] `test_session_owner_renew_rejects_stale_token` +- [ ] `test_run_agent_rejects_non_owner_instance` +- [ ] `test_restore_checkpoint_rejects_stale_owner` +- [ ] `test_close_session_rejects_stale_owner` +- [ ] `test_approval_response_is_rejected_after_owner_change` +- [ ] `test_session_failover_rebuilds_from_persisted_state_without_resuming_inflight_turn` +- [ ] `uv run pytest tests/agentkit/ tests/ui/ -k "owner or lease or fencing or failover" -v` + +## References + +- `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` +- `docs/adr/0003-http-sessions-use-one-stable-tape-timeline.md` +- `src/coding_agent/ui/session_manager.py` +- `src/coding_agent/ui/http_server.py` +- `src/coding_agent/ui/session_store.py` +- `src/agentkit/storage/pg.py` +- Historical design context: `docs/specs/checkpoint-design-section2a.md` diff --git a/coding-agent/docs/pg-phase2-multi-instance-checklist.md b/coding-agent/docs/pg-phase2-multi-instance-checklist.md new file mode 100644 index 000000000..4109688d0 --- /dev/null +++ b/coding-agent/docs/pg-phase2-multi-instance-checklist.md @@ -0,0 +1,168 @@ +# PostgreSQL Phase 2 — Multi-instance checklist + +This document is an implementation-oriented companion to ADR-0013. It does not replace the ADR; it expands the concrete pieces that Phase 2 needs so implementation can be planned incrementally. + +## Scope Summary + +Phase 1 made these durable in PostgreSQL: + +- HTTP session metadata +- tape history +- checkpoint snapshots/catalogs + +Phase 2 defines who is allowed to act on that persisted state when more than one HTTP instance exists. + +## 1. Ownership data model + +Add a dedicated owner/lease table, for example: + +```sql +CREATE TABLE session_owners ( + session_id TEXT PRIMARY KEY, + owner_id TEXT NOT NULL, + lease_expires_at TIMESTAMPTZ NOT NULL, + fencing_token BIGINT NOT NULL, + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +``` + +Recommended semantics: + +- `owner_id`: stable process/pod identifier +- `lease_expires_at`: absolute lease expiry +- `fencing_token`: monotonically increasing per successful acquire +- `updated_at`: audit/debug support + +## 2. Ownership primitives + +Define one narrow app-layer interface for ownership: + +```python +class SessionOwnerStore(Protocol): + async def acquire(self, session_id: str, owner_id: str, lease_ttl_seconds: float) -> OwnerLease: ... + async def renew(self, session_id: str, owner_id: str, fencing_token: int, lease_ttl_seconds: float) -> OwnerLease: ... + async def release(self, session_id: str, owner_id: str, fencing_token: int) -> None: ... + async def get(self, session_id: str) -> OwnerLease | None: ... +``` + +Acquire rules: + +- succeed if no live lease exists +- fail if another owner still holds a live lease +- increment fencing token on every successful acquire + +Renew rules: + +- only current owner + current token may renew +- stale token must fail + +Release rules: + +- only current owner + current token may release +- stale release must be ignored/rejected + +## 3. Runtime owner checks + +Guard these actions behind ownership validation: + +- `SessionManager.run_agent` +- `SessionManager.capture_checkpoint` +- `SessionManager.restore_checkpoint` +- `SessionManager.close_session` +- `SessionManager.shutdown_session_runtime` + +Minimum rule: + +- if this instance is not the active owner for the session, do not execute runtime-sensitive work +- return a conflict/error that callers can treat as a retry or routing problem + +## 4. Fencing points + +Fencing is only useful if stale owners are rejected at side-effect boundaries. + +Phase 2 MVP should require fencing on: + +- turn start (`run_agent`) +- checkpoint restore +- session close +- approval response submission + +Phase 2 later can extend fencing to: + +- event publishing metadata +- checkpoint capture metadata +- owner-sensitive session metadata writes + +## 5. Event routing + +Current HTTP event queues are local in-memory structures. Phase 2 must choose a routing model. + +### MVP recommendation: sticky routing + +- clients reconnect to the same owner instance +- only owner instance serves live event stream for a session +- non-owner instance returns a conflict/redirect-style error or terminates the stream cleanly + +### Future option: brokered routing + +- owner publishes events to shared event transport +- any edge instance can proxy SSE + +Do not attempt brokered routing in the first Phase 2 slice unless ownership is already stable. + +## 6. Approval routing + +Current approval state is runtime-local. Phase 2 needs explicit routing: + +- approval requests are owned by the current session owner +- approval responses must include session + request identity and be rejected if owner/token changed + +Suggested persisted approval metadata: + +- `request_id` +- `session_id` +- `owner_id` +- `fencing_token` +- `status` +- `created_at` + +## 7. Failover semantics + +Phase 2 MVP should choose **at-most-once** for in-flight turns. + +That means: + +- if the owner dies during a turn, the turn is not resumed mid-flight +- a new owner may rebuild cold state from persisted session metadata, tape history, and checkpoints +- clients must retry or reconnect after failover + +This is consistent with the current repository boundary where runtime objects remain local-authoritative and are not serializable. + +## 8. Restore / close consistency + +Define these invariants: + +- `run_agent`, `restore_checkpoint`, and `close_session` are mutually exclusive owner-sensitive operations +- non-owner instances cannot execute them locally +- stale owners must fail before mutating shared state + +## 9. Suggested implementation order + +1. Add `session_owners` table + PG store +2. Add acquire / renew / release APIs + tests +3. Add owner checks to `run_agent` +4. Add owner checks to restore / close / shutdown +5. Add approval request/response ownership metadata +6. Add sticky-routing behavior for SSE and approval endpoints +7. Add failover tests and operational metrics + +## 10. Suggested tests + +- `test_session_owner_acquire_conflicts_with_live_lease` +- `test_session_owner_renew_rejects_stale_token` +- `test_session_owner_release_rejects_stale_token` +- `test_run_agent_rejects_non_owner_instance` +- `test_close_session_rejects_non_owner_instance` +- `test_restore_checkpoint_rejects_stale_owner` +- `test_approval_response_is_rejected_after_owner_change` +- `test_failover_rebuilds_runtime_from_persisted_tape_without_resuming_inflight_turn` From 86b479b377ab23a4c09b37e48759bb407be53ef8 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sun, 19 Apr 2026 23:12:09 +0800 Subject: [PATCH 2/7] docs(adr): define execution binding layer Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...xecution-binding-from-runtime-ownership.md | 66 + ...ssion-execution-binding-and-thin-phase2.md | 1238 +++++++++++++++++ 2 files changed, 1304 insertions(+) create mode 100644 coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md create mode 100644 coding-agent/docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md diff --git a/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md b/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md new file mode 100644 index 000000000..72196ee7a --- /dev/null +++ b/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md @@ -0,0 +1,66 @@ +# ADR-0014: Separate session execution binding from runtime ownership + +**Status**: Proposed +**Date**: 2026-04-19 + +## Context + +ADR-0013 defines Phase 2 multi-instance session ownership through owner records, leases, and fencing tokens. That layer answers "which instance may drive this session right now." It does not answer "where do the session's tools and files run," and it does not address how a session moves between execution environments while keeping the same durable identity. + +Today `SessionManager` conflates three concepts inside the `Session` object and its `repo_path` field: + +1. **Durable session metadata** (id, provider, model, policy, tape_id) that survives restarts. +2. **Execution/workspace binding** (`repo_path`, tool filesystem scope, shell working directory) that decides where `file_read`, `file_write`, and `bash_run` operate. +3. **Current runtime owner** (`task`, `approval_event`, `event_queues`, `runtime_pipeline`, `runtime_ctx`, `runtime_adapter`) that is local-authoritative and ephemeral. + +These three concepts change on different timelines. Session metadata changes when the user switches models or policies. Execution binding changes when a user moves from a local repository to a cloud workspace, or when a local `repo_path` is remounted. Runtime owner changes every time a lease expires, an instance restarts, or load balancing shifts traffic. + +Because `SessionManager` stores `repo_path` directly on the session and passes it to `create_agent_for_session` as `workspace_root`, there is no place to introduce a cloud workspace binding without overloading `repo_path` with incompatible semantics. The HTTP server also keeps approval state, event queues, and task ownership in local memory, which makes it impossible to reason about failover without first knowing whether the new owner can satisfy the old owner's execution contract. + +## Decision + +Introduce a first-class **execution binding** abstraction that sits beside the ownership layer defined in ADR-0013. The binding layer decides where a session's tools execute; the ownership layer decides which instance drives the session. The two layers are orthogonal and change independently. + +Specifically: + +- Add an `ExecutionBinding` dataclass/protocol in `coding_agent.ui.execution_binding` with at least two concrete variants: + - `LocalExecutionBinding(workspace_root: Path)` for local filesystem mode. + - `CloudWorkspaceBinding(workspace_url: str, workspace_id: str)` for remote workspace mode. +- Store the serialized binding on the session metadata table as `execution_binding`, separate from `repo_path`. Migrate the current `repo_path` field to mean "local workspace root" only. +- Add a `BindingResolver` interface that turns an `ExecutionBinding` into the `workspace_root` and tool configuration that `create_agent_for_session` expects. The resolver lives in `coding_agent.ui.binding_resolver`. +- Update `SessionManager` to resolve the binding before calling `create_agent_for_session`, so `run_agent`, `ensure_session_runtime`, and checkpoint restore all use the resolved workspace consistently. +- Update `Session.to_store_data` and `Session.from_store_data` to round-trip the binding as a typed dict, and default missing bindings to `LocalExecutionBinding(repo_path)` for backward compatibility. +- Keep the `Session` runtime fields (`task`, `approval_event`, `event_queues`, `runtime_pipeline`, `runtime_ctx`, `runtime_adapter`) untouched. Those belong to the ownership layer (ADR-0013) and remain local-authoritative. + +This ADR does not add cloud workspace tool implementations. It only creates the abstraction and the local binding variant so that future cloud bindings slot in without changing `SessionManager` or `http_server.py`. + +## Alternatives Rejected + +- Overload `repo_path` to mean both local path and cloud workspace identifier — rejected because it creates a semantic mismatch. A `Path` cannot represent a cloud workspace URL, and conditional parsing based on string prefixes is fragile. +- Encode binding information inside the `CheckpointSnapshot.extra` field — rejected because binding is session-level metadata, not checkpoint-level state. Restoring a checkpoint should not change where tools execute. +- Wait until cloud workspace tools exist before adding the abstraction — rejected because `SessionManager` and `http_server.py` already carry local-only assumptions (approval state, event queues, task ownership) that will harden into technical debt without a binding boundary. +- Merge execution binding and runtime owner into a single "session placement" record — rejected because owner identity and workspace identity change on different timelines. A session can fail over to a new owner while keeping the same binding, and a session can change binding while staying on the same owner. + +## Acceptance Criteria + +- [ ] `test_local_binding_resolves_to_workspace_root_for_create_agent` +- [ ] `test_session_metadata_round_trips_local_execution_binding` +- [ ] `test_session_metadata_defaults_missing_binding_to_local_from_repo_path` +- [ ] `test_cloud_workspace_binding_serializes_to_typed_dict` +- [ ] `test_binding_resolver_rejects_unknown_binding_kind` +- [ ] `test_run_agent_uses_resolved_workspace_root_from_binding` +- [ ] `test_restore_checkpoint_preserves_execution_binding` +- [ ] `test_http_server_create_session_stores_local_binding_by_default` +- [ ] `uv run pytest tests/ui/test_session_manager_public_api.py tests/ui/test_session_manager_runtime.py tests/ui/test_http_server.py -k "binding" -v` + +## References + +- `docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md` +- `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` +- `src/coding_agent/ui/session_manager.py` +- `src/coding_agent/ui/http_server.py` +- `src/coding_agent/app.py` +- `src/coding_agent/plugins/core_tools.py` +- `tests/ui/test_session_manager_public_api.py` +- `tests/ui/test_session_manager_runtime.py` +- `tests/ui/test_http_server.py` diff --git a/coding-agent/docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md b/coding-agent/docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md new file mode 100644 index 000000000..c7d78a11e --- /dev/null +++ b/coding-agent/docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md @@ -0,0 +1,1238 @@ +# Session Execution Binding and Thin Phase 2 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a first-class execution binding abstraction that separates WHERE a session runs from WHO owns it, then build the thin Phase 2 substrate (owners, leases, fencing) on top of Phase 1 PostgreSQL persistence. + +**Architecture:** Introduce `ExecutionBinding` as a typed dataclass hierarchy in `coding_agent.ui.execution_binding`, with a `BindingResolver` that converts bindings into `workspace_root` for `create_agent_for_session`. Layer ADR-0013's owner/lease/fencing on top via `SessionOwnerStore` in `agentkit.storage.pg`. Keep runtime state local-authoritative and do not attempt cross-owner serialization. + +**Tech Stack:** Python 3.12, `uv`, `pytest`, PostgreSQL (via `asyncpg`), FastAPI, existing `agentkit` pipeline. + +--- + +## File Map + +| File | Responsibility | +|------|---------------| +| `src/coding_agent/ui/execution_binding.py` | `ExecutionBinding` base class, `LocalExecutionBinding`, `CloudWorkspaceBinding`, serialization helpers | +| `src/coding_agent/ui/binding_resolver.py` | `BindingResolver` protocol and concrete resolver that turns bindings into `workspace_root` and tool config | +| `src/coding_agent/ui/session_owner_store.py` | App-layer `SessionOwnerStore` with acquire/renew/release and fencing token primitives | +| `src/agentkit/storage/pg.py` | PostgreSQL `session_owners` table schema, SQL for atomic acquire/renew/release | +| `src/coding_agent/ui/session_manager.py` | Integrate binding resolution into `run_agent`, `ensure_session_runtime`, checkpoint restore; add owner checks | +| `src/coding_agent/ui/http_server.py` | Add owner checks to prompt/approve/restore endpoints; sticky routing guards | +| `tests/ui/test_execution_binding.py` | Binding serialization, deserialization, resolver behavior | +| `tests/ui/test_session_owner_store.py` | Owner acquire/renew/release, fencing token validation, lease expiry | +| `tests/ui/test_session_manager_owner_checks.py` | Owner checks reject stale owners in `run_agent`, restore, close | +| `tests/ui/test_http_server_failover.py` | Sticky routing rejection, stale-owner 409/403 responses | +| `tests/agentkit/storage/test_pg.py` | `PGSessionOwnerStore` round-trip, atomic conflict behavior | + +--- + +## Scope Exclusions + +- **In-flight turn resume is out of scope.** After owner loss, a new owner rebuilds cold state from persisted data but does not resume partially executed runtime state. This matches ADR-0013's at-most-once contract. +- **Brokered event routing is out of scope for the first slice.** Sticky routing is acceptable; cross-instance event forwarding comes later. +- **Cloud workspace tool implementations are out of scope.** We define `CloudWorkspaceBinding` but do not implement cloud file/shell tools. + +--- + +## Current Risks in `SessionManager` and `http_server.py` + +Before implementing, the engineer should understand these local-only assumptions that make multi-instance deployment unsafe today: + +1. **Local approval state:** `session.approval_event` is an `asyncio.Event` in local memory. If the owner changes, the old event is lost and the new owner has no signal. +2. **Local event queues:** `session.event_queues` is a list of `asyncio.Queue` objects. They do not survive owner change and cannot be reached from another instance. +3. **Local task/runtime ownership:** `session.task`, `session.runtime_pipeline`, `session.runtime_ctx`, and `session.runtime_adapter` are all local objects. They are not serializable across owners. +4. **`repo_path` semantic mismatch:** `repo_path` is a `Path` on the `Session` object. It is passed to `create_agent_for_session` as `workspace_root`. In a cloud workspace scenario, `repo_path` cannot represent a remote URL, and conditional string parsing is fragile. + +The binding abstraction fixes risk 4. The owner/lease/fencing layer fixes risks 1-3 by making owner-sensitive actions fail fast when the local instance is no longer the owner. + +--- + +## Task 1: Introduce Execution Binding Abstraction + +**Files:** +- Create: `src/coding_agent/ui/execution_binding.py` +- Create: `tests/ui/test_execution_binding.py` + +- [ ] **Step 1: Write the binding dataclasses** + +```python +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Literal + + +@dataclass(frozen=True) +class ExecutionBinding: + kind: Literal["local", "cloud"] + + def to_dict(self) -> dict[str, Any]: + raise NotImplementedError + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> ExecutionBinding: + kind = data.get("kind") + if kind == "local": + return LocalExecutionBinding.from_dict(data) + if kind == "cloud": + return CloudWorkspaceBinding.from_dict(data) + raise ValueError(f"unknown binding kind: {kind}") + + +@dataclass(frozen=True) +class LocalExecutionBinding(ExecutionBinding): + workspace_root: str + + def __post_init__(self) -> None: + object.__setattr__(self, "kind", "local") + + def to_dict(self) -> dict[str, Any]: + return {"kind": "local", "workspace_root": self.workspace_root} + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> LocalExecutionBinding: + root = data.get("workspace_root") + if not isinstance(root, str): + raise TypeError("local binding requires string workspace_root") + return cls(workspace_root=root) + + +@dataclass(frozen=True) +class CloudWorkspaceBinding(ExecutionBinding): + workspace_url: str + workspace_id: str + + def __post_init__(self) -> None: + object.__setattr__(self, "kind", "cloud") + + def to_dict(self) -> dict[str, Any]: + return { + "kind": "cloud", + "workspace_url": self.workspace_url, + "workspace_id": self.workspace_id, + } + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> CloudWorkspaceBinding: + url = data.get("workspace_url") + wid = data.get("workspace_id") + if not isinstance(url, str) or not isinstance(wid, str): + raise TypeError("cloud binding requires string workspace_url and workspace_id") + return cls(workspace_url=url, workspace_id=wid) +``` + +- [ ] **Step 2: Write the failing tests** + +```python +from pathlib import Path + +import pytest + +from coding_agent.ui.execution_binding import ( + CloudWorkspaceBinding, + ExecutionBinding, + LocalExecutionBinding, +) + + +def test_local_binding_round_trip() -> None: + binding = LocalExecutionBinding(workspace_root="/tmp/repo") + restored = ExecutionBinding.from_dict(binding.to_dict()) + assert isinstance(restored, LocalExecutionBinding) + assert restored.workspace_root == "/tmp/repo" + + +def test_cloud_binding_round_trip() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + restored = ExecutionBinding.from_dict(binding.to_dict()) + assert isinstance(restored, CloudWorkspaceBinding) + assert restored.workspace_url == "https://workspace.example.com" + assert restored.workspace_id == "ws-123" + + +def test_unknown_binding_kind_raises() -> None: + with pytest.raises(ValueError, match="unknown binding kind"): + ExecutionBinding.from_dict({"kind": "unknown"}) + + +def test_local_binding_requires_string_workspace_root() -> None: + with pytest.raises(TypeError, match="string workspace_root"): + LocalExecutionBinding.from_dict({"kind": "local", "workspace_root": 123}) +``` + +- [ ] **Step 3: Run tests to verify they pass** + +Run: `uv run pytest tests/ui/test_execution_binding.py -v` +Expected: PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/coding_agent/ui/execution_binding.py tests/ui/test_execution_binding.py +git commit -m "feat: add ExecutionBinding abstraction for local and cloud workspaces" +``` + +--- + +## Task 2: Add Binding Resolver + +**Files:** +- Create: `src/coding_agent/ui/binding_resolver.py` +- Modify: `tests/ui/test_execution_binding.py` + +- [ ] **Step 1: Write the resolver** + +```python +from __future__ import annotations + +from pathlib import Path +from typing import Any, Protocol + +from coding_agent.ui.execution_binding import ( + CloudWorkspaceBinding, + ExecutionBinding, + LocalExecutionBinding, +) + + +class BindingResolver(Protocol): + def resolve_workspace_root(self, binding: ExecutionBinding) -> Path: ... + + def resolve_tool_config(self, binding: ExecutionBinding) -> dict[str, Any]: ... + + +class DefaultBindingResolver: + def resolve_workspace_root(self, binding: ExecutionBinding) -> Path: + if isinstance(binding, LocalExecutionBinding): + return Path(binding.workspace_root).resolve() + if isinstance(binding, CloudWorkspaceBinding): + raise NotImplementedError( + "cloud workspace resolution is not yet implemented" + ) + raise ValueError(f"unsupported binding type: {type(binding).__name__}") + + def resolve_tool_config(self, binding: ExecutionBinding) -> dict[str, Any]: + if isinstance(binding, LocalExecutionBinding): + return {"workspace_root": str(self.resolve_workspace_root(binding))} + if isinstance(binding, CloudWorkspaceBinding): + raise NotImplementedError( + "cloud workspace tool config is not yet implemented" + ) + raise ValueError(f"unsupported binding type: {type(binding).__name__}") +``` + +- [ ] **Step 2: Add resolver tests** + +Append to `tests/ui/test_execution_binding.py`: + +```python +from pathlib import Path + +from coding_agent.ui.binding_resolver import DefaultBindingResolver + + +def test_local_resolver_returns_absolute_path() -> None: + binding = LocalExecutionBinding(workspace_root="/tmp/repo") + resolver = DefaultBindingResolver() + assert resolver.resolve_workspace_root(binding) == Path("/tmp/repo").resolve() + + +def test_cloud_resolver_raises_not_implemented() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + resolver = DefaultBindingResolver() + with pytest.raises(NotImplementedError): + resolver.resolve_workspace_root(binding) +``` + +- [ ] **Step 3: Run tests** + +Run: `uv run pytest tests/ui/test_execution_binding.py -v` +Expected: PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/coding_agent/ui/binding_resolver.py tests/ui/test_execution_binding.py +git commit -m "feat: add DefaultBindingResolver for local execution" +``` + +--- + +## Task 3: Integrate Binding into Session Metadata + +**Files:** +- Modify: `src/coding_agent/ui/session_manager.py` +- Modify: `tests/ui/test_session_manager_public_api.py` + +- [ ] **Step 1: Add binding field to `Session`** + +In `src/coding_agent/ui/session_manager.py`, import `ExecutionBinding` and `LocalExecutionBinding`, then modify the `Session` dataclass: + +```python +from coding_agent.ui.execution_binding import ExecutionBinding, LocalExecutionBinding +``` + +Add to `Session`: + +```python + execution_binding: ExecutionBinding = field( + default_factory=lambda: LocalExecutionBinding(workspace_root=str(Path.cwd())) + ) +``` + +- [ ] **Step 2: Update `to_store_data` and `from_store_data`** + +In `to_store_data`, add: + +```python + "execution_binding": self.execution_binding.to_dict(), +``` + +In `from_store_data`, after `tape_id_raw` handling, add: + +```python + binding_raw = data.get("execution_binding") + if binding_raw is not None: + if not isinstance(binding_raw, dict): + raise TypeError("session metadata has invalid execution_binding") + execution_binding = ExecutionBinding.from_dict(binding_raw) + else: + execution_binding = LocalExecutionBinding( + workspace_root=str( + Path(repo_path_raw).resolve() + if repo_path_raw is not None + else Path.cwd() + ) + ) +``` + +Pass `execution_binding=execution_binding` into the `cls(...)` constructor. + +- [ ] **Step 3: Update `create_session` to store the binding** + +In `create_session`, after `repo_path` is resolved, set: + +```python + binding = LocalExecutionBinding( + workspace_root=str(repo_path.resolve()) if repo_path else str(Path.cwd()) + ) +``` + +Pass `execution_binding=binding` into the `Session(...)` constructor. + +- [ ] **Step 4: Update `run_agent` to resolve workspace from binding** + +In `run_agent`, replace the direct `session.repo_path` usage with resolver output: + +```python + from coding_agent.ui.binding_resolver import DefaultBindingResolver + resolver = DefaultBindingResolver() + workspace_root = resolver.resolve_workspace_root( + session.execution_binding + ) + pipeline, ctx = self._create_agent_for_session( + workspace_root=workspace_root, + ... + ) +``` + +Do the same in `ensure_session_runtime` and `_restore_checkpoint`. + +- [ ] **Step 5: Write the failing tests** + +Add to `tests/ui/test_session_manager_public_api.py`: + +```python +from coding_agent.ui.execution_binding import ExecutionBinding, LocalExecutionBinding + + +def test_session_metadata_round_trips_local_execution_binding() -> None: + store = InMemorySessionStore() + manager = SessionManager(store=store) + session = Session( + id="binding-session", + created_at=datetime.now(), + last_activity=datetime.now(), + approval_store=ApprovalStore(), + execution_binding=LocalExecutionBinding(workspace_root="/tmp/bound-repo"), + ) + manager.register_session(session) + + reloaded = SessionManager(store=store).get_session("binding-session") + assert isinstance(reloaded.execution_binding, LocalExecutionBinding) + assert reloaded.execution_binding.workspace_root == "/tmp/bound-repo" + + +def test_session_metadata_defaults_missing_binding_to_local_from_repo_path() -> None: + store = InMemorySessionStore() + manager = SessionManager(store=store) + session = Session( + id="legacy-session", + created_at=datetime.now(), + last_activity=datetime.now(), + approval_store=ApprovalStore(), + repo_path=Path("/tmp/legacy-repo"), + ) + # Do not set execution_binding; simulate pre-binding metadata + store.save(session.id, { + "id": session.id, + "created_at": session.created_at.isoformat(), + "last_activity": session.last_activity.isoformat(), + "repo_path": "/tmp/legacy-repo", + "approval_policy": "auto", + "provider_name": None, + "model_name": None, + "base_url": None, + "max_steps": 30, + "tape_id": None, + }) + + reloaded = SessionManager(store=store).get_session("legacy-session") + assert isinstance(reloaded.execution_binding, LocalExecutionBinding) + assert reloaded.execution_binding.workspace_root == "/tmp/legacy-repo" +``` + +- [ ] **Step 6: Run tests** + +Run: `uv run pytest tests/ui/test_session_manager_public_api.py -k "binding" -v` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add src/coding_agent/ui/session_manager.py tests/ui/test_session_manager_public_api.py +git commit -m "feat: integrate ExecutionBinding into Session metadata" +``` + +--- + +## Task 4: Add Thin Phase 2 Substrate (Session Owners, Lease, Fencing) + +**Files:** +- Create: `src/coding_agent/ui/session_owner_store.py` +- Modify: `src/agentkit/storage/pg.py` +- Create: `tests/ui/test_session_owner_store.py` +- Modify: `tests/agentkit/storage/test_pg.py` + +- [ ] **Step 1: Add `session_owners` table to `agentkit.storage.pg`** + +In `src/agentkit/storage/pg.py`, add a new class `PGSessionOwnerStore` after `PGCheckpointStore`: + +```python +class PGSessionOwnerStore: + _CREATE_TABLE_SQL: Final[str] = """ + CREATE TABLE IF NOT EXISTS session_owners ( + session_id TEXT PRIMARY KEY, + owner_id TEXT NOT NULL, + lease_expires_at TIMESTAMPTZ NOT NULL, + fencing_token BIGINT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() + ) + """ + + _ACQUIRE_SQL: Final[str] = """ + INSERT INTO session_owners (session_id, owner_id, lease_expires_at, fencing_token) + VALUES ($1, $2, NOW() + $3::interval, $4) + ON CONFLICT (session_id) DO NOTHING + """ + + _RENEW_SQL: Final[str] = """ + UPDATE session_owners + SET lease_expires_at = NOW() + $1::interval, + fencing_token = $2, + updated_at = NOW() + WHERE session_id = $3 + AND owner_id = $4 + AND lease_expires_at > NOW() + AND fencing_token = $5 + """ + + _RELEASE_SQL: Final[str] = """ + DELETE FROM session_owners + WHERE session_id = $1 + AND owner_id = $2 + AND fencing_token = $3 + """ + + _GET_SQL: Final[str] = ( + "SELECT owner_id, lease_expires_at, fencing_token FROM session_owners " + "WHERE session_id = $1" + ) + + def __init__(self, *, pool: PGPool) -> None: + self._pool: PGPool = pool + self._schema_ready: bool = False + + async def _ensure_schema(self) -> AsyncPGPool: + pool = await self._pool.get_pool() + if not self._schema_ready: + _ = await pool.execute(self._CREATE_TABLE_SQL) + self._schema_ready = True + return pool + + async def acquire( + self, + session_id: str, + owner_id: str, + lease_seconds: float, + fencing_token: int, + ) -> bool: + pool = await self._ensure_schema() + result = await pool.execute( + self._ACQUIRE_SQL, + session_id, + owner_id, + f"{lease_seconds} seconds", + fencing_token, + ) + return "INSERT 0 1" in result + + async def renew( + self, + session_id: str, + owner_id: str, + lease_seconds: float, + new_fencing_token: int, + current_fencing_token: int, + ) -> bool: + pool = await self._ensure_schema() + result = await pool.execute( + self._RENEW_SQL, + f"{lease_seconds} seconds", + new_fencing_token, + session_id, + owner_id, + current_fencing_token, + ) + return "UPDATE 1" in result + + async def release( + self, + session_id: str, + owner_id: str, + fencing_token: int, + ) -> bool: + pool = await self._ensure_schema() + result = await pool.execute( + self._RELEASE_SQL, + session_id, + owner_id, + fencing_token, + ) + return "DELETE 1" in result + + async def get_owner(self, session_id: str) -> dict[str, object] | None: + pool = await self._ensure_schema() + row = await pool.fetchrow(self._GET_SQL, session_id) + if row is None: + return None + return { + "owner_id": row.get("owner_id"), + "lease_expires_at": row.get("lease_expires_at"), + "fencing_token": row.get("fencing_token"), + } +``` + +- [ ] **Step 2: Add app-layer `SessionOwnerStore`** + +Create `src/coding_agent/ui/session_owner_store.py`: + +```python +from __future__ import annotations + +from typing import Any + +from agentkit.storage.pg import PGPool, PGSessionOwnerStore + + +class SessionOwnerStore: + def __init__(self, *, pg_pool: PGPool) -> None: + self._pg = PGSessionOwnerStore(pool=pg_pool) + + async def acquire( + self, + session_id: str, + owner_id: str, + lease_seconds: float = 30.0, + fencing_token: int = 1, + ) -> bool: + return await self._pg.acquire(session_id, owner_id, lease_seconds, fencing_token) + + async def renew( + self, + session_id: str, + owner_id: str, + lease_seconds: float = 30.0, + new_fencing_token: int = 2, + current_fencing_token: int = 1, + ) -> bool: + return await self._pg.renew( + session_id, owner_id, lease_seconds, new_fencing_token, current_fencing_token + ) + + async def release( + self, + session_id: str, + owner_id: str, + fencing_token: int, + ) -> bool: + return await self._pg.release(session_id, owner_id, fencing_token) + + async def get_owner(self, session_id: str) -> dict[str, Any] | None: + return await self._pg.get_owner(session_id) +``` + +- [ ] **Step 3: Write tests for owner store** + +Create `tests/ui/test_session_owner_store.py`: + +```python +from __future__ import annotations + +import pytest + +from coding_agent.ui.session_owner_store import SessionOwnerStore + + +class FakePGOwnerStore: + def __init__(self) -> None: + self._owners: dict[str, dict[str, object]] = {} + + async def acquire( + self, session_id: str, owner_id: str, lease_seconds: float, fencing_token: int + ) -> bool: + if session_id in self._owners: + return False + self._owners[session_id] = { + "owner_id": owner_id, + "lease_expires_at": "fake", + "fencing_token": fencing_token, + } + return True + + async def renew( + self, + session_id: str, + owner_id: str, + lease_seconds: float, + new_fencing_token: int, + current_fencing_token: int, + ) -> bool: + owner = self._owners.get(session_id) + if owner is None: + return False + if owner["owner_id"] != owner_id or owner["fencing_token"] != current_fencing_token: + return False + owner["fencing_token"] = new_fencing_token + return True + + async def release( + self, session_id: str, owner_id: str, fencing_token: int + ) -> bool: + owner = self._owners.get(session_id) + if owner is None: + return False + if owner["owner_id"] != owner_id or owner["fencing_token"] != fencing_token: + return False + del self._owners[session_id] + return True + + async def get_owner(self, session_id: str) -> dict[str, object] | None: + return self._owners.get(session_id) + + +@pytest.fixture +def owner_store() -> SessionOwnerStore: + store = SessionOwnerStore.__new__(SessionOwnerStore) + store._pg = FakePGOwnerStore() + return store + + +@pytest.mark.asyncio +async def test_session_owner_acquire_conflicts_with_live_lease( + owner_store: SessionOwnerStore, +) -> None: + assert await owner_store.acquire("s1", "owner-a", lease_seconds=30.0, fencing_token=1) + assert not await owner_store.acquire("s1", "owner-b", lease_seconds=30.0, fencing_token=2) + + +@pytest.mark.asyncio +async def test_session_owner_renew_rejects_stale_token( + owner_store: SessionOwnerStore, +) -> None: + await owner_store.acquire("s1", "owner-a", lease_seconds=30.0, fencing_token=1) + assert not await owner_store.renew( + "s1", "owner-a", lease_seconds=30.0, new_fencing_token=3, current_fencing_token=99 + ) + + +@pytest.mark.asyncio +async def test_session_owner_release_rejects_wrong_owner( + owner_store: SessionOwnerStore, +) -> None: + await owner_store.acquire("s1", "owner-a", lease_seconds=30.0, fencing_token=1) + assert not await owner_store.release("s1", "owner-b", fencing_token=1) +``` + +- [ ] **Step 4: Add PG owner store tests** + +Append to `tests/agentkit/storage/test_pg.py`: + +```python +class TestPGSessionOwnerStore: + @pytest.fixture + def fake_pool(self) -> FakePool: + return FakePool() + + @pytest.fixture + def pool(self, fake_pool: FakePool) -> PGPool: + async def fake_pool_factory(**_: object) -> FakePool: + return fake_pool + + return PGPool(dsn="postgresql://example", pool_factory=fake_pool_factory) + + @pytest.fixture + def store(self, pool: PGPool) -> PGSessionOwnerStore: + return PGSessionOwnerStore(pool=pool) + + @pytest.mark.asyncio + async def test_acquire_returns_true_on_new_session(self, store: PGSessionOwnerStore): + result = await store.acquire("s1", "owner-a", 30.0, 1) + assert result is True + + @pytest.mark.asyncio + async def test_acquire_returns_false_on_existing_session(self, store: PGSessionOwnerStore): + await store.acquire("s1", "owner-a", 30.0, 1) + result = await store.acquire("s1", "owner-b", 30.0, 2) + assert result is False + + @pytest.mark.asyncio + async def test_renew_updates_fencing_token(self, store: PGSessionOwnerStore): + await store.acquire("s1", "owner-a", 30.0, 1) + result = await store.renew("s1", "owner-a", 30.0, 2, 1) + assert result is True + owner = await store.get_owner("s1") + assert owner is not None + assert owner["fencing_token"] == 2 + + @pytest.mark.asyncio + async def test_release_removes_owner(self, store: PGSessionOwnerStore): + await store.acquire("s1", "owner-a", 30.0, 1) + result = await store.release("s1", "owner-a", 1) + assert result is True + assert await store.get_owner("s1") is None +``` + +Update `FakePool.execute` in `tests/agentkit/storage/test_pg.py` to handle the new SQL: + +```python + if "INSERT INTO session_owners" in query: + session_id, owner_id, lease_expires_at, fencing_token = args + if not isinstance(session_id, str): + raise TypeError("session_id must be a string") + if session_id in self.sessions: + return "INSERT 0 0" + self.sessions[session_id] = { + "owner_id": owner_id, + "lease_expires_at": lease_expires_at, + "fencing_token": fencing_token, + } + return "INSERT 0 1" + if "UPDATE session_owners" in query: + lease_expires_at, new_fencing_token, session_id, owner_id, current_fencing_token = args + owner = self.sessions.get(session_id) + if owner is None: + return "UPDATE 0" + if owner["owner_id"] != owner_id or owner["fencing_token"] != current_fencing_token: + return "UPDATE 0" + owner["fencing_token"] = new_fencing_token + owner["lease_expires_at"] = lease_expires_at + return "UPDATE 1" + if "DELETE FROM session_owners" in query: + session_id, owner_id, fencing_token = args + owner = self.sessions.get(session_id) + if owner is None: + return "DELETE 0" + if owner["owner_id"] != owner_id or owner["fencing_token"] != fencing_token: + return "DELETE 0" + del self.sessions[session_id] + return "DELETE 1" + if "SELECT owner_id, lease_expires_at, fencing_token FROM session_owners" in query: + (session_id,) = args + owner = self.sessions.get(session_id) + if owner is None: + return None + return { + "owner_id": owner["owner_id"], + "lease_expires_at": owner["lease_expires_at"], + "fencing_token": owner["fencing_token"], + } + if "CREATE TABLE IF NOT EXISTS session_owners" in query: + return "CREATE TABLE" +``` + +Also update `FakePool.fetchrow` to handle the owner SELECT: + +```python + if "SELECT owner_id, lease_expires_at, fencing_token FROM session_owners" in query: + (session_id,) = args + owner = self.sessions.get(session_id) + if owner is None: + return None + return { + "owner_id": owner["owner_id"], + "lease_expires_at": owner["lease_expires_at"], + "fencing_token": owner["fencing_token"], + } +``` + +- [ ] **Step 5: Run tests** + +Run: `uv run pytest tests/ui/test_session_owner_store.py tests/agentkit/storage/test_pg.py -v` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/agentkit/storage/pg.py src/coding_agent/ui/session_owner_store.py tests/ui/test_session_owner_store.py tests/agentkit/storage/test_pg.py +git commit -m "feat: add Phase 2 session owner store with lease and fencing" +``` + +--- + +## Task 5: Add Owner Checks to `SessionManager` + +**Files:** +- Modify: `src/coding_agent/ui/session_manager.py` +- Create: `tests/ui/test_session_manager_owner_checks.py` + +- [ ] **Step 1: Add owner checking to `SessionManager`** + +Inject an optional `owner_store` into `SessionManager.__init__`: + +```python + def __init__( + self, + store: SessionStore | None = None, + *, + storage_config: dict[str, Any] | None = None, + pg_pool: object | None = None, + tape_store: TapeStore | None = None, + checkpoint_store: CheckpointStore | None = None, + checkpoint_service: CheckpointService | None = None, + create_agent_fn: Callable[..., tuple[Any, Any]] | None = None, + owner_store: SessionOwnerStore | None = None, + ): +``` + +Store `self._owner_store = owner_store`. + +Add an internal helper: + +```python + async def _assert_owner(self, session_id: str, owner_id: str, fencing_token: int) -> None: + if self._owner_store is None: + return + owner = await self._owner_store.get_owner(session_id) + if owner is None: + raise RuntimeError("session has no owner") + if owner["owner_id"] != owner_id or owner["fencing_token"] != fencing_token: + raise RuntimeError("stale owner or fencing token rejected") +``` + +Add owner checks at the start of `run_agent`, `_restore_checkpoint`, and `close_session`: + +```python + async def run_agent(self, session_id: str, prompt: str) -> None: + await self._assert_owner(session_id, self._owner_id, self._fencing_token) + ... +``` + +Note: `self._owner_id` and `self._fencing_token` should be instance-level config passed at construction or defaulted to `None` when `owner_store` is `None`. + +- [ ] **Step 2: Write owner check tests** + +Create `tests/ui/test_session_manager_owner_checks.py`: + +```python +from __future__ import annotations + +import pytest + +from coding_agent.ui.session_manager import SessionManager +from coding_agent.ui.session_owner_store import SessionOwnerStore +from coding_agent.ui.session_store import InMemorySessionStore + + +class FakeOwnerStore(SessionOwnerStore): + def __init__(self) -> None: + self._owners: dict[str, dict[str, object]] = {} + + async def acquire( + self, session_id: str, owner_id: str, lease_seconds: float, fencing_token: int + ) -> bool: + if session_id in self._owners: + return False + self._owners[session_id] = { + "owner_id": owner_id, + "lease_expires_at": "fake", + "fencing_token": fencing_token, + } + return True + + async def get_owner(self, session_id: str) -> dict[str, object] | None: + return self._owners.get(session_id) + + +@pytest.mark.asyncio +async def test_run_agent_rejects_non_owner_instance() -> None: + owner_store = FakeOwnerStore() + manager = SessionManager( + store=InMemorySessionStore(), + owner_store=owner_store, + ) + session_id = await manager.create_session() + await owner_store.acquire(session_id, "owner-a", 30.0, 1) + + # Manager has no owner_id set; run_agent should reject + with pytest.raises(RuntimeError, match="stale owner or fencing token rejected"): + await manager.run_agent(session_id, "hello") + + +@pytest.mark.asyncio +async def test_restore_checkpoint_rejects_stale_owner() -> None: + owner_store = FakeOwnerStore() + manager = SessionManager( + store=InMemorySessionStore(), + owner_store=owner_store, + ) + session_id = await manager.create_session() + await owner_store.acquire(session_id, "owner-a", 30.0, 1) + + with pytest.raises(RuntimeError, match="stale owner or fencing token rejected"): + await manager.restore_checkpoint(session_id, "cp-1") + + +@pytest.mark.asyncio +async def test_close_session_rejects_stale_owner() -> None: + owner_store = FakeOwnerStore() + manager = SessionManager( + store=InMemorySessionStore(), + owner_store=owner_store, + ) + session_id = await manager.create_session() + await owner_store.acquire(session_id, "owner-a", 30.0, 1) + + with pytest.raises(RuntimeError, match="stale owner or fencing token rejected"): + await manager.close_session(session_id) +``` + +- [ ] **Step 3: Run tests** + +Run: `uv run pytest tests/ui/test_session_manager_owner_checks.py -v` +Expected: PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/coding_agent/ui/session_manager.py tests/ui/test_session_manager_owner_checks.py +git commit -m "feat: add owner checks to SessionManager runtime operations" +``` + +--- + +## Task 6: Add Local-Execution Binding Integration + +**Files:** +- Modify: `src/coding_agent/ui/http_server.py` +- Modify: `tests/ui/test_http_server.py` + +- [ ] **Step 1: Update `create_session` endpoint to store binding** + +In `src/coding_agent/ui/http_server.py`, import `LocalExecutionBinding` and modify `create_session`: + +```python +from coding_agent.ui.execution_binding import LocalExecutionBinding +``` + +After session creation, update the session metadata with the binding: + +```python + session = session_manager.get_session(session_id) + session.execution_binding = LocalExecutionBinding( + workspace_root=str(repo_path.resolve()) if repo_path else str(Path.cwd()) + ) + session_manager.register_session(session) +``` + +- [ ] **Step 2: Add HTTP binding tests** + +Add to `tests/ui/test_http_server.py`: + +```python +from coding_agent.ui.execution_binding import LocalExecutionBinding + + +class TestExecutionBinding: + async def test_create_session_stores_local_binding_by_default(self, client): + response = await client.post("/sessions", json={}) + session_id = response.json()["session_id"] + + session = session_manager.get_session(session_id) + assert isinstance(session.execution_binding, LocalExecutionBinding) + assert session.execution_binding.workspace_root == str(Path.cwd()) + + async def test_create_session_stores_local_binding_with_repo_path(self, client, tmp_path): + response = await client.post("/sessions", json={"repo_path": str(tmp_path)}) + session_id = response.json()["session_id"] + + session = session_manager.get_session(session_id) + assert isinstance(session.execution_binding, LocalExecutionBinding) + assert session.execution_binding.workspace_root == str(tmp_path.resolve()) +``` + +- [ ] **Step 3: Run tests** + +Run: `uv run pytest tests/ui/test_http_server.py -k "binding" -v` +Expected: PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/coding_agent/ui/http_server.py tests/ui/test_http_server.py +git commit -m "feat: store local execution binding on HTTP session creation" +``` + +--- + +## Task 7: Add Cloud-Workspace Binding (Skeleton) + +**Files:** +- Modify: `src/coding_agent/ui/binding_resolver.py` +- Modify: `tests/ui/test_execution_binding.py` + +- [ ] **Step 1: Extend resolver to reject cloud binding gracefully** + +The resolver already raises `NotImplementedError` for cloud bindings. Add a typed exception for callers to catch: + +```python +class CloudBindingNotImplementedError(NotImplementedError): + pass +``` + +Update `DefaultBindingResolver` to raise `CloudBindingNotImplementedError` instead of plain `NotImplementedError`. + +- [ ] **Step 2: Add cloud binding tests** + +Add to `tests/ui/test_execution_binding.py`: + +```python +from coding_agent.ui.binding_resolver import ( + CloudBindingNotImplementedError, + DefaultBindingResolver, +) + + +def test_cloud_binding_raises_typed_not_implemented() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + resolver = DefaultBindingResolver() + with pytest.raises(CloudBindingNotImplementedError): + resolver.resolve_workspace_root(binding) +``` + +- [ ] **Step 3: Run tests** + +Run: `uv run pytest tests/ui/test_execution_binding.py -v` +Expected: PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/coding_agent/ui/binding_resolver.py tests/ui/test_execution_binding.py +git commit -m "feat: add CloudBindingNotImplementedError for future cloud workspace support" +``` + +--- + +## Task 8: Add Sticky Routing Hardening and Failover Tests + +**Files:** +- Create: `tests/ui/test_http_server_failover.py` + +- [ ] **Step 1: Write failover tests** + +Create `tests/ui/test_http_server_failover.py`: + +```python +from __future__ import annotations + +import pytest + +from coding_agent.ui.http_server import session_manager +from coding_agent.ui.session_owner_store import SessionOwnerStore +from coding_agent.ui.session_store import InMemorySessionStore + + +class FakeOwnerStore(SessionOwnerStore): + def __init__(self) -> None: + self._owners: dict[str, dict[str, object]] = {} + + async def acquire( + self, session_id: str, owner_id: str, lease_seconds: float, fencing_token: int + ) -> bool: + if session_id in self._owners: + return False + self._owners[session_id] = { + "owner_id": owner_id, + "lease_expires_at": "fake", + "fencing_token": fencing_token, + } + return True + + async def get_owner(self, session_id: str) -> dict[str, object] | None: + return self._owners.get(session_id) + + +@pytest.fixture +def owner_store() -> FakeOwnerStore: + return FakeOwnerStore() + + +@pytest.mark.asyncio +async def test_approval_response_is_rejected_after_owner_change(owner_store: FakeOwnerStore) -> None: + manager = session_manager.__class__( + store=InMemorySessionStore(), + owner_store=owner_store, + ) + session_id = await manager.create_session() + await owner_store.acquire(session_id, "owner-a", 30.0, 1) + + with pytest.raises(RuntimeError, match="stale owner or fencing token rejected"): + await manager.submit_approval(session_id, "req-1", approved=True) + + +@pytest.mark.asyncio +async def test_session_failover_rebuilds_from_persisted_state_without_resuming_inflight_turn( + owner_store: FakeOwnerStore, +) -> None: + store = InMemorySessionStore() + first_manager = session_manager.__class__(store=store) + session_id = await first_manager.create_session() + await first_manager.ensure_session_runtime(session_id) + + # Simulate owner loss: new manager takes over + await owner_store.acquire(session_id, "owner-b", 30.0, 2) + second_manager = session_manager.__class__( + store=store, + owner_store=owner_store, + ) + + # The new manager can load metadata but does not resume runtime + session = second_manager.get_session(session_id) + assert session.runtime_pipeline is None + assert session.runtime_ctx is None + assert session.runtime_adapter is None +``` + +- [ ] **Step 2: Run tests** + +Run: `uv run pytest tests/ui/test_http_server_failover.py -v` +Expected: PASS + +- [ ] **Step 3: Commit** + +```bash +git add tests/ui/test_http_server_failover.py +git commit -m "test: add sticky routing and failover boundary tests" +``` + +--- + +## Task 9: Final Integration Verification + +**Files:** +- All modified files + +- [ ] **Step 1: Run the full test suite for affected modules** + +Run: +```bash +uv run pytest tests/ui/test_execution_binding.py tests/ui/test_session_owner_store.py tests/ui/test_session_manager_public_api.py tests/ui/test_session_manager_runtime.py tests/ui/test_session_manager_owner_checks.py tests/ui/test_http_server.py tests/ui/test_http_server_failover.py tests/agentkit/storage/test_pg.py -v +``` + +Expected: All PASS + +- [ ] **Step 2: Run the ADR-0014 acceptance criteria command** + +Run: +```bash +uv run pytest tests/ui/test_session_manager_public_api.py tests/ui/test_session_manager_runtime.py tests/ui/test_http_server.py -k "binding" -v +``` + +Expected: All PASS + +- [ ] **Step 3: Run the ADR-0013 acceptance criteria command** + +Run: +```bash +uv run pytest tests/agentkit/ tests/ui/ -k "owner or lease or fencing or failover" -v +``` + +Expected: All PASS + +- [ ] **Step 4: Commit** + +```bash +git add -A +git commit -m "test: verify full integration of execution binding and Phase 2 substrate" +``` + +--- + +## Task Packet + +When starting implementation, pass the following context to the agent: + +**Goal:** Implement the execution binding abstraction and thin Phase 2 owner/lease/fencing substrate as described in ADR-0014 and this plan. + +**Key files:** +- `src/coding_agent/ui/execution_binding.py` (new) +- `src/coding_agent/ui/binding_resolver.py` (new) +- `src/coding_agent/ui/session_owner_store.py` (new) +- `src/agentkit/storage/pg.py` (append `PGSessionOwnerStore`) +- `src/coding_agent/ui/session_manager.py` (integrate binding + owner checks) +- `src/coding_agent/ui/http_server.py` (store binding on create) + +**Tests to add:** +- `tests/ui/test_execution_binding.py` +- `tests/ui/test_session_owner_store.py` +- `tests/ui/test_session_manager_owner_checks.py` +- `tests/ui/test_http_server_failover.py` +- Additions to `tests/ui/test_session_manager_public_api.py` +- Additions to `tests/ui/test_http_server.py` +- Additions to `tests/agentkit/storage/test_pg.py` + +**Verification commands:** +```bash +uv run pytest tests/ui/test_session_manager_public_api.py tests/ui/test_session_manager_runtime.py tests/ui/test_http_server.py -k "binding" -v +uv run pytest tests/agentkit/ tests/ui/ -k "owner or lease or fencing or failover" -v +``` + +**What to avoid:** +- Do not implement cloud workspace tools. +- Do not implement in-flight turn resume. +- Do not implement brokered event routing. +- Do not assume runtime objects are serializable across owners. From 0b499df7d4e0be5635af3d0dd2880afbf21f6885 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sun, 19 Apr 2026 23:32:37 +0800 Subject: [PATCH 3/7] docs(prompt): add execution binding task packet Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...on-execution-binding-phase2-task-packet.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 coding-agent/.opencode/prompts/session-execution-binding-phase2-task-packet.md diff --git a/coding-agent/.opencode/prompts/session-execution-binding-phase2-task-packet.md b/coding-agent/.opencode/prompts/session-execution-binding-phase2-task-packet.md new file mode 100644 index 000000000..f2f3618f6 --- /dev/null +++ b/coding-agent/.opencode/prompts/session-execution-binding-phase2-task-packet.md @@ -0,0 +1,56 @@ +Goal: +Add a first-class execution binding abstraction that separates WHERE a session runs from WHO owns it, then build the thin Phase 2 ownership substrate (session owners, leases, fencing) on top of Phase 1 PostgreSQL persistence. + +Scope: +- Introduce `ExecutionBinding` dataclass hierarchy in `src/coding_agent/ui/execution_binding.py` with `LocalExecutionBinding` and `CloudWorkspaceBinding` variants, plus typed serialization helpers. +- Add `BindingResolver` protocol and `DefaultBindingResolver` in `src/coding_agent/ui/binding_resolver.py` that converts bindings into `workspace_root` and tool config for `create_agent_for_session`. +- Integrate binding into `Session` metadata: store `execution_binding` on the session, round-trip it through `to_store_data` / `from_store_data`, default missing bindings to `LocalExecutionBinding(repo_path)` for backward compatibility, and resolve the binding in `run_agent`, `ensure_session_runtime`, and `_restore_checkpoint`. +- Add thin Phase 2 ownership substrate: + - `PGSessionOwnerStore` in `src/agentkit/storage/pg.py` with `session_owners` table schema and atomic acquire/renew/release SQL. + - App-layer `SessionOwnerStore` in `src/coding_agent/ui/session_owner_store.py` wrapping the PG store. + - Owner checks in `SessionManager` on `run_agent`, `_restore_checkpoint`, and `close_session` using `_assert_owner`. + +Out of scope: +- HTTP server local-execution binding integration (endpoint changes and HTTP-level binding tests). +- Cloud workspace tool implementations or resolver behavior beyond `NotImplementedError`. +- Sticky routing hardening, brokered event routing, or failover boundary tests. +- In-flight turn resume after owner loss. + +Context: +- ADRs: + - `docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md` + - `docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md` + - `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` +- Relevant files: + - `src/coding_agent/ui/execution_binding.py` (new) + - `src/coding_agent/ui/binding_resolver.py` (new) + - `src/coding_agent/ui/session_owner_store.py` (new) + - `src/agentkit/storage/pg.py` (append `PGSessionOwnerStore`) + - `src/coding_agent/ui/session_manager.py` (integrate binding + owner checks) + - `tests/ui/test_execution_binding.py` (new) + - `tests/ui/test_session_owner_store.py` (new) + - `tests/ui/test_session_manager_owner_checks.py` (new) + - `tests/ui/test_session_manager_public_api.py` (binding round-trip tests) + - `tests/agentkit/storage/test_pg.py` (PG owner store tests) +- Implementation plan: + - `docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md` + +Target tests: +- `uv run pytest tests/ui/test_execution_binding.py -v` +- `uv run pytest tests/ui/test_session_owner_store.py -v` +- `uv run pytest tests/ui/test_session_manager_owner_checks.py -v` +- `uv run pytest tests/ui/test_session_manager_public_api.py -k "binding" -v` +- `uv run pytest tests/agentkit/storage/test_pg.py -k "owner" -v` +- `uv run pytest tests/ui/test_execution_binding.py tests/ui/test_session_owner_store.py tests/ui/test_session_manager_owner_checks.py tests/ui/test_session_manager_public_api.py tests/agentkit/storage/test_pg.py -v` + +Loop policy: +- Engineer implements the smallest correct change and runs the target tests. +- Reviewer reviews only the resulting diff and affected tests. +- Reviewer reports only P1/P2 findings. +- Engineer fixes only accepted P1/P2 findings and reruns the same target tests. +- Verifier reruns the exact target tests and reports pass/fail only. + +Stop conditions: +- At most one review/fix/retest cycle. +- Escalate architectural redirection or scope expansion to the human. +- Ignore non-blocking optimization suggestions. From aea8ecbd2bc222380c7e3d5250b87f893f5abdf0 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sun, 19 Apr 2026 23:46:37 +0800 Subject: [PATCH 4/7] feat(ui): add execution binding foundation Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../src/coding_agent/ui/binding_resolver.py | 36 +++++++++++ .../src/coding_agent/ui/execution_binding.py | 61 ++++++++++++++++++ .../tests/ui/test_execution_binding.py | 63 +++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 coding-agent/src/coding_agent/ui/binding_resolver.py create mode 100644 coding-agent/src/coding_agent/ui/execution_binding.py create mode 100644 coding-agent/tests/ui/test_execution_binding.py diff --git a/coding-agent/src/coding_agent/ui/binding_resolver.py b/coding-agent/src/coding_agent/ui/binding_resolver.py new file mode 100644 index 000000000..c0ca2ebfb --- /dev/null +++ b/coding-agent/src/coding_agent/ui/binding_resolver.py @@ -0,0 +1,36 @@ +from __future__ import annotations + +from pathlib import Path +from typing import Any, Protocol + +from coding_agent.ui.execution_binding import ( + CloudWorkspaceBinding, + ExecutionBinding, + LocalExecutionBinding, +) + + +class BindingResolver(Protocol): + def resolve_workspace_root(self, binding: ExecutionBinding) -> Path: ... + + def resolve_tool_config(self, binding: ExecutionBinding) -> dict[str, Any]: ... + + +class DefaultBindingResolver: + def resolve_workspace_root(self, binding: ExecutionBinding) -> Path: + if isinstance(binding, LocalExecutionBinding): + return Path(binding.workspace_root).resolve() + if isinstance(binding, CloudWorkspaceBinding): + raise NotImplementedError( + "cloud workspace resolution is not yet implemented" + ) + raise ValueError(f"unsupported binding type: {type(binding).__name__}") + + def resolve_tool_config(self, binding: ExecutionBinding) -> dict[str, Any]: + if isinstance(binding, LocalExecutionBinding): + return {"workspace_root": str(self.resolve_workspace_root(binding))} + if isinstance(binding, CloudWorkspaceBinding): + raise NotImplementedError( + "cloud workspace tool config is not yet implemented" + ) + raise ValueError(f"unsupported binding type: {type(binding).__name__}") diff --git a/coding-agent/src/coding_agent/ui/execution_binding.py b/coding-agent/src/coding_agent/ui/execution_binding.py new file mode 100644 index 000000000..d0976ea4b --- /dev/null +++ b/coding-agent/src/coding_agent/ui/execution_binding.py @@ -0,0 +1,61 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, ClassVar, Literal + + +@dataclass(frozen=True) +class ExecutionBinding: + kind: ClassVar[Literal["local", "cloud"]] + + def to_dict(self) -> dict[str, Any]: + raise NotImplementedError + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> ExecutionBinding: + kind = data.get("kind") + if kind == "local": + return LocalExecutionBinding.from_dict(data) + if kind == "cloud": + return CloudWorkspaceBinding.from_dict(data) + raise ValueError(f"unknown binding kind: {kind}") + + +@dataclass(frozen=True) +class LocalExecutionBinding(ExecutionBinding): + workspace_root: str + kind: ClassVar[Literal["local"]] = "local" + + def to_dict(self) -> dict[str, Any]: + return {"kind": self.kind, "workspace_root": self.workspace_root} + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> LocalExecutionBinding: + root = data.get("workspace_root") + if not isinstance(root, str): + raise TypeError("local binding requires string workspace_root") + return cls(workspace_root=root) + + +@dataclass(frozen=True) +class CloudWorkspaceBinding(ExecutionBinding): + workspace_url: str + workspace_id: str + kind: ClassVar[Literal["cloud"]] = "cloud" + + def to_dict(self) -> dict[str, Any]: + return { + "kind": self.kind, + "workspace_url": self.workspace_url, + "workspace_id": self.workspace_id, + } + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> CloudWorkspaceBinding: + workspace_url = data.get("workspace_url") + workspace_id = data.get("workspace_id") + if not isinstance(workspace_url, str) or not isinstance(workspace_id, str): + raise TypeError( + "cloud binding requires string workspace_url and workspace_id" + ) + return cls(workspace_url=workspace_url, workspace_id=workspace_id) diff --git a/coding-agent/tests/ui/test_execution_binding.py b/coding-agent/tests/ui/test_execution_binding.py new file mode 100644 index 000000000..259c59fb6 --- /dev/null +++ b/coding-agent/tests/ui/test_execution_binding.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import pytest + +from coding_agent.ui.binding_resolver import DefaultBindingResolver +from coding_agent.ui.execution_binding import ( + CloudWorkspaceBinding, + ExecutionBinding, + LocalExecutionBinding, +) + + +def test_local_binding_round_trip() -> None: + binding = LocalExecutionBinding(workspace_root="/tmp/repo") + + restored = ExecutionBinding.from_dict(binding.to_dict()) + + assert isinstance(restored, LocalExecutionBinding) + assert restored.workspace_root == "/tmp/repo" + + +def test_cloud_binding_round_trip() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + + restored = ExecutionBinding.from_dict(binding.to_dict()) + + assert isinstance(restored, CloudWorkspaceBinding) + assert restored.workspace_url == "https://workspace.example.com" + assert restored.workspace_id == "ws-123" + + +def test_unknown_binding_kind_raises() -> None: + with pytest.raises(ValueError, match="unknown binding kind"): + ExecutionBinding.from_dict({"kind": "unknown"}) + + +def test_local_binding_requires_string_workspace_root() -> None: + with pytest.raises(TypeError, match="string workspace_root"): + LocalExecutionBinding.from_dict({"kind": "local", "workspace_root": 123}) + + +def test_local_resolver_returns_absolute_path() -> None: + binding = LocalExecutionBinding(workspace_root="/tmp/repo") + resolver = DefaultBindingResolver() + + assert str(resolver.resolve_workspace_root(binding)).endswith("/tmp/repo") + assert resolver.resolve_tool_config(binding) == { + "workspace_root": str(resolver.resolve_workspace_root(binding)) + } + + +def test_cloud_resolver_raises_not_implemented() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + resolver = DefaultBindingResolver() + + with pytest.raises(NotImplementedError, match="cloud workspace"): + resolver.resolve_workspace_root(binding) From d208f3638346dbd7c5b38189c28013f30dca649f Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sun, 19 Apr 2026 23:46:37 +0800 Subject: [PATCH 5/7] feat(ui): bind session runtime to execution metadata Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../src/coding_agent/ui/session_manager.py | 42 +++++- .../ui/test_session_manager_public_api.py | 46 +++++++ .../tests/ui/test_session_manager_runtime.py | 128 +++++++++++++++++- 3 files changed, 206 insertions(+), 10 deletions(-) diff --git a/coding-agent/src/coding_agent/ui/session_manager.py b/coding-agent/src/coding_agent/ui/session_manager.py index 02d5969dd..f3f42b74b 100644 --- a/coding-agent/src/coding_agent/ui/session_manager.py +++ b/coding-agent/src/coding_agent/ui/session_manager.py @@ -44,6 +44,8 @@ SessionStore, create_session_store, ) +from coding_agent.ui.binding_resolver import DefaultBindingResolver +from coding_agent.ui.execution_binding import ExecutionBinding, LocalExecutionBinding logger = logging.getLogger(__name__) @@ -99,6 +101,11 @@ class Session: wire: LocalWire = field(init=False) approval_store: ApprovalStore = field(default_factory=ApprovalStore) repo_path: Path | None = None + execution_binding: ExecutionBinding = field( + default_factory=lambda: LocalExecutionBinding( + workspace_root=str(Path.cwd().resolve()) + ) + ) approval_policy: ApprovalPolicy = ApprovalPolicy.AUTO provider: Any | None = None provider_name: str | None = None @@ -136,6 +143,7 @@ def to_store_data(self) -> dict[str, Any]: "created_at": self.created_at.isoformat(), "last_activity": self.last_activity.isoformat(), "repo_path": None if self.repo_path is None else str(self.repo_path), + "execution_binding": self.execution_binding.to_dict(), "approval_policy": self.approval_policy.value, "provider_name": self.provider_name, "model_name": self.model_name, @@ -164,6 +172,16 @@ def from_store_data(cls, data: dict[str, Any]) -> Session: tape_id_raw = data.get("tape_id") if tape_id_raw is not None and not isinstance(tape_id_raw, str): raise TypeError("session metadata has invalid tape_id") + binding_raw = data.get("execution_binding") + if binding_raw is not None: + if not isinstance(binding_raw, dict): + raise TypeError("session metadata has invalid execution_binding") + execution_binding = ExecutionBinding.from_dict(binding_raw) + else: + workspace_root = ( + repo_path_raw if repo_path_raw is not None else str(Path.cwd()) + ) + execution_binding = LocalExecutionBinding(workspace_root=workspace_root) session = cls( id=_required_session_str(data, "id"), created_at=datetime.fromisoformat( @@ -174,6 +192,7 @@ def from_store_data(cls, data: dict[str, Any]) -> Session: ), approval_store=ApprovalStore(), repo_path=None if repo_path_raw is None else Path(repo_path_raw), + execution_binding=execution_binding, approval_policy=ApprovalPolicy(approval_policy_raw), provider_name=provider_name_raw, model_name=model_name_raw, @@ -630,7 +649,7 @@ async def _restore_checkpoint(self, session: Session, checkpoint_id: str) -> Non ApprovalPolicy.AUTO: "auto", } pipeline, ctx = self._create_agent_for_session( - workspace_root=session.repo_path, + workspace_root=self._resolve_workspace_root(session), model_override=restored_config.model_name, provider_override=restored_config.provider_name, base_url_override=restored_config.base_url, @@ -693,6 +712,11 @@ def _persist_session(self, session: Session) -> None: self._session_cache[session.id] = session self._store.save(session.id, cast(dict[str, Any], session.to_store_data())) + def _resolve_workspace_root(self, session: Session) -> Path: + return DefaultBindingResolver().resolve_workspace_root( + session.execution_binding + ) + def _invalidate_runtime(self, session: Session) -> None: session.runtime_pipeline = None session.runtime_ctx = None @@ -751,12 +775,22 @@ async def create_session( if base_url is None: base_url = cfg.base_url + resolved_repo_path = repo_path.resolve() if repo_path is not None else None + binding = LocalExecutionBinding( + workspace_root=( + str(resolved_repo_path) + if resolved_repo_path is not None + else str(Path.cwd().resolve()) + ) + ) + session = Session( id=session_id, approval_store=approval_store, created_at=now, last_activity=now, - repo_path=repo_path, + repo_path=resolved_repo_path, + execution_binding=binding, approval_policy=approval_policy, provider=provider, provider_name=provider_name, @@ -964,7 +998,7 @@ async def run_agent( if pipeline is None or ctx is None or adapter is None: pipeline, ctx = self._create_agent_for_session( - workspace_root=session.repo_path, + workspace_root=self._resolve_workspace_root(session), model_override=session.model_name, provider_override=session.provider_name, base_url_override=session.base_url, @@ -1198,7 +1232,7 @@ async def ensure_session_runtime(self, session_id: str) -> Any: } consumer = self._make_session_consumer(session) pipeline, ctx = self._create_agent_for_session( - workspace_root=session.repo_path, + workspace_root=self._resolve_workspace_root(session), model_override=session.model_name, provider_override=session.provider_name, base_url_override=session.base_url, diff --git a/coding-agent/tests/ui/test_session_manager_public_api.py b/coding-agent/tests/ui/test_session_manager_public_api.py index 935412617..fd574dd44 100644 --- a/coding-agent/tests/ui/test_session_manager_public_api.py +++ b/coding-agent/tests/ui/test_session_manager_public_api.py @@ -14,6 +14,7 @@ from coding_agent.approval.store import ApprovalStore from coding_agent.approval import ApprovalPolicy from agentkit.config.loader import ConfigError +from coding_agent.ui.execution_binding import LocalExecutionBinding from coding_agent.wire.protocol import ( ApprovalRequest, CompletionStatus, @@ -157,6 +158,51 @@ async def test_create_session_persists_configured_restart_metadata_by_default() assert payload["base_url"] == "http://llm.default" +def test_session_metadata_round_trips_local_execution_binding() -> None: + store = InMemorySessionStore() + manager = SessionManager(store=store) + session = Session( + id="binding-session", + created_at=datetime.now(), + last_activity=datetime.now(), + approval_store=ApprovalStore(), + execution_binding=LocalExecutionBinding(workspace_root="/tmp/bound-repo"), + ) + + manager.register_session(session) + + reloaded = SessionManager(store=store).get_session("binding-session") + + assert isinstance(reloaded.execution_binding, LocalExecutionBinding) + assert reloaded.execution_binding.workspace_root == "/tmp/bound-repo" + + +def test_session_metadata_defaults_missing_binding_to_local_from_repo_path() -> None: + store = InMemorySessionStore() + created_at = datetime.now() + last_activity = datetime.now() + store.save( + "legacy-session", + { + "id": "legacy-session", + "created_at": created_at.isoformat(), + "last_activity": last_activity.isoformat(), + "repo_path": "/tmp/legacy-repo", + "approval_policy": "auto", + "provider_name": None, + "model_name": None, + "base_url": None, + "max_steps": 30, + "tape_id": None, + }, + ) + + reloaded = SessionManager(store=store).get_session("legacy-session") + + assert isinstance(reloaded.execution_binding, LocalExecutionBinding) + assert reloaded.execution_binding.workspace_root == "/tmp/legacy-repo" + + def test_event_queue_mutations_do_not_persist_sync_runtime_only_state() -> None: manager = SessionManager(store=InMemorySessionStore()) session = Session( diff --git a/coding-agent/tests/ui/test_session_manager_runtime.py b/coding-agent/tests/ui/test_session_manager_runtime.py index d358e6f57..87371c084 100644 --- a/coding-agent/tests/ui/test_session_manager_runtime.py +++ b/coding-agent/tests/ui/test_session_manager_runtime.py @@ -2,6 +2,7 @@ import asyncio import types +from pathlib import Path from typing import cast from unittest.mock import patch @@ -10,6 +11,7 @@ from agentkit.tape.models import Entry from agentkit.tape.tape import Tape from coding_agent.approval import ApprovalPolicy +from coding_agent.ui.execution_binding import LocalExecutionBinding from coding_agent.wire.protocol import ( ApprovalRequest, CompletionStatus, @@ -347,8 +349,8 @@ def done(self) -> bool: def cancel(self) -> None: self.cancel_calls += 1 - task = cast(asyncio.Task[None], FakeTask()) - session.task = task + fake_task = FakeTask() + session.task = cast(asyncio.Task[None], cast(object, fake_task)) with patch( "coding_agent.ui.session_manager.asyncio.wait_for", @@ -358,7 +360,7 @@ def cancel(self) -> None: await manager.close_session(session_id) assert manager.has_session(session_id) is True - assert cast(FakeTask, task).cancel_calls == 1 + assert fake_task.cancel_calls == 1 @pytest.mark.asyncio @@ -377,8 +379,8 @@ def done(self) -> bool: def cancel(self) -> None: self.cancel_calls += 1 - task = cast(asyncio.Task[None], FakeTask()) - session.task = task + fake_task = FakeTask() + session.task = cast(asyncio.Task[None], cast(object, fake_task)) with patch( "coding_agent.ui.session_manager.asyncio.wait_for", @@ -388,7 +390,7 @@ def cancel(self) -> None: await manager.shutdown_session_runtime(session_id) assert manager.has_session(session_id) is True - assert cast(FakeTask, task).cancel_calls == 1 + assert fake_task.cancel_calls == 1 @pytest.mark.asyncio @@ -1027,6 +1029,120 @@ def fake_create_agent(**kwargs): assert session.approval_policy is ApprovalPolicy.INTERACTIVE +@pytest.mark.asyncio +async def test_run_agent_uses_resolved_workspace_root_from_binding() -> None: + manager = SessionManager(store=InMemorySessionStore()) + session_id = await manager.create_session() + session = manager.get_session(session_id) + session.repo_path = Path("/tmp/not-used-directly") + session.execution_binding = LocalExecutionBinding( + workspace_root="/tmp/bound-workspace" + ) + manager.register_session(session) + + class FakeAdapter: + def __init__(self, pipeline, ctx, consumer) -> None: + del pipeline, consumer + self.ctx = ctx + + async def run_turn(self, prompt: str) -> None: + del prompt + + fake_pipeline = types.SimpleNamespace( + _registry=types.SimpleNamespace( + get=lambda _: types.SimpleNamespace(_instance=None) + ) + ) + fake_ctx = types.SimpleNamespace(config={}, tape=Tape()) + captured_kwargs: dict[str, object] = {} + + def fake_create_agent(**kwargs): + captured_kwargs.update(kwargs) + return fake_pipeline, fake_ctx + + with pytest.MonkeyPatch.context() as mp: + mp.setattr("coding_agent.__main__.create_agent", fake_create_agent) + mp.setattr("coding_agent.ui.session_manager.PipelineAdapter", FakeAdapter) + await manager.run_agent(session_id, "hello") + + assert captured_kwargs["workspace_root"] == Path("/tmp/bound-workspace").resolve() + + +@pytest.mark.asyncio +async def test_restore_checkpoint_preserves_execution_binding() -> None: + store = InMemorySessionStore() + manager = SessionManager(store=store) + session_id = await manager.create_session() + session = manager.get_session(session_id) + session.tape_id = "binding-restore-tape" + session.execution_binding = LocalExecutionBinding( + workspace_root="/tmp/restore-bound" + ) + manager.register_session(session) + + snapshot = types.SimpleNamespace( + meta=types.SimpleNamespace( + checkpoint_id="cp-binding", + tape_id="binding-restore-tape", + entry_count=0, + window_start=0, + ), + tape_entries=(), + plugin_states={}, + extra={}, + ) + captured_kwargs: dict[str, object] = {} + + class FakeCheckpointService: + async def restore(self, checkpoint_id: str): + assert checkpoint_id == "cp-binding" + return snapshot + + async def list(self, tape_id: str): + assert tape_id == "binding-restore-tape" + return [snapshot.meta] + + async def delete(self, checkpoint_id: str) -> None: + return None + + class FakeTapeStore: + async def truncate(self, tape_id: str, keep: int) -> None: + return None + + class FakeAdapter: + def __init__(self, pipeline, ctx, consumer) -> None: + del pipeline, consumer + self.ctx = ctx + + async def initialize(self) -> None: + return None + + fake_pipeline = types.SimpleNamespace( + _registry=types.SimpleNamespace( + get=lambda _: types.SimpleNamespace(_instance=None) + ) + ) + + def fake_create_agent(**kwargs): + captured_kwargs.update(kwargs) + return fake_pipeline, types.SimpleNamespace( + config={}, tape=kwargs.get("tape"), plugin_states={} + ) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr("coding_agent.__main__.create_agent", fake_create_agent) + mp.setattr("coding_agent.ui.session_manager.PipelineAdapter", FakeAdapter) + mp.setattr( + manager, "_checkpoint_service", FakeCheckpointService(), raising=False + ) + mp.setattr(manager, "_tape_store", FakeTapeStore(), raising=False) + await manager._restore_checkpoint(session, "cp-binding") + + assert captured_kwargs["workspace_root"] == Path("/tmp/restore-bound").resolve() + assert isinstance(session.execution_binding, LocalExecutionBinding) + assert session.execution_binding.workspace_root == "/tmp/restore-bound" + + @pytest.mark.asyncio async def test_restore_legacy_checkpoint_without_session_config_uses_current_session_metadata() -> ( None From 047752432ec41e610f24d0348cf10c8aec644a27 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Mon, 20 Apr 2026 00:26:39 +0800 Subject: [PATCH 6/7] fix(ui): inject binding resolver Keep the workspace resolver stable on SessionManager and cover the missing cloud binding failures. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../src/coding_agent/ui/session_manager.py | 18 ++++++---- .../tests/ui/test_execution_binding.py | 35 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/coding-agent/src/coding_agent/ui/session_manager.py b/coding-agent/src/coding_agent/ui/session_manager.py index f3f42b74b..3bb3c3ece 100644 --- a/coding-agent/src/coding_agent/ui/session_manager.py +++ b/coding-agent/src/coding_agent/ui/session_manager.py @@ -44,7 +44,7 @@ SessionStore, create_session_store, ) -from coding_agent.ui.binding_resolver import DefaultBindingResolver +from coding_agent.ui.binding_resolver import BindingResolver, DefaultBindingResolver from coding_agent.ui.execution_binding import ExecutionBinding, LocalExecutionBinding logger = logging.getLogger(__name__) @@ -93,14 +93,18 @@ async def complete(self, messages: list[dict[str, Any]]) -> str: @dataclass class Session: - """A managed agent session.""" + """A managed agent session. + + Note: ``repo_path`` is legacy/backward-compat metadata only; + ``execution_binding`` is the authoritative workspace contract. + """ id: str created_at: datetime last_activity: datetime wire: LocalWire = field(init=False) approval_store: ApprovalStore = field(default_factory=ApprovalStore) - repo_path: Path | None = None + repo_path: Path | None = None # legacy/backward-compat metadata only execution_binding: ExecutionBinding = field( default_factory=lambda: LocalExecutionBinding( workspace_root=str(Path.cwd().resolve()) @@ -142,6 +146,8 @@ def to_store_data(self) -> dict[str, Any]: "id": self.id, "created_at": self.created_at.isoformat(), "last_activity": self.last_activity.isoformat(), + # repo_path is legacy/backward-compat metadata; execution_binding is + # the authoritative workspace contract. "repo_path": None if self.repo_path is None else str(self.repo_path), "execution_binding": self.execution_binding.to_dict(), "approval_policy": self.approval_policy.value, @@ -351,6 +357,7 @@ def __init__( checkpoint_store: CheckpointStore | None = None, checkpoint_service: CheckpointService | None = None, create_agent_fn: Callable[..., tuple[Any, Any]] | None = None, + binding_resolver: BindingResolver | None = None, ): self._storage_config = storage_config or {} self._pg_pool = pg_pool @@ -370,6 +377,7 @@ def __init__( resolved_checkpoint_store ) self._create_agent = create_agent_fn + self._binding_resolver = binding_resolver or DefaultBindingResolver() def _get_pg_pool(self) -> AsyncPGSessionPool: if self._pg_pool is not None: @@ -713,9 +721,7 @@ def _persist_session(self, session: Session) -> None: self._store.save(session.id, cast(dict[str, Any], session.to_store_data())) def _resolve_workspace_root(self, session: Session) -> Path: - return DefaultBindingResolver().resolve_workspace_root( - session.execution_binding - ) + return self._binding_resolver.resolve_workspace_root(session.execution_binding) def _invalidate_runtime(self, session: Session) -> None: session.runtime_pipeline = None diff --git a/coding-agent/tests/ui/test_execution_binding.py b/coding-agent/tests/ui/test_execution_binding.py index 259c59fb6..45c429763 100644 --- a/coding-agent/tests/ui/test_execution_binding.py +++ b/coding-agent/tests/ui/test_execution_binding.py @@ -61,3 +61,38 @@ def test_cloud_resolver_raises_not_implemented() -> None: with pytest.raises(NotImplementedError, match="cloud workspace"): resolver.resolve_workspace_root(binding) + + +@pytest.mark.parametrize( + ("payload", "message"), + [ + ( + {"kind": "cloud", "workspace_url": 123, "workspace_id": "ws-123"}, + "string workspace_url and workspace_id", + ), + ( + { + "kind": "cloud", + "workspace_url": "https://workspace.example.com", + "workspace_id": 123, + }, + "string workspace_url and workspace_id", + ), + ], +) +def test_cloud_binding_rejects_invalid_field_types( + payload: dict[str, object], message: str +) -> None: + with pytest.raises(TypeError, match=message): + CloudWorkspaceBinding.from_dict(payload) + + +def test_cloud_resolver_tool_config_raises_not_implemented() -> None: + binding = CloudWorkspaceBinding( + workspace_url="https://workspace.example.com", + workspace_id="ws-123", + ) + resolver = DefaultBindingResolver() + + with pytest.raises(NotImplementedError, match="cloud workspace"): + resolver.resolve_tool_config(binding) From 768724ce0d455e280af9d8a99781161e7ebd2cf7 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Mon, 20 Apr 2026 11:37:39 +0800 Subject: [PATCH 7/7] test(ui): harden execution binding coverage Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...xecution-binding-from-runtime-ownership.md | 2 +- .../tests/ui/test_execution_binding.py | 21 ++++++++------- .../ui/test_session_manager_public_api.py | 16 +++++++----- .../tests/ui/test_session_manager_runtime.py | 26 ++++++++++--------- 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md b/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md index 72196ee7a..a1c52ee98 100644 --- a/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md +++ b/coding-agent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md @@ -24,7 +24,7 @@ Introduce a first-class **execution binding** abstraction that sits beside the o Specifically: - Add an `ExecutionBinding` dataclass/protocol in `coding_agent.ui.execution_binding` with at least two concrete variants: - - `LocalExecutionBinding(workspace_root: Path)` for local filesystem mode. + - `LocalExecutionBinding(workspace_root: str)` for local filesystem mode. - `CloudWorkspaceBinding(workspace_url: str, workspace_id: str)` for remote workspace mode. - Store the serialized binding on the session metadata table as `execution_binding`, separate from `repo_path`. Migrate the current `repo_path` field to mean "local workspace root" only. - Add a `BindingResolver` interface that turns an `ExecutionBinding` into the `workspace_root` and tool configuration that `create_agent_for_session` expects. The resolver lives in `coding_agent.ui.binding_resolver`. diff --git a/coding-agent/tests/ui/test_execution_binding.py b/coding-agent/tests/ui/test_execution_binding.py index 45c429763..3e1e9382a 100644 --- a/coding-agent/tests/ui/test_execution_binding.py +++ b/coding-agent/tests/ui/test_execution_binding.py @@ -1,5 +1,7 @@ from __future__ import annotations +from pathlib import Path + import pytest from coding_agent.ui.binding_resolver import DefaultBindingResolver @@ -10,13 +12,14 @@ ) -def test_local_binding_round_trip() -> None: - binding = LocalExecutionBinding(workspace_root="/tmp/repo") +def test_local_binding_round_trip(tmp_path: Path) -> None: + workspace = tmp_path / "repo" + binding = LocalExecutionBinding(workspace_root=str(workspace)) restored = ExecutionBinding.from_dict(binding.to_dict()) assert isinstance(restored, LocalExecutionBinding) - assert restored.workspace_root == "/tmp/repo" + assert restored.workspace_root == str(workspace) def test_cloud_binding_round_trip() -> None: @@ -42,14 +45,14 @@ def test_local_binding_requires_string_workspace_root() -> None: LocalExecutionBinding.from_dict({"kind": "local", "workspace_root": 123}) -def test_local_resolver_returns_absolute_path() -> None: - binding = LocalExecutionBinding(workspace_root="/tmp/repo") +def test_local_resolver_returns_absolute_path(tmp_path: Path) -> None: + workspace = tmp_path / "repo" + binding = LocalExecutionBinding(workspace_root=str(workspace)) resolver = DefaultBindingResolver() - assert str(resolver.resolve_workspace_root(binding)).endswith("/tmp/repo") - assert resolver.resolve_tool_config(binding) == { - "workspace_root": str(resolver.resolve_workspace_root(binding)) - } + resolved = resolver.resolve_workspace_root(binding) + assert resolved == workspace.resolve() + assert resolver.resolve_tool_config(binding) == {"workspace_root": str(resolved)} def test_cloud_resolver_raises_not_implemented() -> None: diff --git a/coding-agent/tests/ui/test_session_manager_public_api.py b/coding-agent/tests/ui/test_session_manager_public_api.py index fd574dd44..4dc90c1ba 100644 --- a/coding-agent/tests/ui/test_session_manager_public_api.py +++ b/coding-agent/tests/ui/test_session_manager_public_api.py @@ -158,15 +158,16 @@ async def test_create_session_persists_configured_restart_metadata_by_default() assert payload["base_url"] == "http://llm.default" -def test_session_metadata_round_trips_local_execution_binding() -> None: +def test_session_metadata_round_trips_local_execution_binding(tmp_path: Path) -> None: store = InMemorySessionStore() manager = SessionManager(store=store) + bound_repo = tmp_path / "bound-repo" session = Session( id="binding-session", created_at=datetime.now(), last_activity=datetime.now(), approval_store=ApprovalStore(), - execution_binding=LocalExecutionBinding(workspace_root="/tmp/bound-repo"), + execution_binding=LocalExecutionBinding(workspace_root=str(bound_repo)), ) manager.register_session(session) @@ -174,20 +175,23 @@ def test_session_metadata_round_trips_local_execution_binding() -> None: reloaded = SessionManager(store=store).get_session("binding-session") assert isinstance(reloaded.execution_binding, LocalExecutionBinding) - assert reloaded.execution_binding.workspace_root == "/tmp/bound-repo" + assert reloaded.execution_binding.workspace_root == str(bound_repo) -def test_session_metadata_defaults_missing_binding_to_local_from_repo_path() -> None: +def test_session_metadata_defaults_missing_binding_to_local_from_repo_path( + tmp_path: Path, +) -> None: store = InMemorySessionStore() created_at = datetime.now() last_activity = datetime.now() + legacy_repo = tmp_path / "legacy-repo" store.save( "legacy-session", { "id": "legacy-session", "created_at": created_at.isoformat(), "last_activity": last_activity.isoformat(), - "repo_path": "/tmp/legacy-repo", + "repo_path": str(legacy_repo), "approval_policy": "auto", "provider_name": None, "model_name": None, @@ -200,7 +204,7 @@ def test_session_metadata_defaults_missing_binding_to_local_from_repo_path() -> reloaded = SessionManager(store=store).get_session("legacy-session") assert isinstance(reloaded.execution_binding, LocalExecutionBinding) - assert reloaded.execution_binding.workspace_root == "/tmp/legacy-repo" + assert reloaded.execution_binding.workspace_root == str(legacy_repo) def test_event_queue_mutations_do_not_persist_sync_runtime_only_state() -> None: diff --git a/coding-agent/tests/ui/test_session_manager_runtime.py b/coding-agent/tests/ui/test_session_manager_runtime.py index 87371c084..7d2c113d9 100644 --- a/coding-agent/tests/ui/test_session_manager_runtime.py +++ b/coding-agent/tests/ui/test_session_manager_runtime.py @@ -350,7 +350,7 @@ def cancel(self) -> None: self.cancel_calls += 1 fake_task = FakeTask() - session.task = cast(asyncio.Task[None], cast(object, fake_task)) + session.task = cast(asyncio.Task[None], fake_task) with patch( "coding_agent.ui.session_manager.asyncio.wait_for", @@ -380,7 +380,7 @@ def cancel(self) -> None: self.cancel_calls += 1 fake_task = FakeTask() - session.task = cast(asyncio.Task[None], cast(object, fake_task)) + session.task = cast(asyncio.Task[None], fake_task) with patch( "coding_agent.ui.session_manager.asyncio.wait_for", @@ -1030,13 +1030,16 @@ def fake_create_agent(**kwargs): @pytest.mark.asyncio -async def test_run_agent_uses_resolved_workspace_root_from_binding() -> None: +async def test_run_agent_uses_resolved_workspace_root_from_binding( + tmp_path: Path, +) -> None: manager = SessionManager(store=InMemorySessionStore()) session_id = await manager.create_session() session = manager.get_session(session_id) - session.repo_path = Path("/tmp/not-used-directly") + session.repo_path = tmp_path / "not-used-directly" + bound_workspace = tmp_path / "bound-workspace" session.execution_binding = LocalExecutionBinding( - workspace_root="/tmp/bound-workspace" + workspace_root=str(bound_workspace) ) manager.register_session(session) @@ -1065,19 +1068,18 @@ def fake_create_agent(**kwargs): mp.setattr("coding_agent.ui.session_manager.PipelineAdapter", FakeAdapter) await manager.run_agent(session_id, "hello") - assert captured_kwargs["workspace_root"] == Path("/tmp/bound-workspace").resolve() + assert captured_kwargs["workspace_root"] == bound_workspace.resolve() @pytest.mark.asyncio -async def test_restore_checkpoint_preserves_execution_binding() -> None: +async def test_restore_checkpoint_preserves_execution_binding(tmp_path: Path) -> None: store = InMemorySessionStore() manager = SessionManager(store=store) session_id = await manager.create_session() session = manager.get_session(session_id) session.tape_id = "binding-restore-tape" - session.execution_binding = LocalExecutionBinding( - workspace_root="/tmp/restore-bound" - ) + restore_bound = tmp_path / "restore-bound" + session.execution_binding = LocalExecutionBinding(workspace_root=str(restore_bound)) manager.register_session(session) snapshot = types.SimpleNamespace( @@ -1138,9 +1140,9 @@ def fake_create_agent(**kwargs): mp.setattr(manager, "_tape_store", FakeTapeStore(), raising=False) await manager._restore_checkpoint(session, "cp-binding") - assert captured_kwargs["workspace_root"] == Path("/tmp/restore-bound").resolve() + assert captured_kwargs["workspace_root"] == restore_bound.resolve() assert isinstance(session.execution_binding, LocalExecutionBinding) - assert session.execution_binding.workspace_root == "/tmp/restore-bound" + assert session.execution_binding.workspace_root == str(restore_bound) @pytest.mark.asyncio