Pre-release fixes: anchor sdist includes, and stop retrying non-idempotent writes

CHANGELOG.md

@@ -28,6 +28,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Automatic retry no longer repeats non-idempotent requests. The transport
+  retried 429/503/504 and network errors for every method, so a lost response to
+  a POST/PATCH/DELETE that the server had already applied would duplicate a
+  create or re-run a bound action. Some bound actions in this API take no
+  arguments and mutate on every call, so a repeat is never harmless. Retries now
+  require either a read-only method or an explicit `Idempotency-Key`; otherwise
+  the error surfaces so the caller can decide. **This is a behaviour change**: a
+  transient 503 on a write that previously succeeded after a silent retry will
+  now raise. Pass `idempotency_key=` to opt back in.
+
+- The sdist no longer sweeps in nested checkouts. `[tool.hatch.build.targets.sdist]`
+  listed patterns like `docs/` and `src/` without a leading slash, so they matched at
+  any depth — and `.claude/worktrees/agent-*/` holds full copies of the repo. The
+  0.7.0 sdist picked up 1350 extra files that way (2 MiB instead of 434 KiB),
+  including older copies of `docs/configuration.md` and `docs/multi-company.md` from
+  before example identifiers were replaced with placeholders. The wheel was never
+  affected, since it builds from `packages`, and `git ls-files` was clean — only
+  inspecting the built artifact showed it. Patterns are now anchored to the project
+  root.
+
 - `build_url` now validates `entity_set_name` and `record_id` as single URL path
   components, rejecting raw `/`, `\`, `?`, `#` and the `.`/`..` segments. Both
   values are spliced directly into the request path, so a record key containing

pyproject.toml

@@ -108,17 +108,25 @@ packages = ["src/bcli", "src/bcli_cli", "src/bcli_mcp"]
 "packs" = "bcli/packs/_builtin"
 
 [tool.hatch.build.targets.sdist]
+# Leading slashes anchor these to the project root. Without them the patterns
+# match at any depth, so a nested checkout — .claude/worktrees/agent-*/ holds
+# full copies of the repo — had its own docs/, src/ and tests/ swept into the
+# sdist: 1350 extra files, including pre-sanitisation docs still carrying real
+# tenant identifiers. The wheel was unaffected (it builds from `packages`), and
+# `git ls-files` was clean, so only inspecting the built artifact caught it.
+# If you add a pattern here, anchor it, then verify with:
+#   uv build && tar -tzf dist/bc_cli-*.tar.gz | awk -F/ 'NF>1{print $2}' | sort -u
 include = [
-    "src/",
-    "packs/",
-    "tests/",
-    "examples/",
-    "docs/",
-    "README.md",
-    "LICENSE",
-    "NOTICE",
-    "CHANGELOG.md",
-    "pyproject.toml",
+    "/src",
+    "/packs",
+    "/tests",
+    "/examples",
+    "/docs",
+    "/README.md",
+    "/LICENSE",
+    "/NOTICE",
+    "/CHANGELOG.md",
+    "/pyproject.toml",
 ]
 
 [tool.pytest.ini_options]

src/bcli/client/_transport.py

@@ -41,6 +41,13 @@
 # Retryable status codes
 _RETRYABLE = {429, 503, 504}
 
+#: Methods that can be repeated without applying an effect twice. Deliberately
+#: excludes DELETE and PUT: both are idempotent by HTTP semantics, but a repeat
+#: here surfaces as a 404 or overwrites a concurrent change, and neither is what
+#: an automatic retry should decide on the caller's behalf. Pass an
+#: ``idempotency_key`` to opt a mutation back into retrying.
+_IDEMPOTENT_METHODS = frozenset({"GET", "HEAD"})
+
 DEFAULT_TIMEOUT = 60
 DEFAULT_MAX_RETRIES = 3
 INITIAL_BACKOFF = 1.0  # seconds
@@ -144,6 +151,21 @@ async def _request(
         backoff = INITIAL_BACKOFF
         t0 = time.monotonic()
 
+        # Repeating a request is only safe when it cannot apply an effect twice.
+        # A read can always be repeated. A POST / PATCH / DELETE cannot: the
+        # server may already have applied it when the response was lost, so a
+        # retry duplicates a create or re-runs a business action. Some bound
+        # actions in this API take no arguments and mutate on every invocation,
+        # so there is no such thing as a harmless repeat — one 503 from a gateway
+        # could recalculate twice with nothing in the log to say so.
+        #
+        # An Idempotency-Key re-enables retry, because that is what lets a
+        # gateway (or a future server-side implementation) collapse the
+        # duplicate. Without one, the error surfaces instead: a visible
+        # transient failure the caller can retry deliberately beats an invisible
+        # double-write.
+        retry_safe = method.upper() in _IDEMPOTENT_METHODS or idempotency_key is not None
+
         for attempt in range(self._max_retries + 1):
             try:
                 auth_headers = await self._inject_auth()
@@ -194,8 +216,8 @@ async def _request(
                 bc_message, correlation_id = _parse_bc_error(response)
                 status = response.status_code
 
-                # Retry on retryable errors
-                if status in _RETRYABLE and attempt < self._max_retries:
+                # Retry on retryable errors — but only if repeating is safe.
+                if status in _RETRYABLE and attempt < self._max_retries and retry_safe:
                     retry_after = _get_retry_after(response)
                     wait = retry_after if retry_after else backoff
                     logger.warning(
@@ -238,7 +260,10 @@ async def _request(
                 httpx.RemoteProtocolError,
             ) as e:
                 last_error = e
-                if attempt < self._max_retries:
+                # The most dangerous case for a mutation: the request may have
+                # reached the server and been applied before the connection
+                # dropped, so there is no way to know a retry is safe.
+                if attempt < self._max_retries and retry_safe:
                     logger.warning(
                         "Network error on %s %s: %s, retrying in %.1fs",
                         method, url, e, backoff,

tests/test_client/test_retry_idempotency.py

@@ -0,0 +1,124 @@
+"""Automatic retry must not silently re-run a non-idempotent request.
+
+The transport retried 429/503/504 and network errors for *every* method. For a
+GET that is free. For a POST, PATCH or DELETE it is not: the server may already
+have applied the request when the response was lost, so a retry duplicates a
+create or re-runs a business action.
+
+That is not theoretical for this SDK. Bound actions like the LLP-utilisation
+recalculation take no arguments and mutate on every invocation — there is no
+such thing as a harmless repeat. A single 503 from a gateway could recalculate
+twice, and nothing in the log would say so.
+
+So retries are now allowed only when repeating the request is safe:
+
+* the method is read-only (GET / HEAD), or
+* the caller supplied an ``Idempotency-Key``, which is what lets a gateway (or a
+  future server-side implementation) collapse the duplicate.
+
+Otherwise the error surfaces. A visible transient failure the caller can retry
+deliberately is strictly better than an invisible double-write.
+"""
+
+from __future__ import annotations
+
+import httpx
+import pytest
+
+from bcli.client._transport import BCTransport
+from bcli.errors import ServerError
+
+
+class _StubAuth:
+    async def get_access_token(self) -> str:
+        return "token"
+
+    def clear_cache(self) -> None:
+        return None
+
+
+def _transport(handler: httpx.MockTransport) -> BCTransport:
+    t = BCTransport(_StubAuth(), max_retries=2)
+    t._client = httpx.AsyncClient(transport=handler)
+    return t
+
+
+def _counting_handler(status: int):
+    """Always answers `status`; records how many times it was called."""
+    calls: list[str] = []
+
+    def handle(request: httpx.Request) -> httpx.Response:
+        calls.append(request.method)
+        return httpx.Response(status, json={"error": {"message": "nope"}})
+
+    return httpx.MockTransport(handle), calls
+
+
+class TestReadsStillRetry:
+    @pytest.mark.parametrize("status", [429, 503, 504])
+    async def test_get_retries_as_before(self, status):
+        handler, calls = _counting_handler(status)
+        t = _transport(handler)
+        with pytest.raises(Exception):
+            await t._request("GET", "https://example.test/x")
+        assert len(calls) == 3, "GET should still use all attempts"
+
+
+class TestMutationsDoNotRetryWithoutAKey:
+    @pytest.mark.parametrize("method", ["POST", "PATCH", "DELETE"])
+    @pytest.mark.parametrize("status", [429, 503, 504])
+    async def test_single_attempt_only(self, method, status):
+        handler, calls = _counting_handler(status)
+        t = _transport(handler)
+        with pytest.raises(Exception):
+            await t._request(method, "https://example.test/x")
+        assert len(calls) == 1, (
+            f"{method} was retried without an idempotency key; a lost response "
+            f"would duplicate the write"
+        )
+
+    @pytest.mark.parametrize("method", ["POST", "PATCH", "DELETE"])
+    async def test_network_error_is_not_retried_either(self, method):
+        """The dangerous case: the request may have been applied before the
+        connection dropped, so there is no way to know a retry is safe."""
+        calls: list[str] = []
+
+        def handle(request: httpx.Request) -> httpx.Response:
+            calls.append(request.method)
+            raise httpx.ReadTimeout("boom", request=request)
+
+        t = _transport(httpx.MockTransport(handle))
+        with pytest.raises(ServerError):
+            await t._request(method, "https://example.test/x")
+        assert len(calls) == 1
+
+
+class TestAnIdempotencyKeyReEnablesRetry:
+    @pytest.mark.parametrize("method", ["POST", "PATCH", "DELETE"])
+    async def test_key_allows_retry(self, method):
+        handler, calls = _counting_handler(503)
+        t = _transport(handler)
+        with pytest.raises(Exception):
+            await t._request(method, "https://example.test/x", idempotency_key="k-1")
+        assert len(calls) == 3
+
+    async def test_the_key_is_actually_sent(self):
+        seen: list[str | None] = []
+
+        def handle(request: httpx.Request) -> httpx.Response:
+            seen.append(request.headers.get("Idempotency-Key"))
+            return httpx.Response(200, json={})
+
+        t = _transport(httpx.MockTransport(handle))
+        await t._request("POST", "https://example.test/x", idempotency_key="k-2")
+        assert seen == ["k-2"]
+
+
+class TestSuccessPathUnaffected:
+    @pytest.mark.parametrize("method", ["GET", "POST", "PATCH", "DELETE"])
+    async def test_a_successful_mutation_still_works(self, method):
+        def handle(request: httpx.Request) -> httpx.Response:
+            return httpx.Response(200, json={"ok": True})
+
+        t = _transport(httpx.MockTransport(handle))
+        assert await t._request(method, "https://example.test/x") == {"ok": True}