From 7b4b5e72f39393bf307d15bb0288ca908d53b407 Mon Sep 17 00:00:00 2001 From: gitea-spec-agent Date: Mon, 12 Oct 2026 05:20:11 +0000 Subject: [PATCH] [verified] Document broker renew/status response contracts --- .../references/native-review.md | 39 ++++++++++++ harness/operations/repository-cache.md | 10 +++ .../tests/common/test_managed_lock_broker.py | 63 +++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/harness/agents/review/skills/gitea-code-review/references/native-review.md b/harness/agents/review/skills/gitea-code-review/references/native-review.md index a7b6df1..a4f156f 100644 --- a/harness/agents/review/skills/gitea-code-review/references/native-review.md +++ b/harness/agents/review/skills/gitea-code-review/references/native-review.md @@ -50,6 +50,45 @@ The request callback is `authenticated_request(relative_api_path, payload=None) `ApiAdapter.snapshot(target)` collects repository/Issue/PR metadata, identity/spec identity, all changed files, commits, reviews, historical review comments, Issue/PR comments and head statuses. Use this same collector for the baseline and subsequent snapshots. Every collection records contiguous 50-item pages, declared totals and Link headers; duplicates, inconsistent totals and truncated enumeration fail closed. A saved older collector snapshot may be replayed if it provides these complete facts, but differently shaped collectors must not be mixed for live source-drift comparison. +### Broker response preflight before publication + +Use the [operation-specific broker response contract](../../../../../operations/repository-cache.md#operation-specific-broker-responses). `renew` supplies only `status`/`deadline`; read `repository_held` and identity from a separate `status` request. Python uses `ttl=`, not the wire/CLI name `ttl_seconds`. Keep the admitted handle and the retained canonical `harness/lib/managed_lock_broker.py` module as `broker`; do not read its token or load another profile's helper. + +The following is a bounded caller example, not a new adapter or live publish CLI. `broker_handle`, `ttl_seconds`, `absolute_cache_policy`, `plan` and `native_review` are already-admitted workflow inputs. Renew before expiry, reject errors/missing or mistyped fields before publication, and pass the new deadline to the existing lease checker: + +```python +import math +import time + +try: + renewed = broker.request_handle(broker_handle, "renew", ttl=ttl_seconds) +except Exception: + raise RuntimeError("broker renew unavailable before publication") from None +if (type(renewed) is not dict or "error" in renewed + or set(renewed) != {"status", "deadline"} + or renewed["status"] not in ("held", "repository-held") + or type(renewed["deadline"]) not in (int, float) + or not math.isfinite(renewed["deadline"]) + or time.monotonic() >= renewed["deadline"]): + raise RuntimeError("broker renew response rejected before publication") +try: + status = broker.request_handle(broker_handle, "status") +except Exception: + raise RuntimeError("broker status unavailable before publication") from None +if (type(status) is not dict or "error" in status + or type(status.get("repository_held")) is not bool + or status.get("status") != renewed["status"] + or status.get("deadline") != renewed["deadline"]): + raise RuntimeError("broker status response rejected before publication") +deadline_monotonic = renewed["deadline"] +lease = native_review.BrokerIssueLease( + broker_handle, absolute_cache_policy, deadline_monotonic) +lease.check(plan) +# Continue with the existing source/adoption/publication gates below. +``` + +`repository_held` is an observation, not an additional publication permission or a requirement to acquire/release the repository lock here. The existing lease checker re-reads authoritative broker status and validates identity/source; the two preliminary responses alone do not authorize a POST. A failure before calling `publish` means this wrapper has not invoked a POST, not that a prior uncertain attempt is absent. Keep the existing journal and publication result, report this phase separately, and do not automatically release/reacquire the lease, retry POST, or repeat completed review/suite work. Canonical cleanup remains unchanged. + ## Live integration After absolute root discovery, role-authenticated transport capability validation, complete review, and Issue lease acquisition, load the module once using `importlib.util.spec_from_file_location` and retain it. These are existing workflow objects, **not** placeholders for an alternative credential source: diff --git a/harness/operations/repository-cache.md b/harness/operations/repository-cache.md index f8addb6..41d0aa5 100644 --- a/harness/operations/repository-cache.md +++ b/harness/operations/repository-cache.md @@ -78,6 +78,16 @@ Bootstrap 이후 모든 Issue workflow는 role-neutral shared managed-lock broke - 정상 release 응답은 기존 `{"status":"released"}`를 유지한다. 정리가 일부 실패하면 broker는 `{"error":"cleanup-incomplete","cleanup":...}`를 응답하고 종료하며, `cleanup`의 `lease_released`, `socket_removed`, `handle_saved`, `errors`로 해제와 잔여 정리를 구분한다. Handle 기록 가능 시 같은 결과를 비밀값 없이 저장하고 stderr에도 오류 코드만 보고한다. 부분 실패는 nonzero process exit로 보고하며, 성공한 cleanup을 재적용하거나 실패 이력을 성공으로 덮어쓰지 않는다. 성공 기록을 실패로 덮어쓰지 않는다. 이 오류만으로 이미 해제한 lease를 재해제하거나 workflow 외부 write를 반복하지 않는다. - 완료 gate는 기존 release → wait/reap → Issue probe 순서에 더해 broker의 정상 종료와 run-owned 소켓 경로 부재를 확인한다. `probe free=true`는 lock 해제 증거이지 소켓 권한·삭제 증거가 아니다. Lease 해제와 소켓·handle 정리 실패는 별도로 보고하고 완료로 간주하지 않는다. +### Operation-specific broker responses + +Broker 응답은 action별 계약이다. 다른 action의 필드가 있다고 가정하거나 missing 필드를 성공 값으로 채우지 않는다. + +- `renew` 성공 응답은 정확히 `status`, `deadline` 두 필드다. `status`는 `held` 또는 `repository-held`이고, `deadline`은 갱신된 유한 monotonic seconds 숫자다. `repository_held`와 identity 필드는 반환하지 않는다. 갱신 성공은 publication 또는 source 검증 성공이 아니다. +- `status` 성공 응답은 정확히 `status`, `deadline`, `repository_held`, `root`, `owner`, `repository`, `issue`, `role`, `pid`다. `repository_held`는 boolean이며 identity와 lock 상태 확인에는 이 응답을 사용한다. `deadline`은 wall-clock 시간이 아니다. 응답의 identity와 live deadline은 workflow가 독립적으로 승인한 target/role/handle에 대조해야 한다. +- Python public API는 `request_handle(handle_path, action, ttl=None)`이다. 갱신은 `request_handle(HANDLE, "renew", ttl=TTL)`로 호출한다. Python keyword는 `ttl`이고 wire/CLI 이름은 `ttl_seconds`/`--ttl-seconds`다. Client가 handle 인증을 캡슐화하므로 token을 직접 읽거나 출력하지 않는다. +- 실패 응답의 `error`는 성공 schema와 별개다. Python API는 error dict를 반환할 수 있고 transport/path/decoding 예외를 발생시킬 수도 있다. 호출자는 먼저 오류·필드·타입을 검사하고 실패 시 후속 작업을 차단한다. CLI `request`는 error dict에 nonzero exit를 반환한다. Raw 예외·handle·credential은 로그에 복사하지 않는다. +- Review finalization에서는 `renew` 뒤 별도 `status`를 읽고 기존 `BrokerIssueLease.check(plan)`으로 승인된 identity/source/lease를 검증한다. Wrapper의 pre-publication 실패를 Native POST의 uncertain 결과로 혼동하지 않는다. 기존 journal/attempt 상태는 보존하고, 이 오류만으로 lease release/reacquire 또는 POST 재시도를 자동 수행하지 않는다. Reviewer 사용 예제는 [native-review reference](../agents/review/skills/gitea-code-review/references/native-review.md#broker-response-preflight-before-publication)를 따른다. + ### Canonical command templates ```sh diff --git a/harness/tests/common/test_managed_lock_broker.py b/harness/tests/common/test_managed_lock_broker.py index f4b1b54..09a6aec 100644 --- a/harness/tests/common/test_managed_lock_broker.py +++ b/harness/tests/common/test_managed_lock_broker.py @@ -84,6 +84,69 @@ class ManagedLockBrokerTests(unittest.TestCase): finally: os.close(parent_fd) + def test_operation_specific_broker_response_documentation(self): + import ast + harness = Path(__file__).parents[2] + operations = (harness / "operations/repository-cache.md").read_text() + reference = (harness / "agents/review/skills/gitea-code-review/references/native-review.md").read_text() + self.assertIn("### Operation-specific broker responses", operations) + contract = operations.split("### Operation-specific broker responses", 1)[1].split("### Canonical command templates", 1)[0] + for field in ("status", "deadline", "repository_held", "root", "owner", "repository", "issue", "role", "pid"): + self.assertIn(f"`{field}`", contract) + self.assertIn("request_handle(handle_path, action, ttl=None)", contract) + self.assertIn("### Broker response preflight before publication", reference) + section = reference.split("### Broker response preflight before publication", 1)[1].split("## Live integration", 1)[0] + recipe = section.split("```python\n", 1)[1].split("```", 1)[0] + ast.parse(recipe) + self.assertIn('broker.request_handle(broker_handle, "renew", ttl=ttl_seconds)', recipe) + self.assertIn('broker.request_handle(broker_handle, "status")', recipe) + self.assertIn('"error" in renewed', recipe) + self.assertIn('"error" in status', recipe) + self.assertIn('status.get("repository_held")', recipe) + self.assertNotIn('renewed["repository_held"]', recipe) + self.assertIn('lease.check(plan)', recipe) + + def test_status_and_renew_have_distinct_success_and_error_shapes(self): + with tempfile.TemporaryDirectory() as d: + root = Path(d) + proc, handle, sock = self.start(root, ttl=30, role="gitea-review-agent") + status_keys = {"status", "deadline", "repository_held", "root", "owner", "repository", "issue", "role", "pid"} + try: + for held in (False, True): + with self.subTest(repository_held=held): + if held: + self.assertEqual(broker.request_handle(handle, "acquire_repository"), {"status": "repository-held"}) + renewed = broker.request_handle(handle, "renew", ttl=30) + self.assertEqual(set(renewed), {"status", "deadline"}) + self.assertEqual(renewed["status"], "repository-held" if held else "held") + self.assertIs(type(renewed["deadline"]), float) + self.assertGreater(renewed["deadline"], time.monotonic()) + with self.assertRaises(KeyError): + renewed["repository_held"] + status = broker.request_handle(handle, "status") + self.assertEqual(set(status), status_keys) + self.assertIs(status["repository_held"], held) + self.assertEqual(status["status"], renewed["status"]) + self.assertEqual(status["deadline"], renewed["deadline"]) + self.assertEqual({key: status[key] for key in ("root", "owner", "repository", "issue", "role", "pid")}, + {"root": str(root), "owner": "alice", "repository": "ledger", "issue": 7, + "role": "gitea-review-agent", "pid": proc.pid}) + rejected = broker.request_handle(handle, "renew", ttl=0) + self.assertEqual(rejected, {"error": "invalid ttl_seconds"}) + unchanged = broker.request_handle(handle, "status") + self.assertEqual(unchanged, status) + finally: + try: + if proc.poll() is None: + self.assertEqual(broker.request_handle(handle, "release"), {"status": "released"}) + self.assertEqual(proc.wait(timeout=3), 0) + finally: + if proc.poll() is None: + proc.kill(); proc.wait(timeout=3) + proc.stdout.close(); proc.stderr.close() + self.assertFalse(sock.exists()) + self.assertTrue(probe_free(root / "repositories/alice/ledger/locks/issue-7.lock")) + def test_constructor_rejects_lexical_root_and_handle_aliases(self): with tempfile.TemporaryDirectory() as d: root = Path(d); ensure_cache(root)