mirror of
https://github.com/magnus919/agent-skills.git
synced 2026-09-11 19:47:12 +03:00
fix(openlibrary): warn when merge-stub redirect walk exhausts its budget
Addresses the tracked follow-up debt from thicken-openlibrary: the client already bounds /type/redirect stub chasing at MAX_REDIRECT_HOPS, but a chain that outlives the budget silently handed back an opaque stub, which downstream commands rendered as an empty-shaped record with no hint why. The walk now emits a stderr warning naming the unresolved location before returning; mocked test drives HOPS+1 chained stubs end to end. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
parent
1ca147eecd
commit
3bcf476202
@@ -148,6 +148,11 @@ class OpenLibraryClient:
|
|||||||
params = None
|
params = None
|
||||||
hops += 1
|
hops += 1
|
||||||
continue
|
continue
|
||||||
|
if is_redirect_stub(data):
|
||||||
|
# Bounded stub walk exhausted (>N chained merges); make that
|
||||||
|
# visible instead of handing back an opaque stub silently.
|
||||||
|
warn(f"Redirect chain did not resolve within {MAX_REDIRECT_HOPS}"
|
||||||
|
f" hops (still stuck at {data.get('location', '?')})")
|
||||||
self.last_url = resp.url
|
self.last_url = resp.url
|
||||||
return data
|
return data
|
||||||
|
|
||||||
|
|||||||
@@ -379,6 +379,25 @@ class MockedClientTests(unittest.TestCase):
|
|||||||
"https://openlibrary.org" + WORK_KEY + ".json")
|
"https://openlibrary.org" + WORK_KEY + ".json")
|
||||||
self.assertEqual(json.loads(out)["title"], "Nineteen Eighty-Four")
|
self.assertEqual(json.loads(out)["title"], "Nineteen Eighty-Four")
|
||||||
|
|
||||||
|
def test_redirect_stub_chain_beyond_hop_budget_warns_instead_of_silence(self):
|
||||||
|
# A work that keeps resolving into further merge stubs exhausts the
|
||||||
|
# bounded walk; the CLI must say so (stderr warning) rather than emit
|
||||||
|
# an unexplained /type/redirect payload.
|
||||||
|
stub = FakeResponse(200, {"type": {"key": "/type/redirect"},
|
||||||
|
"location": WORK_KEY})
|
||||||
|
responses = [stub] * (ol_cli.MAX_REDIRECT_HOPS + 1)
|
||||||
|
with mock.patch.object(requests, "get",
|
||||||
|
side_effect=responses) as req:
|
||||||
|
code, out, err = run_cli("--json", "work", "OL24776360W")
|
||||||
|
self.assertEqual(code, 0)
|
||||||
|
self.assertEqual(req.call_count, ol_cli.MAX_REDIRECT_HOPS + 1)
|
||||||
|
self.assertIn("did not resolve", err)
|
||||||
|
# Without the resolution the command degrades to an empty-shaped
|
||||||
|
# record; the stderr warning is what keeps that from being silent.
|
||||||
|
self.assertEqual(json.loads(out), {
|
||||||
|
"key": "OL24776360W", "title": "?", "authors": [],
|
||||||
|
"description": "", "subjects": [], "cover_url": None})
|
||||||
|
|
||||||
def test_text_wrapper_dict_is_unwrapped(self):
|
def test_text_wrapper_dict_is_unwrapped(self):
|
||||||
self.assertEqual(ol_cli.unwrap_text({"type": "/type/text", "value": "hi"}), "hi")
|
self.assertEqual(ol_cli.unwrap_text({"type": "/type/text", "value": "hi"}), "hi")
|
||||||
self.assertEqual(ol_cli.unwrap_text("plain"), "plain")
|
self.assertEqual(ol_cli.unwrap_text("plain"), "plain")
|
||||||
|
|||||||
Reference in New Issue
Block a user