Skip to content

Commit 447d2e4

Browse files
captainpacketclaude
andcommitted
fix: describe the unpublished endpoints, and stop publishing other people's drafts
The defect reports kept landing on the same root cause, so this addresses that rather than only its symptoms. Forward's published endpoints have a description generated from its server, so a parser cannot disagree with them for long. The unpublished ones had nothing: their responses were read key by key, by hand, and their fixtures were written from the same assumptions as the parsers. When both were wrong together nothing noticed, which is how a commit id nested under lastCommit came to be read from a flat key Forward never sends. They now have real schemas, merged into the generated description, so models are generated for them exactly as for published responses and parsing validates against a declared shape. The recorded examples are run through the SDK's own parsers, and an unpublished operation that documents no shape is a test failure. Two behaviours found by running both clients against one fake Forward: publish() staged over pre-existing drafts and committed them. A commit names paths rather than changes, so an unrelated half-finished edit sitting on a path being published was committed along with it, and neither party was told. It now refuses, with overwrite_drafts to say otherwise deliberately. The 409 strip-and-retry read its paths from the exception's display string when the error body would not validate. That string embeds the raw body inside formatting, so splitting it produced fragments matching nothing, only some paths were stripped, and the retry failed again on a 409 that looked like a different problem. It now reads the parsed body, or the raw body as JSON, and declines to strip at all when the paths cannot be identified. This endpoint is unpublished, so its error envelope is the least guaranteed anywhere in the SDK, which makes it the worst possible place to key behaviour off a display string. That pattern is one this SDK exists to remove from its consumers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERUwz8RFoDEdZfp4hFbGHz
1 parent 3a69a4f commit 447d2e4

15 files changed

Lines changed: 1535 additions & 70 deletions

File tree

scripts/downconvert_spec.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ class would be actively harmful here.
4444
SOURCE_VERSION = "3.2.0"
4545
TARGET_VERSION = "3.1.0"
4646

47+
#: Endpoints Forward does not publish, described by hand. Merged in here so
48+
#: models are generated for their responses exactly as for published ones, and
49+
#: so request validation covers them too. Without this their shapes are read by
50+
#: hand, which is what lost a commit id nested under `lastCommit`.
51+
UNPUBLISHED = Path("spec/unpublished.yaml")
52+
4753
STREAMING_MEDIA_TYPES = ("application/jsonl", "application/x-ndjson", "application/json-seq")
4854

4955
# Schemas whose contents are defined by the user's NQE query rather than by the
@@ -248,6 +254,38 @@ def _coerce_scalar(value: Any, declared: str) -> Any | None:
248254
return None
249255

250256

257+
def merge_unpublished(doc: dict[str, Any], extra: dict[str, Any]) -> int:
258+
"""Fold the hand-written description of unpublished endpoints into ``doc``.
259+
260+
Their operations are marked so that anything reading the merged document can
261+
tell them apart from what Forward publishes.
262+
"""
263+
merged = 0
264+
schemas = doc.setdefault("components", {}).setdefault("schemas", {})
265+
for name, schema in (extra.get("components", {}).get("schemas") or {}).items():
266+
if name in schemas:
267+
raise UnsupportedSpecFeature(
268+
f"unpublished schema {name!r} collides with a published one; rename it"
269+
)
270+
schemas[name] = schema
271+
272+
for path, item in (extra.get("paths") or {}).items():
273+
target = doc.setdefault("paths", {}).setdefault(path, {})
274+
for method, operation in item.items():
275+
if method in target:
276+
raise UnsupportedSpecFeature(
277+
f"unpublished {method.upper()} {path} collides with a published operation"
278+
)
279+
if isinstance(operation, dict):
280+
operation.setdefault("x-forward-stability", "unpublished")
281+
merged += 1
282+
target[method] = operation
283+
284+
for tag in extra.get("tags") or []:
285+
doc.setdefault("tags", []).append(tag)
286+
return merged
287+
288+
251289
def downconvert(doc: dict[str, Any]) -> tuple[dict[str, Any], dict[str, int]]:
252290
if doc.get("openapi") != SOURCE_VERSION:
253291
raise UnsupportedSpecFeature(
@@ -269,11 +307,19 @@ def downconvert(doc: dict[str, Any]) -> tuple[dict[str, Any], dict[str, int]]:
269307
def main(argv: list[str] | None = None) -> int:
270308
parser = argparse.ArgumentParser(description=__doc__)
271309
parser.add_argument("--input", type=Path, default=Path("spec/forward-openapi.yaml"))
310+
parser.add_argument("--unpublished", type=Path, default=UNPUBLISHED)
272311
parser.add_argument("--output", type=Path, default=Path("spec/forward-openapi-3.1.json"))
273312
args = parser.parse_args(argv)
274313

275314
doc = yaml.safe_load(args.input.read_text(encoding="utf-8"))
315+
316+
unpublished = 0
317+
if args.unpublished.exists():
318+
extra = yaml.safe_load(args.unpublished.read_text(encoding="utf-8")) or {}
319+
unpublished = merge_unpublished(doc, extra)
320+
276321
converted, stats = downconvert(doc)
322+
stats["unpublished_operations"] = unpublished
277323
args.output.write_text(json.dumps(converted, indent=2, sort_keys=True) + "\n", encoding="utf-8")
278324

279325
print(f"wrote {args.output}")

scripts/gen_public_models.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
3232
"DiffEntryType": "Type32",
3333
}
3434

35+
#: Generated names the SDK supersedes with a friendlier type of its own. The
36+
#: generated ones mirror the wire shape (a nested commit object, for instance);
37+
#: the hand-written ones are flat and are what callers use. Exporting both under
38+
#: one name would leave which you got depending on import order.
39+
SUPERSEDED = frozenset({"RepositoryQuery", "DraftChange"})
40+
3541
#: Types the SDK defines itself, alongside the generated ones, because they
3642
#: describe endpoints Forward does not publish a schema for.
3743
HAND_WRITTEN = (
@@ -65,7 +71,9 @@ def model_names() -> list[str]:
6571
return sorted(
6672
name
6773
for name in CLASS_DECLARATION.findall(source)
68-
if not name.startswith("_") and name not in {"ForwardModel", "OpenEnum"}
74+
if not name.startswith("_")
75+
and name not in {"ForwardModel", "OpenEnum"}
76+
and name not in SUPERSEDED
6977
)
7078

7179

0 commit comments

Comments
 (0)