From c1ea006355014ab2644551874639dc272ec76eb3 Mon Sep 17 00:00:00 2001 From: alissa-develop-daemon Date: Thu, 13 Aug 2026 03:29:04 +0000 Subject: [PATCH 1/2] Add typed /v1/bridge queue-mode client bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give the SDK a shared REST client/auth core and, on top of it, typed bindings for the Local Bridge queue-mode surface: the four executor endpoints (register/list/heartbeat/stop) and the seven job endpoints (feed/detail/claim/start/progress/fulfill/fail). The SDK had no HTTP layer at all, so `alissa.sdk.api` introduces one — stdlib only, matching the core's zero-dependency contract. `ApiClient` owns the base URL, the bearer token, JSON encoding and the error envelope; a binding module owns paths and shapes and nothing else, so there is never a second HTTP stack. The transport is a one-method seam, which is what keeps every test offline. Bindings only: no polling loop, no claim state machine, no nonce minting, no tmux. The executor daemon is the Node `alissa` CLI's, and a second implementation of those rules in another language is a divergence waiting to happen. This is plumbing for observers and tooling. The design doc's §5.3 codes each get an exception class, so callers branch on a code rather than on the prose of `message`. The four 409s share a `RetryAfterReadError` base because they all mean "re-read the row and retry" rather than "give up", and they carry their structured detail (`currentClaimSeq`, `stickyExecutorId`, the observed `status`, `held`/`cap`) as typed properties. An unmodelled code degrades to a plain `ApiError` holding the code verbatim rather than crashing a deployed reader. Response models are frozen stdlib dataclasses mirroring the zod schemas in studio.alissa.app's `api/src/schemas/bridge.ts`, modelled against what the wire actually returns: the feed carries the whole spec including the prompt (`summarizeJobForFeed` projects `spec` whole) and it is the *result* side — result, error, timing, cancelRequested — that only `GET /jobs/:jobId` has. Tests replay recorded response bodies through an injected transport, covering all eleven endpoints, both halves of each binding (request shape and typed response), the noop/absorb and cancel paths, and one branch per §5.3 error family including STALE_CONSUMER. Co-Authored-By: Claude Opus 5 Co-Authored-By: alissa-app --- README.md | 19 + alissa/README.md | 51 ++ alissa/src/main/alissa/sdk/api/__init__.py | 64 ++ .../main/alissa/sdk/api/bridge/__init__.py | 61 ++ .../src/main/alissa/sdk/api/bridge/client.py | 269 ++++++++ .../src/main/alissa/sdk/api/bridge/models.py | 650 ++++++++++++++++++ alissa/src/main/alissa/sdk/api/client.py | 228 ++++++ alissa/src/main/alissa/sdk/api/errors.py | 228 ++++++ alissa/src/main/alissa/sdk/version | 2 +- .../test_alissa/test_sdk/test_api/conftest.py | 94 +++ .../fixtures/error_capacity_exceeded.json | 6 + .../test_api/fixtures/error_cas_conflict.json | 6 + .../test_api/fixtures/error_conflict.json | 5 + .../test_api/fixtures/error_forbidden.json | 4 + .../test_api/fixtures/error_not_found.json | 4 + .../fixtures/error_precondition_failed.json | 4 + .../fixtures/error_stale_consumer.json | 5 + .../fixtures/error_sticky_violation.json | 6 + .../test_api/fixtures/error_unauthorized.json | 4 + .../test_api/fixtures/error_unknown_code.json | 5 + .../test_api/fixtures/error_validation.json | 4 + .../test_api/fixtures/executor_heartbeat.json | 4 + .../fixtures/executor_heartbeat_missing.json | 4 + .../test_api/fixtures/executor_stop.json | 6 + .../test_api/fixtures/executors_list.json | 40 ++ .../test_api/fixtures/executors_register.json | 18 + .../test_sdk/test_api/fixtures/job_claim.json | 42 ++ .../test_api/fixtures/job_detail.json | 80 +++ .../test_api/fixtures/job_detail_failed.json | 24 + .../test_api/fixtures/job_fail_retry.json | 5 + .../test_api/fixtures/job_fail_terminal.json | 5 + .../test_api/fixtures/job_fulfill.json | 3 + .../test_api/fixtures/job_fulfill_noop.json | 5 + .../test_api/fixtures/job_progress.json | 5 + .../fixtures/job_progress_coalesced.json | 5 + .../test_sdk/test_api/fixtures/job_start.json | 5 + .../fixtures/job_start_cancelled.json | 5 + .../test_api/fixtures/job_start_noop.json | 5 + .../test_sdk/test_api/fixtures/jobs_feed.json | 69 ++ .../test_sdk/test_api/test_api_client.py | 115 ++++ .../test_sdk/test_api/test_bridge_errors.py | 163 +++++ .../test_api/test_bridge_executors.py | 176 +++++ .../test_sdk/test_api/test_bridge_jobs.py | 349 ++++++++++ .../test_api/test_urllib_transport.py | 86 +++ 44 files changed, 2937 insertions(+), 1 deletion(-) create mode 100644 alissa/src/main/alissa/sdk/api/__init__.py create mode 100644 alissa/src/main/alissa/sdk/api/bridge/__init__.py create mode 100644 alissa/src/main/alissa/sdk/api/bridge/client.py create mode 100644 alissa/src/main/alissa/sdk/api/bridge/models.py create mode 100644 alissa/src/main/alissa/sdk/api/client.py create mode 100644 alissa/src/main/alissa/sdk/api/errors.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/conftest.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_capacity_exceeded.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_cas_conflict.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_conflict.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_forbidden.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_not_found.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_precondition_failed.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_stale_consumer.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_sticky_violation.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unauthorized.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unknown_code.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_validation.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat_missing.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_stop.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_list.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_register.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_claim.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail_failed.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_retry.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_terminal.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill_noop.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress_coalesced.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_cancelled.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_noop.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/fixtures/jobs_feed.json create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_errors.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_executors.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_jobs.py create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py diff --git a/README.md b/README.md index 367627d..43e6422 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,24 @@ pip install 'alissa[tools.github.revloop]' # + a tool, merged into alissa. pip install 'alissa[all]' # + every tool ``` +Beyond the namespace, the core ships typed REST bindings under `alissa.sdk.api` +— a shared client/auth core plus one module per API surface. Today that is +`alissa.sdk.api.bridge`, the Local Bridge queue-mode executor and job endpoints: + +```python +from alissa.sdk.api import BridgeClient + +bridge = BridgeClient() # token from $ALISSA_API_TOKEN +for executor in bridge.list_executors(): + print(executor.executor_id, executor.status) + +job = bridge.get_job("j57bridge0001") +print(job.spec.title, job.status, f"attempt {job.attempt}/{job.max_attempts}") +``` + +See [`alissa/README.md`](./alissa/README.md#api-bindings-alissasdkapi) for the +error model and the rest of the surface. + ## How ownership works `alissa` is the anchor distribution: the thing you `pip install`, the top-level @@ -48,6 +66,7 @@ alissa-python-sdk/ │ ├── MANIFEST.in │ └── src/ │ ├── main/alissa/sdk/ ← owned leaf: SDK surface (+ plain-text `version` file) +│ │ └── api/ ← REST bindings: shared client/auth core + per-surface modules │ ├── main/alissa/utils/ ← owned leaf: shared helpers (alissa.utils.version) │ └── test/test_alissa/ ← mirrors main as test_* ├── .github/workflows/ ← style / types / tests / wheel / publish (matrix: alissa) diff --git a/alissa/README.md b/alissa/README.md index b0136d3..51c7693 100644 --- a/alissa/README.md +++ b/alissa/README.md @@ -85,6 +85,57 @@ probing each through the import machinery — so it is correct for both wheel an editable installs. The curated set lives in `src/main/alissa/sdk/_tools.py`, the same registry `setup.py` builds the extras from. +## API bindings (`alissa.sdk.api`) + +Typed access to the Alissa REST API — stdlib-only, like the rest of the core. +One `ApiClient` owns the base URL, the bearer token, JSON and the error +envelope; each API surface is a binding module on top of it, so there is never a +second HTTP stack in this SDK. + +Configuration: `ALISSA_API_TOKEN` (or `token=`) and `ALISSA_BASE` (or +`base_url=`, default `https://api.alissa.app`). + +### Local Bridge queue mode (`alissa.sdk.api.bridge`) + +The `/v1/bridge` executor and job surface: four executor endpoints (register, +list, heartbeat, stop) and seven job endpoints (feed, detail, claim, start, +progress, fulfill, fail). + +**Bindings only.** The executor *daemon* — polling, claim state machine, tmux — +is the Node `alissa` CLI's, and none of it lives here. This module is typed +request/response plumbing for observers and tooling. + +```python +from alissa.sdk.api import BridgeClient + +bridge = BridgeClient() # token from $ALISSA_API_TOKEN + +# Which machines are registered, and are they alive? +for executor in bridge.list_executors(): + print(executor.executor_id, executor.status, executor.last_heartbeat_at) + +# Tail one job's status. +job = bridge.get_job("j57bridge0001") +print(job.spec.title, job.status, f"attempt {job.attempt}/{job.max_attempts}") +print(job.progress_note or job.error or "") +``` + +Errors are branchable by **code**, never by message text. The queue's four 409s +share one base, because they all mean *re-read the row and retry* rather than +*give up*: + +```python +from alissa.sdk.api import RetryAfterReadError, StaleConsumerError + +try: + claim = bridge.claim_job(job_id, executor_id=executor_id, + consumer_id=nonce, claim_seq=claim_seq) +except StaleConsumerError: + pass # a newer attempt owns this row — stop +except RetryAfterReadError as err: + print(err.code, err.observed_status) # re-poll, do not spin +``` + ## Shared utilities (`alissa.utils`) Helpers the SDK factors out so every `alissa.*` distribution reuses one diff --git a/alissa/src/main/alissa/sdk/api/__init__.py b/alissa/src/main/alissa/sdk/api/__init__.py new file mode 100644 index 0000000..730a62d --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/__init__.py @@ -0,0 +1,64 @@ +"""``alissa.sdk.api`` — typed bindings for the Alissa REST API. + +Two layers, deliberately separated: + +* **The core** — :class:`~alissa.sdk.api.client.ApiClient` (base URL, bearer + token, JSON, timeouts) and :mod:`alissa.sdk.api.errors` (the failure envelope + as a class per code). Every binding shares these; there is no second HTTP + stack anywhere in this SDK. +* **The bindings** — one subpackage per API surface, owning paths and shapes and + nothing else. Today: :mod:`alissa.sdk.api.bridge`, the Local Bridge queue-mode + executor and job endpoints. + +Both are stdlib-only, matching the SDK core's zero-dependency contract. + + from alissa.sdk.api import ApiClient, BridgeClient, StaleConsumerError + + bridge = BridgeClient(ApiClient(token="alissa_…")) + for executor in bridge.list_executors(): + print(executor.executor_id, executor.status) +""" +from .bridge import BridgeClient +from .client import DEFAULT_BASE_URL, ApiClient, HttpRequest, HttpResponse, Transport, UrllibTransport +from .errors import ( + AlissaError, + ApiError, + CapacityExceededError, + CasConflictError, + ConflictError, + ForbiddenError, + MissingTokenError, + NotFoundError, + PreconditionFailedError, + RetryAfterReadError, + StaleConsumerError, + StickyViolationError, + TransportError, + UnauthorizedError, + ValidationError, +) + +__all__ = [ + "ApiClient", + "BridgeClient", + "DEFAULT_BASE_URL", + "HttpRequest", + "HttpResponse", + "Transport", + "UrllibTransport", + "AlissaError", + "ApiError", + "CapacityExceededError", + "CasConflictError", + "ConflictError", + "ForbiddenError", + "MissingTokenError", + "NotFoundError", + "PreconditionFailedError", + "RetryAfterReadError", + "StaleConsumerError", + "StickyViolationError", + "TransportError", + "UnauthorizedError", + "ValidationError", +] diff --git a/alissa/src/main/alissa/sdk/api/bridge/__init__.py b/alissa/src/main/alissa/sdk/api/bridge/__init__.py new file mode 100644 index 0000000..454da91 --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/bridge/__init__.py @@ -0,0 +1,61 @@ +"""``alissa.sdk.api.bridge`` — typed bindings for the Local Bridge queue mode. + +The ``/v1/bridge`` executor and job surface of the Alissa API, mirroring +``api/src/schemas/bridge.ts`` (design: ``docs/design/local-bridge-queue-mode.md`` +§5). :mod:`~alissa.sdk.api.bridge.client` holds the eleven endpoint methods; +:mod:`~alissa.sdk.api.bridge.models` holds the response structures. + +Request/response plumbing only — the executor daemon itself is the Node `alissa` +CLI's, and lives nowhere in this package. +""" +from .client import BRIDGE_PREFIX, EXECUTOR_KIND, BridgeClient +from .models import ( + ExecutorCapabilities, + ExecutorHeartbeat, + ExecutorRegistration, + ExecutorStopResult, + ExecutorSummary, + JobAcceptanceCriterion, + JobClaim, + JobDeliverable, + JobDetail, + JobFailAck, + JobFeed, + JobFeedItem, + JobFulfillAck, + JobProgressAck, + JobReference, + JobResult, + JobResultAcceptance, + JobResultLink, + JobSpec, + JobStartAck, + ResumedJob, +) + +__all__ = [ + "BRIDGE_PREFIX", + "EXECUTOR_KIND", + "BridgeClient", + "ExecutorCapabilities", + "ExecutorHeartbeat", + "ExecutorRegistration", + "ExecutorStopResult", + "ExecutorSummary", + "JobAcceptanceCriterion", + "JobClaim", + "JobDeliverable", + "JobDetail", + "JobFailAck", + "JobFeed", + "JobFeedItem", + "JobFulfillAck", + "JobProgressAck", + "JobReference", + "JobResult", + "JobResultAcceptance", + "JobResultLink", + "JobSpec", + "JobStartAck", + "ResumedJob", +] diff --git a/alissa/src/main/alissa/sdk/api/bridge/client.py b/alissa/src/main/alissa/sdk/api/bridge/client.py new file mode 100644 index 0000000..7390f4b --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/bridge/client.py @@ -0,0 +1,269 @@ +"""Bindings for the Local Bridge queue-mode surface (``/v1/bridge``). + +Eleven endpoints, one method each: the executor lifecycle (§5.1) and the job +lifecycle (§5.2) of ``docs/design/local-bridge-queue-mode.md``, as implemented by +``api/src/routes/bridge.ts`` in fahera-mx/studio.alissa.app. + +**Bindings only.** There is no polling loop, no claim state machine, no nonce +minting and no tmux here, on purpose: the executor *daemon* is the Node `alissa` +CLI's, and a second implementation of those rules in another language is a +divergence waiting to happen. What this module is for is observers and tooling — +reading the queue, tailing a job, and operating a row by hand. + + from alissa.sdk.api import BridgeClient + + bridge = BridgeClient() # token from $ALISSA_API_TOKEN + for executor in bridge.list_executors(): + print(executor.executor_id, executor.status) + + job = bridge.get_job("j57…") + print(job.status, job.attempt, "/", job.max_attempts, job.progress_note) +""" +from __future__ import annotations + +from typing import Any, Mapping, Sequence + +from ..client import ApiClient, encode_path +from .models import ( + ExecutorCapabilities, + ExecutorHeartbeat, + ExecutorRegistration, + ExecutorStopResult, + ExecutorSummary, + JobClaim, + JobDetail, + JobFailAck, + JobFeed, + JobFulfillAck, + JobProgressAck, + JobResult, + JobStartAck, +) + +#: Path prefix for the whole surface. +BRIDGE_PREFIX = "/v1/bridge" + +#: The only executor kind this contract accepts (``z.literal("alissa-code")``). +EXECUTOR_KIND = "alissa-code" + + +class BridgeClient: + """Typed access to ``/v1/bridge``'s executor and job endpoints. + + :param client: an :class:`~alissa.sdk.api.client.ApiClient` to send through. + Pass one to share a transport (or to inject a fake); omit it and the + remaining keyword arguments build one. + :param token: forwarded to :class:`ApiClient` when ``client`` is omitted. + :param base_url: forwarded to :class:`ApiClient` when ``client`` is omitted. + + Every method raises the typed errors in :mod:`alissa.sdk.api.errors` — in + particular :class:`~alissa.sdk.api.errors.RetryAfterReadError` and its four + subclasses for §5.3's 409s, which mean *re-read the row and retry*, not + *give up*. + """ + + def __init__( + self, + client: ApiClient | None = None, + *, + token: str | None = None, + base_url: str | None = None, + ) -> None: + self.client = client or ApiClient(token=token, base_url=base_url) + + # ── Executor lifecycle (§5.1) ──────────────────────────────────────────── + + def register_executor( + self, + executor_id: str, + *, + label: str, + hostname: str, + fingerprint: str, + kind: str = EXECUTOR_KIND, + cli_version: str | None = None, + poll_seconds: int | None = None, + worker_name: str | None = None, + capabilities: ExecutorCapabilities | None = None, + ) -> ExecutorRegistration: + """``POST /v1/bridge/executors`` — register or take over this machine's executor. + + Upsert by (user, executorId) with takeover semantics: an existing row is + refreshed and any close cleared. ``fingerprint`` is a stable machine id + kept server-side — renaming a laptop must not create a second executor. + + The response's ``resumed`` list is this executor's own non-terminal jobs, + so a restarting caller reconciles in one round trip. + """ + payload: dict[str, Any] = { + "executorId": executor_id, + "kind": kind, + "label": label, + "hostname": hostname, + "fingerprint": fingerprint, + "cliVersion": cli_version, + "pollSeconds": poll_seconds, + "workerName": worker_name, + "capabilities": capabilities.to_wire() if capabilities is not None else None, + } + return ExecutorRegistration.from_wire(self.client.post(f"{BRIDGE_PREFIX}/executors", body=payload)) + + def list_executors(self) -> tuple[ExecutorSummary, ...]: + """``GET /v1/bridge/executors`` — this user's executors. + + ``status`` is derived from the heartbeat and never stored; machine + fingerprints are not returned. + """ + payload = self.client.get(f"{BRIDGE_PREFIX}/executors") + rows = payload.get("executors") if isinstance(payload, Mapping) else None + return tuple(ExecutorSummary.from_wire(row) for row in (rows or ())) + + def heartbeat_executor(self, executor_id: str) -> ExecutorHeartbeat: + """``POST /v1/bridge/executors/{id}/heartbeat`` — report this executor alive. + + For a caller that is not polling, and as the fallback when a poll fails — + a polling daemon's beat rides :meth:`list_jobs` instead. Writes coalesce + to one a minute. ``beat == "missing"`` means re-register. + """ + path = f"{BRIDGE_PREFIX}/executors/{encode_path(executor_id)}/heartbeat" + # An empty JSON object rather than no body at all: the route reads + # nothing from it, but a bodyless POST is the kind of request proxies + # and body parsers disagree about. + return ExecutorHeartbeat.from_wire(self.client.post(path, body={})) + + def stop_executor(self, executor_id: str, *, reason: str | None = None) -> ExecutorStopResult: + """``POST /v1/bridge/executors/{id}/stop`` — close it and release its jobs. + + Non-terminal jobs terminate immediately as ``executor_stopped`` — a + deliberate stop is proof of death, so there is no reason to wait for the + sweep, and it fires no escalation the way ``executor_lost`` does. + Idempotent: a repeat call releases nothing. + """ + path = f"{BRIDGE_PREFIX}/executors/{encode_path(executor_id)}/stop" + return ExecutorStopResult.from_wire(self.client.post(path, body={"reason": reason})) + + # ── Jobs (§5.2) ────────────────────────────────────────────────────────── + + def list_jobs( + self, + *, + executor_id: str, + status: str | Sequence[str] | None = None, + limit: int | None = None, + ) -> JobFeed: + """``GET /v1/bridge/jobs`` — the claimable slice for one executor. + + :param executor_id: required. Jobs are pinned to one executor and never + migrated, so there is no "all executors" feed. + :param status: ``pending`` (the server's default), ``claimed`` or + ``running`` — one, or a sequence. A misspelled status is a 400, never + a silently empty page. + :param limit: 1–50, default 25 server-side. + + The heartbeat rides this call, which is why the response carries ``beat``. + """ + statuses = [status] if isinstance(status, str) else (list(status) if status is not None else None) + query: dict[str, Any] = { + "executorId": executor_id, + "status": ",".join(statuses) if statuses else None, + "limit": limit, + } + return JobFeed.from_wire(self.client.get(f"{BRIDGE_PREFIX}/jobs", query=query)) + + def get_job(self, job_id: str) -> JobDetail: + """``GET /v1/bridge/jobs/{jobId}`` — one job, with lifecycle timing. + + For reconciling a row you already hold — an id from a registration's + ``resumed``, or a job you lost track of mid-run. Carries + ``cancel_requested`` and the full timing the feed omits. + """ + return JobDetail.from_wire(self.client.get(f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}")["job"]) + + def claim_job(self, job_id: str, *, executor_id: str, consumer_id: str, claim_seq: int) -> JobClaim: + """``POST /v1/bridge/jobs/{id}/claim`` — compare-and-swap on ``claimSeq``. + + Send back the ``claim_seq`` read from the feed and a per-attempt + ``consumer_id`` you have persisted: that nonce is what every later write + is checked against, so a caller that hung and woke up cannot overwrite + the attempt that replaced it. + + :raises CasConflictError: the generation moved — re-poll. + :raises StickyViolationError: the job is pinned to another executor. + :raises ConflictError: the row is not ``pending``. + :raises CapacityExceededError: this executor is at ``maxConcurrentJobs``. + :raises PreconditionFailedError: the executor is closed; re-register first. + """ + path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/claim" + body = {"executorId": executor_id, "consumerId": consumer_id, "claimSeq": claim_seq} + return JobClaim.from_wire(self.client.post(path, body=body)) + + def start_job( + self, + job_id: str, + *, + consumer_id: str, + executor_session_id: str | None = None, + tmux_session: str | None = None, + ) -> JobStartAck: + """``POST /v1/bridge/jobs/{id}/start`` — a session now exists for this job. + + Also the first place a cancel becomes observable: check + ``cancel_requested`` on the response before spawning, or you start a + session you are about to tear down. + """ + path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/start" + body = { + "consumerId": consumer_id, + "executorSessionId": executor_session_id, + "tmuxSession": tmux_session, + } + return JobStartAck.from_wire(self.client.post(path, body=body)) + + def progress_job(self, job_id: str, *, consumer_id: str, note: str | None = None) -> JobProgressAck: + """``POST /v1/bridge/jobs/{id}/progress`` — beat a running job, observe a cancel. + + Required every five minutes or the stall deadline collects the row. + ``note`` is a one-line UI status of at most 500 characters, last write + wins; a note posted inside the one-a-minute coalescing window is dropped + (``coalesced``). It is not a log stream. + """ + path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/progress" + return JobProgressAck.from_wire(self.client.post(path, body={"consumerId": consumer_id, "note": note})) + + def fulfill_job(self, job_id: str, *, consumer_id: str, result: JobResult) -> JobFulfillAck: + """``POST /v1/bridge/jobs/{id}/fulfill`` — deliver the result. + + A result arriving after a deadline swept the row is absorbed rather than + errored: ``noop`` with the ``current_status`` that absorbed it. + """ + path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/fulfill" + body = {"consumerId": consumer_id, "result": result.to_wire()} + return JobFulfillAck.from_wire(self.client.post(path, body=body)) + + def fail_job( + self, + job_id: str, + *, + consumer_id: str, + error: str, + retryable: bool, + failure_kind: str | None = None, + ) -> JobFailAck: + """``POST /v1/bridge/jobs/{id}/fail`` — report a failed run. + + :param retryable: ``True`` hands the row back to **this** executor with a + fresh generation (jobs are never migrated to another machine) until + the attempt budget is spent, at which point it goes terminal instead. + :param failure_kind: only ``executor_error``, ``spec_rejected`` or + ``cancelled``. The server-only kinds (``executor_lost``, + ``executor_stopped``, ``no_executor``, ``stalled``, ``deadline``) are + refused here so a caller cannot forge them. + """ + path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/fail" + body = { + "consumerId": consumer_id, + "error": error, + "retryable": retryable, + "failureKind": failure_kind, + } + return JobFailAck.from_wire(self.client.post(path, body=body)) diff --git a/alissa/src/main/alissa/sdk/api/bridge/models.py b/alissa/src/main/alissa/sdk/api/bridge/models.py new file mode 100644 index 0000000..8bd1c47 --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/bridge/models.py @@ -0,0 +1,650 @@ +"""Typed structures for the Local Bridge queue-mode wire (``/v1/bridge``). + +Every model here mirrors a zod schema in ``api/src/schemas/bridge.ts`` of +fahera-mx/studio.alissa.app (design: ``docs/design/local-bridge-queue-mode.md`` +§5). Nothing is invented and nothing is inferred: a field exists here only +because the server sends it, optionality matches the schema's, and the names are +the wire's names in snake_case. + +Two conventions worth knowing before reading: + +* **Plain stdlib dataclasses, frozen.** The SDK core carries zero third-party + dependencies, so there is no pydantic to validate against — ``from_wire`` maps + and coerces, it does not enforce. Responses are server-shaped data, and a + binding that rejected a field the server legitimately added would be a + liability, not a safety net. +* **Unknown keys are dropped, absent keys become ``None``.** That is what makes + these forward-compatible: a new optional field on the server does not break a + deployed reader. + +.. note:: + + The **feed carries the full spec, prompt included** — ``summarizeJobForFeed`` + in ``convex/apiBridgeOps.ts`` projects ``spec`` whole. What the feed omits is + the *result* side (``result``, ``error``, timing, ``cancelRequested``); those + are :class:`JobDetail`, i.e. ``GET /v1/bridge/jobs/{jobId}``. +""" +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Mapping, Sequence + +__all__ = [ + "ExecutorCapabilities", + "ExecutorSummary", + "ExecutorRegistration", + "ExecutorHeartbeat", + "ExecutorStopResult", + "ResumedJob", + "JobDeliverable", + "JobAcceptanceCriterion", + "JobReference", + "JobSpec", + "JobFeedItem", + "JobFeed", + "JobDetail", + "JobClaim", + "JobStartAck", + "JobProgressAck", + "JobResultLink", + "JobResultAcceptance", + "JobResult", + "JobFulfillAck", + "JobFailAck", +] + + +def _mapping(value: Any) -> dict[str, Any]: + return dict(value) if isinstance(value, Mapping) else {} + + +def _sequence(value: Any) -> list[Any]: + return list(value) if isinstance(value, Sequence) and not isinstance(value, (str, bytes)) else [] + + +def _str_tuple(value: Any) -> tuple[str, ...] | None: + """Optional list-of-strings field: ``None`` when absent, never a silent ``()``. + + The distinction is load-bearing on this wire — an absent + ``capabilities.workspaceRoots`` means "accepts any workspace", while an + empty one would mean "accepts none". + """ + if value is None: + return None + return tuple(str(item) for item in _sequence(value)) + + +# ── Executor lifecycle (§5.1) ──────────────────────────────────────────────── + + +@dataclass(frozen=True, slots=True) +class ExecutorCapabilities: + """What an executor will accept. Every field optional, and absence means something. + + ``workspace_roots`` absent ⇒ any workspace. ``max_concurrent_jobs`` is + clamped server-side to [1, 16]. ``handoffs`` absent ⇒ the daemon's default. + ``tags`` is reserved for v2 pool routing and unread today. + """ + + workspace_roots: tuple[str, ...] | None = None + max_concurrent_jobs: int | None = None + handoffs: tuple[str, ...] | None = None + tags: tuple[str, ...] | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorCapabilities": + return cls( + workspace_roots=_str_tuple(payload.get("workspaceRoots")), + max_concurrent_jobs=payload.get("maxConcurrentJobs"), + handoffs=_str_tuple(payload.get("handoffs")), + tags=_str_tuple(payload.get("tags")), + ) + + def to_wire(self) -> dict[str, Any]: + """The registration payload. Unset fields are omitted, never sent as null.""" + payload: dict[str, Any] = {} + if self.workspace_roots is not None: + payload["workspaceRoots"] = list(self.workspace_roots) + if self.max_concurrent_jobs is not None: + payload["maxConcurrentJobs"] = self.max_concurrent_jobs + if self.handoffs is not None: + payload["handoffs"] = list(self.handoffs) + if self.tags is not None: + payload["tags"] = list(self.tags) + return payload + + +@dataclass(frozen=True, slots=True) +class ResumedJob: + """A non-terminal job the executor already held, returned by registration.""" + + job_id: str + status: str + attempt: int + #: Absent on a ``pending`` row — nothing holds it yet. + consumer_id: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ResumedJob": + return cls( + job_id=payload["jobId"], + status=payload["status"], + attempt=payload["attempt"], + consumer_id=payload.get("consumerId"), + ) + + +@dataclass(frozen=True, slots=True) +class ExecutorRegistration: + """``POST /v1/bridge/executors`` — the upsert, plus anything already in flight.""" + + executor_id: str + #: The row was still open: another daemon may be running under this id. + took_over: bool + #: This id arrived from a different machine. Benign after a re-image; a flap + #: means two machines are fighting over one executor id. + fingerprint_changed: bool + #: This executor's own non-terminal jobs, so a restart reconciles in one round trip. + resumed: tuple[ResumedJob, ...] = () + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorRegistration": + return cls( + executor_id=payload["executorId"], + took_over=bool(payload.get("tookOver")), + fingerprint_changed=bool(payload.get("fingerprintChanged")), + resumed=tuple(ResumedJob.from_wire(_mapping(row)) for row in _sequence(payload.get("resumed"))), + ) + + +@dataclass(frozen=True, slots=True) +class ExecutorHeartbeat: + """``POST /v1/bridge/executors/{id}/heartbeat`` — what the beat did. + + ``beat`` is one of ``missing`` (re-register), ``closed`` (the row was stopped + deliberately), ``resumed`` (a stale close was undone), ``touched``, or + ``coalesced`` (counted, not written). A ``missing`` row comes back as a + *value*, not a 404. + """ + + executor_id: str + beat: str + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorHeartbeat": + return cls(executor_id=payload["executorId"], beat=payload["beat"]) + + +@dataclass(frozen=True, slots=True) +class ExecutorStopResult: + """``POST /v1/bridge/executors/{id}/stop`` — whether it closed, and what went with it.""" + + executor_id: str + found: bool + already_ended: bool + #: Non-terminal jobs terminated as ``executor_stopped``. A repeat call + #: releases nothing — the endpoint is idempotent. + released_jobs: int + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorStopResult": + return cls( + executor_id=payload["executorId"], + found=bool(payload.get("found")), + already_ended=bool(payload.get("alreadyEnded")), + released_jobs=payload.get("releasedJobs", 0), + ) + + +@dataclass(frozen=True, slots=True) +class ExecutorSummary: + """One row of ``GET /v1/bridge/executors``. + + No ``fingerprint``: machine identifiers stay server-side. ``status`` is + derived from the heartbeat and never stored — ``"active"`` or the end reason. + All timestamps are epoch milliseconds, as the wire sends them. + """ + + executor_id: str + kind: str + label: str + hostname: str + started_at: int + last_heartbeat_at: int + status: str + cli_version: str | None = None + poll_seconds: int | None = None + worker_name: str | None = None + capabilities: ExecutorCapabilities | None = None + ended_at: int | None = None + end_reason: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorSummary": + capabilities = payload.get("capabilities") + return cls( + executor_id=payload["executorId"], + kind=payload["kind"], + label=payload["label"], + hostname=payload["hostname"], + started_at=payload["startedAt"], + last_heartbeat_at=payload["lastHeartbeatAt"], + status=payload["status"], + cli_version=payload.get("cliVersion"), + poll_seconds=payload.get("pollSeconds"), + worker_name=payload.get("workerName"), + capabilities=( + ExecutorCapabilities.from_wire(_mapping(capabilities)) if capabilities is not None else None + ), + ended_at=payload.get("endedAt"), + end_reason=payload.get("endReason"), + ) + + +# ── Job spec (§5.2) ────────────────────────────────────────────────────────── + + +@dataclass(frozen=True, slots=True) +class JobDeliverable: + """What the job is expected to produce: ``pull_request``/``patch``/``artifact``/``report``.""" + + kind: str + description: str + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobDeliverable": + return cls(kind=payload["kind"], description=payload["description"]) + + +@dataclass(frozen=True, slots=True) +class JobAcceptanceCriterion: + """One acceptance criterion on the spec: ``type`` is ``manual`` or ``automated``.""" + + id: str + description: str + type: str + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobAcceptanceCriterion": + return cls(id=payload["id"], description=payload["description"], type=payload["type"]) + + +@dataclass(frozen=True, slots=True) +class JobReference: + """A pointer carried with the spec: ``task``, ``url``, ``repo`` or ``evidence``.""" + + kind: str + ref: str + label: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobReference": + return cls(kind=payload["kind"], ref=payload["ref"], label=payload.get("label")) + + +@dataclass(frozen=True, slots=True) +class JobSpec: + """The contract the executor was handed. **Never a credential channel.** + + ``env`` carries variable *names* only — the daemon resolves values from its + own environment, and nothing here ever holds a secret. + """ + + title: str + prompt: str + deliverable: JobDeliverable + acceptance: tuple[JobAcceptanceCriterion, ...] = () + references: tuple[JobReference, ...] | None = None + workspace_root: str | None = None + handoff: str | None = None + env: tuple[str, ...] | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobSpec": + references = payload.get("references") + return cls( + title=payload["title"], + prompt=payload["prompt"], + deliverable=JobDeliverable.from_wire(_mapping(payload.get("deliverable"))), + acceptance=tuple( + JobAcceptanceCriterion.from_wire(_mapping(row)) for row in _sequence(payload.get("acceptance")) + ), + references=( + tuple(JobReference.from_wire(_mapping(row)) for row in _sequence(references)) + if references is not None + else None + ), + workspace_root=payload.get("workspaceRoot"), + handoff=payload.get("handoff"), + env=_str_tuple(payload.get("env")), + ) + + +# ── Job feed and detail (§5.2) ─────────────────────────────────────────────── + + +@dataclass(frozen=True, slots=True) +class JobFeedItem: + """One claimable row from ``GET /v1/bridge/jobs``. + + ``claim_seq`` is the CAS generation — send it back verbatim when you claim, + or the server answers ``CAS_CONFLICT``. ``status`` is one of ``pending``, + ``claimed``, ``running``: the feed is the claimable slice, never a history. + """ + + job_id: str + claim_seq: int + attempt: int + max_attempts: int + status: str + spec: JobSpec + created_at: int + pending_expires_at: int + consumer_id: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobFeedItem": + return cls( + job_id=payload["jobId"], + claim_seq=payload["claimSeq"], + attempt=payload["attempt"], + max_attempts=payload["maxAttempts"], + status=payload["status"], + spec=JobSpec.from_wire(_mapping(payload.get("spec"))), + created_at=payload["createdAt"], + pending_expires_at=payload["pendingExpiresAt"], + consumer_id=payload.get("consumerId"), + ) + + +@dataclass(frozen=True, slots=True) +class JobFeed: + """``GET /v1/bridge/jobs`` — the claimable slice plus the folded-in heartbeat. + + The poll *is* the liveness signal, which is why ``beat`` rides back on it; + ``beat == "missing"`` means re-register before doing anything else. + """ + + jobs: tuple[JobFeedItem, ...] + beat: str + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobFeed": + return cls( + jobs=tuple(JobFeedItem.from_wire(_mapping(row)) for row in _sequence(payload.get("jobs"))), + beat=payload["beat"], + ) + + +@dataclass(frozen=True, slots=True) +class JobResultLink: + """A labelled link on a job result (at most 25 per result).""" + + label: str + url: str + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobResultLink": + return cls(label=payload["label"], url=payload["url"]) + + def to_wire(self) -> dict[str, Any]: + return {"label": self.label, "url": self.url} + + +@dataclass(frozen=True, slots=True) +class JobResultAcceptance: + """The executor's self-report for one acceptance criterion — a claim, not a decision.""" + + id: str + met: bool + note: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobResultAcceptance": + return cls(id=payload["id"], met=bool(payload["met"]), note=payload.get("note")) + + def to_wire(self) -> dict[str, Any]: + payload: dict[str, Any] = {"id": self.id, "met": self.met} + if self.note is not None: + payload["note"] = self.note + return payload + + +@dataclass(frozen=True, slots=True) +class JobResult: + """What ``fulfill`` delivers, and what ``GET /jobs/{id}`` returns once it has. + + Server-side caps: ``summary`` ≤ 2 000 UTF-8 bytes, ``markdown`` ≤ 200 000, + ``links`` ≤ 25. They are the server's to enforce — this binding does not + pre-reject, so a cap change does not need an SDK release. + """ + + summary: str + markdown: str | None = None + links: tuple[JobResultLink, ...] | None = None + acceptance: tuple[JobResultAcceptance, ...] | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobResult": + links = payload.get("links") + acceptance = payload.get("acceptance") + return cls( + summary=payload["summary"], + markdown=payload.get("markdown"), + links=( + tuple(JobResultLink.from_wire(_mapping(row)) for row in _sequence(links)) + if links is not None + else None + ), + acceptance=( + tuple(JobResultAcceptance.from_wire(_mapping(row)) for row in _sequence(acceptance)) + if acceptance is not None + else None + ), + ) + + def to_wire(self) -> dict[str, Any]: + payload: dict[str, Any] = {"summary": self.summary} + if self.markdown is not None: + payload["markdown"] = self.markdown + if self.links is not None: + payload["links"] = [link.to_wire() for link in self.links] + if self.acceptance is not None: + payload["acceptance"] = [row.to_wire() for row in self.acceptance] + return payload + + +@dataclass(frozen=True, slots=True) +class JobDetail: + """``GET /v1/bridge/jobs/{jobId}`` — the feed row plus the state it cannot infer. + + ``status`` widens past the feed's three to include ``fulfilled``, ``failed`` + and ``timeout``. ``failure_kind`` is a plain string, not an enum: a daemon + may only *claim* ``executor_error``/``spec_rejected``/``cancelled``, but the + server writes kinds of its own (``executor_lost``, ``executor_stopped``, + ``no_executor``, ``stalled``, ``deadline``) that a reader must not choke on. + + ``discardedResult`` — the late-fulfil stash — is deliberately not on this + wire, so there is deliberately no field for it here. + """ + + job_id: str + claim_seq: int + attempt: int + max_attempts: int + status: str + spec: JobSpec + created_at: int + pending_expires_at: int + cancel_requested: bool + consumer_id: str | None = None + sticky_executor_id: str | None = None + claimed_by_executor_id: str | None = None + progress_note: str | None = None + executor_session_id: str | None = None + tmux_session: str | None = None + result: JobResult | None = None + error: str | None = None + failure_kind: str | None = None + claimed_at: int | None = None + started_at: int | None = None + last_progress_at: int | None = None + deadline_at: int | None = None + completed_at: int | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobDetail": + result = payload.get("result") + return cls( + job_id=payload["jobId"], + claim_seq=payload["claimSeq"], + attempt=payload["attempt"], + max_attempts=payload["maxAttempts"], + status=payload["status"], + spec=JobSpec.from_wire(_mapping(payload.get("spec"))), + created_at=payload["createdAt"], + pending_expires_at=payload["pendingExpiresAt"], + cancel_requested=bool(payload.get("cancelRequested")), + consumer_id=payload.get("consumerId"), + sticky_executor_id=payload.get("stickyExecutorId"), + claimed_by_executor_id=payload.get("claimedByExecutorId"), + progress_note=payload.get("progressNote"), + executor_session_id=payload.get("executorSessionId"), + tmux_session=payload.get("tmuxSession"), + result=JobResult.from_wire(_mapping(result)) if result is not None else None, + error=payload.get("error"), + failure_kind=payload.get("failureKind"), + claimed_at=payload.get("claimedAt"), + started_at=payload.get("startedAt"), + last_progress_at=payload.get("lastProgressAt"), + deadline_at=payload.get("deadlineAt"), + completed_at=payload.get("completedAt"), + ) + + +# ── Job lifecycle acknowledgements (§5.2) ──────────────────────────────────── + + +@dataclass(frozen=True, slots=True) +class JobClaim: + """``POST /v1/bridge/jobs/{id}/claim`` — the claim, the spec and the wall clock. + + Past ``deadline_at`` the sweep collects the row as ``deadline``; it is wall + clock (epoch ms), not a duration. + """ + + ok: bool + job_id: str + claim_seq: int + deadline_at: int + spec: JobSpec + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobClaim": + return cls( + ok=bool(payload.get("ok")), + job_id=payload["jobId"], + claim_seq=payload["claimSeq"], + deadline_at=payload["deadlineAt"], + spec=JobSpec.from_wire(_mapping(payload.get("spec"))), + ) + + +@dataclass(frozen=True, slots=True) +class JobStartAck: + """``POST /v1/bridge/jobs/{id}/start`` — running, or absorbed. + + ``cancel_requested`` is surfaced here as well as on progress, so a job + cancelled while merely *claimed* is observable **before** a session is + spawned. ``noop`` means the row was already terminal and nothing was + written; ``current_status`` is the status that absorbed the call. + """ + + ok: bool + status: str | None = None + cancel_requested: bool | None = None + noop: bool | None = None + current_status: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobStartAck": + return cls( + ok=bool(payload.get("ok")), + status=payload.get("status"), + cancel_requested=payload.get("cancelRequested"), + noop=payload.get("noop"), + current_status=payload.get("currentStatus"), + ) + + +@dataclass(frozen=True, slots=True) +class JobProgressAck: + """``POST /v1/bridge/jobs/{id}/progress`` — the beat, and any pending cancel. + + ``coalesced`` means the beat counted but the note was dropped: writes + coalesce to one a minute. It is a status line, not a log stream. + """ + + ok: bool + coalesced: bool | None = None + cancel_requested: bool | None = None + noop: bool | None = None + current_status: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobProgressAck": + return cls( + ok=bool(payload.get("ok")), + coalesced=payload.get("coalesced"), + cancel_requested=payload.get("cancelRequested"), + noop=payload.get("noop"), + current_status=payload.get("currentStatus"), + ) + + +@dataclass(frozen=True, slots=True) +class JobFulfillAck: + """``POST /v1/bridge/jobs/{id}/fulfill`` — fulfilled, or absorbed. + + A result arriving after a deadline already swept the row is absorbed rather + than errored (``noop`` with ``current_status``), and the payload is stashed + server-side so hours of real work stay recoverable. + """ + + ok: bool + noop: bool | None = None + current_status: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobFulfillAck": + return cls( + ok=bool(payload.get("ok")), + noop=payload.get("noop"), + current_status=payload.get("currentStatus"), + ) + + +@dataclass(frozen=True, slots=True) +class JobFailAck: + """``POST /v1/bridge/jobs/{id}/fail`` — where the row landed. + + ``status`` is ``pending`` when a retryable failure handed the row back to + **this** executor with a fresh generation (jobs are never migrated), or + ``failed`` once the attempt budget is spent. + """ + + ok: bool + status: str | None = None + attempt: int | None = None + noop: bool | None = None + current_status: str | None = None + + @classmethod + def from_wire(cls, payload: Mapping[str, Any]) -> "JobFailAck": + return cls( + ok=bool(payload.get("ok")), + status=payload.get("status"), + attempt=payload.get("attempt"), + noop=payload.get("noop"), + current_status=payload.get("currentStatus"), + ) diff --git a/alissa/src/main/alissa/sdk/api/client.py b/alissa/src/main/alissa/sdk/api/client.py new file mode 100644 index 0000000..df5844a --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/client.py @@ -0,0 +1,228 @@ +"""The SDK's HTTP/auth core: base URL, bearer token, JSON, error envelope. + +One transport for every ``alissa.sdk.api.*`` binding. A binding module (see +:mod:`alissa.sdk.api.bridge`) owns *paths and shapes*; it never opens a socket, +never reads the environment, and never decodes an error — those live here, once. + +Zero third-party dependencies, matching the rest of the SDK core: the transport +is :mod:`urllib.request` from the standard library. The one seam is +:class:`Transport` — inject your own to test bindings against recorded +responses without a network, which is exactly how this repo's unit tests run. + + from alissa.sdk.api import ApiClient + + client = ApiClient() # token from $ALISSA_API_TOKEN + client = ApiClient(token="alissa_…", base_url="https://api.alissa.app") +""" +from __future__ import annotations + +import json +import os +import urllib.error +import urllib.parse +import urllib.request +from dataclasses import dataclass, field +from typing import Any, Mapping, Protocol + +from ..version import version as _sdk_version +from .errors import ApiError, MissingTokenError, TransportError, error_from_envelope + +#: Where the Alissa REST API lives. Every path is versioned under ``/v1``. +DEFAULT_BASE_URL = "https://api.alissa.app" + +#: Environment variables the client falls back to, matching the `alissa` CLI. +ENV_TOKEN = "ALISSA_API_TOKEN" +ENV_BASE_URL = "ALISSA_BASE" + +USER_AGENT = f"alissa-python-sdk/{_sdk_version.value}" + +#: Seconds before a request is abandoned. Deliberately finite: a binding used +#: inside an observer loop must fail rather than hang. +DEFAULT_TIMEOUT = 30.0 + + +@dataclass(frozen=True, slots=True) +class HttpRequest: + """One outbound request, fully resolved — URL, headers and encoded body.""" + + method: str + url: str + headers: Mapping[str, str] = field(default_factory=dict) + body: bytes | None = None + + +@dataclass(frozen=True, slots=True) +class HttpResponse: + """One inbound response: the status and the raw body, nothing interpreted.""" + + status: int + body: bytes = b"" + headers: Mapping[str, str] = field(default_factory=dict) + + +class Transport(Protocol): + """How :class:`ApiClient` reaches the network. + + An error status is a **response**, not an exception: the API's failure + envelope only exists in the body of a 4xx/5xx, so a transport that raised + would throw the contract away. Raise :class:`~alissa.sdk.api.errors.TransportError` + only when there is no response at all. + """ + + def send(self, request: HttpRequest) -> HttpResponse: # pragma: no cover - protocol + ... + + +class UrllibTransport: + """The default transport — :mod:`urllib.request`, no third-party dependency.""" + + def __init__(self, timeout: float = DEFAULT_TIMEOUT) -> None: + self.timeout = timeout + + def send(self, request: HttpRequest) -> HttpResponse: + req = urllib.request.Request( + request.url, + data=request.body, + headers=dict(request.headers), + method=request.method, + ) + try: + with urllib.request.urlopen(req, timeout=self.timeout) as response: + return HttpResponse( + status=response.status, + body=response.read(), + headers=dict(response.headers.items()), + ) + except urllib.error.HTTPError as exc: + # A 4xx/5xx carries the API's error envelope in its body — hand it + # back as a response so the client can decode it into a typed error. + return HttpResponse( + status=exc.code, + body=exc.read(), + headers=dict(exc.headers.items()) if exc.headers else {}, + ) + except urllib.error.URLError as exc: + raise TransportError(f"{request.method} {request.url} failed: {exc.reason}") from exc + except OSError as exc: # socket timeouts and friends + raise TransportError(f"{request.method} {request.url} failed: {exc}") from exc + + +class ApiClient: + """Authenticated JSON access to the Alissa REST API. + + :param token: personal access token; falls back to ``$ALISSA_API_TOKEN``. + :param base_url: API root; falls back to ``$ALISSA_BASE``, then + :data:`DEFAULT_BASE_URL`. + :param timeout: seconds, applied by the default transport. + :param transport: inject to bypass the network (tests, recorded fixtures). + + The token is resolved lazily, at the first request rather than at + construction, so building a client in a module that is imported before the + environment is configured is not itself an error. + """ + + def __init__( + self, + token: str | None = None, + base_url: str | None = None, + *, + timeout: float = DEFAULT_TIMEOUT, + transport: Transport | None = None, + ) -> None: + self._token = token + self.base_url = (base_url or os.environ.get(ENV_BASE_URL) or DEFAULT_BASE_URL).rstrip("/") + self.transport: Transport = transport or UrllibTransport(timeout=timeout) + + @property + def token(self) -> str: + """The bearer token, resolved from the constructor or the environment.""" + token = self._token or os.environ.get(ENV_TOKEN) + if not token: + raise MissingTokenError( + f"No Alissa API token. Pass token=… or set ${ENV_TOKEN}." + ) + return token + + def url_for(self, path: str, query: Mapping[str, Any] | None = None) -> str: + """Absolute URL for ``path``, with ``None``-valued query keys dropped.""" + url = f"{self.base_url}/{path.lstrip('/')}" + pairs = _query_pairs(query) + return f"{url}?{urllib.parse.urlencode(pairs)}" if pairs else url + + def request( + self, + method: str, + path: str, + *, + query: Mapping[str, Any] | None = None, + body: Mapping[str, Any] | None = None, + ) -> Any: + """Send one request and return its decoded JSON body. + + ``body`` keys whose value is ``None`` are dropped rather than sent as + ``null`` — the API's zod schemas treat an absent optional and an + explicit ``null`` differently, and only the former means "not supplied". + + :raises ApiError: the API answered with its error envelope. + :raises TransportError: no response, or a response that is not JSON. + """ + encoded = None + headers = {"Accept": "application/json", "User-Agent": USER_AGENT} + if body is not None: + encoded = json.dumps(_without_none(body)).encode("utf-8") + headers["Content-Type"] = "application/json" + headers["Authorization"] = f"Bearer {self.token}" + + response = self.transport.send( + HttpRequest(method=method.upper(), url=self.url_for(path, query), headers=headers, body=encoded) + ) + return self._decode(method, path, response) + + def get(self, path: str, *, query: Mapping[str, Any] | None = None) -> Any: + return self.request("GET", path, query=query) + + def post(self, path: str, *, body: Mapping[str, Any] | None = None) -> Any: + return self.request("POST", path, body=body) + + def _decode(self, method: str, path: str, response: HttpResponse) -> Any: + try: + payload = json.loads(response.body) if response.body else None + except ValueError: + payload = None + + if 200 <= response.status < 300: + if payload is None: + raise TransportError( + f"{method.upper()} {path} returned {response.status} with a non-JSON body." + ) + return payload + + if isinstance(payload, dict): + raise error_from_envelope(response.status, payload) + raise ApiError( + f"{method.upper()} {path} failed with HTTP {response.status}.", + code=f"HTTP_{response.status}", + http_status=response.status, + ) + + +def _query_pairs(query: Mapping[str, Any] | None) -> list[tuple[str, str]]: + """Flatten a query mapping, dropping ``None`` and repeating list values.""" + pairs: list[tuple[str, str]] = [] + for key, value in (query or {}).items(): + if value is None: + continue + if isinstance(value, (list, tuple)): + pairs.extend((key, str(item)) for item in value) + else: + pairs.append((key, str(value))) + return pairs + + +def _without_none(body: Mapping[str, Any]) -> dict[str, Any]: + return {key: value for key, value in body.items() if value is not None} + + +def encode_path(*segments: str) -> str: + """Percent-encode path segments so an id can never inject a path.""" + return "/".join(urllib.parse.quote(str(segment), safe="") for segment in segments) diff --git a/alissa/src/main/alissa/sdk/api/errors.py b/alissa/src/main/alissa/sdk/api/errors.py new file mode 100644 index 0000000..6bc9a07 --- /dev/null +++ b/alissa/src/main/alissa/sdk/api/errors.py @@ -0,0 +1,228 @@ +"""Typed errors for the Alissa REST API. + +The API answers every failure with the same JSON envelope — a machine-readable +``error`` code, a human ``message``, and (for a handful of codes) extra +structured detail:: + + { "error": "STALE_CONSUMER", "message": "…", "status": "running" } + +This module turns that envelope into an exception *class per code*, so callers +branch on a type or on :attr:`ApiError.code` and never on the prose of +``message`` — the message is free text the server may reword at any time, the +code is the contract. + +The Local Bridge queue-mode codes (``docs/design/local-bridge-queue-mode.md`` +§5.3) are the reason this exists. Its four 409s are all *retryable after a +re-read* rather than terminal, which is a materially different instruction than +"this failed" — they share :class:`RetryAfterReadError` so a caller can say +"re-poll and try again" in one ``except`` clause:: + + try: + claim = bridge.claim_job(job_id, executor_id=…, consumer_id=…, claim_seq=…) + except StaleConsumerError: + return # another attempt owns this row now + except RetryAfterReadError: + jobs = bridge.list_jobs(executor_id=…) # re-read, do not spin + +An unmapped code is **not** an error to this layer: it becomes a plain +:class:`ApiError` carrying the code verbatim, so a server that grows a new code +degrades to "branchable by string" rather than to a crash. +""" +from __future__ import annotations + +from typing import Any, Mapping + + +class AlissaError(Exception): + """Base class for every error this SDK raises.""" + + +class MissingTokenError(AlissaError): + """No API token was passed and none was found in the environment.""" + + +class TransportError(AlissaError): + """The request never produced a usable JSON response. + + Raised for connection failures, timeouts, and bodies that are not JSON — + i.e. everything that is *not* the API telling us something in its envelope. + """ + + +class ApiError(AlissaError): + """An error the API reported in its ``{ "error", "message" }`` envelope. + + :attr:`code` is the branchable value. :attr:`http_status` is the transport + status that carried it, and :attr:`detail` holds any extra fields the code + is documented to carry (kept in the wire's own spelling, so nothing is lost + for a code this SDK does not model yet). + """ + + #: The envelope ``error`` code this subclass is registered for. + code: str = "UNKNOWN" + + def __init__( + self, + message: str, + *, + code: str | None = None, + http_status: int = 0, + detail: Mapping[str, Any] | None = None, + ) -> None: + super().__init__(message) + self.message = message + if code is not None: + self.code = code + self.http_status = http_status + self.detail: dict[str, Any] = dict(detail or {}) + + def __str__(self) -> str: + return f"{self.code}: {self.message}" + + +class ValidationError(ApiError): + """``VALIDATION_ERROR`` (400) — body shape, slug shape or a length cap.""" + + code = "VALIDATION_ERROR" + + +class UnauthorizedError(ApiError): + """``UNAUTHORIZED`` (401) — missing, expired or revoked token.""" + + code = "UNAUTHORIZED" + + +class ForbiddenError(ApiError): + """``FORBIDDEN`` (403) — plan entitlement; queue mode is Ultra-gated.""" + + code = "FORBIDDEN" + + +class NotFoundError(ApiError): + """``NOT_FOUND`` (404) — no such row *for this user*. + + Also the answer for another user's row: absence and inaccessibility are + deliberately indistinguishable. + """ + + code = "NOT_FOUND" + + +class PreconditionFailedError(ApiError): + """``PRECONDITION_FAILED`` (412) — executor closed, or the executor cap is reached. + + Note the status: §5.3 pins this code at **412** on the bridge surface, where + the rest of the Alissa API answers 409. Branch on the code, not the status. + """ + + code = "PRECONDITION_FAILED" + + +class CapacityExceededError(ApiError): + """``CAPACITY_EXCEEDED`` (429) — the executor is at ``maxConcurrentJobs``.""" + + code = "CAPACITY_EXCEEDED" + + @property + def held(self) -> int | None: + """Jobs this executor currently holds, when the server reported it.""" + value = self.detail.get("held") + return value if isinstance(value, int) else None + + @property + def cap(self) -> int | None: + """The executor's concurrency cap, when the server reported it.""" + value = self.detail.get("cap") + return value if isinstance(value, int) else None + + +class RetryAfterReadError(ApiError): + """Base of §5.3's four 409s: re-read the row and retry, do not spin. + + Every one of them carries the ``status`` actually observed on the row, which + is what makes the re-read cheap — see :attr:`observed_status`. + """ + + @property + def observed_status(self) -> str | None: + """The row status the server observed, when it reported one.""" + value = self.detail.get("status") + return value if isinstance(value, str) else None + + +class ConflictError(RetryAfterReadError): + """``CONFLICT`` (409) — the job is not in the state this call requires.""" + + code = "CONFLICT" + + +class CasConflictError(RetryAfterReadError): + """``CAS_CONFLICT`` (409) — the ``claimSeq`` sent did not match the row.""" + + code = "CAS_CONFLICT" + + @property + def current_claim_seq(self) -> int | None: + """The generation the row is actually on — claim against this, or re-poll.""" + value = self.detail.get("currentClaimSeq") + return value if isinstance(value, int) else None + + +class StickyViolationError(RetryAfterReadError): + """``STICKY_VIOLATION`` (409) — this job is sticky to a different executor.""" + + code = "STICKY_VIOLATION" + + @property + def sticky_executor_id(self) -> str | None: + """The executor the job is pinned to.""" + value = self.detail.get("stickyExecutorId") + return value if isinstance(value, str) else None + + +class StaleConsumerError(RetryAfterReadError): + """``STALE_CONSUMER`` (409) — the ``consumerId`` is not the row's current attempt. + + The nonce you are echoing belongs to an attempt that has been superseded: + a daemon that hung and woke up must **not** write over the attempt that + replaced it. Stop working this job — do not retry with the same nonce. + """ + + code = "STALE_CONSUMER" + + +#: Envelope code → exception class. Codes absent here become a plain +#: :class:`ApiError` carrying the code, so a new server-side code is degraded +#: to "branchable by string" rather than swallowed or crashed on. +ERROR_CLASSES: dict[str, type[ApiError]] = { + cls.code: cls + for cls in ( + ValidationError, + UnauthorizedError, + ForbiddenError, + NotFoundError, + PreconditionFailedError, + CapacityExceededError, + ConflictError, + CasConflictError, + StickyViolationError, + StaleConsumerError, + ) +} + + +def error_from_envelope(http_status: int, payload: Mapping[str, Any]) -> ApiError: + """Build the typed error for one ``{ "error", "message", … }`` envelope. + + Every key other than ``error`` and ``message`` is kept in :attr:`ApiError.detail` + under its wire spelling — the server whitelists detail per code, and this + layer must not decide which of those keys matter. + """ + code = payload.get("error") + code = code if isinstance(code, str) and code else f"HTTP_{http_status}" + message = payload.get("message") + message = message if isinstance(message, str) else "Request failed." + detail = {key: value for key, value in payload.items() if key not in ("error", "message")} + + cls = ERROR_CLASSES.get(code, ApiError) + return cls(message, code=code, http_status=http_status, detail=detail) diff --git a/alissa/src/main/alissa/sdk/version b/alissa/src/main/alissa/sdk/version index 6c6aa7c..341cf11 100644 --- a/alissa/src/main/alissa/sdk/version +++ b/alissa/src/main/alissa/sdk/version @@ -1 +1 @@ -0.1.0 \ No newline at end of file +0.2.0 \ No newline at end of file diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/conftest.py b/alissa/src/test/test_alissa/test_sdk/test_api/conftest.py new file mode 100644 index 0000000..78a2179 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/conftest.py @@ -0,0 +1,94 @@ +"""Offline plumbing for the API-binding tests. + +Every test in this directory runs against **recorded response bodies** in +``fixtures/`` — the JSON these endpoints actually answer with — replayed through +an injected transport. Nothing here opens a socket, and nothing reads a real +token: a binding whose tests needed the network could only be run by someone +holding an Ultra plan and a live executor. + +:class:`RecordedTransport` is also the assertion surface for the *request* half +of each binding. It records what was sent, so a test can check the method, the +URL, the query string and the JSON body — which is where "no invented fields" +is actually enforced. +""" +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest + +from alissa.sdk.api import ApiClient, BridgeClient, HttpRequest, HttpResponse + +FIXTURES = Path(__file__).parent / "fixtures" + +TEST_TOKEN = "alissa_test_token" +TEST_BASE_URL = "https://api.test.invalid" + + +def load_fixture(name: str) -> Any: + """Read one recorded response body by file name (``jobs_feed.json``).""" + with open(FIXTURES / name, "r") as handle: + return json.load(handle) + + +class RecordedTransport: + """A transport that replays queued responses and records what was sent. + + Queue responses with :meth:`reply` (a 2xx fixture) or :meth:`fail` (an error + envelope with its documented status). Responses are served in FIFO order; + running out is a test bug, not a runtime condition, so it raises. + """ + + def __init__(self) -> None: + self.requests: list[HttpRequest] = [] + self._responses: list[HttpResponse] = [] + + def reply(self, fixture: str, status: int = 200) -> "RecordedTransport": + return self.respond(status, load_fixture(fixture)) + + def fail(self, fixture: str, status: int) -> "RecordedTransport": + return self.respond(status, load_fixture(fixture)) + + def respond(self, status: int, payload: Any) -> "RecordedTransport": + self._responses.append(HttpResponse(status=status, body=json.dumps(payload).encode("utf-8"))) + return self + + def respond_raw(self, status: int, body: bytes) -> "RecordedTransport": + self._responses.append(HttpResponse(status=status, body=body)) + return self + + def send(self, request: HttpRequest) -> HttpResponse: + self.requests.append(request) + if not self._responses: + raise AssertionError(f"No queued response for {request.method} {request.url}") + return self._responses.pop(0) + + # ── assertion helpers ──────────────────────────────────────────────────── + + @property + def last(self) -> HttpRequest: + assert self.requests, "no request was sent" + return self.requests[-1] + + @property + def last_body(self) -> Any: + """The JSON body of the last request, or ``None`` when it carried none.""" + body = self.last.body + return json.loads(body) if body else None + + +@pytest.fixture +def transport() -> RecordedTransport: + return RecordedTransport() + + +@pytest.fixture +def client(transport: RecordedTransport) -> ApiClient: + return ApiClient(token=TEST_TOKEN, base_url=TEST_BASE_URL, transport=transport) + + +@pytest.fixture +def bridge(client: ApiClient) -> BridgeClient: + return BridgeClient(client) diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_capacity_exceeded.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_capacity_exceeded.json new file mode 100644 index 0000000..897b28d --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_capacity_exceeded.json @@ -0,0 +1,6 @@ +{ + "error": "CAPACITY_EXCEEDED", + "message": "Executor is at maxConcurrentJobs.", + "held": 2, + "cap": 2 +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_cas_conflict.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_cas_conflict.json new file mode 100644 index 0000000..0e22fbf --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_cas_conflict.json @@ -0,0 +1,6 @@ +{ + "error": "CAS_CONFLICT", + "message": "claimSeq mismatch.", + "currentClaimSeq": 7, + "status": "pending" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_conflict.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_conflict.json new file mode 100644 index 0000000..62631af --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_conflict.json @@ -0,0 +1,5 @@ +{ + "error": "CONFLICT", + "message": "Job is not pending.", + "status": "running" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_forbidden.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_forbidden.json new file mode 100644 index 0000000..4bf3005 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_forbidden.json @@ -0,0 +1,4 @@ +{ + "error": "FORBIDDEN", + "message": "Queue mode is an Ultra-plan feature." +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_not_found.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_not_found.json new file mode 100644 index 0000000..cf0540b --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_not_found.json @@ -0,0 +1,4 @@ +{ + "error": "NOT_FOUND", + "message": "No such job for this token's user." +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_precondition_failed.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_precondition_failed.json new file mode 100644 index 0000000..143feca --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_precondition_failed.json @@ -0,0 +1,4 @@ +{ + "error": "PRECONDITION_FAILED", + "message": "Executor is closed; re-register before claiming." +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_stale_consumer.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_stale_consumer.json new file mode 100644 index 0000000..40e4ea2 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_stale_consumer.json @@ -0,0 +1,5 @@ +{ + "error": "STALE_CONSUMER", + "message": "consumerId does not match the current attempt.", + "status": "running" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_sticky_violation.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_sticky_violation.json new file mode 100644 index 0000000..e78e599 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_sticky_violation.json @@ -0,0 +1,6 @@ +{ + "error": "STICKY_VIOLATION", + "message": "Job is sticky to another executor.", + "stickyExecutorId": "mac-mini", + "status": "pending" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unauthorized.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unauthorized.json new file mode 100644 index 0000000..4679092 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unauthorized.json @@ -0,0 +1,4 @@ +{ + "error": "UNAUTHORIZED", + "message": "Missing or invalid token." +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unknown_code.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unknown_code.json new file mode 100644 index 0000000..c717f59 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_unknown_code.json @@ -0,0 +1,5 @@ +{ + "error": "SOME_FUTURE_CODE", + "message": "Not modelled yet.", + "hint": "re-read" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_validation.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_validation.json new file mode 100644 index 0000000..e12a0a3 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/error_validation.json @@ -0,0 +1,4 @@ +{ + "error": "VALIDATION_ERROR", + "message": "limit must be an integer between 1 and 50." +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat.json new file mode 100644 index 0000000..e7e3f32 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat.json @@ -0,0 +1,4 @@ +{ + "executorId": "macbook-pro-work", + "beat": "coalesced" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat_missing.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat_missing.json new file mode 100644 index 0000000..0cfd76e --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_heartbeat_missing.json @@ -0,0 +1,4 @@ +{ + "executorId": "macbook-pro-work", + "beat": "missing" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_stop.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_stop.json new file mode 100644 index 0000000..594b267 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executor_stop.json @@ -0,0 +1,6 @@ +{ + "executorId": "macbook-pro-work", + "found": true, + "alreadyEnded": false, + "releasedJobs": 3 +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_list.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_list.json new file mode 100644 index 0000000..d566160 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_list.json @@ -0,0 +1,40 @@ +{ + "executors": [ + { + "executorId": "macbook-pro-work", + "kind": "alissa-code", + "label": "MacBook Pro (work)", + "hostname": "mbp-work.local", + "cliVersion": "1.4.2", + "pollSeconds": 15, + "workerName": "worker-mbp", + "capabilities": { + "workspaceRoots": [ + "/workspace" + ], + "maxConcurrentJobs": 2, + "handoffs": [ + "claude", + "codex" + ], + "tags": [ + "gpu" + ] + }, + "startedAt": 1786500000000, + "lastHeartbeatAt": 1786500600000, + "status": "active" + }, + { + "executorId": "retired-mini", + "kind": "alissa-code", + "label": "Mac mini", + "hostname": "mini.local", + "startedAt": 1786400000000, + "lastHeartbeatAt": 1786400900000, + "endedAt": 1786401000000, + "endReason": "executor_stopped", + "status": "executor_stopped" + } + ] +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_register.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_register.json new file mode 100644 index 0000000..adf418c --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/executors_register.json @@ -0,0 +1,18 @@ +{ + "executorId": "macbook-pro-work", + "tookOver": true, + "fingerprintChanged": false, + "resumed": [ + { + "jobId": "j57bridge0001", + "status": "running", + "consumerId": "consumer-a1", + "attempt": 2 + }, + { + "jobId": "j57bridge0002", + "status": "pending", + "attempt": 1 + } + ] +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_claim.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_claim.json new file mode 100644 index 0000000..e424cf4 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_claim.json @@ -0,0 +1,42 @@ +{ + "ok": true, + "jobId": "j57bridge0002", + "claimSeq": 5, + "deadlineAt": 1786507200000, + "spec": { + "title": "Wire the queue-mode bindings", + "prompt": "Implement the typed bindings described in the linked design doc.", + "deliverable": { + "kind": "pull_request", + "description": "A draft PR against main." + }, + "acceptance": [ + { + "id": "surface", + "description": "All eleven endpoints bound.", + "type": "manual" + }, + { + "id": "tests", + "description": "Unit tests pass offline.", + "type": "automated" + } + ], + "references": [ + { + "kind": "task", + "ref": "TASK-1115046778", + "label": "Origin task" + }, + { + "kind": "repo", + "ref": "fahera-mx/alissa-python-sdk" + } + ], + "workspaceRoot": "/workspace", + "handoff": "claude", + "env": [ + "ALISSA_API_TOKEN" + ] + } +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail.json new file mode 100644 index 0000000..1adf4a9 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail.json @@ -0,0 +1,80 @@ +{ + "job": { + "jobId": "j57bridge0001", + "claimSeq": 9, + "attempt": 2, + "maxAttempts": 3, + "status": "fulfilled", + "spec": { + "title": "Wire the queue-mode bindings", + "prompt": "Implement the typed bindings described in the linked design doc.", + "deliverable": { + "kind": "pull_request", + "description": "A draft PR against main." + }, + "acceptance": [ + { + "id": "surface", + "description": "All eleven endpoints bound.", + "type": "manual" + }, + { + "id": "tests", + "description": "Unit tests pass offline.", + "type": "automated" + } + ], + "references": [ + { + "kind": "task", + "ref": "TASK-1115046778", + "label": "Origin task" + }, + { + "kind": "repo", + "ref": "fahera-mx/alissa-python-sdk" + } + ], + "workspaceRoot": "/workspace", + "handoff": "claude", + "env": [ + "ALISSA_API_TOKEN" + ] + }, + "createdAt": 1786499000000, + "pendingExpiresAt": 1786502600000, + "consumerId": "consumer-a1", + "stickyExecutorId": "macbook-pro-work", + "claimedByExecutorId": "macbook-pro-work", + "cancelRequested": false, + "progressNote": "running the test suite", + "executorSessionId": "cs57session01", + "tmuxSession": "ali-bridge-j57bridge0001", + "result": { + "summary": "Opened the draft PR.", + "markdown": "## What changed\n\nEverything the spec asked for.", + "links": [ + { + "label": "PR", + "url": "https://github.com/fahera-mx/alissa-python-sdk/pull/3" + } + ], + "acceptance": [ + { + "id": "surface", + "met": true, + "note": "eleven endpoints" + }, + { + "id": "tests", + "met": true + } + ] + }, + "claimedAt": 1786499100000, + "startedAt": 1786499200000, + "lastProgressAt": 1786500100000, + "deadlineAt": 1786506800000, + "completedAt": 1786500200000 + } +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail_failed.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail_failed.json new file mode 100644 index 0000000..d5b0966 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_detail_failed.json @@ -0,0 +1,24 @@ +{ + "job": { + "jobId": "j57bridge0003", + "claimSeq": 12, + "attempt": 3, + "maxAttempts": 3, + "status": "failed", + "spec": { + "title": "A job the sweep collected", + "prompt": "Do the thing.", + "deliverable": { + "kind": "patch", + "description": "A patch file." + }, + "acceptance": [] + }, + "createdAt": 1786490000000, + "pendingExpiresAt": 1786493600000, + "cancelRequested": true, + "error": "the executor stopped answering", + "failureKind": "executor_lost", + "completedAt": 1786494000000 + } +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_retry.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_retry.json new file mode 100644 index 0000000..cad522f --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_retry.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "status": "pending", + "attempt": 3 +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_terminal.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_terminal.json new file mode 100644 index 0000000..787ca33 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fail_terminal.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "status": "failed", + "attempt": 3 +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill.json new file mode 100644 index 0000000..0287aed --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill.json @@ -0,0 +1,3 @@ +{ + "ok": true +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill_noop.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill_noop.json new file mode 100644 index 0000000..daba5e8 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_fulfill_noop.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "noop": true, + "currentStatus": "timeout" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress.json new file mode 100644 index 0000000..9946c95 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "coalesced": false, + "cancelRequested": false +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress_coalesced.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress_coalesced.json new file mode 100644 index 0000000..1bdc468 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_progress_coalesced.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "coalesced": true, + "cancelRequested": true +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start.json new file mode 100644 index 0000000..68f603e --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "status": "running", + "cancelRequested": false +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_cancelled.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_cancelled.json new file mode 100644 index 0000000..5fbcaea --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_cancelled.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "status": "running", + "cancelRequested": true +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_noop.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_noop.json new file mode 100644 index 0000000..daba5e8 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/job_start_noop.json @@ -0,0 +1,5 @@ +{ + "ok": true, + "noop": true, + "currentStatus": "timeout" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/jobs_feed.json b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/jobs_feed.json new file mode 100644 index 0000000..5e04b45 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/fixtures/jobs_feed.json @@ -0,0 +1,69 @@ +{ + "jobs": [ + { + "jobId": "j57bridge0002", + "claimSeq": 4, + "attempt": 1, + "maxAttempts": 3, + "status": "pending", + "spec": { + "title": "Wire the queue-mode bindings", + "prompt": "Implement the typed bindings described in the linked design doc.", + "deliverable": { + "kind": "pull_request", + "description": "A draft PR against main." + }, + "acceptance": [ + { + "id": "surface", + "description": "All eleven endpoints bound.", + "type": "manual" + }, + { + "id": "tests", + "description": "Unit tests pass offline.", + "type": "automated" + } + ], + "references": [ + { + "kind": "task", + "ref": "TASK-1115046778", + "label": "Origin task" + }, + { + "kind": "repo", + "ref": "fahera-mx/alissa-python-sdk" + } + ], + "workspaceRoot": "/workspace", + "handoff": "claude", + "env": [ + "ALISSA_API_TOKEN" + ] + }, + "createdAt": 1786500000000, + "pendingExpiresAt": 1786503600000 + }, + { + "jobId": "j57bridge0001", + "claimSeq": 9, + "attempt": 2, + "maxAttempts": 3, + "status": "running", + "spec": { + "title": "Tail a job", + "prompt": "Report the queue's state.", + "deliverable": { + "kind": "report", + "description": "A short status report." + }, + "acceptance": [] + }, + "createdAt": 1786499000000, + "pendingExpiresAt": 1786502600000, + "consumerId": "consumer-a1" + } + ], + "beat": "touched" +} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py new file mode 100644 index 0000000..237434c --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py @@ -0,0 +1,115 @@ +"""The shared client/auth core: token resolution, URL building, JSON, decoding. + +These lock the behaviours every binding inherits, so a binding's own tests do +not have to re-assert them: bearer auth on every call, ``None``-valued fields +dropped rather than sent as ``null``, and a non-JSON response failing as a +transport problem rather than as an API error. +""" +from __future__ import annotations + +import pytest + +from alissa.sdk.api import ApiClient, DEFAULT_BASE_URL, MissingTokenError, TransportError +from alissa.sdk.api.client import ENV_BASE_URL, ENV_TOKEN, USER_AGENT, encode_path +from alissa.sdk.api.errors import ApiError + +from conftest import TEST_BASE_URL, TEST_TOKEN, RecordedTransport + + +def test_defaults_to_the_public_api(monkeypatch): + monkeypatch.delenv(ENV_BASE_URL, raising=False) + assert ApiClient(token="t").base_url == DEFAULT_BASE_URL + assert DEFAULT_BASE_URL == "https://api.alissa.app" + + +def test_base_url_comes_from_the_environment_and_loses_its_trailing_slash(monkeypatch): + monkeypatch.setenv(ENV_BASE_URL, "https://api.staging.invalid/") + assert ApiClient(token="t").base_url == "https://api.staging.invalid" + + +def test_token_falls_back_to_the_environment(monkeypatch): + monkeypatch.setenv(ENV_TOKEN, "alissa_from_env") + assert ApiClient().token == "alissa_from_env" + + +def test_explicit_token_wins_over_the_environment(monkeypatch): + monkeypatch.setenv(ENV_TOKEN, "alissa_from_env") + assert ApiClient(token="alissa_explicit").token == "alissa_explicit" + + +def test_missing_token_is_typed_and_raised_lazily(monkeypatch): + monkeypatch.delenv(ENV_TOKEN, raising=False) + client = ApiClient() # constructing is fine — only using it is not + with pytest.raises(MissingTokenError): + _ = client.token + + +def test_url_building_drops_none_and_repeats_lists(): + client = ApiClient(token=TEST_TOKEN, base_url=TEST_BASE_URL) + + assert client.url_for("/v1/ping") == f"{TEST_BASE_URL}/v1/ping" + assert client.url_for("v1/ping") == f"{TEST_BASE_URL}/v1/ping" + assert client.url_for("/v1/x", {"a": 1, "b": None}) == f"{TEST_BASE_URL}/v1/x?a=1" + assert client.url_for("/v1/x", {"s": ["a", "b"]}) == f"{TEST_BASE_URL}/v1/x?s=a&s=b" + + +def test_path_segments_are_percent_encoded(): + # An id is user-controlled data; it must never be able to inject a path. + assert encode_path("a/b") == "a%2Fb" + assert encode_path("a", "b c") == "a/b%20c" + + +def test_every_request_carries_bearer_auth_and_the_sdk_user_agent(client, transport): + transport.respond(200, {"ok": True}) + + client.get("/v1/ping") + + assert transport.last.headers["Authorization"] == f"Bearer {TEST_TOKEN}" + assert transport.last.headers["Accept"] == "application/json" + assert transport.last.headers["User-Agent"] == USER_AGENT + assert USER_AGENT.startswith("alissa-python-sdk/") + # A GET carries no body, so it must not claim a content type. + assert "Content-Type" not in transport.last.headers + assert transport.last.body is None + + +def test_none_valued_body_fields_are_omitted_not_sent_as_null(client, transport): + transport.respond(200, {"ok": True}) + + client.post("/v1/x", body={"kept": "yes", "dropped": None}) + + assert transport.last_body == {"kept": "yes"} + assert transport.last.headers["Content-Type"] == "application/json" + + +def test_error_envelope_becomes_a_typed_error(client, transport): + transport.fail("error_not_found.json", status=404) + + with pytest.raises(ApiError) as caught: + client.get("/v1/bridge/jobs/nope") + + assert caught.value.code == "NOT_FOUND" + assert caught.value.http_status == 404 + + +def test_non_json_error_body_still_raises_a_coded_api_error(client, transport): + transport.respond_raw(502, b"bad gateway") + + with pytest.raises(ApiError) as caught: + client.get("/v1/ping") + + assert caught.value.code == "HTTP_502" + assert caught.value.http_status == 502 + + +def test_non_json_success_body_is_a_transport_error(client, transport): + transport.respond_raw(200, b"not json at all") + + with pytest.raises(TransportError): + client.get("/v1/ping") + + +def test_transport_is_the_only_seam_the_tests_need(): + # The default client builds its own transport; ours is injected. This is the + # property that keeps every test in this directory offline. + assert isinstance(ApiClient(token="t", transport=RecordedTransport()).transport, RecordedTransport) diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_errors.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_errors.py new file mode 100644 index 0000000..dff9a57 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_errors.py @@ -0,0 +1,163 @@ +"""§5.3's error codes, raised through the bindings that answer with them. + +The point of this file is the promise in the issue: a caller branches on a +**code** (or its class), never on the prose of ``message``. One test per code +family, each driven by the envelope the API actually sends — including the +structured detail two of them are documented to carry. +""" +from __future__ import annotations + +import pytest + +from alissa.sdk.api import ( + ApiError, + CapacityExceededError, + CasConflictError, + ConflictError, + ForbiddenError, + NotFoundError, + PreconditionFailedError, + RetryAfterReadError, + StaleConsumerError, + StickyViolationError, + UnauthorizedError, + ValidationError, +) +from alissa.sdk.api.bridge import JobResult + + +def test_stale_consumer_is_typed_branchable_and_carries_the_observed_status(bridge, transport): + # The nonce being echoed belongs to a superseded attempt: this caller must + # stop, not retry. That is why it is its own class. + transport.fail("error_stale_consumer.json", status=409) + + with pytest.raises(StaleConsumerError) as caught: + bridge.progress_job("j57bridge0001", consumer_id="stale-nonce", note="still here") + + error = caught.value + assert error.code == "STALE_CONSUMER" + assert error.http_status == 409 + assert error.observed_status == "running" + assert isinstance(error, RetryAfterReadError) + assert str(error).startswith("STALE_CONSUMER: ") + + +def test_cas_conflict_reports_the_generation_to_re_read(bridge, transport): + transport.fail("error_cas_conflict.json", status=409) + + with pytest.raises(CasConflictError) as caught: + bridge.claim_job("j", executor_id="mbp", consumer_id="c", claim_seq=4) + + assert caught.value.code == "CAS_CONFLICT" + assert caught.value.current_claim_seq == 7 + assert caught.value.observed_status == "pending" + + +def test_conflict_reports_the_status_actually_observed(bridge, transport): + transport.fail("error_conflict.json", status=409) + + with pytest.raises(ConflictError) as caught: + bridge.claim_job("j", executor_id="mbp", consumer_id="c", claim_seq=4) + + assert caught.value.code == "CONFLICT" + assert caught.value.observed_status == "running" + + +def test_sticky_violation_names_the_executor_the_job_is_pinned_to(bridge, transport): + transport.fail("error_sticky_violation.json", status=409) + + with pytest.raises(StickyViolationError) as caught: + bridge.claim_job("j", executor_id="mbp", consumer_id="c", claim_seq=4) + + assert caught.value.sticky_executor_id == "mac-mini" + + +def test_every_409_shares_one_retry_after_read_base(bridge, transport): + # The whole family means "re-read the row and retry", not "give up" — a + # caller must be able to say that once. + for fixture in ( + "error_conflict.json", + "error_cas_conflict.json", + "error_sticky_violation.json", + "error_stale_consumer.json", + ): + transport.fail(fixture, status=409) + with pytest.raises(RetryAfterReadError): + bridge.start_job("j", consumer_id="c") + + +def test_capacity_exceeded_reports_held_and_cap(bridge, transport): + transport.fail("error_capacity_exceeded.json", status=429) + + with pytest.raises(CapacityExceededError) as caught: + bridge.claim_job("j", executor_id="mbp", consumer_id="c", claim_seq=4) + + assert caught.value.http_status == 429 + assert (caught.value.held, caught.value.cap) == (2, 2) + # Not a re-read: this executor is full, and re-polling changes nothing. + assert not isinstance(caught.value, RetryAfterReadError) + + +def test_precondition_failed_is_412_on_this_surface(bridge, transport): + # §5.3 pins it at 412 where the rest of the Alissa API answers 409 — which + # is exactly why callers must branch on the code, not the status. + transport.fail("error_precondition_failed.json", status=412) + + with pytest.raises(PreconditionFailedError) as caught: + bridge.claim_job("j", executor_id="mbp", consumer_id="c", claim_seq=4) + + assert caught.value.code == "PRECONDITION_FAILED" + assert caught.value.http_status == 412 + + +def test_forbidden_is_the_ultra_plan_gate(bridge, transport): + transport.fail("error_forbidden.json", status=403) + + with pytest.raises(ForbiddenError): + bridge.register_executor("mbp", label="L", hostname="h", fingerprint="f") + + +def test_unauthorized_covers_a_bad_or_revoked_token(bridge, transport): + transport.fail("error_unauthorized.json", status=401) + + with pytest.raises(UnauthorizedError): + bridge.list_executors() + + +def test_not_found_is_also_the_answer_for_another_users_row(bridge, transport): + transport.fail("error_not_found.json", status=404) + + with pytest.raises(NotFoundError): + bridge.get_job("someone-elses-job") + + +def test_validation_error_is_raised_for_a_malformed_query(bridge, transport): + transport.fail("error_validation.json", status=400) + + with pytest.raises(ValidationError) as caught: + bridge.list_jobs(executor_id="mbp", limit=999) + + assert caught.value.code == "VALIDATION_ERROR" + + +def test_fulfill_and_fail_raise_the_same_typed_errors(bridge, transport): + transport.fail("error_stale_consumer.json", status=409) + with pytest.raises(StaleConsumerError): + bridge.fulfill_job("j", consumer_id="stale", result=JobResult(summary="Done.")) + + transport.fail("error_stale_consumer.json", status=409) + with pytest.raises(StaleConsumerError): + bridge.fail_job("j", consumer_id="stale", error="boom", retryable=True) + + +def test_an_unmodelled_code_survives_as_a_plain_api_error(bridge, transport): + # A code this SDK has never heard of must stay branchable by string rather + # than crash or be swallowed. + transport.fail("error_unknown_code.json", status=409) + + with pytest.raises(ApiError) as caught: + bridge.stop_executor("mbp") + + assert type(caught.value) is ApiError + assert caught.value.code == "SOME_FUTURE_CODE" + assert caught.value.detail == {"hint": "re-read"} diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_executors.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_executors.py new file mode 100644 index 0000000..8c1077b --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_executors.py @@ -0,0 +1,176 @@ +"""The four executor endpoints of ``/v1/bridge`` (§5.1), against recorded responses. + +Each test asserts both halves of the binding: the request it puts on the wire +(method, path, body — where "no invented fields" is enforced) and the typed +structure it hands back. +""" +from __future__ import annotations + +from alissa.sdk.api.bridge import EXECUTOR_KIND, ExecutorCapabilities + +from conftest import TEST_BASE_URL + + +def test_register_sends_the_declared_fields_and_returns_resumed_jobs(bridge, transport): + transport.reply("executors_register.json") + + registration = bridge.register_executor( + "macbook-pro-work", + label="MacBook Pro (work)", + hostname="mbp-work.local", + fingerprint="fp-2f9c", + cli_version="1.4.2", + poll_seconds=15, + worker_name="worker-mbp", + capabilities=ExecutorCapabilities( + workspace_roots=("/workspace",), + max_concurrent_jobs=2, + handoffs=("claude",), + ), + ) + + assert transport.last.method == "POST" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/executors" + assert transport.last_body == { + "executorId": "macbook-pro-work", + "kind": "alissa-code", + "label": "MacBook Pro (work)", + "hostname": "mbp-work.local", + "fingerprint": "fp-2f9c", + "cliVersion": "1.4.2", + "pollSeconds": 15, + "workerName": "worker-mbp", + "capabilities": { + "workspaceRoots": ["/workspace"], + "maxConcurrentJobs": 2, + "handoffs": ["claude"], + }, + } + + assert registration.executor_id == "macbook-pro-work" + assert registration.took_over is True + assert registration.fingerprint_changed is False + assert [job.job_id for job in registration.resumed] == ["j57bridge0001", "j57bridge0002"] + # A pending resumed row has nothing holding it — the field is absent, not empty. + assert registration.resumed[0].consumer_id == "consumer-a1" + assert registration.resumed[1].consumer_id is None + assert registration.resumed[1].attempt == 1 + + +def test_register_omits_every_unset_optional(bridge, transport): + transport.reply("executors_register.json") + + bridge.register_executor( + "macbook-pro-work", + label="MacBook Pro (work)", + hostname="mbp-work.local", + fingerprint="fp-2f9c", + ) + + assert transport.last_body == { + "executorId": "macbook-pro-work", + "kind": EXECUTOR_KIND, + "label": "MacBook Pro (work)", + "hostname": "mbp-work.local", + "fingerprint": "fp-2f9c", + } + + +def test_capabilities_absence_and_emptiness_stay_distinguishable(bridge, transport): + # Absent workspaceRoots means "accepts any workspace"; an empty list would + # mean "accepts none". The binding must not collapse the two. + assert ExecutorCapabilities().to_wire() == {} + assert ExecutorCapabilities(workspace_roots=()).to_wire() == {"workspaceRoots": []} + # `tags` is reserved for v2 pool routing and unread today, but it is on the + # wire — a binding that dropped it would be lossy, not tidy. + assert ExecutorCapabilities(tags=("gpu",)).to_wire() == {"tags": ["gpu"]} + + transport.reply("executors_list.json") + executors = bridge.list_executors() + assert executors[0].capabilities is not None + assert executors[0].capabilities.workspace_roots == ("/workspace",) + assert executors[1].capabilities is None + + +def test_list_executors_projects_every_row(bridge, transport): + transport.reply("executors_list.json") + + executors = bridge.list_executors() + + assert transport.last.method == "GET" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/executors" + + active, retired = executors + assert active.executor_id == "macbook-pro-work" + assert active.kind == "alissa-code" + assert active.label == "MacBook Pro (work)" + assert active.hostname == "mbp-work.local" + assert active.cli_version == "1.4.2" + assert active.poll_seconds == 15 + assert active.worker_name == "worker-mbp" + assert active.started_at == 1786500000000 + assert active.last_heartbeat_at == 1786500600000 + assert active.status == "active" + assert active.ended_at is None and active.end_reason is None + + assert retired.status == "executor_stopped" + assert retired.ended_at == 1786401000000 + assert retired.end_reason == "executor_stopped" + assert retired.cli_version is None + + +def test_executor_summary_never_carries_a_fingerprint(bridge, transport): + # Machine identifiers stay server-side; modelling one would invent a field. + transport.reply("executors_list.json") + + summary = bridge.list_executors()[0] + + assert not hasattr(summary, "fingerprint") + + +def test_heartbeat_posts_to_the_executor_and_reports_what_the_beat_did(bridge, transport): + transport.reply("executor_heartbeat.json") + + beat = bridge.heartbeat_executor("macbook-pro-work") + + assert transport.last.method == "POST" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/executors/macbook-pro-work/heartbeat" + assert beat.executor_id == "macbook-pro-work" + assert beat.beat == "coalesced" + + +def test_a_missing_executor_comes_back_as_a_value_not_an_error(bridge, transport): + # §5.1: "missing" means re-register. It is deliberately not a 404. + transport.reply("executor_heartbeat_missing.json") + + assert bridge.heartbeat_executor("macbook-pro-work").beat == "missing" + + +def test_stop_reports_how_many_jobs_went_with_it(bridge, transport): + transport.reply("executor_stop.json") + + stopped = bridge.stop_executor("macbook-pro-work", reason="signal") + + assert transport.last.method == "POST" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/executors/macbook-pro-work/stop" + assert transport.last_body == {"reason": "signal"} + assert stopped.executor_id == "macbook-pro-work" + assert stopped.found is True + assert stopped.already_ended is False + assert stopped.released_jobs == 3 + + +def test_stop_without_a_reason_sends_an_empty_body(bridge, transport): + transport.reply("executor_stop.json") + + bridge.stop_executor("macbook-pro-work") + + assert transport.last_body == {} + + +def test_executor_ids_are_percent_encoded_into_the_path(bridge, transport): + transport.reply("executor_heartbeat.json") + + bridge.heartbeat_executor("weird/id") + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/executors/weird%2Fid/heartbeat" diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_jobs.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_jobs.py new file mode 100644 index 0000000..ad4969d --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_jobs.py @@ -0,0 +1,349 @@ +"""The seven job endpoints of ``/v1/bridge`` (§5.2), against recorded responses. + +Feed, detail, claim, start, progress, fulfill, fail — request shape and typed +response for each, plus the noop/absorb and cancel paths that a reader of this +surface has to be able to see. +""" +from __future__ import annotations + +from alissa.sdk.api.bridge import JobResult, JobResultAcceptance, JobResultLink + +from conftest import TEST_BASE_URL + + +# ── Feed ───────────────────────────────────────────────────────────────────── + + +def test_feed_requires_an_executor_and_folds_in_the_heartbeat(bridge, transport): + transport.reply("jobs_feed.json") + + feed = bridge.list_jobs(executor_id="macbook-pro-work") + + assert transport.last.method == "GET" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs?executorId=macbook-pro-work" + # The poll IS the liveness signal, so the beat rides back on it. + assert feed.beat == "touched" + assert [job.job_id for job in feed.jobs] == ["j57bridge0002", "j57bridge0001"] + + +def test_feed_statuses_are_comma_joined_and_limit_passes_through(bridge, transport): + transport.reply("jobs_feed.json") + + bridge.list_jobs(executor_id="mbp", status=["pending", "running"], limit=10) + + assert transport.last.url == ( + f"{TEST_BASE_URL}/v1/bridge/jobs?executorId=mbp&status=pending%2Crunning&limit=10" + ) + + +def test_a_single_status_may_be_passed_as_a_bare_string(bridge, transport): + transport.reply("jobs_feed.json") + + bridge.list_jobs(executor_id="mbp", status="claimed") + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs?executorId=mbp&status=claimed" + + +def test_feed_rows_project_the_cas_generation_and_the_attempt_budget(bridge, transport): + transport.reply("jobs_feed.json") + + pending, running = bridge.list_jobs(executor_id="mbp").jobs + + assert (pending.claim_seq, pending.attempt, pending.max_attempts) == (4, 1, 3) + assert pending.status == "pending" + assert pending.created_at == 1786500000000 + assert pending.pending_expires_at == 1786503600000 + # Nothing holds a pending row yet. + assert pending.consumer_id is None + assert running.consumer_id == "consumer-a1" + + +def test_feed_carries_the_whole_spec_including_the_prompt(bridge, transport): + # summarizeJobForFeed projects `spec` whole — the feed omits the RESULT side + # (result/error/timing), not the prompt. Modelling a promptless feed spec + # would be inventing a projection the server does not perform. + transport.reply("jobs_feed.json") + + spec = bridge.list_jobs(executor_id="mbp").jobs[0].spec + + assert spec.title == "Wire the queue-mode bindings" + assert spec.prompt.startswith("Implement the typed bindings") + assert spec.deliverable.kind == "pull_request" + assert [criterion.id for criterion in spec.acceptance] == ["surface", "tests"] + assert spec.acceptance[1].type == "automated" + assert spec.references is not None + assert spec.references[0].ref == "TASK-1115046778" + assert spec.references[1].label is None + assert spec.workspace_root == "/workspace" + assert spec.handoff == "claude" + # env is variable NAMES only — never a credential channel. + assert spec.env == ("ALISSA_API_TOKEN",) + + +def test_a_spec_without_optional_blocks_keeps_them_absent(bridge, transport): + transport.reply("jobs_feed.json") + + spec = bridge.list_jobs(executor_id="mbp").jobs[1].spec + + assert spec.acceptance == () + assert spec.references is None + assert spec.env is None + assert spec.workspace_root is None + + +# ── Detail ─────────────────────────────────────────────────────────────────── + + +def test_detail_unwraps_the_job_envelope_and_carries_lifecycle_state(bridge, transport): + transport.reply("job_detail.json") + + job = bridge.get_job("j57bridge0001") + + assert transport.last.method == "GET" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0001" + assert job.job_id == "j57bridge0001" + assert job.status == "fulfilled" + assert job.sticky_executor_id == "macbook-pro-work" + assert job.claimed_by_executor_id == "macbook-pro-work" + assert job.cancel_requested is False + assert job.progress_note == "running the test suite" + assert job.executor_session_id == "cs57session01" + assert job.tmux_session == "ali-bridge-j57bridge0001" + assert (job.claimed_at, job.started_at) == (1786499100000, 1786499200000) + assert (job.last_progress_at, job.deadline_at) == (1786500100000, 1786506800000) + assert job.completed_at == 1786500200000 + + +def test_detail_models_the_result_body_the_feed_omits(bridge, transport): + transport.reply("job_detail.json") + + result = bridge.get_job("j57bridge0001").result + + assert result is not None + assert result.summary == "Opened the draft PR." + assert result.markdown is not None and result.markdown.startswith("## What changed") + assert result.links is not None and result.links[0].label == "PR" + assert result.acceptance is not None + assert [(row.id, row.met) for row in result.acceptance] == [("surface", True), ("tests", True)] + assert result.acceptance[0].note == "eleven endpoints" + assert result.acceptance[1].note is None + + +def test_detail_accepts_a_server_only_failure_kind(bridge, transport): + # A daemon may only claim executor_error/spec_rejected/cancelled, but the + # server writes kinds of its own. A reader that enumerated the daemon's + # three would choke on every swept row. + transport.reply("job_detail_failed.json") + + job = bridge.get_job("j57bridge0003") + + assert job.status == "failed" + assert job.failure_kind == "executor_lost" + assert job.error == "the executor stopped answering" + assert job.cancel_requested is True + assert job.result is None + + +def test_job_ids_are_percent_encoded_into_the_path(bridge, transport): + transport.reply("job_detail.json") + + bridge.get_job("weird/id") + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/weird%2Fid" + + +# ── Claim ──────────────────────────────────────────────────────────────────── + + +def test_claim_echoes_the_generation_and_returns_the_wall_clock_deadline(bridge, transport): + transport.reply("job_claim.json") + + claim = bridge.claim_job( + "j57bridge0002", executor_id="macbook-pro-work", consumer_id="consumer-b2", claim_seq=4 + ) + + assert transport.last.method == "POST" + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0002/claim" + assert transport.last_body == { + "executorId": "macbook-pro-work", + "consumerId": "consumer-b2", + "claimSeq": 4, + } + assert claim.ok is True + assert claim.job_id == "j57bridge0002" + assert claim.claim_seq == 5 + assert claim.deadline_at == 1786507200000 + assert claim.spec.title == "Wire the queue-mode bindings" + + +# ── Start ──────────────────────────────────────────────────────────────────── + + +def test_start_reports_the_session_and_any_pending_cancel(bridge, transport): + transport.reply("job_start.json") + + ack = bridge.start_job( + "j57bridge0002", + consumer_id="consumer-b2", + executor_session_id="cs57session02", + tmux_session="ali-bridge-j57bridge0002", + ) + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0002/start" + assert transport.last_body == { + "consumerId": "consumer-b2", + "executorSessionId": "cs57session02", + "tmuxSession": "ali-bridge-j57bridge0002", + } + assert ack.ok is True + assert ack.status == "running" + assert ack.cancel_requested is False + + +def test_start_surfaces_a_cancel_before_a_session_is_spawned(bridge, transport): + # §5.4: a job cancelled while merely `claimed` must be observable here, or + # the caller starts a session it is about to tear down. + transport.reply("job_start_cancelled.json") + + assert bridge.start_job("j", consumer_id="c").cancel_requested is True + + +def test_start_at_a_terminal_row_is_a_noop_carrying_the_absorbing_status(bridge, transport): + transport.reply("job_start_noop.json") + + ack = bridge.start_job("j", consumer_id="c") + + assert ack.noop is True + assert ack.current_status == "timeout" + assert ack.status is None + + +def test_start_omits_the_optional_session_fields(bridge, transport): + transport.reply("job_start.json") + + bridge.start_job("j", consumer_id="c") + + assert transport.last_body == {"consumerId": "c"} + + +# ── Progress ───────────────────────────────────────────────────────────────── + + +def test_progress_posts_the_note_and_reports_the_beat(bridge, transport): + transport.reply("job_progress.json") + + ack = bridge.progress_job("j57bridge0002", consumer_id="consumer-b2", note="running tests") + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0002/progress" + assert transport.last_body == {"consumerId": "consumer-b2", "note": "running tests"} + assert ack.ok is True + assert ack.coalesced is False + assert ack.cancel_requested is False + + +def test_a_coalesced_progress_write_is_visible_to_the_caller(bridge, transport): + # Writes coalesce to one a minute: the beat counted, the note was dropped. + transport.reply("job_progress_coalesced.json") + + ack = bridge.progress_job("j", consumer_id="c", note="dropped") + + assert ack.coalesced is True + assert ack.cancel_requested is True + + +def test_progress_without_a_note_is_a_bare_beat(bridge, transport): + transport.reply("job_progress.json") + + bridge.progress_job("j", consumer_id="c") + + assert transport.last_body == {"consumerId": "c"} + + +# ── Fulfill ────────────────────────────────────────────────────────────────── + + +def test_fulfill_serializes_the_full_result(bridge, transport): + transport.reply("job_fulfill.json") + + ack = bridge.fulfill_job( + "j57bridge0002", + consumer_id="consumer-b2", + result=JobResult( + summary="Done.", + markdown="## Notes", + links=(JobResultLink(label="PR", url="https://example.invalid/pr/1"),), + acceptance=( + JobResultAcceptance(id="surface", met=True, note="all eleven"), + JobResultAcceptance(id="tests", met=False), + ), + ), + ) + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0002/fulfill" + assert transport.last_body == { + "consumerId": "consumer-b2", + "result": { + "summary": "Done.", + "markdown": "## Notes", + "links": [{"label": "PR", "url": "https://example.invalid/pr/1"}], + "acceptance": [ + {"id": "surface", "met": True, "note": "all eleven"}, + {"id": "tests", "met": False}, + ], + }, + } + assert ack.ok is True + assert ack.noop is None + + +def test_a_minimal_result_sends_only_its_summary(bridge, transport): + transport.reply("job_fulfill.json") + + bridge.fulfill_job("j", consumer_id="c", result=JobResult(summary="Done.")) + + assert transport.last_body == {"consumerId": "c", "result": {"summary": "Done."}} + + +def test_a_late_fulfill_is_absorbed_rather_than_errored(bridge, transport): + transport.reply("job_fulfill_noop.json") + + ack = bridge.fulfill_job("j", consumer_id="c", result=JobResult(summary="Done.")) + + assert ack.noop is True + assert ack.current_status == "timeout" + + +# ── Fail ───────────────────────────────────────────────────────────────────── + + +def test_a_retryable_failure_hands_the_row_back_as_pending(bridge, transport): + transport.reply("job_fail_retry.json") + + ack = bridge.fail_job( + "j57bridge0002", + consumer_id="consumer-b2", + error="the agent crashed", + retryable=True, + failure_kind="executor_error", + ) + + assert transport.last.url == f"{TEST_BASE_URL}/v1/bridge/jobs/j57bridge0002/fail" + assert transport.last_body == { + "consumerId": "consumer-b2", + "error": "the agent crashed", + "retryable": True, + "failureKind": "executor_error", + } + assert ack.status == "pending" + assert ack.attempt == 3 + + +def test_a_terminal_failure_reports_the_spent_budget(bridge, transport): + transport.reply("job_fail_terminal.json") + + ack = bridge.fail_job("j", consumer_id="c", error="gave up", retryable=False) + + # retryable is a real False, not an omitted optional — it must survive. + assert transport.last_body == {"consumerId": "c", "error": "gave up", "retryable": False} + assert ack.status == "failed" + assert ack.attempt == 3 diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py new file mode 100644 index 0000000..f22c0df --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py @@ -0,0 +1,86 @@ +"""The default transport, driven with a stubbed ``urlopen`` — still no network. + +The one rule this file exists to lock: **a 4xx/5xx is a response, not an +exception**. ``urllib`` raises ``HTTPError`` for error statuses, and the API's +whole error contract lives in the *body* of those responses — a transport that +let the exception through would throw the contract away, and every typed error +in this SDK would degrade to "something went wrong". +""" +from __future__ import annotations + +import io +import urllib.error +import urllib.request + +import pytest + +from alissa.sdk.api import HttpRequest, TransportError, UrllibTransport + + +class _FakeResponse: + def __init__(self, status: int, body: bytes) -> None: + self.status = status + self._body = body + self.headers = {"Content-Type": "application/json"} + + def read(self) -> bytes: + return self._body + + def __enter__(self) -> "_FakeResponse": + return self + + def __exit__(self, *exc_info: object) -> None: + return None + + +def _request() -> HttpRequest: + return HttpRequest(method="POST", url="https://api.test.invalid/v1/x", headers={}, body=b"{}") + + +def test_a_success_is_passed_through_with_its_body_and_headers(monkeypatch): + monkeypatch.setattr(urllib.request, "urlopen", lambda req, timeout: _FakeResponse(200, b'{"ok":true}')) + + response = UrllibTransport().send(_request()) + + assert response.status == 200 + assert response.body == b'{"ok":true}' + assert response.headers["Content-Type"] == "application/json" + + +def test_an_http_error_comes_back_as_a_response_so_the_envelope_survives(monkeypatch): + body = b'{"error":"STALE_CONSUMER","message":"nope","status":"running"}' + + def raise_http_error(req, timeout): + raise urllib.error.HTTPError(req.full_url, 409, "Conflict", {}, io.BytesIO(body)) + + monkeypatch.setattr(urllib.request, "urlopen", raise_http_error) + + response = UrllibTransport().send(_request()) + + assert response.status == 409 + assert response.body == body + + +def test_a_connection_failure_is_a_transport_error(monkeypatch): + def raise_url_error(req, timeout): + raise urllib.error.URLError("name or service not known") + + monkeypatch.setattr(urllib.request, "urlopen", raise_url_error) + + with pytest.raises(TransportError): + UrllibTransport().send(_request()) + + +def test_a_timeout_is_a_transport_error(monkeypatch): + def raise_timeout(req, timeout): + raise TimeoutError("timed out") + + monkeypatch.setattr(urllib.request, "urlopen", raise_timeout) + + with pytest.raises(TransportError): + UrllibTransport(timeout=0.001).send(_request()) + + +def test_the_timeout_is_finite_by_default(): + # An observer loop must fail rather than hang forever on a dead connection. + assert 0 < UrllibTransport().timeout < 120 From 051adb43f2e3fbe78a719fe55ec3fcc19e0e0646 Mon Sep 17 00:00:00 2001 From: alissa-develop-daemon Date: Thu, 13 Aug 2026 03:46:37 +0000 Subject: [PATCH 2/2] Address review round 1: env-var split, redirects, shape drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four dispositions from the round-1 review, plus the version answer. Honour the CLI's `$ALISSA_API_BASE` as a base-URL fallback. The comment above `ENV_BASE_URL` claimed the pair matched the Node `alissa` CLI; for the token that holds, but the CLI resolves `$ALISSA_API_BASE` (cli/src/config.ts), not `$ALISSA_BASE`. Keeping `ALISSA_BASE` first preserves the Python sibling's spelling, and reading the CLI's as a fallback closes the misroute where an operator points a machine at staging the documented CLI way and this SDK keeps talking to production with a live token. Both READMEs now say which is which. Stop following redirects. `urlopen`'s default opener rebuilds a redirected request with every header copied across — `Authorization` included, and across hosts — so a 3xx from the configured base would hand the bearer token to another origin. `requests` strips it; stdlib does not. The API is a fixed JSON surface with no reason to redirect, so the transport builds an opener that refuses them: an unexpected 3xx surfaces as a visible `HTTP_3xx` `ApiError` rather than silently succeeding somewhere else. Keep server shape drift inside `AlissaError`. `get_job` unwrapped `["job"]` directly, and the models index their required fields, so a drift between the schemas these were written from and the runtime escaped as a bare `KeyError` or `TypeError` — outside the hierarchy `errors.py` exists to guarantee, and past the `except AlissaError` the docs teach. Every `from_wire` call now goes through one boundary that converts those into `TransportError`, which already means "the response was not what the contract says". Read the required `releasedJobs` as a required field. `.get(..., 0)` made a missing count report "nothing was released" — a wrong answer rather than a failure — and it was the only required field in the module read that way. Drop the 0.1.0 → 0.2.0 bump. Merging publishes to PyPI irreversibly, and a version number can never be reused; that call belongs to a maintainer, not to this branch, and it is now cheaper to make after a real call has been made against the bridge surface. Nothing here depends on the bump, and this commit also adds public surface (`ENV_BASE_URL_CLI`) that a release would freeze. Co-Authored-By: Claude Opus 5 Co-Authored-By: alissa-app --- alissa/README.md | 11 +++ .../src/main/alissa/sdk/api/bridge/client.py | 84 ++++++++++++++--- .../src/main/alissa/sdk/api/bridge/models.py | 2 +- alissa/src/main/alissa/sdk/api/client.py | 51 +++++++++- alissa/src/main/alissa/sdk/version | 2 +- .../test_sdk/test_api/test_api_client.py | 24 ++++- .../test_api/test_bridge_shape_drift.py | 93 +++++++++++++++++++ .../test_api/test_urllib_transport.py | 60 +++++++++--- 8 files changed, 292 insertions(+), 35 deletions(-) create mode 100644 alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_shape_drift.py diff --git a/alissa/README.md b/alissa/README.md index 51c7693..b680716 100644 --- a/alissa/README.md +++ b/alissa/README.md @@ -95,6 +95,17 @@ second HTTP stack in this SDK. Configuration: `ALISSA_API_TOKEN` (or `token=`) and `ALISSA_BASE` (or `base_url=`, default `https://api.alissa.app`). +> **On the base-URL variable.** The Node `alissa` CLI spells this knob +> `ALISSA_API_BASE`, not `ALISSA_BASE`. This SDK reads `ALISSA_BASE` first +> (matching its Python siblings) and falls back to `ALISSA_API_BASE`, so +> exporting either one points the CLI *and* the SDK at the same deployment. +> Set both and `ALISSA_BASE` wins. + +Requests never follow redirects: the API is a fixed JSON surface with no reason +to issue one, and `urllib` would copy the `Authorization` header onto a +cross-origin `Location`. An unexpected 3xx surfaces as an `ApiError` with code +`HTTP_302` (or whichever status arrived) rather than a silent off-origin call. + ### Local Bridge queue mode (`alissa.sdk.api.bridge`) The `/v1/bridge` executor and job surface: four executor endpoints (register, diff --git a/alissa/src/main/alissa/sdk/api/bridge/client.py b/alissa/src/main/alissa/sdk/api/bridge/client.py index 7390f4b..e96c92f 100644 --- a/alissa/src/main/alissa/sdk/api/bridge/client.py +++ b/alissa/src/main/alissa/sdk/api/bridge/client.py @@ -21,9 +21,10 @@ """ from __future__ import annotations -from typing import Any, Mapping, Sequence +from typing import Any, Callable, Mapping, Sequence, TypeVar from ..client import ApiClient, encode_path +from ..errors import TransportError from .models import ( ExecutorCapabilities, ExecutorHeartbeat, @@ -46,6 +47,38 @@ #: The only executor kind this contract accepts (``z.literal("alissa-code")``). EXECUTOR_KIND = "alissa-code" +_Model = TypeVar("_Model") + + +def _decoded(build: Callable[[Mapping[str, Any]], _Model], payload: Any, endpoint: str) -> _Model: + """Build a model from a decoded body, keeping every failure an ``AlissaError``. + + The models index their required fields directly (``payload["jobId"]``), so a + drift between the server's shape and the schemas these were written from + would escape as a ``KeyError`` or ``TypeError`` from inside ``from_wire`` — + neither of which is an :class:`~alissa.sdk.api.errors.AlissaError`, so the + ``except AlissaError`` pattern this package documents would not catch it. + Shape drift is the one residual risk on this surface; funnel it into + :class:`~alissa.sdk.api.errors.TransportError`, which already means "the + response was not what the contract says". + """ + if not isinstance(payload, Mapping): + raise TransportError(f"{endpoint}: expected a JSON object, got {type(payload).__name__}.") + try: + return build(payload) + except KeyError as exc: + raise TransportError(f"{endpoint}: the response is missing {exc}.") from exc + except (TypeError, ValueError) as exc: + raise TransportError(f"{endpoint}: the response did not match the expected shape: {exc}") from exc + + +def _unwrapped(payload: Any, key: str, endpoint: str) -> Mapping[str, Any]: + """Pull the single-object envelope key some responses wrap their row in.""" + inner = payload.get(key) if isinstance(payload, Mapping) else None + if not isinstance(inner, Mapping): + raise TransportError(f"{endpoint}: the response carries no {key!r} object.") + return inner + class BridgeClient: """Typed access to ``/v1/bridge``'s executor and job endpoints. @@ -106,7 +139,11 @@ def register_executor( "workerName": worker_name, "capabilities": capabilities.to_wire() if capabilities is not None else None, } - return ExecutorRegistration.from_wire(self.client.post(f"{BRIDGE_PREFIX}/executors", body=payload)) + return _decoded( + ExecutorRegistration.from_wire, + self.client.post(f"{BRIDGE_PREFIX}/executors", body=payload), + "POST /v1/bridge/executors", + ) def list_executors(self) -> tuple[ExecutorSummary, ...]: """``GET /v1/bridge/executors`` — this user's executors. @@ -116,7 +153,8 @@ def list_executors(self) -> tuple[ExecutorSummary, ...]: """ payload = self.client.get(f"{BRIDGE_PREFIX}/executors") rows = payload.get("executors") if isinstance(payload, Mapping) else None - return tuple(ExecutorSummary.from_wire(row) for row in (rows or ())) + endpoint = "GET /v1/bridge/executors" + return tuple(_decoded(ExecutorSummary.from_wire, row, endpoint) for row in (rows or ())) def heartbeat_executor(self, executor_id: str) -> ExecutorHeartbeat: """``POST /v1/bridge/executors/{id}/heartbeat`` — report this executor alive. @@ -129,7 +167,11 @@ def heartbeat_executor(self, executor_id: str) -> ExecutorHeartbeat: # An empty JSON object rather than no body at all: the route reads # nothing from it, but a bodyless POST is the kind of request proxies # and body parsers disagree about. - return ExecutorHeartbeat.from_wire(self.client.post(path, body={})) + return _decoded( + ExecutorHeartbeat.from_wire, + self.client.post(path, body={}), + "POST /v1/bridge/executors/{id}/heartbeat", + ) def stop_executor(self, executor_id: str, *, reason: str | None = None) -> ExecutorStopResult: """``POST /v1/bridge/executors/{id}/stop`` — close it and release its jobs. @@ -140,7 +182,11 @@ def stop_executor(self, executor_id: str, *, reason: str | None = None) -> Execu Idempotent: a repeat call releases nothing. """ path = f"{BRIDGE_PREFIX}/executors/{encode_path(executor_id)}/stop" - return ExecutorStopResult.from_wire(self.client.post(path, body={"reason": reason})) + return _decoded( + ExecutorStopResult.from_wire, + self.client.post(path, body={"reason": reason}), + "POST /v1/bridge/executors/{id}/stop", + ) # ── Jobs (§5.2) ────────────────────────────────────────────────────────── @@ -168,7 +214,11 @@ def list_jobs( "status": ",".join(statuses) if statuses else None, "limit": limit, } - return JobFeed.from_wire(self.client.get(f"{BRIDGE_PREFIX}/jobs", query=query)) + return _decoded( + JobFeed.from_wire, + self.client.get(f"{BRIDGE_PREFIX}/jobs", query=query), + "GET /v1/bridge/jobs", + ) def get_job(self, job_id: str) -> JobDetail: """``GET /v1/bridge/jobs/{jobId}`` — one job, with lifecycle timing. @@ -177,7 +227,9 @@ def get_job(self, job_id: str) -> JobDetail: ``resumed``, or a job you lost track of mid-run. Carries ``cancel_requested`` and the full timing the feed omits. """ - return JobDetail.from_wire(self.client.get(f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}")["job"]) + endpoint = "GET /v1/bridge/jobs/{jobId}" + payload = self.client.get(f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}") + return _decoded(JobDetail.from_wire, _unwrapped(payload, "job", endpoint), endpoint) def claim_job(self, job_id: str, *, executor_id: str, consumer_id: str, claim_seq: int) -> JobClaim: """``POST /v1/bridge/jobs/{id}/claim`` — compare-and-swap on ``claimSeq``. @@ -195,7 +247,7 @@ def claim_job(self, job_id: str, *, executor_id: str, consumer_id: str, claim_se """ path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/claim" body = {"executorId": executor_id, "consumerId": consumer_id, "claimSeq": claim_seq} - return JobClaim.from_wire(self.client.post(path, body=body)) + return _decoded(JobClaim.from_wire, self.client.post(path, body=body), "POST /v1/bridge/jobs/{id}/claim") def start_job( self, @@ -217,7 +269,7 @@ def start_job( "executorSessionId": executor_session_id, "tmuxSession": tmux_session, } - return JobStartAck.from_wire(self.client.post(path, body=body)) + return _decoded(JobStartAck.from_wire, self.client.post(path, body=body), "POST /v1/bridge/jobs/{id}/start") def progress_job(self, job_id: str, *, consumer_id: str, note: str | None = None) -> JobProgressAck: """``POST /v1/bridge/jobs/{id}/progress`` — beat a running job, observe a cancel. @@ -228,7 +280,11 @@ def progress_job(self, job_id: str, *, consumer_id: str, note: str | None = None (``coalesced``). It is not a log stream. """ path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/progress" - return JobProgressAck.from_wire(self.client.post(path, body={"consumerId": consumer_id, "note": note})) + return _decoded( + JobProgressAck.from_wire, + self.client.post(path, body={"consumerId": consumer_id, "note": note}), + "POST /v1/bridge/jobs/{id}/progress", + ) def fulfill_job(self, job_id: str, *, consumer_id: str, result: JobResult) -> JobFulfillAck: """``POST /v1/bridge/jobs/{id}/fulfill`` — deliver the result. @@ -238,7 +294,11 @@ def fulfill_job(self, job_id: str, *, consumer_id: str, result: JobResult) -> Jo """ path = f"{BRIDGE_PREFIX}/jobs/{encode_path(job_id)}/fulfill" body = {"consumerId": consumer_id, "result": result.to_wire()} - return JobFulfillAck.from_wire(self.client.post(path, body=body)) + return _decoded( + JobFulfillAck.from_wire, + self.client.post(path, body=body), + "POST /v1/bridge/jobs/{id}/fulfill", + ) def fail_job( self, @@ -266,4 +326,4 @@ def fail_job( "retryable": retryable, "failureKind": failure_kind, } - return JobFailAck.from_wire(self.client.post(path, body=body)) + return _decoded(JobFailAck.from_wire, self.client.post(path, body=body), "POST /v1/bridge/jobs/{id}/fail") diff --git a/alissa/src/main/alissa/sdk/api/bridge/models.py b/alissa/src/main/alissa/sdk/api/bridge/models.py index 8bd1c47..4386853 100644 --- a/alissa/src/main/alissa/sdk/api/bridge/models.py +++ b/alissa/src/main/alissa/sdk/api/bridge/models.py @@ -192,7 +192,7 @@ def from_wire(cls, payload: Mapping[str, Any]) -> "ExecutorStopResult": executor_id=payload["executorId"], found=bool(payload.get("found")), already_ended=bool(payload.get("alreadyEnded")), - released_jobs=payload.get("releasedJobs", 0), + released_jobs=payload["releasedJobs"], ) diff --git a/alissa/src/main/alissa/sdk/api/client.py b/alissa/src/main/alissa/sdk/api/client.py index df5844a..616419d 100644 --- a/alissa/src/main/alissa/sdk/api/client.py +++ b/alissa/src/main/alissa/sdk/api/client.py @@ -30,10 +30,20 @@ #: Where the Alissa REST API lives. Every path is versioned under ``/v1``. DEFAULT_BASE_URL = "https://api.alissa.app" -#: Environment variables the client falls back to, matching the `alissa` CLI. +#: Bearer token variable, matching the `alissa` CLI and every other Alissa tool. ENV_TOKEN = "ALISSA_API_TOKEN" + +#: API root variable. ``ALISSA_BASE`` is the Python-side spelling — it is what +#: the sibling `alissa-tools-github-orcloop` client reads, and what this SDK +#: documents. ENV_BASE_URL = "ALISSA_BASE" +#: The Node `alissa` CLI spells the same knob differently: ``cli/src/config.ts`` +#: resolves ``$ALISSA_API_BASE``. Honoured as a fallback so that pointing a +#: machine at a staging deployment the CLI's way moves this SDK with it, instead +#: of leaving it talking to production with a live token. +ENV_BASE_URL_CLI = "ALISSA_API_BASE" + USER_AGENT = f"alissa-python-sdk/{_sdk_version.value}" #: Seconds before a request is abandoned. Deliberately finite: a binding used @@ -73,11 +83,37 @@ def send(self, request: HttpRequest) -> HttpResponse: # pragma: no cover - prot ... +class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): + """Refuse 3xx instead of following it — the token must not leave the origin. + + CPython's redirect handler rebuilds the request with **every** header + copied across (only ``content-length``/``content-type`` are dropped), and + it does so even when the redirect crosses to another host — so following a + 3xx would hand the user's bearer token to whatever origin the ``Location`` + names. ``requests`` strips ``Authorization`` on a cross-host redirect for + exactly this reason; ``urllib`` does not, and stdlib-only means inheriting + that. + + The API is a fixed JSON surface with no reason to redirect, so the tight + answer is to not follow at all. Returning ``None`` leaves the 3xx as an + ``HTTPError``, which :meth:`UrllibTransport.send` hands back as a normal + response — an unexpected redirect surfaces as a visible ``HTTP_3xx`` + :class:`~alissa.sdk.api.errors.ApiError` rather than silently succeeding + somewhere else. + """ + + def redirect_request(self, *args: Any, **kwargs: Any) -> None: + return None + + class UrllibTransport: """The default transport — :mod:`urllib.request`, no third-party dependency.""" def __init__(self, timeout: float = DEFAULT_TIMEOUT) -> None: self.timeout = timeout + #: Deliberately not the module-level default opener: this one does not + #: follow redirects (see :class:`_NoRedirectHandler`). + self.opener = urllib.request.build_opener(_NoRedirectHandler) def send(self, request: HttpRequest) -> HttpResponse: req = urllib.request.Request( @@ -87,7 +123,7 @@ def send(self, request: HttpRequest) -> HttpResponse: method=request.method, ) try: - with urllib.request.urlopen(req, timeout=self.timeout) as response: + with self.opener.open(req, timeout=self.timeout) as response: return HttpResponse( status=response.status, body=response.read(), @@ -111,8 +147,8 @@ class ApiClient: """Authenticated JSON access to the Alissa REST API. :param token: personal access token; falls back to ``$ALISSA_API_TOKEN``. - :param base_url: API root; falls back to ``$ALISSA_BASE``, then - :data:`DEFAULT_BASE_URL`. + :param base_url: API root; falls back to ``$ALISSA_BASE``, then to the + `alissa` CLI's ``$ALISSA_API_BASE``, then :data:`DEFAULT_BASE_URL`. :param timeout: seconds, applied by the default transport. :param transport: inject to bypass the network (tests, recorded fixtures). @@ -130,7 +166,12 @@ def __init__( transport: Transport | None = None, ) -> None: self._token = token - self.base_url = (base_url or os.environ.get(ENV_BASE_URL) or DEFAULT_BASE_URL).rstrip("/") + self.base_url = ( + base_url + or os.environ.get(ENV_BASE_URL) + or os.environ.get(ENV_BASE_URL_CLI) + or DEFAULT_BASE_URL + ).rstrip("/") self.transport: Transport = transport or UrllibTransport(timeout=timeout) @property diff --git a/alissa/src/main/alissa/sdk/version b/alissa/src/main/alissa/sdk/version index 341cf11..6c6aa7c 100644 --- a/alissa/src/main/alissa/sdk/version +++ b/alissa/src/main/alissa/sdk/version @@ -1 +1 @@ -0.2.0 \ No newline at end of file +0.1.0 \ No newline at end of file diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py index 237434c..f97bfd9 100644 --- a/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_api_client.py @@ -10,7 +10,7 @@ import pytest from alissa.sdk.api import ApiClient, DEFAULT_BASE_URL, MissingTokenError, TransportError -from alissa.sdk.api.client import ENV_BASE_URL, ENV_TOKEN, USER_AGENT, encode_path +from alissa.sdk.api.client import ENV_BASE_URL, ENV_BASE_URL_CLI, ENV_TOKEN, USER_AGENT, encode_path from alissa.sdk.api.errors import ApiError from conftest import TEST_BASE_URL, TEST_TOKEN, RecordedTransport @@ -18,6 +18,7 @@ def test_defaults_to_the_public_api(monkeypatch): monkeypatch.delenv(ENV_BASE_URL, raising=False) + monkeypatch.delenv(ENV_BASE_URL_CLI, raising=False) assert ApiClient(token="t").base_url == DEFAULT_BASE_URL assert DEFAULT_BASE_URL == "https://api.alissa.app" @@ -27,6 +28,27 @@ def test_base_url_comes_from_the_environment_and_loses_its_trailing_slash(monkey assert ApiClient(token="t").base_url == "https://api.staging.invalid" +def test_the_cli_spelling_of_the_base_url_is_honoured_as_a_fallback(monkeypatch): + # The Node `alissa` CLI reads $ALISSA_API_BASE. Ignoring it would let an + # operator point their machine at staging the documented CLI way while this + # SDK kept talking to production with a live token. + monkeypatch.delenv(ENV_BASE_URL, raising=False) + monkeypatch.setenv(ENV_BASE_URL_CLI, "https://api.staging.invalid") + assert ApiClient(token="t").base_url == "https://api.staging.invalid" + + +def test_the_python_spelling_wins_when_both_are_set(monkeypatch): + monkeypatch.setenv(ENV_BASE_URL, "https://python.invalid") + monkeypatch.setenv(ENV_BASE_URL_CLI, "https://cli.invalid") + assert ApiClient(token="t").base_url == "https://python.invalid" + + +def test_an_explicit_base_url_wins_over_both_variables(monkeypatch): + monkeypatch.setenv(ENV_BASE_URL, "https://python.invalid") + monkeypatch.setenv(ENV_BASE_URL_CLI, "https://cli.invalid") + assert ApiClient(token="t", base_url="https://explicit.invalid").base_url == "https://explicit.invalid" + + def test_token_falls_back_to_the_environment(monkeypatch): monkeypatch.setenv(ENV_TOKEN, "alissa_from_env") assert ApiClient().token == "alissa_from_env" diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_shape_drift.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_shape_drift.py new file mode 100644 index 0000000..dd53154 --- /dev/null +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_bridge_shape_drift.py @@ -0,0 +1,93 @@ +"""What happens when the server's shape is not the shape these models expect. + +The fixtures in this directory are written from ``api/src/schemas/bridge.ts``, +not captured from a live response, so a drift between schema and runtime is the +one residual risk on this surface. These tests do not claim the drift cannot +happen — they lock what it *looks like* when it does: always an +:class:`AlissaError`, never a raw ``KeyError`` escaping ``from_wire``. + +That matters because the module docstrings and ``alissa/README.md`` teach +``except AlissaError`` as the way to wrap a binding call. An exception outside +that hierarchy would walk straight past the handler the docs told callers to +write. +""" +from __future__ import annotations + +import pytest + +from alissa.sdk.api import AlissaError, TransportError +from alissa.sdk.api.bridge.client import _decoded + + +def test_a_detail_response_without_its_job_envelope_is_a_transport_error(bridge, transport): + transport.respond(200, {"ok": True}) + + with pytest.raises(TransportError): + bridge.get_job("j57bridge0001") + + +def test_a_detail_whose_job_is_not_an_object_is_a_transport_error(bridge, transport): + transport.respond(200, {"job": "j57bridge0001"}) + + with pytest.raises(TransportError): + bridge.get_job("j57bridge0001") + + +def test_a_missing_required_field_is_a_transport_error_not_a_key_error(bridge, transport): + # `jobId` is required by BridgeJobDetailSchema; dropping it is the drift. + transport.respond(200, {"job": {"status": "running"}}) + + with pytest.raises(TransportError) as caught: + bridge.get_job("j57bridge0001") + + assert "jobId" in str(caught.value) + + +def test_the_drift_error_is_catchable_as_the_documented_base_class(bridge, transport): + transport.respond(200, {"job": {"status": "running"}}) + + with pytest.raises(AlissaError): + bridge.get_job("j57bridge0001") + + +def test_a_feed_row_missing_its_id_is_a_transport_error(bridge, transport): + transport.respond(200, {"jobs": [{"status": "pending"}], "beat": "ok"}) + + with pytest.raises(TransportError): + bridge.list_jobs(executor_id="exec-1") + + +def test_an_executor_row_missing_its_id_is_a_transport_error(bridge, transport): + transport.respond(200, {"executors": [{"kind": "alissa-code"}]}) + + with pytest.raises(TransportError): + bridge.list_executors() + + +def test_a_stop_response_without_released_jobs_is_a_transport_error(bridge, transport): + # `releasedJobs` is required (z.number()). Defaulting it to 0 would report + # "nothing was released" — a meaningful, wrong answer — instead of failing. + transport.respond(200, {"executorId": "exec-1", "found": True, "alreadyEnded": False}) + + with pytest.raises(TransportError): + bridge.stop_executor("exec-1") + + +def test_a_response_that_is_not_an_object_at_all_is_a_transport_error(bridge, transport): + transport.respond(200, ["not", "an", "object"]) + + with pytest.raises(TransportError): + bridge.claim_job("j57bridge0001", executor_id="exec-1", consumer_id="c1", claim_seq=1) + + +def test_a_model_raising_a_type_error_is_converted_too(): + # No model here coerces in a way that can raise this today — the guard is + # for the ones that will, so it is driven directly rather than through a + # payload that pretends otherwise. + def explode(_payload): + raise TypeError("'NoneType' object is not subscriptable") + + with pytest.raises(TransportError) as caught: + _decoded(explode, {"jobId": "j57bridge0001"}, "GET /v1/bridge/jobs/{jobId}") + + assert "GET /v1/bridge/jobs/{jobId}" in str(caught.value) diff --git a/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py b/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py index f22c0df..d270895 100644 --- a/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py +++ b/alissa/src/test/test_alissa/test_sdk/test_api/test_urllib_transport.py @@ -5,16 +5,21 @@ whole error contract lives in the *body* of those responses — a transport that let the exception through would throw the contract away, and every typed error in this SDK would degrade to "something went wrong". + +The second rule, locked at the bottom: **the transport does not follow +redirects**, because ``urllib`` would carry the bearer token across origins. """ from __future__ import annotations import io import urllib.error import urllib.request +from types import SimpleNamespace import pytest from alissa.sdk.api import HttpRequest, TransportError, UrllibTransport +from alissa.sdk.api.client import _NoRedirectHandler class _FakeResponse: @@ -37,50 +42,75 @@ def _request() -> HttpRequest: return HttpRequest(method="POST", url="https://api.test.invalid/v1/x", headers={}, body=b"{}") -def test_a_success_is_passed_through_with_its_body_and_headers(monkeypatch): - monkeypatch.setattr(urllib.request, "urlopen", lambda req, timeout: _FakeResponse(200, b'{"ok":true}')) +def _transport(open_impl, **kwargs) -> UrllibTransport: + """A transport whose opener is stubbed — the one seam, still no socket.""" + transport = UrllibTransport(**kwargs) + transport.opener = SimpleNamespace(open=open_impl) + return transport + - response = UrllibTransport().send(_request()) +def test_a_success_is_passed_through_with_its_body_and_headers(): + response = _transport(lambda req, timeout: _FakeResponse(200, b'{"ok":true}')).send(_request()) assert response.status == 200 assert response.body == b'{"ok":true}' assert response.headers["Content-Type"] == "application/json" -def test_an_http_error_comes_back_as_a_response_so_the_envelope_survives(monkeypatch): +def test_an_http_error_comes_back_as_a_response_so_the_envelope_survives(): body = b'{"error":"STALE_CONSUMER","message":"nope","status":"running"}' def raise_http_error(req, timeout): raise urllib.error.HTTPError(req.full_url, 409, "Conflict", {}, io.BytesIO(body)) - monkeypatch.setattr(urllib.request, "urlopen", raise_http_error) - - response = UrllibTransport().send(_request()) + response = _transport(raise_http_error).send(_request()) assert response.status == 409 assert response.body == body -def test_a_connection_failure_is_a_transport_error(monkeypatch): +def test_a_connection_failure_is_a_transport_error(): def raise_url_error(req, timeout): raise urllib.error.URLError("name or service not known") - monkeypatch.setattr(urllib.request, "urlopen", raise_url_error) - with pytest.raises(TransportError): - UrllibTransport().send(_request()) + _transport(raise_url_error).send(_request()) -def test_a_timeout_is_a_transport_error(monkeypatch): +def test_a_timeout_is_a_transport_error(): def raise_timeout(req, timeout): raise TimeoutError("timed out") - monkeypatch.setattr(urllib.request, "urlopen", raise_timeout) - with pytest.raises(TransportError): - UrllibTransport(timeout=0.001).send(_request()) + _transport(raise_timeout, timeout=0.001).send(_request()) def test_the_timeout_is_finite_by_default(): # An observer loop must fail rather than hang forever on a dead connection. assert 0 < UrllibTransport().timeout < 120 + + +def test_redirects_are_not_followed_so_the_bearer_token_stays_on_one_origin(): + # urllib's redirect handler copies every header onto the new request, + # Authorization included, and does it across hosts. The transport must not + # give it the chance. + assert not any( + type(handler) is urllib.request.HTTPRedirectHandler for handler in UrllibTransport().opener.handlers + ) + assert any(isinstance(handler, _NoRedirectHandler) for handler in UrllibTransport().opener.handlers) + + +def test_the_redirect_handler_refuses_rather_than_rewriting_the_request(): + # Returning None is what leaves the 3xx as an HTTPError, which send() hands + # back as a plain response — so an unexpected redirect stays visible. + assert _NoRedirectHandler().redirect_request(None, None, 302, "Found", {}, "https://evil.invalid/") is None + + +def test_a_redirect_surfaces_as_a_response_carrying_its_status(): + def raise_redirect(req, timeout): + raise urllib.error.HTTPError(req.full_url, 302, "Found", {}, io.BytesIO(b"")) + + response = _transport(raise_redirect).send(_request()) + + assert response.status == 302 + assert response.body == b""