feat(http): X-Session-Id fallback + drop workspace suffix

CHANGELOG.md

@@ -30,6 +30,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **`X-Session-Id` header is now recognized as a fallback** for the existing `X-Client-Session-Id` correlation mechanism (PR #71). opencode and other Vercel AI SDK-based clients send `X-Session-Id` by default; amplifier-agent now picks it up automatically, so session-resume + client-authoritative reconciliation works for opencode with zero config. `X-Client-Session-Id` remains authoritative when both headers are present.
+
+- **Workspace name is no longer suffixed with the client session id.** Previously, `X-Client-Session-Id: abc` would route requests into `workspaces/<base>-abc/`. Now the workspace stays at `<base>` and per-client distinction is purely at the session_id level (`workspaces/<base>/sessions/http-abc/`). This keeps workspace-level hook state (context-intelligence, etc.) shared across all sessions of the same server process, where it belongs.
+
 - **`amplifier-agent serve chat-completions` lifespan now triggers the same module-install path that `amplifier-agent run` uses.** Previously, a fresh `uv tool install amplifier-agent` followed by `serve chat-completions` would fail with `ProviderModuleNotInstalledError` because the lazy-install that `run` gets via `create_session() → session.initialize() → resolver.async_resolve()` never fires for `serve`. The lifespan now calls `prepared.resolver.async_resolve(module_id, source)` for every `PROVIDER_CATALOG` entry before the providers loop — idempotent (no-op on warm cache) and asynchronous (lifespan waits for completion before opening the wire).
 
 - **`single_turn.py` now explicitly clears `mount_plan["providers"]` before `inject_provider`.** `bundle.md` now populates the top-level `providers:` section with 4 stubs so `bundle.prepare()` installs them. Without the clear, `inject_provider`'s "no-op if providers already present" guard would fire and skip injecting the runtime provider (with env-var credentials). This mirrors the pattern already used by `_session_runner.run_chat_turn`.

src/amplifier_agent_http/routes/chat_completions.py

@@ -593,31 +593,24 @@ async def chat_completions(
     # is in the v2 backlog (would need per-session mount-plan isolation).
     base_workspace = getattr(request.app.state, "resolved_workspace", None)
 
-    # Client session correlation (per-request workspace override).
-    # When the client attaches ``X-Client-Session-Id`` to outbound requests,
-    # the server suffixes the resolved workspace with the supplied value so
-    # all turns of one logical client session land under the same on-disk
-    # bucket:
+    # Client session correlation. When the client attaches X-Client-Session-Id
+    # (amplifier-native) or X-Session-Id (opencode / Vercel AI SDK default),
+    # the deterministic session_id `http-<client-sid>` is used so all turns of
+    # one logical client session land in the same on-disk session bucket
+    # under workspaces/<workspace>/sessions/. The workspace itself is NOT
+    # suffixed -- it stays at server-process scope so hook-level state
+    # (context-intelligence, workspace-scoped tools, etc.) shares cleanly
+    # across client sessions.
     #
-    #   ~/.amplifier-agent/state/workspaces/<base>-<client-sid>/sessions/...
+    #   ~/.amplifier-agent/state/workspaces/<base>/sessions/http-<client-sid>/
     #
-    # Without the header, fall back to the base workspace -- behavior is
-    # identical to non-correlating clients. The header is purely additive
-    # and opt-in; client-side adapter repos own the policy of when to
-    # attach it. amplifier-agent has no opinion on the value's shape beyond
-    # requiring a non-empty trimmed string.
-    client_session_id = request.headers.get("X-Client-Session-Id")
+    # X-Client-Session-Id is authoritative when both headers are present;
+    # X-Session-Id is the fallback for opencode and other Vercel AI SDK clients.
+    client_session_id = request.headers.get("X-Client-Session-Id") or request.headers.get("X-Session-Id")
     client_session_id_clean: str = ""
-    if client_session_id and base_workspace:
-        # Strip whitespace, defensively constrain to a safe slug shape.
-        # Clients are expected to send path-safe IDs.
+    if client_session_id:
         client_session_id_clean = client_session_id.strip()
-        if client_session_id_clean:
-            workspace = f"{base_workspace}-{client_session_id_clean}"
-        else:
-            workspace = base_workspace
-    else:
-        workspace = base_workspace
+    workspace = base_workspace
 
     # Determine the amplifier session_id and resume flag for this turn.
     # When the client provides X-Client-Session-Id, we use it deterministically:
@@ -627,9 +620,8 @@ async def chat_completions(
     sid: str | None
     is_resumed: bool
     if client_session_id_clean:
-        # workspace is guaranteed non-None here: client_session_id_clean is
-        # only set when client_session_id is present AND base_workspace is
-        # truthy (see the outer if-condition above).
+        # client_session_id_clean is only set when client_session_id is
+        # present and non-empty after stripping whitespace.
         sid = f"http-{client_session_id_clean}"
         _ws_root = workspaces_root()
         _state_dir = _ws_root / str(workspace) / "sessions" / sid

tests/http/test_reconciler.py

@@ -323,7 +323,8 @@ async def _fake_run(**kwargs: Any) -> str:
         )
 
     # After turn 2, the store should contain the turn-2 client view (T2).
-    workspace_slug = f"test-ws-{sid_clean}"
+    # workspace is NOT suffixed -- stays at base_workspace ("test-ws").
+    workspace_slug = "test-ws"
     store = SessionStore(tmp_path / workspace_slug)
     loaded = store.load(session_id)
     assert loaded is not None

tests/http/test_session_id_fallback.py

@@ -0,0 +1,232 @@
+"""Tests for X-Session-Id fallback and no-workspace-suffix behavior.
+
+Covers:
+- X-Session-Id alone (no X-Client-Session-Id): server uses it as the client
+  session id, creates session_dir at workspaces/<base>/sessions/http-<sid>/.
+  Workspace is NOT suffixed.
+- Both headers present: X-Client-Session-Id takes precedence (amplifier-native
+  is authoritative).
+- Neither header present: fresh random sid per turn, no resume (legacy behavior
+  unchanged).
+- Workspace path is never suffixed with the client_sid regardless of which
+  header is used.
+"""
+
+from __future__ import annotations
+
+from contextlib import asynccontextmanager
+from pathlib import Path
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+from amplifier_agent_http.routes import chat_completions as cc_module
+
+# ---------------------------------------------------------------------------
+# Shared helpers (mirrors test_reconciler.py)
+# ---------------------------------------------------------------------------
+
+AUTH = {"Authorization": "Bearer test-key"}
+
+_REGISTRY = {"claude-3-5-sonnet-20241022": "anthropic"}
+
+
+def _make_test_app(
+    *,
+    registry: dict[str, str] | None = None,
+    workspace: str | None = None,
+) -> FastAPI:
+    """Build a minimal FastAPI app for session-id fallback tests."""
+    prepared_mock = MagicMock()
+    prepared_mock.mount_plan = {}
+    state_registry = registry or {}
+
+    @asynccontextmanager
+    async def _noop_lifespan(application: FastAPI):
+        application.state.config = MagicMock()
+        application.state.config.model_id = "amplifier"
+        application.state.config.api_key = "test-key"
+        application.state.prepared = prepared_mock
+        application.state.agent_configs = {}
+        application.state.resolved_workspace = workspace
+        application.state.host_config = {}
+        application.state.available_models = []
+        application.state.served_models_registry = state_registry
+        yield
+
+    app = FastAPI(lifespan=_noop_lifespan)
+    app.include_router(cc_module.router)
+    return app
+
+
+def _chat_payload(model: str = "claude-3-5-sonnet-20241022", **kwargs: Any) -> dict[str, Any]:
+    base: dict[str, Any] = {
+        "model": model,
+        "messages": [{"role": "user", "content": "hello"}],
+    }
+    base.update(kwargs)
+    return base
+
+
+# ---------------------------------------------------------------------------
+# Tests
+# ---------------------------------------------------------------------------
+
+
+def test_x_session_id_only_uses_it_as_client_sid(tmp_path: Path) -> None:
+    """Only X-Session-Id set (no X-Client-Session-Id): server uses it as the
+    client session id, creating a deterministic ``http-<sid>`` and storing the
+    session at workspaces/<base>/sessions/http-<sid>/.  Workspace is NOT
+    suffixed."""
+    app = _make_test_app(registry=_REGISTRY, workspace="test-ws")
+    captured: dict[str, Any] = {}
+
+    async def _fake_run(**kwargs: Any) -> str:
+        captured.update(kwargs)
+        return "ok"
+
+    opencode_sid = "ses_10dba803effekTqUujLAFyL9me"
+
+    with (
+        patch(
+            "amplifier_agent_http.routes.chat_completions.run_chat_turn",
+            side_effect=_fake_run,
+        ),
+        patch(
+            "amplifier_agent_http.routes.chat_completions.workspaces_root",
+            return_value=tmp_path,
+        ),
+        TestClient(app, raise_server_exceptions=False) as client,
+    ):
+        resp = client.post(
+            "/v1/chat/completions",
+            json=_chat_payload(),
+            headers={**AUTH, "X-Session-Id": opencode_sid},
+        )
+
+    assert resp.status_code == 200
+    # session_id must be the deterministic http- prefixed form.
+    assert captured.get("session_id") == f"http-{opencode_sid}"
+    # workspace passed to run_chat_turn must be the base workspace, not suffixed.
+    assert captured.get("workspace") == "test-ws"
+
+
+def test_x_client_session_id_wins_over_x_session_id(tmp_path: Path) -> None:
+    """Both headers present: X-Client-Session-Id takes precedence.  The
+    amplifier-native header is authoritative when both are sent."""
+    app = _make_test_app(registry=_REGISTRY, workspace="test-ws")
+    captured: dict[str, Any] = {}
+
+    async def _fake_run(**kwargs: Any) -> str:
+        captured.update(kwargs)
+        return "ok"
+
+    with (
+        patch(
+            "amplifier_agent_http.routes.chat_completions.run_chat_turn",
+            side_effect=_fake_run,
+        ),
+        patch(
+            "amplifier_agent_http.routes.chat_completions.workspaces_root",
+            return_value=tmp_path,
+        ),
+        TestClient(app, raise_server_exceptions=False) as client,
+    ):
+        resp = client.post(
+            "/v1/chat/completions",
+            json=_chat_payload(),
+            headers={
+                **AUTH,
+                "X-Client-Session-Id": "my-native-sid",
+                "X-Session-Id": "opencode-sdk-sid",
+            },
+        )
+
+    assert resp.status_code == 200
+    # X-Client-Session-Id (amplifier-native) must win.
+    assert captured.get("session_id") == "http-my-native-sid"
+
+
+def test_neither_header_set_falls_back_to_random_sid(tmp_path: Path) -> None:
+    """No session headers: behavior is unchanged from pre-PR -- fresh random
+    sid per turn (session_id=None from route perspective), no resume.
+    Workspace is base_workspace."""
+    app = _make_test_app(registry=_REGISTRY, workspace="test-ws")
+    captured_calls: list[dict[str, Any]] = []
+
+    async def _fake_run(**kwargs: Any) -> str:
+        captured_calls.append(dict(kwargs))
+        return "ok"
+
+    with (
+        patch(
+            "amplifier_agent_http.routes.chat_completions.run_chat_turn",
+            side_effect=_fake_run,
+        ),
+        patch(
+            "amplifier_agent_http.routes.chat_completions.workspaces_root",
+            return_value=tmp_path,
+        ),
+        TestClient(app, raise_server_exceptions=False) as client,
+    ):
+        resp1 = client.post(
+            "/v1/chat/completions",
+            json=_chat_payload(),
+            headers=AUTH,  # no session headers
+        )
+        resp2 = client.post(
+            "/v1/chat/completions",
+            json=_chat_payload(),
+            headers=AUTH,
+        )
+
+    assert resp1.status_code == 200
+    assert resp2.status_code == 200
+    # No session id -- runner picks a random sid each time.
+    assert captured_calls[0]["session_id"] is None
+    assert captured_calls[0]["is_resumed"] is False
+    assert captured_calls[1]["session_id"] is None
+    assert captured_calls[1]["is_resumed"] is False
+
+
+def test_x_session_id_workspace_is_not_suffixed(tmp_path: Path) -> None:
+    """Specifically assert the workspace path is NOT suffixed with the
+    client_sid.  The session_dir lives at
+    workspaces/<base>/sessions/http-<sid>/, NOT
+    workspaces/<base>-<sid>/sessions/http-<sid>/."""
+    app = _make_test_app(registry=_REGISTRY, workspace="test-ws")
+
+    async def _fake_run(**kwargs: Any) -> str:
+        return "ok"
+
+    opencode_sid = "ses_abc123"
+    session_id = f"http-{opencode_sid}"
+
+    with (
+        patch(
+            "amplifier_agent_http.routes.chat_completions.run_chat_turn",
+            side_effect=_fake_run,
+        ),
+        patch(
+            "amplifier_agent_http.routes.chat_completions.workspaces_root",
+            return_value=tmp_path,
+        ),
+        TestClient(app, raise_server_exceptions=False) as client,
+    ):
+        client.post(
+            "/v1/chat/completions",
+            json=_chat_payload(),
+            headers={**AUTH, "X-Session-Id": opencode_sid},
+        )
+
+    # The session_dir must be under the un-suffixed base workspace.
+    correct_session_dir = tmp_path / "test-ws" / "sessions" / session_id
+    wrong_workspace_dir = tmp_path / f"test-ws-{opencode_sid}"
+
+    assert correct_session_dir.exists(), (
+        f"Expected session_dir at {correct_session_dir} but it does not exist. "
+        f"tmp_path contents: {list(tmp_path.iterdir())}"
+    )
+    assert not wrong_workspace_dir.exists(), f"Workspace was incorrectly suffixed: {wrong_workspace_dir} exists."